[PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Jiang Lu]

On PPC44x core, the WRC(Watchdog-timer Reset Control) field of TCR
of timer can not reset by software after set to a non-zero value.
Which means software can not reset the timeout behaviour of watchdog timer.

This patch selects WATCHDOG_NOWAYOUT option for 44x platforms to
indicate the watchdog timer can not be disabled once fired.

Signed-off-by: Jiang Lu <lu.jiang@windriver.com>
---
 drivers/watchdog/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 3709624..41f3dff 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1084,6 +1084,7 @@ config PIKA_WDT
 config BOOKE_WDT
 	tristate "PowerPC Book-E Watchdog Timer"
 	depends on BOOKE || 4xx
+	select WATCHDOG_NOWAYOUT if 44x
 	---help---
 	  Watchdog driver for PowerPC Book-E chips, such as the Freescale
 	  MPC85xx SOCs and the IBM PowerPC 440.
-- 
1.7.7

Re: [PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Kumar Gala]

On Jul 12, 2012, at 9:44 PM, Jiang Lu wrote:

> On PPC44x core, the WRC(Watchdog-timer Reset Control) field of TCR
> of timer can not reset by software after set to a non-zero value.
> Which means software can not reset the timeout behaviour of watchdog timer.
> 
> This patch selects WATCHDOG_NOWAYOUT option for 44x platforms to
> indicate the watchdog timer can not be disabled once fired.
> 
> Signed-off-by: Jiang Lu <lu.jiang@windriver.com>
> ---
> drivers/watchdog/Kconfig |    1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)

I believe this is not 44x specific, but how Book-E watchdog is architected.

> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 3709624..41f3dff 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1084,6 +1084,7 @@ config PIKA_WDT
> config BOOKE_WDT
> 	tristate "PowerPC Book-E Watchdog Timer"
> 	depends on BOOKE || 4xx
> +	select WATCHDOG_NOWAYOUT if 44x

This should probably be 'select WATCHDOG_NOWAYOUT if BOOKE'

> 	---help---
> 	  Watchdog driver for PowerPC Book-E chips, such as the Freescale
> 	  MPC85xx SOCs and the IBM PowerPC 440.
> -- 
> 1.7.7
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Josh Boyer]

On Fri, Jul 13, 2012 at 7:50 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
>
> On Jul 12, 2012, at 9:44 PM, Jiang Lu wrote:
>
>> On PPC44x core, the WRC(Watchdog-timer Reset Control) field of TCR
>> of timer can not reset by software after set to a non-zero value.
>> Which means software can not reset the timeout behaviour of watchdog timer.
>>
>> This patch selects WATCHDOG_NOWAYOUT option for 44x platforms to
>> indicate the watchdog timer can not be disabled once fired.
>>
>> Signed-off-by: Jiang Lu <lu.jiang@windriver.com>
>> ---
>> drivers/watchdog/Kconfig |    1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> I believe this is not 44x specific, but how Book-E watchdog is architected.

That is my understanding as well.

>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 3709624..41f3dff 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1084,6 +1084,7 @@ config PIKA_WDT
>> config BOOKE_WDT
>>       tristate "PowerPC Book-E Watchdog Timer"
>>       depends on BOOKE || 4xx
>> +     select WATCHDOG_NOWAYOUT if 44x
>
> This should probably be 'select WATCHDOG_NOWAYOUT if BOOKE'

I kind of disagree with this change.  It's a user selectable option for
a reason.

Right now, if the option is not set we call booke_wdt_disable which
indeed does not actually _disable_ the WDT, but it does set the timer
period to the maxium value.  We could go one step further and implement
a simple timer that pops and calls booke_wdt_ping if WATCHDOG_NOWAYOUT
is not set, then rearms itself.  That would leave the user with the
ability to perform recovery of the userspace process that exited or
died and was responsible for pinging the watchdog.

josh

Re: [PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Kumar Gala]

On Jul 13, 2012, at 7:25 AM, Josh Boyer wrote:

> On Fri, Jul 13, 2012 at 7:50 AM, Kumar Gala =
<galak@kernel.crashing.org> wrote:
>>=20
>> On Jul 12, 2012, at 9:44 PM, Jiang Lu wrote:
>>=20
>>> On PPC44x core, the WRC(Watchdog-timer Reset Control) field of TCR
>>> of timer can not reset by software after set to a non-zero value.
>>> Which means software can not reset the timeout behaviour of watchdog =
timer.
>>>=20
>>> This patch selects WATCHDOG_NOWAYOUT option for 44x platforms to
>>> indicate the watchdog timer can not be disabled once fired.
>>>=20
>>> Signed-off-by: Jiang Lu <lu.jiang@windriver.com>
>>> ---
>>> drivers/watchdog/Kconfig |    1 +
>>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>=20
>> I believe this is not 44x specific, but how Book-E watchdog is =
architected.
>=20
> That is my understanding as well.
>=20
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index 3709624..41f3dff 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -1084,6 +1084,7 @@ config PIKA_WDT
>>> config BOOKE_WDT
>>>      tristate "PowerPC Book-E Watchdog Timer"
>>>      depends on BOOKE || 4xx
>>> +     select WATCHDOG_NOWAYOUT if 44x
>>=20
>> This should probably be 'select WATCHDOG_NOWAYOUT if BOOKE'
>=20
> I kind of disagree with this change.  It's a user selectable option =
for
> a reason.
>=20
> Right now, if the option is not set we call booke_wdt_disable which
> indeed does not actually _disable_ the WDT, but it does set the timer
> period to the maxium value.  We could go one step further and =
implement
> a simple timer that pops and calls booke_wdt_ping if WATCHDOG_NOWAYOUT
> is not set, then rearms itself.  That would leave the user with the
> ability to perform recovery of the userspace process that exited or
> died and was responsible for pinging the watchdog.
>=20
> josh

My only care was about it being Book-E and not 44x.  Otherwise I'm happy =
to defer to your take on this.

- k

Re: [PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Lu.Jiang]

于 2012年07月13日 19:50, Kumar Gala 写道:
> On Jul 12, 2012, at 9:44 PM, Jiang Lu wrote:
>
>> On PPC44x core, the WRC(Watchdog-timer Reset Control) field of TCR
>> of timer can not reset by software after set to a non-zero value.
>> Which means software can not reset the timeout behaviour of watchdog timer.
>>
>> This patch selects WATCHDOG_NOWAYOUT option for 44x platforms to
>> indicate the watchdog timer can not be disabled once fired.
>>
>> Signed-off-by: Jiang Lu <lu.jiang@windriver.com>
>> ---
>> drivers/watchdog/Kconfig |    1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
> I believe this is not 44x specific, but how Book-E watchdog is architected.
>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 3709624..41f3dff 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1084,6 +1084,7 @@ config PIKA_WDT
>> config BOOKE_WDT
>> 	tristate "PowerPC Book-E Watchdog Timer"
>> 	depends on BOOKE || 4xx
>> +	select WATCHDOG_NOWAYOUT if 44x
> This should probably be 'select WATCHDOG_NOWAYOUT if BOOKE'

On ppc44x's processor, if we disabled 'WATCHDOG_NOWAYOUT ' option. The 
driver's release routine will try to disable the watchdog , by clearing 
the WIE & WTDP field in TCR.
Since the ppc44x's watch dog can not reset by software, such operation 
only set the timeout value(WDTP) to minimum, and cause the system reboot 
immediately.

I checked ppc 476, 405 & 450's manual, these document said the 
WRC(Watchdog-timer Reset Control) field of TCR of timer
can not reset by software after set to a non-zero value. I think all 
ppc44x core should got same limitation.

While on FSL's platform, we did not met such issue. So I think we should 
select 'WATCHDOG_NOWAYOUT' option for 44x platform.

Regards,
Jiang Lu

>
>> 	---help---
>> 	  Watchdog driver for PowerPC Book-E chips, such as the Freescale
>> 	  MPC85xx SOCs and the IBM PowerPC 440.
>> -- 
>> 1.7.7
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

Re: [PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Scott Wood]

On 07/15/2012 09:07 PM, Lu.Jiang wrote:
> =E4=BA=8E 2012=E5=B9=B407=E6=9C=8813=E6=97=A5 19:50, Kumar Gala =E5=86=99=
=E9=81=93:
>> On Jul 12, 2012, at 9:44 PM, Jiang Lu wrote:
>>
>>> On PPC44x core, the WRC(Watchdog-timer Reset Control) field of TCR
>>> of timer can not reset by software after set to a non-zero value.
>>> Which means software can not reset the timeout behaviour of watchdog
>>> timer.
>>>
>>> This patch selects WATCHDOG_NOWAYOUT option for 44x platforms to
>>> indicate the watchdog timer can not be disabled once fired.
>>>
>>> Signed-off-by: Jiang Lu <lu.jiang@windriver.com>
>>> ---
>>> drivers/watchdog/Kconfig |    1 +
>>> 1 files changed, 1 insertions(+), 0 deletions(-)
>> I believe this is not 44x specific, but how Book-E watchdog is
>> architected.
>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index 3709624..41f3dff 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -1084,6 +1084,7 @@ config PIKA_WDT
>>> config BOOKE_WDT
>>>     tristate "PowerPC Book-E Watchdog Timer"
>>>     depends on BOOKE || 4xx
>>> +    select WATCHDOG_NOWAYOUT if 44x
>> This should probably be 'select WATCHDOG_NOWAYOUT if BOOKE'
>=20
> On ppc44x's processor, if we disabled 'WATCHDOG_NOWAYOUT ' option. The
> driver's release routine will try to disable the watchdog , by clearing
> the WIE & WTDP field in TCR.
> Since the ppc44x's watch dog can not reset by software, such operation
> only set the timeout value(WDTP) to minimum, and cause the system reboo=
t
> immediately.
>=20
> I checked ppc 476, 405 & 450's manual, these document said the
> WRC(Watchdog-timer Reset Control) field of TCR of timer
> can not reset by software after set to a non-zero value. I think all
> ppc44x core should got same limitation.

