diff mbox series

[3/3] watchdog: honour hw_margin_ms DT property

Message ID 20200313160458.19727-4-rasmus.villemoes@prevas.dk
State Awaiting Upstream
Delegated to: Stefan Roese
Headers show
Series watchdog: honour hw_margin_ms property | expand

Commit Message

Rasmus Villemoes March 13, 2020, 4:04 p.m. UTC 
Some watchdog devices, e.g. external gpio-triggered ones, must be
reset more often than once per second, which means that the current
rate-limiting logic in watchdog_reset() fails to keep the board alive.

gpio-wdt.txt in the linux source tree defines a "hw_margin_ms"
property used to specifiy the maximum time allowed between resetting
the device. Allow any watchdog device to specify such a property, and
then use a reset period of one quarter of that. We keep the current
default of resetting once every 1000ms.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/watchdog/wdt-uclass.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Stefan Roese March 14, 2020, 11:57 a.m. UTC | #1 
On 13.03.20 17:04, Rasmus Villemoes wrote:
> Some watchdog devices, e.g. external gpio-triggered ones, must be
> reset more often than once per second, which means that the current
> rate-limiting logic in watchdog_reset() fails to keep the board alive.
> 
> gpio-wdt.txt in the linux source tree defines a "hw_margin_ms"
> property used to specifiy the maximum time allowed between resetting
> the device. Allow any watchdog device to specify such a property, and
> then use a reset period of one quarter of that. We keep the current
> default of resetting once every 1000ms.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>   drivers/watchdog/wdt-uclass.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index fb3e247c5f..436a52fd08 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -15,6 +15,12 @@ DECLARE_GLOBAL_DATA_PTR;
>   
>   #define WATCHDOG_TIMEOUT_SECS	(CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000)
>   
> +/*
> + * Reset every 1000ms, or however often is required as indicated by a
> + * hw_margin_ms property.
> + */
> +static ulong reset_period = 1000;
> +
>   int initr_watchdog(void)
>   {
>   	u32 timeout = WATCHDOG_TIMEOUT_SECS;
> @@ -36,6 +42,8 @@ int initr_watchdog(void)
>   	if (CONFIG_IS_ENABLED(OF_CONTROL)) {
>   		timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
>   					       WATCHDOG_TIMEOUT_SECS);
> +		reset_period = dev_read_u32_default(gd->watchdog_dev, "hw_margin_ms",
> +						    4*reset_period)/4;

Nitpicking: checkpatch will most likely complain about missing spaces
here.

>   	}
>   
>   	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> @@ -117,7 +125,7 @@ void watchdog_reset(void)
>   	/* Do not reset the watchdog too often */
>   	now = get_timer(0);
>   	if (time_after(now, next_reset)) {
> -		next_reset = now + 1000;	/* reset every 1000ms */
> +		next_reset = now + reset_period;
>   		wdt_reset(gd->watchdog_dev);
>   	}
>   }
> 

Other than that:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan
Rasmus Villemoes March 16, 2020, 3:39 p.m. UTC | #2 
On 14/03/2020 12.57, Stefan Roese wrote:
> On 13.03.20 17:04, Rasmus Villemoes wrote:
>>   int initr_watchdog(void)
>>   {
>>       u32 timeout = WATCHDOG_TIMEOUT_SECS;
>> @@ -36,6 +42,8 @@ int initr_watchdog(void)
>>       if (CONFIG_IS_ENABLED(OF_CONTROL)) {
>>           timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
>>                              WATCHDOG_TIMEOUT_SECS);
>> +        reset_period = dev_read_u32_default(gd->watchdog_dev,
>> "hw_margin_ms",
>> +                            4*reset_period)/4;
> 
> Nitpicking: checkpatch will most likely complain about missing spaces
> here.

Right. Want me to resend, or can you fix if/when applying?

Rasmus
Stefan Roese March 17, 2020, 7:25 a.m. UTC | #3 
On 16.03.20 16:39, Rasmus Villemoes wrote:
> On 14/03/2020 12.57, Stefan Roese wrote:
>> On 13.03.20 17:04, Rasmus Villemoes wrote:
>>>    int initr_watchdog(void)
>>>    {
>>>        u32 timeout = WATCHDOG_TIMEOUT_SECS;
>>> @@ -36,6 +42,8 @@ int initr_watchdog(void)
>>>        if (CONFIG_IS_ENABLED(OF_CONTROL)) {
>>>            timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
>>>                               WATCHDOG_TIMEOUT_SECS);
>>> +        reset_period = dev_read_u32_default(gd->watchdog_dev,
>>> "hw_margin_ms",
>>> +                            4*reset_period)/4;
>>
>> Nitpicking: checkpatch will most likely complain about missing spaces
>> here.
> 
> Right. Want me to resend, or can you fix if/when applying?

If there will be no other changes requiring a v2, then I can fix this
while applying.

Thanks,
Stefan
diff mbox series

Patch

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index fb3e247c5f..436a52fd08 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -15,6 +15,12 @@  DECLARE_GLOBAL_DATA_PTR;
 
 #define WATCHDOG_TIMEOUT_SECS	(CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000)
 
+/*
+ * Reset every 1000ms, or however often is required as indicated by a
+ * hw_margin_ms property.
+ */
+static ulong reset_period = 1000;
+
 int initr_watchdog(void)
 {
 	u32 timeout = WATCHDOG_TIMEOUT_SECS;
@@ -36,6 +42,8 @@  int initr_watchdog(void)
 	if (CONFIG_IS_ENABLED(OF_CONTROL)) {
 		timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
 					       WATCHDOG_TIMEOUT_SECS);
+		reset_period = dev_read_u32_default(gd->watchdog_dev, "hw_margin_ms",
+						    4*reset_period)/4;
 	}
 
 	wdt_start(gd->watchdog_dev, timeout * 1000, 0);
@@ -117,7 +125,7 @@  void watchdog_reset(void)
 	/* Do not reset the watchdog too often */
 	now = get_timer(0);
 	if (time_after(now, next_reset)) {
-		next_reset = now + 1000;	/* reset every 1000ms */
+		next_reset = now + reset_period;
 		wdt_reset(gd->watchdog_dev);
 	}
 }