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

[Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>]

On Wed, Jan 12, 2022 at 09:45:25PM +0000, Mark Brown wrote:
> 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?)

It's probably counteractive to finding an agreement because there are
too many opinions on that matter. But I didn't dare to strip it down,
too :-)

> > 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.

Yeah, I tripped over that one already, too. And according to my counting
this results in three different senses now :-\ :

 a) regulator
    regulator_get returns a dummy, regulator_get_optional returns ERR_PTR(-ENODEV)
 b) clk + gpiod
    ..._get returns ERR_PTR(-ENODEV), ..._get_optional returns a dummy
 c) platform_get_irq()
    platform_get_irq_optional() is just a silent variant of
    platform_get_irq(); the return values are identical.
    
This is all very unfortunate. In my eyes b) is the most sensible
sense, but the past showed that we don't agree here. (The most annoying
part of regulator_get is the warning that is emitted that regularily
makes customers ask what happens here and if this is fixable.)

I think at least c) is easy to resolve because
platform_get_irq_optional() isn't that old yet and mechanically
replacing it by platform_get_irq_silent() should be easy and safe.
And this is orthogonal to the discussion if -ENOXIO is a sensible return
value and if it's as easy as it could be to work with errors on irq
lookups.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature