linux-kernel - Re: [PATCH v4 3/6] dt-bindings: PCI: qcom: Add IPQ5018 SoC

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] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <DS7PR19MB88839F5961CB1C0AAC1902959DD22@DS7PR19MB8883.namprd19.prod.outlook.com>
Date: Fri, 14 Mar 2025 12:42:41 +0400
From: George Moussalem <george.moussalem@...look.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-pci@...r.kernel.org, linux-phy@...ts.infradead.org,
 andersson@...nel.org, bhelgaas@...gle.com, conor+dt@...nel.org,
 devicetree@...r.kernel.org, lumag@...nel.org, kishon@...nel.org,
 konradybcio@...nel.org, krzk+dt@...nel.org, kw@...ux.com,
 lpieralisi@...nel.org, manivannan.sadhasivam@...aro.org,
 p.zabel@...gutronix.de, quic_nsekar@...cinc.com, robh@...nel.org,
 robimarko@...il.com, vkoul@...nel.org, quic_srichara@...cinc.com
Subject: Re: [PATCH v4 3/6] dt-bindings: PCI: qcom: Add IPQ5018 SoC



On 3/14/25 12:20, Krzysztof Kozlowski wrote:
> On Fri, Mar 14, 2025 at 09:56:41AM +0400, George Moussalem wrote:
>> From: Nitheesh Sekar <quic_nsekar@...cinc.com>
>>
>> Add support for the PCIe controller on the Qualcomm
>> IPQ5108 SoC to the bindings.
>>
>> Signed-off-by: Nitheesh Sekar <quic_nsekar@...cinc.com>
>> Signed-off-by: Sricharan Ramabadhran <quic_srichara@...cinc.com>
>> Signed-off-by: George Moussalem <george.moussalem@...look.com>
>> ---
>>   .../devicetree/bindings/pci/qcom,pcie.yaml    | 59 +++++++++++++++++++
>>   1 file changed, 59 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> index 8f628939209e..d8befaa558e2 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> @@ -21,6 +21,7 @@ properties:
>>             - qcom,pcie-apq8064
>>             - qcom,pcie-apq8084
>>             - qcom,pcie-ipq4019
>> +          - qcom,pcie-ipq5018
>>             - qcom,pcie-ipq6018
>>             - qcom,pcie-ipq8064
>>             - qcom,pcie-ipq8064-v2
>> @@ -322,6 +323,63 @@ allOf:
>>               - const: ahb # AHB reset
>>               - const: phy_ahb # PHY AHB reset
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,pcie-ipq5018
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 5
>> +          maxItems: 5
>> +        reg-names:
>> +          items:
>> +            - const: parf # Qualcomm specific registers
>> +            - const: dbi # DesignWare PCIe registers
>> +            - const: elbi # External local bus interface registers
>> +            - const: atu # ATU address space
>> +            - const: config # PCIe configuration space
> 
> Keep the same order as other IPQ, so dbi+elbi+atu+parf+config. Same for
> everything else, so standard rule applies: devices are supposed to use
> ordering from existing variants.
> 
> There is some huge mess with IPQ PCI bindings, including things on the
> list. Apparently it became my job to oversee Qualcomm PCI work... well,
> I do not have time for that, so rather I expect contributors to
> cooperate in this matter.
> 
> Don't throw your patches over the wall.
> 
> If you need to rework the patch, take the ownership and rework it.
> 
> 

Thanks Krzysztof. I did reorder them deliberately based on unit 
addresses as discussed also in other threads about IPQ9574 and IPQ5332 
as I thought it would be neater that way. I'll change it back, reuse 
other sections in the dt as much as possible, and follow your guidance 
instead.

> 
> 
> 
>> +        clocks:
>> +          minItems: 6
>> +          maxItems: 6
>> +        clock-names:
>> +          items:
>> +            - const: iface # PCIe to SysNOC BIU clock
>> +            - const: axi_m # AXI Master clock
>> +            - const: axi_s # AXI Slave clock
>> +            - const: ahb # AHB clock
>> +            - const: aux # Auxiliary clock
>> +            - const: axi_bridge # AXI bridge clock
>> +        resets:
>> +          minItems: 8
>> +          maxItems: 8
>> +        reset-names:
>> +          items:
>> +            - const: pipe # PIPE reset
>> +            - const: sleep # Sleep reset
>> +            - const: sticky # Core sticky reset
>> +            - const: axi_m # AXI master reset
>> +            - const: axi_s # AXI slave reset
>> +            - const: ahb # AHB reset
>> +            - const: axi_m_sticky # AXI master sticky reset
>> +            - const: axi_s_sticky # AXI slave sticky reset
>> +        interrupts:
>> +          minItems: 8
>> +          maxItems: 8
> 
> 8 items...
> 
>> +        interrupt-names:
>> +          items:
>> +            - const: msi0
>> +            - const: msi1
>> +            - const: msi2
>> +            - const: msi3
>> +            - const: msi4
>> +            - const: msi5
>> +            - const: msi6
>> +            - const: msi7
>> +            - const: global
> 
> And here 9 items. You got comment on this. What's more, I doubt that DTS
> was tsted.

Will fix, thanks.

> 
> Best regards,
> Krzysztof
> 

Best regards,
George