Hi,
here is a small patch-set for the omap-iommu driver to make
it use new features of the iommu-core. Please review.
Thanks,
Joerg
Joerg Roedel (5):
iommu/omap: Move data structures to omap-iommu.h
iommu/omap: Permanently keep iommu_dev pointer in arch_data
iommu/omap: Set dev->archdata.iommu = NULL in omap_iommu_remove_device
iommu/omap: Make use of 'struct iommu_device'
iommu/omap: Add iommu-group support
drivers/iommu/omap-iommu.c | 137 ++++++++++++++++++++-----------
drivers/iommu/omap-iommu.h | 35 ++++++++
include/linux/platform_data/iommu-omap.h | 15 ----
3 files changed, 124 insertions(+), 63 deletions(-)
--
1.9.1
From: Joerg Roedel <[email protected]>
Modify the driver to register individual iommus and
establish links between devices and iommus in sysfs.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/omap-iommu.c | 25 +++++++++++++++++++++++++
drivers/iommu/omap-iommu.h | 2 ++
2 files changed, 27 insertions(+)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 04b3718..5e3a946 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -36,6 +36,8 @@
#include "omap-iopgtable.h"
#include "omap-iommu.h"
+static const struct iommu_ops omap_iommu_ops;
+
#define to_iommu(dev) \
((struct omap_iommu *)platform_get_drvdata(to_platform_device(dev)))
@@ -946,6 +948,16 @@ static int omap_iommu_probe(struct platform_device *pdev)
pm_runtime_irq_safe(obj->dev);
pm_runtime_enable(obj->dev);
+ err = iommu_device_sysfs_add(&obj->iommu, obj->dev, NULL, obj->name);
+ if (err)
+ return err;
+
+ iommu_device_set_ops(&obj->iommu, &omap_iommu_ops);
+
+ err = iommu_device_register(&obj->iommu);
+ if (err)
+ return err;
+
omap_iommu_debugfs_add(obj);
dev_info(&pdev->dev, "%s registered\n", obj->name);
@@ -956,6 +968,9 @@ static int omap_iommu_remove(struct platform_device *pdev)
{
struct omap_iommu *obj = platform_get_drvdata(pdev);
+ iommu_device_sysfs_remove(&obj->iommu);
+ iommu_device_unregister(&obj->iommu);
+
omap_iommu_debugfs_remove(obj);
pm_runtime_disable(obj->dev);
@@ -1205,6 +1220,7 @@ static int omap_iommu_add_device(struct device *dev)
struct omap_iommu *iommu;
struct device_node *np;
struct platform_device *pdev;
+ int ret;
/*
* Allocate the archdata iommu structure for DT-based devices.
@@ -1237,6 +1253,12 @@ static int omap_iommu_add_device(struct device *dev)
return -ENOMEM;
}
+ ret = iommu_device_link(&iommu->iommu, dev);
+ if (ret) {
+ of_node_put(np);
+ return ret;
+ }
+
arch_data->iommu_dev = iommu;
dev->archdata.iommu = arch_data;
@@ -1252,8 +1274,11 @@ static void omap_iommu_remove_device(struct device *dev)
if (!dev->of_node || !arch_data)
return;
+ iommu_device_unlink(&arch_data->iommu_dev->iommu, dev);
+
dev->archdata.iommu = NULL;
kfree(arch_data);
+
}
static const struct iommu_ops omap_iommu_ops = {
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 05bd279..07e6ea1 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -67,6 +67,8 @@ struct omap_iommu {
int has_bus_err_back;
u32 id;
+
+ struct iommu_device iommu;
};
/**
--
1.9.1
From: Joerg Roedel <[email protected]>
Instead of finding the matching IOMMU for a device using
string comparision functions, keep the pointer to the
iommu_dev in arch_data permanently populated.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/omap-iommu.c | 56 ++++++++++++++++++++--------------------------
drivers/iommu/omap-iommu.h | 2 +-
2 files changed, 25 insertions(+), 33 deletions(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index e9c9b08..45df5c8 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -802,33 +802,14 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
return IRQ_NONE;
}
-static int device_match_by_alias(struct device *dev, void *data)
-{
- struct omap_iommu *obj = to_iommu(dev);
- const char *name = data;
-
- pr_debug("%s: %s %s\n", __func__, obj->name, name);
-
- return strcmp(obj->name, name) == 0;
-}
-
/**
* omap_iommu_attach() - attach iommu device to an iommu domain
- * @name: name of target omap iommu device
+ * @obj: target omap iommu device
* @iopgd: page table
**/
-static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd)
+static int omap_iommu_attach(struct omap_iommu *obj, u32 *iopgd)
{
int err;
- struct device *dev;
- struct omap_iommu *obj;
-
- dev = driver_find_device(&omap_iommu_driver.driver, NULL, (void *)name,
- device_match_by_alias);
- if (!dev)
- return ERR_PTR(-ENODEV);
-
- obj = to_iommu(dev);
spin_lock(&obj->iommu_lock);
@@ -841,11 +822,13 @@ static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd)
spin_unlock(&obj->iommu_lock);
dev_dbg(obj->dev, "%s: %s\n", __func__, obj->name);
- return obj;
+
+ return 0;
err_enable:
spin_unlock(&obj->iommu_lock);
- return ERR_PTR(err);
+
+ return err;
}
/**
@@ -1061,11 +1044,11 @@ static size_t omap_iommu_unmap(struct iommu_domain *domain, unsigned long da,
omap_iommu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
struct omap_iommu_domain *omap_domain = to_omap_domain(domain);
- struct omap_iommu *oiommu;
struct omap_iommu_arch_data *arch_data = dev->archdata.iommu;
+ struct omap_iommu *oiommu;
int ret = 0;
- if (!arch_data || !arch_data->name) {
+ if (!arch_data || !arch_data->iommu_dev) {
dev_err(dev, "device doesn't have an associated iommu\n");
return -EINVAL;
}
@@ -1079,15 +1062,17 @@ static size_t omap_iommu_unmap(struct iommu_domain *domain, unsigned long da,
goto out;
}
+ oiommu = arch_data->iommu_dev;
+
/* get a handle to and enable the omap iommu */
- oiommu = omap_iommu_attach(arch_data->name, omap_domain->pgtable);
- if (IS_ERR(oiommu)) {
- ret = PTR_ERR(oiommu);
+ ret = omap_iommu_attach(oiommu, omap_domain->pgtable);
+ if (ret) {
dev_err(dev, "can't get omap iommu: %d\n", ret);
goto out;
}
- omap_domain->iommu_dev = arch_data->iommu_dev = oiommu;
+ arch_data->domain = omap_domain;
+ omap_domain->iommu_dev = oiommu;
omap_domain->dev = dev;
oiommu->domain = domain;
@@ -1112,7 +1097,8 @@ static void _omap_iommu_detach_dev(struct omap_iommu_domain *omap_domain,
omap_iommu_detach(oiommu);
- omap_domain->iommu_dev = arch_data->iommu_dev = NULL;
+ omap_domain->iommu_dev = NULL;
+ arch_data->domain = NULL;
omap_domain->dev = NULL;
oiommu->domain = NULL;
}
@@ -1216,6 +1202,7 @@ static phys_addr_t omap_iommu_iova_to_phys(struct iommu_domain *domain,
static int omap_iommu_add_device(struct device *dev)
{
struct omap_iommu_arch_data *arch_data;
+ struct omap_iommu *iommu;
struct device_node *np;
struct platform_device *pdev;
@@ -1238,13 +1225,19 @@ static int omap_iommu_add_device(struct device *dev)
return -EINVAL;
}
+ iommu = platform_get_drvdata(pdev);
+ if (!iommu) {
+ of_node_put(np);
+ return -EINVAL;
+ }
+
arch_data = kzalloc(sizeof(*arch_data), GFP_KERNEL);
if (!arch_data) {
of_node_put(np);
return -ENOMEM;
}
- arch_data->name = kstrdup(dev_name(&pdev->dev), GFP_KERNEL);
+ arch_data->iommu_dev = iommu;
dev->archdata.iommu = arch_data;
of_node_put(np);
@@ -1259,7 +1252,6 @@ static void omap_iommu_remove_device(struct device *dev)
if (!dev->of_node || !arch_data)
return;
- kfree(arch_data->name);
kfree(arch_data);
}
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 3cd414a..05bd279 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -80,8 +80,8 @@ struct omap_iommu {
* utilize omap-specific plumbing anymore.
*/
struct omap_iommu_arch_data {
- const char *name;
struct omap_iommu *iommu_dev;
+ struct omap_iommu_domain *domain;
};
struct cr_regs {
--
1.9.1
From: Joerg Roedel <[email protected]>
The internal data-structures are scattered over various
header and C files. Consolidate them in omap-iommu.h.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/omap-iommu.c | 16 ----------------
drivers/iommu/omap-iommu.h | 32 ++++++++++++++++++++++++++++++++
include/linux/platform_data/iommu-omap.h | 15 ---------------
3 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index e2583cc..e9c9b08 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -42,22 +42,6 @@
/* bitmap of the page sizes currently supported */
#define OMAP_IOMMU_PGSIZES (SZ_4K | SZ_64K | SZ_1M | SZ_16M)
-/**
- * struct omap_iommu_domain - omap iommu domain
- * @pgtable: the page table
- * @iommu_dev: an omap iommu device attached to this domain. only a single
- * iommu device can be attached for now.
- * @dev: Device using this domain.
- * @lock: domain lock, should be taken when attaching/detaching
- */
-struct omap_iommu_domain {
- u32 *pgtable;
- struct omap_iommu *iommu_dev;
- struct device *dev;
- spinlock_t lock;
- struct iommu_domain domain;
-};
-
#define MMU_LOCK_BASE_SHIFT 10
#define MMU_LOCK_BASE_MASK (0x1f << MMU_LOCK_BASE_SHIFT)
#define MMU_LOCK_BASE(x) \
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 59628e5..3cd414a 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -14,6 +14,7 @@
#define _OMAP_IOMMU_H
#include <linux/bitops.h>
+#include <linux/iommu.h>
#define for_each_iotlb_cr(obj, n, __i, cr) \
for (__i = 0; \
@@ -27,6 +28,22 @@ struct iotlb_entry {
u32 endian, elsz, mixed;
};
+/**
+ * struct omap_iommu_domain - omap iommu domain
+ * @pgtable: the page table
+ * @iommu_dev: an omap iommu device attached to this domain. only a single
+ * iommu device can be attached for now.
+ * @dev: Device using this domain.
+ * @lock: domain lock, should be taken when attaching/detaching
+ */
+struct omap_iommu_domain {
+ u32 *pgtable;
+ struct omap_iommu *iommu_dev;
+ struct device *dev;
+ spinlock_t lock;
+ struct iommu_domain domain;
+};
+
struct omap_iommu {
const char *name;
void __iomem *regbase;
@@ -52,6 +69,21 @@ struct omap_iommu {
u32 id;
};
+/**
+ * struct iommu_arch_data - omap iommu private data
+ * @name: name of the iommu device
+ * @iommu_dev: handle of the iommu device
+ *
+ * This is an omap iommu private data object, which binds an iommu user
+ * to its iommu device. This object should be placed at the iommu user's
+ * dev_archdata so generic IOMMU API can be used without having to
+ * utilize omap-specific plumbing anymore.
+ */
+struct omap_iommu_arch_data {
+ const char *name;
+ struct omap_iommu *iommu_dev;
+};
+
struct cr_regs {
u32 cam;
u32 ram;
diff --git a/include/linux/platform_data/iommu-omap.h b/include/linux/platform_data/iommu-omap.h
index 0496d17..8c2a77c 100644
--- a/include/linux/platform_data/iommu-omap.h
+++ b/include/linux/platform_data/iommu-omap.h
@@ -14,21 +14,6 @@
#define MMU_REG_SIZE 256
-/**
- * struct iommu_arch_data - omap iommu private data
- * @name: name of the iommu device
- * @iommu_dev: handle of the iommu device
- *
- * This is an omap iommu private data object, which binds an iommu user
- * to its iommu device. This object should be placed at the iommu user's
- * dev_archdata so generic IOMMU API can be used without having to
- * utilize omap-specific plumbing anymore.
- */
-struct omap_iommu_arch_data {
- const char *name;
- struct omap_iommu *iommu_dev;
-};
-
struct iommu_platform_data {
const char *name;
const char *reset_name;
--
1.9.1
From: Joerg Roedel <[email protected]>
Don't leave a stale pointer in case the device continues to
exist for some more time.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/omap-iommu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 45df5c8..04b3718 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1252,6 +1252,7 @@ static void omap_iommu_remove_device(struct device *dev)
if (!dev->of_node || !arch_data)
return;
+ dev->archdata.iommu = NULL;
kfree(arch_data);
}
--
1.9.1
From: Joerg Roedel <[email protected]>
Support for IOMMU groups will become mandatory for drivers,
so add it to the omap iommu driver.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/iommu/omap-iommu.c | 43 +++++++++++++++++++++++++++++++++++++++++--
drivers/iommu/omap-iommu.h | 1 +
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 5e3a946..bd5e014 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -948,26 +948,42 @@ static int omap_iommu_probe(struct platform_device *pdev)
pm_runtime_irq_safe(obj->dev);
pm_runtime_enable(obj->dev);
+ obj->group = iommu_group_alloc();
+ if (IS_ERR(obj->group))
+ return PTR_ERR(obj->group);
+
err = iommu_device_sysfs_add(&obj->iommu, obj->dev, NULL, obj->name);
if (err)
- return err;
+ goto out_group;
iommu_device_set_ops(&obj->iommu, &omap_iommu_ops);
err = iommu_device_register(&obj->iommu);
if (err)
- return err;
+ goto out_sysfs;
omap_iommu_debugfs_add(obj);
dev_info(&pdev->dev, "%s registered\n", obj->name);
+
return 0;
+
+out_sysfs:
+ iommu_device_sysfs_remove(&obj->iommu);
+
+out_group:
+ iommu_group_put(obj->group);
+
+ return err;
}
static int omap_iommu_remove(struct platform_device *pdev)
{
struct omap_iommu *obj = platform_get_drvdata(pdev);
+ iommu_group_put(obj->group);
+ obj->group = NULL;
+
iommu_device_sysfs_remove(&obj->iommu);
iommu_device_unregister(&obj->iommu);
@@ -1217,6 +1233,7 @@ static phys_addr_t omap_iommu_iova_to_phys(struct iommu_domain *domain,
static int omap_iommu_add_device(struct device *dev)
{
struct omap_iommu_arch_data *arch_data;
+ struct iommu_group *group;
struct omap_iommu *iommu;
struct device_node *np;
struct platform_device *pdev;
@@ -1262,6 +1279,15 @@ static int omap_iommu_add_device(struct device *dev)
arch_data->iommu_dev = iommu;
dev->archdata.iommu = arch_data;
+ /*
+ * IOMMU group initialization calls into omap_device_group, which needs
+ * a valid dev->archdata.iommu pointer
+ */
+ group = iommu_group_get_for_dev(dev);
+ if (IS_ERR(group))
+ return PTR_ERR(group);
+ iommu_group_put(group);
+
of_node_put(np);
return 0;
@@ -1275,12 +1301,24 @@ static void omap_iommu_remove_device(struct device *dev)
return;
iommu_device_unlink(&arch_data->iommu_dev->iommu, dev);
+ iommu_group_remove_device(dev);
dev->archdata.iommu = NULL;
kfree(arch_data);
}
+static struct iommu_group *omap_device_group(struct device *dev)
+{
+ struct omap_iommu_arch_data *arch_data = dev->archdata.iommu;
+ struct iommu_group *group = NULL;
+
+ if (arch_data->iommu_dev)
+ group = arch_data->iommu_dev->group;
+
+ return group;
+}
+
static const struct iommu_ops omap_iommu_ops = {
.domain_alloc = omap_iommu_domain_alloc,
.domain_free = omap_iommu_domain_free,
@@ -1292,6 +1330,7 @@ static void omap_iommu_remove_device(struct device *dev)
.iova_to_phys = omap_iommu_iova_to_phys,
.add_device = omap_iommu_add_device,
.remove_device = omap_iommu_remove_device,
+ .device_group = omap_device_group,
.pgsize_bitmap = OMAP_IOMMU_PGSIZES,
};
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 07e6ea1..28483c5 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -69,6 +69,7 @@ struct omap_iommu {
u32 id;
struct iommu_device iommu;
+ struct iommu_group *group;
};
/**
--
1.9.1
Hi Joerg,
On 03/31/2017 07:10 AM, Joerg Roedel wrote:
> Hi,
>
> here is a small patch-set for the omap-iommu driver to make
> it use new features of the iommu-core. Please review.
Thanks for the patches - this has been on my TODO list but you beat me
in getting there first :). Are these for 4.12?
I have given the series a quick try and these currently break the
functionality. I am investigating the failures, so will mostly need some
additional changes. Let me know if you want me to take up posting of the
v2. Meanwhile, I have added some comments on the patches.
regards
Suman
>
> Thanks,
>
> Joerg
>
> Joerg Roedel (5):
> iommu/omap: Move data structures to omap-iommu.h
> iommu/omap: Permanently keep iommu_dev pointer in arch_data
> iommu/omap: Set dev->archdata.iommu = NULL in omap_iommu_remove_device
> iommu/omap: Make use of 'struct iommu_device'
> iommu/omap: Add iommu-group support
>
> drivers/iommu/omap-iommu.c | 137 ++++++++++++++++++++-----------
> drivers/iommu/omap-iommu.h | 35 ++++++++
> include/linux/platform_data/iommu-omap.h | 15 ----
> 3 files changed, 124 insertions(+), 63 deletions(-)
>
On 03/31/2017 07:10 AM, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> The internal data-structures are scattered over various
> header and C files. Consolidate them in omap-iommu.h.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/omap-iommu.c | 16 ----------------
> drivers/iommu/omap-iommu.h | 32 ++++++++++++++++++++++++++++++++
> include/linux/platform_data/iommu-omap.h | 15 ---------------
> 3 files changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index e2583cc..e9c9b08 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -42,22 +42,6 @@
> /* bitmap of the page sizes currently supported */
> #define OMAP_IOMMU_PGSIZES (SZ_4K | SZ_64K | SZ_1M | SZ_16M)
>
> -/**
> - * struct omap_iommu_domain - omap iommu domain
> - * @pgtable: the page table
> - * @iommu_dev: an omap iommu device attached to this domain. only a single
> - * iommu device can be attached for now.
> - * @dev: Device using this domain.
> - * @lock: domain lock, should be taken when attaching/detaching
> - */
> -struct omap_iommu_domain {
> - u32 *pgtable;
> - struct omap_iommu *iommu_dev;
> - struct device *dev;
> - spinlock_t lock;
> - struct iommu_domain domain;
> -};
> -
> #define MMU_LOCK_BASE_SHIFT 10
> #define MMU_LOCK_BASE_MASK (0x1f << MMU_LOCK_BASE_SHIFT)
> #define MMU_LOCK_BASE(x) \
> diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
> index 59628e5..3cd414a 100644
> --- a/drivers/iommu/omap-iommu.h
> +++ b/drivers/iommu/omap-iommu.h
> @@ -14,6 +14,7 @@
> #define _OMAP_IOMMU_H
>
> #include <linux/bitops.h>
> +#include <linux/iommu.h>
>
> #define for_each_iotlb_cr(obj, n, __i, cr) \
> for (__i = 0; \
> @@ -27,6 +28,22 @@ struct iotlb_entry {
> u32 endian, elsz, mixed;
> };
>
> +/**
> + * struct omap_iommu_domain - omap iommu domain
> + * @pgtable: the page table
> + * @iommu_dev: an omap iommu device attached to this domain. only a single
> + * iommu device can be attached for now.
> + * @dev: Device using this domain.
> + * @lock: domain lock, should be taken when attaching/detaching
> + */
> +struct omap_iommu_domain {
> + u32 *pgtable;
> + struct omap_iommu *iommu_dev;
> + struct device *dev;
> + spinlock_t lock;
> + struct iommu_domain domain;
> +};
> +
> struct omap_iommu {
> const char *name;
> void __iomem *regbase;
> @@ -52,6 +69,21 @@ struct omap_iommu {
> u32 id;
> };
>
> +/**
> + * struct iommu_arch_data - omap iommu private data
> + * @name: name of the iommu device
> + * @iommu_dev: handle of the iommu device
> + *
> + * This is an omap iommu private data object, which binds an iommu user
> + * to its iommu device. This object should be placed at the iommu user's
> + * dev_archdata so generic IOMMU API can be used without having to
> + * utilize omap-specific plumbing anymore.
> + */
> +struct omap_iommu_arch_data {
> + const char *name;
> + struct omap_iommu *iommu_dev;
> +};
> +
> struct cr_regs {
> u32 cam;
> u32 ram;
> diff --git a/include/linux/platform_data/iommu-omap.h b/include/linux/platform_data/iommu-omap.h
> index 0496d17..8c2a77c 100644
> --- a/include/linux/platform_data/iommu-omap.h
> +++ b/include/linux/platform_data/iommu-omap.h
> @@ -14,21 +14,6 @@
>
> #define MMU_REG_SIZE 256
This define can be dropped as well since you are already modifying this
file, it's already defined in drivers/iommu/omap-iommu.h.
regards
Suman
>
> -/**
> - * struct iommu_arch_data - omap iommu private data
> - * @name: name of the iommu device
> - * @iommu_dev: handle of the iommu device
> - *
> - * This is an omap iommu private data object, which binds an iommu user
> - * to its iommu device. This object should be placed at the iommu user's
> - * dev_archdata so generic IOMMU API can be used without having to
> - * utilize omap-specific plumbing anymore.
> - */
> -struct omap_iommu_arch_data {
> - const char *name;
> - struct omap_iommu *iommu_dev;
> -};
> -
> struct iommu_platform_data {
> const char *name;
> const char *reset_name;
>
Hi Joerg,
On 03/31/2017 07:10 AM, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> Don't leave a stale pointer in case the device continues to
> exist for some more time.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/omap-iommu.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 45df5c8..04b3718 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1252,6 +1252,7 @@ static void omap_iommu_remove_device(struct device *dev)
> if (!dev->of_node || !arch_data)
> return;
>
> + dev->archdata.iommu = NULL;
This can be squashed into Patch 2 to go alongside the matching change in
omap_iommu_attach_device().
regards
Suman
> kfree(arch_data);
> }
>
>
Hi Joerg,
On 03/31/2017 07:10 AM, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> Instead of finding the matching IOMMU for a device using
> string comparision functions, keep the pointer to the
> iommu_dev in arch_data permanently populated.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/omap-iommu.c | 56 ++++++++++++++++++++--------------------------
> drivers/iommu/omap-iommu.h | 2 +-
> 2 files changed, 25 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index e9c9b08..45df5c8 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -802,33 +802,14 @@ static irqreturn_t iommu_fault_handler(int irq, void *data)
> return IRQ_NONE;
> }
>
> -static int device_match_by_alias(struct device *dev, void *data)
> -{
> - struct omap_iommu *obj = to_iommu(dev);
> - const char *name = data;
> -
> - pr_debug("%s: %s %s\n", __func__, obj->name, name);
> -
> - return strcmp(obj->name, name) == 0;
> -}
> -
> /**
> * omap_iommu_attach() - attach iommu device to an iommu domain
> - * @name: name of target omap iommu device
> + * @obj: target omap iommu device
> * @iopgd: page table
> **/
> -static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd)
> +static int omap_iommu_attach(struct omap_iommu *obj, u32 *iopgd)
> {
> int err;
> - struct device *dev;
> - struct omap_iommu *obj;
> -
> - dev = driver_find_device(&omap_iommu_driver.driver, NULL, (void *)name,
> - device_match_by_alias);
> - if (!dev)
> - return ERR_PTR(-ENODEV);
> -
> - obj = to_iommu(dev);
>
> spin_lock(&obj->iommu_lock);
>
> @@ -841,11 +822,13 @@ static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd)
> spin_unlock(&obj->iommu_lock);
>
> dev_dbg(obj->dev, "%s: %s\n", __func__, obj->name);
> - return obj;
> +
> + return 0;
>
> err_enable:
> spin_unlock(&obj->iommu_lock);
> - return ERR_PTR(err);
> +
> + return err;
> }
>
> /**
> @@ -1061,11 +1044,11 @@ static size_t omap_iommu_unmap(struct iommu_domain *domain, unsigned long da,
> omap_iommu_attach_dev(struct iommu_domain *domain, struct device *dev)
> {
> struct omap_iommu_domain *omap_domain = to_omap_domain(domain);
> - struct omap_iommu *oiommu;
> struct omap_iommu_arch_data *arch_data = dev->archdata.iommu;
> + struct omap_iommu *oiommu;
> int ret = 0;
>
> - if (!arch_data || !arch_data->name) {
> + if (!arch_data || !arch_data->iommu_dev) {
> dev_err(dev, "device doesn't have an associated iommu\n");
> return -EINVAL;
> }
> @@ -1079,15 +1062,17 @@ static size_t omap_iommu_unmap(struct iommu_domain *domain, unsigned long da,
> goto out;
> }
>
> + oiommu = arch_data->iommu_dev;
> +
> /* get a handle to and enable the omap iommu */
> - oiommu = omap_iommu_attach(arch_data->name, omap_domain->pgtable);
> - if (IS_ERR(oiommu)) {
> - ret = PTR_ERR(oiommu);
> + ret = omap_iommu_attach(oiommu, omap_domain->pgtable);
> + if (ret) {
> dev_err(dev, "can't get omap iommu: %d\n", ret);
> goto out;
> }
>
> - omap_domain->iommu_dev = arch_data->iommu_dev = oiommu;
> + arch_data->domain = omap_domain;
> + omap_domain->iommu_dev = oiommu;
> omap_domain->dev = dev;
> oiommu->domain = domain;
>
> @@ -1112,7 +1097,8 @@ static void _omap_iommu_detach_dev(struct omap_iommu_domain *omap_domain,
>
> omap_iommu_detach(oiommu);
>
> - omap_domain->iommu_dev = arch_data->iommu_dev = NULL;
> + omap_domain->iommu_dev = NULL;
> + arch_data->domain = NULL;
> omap_domain->dev = NULL;
> oiommu->domain = NULL;
> }
> @@ -1216,6 +1202,7 @@ static phys_addr_t omap_iommu_iova_to_phys(struct iommu_domain *domain,
> static int omap_iommu_add_device(struct device *dev)
> {
> struct omap_iommu_arch_data *arch_data;
> + struct omap_iommu *iommu;
> struct device_node *np;
> struct platform_device *pdev;
>
> @@ -1238,13 +1225,19 @@ static int omap_iommu_add_device(struct device *dev)
> return -EINVAL;
> }
>
> + iommu = platform_get_drvdata(pdev);
> + if (!iommu) {
> + of_node_put(np);
> + return -EINVAL;
> + }
This change is causing the issues. OMAP IOMMU driver is not probed yet,
but this gets called during the driver's init function in
bus_set_iommu() and bus's iommu_ops gets set to NULL. The add_device
today is used to add the linking to an IOMMU device (currently name
primarily to support the legacy non-DT mode which is no longer an issue,
but a dev pointer can be used instead here, and the real enabling is
done during the domain's attach_dev callback.
> +
> arch_data = kzalloc(sizeof(*arch_data), GFP_KERNEL);
> if (!arch_data) {
> of_node_put(np);
> return -ENOMEM;
> }
>
> - arch_data->name = kstrdup(dev_name(&pdev->dev), GFP_KERNEL);
> + arch_data->iommu_dev = iommu;
> dev->archdata.iommu = arch_data;
>
> of_node_put(np);
> @@ -1259,7 +1252,6 @@ static void omap_iommu_remove_device(struct device *dev)
> if (!dev->of_node || !arch_data)
> return;
>
> - kfree(arch_data->name);
> kfree(arch_data);
> }
>
> diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
> index 3cd414a..05bd279 100644
> --- a/drivers/iommu/omap-iommu.h
> +++ b/drivers/iommu/omap-iommu.h
> @@ -80,8 +80,8 @@ struct omap_iommu {
> * utilize omap-specific plumbing anymore.
> */
> struct omap_iommu_arch_data {
> - const char *name;
Missing the removal from the structure kerneldoc documentation.
> struct omap_iommu *iommu_dev;
> + struct omap_iommu_domain *domain;
This doesn't seem to serve any purpose for now, is there a plan/purpose
for this? The iommu_domain is being stored within the omap_iommu
structure at the moment and it is stored during attach_dev() callback.
regards
Suman
> };
>
> struct cr_regs {
>
Hi Suman,
On Mon, Apr 03, 2017 at 03:35:46PM -0500, Suman Anna wrote:
> > + iommu = platform_get_drvdata(pdev);
> > + if (!iommu) {
> > + of_node_put(np);
> > + return -EINVAL;
> > + }
>
> This change is causing the issues. OMAP IOMMU driver is not probed yet,
> but this gets called during the driver's init function in
> bus_set_iommu() and bus's iommu_ops gets set to NULL. The add_device
> today is used to add the linking to an IOMMU device (currently name
> primarily to support the legacy non-DT mode which is no longer an issue,
> but a dev pointer can be used instead here, and the real enabling is
> done during the domain's attach_dev callback.
Yeah, okay. Solving this problem requires more sophisticated handling in
the iommu core code, which is not implemented yet.
I drop this patch for now and do the device-linking and group-handling
in attach_dev. This makes iommu-groups on omap useless for now, but it
is a start.
Joerg