* [PATCH 1/3] platform/x86: hp: hp_accel: Propagate errors from optional IRQ lookup
@ 2026-08-10 5:26 phucduc.bui
2026-08-10 5:26 ` [PATCH 2/3] platform/x86: intel: punit_ipc: " phucduc.bui
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: phucduc.bui @ 2026-08-10 5:26 UTC (permalink / raw)
To: Eric Piel, Hans de Goede, ilpo.jarvinen, Zha Qipeng
Cc: Mika Westerberg, Andy Shevchenko, platform-driver-x86,
linux-kernel, bui duc phuc
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 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 <phucduc.bui@gmail.com>
---
drivers/platform/x86/hp/hp_accel.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/platform/x86/hp/hp_accel.c b/drivers/platform/x86/hp/hp_accel.c
index 39b73dc473f1..8f6b03196540 100644
--- a/drivers/platform/x86/hp/hp_accel.c
+++ b/drivers/platform/x86/hp/hp_accel.c
@@ -309,6 +309,8 @@ static int lis3lv02d_probe(struct platform_device *device)
/* obtain IRQ number of our device from ACPI */
ret = platform_get_irq_optional(device, 0);
+ if (ret < 0 && ret != -ENXIO)
+ return ret;
if (ret > 0)
lis3_dev.irq = ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] platform/x86: intel: punit_ipc: Propagate errors from optional IRQ lookup
2026-08-10 5:26 [PATCH 1/3] platform/x86: hp: hp_accel: Propagate errors from optional IRQ lookup phucduc.bui
@ 2026-08-10 5:26 ` phucduc.bui
2026-08-10 7:00 ` Andy Shevchenko
2026-08-10 5:26 ` [PATCH 3/3] platform/x86: intel_scu_ipc: Handle " phucduc.bui
2026-08-10 6:50 ` [PATCH 1/3] platform/x86: hp: hp_accel: Propagate " Andy Shevchenko
2 siblings, 1 reply; 7+ messages in thread
From: phucduc.bui @ 2026-08-10 5:26 UTC (permalink / raw)
To: Eric Piel, Hans de Goede, ilpo.jarvinen, Zha Qipeng
Cc: Mika Westerberg, Andy Shevchenko, platform-driver-x86,
linux-kernel, bui duc phuc
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 optional IRQ is available and the driver can fall back to polling
mode. Other errors should be propagated so that the caller can handle
them appropriately.
Treat -ENXIO as the only case where the driver falls back to polling
mode, and propagate all other errors returned by
platform_get_irq_optional().
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
drivers/platform/x86/intel/punit_ipc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/platform/x86/intel/punit_ipc.c b/drivers/platform/x86/intel/punit_ipc.c
index 6d770b950dfb..590b145fd5ca 100644
--- a/drivers/platform/x86/intel/punit_ipc.c
+++ b/drivers/platform/x86/intel/punit_ipc.c
@@ -245,6 +245,8 @@ static int intel_punit_ipc_probe(struct platform_device *pdev)
irq = platform_get_irq_optional(pdev, 0);
if (irq < 0) {
+ if (irq != -ENXIO)
+ return irq;
dev_warn(&pdev->dev, "Invalid IRQ, using polling mode\n");
} else {
ret = devm_request_irq(&pdev->dev, irq, intel_punit_ioc,
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] platform/x86: intel_scu_ipc: Handle errors from optional IRQ lookup
2026-08-10 5:26 [PATCH 1/3] platform/x86: hp: hp_accel: Propagate errors from optional IRQ lookup phucduc.bui
2026-08-10 5:26 ` [PATCH 2/3] platform/x86: intel: punit_ipc: " phucduc.bui
@ 2026-08-10 5:26 ` phucduc.bui
2026-08-10 7:04 ` Andy Shevchenko
2026-08-10 6:50 ` [PATCH 1/3] platform/x86: hp: hp_accel: Propagate " Andy Shevchenko
2 siblings, 1 reply; 7+ messages in thread
From: phucduc.bui @ 2026-08-10 5:26 UTC (permalink / raw)
To: Eric Piel, Hans de Goede, ilpo.jarvinen, Zha Qipeng
Cc: Mika Westerberg, Andy Shevchenko, platform-driver-x86,
linux-kernel, bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
platform_get_irq_optional() can return errors such as -EPROBE_DEFER,
but the driver currently stores the return value directly in
scu_data.irq and continues probing.
Propagate negative errors other than -ENXIO using dev_err_probe(), and
only assign the IRQ to scu_data.irq when a valid IRQ number is
returned.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
drivers/platform/x86/intel_scu_pltdrv.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel_scu_pltdrv.c b/drivers/platform/x86/intel_scu_pltdrv.c
index d5ab62cbf5cc..4686960c44ac 100644
--- a/drivers/platform/x86/intel_scu_pltdrv.c
+++ b/drivers/platform/x86/intel_scu_pltdrv.c
@@ -21,8 +21,14 @@ static int intel_scu_platform_probe(struct platform_device *pdev)
struct intel_scu_ipc_data scu_data = {};
struct intel_scu_ipc_dev *scu;
const struct resource *res;
+ int ret;
+
+ ret = platform_get_irq_optional(pdev, 0);
+ if (ret < 0 && ret != -ENXIO)
+ return ret;
+ if (ret > 0)
+ scu_data.irq = ret;
- scu_data.irq = platform_get_irq_optional(pdev, 0);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -ENOMEM;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] platform/x86: hp: hp_accel: Propagate errors from optional IRQ lookup
2026-08-10 5:26 [PATCH 1/3] platform/x86: hp: hp_accel: Propagate errors from optional IRQ lookup phucduc.bui
2026-08-10 5:26 ` [PATCH 2/3] platform/x86: intel: punit_ipc: " phucduc.bui
2026-08-10 5:26 ` [PATCH 3/3] platform/x86: intel_scu_ipc: Handle " phucduc.bui
@ 2026-08-10 6:50 ` Andy Shevchenko
2 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2026-08-10 6:50 UTC (permalink / raw)
To: phucduc.bui
Cc: Eric Piel, Hans de Goede, ilpo.jarvinen, Zha Qipeng,
Mika Westerberg, Andy Shevchenko, platform-driver-x86,
linux-kernel
On Mon, Aug 10, 2026 at 8:26 AM <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 optional IRQ is available, while other errors should be propagated.
>
> Propagate all error codes returned by platform_get_irq_optional() other
> than -ENXIO.
Please, slow down with these patches.
Always try first a single or maximum a couple of patches of a kind to
see the comments, now you have to reconsider all what you already have
and basically wasted your time (instead of doing this better). See my
other reply in the other thread for the same problem.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] platform/x86: intel: punit_ipc: Propagate errors from optional IRQ lookup
2026-08-10 5:26 ` [PATCH 2/3] platform/x86: intel: punit_ipc: " phucduc.bui
@ 2026-08-10 7:00 ` Andy Shevchenko
2026-08-11 3:30 ` Bui Duc Phuc
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2026-08-10 7:00 UTC (permalink / raw)
To: phucduc.bui
Cc: Eric Piel, Hans de Goede, ilpo.jarvinen, Zha Qipeng,
Mika Westerberg, Andy Shevchenko, platform-driver-x86,
linux-kernel
On Mon, Aug 10, 2026 at 8:26 AM <phucduc.bui@gmail.com> wrote:
> 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 and the driver can fall back to polling
> mode. Other errors should be propagated so that the caller can handle
> them appropriately.
>
> Treat -ENXIO as the only case where the driver falls back to polling
> mode, and propagate all other errors returned by
> platform_get_irq_optional().
...
> irq = platform_get_irq_optional(pdev, 0);
> if (irq < 0) {
> + if (irq != -ENXIO)
> + return irq;
> dev_warn(&pdev->dev, "Invalid IRQ, using polling mode\n");
I can admit that the warning message is not comprehensive, but after
this patch it obviously makes a regression. A previously working
driver for even some Linux failures (besides deferred probe) now
becomes unusable. Have you studied the possible error codes returned
by platform_get_irq_optional()? What are they? Perhaps you need to
improve the documentation of that API?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] platform/x86: intel_scu_ipc: Handle errors from optional IRQ lookup
2026-08-10 5:26 ` [PATCH 3/3] platform/x86: intel_scu_ipc: Handle " phucduc.bui
@ 2026-08-10 7:04 ` Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2026-08-10 7:04 UTC (permalink / raw)
To: phucduc.bui
Cc: Eric Piel, Hans de Goede, ilpo.jarvinen, Zha Qipeng,
Mika Westerberg, Andy Shevchenko, platform-driver-x86,
linux-kernel
On Mon, Aug 10, 2026 at 8:26 AM <phucduc.bui@gmail.com> wrote:
> platform_get_irq_optional() can return errors such as -EPROBE_DEFER,
> but the driver currently stores the return value directly in
> scu_data.irq and continues probing.
>
> Propagate negative errors other than -ENXIO using dev_err_probe(), and
> only assign the IRQ to scu_data.irq when a valid IRQ number is
> returned.
You haven't read the code, have you?
Check the implementation of __intel_scu_ipc_register() in
drivers/platform/x86/intel_scu_ipc.c and act accordingly.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] platform/x86: intel: punit_ipc: Propagate errors from optional IRQ lookup
2026-08-10 7:00 ` Andy Shevchenko
@ 2026-08-11 3:30 ` Bui Duc Phuc
0 siblings, 0 replies; 7+ messages in thread
From: Bui Duc Phuc @ 2026-08-11 3:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Eric Piel, Hans de Goede, ilpo.jarvinen, Zha Qipeng,
Mika Westerberg, Andy Shevchenko, platform-driver-x86,
linux-kernel
Hi Andy,
Thank you for your review .
>
> > irq = platform_get_irq_optional(pdev, 0);
> > if (irq < 0) {
> > + if (irq != -ENXIO)
> > + return irq;
> > dev_warn(&pdev->dev, "Invalid IRQ, using polling mode\n");
>
> I can admit that the warning message is not comprehensive, but after
> this patch it obviously makes a regression. A previously working
> driver for even some Linux failures (besides deferred probe) now
> becomes unusable. Have you studied the possible error codes returned
> by platform_get_irq_optional()? What are they? Perhaps you need to
> improve the documentation of that API?
>
So I understand you'd prefer to keep the current behavior of the function,
to make sure existing systems keep working fine?
However, I think if this IRQs is truly meant to be optional, then any error
other than -ENXIO should be captured and returned
so that developers can actually investigate and fix the underlying issue.
Silently swallowing an error and falling back to polling doesn't
really guarantee
the system is working correctly or safely either, does it?
That said, if you'd rather keep the current implementation as-is,
then I think it would make more sense to switch to platform_get_irq() instead.
That would more accurately reflect that this driver doesn't actually
treat the IRQ
as optional, rather than using an "optional" lookup function while implementing
it as if it weren't optional.
https://elixir.bootlin.com/linux/v7.2-rc6/source/drivers/base/platform.c#L301
Best regards,
Phuc
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-11 3:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-10 5:26 [PATCH 1/3] platform/x86: hp: hp_accel: Propagate errors from optional IRQ lookup phucduc.bui
2026-08-10 5:26 ` [PATCH 2/3] platform/x86: intel: punit_ipc: " phucduc.bui
2026-08-10 7:00 ` Andy Shevchenko
2026-08-11 3:30 ` Bui Duc Phuc
2026-08-10 5:26 ` [PATCH 3/3] platform/x86: intel_scu_ipc: Handle " phucduc.bui
2026-08-10 7:04 ` Andy Shevchenko
2026-08-10 6:50 ` [PATCH 1/3] platform/x86: hp: hp_accel: Propagate " Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.