Messages in this thread | | | Date | Wed, 8 Mar 2023 14:19:58 +0530 | Subject | Re: [PATCH 1/7] dt-bindings: PCI: qcom: Add IPQ9574 specific compatible | From | Devi Priya <> |
| |
On 3/7/2023 8:26 PM, Dmitry Baryshkov wrote: > On Tue, 7 Mar 2023 at 16:40, Devi Priya <quic_devipriy@quicinc.com> wrote: >> >> >> >> On 3/7/2023 6:26 PM, Manivannan Sadhasivam wrote: >>> 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@kernel.org> пишет: >>>>>>> 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@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. >>>>>>> >>>>>>> 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. >> Can we use 'syscon' here, as we are not scaling the interconnect >> frequency and this is just a single register write for setting >> the rate adapter? > > It should be done outside of the PCIe driver. > It is not "just a single register". It is also setting the anoc/snoc > clocks for USB. And maybe something else, which we haven't seen at > this moment. You are still setting up the NoC, even if the icc > frequency is not scaled. > Sure Dmitry, Got it
Regards, Devi Priya
| |