[PATCH 0/9] watchdog: Remove redundant error messages on IRQ request failure

[PATCH 0/9] watchdog: Remove redundant error messages on IRQ request failure

[Pan Chuang]

Commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()")
added automatic error logging to devm_request_threaded_irq() and
devm_request_any_context_irq() via the new devm_request_result() helper,
which prints device name, IRQ number, handler functions, and error code
on failure.

Since devm_request_irq() is a static inline wrapper around
devm_request_threaded_irq(), it also benefits from this
automatic logging.

Remove the now-redundant dev_err() and dev_err_probe() calls
in watchdog drivers that follow these devm_request_*_irq()
functions, as the core now provides more detailed diagnostic
information on failure.

Pan Chuang (9):
  watchdog: bd96801_wdt: Remove redundant dev_err_probe()
  watchdog: cadence_wdt: Remove redundant dev_err()
  watchdog: intel-mid_wdt: Remove redundant dev_err()
  watchdog: keembay: Remove redundant dev_err_probe()
  watchdog: marvell_gti: Remove redundant dev_err_probe()
  watchdog: orion: Remove redundant dev_err()
  watchdog: realtek_otto: Remove redundant dev_err_probe()
  watchdog: sama5d4: Remove redundant dev_err()
  watchdog: sprd: Remove redundant dev_err()

 drivers/watchdog/bd96801_wdt.c      | 3 +--
 drivers/watchdog/cadence_wdt.c      | 6 +-----
 drivers/watchdog/intel-mid_wdt.c    | 4 +---
 drivers/watchdog/keembay_wdt.c      | 4 ++--
 drivers/watchdog/marvell_gti_wdt.c  | 2 +-
 drivers/watchdog/orion_wdt.c        | 8 ++------
 drivers/watchdog/realtek_otto_wdt.c | 2 +-
 drivers/watchdog/sama5d4_wdt.c      | 4 +---
 drivers/watchdog/sprd_wdt.c         | 4 +---
 9 files changed, 11 insertions(+), 26 deletions(-)

-- 
2.34.1

[PATCH 1/9] watchdog: bd96801_wdt: Remove redundant dev_err_probe()

[Pan Chuang]

Since commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
devm_request_threaded_irq() automatically logs detailed error messages on
failure. Remove the now-redundant driver-specific dev_err_probe() call.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/watchdog/bd96801_wdt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/watchdog/bd96801_wdt.c b/drivers/watchdog/bd96801_wdt.c
index 12b74fd2bc05..6a1c5a4d39bf 100644
--- a/drivers/watchdog/bd96801_wdt.c
+++ b/drivers/watchdog/bd96801_wdt.c
@@ -390,8 +390,7 @@ static int bd96801_wdt_probe(struct platform_device *pdev)
 						IRQF_ONESHOT,  "bd96801-wdg",
 						NULL);
 		if (ret)
-			return dev_err_probe(&pdev->dev, ret,
-					     "Failed to register IRQ\n");
+			return ret;
 	}
 
 	return devm_watchdog_register_device(&pdev->dev, &w->wdt);
-- 
2.34.1

Re: [PATCH 1/9] watchdog: bd96801_wdt: Remove redundant dev_err_probe()

[sashiko-bot]

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

Pre-existing issues:
- [High] Incorrect unit conversion drastically reduces max_hw_heartbeat_ms, causing excessive I2C traffic or unexpected reboots.
- [High] Missing NULL pointer check for dev_get_regmap() leads to kernel panic.
--

--- Patch [1]: [PATCH 1/9] watchdog: bd96801_wdt: Remove redundant dev_err_probe() ---
commit e5ff79584985ca3f41c7aadbe71a7bb46a6c07a7
Author: Pan Chuang <panchuang@vivo.com>

watchdog: bd96801_wdt: Remove redundant dev_err_probe()

This patch removes a redundant driver-specific dev_err_probe() call from
bd96801_wdt_probe(). The function devm_request_threaded_irq() automatically
logs detailed error messages on failure since commit 55b48e23f5c4.

> diff --git a/drivers/watchdog/bd96801_wdt.c b/drivers/watchdog/bd96801_wdt.c
> index 12b74fd2bc052..6a1c5a4d39bfd 100644
> --- a/drivers/watchdog/bd96801_wdt.c
> +++ b/drivers/watchdog/bd96801_wdt.c
> @@ -390,8 +390,7 @@ static int bd96801_wdt_probe(struct platform_device *pdev)

[Severity: High]
This is a pre-existing issue, but does this code cause a severe performance
regression or unexpected system resets?

Looking at bd96801_set_heartbeat_from_hw() in drivers/watchdog/bd96801_wdt.c:

drivers/watchdog/bd96801_wdt.c:bd96801_set_heartbeat_from_hw() {
        ...
        w->wdt.max_hw_heartbeat_ms = (fast << sel) / USEC_PER_MSEC;

        if ((conf_reg & BD96801_WD_TYPE_MASK) == BD96801_WD_TYPE_WIN)
                w->wdt.min_hw_heartbeat_ms = fast / USEC_PER_MSEC;
        ...
}

When the driver probes and the watchdog is already running, this reads the
hardware timeout. The hardware value is in units of 100 microseconds. To
convert to milliseconds, it should be divided by 10. However, the code
divides by USEC_PER_MSEC (1000).

This reduces the reported max_hw_heartbeat_ms by a factor of 100. If the
value rounds down to 0, the watchdog core will incorrectly assume the
hardware can handle large timeouts and schedule pings based on the user
timeout, triggering a system reset on boot. If it evaluates to a small
non-zero value, the watchdog core's worker thread will aggressively wake
up and ping the I2C device, causing extreme I2C bus contention.

[Severity: High]
This isn't a bug introduced by this patch, but can a missing NULL pointer
check lead to a kernel panic?

Looking at bd96801_wdt_probe() in drivers/watchdog/bd96801_wdt.c:

drivers/watchdog/bd96801_wdt.c:bd96801_wdt_probe() {
        ...
        w->regmap = dev_get_regmap(pdev->dev.parent, NULL);
        w->dev = &pdev->dev;
        ...
        ret = regmap_read(w->regmap, BD96801_REG_WD_CONF, &val);
        ...
}

If the parent device lacks a regmap (e.g., due to a malformed device tree or
a parent driver initialization bug), dev_get_regmap() returns NULL.

The driver unconditionally passes this pointer to regmap_read(), which will
dereference it and cause a crash.

>  						IRQF_ONESHOT,  "bd96801-wdg",
>  						NULL);
>  		if (ret)
> -			return dev_err_probe(&pdev->dev, ret,
> -					     "Failed to register IRQ\n");
> +			return ret;
>  	}
>  
>  	return devm_watchdog_register_device(&pdev->dev, &w->wdt);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084218.79073-1-panchuang@vivo.com?part=1

[PATCH 2/9] watchdog: cadence_wdt: Remove redundant dev_err()

[Pan Chuang]

Since commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
devm_request_irq() automatically logs detailed error messages on
failure. Remove the now-redundant driver-specific dev_err() call.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/watchdog/cadence_wdt.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/watchdog/cadence_wdt.c b/drivers/watchdog/cadence_wdt.c
index 23d41043863f..ad46b33ce5f2 100644
--- a/drivers/watchdog/cadence_wdt.c
+++ b/drivers/watchdog/cadence_wdt.c
@@ -312,12 +312,8 @@ static int cdns_wdt_probe(struct platform_device *pdev)
 	if (!wdt->rst && irq >= 0) {
 		ret = devm_request_irq(dev, irq, cdns_wdt_irq_handler, 0,
 				       pdev->name, pdev);
-		if (ret) {
-			dev_err(dev,
-				"cannot register interrupt handler err=%d\n",
-				ret);
+		if (ret)
 			return ret;
-		}
 	}
 
 	/* Initialize the members of cdns_wdt structure */
-- 
2.34.1

Re: [PATCH 2/9] watchdog: cadence_wdt: Remove redundant dev_err()

[sashiko-bot]

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

Pre-existing issues:
- [High] Incorrect handling of platform_get_irq() return value leads to silently broken watchdogs, ignored probe deferrals, and spurious boot errors.
- [High] The driver advertises a hardcoded max_timeout of 516 seconds, which causes silent truncation and premature system reboots on platforms with slower clocks.
- [High] cdns_wdt_settimeout() unconditionally enables the hardware watchdog, which can cause spurious system resets.
--

--- Patch [2]: [PATCH 2/9] watchdog: cadence_wdt: Remove redundant dev_err() ---
commit 8e2910f9fc6c98ef77eadf7acc598ee3e8c4f9f7
Author: Pan Chuang <panchuang@vivo.com>

watchdog: cadence_wdt: Remove redundant dev_err()

This commit removes a redundant dev_err() call after devm_request_irq() 
because devm_request_irq() now automatically logs detailed error messages 
on failure.

> diff --git a/drivers/watchdog/cadence_wdt.c b/drivers/watchdog/cadence_wdt.c
> index 23d41043863f6..ad46b33ce5f2e 100644
> --- a/drivers/watchdog/cadence_wdt.c
> +++ b/drivers/watchdog/cadence_wdt.c
> @@ -312,12 +312,8 @@ static int cdns_wdt_probe(struct platform_device *pdev)

[Severity: High]
This is a pre-existing issue, but is the driver advertising a maximum timeout
that exceeds hardware capabilities?

In cdns_wdt_probe(), the max_timeout is hardcoded:

	cdns_wdt_device->max_timeout = CDNS_WDT_MAX_TIMEOUT;

If this is 516 seconds, but the hardware uses a slower clock (e.g., 75MHz
with a 512 prescaler), a requested timeout of 516 seconds calculates to
18454 in cdns_wdt_start(), which is silently clamped:

	count = (wdd->timeout * (clock_f / wdt->prescaler)) /
		 CDNS_WDT_COUNTER_VALUE_DIVISOR + 1;

	if (count > CDNS_WDT_COUNTER_MAX)
		count = CDNS_WDT_COUNTER_MAX;

Since the hardware maximum is only 4095 (12-bit), could this result in an
actual timeout of ~114.5 seconds, causing premature system reboots before
the watchdog daemon expects to ping it?

>  	if (!wdt->rst && irq >= 0) {

[Severity: High]
This is a pre-existing issue, but does this correctly handle cases where the
IRQ is deferred or omitted?

Looking at cdns_wdt_probe(), platform_get_irq() is called unconditionally
before this block:

	wdt->rst = of_property_read_bool(dev->of_node, "reset-on-timeout");
	irq = platform_get_irq(pdev, 0);

If reset-on-timeout is specified, the IRQ is unneeded, but an error is still
logged to dmesg. If reset-on-timeout is false and platform_get_irq() returns
-EPROBE_DEFER, does this silently ignore the error since irq >= 0 fails,
completing probe without an IRQ and leaving the watchdog broken?

>  		ret = devm_request_irq(dev, irq, cdns_wdt_irq_handler, 0,
>  				       pdev->name, pdev);
> -		if (ret) {
> -			dev_err(dev,
> -				"cannot register interrupt handler err=%d\n",
> -				ret);
> +		if (ret)
>  			return ret;
> -		}
>  	}
>  
>  	/* Initialize the members of cdns_wdt structure */

[Severity: High]
This is a pre-existing issue, but could configuring the timeout unexpectedly
start the hardware watchdog?

When the timeout is configured via WDIOC_SETTIMEOUT, the core calls
cdns_wdt_settimeout(), which directly calls cdns_wdt_start():

static int cdns_wdt_settimeout(struct watchdog_device *wdd,
			       unsigned int new_time)
{
	wdd->timeout = new_time;

	return cdns_wdt_start(wdd);
}

Inside cdns_wdt_start(), the timer is fully enabled:

	data = CDNS_WDT_ZMR_WDEN_MASK | CDNS_WDT_ZMR_RSTLEN_16 |
	       CDNS_WDT_ZMR_ZKEY_VAL;

If userspace had previously stopped the watchdog, won't this start the
hardware timer without the watchdog core knowing it is active? Could this
lead to spurious system resets since the core won't send keepalive pings for
a stopped device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084218.79073-1-panchuang@vivo.com?part=2

[PATCH 3/9] watchdog: intel-mid_wdt: Remove redundant dev_err()

[Pan Chuang]

Since commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
devm_request_irq() automatically logs detailed error messages on
failure. Remove the now-redundant driver-specific dev_err() call.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/watchdog/intel-mid_wdt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
index 756d262dc580..b1e1b77ed190 100644
--- a/drivers/watchdog/intel-mid_wdt.c
+++ b/drivers/watchdog/intel-mid_wdt.c
@@ -165,10 +165,8 @@ static int mid_wdt_probe(struct platform_device *pdev)
 	ret = devm_request_irq(dev, pdata->irq, mid_wdt_irq,
 			       IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
 			       wdt_dev);
-	if (ret) {
-		dev_err(dev, "error requesting warning irq %d\n", pdata->irq);
+	if (ret)
 		return ret;
-	}
 
 	/*
 	 * The firmware followed by U-Boot leaves the watchdog running
-- 
2.34.1

Re: [PATCH 3/9] watchdog: intel-mid_wdt: Remove redundant dev_err()

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver advertises the WDIOF_SETTIMEOUT capability but lacks the required .set_timeout callback, causing timeout updates to silently fail.
- [High] The shared IRQ handler mid_wdt_irq unconditionally panics the system, violating shared IRQ semantics and risking spurious panics.
--

--- Patch [3]: [PATCH 3/9] watchdog: intel-mid_wdt: Remove redundant dev_err() ---
commit f0c7169449cddf6ac1b4686957e0ef5aa4340ca5
Author: Pan Chuang <panchuang@vivo.com>

watchdog: intel-mid_wdt: Remove redundant dev_err()

This commit removes a redundant dev_err() call after devm_request_irq().
The core function now automatically logs detailed error messages on failure,
making the driver-specific logging unnecessary.

> diff --git a/drivers/watchdog/intel-mid_wdt.c b/drivers/watchdog/intel-mid_wdt.c
> index 756d262dc5808..b1e1b77ed1905 100644
> --- a/drivers/watchdog/intel-mid_wdt.c
> +++ b/drivers/watchdog/intel-mid_wdt.c
> @@ -165,10 +165,8 @@ static int mid_wdt_probe(struct platform_device *pdev)
>  	ret = devm_request_irq(dev, pdata->irq, mid_wdt_irq,
>  			       IRQF_SHARED | IRQF_NO_SUSPEND, "watchdog",
>  			       wdt_dev);

[Severity: High]
This is a pre-existing issue, but does this interrupt handler violate
shared IRQ semantics?

The interrupt is requested here with IRQF_SHARED, which explicitly allows
other devices to share the line.

However, looking at the handler:

drivers/watchdog/intel-mid_wdt.c:mid_wdt_irq()
{
	panic("Kernel Watchdog");

	/* This code should not be reached */
	return IRQ_HANDLED;
}

Since it unconditionally calls panic() without verifying that the watchdog
device actually triggered the interrupt, couldn't any other device sharing
this IRQ line cause a spurious kernel panic?

[Severity: High]
This is also a pre-existing issue, but does the driver support updating
the timeout properly?

The driver advertises the ability to change the timeout to userspace:

drivers/watchdog/intel-mid_wdt.c:mid_wdt_info
	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE,

But the operations struct is missing the .set_timeout callback:

drivers/watchdog/intel-mid_wdt.c:mid_wdt_ops
static const struct watchdog_ops mid_wdt_ops = {
	.owner = THIS_MODULE,
	.start = wdt_start,
	.stop = wdt_stop,
	.ping = wdt_ping,
};

