Re: [PATCH v2 1/2] dt-bindings: PCI: eic7700: Add Eswin eic7700 PCIe host controller

[Krzysztof Kozlowski <krzk@kernel.org>]

On 18/09/2025 12:15, zhangsenchuan wrote:
>>> +  num-lanes:
>>> +    const: 4
>>
>> If that's const, you do not need it. It's implied by the compatible.
>> I see some other bindings do similarly and I think that's not the
>> correct choice.
>>
>> Well, maybe @Rob knows if PCI is different here anyhow?
> 
> Dear Krzysztof
> 
> Thank you very much for your review.
> You're right，If that's const,  I don't think it's necessary either.
> After investigation, the description of the "num-lanes" attribute here 
> is incorrect. The correct one should be the following description:
>     num-lanes:
>       maximum: 4

If 1, 2 or 4 lanes are correct, for this compatible, then yes.

> If so, the num-lanes attribute should need to be described here.
> What do you think?
> 


...

>>> +            reset-names = "cfg", "powerup", "pwren";
>>> +            interrupts = <220>, <179>, <180>, <181>, <182>, <183>, <184>, <185>, <186>;
>>> +            interrupt-names = "msi", "inta", "intb", "intc", "intd",
>>> +                              "inte", "intf", "intg", "inth";
>>> +            interrupt-parent = <&plic>;
>>> +            interrupt-map-mask = <0x0 0x0 0x0 0x7>;
>>> +            interrupt-map = <0x0 0x0 0x0 0x1 &plic 179>,
>>> +                            <0x0 0x0 0x0 0x2 &plic 180>,
>>> +                            <0x0 0x0 0x0 0x3 &plic 181>,
>>> +                            <0x0 0x0 0x0 0x4 &plic 182>;
>>> +            device_type = "pci";
>>> +            num-lanes = <0x4>;
>>
>> That's not a hex number, but decimal.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Manivannan and Bjorn have provided me with some excellent suggestions for my yaml. 
> My yaml will be refactored in the next patch, and I might need you to review it 
> again for me in the next patch. I'm a little wondering if I need to add
> "Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>" in next patch.


If you are changing it significantly, then drop/ignore my tag and write
in the changelog reasons why the tag was dropped. Usually adding new
properties is a significant change. Changing some clock name from A to B
is not a significant change. Other cases vary. More important is to
explain the differences and reason of dropping tag.


Best regards,
Krzysztof