From: Andy Shevchenko <andriy.shevchenko@intel•com>
To: "Yu-Chun Lin [林祐君]" <eleanor.lin@realtek•com>
Cc: "linusw@kernel•org" <linusw@kernel•org>,
"mwalle@kernel•org" <mwalle@kernel•org>,
"robh@kernel•org" <robh@kernel•org>,
"krzk+dt@kernel•org" <krzk+dt@kernel•org>,
"conor+dt@kernel•org" <conor+dt@kernel•org>,
"afaerber@suse•com" <afaerber@suse•com>,
"wbg@kernel•org" <wbg@kernel•org>,
"mathieu.dubois-briand@bootlin•com"
<mathieu.dubois-briand@bootlin•com>,
"lars@metafoo•de" <lars@metafoo•de>,
"Michael.Hennerich@analog•com" <Michael.Hennerich@analog•com>,
"jic23@kernel•org" <jic23@kernel•org>,
"nuno.sa@analog•com" <nuno.sa@analog•com>,
"andy@kernel•org" <andy@kernel•org>,
"dlechner@baylibre•com" <dlechner@baylibre•com>,
"TY_Chang[張子逸]" <tychang@realtek•com>,
"linux-gpio@vger•kernel.org" <linux-gpio@vger•kernel.org>,
"devicetree@vger•kernel.org" <devicetree@vger•kernel.org>,
"linux-kernel@vger•kernel.org" <linux-kernel@vger•kernel.org>,
"linux-arm-kernel@lists•infradead.org"
<linux-arm-kernel@lists•infradead.org>,
"linux-realtek-soc@lists•infradead.org"
<linux-realtek-soc@lists•infradead.org>,
"linux-iio@vger•kernel.org" <linux-iio@vger•kernel.org>,
"CY_Huang[黃鉦晏]" <cy.huang@realtek•com>,
"Stanley Chang[昌育德]" <stanley_chang@realtek•com>,
"James Tai [戴志峰]" <james.tai@realtek•com>,
"brgl@kernel•org" <brgl@kernel•org>
Subject: Re: [PATCH v3 2/7] gpio: regmap: add gpio_regmap_get_gpiochip() accessor
Date: Wed, 3 Jun 2026 03:34:40 +0300 [thread overview]
Message-ID: <ah92oEavMu4QRn8y@ashevche-desk.local> (raw)
In-Reply-To: <adff3a2d21a64d3ea3b408d62157ee1e@realtek.com>
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
next prev parent reply other threads:[~2026-06-03 0:35 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-12 3:33 [PATCH v3 0/7] gpio: realtek: Add support for Realtek DHC RTD1625 Yu-Chun Lin
2026-05-12 3:33 ` [PATCH v3 1/7] gpio: Replace "default y" with "default ARCH_REALTEK" in Kconfig Yu-Chun Lin
2026-05-12 3:33 ` [PATCH v3 2/7] gpio: regmap: add gpio_regmap_get_gpiochip() accessor Yu-Chun Lin
2026-05-12 11:20 ` Andy Shevchenko
2026-05-25 12:04 ` Yu-Chun Lin [林祐君]
2026-06-03 0:34 ` Andy Shevchenko [this message]
2026-05-12 3:33 ` [PATCH v3 3/7] gpio: regmap: Add gpio_regmap_operation and write-enable support Yu-Chun Lin
2026-05-12 11:26 ` Andy Shevchenko
2026-05-12 14:37 ` Jonathan Cameron
2026-05-13 7:40 ` Linus Walleij
2026-05-12 3:33 ` [PATCH v3 4/7] gpio: regmap: Add set_config callback Yu-Chun Lin
2026-05-12 18:12 ` Andy Shevchenko
2026-05-12 3:33 ` [PATCH v3 5/7] dt-bindings: gpio: realtek: Add realtek,rtd1625-gpio Yu-Chun Lin
2026-05-12 3:33 ` [PATCH v3 6/7] gpio: realtek: Add driver for Realtek DHC RTD1625 SoC Yu-Chun Lin
2026-05-12 18:50 ` Andy Shevchenko
2026-05-12 3:33 ` [PATCH v3 7/7] arm64: dts: realtek: Add GPIO support for RTD1625 Yu-Chun Lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ah92oEavMu4QRn8y@ashevche-desk.local \
--to=andriy.shevchenko@intel$(echo .)com \
--cc=Michael.Hennerich@analog$(echo .)com \
--cc=afaerber@suse$(echo .)com \
--cc=andy@kernel$(echo .)org \
--cc=brgl@kernel$(echo .)org \
--cc=conor+dt@kernel$(echo .)org \
--cc=cy.huang@realtek$(echo .)com \
--cc=devicetree@vger$(echo .)kernel.org \
--cc=dlechner@baylibre$(echo .)com \
--cc=eleanor.lin@realtek$(echo .)com \
--cc=james.tai@realtek$(echo .)com \
--cc=jic23@kernel$(echo .)org \
--cc=krzk+dt@kernel$(echo .)org \
--cc=lars@metafoo$(echo .)de \
--cc=linusw@kernel$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-gpio@vger$(echo .)kernel.org \
--cc=linux-iio@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-realtek-soc@lists$(echo .)infradead.org \
--cc=mathieu.dubois-briand@bootlin$(echo .)com \
--cc=mwalle@kernel$(echo .)org \
--cc=nuno.sa@analog$(echo .)com \
--cc=robh@kernel$(echo .)org \
--cc=stanley_chang@realtek$(echo .)com \
--cc=tychang@realtek$(echo .)com \
--cc=wbg@kernel$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox