| Message ID | 1320921327-21855-1-git-send-email-holger.brunck@keymile.com |
|---|---|
| State | Superseded |
| Delegated to: | Kim Phillips |
| Headers | show |
Hello Holger, Holger Brunck wrote: > From: Andreas Huber <andreas.huber@keymile.com> > > commit b11f53f3 (keymile: Fix Coding style issues for keymile boards) > introduces a bug according the SDRAM initialization for all > km83xx boards. > > im->ddr.sdram_cfg |= SDRAM_CFG_MEM_EN; > was replaced with > out_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); > and this is wrong, because this overwrites the intial value > CONFIG_SYS_DDR_SDRAM_CFG. > > Signed-off-by: Andreas Huber <andreas.huber@keymile.com> > Signed-off-by: Holger Brunck <holger.brunck@keymile.com> > cc: Heiko Schocher <hs@denx.de> > cc: Kim Phillips <kim.phillips@freescale.com> > --- > board/keymile/km83xx/km83xx.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/board/keymile/km83xx/km83xx.c b/board/keymile/km83xx/km83xx.c > index 17560c8..c0238c8 100644 > --- a/board/keymile/km83xx/km83xx.c > +++ b/board/keymile/km83xx/km83xx.c > @@ -217,7 +217,8 @@ int fixed_sdram(void) > out_be32(&im->ddr.sdram_interval, CONFIG_SYS_DDR_INTERVAL); > out_be32(&im->ddr.sdram_clk_cntl, CONFIG_SYS_DDR_CLK_CNTL); > udelay(200); > - out_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); > + out_be32(&im->ddr.sdram_cfg, > + SDRAM_CFG_MEM_EN | CONFIG_SYS_DDR_SDRAM_CFG); Wouldn't a setbits_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); be better? > > msize = CONFIG_SYS_DDR_SIZE << 20; > disable_addr_trans(); bye, Heiko
Hi Heiko, On 11/10/2011 12:59 PM, Heiko Schocher wrote: > Holger Brunck wrote: >> From: Andreas Huber <andreas.huber@keymile.com> >> >> commit b11f53f3 (keymile: Fix Coding style issues for keymile boards) >> introduces a bug according the SDRAM initialization for all >> km83xx boards. >> >> im->ddr.sdram_cfg |= SDRAM_CFG_MEM_EN; >> was replaced with >> out_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); >> and this is wrong, because this overwrites the intial value >> CONFIG_SYS_DDR_SDRAM_CFG. >> >> Signed-off-by: Andreas Huber <andreas.huber@keymile.com> >> Signed-off-by: Holger Brunck <holger.brunck@keymile.com> >> cc: Heiko Schocher <hs@denx.de> >> cc: Kim Phillips <kim.phillips@freescale.com> >> --- >> board/keymile/km83xx/km83xx.c | 3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/board/keymile/km83xx/km83xx.c b/board/keymile/km83xx/km83xx.c >> index 17560c8..c0238c8 100644 >> --- a/board/keymile/km83xx/km83xx.c >> +++ b/board/keymile/km83xx/km83xx.c >> @@ -217,7 +217,8 @@ int fixed_sdram(void) >> out_be32(&im->ddr.sdram_interval, CONFIG_SYS_DDR_INTERVAL); >> out_be32(&im->ddr.sdram_clk_cntl, CONFIG_SYS_DDR_CLK_CNTL); >> udelay(200); >> - out_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); >> + out_be32(&im->ddr.sdram_cfg, >> + SDRAM_CFG_MEM_EN | CONFIG_SYS_DDR_SDRAM_CFG); > > Wouldn't a > > setbits_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); > > be better? > yes this works too and is slightly nicer, I will send an updated version. Thanks! Best regards Holger
Hello Holger, Holger Brunck wrote: > Hi Heiko, > > On 11/10/2011 12:59 PM, Heiko Schocher wrote: >> Holger Brunck wrote: >>> From: Andreas Huber <andreas.huber@keymile.com> >>> >>> commit b11f53f3 (keymile: Fix Coding style issues for keymile boards) >>> introduces a bug according the SDRAM initialization for all >>> km83xx boards. >>> >>> im->ddr.sdram_cfg |= SDRAM_CFG_MEM_EN; >>> was replaced with >>> out_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); >>> and this is wrong, because this overwrites the intial value >>> CONFIG_SYS_DDR_SDRAM_CFG. >>> >>> Signed-off-by: Andreas Huber <andreas.huber@keymile.com> >>> Signed-off-by: Holger Brunck <holger.brunck@keymile.com> >>> cc: Heiko Schocher <hs@denx.de> >>> cc: Kim Phillips <kim.phillips@freescale.com> >>> --- >>> board/keymile/km83xx/km83xx.c | 3 ++- >>> 1 files changed, 2 insertions(+), 1 deletions(-) >>> >>> diff --git a/board/keymile/km83xx/km83xx.c b/board/keymile/km83xx/km83xx.c >>> index 17560c8..c0238c8 100644 >>> --- a/board/keymile/km83xx/km83xx.c >>> +++ b/board/keymile/km83xx/km83xx.c >>> @@ -217,7 +217,8 @@ int fixed_sdram(void) >>> out_be32(&im->ddr.sdram_interval, CONFIG_SYS_DDR_INTERVAL); >>> out_be32(&im->ddr.sdram_clk_cntl, CONFIG_SYS_DDR_CLK_CNTL); >>> udelay(200); >>> - out_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); >>> + out_be32(&im->ddr.sdram_cfg, >>> + SDRAM_CFG_MEM_EN | CONFIG_SYS_DDR_SDRAM_CFG); >> Wouldn't a >> >> setbits_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); >> >> be better? >> > > yes this works too and is slightly nicer, I will send an updated version. Ok, thanks! Please fell free to add my Acked-by: Heiko Schocher<hs@denx.de> to it! bye, Heiko
diff --git a/board/keymile/km83xx/km83xx.c b/board/keymile/km83xx/km83xx.c index 17560c8..c0238c8 100644 --- a/board/keymile/km83xx/km83xx.c +++ b/board/keymile/km83xx/km83xx.c @@ -217,7 +217,8 @@ int fixed_sdram(void) out_be32(&im->ddr.sdram_interval, CONFIG_SYS_DDR_INTERVAL); out_be32(&im->ddr.sdram_clk_cntl, CONFIG_SYS_DDR_CLK_CNTL); udelay(200); - out_be32(&im->ddr.sdram_cfg, SDRAM_CFG_MEM_EN); + out_be32(&im->ddr.sdram_cfg, + SDRAM_CFG_MEM_EN | CONFIG_SYS_DDR_SDRAM_CFG); msize = CONFIG_SYS_DDR_SIZE << 20; disable_addr_trans();