Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED

[Jonathan Cameron]

On Thu, 14 Feb 2019 10:51:49 +0000
"Popa, Stefan Serban" <StefanSerban.Popa@xxxxxxxxxx> wrote:

> On Mi, 2019-02-13 at 22:25 -0200, Rodrigo Ribeiro wrote:
> > [External]
> > 
> > 
> > Em ter, 29 de jan de 2019 às 07:10, Alexandru Ardelean <ardeleanalex@gmai  
> > l.com> escreveu:
> > >
> > > On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron <jic23@xxxxxxxxxx>  
> > wrote:  
> > > >
> > > > On Fri, 25 Jan 2019 22:14:32 -0200
> > > > Rodrigo Ribeiro <rodrigorsdc@xxxxxxxxx> wrote:
> > > >  
> > > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro
> > > > > <rodrigorsdc@xxxxxxxxx> escreveu:  
> > > > > >
> > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean
> > > > > > <ardeleanalex@xxxxxxxxx> escreveu:  
> > > > > > >
> > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro <rodrigorsdc@gm  
> > ail.com> wrote:  
> > > > > > > >
> > > > > > > > Remove the checkpatch.pl check:
> > > > > > > >
> > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'?  
> > > > > > >
> > > > > > > Hey,  
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Thanks for answering.
> > > > > >  
> > > > > > > A bit curios about this one.
> > > > > > > Are you using this chip/driver ?  
> > > > > >
> > > > > > No, I am just a student at USP (University of São Paulo) starting  
> > in Linux  
> > > > > > Kernel and a new member of the USP Linux Kernel group that has  
> > contributed  
> > > > > > for a few months.
> > > > > >  
> > > > > > > Thing is: the part is nearing EOL, and it could be an idea to  
> > be  
> > > > > > > marked for removal (since it's still in staging).
> > > > > > > But if there are users for this driver, it could be left around  
> > for a while.  
> > > > > >
> > > > > > This is my first patch in Linux Kernel, but if the driver will be  
> > removed, I  
> > > > > > can send a patch for another driver. Is there any driver that I  
> > can  
> > > > > > fix a style warning?  
> > > > >
> > > > > Maybe, one checkstyle patch is enough, right? Which drivers can I  
> > truly  
> > > > > contribute to?  
> > > >
> > > > How about the ad7150?  That one is still listed as production.
> > > > What do you think Alex, you probably have better visibility on the
> > > > status of these parts and their drivers than I do!
> > > >
> > > > (note I haven't even opened that one for a few years so no idea
> > > > what state the driver is in!)
> > > >  
> > >
> > > ad7150 is a good alternative.
> > > At a first glance over the driver it looks like it could do with some
> > > polish/conversions to newer IIO constructs (like IIO triggers, maybe
> > > some newer event handling mechanisms?).
> > > I'll sync with Stefan [Popa] to see about this stuff at a later point  
> > in time.  
> > >
> > > I'd also add here the adxl345 driver.
> > > This is mostly informational for anyone who'd find this interesting.
> > > There are 2 drivers for this chip, one in IIO
> > > [drivers/iio/accel/adxl345] and another one in
> > > "drivers/misc/adxl34x.c" as part of the input sub-system.
> > > What would be interesting here is to finalize the IIO driver [ I think
> > > some features are lacking behind the input driver], and make the input
> > > driver a consumer of the IIO driver.
> > >
> > > From my side, both these variants are fine to take on.
> > > The ad7150 is a good idea as a starter project, and the adxl one is
> > > more of a long-term medium-level project.
> > >
> > > Thanks
> > > Alex
> > >  
> > 
> > Hi Alex, thanks for suggestions.
> > 
> > I read the IIO trigger documentation 
> > (https://www.kernel.org/doc/html/v4.12/driver-api/iio/triggers.html) and
> > ask one question: What is the difference between events and triggers? 
> > They are sounds me same things.
> > 
> > I am trying to understand how to implement a IIO trigger by reading
> > the IIO Simple Dummy code but this driver does not implements IIO
> > triggers
> > but only IIO events. Is there a didactic example like IIO Simple Dummy
> > that implements IIO triggers?
> > 
> > Thanks
> > Rodrigo
> >   
> 
> Hi Rodrigo,
> 
> From what I know, IIO Events are not used for passing readings from devices
> to userspace, but rather out of bounds information such as crossing of
> voltage thresholds, proximity detection, motion detection, etc.
> Triggers are typically used to determine when valid data can be read from a
> device which is then stored in the buffer.
> 
> After a quick look over the AD7150, I think that using IIO events, might be
> the correct approach, since it is a proximity sensing device. 

To answer the question on generic triggers (agreed, probably not relevant here
from what Stefan has said), there are several software triggers under
drivers/iio/triggers.

Wasn't a lot of point in adding another one to the dummy driver given you
can use the hrtimer, or sysfs triggers directly.
(loop will result in craziness given the near zero read time of the
dummy driver ;)

Jonathan

> 
> -Stefan
> 
> > > > Jonathan
> > > >  
> > > > >  
> > > > > > Thanks
> > > > > >
> > > > > >
> > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean
> > > > > > <ardeleanalex@xxxxxxxxx> escreveu:  
> > > > > > >
> > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro <rodrigorsdc@gm  
> > ail.com> wrote:  
> > > > > > > >
> > > > > > > > Remove the checkpatch.pl check:
> > > > > > > >
> > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'?  
> > > > > > >
> > > > > > > Hey,
> > > > > > >
> > > > > > > A bit curios about this one.
> > > > > > > Are you using this chip/driver ?
> > > > > > >
> > > > > > > Thing is: the part is nearing EOL, and it could be an idea to  
> > be  
> > > > > > > marked for removal (since it's still in staging).
> > > > > > > But if there are users for this driver, it could be left around  
> > for a while.  
> > > > > > >
> > > > > > > Thanks
> > > > > > > Alex
> > > > > > >  
> > > > > > > >
> > > > > > > > Signed-off-by: Rodrigo Ribeiro <rodrigorsdc@xxxxxxxxx>
> > > > > > > > Signed-off-by: Rafael Tsuha <rafael.tsuha@xxxxxx>
> > > > > > > > ---
> > > > > > > > This macro is not used anywhere. Should we just correct the
> > > > > > > > spelling or remove the macro?
> > > > > > > >
> > > > > > > >  drivers/staging/iio/cdc/ad7152.c | 2 +-
> > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c  
> > b/drivers/staging/iio/cdc/ad7152.c  
> > > > > > > > index 25f51db..c9da6d4 100644
> > > > > > > > --- a/drivers/staging/iio/cdc/ad7152.c
> > > > > > > > +++ b/drivers/staging/iio/cdc/ad7152.c
> > > > > > > > @@ -35,7 +35,7 @@
> > > > > > > >  #define AD7152_REG_CH2_GAIN_HIGH       12
> > > > > > > >  #define AD7152_REG_CH2_SETUP           14
> > > > > > > >  #define AD7152_REG_CFG                 15
> > > > > > > > -#define AD7152_REG_RESEVERD            16
> > > > > > > > +#define AD7152_REG_RESERVED            16
> > > > > > > >  #define AD7152_REG_CAPDAC_POS          17
> > > > > > > >  #define AD7152_REG_CAPDAC_NEG          18
> > > > > > > >  #define AD7152_REG_CFG2                        26
> > > > > > > > --
> > > > > > > > 2.7.4
> > > > > > > >  
> > >