mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* reset / watchdog on an imx7d soc
@ 2020-06-23 13:45 Giorgio Dal Molin
  2020-06-23 13:53 ` Fabio Estevam
  0 siblings, 1 reply; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-06-23 13:45 UTC (permalink / raw)
  To: barebox

Hi,

I'm having problems with the reset command on an imx7d soc.

As far as I understand it, to reboot barebox on an imx7 the 'reset' command
invokes the watchdog1, forcing it to do its job:

in drivers/watchdog/imxwd.c:
static void imx21_soc_reset(struct imx_wd *priv)
...
		val |= IMX21_WDOG_WCR_WDA; /* do not assert ext-reset */

	imxwd_write(priv, IMX21_WDOG_WCR, val);
...


What I see is that shortly after the imxwd_write() call the soc hangs but it
doesn't reboot.

Can anyone confirm that the current barebox is able to restart an imx7 soc without
using external signals ?

Do I need to enable / configure some other soc controller ?

Currently I just enable the wdog1 in my device tree:

...
&wdog1 {
	status = "okay";
};
...

giorgio

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-23 13:45 reset / watchdog on an imx7d soc Giorgio Dal Molin
@ 2020-06-23 13:53 ` Fabio Estevam
  2020-06-23 15:11   ` Giorgio Dal Molin
  0 siblings, 1 reply; 20+ messages in thread
From: Fabio Estevam @ 2020-06-23 13:53 UTC (permalink / raw)
  To: Giorgio Dal Molin; +Cc: Barebox List

Hi Giorgio,

On Tue, Jun 23, 2020 at 10:48 AM Giorgio Dal Molin
<giorgio.nicole@arcor.de> wrote:

> Can anyone confirm that the current barebox is able to restart an imx7 soc without
> using external signals ?

You really need to use the WDOG_B pin to trigger the reset due to an
i.MX7 erratum.

Please check the i.MX7 errata document:
https://www.nxp.com/docs/en/errata/IMX7DS_3N09P.pdf

Search for "e10574 Watchdog: A watchdog timeout or software trigger
will not reset the SOC"

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-23 13:53 ` Fabio Estevam
@ 2020-06-23 15:11   ` Giorgio Dal Molin
  2020-06-29  8:44     ` Ahmad Fatoum
  0 siblings, 1 reply; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-06-23 15:11 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Barebox List

Hi Fabio,

thank you for your quick reply.

I've already found the errata you linked in your mail but I had no success
applying the suggestion there; maybe I'm doing it wrong.
Let's take the Option 3 there:

  Use the SNVS LPCR register to turn off the system power

It suggests to set the SNVS_LPCR[TOP] bit together with SNVS_LPCR[DP_EN], this
would mean something like:

# mw -l 0x30370038 0x00000060

But the effect of this write is to hang the soc.

The same happens with the Option 2 in the errata:

   Use SRC_A7RCR0[A7_CORE_POR_RESET0] to reset the ARM A7.

To do this I use:

# mw -l 0x30390004 0x00000003

but again the soc just hangs as with the watchdog.

giorgio

> On June 23, 2020 at 3:53 PM Fabio Estevam <festevam@gmail.com> wrote:
> 
> 
> Hi Giorgio,
> 
> On Tue, Jun 23, 2020 at 10:48 AM Giorgio Dal Molin
> <giorgio.nicole@arcor.de> wrote:
> 
> > Can anyone confirm that the current barebox is able to restart an imx7 soc without
> > using external signals ?
> 
> You really need to use the WDOG_B pin to trigger the reset due to an
> i.MX7 erratum.
> 
> Please check the i.MX7 errata document:
> https://www.nxp.com/docs/en/errata/IMX7DS_3N09P.pdf
> 
> Search for "e10574 Watchdog: A watchdog timeout or software trigger
> will not reset the SOC"
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-23 15:11   ` Giorgio Dal Molin
@ 2020-06-29  8:44     ` Ahmad Fatoum
  2020-06-29 10:53       ` Giorgio Dal Molin
  0 siblings, 1 reply; 20+ messages in thread
From: Ahmad Fatoum @ 2020-06-29  8:44 UTC (permalink / raw)
  To: Giorgio Dal Molin, Fabio Estevam; +Cc: Barebox List

Hello Giorgio,

On 6/23/20 5:11 PM, Giorgio Dal Molin wrote:
> Hi Fabio,
> 
> thank you for your quick reply.
> 
> I've already found the errata you linked in your mail but I had no success
> applying the suggestion there; maybe I'm doing it wrong.
> Let's take the Option 3 there:

Does reset within Linux (with say the imx_v6_v7_defconfg) work?

-- 
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-29  8:44     ` Ahmad Fatoum
@ 2020-06-29 10:53       ` Giorgio Dal Molin
  2020-06-29 13:30         ` Ahmad Fatoum
  0 siblings, 1 reply; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-06-29 10:53 UTC (permalink / raw)
  To: Ahmad Fatoum, Fabio Estevam; +Cc: Barebox List

Hi,

> On June 29, 2020 at 10:44 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
> 
> Hello Giorgio,
> 
> On 6/23/20 5:11 PM, Giorgio Dal Molin wrote:
> > Hi Fabio,
> > 
> > thank you for your quick reply.
> > 
> > I've already found the errata you linked in your mail but I had no success
> > applying the suggestion there; maybe I'm doing it wrong.
> > Let's take the Option 3 there:
> 
> Does reset within Linux (with say the imx_v6_v7_defconfg) work?

