public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/2] Add ECAM aperture for Tegra234
@ 2022-11-14 14:09 Jon Hunter
  2022-11-14 14:09 ` [PATCH V2 1/2] dt-bindings: PCI: tegra234: Add ECAM support Jon Hunter
  2022-11-14 14:09 ` [PATCH V2 2/2] arm64: tegra: Add ECAM aperture info for all the PCIe controllers Jon Hunter
  0 siblings, 2 replies; 6+ messages in thread
From: Jon Hunter @ 2022-11-14 14:09 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Thierry Reding
  Cc: linux-pci, devicetree, linux-tegra, vidyas, mmaddireddy,
	Jon Hunter

This series adds ECAM aperture information in both device-tree and
documentation files for Tegra234.

Vidya Sagar (2):
  dt-bindings: PCI: tegra234: Add ECAM support
  arm64: tegra: Add ECAM aperture info for all the PCIe controllers

 .../bindings/pci/nvidia,tegra194-pcie.yaml    | 76 +++++++++++++++----
 .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
 arch/arm64/boot/dts/nvidia/tegra234.dtsi      | 55 ++++++++------
 3 files changed, 95 insertions(+), 38 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V2 1/2] dt-bindings: PCI: tegra234: Add ECAM support
  2022-11-14 14:09 [PATCH V2 0/2] Add ECAM aperture for Tegra234 Jon Hunter
@ 2022-11-14 14:09 ` Jon Hunter
  2022-11-14 14:23   ` Krzysztof Kozlowski
  2022-11-14 14:09 ` [PATCH V2 2/2] arm64: tegra: Add ECAM aperture info for all the PCIe controllers Jon Hunter
  1 sibling, 1 reply; 6+ messages in thread
From: Jon Hunter @ 2022-11-14 14:09 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Thierry Reding
  Cc: linux-pci, devicetree, linux-tegra, vidyas, mmaddireddy,
	Jon Hunter

From: Vidya Sagar <vidyas@nvidia.com>

Add support for ECAM aperture that is only supported for Tegra234
devices.

Co-developed-by: Vidya Sagar <vidyas@nvidia.com>
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
Co-developed-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
Changes since V1:
- Restricted the ECAM aperture to only Tegra234 devices that support it.

 .../bindings/pci/nvidia,tegra194-pcie.yaml    | 76 +++++++++++++++----
 .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
 2 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
index 75da3e8eecb9..7ae0f37f5364 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
@@ -27,21 +27,12 @@ properties:
       - nvidia,tegra234-pcie
 
   reg:
-    items:
-      - description: controller's application logic registers
-      - description: configuration registers
-      - description: iATU and DMA registers. This is where the iATU (internal
-          Address Translation Unit) registers of the PCIe core are made
-          available for software access.
-      - description: aperture where the Root Port's own configuration
-          registers are available.
+    minItems: 4
+    maxItems: 5
 
   reg-names:
-    items:
-      - const: appl
-      - const: config
-      - const: atu_dma
-      - const: dbi
+    minItems: 4
+    maxItems: 5
 
   interrupts:
     items:
@@ -202,6 +193,60 @@ properties:
 
 allOf:
   - $ref: /schemas/pci/snps,dw-pcie.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra194-pcie
+    then:
+      properties:
+        reg:
+          minItems: 4
+          maxItems: 4
+          items:
+            - description: controller's application logic registers
+            - description: configuration registers
+            - description: iATU and DMA registers. This is where the iATU (internal
+                Address Translation Unit) registers of the PCIe core are made
+                available for software access.
+            - description: aperture where the Root Port's own configuration
+                registers are available.
+        reg-names:
+          items:
+            - const: appl
+            - const: config
+            - const: atu_dma
+            - const: dbi
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra234-pcie
+    then:
+      properties:
+        reg:
+          minItems: 5
+          maxItems: 5
+          items:
+            - description: controller's application logic registers
+            - description: configuration registers
+            - description: iATU and DMA registers. This is where the iATU (internal
+                Address Translation Unit) registers of the PCIe core are made
+                available for software access.
+            - description: aperture where the Root Port's own configuration
+                registers are available.
+            - description: aperture to access the configuration space through ECAM.
+        reg-names:
+          items:
+            - const: appl
+            - const: config
+            - const: atu_dma
+            - const: dbi
+            - const: ecam
+
 
 unevaluatedProperties: false
 
@@ -305,8 +350,9 @@ examples:
             reg = <0x00 0x14160000 0x0 0x00020000>, /* appl registers (128K)      */
                   <0x00 0x36000000 0x0 0x00040000>, /* configuration space (256K) */
                   <0x00 0x36040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-                  <0x00 0x36080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-            reg-names = "appl", "config", "atu_dma", "dbi";
+                  <0x00 0x36080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+                  <0x24 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+            reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
             #address-cells = <3>;
             #size-cells = <2>;
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 7287d395e1b6..7e0b015f1414 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -35,7 +35,7 @@ properties:
     maxItems: 5
     items:
       enum: [ dbi, dbi2, config, atu, atu_dma, app, appl, elbi, mgmt, ctrl,
-              parf, cfg, link, ulreg, smu, mpu, apb, phy ]
+              parf, cfg, link, ulreg, smu, mpu, apb, phy, ecam ]
 
   num-lanes:
     description: |
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH V2 2/2] arm64: tegra: Add ECAM aperture info for all the PCIe controllers
  2022-11-14 14:09 [PATCH V2 0/2] Add ECAM aperture for Tegra234 Jon Hunter
  2022-11-14 14:09 ` [PATCH V2 1/2] dt-bindings: PCI: tegra234: Add ECAM support Jon Hunter
@ 2022-11-14 14:09 ` Jon Hunter
  1 sibling, 0 replies; 6+ messages in thread
From: Jon Hunter @ 2022-11-14 14:09 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Thierry Reding
  Cc: linux-pci, devicetree, linux-tegra, vidyas, mmaddireddy,
	Jon Hunter

From: Vidya Sagar <vidyas@nvidia.com>

Add the ECAM aperture information for all the PCIe controllers of
Tegra234.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
Changes since V1:
- None

 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 55 ++++++++++++++----------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index ed7676c9521b..37afe1e03700 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -2235,8 +2235,9 @@ pcie@140a0000 {
 		reg = <0x00 0x140a0000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x2a000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x2a040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x2a080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x2a080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x35 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2288,8 +2289,9 @@ pcie@140c0000 {
 		reg = <0x00 0x140c0000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x2c000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x2c040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x2c080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x2c080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x38 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2341,8 +2343,9 @@ pcie@140e0000 {
 		reg = <0x00 0x140e0000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x2e000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x2e040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x2e080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x2e080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x3b 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2394,8 +2397,9 @@ pcie@14100000 {
 		reg = <0x00 0x14100000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x30000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x30040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x30080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x30080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x20 0xb0000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2447,8 +2451,9 @@ pcie@14120000 {
 		reg = <0x00 0x14120000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x32000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x32040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x32080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x32080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x20 0xf0000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2500,8 +2505,9 @@ pcie@14140000 {
 		reg = <0x00 0x14140000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x34000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x34040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x34080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x34080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x21 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2553,8 +2559,9 @@ pcie@14160000 {
 		reg = <0x00 0x14160000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x36000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x36040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x36080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x36080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x24 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2606,8 +2613,9 @@ pcie@14180000 {
 		reg = <0x00 0x14180000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x38000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x38040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x38080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x38080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x27 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2659,8 +2667,9 @@ pcie@141a0000 {
 		reg = <0x00 0x141a0000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x3a000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x3a040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x3a080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x3a080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x2b 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2712,8 +2721,9 @@ pcie@141c0000 {
 		reg = <0x00 0x141c0000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x3c000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x3c040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x3c080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x3c080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x2e 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
@@ -2765,8 +2775,9 @@ pcie@141e0000 {
 		reg = <0x00 0x141e0000 0x0 0x00020000>, /* appl registers (128K)      */
 		      <0x00 0x3e000000 0x0 0x00040000>, /* configuration space (256K) */
 		      <0x00 0x3e040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-		      <0x00 0x3e080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-		reg-names = "appl", "config", "atu_dma", "dbi";
+		      <0x00 0x3e080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+		      <0x32 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+		reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
 		#address-cells = <3>;
 		#size-cells = <2>;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2 1/2] dt-bindings: PCI: tegra234: Add ECAM support
  2022-11-14 14:09 ` [PATCH V2 1/2] dt-bindings: PCI: tegra234: Add ECAM support Jon Hunter
