iio: vcnl4000 ALS/proximity driver

iio: vcnl4000 ALS/proximity driver

[Peter Meerwald]

I'm submitting a minimal, work-in-progress driver for the 
Vishay VCNL4000 combined ambient light/proximity sensor to -staging

the driver is missing a way to adjust the IR current determining 
the range for proximity sensing

is scale/calibscale desirable?
I am lost with the meaning and use of IIO modifiers...

ultimately, periodic proximity sensing and thresholding with event 
handling should be implemented... I do have a misc/input driver for 
that but want to built upon IIO

[PATCH] iio: add vcnl4000 combined ALS and proximity sensor

[Peter Meerwald]

minimal driver, submitting to staging

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/light/Kconfig    |   11 ++
 drivers/staging/iio/light/Makefile   |    1 +
 drivers/staging/iio/light/vcnl4000.c |  265 ++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/staging/iio/light/vcnl4000.c

diff --git a/drivers/staging/iio/light/Kconfig b/drivers/staging/iio/light/Kconfig
index 4bed30e..6b8d022 100644
--- a/drivers/staging/iio/light/Kconfig
+++ b/drivers/staging/iio/light/Kconfig
@@ -50,4 +50,15 @@ config TSL2x7x
 	 tmd2672, tsl2772, tmd2772 devices.
 	 Provides iio_events and direct access via sysfs.
 
+config SENSORS_VCNL4000
+	tristate "VCNL4000 combined ALS and proximity sensor"
+	depends on I2C
+	default n
+	---help---
+	  Say Y here if you want to build a driver for the VCNL4000
+	  combined ambient light and proximity sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called vcnl4000.
+
 endmenu
diff --git a/drivers/staging/iio/light/Makefile b/drivers/staging/iio/light/Makefile
index 141af1e..ea2c2f2 100644
--- a/drivers/staging/iio/light/Makefile
+++ b/drivers/staging/iio/light/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_SENSORS_ISL29018)	+= isl29018.o
 obj-$(CONFIG_SENSORS_ISL29028)	+= isl29028.o
 obj-$(CONFIG_TSL2583)	+= tsl2583.o
 obj-$(CONFIG_TSL2x7x)	+= tsl2x7x_core.o
