On Wed, Mar 25, 2020 at 5:14 PM Ivan Mikhaylov <i.mikhaylov@xxxxxxxxx> 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
