All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] watchdog: ath79_wdt: select OF and COMPILE_TEST
@ 2026-05-18 21:09 Rosen Penev
  2026-05-18 21:19 ` sashiko-bot
  2026-05-19 18:08 ` kernel test robot
  0 siblings, 2 replies; 4+ messages in thread
From: Rosen Penev @ 2026-05-18 21:09 UTC (permalink / raw)
  To: linux-watchdog; +Cc: Wim Van Sebroeck, Guenter Roeck, open list

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] watchdog: ath79_wdt: select OF and COMPILE_TEST
  2026-05-18 21:09 [PATCH] watchdog: ath79_wdt: select OF and COMPILE_TEST Rosen Penev
@ 2026-05-18 21:19 ` sashiko-bot
  2026-05-18 21:38   ` Rosen Penev
  2026-05-19 18:08 ` kernel test robot
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-05-18 21:19 UTC (permalink / raw)
  To: Rosen Penev; +Cc: linux-watchdog

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Permanent failure of deferred probing and device re-binding due to uncleared global `wdt_base` pointer on error paths.
- [Medium] Mismatch between Kconfig changes and commit message regarding OF dependency.
--

commit 279311df3b831cb8a967974beeea1a51baecd0ac
Author: Rosen Penev <rosenp@gmail.com>

watchdog: ath79_wdt: select OF and COMPILE_TEST

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

Add COMPILE_TEST for all platforms, not just ARM.

> watchdog: ath79_wdt: select OF and COMPILE_TEST
> 
> This has always been an OF driver. Select OF in Kconfig.

There is a discrepancy between the commit message and the implemented code.
The commit message states it will "select OF", but the patch implements
"depends on OF". Could the commit message be updated to match the
implementation?

> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index dc78729ba2a5d..1590f82941ba8 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1811,7 +1811,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

[ ... ]

While reviewing this patch, I noticed an existing issue in the probe
function's error handling. The driver uses a global pointer to enforce a
singleton instance:

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);

	wdt_clk = devm_clk_get_enabled(&pdev->dev, "wdt");
	if (IS_ERR(wdt_clk))
		return PTR_ERR(wdt_clk);
    ...
}

If devm_platform_ioremap_resource() returns an error pointer, or if a
later step like devm_clk_get_enabled() returns -EPROBE_DEFER, the global
wdt_base pointer is never cleared.

Does this permanently break deferred probing and device re-binding? On a
second probe attempt, the uncleared wdt_base pointer would still evaluate
to true, causing the probe to immediately fail with -EBUSY.

Similarly, since ath79_wdt_remove() does not clear wdt_base, will unloading
and reloading the driver also fail?

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] watchdog: ath79_wdt: select OF and COMPILE_TEST
  2026-05-18 21:19 ` sashiko-bot
@ 2026-05-18 21:38   ` Rosen Penev
  0 siblings, 0 replies; 4+ messages in thread
From: Rosen Penev @ 2026-05-18 21:38 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-watchdog

On Mon, May 18, 2026 at 2:19 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Permanent failure of deferred probing and device re-binding due to uncleared global `wdt_base` pointer on error paths.
> - [Medium] Mismatch between Kconfig changes and commit message regarding OF dependency.
> --
>
> commit 279311df3b831cb8a967974beeea1a51baecd0ac
> Author: Rosen Penev <rosenp@gmail.com>
>
> watchdog: ath79_wdt: select OF and COMPILE_TEST
>
> This has always been an OF driver. Select OF in Kconfig.
>
> Add COMPILE_TEST for all platforms, not just ARM.
>
> > watchdog: ath79_wdt: select OF and COMPILE_TEST
> >
> > This has always been an OF driver. Select OF in Kconfig.
>
> There is a discrepancy between the commit message and the implemented code.
> The commit message states it will "select OF", but the patch implements
> "depends on OF". Could the commit message be updated to match the
> implementation?
Yeah probably.
>
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index dc78729ba2a5d..1590f82941ba8 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -1811,7 +1811,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
>
> [ ... ]
>
> While reviewing this patch, I noticed an existing issue in the probe
> function's error handling. The driver uses a global pointer to enforce a
> singleton instance:
Yes, this is an ancient driver that needs to be migrated to
watchdog_device. This is not relevant to COMPILE_TEST though.
>
> 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);
>
>         wdt_clk = devm_clk_get_enabled(&pdev->dev, "wdt");
>         if (IS_ERR(wdt_clk))
>                 return PTR_ERR(wdt_clk);
>     ...
> }
>
> If devm_platform_ioremap_resource() returns an error pointer, or if a
> later step like devm_clk_get_enabled() returns -EPROBE_DEFER, the global
> wdt_base pointer is never cleared.
>
> Does this permanently break deferred probing and device re-binding? On a
> second probe attempt, the uncleared wdt_base pointer would still evaluate
> to true, causing the probe to immediately fail with -EBUSY.
>
> Similarly, since ath79_wdt_remove() does not clear wdt_base, will unloading
> and reloading the driver also fail?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260518210957.10545-1-rosenp@gmail.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] watchdog: ath79_wdt: select OF and COMPILE_TEST
  2026-05-18 21:09 [PATCH] watchdog: ath79_wdt: select OF and COMPILE_TEST Rosen Penev
  2026-05-18 21:19 ` sashiko-bot