@ 2022-11-14 14:23   ` Krzysztof Kozlowski
  2022-11-14 15:36     ` Jon Hunter
  0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-14 14:23 UTC (permalink / raw)
  To: Jon Hunter, Rob Herring, Krzysztof Kozlowski, Thierry Reding
  Cc: linux-pci, devicetree, linux-tegra, vidyas, mmaddireddy

On 14/11/2022 15:09, Jon Hunter wrote:
> From: Vidya Sagar <vidyas@nvidia.com>
> 
> Add support for ECAM aperture that is only supported for Tegra234
> devices.
> 
> Co-developed-by: Vidya Sagar <vidyas@nvidia.com>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> Co-developed-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> Changes since V1:
> - Restricted the ECAM aperture to only Tegra234 devices that support it.
> 
>  .../bindings/pci/nvidia,tegra194-pcie.yaml    | 76 +++++++++++++++----
>  .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
>  2 files changed, 62 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> index 75da3e8eecb9..7ae0f37f5364 100644
> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> @@ -27,21 +27,12 @@ properties:
>        - nvidia,tegra234-pcie
>  
>    reg:
> -    items:
> -      - description: controller's application logic registers
> -      - description: configuration registers
> -      - description: iATU and DMA registers. This is where the iATU (internal
> -          Address Translation Unit) registers of the PCIe core are made
> -          available for software access.
> -      - description: aperture where the Root Port's own configuration
> -          registers are available.
> +    minItems: 4
> +    maxItems: 5
>  
>    reg-names:
> -    items:
> -      - const: appl
> -      - const: config
> -      - const: atu_dma
> -      - const: dbi
> +    minItems: 4
> +    maxItems: 5
>  
>    interrupts:
>      items:
> @@ -202,6 +193,60 @@ properties:
>  
>  allOf:
>    - $ref: /schemas/pci/snps,dw-pcie.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - nvidia,tegra194-pcie
> +    then:
> +      properties:
> +        reg:
> +          minItems: 4
> +          maxItems: 4

How you wrote it, you do not need min/maxItems here, because you have
items below. However see further comment.

> +          items:
> +            - description: controller's application logic registers
> +            - description: configuration registers
> +            - description: iATU and DMA registers. This is where the iATU (internal
> +                Address Translation Unit) registers of the PCIe core are made
> +                available for software access.
> +            - description: aperture where the Root Port's own configuration
> +                registers are available.
> +        reg-names:
> +          items:
> +            - const: appl
> +            - const: config
> +            - const: atu_dma
> +            - const: dbi
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - nvidia,tegra234-pcie
> +    then:
> +      properties:
> +        reg:
> +          minItems: 5
> +          maxItems: 5

Similar issue.

> +          items:
> +            - description: controller's application logic registers
> +            - description: configuration registers
> +            - description: iATU and DMA registers. This is where the iATU (internal
> +                Address Translation Unit) registers of the PCIe core are made
> +                available for software access.
> +            - description: aperture where the Root Port's own configuration
> +                registers are available.
> +            - description: aperture to access the configuration space through ECAM.

This is unnecessarily duplicated. You can keep the descriptions of items
and reg-names items in top level (with min 4 and max 5) and restrict
maxItems for 194 and minItems for 234 here.


> +        reg-names:
> +          items:
> +            - const: appl
> +            - const: config
> +            - const: atu_dma
> +            - const: dbi
> +            - const: ecam
> +

