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

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

[Can Guo]

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@oss.qualcomm.com>
---
 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;
 	}
 
 	/* 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) {
-- 
2.34.1

[PATCH 2/3] scsi: ufs: core: Tolerate RX_FOM read failures in TX EQTR

[Can Guo]

ufshcd_get_rx_fom() aborted TX EQTR when a per-lane RX_FOM DME read failed.
That makes the whole training flow fragile even though these reads can be
treated as best effort.

Keep TX EQTR running by logging RX_FOM read failures and continuing.
Make failed lanes deterministic by initializing each lane FOM to 0 before
reading and only updating it when the DME read succeeds. This avoids
propagating stale or uninitialized values into EQTR evaluation.

Also update the kerneldoc return description to match behavior: RX_FOM
DME read failures are logged for debug visibility, while get_rx_fom()
vops failures are still propagated to the caller.

Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
---
 drivers/ufs/core/ufs-txeq.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
index 9dca0cd344b8..23a12e221d31 100644
--- a/drivers/ufs/core/ufs-txeq.c
+++ b/drivers/ufs/core/ufs-txeq.c
@@ -482,7 +482,9 @@ static void ufshcd_evaluate_tx_eqtr_fom(struct ufs_hba *hba,
  * @h_iter: host TX EQTR iterator data structure
  * @d_iter: device TX EQTR iterator data structure
  *
- * Returns 0 on success, negative error code otherwise
+ * Returns 0 on success, negative error code if get_rx_fom vops fails.
+ * RX_FOM DME get failures are debug-logged and treated as 0 FOM for
+ * that lane.
  */
 static int ufshcd_get_rx_fom(struct ufs_hba *hba,
 			     struct ufs_pa_layer_attr *pwr_mode,
@@ -494,22 +496,30 @@ static int ufshcd_get_rx_fom(struct ufs_hba *hba,
 
 	/* Get FOM of host's TX lanes from device's RX_FOM. */
 	for (lane = 0; lane < pwr_mode->lane_tx; lane++) {
+		h_iter->fom[lane] = 0;
 		ret = ufshcd_dme_peer_get(hba, UIC_ARG_MIB_SEL(RX_FOM,
 					  UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
 					  &fom);
-		if (ret)
-			return ret;
+		if (ret) {
+			dev_dbg(hba->dev, "Failed to get FOM for Host TX Lane %d: %d\n",
+				lane, ret);
+			continue;
+		}
 
 		h_iter->fom[lane] = (u8)fom;
 	}
 
 	/* Get FOM of device's TX lanes from host's RX_FOM. */
 	for (lane = 0; lane < pwr_mode->lane_rx; lane++) {
+		d_iter->fom[lane] = 0;
 		ret = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(RX_FOM,
 				     UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
 				     &fom);
-		if (ret)
-			return ret;
+		if (ret) {
+			dev_dbg(hba->dev, "Failed to get FOM for Device TX Lane %d: %d\n",
+				lane, ret);
+			continue;
+		}
 
 		d_iter->fom[lane] = (u8)fom;
 	}
-- 
2.34.1

[PATCH 3/3] scsi: ufs: core: Always run tx_eqtr POST_CHANGE notify

[Can Guo]

ufshcd_tx_eqtr() skips POST_CHANGE notify when __ufshcd_tx_eqtr()
fails. That can leave variant cleanup incomplete when PRE_CHANGE saved
temporary state that POST_CHANGE is expected to restore.

Always call POST_CHANGE once PRE_CHANGE has succeeded. Keep the TX EQTR
result as the primary return value, and only propagate POST_CHANGE
failure when TX EQTR itself succeeded.

Log PRE_CHANGE and POST_CHANGE notify failures to make variant callback
failures visible in TX EQTR error paths.

Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
---
 drivers/ufs/core/ufs-txeq.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
index 23a12e221d31..c39a623b4fe1 100644
--- a/drivers/ufs/core/ufs-txeq.c
+++ b/drivers/ufs/core/ufs-txeq.c
@@ -1224,6 +1224,7 @@ static int ufshcd_tx_eqtr(struct ufs_hba *hba,
 {
 	struct ufs_pa_layer_attr old_pwr_info;
 	unsigned int noio_flag;
+	int notify_ret;
 	int ret;
 
 	/*
@@ -1253,14 +1254,19 @@ static int ufshcd_tx_eqtr(struct ufs_hba *hba,
 	}
 
 	ret = ufshcd_vops_tx_eqtr_notify(hba, PRE_CHANGE, pwr_mode);
-	if (ret)
+	if (ret) {
+		dev_err(hba->dev, "TX EQTR PRE_CHANGE notify failed: %d\n", ret);
 		goto out;
+	}
 
 	ret = __ufshcd_tx_eqtr(hba, params, pwr_mode);
-	if (ret)
-		goto out;
 
-	ret = ufshcd_vops_tx_eqtr_notify(hba, POST_CHANGE, pwr_mode);
+	notify_ret = ufshcd_vops_tx_eqtr_notify(hba, POST_CHANGE, pwr_mode);
+	if (notify_ret)
+		dev_err(hba->dev, "TX EQTR POST_CHANGE notify failed: %d\n", notify_ret);
+
+	if (!ret)
+		ret = notify_ret;
 
 out:
 	if (ret)
-- 
2.34.1

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@oss.qualcomm.com>
> ---
>  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

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

Re: [PATCH 2/3] scsi: ufs: core: Tolerate RX_FOM read failures in TX EQTR

[Manivannan Sadhasivam]

On Sat, Jun 20, 2026 at 01:03:21AM -0700, Can Guo wrote:
> ufshcd_get_rx_fom() aborted TX EQTR when a per-lane RX_FOM DME read failed.
> That makes the whole training flow fragile even though these reads can be
> treated as best effort.
> 
> Keep TX EQTR running by logging RX_FOM read failures and continuing.
> Make failed lanes deterministic by initializing each lane FOM to 0 before
> reading and only updating it when the DME read succeeds. This avoids
> propagating stale or uninitialized values into EQTR evaluation.
> 
> Also update the kerneldoc return description to match behavior: RX_FOM
> DME read failures are logged for debug visibility, while get_rx_fom()
> vops failures are still propagated to the caller.
> 
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
>  drivers/ufs/core/ufs-txeq.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
> index 9dca0cd344b8..23a12e221d31 100644
> --- a/drivers/ufs/core/ufs-txeq.c
> +++ b/drivers/ufs/core/ufs-txeq.c
> @@ -482,7 +482,9 @@ static void ufshcd_evaluate_tx_eqtr_fom(struct ufs_hba *hba,
>   * @h_iter: host TX EQTR iterator data structure
>   * @d_iter: device TX EQTR iterator data structure
>   *
> - * Returns 0 on success, negative error code otherwise
> + * Returns 0 on success, negative error code if get_rx_fom vops fails.
> + * RX_FOM DME get failures are debug-logged and treated as 0 FOM for
> + * that lane.
>   */
>  static int ufshcd_get_rx_fom(struct ufs_hba *hba,
>  			     struct ufs_pa_layer_attr *pwr_mode,
> @@ -494,22 +496,30 @@ static int ufshcd_get_rx_fom(struct ufs_hba *hba,
>  
>  	/* Get FOM of host's TX lanes from device's RX_FOM. */
>  	for (lane = 0; lane < pwr_mode->lane_tx; lane++) {
> +		h_iter->fom[lane] = 0;
>  		ret = ufshcd_dme_peer_get(hba, UIC_ARG_MIB_SEL(RX_FOM,
>  					  UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>  					  &fom);
> -		if (ret)
> -			return ret;
> +		if (ret) {
> +			dev_dbg(hba->dev, "Failed to get FOM for Host TX Lane %d: %d\n",
> +				lane, ret);
> +			continue;
> +		}
>  
>  		h_iter->fom[lane] = (u8)fom;
>  	}
>  
>  	/* Get FOM of device's TX lanes from host's RX_FOM. */
>  	for (lane = 0; lane < pwr_mode->lane_rx; lane++) {
> +		d_iter->fom[lane] = 0;
>  		ret = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(RX_FOM,
>  				     UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>  				     &fom);
> -		if (ret)
> -			return ret;
> +		if (ret) {
> +			dev_dbg(hba->dev, "Failed to get FOM for Device TX Lane %d: %d\n",
> +				lane, ret);
> +			continue;
> +		}
>  
>  		d_iter->fom[lane] = (u8)fom;
>  	}
> -- 
> 2.34.1
> 

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

Re: [PATCH 3/3] scsi: ufs: core: Always run tx_eqtr POST_CHANGE notify

[Manivannan Sadhasivam]

On Sat, Jun 20, 2026 at 01:03:22AM -0700, Can Guo wrote:
> ufshcd_tx_eqtr() skips POST_CHANGE notify when __ufshcd_tx_eqtr()
> fails. That can leave variant cleanup incomplete when PRE_CHANGE saved
> temporary state that POST_CHANGE is expected to restore.
> 
> Always call POST_CHANGE once PRE_CHANGE has succeeded. Keep the TX EQTR
> result as the primary return value, and only propagate POST_CHANGE
> failure when TX EQTR itself succeeded.
> 
> Log PRE_CHANGE and POST_CHANGE notify failures to make variant callback
> failures visible in TX EQTR error paths.
> 
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
>  drivers/ufs/core/ufs-txeq.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
> index 23a12e221d31..c39a623b4fe1 100644
> --- a/drivers/ufs/core/ufs-txeq.c
> +++ b/drivers/ufs/core/ufs-txeq.c
> @@ -1224,6 +1224,7 @@ static int ufshcd_tx_eqtr(struct ufs_hba *hba,
>  {
>  	struct ufs_pa_layer_attr old_pwr_info;
>  	unsigned int noio_flag;
> +	int notify_ret;
>  	int ret;
>  
>  	/*
> @@ -1253,14 +1254,19 @@ static int ufshcd_tx_eqtr(struct ufs_hba *hba,
>  	}
>  
>  	ret = ufshcd_vops_tx_eqtr_notify(hba, PRE_CHANGE, pwr_mode);
> -	if (ret)
> +	if (ret) {
> +		dev_err(hba->dev, "TX EQTR PRE_CHANGE notify failed: %d\n", ret);
>  		goto out;
> +	}
>  
>  	ret = __ufshcd_tx_eqtr(hba, params, pwr_mode);
> -	if (ret)
> -		goto out;
>  
> -	ret = ufshcd_vops_tx_eqtr_notify(hba, POST_CHANGE, pwr_mode);
> +	notify_ret = ufshcd_vops_tx_eqtr_notify(hba, POST_CHANGE, pwr_mode);
> +	if (notify_ret)
> +		dev_err(hba->dev, "TX EQTR POST_CHANGE notify failed: %d\n", notify_ret);
> +
> +	if (!ret)
> +		ret = notify_ret;
>  
>  out:
>  	if (ret)
> -- 
> 2.34.1
> 

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

Re: [PATCH 2/3] scsi: ufs: core: Tolerate RX_FOM read failures in TX EQTR

[Peter Wang (王信友)]

On Sat, 2026-06-20 at 01:03 -0700, Can Guo wrote:
> @@ -494,22 +496,30 @@ static int ufshcd_get_rx_fom(struct ufs_hba
> *hba,
> 
>         /* Get FOM of host's TX lanes from device's RX_FOM. */
>         for (lane = 0; lane < pwr_mode->lane_tx; lane++) {
> +               h_iter->fom[lane] = 0;
>                 ret = ufshcd_dme_peer_get(hba,
> UIC_ARG_MIB_SEL(RX_FOM,
>                                          
> UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>                                           &fom);
> -               if (ret)
> -                       return ret;
> +               if (ret) {
> +                       dev_dbg(hba->dev, "Failed to get FOM for Host
> TX Lane %d: %d\n",
> +                               lane, ret);
> +                       continue;
> +               }
> 

Hi Can,

I suggest setting h_iter->fom[lane] = 0 when an error occurs, 
as this approach is clearer and improves code efficiency, 
for example:
if (ret) {
    h_iter->fom[lane] = 0;
    ...
    continue;
}


>                 h_iter->fom[lane] = (u8)fom;
>         }
> 
>         /* Get FOM of device's TX lanes from host's RX_FOM. */
>         for (lane = 0; lane < pwr_mode->lane_rx; lane++) {
> +               d_iter->fom[lane] = 0;
>                 ret = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(RX_FOM,
>                                     
> UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>                                      &fom);
> -               if (ret)
> -                       return ret;
> +               if (ret) {
> +                       dev_dbg(hba->dev, "Failed to get FOM for
> Device TX Lane %d: %d\n",
> +                               lane, ret);
> +                       continue;
> +               }
> 

The same applies as above.

Thanks
Peter

Re: [PATCH 3/3] scsi: ufs: core: Always run tx_eqtr POST_CHANGE notify

[Peter Wang (王信友)]

On Sat, 2026-06-20 at 01:03 -0700, Can Guo wrote:
> ufshcd_tx_eqtr() skips POST_CHANGE notify when __ufshcd_tx_eqtr()
> fails. That can leave variant cleanup incomplete when PRE_CHANGE
> saved
> temporary state that POST_CHANGE is expected to restore.
> 
> Always call POST_CHANGE once PRE_CHANGE has succeeded. Keep the TX
> EQTR
> result as the primary return value, and only propagate POST_CHANGE
> failure when TX EQTR itself succeeded.
> 
> Log PRE_CHANGE and POST_CHANGE notify failures to make variant
> callback
> failures visible in TX EQTR error paths.
> 
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
> ---

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 2/3] scsi: ufs: core: Tolerate RX_FOM read failures in TX EQTR

[Can Guo]

On 6/23/2026 5:05 PM, Peter Wang (王信友) wrote:
>
> On Sat, 2026-06-20 at 01:03 -0700, Can Guo wrote:
> > @@ -494,22 +496,30 @@ static int ufshcd_get_rx_fom(struct ufs_hba
> > *hba,
> > 
> >         /* Get FOM of host's TX lanes from device's RX_FOM. */
> >         for (lane = 0; lane < pwr_mode->lane_tx; lane++) {
> > +               h_iter->fom[lane] = 0;
> >                 ret = ufshcd_dme_peer_get(hba,
> > UIC_ARG_MIB_SEL(RX_FOM,
> >                                          
> > UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
> >                                           &fom);
> > -               if (ret)
> > -                       return ret;
> > +               if (ret) {
> > +                       dev_dbg(hba->dev, "Failed to get FOM for Host
> > TX Lane %d: %d\n",
> > +                               lane, ret);
> > +                       continue;
> > +               }
> > 
>
> Hi Can,
>
> I suggest setting h_iter->fom[lane] = 0 when an error occurs,
> as this approach is clearer and improves code efficiency,
> for example:
> if (ret) {
>      h_iter->fom[lane] = 0;
>      ...
>      continue;
> }
Thanks for suggestion, and I like it, let me adopt in next version.

Thanks,
Can Guo.
>
>
> >                 h_iter->fom[lane] = (u8)fom;
> >         }
> > 
> >         /* Get FOM of device's TX lanes from host's RX_FOM. */
> >         for (lane = 0; lane < pwr_mode->lane_rx; lane++) {
> > +               d_iter->fom[lane] = 0;
> >                 ret = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(RX_FOM,
> >                                     
> > UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
> >                                      &fom);
> > -               if (ret)
> > -                       return ret;
> > +               if (ret) {
> > +                       dev_dbg(hba->dev, "Failed to get FOM for
> > Device TX Lane %d: %d\n",
> > +                               lane, ret);
> > +                       continue;
> > +               }
> > 
>
> The same applies as above.
>
> Thanks
> Peter
>
> ************* MEDIATEK Confidentiality Notice
>   ********************
> The information contained in this e-mail message (including any
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be
> conveyed only to the designated recipient(s). Any use, dissemination,
> distribution, printing, retaining or copying of this e-mail (including its
> attachments) by unintended recipient(s) is strictly prohibited and may
> be unlawful. If you are not an intended recipient of this e-mail, or believe
>   
> that you have received this e-mail in error, please notify the sender
> immediately (by replying to this e-mail), delete any and all copies of
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!

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

[Can Guo]

On 6/23/2026 2:33 PM, Manivannan Sadhasivam wrote:
> 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@oss.qualcomm.com>
>> ---
>>   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?
Hi Mani,

Thanks for the review.

In many cases yes, but not always. `ufshcd_apply_tx_eq_settings()` 
writes multiple
DME attributes, so a failure may happen after partial programming. 
Routing all failure
cases through a single cleanup path makes us always attempt restore and 
link recovery,
instead of exiting early with possibly transient settings.
>
>>   	}
>>   
>>   	/* 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.
Correct, this is intentional for the TX EQTR sweep. This helper runs 
per-iteration, and I want a
failed sample to be treated as FOM=0 so the sweep can continue to the 
next point, rather than
aborting the full procedure. We only stop/abort TX EQTR sweep when 
cleanup/recovery itself fails.

Thanks,
Can Guo.
>
> - Mani
>

Re: [PATCH 3/3] scsi: ufs: core: Always run tx_eqtr POST_CHANGE notify

[Bart Van Assche]

On 6/20/26 1:03 AM, Can Guo wrote:
> ufshcd_tx_eqtr() skips POST_CHANGE notify when __ufshcd_tx_eqtr()
> fails. That can leave variant cleanup incomplete when PRE_CHANGE saved
> temporary state that POST_CHANGE is expected to restore.
> 
> Always call POST_CHANGE once PRE_CHANGE has succeeded. Keep the TX EQTR
> result as the primary return value, and only propagate POST_CHANGE
> failure when TX EQTR itself succeeded.
> 
> Log PRE_CHANGE and POST_CHANGE notify failures to make variant callback
> failures visible in TX EQTR error paths.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>