dt-bindings: host1x: Add missing 'dma-coherent'

PATCH In review Jon Hunter 3 participants 7 messages Apr 9, 2024 – Apr 12, 2024

Subsystems (6)

Device tree bindings DRM drivers DRM misc Tegra architecture support +2 more subsystems

lore

Share & tools

lore

Jon Hunter

author maintainer

2024-04-09 08:23 UTC #0 lore raw

PATCH 1/1 dt-bindings: host1x: Add missing 'dma-coherent'

The dtbs_check reports that the 'dma-coherent' property is "unevaluated and invalid" for the host1x@13e00000 device on Tegra194 and Tegra234 platforms. Fix this by updating the dt-binding document for host1x to add the 'dma-coherent' property for these devices.

Fixes 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234")

Signed-off-by Jon Hunter <[email protected]>

---
 .../bindings/display/tegra/nvidia,tegra20-host1x.yaml | 11 +++++++++++
 1 file changed, 11 insertions(+)

Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +11 -0

index 94c5242c03b2..3563378a01af 100644
@@ -177,6 +177,15 @@ allOf:
 
       required:
         - reg-names
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra194-host1x
+    then:
+      properties:
+        dma-coherent: true
   - if:
       properties:
         compatible:

@@ -226,6 +235,8 @@ allOf:
             use. Should be a mapping of IDs 0..n to IOMMU entries corresponding to
             usable stream IDs.
 
+        dma-coherent: true
+
       required:
         - reg-names

Signature · 1 line

2.34.1

Krzysztof Kozlowski

2024-04-10 06:18 UTC #1 lore raw

Quoted 4 lines · Jon Hunter

The dtbs_check reports that the 'dma-coherent' property is "unevaluated and invalid" for the host1x@13e00000 device on Tegra194 and Tegra234 platforms. Fix this by updating the dt-binding document for host1x to add the 'dma-coherent' property for these devices.

That's not really proper reason. What if DTS is wrong? The reason could be if device is actually DMA coherent...

Quoted 14 lines

Fixes 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234")
Signed-off-by Jon Hunter <[email protected]>
---
 .../bindings/display/tegra/nvidia,tegra20-host1x.yaml | 11 +++++++++++
 1 file changed, 11 insertions(+)
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +0 -0
index 94c5242c03b2..3563378a01af 100644
@@ -177,6 +177,15 @@ allOf:
 
       required:

Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml allOf: · line 188

Quoted 10 lines

Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +9 -0

         - reg-names
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra194-host1x
+    then:
+      properties:
+        dma-coherent: true

Do not define properties in allOf. Put it in top-level. If not all devices are DMA coherent, you can disallow it for certain variants (:false).

Best regards,
Krzysztof

Jon Hunter

author maintainer

2024-04-11 10:09 UTC #2 lore raw

Quoted 8 lines · Krzysztof Kozlowski

Quoted 4 lines · Jon Hunter
The dtbs_check reports that the 'dma-coherent' property is "unevaluated and invalid" for the host1x@13e00000 device on Tegra194 and Tegra234 platforms. Fix this by updating the dt-binding document for host1x to add the 'dma-coherent' property for these devices.
That's not really proper reason. What if DTS is wrong? The reason could be if device is actually DMA coherent...

In this case the DTS is correct. I guess I should have been more explicit about that.

Quoted 12 lines

Quoted 12 lines
Fixes 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234")
Signed-off-by Jon Hunter <[email protected]>
---
  .../bindings/display/tegra/nvidia,tegra20-host1x.yaml | 11 +++++++++++
  1 file changed, 11 insertions(+)
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +0 -0
index 94c5242c03b2..3563378a01af 100644
@@ -177,6 +177,15 @@ allOf:
 

Quoted 14 lines

Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml allOf: · line 188
Quoted 11 lines
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +9 -0
        required:
          - reg-names
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra194-host1x
+    then:
+      properties:
+        dma-coherent: true
Do not define properties in allOf. Put it in top-level. If not all devices are DMA coherent, you can disallow it for certain variants (:false).

So for host1x we currently have the following devices supported ...

properties:
compatible:
oneOf:
- enum:
- nvidia,tegra20-host1x
- nvidia,tegra30-host1x
- nvidia,tegra114-host1x
- nvidia,tegra124-host1x
- nvidia,tegra210-host1x
- nvidia,tegra186-host1x
- nvidia,tegra194-host1x
- nvidia,tegra234-host1x

- items:
- const: nvidia,tegra132-host1x
- const: nvidia,tegra124-host1x

Now only the Tegra194 and Tegra234 are coherent (which are the latest devices). So rather than add this to the top and then filter out all those that are not supported, I opted for the above because there is only 2 devices that need this. Admittedly, as much as I like the yaml bindings, for things like this, it is not really clear which is the best way to handle, so appreciate the guidance.

Jon

Signature · 1 line

nvpublic

Krzysztof Kozlowski

2024-04-11 10:43 UTC #3 lore raw

Quoted 62 lines · Jon Hunter

Quoted 8 lines · Krzysztof Kozlowski
Quoted 4 lines · Jon Hunter
The dtbs_check reports that the 'dma-coherent' property is "unevaluated and invalid" for the host1x@13e00000 device on Tegra194 and Tegra234 platforms. Fix this by updating the dt-binding document for host1x to add the 'dma-coherent' property for these devices.
That's not really proper reason. What if DTS is wrong? The reason could be if device is actually DMA coherent...
In this case the DTS is correct. I guess I should have been more explicit about that.
Quoted 12 lines
Quoted 12 lines
Fixes 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234")
Signed-off-by Jon Hunter <[email protected]>
---
  .../bindings/display/tegra/nvidia,tegra20-host1x.yaml | 11 +++++++++++
  1 file changed, 11 insertions(+)
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +0 -0
index 94c5242c03b2..3563378a01af 100644
@@ -177,6 +177,15 @@ allOf:
 
Quoted 14 lines
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml allOf: · line 188
Quoted 11 lines
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +9 -0
        required:
          - reg-names
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra194-host1x
+    then:
+      properties:
+        dma-coherent: true
Do not define properties in allOf. Put it in top-level. If not all devices are DMA coherent, you can disallow it for certain variants (:false).
So for host1x we currently have the following devices supported ...
properties:
compatible:
oneOf:
- enum:
- nvidia,tegra20-host1x
- nvidia,tegra30-host1x
- nvidia,tegra114-host1x
- nvidia,tegra124-host1x
- nvidia,tegra210-host1x
- nvidia,tegra186-host1x
- nvidia,tegra194-host1x
- nvidia,tegra234-host1x
- items:
- const: nvidia,tegra132-host1x
- const: nvidia,tegra124-host1x

Quoted 6 lines

Now only the Tegra194 and Tegra234 are coherent (which are the latest devices). So rather than add this to the top and then filter out all those that are not supported, I opted for the above because there is only 2 devices that need this. Admittedly, as much as I like the yaml bindings, for things like this, it is not really clear which is the best way to handle, so appreciate the guidance.

The way to handle is that you must define properties top-level.

For simplification you could keep the if which duplicates the dma-coherent:true but add else: which forbids it

if:
...
then:
properties:
dma-coherent: true
else:
properties:
dma-coherent: false

Or just ignore the problem. Binding is not 1-to-1 with DTS.

Best regards,
Krzysztof

Rob Herring

maintainer

2024-04-11 14:43 UTC #4 lore raw

Quoted 73 lines · Krzysztof Kozlowski

Quoted 62 lines · Jon Hunter
Quoted 8 lines · Krzysztof Kozlowski
Quoted 4 lines · Jon Hunter
The dtbs_check reports that the 'dma-coherent' property is "unevaluated and invalid" for the host1x@13e00000 device on Tegra194 and Tegra234 platforms. Fix this by updating the dt-binding document for host1x to add the 'dma-coherent' property for these devices.
That's not really proper reason. What if DTS is wrong? The reason could be if device is actually DMA coherent...
In this case the DTS is correct. I guess I should have been more explicit about that.
Quoted 12 lines
Quoted 12 lines
Fixes 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234")
Signed-off-by Jon Hunter <[email protected]>
---
  .../bindings/display/tegra/nvidia,tegra20-host1x.yaml | 11 +++++++++++
  1 file changed, 11 insertions(+)
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +0 -0
index 94c5242c03b2..3563378a01af 100644
@@ -177,6 +177,15 @@ allOf:
 
Quoted 14 lines
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml allOf: · line 188
Quoted 11 lines
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +9 -0
        required:
          - reg-names
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra194-host1x
+    then:
+      properties:
+        dma-coherent: true
Do not define properties in allOf. Put it in top-level. If not all devices are DMA coherent, you can disallow it for certain variants (:false).
So for host1x we currently have the following devices supported ...
properties:
compatible:
oneOf:
- enum:
- nvidia,tegra20-host1x
- nvidia,tegra30-host1x
- nvidia,tegra114-host1x
- nvidia,tegra124-host1x
- nvidia,tegra210-host1x
- nvidia,tegra186-host1x
- nvidia,tegra194-host1x
- nvidia,tegra234-host1x
- items:
- const: nvidia,tegra132-host1x
- const: nvidia,tegra124-host1x
Quoted 6 lines
Now only the Tegra194 and Tegra234 are coherent (which are the latest devices). So rather than add this to the top and then filter out all those that are not supported, I opted for the above because there is only 2 devices that need this. Admittedly, as much as I like the yaml bindings, for things like this, it is not really clear which is the best way to handle, so appreciate the guidance.
The way to handle is that you must define properties top-level.

Quoted 8 lines

For simplification you could keep the if which duplicates the dma-coherent:true but add else: which forbids it
if:
...
then:
properties:
dma-coherent: true

I think just 'then: true' will work here.

Technically, json-schema allows:

if:
...
else:
...

Actually any combination of the 3 keywords present is valid, but we've disallowed the above and other nonsense ones in the meta-schema. We can always relax things if there's a need.

Rob

Rob Herring

maintainer

2024-04-11 14:36 UTC #5 lore raw

In reply to Jon Hunter

Quoted 63 lines · Jon Hunter

Quoted 8 lines · Krzysztof Kozlowski
Quoted 4 lines · Jon Hunter
The dtbs_check reports that the 'dma-coherent' property is "unevaluated and invalid" for the host1x@13e00000 device on Tegra194 and Tegra234 platforms. Fix this by updating the dt-binding document for host1x to add the 'dma-coherent' property for these devices.
That's not really proper reason. What if DTS is wrong? The reason could be if device is actually DMA coherent...
In this case the DTS is correct. I guess I should have been more explicit about that.
Quoted 12 lines
Quoted 12 lines
Fixes 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234")
Signed-off-by Jon Hunter <[email protected]>
---
  .../bindings/display/tegra/nvidia,tegra20-host1x.yaml | 11 +++++++++++
  1 file changed, 11 insertions(+)
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +0 -0
index 94c5242c03b2..3563378a01af 100644
@@ -177,6 +177,15 @@ allOf:
 
Quoted 14 lines
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml allOf: · line 188
Quoted 11 lines
Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml +9 -0
        required:
          - reg-names
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra194-host1x
+    then:
+      properties:
+        dma-coherent: true
Do not define properties in allOf. Put it in top-level. If not all devices are DMA coherent, you can disallow it for certain variants (:false).
So for host1x we currently have the following devices supported ...
properties:
compatible:
oneOf:
- enum:
- nvidia,tegra20-host1x
- nvidia,tegra30-host1x
- nvidia,tegra114-host1x
- nvidia,tegra124-host1x
- nvidia,tegra210-host1x
- nvidia,tegra186-host1x
- nvidia,tegra194-host1x
- nvidia,tegra234-host1x
- items:
- const: nvidia,tegra132-host1x
- const: nvidia,tegra124-host1x

Quoted 6 lines

Now only the Tegra194 and Tegra234 are coherent (which are the latest devices). So rather than add this to the top and then filter out all those that are not supported, I opted for the above because there is only 2 devices that need this. Admittedly, as much as I like the yaml bindings, for things like this, it is not really clear which is the best way to handle, so appreciate the guidance.

I would say how you have it is fine, but that would only be for common boolean properties with no possible constraints. Having that exception would make the preferences less clear I think.

Rob

Krzysztof Kozlowski

Reviewed-by on v1 1/1

2024-04-12 05:26 UTC #6 lore raw

In reply to Jon Hunter

Quoted 8 lines · Jon Hunter

The dtbs_check reports that the 'dma-coherent' property is "unevaluated and invalid" for the host1x@13e00000 device on Tegra194 and Tegra234 platforms. Fix this by updating the dt-binding document for host1x to add the 'dma-coherent' property for these devices.
Fixes 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234")
Signed-off-by Jon Hunter <[email protected]>
---

With mentioning in commit msg that these devices are actually dma-coherent (and after Rob's explanation):

Reviewed-by Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof