[U-Boot] [PATCH] Streamlined mpc512x fixed_sdram init sequence.

[m marx]

Yes Wolfgang,

Sorry about the sloppy little problems with my 2 patches.  I need to pay
more attention to coding style ...

As far as the weirdness in array index vs immap struct naming mismatch ...

>  +u32 default_mddrc_config[4] = {
>  +CONFIG_SYS_MDDRC_SYS_CFG,
>  +CONFIG_SYS_MDDRC_TIME_CFG0,
>  +CONFIG_SYS_MDDRC_TIME_CFG1,
>  +CONFIG_SYS_MDDRC_TIME_CFG2
 
>  +/* 
>  + * Initialize MDDRC
>  + * put MDDRC in CMD mode and 
>  + * set the max time between refreshes to 0 during init process 
>  + */
>  +out_be32(mddrc.ddr_sys_config, mddrc_config[0] | MDDRC_SYS_CFG_CMD_MASK);
>  +out_be32(mddrc.ddr_time_config0, mddrc_config[1] & MDDRC_REFRESH_ZERO_MASK);
>  +out_be32(mddrc.ddr_time_config1, mddrc_config[2]);
>  +out_be32(mddrc.ddr_time_config2, mddrc_config[3]);

> I cannot help it, but every time I see this I think the code is
> wrong. Guess I have seen too many copy & paste errors in this style.
> When I see "...time_config2" I also want to see "mddrc_config[2]", i. e.
> identical indices. We should reorder the struct.

The 4 memory config registers are in memory order and unfortunatley the
register names throw you off.   Some hardware guy did this in the register
naming -- obviously !!!   So even though the array index doesn't
match the constant it does makes sense. 
What I think I should do is ... instead of default_mddrc_config calling my
array default_mddrc_reg ... or create an independant struct with just
these 4 regs and name them the same as the immap struct.   Actually -- I
like the later idea best .... I might do this soon and when I redo my 5125
patch incorporate it.

Very Bestest,
Martha