[RFC PATCH 0/1] Suspending i.MX watchdog in WAIT mode

[RFC PATCH 0/1] Suspending i.MX watchdog in WAIT mode

[Andrej Picej]

Hi all,

we are using i.MX6UL with its watchdog WDOG1 and kernel 5.15.62. It was
discovered that the watchdog triggers reset when the device is put into
'Suspend-To-Idle' (WAIT) state.

i.MX6UL watchdog has a WDW (Watchdog Disable for Wait) bit in WCR
(Watchdog Control Register) which can put the watchdog in suspend when
the device is put to WAIT mode. Similarly, WDZST bit is already set in
imx2_wdt driver by default, which suspends the watchdog in STOP and DOZE
modes.

This RFC patch suspends watchdog when the device is in WAIT mode, which
fixes our problem. During development, we noticed some reports where
setting WDW bit caused inconsistent timeout events or inability of
watchdog to reset the board. We didn't have these problems but I am
curious if there is a case where device is put into WAIT mode and
watchdog should be enabled?

Maybe for cases where watchdog is used for WAIT mode supervision? So
basically to reset the system if device doesn't exit WAIT mode on its
own?

The problem can be recreated with:

	imx6ul-dev:~# echo freeze > /sys/power/state 
	[  101.093336] PM: suspend entry (s2idle)
	[  101.097785] Filesystems sync: 0.000 seconds
	[  101.122295] Freezing user space processes ... (elapsed 0.001 seconds) done.
	[  101.130637] OOM killer disabled.
	[  101.133998] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
	[  101.142941] printk: Suspending console(s) (use no_console_suspend to debug)
	...
Device resets after watchdog timeout expires! ~105s

Thank you for your feedback.

Best regards,
Andrej

Andrej Picej (1):
  watchdog: imx2_wdg: suspend watchdog in WAIT mode

 drivers/watchdog/imx2_wdt.c | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.25.1

[RFC PATCH 1/1] watchdog: imx2_wdg: suspend watchdog in WAIT mode

[Andrej Picej]

Putting device into the "Suspend-To-Idle" mode causes watchdog to
trigger and reset the board after set watchdog timeout period elapses.
Setting WDW bit in WCR (Watchdog Control Register) suspends watchdog in
WAIT mode (corresponds to "Suspend-To-Idle" mode) and allows board to be
in idle for infinite amount of time. Watchdog operation is restored
after exiting WAIT mode as expected.

Signed-off-by: Andrej Picej <andrej.picej@norik.com>
---
 drivers/watchdog/imx2_wdt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 51bfb796898b..f888f29f50dd 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -35,6 +35,7 @@
 
 #define IMX2_WDT_WCR		0x00		/* Control Register */
 #define IMX2_WDT_WCR_WT		(0xFF << 8)	/* -> Watchdog Timeout Field */
+#define IMX2_WDT_WCR_WDW	BIT(7)		/* -> Watchdog disable for WAIT */
 #define IMX2_WDT_WCR_WDA	BIT(5)		/* -> External Reset WDOG_B */
 #define IMX2_WDT_WCR_SRS	BIT(4)		/* -> Software Reset Signal */
 #define IMX2_WDT_WCR_WRE	BIT(3)		/* -> WDOG Reset Enable */
@@ -128,6 +129,8 @@ static inline void imx2_wdt_setup(struct watchdog_device *wdog)
 
 	/* Suspend timer in low power mode, write once-only */
 	val |= IMX2_WDT_WCR_WDZST;
+	/* Suspend timer in low power WAIT mode, write once-only */
+	val |= IMX2_WDT_WCR_WDW;
 	/* Strip the old watchdog Time-Out value */
 	val &= ~IMX2_WDT_WCR_WT;
 	/* Generate internal chip-level reset if WDOG times out */
-- 
2.25.1

Re: [RFC PATCH 0/1] Suspending i.MX watchdog in WAIT mode

[Guenter Roeck]

On 9/21/22 05:46, Andrej Picej wrote:
> Hi all,
> 
> we are using i.MX6UL with its watchdog WDOG1 and kernel 5.15.62. It was
> discovered that the watchdog triggers reset when the device is put into
> 'Suspend-To-Idle' (WAIT) state.
> 

Is that equivalent to "suspend" from Linux perspective, or some other
mode ? How does the device get into this state ?

Guenter

