lkml.org 
[lkml]   [2022]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 1/3] watchdog: imx2_wdg: suspend watchdog in WAIT mode
From


On 26. 10. 22 08:01, Alexander Stein wrote:
> Hello Andrej,
>
> Am Dienstag, 25. Oktober 2022, 13:21:18 CEST schrieb Andrej Picej:
>> Hi Alexander,
>>
>> On 25. 10. 22 11:38, Alexander Stein wrote:
>>> Am Dienstag, 25. Oktober 2022, 09:25:31 CEST schrieb Andrej Picej:
>>>> Putting device into the "Suspend-To-Idle" mode causes watchdog to
>>>> trigger and reset the board after set watchdog timeout period elapses.
>>>>
>>>> Introduce new device-tree property "fsl,suspend-in-wait" which suspends
>>>> watchdog in WAIT mode. This is done by setting WDW bit in WCR
>>>> (Watchdog Control Register) Watchdog operation is restored after exiting
>>>> WAIT mode as expected. WAIT mode coresponds with Linux's
>>>> "Suspend-To-Idle".
>>>>
>>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>>> Reviewed-by: Fabio Estevam <festevam@gmail.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - validate the property with compatible string, as this functionality
>>>>
>>>> is not supported by all devices.
>>>>
>>>> ---
>>>>
>>>> drivers/watchdog/imx2_wdt.c | 37 +++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 37 insertions(+)
>>>>
>>>> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
>>>> index d0c5d47ddede..dd9866c6f1e5 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
>>>
>>> */
>>>
>>>> @@ -67,6 +68,27 @@ struct imx2_wdt_device {
>>>>
>>>> bool ext_reset;
>>>> bool clk_is_on;
>>>> bool no_ping;
>>>>
>>>> + bool sleep_wait;
>>>> +};
>>>> +
>>>> +static const char * const wdw_boards[] __initconst = {
>>>> + "fsl,imx25-wdt",
>>>> + "fsl,imx35-wdt",
>>>> + "fsl,imx50-wdt",
>>>> + "fsl,imx51-wdt",
>>>> + "fsl,imx53-wdt",
>>>> + "fsl,imx6q-wdt",
>>>> + "fsl,imx6sl-wdt",
>>>> + "fsl,imx6sll-wdt",
>>>> + "fsl,imx6sx-wdt",
>>>> + "fsl,imx6ul-wdt",
>>>> + "fsl,imx7d-wdt",
>>>> + "fsl,imx8mm-wdt",
>>>> + "fsl,imx8mn-wdt",
>>>> + "fsl,imx8mp-wdt",
>>>> + "fsl,imx8mq-wdt",
>>>> + "fsl,vf610-wdt",
>>>> + NULL
>>>>
>>>> };
>>>
>>> So the models listed in
>>> Documentation/devicetree/bindings/watchdog/fsl-imx-
>>> wdt.yaml not supporting this feature are
>>> * fsl,imx21-wdt
>>> * fsl,imx27-wdt
>>> * fsl,imx31-wdt
>>> * fsl,ls1012a-wdt
>>> * fsl,ls1043a-wdt
>>> ?
>>
>> yes, you are correct.
>>
>>> But all models are listed as compatible to fsl,imx21-wdt. So there is
>>> something wrong here. IMHO this sounds like the compatible list has to be
>>> split and updated. Depending on that this feature can be detected.
>>> Maintaining another list seems error prone to me.
>>
>> So basically the compatible lists would be split into two groups, one
>> for the devices which support this WDW bit and the rest which don't
>> support this?
>
> This was my idea, so only one set has to be maintained.
>
>> You got a point here, but...this means that every processors
>> device-tree, which has two compatible strings (with "fsl,imx21-wdt")
>> should be updated, right? That sounds like quite a lot of changes, which
>> I'd like to avoid if possible.
>
> Well, the compatible list right now doesn't reflect the hardware features/
> compatibility correctly, so IMHO it should be fixed.
> But apparently Krzysztof is okay having the special property only applicable
> for a specific set of devices. But in this case you will have to maintain two
> sets of device models (bindings + driver) to which WDW applies/does not apply
> to.
>
Ok, lets see what @Krzysztof has to say about this.

Best regards,
Andrej

\
 
 \ /
  Last update: 2022-10-26 09:04    [W:0.261 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site