* [PATCH] drm/bridge: it6505: replace scnprintf with sysfs_emit_at in debugfs show
@ 2025-06-29 23:35 Marcelo Moreira
2025-06-30 5:09 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Marcelo Moreira @ 2025-06-29 23:35 UTC (permalink / raw)
To: Andrzej Hajda, skhan, linux-kernel-mentees, ~lkcamp/patches
Cc: Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
linux-kernel
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.
Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
---
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);
-
return ret;
}
--
2.50.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/bridge: it6505: replace scnprintf with sysfs_emit_at in debugfs show
2025-06-29 23:35 [PATCH] drm/bridge: it6505: replace scnprintf with sysfs_emit_at in debugfs show Marcelo Moreira
@ 2025-06-30 5:09 ` Greg KH
2025-07-01 0:35 ` Marcelo Moreira
0 siblings, 1 reply; 3+ messages in thread
From: Greg KH @ 2025-06-30 5:09 UTC (permalink / raw)
To: Marcelo Moreira
Cc: Andrzej Hajda, skhan, linux-kernel-mentees, ~lkcamp/patches,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
linux-kernel
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@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/bridge: it6505: replace scnprintf with sysfs_emit_at in debugfs show
2025-06-30 5:09 ` Greg KH
@ 2025-07-01 0:35 ` Marcelo Moreira
0 siblings, 0 replies; 3+ messages in thread
From: Marcelo Moreira @ 2025-07-01 0:35 UTC (permalink / raw)
To: Greg KH
Cc: Andrzej Hajda, skhan, linux-kernel-mentees, ~lkcamp/patches,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
linux-kernel
Em seg., 30 de jun. de 2025 às 02:09, Greg KH <greg@kroah.com> escreveu:
>
> 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()?
>
You're right, thanks Greg.
> >
> > Signed-off-by: Marcelo Moreira <marcelomoreira1905@gmail.com>
> > ---
> > 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 again for the clarification, I'll be more attentive for future
submissions.
Specifically for this patch I forgot to run checkpatch, sorry. Sorry
for all the inconvenience.
Thanks Greg!
--
Cheers,
Marcelo Moreira
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-01 7:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-29 23:35 [PATCH] drm/bridge: it6505: replace scnprintf with sysfs_emit_at in debugfs show Marcelo Moreira
2025-06-30 5:09 ` Greg KH
2025-07-01 0:35 ` Marcelo Moreira
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).