This series fixes some issues in rockchip iommu driver, and add of_iommu
support in it.
Also drop of_iommu early initialisation hooks as Robin suggested.
Jeffy Chen (5):
iommu/of: Drop early initialisation hooks
iommu/rockchip: Fix error handling in probe
iommu/rockchip: Fix error handling in init
iommu/rockchip: Use IOMMU device for dma mapping operations
iommu/rockchip: Use OF_IOMMU to attach devices automatically
Tomasz Figa (4):
iommu/rockchip: Fix error handling in attach
iommu/rockchip: Use iopoll helpers to wait for hardware
iommu/rockchip: Fix TLB flush of secondary IOMMUs
iommu/rockchip: Use iommu_group_get_for_dev() for add_device
drivers/iommu/arm-smmu-v3.c | 2 +-
drivers/iommu/arm-smmu.c | 12 +-
drivers/iommu/exynos-iommu.c | 2 +-
drivers/iommu/ipmmu-vmsa.c | 17 +--
drivers/iommu/msm_iommu.c | 24 +--
drivers/iommu/of_iommu.c | 16 --
drivers/iommu/qcom_iommu.c | 2 +-
drivers/iommu/rockchip-iommu.c | 331 ++++++++++++++++++++---------------------
include/linux/of_iommu.h | 6 +-
9 files changed, 184 insertions(+), 228 deletions(-)
--
2.11.0
With the probe-deferral mechanism, early initialisation hooks are no
longer needed.
Suggested-by: Robin Murphy <[email protected]>
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/iommu/arm-smmu-v3.c | 2 +-
drivers/iommu/arm-smmu.c | 12 ++++++------
drivers/iommu/exynos-iommu.c | 2 +-
drivers/iommu/ipmmu-vmsa.c | 17 ++---------------
drivers/iommu/msm_iommu.c | 24 +++++++-----------------
drivers/iommu/of_iommu.c | 16 ----------------
drivers/iommu/qcom_iommu.c | 2 +-
include/linux/of_iommu.h | 6 ++----
8 files changed, 20 insertions(+), 61 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 744592d330ca..3f2f1fc68b52 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2971,7 +2971,7 @@ static struct platform_driver arm_smmu_driver = {
};
module_platform_driver(arm_smmu_driver);
-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", NULL);
+IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
MODULE_AUTHOR("Will Deacon <[email protected]>");
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 78d4c6b8f1ba..69e7c60792a8 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2211,12 +2211,12 @@ static struct platform_driver arm_smmu_driver = {
};
module_platform_driver(arm_smmu_driver);
-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1", NULL);
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2", NULL);
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400", NULL);
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", NULL);
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", NULL);
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", NULL);
+IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
+IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
+IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
+IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
+IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
+IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
MODULE_AUTHOR("Will Deacon <[email protected]>");
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 79c45650f8de..1c7f926fad0e 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1394,4 +1394,4 @@ static int __init exynos_iommu_init(void)
}
core_initcall(exynos_iommu_init);
-IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu", NULL);
+IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 8dce3a9de9d8..07b711bb4b16 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1081,12 +1081,8 @@ static struct platform_driver ipmmu_driver = {
static int __init ipmmu_init(void)
{
- static bool setup_done;
int ret;
- if (setup_done)
- return 0;
-
ret = platform_driver_register(&ipmmu_driver);
if (ret < 0)
return ret;
@@ -1096,7 +1092,6 @@ static int __init ipmmu_init(void)
bus_set_iommu(&platform_bus_type, &ipmmu_ops);
#endif
- setup_done = true;
return 0;
}
@@ -1109,16 +1104,8 @@ subsys_initcall(ipmmu_init);
module_exit(ipmmu_exit);
#ifdef CONFIG_IOMMU_DMA
-static int __init ipmmu_vmsa_iommu_of_setup(struct device_node *np)
-{
- ipmmu_init();
- return 0;
-}
-
-IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa",
- ipmmu_vmsa_iommu_of_setup);
-IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795",
- ipmmu_vmsa_iommu_of_setup);
+IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
+IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
#endif
MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 04f4d51ffacb..a41d4251b0a9 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -856,7 +856,7 @@ static struct platform_driver msm_iommu_driver = {
.remove = msm_iommu_remove,
};
-static int __init msm_iommu_driver_init(void)
+static int __init msm_iommu_init(void)
{
int ret;
@@ -864,30 +864,20 @@ static int __init msm_iommu_driver_init(void)
if (ret != 0)
pr_err("Failed to register IOMMU driver\n");
+ bus_set_iommu(&platform_bus_type, &msm_iommu_ops);
+
return ret;
}
-static void __exit msm_iommu_driver_exit(void)
+static void __exit msm_iommu_exit(void)
{
platform_driver_unregister(&msm_iommu_driver);
}
-subsys_initcall(msm_iommu_driver_init);
-module_exit(msm_iommu_driver_exit);
-
-static int __init msm_iommu_init(void)
-{
- bus_set_iommu(&platform_bus_type, &msm_iommu_ops);
- return 0;
-}
-
-static int __init msm_iommu_of_setup(struct device_node *np)
-{
- msm_iommu_init();
- return 0;
-}
+subsys_initcall(msm_iommu_init);
+module_exit(msm_iommu_exit);
-IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu", msm_iommu_of_setup);
+IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Stepan Moskovchenko <[email protected]>");
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 50947ebb6d17..5c36a8b7656a 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -231,19 +231,3 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
return ops;
}
-
-static int __init of_iommu_init(void)
-{
- struct device_node *np;
- const struct of_device_id *match, *matches = &__iommu_of_table;
-
- for_each_matching_node_and_match(np, matches, &match) {
- const of_iommu_init_fn init_fn = match->data;
-
- if (init_fn && init_fn(np))
- pr_err("Failed to initialise IOMMU %pOF\n", np);
- }
-
- return 0;
-}
-postcore_initcall_sync(of_iommu_init);
diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index e07f02d00c68..65b9c99707f8 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -947,7 +947,7 @@ static void __exit qcom_iommu_exit(void)
module_init(qcom_iommu_init);
module_exit(qcom_iommu_exit);
-IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1", NULL);
+IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1");
MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations");
MODULE_LICENSE("GPL v2");
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index cddfaff4d0b7..ae03752e8e3a 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -34,9 +34,7 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
extern struct of_device_id __iommu_of_table;
-typedef int (*of_iommu_init_fn)(struct device_node *);
-
-#define IOMMU_OF_DECLARE(name, compat, fn) \
- _OF_DECLARE(iommu, name, compat, fn, of_iommu_init_fn)
+#define IOMMU_OF_DECLARE(name, compat) \
+ OF_DECLARE_1(iommu, name, compat, NULL)
#endif /* __OF_IOMMU_H */
--
2.11.0
From: Tomasz Figa <[email protected]>
Currently if the driver encounters an error while attaching device, it
will leave the IOMMU in an inconsistent state. Even though it shouldn't
really happen in reality, let's just add proper error path to keep
things consistent.
Signed-off-by: Tomasz Figa <[email protected]>
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 9d991c2d8767..ee805e1dfba7 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -826,17 +826,10 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
ret = rk_iommu_force_reset(iommu);
if (ret)
- return ret;
+ goto err_disable_stall;
iommu->domain = domain;
- for (i = 0; i < iommu->num_irq; i++) {
- ret = devm_request_irq(iommu->dev, iommu->irq[i], rk_iommu_irq,
- IRQF_SHARED, dev_name(dev), iommu);
- if (ret)
- return ret;
- }
-
for (i = 0; i < iommu->num_mmu; i++) {
rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR,
rk_domain->dt_dma);
@@ -844,9 +837,16 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
rk_iommu_write(iommu->bases[i], RK_MMU_INT_MASK, RK_MMU_IRQ_MASK);
}
+ for (i = 0; i < iommu->num_irq; i++) {
+ ret = devm_request_irq(iommu->dev, iommu->irq[i], rk_iommu_irq,
+ IRQF_SHARED, dev_name(dev), iommu);
+ if (ret)
+ goto err_free_irq;
+ }
+
ret = rk_iommu_enable_paging(iommu);
if (ret)
- return ret;
+ goto err_free_irq;
spin_lock_irqsave(&rk_domain->iommus_lock, flags);
list_add_tail(&iommu->node, &rk_domain->iommus);
@@ -857,6 +857,14 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
rk_iommu_disable_stall(iommu);
return 0;
+
+err_free_irq:
+ while (i--)
+ devm_free_irq(iommu->dev, iommu->irq[i], iommu);
+err_disable_stall:
+ rk_iommu_disable_stall(iommu);
+
+ return ret;
}
static void rk_iommu_detach_device(struct iommu_domain *domain,
--
2.11.0
Add missing iommu_device_sysfs_remove in error path.
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index ee805e1dfba7..a05844cabb45 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1201,8 +1201,12 @@ static int rk_iommu_probe(struct platform_device *pdev)
iommu_device_set_ops(&iommu->iommu, &rk_iommu_ops);
err = iommu_device_register(&iommu->iommu);
+ if (err) {
+ iommu_device_sysfs_remove(&iommu->iommu);
+ return err;
+ }
- return err;
+ return 0;
}
static int rk_iommu_remove(struct platform_device *pdev)
--
2.11.0
It's hard to undo bus_set_iommu() in the error path, so move it to the
end of init call.
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index a05844cabb45..5f141390b4d7 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1247,17 +1247,25 @@ static int __init rk_iommu_init(void)
of_node_put(np);
- ret = bus_set_iommu(&platform_bus_type, &rk_iommu_ops);
- if (ret)
- return ret;
-
ret = platform_driver_register(&rk_iommu_domain_driver);
if (ret)
return ret;
ret = platform_driver_register(&rk_iommu_driver);
if (ret)
- platform_driver_unregister(&rk_iommu_domain_driver);
+ goto err_unreg_domain_drv;
+
+ ret = bus_set_iommu(&platform_bus_type, &rk_iommu_ops);
+ if (ret)
+ goto err_unreg_iommu_drv;
+
+ return 0;
+
+err_unreg_iommu_drv:
+ platform_driver_unregister(&rk_iommu_driver);
+err_unreg_domain_drv:
+ platform_driver_unregister(&rk_iommu_domain_driver);
+
return ret;
}
static void __exit rk_iommu_exit(void)
--
2.11.0
From: Tomasz Figa <[email protected]>
This patch converts the rockchip-iommu driver to use the in-kernel
iopoll helpers to wait for certain status bits to change in registers
instead of an open-coded custom macro.
Signed-off-by: Tomasz Figa <[email protected]>
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 68 ++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 36 deletions(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 5f141390b4d7..6b797e085340 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -13,7 +13,7 @@
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/iommu.h>
-#include <linux/jiffies.h>
+#include <linux/iopoll.h>
#include <linux/list.h>
#include <linux/mm.h>
#include <linux/module.h>
@@ -36,7 +36,8 @@
#define RK_MMU_AUTO_GATING 0x24
#define DTE_ADDR_DUMMY 0xCAFEBABE
-#define FORCE_RESET_TIMEOUT 100 /* ms */
+#define FORCE_RESET_TIMEOUT 100000 /* us */
+#define POLL_TIMEOUT 1000 /* us */
/* RK_MMU_STATUS fields */
#define RK_MMU_STATUS_PAGING_ENABLED BIT(0)
@@ -73,8 +74,6 @@
*/
#define RK_IOMMU_PGSIZE_BITMAP 0x007ff000
-#define IOMMU_REG_POLL_COUNT_FAST 1000
-
struct rk_iommu_domain {
struct list_head iommus;
struct platform_device *pdev;
@@ -111,27 +110,6 @@ static struct rk_iommu_domain *to_rk_domain(struct iommu_domain *dom)
return container_of(dom, struct rk_iommu_domain, domain);
}
-/**
- * Inspired by _wait_for in intel_drv.h
- * This is NOT safe for use in interrupt context.
- *
- * Note that it's important that we check the condition again after having
- * timed out, since the timeout could be due to preemption or similar and
- * we've never had a chance to check the condition before the timeout.
- */
-#define rk_wait_for(COND, MS) ({ \
- unsigned long timeout__ = jiffies + msecs_to_jiffies(MS) + 1; \
- int ret__ = 0; \
- while (!(COND)) { \
- if (time_after(jiffies, timeout__)) { \
- ret__ = (COND) ? 0 : -ETIMEDOUT; \
- break; \
- } \
- usleep_range(50, 100); \
- } \
- ret__; \
-})
-
/*
* The Rockchip rk3288 iommu uses a 2-level page table.
* The first level is the "Directory Table" (DT).
@@ -335,9 +313,21 @@ static bool rk_iommu_is_paging_enabled(struct rk_iommu *iommu)
return enable;
}
+static bool rk_iommu_is_reset_done(struct rk_iommu *iommu)
+{
+ bool done = true;
+ int i;
+
+ for (i = 0; i < iommu->num_mmu; i++)
+ done &= rk_iommu_read(iommu->bases[i], RK_MMU_DTE_ADDR) == 0;
+
+ return done;
+}
+
static int rk_iommu_enable_stall(struct rk_iommu *iommu)
{
int ret, i;
+ bool val;
if (rk_iommu_is_stall_active(iommu))
return 0;
@@ -348,7 +338,8 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu)
rk_iommu_command(iommu, RK_MMU_CMD_ENABLE_STALL);
- ret = rk_wait_for(rk_iommu_is_stall_active(iommu), 1);
+ ret = readx_poll_timeout(rk_iommu_is_stall_active, iommu, val,
+ val, 100, POLL_TIMEOUT);
if (ret)
for (i = 0; i < iommu->num_mmu; i++)
dev_err(iommu->dev, "Enable stall request timed out, status: %#08x\n",
@@ -360,13 +351,15 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu)
static int rk_iommu_disable_stall(struct rk_iommu *iommu)
{
int ret, i;
+ bool val;
if (!rk_iommu_is_stall_active(iommu))
return 0;
rk_iommu_command(iommu, RK_MMU_CMD_DISABLE_STALL);
- ret = rk_wait_for(!rk_iommu_is_stall_active(iommu), 1);
+ ret = readx_poll_timeout(rk_iommu_is_stall_active, iommu, val,
+ !val, 100, POLL_TIMEOUT);
if (ret)
for (i = 0; i < iommu->num_mmu; i++)
dev_err(iommu->dev, "Disable stall request timed out, status: %#08x\n",
@@ -378,13 +371,15 @@ static int rk_iommu_disable_stall(struct rk_iommu *iommu)
static int rk_iommu_enable_paging(struct rk_iommu *iommu)
{
int ret, i;
+ bool val;
if (rk_iommu_is_paging_enabled(iommu))
return 0;
rk_iommu_command(iommu, RK_MMU_CMD_ENABLE_PAGING);
- ret = rk_wait_for(rk_iommu_is_paging_enabled(iommu), 1);
+ ret = readx_poll_timeout(rk_iommu_is_paging_enabled, iommu, val,
+ val, 100, POLL_TIMEOUT);
if (ret)
for (i = 0; i < iommu->num_mmu; i++)
dev_err(iommu->dev, "Enable paging request timed out, status: %#08x\n",
@@ -396,13 +391,15 @@ static int rk_iommu_enable_paging(struct rk_iommu *iommu)
static int rk_iommu_disable_paging(struct rk_iommu *iommu)
{
int ret, i;
+ bool val;
if (!rk_iommu_is_paging_enabled(iommu))
return 0;
rk_iommu_command(iommu, RK_MMU_CMD_DISABLE_PAGING);
- ret = rk_wait_for(!rk_iommu_is_paging_enabled(iommu), 1);
+ ret = readx_poll_timeout(rk_iommu_is_paging_enabled, iommu, val,
+ !val, 100, POLL_TIMEOUT);
if (ret)
for (i = 0; i < iommu->num_mmu; i++)
dev_err(iommu->dev, "Disable paging request timed out, status: %#08x\n",
@@ -415,6 +412,7 @@ static int rk_iommu_force_reset(struct rk_iommu *iommu)
{
int ret, i;
u32 dte_addr;
+ bool val;
if (iommu->reset_disabled)
return 0;
@@ -435,13 +433,11 @@ static int rk_iommu_force_reset(struct rk_iommu *iommu)
rk_iommu_command(iommu, RK_MMU_CMD_FORCE_RESET);
- for (i = 0; i < iommu->num_mmu; i++) {
- ret = rk_wait_for(rk_iommu_read(iommu->bases[i], RK_MMU_DTE_ADDR) == 0x00000000,
- FORCE_RESET_TIMEOUT);
- if (ret) {
- dev_err(iommu->dev, "FORCE_RESET command timed out\n");
- return ret;
- }
+ ret = readx_poll_timeout(rk_iommu_is_reset_done, iommu, val,
+ val, 100, FORCE_RESET_TIMEOUT);
+ if (ret) {
+ dev_err(iommu->dev, "FORCE_RESET command timed out\n");
+ return ret;
}
return 0;
--
2.11.0
From: Tomasz Figa <[email protected]>
Due to the bug in current code, only first IOMMU has the TLB lines
flushed in rk_iommu_zap_lines. This patch fixes the inner loop to
execute for all IOMMUs and properly flush the TLB.
Signed-off-by: Tomasz Figa <[email protected]>
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 6b797e085340..cfeafbf54096 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -274,19 +274,21 @@ static void rk_iommu_base_command(void __iomem *base, u32 command)
{
writel(command, base + RK_MMU_COMMAND);
}
-static void rk_iommu_zap_lines(struct rk_iommu *iommu, dma_addr_t iova,
+static void rk_iommu_zap_lines(struct rk_iommu *iommu, dma_addr_t iova_start,
size_t size)
{
int i;
-
- dma_addr_t iova_end = iova + size;
+ dma_addr_t iova_end = iova_start + size;
/*
* TODO(djkurtz): Figure out when it is more efficient to shootdown the
* entire iotlb rather than iterate over individual iovas.
*/
- for (i = 0; i < iommu->num_mmu; i++)
- for (; iova < iova_end; iova += SPAGE_SIZE)
+ for (i = 0; i < iommu->num_mmu; i++) {
+ dma_addr_t iova;
+
+ for (iova = iova_start; iova < iova_end; iova += SPAGE_SIZE)
rk_iommu_write(iommu->bases[i], RK_MMU_ZAP_ONE_LINE, iova);
+ }
}
static bool rk_iommu_is_stall_active(struct rk_iommu *iommu)
--
2.11.0
Use the first registered IOMMU device for dma mapping operations, and
drop the domain platform device.
This is similar to exynos iommu driver.
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 96 ++++++++++++------------------------------
1 file changed, 28 insertions(+), 68 deletions(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 86f8190d7bed..ab18a80fdd12 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -76,7 +76,6 @@
struct rk_iommu_domain {
struct list_head iommus;
- struct platform_device *pdev;
u32 *dt; /* page directory table */
dma_addr_t dt_dma;
spinlock_t iommus_lock; /* lock for iommus list */
@@ -97,12 +96,14 @@ struct rk_iommu {
struct iommu_domain *domain; /* domain to which iommu is attached */
};
+static struct device *dma_dev;
+
static inline void rk_table_flush(struct rk_iommu_domain *dom, dma_addr_t dma,
unsigned int count)
{
size_t size = count * sizeof(u32); /* count of u32 entry */
- dma_sync_single_for_device(&dom->pdev->dev, dma, size, DMA_TO_DEVICE);
+ dma_sync_single_for_device(dma_dev, dma, size, DMA_TO_DEVICE);
}
static struct rk_iommu_domain *to_rk_domain(struct iommu_domain *dom)
@@ -606,7 +607,6 @@ static void rk_iommu_zap_iova_first_last(struct rk_iommu_domain *rk_domain,
static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain,
dma_addr_t iova)
{
- struct device *dev = &rk_domain->pdev->dev;
u32 *page_table, *dte_addr;
u32 dte_index, dte;
phys_addr_t pt_phys;
@@ -624,9 +624,9 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain,
if (!page_table)
return ERR_PTR(-ENOMEM);
- pt_dma = dma_map_single(dev, page_table, SPAGE_SIZE, DMA_TO_DEVICE);
- if (dma_mapping_error(dev, pt_dma)) {
- dev_err(dev, "DMA mapping error while allocating page table\n");
+ pt_dma = dma_map_single(dma_dev, page_table, SPAGE_SIZE, DMA_TO_DEVICE);
+ if (dma_mapping_error(dma_dev, pt_dma)) {
+ dev_err(dma_dev, "DMA mapping error while allocating page table\n");
free_page((unsigned long)page_table);
return ERR_PTR(-ENOMEM);
}
@@ -905,29 +905,20 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
{
struct rk_iommu_domain *rk_domain;
- struct platform_device *pdev;
- struct device *iommu_dev;
if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
return NULL;
- /* Register a pdev per domain, so DMA API can base on this *dev
- * even some virtual master doesn't have an iommu slave
- */
- pdev = platform_device_register_simple("rk_iommu_domain",
- PLATFORM_DEVID_AUTO, NULL, 0);
- if (IS_ERR(pdev))
+ if (!dma_dev)
return NULL;
- rk_domain = devm_kzalloc(&pdev->dev, sizeof(*rk_domain), GFP_KERNEL);
+ rk_domain = devm_kzalloc(dma_dev, sizeof(*rk_domain), GFP_KERNEL);
if (!rk_domain)
- goto err_unreg_pdev;
-
- rk_domain->pdev = pdev;
+ return NULL;
if (type == IOMMU_DOMAIN_DMA &&
iommu_get_dma_cookie(&rk_domain->domain))
- goto err_unreg_pdev;
+ return NULL;
/*
* rk32xx iommus use a 2 level pagetable.
@@ -938,11 +929,10 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
if (!rk_domain->dt)
goto err_put_cookie;
- iommu_dev = &pdev->dev;
- rk_domain->dt_dma = dma_map_single(iommu_dev, rk_domain->dt,
+ rk_domain->dt_dma = dma_map_single(dma_dev, rk_domain->dt,
SPAGE_SIZE, DMA_TO_DEVICE);
- if (dma_mapping_error(iommu_dev, rk_domain->dt_dma)) {
- dev_err(iommu_dev, "DMA map error for DT\n");
+ if (dma_mapping_error(dma_dev, rk_domain->dt_dma)) {
+ dev_err(dma_dev, "DMA map error for DT\n");
goto err_free_dt;
}
@@ -963,8 +953,6 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
err_put_cookie:
if (type == IOMMU_DOMAIN_DMA)
iommu_put_dma_cookie(&rk_domain->domain);
-err_unreg_pdev:
- platform_device_unregister(pdev);
return NULL;
}
@@ -981,20 +969,18 @@ static void rk_iommu_domain_free(struct iommu_domain *domain)
if (rk_dte_is_pt_valid(dte)) {
phys_addr_t pt_phys = rk_dte_pt_address(dte);
u32 *page_table = phys_to_virt(pt_phys);
- dma_unmap_single(&rk_domain->pdev->dev, pt_phys,
+ dma_unmap_single(dma_dev, pt_phys,
SPAGE_SIZE, DMA_TO_DEVICE);
free_page((unsigned long)page_table);
}
}
- dma_unmap_single(&rk_domain->pdev->dev, rk_domain->dt_dma,
+ dma_unmap_single(dma_dev, rk_domain->dt_dma,
SPAGE_SIZE, DMA_TO_DEVICE);
free_page((unsigned long)rk_domain->dt);
if (domain->type == IOMMU_DOMAIN_DMA)
iommu_put_dma_cookie(&rk_domain->domain);
-
- platform_device_unregister(rk_domain->pdev);
}
static bool rk_iommu_is_dev_iommu_master(struct device *dev)
@@ -1120,30 +1106,6 @@ static const struct iommu_ops rk_iommu_ops = {
.pgsize_bitmap = RK_IOMMU_PGSIZE_BITMAP,
};
-static int rk_iommu_domain_probe(struct platform_device *pdev)
-{
- struct device *dev = &pdev->dev;
-
- dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms), GFP_KERNEL);
- if (!dev->dma_parms)
- return -ENOMEM;
-
- /* Set dma_ops for dev, otherwise it would be dummy_dma_ops */
- arch_setup_dma_ops(dev, 0, DMA_BIT_MASK(32), NULL, false);
-
- dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
- dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(32));
-
- return 0;
-}
-
-static struct platform_driver rk_iommu_domain_driver = {
- .probe = rk_iommu_domain_probe,
- .driver = {
- .name = "rk_iommu_domain",
- },
-};
-
static int rk_iommu_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1210,6 +1172,14 @@ static int rk_iommu_probe(struct platform_device *pdev)
return err;
}
+ /*
+ * Use the first registered IOMMU device for domain to use with DMA
+ * API, since a domain might not physically correspond to a single
+ * IOMMU device..
+ */
+ if (!dma_dev)
+ dma_dev = &pdev->dev;
+
return 0;
}
@@ -1251,31 +1221,21 @@ static int __init rk_iommu_init(void)
of_node_put(np);
- ret = platform_driver_register(&rk_iommu_domain_driver);
- if (ret)
- return ret;
-
ret = platform_driver_register(&rk_iommu_driver);
if (ret)
- goto err_unreg_domain_drv;
+ return ret;
ret = bus_set_iommu(&platform_bus_type, &rk_iommu_ops);
- if (ret)
- goto err_unreg_iommu_drv;
+ if (ret) {
+ platform_driver_unregister(&rk_iommu_driver);
+ return ret;
+ }
return 0;
-
-err_unreg_iommu_drv:
- platform_driver_unregister(&rk_iommu_driver);
-err_unreg_domain_drv:
- platform_driver_unregister(&rk_iommu_domain_driver);
-
- return ret;
}
static void __exit rk_iommu_exit(void)
{
platform_driver_unregister(&rk_iommu_driver);
- platform_driver_unregister(&rk_iommu_domain_driver);
}
subsys_initcall(rk_iommu_init);
--
2.11.0
Converts the rockchip-iommu driver to use the OF_IOMMU infrastructure,
which allows attaching master devices to their IOMMUs automatically
according to DT properties.
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index ab18a80fdd12..c375a2522bfd 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -18,6 +18,7 @@
#include <linux/mm.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_iommu.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
@@ -1091,6 +1092,13 @@ static struct iommu_group *rk_iommu_device_group(struct device *dev)
return ERR_PTR(ret);
}
+static int rk_iommu_of_xlate(struct device *dev,
+ struct of_phandle_args *spec)
+{
+ /* We don't have any phandle args, so just return 0. */
+ return 0;
+}
+
static const struct iommu_ops rk_iommu_ops = {
.domain_alloc = rk_iommu_domain_alloc,
.domain_free = rk_iommu_domain_free,
@@ -1104,6 +1112,7 @@ static const struct iommu_ops rk_iommu_ops = {
.iova_to_phys = rk_iommu_iova_to_phys,
.device_group = rk_iommu_device_group,
.pgsize_bitmap = RK_IOMMU_PGSIZE_BITMAP,
+ .of_xlate = rk_iommu_of_xlate,
};
static int rk_iommu_probe(struct platform_device *pdev)
@@ -1166,6 +1175,8 @@ static int rk_iommu_probe(struct platform_device *pdev)
return err;
iommu_device_set_ops(&iommu->iommu, &rk_iommu_ops);
+ iommu_device_set_fwnode(&iommu->iommu, &dev->of_node->fwnode);
+
err = iommu_device_register(&iommu->iommu);
if (err) {
iommu_device_sysfs_remove(&iommu->iommu);
@@ -1241,6 +1252,8 @@ static void __exit rk_iommu_exit(void)
subsys_initcall(rk_iommu_init);
module_exit(rk_iommu_exit);
+IOMMU_OF_DECLARE(rk_iommu_of, "rockchip,iommu");
+
MODULE_DESCRIPTION("IOMMU API for Rockchip");
MODULE_AUTHOR("Simon Xue <[email protected]> and Daniel Kurtz <[email protected]>");
MODULE_ALIAS("platform:rockchip-iommu");
--
2.11.0
From: Tomasz Figa <[email protected]>
IOMMU drivers are supposed to call this function instead of manually
creating a group in their .add_device callback. This behavior is not
strictly required by ARM DMA mapping implementation, but ARM64 already
relies on it. This patch fixes the rockchip-iommu driver to comply with
this requirement.
Signed-off-by: Tomasz Figa <[email protected]>
Signed-off-by: Jeffy Chen <[email protected]>
---
drivers/iommu/rockchip-iommu.c | 122 +++++++++++++++++++++--------------------
1 file changed, 64 insertions(+), 58 deletions(-)
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index cfeafbf54096..86f8190d7bed 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -802,6 +802,40 @@ static struct rk_iommu *rk_iommu_from_dev(struct device *dev)
return rk_iommu;
}
+static void rk_iommu_detach_device(struct iommu_domain *domain,
+ struct device *dev)
+{
+ struct rk_iommu *iommu;
+ struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
+ unsigned long flags;
+ int i;
+
+ /* Allow 'virtual devices' (eg drm) to detach from domain */
+ iommu = rk_iommu_from_dev(dev);
+ if (!iommu)
+ return;
+
+ spin_lock_irqsave(&rk_domain->iommus_lock, flags);
+ list_del_init(&iommu->node);
+ spin_unlock_irqrestore(&rk_domain->iommus_lock, flags);
+
+ /* Ignore error while disabling, just keep going */
+ rk_iommu_enable_stall(iommu);
+ rk_iommu_disable_paging(iommu);
+ for (i = 0; i < iommu->num_mmu; i++) {
+ rk_iommu_write(iommu->bases[i], RK_MMU_INT_MASK, 0);
+ rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR, 0);
+ }
+ rk_iommu_disable_stall(iommu);
+
+ for (i = 0; i < iommu->num_irq; i++)
+ devm_free_irq(iommu->dev, iommu->irq[i], iommu);
+
+ iommu->domain = NULL;
+
+ dev_dbg(dev, "Detached from iommu domain\n");
+}
+
static int rk_iommu_attach_device(struct iommu_domain *domain,
struct device *dev)
{
@@ -818,6 +852,9 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
if (!iommu)
return 0;
+ if (iommu->domain)
+ rk_iommu_detach_device(domain, dev);
+
ret = rk_iommu_enable_stall(iommu);
if (ret)
return ret;
@@ -865,40 +902,6 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
return ret;
}
-static void rk_iommu_detach_device(struct iommu_domain *domain,
- struct device *dev)
-{
- struct rk_iommu *iommu;
- struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
- unsigned long flags;
- int i;
-
- /* Allow 'virtual devices' (eg drm) to detach from domain */
- iommu = rk_iommu_from_dev(dev);
- if (!iommu)
- return;
-
- spin_lock_irqsave(&rk_domain->iommus_lock, flags);
- list_del_init(&iommu->node);
- spin_unlock_irqrestore(&rk_domain->iommus_lock, flags);
-
- /* Ignore error while disabling, just keep going */
- rk_iommu_enable_stall(iommu);
- rk_iommu_disable_paging(iommu);
- for (i = 0; i < iommu->num_mmu; i++) {
- rk_iommu_write(iommu->bases[i], RK_MMU_INT_MASK, 0);
- rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR, 0);
- }
- rk_iommu_disable_stall(iommu);
-
- for (i = 0; i < iommu->num_irq; i++)
- devm_free_irq(iommu->dev, iommu->irq[i], iommu);
-
- iommu->domain = NULL;
-
- dev_dbg(dev, "Detached from iommu domain\n");
-}
-
static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
{
struct rk_iommu_domain *rk_domain;
@@ -1049,41 +1052,20 @@ static int rk_iommu_add_device(struct device *dev)
{
struct iommu_group *group;
struct rk_iommu *iommu;
- int ret;
if (!rk_iommu_is_dev_iommu_master(dev))
return -ENODEV;
- group = iommu_group_get(dev);
- if (!group) {
- group = iommu_group_alloc();
- if (IS_ERR(group)) {
- dev_err(dev, "Failed to allocate IOMMU group\n");
- return PTR_ERR(group);
- }
- }
-
- ret = iommu_group_add_device(group, dev);
- if (ret)
- goto err_put_group;
-
- ret = rk_iommu_group_set_iommudata(group, dev);
- if (ret)
- goto err_remove_device;
+ group = iommu_group_get_for_dev(dev);
+ if (IS_ERR(group))
+ return PTR_ERR(group);
iommu = rk_iommu_from_dev(dev);
if (iommu)
iommu_device_link(&iommu->iommu, dev);
iommu_group_put(group);
-
return 0;
-
-err_remove_device:
- iommu_group_remove_device(dev);
-err_put_group:
- iommu_group_put(group);
- return ret;
}
static void rk_iommu_remove_device(struct device *dev)
@@ -1100,6 +1082,29 @@ static void rk_iommu_remove_device(struct device *dev)
iommu_group_remove_device(dev);
}
+static struct iommu_group *rk_iommu_device_group(struct device *dev)
+{
+ struct iommu_group *group;
+ int ret;
+
+ group = iommu_group_get(dev);
+ if (!group) {
+ group = iommu_group_alloc();
+ if (IS_ERR(group))
+ return group;
+ }
+
+ ret = rk_iommu_group_set_iommudata(group, dev);
+ if (ret)
+ goto err_put_group;
+
+ return group;
+
+err_put_group:
+ iommu_group_put(group);
+ return ERR_PTR(ret);
+}
+
static const struct iommu_ops rk_iommu_ops = {
.domain_alloc = rk_iommu_domain_alloc,
.domain_free = rk_iommu_domain_free,
@@ -1111,6 +1116,7 @@ static const struct iommu_ops rk_iommu_ops = {
.add_device = rk_iommu_add_device,
.remove_device = rk_iommu_remove_device,
.iova_to_phys = rk_iommu_iova_to_phys,
+ .device_group = rk_iommu_device_group,
.pgsize_bitmap = RK_IOMMU_PGSIZE_BITMAP,
};
--
2.11.0
Hi Jeffy,
On 2018-01-11 09:22, Jeffy Chen wrote:
> With the probe-deferral mechanism, early initialisation hooks are no
> longer needed.
>
> Suggested-by: Robin Murphy <[email protected]>
> Signed-off-by: Jeffy Chen <[email protected]>
> ---
>
> drivers/iommu/arm-smmu-v3.c | 2 +-
> drivers/iommu/arm-smmu.c | 12 ++++++------
> drivers/iommu/exynos-iommu.c | 2 +-
For Exynos IOMMU:
Acked-by: Marek Szyprowski <[email protected]>
IPMMU and MSM IOMMU are no longer multi-platform safe after this patch.
It breaks them in the same way as my commit 928055a01b3f ("iommu/exynos:
Remove custom platform device registration code") broke Exynos IOMMU.
You need a similar fix for them:
https://www.spinics.net/lists/arm-kernel/msg627648.html
> drivers/iommu/ipmmu-vmsa.c | 17 ++---------------
> drivers/iommu/msm_iommu.c | 24 +++++++-----------------
> drivers/iommu/of_iommu.c | 16 ----------------
> drivers/iommu/qcom_iommu.c | 2 +-
> include/linux/of_iommu.h | 6 ++----
> 8 files changed, 20 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 744592d330ca..3f2f1fc68b52 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2971,7 +2971,7 @@ static struct platform_driver arm_smmu_driver = {
> };
> module_platform_driver(arm_smmu_driver);
>
> -IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", NULL);
> +IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
>
> MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
> MODULE_AUTHOR("Will Deacon <[email protected]>");
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 78d4c6b8f1ba..69e7c60792a8 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2211,12 +2211,12 @@ static struct platform_driver arm_smmu_driver = {
> };
> module_platform_driver(arm_smmu_driver);
>
> -IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1", NULL);
> -IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2", NULL);
> -IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400", NULL);
> -IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", NULL);
> -IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", NULL);
> -IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", NULL);
> +IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
> +IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
> +IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
> +IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
> +IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
> +IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
>
> MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
> MODULE_AUTHOR("Will Deacon <[email protected]>");
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 79c45650f8de..1c7f926fad0e 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1394,4 +1394,4 @@ static int __init exynos_iommu_init(void)
> }
> core_initcall(exynos_iommu_init);
>
> -IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu", NULL);
> +IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 8dce3a9de9d8..07b711bb4b16 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -1081,12 +1081,8 @@ static struct platform_driver ipmmu_driver = {
>
> static int __init ipmmu_init(void)
> {
> - static bool setup_done;
> int ret;
>
> - if (setup_done)
> - return 0;
> -
> ret = platform_driver_register(&ipmmu_driver);
> if (ret < 0)
> return ret;
> @@ -1096,7 +1092,6 @@ static int __init ipmmu_init(void)
> bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> #endif
>
> - setup_done = true;
> return 0;
> }
>
> @@ -1109,16 +1104,8 @@ subsys_initcall(ipmmu_init);
> module_exit(ipmmu_exit);
>
> #ifdef CONFIG_IOMMU_DMA
> -static int __init ipmmu_vmsa_iommu_of_setup(struct device_node *np)
> -{
> - ipmmu_init();
> - return 0;
> -}
> -
> -IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa",
> - ipmmu_vmsa_iommu_of_setup);
> -IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795",
> - ipmmu_vmsa_iommu_of_setup);
> +IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
> +IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
> #endif
>
> MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 04f4d51ffacb..a41d4251b0a9 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -856,7 +856,7 @@ static struct platform_driver msm_iommu_driver = {
> .remove = msm_iommu_remove,
> };
>
> -static int __init msm_iommu_driver_init(void)
> +static int __init msm_iommu_init(void)
> {
> int ret;
>
> @@ -864,30 +864,20 @@ static int __init msm_iommu_driver_init(void)
> if (ret != 0)
> pr_err("Failed to register IOMMU driver\n");
>
> + bus_set_iommu(&platform_bus_type, &msm_iommu_ops);
> +
> return ret;
> }
>
> -static void __exit msm_iommu_driver_exit(void)
> +static void __exit msm_iommu_exit(void)
> {
> platform_driver_unregister(&msm_iommu_driver);
> }
>
> -subsys_initcall(msm_iommu_driver_init);
> -module_exit(msm_iommu_driver_exit);
> -
> -static int __init msm_iommu_init(void)
> -{
> - bus_set_iommu(&platform_bus_type, &msm_iommu_ops);
> - return 0;
> -}
> -
> -static int __init msm_iommu_of_setup(struct device_node *np)
> -{
> - msm_iommu_init();
> - return 0;
> -}
> +subsys_initcall(msm_iommu_init);
> +module_exit(msm_iommu_exit);
>
> -IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu", msm_iommu_of_setup);
> +IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
>
> MODULE_LICENSE("GPL v2");
> MODULE_AUTHOR("Stepan Moskovchenko <[email protected]>");
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 50947ebb6d17..5c36a8b7656a 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -231,19 +231,3 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
>
> return ops;
> }
> -
> -static int __init of_iommu_init(void)
> -{
> - struct device_node *np;
> - const struct of_device_id *match, *matches = &__iommu_of_table;
> -
> - for_each_matching_node_and_match(np, matches, &match) {
> - const of_iommu_init_fn init_fn = match->data;
> -
> - if (init_fn && init_fn(np))
> - pr_err("Failed to initialise IOMMU %pOF\n", np);
> - }
> -
> - return 0;
> -}
> -postcore_initcall_sync(of_iommu_init);
> diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
> index e07f02d00c68..65b9c99707f8 100644
> --- a/drivers/iommu/qcom_iommu.c
> +++ b/drivers/iommu/qcom_iommu.c
> @@ -947,7 +947,7 @@ static void __exit qcom_iommu_exit(void)
> module_init(qcom_iommu_init);
> module_exit(qcom_iommu_exit);
>
> -IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1", NULL);
> +IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1");
>
> MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations");
> MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
> index cddfaff4d0b7..ae03752e8e3a 100644
> --- a/include/linux/of_iommu.h
> +++ b/include/linux/of_iommu.h
> @@ -34,9 +34,7 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
>
> extern struct of_device_id __iommu_of_table;
>
> -typedef int (*of_iommu_init_fn)(struct device_node *);
> -
> -#define IOMMU_OF_DECLARE(name, compat, fn) \
> - _OF_DECLARE(iommu, name, compat, fn, of_iommu_init_fn)
> +#define IOMMU_OF_DECLARE(name, compat) \
> + OF_DECLARE_1(iommu, name, compat, NULL)
>
> #endif /* __OF_IOMMU_H */
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Marek,
Thanks for your reply.
On 01/11/2018 05:40 PM, Marek Szyprowski wrote:
> Hi Jeffy,
>
> On 2018-01-11 09:22, Jeffy Chen wrote:
>> With the probe-deferral mechanism, early initialisation hooks are no
>> longer needed.
>>
>> Suggested-by: Robin Murphy <[email protected]>
>> Signed-off-by: Jeffy Chen <[email protected]>
>> ---
>>
>> drivers/iommu/arm-smmu-v3.c | 2 +-
>> drivers/iommu/arm-smmu.c | 12 ++++++------
>> drivers/iommu/exynos-iommu.c | 2 +-
>
> For Exynos IOMMU:
> Acked-by: Marek Szyprowski <[email protected]>
>
> IPMMU and MSM IOMMU are no longer multi-platform safe after this patch.
> It breaks them in the same way as my commit 928055a01b3f ("iommu/exynos:
> Remove custom platform device registration code") broke Exynos IOMMU.
>
> You need a similar fix for them:
> https://www.spinics.net/lists/arm-kernel/msg627648.html
hmmm, right, i did saw this fix in the rockchip iommu driver too.
and there're also some other iommu drivers put bus_set_iommu in their
probe() to avoid that.
maybe we can do it in the iommu framework?
for example:
1/ add a bus type member to struct iommu_device
2/ and a iommu_device_set_bus()
3/ do the bus_set_iommu stuff in iommu_device_register()
4/ undo bus_set_iommu in iommu_device_unregister()
Hi Jeffy,
On 11/01/18 11:14, JeffyChen wrote:
> Hi Marek,
>
> Thanks for your reply.
>
> On 01/11/2018 05:40 PM, Marek Szyprowski wrote:
>> Hi Jeffy,
>>
>> On 2018-01-11 09:22, Jeffy Chen wrote:
>>> With the probe-deferral mechanism, early initialisation hooks are no
>>> longer needed.
>>>
>>> Suggested-by: Robin Murphy <[email protected]>
In fact, shortly after I said that I had a "how hard can it be?" moment
and took a crack at it myself - sorry, I should probably have cc'd you
on that series[1].
>>> Signed-off-by: Jeffy Chen <[email protected]>
>>> ---
>>>
>>> drivers/iommu/arm-smmu-v3.c | 2 +-
>>> drivers/iommu/arm-smmu.c | 12 ++++++------
>>> drivers/iommu/exynos-iommu.c | 2 +-
>>
>> For Exynos IOMMU:
>> Acked-by: Marek Szyprowski <[email protected]>
>>
>> IPMMU and MSM IOMMU are no longer multi-platform safe after this patch.
>> It breaks them in the same way as my commit 928055a01b3f ("iommu/exynos:
>> Remove custom platform device registration code") broke Exynos IOMMU.
>>
>> You need a similar fix for them:
>> https://www.spinics.net/lists/arm-kernel/msg627648.html
>
> hmmm, right, i did saw this fix in the rockchip iommu driver too.
>
> and there're also some other iommu drivers put bus_set_iommu in their
> probe() to avoid that.
>
> maybe we can do it in the iommu framework?
>
> for example:
> 1/ add a bus type member to struct iommu_device
> 2/ and a iommu_device_set_bus()
> 3/ do the bus_set_iommu stuff in iommu_device_register()
> 4/ undo bus_set_iommu in iommu_device_unregister()
Ultimately we'd like to get rid of the bus relationship altogether, so I
don't think it's really worth adding more infrastructure around it.
Having of-iommu-based drivers set bus ops at probe time, and others
conditionally from an initcall, is pretty clean and simple, so I'd
rather stick with that approach for now.
Robin.
[1]
https://lists.linuxfoundation.org/pipermail/iommu/2018-January/025395.html
Hi Jeffy,
On 2018-01-11 12:14, JeffyChen wrote:
> Hi Marek,
>
> Thanks for your reply.
>
> On 01/11/2018 05:40 PM, Marek Szyprowski wrote:
>> Hi Jeffy,
>>
>> On 2018-01-11 09:22, Jeffy Chen wrote:
>>> With the probe-deferral mechanism, early initialisation hooks are no
>>> longer needed.
>>>
>>> Suggested-by: Robin Murphy <[email protected]>
>>> Signed-off-by: Jeffy Chen <[email protected]>
>>> ---
>>>
>>> drivers/iommu/arm-smmu-v3.c | 2 +-
>>> drivers/iommu/arm-smmu.c | 12 ++++++------
>>> drivers/iommu/exynos-iommu.c | 2 +-
>>
>> For Exynos IOMMU:
>> Acked-by: Marek Szyprowski <[email protected]>
>>
>> IPMMU and MSM IOMMU are no longer multi-platform safe after this patch.
>> It breaks them in the same way as my commit 928055a01b3f ("iommu/exynos:
>> Remove custom platform device registration code") broke Exynos IOMMU.
>>
>> You need a similar fix for them:
>> https://www.spinics.net/lists/arm-kernel/msg627648.html
>
> hmmm, right, i did saw this fix in the rockchip iommu driver too.
>
> and there're also some other iommu drivers put bus_set_iommu in their
> probe() to avoid that.
>
> maybe we can do it in the iommu framework?
>
> for example:
> 1/ add a bus type member to struct iommu_device
> 2/ and a iommu_device_set_bus()
> 3/ do the bus_set_iommu stuff in iommu_device_register()
> 4/ undo bus_set_iommu in iommu_device_unregister()
Frankly, in case the device-tree based systems bus_set_iommu()
should not be needed at all. However for some legacy reasons it
is still required by a few bits of code (at least it was needed
some time ago). Probably it would be best if this is finally
resolved.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On 11/01/18 08:22, Jeffy Chen wrote:
> From: Tomasz Figa <[email protected]>
>
> Currently if the driver encounters an error while attaching device, it
> will leave the IOMMU in an inconsistent state. Even though it shouldn't
> really happen in reality, let's just add proper error path to keep
> things consistent.
>
> Signed-off-by: Tomasz Figa <[email protected]>
> Signed-off-by: Jeffy Chen <[email protected]>
> ---
>
> drivers/iommu/rockchip-iommu.c | 26 +++++++++++++++++---------
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 9d991c2d8767..ee805e1dfba7 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -826,17 +826,10 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
>
> ret = rk_iommu_force_reset(iommu);
> if (ret)
> - return ret;
> + goto err_disable_stall;
>
> iommu->domain = domain;
>
> - for (i = 0; i < iommu->num_irq; i++) {
> - ret = devm_request_irq(iommu->dev, iommu->irq[i], rk_iommu_irq,
> - IRQF_SHARED, dev_name(dev), iommu);
> - if (ret)
> - return ret;
> - }
> -
> for (i = 0; i < iommu->num_mmu; i++) {
> rk_iommu_write(iommu->bases[i], RK_MMU_DTE_ADDR,
> rk_domain->dt_dma);
> @@ -844,9 +837,16 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
> rk_iommu_write(iommu->bases[i], RK_MMU_INT_MASK, RK_MMU_IRQ_MASK);
> }
>
> + for (i = 0; i < iommu->num_irq; i++) {
> + ret = devm_request_irq(iommu->dev, iommu->irq[i], rk_iommu_irq,
> + IRQF_SHARED, dev_name(dev), iommu);
Why aren't we simply requesting the IRQ once in rk_iommu_probe()? Given
that the hardware doesn't handle multiple translation contexts, there
doesn't seem to be much point in being this dynamic about it.
Robin.
> + if (ret)
> + goto err_free_irq;
> + }
> +
> ret = rk_iommu_enable_paging(iommu);
> if (ret)
> - return ret;
> + goto err_free_irq;
>
> spin_lock_irqsave(&rk_domain->iommus_lock, flags);
> list_add_tail(&iommu->node, &rk_domain->iommus);
> @@ -857,6 +857,14 @@ static int rk_iommu_attach_device(struct iommu_domain *domain,
> rk_iommu_disable_stall(iommu);
>
> return 0;
> +
> +err_free_irq:
> + while (i--)
> + devm_free_irq(iommu->dev, iommu->irq[i], iommu);
> +err_disable_stall:
> + rk_iommu_disable_stall(iommu);
> +
> + return ret;
> }
>
> static void rk_iommu_detach_device(struct iommu_domain *domain,
>
Hi Robin,
thanks for your reply.
On 01/11/2018 11:47 PM, Robin Murphy wrote:
>>
>> + for (i = 0; i < iommu->num_irq; i++) {
>> + ret = devm_request_irq(iommu->dev, iommu->irq[i], rk_iommu_irq,
>> + IRQF_SHARED, dev_name(dev), iommu);
>
> Why aren't we simply requesting the IRQ once in rk_iommu_probe()? Given
> that the hardware doesn't handle multiple translation contexts, there
> doesn't seem to be much point in being this dynamic about it.
>
it make sense, will do it in next version:)
> Robin.
Hi Robin,
Thnaks for your reply.
On 01/11/2018 08:24 PM, Robin Murphy wrote:
> Hi Jeffy,
>
> On 11/01/18 11:14, JeffyChen wrote:
>> Hi Marek,
>>
>> Thanks for your reply.
>>
>> On 01/11/2018 05:40 PM, Marek Szyprowski wrote:
>>> Hi Jeffy,
>>>
>>> On 2018-01-11 09:22, Jeffy Chen wrote:
>>>> With the probe-deferral mechanism, early initialisation hooks are no
>>>> longer needed.
>>>>
>>>> Suggested-by: Robin Murphy <[email protected]>
>
> In fact, shortly after I said that I had a "how hard can it be?" moment
> and took a crack at it myself - sorry, I should probably have cc'd you
> on that series[1].
hmmm, i'll drop this patch in the next version.
and maybe rebase my patch[9] (iommu/rockchip: Use OF_IOMMU to attach
devices automatically) on that series
>
>>>> Signed-off-by: Jeffy Chen <[email protected]>
>>>> ---
>>>>
>>>> drivers/iommu/arm-smmu-v3.c | 2 +-
>>>> drivers/iommu/arm-smmu.c | 12 ++++++------
>>>> drivers/iommu/exynos-iommu.c | 2 +-
>>>
>>> For Exynos IOMMU:
>>> Acked-by: Marek Szyprowski <[email protected]>
>>>
>>> IPMMU and MSM IOMMU are no longer multi-platform safe after this patch.
>>> It breaks them in the same way as my commit 928055a01b3f ("iommu/exynos:
>>> Remove custom platform device registration code") broke Exynos IOMMU.
>>>
>>> You need a similar fix for them:
>>> https://www.spinics.net/lists/arm-kernel/msg627648.html
>>
>> hmmm, right, i did saw this fix in the rockchip iommu driver too.
>>
>> and there're also some other iommu drivers put bus_set_iommu in their
>> probe() to avoid that.
>>
>> maybe we can do it in the iommu framework?
>>
>> for example:
>> 1/ add a bus type member to struct iommu_device
>> 2/ and a iommu_device_set_bus()
>> 3/ do the bus_set_iommu stuff in iommu_device_register()
>> 4/ undo bus_set_iommu in iommu_device_unregister()
>
> Ultimately we'd like to get rid of the bus relationship altogether, so I
> don't think it's really worth adding more infrastructure around it.
> Having of-iommu-based drivers set bus ops at probe time, and others
> conditionally from an initcall, is pretty clean and simple, so I'd
> rather stick with that approach for now.
ok, make sense:)
>
> Robin.
>
> [1]
> https://lists.linuxfoundation.org/pipermail/iommu/2018-January/025395.html
>
>
>