lkml.org 
[lkml]   [2020]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 04/12] bus: mhi: core: Move to SYS_ERROR regardless of RDDM capability
Hi Mani,

On 2020-10-30 06:52, Manivannan Sadhasivam wrote:
> On Thu, Oct 29, 2020 at 09:10:49PM -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 and a
>> SYS_ERROR state change event has already arrived.
>
> I don't quite understand this statement. Can you elaborate? This
> doesn't
> relate to what the patch is doing.
>
So the mhi_intvec_threaded_handler() (BHI) MSI that fires to switch the
EE
to RDDM may come much later than the SYS_ERROR state change event from
the
control event ring. We currently, do not move to MHI_PM_SYS_ERR_DETECT
state if RDDM is supported i.e. mhi_cntrl->rddm_image is set. However,
it
means that we will remain in an "active" MHI PM state for the duration
of
time until RDDM EE (BHI) MSI comes in. We have seen it take 5 seconds in
some bad cases.
>> This can result
>> in scenarios where MHI controller and client drivers are unaware
>> of the error state of the device. Remove the check for rddm_image
>> when processing the SYS_ERROR state change as it is present in
>> mhi_pm_sys_err_handler() already and prevent further activity
>> until the expected RDDM execution environment change occurs or
>> the controller driver decides further action.
>>
>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> ---
>> drivers/bus/mhi/core/main.c | 12 ++++--------
>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
>> index 2cff5dd..1f32d67 100644
>> --- a/drivers/bus/mhi/core/main.c
>> +++ b/drivers/bus/mhi/core/main.c
>> @@ -733,19 +733,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)
>
> But you are still checking for RDDM EE?
>
> Please explain why you want to skip RDDM check.
>
> Thanks,
> Mani
>
Yes, the point is to only remove the mhi_cntrl->rddm_image check but
still
retain the "has EE moved to become RDDM" check. This allows us to avoid
any
extra processing of moving states to MHI_PM_SYS_ERR_DETECT state if it
may
be unnecessary (EE already changed to RDDM). The mhi_cntrl->rddm_image
is
also present in mhi_pm_sys_err_handler(mhi_cntrl) function so it is not
needed here.
>> + 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
>>

This is why I mention the word RDDM "capability". If controller supports
RDDM
is not enough to skip the move to MHI_PM_SYS_ERR_DETECT state as it is
safer
to move and stop client drivers from pushing data.

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

\
 
 \ /
  Last update: 2020-10-30 20:31    [W:0.083 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site