[v4,1/4] dt-bindings: PCI: qcom-ep: Add support for SA8775P SoC

Message ID 1698413592-26523-2-git-send-email-quic_msarkar@quicinc.com
State New
Headers
Series arm64: qcom: sa8775p: add support for EP PCIe |

Commit Message

Mrinmay Sarkar Oct. 27, 2023, 1:33 p.m. UTC
  Add devicetree bindings support for SA8775P SoC. It has DMA register
space and dma interrupt to support HDMA.

Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
---
 .../devicetree/bindings/pci/qcom,pcie-ep.yaml      | 48 +++++++++++++++++++++-
 1 file changed, 46 insertions(+), 2 deletions(-)
  

Comments

Krzysztof Kozlowski Oct. 27, 2023, 1:50 p.m. UTC | #1
On 27/10/2023 15:33, Mrinmay Sarkar wrote:
> Add devicetree bindings support for SA8775P SoC. It has DMA register
> space and dma interrupt to support HDMA.
> 
> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>

Unfortunately I do not see any of my comment addressed. :(

This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.

Best regards,
Krzysztof
  
Mrinmay Sarkar Oct. 30, 2023, 10:19 a.m. UTC | #2
On 10/27/2023 7:20 PM, Krzysztof Kozlowski wrote:
> On 27/10/2023 15:33, Mrinmay Sarkar wrote:
>> Add devicetree bindings support for SA8775P SoC. It has DMA register
>> space and dma interrupt to support HDMA.
>>
>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
> Unfortunately I do not see any of my comment addressed. :(
>
> This is a friendly reminder during the review process.
>
> It seems my or other reviewer's previous comments were not fully
> addressed. Maybe the feedback got lost between the quotes, maybe you
> just forgot to apply it. Please go back to the previous discussion and
> either implement all requested changes or keep discussing them.
>
> Thank you.
>
> Best regards,
> Krzysztof
Thanks Krzysztof for your review and patience.
Sorry I missed your previous comment.

If I understand correctly by constraining IO space/interrupt,
you mean to add maxItems for reg and interrupt for other variants.
If so, I verified adding maxItems for these properties and dtb check
seems to be good. I will post the same in the next patch series.

Thanks,
Mrinmay
>
  
Krzysztof Kozlowski Oct. 30, 2023, 10:50 a.m. UTC | #3
On 30/10/2023 11:19, Mrinmay Sarkar wrote:
> 
> On 10/27/2023 7:20 PM, Krzysztof Kozlowski wrote:
>> On 27/10/2023 15:33, Mrinmay Sarkar wrote:
>>> Add devicetree bindings support for SA8775P SoC. It has DMA register
>>> space and dma interrupt to support HDMA.
>>>
>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
>> Unfortunately I do not see any of my comment addressed. :(
>>
>> This is a friendly reminder during the review process.
>>
>> It seems my or other reviewer's previous comments were not fully
>> addressed. Maybe the feedback got lost between the quotes, maybe you
>> just forgot to apply it. Please go back to the previous discussion and
>> either implement all requested changes or keep discussing them.
>>
>> Thank you.
>>
>> Best regards,
>> Krzysztof
> Thanks Krzysztof for your review and patience.
> Sorry I missed your previous comment.

Multiple comments. Also from Mani and maybe from others?

> 
> If I understand correctly by constraining IO space/interrupt,
> you mean to add maxItems for reg and interrupt for other variants.
> If so, I verified adding maxItems for these properties and dtb check
> seems to be good. I will post the same in the next patch series.
> 
> Thanks,
> Mrinmay
>>

Best regards,
Krzysztof
  
Mrinmay Sarkar Oct. 30, 2023, 12:22 p.m. UTC | #4
On 10/30/2023 4:20 PM, Krzysztof Kozlowski wrote:
> On 30/10/2023 11:19, Mrinmay Sarkar wrote:
>> On 10/27/2023 7:20 PM, Krzysztof Kozlowski wrote:
>>> On 27/10/2023 15:33, Mrinmay Sarkar wrote:
>>>> Add devicetree bindings support for SA8775P SoC. It has DMA register
>>>> space and dma interrupt to support HDMA.
>>>>
>>>> Signed-off-by: Mrinmay Sarkar <quic_msarkar@quicinc.com>
>>> Unfortunately I do not see any of my comment addressed. :(
>>>
>>> This is a friendly reminder during the review process.
>>>
>>> It seems my or other reviewer's previous comments were not fully
>>> addressed. Maybe the feedback got lost between the quotes, maybe you
>>> just forgot to apply it. Please go back to the previous discussion and
>>> either implement all requested changes or keep discussing them.
>>>
>>> Thank you.
>>>
>>> Best regards,
>>> Krzysztof
>> Thanks Krzysztof for your review and patience.
>> Sorry I missed your previous comment.
> Multiple comments. Also from Mani and maybe from others?
I have discussed and addressed all the comments from others.
As per the discussions. Updated commit messages and made
necessary changes in dtsi node.

please correct me otherwise.

>> If I understand correctly by constraining IO space/interrupt,
>> you mean to add maxItems for reg and interrupt for other variants.
>> If so, I verified adding maxItems for these properties and dtb check
>> seems to be good. I will post the same in the next patch series.
>>
>> Thanks,
>> Mrinmay
> Best regards,
> Krzysztof
Thanks,
Mrinmay
>
  

Patch

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
index a223ce0..567efc4 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
@@ -13,6 +13,7 @@  properties:
   compatible:
     oneOf:
       - enum:
+          - qcom,sa8775p-pcie-ep
           - qcom,sdx55-pcie-ep
           - qcom,sm8450-pcie-ep
       - items:
@@ -20,6 +21,7 @@  properties:
           - const: qcom,sdx55-pcie-ep
 
   reg:
+    minItems: 6
     items:
       - description: Qualcomm-specific PARF configuration registers
       - description: DesignWare PCIe registers
@@ -27,8 +29,10 @@  properties:
       - description: Address Translation Unit (ATU) registers
       - description: Memory region used to map remote RC address space
       - description: BAR memory region
+      - description: DMA register space
 
   reg-names:
+    minItems: 6
     items:
       - const: parf
       - const: dbi
@@ -36,13 +40,14 @@  properties:
       - const: atu
       - const: addr_space
       - const: mmio
+      - const: dma
 
   clocks:
-    minItems: 7
+    minItems: 5
     maxItems: 8
 
   clock-names:
-    minItems: 7
+    minItems: 5
     maxItems: 8
 
   qcom,perst-regs:
@@ -57,14 +62,18 @@  properties:
           - description: Perst separation enable offset
 
   interrupts:
+    minItems: 2
     items:
       - description: PCIe Global interrupt
       - description: PCIe Doorbell interrupt
+      - description: DMA interrupt
 
   interrupt-names:
+    minItems: 2
     items:
       - const: global
       - const: doorbell
+      - const: dma
 
   reset-gpios:
     description: GPIO used as PERST# input signal
@@ -173,6 +182,41 @@  allOf:
             - const: ddrss_sf_tbu
             - const: aggre_noc_axi
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sa8775p-pcie-ep
+    then:
+      properties:
+        reg:
+          minItems: 7
+          maxItems: 7
+        reg-names:
+          minItems: 7
+          maxItems: 7
+        clocks:
+          items:
+            - description: PCIe Auxiliary clock
+            - description: PCIe CFG AHB clock
+            - description: PCIe Master AXI clock
+            - description: PCIe Slave AXI clock
+            - description: PCIe Slave Q2A AXI clock
+        clock-names:
+          items:
+            - const: aux
+            - const: cfg
+            - const: bus_master
+            - const: bus_slave
+            - const: slave_q2a
+        interrupts:
+          minItems: 3
+          maxItems: 3
+        interrupt-names:
+          minItems: 3
+          maxItems: 3
+
 unevaluatedProperties: false
 
 examples: