Re: [PATCH] iio: cros_ec: use sysfs_emit() in show callbacks

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 13 Nov 2026 09:23:05 +0000
David Laight <david.laight.linux@gmail.com> wrote:

> On Fri, 13 Nov 2026 01:36:46 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Thu, 12 Nov 2026 22:02:18 +0000
> > David Laight <david.laight.linux@gmail.com> wrote:
> >   
> > > On Thu, 12 Nov 2026 02:04:39 +0000
> > > Jonathan Cameron <jic23@kernel.org> wrote:
> > >     
> > > > On Wed, 11 Nov 2026 16:39:37 +0200
> > > > Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > > >       
> > > > > On Wed, Nov 11, 2026 at 07:26:08AM +0000, Di Wu wrote:        
> > > > > > Replace sprintf() and snprintf() with sysfs_emit() in the sysfs show
> > > > > > and ext_info read callbacks. sysfs_emit() is aware of the sysfs buffer
> > > > > > PAGE_SIZE limit and is the preferred helper for show callbacks.          
> > > > > 
> > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > > > > 
> > > > > ...
> > > > >         
> > > > > > -	return sprintf(buf, "%d.%06u\n",
> > > > > > -		       latency / 1000,
> > > > > > -		       (latency % 1000) * 1000);
> > > > > > +	return sysfs_emit(buf, "%d.%06u\n",
> > > > > > +			  latency / 1000,
> > > > > > +			  (latency % 1000) * 1000);          
> > > > > 
> > > > > As David said, since you touched these lines, make the less amount of them
> > > > > while at it.
> > > > > 
> > > > > (But no need to resend, usually Jonathan is kind to tweak this small thing.)
> > > > >         
> > > > Done and applied.
> > > > 
> > > >       
> > > 
> > > I just looked at that line again.
> > > Why is it multiplying the remainder by 1000?
> > > It could be:
> > > 	return sysfs_emit(buf, "%d.%03u000\n", latency / 1000, latency % 1000);
> > > if the 6 digits after the decimal point are required.    
> > 
> > Good point, I guess unrelated change that we could have on top.  
> 
> Indeed.
> 
> > Those trailing zeros shouldn't matter.  
> 
> There is a matching oddity in the set function (just above).
> It calls iio_str_to_fixpoint(buf, 1000000, &integer, &fract)
> then does latency = integer * 1000 + fract/1000.
> Just replace the 100000 by 1000 and you don't need the divide.
> 
> iio_str_to_fixpoint() will ignore are extra fractional digits.
> 
> Although I'm not at all sure its handling of negative value is correct.
> It converts -1.5 to -1/5 and -0.5 to 0/-5.
> I can't see how that is useful since i*10+f for both gives -5.
> 
> I'd have though that either the fractional part should always be positive
> so -1.4 would need converting to -2/6 or it would have the same sign as
> the integer part and -1.4 would be converted to -1/-4.

That is rather odd use of the output of iio_str_to_fixpoint()
which admittedly is a bit unusual because of the need to handle there
being no where to put that negative when the integer is 0.

Can get away with it here though as latencies aren't negative so.
The code doesn't seem to actually check that though which might
be a nice useability improvement rather than setting it to something odd.

Jonathan





> 
> David
> 
> >   
> > > 
> > > David    
> >   
>