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

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

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@oss.qualcomm.com>
>> ---
>>   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.