Re: [PATCH 9/10 v2] Generic Watchdog Timer Driver

From: Wim Van Sebroeck
Date: Wed Jun 22 2011 - 16:05:22 EST

Hi Arnd,

> > watchdog: WatchDog Timer Driver Core - Part 9
> >
> > Add support for extra ioctl calls by adding a
> > ioctl watchdog operation. This operation will be
> > called before we do our own handling of ioctl
> > commands. This way we can override the internal
> > ioctl command handling and we can also add
> > extra ioctl commands. The ioctl watchdog operation
> > should return the appropriate error codes or
> > -ENOIOCTLCMD if the ioctl command should be handled
> > through the internal ioctl handling of the framework.
> >
> > Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Wim Van Sebroeck <wim@xxxxxxxxx>
> Hmm, I'm not sure about this one. It does make the conversion
> of existing drivers easier but doesn't encourage doing a good
> job there.

That's why we also have a maintainer and a mailing list to review
patches... I can understand your concerns, but prefer the flexibility
and simpleness here.

> How about instead providing a compatibility helper module that
> provides helper functions like this:
> int watchdog_compat_getstatus(struct watchdog_device *wdd)
> {
> int __user *tmp = compat_alloc_userspace(sizeof (int));
> int ret;
> ret = wdd->ops->ioctl(wdd, WDIOC_GETSTATUS, tmp);
> if (ret)
> return ret;
> __get_user(ret, tmp);
> return ret;
> }
> EXPORT_SYMBOL_GPL(watchdog_compat_getstatus);
> This would let you use the common ioctl implementation for all
> watchdogs without the option of an individual driver overriding
> it, but a driver could still provide an ioctl method that only
> gets called by the watchdog_compat_* functions.

Hmmm, not really in favour of that, this will creat a lot more overhead.

Kind regards,

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at