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
