bus: mhi: ep: Change state_lock to mutex

[ Upstream commit 7b96306c051550829ae3d195865ed2627b3c9aa3 ]

state_lock, the spinlock type is meant to protect race against concurrent
MHI state transitions. In mhi_ep_set_m0_state(), while the state_lock is
being held, the channels are resumed in mhi_ep_resume_channels() if the
previous state was M3. This causes sleeping in atomic bug, since
mhi_ep_resume_channels() use mutex internally.

Since the state_lock is supposed to be held throughout the state change,
it is not ideal to drop the lock before calling mhi_ep_resume_channels().
So to fix this issue, let's change the type of state_lock to mutex. This
would also allow holding the lock throughout all state transitions thereby
avoiding any potential race.

Cc: <stable@vger.kernel.org> # 5.19
Fixes: 9ac68db5eafd ("bus: mhi: ep: Add support for suspending and resuming channels")
Reported-by: Dan Carpenter <error27@gmail.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
index b06548005985cdb7a9aed90d9266213460035d99..edd153dda40c07982ab9240b8b1d19c5287f56cf 100644 (file)
--- a/drivers/bus/mhi/ep/main.c
+++ b/drivers/bus/mhi/ep/main.c
@@ -994,11 +994,11 @@ static void mhi_ep_reset_worker(struct work_struct *work)
 
        mhi_ep_power_down(mhi_cntrl);
 
-       spin_lock_bh(&mhi_cntrl->state_lock);
+       mutex_lock(&mhi_cntrl->state_lock);
+
        /* Reset MMIO to signal host that the MHI_RESET is completed in endpoint */
        mhi_ep_mmio_reset(mhi_cntrl);
        cur_state = mhi_cntrl->mhi_state;
-       spin_unlock_bh(&mhi_cntrl->state_lock);
 
        /*
         * Only proceed further if the reset is due to SYS_ERR. The host will
@@ -1007,6 +1007,8 @@ static void mhi_ep_reset_worker(struct work_struct *work)
         */
        if (cur_state == MHI_STATE_SYS_ERR)
                mhi_ep_power_up(mhi_cntrl);
+
+       mutex_unlock(&mhi_cntrl->state_lock);
 }
 
 /*
@@ -1379,8 +1381,8 @@ int mhi_ep_register_controller(struct mhi_ep_cntrl *mhi_cntrl,
 
        INIT_LIST_HEAD(&mhi_cntrl->st_transition_list);
        INIT_LIST_HEAD(&mhi_cntrl->ch_db_list);
-       spin_lock_init(&mhi_cntrl->state_lock);
        spin_lock_init(&mhi_cntrl->list_lock);
+       mutex_init(&mhi_cntrl->state_lock);
        mutex_init(&mhi_cntrl->event_lock);
 
        /* Set MHI version and AMSS EE before enumeration */

diff --git a/drivers/bus/mhi/ep/sm.c b/drivers/bus/mhi/ep/sm.c
index 3655c19e23c7b5f0c86f5bda14212016c3a3f317..fd200b2ac0bb265c630f245417dead76e5b9742c 100644 (file)
--- a/drivers/bus/mhi/ep/sm.c
+++ b/drivers/bus/mhi/ep/sm.c
@@ -63,24 +63,23 @@ int mhi_ep_set_m0_state(struct mhi_ep_cntrl *mhi_cntrl)
        int ret;
 
        /* If MHI is in M3, resume suspended channels */
-       spin_lock_bh(&mhi_cntrl->state_lock);
+       mutex_lock(&mhi_cntrl->state_lock);
+
        old_state = mhi_cntrl->mhi_state;
        if (old_state == MHI_STATE_M3)
                mhi_ep_resume_channels(mhi_cntrl);
 
        ret = mhi_ep_set_mhi_state(mhi_cntrl, MHI_STATE_M0);
-       spin_unlock_bh(&mhi_cntrl->state_lock);
-
        if (ret) {
                mhi_ep_handle_syserr(mhi_cntrl);
-               return ret;
+               goto err_unlock;
        }
 
        /* Signal host that the device moved to M0 */
        ret = mhi_ep_send_state_change_event(mhi_cntrl, MHI_STATE_M0);
        if (ret) {
                dev_err(dev, "Failed sending M0 state change event\n");
-               return ret;
+               goto err_unlock;
        }
 
        if (old_state == MHI_STATE_READY) {
@@ -88,11 +87,14 @@ int mhi_ep_set_m0_state(struct mhi_ep_cntrl *mhi_cntrl)
                ret = mhi_ep_send_ee_event(mhi_cntrl, MHI_EE_AMSS);
                if (ret) {
                        dev_err(dev, "Failed sending AMSS EE event\n");
-                       return ret;
+                       goto err_unlock;
                }
        }
 
