Re: [PATCH] watchdog: ath79: convert to watchdog_device

[Guenter Roeck]

On Mon, May 18, 2026 at 05:33:26PM -0700, Rosen Penev wrote:
> Converts from the legacy miscdevice/file_operations interface to the

s/Converts/Convert/

> watchdog_device framework, simplifying the driver and properly handling
> EPROBE_DEFER.
> 
> Use wdd->timeout in all callbacks instead of a global, fix get_timeleft()
> to return seconds instead of raw clock ticks, store max_timeout in the
> watchdog_device struct, wire up nowayout via watchdog_set_nowayout(), pass
> the module parameter to watchdog_init_timeout(), add .owner to
> watchdog_ops, and remove the leftover file-ops state (wdt_flags and the
> busy/expect-close bits).
> 
> Tested on an Archer C7v2. sysupgrade watchdog handoff works fine.
> 
Sorry for having to ask, but is this real or an AI hallucination ?

> Assisted-by: Claude:Sonnet-4.6
> Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>

$ scripts/checkpatch.pl --strict index.html.1
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#163: FILE: drivers/watchdog/ath79_wdt.c:62:
+static inline void ath79_wdt_wr(struct ath79_wdt *wdt, unsigned reg, u32 val)

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#170: FILE: drivers/watchdog/ath79_wdt.c:67:
+static inline u32 ath79_wdt_rr(struct ath79_wdt *wdt, unsigned reg)

Please run checkpatch --strict on your patches.

The patch does not apply to the tip of the watchdog-next branch.

$ git am -s -3 index.html
Applying: watchdog: ath79: convert to watchdog_device
error: sha1 information is lacking or useless (drivers/watchdog/ath79_wdt.c).
error: could not build fake ancestor

Please base it on a known reference so it can be applied.

Additional patches inline.

Thanks,
Guenter

> ---
>  drivers/watchdog/ath79_wdt.c | 244 +++++++++++------------------------
>  1 file changed, 73 insertions(+), 171 deletions(-)
> 
> diff --git a/drivers/watchdog/ath79_wdt.c b/drivers/watchdog/ath79_wdt.c
> index 409a40b14901..d83957712c02 100644
> --- a/drivers/watchdog/ath79_wdt.c
> +++ b/drivers/watchdog/ath79_wdt.c
> @@ -15,13 +15,10 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

No longer needed.

