Re: [PATCH v1 1/5] dt-bindings: PCI: qcom-ep: Add support for SA8775P SoC

From: Mrinmay Sarkar
Date: Fri Oct 13 2023 - 08:55:52 EST


On 10/11/2023 5:13 PM, Dmitry Baryshkov wrote:
On Wed, 11 Oct 2023 at 14:14, Mrinmay Sarkar <quic_msarkar@xxxxxxxxxxx> wrote:

On 10/6/2023 4:24 PM, Shazad Hussain wrote:

On 9/22/2023 12:08 AM, Rob Herring wrote:
On Wed, Sep 20, 2023 at 07:25:08PM +0530, Mrinmay Sarkar wrote:
Add devicetree bindings support for SA8775P SoC.
Define reg and interrupt per platform.

Signed-off-by: Mrinmay Sarkar <quic_msarkar@xxxxxxxxxxx>
---
.../devicetree/bindings/pci/qcom,pcie-ep.yaml | 130
+++++++++++++++++----
1 file changed, 108 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
index a223ce0..e860e8f 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,29 +21,19 @@ properties:
- const: qcom,sdx55-pcie-ep
reg:
- items:
- - description: Qualcomm-specific PARF configuration registers
- - description: DesignWare PCIe registers
- - description: External local bus interface registers
- - description: Address Translation Unit (ATU) registers
- - description: Memory region used to map remote RC address space
- - description: BAR memory region
+ minItems: 6
+ maxItems: 7
reg-names:
- items:
- - const: parf
- - const: dbi
- - const: elbi
- - const: atu
- - const: addr_space
- - const: mmio
+ minItems: 6
+ maxItems: 7
Don't move these into if/then schemas. Then we are duplicating the
names, and there is no reason to keep them aligned for new compatibles.

Rob
Hi Rob,
As we have one extra reg property (dma) required for sa8775p-pcie-ep,
isn't it expected to be moved in if/then as per number of regs
required. Anyways we would have duplication of some properties for new
compatibles where the member numbers differs for a property.

Are you suggesting to add the extra reg property (dma) in the existing
reg and reg-names list, and add minItems/maxItems for all compatibles
present in this file ?
This is what we have been doing in other cases: if the list is an
extension of the current list, there is no need to duplicate it. One
can use min/maxItems instead.
Hi Dmitry

we have tried using min/maxItems rather than duplicating but somehow
catch up with some warnings in dt_bindings check

//local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: reg: [[29360128, 12288], [1073741824, 3869], [1073745696, 200], [1073745920, 4096], [1073750016, 4096], [29372416, 12288]] is too short//
//        from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#//
///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: reg-names: ['parf', 'dbi', 'elbi', 'atu', 'addr_space', 'mmio'] is too short//
//        from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#//
///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: interrupts: [[0, 140, 4], [0, 145, 4]] is too short//
//        from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#//
///local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short//
//        from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-ep.yaml#//
/

//local/mnt/workspace/Mrinmay/lemans/next-20230914/linux-next/out/Documentation/devicetree/bindings/pci/qcom,pcie-ep.example.dtb: pcie-ep@1c00000: interrupt-names: ['global', 'doorbell'] is too short/

added the patch in attachment.

--Mrinmay

-Shazad
Here we have defined reg and interrupt per platform as clocks is defined.

-Mrinmay

From 520653ae6996366942f21a8942b5d8ac33e30ee3 Mon Sep 17 00:00:00 2001
From: Mrinmay Sarkar <quic_msarkar@xxxxxxxxxxx>
Date: Fri, 13 Oct 2023 18:09:56 +0530
Subject: [PATCH] dt-bindings: PCI: qcom-ep: Add support for SA8775P SoC

Add devicetree bindings support for SA8775P SoC.

Signed-off-by: Mrinmay Sarkar <quic_msarkar@xxxxxxxxxxx>
---
.../devicetree/bindings/pci/qcom,pcie-ep.yaml | 73 ++++++++++++++++++-
1 file changed, 70 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
index a223ce029cab..00eef92685a2 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-ep.yaml
@@ -13,9 +13,11 @@ properties:
compatible:
oneOf:
- enum:
+ - qcom,sa8775p-pcie-ep
- qcom,sdx55-pcie-ep
- qcom,sm8450-pcie-ep
- items:
+ - const: qcom,sa8775p-pcie-ep
- const: qcom,sdx65-pcie-ep
- const: qcom,sdx55-pcie-ep

@@ -27,6 +29,7 @@ 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:
items:
@@ -36,13 +39,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:
@@ -60,11 +64,13 @@ properties:
items:
- description: PCIe Global interrupt
- description: PCIe Doorbell interrupt
+ - description: DMA interrupt

interrupt-names:
items:
- const: global
- const: doorbell
+ - const: dma

reset-gpios:
description: GPIO used as PERST# input signal
@@ -125,7 +131,13 @@ allOf:
- qcom,sdx55-pcie-ep
then:
properties:
- clocks:
+ reg:
+ maxItems: 6
+ minItems: 6
+ reg-names:
+ maxItems: 6
+ minItems: 6
+ clocks:
items:
- description: PCIe Auxiliary clock
- description: PCIe CFG AHB clock
@@ -143,6 +155,12 @@ allOf:
- const: slave_q2a
- const: sleep
- const: ref
+ interrupts:
+ maxItems: 2
+ minItems: 2
+ interrupt-names:
+ maxItems: 3
+ minItems: 3

- if:
properties:
@@ -152,6 +170,13 @@ allOf:
- qcom,sm8450-pcie-ep
then:
properties:
+ properties:
+ reg:
+ maxItems: 6
+ minItems: 6
+ reg-names:
+ maxItems: 6
+ minItems: 6
clocks:
items:
- description: PCIe Auxiliary clock
@@ -172,6 +197,48 @@ allOf:
- const: ref
- const: ddrss_sf_tbu
- const: aggre_noc_axi
+ interrupts:
+ maxItems: 2
+ minItems: 2
+ interrupt-names:
+ maxItems: 3
+ minItems: 3
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,sa8775p-pcie-ep
+ then:
+ properties:
+ properties:
+ reg:
+ maxItems: 7
+ minItems: 7
+ reg-names:
+ maxItems: 7
+ minItems: 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:
+ maxItems: 3
+ minItems: 3
+ interrupt-names:
+ maxItems: 3
+ minItems: 3

unevaluatedProperties: false

--
2.17.1