Re: [PATCH] drm/bridge: it6505: replace scnprintf with sysfs_emit_at in debugfs show

[Greg KH]

On Sun, Jun 29, 2025 at 08:35:09PM -0300, Marcelo Moreira wrote:
> Update the receive_timing_debugfs_show() function to utilize
> sysfs_emit_at() for formatting output to the debugfs buffer.
> This change adheres to the recommendation outlined
> in Documentation/filesystems/sysfs.rst.
> 
> This modification aligns with current sysfs guidelines.

But this isn't a sysfs file, it's a debugfs file, so why are you calling
sysfs_emit_at()?

> 
> Signed-off-by: Marcelo Moreira <marcelomoreira1905@xxxxxxxxx>
> ---
>  drivers/gpu/drm/bridge/ite-it6505.c | 46 ++++++++++++++---------------
>  1 file changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c
> index 1383d1e21afe..98bea08a14e4 100644
> --- a/drivers/gpu/drm/bridge/ite-it6505.c
> +++ b/drivers/gpu/drm/bridge/ite-it6505.c
> @@ -3427,37 +3427,35 @@ static ssize_t receive_timing_debugfs_show(struct file *file, char __user *buf,
>  	struct it6505 *it6505 = file->private_data;
>  	struct drm_display_mode *vid;
>  	u8 read_buf[READ_BUFFER_SIZE];
> -	u8 *str = read_buf, *end = read_buf + READ_BUFFER_SIZE;
> -	ssize_t ret, count;
> +	ssize_t ret;
> +	ssize_t count = 0;
>  
>  	if (!it6505)
>  		return -ENODEV;
>  
>  	it6505_calc_video_info(it6505);
>  	vid = &it6505->video_info;
> -	str += scnprintf(str, end - str, "---video timing---\n");
> -	str += scnprintf(str, end - str, "PCLK:%d.%03dMHz\n",
> -			 vid->clock / 1000, vid->clock % 1000);
> -	str += scnprintf(str, end - str, "HTotal:%d\n", vid->htotal);
> -	str += scnprintf(str, end - str, "HActive:%d\n", vid->hdisplay);
> -	str += scnprintf(str, end - str, "HFrontPorch:%d\n",
> -			 vid->hsync_start - vid->hdisplay);
> -	str += scnprintf(str, end - str, "HSyncWidth:%d\n",
> -			 vid->hsync_end - vid->hsync_start);
> -	str += scnprintf(str, end - str, "HBackPorch:%d\n",
> -			 vid->htotal - vid->hsync_end);
> -	str += scnprintf(str, end - str, "VTotal:%d\n", vid->vtotal);
> -	str += scnprintf(str, end - str, "VActive:%d\n", vid->vdisplay);
> -	str += scnprintf(str, end - str, "VFrontPorch:%d\n",
> -			 vid->vsync_start - vid->vdisplay);
> -	str += scnprintf(str, end - str, "VSyncWidth:%d\n",
> -			 vid->vsync_end - vid->vsync_start);
> -	str += scnprintf(str, end - str, "VBackPorch:%d\n",
> -			 vid->vtotal - vid->vsync_end);
> -
> -	count = str - read_buf;
> +	count += sysfs_emit_at(read_buf, count, "---video timing---\n");
> +	count += sysfs_emit_at(read_buf, count, "PCLK:%d.%03dMHz\n",
> +			vid->clock / 1000, vid->clock % 1000);
> +	count += sysfs_emit_at(read_buf, count, "HTotal:%d\n", vid->htotal);
> +	count += sysfs_emit_at(read_buf, count, "HActive:%d\n", vid->hdisplay);
> +	count += sysfs_emit_at(read_buf, count, "HFrontPorch:%d\n",
> +			vid->hsync_start - vid->hdisplay);
> +	count += sysfs_emit_at(read_buf, count, "HSyncWidth:%d\n",
> +			vid->hsync_end - vid->hsync_start);
> +	count += sysfs_emit_at(read_buf, count, "HBackPorch:%d\n",
> +			vid->htotal - vid->hsync_end);
> +	count += sysfs_emit_at(read_buf, count, "VTotal:%d\n", vid->vtotal);
> +	count += sysfs_emit_at(read_buf, count, "VActive:%d\n", vid->vdisplay);
> +	count += sysfs_emit_at(read_buf, count, "VFrontPorch:%d\n",
> +			vid->vsync_start - vid->vdisplay);
> +	count += sysfs_emit_at(read_buf, count, "VSyncWidth:%d\n",
> +			vid->vsync_end - vid->vsync_start);
> +	count += sysfs_emit_at(read_buf, count, "VBackPorch:%d\n",
> +			vid->vtotal - vid->vsync_end);
> +	
>  	ret = simple_read_from_buffer(buf, len, ppos, read_buf, count);
> -

Shouldn't this all be using seq_print() instead?

Again, don't use sysfs_emit*() functions for non-sysfs files, as you do
NOT know the size of the buffer here (hint, it's not the same).

And, your patch added trailing whitespace, did you forget to run it
through checkpatch.pl before sending it?

thanks,

greg k-h