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

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, May 18, 2026 at 08:42:38AM +0000, Sherry Sun wrote:
> > Subject: Re: [PATCH V14 02/12] PCI: host-generic: Add common helpers for
> > parsing Root Port properties
> > 
> > On Wed, Apr 22, 2026 at 05:35:39PM +0800, Sherry Sun wrote:
> > > Introduce generic helper functions to parse Root Port device tree
> > > nodes and extract common properties like reset GPIOs. This allows
> > > multiple PCI host controller drivers to share the same parsing logic.
> > >
> > > Define struct pci_host_port to hold common Root Port properties
> > > (currently only list of PERST# GPIO descriptors) and add
> > > pci_host_common_parse_ports() to parse Root Port nodes from device
> > tree.
> > >
> > > Also add the 'ports' list to struct pci_host_bridge for better
> > > maintain parsed Root Port information.
> > > ...
> > 
> > > +static int pci_host_common_parse_port(struct device *dev,
> > > +				      struct pci_host_bridge *bridge,
> > > +				      struct device_node *node)
> > > +{
> > > +	struct pci_host_port *port;
> > > +	int ret;
> > > +
> > > +	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> > > +	if (!port)
> > > +		return -ENOMEM;
> > > +
> > > +	INIT_LIST_HEAD(&port->perst);
> > > +
> > > +	ret = pci_host_common_parse_perst(dev, port, node);
> > > +	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;
> > 
> > This doesn't seem right to me.  The parser of per-Root Port properties should
> > not be responsible for deciding whether legacy methods are valid, i.e.,
> > whether a property is in the Root Complex node.  I think it's up to the caller
> > to decide whether it needs to look elsewhere.
> > 
> > I don't think this even needs to return a "success/failure" value because there
> > may be more properties in the future, and not all will be required.  This
> > function can't tell which properties a specific driver requires and which are
> > optional.
> > 
> > The caller can check whether we found what it needs and fall back to a legacy
> > method as needed.
> 
> Hi Bjorn,
> The code here was suggested by Mani, https://lore.kernel.org/all/lnzprzrdwra7pn7d6m3sbj5pvjy64blwpjl6i3lmlnfbyho63b@czpyhpgz5vum/.
> I think your suggestion here is reasonable, the per-Root Port parser shouldn't
> check the RC-level binding. That's a policy decision that belongs to the caller.
> 
> Hi Mani, if you also agree, I'll rework this so that:
> 1. pci_host_common_parse_port() only parses properties from the Root Port
>     (and its children) without checking the RC node.
> 2. The function won't return failure for "property not found" - it will only return
>      errors for real failures (e.g., -ENOMEM, GPIO acquisition errors).
> 3. The legacy fallback logic will be moved to the caller, which can inspect the
>      parsed result and decide whether to fall back to the legacy binding.

This is only used for imx6 so far, so I think this is OK as-is for
v7.2.  We can file this under "possible future rework or kernel
mentee project."