On Fri, Dec 18, 2020 at 3:49 PM Thierry Reding [off-list ref] wrote:

quoted

I don't quite get this. This makes sense if there is one parent IRQ
per interrupt, but if one of the users of a GPIO in a bank sets the
IRQ type to edge and then another one comes in and set another
of the lines to level and then the function comes here, what type
gets set on the parent? Whichever comes last?

Normally with banked GPIOs collecting several lines in a cascaded
fashion, the GPIO out of the bank toward the GIC is level triggered.

I don't understand how this GPIO controller can be hierarchical,
it looks cascaded by the definition of the document
Documentation/driver-api/gpio/driver.rst

This is basically the same implementation that we've used in the
gpio-tegra186 driver. The goal here is to support wake events, which are
a mechanism for the PMC (which, among other things control wakeup of the
CPU complex from sleep). Wake events are a somewhat non-trivial concept
and I keep second-guessing this myself everytime I look at it...

So basically with these wake events we have a selected number of GPIOs
that are routed to the PMC and which can wake the system from sleep. To
make this work, the PMC IRQ domain becomes the parent of the GPIO IRQ
domain, so what we're forwarding the ->set_type() and ->set_wake()
operations to here is the PMC parent, rather than the parent IRQs which
are, I suppose, somewhat unfortunately named for this particular use-
case.

I suppose given the definition in the documentation the GPIO controller
is both hierarchical (it's a child of the PMC IRQ domain) and cascaded
(sets of GPIOs routed to a number of "parent" interrupts).

What usually helps me in understanding this better is to look at GPIO
and IRQ functionality as separate things. The GPIO controller is
cascaded from the point of view of the GPIOs and how the Linux virtual
interrupts are mapped to physical interrupts. On the other hand the GPIO
controller is hierarchical from an IRQ domain point of view because some
of the GPIO interrupts also have a parent interrupt in the PMC.

I hope that clarifies things a little bit. More specifically the
irq_chip_set_type_parent() isn't actually going to cause the type to be
set on the cascaded interrupts that go to the GIC, but on the parent
interrupts within the PMC (i.e. the wake events) which have separate
registers to program the type and wake enables.

Note that because not all GPIOs may have a corresponding wake event
(i.e. no parent, hierarchically speaking) that's also why we first
check for data->parent_data. See this email thread for a bit more
background information from Marc, who added proper support for this
recently:

It's clear to me that I need to finally educate myself more on
hierarchical IRQs. I don't want to block this patch though until that
happens. I trust that Thierry knows what he's doing here and so I've
applied it for next.

Bart