diff mbox series

[v9,3/7] scsi: ufs-qcom: Adjust bus bandwidth voting and unvoting

Message ID 1581388671-18078-4-git-send-email-cang@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series UFS driver general fixes bundle 4 | expand

Commit Message

Can Guo Feb. 11, 2020, 2:37 a.m. UTC 
The bus bandwidth voting is required to be done before the bus clocks
are enabled, and the unvoting is required to be done only after the bus
clocks are disabled.

Signed-off-by: Can Guo <cang@codeaurora.org>
Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufs-qcom.c | 78 ++++++++++++++++++++++++++++++---------------
 1 file changed, 53 insertions(+), 25 deletions(-)

Comments

hongwus@codeaurora.org Feb. 11, 2020, 3:09 a.m. UTC | #1 
Hi Can,


On 2020-02-11 10:37, Can Guo wrote:
> The bus bandwidth voting is required to be done before the bus clocks
> are enabled, and the unvoting is required to be done only after the bus
> clocks are disabled.
> 
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufs-qcom.c | 78 
> ++++++++++++++++++++++++++++++---------------
>  1 file changed, 53 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index c69c29a1c..ded08fb 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -38,7 +38,6 @@ enum {
> 
>  static struct ufs_qcom_host *ufs_qcom_hosts[MAX_UFS_QCOM_HOSTS];
> 
> -static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int 
> vote);
>  static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host 
> *host);
>  static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba 
> *hba,
>  						       u32 clk_cycles);
> @@ -674,7 +673,7 @@ static void ufs_qcom_get_speed_mode(struct
> ufs_pa_layer_attr *p, char *result)
>  	}
>  }
> 
> -static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote)
> +static int __ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int 
> vote)
>  {
>  	int err = 0;
> 
> @@ -705,7 +704,7 @@ static int ufs_qcom_update_bus_bw_vote(struct
> ufs_qcom_host *host)
> 
>  	vote = ufs_qcom_get_bus_vote(host, mode);
>  	if (vote >= 0)
> -		err = ufs_qcom_set_bus_vote(host, vote);
> +		err = __ufs_qcom_set_bus_vote(host, vote);
>  	else
>  		err = vote;
> 
> @@ -716,6 +715,35 @@ static int ufs_qcom_update_bus_bw_vote(struct
> ufs_qcom_host *host)
>  	return err;
>  }
> 
> +static int ufs_qcom_set_bus_vote(struct ufs_hba *hba, bool on)
> +{
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +	int vote, err;
> +
> +	/*
> +	 * In case ufs_qcom_init() is not yet done, simply ignore.
> +	 * This ufs_qcom_set_bus_vote() shall be called from
> +	 * ufs_qcom_init() after init is done.
> +	 */
> +	if (!host)
> +		return 0;
> +
> +	if (on) {
> +		vote = host->bus_vote.saved_vote;
> +		if (vote == host->bus_vote.min_bw_vote)
> +			ufs_qcom_update_bus_bw_vote(host);
> +	} else {
> +		vote = host->bus_vote.min_bw_vote;
> +	}
> +
> +	err = __ufs_qcom_set_bus_vote(host, vote);
> +	if (err)
> +		dev_err(hba->dev, "%s: set bus vote failed %d\n",
> +				 __func__, err);
> +
> +	return err;
> +}
> +
>  static ssize_t
>  show_ufs_to_mem_max_bus_bw(struct device *dev, struct device_attribute 
> *attr,
>  			char *buf)
> @@ -792,7 +820,7 @@ static int ufs_qcom_update_bus_bw_vote(struct
> ufs_qcom_host *host)
>  	return 0;
>  }
> 
> -static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote)
> +static int ufs_qcom_set_bus_vote(struct ufs_hba *host, bool on)
>  {
>  	return 0;
>  }
> @@ -1030,8 +1058,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba
> *hba, bool on,
>  				 enum ufs_notify_change_status status)
>  {
>  	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> -	int err;
> -	int vote = 0;
> +	int err = 0;
> 
>  	/*
>  	 * In case ufs_qcom_init() is not yet done, simply ignore.
> @@ -1041,28 +1068,28 @@ static int ufs_qcom_setup_clocks(struct
> ufs_hba *hba, bool on,
>  	if (!host)
>  		return 0;
> 
> -	if (on && (status == POST_CHANGE)) {
> -		/* enable the device ref clock for HS mode*/
> -		if (ufshcd_is_hs_mode(&hba->pwr_info))
> -			ufs_qcom_dev_ref_clk_ctrl(host, true);
> -		vote = host->bus_vote.saved_vote;
> -		if (vote == host->bus_vote.min_bw_vote)
> -			ufs_qcom_update_bus_bw_vote(host);
> -
> -	} else if (!on && (status == PRE_CHANGE)) {
> -		if (!ufs_qcom_is_link_active(hba)) {
> -			/* disable device ref_clk */
> -			ufs_qcom_dev_ref_clk_ctrl(host, false);
> +	switch(status) {
> +	case PRE_CHANGE:
> +		if (on) {
> +			err = ufs_qcom_set_bus_vote(hba, true);
> +		} else {
> +			if (!ufs_qcom_is_link_active(hba)) {
> +				/* disable device ref_clk */
> +				ufs_qcom_dev_ref_clk_ctrl(host, false);
> +			}
>  		}
> -
> -		vote = host->bus_vote.min_bw_vote;
> +		break;
> +	case POST_CHANGE:
> +		if (on) {
> +			/* enable the device ref clock for HS mode*/
> +			if (ufshcd_is_hs_mode(&hba->pwr_info))
> +				ufs_qcom_dev_ref_clk_ctrl(host, true);
> +		} else {
> +			err = ufs_qcom_set_bus_vote(hba, false);
> +		}
> +		break;
>  	}
> 
> -	err = ufs_qcom_set_bus_vote(host, vote);
> -	if (err)
> -		dev_err(hba->dev, "%s: set bus vote failed %d\n",
> -				__func__, err);
> -
>  	return err;
>  }
> 
> @@ -1238,6 +1265,7 @@ static int ufs_qcom_init(struct ufs_hba *hba)
>  	ufs_qcom_set_caps(hba);
>  	ufs_qcom_advertise_quirks(hba);
> 
> +	ufs_qcom_set_bus_vote(hba, true);
>  	ufs_qcom_setup_clocks(hba, true, POST_CHANGE);
> 
>  	if (hba->dev->id < MAX_UFS_QCOM_HOSTS)


   Please add space after switch.
+    switch(status) {
+    case PRE_CHANGE


Reviewed-by: Hongwu Su <hongwus@codeaurora.org>
Can Guo Feb. 11, 2020, 3:13 a.m. UTC | #2 
On 2020-02-11 11:09, hongwus@codeaurora.org wrote:
> Hi Can,
> 
> 
> On 2020-02-11 10:37, Can Guo wrote:
>> The bus bandwidth voting is required to be done before the bus clocks
>> are enabled, and the unvoting is required to be done only after the 
>> bus
>> clocks are disabled.
>> 
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
>> ---
>>  drivers/scsi/ufs/ufs-qcom.c | 78 
>> ++++++++++++++++++++++++++++++---------------
>>  1 file changed, 53 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
>> index c69c29a1c..ded08fb 100644
>> --- a/drivers/scsi/ufs/ufs-qcom.c
>> +++ b/drivers/scsi/ufs/ufs-qcom.c
>> @@ -38,7 +38,6 @@ enum {
>> 
>>  static struct ufs_qcom_host *ufs_qcom_hosts[MAX_UFS_QCOM_HOSTS];
>> 
>> -static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int 
>> vote);
>>  static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host 
>> *host);
>>  static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba 
>> *hba,
>>  						       u32 clk_cycles);
>> @@ -674,7 +673,7 @@ static void ufs_qcom_get_speed_mode(struct
>> ufs_pa_layer_attr *p, char *result)
>>  	}
>>  }
>> 
>> -static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int 
>> vote)
>> +static int __ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int 
>> vote)
>>  {
>>  	int err = 0;
>> 
>> @@ -705,7 +704,7 @@ static int ufs_qcom_update_bus_bw_vote(struct
>> ufs_qcom_host *host)
>> 
>>  	vote = ufs_qcom_get_bus_vote(host, mode);
>>  	if (vote >= 0)
>> -		err = ufs_qcom_set_bus_vote(host, vote);
>> +		err = __ufs_qcom_set_bus_vote(host, vote);
>>  	else
>>  		err = vote;
>> 
>> @@ -716,6 +715,35 @@ static int ufs_qcom_update_bus_bw_vote(struct
>> ufs_qcom_host *host)
>>  	return err;
>>  }
>> 
>> +static int ufs_qcom_set_bus_vote(struct ufs_hba *hba, bool on)
>> +{
>> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> +	int vote, err;
>> +
>> +	/*
>> +	 * In case ufs_qcom_init() is not yet done, simply ignore.
>> +	 * This ufs_qcom_set_bus_vote() shall be called from
>> +	 * ufs_qcom_init() after init is done.
>> +	 */
>> +	if (!host)
>> +		return 0;
>> +
>> +	if (on) {
>> +		vote = host->bus_vote.saved_vote;
>> +		if (vote == host->bus_vote.min_bw_vote)
>> +			ufs_qcom_update_bus_bw_vote(host);
>> +	} else {
>> +		vote = host->bus_vote.min_bw_vote;
>> +	}
>> +
>> +	err = __ufs_qcom_set_bus_vote(host, vote);
>> +	if (err)
>> +		dev_err(hba->dev, "%s: set bus vote failed %d\n",
>> +				 __func__, err);
>> +
>> +	return err;
>> +}
>> +
>>  static ssize_t
>>  show_ufs_to_mem_max_bus_bw(struct device *dev, struct 
>> device_attribute *attr,
>>  			char *buf)
>> @@ -792,7 +820,7 @@ static int ufs_qcom_update_bus_bw_vote(struct
>> ufs_qcom_host *host)
>>  	return 0;
>>  }
>> 
>> -static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int 
>> vote)
>> +static int ufs_qcom_set_bus_vote(struct ufs_hba *host, bool on)
>>  {
>>  	return 0;
>>  }
>> @@ -1030,8 +1058,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba
>> *hba, bool on,
>>  				 enum ufs_notify_change_status status)
>>  {
>>  	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> -	int err;
>> -	int vote = 0;
>> +	int err = 0;
>> 
>>  	/*
>>  	 * In case ufs_qcom_init() is not yet done, simply ignore.
>> @@ -1041,28 +1068,28 @@ static int ufs_qcom_setup_clocks(struct
>> ufs_hba *hba, bool on,
>>  	if (!host)
>>  		return 0;
>> 
>> -	if (on && (status == POST_CHANGE)) {
>> -		/* enable the device ref clock for HS mode*/
>> -		if (ufshcd_is_hs_mode(&hba->pwr_info))
>> -			ufs_qcom_dev_ref_clk_ctrl(host, true);
>> -		vote = host->bus_vote.saved_vote;
>> -		if (vote == host->bus_vote.min_bw_vote)
>> -			ufs_qcom_update_bus_bw_vote(host);
>> -
>> -	} else if (!on && (status == PRE_CHANGE)) {
>> -		if (!ufs_qcom_is_link_active(hba)) {
>> -			/* disable device ref_clk */
>> -			ufs_qcom_dev_ref_clk_ctrl(host, false);
>> +	switch(status) {
>> +	case PRE_CHANGE:
>> +		if (on) {
>> +			err = ufs_qcom_set_bus_vote(hba, true);
>> +		} else {
>> +			if (!ufs_qcom_is_link_active(hba)) {
>> +				/* disable device ref_clk */
>> +				ufs_qcom_dev_ref_clk_ctrl(host, false);
>> +			}
>>  		}
>> -
>> -		vote = host->bus_vote.min_bw_vote;
>> +		break;
>> +	case POST_CHANGE:
>> +		if (on) {
>> +			/* enable the device ref clock for HS mode*/
>> +			if (ufshcd_is_hs_mode(&hba->pwr_info))
>> +				ufs_qcom_dev_ref_clk_ctrl(host, true);
>> +		} else {
>> +			err = ufs_qcom_set_bus_vote(hba, false);
>> +		}
>> +		break;
>>  	}
>> 
>> -	err = ufs_qcom_set_bus_vote(host, vote);
>> -	if (err)
>> -		dev_err(hba->dev, "%s: set bus vote failed %d\n",
>> -				__func__, err);
>> -
>>  	return err;
>>  }
>> 
>> @@ -1238,6 +1265,7 @@ static int ufs_qcom_init(struct ufs_hba *hba)
>>  	ufs_qcom_set_caps(hba);
>>  	ufs_qcom_advertise_quirks(hba);
>> 
>> +	ufs_qcom_set_bus_vote(hba, true);
>>  	ufs_qcom_setup_clocks(hba, true, POST_CHANGE);
>> 
>>  	if (hba->dev->id < MAX_UFS_QCOM_HOSTS)
> 
> 
>   Please add space after switch.
> +    switch(status) {
> +    case PRE_CHANGE
> 
> 
> Reviewed-by: Hongwu Su <hongwus@codeaurora.org>

Thanks.

Can Guo.
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index c69c29a1c..ded08fb 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -38,7 +38,6 @@  enum {
 
 static struct ufs_qcom_host *ufs_qcom_hosts[MAX_UFS_QCOM_HOSTS];
 
-static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote);
 static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host);
 static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
 						       u32 clk_cycles);
