Re: [PATCH 1/2] platform: make platform_get_irq_optional() optional

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

On Wed, Jan 12, 2022 at 10:31:21PM +0100, Uwe Kleine-König wrote:
> On Wed, Jan 12, 2022 at 11:27:02AM +0100, Geert Uytterhoeven wrote:

(Do we really need *all* the CCs here?)

> That convinces me, that platform_get_irq_optional() is a bad name. The
> only difference to platform_get_irq is that it's silent. And returning
> a dummy irq value (which would make it aligned with the other _optional
> functions) isn't possible.

There is regulator_get_optional() which is I believe the earliest of
these APIs, it doesn't return a dummy either (and is silent too) - this
is because regulator_get() does return a dummy since it's the vastly
common case that regulators must be physically present and them not
being found is due to there being an error in the system description.
It's unfortunate that we've ended up with these two different senses for
_optional(), people frequently get tripped up by it.

> > To me it sounds much more logical for the driver to check if an
> > optional irq is non-zero (available) or zero (not available), than to
> > sprinkle around checks for -ENXIO. In addition, you have to remember
> > that this one returns -ENXIO, while other APIs use -ENOENT or -ENOSYS
> > (or some other error code) to indicate absence. I thought not having
> > to care about the actual error code was the main reason behind the
> > introduction of the *_optional() APIs.

> No, the main benefit of gpiod_get_optional() (and clk_get_optional()) is
> that you can handle an absent GPIO (or clk) as if it were available.

Similarly for the regulator API, kind of.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]