Re: gpio-mt7621 unroutable IRQs to bank0

From: Sergio Paracuellos

Date: Thu May 07 2026 - 00:06:51 EST

On Thu, May 7, 2026 at 12:32 AM Vicente Bergas <vicencb@xxxxxxxxx> wrote:
>
> 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.

Linus, Bartosz, any advice regarding this?

>
> ```
> --- 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)
> ```

Vicente, I know that you are using openwrt dts but take into account
that there are already in tree device trees that must be updated as
well if you end up updating the mt7621-gpio yaml doc:
- https://elixir.bootlin.com/linux/v7.0.1/source/arch/mips/boot/dts/ralink/mt7621.dtsi#L180
- https://elixir.bootlin.com/linux/v7.0.1/source/arch/mips/boot/dts/ralink/mt7628a.dtsi#L173

Thanks,
Sergio Paracuellos