On Thu, Dec 15, 2022 at 10:54:00AM +0100, Matthias Feser wrote:
> Hi,
>
> we are using the AM3352 processor in combination with a single 512MB
> Micron DDR3 chip running the barebox bootloader in our products for
> several years now. A minor amount of boards (around 5 of 2000) fail
> the production test, because they do not boot properly after warm
> reset. In such cases the MLO is loaded, initializes the EMIF and then
> crashes after a certain amount of accesses to the DDR3. After a cold
> reset all of these boards run stable and produce no errors when
> running a deep RAM test.
>
> I am currently in discussion with a TI employee about this topic. He
> told me that the bootloader should detect a warm reset and EMIF should
> not be reinitialized, because DDR3 is automatically put into
> self-refresh on warm reset. So far he hasn?t told me what the
> desired init sequence actually is. From what I have observed while
> debugging, at least the EMIF clock has to be enabled and CKE brought
> high. Unfortunately the TRM does not give guidance about this.
>
> Our board initialization code is very similar to other AM335x based
> boards like beaglebone (400MHz DDR clock), which effectively always
> initializes the EMIF in the same way by calling am335x_sdram_init(),
> no matter if cold or warm reset has brought up the processor. From
> investigating the signals DDR_RESET and DDR_CKE with an oscilloscope,
> I can tell that even with this same init code the hardware behaves
> differently in both reset cases.
>
> On cold reset both DDR_RESET and DDR_CKE remain low until
> initialization, and there is a delay of roughly 436us between the
> rising edges of DDR_RESET and DDR_CKE. After warm reset DDR_RESET is
> high and DDR_CKE is low. EMIF initialization results in a short pulse
> on DDR_RESET with 5us low phase and there is only about 38us delay
> between the rising edges of DDR_RESET and DDR_CKE.
> Both cases violate the DDR3 specification, according to which the
> delay between the rising edges of DDR_RESET and DDR_CKE has to be
> 500us min.
>
> In am33xx_config_sdram() a value of 0x2800 is written to
> EMIF4_SDRAM_REF_CTRL. TI recommends a value of 0x3100 during
> initialization, which is used in u-boot EMIF initialization code and
> does not violate the DDR3 specification. I think barebox EMIF init
> code requires some revision.
>
> I also wonder why EMIF4_SDRAM_CONFIG, REF_CTRL and REF_CTRL_SHADOW are
> accessed twice when regs->zq_config is not zero (see code snippet
> below taken from barebox 2022.12.0). Is there any reason behind this?
The code is from U-Boot.
>
> if (regs->zq_config) {
> /*
> * A value of 0x2800 for the REF CTRL will give us
> * about 570us for a delay, which will be long enough
> * to configure things.
> */
> writel(0x2800, emif4 + EMIF4_SDRAM_REF_CTRL);
> writel(regs->zq_config, emif4 + EMIF4_ZQ_CONFIG);
> writel(regs->sdram_config, CM_EMIF_SDRAM_CONFIG);
> writel(regs->sdram_config, emif4 + EMIF4_SDRAM_CONFIG);
> writel(regs->sdram_ref_ctrl, emif4 + EMIF4_SDRAM_REF_CTRL);
> writel(regs->sdram_ref_ctrl, emif4 + EMIF4_SDRAM_REF_CTRL_SHADOW);
> }
>
> writel(regs->sdram_ref_ctrl, emif4 + EMIF4_SDRAM_REF_CTRL);
> writel(regs->sdram_ref_ctrl, emif4 + EMIF4_SDRAM_REF_CTRL_SHADOW);
> writel(regs->sdram_config, emif4 + EMIF4_SDRAM_CONFIG);
>
The second access to EMIF4_SDRAM_REF_CTRL goes down to U-Boot commit
1c382ead7a00 ("am33xx: Update DDR3 EMIF configuration sequence"). This
commit also introduces the 0x2800 value we still find in barebox.
The 0x2800 value was removed in fc46bae2ae38 ("arm: am437x: Enable
hardware leveling for EMIF"). In this commit we also see the value
0x3100 for the first time.
I can't say much more to this, except that I am happy to accept patches
to clean that up.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
