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

PATCH Superseded [email protected] 4 participants 9 messages Aug 7 – Aug 8

Subsystems (3)

USB Arm i.MX and Layerscape Chipidea USB high speed dual role controller

lore

Share & tools

lore

[email protected]

author

2026-08-07 06:56 UTC #0 lore raw

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

From: bui duc phuc <[email protected]>

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 <[email protected]>

---

Link v1:
https://lore.kernel.org/all/[email protected]/
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(+)

drivers/usb/chipidea/ci_hdrc_imx.c +4 -0

index 56d2ba824a0b..7387c9903e80 100644
@@ -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) {

Signature · 1 line

2.43.0

[email protected]

author

2026-08-07 06:57 UTC #1 lore raw

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

From: bui duc phuc <[email protected]>

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 <[email protected]>

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

Signed-off-by bui duc phuc <[email protected]>

---

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

drivers/usb/chipidea/ci_hdrc_imx.c +1 -1

index 7387c9903e80..ec2280ae82dc 100644
@@ -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;
 		}

Signature · 1 line

2.43.0

[email protected]

AI review

2026-08-07 07:07 UTC #2 lore raw

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

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 <[email protected]>

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/[email protected]/ open here

drivers/usb/chipidea/ci_hdrc_imx.c static int ci_hdrc_imx_probe(struct platform_device *pdev) · line 536

Quoted 11 lines

drivers/usb/chipidea/ci_hdrc_imx.c +1 -1

index 7387c9903e800..ec2280ae82dc5 100644
@@ -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?

Quoted 2 lines

drivers/usb/chipidea/ci_hdrc_imx.c +0 -0
 		}
 

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

Peter Chen

maintainer Acked-by on v2 2/2

2026-08-07 09:20 UTC #3 lore raw

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

In reply to [email protected]

Quoted 6 lines · On 26-08-07 13:56:19, [email protected] wrote:

From: bui duc phuc <[email protected]>
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.

Quoted 6 lines

Store the error returned by dev_err_probe() in ret and jump to the error path to propagate the failure.
Reported-by Sashiko Bot <[email protected]>
Link https://lore.kernel.org/all/[email protected]/ open here
Signed-off-by bui duc phuc <[email protected]>

Acked-by Peter Chen <[email protected]>

Peter

Quoted 21 lines

---

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

drivers/usb/chipidea/ci_hdrc_imx.c +1 -1

index 7387c9903e80..ec2280ae82dc 100644
@@ -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;
 		}

Signature · 1 line

2.43.0

Signature · 2 lines

Thanks,
Peter Chen

Frank Li

Reviewed-by on v2 2/2

2026-08-07 19:18 UTC #4 lore raw

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

In reply to [email protected]

Quoted 6 lines · On Fri, Aug 07, 2026 at 01:56:19PM +0700, [email protected] wrote:

From: bui duc phuc <[email protected]>
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.

Quoted 7 lines

Store the error returned by dev_err_probe() in ret and jump to the error path to propagate the failure.
Reported-by Sashiko Bot <[email protected]>
Link https://lore.kernel.org/all/[email protected]/ open here
Signed-off-by bui duc phuc <[email protected]>
---

Reviewed-by Frank Li <[email protected]>

Quoted 21 lines

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

drivers/usb/chipidea/ci_hdrc_imx.c +1 -2

index 7387c9903e80..ec2280ae82dc 100644
@@ -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

[email protected]

AI review

2026-08-07 07:03 UTC #5 lore raw

In reply to [email protected]

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 <[email protected]>

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

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

drivers/usb/chipidea/ci_hdrc_imx.c static int ci_hdrc_imx_probe(struct platform_device *pdev) · line 531

Quoted 12 lines

drivers/usb/chipidea/ci_hdrc_imx.c +4 -0

index 56d2ba824a0b2..7387c9903e800 100644
@@ -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?

drivers/usb/chipidea/ci_hdrc_imx.c static int ci_hdrc_imx_probe(struct platform_device *pdev) · line 534

Quoted 3 lines

drivers/usb/chipidea/ci_hdrc_imx.c +0 -0

 	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?

Quoted 3 lines

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Frank Li

2026-08-07 19:20 UTC #6 lore raw

Quoted 44 lines · On Fri, Aug 07, 2026 at 07:03:52AM +0000, [email protected] 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 <[email protected]>
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/[email protected]/ open here
drivers/usb/chipidea/ci_hdrc_imx.c static int ci_hdrc_imx_probe(struct platform_device *pdev) · line 531
Quoted 12 lines
drivers/usb/chipidea/ci_hdrc_imx.c +4 -0
index 56d2ba824a0b2..7387c9903e800 100644
@@ -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:

Quoted 7 lines

	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

Quoted 28 lines

drivers/usb/chipidea/ci_hdrc_imx.c static int ci_hdrc_imx_probe(struct platform_device *pdev) · line 534
Quoted 4 lines
drivers/usb/chipidea/ci_hdrc_imx.c +0 -0
 	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?
Quoted 3 lines
			ret = dev_err_probe(dev, -ENOMEM, "failed to create irq_name\n");
			goto err_clk;
		}
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Bui Duc Phuc

author

2026-08-08 09:14 UTC #7 lore raw

Hi Frank,

Thanks for pointing this out.

Quoted 14 lines

Quoted 14 lines
[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:

Quoted 7 lines

Quoted 3 lines
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

Peter Chen

maintainer Acked-by on v2 1/2

2026-08-07 09:16 UTC #8 lore raw

In reply to [email protected]

Quoted 10 lines · On 26-08-07 13:56:18, [email protected] wrote:

From: bui duc phuc <[email protected]>
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 <[email protected]>

Acked-by Peter Chen <[email protected]>

Peter

Quoted 30 lines

---
Link v1:
https://lore.kernel.org/all/[email protected]/
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(+)
drivers/usb/chipidea/ci_hdrc_imx.c +4 -0
index 56d2ba824a0b..7387c9903e80 100644
@@ -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) {
Signature · 1 line
2.43.0

Signature · 2 lines

Thanks,
Peter Chen