No need for blank line.
>  
>  unevaluatedProperties: false
>  

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2 1/2] dt-bindings: PCI: tegra234: Add ECAM support
  2022-11-14 14:23   ` Krzysztof Kozlowski
@ 2022-11-14 15:36     ` Jon Hunter
  2022-11-14 15:57       ` Jon Hunter
  0 siblings, 1 reply; 6+ messages in thread
From: Jon Hunter @ 2022-11-14 15:36 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
	Thierry Reding
  Cc: linux-pci, devicetree, linux-tegra, vidyas, mmaddireddy


On 14/11/2022 14:23, Krzysztof Kozlowski wrote:
> On 14/11/2022 15:09, Jon Hunter wrote:
>> From: Vidya Sagar <vidyas@nvidia.com>
>>
>> Add support for ECAM aperture that is only supported for Tegra234
>> devices.
>>
>> Co-developed-by: Vidya Sagar <vidyas@nvidia.com>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> Co-developed-by: Jon Hunter <jonathanh@nvidia.com>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>> Changes since V1:
>> - Restricted the ECAM aperture to only Tegra234 devices that support it.
>>
>>   .../bindings/pci/nvidia,tegra194-pcie.yaml    | 76 +++++++++++++++----
>>   .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
>>   2 files changed, 62 insertions(+), 16 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>> index 75da3e8eecb9..7ae0f37f5364 100644
>> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>> @@ -27,21 +27,12 @@ properties:
>>         - nvidia,tegra234-pcie
>>   
>>     reg:
>> -    items:
>> -      - description: controller's application logic registers
>> -      - description: configuration registers
>> -      - description: iATU and DMA registers. This is where the iATU (internal
>> -          Address Translation Unit) registers of the PCIe core are made
>> -          available for software access.
>> -      - description: aperture where the Root Port's own configuration
>> -          registers are available.
>> +    minItems: 4
>> +    maxItems: 5
>>   
>>     reg-names:
>> -    items:
>> -      - const: appl
>> -      - const: config
>> -      - const: atu_dma
>> -      - const: dbi
>> +    minItems: 4
>> +    maxItems: 5
>>   
>>     interrupts:
>>       items:
>> @@ -202,6 +193,60 @@ properties:
>>   
>>   allOf:
>>     - $ref: /schemas/pci/snps,dw-pcie.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - nvidia,tegra194-pcie
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 4
>> +          maxItems: 4
> 
> How you wrote it, you do not need min/maxItems here, because you have
> items below. However see further comment.
> 
>> +          items:
>> +            - description: controller's application logic registers
>> +            - description: configuration registers
>> +            - description: iATU and DMA registers. This is where the iATU (internal
>> +                Address Translation Unit) registers of the PCIe core are made
>> +                available for software access.
>> +            - description: aperture where the Root Port's own configuration
>> +                registers are available.
>> +        reg-names:
>> +          items:
>> +            - const: appl
>> +            - const: config
>> +            - const: atu_dma
>> +            - const: dbi
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - nvidia,tegra234-pcie
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 5
>> +          maxItems: 5
> 
> Similar issue.
> 
>> +          items:
>> +            - description: controller's application logic registers
>> +            - description: configuration registers
>> +            - description: iATU and DMA registers. This is where the iATU (internal
>> +                Address Translation Unit) registers of the PCIe core are made
>> +                available for software access.
>> +            - description: aperture where the Root Port's own configuration
>> +                registers are available.
>> +            - description: aperture to access the configuration space through ECAM.
> 
> This is unnecessarily duplicated. You can keep the descriptions of items
> and reg-names items in top level (with min 4 and max 5) and restrict
> maxItems for 194 and minItems for 234 here.


Yes I wondered if there was a good way to avoid duplication. It looks
like I cannot have 'maxItems' and 'items' at the top-level, but
obviously I can set 'maxItems' appropriately for each device.

Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml: properties:reg: {'minItems': 4, 'maxItems': 5, 'items': [{'description': "controller's application logic registers"}, {'description': 'configuration registers'}, {'description': 'iATU and DMA registers. This is where the iATU (internal Address Translation Unit) registers of the PCIe core are made available for software access.'}, {'description': "aperture where the Root Port's own configuration registers are available."}, {'description': 'aperture to access the configuration space through ECAM.'}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list

Thanks
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2 1/2] dt-bindings: PCI: tegra234: Add ECAM support
  2022-11-14 15:36     ` Jon Hunter
