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

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

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

Hi,

On Mon Jun 8, 2026 at 4:10 PM CEST, Bartosz Golaszewski wrote:
> On Wed, 3 Jun 2026 02:34:40 +0200, Andy Shevchenko
> <andriy.shevchenko@intel.com> said:
>> On Mon, May 25, 2026 at 12:04:09PM +0000, Yu-Chun Lin [林祐君] wrote:
>>> > On Tue, May 12, 2026 at 11:33:12AM +0800, Yu-Chun Lin wrote:
>>> > > Expose an accessor function to retrieve the gpio_chip pointer from a
>>> > > gpio_regmap instance.
>>> > >
>>> > > This is needed by drivers that use gpio_regmap but also manage their
>>> > > own irq_chip, where gpiochip_enable_irq()/gpiochip_disable_irq() must
>>> > > be called with the gpio_chip pointer.
>>> > >
>>> > > Add gpio_regmap_get_gpiochip() to allow drivers with complex custom
>>> > > IRQ implementations.
>>> >
>>> > Hmm... Can't we rather add
>>> > gpio_regmap_enable_irq()/gpio_regmap_disable_irq()
>>> > that take regmap or GPIO regmap (whatever suits better for the purpose) and
>>> > do the magic inside GPIO regmap library code?
>>
>>> Thanks for the review! I apologize for the misleading commit message.
>>> The real reason I need the struct gpio_chip pointer is to properly set up a custom
>>> IRQ domain. Our SoC GPIO controller is quite complex. It routes different trigger
>>> types to multiple parent IRQs, which doesn't fit the generic regmap_irq framework.
>>> Therefore, we have to create our own irq_domain and pass it to
>>> gpio_regmap_config.irq_domain.
>>>
>>> The core problem occurs inside our custom irq_domain_ops.map() callback:
>>>
>>> static int rtd1625_gpio_irq_map(struct irq_domain *domain, unsigned int irq,
>>>                                 irq_hw_number_t hwirq)
>>> {
>>> 	struct rtd1625_gpio *data = domain->host_data;
>>> 	struct gpio_chip *gc = data->gpio_chip;
>>>
>>> 	/*
>>> 	 * The second argument MUST be struct gpio_chip *.
>>> 	 * If we pass our custom data structure here, the kernel will panic later
>>> 	 * in gpiochip_irq_reqres() when it calls irq_data_get_irq_chip_data()
>>> 	 * and strictly expects it to be a gpio_chip.
>>> 	 */
>>> 	irq_set_chip_data(irq, gc);
>>>
>>> 	irq_set_lockdep_class(irq, &rtd1625_gpio_irq_lock_class,
>>> 				&rtd1625_gpio_irq_request_class);
>>>
>>> 	irq_set_chip_and_handler(irq, &rtd1625_iso_gpio_irq_chip, handle_bad_irq);
>>> 	irq_set_noprobe(irq);
>>>
>>> 	return 0;
>>> }
>>>
>>> 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?

>> This is all good and needs to be depicted in the cover-letter and/or commit message.
>>
>>> Before I send a v4, I see 3 possible paths:
>>>
>>> Option 1: Keep the accessor (Current v3 approach)
>>> We keep gpio_regmap_get_gpiochip() but I will completely rewrite the commit message
>>> to explain the custom irq_domain_ops.map and lockdep requirements.
>>>
>>> Option 2: Let gpiolib create the irq_domain via gpio_regmap_config
>>> Instead of creating the irq_domain in our driver, we add all necessary IRQ fields
>>> (irq_chip, irq_handler, irq_parents, etc.) into struct gpio_regmap_config. Then
>>> gpio-regmap.c populates the gpio_irq_chip structure before calling
>>> gpiochip_add_data(). This prevents an early return and allows the core gpiolib
>>> (gpiochip_add_irqchip()) to automatically create the irq_domain for us.
>>> Drawback: This adds a lot of fields to gpio_regmap_config and might violate the
>>> original design philosophy of gpio-regmap.c (commit ebe363197e52), which explicitly
>>> states that it does not implement its own IRQ chip and delegates it to the parent
>>> driver.
>>>
>>> Option 3: Drop gpio-regmap entirely (Revert to v2 approach)
>>> Currently, all drivers using gpio-regmap (mostly simple CPLDs and external I/O cards)
>>> use regmap-irq to get their domain. Since our SoC has a complex IRQ routing scheme
>>> with multiple parents, maybe gpio-regmap is simply not the right tool for this
>>> hardware, and we should just implement a standard GPIO driver directly using gpiolib.
>>>
>>> Which approach would you prefer upstream?
>>
>> This question to Bart, Linus, and poissibly gpio-regmap stakeholders. I'm not sure
>> that my personal opinion will be the best fit here.
>>
>
> My preference would be for #2 but I understand that this could risk getting
> stuck in endless bikeshedding so I'm fine with going #3 with potential for
> future refactoring if we have more similar users.

Yeah, I'd like to keep that stuff out of gpio-regmap. But I'm on the
same boat regarding the refactoring if we have more data and
potential users.

-michael

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