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

[Bart Van Assche]

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

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

Thanks,

Bart.