Re: [PATCH V2 1/2] dt-bindings: PCI: tegra234: Add ECAM support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxx>

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

Co-developed-by: Vidya Sagar <vidyas@xxxxxxxxxx>
Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
Co-developed-by: Jon Hunter <jonathanh@xxxxxxxxxx>
Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
---
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



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux