On Fri, Aug 26, 2022 at 04:56:38PM +0300, Sergei Antonov wrote:
> Add a missing _restart function to watchdog_ops.
The restart function isn't "missing" because it is optional.
> FTWDT010 resets system when it is started with timeout = 0.
>
> Signed-off-by: Sergei Antonov <saproj@xxxxxxxxx>
> ---
> drivers/watchdog/ftwdt010_wdt.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/watchdog/ftwdt010_wdt.c b/drivers/watchdog/ftwdt010_wdt.c
> index 21dcc7765688..9eaaaca1094d 100644
> --- a/drivers/watchdog/ftwdt010_wdt.c
> +++ b/drivers/watchdog/ftwdt010_wdt.c
> @@ -93,6 +93,13 @@ static int ftwdt010_wdt_set_timeout(struct watchdog_device *wdd,
> return 0;
> }
>
> +static int ftwdt010_wdt_restart(struct watchdog_device *wdd,
> + unsigned long action, void *data)
> +{
> + wdd->timeout = 0;
> + return ftwdt010_wdt_start(wdd);
While technically correct, this crates the impression that
ftwdt010_wdt_start() might return an error - which it never does.
Given that, I would prefer
ftwdt010_wdt_start(wdd);
return 0;
Also, did you make sure that interrupt support does not interfer with
restart ? We don't want to get an interrupt when the watchdog fires due
to a call to the restart handler.
Thanks,
Guenter
> +}
> +
> static irqreturn_t ftwdt010_wdt_interrupt(int irq, void *data)
> {
> struct ftwdt010_wdt *gwdt = data;
> @@ -107,6 +114,7 @@ static const struct watchdog_ops ftwdt010_wdt_ops = {
> .stop = ftwdt010_wdt_stop,
> .ping = ftwdt010_wdt_ping,
> .set_timeout = ftwdt010_wdt_set_timeout,
> + .restart = ftwdt010_wdt_restart,
> .owner = THIS_MODULE,
> };
>
> --
> 2.34.1
>
