ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

PATCH Superseded Hiroshi Doyu 4 participants 9 messages Jun 26, 2013 – Jul 1, 2013

Subsystems (3)

Device tree bindings Tegra architecture support IOMMU

lore

Share & tools

lore

Hiroshi Doyu

author

2013-06-26 09:28 UTC #0 lore raw

PATCH 3/23 ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

Add "nvidia,memory-clients" to identify which swgroup ID a device belongs to.

Signed-off-by Hiroshi Doyu <[email protected]>

---
 .../devicetree/bindings/iommu/nvidia,tegra30-smmu.txt | 11 +++++++++++
 arch/arm/boot/dts/tegra30.dtsi                        | 19 +++++++++++++++++++
 2 files changed, 30 insertions(+)

Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt +11 -0

index 6be51f6..2081e27 100644
@@ -11,6 +11,7 @@ Required properties:
 - nvidia,swgroups: A bit map of supported HardWare Accelerators(HWA).
   Each bit represents one sgroup. The assignments may be found in header
   file <dt-bindings/iommu/tegra-swgid.h>.
+- nvidia,memory-clients: Indicates which swgroups a device belongs to.
 
 Example:
 	iommu {

@@ -23,3 +24,13 @@ Example:
 		nvidia,swgroups = <0x00000000 0x000779ff>;
 		nvidia,ahb = <&ahb>;
 	};
+
+	host1x {
+		compatible = "nvidia,tegra30-host1x", "simple-bus";
+		nvidia,memory-clients = <SWGID_HC>;
+		....
+		gr3d {
+			compatible = "nvidia,tegra30-gr3d";
+			nvidia,memory-clients = <SWGID_NV SWGID_NV2>;
+			....
+		};

arch/arm/boot/dts/tegra30.dtsi +19 -0

index 14ec3f9..3fcee3f 100644
@@ -1,5 +1,6 @@
 #include <dt-bindings/clock/tegra30-car.h>
 #include <dt-bindings/gpio/tegra-gpio.h>
+#include <dt-bindings/iommu/tegra-swgid.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 
 #include "skeleton.dtsi"

@@ -22,6 +23,7 @@
 		interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>, /* syncpt */
 			     <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>; /* general */
 		clocks = <&tegra_car TEGRA30_CLK_HOST1X>;
+		nvidia,memory-clients = <SWGID_HC>;
 
 		#address-cells = <1>;
 		#size-cells = <1>;

@@ -33,6 +35,7 @@
 			reg = <0x54040000 0x00040000>;
 			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_MPE>;
+			nvidia,memory-clients = <SWGID_MPE>;
 		};
 
 		vi {

@@ -40,6 +43,7 @@
 			reg = <0x54080000 0x00040000>;
 			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_VI>;
+			nvidia,memory-clients = <SWGID_VI>;
 		};
 
 		epp {

@@ -47,6 +51,7 @@
 			reg = <0x540c0000 0x00040000>;
 			interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_EPP>;
+			nvidia,memory-clients = <SWGID_EPP>;
 		};
 
 		isp {

@@ -54,6 +59,7 @@
 			reg = <0x54100000 0x00040000>;
 			interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_ISP>;
+			nvidia,memory-clients = <SWGID_ISP>;
 		};
 
 		gr2d {

@@ -61,6 +67,7 @@
 			reg = <0x54140000 0x00040000>;
 			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_GR2D>;
+			nvidia,memory-clients = <SWGID_G2>;
 		};
 
 		gr3d {

@@ -68,6 +75,7 @@
 			reg = <0x54180000 0x00040000>;
 			clocks = <&tegra_car 24 &tegra_car 98>;
 			clock-names = "3d", "3d2";
+			nvidia,memory-clients = <SWGID_NV SWGID_NV2>;
 		};
 
 		dc at 54200000 {

@@ -77,6 +85,7 @@
 			clocks = <&tegra_car TEGRA30_CLK_DISP1>,
 				 <&tegra_car TEGRA30_CLK_PLL_P>;
 			clock-names = "disp1", "parent";
+			nvidia,memory-clients = <SWGID_DC>;
 
 			rgb {
 				status = "disabled";

@@ -90,6 +99,7 @@
 			clocks = <&tegra_car TEGRA30_CLK_DISP2>,
 				 <&tegra_car TEGRA30_CLK_PLL_P>;
 			clock-names = "disp2", "parent";
+			nvidia,memory-clients = <SWGID_DCB>;
 
 			rgb {
 				status = "disabled";

@@ -246,6 +256,7 @@
 		interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 8>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTA>;
+		nvidia,memory-clients = <SWGID_PPCS>;
 		status = "disabled";
 	};

@@ -256,6 +267,7 @@
 		interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 9>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTB>;
+		nvidia,memory-clients = <SWGID_PPCS>;
 		status = "disabled";
 	};

@@ -266,6 +278,7 @@
 		interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 10>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTC>;
+		nvidia,memory-clients = <SWGID_PPCS>;
 		status = "disabled";
 	};

