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

[Andrej Picej <andrej.picej@norik.com>]

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