Thread (92 messages) 92 messages, 12 authors, 2022-01-25

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

From: Uwe Kleine-König <hidden>
Date: 2022-01-13 11:09:56
Also in: alsa-devel, kvm, linux-edac, linux-gpio, linux-i2c, linux-iio, linux-mediatek, linux-mmc, linux-phy, linux-pm, linux-pwm, linux-renesas-soc, linux-serial, linux-spi, lkml, platform-driver-x86

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:
quoted
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 :-)
quoted
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/ |

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help