All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.