[PATCH v3 2/7] gpio: regmap: add gpio_regmap_get_gpiochip() accessor
Yu-Chun Lin [林祐君]
eleanor.lin at realtek.com
Wed Jun 17 01:36:34 PDT 2026
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 at 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
More information about the linux-arm-kernel
mailing list