mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Giorgio Dal Molin <giorgio.nicole@arcor.de>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>
Cc: Barebox List <barebox@lists.infradead.org>
Subject: Re: reset / watchdog on an imx7d soc
Date: Thu, 2 Jul 2020 17:28:07 +0200 (CEST)	[thread overview]
Message-ID: <936755248.7359.1593703689870@mail.vodafone.de> (raw)
In-Reply-To: <b095fd3c-7cda-1394-ba4d-6dd882606920@pengutronix.de>

Hi,

> On July 2, 2020 at 11:25 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
> 
> 
> 
> On 7/1/20 11:52 AM, Giorgio Dal Molin wrote:
> > Hi,
> > 
> >> On June 29, 2020 at 6:39 PM Fabio Estevam <festevam@gmail.com> wrote:
> >>
> >>
> >> Hi Giorgio,
> >>
> >> On Mon, Jun 29, 2020 at 1:33 PM Giorgio Dal Molin
> >> <giorgio.nicole@arcor.de> 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@lists.infradead.org
> >> 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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  parent reply	other threads:[~2020-07-02 15:28 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-23 13:45 Giorgio Dal Molin
2020-06-23 13:53 ` Fabio Estevam
2020-06-23 15:11   ` Giorgio Dal Molin
2020-06-29  8:44     ` Ahmad Fatoum
2020-06-29 10:53       ` Giorgio Dal Molin
2020-06-29 13:30         ` Ahmad Fatoum
2020-06-29 15:30           ` Giorgio Dal Molin
2020-06-29 15:35             ` Ahmad Fatoum
2020-06-29 16:03               ` Giorgio Dal Molin
2020-06-29 16:11                 ` Ahmad Fatoum
2020-06-29 16:33                   ` Giorgio Dal Molin
2020-06-29 16:39                     ` Fabio Estevam
2020-07-01  9:52                       ` Giorgio Dal Molin
2020-07-02  9:25                         ` Ahmad Fatoum
2020-07-02 14:51                           ` Giorgio Dal Molin
2020-07-02 15:28                           ` Giorgio Dal Molin [this message]
2020-07-02 16:05                             ` Lucas Stach
2020-07-03 14:13                               ` Giorgio Dal Molin
2020-07-02 16:24                             ` Fabio Estevam
2020-07-07  5:52                               ` Giorgio Dal Molin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=936755248.7359.1593703689870@mail.vodafone.de \
    --to=giorgio.nicole@arcor.de \
    --cc=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=festevam@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox