Re: [PATCH 2/2] scsi: ufs: core: Add support for static TX Equalization settings

[Can Guo <can.guo@oss.qualcomm.com>]

On 5/28/2026 2:13 PM, Manivannan Sadhasivam wrote:
> On Wed, May 27, 2026 at 07:40:55AM -0700, Can Guo wrote:
>> Static TX Equalization settings and TX Precode enable indication from DT
>> 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@oss.qualcomm.com>
>> ---
>>   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;
> Maybe it's me, but I'm not able to understand how you want to apply these static
> 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.
Thanks for the review.

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
   known-good values. TX EQTR can be skipped.
- !is_valid -> no settings available yet; TX EQTR must run.

So the condition check on (!params->is_valid || params->is_static || 
force_tx_eqtr) handles
all three cases.

Thanks,
Can Guo.
>
> - Mani
>