No. I see the same behavior also with the kernel: after a 'shutdown -r'
the system 'goes down' but doesn't reboot, it just hang:

~ # shutdown -r
system is going down for reboot NOW
...
[   46.248222] 000: ci_hdrc ci_hdrc.0: remove, state 1
[   46.248253] 000: usb usb1: USB disconnect, device number 1
[   46.248263] 000: usb 1-1: USB disconnect, device number 2


The same happens if I kill the 'watchdog' process: after ~2 seconds
the system just hangs.

I'm sure there must be a solution because the original u-boot bootloader
for the cpu module I'm using reboots the system as expected, through the wdog1;
unfortunately it's not so trivial to find what makes the difference.

giorgio


> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-29 10:53       ` Giorgio Dal Molin
@ 2020-06-29 13:30         ` Ahmad Fatoum
  2020-06-29 15:30           ` Giorgio Dal Molin
  0 siblings, 1 reply; 20+ messages in thread
From: Ahmad Fatoum @ 2020-06-29 13:30 UTC (permalink / raw)
  To: Giorgio Dal Molin, Fabio Estevam; +Cc: Barebox List

Hello,

On 6/29/20 12:53 PM, Giorgio Dal Molin wrote:
> Hi,
> 
>> On June 29, 2020 at 10:44 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>>
>> Hello Giorgio,
>>
>> On 6/23/20 5:11 PM, Giorgio Dal Molin wrote:
>>> Hi Fabio,
>>>
>>> thank you for your quick reply.
>>>
>>> I've already found the errata you linked in your mail but I had no success
>>> applying the suggestion there; maybe I'm doing it wrong.
>>> Let's take the Option 3 there:
>>
>> Does reset within Linux (with say the imx_v6_v7_defconfg) work?
> 
> No. I see the same behavior also with the kernel: after a 'shutdown -r'
> the system 'goes down' but doesn't reboot, it just hang:
> 
> ~ # shutdown -r
> system is going down for reboot NOW
> ...
> [   46.248222] 000: ci_hdrc ci_hdrc.0: remove, state 1
> [   46.248253] 000: usb usb1: USB disconnect, device number 1
> [   46.248263] 000: usb 1-1: USB disconnect, device number 2
> 
> 
> The same happens if I kill the 'watchdog' process: after ~2 seconds
> the system just hangs.
> 
> I'm sure there must be a solution because the original u-boot bootloader
> for the cpu module I'm using reboots the system as expected, through the wdog1;
> unfortunately it's not so trivial to find what makes the difference.

Linux probably attempts reset via PSCI. Check for psci_system_reset
in your vendor's U-Boot and transplant it into barebox.

> 
> giorgio
> 
> 
>>
>> -- 
>> 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
> 

-- 
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-29 13:30         ` Ahmad Fatoum
@ 2020-06-29 15:30           ` Giorgio Dal Molin
  2020-06-29 15:35             ` Ahmad Fatoum
  0 siblings, 1 reply; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-06-29 15:30 UTC (permalink / raw)
  To: Ahmad Fatoum, Fabio Estevam; +Cc: Barebox List

Hi,

> On June 29, 2020 at 3:30 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
> 
> Hello,
> 
> On 6/29/20 12:53 PM, Giorgio Dal Molin wrote:
> > Hi,
> > 
> >> On June 29, 2020 at 10:44 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>
> >>
> >> Hello Giorgio,
> >>
> >> On 6/23/20 5:11 PM, Giorgio Dal Molin wrote:
> >>> Hi Fabio,
> >>>
> >>> thank you for your quick reply.
> >>>
> >>> I've already found the errata you linked in your mail but I had no success
> >>> applying the suggestion there; maybe I'm doing it wrong.
> >>> Let's take the Option 3 there:
> >>
> >> Does reset within Linux (with say the imx_v6_v7_defconfg) work?
> > 
> > No. I see the same behavior also with the kernel: after a 'shutdown -r'
> > the system 'goes down' but doesn't reboot, it just hang:
> > 
> > ~ # shutdown -r
> > system is going down for reboot NOW
> > ...
> > [   46.248222] 000: ci_hdrc ci_hdrc.0: remove, state 1
> > [   46.248253] 000: usb usb1: USB disconnect, device number 1
> > [   46.248263] 000: usb 1-1: USB disconnect, device number 2
> > 
> > 
> > The same happens if I kill the 'watchdog' process: after ~2 seconds
> > the system just hangs.
> > 
> > I'm sure there must be a solution because the original u-boot bootloader
> > for the cpu module I'm using reboots the system as expected, through the wdog1;
> > unfortunately it's not so trivial to find what makes the difference.
> 
> Linux probably attempts reset via PSCI. Check for psci_system_reset
> in your vendor's U-Boot and transplant it into barebox.

U-Boot has actually a psci_system_reset(), it also rely on wdog1:

__secure void psci_system_reset(void)
{
	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;

	/* make sure WDOG1 clock is enabled */
	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
	writew(WCR_WDE, &wdog->wcr);

	while (1)
		wfi();
}

It is also compiled in the uboot image, because if I add a syntax error in
its body the compiler tells me; anyway I think it's not called when I issue
the 'reset' command at the prompt because the 'reset' works properly even if
I comment out the whole body of the function or if I add an endless loop like
while(1); to it.

Anyway I ported it to barebox, defining a void psci_system_reset(void) in
arch/arm/mach-imx/imx7.c:

