On Tue, 7 Mar 2023 at 11:45, Devi Priya <quic_devipriy@xxxxxxxxxxx> 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. I have been hesitant at some point, but this looks more and more like a special kind of interconnect. Please consider moving all the NoC stuff into a separate driver implementing the ICC API. > > > > > 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 -- With best wishes Dmitry
