From 149c51d5f2b7b2028446e3757ff6b17e90092c48 Mon Sep 17 00:00:00 2001 From: Patrick Daly Date: Thu, 27 Oct 2016 18:08:01 -0700 Subject: [PATCH] iommu: arm-smmu: Fix clock reference count error When an atomic iommu domain attaches, an additional vote for both clk_prepare, bus_bw, and regulator_enable must be held. The prior logic only did this if the atomic domain was the first to attach to the iommu. Fix this. As a side effect, add reference counting for bus_bandwidth voting such that a call to arm_smmu_enable_clock() followed by arm_smmu_disable_clocks() will not always result in a bus bandwidth vote of zero. Change-Id: I7f88ea845a281c8c1def4f642e61262b53b60e1a Signed-off-by: Patrick Daly --- drivers/iommu/arm-smmu.c | 73 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 58 insertions(+), 15 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index eac6d07e6097..791f2fe70236 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -407,6 +407,9 @@ struct arm_smmu_device { unsigned int clock_refs_count; spinlock_t clock_refs_lock; + struct mutex power_lock; + unsigned int power_count; + struct msm_bus_client_handle *bus_client; char *bus_client_name; @@ -919,17 +922,43 @@ static int arm_smmu_unrequest_bus(struct arm_smmu_device *smmu) static int arm_smmu_disable_regulators(struct arm_smmu_device *smmu) { + int ret = 0; + + mutex_lock(&smmu->power_lock); + if (smmu->power_count == 0) { + WARN(1, "%s: Mismatched power count\n", dev_name(smmu->dev)); + mutex_unlock(&smmu->power_lock); + return -EINVAL; + } else if (smmu->power_count > 1) { + smmu->power_count -= 1; + mutex_unlock(&smmu->power_lock); + return 0; + } + arm_smmu_unprepare_clocks(smmu); arm_smmu_unrequest_bus(smmu); - if (!smmu->gdsc) - return 0; - return regulator_disable(smmu->gdsc); + if (smmu->gdsc) { + ret = regulator_disable(smmu->gdsc); + WARN(ret, "%s: Regulator disable failed\n", + dev_name(smmu->dev)); + } + + smmu->power_count = 0; + mutex_unlock(&smmu->power_lock); + return ret; } static int arm_smmu_enable_regulators(struct arm_smmu_device *smmu) { int ret; + mutex_lock(&smmu->power_lock); + if (smmu->power_count) { + smmu->power_count++; + mutex_unlock(&smmu->power_lock); + return 0; + } + if (smmu->gdsc) { ret = regulator_enable(smmu->gdsc); if (WARN_ON_ONCE(ret)) @@ -944,6 +973,8 @@ static int arm_smmu_enable_regulators(struct arm_smmu_device *smmu) if (WARN_ON_ONCE(ret)) goto out_bus; + smmu->power_count = 1; + mutex_unlock(&smmu->power_lock); return ret; out_bus: @@ -952,6 +983,7 @@ out_reg: if (smmu->gdsc) regulator_disable(smmu->gdsc); out: + mutex_unlock(&smmu->power_lock); return ret; } @@ -2217,8 +2249,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) * refcounting) is that we never disable regulators while a * client is attached in these cases. */ - if (!(smmu->options & ARM_SMMU_OPT_REGISTER_SAVE) || - atomic_ctx) { + if (!(smmu->options & ARM_SMMU_OPT_REGISTER_SAVE)) { ret = arm_smmu_enable_regulators(smmu); if (ret) goto err_unlock; @@ -2235,12 +2266,18 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) } smmu->attach_count++; + if (atomic_ctx) { + ret = arm_smmu_enable_regulators(smmu); + if (ret) + goto err_disable_clocks; + } + if (arm_smmu_is_static_cb(smmu)) { ret = arm_smmu_populate_cb(smmu, smmu_domain, dev); if (ret) { dev_err(dev, "Failed to get valid context bank\n"); - goto err_disable_clocks; + goto err_atomic_ctx; } smmu_domain->slave_side_secure = true; } @@ -2248,13 +2285,13 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) cfg = find_smmu_master_cfg(dev); if (!cfg) { ret = -ENODEV; - goto err_disable_clocks; + goto err_atomic_ctx; } /* Ensure that the domain is finalised */ ret = arm_smmu_init_domain_context(domain, smmu, cfg); if (IS_ERR_VALUE(ret)) - goto err_disable_clocks; + goto err_atomic_ctx; /* * Sanity check the domain. We don't support domains across @@ -2280,12 +2317,15 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) err_destroy_domain_context: arm_smmu_destroy_domain_context(domain); +err_atomic_ctx: + if (atomic_ctx) + arm_smmu_disable_regulators(smmu); err_disable_clocks: arm_smmu_disable_clocks(smmu); --smmu->attach_count; err_disable_regulators: if (!smmu->attach_count && - (!(smmu->options & ARM_SMMU_OPT_REGISTER_SAVE) || atomic_ctx)) + (!(smmu->options & ARM_SMMU_OPT_REGISTER_SAVE))) arm_smmu_disable_regulators(smmu); err_unlock: mutex_unlock(&smmu->attach_lock); @@ -2293,8 +2333,7 @@ err_unlock: return ret; } -static void arm_smmu_power_off(struct arm_smmu_device *smmu, - bool force_regulator_disable) +static void arm_smmu_power_off(struct arm_smmu_device *smmu) { /* Turn the thing off */ if (arm_smmu_enable_clocks(smmu)) @@ -2302,8 +2341,7 @@ static void arm_smmu_power_off(struct arm_smmu_device *smmu, writel_relaxed(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0); arm_smmu_disable_clocks(smmu); - if (!(smmu->options & ARM_SMMU_OPT_REGISTER_SAVE) - || force_regulator_disable) + if (!(smmu->options & ARM_SMMU_OPT_REGISTER_SAVE)) arm_smmu_disable_regulators(smmu); } @@ -2345,6 +2383,8 @@ static void arm_smmu_detach_dev(struct iommu_domain *domain, struct device *dev) if (smmu_domain->attributes & (1 << DOMAIN_ATTR_DYNAMIC)) { arm_smmu_detach_dynamic(domain, smmu); mutex_unlock(&smmu_domain->init_mutex); + if (atomic_ctx) + arm_smmu_disable_regulators(smmu); return; } @@ -2358,7 +2398,9 @@ static void arm_smmu_detach_dev(struct iommu_domain *domain, struct device *dev) arm_smmu_domain_remove_master(smmu_domain, cfg); arm_smmu_destroy_domain_context(domain); if (!--smmu->attach_count) - arm_smmu_power_off(smmu, atomic_ctx); + arm_smmu_power_off(smmu); + if (atomic_ctx) + arm_smmu_disable_regulators(smmu); unlock: mutex_unlock(&smmu->attach_lock); mutex_unlock(&smmu_domain->init_mutex); @@ -3789,6 +3831,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev) } smmu->dev = dev; mutex_init(&smmu->attach_lock); + mutex_init(&smmu->power_lock); spin_lock_init(&smmu->atos_lock); spin_lock_init(&smmu->clock_refs_lock); INIT_LIST_HEAD(&smmu->static_cbndx_list); @@ -3970,7 +4013,7 @@ static int arm_smmu_device_remove(struct platform_device *pdev) * still powered on. Power off now. */ if (smmu->attach_count) - arm_smmu_power_off(smmu, false); + arm_smmu_power_off(smmu); mutex_unlock(&smmu->attach_lock); msm_bus_scale_unregister(smmu->bus_client); -- 2.11.0