On Tue, Mar 07, 2023 at 03:15:08PM +0530, Devi Priya wrote: > > > On 3/3/2023 11:10 PM, Manivannan Sadhasivam wrote: > > On Fri, Mar 03, 2023 at 05:16:58PM +0200, Dmitry Baryshkov wrote: > > > 28 февраля 2023 г. 08:33:58 GMT+02:00, Manivannan Sadhasivam <mani@xxxxxxxxxx> пишет: > > > > On Tue, Feb 28, 2023 at 10:56:53AM +0530, Devi Priya wrote: > > > > > > > > > > > > > > > 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@xxxxxxxxxxx> > > > > > > > --- > > > > > > > .../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. > > > > > > > > Hmm, this is something new. How can you achieve higher throughput with this > > > > region? Can you explain more on how it is used? > > > > > > Based on the name of the region, it looks like it is an interconnect region. > > > > > > > Well, we only have BCM based interconnects so far. That's why I was curious > > about this region and its purpose. > For connected PCIe slave devices that are running at frequency lesser > than the ANOC frequency (342MHz), the rate adapter of ANOC needs to be > configured > > > > > Devi, if this is the case, then you have to handle it through the interconnect driver, rather than poking directly into these registers. > > > > If that so, it doesn't need to be added in this series itself. I believe that > > without aggr_noc region, the PCIe controller can still function properly with > > reduced performance. But you can add the interconnect support later as a > > separate series. > Sure, okay. The ANOC runs at a fixed frequency of 342MHz and the > interconnect clocks are not scaled. The aggr_noc register is just a magic > register for configuring it's rate adapter to ensure no wait cycles are > inserted. > If the purpose of the aggr_noc region is to configure the interconnect clock, then it should be modeled as an interconnect driver. Thanks, Mani > > > > Thanks, > > Mani > > > > > > > > > > > > > > > > Thanks, > > > > Mani > > > > > > > > > > > > > > > > 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 > > > > > > > > > > Thanks, > Devi Priya -- மணிவண்ணன் சதாசிவம்
