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

[Bean Huo]

Can, 


Sorry for the late review of this patch. I have several questions:


On Fri, 2026-05-01 at 06:44 -0700, Can Guo wrote:
> @@ -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) {

When use_adaptive_txeq is on and params->is_static is true, EQTR will overwrite
the static values. That is reasonable since EQTR is more accurate. Is the
is_static check really needed here?

>                 int ret;
>  
> 

...

>  
>  void ufshcd_retrieve_tx_eq_settings(struct ufs_hba *hba)
> diff --git a/drivers/ufs/host/ufshcd-pltfrm.c b/drivers/ufs/host/ufshcd-
> pltfrm.c
> index c2dafb583cf5..de6302e8c067 100644
> --- a/drivers/ufs/host/ufshcd-pltfrm.c
> +++ b/drivers/ufs/host/ufshcd-pltfrm.c
> @@ -210,6 +210,86 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba
> *hba)
>         }
>  }
>  
> +static void ufshcd_parse_static_tx_eq_settings(struct ufs_hba *hba)
> +{
> +       size_t sz = hba->lanes_per_direction * 2 * TX_EQ_SETTINGS_TUPLE_SZ;
> +       u32 settings[UFS_MAX_LANES * 2 * TX_EQ_SETTINGS_TUPLE_SZ];
> +       u32 *host_settings, *device_settings;
> +       u32 lpd = hba->lanes_per_direction;
> +       struct ufshcd_tx_eq_params *params;
> 
....
> +
> +               params = &hba->tx_eq_params[gear - 1];
> +               host_settings = settings;
> +               device_settings = settings + lpd * TX_EQ_SETTINGS_TUPLE_SZ;
> +
> +               for (lane = 0; lane < lpd; lane++) {
> +                       params->host[lane].preshoot = host_settings[0];
> +                       params->host[lane].deemphasis = host_settings[1];
> +                       params->host[lane].precode_en = host_settings[2];
> +                       host_settings += TX_EQ_SETTINGS_TUPLE_SZ;
> +
> +                       params->device[lane].preshoot = device_settings[0];
> +                       params->device[lane].deemphasis = device_settings[1];
> +                       params->device[lane].precode_en = device_settings[2];
> +                       device_settings += TX_EQ_SETTINGS_TUPLE_SZ;
> +               }
> +
> +               params->is_valid = true;
> +               params->is_static = true;

I want to confirm I understand the code correctly. Please tell me if I am wrong:

1, When use_adaptive_txeq = 0: static values are used directly as TX EQ for HS-
G4 to G6. But ufshcd_config_tx_eq_settings() returns early when
use_adaptive_txeq = 0. So which function applies the static values in this
case? 

2, when use_adaptive_txeq = 1: static host values are used as the fixed host TX
EQ during EQTR. This is because ufs_qcom_get_rx_fom() only sweeps the device
side. It reads host values from hba->tx_eq_params[gear-1]->host[]. The static
values also work as the per-lane fallback in ufshcd_update_tx_eq_params() when
FOM is 0.

> +       }
> +}
> +
>  /**
>   * ufshcd_parse_clock_min_max_freq  - Parse MIN and MAX clocks freq
>   * @hba: per adapter instance
> @@ -528,6 +608,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>  
>         ufshcd_init_lanes_per_dir(hba);
>  
> +       ufshcd_parse_static_tx_eq_settings(hba);
> +
>         err = ufshcd_parse_operating_points(hba);
>         if (err) {
>                 dev_err(dev, "%s: OPP parse failed %d\n", __func__, err);
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index f48d6416e299..2d385d42fcff 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -359,6 +359,7 @@ struct ufshcd_tx_eqtr_record {
>   * @is_valid: True if parameter contains valid TX Equalization settings
>   * @is_applied: True if settings have been applied to UniPro of both sides
>   * @is_trained: True if parameters obtained from TX EQTR procedure
> + * @is_static: True if settings are static
>   */
>  struct ufshcd_tx_eq_params {
>         struct ufshcd_tx_eq_settings host[UFS_MAX_LANES];
> @@ -367,8 +368,12 @@ struct ufshcd_tx_eq_params {
>         bool is_valid;
>         bool is_applied;
>         bool is_trained;
> +       bool is_static;

is_static is added next to is_trained, which was added in your "Add persistent
TX Equalization settings support" series, That series still has Brian's open
question about wTxEQGnSettingsExt Bit[15] being RFU per JESD220H:

https://patchwork.kernel.org/project/linux-scsi/cover/20260424151420.111675-1-can.guo@xxxxxxxxxxxxxxx

Is this the reason why "Add persistent..." has not been merged?

I'd prefer to wait until that discussion concludes before tagging this one. I
hope this is ok for you.

Kind regards,
Bean