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

[Tom Rini]

On Fri, Mar 13, 2026 at 09:28:23AM +0100, Max Merchel wrote:
> 
> 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.

Developers should be enabling OF_UPSTREAM and so that won't have slipped
through?

-- 
Tom

Attachment: signature.asc
Description: PGP signature