Linux Watchdog driver development
 help / color / mirror / Atom feed
* [PATCH] watchdog: orion: Use of_device_get_match_data()
@ 2026-05-19  0:37 Rosen Penev
  2026-05-19  0:52 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Rosen Penev @ 2026-05-19  0:37 UTC (permalink / raw)
  To: linux-watchdog; +Cc: Wim Van Sebroeck, Guenter Roeck, open list

Use of_device_get_match_data() to fetch watchdog match data directly
instead of open-coding an of_match_device() lookup.

Keep the existing legacy fallback when no match data is available.

This also lets the driver drop the of_device.h include.

Assisted-by: Codex:GPT-5.5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/watchdog/orion_wdt.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 0e145f762f6f..a92701ff2653 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -21,7 +21,6 @@
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/of.h>
-#include <linux/of_device.h>

 /* RSTOUT mask register physical address for Orion5x, Kirkwood and Dove */
 #define ORION_RSTOUT_MASK_OFFSET	0x20108
@@ -554,7 +553,6 @@ static int orion_wdt_get_regs(struct platform_device *pdev,
 static int orion_wdt_probe(struct platform_device *pdev)
 {
 	struct orion_watchdog *dev;
-	const struct of_device_id *match;
 	unsigned int wdt_max_duration;	/* (seconds) */
 	int ret, irq;

@@ -563,15 +561,14 @@ static int orion_wdt_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;

-	match = of_match_device(orion_wdt_of_match_table, &pdev->dev);
-	if (!match)
+	dev->data = of_device_get_match_data(&pdev->dev);
+	if (!dev->data)
 		/* Default legacy match */
-		match = &orion_wdt_of_match_table[0];
+		dev->data = orion_wdt_of_match_table[0].data;

 	dev->wdt.info = &orion_wdt_info;
 	dev->wdt.ops = &orion_wdt_ops;
 	dev->wdt.min_timeout = 1;
-	dev->data = match->data;

 	ret = orion_wdt_get_regs(pdev, dev);
 	if (ret)
--
2.54.0


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

* Re: [PATCH] watchdog: orion: Use of_device_get_match_data()
  2026-05-19  0:37 [PATCH] watchdog: orion: Use of_device_get_match_data() Rosen Penev
@ 2026-05-19  0:52 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-05-19  0:52 UTC (permalink / raw)
  To: Rosen Penev; +Cc: linux-watchdog

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The clock is explicitly disabled before the `devm`-managed interrupt is freed, leading to a potential bus fault or system hang. This problem wasn't introduced by this patch, but it is a pre-existing high-severity bug.
--

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

watchdog: orion: Use of_device_get_match_data()

Use of_device_get_match_data() to fetch watchdog match data directly
instead of open-coding an of_match_device() lookup.

> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 0e145f762f6f2..a92701ff26530 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
[ ... ]
> @@ -563,15 +561,14 @@ static int orion_wdt_probe(struct platform_device *pdev)
>  	if (!dev)
>  		return -ENOMEM;
>  
> -	match = of_match_device(orion_wdt_of_match_table, &pdev->dev);
> -	if (!match)
> +	dev->data = of_device_get_match_data(&pdev->dev);
> +	if (!dev->data)
>  		/* Default legacy match */
> -		match = &orion_wdt_of_match_table[0];
> +		dev->data = orion_wdt_of_match_table[0].data;
>  
>  	dev->wdt.info = &orion_wdt_info;
>  	dev->wdt.ops = &orion_wdt_ops;
>  	dev->wdt.min_timeout = 1;
> -	dev->data = match->data;
>  
>  	ret = orion_wdt_get_regs(pdev, dev);
>  	if (ret)

While reviewing this patch, I noticed a pre-existing issue in the probe
error path.

If watchdog_register_device() fails later in orion_wdt_probe(), the code
jumps to the disable_clk label:

        ret = watchdog_register_device(&dev->wdt);
        if (ret)
                goto disable_clk;

[ ... ]

disable_clk:
        clk_disable_unprepare(dev->clk);
        clk_put(dev->clk);
        return ret;

Since devm_request_irq() is used to allocate the interrupts, the devres
core will automatically free them only after the probe function returns.

Does this create a teardown window where the clock is explicitly disabled
but the interrupts remain active?

If a watchdog pre-timeout interrupt fires during this window, could the
handlers (orion_wdt_pre_irq or orion_wdt_irq) attempt to access
memory-mapped hardware registers while the clock is turned off, resulting
in a bus fault or system hang?

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

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

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

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-19  0:37 [PATCH] watchdog: orion: Use of_device_get_match_data() Rosen Penev
2026-05-19  0:52 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox