Re: [PATCH v2] gpio: mt7621: fix interrupt banks mapping on gpio chips

[Bartosz Golaszewski]

On Mon, 8 Jun 2026 11:40:57 +0200, Sergio Paracuellos
<sergio.paracuellos@xxxxxxxxx> said:
> Hi,
>
> On Mon, Jun 8, 2026 at 11:05 AM Bartosz Golaszewski <brgl@xxxxxxxxxx> wrote:
>>
>> On Tue, 2 Jun 2026 16:25:13 +0200, Sergio Paracuellos
>> <sergio.paracuellos@xxxxxxxxx> said:
>> > The GPIO controller's registers are organized as sets of eight 32-bit
>> > registers with each set controlling a bank of up to 32 pins. A single
>> > interrupt is shared for all of the banks handled by the controller.
>> > The driver implements this using three gpio chip instances every one
>> > with its own irq chip. Every single pin can generate interrupts having
>> > a total of 96 possible interrupts here. It looks like there is a problem
>> > with interrupts being properly mapped to the gpio bank using this solution.
>> > This problem report is in the following lore's link [0].
>> >
>> > Device tree is using two cells for this, so only the interrupt pin and the
>> > interrupt type are described there. Changing to have three cells to setup
>> > also the bank and implement 'of_node_instance_match()' would also work but
>> > this would be an ABI breakage and also a bit incoherent since gpios itself
>> > are also using two cells and properly mapped in desired bank using through
>> > its pin number on 'of_xlate()'.
>> >
>> > That said, register a linear IRQ domain of the total of 96 interrupts shared
>> > with the three gpio chip instances so the bank and the interrupt is properly
>> > decoded and devices using gpio IRQs properly work.
>> >
>> > [0]: https://lore.kernel.org/linux-gpio/CAAMcf8C_A9dJ_v4QRKtb9eGNOpJ7BZNOGsFP4i2WFOZxOVBPnQ@xxxxxxxxxxxxxx/T/#u
>> >
>> > Fixes: 4ba9c3afda41 ("gpio: mt7621: Add a driver for MT7621")
>> > Co-developed-by: Vicente Bergas <vicencb@xxxxxxxxx>
>> > Signed-off-by: Vicente Bergas <vicencb@xxxxxxxxx>
>> > Tested-by: Vicente Bergas <vicencb@xxxxxxxxx>
>> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
>> > ---
>>
>> Hi!
>>
>> Can you look at the sashiko review? Especially the bit about tracking the
>> GPIOD_FLAG_IRQ_IS_ENABLED flag looks correct.
>
> I got rid of those two calls (gpiochip_enable_irq() and
> gpiochip_disable_irq()) because the driver "gpio-brcmstb" which is the
> one I based my changes on was not used them at all. We have not found
> anything weird related with that on testing. I do believe that since
> we are using our own callbacks for 'irq_request_resources()' and
> 'irq_release_resources()' we are safe here. Regarding the others I am
> not sure, but the introduction of the remove stuff for the irq domain
> is because there are no devm_* functions for that. Other resources in
> driver are using devm versions so I think the changes are ok as they
> are...
>

It's about GPIO core: a GPIO that appears as "free" (users can request it) but
was earlier enabled for interrupts cannot be requested in output mode - only
input works. Without this flag set, gpiod_direction_output_nonotify() will allow
you to set direction to output.

Bart