| Message ID | 20210512084724.14634-8-shruthi.sanil@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Intel Keem Bay WDT bug fixes | expand |
On Wed, May 12, 2021 at 02:17:21PM +0530, shruthi.sanil@intel.com wrote: > From: Shruthi Sanil <shruthi.sanil@intel.com> > > Corrected the typo in the function keembay_wdt_resume, we need to > enable the WDT if it is disabled/not active. > > Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc") > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Tested-by: Kris Pan <kris.pan@intel.com> > Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com> > --- > drivers/watchdog/keembay_wdt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c > index dd192b8dff55..10896415f8c7 100644 > --- a/drivers/watchdog/keembay_wdt.c > +++ b/drivers/watchdog/keembay_wdt.c > @@ -260,7 +260,7 @@ static int __maybe_unused keembay_wdt_resume(struct device *dev) > { > struct keembay_wdt *wdt = dev_get_drvdata(dev); > > - if (watchdog_active(&wdt->wdd)) > + if (!watchdog_active(&wdt->wdd)) Have you tested this ? "watchdog_active" refers to the watchdog core state. Your code now keeps the watchdog stopped after resume if it was running before, and starts it if it wasn't. Please run through a suspend/resume cycle with watchdog disabled and see what happens. Guenter > return keembay_wdt_start(&wdt->wdd); > > return 0; > -- > 2.17.1 >
> -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Wednesday, May 12, 2021 7:32 PM > To: Sanil, Shruthi <shruthi.sanil@intel.com> > Cc: wim@linux-watchdog.org; linux-watchdog@vger.kernel.org; linux- > kernel@vger.kernel.org; andriy.shevchenko@linux.intel.com; > kris.pan@linux.intel.com; mgross@linux.intel.com; Thokala, Srikanth > <srikanth.thokala@intel.com>; Raja Subramanian, Lakshmi Bai > <lakshmi.bai.raja.subramanian@intel.com>; Sangannavar, Mallikarjunappa > <mallikarjunappa.sangannavar@intel.com> > Subject: Re: [PATCH 07/10] watchdog: keembay: Update the check in > keembay_wdt_resume() > > On Wed, May 12, 2021 at 02:17:21PM +0530, shruthi.sanil@intel.com wrote: > > From: Shruthi Sanil <shruthi.sanil@intel.com> > > > > Corrected the typo in the function keembay_wdt_resume, we need to > > enable the WDT if it is disabled/not active. > > > > Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay > > Soc") > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Tested-by: Kris Pan <kris.pan@intel.com> > > Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com> > > --- > > drivers/watchdog/keembay_wdt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/watchdog/keembay_wdt.c > > b/drivers/watchdog/keembay_wdt.c index dd192b8dff55..10896415f8c7 > > 100644 > > --- a/drivers/watchdog/keembay_wdt.c > > +++ b/drivers/watchdog/keembay_wdt.c > > @@ -260,7 +260,7 @@ static int __maybe_unused > > keembay_wdt_resume(struct device *dev) { > > struct keembay_wdt *wdt = dev_get_drvdata(dev); > > > > - if (watchdog_active(&wdt->wdd)) > > + if (!watchdog_active(&wdt->wdd)) > > Have you tested this ? "watchdog_active" refers to the watchdog core state. > Your code now keeps the watchdog stopped after resume if it was running > before, and starts it if it wasn't. Please run through a suspend/resume cycle > with watchdog disabled and see what happens. > > Guenter I had understood it wrongly. I was assuming that watchdog_active refers to the actual state of the HW. Hence I made that change. But since watchdog_active refers to the state of the core, the change that I have done is incorrect. Hence I'll drop this patch. Thanks Shruthi > > > return keembay_wdt_start(&wdt->wdd); > > > > return 0; > > -- > > 2.17.1 > >
diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c index dd192b8dff55..10896415f8c7 100644 --- a/drivers/watchdog/keembay_wdt.c +++ b/drivers/watchdog/keembay_wdt.c @@ -260,7 +260,7 @@ static int __maybe_unused keembay_wdt_resume(struct device *dev) { struct keembay_wdt *wdt = dev_get_drvdata(dev); - if (watchdog_active(&wdt->wdd)) + if (!watchdog_active(&wdt->wdd)) return keembay_wdt_start(&wdt->wdd); return 0;