@@ -276,6 +289,7 @@
 		interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 19>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTD>;
+		nvidia,memory-clients = <SWGID_PPCS>;
 		status = "disabled";
 	};

@@ -286,6 +300,7 @@
 		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 20>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTE>;
+		nvidia,memory-clients = <14>;
 		status = "disabled";
 	};

@@ -535,6 +550,7 @@
 		reg = <0x78000000 0x200>;
 		interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC1>;
+		nvidia,memory-clients = <SWGID_PPCS>;
 		status = "disabled";
 	};

@@ -543,6 +559,7 @@
 		reg = <0x78000200 0x200>;
 		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC2>;
+		nvidia,memory-clients = <SWGID_PPCS>;
 		status = "disabled";
 	};

@@ -551,6 +568,7 @@
 		reg = <0x78000400 0x200>;
 		interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC3>;
+		nvidia,memory-clients = <SWGID_PPCS>;
 		status = "disabled";
 	};

@@ -559,6 +577,7 @@
 		reg = <0x78000600 0x200>;
 		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC4>;
+		nvidia,memory-clients = <SWGID_PPCS>;
 		status = "disabled";
 	};

Signature · 1 line

1.8.1.5

Thierry Reding

2013-06-26 10:10 UTC #1 lore raw

PATCH 3/23 ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

Quoted 2 lines · Hiroshi Doyu

Add "nvidia,memory-clients" to identify which swgroup ID a device belongs to.

Why not call the property "nvidia,swgid" instead? That seems a lot more intuitive.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130626/635ee5df/attachment-0001.sig>

Hiroshi Doyu

author

2013-06-26 10:33 UTC #2 lore raw

PATCH 3/23 ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

Thierry Reding <[email protected]> wrote @ Wed, 26 Jun 2013 12:10:30 +0200:

Quoted 8 lines

* PGP Signed by an unknown key
Quoted 2 lines · Hiroshi Doyu
Add "nvidia,memory-clients" to identify which swgroup ID a device belongs to.
Why not call the property "nvidia,swgid" instead? That seems a lot more intuitive.

Tegra powerdomain also needs this info. Joseph?

Joseph Lo

2013-06-26 11:04 UTC #3 lore raw

PATCH 3/23 ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

Quoted 4 lines · Hiroshi Doyu

Thierry Reding <[email protected]> wrote @ Wed, 26 Jun 2013 12:10:30 +0200:
Quoted 2 lines
* PGP Signed by an unknown key

Quoted 8 lines

Quoted 6 lines · Hiroshi Doyu
Quoted 2 lines
Add "nvidia,memory-clients" to identify which swgroup ID a device belongs to.
Why not call the property "nvidia,swgid" instead? That seems a lot more intuitive.
Tegra powerdomain also needs this info. Joseph?

Ah, yes. We need some kind of this information to know the relation in the power domain when we wants to power down it. But it didn't upstream yet and still in very early stage of the design. I believe you can ignore that and do the good design here for your usage. We can re-use the binding if it point to the same HW behind that.

Thanks,
Joseph

Hiroshi Doyu

author

2013-07-01 08:18 UTC #4 lore raw

PATCH 3/23 ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

Joseph Lo <[email protected]> wrote @ Wed, 26 Jun 2013 13:04:06 +0200:

Quoted 12 lines · Hiroshi Doyu

Quoted 11 lines
Thierry Reding <[email protected]> wrote @ Wed, 26 Jun 2013 12:10:30 +0200:
Quoted 8 lines
* PGP Signed by an unknown key
Quoted 2 lines · Hiroshi Doyu
Add "nvidia,memory-clients" to identify which swgroup ID a device belongs to.
Why not call the property "nvidia,swgid" instead? That seems a lot more intuitive.

Quoted 7 lines

Quoted 1 line
Tegra powerdomain also needs this info. Joseph?
Ah, yes. We need some kind of this information to know the relation in the power domain when we wants to power down it. But it didn't upstream yet and still in very early stage of the design. I believe you can ignore that and do the good design here for your usage. We can re-use the binding if it point to the same HW behind that.

Ok, at least, I'll move this "tegra-swgid.h" to under "include/dt-bindings/memory/". This look to suite better than "include/dt-bindings/iommu/" since this isn't specific to iommu but others needs from memory client POV.

Thierry Reding

2013-06-26 10:18 UTC #5 lore raw

PATCH 3/23 ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

In reply to Hiroshi Doyu

On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
[...]

Quoted 1 line

Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt +0 -0

[...]

Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt Example: · line 30

Quoted 8 lines

Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt +4 -0

