On Tue, Jul 28, 2015 at 02:31:42PM +0000, Egli, Samuel wrote: > Hi all, > me again. I was wondering, if somebody of you has time to check these > changes? I would appreciate some feedback. Ultimately, it should > also go upstream but my focus here is reviewing the content of > these changes.
Did you have a chance to look into the thing I mentioned in the other thread, about switching to arch/arm/cpu/armv7/arch_timer.c for am33xx? And wrt the EMIF changes, I'm hoping James can chime in since he knows that block inside and out :) > > Thanks, > > Sam > > > -----Original Message----- > > From: Egli, Samuel > > Sent: Freitag, 17. Juli 2015 13:47 > > To: [email protected]; [email protected]; [email protected] > > Cc: [email protected]; [email protected]; Stefan Roese; Meier, Roger; Senn, > > Joerg > > Subject: [RFC] am33xx: add 600us wait in DDR3 initialization sequence > > > > Hi all, > > the current implementation in ./arch/arm/cpu/armv7/am33xx/emif4.c does not > > respect the the initialization steps as documented in the DDR3 > > specification [1]. We noticed that for our am335x based siemens boards a > > delay after config_ddr(...) call was necessary otherwise boot would fail. > > See 61159b76844437bf9004c3a38b5a4ff1a24860d5 > > commit that added a 2ms delay which was merly combating symptoms. > > > > This is because SDRAM wouldn't be ready yet. We didn't realy know why it > > should be necessary but the delay did the job. However, we figured out now > > that an important wait time is not respected, specifically, step 4: > > > > "After RESET# transitions HIGH, wait 500µs (minus one clock) with CKE LOW" > > [1] > > > > We also noticed that it is vendor specific and some SDRAM wouldn't bother > > if > > 500 us wait is done or not. > > > > See below the patch that guarantees that we have a 600µs wait time before > > the clock enable gets enabled. Maybe a comment on the main steps: > > > > (1) The first time we call config_sdram the CKE controll is still refused > > EMIF/DDR PHY. But it has the effect that the init sequence is started > > anyway, and as a consequence puts RESET to high which is the sole goal > > of this line. > > > > In (2) + (3) we wait 600us. 100us more to be on the safe side and then > > give control to EMIF/DDR PHY. > > > > (4) Now, that the wait time is ensured and CKE can be used by the EMIF/DDR > > PHY > > we trigger the real sdram configuration sequence. > > > > > > This patch works fine for us but I'm not sure if some revising of the > > EMIF_4D5 case is necessary, too. With this patch no delay after > > config_ddr(...) is necessary anymore. > > > > One thing that I cannot explain well, however, is why no issue was > > observed with other TI boards. A possible explenation is that some delay > > stems from code execution time that is done after sdram config before > > loading u-boot. > > > > But maybe it has also to do with different DDR3 vendors being used. Our > > observation was that Samsung SDRAM is more likely to not work. And that > > smaller sized SDRAM is less/not prone to ignoring the spec. I.e. SDRAM > > with only 128 MB didn't cause any problem. But 256/512 MB from Samsung > > would not work. > > > > Kind regards > > > > Sam > > > > [1] http://www.micron.com/products/datasheets/%7B2ADCEEB1-307F-4B37-99C9- > > 7302CAA8BC5C%7D?page=8#topic=c_Initialization > > > > > > --- > > arch/arm/cpu/armv7/am33xx/emif4.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/arch/arm/cpu/armv7/am33xx/emif4.c > > b/arch/arm/cpu/armv7/am33xx/emif4.c > > index 9cf816c..084b45a 100644 > > --- a/arch/arm/cpu/armv7/am33xx/emif4.c > > +++ b/arch/arm/cpu/armv7/am33xx/emif4.c > > @@ -109,10 +109,6 @@ void config_ddr(unsigned int pll, const struct > > ctrl_ioregs *ioregs, > > config_ddr_data(data, nr); > > #ifdef CONFIG_AM33XX > > config_io_ctrl(ioregs); > > - > > - /* Set CKE to be controlled by EMIF/DDR PHY */ > > - writel(DDR_CKE_CTRL_NORMAL, &ddrctrl->ddrckectrl); > > - > > #endif > > #ifdef CONFIG_AM43XX > > writel(readl(&cm_device->cm_dll_ctrl) & ~0x1, &cm_device- > > >cm_dll_ctrl); @@ -131,9 +127,21 @@ void config_ddr(unsigned int pll, > > const struct ctrl_ioregs *ioregs, > > /* Program EMIF instance */ > > config_ddr_phy(regs, nr); > > set_sdram_timings(regs, nr); > > + > > if (get_emif_rev(EMIF1_BASE) == EMIF_4D5) > > config_sdram_emif4d5(regs, nr); > > - else > > + else { > > + /* (1) Set reset signal with CKE still off */ > > + config_sdram(regs, nr); > > + > > + /* (2) Add 600us delay between Reset and CKE */ > > + udelay(600); > > + > > + /* (3) Set CKE to be controlled by EMIF/DDR PHY */ > > + writel(DDR_CKE_CTRL_NORMAL, &ddrctrl->ddrckectrl); > > + > > + /* (4) Configure sdram */ > > config_sdram(regs, nr); > > + } > > } > > #endif > > -- > > 1.7.10.4 -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
