iio: st_lsm6dsx: Replace scnprintf with sysfs_emit

PATCH Superseded Akshay Bansod 3 participants 6 messages Jul 23, 2025 – Aug 11, 2025

Subsystems (2)

IIO ST LSM6DSx IMU IIO driver

lore

Share & tools

lore

Akshay Bansod

author

2025-07-23 14:14 UTC #0 lore raw

PATCH v3 1/1 iio: st_lsm6dsx: Replace scnprintf with sysfs_emit

Update the sysfs interface for sampling frequency and scale attributes. Replace scnprintf() with sysfs_emit_at() which is PAGE_SIZE-aware and recommended for use in sysfs.

Signed-off-by Akshay Bansod <[email protected]>

---

changes in v3:
- Use sysfs_emit_at(buf, len - 1, "\n") instead of directly modifying buf[len - 1] for newline termination, aligning with sysfs_emit_at() usage.
- Link to v2: https://lore.kernel.org/linux-iio/[email protected]/

changes in v2:
- Fixed indentation for line wrap
- Link to v1: https://lore.kernel.org/linux-iio/[email protected]/

Testing:
- Built the driver (st_lsm6dsx_i2c) as a module.
- Tested using i2c-stub to mock the device.
- Verified that reading sysfs attributes like sampling_frequency_available works correctly and shows no change in functionality.

---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +8 -7

index c65ad4982..f0aab41f3 100644
@@ -2035,10 +2035,10 @@ st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev,
 
 	odr_table = &sensor->hw->settings->odr_table[sensor->id];
 	for (i = 0; i < odr_table->odr_len; i++)
-		len += scnprintf(buf + len, PAGE_SIZE - len, "%d.%03d ",
-				 odr_table->odr_avl[i].milli_hz / 1000,
-				 odr_table->odr_avl[i].milli_hz % 1000);
-	buf[len - 1] = '\n';
+		len += sysfs_emit_at(buf, len, "%d.%03d ",
+				     odr_table->odr_avl[i].milli_hz / 1000,
+				     odr_table->odr_avl[i].milli_hz % 1000);
+	sysfs_emit_at(buf, len - 1, "\n");
 
 	return len;
 }

@@ -2054,9 +2054,10 @@ static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev,
 
 	fs_table = &hw->settings->fs_table[sensor->id];
 	for (i = 0; i < fs_table->fs_len; i++)
-		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ",
-				 fs_table->fs_avl[i].gain);
-	buf[len - 1] = '\n';
+		len += sysfs_emit_at(buf, len, "0.%09u ",
+				     fs_table->fs_avl[i].gain);
+
+	sysfs_emit_at(buf, len - 1, "\n");
 
 	return len;
 }

Signature · 1 line

2.49.0

Andy Shevchenko

2025-07-23 14:42 UTC #1 lore raw

Quoted 3 lines · Akshay Bansod

Update the sysfs interface for sampling frequency and scale attributes. Replace scnprintf() with sysfs_emit_at() which is PAGE_SIZE-aware and recommended for use in sysfs.

...

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev, · line 2060

Quoted 9 lines

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +4 -3

 	fs_table = &hw->settings->fs_table[sensor->id];
 	for (i = 0; i < fs_table->fs_len; i++)
-		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ",
-				 fs_table->fs_avl[i].gain);
-	buf[len - 1] = '\n';
+		len += sysfs_emit_at(buf, len, "0.%09u ",
+				     fs_table->fs_avl[i].gain);
+
+	sysfs_emit_at(buf, len - 1, "\n");

Still looks a bit weird (while working).

Quoted 1 line

return len;

I deally we should have a helper doing all this under the hood for plenty of the (existing) users in the kernel.

In any case, I leave this change to others to comment, I don't object pushing it in this form, either way len - 1 is simply weird.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Jonathan Cameron

maintainer

2025-07-23 15:41 UTC #2 lore raw

Quoted 19 lines · On Wed, 23 Jul 2025 17:42:28 +0300 Andy Shevchenko…