@@ -23,3 +24,13 @@ Example:
 		nvidia,swgroups = <0x00000000 0x000779ff>;
 		nvidia,ahb = <&ahb>;
 	};
+
+	host1x {
+		compatible = "nvidia,tegra30-host1x", "simple-bus";
+		nvidia,memory-clients = <SWGID_HC>;

And this could use the SWGID(HC) to match up with how GPIOs are referenced in the DTS files. Though I see that the clocks don't use a parameterized version either, so things are inconsistent anyway. But if SWGID() isn't used then maybe it shouldn't be provided by the header file in the first place.

Oh, one other thing: both GPIO and CAR use the TEGRA_ prefix, perhaps this should use it as well?

Quoted 1 line

arch/arm/boot/dts/tegra30.dtsi +0 -0

[...]

arch/arm/boot/dts/tegra30.dtsi line 4

Quoted 8 lines

index 14ec3f9..3fcee3f 100644

arch/arm/boot/dts/tegra30.dtsi +1 -0

@@ -1,5 +1,6 @@
 #include <dt-bindings/clock/tegra30-car.h>
 #include <dt-bindings/gpio/tegra-gpio.h>
+#include <dt-bindings/iommu/tegra-swgid.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>

Nit: these includes seem to be ordered alphabetically; if so then iommu should go below interrupt-controller.

arch/arm/boot/dts/tegra30.dtsi line 303

Quoted 5 lines

arch/arm/boot/dts/tegra30.dtsi +1 -0

@@ -286,6 +300,7 @@
 		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 20>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTE>;
+		nvidia,memory-clients = <14>;

SWGID_PPCS?

Thierry Reding

2013-06-26 10:33 UTC #6 lore raw

PATCH 3/23 ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

Quoted 4 lines · Thierry Reding

On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
[...]
Quoted 1 line
Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt +0 -0
[...]

Quoted 11 lines

Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt Example: · line 30
Quoted 8 lines
Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt +4 -0
@@ -23,3 +24,13 @@ Example:
 		nvidia,swgroups = <0x00000000 0x000779ff>;
 		nvidia,ahb = <&ahb>;
 	};
+
+	host1x {
+		compatible = "nvidia,tegra30-host1x", "simple-bus";
+		nvidia,memory-clients = <SWGID_HC>;
And this could use the SWGID(HC) to match up with how GPIOs are referenced in the DTS files.

Scratch that. SWGID() yields a mask for the corresponding swgroup so it's not the same thing.

Hiroshi Doyu

author

2013-06-26 11:02 UTC #7 lore raw

PATCH 3/23 ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

In reply to Thierry Reding

Thierry Reding <[email protected]> wrote @ Wed, 26 Jun 2013 12:18:17 +0200:

Quoted 16 lines

Quoted 1 line
Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt +0 -0
[...]
Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt Example: · line 30
Quoted 8 lines
Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt +4 -0
@@ -23,3 +24,13 @@ Example:
 		nvidia,swgroups = <0x00000000 0x000779ff>;
 		nvidia,ahb = <&ahb>;
 	};
+
+	host1x {
+		compatible = "nvidia,tegra30-host1x", "simple-bus";
+		nvidia,memory-clients = <SWGID_HC>;
And this could use the SWGID(HC) to match up with how GPIOs are referenced in the DTS files. Though I see that the clocks don't use a parameterized version either, so things are inconsistent anyway. But if SWGID() isn't used then maybe it shouldn't be provided by the header file in the first place.

I think that the above SWGID_* is still easier to understand which swgroup a device belongs to rather than raw number intuitively.

Quoted 2 lines

Oh, one other thing: both GPIO and CAR use the TEGRA_ prefix, perhaps this should use it as well?

Sounds quite reasonable to me.

Stephen Warren

2013-07-01 23:07 UTC #8 lore raw

PATCH 3/23 ARM: dt: tegra30: iommu: Add "nvidia, memory-clients"

In reply to Thierry Reding

Quoted 7 lines · Thierry Reding

On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
[...]
Quoted 5 lines
diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt

[...]

Quoted 7 lines

Quoted 4 lines
(quoted diff) +0 -0
@@ -23,3 +24,13 @@ Example: nvidia,swgroups = <0x00000000
0x000779ff>; nvidia,ahb = <&ahb>; }; + + host1x { + compatible =
"nvidia,tegra30-host1x", "simple-bus"; + nvidia,memory-clients =
<SWGID_HC>;
And this could use the SWGID(HC) to match up with how GPIOs are referenced in the DTS files.

I only made TEGRA_GPIO(bank, id) a macro because there was a regular bank/id structure to the GPIO numbering. I haven't read these patches yet, but I assume SWGIDs are just a flat numbering space with arbitrarily assigned number in HW, so there's no much point saying SWGID(X) rather than SWGID_X.