[PATCH 0/2] PCI: imx6: Improve PERST# fallback logic

[PATCH 0/2] PCI: imx6: Improve PERST# fallback logic

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

The pci_host_common_parse_port() shouldn't decide whether to fall back
to the legacy RC-level binding by checking for "reset-gpios/reset-gpio"
properties on the RC node and returning -ENODEV. That's a policy
decision belongs to the caller, not this common helper.

This patch set improves the PERST# (PCIe reset) GPIO fallback logic
across the generic PCI host bridge helper and the i.MX6 PCIe driver.

Sherry Sun (2):
  PCI: host-generic: Simplify return value handling in
    pci_host_common_parse_port(s)
  PCI: imx6: Add imx_pcie_perst_found() to inspect the parsed result

 drivers/pci/controller/dwc/pci-imx6.c    | 25 +++++++++++++-------
 drivers/pci/controller/pci-host-common.c | 29 ++++--------------------
 2 files changed, 22 insertions(+), 32 deletions(-)


base-commit: 550604d6c9b9efc8d068aff94dc301694a7afdee
-- 
2.37.1

[PATCH 1/2] PCI: host-generic: Simplify return value handling in pci_host_common_parse_port(s)

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

The pci_host_common_parse_port() shouldn't check the RC-level binding.
That's a policy decision that belongs to the caller, not this common
helper.

Simplify pci_host_common_parse_port() to only parses properties from the
Root Port(and its children) without checking the RC node. Also change
pci_host_common_parse_ports() to return 0 when no ports are found, since
it is not an error.

So now both functions won't return failure for "property not found" or
"port not found", they purely returns 0 on success and a negative error
code on real failures.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/pci/controller/pci-host-common.c | 29 ++++--------------------
 1 file changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index 67455caaf9e6..7ce5a939e3d9 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -108,8 +108,7 @@ static int pci_host_common_parse_perst(struct device *dev,
  * dependencies and the driver may fail to operate if required resources
  * are missing.
  *
- * Return: 0 on success, -ENODEV if PERST# found in RC node (legacy binding
- * should be used), Other negative error codes on failure.
+ * Return: 0 on success, negative error codes on failure.
  */
 static int pci_host_common_parse_port(struct device *dev,
 				      struct pci_host_bridge *bridge,
@@ -128,22 +127,6 @@ static int pci_host_common_parse_port(struct device *dev,
 	if (ret)
 		return ret;
 
-	/*
-	 * 1. PERST# found in RP or its child nodes - list is not empty,
-	 *    continue
-	 *
-	 * 2. PERST# not found in RP/children, but found in RC node -
-	 *    return -ENODEV to fallback legacy binding
-	 *
-	 * 3. PERST# not found anywhere - list is empty, continue (optional
-	 *    PERST#)
-	 */
-	if (list_empty(&port->perst)) {
-		if (of_property_present(dev->of_node, "reset-gpios") ||
-		    of_property_present(dev->of_node, "reset-gpio"))
-			return -ENODEV;
-	}
-
 	INIT_LIST_HEAD(&port->list);
 	list_add_tail(&port->list, &bridge->ports);
 
@@ -158,13 +141,11 @@ static int pci_host_common_parse_port(struct device *dev,
  * Iterate through child nodes of the host bridge and parse Root Port
  * properties (currently only reset GPIOs).
  *
- * Return: 0 on success, -ENODEV if no ports found or PERST# found in RC
- * node (legacy binding should be used), Other negative error codes on
- * failure.
+ * Return: 0 on success, negative error codes on failure.
  */
 int pci_host_common_parse_ports(struct device *dev, struct pci_host_bridge *bridge)
 {
-	int ret = -ENODEV;
+	int ret = 0;
 
 	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
 		if (!of_node_is_type(of_port, "pci"))
@@ -174,8 +155,8 @@ int pci_host_common_parse_ports(struct device *dev, struct pci_host_bridge *brid
 			goto err_cleanup;
 	}
 
-	if (ret)
-		return ret;
+	if (list_empty(&bridge->ports))
+		return 0;
 
 	return devm_add_action_or_reset(dev, pci_host_common_delete_ports,
 					&bridge->ports);
-- 
2.37.1

Re: [PATCH 1/2] PCI: host-generic: Simplify return value handling in pci_host_common_parse_port(s)

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Changing the return value of `pci_host_common_parse_ports()` to 0 instead of `-ENODEV` silently breaks the legacy device tree fallback logic in `imx_pcie_host_init()`.
--

commit 8c94bf783e4da5a465164e0d82606c1c5b6786e2
Author: Sherry Sun <sherry.sun@nxp.com>

PCI: host-generic: Simplify return value handling in pci_host_common_parse_port(s)

This commit simplifies pci_host_common_parse_port() by removing the
RC-level binding check, leaving it as a policy decision for the caller.
It also changes pci_host_common_parse_ports() to return 0 when no ports
are found, instead of -ENODEV.

> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index 67455caaf9e68..7ce5a939e3d94 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
[ ... ]
> @@ -158,13 +141,11 @@ static int pci_host_common_parse_port(struct device *dev,
>   * Iterate through child nodes of the host bridge and parse Root Port
>   * properties (currently only reset GPIOs).
>   *
> - * Return: 0 on success, -ENODEV if no ports found or PERST# found in RC
> - * node (legacy binding should be used), Other negative error codes on
> - * failure.
> + * Return: 0 on success, negative error codes on failure.
>   */
>  int pci_host_common_parse_ports(struct device *dev, struct pci_host_bridge *bridge)
>  {
> -	int ret = -ENODEV;
> +	int ret = 0;
>  
>  	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
>  		if (!of_node_is_type(of_port, "pci"))
[ ... ]
> @@ -174,8 +155,8 @@ int pci_host_common_parse_ports(struct device *dev, struct pci_host_bridge *brid
>  			goto err_cleanup;
>  	}
>  
> -	if (ret)
> -		return ret;
> +	if (list_empty(&bridge->ports))
> +		return 0;

[Severity: High]
Does changing this return value to 0 break the legacy device tree fallback
logic in imx_pcie_host_init()?

Before this patch, pci_host_common_parse_ports() returned -ENODEV if no
ports were found or if a legacy reset-gpios property was detected in the
Root Complex node. The caller imx_pcie_host_init() explicitly relies on
ret == -ENODEV to safely fall back to imx_pcie_parse_legacy_binding().

By removing the RC-level check and returning 0 on empty lists,
imx_pcie_host_init() will see success instead of an error, completely
bypassing the legacy binding fallback. This would cause the PERST# GPIO
to never be parsed or requested, leading to device initialization failure
on i.MX6 platforms using older legacy device tree bindings.

I see that this issue is fixed later in the series by a subsequent commit
("PCI: imx6: Add imx_pcie_perst_found() to inspect the parsed result").
However, kernel rules require each intermediate patch to be self-contained
without introducing regressions. Does this intermediate state break
bisectability for i.MX6 platforms?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522034344.1147775-1-sherry.sun@oss.nxp.com?part=1

RE: [PATCH 1/2] PCI: host-generic: Simplify return value handling in pci_host_common_parse_port(s)

[Hongxing Zhu]

> -----Original Message-----
> From: Sherry Sun (OSS) <sherry.sun@oss.nxp.com>
> Sent: Friday, May 22, 2026 11:44 AM
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de; Frank Li
> <frank.li@nxp.com>; bhelgaas@google.com; lpieralisi@kernel.org;
> kwilczynski@kernel.org; mani@kernel.org; robh@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> will@kernel.org
> Cc: imx@lists.linux.dev; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Sherry Sun
> <sherry.sun@nxp.com>
> Subject: [PATCH 1/2] PCI: host-generic: Simplify return value handling in
> pci_host_common_parse_port(s)
> 
> From: Sherry Sun <sherry.sun@nxp.com>
> 
> The pci_host_common_parse_port() shouldn't check the RC-level binding.
> That's a policy decision that belongs to the caller, not this common helper.
> 
> Simplify pci_host_common_parse_port() to only parses properties from the Root
"to only parses"/s/"to only parse"

> Port(and its children) without checking the RC node. Also change
Missing space after "Port".

> pci_host_common_parse_ports() to return 0 when no ports are found, since it is
> not an error.
> 
> So now both functions won't return failure for "property not found" or "port not
> found", they purely returns 0 on success and a negative error code on real
returns/s/return

Best Regards
Richard Zhu

> failures.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
>  drivers/pci/controller/pci-host-common.c | 29 ++++--------------------
>  1 file changed, 5 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-
> host-common.c
> index 67455caaf9e6..7ce5a939e3d9 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -108,8 +108,7 @@ static int pci_host_common_parse_perst(struct device
> *dev,
>   * dependencies and the driver may fail to operate if required resources
>   * are missing.
>   *
> - * Return: 0 on success, -ENODEV if PERST# found in RC node (legacy binding
> - * should be used), Other negative error codes on failure.
> + * Return: 0 on success, negative error codes on failure.
>   */
>  static int pci_host_common_parse_port(struct device *dev,
>  				      struct pci_host_bridge *bridge, @@ -128,22
> +127,6 @@ static int pci_host_common_parse_port(struct device *dev,
>  	if (ret)
>  		return ret;
> 
> -	/*
> -	 * 1. PERST# found in RP or its child nodes - list is not empty,
> -	 *    continue
> -	 *
> -	 * 2. PERST# not found in RP/children, but found in RC node -
> -	 *    return -ENODEV to fallback legacy binding
> -	 *
> -	 * 3. PERST# not found anywhere - list is empty, continue (optional
> -	 *    PERST#)
> -	 */
> -	if (list_empty(&port->perst)) {
> -		if (of_property_present(dev->of_node, "reset-gpios") ||
> -		    of_property_present(dev->of_node, "reset-gpio"))
> -			return -ENODEV;
> -	}
> -
>  	INIT_LIST_HEAD(&port->list);
>  	list_add_tail(&port->list, &bridge->ports);
> 
> @@ -158,13 +141,11 @@ static int pci_host_common_parse_port(struct device
> *dev,
>   * Iterate through child nodes of the host bridge and parse Root Port
>   * properties (currently only reset GPIOs).
>   *
> - * Return: 0 on success, -ENODEV if no ports found or PERST# found in RC
> - * node (legacy binding should be used), Other negative error codes on
> - * failure.
> + * Return: 0 on success, negative error codes on failure.
>   */
>  int pci_host_common_parse_ports(struct device *dev, struct pci_host_bridge
> *bridge)  {
> -	int ret = -ENODEV;
> +	int ret = 0;
> 
>  	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
>  		if (!of_node_is_type(of_port, "pci")) @@ -174,8 +155,8 @@ int
> pci_host_common_parse_ports(struct device *dev, struct pci_host_bridge *brid
>  			goto err_cleanup;
>  	}
> 
> -	if (ret)
> -		return ret;
> +	if (list_empty(&bridge->ports))
> +		return 0;
> 
>  	return devm_add_action_or_reset(dev, pci_host_common_delete_ports,
>  					&bridge->ports);
> --
> 2.37.1

RE: [PATCH 1/2] PCI: host-generic: Simplify return value handling in pci_host_common_parse_port(s)

[Sherry Sun]

> > Subject: [PATCH 1/2] PCI: host-generic: Simplify return value handling
> > in
> > pci_host_common_parse_port(s)
> >
> > From: Sherry Sun <sherry.sun@nxp.com>
> >
> > The pci_host_common_parse_port() shouldn't check the RC-level binding.
> > That's a policy decision that belongs to the caller, not this common helper.
> >
> > Simplify pci_host_common_parse_port() to only parses properties from
> > the Root
> "to only parses"/s/"to only parse"
> 
> > Port(and its children) without checking the RC node. Also change
> Missing space after "Port".
> 
> > pci_host_common_parse_ports() to return 0 when no ports are found,
> > since it is not an error.
> >
> > So now both functions won't return failure for "property not found" or
> > "port not found", they purely returns 0 on success and a negative
> > error code on real
> returns/s/return

Thanks, will fix these issues.

Best Regards
Sherry Sun

> 
> Best Regards
> Richard Zhu
> 
> > failures.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> >  drivers/pci/controller/pci-host-common.c | 29
> > ++++--------------------
> >  1 file changed, 5 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pci-host-common.c
> > b/drivers/pci/controller/pci- host-common.c index
> > 67455caaf9e6..7ce5a939e3d9 100644
> > --- a/drivers/pci/controller/pci-host-common.c
> > +++ b/drivers/pci/controller/pci-host-common.c
> > @@ -108,8 +108,7 @@ static int pci_host_common_parse_perst(struct
> > device *dev,
> >   * dependencies and the driver may fail to operate if required resources
> >   * are missing.
> >   *
> > - * Return: 0 on success, -ENODEV if PERST# found in RC node (legacy
> > binding
> > - * should be used), Other negative error codes on failure.
> > + * Return: 0 on success, negative error codes on failure.
> >   */
> >  static int pci_host_common_parse_port(struct device *dev,
> >  				      struct pci_host_bridge *bridge, @@ -
> 128,22
> > +127,6 @@ static int pci_host_common_parse_port(struct device *dev,
> >  	if (ret)
> >  		return ret;
> >
> > -	/*
> > -	 * 1. PERST# found in RP or its child nodes - list is not empty,
> > -	 *    continue
> > -	 *
> > -	 * 2. PERST# not found in RP/children, but found in RC node -
> > -	 *    return -ENODEV to fallback legacy binding
> > -	 *
> > -	 * 3. PERST# not found anywhere - list is empty, continue (optional
> > -	 *    PERST#)
> > -	 */
> > -	if (list_empty(&port->perst)) {
> > -		if (of_property_present(dev->of_node, "reset-gpios") ||
> > -		    of_property_present(dev->of_node, "reset-gpio"))
> > -			return -ENODEV;
> > -	}
> > -
> >  	INIT_LIST_HEAD(&port->list);
> >  	list_add_tail(&port->list, &bridge->ports);
> >
> > @@ -158,13 +141,11 @@ static int pci_host_common_parse_port(struct
> > device *dev,
> >   * Iterate through child nodes of the host bridge and parse Root Port
> >   * properties (currently only reset GPIOs).
> >   *
> > - * Return: 0 on success, -ENODEV if no ports found or PERST# found in
> > RC
> > - * node (legacy binding should be used), Other negative error codes
> > on
> > - * failure.
> > + * Return: 0 on success, negative error codes on failure.
> >   */
> >  int pci_host_common_parse_ports(struct device *dev, struct
> > pci_host_bridge
> > *bridge)  {
> > -	int ret = -ENODEV;
> > +	int ret = 0;
> >
> >  	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
> >  		if (!of_node_is_type(of_port, "pci")) @@ -174,8 +155,8 @@
> int
> > pci_host_common_parse_ports(struct device *dev, struct pci_host_bridge
> *brid
> >  			goto err_cleanup;
> >  	}
> >
> > -	if (ret)
> > -		return ret;
> > +	if (list_empty(&bridge->ports))
> > +		return 0;
> >
> >  	return devm_add_action_or_reset(dev,
> pci_host_common_delete_ports,
> >  					&bridge->ports);
> > --
> > 2.37.1

[PATCH 2/2] PCI: imx6: Add imx_pcie_perst_found() to inspect the parsed result

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

Since pci_host_common_parse_port() doesn't return failure for "property
not found"(-ENODEV), the caller should inspect the parsed result and
decide whether to fall back to the legacy binding.
Add imx_pcie_perst_found() to inspect the parsed result.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index b137551871fc..34756f28fcc6 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1287,6 +1287,18 @@ static void imx_pcie_assert_perst(struct imx_pcie *imx_pcie, bool assert)
 	}
 }
 
+static bool imx_pcie_perst_found(struct pci_host_bridge *bridge)
+{
+	struct pci_host_port *port;
+
+	list_for_each_entry(port, &bridge->ports, list) {
+		if (!list_empty(&port->perst))
+			return true;
+	}
+
+	return false;
+}
+
 static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -1299,15 +1311,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 		/* Parse Root Port nodes if present */
 		ret = pci_host_common_parse_ports(dev, bridge);
 		if (ret) {
-			if (ret != -ENODEV) {
-				dev_err(dev, "Failed to parse Root Port nodes: %d\n", ret);
-				return ret;
-			}
+			dev_err(dev, "Failed to parse Root Port nodes: %d\n", ret);
+			return ret;
+		}
 
-			/*
-			 * Fall back to legacy binding for DT backwards
-			 * compatibility
-			 */
+		/* Fallback to legacy binding for DT backwards compatibility. */
+		if (!imx_pcie_perst_found(bridge)) {
 			ret = imx_pcie_parse_legacy_binding(imx_pcie);
 			if (ret)
 				return ret;
-- 
2.37.1

RE: [PATCH 2/2] PCI: imx6: Add imx_pcie_perst_found() to inspect the parsed result

[Hongxing Zhu]

> -----Original Message-----
> From: Sherry Sun (OSS) <sherry.sun@oss.nxp.com>
> Sent: Friday, May 22, 2026 11:44 AM
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de; Frank Li
> <frank.li@nxp.com>; bhelgaas@google.com; lpieralisi@kernel.org;
> kwilczynski@kernel.org; mani@kernel.org; robh@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> will@kernel.org
> Cc: imx@lists.linux.dev; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Sherry Sun
> <sherry.sun@nxp.com>
> Subject: [PATCH 2/2] PCI: imx6: Add imx_pcie_perst_found() to inspect the
> parsed result
> 
> From: Sherry Sun <sherry.sun@nxp.com>
> 
> Since pci_host_common_parse_port() doesn't return failure for "property not
> found"(-ENODEV), the caller should inspect the parsed result and decide whether
One space should be placed before (-ENODEV).

> to fall back to the legacy binding.
> Add imx_pcie_perst_found() to inspect the parsed result.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>

Best Regards
Richard Zhu

> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index b137551871fc..34756f28fcc6 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1287,6 +1287,18 @@ static void imx_pcie_assert_perst(struct imx_pcie
> *imx_pcie, bool assert)
>  	}
>  }
> 
> +static bool imx_pcie_perst_found(struct pci_host_bridge *bridge) {
> +	struct pci_host_port *port;
> +
> +	list_for_each_entry(port, &bridge->ports, list) {
> +		if (!list_empty(&port->perst))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  static int imx_pcie_host_init(struct dw_pcie_rp *pp)  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp); @@ -1299,15 +1311,12
> @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  		/* Parse Root Port nodes if present */
>  		ret = pci_host_common_parse_ports(dev, bridge);
>  		if (ret) {
> -			if (ret != -ENODEV) {
> -				dev_err(dev, "Failed to parse Root Port
> nodes: %d\n", ret);
> -				return ret;
> -			}
> +			dev_err(dev, "Failed to parse Root Port nodes: %d\n",
> ret);
> +			return ret;
> +		}
> 
> -			/*
> -			 * Fall back to legacy binding for DT backwards
> -			 * compatibility
> -			 */
> +		/* Fallback to legacy binding for DT backwards compatibility. */
> +		if (!imx_pcie_perst_found(bridge)) {
>  			ret = imx_pcie_parse_legacy_binding(imx_pcie);
>  			if (ret)
>  				return ret;
> --
> 2.37.1