Messages in this thread | | | Date | Fri, 15 Nov 2019 09:33:22 +0800 | From | Can Guo <> | Subject | Re: [PATCH v4 7/7] scsi: ufs: Fix error handing during hibern8 enter |
| |
On 2019-11-14 19:06, Avri Altman wrote: >> >> >> From: Subhash Jadavani <subhashj@codeaurora.org> >> >> During clock gating (ufshcd_gate_work()), we first put the link >> hibern8 by >> calling ufshcd_uic_hibern8_enter() and if ufshcd_uic_hibern8_enter() >> returns >> success (0) then we gate all the clocks. >> Now let’s zoom in to what ufshcd_uic_hibern8_enter() does internally: >> It calls __ufshcd_uic_hibern8_enter() which on detecting the >> LINERESET, >> initiates the full recovery and puts the link back to highest HS gear >> and returns >> success (0) to ufshcd_uic_hibern8_enter() which is the issue as link >> is still in >> active state due to recovery! >> Now ufshcd_uic_hibern8_enter() returns success to ufshcd_gate_work() >> and >> hence it goes ahead with gating the UFS clock while link is still in >> active state >> hence I believe controller would raise UIC error interrupts. But when >> we service >> the interrupt, clocks might have already been disabled! >> >> This change fixes for this by returning failure from >> __ufshcd_uic_hibern8_enter() if recovery succeeds as link is still not >> in hibern8, >> upon receiving the error ufshcd_hibern8_enter() would initiate retry >> to put the >> link state back into hibern8. >> >> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> --- >> drivers/scsi/ufs/ufshcd.c | 19 ++++++++++++++----- >> 1 file changed, 14 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index >> 7a5a904..934c27a 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -3891,15 +3891,24 @@ static int __ufshcd_uic_hibern8_enter(struct >> ufs_hba *hba) >> ktime_to_us(ktime_sub(ktime_get(), >> start)), ret); >> >> if (ret) { >> + int err; >> + >> dev_err(hba->dev, "%s: hibern8 enter failed. ret = >> %d\n", >> __func__, ret); >> >> /* >> - * If link recovery fails then return error so that >> caller >> - * don't retry the hibern8 enter again. >> + * If link recovery fails then return error code >> (-ENOLINK) >> + * returned ufshcd_link_recovery(). >> + * If link recovery succeeds then return -EAGAIN to >> attempt >> + * hibern8 enter retry again. > You no longer returning -ENOLINK, and either way retrying, regardless > of the error code. > Better check that the commit log is still telling the correct story, > taking into consideration all those recent fixes and all. >
Thanks for pointing this.
Best Regards, Can Guo.
>> */ >> - if (ufshcd_link_recovery(hba)) >> - ret = -ENOLINK; >> + err = ufshcd_link_recovery(hba); >> + if (err) { >> + dev_err(hba->dev, "%s: link recovery failed", >> __func__); >> + ret = err; >> + } else { >> + ret = -EAGAIN; >> + } >> } else >> ufshcd_vops_hibern8_notify(hba, >> UIC_CMD_DME_HIBER_ENTER, >> >> POST_CHANGE); @@ -3913,7 +3922,7 @@ >> static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba) >> >> for (retries = UIC_HIBERN8_ENTER_RETRIES; retries > 0; >> retries--) { >> ret = __ufshcd_uic_hibern8_enter(hba); >> - if (!ret || ret == -ENOLINK) >> + if (!ret) >> goto out; >> } >> out: >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project
| |