On Fri, Dec 18, 2020 at 3:49 PM Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > > > > > 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