When a userspace daemon attempts to change the timeout, the core watchdog
subsystem will call the .ping callback instead of programming the hardware.

Does this result in the hardware continuing to use the default timeout while
userspace believes a longer timeout has been set, potentially causing
unexpected watchdog resets?

> -	if (ret) {
> -		dev_err(dev, "error requesting warning irq %d\n", pdata->irq);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	/*
>  	 * The firmware followed by U-Boot leaves the watchdog running

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084218.79073-1-panchuang@vivo.com?part=3

[PATCH 4/9] watchdog: keembay: Remove redundant dev_err_probe()

[Pan Chuang]

Since commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
devm_request_irq() automatically logs detailed error messages on
failure. Remove the now-redundant driver-specific dev_err_probe() calls.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/watchdog/keembay_wdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index 3854249c7455..7c8e53921e40 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -214,7 +214,7 @@ static int keembay_wdt_probe(struct platform_device *pdev)
 	ret = devm_request_irq(dev, wdt->th_irq, keembay_wdt_th_isr, 0,
 			       "keembay-wdt", wdt);
 	if (ret)
-		return dev_err_probe(dev, ret, "Failed to request IRQ for threshold\n");
+		return ret;
 
 	wdt->to_irq = platform_get_irq_byname(pdev, "timeout");
 	if (wdt->to_irq < 0)
@@ -223,7 +223,7 @@ static int keembay_wdt_probe(struct platform_device *pdev)
 	ret = devm_request_irq(dev, wdt->to_irq, keembay_wdt_to_isr, 0,
 			       "keembay-wdt", wdt);
 	if (ret)
-		return dev_err_probe(dev, ret, "Failed to request IRQ for timeout\n");
+		return ret;
 
 	wdt->wdd.parent		= dev;
 	wdt->wdd.info		= &keembay_wdt_info;
-- 
2.34.1

Re: [PATCH 4/9] watchdog: keembay: Remove redundant dev_err_probe()

[sashiko-bot]

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

Pre-existing issues:
- [High] Race condition in probe leading to a NULL pointer dereference in the interrupt handler.
- [High] Watchdog clock may be disabled by the Common Clock Framework (CCF) late initcall, silently stopping the watchdog.
--

--- Patch [4]: [PATCH 4/9] watchdog: keembay: Remove redundant dev_err_probe() ---
commit ae07b07078ab54b39721c2064f0c148ab624e578
Author: Pan Chuang <panchuang@vivo.com>
watchdog: keembay: Remove redundant dev_err_probe()

This commit removes redundant dev_err_probe() calls from devm_request_irq() 
error paths. The core devm_request_irq() already handles logging detailed 
error messages.

> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index 2a39114dbc641..fa8c912e83aa0 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -215,7 +215,7 @@ static int keembay_wdt_probe(struct platform_device *pdev)

[Severity: High]
This is a pre-existing issue, but does this driver safely handle the Common
Clock Framework late initcalls?

Looking at keembay_wdt_probe(), the driver fetches the clock but never
explicitly enables it:

drivers/watchdog/keembay_wdt.c:keembay_wdt_probe() {
    ...
    /* we do not need to enable the clock as it is enabled by default */
    wdt->clk = devm_clk_get(dev, NULL);
    ...
}

Because clk_prepare_enable() is never called, the CCF late initcall
(clk_disable_unused) will find this clock with a reference count of zero and
disable it to save power. When this happens, won't the watchdog hardware
silently stop ticking?

>  	ret = devm_request_irq(dev, wdt->th_irq, keembay_wdt_th_isr, 0,
>  			       "keembay-wdt", wdt);

[Severity: High]
This is a pre-existing issue, but can this early IRQ request cause a NULL
pointer dereference if the watchdog is already running?

If the bootloader has enabled the watchdog and a pre-timeout interrupt is
pending, devm_request_irq() will unmask the interrupt line and the ISR will
fire immediately.

The ISR calls keembay_wdt_set_pretimeout_reg(), which expects the watchdog
driver data to be initialized:

drivers/watchdog/keembay_wdt.c:keembay_wdt_set_pretimeout_reg() {
    ...
    struct keembay_wdt *wdt = watchdog_get_drvdata(wdog);
    ...
    keembay_wdt_writel(wdt, TIM_WATCHDOG_INT_THRES, th_val * wdt->rate);
    ...
}

Since watchdog_set_drvdata() is called further down in keembay_wdt_probe()
(after the IRQ is requested), watchdog_get_drvdata() would return NULL here,
leading to a crash when wdt->rate is accessed. 

Could the IRQ requests be moved after the watchdog structure is fully
initialized?

>  	if (ret)
> -		return dev_err_probe(dev, ret, "Failed to request IRQ for threshold\n");
> +		return ret;
>  
>  	wdt->to_irq = platform_get_irq_byname(pdev, "timeout");
>  	if (wdt->to_irq < 0)

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084218.79073-1-panchuang@vivo.com?part=4

[PATCH 5/9] watchdog: marvell_gti: Remove redundant dev_err_probe()

[Pan Chuang]

Since commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
devm_request_irq() automatically logs detailed error messages on
failure. Remove the now-redundant driver-specific dev_err_probe() call.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/watchdog/marvell_gti_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/marvell_gti_wdt.c b/drivers/watchdog/marvell_gti_wdt.c
index 298089d45ab8..31167034963c 100644
--- a/drivers/watchdog/marvell_gti_wdt.c
+++ b/drivers/watchdog/marvell_gti_wdt.c
@@ -321,7 +321,7 @@ static int gti_wdt_probe(struct platform_device *pdev)
 	err = devm_request_irq(dev, irq, gti_wdt_interrupt, 0,
 			       pdev->name, &priv->wdev);
 	if (err)
-		return dev_err_probe(dev, err, "Failed to register interrupt handler\n");
+		return err;
 
 	dev_info(dev, "Watchdog enabled (timeout=%d sec)\n", wdog_dev->timeout);
 	return 0;
-- 
2.34.1

Re: [PATCH 5/9] watchdog: marvell_gti: Remove redundant dev_err_probe()

[sashiko-bot]

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

Pre-existing issues:
- [High] Double disable and unprepare of the clock due to a redundant devres action.
- [Medium] The watchdog device is exposed to userspace before its IRQ is fully requested, creating a race condition.
- [Medium] Pretimeout value is not updated when the pretimeout interrupt is disabled, leaving a stale state.
--

--- Patch [5]: [PATCH 5/9] watchdog: marvell_gti: Remove redundant dev_err_probe() ---
commit 2a9a85c950abb5fda8867255ce0f94190626fc24
Author: Pan Chuang <panchuang@vivo.com>

watchdog: marvell_gti: Remove redundant dev_err_probe()
    
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() automatically logs detailed error
messages on failure. Remove the now-redundant driver-specific dev_err_probe()
call.

> diff --git a/drivers/watchdog/marvell_gti_wdt.c b/drivers/watchdog/marvell_gti_wdt.c
> index 298089d45ab89..31167034963c1 100644
> --- a/drivers/watchdog/marvell_gti_wdt.c
> +++ b/drivers/watchdog/marvell_gti_wdt.c
> @@ -321,7 +321,7 @@ static int gti_wdt_probe(struct platform_device *pdev)
>  	err = devm_request_irq(dev, irq, gti_wdt_interrupt, 0,
>  			       pdev->name, &priv->wdev);

[Severity: Medium]
This is a pre-existing issue, but is there a race condition here if the device
is exposed to userspace before its IRQ is fully requested?

In drivers/watchdog/marvell_gti_wdt.c:gti_wdt_probe(), the watchdog device is
registered before requesting the interrupt:

	err = devm_watchdog_register_device(dev, wdog_dev);
	if (err)
		return err;

	irq = platform_get_irq(pdev, 0);
	if (irq < 0)
		return irq;

	err = devm_request_irq(dev, irq, gti_wdt_interrupt, 0,
			       pdev->name, &priv->wdev);

If userspace opens /dev/watchdog and starts the watchdog immediately after
devm_watchdog_register_device() completes, but before devm_request_irq()
registers the handler, could this lead to unhandled hardware interrupts or
spurious interrupt warnings?

Should device registration be the final step in probe() to guarantee all
hardware resources are fully prepared before userspace can interact with them?

>  	if (err)
> -		return dev_err_probe(dev, err, "Failed to register interrupt handler\n");
> +		return err;
>  
>  	dev_info(dev, "Watchdog enabled (timeout=%d sec)\n", wdog_dev->timeout);
>  	return 0;

[Severity: High]
This is a pre-existing issue, but does this code cause a double disable and
unprepare of the clock?

Looking at drivers/watchdog/marvell_gti_wdt.c:gti_wdt_get_cntfrq():

	priv->sclk = devm_clk_get_enabled(&pdev->dev, NULL);
	if (IS_ERR(priv->sclk))
		return PTR_ERR(priv->sclk);

	err = devm_add_action_or_reset(&pdev->dev,
				       gti_clk_disable_unprepare, priv->sclk);

Since devm_clk_get_enabled() automatically adds a devres action to disable
and unprepare the clock on teardown, does registering a redundant devres action
(gti_clk_disable_unprepare) for the same clock cause clk_disable_unprepare()
to be called twice on a single reference during driver unbind or probe error
paths?

[Severity: Medium]
This is a pre-existing issue, but does the pretimeout value remain stale when
the pretimeout interrupt is disabled?

Looking at drivers/watchdog/marvell_gti_wdt.c:gti_wdt_set_pretimeout():

	if (!timeout) {
		/* Disable Interrupt */
		writeq(GTI_CWD_INT_ENA_CLR_VAL(priv->wdt_timer_idx),
		       priv->base + GTI_CWD_INT_ENA_CLR);
		return 0;
	}

When timeout is 0, the code disables the pretimeout in hardware and returns 0.

However, because the driver implements the set_pretimeout callback, does the
watchdog core framework delegate software state tracking to it?

Since the driver fails to update wdev->pretimeout = 0, would userspace queries
via WDIOC_GETPRETIMEOUT return the old, non-zero value, falsely indicating
pretimeout is still active?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084218.79073-1-panchuang@vivo.com?part=5

[PATCH 6/9] watchdog: orion: Remove redundant dev_err()

[Pan Chuang]

Since commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
devm_request_irq() automatically logs detailed error messages on
failure. Remove the now-redundant driver-specific dev_err() calls.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/watchdog/orion_wdt.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 0e145f762f6f..1802634cc7cd 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -613,10 +613,8 @@ static int orion_wdt_probe(struct platform_device *pdev)
 		 */
 		ret = devm_request_irq(&pdev->dev, irq, orion_wdt_irq, 0,
 				       pdev->name, dev);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "failed to request IRQ\n");
+		if (ret < 0)
 			goto disable_clk;
-		}
 	}
 
 	/* Optional 2nd interrupt for pretimeout */
@@ -625,10 +623,8 @@ static int orion_wdt_probe(struct platform_device *pdev)
 		orion_wdt_info.options |= WDIOF_PRETIMEOUT;
 		ret = devm_request_irq(&pdev->dev, irq, orion_wdt_pre_irq,
 				       0, pdev->name, dev);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "failed to request IRQ\n");
+		if (ret < 0)
 			goto disable_clk;
-		}
 	}
 
 
-- 
2.34.1

Re: [PATCH 6/9] watchdog: orion: Remove redundant dev_err()

[sashiko-bot]

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

Pre-existing issues:
- [High] The probe function silently ignores probe deferral and other errors from platform_get_irq_optional().
- [High] The driver incorrectly modifies a static global watchdog_info structure during device probe.
--

--- Patch [6]: [PATCH 6/9] watchdog: orion: Remove redundant dev_err() ---
commit 820e86c1dcc7da8eb296b1915c4c9eb8f9dc8666
Author: Pan Chuang <panchuang@vivo.com>

watchdog: orion: Remove redundant dev_err()

This commit removes redundant dev_err() messages in the driver, as
devm_request_irq() automatically logs errors on failure.

> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 0e145f762f6f2..1802634cc7cda 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -613,10 +613,8 @@ static int orion_wdt_probe(struct platform_device *pdev)
>  		 */
>  		ret = devm_request_irq(&pdev->dev, irq, orion_wdt_irq, 0,
>  				       pdev->name, dev);

[Severity: High]
This isn't a bug introduced by this patch, but does orion_wdt_probe() silently
ignore probe deferrals when getting these interrupts?

Earlier in this function, the primary and secondary interrupts are fetched:

    irq = platform_get_irq_optional(pdev, 0);
    if (irq > 0) {

If the interrupt controller is not yet ready, platform_get_irq_optional()
returns -EPROBE_DEFER. Because of the if (irq > 0) check, we silently skip
the interrupts instead of propagating the error and deferring the probe. This
can cause the watchdog to permanently drop its interrupt-driven capabilities on
boot.

> -		if (ret < 0) {
> -			dev_err(&pdev->dev, "failed to request IRQ\n");
> +		if (ret < 0)
>  			goto disable_clk;
> -		}
>  	}
>  
>  	/* Optional 2nd interrupt for pretimeout */
> @@ -625,10 +623,8 @@ static int orion_wdt_probe(struct platform_device *pdev)
>  		orion_wdt_info.options |= WDIOF_PRETIMEOUT;

[Severity: High]
This is a pre-existing issue, but does this permanently corrupt the static
global orion_wdt_info struct for the lifetime of the module?

If the driver successfully finds a pretimeout interrupt during probe, it
permanently modifies the shared static global orion_wdt_info structure by
setting WDIOF_PRETIMEOUT.

If the device is subsequently unbound and re-bound to a hardware configuration
without the interrupt, or if multiple SoC instances exist with differing
interrupt capabilities, the driver will inherit the corrupted global state and
falsely advertise the pretimeout feature to userspace.

>  		ret = devm_request_irq(&pdev->dev, irq, orion_wdt_pre_irq,
>  				       0, pdev->name, dev);
> -		if (ret < 0) {
> -			dev_err(&pdev->dev, "failed to request IRQ\n");
> +		if (ret < 0)
>  			goto disable_clk;
> -		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084218.79073-1-panchuang@vivo.com?part=6

[PATCH 7/9] watchdog: realtek_otto: Remove redundant dev_err_probe()

[Pan Chuang]

Since commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
devm_request_irq() automatically logs detailed error messages on
failure. Remove the now-redundant driver-specific dev_err_probe() call.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/watchdog/realtek_otto_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/realtek_otto_wdt.c b/drivers/watchdog/realtek_otto_wdt.c
index 9094f2189f55..a07bcf1de7c7 100644
--- a/drivers/watchdog/realtek_otto_wdt.c
+++ b/drivers/watchdog/realtek_otto_wdt.c
@@ -311,7 +311,7 @@ static int otto_wdt_probe(struct platform_device *pdev)
 	ret = devm_request_irq(dev, ctrl->irq_phase1, otto_wdt_phase1_isr, 0,
 			"realtek-otto-wdt", ctrl);
 	if (ret)
-		return dev_err_probe(dev, ret, "Failed to get IRQ for phase1\n");
+		return ret;
 
 	ret = otto_wdt_probe_reset_mode(ctrl);
 	if (ret)
-- 
2.34.1

Re: [PATCH 7/9] watchdog: realtek_otto: Remove redundant dev_err_probe()

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver aborts probing if the `realtek,reset-mode` property is not provided in the Device Tree, violating the DT binding contract.
- [High] The driver does not properly implement `max_hw_heartbeat_ms`, causing probe and runtime failures for valid large timeouts.
--

--- Patch [7]: [PATCH 7/9] watchdog: realtek_otto: Remove redundant dev_err_probe() ---
commit e807bbe77a13713af6db50c1b1c784d911002006
Author: Pan Chuang <panchuang@vivo.com>

watchdog: realtek_otto: Remove redundant dev_err_probe()

Since commit 55b48e23f5c4, devm_request_irq() automatically logs detailed
error messages on failure. Remove the now-redundant driver-specific
dev_err_probe() call.

> diff --git a/drivers/watchdog/realtek_otto_wdt.c b/drivers/watchdog/realtek_otto_wdt.c
> index 01b3ef89bacfc..e29c99cbd8140 100644
> --- a/drivers/watchdog/realtek_otto_wdt.c
> +++ b/drivers/watchdog/realtek_otto_wdt.c
> @@ -312,7 +312,7 @@ static int otto_wdt_probe(struct platform_device *pdev)
>  	ret = devm_request_irq(dev, ctrl->irq_phase1, otto_wdt_phase1_isr, 0,
>  			"realtek-otto-wdt", ctrl);
>  	if (ret)
> -		return dev_err_probe(dev, ret, "Failed to get IRQ for phase1\n");
> +		return ret;
>  
>  	ret = otto_wdt_probe_reset_mode(ctrl);
>  	if (ret)

[Severity: High]
This is a pre-existing issue, but does this code cause the probe to fail if
the optional realtek,reset-mode property is omitted from the device tree?

If the property is missing, fwnode_property_string_array_count() will return
-EINVAL here:

drivers/watchdog/realtek_otto_wdt.c:otto_wdt_probe_reset_mode() {
	...
	mode_count = fwnode_property_string_array_count(node, mode_property);
	if (mode_count < 0)
		return mode_count;
	...
}

Because otto_wdt_probe() blindly propagates this return value, the device
becomes unavailable instead of falling back to the documented 'cpu' default.

[Severity: High]
This is also a pre-existing issue, but does otto_wdt_determine_timeouts()
fail to utilize the watchdog core's max_hw_heartbeat_ms feature properly?

When a timeout exceeds the maximum hardware capacity (for example, if
timeout-sec is set to 60s in the device tree), the function calculates
a prescale greater than OTTO_WDT_PRESCALE_MAX and returns -EINVAL:

drivers/watchdog/realtek_otto_wdt.c:otto_wdt_determine_timeouts() {
	...
	prescale = prescale_next;
	if (prescale > OTTO_WDT_PRESCALE_MAX)
		return -EINVAL;
	...
}

This causes probe or ioctl failures for valid large timeouts instead of 
allowing the core to manage it. Additionally, the function incorrectly 
overwrites the requested timeout:

drivers/watchdog/realtek_otto_wdt.c:otto_wdt_determine_timeouts() {
	...
	timeout_ms = total_ticks * tick_ms;
	ctrl->wdev.timeout = timeout_ms / 1000;
	...
}

The watchdog core requires the driver to keep wdev->timeout as requested
by the user, and only clamp the internal hardware configuration, letting
the core ping the hardware periodically for longer timeouts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084218.79073-1-panchuang@vivo.com?part=7

[PATCH 8/9] watchdog: sama5d4: Remove redundant dev_err()

[Pan Chuang]

Since commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
devm_request_irq() automatically logs detailed error messages on
failure. Remove the now-redundant driver-specific dev_err() call.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/watchdog/sama5d4_wdt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 704b786cc2ec..ecdd9aeab79c 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -291,10 +291,8 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
 		ret = devm_request_irq(dev, irq, sama5d4_wdt_irq_handler,
 				       IRQF_SHARED | IRQF_IRQPOLL |
 				       IRQF_NO_SUSPEND, pdev->name, pdev);
-		if (ret) {
-			dev_err(dev, "cannot register interrupt handler\n");
+		if (ret)
 			return ret;
-		}
 	}
 
 	watchdog_init_timeout(wdd, wdt_timeout, dev);
-- 
2.34.1

Re: [PATCH 8/9] watchdog: sama5d4: Remove redundant dev_err()

[sashiko-bot]

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

Pre-existing issues:
- [High] Probe order bug: shared interrupt is requested before drvdata is initialized, leading to a potential NULL pointer dereference in the interrupt handler.
--

--- Patch [8]: [PATCH 8/9] watchdog: sama5d4: Remove redundant dev_err() ---
commit 47cdf2034aad624309814a76e94fadcc6063281c
Author: Pan Chuang <panchuang@vivo.com>

watchdog: sama5d4: Remove redundant dev_err()

Since devm_request_irq() automatically logs detailed error messages on
failure, this commit removes the now-redundant driver-specific dev_err()
call from the probe path.

> diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
> index 704b786cc2ec6..ecdd9aeab79cc 100644
> --- a/drivers/watchdog/sama5d4_wdt.c
> +++ b/drivers/watchdog/sama5d4_wdt.c
> @@ -291,10 +291,8 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
>  		ret = devm_request_irq(dev, irq, sama5d4_wdt_irq_handler,
>  				       IRQF_SHARED | IRQF_IRQPOLL |
>  				       IRQF_NO_SUSPEND, pdev->name, pdev);

[Severity: High]
This is a pre-existing issue, but since IRQF_SHARED is used here, could the
interrupt fire immediately if another device on the same line triggers it, or
if there is a pending hardware interrupt?

If that happens, sama5d4_wdt_irq_handler() will be called with pdev as dev_id:

drivers/watchdog/sama5d4_wdt.c:sama5d4_wdt_irq_handler() {
	struct sama5d4_wdt *wdt = platform_get_drvdata(dev_id);
	...
	if (wdt->sam9x60_support)
	...
}

Because platform_set_drvdata() is called much later in sama5d4_wdt_probe(),
platform_get_drvdata(dev_id) will return a null pointer. Does this result in
a null pointer dereference during probe?

> -		if (ret) {
> -			dev_err(dev, "cannot register interrupt handler\n");
> +		if (ret)
>  			return ret;
> -		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084218.79073-1-panchuang@vivo.com?part=8

Re: [PATCH 8/9] watchdog: sama5d4: Remove redundant dev_err()

[Alexandre Belloni]

On 22/07/2026 16:42:16+0800, Pan Chuang wrote:
> Since commit
> 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
> devm_request_irq() automatically logs detailed error messages on
> failure. Remove the now-redundant driver-specific dev_err() call.
> 
> Signed-off-by: Pan Chuang <panchuang@vivo.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/watchdog/sama5d4_wdt.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
> index 704b786cc2ec..ecdd9aeab79c 100644
> --- a/drivers/watchdog/sama5d4_wdt.c
> +++ b/drivers/watchdog/sama5d4_wdt.c
> @@ -291,10 +291,8 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
>  		ret = devm_request_irq(dev, irq, sama5d4_wdt_irq_handler,
>  				       IRQF_SHARED | IRQF_IRQPOLL |
>  				       IRQF_NO_SUSPEND, pdev->name, pdev);
> -		if (ret) {
> -			dev_err(dev, "cannot register interrupt handler\n");
> +		if (ret)
>  			return ret;
> -		}
>  	}
>  
>  	watchdog_init_timeout(wdd, wdt_timeout, dev);
> -- 
> 2.34.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 9/9] watchdog: sprd: Remove redundant dev_err()

[Pan Chuang]

Since commit
55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"),
devm_request_irq() automatically logs detailed error messages on
failure. Remove the now-redundant driver-specific dev_err() call.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/watchdog/sprd_wdt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/watchdog/sprd_wdt.c b/drivers/watchdog/sprd_wdt.c
index aacf04616fef..616b4d7f21d7 100644
--- a/drivers/watchdog/sprd_wdt.c
+++ b/drivers/watchdog/sprd_wdt.c
@@ -293,10 +293,8 @@ static int sprd_wdt_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(dev, wdt->irq, sprd_wdt_isr, IRQF_NO_SUSPEND,
 			       "sprd-wdt", (void *)wdt);
-	if (ret) {
-		dev_err(dev, "failed to register irq\n");
+	if (ret)
 		return ret;
-	}
 
 	wdt->wdd.info = &sprd_wdt_info;
 	wdt->wdd.ops = &sprd_wdt_ops;
-- 
2.34.1

Re: [PATCH 9/9] watchdog: sprd: Remove redundant dev_err()

[sashiko-bot]

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

Pre-existing issues:
- [High] Incorrect devres ordering leaves the IRQ handler active while device clocks are disabled, risking a Data Abort (kernel panic).
--

--- Patch [9]: [PATCH 9/9] watchdog: sprd: Remove redundant dev_err() ---
commit a68eda5f5b396a6d33ede72c1a4cc217df36d52f
Author: Pan Chuang <panchuang@vivo.com>

watchdog: sprd: Remove redundant dev_err()

This commit removes a redundant driver-specific dev_err() call after
devm_request_irq(). The core already logs detailed error messages on
failure.

> diff --git a/drivers/watchdog/sprd_wdt.c b/drivers/watchdog/sprd_wdt.c
> index aacf04616fefe..616b4d7f21d78 100644
> --- a/drivers/watchdog/sprd_wdt.c
> +++ b/drivers/watchdog/sprd_wdt.c
> @@ -293,10 +293,8 @@ static int sprd_wdt_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(dev, wdt->irq, sprd_wdt_isr, IRQF_NO_SUSPEND,
>  			       "sprd-wdt", (void *)wdt);

[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition
with the devres ordering.

Since devm_request_irq() is called before sprd_wdt_enable(), could a pending
interrupt (e.g., from the bootloader) trigger sprd_wdt_isr() while the device
clocks are still disabled?

Also, since devres teardown is LIFO, sprd_wdt_disable() (registered further
down via devm_add_action_or_reset()) will execute before the IRQ is freed
on probe failure or device removal. This leaves a window where the ISR could
fire and access registers with clocks disabled:

sprd_wdt_isr() {
	...
	sprd_wdt_unlock(wdt->base);
	writel_relaxed(SPRD_WDT_INT_CLEAR_BIT, wdt->base + SPRD_WDT_INT_CLR);
	...
}

Would it be safer to request the IRQ after the clocks are enabled and the
disable action is registered, to prevent a potential Data Abort?

> -	if (ret) {
> -		dev_err(dev, "failed to register irq\n");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	wdt->wdd.info = &sprd_wdt_info;
>  	wdt->wdd.ops = &sprd_wdt_ops;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084218.79073-1-panchuang@vivo.com?part=9