...
#define CCM_BASE_ADDR   0x30380000
#define CCM_ROOT_WDOG   0xbb80
#define CCM_CCGR_WDOG1  0x49c8
#define WCR_WDE	        0x04
#define WDOG1_WCR       0x30280000
#define wfi() __asm__ __volatile__ ("wfi" : : : "memory")

static void imx7_system_reset(void)
{
	printf("%s: called now.\n",__func__);

	// make sure WDOG1 clock is enabled
	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
	writew(WCR_WDE, WDOG1_WCR);

	while (1)
		wfi();
};

static struct psci_ops imx7_psci_ops = {
	.cpu_on = imx7_cpu_on,
	.cpu_off = imx7_cpu_off,
	.system_reset = imx7_system_reset,
};

and the following case to smc.c:

static int do_smc(int argc, char *argv[])
{
...
		case 'r':
			printf("issue psci reset...\n");
			arm_smccc_smc(ARM_PSCI_0_2_FN_SYSTEM_RESET,
				      0, 0, 0, 0, 0, 0, 0, &res);
			printf("reset issued...\n");
			break;
...

to be able to call it.
Now, using the modified 'smc' command I can see my printf's:

samx7: / smc -n
samx7: / smc -r
issue psci reset...
psci_system_reset called.
imx7_system_reset: called now.

but the soc still just hangs.

giorgio

> 
> > 
> > giorgio
> > 
> > 
> >>
> >> -- 
> >> 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
> > 
> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-29 15:30           ` Giorgio Dal Molin
@ 2020-06-29 15:35             ` Ahmad Fatoum
  2020-06-29 16:03               ` Giorgio Dal Molin
  0 siblings, 1 reply; 20+ messages in thread
From: Ahmad Fatoum @ 2020-06-29 15:35 UTC (permalink / raw)
  To: Giorgio Dal Molin, Fabio Estevam; +Cc: Barebox List

Hello Giorgio,

On 6/29/20 5:30 PM, Giorgio Dal Molin wrote:
> Hi,
> 
>> On June 29, 2020 at 3:30 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>>
>> Hello,
>>
>> On 6/29/20 12:53 PM, Giorgio Dal Molin wrote:
>>> Hi,
>>>
>>>> On June 29, 2020 at 10:44 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>
>>>>
>>>> Hello Giorgio,
>>>>
>>>> On 6/23/20 5:11 PM, Giorgio Dal Molin wrote:
>>>>> Hi Fabio,
>>>>>
>>>>> thank you for your quick reply.
>>>>>
>>>>> I've already found the errata you linked in your mail but I had no success
>>>>> applying the suggestion there; maybe I'm doing it wrong.
>>>>> Let's take the Option 3 there:
>>>>
>>>> Does reset within Linux (with say the imx_v6_v7_defconfg) work?
>>>
>>> No. I see the same behavior also with the kernel: after a 'shutdown -r'
>>> the system 'goes down' but doesn't reboot, it just hang:
>>>
>>> ~ # shutdown -r
>>> system is going down for reboot NOW
>>> ...
>>> [   46.248222] 000: ci_hdrc ci_hdrc.0: remove, state 1
>>> [   46.248253] 000: usb usb1: USB disconnect, device number 1
>>> [   46.248263] 000: usb 1-1: USB disconnect, device number 2
>>>
>>>
>>> The same happens if I kill the 'watchdog' process: after ~2 seconds
>>> the system just hangs.
>>>
>>> I'm sure there must be a solution because the original u-boot bootloader
>>> for the cpu module I'm using reboots the system as expected, through the wdog1;
>>> unfortunately it's not so trivial to find what makes the difference.
>>
>> Linux probably attempts reset via PSCI. Check for psci_system_reset
>> in your vendor's U-Boot and transplant it into barebox.
> 
> U-Boot has actually a psci_system_reset(), it also rely on wdog1:
> 
> __secure void psci_system_reset(void)
> {
> 	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
> 
> 	/* make sure WDOG1 clock is enabled */
> 	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
> 	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
> 	writew(WCR_WDE, &wdog->wcr);
> 
> 	while (1)
> 		wfi();
> }
> 
> It is also compiled in the uboot image, because if I add a syntax error in
> its body the compiler tells me; anyway I think it's not called when I issue
> the 'reset' command at the prompt because the 'reset' works properly even if
> I comment out the whole body of the function or if I add an endless loop like
> while(1); to it.

Yes. It should be called by the kernel when CONFIG_ARM_PSCI_FW=y
and the device tree has the correct node. 
> Anyway I ported it to barebox, defining a void psci_system_reset(void) in
> arch/arm/mach-imx/imx7.c:
> 
> ...
> #define CCM_BASE_ADDR   0x30380000
> #define CCM_ROOT_WDOG   0xbb80
> #define CCM_CCGR_WDOG1  0x49c8
> #define WCR_WDE	        0x04
> #define WDOG1_WCR       0x30280000
> #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
> 
> static void imx7_system_reset(void)
> {
> 	printf("%s: called now.\n",__func__);
> 
> 	// make sure WDOG1 clock is enabled
> 	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
> 	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
> 	writew(WCR_WDE, WDOG1_WCR);
> 
> 	while (1)
> 		wfi();
> };
> 
> static struct psci_ops imx7_psci_ops = {
> 	.cpu_on = imx7_cpu_on,
> 	.cpu_off = imx7_cpu_off,
> 	.system_reset = imx7_system_reset,
> };
> 
> and the following case to smc.c:
> 
> static int do_smc(int argc, char *argv[])
> {
> ...
> 		case 'r':
> 			printf("issue psci reset...\n");
> 			arm_smccc_smc(ARM_PSCI_0_2_FN_SYSTEM_RESET,
> 				      0, 0, 0, 0, 0, 0, 0, &res);
> 			printf("reset issued...\n");
> 			break;
> ...
> 
> to be able to call it.
> Now, using the modified 'smc' command I can see my printf's:
> 
> samx7: / smc -n
> samx7: / smc -r
> issue psci reset...
> psci_system_reset called.
> imx7_system_reset: called now.
> 
> but the soc still just hangs.

What does U-Boot's reset commando do on an i.MX7?

> 
> giorgio
> 
>>
>>>
>>> giorgio
>>>
>>>
>>>>
>>>> -- 
>>>> 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
>>>
>>
>> -- 
>> 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
> 

-- 
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-29 15:35             ` Ahmad Fatoum
@ 2020-06-29 16:03               ` Giorgio Dal Molin
  2020-06-29 16:11                 ` Ahmad Fatoum
  0 siblings, 1 reply; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-06-29 16:03 UTC (permalink / raw)
  To: Ahmad Fatoum, Fabio Estevam; +Cc: Barebox List

Hi,

> On June 29, 2020 at 5:35 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
> 
> Hello Giorgio,
> 
> On 6/29/20 5:30 PM, Giorgio Dal Molin wrote:
> > Hi,
> > 
> >> On June 29, 2020 at 3:30 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>
> >>
> >> Hello,
> >>
> >> On 6/29/20 12:53 PM, Giorgio Dal Molin wrote:
> >>> Hi,
> >>>
> >>>> On June 29, 2020 at 10:44 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>>
> >>>>
> >>>> Hello Giorgio,
> >>>>
> >>>> On 6/23/20 5:11 PM, Giorgio Dal Molin wrote:
> >>>>> Hi Fabio,
> >>>>>
> >>>>> thank you for your quick reply.
> >>>>>
> >>>>> I've already found the errata you linked in your mail but I had no success
> >>>>> applying the suggestion there; maybe I'm doing it wrong.
> >>>>> Let's take the Option 3 there:
> >>>>
> >>>> Does reset within Linux (with say the imx_v6_v7_defconfg) work?
> >>>
> >>> No. I see the same behavior also with the kernel: after a 'shutdown -r'
> >>> the system 'goes down' but doesn't reboot, it just hang:
> >>>
> >>> ~ # shutdown -r
> >>> system is going down for reboot NOW
> >>> ...
> >>> [   46.248222] 000: ci_hdrc ci_hdrc.0: remove, state 1
> >>> [   46.248253] 000: usb usb1: USB disconnect, device number 1
> >>> [   46.248263] 000: usb 1-1: USB disconnect, device number 2
> >>>
> >>>
> >>> The same happens if I kill the 'watchdog' process: after ~2 seconds
> >>> the system just hangs.
> >>>
> >>> I'm sure there must be a solution because the original u-boot bootloader
> >>> for the cpu module I'm using reboots the system as expected, through the wdog1;
> >>> unfortunately it's not so trivial to find what makes the difference.
> >>
> >> Linux probably attempts reset via PSCI. Check for psci_system_reset
> >> in your vendor's U-Boot and transplant it into barebox.
> > 
> > U-Boot has actually a psci_system_reset(), it also rely on wdog1:
> > 
> > __secure void psci_system_reset(void)
> > {
> > 	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
> > 
> > 	/* make sure WDOG1 clock is enabled */
> > 	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
> > 	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
> > 	writew(WCR_WDE, &wdog->wcr);
> > 
> > 	while (1)
> > 		wfi();
> > }
> > 
> > It is also compiled in the uboot image, because if I add a syntax error in
> > its body the compiler tells me; anyway I think it's not called when I issue
> > the 'reset' command at the prompt because the 'reset' works properly even if
> > I comment out the whole body of the function or if I add an endless loop like
> > while(1); to it.
> 
> Yes. It should be called by the kernel when CONFIG_ARM_PSCI_FW=y
> and the device tree has the correct node. 
> > Anyway I ported it to barebox, defining a void psci_system_reset(void) in
> > arch/arm/mach-imx/imx7.c:
> > 
> > ...
> > #define CCM_BASE_ADDR   0x30380000
> > #define CCM_ROOT_WDOG   0xbb80
> > #define CCM_CCGR_WDOG1  0x49c8
> > #define WCR_WDE	        0x04
> > #define WDOG1_WCR       0x30280000
> > #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
> > 
> > static void imx7_system_reset(void)
> > {
> > 	printf("%s: called now.\n",__func__);
> > 
> > 	// make sure WDOG1 clock is enabled
> > 	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
> > 	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
> > 	writew(WCR_WDE, WDOG1_WCR);
> > 
> > 	while (1)
> > 		wfi();
> > };
> > 
> > static struct psci_ops imx7_psci_ops = {
> > 	.cpu_on = imx7_cpu_on,
> > 	.cpu_off = imx7_cpu_off,
> > 	.system_reset = imx7_system_reset,
> > };
> > 
> > and the following case to smc.c:
> > 
> > static int do_smc(int argc, char *argv[])
> > {
> > ...
> > 		case 'r':
> > 			printf("issue psci reset...\n");
> > 			arm_smccc_smc(ARM_PSCI_0_2_FN_SYSTEM_RESET,
> > 				      0, 0, 0, 0, 0, 0, 0, &res);
> > 			printf("reset issued...\n");
> > 			break;
> > ...
> > 
> > to be able to call it.
> > Now, using the modified 'smc' command I can see my printf's:
> > 
> > samx7: / smc -n
> > samx7: / smc -r
> > issue psci reset...
> > psci_system_reset called.
> > imx7_system_reset: called now.
> > 
> > but the soc still just hangs.
> 
> What does U-Boot's reset commando do on an i.MX7?

It ends up calling this function in the imx_watchdog.c:

void __attribute__((weak)) reset_cpu(ulong addr)
{
	struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;

	printf("%s: called now\n",__func__);
	udelay(500);

	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
		 */
	}
}

I added the printf / udelay to be sure. WCR_WT_MSK is (0xff << 8).

What I also observed is that if I upload the uboot image with the imx-usb-loader it boots
and runs normally but the reset in this case also just hangs the soc without rebooting it,
like barebox; so there must be a difference between an 'usb uploaded' boot and a 'qspi native'
boot.

giorgio

> 
> > 
> > giorgio
> > 
> >>
> >>>
> >>> giorgio
> >>>
> >>>
> >>>>
> >>>> -- 
> >>>> 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
> >>>
> >>
> >> -- 
> >> 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
> > 
> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-29 16:03               ` Giorgio Dal Molin
@ 2020-06-29 16:11                 ` Ahmad Fatoum
  2020-06-29 16:33                   ` Giorgio Dal Molin
  0 siblings, 1 reply; 20+ messages in thread
From: Ahmad Fatoum @ 2020-06-29 16:11 UTC (permalink / raw)
  To: Giorgio Dal Molin, Fabio Estevam; +Cc: Barebox List

On 6/29/20 6:03 PM, Giorgio Dal Molin wrote:
> Hi,
> 
>> On June 29, 2020 at 5:35 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>>
>> Hello Giorgio,
>>
>> On 6/29/20 5:30 PM, Giorgio Dal Molin wrote:
>>> Hi,
>>>
>>>> On June 29, 2020 at 3:30 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>
>>>>
>>>> Hello,
>>>>
>>>> On 6/29/20 12:53 PM, Giorgio Dal Molin wrote:
>>>>> Hi,
>>>>>
>>>>>> On June 29, 2020 at 10:44 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>>>>
>>>>>>
>>>>>> Hello Giorgio,
>>>>>>
>>>>>> On 6/23/20 5:11 PM, Giorgio Dal Molin wrote:
>>>>>>> Hi Fabio,
>>>>>>>
>>>>>>> thank you for your quick reply.
>>>>>>>
>>>>>>> I've already found the errata you linked in your mail but I had no success
>>>>>>> applying the suggestion there; maybe I'm doing it wrong.
>>>>>>> Let's take the Option 3 there:
>>>>>>
>>>>>> Does reset within Linux (with say the imx_v6_v7_defconfg) work?
>>>>>
>>>>> No. I see the same behavior also with the kernel: after a 'shutdown -r'
>>>>> the system 'goes down' but doesn't reboot, it just hang:
>>>>>
>>>>> ~ # shutdown -r
>>>>> system is going down for reboot NOW
>>>>> ...
>>>>> [   46.248222] 000: ci_hdrc ci_hdrc.0: remove, state 1
>>>>> [   46.248253] 000: usb usb1: USB disconnect, device number 1
>>>>> [   46.248263] 000: usb 1-1: USB disconnect, device number 2
>>>>>
>>>>>
>>>>> The same happens if I kill the 'watchdog' process: after ~2 seconds
>>>>> the system just hangs.
>>>>>
>>>>> I'm sure there must be a solution because the original u-boot bootloader
>>>>> for the cpu module I'm using reboots the system as expected, through the wdog1;
>>>>> unfortunately it's not so trivial to find what makes the difference.
>>>>
>>>> Linux probably attempts reset via PSCI. Check for psci_system_reset
>>>> in your vendor's U-Boot and transplant it into barebox.
>>>
>>> U-Boot has actually a psci_system_reset(), it also rely on wdog1:
>>>
>>> __secure void psci_system_reset(void)
>>> {
>>> 	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
>>>
>>> 	/* make sure WDOG1 clock is enabled */
>>> 	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
>>> 	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
>>> 	writew(WCR_WDE, &wdog->wcr);
>>>
>>> 	while (1)
>>> 		wfi();
>>> }
>>>
>>> It is also compiled in the uboot image, because if I add a syntax error in
>>> its body the compiler tells me; anyway I think it's not called when I issue
>>> the 'reset' command at the prompt because the 'reset' works properly even if
>>> I comment out the whole body of the function or if I add an endless loop like
>>> while(1); to it.
>>
>> Yes. It should be called by the kernel when CONFIG_ARM_PSCI_FW=y
>> and the device tree has the correct node. 
>>> Anyway I ported it to barebox, defining a void psci_system_reset(void) in
>>> arch/arm/mach-imx/imx7.c:
>>>
>>> ...
>>> #define CCM_BASE_ADDR   0x30380000
>>> #define CCM_ROOT_WDOG   0xbb80
>>> #define CCM_CCGR_WDOG1  0x49c8
>>> #define WCR_WDE	        0x04
>>> #define WDOG1_WCR       0x30280000
>>> #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
>>>
>>> static void imx7_system_reset(void)
>>> {
>>> 	printf("%s: called now.\n",__func__);
>>>
>>> 	// make sure WDOG1 clock is enabled
>>> 	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
>>> 	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
>>> 	writew(WCR_WDE, WDOG1_WCR);
>>>
>>> 	while (1)
>>> 		wfi();
>>> };
>>>
>>> static struct psci_ops imx7_psci_ops = {
>>> 	.cpu_on = imx7_cpu_on,
>>> 	.cpu_off = imx7_cpu_off,
>>> 	.system_reset = imx7_system_reset,
>>> };
>>>
>>> and the following case to smc.c:
>>>
>>> static int do_smc(int argc, char *argv[])
>>> {
>>> ...
>>> 		case 'r':
>>> 			printf("issue psci reset...\n");
>>> 			arm_smccc_smc(ARM_PSCI_0_2_FN_SYSTEM_RESET,
>>> 				      0, 0, 0, 0, 0, 0, 0, &res);
>>> 			printf("reset issued...\n");
>>> 			break;
>>> ...
>>>
>>> to be able to call it.
>>> Now, using the modified 'smc' command I can see my printf's:
>>>
>>> samx7: / smc -n
>>> samx7: / smc -r
>>> issue psci reset...
>>> psci_system_reset called.
>>> imx7_system_reset: called now.
>>>
>>> but the soc still just hangs.
>>
>> What does U-Boot's reset commando do on an i.MX7?
> 
> It ends up calling this function in the imx_watchdog.c:
> 
> void __attribute__((weak)) reset_cpu(ulong addr)
> {
> 	struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> 
> 	printf("%s: called now\n",__func__);
> 	udelay(500);
> 
> 	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
> 		 */
> 	}
> }
> 
> I added the printf / udelay to be sure. WCR_WT_MSK is (0xff << 8).

Hmm and same code doesn't reset it in barebox? That's strange.
Is there something in the DCD table that you are missing in barebox?

> 
> What I also observed is that if I upload the uboot image with the imx-usb-loader it boots
> and runs normally but the reset in this case also just hangs the soc without rebooting it,
> like barebox; so there must be a difference between an 'usb uploaded' boot and a 'qspi native'
> boot.

Are you sure it's not resetting to serial download mode again?

> 
> giorgio
> 
>>
>>>
>>> giorgio
>>>
>>>>
>>>>>
>>>>> giorgio
>>>>>
>>>>>
>>>>>>
>>>>>> -- 
>>>>>> 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
>>>>>
>>>>
>>>> -- 
>>>> 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
>>>
>>
>> -- 
>> 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
> 

-- 
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-29 16:11                 ` Ahmad Fatoum
@ 2020-06-29 16:33                   ` Giorgio Dal Molin
  2020-06-29 16:39                     ` Fabio Estevam
  0 siblings, 1 reply; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-06-29 16:33 UTC (permalink / raw)
  To: Ahmad Fatoum, Fabio Estevam; +Cc: Barebox List

Hi,

> On June 29, 2020 at 6:11 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
> 
> On 6/29/20 6:03 PM, Giorgio Dal Molin wrote:
> > Hi,
> > 
> >> On June 29, 2020 at 5:35 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>
> >>
> >> Hello Giorgio,
> >>
> >> On 6/29/20 5:30 PM, Giorgio Dal Molin wrote:
> >>> Hi,
> >>>
> >>>> On June 29, 2020 at 3:30 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>>
> >>>>
> >>>> Hello,
> >>>>
> >>>> On 6/29/20 12:53 PM, Giorgio Dal Molin wrote:
> >>>>> Hi,
> >>>>>
> >>>>>> On June 29, 2020 at 10:44 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>>>>
> >>>>>>
> >>>>>> Hello Giorgio,
> >>>>>>
> >>>>>> On 6/23/20 5:11 PM, Giorgio Dal Molin wrote:
> >>>>>>> Hi Fabio,
> >>>>>>>
> >>>>>>> thank you for your quick reply.
> >>>>>>>
> >>>>>>> I've already found the errata you linked in your mail but I had no success
> >>>>>>> applying the suggestion there; maybe I'm doing it wrong.
> >>>>>>> Let's take the Option 3 there:
> >>>>>>
> >>>>>> Does reset within Linux (with say the imx_v6_v7_defconfg) work?
> >>>>>
> >>>>> No. I see the same behavior also with the kernel: after a 'shutdown -r'
> >>>>> the system 'goes down' but doesn't reboot, it just hang:
> >>>>>
> >>>>> ~ # shutdown -r
> >>>>> system is going down for reboot NOW
> >>>>> ...
> >>>>> [   46.248222] 000: ci_hdrc ci_hdrc.0: remove, state 1
> >>>>> [   46.248253] 000: usb usb1: USB disconnect, device number 1
> >>>>> [   46.248263] 000: usb 1-1: USB disconnect, device number 2
> >>>>>
> >>>>>
> >>>>> The same happens if I kill the 'watchdog' process: after ~2 seconds
> >>>>> the system just hangs.
> >>>>>
> >>>>> I'm sure there must be a solution because the original u-boot bootloader
> >>>>> for the cpu module I'm using reboots the system as expected, through the wdog1;
> >>>>> unfortunately it's not so trivial to find what makes the difference.
> >>>>
> >>>> Linux probably attempts reset via PSCI. Check for psci_system_reset
> >>>> in your vendor's U-Boot and transplant it into barebox.
> >>>
> >>> U-Boot has actually a psci_system_reset(), it also rely on wdog1:
> >>>
> >>> __secure void psci_system_reset(void)
> >>> {
> >>> 	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
> >>>
> >>> 	/* make sure WDOG1 clock is enabled */
> >>> 	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
> >>> 	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
> >>> 	writew(WCR_WDE, &wdog->wcr);
> >>>
> >>> 	while (1)
> >>> 		wfi();
> >>> }
> >>>
> >>> It is also compiled in the uboot image, because if I add a syntax error in
> >>> its body the compiler tells me; anyway I think it's not called when I issue
> >>> the 'reset' command at the prompt because the 'reset' works properly even if
> >>> I comment out the whole body of the function or if I add an endless loop like
> >>> while(1); to it.
> >>
> >> Yes. It should be called by the kernel when CONFIG_ARM_PSCI_FW=y
> >> and the device tree has the correct node. 
> >>> Anyway I ported it to barebox, defining a void psci_system_reset(void) in
> >>> arch/arm/mach-imx/imx7.c:
> >>>
> >>> ...
> >>> #define CCM_BASE_ADDR   0x30380000
> >>> #define CCM_ROOT_WDOG   0xbb80
> >>> #define CCM_CCGR_WDOG1  0x49c8
> >>> #define WCR_WDE	        0x04
> >>> #define WDOG1_WCR       0x30280000
> >>> #define wfi() __asm__ __volatile__ ("wfi" : : : "memory")
> >>>
> >>> static void imx7_system_reset(void)
> >>> {
> >>> 	printf("%s: called now.\n",__func__);
> >>>
> >>> 	// make sure WDOG1 clock is enabled
> >>> 	writel(0x1 << 28, CCM_BASE_ADDR + CCM_ROOT_WDOG);
> >>> 	writel(0x3, CCM_BASE_ADDR + CCM_CCGR_WDOG1);
> >>> 	writew(WCR_WDE, WDOG1_WCR);
> >>>
> >>> 	while (1)
> >>> 		wfi();
> >>> };
> >>>
> >>> static struct psci_ops imx7_psci_ops = {
> >>> 	.cpu_on = imx7_cpu_on,
> >>> 	.cpu_off = imx7_cpu_off,
> >>> 	.system_reset = imx7_system_reset,
> >>> };
> >>>
> >>> and the following case to smc.c:
> >>>
> >>> static int do_smc(int argc, char *argv[])
> >>> {
> >>> ...
> >>> 		case 'r':
> >>> 			printf("issue psci reset...\n");
> >>> 			arm_smccc_smc(ARM_PSCI_0_2_FN_SYSTEM_RESET,
> >>> 				      0, 0, 0, 0, 0, 0, 0, &res);
> >>> 			printf("reset issued...\n");
> >>> 			break;
> >>> ...
> >>>
> >>> to be able to call it.
> >>> Now, using the modified 'smc' command I can see my printf's:
> >>>
> >>> samx7: / smc -n
> >>> samx7: / smc -r
> >>> issue psci reset...
> >>> psci_system_reset called.
> >>> imx7_system_reset: called now.
> >>>
> >>> but the soc still just hangs.
> >>
> >> What does U-Boot's reset commando do on an i.MX7?
> > 
> > It ends up calling this function in the imx_watchdog.c:
> > 
> > void __attribute__((weak)) reset_cpu(ulong addr)
> > {
> > 	struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> > 
> > 	printf("%s: called now\n",__func__);
> > 	udelay(500);
> > 
> > 	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
> > 		 */
> > 	}
> > }
> > 
> > I added the printf / udelay to be sure. WCR_WT_MSK is (0xff << 8).
> 
> Hmm and same code doesn't reset it in barebox? That's strange.
> Is there something in the DCD table that you are missing in barebox?

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.

giorgio

> 
> > 
> > What I also observed is that if I upload the uboot image with the imx-usb-loader it boots
> > and runs normally but the reset in this case also just hangs the soc without rebooting it,
> > like barebox; so there must be a difference between an 'usb uploaded' boot and a 'qspi native'
> > boot.
> 
> Are you sure it's not resetting to serial download mode again?
> 
> > 
> > giorgio
> > 
> >>
> >>>
> >>> giorgio
> >>>
> >>>>
> >>>>>
> >>>>> giorgio
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> -- 
> >>>>>> 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
> >>>>>
> >>>>
> >>>> -- 
> >>>> 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
> >>>
> >>
> >> -- 
> >> 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
> > 
> 
> -- 
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-29 16:33                   ` Giorgio Dal Molin
@ 2020-06-29 16:39                     ` Fabio Estevam
  2020-07-01  9:52                       ` Giorgio Dal Molin
  0 siblings, 1 reply; 20+ messages in thread
From: Fabio Estevam @ 2020-06-29 16:39 UTC (permalink / raw)
  To: Giorgio Dal Molin; +Cc: Barebox List, Ahmad Fatoum

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

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-06-29 16:39                     ` Fabio Estevam
@ 2020-07-01  9:52                       ` Giorgio Dal Molin
  2020-07-02  9:25                         ` Ahmad Fatoum
  0 siblings, 1 reply; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-07-01  9:52 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Barebox List, Ahmad Fatoum

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.

giorgio

> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  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
  0 siblings, 2 replies; 20+ messages in thread
From: Ahmad Fatoum @ 2020-07-02  9:25 UTC (permalink / raw)
  To: Giorgio Dal Molin, Fabio Estevam; +Cc: Barebox List



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..

> 
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-07-02  9:25                         ` Ahmad Fatoum
@ 2020-07-02 14:51                           ` Giorgio Dal Molin
  2020-07-02 15:28                           ` Giorgio Dal Molin
  1 sibling, 0 replies; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-07-02 14:51 UTC (permalink / raw)
  To: Ahmad Fatoum, Fabio Estevam; +Cc: Barebox List

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-07-02  9:25                         ` Ahmad Fatoum
  2020-07-02 14:51                           ` Giorgio Dal Molin
@ 2020-07-02 15:28                           ` Giorgio Dal Molin
  2020-07-02 16:05                             ` Lucas Stach
  2020-07-02 16:24                             ` Fabio Estevam
  1 sibling, 2 replies; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-07-02 15:28 UTC (permalink / raw)
  To: Ahmad Fatoum, Fabio Estevam; +Cc: Barebox List

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-07-02 15:28                           ` Giorgio Dal Molin
@ 2020-07-02 16:05                             ` Lucas Stach
  2020-07-03 14:13                               ` Giorgio Dal Molin
  2020-07-02 16:24                             ` Fabio Estevam
  1 sibling, 1 reply; 20+ messages in thread
From: Lucas Stach @ 2020-07-02 16:05 UTC (permalink / raw)
  To: Giorgio Dal Molin, Ahmad Fatoum, Fabio Estevam; +Cc: Barebox List

Hi Giorgio,

Am Donnerstag, den 02.07.2020, 17:28 +0200 schrieb Giorgio Dal Molin:
[...]
> 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);
> 

I'm not sure if the i.MX7 needs the same double write. Just in case,
can you add a dummy read to the IMX21_WDOG_WCR register after the
writes to make sure the writes actually get flushed to the device
before the delay?

Regards,
Lucas

> ...
> 
> 	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


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-07-02 15:28                           ` Giorgio Dal Molin
  2020-07-02 16:05                             ` Lucas Stach
@ 2020-07-02 16:24                             ` Fabio Estevam
  2020-07-07  5:52                               ` Giorgio Dal Molin
  1 sibling, 1 reply; 20+ messages in thread
From: Fabio Estevam @ 2020-07-02 16:24 UTC (permalink / raw)
  To: Giorgio Dal Molin; +Cc: Barebox List, Ahmad Fatoum

Hi Giorgio,

On Thu, Jul 2, 2020 at 12:28 PM Giorgio Dal Molin
<giorgio.nicole@arcor.de> wrote:

> 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.

Just curious: are you able to reboot reliably if you issue a "reboot"
command from the Linux prompt?

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-07-02 16:05                             ` Lucas Stach
@ 2020-07-03 14:13                               ` Giorgio Dal Molin
  0 siblings, 0 replies; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-07-03 14:13 UTC (permalink / raw)
  To: Lucas Stach, Ahmad Fatoum, Fabio Estevam; +Cc: Barebox List

