Re: [PATCH v5 10/14] dt-bindings: PCI: Add CIX Sky1 PCIe Root Complex bindings

[Krzysztof Kozlowski <krzk@kernel.org>]

On 30/06/2025 10:29, Hans Zhang wrote:
>>> +
>>> +  num-lanes:
>>> +    maximum: 8
>>> +
>>> +  ranges:
>>> +    maxItems: 3
>>> +
>>> +  msi-map:
>>> +    maxItems: 1
>>> +
>>> +  vendor-id:
>>> +    const: 0x1f6c
>>
>> Why? This is implied by compatible.
> 
> Because when we designed the SOC RTL, it was not set to the vendor id 
> and device id of our company. We are members of PCI-SIG. So we need to 
> set the vendor id and device id in the Root Port driver. Otherwise, the 
> output of lspci will be displayed incorrectly.

Please read carefully. Previous discussions were also pointlessly
ping-ponging on irrelevant arguments. Did I suggest you do not have to
set it in root port driver? No. If this is const here, this is implied
by compatible and completely redundant, because your driver knows this
value already. It already has all the information to deduce this value
from the compatible.




> 
>>
>>> +
>>> +  device-id:
>>> +    enum:
>>> +      - 0x0001
>>
>> Why? This is implied by compatible.
> 
> The reason is the same as above.
> 
>>
>>> +
>>> +  cdns,no-inbound-bar:
>>
>> That's not a cdns binding, so wrong prefix.
> 
> It will be added to Cadence's Doc. I will add a separate patch. What do 
> you think?
> 
>>
>>> +    description: |
>>
>> Do not need '|' unless you need to preserve formatting.
> 
> Will delete '|'.
> 
>>
>>> +      Indicates the PCIe controller does not require an inbound BAR region.
>>
>> And anyway this is implied by compatible, drop.
>>
> 
> Because Cadence core driver has this judgment, the latest code of the 
> current linux master all has this process. As follows:
> int cdns_pcie_host_init(struct cdns_pcie_rc *rc)
>      cdns_pcie_host_init_address_translation(rc);
> 	cdns_pcie_host_map_dma_ranges(rc);
> 	   cdns_pcie_host_bar_ib_config

And you cannot fix or change drivers? How does it matter for discussion
here?

> 
> So this attribute has been added here, or is there a better way?

Of course, like every other driver in Linux kernel. This is FIXED for
your platform, so set it in your CIX driver.



Best regards,
Krzysztof