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

[hongwus]

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@xxxxxxxxxxxxxx>
> Reviewed-by: Asutosh Das <asutoshd@xxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxx>