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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1ec41f26-8418-4f96-b7e2-9b851c926fa7@kernel.org>
Date: Fri, 19 Sep 2025 13:41:33 +0900
From: Krzysztof Kozlowski <krzk@...nel.org>
To: zhangsenchuan <zhangsenchuan@...incomputing.com>
Cc: bhelgaas@...gle.com, lpieralisi@...nel.org, kwilczynski@...nel.org,
 mani@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, p.zabel@...gutronix.de,
 johan+linaro@...nel.org, quic_schintav@...cinc.com, shradha.t@...sung.com,
 cassel@...nel.org, thippeswamy.havalige@....com,
 mayank.rana@....qualcomm.com, inochiama@...il.com,
 ningyu@...incomputing.com, linmin@...incomputing.com,
 pinkesh.vaghela@...fochips.com
Subject: Re: [PATCH v2 1/2] dt-bindings: PCI: eic7700: Add Eswin eic7700 PCIe
 host controller

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