Linux GPIO subsystem development
 help / color / mirror / Atom feed
* gpio-mt7621 unroutable IRQs to bank0
@ 2026-05-02 21:52 Vicente Bergas
  2026-05-05 12:01 ` Linus Walleij
  0 siblings, 1 reply; 15+ messages in thread
From: Vicente Bergas @ 2026-05-02 21:52 UTC (permalink / raw)
  To: Linus Walleij, Sergio Paracuellos, Bartosz Golaszewski,
	Thomas Gleixner, Grant Likely, Anna-Maria Behnsen
  Cc: Linux Kernel Mailing List, linux-gpio

Hi,
Grant Likely noticed a potential issue and
documented it as a comment in kernel/irq/irqdomain.c:
 * We might want to match the legacy controller last since
 * it might potentially be set to match all interrupts in
 * the absence of a device node. This isn't a problem so far
 * yet though...

There is a bug that is affecting me and
it is triggered by the forseen potential issue.

drivers/gpio/gpio-mt7621.c is registering 96 GPIOs,
all of them capable of generating IRQs.
But the `struct irq_chip` can only handle 32 IRQs,
so, 3 banks are instantiated.

The DTS file specifies the interrupt for one device as
 interrupt-parent = <&gpio>;
 interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
which should be routed to Bank0 GPIO0 but
it is instead routed to Bank2. This is the bug.

The call trace that leads to the problem is:
 drivers/i2c/i2c-core-base.c:i2c_device_probe
 drivers/base/property.c:fwnode_irq_get
 drivers/of/irq.c:of_irq_get
 include/linux/irqdomain.h:irq_find_host
 include/linux/irqdomain.h:irq_find_matching_host
 include/linux/irqdomain.h:irq_find_matching_fwnode
 kernel/irq/irqdomain.c:irq_find_matching_fwspec

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.

--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -538,7 +538,6 @@ irq_find_matching_fwspec

         if (rc) {
             found = h;
-            break;
         }
     }
     mutex_unlock(&irq_domain_mutex);

For a bit more context, the platform used is an mt7628an.
The problematic device is:
&i2c {
  pinctrl-names = "default";
  pinctrl-0 = <&i2c_pins>;
  status = "okay";
  ft6336u: touchscreen@38 {
    compatible = "focaltech,ft6236";
    reg = <0x38>;
    interrupt-parent = <&gpio>;
    interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
    reset-gpios = <&gpio 2 GPIO_ACTIVE_LOW>;
  };
};

Please, can you fix this bug?

I'll will be able to test, but because my platform is only supported
after applying openwrt patches, i can only test on v6.18.y.

Regards,
  Vicente.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-02 21:52 gpio-mt7621 unroutable IRQs to bank0 Vicente Bergas
@ 2026-05-05 12:01 ` Linus Walleij
  2026-05-05 14:21   ` Thomas Gleixner
  0 siblings, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2026-05-05 12:01 UTC (permalink / raw)
  To: Vicente Bergas
  Cc: Sergio Paracuellos, Bartosz Golaszewski, Thomas Gleixner,
	Grant Likely, Anna-Maria Behnsen, Linux Kernel Mailing List,
	linux-gpio

Hi Vicente,

On Sat, May 2, 2026 at 11:52 PM Vicente Bergas <vicencb@gmail.com> wrote:

> drivers/gpio/gpio-mt7621.c is registering 96 GPIOs,
> all of them capable of generating IRQs.
> But the `struct irq_chip` can only handle 32 IRQs,
> so, 3 banks are instantiated.
>
> The DTS file specifies the interrupt for one device as
>  interrupt-parent = <&gpio>;
>  interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
> which should be routed to Bank0 GPIO0 but
> it is instead routed to Bank2. This is the bug.
>
> The call trace that leads to the problem is:
>  drivers/i2c/i2c-core-base.c:i2c_device_probe
>  drivers/base/property.c:fwnode_irq_get
>  drivers/of/irq.c:of_irq_get
>  include/linux/irqdomain.h:irq_find_host
>  include/linux/irqdomain.h:irq_find_matching_host
>  include/linux/irqdomain.h:irq_find_matching_fwnode
>  kernel/irq/irqdomain.c:irq_find_matching_fwspec
>
> 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.

There is gpiochip_irqchip_add_domain() which is currently
only used in drivers/gpio/gpio-regmap.c, but it should do
the trick I think.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-05 12:01 ` Linus Walleij
@ 2026-05-05 14:21   ` Thomas Gleixner
  2026-05-05 15:05     ` Sergio Paracuellos
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2026-05-05 14:21 UTC (permalink / raw)
  To: Linus Walleij, Vicente Bergas
  Cc: Sergio Paracuellos, Bartosz Golaszewski, Grant Likely,
	Anna-Maria Behnsen, Linux Kernel Mailing List, linux-gpio

