Re: [PATCH] iio: st_lsm6dsx: Replace scnprintf with sysfs_emit

From: Andy Shevchenko
Date: Thu Jul 03 2025 - 05:05:52 EST


Wed, Jul 02, 2025 at 10:53:31AM -0500, David Lechner kirjoitti:
> On 7/2/25 10:33 AM, Jonathan Cameron wrote:
> > On Wed, 2 Jul 2025 10:04:23 -0500
> > David Lechner <dlechner@xxxxxxxxxxxx> wrote:
> >> On 7/2/25 9:55 AM, Andy Shevchenko wrote:
> >>> On Wed, Jul 02, 2025 at 09:16:51AM -0500, David Lechner wrote:
> >>>> On 7/2/25 8:58 AM, Akshay Bansod wrote:

...

> >>>>> + 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 `1000`
> >> should 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.
>
> 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).


> >> [1]: https://www.scientificamerican.com/article/your-brain-finds-it-easy-to-size-up-four-objects-but-not-five-heres-why/

--
With Best Regards,
Andy Shevchenko