From 16caf4dcb7312662277a08484978c6bc515a5a93 Mon Sep 17 00:00:00 2001 From: Bhaumik Bhatt Date: Tue, 26 May 2020 18:09:17 -0700 Subject: [PATCH 1/3] mhi: core: block fast suspends on controller device bus vote Currently only a pending packets check is used to block fast suspends. Since it does not exactly reflect what the core driver intends to do, allow a bus vote to the controller device to block fast suspends as well. Change-Id: I31266d296e855027f6b49b2dcfe4606bb48ac221 Signed-off-by: Bhaumik Bhatt --- drivers/bus/mhi/core/mhi_main.c | 6 +----- drivers/bus/mhi/core/mhi_pm.c | 7 +++++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index 4ba83b09dc0c..1af6ebe9ddd2 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -1475,13 +1475,10 @@ int mhi_process_bw_scale_ev_ring(struct mhi_controller *mhi_cntrl, read_unlock_bh(&mhi_cntrl->pm_lock); spin_unlock_bh(&mhi_event->lock); - atomic_inc(&mhi_cntrl->pending_pkts); ret = mhi_device_get_sync(mhi_cntrl->mhi_dev, MHI_VOTE_DEVICE | MHI_VOTE_BUS); - if (ret) { - atomic_dec(&mhi_cntrl->pending_pkts); + if (ret) goto exit_bw_scale_process; - } mutex_lock(&mhi_cntrl->pm_mutex); @@ -1499,7 +1496,6 @@ int mhi_process_bw_scale_ev_ring(struct mhi_controller *mhi_cntrl, read_unlock_bh(&mhi_cntrl->pm_lock); mhi_device_put(mhi_cntrl->mhi_dev, MHI_VOTE_DEVICE | MHI_VOTE_BUS); - atomic_dec(&mhi_cntrl->pending_pkts); mutex_unlock(&mhi_cntrl->pm_mutex); diff --git a/drivers/bus/mhi/core/mhi_pm.c b/drivers/bus/mhi/core/mhi_pm.c index ff265b8947f2..40275e3f4439 100644 --- a/drivers/bus/mhi/core/mhi_pm.c +++ b/drivers/bus/mhi/core/mhi_pm.c @@ -1223,6 +1223,7 @@ int mhi_pm_fast_suspend(struct mhi_controller *mhi_cntrl, bool notify_client) int ret; enum MHI_PM_STATE new_state; struct mhi_chan *itr, *tmp; + struct mhi_device *mhi_dev = mhi_cntrl->mhi_dev; read_lock_bh(&mhi_cntrl->pm_lock); if (mhi_cntrl->pm_state == MHI_PM_DISABLE) { @@ -1237,7 +1238,8 @@ int mhi_pm_fast_suspend(struct mhi_controller *mhi_cntrl, bool notify_client) read_unlock_bh(&mhi_cntrl->pm_lock); /* do a quick check to see if any pending votes to keep us busy */ - if (atomic_read(&mhi_cntrl->pending_pkts)) { + if (atomic_read(&mhi_cntrl->pending_pkts) || + atomic_read(&mhi_dev->bus_vote)) { MHI_VERB("Busy, aborting M3\n"); return -EBUSY; } @@ -1256,7 +1258,8 @@ int mhi_pm_fast_suspend(struct mhi_controller *mhi_cntrl, bool notify_client) * Check the votes once more to see if we should abort * suspend. */ - if (atomic_read(&mhi_cntrl->pending_pkts)) { + if (atomic_read(&mhi_cntrl->pending_pkts) || + atomic_read(&mhi_dev->bus_vote)) { MHI_VERB("Busy, aborting M3\n"); ret = -EBUSY; goto error_suspend; From 453fb4d1994c14d83c0e2fbacc8950021640b0fe Mon Sep 17 00:00:00 2001 From: Bhaumik Bhatt Date: Fri, 3 Apr 2020 14:13:12 -0700 Subject: [PATCH 2/3] mhi: core: improve time synchronization events handling Validate event ring accesses before walking special event rings and update processing time synchronization such that a doorbell is rung and future requests are queued until a response is received. Also, ensure events are only processed when they are pending. Change-Id: Ib8c9385530f822af3e53efb96b82c0f26c3438b7 Signed-off-by: Bhaumik Bhatt --- drivers/bus/mhi/core/mhi_init.c | 2 +- drivers/bus/mhi/core/mhi_internal.h | 8 +- drivers/bus/mhi/core/mhi_main.c | 160 ++++++++++++++++------------ drivers/bus/mhi/core/mhi_pm.c | 3 + 4 files changed, 97 insertions(+), 76 deletions(-) diff --git a/drivers/bus/mhi/core/mhi_init.c b/drivers/bus/mhi/core/mhi_init.c index a5284a1c25d3..18d39982d1f6 100644 --- a/drivers/bus/mhi/core/mhi_init.c +++ b/drivers/bus/mhi/core/mhi_init.c @@ -1234,7 +1234,7 @@ static int of_parse_ev_cfg(struct mhi_controller *mhi_cntrl, mhi_event->process_event = mhi_process_ctrl_ev_ring; break; case MHI_ER_TSYNC_ELEMENT_TYPE: - mhi_event->process_event = mhi_process_tsync_event_ring; + mhi_event->process_event = mhi_process_tsync_ev_ring; break; case MHI_ER_BW_SCALE_ELEMENT_TYPE: mhi_event->process_event = mhi_process_bw_scale_ev_ring; diff --git a/drivers/bus/mhi/core/mhi_internal.h b/drivers/bus/mhi/core/mhi_internal.h index 34c518d73d16..42a8b9540ed9 100644 --- a/drivers/bus/mhi/core/mhi_internal.h +++ b/drivers/bus/mhi/core/mhi_internal.h @@ -716,8 +716,6 @@ struct mhi_chan { struct tsync_node { struct list_head node; u32 sequence; - u32 int_sequence; - u64 local_time; u64 remote_time; struct mhi_device *mhi_dev; void (*cb_func)(struct mhi_device *mhi_dev, u32 sequence, @@ -727,7 +725,9 @@ struct tsync_node { struct mhi_timesync { void __iomem *time_reg; u32 int_sequence; + u64 local_time; bool db_support; + bool db_response_pending; spinlock_t lock; /* list protection */ struct list_head head; }; @@ -786,8 +786,8 @@ int mhi_process_data_event_ring(struct mhi_controller *mhi_cntrl, struct mhi_event *mhi_event, u32 event_quota); int mhi_process_ctrl_ev_ring(struct mhi_controller *mhi_cntrl, struct mhi_event *mhi_event, u32 event_quota); -int mhi_process_tsync_event_ring(struct mhi_controller *mhi_cntrl, - struct mhi_event *mhi_event, u32 event_quota); +int mhi_process_tsync_ev_ring(struct mhi_controller *mhi_cntrl, + struct mhi_event *mhi_event, u32 event_quota); int mhi_process_bw_scale_ev_ring(struct mhi_controller *mhi_cntrl, struct mhi_event *mhi_event, u32 event_quota); int mhi_send_cmd(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan, diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index 1af6ebe9ddd2..c9d750a669ef 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -1348,82 +1348,89 @@ int mhi_process_data_event_ring(struct mhi_controller *mhi_cntrl, return count; } -int mhi_process_tsync_event_ring(struct mhi_controller *mhi_cntrl, - struct mhi_event *mhi_event, - u32 event_quota) +int mhi_process_tsync_ev_ring(struct mhi_controller *mhi_cntrl, + struct mhi_event *mhi_event, + u32 event_quota) { - struct mhi_tre *dev_rp, *local_rp; + struct mhi_tre *dev_rp; struct mhi_ring *ev_ring = &mhi_event->ring; struct mhi_event_ctxt *er_ctxt = &mhi_cntrl->mhi_ctxt->er_ctxt[mhi_event->er_index]; struct mhi_timesync *mhi_tsync = mhi_cntrl->mhi_tsync; - int count = 0; - u32 int_sequence, unit; + u32 sequence; u64 remote_time; + int ret = 0; - if (unlikely(MHI_EVENT_ACCESS_INVALID(mhi_cntrl->pm_state))) { - MHI_LOG("No EV access, PM_STATE:%s\n", - to_mhi_pm_state_str(mhi_cntrl->pm_state)); - return -EIO; - } - + spin_lock_bh(&mhi_event->lock); dev_rp = mhi_to_virtual(ev_ring, er_ctxt->rp); - local_rp = ev_ring->rp; - - while (dev_rp != local_rp) { - enum MHI_PKT_TYPE type = MHI_TRE_GET_EV_TYPE(local_rp); - struct tsync_node *tsync_node; - - MHI_VERB("Processing Event:0x%llx 0x%08x 0x%08x\n", - local_rp->ptr, local_rp->dword[0], local_rp->dword[1]); - - MHI_ASSERT(type != MHI_PKT_TYPE_TSYNC_EVENT, "!TSYNC event"); - - int_sequence = MHI_TRE_GET_EV_TSYNC_SEQ(local_rp); - unit = MHI_TRE_GET_EV_TSYNC_UNIT(local_rp); - remote_time = MHI_TRE_GET_EV_TIME(local_rp); - - do { - spin_lock(&mhi_tsync->lock); - tsync_node = list_first_entry_or_null(&mhi_tsync->head, - struct tsync_node, node); - if (!tsync_node) { - spin_unlock(&mhi_tsync->lock); - break; - } - - list_del(&tsync_node->node); - spin_unlock(&mhi_tsync->lock); - - /* - * device may not able to process all time sync commands - * host issue and only process last command it receive - */ - if (tsync_node->int_sequence == int_sequence) { - tsync_node->cb_func(tsync_node->mhi_dev, - tsync_node->sequence, - tsync_node->local_time, - remote_time); - kfree(tsync_node); - } else { - kfree(tsync_node); - } - } while (true); - - mhi_recycle_ev_ring_element(mhi_cntrl, ev_ring); - local_rp = ev_ring->rp; - dev_rp = mhi_to_virtual(ev_ring, er_ctxt->rp); - count++; + if (ev_ring->rp == dev_rp) { + spin_unlock_bh(&mhi_event->lock); + goto exit_tsync_process; } + /* if rp points to base, we need to wrap it around */ + if (dev_rp == ev_ring->base) + dev_rp = ev_ring->base + ev_ring->len; + dev_rp--; + + /* fast forward to currently processed element and recycle er */ + ev_ring->rp = dev_rp; + ev_ring->wp = dev_rp - 1; + if (ev_ring->wp < ev_ring->base) + ev_ring->wp = ev_ring->base + ev_ring->len - ev_ring->el_size; + mhi_recycle_fwd_ev_ring_element(mhi_cntrl, ev_ring); + + MHI_ASSERT(MHI_TRE_GET_EV_TYPE(dev_rp) != MHI_PKT_TYPE_TSYNC_EVENT, + "!TSYNC event"); + + sequence = MHI_TRE_GET_EV_TSYNC_SEQ(dev_rp); + remote_time = MHI_TRE_GET_EV_TIME(dev_rp); + + MHI_VERB("Received TSYNC event with seq:0x%llx time:0x%llx\n", + sequence, remote_time); + read_lock_bh(&mhi_cntrl->pm_lock); if (likely(MHI_DB_ACCESS_VALID(mhi_cntrl))) mhi_ring_er_db(mhi_event); read_unlock_bh(&mhi_cntrl->pm_lock); + spin_unlock_bh(&mhi_event->lock); + mutex_lock(&mhi_cntrl->tsync_mutex); + + if (unlikely(mhi_tsync->int_sequence != sequence)) { + MHI_ASSERT(1, "Unexpected response:0x%llx Expected:0x%llx\n", + sequence, mhi_tsync->int_sequence); + mutex_unlock(&mhi_cntrl->tsync_mutex); + goto exit_tsync_process; + } + + do { + struct tsync_node *tsync_node; + + spin_lock(&mhi_tsync->lock); + tsync_node = list_first_entry_or_null(&mhi_tsync->head, + struct tsync_node, node); + if (!tsync_node) { + spin_unlock(&mhi_tsync->lock); + break; + } + + list_del(&tsync_node->node); + spin_unlock(&mhi_tsync->lock); + + tsync_node->cb_func(tsync_node->mhi_dev, + tsync_node->sequence, + mhi_tsync->local_time, remote_time); + kfree(tsync_node); + } while (true); + + mhi_tsync->db_response_pending = false; + mutex_unlock(&mhi_cntrl->tsync_mutex); + +exit_tsync_process: MHI_VERB("exit er_index:%u\n", mhi_event->er_index); - return count; + return ret; } int mhi_process_bw_scale_ev_ring(struct mhi_controller *mhi_cntrl, @@ -2538,7 +2545,7 @@ int mhi_get_remote_time(struct mhi_device *mhi_dev, struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl; struct mhi_timesync *mhi_tsync = mhi_cntrl->mhi_tsync; struct tsync_node *tsync_node; - int ret; + int ret = 0; /* not all devices support all time features */ mutex_lock(&mhi_cntrl->tsync_mutex); @@ -2562,6 +2569,10 @@ int mhi_get_remote_time(struct mhi_device *mhi_dev, } read_unlock_bh(&mhi_cntrl->pm_lock); + MHI_LOG("Enter with pm_state:%s MHI_STATE:%s\n", + to_mhi_pm_state_str(mhi_cntrl->pm_state), + TO_MHI_STATE_STR(mhi_cntrl->dev_state)); + /* * technically we can use GFP_KERNEL, but wants to avoid * # of times scheduling out @@ -2572,15 +2583,17 @@ int mhi_get_remote_time(struct mhi_device *mhi_dev, goto error_no_mem; } + tsync_node->sequence = sequence; + tsync_node->cb_func = cb_func; + tsync_node->mhi_dev = mhi_dev; + + if (mhi_tsync->db_response_pending) + goto skip_tsync_db; + mhi_tsync->int_sequence++; if (mhi_tsync->int_sequence == 0xFFFFFFFF) mhi_tsync->int_sequence = 0; - tsync_node->sequence = sequence; - tsync_node->int_sequence = mhi_tsync->int_sequence; - tsync_node->cb_func = cb_func; - tsync_node->mhi_dev = mhi_dev; - /* disable link level low power modes */ ret = mhi_cntrl->lpm_disable(mhi_cntrl, mhi_cntrl->priv_data); if (ret) { @@ -2589,10 +2602,6 @@ int mhi_get_remote_time(struct mhi_device *mhi_dev, goto error_invalid_state; } - spin_lock(&mhi_tsync->lock); - list_add_tail(&tsync_node->node, &mhi_tsync->head); - spin_unlock(&mhi_tsync->lock); - /* * time critical code, delay between these two steps should be * deterministic as possible. @@ -2600,9 +2609,9 @@ int mhi_get_remote_time(struct mhi_device *mhi_dev, preempt_disable(); local_irq_disable(); - tsync_node->local_time = + mhi_tsync->local_time = mhi_cntrl->time_get(mhi_cntrl, mhi_cntrl->priv_data); - writel_relaxed_no_log(tsync_node->int_sequence, mhi_cntrl->tsync_db); + writel_relaxed_no_log(mhi_tsync->int_sequence, mhi_cntrl->tsync_db); /* write must go thru immediately */ wmb(); @@ -2611,6 +2620,15 @@ int mhi_get_remote_time(struct mhi_device *mhi_dev, mhi_cntrl->lpm_enable(mhi_cntrl, mhi_cntrl->priv_data); + MHI_VERB("time DB request with seq:0x%llx\n", mhi_tsync->int_sequence); + + mhi_tsync->db_response_pending = true; + +skip_tsync_db: + spin_lock(&mhi_tsync->lock); + list_add_tail(&tsync_node->node, &mhi_tsync->head); + spin_unlock(&mhi_tsync->lock); + ret = 0; error_invalid_state: diff --git a/drivers/bus/mhi/core/mhi_pm.c b/drivers/bus/mhi/core/mhi_pm.c index 40275e3f4439..e8f945b9d799 100644 --- a/drivers/bus/mhi/core/mhi_pm.c +++ b/drivers/bus/mhi/core/mhi_pm.c @@ -823,6 +823,9 @@ void mhi_special_purpose_work(struct work_struct *work) TO_MHI_STATE_STR(mhi_cntrl->dev_state), TO_MHI_EXEC_STR(mhi_cntrl->ee)); + if (unlikely(MHI_EVENT_ACCESS_INVALID(mhi_cntrl->pm_state))) + return; + /* check special purpose event rings and process events */ list_for_each_entry(mhi_event, &mhi_cntrl->sp_ev_rings, node) mhi_event->process_event(mhi_cntrl, mhi_event, U32_MAX); From 419686cc9687ad915234a052662e7abdb20889a0 Mon Sep 17 00:00:00 2001 From: Bhaumik Bhatt Date: Wed, 20 May 2020 18:15:07 -0700 Subject: [PATCH 3/3] mhi: core: Enable both time synchronization methods to co-exist Due to co-existence of both time synchronization methods on a platform, it is likely that a doorbell method response can be pending while another host client requests for time using the MMIO or synchronous method. Wait for completion of the doorbell method and return the local and remote times from that request as the device may not be able to handle both requests or host is likely to end up reading corrupted time values. Also, ensure the doorbell method request is done while holding the bus vote so the host stays awake and receives the response as soon as possible. Change-Id: If876905eb523fd65f3758f13b75c035de4108d8d Signed-off-by: Bhaumik Bhatt --- drivers/bus/mhi/core/mhi_init.c | 3 ++ drivers/bus/mhi/core/mhi_internal.h | 2 + drivers/bus/mhi/core/mhi_main.c | 66 +++++++++++++++++++++-------- 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/drivers/bus/mhi/core/mhi_init.c b/drivers/bus/mhi/core/mhi_init.c index 18d39982d1f6..259a801823db 100644 --- a/drivers/bus/mhi/core/mhi_init.c +++ b/drivers/bus/mhi/core/mhi_init.c @@ -348,6 +348,9 @@ void mhi_destroy_sysfs(struct mhi_controller *mhi_cntrl) } spin_unlock(&mhi_tsync->lock); + if (mhi_tsync->db_response_pending) + complete(&mhi_tsync->db_completion); + kfree(mhi_cntrl->mhi_tsync); mhi_cntrl->mhi_tsync = NULL; mutex_unlock(&mhi_cntrl->tsync_mutex); diff --git a/drivers/bus/mhi/core/mhi_internal.h b/drivers/bus/mhi/core/mhi_internal.h index 42a8b9540ed9..d5148f96794e 100644 --- a/drivers/bus/mhi/core/mhi_internal.h +++ b/drivers/bus/mhi/core/mhi_internal.h @@ -726,8 +726,10 @@ struct mhi_timesync { void __iomem *time_reg; u32 int_sequence; u64 local_time; + u64 remote_time; bool db_support; bool db_response_pending; + struct completion db_completion; spinlock_t lock; /* list protection */ struct list_head head; }; diff --git a/drivers/bus/mhi/core/mhi_main.c b/drivers/bus/mhi/core/mhi_main.c index c9d750a669ef..5e780a028a99 100644 --- a/drivers/bus/mhi/core/mhi_main.c +++ b/drivers/bus/mhi/core/mhi_main.c @@ -1400,6 +1400,10 @@ int mhi_process_tsync_ev_ring(struct mhi_controller *mhi_cntrl, if (unlikely(mhi_tsync->int_sequence != sequence)) { MHI_ASSERT(1, "Unexpected response:0x%llx Expected:0x%llx\n", sequence, mhi_tsync->int_sequence); + + mhi_device_put(mhi_cntrl->mhi_dev, + MHI_VOTE_DEVICE | MHI_VOTE_BUS); + mutex_unlock(&mhi_cntrl->tsync_mutex); goto exit_tsync_process; } @@ -1425,6 +1429,11 @@ int mhi_process_tsync_ev_ring(struct mhi_controller *mhi_cntrl, } while (true); mhi_tsync->db_response_pending = false; + mhi_tsync->remote_time = remote_time; + complete(&mhi_tsync->db_completion); + + mhi_device_put(mhi_cntrl->mhi_dev, MHI_VOTE_DEVICE | MHI_VOTE_BUS); + mutex_unlock(&mhi_cntrl->tsync_mutex); exit_tsync_process: @@ -2469,13 +2478,37 @@ int mhi_get_remote_time_sync(struct mhi_device *mhi_dev, { struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl; struct mhi_timesync *mhi_tsync = mhi_cntrl->mhi_tsync; + u64 local_time; int ret; - mutex_lock(&mhi_cntrl->tsync_mutex); /* not all devices support time features */ - if (!mhi_tsync) { - ret = -EIO; - goto error_unlock; + if (!mhi_tsync) + return -EINVAL; + + if (unlikely(MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state))) { + MHI_ERR("MHI is not in active state, pm_state:%s\n", + to_mhi_pm_state_str(mhi_cntrl->pm_state)); + return -EIO; + } + + mutex_lock(&mhi_cntrl->tsync_mutex); + + /* return times from last async request completion */ + if (mhi_tsync->db_response_pending) { + local_time = mhi_tsync->local_time; + mutex_unlock(&mhi_cntrl->tsync_mutex); + + ret = wait_for_completion_timeout(&mhi_tsync->db_completion, + msecs_to_jiffies(mhi_cntrl->timeout_ms)); + if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state) || !ret) { + MHI_ERR("Pending DB request did not complete, abort\n"); + return -EAGAIN; + } + + *t_host = local_time; + *t_dev = mhi_tsync->remote_time; + + return 0; } /* bring to M0 state */ @@ -2548,14 +2581,13 @@ int mhi_get_remote_time(struct mhi_device *mhi_dev, int ret = 0; /* not all devices support all time features */ - mutex_lock(&mhi_cntrl->tsync_mutex); - if (!mhi_tsync || !mhi_tsync->db_support) { - ret = -EIO; - goto error_unlock; - } + if (!mhi_tsync || !mhi_tsync->db_support) + return -EINVAL; - /* tsync db can only be rung in M0 state */ - ret = __mhi_device_get_sync(mhi_cntrl); + mutex_lock(&mhi_cntrl->tsync_mutex); + + ret = mhi_device_get_sync(mhi_cntrl->mhi_dev, + MHI_VOTE_DEVICE | MHI_VOTE_BUS); if (ret) goto error_unlock; @@ -2623,21 +2655,21 @@ int mhi_get_remote_time(struct mhi_device *mhi_dev, MHI_VERB("time DB request with seq:0x%llx\n", mhi_tsync->int_sequence); mhi_tsync->db_response_pending = true; + init_completion(&mhi_tsync->db_completion); skip_tsync_db: spin_lock(&mhi_tsync->lock); list_add_tail(&tsync_node->node, &mhi_tsync->head); spin_unlock(&mhi_tsync->lock); - ret = 0; + mutex_unlock(&mhi_cntrl->tsync_mutex); + + return 0; error_invalid_state: - if (ret) - kfree(tsync_node); + kfree(tsync_node); error_no_mem: - read_lock_bh(&mhi_cntrl->pm_lock); - mhi_cntrl->wake_put(mhi_cntrl, false); - read_unlock_bh(&mhi_cntrl->pm_lock); + mhi_device_put(mhi_cntrl->mhi_dev, MHI_VOTE_DEVICE | MHI_VOTE_BUS); error_unlock: mutex_unlock(&mhi_cntrl->tsync_mutex); return ret;