Re: [PATCH v1 1/1] scsi: ufs: full reinit upon resume if link was off

[Can Guo]

Hi Alim,

On 2020-04-14 10:30, Alim Akhtar wrote:
> Hi Can,
>
> > -----Original Message-----
> > From: Can Guo <cang@xxxxxxxxxxxxxx>
> > Sent: 28 March 2020 07:58
> > To: asutoshd@xxxxxxxxxxxxxx; nguyenb@xxxxxxxxxxxxxx;
> > hongwus@xxxxxxxxxxxxxx; rnayak@xxxxxxxxxxxxxx; linux-
> > scsi@xxxxxxxxxxxxxxx; kernel-team@xxxxxxxxxxx; saravanak@xxxxxxxxxx;
> > salyzyn@xxxxxxxxxx; cang@xxxxxxxxxxxxxx
> > Cc: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>; Avri Altman
> > <avri.altman@xxxxxxx>; James E.J. Bottomley <jejb@xxxxxxxxxxxxx>; 
> > Martin
> > K. Petersen <martin.petersen@xxxxxxxxxx>; Stanley Chu
> > <stanley.chu@xxxxxxxxxxxx>; Bean Huo <beanhuo@xxxxxxxxxx>; Bart Van
> > Assche <bvanassche@xxxxxxx>; Venkat Gopalakrishnan
> > <venkatg@xxxxxxxxxxxxxx>; Tomas Winkler <tomas.winkler@xxxxxxxxx>; 
> > open
> > list <linux-kernel@xxxxxxxxxxxxxxx>
> > Subject: [PATCH v1 1/1] scsi: ufs: full reinit upon resume if link was 
> > off
> >
> > From: Asutosh Das <asutoshd@xxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxx>
> > Signed-off-by: Can Guo <cang@xxxxxxxxxxxxxx>
> > ---
> I don't have a way to test this path as of now, changes looks ok 
> though.
> Reviewed-by: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
>
> >  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.