Re: [PATCH v3 4/4] iio: cdc: add Texas Instruments FDC1004 driver

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 27 Oct 2026 05:04:38 -0500
Victor Chernyakin <chernyakin.victor.j@outlook.com> wrote:

> > > I don't like the duplication either, but I think the real problem here is
> > > that read_avail is just a somewhat awkward interface that doesn't allow me
> > > to return s64 values directly. To me, having this separate decomposed table
> > > just for read_avail and otherwise working with s64 values is the least bad
> > > approach.  
> > 
> > What about adding .read_avail64() (in a separate change)? Problem solved!  
> 
> If Jonathan is open to such a new API, I could work on that! I think
> if we're going to add it, instead of just changing the two int* parameters
> to one s64*, we could add a union for the function to write into. 

Doing it for read_avail() and not read_raw() would be a odd.
For read_raw() we went down the decompose path because in initial drivers
it was only one or two out of a whole bunch of values that needed
the 64 bit form.

The challenge of touching these interfaces is how to make things both
backwards compatible and avoid need to do major refactors.
There is precedence in the addition of read_raw_multi().
Drivers that use that have to use it for everything rather than mix and match
with read_raw().

Note that we need to also carefully think about impacts on the inkern
interfaces as they use all these callbacks and the consumer drivers need
to understand the results - anything in this space goes beyond the
simple sysfs interfaces.

So there are various things we could do and I'm not sure which one makes sense.
Maybe we do have enough users now to justify a new interface. I was unconvinced
in the past but things do chagne.

Anyhow, options - note I don't want to do read_avail only as that to me
leaves us in a silly intermediate place.  So just consider read_raw().
read_avail() should follow naturally.

1. read_raw_multi() path.  Drivers opt in to do everything in 64 bit.
   Maybe painful and that inkern.c consumer bit needs some papering over.
   I haven't thought about how painful that will be.
2. read_avail() vs read_raw() path.  Given not all properties have meaningful
   available values. Hence thes two use separate masks and opt in.  Here we
   need to avoid that as would be significant bloat.
3. Mix and match. read_raw() for some things, read_raw_multi() for others.
   How to decide?  I don't like try one and see if it returns an error.
   Could use something similar to the write_fmt callback.  So drivers
   could say what the format of a read_raw was and use that to pick between
   callbacks.

Generally I'm not sure.  Probably needs some experimentation to find the
cleaners solution for all the paths that use these callbacks.


> That way,
> not everything needs to be represented as raw s64's; we could introduce
> a proper range type for example:
> 
> struct iio_range {
>     s64 min;
>     s64 step;
>     s64 max;
> };
> 
> struct iio_available_value {
>     enum iio_available_type type; /* IIO_AVAIL_RANGE, etc. */
>     union {
>         const struct iio_range *range;
>         struct {
>             const s64 *vals;
>             int num_vals;
>         } list;
>     };
> };
> 
> /* Used like so: */
> 
> static const struct iio_range fdc1004_calibbias = {
> 	.min  = -0x8000 * 488281250UL,
> 	.step = +0x0001 * 488281250UL,
> 	.max  = +0x7FFF * 488281250UL,
> };
> 
> static int fdc1004_read_avail_ext(struct iio_dev *indio_dev,
> 			                      const struct iio_chan_spec *chan,
> 			                      iio_available_value *out, long info)
> {
> 	switch (info) {
> 	case IIO_CHAN_INFO_CALIBBIAS:
>         out->range = &fdc1004_calibbias; /* No casts! */
>         out->type = IIO_AVAIL_RANGE;
> 		return IIO_VAL_DECIMAL64_FEMTO;
>     }
> }
This ends up as a bit of an implementation detail. Might make
sense to have such a structure / union. I'm not sure yet.

Jonathan
>