On Fri, Sep 18, 2020 at 07:02:35PM -0700, Bhaumik Bhatt wrote:
> Clients on the host may see the device in an active state for a short
Clients? Are you referring client drivers or controllers? Please don't mix
the conventions.
> period of time after the host detects a device error or power down.
> 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.
>
How the change of dev_state is visible to the so called "client"?
> Signed-off-by: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
> ---
> 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;
> +
Does this change belong to this patch?
Thanks,
Mani
> 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
>
