[0/3] Suspending i.MX watchdog in WAIT mode

  The i.MX6 watchdog can't be stopped once started. This means that
special hardware suspend needs to be configured when the device enters
low-power modes.
Usually i.MX devices have two bits which deal with this:
- WDZST bit disables the timer in "deeper" low power modes and
- WDW bit disables the timer in "WAIT" mode which corresponds with
Linux's "freeze" low-power mode.

WDZST bit support is already in place since 1a9c5efa576e ("watchdog: imx2_wdt: disable watchdog timer during low power mode").
WDW bit is not common for all imx2-wdt supported devices, therefore use
a new device-tree property "fsl,suspend-in-wait" which suspends the
watchdog in "WAIT" mode.

Andrej Picej (3):
  watchdog: imx2_wdg: suspend watchdog in WAIT mode
  dt-bindings: watchdog: fsl-imx: document suspend in wait mode
  ARM: dts: imx6ul/ull: suspend i.MX6UL watchdog in wait mode

 .../devicetree/bindings/watchdog/fsl-imx-wdt.yaml          | 5 +++++
 arch/arm/boot/dts/imx6ul-phytec-phycore-som.dtsi           | 4 ++++
 drivers/watchdog/imx2_wdt.c                                | 7 +++++++
 3 files changed, 16 insertions(+)
  

Hi Andrej,

On Wed, Oct 19, 2022 at 8:17 AM Andrej Picej <andrej.picej@norik.com> wrote:
>
> The i.MX6 watchdog can't be stopped once started. This means that
> special hardware suspend needs to be configured when the device enters
> low-power modes.
> Usually i.MX devices have two bits which deal with this:
> - WDZST bit disables the timer in "deeper" low power modes and
> - WDW bit disables the timer in "WAIT" mode which corresponds with
> Linux's "freeze" low-power mode.
>
> WDZST bit support is already in place since 1a9c5efa576e ("watchdog: imx2_wdt: disable watchdog timer during low power mode").
> WDW bit is not common for all imx2-wdt supported devices, therefore use
> a new device-tree property "fsl,suspend-in-wait" which suspends the
> watchdog in "WAIT" mode.
>
> Andrej Picej (3):
>   watchdog: imx2_wdg: suspend watchdog in WAIT mode
>   dt-bindings: watchdog: fsl-imx: document suspend in wait mode
>   ARM: dts: imx6ul/ull: suspend i.MX6UL watchdog in wait mode

For the series:

Reviewed-by: Fabio Estevam <festevam@gmail.com>
  

Hello Andrej,

Am Mittwoch, 19. Oktober 2022, 13:17:13 CEST schrieb Andrej Picej:
> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> ---
>  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?

Best regards,
Alexander
  

On Wed, Oct 19, 2022 at 01:17:11PM +0200, Andrej Picej wrote:
> The i.MX6 watchdog can't be stopped once started. This means that
> special hardware suspend needs to be configured when the device enters
> low-power modes.
> Usually i.MX devices have two bits which deal with this:
> - WDZST bit disables the timer in "deeper" low power modes and
> - WDW bit disables the timer in "WAIT" mode which corresponds with
> Linux's "freeze" low-power mode.
> 
> WDZST bit support is already in place since 1a9c5efa576e ("watchdog: imx2_wdt: disable watchdog timer during low power mode").
> WDW bit is not common for all imx2-wdt supported devices, therefore use
> a new device-tree property "fsl,suspend-in-wait" which suspends the
> watchdog in "WAIT" mode.

I think that needs to be validated using the "compatible" property;
it should not be possible to set/accept the new flag for devices
which don't support it.

Thanks,
Guenter

> 
> Andrej Picej (3):
>   watchdog: imx2_wdg: suspend watchdog in WAIT mode
>   dt-bindings: watchdog: fsl-imx: document suspend in wait mode
>   ARM: dts: imx6ul/ull: suspend i.MX6UL watchdog in wait mode
> 
>  .../devicetree/bindings/watchdog/fsl-imx-wdt.yaml          | 5 +++++
>  arch/arm/boot/dts/imx6ul-phytec-phycore-som.dtsi           | 4 ++++
>  drivers/watchdog/imx2_wdt.c                                | 7 +++++++
>  3 files changed, 16 insertions(+)
> 
> -- 
> 2.25.1
>
  

On 19/10/2022 07:17, Andrej Picej wrote:
> The i.MX6 watchdog can't be stopped once started. This means that
> special hardware suspend needs to be configured when the device enters
> low-power modes.
> Usually i.MX devices have two bits which deal with this:
> - WDZST bit disables the timer in "deeper" low power modes and
> - WDW bit disables the timer in "WAIT" mode which corresponds with
> Linux's "freeze" low-power mode.
> 
> WDZST bit support is already in place since 1a9c5efa576e ("watchdog: imx2_wdt: disable watchdog timer during low power mode").
> WDW bit is not common for all imx2-wdt supported devices, therefore use
> a new device-tree property "fsl,suspend-in-wait" which suspends the
> watchdog in "WAIT" mode.
> 
> Andrej Picej (3):
>   watchdog: imx2_wdg: suspend watchdog in WAIT mode
>   dt-bindings: watchdog: fsl-imx: document suspend in wait mode
>   ARM: dts: imx6ul/ull: suspend i.MX6UL watchdog in wait mode
> 
>  .../devicetree/bindings/watchdog/fsl-imx-wdt.yaml          | 5 +++++

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.

Best regards,
Krzysztof
  

Hi Guenter,

On 19. 10. 22 17:30, Guenter Roeck wrote:
> On Wed, Oct 19, 2022 at 01:17:11PM +0200, Andrej Picej wrote:
>> The i.MX6 watchdog can't be stopped once started. This means that
>> special hardware suspend needs to be configured when the device enters
>> low-power modes.
>> Usually i.MX devices have two bits which deal with this:
>> - WDZST bit disables the timer in "deeper" low power modes and
>> - WDW bit disables the timer in "WAIT" mode which corresponds with
>> Linux's "freeze" low-power mode.
>>
>> WDZST bit support is already in place since 1a9c5efa576e ("watchdog: imx2_wdt: disable watchdog timer during low power mode").
>> WDW bit is not common for all imx2-wdt supported devices, therefore use
>> a new device-tree property "fsl,suspend-in-wait" which suspends the
>> watchdog in "WAIT" mode.
> 
> I think that needs to be validated using the "compatible" property;
> it should not be possible to set/accept the new flag for devices
> which don't support it.

Ok, I can add that to a v2.

Thanks,
Andrej

> 
> Thanks,
> Guenter
>
  

Hi Krzysztof,

On 19. 10. 22 17:46, Krzysztof Kozlowski wrote:
> On 19/10/2022 07:17, Andrej Picej wrote:
>> The i.MX6 watchdog can't be stopped once started. This means that
>> special hardware suspend needs to be configured when the device enters
>> low-power modes.
>> Usually i.MX devices have two bits which deal with this:
>> - WDZST bit disables the timer in "deeper" low power modes and
>> - WDW bit disables the timer in "WAIT" mode which corresponds with
>> Linux's "freeze" low-power mode.
>>
>> WDZST bit support is already in place since 1a9c5efa576e ("watchdog: imx2_wdt: disable watchdog timer during low power mode").
>> WDW bit is not common for all imx2-wdt supported devices, therefore use
>> a new device-tree property "fsl,suspend-in-wait" which suspends the
>> watchdog in "WAIT" mode.
>>
>> Andrej Picej (3):
>>    watchdog: imx2_wdg: suspend watchdog in WAIT mode
>>    dt-bindings: watchdog: fsl-imx: document suspend in wait mode
>>    ARM: dts: imx6ul/ull: suspend i.MX6UL watchdog in wait mode
>>
>>   .../devicetree/bindings/watchdog/fsl-imx-wdt.yaml          | 5 +++++
> 
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC.  It might happen, that command when run on an older
> kernel, gives you outdated entries.  Therefore please be sure you base
> your patches on recent Linux kernel.

I thought I did. I run that script on linux-watchdog.git, master branch.
I thought I should base my patches meant for watchdog subsystem there?