@@ -674,7 +673,7 @@  static void ufs_qcom_get_speed_mode(struct ufs_pa_layer_attr *p, char *result)
 	}
 }
 
-static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote)
+static int __ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote)
 {
 	int err = 0;
 
@@ -705,7 +704,7 @@  static int ufs_qcom_update_bus_bw_vote(struct ufs_qcom_host *host)
 
 	vote = ufs_qcom_get_bus_vote(host, mode);
 	if (vote >= 0)
-		err = ufs_qcom_set_bus_vote(host, vote);
+		err = __ufs_qcom_set_bus_vote(host, vote);
 	else
 		err = vote;
 
@@ -716,6 +715,35 @@  static int ufs_qcom_update_bus_bw_vote(struct ufs_qcom_host *host)
 	return err;
 }
 
+static int ufs_qcom_set_bus_vote(struct ufs_hba *hba, bool on)
+{
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	int vote, err;
+
+	/*
+	 * In case ufs_qcom_init() is not yet done, simply ignore.
+	 * This ufs_qcom_set_bus_vote() shall be called from
+	 * ufs_qcom_init() after init is done.
+	 */
+	if (!host)
+		return 0;
+
+	if (on) {
+		vote = host->bus_vote.saved_vote;
+		if (vote == host->bus_vote.min_bw_vote)
+			ufs_qcom_update_bus_bw_vote(host);
+	} else {
+		vote = host->bus_vote.min_bw_vote;
+	}
+
+	err = __ufs_qcom_set_bus_vote(host, vote);
+	if (err)
+		dev_err(hba->dev, "%s: set bus vote failed %d\n",
+				 __func__, err);
+
+	return err;
+}
+
 static ssize_t
 show_ufs_to_mem_max_bus_bw(struct device *dev, struct device_attribute *attr,
 			char *buf)
