Re: [PATCH v2 1/2] usb: chipidea: ci_hdrc_imx: Propagate errors from optional IRQ lookup

[sashiko-bot@kernel.org]

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

New issues:
- [High] The newly added error path for `platform_get_irq_optional()` bypasses `usb_phy_shutdown()`, potentially leaking the PHY initialization.

Pre-existing issues:
- [High] Pre-existing error handlers for IRQ allocation skip `usb_phy_shutdown()`, potentially leaking the initialized PHY.
--

--- Patch [1]: [PATCH v2 1/2] usb: chipidea: ci_hdrc_imx: Propagate errors from optional IRQ lookup ---
commit 6c82c9cecd7d9e9567b9f2902520e1c741b42e44
Author: bui duc phuc <phucduc.bui@gmail.com>

usb: chipidea: ci_hdrc_imx: Propagate errors from optional IRQ lookup

platform_get_irq_optional() returns a positive IRQ number on success or
a negative error code on failure. For an optional IRQ, -ENXIO indicates
that no IRQ is available, while other errors should be propagated.

Link: https://lore.kernel.org/all/20260806065254.28204-1-phucduc.bui@gmail.com/

> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 56d2ba824a0b2..7387c9903e800 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -525,6 +525,10 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  		data->supports_runtime_pm = true;
>  
>  	data->wakeup_irq = platform_get_irq_optional(pdev, 1);
> +	if (data->wakeup_irq < 0 && data->wakeup_irq != -ENXIO) {
> +		ret = data->wakeup_irq;
> +		goto err_clk;
> +	}

[Severity: High]
Does this jump skip the necessary cleanup for the USB PHY?

If usb_phy_init() succeeded earlier in ci_hdrc_imx_probe() and
platform_get_irq_optional() returns an error like -EPROBE_DEFER,
jumping directly to err_clk bypasses the phy_shutdown label:

ci_hdrc_imx_probe()
...
phy_shutdown:
	if (data->override_phy_control)
		usb_phy_shutdown(data->phy);
err_clk:

Can this leak the initialized PHY state, regulators, or clocks, especially
since -EPROBE_DEFER can be hit multiple times? Should this be goto
phy_shutdown instead?

>  	if (data->wakeup_irq > 0) {
>  		irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", pdata.name);
>  		if (!irq_name) {

[Severity: High]
This is a pre-existing issue, but do the error handlers for IRQ allocation
also incorrectly skip the PHY shutdown?

If devm_kasprintf() fails or if devm_request_threaded_irq() fails below:

ci_hdrc_imx_probe()
...
		ret = devm_request_threaded_irq(dev, data->wakeup_irq,
...
		if (ret)
			goto err_clk;

The error paths jump to err_clk, missing the phy_shutdown cleanup.
Could this lead to similar resource leaks if the USB PHY was successfully
initialized earlier?

> 			ret = dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
> 			goto err_clk;
> 		}

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