2019-10-24 11:42:43

by Asutosh Das (asd)

[permalink] [raw]
Subject: [PATCH v3 2/2] scsi: ufs-qcom: enter and exit hibern8 during clock scaling

Qualcomm controller needs to be in hibern8 before scaling clocks.
This change puts the controller in hibern8 state before scaling
and brings it out after scaling of clocks.

Signed-off-by: Asutosh Das <[email protected]>
---
drivers/scsi/ufs/ufs-qcom.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index a5b7148..55b1de5 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1305,18 +1305,27 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
int err = 0;

if (status == PRE_CHANGE) {
+ err = ufshcd_uic_hibern8_enter(hba);
+ if (err)
+ return err;
if (scale_up)
err = ufs_qcom_clk_scale_up_pre_change(hba);
else
err = ufs_qcom_clk_scale_down_pre_change(hba);
+ if (err)
+ ufshcd_uic_hibern8_exit(hba);
+
} else {
if (scale_up)
err = ufs_qcom_clk_scale_up_post_change(hba);
else
err = ufs_qcom_clk_scale_down_post_change(hba);

- if (err || !dev_req_params)
+
+ if (err || !dev_req_params) {
+ ufshcd_uic_hibern8_exit(hba);
goto out;
+ }

ufs_qcom_cfg_timers(hba,
dev_req_params->gear_rx,
@@ -1324,6 +1333,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
dev_req_params->hs_rate,
false);
ufs_qcom_update_bus_bw_vote(host);
+ ufshcd_uic_hibern8_exit(hba);
}

out:
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.


2019-11-11 15:57:09

by Pedro Sousa

[permalink] [raw]
Subject: RE: [PATCH v3 2/2] scsi: ufs-qcom: enter and exit hibern8 during clock scaling

Hi Asutosh,

Please check comments.

-----Original Message-----
From: Asutosh Das <[email protected]>
Sent: Wednesday, October 23, 2019 5:49 PM
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Asutosh Das <[email protected]>; Andy Gross <[email protected]>; Alim Akhtar <[email protected]>; Avri Altman <[email protected]>; Pedro Sousa <[email protected]>; James E.J. Bottomley <[email protected]>; open list:ARM/QUALCOMM SUPPORT <[email protected]>; open list <[email protected]>
Subject: [PATCH v3 2/2] scsi: ufs-qcom: enter and exit hibern8 during clock scaling

Qualcomm controller needs to be in hibern8 before scaling clocks.
This change puts the controller in hibern8 state before scaling
and brings it out after scaling of clocks.

Signed-off-by: Asutosh Das <[email protected]>
---
drivers/scsi/ufs/ufs-qcom.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index a5b7148..55b1de5 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1305,18 +1305,27 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
int err = 0;

if (status == PRE_CHANGE) {
+ err = ufshcd_uic_hibern8_enter(hba);
+ if (err)
+ return err;
if (scale_up)
err = ufs_qcom_clk_scale_up_pre_change(hba);
else
err = ufs_qcom_clk_scale_down_pre_change(hba);
+ if (err)
+ ufshcd_uic_hibern8_exit(hba);
+
} else {
if (scale_up)
err = ufs_qcom_clk_scale_up_post_change(hba);
else
err = ufs_qcom_clk_scale_down_post_change(hba);

- if (err || !dev_req_params)
+
+ if (err || !dev_req_params) {
+ ufshcd_uic_hibern8_exit(hba);
goto out;
+ }

ufs_qcom_cfg_timers(hba,
dev_req_params->gear_rx,
@@ -1324,6 +1333,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
dev_req_params->hs_rate,
false);
ufs_qcom_update_bus_bw_vote(host);
+ ufshcd_uic_hibern8_exit(hba);

Here you are creating the possibility of returning a success even if hibern8 exit fails.
If hibern8 exit fails the ufs recovery will be triggered and "err" variable will not get updated
in this function, how is this handled? Did you tested this possibility?

}

out:
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

This is just a doubt, it might make sense in this use case, please give me your thoughts.

Thank you,
Pedro Sousa



2020-05-20 16:57:40

by Asutosh Das (asd)

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] scsi: ufs-qcom: enter and exit hibern8 during clock scaling

Hi Pedro,

On 11/11/2019 7:54 AM, Pedro Sousa wrote:
> Hi Asutosh,
>
> Please check comments.
Sorry for missing out on this and thanks for your review.

>
> -----Original Message-----
> From: Asutosh Das <[email protected]>
> Sent: Wednesday, October 23, 2019 5:49 PM
> To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; Asutosh Das <[email protected]>; Andy Gross <[email protected]>; Alim Akhtar <[email protected]>; Avri Altman <[email protected]>; Pedro Sousa <[email protected]>; James E.J. Bottomley <[email protected]>; open list:ARM/QUALCOMM SUPPORT <[email protected]>; open list <[email protected]>
> Subject: [PATCH v3 2/2] scsi: ufs-qcom: enter and exit hibern8 during clock scaling
>
> Qualcomm controller needs to be in hibern8 before scaling clocks.
> This change puts the controller in hibern8 state before scaling
> and brings it out after scaling of clocks.
>
> Signed-off-by: Asutosh Das <[email protected]>
> ---
> drivers/scsi/ufs/ufs-qcom.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index a5b7148..55b1de5 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -1305,18 +1305,27 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
> int err = 0;
>
> if (status == PRE_CHANGE) {
> + err = ufshcd_uic_hibern8_enter(hba);
> + if (err)
> + return err;
> if (scale_up)
> err = ufs_qcom_clk_scale_up_pre_change(hba);
> else
> err = ufs_qcom_clk_scale_down_pre_change(hba);
> + if (err)
> + ufshcd_uic_hibern8_exit(hba);
> +
> } else {
> if (scale_up)
> err = ufs_qcom_clk_scale_up_post_change(hba);
> else
> err = ufs_qcom_clk_scale_down_post_change(hba);
>
> - if (err || !dev_req_params)
> +
> + if (err || !dev_req_params) {
> + ufshcd_uic_hibern8_exit(hba);
> goto out;
> + }
>
> ufs_qcom_cfg_timers(hba,
> dev_req_params->gear_rx,
> @@ -1324,6 +1333,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
> dev_req_params->hs_rate,
> false);
> ufs_qcom_update_bus_bw_vote(host);
> + ufshcd_uic_hibern8_exit(hba);
>
> Here you are creating the possibility of returning a success even if hibern8 exit fails.
> If hibern8 exit fails the ufs recovery will be triggered and "err" variable will not get updated
> in this function, how is this handled? Did you tested this possibility?
>
> }
>
> out:
>


Yes - I agree with your comment. The error should be propagated from
this function correctly to the caller. I'll push another version.

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project