Re: [PATCH v4 2/2] gpio: realtek-otto: add pinctrl support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional use of GPIO_GENERIC_PINCTRL_BACKEND breaks GPIO requests and direction configuration on SoCs without a pinctrl driver.
--

commit e286232a1c082edc58e67d8d4c62156ff41a0294
Author: Michael Zavertkin <misha.zavertkin@mail.ru>

gpio: realtek-otto: add pinctrl support

This commit adds pinctrl backend support to the realtek-otto GPIO driver
so that RTL9607C can use pinctrl to request pins.

> diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-otto.c
> index 37ef56f453187..b53c2a9a04c76 100644
> --- a/drivers/gpio/gpio-realtek-otto.c
> +++ b/drivers/gpio/gpio-realtek-otto.c
> @@ -394,13 +394,14 @@ static int realtek_gpio_probe(struct platform_device *pdev)
>  
>  	raw_spin_lock_init(&ctrl->lock);
>  
> +	gen_gc_flags = GPIO_GENERIC_PINCTRL_BACKEND;
> +

[Severity: High]
Does unconditionally setting GPIO_GENERIC_PINCTRL_BACKEND break GPIO
functionality for SoCs that lack a pinctrl driver?

When a GPIO is requested or its direction is set on an SoC without a pinctrl
driver, gpio_generic_chip_init() routes the calls to pinctrl_gpio_request() and
pinctrl_gpio_direction(). 

Because pinctrl_gpio_direction() calls pinctrl_get_device_gpio_range(), it will
return -EPROBE_DEFER when no pinctrl device overlaps with the GPIO chip. This
would cause all GPIO requests and direction configurations to fail indefinitely,
rendering GPIO unusable and breaking dependent devices on hardware like RTL8380
or RTL9300, which the commit message acknowledges might have pinctrl drivers
implemented later.

Should this backend flag only be set conditionally, perhaps by checking for
gpio-ranges in the device tree?

>  	if (dev_flags & GPIO_PORTS_REVERSED) {
> -		gen_gc_flags = 0;
>  		ctrl->bank_read = realtek_gpio_bank_read;
>  		ctrl->bank_write = realtek_gpio_bank_write;
>  		ctrl->line_imr_pos = realtek_gpio_line_imr_pos;
>  	} else {
> -		gen_gc_flags = GPIO_GENERIC_BIG_ENDIAN_BYTE_ORDER;
> +		gen_gc_flags |= GPIO_GENERIC_BIG_ENDIAN_BYTE_ORDER;
>  		ctrl->bank_read = realtek_gpio_bank_read_swapped;
>  		ctrl->bank_write = realtek_gpio_bank_write_swapped;
>  		ctrl->line_imr_pos = realtek_gpio_line_imr_pos_swapped;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787830440.git.misha.zavertkin@mail.ru?part=2