> i.MX6UL watchdog has a WDW (Watchdog Disable for Wait) bit in WCR
> (Watchdog Control Register) which can put the watchdog in suspend when
> the device is put to WAIT mode. Similarly, WDZST bit is already set in
> imx2_wdt driver by default, which suspends the watchdog in STOP and DOZE
> modes.
> 
> This RFC patch suspends watchdog when the device is in WAIT mode, which
> fixes our problem. During development, we noticed some reports where
> setting WDW bit caused inconsistent timeout events or inability of
> watchdog to reset the board. We didn't have these problems but I am
> curious if there is a case where device is put into WAIT mode and
> watchdog should be enabled?
> 
> Maybe for cases where watchdog is used for WAIT mode supervision? So
> basically to reset the system if device doesn't exit WAIT mode on its
> own?
> 
> The problem can be recreated with:
> 
> 	imx6ul-dev:~# echo freeze > /sys/power/state
> 	[  101.093336] PM: suspend entry (s2idle)
> 	[  101.097785] Filesystems sync: 0.000 seconds
> 	[  101.122295] Freezing user space processes ... (elapsed 0.001 seconds) done.
> 	[  101.130637] OOM killer disabled.
> 	[  101.133998] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> 	[  101.142941] printk: Suspending console(s) (use no_console_suspend to debug)
> 	...
> Device resets after watchdog timeout expires! ~105s
> 
> Thank you for your feedback.
> 
> Best regards,
> Andrej
> 
> Andrej Picej (1):
>    watchdog: imx2_wdg: suspend watchdog in WAIT mode
> 
>   drivers/watchdog/imx2_wdt.c | 3 +++
>   1 file changed, 3 insertions(+)
> 

Re: [RFC PATCH 0/1] Suspending i.MX watchdog in WAIT mode

[Andrej Picej]

Hi Guenter,

On 21. 09. 22 16:18, Guenter Roeck wrote:
> On 9/21/22 05:46, Andrej Picej wrote:
>> Hi all,
>>
>> we are using i.MX6UL with its watchdog WDOG1 and kernel 5.15.62. It was
>> discovered that the watchdog triggers reset when the device is put into
>> 'Suspend-To-Idle' (WAIT) state.
>>
> 
> Is that equivalent to "suspend" from Linux perspective, or some other
> mode ? How does the device get into this state ?

I think WAIT mode maps to System idle mode in linux [1].

Sorry don't quite understand your second question.
Do you mean how we trigger this state?
We trigger this state with:
 >     imx6ul-dev:~# echo freeze > /sys/power/state

If you mean what is done prior to entering this state?
The device enters WAIT mode when CLPCR bit is set to WAIT. The device
enters WAIT mode by gating the CPU clock, all other clocks can be gated
by programming CGR bits in WAIT mode.

[1] i.MX Linux Reference Manual, Rev. 0, 07/2016

Andrej

> 
> Guenter
> 
>> i.MX6UL watchdog has a WDW (Watchdog Disable for Wait) bit in WCR
>> (Watchdog Control Register) which can put the watchdog in suspend when
>> the device is put to WAIT mode. Similarly, WDZST bit is already set in
>> imx2_wdt driver by default, which suspends the watchdog in STOP and DOZE
>> modes.
>>
>> This RFC patch suspends watchdog when the device is in WAIT mode, which
>> fixes our problem. During development, we noticed some reports where
>> setting WDW bit caused inconsistent timeout events or inability of
>> watchdog to reset the board. We didn't have these problems but I am
>> curious if there is a case where device is put into WAIT mode and
>> watchdog should be enabled?
>>
>> Maybe for cases where watchdog is used for WAIT mode supervision? So
>> basically to reset the system if device doesn't exit WAIT mode on its
>> own?
>>
>> The problem can be recreated with:
>>
>>     imx6ul-dev:~# echo freeze > /sys/power/state
>>     [  101.093336] PM: suspend entry (s2idle)
>>     [  101.097785] Filesystems sync: 0.000 seconds
>>     [  101.122295] Freezing user space processes ... (elapsed 0.001 
>> seconds) done.
>>     [  101.130637] OOM killer disabled.
>>     [  101.133998] Freezing remaining freezable tasks ... (elapsed 
>> 0.001 seconds) done.
>>     [  101.142941] printk: Suspending console(s) (use 
>> no_console_suspend to debug)
>>     ...
>> Device resets after watchdog timeout expires! ~105s
>>
>> Thank you for your feedback.
>>
>> Best regards,
>> Andrej
>>
>> Andrej Picej (1):
>>    watchdog: imx2_wdg: suspend watchdog in WAIT mode
>>
>>   drivers/watchdog/imx2_wdt.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
> 

Re: [RFC PATCH 0/1] Suspending i.MX watchdog in WAIT mode

[Guenter Roeck]

On 9/22/22 00:17, Andrej Picej wrote:
> Hi Guenter,
> 
> On 21. 09. 22 16:18, Guenter Roeck wrote:
>> On 9/21/22 05:46, Andrej Picej wrote:
>>> Hi all,
>>>
>>> we are using i.MX6UL with its watchdog WDOG1 and kernel 5.15.62. It was
>>> discovered that the watchdog triggers reset when the device is put into
>>> 'Suspend-To-Idle' (WAIT) state.
>>>
>>
>> Is that equivalent to "suspend" from Linux perspective, or some other
>> mode ? How does the device get into this state ?
> 
> I think WAIT mode maps to System idle mode in linux [1].
> 

