diff mbox series

watchdog: bcm2835_wdt: Fix WDIOC_SETTIMEOUT handling

Message ID 20231112173251.4827-1-wahrenst@gmx.net (mailing list archive)
State New, archived
Headers show
Series watchdog: bcm2835_wdt: Fix WDIOC_SETTIMEOUT handling | expand

Commit Message

Stefan Wahren Nov. 12, 2023, 5:32 p.m. UTC
Users report about the unexpected behavior for setting timeouts above
15 sec on Raspberry Pi. According to watchdog-api.rst the ioctl
WDIOC_SETTIMEOUT shouldn't fail because of hardware limitations.
But looking at the code shows that max_timeout based on the
register value PM_WDOG_TIME_SET, which is the maximum.

Since 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat
in watchdog core") the watchdog core is able to handle this problem.

This fix has been tested with watchdog-test from selftests.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217374
Fixes: 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat in watchdog core")
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/watchdog/bcm2835_wdt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--
2.34.1

Comments

Guenter Roeck Nov. 13, 2023, 2:31 p.m. UTC | #1
On Sun, Nov 12, 2023 at 06:32:51PM +0100, Stefan Wahren wrote:
> Users report about the unexpected behavior for setting timeouts above
> 15 sec on Raspberry Pi. According to watchdog-api.rst the ioctl
> WDIOC_SETTIMEOUT shouldn't fail because of hardware limitations.
> But looking at the code shows that max_timeout based on the
> register value PM_WDOG_TIME_SET, which is the maximum.
> 
> Since 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat
> in watchdog core") the watchdog core is able to handle this problem.
> 
> This fix has been tested with watchdog-test from selftests.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217374
> Fixes: 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat in watchdog core")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/bcm2835_wdt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
> index 7a855289ff5e..bb001c5d7f17 100644
> --- a/drivers/watchdog/bcm2835_wdt.c
> +++ b/drivers/watchdog/bcm2835_wdt.c
> @@ -42,6 +42,7 @@
> 
>  #define SECS_TO_WDOG_TICKS(x) ((x) << 16)
>  #define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
> +#define WDOG_TICKS_TO_MSECS(x) ((x) * 1000 >> 16)
> 
>  struct bcm2835_wdt {
>  	void __iomem		*base;
> @@ -140,7 +141,7 @@ static struct watchdog_device bcm2835_wdt_wdd = {
>  	.info =		&bcm2835_wdt_info,
>  	.ops =		&bcm2835_wdt_ops,
>  	.min_timeout =	1,
> -	.max_timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
> +	.max_hw_heartbeat_ms =	WDOG_TICKS_TO_MSECS(PM_WDOG_TIME_SET),
>  	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
>  };
> 
> --
> 2.34.1
>
Florian Fainelli Nov. 13, 2023, 4:33 p.m. UTC | #2
On 11/12/23 09:32, Stefan Wahren wrote:
> Users report about the unexpected behavior for setting timeouts above
> 15 sec on Raspberry Pi. According to watchdog-api.rst the ioctl
> WDIOC_SETTIMEOUT shouldn't fail because of hardware limitations.
> But looking at the code shows that max_timeout based on the
> register value PM_WDOG_TIME_SET, which is the maximum.
> 
> Since 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat
> in watchdog core") the watchdog core is able to handle this problem.
> 
> This fix has been tested with watchdog-test from selftests.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217374
> Fixes: 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat in watchdog core")
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Stefan Wahren Nov. 30, 2023, 11:57 a.m. UTC | #3
Hi,

Am 13.11.23 um 15:31 schrieb Guenter Roeck:
> On Sun, Nov 12, 2023 at 06:32:51PM +0100, Stefan Wahren wrote:
>> Users report about the unexpected behavior for setting timeouts above
>> 15 sec on Raspberry Pi. According to watchdog-api.rst the ioctl
>> WDIOC_SETTIMEOUT shouldn't fail because of hardware limitations.
>> But looking at the code shows that max_timeout based on the
>> register value PM_WDOG_TIME_SET, which is the maximum.
>>
>> Since 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat
>> in watchdog core") the watchdog core is able to handle this problem.
>>
>> This fix has been tested with watchdog-test from selftests.
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217374
>> Fixes: 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat in watchdog core")
>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
who takes care of this patch?
Wim Van Sebroeck Dec. 17, 2023, 3:34 p.m. UTC | #4
Hi Stefan,

> Hi,
> 
> Am 13.11.23 um 15:31 schrieb Guenter Roeck:
> >On Sun, Nov 12, 2023 at 06:32:51PM +0100, Stefan Wahren wrote:
> >>Users report about the unexpected behavior for setting timeouts above
> >>15 sec on Raspberry Pi. According to watchdog-api.rst the ioctl
> >>WDIOC_SETTIMEOUT shouldn't fail because of hardware limitations.
> >>But looking at the code shows that max_timeout based on the
> >>register value PM_WDOG_TIME_SET, which is the maximum.
> >>
> >>Since 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat
> >>in watchdog core") the watchdog core is able to handle this problem.
> >>
> >>This fix has been tested with watchdog-test from selftests.
> >>
> >>Link: https://bugzilla.kernel.org/show_bug.cgi?id=217374
> >>Fixes: 664a39236e71 ("watchdog: Introduce hardware maximum heartbeat in watchdog core")
> >>Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> >Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> who takes care of this patch?

I did.

Kind regards,
Wim.
diff mbox series

Patch

diff --git a/drivers/watchdog/bcm2835_wdt.c b/drivers/watchdog/bcm2835_wdt.c
index 7a855289ff5e..bb001c5d7f17 100644
--- a/drivers/watchdog/bcm2835_wdt.c
+++ b/drivers/watchdog/bcm2835_wdt.c
@@ -42,6 +42,7 @@ 

 #define SECS_TO_WDOG_TICKS(x) ((x) << 16)
 #define WDOG_TICKS_TO_SECS(x) ((x) >> 16)
+#define WDOG_TICKS_TO_MSECS(x) ((x) * 1000 >> 16)

 struct bcm2835_wdt {
 	void __iomem		*base;
@@ -140,7 +141,7 @@  static struct watchdog_device bcm2835_wdt_wdd = {
 	.info =		&bcm2835_wdt_info,
 	.ops =		&bcm2835_wdt_ops,
 	.min_timeout =	1,
-	.max_timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
+	.max_hw_heartbeat_ms =	WDOG_TICKS_TO_MSECS(PM_WDOG_TIME_SET),
 	.timeout =	WDOG_TICKS_TO_SECS(PM_WDOG_TIME_SET),
 };