Quoted 3 lines · Akshay Bansod
Update the sysfs interface for sampling frequency and scale attributes. Replace scnprintf() with sysfs_emit_at() which is PAGE_SIZE-aware and recommended for use in sysfs.
...
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev, · line 2060
Quoted 9 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +4 -3
 	fs_table = &hw->settings->fs_table[sensor->id];
 	for (i = 0; i < fs_table->fs_len; i++)
-		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ",
-				 fs_table->fs_avl[i].gain);
-	buf[len - 1] = '\n';
+		len += sysfs_emit_at(buf, len, "0.%09u ",
+				     fs_table->fs_avl[i].gain);
+
+	sysfs_emit_at(buf, len - 1, "\n");  
Still looks a bit weird (while working).

Quoted 4 lines

Quoted 1 line
return len;
I deally we should have a helper doing all this under the hood for plenty of the (existing) users in the kernel.

hmm I'm not sure generic is terribly easy and I'd prefer this using the read_avail callbacks that require the data in an array where ever possible. Mind you that does the same print at len - 1 as this. Let's play. Completely untested.

	for (i = 0; i < fs_table->fs_len; i++)
		len += sysfs_emit_at(buf, len, "0x%09u%c",
				     fs_table->fs_avl[i].gain,
				     ((i == fs_table->fs_len - 1) ? '\n', ' '));

better?

It's definitely not more readable than the above, but it does avoid the write to len - 1.

Quoted 5 lines

In any case, I leave this change to others to comment, I don't object pushing it in this form, either way len - 1 is simply weird.

Andy Shevchenko

2025-07-24 11:47 UTC #3 lore raw

Quoted 3 lines · Jonathan Cameron

Quoted 1 line · On Wed, 23 Jul 2025 17:42:28 +0300 Andy Shevchenko…
On Wed, Jul 23, 2025 at 07:43:59PM +0530, Akshay Bansod wrote:

...

Quoted 12 lines

Quoted 12 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev, · line 2060
Quoted 9 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +4 -3
 	fs_table = &hw->settings->fs_table[sensor->id];
 	for (i = 0; i < fs_table->fs_len; i++)
-		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ",
-				 fs_table->fs_avl[i].gain);
-	buf[len - 1] = '\n';
+		len += sysfs_emit_at(buf, len, "0.%09u ",
+				     fs_table->fs_avl[i].gain);
+
+	sysfs_emit_at(buf, len - 1, "\n");  
Still looks a bit weird (while working).

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
return len;
I deally we should have a helper doing all this under the hood for plenty of the (existing) users in the kernel.
hmm I'm not sure generic is terribly easy

I agree, I have some plans for %p specifier extension, but I was stuck with it and it in half-basked state in some of my local Git branches.

Quoted 11 lines

and I'd prefer this using the
read_avail callbacks that require the data in an array where ever possible. Mind you that does the same print at len - 1 as this. Let's play. Completely untested.
	for (i = 0; i < fs_table->fs_len; i++)
		len += sysfs_emit_at(buf, len, "0x%09u%c",
				     fs_table->fs_avl[i].gain,
				     ((i == fs_table->fs_len - 1) ? '\n', ' '));
better?

Without extra parentheses this makes the job.

Quoted 5 lines

It's definitely not more readable than the above, but it does avoid the write to len - 1.
Quoted 2 lines
In any case, I leave this change to others to comment, I don't object pushing it in this form, either way len - 1 is simply weird.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Jonathan Cameron

maintainer

2025-08-02 11:50 UTC #4 lore raw

Quoted 34 lines · On Thu, 24 Jul 2025 14:47:31 +0300 Andy Shevchenko…

Quoted 3 lines · Jonathan Cameron
Quoted 1 line · On Wed, 23 Jul 2025 17:42:28 +0300 Andy Shevchenko…
On Wed, Jul 23, 2025 at 07:43:59PM +0530, Akshay Bansod wrote:
...
Quoted 12 lines
Quoted 12 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev, · line 2060
Quoted 9 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +4 -3
 	fs_table = &hw->settings->fs_table[sensor->id];
 	for (i = 0; i < fs_table->fs_len; i++)