Sorry, I am not going to read that entire manual.

> Sorry don't quite understand your second question.
> Do you mean how we trigger this state?
> We trigger this state with:
>  >     imx6ul-dev:~# echo freeze > /sys/power/state
> 

So it is not "suspend" ? I am not sure if it is appropriate to stop
the watchdog timer in this situation. Normally it is only stopped
in suspend mode.

Also, how does this interact or interfere with the suspend/resume code
in the driver, and does it behave the same for all chips supported by
the driver ? For example, in the current code, i.MX7D is handled
differently. What compatible entry are you using anyway ? There is none
for i.MX6UL. Did you make sure that the same bit doesn't mean something
else for other chips ?

Thanks,
Guenter

> If you mean what is done prior to entering this state?
> The device enters WAIT mode when CLPCR bit is set to WAIT. The device
> enters WAIT mode by gating the CPU clock, all other clocks can be gated
> by programming CGR bits in WAIT mode.
> 
> [1] i.MX Linux Reference Manual, Rev. 0, 07/2016
> 
> Andrej
> 
>>
>> Guenter
>>
>>> i.MX6UL watchdog has a WDW (Watchdog Disable for Wait) bit in WCR
>>> (Watchdog Control Register) which can put the watchdog in suspend when
>>> the device is put to WAIT mode. Similarly, WDZST bit is already set in
>>> imx2_wdt driver by default, which suspends the watchdog in STOP and DOZE
>>> modes.
>>>
>>> This RFC patch suspends watchdog when the device is in WAIT mode, which
>>> fixes our problem. During development, we noticed some reports where
>>> setting WDW bit caused inconsistent timeout events or inability of
>>> watchdog to reset the board. We didn't have these problems but I am
>>> curious if there is a case where device is put into WAIT mode and
>>> watchdog should be enabled?
>>>
>>> Maybe for cases where watchdog is used for WAIT mode supervision? So
>>> basically to reset the system if device doesn't exit WAIT mode on its
>>> own?
>>>
>>> The problem can be recreated with:
>>>
>>>     imx6ul-dev:~# echo freeze > /sys/power/state
>>>     [  101.093336] PM: suspend entry (s2idle)
>>>     [  101.097785] Filesystems sync: 0.000 seconds
>>>     [  101.122295] Freezing user space processes ... (elapsed 0.001 seconds) done.
>>>     [  101.130637] OOM killer disabled.
>>>     [  101.133998] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
>>>     [  101.142941] printk: Suspending console(s) (use no_console_suspend to debug)
>>>     ...
>>> Device resets after watchdog timeout expires! ~105s
>>>
>>> Thank you for your feedback.
>>>
>>> Best regards,
>>> Andrej
>>>
>>> Andrej Picej (1):
>>>    watchdog: imx2_wdg: suspend watchdog in WAIT mode
>>>
>>>   drivers/watchdog/imx2_wdt.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>
> 

Re: [RFC PATCH 0/1] Suspending i.MX watchdog in WAIT mode

[Andrej Picej]

On 22. 09. 22 16:56, Guenter Roeck wrote:
> On 9/22/22 00:17, Andrej Picej wrote:
>> Hi Guenter,
>>
>> On 21. 09. 22 16:18, Guenter Roeck wrote:
>>> On 9/21/22 05:46, Andrej Picej wrote:
>>>> Hi all,
>>>>
>>>> we are using i.MX6UL with its watchdog WDOG1 and kernel 5.15.62. It was
>>>> discovered that the watchdog triggers reset when the device is put into
>>>> 'Suspend-To-Idle' (WAIT) state.
>>>>
>>>
>>> Is that equivalent to "suspend" from Linux perspective, or some other
>>> mode ? How does the device get into this state ?
>>
>> I think WAIT mode maps to System idle mode in linux [1].
>>
> 
> Sorry, I am not going to read that entire manual.
> 

Perfectly understandable, you are busy guys, basically in chapter 21.1.1 
it is written:
"System idle maps to WAIT mode."

>> Sorry don't quite understand your second question.
>> Do you mean how we trigger this state?
>> We trigger this state with:
>>  >     imx6ul-dev:~# echo freeze > /sys/power/state
>>
> 
> So it is not "suspend" ? I am not sure if it is appropriate to stop
> the watchdog timer in this situation. Normally it is only stopped
> in suspend mode.
> 

Well suspend/resume functions are still called when entering this 
"freeze" mode, but watchdog can not be disabled by software or pinged in 
this mode, that's why our idea is to set this bit.

Basically, that was my main question. Is it appropriate to stop watchdog 
in this situation? I guess not. Probably it is not meant to enter this 
mode for longer period of time.

> Also, how does this interact or interfere with the suspend/resume code
> in the driver, and does it behave the same for all chips supported by
> the driver ? For example, in the current code, i.MX7D is handled
> differently. What compatible entry are you using anyway ? There is none
> for i.MX6UL. Did you make sure that the same bit doesn't mean something
> else for other chips ?
> 

I don't think it interact/interferes with suspend/resume code in any way 
since mx2+ watchdog is not stoppable during runtime. Not sure if 
watchdogs on other devices behave the same, sorry.

i.MX6UL devices use "fsl,imx6ul-wdt", "fsl,imx21-wdt" for compatible 
entry. I checked control registers with other supported devices by this 
driver:

- fsl,imx25-wdt (same behaviour -> WDW)
- fsl,imx27-wdt (bit is reserved)
- fsl,imx31-wdt (bit is reserved)
- fsl,imx35-wdt (same behaviour -> WDW)
- fsl,imx50-wdt (same behaviour -> WDW)
- fsl,imx51-wdt (same behaviour -> WDW)
- fsl,imx53-wdt (same behaviour -> WDW)
- fsl,imx6q-wdt (same behaviour -> WDW)
- fsl,imx6sl-wdt (same behaviour -> WDW)
- fsl,imx6sll-wdt (same behaviour -> WDW)
- fsl,imx6sx-wdt (same behaviour -> WDW)
- fsl,imx6ul-wdt (same behaviour -> WDW)
- fsl,imx7d-wdt (same behaviour -> WDW)
- fsl,imx8mm-wdt (same behaviour -> WDW)
- fsl,imx8mn-wdt (same behaviour -> WDW)
- fsl,imx8mp-wdt (same behaviour -> WDW)
- fsl,imx8mq-wdt (same behaviour -> WDW)
- fsl,ls1012a-wdt (bit is reserved)
- fsl,ls1043a-wdt (bit is reserved)
- fsl,vf610-wdt (same behaviour -> WDW)
- fsl,imx21-wdt (reserved)

Looking at this table WDW is not really i.MX6UL specific. It is strange 
that more people don't experience this problem. I guess using "freeze" 
mode is not very common, which is understandable, since other low power 
modes are more energy efficient.

Anyway, if some people are using this "feature" of watchdog for WAIT 
mode supervision, setting this bit would break their use. I just wanted 
to get your opinion on this, which I got.

Andrej

> Thanks,
> Guenter
> 
>> If you mean what is done prior to entering this state?
>> The device enters WAIT mode when CLPCR bit is set to WAIT. The device
>> enters WAIT mode by gating the CPU clock, all other clocks can be gated
>> by programming CGR bits in WAIT mode.
>>
>> [1] i.MX Linux Reference Manual, Rev. 0, 07/2016
>>
>> Andrej
>>
>>>
>>> Guenter
>>>
>>>> i.MX6UL watchdog has a WDW (Watchdog Disable for Wait) bit in WCR
>>>> (Watchdog Control Register) which can put the watchdog in suspend when
>>>> the device is put to WAIT mode. Similarly, WDZST bit is already set in
>>>> imx2_wdt driver by default, which suspends the watchdog in STOP and 
>>>> DOZE
>>>> modes.
>>>>
>>>> This RFC patch suspends watchdog when the device is in WAIT mode, which
>>>> fixes our problem. During development, we noticed some reports where
>>>> setting WDW bit caused inconsistent timeout events or inability of
>>>> watchdog to reset the board. We didn't have these problems but I am
>>>> curious if there is a case where device is put into WAIT mode and
>>>> watchdog should be enabled?
>>>>
>>>> Maybe for cases where watchdog is used for WAIT mode supervision? So
>>>> basically to reset the system if device doesn't exit WAIT mode on its
>>>> own?
>>>>
>>>> The problem can be recreated with:
>>>>
>>>>     imx6ul-dev:~# echo freeze > /sys/power/state
>>>>     [  101.093336] PM: suspend entry (s2idle)
>>>>     [  101.097785] Filesystems sync: 0.000 seconds
>>>>     [  101.122295] Freezing user space processes ... (elapsed 0.001 
>>>> seconds) done.
>>>>     [  101.130637] OOM killer disabled.
>>>>     [  101.133998] Freezing remaining freezable tasks ... (elapsed 
>>>> 0.001 seconds) done.
>>>>     [  101.142941] printk: Suspending console(s) (use 
>>>> no_console_suspend to debug)
>>>>     ...
>>>> Device resets after watchdog timeout expires! ~105s
>>>>
>>>> Thank you for your feedback.
>>>>
>>>> Best regards,
>>>> Andrej
>>>>
>>>> Andrej Picej (1):
>>>>    watchdog: imx2_wdg: suspend watchdog in WAIT mode
>>>>
>>>>   drivers/watchdog/imx2_wdt.c | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>
>>
> 

Re: [RFC PATCH 0/1] Suspending i.MX watchdog in WAIT mode

[Guenter Roeck]

