[PATCHv2 0/3] watchdog: sama5d4: fix IRQ and timeout bugs, use platform_get_irq_optional

[PATCHv2 0/3] watchdog: sama5d4: fix IRQ and timeout bugs, use platform_get_irq_optional

[Rosen Penev]

This series fixes three issues in the sama5d4 watchdog driver:

Patch 1: Return IRQ_NONE when the status register indicates no watchdog
interrupt is pending, and pass the configured timeout (not the hardcoded
default) to sama5d4_wdt_init().

Patch 2: Replace irq_of_parse_and_map() with platform_get_irq_optional()
for simpler error handling and EPROBE_DEFER support.

Patch 3: Move platform_set_drvdata() before devm_request_irq() to prevent
a NULL pointer dereference in the interrupt handler if the shared System
IRQ fires immediately after registration.

v2: address sashiko review

Rosen Penev (3):
  watchdog: sama5d4: fix shared IRQ and hardcoded timeout issues
  watchdog: sama5d4: use platform_get_irq_optional()
  watchdog: sama5d4: fix NULL deref in irq handler

 drivers/watchdog/sama5d4_wdt.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

--
2.54.0

[PATCHv2 1/3] watchdog: sama5d4: fix shared IRQ and hardcoded timeout issues

[Rosen Penev]

Fix three pre-existing issues in the sama5d4 watchdog driver:

1. Unsafe IRQF_SHARED | IRQF_NO_SUSPEND combination: The watchdog
   interrupt is a dedicated peripheral line, not shared with other
   devices.

2. Unconditional IRQ_HANDLED on shared line: The handler returned
   IRQ_HANDLED even when the status register indicated no watchdog
   interrupt was pending.  Return IRQ_NONE in that case so the kernel
   can properly detect spurious interrupts on the line.

3. Hardcoded 16-second timeout: sama5d4_wdt_init() unconditionally
   used WDT_DEFAULT_TIMEOUT (16s) for the hardware timeout, ignoring
   any timeout configured via device tree (watchdog_init_timeout) or
   userspace.  Pass wdd->timeout to sama5d4_wdt_init() so the
   configured timeout is honored during probe and resume.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/watchdog/sama5d4_wdt.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 704b786cc2ec..f74f1e8956b5 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -169,11 +169,12 @@ static irqreturn_t sama5d4_wdt_irq_handler(int irq, void *dev_id)
 	else
 		reg = wdt_read(wdt, AT91_WDT_SR);
 
-	if (reg) {
-		pr_crit("Atmel Watchdog Software Reset\n");
-		emergency_restart();
-		pr_crit("Reboot didn't succeed\n");
-	}
+	if (!reg)
+		return IRQ_NONE;
+
+	pr_crit("Atmel Watchdog Software Reset\n");
+	emergency_restart();
+	pr_crit("Reboot didn't succeed\n");
 
 	return IRQ_HANDLED;
 }
@@ -197,11 +198,11 @@ static int of_sama5d4_wdt_init(struct device_node *np, struct sama5d4_wdt *wdt)
 	return 0;
 }
 
-static int sama5d4_wdt_init(struct sama5d4_wdt *wdt)
+static int sama5d4_wdt_init(struct sama5d4_wdt *wdt, unsigned int timeout)
 {
 	u32 reg, val;
 
-	val = WDT_SEC2TICKS(WDT_DEFAULT_TIMEOUT);
+	val = WDT_SEC2TICKS(timeout);
 	/*
 	 * When booting and resuming, the bootloader may have changed the
 	 * watchdog configuration.
@@ -305,7 +306,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
 		set_bit(WDOG_HW_RUNNING, &wdd->status);
 	}
 
-	ret = sama5d4_wdt_init(wdt);
+	ret = sama5d4_wdt_init(wdt, wdd->timeout);
 	if (ret)
 		return ret;
 
@@ -358,7 +359,7 @@ static int sama5d4_wdt_resume_early(struct device *dev)
 	 * This should only be done when the registers are lost on suspend but
 	 * there is no way to get this information right now.
 	 */
-	sama5d4_wdt_init(wdt);
+	sama5d4_wdt_init(wdt, wdt->wdd.timeout);
 
 	if (watchdog_active(&wdt->wdd))
 		sama5d4_wdt_start(&wdt->wdd);
-- 
2.54.0

Re: [PATCHv2 1/3] watchdog: sama5d4: fix shared IRQ and hardcoded timeout issues

[Guenter Roeck]

On 6/8/26 13:09, Rosen Penev wrote:
> Fix three pre-existing issues in the sama5d4 watchdog driver:
> 
> 1. Unsafe IRQF_SHARED | IRQF_NO_SUSPEND combination: The watchdog
>     interrupt is a dedicated peripheral line, not shared with other
>     devices.
> 

Not addressed in this patch, and can not be addressed without testing
on real hardware. We'll have to leave that alone.

> 2. Unconditional IRQ_HANDLED on shared line: The handler returned
>     IRQ_HANDLED even when the status register indicated no watchdog
>     interrupt was pending.  Return IRQ_NONE in that case so the kernel
>     can properly detect spurious interrupts on the line.
> 
> 3. Hardcoded 16-second timeout: sama5d4_wdt_init() unconditionally
>     used WDT_DEFAULT_TIMEOUT (16s) for the hardware timeout, ignoring
>     any timeout configured via device tree (watchdog_init_timeout) or
>     userspace.  Pass wdd->timeout to sama5d4_wdt_init() so the
>     configured timeout is honored during probe and resume.
> 
While I think this is a real problem, changing the code would require
testing on real hardware. We'll have to leave this alone.

Please resubmit, only fixing issue 2). Please list the other problems as
explicitly not fixed in the summary e-mail.

This also applies to the new problem regarding AT91_WDT_WDDIS vs.
AT91_SAM9X60_WDDIS. If that is a real problem, it can only be fixed
by someone with access to hardware and with chip knowledge.

Besides that, I would suggest to re-order the patches.
Patch 3 (the null pointer fix) should be first, followed by this patch.
Patch 2 should be last.

Thanks,
Guenter

[PATCHv2 2/3] watchdog: sama5d4: use platform_get_irq_optional()

[Rosen Penev]

irq_of_parse_and_map() requires irq_dispose_mapping() on failure. Don't
bother with it as platform_get_irq_optional() doesn't need it.

Also handle EPROBE_DEFER.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/watchdog/sama5d4_wdt.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index f74f1e8956b5..2536e2f2ab32 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -11,7 +11,6 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/reboot.h>
 #include <linux/watchdog.h>
@@ -245,7 +244,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
 	struct watchdog_device *wdd;
 	struct sama5d4_wdt *wdt;
 	void __iomem *regs;
-	u32 irq = 0;
+	int irq = 0;
 	u32 reg;
 	int ret;
 
@@ -281,8 +280,11 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
 		return ret;
 
 	if (wdt->need_irq) {
-		irq = irq_of_parse_and_map(dev->of_node, 0);
-		if (!irq) {
+		irq = platform_get_irq_optional(pdev, 0);
+		if (irq == -EPROBE_DEFER)
+			return irq;
+
+		if (irq < 0) {
 			dev_warn(dev, "failed to get IRQ from DT\n");
 			wdt->need_irq = false;
 		}
-- 
2.54.0

Re: [PATCHv2 2/3] watchdog: sama5d4: use platform_get_irq_optional()

[Guenter Roeck]

On 6/8/26 13:09, Rosen Penev wrote:
> irq_of_parse_and_map() requires irq_dispose_mapping() on failure. Don't
> bother with it as platform_get_irq_optional() doesn't need it.
> 
> Also handle EPROBE_DEFER.
> 

That is misleading. irq_of_parse_and_map() did not return errors,
so this is not an "also" but made necessary by the use of
platform_get_irq_optional(). That means that replacing the
call with platform_get_irq_optional() is not a 1:1 replacement,
making its use risky without extensive testing.

A much safer solution is to add a call to devm_add_action_or_reset()
after successful irq_of_parse_and_map() and to call irq_dispose_mapping()
from its callback function.

Thanks,
Guenter

[PATCHv2 3/3] watchdog: sama5d4: fix NULL deref in irq handler

[Rosen Penev]

Move platform_set_drvdata() before devm_request_irq() so that the
interrupt handler can safely dereference the driver data via
platform_get_drvdata().  An interrupt on the shared System IRQ line
(SAM9X60/SAM9X7) could fire between request_irq and the late
platform_set_drvdata() call, causing the handler to dereference NULL.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/watchdog/sama5d4_wdt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 2536e2f2ab32..7cd6f35c8e21 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -268,6 +268,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
 						: AT91_WDT_WDDIS;
 
 	watchdog_set_drvdata(wdd, wdt);
+	platform_set_drvdata(pdev, wdt);
 
 	regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(regs))
@@ -319,8 +320,6 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	platform_set_drvdata(pdev, wdt);
-
 	dev_info(dev, "initialized (timeout = %d sec, nowayout = %d)\n",
 		 wdd->timeout, nowayout);
 
-- 
2.54.0

Re: [PATCHv2 0/3] watchdog: sama5d4: fix IRQ and timeout bugs, use platform_get_irq_optional

[Nicolas Ferre]

On 08/06/2026 at 13:09, Rosen Penev wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> This series fixes three issues in the sama5d4 watchdog driver:
> 
> Patch 1: Return IRQ_NONE when the status register indicates no watchdog
> interrupt is pending, and pass the configured timeout (not the hardcoded
> default) to sama5d4_wdt_init().
> 
> Patch 2: Replace irq_of_parse_and_map() with platform_get_irq_optional()
> for simpler error handling and EPROBE_DEFER support.
> 
> Patch 3: Move platform_set_drvdata() before devm_request_irq() to prevent
> a NULL pointer dereference in the interrupt handler if the shared System
> IRQ fires immediately after registration.
> 
> v2: address sashiko review

Rosen,

I'd like to hear how these issues where found and how the patches were 
tested.

Best regards,
   Nicolas

> Rosen Penev (3):
>    watchdog: sama5d4: fix shared IRQ and hardcoded timeout issues
>    watchdog: sama5d4: use platform_get_irq_optional()
>    watchdog: sama5d4: fix NULL deref in irq handler
> 
>   drivers/watchdog/sama5d4_wdt.c | 32 +++++++++++++++++---------------
>   1 file changed, 17 insertions(+), 15 deletions(-)
> 
> --
> 2.54.0
> 

Re: [PATCHv2 0/3] watchdog: sama5d4: fix IRQ and timeout bugs, use platform_get_irq_optional

[Rosen Penev]

On Mon, Jun 8, 2026 at 1:38 PM Nicolas Ferre
<nicolas.ferre@microchip.com> wrote:
>
> On 08/06/2026 at 13:09, Rosen Penev wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > This series fixes three issues in the sama5d4 watchdog driver:
> >
> > Patch 1: Return IRQ_NONE when the status register indicates no watchdog
> > interrupt is pending, and pass the configured timeout (not the hardcoded
> > default) to sama5d4_wdt_init().
> >
> > Patch 2: Replace irq_of_parse_and_map() with platform_get_irq_optional()
> > for simpler error handling and EPROBE_DEFER support.
> >
> > Patch 3: Move platform_set_drvdata() before devm_request_irq() to prevent
> > a NULL pointer dereference in the interrupt handler if the shared System
> > IRQ fires immediately after registration.
> >
> > v2: address sashiko review
>
> Rosen,
>
> I'd like to hear how these issues where found and how the patches were
> tested.
They were not. Maintainer has explicitly asked me to fix issues
discovered by https://sashiko.dev

I originally only had patch 2, which is true of multiple places.
>
> Best regards,
>    Nicolas
>
> > Rosen Penev (3):
> >    watchdog: sama5d4: fix shared IRQ and hardcoded timeout issues
> >    watchdog: sama5d4: use platform_get_irq_optional()
> >    watchdog: sama5d4: fix NULL deref in irq handler
> >
> >   drivers/watchdog/sama5d4_wdt.c | 32 +++++++++++++++++---------------
> >   1 file changed, 17 insertions(+), 15 deletions(-)
> >
> > --
> > 2.54.0
> >
>