From: Guenter Roeck <linux@roeck-us.net>
To: Ian Abbott <abbotti@mev.co.uk>, linux-watchdog@vger.kernel.org
Cc: Wim Van Sebroeck <wim@iguana.be>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/5] watchdog: dw_wdt: allow only one probe + various fixes
Date: Fri, 26 Jun 2015 06:54:24 -0700 [thread overview]
Message-ID: <558D5990.30102@roeck-us.net> (raw)
In-Reply-To: <1435324873-18318-1-git-send-email-abbotti@mev.co.uk>
On 06/26/2015 06:21 AM, Ian Abbott wrote:
> The dw_wdt watchdog driver supports a single DesignWare watchdog device
> using a statically allocated variable 'dw_wdt' to manage it, but there
> are some bits of hardware containing two such devices. If the "probe"
> is called for more than one device, the 'dw_wdt' variable gets
> clobbered, generally resulting in a kernel crash. Change it to reject
> additional probed devices.
>
> We probably don't expect the "remove" function to get called, but if it
> does, make sure the Linux timer used by device gets unqueued.
>
> The timer also gets unqueued if the watchdog device is closed by the
> user process unexpectedly. However, this is not currently done
> asynchronously and the timer function normally requeues the timer. If
> it manages to requeue the timer, the next call of the timer function
> will behave as though the close was expected and continue patting the
> dog every half a second when it should have stopped doing so. Unqueue
> the timer synchronously to avoid that.
>
> NOTE: There is another potential problem that I haven't bothered to fix.
> If the "remove" function is called while the watchdog device is still
> open, any in-progress or future file operations are likely to screw
> things up.
>
> 1) watchdog: dw_wdt: prepare for more atomic bits
> 2) watchdog: dw_wdt: reject additional device probes
> 3) watchdog: dw_wdt: only unregister restart handler if registered
> 4) watchdog: dw_wdt: unqueue timer on device removal
> 5) watchdog: dw_wdt: unqueue timer synchronously on unexpected close
>
> drivers/watchdog/dw_wdt.c | 46 +++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 35 insertions(+), 11 deletions(-)
Hi Ian,
any chance you can convert the driver to use the watchdog APIs instead ?
This way it could support more than one watchdog, and the code would be
much simpler.
Thanks,
Guenter
prev parent reply other threads:[~2015-06-26 13:54 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-26 13:21 [PATCH 0/5] watchdog: dw_wdt: allow only one probe + various fixes Ian Abbott
2015-06-26 13:21 ` [PATCH 1/5] watchdog: dw_wdt: prepare for more atomic bits Ian Abbott
2015-06-26 13:21 ` [PATCH 2/5] watchdog: dw_wdt: reject additional device probes Ian Abbott
2015-06-26 13:21 ` [PATCH 3/5] watchdog: dw_wdt: only unregister restart handler if registered Ian Abbott
2015-06-26 13:21 ` [PATCH 4/5] watchdog: dw_wdt: unqueue timer on device removal Ian Abbott
2015-06-26 13:21 ` [PATCH 5/5] watchdog: dw_wdt: unqueue timer synchronously on unexpected close Ian Abbott
2015-06-26 13:54 ` Guenter Roeck [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=558D5990.30102@roeck-us.net \
--to=linux@roeck-us.net \
--cc=abbotti@mev.co.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=wim@iguana.be \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.