Hi Sergey,
On Mon, Jan 24, 2022 at 10:02 PM Sergey Shtylyov s.shtylyov@omp.ru wrote:
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?
[...]
- 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?
So far no one reported seeing the big fat warning ;-)
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...
So far no one reported seeing the big fat warning?
- 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...
It might one day, leading to very hard to track bugs.
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds