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

[Frank Li]

On Wed, Mar 11, 2026 at 10:03:01AM +0000, Sherry Sun wrote:
> > 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)

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

Frank

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