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
