Sorry, with so many drivers to patch, I went with a recommended
list of "interested parties". I'll add you to that list. It is posted
to linux-kernel in the hopes that interested persons do get a chance to
look at it. I got no responses outside of my "interested parties" when
I posted this patch originally. It's good to hear from you.
> > +
> > + case WDIOC_SETTIMEOUT:
> > + if (get_user(new_margin, (int *)arg))
> > + return -EFAULT;
> > + if ((new_margin < 1) || (new_margin > 255))
> > + return -EINVAL;
> > + wd_margin = new_margin;
> > + wafwdt_stop();
> > + wafwdt_start();
> > + /* Fall */
> > + case WDIOC_GETTIMEOUT:
> > + return put_user(wd_margin, (int *)arg);
> I really wouldnt do wafwdt_stop(); wafwdt_start(); here. The new timeout
> will be set on the next watchdog ping anyway, and you need to spin_lock
> and unlock round this too. Much cleaner just to drop it.
Um, am I missreading:
59 static void wafwdt_ping(void)
61 /* pat watchdog */
I don't see it writing the new timeout. Also, the semantic of
WDIOC_SETTIMEOUT (at least as I've implemented it in all the drivers
I've touched) is to ping the device to verify the new timeout is active.
I also note that the calls to wafwdt_stop()/start() in the
open()/close() functions doesn't take the spinlock. Granted, open() and
close() should be protected by wafwdt_is_open.
If I add the locking, are you comfortable with the changes?
"To fall in love is to create a religion that has a fallible god." -Jorge Luis Borges
Joel Becker Senior Member of Technical Staff Oracle Corporation E-mail: firstname.lastname@example.org Phone: (650) 506-8127 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to email@example.com More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/