Hi,

> On July 2, 2020 at 6:05 PM Lucas Stach <l.stach@pengutronix.de> wrote:
> 
> 
> Hi Giorgio,
> 
> Am Donnerstag, den 02.07.2020, 17:28 +0200 schrieb Giorgio Dal Molin:
> [...]
> > 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);
> > 
> 
> I'm not sure if the i.MX7 needs the same double write. Just in case,
> can you add a dummy read to the IMX21_WDOG_WCR register after the
> writes to make sure the writes actually get flushed to the device
> before the delay?
> 

thank you for the suggestion, I had also such cases in the past.
In this case it seems to make no difference.

To speed up the reset tests I have now an 'init' script that automatically issues
a 'reset' after 2s timeout.

Now, putting the while(1); right after the watchdog trigger in imx21_soc_reset():

  imxwd_write(priv, IMX21_WDOG_WCR, val);
  while(1);

gives the 'best results' at rebooting: that means I still have occasional hangs
but they are very seldom, the 2s reboot loop normally go on for minutes (~50-100 reboots)
before hanging.
This behavior doesn't change reading the reg. IMX21_WDOG_WCR back after writing it or
repeating the write two more time (errata ERR004346).

What makes the hang MUCH more likely is the call to mdelay(1000): with it in place I have
almost always a hang within 5 tries.

