Hi, > On July 2, 2020 at 11:25 AM Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> wrote: > > > > > On 7/1/20 11:52 AM, Giorgio Dal Molin wrote: > > Hi, > > > >> On June 29, 2020 at 6:39 PM Fabio Estevam <festevam@xxxxxxxxx> wrote: > >> > >> > >> Hi Giorgio, > >> > >> On Mon, Jun 29, 2020 at 1:33 PM Giorgio Dal Molin > >> <giorgio.nicole@xxxxxxxx> wrote: > >> > >>> U-Boot configures the ddr3 with c code in its board code 'lowlevel.c'. > >>> Looking at the code I noticed this special treatment: > >>> > >>> static void spl_dram_init(void) > >>> { > >>> ... > >>> /* > >>> * Make sure that both aresetn/core_ddrc_rstn and preset/PHY reset > >>> * bits are set after WDOG reset event. DDRC_PRST can only be > >>> * released when DDRC clock inputs are stable for at least 30 cycles. > >>> */ > >>> writel(SRC_DDRC_RCR_DDRC_CORE_RST_MASK | SRC_DDRC_RCR_DDRC_PRST_MASK, &src_regs->ddrc_rcr); > >>> udelay(500); > >>> ... > >>> > >>> This writel() set both reset bits, the DDRC_CORE (0x2) and the DDRC_PRST (0x1) of the SRC > >>> register 0x30391000. > >>> Unfortunately, if I try also to set both bits in my DCD table then barebox doesn't boot anymore; > >>> I also tried to port the uboot spl_dram_init(void) to my barebox lowlevel.c and I could eventually > >>> boot barebox with an empty DCD but still adding the second bit (SRC_DDRC_RCR_DDRC_PRST_MASK) > >>> hangs the soc. > >> > >> Does it help if you try to apply this U-Boot commit to Barebox? > >> https://gitlab.denx.de/u-boot/u-boot/-/commit/0e06d63d195670f5181958f43216d7106c05357f > > > > I've made some more tests with the imx7d and found that the following DCD sequence: > > > > wm 32 0x30391000 0x00000003 // <== added this write > > wm 32 0x30391000 0x00000002 > > ... > > > > have an (unreliable) effect: I can now some time reboot barebox with a 'reset' > > command and after the reboot I can see the correct reset reason on the serial console: > > > > barebox 2020.06.0-00327-g712fde835-dirty #2 Wed Jul 1 10:21:11 CEST 2020 > > > > Board: Kontron SMARC-sAMX7 > > detected i.MX7d revision 1.3 > > i.MX reset reason POR (SRSR: 0x00000001) > > ... > > > > samx7: / > > samx7: / reset > > > > barebox 2020.06.0-00327-g712fde835-dirty #2 Wed Jul 1 10:21:11 CEST 2020 > > > > Board: Kontron SMARC-sAMX7 > > detected i.MX7d revision 1.3 > > i.MX reset reason WDG (SRSR: 0x00000010) > > mdio_bus: miibus0: probed > > ... > > > > samx7: / > > > > > > This is the first time I see a reset working on my imx7 module with barebox; the > > problem is now that the reboot process is not reliable: it works a couple of times > > (not deterministic) and then it hangs the soc forcing me to push the reset button. > > > > As a possible fix I tried adding some 'nop' in the DCD around the two wm 32 to simulate > > a delay but it makes no difference. > > IIRC, you can poll an address for a set bit in the DCD table for N times. If you poll > something that doesn't change, you can adjust N to simulate a delay.. > I've already had the same idea, the 'check data' DCD 'command' has an optional [count] argument (IMX7 dev. man. 6.6.7.2.2)... To use it I had to patch the barebox tool 'scripts/imx/imx-image' to handle the new parameter; now I can write something like: wm 32 0x30340004 0x4F400005 wm 32 0x30391000 0x00000003 // SRC_DDRC_RCR: DDR Controller Reset Control - enable reset check 32 until_all_bits_set 0x30340004 0xffffffff 0xffff wm 32 0x30391000 0x00000002 // SRC_DDRC_RCR: DDR Controller Reset Control - enable reset in my DCD. Unfortunately it does not work, the soc does not boot at all. It could be that the ROM code performs the 'count' dummy checks on my reg. (0x30340004), finds them all unsuccessful and hangs the system at the end. If I change the check to: check 32 until_all_bits_set 0x30340004 0x0f000000 0xffff than it boots normally (but the reset is unreliable). Comparing again the u-boot and barebox code I've found another possible place that effectively makes a difference: u-boot, after triggering the watchdog goes directly in an endless loop: void __attribute__((weak)) reset_cpu(ulong addr) { struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR; clrsetbits_le16(&wdog->wcr, WCR_WT_MSK, WCR_WDE); writew(0x5555, &wdog->wsr); writew(0xaaaa, &wdog->wsr); /* load minimum 1/2 second timeout */ while (1) { /* * spin for .5 seconds before reset */ } } barebox does something similar to trigger the wdog but afterwards also calls mdelay() and a printf() before falling in the endless loop: static void imx21_soc_reset(struct imx_wd *priv) { ... imxwd_write(priv, IMX21_WDOG_WCR, val); /* Two additional writes due to errata ERR004346 */ imxwd_write(priv, IMX21_WDOG_WCR, val); imxwd_write(priv, IMX21_WDOG_WCR, val); ... mdelay(1000); hang(); // <== this also calls printf() before for(;;); What I've found is that if I put the endless loop right at the end of imx21_soc_reset(), after the imxwd_write's, then the reboot process becomes reliable. (The changes in the DCD with the addition of the write: wm 32 0x30391000 0x00000003 at the beginning, before the write: wm 32 0x30391000 0x00000002 are also essential, only the delay between the two seems to be not a big deal). giorgio > > > > giorgio > > > >> > >> _______________________________________________ > >> barebox mailing list > >> barebox@xxxxxxxxxxxxxxxxxxx > >> http://lists.infradead.org/mailman/listinfo/barebox > > > > -- > 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 | > > _______________________________________________ > barebox mailing list > barebox@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/barebox _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox