Re: [PATCH v2 1/2] watchdog: orion_wdt: Propagate errors from optional IRQ lookup

[email protected]
Newsgroups org.kernel.vger.linux-watchdog
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning directly on `platform_get_irq_optional` error bypasses clock cleanup, resulting in a resource leak.
--

--- Patch [1]: [PATCH v2 1/2] watchdog: orion_wdt: Propagate errors from optional IRQ lookup ---
commit 2b23b971e11df73076724d1b0329016fb3313456
Author: bui duc phuc <[email protected]>

watchdog: orion_wdt: Propagate errors from optional IRQ lookup

This commit updates the optional IRQ handling in the probe function. Instead
of continuing without an IRQ on all errors, it propagates errors other than
-ENXIO (such as -EPROBE_DEFER or -EINVAL) returned by
platform_get_irq_optional().

Link: https://lore.kernel.org/all/[email protected]/

> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> --- 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 < 0 && irq != -ENXIO)
> +		return irq;

[Severity: High]
Does this early return bypass the clock cleanup? 

Earlier in orion_wdt_probe(), the hardware clock is initialized and enabled by
dev->data->clock_init(pdev, dev). The other error paths in this function jump
to the disable_clk label to clean it up.

If we return directly here when an error like -EPROBE_DEFER occurs, it appears
we might leak the hardware clock reference and its enablement state because we
bypass clk_disable_unprepare() and clk_put() in the disable_clk error path. 
This could lead to repeated resource leaks if probe deferral happens multiple 
times.

Should this use goto disable_clk instead of returning directly?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.