From ad557197ebfd59d958644cf5dfedd7049e9223e8 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Fri, 13 Dec 2019 14:34:14 -0800 Subject: [PATCH] mhi: core: Synchronize time sync operation and removal If time sync is requested by user space when endpoint is shutting down dev_wake count gets incremented. This is causing assert when shutdown API checks the dev_wake count and it is still non-zero. Fix this issue by protecting time sync API and removal of time sync under tsync_mutex. This helps to take care of a race condition when time sync is requested while time sync remove in progress and also makes sure that dev_wake count becomes zero when time sync remove is done to prevent assert condition. Change-Id: I4e758611613e541537dd900306aa81dd3b4a9f3a Signed-off-by: Hemant Kumar --- drivers/bus/mhi/core/mhi_init.c | 2 +- drivers/bus/mhi/core/mhi_internal.h | 1 - drivers/bus/mhi/core/mhi_main.c | 29 ++++++++++++++++++----------- drivers/bus/mhi/core/mhi_pm.c | 6 +++--- include/linux/mhi.h | 1 + 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/drivers/bus/mhi/core/mhi_init.c b/drivers/bus/mhi/core/mhi_init.c index 8e3a5861f4ae..30005f38e37c 100644 --- a/drivers/bus/mhi/core/mhi_init.c +++ b/drivers/bus/mhi/core/mhi_init.c @@ -592,7 +592,6 @@ int mhi_init_timesync(struct mhi_controller *mhi_cntrl) return -ENOMEM; spin_lock_init(&mhi_tsync->lock); - mutex_init(&mhi_tsync->lpm_mutex); INIT_LIST_HEAD(&mhi_tsync->head); init_completion(&mhi_tsync->completion); @@ -1356,6 +1355,7 @@ int of_register_mhi_controller(struct mhi_controller *mhi_cntrl) INIT_LIST_HEAD(&mhi_cntrl->transition_list); mutex_init(&mhi_cntrl->pm_mutex); + mutex_init(&mhi_cntrl->tsync_mutex); rwlock_init(&mhi_cntrl->pm_lock); spin_lock_init(&mhi_cntrl->transition_lock); spin_lock_init(&mhi_cntrl->wlock); diff --git a/drivers/bus/mhi/core/mhi_internal.h b/drivers/bus/mhi/core/mhi_internal.h index da76271371c3..772a7b71937e 100644 --- a/drivers/bus/mhi/core/mhi_internal.h +++ b/drivers/bus/mhi/core/mhi_internal.h @@ -714,7 +714,6 @@ struct mhi_timesync { enum MHI_EV_CCS ccs; struct completion completion; spinlock_t lock; /* list protection */ - struct mutex lpm_mutex; /* lpm protection */ struct list_head head; }; diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index 0cd40bf99786..65f1f4866c72 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -759,10 +759,12 @@ static const struct attribute_group mhi_tsync_group = { void mhi_destroy_timesync(struct mhi_controller *mhi_cntrl) { if (mhi_cntrl->mhi_tsync) { + mutex_lock(&mhi_cntrl->tsync_mutex); sysfs_remove_group(&mhi_cntrl->mhi_dev->dev.kobj, &mhi_tsync_group); kfree(mhi_cntrl->mhi_tsync); mhi_cntrl->mhi_tsync = NULL; + mutex_unlock(&mhi_cntrl->tsync_mutex); } } @@ -2346,16 +2348,17 @@ int mhi_get_remote_time_sync(struct mhi_device *mhi_dev, struct mhi_timesync *mhi_tsync = mhi_cntrl->mhi_tsync; int ret; + mutex_lock(&mhi_cntrl->tsync_mutex); /* not all devices support time feature */ - if (!mhi_tsync) - return -EIO; + if (!mhi_tsync) { + ret = -EIO; + goto err_unlock; + } /* bring to M0 state */ ret = __mhi_device_get_sync(mhi_cntrl); if (ret) - return ret; - - mutex_lock(&mhi_tsync->lpm_mutex); + goto err_unlock; read_lock_bh(&mhi_cntrl->pm_lock); if (unlikely(MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state))) { @@ -2389,8 +2392,8 @@ int mhi_get_remote_time_sync(struct mhi_device *mhi_dev, error_invalid_state: mhi_cntrl->wake_put(mhi_cntrl, false); read_unlock_bh(&mhi_cntrl->pm_lock); - mutex_unlock(&mhi_tsync->lpm_mutex); - +err_unlock: + mutex_unlock(&mhi_cntrl->tsync_mutex); return ret; } EXPORT_SYMBOL(mhi_get_remote_time_sync); @@ -2418,13 +2421,16 @@ int mhi_get_remote_time(struct mhi_device *mhi_dev, int ret; /* not all devices support time feature */ - if (!mhi_tsync) - return -EIO; + mutex_lock(&mhi_cntrl->tsync_mutex); + if (!mhi_tsync) { + ret = -EIO; + goto err_unlock; + } /* tsync db can only be rung in M0 state */ ret = __mhi_device_get_sync(mhi_cntrl); if (ret) - return ret; + goto err_unlock; /* * technically we can use GFP_KERNEL, but wants to avoid @@ -2483,7 +2489,8 @@ error_no_mem: read_lock_bh(&mhi_cntrl->pm_lock); mhi_cntrl->wake_put(mhi_cntrl, false); read_unlock_bh(&mhi_cntrl->pm_lock); - +err_unlock: + mutex_unlock(&mhi_cntrl->tsync_mutex); return ret; } EXPORT_SYMBOL(mhi_get_remote_time); diff --git a/drivers/bus/mhi/core/mhi_pm.c b/drivers/bus/mhi/core/mhi_pm.c index a7c297c8d0b6..464c150a7f95 100644 --- a/drivers/bus/mhi/core/mhi_pm.c +++ b/drivers/bus/mhi/core/mhi_pm.c @@ -635,6 +635,9 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl, flush_work(&mhi_cntrl->fw_worker); flush_work(&mhi_cntrl->low_priority_worker); + /* remove support for time sync */ + mhi_destroy_timesync(mhi_cntrl); + mutex_lock(&mhi_cntrl->pm_mutex); MHI_ASSERT(atomic_read(&mhi_cntrl->dev_wake), "dev_wake != 0"); @@ -669,9 +672,6 @@ static void mhi_pm_disable_transition(struct mhi_controller *mhi_cntrl, er_ctxt->wp = er_ctxt->rbase; } - /* remove support for time sync */ - mhi_destroy_timesync(mhi_cntrl); - if (cur_state == MHI_PM_SYS_ERR_PROCESS) { mhi_ready_state_transition(mhi_cntrl); } else { diff --git a/include/linux/mhi.h b/include/linux/mhi.h index 1537bdadf45c..6bdf731936b9 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -303,6 +303,7 @@ struct mhi_controller { /* caller should grab pm_mutex for suspend/resume operations */ struct mutex pm_mutex; + struct mutex tsync_mutex; bool pre_init; rwlock_t pm_lock; u32 pm_state;