Re: [PATCH v2 3/9] ufs: core: Introduce ufshcd_post_device_init()

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Thu, Aug 22, 2024 at 02:36:04PM -0700, Bart Van Assche wrote:
> Prepare for introducing a second caller by moving more code from
> ufshcd_device_init() into a new function.
> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

One nitpick below.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---
>  drivers/ufs/core/ufshcd.c | 64 ++++++++++++++++++++++-----------------
>  1 file changed, 37 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 04d94bf5cc2d..dcc417d7e19c 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8755,6 +8755,42 @@ static int ufshcd_activate_link(struct ufs_hba *hba)
>  	return 0;
>  }
>  
> +static int ufshcd_post_device_init(struct ufs_hba *hba)
> +{
> +	int ret;
> +
> +	ufshcd_tune_unipro_params(hba);
> +
> +	/* UFS device is also active now */
> +	ufshcd_set_ufs_dev_active(hba);
> +	ufshcd_force_reset_auto_bkops(hba);
> +
> +	ufshcd_set_timestamp_attr(hba);
> +	schedule_delayed_work(&hba->ufs_rtc_update_work,
> +			      msecs_to_jiffies(UFS_RTC_UPDATE_INTERVAL_MS));
> +
> +	if (!hba->max_pwr_info.is_valid)
> +		return 0;
> +
> +	/* Gear up to HS gear. */

Maybe this comment now belongs above ufshcd_config_pwr_mode()?

- Mani

> +
> +	/*
> +	 * Set the right value to bRefClkFreq before attempting to
> +	 * switch to HS gears.
> +	 */
> +	if (hba->dev_ref_clk_freq != REF_CLK_FREQ_INVAL)
> +		ufshcd_set_dev_ref_clk(hba);
> +	ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
> +	if (ret) {
> +		dev_err(hba->dev,
> +			"%s: Failed setting power mode, err = %d\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
>  {
>  	struct Scsi_Host *host = hba->host;
> @@ -8813,33 +8849,7 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
>  		}
>  	}
>  
> -	ufshcd_tune_unipro_params(hba);
> -
> -	/* UFS device is also active now */
> -	ufshcd_set_ufs_dev_active(hba);
> -	ufshcd_force_reset_auto_bkops(hba);
> -
> -	ufshcd_set_timestamp_attr(hba);
> -	schedule_delayed_work(&hba->ufs_rtc_update_work,
> -			      msecs_to_jiffies(UFS_RTC_UPDATE_INTERVAL_MS));
> -
> -	/* Gear up to HS gear if supported */
> -	if (hba->max_pwr_info.is_valid) {
> -		/*
> -		 * Set the right value to bRefClkFreq before attempting to
> -		 * switch to HS gears.
> -		 */
> -		if (hba->dev_ref_clk_freq != REF_CLK_FREQ_INVAL)
> -			ufshcd_set_dev_ref_clk(hba);
> -		ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
> -		if (ret) {
> -			dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n",
> -					__func__, ret);
> -			return ret;
> -		}
> -	}
> -
> -	return 0;
> +	return ufshcd_post_device_init(hba);
>  }
>  
>  /**

-- 
மணிவண்ணன் சதாசிவம்