Very Elegant Wolfgang - I like it ... I'll take a stab and I'll try not to
tarnish it!
-M
>
> Looking at the code it seems we have just a list of constants that
> need to get written all to the same mddrc.ddr_command register. To
> make this configurable, we should probably not try to add more such
> stuff, but separate code and data to simplify the design.
>
> I think we should turn the existing code:
>
> /* Initialize DDR */
> for (i = 0; i < 10; i++)
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
>
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM2);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM2);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EM3);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_EN_DLL);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_RFSH);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_INIT_DEV_OP);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_OCD_DEFAULT);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_PCHG_ALL);
> out_be32(&im->mddrc.ddr_command, CONFIG_SYS_MICRON_NOP);
>
> into something like this:
>
> u32 micron_table[] = {
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_NOP,
>
> CONFIG_SYS_MICRON_PCHG_ALL,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_RFSH,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_RFSH,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_INIT_DEV_OP,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_EM2,
> CONFIG_SYS_MICRON_NOP,
> CONFIG_SYS_MICRON_PCHG_ALL,
> CONFIG_SYS_MICRON_EM2,
> CONFIG_SYS_MICRON_EM3,
> CONFIG_SYS_MICRON_EN_DLL,
> CONFIG_SYS_MICRON_INIT_DEV_OP,
> CONFIG_SYS_MICRON_PCHG_ALL,
> CONFIG_SYS_MICRON_RFSH,
> CONFIG_SYS_MICRON_INIT_DEV_OP,
> CONFIG_SYS_MICRON_OCD_DEFAULT,
> CONFIG_SYS_MICRON_PCHG_ALL,
> CONFIG_SYS_MICRON_NOP,
> };
> ...
>
> long int fixed_sdram(u32 ram_table[], int table_size)
> {
> int i;
> ...
> for (i=0; i<table_size; ++i)
> out_be32(&im->mddrc.ddr_sys_config, ram_table[i])
> ...
> }
>
> The board specific code would then be changed from
>
> fixed_sdram()
>
> into for example
>
> fixed_sdram(micron_table,sizeof(micron_table)/sizeof(u32))
>
> In board/freescale/mpc5121ads/mpc5121ads.c, you could provide an
> additional set of data (elpida_table[]), and then pass either of these
> to fixed_sdram().
>
> What do you think?
>
> 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: w...@denx.de
> The optimum committee has no members.
> - Norman Augustine
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