+obj-$(CONFIG_SENSORS_VCNL4000)	+= vcnl4000.o
diff --git a/drivers/staging/iio/light/vcnl4000.c b/drivers/staging/iio/light/vcnl4000.c
new file mode 100644
index 0000000..c513b4b
--- /dev/null
+++ b/drivers/staging/iio/light/vcnl4000.c
@@ -0,0 +1,265 @@
+/*
+ * vcnl4000.c - Support for Vishay VCNL4000 combined ambient light and
+ * proximity sensor
+ *
+ * Copyright 2012 Peter Meerwald <pmeerw@pmeerw.net>
+ *
+ * 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
+ * directory of this archive for more details.
+ *
+ * IIO driver for VCNL4000 (7-bit I2C slave address 0x13)
+ *
+ * TODO:
+ *   allow to adjust IR current
+ *   need scale/calibscale?
+ *   proximity threshold and event handling
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define VCNL4000_DRV_NAME "vcnl4000"
+
+#define VCNL4000_COMMAND	0x80 /* Command register */
+#define VCNL4000_PROD_REV	0x81 /* Product ID and Revision ID */
+#define VCNL4000_LED_CURRENT	0x83 /* IR LED current for proximity mode */
+#define VCNL4000_AL_PARAM	0x84 /* Ambient light parameter register */
+#define VCNL4000_AL_RESULT_HI	0x85 /* Ambient light result register, MSB */
+#define VCNL4000_AL_RESULT_LO	0x86 /* Ambient light result register, LSB */
+#define VCNL4000_PS_RESULT_HI	0x87 /* Proximity result register, MSB */
+#define VCNL4000_PS_RESULT_LO	0x88 /* Proximity result register, LSB */
+#define VCNL4000_PS_MEAS_FREQ	0x89 /* Proximity measurement signal frequency */
+#define VCNL4000_PS_MOD_ADJ	0x8a /* Proximity modulator timing adjustment */
+
+/* Bit masks for COMMAND register */
+#define VCNL4000_ALS_RDY	0x40 /* ALS data ready? */
+#define VCNL4000_PS_RDY		0x20 /* proximity data ready? */
+#define VCNL4000_ALS_OD		0x10 /* start on-demand ALS measurement */
+#define VCNL4000_PS_OD		0x08 /* start on-demand proximity measurement */
+
+struct vcnl4000_data {
+	struct i2c_client *client;
+};
+
+static const struct i2c_device_id vcnl4000_id[] = {
+	{ "vcnl4000", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, vcnl4000_id);
+
+static int vcnl4000_read(struct vcnl4000_data *data, u8 reg, const char *msg)
+{
+	int ret = i2c_smbus_read_byte_data(data->client, reg);
+	if (ret < 0)
+		dev_err(&data->client->dev,
+			"vcnl4000_read(%s) failed, error %d\n",
+			msg, ret);
+	return ret;
+}
+
+static int vcnl4000_write(struct vcnl4000_data *data,
+				u8 reg, u8 val, const char *msg)
+{
+	int ret = i2c_smbus_write_byte_data(data->client, reg, val);
+	if (ret < 0)
+		dev_err(&data->client->dev,
+			"vcnl4000_write(%s) failed, error %d\n",
+			msg, ret);
+	return ret;
+}
+
+
+static int vcnl4000_get_als(struct vcnl4000_data *data, int *val)
+{
+	u16 buf;
+	int ret;
+
+	ret = vcnl4000_write(data, VCNL4000_COMMAND,
+		VCNL4000_ALS_OD, "COMMAND");
+	if (ret < 0)
+		return ret;
+
+	/* wait for ALS data to become ready */
+	do {
+		ret = vcnl4000_read(data, VCNL4000_COMMAND, "COMMAND");
+		if (ret < 0)
+			return ret;
+	} while (!(ret & VCNL4000_ALS_RDY));
+
+	ret = i2c_smbus_read_i2c_block_data(data->client,
+		VCNL4000_AL_RESULT_HI, sizeof(buf), (u8 *) &buf);
+	if (ret < 0) {
+		dev_err(&data->client->dev,
+			"i2c_smbus_read_block_data() failed, error %d\n", ret);
+		return ret;
+	}
+
+	*val = __be16_to_cpu(buf);
+
+	return 0;
+}
+
+static int vcnl4000_get_ps(struct vcnl4000_data *data, int *val)
+{
+	u16 buf;
+	int ret;
+
+	ret = vcnl4000_write(data, VCNL4000_COMMAND,
+		VCNL4000_PS_OD, "COMMAND");
+	if (ret < 0)
+		return ret;
+
+	/* wait for proximity data to become ready */
+	do {
+		ret = vcnl4000_read(data, VCNL4000_COMMAND, "COMMAND");
+		if (ret < 0)
+			return ret;
+	} while (!(ret & VCNL4000_PS_RDY));
+
+	ret = i2c_smbus_read_i2c_block_data(data->client,
+		VCNL4000_PS_RESULT_HI, sizeof(buf), (u8 *) &buf);
+	if (ret < 0) {
+		dev_err(&data->client->dev,
+			"i2c_smbus_read_block_data() failed, error %d\n", ret);
+		return ret;
+	}
+
+	*val = __be16_to_cpu(buf);
+
+	return 0;
+}
+
+static const struct iio_chan_spec vcnl4000_channels[] = {
+	{
+		.type = IIO_INTENSITY,
+		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,
+	}, {
+		.type = IIO_PROXIMITY,
+		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,
+	}, {
+		.type = IIO_LIGHT,
+		.info_mask = IIO_CHAN_INFO_PROCESSED_SEPARATE_BIT,
+	}
+};
+
+static int vcnl4000_read_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int *val, int *val2, long mask)
+{
+	int ret = -EINVAL;
+	struct vcnl4000_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_INTENSITY:
+			ret = vcnl4000_get_als(data, val);
+			if (ret < 0)
+				return ret;
+			ret = IIO_VAL_INT;
+			break;
+		case IIO_PROXIMITY:
+			ret = vcnl4000_get_ps(data, val);
+			if (ret < 0)
+				return ret;
+			ret = IIO_VAL_INT;
+			break;
+		default:
+			break;
+		}
+		break;
+	case IIO_CHAN_INFO_PROCESSED:
+		if (chan->type == IIO_LIGHT) {
+			ret = vcnl4000_get_als(data, val);
+			if (ret < 0)
+				return ret;
+			*val = (*val + 3) / 4;
+			ret = IIO_VAL_INT;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info vcnl4000_info = {
+	.read_raw = vcnl4000_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int __devinit vcnl4000_probe(struct i2c_client *client,
+					const struct i2c_device_id *id)
+{
+	struct vcnl4000_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = iio_device_alloc(sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	ret = vcnl4000_read(data, VCNL4000_PROD_REV, "PROD_REV");
+	if (ret < 0)
+		goto error_free_dev;
+
+	dev_info(&client->dev, "VCNL4000 Ambient light/proximity sensor, "
+		"Prod %02x, Rev: %02x\n", ret >> 4, ret & 0xf);
+
+	ret = vcnl4000_read(data, VCNL4000_LED_CURRENT, "LED_CURRENT");
+	if (ret < 0)
+		goto error_free_dev;
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &vcnl4000_info;
+	indio_dev->channels = vcnl4000_channels;
+	indio_dev->num_channels = ARRAY_SIZE(vcnl4000_channels);
+	indio_dev->name = VCNL4000_DRV_NAME;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto error_free_dev;
+
+	return 0;
+
+error_free_dev:
+	iio_device_free(indio_dev);
+	return ret;
+}
+
+static int __devexit vcnl4000_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+	iio_device_free(indio_dev);
+
+	return 0;
+}
+
+static struct i2c_driver vcnl4000_driver = {
+	.driver = {
+		.name   = VCNL4000_DRV_NAME,
+		.owner  = THIS_MODULE,
+	},
+	.probe  = vcnl4000_probe,
+	.remove = __devexit_p(vcnl4000_remove),
+	.id_table = vcnl4000_id,
+};
+
+module_i2c_driver(vcnl4000_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("Vishay VCNL4000 proximity/ambient light sensor driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

Re: [PATCH] iio: add vcnl4000 combined ALS and proximity sensor

[Lars-Peter Clausen]

On 06/11/2012 12:39 AM, Peter Meerwald wrote:
> minimal driver, submitting to staging
> 
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

Just some minor nitpicks inline.

> ---
> [...]
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define VCNL4000_DRV_NAME "vcnl4000"
> +
> +#define VCNL4000_COMMAND	0x80 /* Command register */
> +#define VCNL4000_PROD_REV	0x81 /* Product ID and Revision ID */
> +#define VCNL4000_LED_CURRENT	0x83 /* IR LED current for proximity mode */
> +#define VCNL4000_AL_PARAM	0x84 /* Ambient light parameter register */
> +#define VCNL4000_AL_RESULT_HI	0x85 /* Ambient light result register, MSB */
> +#define VCNL4000_AL_RESULT_LO	0x86 /* Ambient light result register, LSB */
> +#define VCNL4000_PS_RESULT_HI	0x87 /* Proximity result register, MSB */
> +#define VCNL4000_PS_RESULT_LO	0x88 /* Proximity result register, LSB */
> +#define VCNL4000_PS_MEAS_FREQ	0x89 /* Proximity measurement signal frequency */
> +#define VCNL4000_PS_MOD_ADJ	0x8a /* Proximity modulator timing adjustment */
> +
> +/* Bit masks for COMMAND register */
> +#define VCNL4000_ALS_RDY	0x40 /* ALS data ready? */
> +#define VCNL4000_PS_RDY		0x20 /* proximity data ready? */
> +#define VCNL4000_ALS_OD		0x10 /* start on-demand ALS measurement */
> +#define VCNL4000_PS_OD		0x08 /* start on-demand proximity measurement */
> +
> +struct vcnl4000_data {
> +	struct i2c_client *client;
> +};
> +
> +static const struct i2c_device_id vcnl4000_id[] = {
> +	{ "vcnl4000", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, vcnl4000_id);
> +
> +static int vcnl4000_read(struct vcnl4000_data *data, u8 reg, const char *msg)
> +{
> +	int ret = i2c_smbus_read_byte_data(data->client, reg);
> +	if (ret < 0)
> +		dev_err(&data->client->dev,
> +			"vcnl4000_read(%s) failed, error %d\n",
> +			msg, ret);
> +	return ret;
> +}
> +
> +static int vcnl4000_write(struct vcnl4000_data *data,
> +				u8 reg, u8 val, const char *msg)
> +{
> +	int ret = i2c_smbus_write_byte_data(data->client, reg, val);
> +	if (ret < 0)
> +		dev_err(&data->client->dev,
> +			"vcnl4000_write(%s) failed, error %d\n",
> +			msg, ret);
> +	return ret;
> +}
> +
> +
> +static int vcnl4000_get_als(struct vcnl4000_data *data, int *val)
> +{
> +	u16 buf;

__be16

> +	int ret;
> +
> +	ret = vcnl4000_write(data, VCNL4000_COMMAND,
> +		VCNL4000_ALS_OD, "COMMAND");
> +	if (ret < 0)
> +		return ret;
> +
> +	/* wait for ALS data to become ready */
> +	do {
> +		ret = vcnl4000_read(data, VCNL4000_COMMAND, "COMMAND");
> +		if (ret < 0)
> +			return ret;
> +	} while (!(ret & VCNL4000_ALS_RDY));

This should have a timeout, so it does not get stuck in a endless loop in
case there is an issue with the hardware.

> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client,
> +		VCNL4000_AL_RESULT_HI, sizeof(buf), (u8 *) &buf);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev,
> +			"i2c_smbus_read_block_data() failed, error %d\n", ret);
> +		return ret;
> +	}
> +
> +	*val = __be16_to_cpu(buf);

just be16_to_cpu, without the underscores

> +
> +	return 0;
> +}
> +
> +static int vcnl4000_get_ps(struct vcnl4000_data *data, int *val)
> +{
> +	u16 buf;
> +	int ret;

This function has the same structure as the one above, maybe put this in a
common function which takes the OD and RDY bits and the RESULT register as
parameters.

> +
> +	ret = vcnl4000_write(data, VCNL4000_COMMAND,
> +		VCNL4000_PS_OD, "COMMAND");
> +	if (ret < 0)
> +		return ret;
> +
> +	/* wait for proximity data to become ready */
> +	do {
> +		ret = vcnl4000_read(data, VCNL4000_COMMAND, "COMMAND");
> +		if (ret < 0)
> +			return ret;
> +	} while (!(ret & VCNL4000_PS_RDY));
> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client,
> +		VCNL4000_PS_RESULT_HI, sizeof(buf), (u8 *) &buf);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev,
> +			"i2c_smbus_read_block_data() failed, error %d\n", ret);
> +		return ret;
> +	}
> +
> +	*val = __be16_to_cpu(buf);
> +
> +	return 0;
> +}
> +
> +static const struct iio_chan_spec vcnl4000_channels[] = {
> +	{
> +		.type = IIO_INTENSITY,
> +		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,
> +	}, {
> +		.type = IIO_PROXIMITY,
> +		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,
> +	}, {
> +		.type = IIO_LIGHT,
> +		.info_mask = IIO_CHAN_INFO_PROCESSED_SEPARATE_BIT,
> +	}
> +};
> +
> +static int vcnl4000_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	int ret = -EINVAL;
> +	struct vcnl4000_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->type) {
> +		case IIO_INTENSITY:
> +			ret = vcnl4000_get_als(data, val);
> +			if (ret < 0)
> +				return ret;
> +			ret = IIO_VAL_INT;
> +			break;
> +		case IIO_PROXIMITY:
> +			ret = vcnl4000_get_ps(data, val);
> +			if (ret < 0)
> +				return ret;
> +			ret = IIO_VAL_INT;
> +			break;
> +		default:
> +			break;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_PROCESSED:
> +		if (chan->type == IIO_LIGHT) {
> +			ret = vcnl4000_get_als(data, val);
> +			if (ret < 0)
> +				return ret;
> +			*val = (*val + 3) / 4;

Does it make sense to do this processing here? Or should it rather return
the raw value and have a scale attribute?

> +			ret = IIO_VAL_INT;
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info vcnl4000_info = {
> +	.read_raw = vcnl4000_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int __devinit vcnl4000_probe(struct i2c_client *client,
> +					const struct i2c_device_id *id)
> +{
> +	struct vcnl4000_data *data;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = iio_device_alloc(sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	ret = vcnl4000_read(data, VCNL4000_PROD_REV, "PROD_REV");
> +	if (ret < 0)
> +		goto error_free_dev;
> +
> +	dev_info(&client->dev, "VCNL4000 Ambient light/proximity sensor, "
> +		"Prod %02x, Rev: %02x\n", ret >> 4, ret & 0xf);
> +
> +	ret = vcnl4000_read(data, VCNL4000_LED_CURRENT, "LED_CURRENT");
> +	if (ret < 0)
> +		goto error_free_dev;

The result of this read is never used.

> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &vcnl4000_info;
> +	indio_dev->channels = vcnl4000_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(vcnl4000_channels);
> +	indio_dev->name = VCNL4000_DRV_NAME;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0)
> +		goto error_free_dev;
> +
> +	return 0;
> +
> +error_free_dev:
> +	iio_device_free(indio_dev);
> +	return ret;
> +}
> +
> +[...]

Re: [PATCH] iio: add vcnl4000 combined ALS and proximity sensor

[Jonathan Cameron]

On 06/10/2012 11:39 PM, Peter Meerwald wrote:
> minimal driver, submitting to staging
> 
If we are happy interfaces that are here aren't going to change,
skip the staging bit and submit under drivers/iio directly.
Nothing wrong with a driver supporting some functionality!

Obviously I'm not going to repeat Lars-Peter's comments
(or not most of them anyway ;)

Mostly short and clean which always makes me happy.

> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> ---
>  drivers/staging/iio/light/Kconfig    |   11 ++
>  drivers/staging/iio/light/Makefile   |    1 +
>  drivers/staging/iio/light/vcnl4000.c |  265 ++++++++++++++++++++++++++++++++++
>  3 files changed, 277 insertions(+)
>  create mode 100644 drivers/staging/iio/light/vcnl4000.c
> 
> diff --git a/drivers/staging/iio/light/Kconfig b/drivers/staging/iio/light/Kconfig
> index 4bed30e..6b8d022 100644
> --- a/drivers/staging/iio/light/Kconfig
> +++ b/drivers/staging/iio/light/Kconfig
> @@ -50,4 +50,15 @@ config TSL2x7x
>  	 tmd2672, tsl2772, tmd2772 devices.
>  	 Provides iio_events and direct access via sysfs.
>  
> +config SENSORS_VCNL4000
> +	tristate "VCNL4000 combined ALS and proximity sensor"
> +	depends on I2C
> +	default n
> +	---help---
> +	  Say Y here if you want to build a driver for the VCNL4000
> +	  combined ambient light and proximity sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called vcnl4000.
> +
>  endmenu
> diff --git a/drivers/staging/iio/light/Makefile b/drivers/staging/iio/light/Makefile
> index 141af1e..ea2c2f2 100644
> --- a/drivers/staging/iio/light/Makefile
> +++ b/drivers/staging/iio/light/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_SENSORS_ISL29018)	+= isl29018.o
>  obj-$(CONFIG_SENSORS_ISL29028)	+= isl29028.o
>  obj-$(CONFIG_TSL2583)	+= tsl2583.o
>  obj-$(CONFIG_TSL2x7x)	+= tsl2x7x_core.o
> +obj-$(CONFIG_SENSORS_VCNL4000)	+= vcnl4000.o
> diff --git a/drivers/staging/iio/light/vcnl4000.c b/drivers/staging/iio/light/vcnl4000.c
> new file mode 100644
> index 0000000..c513b4b
> --- /dev/null
> +++ b/drivers/staging/iio/light/vcnl4000.c
> @@ -0,0 +1,265 @@
> +/*
> + * vcnl4000.c - Support for Vishay VCNL4000 combined ambient light and
> + * proximity sensor
> + *
> + * Copyright 2012 Peter Meerwald <pmeerw@pmeerw.net>
> + *
> + * 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
> + * directory of this archive for more details.
> + *
> + * IIO driver for VCNL4000 (7-bit I2C slave address 0x13)
> + *
> + * TODO:
> + *   allow to adjust IR current
> + *   need scale/calibscale?
> + *   proximity threshold and event handling
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define VCNL4000_DRV_NAME "vcnl4000"
> +
> +#define VCNL4000_COMMAND	0x80 /* Command register */
> +#define VCNL4000_PROD_REV	0x81 /* Product ID and Revision ID */
> +#define VCNL4000_LED_CURRENT	0x83 /* IR LED current for proximity mode */
> +#define VCNL4000_AL_PARAM	0x84 /* Ambient light parameter register */
> +#define VCNL4000_AL_RESULT_HI	0x85 /* Ambient light result register, MSB */
> +#define VCNL4000_AL_RESULT_LO	0x86 /* Ambient light result register, LSB */
> +#define VCNL4000_PS_RESULT_HI	0x87 /* Proximity result register, MSB */
> +#define VCNL4000_PS_RESULT_LO	0x88 /* Proximity result register, LSB */
> +#define VCNL4000_PS_MEAS_FREQ	0x89 /* Proximity measurement signal frequency */
> +#define VCNL4000_PS_MOD_ADJ	0x8a /* Proximity modulator timing adjustment */
> +
> +/* Bit masks for COMMAND register */
> +#define VCNL4000_ALS_RDY	0x40 /* ALS data ready? */
> +#define VCNL4000_PS_RDY		0x20 /* proximity data ready? */
> +#define VCNL4000_ALS_OD		0x10 /* start on-demand ALS measurement */
> +#define VCNL4000_PS_OD		0x08 /* start on-demand proximity measurement */
> +
> +struct vcnl4000_data {
> +	struct i2c_client *client;
> +};
> +
> +static const struct i2c_device_id vcnl4000_id[] = {
> +	{ "vcnl4000", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, vcnl4000_id);
> +
> +static int vcnl4000_read(struct vcnl4000_data *data, u8 reg, const char *msg)
> +{
> +	int ret = i2c_smbus_read_byte_data(data->client, reg);
> +	if (ret < 0)
> +		dev_err(&data->client->dev,
> +			"vcnl4000_read(%s) failed, error %d\n",
> +			msg, ret);
Personally I'm a fan of not bothering to wrap standard functions like
this.  The debugging info (which 'should' never happen) isn't
worth the added lines of code...  Still if you are particularly
attached to them, I'm not that bothered.  I'm particularly
anti the passing of a string purely to paste into an error
message....

> +	return ret;
> +}
> +
> +static int vcnl4000_write(struct vcnl4000_data *data,
> +				u8 reg, u8 val, const char *msg)
> +{
> +	int ret = i2c_smbus_write_byte_data(data->client, reg, val);
> +	if (ret < 0)
> +		dev_err(&data->client->dev,
> +			"vcnl4000_write(%s) failed, error %d\n",
> +			msg, ret);
> +	return ret;
> +}
> +
> +
> +static int vcnl4000_get_als(struct vcnl4000_data *data, int *val)
> +{
> +	u16 buf;
> +	int ret;
> +
> +	ret = vcnl4000_write(data, VCNL4000_COMMAND,
> +		VCNL4000_ALS_OD, "COMMAND");
> +	if (ret < 0)
> +		return ret;
> +
> +	/* wait for ALS data to become ready */
> +	do {
> +		ret = vcnl4000_read(data, VCNL4000_COMMAND, "COMMAND");
> +		if (ret < 0)
> +			return ret;
> +	} while (!(ret & VCNL4000_ALS_RDY));
> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client,
> +		VCNL4000_AL_RESULT_HI, sizeof(buf), (u8 *) &buf);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev,
> +			"i2c_smbus_read_block_data() failed, error %d\n", ret);
More informative to say what the read is of? Don't think we care
that it was a block data read...
> +		return ret;
> +	}
> +
> +	*val = __be16_to_cpu(buf);
> +
> +	return 0;
> +}
> +
> +static int vcnl4000_get_ps(struct vcnl4000_data *data, int *val)
> +{
> +	u16 buf;
> +	int ret;
> +
> +	ret = vcnl4000_write(data, VCNL4000_COMMAND,
> +		VCNL4000_PS_OD, "COMMAND");
> +	if (ret < 0)
> +		return ret;
> +
> +	/* wait for proximity data to become ready */
> +	do {
> +		ret = vcnl4000_read(data, VCNL4000_COMMAND, "COMMAND");
> +		if (ret < 0)
> +			return ret;
> +	} while (!(ret & VCNL4000_PS_RDY));
> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client,
> +		VCNL4000_PS_RESULT_HI, sizeof(buf), (u8 *) &buf);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev,
> +			"i2c_smbus_read_block_data() failed, error %d\n", ret);
> +		return ret;
> +	}
> +
> +	*val = __be16_to_cpu(buf);
> +
> +	return 0;
> +}
> +
> +static const struct iio_chan_spec vcnl4000_channels[] = {
> +	{
> +		.type = IIO_INTENSITY,
> +		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,
> +	}, {
> +		.type = IIO_PROXIMITY,
> +		.info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT,
> +	}, {
> +		.type = IIO_LIGHT,
Some indication of what frequency range is covered would be good.
The modifiers are a bit less than ideal but there we are ;)
> +		.info_mask = IIO_CHAN_INFO_PROCESSED_SEPARATE_BIT,
> +	}
> +};
> +
> +static int vcnl4000_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	int ret = -EINVAL;
> +	struct vcnl4000_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->type) {
> +		case IIO_INTENSITY:
> +			ret = vcnl4000_get_als(data, val);
> +			if (ret < 0)
> +				return ret;
> +			ret = IIO_VAL_INT;
> +			break;
> +		case IIO_PROXIMITY:
> +			ret = vcnl4000_get_ps(data, val);
> +			if (ret < 0)
> +				return ret;
> +			ret = IIO_VAL_INT;
> +			break;
> +		default:
> +			break;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_PROCESSED:
> +		if (chan->type == IIO_LIGHT) {
> +			ret = vcnl4000_get_als(data, val);
> +			if (ret < 0)
> +				return ret;
> +			*val = (*val + 3) / 4;
> +			ret = IIO_VAL_INT;
Wouldn't bothere with both processed and raw here given simplicity
of the calculation. Pick one and run with it.
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info vcnl4000_info = {
> +	.read_raw = vcnl4000_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int __devinit vcnl4000_probe(struct i2c_client *client,
> +					const struct i2c_device_id *id)
> +{
> +	struct vcnl4000_data *data;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = iio_device_alloc(sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	ret = vcnl4000_read(data, VCNL4000_PROD_REV, "PROD_REV");
> +	if (ret < 0)
> +		goto error_free_dev;
> +
> +	dev_info(&client->dev, "VCNL4000 Ambient light/proximity sensor, "
> +		"Prod %02x, Rev: %02x\n", ret >> 4, ret & 0xf);
> +
> +	ret = vcnl4000_read(data, VCNL4000_LED_CURRENT, "LED_CURRENT");
> +	if (ret < 0)
> +		goto error_free_dev;
Why?
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &vcnl4000_info;
> +	indio_dev->channels = vcnl4000_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(vcnl4000_channels);
> +	indio_dev->name = VCNL4000_DRV_NAME;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0)
> +		goto error_free_dev;
> +
> +	return 0;
> +
> +error_free_dev:
> +	iio_device_free(indio_dev);
> +	return ret;
> +}
> +
> +static int __devexit vcnl4000_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_device_free(indio_dev);
> +
> +	return 0;
> +}
> +
> +static struct i2c_driver vcnl4000_driver = {
> +	.driver = {
> +		.name   = VCNL4000_DRV_NAME,
> +		.owner  = THIS_MODULE,
> +	},
> +	.probe  = vcnl4000_probe,
> +	.remove = __devexit_p(vcnl4000_remove),
> +	.id_table = vcnl4000_id,
> +};
> +
> +module_i2c_driver(vcnl4000_driver);
> +
> +MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
> +MODULE_DESCRIPTION("Vishay VCNL4000 proximity/ambient light sensor driver");
> +MODULE_LICENSE("GPL");

Re: iio: vcnl4000 ALS/proximity driver

[Jonathan Cameron]

On 06/10/2012 11:39 PM, Peter Meerwald wrote:
> I'm submitting a minimal, work-in-progress driver for the 
> Vishay VCNL4000 combined ambient light/proximity sensor to -staging
> 
> the driver is missing a way to adjust the IR current determining 
> the range for proximity sensing
> 
> is scale/calibscale desirable?
> I am lost with the meaning and use of IIO modifiers...
Quick and dirty summary.
Calibscale is internally applied within the hardware (or occasionally
software but looks like it was in the hardware from point of view
of userspace!)

scale is applied in userspace to convert to SI units.
> 
> ultimately, periodic proximity sensing and thresholding with event 
> handling should be implemented... I do have a misc/input driver for 
> that but want to built upon IIO
cool.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iio: vcnl4000 ALS/proximity driver

[Peter Meerwald]

Hello Jonathan,

> > I am lost with the meaning and use of IIO modifiers...
> Quick and dirty summary.
> Calibscale is internally applied within the hardware (or occasionally
> software but looks like it was in the hardware from point of view
> of userspace!)

thank you for clarification

Am I missing documentation somewhere?


some comments in iio/types.h iio/iio.h would avoid guesswork...

enum iio_chan_type {
	/* real channel types */	
are there unreal channels also? :)

what is IIO_ANGL_VEL vs. IIO_ANGL? VOLTAGE vs. ALTVOLTAGE?

what is LIGHT_BOTH?

it would be helpful what IIO_CHAN_INFO_SHARED_BIT vs 
IIO_CHAN_INFO_SEPARATE_BIT relates to?

IIO_CHAN_INFO_SAMP_FREQ vs IIO_CHAN_INFO_FREQUENCY?

thanks, regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: iio: vcnl4000 ALS/proximity driver

[Jonathan Cameron]

Peter Meerwald <pmeerw@pmeerw.net> wrote:

>Hello Jonathan,
>
>> > I am l=
ost with the meaning and use of IIO modifiers...
>> Quick and dirty summary=
.
>> Calibscale is internally applied within the hardware (or occasionally
=
>> software but looks like it was in the hardware from point of view
>> of =
userspace!)
>
>thank you for clarification
>
>Am I missing documentation so=
mewhere?
>
Documentation/ABI/testing/sysfs-bus-iio.

>
>some comments in ii=
o/types.h iio/iio.h would avoid guesswork...
>
>enum iio_chan_type {
>	/* r=
eal channel types */	
>are there unreal channels also? :)
>
>what is IIO_AN=
GL_VEL vs. IIO_ANGL? VOLTAGE vs. ALTVOLTAGE?
>
>what is LIGHT_BOTH?
>
>it w=
ould be helpful what IIO_CHAN_INFO_SHARED_BIT vs 
>IIO_CHAN_INFO_SEPARATE_B=
IT relates to?
>
>IIO_CHAN_INFO_SAMP_FREQ vs IIO_CHAN_INFO_FREQUENCY?
>
>th=
anks, regards, p.
>
>-- 
>
>Peter Meerwald
>+43-664-2444418 (mobile)
>--
>T=
o unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the=
 body of a message to majordomo@vger.kernel.org
>More majordomo info at  ht=
tp://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android phone wi=
th K-9 Mail. Please excuse my brevity.

Re: iio: vcnl4000 ALS/proximity driver

[Jonathan Cameron]

On 6/12/2012 6:49 AM, Jonathan Cameron wrote:
>
> Peter Meerwald<pmeerw@pmeerw.net>  wrote:
>
>> Hello Jonathan,
>>
>>>> I am lost with the meaning and use of IIO modifiers...
>>> Quick and dirty summary.
>>> Calibscale is internally applied within the hardware (or occasionally
>>> software but looks like it was in the hardware from point of view
>>> of userspace!)
>> thank you for clarification
>>
>> Am I missing documentation somewhere?
>>
> Documentation/ABI/testing/sysfs-bus-iio.
+ drivers/staging/iio/documentation/sysfs-* for other bits and bobs. 
Mostly it
should be fairly obvious how stuff lines up.
>
>> some comments in iio/types.h iio/iio.h would avoid guesswork...
>>
>> enum iio_chan_type {
>> 	/* real channel types */	
>> are there unreal channels also? :)
ah, there were.  That comment is out of date.  This sort of thing is why
we try to keep minimal commenting in there unless absolutely necessary.
I suppose we could put a reference to say see the docs files...
>> what is IIO_ANGL_VEL vs. IIO_ANGL? VOLTAGE vs. ALTVOLTAGE?
Those should be pretty obvious and line up with the documentation.
  Angl_vel is the angular velocity. angl is the the angle.  Voltage is a 
dc voltage,
altvoltage is an alternating voltage. (this can mater as some 'interesting'
parts can sample the instantaneous voltage and also provide the rms voltage
under the assumption that it's a sinusoid.).
>>
>> what is LIGHT_BOTH?
Visible + infrared  (agreed that one is not good.  We've talked about
changing that, but nothing has happened about it yet...)
>>
>> it would be helpful what IIO_CHAN_INFO_SHARED_BIT vs
>> IIO_CHAN_INFO_SEPARATE_BIT relates to?
Maybe that one needs a comment..
Shared is across a channel type. E.g. in_accel_scale
vs separate which is per channel in_accel_x_scale.
>>
>> IIO_CHAN_INFO_SAMP_FREQ vs IIO_CHAN_INFO_FREQUENCY?
That should be in the sysfs docs.  samping frequency refers to sampling
whereas frequency refers to the frequency of the thing being measured
or (more typically) output.
>>
>> thanks, regards, p.
>>
>> -- 
>>
>> Peter Meerwald
>> +43-664-2444418 (mobile)
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: iio: vcnl4000 ALS/proximity driver

[Peter Meerwald]

Hello,

> > > Am I missing documentation somewhere?
> > > 
> > Documentation/ABI/testing/sysfs-bus-iio.
> + drivers/staging/iio/documentation/sysfs-* for other bits and bobs. Mostly it
> should be fairly obvious how stuff lines up.

well, the last statement should be up to the user of the documentation, 
not the creator ;)

> > > some comments in iio/types.h iio/iio.h would avoid guesswork...
> ah, there were.  That comment is out of date.  This sort of thing is why
> we try to keep minimal commenting in there unless absolutely necessary.

is the What: line in sysfs-bus-iio supposed to be comprehensive?

I am worried because concepts are explained redundantly in sysfs-bus-iio 
for particular channel types; one such concept is that a raw 
value can/shall be modified by userspace by offset and scale

this could be easily (?) done near the iio_chan_info_enum definition in 
iio.h

sysfs-bus-iio documents what you might see in the wild but not how to 
get there

> I suppose we could put a reference to say see the docs files...

I have seen the doc, but I find it hard to deduce the concepts from a 
collection of sysfs file names

it is not clear/obvious how to arrive at 
/sys/.../iio:deviceX/in_capacitanceY-in_capacitanceZ_raw or
/sys/bus/iio/devices/iio:deviceX/out_voltageY&Z_raw

in iio.h:
 * @channel:		What number or name do we wish to assign the channel.

what is meant by 'name' here? 

so channel is a numerical identifier that is appended to the channel name 
if indexed is 1?

* @indexed:		Specify the channel has a numerical index. If not,
 *			the value in channel will be suppressed for attribute
 *			but not for event codes. Typically set it to 0 when
 *			the index is false.

I am not sure what an 'attribute code' is (sysfs name?)
I am not sure what 'index is false' means


regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: iio: vcnl4000 ALS/proximity driver

[Jonathan Cameron]

On 6/12/2012 10:48 AM, Peter Meerwald wrote:
> Hello,
>
>>>> Am I missing documentation somewhere?
>>>>
>>> Documentation/ABI/testing/sysfs-bus-iio.
>> + drivers/staging/iio/documentation/sysfs-* for other bits and bobs. Mostly it
>> should be fairly obvious how stuff lines up.
>
> well, the last statement should be up to the user of the documentation,
> not the creator ;)
True enough.
>
>>>> some comments in iio/types.h iio/iio.h would avoid guesswork...
>> ah, there were.  That comment is out of date.  This sort of thing is why
>> we try to keep minimal commenting in there unless absolutely necessary.
>
> is the What: line in sysfs-bus-iio supposed to be comprehensive?
Yes.  All sysfs entries should be documented.
If a specific varient of a general element should be in sysfs-bus-iio
If class specific (e.g. only make sense for light sensors) should be
in sysfs-bus-iio-light etc. If device specific (and these take a fair
bit of persuation to be let in) should be in sysfs-bus-iio-light-tsl2563 
etc.
>
> I am worried because concepts are explained redundantly in sysfs-bus-iio
> for particular channel types; one such concept is that a raw
> value can/shall be modified by userspace by offset and scale
Good to clean those up, but they must be documented in those file.  It's
pretty much a kernel rule. Those docs are for writers of userspace code
as well. They shouldn't have to go dig in headers to find this stuff out.
>
> this could be easily (?) done near the iio_chan_info_enum definition in
> iio.h

it would be nice to do it there, but the abi docs are the definitive source.
>
> sysfs-bus-iio documents what you might see in the wild but not how to
> get there
true.
>
>> I suppose we could put a reference to say see the docs files...
>
> I have seen the doc, but I find it hard to deduce the concepts from a
> collection of sysfs file names
>
> it is not clear/obvious how to arrive at
> /sys/.../iio:deviceX/in_capacitanceY-in_capacitanceZ_raw or
Nice complex case...
> /sys/bus/iio/devices/iio:deviceX/out_voltageY&Z_raw
err. that's curious. You can't set that...  Goes to show I should review
changes to this file more carefully. I just shouted down adding the 
stuff that would provide that precisely because it made no sense.
>
> in iio.h:
>   * @channel:		What number or name do we wish to assign the channel.
>
> what is meant by 'name' here?
Oops. Comment has rotted.  Drop the name bit if you fancy doing a patch.
>
> so channel is a numerical identifier that is appended to the channel name
> if indexed is 1?
yes.
>
> * @indexed:		Specify the channel has a numerical index. If not,
>   *			the value in channel will be suppressed for attribute
>   *			but not for event codes. Typically set it to 0 when
>   *			the index is false.
>
> I am not sure what an 'attribute code' is (sysfs name?)
attribute name  (which is just a sysfs name as you call it) Again,
could do with fixing.
> I am not sure what 'index is false' means
index  == 0; Oops, more good points. Thanks!

Given you are doing an excellent job, feel free to send patches.
>
>
> regards, p.
>

Re: iio: vcnl4000 ALS/proximity driver

[Jonathan Cameron]

On 6/12/2012 12:01 PM, Jonathan Cameron wrote:
> On 6/12/2012 10:48 AM, Peter Meerwald wrote:
>> Hello,
>>
>>>>> Am I missing documentation somewhere?
>>>>>
>>>> Documentation/ABI/testing/sysfs-bus-iio.
>>> + drivers/staging/iio/documentation/sysfs-* for other bits and bobs.
>>> Mostly it
>>> should be fairly obvious how stuff lines up.
>>
>> well, the last statement should be up to the user of the documentation,
>> not the creator ;)
> True enough.
>>
>>>>> some comments in iio/types.h iio/iio.h would avoid guesswork...
>>> ah, there were. That comment is out of date. This sort of thing is why
>>> we try to keep minimal commenting in there unless absolutely necessary.
>>
>> is the What: line in sysfs-bus-iio supposed to be comprehensive?
> Yes. All sysfs entries should be documented.
> If a specific varient of a general element should be in sysfs-bus-iio
> If class specific (e.g. only make sense for light sensors) should be
> in sysfs-bus-iio-light etc. If device specific (and these take a fair
> bit of persuation to be let in) should be in sysfs-bus-iio-light-tsl2563
> etc.
>>
>> I am worried because concepts are explained redundantly in sysfs-bus-iio
>> for particular channel types; one such concept is that a raw
>> value can/shall be modified by userspace by offset and scale
> Good to clean those up, but they must be documented in those file. It's
> pretty much a kernel rule. Those docs are for writers of userspace code
> as well. They shouldn't have to go dig in headers to find this stuff out.
>>
>> this could be easily (?) done near the iio_chan_info_enum definition in
>> iio.h
>
> it would be nice to do it there, but the abi docs are the definitive
> source.
>>
>> sysfs-bus-iio documents what you might see in the wild but not how to
>> get there
> true.
>>
>>> I suppose we could put a reference to say see the docs files...
>>
>> I have seen the doc, but I find it hard to deduce the concepts from a
>> collection of sysfs file names
>>
>> it is not clear/obvious how to arrive at
>> /sys/.../iio:deviceX/in_capacitanceY-in_capacitanceZ_raw or
> Nice complex case...
>> /sys/bus/iio/devices/iio:deviceX/out_voltageY&Z_raw
> err. that's curious. You can't set that... Goes to show I should review
> changes to this file more carefully. I just shouted down adding the
> stuff that would provide that precisely because it made no sense.
I also don't think anyone is using that. If you are in the patching mood
feel free to squash that entry in the docs.
>>
>> in iio.h:
>> * @channel: What number or name do we wish to assign the channel.
>>
>> what is meant by 'name' here?
> Oops. Comment has rotted. Drop the name bit if you fancy doing a patch.
>>
>> so channel is a numerical identifier that is appended to the channel name
>> if indexed is 1?
> yes.
>>
>> * @indexed: Specify the channel has a numerical index. If not,
>> * the value in channel will be suppressed for attribute
>> * but not for event codes. Typically set it to 0 when
>> * the index is false.
>>
>> I am not sure what an 'attribute code' is (sysfs name?)
> attribute name (which is just a sysfs name as you call it) Again,
> could do with fixing.
>> I am not sure what 'index is false' means
> index == 0; Oops, more good points. Thanks!
>
> Given you are doing an excellent job, feel free to send patches.
>>
>>
>> regards, p.
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: iio: vcnl4000 ALS/proximity driver

[Peter Meerwald]

Hello Jonathan,

> > /sys/bus/iio/devices/iio:deviceX/out_voltageY&Z_raw
> err. that's curious. You can't set that...  Goes to show I should review
> changes to this file more carefully. I just shouted down adding the stuff that
> would provide that precisely because it made no sense.

I leave that to you

> > in iio.h:
> >   * @channel:		What number or name do we wish to assign the channel.
> > what is meant by 'name' here?
> Oops. Comment has rotted.  Drop the name bit if you fancy doing a patch.

will do

> > * @indexed:		Specify the channel has a numerical index. If not,
> >   *			the value in channel will be suppressed for attribute
> >   *			but not for event codes. Typically set it to 0 when
> >   *			the index is false.
> > I am not sure what an 'attribute code' is (sysfs name?)
> attribute name  (which is just a sysfs name as you call it) Again,
> could do with fixing.

* @indexed:		Specify the channel has a numerical index. If not,
 *			the channel index number will be suppressed for sysfs 
 *			attributes but not for event codes. Typically set it 
 *			to 0 when the index is 0.

> > I am not sure what 'index is false' means
> index  == 0; Oops, more good points. Thanks!

or should it be 'Typically set it to 0 when the index is 0 and there is 
just one channel.'?

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: iio: vcnl4000 ALS/proximity driver

