Re: [PATCH v15 06/12] iio: core: add decimal value formatting into 64-bit value

[Nuno Sá]

On Mon, 2026-06-01 at 16:12 +0100, Rodrigo Alencar wrote:
> On 26/06/01 10:43AM, Nuno Sá wrote:
> > On Sun, May 31, 2026 at 09:30:49AM +0100, Rodrigo Alencar via B4 Relay wrote:
> > > From: Rodrigo Alencar <rodrigo.alencar@xxxxxxxxxx>
> > > 
> > > Create new format types for iio values (IIO_VAL_DECIMAL64_*), which
> > > defines the representation of fixed decimal point values into a single
> > > 64-bit number. This new format increases the range of represented values,
> > > allowing for integer parts greater than 2^32, as bits are not "wasted"
> > > in the fractional part, which can be seen in IIO_VAL_INT_PLUS_MICRO and
> > > IIO_VAL_INT_PLUS_NANO. Helpers are created to compose and decompose 64-bit
> > > decimals into integer values used in IIO formatting interfaces, which
> > > creates consistency and avoid error-prone manual assignments when using
> > > wordpart macros. When doing the parsing, kstrtodec64() is used with the
> > > scale defined by the specific decimal format type.
> > > 
> > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@xxxxxxxxxx>
> > > ---
> > >  drivers/iio/industrialio-core.c | 47 +++++++++++++++++++++++++++++++++--------
> > >  include/linux/iio/types.h       | 30 ++++++++++++++++++++++++++
> > >  2 files changed, 68 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > > index bd6f4f9f4533..a88088cac641 100644
> > > --- a/drivers/iio/industrialio-core.c
> > > +++ b/drivers/iio/industrialio-core.c
> > > @@ -19,6 +19,7 @@
> > >  #include <linux/idr.h>
> > >  #include <linux/kdev_t.h>
> > >  #include <linux/kernel.h>
> > > +#include <linux/math64.h>
> > >  #include <linux/module.h>
> > >  #include <linux/mutex.h>
> > >  #include <linux/poll.h>
> > > @@ -26,7 +27,6 @@
> > >  #include <linux/sched.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/wait.h>
> > > -#include <linux/wordpart.h>
> > >  
> > >  #include <linux/iio/buffer.h>
> > >  #include <linux/iio/buffer_impl.h>
> > > @@ -655,6 +655,7 @@ static ssize_t __iio_format_value(char *buf, size_t offset,
> > > unsigned int type,
> > >  				  int size, const int *vals)
> > >  {
> > >  	int tmp0, tmp1;
> > > +	int l = 0;
> > >  	s64 tmp2;
> > >  	bool scale_db = false;
> > >  
> > > @@ -698,7 +699,6 @@ static ssize_t __iio_format_value(char *buf, size_t offset,
> > > unsigned int type,
> > >  	case IIO_VAL_INT_MULTIPLE:
> > >  	{
> > >  		int i;
> > > -		int l = 0;
> > >  
> > >  		for (i = 0; i < size; ++i)
> > >  			l += sysfs_emit_at(buf, offset + l, "%d ", vals[i]);
> > > @@ -707,8 +707,25 @@ static ssize_t __iio_format_value(char *buf, size_t
> > > offset, unsigned int type,
> > >  	case IIO_VAL_CHAR:
> > >  		return sysfs_emit_at(buf, offset, "%c", (char)vals[0]);
> > >  	case IIO_VAL_INT_64:
> > > -		tmp2 = (s64)((((u64)vals[1]) << 32) | (u32)vals[0]);
> > > +		tmp2 = iio_val_s64_from_s32s(vals);
> > 
> > I might be missing something but can't we just call
> > iio_val_s64_compose()? Likely even inline in sysfs_emit_at()?
> 
> There is a compose() already.
> 

Yes and I was suggesting using that one instead iio_val_s64_from_s32s() :). To be
consistent to what you use in the other path (which is decompose() if I'm not
mistaken).

>  
> > It would match your call to iio_val_s64_decompose() below.
> 
> here are the helpers prototype:
> 
> 	s64 iio_val_s64_compose(s32 val0, s32 val1);
> 	s64 iio_val_s64_from_s32s(const s32 *vals);
> 
> 	void iio_val_s64_decompose(s64 dec64, s32 *val0, s32 *val1);
> 	void iio_val_s64_to_s32s(s64 dec64, s32 *vals);
>  

Yes and it feels that iio_val_s64_compose() and iio_val_s64_decompose() are the only
ones we really need? (Maybe with other naming if you prefer iio_val_s64_from_s32s()
and iio_val_s64_to_s32s()).

> > And the above makes me wonder if the compose()/decompose() are not the
> > only helpers we need? At least in terms of parameters? I mean, just
> > assuming we only have two integers instead of allowing s32* and opening
> > the door for misbehave :)?
> 
> I suppose we would really need some sort of:
> 
> union iio_val {
> 	s32 val32[2];
> 	s64 val64;
> };
> 
> or even add a:
> 
> 	struct { void *ptr, size_t size }

I just meant using two where we just have (s32 val1, s32 vals2) given that is
what IIO has anyways. No need to overthinking it for now IMO.

- Nuno Sá

> > - Nuno Sá
> >