On 2020-10-09 09:32, Manivannan Sadhasivam wrote:
On Fri, Sep 18, 2020 at 07:02:32PM -0700, Bhaumik Bhatt wrote:
In some cases, the entry of device to RDDM execution environment
can occur after a significant amount of time has elapsed after the
SYS_ERROR state change event has arrived. This can result in scenarios
where users of the MHI bus are unaware of the error state of the
Who are all the users of MHI bus? Client drivers?
Both client and controller drivers. I will change it to that.
device. Hence, moving the MHI bus to a SYS_ERROR detected state will
prevent further client activity and wait for the RDDM entry.
Signed-off-by: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
---
drivers/bus/mhi/core/main.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 2cff5dd..1c8e332 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -376,6 +376,7 @@ irqreturn_t mhi_intvec_threaded_handler(int
irq_number, void *priv)
enum mhi_state state = MHI_STATE_MAX;
enum mhi_pm_state pm_state = 0;
enum mhi_ee_type ee = 0;
+ bool handle_rddm = false;
write_lock_irq(&mhi_cntrl->pm_lock);
if (!MHI_REG_ACCESS_VALID(mhi_cntrl->pm_state)) {
@@ -400,6 +401,17 @@ irqreturn_t mhi_intvec_threaded_handler(int
irq_number, void *priv)
/* If device supports RDDM don't bother processing SYS error */
if (mhi_cntrl->rddm_image) {
if (mhi_cntrl->ee == MHI_EE_RDDM && mhi_cntrl->ee != ee) {
+ /* prevent clients from queueing any more packets */
+ write_lock_irq(&mhi_cntrl->pm_lock);
+ pm_state = mhi_tryset_pm_state(mhi_cntrl,
+ MHI_PM_SYS_ERR_DETECT);
The condition above already moves MHI to MHI_PM_SYS_ERR_DETECT if the
state
is MHI_STATE_SYS_ERR. Why are you doing it here again?
Thanks,
Mani
I added it there because any first move to RDDM required the MHI host to
be
inactive or in an "error" state.
However, upon further thought, I have made changes that negate this need
and
instead make the if (mhi_cntrl->rddm_image) check dependent on the
pm_state being
MHI_PM_SYS_ERR_DETECT.
Reason is: a first move RDDM comes after a SYS_ERROR in MHI state, since
PM state
will already by SYS_ERROR detect by then, no client drivers or
controllers will be able
to use the bus.
+ if (pm_state == MHI_PM_SYS_ERR_DETECT)
+ handle_rddm = true;
+ write_unlock_irq(&mhi_cntrl->pm_lock);
+ }
+
+ if (handle_rddm) {
+ dev_err(dev, "RDDM event occurred!\n");
mhi_cntrl->status_cb(mhi_cntrl, MHI_CB_EE_RDDM);
wake_up_all(&mhi_cntrl->state_event);
}
@@ -733,19 +745,15 @@ int mhi_process_ctrl_ev_ring(struct
mhi_controller *mhi_cntrl,
break;
case MHI_STATE_SYS_ERR:
{
- enum mhi_pm_state new_state;
-
- /* skip SYS_ERROR handling if RDDM supported */
- if (mhi_cntrl->ee == MHI_EE_RDDM ||
- mhi_cntrl->rddm_image)
- break;
+ enum mhi_pm_state state = MHI_PM_STATE_MAX;
dev_dbg(dev, "System error detected\n");
write_lock_irq(&mhi_cntrl->pm_lock);
- new_state = mhi_tryset_pm_state(mhi_cntrl,
+ if (mhi_cntrl->ee != MHI_EE_RDDM)
+ state = mhi_tryset_pm_state(mhi_cntrl,
MHI_PM_SYS_ERR_DETECT);
write_unlock_irq(&mhi_cntrl->pm_lock);
- if (new_state == MHI_PM_SYS_ERR_DETECT)
+ if (state == MHI_PM_SYS_ERR_DETECT)
mhi_pm_sys_err_handler(mhi_cntrl);
break;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project