* Re: [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks
2026-11-11 7:26 [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks Di Wu
@ 2026-11-11 8:36 ` Benson Leung
2026-11-11 11:17 ` David Laight
2026-11-11 14:39 ` Andy Shevchenko
2 siblings, 0 replies; 9+ messages in thread
From: Benson Leung @ 2026-11-11 8:36 UTC (permalink / raw)
To: Di Wu
Cc: jic23, dlechner, nuno.sa, andy, bleung, groeck, tzungbi,
linux-iio, chrome-platform, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2102 bytes --]
On Wed, Nov 11, 2026 at 07:26:08AM +0000, Di Wu wrote:
> Replace sprintf() and snprintf() with sysfs_emit() in the sysfs show
> and ext_info read callbacks. sysfs_emit() is aware of the sysfs buffer
> PAGE_SIZE limit and is the preferred helper for show callbacks.
>
> Signed-off-by: Di Wu <diwux@google.com>
Reviewed-by: Benson Leung <bleung@chromium.org>
> ---
> .../iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index d03e40c91582..624d4b32b034 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -149,9 +149,9 @@ static ssize_t cros_ec_sensor_get_report_latency(struct device *dev,
> if (ret < 0)
> return ret;
>
> - return sprintf(buf, "%d.%06u\n",
> - latency / 1000,
> - (latency % 1000) * 1000);
> + return sysfs_emit(buf, "%d.%06u\n",
> + latency / 1000,
> + (latency % 1000) * 1000);
> }
>
> static IIO_DEVICE_ATTR(hwfifo_timeout, 0644,
> @@ -165,7 +165,7 @@ static ssize_t hwfifo_watermark_max_show(struct device *dev,
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
>
> - return sprintf(buf, "%d\n", st->fifo_max_event_count);
> + return sysfs_emit(buf, "%d\n", st->fifo_max_event_count);
> }
>
> static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0);
> @@ -465,7 +465,7 @@ static ssize_t cros_ec_sensors_id(struct iio_dev *indio_dev,
> {
> struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", st->param.info.sensor_num);
> + return sysfs_emit(buf, "%d\n", st->param.info.sensor_num);
> }
>
> const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[] = {
>
> base-commit: b70566610b48ba49dec636019da4d696db64b085
> --
> 2.56.0.455.g6c5a503b38-goog
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks
2026-11-11 7:26 [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks Di Wu
2026-11-11 8:36 ` Benson Leung
@ 2026-11-11 11:17 ` David Laight
2026-11-11 14:39 ` Andy Shevchenko
2 siblings, 0 replies; 9+ messages in thread
From: David Laight @ 2026-11-11 11:17 UTC (permalink / raw)
To: Di Wu
Cc: jic23, dlechner, nuno.sa, andy, bleung, groeck, tzungbi,
linux-iio, chrome-platform, linux-kernel
On Wed, 11 Nov 2026 07:26:08 +0000
Di Wu <diwux@google.com> wrote:
> Replace sprintf() and snprintf() with sysfs_emit() in the sysfs show
> and ext_info read callbacks. sysfs_emit() is aware of the sysfs buffer
> PAGE_SIZE limit and is the preferred helper for show callbacks.
Reviewed-by: David Laight <david.laight.linux@gmail.com>
>
> Signed-off-by: Di Wu <diwux@google.com>
> ---
> .../iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index d03e40c91582..624d4b32b034 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -149,9 +149,9 @@ static ssize_t cros_ec_sensor_get_report_latency(struct device *dev,
> if (ret < 0)
> return ret;
>
> - return sprintf(buf, "%d.%06u\n",
> - latency / 1000,
> - (latency % 1000) * 1000);
> + return sysfs_emit(buf, "%d.%06u\n",
> + latency / 1000,
> + (latency % 1000) * 1000);
At least two of those lines can be merged.
I think it is 86 chars - maybe ok.
Personally I'd split after the format.
David
> }
>
> static IIO_DEVICE_ATTR(hwfifo_timeout, 0644,
> @@ -165,7 +165,7 @@ static ssize_t hwfifo_watermark_max_show(struct device *dev,
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
>
> - return sprintf(buf, "%d\n", st->fifo_max_event_count);
> + return sysfs_emit(buf, "%d\n", st->fifo_max_event_count);
> }
>
> static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0);
> @@ -465,7 +465,7 @@ static ssize_t cros_ec_sensors_id(struct iio_dev *indio_dev,
> {
> struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
>
> - return snprintf(buf, PAGE_SIZE, "%d\n", st->param.info.sensor_num);
> + return sysfs_emit(buf, "%d\n", st->param.info.sensor_num);
> }
>
> const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[] = {
>
> base-commit: b70566610b48ba49dec636019da4d696db64b085
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks
2026-11-11 7:26 [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks Di Wu
2026-11-11 8:36 ` Benson Leung
2026-11-11 11:17 ` David Laight
@ 2026-11-11 14:39 ` Andy Shevchenko
2026-11-12 2:04 ` Jonathan Cameron
2 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2026-11-11 14:39 UTC (permalink / raw)
To: Di Wu
Cc: jic23, dlechner, nuno.sa, andy, bleung, groeck, tzungbi,
linux-iio, chrome-platform, linux-kernel
On Wed, Nov 11, 2026 at 07:26:08AM +0000, Di Wu wrote:
> Replace sprintf() and snprintf() with sysfs_emit() in the sysfs show
> and ext_info read callbacks. sysfs_emit() is aware of the sysfs buffer
> PAGE_SIZE limit and is the preferred helper for show callbacks.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
...
> - return sprintf(buf, "%d.%06u\n",
> - latency / 1000,
> - (latency % 1000) * 1000);
> + return sysfs_emit(buf, "%d.%06u\n",
> + latency / 1000,
> + (latency % 1000) * 1000);
As David said, since you touched these lines, make the less amount of them
while at it.
(But no need to resend, usually Jonathan is kind to tweak this small thing.)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks
2026-11-11 14:39 ` Andy Shevchenko
@ 2026-11-12 2:04 ` Jonathan Cameron
2026-11-12 22:02 ` David Laight
0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2026-11-12 2:04 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Di Wu, dlechner, nuno.sa, andy, bleung, groeck, tzungbi,
linux-iio, chrome-platform, linux-kernel
On Wed, 11 Nov 2026 16:39:37 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Wed, Nov 11, 2026 at 07:26:08AM +0000, Di Wu wrote:
> > Replace sprintf() and snprintf() with sysfs_emit() in the sysfs show
> > and ext_info read callbacks. sysfs_emit() is aware of the sysfs buffer
> > PAGE_SIZE limit and is the preferred helper for show callbacks.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> ...
>
> > - return sprintf(buf, "%d.%06u\n",
> > - latency / 1000,
> > - (latency % 1000) * 1000);
> > + return sysfs_emit(buf, "%d.%06u\n",
> > + latency / 1000,
> > + (latency % 1000) * 1000);
>
> As David said, since you touched these lines, make the less amount of them
> while at it.
>
> (But no need to resend, usually Jonathan is kind to tweak this small thing.)
>
Done and applied.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks
2026-11-12 2:04 ` Jonathan Cameron
@ 2026-11-12 22:02 ` David Laight
2026-11-13 1:36 ` Jonathan Cameron
0 siblings, 1 reply; 9+ messages in thread
From: David Laight @ 2026-11-12 22:02 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Di Wu, dlechner, nuno.sa, andy, bleung, groeck,
tzungbi, linux-iio, chrome-platform, linux-kernel
On Thu, 12 Nov 2026 02:04:39 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Wed, 11 Nov 2026 16:39:37 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>
> > On Wed, Nov 11, 2026 at 07:26:08AM +0000, Di Wu wrote:
> > > Replace sprintf() and snprintf() with sysfs_emit() in the sysfs show
> > > and ext_info read callbacks. sysfs_emit() is aware of the sysfs buffer
> > > PAGE_SIZE limit and is the preferred helper for show callbacks.
> >
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> >
> > ...
> >
> > > - return sprintf(buf, "%d.%06u\n",
> > > - latency / 1000,
> > > - (latency % 1000) * 1000);
> > > + return sysfs_emit(buf, "%d.%06u\n",
> > > + latency / 1000,
> > > + (latency % 1000) * 1000);
> >
> > As David said, since you touched these lines, make the less amount of them
> > while at it.
> >
> > (But no need to resend, usually Jonathan is kind to tweak this small thing.)
> >
> Done and applied.
>
>
I just looked at that line again.
Why is it multiplying the remainder by 1000?
It could be:
return sysfs_emit(buf, "%d.%03u000\n", latency / 1000, latency % 1000);
if the 6 digits after the decimal point are required.
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks
2026-11-12 22:02 ` David Laight
@ 2026-11-13 1:36 ` Jonathan Cameron
2026-11-13 9:23 ` David Laight
0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2026-11-13 1:36 UTC (permalink / raw)
To: David Laight
Cc: Andy Shevchenko, Di Wu, dlechner, nuno.sa, andy, bleung, groeck,
tzungbi, linux-iio, chrome-platform, linux-kernel
On Thu, 12 Nov 2026 22:02:18 +0000
David Laight <david.laight.linux@gmail.com> wrote:
> On Thu, 12 Nov 2026 02:04:39 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Wed, 11 Nov 2026 16:39:37 +0200
> > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> >
> > > On Wed, Nov 11, 2026 at 07:26:08AM +0000, Di Wu wrote:
> > > > Replace sprintf() and snprintf() with sysfs_emit() in the sysfs show
> > > > and ext_info read callbacks. sysfs_emit() is aware of the sysfs buffer
> > > > PAGE_SIZE limit and is the preferred helper for show callbacks.
> > >
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > >
> > > ...
> > >
> > > > - return sprintf(buf, "%d.%06u\n",
> > > > - latency / 1000,
> > > > - (latency % 1000) * 1000);
> > > > + return sysfs_emit(buf, "%d.%06u\n",
> > > > + latency / 1000,
> > > > + (latency % 1000) * 1000);
> > >
> > > As David said, since you touched these lines, make the less amount of them
> > > while at it.
> > >
> > > (But no need to resend, usually Jonathan is kind to tweak this small thing.)
> > >
> > Done and applied.
> >
> >
>
> I just looked at that line again.
> Why is it multiplying the remainder by 1000?
> It could be:
> return sysfs_emit(buf, "%d.%03u000\n", latency / 1000, latency % 1000);
> if the 6 digits after the decimal point are required.
Good point, I guess unrelated change that we could have on top.
Those trailing zeros shouldn't matter.
>
> David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks
2026-11-13 1:36 ` Jonathan Cameron
@ 2026-11-13 9:23 ` David Laight
2026-11-14 2:15 ` Jonathan Cameron
0 siblings, 1 reply; 9+ messages in thread
From: David Laight @ 2026-11-13 9:23 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, Di Wu, dlechner, nuno.sa, andy, bleung, groeck,
tzungbi, linux-iio, chrome-platform, linux-kernel
On Fri, 13 Nov 2026 01:36:46 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Thu, 12 Nov 2026 22:02:18 +0000
> David Laight <david.laight.linux@gmail.com> wrote:
>
> > On Thu, 12 Nov 2026 02:04:39 +0000
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > > On Wed, 11 Nov 2026 16:39:37 +0200
> > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > >
> > > > On Wed, Nov 11, 2026 at 07:26:08AM +0000, Di Wu wrote:
> > > > > Replace sprintf() and snprintf() with sysfs_emit() in the sysfs show
> > > > > and ext_info read callbacks. sysfs_emit() is aware of the sysfs buffer
> > > > > PAGE_SIZE limit and is the preferred helper for show callbacks.
> > > >
> > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > > >
> > > > ...
> > > >
> > > > > - return sprintf(buf, "%d.%06u\n",
> > > > > - latency / 1000,
> > > > > - (latency % 1000) * 1000);
> > > > > + return sysfs_emit(buf, "%d.%06u\n",
> > > > > + latency / 1000,
> > > > > + (latency % 1000) * 1000);
> > > >
> > > > As David said, since you touched these lines, make the less amount of them
> > > > while at it.
> > > >
> > > > (But no need to resend, usually Jonathan is kind to tweak this small thing.)
> > > >
> > > Done and applied.
> > >
> > >
> >
> > I just looked at that line again.
> > Why is it multiplying the remainder by 1000?
> > It could be:
> > return sysfs_emit(buf, "%d.%03u000\n", latency / 1000, latency % 1000);
> > if the 6 digits after the decimal point are required.
>
> Good point, I guess unrelated change that we could have on top.
Indeed.
> Those trailing zeros shouldn't matter.
There is a matching oddity in the set function (just above).
It calls iio_str_to_fixpoint(buf, 1000000, &integer, &fract)
then does latency = integer * 1000 + fract/1000.
Just replace the 100000 by 1000 and you don't need the divide.
iio_str_to_fixpoint() will ignore are extra fractional digits.
Although I'm not at all sure its handling of negative value is correct.
It converts -1.5 to -1/5 and -0.5 to 0/-5.
I can't see how that is useful since i*10+f for both gives -5.
I'd have though that either the fractional part should always be positive
so -1.4 would need converting to -2/6 or it would have the same sign as
the integer part and -1.4 would be converted to -1/-4.
David
>
> >
> > David
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks
2026-11-13 9:23 ` David Laight
@ 2026-11-14 2:15 ` Jonathan Cameron
0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2026-11-14 2:15 UTC (permalink / raw)
To: David Laight
Cc: Andy Shevchenko, Di Wu, dlechner, nuno.sa, andy, bleung, groeck,
tzungbi, linux-iio, chrome-platform, linux-kernel
On Fri, 13 Nov 2026 09:23:05 +0000
David Laight <david.laight.linux@gmail.com> wrote:
> On Fri, 13 Nov 2026 01:36:46 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Thu, 12 Nov 2026 22:02:18 +0000
> > David Laight <david.laight.linux@gmail.com> wrote:
> >
> > > On Thu, 12 Nov 2026 02:04:39 +0000
> > > Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > > On Wed, 11 Nov 2026 16:39:37 +0200
> > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > > >
> > > > > On Wed, Nov 11, 2026 at 07:26:08AM +0000, Di Wu wrote:
> > > > > > Replace sprintf() and snprintf() with sysfs_emit() in the sysfs show
> > > > > > and ext_info read callbacks. sysfs_emit() is aware of the sysfs buffer
> > > > > > PAGE_SIZE limit and is the preferred helper for show callbacks.
> > > > >
> > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > > > >
> > > > > ...
> > > > >
> > > > > > - return sprintf(buf, "%d.%06u\n",
> > > > > > - latency / 1000,
> > > > > > - (latency % 1000) * 1000);
> > > > > > + return sysfs_emit(buf, "%d.%06u\n",
> > > > > > + latency / 1000,
> > > > > > + (latency % 1000) * 1000);
> > > > >
> > > > > As David said, since you touched these lines, make the less amount of them
> > > > > while at it.
> > > > >
> > > > > (But no need to resend, usually Jonathan is kind to tweak this small thing.)
> > > > >
> > > > Done and applied.
> > > >
> > > >
> > >
> > > I just looked at that line again.
> > > Why is it multiplying the remainder by 1000?
> > > It could be:
> > > return sysfs_emit(buf, "%d.%03u000\n", latency / 1000, latency % 1000);
> > > if the 6 digits after the decimal point are required.
> >
> > Good point, I guess unrelated change that we could have on top.
>
> Indeed.
>
> > Those trailing zeros shouldn't matter.
>
> There is a matching oddity in the set function (just above).
> It calls iio_str_to_fixpoint(buf, 1000000, &integer, &fract)
> then does latency = integer * 1000 + fract/1000.
> Just replace the 100000 by 1000 and you don't need the divide.
>
> iio_str_to_fixpoint() will ignore are extra fractional digits.
>
> Although I'm not at all sure its handling of negative value is correct.
> It converts -1.5 to -1/5 and -0.5 to 0/-5.
> I can't see how that is useful since i*10+f for both gives -5.
>
> I'd have though that either the fractional part should always be positive
> so -1.4 would need converting to -2/6 or it would have the same sign as
> the integer part and -1.4 would be converted to -1/-4.
That is rather odd use of the output of iio_str_to_fixpoint()
which admittedly is a bit unusual because of the need to handle there
being no where to put that negative when the integer is 0.
Can get away with it here though as latencies aren't negative so.
The code doesn't seem to actually check that though which might
be a nice useability improvement rather than setting it to something odd.
Jonathan
>
> David
>
> >
> > >
> > > David
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread