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

[sashiko-bot@kernel.org]

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?

>  
> +/**
> + * 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?

> +}
> +EXPORT_SYMBOL_GPL(gpiochip_generic_get_config);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903075940.2089367-1-mehmet.fide@gmail.com?part=2