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

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

On 6/11/2026 4:08 AM, Bart Van Assche wrote:
> On 6/10/26 12:15 AM, Can Guo wrote:
>> Parse board-specific static TX Equalization settings from DT for each HS
>> gear and store them in hba->tx_eq_params.
>
> The word "static" means "showing little change". My understanding is
> that the settings from the DT tree are used if equalization training is
> not performed. If my understanding is correct, I think the use of the
> word "static" is misleading. Maybe "default" or "from_dt" reflects the
> purpose of these settings better?
Thanks for your review.

'from_dt' sounds better.
>
>> When adaptive TX Equalization is used, these static settings are not 
>> final:
>
> What is the meaning of "adaptive" in the above sentence? I haven't 
> found that word in the UFSHCI 5.0 standard nor in the UFS 5.0 standard 
> in the
> context of TX equalization. Should that word perhaps be left out?
'adaptive' is not standard term in spec, I used it to refer to on-demand TX
Equalization Training procedure I put up in the first patch series. If it is
confusing, I can remove it from the commit msg.
>
>> +    /*
>> +     * TX EQTR must run for the following cases:
>> +     * 1. TX EQ settings are invalid.
>> +     * 2. TX EQ settings are valid but static, i.e., populated from DT.
>> +     * 3. TX EQTR procedure is forced.
>> +     */
>
> What is the difference between "TX EQ" and "TX EQTR"? If both refer to
> TX equalization, please use the same acronym for all three bullets.
TX EQ is TX Equalization.
TX EQTR is TX Equalization Training.
>
>>       params->is_valid = true;
>> +    params->is_static = false;
>>   }
>
> Why is "is_static" changed into false here? A comment might be
> appropriate.
After TX EQTR procedure, the params is populated with training results,
which are no longer 'from_dt'. I will add some comments in next version.
>
>> + ufshcd_parse_static_tx_eq_settings(hba);
>
> Please consider changing "static" into "default" or "dt" (device tree)
> in the above function name. I think that will make the code easier to
> follow.
Sure, will use 'from_dt'.

Thanks,
Can Guo.
>
> Otherwise this patch looks good to me.
>
> Thanks,
>
> Bart.