[PATCH 0/2] watchdog: ath79: modernize and broaden build coverage

[PATCH 0/2] watchdog: ath79: modernize and broaden build coverage

[Rosen Penev]

Two cleanups for the Atheros AR71XX/AR724X/AR913X watchdog driver.

Patch 1 splits OF out as a hard dependency (the driver has always been
OF-only) and broadens COMPILE_TEST coverage from ARM to all
architectures, so the driver can be built on non-MIPS targets.

Patch 2 moves the driver off the legacy miscdevice/file_operations
interface to the watchdog_device framework.  This drops ~100 lines of
hand-rolled ioctl, open/release, and busy/expect-close bookkeeping in
favor of framework callbacks, fixes get_timeleft() to return seconds
instead of raw clock ticks, and properly propagates -EPROBE_DEFER from
clk_get().  nowayout, the timeout module parameter, and module .owner
are wired through the standard helpers.

Tested on an Archer C7v2; sysupgrade watchdog handoff still works.

Rosen Penev (2):
  watchdog: ath79_wdt: select OF and COMPILE_TEST
  watchdog: ath79: convert to watchdog_device

 drivers/watchdog/Kconfig     |   3 +-
 drivers/watchdog/ath79_wdt.c | 248 +++++++++++------------------------
 2 files changed, 76 insertions(+), 175 deletions(-)

-- 
2.54.0

[PATCH 1/2] watchdog: ath79_wdt: select OF and COMPILE_TEST

[Rosen Penev]

This has always been an OF driver. Select OF in Kconfig.

Add COMPILE_TEST for all platforms, not just ARM.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/watchdog/Kconfig     | 3 ++-
 drivers/watchdog/ath79_wdt.c | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 08cb8612d41f..8319c503319a 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1785,7 +1785,8 @@ config M54xx_WATCHDOG
 
 config ATH79_WDT
 	tristate "Atheros AR71XX/AR724X/AR913X hardware watchdog"
-	depends on ATH79 || (ARM && COMPILE_TEST)
+	depends on ATH79 || COMPILE_TEST
+	depends on OF
 	help
 	  Hardware driver for the built-in watchdog timer on the Atheros
 	  AR71XX/AR724X/AR913X SoCs.
diff --git a/drivers/watchdog/ath79_wdt.c b/drivers/watchdog/ath79_wdt.c
index 7df703e9852a..409a40b14901 100644
--- a/drivers/watchdog/ath79_wdt.c
+++ b/drivers/watchdog/ath79_wdt.c
@@ -295,13 +295,11 @@ static void ath79_wdt_shutdown(struct platform_device *pdev)
 	ath79_wdt_disable();
 }
 
-#ifdef CONFIG_OF
 static const struct of_device_id ath79_wdt_match[] = {
 	{ .compatible = "qca,ar7130-wdt" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, ath79_wdt_match);
-#endif
 
 static struct platform_driver ath79_wdt_driver = {
 	.probe		= ath79_wdt_probe,
@@ -309,7 +307,7 @@ static struct platform_driver ath79_wdt_driver = {
 	.shutdown	= ath79_wdt_shutdown,
 	.driver		= {
 		.name	= DRIVER_NAME,
-		.of_match_table = of_match_ptr(ath79_wdt_match),
+		.of_match_table = ath79_wdt_match,
 	},
 };
 
-- 
2.54.0

[PATCH 2/2] watchdog: ath79: convert to watchdog_device

[Rosen Penev]

Converts from the legacy miscdevice/file_operations interface to the
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.

Assisted-by: Claude:Sonnet-4.6
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 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..eeb7c33377fb 100644
--- a/drivers/watchdog/ath79_wdt.c
+++ b/drivers/watchdog/ath79_wdt.c
@@ -15,13 +15,10 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#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 int 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 int 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, unsigned int timeout)
 {
-	ath79_wdt_wr(WDOG_REG_TIMER, wdt_freq * timeout);
+	ath79_wdt_wr(wdt, WDOG_REG_TIMER, wdt->freq * 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, unsigned int timeout)
 {
-	ath79_wdt_keepalive();
+	ath79_wdt_keepalive(wdt, timeout);
 
 	/*
 	 * 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, wdd->timeout);
 
 	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, val);
 
-	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, wdd->timeout);
 
 	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,
+	.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;
+	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@openwrt.org");
-MODULE_AUTHOR("Imre Kaloz <kaloz@openwrt.org");
+MODULE_AUTHOR("Gabor Juhos <juhosg@openwrt.org>");
+MODULE_AUTHOR("Imre Kaloz <kaloz@openwrt.org>");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
2.54.0

Re: [PATCH 0/2] watchdog: ath79: modernize and broaden build coverage

[Guenter Roeck]

On 5/22/26 16:25, Rosen Penev wrote:
> Two cleanups for the Atheros AR71XX/AR724X/AR913X watchdog driver.
> 
> Patch 1 splits OF out as a hard dependency (the driver has always been
> OF-only) and broadens COMPILE_TEST coverage from ARM to all
> architectures, so the driver can be built on non-MIPS targets.
> 
> Patch 2 moves the driver off the legacy miscdevice/file_operations
> interface to the watchdog_device framework.  This drops ~100 lines of
> hand-rolled ioctl, open/release, and busy/expect-close bookkeeping in
> favor of framework callbacks, fixes get_timeleft() to return seconds
> instead of raw clock ticks, and properly propagates -EPROBE_DEFER from
> clk_get().  nowayout, the timeout module parameter, and module .owner
> are wired through the standard helpers.
> 
> Tested on an Archer C7v2; sysupgrade watchdog handoff still works.
> 
> Rosen Penev (2):
>    watchdog: ath79_wdt: select OF and COMPILE_TEST
>    watchdog: ath79: convert to watchdog_device
> 
>   drivers/watchdog/Kconfig     |   3 +-
>   drivers/watchdog/ath79_wdt.c | 248 +++++++++++------------------------
>   2 files changed, 76 insertions(+), 175 deletions(-)
> 

Is this a resend, ignoring the code review feedback, or v2 without
v2 tag and no change log ?

Since there is neither a v2 tag nor a change log, I assume it is a resend.
I am going to ignore it, assuming that the code review comments have not
been addressed.

Guenter