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

[sashiko-bot@kernel.org]

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