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

On 8/7/26 2:49 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The static DT node verification function `of_pci_verify_node()` is mistakenly placed under `#ifdef CONFIG_PCI_DYNAMIC_OF_NODES`, making it a silent no-op on systems that only use statically-defined nodes.

This is a good observation.  I think it's correct that
this check is worth doing on static DT nodes even if
CONFIG_PCI_DYNAMIC_OF_NODES is not enabled.

I will plan to send v2 of this series with this
suggestion implemented.

					-Alex
> --
> 
> --- Patch [4]: [PATCH 4/4] PCI: of: introduce of_pci_verify_node() ---
> commit 29170ab92dd207c740eed74438cd2e09bad49131
> Author: Alex Elder <[email protected]>
> 
> PCI: of: introduce of_pci_verify_node()
> 
> This commit introduces a runtime check to verify that statically-defined
> (pre-existing) devicetree nodes for non-bridge PCI endpoints do not
> incorrectly contain the device_type = "pci" property. It adds a call to
> of_pci_verify_node() during PCI bus device addition to catch these errors.
> 
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -1265,6 +1265,7 @@ static inline int of_pci_get_equalization_presets(struct device *dev,
>>   struct of_changeset;
>>   
>>   #ifdef CONFIG_PCI_DYNAMIC_OF_NODES
>> +void of_pci_verify_node(struct pci_dev *pdev);
> 
> [Severity: Medium]
> Since the intent is to verify statically-defined devicetree nodes, will
> placing this declaration inside the CONFIG_PCI_DYNAMIC_OF_NODES block
> accidentally disable the check on platforms that only use static nodes?
> 
> If CONFIG_PCI_DYNAMIC_OF_NODES is disabled, this falls back to the empty
> stub below, causing the verification to be silently skipped.
> 
>>   void of_pci_make_dev_node(struct pci_dev *pdev);
>>   void of_pci_remove_node(struct pci_dev *pdev);
> 
> [ ... ]
> 
>>   #else
>> +static inline void of_pci_verify_node(struct pci_dev *pdev) { }
>>   static inline void of_pci_make_dev_node(struct pci_dev *pdev) { }
>