Re: [External Mail]Re: [PATCH] ufs: core: fix bus timeout in ufshcd_wl_resume flow
From: 章辉
Date: Thu Aug 15 2024 - 09:07:55 EST
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