On 9/23/22 00:27, Andrej Picej wrote:
> On 22. 09. 22 16:56, Guenter Roeck wrote:
>> On 9/22/22 00:17, Andrej Picej wrote:
>>> Hi Guenter,
>>>
>>> On 21. 09. 22 16:18, Guenter Roeck wrote:
>>>> On 9/21/22 05:46, Andrej Picej wrote:
>>>>> Hi all,
>>>>>
>>>>> we are using i.MX6UL with its watchdog WDOG1 and kernel 5.15.62. It was
>>>>> discovered that the watchdog triggers reset when the device is put into
>>>>> 'Suspend-To-Idle' (WAIT) state.
>>>>>
>>>>
>>>> Is that equivalent to "suspend" from Linux perspective, or some other
>>>> mode ? How does the device get into this state ?
>>>
>>> I think WAIT mode maps to System idle mode in linux [1].
>>>
>>
>> Sorry, I am not going to read that entire manual.
>>
> 
> Perfectly understandable, you are busy guys, basically in chapter 21.1.1 it is written:
> "System idle maps to WAIT mode."
> 
>>> Sorry don't quite understand your second question.
>>> Do you mean how we trigger this state?
>>> We trigger this state with:
>>>  >     imx6ul-dev:~# echo freeze > /sys/power/state
>>>
>>
>> So it is not "suspend" ? I am not sure if it is appropriate to stop
>> the watchdog timer in this situation. Normally it is only stopped
>> in suspend mode.
>>
> 
> Well suspend/resume functions are still called when entering this "freeze" mode, but watchdog can not be disabled by software or pinged in this mode, that's why our idea is to set this bit.
> 
> Basically, that was my main question. Is it appropriate to stop watchdog in this situation? I guess not. Probably it is not meant to enter this mode for longer period of time.
> 
>> Also, how does this interact or interfere with the suspend/resume code
>> in the driver, and does it behave the same for all chips supported by
>> the driver ? For example, in the current code, i.MX7D is handled
>> differently. What compatible entry are you using anyway ? There is none
>> for i.MX6UL. Did you make sure that the same bit doesn't mean something
>> else for other chips ?
>>
> 
> I don't think it interact/interferes with suspend/resume code in any way since mx2+ watchdog is not stoppable during runtime. Not sure if watchdogs on other devices behave the same, sorry.
> 
> i.MX6UL devices use "fsl,imx6ul-wdt", "fsl,imx21-wdt" for compatible entry. I checked control registers with other supported devices by this driver:
> 
> - fsl,imx25-wdt (same behaviour -> WDW)
> - fsl,imx27-wdt (bit is reserved)
> - fsl,imx31-wdt (bit is reserved)
> - fsl,imx35-wdt (same behaviour -> WDW)
> - fsl,imx50-wdt (same behaviour -> WDW)
> - fsl,imx51-wdt (same behaviour -> WDW)
> - fsl,imx53-wdt (same behaviour -> WDW)
> - fsl,imx6q-wdt (same behaviour -> WDW)
> - fsl,imx6sl-wdt (same behaviour -> WDW)
> - fsl,imx6sll-wdt (same behaviour -> WDW)
> - fsl,imx6sx-wdt (same behaviour -> WDW)
> - fsl,imx6ul-wdt (same behaviour -> WDW)
> - fsl,imx7d-wdt (same behaviour -> WDW)
> - fsl,imx8mm-wdt (same behaviour -> WDW)
> - fsl,imx8mn-wdt (same behaviour -> WDW)
> - fsl,imx8mp-wdt (same behaviour -> WDW)
> - fsl,imx8mq-wdt (same behaviour -> WDW)
> - fsl,ls1012a-wdt (bit is reserved)
> - fsl,ls1043a-wdt (bit is reserved)
> - fsl,vf610-wdt (same behaviour -> WDW)
> - fsl,imx21-wdt (reserved)
> 

And then there is fsl,imx7ulp-wdt which has its own driver.

> Looking at this table WDW is not really i.MX6UL specific. It is strange that more people don't experience this problem. I guess using "freeze" mode is not very common, which is understandable, since other low power modes are more energy efficient.
>  > Anyway, if some people are using this "feature" of watchdog for WAIT mode supervision, setting this bit would break their use. I just wanted to get your opinion on this, which I got.
> 

Oh, no, you are just giving up too early.

Other watchdogs do get stopped in suspend. That is why the suspend function
exists and is used in watchdog drivers in the first place. Yes, fsl,imx21-wdt
can not handle that, but that doesn't mean that others can't either. The
current workaround for fsl,imx21-wdt is to set the timeout to the maximum
and to stop the watchdog timer clock. You say that the suspend function is
called, so the watchdog clock _should_ be stopped. However, it looks like
that is not the case. Can you try to figure out the reason ?

Either case, I think it would be worthwhile exploring if the WDW bit can
be used on the CPUs supporting it to stop the watchdog in suspend mode.
How does your sysytem behave if put into suspend ? Does the watchdog still
reset it ? How is the watchdog clock configured ?

Thanks,
Guenter

