Re: [PATCH v2 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor

Andy Shevchenko
2020-03-25T15:54:07Z

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, &reg);

> + 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, &reg);
> + 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