Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings
From: Can Guo
Date: Thu May 28 2026 - 04:48:09 EST
On 5/28/2026 4:16 PM, Manivannan Sadhasivam wrote:
On Thu, May 28, 2026 at 03:24:37PM +0800, Can Guo wrote:The code is correct. My reply was explaining why the check is there, but not
You use '&&' here, but '||' in the code. When you use '||', then I see no point
On 5/28/2026 2:13 PM, Manivannan Sadhasivam wrote:
On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:Thanks for the review.
Static TX Equalization settings and TX Precode enable indication from DTMaybe it's me, but I'm not able to understand how you want to apply these static
properties txeq-preshoot-g[1-6], txeq-deemphasis-g[1-6], and
tx-precode-enable-g6 are board-specific baseline values. Values are
provided as per-lane tuples:
<Host_Lane0 Device_Lane0>, [<Host_Lane1 Device_Lane1>]
Parse DT u32 properties with explicit range checks by using
of_property_count_u32_elems()/of_property_read_u32_array().
When adaptive TX Equalization is used, these static settings are not final:
- If valid settings are retrieved from qTxEQGnSettings/wTxEQGnSettingsExt,
those retrieved settings override static DT settings.
- If retrieval is not available/valid, TX EQTR runs and trained settings
override static DT settings.
So static DT settings are a fallback and are intended for cases where
adaptive TX Equalization is not enabled/used. Adaptive TX Equalization
remains the primary path when enabled.
No behavior changes for platforms that do not provide these properties.
Signed-off-by: Can Guo <can.guo@xxxxxxxxxxxxxxxx>
---
drivers/ufs/core/ufs-txeq.c | 4 +-
drivers/ufs/host/ufshcd-pltfrm.c | 128 +++++++++++++++++++++++++++++++
include/ufs/ufshcd.h | 2 +
3 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
index 4b264adfdf49..634ec039e129 100644
--- a/drivers/ufs/core/ufs-txeq.c
+++ b/drivers/ufs/core/ufs-txeq.c
@@ -1297,7 +1297,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
}
params = &hba->tx_eq_params[gear - 1];
- if (!params->is_valid || force_tx_eqtr) {
+ if (!params->is_valid || params->is_static || force_tx_eqtr) {
int ret;
ret = ufshcd_tx_eqtr(hba, params, pwr_mode);
@@ -1310,6 +1310,7 @@ int ufshcd_config_tx_eq_settings(struct ufs_hba *hba,
/* Mark TX Equalization settings as valid */
params->is_valid = true;
params->is_trained = true;
+ params->is_static = false;
params->is_applied = false;
}
@@ -1495,6 +1496,7 @@ static void ufshcd_extract_tx_eq_settings_attrs(struct ufs_hba *hba, u8 gear)
}
params->is_valid = true;
+ params->is_static = false;
EQ settings. In commit message you said, the static values should be used as a
fallback, but you just check for 'params->is_static' while triggering
ufshcd_tx_eqtr() which is supposed to perform adaptive TX EQ training. IMO, you
don't need any check at all for applying static setting. If '(!params->is_valid
|| force_tx_eqtr)' condition is not satisfied, then the static setting should be
used.
The distinction is between two different sources that can pre-populate
txeq_params with
is_valid set to true before ufshcd_config_tx_eq_settings() is called:
1. DT properties — parsed by ufshcd_pltfrm_parse_tx_eq_settings(),
sets is_valid = true, is_static = true.
2. UFS Attributes (qTxEQGnSettings/wTxEQGnSettingsExt) — retrieved by
ufshcd_retrieve_tx_eq_settings() (introduced in the 2nd series),
sets is_valid = true, is_static = false.
Since both sources set is_valid = true, the is_valid flag alone cannot tell
them apart.
The is_static flag is the discriminator:
- is_valid && is_static -> settings came from DT; they are a board-level
baseline.
TX EQTR should still run to find optimal settings, which will then
overwrite the static ones.
- is_valid && !is_static -> settings came from UFS Attributes; they are
previously trained
for 'is_static' check.
explaining the check itself.
Original check in the code is (!params->is_valid || force_tx_eqtr).
Static TX EQ settings are valid, so '!params->is_valid' is false, TX EQTR would be skipped.
Update the check as (!params->is_valid || params->is_static || force_tx_eqtr) so TX EQTR
must run when static settings are provided.
Thanks,
Can Guo.
- Mani