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

[Max Merchel]

On 3/13/26 09:14, Krzysztof Kozlowski wrote:
> On 13/03/2026 09:12, Max Merchel wrote:
> > 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");
> Why? What would end user seeing this in their serial console do? The
> deprecated property is not a mistake, it's just deprecated.

You're right. It's not a message for the end user, but for the 
developers. Therefore, it would be better to add a debug message.

> Best regards,
> Krzysztof

--
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/