2020-05-07 16:23:13

by Jeffrey Hugo

Subject: [PATCH v4] bus: mhi: core: Handle syserr during power_up

The MHI device may be in the syserr state when we attempt to init it in
power_up(). Since we have no local state, the handling is simple -
reset the device and wait for it to transition out of the reset state.

Signed-off-by: Jeffrey Hugo <[email protected]>
---

v4:
-Implemented Hemant's suggested solution from v2. The spec will be amended
to indicate the intvec interrupt will be triggered for the reset state change
which matches the current implementations and enables Hemant's solution to
be used, which is cleaner.

drivers/bus/mhi/core/pm.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)

diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index dc83d65..ddef693 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -760,6 +760,7 @@ static void mhi_deassert_dev_wake(struct mhi_controller *mhi_cntrl,

int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
{
+ enum mhi_state state;
enum mhi_ee_type current_ee;
enum dev_st_transition next_state;
struct device *dev = &mhi_cntrl->mhi_dev->dev;
@@ -829,6 +830,32 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
goto error_bhi_offset;
}

+ state = mhi_get_mhi_state(mhi_cntrl);
+ if (state == MHI_STATE_SYS_ERR) {
+ mhi_set_mhi_state(mhi_cntrl, MHI_STATE_RESET);
+ ret = wait_event_timeout(mhi_cntrl->state_event,
+ MHI_PM_IN_FATAL_STATE(mhi_cntrl->pm_state) ||
+ mhi_read_reg_field(mhi_cntrl,
+ mhi_cntrl->regs,
+ MHICTRL,
+ MHICTRL_RESET_MASK,
+ MHICTRL_RESET_SHIFT,
+ &val) ||
+ !val,
+ msecs_to_jiffies(mhi_cntrl->timeout_ms));
+ if (ret) {
+ ret = -EIO;
+ dev_info(dev, "Failed to reset MHI due to syserr state\n");
+ goto error_bhi_offset;
+ }
+
+ /*
+ * device cleares INTVEC as part of RESET processing,
+ * re-program it
+ */
+ mhi_write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0);
+ }
+
/* Transition to next state */
next_state = MHI_IN_PBL(current_ee) ?
DEV_ST_TRANSITION_PBL : DEV_ST_TRANSITION_READY;
--
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

2020-05-12 01:55:02

by Hemant Kumar

Subject: Re: [PATCH v4] bus: mhi: core: Handle syserr during power_up

On 5/7/20 9:19 AM, Jeffrey Hugo wrote:
> The MHI device may be in the syserr state when we attempt to init it in
> power_up(). Since we have no local state, the handling is simple -
> reset the device and wait for it to transition out of the reset state.
>
> Signed-off-by: Jeffrey Hugo <[email protected]>

Reviewed-by: Hemant Kumar <[email protected]>

2020-05-14 05:25:41

by Manivannan Sadhasivam

Subject: Re: [PATCH v4] bus: mhi: core: Handle syserr during power_up

On Thu, May 07, 2020 at 10:19:31AM -0600, Jeffrey Hugo wrote:
> The MHI device may be in the syserr state when we attempt to init it in
> power_up(). Since we have no local state, the handling is simple -
> reset the device and wait for it to transition out of the reset state.
>
> Signed-off-by: Jeffrey Hugo <[email protected]>

Reviewed-by: Manivannan Sadhasivam <[email protected]>

Will apply this patch to mhi-next and include in the 5.8 series.

Thanks,
Mani

> ---
>
> v4:
> -Implemented Hemant's suggested solution from v2. The spec will be amended
> to indicate the intvec interrupt will be triggered for the reset state change
> which matches the current implementations and enables Hemant's solution to
> be used, which is cleaner.
>
> drivers/bus/mhi/core/pm.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index dc83d65..ddef693 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -760,6 +760,7 @@ static void mhi_deassert_dev_wake(struct mhi_controller *mhi_cntrl,
>
> int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
> {
> + enum mhi_state state;
> enum mhi_ee_type current_ee;
> enum dev_st_transition next_state;
> struct device *dev = &mhi_cntrl->mhi_dev->dev;
> @@ -829,6 +830,32 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
> goto error_bhi_offset;
> }
>
> + state = mhi_get_mhi_state(mhi_cntrl);
> + if (state == MHI_STATE_SYS_ERR) {
> + mhi_set_mhi_state(mhi_cntrl, MHI_STATE_RESET);
> + ret = wait_event_timeout(mhi_cntrl->state_event,
> + MHI_PM_IN_FATAL_STATE(mhi_cntrl->pm_state) ||
> + mhi_read_reg_field(mhi_cntrl,
> + mhi_cntrl->regs,
> + MHICTRL,
> + MHICTRL_RESET_MASK,
> + MHICTRL_RESET_SHIFT,
> + &val) ||
> + !val,
> + msecs_to_jiffies(mhi_cntrl->timeout_ms));
> + if (ret) {
> + ret = -EIO;
> + dev_info(dev, "Failed to reset MHI due to syserr state\n");
> + goto error_bhi_offset;
> + }
> +
> + /*
> + * device cleares INTVEC as part of RESET processing,
> + * re-program it
> + */
> + mhi_write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0);
> + }
> +
> /* Transition to next state */
> next_state = MHI_IN_PBL(current_ee) ?
> DEV_ST_TRANSITION_PBL : DEV_ST_TRANSITION_READY;
> --
> Qualcomm Technologies, Inc. is a member of the
> Code Aurora Forum, a Linux Foundation Collaborative Project.