[PATCH v2 0/3] scsi: ufs: Harden TX EQTR error handling paths

[PATCH v2 0/3] scsi: ufs: Harden TX EQTR error handling paths

[Can Guo]

TX Equalization training currently has a few error-path gaps that can
make the flow brittle and can leave variant/device cleanup incomplete.

This series hardens TX EQTR in three places:

1. ufs-qcom: route SW FOM setup failures through the shared cleanup path
   so temporary device TX EQ settings are restored and link recovery is
   always attempted before exit.
2. core: treat RX_FOM DME read failures as best effort so TX EQTR can
   continue, and force failed lanes to deterministic 0 FOM.
3. core: always run tx_eqtr POST_CHANGE notify once PRE_CHANGE succeeds,
   even when TX EQTR fails, so variant cleanup is not skipped.

Together these changes improve TX EQTR robustness without changing the
normal success path.

Dependency note:
PATCH 3/3 depends on the patch below, which is still under review:
https://lore.kernel.org/all/c71af930-c7b4-4480-b125-f35cbe35a16f@oss.qualcomm.com/

Please apply this series on top of that patch (or a tree containing it).

v1 -> v2:
- Adopted Peter's comment (Patch 2)

Can Guo (3):
  scsi: ufs: ufs-qcom: Restore TX Equalization settings on FOM failure
  scsi: ufs: core: Tolerate RX_FOM read failures in TX EQTR
  scsi: ufs: core: Always run tx_eqtr POST_CHANGE notify

 drivers/ufs/core/ufs-txeq.c | 33 ++++++++++++++++++++++++---------
 drivers/ufs/host/ufs-qcom.c |  9 ++++-----
 2 files changed, 28 insertions(+), 14 deletions(-)

-- 
2.34.1

[PATCH v2 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

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

[Bean Huo]

On Thu, 2026-06-25 at 05:13 -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>

Loods good to me.

Reviewed-by: Bean Huo <beanhuo@micron.com>

[PATCH v2 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 handled as warnings, 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 | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
index 9dca0cd344b8..e1302ea9f27e 100644
--- a/drivers/ufs/core/ufs-txeq.c
+++ b/drivers/ufs/core/ufs-txeq.c
@@ -482,7 +482,8 @@ 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 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,
@@ -497,8 +498,12 @@ static int ufshcd_get_rx_fom(struct ufs_hba *hba,
 		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) {
+			h_iter->fom[lane] = 0;
+			dev_dbg(hba->dev, "Failed to get FOM for Host TX Lane %d: %d\n",
+				 lane, ret);
+			continue;
+		}
 
 		h_iter->fom[lane] = (u8)fom;
 	}
@@ -508,8 +513,12 @@ static int ufshcd_get_rx_fom(struct ufs_hba *hba,
 		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) {
+			d_iter->fom[lane] = 0;
+			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 v2 2/3] scsi: ufs: core: Tolerate RX_FOM read failures in TX EQTR

[Peter Wang (王信友)]

On Thu, 2026-06-25 at 05:13 -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 handled as warnings, while get_rx_fom() vops
> failures are still propagated to the caller.
> 
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>

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

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

[Bean Huo]

On Thu, 2026-06-25 at 05:13 -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 handled as warnings, 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 | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
> index 9dca0cd344b8..e1302ea9f27e 100644
> --- a/drivers/ufs/core/ufs-txeq.c
> +++ b/drivers/ufs/core/ufs-txeq.c
> @@ -482,7 +482,8 @@ 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 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,
> @@ -497,8 +498,12 @@ static int ufshcd_get_rx_fom(struct ufs_hba *hba,
>                 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) {
> +                       h_iter->fom[lane] = 0;
> +                       dev_dbg(hba->dev, "Failed to get FOM for Host TX Lane
> %d: %d\n",
> +                                lane, ret);
> +                       continue;
> +               }
>  
>                 h_iter->fom[lane] = (u8)fom;
>         }
> @@ -508,8 +513,12 @@ static int ufshcd_get_rx_fom(struct ufs_hba *hba,
>                 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) {
> +                       d_iter->fom[lane] = 0;
> +                       dev_dbg(hba->dev, "Failed to get FOM for Device TX
> Lane %d: %d\n",
> +                                lane, ret);
> +                       continue;
> +               }


Logic is correct, only one nit: dev_dbg() is silent in production, dev_warn or
dev_warn_ratelimited would match the stated "warnings" in the commit message
intented.


Reviewed-by: Bean Huo <beanhuo@micron.com>

[PATCH v2 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.

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
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 e1302ea9f27e..7f908ea97ec3 100644
--- a/drivers/ufs/core/ufs-txeq.c
+++ b/drivers/ufs/core/ufs-txeq.c
@@ -1223,6 +1223,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;
 
 	/*
@@ -1252,14 +1253,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 v2 3/3] scsi: ufs: core: Always run tx_eqtr POST_CHANGE notify

[Bean Huo]

On Thu, 2026-06-25 at 05:13 -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.
> 
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> Reviewed-by: Peter Wang <peter.wang@mediatek.com>
> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>

Loods good to me!

Reviewed-by: Bean Huo <beanhuo@micron.com>

Re: [PATCH v2 0/3] scsi: ufs: Harden TX EQTR error handling paths

[Ziqi Chen]

On 6/25/2026 8:13 PM, Can Guo wrote:
> TX Equalization training currently has a few error-path gaps that can
> make the flow brittle and can leave variant/device cleanup incomplete.
>
> This series hardens TX EQTR in three places:
>
> 1. ufs-qcom: route SW FOM setup failures through the shared cleanup path
>     so temporary device TX EQ settings are restored and link recovery is
>     always attempted before exit.
> 2. core: treat RX_FOM DME read failures as best effort so TX EQTR can
>     continue, and force failed lanes to deterministic 0 FOM.
> 3. core: always run tx_eqtr POST_CHANGE notify once PRE_CHANGE succeeds,
>     even when TX EQTR fails, so variant cleanup is not skipped.
>
> Together these changes improve TX EQTR robustness without changing the
> normal success path.
>
> Dependency note:
> PATCH 3/3 depends on the patch below, which is still under review:
> https://lore.kernel.org/all/c71af930-c7b4-4480-b125-f35cbe35a16f@oss.qualcomm.com/
>
> Please apply this series on top of that patch (or a tree containing it).
>
> v1 -> v2:
> - Adopted Peter's comment (Patch 2)
>
> Can Guo (3):
>    scsi: ufs: ufs-qcom: Restore TX Equalization settings on FOM failure
>    scsi: ufs: core: Tolerate RX_FOM read failures in TX EQTR
>    scsi: ufs: core: Always run tx_eqtr POST_CHANGE notify
>
>   drivers/ufs/core/ufs-txeq.c | 33 ++++++++++++++++++++++++---------
>   drivers/ufs/host/ufs-qcom.c |  9 ++++-----
>   2 files changed, 28 insertions(+), 14 deletions(-)
Reviewed-by: Ziqi Chen <ziqi.chen@oss.qualcomm.com>