Re: [PATCH v5 2/3] gpiolib: add get_config() and gpiochip_generic_get_config()

[Bough Chen <haibo.chen@oss.nxp.com>]

On Thu, Sep 03, 2026 at 08:14:31AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The fallback stub for `pinctrl_gpio_get_config()` returns 0 without modifying the output `config` parameter, causing callers to use query enums or uninitialized memory as hardware states.
> - [High] `gpiochip_generic_config()` returns 0 (success) when `CONFIG_PINCTRL` is disabled, silently ignoring hardware configuration requests and creating an API inconsistency.
> --
> 
> commit 5e30160f0acecf52b9ebfedbbaf09c6e99e8ce08
> Author: Mehmet Fide <mehmet.fide@screeningeagle.com>
> Subject: gpiolib: add get_config() and gpiochip_generic_get_config()
> 
> This commit introduces a new get_config() callback for gpiochip and provides
> gpiochip_generic_get_config() as the generic pin control backend implementation.
> This allows a chip to read back a line's configuration, which is useful for
> determining line direction when the pin controller owns it.
> 
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index ef8ccaf17c9ce..fb3ef6754a9ee 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -2440,6 +2440,29 @@ int gpiochip_generic_config(struct gpio_chip *gc, unsigned int offset,
> >  }
> >  EXPORT_SYMBOL_GPL(gpiochip_generic_config);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but right above this new function,
> gpiochip_generic_config() delegates to pinctrl_gpio_set_config(). When
> CONFIG_PINCTRL is disabled, it skips the pin_ranges check and calls the stub,
> which silently returns 0:
> 
> include/linux/pinctrl/consumer.h:pinctrl_gpio_set_config() {
>     return 0;
> }
> 
> Does this cause hardware configuration requests to be silently ignored while
> reporting success to the caller? The new getter correctly avoids this by
> returning -ENOTSUPP when the backend is disabled. Should the pre-existing
> setter stub also be updated to return -ENOTSUPP to ensure consistent API
> behavior?

I think Sashiko is correct, better to aligned here, but in a separate patch.

> 
> >  
> > +/**
> > + * gpiochip_generic_get_config() - read back the configuration of a pin
> > + * @gc: the gpiochip owning the GPIO
> > + * @offset: the offset of the GPIO to query
> > + * @config: the packed parameter to query, replaced by its bare argument
> > + *
> > + * Returns:
> > + * 0 on success, or negative errno on failure.
> > + */
> > +int gpiochip_generic_get_config(struct gpio_chip *gc, unsigned int offset,
> > +				unsigned long *config)
> > +{
> > +#ifdef CONFIG_PINCTRL
> > +	if (list_empty(&gc->gpiodev->pin_ranges))
> > +		return -ENOTSUPP;
> > +
> > +	return pinctrl_gpio_get_config(gc, offset, config);
> > +#else
> > +	return -ENOTSUPP;
> > +#endif
> 
> [Severity: High]
> This is a pre-existing issue, but I notice this helper explicitly works
> around the behavior of the fallback stub for pinctrl_gpio_get_config(). 
> 
> Since the stub returns 0 without modifying the output config parameter:
> 
> include/linux/pinctrl/consumer.h:pinctrl_gpio_get_config() {
>     return 0;
> }
> 
> direct callers (such as pin_control_gpio_get_direction() in
> drivers/gpio/gpio-by-pinctrl.c) are left reading uninitialized memory or
> query enum values as hardware states when CONFIG_PINCTRL is disabled. 
> 
> Should the root stub in include/linux/pinctrl/consumer.h be fixed to return
> -ENOTSUPP directly to prevent other callers from breaking?

This is also correct, and more critical than the first one. If changing the
CONFIG_PINCTRL=n stub in include/linux/pinctrl/consumer.h to return -ENOTSUPP,
then I think you can drop the explicit "#else return -ENOTSUPP" in 
gpiochip_generic_get_config()

It depends on you to send a new version or a separate patch for this.

Regards
Haibo Chen
> 
> > +}
> > +EXPORT_SYMBOL_GPL(gpiochip_generic_get_config);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260903075940.2089367-1-mehmet.fide@gmail.com?part=2