perf/arm-smmuv3: Propagate errors from optional IRQ lookup

PATCH In review [email protected] 2 participants 10 messages Aug 11 – Oct 7 Arm PMU profiling and debugging

lore

Share & tools

lore

[email protected]

author

2026-08-11 04:19 UTC #0 lore raw

PATCH 1/1 perf/arm-smmuv3: 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 optional IRQ is available, while other errors should be propagated.

Propagate all error codes returned by platform_get_irq_optional() other than -ENXIO.

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

---
 drivers/perf/arm_smmuv3_pmu.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/perf/arm_smmuv3_pmu.c +2 -0

index 621f02a7f43b..6c942697e57f 100644
@@ -893,6 +893,8 @@ static int smmu_pmu_probe(struct platform_device *pdev)
 	}
 
 	irq = platform_get_irq_optional(pdev, 0);
+	if (irq < 0 && irq != -ENXIO)
+		return irq;
 	if (irq > 0)
 		smmu_pmu->irq = irq;

Signature · 1 line

2.43.0

Will Deacon

maintainer

2026-10-02 16:43 UTC #1 lore raw

Quoted 19 lines · On Tue, Aug 11, 2026 at 11:19:34AM +0700, [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 optional IRQ is available, while other errors should be propagated.
Propagate all error codes returned by platform_get_irq_optional() other than -ENXIO.
Signed-off-by bui duc phuc <[email protected]>
---
 drivers/perf/arm_smmuv3_pmu.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/perf/arm_smmuv3_pmu.c +0 -0
index 621f02a7f43b..6c942697e57f 100644

drivers/perf/arm_smmuv3_pmu.c static int smmu_pmu_probe(struct platform_device *pdev) · line 897

Quoted 5 lines

drivers/perf/arm_smmuv3_pmu.c +2 -0

@@ -893,6 +893,8 @@ static int smmu_pmu_probe(struct platform_device *pdev)
 	}
 
 	irq = platform_get_irq_optional(pdev, 0);
+	if (irq < 0 && irq != -ENXIO)
+		return irq;

Not sure about this. If it's optional, why should we bail the probe if we don't manage to get an irq? Surely it's better to continue without the interrupt in that case?

Will

Bui Duc Phuc

author

2026-10-03 00:07 UTC #2 lore raw

Hi Will,

Thank you for your review.

Quoted 5 lines

Not sure about this. If it's optional, why should we bail the probe if we don't manage to get an irq? Surely it's better to continue without the interrupt in that case?

platform_get_irq_optional() returns -ENXIO when no IRQ is available. Other negative return values indicate errors, including -EPROBE_DEFER. In the case of -EPROBE_DEFER, we should propagate the error so that the probe can be retried later.

Best regard,
Phuc

Will Deacon

maintainer

2026-10-03 07:43 UTC #3 lore raw

Quoted 9 lines · Bui Duc Phuc

Quoted 4 lines
Not sure about this. If it's optional, why should we bail the probe if we don't manage to get an irq? Surely it's better to continue without the interrupt in that case?
platform_get_irq_optional() returns -ENXIO when no IRQ is available. Other negative return values indicate errors, including -EPROBE_DEFER. In the case of -EPROBE_DEFER, we should propagate the error so that the probe can be retried later.

