[PATCHv7 00/12] Unifying SMMU driver among Tegra SoCs

[PATCHv7 00/12] Unifying SMMU driver among Tegra SoCs

[Hiroshi Doyu]

Hi,

This series provide:

(0) IOMMU standard DT binding("iommus")
(1) Unified IOMMU(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 been some discussion[1] about device population order. Some
devices needs to be populated earlier than other devices regardless of
their bus topology. For the solution I implemented an IOMMU hook in
driver core:

  [PATCHv7 04/13] driver/core: populate devices in order for IOMMUs

which is based on:
  http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006933.html

The main problem here is,

IOMMU devices on the bus need to be poplulated first, then iommu
master devices are done later.

With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
whether a device can be an iommu msater or not. If a device can, we'll
defer to populate that device till an iommu device is populated. Then,
those defered iommu master devices are populated and configured with
help of the already populated iommu device via a new IOMMU API
iommu_ops->driver_bound().

This "iommus=" binding is expected used as the global/standard binding.

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

v6:
Minior fixes.
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/213082.html

v5:
Use "iommus=" DT bindings as a standard IOMMU binding.
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/212331.html

v4:
Add a hook in driver core to control device populatin order.
Introduced arm,smmu "mmu-master" binding instead of tegra own.
Removed DT patches from this series.
  http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006931.html

v3:
Updated based on Stephen Warren's feedback
  http://lists.linuxfoundation.org/pipermail/iommu/2013-October/006724.html

v2:
Updated based on Thierry Reding's and Stephen Warren's feedback
  http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/181888.html

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

Available in the git repository at:

  git://git-HoETi0wPbwRDw2glCA4ptUEOCMrvLtNR@public.gmane.org/user/hdoyu/linux.git smmu-upstreaming@20131212

Hiroshi Doyu (12):
  of: introduce of_property_for_each_phandle_with_args()
  iommu/of: introduce a global iommu device list
  iommu/of: check if dependee iommu is ready or not
  driver/core: populate devices in order for IOMMUs
  iommu/core: add ops->{bound,unbind}_driver()
  ARM: tegra: create a DT header defining SWGROUP ID
  iommu/tegra: smmu: register device to iommu dynamically
  iommu/tegra: smmu: calculate ASID register offset by ID
  iommu/tegra: smmu: get swgroups from DT "iommus="
  iommu/tegra: smmu: allow duplicate ASID wirte
  iommu/tegra: smmu: Rename hwgrp -> swgroups
  iommu/tegra: smmu: add SMMU to an global iommu list

 .../bindings/iommu/nvidia,tegra30-smmu.txt         |  30 +-
 drivers/base/dd.c                                  |   5 +
 drivers/iommu/Kconfig                              |   1 +
 drivers/iommu/iommu.c                              |  13 +-
 drivers/iommu/of_iommu.c                           |  51 +++
 drivers/iommu/tegra-smmu.c                         | 383 +++++++++++++--------
 drivers/of/base.c                                  |  46 +++
 include/dt-bindings/memory/tegra-swgroup.h         |  50 +++
 include/linux/iommu.h                              |   4 +
 include/linux/of.h                                 |  32 ++
 include/linux/of_iommu.h                           |  22 ++
 11 files changed, 487 insertions(+), 150 deletions(-)
 create mode 100644 include/dt-bindings/memory/tegra-swgroup.h

-- 
1.8.1.5

[1]
  "[RFC] early init and DT platform devices allocation/registration"
    https://lists.ozlabs.org/pipermail/devicetree-discuss/2013-June/thread.html#36542
  "Report from 2013 ARM kernel summit"
    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/210426.html
  "[RFC PATCH] Documentation: devicetree: add description for generic bus properties"
    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/215042.html

[PATCHv7 01/12] of: introduce of_property_for_each_phandle_with_args()

[Hiroshi Doyu]

Iterating over a property containing a list of phandles with arguments
is a common operation for device drivers. This patch adds a new
of_property_for_each_phandle_with_args() macro to make the iteration
simpler.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
v7:
Fixed some minors pointed by Rob and Stephen.

v6++++:
Iterate without intrducing a new struct.

v6+++:
Introduced a new struct "of_phandle_iter" to keep the state when
iterating over the list.

v6++:
Optimized to avoid O(n^2), suggested by Stephen Warren.
http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007066.html

I didn't introduce any struct to hold params and state here.

v6+:
Use the description, which Grant Likely proposed, to be full enough
that a future reader can figure out why a patch was written.

v5:
New patch for v5.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/of/base.c  | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f807d0e..cd4ab05 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1201,6 +1201,52 @@ void of_print_phandle_args(const char *msg, const struct of_phandle_args *args)
 	printk("\n");
 }
 
+const __be32 *of_phandle_iter_next(const char *cells_name, int cell_count,
+				   const __be32 *cur, const __be32 *end,
+				   struct of_phandle_args *out_args)
+{
+	struct device_node *dn;
+	int i;
+
+	if (!cells_name && !cell_count)
+		return NULL;
+
+	if (!cur || (cur >= end))
+		return NULL;
+
+	dn = of_find_node_by_phandle(be32_to_cpup(cur++));
+	if (!dn)
+		return NULL;
+
+	if (cells_name)
+		if (of_property_read_u32(dn, cells_name, &cell_count))
+			return NULL;
+
+	out_args->np = dn;
+	out_args->args_count = cell_count;
+	for (i = 0; i < cell_count; i++)
+		out_args->args[i] = be32_to_cpup(cur++);
+
+	return cur;
+}
+EXPORT_SYMBOL_GPL(of_phandle_iter_next);
+
+const __be32 *of_phandle_iter_init(const struct device_node *np,
+				   const char *list_name,
+				   const __be32 **end)
+{
+	size_t bytes;
+	const __be32 *cur;
+
+	cur = of_get_property(np, list_name, &bytes);
+	*end = cur;
+	if (bytes)
+		*end += bytes / sizeof(*cur);
+
+	return cur;
+}
+EXPORT_SYMBOL_GPL(of_phandle_iter_init);
+
 static int __of_parse_phandle_with_args(const struct device_node *np,
 					const char *list_name,
 					const char *cells_name,
diff --git a/include/linux/of.h b/include/linux/of.h
index 276c546..4345582 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -303,6 +303,14 @@ extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
 extern int of_count_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name);
 
+extern const __be32 *of_phandle_iter_init(const struct device_node *np,
+					  const char *list_name,
+					  const __be32 **end);
+extern const __be32 *of_phandle_iter_next(const char *cells_name,
+					  int cell_count,
+					  const __be32 *cur, const __be32 *end,
+					  struct of_phandle_args *out_args);
+
 extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align));
 extern int of_alias_get_id(struct device_node *np, const char *stem);
 
@@ -527,6 +535,22 @@ static inline int of_count_phandle_with_args(struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline const __be32 *of_phandle_iter_init(const struct device_node *np,
+						 const char *list_name,
+						 const __be32 **end)
+{
+	return NULL;
+}
+
+static inline const __be32 *of_phandle_iter_next(const char *cells_name,
+						 int cell_count,
+						 const __be32 *cur,
+						 const __be32 *end,
+						 struct of_phandle_args *out_args);
+{
+	return NULL;
+}
+
 static inline int of_alias_get_id(struct device_node *np, const char *stem)
 {
 	return -ENOSYS;
@@ -613,6 +637,14 @@ static inline int of_property_read_u32(const struct device_node *np,
 		s;						\
 		s = of_prop_next_string(prop, s))
 
+#define of_property_for_each_phandle_with_args(node, list_name, cells_name, \
+					       cell_count, out_args, cur, end) \
+	for (cur = of_phandle_iter_init(node, list_name, &end),		\
+		     cur = of_phandle_iter_next(cells_name, cell_count, \
+						cur, end, &out_args);	\
+	     cur;							\
+	     cur = of_phandle_iter_next(cells_name, cell_count, cur, end, &out_args))
+
 #if defined(CONFIG_PROC_FS) && defined(CONFIG_PROC_DEVICETREE)
 extern void proc_device_tree_add_node(struct device_node *, struct proc_dir_entry *);
 extern void proc_device_tree_add_prop(struct proc_dir_entry *pde, struct property *prop);
-- 
1.8.1.5

Re: [PATCHv7 01/12] of: introduce of_property_for_each_phandle_with_args()

[Stephen Warren]

On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> Iterating over a property containing a list of phandles with arguments
> is a common operation for device drivers. This patch adds a new
> of_property_for_each_phandle_with_args() macro to make the iteration
> simpler.

This version looks good to me.
Reviewed-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

But if you want to rewrite it to use a struct to store state between
_init()/_iter(), that'd probably be fine too.

[PATCHv7 02/12] iommu/of: introduce a global iommu device list

[Hiroshi Doyu]

This enables to find an populated IOMMU device via a device node. This
can be used to see if an dependee IOMMU is populated or not to keep
correct device population order. Client devices need to wait an IOMMU
to be populated.

Suggested by Thierry Reding and copied his example code.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
v6:
New for v6.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/of_iommu.c | 37 +++++++++++++++++++++++++++++++++++++
 include/linux/of_iommu.h | 16 ++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index ee249bc..5d1aeb9 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -20,6 +20,43 @@
 #include <linux/export.h>
 #include <linux/limits.h>
 #include <linux/of.h>
+#include <linux/of_iommu.h>
+#include <linux/device.h>
+
+static DEFINE_MUTEX(iommus_lock);
+static LIST_HEAD(iommus_list);
+
+void iommu_add(struct iommu *iommu)
+{
+	INIT_LIST_HEAD(&iommu->list);
+	mutex_lock(&iommus_lock);
+	list_add_tail(&iommu->list, &iommus_list);
+	mutex_unlock(&iommus_lock);
+}
+
+void iommu_del(struct iommu *iommu)
+{
+	INIT_LIST_HEAD(&iommu->list);
+	mutex_lock(&iommus_lock);
+	list_del(&iommu->list);
+	mutex_unlock(&iommus_lock);
+}
+
+static struct iommu *of_find_iommu_by_node(struct device_node *np)
+{
+	struct iommu *iommu;
+
+	mutex_lock(&iommus_lock);
+	list_for_each_entry(iommu, &iommus_list, list) {
+		if (iommu->dev->of_node == np) {
+			mutex_unlock(&iommus_lock);
+			return iommu;
+		}
+	}
+	mutex_unlock(&iommus_lock);
+
+	return NULL;
+}
 
 /**
  * of_get_dma_window - Parse *dma-window property and returns 0 if found.
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 51a560f..a0aa9d4 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -3,10 +3,18 @@
 
 #ifdef CONFIG_OF_IOMMU
 
+struct iommu {
+	struct list_head list;
+	struct device *dev;
+};
+
 extern int of_get_dma_window(struct device_node *dn, const char *prefix,
 			     int index, unsigned long *busno, dma_addr_t *addr,
 			     size_t *size);
 
+extern void iommu_add(struct iommu *iommu);
+extern void iommu_del(struct iommu *iommu);
+
 #else
 
 static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
@@ -16,6 +24,14 @@ static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
 	return -EINVAL;
 }
 
+static inline void iommu_add(struct iommu *iommu)
+{
+}
+
+static inline void iommu_del(struct iommu *iommu)
+{
+}
+
 #endif	/* CONFIG_OF_IOMMU */
 
 #endif /* __OF_IOMMU_H */
-- 
1.8.1.5

Re: [PATCHv7 02/12] iommu/of: introduce a global iommu device list

[Stephen Warren]

On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> This enables to find an populated IOMMU device via a device node. This
> can be used to see if an dependee IOMMU is populated or not to keep
> correct device population order. Client devices need to wait an IOMMU
> to be populated.
> 
> Suggested by Thierry Reding and copied his example code.

> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c

> +static struct iommu *of_find_iommu_by_node(struct device_node *np)
> +{
> +	struct iommu *iommu;
> +
> +	mutex_lock(&iommus_lock);
> +	list_for_each_entry(iommu, &iommus_list, list) {
> +		if (iommu->dev->of_node == np) {
> +			mutex_unlock(&iommus_lock);

Now that the list is unlocked, the IOMMU could be removed from the list,
and the driver unregistered. Should you do something like
iommu_get(iommu); before the mutex_unlock() call, and call iommu_put()
somewhere (i.e. in the next patch ...)?

[PATCHv7 03/12] iommu/of: check if dependee iommu is ready or not

[Hiroshi Doyu]

IOMMU devices on the bus need to be poplulated first, then iommu
master devices are done later.

With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
whether a device can be an iommu msater or not. If a device can, we'll
defer to populate that device till an depending iommu device is
populated.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v6:
Spinned off only of_iommu part from:
  [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs

v5:
Use "iommus=" binding instread of arm,smmu's "#stream-id-cells".

v4:
This is newly added, and the successor of the following RFC:
  [RFC][PATCHv3+ 1/2] driver/core: Add of_iommu_attach()
  http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006914.html

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/of_iommu.c | 14 ++++++++++++++
 include/linux/of_iommu.h |  6 ++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5d1aeb9..557c0c8 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -125,3 +125,17 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_get_dma_window);
+
+int of_iommu_attach(struct device *dev)
+{
+	const __be32 *cur, *end;
+	struct of_phandle_args args;
+
+	of_property_for_each_phandle_with_args(dev->of_node, "iommus",
+				       "#iommu-cells", 0, args, cur, end) {
+		if (!of_find_iommu_by_node(args.np))
+			return -EPROBE_DEFER;
+	}
+
+	return 0;
+}
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index a0aa9d4..14c9a5c 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -14,6 +14,7 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix,
 
 extern void iommu_add(struct iommu *iommu);
 extern void iommu_del(struct iommu *iommu);
+extern int of_iommu_attach(struct device *dev);
 
 #else
 
@@ -32,6 +33,11 @@ static inline void iommu_del(struct iommu *iommu)
 {
 }
 
+static inline int of_iommu_attach(struct device *dev)
+{
+	return 0;
+}
+
 #endif	/* CONFIG_OF_IOMMU */
 
 #endif /* __OF_IOMMU_H */
-- 
1.8.1.5

Re: [PATCHv7 03/12] iommu/of: check if dependee iommu is ready or not

[Stephen Warren]

On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> IOMMU devices on the bus need to be poplulated first, then iommu
> master devices are done later.
> 
> With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
> whether a device can be an iommu msater or not. If a device can, we'll
> defer to populate that device till an depending iommu device is
> populated.

> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c

> +int of_iommu_attach(struct device *dev)
> +{
> +	const __be32 *cur, *end;
> +	struct of_phandle_args args;
> +
> +	of_property_for_each_phandle_with_args(dev->of_node, "iommus",
> +				       "#iommu-cells", 0, args, cur, end) {
> +		if (!of_find_iommu_by_node(args.np))
> +			return -EPROBE_DEFER;

I think that since of_find_iommu_by_node() should be calling
iommu_get(iommu), the error-case here should loop over the IOMMUs that
have successfully been acquired, and call iommu_put().

You'd also want to implement of_iommu_detach() or iommu_detach() to undo
the iommu_get()s that happen within this function.

[PATCHv7 04/12] driver/core: populate devices in order for IOMMUs

[Hiroshi Doyu]

IOMMU devices on the bus need to be poplulated first, then iommu
master devices are done later.

With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
whether a device can be an iommu msater or not. If a device can, we'll
defer to populate that device till an iommu device is populated. Then,
those deferred iommu master devices are populated and configured with
help of the already populated iommu device.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
---
This is related to the following discussion:
  [RFC PATCH] Documentation: devicetree: add description for generic bus properties
  http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/215042.html

v6:
Spinned off only driver core part from:
  [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs

v5:
Use "iommus=" binding instread of arm,smmu's "#stream-id-cells".

v4:
This is newly added, and the successor of the following RFC:
  [RFC][PATCHv3+ 1/2] driver/core: Add of_iommu_attach()
  http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006914.html

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

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0605176..0605f52 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -25,6 +25,7 @@
 #include <linux/async.h>
 #include <linux/pm_runtime.h>
 #include <linux/pinctrl/devinfo.h>
+#include <linux/of_iommu.h>
 
 #include "base.h"
 #include "power/power.h"
@@ -273,6 +274,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 
 	dev->driver = drv;
 
+	ret = of_iommu_attach(dev);
+	if (ret)
+		goto probe_failed;
+
 	/* If using pinctrl, bind pins now before probing */
 	ret = pinctrl_bind_pins(dev);
 	if (ret)
-- 
1.8.1.5

[PATCHv7 05/12] iommu/core: add ops->{bound,unbind}_driver()

[Hiroshi Doyu]

ops->{bound,unbind}_driver() functions are called at
BUS_NOTIFY_{BOUND,UNBIND}_DRIVER respectively.

This is necessary to control the device population order. IOMMU master
devices depend on an IOMMU device instanciation. IOMMU master devices
can be registered to an IOMMU only after it's successfully
populated. This IOMMU registration is done via
ops->bound_driver(). Currently this population can be deferred if
depending IOMMU device hasn't yet been populated in driver core. This
cannot be done via ops->add_device() since after add_device() device's
population/instanciation can be still deferred via probe().

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v6:
New for v6.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/iommu.c | 13 +++++++++++--
 include/linux/iommu.h |  4 ++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e5555fc..5469d36 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -540,14 +540,23 @@ static int iommu_bus_notifier(struct notifier_block *nb,
 	 * ADD/DEL call into iommu driver ops if provided, which may
 	 * result in ADD/DEL notifiers to group->notifier
 	 */
-	if (action == BUS_NOTIFY_ADD_DEVICE) {
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
 		if (ops->add_device)
 			return ops->add_device(dev);
-	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
+	case BUS_NOTIFY_DEL_DEVICE:
 		if (ops->remove_device && dev->iommu_group) {
 			ops->remove_device(dev);
 			return 0;
 		}
+	case BUS_NOTIFY_BOUND_DRIVER:
+		if (ops->bound_driver)
+			ops->bound_driver(dev);
+		break;
+	case BUS_NOTIFY_UNBIND_DRIVER:
+		if (ops->unbind_driver)
+			ops->unbind_driver(dev);
+		break;
 	}
 
 	/*
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a444c79..a0e92be 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -96,6 +96,8 @@ enum iommu_attr {
  * @domain_has_cap: domain capabilities query
  * @add_device: add device to iommu grouping
  * @remove_device: remove device from iommu grouping
+ * @bound_driver: called at BUS_NOTIFY_BOUND_DRIVER
+ * @unbind_driver: called at BUS_NOTIFY_UNBIND_DRIVER
  * @domain_get_attr: Query domain attributes
  * @domain_set_attr: Change domain attributes
  * @pgsize_bitmap: bitmap of supported page sizes
@@ -114,6 +116,8 @@ struct iommu_ops {
 			      unsigned long cap);
 	int (*add_device)(struct device *dev);
 	void (*remove_device)(struct device *dev);
+	int (*bound_driver)(struct device *dev);
+	void (*unbind_driver)(struct device *dev);
 	int (*device_group)(struct device *dev, unsigned int *groupid);
 	int (*domain_get_attr)(struct iommu_domain *domain,
 			       enum iommu_attr attr, void *data);
-- 
1.8.1.5

Re: [PATCHv7 05/12] iommu/core: add ops->{bound,unbind}_driver()

[Stephen Warren]

On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> ops->{bound,unbind}_driver() functions are called at
> BUS_NOTIFY_{BOUND,UNBIND}_DRIVER respectively.

Some explanation re: why the existing {add,remove}_device hooks aren't
enough would be useful. I didn't really get that from the current commit
description. What actions would an IOMMU driver take in these new
callbacks that could not be taken in {add,remove}_device? Surely the
IOMMU could set up any internal data structures, AS, or page tables,
etc. within add_device, rather than deferring to bound_driver, which
presumably happens after the device's probe(), yet the driver might want
to make the HW do bus-master accesses within probe(), or at least map
stuff into the page table...

> This is necessary to control the device population order. IOMMU master
> devices depend on an IOMMU device instanciation. IOMMU master devices

Is an "IOMMU master device" the device that issues transactions on the
bus, which are affected by the IOMMU? The more common term for this is
"bus master"; "IOMMU master device" sounds too similar to the "IOMMU"
itself.

Re: [PATCHv7 05/12] iommu/core: add ops->{bound,unbind}_driver()

[Joerg Roedel]

On Thu, Dec 12, 2013 at 09:57:06AM +0200, Hiroshi Doyu wrote:
> ops->{bound,unbind}_driver() functions are called at
> BUS_NOTIFY_{BOUND,UNBIND}_DRIVER respectively.
> 
> This is necessary to control the device population order. IOMMU master
> devices depend on an IOMMU device instanciation. IOMMU master devices
> can be registered to an IOMMU only after it's successfully
> populated. This IOMMU registration is done via
> ops->bound_driver(). Currently this population can be deferred if
> depending IOMMU device hasn't yet been populated in driver core. This
> cannot be done via ops->add_device() since after add_device() device's
> population/instanciation can be still deferred via probe().

How about doing dependency checking in the add/remove_device callbacks
instead? When a device is about to be initialized where the IOMMU is not
set up yet, just setup the IOMMU first before initializing the device?


	Joerg

[PATCHv7 06/12] ARM: tegra: create a DT header defining SWGROUP 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 which a Tegra SoC
supports. This unique ID can be used to calculate MC_SMMU_<swgroup
name>_ASID_0 register offset and MC_<swgroup name>_HOTRESET_*_0
register bit. 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. For HOTRESET
bit shifting we need another conversion table, which will come later.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v6:
Use 0xffffffff instead of ~0UL since dtc expand this to ~0ULL.

v5:
Added new macro TEGRA_SWGROUP_CELLS() and WO_U32_OF_U64().

v4:
This is almost same as the previous v3. Just TEGRA_SWGROUP_MAX is
added.
  [PATCHv3 15/19] ARM: tegra: Create a DT header defining SWGROUP ID

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..9c279f1
--- /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	/* 0x238 */
+#define TEGRA_SWGROUP_AVPC	1	/* 0x23c */
+#define TEGRA_SWGROUP_DC	2	/* 0x240 */
+#define TEGRA_SWGROUP_DCB	3	/* 0x244 */
+#define TEGRA_SWGROUP_EPP	4	/* 0x248 */
+#define TEGRA_SWGROUP_G2	5	/* 0x24c */
+#define TEGRA_SWGROUP_HC	6	/* 0x250 */
+#define TEGRA_SWGROUP_HDA	7	/* 0x254 */
+#define TEGRA_SWGROUP_ISP	8	/* 0x258 */
+#define TEGRA_SWGROUP_ISP2	SWGROUP_ISP
+#define TEGRA_SWGROUP_DC14	9	/* 0x490 *//* Exceptional non-linear */
+#define TEGRA_SWGROUP_DC12	10	/* 0xa88 *//* Exceptional non-linear */
+#define TEGRA_SWGROUP_MPE	11	/* 0x264 */
+#define TEGRA_SWGROUP_MSENC	SWGROUP_MPE
+#define TEGRA_SWGROUP_NV	12	/* 0x268 */
+#define TEGRA_SWGROUP_NV2	13	/* 0x26c */
+#define TEGRA_SWGROUP_PPCS	14	/* 0x270 */
+#define TEGRA_SWGROUP_SATA2	15	/* 0x274 */
+#define TEGRA_SWGROUP_SATA	16	/* 0x278 */
+#define TEGRA_SWGROUP_VDE	17	/* 0x27c */
+#define TEGRA_SWGROUP_VI	18	/* 0x280 */
+#define TEGRA_SWGROUP_VIC	19	/* 0x284 */
+#define TEGRA_SWGROUP_XUSB_HOST	20	/* 0x288 */
+#define TEGRA_SWGROUP_XUSB_DEV	21	/* 0x28c */
+#define TEGRA_SWGROUP_A9AVP	22	/* 0x290 */
+#define TEGRA_SWGROUP_TSEC	23	/* 0x294 */
+#define TEGRA_SWGROUP_PPCS1	24	/* 0x298 */
+#define TEGRA_SWGROUP_SDMMC1A	25	/* 0xa94 *//* Linear shift again */
+#define TEGRA_SWGROUP_SDMMC2A	26	/* 0xa98 */
+#define TEGRA_SWGROUP_SDMMC3A	27	/* 0xa9c */
+#define TEGRA_SWGROUP_SDMMC4A	28	/* 0xaa0 */
+#define TEGRA_SWGROUP_ISP2B	29	/* 0xaa4 */
+#define TEGRA_SWGROUP_GPU	30	/* 0xaa8 */
+#define TEGRA_SWGROUP_GPUB	31	/* 0xaac */
+#define TEGRA_SWGROUP_PPCS2	32	/* 0xab0 */
+
+#define TWO_U32_OF_U64(x)	((x) & 0xffffffff) ((x) >> 32)
+#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
+#define TEGRA_SWGROUP_CELLS(x)	TWO_U32_OF_U64(TEGRA_SWGROUP_BIT(x))
+
+#define TEGRA_SWGROUP_MAX	64
+
+#endif /* _DT_BINDINGS_MEMORY_TEGRA_SWGROUP_H */
-- 
1.8.1.5

Re: [PATCHv7 06/12] ARM: tegra: create a DT header defining SWGROUP ID

[Mark Zhang]

On 12/12/2013 03:57 PM, 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 which a Tegra SoC
> supports. This unique ID can be used to calculate MC_SMMU_<swgroup
> name>_ASID_0 register offset and MC_<swgroup name>_HOTRESET_*_0
> register bit. 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. For HOTRESET
> bit shifting we need another conversion table, which will come later.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> v6:
> Use 0xffffffff instead of ~0UL since dtc expand this to ~0ULL.
> 
> v5:
> Added new macro TEGRA_SWGROUP_CELLS() and WO_U32_OF_U64().
> 
> v4:
> This is almost same as the previous v3. Just TEGRA_SWGROUP_MAX is
> added.
>   [PATCHv3 15/19] ARM: tegra: Create a DT header defining SWGROUP ID
> 
> 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..9c279f1
> --- /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	/* 0x238 */
> +#define TEGRA_SWGROUP_AVPC	1	/* 0x23c */
> +#define TEGRA_SWGROUP_DC	2	/* 0x240 */
> +#define TEGRA_SWGROUP_DCB	3	/* 0x244 */
> +#define TEGRA_SWGROUP_EPP	4	/* 0x248 */
> +#define TEGRA_SWGROUP_G2	5	/* 0x24c */
> +#define TEGRA_SWGROUP_HC	6	/* 0x250 */
> +#define TEGRA_SWGROUP_HDA	7	/* 0x254 */
> +#define TEGRA_SWGROUP_ISP	8	/* 0x258 */
> +#define TEGRA_SWGROUP_ISP2	SWGROUP_ISP
> +#define TEGRA_SWGROUP_DC14	9	/* 0x490 *//* Exceptional non-linear */
> +#define TEGRA_SWGROUP_DC12	10	/* 0xa88 *//* Exceptional non-linear */
> +#define TEGRA_SWGROUP_MPE	11	/* 0x264 */
> +#define TEGRA_SWGROUP_MSENC	SWGROUP_MPE

Need to change this to:

#define TEGRA_SWGROUP_MSENC	11

The reason is that, this makes "TEGRA_SWGROUP_BIT" doesn't work. So if I
write "TEGRA_SWGROUP_CELLS(MSENC)" in dt, that causes a dt compiling error.

"TEGRA_SWGROUP_ISP2" has the same issue.

Mark
> +#define TEGRA_SWGROUP_NV	12	/* 0x268 */
> +#define TEGRA_SWGROUP_NV2	13	/* 0x26c */
> +#define TEGRA_SWGROUP_PPCS	14	/* 0x270 */
> +#define TEGRA_SWGROUP_SATA2	15	/* 0x274 */
> +#define TEGRA_SWGROUP_SATA	16	/* 0x278 */
> +#define TEGRA_SWGROUP_VDE	17	/* 0x27c */
> +#define TEGRA_SWGROUP_VI	18	/* 0x280 */
> +#define TEGRA_SWGROUP_VIC	19	/* 0x284 */
> +#define TEGRA_SWGROUP_XUSB_HOST	20	/* 0x288 */
> +#define TEGRA_SWGROUP_XUSB_DEV	21	/* 0x28c */
> +#define TEGRA_SWGROUP_A9AVP	22	/* 0x290 */
> +#define TEGRA_SWGROUP_TSEC	23	/* 0x294 */
> +#define TEGRA_SWGROUP_PPCS1	24	/* 0x298 */
> +#define TEGRA_SWGROUP_SDMMC1A	25	/* 0xa94 *//* Linear shift again */
> +#define TEGRA_SWGROUP_SDMMC2A	26	/* 0xa98 */
> +#define TEGRA_SWGROUP_SDMMC3A	27	/* 0xa9c */
> +#define TEGRA_SWGROUP_SDMMC4A	28	/* 0xaa0 */
> +#define TEGRA_SWGROUP_ISP2B	29	/* 0xaa4 */
> +#define TEGRA_SWGROUP_GPU	30	/* 0xaa8 */
> +#define TEGRA_SWGROUP_GPUB	31	/* 0xaac */
> +#define TEGRA_SWGROUP_PPCS2	32	/* 0xab0 */
> +
> +#define TWO_U32_OF_U64(x)	((x) & 0xffffffff) ((x) >> 32)
> +#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
> +#define TEGRA_SWGROUP_CELLS(x)	TWO_U32_OF_U64(TEGRA_SWGROUP_BIT(x))
> +
> +#define TEGRA_SWGROUP_MAX	64
> +
> +#endif /* _DT_BINDINGS_MEMORY_TEGRA_SWGROUP_H */
> 

Re: [PATCHv7 06/12] ARM: tegra: create a DT header defining SWGROUP ID

[Stephen Warren]

On 12/18/2013 01:02 AM, Mark Zhang wrote:
> On 12/12/2013 03:57 PM, 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 which a Tegra SoC
>> supports. This unique ID can be used to calculate MC_SMMU_<swgroup
>> name>_ASID_0 register offset and MC_<swgroup name>_HOTRESET_*_0
>> register bit. 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. For HOTRESET
>> bit shifting we need another conversion table, which will come later.

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

>> +#define TEGRA_SWGROUP_MPE	11	/* 0x264 */
>> +#define TEGRA_SWGROUP_MSENC	SWGROUP_MPE
> 
> Need to change this to:
> 
> #define TEGRA_SWGROUP_MSENC	11
> 
> The reason is that, this makes "TEGRA_SWGROUP_BIT" doesn't work. So if I
> write "TEGRA_SWGROUP_CELLS(MSENC)" in dt, that causes a dt compiling error.

I guess it's because TEGRA_SWGROUP_BIT needs to expand its argument
twice, which can be done.

That all said, just defining all the names directly to constants is
probably the most direct fix.

Re: [PATCHv7 06/12] ARM: tegra: create a DT header defining SWGROUP ID

[Thierry Reding]

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

On Wed, Dec 18, 2013 at 09:27:29AM -0700, Stephen Warren wrote:
> On 12/18/2013 01:02 AM, Mark Zhang wrote:
> > On 12/12/2013 03:57 PM, 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 which a Tegra SoC
> >> supports. This unique ID can be used to calculate MC_SMMU_<swgroup
> >> name>_ASID_0 register offset and MC_<swgroup name>_HOTRESET_*_0
> >> register bit. 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. For HOTRESET
> >> bit shifting we need another conversion table, which will come later.
> 
> >> diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
> 
> >> +#define TEGRA_SWGROUP_MPE	11	/* 0x264 */
> >> +#define TEGRA_SWGROUP_MSENC	SWGROUP_MPE
> > 
> > Need to change this to:
> > 
> > #define TEGRA_SWGROUP_MSENC	11
> > 
> > The reason is that, this makes "TEGRA_SWGROUP_BIT" doesn't work. So if I
> > write "TEGRA_SWGROUP_CELLS(MSENC)" in dt, that causes a dt compiling error.
> 
> I guess it's because TEGRA_SWGROUP_BIT needs to expand its argument
> twice, which can be done.
> 
> That all said, just defining all the names directly to constants is
> probably the most direct fix.

Erm... isn't this simply a typo, where:

	#define TEGRA_SWGROUP_MSENC	SWGROUP_MPE

should simply be

	#define TEGRA_SWGROUP_MSENC	TEGRA_SWGROUP_MPE

?

That certainly works for me.

Thierry

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

Re: [PATCHv7 06/12] ARM: tegra: create a DT header defining SWGROUP ID

[Stephen Warren]

On 12/20/2013 05:35 AM, Thierry Reding wrote:
> On Wed, Dec 18, 2013 at 09:27:29AM -0700, Stephen Warren wrote:
>> On 12/18/2013 01:02 AM, Mark Zhang wrote:
>>> On 12/12/2013 03:57 PM, 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 which a Tegra SoC
>>>> supports. This unique ID can be used to calculate MC_SMMU_<swgroup
>>>> name>_ASID_0 register offset and MC_<swgroup name>_HOTRESET_*_0
>>>> register bit. 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. For HOTRESET
>>>> bit shifting we need another conversion table, which will come later.
>>
>>>> diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
>>
>>>> +#define TEGRA_SWGROUP_MPE	11	/* 0x264 */
>>>> +#define TEGRA_SWGROUP_MSENC	SWGROUP_MPE
>>>
>>> Need to change this to:
>>>
>>> #define TEGRA_SWGROUP_MSENC	11
>>>
>>> The reason is that, this makes "TEGRA_SWGROUP_BIT" doesn't work. So if I
>>> write "TEGRA_SWGROUP_CELLS(MSENC)" in dt, that causes a dt compiling error.
>>
>> I guess it's because TEGRA_SWGROUP_BIT needs to expand its argument
>> twice, which can be done.
>>
>> That all said, just defining all the names directly to constants is
>> probably the most direct fix.
> 
> Erm... isn't this simply a typo, where:
> 
> 	#define TEGRA_SWGROUP_MSENC	SWGROUP_MPE
> 
> should simply be
> 
> 	#define TEGRA_SWGROUP_MSENC	TEGRA_SWGROUP_MPE
> 
> ?
> 
> That certainly works for me.

You're right. I guess ## is defined to re-expand its result, so
TEGRA_SWGROUP_BIT(ISP2) -> TEGRA_SWGROUP_##ISP2 -> TEGRA_SWGROUP_ISP2 ->
TEGRA_SWGROUP_ISP -> 8.

[PATCHv7 07/12] iommu/tegra: smmu: register device to iommu dynamically

[Hiroshi Doyu]

platform_devices are registered as IOMMU'able dynamically via
add_device() and remove_device().

Tegra SMMU can have multiple address spaces(AS). IOMMU'able devices
can belong to one of them. Multiple IOVA maps are created at boot-up,
which can be attached to devices later. 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. DMA API(ARM)
needs ARM_DMA_USE_IOMMU to be enabled.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v6:
Use smmu_iommu_{bound,unbind}_driver() instead of
smmu_iommu_{add,del}_device() to register devices to SMMU.

v5:
Add check NUM_OF_STATIC_MAPS < #asids.

v4:
Combined the following from v3. This makes more sense what they do.
  [PATCHv3 06/19] iommu/tegra: smmu: Select ARM_DMA_USE_IOMMU in Kconfig
  [PATCHv3 07/19] iommu/tegra: smmu: Create default IOVA maps
  [PATCHv3 08/19] iommu/tegra: smmu: Register platform_device to IOMMU dynamically
  [PATCHv3 19/19] iommu/tegra: smmu: Support Multiple ASID

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

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 3e7fdbb..0a86d63 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -170,6 +170,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
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 3c772c9..99b4bd4 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -39,6 +39,9 @@
 
 #include <asm/page.h>
 #include <asm/cacheflush.h>
+#include <asm/dma-iommu.h>
+
+#include <dt-bindings/memory/tegra-swgroup.h>
 
 enum smmu_hwgrp {
 	HWGRP_AFI,
@@ -319,6 +322,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 */
 };
@@ -947,6 +952,44 @@ 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("AHB bus children"), which has SDMMC
+ * ASID[2][3].. open for drivers, first come, first served.
+ */
+enum {
+	SYSTEM_DEFAULT,
+	SYSTEM_PROTECTED,
+	NUM_OF_STATIC_MAPS,
+};
+
+static int smmu_iommu_bound_driver(struct device *dev)
+{
+	int err = -EPROBE_DEFER;
+	u32 swgroups = dev->platform_data;
+	struct dma_iommu_mapping *map = NULL;
+
+	if (test_bit(TEGRA_SWGROUP_PPCS, swgroups))
+		map = smmu_handle->map[SYSTEM_PROTECTED];
+	else
+		map = smmu_handle->map[SYSTEM_DEFAULT];
+
+	if (map)
+		err = arm_iommu_attach_device(dev, map);
+	else
+		return -EPROBE_DEFER;
+
+	pr_debug("swgroups=%08lx map=%p err=%d %s\n",
+		 swgroups, map, err, dev_name(dev));
+	return err;
+}
+
+static void smmu_iommu_unbind_driver(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,
@@ -956,6 +999,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,
+	.bound_driver	= smmu_iommu_bound_driver,
+	.unbind_driver	= smmu_iommu_unbind_driver,
 	.pgsize_bitmap	= SMMU_IOMMU_PGSIZES,
 };
 
@@ -1144,6 +1189,23 @@ static int tegra_smmu_resume(struct device *dev)
 	return err;
 }
 
+static void 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 (IS_ERR(smmu->map[i]))
+			dev_err(smmu->dev,
+				"Couldn't create: asid=%d map=%p %pa-%pa\n",
+				i, smmu->map[i], &base, &base + size - 1);
+	}
+}
+
 static int tegra_smmu_probe(struct platform_device *pdev)
 {
 	struct smmu_device *smmu;
@@ -1160,13 +1222,18 @@ 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);
+	if (asids < NUM_OF_STATIC_MAPS)
+		return -EINVAL;
+
+	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);
@@ -1236,6 +1303,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);
+	tegra_smmu_create_default_map(smmu);
 	return 0;
 }
 
-- 
1.8.1.5

Re: [PATCHv7 07/12] iommu/tegra: smmu: register device to iommu dynamically

[Stephen Warren]

On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> platform_devices are registered as IOMMU'able dynamically via
> add_device() and remove_device().

That says {add,remove}_device() ...

> Tegra SMMU can have multiple address spaces(AS). IOMMU'able devices
> can belong to one of them. Multiple IOVA maps are created at boot-up,
> which can be attached to devices later. 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. DMA API(ARM)
> needs ARM_DMA_USE_IOMMU to be enabled.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> v6:
> Use smmu_iommu_{bound,unbind}_driver() instead of
> smmu_iommu_{add,del}_device() to register devices to SMMU.

... yet that says it's been changed to {bound,unbind}_driver(). BTW,
what was the reason for this change?

[PATCHv7 08/12] iommu/tegra: smmu: calculate ASID register offset by ID

[Hiroshi Doyu]

ASID register offset is caclulated by SWGROUP ID so that we can get
rid of old SoC specific MACROs. This ID conversion is needed for the
unified SMMU driver over Tegra SoCs. We use dt-bindings MACRO instead
of SoC dependent MACROs. The formula is:

  MC_SMMU_<swgroup name>_ASID_0 = MC_SMMU_AFI_ASID_0 + ID * 4;

Now SWGROUP ID is the global HardWare Accelerator(HWA) identifier
among all Tegra SoC except Tegra2.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v5:
Added SMMU_ASID_BASE(== SMMU_AFI_ASID).
Removed unused ASID offset definitions.
Use 'unsigned long *' instead of u64 for swgroups bitmap.

v4:
Combined the following patches from v3:
[PATCHv3 09/19] iommu/tegra: smmu: Calculate ASID register offset by ID
[PATCHv3 16/19] iommu/tegra: smmu: Use dt-bindings MACRO

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

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 99b4bd4..6ab977a 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -43,46 +43,6 @@
 
 #include <dt-bindings/memory/tegra-swgroup.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)
 
@@ -152,21 +112,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_ASID_BASE	SMMU_AFI_ASID
 
 #define SMMU_PDE_NEXT_SHIFT		28
 
@@ -238,27 +184,7 @@ 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) (smmu_hwgrp_asid_reg[x])
+#define HWGRP_ASID_REG(x) ((x) * sizeof(u32) + SMMU_ASID_BASE)
 
 /*
  * Per client for address space
@@ -267,7 +193,7 @@ struct smmu_client {
 	struct device		*dev;
 	struct list_head	list;
 	struct smmu_as		*as;
-	u32			hwgrp;
+	unsigned long		hwgrp[2];
 };
 
 /*
@@ -384,41 +310,37 @@ 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)
-
 static int __smmu_client_set_hwgrp(struct smmu_client *c,
-				   unsigned long map, int on)
+				   unsigned long *map, int on)
 {
 	int i;
 	struct smmu_as *as = c->as;
 	u32 val, offs, mask = SMMU_ASID_ENABLE(as->asid);
 	struct smmu_device *smmu = as->smmu;
 
-	WARN_ON(!on && map);
-	if (on && !map)
-		return -EINVAL;
 	if (!on)
-		map = smmu_client_hwgrp(c);
+		map = c->hwgrp;
 
-	for_each_set_bit(i, &map, HWGRP_COUNT) {
+	for_each_set_bit(i, map, TEGRA_SWGROUP_MAX) {
 		offs = HWGRP_ASID_REG(i);
 		val = smmu_read(smmu, offs);
 		if (on) {
 			if (WARN_ON(val & mask))
 				goto err_hw_busy;
 			val |= mask;
+			memcpy(c->hwgrp, map, sizeof(u64));
 		} else {
 			WARN_ON((val & mask) == mask);
 			val &= ~mask;
 		}
 		smmu_write(smmu, val, offs);
 	}
+
 	FLUSH_SMMU_REGS(smmu);
-	c->hwgrp = map;
 	return 0;
 
 err_hw_busy:
-	for_each_set_bit(i, &map, HWGRP_COUNT) {
+	for_each_set_bit(i, map, TEGRA_SWGROUP_MAX) {
 		offs = HWGRP_ASID_REG(i);
 		val = smmu_read(smmu, offs);
 		val &= ~mask;
@@ -427,17 +349,18 @@ err_hw_busy:
 	return -EBUSY;
 }
 
-static int smmu_client_set_hwgrp(struct smmu_client *c, u32 map, int on)
+static int smmu_client_set_hwgrp(struct smmu_client *c,
+				 unsigned long *map, int on)
 {
-	u32 val;
+	int err;
 	unsigned long flags;
 	struct smmu_as *as = c->as;
 	struct smmu_device *smmu = as->smmu;
 
 	spin_lock_irqsave(&smmu->lock, flags);
-	val = __smmu_client_set_hwgrp(c, map, on);
+	err = __smmu_client_set_hwgrp(c, map, on);
 	spin_unlock_irqrestore(&smmu->lock, flags);
-	return val;
+	return err;
 }
 
 /*
@@ -796,7 +719,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;
+	unsigned long *map;
 	int err;
 
 	client = devm_kzalloc(smmu->dev, sizeof(*c), GFP_KERNEL);
@@ -804,7 +727,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 = (unsigned long *)dev->platform_data;
 	if (!map)
 		return -EINVAL;
 
@@ -828,7 +751,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 (test_bit(TEGRA_SWGROUP_AVPC, map)) {
 		struct page *page;
 
 		page = as->smmu->avp_vector_page;
-- 
1.8.1.5

Re: [PATCHv7 08/12] iommu/tegra: smmu: calculate ASID register offset by ID

[Stephen Warren]

On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> ASID register offset is caclulated by SWGROUP ID so that we can get
> rid of old SoC specific MACROs. This ID conversion is needed for the
> unified SMMU driver over Tegra SoCs. We use dt-bindings MACRO instead
> of SoC dependent MACROs. The formula is:
> 
>   MC_SMMU_<swgroup name>_ASID_0 = MC_SMMU_AFI_ASID_0 + ID * 4;
> 
> Now SWGROUP ID is the global HardWare Accelerator(HWA) identifier
> among all Tegra SoC except Tegra2.

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

>  static int __smmu_client_set_hwgrp(struct smmu_client *c,
> -				   unsigned long map, int on)
> +				   unsigned long *map, int on)
>  {
>  	int i;
>  	struct smmu_as *as = c->as;
>  	u32 val, offs, mask = SMMU_ASID_ENABLE(as->asid);
>  	struct smmu_device *smmu = as->smmu;
>  
>  	if (!on)
> +		map = c->hwgrp;
>  
> +	for_each_set_bit(i, map, TEGRA_SWGROUP_MAX) {
>  		offs = HWGRP_ASID_REG(i);
>  		val = smmu_read(smmu, offs);
>  		if (on) {
>  			if (WARN_ON(val & mask))
>  				goto err_hw_busy;
>  			val |= mask;
> +			memcpy(c->hwgrp, map, sizeof(u64));
>  		} else {
>  			WARN_ON((val & mask) == mask);
>  			val &= ~mask;
>  		}
>  		smmu_write(smmu, val, offs);
>  	}

This function doesn't make a lot of sense to me. The registers it's
manipulating aren't a bitmask, so I don't see why the code is performing
bitmask AND/OR operations on the register. Instead, don't you want the
following:

#define SMMU_ASID_ENABLE	(1 << 31)
#define SMMU_ASID_ENABLED(asid)	(SMMU_ASID_ENABLE | asid)
#define SMMU_ASID_DISABLE	0

static int __smmu_client_set_hwgrp(struct smmu_client *c,
				   unsigned long *map, int on)
{
	int i;
	struct smmu_as *as = c->as;
	u32 val, offs, new_val;
	struct smmu_device *smmu = as->smmu;

	if (!on)
		map = c->hwgrp;

	for_each_set_bit(i, map, TEGRA_SWGROUP_MAX) {
		offs = HWGRP_ASID_REG(i);
		val = smmu_read(smmu, offs);
		if (on)
			new_val = SMMU_ASID_ENABLED(as->asid);
		else
			new_val = 0;
		WARN_ON(val & SMMU_ASID_ENABLE ==
			new_val & SMMU_ASID_ENABLE);
		if (on) {
			if (val & SMMU_ASID_ENABLE)
				goto err_hw_busy;
			memcpy(c->hwgrp, map, sizeof(u64));
		}
		smmu_write(smmu, val, offs);
	}

> @@ -804,7 +727,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 = (unsigned long *)dev->platform_data;
>  	if (!map)
>  		return -EINVAL;

Presumably we can simply delete that now since everything using this
driver is on DT; we can't rely on the platform_data of client struct
devices...

[PATCHv7 09/12] iommu/tegra: smmu: get swgroups from DT "iommus="

[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>
---
v6:
- Explained "#iommu-cells" in the binding document.
- Fixed old "nvidia,memory-clients" with 'iommus" in the binding
  document.
- Move smmu_of_get_swgroups() here from the previous patch not to break
  git bisecting.

v5:
"iommu=" in a device DT is used instead of "mmu-masters" in an iommu
DT. This is "iommu=" version of:

  [PATCHv4 5/7] iommu/tegra: smmu: Support "mmu-masters" binding

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

diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
index 89fb543..fd53f54 100644
--- a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
@@ -1,6 +1,6 @@
 NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit)
 
-Required properties:
+Required properties in the IOMMU node:
 - compatible : "nvidia,tegra30-smmu"
 - reg : Should contain 3 register banks(address and length) for each
   of the SMMU register blocks.
@@ -8,9 +8,23 @@ Required properties:
 - nvidia,#asids : # of ASIDs
 - dma-window : IOVA start address and length.
 - nvidia,ahb : phandle to the ahb bus connected to SMMU.
+- iommus: phandle to an iommu device which a device is
+  attached to and indicates which swgroups a device belongs to(SWGROUP ID).
+  SWGROUP ID is from 0 to 63, and a device can belong to multiple SWGROUPS.
+- #iommu-cells. Should be 2. In client IOMMU specifiers, the two cells
+  represent a 64-bit bitmask of SWGROUP IDs under which the device
+  initiates transactions. The least significant word is first. See
+  <dt-bindings/memory/tegra-swgroup.h> for a list of valid values.
+
+Required properties in device nodes affected by the IOMMU:
+- iommus: A list of phandle plus specifier pairs for each IOMMU that
+  affects master transactions initiated by the device. The number of
+  cells in each specifier is defined by the #iommu-cells property in
+  the IOMMU node referred to by the phandle. The meaning of the
+  specifier cells is defined by the referenced IOMMU's binding.
 
 Example:
-	smmu {
+	smmu: iommu {
 		compatible = "nvidia,tegra30-smmu";
 		reg = <0x7000f010 0x02c
 		       0x7000f1f0 0x010
@@ -18,4 +32,16 @@ Example:
 		nvidia,#asids = <4>;		/* # of ASIDs */
 		dma-window = <0 0x40000000>;	/* IOVA start & length */
 		nvidia,ahb = <&ahb>;
+		#iommu-cells = <2>;
 	};
