Re: [PATCH v1 07/10] bus: mhi: core: Move to SYS_ERROR regardless of RDDM capability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux