Patchwork [U-Boot] common/lcd: fix eldk 4.2 compile warning

login
register
mail settings
Submitter Andreas Bießmann
Date Oct. 23, 2012, 11:45 a.m.
Message ID <1350992711-352-1-git-send-email-andreas.devel@googlemail.com>
Download mbox | patch
Permalink /patch/193466/
State Superseded
Delegated to: Andreas Bießmann
Headers show

Comments

Andreas Bießmann - Oct. 23, 2012, 11:45 a.m.
This fixes following warning only generated with eldk-4.2:

---8<---
lcd.c: In function 'bitmap_plot':
lcd.c:527: warning: dereferencing type-punned pointer will break strict-aliasing rules
--->8---

Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com>
Cc: Marek Vasut <marex@denx.de>
---
 common/lcd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Marek Vasut - Oct. 23, 2012, 12:14 p.m.
Dear Andreas Bießmann,

> This fixes following warning only generated with eldk-4.2:
> 
> ---8<---
> lcd.c: In function 'bitmap_plot':
> lcd.c:527: warning: dereferencing type-punned pointer will break
> strict-aliasing rules --->8---
> 
> Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>  common/lcd.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/lcd.c b/common/lcd.c
> index b6be800..9a4435f 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -524,7 +524,7 @@ static inline ushort *configuration_get_cmap(void)
>  void bitmap_plot(int x, int y)
>  {
>  #ifdef CONFIG_ATMEL_LCD
> -	uint *cmap = (uint *)bmp_logo_palette;
> +	uint *cmap = (void *)bmp_logo_palette;

Are you _sure_ this is the right fix ? Why is the atmel LCD driver so separate 
anyway?

Thanks!

>  #else
>  	ushort *cmap = (ushort *)bmp_logo_palette;
>  #endif

Best regards,
Marek Vasut
Andreas Bießmann - Oct. 23, 2012, 12:37 p.m.
Dear Marek Vasut,

On 23.10.2012 14:14, Marek Vasut wrote:
> Dear Andreas Bießmann,
> 
>> This fixes following warning only generated with eldk-4.2:
>>
>> ---8<---
>> lcd.c: In function 'bitmap_plot':
>> lcd.c:527: warning: dereferencing type-punned pointer will break
>> strict-aliasing rules --->8---
>>
>> Signed-off-by: Andreas Bießmann <andreas.devel@googlemail.com>
>> Cc: Marek Vasut <marex@denx.de>
>> ---
>>  common/lcd.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/lcd.c b/common/lcd.c
>> index b6be800..9a4435f 100644
>> --- a/common/lcd.c
>> +++ b/common/lcd.c
>> @@ -524,7 +524,7 @@ static inline ushort *configuration_get_cmap(void)
>>  void bitmap_plot(int x, int y)
>>  {
>>  #ifdef CONFIG_ATMEL_LCD
>> -	uint *cmap = (uint *)bmp_logo_palette;
>> +	uint *cmap = (void *)bmp_logo_palette;
> 
> Are you _sure_ this is the right fix ? 

yes, I am. This patch is runtime tested on an at91sam9263ek.

I first tried to consolidate the cmap and just use the ushort as in the
else path. But unfortunately this require some more rework of this driver.

> Why is the atmel LCD driver so separate 
> anyway?

Sorry, I don't know. This should really be reworked some time.
But I think for now this quick fix can be applied to silence your
eldk-4.2 warning.

Best regards

Andreas Bießmann
Marek Vasut - Oct. 23, 2012, 10:09 p.m.
Dear Andreas Bießmann,

[..]
> 
> yes, I am. This patch is runtime tested on an at91sam9263ek.
> 
> I first tried to consolidate the cmap and just use the ushort as in the
> else path. But unfortunately this require some more rework of this driver.

Can you do such rework? You're the atmel guy afterall.

> > Why is the atmel LCD driver so separate
> > anyway?
> 
> Sorry, I don't know. This should really be reworked some time.
> But I think for now this quick fix can be applied to silence your
> eldk-4.2 warning.

I don't like hiding problems, can we aim for a proper solution please?

[...]

Best regards,
Marek Vasut
Andreas Bießmann - Oct. 24, 2012, 7:57 a.m.
Dear Marek Vasut,

On 24.10.2012 00:09, Marek Vasut wrote:
> Dear Andreas Bießmann,
> 
> [..]
>>
>> yes, I am. This patch is runtime tested on an at91sam9263ek.
>>
>> I first tried to consolidate the cmap and just use the ushort as in the
>> else path. But unfortunately this require some more rework of this driver.
> 
> Can you do such rework? You're the atmel guy afterall.

I can do such rework, although I have currently not much time to work on
that issue. Until 2013.01 release in January this should be doable but
not in the next few days until MW closes.

>>> Why is the atmel LCD driver so separate
>>> anyway?
>>
>> Sorry, I don't know. This should really be reworked some time.
>> But I think for now this quick fix can be applied to silence your
>> eldk-4.2 warning.
> 
> I don't like hiding problems, can we aim for a proper solution please?

You are right, we should do so.

Best regards

Andreas Bießmann

Patch

diff --git a/common/lcd.c b/common/lcd.c
index b6be800..9a4435f 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -524,7 +524,7 @@  static inline ushort *configuration_get_cmap(void)
 void bitmap_plot(int x, int y)
 {
 #ifdef CONFIG_ATMEL_LCD
-	uint *cmap = (uint *)bmp_logo_palette;
+	uint *cmap = (void *)bmp_logo_palette;
 #else
 	ushort *cmap = (ushort *)bmp_logo_palette;
 #endif