Quoting Sai Prakash Ranjan (2019-01-17 07:19:42) > diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c > index 780971318810..5dfd604477a4 100644 > --- a/drivers/watchdog/qcom-wdt.c > +++ b/drivers/watchdog/qcom-wdt.c > @@ -245,6 +245,28 @@ static int qcom_wdt_remove(struct platform_device *pdev) > return 0; > } > > +static int __maybe_unused qcom_wdt_suspend(struct device *dev) > +{ > + struct qcom_wdt *wdt = dev_get_drvdata(dev); > + > + if (watchdog_active(&wdt->wdd)) > + qcom_wdt_stop(&wdt->wdd); > + > + return 0; > +} > + > +static int __maybe_unused qcom_wdt_resume(struct device *dev) > +{ > + struct qcom_wdt *wdt = dev_get_drvdata(dev); > + > + if (watchdog_active(&wdt->wdd)) > + qcom_wdt_start(&wdt->wdd); > + > + return 0; > +} This looks fairly generic. For example, the Mediatek driver also stops and starts (but also pings after starting). Grepping for 'active' in drivers/watchdog/ finds more examples. Could there be some functions in watchdog core that do the common things like watchdog_stop() and watchdog_start() and watchdog_start_and_ping()? Or maybe a bit can be set during registration so that the 'struct class watchdog_class' can get PM ops to stop and start on suspend/resume of the watchdog character device? Nothing is wrong with the patch, I'm just bemoaning the amount of code duplication here.