Best regards,
Andrej

> 
> Best regards,
> Krzysztof
>
  

On 20/10/2022 01:49, Andrej Picej wrote:
> Hi Krzysztof,
> 
> On 19. 10. 22 17:46, Krzysztof Kozlowski wrote:
>> On 19/10/2022 07:17, Andrej Picej wrote:
>>> The i.MX6 watchdog can't be stopped once started. This means that
>>> special hardware suspend needs to be configured when the device enters
>>> low-power modes.
>>> Usually i.MX devices have two bits which deal with this:
>>> - WDZST bit disables the timer in "deeper" low power modes and
>>> - WDW bit disables the timer in "WAIT" mode which corresponds with
>>> Linux's "freeze" low-power mode.
>>>
>>> WDZST bit support is already in place since 1a9c5efa576e ("watchdog: imx2_wdt: disable watchdog timer during low power mode").
>>> WDW bit is not common for all imx2-wdt supported devices, therefore use
>>> a new device-tree property "fsl,suspend-in-wait" which suspends the
>>> watchdog in "WAIT" mode.
>>>
>>> Andrej Picej (3):
>>>    watchdog: imx2_wdg: suspend watchdog in WAIT mode
>>>    dt-bindings: watchdog: fsl-imx: document suspend in wait mode
>>>    ARM: dts: imx6ul/ull: suspend i.MX6UL watchdog in wait mode
>>>
>>>   .../devicetree/bindings/watchdog/fsl-imx-wdt.yaml          | 5 +++++
>>
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC.  It might happen, that command when run on an older
>> kernel, gives you outdated entries.  Therefore please be sure you base
>> your patches on recent Linux kernel.
> 
> I thought I did. I run that script on linux-watchdog.git, master branch.
> I thought I should base my patches meant for watchdog subsystem there?

Maintainer's tree should be fine, but then the issue is somewhere else,
because your CC list was not complete.


Best regards,
Krzysztof
  

On 20. 10. 22 14:04, Krzysztof Kozlowski wrote:
> On 20/10/2022 01:49, Andrej Picej wrote:
>> Hi Krzysztof,
>>
>> On 19. 10. 22 17:46, Krzysztof Kozlowski wrote:
>>> On 19/10/2022 07:17, Andrej Picej wrote:
>>>> The i.MX6 watchdog can't be stopped once started. This means that
>>>> special hardware suspend needs to be configured when the device enters
>>>> low-power modes.
>>>> Usually i.MX devices have two bits which deal with this:
>>>> - WDZST bit disables the timer in "deeper" low power modes and
>>>> - WDW bit disables the timer in "WAIT" mode which corresponds with
>>>> Linux's "freeze" low-power mode.
>>>>
>>>> WDZST bit support is already in place since 1a9c5efa576e ("watchdog: imx2_wdt: disable watchdog timer during low power mode").
>>>> WDW bit is not common for all imx2-wdt supported devices, therefore use
>>>> a new device-tree property "fsl,suspend-in-wait" which suspends the
>>>> watchdog in "WAIT" mode.
>>>>
>>>> Andrej Picej (3):
>>>>     watchdog: imx2_wdg: suspend watchdog in WAIT mode
>>>>     dt-bindings: watchdog: fsl-imx: document suspend in wait mode
>>>>     ARM: dts: imx6ul/ull: suspend i.MX6UL watchdog in wait mode
>>>>
>>>>    .../devicetree/bindings/watchdog/fsl-imx-wdt.yaml          | 5 +++++
>>>
>>> Please use scripts/get_maintainers.pl to get a list of necessary people
>>> and lists to CC.  It might happen, that command when run on an older
>>> kernel, gives you outdated entries.  Therefore please be sure you base
>>> your patches on recent Linux kernel.
>>
>> I thought I did. I run that script on linux-watchdog.git, master branch.
>> I thought I should base my patches meant for watchdog subsystem there?
> 
> Maintainer's tree should be fine, but then the issue is somewhere else,
> because your CC list was not complete.
> 

Ok I see that two email addresses were not added. Sorry for that. I will 
make sure that the CC list is complete next time.

Thanks,
Andrej