>  
> -#include <linux/bitops.h>
>  #include <linux/delay.h>
>  #include <linux/errno.h>
> -#include <linux/fs.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> -#include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
>  #include <linux/platform_device.h>
> @@ -31,7 +28,6 @@
>  #include <linux/err.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> -#include <linux/uaccess.h>
>  
>  #define DRIVER_NAME	"ath79-wdt"
>  
> @@ -57,37 +53,32 @@ module_param(timeout, int, 0);
>  MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds "
>  			  "(default=" __MODULE_STRING(WDT_TIMEOUT) "s)");
>  
> -static unsigned long wdt_flags;
> -
> -#define WDT_FLAGS_BUSY		0
> -#define WDT_FLAGS_EXPECT_CLOSE	1
> -
> -static struct clk *wdt_clk;
> -static unsigned long wdt_freq;
> -static int boot_status;
> -static int max_timeout;
> -static void __iomem *wdt_base;
> +struct ath79_wdt {
> +	void __iomem *base;
> +	unsigned long freq;
> +	struct watchdog_device wdd;
> +};
>  
> -static inline void ath79_wdt_wr(unsigned reg, u32 val)
> +static inline void ath79_wdt_wr(struct ath79_wdt *wdt, unsigned reg, u32 val)
>  {
> -	iowrite32(val, wdt_base + reg);
> +	iowrite32(val, wdt->base + reg);
>  }
>  
> -static inline u32 ath79_wdt_rr(unsigned reg)
> +static inline u32 ath79_wdt_rr(struct ath79_wdt *wdt, unsigned reg)
>  {
> -	return ioread32(wdt_base + reg);
> +	return ioread32(wdt->base + reg);
>  }
>  
> -static inline void ath79_wdt_keepalive(void)
> +static inline void ath79_wdt_keepalive(struct ath79_wdt *wdt)
>  {
> -	ath79_wdt_wr(WDOG_REG_TIMER, wdt_freq * timeout);
> +	ath79_wdt_wr(wdt, WDOG_REG_TIMER, wdt->freq * wdt->wdd.timeout);
>  	/* flush write */
> -	ath79_wdt_rr(WDOG_REG_TIMER);
> +	ath79_wdt_rr(wdt, WDOG_REG_TIMER);
>  }
>  
> -static inline void ath79_wdt_enable(void)
> +static inline void ath79_wdt_enable(struct ath79_wdt *wdt)
>  {
> -	ath79_wdt_keepalive();
> +	ath79_wdt_keepalive(wdt);
>  
>  	/*
>  	 * Updating the TIMER register requires a few microseconds
> @@ -97,202 +88,115 @@ static inline void ath79_wdt_enable(void)
>  	 */
>  	udelay(2);
>  
> -	ath79_wdt_wr(WDOG_REG_CTRL, WDOG_CTRL_ACTION_FCR);
> +	ath79_wdt_wr(wdt, WDOG_REG_CTRL, WDOG_CTRL_ACTION_FCR);
>  	/* flush write */
> -	ath79_wdt_rr(WDOG_REG_CTRL);
> +	ath79_wdt_rr(wdt, WDOG_REG_CTRL);
>  }
>  
> -static inline void ath79_wdt_disable(void)
> +static inline void ath79_wdt_disable(struct ath79_wdt *wdt)
>  {
> -	ath79_wdt_wr(WDOG_REG_CTRL, WDOG_CTRL_ACTION_NONE);
> +	ath79_wdt_wr(wdt, WDOG_REG_CTRL, WDOG_CTRL_ACTION_NONE);
>  	/* flush write */
> -	ath79_wdt_rr(WDOG_REG_CTRL);
> +	ath79_wdt_rr(wdt, WDOG_REG_CTRL);
>  }
>  
> -static int ath79_wdt_set_timeout(int val)
> +static int ath79_wdt_ping(struct watchdog_device *wdd)
>  {
> -	if (val < 1 || val > max_timeout)
> -		return -EINVAL;
> +	struct ath79_wdt *wdt = watchdog_get_drvdata(wdd);
>  
> -	timeout = val;
> -	ath79_wdt_keepalive();
> +	ath79_wdt_keepalive(wdt);
>  
>  	return 0;
>  }
>  
> -static int ath79_wdt_open(struct inode *inode, struct file *file)
> +static int ath79_wdt_set_timeout(struct watchdog_device *wdd, unsigned int val)
>  {
> -	if (test_and_set_bit(WDT_FLAGS_BUSY, &wdt_flags))
> -		return -EBUSY;
> +	struct ath79_wdt *wdt = watchdog_get_drvdata(wdd);
>  
> -	clear_bit(WDT_FLAGS_EXPECT_CLOSE, &wdt_flags);
> -	ath79_wdt_enable();
> +	wdd->timeout = val;
> +	ath79_wdt_keepalive(wdt);
>  
> -	return stream_open(inode, file);
> +	return 0;
>  }
>  
> -static int ath79_wdt_release(struct inode *inode, struct file *file)
> +static int ath79_wdt_start(struct watchdog_device *wdd)
>  {
> -	if (test_bit(WDT_FLAGS_EXPECT_CLOSE, &wdt_flags))
> -		ath79_wdt_disable();
> -	else {
> -		pr_crit("device closed unexpectedly, watchdog timer will not stop!\n");
> -		ath79_wdt_keepalive();
> -	}
> +	struct ath79_wdt *wdt = watchdog_get_drvdata(wdd);
>  
> -	clear_bit(WDT_FLAGS_BUSY, &wdt_flags);
> -	clear_bit(WDT_FLAGS_EXPECT_CLOSE, &wdt_flags);
> +	ath79_wdt_enable(wdt);
>  
>  	return 0;
>  }
>  
> -static ssize_t ath79_wdt_write(struct file *file, const char *data,
> -				size_t len, loff_t *ppos)
> +static unsigned int ath79_wdt_get_timeleft(struct watchdog_device *wdd)
>  {
> -	if (len) {
> -		if (!nowayout) {
> -			size_t i;
> -
> -			clear_bit(WDT_FLAGS_EXPECT_CLOSE, &wdt_flags);
> +	struct ath79_wdt *wdt = watchdog_get_drvdata(wdd);
>  
> -			for (i = 0; i != len; i++) {
> -				char c;
> -
> -				if (get_user(c, data + i))
> -					return -EFAULT;
> +	return ath79_wdt_rr(wdt, WDOG_REG_TIMER) / wdt->freq;
> +}
>  
> -				if (c == 'V')
> -					set_bit(WDT_FLAGS_EXPECT_CLOSE,
> -						&wdt_flags);
> -			}
> -		}
> +static int ath79_wdt_stop(struct watchdog_device *wdd)
> +{
> +	struct ath79_wdt *wdt = watchdog_get_drvdata(wdd);
>  
> -		ath79_wdt_keepalive();
> -	}
> +	ath79_wdt_disable(wdt);
>  
> -	return len;
> +	return 0;
>  }
>  
>  static const struct watchdog_info ath79_wdt_info = {
> -	.options		= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> -				  WDIOF_MAGICCLOSE | WDIOF_CARDRESET,
> -	.firmware_version	= 0,
> -	.identity		= "ATH79 watchdog",
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_CARDRESET,
> +	.firmware_version = 0,

Unnecessary; can be dropped.

> +	.identity = "ATH79 watchdog",
>  };
>  
> -static long ath79_wdt_ioctl(struct file *file, unsigned int cmd,
> -			    unsigned long arg)
> -{
> -	void __user *argp = (void __user *)arg;
> -	int __user *p = argp;
> -	int err;
> -	int t;
> -
> -	switch (cmd) {
> -	case WDIOC_GETSUPPORT:
> -		err = copy_to_user(argp, &ath79_wdt_info,
> -				   sizeof(ath79_wdt_info)) ? -EFAULT : 0;
> -		break;
> -
> -	case WDIOC_GETSTATUS:
> -		err = put_user(0, p);
> -		break;
> -
> -	case WDIOC_GETBOOTSTATUS:
> -		err = put_user(boot_status, p);
> -		break;
> -
> -	case WDIOC_KEEPALIVE:
> -		ath79_wdt_keepalive();
> -		err = 0;
> -		break;
> -
> -	case WDIOC_SETTIMEOUT:
> -		err = get_user(t, p);
> -		if (err)
> -			break;
> -
> -		err = ath79_wdt_set_timeout(t);
> -		if (err)
> -			break;
> -		fallthrough;
> -
> -	case WDIOC_GETTIMEOUT:
> -		err = put_user(timeout, p);
> -		break;
> -
> -	default:
> -		err = -ENOTTY;
> -		break;
> -	}
> -
> -	return err;
> -}
> -
> -static const struct file_operations ath79_wdt_fops = {
> -	.owner		= THIS_MODULE,
> -	.write		= ath79_wdt_write,
> -	.unlocked_ioctl	= ath79_wdt_ioctl,
> -	.compat_ioctl	= compat_ptr_ioctl,
> -	.open		= ath79_wdt_open,
> -	.release	= ath79_wdt_release,
> -};
> -
> -static struct miscdevice ath79_wdt_miscdev = {
> -	.minor = WATCHDOG_MINOR,
> -	.name = "watchdog",
> -	.fops = &ath79_wdt_fops,
> +static const struct watchdog_ops ath79_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = ath79_wdt_start,
> +	.stop = ath79_wdt_stop,
> +	.ping = ath79_wdt_ping,
> +	.set_timeout = ath79_wdt_set_timeout,
> +	.get_timeleft = ath79_wdt_get_timeleft,
>  };
>  
>  static int ath79_wdt_probe(struct platform_device *pdev)
>  {
> +	struct ath79_wdt *wdt;
> +	struct clk *wdt_clk;
>  	u32 ctrl;
> -	int err;
>  
> -	if (wdt_base)
> -		return -EBUSY;
> +	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> +	if (!wdt)
> +		return -ENOMEM;
>  
> -	wdt_base = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(wdt_base))
> -		return PTR_ERR(wdt_base);
> +	wdt->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(wdt->base))
> +		return PTR_ERR(wdt->base);
>  
>  	wdt_clk = devm_clk_get_enabled(&pdev->dev, "wdt");
>  	if (IS_ERR(wdt_clk))
>  		return PTR_ERR(wdt_clk);
>  
> -	wdt_freq = clk_get_rate(wdt_clk);
> -	if (!wdt_freq)
> +	wdt->freq = clk_get_rate(wdt_clk);
> +	if (!wdt->freq)
>  		return -EINVAL;
>  
> -	max_timeout = (0xfffffffful / wdt_freq);
> -	if (timeout < 1 || timeout > max_timeout) {
> -		timeout = max_timeout;
> -		dev_info(&pdev->dev,
> -			"timeout value must be 0 < timeout < %d, using %d\n",
> -			max_timeout, timeout);
> -	}
> -
> -	ctrl = ath79_wdt_rr(WDOG_REG_CTRL);
> -	boot_status = (ctrl & WDOG_CTRL_LAST_RESET) ? WDIOF_CARDRESET : 0;
> +	wdt->wdd.max_timeout = U32_MAX / wdt->freq;
>  
> -	err = misc_register(&ath79_wdt_miscdev);
> -	if (err) {
> -		dev_err(&pdev->dev,
> -			"unable to register misc device, err=%d\n", err);
> -		return err;
> -	}
> +	ctrl = ath79_wdt_rr(wdt, WDOG_REG_CTRL);
> +	wdt->wdd.bootstatus = (ctrl & WDOG_CTRL_LAST_RESET) ? WDIOF_CARDRESET : 0;
>  
> -	return 0;
> -}
> +	wdt->wdd.info = &ath79_wdt_info;
> +	wdt->wdd.ops = &ath79_wdt_ops;
> +	wdt->wdd.min_timeout = 1;
> +	wdt->wdd.timeout = WDT_TIMEOUT;

