[PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Hiroshi Doyu]

Hi,

This series provides:

(1) Unified SMMU driver among Tegra SoCs
(2) Multiple Address Space support(MASID) in IOMMU(SMMMU)
(3) Tegra IOMMU'able devices, most of platform devices are IOMMU'able.

There's some discussion[1] about device population order, which can
solve the following patches.

  [HACK] of: dev_node has struct device pointer
  ARM: tegra: Populate AHB/IOMMU earlier than others

Also "ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations" may
not be necessary by [2]

Tested IOMMU functionality with T30 SD/MMC. Any further testing with
T114 and/or other devices would be really appreciated.

v2:
Updated based on Thierry Reding's and Stephen Warren's feedback

v1:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/180267.html

Hiroshi Doyu (22):
  [HACK] of: dev_node has struct device pointer
  ARM: tegra: Populate AHB/IOMMU earlier than others
  ARM: tegra: Create a DT header defining swgroups ID
  ARM: dt: tegra30: iommu: Add "nvidia,swgroup"
  ARM: dt: tegra30: iommu: Add "nvidia,memory-client"
  ARM: dt: tegra114: iommu: Fix IOMMU register address
  ARM: dt: tegra114: iommu: Add "nvidia,swgroups"
  ARM: dt: tegra114: Add "nvidia,memory-client"
  amba: Move AHB to core_initcall
  iommu/tegra: smmu: Move IOMMU to core_initcall
  iommu/tegra: smmu: Add Tegra 114 support
  iommu/tegra: smmu: Select ARM_DMA_USE_IOMMU in Kconfig
  iommu/tegra: smmu: Create default IOVA maps
  iommu/tegra: smmu: Register platform_device to IOMMU dynamically
  iommu/tegra: smmu: Calculate ASID register offset by ID
  iommu/tegra: smmu: Get "nvidia,swgroup" from DT
  iommu/tegra: smmu: Unfied driver for Tegra SoCs
  iommu/tegra: smmu: Use dt-bindings MACRO
  iommu/tegra: smmu: Workaround PCIe IOMMU'able
  iommu/tegra: smmu: Get "nvidia,memory-client" from DT
  iommu/tegra: smmu: Support Multiple ASID
  ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations

 .../bindings/iommu/nvidia,tegra30-smmu.txt         |   20 +-
 arch/arm/boot/dts/tegra114.dtsi                    |   13 +-
 arch/arm/boot/dts/tegra30.dtsi                     |   21 ++
 arch/arm/mach-tegra/tegra.c                        |   22 ++
 arch/arm/mm/dma-mapping.c                          |    7 +
 drivers/amba/tegra-ahb.c                           |    7 +-
 drivers/iommu/Kconfig                              |    1 +
 drivers/iommu/tegra-smmu.c                         |  211 +++++++++++---------
 drivers/of/base.c                                  |   23 +++
 drivers/of/platform.c                              |    8 +
 include/dt-bindings/memory/tegra-swgroup.h         |   50 +++++
 include/linux/of.h                                 |   16 ++
 12 files changed, 296 insertions(+), 103 deletions(-)
 create mode 100644 include/dt-bindings/memory/tegra-swgroup.h

-- 
1.7.9.5

[1] https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/thread.html#36542
[2] http://marc.info/?l=linux-tegra&m=137173895620957&w=1

[PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Hiroshi Doyu]

Hi,

This series provides:

(1) Unified SMMU driver among Tegra SoCs
(2) Multiple Address Space support(MASID) in IOMMU(SMMMU)
(3) Tegra IOMMU'able devices, most of platform devices are IOMMU'able.

There's some discussion[1] about device population order, which can
solve the following patches.

  [HACK] of: dev_node has struct device pointer
  ARM: tegra: Populate AHB/IOMMU earlier than others

Also "ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations" may
not be necessary by [2]

Tested IOMMU functionality with T30 SD/MMC. Any further testing with
T114 and/or other devices would be really appreciated.

v2:
Updated based on Thierry Reding's and Stephen Warren's feedback

v1:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/180267.html

Hiroshi Doyu (22):
  [HACK] of: dev_node has struct device pointer
  ARM: tegra: Populate AHB/IOMMU earlier than others
  ARM: tegra: Create a DT header defining swgroups ID
  ARM: dt: tegra30: iommu: Add "nvidia,swgroup"
  ARM: dt: tegra30: iommu: Add "nvidia,memory-client"
  ARM: dt: tegra114: iommu: Fix IOMMU register address
  ARM: dt: tegra114: iommu: Add "nvidia,swgroups"
  ARM: dt: tegra114: Add "nvidia,memory-client"
  amba: Move AHB to core_initcall
  iommu/tegra: smmu: Move IOMMU to core_initcall
  iommu/tegra: smmu: Add Tegra 114 support
  iommu/tegra: smmu: Select ARM_DMA_USE_IOMMU in Kconfig
  iommu/tegra: smmu: Create default IOVA maps
  iommu/tegra: smmu: Register platform_device to IOMMU dynamically
  iommu/tegra: smmu: Calculate ASID register offset by ID
  iommu/tegra: smmu: Get "nvidia,swgroup" from DT
  iommu/tegra: smmu: Unfied driver for Tegra SoCs
  iommu/tegra: smmu: Use dt-bindings MACRO
  iommu/tegra: smmu: Workaround PCIe IOMMU'able
  iommu/tegra: smmu: Get "nvidia,memory-client" from DT
  iommu/tegra: smmu: Support Multiple ASID
  ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations

 .../bindings/iommu/nvidia,tegra30-smmu.txt         |   20 +-
 arch/arm/boot/dts/tegra114.dtsi                    |   13 +-
 arch/arm/boot/dts/tegra30.dtsi                     |   21 ++
 arch/arm/mach-tegra/tegra.c                        |   22 ++
 arch/arm/mm/dma-mapping.c                          |    7 +
 drivers/amba/tegra-ahb.c                           |    7 +-
 drivers/iommu/Kconfig                              |    1 +
 drivers/iommu/tegra-smmu.c                         |  211 +++++++++++---------
 drivers/of/base.c                                  |   23 +++
 drivers/of/platform.c                              |    8 +
 include/dt-bindings/memory/tegra-swgroup.h         |   50 +++++
 include/linux/of.h                                 |   16 ++
 12 files changed, 296 insertions(+), 103 deletions(-)
 create mode 100644 include/dt-bindings/memory/tegra-swgroup.h

-- 
1.7.9.5

[1] https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/thread.html#36542
[2] http://marc.info/?l=linux-tegra&m=137173895620957&w=1

[PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Hiroshi Doyu]

To prevent of_platform_populate() from trying to populate duplicate
devices if a device has been already populated.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Need to take care of early_platform_devices, or alternative solution.
---
 drivers/of/base.c     |   23 +++++++++++++++++++++++
 drivers/of/platform.c |    8 ++++++++
 include/linux/of.h    |   16 ++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 5c54279..99062dd 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -230,6 +230,29 @@ const void *of_get_property(const struct device_node *np, const char *name,
 }
 EXPORT_SYMBOL(of_get_property);
 
+struct device *of_get_device(const struct device_node *node)
+{
+	struct device *dev;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&devtree_lock, flags);
+	dev = node->dev;
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
+
+	return dev;
+}
+EXPORT_SYMBOL(of_get_device);
+
+void of_set_device(struct device_node *node, struct device *dev)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&devtree_lock, flags);
+	node->dev = dev;
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
+}
+EXPORT_SYMBOL(of_set_device);
+
 /** Checks if the given "compat" string matches one of the strings in
  * the device's "compatible" property
  */
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index e0a6514..a8f6b09 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -203,10 +203,17 @@ struct platform_device *of_platform_device_create_pdata(
 					struct device *parent)
 {
 	struct platform_device *dev;
+	struct device *tmp;
 
 	if (!of_device_is_available(np))
 		return NULL;
 
+	tmp = of_get_device(np);
+	if (tmp) {
+		dev_info(tmp, "Already populated\n");
+		return to_platform_device(tmp);
+	}
+
 	dev = of_device_alloc(np, bus_id, parent);
 	if (!dev)
 		return NULL;
@@ -228,6 +235,7 @@ struct platform_device *of_platform_device_create_pdata(
 		return NULL;
 	}
 
+	of_set_device(np, &dev->dev);
 	return dev;
 }
 
diff --git a/include/linux/of.h b/include/linux/of.h
index 1fd08ca..b548522 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -60,6 +60,9 @@ struct device_node {
 	struct	kref kref;
 	unsigned long _flags;
 	void	*data;
+
+	struct device *dev;		/* Set only after populated */
+
 #if defined(CONFIG_SPARC)
 	const char *path_component_name;
 	unsigned int unique_id;
@@ -268,6 +271,8 @@ extern const void *of_get_property(const struct device_node *node,
 				int *lenp);
 #define for_each_property_of_node(dn, pp) \
 	for (pp = dn->properties; pp != NULL; pp = pp->next)
+extern struct device *of_get_device(const struct device_node *node);
+extern void of_set_device(struct device_node *node, struct device *dev);
 
 extern int of_n_addr_cells(struct device_node *np);
 extern int of_n_size_cells(struct device_node *np);
@@ -459,6 +464,17 @@ static inline const void *of_get_property(const struct device_node *node,
 	return NULL;
 }
 
+static inline struct device *of_get_device(const struct device_node *node)
+{
+	return NULL;
+}
+
+static inline void of_set_device(const struct device_node *node,
+				 struct device *dev);
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u64(const struct device_node *np,
 				       const char *propname, u64 *out_value)
 {
-- 
1.7.9.5

[PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Hiroshi Doyu]

To prevent of_platform_populate() from trying to populate duplicate
devices if a device has been already populated.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
Need to take care of early_platform_devices, or alternative solution.
---
 drivers/of/base.c     |   23 +++++++++++++++++++++++
 drivers/of/platform.c |    8 ++++++++
 include/linux/of.h    |   16 ++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 5c54279..99062dd 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -230,6 +230,29 @@ const void *of_get_property(const struct device_node *np, const char *name,
 }
 EXPORT_SYMBOL(of_get_property);
 
+struct device *of_get_device(const struct device_node *node)
+{
+	struct device *dev;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&devtree_lock, flags);
+	dev = node->dev;
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
+
+	return dev;
+}
+EXPORT_SYMBOL(of_get_device);
+
+void of_set_device(struct device_node *node, struct device *dev)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&devtree_lock, flags);
+	node->dev = dev;
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
+}
+EXPORT_SYMBOL(of_set_device);
+
 /** Checks if the given "compat" string matches one of the strings in
  * the device's "compatible" property
  */
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index e0a6514..a8f6b09 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -203,10 +203,17 @@ struct platform_device *of_platform_device_create_pdata(
 					struct device *parent)
 {
 	struct platform_device *dev;
+	struct device *tmp;
 
 	if (!of_device_is_available(np))
 		return NULL;
 
+	tmp = of_get_device(np);
+	if (tmp) {
+		dev_info(tmp, "Already populated\n");
+		return to_platform_device(tmp);
+	}
+
 	dev = of_device_alloc(np, bus_id, parent);
 	if (!dev)
 		return NULL;
@@ -228,6 +235,7 @@ struct platform_device *of_platform_device_create_pdata(
 		return NULL;
 	}
 
+	of_set_device(np, &dev->dev);
 	return dev;
 }
 
