RE: [PATCH V1 5/6] watchdog: da9062: DA9062 watchdog driver

[Opensource [Steve Twiss]]

On 07 May 2015 18:58 Guenter Roeck wrote:

Hi Guenter,

> > +static void da9062_apply_window_protection(struct da9062_watchdog *wdt)
> > +{
> > +       unsigned long delay = msecs_to_jiffies(DA9062_RESET_PROTECTION_MS);
> > +       unsigned long timeout = wdt->j_time_stamp + delay;
> > +       unsigned long now = jiffies;
> > +       unsigned int diff_ms;
> > +
> > +       /* if time-limit has not elapsed then wait for remainder */
> > +       if (time_before(now, timeout)) {
> > +               diff_ms = jiffies_to_msecs(timeout-now);
> > +               dev_dbg(wdt->hw->dev,
> > +                       "Delaying watchdog ping by %u msecs\n", diff_ms);
>
> I would not bother about the dev_dbg, but that is your call.
> 

.. easily removed ... I only have it in because I am worried about the case when the
watchdog gets kicked too often & quickly. If it is okay, I will leave that in because the
edge-case should be made known somewhere. Perhaps I will make it a better
debug description like "Watchdog kicked too quickly. Delaying %d ms"

> > +               mdelay(diff_ms);
>
> Can you use usleep_range() ?
> 

I put mdelay() ?? I meant to put msleep(). That's probably what I need to do now.

I did take a look at usleep_range() as I trawled for the best delay call, but usleep_range()
requires a lower and upper bound and I only have a lower bound in my case -- also, the
kernel docs "Documentation/timers/timers-howto.txt" suggest:

	SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms):
		* Use usleep_range
	SLEEPING FOR LARGER MSECS ( 10ms+ )
		* Use msleep [...]

I guess the majority of the time there would be zero delay-time and the code branch
wouldn't be triggered anyway, but when it is triggered the time can be anything up to
300 msecs. So I was intending to use msleep().

> Othewise looks good. BTW, I had to do something similar in
> drivers/hwmon/pmbus/zl6100.c; this is where the idea comes from.

Ah, I see, thanks.

Regards,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/