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

Re: [PATCH] driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent()

From: Sergey Shtylyov <hidden>
Date: 2022-01-14 17:55:25
Also in: alsa-devel, kvm, linux-edac, linux-gpio, linux-i2c, linux-iio, linux-mediatek, linux-mmc, linux-pm, linux-pwm, linux-renesas-soc, linux-serial, linux-spi, lkml, netdev, platform-driver-x86

Possibly related (same subject, not in this thread)

On 1/14/22 12:42 AM, Florian Fainelli wrote:
quoted
The subsystems regulator, clk and gpio have the concept of a dummy
resource. For regulator, clk and gpio there is a semantic difference
between the regular _get() function and the _get_optional() variant.
(One might return the dummy resource, the other won't. Unfortunately
which one implements which isn't the same for these three.) The
difference between platform_get_irq() and platform_get_irq_optional() is
only that the former might emit an error message and the later won't.

To prevent people's expectations that there is a semantic difference
between these too, rename platform_get_irq_optional() to
platform_get_irq_silent() to make the actual difference more obvious.

The #define for the old name can and should be removed once all patches
currently in flux still relying on platform_get_irq_optional() are
fixed.

Signed-off-by: Uwe Kleine-König <redacted>
[...]
quoted
quoted
quoted
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.
It'd certainly be good to name anything that doesn't correspond to one
of the existing semantics for the API (!) something different rather
than adding yet another potentially overloaded meaning.
It seems we're (at least) three who agree about this. Here is a patch
fixing the name.
From an API naming perspective this does not make much sense anymore with the name chosen,
it is understood that whent he function is called platform_get_irq_optional(), optional applies
to the IRQ. An optional IRQ is something people can reason about because it makes sense.
   Right! :-)
What is a a "silent" IRQ however? It does not apply to the object it is trying to fetch to
anymore, but to the message that may not be printed in case the resource failed to be obtained,
because said resource is optional. Woah, that's quite a stretch.
   Right again! :-)
Following the discussion and original 2 patches set from Sergey, it is not entirely clear to me
anymore what is it that we are trying to fix.
   Andy and me tried to fix the platform_get_irq[_byname]_optional() value, corresponding to
a missing (optional) IRQ resource from -ENXIO to 0, in order to keep the callers error code
agnostic. This change completely aligns e.g. platform_get_irq_optional() with clk_get_optional()
and gpiod_get_optional()...
   Unforunately, we can't "fix" request_irq() and company to treat 0 as missing IRQ -- they have
to keep the ability to get called from the arch/ code (that doesn't use platform_get_irq(), etc.
I nearly forgot, I would paint it blue, sky blue, not navy blue, not light blue ;)
   :-)

PS: Florian, something was wrong with your mail client -- I had to manually wrap your quotes,
else there were super long unbroken paragraphs...

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help