On Mon, May 04, 2015 at 05:19:26PM +0200, Arnd Bergmann wrote:
On Monday 04 May 2015 20:04:47 fu.wei@linaro.org wrote:
drivers/watchdog/Kconfig | 10 + drivers/watchdog/Makefile | 1 + drivers/watchdog/sbsa_gwdt.c | 578 +++++++++++++++++++++++++++++++++++++++++++ drivers/watchdog/sbsa_gwdt.h | 99 ++++++++
To clarify my earlier comment: I actually meant to suggest moving the contents of sbsa_gwdt.h into sbsa_gwdt.c itself, and having only one file.
+static long sbsa_gwdt_ioctl(struct watchdog_device *wdd, unsigned int cmd,
unsigned long arg)+{
struct sbsa_gwdt *gwdt = watchdog_get_drvdata(wdd);void __user *argp = (void __user *)arg;int ret = -ENOIOCTLCMD;unsigned int __user *p = argp;unsigned int new_value;/* FIXME: what else do we need here?? */switch (cmd) {case WDIOC_SETPRETIMEOUT:if (get_user(new_value, p))return -EFAULT;ret = sbsa_gwdt_set_pretimeout(wdd, new_value);break;case WDIOC_GETPRETIMEOUT:ret = put_user(gwdt->pretimeout, (unsigned int __user*)arg);
break;}return ret;+}
Here, I'd like to see the two ioctl commands get handled by the ioctl handler in drivers/watchdog/watchdog_dev.c in the same way that WDIOC_GETTIMEOUT/WDIOC_SETTIMEOUT are. Just add one more patch to the series to introduce a generic watchdog_set_pretimeout() function and a 'pretimeout' field in 'struct watchdog_device'.
Makes sense.
Guenter