[v6] iio: accel: sca3000: simplify by using newer infrastructure

[PATCH v6 4/4] iio: accel: sca3000: use sysfs_emit_at() instead of sprintf()

Posted by Andrew Ijano 3 months, 3 weeks ago

Use sysfs_emit_at() instead of sprintf() for sysfs operations as
suggested in the documentation, since it is aware of PAGE_SIZE buffer.

Signed-off-by: Andrew Ijano <andrew.lopes@alumni.usp.br>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/sca3000.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 058a2d67c91c..bc0046b19511 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -423,16 +423,16 @@ sca3000_show_available_3db_freqs(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct sca3000_state *st = iio_priv(indio_dev);
-	int len;
+	unsigned int len = 0;
 
-	len = sprintf(buf, "%d", st->info->measurement_mode_3db_freq);
+	len = sysfs_emit_at(buf, len, "%d", st->info->measurement_mode_3db_freq);
 	if (st->info->option_mode_1)
-		len += sprintf(buf + len, " %d",
+		len += sysfs_emit_at(buf, len, " %d",
 			       st->info->option_mode_1_3db_freq);
 	if (st->info->option_mode_2)
-		len += sprintf(buf + len, " %d",
+		len += sysfs_emit_at(buf, len, " %d",
 			       st->info->option_mode_2_3db_freq);
-	len += sprintf(buf + len, "\n");
+	len += sysfs_emit_at(buf, len, "\n");
 
 	return len;
 }
@@ -783,7 +783,6 @@ static ssize_t sca3000_read_av_freq(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct sca3000_state *st = iio_priv(indio_dev);
-	unsigned int len = 0;
 	int ret;
 
 	scoped_guard(mutex, &st->lock) {
@@ -794,25 +793,22 @@ static ssize_t sca3000_read_av_freq(struct device *dev,
 
 	switch (ret & SCA3000_REG_MODE_MODE_MASK) {
 	case SCA3000_REG_MODE_MEAS_MODE_NORMAL:
-		len += sprintf(buf + len, "%d %d %d\n",
+		return sysfs_emit(buf, "%d %d %d\n",
 			       st->info->measurement_mode_freq,
 			       st->info->measurement_mode_freq / 2,
 			       st->info->measurement_mode_freq / 4);
-		break;
 	case SCA3000_REG_MODE_MEAS_MODE_OP_1:
-		len += sprintf(buf + len, "%d %d %d\n",
+		return sysfs_emit(buf, "%d %d %d\n",
 			       st->info->option_mode_1_freq,
 			       st->info->option_mode_1_freq / 2,
 			       st->info->option_mode_1_freq / 4);
-		break;
 	case SCA3000_REG_MODE_MEAS_MODE_OP_2:
-		len += sprintf(buf + len, "%d %d %d\n",
+		return sysfs_emit(buf, "%d %d %d\n",
 			       st->info->option_mode_2_freq,
 			       st->info->option_mode_2_freq / 2,
 			       st->info->option_mode_2_freq / 4);
-		break;
 	}
-	return len;
+	return 0;
 }
 
 /*
-- 
2.49.0

Re: [PATCH v6 4/4] iio: accel: sca3000: use sysfs_emit_at() instead of sprintf()

Posted by Jonathan Cameron 3 months, 2 weeks ago

On Wed, 18 Jun 2025 00:12:19 -0300
Andrew Ijano <andrew.ijano@gmail.com> wrote:

> Use sysfs_emit_at() instead of sprintf() for sysfs operations as
> suggested in the documentation, since it is aware of PAGE_SIZE buffer.
> 
> Signed-off-by: Andrew Ijano <andrew.lopes@alumni.usp.br>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Hi.

A few comments inline,

Thanks,

J
> ---
>  drivers/iio/accel/sca3000.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
> index 058a2d67c91c..bc0046b19511 100644
> --- a/drivers/iio/accel/sca3000.c
> +++ b/drivers/iio/accel/sca3000.c
> @@ -423,16 +423,16 @@ sca3000_show_available_3db_freqs(struct device *dev,
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct sca3000_state *st = iio_priv(indio_dev);
> -	int len;
> +	unsigned int len = 0;

No need to initialize as set on the next line

>  
> -	len = sprintf(buf, "%d", st->info->measurement_mode_3db_freq);
> +	len = sysfs_emit_at(buf, len, "%d", st->info->measurement_mode_3db_freq);

sysfs_emit() when you know you are at the start.


>  	if (st->info->option_mode_1)
> -		len += sprintf(buf + len, " %d",
> +		len += sysfs_emit_at(buf, len, " %d",
>  			       st->info->option_mode_1_3db_freq);
Fix alignment.

>  	if (st->info->option_mode_2)
> -		len += sprintf(buf + len, " %d",
> +		len += sysfs_emit_at(buf, len, " %d",
>  			       st->info->option_mode_2_3db_freq);

same here.

> -	len += sprintf(buf + len, "\n");
> +	len += sysfs_emit_at(buf, len, "\n");
>  
>  	return len;
>  }
> @@ -783,7 +783,6 @@ static ssize_t sca3000_read_av_freq(struct device *dev,
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct sca3000_state *st = iio_priv(indio_dev);
> -	unsigned int len = 0;
>  	int ret;
>  
>  	scoped_guard(mutex, &st->lock) {
> @@ -794,25 +793,22 @@ static ssize_t sca3000_read_av_freq(struct device *dev,
>  
>  	switch (ret & SCA3000_REG_MODE_MODE_MASK) {
>  	case SCA3000_REG_MODE_MEAS_MODE_NORMAL:
> -		len += sprintf(buf + len, "%d %d %d\n",
> +		return sysfs_emit(buf, "%d %d %d\n",
>  			       st->info->measurement_mode_freq,
>  			       st->info->measurement_mode_freq / 2,
>  			       st->info->measurement_mode_freq / 4);
> -		break;
>  	case SCA3000_REG_MODE_MEAS_MODE_OP_1:
> -		len += sprintf(buf + len, "%d %d %d\n",
> +		return sysfs_emit(buf, "%d %d %d\n",
>  			       st->info->option_mode_1_freq,
>  			       st->info->option_mode_1_freq / 2,
>  			       st->info->option_mode_1_freq / 4);
> -		break;
>  	case SCA3000_REG_MODE_MEAS_MODE_OP_2:
> -		len += sprintf(buf + len, "%d %d %d\n",
> +		return sysfs_emit(buf, "%d %d %d\n",
>  			       st->info->option_mode_2_freq,
>  			       st->info->option_mode_2_freq / 2,
>  			       st->info->option_mode_2_freq / 4);
> -		break;
>  	}
> -	return len;
> +	return 0;
>  }
>  
>  /*

Re: [PATCH v6 4/4] iio: accel: sca3000: use sysfs_emit_at() instead of sprintf()

Posted by Andrew Ijano 3 months ago

On Sat, Jun 21, 2025 at 2:58 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > @@ -423,16 +423,16 @@ sca3000_show_available_3db_freqs(struct device *dev,
> >  {
> >       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> >       struct sca3000_state *st = iio_priv(indio_dev);
> > -     int len;
> > +     unsigned int len = 0;
>
> No need to initialize as set on the next line

That makes sense! I´ll change that.

>
> >
> > -     len = sprintf(buf, "%d", st->info->measurement_mode_3db_freq);
> > +     len = sysfs_emit_at(buf, len, "%d", st->info->measurement_mode_3db_freq);
>
> sysfs_emit() when you know you are at the start.
>
Ok! Thanks.

>
> >       if (st->info->option_mode_1)
> > -             len += sprintf(buf + len, " %d",
> > +             len += sysfs_emit_at(buf, len, " %d",
> >                              st->info->option_mode_1_3db_freq);
> Fix alignment.
>
> >       if (st->info->option_mode_2)
> > -             len += sprintf(buf + len, " %d",
> > +             len += sysfs_emit_at(buf, len, " %d",
> >                              st->info->option_mode_2_3db_freq);
>
> same here.

Actually, both cases are aligned. I checked the code and they have the
same number of tabs, and in this email they have the same number of
spaces.
However, since I'm not reading this diff with a monospaced font, for
me it appears to be different but this is caused by the difference in
size of "-" and "+".
Maybe this is why it appears to be different for you too?

Thanks,
Andrew

Re: [PATCH v6 4/4] iio: accel: sca3000: use sysfs_emit_at() instead of sprintf()

Posted by Jonathan Cameron 3 months ago

On Sat, 5 Jul 2025 00:45:05 -0300
Andrew Ijano <andrew.ijano@gmail.com> wrote:

> On Sat, Jun 21, 2025 at 2:58 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > @@ -423,16 +423,16 @@ sca3000_show_available_3db_freqs(struct device *dev,
> > >  {
> > >       struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > >       struct sca3000_state *st = iio_priv(indio_dev);
> > > -     int len;
> > > +     unsigned int len = 0;  
> >
> > No need to initialize as set on the next line  
> 
> That makes sense! I´ll change that.
> 
> >  
> > >
> > > -     len = sprintf(buf, "%d", st->info->measurement_mode_3db_freq);
> > > +     len = sysfs_emit_at(buf, len, "%d", st->info->measurement_mode_3db_freq);  
> >
> > sysfs_emit() when you know you are at the start.
> >  
> Ok! Thanks.
> 
> >  
> > >       if (st->info->option_mode_1)
> > > -             len += sprintf(buf + len, " %d",
> > > +             len += sysfs_emit_at(buf, len, " %d",
> > >                              st->info->option_mode_1_3db_freq);  
> > Fix alignment.
> >  
> > >       if (st->info->option_mode_2)
> > > -             len += sprintf(buf + len, " %d",
> > > +             len += sysfs_emit_at(buf, len, " %d",
> > >                              st->info->option_mode_2_3db_freq);  
> >
> > same here.  
> 
> Actually, both cases are aligned. I checked the code and they have the
> same number of tabs, and in this email they have the same number of
> spaces.
> However, since I'm not reading this diff with a monospaced font, for
> me it appears to be different but this is caused by the difference in
> size of "-" and "+".
> Maybe this is why it appears to be different for you too?
Key as in earlier reply is parameters that need to go on an additional line
are not aligned as particular number of tabs, they are aligned to under
the parameters on the first line.

The only normal reason to break with that rule is on particularly long
lines.

Jonathan

> 
> Thanks,
> Andrew