From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Ivan Mikhaylov <i.mikhaylov@yadro.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
Hartmut Knaack <knaack.h@gmx.de>,
Lars-Peter Clausen <lars@metafoo.de>,
Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
linux-iio <linux-iio@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
devicetree <devicetree@vger.kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH v2 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor
Date: Wed, 25 Mar 2020 17:53:57 +0200 [thread overview]
Message-ID: <CAHp75VfjqtAtS-iXS6vz452m-EUtPcNt7EPm1JvQwC4VeX-k1A@mail.gmail.com> (raw)
In-Reply-To: <20200325151211.19949-3-i.mikhaylov@yadro.com>
On Wed, Mar 25, 2020 at 5:14 PM Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote:
>
> Proximity sensor driver based on light/vcnl4000.c code.
> For now supports only the single on-demand measurement.
>
> The VCNL3020 is a fully integrated proximity sensor. Fully
> integrated means that the infrared emitter is included in the
> package. It has 16-bit resolution. It includes a signal
> processing IC and features standard I2C communication
> interface. It features an interrupt function.
Thank you for an update, my comments below.
...
> +config VCNL3020
> + tristate "VCNL3020 proximity sensor"
> + depends on I2C
REGMAP_I2C
...
> +struct vcnl3020_data {
> + struct regmap *regmap;
> + struct i2c_client *client;
Since you have switched to regmap I2C API, do you really need client
here, perhaps struct device *dev would be enough?
> + u8 rev;
> + struct mutex lock;
> +};
...
> + rc = regmap_read(data->regmap, VCNL_PROD_REV, ®);
> + if (rc < 0) {
I think you may drop all these ' < 0' checks for regmap, otherwise can
you elaborate what positive return code, if any, means?
> + dev_err(&data->client->dev,
> + "Error (%d) reading product revision", rc);
> + return rc;
> + }
...
> + rc = regmap_write(data->regmap, VCNL_LED_CURRENT, led_current);
> + if (rc < 0) {
...after above change...
> + dev_err(&data->client->dev, "Error (%d) setting LED current",
> + rc);
> + return rc;
> + }
> +
> + return 0;
...simple return rc; here.
...
> + /* wait for data to become ready */
> + do {
> + rc = regmap_read(data->regmap, VCNL_COMMAND, ®);
> + if (rc < 0)
> + goto err_unlock;
> + if (reg & VCNL_PS_RDY)
> + break;
> + msleep(20); /* measurement takes up to 100 ms */
> + } while (--tries);
regmap_read_poll_timeput()
...
> +static const struct iio_chan_spec vcnl3020_channels[] = {
> + {
> + .type = IIO_PROXIMITY,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + }
Leave comma here.
> +};
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2020-03-25 15:54 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-25 15:12 [PATCH v2 0/2] iio: proximity: driver for vcnl3020 Ivan Mikhaylov
2020-03-25 15:12 ` [PATCH v2 1/2] iio: proximity: provide device tree binding document Ivan Mikhaylov
2020-03-26 16:54 ` Rob Herring
2020-03-25 15:12 ` [PATCH v2 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor Ivan Mikhaylov
2020-03-25 15:53 ` Andy Shevchenko [this message]
2020-03-25 15:47 ` [PATCH v2 0/2] iio: proximity: driver for vcnl3020 Andy Shevchenko
2020-03-26 13:06 ` Ivan Mikhaylov
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=CAHp75VfjqtAtS-iXS6vz452m-EUtPcNt7EPm1JvQwC4VeX-k1A@mail.gmail.com \
--to=andy.shevchenko@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=i.mikhaylov@yadro.com \
--cc=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=pmeerw@pmeerw.net \
--cc=robh+dt@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).