@ 2022-11-14 15:57       ` Jon Hunter
  0 siblings, 0 replies; 6+ messages in thread
From: Jon Hunter @ 2022-11-14 15:57 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
	Thierry Reding
  Cc: linux-pci, devicetree, linux-tegra, vidyas, mmaddireddy



On 14/11/2022 15:36, Jon Hunter wrote:
> 
> On 14/11/2022 14:23, Krzysztof Kozlowski wrote:
>> On 14/11/2022 15:09, Jon Hunter wrote:
>>> From: Vidya Sagar <vidyas@nvidia.com>
>>>
>>> Add support for ECAM aperture that is only supported for Tegra234
>>> devices.
>>>
>>> Co-developed-by: Vidya Sagar <vidyas@nvidia.com>
>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>>> Co-developed-by: Jon Hunter <jonathanh@nvidia.com>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>> Changes since V1:
>>> - Restricted the ECAM aperture to only Tegra234 devices that support it.
>>>
>>>   .../bindings/pci/nvidia,tegra194-pcie.yaml    | 76 +++++++++++++++----
>>>   .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
>>>   2 files changed, 62 insertions(+), 16 deletions(-)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml 
>>> b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>>> index 75da3e8eecb9..7ae0f37f5364 100644
>>> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>>> @@ -27,21 +27,12 @@ properties:
>>>         - nvidia,tegra234-pcie
>>>     reg:
>>> -    items:
>>> -      - description: controller's application logic registers
>>> -      - description: configuration registers
>>> -      - description: iATU and DMA registers. This is where the iATU 
>>> (internal
>>> -          Address Translation Unit) registers of the PCIe core are made
>>> -          available for software access.
>>> -      - description: aperture where the Root Port's own configuration
>>> -          registers are available.
>>> +    minItems: 4
>>> +    maxItems: 5
>>>     reg-names:
>>> -    items:
>>> -      - const: appl
>>> -      - const: config
>>> -      - const: atu_dma
>>> -      - const: dbi
>>> +    minItems: 4
>>> +    maxItems: 5
>>>     interrupts:
>>>       items:
>>> @@ -202,6 +193,60 @@ properties:
>>>   allOf:
>>>     - $ref: /schemas/pci/snps,dw-pcie.yaml#
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - nvidia,tegra194-pcie
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 4
>>> +          maxItems: 4
>>
>> How you wrote it, you do not need min/maxItems here, because you have
>> items below. However see further comment.
>>
>>> +          items:
>>> +            - description: controller's application logic registers
>>> +            - description: configuration registers
>>> +            - description: iATU and DMA registers. This is where the 
>>> iATU (internal
>>> +                Address Translation Unit) registers of the PCIe core 
>>> are made
>>> +                available for software access.
>>> +            - description: aperture where the Root Port's own 
>>> configuration
>>> +                registers are available.
>>> +        reg-names:
>>> +          items:
>>> +            - const: appl
>>> +            - const: config
>>> +            - const: atu_dma
>>> +            - const: dbi
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - nvidia,tegra234-pcie
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 5
>>> +          maxItems: 5
>>
>> Similar issue.
>>
>>> +          items:
>>> +            - description: controller's application logic registers
>>> +            - description: configuration registers
>>> +            - description: iATU and DMA registers. This is where the 
>>> iATU (internal
>>> +                Address Translation Unit) registers of the PCIe core 
>>> are made
>>> +                available for software access.
>>> +            - description: aperture where the Root Port's own 
>>> configuration
>>> +                registers are available.
>>> +            - description: aperture to access the configuration 
>>> space through ECAM.
>>
>> This is unnecessarily duplicated. You can keep the descriptions of items
>> and reg-names items in top level (with min 4 and max 5) and restrict
>> maxItems for 194 and minItems for 234 here.
> 
> 
> Yes I wondered if there was a good way to avoid duplication. It looks
> like I cannot have 'maxItems' and 'items' at the top-level, but
> obviously I can set 'maxItems' appropriately for each device.

Ah I get it. The 'items' list is defining the max. I have fixed up and 
sent out a new revision.

Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-14 15:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-14 14:09 [PATCH V2 0/2] Add ECAM aperture for Tegra234 Jon Hunter
2022-11-14 14:09 ` [PATCH V2 1/2] dt-bindings: PCI: tegra234: Add ECAM support Jon Hunter
2022-11-14 14:23   ` Krzysztof Kozlowski
2022-11-14 15:36     ` Jon Hunter
2022-11-14 15:57       ` Jon Hunter
2022-11-14 14:09 ` [PATCH V2 2/2] arm64: tegra: Add ECAM aperture info for all the PCIe controllers Jon Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox