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

[Alex Elder <elder@riscstar.com>]

On 9/24/26 10:09 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] pci: of: unintended error-level logging in of_pci_verify_node()
> --
> 
> commit 0e8b20efbbe63f895449027a5712b691b9eeaa56
> Author: Alex Elder <elder@riscstar.com>
> 
> PCI: of: introduce of_pci_verify_node()
> 
> This commit introduces of_pci_verify_node() to verify the device_type property
> for PCI devices. It adds a runtime check during device enumeration to warn if a
> bridge lacks a "pci" or "pciex" device type or if an endpoint improperly
> includes one.
> 
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index a51dff91b196d..5a040ed836744 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
> 
> [ ... ]
> 
>> +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") ||
>> +			  of_node_is_type(np, "pciex");
>> +
>> +	/* 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]
> The commit message states that these warnings are "meant to help developers;
> users should never see them."

I suppose I should have said "error messages" rather than "warnings"
here.  I think dev_err() is appropriate, and unless a human objects
to this, I do not plan to update the series to address this.

					-Alex


> However, dev_err() logs unconditionally at the error level in
> of_pci_verify_node(), making it prominently visible to end users and
> potentially failing automated test suites on boards with legacy devicetree
> nodes.
> 
> Should dev_dbg() or dev_info() be used here instead to match the stated
> developer-only intent?
> 
>> +}
>