mbox series

[v3,0/5] Add a VCNL4040 light and proximity driver

Message ID 20190321154047.23236-1-angus@akkea.ca
Headers show
Series Add a VCNL4040 light and proximity driver | expand

Message

Angus Ainslie March 21, 2019, 3:40 p.m. UTC 
Extend the Vishay VCNL4000/4010/4020/4200 driver to work with the VCNL4040
chip.

Changes since V2:
    Added Tested-by tag.
    Changed 0x00 to 0 for clarity.

Changes since V1:
    Separated byte vs word bug fix.
    Separated device tree hooks.
    Separated devicetree binding patches.

Angus Ainslie (Purism) (5):
  iio: light: vcnl4000 use word writes instead of byte writes
  iio: light: vcnl4000 add devicetree hooks
  dt-bindings: iio: light: add vcnl4000 devicetree bindings
  iio: light: vcnl4000 add support for the VCNL4040 proximity and light
    sensor
  dt-bindings: iio: light: add vcnl4040 devicetree bindings

 .../bindings/iio/light/vcnl4000.txt           | 24 ++++++
 drivers/iio/light/vcnl4000.c                  | 77 ++++++++++++++++---
 2 files changed, 91 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/light/vcnl4000.txt

Comments

Jonathan Cameron March 24, 2019, 5:18 p.m. UTC | #1 
On Thu, 21 Mar 2019 08:40:43 -0700
"Angus Ainslie (Purism)" <angus@akkea.ca> wrote:

> The VCNL4200 datasheet says that word read and writes should be used
> to access the registers.
> 
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> Tested-by: Tomas Novotny <tomas@novotny.cz>

So we did discuss if this was a fix for any of the existing devices.
Not sure we reached a conclusion as clearly the worked for Tomas without
this.  Anyhow, for now applied to the togreg branch of iio.git and pushed
out as testing for the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/light/vcnl4000.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 04fd0d4b6f19..5e0a8eb83ebc 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -140,10 +140,10 @@ static int vcnl4200_init(struct vcnl4000_data *data)
>  	data->rev = (ret >> 8) & 0xf;
>  
>  	/* Set defaults and enable both channels */
> -	ret = i2c_smbus_write_byte_data(data->client, VCNL4200_AL_CONF, 0x00);
> +	ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, 0);
>  	if (ret < 0)
>  		return ret;
> -	ret = i2c_smbus_write_byte_data(data->client, VCNL4200_PS_CONF1, 0x00);
> +	ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, 0);
>  	if (ret < 0)
>  		return ret;
>
Jonathan Cameron March 24, 2019, 5:19 p.m. UTC | #2 
On Thu, 21 Mar 2019 08:40:44 -0700
"Angus Ainslie (Purism)" <angus@akkea.ca> wrote:

> Add an of_match table for devicetree probing.
> 
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>

I made a small tweak (see inline) for an issue I'd missed
previously (which is pretty obscure!)

