Messages in this thread | | | Subject | Re: [PATCH] driver core: platform: Rename platform_get_irq_optional() to platform_get_irq_silent() | From | Sergey Shtylyov <> | Date | Tue, 25 Jan 2022 00:02:06 +0300 |
| |
Hello!
On 1/24/22 6:01 PM, Andy Shevchenko wrote:
>>>>>>> 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? [...] >>> 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...
>>> 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
| |