-		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ",
-				 fs_table->fs_avl[i].gain);
-	buf[len - 1] = '\n';
+		len += sysfs_emit_at(buf, len, "0.%09u ",
+				     fs_table->fs_avl[i].gain);
+
+	sysfs_emit_at(buf, len - 1, "\n");    
Still looks a bit weird (while working).
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
return len;
I deally we should have a helper doing all this under the hood for plenty of the (existing) users in the kernel.
hmm I'm not sure generic is terribly easy
I agree, I have some plans for %p specifier extension, but I was stuck with it and it in half-basked state in some of my local Git branches.
Quoted 5 lines
and I'd prefer this using the
read_avail callbacks that require the data in an array where ever possible. Mind you that does the same print at len - 1 as this. Let's play. Completely untested.

Quoted 9 lines

Quoted 6 lines

	for (i = 0; i < fs_table->fs_len; i++)
		len += sysfs_emit_at(buf, len, "0x%09u%c",
				     fs_table->fs_avl[i].gain,
				     ((i == fs_table->fs_len - 1) ? '\n', ' '));

better?

Without extra parentheses this makes the job.

Akshay, can you spin a new version along those lines?

Quoted 7 lines

Quoted 6 lines
It's definitely not more readable than the above, but it does avoid the write to len - 1.
Quoted 2 lines
In any case, I leave this change to others to comment, I don't object pushing it in this form, either way len - 1 is simply weird.

akshay bansod

author

2025-08-11 17:00 UTC #5 lore raw

Quoted 42 lines · On Saturday, 2 August 2025 10:26 pm +0530 Jonathan Cameron wrote:

Quoted 39 lines · On Thu, 24 Jul 2025 14:47:31 +0300 Andy Shevchenko…
Quoted 3 lines · Jonathan Cameron
Quoted 1 line · On Wed, 23 Jul 2025 17:42:28 +0300 Andy Shevchenko…
On Wed, Jul 23, 2025 at 07:43:59PM +0530, Akshay Bansod wrote:
...
Quoted 12 lines
Quoted 12 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev, · line 2060
Quoted 9 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +4 -3
 	fs_table = &hw->settings->fs_table[sensor->id];
 	for (i = 0; i < fs_table->fs_len; i++)
-		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ",
-				 fs_table->fs_avl[i].gain);
-	buf[len - 1] = '\n';
+		len += sysfs_emit_at(buf, len, "0.%09u ",
+				     fs_table->fs_avl[i].gain);
+
+	sysfs_emit_at(buf, len - 1, "\n");    
Still looks a bit weird (while working).
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
return len;
I deally we should have a helper doing all this under the hood for plenty of the (existing) users in the kernel.
hmm I'm not sure generic is terribly easy
I agree, I have some plans for %p specifier extension, but I was stuck with it and it in half-basked state in some of my local Git branches.
Quoted 10 lines
and I'd prefer this using the
read_avail callbacks that require the data in an array where ever possible. Mind you that does the same print at len - 1 as this. Let's play. Completely untested.
	for (i = 0; i < fs_table->fs_len; i++)
		len += sysfs_emit_at(buf, len, "0x%09u%c",
				     fs_table->fs_avl[i].gain,
				     ((i == fs_table->fs_len - 1) ? '\n', ' '));

Quoted 5 lines

Quoted 4 lines
Quoted 1 line
better?
Without extra parentheses this makes the job.
Akshay, can you spin a new version along those lines?

Apologies for the delay. Here's the revision
https://lore.kernel.org/linux-iio/[email protected]/

Quoted 9 lines

Quoted 7 lines
Quoted 6 lines
It's definitely not more readable than the above, but it does avoid the write to len - 1.
Quoted 2 lines
In any case, I leave this change to others to comment, I don't object pushing it in this form, either way len - 1 is simply weird.