Jonathan
> ---
>  drivers/iio/light/vcnl4000.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 5e0a8eb83ebc..308cb2d2b641 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -363,9 +363,31 @@ static int vcnl4000_probe(struct i2c_client *client,
>  	return devm_iio_device_register(&client->dev, indio_dev);
>  }
>  
> +static const struct of_device_id vcnl_4000_of_match[] = {
> +	{
> +		.compatible = "vishay,vcnl4000",
> +		.data = "VCNL4000",
> +	},
> +	{
> +		.compatible = "vishay,vcnl4010",
> +		.data = "VCNL4010",
> +	},
> +	{
> +		.compatible = "vishay,vcnl4010",
> +		.data = "VCNL4020",
> +	},
> +	{
> +		.compatible = "vishay,vcnl4200",
> +		.data = "VCNL4200",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, vcnl4000_of_match);
> +
>  static struct i2c_driver vcnl4000_driver = {
>  	.driver = {
>  		.name   = VCNL4000_DRV_NAME,
> +		.of_match_table = of_match_ptr(vcnl_4000_of_match),
Ah. I'd missed this previously.  Don't use the of_match_ptr
magic anymore. It prevents ACPI probing via the weird
DSDT entry that basically says "here be devicetree".
>  	},
>  	.probe  = vcnl4000_probe,
>  	.id_table = vcnl4000_id,
Jonathan Cameron March 24, 2019, 5:22 p.m. UTC | #3 
On Sun, 24 Mar 2019 17:19:30 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 21 Mar 2019 08:40:44 -0700
> "Angus Ainslie (Purism)" <angus@akkea.ca> wrote:
> 
> > Add an of_match table for devicetree probing.
> > 
> > Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>  
> 
> I made a small tweak (see inline) for an issue I'd missed
> previously (which is pretty obscure!)
> 
> Jonathan
> > ---
> >  drivers/iio/light/vcnl4000.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> > index 5e0a8eb83ebc..308cb2d2b641 100644
> > --- a/drivers/iio/light/vcnl4000.c
> > +++ b/drivers/iio/light/vcnl4000.c
> > @@ -363,9 +363,31 @@ static int vcnl4000_probe(struct i2c_client *client,
> >  	return devm_iio_device_register(&client->dev, indio_dev);
> >  }
> >  
> > +static const struct of_device_id vcnl_4000_of_match[] = {
> > +	{
> > +		.compatible = "vishay,vcnl4000",
> > +		.data = "VCNL4000",
> > +	},
> > +	{
> > +		.compatible = "vishay,vcnl4010",
> > +		.data = "VCNL4010",
> > +	},
> > +	{
> > +		.compatible = "vishay,vcnl4010",
> > +		.data = "VCNL4020",
> > +	},
> > +	{
> > +		.compatible = "vishay,vcnl4200",
> > +		.data = "VCNL4200",
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, vcnl4000_of_match);
vcnl_4000_of_match.

Fixed up.
> > +
> >  static struct i2c_driver vcnl4000_driver = {
> >  	.driver = {
> >  		.name   = VCNL4000_DRV_NAME,
> > +		.of_match_table = of_match_ptr(vcnl_4000_of_match),  
> Ah. I'd missed this previously.  Don't use the of_match_ptr
> magic anymore. It prevents ACPI probing via the weird
> DSDT entry that basically says "here be devicetree".
> >  	},
> >  	.probe  = vcnl4000_probe,
> >  	.id_table = vcnl4000_id,  
>
Jonathan Cameron March 24, 2019, 5:24 p.m. UTC | #4 
On Thu, 21 Mar 2019 08:40:46 -0700
"Angus Ainslie (Purism)" <angus@akkea.ca> wrote:

> The VCNL4040 is almost identical to the VCNL4200 as far as register
> layout goes but just need to check a different ID register location.
> 
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/light/vcnl4000.c | 51 ++++++++++++++++++++++++++++++------
>  1 file changed, 43 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index 308cb2d2b641..12c52f2b853a 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -1,8 +1,9 @@
>  /*
> - * vcnl4000.c - Support for Vishay VCNL4000/4010/4020/4200 combined ambient
> + * vcnl4000.c - Support for Vishay VCNL4000/4010/4020/4040/4200 combined ambient
>   * light and proximity sensor
>   *
>   * Copyright 2012 Peter Meerwald <pmeerw@pmeerw.net>
> + * Copyright 2019 Pursim SPC
>   *
>   * This file is subject to the terms and conditions of version 2 of
>   * the GNU General Public License.  See the file COPYING in the main
> @@ -10,13 +11,14 @@
>   *
>   * IIO driver for:
>   *   VCNL4000/10/20 (7-bit I2C slave address 0x13)
> + *   VCNL4040 (7-bit I2C slave address 0x60)
>   *   VCNL4200 (7-bit I2C slave address 0x51)
>   *
>   * TODO:
>   *   allow to adjust IR current
>   *   proximity threshold and event handling
>   *   periodic ALS/proximity measurement (VCNL4010/20)
> - *   interrupts (VCNL4010/20, VCNL4200)
> + *   interrupts (VCNL4010/20/40, VCNL4200)
>   */
>  
>  #include <linux/module.h>
> @@ -30,6 +32,7 @@
>  #define VCNL4000_DRV_NAME "vcnl4000"
>  #define VCNL4000_PROD_ID	0x01
>  #define VCNL4010_PROD_ID	0x02 /* for VCNL4020, VCNL4010 */
> +#define VCNL4040_PROD_ID	0x86
>  #define VCNL4200_PROD_ID	0x58
>  
>  #define VCNL4000_COMMAND	0x80 /* Command register */
> @@ -49,6 +52,8 @@
>  #define VCNL4200_AL_DATA	0x09 /* Ambient light data */
>  #define VCNL4200_DEV_ID		0x0e /* Device ID, slave address and version */
>  
> +#define VCNL4040_DEV_ID		0x0c /* Device ID and version */
> +
>  /* Bit masks for COMMAND register */
>  #define VCNL4000_AL_RDY		BIT(6) /* ALS data ready? */
>  #define VCNL4000_PS_RDY		BIT(5) /* proximity data ready? */
> @@ -58,6 +63,7 @@
>  enum vcnl4000_device_ids {
>  	VCNL4000,
>  	VCNL4010,
> +	VCNL4040,
>  	VCNL4200,
>  };
>  
> @@ -90,6 +96,7 @@ static const struct i2c_device_id vcnl4000_id[] = {
>  	{ "vcnl4000", VCNL4000 },
>  	{ "vcnl4010", VCNL4010 },
>  	{ "vcnl4020", VCNL4010 },
> +	{ "vcnl4040", VCNL4040 },
>  	{ "vcnl4200", VCNL4200 },
>  	{ }
>  };
> @@ -128,14 +135,26 @@ static int vcnl4000_init(struct vcnl4000_data *data)
>  
>  static int vcnl4200_init(struct vcnl4000_data *data)
>  {
> -	int ret;
> +	int ret, id;
>  
>  	ret = i2c_smbus_read_word_data(data->client, VCNL4200_DEV_ID);
>  	if (ret < 0)
>  		return ret;
>  
> -	if ((ret & 0xff) != VCNL4200_PROD_ID)
> -		return -ENODEV;
> +	id = ret & 0xff;
> +
> +	if (id != VCNL4200_PROD_ID) {
> +		ret = i2c_smbus_read_word_data(data->client, VCNL4040_DEV_ID);
> +		if (ret < 0)
> +			return ret;
> +
> +		id = ret & 0xff;
> +
> +		if (id != VCNL4040_PROD_ID)
> +			return -ENODEV;
> +	}
> +
> +	dev_dbg(&data->client->dev, "device id 0x%x", id);
>  
>  	data->rev = (ret >> 8) & 0xf;
>  
> @@ -150,9 +169,19 @@ static int vcnl4200_init(struct vcnl4000_data *data)
>  	data->al_scale = 24000;
>  	data->vcnl4200_al.reg = VCNL4200_AL_DATA;
>  	data->vcnl4200_ps.reg = VCNL4200_PS_DATA;
> -	/* Integration time is 50ms, but the experiments show 54ms in total. */
> -	data->vcnl4200_al.sampling_rate = ktime_set(0, 54000 * 1000);
> -	data->vcnl4200_ps.sampling_rate = ktime_set(0, 4200 * 1000);
> +	switch (id) {
> +	case VCNL4200_PROD_ID:
> +		/* Integration time is 50ms, but the experiments */
> +		/* show 54ms in total. */
> +		data->vcnl4200_al.sampling_rate = ktime_set(0, 54000 * 1000);
> +		data->vcnl4200_ps.sampling_rate = ktime_set(0, 4200 * 1000);
> +		break;
> +	case VCNL4040_PROD_ID:
> +		/* Integration time is 80ms, add 10ms. */
> +		data->vcnl4200_al.sampling_rate = ktime_set(0, 100000 * 1000);
> +		data->vcnl4200_ps.sampling_rate = ktime_set(0, 100000 * 1000);
> +		break;
> +	}
>  	data->vcnl4200_al.last_measurement = ktime_set(0, 0);
>  	data->vcnl4200_ps.last_measurement = ktime_set(0, 0);
>  	mutex_init(&data->vcnl4200_al.lock);
> @@ -271,6 +300,12 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
>  		.measure_light = vcnl4000_measure_light,
>  		.measure_proximity = vcnl4000_measure_proximity,
>  	},
> +	[VCNL4040] = {
> +		.prod = "VCNL4040",
> +		.init = vcnl4200_init,
> +		.measure_light = vcnl4200_measure_light,
> +		.measure_proximity = vcnl4200_measure_proximity,
> +	},
>  	[VCNL4200] = {
>  		.prod = "VCNL4200",
>  		.init = vcnl4200_init,