Re: [PATCH 1/2] scsi: ufs: ufs-qcom: Update PHY settings only when scaling to higher gears

[Can Guo <quic_cang@quicinc.com>]

On 9/8/2023 10:53 PM, Manivannan Sadhasivam wrote:
> The "hs_gear" variable is used to program the PHY settings (submode) during
> ufs_qcom_power_up_sequence(). Currently, it is being updated every time the
> agreed gear changes. Due to this, if the gear got downscaled before suspend
> (runtime/system), then while resuming, the PHY settings for the lower gear
> will be applied first and later when scaling to max gear with REINIT, the
> PHY settings for the max gear will be applied.
>
> This adds a latency while resuming and also really not needed as the PHY
> gear settings are backwards compatible i.e., we can continue using the PHY
> settings for max gear with lower gear speed.
>
> So let's update the "hs_gear" variable _only_ when the agreed gear is
> greater than the current one. This guarantees that the PHY settings will be
> changed only during probe time and fatal error condition.
>
> Due to this, UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH can now be skipped
> when the PM operation is in progress.
>
> Cc: stable@vger.kernel.org
> Fixes: 96a7141da332 ("scsi: ufs: core: Add support for reinitializing the UFS device")
> Reported-by: Can Guo <quic_cang@quicinc.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>   drivers/ufs/core/ufshcd.c   | 3 ++-
>   drivers/ufs/host/ufs-qcom.c | 9 +++++++--
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 34472871610d..1f0a9d96e613 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8782,7 +8782,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool init_dev_params)
>   	if (ret)
>   		goto out;
>   
> -	if (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH) {
> +	if (!hba->pm_op_in_progress &&
> +	    (hba->quirks & UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH)) {
>   		/* Reset the device and controller before doing reinit */
>   		ufshcd_device_reset(hba);
>   		ufshcd_hba_stop(hba);
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 78689d3479e4..ebb8054a3b3e 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -909,8 +909,13 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
>   			return ret;
>   		}
>   
> -		/* Use the agreed gear */
> -		host->hs_gear = dev_req_params->gear_tx;
> +		/*
> +		 * Update hs_gear only when the gears are scaled to a higher value. This is because,
> +		 * the PHY gear settings are backwards compatible and we only need to change the PHY
> +		 * settings while scaling to higher gears.
> +		 */
> +		if (dev_req_params->gear_tx > host->hs_gear)
> +			host->hs_gear = dev_req_params->gear_tx;
>   
>   		/* enable the device ref clock before changing to HS mode */
>   		if (!ufshcd_is_hs_mode(&hba->pwr_info) &&

Reviewed-by: Can Guo <quic_cang@quicinc.com>

Tested-by: Can Guo <quic_cang@quicinc.com>