All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Markus Mayer <markus.mayer@linaro.org>, Wim Van Sebroeck <wim@iguana.be>
Cc: Linux Watchdog List <linux-watchdog@vger.kernel.org>
Subject: Re: [RFC 0/2] Proposal for Watchdog Timer Resolution
Date: Sun, 01 Dec 2013 10:54:05 -0800	[thread overview]
Message-ID: <529B85CD.9040505@roeck-us.net> (raw)
In-Reply-To: <1385760030-24264-1-git-send-email-markus.mayer@linaro.org>

On 11/29/2013 01:20 PM, Markus Mayer wrote:
> I would like to propose the following extension to the watchdog
> framework to allow userland programs access to the resolution of the
> watchdog timer should the hardware support such a feature. The first
> driver to make use of this feature would be the BCM Kona watchdog
> driver.
>
> I would also like to propose introducing a sysfs interface to the
> watchdog framework in a later phase of updating the watchdog framework.
> The sysfs interface would exist in addition to the existing ioctl
> interface. In the future, the ioctl interface could be deprecated in
> favour of the sysfs interface or both interfaces could be kept
> indefinitely.
>
> Phase 1:
>
> Add get resolution/set resolution ioctls.

Hi Markus,

I am not entirely sure I understand the rationale for this one, or if it is really
valuable for the user.

One example where a resolution might be applicable is the w83627hf_wdt.c driver.
The Winbond/Nuvoton chips support watchdog timeouts either in seconds or in
minutes, based on some configuration register. Currently the driver supports
only seconds (1..255). I had thought about extending it to minutes, but my
approach would have been to automatically select minutes-based timeouts if the
requested timeout is too large (> 255 seconds).

The same approach should be possible for the bcm_kona driver as well; just select
the optimal resolution based on the requested timeout. I think this would make more
sense than manual configuration from a user perspective, to keep the API as simple
as possible while supporting a wide range of timeouts.

On a side note, I don't think you specify anywhere what "resolution" actually means;
it is just a number which, as far as I can see, does not seem to resemble a specific
time. In the case of the bcm_kona driver, it appears to be that values of 0..15 map
to 30us .. 1s in reverse order, which translates to a maximum timeout of anywhere
between 32s and about 12 days.

Based on this, you could set the maximum permitted timeout for the driver to 12 days
or 748800 seconds (or , and then calculate the required resolution from the actually
selected timeout.

Example:
	requested timeout: 24 x 3600 = 86400 seconds, ie 1 day
	Required resolution: 86400 s / 0xfffff = 82.4 ms ==> resolution = 3.

On the other side, I am not sure if all this complexity is really needed.
Even at maximum resolution of 1s you can always provide the remaining timeout
exactly down to the second to the user, which is the granularity provided
by the watchdog subsystem. So, if you want to be able to support 12-day
timeouts, why don't you just set the resolution to the maximum supported ?

> Add necessary flags and call-back functions.
> Extend bcm_kona_wdt.c to make use of the new functionality.
>
> Phase 2:
>
> Abstract the ioctl interface (into watchdog_ioctl.c) with a new set of
> per-driver ops, similar to what other frameworks are already doing.
>
> Phase 3:
>
> Introduce a sysfs interface (watchdog_sysfs.c) hooking into the already
> established per-driver ops. ioctl and sysfs calls would use the same
> framework and driver functions, requiring no code duplication.
>
> Please let me know if this sounds sensible or whether an alternate
> approach would be preferred.
>
I would like to see a sysfs interface, so I would like to see phase 3.
I'd have to see a patch set for phase 2; not really sure what this entails.

Thanks,
Guenter


      parent reply	other threads:[~2013-12-01 18:54 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-29 21:20 [RFC 0/2] Proposal for Watchdog Timer Resolution Markus Mayer
2013-11-29 21:20 ` [RFC 1/2] watchdog: Add support to set timer resolution Markus Mayer
2013-11-29 21:20 ` [RFC 2/2] watchdog: bcm281xx: Support for timer resolution ioctls Markus Mayer
2013-12-01 18: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=529B85CD.9040505@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=markus.mayer@linaro.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.