I'm also verifying the system reboot after a complete kernel/userland startup: in this case,
with the DCD addition:

wm 32 0x30391000 0x00000003
nop
...
wm 32 0x30391000 0x00000002
...

the kernel ( == the wdog1 kernel driver ) is also able to restart the system.
For this case I've also setup a reboot loop to automate the test. Currently it's still
running, after ~30 minutes == ~30 reboots.

giorgio

> > ...
> > 
> > 	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
> 
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: reset / watchdog on an imx7d soc
  2020-07-02 16:24                             ` Fabio Estevam
@ 2020-07-07  5:52                               ` Giorgio Dal Molin
  0 siblings, 0 replies; 20+ messages in thread
From: Giorgio Dal Molin @ 2020-07-07  5:52 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: Barebox List

Hi,

> On July 2, 2020 at 6:24 PM Fabio Estevam <festevam@gmail.com> wrote:
> 
> 
> Hi Giorgio,
> 
> On Thu, Jul 2, 2020 at 12:28 PM Giorgio Dal Molin
> <giorgio.nicole@arcor.de> wrote:
> 
> > 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.
> 
> Just curious: are you able to reboot reliably if you issue a "reboot"
> command from the Linux prompt?
> 

yesterday I ran a long reboot loop test:

 system start => kernel / userland boot => reboot

and at least captured a hang.

So, from what I can see, a kernel reboot hang is just as likely as a bootloader's
one.

giorgio

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-07-07  5:53 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-23 13:45 reset / watchdog on an imx7d soc 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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox