Re: [PATCH v3 13/13] iio: core: Clarify the modes

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Jonathan,

jic23@kernel.org wrote on Sun, 13 Feb 2022 18:42:24 +0000:

> On Mon,  7 Feb 2022 15:38:40 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > As part of a previous discussion with Jonathan Cameron [1], it appeared
> > necessary to clarify the meaning of each mode so that new developers
> > could understand better what they should use or not use and when.
> > 
> > The idea of renaming these modes as been let aside because naming is a
> > big deal and requires a lot of thinking. So for now let's focus on
> > correctly explaining what each mode implies.
> > 
> > [1] https://lore.kernel.org/linux-iio/20210930165510.2295e6c4@jic23-huawei/
> > 
> > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> One trivial thing inline as a result of edits in v3.
> 
> Otherwise, I want to let this series sit a little longer and ideally get
> some eyes on the st_sensors patches.

Sure.

> 
> Jonathan
> 
> > ---
> >  include/linux/iio/iio.h | 49 ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 48 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index 85cb924debd9..e383b0d96035 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -315,7 +315,54 @@ static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
> >  s64 iio_get_time_ns(const struct iio_dev *indio_dev);
> >  unsigned int iio_get_time_res(const struct iio_dev *indio_dev);
> >  
> > -/* Device operating modes */
> > +/**
> > + * Device operating modes
> > + * @INDIO_DIRECT_MODE: There is an access to either:
> > + * a) The last single value available for devices that do not provide
> > + *    on-demand reads.
> > + * b) A new value after performing an on-demand read otherwise.  
> 
> 
> > + * On most devices, this is a single-shot read. On some devices with data
> > + * streams without an 'on-demand' function, this might also be the 'last value'
> > + * feature.  
> 
> This block duplicates what you now have as a/b above. I can drop it whilst
> applying if nothing else comes up.

We can get rid of it indeed. Let's see what ST people have in mind
regarding the st_sensors patches.

> >  Above all, this mode internally means that we are not in any of the
> > + * other modes, and sysfs reads should work.
> > + * Device drivers should inform the core if they support this mode.
> > + * @INDIO_BUFFER_TRIGGERED: Common mode when dealing with kfifo buffers.
> > + * It indicates that an explicit trigger is required. This requests the core to
> > + * attach a poll function when enabling the buffer, which is indicated by the
> > + * _TRIGGERED suffix.
> > + * The core will ensure this mode is set when registering a triggered buffer
> > + * with iio_triggered_buffer_setup().
> > + * @INDIO_BUFFER_SOFTWARE: Another kfifo buffer mode, but not event triggered.
> > + * No poll function can be attached because there is no triggered infrastructure
> > + * we can use to cause capture. There is a kfifo that the driver will fill, but
> > + * not "only one scan at a time". Typically, hardware will have a buffer that
> > + * can hold multiple scans. Software may read one or more scans at a single time
> > + * and push the available data to a Kfifo. This means the core will not attach
> > + * any poll function when enabling the buffer.
> > + * The core will ensure this mode is set when registering a simple kfifo buffer
> > + * with devm_iio_kfifo_buffer_setup().
> > + * @INDIO_BUFFER_HARDWARE: For specific hardware, if unsure do not use this mode.
> > + * Same as above but this time the buffer is not a kfifo where we have direct
> > + * access to the data. Instead, the consumer driver must access the data through
> > + * non software visible channels (or DMA when there is no demux possible in
> > + * software)
> > + * The core will ensure this mode is set when registering a dmaengine buffer
> > + * with devm_iio_dmaengine_buffer_setup().
> > + * @INDIO_EVENT_TRIGGERED: Very unusual mode.
> > + * Triggers usually refer to an external event which will start data capture.
> > + * Here it is kind of the opposite as, a particular state of the data might
> > + * produce an event which can be considered as an event. We don't necessarily
> > + * have access to the data itself, but to the event produced. For example, this
> > + * can be a threshold detector. The internal path of this mode is very close to
> > + * the INDIO_BUFFER_TRIGGERED mode.
> > + * The core will ensure this mode is set when registering a triggered event.
> > + * @INDIO_HARDWARE_TRIGGERED: Very unusual mode.
> > + * Here, triggers can result in data capture and can be routed to multiple
> > + * hardware components, which make them close to regular triggers in the way
> > + * they must be managed by the core, but without the entire interrupts/poll
> > + * functions burden. Interrupts are irrelevant as the data flow is hardware
> > + * mediated and distributed.
> > + */
> >  #define INDIO_DIRECT_MODE		0x01
> >  #define INDIO_BUFFER_TRIGGERED		0x02
> >  #define INDIO_BUFFER_SOFTWARE		0x04  
> 


Thanks,
Miquèl