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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
