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

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

[Rosen Penev]

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.

v2: addressed review comments

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 | 250 +++++++++++------------------------
 2 files changed, 77 insertions(+), 176 deletions(-)

--
2.54.0

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

[Rosen Penev]

This has always been an OF driver. Depend on 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

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

[Rosen Penev]

Convert 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 | 246 +++++++++++------------------------
 1 file changed, 74 insertions(+), 172 deletions(-)

diff --git a/drivers/watchdog/ath79_wdt.c b/drivers/watchdog/ath79_wdt.c
index 409a40b14901..90c7e2578018 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",
-};
-
-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,
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_CARDRESET | WDIOF_MAGICCLOSE,
+	.firmware_version = 0,
+	.identity = "ATH79 watchdog",
 };

-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(wdt, WDOG_REG_CTRL);
+	wdt->wdd.bootstatus = (ctrl & WDOG_CTRL_LAST_RESET) ? WDIOF_CARDRESET : 0;

-	ctrl = ath79_wdt_rr(WDOG_REG_CTRL);
-	boot_status = (ctrl & WDOG_CTRL_LAST_RESET) ? WDIOF_CARDRESET : 0;
+	wdt->wdd.parent = &pdev->dev;
+	wdt->wdd.info = &ath79_wdt_info;
+	wdt->wdd.ops = &ath79_wdt_ops;
+	wdt->wdd.min_timeout = 1;
+	wdt->wdd.timeout = WDT_TIMEOUT;
+	wdt->wdd.max_timeout = U32_MAX / wdt->freq;
+	watchdog_init_timeout(&wdt->wdd, timeout, &pdev->dev);
+	watchdog_set_nowayout(&wdt->wdd, nowayout);
+	watchdog_stop_on_reboot(&wdt->wdd);

-	err = misc_register(&ath79_wdt_miscdev);
-	if (err) {
-		dev_err(&pdev->dev,
-			"unable to register misc device, err=%d\n", err);
-		return err;
-	}
+	watchdog_set_drvdata(&wdt->wdd, wdt);

-	return 0;
-}
-
-static void ath79_wdt_remove(struct platform_device *pdev)
-{
-	misc_deregister(&ath79_wdt_miscdev);
-}
-
-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: [PATCHv2 1/2] watchdog: ath79_wdt: select OF and COMPILE_TEST

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The global pointer `wdt_base` is assigned during probe but is never cleared on error paths or during driver removal, breaking deferred probing and driver rebinding.
- [High] The driver fails to prevent concurrent access from open miscdevice file descriptors after the device is unbound, leading to a use-after-free or page fault.
- [High] The watchdog hardware is not disabled during driver removal before its clock is disabled, potentially leading to uncontrolled system resets or hardware locks.
--

commit 71b88482365f22ded9cd7ae662eb7b028ce262ea
Author: Rosen Penev <rosenp@gmail.com>

watchdog: ath79_wdt: select OF and COMPILE_TEST

This commit makes the driver depend on OF in Kconfig, as it is inherently
an OF driver. It also adds COMPILE_TEST support for all platforms instead
of just ARM.

> diff --git a/drivers/watchdog/ath79_wdt.c b/drivers/watchdog/ath79_wdt.c
> index 7df703e9852a6..409a40b149015 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

[Severity: High]
This is a pre-existing issue, but does the driver properly clear the global
wdt_base pointer on error paths and during removal?

The wdt_base pointer is assigned during probe, but if a subsequent
step like devm_clk_get_enabled() or misc_register() fails, wdt_base is
never cleared.

When the driver core later retries the probe, the initial check evaluates
to true, causing the probe to indefinitely return -EBUSY:

drivers/watchdog/ath79_wdt.c:ath79_wdt_probe() {
    ...
	if (wdt_base)
		return -EBUSY;

	wdt_base = devm_platform_ioremap_resource(pdev, 0);
	if (IS_ERR(wdt_base))
		return PTR_ERR(wdt_base);
    ...
}

Similarly, ath79_wdt_remove() does not clear wdt_base when the device is
unbound. Because devres automatically unmaps the memory, wdt_base is left
as a dangling pointer, preventing future unbind/rebind operations.

Should wdt_base be cleared on error paths and in the remove function?

[Severity: High]
This is a pre-existing issue, but does ath79_wdt_remove() prevent concurrent
access from open miscdevice file descriptors after the device is unbound?

The driver uses devm_platform_ioremap_resource(), which automatically unmaps
wdt_base when the device is removed. However, misc_deregister() does not
revoke existing open file descriptors.

If a user space application keeps /dev/watchdog open and calls write,
ioctl, or close after the driver is unbound, functions like
ath79_wdt_release() or ath79_wdt_write() will dereference the unmapped
wdt_base pointer.

[Severity: High]
This is a pre-existing issue, but is the watchdog hardware safely disabled
during removal?

The watchdog hardware is not explicitly disabled before its clock is
disabled. Because wdt_clk is acquired using devm_clk_get_enabled(), the
device manager will automatically disable the clock immediately after the
remove function completes.

