* [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO from optional IRQ lookup @ 2026-08-10 11:18 phucduc.bui 2026-08-10 11:18 ` [PATCH v2 2/2] thermal/drivers/brcmstb_thermal: Propagate errors " phucduc.bui 2026-08-10 12:19 ` [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO " Niklas Söderlund 0 siblings, 2 replies; 5+ messages in thread From: phucduc.bui @ 2026-08-10 11:18 UTC (permalink / raw) To: Markus Mayer, Broadcom internal kernel review list, rafael, Daniel Lezcano, Zhang Rui, Lukasz Luba, Florian Fainelli, niklas.soderlund, Geert Uytterhoeven, Magnus Damm Cc: linux-pm, linux-renesas-soc, linux-kernel, linux-arm-kernel, bui duc phuc From: bui duc phuc <phucduc.bui@gmail.com> rcar_gen3_thermal_request_irqs() may return -ENXIO when no optional IRQ is available. The caller currently treats all negative return values the same way, disabling hardware trip-point support and continuing probe. Distinguish -ENXIO from other errors so that the existing behavior is preserved when the optional IRQ is absent, while errors such as -EPROBE_DEFER, -EINVAL, and other failures are propagated to the caller. Signed-off-by: bui duc phuc <phucduc.bui@gmail.com> --- Link v1 : https://lore.kernel.org/all/20260807093058.42840-1-phucduc.bui@gmail.com/ Changes in v2: - Update the commit message. - Move error handling to the caller. drivers/thermal/renesas/rcar_gen3_thermal.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/thermal/renesas/rcar_gen3_thermal.c b/drivers/thermal/renesas/rcar_gen3_thermal.c index 94804816e9e1..04a57ff724f4 100644 --- a/drivers/thermal/renesas/rcar_gen3_thermal.c +++ b/drivers/thermal/renesas/rcar_gen3_thermal.c @@ -527,8 +527,11 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev) priv->info = of_device_get_match_data(dev); platform_set_drvdata(pdev, priv); - if (rcar_gen3_thermal_request_irqs(priv, pdev)) + ret = rcar_gen3_thermal_request_irqs(priv, pdev); + if (ret == -ENXIO) priv->ops.set_trips = NULL; + else if (ret) + return ret; pm_runtime_enable(dev); pm_runtime_get_sync(dev); -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] thermal/drivers/brcmstb_thermal: Propagate errors from optional IRQ lookup 2026-08-10 11:18 [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO from optional IRQ lookup phucduc.bui @ 2026-08-10 11:18 ` phucduc.bui 2026-08-10 12:19 ` [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO " Niklas Söderlund 1 sibling, 0 replies; 5+ messages in thread From: phucduc.bui @ 2026-08-10 11:18 UTC (permalink / raw) To: Markus Mayer, Broadcom internal kernel review list, rafael, Daniel Lezcano, Zhang Rui, Lukasz Luba, Florian Fainelli, niklas.soderlund, Geert Uytterhoeven, Magnus Damm Cc: linux-pm, linux-renesas-soc, linux-kernel, linux-arm-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 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> --- drivers/thermal/broadcom/brcmstb_thermal.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c b/drivers/thermal/broadcom/brcmstb_thermal.c index a9ffa596f7c0..477d078ce326 100644 --- a/drivers/thermal/broadcom/brcmstb_thermal.c +++ b/drivers/thermal/broadcom/brcmstb_thermal.c @@ -350,7 +350,9 @@ static int brcmstb_thermal_probe(struct platform_device *pdev) priv->thermal = thermal; irq = platform_get_irq_optional(pdev, 0); - if (irq >= 0) { + if (irq < 0 && irq != -ENXIO) + return irq; + if (irq > 0) { ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, brcmstb_tmon_irq_thread, IRQF_ONESHOT, -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO from optional IRQ lookup 2026-08-10 11:18 [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO from optional IRQ lookup phucduc.bui 2026-08-10 11:18 ` [PATCH v2 2/2] thermal/drivers/brcmstb_thermal: Propagate errors " phucduc.bui @ 2026-08-10 12:19 ` Niklas Söderlund 2026-08-11 2:02 ` Bui Duc Phuc 1 sibling, 1 reply; 5+ messages in thread From: Niklas Söderlund @ 2026-08-10 12:19 UTC (permalink / raw) To: phucduc.bui Cc: Markus Mayer, Broadcom internal kernel review list, rafael, Daniel Lezcano, Zhang Rui, Lukasz Luba, Florian Fainelli, Geert Uytterhoeven, Magnus Damm, linux-pm, linux-renesas-soc, linux-kernel, linux-arm-kernel Hi Bui, Thanks for your work. On 2026-08-10 18:18:40 +0700, phucduc.bui@gmail.com wrote: > From: bui duc phuc <phucduc.bui@gmail.com> > > rcar_gen3_thermal_request_irqs() may return -ENXIO when no optional > IRQ is available. The caller currently treats all negative return values > the same way, disabling hardware trip-point support and continuing probe. > > Distinguish -ENXIO from other errors so that the existing behavior is > preserved when the optional IRQ is absent, while errors such as > -EPROBE_DEFER, -EINVAL, and other failures are propagated to the > caller. > > Signed-off-by: bui duc phuc <phucduc.bui@gmail.com> > --- > > Link v1 : > https://lore.kernel.org/all/20260807093058.42840-1-phucduc.bui@gmail.com/ > Changes in v2: > - Update the commit message. > - Move error handling to the caller. > > drivers/thermal/renesas/rcar_gen3_thermal.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/renesas/rcar_gen3_thermal.c b/drivers/thermal/renesas/rcar_gen3_thermal.c > index 94804816e9e1..04a57ff724f4 100644 > --- a/drivers/thermal/renesas/rcar_gen3_thermal.c > +++ b/drivers/thermal/renesas/rcar_gen3_thermal.c > @@ -527,8 +527,11 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev) > priv->info = of_device_get_match_data(dev); > platform_set_drvdata(pdev, priv); > > - if (rcar_gen3_thermal_request_irqs(priv, pdev)) > + ret = rcar_gen3_thermal_request_irqs(priv, pdev); > + if (ret == -ENXIO) > priv->ops.set_trips = NULL; > + else if (ret) > + return ret; I'm not sure I like this. The old behavior is that for any reason we can't get the optional IRQ the driver still works, but without hardware trip points. While the new behavior is that if *something* other then the IRQ not being available we fail probe. What is the gain here? What benefit is there from the driver not providing thermal readouts if say for example getting the optional IRQ would return -EINVAL ? > > pm_runtime_enable(dev); > pm_runtime_get_sync(dev); > -- > 2.43.0 > -- Kind Regards, Niklas Söderlund ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO from optional IRQ lookup 2026-08-10 12:19 ` [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO " Niklas Söderlund @ 2026-08-11 2:02 ` Bui Duc Phuc 2026-08-11 7:28 ` Geert Uytterhoeven 0 siblings, 1 reply; 5+ messages in thread From: Bui Duc Phuc @ 2026-08-11 2:02 UTC (permalink / raw) To: Niklas Söderlund Cc: Markus Mayer, Broadcom internal kernel review list, rafael, Daniel Lezcano, Zhang Rui, Lukasz Luba, Florian Fainelli, Geert Uytterhoeven, Magnus Damm, linux-pm, linux-renesas-soc, linux-kernel, linux-arm-kernel Hi Niklas, Thank you for your feedback. > > - if (rcar_gen3_thermal_request_irqs(priv, pdev)) > > + ret = rcar_gen3_thermal_request_irqs(priv, pdev); > > + if (ret == -ENXIO) > > priv->ops.set_trips = NULL; > > + else if (ret) > > + return ret; > > I'm not sure I like this. > > The old behavior is that for any reason we can't get the optional IRQ > the driver still works, but without hardware trip points. > Regarding the old behavior, it was handled like this: ------------------------------ irq = platform_get_irq_optional(pdev, i); if (irq < 0) return irq; ------------------------------ As I understand it, this is essentially the behavior of platform_get_irq() itself: https://elixir.bootlin.com/linux/v7.2-rc6/source/drivers/base/platform.c#L301 This could be understood as rcar-gen3 not actually supporting an optional IRQ. If you still want to keep the old behavior, then I think we should switch to platform_get_irq() instead of platform_get_irq_optional(), since using the latter could give a misleading impression that rcar-gen3 supports an optional IRQ. Incidentally, I also don't quite understand why rcar-gen2 supports an optional IRQ while rcar-gen3 does not. Is there a hardware limitation on rcar-gen3 that requires the IRQ to be present, or is this just a difference in the driver implementation? https://elixir.bootlin.com/linux/v7.2-rc6/source/drivers/thermal/renesas/rcar_thermal.c#L422 > While the new behavior is that if *something* other then the IRQ not > being available we fail probe. What is the gain here? What benefit is > there from the driver not providing thermal readouts if say for example > getting the optional IRQ would return -EINVAL ? > Regarding the benefit of the new behavior: If rcar-gen3 actually supports an optional IRQ, I think the benefit is that it reflects the actual state of the system. It allows the developer to know that something went wrong and investigate or fix the underlying issue. In the case of `-EPROBE_DEFER`, the benefit is even clearer, as it allows the kernel to defer the probe and retry it later when the dependency becomes available. If we silently ignore the error as we do currently, the driver may still be able to provide thermal information as you mentioned. However, can we still guarantee the correctness and safety of the system? Best regards, Phuc ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO from optional IRQ lookup 2026-08-11 2:02 ` Bui Duc Phuc @ 2026-08-11 7:28 ` Geert Uytterhoeven 0 siblings, 0 replies; 5+ messages in thread From: Geert Uytterhoeven @ 2026-08-11 7:28 UTC (permalink / raw) To: Bui Duc Phuc Cc: Niklas Söderlund, Markus Mayer, Broadcom internal kernel review list, rafael, Daniel Lezcano, Zhang Rui, Lukasz Luba, Florian Fainelli, Geert Uytterhoeven, Magnus Damm, linux-pm, linux-renesas-soc, linux-kernel, linux-arm-kernel Hi Phuc, On Tue, 11 Aug 2026 at 04:02, Bui Duc Phuc <phucduc.bui@gmail.com> wrote: > > > - if (rcar_gen3_thermal_request_irqs(priv, pdev)) > > > + ret = rcar_gen3_thermal_request_irqs(priv, pdev); > > > + if (ret == -ENXIO) > > > priv->ops.set_trips = NULL; > > > + else if (ret) > > > + return ret; > > > > I'm not sure I like this. > > > > The old behavior is that for any reason we can't get the optional IRQ > > the driver still works, but without hardware trip points. > > Regarding the old behavior, it was handled like this: > ------------------------------ > irq = platform_get_irq_optional(pdev, i); > if (irq < 0) > return irq; > ------------------------------ > As I understand it, this is essentially the behavior of > platform_get_irq() itself: > > https://elixir.bootlin.com/linux/v7.2-rc6/source/drivers/base/platform.c#L301 Except that platform_get_irq() prints an error message, too, which we want to suppress. > This could be understood as rcar-gen3 not actually supporting an optional IRQ. > If you still want to keep the old behavior, then I think we should switch to > platform_get_irq() instead of platform_get_irq_optional(), since using > the latter > could give a misleading impression that rcar-gen3 supports an optional IRQ. > > Incidentally, I also don't quite understand why rcar-gen2 supports an > optional IRQ > while rcar-gen3 does not. Is there a hardware limitation on rcar-gen3 > that requires > the IRQ to be present, or is this just a difference in the driver > implementation? On R-Car Gen3, thermal has an interrupt. On R-Car Gen4, thermal does not have an interrupt, but routes this signal to the ECM. > Regarding the benefit of the new behavior: > > If rcar-gen3 actually supports an optional IRQ, I think the benefit is > that it reflects > the actual state of the system. It allows the developer to know that > something went > wrong and investigate or fix the underlying issue. In the case of > `-EPROBE_DEFER`, > the benefit is even clearer, as it allows the kernel to defer the > probe and retry it later > when the dependency becomes available. > > If we silently ignore the error as we do currently, the driver may > still be able to provide > thermal information as you mentioned. However, can we still guarantee > the correctness > and safety of the system? -EPROBE_DEFER should indeed be handled correctly. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-11 7:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-10 11:18 [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO from optional IRQ lookup phucduc.bui 2026-08-10 11:18 ` [PATCH v2 2/2] thermal/drivers/brcmstb_thermal: Propagate errors " phucduc.bui 2026-08-10 12:19 ` [PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO " Niklas Söderlund 2026-08-11 2:02 ` Bui Duc Phuc 2026-08-11 7:28 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox