Re: [PATCH v1 01/13] dt-bindings: Add bindings for Tegra234 Host1x and VIC

From: Mikko Perttunen
Date: Tue May 17 2022 - 04:44:44 EST


On 5/16/22 19:33, Rob Herring wrote:
On Mon, May 16, 2022 at 01:02:01PM +0300, cyndis@xxxxxxxx wrote:
From: Mikko Perttunen <mperttunen@xxxxxxxxxx>

Update VIC and Host1x bindings for changes in Tegra234.

Namely,
- New compatible strings
- Sharded syncpoint interrupts
- Optional reset.

Signed-off-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
---
.../display/tegra/nvidia,tegra124-vic.yaml | 1 +
.../display/tegra/nvidia,tegra20-host1x.yaml | 108 +++++++++++++++---
2 files changed, 95 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
index 37bb5ddc1963..7200095ef19e 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra124-vic.yaml
@@ -21,6 +21,7 @@ properties:
- nvidia,tegra210-vic
- nvidia,tegra186-vic
- nvidia,tegra194-vic
+ - nvidia,tegra234-vic
- items:
- const: nvidia,tegra132-vic
diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
index 0adeb03b9e3a..83c58b7dae98 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
@@ -24,6 +24,7 @@ properties:
- nvidia,tegra210-host1x
- nvidia,tegra186-host1x
- nvidia,tegra194-host1x
+ - nvidia,tegra234-host1x
- items:
- const: nvidia,tegra132-host1x
@@ -31,23 +32,19 @@ properties:
reg:
minItems: 1
- maxItems: 2
+ maxItems: 3
reg-names:
minItems: 1
- maxItems: 2
+ maxItems: 3
interrupts:
- items:
- - description: host1x syncpoint interrupt
- - description: host1x general interrupt
minItems: 1
+ maxItems: 9
interrupt-names:
- items:
- - const: syncpt
- - const: host1x
minItems: 1
+ maxItems: 9
'#address-cells':
description: The number of cells used to represent physical base addresses
@@ -110,13 +107,32 @@ required:
- reg
- clocks
- clock-names
- - resets
- - reset-names

Shouldn't these still be required on some platforms?

Yes, I'll add them back in the tegra20..tegra210 conditional.


additionalProperties:
type: object
allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nvidia,tegra20-host1x
+ - nvidia,tegra30-host1x
+ - nvidia,tegra114-host1x
+ - nvidia,tegra124-host1x
+ - nvidia,tegra210-host1x
+ then:
+ properties:
+ interrupts:
+ items:
+ - description: host1x syncpoint interrupt
+ - description: host1x general interrupt
+
+ interrupt-names:
+ items:
+ - const: syncpt
+ - const: host1x
- if:
properties:
compatible:
@@ -133,10 +149,10 @@ allOf:
reg:
items:
- - description: physical base address and length of the register
- region assigned to the VM
- description: physical base address and length of the register
region used by the hypervisor
+ - description: physical base address and length of the register
+ region assigned to the VM

You can't just change the order at least without a good explanation why
in the commit message. It's an ABI.

Yeah, this doesn't change ABI, it's just a documentation bugfix, but indeed I should have mentioned it in the commit message. In 'reg-names' the order is given as 'hypervisor, vm' and the descriptions here were the wrong way around.


resets:
maxItems: 1
@@ -144,6 +160,70 @@ allOf:
reset-names:
maxItems: 1
+ interrupts:
+ items:
+ - description: host1x syncpoint interrupt
+ - description: host1x general interrupt
+
+ interrupt-names:
+ items:
+ - const: syncpt
+ - const: host1x
+
+ iommu-map:
+ description: Specification of stream IDs available for memory context device
+ use. Should be a mapping of IDs 0..n to IOMMU entries corresponding to
+ usable stream IDs.
+
+ required:
+ - reg-names
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nvidia,tegra234-host1x
+ then:
+ properties:
+ reg-names:
+ items:
+ - const: common
+ - const: hypervisor
+ - const: vm
+
+ reg:
+ items:
+ - description: physical base address and length of the register
+ region used by host1x server
+ - description: physical base address and length of the register
+ region used by the hypervisor
+ - description: physical base address and length of the register
+ region assigned to the VM

I guess this is just copied, but 'physical base address and length of
the ' is redundant. That's every 'reg'.

I'll fix these up in the next revision.

Thanks,
Mikko


+
+ interrupts:
+ items:
+ - description: host1x syncpoint interrupt 0
+ - description: host1x syncpoint interrupt 1
+ - description: host1x syncpoint interrupt 2
+ - description: host1x syncpoint interrupt 3
+ - description: host1x syncpoint interrupt 4
+ - description: host1x syncpoint interrupt 5
+ - description: host1x syncpoint interrupt 6
+ - description: host1x syncpoint interrupt 7
+ - description: host1x general interrupt
+
+ interrupt-names:
+ items:
+ - const: syncpt0
+ - const: syncpt1
+ - const: syncpt2
+ - const: syncpt3
+ - const: syncpt4
+ - const: syncpt5
+ - const: syncpt6
+ - const: syncpt7
+ - const: host1x
+
iommu-map:
description: Specification of stream IDs available for memory context device
use. Should be a mapping of IDs 0..n to IOMMU entries corresponding to
@@ -160,8 +240,8 @@ examples:
host1x@50000000 {
compatible = "nvidia,tegra20-host1x";
reg = <0x50000000 0x00024000>;
- interrupts = <0 65 0x04 /* mpcore syncpt */
- 0 67 0x04>; /* mpcore general */
+ interrupts = <0 65 0x04>, /* mpcore syncpt */
+ <0 67 0x04>; /* mpcore general */
interrupt-names = "syncpt", "host1x";
clocks = <&tegra_car TEGRA20_CLK_HOST1X>;
clock-names = "host1x";
--
2.36.1