On Sat, Jul 25, 2020 at 1:03 AM Serge Semin
[off-list ref] wrote:

According to the DW APB GPIO databook it can be configured to provide either a
combined IRQ line or multiple interrupt signals for each GPIO. It's up to
the platform which of those signals are connected to an embedded IRQ
controller. So I guess theoretically the array values can be sparse.

Anyway now I see it's rather problematic. I didn't forget about the sparse IRQs
array case. I just thought it would work out-of-box. Before getting your comment
and digging deeper into the IRQ subsystem I had thought that it wasn't a problem
passing invalid IRQ numbers to the irq_set_chained_handler_and_data() especially
seeing zero IRQ number was supposed to be considered as invalid. That method shall
just ignore the invalid IRQs since the method irq_to_desc() calling radix_tree_lookup()
will fail to find a descriptor with invalid IRQ value and return NULL. So after
getting a NULL irq_desc the method irq_set_chained_handler_and_data() would
have stopped setting the handler. But turns out it may work only for
CONFIG_SPARSE_IRQ. If that config isn't enabled, then a very first IRQ
descriptor will be returned for zero IRQ number. That descriptor will be
initialized with the passed parent_handler callback, which isn't what we want.

Ouch but different beahviour on the outside of the irqchip API depending
on whether IRQs are sparse or not on some particular system seems to
be a problem with irqchip reallty, if we wanna get to the bottom of things.
(paging Marc)

So in order to fix the problem we could follow either of the next paths:
1) Just make sure the passed IRQs array is not sparse for instance by remapping
   it to be linear.
2) Move "if (gc->irq.parents[i]) irq_set_chained_handler_and_data()" statement to the
   gpiochip_add_irqchip() method.

What to you think? Linus?

What about (3) fixing irqchip?

Else (2), making the code inside gpiolib be careful and skip over
invalid IRQs.

Yours,
Linus Walleij