Messages in this thread | | | Subject | Re: [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller | From | Mark Salyzyn <> | Date | Thu, 31 Oct 2019 07:44:29 -0700 |
| |
On 10/22/19 9:13 PM, Can Guo wrote: > Some UFS host controllers need their specific implementations of resetting > to get them into a good state. Provide a new vops to allow the platform > driver to implement this own reset operation. > > Signed-off-by: Can Guo <cang@codeaurora.org> > --- > drivers/scsi/ufs/ufshcd.c | 16 ++++++++++++++++ > drivers/scsi/ufs/ufshcd.h | 10 ++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index c28c144..161e3c4 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -3859,6 +3859,14 @@ static int ufshcd_link_recovery(struct ufs_hba *hba) > ufshcd_set_eh_in_progress(hba); > spin_unlock_irqrestore(hba->host->host_lock, flags); > > + ret = ufshcd_vops_full_reset(hba); > + if (ret) > + dev_warn(hba->dev, "%s: full reset returned %d\n", > + __func__, ret); > + > + /* Reset the attached device */ > + ufshcd_vops_device_reset(hba); > + > ret = ufshcd_host_reset_and_restore(hba); > > spin_lock_irqsave(hba->host->host_lock, flags);
In all your cases, especially after this adjustment, ufshcd_vops_full_reset is called blindly (+error checking message) before ufshcd_vops_device_reset. What about dropping the .full_reset (should really have been called .hw_reset or .host_reset) addition to the vops, just adding ufshcd_vops_device_reset call here before ufshcd_host_reset_and_restore, and in the driver folding the ufshcd_vops_full_reset code into the .device_reset handler?
Would that be workable? It would be simpler if so.
I can see a desire for the heads up (ufshcd_vops_full_reset+)ufshcd_vops_device_reset calls before ufshcd_host_reset_and_restore because that function will spin 10 seconds waiting for a response from a standardized register, that itself could be hardware locked up requiring product specific reset procedures. But if that is the case, then what about all the other calls to ufshcd_host_reset_and_restore in this file that are not provided the heads up? My guess is that the host device only demonstrated issues in the ufshcd_link_recovery handling path? Are you sure this is the only path that tickles the controller into a hardware lockup state?
Sincerely -- Mark Salyzyn
| |