| Message ID | 20200611191750.28096-8-a.fatoum@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | watchdog: f71808e_wdt: migrate to kernel | expand |
On Thu, Jun 11, 2020 at 09:17:48PM +0200, Ahmad Fatoum wrote: > Migrating the driver lets us drop the watchdog misc device boilerplate > and reduce size by 270~ lines. It also brings us support for new > functionality like CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED. > > This changes behavior on module unloading: Whereas before, the watchdog > was automatically stopped on module unload, now explicit stopping of > the watchdog before unload is required. > Any chance you can convert the driver to a platform device ? Thanks, Guenter > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > drivers/watchdog/Kconfig | 1 + > drivers/watchdog/f71808e_wdt.c | 497 ++++++++------------------------- > 2 files changed, 111 insertions(+), 387 deletions(-) > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 0663c604bd64..c312dd5fc0ca 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -1052,6 +1052,7 @@ config EBC_C384_WDT > config F71808E_WDT > tristate "Fintek F718xx, F818xx Super I/O Watchdog" > depends on X86 > + select WATCHDOG_CORE > help > This is the driver for the hardware watchdog on the Fintek F71808E, > F71862FG, F71868, F71869, F71882FG, F71889FG, F81803, F81865, and > diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c > index 9299ad4d4a52..7c42cbf9912e 100644 > --- a/drivers/watchdog/f71808e_wdt.c > +++ b/drivers/watchdog/f71808e_wdt.c > @@ -9,16 +9,10 @@ > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > #include <linux/err.h> > -#include <linux/fs.h> > #include <linux/init.h> > #include <linux/io.h> > #include <linux/ioport.h> > -#include <linux/miscdevice.h> > #include <linux/module.h> > -#include <linux/mutex.h> > -#include <linux/notifier.h> > -#include <linux/reboot.h> > -#include <linux/uaccess.h> > #include <linux/watchdog.h> > > #define DRVNAME "f71808e_wdt" > @@ -129,23 +123,15 @@ struct f71808e_variant { > }; > > struct watchdog_data { > + struct watchdog_device wdd; > unsigned short sioaddr; > const struct f71808e_variant *variant; > - unsigned long opened; > - struct mutex lock; > - char expect_close; > struct watchdog_info ident; > > - unsigned short timeout; > u8 timer_val; /* content for the wd_time register */ > char minutes_mode; > u8 pulse_val; /* pulse width flag */ > char pulse_mode; /* enable pulse output mode? */ > - char caused_reboot; /* last reboot was by the watchdog */ > -}; > - > -static struct watchdog_data watchdog = { > - .lock = __MUTEX_INITIALIZER(watchdog.lock), > }; > > static inline bool has_f81865_wdo_conf(struct watchdog_data *wd) > @@ -216,487 +202,225 @@ static inline void superio_exit(int base) > release_region(base, 2); > } > > -static int watchdog_set_timeout(int timeout) > +static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout) > { > - if (timeout <= 0 > - || timeout > max_timeout) { > - pr_err("watchdog timeout out of range\n"); > - return -EINVAL; > - } > + struct watchdog_data *wd = watchdog_get_drvdata(wdd); > > - mutex_lock(&watchdog.lock); > - > - watchdog.timeout = timeout; > - if (timeout > 0xff) { > - watchdog.timer_val = DIV_ROUND_UP(timeout, 60); > - watchdog.minutes_mode = true; > + wdd->timeout = new_timeout; > + if (new_timeout > 0xff) { > + wd->timer_val = DIV_ROUND_UP(new_timeout, 60); > + wd->minutes_mode = true; > } else { > - watchdog.timer_val = timeout; > - watchdog.minutes_mode = false; > + wd->timer_val = new_timeout; > + wd->minutes_mode = false; > } > > - mutex_unlock(&watchdog.lock); > - > return 0; > } > > -static int watchdog_set_pulse_width(unsigned int pw) > +static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw) > { > - int err = 0; > unsigned int t1 = 25, t2 = 125, t3 = 5000; > > - if (watchdog.variant->id == SIO_F71868_ID) { > + if (wd->variant->id == SIO_F71868_ID) { > t1 = 30; > t2 = 150; > t3 = 6000; > } > > - mutex_lock(&watchdog.lock); > - > if (pw <= 1) { > - watchdog.pulse_val = 0; > + wd->pulse_val = 0; > } else if (pw <= t1) { > - watchdog.pulse_val = 1; > + wd->pulse_val = 1; > } else if (pw <= t2) { > - watchdog.pulse_val = 2; > + wd->pulse_val = 2; > } else if (pw <= t3) { > - watchdog.pulse_val = 3; > + wd->pulse_val = 3; > } else { > pr_err("pulse width out of range\n"); > - err = -EINVAL; > - goto exit_unlock; > + return -EINVAL; > } > > - watchdog.pulse_mode = pw; > + wd->pulse_mode = pw; > > -exit_unlock: > - mutex_unlock(&watchdog.lock); > - return err; > + return 0; > } > > -static int watchdog_keepalive(void) > +static int watchdog_keepalive(struct watchdog_device *wdd) > { > - int err = 0; > + struct watchdog_data *wd = watchdog_get_drvdata(wdd); > + int err; > > - mutex_lock(&watchdog.lock); > - err = superio_enter(watchdog.sioaddr); > + err = superio_enter(wd->sioaddr); > if (err) > - goto exit_unlock; > - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); > + return err; > + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); > > - if (watchdog.minutes_mode) > + if (wd->minutes_mode) > /* select minutes for timer units */ > - superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, > + superio_set_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, > F71808FG_FLAG_WD_UNIT); > else > /* select seconds for timer units */ > - superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, > + superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, > F71808FG_FLAG_WD_UNIT); > > /* Set timer value */ > - superio_outb(watchdog.sioaddr, F71808FG_REG_WD_TIME, > - watchdog.timer_val); > + superio_outb(wd->sioaddr, F71808FG_REG_WD_TIME, > + wd->timer_val); > > - superio_exit(watchdog.sioaddr); > + superio_exit(wd->sioaddr); > > -exit_unlock: > - mutex_unlock(&watchdog.lock); > - return err; > + return 0; > } > > -static int watchdog_start(void) > +static int watchdog_start(struct watchdog_device *wdd) > { > + struct watchdog_data *wd = watchdog_get_drvdata(wdd); > int err; > > /* Make sure we don't die as soon as the watchdog is enabled below */ > - err = watchdog_keepalive(); > + err = watchdog_keepalive(wdd); > if (err) > return err; > > - mutex_lock(&watchdog.lock); > - err = superio_enter(watchdog.sioaddr); > + err = superio_enter(wd->sioaddr); > if (err) > - goto exit_unlock; > - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); > + return err; > + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); > > /* Watchdog pin configuration */ > - watchdog.variant->pinconf(&watchdog); > + wd->variant->pinconf(wd); > > - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); > - superio_set_bit(watchdog.sioaddr, SIO_REG_ENABLE, 0); > + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); > + superio_set_bit(wd->sioaddr, SIO_REG_ENABLE, 0); > > - if (has_f81865_wdo_conf(&watchdog)) > - superio_set_bit(watchdog.sioaddr, F81865_REG_WDO_CONF, > + if (has_f81865_wdo_conf(wd)) > + superio_set_bit(wd->sioaddr, F81865_REG_WDO_CONF, > F81865_FLAG_WDOUT_EN); > else > - superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDO_CONF, > + superio_set_bit(wd->sioaddr, F71808FG_REG_WDO_CONF, > F71808FG_FLAG_WDOUT_EN); > > - superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, > + superio_set_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, > F71808FG_FLAG_WD_EN); > > - if (watchdog.pulse_mode) { > + set_bit(WDOG_HW_RUNNING, &wdd->status); > + > + if (wd->pulse_mode) { > /* Select "pulse" output mode with given duration */ > - u8 wdt_conf = superio_inb(watchdog.sioaddr, > - F71808FG_REG_WDT_CONF); > + u8 wdt_conf = superio_inb(wd->sioaddr, F71808FG_REG_WDT_CONF); > > /* Set WD_PSWIDTH bits (1:0) */ > - wdt_conf = (wdt_conf & 0xfc) | (watchdog.pulse_val & 0x03); > + wdt_conf = (wdt_conf & 0xfc) | (wd->pulse_val & 0x03); > /* Set WD_PULSE to "pulse" mode */ > wdt_conf |= BIT(F71808FG_FLAG_WD_PULSE); > > - superio_outb(watchdog.sioaddr, F71808FG_REG_WDT_CONF, > - wdt_conf); > + superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF, wdt_conf); > } else { > /* Select "level" output mode */ > - superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, > - F71808FG_FLAG_WD_PULSE); > + superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, > + F71808FG_FLAG_WD_PULSE); > } > > - superio_exit(watchdog.sioaddr); > -exit_unlock: > - mutex_unlock(&watchdog.lock); > + superio_exit(wd->sioaddr); > > return err; > } > > -static int watchdog_stop(void) > -{ > - int err = 0; > - > - mutex_lock(&watchdog.lock); > - err = superio_enter(watchdog.sioaddr); > - if (err) > - goto exit_unlock; > - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); > - > - superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, > - F71808FG_FLAG_WD_EN); > - > - superio_exit(watchdog.sioaddr); > - > -exit_unlock: > - mutex_unlock(&watchdog.lock); > - > - return err; > -} > - > -static int watchdog_get_status(void) > -{ > - int status = 0; > - > - mutex_lock(&watchdog.lock); > - status = (watchdog.caused_reboot) ? WDIOF_CARDRESET : 0; > - mutex_unlock(&watchdog.lock); > - > - return status; > -} > - > -static bool watchdog_is_running(void) > -{ > - /* > - * if we fail to determine the watchdog's status assume it to be > - * running to be on the safe side > - */ > - bool is_running = true; > - > - mutex_lock(&watchdog.lock); > - if (superio_enter(watchdog.sioaddr)) > - goto exit_unlock; > - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); > - > - is_running = (superio_inb(watchdog.sioaddr, SIO_REG_ENABLE) & BIT(0)) > - && (superio_inb(watchdog.sioaddr, F71808FG_REG_WDT_CONF) > - & BIT(F71808FG_FLAG_WD_EN)); > - > - superio_exit(watchdog.sioaddr); > - > -exit_unlock: > - mutex_unlock(&watchdog.lock); > - return is_running; > -} > - > -/* /dev/watchdog api */ > - > -static int watchdog_open(struct inode *inode, struct file *file) > +static int watchdog_stop(struct watchdog_device *wdd) > { > + struct watchdog_data *wd = watchdog_get_drvdata(wdd); > int err; > > - /* If the watchdog is alive we don't need to start it again */ > - if (test_and_set_bit(0, &watchdog.opened)) > - return -EBUSY; > - > - err = watchdog_start(); > - if (err) { > - clear_bit(0, &watchdog.opened); > + err = superio_enter(wd->sioaddr); > + if (err) > return err; > - } > + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); > > - if (nowayout) > - __module_get(THIS_MODULE); > + superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, > + F71808FG_FLAG_WD_EN); > > - watchdog.expect_close = 0; > - return stream_open(inode, file); > -} > - > -static int watchdog_release(struct inode *inode, struct file *file) > -{ > - clear_bit(0, &watchdog.opened); > + superio_exit(wd->sioaddr); > > - if (!watchdog.expect_close) { > - watchdog_keepalive(); > - pr_crit("Unexpected close, not stopping watchdog!\n"); > - } else if (!nowayout) { > - watchdog_stop(); > - } > return 0; > } > > -/* > - * watchdog_write: > - * @file: file handle to the watchdog > - * @buf: buffer to write > - * @count: count of bytes > - * @ppos: pointer to the position to write. No seeks allowed > - * > - * A write to a watchdog device is defined as a keepalive signal. Any > - * write of data will do, as we we don't define content meaning. > - */ > - > -static ssize_t watchdog_write(struct file *file, const char __user *buf, > - size_t count, loff_t *ppos) > -{ > - if (count) { > - if (!nowayout) { > - size_t i; > - > - /* In case it was set long ago */ > - bool expect_close = false; > - > - for (i = 0; i != count; i++) { > - char c; > - if (get_user(c, buf + i)) > - return -EFAULT; > - if (c == 'V') > - expect_close = true; > - } > - > - /* Properly order writes across fork()ed processes */ > - mutex_lock(&watchdog.lock); > - watchdog.expect_close = expect_close; > - mutex_unlock(&watchdog.lock); > - } > - > - /* someone wrote to us, we should restart timer */ > - watchdog_keepalive(); > - } > - return count; > -} > - > -/* > - * watchdog_ioctl: > - * @inode: inode of the device > - * @file: file handle to the device > - * @cmd: watchdog command > - * @arg: argument pointer > - * > - * The watchdog API defines a common set of functions for all watchdogs > - * according to their available features. > - */ > -static long watchdog_ioctl(struct file *file, unsigned int cmd, > - unsigned long arg) > -{ > - int status; > - int new_options; > - int new_timeout; > - union { > - struct watchdog_info __user *ident; > - int __user *i; > - } uarg; > - > - uarg.i = (int __user *)arg; > - > - switch (cmd) { > - case WDIOC_GETSUPPORT: > - return copy_to_user(uarg.ident, &watchdog.ident, > - sizeof(watchdog.ident)) ? -EFAULT : 0; > - > - case WDIOC_GETSTATUS: > - status = watchdog_get_status(); > - if (status < 0) > - return status; > - return put_user(status, uarg.i); > - > - case WDIOC_GETBOOTSTATUS: > - return put_user(0, uarg.i); > - > - case WDIOC_SETOPTIONS: > - if (get_user(new_options, uarg.i)) > - return -EFAULT; > - > - if (new_options & WDIOS_DISABLECARD) > - watchdog_stop(); > - > - if (new_options & WDIOS_ENABLECARD) > - return watchdog_start(); > - /* fall through */ > - > - case WDIOC_KEEPALIVE: > - watchdog_keepalive(); > - return 0; > - > - case WDIOC_SETTIMEOUT: > - if (get_user(new_timeout, uarg.i)) > - return -EFAULT; > - > - if (watchdog_set_timeout(new_timeout)) > - return -EINVAL; > - > - watchdog_keepalive(); > - /* fall through */ > - > - case WDIOC_GETTIMEOUT: > - return put_user(watchdog.timeout, uarg.i); > - > - default: > - return -ENOTTY; > - > - } > -} > +static const struct watchdog_ops f71808e_wdog_ops = { > + .owner = THIS_MODULE, > + .start = watchdog_start, > + .stop = watchdog_stop, > + .ping = watchdog_keepalive, > + .set_timeout = watchdog_set_timeout, > +}; > > -static int watchdog_notify_sys(struct notifier_block *this, unsigned long code, > - void *unused) > +static bool watchdog_is_running(struct watchdog_data *wd, u8 wdt_conf) > { > - if (code == SYS_DOWN || code == SYS_HALT) > - watchdog_stop(); > - return NOTIFY_DONE; > + return (superio_inb(wd->sioaddr, SIO_REG_ENABLE) & BIT(0)) > + && (wdt_conf & BIT(F71808FG_FLAG_WD_EN)); > } > > -static const struct file_operations watchdog_fops = { > - .owner = THIS_MODULE, > - .llseek = no_llseek, > - .open = watchdog_open, > - .release = watchdog_release, > - .write = watchdog_write, > - .unlocked_ioctl = watchdog_ioctl, > - .compat_ioctl = compat_ptr_ioctl, > -}; > - > -static struct miscdevice watchdog_miscdev = { > - .minor = WATCHDOG_MINOR, > - .name = "watchdog", > - .fops = &watchdog_fops, > -}; > - > -static struct notifier_block watchdog_notifier = { > - .notifier_call = watchdog_notify_sys, > -}; > - > -static int __init watchdog_init(int sioaddr) > +static int __init watchdog_init(struct watchdog_data *wd) > { > + struct watchdog_device *wdd = &wd->wdd; > int wdt_conf, err = 0; > > /* No need to lock watchdog.lock here because no entry points > * into the module have been registered yet. > */ > - watchdog.sioaddr = sioaddr; > - watchdog.ident.options = WDIOF_MAGICCLOSE > + wd->ident.options = WDIOF_SETTIMEOUT > + | WDIOF_MAGICCLOSE > | WDIOF_KEEPALIVEPING > | WDIOF_CARDRESET; > > - snprintf(watchdog.ident.identity, > - sizeof(watchdog.ident.identity), "%s watchdog", > - watchdog.variant->wdt_name); > + snprintf(wd->ident.identity, sizeof(wd->ident.identity), > + "%s watchdog", wd->variant->wdt_name); > > - err = superio_enter(sioaddr); > + err = superio_enter(wd->sioaddr); > if (err) > return err; > - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); > + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); > > - wdt_conf = superio_inb(sioaddr, F71808FG_REG_WDT_CONF); > - watchdog.caused_reboot = wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS); > + wdt_conf = superio_inb(wd->sioaddr, F71808FG_REG_WDT_CONF); > > /* > * We don't want WDTMOUT_STS to stick around till regular reboot. > * Write 1 to the bit to clear it to zero. > */ > - superio_outb(sioaddr, F71808FG_REG_WDT_CONF, > + superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF, > wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS)); > > - superio_exit(sioaddr); > + if (watchdog_is_running(wd, wdt_conf)) > + set_bit(WDOG_HW_RUNNING, &wdd->status); > > - err = watchdog_set_timeout(timeout); > - if (err) > - return err; > - err = watchdog_set_pulse_width(pulse_width); > - if (err) > - return err; > + superio_exit(wd->sioaddr); > > - err = register_reboot_notifier(&watchdog_notifier); > + err = watchdog_set_pulse_width(wd, pulse_width); > if (err) > return err; > > - err = misc_register(&watchdog_miscdev); > - if (err) { > - pr_err("cannot register miscdev on minor=%d\n", > - watchdog_miscdev.minor); > - goto exit_reboot; > - } > - > - if (start_withtimeout) { > - if (start_withtimeout <= 0 > - || start_withtimeout > max_timeout) { > - pr_err("starting timeout out of range\n"); > - err = -EINVAL; > - goto exit_miscdev; > - } > - > - err = watchdog_start(); > - if (err) { > - pr_err("cannot start watchdog timer\n"); > - goto exit_miscdev; > - } > - > - mutex_lock(&watchdog.lock); > - err = superio_enter(sioaddr); > - if (err) > - goto exit_unlock; > - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); > - > - if (start_withtimeout > 0xff) { > - /* select minutes for timer units */ > - superio_set_bit(sioaddr, F71808FG_REG_WDT_CONF, > - F71808FG_FLAG_WD_UNIT); > - superio_outb(sioaddr, F71808FG_REG_WD_TIME, > - DIV_ROUND_UP(start_withtimeout, 60)); > - } else { > - /* select seconds for timer units */ > - superio_clear_bit(sioaddr, F71808FG_REG_WDT_CONF, > - F71808FG_FLAG_WD_UNIT); > - superio_outb(sioaddr, F71808FG_REG_WD_TIME, > - start_withtimeout); > - } > - > - superio_exit(sioaddr); > - mutex_unlock(&watchdog.lock); > - > - if (nowayout) > - __module_get(THIS_MODULE); > + wdd->info = &wd->ident; > + wdd->ops = &f71808e_wdog_ops; > + wdd->min_timeout = 1; > + wdd->max_timeout = max_timeout; > + wdd->timeout = timeout; > > - pr_info("watchdog started with initial timeout of %u sec\n", > - start_withtimeout); > - } > + watchdog_set_nowayout(wdd, nowayout); > + watchdog_stop_on_unregister(wdd); > + watchdog_stop_on_reboot(wdd); > + watchdog_init_timeout(wdd, start_withtimeout, NULL); > + watchdog_set_drvdata(wdd, wd); > > - return 0; > + if (wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS)) > + wdd->bootstatus = WDIOF_CARDRESET; > > -exit_unlock: > - mutex_unlock(&watchdog.lock); > -exit_miscdev: > - misc_deregister(&watchdog_miscdev); > -exit_reboot: > - unregister_reboot_notifier(&watchdog_notifier); > + /* > + * WATCHDOG_HANDLE_BOOT_ENABLED can result in keepalive being directly > + * called without a set_timeout before, so it needs to be done here once > + */ > + watchdog_set_timeout(wdd, wdd->timeout); > > - return err; > + return watchdog_register_device(wdd); > } > > static void f71808fg_pinconf(struct watchdog_data *wd) > @@ -812,8 +536,11 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr) > return variant; > } > > +static struct watchdog_data watchdog; > + > static int __init f71808e_init(void) > { > + struct watchdog_data *wd = &watchdog; > static const unsigned short addrs[] = { 0x2e, 0x4e }; > struct f71808e_variant *variant; > int i; > @@ -831,19 +558,15 @@ static int __init f71808e_init(void) > if (i == ARRAY_SIZE(addrs)) > return PTR_ERR(variant); > > - watchdog.variant = variant; > + wd->variant = variant; > + wd->sioaddr = addrs[i]; > > - return watchdog_init(addrs[i]); > + return watchdog_init(wd); > } > > static void __exit f71808e_exit(void) > { > - if (watchdog_is_running()) { > - pr_warn("Watchdog timer still running, stopping it\n"); > - watchdog_stop(); > - } > - misc_deregister(&watchdog_miscdev); > - unregister_reboot_notifier(&watchdog_notifier); > + watchdog_unregister_device(&watchdog.wdd); > } > > MODULE_DESCRIPTION("F71808E Watchdog Driver");
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 0663c604bd64..c312dd5fc0ca 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -1052,6 +1052,7 @@ config EBC_C384_WDT config F71808E_WDT tristate "Fintek F718xx, F818xx Super I/O Watchdog" depends on X86 + select WATCHDOG_CORE help This is the driver for the hardware watchdog on the Fintek F71808E, F71862FG, F71868, F71869, F71882FG, F71889FG, F81803, F81865, and diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c index 9299ad4d4a52..7c42cbf9912e 100644 --- a/drivers/watchdog/f71808e_wdt.c +++ b/drivers/watchdog/f71808e_wdt.c @@ -9,16 +9,10 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/err.h> -#include <linux/fs.h> #include <linux/init.h> #include <linux/io.h> #include <linux/ioport.h> -#include <linux/miscdevice.h> #include <linux/module.h> -#include <linux/mutex.h> -#include <linux/notifier.h> -#include <linux/reboot.h> -#include <linux/uaccess.h> #include <linux/watchdog.h> #define DRVNAME "f71808e_wdt" @@ -129,23 +123,15 @@ struct f71808e_variant { }; struct watchdog_data { + struct watchdog_device wdd; unsigned short sioaddr; const struct f71808e_variant *variant; - unsigned long opened; - struct mutex lock; - char expect_close; struct watchdog_info ident; - unsigned short timeout; u8 timer_val; /* content for the wd_time register */ char minutes_mode; u8 pulse_val; /* pulse width flag */ char pulse_mode; /* enable pulse output mode? */ - char caused_reboot; /* last reboot was by the watchdog */ -}; - -static struct watchdog_data watchdog = { - .lock = __MUTEX_INITIALIZER(watchdog.lock), }; static inline bool has_f81865_wdo_conf(struct watchdog_data *wd) @@ -216,487 +202,225 @@ static inline void superio_exit(int base) release_region(base, 2); } -static int watchdog_set_timeout(int timeout) +static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout) { - if (timeout <= 0 - || timeout > max_timeout) { - pr_err("watchdog timeout out of range\n"); - return -EINVAL; - } + struct watchdog_data *wd = watchdog_get_drvdata(wdd); - mutex_lock(&watchdog.lock); - - watchdog.timeout = timeout; - if (timeout > 0xff) { - watchdog.timer_val = DIV_ROUND_UP(timeout, 60); - watchdog.minutes_mode = true; + wdd->timeout = new_timeout; + if (new_timeout > 0xff) { + wd->timer_val = DIV_ROUND_UP(new_timeout, 60); + wd->minutes_mode = true; } else { - watchdog.timer_val = timeout; - watchdog.minutes_mode = false; + wd->timer_val = new_timeout; + wd->minutes_mode = false; } - mutex_unlock(&watchdog.lock); - return 0; } -static int watchdog_set_pulse_width(unsigned int pw) +static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw) { - int err = 0; unsigned int t1 = 25, t2 = 125, t3 = 5000; - if (watchdog.variant->id == SIO_F71868_ID) { + if (wd->variant->id == SIO_F71868_ID) { t1 = 30; t2 = 150; t3 = 6000; } - mutex_lock(&watchdog.lock); - if (pw <= 1) { - watchdog.pulse_val = 0; + wd->pulse_val = 0; } else if (pw <= t1) { - watchdog.pulse_val = 1; + wd->pulse_val = 1; } else if (pw <= t2) { - watchdog.pulse_val = 2; + wd->pulse_val = 2; } else if (pw <= t3) { - watchdog.pulse_val = 3; + wd->pulse_val = 3; } else { pr_err("pulse width out of range\n"); - err = -EINVAL; - goto exit_unlock; + return -EINVAL; } - watchdog.pulse_mode = pw; + wd->pulse_mode = pw; -exit_unlock: - mutex_unlock(&watchdog.lock); - return err; + return 0; } -static int watchdog_keepalive(void) +static int watchdog_keepalive(struct watchdog_device *wdd) { - int err = 0; + struct watchdog_data *wd = watchdog_get_drvdata(wdd); + int err; - mutex_lock(&watchdog.lock); - err = superio_enter(watchdog.sioaddr); + err = superio_enter(wd->sioaddr); if (err) - goto exit_unlock; - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); + return err; + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); - if (watchdog.minutes_mode) + if (wd->minutes_mode) /* select minutes for timer units */ - superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, + superio_set_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, F71808FG_FLAG_WD_UNIT); else /* select seconds for timer units */ - superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, + superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, F71808FG_FLAG_WD_UNIT); /* Set timer value */ - superio_outb(watchdog.sioaddr, F71808FG_REG_WD_TIME, - watchdog.timer_val); + superio_outb(wd->sioaddr, F71808FG_REG_WD_TIME, + wd->timer_val); - superio_exit(watchdog.sioaddr); + superio_exit(wd->sioaddr); -exit_unlock: - mutex_unlock(&watchdog.lock); - return err; + return 0; } -static int watchdog_start(void) +static int watchdog_start(struct watchdog_device *wdd) { + struct watchdog_data *wd = watchdog_get_drvdata(wdd); int err; /* Make sure we don't die as soon as the watchdog is enabled below */ - err = watchdog_keepalive(); + err = watchdog_keepalive(wdd); if (err) return err; - mutex_lock(&watchdog.lock); - err = superio_enter(watchdog.sioaddr); + err = superio_enter(wd->sioaddr); if (err) - goto exit_unlock; - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); + return err; + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); /* Watchdog pin configuration */ - watchdog.variant->pinconf(&watchdog); + wd->variant->pinconf(wd); - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); - superio_set_bit(watchdog.sioaddr, SIO_REG_ENABLE, 0); + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); + superio_set_bit(wd->sioaddr, SIO_REG_ENABLE, 0); - if (has_f81865_wdo_conf(&watchdog)) - superio_set_bit(watchdog.sioaddr, F81865_REG_WDO_CONF, + if (has_f81865_wdo_conf(wd)) + superio_set_bit(wd->sioaddr, F81865_REG_WDO_CONF, F81865_FLAG_WDOUT_EN); else - superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDO_CONF, + superio_set_bit(wd->sioaddr, F71808FG_REG_WDO_CONF, F71808FG_FLAG_WDOUT_EN); - superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, + superio_set_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, F71808FG_FLAG_WD_EN); - if (watchdog.pulse_mode) { + set_bit(WDOG_HW_RUNNING, &wdd->status); + + if (wd->pulse_mode) { /* Select "pulse" output mode with given duration */ - u8 wdt_conf = superio_inb(watchdog.sioaddr, - F71808FG_REG_WDT_CONF); + u8 wdt_conf = superio_inb(wd->sioaddr, F71808FG_REG_WDT_CONF); /* Set WD_PSWIDTH bits (1:0) */ - wdt_conf = (wdt_conf & 0xfc) | (watchdog.pulse_val & 0x03); + wdt_conf = (wdt_conf & 0xfc) | (wd->pulse_val & 0x03); /* Set WD_PULSE to "pulse" mode */ wdt_conf |= BIT(F71808FG_FLAG_WD_PULSE); - superio_outb(watchdog.sioaddr, F71808FG_REG_WDT_CONF, - wdt_conf); + superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF, wdt_conf); } else { /* Select "level" output mode */ - superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, - F71808FG_FLAG_WD_PULSE); + superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, + F71808FG_FLAG_WD_PULSE); } - superio_exit(watchdog.sioaddr); -exit_unlock: - mutex_unlock(&watchdog.lock); + superio_exit(wd->sioaddr); return err; } -static int watchdog_stop(void) -{ - int err = 0; - - mutex_lock(&watchdog.lock); - err = superio_enter(watchdog.sioaddr); - if (err) - goto exit_unlock; - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); - - superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF, - F71808FG_FLAG_WD_EN); - - superio_exit(watchdog.sioaddr); - -exit_unlock: - mutex_unlock(&watchdog.lock); - - return err; -} - -static int watchdog_get_status(void) -{ - int status = 0; - - mutex_lock(&watchdog.lock); - status = (watchdog.caused_reboot) ? WDIOF_CARDRESET : 0; - mutex_unlock(&watchdog.lock); - - return status; -} - -static bool watchdog_is_running(void) -{ - /* - * if we fail to determine the watchdog's status assume it to be - * running to be on the safe side - */ - bool is_running = true; - - mutex_lock(&watchdog.lock); - if (superio_enter(watchdog.sioaddr)) - goto exit_unlock; - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); - - is_running = (superio_inb(watchdog.sioaddr, SIO_REG_ENABLE) & BIT(0)) - && (superio_inb(watchdog.sioaddr, F71808FG_REG_WDT_CONF) - & BIT(F71808FG_FLAG_WD_EN)); - - superio_exit(watchdog.sioaddr); - -exit_unlock: - mutex_unlock(&watchdog.lock); - return is_running; -} - -/* /dev/watchdog api */ - -static int watchdog_open(struct inode *inode, struct file *file) +static int watchdog_stop(struct watchdog_device *wdd) { + struct watchdog_data *wd = watchdog_get_drvdata(wdd); int err; - /* If the watchdog is alive we don't need to start it again */ - if (test_and_set_bit(0, &watchdog.opened)) - return -EBUSY; - - err = watchdog_start(); - if (err) { - clear_bit(0, &watchdog.opened); + err = superio_enter(wd->sioaddr); + if (err) return err; - } + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); - if (nowayout) - __module_get(THIS_MODULE); + superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF, + F71808FG_FLAG_WD_EN); - watchdog.expect_close = 0; - return stream_open(inode, file); -} - -static int watchdog_release(struct inode *inode, struct file *file) -{ - clear_bit(0, &watchdog.opened); + superio_exit(wd->sioaddr); - if (!watchdog.expect_close) { - watchdog_keepalive(); - pr_crit("Unexpected close, not stopping watchdog!\n"); - } else if (!nowayout) { - watchdog_stop(); - } return 0; } -/* - * watchdog_write: - * @file: file handle to the watchdog - * @buf: buffer to write - * @count: count of bytes - * @ppos: pointer to the position to write. No seeks allowed - * - * A write to a watchdog device is defined as a keepalive signal. Any - * write of data will do, as we we don't define content meaning. - */ - -static ssize_t watchdog_write(struct file *file, const char __user *buf, - size_t count, loff_t *ppos) -{ - if (count) { - if (!nowayout) { - size_t i; - - /* In case it was set long ago */ - bool expect_close = false; - - for (i = 0; i != count; i++) { - char c; - if (get_user(c, buf + i)) - return -EFAULT; - if (c == 'V') - expect_close = true; - } - - /* Properly order writes across fork()ed processes */ - mutex_lock(&watchdog.lock); - watchdog.expect_close = expect_close; - mutex_unlock(&watchdog.lock); - } - - /* someone wrote to us, we should restart timer */ - watchdog_keepalive(); - } - return count; -} - -/* - * watchdog_ioctl: - * @inode: inode of the device - * @file: file handle to the device - * @cmd: watchdog command - * @arg: argument pointer - * - * The watchdog API defines a common set of functions for all watchdogs - * according to their available features. - */ -static long watchdog_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) -{ - int status; - int new_options; - int new_timeout; - union { - struct watchdog_info __user *ident; - int __user *i; - } uarg; - - uarg.i = (int __user *)arg; - - switch (cmd) { - case WDIOC_GETSUPPORT: - return copy_to_user(uarg.ident, &watchdog.ident, - sizeof(watchdog.ident)) ? -EFAULT : 0; - - case WDIOC_GETSTATUS: - status = watchdog_get_status(); - if (status < 0) - return status; - return put_user(status, uarg.i); - - case WDIOC_GETBOOTSTATUS: - return put_user(0, uarg.i); - - case WDIOC_SETOPTIONS: - if (get_user(new_options, uarg.i)) - return -EFAULT; - - if (new_options & WDIOS_DISABLECARD) - watchdog_stop(); - - if (new_options & WDIOS_ENABLECARD) - return watchdog_start(); - /* fall through */ - - case WDIOC_KEEPALIVE: - watchdog_keepalive(); - return 0; - - case WDIOC_SETTIMEOUT: - if (get_user(new_timeout, uarg.i)) - return -EFAULT; - - if (watchdog_set_timeout(new_timeout)) - return -EINVAL; - - watchdog_keepalive(); - /* fall through */ - - case WDIOC_GETTIMEOUT: - return put_user(watchdog.timeout, uarg.i); - - default: - return -ENOTTY; - - } -} +static const struct watchdog_ops f71808e_wdog_ops = { + .owner = THIS_MODULE, + .start = watchdog_start, + .stop = watchdog_stop, + .ping = watchdog_keepalive, + .set_timeout = watchdog_set_timeout, +}; -static int watchdog_notify_sys(struct notifier_block *this, unsigned long code, - void *unused) +static bool watchdog_is_running(struct watchdog_data *wd, u8 wdt_conf) { - if (code == SYS_DOWN || code == SYS_HALT) - watchdog_stop(); - return NOTIFY_DONE; + return (superio_inb(wd->sioaddr, SIO_REG_ENABLE) & BIT(0)) + && (wdt_conf & BIT(F71808FG_FLAG_WD_EN)); } -static const struct file_operations watchdog_fops = { - .owner = THIS_MODULE, - .llseek = no_llseek, - .open = watchdog_open, - .release = watchdog_release, - .write = watchdog_write, - .unlocked_ioctl = watchdog_ioctl, - .compat_ioctl = compat_ptr_ioctl, -}; - -static struct miscdevice watchdog_miscdev = { - .minor = WATCHDOG_MINOR, - .name = "watchdog", - .fops = &watchdog_fops, -}; - -static struct notifier_block watchdog_notifier = { - .notifier_call = watchdog_notify_sys, -}; - -static int __init watchdog_init(int sioaddr) +static int __init watchdog_init(struct watchdog_data *wd) { + struct watchdog_device *wdd = &wd->wdd; int wdt_conf, err = 0; /* No need to lock watchdog.lock here because no entry points * into the module have been registered yet. */ - watchdog.sioaddr = sioaddr; - watchdog.ident.options = WDIOF_MAGICCLOSE + wd->ident.options = WDIOF_SETTIMEOUT + | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING | WDIOF_CARDRESET; - snprintf(watchdog.ident.identity, - sizeof(watchdog.ident.identity), "%s watchdog", - watchdog.variant->wdt_name); + snprintf(wd->ident.identity, sizeof(wd->ident.identity), + "%s watchdog", wd->variant->wdt_name); - err = superio_enter(sioaddr); + err = superio_enter(wd->sioaddr); if (err) return err; - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); + superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT); - wdt_conf = superio_inb(sioaddr, F71808FG_REG_WDT_CONF); - watchdog.caused_reboot = wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS); + wdt_conf = superio_inb(wd->sioaddr, F71808FG_REG_WDT_CONF); /* * We don't want WDTMOUT_STS to stick around till regular reboot. * Write 1 to the bit to clear it to zero. */ - superio_outb(sioaddr, F71808FG_REG_WDT_CONF, + superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF, wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS)); - superio_exit(sioaddr); + if (watchdog_is_running(wd, wdt_conf)) + set_bit(WDOG_HW_RUNNING, &wdd->status); - err = watchdog_set_timeout(timeout); - if (err) - return err; - err = watchdog_set_pulse_width(pulse_width); - if (err) - return err; + superio_exit(wd->sioaddr); - err = register_reboot_notifier(&watchdog_notifier); + err = watchdog_set_pulse_width(wd, pulse_width); if (err) return err; - err = misc_register(&watchdog_miscdev); - if (err) { - pr_err("cannot register miscdev on minor=%d\n", - watchdog_miscdev.minor); - goto exit_reboot; - } - - if (start_withtimeout) { - if (start_withtimeout <= 0 - || start_withtimeout > max_timeout) { - pr_err("starting timeout out of range\n"); - err = -EINVAL; - goto exit_miscdev; - } - - err = watchdog_start(); - if (err) { - pr_err("cannot start watchdog timer\n"); - goto exit_miscdev; - } - - mutex_lock(&watchdog.lock); - err = superio_enter(sioaddr); - if (err) - goto exit_unlock; - superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT); - - if (start_withtimeout > 0xff) { - /* select minutes for timer units */ - superio_set_bit(sioaddr, F71808FG_REG_WDT_CONF, - F71808FG_FLAG_WD_UNIT); - superio_outb(sioaddr, F71808FG_REG_WD_TIME, - DIV_ROUND_UP(start_withtimeout, 60)); - } else { - /* select seconds for timer units */ - superio_clear_bit(sioaddr, F71808FG_REG_WDT_CONF, - F71808FG_FLAG_WD_UNIT); - superio_outb(sioaddr, F71808FG_REG_WD_TIME, - start_withtimeout); - } - - superio_exit(sioaddr); - mutex_unlock(&watchdog.lock); - - if (nowayout) - __module_get(THIS_MODULE); + wdd->info = &wd->ident; + wdd->ops = &f71808e_wdog_ops; + wdd->min_timeout = 1; + wdd->max_timeout = max_timeout; + wdd->timeout = timeout; - pr_info("watchdog started with initial timeout of %u sec\n", - start_withtimeout); - } + watchdog_set_nowayout(wdd, nowayout); + watchdog_stop_on_unregister(wdd); + watchdog_stop_on_reboot(wdd); + watchdog_init_timeout(wdd, start_withtimeout, NULL); + watchdog_set_drvdata(wdd, wd); - return 0; + if (wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS)) + wdd->bootstatus = WDIOF_CARDRESET; -exit_unlock: - mutex_unlock(&watchdog.lock); -exit_miscdev: - misc_deregister(&watchdog_miscdev); -exit_reboot: - unregister_reboot_notifier(&watchdog_notifier); + /* + * WATCHDOG_HANDLE_BOOT_ENABLED can result in keepalive being directly + * called without a set_timeout before, so it needs to be done here once + */ + watchdog_set_timeout(wdd, wdd->timeout); - return err; + return watchdog_register_device(wdd); } static void f71808fg_pinconf(struct watchdog_data *wd) @@ -812,8 +536,11 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr) return variant; } +static struct watchdog_data watchdog; + static int __init f71808e_init(void) { + struct watchdog_data *wd = &watchdog; static const unsigned short addrs[] = { 0x2e, 0x4e }; struct f71808e_variant *variant; int i; @@ -831,19 +558,15 @@ static int __init f71808e_init(void) if (i == ARRAY_SIZE(addrs)) return PTR_ERR(variant); - watchdog.variant = variant; + wd->variant = variant; + wd->sioaddr = addrs[i]; - return watchdog_init(addrs[i]); + return watchdog_init(wd); } static void __exit f71808e_exit(void) { - if (watchdog_is_running()) { - pr_warn("Watchdog timer still running, stopping it\n"); - watchdog_stop(); - } - misc_deregister(&watchdog_miscdev); - unregister_reboot_notifier(&watchdog_notifier); + watchdog_unregister_device(&watchdog.wdd); } MODULE_DESCRIPTION("F71808E Watchdog Driver");
Migrating the driver lets us drop the watchdog misc device boilerplate and reduce size by 270~ lines. It also brings us support for new functionality like CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED. This changes behavior on module unloading: Whereas before, the watchdog was automatically stopped on module unload, now explicit stopping of the watchdog before unload is required. Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- drivers/watchdog/Kconfig | 1 + drivers/watchdog/f71808e_wdt.c | 497 ++++++++------------------------- 2 files changed, 111 insertions(+), 387 deletions(-)