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'.

        Arnd