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

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

[Can Guo]

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.

Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
---
 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.

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

[Stanley Chu]

Hi Can,

On Fri, 2020-03-27 at 19:27 -0700, Can Guo wrote:
> 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.
> 
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>

Personally I like this patch because the re-initialization performance
is improved in my test platform with my patch "scsi: ufs: set device as
active power mode after resetting device" working together. But I am not
sure if every vendor is happy about this.

Anyway,
Acked-by: Stanley Chu <stanley.chu@mediatek.com>

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

[Bean Huo (beanhuo)]

> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
Reviewed-by: Bean Huo <beanhuo@micron.com>

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

[Martin K. Petersen]

> 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?

-- 
Martin K. Petersen	Oracle Linux Engineering

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

[Avri Altman]

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

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

[Alim Akhtar]

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.

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@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.