Re: [RFC PATCH 0/1] Suspending i.MX watchdog in WAIT mode

[Andrej Picej]

On 23. 09. 22 15:48, Guenter Roeck wrote:
> On 9/23/22 00:27, Andrej Picej wrote:
>> On 22. 09. 22 16:56, Guenter Roeck wrote:
>>> On 9/22/22 00:17, Andrej Picej wrote:
>>>> Hi Guenter,
>>>>
>>>> On 21. 09. 22 16:18, Guenter Roeck wrote:
>>>>> On 9/21/22 05:46, Andrej Picej wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> we are using i.MX6UL with its watchdog WDOG1 and kernel 5.15.62. 
>>>>>> It was
>>>>>> discovered that the watchdog triggers reset when the device is put 
>>>>>> into
>>>>>> 'Suspend-To-Idle' (WAIT) state.
>>>>>>
>>>>>
>>>>> Is that equivalent to "suspend" from Linux perspective, or some other
>>>>> mode ? How does the device get into this state ?
>>>>
>>>> I think WAIT mode maps to System idle mode in linux [1].
>>>>
>>>
>>> Sorry, I am not going to read that entire manual.
>>>
>>
>> Perfectly understandable, you are busy guys, basically in chapter 
>> 21.1.1 it is written:
>> "System idle maps to WAIT mode."
>>
>>>> Sorry don't quite understand your second question.
>>>> Do you mean how we trigger this state?
>>>> We trigger this state with:
>>>>  >     imx6ul-dev:~# echo freeze > /sys/power/state
>>>>
>>>
>>> So it is not "suspend" ? I am not sure if it is appropriate to stop
>>> the watchdog timer in this situation. Normally it is only stopped
>>> in suspend mode.
>>>
>>
>> Well suspend/resume functions are still called when entering this 
>> "freeze" mode, but watchdog can not be disabled by software or pinged 
>> in this mode, that's why our idea is to set this bit.
>>
>> Basically, that was my main question. Is it appropriate to stop 
>> watchdog in this situation? I guess not. Probably it is not meant to 
>> enter this mode for longer period of time.
>>
>>> Also, how does this interact or interfere with the suspend/resume code
>>> in the driver, and does it behave the same for all chips supported by
>>> the driver ? For example, in the current code, i.MX7D is handled
>>> differently. What compatible entry are you using anyway ? There is none
>>> for i.MX6UL. Did you make sure that the same bit doesn't mean something
>>> else for other chips ?
>>>
>>
>> I don't think it interact/interferes with suspend/resume code in any 
>> way since mx2+ watchdog is not stoppable during runtime. Not sure if 
>> watchdogs on other devices behave the same, sorry.
>>
>> i.MX6UL devices use "fsl,imx6ul-wdt", "fsl,imx21-wdt" for compatible 
>> entry. I checked control registers with other supported devices by 
>> this driver:
>>
>> - fsl,imx25-wdt (same behaviour -> WDW)
>> - fsl,imx27-wdt (bit is reserved)
>> - fsl,imx31-wdt (bit is reserved)
>> - fsl,imx35-wdt (same behaviour -> WDW)
>> - fsl,imx50-wdt (same behaviour -> WDW)
>> - fsl,imx51-wdt (same behaviour -> WDW)
>> - fsl,imx53-wdt (same behaviour -> WDW)
>> - fsl,imx6q-wdt (same behaviour -> WDW)
>> - fsl,imx6sl-wdt (same behaviour -> WDW)
>> - fsl,imx6sll-wdt (same behaviour -> WDW)
>> - fsl,imx6sx-wdt (same behaviour -> WDW)
>> - fsl,imx6ul-wdt (same behaviour -> WDW)
>> - fsl,imx7d-wdt (same behaviour -> WDW)
>> - fsl,imx8mm-wdt (same behaviour -> WDW)
>> - fsl,imx8mn-wdt (same behaviour -> WDW)
>> - fsl,imx8mp-wdt (same behaviour -> WDW)
>> - fsl,imx8mq-wdt (same behaviour -> WDW)
>> - fsl,ls1012a-wdt (bit is reserved)
>> - fsl,ls1043a-wdt (bit is reserved)
>> - fsl,vf610-wdt (same behaviour -> WDW)
>> - fsl,imx21-wdt (reserved)
>>
> 
> And then there is fsl,imx7ulp-wdt which has its own driver.
> 
>> Looking at this table WDW is not really i.MX6UL specific. It is 
>> strange that more people don't experience this problem. I guess using 
>> "freeze" mode is not very common, which is understandable, since other 
>> low power modes are more energy efficient.
>>  > Anyway, if some people are using this "feature" of watchdog for 
>> WAIT mode supervision, setting this bit would break their use. I just 
>> wanted to get your opinion on this, which I got.
>>
> 
> Oh, no, you are just giving up too early.

Well I'm not really sure if it is worth the struggle. But if you insist, 
I'll dig a bit deeper :).

