Dear Martha M Stan,
In message <12535564342520-git-send-email-mm...@silicontkx.com> you wrote:
> Signed-off-by: Martha M Stan <mm...@silicontkx.com>
> ---
> board/davedenx/aria/aria.c | 2 +-
> board/esd/mecp5123/mecp5123.c | 2 +-
> board/freescale/mpc5121ads/mpc5121ads.c | 2 +-
> cpu/mpc512x/fixed_sdram.c | 104 ++++++++++++++++++++----------
> include/asm-ppc/immap_512x.h | 4 +
> include/asm-ppc/mpc512x.h | 2 +-
> include/configs/aria.h | 22 +++----
> include/configs/mecp5123.h | 23 +++----
> include/configs/mpc5121ads.h | 30 ++++-----
> 9 files changed, 109 insertions(+), 82 deletions(-)
Applying: Streamlined mpc512x fixed_sdram init sequence.
/home/wd/git/u-boot/work/.git/rebase-apply/patch:60: trailing whitespace.
/*
/home/wd/git/u-boot/work/.git/rebase-apply/patch:61: trailing whitespace.
* MDDRC Config Runtime Settings in order of the 4 MDDRC cfg registers
/home/wd/git/u-boot/work/.git/rebase-apply/patch:152: trailing whitespace.
/*
/home/wd/git/u-boot/work/.git/rebase-apply/patch:154: trailing whitespace.
* put MDDRC in CMD mode and
/home/wd/git/u-boot/work/.git/rebase-apply/patch:155: trailing whitespace.
* set the max time between refreshes to 0 during init process
warning: 5 lines applied after fixing whitespace errors.
Please pay more attention to Coding Style!
> diff --git a/cpu/mpc512x/fixed_sdram.c b/cpu/mpc512x/fixed_sdram.c
> index d906903..63a3035 100644
> --- a/cpu/mpc512x/fixed_sdram.c
> +++ b/cpu/mpc512x/fixed_sdram.c
> @@ -25,18 +25,70 @@
> #include <asm/io.h>
> #include <asm/mpc512x.h>
>
> +/*
> + * MDDRC Config Runtime Settings in order of the 4 MDDRC cfg registers
> + */
> +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
> +};
Hm... we should move the first entry down... see below...
> -long int fixed_sdram(void)
> +long int fixed_sdram(u32 *mddrc_config, u32 *dram_init_seq, int seq_sz)
> {
> volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
> u32 msize = CONFIG_SYS_DDR_SIZE * 1024 * 1024;
> u32 msize_log2 = __ilog2(msize);
> u32 i;
>
> + /* take default settings and init sequence if necessary */
> + if (mddrc_config == 0)
> + mddrc_config = default_mddrc_config;
Please write NULL when you mean a NULL pointer.
> + if (dram_init_seq == 0) {
> + dram_init_seq = default_init_seq;
> + seq_sz = sizeof(default_init_seq)/sizeof(u32);
> + }
Ditto.
> + /*
> + * Initialize MDDRC
> + * put MDDRC in CMD mode and
> + * set the max time between refreshes to 0 during init process
> + */
> + out_be32(&im->mddrc.ddr_sys_config, mddrc_config[0] |
> MDDRC_SYS_CFG_CMD_MASK);
> + out_be32(&im->mddrc.ddr_time_config0, mddrc_config[1] &
> MDDRC_REFRESH_ZERO_MASK);
> + out_be32(&im->mddrc.ddr_time_config1, mddrc_config[2]);
> + out_be32(&im->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.
To make some progress, I made the changes above, and applied the patch.
Thanks.
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 one charm of marriage is that it makes a life of deception a
neccessity." - Oscar Wilde
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
