* [PATCH RFT] pci: imx: Properly support upstream Linux reset-gpios property
@ 2026-03-12 19:26 Krzysztof Kozlowski
2026-03-13 8:12 ` Max Merchel
0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-12 19:26 UTC (permalink / raw)
To: Tom Rini, u-boot; +Cc: linux-kernel, Krzysztof Kozlowski
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@oss.qualcomm.com>
---
1. https://lore.kernel.org/linux-devicetree/20260312-dts-snps-reset-gpios-v2-12-0d5040eb4a1e@oss.qualcomm.com/
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");
+ } 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);
--
2.51.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFT] pci: imx: Properly support upstream Linux reset-gpios property
2026-03-12 19:26 [PATCH RFT] pci: imx: Properly support upstream Linux reset-gpios property Krzysztof Kozlowski
@ 2026-03-13 8:12 ` Max Merchel
2026-03-13 8:14 ` Krzysztof Kozlowski
0 siblings, 1 reply; 5+ messages in thread
From: Max Merchel @ 2026-03-13 8:12 UTC (permalink / raw)
To: Krzysztof Kozlowski, Tom Rini, u-boot; +Cc: linux-kernel
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@oss.qualcomm.com>
>
> ---
>
> 1. https://lore.kernel.org/linux-devicetree/20260312-dts-snps-reset-gpios-v2-12-0d5040eb4a1e@oss.qualcomm.com/
> 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/
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFT] pci: imx: Properly support upstream Linux reset-gpios property
2026-03-13 8:12 ` Max Merchel
@ 2026-03-13 8:14 ` Krzysztof Kozlowski
2026-03-13 8:28 ` Max Merchel
0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-13 8:14 UTC (permalink / raw)
To: Max Merchel, Tom Rini, u-boot; +Cc: linux-kernel
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@oss.qualcomm.com>
>>
>> ---
>>
>> 1. https://lore.kernel.org/linux-devicetree/20260312-dts-snps-reset-gpios-v2-12-0d5040eb4a1e@oss.qualcomm.com/
>> 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.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFT] pci: imx: Properly support upstream Linux reset-gpios property
2026-03-13 8:14 ` Krzysztof Kozlowski
@ 2026-03-13 8:28 ` Max Merchel
2026-03-13 14:53 ` Tom Rini
0 siblings, 1 reply; 5+ messages in thread
From: Max Merchel @ 2026-03-13 8:28 UTC (permalink / raw)
To: Krzysztof Kozlowski, Tom Rini, u-boot; +Cc: linux-kernel
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@oss.qualcomm.com>
>>>
>>> ---
>>>
>>> 1. https://lore.kernel.org/linux-devicetree/20260312-dts-snps-reset-gpios-v2-12-0d5040eb4a1e@oss.qualcomm.com/
>>> 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/
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFT] pci: imx: Properly support upstream Linux reset-gpios property
2026-03-13 8:28 ` Max Merchel
@ 2026-03-13 14:53 ` Tom Rini
0 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2026-03-13 14:53 UTC (permalink / raw)
To: Max Merchel; +Cc: Krzysztof Kozlowski, u-boot, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3166 bytes --]
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@oss.qualcomm.com>
> > > >
> > > > ---
> > > >
> > > > 1. https://lore.kernel.org/linux-devicetree/20260312-dts-snps-reset-gpios-v2-12-0d5040eb4a1e@oss.qualcomm.com/
> > > > 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-13 14:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-12 19:26 [PATCH RFT] pci: imx: Properly support upstream Linux reset-gpios property Krzysztof Kozlowski
2026-03-13 8:12 ` Max Merchel
2026-03-13 8:14 ` Krzysztof Kozlowski
2026-03-13 8:28 ` Max Merchel
2026-03-13 14:53 ` Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®