@ 2026-05-19 18:08 ` kernel test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2026-05-19 18:08 UTC (permalink / raw)
  To: Rosen Penev, linux-watchdog
  Cc: oe-kbuild-all, Wim Van Sebroeck, Guenter Roeck, linux-kernel

Hi Rosen,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon]
[also build test WARNING on groeck-staging/hwmon-next groeck-staging/watchdog groeck-staging/watchdog-next linus/master v7.1-rc4 next-20260518]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rosen-Penev/watchdog-ath79_wdt-select-OF-and-COMPILE_TEST/20260519-051137
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon
patch link:    https://lore.kernel.org/r/20260518210957.10545-1-rosenp%40gmail.com
patch subject: [PATCH] watchdog: ath79_wdt: select OF and COMPILE_TEST
config: riscv-randconfig-r122-20260519 (https://download.01.org/0day-ci/archive/20260520/202605200244.R8jDfTUk-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 8.5.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260520/202605200244.R8jDfTUk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202605200244.R8jDfTUk-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/watchdog/ath79_wdt.c:161:37: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected char const [noderef] __user *__p @@     got char const * @@
   drivers/watchdog/ath79_wdt.c:161:37: sparse:     expected char const [noderef] __user *__p
   drivers/watchdog/ath79_wdt.c:161:37: sparse:     got char const *
>> drivers/watchdog/ath79_wdt.c:234:27: sparse: sparse: incorrect type in initializer (incompatible argument 2 (different address spaces)) @@     expected long ( *write )( ... ) @@     got long ( * )( ... ) @@
   drivers/watchdog/ath79_wdt.c:234:27: sparse:     expected long ( *write )( ... )
   drivers/watchdog/ath79_wdt.c:234:27: sparse:     got long ( * )( ... )

vim +161 drivers/watchdog/ath79_wdt.c

f8394f61c66f48 Gabor Juhos         2011-01-04  148  
f8394f61c66f48 Gabor Juhos         2011-01-04  149  static ssize_t ath79_wdt_write(struct file *file, const char *data,
f8394f61c66f48 Gabor Juhos         2011-01-04  150  				size_t len, loff_t *ppos)
f8394f61c66f48 Gabor Juhos         2011-01-04  151  {
f8394f61c66f48 Gabor Juhos         2011-01-04  152  	if (len) {
f8394f61c66f48 Gabor Juhos         2011-01-04  153  		if (!nowayout) {
f8394f61c66f48 Gabor Juhos         2011-01-04  154  			size_t i;
f8394f61c66f48 Gabor Juhos         2011-01-04  155  
f8394f61c66f48 Gabor Juhos         2011-01-04  156  			clear_bit(WDT_FLAGS_EXPECT_CLOSE, &wdt_flags);
f8394f61c66f48 Gabor Juhos         2011-01-04  157  
f8394f61c66f48 Gabor Juhos         2011-01-04  158  			for (i = 0; i != len; i++) {
f8394f61c66f48 Gabor Juhos         2011-01-04  159  				char c;
f8394f61c66f48 Gabor Juhos         2011-01-04  160  
f8394f61c66f48 Gabor Juhos         2011-01-04 @161  				if (get_user(c, data + i))
f8394f61c66f48 Gabor Juhos         2011-01-04  162  					return -EFAULT;
f8394f61c66f48 Gabor Juhos         2011-01-04  163  
f8394f61c66f48 Gabor Juhos         2011-01-04  164  				if (c == 'V')
f8394f61c66f48 Gabor Juhos         2011-01-04  165  					set_bit(WDT_FLAGS_EXPECT_CLOSE,
f8394f61c66f48 Gabor Juhos         2011-01-04  166  						&wdt_flags);
f8394f61c66f48 Gabor Juhos         2011-01-04  167  			}
f8394f61c66f48 Gabor Juhos         2011-01-04  168  		}
f8394f61c66f48 Gabor Juhos         2011-01-04  169  
f8394f61c66f48 Gabor Juhos         2011-01-04  170  		ath79_wdt_keepalive();
f8394f61c66f48 Gabor Juhos         2011-01-04  171  	}
f8394f61c66f48 Gabor Juhos         2011-01-04  172  
f8394f61c66f48 Gabor Juhos         2011-01-04  173  	return len;
f8394f61c66f48 Gabor Juhos         2011-01-04  174  }
f8394f61c66f48 Gabor Juhos         2011-01-04  175  
f8394f61c66f48 Gabor Juhos         2011-01-04  176  static const struct watchdog_info ath79_wdt_info = {
f8394f61c66f48 Gabor Juhos         2011-01-04  177  	.options		= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
f8394f61c66f48 Gabor Juhos         2011-01-04  178  				  WDIOF_MAGICCLOSE | WDIOF_CARDRESET,
f8394f61c66f48 Gabor Juhos         2011-01-04  179  	.firmware_version	= 0,
f8394f61c66f48 Gabor Juhos         2011-01-04  180  	.identity		= "ATH79 watchdog",
f8394f61c66f48 Gabor Juhos         2011-01-04  181  };
f8394f61c66f48 Gabor Juhos         2011-01-04  182  
f8394f61c66f48 Gabor Juhos         2011-01-04  183  static long ath79_wdt_ioctl(struct file *file, unsigned int cmd,
f8394f61c66f48 Gabor Juhos         2011-01-04  184  			    unsigned long arg)
f8394f61c66f48 Gabor Juhos         2011-01-04  185  {
f8394f61c66f48 Gabor Juhos         2011-01-04  186  	void __user *argp = (void __user *)arg;
f8394f61c66f48 Gabor Juhos         2011-01-04  187  	int __user *p = argp;
f8394f61c66f48 Gabor Juhos         2011-01-04  188  	int err;
f8394f61c66f48 Gabor Juhos         2011-01-04  189  	int t;
f8394f61c66f48 Gabor Juhos         2011-01-04  190  
f8394f61c66f48 Gabor Juhos         2011-01-04  191  	switch (cmd) {
f8394f61c66f48 Gabor Juhos         2011-01-04  192  	case WDIOC_GETSUPPORT:
f8394f61c66f48 Gabor Juhos         2011-01-04  193  		err = copy_to_user(argp, &ath79_wdt_info,
f8394f61c66f48 Gabor Juhos         2011-01-04  194  				   sizeof(ath79_wdt_info)) ? -EFAULT : 0;
f8394f61c66f48 Gabor Juhos         2011-01-04  195  		break;
f8394f61c66f48 Gabor Juhos         2011-01-04  196  
f8394f61c66f48 Gabor Juhos         2011-01-04  197  	case WDIOC_GETSTATUS:
f8394f61c66f48 Gabor Juhos         2011-01-04  198  		err = put_user(0, p);
f8394f61c66f48 Gabor Juhos         2011-01-04  199  		break;
f8394f61c66f48 Gabor Juhos         2011-01-04  200  
f8394f61c66f48 Gabor Juhos         2011-01-04  201  	case WDIOC_GETBOOTSTATUS:
f8394f61c66f48 Gabor Juhos         2011-01-04  202  		err = put_user(boot_status, p);
f8394f61c66f48 Gabor Juhos         2011-01-04  203  		break;
f8394f61c66f48 Gabor Juhos         2011-01-04  204  
f8394f61c66f48 Gabor Juhos         2011-01-04  205  	case WDIOC_KEEPALIVE:
f8394f61c66f48 Gabor Juhos         2011-01-04  206  		ath79_wdt_keepalive();
f8394f61c66f48 Gabor Juhos         2011-01-04  207  		err = 0;
f8394f61c66f48 Gabor Juhos         2011-01-04  208  		break;
f8394f61c66f48 Gabor Juhos         2011-01-04  209  
f8394f61c66f48 Gabor Juhos         2011-01-04  210  	case WDIOC_SETTIMEOUT:
f8394f61c66f48 Gabor Juhos         2011-01-04  211  		err = get_user(t, p);
f8394f61c66f48 Gabor Juhos         2011-01-04  212  		if (err)
f8394f61c66f48 Gabor Juhos         2011-01-04  213  			break;
f8394f61c66f48 Gabor Juhos         2011-01-04  214  
f8394f61c66f48 Gabor Juhos         2011-01-04  215  		err = ath79_wdt_set_timeout(t);
f8394f61c66f48 Gabor Juhos         2011-01-04  216  		if (err)
f8394f61c66f48 Gabor Juhos         2011-01-04  217  			break;
bd490f8222510d Gustavo A. R. Silva 2020-07-07  218  		fallthrough;
f8394f61c66f48 Gabor Juhos         2011-01-04  219  
f8394f61c66f48 Gabor Juhos         2011-01-04  220  	case WDIOC_GETTIMEOUT:
f8394f61c66f48 Gabor Juhos         2011-01-04  221  		err = put_user(timeout, p);
f8394f61c66f48 Gabor Juhos         2011-01-04  222  		break;
f8394f61c66f48 Gabor Juhos         2011-01-04  223  
f8394f61c66f48 Gabor Juhos         2011-01-04  224  	default:
f8394f61c66f48 Gabor Juhos         2011-01-04  225  		err = -ENOTTY;
f8394f61c66f48 Gabor Juhos         2011-01-04  226  		break;
f8394f61c66f48 Gabor Juhos         2011-01-04  227  	}
f8394f61c66f48 Gabor Juhos         2011-01-04  228  
f8394f61c66f48 Gabor Juhos         2011-01-04  229  	return err;
f8394f61c66f48 Gabor Juhos         2011-01-04  230  }
f8394f61c66f48 Gabor Juhos         2011-01-04  231  
f8394f61c66f48 Gabor Juhos         2011-01-04  232  static const struct file_operations ath79_wdt_fops = {
f8394f61c66f48 Gabor Juhos         2011-01-04  233  	.owner		= THIS_MODULE,
f8394f61c66f48 Gabor Juhos         2011-01-04 @234  	.write		= ath79_wdt_write,
f8394f61c66f48 Gabor Juhos         2011-01-04  235  	.unlocked_ioctl	= ath79_wdt_ioctl,
b6dfb2477fb0bf Arnd Bergmann       2019-06-03  236  	.compat_ioctl	= compat_ptr_ioctl,
f8394f61c66f48 Gabor Juhos         2011-01-04  237  	.open		= ath79_wdt_open,
f8394f61c66f48 Gabor Juhos         2011-01-04  238  	.release	= ath79_wdt_release,
f8394f61c66f48 Gabor Juhos         2011-01-04  239  };
f8394f61c66f48 Gabor Juhos         2011-01-04  240  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-19 18:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-18 21:09 [PATCH] watchdog: ath79_wdt: select OF and COMPILE_TEST Rosen Penev
2026-05-18 21:19 ` sashiko-bot
2026-05-18 21:38   ` Rosen Penev
2026-05-19 18:08 ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.