On 9/19/2020 7:32 AM, Bhaumik Bhatt wrote:
> Clients on the host may see the device in an active state for a short
> period of time after the host detects a device error or power down.

What scenario is referred as 'device error' here?
And power down is the non-graceful power_down by controller?

> Prevent any further host activity which could lead to race conditions
> or multiple callbacks to the controller or any timeouts seen by
> clients attempting to push data as they must be notified of the host
> intent sooner rather than later. This also allows the host and device
> states to be in sync with one another and prevents unnecessary host
> operations.
>
> Signed-off-by: Bhaumik Bhatt <bbh...@codeaurora.org>
> ---
>  drivers/bus/mhi/core/main.c |  4 ++++
>  drivers/bus/mhi/core/pm.c   | 31 +++++++++++++++++++------------
>  2 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 1c8e332..5ec89e9 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -400,6 +400,10 @@ 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) {
> +             /* host may be performing a device power down already */
> +             if (!mhi_is_active(mhi_cntrl))
> +                     goto exit_intvec;
> +
>               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);
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index 16c04ab..4e2cb41 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -469,15 +469,10 @@ static void mhi_pm_disable_transition(struct 
> mhi_controller *mhi_cntrl,
>       write_lock_irq(&mhi_cntrl->pm_lock);
>       prev_state = mhi_cntrl->pm_state;
>       cur_state = mhi_tryset_pm_state(mhi_cntrl, transition_state);
> -     if (cur_state == transition_state) {
> -             mhi_cntrl->ee = MHI_EE_DISABLE_TRANSITION;
> +     if (cur_state == MHI_PM_SYS_ERR_PROCESS)
>               mhi_cntrl->dev_state = MHI_STATE_RESET;
> -     }
>       write_unlock_irq(&mhi_cntrl->pm_lock);
>  
> -     /* Wake up threads waiting for state transition */
> -     wake_up_all(&mhi_cntrl->state_event);
> -
>       if (cur_state != transition_state) {
>               dev_err(dev, "Failed to transition to state: %s from: %s\n",
>                       to_mhi_pm_state_str(transition_state),
> @@ -486,6 +481,11 @@ static void mhi_pm_disable_transition(struct 
> mhi_controller *mhi_cntrl,
>               return;
>       }
>  
> +     mhi_cntrl->ee = MHI_EE_DISABLE_TRANSITION;
> +
> +     /* Wake up threads waiting for state transition */
> +     wake_up_all(&mhi_cntrl->state_event);
> +
>       /* Trigger MHI RESET so that the device will not access host memory */
>       if (MHI_REG_ACCESS_VALID(prev_state)) {
>               u32 in_reset = -1;
> @@ -1051,22 +1051,29 @@ void mhi_power_down(struct mhi_controller *mhi_cntrl, 
> bool graceful)
>       enum dev_st_transition next_state = DEV_ST_TRANSITION_DISABLE;
>       struct device *dev = &mhi_cntrl->mhi_dev->dev;
>  
> +     mutex_lock(&mhi_cntrl->pm_mutex);
> +     write_lock_irq(&mhi_cntrl->pm_lock);
> +
>       /* If it's not a graceful shutdown, force MHI to linkdown state */
>       if (!graceful) {
> -             mutex_lock(&mhi_cntrl->pm_mutex);
> -             write_lock_irq(&mhi_cntrl->pm_lock);
>               cur_state = mhi_tryset_pm_state(mhi_cntrl,
>                                               MHI_PM_LD_ERR_FATAL_DETECT);
> -             write_unlock_irq(&mhi_cntrl->pm_lock);
> -             mutex_unlock(&mhi_cntrl->pm_mutex);
> -             if (cur_state != MHI_PM_LD_ERR_FATAL_DETECT)
> +             if (cur_state != MHI_PM_LD_ERR_FATAL_DETECT) {
>                       dev_dbg(dev, "Failed to move to state: %s from: %s\n",
>                               to_mhi_pm_state_str(MHI_PM_LD_ERR_FATAL_DETECT),
>                               to_mhi_pm_state_str(mhi_cntrl->pm_state));
> -             else
> +             } else {
>                       next_state = DEV_ST_TRANSITION_FATAL;
> +                     wake_up_all(&mhi_cntrl->state_event);
> +             }
>       }
>  
> +     /* mark device inactive to avoid any further host processing */
> +     mhi_cntrl->dev_state = MHI_STATE_RESET;
> +
> +     write_unlock_irq(&mhi_cntrl->pm_lock);
> +     mutex_unlock(&mhi_cntrl->pm_mutex);
> +
>       mhi_queue_state_transition(mhi_cntrl, next_state);
>  
>       /* Wait for shutdown to complete */

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Reply via email to