diff --git a/include/linux/of.h b/include/linux/of.h
index 1fd08ca..b548522 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -60,6 +60,9 @@ struct device_node {
 	struct	kref kref;
 	unsigned long _flags;
 	void	*data;
+
+	struct device *dev;		/* Set only after populated */
+
 #if defined(CONFIG_SPARC)
 	const char *path_component_name;
 	unsigned int unique_id;
@@ -268,6 +271,8 @@ extern const void *of_get_property(const struct device_node *node,
 				int *lenp);
 #define for_each_property_of_node(dn, pp) \
 	for (pp = dn->properties; pp != NULL; pp = pp->next)
+extern struct device *of_get_device(const struct device_node *node);
+extern void of_set_device(struct device_node *node, struct device *dev);
 
 extern int of_n_addr_cells(struct device_node *np);
 extern int of_n_size_cells(struct device_node *np);
@@ -459,6 +464,17 @@ static inline const void *of_get_property(const struct device_node *node,
 	return NULL;
 }
 
+static inline struct device *of_get_device(const struct device_node *node)
+{
+	return NULL;
+}
+
+static inline void of_set_device(const struct device_node *node,
+				 struct device *dev);
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u64(const struct device_node *np,
 				       const char *propname, u64 *out_value)
 {
-- 
1.7.9.5

Re: [PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> To prevent of_platform_populate() from trying to populate duplicate
> devices if a device has been already populated.

You need to send drivers/of patches to the DT maintainer and
devicetree-discuss mailing list.

> Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> Need to take care of early_platform_devices, or alternative solution.

I assume that's a TODO item... Are you planning on fleshing out this
patch to address that issue?

> diff --git a/drivers/of/base.c b/drivers/of/base.c

> +struct device *of_get_device(const struct device_node *node)
> +{
> +	struct device *dev;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&devtree_lock, flags);
> +	dev = node->dev;
> +	raw_spin_unlock_irqrestore(&devtree_lock, flags);

The read (and write in of_set_device()) aren't atomic already? I guess
perhaps they aren't necessarily. It sure seems like some higher-level
locking is needed in of_platform_create_pdata() though, since there's
quite a window there between the get() and set() calls. Or, is there
already some locking in place above that, in which case, I 'm not sure
why there's a need for this level of locking here...

> diff --git a/drivers/of/platform.c b/drivers/of/platform.c

> +	tmp = of_get_device(np);
> +	if (tmp) {
> +		dev_info(tmp, "Already populated\n");
> +		return to_platform_device(tmp);
> +	}

Did you check all callers of this function to make sure that they also
don't do any kind of double-processing?


> diff --git a/include/linux/of.h b/include/linux/of.h

> @@ -60,6 +60,9 @@ struct device_node {
>  	struct	kref kref;
>  	unsigned long _flags;
>  	void	*data;
> +
> +	struct device *dev;		/* Set only after populated */
> +
>  #if defined(CONFIG_SPARC)

Are the extra blank lines needed?

[PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> To prevent of_platform_populate() from trying to populate duplicate
> devices if a device has been already populated.

You need to send drivers/of patches to the DT maintainer and
devicetree-discuss mailing list.

> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> ---
> Need to take care of early_platform_devices, or alternative solution.

I assume that's a TODO item... Are you planning on fleshing out this
patch to address that issue?

> diff --git a/drivers/of/base.c b/drivers/of/base.c

> +struct device *of_get_device(const struct device_node *node)
> +{
> +	struct device *dev;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&devtree_lock, flags);
> +	dev = node->dev;
> +	raw_spin_unlock_irqrestore(&devtree_lock, flags);

The read (and write in of_set_device()) aren't atomic already? I guess
perhaps they aren't necessarily. It sure seems like some higher-level
locking is needed in of_platform_create_pdata() though, since there's
quite a window there between the get() and set() calls. Or, is there
already some locking in place above that, in which case, I 'm not sure
why there's a need for this level of locking here...

> diff --git a/drivers/of/platform.c b/drivers/of/platform.c

> +	tmp = of_get_device(np);
> +	if (tmp) {
> +		dev_info(tmp, "Already populated\n");
> +		return to_platform_device(tmp);
> +	}

Did you check all callers of this function to make sure that they also
don't do any kind of double-processing?


> diff --git a/include/linux/of.h b/include/linux/of.h

> @@ -60,6 +60,9 @@ struct device_node {
>  	struct	kref kref;
>  	unsigned long _flags;
>  	void	*data;
> +
> +	struct device *dev;		/* Set only after populated */
> +
>  #if defined(CONFIG_SPARC)

Are the extra blank lines needed?

Re: [PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

On Tue, Jul 16, 2013 at 04:57:03PM -0600, Stephen Warren wrote:
> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > To prevent of_platform_populate() from trying to populate duplicate
> > devices if a device has been already populated.
> 
> You need to send drivers/of patches to the DT maintainer and
> devicetree-discuss mailing list.
> 
> > Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> > Need to take care of early_platform_devices, or alternative solution.
> 
> I assume that's a TODO item... Are you planning on fleshing out this
> patch to address that issue?

There was some more discussion about it, which can be found in the
following thread:

	https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036934.html

That's one of the last messages and it should have most of the
background. If not you may have to walk up the thread for more context.
In a nutshell I raised the question as to why we can't simply call
of_platform_populate() earlier and side-step all the workarounds that
have found their way into the kernel to side-step the issue of their not
being a struct device associated with the struct device_node.

If of_platform_populate() can be made to run early, then we can restore
a whole lot of consistency in how "early" drivers on an OF kernel work.

Thierry

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Thierry Reding]

On Tue, Jul 16, 2013 at 04:57:03PM -0600, Stephen Warren wrote:
> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > To prevent of_platform_populate() from trying to populate duplicate
> > devices if a device has been already populated.
> 
> You need to send drivers/of patches to the DT maintainer and
> devicetree-discuss mailing list.
> 
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> > Need to take care of early_platform_devices, or alternative solution.
> 
> I assume that's a TODO item... Are you planning on fleshing out this
> patch to address that issue?

There was some more discussion about it, which can be found in the
following thread:

	https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036934.html

That's one of the last messages and it should have most of the
background. If not you may have to walk up the thread for more context.
In a nutshell I raised the question as to why we can't simply call
of_platform_populate() earlier and side-step all the workarounds that
have found their way into the kernel to side-step the issue of their not
being a struct device associated with the struct device_node.

If of_platform_populate() can be made to run early, then we can restore
a whole lot of consistency in how "early" drivers on an OF kernel work.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130716/db0a8b7f/attachment.sig>

Re: [PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Hiroshi Doyu]

Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote @ Wed, 17 Jul 2013 01:16:30 +0200:

> * PGP Signed by an unknown key
> 
> On Tue, Jul 16, 2013 at 04:57:03PM -0600, Stephen Warren wrote:
> > On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > > To prevent of_platform_populate() from trying to populate duplicate
> > > devices if a device has been already populated.
> > 
> > You need to send drivers/of patches to the DT maintainer and
> > devicetree-discuss mailing list.
> > 
> > > Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > ---
> > > Need to take care of early_platform_devices, or alternative solution.
> > 
> > I assume that's a TODO item... Are you planning on fleshing out this
> > patch to address that issue?
>
> There was some more discussion about it, which can be found in the
> following thread:
> 
> 	https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036934.html
> 
> That's one of the last messages and it should have most of the
> background. If not you may have to walk up the thread for more context.
> In a nutshell I raised the question as to why we can't simply call
> of_platform_populate() earlier and side-step all the workarounds that
> have found their way into the kernel to side-step the issue of their not
> being a struct device associated with the struct device_node.

This is the thread which Lorenzo started originally. He may have some
plan to implement/proceed with this, Lorenzo?

[PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Hiroshi Doyu]

Thierry Reding <thierry.reding@gmail.com> wrote @ Wed, 17 Jul 2013 01:16:30 +0200:

> * PGP Signed by an unknown key
> 
> On Tue, Jul 16, 2013 at 04:57:03PM -0600, Stephen Warren wrote:
> > On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > > To prevent of_platform_populate() from trying to populate duplicate
> > > devices if a device has been already populated.
> > 
> > You need to send drivers/of patches to the DT maintainer and
> > devicetree-discuss mailing list.
> > 
> > > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > > ---
> > > Need to take care of early_platform_devices, or alternative solution.
> > 
> > I assume that's a TODO item... Are you planning on fleshing out this
> > patch to address that issue?
>
> There was some more discussion about it, which can be found in the
> following thread:
> 
> 	https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036934.html
> 
> That's one of the last messages and it should have most of the
> background. If not you may have to walk up the thread for more context.
> In a nutshell I raised the question as to why we can't simply call
> of_platform_populate() earlier and side-step all the workarounds that
> have found their way into the kernel to side-step the issue of their not
> being a struct device associated with the struct device_node.

This is the thread which Lorenzo started originally. He may have some
plan to implement/proceed with this, Lorenzo?

Re: [PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Lorenzo Pieralisi]

Hi Hiroshi,

On Mon, Jul 29, 2013 at 11:13:44AM +0100, Hiroshi Doyu wrote:
> Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote @ Wed, 17 Jul 2013 01:16:30 +0200:
> 
> > * PGP Signed by an unknown key
> > 
> > On Tue, Jul 16, 2013 at 04:57:03PM -0600, Stephen Warren wrote:
> > > On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > > > To prevent of_platform_populate() from trying to populate duplicate
> > > > devices if a device has been already populated.
> > > 
> > > You need to send drivers/of patches to the DT maintainer and
> > > devicetree-discuss mailing list.
> > > 
> > > > Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > > ---
> > > > Need to take care of early_platform_devices, or alternative solution.
> > > 
> > > I assume that's a TODO item... Are you planning on fleshing out this
> > > patch to address that issue?
> >
> > There was some more discussion about it, which can be found in the
> > following thread:
> > 
> > 	https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036934.html
> > 
> > That's one of the last messages and it should have most of the
> > background. If not you may have to walk up the thread for more context.
> > In a nutshell I raised the question as to why we can't simply call
> > of_platform_populate() earlier and side-step all the workarounds that
> > have found their way into the kernel to side-step the issue of their not
> > being a struct device associated with the struct device_node.
> 
> This is the thread which Lorenzo started originally. He may have some
> plan to implement/proceed with this, Lorenzo?

I was trying to raise the issue and apparently I succeeded. For now,
I put in place yet-another-temporary-solution to the problem in one
of the drivers I need for power management, but certainly the problem is
not solved, and I can't allocate time to do it myself unfortunately.

I am more than happy to review patches though.

Thanks,
Lorenzo

[PATCH v2 01/22] [HACK] of: dev_node has struct device pointer

[Lorenzo Pieralisi]

Hi Hiroshi,

On Mon, Jul 29, 2013 at 11:13:44AM +0100, Hiroshi Doyu wrote:
> Thierry Reding <thierry.reding@gmail.com> wrote @ Wed, 17 Jul 2013 01:16:30 +0200:
> 
> > * PGP Signed by an unknown key
> > 
> > On Tue, Jul 16, 2013 at 04:57:03PM -0600, Stephen Warren wrote:
> > > On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > > > To prevent of_platform_populate() from trying to populate duplicate
> > > > devices if a device has been already populated.
> > > 
> > > You need to send drivers/of patches to the DT maintainer and
> > > devicetree-discuss mailing list.
> > > 
> > > > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > > > ---
> > > > Need to take care of early_platform_devices, or alternative solution.
> > > 
> > > I assume that's a TODO item... Are you planning on fleshing out this
> > > patch to address that issue?
> >
> > There was some more discussion about it, which can be found in the
> > following thread:
> > 
> > 	https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/036934.html
> > 
> > That's one of the last messages and it should have most of the
> > background. If not you may have to walk up the thread for more context.
> > In a nutshell I raised the question as to why we can't simply call
> > of_platform_populate() earlier and side-step all the workarounds that
> > have found their way into the kernel to side-step the issue of their not
> > being a struct device associated with the struct device_node.
> 
> This is the thread which Lorenzo started originally. He may have some
> plan to implement/proceed with this, Lorenzo?

I was trying to raise the issue and apparently I succeeded. For now,
I put in place yet-another-temporary-solution to the problem in one
of the drivers I need for power management, but certainly the problem is
not solved, and I can't allocate time to do it myself unfortunately.

I am more than happy to review patches though.

Thanks,
Lorenzo

[PATCH v2 02/22] ARM: tegra: Populate AHB/IOMMU earlier than others

[Hiroshi Doyu]

Populate AHB/IOMMU earlier than others. IOMMU depends on AHB. IOMMU
needs to be instanciated earlier than others so that IOMMU can
register other platform devices as IOMMU'able. Once IOMMU is
populated, IOMMU/AHB nodes are detached to prevent another
registeration.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/mach-tegra/tegra.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 0d1e412..fe432d1 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -80,6 +80,26 @@ static struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
 	{}
 };
 
+static void tegra_of_platform_populate_iommu(void)
+{
+	int i;
+	struct platform_device *pdev;
+	const char * const path[] = {"/ahb", "/iommu", };
+
+	for (i = 0; i < ARRAY_SIZE(path); i++) {
+		struct device_node *np;
+
+		np  = of_find_node_by_path(path[i]);
+		if (!np)
+			break;
+
+		pdev = of_platform_device_create(np, NULL, NULL);
+		of_node_put(np);
+		if (!pdev)
+			break;
+	}
+}
+
 static void __init tegra_dt_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
@@ -107,6 +127,8 @@ static void __init tegra_dt_init(void)
 
 	parent = soc_device_to_device(soc_dev);
 
+	tegra_of_platform_populate_iommu();
+
 	/*
 	 * Finished with the static registrations now; fill in the missing
 	 * devices
-- 
1.7.9.5

[PATCH v2 02/22] ARM: tegra: Populate AHB/IOMMU earlier than others

[Hiroshi Doyu]

Populate AHB/IOMMU earlier than others. IOMMU depends on AHB. IOMMU
needs to be instanciated earlier than others so that IOMMU can
register other platform devices as IOMMU'able. Once IOMMU is
populated, IOMMU/AHB nodes are detached to prevent another
registeration.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mach-tegra/tegra.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c
index 0d1e412..fe432d1 100644
--- a/arch/arm/mach-tegra/tegra.c
+++ b/arch/arm/mach-tegra/tegra.c
@@ -80,6 +80,26 @@ static struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
 	{}
 };
 
+static void tegra_of_platform_populate_iommu(void)
+{
+	int i;
+	struct platform_device *pdev;
+	const char * const path[] = {"/ahb", "/iommu", };
+
+	for (i = 0; i < ARRAY_SIZE(path); i++) {
+		struct device_node *np;
+
+		np  = of_find_node_by_path(path[i]);
+		if (!np)
+			break;
+
+		pdev = of_platform_device_create(np, NULL, NULL);
+		of_node_put(np);
+		if (!pdev)
+			break;
+	}
+}
+
 static void __init tegra_dt_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
@@ -107,6 +127,8 @@ static void __init tegra_dt_init(void)
 
 	parent = soc_device_to_device(soc_dev);
 
+	tegra_of_platform_populate_iommu();
+
 	/*
 	 * Finished with the static registrations now; fill in the missing
 	 * devices
-- 
1.7.9.5

Re: [PATCH v2 02/22] ARM: tegra: Populate AHB/IOMMU earlier than others

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Populate AHB/IOMMU earlier than others. IOMMU depends on AHB. IOMMU
> needs to be instanciated earlier than others so that IOMMU can
> register other platform devices as IOMMU'able. Once IOMMU is
> populated, IOMMU/AHB nodes are detached to prevent another
> registeration.

Hmmm. I'm still in two minds about this. If just the IOMMU was needed,
then this wouldn't be too bad. But then there's the dependency on the
AHB device too. And in general, there could be, say, a dependency on a
SoC power-controller, clock controller, reset module, etc. This could
become a huge slippery slope.

Modelling the IOMMU as an explicit bus with the bus-master-capable
devices underneath it would make everything work automatically...

> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c

> +static void tegra_of_platform_populate_iommu(void)
> +{
> +	int i;
> +	struct platform_device *pdev;
> +	const char * const path[] = {"/ahb", "/iommu", };
> +
> +	for (i = 0; i < ARRAY_SIZE(path); i++) {
> +		struct device_node *np;
> +
> +		np  = of_find_node_by_path(path[i]);

I think this should at least look up the nodes by compatible value. See
how e.g. mach-tegra/pmc.c finds the right DT node.

[PATCH v2 02/22] ARM: tegra: Populate AHB/IOMMU earlier than others

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Populate AHB/IOMMU earlier than others. IOMMU depends on AHB. IOMMU
> needs to be instanciated earlier than others so that IOMMU can
> register other platform devices as IOMMU'able. Once IOMMU is
> populated, IOMMU/AHB nodes are detached to prevent another
> registeration.

Hmmm. I'm still in two minds about this. If just the IOMMU was needed,
then this wouldn't be too bad. But then there's the dependency on the
AHB device too. And in general, there could be, say, a dependency on a
SoC power-controller, clock controller, reset module, etc. This could
become a huge slippery slope.

Modelling the IOMMU as an explicit bus with the bus-master-capable
devices underneath it would make everything work automatically...

> diff --git a/arch/arm/mach-tegra/tegra.c b/arch/arm/mach-tegra/tegra.c

> +static void tegra_of_platform_populate_iommu(void)
> +{
> +	int i;
> +	struct platform_device *pdev;
> +	const char * const path[] = {"/ahb", "/iommu", };
> +
> +	for (i = 0; i < ARRAY_SIZE(path); i++) {
> +		struct device_node *np;
> +
> +		np  = of_find_node_by_path(path[i]);

I think this should at least look up the nodes by compatible value. See
how e.g. mach-tegra/pmc.c finds the right DT node.

[PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Hiroshi Doyu]

Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.

This will allow the same header to be used by both device tree files,
and drivers implementing this binding, which guarantees that the two
stay in sync. This also makes device trees more readable by using names
instead of magic numbers.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 include/dt-bindings/memory/tegra-swgroup.h |   50 ++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 include/dt-bindings/memory/tegra-swgroup.h

diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
new file mode 100644
index 0000000..4cfa769
--- /dev/null
+++ b/include/dt-bindings/memory/tegra-swgroup.h
@@ -0,0 +1,50 @@
+/*
+ * This header provides constants for binding nvidia,swgroup ID
+ */
+
+#ifndef _DT_BINDINGS_MEMORY_TEGRA_SWGROUP_H
+#define _DT_BINDINGS_MEMORY_TEGRA_SWGROUP_H
+
+#define TEGRA_SWGROUP_AFI 0
+#define TEGRA_SWGROUP_AVPC 1
+#define TEGRA_SWGROUP_DC 2
+#define TEGRA_SWGROUP_DCB 3
+#define TEGRA_SWGROUP_EPP 4
+#define TEGRA_SWGROUP_G2 5
+#define TEGRA_SWGROUP_HC 6
+#define TEGRA_SWGROUP_HDA 7
+#define TEGRA_SWGROUP_ISP 8
+#define TEGRA_SWGROUP_ISP2 SWGID_ISP
+/* UNUSED: 9 */
+/* UNUSED: 10 */
+#define TEGRA_SWGROUP_MPE 11
+#define TEGRA_SWGROUP_MSENC SWGID_MPE
+#define TEGRA_SWGROUP_NV 12
+#define TEGRA_SWGROUP_NV2 13
+#define TEGRA_SWGROUP_PPCS 14
+#define TEGRA_SWGROUP_SATA2 15
+#define TEGRA_SWGROUP_SATA 16
+#define TEGRA_SWGROUP_VDE 17
+#define TEGRA_SWGROUP_VI 18
+#define TEGRA_SWGROUP_VIC 19
+#define TEGRA_SWGROUP_XUSB_HOST 20
+#define TEGRA_SWGROUP_XUSB_DEV 21
+#define TEGRA_SWGROUP_A9AVP 22
+#define TEGRA_SWGROUP_TSEC 23
+#define TEGRA_SWGROUP_PPCS1 24
+/* UNUSED: 25 */
+/* UNUSED: 26 */
+/* UNUSED: 27 */
+/* UNUSED: 28 */
+/* UNUSED: 29 */
+/* UNUSED: 30 */
+/* UNUSED: 31 */
+
+/* Reserved: 32-63 */
+
+#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
+
+#define TEGRA30_SWGROUP_ALL	<0x00000000 0x000779ff>
+#define TEGRA114_SWGROUP_ALL	<0x00000000 0x01b659fe>
+
+#endif /* _DT_BINDINGS_MEMORY_TEGRA_SWGROUP_H */
-- 
1.7.9.5

[PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Hiroshi Doyu]

Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.

This will allow the same header to be used by both device tree files,
and drivers implementing this binding, which guarantees that the two
stay in sync. This also makes device trees more readable by using names
instead of magic numbers.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 include/dt-bindings/memory/tegra-swgroup.h |   50 ++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 include/dt-bindings/memory/tegra-swgroup.h

diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
new file mode 100644
index 0000000..4cfa769
--- /dev/null
+++ b/include/dt-bindings/memory/tegra-swgroup.h
@@ -0,0 +1,50 @@
+/*
+ * This header provides constants for binding nvidia,swgroup ID
+ */
+
+#ifndef _DT_BINDINGS_MEMORY_TEGRA_SWGROUP_H
+#define _DT_BINDINGS_MEMORY_TEGRA_SWGROUP_H
+
+#define TEGRA_SWGROUP_AFI 0
+#define TEGRA_SWGROUP_AVPC 1
+#define TEGRA_SWGROUP_DC 2
+#define TEGRA_SWGROUP_DCB 3
+#define TEGRA_SWGROUP_EPP 4
+#define TEGRA_SWGROUP_G2 5
+#define TEGRA_SWGROUP_HC 6
+#define TEGRA_SWGROUP_HDA 7
+#define TEGRA_SWGROUP_ISP 8
+#define TEGRA_SWGROUP_ISP2 SWGID_ISP
+/* UNUSED: 9 */
+/* UNUSED: 10 */
+#define TEGRA_SWGROUP_MPE 11
+#define TEGRA_SWGROUP_MSENC SWGID_MPE
+#define TEGRA_SWGROUP_NV 12
+#define TEGRA_SWGROUP_NV2 13
+#define TEGRA_SWGROUP_PPCS 14
+#define TEGRA_SWGROUP_SATA2 15
+#define TEGRA_SWGROUP_SATA 16
+#define TEGRA_SWGROUP_VDE 17
+#define TEGRA_SWGROUP_VI 18
+#define TEGRA_SWGROUP_VIC 19
+#define TEGRA_SWGROUP_XUSB_HOST 20
+#define TEGRA_SWGROUP_XUSB_DEV 21
+#define TEGRA_SWGROUP_A9AVP 22
+#define TEGRA_SWGROUP_TSEC 23
+#define TEGRA_SWGROUP_PPCS1 24
+/* UNUSED: 25 */
+/* UNUSED: 26 */
+/* UNUSED: 27 */
+/* UNUSED: 28 */
+/* UNUSED: 29 */
+/* UNUSED: 30 */
+/* UNUSED: 31 */
+
+/* Reserved: 32-63 */
+
+#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
+
+#define TEGRA30_SWGROUP_ALL	<0x00000000 0x000779ff>
+#define TEGRA114_SWGROUP_ALL	<0x00000000 0x01b659fe>
+
+#endif /* _DT_BINDINGS_MEMORY_TEGRA_SWGROUP_H */
-- 
1.7.9.5

Re: [PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
> binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.
> 
> This will allow the same header to be used by both device tree files,
> and drivers implementing this binding, which guarantees that the two
> stay in sync. This also makes device trees more readable by using names
> instead of magic numbers.

Why does the driver need these constants; shouldn't it simply support
0..n SW group IDs that all work the same way? Or, is there some reason
for it to know the identities?

> diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h

s/memory/iommu/?

s/tegra-swgroup.h/tegra-smmu.h/?

> +#define TEGRA_SWGROUP_AFI 0

Should this file document which values are valid for which SoCs? Should
there be separate files for each SoC; I don't know if it's guaranteed
that IDs won't be re-assigned between different SoCs.

> +#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
> +
> +#define TEGRA30_SWGROUP_ALL	<0x00000000 0x000779ff>
> +#define TEGRA114_SWGROUP_ALL	<0x00000000 0x01b659fe>

Are those 3 values needed by DT files, or just the driver?

[PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
> binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.
> 
> This will allow the same header to be used by both device tree files,
> and drivers implementing this binding, which guarantees that the two
> stay in sync. This also makes device trees more readable by using names
> instead of magic numbers.

Why does the driver need these constants; shouldn't it simply support
0..n SW group IDs that all work the same way? Or, is there some reason
for it to know the identities?

> diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h

s/memory/iommu/?

s/tegra-swgroup.h/tegra-smmu.h/?

> +#define TEGRA_SWGROUP_AFI 0

Should this file document which values are valid for which SoCs? Should
there be separate files for each SoC; I don't know if it's guaranteed
that IDs won't be re-assigned between different SoCs.

> +#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
> +
> +#define TEGRA30_SWGROUP_ALL	<0x00000000 0x000779ff>
> +#define TEGRA114_SWGROUP_ALL	<0x00000000 0x01b659fe>

Are those 3 values needed by DT files, or just the driver?

Re: [PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Hiroshi Doyu]

Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Wed, 17 Jul 2013 01:07:12 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
> > binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.
> > 
> > This will allow the same header to be used by both device tree files,
> > and drivers implementing this binding, which guarantees that the two
> > stay in sync. This also makes device trees more readable by using names
> > instead of magic numbers.
> 
> Why does the driver need these constants; shouldn't it simply support
> 0..n SW group IDs that all work the same way? Or, is there some reason
> for it to know the identities?
> 
> > diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
> 
> s/memory/iommu/?

This info would be used in PMC code as well, which H/W belongs to
which swgroup. So I use the term "memory(-client)" instead of "iommu".

> s/tegra-swgroup.h/tegra-smmu.h/?

Is "tegra-memory-clients.h" preferable, then?

> > +#define TEGRA_SWGROUP_AFI 0
> 
> Should this file document which values are valid for which SoCs? Should
> there be separate files for each SoC; I don't know if it's guaranteed
> that IDs won't be re-assigned between different SoCs.

I've checked all SoC which I can refer. Those IDs are calculated from
the register offset. But I haven't got any explicit guarantee from H/W
designers yet.

> > +#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
> > +
> > +#define TEGRA30_SWGROUP_ALL	<0x00000000 0x000779ff>
> > +#define TEGRA114_SWGROUP_ALL	<0x00000000 0x01b659fe>
> 
> Are those 3 values needed by DT files, or just the driver?

Only from DT.

[PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Wed, 17 Jul 2013 01:07:12 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
> > binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.
> > 
> > This will allow the same header to be used by both device tree files,
> > and drivers implementing this binding, which guarantees that the two
> > stay in sync. This also makes device trees more readable by using names
> > instead of magic numbers.
> 
> Why does the driver need these constants; shouldn't it simply support
> 0..n SW group IDs that all work the same way? Or, is there some reason
> for it to know the identities?
> 
> > diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
> 
> s/memory/iommu/?

This info would be used in PMC code as well, which H/W belongs to
which swgroup. So I use the term "memory(-client)" instead of "iommu".

> s/tegra-swgroup.h/tegra-smmu.h/?

Is "tegra-memory-clients.h" preferable, then?

> > +#define TEGRA_SWGROUP_AFI 0
> 
> Should this file document which values are valid for which SoCs? Should
> there be separate files for each SoC; I don't know if it's guaranteed
> that IDs won't be re-assigned between different SoCs.

I've checked all SoC which I can refer. Those IDs are calculated from
the register offset. But I haven't got any explicit guarantee from H/W
designers yet.

> > +#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
> > +
> > +#define TEGRA30_SWGROUP_ALL	<0x00000000 0x000779ff>
> > +#define TEGRA114_SWGROUP_ALL	<0x00000000 0x01b659fe>
> 
> Are those 3 values needed by DT files, or just the driver?

Only from DT.

Re: [PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Stephen Warren]

On 07/29/2013 04:53 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Wed, 17 Jul 2013 01:07:12 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
>>> binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.
>>>
>>> This will allow the same header to be used by both device tree files,
>>> and drivers implementing this binding, which guarantees that the two
>>> stay in sync. This also makes device trees more readable by using names
>>> instead of magic numbers.
>>
>> Why does the driver need these constants; shouldn't it simply support
>> 0..n SW group IDs that all work the same way? Or, is there some reason
>> for it to know the identities?
>>
>>> diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
>>
>> s/memory/iommu/?
> 
> This info would be used in PMC code as well, which H/W belongs to
> which swgroup. So I use the term "memory(-client)" instead of "iommu".

Why does the PMC care? An explanation of exactly what SW needs to know
about these groups and why would be a good idea.

>> s/tegra-swgroup.h/tegra-smmu.h/?
> 
> Is "tegra-memory-clients.h" preferable, then?

Perhaps, although aren't these IDs specific to the connection between
modules and the SMMU? Or, are these IDs some more global concept; the HW
modules in question pass the same IDs to both the SMMU and to other HW
modules for some reason?

>>> +#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
>>> +
>>> +#define TEGRA30_SWGROUP_ALL	<0x00000000 0x000779ff>
>>> +#define TEGRA114_SWGROUP_ALL	<0x00000000 0x01b659fe>
>>
>> Are those 3 values needed by DT files, or just the driver?
> 
> Only from DT.

If the values are only needed to write the DT content, I'd tend to
prefer not defining them in a header in <dt-bindings/>; I consider that
directory a definition of the constants that define the ABI itself, not
as a random place to put useful things for DT. You can include headers
in the *.dts directory itself.

[PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Stephen Warren]

On 07/29/2013 04:53 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Wed, 17 Jul 2013 01:07:12 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
>>> binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.
>>>
>>> This will allow the same header to be used by both device tree files,
>>> and drivers implementing this binding, which guarantees that the two
>>> stay in sync. This also makes device trees more readable by using names
>>> instead of magic numbers.
>>
>> Why does the driver need these constants; shouldn't it simply support
>> 0..n SW group IDs that all work the same way? Or, is there some reason
>> for it to know the identities?
>>
>>> diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
>>
>> s/memory/iommu/?
> 
> This info would be used in PMC code as well, which H/W belongs to
> which swgroup. So I use the term "memory(-client)" instead of "iommu".

Why does the PMC care? An explanation of exactly what SW needs to know
about these groups and why would be a good idea.

>> s/tegra-swgroup.h/tegra-smmu.h/?
> 
> Is "tegra-memory-clients.h" preferable, then?

Perhaps, although aren't these IDs specific to the connection between
modules and the SMMU? Or, are these IDs some more global concept; the HW
modules in question pass the same IDs to both the SMMU and to other HW
modules for some reason?

>>> +#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
>>> +
>>> +#define TEGRA30_SWGROUP_ALL	<0x00000000 0x000779ff>
>>> +#define TEGRA114_SWGROUP_ALL	<0x00000000 0x01b659fe>
>>
>> Are those 3 values needed by DT files, or just the driver?
> 
> Only from DT.

If the values are only needed to write the DT content, I'd tend to
prefer not defining them in a header in <dt-bindings/>; I consider that
directory a definition of the constants that define the ABI itself, not
as a random place to put useful things for DT. You can include headers
in the *.dts directory itself.

Re: [PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Hiroshi Doyu]

On Mon, 29 Jul 2013 19:45:15 +0200
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:

> On 07/29/2013 04:53 AM, Hiroshi Doyu wrote:
> > Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Wed, 17 Jul 2013 01:07:12 +0200:
> > 
> >> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> >>> Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
> >>> binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.
> >>>
> >>> This will allow the same header to be used by both device tree files,
> >>> and drivers implementing this binding, which guarantees that the two
> >>> stay in sync. This also makes device trees more readable by using names
> >>> instead of magic numbers.
> >>
> >> Why does the driver need these constants; shouldn't it simply support
> >> 0..n SW group IDs that all work the same way? Or, is there some reason
> >> for it to know the identities?
> >>
> >>> diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
> >>
> >> s/memory/iommu/?
> > 
> > This info would be used in PMC code as well, which H/W belongs to
> > which swgroup. So I use the term "memory(-client)" instead of "iommu".
> 
> Why does the PMC care? An explanation of exactly what SW needs to know
> about these groups and why would be a good idea.

Joseph, can you explain a bit here?

[PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

[Hiroshi Doyu]

On Mon, 29 Jul 2013 19:45:15 +0200
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 07/29/2013 04:53 AM, Hiroshi Doyu wrote:
> > Stephen Warren <swarren@wwwdotorg.org> wrote @ Wed, 17 Jul 2013 01:07:12 +0200:
> > 
> >> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> >>> Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
> >>> binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.
> >>>
> >>> This will allow the same header to be used by both device tree files,
> >>> and drivers implementing this binding, which guarantees that the two
> >>> stay in sync. This also makes device trees more readable by using names
> >>> instead of magic numbers.
> >>
> >> Why does the driver need these constants; shouldn't it simply support
> >> 0..n SW group IDs that all work the same way? Or, is there some reason
> >> for it to know the identities?
> >>
> >>> diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
> >>
> >> s/memory/iommu/?
> > 
> > This info would be used in PMC code as well, which H/W belongs to
> > which swgroup. So I use the term "memory(-client)" instead of "iommu".
> 
> Why does the PMC care? An explanation of exactly what SW needs to know
> about these groups and why would be a good idea.

Joseph, can you explain a bit here?

[PATCH v2 04/22] ARM: dt: tegra30: iommu: Add "nvidia,swgroup"

[Hiroshi Doyu]

This is a bitmap that indicates which HardWare Accelerators(HWA) are
supported on Tegra30 SoC.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/tegra30.dtsi |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index d8783f0..4a9594e 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -1,6 +1,7 @@
 #include <dt-bindings/clock/tegra30-car.h>
 #include <dt-bindings/gpio/tegra-gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/memory/tegra-swgroup.h>
 
 #include "skeleton.dtsi"
 
@@ -461,6 +462,7 @@
 		       0x7000f228 0x05c>;
 		nvidia,#asids = <4>;		/* # of ASIDs */
 		dma-window = <0 0x40000000>;	/* IOVA start & length */
+		nvidia,swgroup = TEGRA30_SWGROUP_ALL;
 		nvidia,ahb = <&ahb>;
 	};
 
-- 
1.7.9.5

[PATCH v2 04/22] ARM: dt: tegra30: iommu: Add "nvidia,swgroup"

[Hiroshi Doyu]

This is a bitmap that indicates which HardWare Accelerators(HWA) are
supported on Tegra30 SoC.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra30.dtsi |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index d8783f0..4a9594e 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -1,6 +1,7 @@
 #include <dt-bindings/clock/tegra30-car.h>
 #include <dt-bindings/gpio/tegra-gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/memory/tegra-swgroup.h>
 
 #include "skeleton.dtsi"
 
@@ -461,6 +462,7 @@
 		       0x7000f228 0x05c>;
 		nvidia,#asids = <4>;		/* # of ASIDs */
 		dma-window = <0 0x40000000>;	/* IOVA start & length */
+		nvidia,swgroup = TEGRA30_SWGROUP_ALL;
 		nvidia,ahb = <&ahb>;
 	};
 
-- 
1.7.9.5

Re: [PATCH v2 04/22] ARM: dt: tegra30: iommu: Add "nvidia,swgroup"

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> This is a bitmap that indicates which HardWare Accelerators(HWA) are
> supported on Tegra30 SoC.

> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi

> +		nvidia,swgroup = TEGRA30_SWGROUP_ALL;

That property isn't in the binding document. We can't add properties
that aren't documented.

Macros should not include structural syntax; the < > around the cell
value is part of the macro the way you've written it, but shouldn't be.
It'd be simpler to just write:

	nvidia,swgroup = <0x00000000 0x000779ff>;

We should only extend the binding in a backwards-compatible fashion, so
we can't require that DTs contain this value. At least, there should be
a default value defined for when the property doesn't exist.

[PATCH v2 04/22] ARM: dt: tegra30: iommu: Add "nvidia,swgroup"

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> This is a bitmap that indicates which HardWare Accelerators(HWA) are
> supported on Tegra30 SoC.

> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi

> +		nvidia,swgroup = TEGRA30_SWGROUP_ALL;

That property isn't in the binding document. We can't add properties
that aren't documented.

Macros should not include structural syntax; the < > around the cell
value is part of the macro the way you've written it, but shouldn't be.
It'd be simpler to just write:

	nvidia,swgroup = <0x00000000 0x000779ff>;

We should only extend the binding in a backwards-compatible fashion, so
we can't require that DTs contain this value. At least, there should be
a default value defined for when the property doesn't exist.

[PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia,memory-client"

[Hiroshi Doyu]

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

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/tegra30.dtsi |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 4a9594e..4aa5b4a 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -23,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-client = <TEGRA_SWGROUP_HC>;
 
 		#address-cells = <1>;
 		#size-cells = <1>;
@@ -34,6 +35,7 @@
 			reg = <0x54040000 0x00040000>;
 			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_MPE>;
+			nvidia,memory-client = <TEGRA_SWGROUP_MPE>;
 		};
 
 		vi {
@@ -41,6 +43,7 @@
 			reg = <0x54080000 0x00040000>;
 			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_VI>;
+			nvidia,memory-client = <TEGRA_SWGROUP_VI>;
 		};
 
 		epp {
@@ -48,6 +51,7 @@
 			reg = <0x540c0000 0x00040000>;
 			interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_EPP>;
+			nvidia,memory-client = <TEGRA_SWGROUP_EPP>;
 		};
 
 		isp {
@@ -55,6 +59,7 @@
 			reg = <0x54100000 0x00040000>;
 			interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_ISP>;
+			nvidia,memory-client = <TEGRA_SWGROUP_ISP>;
 		};
 
 		gr2d {
@@ -62,6 +67,7 @@
 			reg = <0x54140000 0x00040000>;
 			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_GR2D>;
+			nvidia,memory-client = <TEGRA_SWGROUP_G2>;
 		};
 
 		gr3d {
@@ -69,6 +75,8 @@
 			reg = <0x54180000 0x00040000>;
 			clocks = <&tegra_car 24 &tegra_car 98>;
 			clock-names = "3d", "3d2";
+			nvidia,memory-client = <TEGRA_SWGROUP_NV
+						TEGRA_SWGROUP_NV2>;
 		};
 
 		dc@54200000 {
@@ -78,6 +86,7 @@
 			clocks = <&tegra_car TEGRA30_CLK_DISP1>,
 				 <&tegra_car TEGRA30_CLK_PLL_P>;
 			clock-names = "disp1", "parent";
+			nvidia,memory-client = <TEGRA_SWGROUP_DC>;
 
 			rgb {
 				status = "disabled";
@@ -91,6 +100,7 @@
 			clocks = <&tegra_car TEGRA30_CLK_DISP2>,
 				 <&tegra_car TEGRA30_CLK_PLL_P>;
 			clock-names = "disp2", "parent";
+			nvidia,memory-client = <TEGRA_SWGROUP_DCB>;
 
 			rgb {
 				status = "disabled";
@@ -247,6 +257,7 @@
 		interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 8>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTA>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -257,6 +268,7 @@
 		interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 9>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTB>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -267,6 +279,7 @@
 		interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 10>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTC>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -277,6 +290,7 @@
 		interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 19>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTD>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -287,6 +301,7 @@
 		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 20>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTE>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -536,6 +551,7 @@
 		reg = <0x78000000 0x200>;
 		interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC1>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -544,6 +560,7 @@
 		reg = <0x78000200 0x200>;
 		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC2>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -552,6 +569,7 @@
 		reg = <0x78000400 0x200>;
 		interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC3>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -560,6 +578,7 @@
 		reg = <0x78000600 0x200>;
 		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC4>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
-- 
1.7.9.5

[PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia,memory-client"

[Hiroshi Doyu]

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

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra30.dtsi |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 4a9594e..4aa5b4a 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -23,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-client = <TEGRA_SWGROUP_HC>;
 
 		#address-cells = <1>;
 		#size-cells = <1>;
@@ -34,6 +35,7 @@
 			reg = <0x54040000 0x00040000>;
 			interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_MPE>;
+			nvidia,memory-client = <TEGRA_SWGROUP_MPE>;
 		};
 
 		vi {
@@ -41,6 +43,7 @@
 			reg = <0x54080000 0x00040000>;
 			interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_VI>;
+			nvidia,memory-client = <TEGRA_SWGROUP_VI>;
 		};
 
 		epp {
@@ -48,6 +51,7 @@
 			reg = <0x540c0000 0x00040000>;
 			interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_EPP>;
+			nvidia,memory-client = <TEGRA_SWGROUP_EPP>;
 		};
 
 		isp {
@@ -55,6 +59,7 @@
 			reg = <0x54100000 0x00040000>;
 			interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_ISP>;
+			nvidia,memory-client = <TEGRA_SWGROUP_ISP>;
 		};
 
 		gr2d {
@@ -62,6 +67,7 @@
 			reg = <0x54140000 0x00040000>;
 			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA30_CLK_GR2D>;
+			nvidia,memory-client = <TEGRA_SWGROUP_G2>;
 		};
 
 		gr3d {
@@ -69,6 +75,8 @@
 			reg = <0x54180000 0x00040000>;
 			clocks = <&tegra_car 24 &tegra_car 98>;
 			clock-names = "3d", "3d2";
+			nvidia,memory-client = <TEGRA_SWGROUP_NV
+						TEGRA_SWGROUP_NV2>;
 		};
 
 		dc at 54200000 {
@@ -78,6 +86,7 @@
 			clocks = <&tegra_car TEGRA30_CLK_DISP1>,
 				 <&tegra_car TEGRA30_CLK_PLL_P>;
 			clock-names = "disp1", "parent";
+			nvidia,memory-client = <TEGRA_SWGROUP_DC>;
 
 			rgb {
 				status = "disabled";
@@ -91,6 +100,7 @@
 			clocks = <&tegra_car TEGRA30_CLK_DISP2>,
 				 <&tegra_car TEGRA30_CLK_PLL_P>;
 			clock-names = "disp2", "parent";
+			nvidia,memory-client = <TEGRA_SWGROUP_DCB>;
 
 			rgb {
 				status = "disabled";
@@ -247,6 +257,7 @@
 		interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 8>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTA>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -257,6 +268,7 @@
 		interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 9>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTB>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -267,6 +279,7 @@
 		interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 10>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTC>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -277,6 +290,7 @@
 		interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 19>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTD>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -287,6 +301,7 @@
 		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
 		nvidia,dma-request-selector = <&apbdma 20>;
 		clocks = <&tegra_car TEGRA30_CLK_UARTE>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -536,6 +551,7 @@
 		reg = <0x78000000 0x200>;
 		interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC1>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -544,6 +560,7 @@
 		reg = <0x78000200 0x200>;
 		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC2>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -552,6 +569,7 @@
 		reg = <0x78000400 0x200>;
 		interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC3>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
@@ -560,6 +578,7 @@
 		reg = <0x78000600 0x200>;
 		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_SDMMC4>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disabled";
 	};
 
-- 
1.7.9.5

Re: [PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia, memory-client"

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Add "nvidia,memory-client" to identify which swgroup ID a device
> belongs to.

This property also isn't in the documentation.

Again, if client devices are required to have this new property, the DT
isn't backwards-compatible; fail:-(

> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi

> +		nvidia,memory-client = <TEGRA_SWGROUP_HC>;

What if there's more than one SMMU in the system, e.g. since there are
multiple different master-capable buses? Including the SMMU phandle in
the property seems like a good idea so that the SMMU driver knows which
devices are affected by it (although a bus structure with affected
devices under the SMMU represented as nodes under the SMMU DT node seems
a slightly more direct way of representing that, although that solution
does only support systems where bus-master transactions flow up the bus
structure in the same way that CPU transactions flow down it).

What about:

nvidia,smmu = <&smmu TEGRA_SWGROUP_HC>;

?

> @@ -69,6 +75,8 @@
>  			reg = <0x54180000 0x00040000>;
>  			clocks = <&tegra_car 24 &tegra_car 98>;
>  			clock-names = "3d", "3d2";
> +			nvidia,memory-client = <TEGRA_SWGROUP_NV
> +						TEGRA_SWGROUP_NV2>;

Oh, if there can be more than one, I suggest adding "s" to the end of
the property name.

[PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia, memory-client"

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Add "nvidia,memory-client" to identify which swgroup ID a device
> belongs to.

This property also isn't in the documentation.

Again, if client devices are required to have this new property, the DT
isn't backwards-compatible; fail:-(

> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi

> +		nvidia,memory-client = <TEGRA_SWGROUP_HC>;

What if there's more than one SMMU in the system, e.g. since there are
multiple different master-capable buses? Including the SMMU phandle in
the property seems like a good idea so that the SMMU driver knows which
devices are affected by it (although a bus structure with affected
devices under the SMMU represented as nodes under the SMMU DT node seems
a slightly more direct way of representing that, although that solution
does only support systems where bus-master transactions flow up the bus
structure in the same way that CPU transactions flow down it).

What about:

nvidia,smmu = <&smmu TEGRA_SWGROUP_HC>;

?

> @@ -69,6 +75,8 @@
>  			reg = <0x54180000 0x00040000>;
>  			clocks = <&tegra_car 24 &tegra_car 98>;
>  			clock-names = "3d", "3d2";
> +			nvidia,memory-client = <TEGRA_SWGROUP_NV
> +						TEGRA_SWGROUP_NV2>;

Oh, if there can be more than one, I suggest adding "s" to the end of
the property name.

Re: [PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia,memory-client"

[Hiroshi Doyu]

Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Wed, 17 Jul 2013 01:14:33 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Add "nvidia,memory-client" to identify which swgroup ID a device
> > belongs to.
> 
> This property also isn't in the documentation.
> 
> Again, if client devices are required to have this new property, the DT
> isn't backwards-compatible; fail:-(

If there's no this property, just falling back to use the system
default address space. I think that this won't break the
compatibility basically. Only problem may be the inconsistency of
setting this property among the HWs in the same group. If one of them
is set in one AS, but not for the other, then the other H/W won't
work. If we really want to have this kind of constraint implmeneted in
DT, some kind of bus model is needed.

[PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia,memory-client"

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Wed, 17 Jul 2013 01:14:33 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Add "nvidia,memory-client" to identify which swgroup ID a device
> > belongs to.
> 
> This property also isn't in the documentation.
> 
> Again, if client devices are required to have this new property, the DT
> isn't backwards-compatible; fail:-(

If there's no this property, just falling back to use the system
default address space. I think that this won't break the
compatibility basically. Only problem may be the inconsistency of
setting this property among the HWs in the same group. If one of them
is set in one AS, but not for the other, then the other H/W won't
work. If we really want to have this kind of constraint implmeneted in
DT, some kind of bus model is needed.

Re: [PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia,memory-client"

[Stephen Warren]

On 07/29/2013 05:06 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Wed, 17 Jul 2013 01:14:33 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Add "nvidia,memory-client" to identify which swgroup ID a device
>>> belongs to.
>>
>> This property also isn't in the documentation.
>>
>> Again, if client devices are required to have this new property, the DT
>> isn't backwards-compatible; fail:-(
> 
> If there's no this property, just falling back to use the system
> default address space. I think that this won't break the
> compatibility basically. Only problem may be the inconsistency of
> setting this property among the HWs in the same group.

When you say "group" there, what kind of group? I thought that each HW
module had its own "SWGROUP ID", and hence there are no "HWs in the same
group"?

[PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia, memory-client"

[Stephen Warren]

On 07/29/2013 05:06 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Wed, 17 Jul 2013 01:14:33 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Add "nvidia,memory-client" to identify which swgroup ID a device
>>> belongs to.
>>
>> This property also isn't in the documentation.
>>
>> Again, if client devices are required to have this new property, the DT
>> isn't backwards-compatible; fail:-(
> 
> If there's no this property, just falling back to use the system
> default address space. I think that this won't break the
> compatibility basically. Only problem may be the inconsistency of
> setting this property among the HWs in the same group.

When you say "group" there, what kind of group? I thought that each HW
module had its own "SWGROUP ID", and hence there are no "HWs in the same
group"?

Re: [PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia,memory-client"

[Hiroshi Doyu]

On Mon, 29 Jul 2013 19:47:04 +0200
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:

> On 07/29/2013 05:06 AM, Hiroshi Doyu wrote:
> > Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Wed, 17 Jul 2013 01:14:33 +0200:
> > 
> >> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> >>> Add "nvidia,memory-client" to identify which swgroup ID a device
> >>> belongs to.
> >>
> >> This property also isn't in the documentation.
> >>
> >> Again, if client devices are required to have this new property, the DT
> >> isn't backwards-compatible; fail:-(
> > 
> > If there's no this property, just falling back to use the system
> > default address space. I think that this won't break the
> > compatibility basically. Only problem may be the inconsistency of
> > setting this property among the HWs in the same group.
> 
> When you say "group" there, what kind of group? I thought that each HW
> module had its own "SWGROUP ID", and hence there are no "HWs in the same
> group"?

A "swgroup" can have multiple HWs. So there are 3 concept here.

swgroup: one client to issue request from MC perspective
HW: swgroup can have multiple HWs(real H/W units)
device driver: Can have multiple swgroups.

[PATCH v2 05/22] ARM: dt: tegra30: iommu: Add "nvidia,memory-client"

[Hiroshi Doyu]

On Mon, 29 Jul 2013 19:47:04 +0200
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 07/29/2013 05:06 AM, Hiroshi Doyu wrote:
> > Stephen Warren <swarren@wwwdotorg.org> wrote @ Wed, 17 Jul 2013 01:14:33 +0200:
> > 
> >> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> >>> Add "nvidia,memory-client" to identify which swgroup ID a device
> >>> belongs to.
> >>
> >> This property also isn't in the documentation.
> >>
> >> Again, if client devices are required to have this new property, the DT
> >> isn't backwards-compatible; fail:-(
> > 
> > If there's no this property, just falling back to use the system
> > default address space. I think that this won't break the
> > compatibility basically. Only problem may be the inconsistency of
> > setting this property among the HWs in the same group.
> 
> When you say "group" there, what kind of group? I thought that each HW
> module had its own "SWGROUP ID", and hence there are no "HWs in the same
> group"?

A "swgroup" can have multiple HWs. So there are 3 concept here.

swgroup: one client to issue request from MC perspective
HW: swgroup can have multiple HWs(real H/W units)
device driver: Can have multiple swgroups.

[PATCH v2 06/22] ARM: dt: tegra114: iommu: Fix IOMMU register address

[Hiroshi Doyu]

Fix IOMMU register address.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/tegra114.dtsi |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index abf6c40..1f45cf5 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -318,9 +318,9 @@
 
 	iommu {
 		compatible = "nvidia,tegra114-smmu", "nvidia,tegra30-smmu";
-		reg = <0x7000f010 0x02c
-		       0x7000f1f0 0x010
-		       0x7000f228 0x074>;
+		reg = <0x70019010 0x02c
+		       0x700191f0 0x010
+		       0x70019228 0x074>;
 		nvidia,#asids = <4>;
 		dma-window = <0 0x40000000>;
 		nvidia,swgroups = <0x18659fe>;
-- 
1.7.9.5

[PATCH v2 06/22] ARM: dt: tegra114: iommu: Fix IOMMU register address

[Hiroshi Doyu]

Fix IOMMU register address.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra114.dtsi |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index abf6c40..1f45cf5 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -318,9 +318,9 @@
 
 	iommu {
 		compatible = "nvidia,tegra114-smmu", "nvidia,tegra30-smmu";
-		reg = <0x7000f010 0x02c
-		       0x7000f1f0 0x010
-		       0x7000f228 0x074>;
+		reg = <0x70019010 0x02c
+		       0x700191f0 0x010
+		       0x70019228 0x074>;
 		nvidia,#asids = <4>;
 		dma-window = <0 0x40000000>;
 		nvidia,swgroups = <0x18659fe>;
-- 
1.7.9.5

Re: [PATCH v2 06/22] ARM: dt: tegra114: iommu: Fix IOMMU register address

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Fix IOMMU register address.

Oh dear, how serious is this?

That incorrect node got added in v3.9 - a long time ago. Does the SMMU
driver touch registers during probe() or under any other condition right
now? If so, can that hang the CPU or end up trashing some other HW
module's registers?

I'm trying to determine whether we need to urgently send this patch as a
fix Cc: stable.

[PATCH v2 06/22] ARM: dt: tegra114: iommu: Fix IOMMU register address

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Fix IOMMU register address.

Oh dear, how serious is this?

That incorrect node got added in v3.9 - a long time ago. Does the SMMU
driver touch registers during probe() or under any other condition right
now? If so, can that hang the CPU or end up trashing some other HW
module's registers?

I'm trying to determine whether we need to urgently send this patch as a
fix Cc: stable.

Re: [PATCH v2 06/22] ARM: dt: tegra114: iommu: Fix IOMMU register address

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 649 bytes --]

On Tue, Jul 16, 2013 at 05:18:29PM -0600, Stephen Warren wrote:
> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Fix IOMMU register address.
> 
> Oh dear, how serious is this?
> 
> That incorrect node got added in v3.9 - a long time ago. Does the SMMU
> driver touch registers during probe() or under any other condition right
> now? If so, can that hang the CPU or end up trashing some other HW
> module's registers?
> 
> I'm trying to determine whether we need to urgently send this patch as a
> fix Cc: stable.

Judging by the TRM there's nothing in the range from 0x7000ec00 to
0x7000f7ff, so hopefully we got lucky.

Thierry

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v2 06/22] ARM: dt: tegra114: iommu: Fix IOMMU register address

[Thierry Reding]

On Tue, Jul 16, 2013 at 05:18:29PM -0600, Stephen Warren wrote:
> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Fix IOMMU register address.
> 
> Oh dear, how serious is this?
> 
> That incorrect node got added in v3.9 - a long time ago. Does the SMMU
> driver touch registers during probe() or under any other condition right
> now? If so, can that hang the CPU or end up trashing some other HW
> module's registers?
> 
> I'm trying to determine whether we need to urgently send this patch as a
> fix Cc: stable.

Judging by the TRM there's nothing in the range from 0x7000ec00 to
0x7000f7ff, so hopefully we got lucky.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130716/346a833a/attachment.sig>

[PATCH v2 07/22] ARM: dt: tegra114: iommu: Add "nvidia,swgroups"

[Hiroshi Doyu]

This is a bitmap that indicates which HardWare Accelerators(HWA) are
supported on Tegra114 SoC.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/tegra114.dtsi |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index 1f45cf5..9e231eb 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -1,6 +1,7 @@
 #include <dt-bindings/clock/tegra114-car.h>
 #include <dt-bindings/gpio/tegra-gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/memory/tegra-swgroup.h>
 
 #include "skeleton.dtsi"
 
@@ -323,7 +324,7 @@
 		       0x70019228 0x074>;
 		nvidia,#asids = <4>;
 		dma-window = <0 0x40000000>;
-		nvidia,swgroups = <0x18659fe>;
+		nvidia,swgroups = TEGRA114_SWGROUP_ALL;
 		nvidia,ahb = <&ahb>;
 	};
 
-- 
1.7.9.5

[PATCH v2 07/22] ARM: dt: tegra114: iommu: Add "nvidia,swgroups"

[Hiroshi Doyu]

This is a bitmap that indicates which HardWare Accelerators(HWA) are
supported on Tegra114 SoC.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra114.dtsi |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index 1f45cf5..9e231eb 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -1,6 +1,7 @@
 #include <dt-bindings/clock/tegra114-car.h>
 #include <dt-bindings/gpio/tegra-gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/memory/tegra-swgroup.h>
 
 #include "skeleton.dtsi"
 
@@ -323,7 +324,7 @@
 		       0x70019228 0x074>;
 		nvidia,#asids = <4>;
 		dma-window = <0 0x40000000>;
-		nvidia,swgroups = <0x18659fe>;
+		nvidia,swgroups = TEGRA114_SWGROUP_ALL;
 		nvidia,ahb = <&ahb>;
 	};
 
-- 
1.7.9.5

[PATCH v2 08/22] ARM: dt: tegra114: Add "nvidia,memory-client"

[Hiroshi Doyu]

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

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/tegra114.dtsi |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index 9e231eb..df6767f 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -404,6 +404,7 @@
 		reg = <0x78000000 0x200>;
 		interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA114_CLK_SDMMC1>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disable";
 	};
 
@@ -412,6 +413,7 @@
 		reg = <0x78000200 0x200>;
 		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA114_CLK_SDMMC2>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disable";
 	};
 
@@ -420,6 +422,7 @@
 		reg = <0x78000400 0x200>;
 		interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA114_CLK_SDMMC3>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disable";
 	};
 
@@ -428,6 +431,7 @@
 		reg = <0x78000600 0x200>;
 		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA114_CLK_SDMMC4>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disable";
 	};
 
-- 
1.7.9.5

[PATCH v2 08/22] ARM: dt: tegra114: Add "nvidia,memory-client"

[Hiroshi Doyu]

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

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra114.dtsi |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index 9e231eb..df6767f 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -404,6 +404,7 @@
 		reg = <0x78000000 0x200>;
 		interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA114_CLK_SDMMC1>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disable";
 	};
 
@@ -412,6 +413,7 @@
 		reg = <0x78000200 0x200>;
 		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA114_CLK_SDMMC2>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disable";
 	};
 
@@ -420,6 +422,7 @@
 		reg = <0x78000400 0x200>;
 		interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA114_CLK_SDMMC3>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disable";
 	};
 
@@ -428,6 +431,7 @@
 		reg = <0x78000600 0x200>;
 		interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA114_CLK_SDMMC4>;
+		nvidia,memory-client = <TEGRA_SWGROUP_PPCS>;
 		status = "disable";
 	};
 
-- 
1.7.9.5

[PATCH v2 09/22] amba: Move AHB to core_initcall

[Hiroshi Doyu]

Move AHB to core_initcall in order to instanciate this device earlier
than others since IOMMU depends on AHB and then IOMMU driver needs to
reigster other platform devices as IOMMU'able.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/amba/tegra-ahb.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c
index 1f44e56..08e898d 100644
--- a/drivers/amba/tegra-ahb.c
+++ b/drivers/amba/tegra-ahb.c
@@ -283,7 +283,12 @@ static struct platform_driver tegra_ahb_driver = {
 		.pm = &tegra_ahb_pm,
 	},
 };
-module_platform_driver(tegra_ahb_driver);
+
+static int tegra_ahb_init(void)
+{
+	return platform_driver_register(&tegra_ahb_driver);
+}
+core_initcall(tegra_ahb_init);
 
 MODULE_AUTHOR("Hiroshi DOYU <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>");
 MODULE_DESCRIPTION("Tegra AHB driver");
-- 
1.7.9.5

[PATCH v2 09/22] amba: Move AHB to core_initcall

[Hiroshi Doyu]

Move AHB to core_initcall in order to instanciate this device earlier
than others since IOMMU depends on AHB and then IOMMU driver needs to
reigster other platform devices as IOMMU'able.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/amba/tegra-ahb.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c
index 1f44e56..08e898d 100644
--- a/drivers/amba/tegra-ahb.c
+++ b/drivers/amba/tegra-ahb.c
@@ -283,7 +283,12 @@ static struct platform_driver tegra_ahb_driver = {
 		.pm = &tegra_ahb_pm,
 	},
 };
-module_platform_driver(tegra_ahb_driver);
+
+static int tegra_ahb_init(void)
+{
+	return platform_driver_register(&tegra_ahb_driver);
+}
+core_initcall(tegra_ahb_init);
 
 MODULE_AUTHOR("Hiroshi DOYU <hdoyu@nvidia.com>");
 MODULE_DESCRIPTION("Tegra AHB driver");
-- 
1.7.9.5

Re: [PATCH v2 09/22] amba: Move AHB to core_initcall

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Move AHB to core_initcall in order to instanciate this device earlier
> than others since IOMMU depends on AHB and then IOMMU driver needs to
> reigster other platform devices as IOMMU'able.

Oh dear, this really is looking like we shouldn't be playing games by
attempting to force the initialization order. An explicit bus structure
and/or having client devices go and attach to the SMMU, would mean we
didn't have to play games like this:-(

[PATCH v2 09/22] amba: Move AHB to core_initcall

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Move AHB to core_initcall in order to instanciate this device earlier
> than others since IOMMU depends on AHB and then IOMMU driver needs to
> reigster other platform devices as IOMMU'able.

Oh dear, this really is looking like we shouldn't be playing games by
attempting to force the initialization order. An explicit bus structure
and/or having client devices go and attach to the SMMU, would mean we
didn't have to play games like this:-(

[PATCH v2 10/22] iommu/tegra: smmu: Move IOMMU to core_initcall

[Hiroshi Doyu]

Move IOMMU to core_initcall in order to instanciate this device earlier
than others since IOMMU driver needs to reigster other platform
devices as IOMMU'able.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index f6f120e..4c16c90 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -1288,7 +1288,7 @@ static void __exit tegra_smmu_exit(void)
 	platform_driver_unregister(&tegra_smmu_driver);
 }
 
-subsys_initcall(tegra_smmu_init);
+core_initcall(tegra_smmu_init);
 module_exit(tegra_smmu_exit);
 
 MODULE_DESCRIPTION("IOMMU API for SMMU in Tegra30");
-- 
1.7.9.5

[PATCH v2 10/22] iommu/tegra: smmu: Move IOMMU to core_initcall

[Hiroshi Doyu]

Move IOMMU to core_initcall in order to instanciate this device earlier
than others since IOMMU driver needs to reigster other platform
devices as IOMMU'able.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index f6f120e..4c16c90 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -1288,7 +1288,7 @@ static void __exit tegra_smmu_exit(void)
 	platform_driver_unregister(&tegra_smmu_driver);
 }
 
-subsys_initcall(tegra_smmu_init);
+core_initcall(tegra_smmu_init);
 module_exit(tegra_smmu_exit);
 
 MODULE_DESCRIPTION("IOMMU API for SMMU in Tegra30");
-- 
1.7.9.5

Re: [PATCH v2 10/22] iommu/tegra: smmu: Move IOMMU to core_initcall

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Move IOMMU to core_initcall in order to instanciate this device earlier
> than others since IOMMU driver needs to reigster other platform
> devices as IOMMU'able.

If both the AHB and SMMU driver are core_initcall, there's no guarantee
at all re: which order they'll get probed in. Deferred probe won't help
here, since I don't think it's guaranteed to be repeated, so if the
probe order is:

1) SMMU (fails since AHB isn't available)
2) AHB

... then the SMMU might not get probed until /much/ later; probably
after devices that are affected by the SMMU are probed.

However, it all works out if the probe order just accidentally happens
to be:

1) AHB
2) SMMU.

So, this really doesn't look like a viable solution.

[PATCH v2 10/22] iommu/tegra: smmu: Move IOMMU to core_initcall

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Move IOMMU to core_initcall in order to instanciate this device earlier
> than others since IOMMU driver needs to reigster other platform
> devices as IOMMU'able.

If both the AHB and SMMU driver are core_initcall, there's no guarantee
at all re: which order they'll get probed in. Deferred probe won't help
here, since I don't think it's guaranteed to be repeated, so if the
probe order is:

1) SMMU (fails since AHB isn't available)
2) AHB

... then the SMMU might not get probed until /much/ later; probably
after devices that are affected by the SMMU are probed.

However, it all works out if the probe order just accidentally happens
to be:

1) AHB
2) SMMU.

