Discussion:
[PATCH v7 1/7] iommu/exynos: Remove excessive, useless debug
Marek Szyprowski
2016-11-14 10:08:06 UTC
Permalink
Remove excessive, useless debug about skipping TLB invalidation, which
is a normal situation when more aggressive power management is enabled.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/iommu/exynos-iommu.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 30808e9..8ba0d60 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -578,9 +578,6 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
sysmmu_unblock(data);
}
clk_disable(data->clk_master);
- } else {
- dev_dbg(data->master,
- "disabled. Skipping TLB invalidation @ %#x\n", iova);
}
spin_unlock_irqrestore(&data->lock, flags);
}
--
1.9.1
Marek Szyprowski
2016-11-14 10:08:08 UTC
Permalink
Remove remaining leftovers of the ref-count related code in the
__sysmmu_enable/disable functions inline __sysmmu_enable/disable_nocount
to them. Suspend/resume callbacks now checks if master device is set for
given SYSMMU controller instead of relying on the activation count.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/iommu/exynos-iommu.c | 104 ++++++++++++-------------------------------
1 file changed, 29 insertions(+), 75 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 4056228..f45b274 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -237,8 +237,8 @@ struct sysmmu_drvdata {
struct clk *aclk; /* SYSMMU's aclk clock */
struct clk *pclk; /* SYSMMU's pclk clock */
struct clk *clk_master; /* master's device clock */
- int activations; /* number of calls to sysmmu_enable */
spinlock_t lock; /* lock for modyfying state */
+ bool active; /* current status */
struct exynos_iommu_domain *domain; /* domain we belong to */
struct list_head domain_node; /* node for domain clients list */
struct list_head owner_node; /* node for owner controllers list */
@@ -251,25 +251,6 @@ static struct exynos_iommu_domain *to_exynos_domain(struct iommu_domain *dom)
return container_of(dom, struct exynos_iommu_domain, domain);
}

-static bool set_sysmmu_active(struct sysmmu_drvdata *data)
-{
- /* return true if the System MMU was not active previously
- and it needs to be initialized */
- return ++data->activations == 1;
-}
-
-static bool set_sysmmu_inactive(struct sysmmu_drvdata *data)
-{
- /* return true if the System MMU is needed to be disabled */
- BUG_ON(data->activations < 1);
- return --data->activations == 0;
-}
-
-static bool is_sysmmu_active(struct sysmmu_drvdata *data)
-{
- return data->activations > 0;
-}
-
static void sysmmu_unblock(struct sysmmu_drvdata *data)
{
writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL);
@@ -388,7 +369,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
unsigned short reg_status, reg_clear;
int ret = -ENOSYS;

- WARN_ON(!is_sysmmu_active(data));
+ WARN_ON(!data->active);

if (MMU_MAJ_VER(data->version) < 5) {
reg_status = REG_INT_STATUS;
@@ -434,37 +415,19 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}

-static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data)
+static void __sysmmu_disable(struct sysmmu_drvdata *data)
{
+ unsigned long flags;
+
clk_enable(data->clk_master);

+ spin_lock_irqsave(&data->lock, flags);
writel(CTRL_DISABLE, data->sfrbase + REG_MMU_CTRL);
writel(0, data->sfrbase + REG_MMU_CFG);
-
- __sysmmu_disable_clocks(data);
-}
-
-static bool __sysmmu_disable(struct sysmmu_drvdata *data)
-{
- bool disabled;
- unsigned long flags;
-
- spin_lock_irqsave(&data->lock, flags);
-
- disabled = set_sysmmu_inactive(data);
-
- if (disabled) {
- data->pgtable = 0;
- data->domain = NULL;
-
- __sysmmu_disable_nocount(data);
-
- dev_dbg(data->sysmmu, "Disabled\n");
- }
-
+ data->active = false;
spin_unlock_irqrestore(&data->lock, flags);

- return disabled;
+ __sysmmu_disable_clocks(data);
}

static void __sysmmu_init_config(struct sysmmu_drvdata *data)
@@ -481,17 +444,19 @@ static void __sysmmu_init_config(struct sysmmu_drvdata *data)
writel(cfg, data->sfrbase + REG_MMU_CFG);
}

