* [PATCH v6 1/7] iommu/exynos: Remove excessive, useless debug
2016-11-08 13:29 ` Marek Szyprowski
@ 2016-11-08 13:29 ` Marek Szyprowski
-1 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel
Cc: Tomeu Vizoso, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Mark Brown, Kevin Hilman, Rafael J. Wysocki, Tomasz Figa,
Krzysztof Kozlowski, Inki Dae, Tobias Jakobi, Luis R. Rodriguez,
Kukjin Kim, Lukas Wunner
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 <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v6 1/7] iommu/exynos: Remove excessive, useless debug
@ 2016-11-08 13:29 ` Marek Szyprowski
0 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm, linux-kernel, iommu, linux-samsung-soc, Joerg Roedel
Cc: Marek Szyprowski, Inki Dae, Kukjin Kim, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Mark Brown,
Luis R. Rodriguez, Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner,
Kevin Hilman, Tobias Jakobi, Tomasz Figa
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 <m.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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v6 2/7] iommu/exynos: Remove dead code
2016-11-08 13:29 ` Marek Szyprowski
@ 2016-11-08 13:29 ` Marek Szyprowski
-1 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel
Cc: Tomeu Vizoso, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Mark Brown, Kevin Hilman, Rafael J. Wysocki, Tomasz Figa,
Krzysztof Kozlowski, Inki Dae, Tobias Jakobi, Luis R. Rodriguez,
Kukjin Kim, Lukas Wunner
__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 <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v6 2/7] iommu/exynos: Remove dead code
@ 2016-11-08 13:29 ` Marek Szyprowski
0 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm, linux-kernel, iommu, linux-samsung-soc, Joerg Roedel
Cc: Marek Szyprowski, Inki Dae, Kukjin Kim, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Mark Brown,
Luis R. Rodriguez, Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner,
Kevin Hilman, Tobias Jakobi, Tomasz Figa
__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 <m.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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v6 3/7] iommu/exynos: Simplify internal enable/disable functions
2016-11-08 13:29 ` Marek Szyprowski
@ 2016-11-08 13:29 ` Marek Szyprowski
-1 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel
Cc: Tomeu Vizoso, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Mark Brown, Kevin Hilman, Rafael J. Wysocki, Tomasz Figa,
Krzysztof Kozlowski, Inki Dae, Tobias Jakobi, Luis R. Rodriguez,
Kukjin Kim, Lukas Wunner
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 <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
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 */
+ int 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v6 3/7] iommu/exynos: Simplify internal enable/disable functions
@ 2016-11-08 13:29 ` Marek Szyprowski
0 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm, linux-kernel, iommu, linux-samsung-soc, Joerg Roedel
Cc: Marek Szyprowski, Inki Dae, Kukjin Kim, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Mark Brown,
Luis R. Rodriguez, Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner,
Kevin Hilman, Tobias Jakobi, Tomasz Figa
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 <m.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 */
+ int 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
^ permalink raw reply related [flat|nested] 21+ messages in thread[parent not found: <1478611764-6473-4-git-send-email-m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH v6 3/7] iommu/exynos: Simplify internal enable/disable functions
2016-11-08 13:29 ` Marek Szyprowski
@ 2016-11-10 16:40 ` Joerg Roedel
-1 siblings, 0 replies; 21+ messages in thread
From: Joerg Roedel @ 2016-11-10 16:40 UTC (permalink / raw)
To: Marek Szyprowski
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Tomeu Vizoso,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman, Mark Brown,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Krzysztof Kozlowski,
Tomasz Figa, Inki Dae, Kevin Hilman,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Kukjin Kim,
Luis R. Rodriguez, Tobias Jakobi, Lukas Wunner
On Tue, Nov 08, 2016 at 02:29:20PM +0100, Marek Szyprowski wrote:
> 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 <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> 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 */
> + int active; /* current status */
Any reason for this being int and not bool? You only assign true/false
later.
Joerg
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v6 3/7] iommu/exynos: Simplify internal enable/disable functions
@ 2016-11-10 16:40 ` Joerg Roedel
0 siblings, 0 replies; 21+ messages in thread
From: Joerg Roedel @ 2016-11-10 16:40 UTC (permalink / raw)
To: Marek Szyprowski
Cc: linux-pm, linux-kernel, iommu, linux-samsung-soc, Inki Dae,
Kukjin Kim, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
Rafael J. Wysocki, Mark Brown, Luis R. Rodriguez,
Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner, Kevin Hilman,
Tobias Jakobi, Tomasz Figa
On Tue, Nov 08, 2016 at 02:29:20PM +0100, Marek Szyprowski wrote:
> 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 <m.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 */
> + int active; /* current status */
Any reason for this being int and not bool? You only assign true/false
later.
Joerg
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v6 4/7] iommu/exynos: Set master device once on boot
2016-11-08 13:29 ` Marek Szyprowski
@ 2016-11-08 13:29 ` Marek Szyprowski
-1 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel
Cc: Tomeu Vizoso, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Mark Brown, Kevin Hilman, Rafael J. Wysocki, Tomasz Figa,
Krzysztof Kozlowski, Inki Dae, Tobias Jakobi, Luis R. Rodriguez,
Kukjin Kim, Lukas Wunner
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 <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v6 4/7] iommu/exynos: Set master device once on boot
@ 2016-11-08 13:29 ` Marek Szyprowski
0 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm, linux-kernel, iommu, linux-samsung-soc, Joerg Roedel
Cc: Marek Szyprowski, Inki Dae, Kukjin Kim, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Mark Brown,
Luis R. Rodriguez, Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner,
Kevin Hilman, Tobias Jakobi, Tomasz Figa
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 <m.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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v6 5/7] iommu/exynos: Rework and fix internal locking
2016-11-08 13:29 ` Marek Szyprowski
@ 2016-11-08 13:29 ` Marek Szyprowski
-1 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel
Cc: Tomeu Vizoso, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Mark Brown, Kevin Hilman, Rafael J. Wysocki, Tomasz Figa,
Krzysztof Kozlowski, Inki Dae, Tobias Jakobi, Luis R. Rodriguez,
Kukjin Kim, Lukas Wunner
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 <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v6 5/7] iommu/exynos: Rework and fix internal locking
@ 2016-11-08 13:29 ` Marek Szyprowski
0 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm, linux-kernel, iommu, linux-samsung-soc, Joerg Roedel
Cc: Marek Szyprowski, Inki Dae, Kukjin Kim, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Mark Brown,
Luis R. Rodriguez, Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner,
Kevin Hilman, Tobias Jakobi, Tomasz Figa
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 <m.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
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v6 7/7] iommu/exynos: Use device dependency links to control runtime pm
2016-11-08 13:29 ` Marek Szyprowski
@ 2016-11-08 13:29 ` Marek Szyprowski
-1 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Joerg Roedel
Cc: Tomeu Vizoso, Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Mark Brown, Kevin Hilman, Rafael J. Wysocki, Tomasz Figa,
Krzysztof Kozlowski, Inki Dae, Tobias Jakobi, Luis R. Rodriguez,
Kukjin Kim, Lukas Wunner
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.
The dependency links also enforces 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 <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v6 7/7] iommu/exynos: Use device dependency links to control runtime pm
@ 2016-11-08 13:29 ` Marek Szyprowski
0 siblings, 0 replies; 21+ messages in thread
From: Marek Szyprowski @ 2016-11-08 13:29 UTC (permalink / raw)
To: linux-pm, linux-kernel, iommu, linux-samsung-soc, Joerg Roedel
Cc: Marek Szyprowski, Inki Dae, Kukjin Kim, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Mark Brown,
Luis R. Rodriguez, Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner,
Kevin Hilman, Tobias Jakobi, Tomasz Figa
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.
The dependency links also enforces 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 <m.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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v6 7/7] iommu/exynos: Use device dependency links to control runtime pm
2016-11-08 13:29 ` Marek Szyprowski
@ 2016-11-09 23:31 ` Luis R. Rodriguez
-1 siblings, 0 replies; 21+ messages in thread
From: Luis R. Rodriguez @ 2016-11-09 23:31 UTC (permalink / raw)
To: Marek Szyprowski
Cc: linux-pm, linux-kernel, iommu, linux-samsung-soc, Joerg Roedel,
Inki Dae, Kukjin Kim, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Mark Brown,
Luis R. Rodriguez, Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner,
Kevin Hilman, Tobias Jakobi, Tomasz Figa, Laurent Pinchart,
Lars-Peter Clausen
On Tue, Nov 08, 2016 at 02:29:24PM +0100, Marek Szyprowski wrote:
> 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.
>
> The dependency links also enforces 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.
Patches 1-6 seems reasonable to me, however in so far as this patch is
concerned I'd appreaciate if you and Rafael can reply to Lukas Wunner's
questions.
Luis
>
> Signed-off-by: Marek Szyprowski <m.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
>
>
--
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v6 7/7] iommu/exynos: Use device dependency links to control runtime pm
@ 2016-11-09 23:31 ` Luis R. Rodriguez
0 siblings, 0 replies; 21+ messages in thread
From: Luis R. Rodriguez @ 2016-11-09 23:31 UTC (permalink / raw)
To: Marek Szyprowski
Cc: linux-pm, linux-kernel, iommu, linux-samsung-soc, Joerg Roedel,
Inki Dae, Kukjin Kim, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Mark Brown,
Luis R. Rodriguez, Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner,
Kevin Hilman, Tobias Jakobi, Tomasz Figa, Laurent Pinchart,
Lars-Peter Clausen, Dmitry Torokhov, Grant Likely,
Mauro Carvalho Chehab
On Tue, Nov 08, 2016 at 02:29:24PM +0100, Marek Szyprowski wrote:
> 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.
>
> The dependency links also enforces 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.
Patches 1-6 seems reasonable to me, however in so far as this patch is
concerned I'd appreaciate if you and Rafael can reply to Lukas Wunner's
questions.
Luis
>
> Signed-off-by: Marek Szyprowski <m.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
>
>
--
Luis Rodriguez, SUSE LINUX GmbH
Maxfeldstrasse 5; D-90409 Nuernberg
^ permalink raw reply [flat|nested] 21+ messages in thread[parent not found: <20161109233126.GM13978-B4tOwbsTzaBolqkO4TVVkw@public.gmane.org>]
* Re: [PATCH v6 7/7] iommu/exynos: Use device dependency links to control runtime pm
2016-11-09 23:31 ` Luis R. Rodriguez
@ 2016-11-09 23:42 ` Rafael J. Wysocki
-1 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2016-11-09 23:42 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Tomasz Figa, Grant Likely, Laurent Pinchart, Lukas Wunner,
Mauro Carvalho Chehab, Linux Samsung SoC, Kevin Hilman,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, Kukjin Kim,
Linux PM, Inki Dae, Lars-Peter Clausen, Tomeu Vizoso,
Greg Kroah-Hartman, Dmitry Torokhov, Rafael J. Wysocki,
Linux Kernel Mailing List, Tobias Jakobi,
open list:AMD IOMMU (AMD-VI)
On Thu, Nov 10, 2016 at 12:31 AM, Luis R. Rodriguez <mcgrof-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Nov 08, 2016 at 02:29:24PM +0100, Marek Szyprowski wrote:
>> 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.
>>
>> The dependency links also enforces 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.
>
> Patches 1-6 seems reasonable to me, however in so far as this patch is
> concerned I'd appreaciate if you and Rafael can reply to Lukas Wunner's
> questions.
Which questions exactly do you mean?
Thanks,
Rafael
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v6 7/7] iommu/exynos: Use device dependency links to control runtime pm
@ 2016-11-09 23:42 ` Rafael J. Wysocki
0 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2016-11-09 23:42 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Marek Szyprowski, Linux PM, Linux Kernel Mailing List,
open list:AMD IOMMU (AMD-VI), Linux Samsung SoC, Joerg Roedel,
Inki Dae, Kukjin Kim, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Mark Brown,
Greg Kroah-Hartman, Tomeu Vizoso, Lukas Wunner, Kevin Hilman,
Tobias Jakobi, Tomasz Figa, Laurent Pinchart, Lars-Peter Clausen,
Dmitry Torokhov, Grant Likely, Mauro Carvalho Chehab
On Thu, Nov 10, 2016 at 12:31 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Tue, Nov 08, 2016 at 02:29:24PM +0100, Marek Szyprowski wrote:
>> 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.
>>
>> The dependency links also enforces 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.
>
> Patches 1-6 seems reasonable to me, however in so far as this patch is
> concerned I'd appreaciate if you and Rafael can reply to Lukas Wunner's
> questions.
Which questions exactly do you mean?
Thanks,
Rafael
^ permalink raw reply [flat|nested] 21+ messages in thread