[v3,13/13] iio: core: Clarify the modes

Commit Message

Miquel Raynal Feb. 7, 2022, 2:38 p.m. UTC

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 <[e-mail]>
Signed-off-by: Miquel Raynal <[e-mail]>
---
 include/linux/iio/iio.h | 49 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

Comments

Jonathan Cameron Feb. 13, 2022, 6:42 p.m. UTC | #1

On Mon,  7 Feb 2022 15:38:40 +0100
Miquel Raynal <[e-mail]> 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 <[e-mail]>
> Signed-off-by: Miquel Raynal <[e-mail]>
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.

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.

>  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

Miquel Raynal Feb. 14, 2022, 8:53 a.m. UTC | #2

Hi Jonathan,

[e-mail] wrote on Sun, 13 Feb 2022 18:42:24 +0000:

> On Mon,  7 Feb 2022 15:38:40 +0100
> Miquel Raynal <[e-mail]> 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 <[e-mail]>
> > Signed-off-by: Miquel Raynal <[e-mail]>  
> 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

Jonathan Cameron Feb. 27, 2022, 1:35 p.m. UTC | #3

On Mon, 14 Feb 2022 09:53:08 +0100
Miquel Raynal <[e-mail]> wrote:

> Hi Jonathan,
> 
> [e-mail] wrote on Sun, 13 Feb 2022 18:42:24 +0000:
> 
> > On Mon,  7 Feb 2022 15:38:40 +0100
> > Miquel Raynal <[e-mail]> 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 <[e-mail]>
> > > Signed-off-by: Miquel Raynal <[e-mail]>    
> > 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.

Denis, Linus, Lorenzo,

If any of you have time to take a look at patches 4-8 in this series or ideally
to run basic sanity tests with series in place that would be great.
https://patchwork.kernel.org/project/linux-iio/list/?series=611853

I don't have a convenient platform to test that driver on any more and the
changes are invasive enough to make me a little nervous about taking the
series without someone more familiar with that driver taking a look.

Thanks,

Jonathan

> 
> > 
> > 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

Miquel Raynal March 15, 2022, 3:44 p.m. UTC | #4

Hello,

+ Christophe

[e-mail] wrote on Sun, 27 Feb 2022 13:35:49 +0000:

> On Mon, 14 Feb 2022 09:53:08 +0100
> Miquel Raynal <[e-mail]> wrote:
> 
> > Hi Jonathan,
> > 
> > [e-mail] wrote on Sun, 13 Feb 2022 18:42:24 +0000:
> >   
> > > On Mon,  7 Feb 2022 15:38:40 +0100
> > > Miquel Raynal <[e-mail]> 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 <[e-mail]>
> > > > Signed-off-by: Miquel Raynal <[e-mail]>      
> > > 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.  
> 
> Denis, Linus, Lorenzo,
> 
> If any of you have time to take a look at patches 4-8 in this series or ideally
> to run basic sanity tests with series in place that would be great.
> https://patchwork.kernel.org/project/linux-iio/list/?series=611853
> 
> I don't have a convenient platform to test that driver on any more and the
> changes are invasive enough to make me a little nervous about taking the
> series without someone more familiar with that driver taking a look.

I'm adding Christophe from ST who might also help having these patches
tested or at least reviewed.

TLDR: as part of a wider cleanup, I ended up "playing" with locks in
the st_sensors drivers, so any testing or feedback is welcome!

Thanks,
Miquèl

Miquel Raynal April 5, 2022, 8:02 a.m. UTC | #5

Hi Jonathan,

[e-mail] wrote on Tue, 15 Mar 2022 16:44:50 +0100:

> Hello,
> 
> + Christophe
> 
> [e-mail] wrote on Sun, 27 Feb 2022 13:35:49 +0000:
> 
> > On Mon, 14 Feb 2022 09:53:08 +0100
> > Miquel Raynal <[e-mail]> wrote:
> >   
> > > Hi Jonathan,
> > > 
> > > [e-mail] wrote on Sun, 13 Feb 2022 18:42:24 +0000:
> > >     
> > > > On Mon,  7 Feb 2022 15:38:40 +0100
> > > > Miquel Raynal <[e-mail]> 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 <[e-mail]>
> > > > > Signed-off-by: Miquel Raynal <[e-mail]>        
> > > > 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.      

Sometimes there is no other choice than applying patches to get
feedback :) Hopefully nothing unexpected will happen, but as this
series lived almost two month on the mailing list already, I would
propose to merge the series as it is. We can still drop/revert it as we
are rather soon in the new cycle. If rebasing is needed just let me
know.

Cheers,
Miquèl

> > > 
> > > Sure.    
> > 
> > Denis, Linus, Lorenzo,
> > 
> > If any of you have time to take a look at patches 4-8 in this series or ideally
> > to run basic sanity tests with series in place that would be great.
> > https://patchwork.kernel.org/project/linux-iio/list/?series=611853
> > 
> > I don't have a convenient platform to test that driver on any more and the
> > changes are invasive enough to make me a little nervous about taking the
> > series without someone more familiar with that driver taking a look.  
> 
> I'm adding Christophe from ST who might also help having these patches
> tested or at least reviewed.
> 
> TLDR: as part of a wider cleanup, I ended up "playing" with locks in
> the st_sensors drivers, so any testing or feedback is welcome!
> 
> Thanks,
> Miquèl

Jonathan Cameron April 10, 2022, 3:27 p.m. UTC | #6

On Tue, 5 Apr 2022 10:02:00 +0200
Miquel Raynal <[e-mail]> wrote:

> Hi Jonathan,
> 
> [e-mail] wrote on Tue, 15 Mar 2022 16:44:50 +0100:
> 
> > Hello,
> > 
> > + Christophe
> > 
> > [e-mail] wrote on Sun, 27 Feb 2022 13:35:49 +0000:
> >   
> > > On Mon, 14 Feb 2022 09:53:08 +0100
> > > Miquel Raynal <[e-mail]> wrote:
> > >     
> > > > Hi Jonathan,
> > > > 
> > > > [e-mail] wrote on Sun, 13 Feb 2022 18:42:24 +0000:
> > > >       
> > > > > On Mon,  7 Feb 2022 15:38:40 +0100
> > > > > Miquel Raynal <[e-mail]> 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 <[e-mail]>
> > > > > > Signed-off-by: Miquel Raynal <[e-mail]>          
> > > > > 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.        
> 
> Sometimes there is no other choice than applying patches to get
> feedback :) Hopefully nothing unexpected will happen, but as this
> series lived almost two month on the mailing list already, I would
> propose to merge the series as it is. We can still drop/revert it as we
> are rather soon in the new cycle. If rebasing is needed just let me
> know.

Fully agree on pushing forwards with merging this.  I've done the rebase whilst
applying. Other than a bit of fuzz main change was that adxl367 made use
of a kfifo buffer in the meantime so needed updating as well to drop the
buffer type parameter. 

Take a quick look and see if I messed anything up.
Applied to the togreg branch of iio.git and pushed out as testing for 0-day
etc to take a poke at it before I make a mess of linux-next.

Thanks,

Jonathan

> 
> Cheers,
> Miquèl
> 
> > > > 
> > > > Sure.      
> > > 
> > > Denis, Linus, Lorenzo,
> > > 
> > > If any of you have time to take a look at patches 4-8 in this series or ideally
> > > to run basic sanity tests with series in place that would be great.
> > > https://patchwork.kernel.org/project/linux-iio/list/?series=611853
> > > 
> > > I don't have a convenient platform to test that driver on any more and the
> > > changes are invasive enough to make me a little nervous about taking the
> > > series without someone more familiar with that driver taking a look.    
> > 
> > I'm adding Christophe from ST who might also help having these patches
> > tested or at least reviewed.
> > 
> > TLDR: as part of a wider cleanup, I ended up "playing" with locks in
> > the st_sensors drivers, so any testing or feedback is welcome!
> > 
> > Thanks,
> > Miquèl  
> 
>

Miquel Raynal April 11, 2022, 7:12 a.m. UTC | #7

Hi Jonathan,

[e-mail] wrote on Sun, 10 Apr 2022 16:27:41 +0100:

> On Tue, 5 Apr 2022 10:02:00 +0200
> Miquel Raynal <[e-mail]> wrote:
> 
> > Hi Jonathan,
> > 
> > [e-mail] wrote on Tue, 15 Mar 2022 16:44:50 +0100:
> >   
> > > Hello,
> > > 
> > > + Christophe
> > > 
> > > [e-mail] wrote on Sun, 27 Feb 2022 13:35:49 +0000:
> > >     
> > > > On Mon, 14 Feb 2022 09:53:08 +0100
> > > > Miquel Raynal <[e-mail]> wrote:
> > > >       
> > > > > Hi Jonathan,
> > > > > 
> > > > > [e-mail] wrote on Sun, 13 Feb 2022 18:42:24 +0000:
> > > > >         
> > > > > > On Mon,  7 Feb 2022 15:38:40 +0100
> > > > > > Miquel Raynal <[e-mail]> 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 <[e-mail]>
> > > > > > > Signed-off-by: Miquel Raynal <[e-mail]>            
> > > > > > 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.          
> > 
> > Sometimes there is no other choice than applying patches to get
> > feedback :) Hopefully nothing unexpected will happen, but as this
> > series lived almost two month on the mailing list already, I would
> > propose to merge the series as it is. We can still drop/revert it as we
> > are rather soon in the new cycle. If rebasing is needed just let me
> > know.  
> 
> Fully agree on pushing forwards with merging this.  I've done the rebase whilst
> applying. Other than a bit of fuzz main change was that adxl367 made use
> of a kfifo buffer in the meantime so needed updating as well to drop the
> buffer type parameter. 

Great!

> Take a quick look and see if I messed anything up.
> Applied to the togreg branch of iio.git and pushed out as testing for 0-day
> etc to take a poke at it before I make a mess of linux-next.

Yep, found on the testing branch, everything LGTM but TBH I don't fully
remember everything as it was "some" time ago :)

Thanks,
Miquèl

Patch

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. 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