iio: st_lsm6dsx: Replace scnprintf with sysfs_emit
Subsystems (2)
PATCH 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.
---
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 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +2 -2
@@ -2035,7 +2035,7 @@ 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 " , + len +=sysfs_emit_at (buf, len,"%d.%03d " , odr_table->odr_avl[i].milli_hz /1000 , odr_table->odr_avl[i].milli_hz %1000 ); buf[len -1 ] ='\n' ;
@@ -2054,7 +2054,7 @@ 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 " , + len +=sysfs_emit_at (buf, len,"0.%09u " , fs_table->fs_avl[i].gain); buf[len -1 ] ='\n' ;
Signature · 1 line
2.49.0
David Lechner
reviewerQuoted 4 lines · Akshay Bansod
Update the sysfs interface for sampling frequency and scale attributes. Replace
scnprintf()withsysfs_emit_at()which is PAGE_SIZE-aware and recommended for use in sysfs.
Quoted 8 lines
Signed-off-by Akshay Bansod <[email protected]>---
Testing:
- Built the driver (st_lsm6dsx_i2c) as a module.
- Tested usingi2c-stubto mock the device.
- Verified that reading sysfs attributes likesampling_frequency_availableworks correctly and shows no change in functionality.
Nice to see it was actually tested. :-)
Quoted 9 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +0
-0
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev, · line 2040
Quoted 7 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-1@@ -2035,7 +2035,7 @@ 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 " , + len +=sysfs_emit_at (buf, len,"%d.%03d " , odr_table->odr_avl[i].milli_hz /1000 , odr_table->odr_avl[i].milli_hz %1000 );
Let's keep checkpatch happy and change the indent of the wrapped lines to line up with ( since the ( moved.
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev, · line 2058
Quoted 8 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-1buf[len -1 ] ='\n' ;@@ -2054,7 +2054,7 @@ 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 " , + len +=sysfs_emit_at (buf, len,"0.%09u " , fs_table->fs_avl[i].gain);
ditto
Quoted 2 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +0
-0buf[len -1 ] ='\n' ;
Quoted 4 lines · David Lechner
Quoted 3 lines · Akshay Bansod
Update the sysfs interface for sampling frequency and scale attributes. Replace
scnprintf()withsysfs_emit_at()which is PAGE_SIZE-aware and recommended for use in sysfs.
...
Quoted 6 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev, · line 2040
Quoted 3 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-0+ len +=sysfs_emit_at (buf, len,"%d.%03d " , odr_table->odr_avl[i].milli_hz /1000 , odr_table->odr_avl[i].milli_hz %1000 );Let's keep checkpatch happy and change the indent of the wrapped lines to line up with ( since the ( moved.
While I see the point, wouldn't be better to have 1000 replaced with MILLI at the same time?
Signature · 2 lines
With Best Regards, Andy Shevchenko
David Lechner
reviewerQuoted 15 lines · Andy Shevchenko
Quoted 4 lines · David Lechner
Quoted 3 lines · Akshay Bansod
Update the sysfs interface for sampling frequency and scale attributes. Replace
scnprintf()withsysfs_emit_at()which is PAGE_SIZE-aware and recommended for use in sysfs....
Quoted 6 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev, · line 2040
Quoted 3 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-0+ len +=sysfs_emit_at (buf, len,"%d.%03d " , odr_table->odr_avl[i].milli_hz /1000 , odr_table->odr_avl[i].milli_hz %1000 );Let's keep checkpatch happy and change the indent of the wrapped lines to line up with ( since the ( moved.
Quoted 3 lines
While I see the point, wouldn't be better to have 1000 replaced with MILLI at the same time?
For anything with 3 zeros, I don't consider MILLI better (or worse). Science shows that the average human can easily see 3 or 4 things without having to count them [1]. So it is only when we start getting more 0s than that is when I think we should be picky about using macros instead.
And in this particular case, we are converting milli to micro so 1000 should be replaced by (MICRO / MILLI) if we are going to do that.
Quoted 3 lines · David Lechner
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · David Lechner
On 7/2/25 8:58 AM, Akshay Bansod wrote:
...
Quoted 10 lines
Quoted 9 lines
Quoted 6 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev, · line 2040
Quoted 3 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-0+ len +=sysfs_emit_at (buf, len,"%d.%03d " , odr_table->odr_avl[i].milli_hz /1000 , odr_table->odr_avl[i].milli_hz %1000 );Let's keep checkpatch happy and change the indent of the wrapped lines to line up with ( since the ( moved.
While I see the point, wouldn't be better to have 1000 replaced with MILLI at the same time?
Quoted 8 lines
For anything with 3 zeros, I don't consider MILLI better (or worse). Science shows that the average human can easily see 3 or 4 things without having to count them [1]. So it is only when we start getting more 0s than that is when I think we should be picky about using macros instead.
And in this particular case, we are converting milli to micro so
1000should be replaced by(MICRO / MILLI)if we are going to do that.
I see. This changes the picture drastically. Let's leave it for another day then.
Quoted 1 line
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 20 lines · On Wed, 2 Jul 2025 10:04:23 -0500 David Lechner <[email protected]> wrote:
Quoted 15 lines · Andy Shevchenko
Quoted 4 lines · David Lechner
Quoted 3 lines · Akshay Bansod
Update the sysfs interface for sampling frequency and scale attributes. Replace
scnprintf()withsysfs_emit_at()which is PAGE_SIZE-aware and recommended for use in sysfs....
Quoted 6 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev, · line 2040
Quoted 3 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-0+ len +=sysfs_emit_at (buf, len,"%d.%03d " , odr_table->odr_avl[i].milli_hz /1000 , odr_table->odr_avl[i].milli_hz %1000 );Let's keep checkpatch happy and change the indent of the wrapped lines to line up with ( since the ( moved.
Quoted 3 lines
While I see the point, wouldn't be better to have 1000 replaced with MILLI at the same time?
Quoted 8 lines
For anything with 3 zeros, I don't consider MILLI better (or worse). Science shows that the average human can easily see 3 or 4 things without having to count them [1]. So it is only when we start getting more 0s than that is when I think we should be picky about using macros instead.
And in this particular case, we are converting milli to micro so
1000should be replaced by(MICRO / MILLI)if we are going to do that.
No we aren't.
This one is converting from milli_hz to hz + sticking to milli for the decimal part.
Lots of other IIO cases where you would have been right, but I think not here.
David Lechner
reviewerQuoted 29 lines · Jonathan Cameron
Quoted 26 lines · On Wed, 2 Jul 2025 10:04:23 -0500 David Lechner <[email protected]> wrote:
Quoted 15 lines · Andy Shevchenko
Quoted 4 lines · David Lechner
Quoted 3 lines · Akshay Bansod
Update the sysfs interface for sampling frequency and scale attributes. Replace
scnprintf()withsysfs_emit_at()which is PAGE_SIZE-aware and recommended for use in sysfs....
Quoted 6 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev, · line 2040
Quoted 3 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-0+ len +=sysfs_emit_at (buf, len,"%d.%03d " , odr_table->odr_avl[i].milli_hz /1000 , odr_table->odr_avl[i].milli_hz %1000 );Let's keep checkpatch happy and change the indent of the wrapped lines to line up with ( since the ( moved.
Quoted 3 lines
While I see the point, wouldn't be better to have 1000 replaced with MILLI at the same time?
For anything with 3 zeros, I don't consider MILLI better (or worse). Science shows that the average human can easily see 3 or 4 things without having to count them [1]. So it is only when we start getting more 0s than that is when I think we should be picky about using macros instead.
Quoted 8 lines
Quoted 2 lines
And in this particular case, we are converting milli to micro so
1000should be replaced by(MICRO / MILLI)if we are going to do that.No we aren't.
This one is converting from
milli_hzto hz + sticking to milli for the decimal part.Lots of other IIO cases where you would have been right, but I think not here.
Oops. The %03d instead of %06d should have given it away!
Quoted 5 lines
Wed, Jul 02, 2025 at 10:53:31AM -0500, David Lechner kirjoitti:
Quoted 6 lines · Jonathan Cameron
Quoted 5 lines
Quoted 3 lines · On Wed, 2 Jul 2025 10:04:23 -0500 David Lechner <[email protected]> wrote:
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · David Lechner
On 7/2/25 8:58 AM, Akshay Bansod wrote:
...
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 9 lines
Quoted 6 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev, · line 2040
Quoted 3 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-0+ len +=sysfs_emit_at (buf, len,"%d.%03d " , odr_table->odr_avl[i].milli_hz /1000 , odr_table->odr_avl[i].milli_hz %1000 );Let's keep checkpatch happy and change the indent of the wrapped lines to line up with ( since the ( moved.
While I see the point, wouldn't be better to have 1000 replaced with MILLI at the same time?
For anything with 3 zeros, I don't consider MILLI better (or worse). Science shows that the average human can easily see 3 or 4 things without having to count them [1]. So it is only when we start getting more 0s than that is when I think we should be picky about using macros instead.
And in this particular case, we are converting milli to micro so
1000should be replaced by(MICRO / MILLI)if we are going to do that.
Quoted 8 lines
Quoted 6 lines
No we aren't.
This one is converting from
milli_hzto hz + sticking to milli for the decimal part.Lots of other IIO cases where you would have been right, but I think not here.
Oops. The %03d instead of %06d should have given it away!
I'm not sure I got your comment. The '3' vs. '6' will just define the minimum amount of printed digits, it does *not* limit the upper numbers anyhow (it's limited by the 'd', which is (INT_MIN .. INT_MAX).
Quoted 1 line
Quoted 1 line
Signature · 2 lines
With Best Regards, Andy Shevchenko
David Lechner
reviewerQuoted 33 lines · Andy Shevchenko
Wed, Jul 02, 2025 at 10:53:31AM -0500, David Lechner kirjoitti:
Quoted 6 lines · Jonathan Cameron
Quoted 5 lines
Quoted 3 lines · On Wed, 2 Jul 2025 10:04:23 -0500 David Lechner <[email protected]> wrote:
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · David Lechner
On 7/2/25 8:58 AM, Akshay Bansod wrote:
...
Quoted 23 lines
Quoted 23 lines
Quoted 10 lines
Quoted 9 lines
Quoted 6 lines
Review
Quoted 3 lines
(quoted diff) +1
-0+ len += sysfs_emit_at(buf, len, "%d.%03d ", odr_table->odr_avl[i].milli_hz / 1000, odr_table->odr_avl[i].milli_hz % 1000);Let's keep checkpatch happy and change the indent of the wrapped lines to line up with ( since the ( moved.
While I see the point, wouldn't be better to have 1000 replaced with MILLI at the same time?
Quoted 8 lines
For anything with 3 zeros, I don't consider MILLI better (or worse). Science shows that the average human can easily see 3 or 4 things without having to count them [1]. So it is only when we start getting more 0s than that is when I think we should be picky about using macros instead.
And in this particular case, we are converting milli to micro so
1000should be replaced by(MICRO / MILLI)if we are going to do that.No we aren't.
This one is converting from
milli_hzto hz + sticking to milli for the decimal part.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Lots of other IIO cases where you would have been right, but I think not here.
Oops. The %03d instead of %06d should have given it away!
I'm not sure I got your comment. The '3' vs. '6' will just define the minimum amount of printed digits, it does *not* limit the upper numbers anyhow (it's limited by the 'd', which is (
INT_MIN..INT_MAX).
It is after the decimal point in the printed string, so 3 digits after a decimal point is going to be MILLI units. And the % 1000 ensures that we would never get more than 3 digits there.
Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
Quoted 8 lines · On Wednesday, 2 July 2025 8:25 pm +0530 David Lechner wrote:
Quoted 7 lines · Akshay Bansod
Update the sysfs interface for sampling frequency and scale attributes. Replace
scnprintf()withsysfs_emit_at()which is PAGE_SIZE-aware and recommended for use in sysfs.Signed-off-by Akshay Bansod <[email protected]>---
Quoted 8 lines
Quoted 5 lines
Testing:
- Built the driver (st_lsm6dsx_i2c) as a module.
- Tested usingi2c-stubto mock the device.
- Verified that reading sysfs attributes likesampling_frequency_availableworks correctly and shows no change in functionality.Nice to see it was actually tested. :-)
:-)
Quoted 10 lines
Quoted 10 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +0
-0@@ -2035,7 +2035,7 @@ st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev,
Quoted 10 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev, · line 2040
Quoted 6 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-1odr_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 " , + len +=sysfs_emit_at (buf, len,"%d.%03d " , odr_table->odr_avl[i].milli_hz /1000 , odr_table->odr_avl[i].milli_hz %1000 );Let's keep checkpatch happy and change the indent of the wrapped lines to line up with ( since the ( moved.
noted. I wasn't aware of that.
Quoted 11 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev, · line 2058
Quoted 8 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +1
-1buf[len -1 ] ='\n' ;@@ -2054,7 +2054,7 @@ 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 " , + len +=sysfs_emit_at (buf, len,"0.%09u " , fs_table->fs_avl[i].gain);ditto
noted.
Quoted 4 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev, · line 2060
Quoted 2 lines
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +0
-0buf[len -1 ] ='\n' ;
Thanks for the review. I'll revise the patch.
Regards,
Akshay