So, this really doesn't look like a viable solution.

[PATCH v2 11/22] iommu/tegra: smmu: Add Tegra 114 support

[Hiroshi Doyu]

Tegra 114 SMMU uses the almost same logic as one in Tegra 30 except
that they have different HaredWare Accelerators(HWA). Those difference
is provided from DT.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 .../bindings/iommu/nvidia,tegra30-smmu.txt         |    2 +-
 drivers/iommu/tegra-smmu.c                         |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
index 89fb543..ce5c43e 100644
--- a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
@@ -1,7 +1,7 @@
 NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit)
 
 Required properties:
-- compatible : "nvidia,tegra30-smmu"
+- compatible : "nvidia,tegra114-smmu", "nvidia,tegra30-smmu"
 - reg : Should contain 3 register banks(address and length) for each
   of the SMMU register blocks.
 - interrupts : Should contain MC General interrupt.
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 4c16c90..3e03c69 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -1262,6 +1262,7 @@ const struct dev_pm_ops tegra_smmu_pm_ops = {
 };
 
 static struct of_device_id tegra_smmu_of_match[] = {
+	{ .compatible = "nvidia,tegra114-smmu", },
 	{ .compatible = "nvidia,tegra30-smmu", },
 	{ },
 };
-- 
1.7.9.5

