Re: [PATCH 1/7] dt-bindings: PCI: qcom: Add IPQ9574 specific compatible

From: Devi Priya <quic_devipriy@quicinc.com>
To: Manivannan Sadhasivam <mani@kernel.org>
Cc: <agross@kernel.org>, <andersson@kernel.org>,
	<konrad.dybcio@linaro.org>, <lpieralisi@kernel.org>,
	<kw@linux.com>, <robh@kernel.org>, <bhelgaas@google.com>,
	<krzysztof.kozlowski+dt@linaro.org>, <vkoul@kernel.org>,
	<kishon@kernel.org>, <mturquette@baylibre.com>,
	<sboyd@kernel.org>, <p.zabel@pengutronix.de>,
	<svarbanov@mm-sol.com>, <linux-arm-msm@vger.kernel.org>,
	<linux-pci@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-phy@lists.infradead.org>,
	<linux-clk@vger.kernel.org>, <quic_srichara@quicinc.com>,
	<quic_gokulsri@quicinc.com>, <quic_sjaganat@quicinc.com>,
	<quic_kathirav@quicinc.com>, <quic_arajkuma@quicinc.com>,
	<quic_anusha@quicinc.com>
Subject: Re: [PATCH 1/7] dt-bindings: PCI: qcom: Add IPQ9574 specific compatible
Date: Tue, 28 Feb 2023 10:56:53 +0530	[thread overview]
Message-ID: <bd153038-4427-1f11-1941-5f13fec01cf7@quicinc.com> (raw)
In-Reply-To: <20230224082332.GA5443@thinkpad>


On 2/24/2023 1:53 PM, Manivannan Sadhasivam wrote:
> On Tue, Feb 14, 2023 at 10:11:29PM +0530, Devi Priya wrote:
>> Document the compatible for IPQ9574
>>
Hi Mani, Thanks for taking time to review the patch.
> 
> You didn't mention about the "msi-parent" property that is being added
> by this patch
Sure, will update the commit message in the next spin
> 
>> Signed-off-by: Devi Priya <quic_devipriy@quicinc.com>
>> ---
>>   .../devicetree/bindings/pci/qcom,pcie.yaml    | 72 ++++++++++++++++++-
>>   1 file changed, 70 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> index 872817d6d2bd..dabdf2684e2d 100644
>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
>> @@ -26,6 +26,7 @@ properties:
>>             - qcom,pcie-ipq8064-v2
>>             - qcom,pcie-ipq8074
>>             - qcom,pcie-ipq8074-gen3
>> +          - qcom,pcie-ipq9574
>>             - qcom,pcie-msm8996
>>             - qcom,pcie-qcs404
>>             - qcom,pcie-sa8540p
>> @@ -44,11 +45,11 @@ properties:
>>   
>>     reg:
>>       minItems: 4
>> -    maxItems: 5
>> +    maxItems: 6
>>   
>>     reg-names:
>>       minItems: 4
>> -    maxItems: 5
>> +    maxItems: 6
>>   
>>     interrupts:
>>       minItems: 1
>> @@ -105,6 +106,8 @@ properties:
>>       items:
>>         - const: pciephy
>>   
>> +  msi-parent: true
>> +
>>     power-domains:
>>       maxItems: 1
>>   
>> @@ -173,6 +176,27 @@ allOf:
>>               - const: parf # Qualcomm specific registers
>>               - const: config # PCIe configuration space
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,pcie-ipq9574
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 5
>> +          maxItems: 6
>> +        reg-names:
>> +          minItems: 5
>> +          items:
>> +            - const: dbi # DesignWare PCIe registers
>> +            - const: elbi # External local bus interface registers
>> +            - const: atu # ATU address space
>> +            - const: parf # Qualcomm specific registers
>> +            - const: config # PCIe configuration space
>> +            - const: aggr_noc #PCIe aggr_noc
> 
> Why do you need this region unlike other SoCs? Is the driver making use of it?
We have the aggr_noc region in ipq9574 to achieve higher throughput & to 
handle multiple PCIe instances. The driver uses it to rate adapt 1-lane 
PCIe clocks. My bad, missed it. Will add the driver changes in V2.
> 
> Thanks,
> Mani
> 
>> +
>>     - if:
>>         properties:
>>           compatible:
>> @@ -365,6 +389,39 @@ allOf:
>>               - const: ahb # AHB Reset
>>               - const: axi_m_sticky # AXI Master Sticky reset
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,pcie-ipq9574
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 6
>> +          maxItems: 6
>> +        clock-names:
>> +          items:
>> +            - const: ahb  # AHB clock
>> +            - const: aux  # Auxiliary clock
>> +            - const: axi_m # AXI Master clock
>> +            - const: axi_s # AXI Slave clock
>> +            - const: axi_bridge # AXI bridge clock
>> +            - const: rchng
>> +        resets:
>> +          minItems: 8
>> +          maxItems: 8
>> +        reset-names:
>> +          items:
>> +            - const: pipe # PIPE reset
>> +            - const: sticky # Core Sticky reset
>> +            - const: axi_s_sticky # AXI Slave Sticky reset
>> +            - const: axi_s # AXI Slave reset
>> +            - const: axi_m_sticky # AXI Master Sticky reset
>> +            - const: axi_m # AXI Master reset
>> +            - const: aux # AUX Reset
>> +            - const: ahb # AHB Reset
>> +
>>     - if:
>>         properties:
>>           compatible:
>> @@ -681,6 +738,16 @@ allOf:
>>           - interconnects
>>           - interconnect-names
>>   
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,pcie-ipq9574
>> +    then:
>> +      required:
>> +        - msi-parent
>> +
>>     - if:
>>         not:
>>           properties:
>> @@ -693,6 +760,7 @@ allOf:
>>                   - qcom,pcie-ipq8064v2
>>                   - qcom,pcie-ipq8074
>>                   - qcom,pcie-ipq8074-gen3
>> +                - qcom,pcie-ipq9574
>>                   - qcom,pcie-qcs404
>>       then:
>>         required:
>> -- 
>> 2.17.1
>>
> 
Thanks,
Devi Priya