[PATCH 0/3] staging: iio: replace sprintf() with sysfs_emit()

Gabriel Rondon posted 3 patches 2 weeks, 1 day ago
There is a newer version of this series
drivers/staging/iio/adc/ad7816.c              | 16 ++++++-------
drivers/staging/iio/frequency/ad9834.c        |  4 ++--
.../staging/iio/impedance-analyzer/ad5933.c   | 24 +++++++++----------
3 files changed, 22 insertions(+), 22 deletions(-)
[PATCH 0/3] staging: iio: replace sprintf() with sysfs_emit()
Posted by Gabriel Rondon 2 weeks, 1 day ago
Replace the remaining sprintf() calls with sysfs_emit() in sysfs
show functions across three staging IIO drivers: ad7816, ad5933, and
ad9834.

sysfs_emit() is the preferred API for sysfs callbacks as it is aware
of the PAGE_SIZE buffer limit. These are direct replacements with no
functional change. Build tested with gcc 13.3.

Gabriel Rondon (3):
  staging: iio: ad7816: use sysfs_emit() in show functions
  staging: iio: ad5933: use sysfs_emit() in show functions
  staging: iio: ad9834: use sysfs_emit() in show functions

 drivers/staging/iio/adc/ad7816.c              | 16 ++++++-------
 drivers/staging/iio/frequency/ad9834.c        |  4 ++--
 .../staging/iio/impedance-analyzer/ad5933.c   | 24 +++++++++----------
 3 files changed, 22 insertions(+), 22 deletions(-)

-- 
2.33.0
[PATCH v2 0/3] staging: iio: replace sprintf() with sysfs_emit()
Posted by Gabriel Rondon 2 weeks ago
Replace the remaining sprintf() calls with sysfs_emit() in sysfs
show functions across three staging IIO drivers: ad7816, ad5933, and
ad9834.

sysfs_emit() is the preferred API for sysfs callbacks as it is aware
of the PAGE_SIZE buffer limit. Build tested with gcc 13.3.

Changes since v1:
- ad5933: remove unnecessary (int) cast in ad5933_show_frequency()
  and use %llu format specifier for unsigned long long (Andy Shevchenko)
- ad9834: simplify wavetype_available show functions by removing
  intermediate string variable and returning directly (Andy Shevchenko)

Gabriel Rondon (3):
  staging: iio: ad7816: use sysfs_emit() in show functions
  staging: iio: ad5933: use sysfs_emit() in show functions
  staging: iio: ad9834: use sysfs_emit() and simplify show functions

 drivers/staging/iio/adc/ad7816.c              | 16 ++++++-------
 drivers/staging/iio/frequency/ad9834.c        | 20 +++++-----------
 .../staging/iio/impedance-analyzer/ad5933.c   | 24 +++++++++----------
 3 files changed, 26 insertions(+), 34 deletions(-)

-- 
2.33.0
Re: [PATCH v2 0/3] staging: iio: replace sprintf() with sysfs_emit()
Posted by Jonathan Cameron 1 week, 6 days ago
On Fri, 20 Mar 2026 22:24:21 +0000
Gabriel Rondon <grondon@gmail.com> wrote:

> Replace the remaining sprintf() calls with sysfs_emit() in sysfs
> show functions across three staging IIO drivers: ad7816, ad5933, and
> ad9834.
> 
> sysfs_emit() is the preferred API for sysfs callbacks as it is aware
> of the PAGE_SIZE buffer limit. Build tested with gcc 13.3.
> 
Firstly a process thing. Don't send a v2 in reply to a v1. It should
be a new email thread.  I'm not sure where this style is coming from
as I've never encountered any kernel subsystem who ask for it.

The basic reason is threads get very confusing if they mix versions +
a more practical one is that your email appears a bunch of screens up
in the email client and many reviewers (who don't have time to look at
everything) start with most recent emails when looking for things to
review.  Thus you get ignored.

Jonathan

