Re: [PATCH 02/10] iio: core: Enhance the kernel doc of modes and currentmodes iio_dev entries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonathan,

jic23@xxxxxxxxxx wrote on Sat, 15 Jan 2022 15:51:45 +0000:

> On Thu, 16 Dec 2021 09:15:52 +0100
> Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> 
> > Hi Alexandru,
> > 
> > ardeleanalex@xxxxxxxxx wrote on Thu, 16 Dec 2021 08:24:51 +0200:
> >   
> > > On Wed, Dec 15, 2021 at 10:04 PM Miquel Raynal
> > > <miquel.raynal@xxxxxxxxxxx> wrote:    
> > > >
> > > > Let's provide more details about these two variables because their
> > > > understanding may not be straightforward for someone not used to the IIO
> > > > subsystem internal logic. The different modes will soon be also be more
> > > > documented for the same reason.
> > > >
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > > > ---
> > > >  include/linux/iio/iio.h | 10 ++++++++--
> > > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > > > index 06433c2c2968..0312da2e83f8 100644
> > > > --- a/include/linux/iio/iio.h
> > > > +++ b/include/linux/iio/iio.h
> > > > @@ -487,8 +487,14 @@ struct iio_buffer_setup_ops {
> > > >
> > > >  /**
> > > >   * struct iio_dev - industrial I/O device
> > > > - * @modes:             [DRIVER] operating modes supported by device
> > > > - * @currentmode:       [INTERN] current operating mode
> > > > + * @modes:             [DRIVER] list of operating modes supported by the IIO      
> > > 
> > > I'd argue that it may make sense to highlight that this list of modes
> > > is represented as a bitmask.
> > > When reading docs, it may not be obvious at first (I guess).    
> > 
> > That is a good idea. I'll add this to the next iteration.  
> 
> Good.

Done.

> 
> >   
> > > > + *                     device, this list should be initialized before
> > > > + *                     registering the IIO device and can be filed up by the
> > > > + *                     IIO core depending on the features advertised by the
> > > > + *                     driver during other steps of the registration  
> 
> Perhaps make it a little clearer that some bits are set as a result of
> enabling particular features.  Maybe even call out which functions do it.
> From what I recall, that's a very short list.

Done.

> 
> > > > + * @currentmode:       [INTERN] operating mode currently in use, may be
> > > > + *                     eventually checked by device drivers but should be
> > > > + *                     considered read-only as this is a core internal bit  
> 
> We should add an accessor function to read it.  (maybe later in your series? :)

Yup!

> 
> > > >   * @dev:               [DRIVER] device structure, should be assigned a parent
> > > >   *                     and owner
> > > >   * @buffer:            [DRIVER] any buffer present
> > > > --
> > > > 2.27.0
> > > >      
> > 
> > Thanks,
> > Miquèl  
> 


Thanks,
Miquèl




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux