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

[Andy Shevchenko]

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.

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.

-- 
With Best Regards,
Andy Shevchenko