Messages in this thread |  | | From | Sergio Paracuellos <> | | Date | Wed, 6 May 2026 19:43:42 +0200 | | Subject | Re: gpio-mt7621 unroutable IRQs to bank0 |
| |
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
|  |