Re: gpio-mt7621 unroutable IRQs to bank0

[Sergio Paracuellos]

Hi Vicente,

On Tue, May 5, 2026 at 11:36 PM Vicente Bergas <vicencb@xxxxxxxxx> wrote:
>
> On Tue, May 5, 2026 at 5:05 PM Sergio Paracuellos
> <sergio.paracuellos@xxxxxxxxx> wrote:
> >
> > Hi,
> >
> > On Tue, May 5, 2026 at 4:21 PM Thomas Gleixner <tglx@xxxxxxxxxx> wrote:
> > >
> > > On Tue, May 05 2026 at 14:01, Linus Walleij wrote:
> > > > On Sat, May 2, 2026 at 11:52 PM Vicente Bergas <vicencb@xxxxxxxxx> wrote:
> > > >> As a way to prove that this is indeed the problem,
> > > >> the following workaround makes it work.
> > > >> It just inverts the sorting order of all matches,
> > > >> so it picks Bank0 instead of Bank2.
> > > >
> > > > That's a tricksy bug, I can't exactly see where the issue
> > > > is.
> > > >
> > > > I think to solve this you might need to allocate an external
> > > > irqdomain that deal with the three different gpiochip
> > > > instances when translating the irqs.
> > >
> > > struct gpio_chip has this:
> > >
> > >         /**
> > >          * @of_node_instance_match:
> > >          *
> > >          * Determine if a chip is the right instance. Must be implemented by
> > >          * any driver using more than one gpio_chip per device tree node.
> > >          * Returns true if gc is the instance indicated by i (which is the
> > >          * first cell in the phandles for GPIO lines and gpio-ranges).
> > >          */
> > >         bool (*of_node_instance_match)(struct gpio_chip *gc, unsigned int i);
> > >
> > > That driver falls in the category and lacks that callback, no?
> > >
> > > Thanks,
> > >
> > >         tglx
> >
> > The IP core used inside these SoCs has 3 banks of 32 GPIOs each but
> > there is only one device tree node because the registers of all the
> > banks are interwoven inside one single IO range. Thus, the driver
> > internally sets up a gpio controller instance per bank. The driver
> > lacks of_node_instance_match callback but I am not sure if it needs to
> > be implemented in this particular case since the driver is using
> > of_xlate callback for this.
> >
> > Thanks,
> >     Sergio Paracuellos
>
> Hi all,
> just tested with the suggested `of_node_instance_match` by applying
> those changes:
>
> ```
> --- a/drivers/gpio/gpio-mt7621.c
> +++ b/drivers/gpio/gpio-mt7621.c
> @@ -1,3 +1,5 @@
> +#define DEBUG  1
> +
>  // SPDX-License-Identifier: GPL-2.0
>  /*
>   * Copyright (C) 2009-2011 Gabor Juhos <juhosg@xxxxxxxxxxx>
> @@ -206,6 +208,18 @@
>      return gpio % MTK_BANK_WIDTH;
>  }
>
> +static bool mediatek_gpio_node_instance_match(struct gpio_chip *chip,
> unsigned int i)
> +{
> +    struct mtk_gc *rg = to_mediatek_gpio(chip);
> +
> +    struct mtk_gc *rg0 = rg - rg->bank;
> +    struct mtk *mtk = container_of(rg0, struct mtk, gc_map[0]);
> +    dev_dbg(mtk->dev, "%u <=> %d\n", i, rg->bank);
> +    dump_stack();
> +
> +    return i == rg->bank;
> +}
> +
>  static const struct irq_chip mt7621_irq_chip = {
>      .name        = "mt7621-gpio",
>      .irq_mask_ack    = mediatek_gpio_irq_mask,
> @@ -253,6 +267,7 @@
>
>      rg->chip.gc.of_gpio_n_cells = 2;
>      rg->chip.gc.of_xlate = mediatek_gpio_xlate;
> +    rg->chip.gc.of_node_instance_match = mediatek_gpio_node_instance_match;
>      rg->chip.gc.label = devm_kasprintf(dev, GFP_KERNEL, "%s-bank%d",
>                      dev_name(dev), bank);
>      if (!rg->chip.gc.label)
> ```
>
> It turns out that the `mediatek_gpio_node_instance_match` function is
> never called.
>
> Regards,
>   Vicente.

We are using two cells and mediatek_gpio_xlate() callback for this.
IIUC, you would need 3 cells to get this
mediatek_gpio_node_instance_match() called and probably get rid of
mediatek_gpio_xlate() but I have never used of_node_instance_match()
callback so I can be totally wrong.

Best regards,
    Sergio Paracuellos