Re: [PATCH 1/3] scsi: ufs: ufs-qcom: Restore TX Equalization settings on FOM failure

[Manivannan Sadhasivam]

On Sat, Jun 20, 2026 at 01:03:20AM -0700, Can Guo wrote:
> ufs_qcom_get_rx_fom() applies temporary device TX Equalization values
> before forcing HS mode and running the EOM-based SW FOM scan.
> 
> When one of these steps fails, the function can bypass the shared
> cleanup path and leave temporary TX Equalization settings programmed.
> 
> Route those failures through the cleanup label so the original TX EQ
> settings are restored and link recovery runs before exit.
> 
> This path also reuses ret for cleanup, so it may overwrite the original
> error. Keep that on purpose: if cleanup succeeds, the caller can proceed
> with the FOM result for the current iteration.
> 
> Signed-off-by: Can Guo <can.guo@xxxxxxxxxxxxxxxx>
> ---
>  drivers/ufs/host/ufs-qcom.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index c084ccc72523..7d7c001435bf 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -2791,7 +2791,7 @@ static int ufs_qcom_get_rx_fom(struct ufs_hba *hba,
>  	if (ret) {
>  		dev_err(hba->dev, "%s: Failed to apply TX EQ settings for HS-G%u: %d\n",
>  			__func__, gear, ret);
> -		return ret;
> +		goto link_recover_and_restore;

IIUC, if ufshcd_apply_tx_eq_settings() fails, then it means the TX EQ settings
were not applied. So do we really need to restore the original TQ EQ here?

>  	}
>  
>  	/* Force PMC to target HS Gear to use new TX Equalization settings. */
> @@ -2799,16 +2799,15 @@ static int ufs_qcom_get_rx_fom(struct ufs_hba *hba,
>  	if (ret) {
>  		dev_err(hba->dev, "%s: Failed to change power mode to HS-G%u, Rate-%s: %d\n",
>  			__func__, gear, ufs_hs_rate_to_str(rate), ret);
> -		return ret;
> +		goto link_recover_and_restore;
>  	}
>  
>  	ret = ufs_qcom_host_sw_rx_fom(hba, pwr_mode->lane_rx, fom);
> -	if (ret) {
> +	if (ret)
>  		dev_err(hba->dev, "Failed to get SW FOM of TX (PreShoot: %u, DeEmphasis: %u): %d\n",
>  			d_iter->preshoot, d_iter->deemphasis, ret);
> -		return ret;
> -	}
>  
> +link_recover_and_restore:
>  	/* Restore Device's TX Equalization settings. */
>  	ret = ufshcd_apply_tx_eq_settings(hba, &hba->tx_eq_params[gear - 1], gear);
>  	if (ret) {

'ret' will be overridden here and '0' might be returned to the caller.

- Mani

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