| Message ID | 1585362454-5413-1-git-send-email-cang@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1,1/1] scsi: ufs: full reinit upon resume if link was off | expand |
> During suspend, if the link is put to off, it would require a full > initialization during resume. This patch resets and restores both the > hba and the card during initialization. Avri, Alim: Any opinions on this change in behavior wrt. your controllers? Would a quirk or callback be preferred to changing this for everyone?
Hi Can, > -----Original Message----- > From: Can Guo <cang@codeaurora.org> > Sent: 28 March 2020 07:58 > To: asutoshd@codeaurora.org; nguyenb@codeaurora.org; > hongwus@codeaurora.org; rnayak@codeaurora.org; linux- > scsi@vger.kernel.org; kernel-team@android.com; saravanak@google.com; > salyzyn@google.com; cang@codeaurora.org > Cc: Alim Akhtar <alim.akhtar@samsung.com>; Avri Altman > <avri.altman@wdc.com>; James E.J. Bottomley <jejb@linux.ibm.com>; Martin > K. Petersen <martin.petersen@oracle.com>; Stanley Chu > <stanley.chu@mediatek.com>; Bean Huo <beanhuo@micron.com>; Bart Van > Assche <bvanassche@acm.org>; Venkat Gopalakrishnan > <venkatg@codeaurora.org>; Tomas Winkler <tomas.winkler@intel.com>; open > list <linux-kernel@vger.kernel.org> > Subject: [PATCH v1 1/1] scsi: ufs: full reinit upon resume if link was off > > From: Asutosh Das <asutoshd@codeaurora.org> > > During suspend, if the link is put to off, it would require a full initialization during > resume. This patch resets and restores both the hba and the card during > initialization. > In case you have faced issues by not doing what this patch does, it is worth mentioning that in the commit mesg. > Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> > Signed-off-by: Can Guo <cang@codeaurora.org> > --- I don't have a way to test this path as of now, changes looks ok though. Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com> > drivers/scsi/ufs/ufshcd.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index > f19a11e..21e41e5 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -8007,9 +8007,13 @@ static int ufshcd_resume(struct ufs_hba *hba, enum > ufs_pm_op pm_op) > else > goto vendor_suspend; > } else if (ufshcd_is_link_off(hba)) { > - ret = ufshcd_host_reset_and_restore(hba); > /* > - * ufshcd_host_reset_and_restore() should have already > + * A full initialization of the host and the device is required > + * since the link was put to off during suspend. > + */ > + ret = ufshcd_reset_and_restore(hba); > + /* > + * ufshcd_reset_and_restore() should have already > * set the link state as active > */ > if (ret || !ufshcd_is_link_active(hba)) > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux > Foundation Collaborative Project.
Hi Alim, On 2020-04-14 10:30, Alim Akhtar wrote: > Hi Can, > >> -----Original Message----- >> From: Can Guo <cang@codeaurora.org> >> Sent: 28 March 2020 07:58 >> To: asutoshd@codeaurora.org; nguyenb@codeaurora.org; >> hongwus@codeaurora.org; rnayak@codeaurora.org; linux- >> scsi@vger.kernel.org; kernel-team@android.com; saravanak@google.com; >> salyzyn@google.com; cang@codeaurora.org >> Cc: Alim Akhtar <alim.akhtar@samsung.com>; Avri Altman >> <avri.altman@wdc.com>; James E.J. Bottomley <jejb@linux.ibm.com>; >> Martin >> K. Petersen <martin.petersen@oracle.com>; Stanley Chu >> <stanley.chu@mediatek.com>; Bean Huo <beanhuo@micron.com>; Bart Van >> Assche <bvanassche@acm.org>; Venkat Gopalakrishnan >> <venkatg@codeaurora.org>; Tomas Winkler <tomas.winkler@intel.com>; >> open >> list <linux-kernel@vger.kernel.org> >> Subject: [PATCH v1 1/1] scsi: ufs: full reinit upon resume if link was >> off >> >> From: Asutosh Das <asutoshd@codeaurora.org> >> >> During suspend, if the link is put to off, it would require a full > initialization during Good catch. >> resume. This patch resets and restores both the hba and the card >> during >> initialization. >> > In case you have faced issues by not doing what this patch does, it is > worth > mentioning that in the commit mesg. > OK. >> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> --- > I don't have a way to test this path as of now, changes looks ok > though. > Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com> > >> drivers/scsi/ufs/ufshcd.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index >> f19a11e..21e41e5 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -8007,9 +8007,13 @@ static int ufshcd_resume(struct ufs_hba *hba, >> enum >> ufs_pm_op pm_op) >> else >> goto vendor_suspend; >> } else if (ufshcd_is_link_off(hba)) { >> - ret = ufshcd_host_reset_and_restore(hba); >> /* >> - * ufshcd_host_reset_and_restore() should have already >> + * A full initialization of the host and the device is > required Shall fix. >> + * since the link was put to off during suspend. >> + */ >> + ret = ufshcd_reset_and_restore(hba); >> + /* >> + * ufshcd_reset_and_restore() should have already >> * set the link state as active >> */ >> if (ret || !ufshcd_is_link_active(hba)) >> -- >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a >> Linux >> Foundation Collaborative Project. Thanks. Can Guo.
Hi, > > > > During suspend, if the link is put to off, it would require a full > > initialization during resume. This patch resets and restores both the > > hba and the card during initialization. > > Avri, Alim: Any opinions on this change in behavior wrt. your > controllers? Would a quirk or callback be preferred to changing this for > everyone? You have a v2 of this patch and a Reviewed-by tag by Alim. It looks fine to me as well. Thanks, Avri > > -- > Martin K. Petersen Oracle Linux Engineering
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index f19a11e..21e41e5 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -8007,9 +8007,13 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) else goto vendor_suspend; } else if (ufshcd_is_link_off(hba)) { - ret = ufshcd_host_reset_and_restore(hba); /* - * ufshcd_host_reset_and_restore() should have already + * A full initialization of the host and the device is required + * since the link was put to off during suspend. + */ + ret = ufshcd_reset_and_restore(hba); + /* + * ufshcd_reset_and_restore() should have already * set the link state as active */ if (ret || !ufshcd_is_link_active(hba))