[PATCH v2 11/22] iommu/tegra: smmu: Add Tegra 114 support

[Hiroshi Doyu]

Tegra 114 SMMU uses the almost same logic as one in Tegra 30 except
that they have different HaredWare Accelerators(HWA). Those difference
is provided from DT.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 .../bindings/iommu/nvidia,tegra30-smmu.txt         |    2 +-
 drivers/iommu/tegra-smmu.c                         |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
index 89fb543..ce5c43e 100644
--- a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
@@ -1,7 +1,7 @@
 NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit)
 
 Required properties:
-- compatible : "nvidia,tegra30-smmu"
+- compatible : "nvidia,tegra114-smmu", "nvidia,tegra30-smmu"
 - reg : Should contain 3 register banks(address and length) for each
   of the SMMU register blocks.
 - interrupts : Should contain MC General interrupt.
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 4c16c90..3e03c69 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -1262,6 +1262,7 @@ const struct dev_pm_ops tegra_smmu_pm_ops = {
 };
 
 static struct of_device_id tegra_smmu_of_match[] = {
+	{ .compatible = "nvidia,tegra114-smmu", },
 	{ .compatible = "nvidia,tegra30-smmu", },
 	{ },
 };
-- 
1.7.9.5

Re: [PATCH v2 11/22] iommu/tegra: smmu: Add Tegra 114 support

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Tegra 114 SMMU uses the almost same logic as one in Tegra 30 except
> that they have different HaredWare Accelerators(HWA). Those difference
> is provided from DT.

> diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt

>  Required properties:
> -- compatible : "nvidia,tegra30-smmu"
> +- compatible : "nvidia,tegra114-smmu", "nvidia,tegra30-smmu"

So that we don't have to list every single chip in that list, we've
often written that as:

compatible : "nvidia,tegra<chip>-smmu".

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  static struct of_device_id tegra_smmu_of_match[] = {
> +	{ .compatible = "nvidia,tegra114-smmu", },
>  	{ .compatible = "nvidia,tegra30-smmu", },

So, there are no HW differences, and hence no driver changes required?
In that case, there's no point making the driver explicitly support
nvidia,tegra114-smmu, since the DT will always be:

compatible = "nvidia,tegra114-smmu", "nvidia,tegra30-smmu";

... and hence always match on the existing nvidia,tegra30-smmu entry in
that table.

[PATCH v2 11/22] iommu/tegra: smmu: Add Tegra 114 support

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Tegra 114 SMMU uses the almost same logic as one in Tegra 30 except
> that they have different HaredWare Accelerators(HWA). Those difference
> is provided from DT.

> diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt

>  Required properties:
> -- compatible : "nvidia,tegra30-smmu"
> +- compatible : "nvidia,tegra114-smmu", "nvidia,tegra30-smmu"

So that we don't have to list every single chip in that list, we've
often written that as:

compatible : "nvidia,tegra<chip>-smmu".

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  static struct of_device_id tegra_smmu_of_match[] = {
> +	{ .compatible = "nvidia,tegra114-smmu", },
>  	{ .compatible = "nvidia,tegra30-smmu", },

So, there are no HW differences, and hence no driver changes required?
In that case, there's no point making the driver explicitly support
nvidia,tegra114-smmu, since the DT will always be:

compatible = "nvidia,tegra114-smmu", "nvidia,tegra30-smmu";

... and hence always match on the existing nvidia,tegra30-smmu entry in
that table.

[PATCH v2 12/22] iommu/tegra: smmu: Select ARM_DMA_USE_IOMMU in Kconfig

[Hiroshi Doyu]

SMMU expects ARM_DMA_USE_IOMMU to be enabled to use DMA mapping API by
default.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/Kconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 820d85c..cd9318b 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -160,6 +160,7 @@ config TEGRA_IOMMU_SMMU
 	bool "Tegra SMMU IOMMU Support"
 	depends on ARCH_TEGRA && TEGRA_AHB
 	select IOMMU_API
+	select ARM_DMA_USE_IOMMU
 	help
 	  Enables support for remapping discontiguous physical memory
 	  shared with the operating system into contiguous I/O virtual
-- 
1.7.9.5

[PATCH v2 12/22] iommu/tegra: smmu: Select ARM_DMA_USE_IOMMU in Kconfig

[Hiroshi Doyu]

SMMU expects ARM_DMA_USE_IOMMU to be enabled to use DMA mapping API by
default.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/Kconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 820d85c..cd9318b 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -160,6 +160,7 @@ config TEGRA_IOMMU_SMMU
 	bool "Tegra SMMU IOMMU Support"
 	depends on ARCH_TEGRA && TEGRA_AHB
 	select IOMMU_API
+	select ARM_DMA_USE_IOMMU
 	help
 	  Enables support for remapping discontiguous physical memory
 	  shared with the operating system into contiguous I/O virtual
-- 
1.7.9.5

[PATCH v2 13/22] iommu/tegra: smmu: Create default IOVA maps

[Hiroshi Doyu]

Create default IOVA maps at boot-up which can be attached to devices.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c |   28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 3e03c69..1617c90 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -319,6 +319,8 @@ struct smmu_device {
 
 	struct device_node *ahb;
 
+	struct dma_iommu_mapping **map;
+
 	int		num_as;
 	struct smmu_as	as[0];		/* Run-time allocated array */
 };
@@ -1144,6 +1146,26 @@ static int tegra_smmu_resume(struct device *dev)
 	return err;
 }
 
+static int tegra_smmu_create_default_map(struct smmu_device *smmu)
+{
+	int i;
+
+	for (i = 0; i < smmu->num_as; i++) {
+		dma_addr_t base = smmu->iovmm_base;
+		size_t size = smmu->page_count << PAGE_SHIFT;
+
+		smmu->map[i] = arm_iommu_create_mapping(&platform_bus_type,
+							base, size, 0);
+		if (WARN_ON(IS_ERR(smmu->map[i])))
+			return -EINVAL;
+
+		dev_dbg(smmu->dev, "map:%p %08x-%08x\n",
+			smmu->map[i], base, base + size - 1);
+	}
+
+	return 0;
+}
+
 static int tegra_smmu_probe(struct platform_device *pdev)
 {
 	struct smmu_device *smmu;
@@ -1160,13 +1182,15 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
 		return -ENODEV;
 
-	bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
+	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
+					 sizeof(struct dma_iommu_mapping *));
 	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
 	if (!smmu) {
 		dev_err(dev, "failed to allocate smmu_device\n");
 		return -ENOMEM;
 	}
 
+	smmu->map = (struct dma_iommu_mapping **)(smmu->as + asids);
 	smmu->nregs = pdev->num_resources;
 	smmu->regs = devm_kzalloc(dev, 2 * smmu->nregs * sizeof(*smmu->regs),
 				  GFP_KERNEL);
@@ -1238,7 +1262,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	smmu_debugfs_create(smmu);
 	smmu_handle = smmu;
 	bus_set_iommu(&platform_bus_type, &smmu_iommu_ops);
-	return 0;
+	return tegra_smmu_create_default_map(smmu);
 }
 
 static int tegra_smmu_remove(struct platform_device *pdev)
-- 
1.7.9.5

[PATCH v2 13/22] iommu/tegra: smmu: Create default IOVA maps

[Hiroshi Doyu]

Create default IOVA maps at boot-up which can be attached to devices.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |   28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 3e03c69..1617c90 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -319,6 +319,8 @@ struct smmu_device {
 
 	struct device_node *ahb;
 
+	struct dma_iommu_mapping **map;
+
 	int		num_as;
 	struct smmu_as	as[0];		/* Run-time allocated array */
 };
@@ -1144,6 +1146,26 @@ static int tegra_smmu_resume(struct device *dev)
 	return err;
 }
 
+static int tegra_smmu_create_default_map(struct smmu_device *smmu)
+{
+	int i;
+
+	for (i = 0; i < smmu->num_as; i++) {
+		dma_addr_t base = smmu->iovmm_base;
+		size_t size = smmu->page_count << PAGE_SHIFT;
+
+		smmu->map[i] = arm_iommu_create_mapping(&platform_bus_type,
+							base, size, 0);
+		if (WARN_ON(IS_ERR(smmu->map[i])))
+			return -EINVAL;
+
+		dev_dbg(smmu->dev, "map:%p %08x-%08x\n",
+			smmu->map[i], base, base + size - 1);
+	}
+
+	return 0;
+}
+
 static int tegra_smmu_probe(struct platform_device *pdev)
 {
 	struct smmu_device *smmu;
@@ -1160,13 +1182,15 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
 		return -ENODEV;
 
-	bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
+	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
+					 sizeof(struct dma_iommu_mapping *));
 	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
 	if (!smmu) {
 		dev_err(dev, "failed to allocate smmu_device\n");
 		return -ENOMEM;
 	}
 
+	smmu->map = (struct dma_iommu_mapping **)(smmu->as + asids);
 	smmu->nregs = pdev->num_resources;
 	smmu->regs = devm_kzalloc(dev, 2 * smmu->nregs * sizeof(*smmu->regs),
 				  GFP_KERNEL);
@@ -1238,7 +1262,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	smmu_debugfs_create(smmu);
 	smmu_handle = smmu;
 	bus_set_iommu(&platform_bus_type, &smmu_iommu_ops);
-	return 0;
+	return tegra_smmu_create_default_map(smmu);
 }
 
 static int tegra_smmu_remove(struct platform_device *pdev)
-- 
1.7.9.5

Re: [PATCH v2 13/22] iommu/tegra: smmu: Create default IOVA maps

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Create default IOVA maps at boot-up which can be attached to devices.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  static int tegra_smmu_probe(struct platform_device *pdev)
>  {
>  	struct smmu_device *smmu;

> @@ -1160,13 +1182,15 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>  	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
>  		return -ENODEV;
>  
> -	bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
> +	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
> +					 sizeof(struct dma_iommu_mapping *));
>  	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
>  	if (!smmu) {
>  		dev_err(dev, "failed to allocate smmu_device\n");
>  		return -ENOMEM;
>  	}
>  
> +	smmu->map = (struct dma_iommu_mapping **)(smmu->as + asids);

Shouldn't "+ asids" be "+ asids * sizeof(*smmu->as)" to match the
calculation of "bytes" above?

[PATCH v2 13/22] iommu/tegra: smmu: Create default IOVA maps

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Create default IOVA maps at boot-up which can be attached to devices.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  static int tegra_smmu_probe(struct platform_device *pdev)
>  {
>  	struct smmu_device *smmu;

> @@ -1160,13 +1182,15 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>  	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
>  		return -ENODEV;
>  
> -	bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
> +	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
> +					 sizeof(struct dma_iommu_mapping *));
>  	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
>  	if (!smmu) {
>  		dev_err(dev, "failed to allocate smmu_device\n");
>  		return -ENOMEM;
>  	}
>  
> +	smmu->map = (struct dma_iommu_mapping **)(smmu->as + asids);

Shouldn't "+ asids" be "+ asids * sizeof(*smmu->as)" to match the
calculation of "bytes" above?

Re: [PATCH v2 13/22] iommu/tegra: smmu: Create default IOVA maps

[Hiroshi Doyu]

Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Thu, 18 Jul 2013 22:10:29 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Create default IOVA maps at boot-up which can be attached to devices.
> 
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> >  static int tegra_smmu_probe(struct platform_device *pdev)
> >  {
> >  	struct smmu_device *smmu;
> 
> > @@ -1160,13 +1182,15 @@ static int tegra_smmu_probe(struct platform_device *pdev)
> >  	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
> >  		return -ENODEV;
> >  
> > -	bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
> > +	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
> > +					 sizeof(struct dma_iommu_mapping *));
> >  	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
> >  	if (!smmu) {
> >  		dev_err(dev, "failed to allocate smmu_device\n");
> >  		return -ENOMEM;
> >  	}
> >  
> > +	smmu->map = (struct dma_iommu_mapping **)(smmu->as + asids);
> 
> Shouldn't "+ asids" be "+ asids * sizeof(*smmu->as)" to match the
> calculation of "bytes" above?

The structure is:

smmu {
     ....
     struct dma_iommu_mapping **map;
     ....	
     struct smmu_as	as[0];
};

I think that this is correct, but is the following better?

+ smmu->map = (struct dma_iommu_mapping **)&smmu->as[asids];

[PATCH v2 13/22] iommu/tegra: smmu: Create default IOVA maps

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:10:29 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Create default IOVA maps at boot-up which can be attached to devices.
> 
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> >  static int tegra_smmu_probe(struct platform_device *pdev)
> >  {
> >  	struct smmu_device *smmu;
> 
> > @@ -1160,13 +1182,15 @@ static int tegra_smmu_probe(struct platform_device *pdev)
> >  	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
> >  		return -ENODEV;
> >  
> > -	bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
> > +	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
> > +					 sizeof(struct dma_iommu_mapping *));
> >  	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
> >  	if (!smmu) {
> >  		dev_err(dev, "failed to allocate smmu_device\n");
> >  		return -ENOMEM;
> >  	}
> >  
> > +	smmu->map = (struct dma_iommu_mapping **)(smmu->as + asids);
> 
> Shouldn't "+ asids" be "+ asids * sizeof(*smmu->as)" to match the
> calculation of "bytes" above?

The structure is:

smmu {
     ....
     struct dma_iommu_mapping **map;
     ....	
     struct smmu_as	as[0];
};

I think that this is correct, but is the following better?

+ smmu->map = (struct dma_iommu_mapping **)&smmu->as[asids];

Re: [PATCH v2 13/22] iommu/tegra: smmu: Create default IOVA maps

[Stephen Warren]

On 07/29/2013 05:24 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Thu, 18 Jul 2013 22:10:29 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Create default IOVA maps at boot-up which can be attached to devices.
>>
>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>
>>>  static int tegra_smmu_probe(struct platform_device *pdev)
>>>  {
>>>  	struct smmu_device *smmu;
>>
>>> @@ -1160,13 +1182,15 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>>>  	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
>>>  		return -ENODEV;
>>>  
>>> -	bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
>>> +	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
>>> +					 sizeof(struct dma_iommu_mapping *));
>>>  	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
>>>  	if (!smmu) {
>>>  		dev_err(dev, "failed to allocate smmu_device\n");
>>>  		return -ENOMEM;
>>>  	}
>>>  
>>> +	smmu->map = (struct dma_iommu_mapping **)(smmu->as + asids);
>>
>> Shouldn't "+ asids" be "+ asids * sizeof(*smmu->as)" to match the
>> calculation of "bytes" above?
> 
> The structure is:
> 
> smmu {
>      ....
>      struct dma_iommu_mapping **map;
>      ....	
>      struct smmu_as	as[0];
> };
> 
> I think that this is correct, but is the following better?

Oh right, I guess since that's pointer-math, the compiler already
multiplies by the array entry size.

> + smmu->map = (struct dma_iommu_mapping **)&smmu->as[asids];

That would be a bit more obvious.

[PATCH v2 13/22] iommu/tegra: smmu: Create default IOVA maps

[Stephen Warren]

On 07/29/2013 05:24 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:10:29 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Create default IOVA maps at boot-up which can be attached to devices.
>>
>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>
>>>  static int tegra_smmu_probe(struct platform_device *pdev)
>>>  {
>>>  	struct smmu_device *smmu;
>>
>>> @@ -1160,13 +1182,15 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>>>  	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
>>>  		return -ENODEV;
>>>  
>>> -	bytes = sizeof(*smmu) + asids * sizeof(*smmu->as);
>>> +	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
>>> +					 sizeof(struct dma_iommu_mapping *));
>>>  	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
>>>  	if (!smmu) {
>>>  		dev_err(dev, "failed to allocate smmu_device\n");
>>>  		return -ENOMEM;
>>>  	}
>>>  
>>> +	smmu->map = (struct dma_iommu_mapping **)(smmu->as + asids);
>>
>> Shouldn't "+ asids" be "+ asids * sizeof(*smmu->as)" to match the
>> calculation of "bytes" above?
> 
> The structure is:
> 
> smmu {
>      ....
>      struct dma_iommu_mapping **map;
>      ....	
>      struct smmu_as	as[0];
> };
> 
> I think that this is correct, but is the following better?

Oh right, I guess since that's pointer-math, the compiler already
multiplies by the array entry size.

> + smmu->map = (struct dma_iommu_mapping **)&smmu->as[asids];

That would be a bit more obvious.

[PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Hiroshi Doyu]

Register platform_devices to IOMMU dynamically via
ops->{add,remove}_device().

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 1617c90..35f4a16 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -39,6 +39,7 @@
 
 #include <asm/page.h>
 #include <asm/cacheflush.h>
+#include <asm/dma-iommu.h>
 
 enum smmu_hwgrp {
 	HWGRP_AFI,
@@ -949,6 +950,40 @@ static void smmu_iommu_domain_destroy(struct iommu_domain *domain)
 	dev_dbg(smmu->dev, "smmu_as@%p\n", as);
 }
 
