On Fri, Nov 27, 2020 at 3:09 PM Thierry Reding <thierry.reding@xxxxxxxxx> wrote: > From: Thierry Reding <treding@xxxxxxxxxx> > > Convert the Tegra GPIO driver to use the gpio_irq_chip infrastructure. > This allows a bit of boiler plate to be removed and while at it enables > support for hierarchical domains, which is useful to support PMC wake > events on Tegra210 and earlier. > > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx> The patch didn't apply to my "devel" branch for some reason so have a look at that, seems gpio-tegra.c has some changes not in my tree. > struct tegra_gpio_soc_config { > @@ -93,12 +91,12 @@ struct tegra_gpio_soc_config { > struct tegra_gpio_info { > struct device *dev; > void __iomem *regs; > - struct irq_domain *irq_domain; > struct tegra_gpio_bank *bank_info; > const struct tegra_gpio_soc_config *soc; > struct gpio_chip gc; > struct irq_chip ic; > u32 bank_count; > + unsigned int *irqs; So this is hierarchical with several IRQs. > static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type) > { > unsigned int gpio = d->hwirq, port = GPIO_PORT(gpio), lvl_type; > - struct tegra_gpio_bank *bank = irq_data_get_irq_chip_data(d); > - struct tegra_gpio_info *tgi = bank->tgi; > + struct gpio_chip *chip = irq_data_get_irq_chip_data(d); > + struct tegra_gpio_info *tgi = gpiochip_get_data(chip); > + struct tegra_gpio_bank *bank; > unsigned long flags; > - u32 val; > int ret; > + u32 val; > + > + bank = &tgi->bank_info[GPIO_BANK(d->hwirq)]; So the general idea is to look up the bank from the IRQ offset. But... > - return 0; > + if (d->parent_data) > + ret = irq_chip_set_type_parent(d, type); > + > + return ret; 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 Yours, Linus Walleij