Re: [PATCH v2 4/4] PCI: of: introduce of_pci_verify_node()

[Herve Codina <herve.codina@bootlin.com>]

Hi Alex,

On Wed, 12 Aug 2026 12:22:47 -0500
Alex Elder <elder@riscstar.com> wrote:

> Commit 407d1a51921e9 ("PCI: Create device tree node for bridge") linked
> the PCI enumeration process together with devicetree, creating a devicetree
> node for discovered PCI bridges.  Its successor commit ae9813db1dc5a ("PCI:
> Add quirks to generate device tree node for Xilinx Alveo U50") shows how
> to use a PCI final fixup quirk to also create a devicetree node for a
> non-bridge PCI device.  These changes allowed devicetree overlays to
> describe components downstream of a PCI device, by providing a place to
> attach the overlay.
> 
> Note that the dynamic devicetree node is only created if the device didn't
> already have an assigned node.
> 
> Later, commit aa7b4bbcb3a1d ("arm64: dts: qcom: qcs6490-rb3gen2: Add
> TC9563 PCIe switch node") *pre-defined* devicetree nodes to represent the
> PCI device nodes that would (also) be discovered via the PCI enumeration
> process.  The devicetree node in this case is created with the content
> from the DTS file.  So when a (host) bridge is done being initialized
> during PCI enumeration, no node is dynamically created (the commits
> mentioned above do not apply).
> 
> Ideally, any pre-defined PCI devicetree node would contain exactly the
> same information as whatever the dynamic creation process would produce
> (though it could include more).
> 
> However that is not the case for the pre-defined Qualcomm RB3gen2 nodes.
> And in particular, the endpoint (function) nodes include this property:
> 
>     device_type = "pci";
> 
> This is simply wrong; that property is meant only for bridge nodes.
> 
> Rob Herring requested that a runtime check to be added to spot this
> specific error, only for non-bridge PCI devices.
> 
> (There are many things that could be verified for statically-defined
> devicetree nodes, but this is all we'll do for now.)
> 
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
> v2: Verify even if PCI_DYNAMIC_OF_NODES is not defined (Sashiko)
> 
>  drivers/pci/bus.c |  1 +
>  drivers/pci/of.c  | 26 ++++++++++++++++++++++++++
>  drivers/pci/pci.h |  3 +++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 655ed53436d3e..679afbc6d3109 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -351,6 +351,7 @@ void pci_bus_add_device(struct pci_dev *dev)
>  	 * are not assigned yet for some devices.
>  	 */
>  	pcibios_bus_add_device(dev);
> +	of_pci_verify_node(dev);
>  	pci_fixup_device(pci_fixup_final, dev);
>  	if (pci_is_bridge(dev))
>  		of_pci_make_dev_node(dev);
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 8b18c4ba845cf..0bbf1a915b7d2 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -1006,3 +1006,29 @@ int of_pci_get_equalization_presets(struct device *dev,
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
> +
> +/*
> + * Check a pre-existing devicetree node for errors.  The PCI enumeration
> + * process gathered a lot of information about the device, and ideally it
> + * matches what the devicetree node says.
> + */

Can you switch the kernel doc format [0] or move the comment into the function itself ?

[0] https://elixir.bootlin.com/linux/v7.2/source/Documentation/doc-guide/kernel-doc.rst#L67

> +void of_pci_verify_node(struct pci_dev *pdev)
> +{
> +	struct device_node *np = pci_device_to_OF_node(pdev);
> +
> +	/* If there's no pre-existing node, there's nothing to check */
> +	if (!np)
> +		return;
> +
> +	if (pci_is_bridge(pdev))
> +		return;

The property 'device_type = "pci"' is needed for bridges.

You could check that it is well present in case of bridges.

> +
> +	/*
> +	 * Currently we just verify that non-bridges don't contain a
> +	 * device_type = "pci" property.
> +	 */
> +	if (!of_node_is_type(np, "pci"))
> +		return;
> +
> +	dev_err(&pdev->dev, "\"pci\" device_type NOT VALID for PCI endpoint\n");
> +}

Best regards,
Hervé