Also:
	wdt->wdd.parent = &pdev->dev;

> +	watchdog_init_timeout(&wdt->wdd, timeout, &pdev->dev);
> +	watchdog_set_nowayout(&wdt->wdd, nowayout);
> +	watchdog_stop_on_reboot(&wdt->wdd);
>  
> -static void ath79_wdt_remove(struct platform_device *pdev)
> -{
> -	misc_deregister(&ath79_wdt_miscdev);
> -}
> +	watchdog_set_drvdata(&wdt->wdd, wdt);
>  
> -static void ath79_wdt_shutdown(struct platform_device *pdev)
> -{
> -	ath79_wdt_disable();
> +	return devm_watchdog_register_device(&pdev->dev, &wdt->wdd);
>  }
>  
>  static const struct of_device_id ath79_wdt_match[] = {
> @@ -303,8 +207,6 @@ MODULE_DEVICE_TABLE(of, ath79_wdt_match);
>  
>  static struct platform_driver ath79_wdt_driver = {
>  	.probe		= ath79_wdt_probe,
> -	.remove		= ath79_wdt_remove,
> -	.shutdown	= ath79_wdt_shutdown,
>  	.driver		= {
>  		.name	= DRIVER_NAME,
>  		.of_match_table = ath79_wdt_match,
> @@ -314,7 +216,7 @@ static struct platform_driver ath79_wdt_driver = {
>  module_platform_driver(ath79_wdt_driver);
>  
>  MODULE_DESCRIPTION("Atheros AR71XX/AR724X/AR913X hardware watchdog driver");
> -MODULE_AUTHOR("Gabor Juhos <juhosg@xxxxxxxxxxx");
> -MODULE_AUTHOR("Imre Kaloz <kaloz@xxxxxxxxxxx");
> +MODULE_AUTHOR("Gabor Juhos <juhosg@xxxxxxxxxxx>");
> +MODULE_AUTHOR("Imre Kaloz <kaloz@xxxxxxxxxxx>");
>  MODULE_LICENSE("GPL v2");
>  MODULE_ALIAS("platform:" DRIVER_NAME);