* [PATCH v2] watchdog: iTCO_wdt: No need to stop the timer in probe
@ 2021-09-21 10:29 Mika Westerberg
2021-09-21 12:31 ` Guenter Roeck
2023-07-05 11:22 ` Jan Kiszka
0 siblings, 2 replies; 4+ messages in thread
From: Mika Westerberg @ 2021-09-21 10:29 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Malin Jonsson, john.jacques, Mika Westerberg, linux-watchdog
The watchdog core can handle pinging of the watchdog before userspace
opens the device. For this reason instead of stopping the timer, just
mark it as running and let the watchdog core take care of it.
Cc: Malin Jonsson <malin.jonsson@ericsson.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Changes from v1:
- Only set the WDOG_HW_RUNNING flag.
drivers/watchdog/iTCO_wdt.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 643c6c2d0b72..a0e8ad3901a4 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -430,6 +430,16 @@ static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *wd_dev)
return time_left;
}
+static void iTCO_wdt_set_running(struct iTCO_wdt_private *p)
+{
+ u16 val;
+
+ /* Bit 11: TCO Timer Halt -> 0 = The TCO timer is * enabled */
+ val = inw(TCO1_CNT(p));
+ if (!(val & BIT(11)))
+ set_bit(WDOG_HW_RUNNING, &p->wddev.status);
+}
+
/*
* Kernel Interfaces
*/
@@ -572,8 +582,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
watchdog_set_drvdata(&p->wddev, p);
platform_set_drvdata(pdev, p);
- /* Make sure the watchdog is not running */
- iTCO_wdt_stop(&p->wddev);
+ iTCO_wdt_set_running(p);
/* Check that the heartbeat value is within it's range;
if not reset to the default */
--
2.33.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] watchdog: iTCO_wdt: No need to stop the timer in probe
2021-09-21 10:29 [PATCH v2] watchdog: iTCO_wdt: No need to stop the timer in probe Mika Westerberg
@ 2021-09-21 12:31 ` Guenter Roeck
2023-07-05 11:22 ` Jan Kiszka
1 sibling, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2021-09-21 12:31 UTC (permalink / raw)
To: Mika Westerberg
Cc: Wim Van Sebroeck, Malin Jonsson, john.jacques, linux-watchdog
On Tue, Sep 21, 2021 at 01:29:00PM +0300, Mika Westerberg wrote:
> The watchdog core can handle pinging of the watchdog before userspace
> opens the device. For this reason instead of stopping the timer, just
> mark it as running and let the watchdog core take care of it.
>
> Cc: Malin Jonsson <malin.jonsson@ericsson.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Changes from v1:
>
> - Only set the WDOG_HW_RUNNING flag.
>
> drivers/watchdog/iTCO_wdt.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index 643c6c2d0b72..a0e8ad3901a4 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -430,6 +430,16 @@ static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *wd_dev)
> return time_left;
> }
>
> +static void iTCO_wdt_set_running(struct iTCO_wdt_private *p)
> +{
> + u16 val;
> +
> + /* Bit 11: TCO Timer Halt -> 0 = The TCO timer is * enabled */
> + val = inw(TCO1_CNT(p));
> + if (!(val & BIT(11)))
> + set_bit(WDOG_HW_RUNNING, &p->wddev.status);
> +}
> +
> /*
> * Kernel Interfaces
> */
> @@ -572,8 +582,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
> watchdog_set_drvdata(&p->wddev, p);
> platform_set_drvdata(pdev, p);
>
> - /* Make sure the watchdog is not running */
> - iTCO_wdt_stop(&p->wddev);
> + iTCO_wdt_set_running(p);
>
> /* Check that the heartbeat value is within it's range;
> if not reset to the default */
> --
> 2.33.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] watchdog: iTCO_wdt: No need to stop the timer in probe
2021-09-21 10:29 [PATCH v2] watchdog: iTCO_wdt: No need to stop the timer in probe Mika Westerberg
2021-09-21 12:31 ` Guenter Roeck
@ 2023-07-05 11:22 ` Jan Kiszka
2023-07-11 7:09 ` Jan Kiszka
1 sibling, 1 reply; 4+ messages in thread
From: Jan Kiszka @ 2023-07-05 11:22 UTC (permalink / raw)
To: Mika Westerberg, Wim Van Sebroeck, Guenter Roeck
Cc: Malin Jonsson, john.jacques, linux-watchdog
On 21.09.21 12:29, Mika Westerberg wrote:
> The watchdog core can handle pinging of the watchdog before userspace
> opens the device. For this reason instead of stopping the timer, just
> mark it as running and let the watchdog core take care of it.
>
> Cc: Malin Jonsson <malin.jonsson@ericsson.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> Changes from v1:
>
> - Only set the WDOG_HW_RUNNING flag.
>
> drivers/watchdog/iTCO_wdt.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index 643c6c2d0b72..a0e8ad3901a4 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -430,6 +430,16 @@ static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *wd_dev)
> return time_left;
> }
>
> +static void iTCO_wdt_set_running(struct iTCO_wdt_private *p)
> +{
> + u16 val;
> +
> + /* Bit 11: TCO Timer Halt -> 0 = The TCO timer is * enabled */
> + val = inw(TCO1_CNT(p));
> + if (!(val & BIT(11)))
> + set_bit(WDOG_HW_RUNNING, &p->wddev.status);
> +}
> +
> /*
> * Kernel Interfaces
> */
> @@ -572,8 +582,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
> watchdog_set_drvdata(&p->wddev, p);
> platform_set_drvdata(pdev, p);
>
> - /* Make sure the watchdog is not running */
> - iTCO_wdt_stop(&p->wddev);
> + iTCO_wdt_set_running(p);
>
> /* Check that the heartbeat value is within it's range;
> if not reset to the default */
This turned out to be not just a cleanup, it's a fix for
watchdog.handle_boot_enabled=0 as well. The issue is subtle, but this
fix is critical when trying to establish monitoring for unattended early
boot (OTA update scenarios). Can we still get this into stable trees,
along with [1] then? I've tested these two over 5.10 so far, can look
into older ones as well.
Jan
[1]
https://patchwork.kernel.org/project/linux-watchdog/patch/20221028062750.45451-1-mika.westerberg@linux.intel.com/
--
Siemens AG, Technology
Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] watchdog: iTCO_wdt: No need to stop the timer in probe
2023-07-05 11:22 ` Jan Kiszka
@ 2023-07-11 7:09 ` Jan Kiszka
0 siblings, 0 replies; 4+ messages in thread
From: Jan Kiszka @ 2023-07-11 7:09 UTC (permalink / raw)
To: Mika Westerberg, Wim Van Sebroeck, Guenter Roeck
Cc: Malin Jonsson, john.jacques, linux-watchdog
On 05.07.23 13:22, Jan Kiszka wrote:
> On 21.09.21 12:29, Mika Westerberg wrote:
>> The watchdog core can handle pinging of the watchdog before userspace
>> opens the device. For this reason instead of stopping the timer, just
>> mark it as running and let the watchdog core take care of it.
>>
>> Cc: Malin Jonsson <malin.jonsson@ericsson.com>
>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> ---
>> Changes from v1:
>>
>> - Only set the WDOG_HW_RUNNING flag.
>>
>> drivers/watchdog/iTCO_wdt.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
>> index 643c6c2d0b72..a0e8ad3901a4 100644
>> --- a/drivers/watchdog/iTCO_wdt.c
>> +++ b/drivers/watchdog/iTCO_wdt.c
>> @@ -430,6 +430,16 @@ static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *wd_dev)
>> return time_left;
>> }
>>
>> +static void iTCO_wdt_set_running(struct iTCO_wdt_private *p)
>> +{
>> + u16 val;
>> +
>> + /* Bit 11: TCO Timer Halt -> 0 = The TCO timer is * enabled */
>> + val = inw(TCO1_CNT(p));
>> + if (!(val & BIT(11)))
>> + set_bit(WDOG_HW_RUNNING, &p->wddev.status);
>> +}
>> +
>> /*
>> * Kernel Interfaces
>> */
>> @@ -572,8 +582,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
>> watchdog_set_drvdata(&p->wddev, p);
>> platform_set_drvdata(pdev, p);
>>
>> - /* Make sure the watchdog is not running */
>> - iTCO_wdt_stop(&p->wddev);
>> + iTCO_wdt_set_running(p);
>>
>> /* Check that the heartbeat value is within it's range;
>> if not reset to the default */
>
> This turned out to be not just a cleanup, it's a fix for
> watchdog.handle_boot_enabled=0 as well. The issue is subtle, but this
> fix is critical when trying to establish monitoring for unattended early
> boot (OTA update scenarios). Can we still get this into stable trees,
> along with [1] then? I've tested these two over 5.10 so far, can look
> into older ones as well.
>
> Jan
>
> [1]
> https://patchwork.kernel.org/project/linux-watchdog/patch/20221028062750.45451-1-mika.westerberg@linux.intel.com/
>
Any opinions about this?
Jan
--
Siemens AG, Technology
Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-07-11 7:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-21 10:29 [PATCH v2] watchdog: iTCO_wdt: No need to stop the timer in probe Mika Westerberg
2021-09-21 12:31 ` Guenter Roeck
2023-07-05 11:22 ` Jan Kiszka
2023-07-11 7:09 ` Jan Kiszka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).