-static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data)
+static void __sysmmu_enable(struct sysmmu_drvdata *data)
{
+ unsigned long flags;
+
__sysmmu_enable_clocks(data);

+ spin_lock_irqsave(&data->lock, flags);
writel(CTRL_BLOCK, data->sfrbase + REG_MMU_CTRL);
-
__sysmmu_init_config(data);
-
__sysmmu_set_ptbase(data, data->pgtable);
-
writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL);
+ data->active = true;
+ spin_unlock_irqrestore(&data->lock, flags);

/*
* SYSMMU driver keeps master's clock enabled only for the short
@@ -502,37 +467,18 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data)
clk_disable(data->clk_master);
}

-static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable,
- struct exynos_iommu_domain *domain)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&data->lock, flags);
- if (set_sysmmu_active(data)) {
- data->pgtable = pgtable;
- data->domain = domain;
- __sysmmu_enable_nocount(data);
- dev_dbg(data->sysmmu, "Enabled\n");
- }
- spin_unlock_irqrestore(&data->lock, flags);
-
- return 0;
-}
-
static void sysmmu_tlb_invalidate_flpdcache(struct sysmmu_drvdata *data,
sysmmu_iova_t iova)
{
unsigned long flags;

-
spin_lock_irqsave(&data->lock, flags);
- if (is_sysmmu_active(data) && data->version >= MAKE_MMU_VER(3, 3)) {
+ if (data->active && data->version >= MAKE_MMU_VER(3, 3)) {
clk_enable(data->clk_master);
__sysmmu_tlb_invalidate_entry(data, iova, 1);
clk_disable(data->clk_master);
}
spin_unlock_irqrestore(&data->lock, flags);
-
}

static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
@@ -541,7 +487,7 @@ static void sysmmu_tlb_invalidate_entry(struct sysmmu_drvdata *data,
unsigned long flags;

spin_lock_irqsave(&data->lock, flags);
- if (is_sysmmu_active(data)) {
+ if (data->active) {
unsigned int num_inv = 1;

clk_enable(data->clk_master);
@@ -652,10 +598,11 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev)
static int exynos_sysmmu_suspend(struct device *dev)
{
struct sysmmu_drvdata *data = dev_get_drvdata(dev);
+ struct device *master = data->master;

dev_dbg(dev, "suspend\n");
- if (is_sysmmu_active(data)) {
- __sysmmu_disable_nocount(data);
+ if (master) {
+ __sysmmu_disable(data);
pm_runtime_put(dev);
}
return 0;
@@ -664,11 +611,12 @@ static int exynos_sysmmu_suspend(struct device *dev)
static int exynos_sysmmu_resume(struct device *dev)
{
struct sysmmu_drvdata *data = dev_get_drvdata(dev);
+ struct device *master = data->master;

dev_dbg(dev, "resume\n");
- if (is_sysmmu_active(data)) {
+ if (master) {
pm_runtime_get_sync(dev);
- __sysmmu_enable_nocount(data);
+ __sysmmu_enable(data);
}
return 0;
}
@@ -780,6 +728,8 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain)

list_for_each_entry_safe(data, next, &domain->clients, domain_node) {
__sysmmu_disable(data);
+ data->pgtable = 0;
+ data->domain = NULL;
data->master = NULL;
list_del_init(&data->domain_node);
}
@@ -823,6 +773,8 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
list_for_each_entry_safe(data, next, &domain->clients, domain_node) {
__sysmmu_disable(data);
data->master = NULL;
+ data->pgtable = 0;
+ data->domain = NULL;
list_del_init(&data->domain_node);
pm_runtime_put(data->sysmmu);
}
@@ -850,8 +802,10 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
exynos_iommu_detach_device(owner->domain, dev);

list_for_each_entry(data, &owner->controllers, owner_node) {
+ data->pgtable = pagetable;
+ data->domain = domain;
pm_runtime_get_sync(data->sysmmu);
- __sysmmu_enable(data, pagetable, domain);
+ __sysmmu_enable(data);
data->master = dev;

spin_lock_irqsave(&domain->lock, flags);
--
1.9.1
Marek Szyprowski
2016-11-14 10:08:09 UTC
Permalink
To avoid possible races, set master device pointer in each SYSMMU
controller once on boot. Suspend/resume callbacks now properly relies on
the configured iommu domain to enable or disable SYSMMU controller.
While changing the code, also update the sleep debug messages and make
them conditional.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/iommu/exynos-iommu.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index f45b274..28e570b 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -600,10 +600,12 @@ static int exynos_sysmmu_suspend(struct device *dev)
struct sysmmu_drvdata *data = dev_get_drvdata(dev);
struct device *master = data->master;

- dev_dbg(dev, "suspend\n");
if (master) {
- __sysmmu_disable(data);
pm_runtime_put(dev);
+ if (data->domain) {
+ dev_dbg(data->sysmmu, "saving state\n");
+ __sysmmu_disable(data);
+ }
}
return 0;
}
@@ -613,10 +615,12 @@ static int exynos_sysmmu_resume(struct device *dev)
struct sysmmu_drvdata *data = dev_get_drvdata(dev);
struct device *master = data->master;

- dev_dbg(dev, "resume\n");
if (master) {
pm_runtime_get_sync(dev);
- __sysmmu_enable(data);
+ if (data->domain) {
+ dev_dbg(data->sysmmu, "restoring state\n");
+ __sysmmu_enable(data);
+ }
}
return 0;
}
@@ -730,7 +734,6 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain)
__sysmmu_disable(data);
data->pgtable = 0;
data->domain = NULL;
- data->master = NULL;
list_del_init(&data->domain_node);
}

@@ -772,7 +775,6 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
spin_lock_irqsave(&domain->lock, flags);
list_for_each_entry_safe(data, next, &domain->clients, domain_node) {
__sysmmu_disable(data);
- data->master = NULL;
data->pgtable = 0;
data->domain = NULL;
list_del_init(&data->domain_node);
@@ -806,7 +808,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
data->domain = domain;
pm_runtime_get_sync(data->sysmmu);
__sysmmu_enable(data);
- data->master = dev;

spin_lock_irqsave(&domain->lock, flags);
list_add_tail(&data->domain_node, &domain->clients);
@@ -1192,6 +1193,7 @@ static int exynos_iommu_of_xlate(struct device *dev,
}

list_add_tail(&data->owner_node, &owner->controllers);
+ data->master = dev;
return 0;
}
--
1.9.1
Marek Szyprowski
2016-11-14 10:08:07 UTC
Permalink
__sysmmu_enable/disable functions were designed to do ref-count based
operations, but current code always calls them only once, so the code for
checking the conditions and invalid conditions can be simply removed
without any influence to the driver operation.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/iommu/exynos-iommu.c | 65 ++++++++++++--------------------------------
1 file changed, 17 insertions(+), 48 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 8ba0d60..4056228 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -460,9 +460,6 @@ static bool __sysmmu_disable(struct sysmmu_drvdata *data)
__sysmmu_disable_nocount(data);

dev_dbg(data->sysmmu, "Disabled\n");
- } else {
- dev_dbg(data->sysmmu, "%d times left to disable\n",
- data->activations);
}

spin_unlock_irqrestore(&data->lock, flags);
@@ -508,29 +505,18 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data)
static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable,
struct exynos_iommu_domain *domain)
{
- int ret = 0;
unsigned long flags;

spin_lock_irqsave(&data->lock, flags);
if (set_sysmmu_active(data)) {
data->pgtable = pgtable;
data->domain = domain;
-
__sysmmu_enable_nocount(data);
-
dev_dbg(data->sysmmu, "Enabled\n");
- } else {
- ret = (pgtable == data->pgtable) ? 1 : -EBUSY;
-
- dev_dbg(data->sysmmu, "already enabled\n");
}
-
- if (WARN_ON(ret < 0))
- set_sysmmu_inactive(data); /* decrement count */
-
spin_unlock_irqrestore(&data->lock, flags);