Thanks, The -EPROBE_DEFER case seems more compelling, so perhaps we should check for the expliitly (because it won't fail the probe altogether)?

Will

Bui Duc Phuc

author

2026-10-03 11:12 UTC #4 lore raw

Hi Will,

Quoted 5 lines

Quoted 5 lines
Quoted 4 lines
Not sure about this. If it's optional, why should we bail the probe if we don't manage to get an irq? Surely it's better to continue without the interrupt in that case?

Quoted 8 lines

Quoted 4 lines
platform_get_irq_optional() returns -ENXIO when no IRQ is available. Other negative return values indicate errors, including -EPROBE_DEFER. In the case of -EPROBE_DEFER, we should propagate the error so that the probe can be retried later.
Thanks, The -EPROBE_DEFER case seems more compelling, so perhaps we should check for the expliitly (because it won't fail the probe altogether)?

The idea of the _optional getters is that only the "not present" case is turned into a special value (-ENXIO here, NULL for devm_clk_get_optional()), while real errors are still reported to the caller.

Besides -EPROBE_DEFER, platform_get_irq_optional() can return other negative error values depending on how the IRQ is obtained. These indicate an actual error rather than the IRQ simply being absent. Ignoring them would leave the device running without its interrupt and no indication of what went wrong.

So I'd rather keep:

-----------------------------------------------------
irq = platform_get_irq_optional(pdev, 0);
if (irq < 0 && irq != -ENXIO)

        return irq;

-----------------------------------------------------

and continue without the IRQ only for -ENXIO. This is also the usual pattern for platform_get_irq_optional() users.

Best regards,
Phuc

Will Deacon

maintainer

2026-10-04 08:45 UTC #5 lore raw

Quoted 30 lines · Bui Duc Phuc

Hi Will,
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Not sure about this. If it's optional, why should we bail the probe if we don't manage to get an irq? Surely it's better to continue without the interrupt in that case?
Quoted 8 lines
Quoted 4 lines
platform_get_irq_optional() returns -ENXIO when no IRQ is available. Other negative return values indicate errors, including -EPROBE_DEFER. In the case of -EPROBE_DEFER, we should propagate the error so that the probe can be retried later.
Thanks, The -EPROBE_DEFER case seems more compelling, so perhaps we should check for the expliitly (because it won't fail the probe altogether)?
The idea of the _optional getters is that only the "not present" case is turned into a special value (-ENXIO here, NULL for devm_clk_get_optional()), while real errors are still reported to the caller.
Besides -EPROBE_DEFER, platform_get_irq_optional() can return other negative error values depending on how the IRQ is obtained. These indicate an actual error rather than the IRQ simply being absent. Ignoring them would leave the device running without its interrupt and no indication of what went wrong.
So I'd rather keep:

Quoted 8 lines

-----------------------------------------------------
irq = platform_get_irq_optional(pdev, 0);
if (irq < 0 && irq != -ENXIO)
        return irq;
-----------------------------------------------------
and continue without the IRQ only for -ENXIO. This is also the usual pattern for platform_get_irq_optional() users.

Sorry, but how is failing the probe possibly better than continuing without the optional interrupt? Add a diagnostic if you like, but aborting the probe feels completely unnecessary to me.

Will

Bui Duc Phuc

author

2026-10-04 13:49 UTC #6 lore raw

Hi Will,

Quoted 5 lines

Sorry, but how is failing the probe possibly better than continuing without the optional interrupt? Add a diagnostic if you like, but aborting the probe feels completely unnecessary to me.

My understanding is that the driver is designed to be generic and support various hardware configurations, some with this resource (IRQ, GPIO, clock, ...) and some without.

If a configuration describes the resource, it means the board is designed to use it. For that hardware the "optional" nature of the driver no longer applies, so if we fail to get the resource the error should be returned.

A log alone is easy to miss, and the root cause still has to be found and fixed later anyway. Failing the probe makes the problem visible right away, when the board is being brought up.

Best regards,
Phuc

Will Deacon

maintainer

2026-10-04 14:42 UTC #7 lore raw

Quoted 9 lines · Bui Duc Phuc

Quoted 4 lines
Sorry, but how is failing the probe possibly better than continuing without the optional interrupt? Add a diagnostic if you like, but aborting the probe feels completely unnecessary to me.
My understanding is that the driver is designed to be generic and support various hardware configurations, some with this resource (IRQ, GPIO, clock, ...) and some without.

Quoted 8 lines

If a configuration describes the resource, it means the board is designed to use it. For that hardware the "optional" nature of the driver no longer applies, so if we fail to get the resource the error should be returned.
A log alone is easy to miss, and the root cause still has to be found and fixed later anyway. Failing the probe makes the problem visible right away, when the board is being brought up.

If you're doing bring-up, you should probably pay attention to the logs. If you're trying to use the device, you probably don't care about the interrupt.

Will

Bui Duc Phuc

author

2026-10-05 08:02 UTC #8 lore raw

Hi Will,

Quoted 14 lines

Quoted 14 lines
Quoted 4 lines
Sorry, but how is failing the probe possibly better than continuing without the optional interrupt? Add a diagnostic if you like, but aborting the probe feels completely unnecessary to me.
My understanding is that the driver is designed to be generic and support various hardware configurations, some with this resource (IRQ, GPIO, clock, ...) and some without.
If a configuration describes the resource, it means the board is designed to use it. For that hardware the "optional" nature of the driver no longer applies, so if we fail to get the resource the error should be returned.

Quoted 5 lines

Quoted 3 lines
A log alone is easy to miss, and the root cause still has to be found and fixed later anyway. Failing the probe makes the problem visible right away, when the board is being brought up.
If you're doing bring-up, you should probably pay attention to the logs.

OK. Then we should probably write it as:
-------------------------------------------------------------------------
irq = platform_get_irq_optional(pdev, 0);
if (irq < 0 && irq != -ENXIO)

        return dev_err_probe(pdev, irq, "failed to get irq\n");

-------------------------------------------------------------------------

Quoted 2 lines

If you're trying to use the device, you probably don't care about the interrupt.

If we're worried about returning an error here because it might cause the device probe to fail, then shouldn't we hide errors from all the other functions in probe() as well? :-)

Best regards,
Phuc

Will Deacon

maintainer

2026-10-07 14:19 UTC #9 lore raw

Quoted 18 lines · Bui Duc Phuc

Quoted 18 lines
Quoted 17 lines
Quoted 4 lines
Sorry, but how is failing the probe possibly better than continuing without the optional interrupt? Add a diagnostic if you like, but aborting the probe feels completely unnecessary to me.
My understanding is that the driver is designed to be generic and support various hardware configurations, some with this resource (IRQ, GPIO, clock, ...) and some without.
If a configuration describes the resource, it means the board is designed to use it. For that hardware the "optional" nature of the driver no longer applies, so if we fail to get the resource the error should be returned.
A log alone is easy to miss, and the root cause still has to be found and fixed later anyway. Failing the probe makes the problem visible right away, when the board is being brought up.

Quoted 7 lines

Quoted 1 line
If you're doing bring-up, you should probably pay attention to the logs.
OK. Then we should probably write it as:
-------------------------------------------------------------------------
irq = platform_get_irq_optional(pdev, 0);
if (irq < 0 && irq != -ENXIO)
        return dev_err_probe(pdev, irq, "failed to get irq\n");

Won't this bail on errors != ENXIO and != EPROBE_DEFER? I think we should only bail on EPROBE_DEFER. That's also more robust to changes in the error codes that platform_get_irq_optional() can return.

Quoted 8 lines

-------------------------------------------------------------------------
Quoted 2 lines
If you're trying to use the device, you probably don't care about the interrupt.
If we're worried about returning an error here because it might cause the device probe to fail, then shouldn't we hide errors from all the other functions in probe() as well? :-)

Not really. Only the irq is optional; we really can't continue if something like ioremap() fails.

Will