On Tue, May 05 2026 at 14:01, Linus Walleij wrote:
> On Sat, May 2, 2026 at 11:52 PM Vicente Bergas <vicencb@gmail.com> 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-05 14:21   ` Thomas Gleixner
@ 2026-05-05 15:05     ` Sergio Paracuellos
  2026-05-05 21:36       ` Vicente Bergas
  0 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2026-05-05 15:05 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Linus Walleij, Vicente Bergas, Bartosz Golaszewski, Grant Likely,
	Anna-Maria Behnsen, Linux Kernel Mailing List, linux-gpio

Hi,

On Tue, May 5, 2026 at 4:21 PM Thomas Gleixner <tglx@kernel.org> wrote:
>
> On Tue, May 05 2026 at 14:01, Linus Walleij wrote:
> > On Sat, May 2, 2026 at 11:52 PM Vicente Bergas <vicencb@gmail.com> 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-05 15:05     ` Sergio Paracuellos
@ 2026-05-05 21:36       ` Vicente Bergas
  2026-05-06 17:43         ` Sergio Paracuellos
  0 siblings, 1 reply; 15+ messages in thread
From: Vicente Bergas @ 2026-05-05 21:36 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: Thomas Gleixner, Linus Walleij, Bartosz Golaszewski, Grant Likely,
	Linux Kernel Mailing List, linux-gpio

On Tue, May 5, 2026 at 5:05 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Hi,
>
> On Tue, May 5, 2026 at 4:21 PM Thomas Gleixner <tglx@kernel.org> wrote:
> >
> > On Tue, May 05 2026 at 14:01, Linus Walleij wrote:
> > > On Sat, May 2, 2026 at 11:52 PM Vicente Bergas <vicencb@gmail.com> 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@openwrt.org>
@@ -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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-05 21:36       ` Vicente Bergas
@ 2026-05-06 17:43         ` Sergio Paracuellos
  2026-05-06 22:32           ` Vicente Bergas
  0 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2026-05-06 17:43 UTC (permalink / raw)
  To: Vicente Bergas
  Cc: Thomas Gleixner, Linus Walleij, Bartosz Golaszewski, Grant Likely,
	Linux Kernel Mailing List, linux-gpio

Hi Vicente,

On Tue, May 5, 2026 at 11:36 PM Vicente Bergas <vicencb@gmail.com> wrote:
>
> On Tue, May 5, 2026 at 5:05 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > Hi,
> >
> > On Tue, May 5, 2026 at 4:21 PM Thomas Gleixner <tglx@kernel.org> wrote:
> > >
> > > On Tue, May 05 2026 at 14:01, Linus Walleij wrote:
> > > > On Sat, May 2, 2026 at 11:52 PM Vicente Bergas <vicencb@gmail.com> 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@openwrt.org>
> @@ -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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-06 17:43         ` Sergio Paracuellos
@ 2026-05-06 22:32           ` Vicente Bergas
  2026-05-07  4:06             ` Sergio Paracuellos
  0 siblings, 1 reply; 15+ messages in thread
From: Vicente Bergas @ 2026-05-06 22:32 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: Thomas Gleixner, Linus Walleij, Bartosz Golaszewski, Grant Likely,
	Linux Kernel Mailing List, linux-gpio

On Wed, May 6, 2026 at 7:43 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Hi Vicente,
>
> On Tue, May 5, 2026 at 11:36 PM Vicente Bergas <vicencb@gmail.com> wrote:
> >
> > On Tue, May 5, 2026 at 5:05 PM Sergio Paracuellos
> > <sergio.paracuellos@gmail.com> wrote:
> > >
> > > Hi,
> > >
> > > On Tue, May 5, 2026 at 4:21 PM Thomas Gleixner <tglx@kernel.org> wrote:
> > > >
> > > > On Tue, May 05 2026 at 14:01, Linus Walleij wrote:
> > > > > On Sat, May 2, 2026 at 11:52 PM Vicente Bergas <vicencb@gmail.com> 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@openwrt.org>
> > @@ -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)
```

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-06 22:32           ` Vicente Bergas
@ 2026-05-07  4:06             ` Sergio Paracuellos
  2026-05-07  8:54               ` Linus Walleij
  0 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2026-05-07  4:06 UTC (permalink / raw)
  To: Vicente Bergas
  Cc: Thomas Gleixner, Linus Walleij, Bartosz Golaszewski, Grant Likely,
	Linux Kernel Mailing List, linux-gpio

On Thu, May 7, 2026 at 12:32 AM Vicente Bergas <vicencb@gmail.com> wrote:
>
> On Wed, May 6, 2026 at 7:43 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > Hi Vicente,
> >
> > On Tue, May 5, 2026 at 11:36 PM Vicente Bergas <vicencb@gmail.com> wrote:
> > >
> > > On Tue, May 5, 2026 at 5:05 PM Sergio Paracuellos
> > > <sergio.paracuellos@gmail.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Tue, May 5, 2026 at 4:21 PM Thomas Gleixner <tglx@kernel.org> wrote:
> > > > >
> > > > > On Tue, May 05 2026 at 14:01, Linus Walleij wrote:
> > > > > > On Sat, May 2, 2026 at 11:52 PM Vicente Bergas <vicencb@gmail.com> 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@openwrt.org>
> > > @@ -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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-07  4:06             ` Sergio Paracuellos
@ 2026-05-07  8:54               ` Linus Walleij
  2026-05-07 10:52                 ` Sergio Paracuellos
  0 siblings, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2026-05-07  8:54 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: Vicente Bergas, Thomas Gleixner, Bartosz Golaszewski,
	Grant Likely, Linux Kernel Mailing List, linux-gpio

On Thu, May 7, 2026 at 6:06 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:

> Linus, Bartosz, any advice regarding this?

I sent the advice to try to use an external irqdomain already?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-07  8:54               ` Linus Walleij
@ 2026-05-07 10:52                 ` Sergio Paracuellos
  2026-05-12  0:17                   ` Vicente Bergas
  0 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2026-05-07 10:52 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Vicente Bergas, Thomas Gleixner, Bartosz Golaszewski,
	Grant Likely, Linux Kernel Mailing List, linux-gpio

On Thu, May 7, 2026 at 10:54 AM Linus Walleij <linusw@kernel.org> wrote:
>
> On Thu, May 7, 2026 at 6:06 AM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
>
> > Linus, Bartosz, any advice regarding this?
>
> I sent the advice to try to use an external irqdomain already?

True. Sorry, I missed that somehow.

Best regards,
   Sergio Paracuellos

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-07 10:52                 ` Sergio Paracuellos
@ 2026-05-12  0:17                   ` Vicente Bergas
  2026-05-14 12:00                     ` Sergio Paracuellos
  0 siblings, 1 reply; 15+ messages in thread
From: Vicente Bergas @ 2026-05-12  0:17 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: Linus Walleij, Thomas Gleixner, Bartosz Golaszewski, Grant Likely,
	Linux Kernel Mailing List, linux-gpio

On Thu, May 7, 2026 at 12:52 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> On Thu, May 7, 2026 at 10:54 AM Linus Walleij <linusw@kernel.org> wrote:
> >
> > On Thu, May 7, 2026 at 6:06 AM Sergio Paracuellos
> > <sergio.paracuellos@gmail.com> wrote:
> >
> > > Linus, Bartosz, any advice regarding this?
> >
> > I sent the advice to try to use an external irqdomain already?
>
> True. Sorry, I missed that somehow.
>
> Best regards,
>    Sergio Paracuellos

Please, can somebody write some general guidelines on how to use an
external irqdomain?
I'm lost here, i don't know where to start.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: gpio-mt7621 unroutable IRQs to bank0
  2026-05-12  0:17                   ` Vicente Bergas
@ 2026-05-14 12:00                     ` Sergio Paracuellos
  2026-05-22  7:29                       ` [RFC PATCH] gpio: mt7621: fix interrupt banks mapping on gpio chips Sergio Paracuellos
  0 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2026-05-14 12:00 UTC (permalink / raw)
  To: Vicente Bergas
  Cc: Linus Walleij, Thomas Gleixner, Bartosz Golaszewski, Grant Likely,
	Linux Kernel Mailing List, linux-gpio

On Tue, May 12, 2026 at 2:17 AM Vicente Bergas <vicencb@gmail.com> wrote:
>
> On Thu, May 7, 2026 at 12:52 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > On Thu, May 7, 2026 at 10:54 AM Linus Walleij <linusw@kernel.org> wrote:
> > >
> > > On Thu, May 7, 2026 at 6:06 AM Sergio Paracuellos
> > > <sergio.paracuellos@gmail.com> wrote:
> > >
> > > > Linus, Bartosz, any advice regarding this?
> > >
> > > I sent the advice to try to use an external irqdomain already?
> >
> > True. Sorry, I missed that somehow.
> >
> > Best regards,
> >    Sergio Paracuellos
>
> Please, can somebody write some general guidelines on how to use an
> external irqdomain?
> I'm lost here, i don't know where to start.

IIUC we need to register an external irq_domain for the 96 pins that
the three banks (32 gpios each) can reference. Current driver has
three gpio_chip instances but we will have one external domain now so
I don't know but we would need probably a specific xlate function so
when the device tree asks for a gpio on whatever bank the driver maps
the correct offset in the external domain. The other tricky part would
be the irq handler itself that now has to iterate the three banks to
trigger the proper virtual IRQ.

Linus, is my understanding correct?

Thanks,
    Sergio Paracuellos

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RFC PATCH] gpio: mt7621: fix interrupt banks mapping on gpio chips
  2026-05-14 12:00                     ` Sergio Paracuellos
@ 2026-05-22  7:29                       ` Sergio Paracuellos
  2026-05-26 11:38                         ` Bartosz Golaszewski
  0 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2026-05-22  7:29 UTC (permalink / raw)
  To: linux-gpio
  Cc: linusw, brgl, tglx, grant.likely, anna-maria, vicencb,
	linux-kernel

Regarding the issue reported by Vicente[0], we have been trying different
things and we are still having issues to make this work. I have noticed
that the gpio-brcmstb is similar to our use case sharing one interrupt
for all the banks and also using gpio chips instances with 32 pins each.
That said, I tried to setup mt7621 driver in the same way as you can see 
on the following proposed code. With these changes, we are able to make
properly working the previous problem with the touchscreen that was
registered on bank 2 instead of bank 0. Now it is properly registered
on bank 0 and interrupts works perfect and the device is properly
working. However, every single gpio-keys fail to claim the IRQ HW as
follows:

mt7621_gpio 10000600.gpio: Mapping irq 41 for gpio line 38 (bank 1)
gpio gpiochip1: (10000600.gpio-bank1): unable to lock HW IRQ 38 for IRQ
genirq: Failed to request resources for S3 (irq 41) on irqchip mt7621-gpio
gpio-keys keys: error -EINVAL: request_irq(41) gpio_keys_gpio_isr 0x0 S3
gpio-keys keys: Unable to claim irq 41; error -22
gpio-keys keys: probe with driver gpio-keys failed with error -22

So IIUC the kernel is saying that the gpio chip is not IRQ-capable somehow.

Once I touch the irq field just setting up the irq_chip_ops on gpio chip to bypass 
this issue: 

gpio_irq_chip_set_chip(&rg->chip.gc.irq, &mt7621_irq_chip);

the kernel stops calling our custom to_irq callback and calls gpiochip_to_irq
callback instead and also warning as follows:

gpio gpiochip0: (10000600.gpio-bank0): to_irq is redefined in
gpiochip_irqchip_add_allocated_domain and you shouldn't rely on it
gpio gpiochip1: (10000600.gpio-bank1): to_irq is redefined in
gpiochip_irqchip_add_allocated_domain and you shouldn't rely on it
gpio gpiochip2: (10000600.gpio-bank2): to_irq is redefined in
gpiochip_irqchip_add_allocated_domain and you shouldn't rely on it

This register gpio-keys and now are shown on /proc/interrupts but they
are not working (touchscreen still works, don't really understand why):

$ cat /proc/interrupts
40: 10 mt7621-gpio  0  ft6236
41:  0 mt7621-gpio  6  S3
42:  0 mt7621-gpio  7  S2
43:  0 mt7621-gpio 22  S1

I have tried also to get rid of custom to_irq compiling kernel with
IRQ_DOMAIN_HIERARCHY and setting up the followings as replacement of
custom to_irq:

static int
mt7621_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
                                 unsigned int hwirq,
                                 unsigned int type,
                                 unsigned int *parent_hwirq,
                                 unsigned int *parent_type)
{
       *parent_hwirq = gc->irq.child_offset_to_irq(gc, hwirq);
       *parent_type = type;
    
       return 0;
}

static unsigned int
mt7621_gpio_child_offset_to_irq(struct gpio_chip *gc, unsigned int offset)
{
       return gc->offset + offset;
}

rg->chip.gc.irq.child_to_parent_hwirq = mt7621_gpio_child_to_parent_hwirq;
rg->chip.gc.irq.child_offset_to_irq = mt7621_gpio_child_offset_to_irq;

This return us to the original scenario where touchscreen is mapped in the
wrong bank 2 instead of bank 0...

Also tried to setup domain for every single gpio chip using Linus previous
suggestion gpiochip_irqchip_add_domain() but ended up in kernel complaning
also for IRQ HQ lock and getting a kernel panic:

kernel panic "Unhandled kernel unaligned access[#1]")

I am totally lost now. Can you point me out in the right way to make everything
working?

Thanks a lot in advance.

Best regards,
    Sergio Paracuellos

[0]: https://lore.kernel.org/linux-gpio/CAAMcf8C_A9dJ_v4QRKtb9eGNOpJ7BZNOGsFP4i2WFOZxOVBPnQ@mail.gmail.com/T/#u
diff --git a/drivers/gpio/gpio-mt7621.c b/drivers/gpio/gpio-mt7621.c
index 91230be51587..353dc56411eb 100644
--- a/drivers/gpio/gpio-mt7621.c
+++ b/drivers/gpio/gpio-mt7621.c
@@ -29,8 +29,8 @@
 #define GPIO_REG_EDGE		0xA0
 
 struct mtk_gc {
-	struct irq_chip irq_chip;
 	struct gpio_generic_chip chip;
+	struct mtk *parent_priv;
 	int bank;
 	u32 rising;
 	u32 falling;
@@ -43,18 +43,27 @@ struct mtk_gc {
  * data of the platform driver. It is 3
  * separate gpio-chip each one with its
  * own irq_chip.
- * @dev: device instance
+ * @pdev: platform device instance
  * @base: memory base address
  * @gpio_irq: irq number from the device tree
  * @gc_map: array of the gpio chips
  */
 struct mtk {
-	struct device *dev;
+	struct platform_device *pdev;
 	void __iomem *base;
+	struct irq_domain *irq_domain;
 	int gpio_irq;
+	int num_gpios;
 	struct mtk_gc gc_map[MTK_BANK_CNT];
 };
 
+static inline struct mtk *
+mt7621_gpio_gc_to_priv(struct gpio_chip *gc)
+{
+	struct mtk_gc *bank = gpiochip_get_data(gc);
+	return bank->parent_priv;
+}
+
 static inline struct mtk_gc *
 to_mediatek_gpio(struct gpio_chip *chip)
 {
@@ -67,7 +76,7 @@ static inline void
 mtk_gpio_w32(struct mtk_gc *rg, u32 offset, u32 val)
 {
 	struct gpio_chip *gc = &rg->chip.gc;
-	struct mtk *mtk = gpiochip_get_data(gc);
+	struct mtk *mtk = mt7621_gpio_gc_to_priv(gc);
 
 	offset = (rg->bank * GPIO_BANK_STRIDE) + offset;
 	gpio_generic_write_reg(&rg->chip, mtk->base + offset, val);
@@ -77,60 +86,79 @@ static inline u32
 mtk_gpio_r32(struct mtk_gc *rg, u32 offset)
 {
 	struct gpio_chip *gc = &rg->chip.gc;
-	struct mtk *mtk = gpiochip_get_data(gc);
+	struct mtk *mtk = mt7621_gpio_gc_to_priv(gc);
 
 	offset = (rg->bank * GPIO_BANK_STRIDE) + offset;
 	return gpio_generic_read_reg(&rg->chip, mtk->base + offset);
 }
 
-static irqreturn_t
-mediatek_gpio_irq_handler(int irq, void *data)
+static void
+mt7621_gpio_irq_bank_handler(struct mtk_gc *bank)
 {
-	struct gpio_chip *gc = data;
-	struct mtk_gc *rg = to_mediatek_gpio(gc);
-	irqreturn_t ret = IRQ_NONE;
+	struct mtk *priv = bank->parent_priv;
+	struct irq_domain *domain = priv->irq_domain;
+	int hwbase = bank->chip.gc.offset;
 	unsigned long pending;
-	int bit;
+	unsigned int offset;
+
+	pending = mtk_gpio_r32(bank, GPIO_REG_STAT);
+	if (!pending)
+		return;
+
+	mtk_gpio_w32(bank, GPIO_REG_STAT, pending);
+
+	for_each_set_bit(offset, &pending, MTK_BANK_WIDTH)
+		generic_handle_domain_irq(domain, hwbase + offset);
+}
+
+static void
+mt7621_gpio_irq_handler(struct irq_desc *desc)
+{
+	struct mtk *priv = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	int i;
 
-	pending = mtk_gpio_r32(rg, GPIO_REG_STAT);
+	chained_irq_enter(chip, desc);
+	for (i = 0; i < MTK_BANK_CNT; i++) {
+		struct mtk_gc *bank = &priv->gc_map[i];
 
-	for_each_set_bit(bit, &pending, MTK_BANK_WIDTH) {
-		generic_handle_domain_irq(gc->irq.domain, bit);
-		mtk_gpio_w32(rg, GPIO_REG_STAT, BIT(bit));
-		ret |= IRQ_HANDLED;
+		mt7621_gpio_irq_bank_handler(bank);
 	}
+	chained_irq_exit(chip, desc);
+}
 
-	return ret;
+static int
+mt7621_gpio_hwirq_to_offset(irq_hw_number_t hwirq, struct mtk_gc *bank)
+{
+	return hwirq - bank->chip.gc.offset;
 }
 
 static void
 mediatek_gpio_irq_unmask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct mtk_gc *rg = to_mediatek_gpio(gc);
-	int pin = d->hwirq;
+	struct mtk_gc *rg = gpiochip_get_data(gc);
+	u32 mask = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);
 	u32 rise, fall, high, low;
 
-	gpiochip_enable_irq(gc, d->hwirq);
-
 	guard(gpio_generic_lock_irqsave)(&rg->chip);
 
 	rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
 	fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
 	high = mtk_gpio_r32(rg, GPIO_REG_HLVL);
 	low = mtk_gpio_r32(rg, GPIO_REG_LLVL);
-	mtk_gpio_w32(rg, GPIO_REG_REDGE, rise | (BIT(pin) & rg->rising));
-	mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall | (BIT(pin) & rg->falling));
-	mtk_gpio_w32(rg, GPIO_REG_HLVL, high | (BIT(pin) & rg->hlevel));
-	mtk_gpio_w32(rg, GPIO_REG_LLVL, low | (BIT(pin) & rg->llevel));
+	mtk_gpio_w32(rg, GPIO_REG_REDGE, rise | (BIT(mask) & rg->rising));
+	mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall | (BIT(mask) & rg->falling));
+	mtk_gpio_w32(rg, GPIO_REG_HLVL, high | (BIT(mask) & rg->hlevel));
+	mtk_gpio_w32(rg, GPIO_REG_LLVL, low | (BIT(mask) & rg->llevel));
 }
 
 static void
 mediatek_gpio_irq_mask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct mtk_gc *rg = to_mediatek_gpio(gc);
-	int pin = d->hwirq;
+	struct mtk_gc *rg = gpiochip_get_data(gc);
+	u32 mask = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);
 	u32 rise, fall, high, low;
 
 	scoped_guard(gpio_generic_lock_irqsave, &rg->chip) {
@@ -138,22 +166,19 @@ mediatek_gpio_irq_mask(struct irq_data *d)
 		fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
 		high = mtk_gpio_r32(rg, GPIO_REG_HLVL);
 		low = mtk_gpio_r32(rg, GPIO_REG_LLVL);
-		mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall & ~BIT(pin));
-		mtk_gpio_w32(rg, GPIO_REG_REDGE, rise & ~BIT(pin));
-		mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~BIT(pin));
-		mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~BIT(pin));
+		mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall & ~BIT(mask));
+		mtk_gpio_w32(rg, GPIO_REG_REDGE, rise & ~BIT(mask));
+		mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~BIT(mask));
+		mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~BIT(mask));
 	}
-
-	gpiochip_disable_irq(gc, d->hwirq);
 }
 
 static int
 mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct mtk_gc *rg = to_mediatek_gpio(gc);
-	int pin = d->hwirq;
-	u32 mask = BIT(pin);
+	struct mtk_gc *rg = gpiochip_get_data(gc);
+	u32 mask = BIT(mt7621_gpio_hwirq_to_offset(d->hwirq, rg));
 
 	if (type == IRQ_TYPE_PROBE) {
 		if ((rg->rising | rg->falling |
@@ -216,10 +241,124 @@ static const struct irq_chip mt7621_irq_chip = {
 	GPIOCHIP_IRQ_RESOURCE_HELPERS,
 };
 
+static void mt7621_gpio_remove(struct platform_device *pdev)
+{
+	struct mtk *priv = platform_get_drvdata(pdev);
+	int i, offset, virq;
+
+	if (priv->gpio_irq > 0)
+		irq_set_chained_handler_and_data(priv->gpio_irq, NULL, NULL);
+
+	/* Remove all IRQ mappings and delete the domain */
+	if (priv->irq_domain) {
+		for (offset = 0; offset < priv->num_gpios; offset++) {
+			virq = irq_find_mapping(priv->irq_domain, offset);
+			irq_dispose_mapping(virq);
+		}
+		irq_domain_remove(priv->irq_domain);
+	}
+
+	for (i = 0; i < MTK_BANK_CNT; i++) {
+		struct mtk_gc *bank = &priv->gc_map[i];
+
+		gpiochip_remove(&bank->chip.gc);
+	}
+}
+
+static struct mtk_gc *
+mt7621_gpio_hwirq_to_bank(struct mtk *priv, irq_hw_number_t hwirq)
+{
+	int i;
+
+	for (i = 0; i < MTK_BANK_CNT; i++) {
+		struct mtk_gc *bank = &priv->gc_map[i];
+
+		if (hwirq >= bank->chip.gc.offset &&
+		    hwirq < (bank->chip.gc.offset + bank->chip.gc.ngpio))
+			return bank;
+	}
+
+	return NULL;
+}
+
 static int
-mediatek_gpio_bank_probe(struct device *dev, int bank)
+mt7621_gpio_irq_map(struct irq_domain *d, unsigned int irq,
+		    irq_hw_number_t hwirq)
+{
+	struct mtk *priv = d->host_data;
+	struct mtk_gc *bank = mt7621_gpio_hwirq_to_bank(priv, hwirq);
+	struct platform_device *pdev = priv->pdev;
+	int ret;
+
+	if (!bank)
+		return -EINVAL;
+
+	dev_dbg(&pdev->dev, "Mapping irq %d for gpio line %d (bank %d)\n",
+		irq, (int)hwirq, bank->bank);
+
+	ret = irq_set_chip_data(irq, &bank->chip.gc);
+	if (ret < 0)
+		return ret;
+
+	irq_set_chip_and_handler(irq, &mt7621_irq_chip, handle_simple_irq);
+	irq_set_noprobe(irq);
+
+	return 0;
+}
+
+static void
+mt7621_gpio_irq_unmap(struct irq_domain *d, unsigned int irq)
+{
+	irq_set_chip_and_handler(irq, NULL, NULL);
+	irq_set_chip_data(irq, NULL);
+}
+
+static const struct irq_domain_ops mt7621_gpio_irq_domain_ops = {
+	.map = mt7621_gpio_irq_map,
+	.unmap = mt7621_gpio_irq_unmap,
+	.xlate = irq_domain_xlate_twocell,
+};
+
+static int
+mt7621_gpio_irq_setup(struct platform_device *pdev,
+		      struct mtk *priv)
+{
+	struct device *dev = &pdev->dev;
+
+	priv->irq_domain = irq_domain_create_linear(dev_fwnode(dev),
+						    priv->num_gpios,
+						    &mt7621_gpio_irq_domain_ops,
+						    priv);
+	if (!priv->irq_domain) {
+		dev_err(dev, "Couldn't allocate IRQ domain\n");
+		return -ENXIO;
+	}
+
+	irq_set_chained_handler_and_data(priv->gpio_irq,
+					 mt7621_gpio_irq_handler, priv);
+	irq_set_status_flags(priv->gpio_irq, IRQ_DISABLE_UNLAZY);
+
+	return 0;
+}
+
+static int
+mt7621_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
+{
+	struct mtk *priv = mt7621_gpio_gc_to_priv(gc);
+	/* gc_offset is relative to this gpio_chip; want real offset */
+	int hwirq = offset + gc->offset;
+
+	if (hwirq >= priv->num_gpios)
+		return -ENXIO;
+
+	return irq_create_mapping(priv->irq_domain, hwirq);
+}
+
+static int
+mediatek_gpio_bank_probe(struct platform_device *pdev, int bank)
 {
 	struct gpio_generic_chip_config config;
+	struct device *dev = &pdev->dev;
 	struct mtk *mtk = dev_get_drvdata(dev);
 	struct mtk_gc *rg;
 	void __iomem *dat, *set, *ctrl, *diro;
@@ -228,6 +367,7 @@ mediatek_gpio_bank_probe(struct device *dev, int bank)
 	rg = &mtk->gc_map[bank];
 	memset(rg, 0, sizeof(*rg));
 
+	rg->parent_priv = mtk;
 	rg->bank = bank;
 
 	dat = mtk->base + GPIO_REG_DATA + (rg->bank * GPIO_BANK_STRIDE);
@@ -253,47 +393,25 @@ mediatek_gpio_bank_probe(struct device *dev, int bank)
 
 	rg->chip.gc.of_gpio_n_cells = 2;
 	rg->chip.gc.of_xlate = mediatek_gpio_xlate;
+	rg->chip.gc.ngpio = MTK_BANK_WIDTH;
 	rg->chip.gc.label = devm_kasprintf(dev, GFP_KERNEL, "%s-bank%d",
 					dev_name(dev), bank);
 	if (!rg->chip.gc.label)
 		return -ENOMEM;
 
 	rg->chip.gc.offset = bank * MTK_BANK_WIDTH;
+	if (mtk->gpio_irq > 0)
+		rg->chip.gc.to_irq = mt7621_gpio_to_irq;
 
-	if (mtk->gpio_irq) {
-		struct gpio_irq_chip *girq;
-
-		/*
-		 * Directly request the irq here instead of passing
-		 * a flow-handler because the irq is shared.
-		 */
-		ret = devm_request_irq(dev, mtk->gpio_irq,
-				       mediatek_gpio_irq_handler, IRQF_SHARED,
-				       rg->chip.gc.label, &rg->chip.gc);
-
-		if (ret) {
-			dev_err(dev, "Error requesting IRQ %d: %d\n",
-				mtk->gpio_irq, ret);
-			return ret;
-		}
-
-		girq = &rg->chip.gc.irq;
-		gpio_irq_chip_set_chip(girq, &mt7621_irq_chip);
-		/* This will let us handle the parent IRQ in the driver */
-		girq->parent_handler = NULL;
-		girq->num_parents = 0;
-		girq->parents = NULL;
-		girq->default_type = IRQ_TYPE_NONE;
-		girq->handler = handle_simple_irq;
-	}
-
-	ret = devm_gpiochip_add_data(dev, &rg->chip.gc, mtk);
+	ret = devm_gpiochip_add_data(dev, &rg->chip.gc, rg);
 	if (ret < 0) {
 		dev_err(dev, "Could not register gpio %d, ret=%d\n",
 			rg->chip.gc.ngpio, ret);
 		return ret;
 	}
 
+	mtk->num_gpios += rg->chip.gc.ngpio;
+
 	/* set polarity to low for all gpios */
 	mtk_gpio_w32(rg, GPIO_REG_POL, 0);
 
@@ -322,16 +440,26 @@ mediatek_gpio_probe(struct platform_device *pdev)
 	if (mtk->gpio_irq < 0)
 		return mtk->gpio_irq;
 
-	mtk->dev = dev;
+	mtk->pdev = pdev;
 	platform_set_drvdata(pdev, mtk);
 
 	for (i = 0; i < MTK_BANK_CNT; i++) {
-		ret = mediatek_gpio_bank_probe(dev, i);
+		ret = mediatek_gpio_bank_probe(pdev, i);
 		if (ret)
-			return ret;
+			goto fail;
+	}
+
+	if (mtk->gpio_irq > 0) {
+		ret = mt7621_gpio_irq_setup(pdev, mtk);
+		if (ret)
+			goto fail;
 	}
 
 	return 0;
+
+fail:
+	(void)mt7621_gpio_remove(pdev);
+	return ret;
 }
 
 static const struct of_device_id mediatek_gpio_match[] = {

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] gpio: mt7621: fix interrupt banks mapping on gpio chips
  2026-05-22  7:29                       ` [RFC PATCH] gpio: mt7621: fix interrupt banks mapping on gpio chips Sergio Paracuellos
@ 2026-05-26 11:38                         ` Bartosz Golaszewski
  2026-05-26 11:51                           ` Sergio Paracuellos
  0 siblings, 1 reply; 15+ messages in thread
From: Bartosz Golaszewski @ 2026-05-26 11:38 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: linux-gpio, linusw, tglx, grant.likely, anna-maria, vicencb,
	linux-kernel

On Fri, May 22, 2026 at 9:29 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Regarding the issue reported by Vicente[0], we have been trying different
> things and we are still having issues to make this work. I have noticed
> that the gpio-brcmstb is similar to our use case sharing one interrupt
> for all the banks and also using gpio chips instances with 32 pins each.
> That said, I tried to setup mt7621 driver in the same way as you can see
> on the following proposed code. With these changes, we are able to make
> properly working the previous problem with the touchscreen that was
> registered on bank 2 instead of bank 0. Now it is properly registered
> on bank 0 and interrupts works perfect and the device is properly
> working. However, every single gpio-keys fail to claim the IRQ HW as
> follows:
>
> mt7621_gpio 10000600.gpio: Mapping irq 41 for gpio line 38 (bank 1)
> gpio gpiochip1: (10000600.gpio-bank1): unable to lock HW IRQ 38 for IRQ

At which line in gpiolib.c does this fail exactly?