This is (supposed to be) true on FSL e500 as well.

> While on FSL's platform, we did not met such issue.

You tested this and were able to clear WRC on an e500-based chip?  Which
one?

-Scott

Re: [PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Tabi Timur-B04825]

On Fri, Jul 13, 2012 at 7:25 AM, Josh Boyer <jwboyer@gmail.com> wrote:

> Right now, if the option is not set we call booke_wdt_disable which
> indeed does not actually _disable_ the WDT, but it does set the timer
> period to the maxium value.  We could go one step further and implement
> a simple timer that pops and calls booke_wdt_ping if WATCHDOG_NOWAYOUT
> is not set, then rearms itself.  That would leave the user with the
> ability to perform recovery of the userspace process that exited or
> died and was responsible for pinging the watchdog.

I think the maximum value is still decades or centuries, so there's no
real reason to complicate the code.

The NOWAYOUT feature is working as designed, so I don't understand the
need for this patch.

--=20
Timur Tabi
Linux kernel developer at Freescale=

Re: [PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Tabi Timur-B04825]

On Sun, Jul 15, 2012 at 9:07 PM, Lu.Jiang <lu.jiang@windriver.com> wrote:
>
> Since the ppc44x's watch dog can not reset by software, such operation on=
ly
> set the timeout value(WDTP) to minimum, and cause the system reboot
> immediately.

It's supposed to set it to the maximum.  That's what WDTP_MASK is
supposed to do.

--=20
Timur Tabi
Linux kernel developer at Freescale=

Re: [PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Josh Boyer]

On Mon, Jul 16, 2012 at 4:28 PM, Tabi Timur-B04825 <B04825@freescale.com> wrote:
> On Fri, Jul 13, 2012 at 7:25 AM, Josh Boyer <jwboyer@gmail.com> wrote:
>
>> Right now, if the option is not set we call booke_wdt_disable which
>> indeed does not actually _disable_ the WDT, but it does set the timer
>> period to the maxium value.  We could go one step further and implement
>> a simple timer that pops and calls booke_wdt_ping if WATCHDOG_NOWAYOUT
>> is not set, then rearms itself.  That would leave the user with the
>> ability to perform recovery of the userspace process that exited or
>> died and was responsible for pinging the watchdog.
>
> I think the maximum value is still decades or centuries, so there's no
> real reason to complicate the code.

I have no idea about FSL cores, but the 4xx maximum value selects TBU
bit 31.  When that bit flips is going to be determined by the speed of
the clock driving TB.  Most of the 4xx implementations I have seen use
the CPU clock, so to take the example from the 44x user manuals, a 400
MHz clock results in a maximum time period of ~21 seconds.  Multiply
by 3 to get the time before the WDT actually does a reset and you're
at about 1min.

> The NOWAYOUT feature is working as designed, so I don't understand the
> need for this patch.

NOWAYOUT is working.  The patch was basically forcing it on, even if
the end user doesn't want the behavior described.  From what you've
said, that simply doesn't matter in the FSL case because the machine
will run for a really long time.  On 4xx, something needs to be done
to keep the machine running if the user doesn't want the NOWAYOUT
behavior, which is why I suggested a kernel timer.

josh

Re: [PATCH] ppc44x/watchdog: Select WATCHDOG_NOWAYOUT option

[Timur Tabi]

Josh Boyer wrote:

> I have no idea about FSL cores, but the 4xx maximum value selects TBU
> bit 31.  When that bit flips is going to be determined by the speed of
> the clock driving TB.  Most of the 4xx implementations I have seen use
> the CPU clock, so to take the example from the 44x user manuals, a 400
> MHz clock results in a maximum time period of ~21 seconds.  Multiply
> by 3 to get the time before the WDT actually does a reset and you're
> at about 1min.

Ah, that is a problem.

> NOWAYOUT is working.  The patch was basically forcing it on, even if
> the end user doesn't want the behavior described.  From what you've
> said, that simply doesn't matter in the FSL case because the machine
> will run for a really long time.  On 4xx, something needs to be done
> to keep the machine running if the user doesn't want the NOWAYOUT
> behavior, which is why I suggested a kernel timer.

Yes, I agree now -- a kernel timer is a better idea.  And it doesn't need
to be 4xx-specific.  Although, I wonder if it's the time is reliable enough.

We already have an exception handler:

#ifdef CONFIG_BOOKE_WDT
/*
 * Default handler for a Watchdog exception,
 * spins until a reboot occurs
 */
void __attribute__ ((weak)) WatchdogHandler(struct pt_regs *regs)
{
	/* Generic WatchdogHandler, implement your own */
	mtspr(SPRN_TCR, mfspr(SPRN_TCR)&(~TCR_WIE));
	return;
}

void WatchdogException(struct pt_regs *regs)
{
	printk (KERN_EMERG "PowerPC Book-E Watchdog Exception\n");
	WatchdogHandler(regs);
}
#endif

Maybe instead of disabling interrupts, we should ping the watchdog instead?

-- 
Timur Tabi
Linux kernel developer at Freescale