* [PATCH v3 0/4] PCI: of: warn on bogus device_type property
@ 2026-09-01 1:13 Alex Elder
2026-09-01 1:13 ` [PATCH v3 1/4] PCI: of: drop the reg_num argument to of_pci_set_address() Alex Elder
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Alex Elder @ 2026-09-01 1:13 UTC (permalink / raw)
To: bhelgaas, robh, saravanak
Cc: herve.codina, daniel, mohd.anwar, lorenzo.bianconi, linux-pci,
devicetree, linux-kernel
Add a check when adding a PCI device to ensure the device_type
property is (or is not) correctly defined when the device has
a non-null devicetree node pointer.
PCI has a well-defined bus and device discovery process. The
PCI_DYNAMIC_OF_NODES Kconfig option allows PCI devices to *also*
have a devicetree node. This enables certain things that are
not possible with PCI enumeration alone.
While working on a Qualcomm platform, I learned that some PCI
endpoint nodes were defined with device_type = "pci" properties.
Herve Codina pointed out that this was not correct. Rob Herring
indicated that people seem to have trouble getting the PCI
devicetree nodes right, and asked whether we could warn if this
particular problem occurred. During review, Herve Codina also
suggested that bridge nodes be checked to ensure they *do*
have the proper device_type property.
This series implements these checks. A new patch in this version
removes a (now duplicate) warning issued by the devicetree code.
The first two patches are simple cleanups. The third adds the new
PCI devicetree node checks and warnings, and the last removes the
duplicate warning message.
-Alex
Between version 2 and version 3:
- Drop a patch that made a change only needed by a different series
- Switch a function header to use kernel-doc format
- Add a warning if a PCI bridge node has no device_type property
- Added a patch to remove a duplicate warning in the devicetree code
Version 2 is available here:
https://lore.kernel.org/lkml/20260812172247.276554-1-elder@riscstar.com/
Between version 1 and version 2:
- Check the PCI devicetree node even when PCI_DYNAMIC_OF_NODES is
not enabled
Version 1 is available here:
https://lore.kernel.org/lkml/20260807194100.455599-1-elder@riscstar.com/
Alex Elder (4):
PCI: of: drop the reg_num argument to of_pci_set_address()
PCI: of: don't zero flags in of_pci_get_addr_flags()
PCI: of: introduce of_pci_verify_node()
of: address: kill of_node_is_pcie()
drivers/of/address.c | 12 +-----------
drivers/pci/bus.c | 1 +
drivers/pci/of.c | 31 +++++++++++++++++++++++++++++++
drivers/pci/of_property.c | 14 ++++++--------
drivers/pci/pci.h | 3 +++
5 files changed, 42 insertions(+), 19 deletions(-)
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/4] PCI: of: drop the reg_num argument to of_pci_set_address()
2026-09-01 1:13 [PATCH v3 0/4] PCI: of: warn on bogus device_type property Alex Elder
@ 2026-09-01 1:13 ` Alex Elder
2026-09-01 1:13 ` [PATCH v3 2/4] PCI: of: don't zero flags in of_pci_get_addr_flags() Alex Elder
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Alex Elder @ 2026-09-01 1:13 UTC (permalink / raw)
To: bhelgaas, robh, saravanak
Cc: herve.codina, daniel, mohd.anwar, lorenzo.bianconi, linux-pci,
devicetree, linux-kernel
The reg_num argument passed to of_pci_set_address() is always zero,
so get rid of it.
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
drivers/pci/of_property.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index 75a358f73e694..505226b876c56 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -52,7 +52,7 @@ enum of_pci_prop_compatible {
};
static void of_pci_set_address(struct pci_dev *pdev, u32 *prop, u64 addr,
- u32 reg_num, u32 flags, bool reloc)
+ u32 flags, bool reloc)
{
if (pdev) {
prop[0] = FIELD_PREP(OF_PCI_ADDR_FIELD_BUS, pdev->bus->number) |
@@ -61,7 +61,7 @@ static void of_pci_set_address(struct pci_dev *pdev, u32 *prop, u64 addr,
} else
prop[0] = 0;
- prop[0] |= flags | reg_num;
+ prop[0] |= flags;
if (!reloc) {
prop[0] |= OF_PCI_ADDR_FIELD_NONRELOC;
prop[1] = upper_32_bits(addr);
@@ -131,7 +131,7 @@ static int of_pci_prop_ranges(struct pci_dev *pdev, struct of_changeset *ocs,
continue;
val64 = pci_bus_address(pdev, &res[j] - pdev->resource);
- of_pci_set_address(pdev, rp[i].parent_addr, val64, 0, flags,
+ of_pci_set_address(pdev, rp[i].parent_addr, val64, flags,
false);
if (pci_is_bridge(pdev)) {
memcpy(rp[i].child_addr, rp[i].parent_addr,
@@ -164,7 +164,7 @@ static int of_pci_prop_reg(struct pci_dev *pdev, struct of_changeset *ocs,
struct of_pci_addr_pair reg = { 0 };
/* configuration space */
- of_pci_set_address(pdev, reg.phys_addr, 0, 0, 0, true);
+ of_pci_set_address(pdev, reg.phys_addr, 0, 0, true);
return of_changeset_add_prop_u32_array(ocs, np, "reg", (u32 *)®,
sizeof(reg) / sizeof(u32));
@@ -458,7 +458,7 @@ static int of_pci_host_bridge_prop_ranges(struct pci_host_bridge *bridge,
/* PCI bus address */
val64 = res->start;
of_pci_set_address(NULL, &ranges[ranges_sz],
- val64 - window->offset, 0, flags, false);
+ val64 - window->offset, flags, false);
ranges_sz += OF_PCI_ADDRESS_CELLS;
/* Host bus address */
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/4] PCI: of: don't zero flags in of_pci_get_addr_flags()
2026-09-01 1:13 [PATCH v3 0/4] PCI: of: warn on bogus device_type property Alex Elder
2026-09-01 1:13 ` [PATCH v3 1/4] PCI: of: drop the reg_num argument to of_pci_set_address() Alex Elder
@ 2026-09-01 1:13 ` Alex Elder
2026-09-01 1:13 ` [PATCH v3 3/4] PCI: of: introduce of_pci_verify_node() Alex Elder
2026-09-01 1:13 ` [PATCH v3 4/4] of: address: kill of_node_is_pcie() Alex Elder
3 siblings, 0 replies; 10+ messages in thread
From: Alex Elder @ 2026-09-01 1:13 UTC (permalink / raw)
To: bhelgaas, robh, saravanak
Cc: herve.codina, daniel, mohd.anwar, lorenzo.bianconi, linux-pci,
devicetree, linux-kernel
The flags variable whose address is passed to of_pci_get_addr_flags()
is zeroed before assigning a value to it. Skip the zeroing and just
assign it instead.
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Signed-off-by: Alex Elder <elder@riscstar.com>
---
drivers/pci/of_property.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
index 505226b876c56..22fea1905a080 100644
--- a/drivers/pci/of_property.c
+++ b/drivers/pci/of_property.c
@@ -82,12 +82,10 @@ static int of_pci_get_addr_flags(const struct resource *res, u32 *flags)
else
return -EINVAL;
- *flags = 0;
+ *flags = FIELD_PREP(OF_PCI_ADDR_FIELD_SS, ss);
if (res->flags & IORESOURCE_PREFETCH)
*flags |= OF_PCI_ADDR_FIELD_PREFETCH;
- *flags |= FIELD_PREP(OF_PCI_ADDR_FIELD_SS, ss);
-
return 0;
}
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/4] PCI: of: introduce of_pci_verify_node()
2026-09-01 1:13 [PATCH v3 0/4] PCI: of: warn on bogus device_type property Alex Elder
2026-09-01 1:13 ` [PATCH v3 1/4] PCI: of: drop the reg_num argument to of_pci_set_address() Alex Elder
2026-09-01 1:13 ` [PATCH v3 2/4] PCI: of: don't zero flags in of_pci_get_addr_flags() Alex Elder
@ 2026-09-01 1:13 ` Alex Elder
2026-09-01 6:11 ` Herve Codina
2026-09-01 1:13 ` [PATCH v3 4/4] of: address: kill of_node_is_pcie() Alex Elder
3 siblings, 1 reply; 10+ messages in thread
From: Alex Elder @ 2026-09-01 1:13 UTC (permalink / raw)
To: bhelgaas, robh, saravanak
Cc: herve.codina, daniel, mohd.anwar, lorenzo.bianconi, linux-pci,
devicetree, linux-kernel
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. Herve Codina further
suggested we ensure that bridge PCI devices *do* define the device_type
property, with value "pci".
Signed-off-by: Alex Elder <elder@riscstar.com>
---
v3: - Switch a function header to use kernel-doc format
- Add a warning if a PCI bridge node has no device_type property
drivers/pci/bus.c | 1 +
drivers/pci/of.c | 31 +++++++++++++++++++++++++++++++
drivers/pci/pci.h | 3 +++
3 files changed, 35 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 a51dff91b196d..1838a3109514c 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -1085,3 +1085,34 @@ int of_pci_get_equalization_presets(struct device *dev,
return 0;
}
EXPORT_SYMBOL_GPL(of_pci_get_equalization_presets);
+
+/**
+ * of_pci_verify_node - Sanity check some PCI device node properties
+ * @pdev: The PCI device whose device node is checked
+ *
+ * PCI enumeration authoritatively discovers what we need to know about
+ * a PCI device. A devicetree-based platform will represent a PCI root
+ * bridge with a node, but otherwise devicetree doesn't typically include
+ * many PCI nodes. Where such nodes do exist, experience has shown that
+ * the "device_type" property is sometimes wrong, so warn about that.
+ */
+void of_pci_verify_node(struct pci_dev *pdev)
+{
+ struct device_node *np = pci_device_to_OF_node(pdev);
+ bool device_is_bridge;
+ bool device_type_pci;
+
+ /* Nothing to check if there's no pre-existing devicetree node */
+ if (!np)
+ return;
+
+ device_is_bridge = pci_is_bridge(pdev);
+ device_type_pci = of_node_is_type(np, "pci");
+
+ /* Bridges should have device type "pci"; endpoints should not */
+ if (device_is_bridge == device_type_pci)
+ return;
+
+ dev_err(&pdev->dev, "PCI %s have \"pci\" device_type property\n",
+ device_is_bridge ? "bridge should" : "endpoint should not");
+}
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index ba3c3fddddc23..2e33d3bd4b0ba 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1253,6 +1253,7 @@ bool of_pci_supply_present(struct device_node *np);
int of_pci_get_equalization_presets(struct device *dev,
struct pci_eq_presets *presets,
int num_lanes);
+void of_pci_verify_node(struct pci_dev *pdev);
#else
static inline int
of_get_pci_domain_nr(struct device_node *node)
@@ -1308,6 +1309,8 @@ static inline int of_pci_get_equalization_presets(struct device *dev,
return 0;
}
+
+static inline void of_pci_verify_node(struct pci_dev *pdev) { }
#endif /* CONFIG_OF */
struct of_changeset;
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 4/4] of: address: kill of_node_is_pcie()
2026-09-01 1:13 [PATCH v3 0/4] PCI: of: warn on bogus device_type property Alex Elder
` (2 preceding siblings ...)
2026-09-01 1:13 ` [PATCH v3 3/4] PCI: of: introduce of_pci_verify_node() Alex Elder
@ 2026-09-01 1:13 ` Alex Elder
2026-09-01 6:45 ` Herve Codina
3 siblings, 1 reply; 10+ messages in thread
From: Alex Elder @ 2026-09-01 1:13 UTC (permalink / raw)
To: bhelgaas, robh, saravanak
Cc: herve.codina, daniel, mohd.anwar, lorenzo.bianconi, linux-pci,
devicetree, linux-kernel
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@riscstar.com>
---
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,
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/4] PCI: of: introduce of_pci_verify_node()
2026-09-01 1:13 ` [PATCH v3 3/4] PCI: of: introduce of_pci_verify_node() Alex Elder
@ 2026-09-01 6:11 ` Herve Codina
0 siblings, 0 replies; 10+ messages in thread
From: Herve Codina @ 2026-09-01 6:11 UTC (permalink / raw)
To: Alex Elder
Cc: bhelgaas, robh, saravanak, daniel, mohd.anwar, lorenzo.bianconi,
linux-pci, devicetree, linux-kernel
Hi Alex,
On Mon, 31 Aug 2026 20:13:36 -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. Herve Codina further
> suggested we ensure that bridge PCI devices *do* define the device_type
> property, with value "pci".
>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
> v3: - Switch a function header to use kernel-doc format
> - Add a warning if a PCI bridge node has no device_type property
>
> drivers/pci/bus.c | 1 +
> drivers/pci/of.c | 31 +++++++++++++++++++++++++++++++
> drivers/pci/pci.h | 3 +++
> 3 files changed, 35 insertions(+)
>
LGTM.
Reviewed-by: Herve Codina <herve.codina@bootlin.com>
Best regards,
Hervé
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 4/4] of: address: kill of_node_is_pcie()
2026-09-01 1:13 ` [PATCH v3 4/4] of: address: kill of_node_is_pcie() Alex Elder
@ 2026-09-01 6:45 ` Herve Codina
2026-09-01 14:58 ` Alex Elder
0 siblings, 1 reply; 10+ messages in thread
From: Herve Codina @ 2026-09-01 6:45 UTC (permalink / raw)
To: Alex Elder
Cc: bhelgaas, robh, saravanak, daniel, mohd.anwar, lorenzo.bianconi,
linux-pci, devicetree, linux-kernel
Hi Alex,
On Mon, 31 Aug 2026 20:13:37 -0500
Alex Elder <elder@riscstar.com> 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@riscstar.com>
> ---
> 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.
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é
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 4/4] of: address: kill of_node_is_pcie()
2026-09-01 6:45 ` Herve Codina
@ 2026-09-01 14:58 ` Alex Elder
2026-09-01 15:15 ` Herve Codina
0 siblings, 1 reply; 10+ messages in thread
From: Alex Elder @ 2026-09-01 14:58 UTC (permalink / raw)
To: Herve Codina
Cc: bhelgaas, robh, saravanak, daniel, mohd.anwar, lorenzo.bianconi,
linux-pci, devicetree, linux-kernel
On 9/1/26 1:45 AM, Herve Codina wrote:
> Hi Alex,
>
> On Mon, 31 Aug 2026 20:13:37 -0500
> Alex Elder <elder@riscstar.com> 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@riscstar.com>
>> ---
>> 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é
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 4/4] of: address: kill of_node_is_pcie()
2026-09-01 14:58 ` Alex Elder
@ 2026-09-01 15:15 ` Herve Codina
2026-09-01 15:25 ` Alex Elder
0 siblings, 1 reply; 10+ messages in thread
From: Herve Codina @ 2026-09-01 15:15 UTC (permalink / raw)
To: Alex Elder
Cc: bhelgaas, robh, saravanak, daniel, mohd.anwar, lorenzo.bianconi,
linux-pci, devicetree, linux-kernel
Hi Alex,
On Tue, 1 Sep 2026 09:58:10 -0500
Alex Elder <elder@riscstar.com> wrote:
> On 9/1/26 1:45 AM, Herve Codina wrote:
> > Hi Alex,
> >
> > On Mon, 31 Aug 2026 20:13:37 -0500
> > Alex Elder <elder@riscstar.com> 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@riscstar.com>
> >> ---
> >> 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?
On my side, I have no problem to see two warnings and so, IHMO, this patch
can be dropped.
Best regards,
Hervé
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 4/4] of: address: kill of_node_is_pcie()
2026-09-01 15:15 ` Herve Codina
@ 2026-09-01 15:25 ` Alex Elder
0 siblings, 0 replies; 10+ messages in thread
From: Alex Elder @ 2026-09-01 15:25 UTC (permalink / raw)
To: Herve Codina
Cc: bhelgaas, robh, saravanak, daniel, mohd.anwar, lorenzo.bianconi,
linux-pci, devicetree, linux-kernel
On 9/1/26 10:15 AM, Herve Codina wrote:
>>> 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?
> On my side, I have no problem to see two warnings and so, IHMO, this patch
> can be dropped.
Unless I hear otherwise, that's what I will do, for v4, which
I'll send out sometime this week after addressing concerns
pointed out by Sashiko.
Thank you.
-Alex
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-01 15:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-01 1:13 [PATCH v3 0/4] PCI: of: warn on bogus device_type property Alex Elder
2026-09-01 1:13 ` [PATCH v3 1/4] PCI: of: drop the reg_num argument to of_pci_set_address() Alex Elder
2026-09-01 1:13 ` [PATCH v3 2/4] PCI: of: don't zero flags in of_pci_get_addr_flags() Alex Elder
2026-09-01 1:13 ` [PATCH v3 3/4] PCI: of: introduce of_pci_verify_node() Alex Elder
2026-09-01 6:11 ` Herve Codina
2026-09-01 1:13 ` [PATCH v3 4/4] of: address: kill of_node_is_pcie() Alex Elder
2026-09-01 6:45 ` Herve Codina
2026-09-01 14:58 ` Alex Elder
2026-09-01 15:15 ` Herve Codina
2026-09-01 15:25 ` Alex Elder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®