Re: [PATCH 2/2] scsi: ufs: core: Add support to retrieve and store TX Equalization settings
From: Can Guo
Date: Mon Apr 20 2026 - 09:53:44 EST
On 4/20/2026 8:33 PM, Peter Wang (王信友) wrote:
Hi Peter,
On Sun, 2026-04-19 at 06:52 -0700, Can Guo wrote:
> Add support for UFS v5.0 JEDEC attributes qTxEQGnSettings and
> wTxEQGnSettingsExt to enable persistent storage and retrieval of
> optimal TX Equalization settings.
> > This provides a fast-path for TX Equalization by reusing previously
> stored optimal settings, avoiding TX Equalization Training (EQTR)
> procedures during subsequent Power Mode changes.
> > When no valid TX Equalization settings are found, fall back to full
> TX
> EQTR procedures and optionally save the results for future use.
> > The validity of one set of TX Equalization settings is indicated by
> Bit[15] in wTxEQGnSettingsExt.
> > Signed-off-by: Can Guo <can.guo@xxxxxxxxxxxxxxxx>
> ---
> drivers/ufs/core/ufs-txeq.c | 241
> +++++++++++++++++++++++++++++++++
> drivers/ufs/core/ufshcd-priv.h | 2 +
> drivers/ufs/core/ufshcd.c | 5 +
> include/ufs/ufs.h | 2 +
> include/ufs/ufshcd.h | 2 +
> 5 files changed, 252 insertions(+)
> > diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-
> txeq.c
> index b2dc89124353..aa9420b0d1c3 100644
> --- a/drivers/ufs/core/ufs-txeq.c
> +++ b/drivers/ufs/core/ufs-txeq.c
> @@ -14,6 +14,83 @@
> #include <ufs/unipro.h>
> #include "ufshcd-priv.h"
> > +#define TX_EQ_SETTING_MASK 0x7
> +#define TX_EQ_SETTINGS_VALID_BIT BIT(15)
> +
> +/*
> + * Decode Device TX Equalization settings based on qTxEQGnSettings
> bit assignment:
> + * bit[3:0]: Device TX Logical LANE 0 PreShoot
> + * bit[7:4]: Device TX Logical LANE 1 PreShoot
> + * bit[19:16]: Device TX Logical LANE 0 DeEmphasis
> + * bit[23:20]: Device TX Logical LANE 1 DeEmphasis
> + */
> +#define TX_EQ_DEVICE_PRESHOOT_DECODE(eq, lane) \
> + (((eq) >> ((lane) * TX_HS_PRESHOOT_SHIFT)) &
> TX_EQ_SETTING_MASK)
> +#define TX_EQ_DEVICE_DEEMPHASIS_DECODE(eq, lane) \
> + (((eq) >> ((lane) * TX_HS_DEEMPHASIS_SHIFT + 16)) &
> TX_EQ_SETTING_MASK)
> +/*
> + * Decode Host TX Equalization settings based on qTxEQGnSettings bit
> assignment:
> + * bit[35:32]: Host TX Logical LANE 0 PreShoot
> + * bit[39:36]: Host TX Logical LANE 1 PreShoot
> + * bit[51:48]: Host TX Logical LANE 0 DeEmphasis
> + * bit[55:52]: Host TX Logical LANE 1 DeEmphasis
> + */
> +#define TX_EQ_HOST_PRESHOOT_DECODE(eq, lane) \
> + (((eq) >> ((lane) * TX_HS_PRESHOOT_SHIFT + 32)) &
> TX_EQ_SETTING_MASK)
> +#define TX_EQ_HOST_DEEMPHASIS_DECODE(eq, lane) \
> + (((eq) >> ((lane) * TX_HS_DEEMPHASIS_SHIFT + 48)) &
> TX_EQ_SETTING_MASK)
> +
> +/*
> + * Decode Device TX precode_en indication based on
> dTxEQGnSettingsExt bit assignment:
> + * bit[0]: PreCodeEn for Device TX Logical LANE 0
> + * bit[1]: PreCodeEn for Device TX Logical LANE 1
> + */
> +#define TX_EQ_DEVICE_PRECODE_EN_DECODE(eq_ext, lane) \
> + (!!((eq_ext) & (1 << (lane))))
> +/*
> + * Decode Host TX precode_en indication based on dTxEQGnSettingsExt
> bit assignment:
> + * bit[4]: PreCodeEn for Device RX Logical LANE 0
> + * bit[5]: PreCodeEn for Device RX Logical LANE 1
> + */
> +#define TX_EQ_HOST_PRECODE_EN_DECODE(eq_ext, lane) \
> + (!!((eq_ext) & (1 << ((lane) + 4))))
> +
> +/*
> + * Encode qTxEQGnSettings based on bit assignment:
> + * bit[3:0]: Device TX Logical LANE 0 PreShoot
> + * bit[7:4]: Device TX Logical LANE 1 PreShoot
> + * bit[19:16]: Device TX Logical LANE 0 DeEmphasis
> + * bit[23:20]: Device TX Logical LANE 1 DeEmphasis
> + */
> +#define TX_EQ_DEVICE_PRESHOOT_ENCODE(val, lane) \
> + (((val) & TX_EQ_SETTING_MASK) << ((lane) *
> TX_HS_PRESHOOT_SHIFT))
> +#define TX_EQ_DEVICE_DEEMPHASIS_ENCODE(val, lane) \
> + (((val) & TX_EQ_SETTING_MASK) << ((lane) *
> TX_HS_DEEMPHASIS_SHIFT + 16))
> +/*
> + * Encode qTxEQGnSettings based on bit assignment:
> + * bit[35:32]: Host TX Logical LANE 0 PreShoot
> + * bit[39:36]: Host TX Logical LANE 1 PreShoot
> + * bit[51:48]: Host TX Logical LANE 0 DeEmphasis
> + * bit[55:52]: Host TX Logical LANE 1 DeEmphasis
> + */
> +#define TX_EQ_HOST_PRESHOOT_ENCODE(val, lane) \
> + (((val) & TX_EQ_SETTING_MASK) << ((lane) *
> TX_HS_PRESHOOT_SHIFT + 32))
> +#define TX_EQ_HOST_DEEMPHASIS_ENCODE(val, lane) \
> + (((val) & TX_EQ_SETTING_MASK) << ((lane) *
> TX_HS_DEEMPHASIS_SHIFT + 48))
> +
> +/*
> + * Encode dTxEQGnSettingsExt based on bit assignment:
> + * bit[0]: PreCodeEn for Device TX Logical LANE 0
> + * bit[1]: PreCodeEn for Device TX Logical LANE 1
> + */
> +#define TX_EQ_DEVICE_PRECODE_EN_ENCODE(val, lane) ((val) <<
> (lane))
> +/*
> + * Encode dTxEQGnSettingsExt based on bit assignment:
> + * bit[4]: PreCodeEn for Device RX Logical LANE 0
> + * bit[5]: PreCodeEn for Device RX Logical LANE 1
> + */
> +#define TX_EQ_HOST_PRECODE_EN_ENCODE(val, lane) > ((val) << ((lane) + 4))
> +
>
Hi Can,
Could you remove redundant parentheses, such as
(val), (lane), (eq_ext), and (eq)?
> +static unsigned int txeq_setting_sel;
> +module_param_cb(txeq_setting_sel, &txeq_setting_sel_ops,
> &txeq_setting_sel, 0644);
> +MODULE_PARM_DESC(txeq_setting_sel, "The qTxEQGnSettings and
> dTxEQGnSettingsExt Attributes selector used to retrieve and store TX
> Equalization settings");
Why is this selection necessary? Shouldn't we follow
the JEDEC specification? As Bart said, introducing new
kernel module parameters is easy, but removing them is hard.
I proposed the two Attributes to JEDEC spec with two Selectors supported initially
to provide flexibility for use cases where we want to keep two different settings for
the same HS-Gear, for example:
1. 0 for Room temperature, 1 for high temperature.
2. 0 for Rate-A, 1 for Rate-B.
3. 0 for Linux, 1 for SBL.
The usage of the two Selectors is not limited to above examples. Yet it is hard to get
aligned on how to use the two Selectors across different companies. Hence I am adding
a module parameter.
Thanks,
Can Guo.
Thanks
Peter
************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be
conveyed only to the designated recipient(s). Any use, dissemination,
distribution, printing, retaining or copying of this e-mail (including its
attachments) by unintended recipient(s) is strictly prohibited and may
be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender
immediately (by replying to this e-mail), delete any and all copies of
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!