- return ret;
+ return 0;
}

static void sysmmu_tlb_invalidate_flpdcache(struct sysmmu_drvdata *data,
@@ -793,8 +779,8 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain)
spin_lock_irqsave(&domain->lock, flags);

list_for_each_entry_safe(data, next, &domain->clients, domain_node) {
- if (__sysmmu_disable(data))
- data->master = NULL;
+ __sysmmu_disable(data);
+ data->master = NULL;
list_del_init(&data->domain_node);
}

@@ -829,31 +815,23 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
phys_addr_t pagetable = virt_to_phys(domain->pgtable);
struct sysmmu_drvdata *data, *next;
unsigned long flags;
- bool found = false;

if (!has_sysmmu(dev) || owner->domain != iommu_domain)
return;

spin_lock_irqsave(&domain->lock, flags);
list_for_each_entry_safe(data, next, &domain->clients, domain_node) {
- if (data->master == dev) {
- if (__sysmmu_disable(data)) {
- data->master = NULL;
- list_del_init(&data->domain_node);
- }
- pm_runtime_put(data->sysmmu);
- found = true;
- }
+ __sysmmu_disable(data);
+ data->master = NULL;
+ list_del_init(&data->domain_node);
+ pm_runtime_put(data->sysmmu);
}
spin_unlock_irqrestore(&domain->lock, flags);

owner->domain = NULL;

- if (found)
- dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n",
- __func__, &pagetable);
- else
- dev_err(dev, "%s: No IOMMU is attached\n", __func__);
+ dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", __func__,
+ &pagetable);
}

static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
@@ -864,7 +842,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
struct sysmmu_drvdata *data;
phys_addr_t pagetable = virt_to_phys(domain->pgtable);
unsigned long flags;
- int ret = -ENODEV;

if (!has_sysmmu(dev))
return -ENODEV;
@@ -874,27 +851,19 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,

list_for_each_entry(data, &owner->controllers, owner_node) {
pm_runtime_get_sync(data->sysmmu);
- ret = __sysmmu_enable(data, pagetable, domain);
- if (ret >= 0) {
- data->master = dev;
+ __sysmmu_enable(data, pagetable, domain);
+ data->master = dev;

- spin_lock_irqsave(&domain->lock, flags);
- list_add_tail(&data->domain_node, &domain->clients);
- spin_unlock_irqrestore(&domain->lock, flags);
- }
- }
-
- if (ret < 0) {
- dev_err(dev, "%s: Failed to attach IOMMU with pgtable %pa\n",
- __func__, &pagetable);
- return ret;
+ spin_lock_irqsave(&domain->lock, flags);
+ list_add_tail(&data->domain_node, &domain->clients);
+ spin_unlock_irqrestore(&domain->lock, flags);
}

owner->domain = iommu_domain;
- dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa %s\n",
- __func__, &pagetable, (ret == 0) ? "" : ", again");
+ dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__,
+ &pagetable);

- return ret;
+ return 0;
}

static sysmmu_pte_t *alloc_lv2entry(struct exynos_iommu_domain *domain,
--
1.9.1
Marek Szyprowski
2016-11-14 10:08:10 UTC
Permalink
This patch reworks locking in the exynos_iommu_attach/detach_device
functions to ensure that all entries of the sysmmu_drvdata and
exynos_iommu_owner structure are updated under the respective spinlocks,
while runtime pm functions are called without any spinlocks held.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/iommu/exynos-iommu.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 28e570b..a959443 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -731,10 +731,12 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain)
spin_lock_irqsave(&domain->lock, flags);

list_for_each_entry_safe(data, next, &domain->clients, domain_node) {
+ spin_lock(&data->lock);
__sysmmu_disable(data);
data->pgtable = 0;
data->domain = NULL;
list_del_init(&data->domain_node);
+ spin_unlock(&data->lock);
}

spin_unlock_irqrestore(&domain->lock, flags);
@@ -772,17 +774,22 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
if (!has_sysmmu(dev) || owner->domain != iommu_domain)
return;

+ list_for_each_entry(data, &owner->controllers, owner_node) {
+ __sysmmu_disable(data);
+ pm_runtime_put(data->sysmmu);
+ }
+
spin_lock_irqsave(&domain->lock, flags);
list_for_each_entry_safe(data, next, &domain->clients, domain_node) {
- __sysmmu_disable(data);
+ spin_lock(&data->lock);
data->pgtable = 0;
data->domain = NULL;
list_del_init(&data->domain_node);
- pm_runtime_put(data->sysmmu);
+ spin_unlock(&data->lock);
}
+ owner->domain = NULL;
spin_unlock_irqrestore(&domain->lock, flags);

- owner->domain = NULL;

dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", __func__,
&pagetable);
@@ -803,18 +810,22 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
if (owner->domain)
exynos_iommu_detach_device(owner->domain, dev);

+ spin_lock_irqsave(&domain->lock, flags);
list_for_each_entry(data, &owner->controllers, owner_node) {
+ spin_lock(&data->lock);
data->pgtable = pagetable;
data->domain = domain;
+ list_add_tail(&data->domain_node, &domain->clients);
+ spin_unlock(&data->lock);
+ }
+ owner->domain = iommu_domain;
+ spin_unlock_irqrestore(&domain->lock, flags);
+
+ list_for_each_entry(data, &owner->controllers, owner_node) {
pm_runtime_get_sync(data->sysmmu);
__sysmmu_enable(data);
-
- spin_lock_irqsave(&domain->lock, flags);
- list_add_tail(&data->domain_node, &domain->clients);
- spin_unlock_irqrestore(&domain->lock, flags);
}

- owner->domain = iommu_domain;
dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__,
&pagetable);
--
1.9.1
Marek Szyprowski
2016-11-14 10:08:12 UTC
Permalink
This patch uses recently introduced device dependency links to track the
runtime pm state of the master's device. The goal is to let SYSMMU
controller device's runtime PM to follow the runtime PM state of the
respective master's device. This way each SYSMMU controller is active
only when its master's device is active and can properly restore or save
its state instead on runtime PM transition of master's device.
This approach replaces old behavior, when SYSMMU controller was set to
runtime active once after attaching to the master device. In the new
approach SYSMMU controllers no longer prevents respective power domains
to be turned off when master's device is not being used.

This patch reduces total power consumption of idle system, because most
power domains can be finally turned off. For example, on Exynos 4412
based Odroid U3 this patch reduces power consuption from 136mA to 130mA
at 5V (by 4.4%).

The dependency links also enforce proper order of suspending/restoring
devices during system sleep transition, so there is no more need to use
LATE_SYSTEM_SLEEP_PM_OPS-based workaround for ensuring that SYSMMUs are
suspended after their master devices.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/iommu/exynos-iommu.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 5e6d7bb..4b05a15 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -633,8 +633,8 @@ static int __maybe_unused exynos_sysmmu_resume(struct device *dev)

static const struct dev_pm_ops sysmmu_pm_ops = {
SET_RUNTIME_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume, NULL)
- SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
- pm_runtime_force_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
};

