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

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

[phucduc.bui]

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

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.

Propagate errors such as -EPROBE_DEFER and -EINVAL instead of continuing
probe without the IRQ.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---

Link v1:
https://lore.kernel.org/all/20260806065254.28204-1-phucduc.bui@gmail.com/
Changes in v2:
 - Update the commit message and error handling.
 - Propagate all errors from platform_get_irq_optional() except -ENXIO,
   instead of only handling -EPROBE_DEFER.

 drivers/usb/chipidea/ci_hdrc_imx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 56d2ba824a0b..7387c9903e80 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;
+	}
 	if (data->wakeup_irq > 0) {
 		irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", pdata.name);
 		if (!irq_name) {
-- 
2.43.0

[PATCH v2 2/2] usb: chipidea: ci_hdrc_imx: Fix error handling when creating IRQ name

[phucduc.bui]

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

devm_kasprintf() can fail when allocating the wakeup IRQ name. The error
was logged with dev_err_probe(), but its return value was ignored and
probe could continue with a successful return value.

Store the error returned by dev_err_probe() in ret and jump to the error
path to propagate the failure.

Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/all/20260806065755.C9F361F000E9@smtp.kernel.org/
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---

 drivers/usb/chipidea/ci_hdrc_imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 7387c9903e80..ec2280ae82dc 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -532,7 +532,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 	if (data->wakeup_irq > 0) {
 		irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", pdata.name);
 		if (!irq_name) {
-			dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
+			ret = dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
 			goto err_clk;
 		}
 
-- 
2.43.0

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

[sashiko-bot]

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

Re: [PATCH v2 2/2] usb: chipidea: ci_hdrc_imx: Fix error handling when creating IRQ name

[sashiko-bot]

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

Pre-existing issues:
- [High] Error paths for wakeup IRQ allocation bypass the USB PHY shutdown, leading to a resource leak.
--

--- Patch [2]: [PATCH v2 2/2] usb: chipidea: ci_hdrc_imx: Fix error handling when creating IRQ name ---
commit 3a41f09eeb5ba5b2ec80f3833ced7293b9a2ffe6
Author: bui duc phuc <phucduc.bui@gmail.com>

usb: chipidea: ci_hdrc_imx: Fix error handling when creating IRQ name

This patch fixes a bug where the failure of devm_kasprintf() during wakeup
IRQ name allocation was logged but not propagated. It assigns the result of
dev_err_probe() to the return variable to properly fail the probe.

Link: https://lore.kernel.org/all/20260806065755.C9F361F000E9@smtp.kernel.org/

> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 7387c9903e800..ec2280ae82dc5 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -532,7 +532,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	if (data->wakeup_irq > 0) {
>  		irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", pdata.name);
>  		if (!irq_name) {
> -			dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
> +			ret = dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
>  			goto err_clk;

[Severity: High]
This is a pre-existing issue, but does this error path bypass the USB PHY
shutdown, leading to a resource leak?

If usb_phy_init() was called earlier in ci_hdrc_imx_probe(), jumping directly
to err_clk skips the phy_shutdown label. This leaks the PHY initialization
state (such as clocks and regulators).

If probe defers or fails here, this could cause regulator and clock enable
counts to increase endlessly on subsequent probe attempts.

Should this goto point to phy_shutdown instead?

>  		}
>

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

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

[Peter Chen]

On 26-08-07 13:56:18, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> 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.
> 
> Propagate errors such as -EPROBE_DEFER and -EINVAL instead of continuing
> probe without the IRQ.
> 
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
> 
> Link v1:
> https://lore.kernel.org/all/20260806065254.28204-1-phucduc.bui@gmail.com/
> Changes in v2:
>  - Update the commit message and error handling.
>  - Propagate all errors from platform_get_irq_optional() except -ENXIO,
>    instead of only handling -EPROBE_DEFER.
> 
>  drivers/usb/chipidea/ci_hdrc_imx.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 56d2ba824a0b..7387c9903e80 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;
> +	}
>  	if (data->wakeup_irq > 0) {
>  		irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", pdata.name);
>  		if (!irq_name) {
> -- 
> 2.43.0
> 

-- 

Thanks,
Peter Chen

Re: [PATCH v2 2/2] usb: chipidea: ci_hdrc_imx: Fix error handling when creating IRQ name

[Peter Chen]

On 26-08-07 13:56:19, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> devm_kasprintf() can fail when allocating the wakeup IRQ name. The error
> was logged with dev_err_probe(), but its return value was ignored and
> probe could continue with a successful return value.
> 
> Store the error returned by dev_err_probe() in ret and jump to the error
> path to propagate the failure.
> 
> Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/all/20260806065755.C9F361F000E9@smtp.kernel.org/
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
> 
>  drivers/usb/chipidea/ci_hdrc_imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 7387c9903e80..ec2280ae82dc 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -532,7 +532,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	if (data->wakeup_irq > 0) {
>  		irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", pdata.name);
>  		if (!irq_name) {
> -			dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
> +			ret = dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
>  			goto err_clk;
>  		}
>  
> -- 
> 2.43.0
> 

-- 

Thanks,
Peter Chen

Re: [PATCH v2 2/2] usb: chipidea: ci_hdrc_imx: Fix error handling when creating IRQ name

[Frank Li]

On Fri, Aug 07, 2026 at 01:56:19PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> devm_kasprintf() can fail when allocating the wakeup IRQ name. The error
> was logged with dev_err_probe(), but its return value was ignored and
> probe could continue with a successful return value.
>
> Store the error returned by dev_err_probe() in ret and jump to the error
> path to propagate the failure.
>
> Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/all/20260806065755.C9F361F000E9@smtp.kernel.org/
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>  drivers/usb/chipidea/ci_hdrc_imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 7387c9903e80..ec2280ae82dc 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -532,7 +532,7 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
>  	if (data->wakeup_irq > 0) {
>  		irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s:wakeup", pdata.name);
>  		if (!irq_name) {
> -			dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
> +			ret = dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
>  			goto err_clk;
>  		}
>
> --
> 2.43.0
>
>

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

[Frank Li]

On Fri, Aug 07, 2026 at 07:03:52AM +0000, sashiko-bot@kernel.org wrote:
> 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?


Please check this, make sense.

Frank

>
> >  	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

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

[Bui Duc Phuc]

Hi Frank,

Thanks for pointing this out.

> > [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?
>
>
> Please check this, make sense.
>

I agree that this makes sense. I'll make the change and send v3.

Best regards,
Phuc