Re: [PATCH v3 07/12] scsi: ufs: core: Add support to refresh TX Equalization via debugfs

[Can Guo]

Hi Bart,

On 3/14/2026 6:30 AM, Bart Van Assche wrote:
> On 3/8/26 8:14 AM, Can Guo wrote:
> > Drastic environmental changes, such as significant temperature 
> > shifts, can
> > impact link signal integrity. In such cases, refreshing TX 
> > Equalization is
> > necessary to compensate for these environmental changes.
> >
> > Add a debugfs entry, 'tx_eq_ctrl', to allow userspace to manually 
> > trigger
> > the TX Equalization training (EQTR) procedure and apply the identified
> > optimal settings on the fly. These entries are created on a per-gear 
> > basis
> > for High Speed Gear 4 (HS-G4) and above, as TX EQTR is not supported for
> > lower gears.
> >
> > The 'tx_eq_ctrl' entry currently accepts the 'refresh' command to 
> > initiate
> > the procedure. The interface is designed to be scalable to support
> > additional commands in the future.
> >
> > Reading the 'tx_eq_ctrl' entry provides a usage hint to the user,
> > ensuring the interface is self-documenting.
> >
> > The ufshcd's debugfs folder structure will look like below:
> >
> > /sys/kernel/debug/ufshcd/*ufs*/
> > |--tx_eq_hs_gear1/
> > |  |--device_tx_eq_params
> > |  |--host_tx_eq_params
> > |--tx_eq_hs_gear2/
> > |--tx_eq_hs_gear3/
> > |--tx_eq_hs_gear4/
> > |--tx_eq_hs_gear5/
> > |--tx_eq_hs_gear6/
> >     |--device_tx_eq_params
> >     |--device_tx_eqtr_record
> >     |--host_tx_eq_params
> >     |--host_tx_eqtr_record
> >     |--tx_eq_ctrl
> >
> > Signed-off-by: Can Guo <can.guo@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/ufs/core/ufs-debugfs.c | 61 ++++++++++++++++++++++++++
> >   drivers/ufs/core/ufs-txeq.c    | 78 +++++++++++++++++++++++++++++++++-
> >   drivers/ufs/core/ufshcd-priv.h |  5 ++-
> >   drivers/ufs/core/ufshcd.c      |  7 +--
> >   4 files changed, 143 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/ufs/core/ufs-debugfs.c 
> > b/drivers/ufs/core/ufs-debugfs.c
> > index 6f7562846f5b..b3bb2c850ad2 100644
> > --- a/drivers/ufs/core/ufs-debugfs.c
> > +++ b/drivers/ufs/core/ufs-debugfs.c
> > @@ -383,9 +383,70 @@ static const struct file_operations 
> > ufs_tx_eqtr_record_fops = {
> >       .release    = single_release,
> >   };
> >   +static ssize_t ufs_tx_eq_ctrl_write(struct file *file, const char 
> > __user *buf,
> > +                    size_t count, loff_t *ppos)
> > +{
> > +    u32 gear = (u32)(uintptr_t)file->f_inode->i_private;
> > +    struct ufs_hba *hba = hba_from_file(file);
> > +    char kbuf[32];
> > +    int ret;
> > +
> > +    if (count >= sizeof(kbuf))
> > +        return -EINVAL;
> > +
> > +    if (copy_from_user(kbuf, buf, count))
> > +        return -EFAULT;
> > +
> > +    kbuf[count] = '\0';
> > +
> > +    if (!ufshcd_is_tx_eq_supported(hba))
> > +        return -EOPNOTSUPP;
> > +
> > +    if (hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL ||
> > +        !hba->max_pwr_info.is_valid)
> > +        return -EBUSY;
> > +
> > +    if (!hba->ufs_device_wlun)
> > +        return -ENODEV;
> > +
> > +    if (sysfs_streq(kbuf, "refresh")) {
> > +        ret = ufs_debugfs_get_user_access(hba);
> > +        if (ret)
> > +            return ret;
> > +        ret = ufshcd_refresh_tx_eq(hba, gear);
> > +        ufs_debugfs_put_user_access(hba);
> > +    } else {
> > +        /* Unknown operation */
> > +        return -EINVAL;
> > +    }
> > +
> > +    return ret ? ret : count;
> > +}
> > +
> > +static int ufs_tx_eq_ctrl_show(struct seq_file *s, void *data)
> > +{
> > +    seq_puts(s, "write 'refresh' to refresh TX Equalization 
> > settings\n");
> > +    return 0;
> > +}
>
> In the above two functions, since the standard uses the terminology
> "TX equalization training", wouldn't it be more appropriate to use the
> word "retrain" instead of "refresh"?
I chose 'refresh' because the code conducts more than just retraining of 
TX EQ,
the code also carries out a Power Mode change after that, and only by 
doing a
Power Mode change, the new (optimal) TX EQ settings are really used by 
both Host
and Device.
> > +/**
> > + * ufshcd_refresh_tx_eq - Retrain TX Equalization and apply new 
> > settings
>
> Shouldn't the word "refresh" be changed into "retrain" to make the
> function name consistent with the one-line description of this function?
Here refresh = retrain TX EQ + a Power Mode change

Thanks,
Can Guo.
> Thanks,
>
> Bart.