> 
> Other watchdogs do get stopped in suspend. That is why the suspend function
> exists and is used in watchdog drivers in the first place. Yes, 
> fsl,imx21-wdt
> can not handle that, but that doesn't mean that others can't either. The
> current workaround for fsl,imx21-wdt is to set the timeout to the maximum
> and to stop the watchdog timer clock. You say that the suspend function is
> called, so the watchdog clock _should_ be stopped. However, it looks like
> that is not the case. Can you try to figure out the reason ?

Ok, so a quick look at clocks used in watchdog reveals an interesting 
find. The watchdog uses two clocks:
- low-frequency reference clock (ipg_clk_32k) for its counter and 
control operation and
- peripheral bus clock for register read/write operations.

 From reference manual:
> Low frequency (32.768 kHz) clock that continues to run in low-power
> mode. It is assumed that the Clock Controller will provide this clock signal
> synchronized to ipg_clk in the normal mode, and switch to a non-
> synchronized signal in low-power mode when the ipg_clk is off.

In suspend function we are stopping wdog1_clk which is child of ipg_clk. 
This low-frequency reference clock is left running, which means that 
watchdog counter keeps running. The problem is that ipg_clk_32k is meant 
to be running in low-power mode, that's why this clock can not be gated 
off (no gating register).

> 
> Either case, I think it would be worthwhile exploring if the WDW bit can
> be used on the CPUs supporting it to stop the watchdog in suspend mode.
> How does your sysytem behave if put into suspend ? Does the watchdog still
> reset it ? How is the watchdog clock configured ?

If the system is put into suspend mode, the watchdog is suspended, due 
to WDZST bit, which similar to WDW bit suspends the watchdog in 
low-power modes (STOP and DOZE). If this WDZST bit is NOT set, the 
watchdog triggers when system is put into "Suspend-to-RAM" and 
"Standby/Power-On Suspend".

I had a look at some of other devices watchdogs this driver supports and 
it looks like i.MX6, i.MX8, i.MX7 and i.MX5x have this low-frequency 
reference clock in common.

Andrej

> 
> Thanks,
> Guenter

Re: [RFC PATCH 0/1] Suspending i.MX watchdog in WAIT mode

[Andrej Picej]