Disabling the clock while the watchdog timer is still running leaves the
hardware in an active but unclocked state.

drivers/watchdog/ath79_wdt.c:ath79_wdt_remove() {
	misc_deregister(&ath79_wdt_miscdev);
}

Should the timer be stopped explicitly before the clock is disabled?

> @@ -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,
>  	},
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524015042.175271-2-rosenp@gmail.com?part=1

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

[Rosen Penev]

On Sat, May 23, 2026 at 7:11 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The global pointer `wdt_base` is assigned during probe but is never cleared on error paths or during driver removal, breaking deferred probing and driver rebinding.
> - [High] The driver fails to prevent concurrent access from open miscdevice file descriptors after the device is unbound, leading to a use-after-free or page fault.
> - [High] The watchdog hardware is not disabled during driver removal before its clock is disabled, potentially leading to uncontrolled system resets or hardware locks.
I am getting fed up with this garbage.
> --
>
> commit 71b88482365f22ded9cd7ae662eb7b028ce262ea
> Author: Rosen Penev <rosenp@gmail.com>
>
> watchdog: ath79_wdt: select OF and COMPILE_TEST
>
> This commit makes the driver depend on OF in Kconfig, as it is inherently
> an OF driver. It also adds COMPILE_TEST support for all platforms instead
> of just ARM.
>
> > diff --git a/drivers/watchdog/ath79_wdt.c b/drivers/watchdog/ath79_wdt.c
> > index 7df703e9852a6..409a40b149015 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
>
> [Severity: High]
> This is a pre-existing issue, but does the driver properly clear the global
> wdt_base pointer on error paths and during removal?
>
> The wdt_base pointer is assigned during probe, but if a subsequent
> step like devm_clk_get_enabled() or misc_register() fails, wdt_base is
> never cleared.
>
> When the driver core later retries the probe, the initial check evaluates
> to true, causing the probe to indefinitely return -EBUSY:
>
> drivers/watchdog/ath79_wdt.c:ath79_wdt_probe() {
>     ...
>         if (wdt_base)
>                 return -EBUSY;
>
>         wdt_base = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(wdt_base))
>                 return PTR_ERR(wdt_base);
>     ...
> }
>
> Similarly, ath79_wdt_remove() does not clear wdt_base when the device is
> unbound. Because devres automatically unmaps the memory, wdt_base is left
> as a dangling pointer, preventing future unbind/rebind operations.
>
> Should wdt_base be cleared on error paths and in the remove function?
>
> [Severity: High]
> This is a pre-existing issue, but does ath79_wdt_remove() prevent concurrent
> access from open miscdevice file descriptors after the device is unbound?
>
> The driver uses devm_platform_ioremap_resource(), which automatically unmaps
> wdt_base when the device is removed. However, misc_deregister() does not
> revoke existing open file descriptors.
>
> If a user space application keeps /dev/watchdog open and calls write,
> ioctl, or close after the driver is unbound, functions like
> ath79_wdt_release() or ath79_wdt_write() will dereference the unmapped
> wdt_base pointer.
>
> [Severity: High]
> This is a pre-existing issue, but is the watchdog hardware safely disabled
> during removal?
>
> The watchdog hardware is not explicitly disabled before its clock is
> disabled. Because wdt_clk is acquired using devm_clk_get_enabled(), the
> device manager will automatically disable the clock immediately after the
> remove function completes.
>
> Disabling the clock while the watchdog timer is still running leaves the
> hardware in an active but unclocked state.
>
> drivers/watchdog/ath79_wdt.c:ath79_wdt_remove() {
>         misc_deregister(&ath79_wdt_miscdev);
> }
>
> Should the timer be stopped explicitly before the clock is disabled?
>
> > @@ -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,
> >       },
> >  };
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260524015042.175271-2-rosenp@gmail.com?part=1

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

[Guenter Roeck]

On 5/23/26 19:19, Rosen Penev wrote:
> On Sat, May 23, 2026 at 7:11 PM <sashiko-bot@kernel.org> wrote:
>>
>> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] The global pointer `wdt_base` is assigned during probe but is never cleared on error paths or during driver removal, breaking deferred probing and driver rebinding.
>> - [High] The driver fails to prevent concurrent access from open miscdevice file descriptors after the device is unbound, leading to a use-after-free or page fault.
>> - [High] The watchdog hardware is not disabled during driver removal before its clock is disabled, potentially leading to uncontrolled system resets or hardware locks.
> I am getting fed up with this garbage.

I appear to be missing something. Which one of the three reported issues
is garbage ?

If you are really concerned about garbage, I would suggest to
- Only submit patches which fix real issues.
- Do not submit minor patch fixes for a driver to fix one minor issue
   while leaving all the major problems in place, even though they are
   known.
- If the AI feedback really produces false positives, help doing something
   about it instead of declaring it all garbage because some of its findings
   are wrong. After all, you don't declare a human review as garbage because
   it also contains false positives, or because it doesn't find all problems
   in the code, or do you ?

I am not going to apply this patch. It has little if any value and does not
fix major problems in the code.

Guenter