diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h index 1f2857b3f38e..ef60e790a750 100644 --- a/drivers/soc/qcom/rpmh-internal.h +++ b/drivers/soc/qcom/rpmh-internal.h @@ -95,7 +95,7 @@ struct rpmh_ctrlr { * @num_tcs: Number of TCSes in this DRV. * @rsc_pm: CPU PM notifier for controller. * Used when solver mode is not present. - * @cpus_entered_pm: CPU mask for cpus in idle power collapse. + * @cpus_in_pm: Number of CPUs not in idle power collapse. * Used when solver mode is not present. * @tcs: TCS groups. * @tcs_in_use: S/W state of the TCS; only set for ACTIVE_ONLY @@ -103,9 +103,9 @@ struct rpmh_ctrlr { * it was borrowed for an active_only transfer. You * must hold the lock in this struct (AKA drv->lock) in * order to update this. - * @lock: Synchronize state of the controller. - * @pm_lock: Synchronize during PM notifications. - * Used when solver mode is not present. + * @lock: Synchronize state of the controller. If RPMH's cache + * lock will also be held, the order is: drv->lock then + * cache_lock. * @client: Handle to the DRV's client. */ struct rsc_drv { @@ -114,11 +114,10 @@ struct rsc_drv { int id; int num_tcs; struct notifier_block rsc_pm; - struct cpumask cpus_entered_pm; + atomic_t cpus_in_pm; struct tcs_group tcs[TCS_TYPE_NR]; DECLARE_BITMAP(tcs_in_use, MAX_TCS_NR); spinlock_t lock; - spinlock_t pm_lock; struct rpmh_ctrlr client; }; diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index fb142dfbb237..237d7d5cc8a8 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -750,6 +750,8 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, const struct tcs_request *msg) * SLEEP and WAKE sets. If AMCs are busy, controller can not enter * power collapse, so deny from the last cpu's pm notification. * + * Context: Must be called with the drv->lock held. + * * Return: * * False - AMCs are idle * * True - AMCs are busy @@ -764,9 +766,6 @@ static bool rpmh_rsc_ctrlr_is_busy(struct rsc_drv *drv) * dedicated TCS for active state use, then re-purposed wake TCSes * should be checked for not busy, because we used wake TCSes for * active requests in this case. - * - * Since this is called from the last cpu, need not take drv->lock - * before checking tcs_is_free(). */ if (!tcs->num_tcs) tcs = &drv->tcs[WAKE_TCS]; @@ -801,43 +800,62 @@ static int rpmh_rsc_cpu_pm_callback(struct notifier_block *nfb, { struct rsc_drv *drv = container_of(nfb, struct rsc_drv, rsc_pm); int ret = NOTIFY_OK; - - spin_lock(&drv->pm_lock); + int cpus_in_pm; switch (action) { case CPU_PM_ENTER: - cpumask_set_cpu(smp_processor_id(), &drv->cpus_entered_pm); - - if (!cpumask_equal(&drv->cpus_entered_pm, cpu_online_mask)) - goto exit; + cpus_in_pm = atomic_inc_return(&drv->cpus_in_pm); + /* + * NOTE: comments for num_online_cpus() point out that it's + * only a snapshot so we need to be careful. It should be OK + * for us to use, though. It's important for us not to miss + * if we're the last CPU going down so it would only be a + * problem if a CPU went offline right after we did the check + * AND that CPU was not idle AND that CPU was the last non-idle + * CPU. That can't happen. CPUs would have to come out of idle + * before the CPU could go offline. + */ + if (cpus_in_pm < num_online_cpus()) + return NOTIFY_OK; break; case CPU_PM_ENTER_FAILED: case CPU_PM_EXIT: - cpumask_clear_cpu(smp_processor_id(), &drv->cpus_entered_pm); - goto exit; + atomic_dec(&drv->cpus_in_pm); + return NOTIFY_OK; default: - ret = NOTIFY_DONE; - goto exit; + return NOTIFY_DONE; } - ret = rpmh_rsc_ctrlr_is_busy(drv); - if (ret) { - ret = NOTIFY_BAD; - goto exit; + /* + * It's likely we're on the last CPU. Grab the drv->lock and write + * out the sleep/wake commands to RPMH hardware. Grabbing the lock + * means that if we race with another CPU coming up we are still + * guaranteed to be safe. If another CPU came up just after we checked + * and has grabbed the lock or started an active transfer then we'll + * notice we're busy and abort. If another CPU comes up after we start + * flushing it will be blocked from starting an active transfer until + * we're done flushing. If another CPU starts an active transfer after + * we release the lock we're still OK because we're no longer the last + * CPU. + */ + if (spin_trylock(&drv->lock)) { + if (rpmh_rsc_ctrlr_is_busy(drv) || rpmh_flush(&drv->client)) + ret = NOTIFY_BAD; + spin_unlock(&drv->lock); + } else { + /* Another CPU must be up */ + return NOTIFY_OK; } - ret = rpmh_flush(&drv->client); - if (ret) - ret = NOTIFY_BAD; - else - ret = NOTIFY_OK; + if (ret == NOTIFY_BAD) { + /* Double-check if we're here because someone else is up */ + if (cpus_in_pm < num_online_cpus()) + ret = NOTIFY_OK; + else + /* We won't be called w/ CPU_PM_ENTER_FAILED */ + atomic_dec(&drv->cpus_in_pm); + } -exit: - if (ret == NOTIFY_BAD) - /* We won't be called w/ CPU_PM_ENTER_FAILED */ - cpumask_clear_cpu(smp_processor_id(), &drv->cpus_entered_pm); - - spin_unlock(&drv->pm_lock); return ret; } @@ -980,7 +998,6 @@ static int rpmh_rsc_probe(struct platform_device *pdev) solver_config = solver_config >> DRV_HW_SOLVER_SHIFT; if (!solver_config) { drv->rsc_pm.notifier_call = rpmh_rsc_cpu_pm_callback; - spin_lock_init(&drv->pm_lock); cpu_pm_register_notifier(&drv->rsc_pm); } diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c index d1626a1328d7..f2b5b46ccd1f 100644 --- a/drivers/soc/qcom/rpmh.c +++ b/drivers/soc/qcom/rpmh.c @@ -435,9 +435,6 @@ static int send_single(struct rpmh_ctrlr *ctrlr, enum rpmh_state state, * * @ctrlr: Controller making request to flush cached data * - * This function is called from sleep code on the last CPU - * (thus no spinlock needed). - * * Return: * * 0 - Success * * Error code - Otherwise @@ -445,13 +442,21 @@ static int send_single(struct rpmh_ctrlr *ctrlr, enum rpmh_state state, int rpmh_flush(struct rpmh_ctrlr *ctrlr) { struct cache_req *p; - int ret; + int ret = 0; lockdep_assert_irqs_disabled(); + /* + * Currently rpmh_flush() is only called when we think we're running + * on the last processor. If the lock is busy it means another + * processor is up and it's better to abort than spin. + */ + if (!spin_trylock(&ctrlr->cache_lock)) + return -EBUSY; + if (!ctrlr->dirty) { pr_debug("Skipping flush, TCS has latest data.\n"); - return 0; + goto exit; } /* Invalidate the TCSes first to avoid stale data */ @@ -460,7 +465,7 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr) /* First flush the cached batch requests */ ret = flush_batch(ctrlr); if (ret) - return ret; + goto exit; list_for_each_entry(p, &ctrlr->cache, list) { if (!is_req_valid(p)) { @@ -471,16 +476,18 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr) ret = send_single(ctrlr, RPMH_SLEEP_STATE, p->addr, p->sleep_val); if (ret) - return ret; + goto exit; ret = send_single(ctrlr, RPMH_WAKE_ONLY_STATE, p->addr, p->wake_val); if (ret) - return ret; + goto exit; } ctrlr->dirty = false; - return 0; +exit: + spin_unlock(&ctrlr->cache_lock); + return ret; } /**