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-24 21:43:33
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

Possibly related (same subject, not in this thread)

Hello!

On 1/24/22 6:01 PM, Andy Shevchenko wrote:
quoted
quoted
quoted
quoted
quoted
quoted
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.
And similar number of people are on the other side.
If someone already opposed to the renaming (and not only the name) I
must have missed that.

So you think it's a good idea to keep the name
platform_get_irq_optional() despite the "not found" value returned by it
isn't usable as if it were a normal irq number?
I meant that on the other side people who are in favour of Sergey's patch.
Since that I commented already that I opposed the renaming being a standalone
change.

Do you agree that we have several issues with platform_get_irq*() APIs?
[...]
quoted
quoted
2. The vIRQ0 handling: a) WARN() followed by b) returned value 0
I'm happy with the vIRQ0 handling. Today platform_get_irq() and it's
silent variant returns either a valid and usuable irq number or a
negative error value. That's totally fine.
It might return 0.
Actually it seems that the WARN() can only be issued in two cases:
- SPARC with vIRQ0 in one of the array member
- fallback to ACPI for GPIO IRQ resource with index 0
   You have probably missed the recent discovery that arch/sh/boards/board-aps4*.c
causes IRQ0 to be passed as a direct IRQ resource?
But the latter is bogus, because it would mean a bug in the ACPI code.
   Worth changing >= 0 to > 0 there, maybe?
The bottom line here is the SPARC case. Anybody familiar with the platform
can shed a light on this. If there is no such case, we may remove warning
along with ret = 0 case from platfrom_get_irq().
   I'm afraid you're too fast here... :-)
   We'll have a really hard time if we continue to allow IRQ0 to be returned by
platform_get_irq() -- we'll have oto fileter it out in the callers then...
quoted
quoted
3. The specific cookie for "IRQ not found, while no error happened" case
Not sure what you mean here. I have no problem that a situation I can
cope with is called an error for the query function. I just do error
handling and continue happily. So the part "while no error happened" is
irrelevant to me.
I meant that instead of using special error code, 0 is very much good for
the cases when IRQ is not found. It allows to distinguish -ENXIO from the
low layer from -ENXIO with this magic meaning.
   I don't see how -ENXIO can trickle from the lower layers, frankly...

[...]

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