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

[Alex Elder <elder@riscstar.com>]

On 8/31/26 8:21 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The check in of_pci_verify_node() falsely flags compliant CardBus and PowerPC PCIe bridges as having an invalid device_type.

I was unfortunately not considering those types of bridges,
and don't have experience with (or access to) either.

In the devicetree code, of_bus_pci_match() does check for
device_type "pciex" but does not handle "cardbus" (at least,
not explicitly--the "default-flags" bus type might do the
right thing).

I would love to get some pointers from others on how to
do this correctly, but in any case I'll do a little more
research on this before I post version 4.

					-Alex

> --
> 
> commit 72a980b9469901bec37d04c04dd8de642179aa44
> Author: Alex Elder <elder@riscstar.com>
> 
> PCI: of: introduce of_pci_verify_node()
> 
> This commit introduces a runtime check to verify PCI devicetree nodes. It
> ensures that bridge devices have the "pci" device_type property and that
> endpoint devices do not.
> 
>> 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
> [ ... ]
>> +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");
> 
> [Severity: Medium]
> Will this falsely flag compliant CardBus and PowerPC PCIe bridges as having an
> invalid device type?
> 
> The check in of_pci_verify_node() expects all bridges to have a device type of
> exactly "pci". However, valid PCI bridges can legitimately use "cardbus" or
> "pciex" as their device_type property.
> 
> When a PCI bridge with one of these valid types is added, the following call
> chain unconditionally triggers this error:
> 
> pci_bus_add_device()
>    of_pci_verify_node()
> 
> This could emit a false dev_err() during boot on platforms using these
> standard bridges, which might alarm users by polluting the kernel logs.
> 
>> +}
>