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

[Hans Zhang <hans.zhang@cixtech.com>]

On 2025/7/3 04:23, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL
> 
> 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?
> 

Dear Krzysztof,

Thank you very much for your reply.

No, no, no.  You misunderstood me.  I didn't mean to say this because we 
don't study dt-binding doc every day.  So we can only refer to the 
practices of other SOC manufacturers.  If it's incorrect, we will 
definitely listen to your opinion.  Here, I'm just explaining the origin 
of what I did.

Anyway, I have solved this problem by following your method and using 
compatible.

>>
>>
>> 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.
> 

According to my understanding, this is not ABI.

Best regards,
Hans

> 
> Best regards,
> Krzysztof