-       return 0;
+err_unlock:
+       mutex_unlock(&mhi_cntrl->state_lock);
+
+       return ret;
 }
 
 int mhi_ep_set_m3_state(struct mhi_ep_cntrl *mhi_cntrl)
@@ -100,13 +102,12 @@ int mhi_ep_set_m3_state(struct mhi_ep_cntrl *mhi_cntrl)
        struct device *dev = &mhi_cntrl->mhi_dev->dev;
        int ret;
 
-       spin_lock_bh(&mhi_cntrl->state_lock);
-       ret = mhi_ep_set_mhi_state(mhi_cntrl, MHI_STATE_M3);
-       spin_unlock_bh(&mhi_cntrl->state_lock);
+       mutex_lock(&mhi_cntrl->state_lock);
 
+       ret = mhi_ep_set_mhi_state(mhi_cntrl, MHI_STATE_M3);
        if (ret) {
                mhi_ep_handle_syserr(mhi_cntrl);
-               return ret;
+               goto err_unlock;
        }
 
        mhi_ep_suspend_channels(mhi_cntrl);
@@ -115,10 +116,13 @@ int mhi_ep_set_m3_state(struct mhi_ep_cntrl *mhi_cntrl)
        ret = mhi_ep_send_state_change_event(mhi_cntrl, MHI_STATE_M3);
        if (ret) {
                dev_err(dev, "Failed sending M3 state change event\n");
-               return ret;
+               goto err_unlock;
        }
 
-       return 0;
+err_unlock:
+       mutex_unlock(&mhi_cntrl->state_lock);
+
+       return ret;
 }
 
 int mhi_ep_set_ready_state(struct mhi_ep_cntrl *mhi_cntrl)
@@ -127,22 +131,24 @@ int mhi_ep_set_ready_state(struct mhi_ep_cntrl *mhi_cntrl)
        enum mhi_state mhi_state;
        int ret, is_ready;
 
-       spin_lock_bh(&mhi_cntrl->state_lock);
+       mutex_lock(&mhi_cntrl->state_lock);
+
        /* Ensure that the MHISTATUS is set to RESET by host */
        mhi_state = mhi_ep_mmio_masked_read(mhi_cntrl, EP_MHISTATUS, MHISTATUS_MHISTATE_MASK);
        is_ready = mhi_ep_mmio_masked_read(mhi_cntrl, EP_MHISTATUS, MHISTATUS_READY_MASK);
 
        if (mhi_state != MHI_STATE_RESET || is_ready) {
                dev_err(dev, "READY state transition failed. MHI host not in RESET state\n");
-               spin_unlock_bh(&mhi_cntrl->state_lock);
-               return -EIO;
+               ret = -EIO;
+               goto err_unlock;
        }
 
        ret = mhi_ep_set_mhi_state(mhi_cntrl, MHI_STATE_READY);
-       spin_unlock_bh(&mhi_cntrl->state_lock);
-
        if (ret)
                mhi_ep_handle_syserr(mhi_cntrl);
 
+err_unlock:
+       mutex_unlock(&mhi_cntrl->state_lock);
+
        return ret;
 }

diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
index 478aece170462e59f23de633c581e2d9f5d6b681..f198a8ac7ee72c71bae107d4ea9b070a85a86008 100644 (file)
--- a/include/linux/mhi_ep.h
+++ b/include/linux/mhi_ep.h
@@ -70,8 +70,8 @@ struct mhi_ep_db_info {
  * @cmd_ctx_cache_phys: Physical address of the host command context cache
  * @chdb: Array of channel doorbell interrupt info
  * @event_lock: Lock for protecting event rings
- * @list_lock: Lock for protecting state transition and channel doorbell lists
  * @state_lock: Lock for protecting state transitions
+ * @list_lock: Lock for protecting state transition and channel doorbell lists
  * @st_transition_list: List of state transitions
  * @ch_db_list: List of queued channel doorbells
  * @wq: Dedicated workqueue for handling rings and state changes
@@ -117,8 +117,8 @@ struct mhi_ep_cntrl {
 
        struct mhi_ep_db_info chdb[4];
        struct mutex event_lock;
+       struct mutex state_lock;
        spinlock_t list_lock;
-       spinlock_t state_lock;
 
        struct list_head st_transition_list;
        struct list_head ch_db_list;