Subject: Re: [PATCH V7 02/13] PCI: host-generic: Add common helpers for
parsing Root Port properties

On Wed, Mar 11, 2026 at 10:03:01AM +0000, Sherry Sun wrote:

quoted

quoted

From: Frank Li (AI-BOT) <frank.li@nxp.com>

quoted

+int pci_host_common_parse_ports(struct device *dev, struct
+pci_host_bridge *bridge) {
+	int ret = -ENOENT;
+
+	for_each_available_child_of_node_scoped(dev->of_node, of_port) {
+		if (!of_node_is_type(of_port, "pci"))
+			continue;
+		ret = pci_host_common_parse_port(dev, bridge, of_port);
+		if (ret)
+			return ret;
+	}
+
+	return ret;
+}

AI: Returning -ENOENT when no ports are found may be unexpected by
callers.
Consider whether this should return 0 (success, zero ports parsed)
or if callers must explicitly handle -ENOENT. Document the semantics

clearly.

quoted

quoted

Returning -ENOENT when no ports are found is intentional and aligns
with the design requirements for this use case. This pattern is common
in the kernel where parsing functions return -ENOENT when expected

nodes/properties are missing.

quoted

1. -ENOENT semantically means  "the expected entity was not found", which

accurately

quoted

    reflects the situation when no Root Port nodes exist in the device tree.
2. The callers of this function need to distinguish between:
   - Success with ports found (return 0)
   - No ports found in DT (-ENOENT) - this may be a DT configuration issue,

for controllers

quoted

     where ports are optional, they can explicitly check for -ENOENT and treat

it as non-fatal.

quoted

   - Actual parsing errors (other negative error codes like -ENOMEM,
-EPROBE_DEFER)

I think no ports under bridge should treat as 0. otherwise it may broken other
platform, which dt have not added child node yet.

Hi Frank, no, it will not break the old platforms, the caller will explicitly check for
-ENOENT and treat it as non-fatal for such optional properties. You can check my patch#3.
This is a common behavior in linux, for example, we also do the same check when
calling devm_regulator_get_optional() or devm_pm_opp_of_add_table().

@@ -581,6 +582,13 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)

        pp->bridge = bridge;

+       /* Parse Root Port nodes if present */
+       ret = pci_host_common_parse_ports(dev, bridge);
+       if (ret && ret != -ENOENT) {
+               dev_err(dev, "Failed to parse Root Port nodes: %d\n", ret);
+               return ret;
+       }

Best Regards
Sherry

Frank

quoted

quoted

quoted

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index

bccc7a4bdd79..9cc3f4ebb370 100644

--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c

@@ -647,6 +647,7 @@ static void pci_release_host_bridge_dev(struct

device *dev)

 	pci_free_resource_list(&bridge->windows);
 	pci_free_resource_list(&bridge->dma_ranges);
+	pci_free_resource_list(&bridge->ports);

AI: pci_free_resource_list() expects struct resource_entry nodes.
The ports list contains struct pci_host_port nodes. This will
corrupt memory or crash. Use
list_for_each_entry_safe() with proper cleanup, or define a
dedicated
pci_free_host_ports() function.

Yes, thanks for catching this, pci_free_resource_list() expects struct
resource_entry nodes and cannot be used for struct pci_host_port nodes.
I will add back the cleanup function pci_host_common_delete_ports() in
earlier version patchset to properly handles the ports list.

Best regards,
Sherry