@@ -792,7 +820,7 @@  static int ufs_qcom_update_bus_bw_vote(struct ufs_qcom_host *host)
 	return 0;
 }
 
-static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote)
+static int ufs_qcom_set_bus_vote(struct ufs_hba *host, bool on)
 {
 	return 0;
 }
@@ -1030,8 +1058,7 @@  static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 				 enum ufs_notify_change_status status)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
-	int err;
-	int vote = 0;
+	int err = 0;
 
 	/*
 	 * In case ufs_qcom_init() is not yet done, simply ignore.
@@ -1041,28 +1068,28 @@  static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 	if (!host)
 		return 0;
 
-	if (on && (status == POST_CHANGE)) {
-		/* enable the device ref clock for HS mode*/
-		if (ufshcd_is_hs_mode(&hba->pwr_info))
-			ufs_qcom_dev_ref_clk_ctrl(host, true);
-		vote = host->bus_vote.saved_vote;
-		if (vote == host->bus_vote.min_bw_vote)
-			ufs_qcom_update_bus_bw_vote(host);
-
-	} else if (!on && (status == PRE_CHANGE)) {
-		if (!ufs_qcom_is_link_active(hba)) {
-			/* disable device ref_clk */
-			ufs_qcom_dev_ref_clk_ctrl(host, false);
+	switch(status) {
+	case PRE_CHANGE:
+		if (on) {
+			err = ufs_qcom_set_bus_vote(hba, true);
+		} else {
+			if (!ufs_qcom_is_link_active(hba)) {
+				/* disable device ref_clk */
+				ufs_qcom_dev_ref_clk_ctrl(host, false);
+			}
 		}
-
-		vote = host->bus_vote.min_bw_vote;
+		break;
+	case POST_CHANGE:
+		if (on) {
+			/* enable the device ref clock for HS mode*/
+			if (ufshcd_is_hs_mode(&hba->pwr_info))
+				ufs_qcom_dev_ref_clk_ctrl(host, true);
+		} else {
+			err = ufs_qcom_set_bus_vote(hba, false);
+		}
+		break;
 	}
 
-	err = ufs_qcom_set_bus_vote(host, vote);
-	if (err)
-		dev_err(hba->dev, "%s: set bus vote failed %d\n",
-				__func__, err);
-
 	return err;
 }
 
@@ -1238,6 +1265,7 @@  static int ufs_qcom_init(struct ufs_hba *hba)
 	ufs_qcom_set_caps(hba);
 	ufs_qcom_advertise_quirks(hba);
 
+	ufs_qcom_set_bus_vote(hba, true);
 	ufs_qcom_setup_clocks(hba, true, POST_CHANGE);
 
 	if (hba->dev->id < MAX_UFS_QCOM_HOSTS)