Re: [PATCH v3 4/7] gpio: max7360: Add MAX7360 gpio support

["Mathieu Dubois-Briand" <mathieu.dubois-briand@bootlin.com>]

On Tue Jan 14, 2025 at 3:33 PM CET, Linus Walleij wrote:
> On Mon, Jan 13, 2025 at 1:43 PM Mathieu Dubois-Briand
> My most generic feedback is if you have looked at using
> select GPIO_REGMAP for this driver?
>
> The regmap utility library is very helpful, look how other driver
> selecting GPIO_REGMAP gets default implementations
> from the library just git grep GPIO_REGMAP drivers/gpio/
>

I tried to switch to GPIO_REGMAP and I really like it overall, as it
does simplify a lot the code. However, I identified two features that I
was not able to port so far: the request()/free() callbacks and the
interrupts.

So for the request()/free() callbacks, I cannot add them anymore, as
they are set on the gpio_chip structure, and this structure is hidden
behind the gpio_regmap structure. I could easily modify the
gpio_regmap_config structure and gpio_regmap_register() to allow to
provide these callbacks, but is this acceptable? Or should I switch to a
different way to prevent concurrent use of the same pin? I saw you
mentioned the possibility of defining pin control.

On the IRQ side, before switching to GPIO_REGMAP, I was able to define
the IRQ configuration using the irq member of the gpio_chip structure.
This does create the IRQ domain for me in a quite straightforward way.
Again, I will not be able to do that anymore, as the gpio_chip structure
is hidden. 

I saw I can specify my own irq_domain in gpio_regmap_config, so that
would be a way, but I was wondering if there is any way to have
something as easy as previously.

I had a quick look at existing drivers using GPIO_REGMAP and providing
IRQ support: I believe they are all using REGMAP_IRQ. And I believe I
cannot use REGMAP_IRQ here, as if I understood correctly, I would need
to have a register telling me exactly on which GPIO I have a pending
interrupt and I don't have such a thing: all I know is there was an
interrupt related to the GPIOs, and then I have to compare each GPIO
with the previous known state to know which pin is affected.

Do you have any thought about this?

Best regards,
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com