Patchwork [03/23] ARM: dt: tegra30: iommu: Add "nvidia,memory-clients"

login
register
mail settings
Submitter Hiroshi Doyu
Date June 26, 2013, 9:28 a.m.
Message ID <1372238906-9346-4-git-send-email-hdoyu@nvidia.com>
Download mbox | patch
Permalink /patch/254655/
State Superseded, archived
Headers show

Comments

Hiroshi Doyu - June 26, 2013, 9:28 a.m.
Add "nvidia,memory-clients" to identify which swgroup ID a device
belongs to.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 .../devicetree/bindings/iommu/nvidia,tegra30-smmu.txt | 11 +++++++++++
 arch/arm/boot/dts/tegra30.dtsi                        | 19 +++++++++++++++++++
 2 files changed, 30 insertions(+)
Thierry Reding - June 26, 2013, 10:10 a.m.
On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
> 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
Thierry Reding - June 26, 2013, 10:18 a.m.
On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
[...]
> diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
[...]
> @@ -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?

> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
[...]
> index 14ec3f9..3fcee3f 100644
> --- a/arch/arm/boot/dts/tegra30.dtsi
> +++ b/arch/arm/boot/dts/tegra30.dtsi
> @@ -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.

> @@ -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
Hiroshi Doyu - June 26, 2013, 10:33 a.m.
Thierry Reding <thierry.reding@gmail.com> wrote @ Wed, 26 Jun 2013 12:10:30 +0200:

> * PGP Signed by an unknown key
> 
> On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
> > 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?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding - June 26, 2013, 10:33 a.m.
On Wed, Jun 26, 2013 at 12:18:17PM +0200, Thierry Reding wrote:
> On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
> [...]
> > diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
> [...]
> > @@ -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.

Thierry
Hiroshi Doyu - June 26, 2013, 11:02 a.m.
Thierry Reding <thierry.reding@gmail.com> wrote @ Wed, 26 Jun 2013 12:18:17 +0200:

> > diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
> [...]
> > @@ -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.

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

Sounds quite reasonable to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joseph Lo - June 26, 2013, 11:04 a.m.
On Wed, 2013-06-26 at 18:33 +0800, Hiroshi Doyu wrote:
> Thierry Reding <thierry.reding@gmail.com> wrote @ Wed, 26 Jun 2013 12:10:30 +0200:
> 
> > * PGP Signed by an unknown key
> > 
> > On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
> > > 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

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hiroshi Doyu - July 1, 2013, 8:18 a.m.
Joseph Lo <josephl@nvidia.com> wrote @ Wed, 26 Jun 2013 13:04:06 +0200:

> On Wed, 2013-06-26 at 18:33 +0800, Hiroshi Doyu wrote:
> > Thierry Reding <thierry.reding@gmail.com> wrote @ Wed, 26 Jun 2013 12:10:30 +0200:
> > 
> > > * PGP Signed by an unknown key
> > > 
> > > On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
> > > > 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.

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - July 1, 2013, 11:07 p.m.
On 06/26/2013 04:18 AM, Thierry Reding wrote:
> On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote: 
> [...]
>> diff --git
>> a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
>> b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
>
>> 
[...]
>> @@ -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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
index 6be51f6..2081e27 100644
--- a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
@@ -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>;
+			....
+		};
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 14ec3f9..3fcee3f 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -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@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";
 	};