[U-Boot] [PATCH] powerpc: fix 8xx and 82xx type-punning warnings with GCC 4.7

[Wolfgang Denk <wd@denx.de>]

Dear Scott,

In message <1357696756-31079-1-git-send-email-scottwood@freescale.com> you wrote:
> C99's strict aliasing rules are insane to use in low-level code such as a
> bootloader, but as Wolfgang has rejected -fno-strict-aliasing in the
> past, add a union so that 16-bit accesses can be performed.

Sorry, I saw this patch only after re-inventing the fix for 8xx.

>  #ifdef CONFIG_HARD_I2C
> -	*((unsigned short*)(&immr->im_dprambase[PROFF_I2C_BASE])) = 0;
> +	immr->im_dprambase16[PROFF_I2C_BASE / 2] = 0;

I have to admit that I dislike this approach pretty much.

I think we agree that, if we attempted to play strictly by the rules,
this code should probably rewritten using C structs and proper I/O
accessors.  But then, both 8xx and 82xx are essentially dead horses,
and I guess you have no more enthusiasm in cleaning up that old code
than me.  So let's ignore that for now.

But this "...[OFFSET / 2]" is basicly unreadable.  Can we please at
least make this  "...[OFFSET / sizeof(u16)]" so the reader gets a hint
of where this is coming from?

> --- a/arch/powerpc/cpu/mpc8xx/cpu.c
> +++ b/arch/powerpc/cpu/mpc8xx/cpu.c
> @@ -79,7 +79,7 @@ static int check_CPU (long clock, uint pvr, uint immr)
>  	if ((pvr >> 16) != 0x0050)
>  		return -1;
>  
> -	k = (immr << 16) | *((ushort *) & immap->im_cpm.cp_dparam[0xB0]);
> +	k = (immr << 16) | immap->im_cpm.cp_dparam16[0xB0 / 2];
>  	m = 0;
>  	suf = "";
>  
> @@ -195,7 +195,7 @@ static int check_CPU (long clock, uint pvr, uint immr)
>  	if ((pvr >> 16) != 0x0050)
>  		return -1;
>  
> -	k = (immr << 16) | *((ushort *) & immap->im_cpm.cp_dparam[0xB0]);
> +	k = (immr << 16) | in_be16((ushort *)&immap->im_cpm.cp_dparam[0xB0]);

Now this is very inconsistent - you convert the very same code line in
very different ways here.  Please don't.

Is there any specific reason you did not use a similar approach of
using in_be16() in the other locations?  Actually I feel this is still
the most readable version - I prefer this, even though it clashes
with the style of the rest of the code.

Oh, and can we please get rid of this magic number 0xB0 here, and
introduce PROFF_REVNUM like we do everywhere else?

See http://patchwork.ozlabs.org/patch/226185/ for the needed addition
to arch/powerpc/include/asm/8xx_immap.h

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Uncertain fortune is thoroughly mastered by the equity of the  calcu-
lation.                                               - Blaise Pascal