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

[Bart Van Assche]

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?

> 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?

> +	/*
> +	 * 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.

>   	params->is_valid = true;
> +	params->is_static = false;
>   }

Why is "is_static" changed into false here? A comment might be
appropriate.

> +	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.

Otherwise this patch looks good to me.

Thanks,

Bart.