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

["Yu-Chun Lin [林祐君]" <eleanor.lin@realtek.com>]

Hi Andy, Michael and Bartosz,

> 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?
>

The panic was caused by my driver including 'GPIOCHIP_IRQ_RESOURCE_HELPERS',
which forced the call to 'gpiochip_irq_reqres()' and crashed.

>>> This is all good and needs to be depicted in the cover-letter and/or commit message.

Yes, I will do it.

>>>
>>>> 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

Got it. I will go with Option #3 in the upcoming v4 patch.

Therefore, I will drop the patches 2, 3 and 4 from this series, and address
Andy's feedback on patch 6.

Best-regards,
Yu Chun