Re: [External Mail]Re: [PATCH] ufs: core: fix bus timeout in ufshcd_wl_resume flow

[章辉 <zhanghui31@xiaomi.com>]

On 2024/8/14 9:41, Bart Van Assche wrote:
> On 8/13/24 6:47 AM, ZhangHui wrote:
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 5e3c67e96956..e5e3e0277d43 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -3291,6 +3291,8 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba 
>> *hba,
>>       struct ufshcd_lrb *lrbp = &hba->lrb[tag];
>>       int err;
>>
>> +     if (hba->ufshcd_reg_state == UFSHCD_REG_RESET)
>> +             return -EBUSY;
>>       /* Protects use of hba->reserved_slot. */
>>       lockdep_assert_held(&hba->dev_cmd.lock);
>
> Does this change make ufshcd_exec_dev_cmd() unpredictable - it succeeds
> if the controller is in the normal state and fails if error recovery
> is ongoing? If so, which code paths does this affect and/or break?
>
> Additionally, I think the above check is racy. hba->ufshcd_reg_state may
> change after the above code checked it and before ufshcd_exec_dev_cmd()
> has finished. Wouldn't it be better to make code that shouldn't be
> executed while the error handler is ongoing wait until error handling
> has finished?
>
> Thanks,
>
> Bart.
>
hi Bart,

1. If the host needs to send a dev command, the HBA must be enabled.
We have set the ufshcd_reg_state to operational in the ufshcd_hba_enable,
so it is not unpredictable.

2. That's a good question, but I think it makes sense to block dev cmd while
ufs is doing a reset.


Thanks
Zhanghui