Linux Watchdog driver development
 help / color / mirror / Atom feed
* [PATCH 1/2] watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup
@ 2026-08-06  6:28 phucduc.bui
  2026-08-06  6:28 ` [PATCH 2/2] watchdog: stm32_iwdg: " phucduc.bui
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: phucduc.bui @ 2026-08-06  6:28 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Maxime Coquelin,
	Alexandre Torgue, linux-watchdog
  Cc: linux-kernel, linux-stm32, linux-arm-kernel, bui duc phuc

From: bui duc phuc <phucduc.bui@gmail.com>

Return -EPROBE_DEFER from platform_get_irq_optional() so the driver is
re-probed when the interrupt resource becomes available instead of
continuing probe without an IRQ.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/watchdog/orion_wdt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 0e145f762f6f..8595073a8179 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -606,6 +606,8 @@ static int orion_wdt_probe(struct platform_device *pdev)
 
 	/* Request the IRQ only after the watchdog is disabled */
 	irq = platform_get_irq_optional(pdev, 0);
+	if (irq == -EPROBE_DEFER)
+		return irq;
 	if (irq > 0) {
 		/*
 		 * Not all supported platforms specify an interrupt for the
-- 
2.43.0


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

* [PATCH 2/2] watchdog: stm32_iwdg: Propagate -EPROBE_DEFER from IRQ lookup
  2026-08-06  6:28 [PATCH 1/2] watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup phucduc.bui
@ 2026-08-06  6:28 ` phucduc.bui
  2026-08-06  6:40   ` sashiko-bot
  2026-08-06  9:35   ` Guenter Roeck
  2026-08-06  6:35 ` [PATCH 1/2] watchdog: orion_wdt: " sashiko-bot
  2026-08-06  9:30 ` Guenter Roeck
  2 siblings, 2 replies; 8+ messages in thread
From: phucduc.bui @ 2026-08-06  6:28 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Maxime Coquelin,
	Alexandre Torgue, linux-watchdog
  Cc: linux-kernel, linux-stm32, linux-arm-kernel, bui duc phuc

From: bui duc phuc <phucduc.bui@gmail.com>

platform_get_irq_optional() does not returns 0. It returns a positive
IRQ number on success or a negative error code on failure.

Return -EPROBE_DEFER from platform_get_irq_optional() so the driver is
re-probed when the interrupt resource becomes available instead of
continuing probe without an IRQ.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 drivers/watchdog/stm32_iwdg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index b356a272ff9a..102f51084957 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -287,7 +287,9 @@ static int stm32_iwdg_irq_init(struct platform_device *pdev,
 		return 0;
 
 	irq = platform_get_irq_optional(pdev, 0);
-	if (irq <= 0)
+	if (irq == -EPROBE_DEFER)
+		return irq;
+	if (irq < 0)
 		return 0;
 
 	if (of_property_read_bool(np, "wakeup-source")) {
-- 
2.43.0


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

* Re: [PATCH 1/2] watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup
  2026-08-06  6:28 [PATCH 1/2] watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup phucduc.bui
  2026-08-06  6:28 ` [PATCH 2/2] watchdog: stm32_iwdg: " phucduc.bui
@ 2026-08-06  6:35 ` sashiko-bot
  2026-08-06  9:30 ` Guenter Roeck
  2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-08-06  6:35 UTC (permalink / raw)
  To: phucduc.bui; +Cc: linux-watchdog

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning -EPROBE_DEFER directly leaks the clock reference and enable count by bypassing the cleanup path.
--

--- Patch [1]: [PATCH 1/2] watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup ---
commit 0ad8e477efab703bb8fdf2177c651643b3096974
Author: bui duc phuc <phucduc.bui@gmail.com>

watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup

This commit propagates -EPROBE_DEFER from platform_get_irq_optional() so
the driver is re-probed when the interrupt resource becomes available
instead of continuing without an IRQ.

> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 1802634cc7cda..c132533b997d0 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -606,6 +606,8 @@ static int orion_wdt_probe(struct platform_device *pdev)
>  
>  	/* Request the IRQ only after the watchdog is disabled */
>  	irq = platform_get_irq_optional(pdev, 0);
> +	if (irq == -EPROBE_DEFER)
> +		return irq;

[Severity: High]
Does this direct return leak the clock reference and enable count?

Since dev->data->clock_init() was called earlier in orion_wdt_probe(),
would setting ret = irq and jumping to the disable_clk label ensure that
clk_disable_unprepare() and clk_put() are correctly called on probe deferral?

>  	if (irq > 0) {
>  		/*
>  		 * Not all supported platforms specify an interrupt for the

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

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

* Re: [PATCH 2/2] watchdog: stm32_iwdg: Propagate -EPROBE_DEFER from IRQ lookup
  2026-08-06  6:28 ` [PATCH 2/2] watchdog: stm32_iwdg: " phucduc.bui
@ 2026-08-06  6:40   ` sashiko-bot
  2026-08-06  9:35   ` Guenter Roeck
  1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-08-06  6:40 UTC (permalink / raw)
  To: phucduc.bui; +Cc: linux-watchdog

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

New issues:
- [High] Unexpected system reset due to probe deferral while the hardware watchdog is already running.

Pre-existing issues:
- [High] Leaked wake_irq configuration on driver unbind or probe failure causes permanent reprobe failures.
- [High] The driver requests the IRQ before initializing the driver data used by the interrupt handler, creating a race condition that can result in a NULL pointer dereference.
--

--- Patch [2]: [PATCH 2/2] watchdog: stm32_iwdg: Propagate -EPROBE_DEFER from IRQ lookup ---
commit dc7c8342d9ed50f0391276fabd2f26a71397adae
Author: bui duc phuc <phucduc.bui@gmail.com>

watchdog: stm32_iwdg: Propagate -EPROBE_DEFER from IRQ lookup

platform_get_irq_optional() does not return 0. It returns a positive IRQ
number on success or a negative error code on failure. Return -EPROBE_DEFER
from platform_get_irq_optional() so the driver is re-probed.

> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
> index b356a272ff9a0..102f510849578 100644
> --- a/drivers/watchdog/stm32_iwdg.c
> +++ b/drivers/watchdog/stm32_iwdg.c
> @@ -287,7 +287,9 @@ static int stm32_iwdg_irq_init(struct platform_device *pdev,
>  		return 0;
>  
>  	irq = platform_get_irq_optional(pdev, 0);
> -	if (irq <= 0)
> +	if (irq == -EPROBE_DEFER)
> +		return irq;

[Severity: High]
If the hardware watchdog is already running, won't returning -EPROBE_DEFER
here cause a regression by triggering an unexpected system reset?

If the bootloader started the watchdog, the hardware timer is actively
counting down. Because this deferral happens before the driver registers
with the watchdog core or pings the device, the probe will abort. Since
deferred probing can take an unpredictable amount of time, can the watchdog
expire and reset the system before the driver is successfully reprobed?

> +	if (irq < 0)
>  		return 0;
>  
>  	if (of_property_read_bool(np, "wakeup-source")) {
>  		ret = devm_device_init_wakeup(dev);
>  		if (ret)
>  			return ret;
>  
>  		ret = dev_pm_set_wake_irq(dev, irq);

[Severity: High]
This is a pre-existing issue, but does this leak the wake_irq configuration
on driver unbind or probe failure?

Since dev_pm_set_wake_irq() is not device-managed and lacks an explicit
cleanup via dev_pm_clear_wake_irq(), dev->power.wakeirq remains populated.
If a later initialization step fails or the driver is unbound, won't any
subsequent probe attempt hit a warning for an already initialized wake irq
and return -EEXIST, permanently failing reprobes?

>  		if (ret)
>  			return ret;
>  	}
>  
>  	ret = devm_request_irq(dev, irq, stm32_iwdg_isr, 0,
>  			       dev_name(dev), wdd);

[Severity: High]
This is a pre-existing issue, but can this cause a NULL pointer dereference
if the interrupt fires immediately?

In stm32_iwdg_probe(), the interrupt is requested here before driver data
is initialized:

	ret = stm32_iwdg_irq_init(pdev, wdt);
	if (ret)
		return ret;

	watchdog_set_drvdata(wdd, wdt);

If the interrupt fires before watchdog_set_drvdata() is executed, the
interrupt handler will read NULL driver data:

stm32_iwdg_isr() {
	struct watchdog_device *wdd = wdog_arg;
	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
	u32 reg;

	reg = reg_read(wdt->regs, IWDG_EWCR);

Since wdt is NULL, won't dereferencing wdt->regs crash the kernel?

>  	if (ret)
>  		return ret;
>  
>  	wdd->info = &stm32_iwdg_preinfo;
>  	return 0;
>  }

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

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

* Re: [PATCH 1/2] watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup
  2026-08-06  6:28 [PATCH 1/2] watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup phucduc.bui
  2026-08-06  6:28 ` [PATCH 2/2] watchdog: stm32_iwdg: " phucduc.bui
  2026-08-06  6:35 ` [PATCH 1/2] watchdog: orion_wdt: " sashiko-bot
@ 2026-08-06  9:30 ` Guenter Roeck
  2026-08-07  4:20   ` Bui Duc Phuc
  2 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2026-08-06  9:30 UTC (permalink / raw)
  To: phucduc.bui, Wim Van Sebroeck, Maxime Coquelin, Alexandre Torgue,
	linux-watchdog
  Cc: linux-kernel, linux-stm32, linux-arm-kernel

On 8/5/26 23:28, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> Return -EPROBE_DEFER from platform_get_irq_optional() so the driver is
> re-probed when the interrupt resource becomes available instead of
> continuing probe without an IRQ.
> 
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
>   drivers/watchdog/orion_wdt.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 0e145f762f6f..8595073a8179 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -606,6 +606,8 @@ static int orion_wdt_probe(struct platform_device *pdev)
>   
>   	/* Request the IRQ only after the watchdog is disabled */
>   	irq = platform_get_irq_optional(pdev, 0);
> +	if (irq == -EPROBE_DEFER)
> +		return irq;

I think this should be

	if (irq < 0 && irq != -ENXIO)
		return irq;

Guenter

>   	if (irq > 0) {
>   		/*
>   		 * Not all supported platforms specify an interrupt for the


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

* Re: [PATCH 2/2] watchdog: stm32_iwdg: Propagate -EPROBE_DEFER from IRQ lookup
  2026-08-06  6:28 ` [PATCH 2/2] watchdog: stm32_iwdg: " phucduc.bui
  2026-08-06  6:40   ` sashiko-bot
@ 2026-08-06  9:35   ` Guenter Roeck
  2026-08-07  4:23     ` Bui Duc Phuc
  1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2026-08-06  9:35 UTC (permalink / raw)
  To: phucduc.bui, Wim Van Sebroeck, Maxime Coquelin, Alexandre Torgue,
	linux-watchdog
  Cc: linux-kernel, linux-stm32, linux-arm-kernel

On 8/5/26 23:28, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> platform_get_irq_optional() does not returns 0. It returns a positive
> IRQ number on success or a negative error code on failure.
> 
> Return -EPROBE_DEFER from platform_get_irq_optional() so the driver is
> re-probed when the interrupt resource becomes available instead of
> continuing probe without an IRQ.
> 
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
>   drivers/watchdog/stm32_iwdg.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
> index b356a272ff9a..102f51084957 100644
> --- a/drivers/watchdog/stm32_iwdg.c
> +++ b/drivers/watchdog/stm32_iwdg.c
> @@ -287,7 +287,9 @@ static int stm32_iwdg_irq_init(struct platform_device *pdev,
>   		return 0;
>   
>   	irq = platform_get_irq_optional(pdev, 0);
> -	if (irq <= 0)
> +	if (irq == -EPROBE_DEFER)
> +		return irq;
> +	if (irq < 0)
>   		return 0;

	if (irq < 0) {
		if (irq != -ENXIO)
			return irq;
		return 0;
	}

platform_get_irq_optional() does not return 0. and -ENXIO is returned
if there is no interrupt.

Guenter


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

* Re: [PATCH 1/2] watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup
  2026-08-06  9:30 ` Guenter Roeck
@ 2026-08-07  4:20   ` Bui Duc Phuc
  0 siblings, 0 replies; 8+ messages in thread
From: Bui Duc Phuc @ 2026-08-07  4:20 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Wim Van Sebroeck, Maxime Coquelin, Alexandre Torgue,
	linux-watchdog, linux-kernel, linux-stm32, linux-arm-kernel

Hi Guenter,

Thank you for your review.

> > +     if (irq == -EPROBE_DEFER)
> > +             return irq;
>
> I think this should be
>
>         if (irq < 0 && irq != -ENXIO)
>                 return irq;
>

Agreed, that's better.
 -EINVAL also deserves to be returned, not just -EPROBE_DEFER
 only -ENXIO really means "no IRQ".
Will send v2 soon.

Best regards,
Phuc

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

* Re: [PATCH 2/2] watchdog: stm32_iwdg: Propagate -EPROBE_DEFER from IRQ lookup
  2026-08-06  9:35   ` Guenter Roeck
@ 2026-08-07  4:23     ` Bui Duc Phuc
  0 siblings, 0 replies; 8+ messages in thread
From: Bui Duc Phuc @ 2026-08-07  4:23 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Wim Van Sebroeck, Maxime Coquelin, Alexandre Torgue,
	linux-watchdog, linux-kernel, linux-stm32, linux-arm-kernel

Hi Guenter,

Thank you for your review.

> > -     if (irq <= 0)
> > +     if (irq == -EPROBE_DEFER)
> > +             return irq;
> > +     if (irq < 0)
> >               return 0;
>
>         if (irq < 0) {
>                 if (irq != -ENXIO)
>                         return irq;
>                 return 0;
>         }
>
> platform_get_irq_optional() does not return 0. and -ENXIO is returned
> if there is no interrupt.

I will send a v2 soon.

Best regards,
Phuc

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

end of thread, other threads:[~2026-08-07  4:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06  6:28 [PATCH 1/2] watchdog: orion_wdt: Propagate -EPROBE_DEFER from IRQ lookup phucduc.bui
2026-08-06  6:28 ` [PATCH 2/2] watchdog: stm32_iwdg: " phucduc.bui
2026-08-06  6:40   ` sashiko-bot
2026-08-06  9:35   ` Guenter Roeck
2026-08-07  4:23     ` Bui Duc Phuc
2026-08-06  6:35 ` [PATCH 1/2] watchdog: orion_wdt: " sashiko-bot
2026-08-06  9:30 ` Guenter Roeck
2026-08-07  4:20   ` Bui Duc Phuc

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