From: Dan Carpenter <dan.carpenter@oracle.com>
To: Lucas Stankus <lucas.p.stankus@gmail.com>
Cc: lars@metafoo.de, Michael.Hennerich@analog.com, jic23@kernel.org,
gregkh@linuxfoundation.org, linux-iio@vger.kernel.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
Subject: Re: [PATCH v3 2/2] staging: iio: cdc: ad7746: use dt for capacitive channel setup.
Date: Mon, 3 May 2021 17:44:29 +0300 [thread overview]
Message-ID: <20210503144429.GN21598@kadam> (raw)
In-Reply-To: <20210503100720.GP1981@kadam>
On Mon, May 03, 2021 at 01:07:20PM +0300, Dan Carpenter wrote:
> On Sat, May 01, 2021 at 09:32:53AM -0300, Lucas Stankus wrote:
> > diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> > index dfd71e99e872..531f1b96dea2 100644
> > --- a/drivers/staging/iio/cdc/ad7746.c
> > +++ b/drivers/staging/iio/cdc/ad7746.c
> > @@ -18,8 +18,6 @@
> > #include <linux/iio/iio.h>
> > #include <linux/iio/sysfs.h>
> >
> > -#include "ad7746.h"
> > -
> > /*
> > * AD7746 Register Definition
> > */
> > @@ -676,10 +674,11 @@ static const struct iio_info ad7746_info = {
> > static int ad7746_probe(struct i2c_client *client,
> > const struct i2c_device_id *id)
> > {
> > - struct ad7746_platform_data *pdata = client->dev.platform_data;
> > + struct device *dev = &client->dev;
> > struct ad7746_chip_info *chip;
> > struct iio_dev *indio_dev;
> > unsigned char regval = 0;
> > + unsigned int vdd_permille;
> > int ret = 0;
> >
> > indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> > @@ -703,26 +702,39 @@ static int ad7746_probe(struct i2c_client *client,
> > indio_dev->num_channels = ARRAY_SIZE(ad7746_channels);
> > indio_dev->modes = INDIO_DIRECT_MODE;
> >
> > - if (pdata) {
> > - if (pdata->exca_en) {
> > - if (pdata->exca_inv_en)
> > - regval |= AD7746_EXCSETUP_NEXCA;
> > - else
> > - regval |= AD7746_EXCSETUP_EXCA;
> > - }
> > + if (device_property_read_bool(dev, "adi,exca-output-en")) {
> > + if (device_property_read_bool(dev, "adi,exca-output-invert"))
> > + regval |= AD7746_EXCSETUP_NEXCA;
> > + else
> > + regval |= AD7746_EXCSETUP_EXCA;
> > + }
> >
> > - if (pdata->excb_en) {
> > - if (pdata->excb_inv_en)
> > - regval |= AD7746_EXCSETUP_NEXCB;
> > - else
> > - regval |= AD7746_EXCSETUP_EXCB;
> > - }
> > + if (device_property_read_bool(dev, "adi,excb-output-en")) {
> > + if (device_property_read_bool(dev, "adi,excb-output-invert"))
> > + regval |= AD7746_EXCSETUP_NEXCB;
> > + else
> > + regval |= AD7746_EXCSETUP_EXCB;
> > + }
> >
> > - regval |= AD7746_EXCSETUP_EXCLVL(pdata->exclvl);
> > - } else {
> > - dev_warn(&client->dev, "No platform data? using default\n");
> > - regval = AD7746_EXCSETUP_EXCA | AD7746_EXCSETUP_EXCB |
> > - AD7746_EXCSETUP_EXCLVL(3);
> > + ret = device_property_read_u32(dev, "adi,excitation-vdd-permille",
> > + &vdd_permille);
> > + if (!ret) {
>
> This test is reversed. I wonder if the static checkers can catch the
> uninitialized variable bug... It's probably better to write it as:
>
> if (device_property_read_u32(dev, "adi,excitation-vdd-permille",
> &vdd_permille) {
>
> So it matches the others.
Oops. Sorry for the noise. I was wrong on this. I looked at the
device_property_read_bool() code instead of the device_property_read_u32().
It's disappointing that the returns are reversed.
regards,
dan carpenter
next prev parent reply other threads:[~2021-05-03 14:45 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-01 12:32 [PATCH v3 0/2] staging: iio: cdc: ad7746: add dt properties for capacitive channel setup Lucas Stankus
2021-05-01 12:32 ` [PATCH v3 1/2] dt-bindings: staging: iio: cdc: ad7746: add binding documentation for AD7746 Lucas Stankus
2021-05-06 14:25 ` Rob Herring
2021-05-01 12:32 ` [PATCH v3 2/2] staging: iio: cdc: ad7746: use dt for capacitive channel setup Lucas Stankus
2021-05-03 10:07 ` Dan Carpenter
2021-05-03 14:44 ` Dan Carpenter [this message]
2021-05-02 17:50 ` [PATCH v3 0/2] staging: iio: cdc: ad7746: add dt properties " Jonathan Cameron
2021-05-19 20:59 ` Lucas Stankus
2021-05-21 17:30 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210503144429.GN21598@kadam \
--to=dan.carpenter@oracle.com \
--cc=Michael.Hennerich@analog.com \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=lucas.p.stankus@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.