Re: Re: [PATCH 2/3] dt-bindings: watchdog: fsl-imx: document suspend in wait mode

[Alexander Stein]

Am Donnerstag, 20. Oktober 2022, 14:36:10 CEST schrieb Andrej Picej:
> On 20. 10. 22 14:18, Krzysztof Kozlowski wrote:
> > On 20/10/2022 02:23, Andrej Picej wrote:
> >> Hi Alexander and Krzysztof,
> >> 
> >> hope I can reply to both questions here.
> >> 
> >> On 19. 10. 22 17:51, Krzysztof Kozlowski wrote:
> >>> On 19/10/2022 09:00, Alexander Stein wrote:
> >>>> Hello Andrej,
> >>> 
> >>>> Am Mittwoch, 19. Oktober 2022, 13:17:13 CEST schrieb Andrej Picej:
> >>> Missing commit msg.
> >>> 
> >>>>> Signed-off-by: Andrej Picej <andrej.picej@xxxxxxxxx>
> >>>>> ---
> >>>>> 
> >>>>>    Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.yaml | 5
> >>>>>    +++++
> >>>>>    1 file changed, 5 insertions(+)
> >>>>> 
> >>>>> diff --git
> >>>>> a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.yaml
> >>>>> b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.yaml index
> >>>>> fb7695515be1..01b3e04e7e65 100644
> >>>>> --- a/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/watchdog/fsl-imx-wdt.yaml
> >>>>> 
> >>>>> @@ -55,6 +55,11 @@ properties:
> >>>>>          If present, the watchdog device is configured to assert its
> >>>>>          external reset (WDOG_B) instead of issuing a software reset.
> >>>>> 
> >>>>> +  fsl,suspend-in-wait:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/flag
> >>>>> +    description: |
> >>>>> +      If present, the watchdog device is suspended in WAIT mode.
> >>>>> +
> >>>>> 
> >>>>>    required:
> >>>>>      - compatible
> >>>>>      - interrupts
> >>>> 
> >>>> What is the condition the watchdog is suspended in WAIT mode? Is this
> >>>> specific to SoC or platform or something else?
> >> 
> >> Sorry, what exactly do you mean by condition?
> > 
> > Ugh, I also cannot parse it now...

Sorry, Krzysztof already asked the right question: When does one want to 
enable/disable this feature?

> >> When the property
> >> "fsl,suspend-in-wait" is set the watchdog is suspended in WAIT mode, so
> >> this is defined by the user. Didn't want to apply it for all the
> >> supported machines since there could be devices which depend on watchdog
> >> triggering in WAIT mode. We stumbled on this problem on imx6 devices,
> >> but the same bit (with the same description) is found on imx25, imx35,
> >> imx50/51/53, imx7 and imx8.
> > 
> > I meant, what is expected to happen if you do not enable this bit and
> > watchdog triggers in WAIT mode? IOW, why someone might want to enable or
> > disable this property?
> 
> If this is not enabled and you put the device into the Suspend-to-idle
> mode the device resets after 128 seconds. If not, the device can be left
> in that state for infinite time. I'm guessing you want me to better
> explain the property in device tree docs right?
> I can do that in v2.
> 
> >>> And what happens else? When it is not suspended in WAIT mode?
> >> 
> >> When you put the device in "freeze"/"Suspend-To-Idle" low-power mode the
> >> watchdog keeps running and triggers a reset after 128 seconds. So the
> >> maximum length the device can stay in this mode is limited to 128
> >> seconds.
> > 
> > And who wakes up the system before 128 seconds? IOW is there a use case
> > of not enabling this property?
> 
> Well I can think of one, system can be woken up by some other interrupt.
> Like RTC which triggers interrupt (for example every 10s). So if this
> property is left disabled the watchdog can handle errors where other
> wakeup sources don't trigger interrupt or if the system is unable to
> wake from low-power state. In that case the watchdog will do a hard
> reset of the device.
> 
> But I'm not really sure if anybody uses this, just wanted to make sure
> that we keep the default behaviour as it is, since this driver is used
> by many devices and for quite some time.

This sounds more like (application) configuration. If so this should not be 
configured in device tree, IMHO.

Best regards,
Alexander