| Message ID | 20200228142331.13716-3-t-kristo@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | watchdog: add TI K3 SoC watchdog support | expand |
On Fri, Feb 28, 2020 at 04:23:29PM +0200, Tero Kristo wrote: > Current watchdog core pets the timer always after the initial keepalive > time has expired from boot-up. This is incorrect for certain timers that > don't like to be petted immediately when they are started, if they have > not been running over the boot. > > To allow drivers to reset their keepalive timers during startup, add > a new watchdog flag to the api, WDOG_RESET_KEEPALIVE. > > Signed-off-by: Tero Kristo <t-kristo@ti.com> > --- > drivers/watchdog/watchdog_dev.c | 2 ++ > include/linux/watchdog.h | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c > index 8b5c742f24e8..131e40c21703 100644 > --- a/drivers/watchdog/watchdog_dev.c > +++ b/drivers/watchdog/watchdog_dev.c > @@ -283,6 +283,8 @@ static int watchdog_start(struct watchdog_device *wdd) > set_bit(WDOG_ACTIVE, &wdd->status); > wd_data->last_keepalive = started_at; > watchdog_update_worker(wdd); > + if (test_bit(WDOG_RESET_KEEPALIVE, &wdd->status)) > + wd_data->last_hw_keepalive = started_at; I don't think the additional flag is needed. The code should just set last_hw_keepalive. After all, it already sets last_keepalive, which determines when the next internal keepalive will be sent. It makes sense to also set last_hw_keepalive to prevent the next keepalive from being sent too early. Guenter
On 28/02/2020 19:13, Guenter Roeck wrote: > On Fri, Feb 28, 2020 at 04:23:29PM +0200, Tero Kristo wrote: >> Current watchdog core pets the timer always after the initial keepalive >> time has expired from boot-up. This is incorrect for certain timers that >> don't like to be petted immediately when they are started, if they have >> not been running over the boot. >> >> To allow drivers to reset their keepalive timers during startup, add >> a new watchdog flag to the api, WDOG_RESET_KEEPALIVE. >> >> Signed-off-by: Tero Kristo <t-kristo@ti.com> >> --- >> drivers/watchdog/watchdog_dev.c | 2 ++ >> include/linux/watchdog.h | 1 + >> 2 files changed, 3 insertions(+) >> >> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c >> index 8b5c742f24e8..131e40c21703 100644 >> --- a/drivers/watchdog/watchdog_dev.c >> +++ b/drivers/watchdog/watchdog_dev.c >> @@ -283,6 +283,8 @@ static int watchdog_start(struct watchdog_device *wdd) >> set_bit(WDOG_ACTIVE, &wdd->status); >> wd_data->last_keepalive = started_at; >> watchdog_update_worker(wdd); >> + if (test_bit(WDOG_RESET_KEEPALIVE, &wdd->status)) >> + wd_data->last_hw_keepalive = started_at; > > I don't think the additional flag is needed. The code should just set > last_hw_keepalive. After all, it already sets last_keepalive, which > determines when the next internal keepalive will be sent. It makes sense > to also set last_hw_keepalive to prevent the next keepalive from being > sent too early. Ok, I can modify this patch to tweak the last_hw_keepalive unconditionally if you think that is safe to do. I did it like this as there might be some cases where the existing implementations actually expect the ping to happen immediately for some reason (but I guess in those cases the corresponding watchdog drivers would need to be modified.) -Tero -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index 8b5c742f24e8..131e40c21703 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c @@ -283,6 +283,8 @@ static int watchdog_start(struct watchdog_device *wdd) set_bit(WDOG_ACTIVE, &wdd->status); wd_data->last_keepalive = started_at; watchdog_update_worker(wdd); + if (test_bit(WDOG_RESET_KEEPALIVE, &wdd->status)) + wd_data->last_hw_keepalive = started_at; } return err; diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index 417d9f37077a..b56e3f1b1ec3 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -116,6 +116,7 @@ struct watchdog_device { #define WDOG_STOP_ON_REBOOT 2 /* Should be stopped on reboot */ #define WDOG_HW_RUNNING 3 /* True if HW watchdog running */ #define WDOG_STOP_ON_UNREGISTER 4 /* Should be stopped on unregister */ +#define WDOG_RESET_KEEPALIVE 5 /* Reset keepalive timers at start */ struct list_head deferred; };
Current watchdog core pets the timer always after the initial keepalive time has expired from boot-up. This is incorrect for certain timers that don't like to be petted immediately when they are started, if they have not been running over the boot. To allow drivers to reset their keepalive timers during startup, add a new watchdog flag to the api, WDOG_RESET_KEEPALIVE. Signed-off-by: Tero Kristo <t-kristo@ti.com> --- drivers/watchdog/watchdog_dev.c | 2 ++ include/linux/watchdog.h | 1 + 2 files changed, 3 insertions(+)