On 26. 09. 22 09:36, Andrej Picej wrote:
> 
> 
> On 23. 09. 22 15:48, Guenter Roeck wrote:
>> On 9/23/22 00:27, Andrej Picej wrote:
>>> On 22. 09. 22 16:56, Guenter Roeck wrote:
>>>> On 9/22/22 00:17, Andrej Picej wrote:
>>>>> Hi Guenter,
>>>>>
>>>>> On 21. 09. 22 16:18, Guenter Roeck wrote:
>>>>>> On 9/21/22 05:46, Andrej Picej wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> we are using i.MX6UL with its watchdog WDOG1 and kernel 5.15.62. 
>>>>>>> It was
>>>>>>> discovered that the watchdog triggers reset when the device is 
>>>>>>> put into
>>>>>>> 'Suspend-To-Idle' (WAIT) state.
>>>>>>>
>>>>>>
>>>>>> Is that equivalent to "suspend" from Linux perspective, or some other
>>>>>> mode ? How does the device get into this state ?
>>>>>
>>>>> I think WAIT mode maps to System idle mode in linux [1].
>>>>>
>>>>
>>>> Sorry, I am not going to read that entire manual.
>>>>
>>>
>>> Perfectly understandable, you are busy guys, basically in chapter 
>>> 21.1.1 it is written:
>>> "System idle maps to WAIT mode."
>>>
>>>>> Sorry don't quite understand your second question.
>>>>> Do you mean how we trigger this state?
>>>>> We trigger this state with:
>>>>>  >     imx6ul-dev:~# echo freeze > /sys/power/state
>>>>>
>>>>
>>>> So it is not "suspend" ? I am not sure if it is appropriate to stop
>>>> the watchdog timer in this situation. Normally it is only stopped
>>>> in suspend mode.
>>>>
>>>
>>> Well suspend/resume functions are still called when entering this 
>>> "freeze" mode, but watchdog can not be disabled by software or pinged 
>>> in this mode, that's why our idea is to set this bit.
>>>
>>> Basically, that was my main question. Is it appropriate to stop 
>>> watchdog in this situation? I guess not. Probably it is not meant to 
>>> enter this mode for longer period of time.
>>>
>>>> Also, how does this interact or interfere with the suspend/resume code
>>>> in the driver, and does it behave the same for all chips supported by
>>>> the driver ? For example, in the current code, i.MX7D is handled
>>>> differently. What compatible entry are you using anyway ? There is none
>>>> for i.MX6UL. Did you make sure that the same bit doesn't mean something
>>>> else for other chips ?
>>>>
>>>
>>> I don't think it interact/interferes with suspend/resume code in any 
>>> way since mx2+ watchdog is not stoppable during runtime. Not sure if 
>>> watchdogs on other devices behave the same, sorry.
>>>
>>> i.MX6UL devices use "fsl,imx6ul-wdt", "fsl,imx21-wdt" for compatible 
>>> entry. I checked control registers with other supported devices by 
>>> this driver:
>>>
>>> - fsl,imx25-wdt (same behaviour -> WDW)
>>> - fsl,imx27-wdt (bit is reserved)
>>> - fsl,imx31-wdt (bit is reserved)
>>> - fsl,imx35-wdt (same behaviour -> WDW)
>>> - fsl,imx50-wdt (same behaviour -> WDW)
>>> - fsl,imx51-wdt (same behaviour -> WDW)
>>> - fsl,imx53-wdt (same behaviour -> WDW)
>>> - fsl,imx6q-wdt (same behaviour -> WDW)
>>> - fsl,imx6sl-wdt (same behaviour -> WDW)
>>> - fsl,imx6sll-wdt (same behaviour -> WDW)
>>> - fsl,imx6sx-wdt (same behaviour -> WDW)
>>> - fsl,imx6ul-wdt (same behaviour -> WDW)
>>> - fsl,imx7d-wdt (same behaviour -> WDW)
>>> - fsl,imx8mm-wdt (same behaviour -> WDW)
>>> - fsl,imx8mn-wdt (same behaviour -> WDW)
>>> - fsl,imx8mp-wdt (same behaviour -> WDW)
>>> - fsl,imx8mq-wdt (same behaviour -> WDW)
>>> - fsl,ls1012a-wdt (bit is reserved)
>>> - fsl,ls1043a-wdt (bit is reserved)
>>> - fsl,vf610-wdt (same behaviour -> WDW)
>>> - fsl,imx21-wdt (reserved)
>>>
>>
>> And then there is fsl,imx7ulp-wdt which has its own driver.
>>
>>> Looking at this table WDW is not really i.MX6UL specific. It is 
>>> strange that more people don't experience this problem. I guess using 
>>> "freeze" mode is not very common, which is understandable, since 
>>> other low power modes are more energy efficient.
>>>  > Anyway, if some people are using this "feature" of watchdog for 
>>> WAIT mode supervision, setting this bit would break their use. I just 
>>> wanted to get your opinion on this, which I got.
>>>
>>
>> Oh, no, you are just giving up too early.
> 
> Well I'm not really sure if it is worth the struggle. But if you insist, 
> I'll dig a bit deeper :).
> 
>>
>> Other watchdogs do get stopped in suspend. That is why the suspend 
>> function
>> exists and is used in watchdog drivers in the first place. Yes, 
>> fsl,imx21-wdt
>> can not handle that, but that doesn't mean that others can't either. The
>> current workaround for fsl,imx21-wdt is to set the timeout to the maximum
>> and to stop the watchdog timer clock. You say that the suspend 
>> function is
>> called, so the watchdog clock _should_ be stopped. However, it looks like
>> that is not the case. Can you try to figure out the reason ?
> 
> Ok, so a quick look at clocks used in watchdog reveals an interesting 
> find. The watchdog uses two clocks:
> - low-frequency reference clock (ipg_clk_32k) for its counter and 
> control operation and
> - peripheral bus clock for register read/write operations.
> 
>  From reference manual:
>> Low frequency (32.768 kHz) clock that continues to run in low-power
>> mode. It is assumed that the Clock Controller will provide this clock 
>> signal
>> synchronized to ipg_clk in the normal mode, and switch to a non-
>> synchronized signal in low-power mode when the ipg_clk is off.
> 
> In suspend function we are stopping wdog1_clk which is child of ipg_clk. 
> This low-frequency reference clock is left running, which means that 
> watchdog counter keeps running. The problem is that ipg_clk_32k is meant 
> to be running in low-power mode, that's why this clock can not be gated 
> off (no gating register).
> 
>>
>> Either case, I think it would be worthwhile exploring if the WDW bit can
>> be used on the CPUs supporting it to stop the watchdog in suspend mode.
>> How does your sysytem behave if put into suspend ? Does the watchdog 
>> still
>> reset it ? How is the watchdog clock configured ?
> 
> If the system is put into suspend mode, the watchdog is suspended, due 
> to WDZST bit, which similar to WDW bit suspends the watchdog in 
> low-power modes (STOP and DOZE). If this WDZST bit is NOT set, the 
> watchdog triggers when system is put into "Suspend-to-RAM" and 
> "Standby/Power-On Suspend".
> 
> I had a look at some of other devices watchdogs this driver supports and 
> it looks like i.MX6, i.MX8, i.MX7 and i.MX5x have this low-frequency 
> reference clock in common.
> 

What if we used a new device-tree property "fsl,suspend-in-wait" to set 
WDW bit? And we leave WDW bit unset if this property is not defined, 
keeping the default watchdog behaviour as it is? Would that be acceptable?

Best regards,
Andrej