> genirq: Failed to request resources for S3 (irq 41) on irqchip mt7621-gpio
> gpio-keys keys: error -EINVAL: request_irq(41) gpio_keys_gpio_isr 0x0 S3
> gpio-keys keys: Unable to claim irq 41; error -22
> gpio-keys keys: probe with driver gpio-keys failed with error -22
>
> So IIUC the kernel is saying that the gpio chip is not IRQ-capable somehow.
>
> Once I touch the irq field just setting up the irq_chip_ops on gpio chip to bypass
> this issue:
>
> gpio_irq_chip_set_chip(&rg->chip.gc.irq, &mt7621_irq_chip);
>
> the kernel stops calling our custom to_irq callback and calls gpiochip_to_irq
> callback instead and also warning as follows:
>
> gpio gpiochip0: (10000600.gpio-bank0): to_irq is redefined in
> gpiochip_irqchip_add_allocated_domain and you shouldn't rely on it
> gpio gpiochip1: (10000600.gpio-bank1): to_irq is redefined in
> gpiochip_irqchip_add_allocated_domain and you shouldn't rely on it
> gpio gpiochip2: (10000600.gpio-bank2): to_irq is redefined in
> gpiochip_irqchip_add_allocated_domain and you shouldn't rely on it
>

Are you calling gpiochip_add_irqchip() in your changes?

Bart

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC PATCH] gpio: mt7621: fix interrupt banks mapping on gpio chips
  2026-05-26 11:38                         ` Bartosz Golaszewski
@ 2026-05-26 11:51                           ` Sergio Paracuellos
  0 siblings, 0 replies; 15+ messages in thread
From: Sergio Paracuellos @ 2026-05-26 11:51 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: linux-gpio, linusw, tglx, grant.likely, anna-maria, vicencb,
	linux-kernel

Hi Bartosz,

On Tue, May 26, 2026 at 1:38 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Fri, May 22, 2026 at 9:29 AM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > Regarding the issue reported by Vicente[0], we have been trying different
> > things and we are still having issues to make this work. I have noticed
> > that the gpio-brcmstb is similar to our use case sharing one interrupt
> > for all the banks and also using gpio chips instances with 32 pins each.
> > That said, I tried to setup mt7621 driver in the same way as you can see
> > on the following proposed code. With these changes, we are able to make
> > properly working the previous problem with the touchscreen that was
> > registered on bank 2 instead of bank 0. Now it is properly registered
> > on bank 0 and interrupts works perfect and the device is properly
> > working. However, every single gpio-keys fail to claim the IRQ HW as
> > follows:
> >
> > mt7621_gpio 10000600.gpio: Mapping irq 41 for gpio line 38 (bank 1)
> > gpio gpiochip1: (10000600.gpio-bank1): unable to lock HW IRQ 38 for IRQ
>
> At which line in gpiolib.c does this fail exactly?