static const struct of_device_id sysmmu_of_match[] __initconst = {
@@ -781,10 +781,6 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
if (!has_sysmmu(dev) || owner->domain != iommu_domain)
return;

- list_for_each_entry(data, &owner->controllers, owner_node) {
- pm_runtime_put_sync(data->sysmmu);
- }
-
mutex_lock(&owner->rpm_lock);

list_for_each_entry(data, &owner->controllers, owner_node) {
@@ -848,10 +844,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,

mutex_unlock(&owner->rpm_lock);

- list_for_each_entry(data, &owner->controllers, owner_node) {
- pm_runtime_get_sync(data->sysmmu);
- }
-
dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__,
&pagetable);

@@ -1232,6 +1224,14 @@ static int exynos_iommu_of_xlate(struct device *dev,

list_add_tail(&data->owner_node, &owner->controllers);
data->master = dev;
+
+ /*
+ * SYSMMU will be runtime activated via device link (dependency) to its
+ * master device, so there are no direct calls to pm_runtime_get/put
+ * in this driver.
+ */
+ device_link_add(dev, data->sysmmu, DL_FLAG_PM_RUNTIME);
+
return 0;
}
--
1.9.1
Marek Szyprowski
2016-11-14 10:08:11 UTC
Permalink
This patch adds runtime pm implementation, which is based on previous
suspend/resume code. SYSMMU controller is now being enabled/disabled mainly
from the runtime pm callbacks. System sleep callbacks relies on generic
pm_runtime_force_suspend/pm_runtime_force_resume helpers. To ensure
internal state consistency, additional lock for runtime pm transitions
was introduced.

Signed-off-by: Marek Szyprowski <***@samsung.com>
---
drivers/iommu/exynos-iommu.c | 45 +++++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index a959443..5e6d7bb 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -206,6 +206,7 @@ struct sysmmu_fault_info {
struct exynos_iommu_owner {
struct list_head controllers; /* list of sysmmu_drvdata.owner_node */
struct iommu_domain *domain; /* domain this device is attached */
+ struct mutex rpm_lock; /* for runtime pm of all sysmmus */
};

/*
@@ -594,40 +595,46 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev)
return 0;
}

-#ifdef CONFIG_PM_SLEEP
-static int exynos_sysmmu_suspend(struct device *dev)
+static int __maybe_unused exynos_sysmmu_suspend(struct device *dev)
{
struct sysmmu_drvdata *data = dev_get_drvdata(dev);
struct device *master = data->master;

if (master) {
- pm_runtime_put(dev);
+ struct exynos_iommu_owner *owner = master->archdata.iommu;
+
+ mutex_lock(&owner->rpm_lock);
if (data->domain) {
dev_dbg(data->sysmmu, "saving state\n");
__sysmmu_disable(data);
}
+ mutex_unlock(&owner->rpm_lock);
}
return 0;
}

-static int exynos_sysmmu_resume(struct device *dev)
+static int __maybe_unused exynos_sysmmu_resume(struct device *dev)
{
struct sysmmu_drvdata *data = dev_get_drvdata(dev);
struct device *master = data->master;

if (master) {
- pm_runtime_get_sync(dev);
+ struct exynos_iommu_owner *owner = master->archdata.iommu;
+
+ mutex_lock(&owner->rpm_lock);
if (data->domain) {
dev_dbg(data->sysmmu, "restoring state\n");
__sysmmu_enable(data);
}
+ mutex_unlock(&owner->rpm_lock);
}
return 0;
}
-#endif

static const struct dev_pm_ops sysmmu_pm_ops = {
- SET_LATE_SYSTEM_SLEEP_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume)
+ SET_RUNTIME_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume, NULL)
+ SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
};

static const struct of_device_id sysmmu_of_match[] __initconst = {
@@ -775,7 +782,15 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
return;

list_for_each_entry(data, &owner->controllers, owner_node) {
- __sysmmu_disable(data);
+ pm_runtime_put_sync(data->sysmmu);
+ }
+
+ mutex_lock(&owner->rpm_lock);
+
+ list_for_each_entry(data, &owner->controllers, owner_node) {
+ pm_runtime_get_noresume(data->sysmmu);
+ if (pm_runtime_active(data->sysmmu))
+ __sysmmu_disable(data);
pm_runtime_put(data->sysmmu);
}

@@ -790,6 +805,7 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
owner->domain = NULL;
spin_unlock_irqrestore(&domain->lock, flags);

+ mutex_unlock(&owner->rpm_lock);

dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", __func__,
&pagetable);
@@ -810,6 +826,8 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
if (owner->domain)
exynos_iommu_detach_device(owner->domain, dev);

+ mutex_lock(&owner->rpm_lock);
+
spin_lock_irqsave(&domain->lock, flags);
list_for_each_entry(data, &owner->controllers, owner_node) {
spin_lock(&data->lock);
@@ -822,8 +840,16 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
spin_unlock_irqrestore(&domain->lock, flags);

list_for_each_entry(data, &owner->controllers, owner_node) {
+ pm_runtime_get_noresume(data->sysmmu);
+ if (pm_runtime_active(data->sysmmu))
+ __sysmmu_enable(data);
+ pm_runtime_put(data->sysmmu);
+ }
+
+ mutex_unlock(&owner->rpm_lock);
+
+ list_for_each_entry(data, &owner->controllers, owner_node) {
pm_runtime_get_sync(data->sysmmu);
- __sysmmu_enable(data);
}

dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__,
@@ -1200,6 +1226,7 @@ static int exynos_iommu_of_xlate(struct device *dev,
return -ENOMEM;

INIT_LIST_HEAD(&owner->controllers);
+ mutex_init(&owner->rpm_lock);
dev->archdata.iommu = owner;
}
--
1.9.1
Joerg Roedel
2016-11-14 16:13:32 UTC
Permalink
Joerg: I hope you can merge this version on top of Greg's driver-core-next
branch to iommu tree.
Merged Greg's driver-core-next branch and applied these on-top,
thanks Marek.


Joerg

Loading...