lkml.org 
[lkml]   [2021]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [watchdog] watchdog: mei_wdt: request stop on unregister
Tomas,

On Thu, Jan 07, 2021 at 04:12:15PM -0800, Guenter Roeck wrote:
> Hi,
>
> On Thu, Jan 07, 2021 at 09:57:30PM +0200, Tomas Winkler wrote:
> > From: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > Send the stop command to the firmware on watchdog unregister
> > to eleminate false event on suspend.
> >
>
> Normally the watchdog driver would not be expected to unregister
> during suspend, only when the driver is manually unloaded.
> To support suspend/resume, other watchdog drivers implement
> suspend/resume functions to stop the watchdog on suspend and
> to restart it on resume. Unloading a watchdog driver on suspend
> would also have odd implications for userspace watchdog daemons.
>
> On top of that, it should not actually be possible to unregister
> a watchdog while it is in use (because it is open in that case
> and should be marked as busy). watchdog_stop_on_unregister()
> only serves as backup in case someone actually manages to unload
> the driver while the watchdog is running. The function was
> implemented to avoid calls to stop the watchdog in the remove
> function because I can not mathematically prove that there are
> no situations where the watchdog is unloaded while running.
> However, I have not actually been able to do that.
>
> Are you sure this patch is doing what you expect it to do ?
>

I have not heard anything back. I tried to understand how this
patch would resolve a problem during suspend/resume, but
I didn't find anything.

Can you maybe add a log message showing the false event that is
prevented with this patch, and some context explaining how the
patch fixes the problem ?

Thanks,
Guenter

> Thanks,
> Guenter
>
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > ---
> > drivers/watchdog/mei_wdt.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> > index 5391bf3e6b11..c5967d8b4256 100644
> > --- a/drivers/watchdog/mei_wdt.c
> > +++ b/drivers/watchdog/mei_wdt.c
> > @@ -382,6 +382,7 @@ static int mei_wdt_register(struct mei_wdt *wdt)
> >
> > watchdog_set_drvdata(&wdt->wdd, wdt);
> > watchdog_stop_on_reboot(&wdt->wdd);
> > + watchdog_stop_on_unregister(&wdt->wdd);
> >
> > ret = watchdog_register_device(&wdt->wdd);
> > if (ret)
> > --
> > 2.26.2
> >

\
 
 \ /
  Last update: 2021-01-23 19:50    [W:0.053 / U:4.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site