+
+	host1x {
+		compatible = "nvidia,tegra30-host1x", "simple-bus";
+		iommus = <&smmu TEGRA_SWGROUP_CELLS(HC)>;
+		....
+		gr3d {
+			compatible = "nvidia,tegra30-gr3d";
+			iommus = <&smmu TEGRA_SWGROUP_CELLS(NV)
+					TEGRA_SWGROUP_CELLS(NV2)>;
+			....
+		};
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 6ab977a..fd4479a 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -190,6 +190,8 @@ enum {
  * Per client for address space
  */
 struct smmu_client {
+	struct device_node	*of_node;
+	struct rb_node		node;
 	struct device		*dev;
 	struct list_head	list;
 	struct smmu_as		*as;
@@ -233,6 +235,7 @@ struct smmu_device {
 	spinlock_t	lock;
 	char		*name;
 	struct device	*dev;
+	struct rb_root	clients;
 	struct page *avp_vector_page;	/* dummy page shared by all AS's */
 
 	/*
@@ -310,6 +313,96 @@ static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
  */
 #define FLUSH_SMMU_REGS(smmu)	smmu_read(smmu, SMMU_CONFIG)
 
+static struct smmu_client *find_smmu_client(struct smmu_device *smmu,
+					    struct device_node *dev_node)
+{
+	struct rb_node *node = smmu->clients.rb_node;
+
+	while (node) {
+		struct smmu_client *client;
+
+		client = container_of(node, struct smmu_client, node);
+		if (dev_node < client->of_node)
+			node = node->rb_left;
+		else if (dev_node > client->of_node)
+			node = node->rb_right;
+		else
+			return client;
+	}
+
+	return NULL;
+}
+
+static int insert_smmu_client(struct smmu_device *smmu,
+			      struct smmu_client *client)
+{
+	struct rb_node **new, *parent;
+
+	new = &smmu->clients.rb_node;
+	parent = NULL;
+	while (*new) {
+		struct smmu_client *this;
+		this = container_of(*new, struct smmu_client, node);
+
+		parent = *new;
+		if (client->of_node < this->of_node)
+			new = &((*new)->rb_left);
+		else if (client->of_node > this->of_node)
+			new = &((*new)->rb_right);
+		else
+			return -EEXIST;
+	}
+
+	rb_link_node(&client->node, parent, new);
+	rb_insert_color(&client->node, &smmu->clients);
+	return 0;
+}
+
+static int register_smmu_client(struct smmu_device *smmu,
+				struct device *dev, unsigned long *swgroups)
+{
+	struct smmu_client *client;
+
+	client = find_smmu_client(smmu, dev->of_node);
+	if (client) {
+		dev_err(dev,
+			"rejecting multiple registrations for client device %s\n",
+			dev->of_node->full_name);
+		return -EBUSY;
+	}
+
+	client = devm_kzalloc(smmu->dev, sizeof(*client), GFP_KERNEL);
+	if (!client)
+		return -ENOMEM;
+
+	client->dev = dev;
+	client->of_node = dev->of_node;
+	memcpy(client->hwgrp, swgroups, sizeof(u64));
+	return insert_smmu_client(smmu, client);
+}
+
+static int smmu_of_get_swgroups(struct device *dev, unsigned long *swgroups)
+{
+	struct of_phandle_args args;
+	const __be32 *cur, *end;
+
+	of_property_for_each_phandle_with_args(dev->of_node, "iommus",
+					       "#iommu-cells", 0, args, cur, end) {
+		if (args.np != smmu_handle->dev->of_node)
+			continue;
+
+		BUG_ON(args.args_count != 2);
+
+		memcpy(swgroups, args.args, sizeof(u64));
+		pr_debug("swgroups=%08lx %08lx ops=%p %s\n",
+			 swgroups[0], swgroups[1],
+			 dev->bus->iommu_ops, dev_name(dev));
+		return 0;
+	}
+
+	return -ENODEV;
+}
+
 static int __smmu_client_set_hwgrp(struct smmu_client *c,
 				   unsigned long *map, int on)
 {
@@ -719,21 +812,16 @@ 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;
-	unsigned long *map;
 	int err;
 
-	client = devm_kzalloc(smmu->dev, sizeof(*c), GFP_KERNEL);
+	client = find_smmu_client(smmu, dev->of_node);
 	if (!client)
 		return -ENOMEM;
-	client->dev = dev;
-	client->as = as;
-	map = (unsigned long *)dev->platform_data;
-	if (!map)
-		return -EINVAL;
 
-	err = smmu_client_enable_hwgrp(client, map);
+	client->as = as;
+	err = smmu_client_enable_hwgrp(client, client->hwgrp);
 	if (err)
-		goto err_hwgrp;
+		return -EINVAL;
 
 	spin_lock(&as->client_lock);
 	list_for_each_entry(c, &as->client, list) {
@@ -751,7 +839,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 	 * Reserve "page zero" for AVP vectors using a common dummy
 	 * page.
 	 */
-	if (test_bit(TEGRA_SWGROUP_AVPC, map)) {
+	if (test_bit(TEGRA_SWGROUP_AVPC, client->hwgrp)) {
 		struct page *page;
 
 		page = as->smmu->avp_vector_page;
@@ -766,8 +854,6 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 err_client:
 	smmu_client_disable_hwgrp(client);
 	spin_unlock(&as->client_lock);
-err_hwgrp:
-	devm_kfree(smmu->dev, client);
 	return err;
 }
 
@@ -784,7 +870,6 @@ static void smmu_iommu_detach_dev(struct iommu_domain *domain,
 		if (c->dev == dev) {
 			smmu_client_disable_hwgrp(c);
 			list_del(&c->list);
-			devm_kfree(smmu->dev, c);
 			c->as = NULL;
 			dev_dbg(smmu->dev,
 				"%s is detached\n", dev_name(c->dev));
@@ -888,10 +973,23 @@ enum {
 
 static int smmu_iommu_bound_driver(struct device *dev)
 {
-	int err = -EPROBE_DEFER;
-	u32 swgroups = dev->platform_data;
+	int err;
+	unsigned long swgroups[2];
 	struct dma_iommu_mapping *map = NULL;
 
+	err = smmu_of_get_swgroups(dev, swgroups);
+	if (err)
+		return -ENODEV;
+
+	if (!find_smmu_client(smmu_handle, dev->of_node)) {
+		err = register_smmu_client(smmu_handle, dev, swgroups);
+		if (err) {
+			dev_err(dev, "failed to add client %s\n",
+				dev_name(dev));
+			return -EINVAL;
+		}
+	}
+
 	if (test_bit(TEGRA_SWGROUP_PPCS, swgroups))
 		map = smmu_handle->map[SYSTEM_PROTECTED];
 	else
@@ -900,10 +998,10 @@ static int smmu_iommu_bound_driver(struct device *dev)
 	if (map)
 		err = arm_iommu_attach_device(dev, map);
 	else
-		return -EPROBE_DEFER;
+		return -ENODEV;
 
-	pr_debug("swgroups=%08lx map=%p err=%d %s\n",
-		 swgroups, map, err, dev_name(dev));
+	pr_debug("swgroups=%08lx %08lx map=%p err=%d %s\n",
+		 swgroups[0], swgroups[1], map, err, dev_name(dev));
 	return err;
 }
 
@@ -1156,6 +1254,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	smmu->clients = RB_ROOT;
 	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),
-- 
1.8.1.5

Re: [PATCHv7 09/12] iommu/tegra: smmu: get swgroups from DT "iommus="

[Stephen Warren]

On 12/12/2013 12:57 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

> -Required properties:
> +Required properties in the IOMMU node:
...
> +- iommus: phandle to an iommu device which a device is
> +  attached to and indicates which swgroups a device belongs to(SWGROUP ID).
> +  SWGROUP ID is from 0 to 63, and a device can belong to multiple SWGROUPS.

Shouldn't that property have been deleted from the "Required properties
in the IOMMU node" section, when it got move to the "Required properties
in device nodes affected by the IOMMU" section?

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

> +static int smmu_of_get_swgroups(struct device *dev, unsigned long *swgroups)

> +	of_property_for_each_phandle_with_args(dev->of_node, "iommus",
> +					       "#iommu-cells", 0, args, cur, end) {
> +		if (args.np != smmu_handle->dev->of_node)
> +			continue;
> +
> +		BUG_ON(args.args_count != 2);

Wouldn't it be better to WARN() and prevent the IOMMU from affecting
that given client device, i.e. simply return some error?

> @@ -719,21 +812,16 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,

> -	client->dev = dev;
> -	client->as = as;
> -	map = (unsigned long *)dev->platform_data;
> -	if (!map)
> -		return -EINVAL;

OK good, so this does get deleted. Ignore my comment re: this on an
earlier patch then.

[PATCHv7 10/12] iommu/tegra: smmu: allow duplicate ASID wirte

[Hiroshi Doyu]

The device, which belongs to the same ASID, can try to enable the same
ASID as the other swgroup devices. This should be allowed but just
skip the actual register write. If the write value is different, it
will return -EINVAL.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v4:
This was the part of v3, which isn't used any more.
  [PATCHv3 10/19] iommu/tegra: smmu: Get "nvidia,swgroups" from DT

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

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index fd4479a..2b8a302 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -418,9 +418,13 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 		offs = HWGRP_ASID_REG(i);
 		val = smmu_read(smmu, offs);
 		if (on) {
-			if (WARN_ON(val & mask))
-				goto err_hw_busy;
-			val |= mask;
+			if (val) {
+				if (WARN_ON(val != mask))
+					return -EINVAL;
+				goto skip;
+			}
+
+			val = mask;
 			memcpy(c->hwgrp, map, sizeof(u64));
 		} else {
 			WARN_ON((val & mask) == mask);
@@ -430,16 +434,8 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 	}
 
 	FLUSH_SMMU_REGS(smmu);
+skip:
 	return 0;
-
-err_hw_busy:
-	for_each_set_bit(i, map, TEGRA_SWGROUP_MAX) {
-		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,
-- 
1.8.1.5

Re: [PATCHv7 10/12] iommu/tegra: smmu: allow duplicate ASID wirte

[Stephen Warren]

On 12/12/2013 12:57 AM, Hiroshi Doyu wrote:
> The device, which belongs to the same ASID, can try to enable the same
> ASID as the other swgroup devices. This should be allowed but just
> skip the actual register write. If the write value is different, it
> will return -EINVAL.

I guess this simplifies the body of the code a lot. Given this, I would
revise the code I suggested in response to a previous patch, to
something more like:

		offs = HWGRP_ASID_REG(i);
		val = smmu_read(smmu, offs);
		if (on) {
			if (WARN_ON(val && val != new_val))
				return -ENODEV;
			memcpy(c->hwgrp, map, sizeof(u64));
		} else {
			WARN_ON(val);
		}
		smmu_write(smmu, val, offs);

> +			if (val) {
> +				if (WARN_ON(val != mask))
> +					return -EINVAL;
> +				goto skip;

That means that:

a) No error is returned when the ASIDs don't match. Surely you still
want to return an error?

b) "skip" skips setting up all HWGRP IDs for the device, whereas perhaps
only 1 was shared yet the rest still need to be set up. If you really do
want to ignore the error, then surely you want "continue;" here not
"goto skip;"?

> +			}
...
> +skip:
>  	return 0;

[PATCHv7 11/12] iommu/tegra: smmu: Rename hwgrp -> swgroups

[Hiroshi Doyu]

Use the correct term for SWGROUP related variables and macros.

The term "swgroup" is the collection of "memory client". A "memory
client" usually represents a HardWare Accelerator(HWA) like
GPU. Sometimes a strut device can belong to multiple "swgroup" so that
"swgroup's'" is used here. This "swgroups" is the term used in Tegra
TRM. Rename along with TRM.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v4:
New for v4

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

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 2b8a302..2d4b8b6 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -179,12 +179,12 @@ enum {
 
 #define NUM_SMMU_REG_BANKS	3
 
-#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 __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 smmu_client_enable_swgroups(c, m) smmu_client_set_swgroups(c, m, 1)
+#define smmu_client_disable_swgroups(c) smmu_client_set_swgroups(c, 0, 0)
+#define __smmu_client_enable_swgroups(c, m) __smmu_client_set_swgroups(c, m, 1)
+#define __smmu_client_disable_swgroups(c) __smmu_client_set_swgroups(c, 0, 0)
 
-#define HWGRP_ASID_REG(x) ((x) * sizeof(u32) + SMMU_ASID_BASE)
+#define SWGROUPS_ASID_REG(x) ((x) * sizeof(u32) + SMMU_ASID_BASE)
 
 /*
  * Per client for address space
@@ -195,7 +195,7 @@ struct smmu_client {
 	struct device		*dev;
 	struct list_head	list;
 	struct smmu_as		*as;
-	unsigned long		hwgrp[2];
+	unsigned long		swgroups[2];
 };
 
 /*
@@ -377,7 +377,7 @@ static int register_smmu_client(struct smmu_device *smmu,
 
 	client->dev = dev;
 	client->of_node = dev->of_node;
-	memcpy(client->hwgrp, swgroups, sizeof(u64));
+	memcpy(client->swgroups, swgroups, sizeof(u64));
 	return insert_smmu_client(smmu, client);
 }
 
@@ -403,7 +403,7 @@ static int smmu_of_get_swgroups(struct device *dev, unsigned long *swgroups)
 	return -ENODEV;
 }
 
-static int __smmu_client_set_hwgrp(struct smmu_client *c,
+static int __smmu_client_set_swgroups(struct smmu_client *c,
 				   unsigned long *map, int on)
 {
 	int i;
@@ -412,10 +412,10 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 	struct smmu_device *smmu = as->smmu;
 
 	if (!on)
-		map = c->hwgrp;
+		map = c->swgroups;
 
 	for_each_set_bit(i, map, TEGRA_SWGROUP_MAX) {
-		offs = HWGRP_ASID_REG(i);
+		offs = SWGROUPS_ASID_REG(i);
 		val = smmu_read(smmu, offs);
 		if (on) {
 			if (val) {
@@ -425,7 +425,7 @@ static int __smmu_client_set_hwgrp(struct smmu_client *c,
 			}
 
 			val = mask;
-			memcpy(c->hwgrp, map, sizeof(u64));
+			memcpy(c->swgroups, map, sizeof(u64));
 		} else {
 			WARN_ON((val & mask) == mask);
 			val &= ~mask;
@@ -438,7 +438,7 @@ skip:
 	return 0;
 }
 
-static int smmu_client_set_hwgrp(struct smmu_client *c,
+static int smmu_client_set_swgroups(struct smmu_client *c,
 				 unsigned long *map, int on)
 {
 	int err;
@@ -447,7 +447,7 @@ static int smmu_client_set_hwgrp(struct smmu_client *c,
 	struct smmu_device *smmu = as->smmu;
 
 	spin_lock_irqsave(&smmu->lock, flags);
-	err = __smmu_client_set_hwgrp(c, map, on);
+	err = __smmu_client_set_swgroups(c, map, on);
 	spin_unlock_irqrestore(&smmu->lock, flags);
 	return err;
 }
@@ -487,7 +487,7 @@ static int smmu_setup_regs(struct smmu_device *smmu)
 		smmu_write(smmu, val, SMMU_PTB_DATA);
 
 		list_for_each_entry(c, &as->client, list)
-			__smmu_client_set_hwgrp(c, c->hwgrp, 1);
+			__smmu_client_set_swgroups(c, c->swgroups, 1);
 	}
 
 	smmu_write(smmu, smmu->translation_enable_0, SMMU_TRANSLATION_ENABLE_0);
@@ -815,7 +815,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 		return -ENOMEM;
 
 	client->as = as;
-	err = smmu_client_enable_hwgrp(client, client->hwgrp);
+	err = smmu_client_enable_swgroups(client, client->swgroups);
 	if (err)
 		return -EINVAL;
 
@@ -835,7 +835,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 	 * Reserve "page zero" for AVP vectors using a common dummy
 	 * page.
 	 */
-	if (test_bit(TEGRA_SWGROUP_AVPC, client->hwgrp)) {
+	if (test_bit(TEGRA_SWGROUP_AVPC, client->swgroups)) {
 		struct page *page;
 
 		page = as->smmu->avp_vector_page;
@@ -848,7 +848,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 	return 0;
 
 err_client:
-	smmu_client_disable_hwgrp(client);
+	smmu_client_disable_swgroups(client);
 	spin_unlock(&as->client_lock);
 	return err;
 }
@@ -864,7 +864,7 @@ static void smmu_iommu_detach_dev(struct iommu_domain *domain,
 
 	list_for_each_entry(c, &as->client, list) {
 		if (c->dev == dev) {
-			smmu_client_disable_hwgrp(c);
+			smmu_client_disable_swgroups(c);
 			list_del(&c->list);
 			c->as = NULL;
 			dev_dbg(smmu->dev,
-- 
1.8.1.5

[PATCHv7 12/12] iommu/tegra: smmu: add SMMU to an global iommu list

[Hiroshi Doyu]

This allows to inquire if SMMU is populated or not.

Suggested by Thierry Reding and copied his example code.

Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
v6:
New for v6.

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

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 2d4b8b6..d0f0ba7 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -225,6 +225,8 @@ struct smmu_debugfs_info {
  * Per SMMU device - IOMMU device
  */
 struct smmu_device {
+	struct iommu	iommu;
+
 	void __iomem	*regbase;	/* register offset base */
 	void __iomem	**regs;		/* register block start address array */
 	void __iomem	**rege;		/* register block end address array */
@@ -234,7 +236,6 @@ struct smmu_device {
 	unsigned long	page_count;	/* total remappable size */
 	spinlock_t	lock;
 	char		*name;
-	struct device	*dev;
 	struct rb_root	clients;
 	struct page *avp_vector_page;	/* dummy page shared by all AS's */
 
@@ -371,7 +372,7 @@ static int register_smmu_client(struct smmu_device *smmu,
 		return -EBUSY;
 	}
 
-	client = devm_kzalloc(smmu->dev, sizeof(*client), GFP_KERNEL);
+	client = devm_kzalloc(smmu->iommu.dev, sizeof(*client), GFP_KERNEL);
 	if (!client)
 		return -ENOMEM;
 
@@ -388,7 +389,7 @@ static int smmu_of_get_swgroups(struct device *dev, unsigned long *swgroups)
 
 	of_property_for_each_phandle_with_args(dev->of_node, "iommus",
 					       "#iommu-cells", 0, args, cur, end) {
-		if (args.np != smmu_handle->dev->of_node)
+		if (args.np != smmu_handle->iommu.dev->of_node)
 			continue;
 
 		BUG_ON(args.args_count != 2);
@@ -527,7 +528,7 @@ static void free_ptbl(struct smmu_as *as, dma_addr_t iova)
 	unsigned long *pdir = (unsigned long *)page_address(as->pdir_page);
 
 	if (pdir[pdn] != _PDE_VACANT(pdn)) {
-		dev_dbg(as->smmu->dev, "pdn: %lx\n", pdn);
+		dev_dbg(as->smmu->iommu.dev, "pdn: %lx\n", pdn);
 
 		ClearPageReserved(SMMU_EX_PTBL_PAGE(pdir[pdn]));
 		__free_page(SMMU_EX_PTBL_PAGE(pdir[pdn]));
@@ -542,7 +543,7 @@ static void free_pdir(struct smmu_as *as)
 {
 	unsigned addr;
 	int count;
-	struct device *dev = as->smmu->dev;
+	struct device *dev = as->smmu->iommu.dev;
 
 	if (!as->pdir_page)
 		return;
@@ -585,11 +586,11 @@ static unsigned long *locate_pte(struct smmu_as *as,
 		unsigned long addr = SMMU_PDN_TO_ADDR(pdn);
 
 		/* Vacant - allocate a new page table */
-		dev_dbg(as->smmu->dev, "New PTBL pdn: %lx\n", pdn);
+		dev_dbg(as->smmu->iommu.dev, "New PTBL pdn: %lx\n", pdn);
 
 		*ptbl_page_p = alloc_page(GFP_ATOMIC);
 		if (!*ptbl_page_p) {
-			dev_err(as->smmu->dev,
+			dev_err(as->smmu->iommu.dev,
 				"failed to allocate smmu_device page table\n");
 			return NULL;
 		}
@@ -649,7 +650,7 @@ static int alloc_pdir(struct smmu_as *as)
 	/*
 	 * do the allocation, then grab as->lock
 	 */
-	cnt = devm_kzalloc(smmu->dev,
+	cnt = devm_kzalloc(smmu->iommu.dev,
 			   sizeof(cnt[0]) * SMMU_PDIR_COUNT,
 			   GFP_KERNEL);
 	page = alloc_page(GFP_KERNEL | __GFP_DMA);
@@ -663,7 +664,8 @@ static int alloc_pdir(struct smmu_as *as)
 	}
 
 	if (!page || !cnt) {
-		dev_err(smmu->dev, "failed to allocate at %s\n", __func__);
+		dev_err(smmu->iommu.dev,
+			"failed to allocate at %s\n", __func__);
 		err = -ENOMEM;
 		goto err_out;
 	}
@@ -693,7 +695,7 @@ static int alloc_pdir(struct smmu_as *as)
 err_out:
 	spin_unlock_irqrestore(&as->lock, flags);
 
-	devm_kfree(smmu->dev, cnt);
+	devm_kfree(smmu->iommu.dev, cnt);
 	if (page)
 		__free_page(page);
 	return err;
@@ -748,7 +750,7 @@ static int smmu_iommu_map(struct iommu_domain *domain, unsigned long iova,
 	unsigned long pfn = __phys_to_pfn(pa);
 	unsigned long flags;
 
-	dev_info(as->smmu->dev, "[%d] %08lx:%pa\n", as->asid, iova, &pa);
+	dev_info(as->smmu->iommu.dev, "[%d] %08lx:%pa\n", as->asid, iova, &pa);
 
 	if (!pfn_valid(pfn))
 		return -ENOMEM;
@@ -765,7 +767,7 @@ static size_t smmu_iommu_unmap(struct iommu_domain *domain, unsigned long iova,
 	struct smmu_as *as = domain->priv;
 	unsigned long flags;
 
-	dev_dbg(as->smmu->dev, "[%d] %08lx\n", as->asid, iova);
+	dev_dbg(as->smmu->iommu.dev, "[%d] %08lx\n", as->asid, iova);
 
 	spin_lock_irqsave(&as->lock, flags);
 	__smmu_iommu_unmap(as, iova);
@@ -788,7 +790,7 @@ static phys_addr_t smmu_iommu_iova_to_phys(struct iommu_domain *domain,
 	pte = locate_pte(as, iova, true, &page, &count);
 	pfn = *pte & SMMU_PFN_MASK;
 	WARN_ON(!pfn_valid(pfn));
-	dev_dbg(as->smmu->dev,
+	dev_dbg(as->smmu->iommu.dev,
 		"iova:%08llx pfn:%08lx asid:%d\n", (unsigned long long)iova,
 		 pfn, as->asid);
 
@@ -822,7 +824,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 	spin_lock(&as->client_lock);
 	list_for_each_entry(c, &as->client, list) {
 		if (c->dev == dev) {
-			dev_err(smmu->dev,
+			dev_err(smmu->iommu.dev,
 				"%s is already attached\n", dev_name(c->dev));
 			err = -EINVAL;
 			goto err_client;
@@ -844,7 +846,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain *domain,
 		pr_info("Reserve \"page zero\" for AVP vectors using a common dummy\n");
 	}
 
-	dev_dbg(smmu->dev, "%s is attached\n", dev_name(dev));
+	dev_dbg(smmu->iommu.dev, "%s is attached\n", dev_name(dev));
 	return 0;
 
 err_client:
@@ -867,12 +869,12 @@ static void smmu_iommu_detach_dev(struct iommu_domain *domain,
 			smmu_client_disable_swgroups(c);
 			list_del(&c->list);
 			c->as = NULL;
-			dev_dbg(smmu->dev,
+			dev_dbg(smmu->iommu.dev,
 				"%s is detached\n", dev_name(c->dev));
 			goto out;
 		}
 	}
-	dev_err(smmu->dev, "Couldn't find %s\n", dev_name(dev));
+	dev_err(smmu->iommu.dev, "Couldn't find %s\n", dev_name(dev));
 out:
 	spin_unlock(&as->client_lock);
 }
@@ -899,7 +901,7 @@ static int smmu_iommu_domain_init(struct iommu_domain *domain)
 			break;
 	}
 	if (i == smmu->num_as)
-		dev_err(smmu->dev,  "no free AS\n");
+		dev_err(smmu->iommu.dev,  "no free AS\n");
 	return err;
 
 found:
@@ -920,7 +922,7 @@ found:
 		smmu->page_count * SMMU_PAGE_SIZE - 1;
 	domain->geometry.force_aperture = true;
 
-	dev_dbg(smmu->dev, "smmu_as@%p\n", as);
+	dev_dbg(smmu->iommu.dev, "smmu_as@%p\n", as);
 
 	return 0;
 }
@@ -953,7 +955,7 @@ static void smmu_iommu_domain_destroy(struct iommu_domain *domain)
 	spin_unlock_irqrestore(&as->lock, flags);
 
 	domain->priv = NULL;
-	dev_dbg(smmu->dev, "smmu_as@%p\n", as);
+	dev_dbg(smmu->iommu.dev, "smmu_as@%p\n", as);
 }
 
 /*
@@ -1085,7 +1087,7 @@ static ssize_t smmu_debugfs_stats_write(struct file *file,
 		break;
 	}
 
-	dev_dbg(smmu->dev, "%s() %08x, %08x @%08x\n", __func__,
+	dev_dbg(smmu->iommu.dev, "%s() %08x, %08x @%08x\n", __func__,
 		val, smmu_read(smmu, offs), offs);
 
 	return count;
@@ -1107,7 +1109,7 @@ static int smmu_debugfs_stats_show(struct seq_file *s, void *v)
 		val = smmu_read(smmu, offs);
 		seq_printf(s, "%s:%08x ", stats[i], val);
 
-		dev_dbg(smmu->dev, "%s() %s %08x @%08x\n", __func__,
+		dev_dbg(smmu->iommu.dev, "%s() %s %08x @%08x\n", __func__,
 			stats[i], val, offs);
 	}
 	seq_printf(s, "\n");
@@ -1145,7 +1147,7 @@ static void smmu_debugfs_create(struct smmu_device *smmu)
 	if (!smmu->debugfs_info)
 		return;
 
-	root = debugfs_create_dir(dev_name(smmu->dev), NULL);
+	root = debugfs_create_dir(dev_name(smmu->iommu.dev), NULL);
 	if (!root)
 		goto err_out;
 	smmu->debugfs_root = root;
@@ -1217,7 +1219,7 @@ static void tegra_smmu_create_default_map(struct smmu_device *smmu)
 		smmu->map[i] = arm_iommu_create_mapping(&platform_bus_type,
 							base, size, 0);
 		if (IS_ERR(smmu->map[i]))
-			dev_err(smmu->dev,
+			dev_err(smmu->iommu.dev,
 				"Couldn't create: asid=%d map=%p %pa-%pa\n",
 				i, smmu->map[i], &base, &base + size - 1);
 	}
@@ -1285,7 +1287,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	if (!smmu->ahb)
 		return -ENODEV;
 
-	smmu->dev = dev;
+	smmu->iommu.dev = dev;
 	smmu->num_as = asids;
 	smmu->iovmm_base = base;
 	smmu->page_count = size;
@@ -1322,6 +1324,8 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	smmu_handle = smmu;
 	bus_set_iommu(&platform_bus_type, &smmu_iommu_ops);
 	tegra_smmu_create_default_map(smmu);
+
+	iommu_add(&smmu->iommu);
 	return 0;
 }
 
@@ -1337,6 +1341,7 @@ static int tegra_smmu_remove(struct platform_device *pdev)
 		free_pdir(&smmu->as[i]);
 	__free_page(smmu->avp_vector_page);
 	smmu_handle = NULL;
+	iommu_del(&smmu->iommu);
 	return 0;
 }
 
-- 
1.8.1.5

Re: [PATCHv7 04/12] driver/core: populate devices in order for IOMMUs

[Grant Likely]

On Thu, 12 Dec 2013 09:57:05 +0200, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> IOMMU devices on the bus need to be poplulated first, then iommu
> master devices are done later.
> 
> With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
> whether a device can be an iommu msater or not. If a device can, we'll
> defer to populate that device till an iommu device is populated. Then,
> those deferred iommu master devices are populated and configured with
> help of the already populated iommu device.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> ---
> This is related to the following discussion:
>   [RFC PATCH] Documentation: devicetree: add description for generic bus properties
>   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/215042.html
> 
> v6:
> Spinned off only driver core part from:
>   [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs
> 
> v5:
> Use "iommus=" binding instread of arm,smmu's "#stream-id-cells".
> 
> v4:
> This is newly added, and the successor of the following RFC:
>   [RFC][PATCHv3+ 1/2] driver/core: Add of_iommu_attach()
>   http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006914.html
> 
> Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/base/dd.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 0605176..0605f52 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -25,6 +25,7 @@
>  #include <linux/async.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pinctrl/devinfo.h>
> +#include <linux/of_iommu.h>
>  
>  #include "base.h"
>  #include "power/power.h"
> @@ -273,6 +274,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>  
>  	dev->driver = drv;
>  
> +	ret = of_iommu_attach(dev);
> +	if (ret)
> +		goto probe_failed;
> +

As discussed before, I really don't think hooking in to dd.c is the
right thing to do here, and certainly not as a device tree specific
function. ACPI or PCI described devices may have the same constraints
and those won't have DT descriptions.

I don't think we've got a good solution yet for this problem. :-(

g.

Re: [PATCHv7 04/12] driver/core: populate devices in order for IOMMUs

[Greg KH]

On Thu, Dec 12, 2013 at 11:39:20AM +0000, Grant Likely wrote:
> On Thu, 12 Dec 2013 09:57:05 +0200, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > IOMMU devices on the bus need to be poplulated first, then iommu
> > master devices are done later.
> > 
> > With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
> > whether a device can be an iommu msater or not. If a device can, we'll
> > defer to populate that device till an iommu device is populated. Then,
> > those deferred iommu master devices are populated and configured with
> > help of the already populated iommu device.
> > 
> > Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> > ---
> > This is related to the following discussion:
> >   [RFC PATCH] Documentation: devicetree: add description for generic bus properties
> >   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/215042.html
> > 
> > v6:
> > Spinned off only driver core part from:
> >   [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs
> > 
> > v5:
> > Use "iommus=" binding instread of arm,smmu's "#stream-id-cells".
> > 
> > v4:
> > This is newly added, and the successor of the following RFC:
> >   [RFC][PATCHv3+ 1/2] driver/core: Add of_iommu_attach()
> >   http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006914.html
> > 
> > Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> >  drivers/base/dd.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > index 0605176..0605f52 100644
> > --- a/drivers/base/dd.c
> > +++ b/drivers/base/dd.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/async.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/pinctrl/devinfo.h>
> > +#include <linux/of_iommu.h>
> >  
> >  #include "base.h"
> >  #include "power/power.h"
> > @@ -273,6 +274,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> >  
> >  	dev->driver = drv;
> >  
> > +	ret = of_iommu_attach(dev);
> > +	if (ret)
> > +		goto probe_failed;
> > +
> 
> As discussed before, I really don't think hooking in to dd.c is the
> right thing to do here, and certainly not as a device tree specific
> function. ACPI or PCI described devices may have the same constraints
> and those won't have DT descriptions.

I agree, this shouldn't be in the driver core.

greg k-h

Re: [PATCHv7 04/12] driver/core: populate devices in order for IOMMUs

[Thierry Reding]

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

On Thu, Dec 12, 2013 at 06:14:02PM -0800, Greg KH wrote:
> On Thu, Dec 12, 2013 at 11:39:20AM +0000, Grant Likely wrote:
> > On Thu, 12 Dec 2013 09:57:05 +0200, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > > IOMMU devices on the bus need to be poplulated first, then iommu
> > > master devices are done later.
> > > 
> > > With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
> > > whether a device can be an iommu msater or not. If a device can, we'll
> > > defer to populate that device till an iommu device is populated. Then,
> > > those deferred iommu master devices are populated and configured with
> > > help of the already populated iommu device.
> > > 
> > > Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> > > ---
> > > This is related to the following discussion:
> > >   [RFC PATCH] Documentation: devicetree: add description for generic bus properties
> > >   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/215042.html
> > > 
> > > v6:
> > > Spinned off only driver core part from:
> > >   [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs
> > > 
> > > v5:
> > > Use "iommus=" binding instread of arm,smmu's "#stream-id-cells".
> > > 
> > > v4:
> > > This is newly added, and the successor of the following RFC:
> > >   [RFC][PATCHv3+ 1/2] driver/core: Add of_iommu_attach()
> > >   http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006914.html
> > > 
> > > Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > ---
> > >  drivers/base/dd.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > > index 0605176..0605f52 100644
> > > --- a/drivers/base/dd.c
> > > +++ b/drivers/base/dd.c
> > > @@ -25,6 +25,7 @@
> > >  #include <linux/async.h>
> > >  #include <linux/pm_runtime.h>
> > >  #include <linux/pinctrl/devinfo.h>
> > > +#include <linux/of_iommu.h>
> > >  
> > >  #include "base.h"
> > >  #include "power/power.h"
> > > @@ -273,6 +274,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> > >  
> > >  	dev->driver = drv;
> > >  
> > > +	ret = of_iommu_attach(dev);
> > > +	if (ret)
> > > +		goto probe_failed;
> > > +
> > 
> > As discussed before, I really don't think hooking in to dd.c is the
> > right thing to do here, and certainly not as a device tree specific
> > function. ACPI or PCI described devices may have the same constraints
> > and those won't have DT descriptions.
> 
> I agree, this shouldn't be in the driver core.

Okay, so what would be an alternative? Grant's objection makes sense and
we could easily just wrap the call to of_iommu_attach() within a generic
iommu_attach() that could decide at runtime which exact implementation
to call, depending on whether the device is DT, ACPI, PCI or whatnot.

If we don't want something like that in the core either, then the only
other alternative would be to call this from each driver. However given
the desire to handle IOMMUs completely transparently for device drivers
that would be missing the point.

Perhaps moving this into platform_drv_probe() would be more acceptable?
That's still somewhat core, but maybe suburban enough.

Thierry

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

Re: [PATCHv7 04/12] driver/core: populate devices in order for IOMMUs

[Hiroshi Doyu]

Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote @ Sat, 14 Dec 2013 13:24:22 +0100:

> * PGP Signed by an unknown key
> 
> On Thu, Dec 12, 2013 at 06:14:02PM -0800, Greg KH wrote:
> > On Thu, Dec 12, 2013 at 11:39:20AM +0000, Grant Likely wrote:
> > > On Thu, 12 Dec 2013 09:57:05 +0200, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > > > IOMMU devices on the bus need to be poplulated first, then iommu
> > > > master devices are done later.
> > > > 
> > > > With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
> > > > whether a device can be an iommu msater or not. If a device can, we'll
> > > > defer to populate that device till an iommu device is populated. Then,
> > > > those deferred iommu master devices are populated and configured with
> > > > help of the already populated iommu device.
> > > > 
> > > > Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > > Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> > > > ---
> > > > This is related to the following discussion:
> > > >   [RFC PATCH] Documentation: devicetree: add description for generic bus properties
> > > >   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/215042.html
> > > > 
> > > > v6:
> > > > Spinned off only driver core part from:
> > > >   [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs
> > > > 
> > > > v5:
> > > > Use "iommus=" binding instread of arm,smmu's "#stream-id-cells".
> > > > 
> > > > v4:
> > > > This is newly added, and the successor of the following RFC:
> > > >   [RFC][PATCHv3+ 1/2] driver/core: Add of_iommu_attach()
> > > >   http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006914.html
> > > > 
> > > > Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > > ---
> > > >  drivers/base/dd.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> > > > index 0605176..0605f52 100644
> > > > --- a/drivers/base/dd.c
> > > > +++ b/drivers/base/dd.c
> > > > @@ -25,6 +25,7 @@
> > > >  #include <linux/async.h>
> > > >  #include <linux/pm_runtime.h>
> > > >  #include <linux/pinctrl/devinfo.h>
> > > > +#include <linux/of_iommu.h>
> > > >  
> > > >  #include "base.h"
> > > >  #include "power/power.h"
> > > > @@ -273,6 +274,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
> > > >  
> > > >  	dev->driver = drv;
> > > >  
> > > > +	ret = of_iommu_attach(dev);
> > > > +	if (ret)
> > > > +		goto probe_failed;
> > > > +
> > > 
> > > As discussed before, I really don't think hooking in to dd.c is the
> > > right thing to do here, and certainly not as a device tree specific
> > > function. ACPI or PCI described devices may have the same constraints
> > > and those won't have DT descriptions.
> > 
> > I agree, this shouldn't be in the driver core.
> 
> Okay, so what would be an alternative? Grant's objection makes sense and
> we could easily just wrap the call to of_iommu_attach() within a generic
> iommu_attach() that could decide at runtime which exact implementation
> to call, depending on whether the device is DT, ACPI, PCI or whatnot.
> 
> If we don't want something like that in the core either, then the only
> other alternative would be to call this from each driver. However given
> the desire to handle IOMMUs completely transparently for device drivers
> that would be missing the point.

What about using "bus_notifier" to send -EPROBE_DEFER?

The current bus_notifier framework doesn't have the ability to defer
the probe, but I may think that this change is acceptable relatively.

The fundamental problem is that IOMMU doesn't follow the exact bus
model like "chained IOMMU" cases, but this discussion may take long to
be solved. I think that "bus_notifier" with send -EPROBE_DEFER would
cover most of the normal cases, like normal IOMMU device probe
population order.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv7 04/12] driver/core: populate devices in order for IOMMUs

[Stephen Warren]

On 12/12/2013 07:14 PM, Greg KH wrote:
> On Thu, Dec 12, 2013 at 11:39:20AM +0000, Grant Likely wrote:
>> On Thu, 12 Dec 2013 09:57:05 +0200, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>> IOMMU devices on the bus need to be poplulated first, then iommu
>>> master devices are done later.
>>>
>>> With CONFIG_OF_IOMMU, "iommus=" DT binding would be used to identify
>>> whether a device can be an iommu msater or not. If a device can, we'll
>>> defer to populate that device till an iommu device is populated. Then,
>>> those deferred iommu master devices are populated and configured with
>>> help of the already populated iommu device.

>>> This is related to the following discussion:
>>>   [RFC PATCH] Documentation: devicetree: add description for generic bus properties
>>>   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/215042.html

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

>>> @@ -273,6 +274,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
>>>  
>>>  	dev->driver = drv;
>>>  
>>> +	ret = of_iommu_attach(dev);
>>> +	if (ret)
>>> +		goto probe_failed;
>>> +
>>
>> As discussed before, I really don't think hooking in to dd.c is the
>> right thing to do here, and certainly not as a device tree specific
>> function. ACPI or PCI described devices may have the same constraints
>> and those won't have DT descriptions.
> 
> I agree, this shouldn't be in the driver core.

I don't think I agree. It'd greatly simplify driver probe() routines if
the driver core could acquire/set up as many resources as it could on
behalf of drivers. It'd be nice if it pre-mapped any registers, acquired
clocks, regulators, ... That way, we wouldn't need to write all that
code in each individual driver's probe() routine. Now, in many cases
this is rather difficult since there's currently no way for the driver
core to know which resources a driver needs, but when the driver core
can/does know this, shouldn't it simplify the life of drivers?
Longer-term, perhaps drivers can provide the driver core with some
specification of the resources it needs (such as a list of clock,
regulator, ... names), to fill in the missing information.