> Changes since v1:
> - ad5933: remove unnecessary (int) cast in ad5933_show_frequency()
>   and use %llu format specifier for unsigned long long (Andy Shevchenko)
> - ad9834: simplify wavetype_available show functions by removing
>   intermediate string variable and returning directly (Andy Shevchenko)
> 
> Gabriel Rondon (3):
>   staging: iio: ad7816: use sysfs_emit() in show functions
>   staging: iio: ad5933: use sysfs_emit() in show functions
>   staging: iio: ad9834: use sysfs_emit() and simplify show functions
> 
>  drivers/staging/iio/adc/ad7816.c              | 16 ++++++-------
>  drivers/staging/iio/frequency/ad9834.c        | 20 +++++-----------
>  .../staging/iio/impedance-analyzer/ad5933.c   | 24 +++++++++----------
>  3 files changed, 26 insertions(+), 34 deletions(-)
>
Re: [PATCH v2 0/3] staging: iio: replace sprintf() with sysfs_emit()
Posted by Dan Carpenter 1 week, 6 days ago
On Sat, Mar 21, 2026 at 01:03:07PM +0000, Jonathan Cameron wrote:
> On Fri, 20 Mar 2026 22:24:21 +0000
> Gabriel Rondon <grondon@gmail.com> wrote:
> 
> > Replace the remaining sprintf() calls with sysfs_emit() in sysfs
> > show functions across three staging IIO drivers: ad7816, ad5933, and
> > ad9834.
> > 
> > sysfs_emit() is the preferred API for sysfs callbacks as it is aware
> > of the PAGE_SIZE buffer limit. Build tested with gcc 13.3.
> > 
> Firstly a process thing. Don't send a v2 in reply to a v1. It should
> be a new email thread.  I'm not sure where this style is coming from
> as I've never encountered any kernel subsystem who ask for it.
> 

People used to do that in the early days of git, but now no one does.
It breaks patchwork as well.

regards,
dan carpenter
[PATCH v2 1/3] staging: iio: ad7816: use sysfs_emit() in show functions
Posted by Gabriel Rondon 2 weeks ago
Replace sprintf() with sysfs_emit() in all sysfs attribute show
functions. sysfs_emit() is the preferred API for sysfs callbacks as
it is aware of the PAGE_SIZE buffer limit. No functional change.

Signed-off-by: Gabriel Rondon <grondon@gmail.com>
---
 drivers/staging/iio/adc/ad7816.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 172acf135..0e32a2295 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -124,8 +124,8 @@ static ssize_t ad7816_show_mode(struct device *dev,
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
 
 	if (chip->mode)
-		return sprintf(buf, "power-save\n");
-	return sprintf(buf, "full\n");
+		return sysfs_emit(buf, "power-save\n");
+	return sysfs_emit(buf, "full\n");
 }
 
 static ssize_t ad7816_store_mode(struct device *dev,
@@ -156,7 +156,7 @@ static ssize_t ad7816_show_available_modes(struct device *dev,
 					   struct device_attribute *attr,
 					   char *buf)
 {
-	return sprintf(buf, "full\npower-save\n");
+	return sysfs_emit(buf, "full\npower-save\n");
 }
 
 static IIO_DEVICE_ATTR(available_modes, 0444, ad7816_show_available_modes,
@@ -169,7 +169,7 @@ static ssize_t ad7816_show_channel(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7816_chip_info *chip = iio_priv(indio_dev);
 
-	return sprintf(buf, "%d\n", chip->channel_id);
+	return sysfs_emit(buf, "%d\n", chip->channel_id);
 }
 
 static ssize_t ad7816_store_channel(struct device *dev,
@@ -231,9 +231,9 @@ static ssize_t ad7816_show_value(struct device *dev,
 		data &= AD7816_TEMP_FLOAT_MASK;
 		if (value < 0)
 			data = BIT(AD7816_TEMP_FLOAT_OFFSET) - data;
-		return sprintf(buf, "%d.%.2d\n", value, data * 25);
+		return sysfs_emit(buf, "%d.%.2d\n", value, data * 25);
 	}
-	return sprintf(buf, "%u\n", data);
+	return sysfs_emit(buf, "%u\n", data);
 }
 
 static IIO_DEVICE_ATTR(value, 0444, ad7816_show_value, NULL, 0);
@@ -281,9 +281,9 @@ static ssize_t ad7816_show_oti(struct device *dev,
 		value = AD7816_BOUND_VALUE_MIN +
 			(chip->oti_data[chip->channel_id] -
 			AD7816_BOUND_VALUE_BASE);
-		return sprintf(buf, "%d\n", value);
+		return sysfs_emit(buf, "%d\n", value);
 	}
-	return sprintf(buf, "%u\n", chip->oti_data[chip->channel_id]);
+	return sysfs_emit(buf, "%u\n", chip->oti_data[chip->channel_id]);
 }
 
 static inline ssize_t ad7816_set_oti(struct device *dev,
-- 
2.33.0
Re: [PATCH v2 1/3] staging: iio: ad7816: use sysfs_emit() in show functions
Posted by Jonathan Cameron 1 week, 6 days ago
On Fri, 20 Mar 2026 22:24:22 +0000
Gabriel Rondon <grondon@gmail.com> wrote:

> Replace sprintf() with sysfs_emit() in all sysfs attribute show
> functions. sysfs_emit() is the preferred API for sysfs callbacks as
> it is aware of the PAGE_SIZE buffer limit. No functional change.
> 
> Signed-off-by: Gabriel Rondon <grondon@gmail.com>
Always worth a quick check to see if you are overlapping with a series
that might be on the list or already applied.

In this case:
https://lore.kernel.org/all/20260301212819.98597-1-ehanoc@protonmail.com/
[PATCH v2 2/3] staging: iio: ad5933: use sysfs_emit() in show functions
Posted by Gabriel Rondon 2 weeks ago
Replace sprintf() with sysfs_emit() in all sysfs attribute show
functions. sysfs_emit() is the preferred API for sysfs callbacks as
it is aware of the PAGE_SIZE buffer limit.

Also remove the unnecessary (int) cast in ad5933_show_frequency()
and use the correct format specifier %llu for the unsigned long long
freqreg variable.

Signed-off-by: Gabriel Rondon <grondon@gmail.com>
---
 .../staging/iio/impedance-analyzer/ad5933.c   | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 85a422329..e5a4f8d7a 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -285,7 +285,7 @@ static ssize_t ad5933_show_frequency(struct device *dev,
 	freqreg = (u64)freqreg * (u64)(st->mclk_hz / 4);
 	do_div(freqreg, BIT(27));
 
-	return sprintf(buf, "%d\n", (int)freqreg);
+	return sysfs_emit(buf, "%llu\n", freqreg);
 }
 
 static ssize_t ad5933_store_frequency(struct device *dev,
@@ -338,27 +338,27 @@ static ssize_t ad5933_show(struct device *dev,
 	mutex_lock(&st->lock);
 	switch ((u32)this_attr->address) {
 	case AD5933_OUT_RANGE:
-		len = sprintf(buf, "%u\n",
-			      st->range_avail[(st->ctrl_hb >> 1) & 0x3]);
+		len = sysfs_emit(buf, "%u\n",
+				 st->range_avail[(st->ctrl_hb >> 1) & 0x3]);
 		break;
 	case AD5933_OUT_RANGE_AVAIL:
-		len = sprintf(buf, "%u %u %u %u\n", st->range_avail[0],
-			      st->range_avail[3], st->range_avail[2],
-			      st->range_avail[1]);
+		len = sysfs_emit(buf, "%u %u %u %u\n", st->range_avail[0],
+				 st->range_avail[3], st->range_avail[2],
+				 st->range_avail[1]);
 		break;
 	case AD5933_OUT_SETTLING_CYCLES:
-		len = sprintf(buf, "%d\n", st->settling_cycles);
+		len = sysfs_emit(buf, "%d\n", st->settling_cycles);
 		break;
 	case AD5933_IN_PGA_GAIN:
-		len = sprintf(buf, "%s\n",
-			      (st->ctrl_hb & AD5933_CTRL_PGA_GAIN_1) ?
-			      "1" : "0.2");
+		len = sysfs_emit(buf, "%s\n",
+				 (st->ctrl_hb & AD5933_CTRL_PGA_GAIN_1) ?
+				 "1" : "0.2");
 		break;
 	case AD5933_IN_PGA_GAIN_AVAIL:
-		len = sprintf(buf, "1 0.2\n");
+		len = sysfs_emit(buf, "1 0.2\n");
 		break;
 	case AD5933_FREQ_POINTS:
-		len = sprintf(buf, "%d\n", st->freq_points);
+		len = sysfs_emit(buf, "%d\n", st->freq_points);
 		break;
 	default:
 		ret = -EINVAL;
-- 
2.33.0
Re: [PATCH v2 2/3] staging: iio: ad5933: use sysfs_emit() in show functions
Posted by Jonathan Cameron 1 week, 6 days ago
On Fri, 20 Mar 2026 22:24:23 +0000
Gabriel Rondon <grondon@gmail.com> wrote:

> Replace sprintf() with sysfs_emit() in all sysfs attribute show
> functions. sysfs_emit() is the preferred API for sysfs callbacks as
> it is aware of the PAGE_SIZE buffer limit.
> 
> Also remove the unnecessary (int) cast in ad5933_show_frequency()
> and use the correct format specifier %llu for the unsigned long long
> freqreg variable.
> 
> Signed-off-by: Gabriel Rondon <grondon@gmail.com>

You should have replied to Andy's comments to say why you aren't
doing the more substantial refactors he asked for.

From my point of view, this driver is far enough from compliant ABI
that the changes suggested need major rewrites of the driver.
So it is very likely all this code will get ripped out before this
leaves staging.  No particular reason you should know that though
unless you've looked much more closely at what is needed.

Hence my main motivation in picking this up is we won't take other
people's time converting these. I doubt anyone is using staging
drivers as a source of information so the usual point of ensuring
best practice in code that might get copied doesn't apply.

Anyhow with all that in mind. Applied to the testing branch of iio.git.

Thanks,

Jonathan

> ---
>  .../staging/iio/impedance-analyzer/ad5933.c   | 24 +++++++++----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index 85a422329..e5a4f8d7a 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -285,7 +285,7 @@ static ssize_t ad5933_show_frequency(struct device *dev,
>  	freqreg = (u64)freqreg * (u64)(st->mclk_hz / 4);
>  	do_div(freqreg, BIT(27));
>  
> -	return sprintf(buf, "%d\n", (int)freqreg);
> +	return sysfs_emit(buf, "%llu\n", freqreg);
>  }
>  
>  static ssize_t ad5933_store_frequency(struct device *dev,
> @@ -338,27 +338,27 @@ static ssize_t ad5933_show(struct device *dev,
>  	mutex_lock(&st->lock);
>  	switch ((u32)this_attr->address) {
>  	case AD5933_OUT_RANGE:
> -		len = sprintf(buf, "%u\n",
> -			      st->range_avail[(st->ctrl_hb >> 1) & 0x3]);
> +		len = sysfs_emit(buf, "%u\n",
> +				 st->range_avail[(st->ctrl_hb >> 1) & 0x3]);
>  		break;
>  	case AD5933_OUT_RANGE_AVAIL:
> -		len = sprintf(buf, "%u %u %u %u\n", st->range_avail[0],
> -			      st->range_avail[3], st->range_avail[2],
> -			      st->range_avail[1]);
> +		len = sysfs_emit(buf, "%u %u %u %u\n", st->range_avail[0],
> +				 st->range_avail[3], st->range_avail[2],
> +				 st->range_avail[1]);
>  		break;
>  	case AD5933_OUT_SETTLING_CYCLES:
> -		len = sprintf(buf, "%d\n", st->settling_cycles);
> +		len = sysfs_emit(buf, "%d\n", st->settling_cycles);
>  		break;
>  	case AD5933_IN_PGA_GAIN:
> -		len = sprintf(buf, "%s\n",
> -			      (st->ctrl_hb & AD5933_CTRL_PGA_GAIN_1) ?
> -			      "1" : "0.2");
> +		len = sysfs_emit(buf, "%s\n",
> +				 (st->ctrl_hb & AD5933_CTRL_PGA_GAIN_1) ?
> +				 "1" : "0.2");
>  		break;
>  	case AD5933_IN_PGA_GAIN_AVAIL:
> -		len = sprintf(buf, "1 0.2\n");
> +		len = sysfs_emit(buf, "1 0.2\n");
>  		break;
>  	case AD5933_FREQ_POINTS:
> -		len = sprintf(buf, "%d\n", st->freq_points);
> +		len = sysfs_emit(buf, "%d\n", st->freq_points);
>  		break;
>  	default:
>  		ret = -EINVAL;
[PATCH v2 3/3] staging: iio: ad9834: use sysfs_emit() and simplify show functions
Posted by Gabriel Rondon 2 weeks ago
Replace sprintf() with sysfs_emit() in sysfs attribute show functions.
sysfs_emit() is the preferred API for sysfs callbacks as it is aware
of the PAGE_SIZE buffer limit.

Also simplify the wavetype_available show functions by removing
the intermediate string variable and returning directly from each
branch.

Signed-off-by: Gabriel Rondon <grondon@gmail.com>
---
 drivers/staging/iio/frequency/ad9834.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index d339d5e8e..bdb2580e2 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -281,16 +281,12 @@ ssize_t ad9834_show_out0_wavetype_available(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad9834_state *st = iio_priv(indio_dev);
-	char *str;
 
 	if (st->devid == ID_AD9833 || st->devid == ID_AD9837)
-		str = "sine triangle square";
-	else if (st->control & AD9834_OPBITEN)
-		str = "sine";
-	else
-		str = "sine triangle";
-
-	return sprintf(buf, "%s\n", str);
+		return sysfs_emit(buf, "sine triangle square\n");
+	if (st->control & AD9834_OPBITEN)
+		return sysfs_emit(buf, "sine\n");
+	return sysfs_emit(buf, "sine triangle\n");
 }
 
 static IIO_DEVICE_ATTR(out_altvoltage0_out0_wavetype_available, 0444,
@@ -303,14 +299,10 @@ ssize_t ad9834_show_out1_wavetype_available(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad9834_state *st = iio_priv(indio_dev);
-	char *str;
 
 	if (st->control & AD9834_MODE)
-		str = "";
-	else
-		str = "square";
-
-	return sprintf(buf, "%s\n", str);
+		return sysfs_emit(buf, "\n");
+	return sysfs_emit(buf, "square\n");
 }
 
 static IIO_DEVICE_ATTR(out_altvoltage0_out1_wavetype_available, 0444,
-- 
2.33.0
Re: [PATCH v2 3/3] staging: iio: ad9834: use sysfs_emit() and simplify show functions
Posted by Jonathan Cameron 1 week, 6 days ago
On Fri, 20 Mar 2026 22:24:24 +0000
Gabriel Rondon <grondon@gmail.com> wrote:

> Replace sprintf() with sysfs_emit() in sysfs attribute show functions.
> sysfs_emit() is the preferred API for sysfs callbacks as it is aware
> of the PAGE_SIZE buffer limit.
> 
> Also simplify the wavetype_available show functions by removing
> the intermediate string variable and returning directly from each
> branch.
> 
> Signed-off-by: Gabriel Rondon <grondon@gmail.com>
Similar to patch 2, I suspect this code will go through a lot of changes
if anyone does the work to move this driver out of staging.

So I'm applying this mostly to avoid anyone else sending patches
to do the same!

Applied.

Thanks,

Jonathan