Re: gpio-mt7621 unroutable IRQs to bank0

[Vicente Bergas]

On Wed, May 6, 2026 at 7:43 PM Sergio Paracuellos
<sergio.paracuellos@xxxxxxxxx> wrote:
>
> 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

I tried removing of_xlate, but then many other things break.
The following patch makes it work, using both of_xlate and
of_node_instance_match.

Now, i am not sure if this is the proper way to fix the issue.
To me it looks ugly that gpios are specified using a single number
from 0 to 95, but associated interrupts need bank + offset.
A single number is more clear, as that way matches the available
documentation of the platform.

Another painpoint of this approach is that existing DT files need to be changed.

If this can be improved: how?
Otherwise, i will update `mediatek,mt7621-gpio.yaml` and send it all
as a pull request.

```
--- mt7628an.dtsi
+++ mt7628an.dtsi
@@ -99,7 +99,7 @@
       interrupt-parent = <&intc>;
       interrupts = <6>;

-      #interrupt-cells = <2>;
+      #interrupt-cells = <3>;
       interrupt-controller;

       gpio-controller;
--- board.dts
+++ board.dts
@@ -191,7 +191,7 @@
     compatible = "focaltech,ft6236";
     reg = <0x38>;
     interrupt-parent = <&gpio>;
-    interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
+    interrupts = <0 0 IRQ_TYPE_EDGE_FALLING>;
     reset-gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
   };
 };
--- a/drivers/gpio/gpio-mt7621.c
+++ b/drivers/gpio/gpio-mt7621.c
@@ -206,6 +206,11 @@
   return gpio % MTK_BANK_WIDTH;
 }

+static bool mediatek_gpio_node_instance_match(struct gpio_chip *chip,
unsigned int i)
+{
+  return i == to_mediatek_gpio(chip)->bank;
+}
+
 static const struct irq_chip mt7621_irq_chip = {
   .name    = "mt7621-gpio",
   .irq_mask_ack  = mediatek_gpio_irq_mask,
@@ -253,6 +258,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)
```