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

[Krzysztof Kozlowski]

On 30/06/2025 17:30, Hans Zhang wrote:
> 
> 
> On 2025/6/30 19:14, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL
>>
>> 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.
>>
>>
> Dear Krzysztof,
> 
> Thank you very much for your reply.
> 
> These two attributes are also in the following document. Is this place 
> out of date?
> Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml

I would need to spend time to investigate that and I choose to do other
things instead. I am recently very grumpy on arguments "I found this
somewhere else". I found bugs somewhere else, so am I okay to introduce
them?

> 
> 
> We initially used the logic of Cadence common driver as follows:
> drivers/pci/controller/cadence/pcie-cadence-host.c
> of_property_read_u32(np, "vendor-id", &rc->vendor_id);
> 
> of_property_read_u32(np, "device-id", &rc->device_id);
> 
> So, can the code in Cadence be deleted?

Don't know. If this is ABI, then not.


Best regards,
Krzysztof