+/*
+ * ASID[0] for the system default
+ * ASID[1] for PPCS, which has SDMMC
+ * ASID[2][3].. open for drivers, first come, first served.
+ */
+enum {
+	SYSTEM_DEFAULT,
+	SYSTEM_PROTECTED,
+};
+
+static int smmu_iommu_add_device(struct device *dev)
+{
+	int err;
+	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
+
+	if (!map)
+		goto out;
+
+	err = arm_iommu_attach_device(dev, map);
+	if (err) {
+		dev_err(dev, "Failed to attach to map %p\n", map);
+		return err;
+	}
+out:
+	dev_dbg(dev, "Attached to map %p\n", map);
+	return 0;
+}
+
+static void smmu_iommu_remove_device(struct device *dev)
+{
+	dev_dbg(dev, "Detaching from map %p\n", to_dma_iommu_mapping(dev));
+	arm_iommu_detach_device(dev);
+}
+
 static struct iommu_ops smmu_iommu_ops = {
 	.domain_init	= smmu_iommu_domain_init,
 	.domain_destroy	= smmu_iommu_domain_destroy,
@@ -958,6 +993,8 @@ static struct iommu_ops smmu_iommu_ops = {
 	.unmap		= smmu_iommu_unmap,
 	.iova_to_phys	= smmu_iommu_iova_to_phys,
 	.domain_has_cap	= smmu_iommu_domain_has_cap,
+	.add_device	= smmu_iommu_add_device,
+	.remove_device	= smmu_iommu_remove_device,
 	.pgsize_bitmap	= SMMU_IOMMU_PGSIZES,
 };
 
-- 
1.7.9.5

[PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Hiroshi Doyu]

Register platform_devices to IOMMU dynamically via
ops->{add,remove}_device().

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 1617c90..35f4a16 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -39,6 +39,7 @@
 
 #include <asm/page.h>
 #include <asm/cacheflush.h>
+#include <asm/dma-iommu.h>
 
 enum smmu_hwgrp {
 	HWGRP_AFI,
@@ -949,6 +950,40 @@ static void smmu_iommu_domain_destroy(struct iommu_domain *domain)
 	dev_dbg(smmu->dev, "smmu_as@%p\n", as);
 }
 
+/*
+ * ASID[0] for the system default
+ * ASID[1] for PPCS, which has SDMMC
+ * ASID[2][3].. open for drivers, first come, first served.
+ */
+enum {
+	SYSTEM_DEFAULT,
+	SYSTEM_PROTECTED,
+};
+
+static int smmu_iommu_add_device(struct device *dev)
+{
+	int err;
+	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
+
+	if (!map)
+		goto out;
+
+	err = arm_iommu_attach_device(dev, map);
+	if (err) {
+		dev_err(dev, "Failed to attach to map %p\n", map);
+		return err;
+	}
+out:
+	dev_dbg(dev, "Attached to map %p\n", map);
+	return 0;
+}
+
+static void smmu_iommu_remove_device(struct device *dev)
+{
+	dev_dbg(dev, "Detaching from map %p\n", to_dma_iommu_mapping(dev));
+	arm_iommu_detach_device(dev);
+}
+
 static struct iommu_ops smmu_iommu_ops = {
 	.domain_init	= smmu_iommu_domain_init,
 	.domain_destroy	= smmu_iommu_domain_destroy,
@@ -958,6 +993,8 @@ static struct iommu_ops smmu_iommu_ops = {
 	.unmap		= smmu_iommu_unmap,
 	.iova_to_phys	= smmu_iommu_iova_to_phys,
 	.domain_has_cap	= smmu_iommu_domain_has_cap,
+	.add_device	= smmu_iommu_add_device,
+	.remove_device	= smmu_iommu_remove_device,
 	.pgsize_bitmap	= SMMU_IOMMU_PGSIZES,
 };
 
-- 
1.7.9.5

Re: [PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Register platform_devices to IOMMU dynamically via
> ops->{add,remove}_device().

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> +/*
> + * ASID[0] for the system default
> + * ASID[1] for PPCS, which has SDMMC

I have no idea what PPCS is. The patch description for 21/22 implies
much more than SDMMC is "in PPCS"...

> + * ASID[2][3].. open for drivers, first come, first served.
> + */
> +enum {
> +	SYSTEM_DEFAULT,
> +	SYSTEM_PROTECTED,
> +};

Why hard-code this mapping? Can't devices be assigned to ASIDs based on
a DT property? I assume there's nothing in the SMMU HW that requires
specific ASIDs to be used?

I don't see anything in this series which implements the "open for
drivers" part of the description for ASID 2/3/...

Perhaps not so relevant here since this patch only uses SYSTEM_DEFAULT
as the ASID number, but don't you need to validate that smmu->num_as is
large enough to support all values in the enum definition above?

Since this patch doesn't actually use anything other than
SYSTEM_DEFAULT, I wonder if this patch shouldn't be squashed into patch
21/22, where multiple ASIDs are actually used?

[PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Register platform_devices to IOMMU dynamically via
> ops->{add,remove}_device().

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> +/*
> + * ASID[0] for the system default
> + * ASID[1] for PPCS, which has SDMMC

I have no idea what PPCS is. The patch description for 21/22 implies
much more than SDMMC is "in PPCS"...

> + * ASID[2][3].. open for drivers, first come, first served.
> + */
> +enum {
> +	SYSTEM_DEFAULT,
> +	SYSTEM_PROTECTED,
> +};

Why hard-code this mapping? Can't devices be assigned to ASIDs based on
a DT property? I assume there's nothing in the SMMU HW that requires
specific ASIDs to be used?

I don't see anything in this series which implements the "open for
drivers" part of the description for ASID 2/3/...

Perhaps not so relevant here since this patch only uses SYSTEM_DEFAULT
as the ASID number, but don't you need to validate that smmu->num_as is
large enough to support all values in the enum definition above?

Since this patch doesn't actually use anything other than
SYSTEM_DEFAULT, I wonder if this patch shouldn't be squashed into patch
21/22, where multiple ASIDs are actually used?

Re: [PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Hiroshi Doyu]

Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Thu, 18 Jul 2013 22:17:21 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Register platform_devices to IOMMU dynamically via
> > ops->{add,remove}_device().
> 
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> > +/*
> > + * ASID[0] for the system default
> > + * ASID[1] for PPCS, which has SDMMC
> 
> I have no idea what PPCS is. The patch description for 21/22 implies
> much more than SDMMC is "in PPCS"...

"PPCS" is all AHB bus children/(client).

> > + * ASID[2][3].. open for drivers, first come, first served.
> > + */
> > +enum {
> > +	SYSTEM_DEFAULT,
> > +	SYSTEM_PROTECTED,
> > +};
> 
> Why hard-code this mapping? Can't devices be assigned to ASIDs based on
> a DT property? I assume there's nothing in the SMMU HW that requires
> specific ASIDs to be used?

Right. Where should those policy be passed, board DT files?

> I don't see anything in this series which implements the "open for
> drivers" part of the description for ASID 2/3/...

I expect that some smart HW(host1x) will use AS[2/3] for their smarter usage.

[PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:17:21 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Register platform_devices to IOMMU dynamically via
> > ops->{add,remove}_device().
> 
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> > +/*
> > + * ASID[0] for the system default
> > + * ASID[1] for PPCS, which has SDMMC
> 
> I have no idea what PPCS is. The patch description for 21/22 implies
> much more than SDMMC is "in PPCS"...

"PPCS" is all AHB bus children/(client).

> > + * ASID[2][3].. open for drivers, first come, first served.
> > + */
> > +enum {
> > +	SYSTEM_DEFAULT,
> > +	SYSTEM_PROTECTED,
> > +};
> 
> Why hard-code this mapping? Can't devices be assigned to ASIDs based on
> a DT property? I assume there's nothing in the SMMU HW that requires
> specific ASIDs to be used?

Right. Where should those policy be passed, board DT files?

> I don't see anything in this series which implements the "open for
> drivers" part of the description for ASID 2/3/...

I expect that some smart HW(host1x) will use AS[2/3] for their smarter usage.

Re: [PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Stephen Warren]

On 07/29/2013 05:27 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Thu, 18 Jul 2013 22:17:21 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Register platform_devices to IOMMU dynamically via
>>> ops->{add,remove}_device().
>>
>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>
>>> +/*
>>> + * ASID[0] for the system default
>>> + * ASID[1] for PPCS, which has SDMMC
>>
>> I have no idea what PPCS is. The patch description for 21/22 implies
>> much more than SDMMC is "in PPCS"...
> 
> "PPCS" is all AHB bus children/(client).

Just to be clear: I was hoping that the comment would be fixed.

>>> + * ASID[2][3].. open for drivers, first come, first served.
>>> + */
>>> +enum {
>>> +	SYSTEM_DEFAULT,
>>> +	SYSTEM_PROTECTED,
>>> +};
>>
>> Why hard-code this mapping? Can't devices be assigned to ASIDs based on
>> a DT property? I assume there's nothing in the SMMU HW that requires
>> specific ASIDs to be used?
> 
> Right. Where should those policy be passed, board DT files?

Is the HW module (== SW group?) to ASID mapping policy or is a
particular mapping required by HW?

Can the ASID mapping just happen dynamically in SW rather than DT
dictating a particular mapping?

I assume the mapping must be set up before any HW is used, and can't be
modified later, so there's no much chance of deferring any policy
decisions to user-space etc.?

[PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Stephen Warren]

On 07/29/2013 05:27 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:17:21 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Register platform_devices to IOMMU dynamically via
>>> ops->{add,remove}_device().
>>
>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>
>>> +/*
>>> + * ASID[0] for the system default
>>> + * ASID[1] for PPCS, which has SDMMC
>>
>> I have no idea what PPCS is. The patch description for 21/22 implies
>> much more than SDMMC is "in PPCS"...
> 
> "PPCS" is all AHB bus children/(client).

Just to be clear: I was hoping that the comment would be fixed.

>>> + * ASID[2][3].. open for drivers, first come, first served.
>>> + */
>>> +enum {
>>> +	SYSTEM_DEFAULT,
>>> +	SYSTEM_PROTECTED,
>>> +};
>>
>> Why hard-code this mapping? Can't devices be assigned to ASIDs based on
>> a DT property? I assume there's nothing in the SMMU HW that requires
>> specific ASIDs to be used?
> 
> Right. Where should those policy be passed, board DT files?

Is the HW module (== SW group?) to ASID mapping policy or is a
particular mapping required by HW?

Can the ASID mapping just happen dynamically in SW rather than DT
dictating a particular mapping?

I assume the mapping must be set up before any HW is used, and can't be
modified later, so there's no much chance of deferring any policy
decisions to user-space etc.?

Re: [PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Hiroshi Doyu]

On Mon, 29 Jul 2013 19:54:09 +0200
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
...
> >> Why hard-code this mapping? Can't devices be assigned to ASIDs based on
> >> a DT property? I assume there's nothing in the SMMU HW that requires
> >> specific ASIDs to be used?
> > 
> > Right. Where should those policy be passed, board DT files?
> 
> Is the HW module (== SW group?) to ASID mapping policy or is a
> particular mapping required by HW?

only policy.
But swgroup can have multiple HWs, they need to be set in the same swgroup.

> Can the ASID mapping just happen dynamically in SW rather than DT
> dictating a particular mapping?

No info about ASID in DT, at least, right now.
If a board DT file is the place to put policy, then it can be an option.
 
> I assume the mapping must be set up before any HW is used, and can't be
> modified later, so there's no much chance of deferring any policy
> decisions to user-space etc.?

I think that both cases are needed.

Some traditional devices(MMC) can be changed later as it may have rootfs on it.
For some smarter devices like host1x, it want to change ASID dynamically per request/context.

[PATCH v2 14/22] iommu/tegra: smmu: Register platform_device to IOMMU dynamically

[Hiroshi Doyu]

On Mon, 29 Jul 2013 19:54:09 +0200
Stephen Warren <swarren@wwwdotorg.org> wrote:
...
> >> Why hard-code this mapping? Can't devices be assigned to ASIDs based on
> >> a DT property? I assume there's nothing in the SMMU HW that requires
> >> specific ASIDs to be used?
> > 
> > Right. Where should those policy be passed, board DT files?
> 
> Is the HW module (== SW group?) to ASID mapping policy or is a
> particular mapping required by HW?

only policy.
But swgroup can have multiple HWs, they need to be set in the same swgroup.

> Can the ASID mapping just happen dynamically in SW rather than DT
> dictating a particular mapping?

No info about ASID in DT, at least, right now.
If a board DT file is the place to put policy, then it can be an option.
 
> I assume the mapping must be set up before any HW is used, and can't be
> modified later, so there's no much chance of deferring any policy
> decisions to user-space etc.?

I think that both cases are needed.

Some traditional devices(MMC) can be changed later as it may have rootfs on it.
For some smarter devices like host1x, it want to change ASID dynamically per request/context.

[PATCH v2 15/22] iommu/tegra: smmu: Calculate ASID register offset by ID

[Hiroshi Doyu]

Calculate ASID register offset by ID so that we can get rid of SoC
specific MACROs. This is needed for the unified SMMU driver over Tegra
SoCs.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 35f4a16..95c6c80 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -256,7 +256,7 @@ static const u32 smmu_hwgrp_asid_reg[] = {
 	HWGRP_INIT(VDE),
 	HWGRP_INIT(VI),
 };
-#define HWGRP_ASID_REG(x) (smmu_hwgrp_asid_reg[x])
+#define HWGRP_ASID_REG(x) ((x) * sizeof(u32) + SMMU_AFI_ASID)
 
 /*
  * Per client for address space
-- 
1.7.9.5

[PATCH v2 15/22] iommu/tegra: smmu: Calculate ASID register offset by ID

[Hiroshi Doyu]

Calculate ASID register offset by ID so that we can get rid of SoC
specific MACROs. This is needed for the unified SMMU driver over Tegra
SoCs.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 35f4a16..95c6c80 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -256,7 +256,7 @@ static const u32 smmu_hwgrp_asid_reg[] = {
 	HWGRP_INIT(VDE),
 	HWGRP_INIT(VI),
 };
-#define HWGRP_ASID_REG(x) (smmu_hwgrp_asid_reg[x])
+#define HWGRP_ASID_REG(x) ((x) * sizeof(u32) + SMMU_AFI_ASID)
 
 /*
  * Per client for address space
-- 
1.7.9.5

Re: [PATCH v2 15/22] iommu/tegra: smmu: Calculate ASID register offset by ID

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Calculate ASID register offset by ID so that we can get rid of SoC
> specific MACROs. This is needed for the unified SMMU driver over Tegra
> SoCs.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> -#define HWGRP_ASID_REG(x) (smmu_hwgrp_asid_reg[x])
> +#define HWGRP_ASID_REG(x) ((x) * sizeof(u32) + SMMU_AFI_ASID)

I think this removes the last use of smmu_hwgrp_asid_reg[]. Don't you
want to delete that in this patch?

[PATCH v2 15/22] iommu/tegra: smmu: Calculate ASID register offset by ID

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Calculate ASID register offset by ID so that we can get rid of SoC
> specific MACROs. This is needed for the unified SMMU driver over Tegra
> SoCs.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> -#define HWGRP_ASID_REG(x) (smmu_hwgrp_asid_reg[x])
> +#define HWGRP_ASID_REG(x) ((x) * sizeof(u32) + SMMU_AFI_ASID)

I think this removes the last use of smmu_hwgrp_asid_reg[]. Don't you
want to delete that in this patch?

[PATCH v2 16/22] iommu/tegra: smmu: Get "nvidia,swgroup" from DT

[Hiroshi Doyu]

This provides the info about which H/W Accelerators are supported on
Tegra SoC. This info is passed from DT. This is necessary to have the
unified SMMU driver among Tegra SoCs. Instead of using platform data,
DT passes "nvidia,swgroup" now. DT is mandatory in Tegra.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 .../bindings/iommu/nvidia,tegra30-smmu.txt         |    6 +++-
 drivers/iommu/tegra-smmu.c                         |   31 +++++++++-----------
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
index ce5c43e..0c14dca 100644
--- a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
@@ -8,14 +8,18 @@ Required properties:
 - nvidia,#asids : # of ASIDs
 - dma-window : IOVA start address and length.
 - nvidia,ahb : phandle to the ahb bus connected to SMMU.
+- nvidia,swgroups: A bitmap of supported HardWare Accelerators(HWA).
+  Each bit represents one swgroup. The assignments may be found in header
+  file <dt-bindings/memory/tegra-swgroup.h>.
 
 Example:
-	smmu {
+	iommu {
 		compatible = "nvidia,tegra30-smmu";
 		reg = <0x7000f010 0x02c
 		       0x7000f1f0 0x010
 		       0x7000f228 0x05c>;
 		nvidia,#asids = <4>;		/* # of ASIDs */
 		dma-window = <0 0x40000000>;	/* IOVA start & length */
+		nvidia,swgroups = TEGRA30_SWGROUP_ALL;
 		nvidia,ahb = <&ahb>;
 	};
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 95c6c80..c7b33f2 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -265,7 +265,7 @@ struct smmu_client {
 	struct device		*dev;
 	struct list_head	list;
 	struct smmu_as		*as;
-	u32			hwgrp;
+	u64			hwgrp;
 };
 
 /*
@@ -307,6 +307,8 @@ struct smmu_device {
 	struct device	*dev;
 	struct page *avp_vector_page;	/* dummy page shared by all AS's */
 
+	u64 swgroup;			/* swgroup ID bitmap */
+
 	/*
 	 * Register image savers for suspend/resume
 	 */
@@ -382,10 +384,10 @@ static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
  */
 #define FLUSH_SMMU_REGS(smmu)	smmu_read(smmu, SMMU_CONFIG)
 
-#define smmu_client_hwgrp(c) (u32)((c)->dev->platform_data)
+#define smmu_client_hwgrp(c)	(c->as->smmu->swgroup)
 
 static int __smmu_client_set_hwgrp(struct smmu_client *c,
-				   unsigned long map, int on)
+				   u64 map, int on)
 {
 	int i;
 	struct smmu_as *as = c->as;
@@ -398,12 +400,11 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 	if (!on)
 		map = smmu_client_hwgrp(c);
 
-	for_each_set_bit(i, &map, HWGRP_COUNT) {
+	for_each_set_bit(i, (unsigned long *)&map,
+			 sizeof(map) * BITS_PER_BYTE) {
 		offs = HWGRP_ASID_REG(i);
 		val = smmu_read(smmu, offs);
 		if (on) {
-			if (WARN_ON(val & mask))
-				goto err_hw_busy;
 			val |= mask;
 		} else {
 			WARN_ON((val & mask) == mask);
@@ -414,15 +415,6 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 	FLUSH_SMMU_REGS(smmu);
 	c->hwgrp = map;
 	return 0;
-
-err_hw_busy:
-	for_each_set_bit(i, &map, HWGRP_COUNT) {
-		offs = HWGRP_ASID_REG(i);
-		val = smmu_read(smmu, offs);
-		val &= ~mask;
-		smmu_write(smmu, val, offs);
-	}
-	return -EBUSY;
 }
 
 static int smmu_client_set_hwgrp(struct smmu_client *c, u32 map, int on)
@@ -794,7 +786,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 	struct smmu_as *as = domain->priv;
 	struct smmu_device *smmu = as->smmu;
 	struct smmu_client *client, *c;
-	u32 map;
+	u64 map;
 	int err;
 
 	client = devm_kzalloc(smmu->dev, sizeof(*c), GFP_KERNEL);
@@ -802,7 +794,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 		return -ENOMEM;
 	client->dev = dev;
 	client->as = as;
-	map = (unsigned long)dev->platform_data;
+	map = smmu->swgroup;
 	if (!map)
 		return -EINVAL;
 
@@ -1210,6 +1202,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	int i, asids, err = 0;
 	dma_addr_t uninitialized_var(base);
 	size_t bytes, uninitialized_var(size);
+	u64 swgroup;
 
 	if (smmu_handle)
 		return -EIO;
@@ -1219,6 +1212,9 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
 		return -ENODEV;
 
+	if (of_property_read_u64(dev->of_node, "nvidia,swgroup", &swgroup))
+		return -ENODEV;
+
 	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
 					 sizeof(struct dma_iommu_mapping *));
 	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
@@ -1267,6 +1263,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	smmu->num_as = asids;
 	smmu->iovmm_base = base;
 	smmu->page_count = size;
+	smmu->swgroup = swgroup;
 
 	smmu->translation_enable_0 = ~0;
 	smmu->translation_enable_1 = ~0;
-- 
1.7.9.5

[PATCH v2 16/22] iommu/tegra: smmu: Get "nvidia,swgroup" from DT

[Hiroshi Doyu]

This provides the info about which H/W Accelerators are supported on
Tegra SoC. This info is passed from DT. This is necessary to have the
unified SMMU driver among Tegra SoCs. Instead of using platform data,
DT passes "nvidia,swgroup" now. DT is mandatory in Tegra.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 .../bindings/iommu/nvidia,tegra30-smmu.txt         |    6 +++-
 drivers/iommu/tegra-smmu.c                         |   31 +++++++++-----------
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
index ce5c43e..0c14dca 100644
--- a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
@@ -8,14 +8,18 @@ Required properties:
 - nvidia,#asids : # of ASIDs
 - dma-window : IOVA start address and length.
 - nvidia,ahb : phandle to the ahb bus connected to SMMU.
+- nvidia,swgroups: A bitmap of supported HardWare Accelerators(HWA).
+  Each bit represents one swgroup. The assignments may be found in header
+  file <dt-bindings/memory/tegra-swgroup.h>.
 
 Example:
-	smmu {
+	iommu {
 		compatible = "nvidia,tegra30-smmu";
 		reg = <0x7000f010 0x02c
 		       0x7000f1f0 0x010
 		       0x7000f228 0x05c>;
 		nvidia,#asids = <4>;		/* # of ASIDs */
 		dma-window = <0 0x40000000>;	/* IOVA start & length */
+		nvidia,swgroups = TEGRA30_SWGROUP_ALL;
 		nvidia,ahb = <&ahb>;
 	};
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 95c6c80..c7b33f2 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -265,7 +265,7 @@ struct smmu_client {
 	struct device		*dev;
 	struct list_head	list;
 	struct smmu_as		*as;
-	u32			hwgrp;
+	u64			hwgrp;
 };
 
 /*
@@ -307,6 +307,8 @@ struct smmu_device {
 	struct device	*dev;
 	struct page *avp_vector_page;	/* dummy page shared by all AS's */
 
+	u64 swgroup;			/* swgroup ID bitmap */
+
 	/*
 	 * Register image savers for suspend/resume
 	 */
@@ -382,10 +384,10 @@ static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
  */
 #define FLUSH_SMMU_REGS(smmu)	smmu_read(smmu, SMMU_CONFIG)
 
-#define smmu_client_hwgrp(c) (u32)((c)->dev->platform_data)
+#define smmu_client_hwgrp(c)	(c->as->smmu->swgroup)
 
 static int __smmu_client_set_hwgrp(struct smmu_client *c,
-				   unsigned long map, int on)
+				   u64 map, int on)
 {
 	int i;
 	struct smmu_as *as = c->as;
@@ -398,12 +400,11 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 	if (!on)
 		map = smmu_client_hwgrp(c);
 
-	for_each_set_bit(i, &map, HWGRP_COUNT) {
+	for_each_set_bit(i, (unsigned long *)&map,
+			 sizeof(map) * BITS_PER_BYTE) {
 		offs = HWGRP_ASID_REG(i);
 		val = smmu_read(smmu, offs);
 		if (on) {
-			if (WARN_ON(val & mask))
-				goto err_hw_busy;
 			val |= mask;
 		} else {
 			WARN_ON((val & mask) == mask);
@@ -414,15 +415,6 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 	FLUSH_SMMU_REGS(smmu);
 	c->hwgrp = map;
 	return 0;
-
-err_hw_busy:
-	for_each_set_bit(i, &map, HWGRP_COUNT) {
-		offs = HWGRP_ASID_REG(i);
-		val = smmu_read(smmu, offs);
-		val &= ~mask;
-		smmu_write(smmu, val, offs);
-	}
-	return -EBUSY;
 }
 
 static int smmu_client_set_hwgrp(struct smmu_client *c, u32 map, int on)
@@ -794,7 +786,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 	struct smmu_as *as = domain->priv;
 	struct smmu_device *smmu = as->smmu;
 	struct smmu_client *client, *c;
-	u32 map;
+	u64 map;
 	int err;
 
 	client = devm_kzalloc(smmu->dev, sizeof(*c), GFP_KERNEL);
@@ -802,7 +794,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 		return -ENOMEM;
 	client->dev = dev;
 	client->as = as;
-	map = (unsigned long)dev->platform_data;
+	map = smmu->swgroup;
 	if (!map)
 		return -EINVAL;
 
@@ -1210,6 +1202,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	int i, asids, err = 0;
 	dma_addr_t uninitialized_var(base);
 	size_t bytes, uninitialized_var(size);
+	u64 swgroup;
 
 	if (smmu_handle)
 		return -EIO;
@@ -1219,6 +1212,9 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	if (of_property_read_u32(dev->of_node, "nvidia,#asids", &asids))
 		return -ENODEV;
 
+	if (of_property_read_u64(dev->of_node, "nvidia,swgroup", &swgroup))
+		return -ENODEV;
+
 	bytes = sizeof(*smmu) + asids * (sizeof(*smmu->as) +
 					 sizeof(struct dma_iommu_mapping *));
 	smmu = devm_kzalloc(dev, bytes, GFP_KERNEL);
@@ -1267,6 +1263,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	smmu->num_as = asids;
 	smmu->iovmm_base = base;
 	smmu->page_count = size;
+	smmu->swgroup = swgroup;
 
 	smmu->translation_enable_0 = ~0;
 	smmu->translation_enable_1 = ~0;
-- 
1.7.9.5

Re: [PATCH v2 16/22] iommu/tegra: smmu: Get "nvidia,swgroup" from DT

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> This provides the info about which H/W Accelerators are supported on
> Tegra SoC. This info is passed from DT. This is necessary to have the
> unified SMMU driver among Tegra SoCs. Instead of using platform data,
> DT passes "nvidia,swgroup" now. DT is mandatory in Tegra.

> diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt

> +- nvidia,swgroups: A bitmap of supported HardWare Accelerators(HWA).
> +  Each bit represents one swgroup. The assignments may be found in header
> +  file <dt-bindings/memory/tegra-swgroup.h>.

There needs to be a default for this field if one is not specified so
that existing DTs continue to work without modification.

How many cells big is this property?

Is this really a bitmap of HWAs? Surely it's a bitmap of SMMU client IDs?

>  Example:
> -	smmu {
> +	iommu {

The node name shouldn't be interpreted by anything, so there should be
no need to change it at all. Certainly, it should not be changed by this
patch, since this patch is all about SMMU client IDs.

> +		nvidia,swgroups = TEGRA30_SWGROUP_ALL;

As I mentioned before, macros shouldn't include syntax/structure, just
data values.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> @@ -265,7 +265,7 @@ struct smmu_client {
>  	struct device		*dev;
>  	struct list_head	list;
>  	struct smmu_as		*as;
> -	u32			hwgrp;
> +	u64			hwgrp;

Why is that "hwgrp" not "swgrp"? Don't they represent the same thing?

> @@ -307,6 +307,8 @@ struct smmu_device {
>  	struct device	*dev;
>  	struct page *avp_vector_page;	/* dummy page shared by all AS's */
>  
> +	u64 swgroup;			/* swgroup ID bitmap */

If that's a bitmap, then it represents multiple things, so "swgroups"?

> @@ -382,10 +384,10 @@ static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
>   */
>  #define FLUSH_SMMU_REGS(smmu)	smmu_read(smmu, SMMU_CONFIG)
>  
> -#define smmu_client_hwgrp(c) (u32)((c)->dev->platform_data)
> +#define smmu_client_hwgrp(c)	(c->as->smmu->swgroup)

hwgrp-vs-swgrp inconsistency again.

Is this series git bisectable? I worry that by the time patch 14 is
applied, the SMMU starts affecting client devices, whereas none of those
client devices have swgroup values defined as their client data, and
hence without this patch also applied, things might blow up in
interesting ways.

I wonder why the code was ever using dev->platform_data in this way; the
platform data for a device should have its structure/semantics defined
by the driver for that device, not by an SMMU that happens to affect
that device. Ick!

>  static int __smmu_client_set_hwgrp(struct smmu_client *c,
> -				   unsigned long map, int on)
> +				   u64 map, int on)
>  {
>  	int i;
>  	struct smmu_as *as = c->as;
> @@ -398,12 +400,11 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
>  	if (!on)
>  		map = smmu_client_hwgrp(c);
>  
> -	for_each_set_bit(i, &map, HWGRP_COUNT) {
> +	for_each_set_bit(i, (unsigned long *)&map,
> +			 sizeof(map) * BITS_PER_BYTE) {

Why change the type if it just forces you to add this cast?

>  		offs = HWGRP_ASID_REG(i);
>  		val = smmu_read(smmu, offs);
>  		if (on) {
> -			if (WARN_ON(val & mask))
> -				goto err_hw_busy;

Why?

[PATCH v2 16/22] iommu/tegra: smmu: Get "nvidia,swgroup" from DT

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> This provides the info about which H/W Accelerators are supported on
> Tegra SoC. This info is passed from DT. This is necessary to have the
> unified SMMU driver among Tegra SoCs. Instead of using platform data,
> DT passes "nvidia,swgroup" now. DT is mandatory in Tegra.

> diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt

> +- nvidia,swgroups: A bitmap of supported HardWare Accelerators(HWA).
> +  Each bit represents one swgroup. The assignments may be found in header
> +  file <dt-bindings/memory/tegra-swgroup.h>.

There needs to be a default for this field if one is not specified so
that existing DTs continue to work without modification.

How many cells big is this property?

Is this really a bitmap of HWAs? Surely it's a bitmap of SMMU client IDs?

>  Example:
> -	smmu {
> +	iommu {

The node name shouldn't be interpreted by anything, so there should be
no need to change it at all. Certainly, it should not be changed by this
patch, since this patch is all about SMMU client IDs.

> +		nvidia,swgroups = TEGRA30_SWGROUP_ALL;

As I mentioned before, macros shouldn't include syntax/structure, just
data values.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> @@ -265,7 +265,7 @@ struct smmu_client {
>  	struct device		*dev;
>  	struct list_head	list;
>  	struct smmu_as		*as;
> -	u32			hwgrp;
> +	u64			hwgrp;

Why is that "hwgrp" not "swgrp"? Don't they represent the same thing?

> @@ -307,6 +307,8 @@ struct smmu_device {
>  	struct device	*dev;
>  	struct page *avp_vector_page;	/* dummy page shared by all AS's */
>  
> +	u64 swgroup;			/* swgroup ID bitmap */

If that's a bitmap, then it represents multiple things, so "swgroups"?

> @@ -382,10 +384,10 @@ static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
>   */
>  #define FLUSH_SMMU_REGS(smmu)	smmu_read(smmu, SMMU_CONFIG)
>  
> -#define smmu_client_hwgrp(c) (u32)((c)->dev->platform_data)
> +#define smmu_client_hwgrp(c)	(c->as->smmu->swgroup)

hwgrp-vs-swgrp inconsistency again.

Is this series git bisectable? I worry that by the time patch 14 is
applied, the SMMU starts affecting client devices, whereas none of those
client devices have swgroup values defined as their client data, and
hence without this patch also applied, things might blow up in
interesting ways.

I wonder why the code was ever using dev->platform_data in this way; the
platform data for a device should have its structure/semantics defined
by the driver for that device, not by an SMMU that happens to affect
that device. Ick!

>  static int __smmu_client_set_hwgrp(struct smmu_client *c,
> -				   unsigned long map, int on)
> +				   u64 map, int on)
>  {
>  	int i;
>  	struct smmu_as *as = c->as;
> @@ -398,12 +400,11 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
>  	if (!on)
>  		map = smmu_client_hwgrp(c);
>  
> -	for_each_set_bit(i, &map, HWGRP_COUNT) {
> +	for_each_set_bit(i, (unsigned long *)&map,
> +			 sizeof(map) * BITS_PER_BYTE) {

Why change the type if it just forces you to add this cast?

>  		offs = HWGRP_ASID_REG(i);
>  		val = smmu_read(smmu, offs);
>  		if (on) {
> -			if (WARN_ON(val & mask))
> -				goto err_hw_busy;

Why?

Re: [PATCH v2 16/22] iommu/tegra: smmu: Get "nvidia,swgroup" from DT

[Hiroshi Doyu]

Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Thu, 18 Jul 2013 22:28:42 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > This provides the info about which H/W Accelerators are supported on
> > Tegra SoC. This info is passed from DT. This is necessary to have the
> > unified SMMU driver among Tegra SoCs. Instead of using platform data,
> > DT passes "nvidia,swgroup" now. DT is mandatory in Tegra.
> 
> > diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
> 
> > +- nvidia,swgroups: A bitmap of supported HardWare Accelerators(HWA).
> > +  Each bit represents one swgroup. The assignments may be found in header
> > +  file <dt-bindings/memory/tegra-swgroup.h>.
> 
> There needs to be a default for this field if one is not specified so
> that existing DTs continue to work without modification.

Only enabling PPCS(AHB) can be an option because PPCS has SD/MMC where
rootfs can be located ususally.

> How many cells big is this property?

64

> Is this really a bitmap of HWAs? Surely it's a bitmap of SMMU client
> IDs?

At least this info can be used for PMC too.

.....
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> > @@ -265,7 +265,7 @@ struct smmu_client {
> >  	struct device		*dev;
> >  	struct list_head	list;
> >  	struct smmu_as		*as;
> > -	u32			hwgrp;
> > +	u64			hwgrp;
> 
> Why is that "hwgrp" not "swgrp"? Don't they represent the same
> thing?

They are same but initial SMMU driver used the term "hwgroup". Should
this be renamed with another patch or can it be left as it is?

....
> >  static int __smmu_client_set_hwgrp(struct smmu_client *c,
> > -				   unsigned long map, int on)
> > +				   u64 map, int on)
> >  {
> >  	int i;
> >  	struct smmu_as *as = c->as;
> > @@ -398,12 +400,11 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
> >  	if (!on)
> >  		map = smmu_client_hwgrp(c);
> >  
> > -	for_each_set_bit(i, &map, HWGRP_COUNT) {
> > +	for_each_set_bit(i, (unsigned long *)&map,
> > +			 sizeof(map) * BITS_PER_BYTE) {
> 
> Why change the type if it just forces you to add this cast?

u32 map; -> u64 map;

for_each_set_bit() expects "unsigned long *" for any length of bitmap.

[PATCH v2 16/22] iommu/tegra: smmu: Get "nvidia,swgroup" from DT

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:28:42 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > This provides the info about which H/W Accelerators are supported on
> > Tegra SoC. This info is passed from DT. This is necessary to have the
> > unified SMMU driver among Tegra SoCs. Instead of using platform data,
> > DT passes "nvidia,swgroup" now. DT is mandatory in Tegra.
> 
> > diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
> 
> > +- nvidia,swgroups: A bitmap of supported HardWare Accelerators(HWA).
> > +  Each bit represents one swgroup. The assignments may be found in header
> > +  file <dt-bindings/memory/tegra-swgroup.h>.
> 
> There needs to be a default for this field if one is not specified so
> that existing DTs continue to work without modification.

Only enabling PPCS(AHB) can be an option because PPCS has SD/MMC where
rootfs can be located ususally.

> How many cells big is this property?

64

> Is this really a bitmap of HWAs? Surely it's a bitmap of SMMU client
> IDs?

At least this info can be used for PMC too.

.....
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> > @@ -265,7 +265,7 @@ struct smmu_client {
> >  	struct device		*dev;
> >  	struct list_head	list;
> >  	struct smmu_as		*as;
> > -	u32			hwgrp;
> > +	u64			hwgrp;
> 
> Why is that "hwgrp" not "swgrp"? Don't they represent the same
> thing?

They are same but initial SMMU driver used the term "hwgroup". Should
this be renamed with another patch or can it be left as it is?

....
> >  static int __smmu_client_set_hwgrp(struct smmu_client *c,
> > -				   unsigned long map, int on)
> > +				   u64 map, int on)
> >  {
> >  	int i;
> >  	struct smmu_as *as = c->as;
> > @@ -398,12 +400,11 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
> >  	if (!on)
> >  		map = smmu_client_hwgrp(c);
> >  
> > -	for_each_set_bit(i, &map, HWGRP_COUNT) {
> > +	for_each_set_bit(i, (unsigned long *)&map,
> > +			 sizeof(map) * BITS_PER_BYTE) {
> 
> Why change the type if it just forces you to add this cast?

u32 map; -> u64 map;

for_each_set_bit() expects "unsigned long *" for any length of bitmap.

Re: [PATCH v2 16/22] iommu/tegra: smmu: Get "nvidia,swgroup" from DT

[Stephen Warren]

On 07/29/2013 05:39 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Thu, 18 Jul 2013 22:28:42 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> This provides the info about which H/W Accelerators are supported on
>>> Tegra SoC. This info is passed from DT. This is necessary to have the
>>> unified SMMU driver among Tegra SoCs. Instead of using platform data,
>>> DT passes "nvidia,swgroup" now. DT is mandatory in Tegra.
>>
>>> diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
>>
>>> +- nvidia,swgroups: A bitmap of supported HardWare Accelerators(HWA).
>>> +  Each bit represents one swgroup. The assignments may be found in header
>>> +  file <dt-bindings/memory/tegra-swgroup.h>.
>>
>> There needs to be a default for this field if one is not specified so
>> that existing DTs continue to work without modification.
> 
> Only enabling PPCS(AHB) can be an option because PPCS has SD/MMC where
> rootfs can be located ususally.

There's no reason that the root filesystem has to be on SD/MMC. Either
way, the DT binding shouldn't be influenced by the root fs location at all.

I think more explanation of exactly what this property does and why is
required.

>> How many cells big is this property?
> 
> 64

I assume that's bits, so 2 cells? To be clear: the document needs to
include this information, not just this email thread.

>> Is this really a bitmap of HWAs? Surely it's a bitmap of SMMU client
>> IDs?
> 
> At least this info can be used for PMC too.

How and why? A complete explanation of how the SMMU and PMC are expected
to interact is required.

The PMC DT binding should include all information related to the PMC;
the binding definitions probably shouldn't expect a PMC driver to go
grovelling in an SMMU node to find information.

> .....
>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>
>>> @@ -265,7 +265,7 @@ struct smmu_client {
>>>  	struct device		*dev;
>>>  	struct list_head	list;
>>>  	struct smmu_as		*as;
>>> -	u32			hwgrp;
>>> +	u64			hwgrp;
>>
>> Why is that "hwgrp" not "swgrp"? Don't they represent the same
>> thing?
> 
> They are same but initial SMMU driver used the term "hwgroup". Should
> this be renamed with another patch or can it be left as it is?

I thought there had already been a patch to do this rename. Was it not
complete? If so, that work should probably be completed.

> ....
>>>  static int __smmu_client_set_hwgrp(struct smmu_client *c,
>>> -				   unsigned long map, int on)
>>> +				   u64 map, int on)
>>>  {
>>>  	int i;
>>>  	struct smmu_as *as = c->as;
>>> @@ -398,12 +400,11 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
>>>  	if (!on)
>>>  		map = smmu_client_hwgrp(c);
>>>  
>>> -	for_each_set_bit(i, &map, HWGRP_COUNT) {
>>> +	for_each_set_bit(i, (unsigned long *)&map,
>>> +			 sizeof(map) * BITS_PER_BYTE) {
>>
>> Why change the type if it just forces you to add this cast?
> 
> u32 map; -> u64 map;
> 
> for_each_set_bit() expects "unsigned long *" for any length of bitmap.

Shouldn't the map just be an "unsigned long map[]" then, so no casts are
needed anywhere? Equally, that pointer could just be passed to the
function rather than copying the data to the stack?

[PATCH v2 16/22] iommu/tegra: smmu: Get "nvidia,swgroup" from DT

[Stephen Warren]

On 07/29/2013 05:39 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:28:42 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> This provides the info about which H/W Accelerators are supported on
>>> Tegra SoC. This info is passed from DT. This is necessary to have the
>>> unified SMMU driver among Tegra SoCs. Instead of using platform data,
>>> DT passes "nvidia,swgroup" now. DT is mandatory in Tegra.
>>
>>> diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
>>
>>> +- nvidia,swgroups: A bitmap of supported HardWare Accelerators(HWA).
>>> +  Each bit represents one swgroup. The assignments may be found in header
>>> +  file <dt-bindings/memory/tegra-swgroup.h>.
>>
>> There needs to be a default for this field if one is not specified so
>> that existing DTs continue to work without modification.
> 
> Only enabling PPCS(AHB) can be an option because PPCS has SD/MMC where
> rootfs can be located ususally.

There's no reason that the root filesystem has to be on SD/MMC. Either
way, the DT binding shouldn't be influenced by the root fs location at all.

I think more explanation of exactly what this property does and why is
required.

>> How many cells big is this property?
> 
> 64

I assume that's bits, so 2 cells? To be clear: the document needs to
include this information, not just this email thread.

>> Is this really a bitmap of HWAs? Surely it's a bitmap of SMMU client
>> IDs?
> 
> At least this info can be used for PMC too.

How and why? A complete explanation of how the SMMU and PMC are expected
to interact is required.

The PMC DT binding should include all information related to the PMC;
the binding definitions probably shouldn't expect a PMC driver to go
grovelling in an SMMU node to find information.

> .....
>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>
>>> @@ -265,7 +265,7 @@ struct smmu_client {
>>>  	struct device		*dev;
>>>  	struct list_head	list;
>>>  	struct smmu_as		*as;
>>> -	u32			hwgrp;
>>> +	u64			hwgrp;
>>
>> Why is that "hwgrp" not "swgrp"? Don't they represent the same
>> thing?
> 
> They are same but initial SMMU driver used the term "hwgroup". Should
> this be renamed with another patch or can it be left as it is?

I thought there had already been a patch to do this rename. Was it not
complete? If so, that work should probably be completed.

> ....
>>>  static int __smmu_client_set_hwgrp(struct smmu_client *c,
>>> -				   unsigned long map, int on)
>>> +				   u64 map, int on)
>>>  {
>>>  	int i;
>>>  	struct smmu_as *as = c->as;
>>> @@ -398,12 +400,11 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
>>>  	if (!on)
>>>  		map = smmu_client_hwgrp(c);
>>>  
>>> -	for_each_set_bit(i, &map, HWGRP_COUNT) {
>>> +	for_each_set_bit(i, (unsigned long *)&map,
>>> +			 sizeof(map) * BITS_PER_BYTE) {
>>
>> Why change the type if it just forces you to add this cast?
> 
> u32 map; -> u64 map;
> 
> for_each_set_bit() expects "unsigned long *" for any length of bitmap.

Shouldn't the map just be an "unsigned long map[]" then, so no casts are
needed anywhere? Equally, that pointer could just be passed to the
function rather than copying the data to the stack?

[PATCH v2 17/22] iommu/tegra: smmu: Unfied driver for Tegra SoCs

[Hiroshi Doyu]

Support multiple generation of Tegra SoCs with this unified SMMU
driver. Necessary info is expected to be passed from DT so that this
patch can remove obsolete code now.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c |   76 +-------------------------------------------
 1 file changed, 1 insertion(+), 75 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index c7b33f2..f566972 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -41,46 +41,6 @@
 #include <asm/cacheflush.h>
 #include <asm/dma-iommu.h>
 
-enum smmu_hwgrp {
-	HWGRP_AFI,
-	HWGRP_AVPC,
-	HWGRP_DC,
-	HWGRP_DCB,
-	HWGRP_EPP,
-	HWGRP_G2,
-	HWGRP_HC,
-	HWGRP_HDA,
-	HWGRP_ISP,
-	HWGRP_MPE,
-	HWGRP_NV,
-	HWGRP_NV2,
-	HWGRP_PPCS,
-	HWGRP_SATA,
-	HWGRP_VDE,
-	HWGRP_VI,
-
-	HWGRP_COUNT,
-
-	HWGRP_END = ~0,
-};
-
-#define HWG_AFI		(1 << HWGRP_AFI)
-#define HWG_AVPC	(1 << HWGRP_AVPC)
-#define HWG_DC		(1 << HWGRP_DC)
-#define HWG_DCB		(1 << HWGRP_DCB)
-#define HWG_EPP		(1 << HWGRP_EPP)
-#define HWG_G2		(1 << HWGRP_G2)
-#define HWG_HC		(1 << HWGRP_HC)
-#define HWG_HDA		(1 << HWGRP_HDA)
-#define HWG_ISP		(1 << HWGRP_ISP)
-#define HWG_MPE		(1 << HWGRP_MPE)
-#define HWG_NV		(1 << HWGRP_NV)
-#define HWG_NV2		(1 << HWGRP_NV2)
-#define HWG_PPCS	(1 << HWGRP_PPCS)
-#define HWG_SATA	(1 << HWGRP_SATA)
-#define HWG_VDE		(1 << HWGRP_VDE)
-#define HWG_VI		(1 << HWGRP_VI)
-
 /* bitmap of the page sizes currently supported */
 #define SMMU_IOMMU_PGSIZES	(SZ_4K)
 
@@ -150,21 +110,7 @@ enum {
 #define SMMU_TRANSLATION_ENABLE_2		0x230
 
 #define SMMU_AFI_ASID	0x238   /* PCIE */
-#define SMMU_AVPC_ASID	0x23c   /* AVP */
-#define SMMU_DC_ASID	0x240   /* Display controller */
-#define SMMU_DCB_ASID	0x244   /* Display controller B */
-#define SMMU_EPP_ASID	0x248   /* Encoder pre-processor */
-#define SMMU_G2_ASID	0x24c   /* 2D engine */
-#define SMMU_HC_ASID	0x250   /* Host1x */
-#define SMMU_HDA_ASID	0x254   /* High-def audio */
-#define SMMU_ISP_ASID	0x258   /* Image signal processor */
-#define SMMU_MPE_ASID	0x264   /* MPEG encoder */
-#define SMMU_NV_ASID	0x268   /* (3D) */
-#define SMMU_NV2_ASID	0x26c   /* (3D) */
-#define SMMU_PPCS_ASID	0x270   /* AHB */
-#define SMMU_SATA_ASID	0x278   /* SATA */
-#define SMMU_VDE_ASID	0x27c   /* Video decoder */
-#define SMMU_VI_ASID	0x280   /* Video input */
+#define SMMU_SWGRP_ASID_BASE	SMMU_AFI_ASID
 
 #define SMMU_PDE_NEXT_SHIFT		28
 
@@ -236,26 +182,6 @@ enum {
 #define __smmu_client_enable_hwgrp(c, m) __smmu_client_set_hwgrp(c, m, 1)
 #define __smmu_client_disable_hwgrp(c)	__smmu_client_set_hwgrp(c, 0, 0)
 
-#define HWGRP_INIT(client) [HWGRP_##client] = SMMU_##client##_ASID
-
-static const u32 smmu_hwgrp_asid_reg[] = {
-	HWGRP_INIT(AFI),
-	HWGRP_INIT(AVPC),
-	HWGRP_INIT(DC),
-	HWGRP_INIT(DCB),
-	HWGRP_INIT(EPP),
-	HWGRP_INIT(G2),
-	HWGRP_INIT(HC),
-	HWGRP_INIT(HDA),
-	HWGRP_INIT(ISP),
-	HWGRP_INIT(MPE),
-	HWGRP_INIT(NV),
-	HWGRP_INIT(NV2),
-	HWGRP_INIT(PPCS),
-	HWGRP_INIT(SATA),
-	HWGRP_INIT(VDE),
-	HWGRP_INIT(VI),
-};
 #define HWGRP_ASID_REG(x) ((x) * sizeof(u32) + SMMU_AFI_ASID)
 
 /*
-- 
1.7.9.5

[PATCH v2 17/22] iommu/tegra: smmu: Unfied driver for Tegra SoCs

[Hiroshi Doyu]

Support multiple generation of Tegra SoCs with this unified SMMU
driver. Necessary info is expected to be passed from DT so that this
patch can remove obsolete code now.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |   76 +-------------------------------------------
 1 file changed, 1 insertion(+), 75 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index c7b33f2..f566972 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -41,46 +41,6 @@
 #include <asm/cacheflush.h>
 #include <asm/dma-iommu.h>
 
-enum smmu_hwgrp {
-	HWGRP_AFI,
-	HWGRP_AVPC,
-	HWGRP_DC,
-	HWGRP_DCB,
-	HWGRP_EPP,
-	HWGRP_G2,
-	HWGRP_HC,
-	HWGRP_HDA,
-	HWGRP_ISP,
-	HWGRP_MPE,
-	HWGRP_NV,
-	HWGRP_NV2,
-	HWGRP_PPCS,
-	HWGRP_SATA,
-	HWGRP_VDE,
-	HWGRP_VI,
-
-	HWGRP_COUNT,
-
-	HWGRP_END = ~0,
-};
-
-#define HWG_AFI		(1 << HWGRP_AFI)
-#define HWG_AVPC	(1 << HWGRP_AVPC)
-#define HWG_DC		(1 << HWGRP_DC)
-#define HWG_DCB		(1 << HWGRP_DCB)
-#define HWG_EPP		(1 << HWGRP_EPP)
-#define HWG_G2		(1 << HWGRP_G2)
-#define HWG_HC		(1 << HWGRP_HC)
-#define HWG_HDA		(1 << HWGRP_HDA)
-#define HWG_ISP		(1 << HWGRP_ISP)
-#define HWG_MPE		(1 << HWGRP_MPE)
-#define HWG_NV		(1 << HWGRP_NV)
-#define HWG_NV2		(1 << HWGRP_NV2)
-#define HWG_PPCS	(1 << HWGRP_PPCS)
-#define HWG_SATA	(1 << HWGRP_SATA)
-#define HWG_VDE		(1 << HWGRP_VDE)
-#define HWG_VI		(1 << HWGRP_VI)
-
 /* bitmap of the page sizes currently supported */
 #define SMMU_IOMMU_PGSIZES	(SZ_4K)
 
@@ -150,21 +110,7 @@ enum {
 #define SMMU_TRANSLATION_ENABLE_2		0x230
 
 #define SMMU_AFI_ASID	0x238   /* PCIE */
-#define SMMU_AVPC_ASID	0x23c   /* AVP */
-#define SMMU_DC_ASID	0x240   /* Display controller */
-#define SMMU_DCB_ASID	0x244   /* Display controller B */
-#define SMMU_EPP_ASID	0x248   /* Encoder pre-processor */
-#define SMMU_G2_ASID	0x24c   /* 2D engine */
-#define SMMU_HC_ASID	0x250   /* Host1x */
-#define SMMU_HDA_ASID	0x254   /* High-def audio */
-#define SMMU_ISP_ASID	0x258   /* Image signal processor */
-#define SMMU_MPE_ASID	0x264   /* MPEG encoder */
-#define SMMU_NV_ASID	0x268   /* (3D) */
-#define SMMU_NV2_ASID	0x26c   /* (3D) */
-#define SMMU_PPCS_ASID	0x270   /* AHB */
-#define SMMU_SATA_ASID	0x278   /* SATA */
-#define SMMU_VDE_ASID	0x27c   /* Video decoder */
-#define SMMU_VI_ASID	0x280   /* Video input */
+#define SMMU_SWGRP_ASID_BASE	SMMU_AFI_ASID
 
 #define SMMU_PDE_NEXT_SHIFT		28
 
@@ -236,26 +182,6 @@ enum {
 #define __smmu_client_enable_hwgrp(c, m) __smmu_client_set_hwgrp(c, m, 1)
 #define __smmu_client_disable_hwgrp(c)	__smmu_client_set_hwgrp(c, 0, 0)
 
-#define HWGRP_INIT(client) [HWGRP_##client] = SMMU_##client##_ASID
-
-static const u32 smmu_hwgrp_asid_reg[] = {
-	HWGRP_INIT(AFI),
-	HWGRP_INIT(AVPC),
-	HWGRP_INIT(DC),
-	HWGRP_INIT(DCB),
-	HWGRP_INIT(EPP),
-	HWGRP_INIT(G2),
-	HWGRP_INIT(HC),
-	HWGRP_INIT(HDA),
-	HWGRP_INIT(ISP),
-	HWGRP_INIT(MPE),
-	HWGRP_INIT(NV),
-	HWGRP_INIT(NV2),
-	HWGRP_INIT(PPCS),
-	HWGRP_INIT(SATA),
-	HWGRP_INIT(VDE),
-	HWGRP_INIT(VI),
-};
 #define HWGRP_ASID_REG(x) ((x) * sizeof(u32) + SMMU_AFI_ASID)
 
 /*
-- 
1.7.9.5

[PATCH v2 18/22] iommu/tegra: smmu: Use dt-bindings MACRO

[Hiroshi Doyu]

Use dt-bindings MACRO instead of SoC dependent MACROs.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index f566972..33fb7eb 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -41,6 +41,8 @@
 #include <asm/cacheflush.h>
 #include <asm/dma-iommu.h>
 
+#include <dt-bindings/memory/tegra-swgroup.h>
+
 /* bitmap of the page sizes currently supported */
 #define SMMU_IOMMU_PGSIZES	(SZ_4K)
 
@@ -744,7 +746,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 	 * Reserve "page zero" for AVP vectors using a common dummy
 	 * page.
 	 */
-	if (map & HWG_AVPC) {
+	if (map & TEGRA_SWGROUP_BIT(AVPC)) {
 		struct page *page;
 
 		page = as->smmu->avp_vector_page;
-- 
1.7.9.5

[PATCH v2 18/22] iommu/tegra: smmu: Use dt-bindings MACRO

[Hiroshi Doyu]

Use dt-bindings MACRO instead of SoC dependent MACROs.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index f566972..33fb7eb 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -41,6 +41,8 @@
 #include <asm/cacheflush.h>
 #include <asm/dma-iommu.h>
 
+#include <dt-bindings/memory/tegra-swgroup.h>
+
 /* bitmap of the page sizes currently supported */
 #define SMMU_IOMMU_PGSIZES	(SZ_4K)
 
@@ -744,7 +746,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 	 * Reserve "page zero" for AVP vectors using a common dummy
 	 * page.
 	 */
-	if (map & HWG_AVPC) {
+	if (map & TEGRA_SWGROUP_BIT(AVPC)) {
 		struct page *page;
 
 		page = as->smmu->avp_vector_page;
-- 
1.7.9.5

Re: [PATCH v2 18/22] iommu/tegra: smmu: Use dt-bindings MACRO

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Use dt-bindings MACRO instead of SoC dependent MACROs.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> -	if (map & HWG_AVPC) {
> +	if (map & TEGRA_SWGROUP_BIT(AVPC)) {

Patch 17 removes the definition of HWG_AVPC. So, git bisect is broken.

[PATCH v2 18/22] iommu/tegra: smmu: Use dt-bindings MACRO

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Use dt-bindings MACRO instead of SoC dependent MACROs.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> -	if (map & HWG_AVPC) {
> +	if (map & TEGRA_SWGROUP_BIT(AVPC)) {

Patch 17 removes the definition of HWG_AVPC. So, git bisect is broken.

[PATCH v2 19/22] iommu/tegra: smmu: Workaround PCIe IOMMU'able

[Hiroshi Doyu]

Make PCIe work as it is. IOMMU support can be implemented later. We
need the same kind of registration framework for PCIe as platform_bus
does currently.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 33fb7eb..ac5d661 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -330,6 +330,9 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 
 	for_each_set_bit(i, (unsigned long *)&map,
 			 sizeof(map) * BITS_PER_BYTE) {
+		if (i == TEGRA_SWGROUP_AFI) /* FIXME: IOMMU'able PCIe */
+			continue;
+
 		offs = HWGRP_ASID_REG(i);
 		val = smmu_read(smmu, offs);
 		if (on) {
-- 
1.7.9.5

[PATCH v2 19/22] iommu/tegra: smmu: Workaround PCIe IOMMU'able

[Hiroshi Doyu]

Make PCIe work as it is. IOMMU support can be implemented later. We
need the same kind of registration framework for PCIe as platform_bus
does currently.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 33fb7eb..ac5d661 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -330,6 +330,9 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 
 	for_each_set_bit(i, (unsigned long *)&map,
 			 sizeof(map) * BITS_PER_BYTE) {
+		if (i == TEGRA_SWGROUP_AFI) /* FIXME: IOMMU'able PCIe */
+			continue;
+
 		offs = HWGRP_ASID_REG(i);
 		val = smmu_read(smmu, offs);
 		if (on) {
-- 
1.7.9.5

Re: [PATCH v2 19/22] iommu/tegra: smmu: Workaround PCIe IOMMU'able

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Make PCIe work as it is. IOMMU support can be implemented later. We
> need the same kind of registration framework for PCIe as platform_bus
> does currently.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  	for_each_set_bit(i, (unsigned long *)&map,
>  			 sizeof(map) * BITS_PER_BYTE) {
> +		if (i == TEGRA_SWGROUP_AFI) /* FIXME: IOMMU'able PCIe */
> +			continue;

Isn't it just a fluke that TEGRA_SWGROUP_AFI is defined identically for
the SoCs that we support upstream?

If this change is required, don't we need it before the SMMU starts
enabling itself for clients, so that git bisect is maintained?

By deferring the question of registering the IOMMU for PCIe, ar we going
to require non-backwards-compatible changes to DT for the PCIe
controller at some later point in time?

[PATCH v2 19/22] iommu/tegra: smmu: Workaround PCIe IOMMU'able

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Make PCIe work as it is. IOMMU support can be implemented later. We
> need the same kind of registration framework for PCIe as platform_bus
> does currently.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  	for_each_set_bit(i, (unsigned long *)&map,
>  			 sizeof(map) * BITS_PER_BYTE) {
> +		if (i == TEGRA_SWGROUP_AFI) /* FIXME: IOMMU'able PCIe */
> +			continue;

Isn't it just a fluke that TEGRA_SWGROUP_AFI is defined identically for
the SoCs that we support upstream?

If this change is required, don't we need it before the SMMU starts
enabling itself for clients, so that git bisect is maintained?

By deferring the question of registering the IOMMU for PCIe, ar we going
to require non-backwards-compatible changes to DT for the PCIe
controller at some later point in time?

[PATCH v2 20/22] iommu/tegra: smmu: Get "nvidia,memory-client" from DT

[Hiroshi Doyu]

This provides the info about which swgroups a device belongs to. This
info is passed from DT. This is necessary for the unified SMMU driver
among Tegra SoCs since each has different H/W accelerators.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 .../bindings/iommu/nvidia,tegra30-smmu.txt         |   12 ++++++++++++
 drivers/iommu/tegra-smmu.c                         |   20 +++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
index 0c14dca..20ce1fb 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 bitmap of supported HardWare Accelerators(HWA).
   Each bit represents one swgroup. The assignments may be found in header
   file <dt-bindings/memory/tegra-swgroup.h>.
+- nvidia,memory-client: Indicates which swgroups a device belongs to.
 
 Example:
 	iommu {
@@ -23,3 +24,14 @@ Example:
 		nvidia,swgroups = TEGRA30_SWGROUP_ALL;
 		nvidia,ahb = <&ahb>;
 	};
+
+	host1x {
+		compatible = "nvidia,tegra30-host1x", "simple-bus";
+		nvidia,memory-client = <TEGRA_SWGROUP_HC>;
+		....
+		gr3d {
+			compatible = "nvidia,tegra30-gr3d";
+			nvidia,memory-client = <TEGRA_SWGROUP_NV
+						TEGRA_SWGROUP_NV2>;
+			....
+		};
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index ac5d661..8a9434e 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -314,6 +314,24 @@ static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
 
 #define smmu_client_hwgrp(c)	(c->as->smmu->swgroup)
 
+static u64 smmu_of_get_memory_client(struct device *dev)
+{
+	size_t bytes;
+	const char *propname = "nvidia,memory-client";
+	const __be32 *prop;
+	int i;
+	u64 swgroup = 0;
+
+	prop = of_get_property(dev->of_node, propname, &bytes);
+	if (!prop || !bytes)
+		return 0;
+
+	for (i = 0; i < bytes / sizeof(u32); i++, prop++)
+		swgroup |= 1ULL << be32_to_cpup(prop);
+
+	return swgroup;
+}
+
 static int __smmu_client_set_hwgrp(struct smmu_client *c,
 				   u64 map, int on)
 {
@@ -725,7 +743,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 		return -ENOMEM;
 	client->dev = dev;
 	client->as = as;
-	map = smmu->swgroup;
+	map = smmu_of_get_memory_client(dev);
 	if (!map)
 		return -EINVAL;
 
-- 
1.7.9.5

[PATCH v2 20/22] iommu/tegra: smmu: Get "nvidia, memory-client" from DT

[Hiroshi Doyu]

This provides the info about which swgroups a device belongs to. This
info is passed from DT. This is necessary for the unified SMMU driver
among Tegra SoCs since each has different H/W accelerators.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 .../bindings/iommu/nvidia,tegra30-smmu.txt         |   12 ++++++++++++
 drivers/iommu/tegra-smmu.c                         |   20 +++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
index 0c14dca..20ce1fb 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 bitmap of supported HardWare Accelerators(HWA).
   Each bit represents one swgroup. The assignments may be found in header
   file <dt-bindings/memory/tegra-swgroup.h>.
+- nvidia,memory-client: Indicates which swgroups a device belongs to.
 
 Example:
 	iommu {
@@ -23,3 +24,14 @@ Example:
 		nvidia,swgroups = TEGRA30_SWGROUP_ALL;
 		nvidia,ahb = <&ahb>;
 	};
+
+	host1x {
+		compatible = "nvidia,tegra30-host1x", "simple-bus";
+		nvidia,memory-client = <TEGRA_SWGROUP_HC>;
+		....
+		gr3d {
+			compatible = "nvidia,tegra30-gr3d";
+			nvidia,memory-client = <TEGRA_SWGROUP_NV
+						TEGRA_SWGROUP_NV2>;
+			....
+		};
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index ac5d661..8a9434e 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -314,6 +314,24 @@ static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
 
 #define smmu_client_hwgrp(c)	(c->as->smmu->swgroup)
 
+static u64 smmu_of_get_memory_client(struct device *dev)
+{
+	size_t bytes;
+	const char *propname = "nvidia,memory-client";
+	const __be32 *prop;
+	int i;
+	u64 swgroup = 0;
+
+	prop = of_get_property(dev->of_node, propname, &bytes);
+	if (!prop || !bytes)
+		return 0;
+
+	for (i = 0; i < bytes / sizeof(u32); i++, prop++)
+		swgroup |= 1ULL << be32_to_cpup(prop);
+
+	return swgroup;
+}
+
 static int __smmu_client_set_hwgrp(struct smmu_client *c,
 				   u64 map, int on)
 {
@@ -725,7 +743,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 		return -ENOMEM;
 	client->dev = dev;
 	client->as = as;
-	map = smmu->swgroup;
+	map = smmu_of_get_memory_client(dev);
 	if (!map)
 		return -EINVAL;
 
-- 
1.7.9.5

Re: [PATCH v2 20/22] iommu/tegra: smmu: Get "nvidia,memory-client" from DT

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> This provides the info about which swgroups a device belongs to. This
> info is passed from DT. This is necessary for the unified SMMU driver
> among Tegra SoCs since each has different H/W accelerators.

> diff --git 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 bitmap of supported HardWare Accelerators(HWA).
>    Each bit represents one swgroup. The assignments may be found in header
>    file <dt-bindings/memory/tegra-swgroup.h>.
> +- nvidia,memory-client: Indicates which swgroups a device belongs to.

The list of properties you're adding to is for the SMMU node itself. If
specific properties need to exist in SMMU client nodes, you need to
create a separate section/list in the document for that. For example,
see the clear distinction between sections (1) and (2) in
Documentation/devicetree/bindings/gpio/gpio.txt.

What is the format/size of data in this property?

From the example below:

> +			nvidia,memory-client = <TEGRA_SWGROUP_NV
> +						TEGRA_SWGROUP_NV2>;

... it seems like multiple entries are allowed. Should the property be
named "nvidia,memory-clients" instead (a plural)? "memory-clients" seems
like new terminology that exists just for this property. Is "swgroups"
the correct term here?

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> +static u64 smmu_of_get_memory_client(struct device *dev)

> +	for (i = 0; i < bytes / sizeof(u32); i++, prop++)
> +		swgroup |= 1ULL << be32_to_cpup(prop);

Oh, so it's a list of bit numbers instead of a list of masks. Definitely
good to specify in the binding doc!

[PATCH v2 20/22] iommu/tegra: smmu: Get "nvidia,memory-client" from DT

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> This provides the info about which swgroups a device belongs to. This
> info is passed from DT. This is necessary for the unified SMMU driver
> among Tegra SoCs since each has different H/W accelerators.

> diff --git 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 bitmap of supported HardWare Accelerators(HWA).
>    Each bit represents one swgroup. The assignments may be found in header
>    file <dt-bindings/memory/tegra-swgroup.h>.
> +- nvidia,memory-client: Indicates which swgroups a device belongs to.

The list of properties you're adding to is for the SMMU node itself. If
specific properties need to exist in SMMU client nodes, you need to
create a separate section/list in the document for that. For example,
see the clear distinction between sections (1) and (2) in
Documentation/devicetree/bindings/gpio/gpio.txt.

What is the format/size of data in this property?

>From the example below:

> +			nvidia,memory-client = <TEGRA_SWGROUP_NV
> +						TEGRA_SWGROUP_NV2>;

... it seems like multiple entries are allowed. Should the property be
named "nvidia,memory-clients" instead (a plural)? "memory-clients" seems
like new terminology that exists just for this property. Is "swgroups"
the correct term here?

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> +static u64 smmu_of_get_memory_client(struct device *dev)

> +	for (i = 0; i < bytes / sizeof(u32); i++, prop++)
> +		swgroup |= 1ULL << be32_to_cpup(prop);

Oh, so it's a list of bit numbers instead of a list of masks. Definitely
good to specify in the binding doc!

Re: [PATCH v2 20/22] iommu/tegra: smmu: Get "nvidia,memory-client" from DT

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> This provides the info about which swgroups a device belongs to. This
> info is passed from DT. This is necessary for the unified SMMU driver
> among Tegra SoCs since each has different H/W accelerators.

> diff --git 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 bitmap of supported HardWare Accelerators(HWA).
>    Each bit represents one swgroup. The assignments may be found in header
>    file <dt-bindings/memory/tegra-swgroup.h>.
> +- nvidia,memory-client: Indicates which swgroups a device belongs to.

The list of properties you're adding to is for the SMMU node itself. If
specific properties need to exist in SMMU client nodes, you need to
create a separate section/list in the document for that. For example,
see the clear distinction between sections (1) and (2) in
Documentation/devicetree/bindings/gpio/gpio.txt.

What is the format/size of data in this property?

>From the example below:

> +			nvidia,memory-client = <TEGRA_SWGROUP_NV
> +						TEGRA_SWGROUP_NV2>;

... it seems like multiple entries are allowed. Should the property be
named "nvidia,memory-clients" instead (a plural)? "memory-clients" seems
like new terminology that exists just for this property. Is "swgroups"
the correct term here?

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> +static u64 smmu_of_get_memory_client(struct device *dev)

> +	for (i = 0; i < bytes / sizeof(u32); i++, prop++)
> +		swgroup |= 1ULL << be32_to_cpup(prop);

Oh, so it's a list of bit numbers instead of a list of masks. Definitely
good to specify in the binding doc!

[PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Hiroshi Doyu]

Support Multiple Address Space(AS). Tegra SMMU can have multiple
ASes. We reserve 2 of them for static assignment, AS[0] for system
default, AS[1] for AHB clusters as protected domain from others, where
there are many traditional pheripheral devices like USB, SD/MMC. They
should be isolated from some smart devices like host1x for system
robustness. Even if smart devices behaves wrongly, the traditional
devices(SD/MMC, USB) wouldn't be affected, and the system could
continue most likely.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 8a9434e..1945815 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -904,7 +904,18 @@ enum {
 static int smmu_iommu_add_device(struct device *dev)
 {
 	int err;
-	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
+	u64 swgroup;
+	struct dma_iommu_mapping *map = NULL;
+
+	swgroup = smmu_of_get_memory_client(dev);
+	switch (swgroup) {
+	case TEGRA_SWGROUP_BIT(PPCS):
+		map = smmu_handle->map[SYSTEM_PROTECTED];
+		break;
+	default:
+		map = smmu_handle->map[SYSTEM_DEFAULT];
+		break;
+	}
 
 	if (!map)
 		goto out;
@@ -915,7 +926,7 @@ static int smmu_iommu_add_device(struct device *dev)
 		return err;
 	}
 out:
-	dev_dbg(dev, "Attached to map %p\n", map);
+	dev_dbg(dev, "Attached to map %p, swgroup:%08llx\n", map, swgroup);
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Hiroshi Doyu]

Support Multiple Address Space(AS). Tegra SMMU can have multiple
ASes. We reserve 2 of them for static assignment, AS[0] for system
default, AS[1] for AHB clusters as protected domain from others, where
there are many traditional pheripheral devices like USB, SD/MMC. They
should be isolated from some smart devices like host1x for system
robustness. Even if smart devices behaves wrongly, the traditional
devices(SD/MMC, USB) wouldn't be affected, and the system could
continue most likely.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 8a9434e..1945815 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -904,7 +904,18 @@ enum {
 static int smmu_iommu_add_device(struct device *dev)
 {
 	int err;
-	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
+	u64 swgroup;
+	struct dma_iommu_mapping *map = NULL;
+
+	swgroup = smmu_of_get_memory_client(dev);
+	switch (swgroup) {
+	case TEGRA_SWGROUP_BIT(PPCS):
+		map = smmu_handle->map[SYSTEM_PROTECTED];
+		break;
+	default:
+		map = smmu_handle->map[SYSTEM_DEFAULT];
+		break;
+	}
 
 	if (!map)
 		goto out;
@@ -915,7 +926,7 @@ static int smmu_iommu_add_device(struct device *dev)
 		return err;
 	}
 out:
-	dev_dbg(dev, "Attached to map %p\n", map);
+	dev_dbg(dev, "Attached to map %p, swgroup:%08llx\n", map, swgroup);
 	return 0;
 }
 
-- 
1.7.9.5

Re: [PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Support Multiple Address Space(AS). Tegra SMMU can have multiple
> ASes. We reserve 2 of them for static assignment, AS[0] for system
> default, AS[1] for AHB clusters as protected domain from others, where
> there are many traditional pheripheral devices like USB, SD/MMC. They
> should be isolated from some smart devices like host1x for system
> robustness. Even if smart devices behaves wrongly, the traditional
> devices(SD/MMC, USB) wouldn't be affected, and the system could
> continue most likely.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  static int smmu_iommu_add_device(struct device *dev)
>  {
>  	int err;
> -	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
> +	u64 swgroup;
> +	struct dma_iommu_mapping *map = NULL;
> +
> +	swgroup = smmu_of_get_memory_client(dev);
> +	switch (swgroup) {
> +	case TEGRA_SWGROUP_BIT(PPCS):
> +		map = smmu_handle->map[SYSTEM_PROTECTED];
> +		break;
> +	default:
> +		map = smmu_handle->map[SYSTEM_DEFAULT];

I already mentioned this, but just for completeness: What is
smmu->num_as <= SYSTEM_DEFAULT?

[PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Support Multiple Address Space(AS). Tegra SMMU can have multiple
> ASes. We reserve 2 of them for static assignment, AS[0] for system
> default, AS[1] for AHB clusters as protected domain from others, where
> there are many traditional pheripheral devices like USB, SD/MMC. They
> should be isolated from some smart devices like host1x for system
> robustness. Even if smart devices behaves wrongly, the traditional
> devices(SD/MMC, USB) wouldn't be affected, and the system could
> continue most likely.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  static int smmu_iommu_add_device(struct device *dev)
>  {
>  	int err;
> -	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
> +	u64 swgroup;
> +	struct dma_iommu_mapping *map = NULL;
> +
> +	swgroup = smmu_of_get_memory_client(dev);
> +	switch (swgroup) {
> +	case TEGRA_SWGROUP_BIT(PPCS):
> +		map = smmu_handle->map[SYSTEM_PROTECTED];
> +		break;
> +	default:
> +		map = smmu_handle->map[SYSTEM_DEFAULT];

I already mentioned this, but just for completeness: What is
smmu->num_as <= SYSTEM_DEFAULT?

Re: [PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Hiroshi Doyu]

Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Thu, 18 Jul 2013 22:43:06 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Support Multiple Address Space(AS). Tegra SMMU can have multiple
> > ASes. We reserve 2 of them for static assignment, AS[0] for system
> > default, AS[1] for AHB clusters as protected domain from others, where
> > there are many traditional pheripheral devices like USB, SD/MMC. They
> > should be isolated from some smart devices like host1x for system
> > robustness. Even if smart devices behaves wrongly, the traditional
> > devices(SD/MMC, USB) wouldn't be affected, and the system could
> > continue most likely.
> 
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> >  static int smmu_iommu_add_device(struct device *dev)
> >  {
> >  	int err;
> > -	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
> > +	u64 swgroup;
> > +	struct dma_iommu_mapping *map = NULL;
> > +
> > +	swgroup = smmu_of_get_memory_client(dev);
> > +	switch (swgroup) {
> > +	case TEGRA_SWGROUP_BIT(PPCS):
> > +		map = smmu_handle->map[SYSTEM_PROTECTED];
> > +		break;
> > +	default:
> > +		map = smmu_handle->map[SYSTEM_DEFAULT];
> 
> I already mentioned this, but just for completeness: What is
> smmu->num_as <= SYSTEM_DEFAULT?

I think that this belongs to the system operation policy. Which H/W
should be configured to which Address Space(AS). This put all AHB
clients(PPCS) into AS[1](SYSTEM_PROTECTED), and the rest into
AS[0](SYSTEM_DEFAULT). AHB clients are mainly traditional H/Ws like
USB and SD/MMC so that they should be kept separated from the smart
IOMMU clients like host1x.

Is there any place to configure this kind of board specific policy
rather than here?

[PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:43:06 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Support Multiple Address Space(AS). Tegra SMMU can have multiple
> > ASes. We reserve 2 of them for static assignment, AS[0] for system
> > default, AS[1] for AHB clusters as protected domain from others, where
> > there are many traditional pheripheral devices like USB, SD/MMC. They
> > should be isolated from some smart devices like host1x for system
> > robustness. Even if smart devices behaves wrongly, the traditional
> > devices(SD/MMC, USB) wouldn't be affected, and the system could
> > continue most likely.
> 
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> >  static int smmu_iommu_add_device(struct device *dev)
> >  {
> >  	int err;
> > -	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
> > +	u64 swgroup;
> > +	struct dma_iommu_mapping *map = NULL;
> > +
> > +	swgroup = smmu_of_get_memory_client(dev);
> > +	switch (swgroup) {
> > +	case TEGRA_SWGROUP_BIT(PPCS):
> > +		map = smmu_handle->map[SYSTEM_PROTECTED];
> > +		break;
> > +	default:
> > +		map = smmu_handle->map[SYSTEM_DEFAULT];
> 
> I already mentioned this, but just for completeness: What is
> smmu->num_as <= SYSTEM_DEFAULT?

I think that this belongs to the system operation policy. Which H/W
should be configured to which Address Space(AS). This put all AHB
clients(PPCS) into AS[1](SYSTEM_PROTECTED), and the rest into
AS[0](SYSTEM_DEFAULT). AHB clients are mainly traditional H/Ws like
USB and SD/MMC so that they should be kept separated from the smart
IOMMU clients like host1x.

Is there any place to configure this kind of board specific policy
rather than here?

Re: [PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Stephen Warren]

On 07/29/2013 04:31 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Thu, 18 Jul 2013 22:43:06 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Support Multiple Address Space(AS). Tegra SMMU can have multiple
>>> ASes. We reserve 2 of them for static assignment, AS[0] for system
>>> default, AS[1] for AHB clusters as protected domain from others, where
>>> there are many traditional pheripheral devices like USB, SD/MMC. They
>>> should be isolated from some smart devices like host1x for system
>>> robustness. Even if smart devices behaves wrongly, the traditional
>>> devices(SD/MMC, USB) wouldn't be affected, and the system could
>>> continue most likely.
>>
>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>
>>>  static int smmu_iommu_add_device(struct device *dev)
>>>  {
>>>  	int err;
>>> -	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
>>> +	u64 swgroup;
>>> +	struct dma_iommu_mapping *map = NULL;
>>> +
>>> +	swgroup = smmu_of_get_memory_client(dev);
>>> +	switch (swgroup) {
>>> +	case TEGRA_SWGROUP_BIT(PPCS):
>>> +		map = smmu_handle->map[SYSTEM_PROTECTED];
>>> +		break;
>>> +	default:
>>> +		map = smmu_handle->map[SYSTEM_DEFAULT];
>>
>> I already mentioned this, but just for completeness: What is
>> smmu->num_as <= SYSTEM_DEFAULT?
> 
> I think that this belongs to the system operation policy. Which H/W
> should be configured to which Address Space(AS). This put all AHB
> clients(PPCS) into AS[1](SYSTEM_PROTECTED), and the rest into
> AS[0](SYSTEM_DEFAULT). AHB clients are mainly traditional H/Ws like
> USB and SD/MMC so that they should be kept separated from the smart
> IOMMU clients like host1x.
> 
> Is there any place to configure this kind of board specific policy
> rather than here?

I'm not sure that answers the question I asked.

I mean that the driver expects that two AS always exist;
SYSTEM_PROTECTED and SYSTEM_DEFAULT. However, the set of extant ASs is
IIRC defined by the DT content. What if the DT doesn't define two ASs?
Shouldn't there at least be an error-check for this case, so the driver
doesn't just blindly continue and access smmu_handle->map[1] when the
map[] array only has 1 entry?

[PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Stephen Warren]

On 07/29/2013 04:31 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:43:06 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Support Multiple Address Space(AS). Tegra SMMU can have multiple
>>> ASes. We reserve 2 of them for static assignment, AS[0] for system
>>> default, AS[1] for AHB clusters as protected domain from others, where
>>> there are many traditional pheripheral devices like USB, SD/MMC. They
>>> should be isolated from some smart devices like host1x for system
>>> robustness. Even if smart devices behaves wrongly, the traditional
>>> devices(SD/MMC, USB) wouldn't be affected, and the system could
>>> continue most likely.
>>
>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>
>>>  static int smmu_iommu_add_device(struct device *dev)
>>>  {
>>>  	int err;
>>> -	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
>>> +	u64 swgroup;
>>> +	struct dma_iommu_mapping *map = NULL;
>>> +
>>> +	swgroup = smmu_of_get_memory_client(dev);
>>> +	switch (swgroup) {
>>> +	case TEGRA_SWGROUP_BIT(PPCS):
>>> +		map = smmu_handle->map[SYSTEM_PROTECTED];
>>> +		break;
>>> +	default:
>>> +		map = smmu_handle->map[SYSTEM_DEFAULT];
>>
>> I already mentioned this, but just for completeness: What is
>> smmu->num_as <= SYSTEM_DEFAULT?
> 
> I think that this belongs to the system operation policy. Which H/W
> should be configured to which Address Space(AS). This put all AHB
> clients(PPCS) into AS[1](SYSTEM_PROTECTED), and the rest into
> AS[0](SYSTEM_DEFAULT). AHB clients are mainly traditional H/Ws like
> USB and SD/MMC so that they should be kept separated from the smart
> IOMMU clients like host1x.
> 
> Is there any place to configure this kind of board specific policy
> rather than here?

I'm not sure that answers the question I asked.

I mean that the driver expects that two AS always exist;
SYSTEM_PROTECTED and SYSTEM_DEFAULT. However, the set of extant ASs is
IIRC defined by the DT content. What if the DT doesn't define two ASs?
Shouldn't there at least be an error-check for this case, so the driver
doesn't just blindly continue and access smmu_handle->map[1] when the
map[] array only has 1 entry?

Re: [PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Hiroshi Doyu]

On Mon, 29 Jul 2013 19:41:23 +0200
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
...
> > I think that this belongs to the system operation policy. Which H/W
> > should be configured to which Address Space(AS). This put all AHB
> > clients(PPCS) into AS[1](SYSTEM_PROTECTED), and the rest into
> > AS[0](SYSTEM_DEFAULT). AHB clients are mainly traditional H/Ws like
> > USB and SD/MMC so that they should be kept separated from the smart
> > IOMMU clients like host1x.
> > 
> > Is there any place to configure this kind of board specific policy
> > rather than here?
> 
> I'm not sure that answers the question I asked.
> 
> I mean that the driver expects that two AS always exist;
> SYSTEM_PROTECTED and SYSTEM_DEFAULT. However, the set of extant ASs is
> IIRC defined by the DT content. What if the DT doesn't define two ASs?
> Shouldn't there at least be an error-check for this case, so the driver
> doesn't just blindly continue and access smmu_handle->map[1] when the
> map[] array only has 1 entry?

OK, now I got it.

We can create multiple maps on the same AS in theory. So the limitation
is only for how many maps S/W creaed. This can/should be checked only
within kernel side. No constraints from DT.

[PATCH v2 21/22] iommu/tegra: smmu: Support Multiple ASID

[Hiroshi Doyu]

On Mon, 29 Jul 2013 19:41:23 +0200
Stephen Warren <swarren@wwwdotorg.org> wrote:
...
> > I think that this belongs to the system operation policy. Which H/W
> > should be configured to which Address Space(AS). This put all AHB
> > clients(PPCS) into AS[1](SYSTEM_PROTECTED), and the rest into
> > AS[0](SYSTEM_DEFAULT). AHB clients are mainly traditional H/Ws like
> > USB and SD/MMC so that they should be kept separated from the smart
> > IOMMU clients like host1x.
> > 
> > Is there any place to configure this kind of board specific policy
> > rather than here?
> 
> I'm not sure that answers the question I asked.
> 
> I mean that the driver expects that two AS always exist;
> SYSTEM_PROTECTED and SYSTEM_DEFAULT. However, the set of extant ASs is
> IIRC defined by the DT content. What if the DT doesn't define two ASs?
> Shouldn't there at least be an error-check for this case, so the driver
> doesn't just blindly continue and access smmu_handle->map[1] when the
> map[] array only has 1 entry?

OK, now I got it.

We can create multiple maps on the same AS in theory. So the limitation
is only for how many maps S/W creaed. This can/should be checked only
within kernel side. No constraints from DT.

[PATCH v2 22/22] ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations

[Hiroshi Doyu]

arm_iommu_alloc_attrs wants to split pages after allocation in order
to reduce the memory footprint. This does not work well with GFP_COMP
pages, so drop this flag before allocation.

(ref: ea2e7057c0234cfb8b09467d8f137760d371fc72)

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/mm/dma-mapping.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7f9b179..cad35a3 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1322,6 +1322,13 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	struct page **pages;
 	void *addr = NULL;
 
+	/* Following is a work-around (a.k.a. hack) to prevent pages
+	 * with __GFP_COMP being passed to split_page() which cannot
+	 * handle them.  The real problem is that this flag probably
+	 * should be 0 on ARM as it is not supported on this
+	 * platform--see CONFIG_HUGETLB_PAGE. */
+	gfp &= ~(__GFP_COMP);
+
 	*handle = DMA_ERROR_CODE;
 	size = PAGE_ALIGN(size);
 
-- 
1.7.9.5

[PATCH v2 22/22] ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations

[Hiroshi Doyu]

arm_iommu_alloc_attrs wants to split pages after allocation in order
to reduce the memory footprint. This does not work well with GFP_COMP
pages, so drop this flag before allocation.

(ref: ea2e7057c0234cfb8b09467d8f137760d371fc72)

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mm/dma-mapping.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7f9b179..cad35a3 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1322,6 +1322,13 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	struct page **pages;
 	void *addr = NULL;
 
+	/* Following is a work-around (a.k.a. hack) to prevent pages
+	 * with __GFP_COMP being passed to split_page() which cannot
+	 * handle them.  The real problem is that this flag probably
+	 * should be 0 on ARM as it is not supported on this
+	 * platform--see CONFIG_HUGETLB_PAGE. */
+	gfp &= ~(__GFP_COMP);
+
 	*handle = DMA_ERROR_CODE;
 	size = PAGE_ALIGN(size);
 
-- 
1.7.9.5

Re: [PATCH v2 22/22] ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> arm_iommu_alloc_attrs wants to split pages after allocation in order
> to reduce the memory footprint. This does not work well with GFP_COMP
> pages, so drop this flag before allocation.
> 
> (ref: ea2e7057c0234cfb8b09467d8f137760d371fc72)

Include the commit description too, so people have a clue what that
commit is.

Presumably this fix can go through the core ARM tree independently from
this series? Or, will this series cause problems if applied without this
patch being applied first?

BTW, I don't see anything in this series that adds the mandatory
properties to DT for all this to work. I assume if I just apply this
series, everything will blow up? If so, it's usual to say "[PATCH RFC"
rather than "[PATCH" in the email subject.

[PATCH v2 22/22] ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations

[Stephen Warren]

On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> arm_iommu_alloc_attrs wants to split pages after allocation in order
> to reduce the memory footprint. This does not work well with GFP_COMP
> pages, so drop this flag before allocation.
> 
> (ref: ea2e7057c0234cfb8b09467d8f137760d371fc72)

Include the commit description too, so people have a clue what that
commit is.

Presumably this fix can go through the core ARM tree independently from
this series? Or, will this series cause problems if applied without this
patch being applied first?

BTW, I don't see anything in this series that adds the mandatory
properties to DT for all this to work. I assume if I just apply this
series, everything will blow up? If so, it's usual to say "[PATCH RFC"
rather than "[PATCH" in the email subject.

Re: [PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Marc Dietrich]

Am Freitag, 5. Juli 2013, 13:44:35 schrieb Hiroshi Doyu:
> Hi,
> 
> This series provides:
> 
> (1) Unified SMMU driver among Tegra SoCs

does this affect tegra2 in any way?

> (2) Multiple Address Space support(MASID) in IOMMU(SMMMU)
> (3) Tegra IOMMU'able devices, most of platform devices are IOMMU'able.
> 
> There's some discussion[1] about device population order, which can
> solve the following patches.
> 
>   [HACK] of: dev_node has struct device pointer
>   ARM: tegra: Populate AHB/IOMMU earlier than others
> 
> Also "ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations" may
> not be necessary by [2]
> 
> Tested IOMMU functionality with T30 SD/MMC. Any further testing with
> T114 and/or other devices would be really appreciated.
> 
> v2:
> Updated based on Thierry Reding's and Stephen Warren's feedback
> 
> v1:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/180267.html
> 
> Hiroshi Doyu (22):
>   [HACK] of: dev_node has struct device pointer
>   ARM: tegra: Populate AHB/IOMMU earlier than others
>   ARM: tegra: Create a DT header defining swgroups ID
>   ARM: dt: tegra30: iommu: Add "nvidia,swgroup"
>   ARM: dt: tegra30: iommu: Add "nvidia,memory-client"
>   ARM: dt: tegra114: iommu: Fix IOMMU register address
>   ARM: dt: tegra114: iommu: Add "nvidia,swgroups"
>   ARM: dt: tegra114: Add "nvidia,memory-client"
>   amba: Move AHB to core_initcall
>   iommu/tegra: smmu: Move IOMMU to core_initcall
>   iommu/tegra: smmu: Add Tegra 114 support
>   iommu/tegra: smmu: Select ARM_DMA_USE_IOMMU in Kconfig
>   iommu/tegra: smmu: Create default IOVA maps
>   iommu/tegra: smmu: Register platform_device to IOMMU dynamically
>   iommu/tegra: smmu: Calculate ASID register offset by ID
>   iommu/tegra: smmu: Get "nvidia,swgroup" from DT
>   iommu/tegra: smmu: Unfied driver for Tegra SoCs
>   iommu/tegra: smmu: Use dt-bindings MACRO
>   iommu/tegra: smmu: Workaround PCIe IOMMU'able
>   iommu/tegra: smmu: Get "nvidia,memory-client" from DT
>   iommu/tegra: smmu: Support Multiple ASID
>   ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations
> 
>  .../bindings/iommu/nvidia,tegra30-smmu.txt         |   20 +-
>  arch/arm/boot/dts/tegra114.dtsi                    |   13 +-
>  arch/arm/boot/dts/tegra30.dtsi                     |   21 ++
>  arch/arm/mach-tegra/tegra.c                        |   22 ++
>  arch/arm/mm/dma-mapping.c                          |    7 +
>  drivers/amba/tegra-ahb.c                           |    7 +-
>  drivers/iommu/Kconfig                              |    1 +
>  drivers/iommu/tegra-smmu.c                         |  211
> +++++++++++--------- drivers/of/base.c                                  |  
> 23 +++
>  drivers/of/platform.c                              |    8 +
>  include/dt-bindings/memory/tegra-swgroup.h         |   50 +++++
>  include/linux/of.h                                 |   16 ++
>  12 files changed, 296 insertions(+), 103 deletions(-)
>  create mode 100644 include/dt-bindings/memory/tegra-swgroup.h

[PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Marc Dietrich]

Am Freitag, 5. Juli 2013, 13:44:35 schrieb Hiroshi Doyu:
> Hi,
> 
> This series provides:
> 
> (1) Unified SMMU driver among Tegra SoCs

does this affect tegra2 in any way?

> (2) Multiple Address Space support(MASID) in IOMMU(SMMMU)
> (3) Tegra IOMMU'able devices, most of platform devices are IOMMU'able.
> 
> There's some discussion[1] about device population order, which can
> solve the following patches.
> 
>   [HACK] of: dev_node has struct device pointer
>   ARM: tegra: Populate AHB/IOMMU earlier than others
> 
> Also "ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations" may
> not be necessary by [2]
> 
> Tested IOMMU functionality with T30 SD/MMC. Any further testing with
> T114 and/or other devices would be really appreciated.
> 
> v2:
> Updated based on Thierry Reding's and Stephen Warren's feedback
> 
> v1:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-June/180267.html
> 
> Hiroshi Doyu (22):
>   [HACK] of: dev_node has struct device pointer
>   ARM: tegra: Populate AHB/IOMMU earlier than others
>   ARM: tegra: Create a DT header defining swgroups ID
>   ARM: dt: tegra30: iommu: Add "nvidia,swgroup"
>   ARM: dt: tegra30: iommu: Add "nvidia,memory-client"
>   ARM: dt: tegra114: iommu: Fix IOMMU register address
>   ARM: dt: tegra114: iommu: Add "nvidia,swgroups"
>   ARM: dt: tegra114: Add "nvidia,memory-client"
>   amba: Move AHB to core_initcall
>   iommu/tegra: smmu: Move IOMMU to core_initcall
>   iommu/tegra: smmu: Add Tegra 114 support
>   iommu/tegra: smmu: Select ARM_DMA_USE_IOMMU in Kconfig
>   iommu/tegra: smmu: Create default IOVA maps
>   iommu/tegra: smmu: Register platform_device to IOMMU dynamically
>   iommu/tegra: smmu: Calculate ASID register offset by ID
>   iommu/tegra: smmu: Get "nvidia,swgroup" from DT
>   iommu/tegra: smmu: Unfied driver for Tegra SoCs
>   iommu/tegra: smmu: Use dt-bindings MACRO
>   iommu/tegra: smmu: Workaround PCIe IOMMU'able
>   iommu/tegra: smmu: Get "nvidia,memory-client" from DT
>   iommu/tegra: smmu: Support Multiple ASID
>   ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations
> 
>  .../bindings/iommu/nvidia,tegra30-smmu.txt         |   20 +-
>  arch/arm/boot/dts/tegra114.dtsi                    |   13 +-
>  arch/arm/boot/dts/tegra30.dtsi                     |   21 ++
>  arch/arm/mach-tegra/tegra.c                        |   22 ++
>  arch/arm/mm/dma-mapping.c                          |    7 +
>  drivers/amba/tegra-ahb.c                           |    7 +-
>  drivers/iommu/Kconfig                              |    1 +
>  drivers/iommu/tegra-smmu.c                         |  211
> +++++++++++--------- drivers/of/base.c                                  |  
> 23 +++
>  drivers/of/platform.c                              |    8 +
>  include/dt-bindings/memory/tegra-swgroup.h         |   50 +++++
>  include/linux/of.h                                 |   16 ++
>  12 files changed, 296 insertions(+), 103 deletions(-)
>  create mode 100644 include/dt-bindings/memory/tegra-swgroup.h

Re: [PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Hiroshi Doyu]

Hi Marc,

Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org> wrote @ Fri, 5 Jul 2013 13:03:58 +0200:

> Am Freitag, 5. Juli 2013, 13:44:35 schrieb Hiroshi Doyu:
> > Hi,
> > 
> > This series provides:
> > 
> > (1) Unified SMMU driver among Tegra SoCs
> 
> does this affect tegra2 in any way?

No, at all for Tegra 20.
This driver should work with Tegra 30, 114 and 148.

[PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Hiroshi Doyu]

Hi Marc,

Marc Dietrich <marvin24@gmx.de> wrote @ Fri, 5 Jul 2013 13:03:58 +0200:

> Am Freitag, 5. Juli 2013, 13:44:35 schrieb Hiroshi Doyu:
> > Hi,
> > 
> > This series provides:
> > 
> > (1) Unified SMMU driver among Tegra SoCs
> 
> does this affect tegra2 in any way?

No, at all for Tegra 20.
This driver should work with Tegra 30, 114 and 148.

Re: [PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Hiroshi Doyu]

Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote @ Fri, 5 Jul 2013 12:44:35 +0200:

> Hi,
> 
> This series provides:
> 
> (1) Unified SMMU driver among Tegra SoCs
> (2) Multiple Address Space support(MASID) in IOMMU(SMMMU)
> (3) Tegra IOMMU'able devices, most of platform devices are IOMMU'able.
> 
> There's some discussion[1] about device population order, which can
> solve the following patches.
> 
>   [HACK] of: dev_node has struct device pointer
>   ARM: tegra: Populate AHB/IOMMU earlier than others
> 
> Also "ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations" may
> not be necessary by [2]
> 
> Tested IOMMU functionality with T30 SD/MMC. Any further testing with
> T114 and/or other devices would be really appreciated.

For tegra device driver maintainers, please try this series to make
your device work with IOMMU enabled. I guess that some of drivers may
need to set its swgroup ID correctly in DT, and also you may need to
check the usage of DMA mapping API correctly. For your convenience,
you can get this series by git fetch:

git://nv-tegra.nvidia.com/user/hdoyu/linux.git smmu-upstreaming@20130705

Although I will not be always online for a few weeks, any feedback
would be really appreciated.

Re: [PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Stephen Warren]

On 07/08/2013 02:47 AM, Hiroshi Doyu wrote:
> Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote @ Fri, 5 Jul 2013 12:44:35 +0200:
> 
>> Hi,
>>
>> This series provides:
>>
>> (1) Unified SMMU driver among Tegra SoCs
>> (2) Multiple Address Space support(MASID) in IOMMU(SMMMU)
>> (3) Tegra IOMMU'able devices, most of platform devices are IOMMU'able.
>>
>> There's some discussion[1] about device population order, which can
>> solve the following patches.
>>
>>   [HACK] of: dev_node has struct device pointer
>>   ARM: tegra: Populate AHB/IOMMU earlier than others
>>
>> Also "ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations" may
>> not be necessary by [2]
>>
>> Tested IOMMU functionality with T30 SD/MMC. Any further testing with
>> T114 and/or other devices would be really appreciated.
> 
> For tegra device driver maintainers, please try this series to make
> your device work with IOMMU enabled. I guess that some of drivers may
> need to set its swgroup ID correctly in DT, and also you may need to
> check the usage of DMA mapping API correctly. For your convenience,
> you can get this series by git fetch:

So are you saying this series will break some device drivers unless some
driver changes and/or DT additions are made? If so, those changes should
be (an early) part of this series; we can't break functionality.

Re: [PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Hiroshi Doyu]

Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Mon, 8 Jul 2013 17:49:37 +0200:

> > For tegra device driver maintainers, please try this series to make
> > your device work with IOMMU enabled. I guess that some of drivers may
> > need to set its swgroup ID correctly in DT, and also you may need to
> > check the usage of DMA mapping API correctly. For your convenience,
> > you can get this series by git fetch:
> 
> So are you saying this series will break some device drivers unless some
> driver changes and/or DT additions are made? If so, those changes should
> be (an early) part of this series; we can't break functionality.

Agree. Do we have any list of tests to run, especially for device drivers?

Re: [PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Stephen Warren]

On 07/08/2013 11:11 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Mon, 8 Jul 2013 17:49:37 +0200:
> 
>>> For tegra device driver maintainers, please try this series to make
>>> your device work with IOMMU enabled. I guess that some of drivers may
>>> need to set its swgroup ID correctly in DT, and also you may need to
>>> check the usage of DMA mapping API correctly. For your convenience,
>>> you can get this series by git fetch:
>>
>> So are you saying this series will break some device drivers unless some
>> driver changes and/or DT additions are made? If so, those changes should
>> be (an early) part of this series; we can't break functionality.
> 
> Agree. Do we have any list of tests to run, especially for device drivers?

The best bet is to look through arch/arm/boot/dts/tegra*.dtsi, find the
list of all Tegra devices, and test each driver, on all chips.

I'll also send you a link to our internal Wiki with the test-cases I
typically run.

Re: [PATCH v2 00/22] Unified SMMU driver among Tegra SoCs

[Hiroshi Doyu]

Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote @ Mon, 08 Jul 2013 11:47:36 +0300 (EEST):

> Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote @ Fri, 5 Jul 2013 12:44:35 +0200:
>
> > Hi,
> >
> > This series provides:
> >
> > (1) Unified SMMU driver among Tegra SoCs
> > (2) Multiple Address Space support(MASID) in IOMMU(SMMMU)
> > (3) Tegra IOMMU'able devices, most of platform devices are IOMMU'able.
> >
> > There's some discussion[1] about device population order, which can
> > solve the following patches.
> >
> >   [HACK] of: dev_node has struct device pointer
> >   ARM: tegra: Populate AHB/IOMMU earlier than others
> >
> > Also "ARM: dma-mapping: Drop GFP_COMP for DMA memory allocations" may
> > not be necessary by [2]
> >
> > Tested IOMMU functionality with T30 SD/MMC. Any further testing with
> > T114 and/or other devices would be really appreciated.
>
> For tegra device driver maintainers, please try this series to make
> your device work with IOMMU enabled. I guess that some of drivers may
> need to set its swgroup ID correctly in DT, and also you may need to
> check the usage of DMA mapping API correctly. For your convenience,
> you can get this series by git fetch:
>
> git://nv-tegra.nvidia.com/user/hdoyu/linux.git smmu-upstreaming@20130705

Sorry for the delay of v3, I'm preparing v3. I will send them out early Oct.