Re: [PATCH v3 4/4] of: address: kill of_node_is_pcie()

[Alex Elder]

On 9/1/26 1:45 AM, Herve Codina wrote:
> Hi Alex,
>
> On Mon, 31 Aug 2026 20:13:37 -0500
> Alex Elder <elder@xxxxxxxxxxxx> wrote:
>
> > The of_bus->match function for the "PCI" bus type is fairly liberal
> > in what it accepts as a PCI bus devicetree node.  If a node has no
> > device_type property, it even allows a node named "pcie@" to be
> > accepted as represnting a devicetree bus, though it issues a warning
> > in that case.
> >
> > A recent PCI commit introduced of_pci_verify_node().  When a PCI
> > device is added, if it has a devicetree node, that function checks
> > its device_type property.  For PCI bridge devices, if there is no
> > device_type property (value "pci"), a warning is issued.
> >
> > That warning duplicates the warning made by of_node_is_pcie(), and
> > there's no point in that.  Avoid the second (OF) warning by just
> > checking the node name directly in of_bus_pci_match().
> >
> > That leaves of_node_is_pcie() unused, so get rid of it.
> >
> > Signed-off-by: Alex Elder <elder@xxxxxxxxxxxx>
> > ---
> > v3: - Added (new) in this version of the series
> >
> >   drivers/of/address.c | 12 +-----------
> >   1 file changed, 1 insertion(+), 11 deletions(-)
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 499d37ceae210..ee2eb44884d85 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -134,16 +134,6 @@ static unsigned int of_bus_pci_get_flags(const __be32 *addr)
> >    * PCI bus specific translator
> >    */
> >   
> > -static bool of_node_is_pcie(const struct device_node *np)
> > -{
> > -	bool is_pcie = of_node_name_eq(np, "pcie");
> > -
> > -	if (is_pcie)
> > -		pr_warn_once("%pOF: Missing device_type\n", np);
> > -
> > -	return is_pcie;
> > -}
> > -
> >   static int of_bus_pci_match(struct device_node *np)
> >   {
> >   	/*
> > @@ -156,7 +146,7 @@ static int of_bus_pci_match(struct device_node *np)
> >   	 */
> >   	return of_node_is_type(np, "pci") || of_node_is_type(np, "pciex") ||
> >   		of_node_is_type(np, "vci") || of_node_is_type(np, "ht") ||
> > -		of_node_is_pcie(np);
> > +		of_node_name_eq(np, "pcie");
> >   }
> >   
> >   static void of_bus_pci_count_cells(struct device_node *np,
>
> The warning here was printed based on the node name whereas of_node_is_pcie()
> prints the message based on the 'device_type' property of a pci_dev node.

You're right.  Both warnings were getting reported for me, but for
different reasons.

I would be happy to just drop this patch and live with two warnings
in some cases (we should rarely see either one of them anyway, right?).

Is that OK with you?  Does anyone feel this patch should be kept?

Thank you.

					-Alex

> For PCI to PCI bridges, no problem the warning is indeed duplicated but what
> happens for the PCI host controller?
>
> PCI host controller drivers calls pci_host_probe() and are seen by the PCI core
> as a struct pci_host_bridge.
>
> of_node_is_pcie() is called for children of the PCI host controller (i.e. PCI
> devices scanned on the PCI bus handled by the host controller) but not for the
> PCI host controller itself.
>
> The OF node of the host controller must have the 'device_type' property set
> to "pci".
>
> I am not so sure that this warning was duplicated when we consider the PCI
> host controller node.
>
> Can you double check on your side?
>
> Best regards,
> Hervé