This was failing because of using a linear domain with 96 pins in
total but having three gpio chip instances with
32 pins each so it looks gpiolib found hwirq number out of range 0-32
for the chip and refused to mark the pin as IRQ capable.
If I get rid of GPIOCHIP_IRQ_RESOURCE_HELPERS and add my owns
converting irq into the proper pin within the chip
as follows, gpio-keys are properly registered and also properly working:

static int
mt7621_gpio_irq_reqres(struct irq_data *d)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct mtk_gc *rg = gpiochip_get_data(gc);
unsigned int irq = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);

return gpiochip_reqres_irq(gc, irq);
}

static void
mt7621_gpio_irq_relres(struct irq_data *d)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct mtk_gc *rg = gpiochip_get_data(gc);
unsigned int irq = mt7621_gpio_hwirq_to_offset(d->hwirq, rg);

gpiochip_relres_irq(gc, irq);
}

We are still testing but I will send a proper working patch for you to
review and comment during this week.

Thanks,
    Sergio Paracuellos

>

> > genirq: Failed to request resources for S3 (irq 41) on irqchip mt7621-gpio
> > gpio-keys keys: error -EINVAL: request_irq(41) gpio_keys_gpio_isr 0x0 S3
> > gpio-keys keys: Unable to claim irq 41; error -22
> > gpio-keys keys: probe with driver gpio-keys failed with error -22
> >
> > So IIUC the kernel is saying that the gpio chip is not IRQ-capable somehow.
> >
> > Once I touch the irq field just setting up the irq_chip_ops on gpio chip to bypass
> > this issue:
> >
> > gpio_irq_chip_set_chip(&rg->chip.gc.irq, &mt7621_irq_chip);
> >
> > the kernel stops calling our custom to_irq callback and calls gpiochip_to_irq
> > callback instead and also warning as follows:
> >
> > gpio gpiochip0: (10000600.gpio-bank0): to_irq is redefined in
> > gpiochip_irqchip_add_allocated_domain and you shouldn't rely on it
> > gpio gpiochip1: (10000600.gpio-bank1): to_irq is redefined in
> > gpiochip_irqchip_add_allocated_domain and you shouldn't rely on it
> > gpio gpiochip2: (10000600.gpio-bank2): to_irq is redefined in
> > gpiochip_irqchip_add_allocated_domain and you shouldn't rely on it
> >
>
> Are you calling gpiochip_add_irqchip() in your changes?
>
> Bart

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-05-26 11:52 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-02 21:52 gpio-mt7621 unroutable IRQs to bank0 Vicente Bergas
2026-05-05 12:01 ` Linus Walleij
2026-05-05 14:21   ` Thomas Gleixner
2026-05-05 15:05     ` Sergio Paracuellos
2026-05-05 21:36       ` Vicente Bergas
2026-05-06 17:43         ` Sergio Paracuellos
2026-05-06 22:32           ` Vicente Bergas
2026-05-07  4:06             ` Sergio Paracuellos
2026-05-07  8:54               ` Linus Walleij
2026-05-07 10:52                 ` Sergio Paracuellos
2026-05-12  0:17                   ` Vicente Bergas
2026-05-14 12:00                     ` Sergio Paracuellos
2026-05-22  7:29                       ` [RFC PATCH] gpio: mt7621: fix interrupt banks mapping on gpio chips Sergio Paracuellos
2026-05-26 11:38                         ` Bartosz Golaszewski
2026-05-26 11:51                           ` Sergio Paracuellos

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox