Message-ID: <CAOeBkLqaqZJJCrN3D6tq7S3P=yhJs+V5grnaC+FmSeg4bRxfvg@mail.gmail.com>
Date:   Mon, 18 Feb 2019 14:00:58 -0300
From:   Rodrigo Ribeiro <rodrigorsdc@...il.com>
To:     Jonathan Cameron <jonathan.cameron@...wei.com>
Cc:     "Popa, Stefan Serban" <StefanSerban.Popa@...log.com>,
        "ardeleanalex@...il.com" <ardeleanalex@...il.com>,
        "kernel-usp@...glegroups.com" <kernel-usp@...glegroups.com>,
        "lars@...afoo.de" <lars@...afoo.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "knaack.h@....de" <knaack.h@....de>,
        "Ardelean, Alexandru" <Alex.Ardelean@...log.com>,
        "jic23@...nel.org" <jic23@...nel.org>,
        "Hennerich, Michael" <Michael.Hennerich@...log.com>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "pmeerw@...erw.net" <pmeerw@...erw.net>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "rafael.tsuha@....br" <rafael.tsuha@....br>
Subject: Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED

Em seg, 18 de fev de 2019 às 11:46, Jonathan Cameron
<jonathan.cameron@...wei.com> escreveu:
>
> On Thu, 14 Feb 2019 10:51:49 +0000
> "Popa, Stefan Serban" <StefanSerban.Popa@...log.com> 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@...i
> > > l.com> escreveu:
> > > >
> > > > On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron <jic23@...nel.org>
> > > wrote:
> > > > >
> > > > > On Fri, 25 Jan 2019 22:14:32 -0200
> > > > > Rodrigo Ribeiro <rodrigorsdc@...il.com> wrote:
> > > > >
> > > > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro
> > > > > > <rodrigorsdc@...il.com> escreveu:
> > > > > > >
> > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean
> > > > > > > <ardeleanalex@...il.com> 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@...il.com> 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@...il.com>
> > > > > > > > > Signed-off-by: Rafael Tsuha <rafael.tsuha@....br>
> > > > > > > > > ---
> > > > > > > > > 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
> > > > > > > > >
> > > >
>
>

Thanks for answering Jonathan and Stefan.

I would like to contribute with the ad7150 driver. Could you please
give me some hints on what can I do to move this driver
out of staging?

I note that device registration (devm_iio_device_register) is not
the last function called at probe function. Should I change order
to call dev_info before the device registration?

Also did not see any device tree documentation at
Documentation/devicetree/bindings/iio for AD7150 driver.

Powered by blists - more mailing lists