[PATCH v2 1/2] thermal/drivers/rcar_gen3: Handle -ENXIO from optional IRQ lookup

[Bui Duc Phuc]

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