drivers/gpio/gpio-aggregator.c | 1 + 1 file changed, 1 insertion(+)
Restore the set_config operation, as it was lost during the refactoring of
the gpio-aggregator driver while creating the gpio forwarder library.
Fixes: b31c68fd851e7 ("gpio: aggregator: handle runtime registration of gpio_desc in gpiochip_fwd")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202509281206.a7334ae8-lkp@intel.com
Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---
drivers/gpio/gpio-aggregator.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 37600faf4a4b728ceb1937538b3f6b78ab3e90fa..416f265d09d070ee33e30bf6773e9d8fffc242bd 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -723,6 +723,7 @@ struct gpiochip_fwd *devm_gpiochip_fwd_alloc(struct device *dev,
chip->get_multiple = gpio_fwd_get_multiple_locked;
chip->set = gpio_fwd_set;
chip->set_multiple = gpio_fwd_set_multiple_locked;
+ chip->set_config = gpio_fwd_set_config;
chip->to_irq = gpio_fwd_to_irq;
chip->base = -1;
chip->ngpio = ngpios;
---
base-commit: bc061143637532c08d9fc657eec93fdc2588068e
change-id: 20250929-gpio-aggregator-fix-set-config-callback-6d2bff306023
Best regards,
--
Thomas Richard <thomas.richard@bootlin.com>From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
On Mon, 29 Sep 2025 12:03:13 +0200, Thomas Richard wrote:
> Restore the set_config operation, as it was lost during the refactoring of
> the gpio-aggregator driver while creating the gpio forwarder library.
>
>
Applied, thanks!
[1/1] gpio: aggregator: restore the set_config operation
https://git.kernel.org/brgl/linux/c/5232334baec371a3c9d9192ba7d2da2d88a85333
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
On Mon, Sep 29, 2025 at 12:03 PM Thomas Richard
<thomas.richard@bootlin.com> wrote:
>
> Restore the set_config operation, as it was lost during the refactoring of
> the gpio-aggregator driver while creating the gpio forwarder library.
>
> Fixes: b31c68fd851e7 ("gpio: aggregator: handle runtime registration of gpio_desc in gpiochip_fwd")
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202509281206.a7334ae8-lkp@intel.com
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> ---
> drivers/gpio/gpio-aggregator.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
> index 37600faf4a4b728ceb1937538b3f6b78ab3e90fa..416f265d09d070ee33e30bf6773e9d8fffc242bd 100644
> --- a/drivers/gpio/gpio-aggregator.c
> +++ b/drivers/gpio/gpio-aggregator.c
> @@ -723,6 +723,7 @@ struct gpiochip_fwd *devm_gpiochip_fwd_alloc(struct device *dev,
> chip->get_multiple = gpio_fwd_get_multiple_locked;
> chip->set = gpio_fwd_set;
> chip->set_multiple = gpio_fwd_set_multiple_locked;
> + chip->set_config = gpio_fwd_set_config;
> chip->to_irq = gpio_fwd_to_irq;
> chip->base = -1;
> chip->ngpio = ngpios;
>
> ---
Thanks for fixing it. I saw the report but I had already prepared my
big PR for Linus. This will still make v6.18-rc1, I will send it later
into the merge window. Please address Geert's review.
Bartosz
Hi Bartosz,
On 9/29/25 2:45 PM, Bartosz Golaszewski wrote:
> On Mon, Sep 29, 2025 at 12:03 PM Thomas Richard
> <thomas.richard@bootlin.com> wrote:
>>
>> Restore the set_config operation, as it was lost during the refactoring of
>> the gpio-aggregator driver while creating the gpio forwarder library.
>>
>> Fixes: b31c68fd851e7 ("gpio: aggregator: handle runtime registration of gpio_desc in gpiochip_fwd")
>> Reported-by: kernel test robot <oliver.sang@intel.com>
>> Closes: https://lore.kernel.org/oe-lkp/202509281206.a7334ae8-lkp@intel.com
>> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
>> ---
>> drivers/gpio/gpio-aggregator.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
>> index 37600faf4a4b728ceb1937538b3f6b78ab3e90fa..416f265d09d070ee33e30bf6773e9d8fffc242bd 100644
>> --- a/drivers/gpio/gpio-aggregator.c
>> +++ b/drivers/gpio/gpio-aggregator.c
>> @@ -723,6 +723,7 @@ struct gpiochip_fwd *devm_gpiochip_fwd_alloc(struct device *dev,
>> chip->get_multiple = gpio_fwd_get_multiple_locked;
>> chip->set = gpio_fwd_set;
>> chip->set_multiple = gpio_fwd_set_multiple_locked;
>> + chip->set_config = gpio_fwd_set_config;
>> chip->to_irq = gpio_fwd_to_irq;
>> chip->base = -1;
>> chip->ngpio = ngpios;
>>
>> ---
>
> Thanks for fixing it. I saw the report but I had already prepared my
> big PR for Linus. This will still make v6.18-rc1, I will send it later
> into the merge window. Please address Geert's review.
Gentle ping, just in case of you forgot it :)
Geert sent his Reviewed-by tag.
Best Regards,
Thomas
On Wed, Nov 5, 2025 at 10:59 AM Thomas Richard
<thomas.richard@bootlin.com> wrote:
>
> Hi Bartosz,
>
> On 9/29/25 2:45 PM, Bartosz Golaszewski wrote:
> > On Mon, Sep 29, 2025 at 12:03 PM Thomas Richard
> > <thomas.richard@bootlin.com> wrote:
> >>
> >> Restore the set_config operation, as it was lost during the refactoring of
> >> the gpio-aggregator driver while creating the gpio forwarder library.
> >>
> >> Fixes: b31c68fd851e7 ("gpio: aggregator: handle runtime registration of gpio_desc in gpiochip_fwd")
> >> Reported-by: kernel test robot <oliver.sang@intel.com>
> >> Closes: https://lore.kernel.org/oe-lkp/202509281206.a7334ae8-lkp@intel.com
> >> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> >> ---
> >> drivers/gpio/gpio-aggregator.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
> >> index 37600faf4a4b728ceb1937538b3f6b78ab3e90fa..416f265d09d070ee33e30bf6773e9d8fffc242bd 100644
> >> --- a/drivers/gpio/gpio-aggregator.c
> >> +++ b/drivers/gpio/gpio-aggregator.c
> >> @@ -723,6 +723,7 @@ struct gpiochip_fwd *devm_gpiochip_fwd_alloc(struct device *dev,
> >> chip->get_multiple = gpio_fwd_get_multiple_locked;
> >> chip->set = gpio_fwd_set;
> >> chip->set_multiple = gpio_fwd_set_multiple_locked;
> >> + chip->set_config = gpio_fwd_set_config;
> >> chip->to_irq = gpio_fwd_to_irq;
> >> chip->base = -1;
> >> chip->ngpio = ngpios;
> >>
> >> ---
> >
> > Thanks for fixing it. I saw the report but I had already prepared my
> > big PR for Linus. This will still make v6.18-rc1, I will send it later
> > into the merge window. Please address Geert's review.
>
> Gentle ping, just in case of you forgot it :)
> Geert sent his Reviewed-by tag.
>
> Best Regards,
> Thomas
>
It did indeed escape my attention, thanks for the heads-up. Now queued.
Bartosz
Hi Thomas,
On Mon, 29 Sept 2025 at 12:03, Thomas Richard
<thomas.richard@bootlin.com> wrote:
> Restore the set_config operation, as it was lost during the refactoring of
> the gpio-aggregator driver while creating the gpio forwarder library.
>
> Fixes: b31c68fd851e7 ("gpio: aggregator: handle runtime registration of gpio_desc in gpiochip_fwd")
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202509281206.a7334ae8-lkp@intel.com
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
Thanks for your patch!
> --- a/drivers/gpio/gpio-aggregator.c
> +++ b/drivers/gpio/gpio-aggregator.c
> @@ -723,6 +723,7 @@ struct gpiochip_fwd *devm_gpiochip_fwd_alloc(struct device *dev,
> chip->get_multiple = gpio_fwd_get_multiple_locked;
> chip->set = gpio_fwd_set;
> chip->set_multiple = gpio_fwd_set_multiple_locked;
> + chip->set_config = gpio_fwd_set_config;
> chip->to_irq = gpio_fwd_to_irq;
> chip->base = -1;
> chip->ngpio = ngpios;
>
Is there any specific reason why you are doing this unconditionally,
instead of only when any of its parents support .set_config(), like
was done before?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
> > Is there any specific reason why you are doing this unconditionally, > instead of only when any of its parents support .set_config(), like > was done before? > My idea was: it will be handled by the core, so the if statement is not needed. But if we conditionally add the operation we can save some time in case there is no chip supporting set_config(). Thomas
On 10/3/25 3:59 PM, Thomas Richard wrote: >> >> Is there any specific reason why you are doing this unconditionally, >> instead of only when any of its parents support .set_config(), like >> was done before? >> > My idea was: it will be handled by the core, so the if statement is not > needed. But if we conditionally add the operation we can save some time > in case there is no chip supporting set_config(). I just remembered the true reason why I'm doing this unconditionally. The user of the forwarder can override GPIO operations like I do in the pinctrl-upboard driver [1]. And now we can add/remove GPIO desc at runtime, if set_config() is set conditionally in gpiochip_fwd_desc_add() it will override the custom set_config() operation. So the only solution is to set the set_config() operation unconditionally in devm_gpiochip_fwd_alloc(). [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/pinctrl-upboard.c#n1044 Thomas
Hi Thomas,
On Fri, 3 Oct 2025 at 16:30, Thomas Richard <thomas.richard@bootlin.com> wrote:
> On 10/3/25 3:59 PM, Thomas Richard wrote:
> >> Is there any specific reason why you are doing this unconditionally,
> >> instead of only when any of its parents support .set_config(), like
> >> was done before?
> >>
> > My idea was: it will be handled by the core, so the if statement is not
> > needed. But if we conditionally add the operation we can save some time
> > in case there is no chip supporting set_config().
>
> I just remembered the true reason why I'm doing this unconditionally.
>
> The user of the forwarder can override GPIO operations like I do in the
> pinctrl-upboard driver [1].
> And now we can add/remove GPIO desc at runtime, if set_config() is set
> conditionally in gpiochip_fwd_desc_add() it will override the custom
> set_config() operation.
> So the only solution is to set the set_config() operation
> unconditionally in devm_gpiochip_fwd_alloc().
OK, that makes sense, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
I do find this overriding a bit fragile.
And in theory, such a driver could override chip->can_sleep to false,
which might be overwritten again by gpiochip_fwd_desc_add()...
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/pinctrl-upboard.c#n1044
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 10/6/25 9:42 AM, Geert Uytterhoeven wrote: > Hi Thomas, > > On Fri, 3 Oct 2025 at 16:30, Thomas Richard <thomas.richard@bootlin.com> wrote: >> On 10/3/25 3:59 PM, Thomas Richard wrote: >>>> Is there any specific reason why you are doing this unconditionally, >>>> instead of only when any of its parents support .set_config(), like >>>> was done before? >>>> >>> My idea was: it will be handled by the core, so the if statement is not >>> needed. But if we conditionally add the operation we can save some time >>> in case there is no chip supporting set_config(). >> >> I just remembered the true reason why I'm doing this unconditionally. >> >> The user of the forwarder can override GPIO operations like I do in the >> pinctrl-upboard driver [1]. >> And now we can add/remove GPIO desc at runtime, if set_config() is set >> conditionally in gpiochip_fwd_desc_add() it will override the custom >> set_config() operation. >> So the only solution is to set the set_config() operation >> unconditionally in devm_gpiochip_fwd_alloc(). > > OK, that makes sense, so > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > I do find this overriding a bit fragile. > And in theory, such a driver could override chip->can_sleep to false, > which might be overwritten again by gpiochip_fwd_desc_add()... Yes, I agree. Maybe we should not export gpiochip_fwd_get_gpiochip() so it will not be possible to get the gpio_chip and override some properties. And we add some helpers to override the GPIO operations. I can make a try. Regards, Thomas -- Thomas Richard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
© 2016 - 2026 Red Hat, Inc.