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

[sashiko-bot@kernel.org]

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