Re: [PATCH v3 2/7] gpio: regmap: add gpio_regmap_get_gpiochip() accessor

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3418 bytes --]

On Wed Jul 1, 2026 at 1:38 PM CEST, Andy Shevchenko wrote:
> On Wed, Jul 01, 2026 at 01:55:11PM +0300, Andy Shevchenko wrote:
>> On Wed, Jul 01, 2026 at 01:01:10PM +0300, Andy Shevchenko wrote:
>> > On Wed, Jul 1, 2026 at 11:44 AM Michael Walle <mwalle@kernel.org> wrote:
>> > > On Fri Jun 19, 2026 at 11:08 PM CEST, Linus Walleij wrote:
>> > > > On Mon, Jun 8, 2026 at 4:41 PM Michael Walle <mwalle@kernel.org> wrote:
>> > > >
>> > > >> >>> Without an accessor like gpio_regmap_get_gpiochip(), we cannot retrieve the
>> > > >> >>> gpio_chip instantiated inside gpio-regmap.c to fulfill these requirements in our
>> > > >> >>> map() function.
>> > > >>
>> > > >> Why is gpiochip_irq_reqres() called in the first place? Isn't that
>> > > >> only called if the irq handling is set up via gc->irq.chip and not
>> > > >> via gpiochip_irqchip_add_domain() like in gpio-regmap?
>> > > >
>> > > > Not really, the gpiochip_irq_reqres() is called to mark that a
>> > > > GPIO line is used for IRQ, so the gpiolib cannot turn this
>> > > > GPIO into an output line, gpiod_direction_out() will fail
>> > > > on lines used for IRQ. So it's a failsafe.
>> > > >
>> > > > You can live without it of course, but then you don't get
>> > > > this failsafe.
>> > >
>> > > Thanks for the explanation! So did I make a mistake years ago by
>> > > adding the gpiochip_irqchip_add_domain(), see commit 6a45b0e2589f
>> > > ("gpiolib: Introduce gpiochip_irqchip_add_domain()")
>> > >
>> > > As Yu-Chun found, gpiochip_irq_reqres() expect the irq chip data
>> > > to be a gpio_chip, which isn't the case (in general) for an
>> > > externally allocated domain, is it?
>> > 
>> > So the whole issue comes from the fact that the IRQ chip is not marked
>> > as immutable. For immutable IRQ chips (which all GPIO provides should
>> > have) there is no such issue to begin with, id est there is no
>> > gpiochip_irq_reqres() callback assigned (and respective _relres).
>> 
>> Ah, for immutable chips we put either custom ones or
>> GPIOCHIP_IRQ_RESOURCE_HELPERS which actually refers to those callbacks.
>> 
>> So, if the domain is external, it should also provide irq_request_resources
>> and release callbacks. In the custom case we can wrap gpiochip_reqres_irq()
>> and gpiochip_relres_irq() respectively.
>> 
>> But we need to have a struct gpio_chip pointer for them. And note, the
>> IRQ chip data can be anything in that case, so it's not a requirement.
>
> And looking back for implementation in v3 the whole mistake was to use
> GPIOCHIP_IRQ_RESOURCE_HELPERS. It just wanted custom callbacks with
> the IRQ chip data assigned to whatever from which we may then deduce
> struct gpio_chip. It does *not* require to be struct gpio_chip.
> The local driver data structure should keep pointer to struct gpio_regmap.
> That one can be used in the respective .irq_request_resources() and
> .irq_release_resources(). The default ones for gpio-regmap may also
> be provided via a macro, say GPIO_REGMAP_IRQ_RESOURCE_HELPERS.
>
> Hence I don't see the need of having gpio_regmap_get_gpiochip() helper
> and driver can be implemented using gpio-regmap and external IRQ domain.
>
> What did I miss?

IMHO nothing, that's exactly my understanding, too. Except for the
GPIO_REGMAP_IRQ_RESOURCE_HELPERS. See my previous mail. Because that
would have to be set by regmap-irq.

-michael


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]