Re: [PATCH RFT] pci: imx: Properly support upstream Linux reset-gpios property

[Max Merchel]

On 3/12/26 20:26, Krzysztof Kozlowski wrote:
> The driver requests explicitly "reset-gpio" property, not the one with
> "gpios" suffix but upstream Linux kernel deprecated it in 2021.
> Existing upstream Linux kernel DTS is being changed to "reset-gpios"
> property, thus update the driver to read that one too.
>
> Note that driver is probably broken already, because it parsed GPIO in
> standard way respecting the flags and on top of that applied the
> "reset-gpio-active-high" flag, thus "reset-gpio ACTIVE_LOW" with the
> "reset-gpio-active-high" property would be double inverted.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxxxxx>
>
> ---
>
> 1. https://lore.kernel.org/linux-devicetree/20260312-dts-snps-reset-gpios-v2-12-0d5040eb4a1e@xxxxxxxxxxxxxxxx/
> 2. See Linux kernel commit 42694f9f6407 ("dt-bindings: PCI: add snps,dw-pcie.yaml")
> ---
>   drivers/pci/pcie_imx.c | 22 +++++++++++++++++++---
>   1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pcie_imx.c b/drivers/pci/pcie_imx.c
> index 11c4ccbfc555..8d853ecf2c22 100644
> --- a/drivers/pci/pcie_imx.c
> +++ b/drivers/pci/pcie_imx.c
> @@ -728,15 +728,31 @@ static int imx_pcie_dm_write_config(struct udevice *dev, pci_dev_t bdf,
>   static int imx_pcie_dm_probe(struct udevice *dev)
>   {
>   	struct imx_pcie_priv *priv = dev_get_priv(dev);
> +	int ret;
>   
>   #if CONFIG_IS_ENABLED(DM_REGULATOR)
>   	device_get_supply_regulator(dev, "vpcie-supply", &priv->vpcie);
>   #endif
>   
>   	/* if PERST# valid from dt then assert it */
> -	gpio_request_by_name(dev, "reset-gpio", 0, &priv->reset_gpio,
> -			     GPIOD_IS_OUT);
> -	priv->reset_active_high = dev_read_bool(dev, "reset-gpio-active-high");
> +	ret = gpio_request_by_name(dev, "reset-gpio", 0, &priv->reset_gpio,
> +				   GPIOD_IS_OUT);
> +	if (!ret) {
> +		/*
> +		 * Legacy property, invert assert logic based on
> +		 * reset-gpio-active-high. This won't work if flags are not
> +		 * matching the reset-gpio-active-high.
> +		 */
> +		priv->reset_active_high = dev_read_bool(dev, "reset-gpio-active-high");

A warning should also be issued when using the deprecated "reset-gpio" 
property.

            dev_warn(dev, "reset-gpio property deprecated, use 
reset-gpios\n");

> +	} else {
> +		/*
> +		 * Linux kernel upstream property, assert active level based on
> +		 * GPIO flags, thus leave priv->reset_active_high=0.
> +		 */
> +		gpio_request_by_name(dev, "reset-gpios", 0, &priv->reset_gpio,
> +				     GPIOD_IS_OUT);
> +	}
> +
>   	if (dm_gpio_is_valid(&priv->reset_gpio)) {
>   		dm_gpio_set_value(&priv->reset_gpio,
>   				  priv->reset_active_high ? 0 : 1);

--
Best regards,
Max

TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/