[Jonathan Cameron]

On 6/12/2012 1:12 PM, Peter Meerwald wrote:
> Hello Jonathan,
>
>>> /sys/bus/iio/devices/iio:deviceX/out_voltageY&Z_raw
>> err. that's curious. You can't set that...  Goes to show I should review
>> changes to this file more carefully. I just shouted down adding the stuff that
>> would provide that precisely because it made no sense.
>
> I leave that to you
>
>>> in iio.h:
>>>    * @channel:		What number or name do we wish to assign the channel.
>>> what is meant by 'name' here?
>> Oops. Comment has rotted.  Drop the name bit if you fancy doing a patch.
>
> will do
>
>>> * @indexed:		Specify the channel has a numerical index. If not,
>>>    *			the value in channel will be suppressed for attribute
>>>    *			but not for event codes. Typically set it to 0 when
>>>    *			the index is false.
>>> I am not sure what an 'attribute code' is (sysfs name?)
>> attribute name  (which is just a sysfs name as you call it) Again,
>> could do with fixing.
>
> * @indexed:		Specify the channel has a numerical index. If not,
>   *			the channel index number will be suppressed for sysfs
>   *			attributes but not for event codes. Typically set it
>   *			to 0 when the index is 0.
>
>>> I am not sure what 'index is false' means
>> index  == 0; Oops, more good points. Thanks!
>
> or should it be 'Typically set it to 0 when the index is 0 and there is
> just one channel.'?
I'd just drop the typically bit entirely.... It's gibberish as you
pointed out...
>
> regards, p.
>