Re: [PATCH v4 3/3] gpio: mmio: track the direction of chips without direction registers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect parsing of packed pin configuration value in `gpio_mmio_seed_dir_from_pinctrl()` due to conflicting API contract.
--

commit 38b44b07fd26f5fd809a77e8d6044980715db7d8
Author: Mehmet Fide <mehmet.fide@screeningeagle.com>

    gpio: mmio: track the direction of chips without direction registers

This commit tracks the direction of generic GPIO chips with a pinctrl
backend and no direction registers. It keeps the direction in the existing
shadow and updates it under the chip lock, reading the state once during
line request from the pin controller.

> diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
> index 7e4b3e8d609f3..0709ffaab4dec 100644
> --- a/drivers/gpio/gpio-mmio.c
> +++ b/drivers/gpio/gpio-mmio.c
[ ... ]
> @@ -601,20 +622,51 @@ static int gpio_mmio_setup_direction(struct gpio_generic_chip *chip,
[ ... ]
> +static void gpio_mmio_seed_dir_from_pinctrl(struct gpio_chip *gc,
> +					    unsigned int gpio)
> +{
> +	struct gpio_generic_chip *chip = to_gpio_generic_chip(gc);
> +	unsigned long config;
> +
> +	if (!gc->get_config || chip->reg_dir_out || chip->reg_dir_in)
> +		return;
> +
> +	config = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 0);
> +	if (gc->get_config(gc, gpio, &config))
> +		return;
> +
> +	guard(raw_spinlock_irqsave)(&chip->lock);
> +	if (config)
> +		chip->sdir |= gpio_mmio_line2mask(gc, gpio);
> +	else
> +		chip->sdir &= ~gpio_mmio_line2mask(gc, gpio);
> +}

[Severity: Medium]
Are we correctly parsing the packed pin configuration value here?

The documentation for gc->get_config in include/linux/gpio/driver.h states it
uses the packed config format where the packed parameter and argument come out.

Because the packed format places the non-zero parameter ID
(PIN_CONFIG_OUTPUT_ENABLE) in the lower 8 bits, will evaluating config directly
as a boolean always be true?

If a conforming custom GPIO driver implements gc->get_config according to the
documented API, will its direction be unconditionally forced to output mode by
this check?

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