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

[Sherry Sun]

> From: Frank Li (AI-BOT) <frank.li@xxxxxxx>
> 
> > +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.
> 

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.

1. -ENOENT semantically means  "the expected entity was not found", which accurately
    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
     where ports are optional, they can explicitly check for -ENOENT and treat it as non-fatal.
   - Actual parsing errors (other negative error codes like -ENOMEM, -EPROBE_DEFER)

> > 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