* [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver
@ 2012-07-01 10:20 Peter Meerwald
2012-07-01 10:20 ` [PATCH] iio: kernel version typo in sysfs-bus-iio Peter Meerwald
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Peter Meerwald @ 2012-07-01 10:20 UTC (permalink / raw)
To: linux-iio; +Cc: Peter Meerwald
sensor has 4 channels (10-bit each, R/G/B and clear), sensitivity
and gain is controlled in the driver by ext_info integration_time
and CHAN_INFO_HARDWAREGAIN
driver supports triggered buffer and IIO_CHAN_INFO_RAW to get the
sensor data
patch depends on IIO_MOD_LIGHT_RED etc, the patch providing that
(http://permalink.gmane.org/gmane.linux.kernel.iio/4354) has not been
merged yet, a new proposal is following
v3:
* fix warnings
v2: address comments by Lars-Peter Clausen
* buffer allocation now in update_scan_mode instead of in trigger
handler
* simplify trigger code (assume active_scan_mask is not empty, use
for_each_set_bit, use iio_push_to_buffer)
* reorder entry in Makefile and Kconfig
* fix remove
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
drivers/iio/light/Kconfig | 12 ++
drivers/iio/light/Makefile | 1 +
drivers/iio/light/adjd_s311.c | 395 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 408 insertions(+)
create mode 100644 drivers/iio/light/adjd_s311.c
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index f3ea90d..91d15d2 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -3,6 +3,18 @@
#
menu "Light sensors"
+config ADJD_S311
+ tristate "ADJD-S311-CR999 digital color sensor"
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ depends on I2C
+ help
+ If you say yes here you get support for the Avago ADJD-S311-CR999
+ digital color light sensor.
+
+ This driver can also be built as a module. If so, the module
+ will be called adjd_s311.
+
config SENSORS_LM3533
tristate "LM3533 ambient light sensor"
depends on MFD_LM3533
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index 06fa4d3..13f8a78 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -2,5 +2,6 @@
# Makefile for IIO Light sensors
#
+obj-$(CONFIG_ADJD_S311) += adjd_s311.o
obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o
obj-$(CONFIG_VCNL4000) += vcnl4000.o
diff --git a/drivers/iio/light/adjd_s311.c b/drivers/iio/light/adjd_s311.c
new file mode 100644
index 0000000..fdd9df6
--- /dev/null
+++ b/drivers/iio/light/adjd_s311.c
@@ -0,0 +1,395 @@
+/*
+ * adjd_s311.c - Support for ADJD-S311-CR999 digital color sensor
+ *
+ * Copyright (C) 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.
+ *
+ * driver for ADJD-S311-CR999 digital color sensor (10-bit channels for
+ * red, green, blue, clear); 7-bit I2C slave address 0x74
+ *
+ * limitations: no calibration, no offset mode, no sleep mode
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/bitmap.h>
+#include <linux/err.h>
+#include <linux/irq.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#define ADJD_S311_DRV_NAME "adjd_s311"
+
+#define ADJD_S311_CTRL 0x00
+#define ADJD_S311_CONFIG 0x01
+#define ADJD_S311_CAP_RED 0x06
+#define ADJD_S311_CAP_GREEN 0x07
+#define ADJD_S311_CAP_BLUE 0x08
+#define ADJD_S311_CAP_CLEAR 0x09
+#define ADJD_S311_INT_RED_LO 0x0a
+#define ADJD_S311_INT_RED_HI 0x0b
+#define ADJD_S311_INT_GREEN_LO 0x0c
+#define ADJD_S311_INT_GREEN_HI 0x0d
+#define ADJD_S311_INT_BLUE_LO 0x0e
+#define ADJD_S311_INT_BLUE_HI 0x0f
+#define ADJD_S311_INT_CLEAR_LO 0x10
+#define ADJD_S311_INT_CLEAR_HI 0x11
+#define ADJD_S311_DATA_RED_LO 0x40
+#define ADJD_S311_DATA_RED_HI 0x41
+#define ADJD_S311_DATA_GREEN_LO 0x42
+#define ADJD_S311_DATA_GREEN_HI 0x43
+#define ADJD_S311_DATA_BLUE_LO 0x44
+#define ADJD_S311_DATA_BLUE_HI 0x45
+#define ADJD_S311_DATA_CLEAR_LO 0x46
+#define ADJD_S311_DATA_CLEAR_HI 0x47
+#define ADJD_S311_OFFSET_RED 0x48
+#define ADJD_S311_OFFSET_GREEN 0x49
+#define ADJD_S311_OFFSET_BLUE 0x4a
+#define ADJD_S311_OFFSET_CLEAR 0x4b
+
+#define ADJD_S311_CTRL_GOFS 0x02
+#define ADJD_S311_CTRL_GSSR 0x01
+#define ADJD_S311_CAP_MASK 0x0f
+#define ADJD_S311_INT_MASK 0x0fff
+#define ADJD_S311_DATA_MASK 0x03ff
+
+struct adjd_s311_data {
+ struct i2c_client *client;
+ u16 *buffer;
+};
+
+enum adjd_s311_channel_idx {
+ IDX_RED, IDX_GREEN, IDX_BLUE, IDX_CLEAR
+};
+
+#define ADJD_S311_DATA_REG(chan) \
+ (ADJD_S311_DATA_RED_LO + (chan) * 2)
+#define ADJD_S311_INT_REG(chan) \
+ (ADJD_S311_INT_RED_LO + (chan) * 2)
+#define ADJD_S311_CAP_REG(chan) \
+ (ADJD_S311_CAP_RED + (chan))
+
+static int adjd_s311_req_data(struct iio_dev *indio_dev)
+{
+ struct adjd_s311_data *data = iio_priv(indio_dev);
+ int tries = 10;
+
+ int ret = i2c_smbus_write_byte_data(data->client, ADJD_S311_CTRL,
+ ADJD_S311_CTRL_GSSR);
+ if (ret < 0)
+ return ret;
+
+ while (tries--) {
+ ret = i2c_smbus_read_byte_data(data->client, ADJD_S311_CTRL);
+ if (ret < 0)
+ return ret;
+ if (!(ret & ADJD_S311_CTRL_GSSR))
+ break;
+ msleep(20);
+ }
+
+ if (tries < 0) {
+ dev_err(&data->client->dev,
+ "adjd_s311_req_data() failed, data not ready\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int adjd_s311_read_data(struct iio_dev *indio_dev, u8 reg, int *val)
+{
+ struct adjd_s311_data *data = iio_priv(indio_dev);
+
+ int ret = adjd_s311_req_data(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_read_word_data(data->client, reg);
+ if (ret < 0)
+ return ret;
+
+ *val = ret & ADJD_S311_DATA_MASK;
+
+ return 0;
+}
+
+static ssize_t adjd_s311_read_int_time(struct iio_dev *indio_dev,
+ uintptr_t private, const struct iio_chan_spec *chan, char *buf)
+{
+ struct adjd_s311_data *data = iio_priv(indio_dev);
+ s32 ret;
+
+ ret = i2c_smbus_read_word_data(data->client,
+ ADJD_S311_INT_REG(chan->address));
+ if (ret < 0)
+ return ret;
+
+ return sprintf(buf, "%d\n", ret & ADJD_S311_INT_MASK);
+}
+
+static ssize_t adjd_s311_write_int_time(struct iio_dev *indio_dev,
+ uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
+ size_t len)
+{
+ struct adjd_s311_data *data = iio_priv(indio_dev);
+ unsigned long int_time;
+ int ret;
+
+ ret = kstrtoul(buf, 10, &int_time);
+ if (ret)
+ return ret;
+
+ if (int_time > ADJD_S311_INT_MASK)
+ return -EINVAL;
+
+ ret = i2c_smbus_write_word_data(data->client,
+ ADJD_S311_INT_REG(chan->address), int_time);
+ if (ret < 0)
+ return ret;
+
+ return ret ? ret : len;
+}
+
+static irqreturn_t adjd_s311_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct adjd_s311_data *data = iio_priv(indio_dev);
+ struct iio_buffer *buffer = indio_dev->buffer;
+ int len = 0;
+ int i, j = 0;
+
+ int ret = adjd_s311_req_data(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ for_each_set_bit(i, indio_dev->active_scan_mask,
+ indio_dev->masklength) {
+
+ ret = i2c_smbus_read_word_data(data->client,
+ ADJD_S311_DATA_REG(i));
+ if (ret < 0)
+ return ret;
+
+ data->buffer[j++] = ret & ADJD_S311_DATA_MASK;
+ len += 2;
+ }
+
+ if (indio_dev->scan_timestamp)
+ *(s64 *)((phys_addr_t)data->buffer + ALIGN(len, sizeof(s64)))
+ = pf->timestamp;
+ iio_push_to_buffer(buffer, (u8 *)data->buffer, pf->timestamp);
+
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static const struct iio_chan_spec_ext_info adjd_s311_ext_info[] = {
+ {
+ .name = "integration_time",
+ .read = adjd_s311_read_int_time,
+ .write = adjd_s311_write_int_time,
+ },
+ { }
+};
+
+static const struct iio_chan_spec adjd_s311_channels[] = {
+ {
+ .type = IIO_INTENSITY,
+ .modified = 1,
+ .address = IDX_RED,
+ .info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |
+ IIO_CHAN_INFO_HARDWAREGAIN_SEPARATE_BIT,
+ .channel2 = IIO_MOD_LIGHT_RED,
+ .scan_index = 0,
+ .scan_type = IIO_ST('u', 10, 16, 0),
+ .ext_info = adjd_s311_ext_info,
+ }, {
+ .type = IIO_INTENSITY,
+ .modified = 1,
+ .address = IDX_GREEN,
+ .info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |
+ IIO_CHAN_INFO_HARDWAREGAIN_SEPARATE_BIT,
+ .channel2 = IIO_MOD_LIGHT_GREEN,
+ .scan_index = 1,
+ .scan_type = IIO_ST('u', 10, 16, 0),
+ .ext_info = adjd_s311_ext_info,
+ }, {
+ .type = IIO_INTENSITY,
+ .modified = 1,
+ .address = IDX_GREEN,
+ .info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |
+ IIO_CHAN_INFO_HARDWAREGAIN_SEPARATE_BIT,
+ .channel2 = IIO_MOD_LIGHT_BLUE,
+ .scan_index = 2,
+ .scan_type = IIO_ST('u', 10, 16, 0),
+ .ext_info = adjd_s311_ext_info,
+ }, {
+ .type = IIO_INTENSITY,
+ .modified = 1,
+ .address = IDX_CLEAR,
+ .info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |
+ IIO_CHAN_INFO_HARDWAREGAIN_SEPARATE_BIT,
+ .channel2 = IIO_MOD_LIGHT_CLEAR,
+ .scan_index = 3,
+ .scan_type = IIO_ST('u', 10, 16, 0),
+ .ext_info = adjd_s311_ext_info,
+ },
+ IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static int adjd_s311_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct adjd_s311_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = adjd_s311_read_data(indio_dev, chan->address, val);
+ if (ret < 0)
+ return ret;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_HARDWAREGAIN:
+ ret = i2c_smbus_read_byte_data(data->client,
+ ADJD_S311_CAP_REG(chan->address));
+ if (ret < 0)
+ return ret;
+ *val = ret & ADJD_S311_CAP_MASK;
+ return IIO_VAL_INT;
+ }
+ return -EINVAL;
+}
+
+static int adjd_s311_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct adjd_s311_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_HARDWAREGAIN:
+ if (val < 0 || val > ADJD_S311_CAP_MASK)
+ return -EINVAL;
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ ADJD_S311_CAP_REG(chan->address), val);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static int adjd_s311_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ struct adjd_s311_data *data = iio_priv(indio_dev);
+ kfree(data->buffer);
+ data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
+ if (!data->buffer)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static const struct iio_info adjd_s311_info = {
+ .read_raw = adjd_s311_read_raw,
+ .write_raw = adjd_s311_write_raw,
+ .update_scan_mode = adjd_s311_update_scan_mode,
+ .driver_module = THIS_MODULE,
+};
+
+static int __devinit adjd_s311_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct adjd_s311_data *data;
+ struct iio_dev *indio_dev;
+ int err;
+
+ indio_dev = iio_device_alloc(sizeof(*data));
+ if (indio_dev == NULL) {
+ err = -ENOMEM;
+ goto exit;
+ }
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->info = &adjd_s311_info;
+ indio_dev->name = ADJD_S311_DRV_NAME;
+ indio_dev->channels = adjd_s311_channels;
+ indio_dev->num_channels = ARRAY_SIZE(adjd_s311_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ err = iio_triggered_buffer_setup(indio_dev, iio_pollfunc_store_time,
+ adjd_s311_trigger_handler, NULL);
+ if (err < 0)
+ goto exit_free_device;
+
+ err = iio_device_register(indio_dev);
+ if (err)
+ goto exit_unreg_buffer;
+
+ dev_info(&client->dev, "ADJD-S311 color sensor registered\n");
+
+ return 0;
+
+exit_unreg_buffer:
+ iio_triggered_buffer_cleanup(indio_dev);
+exit_free_device:
+ iio_device_free(indio_dev);
+exit:
+ return err;
+}
+
+static int __devexit adjd_s311_remove(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct adjd_s311_data *data = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+ iio_triggered_buffer_cleanup(indio_dev);
+ kfree(data->buffer);
+ iio_device_free(indio_dev);
+
+ return 0;
+}
+
+static const struct i2c_device_id adjd_s311_id[] = {
+ { "adjd_s311", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, adjd_s311_id);
+
+static struct i2c_driver adjd_s311_driver = {
+ .driver = {
+ .name = ADJD_S311_DRV_NAME,
+ },
+ .probe = adjd_s311_probe,
+ .remove = __devexit_p(adjd_s311_remove),
+ .id_table = adjd_s311_id,
+};
+module_i2c_driver(adjd_s311_driver);
+
+MODULE_AUTHOR("Peter Meerwald <pmeerw@pmeerw.net>");
+MODULE_DESCRIPTION("ADJD-S311 color sensor");
+MODULE_LICENSE("GPL");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] iio: kernel version typo in sysfs-bus-iio
2012-07-01 10:20 [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver Peter Meerwald
@ 2012-07-01 10:20 ` Peter Meerwald
2012-07-03 20:02 ` Jonathan Cameron
2012-07-01 10:20 ` [PATCH] iio: add channel modifiers for RGBC (red/green/blue/clear) data Peter Meerwald
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Peter Meerwald @ 2012-07-01 10:20 UTC (permalink / raw)
To: linux-iio; +Cc: Peter Meerwald
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
Documentation/ABI/testing/sysfs-bus-iio | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index a3774a7..2f06d40 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -275,7 +275,7 @@ What: /sys/.../iio:deviceX/in_voltage-voltage_scale_available
What: /sys/.../iio:deviceX/out_voltageX_scale_available
What: /sys/.../iio:deviceX/out_altvoltageX_scale_available
What: /sys/.../iio:deviceX/in_capacitance_scale_available
-KernelVersion: 2.635
+KernelVersion: 2.6.35
Contact: linux-iio@vger.kernel.org
Description:
If a discrete set of scale values is available, they
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] iio: add channel modifiers for RGBC (red/green/blue/clear) data
2012-07-01 10:20 [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver Peter Meerwald
2012-07-01 10:20 ` [PATCH] iio: kernel version typo in sysfs-bus-iio Peter Meerwald
@ 2012-07-01 10:20 ` Peter Meerwald
2012-07-03 19:52 ` Jonathan Cameron
2012-07-01 10:20 ` [PATCH] iio staging: add recently added modifiers to iio_event_monitor Peter Meerwald
2012-07-02 9:28 ` [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver Lars-Peter Clausen
3 siblings, 1 reply; 10+ messages in thread
From: Peter Meerwald @ 2012-07-01 10:20 UTC (permalink / raw)
To: linux-iio; +Cc: Peter Meerwald, Jon Brenner
this patch steals the RGBC-related stuff from Jon Brenner's proposal
(http://permalink.gmane.org/gmane.linux.kernel.iio/4354), CCT parts
are left out for now
the adjd_s311 driver is making use of RGBC modifiers
shouldn't the documentation (sysfs-bus-iio-light) be migrated
from staging?
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Jon Brenner <jbrenner@taosinc.com>
---
drivers/iio/industrialio-core.c | 4 ++++
.../staging/iio/Documentation/sysfs-bus-iio-light | 13 +++++++++++++
include/linux/iio/types.h | 4 ++++
3 files changed, 21 insertions(+)
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index bb3c692..0e2f997 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -74,6 +74,10 @@ static const char * const iio_modifier_names[] = {
[IIO_MOD_SUM_SQUARED_X_Y_Z] = "x^2+y^2+z^2",
[IIO_MOD_LIGHT_BOTH] = "both",
[IIO_MOD_LIGHT_IR] = "ir",
+ [IIO_MOD_LIGHT_CLEAR] = "clear",
+ [IIO_MOD_LIGHT_RED] = "red",
+ [IIO_MOD_LIGHT_GREEN] = "green",
+ [IIO_MOD_LIGHT_BLUE] = "blue",
};
/* relies on pairs of these shared then separate */
diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio-light b/drivers/staging/iio/Documentation/sysfs-bus-iio-light
index d52be03..1f4f6bf 100644
--- a/drivers/staging/iio/Documentation/sysfs-bus-iio-light
+++ b/drivers/staging/iio/Documentation/sysfs-bus-iio-light
@@ -82,3 +82,16 @@ Contact: linux-iio@vger.kernel.org
Description:
This property gets/sets the table of coefficients
used in calculating illuminance in lux.
+
+What: /sys/bus/iio/devices/device[n]/in_intensity_clear[_input|_raw]
+What: /sys/bus/iio/devices/device[n]/in_intensity_red[_input|_raw]
+What: /sys/bus/iio/devices/device[n]/in_intensity_green[_input|_raw]
+What: /sys/bus/iio/devices/device[n]/in_intensity_blue[_input|_raw]
+KernelVersion: 3.6
+Contact: linux-iio@vger.kernel.org
+Description:
+ This property is supported by sensors that have a RGBC
+ sensing mode. This value should be the output from a reading
+ and if expressed in SI units, should include _input. If this
+ value is not in SI units (irradiance, uW/mm^2), then it should
+ include _raw.
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index e250401..63d85a8 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -46,6 +46,10 @@ enum iio_modifier {
IIO_MOD_LIGHT_IR,
IIO_MOD_ROOT_SUM_SQUARED_X_Y,
IIO_MOD_SUM_SQUARED_X_Y_Z,
+ IIO_MOD_LIGHT_CLEAR,
+ IIO_MOD_LIGHT_RED,
+ IIO_MOD_LIGHT_GREEN,
+ IIO_MOD_LIGHT_BLUE,
};
#define IIO_VAL_INT 1
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] iio staging: add recently added modifiers to iio_event_monitor
2012-07-01 10:20 [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver Peter Meerwald
2012-07-01 10:20 ` [PATCH] iio: kernel version typo in sysfs-bus-iio Peter Meerwald
2012-07-01 10:20 ` [PATCH] iio: add channel modifiers for RGBC (red/green/blue/clear) data Peter Meerwald
@ 2012-07-01 10:20 ` Peter Meerwald
2012-07-03 20:00 ` Jonathan Cameron
2012-07-02 9:28 ` [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver Lars-Peter Clausen
3 siblings, 1 reply; 10+ messages in thread
From: Peter Meerwald @ 2012-07-01 10:20 UTC (permalink / raw)
To: linux-iio; +Cc: Peter Meerwald
maybe iio_modifier_names and iio_chan_type_name_spec should be
exported from industrialio-core instead?
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
.../staging/iio/Documentation/iio_event_monitor.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/staging/iio/Documentation/iio_event_monitor.c b/drivers/staging/iio/Documentation/iio_event_monitor.c
index 4326e9e..3a9b000 100644
--- a/drivers/staging/iio/Documentation/iio_event_monitor.c
+++ b/drivers/staging/iio/Documentation/iio_event_monitor.c
@@ -68,6 +68,12 @@ static const char * const iio_modifier_names[] = {
[IIO_MOD_Z] = "z",
[IIO_MOD_LIGHT_BOTH] = "both",
[IIO_MOD_LIGHT_IR] = "ir",
+ [IIO_MOD_ROOT_SUM_SQUARED_X_Y] = "sqrt(x^2+y^2)",
+ [IIO_MOD_SUM_SQUARED_X_Y_Z] = "x^2+y^2+z^2",
+ [IIO_MOD_LIGHT_CLEAR] = "clear",
+ [IIO_MOD_LIGHT_RED] = "red",
+ [IIO_MOD_LIGHT_GREEN] = "green",
+ [IIO_MOD_LIGHT_BLUE] = "blue",
};
static bool event_is_known(struct iio_event_data *event)
@@ -106,6 +112,12 @@ static bool event_is_known(struct iio_event_data *event)
case IIO_MOD_Z:
case IIO_MOD_LIGHT_BOTH:
case IIO_MOD_LIGHT_IR:
+ case IIO_MOD_ROOT_SUM_SQUARED_X_Y:
+ case IIO_MOD_SUM_SQUARED_X_Y_Z:
+ case IIO_MOD_LIGHT_CLEAR:
+ case IIO_MOD_LIGHT_RED:
+ case IIO_MOD_LIGHT_GREEN:
+ case IIO_MOD_LIGHT_BLUE:
break;
default:
return false;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver
2012-07-01 10:20 [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver Peter Meerwald
` (2 preceding siblings ...)
2012-07-01 10:20 ` [PATCH] iio staging: add recently added modifiers to iio_event_monitor Peter Meerwald
@ 2012-07-02 9:28 ` Lars-Peter Clausen
2012-07-02 9:41 ` Lars-Peter Clausen
3 siblings, 1 reply; 10+ messages in thread
From: Lars-Peter Clausen @ 2012-07-02 9:28 UTC (permalink / raw)
To: Peter Meerwald; +Cc: linux-iio
On 07/01/2012 12:20 PM, Peter Meerwald wrote:
> sensor has 4 channels (10-bit each, R/G/B and clear), sensitivity
> and gain is controlled in the driver by ext_info integration_time
> and CHAN_INFO_HARDWAREGAIN
>
> driver supports triggered buffer and IIO_CHAN_INFO_RAW to get the
> sensor data
>
> patch depends on IIO_MOD_LIGHT_RED etc, the patch providing that
> (http://permalink.gmane.org/gmane.linux.kernel.iio/4354) has not been
> merged yet, a new proposal is following
>
> v3:
> * fix warnings
>
> v2: address comments by Lars-Peter Clausen
> * buffer allocation now in update_scan_mode instead of in trigger
> handler
> * simplify trigger code (assume active_scan_mask is not empty, use
> for_each_set_bit, use iio_push_to_buffer)
> * reorder entry in Makefile and Kconfig
> * fix remove
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Looks pretty good to me. One issue in the IRQ handler and two minor
suggestions inline.
> ---
> drivers/iio/light/Kconfig | 12 ++
> drivers/iio/light/Makefile | 1 +
> drivers/iio/light/adjd_s311.c | 395 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 408 insertions(+)
> create mode 100644 drivers/iio/light/adjd_s311.c
> [...]
> +
> +static ssize_t adjd_s311_write_int_time(struct iio_dev *indio_dev,
> + uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
> + size_t len)
> +{
> + struct adjd_s311_data *data = iio_priv(indio_dev);
> + unsigned long int_time;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, &int_time);
> + if (ret)
> + return ret;
> +
> + if (int_time > ADJD_S311_INT_MASK)
> + return -EINVAL;
> +
> + ret = i2c_smbus_write_word_data(data->client,
> + ADJD_S311_INT_REG(chan->address), int_time);
> + if (ret < 0)
> + return ret;
> +
> + return ret ? ret : len;
ret will always be zero here.
> +}
> +
> +static irqreturn_t adjd_s311_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct adjd_s311_data *data = iio_priv(indio_dev);
> + struct iio_buffer *buffer = indio_dev->buffer;
> + int len = 0;
> + int i, j = 0;
> +
> + int ret = adjd_s311_req_data(indio_dev);
> + if (ret < 0)
> + return ret;
I wouldn't return ret here since this is the interrupt handler and I'm not
quite sure how the core reacts if it gets a value which is not a
irqreturn_t, especially considering that irqreturn_t is basically unsigned.
You also have to call iio_trigger_notify_done before leaving the function
> +
> + for_each_set_bit(i, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> +
> + ret = i2c_smbus_read_word_data(data->client,
> + ADJD_S311_DATA_REG(i));
> + if (ret < 0)
> + return ret;
Same here.
> +
> + data->buffer[j++] = ret & ADJD_S311_DATA_MASK;
> + len += 2;
> + }
> +
> + if (indio_dev->scan_timestamp)
> + *(s64 *)((phys_addr_t)data->buffer + ALIGN(len, sizeof(s64)))
> + = pf->timestamp;
> + iio_push_to_buffer(buffer, (u8 *)data->buffer, pf->timestamp);
> +
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> [...]
> +
> +static int adjd_s311_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct adjd_s311_data *data = iio_priv(indio_dev);
> + kfree(data->buffer);
> + data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
The above two lines could be done in one with krealloc(...)
> + if (!data->buffer)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> [...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver
2012-07-02 9:28 ` [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver Lars-Peter Clausen
@ 2012-07-02 9:41 ` Lars-Peter Clausen
2012-07-02 9:55 ` Peter Meerwald
0 siblings, 1 reply; 10+ messages in thread
From: Lars-Peter Clausen @ 2012-07-02 9:41 UTC (permalink / raw)
To: Peter Meerwald; +Cc: linux-iio
On 07/02/2012 11:28 AM, Lars-Peter Clausen wrote:
> On 07/01/2012 12:20 PM, Peter Meerwald wrote:
>> sensor has 4 channels (10-bit each, R/G/B and clear), sensitivity
>> and gain is controlled in the driver by ext_info integration_time
>> and CHAN_INFO_HARDWAREGAIN
>>
>> driver supports triggered buffer and IIO_CHAN_INFO_RAW to get the
>> sensor data
>>
>> patch depends on IIO_MOD_LIGHT_RED etc, the patch providing that
>> (http://permalink.gmane.org/gmane.linux.kernel.iio/4354) has not been
>> merged yet, a new proposal is following
>>
>> v3:
>> * fix warnings
>>
>> v2: address comments by Lars-Peter Clausen
>> * buffer allocation now in update_scan_mode instead of in trigger
>> handler
>> * simplify trigger code (assume active_scan_mask is not empty, use
>> for_each_set_bit, use iio_push_to_buffer)
>> * reorder entry in Makefile and Kconfig
>> * fix remove
>>
>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
>
>
> Looks pretty good to me. One issue in the IRQ handler and two minor
> suggestions inline.
>
>> ---
>> drivers/iio/light/Kconfig | 12 ++
>> drivers/iio/light/Makefile | 1 +
>> drivers/iio/light/adjd_s311.c | 395 +++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 408 insertions(+)
>> create mode 100644 drivers/iio/light/adjd_s311.c
>> [...]
>> +
>> +static ssize_t adjd_s311_write_int_time(struct iio_dev *indio_dev,
>> + uintptr_t private, const struct iio_chan_spec *chan, const char *buf,
>> + size_t len)
>> +{
>> + struct adjd_s311_data *data = iio_priv(indio_dev);
>> + unsigned long int_time;
>> + int ret;
>> +
>> + ret = kstrtoul(buf, 10, &int_time);
>> + if (ret)
>> + return ret;
>> +
>> + if (int_time > ADJD_S311_INT_MASK)
>> + return -EINVAL;
>> +
>> + ret = i2c_smbus_write_word_data(data->client,
>> + ADJD_S311_INT_REG(chan->address), int_time);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return ret ? ret : len;
>
> ret will always be zero here.
>
>> +}
>> +
>> +static irqreturn_t adjd_s311_trigger_handler(int irq, void *p)
>> +{
>> + struct iio_poll_func *pf = p;
>> + struct iio_dev *indio_dev = pf->indio_dev;
>> + struct adjd_s311_data *data = iio_priv(indio_dev);
>> + struct iio_buffer *buffer = indio_dev->buffer;
>> + int len = 0;
>> + int i, j = 0;
>> +
>> + int ret = adjd_s311_req_data(indio_dev);
>> + if (ret < 0)
>> + return ret;
>
> I wouldn't return ret here since this is the interrupt handler and I'm not
> quite sure how the core reacts if it gets a value which is not a
> irqreturn_t, especially considering that irqreturn_t is basically unsigned.
> You also have to call iio_trigger_notify_done before leaving the function
>
Looks like you are not alone with this, I've just wrote a cocci patch which
finds paths which return form the trigger handler without calling
iio_trigger_notify_done and it triggers on quite a few other drivers as well.
@r1@
identifier fn;
identifier indio_dev;
expression bh;
@@
(
iio_alloc_pollfunc(bh, &fn, ...)
|
iio_alloc_pollfunc(bh, fn, ...)
|
iio_triggered_buffer_setup(indio_dev, bh, fn, ...)
)
@@
typedef irqreturn_t;
identifier r1.fn;
@@
irqreturn_t fn(...)
{
... when != iio_trigger_notify_done(...)
when any
* return ...;
... when any
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver
2012-07-02 9:41 ` Lars-Peter Clausen
@ 2012-07-02 9:55 ` Peter Meerwald
0 siblings, 0 replies; 10+ messages in thread
From: Peter Meerwald @ 2012-07-02 9:55 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: linux-iio
Hello,
thank you for reviewing!
> >> + ret = i2c_smbus_write_word_data(data->client,
> >> + ADJD_S311_INT_REG(chan->address), int_time);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + return ret ? ret : len;
> >
> > ret will always be zero here.
right, I think that's a leftover from code calling
i2c_smbus_read_block_data()
> > I wouldn't return ret here since this is the interrupt handler and I'm not
> > quite sure how the core reacts if it gets a value which is not a
> > irqreturn_t, especially considering that irqreturn_t is basically unsigned.
> > You also have to call iio_trigger_notify_done before leaving the function
> >
>
> Looks like you are not alone with this, I've just wrote a cocci patch which
> finds paths which return form the trigger handler without calling
> iio_trigger_notify_done and it triggers on quite a few other drivers as well.
I know, but no wonder; there is zero documentation and even the sample in
iio_simple_dummy_buffer.c looks odd:
static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
{
struct iio_poll_func *pf = p;
...
data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
if (data == NULL)
return -ENOMEM;
I am abusing you as my patch correctness oracle to learn things, sorry :)
regards, p.
--
Peter Meerwald
+43-664-2444418 (mobile)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] iio: add channel modifiers for RGBC (red/green/blue/clear) data
2012-07-01 10:20 ` [PATCH] iio: add channel modifiers for RGBC (red/green/blue/clear) data Peter Meerwald
@ 2012-07-03 19:52 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2012-07-03 19:52 UTC (permalink / raw)
To: Peter Meerwald; +Cc: linux-iio, Jon Brenner
On 07/01/2012 11:20 AM, Peter Meerwald wrote:
> this patch steals the RGBC-related stuff from Jon Brenner's proposal
> (http://permalink.gmane.org/gmane.linux.kernel.iio/4354), CCT parts
> are left out for now
Ideally this would have Jon's ack (or signoff). I'll merge it in the
meantime, but probably won't send to Greg for a day or two if Jon
wants to add anything.
>
> the adjd_s311 driver is making use of RGBC modifiers
>
> shouldn't the documentation (sysfs-bus-iio-light) be migrated
> from staging?
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Jon Brenner <jbrenner@taosinc.com>
> ---
> drivers/iio/industrialio-core.c | 4 ++++
> .../staging/iio/Documentation/sysfs-bus-iio-light | 13 +++++++++++++
> include/linux/iio/types.h | 4 ++++
> 3 files changed, 21 insertions(+)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index bb3c692..0e2f997 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -74,6 +74,10 @@ static const char * const iio_modifier_names[] = {
> [IIO_MOD_SUM_SQUARED_X_Y_Z] = "x^2+y^2+z^2",
> [IIO_MOD_LIGHT_BOTH] = "both",
> [IIO_MOD_LIGHT_IR] = "ir",
> + [IIO_MOD_LIGHT_CLEAR] = "clear",
> + [IIO_MOD_LIGHT_RED] = "red",
> + [IIO_MOD_LIGHT_GREEN] = "green",
> + [IIO_MOD_LIGHT_BLUE] = "blue",
> };
>
> /* relies on pairs of these shared then separate */
> diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio-light b/drivers/staging/iio/Documentation/sysfs-bus-iio-light
> index d52be03..1f4f6bf 100644
> --- a/drivers/staging/iio/Documentation/sysfs-bus-iio-light
> +++ b/drivers/staging/iio/Documentation/sysfs-bus-iio-light
> @@ -82,3 +82,16 @@ Contact: linux-iio@vger.kernel.org
> Description:
> This property gets/sets the table of coefficients
> used in calculating illuminance in lux.
> +
> +What: /sys/bus/iio/devices/device[n]/in_intensity_clear[_input|_raw]
> +What: /sys/bus/iio/devices/device[n]/in_intensity_red[_input|_raw]
> +What: /sys/bus/iio/devices/device[n]/in_intensity_green[_input|_raw]
> +What: /sys/bus/iio/devices/device[n]/in_intensity_blue[_input|_raw]
> +KernelVersion: 3.6
> +Contact: linux-iio@vger.kernel.org
> +Description:
> + This property is supported by sensors that have a RGBC
> + sensing mode. This value should be the output from a reading
> + and if expressed in SI units, should include _input. If this
> + value is not in SI units (irradiance, uW/mm^2), then it should
> + include _raw.
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index e250401..63d85a8 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -46,6 +46,10 @@ enum iio_modifier {
> IIO_MOD_LIGHT_IR,
> IIO_MOD_ROOT_SUM_SQUARED_X_Y,
> IIO_MOD_SUM_SQUARED_X_Y_Z,
> + IIO_MOD_LIGHT_CLEAR,
> + IIO_MOD_LIGHT_RED,
> + IIO_MOD_LIGHT_GREEN,
> + IIO_MOD_LIGHT_BLUE,
> };
>
> #define IIO_VAL_INT 1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] iio staging: add recently added modifiers to iio_event_monitor
2012-07-01 10:20 ` [PATCH] iio staging: add recently added modifiers to iio_event_monitor Peter Meerwald
@ 2012-07-03 20:00 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2012-07-03 20:00 UTC (permalink / raw)
To: Peter Meerwald; +Cc: linux-iio, Lars-Peter Clausen
On 07/01/2012 11:20 AM, Peter Meerwald wrote:
> maybe iio_modifier_names and iio_chan_type_name_spec should be
> exported from industrialio-core instead?
gah, I clean forgot this existed.
Cc Lars-Peter on patches to this, but I have merged as
I can't see anything controversial here (or have I spoken too soon!)
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
merged
> ---
> .../staging/iio/Documentation/iio_event_monitor.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/staging/iio/Documentation/iio_event_monitor.c b/drivers/staging/iio/Documentation/iio_event_monitor.c
> index 4326e9e..3a9b000 100644
> --- a/drivers/staging/iio/Documentation/iio_event_monitor.c
> +++ b/drivers/staging/iio/Documentation/iio_event_monitor.c
> @@ -68,6 +68,12 @@ static const char * const iio_modifier_names[] = {
> [IIO_MOD_Z] = "z",
> [IIO_MOD_LIGHT_BOTH] = "both",
> [IIO_MOD_LIGHT_IR] = "ir",
> + [IIO_MOD_ROOT_SUM_SQUARED_X_Y] = "sqrt(x^2+y^2)",
> + [IIO_MOD_SUM_SQUARED_X_Y_Z] = "x^2+y^2+z^2",
> + [IIO_MOD_LIGHT_CLEAR] = "clear",
> + [IIO_MOD_LIGHT_RED] = "red",
> + [IIO_MOD_LIGHT_GREEN] = "green",
> + [IIO_MOD_LIGHT_BLUE] = "blue",
> };
>
> static bool event_is_known(struct iio_event_data *event)
> @@ -106,6 +112,12 @@ static bool event_is_known(struct iio_event_data *event)
> case IIO_MOD_Z:
> case IIO_MOD_LIGHT_BOTH:
> case IIO_MOD_LIGHT_IR:
> + case IIO_MOD_ROOT_SUM_SQUARED_X_Y:
> + case IIO_MOD_SUM_SQUARED_X_Y_Z:
> + case IIO_MOD_LIGHT_CLEAR:
> + case IIO_MOD_LIGHT_RED:
> + case IIO_MOD_LIGHT_GREEN:
> + case IIO_MOD_LIGHT_BLUE:
> break;
> default:
> return false;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] iio: kernel version typo in sysfs-bus-iio
2012-07-01 10:20 ` [PATCH] iio: kernel version typo in sysfs-bus-iio Peter Meerwald
@ 2012-07-03 20:02 ` Jonathan Cameron
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2012-07-03 20:02 UTC (permalink / raw)
To: Peter Meerwald; +Cc: linux-iio
On 07/01/2012 11:20 AM, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
merged thanks.
> ---
> Documentation/ABI/testing/sysfs-bus-iio | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index a3774a7..2f06d40 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -275,7 +275,7 @@ What: /sys/.../iio:deviceX/in_voltage-voltage_scale_available
> What: /sys/.../iio:deviceX/out_voltageX_scale_available
> What: /sys/.../iio:deviceX/out_altvoltageX_scale_available
> What: /sys/.../iio:deviceX/in_capacitance_scale_available
> -KernelVersion: 2.635
> +KernelVersion: 2.6.35
> Contact: linux-iio@vger.kernel.org
> Description:
> If a discrete set of scale values is available, they
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-07-03 20:02 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-01 10:20 [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver Peter Meerwald
2012-07-01 10:20 ` [PATCH] iio: kernel version typo in sysfs-bus-iio Peter Meerwald
2012-07-03 20:02 ` Jonathan Cameron
2012-07-01 10:20 ` [PATCH] iio: add channel modifiers for RGBC (red/green/blue/clear) data Peter Meerwald
2012-07-03 19:52 ` Jonathan Cameron
2012-07-01 10:20 ` [PATCH] iio staging: add recently added modifiers to iio_event_monitor Peter Meerwald
2012-07-03 20:00 ` Jonathan Cameron
2012-07-02 9:28 ` [PATCH v3] iio: add adjd_s311 I2C digital color sensor driver Lars-Peter Clausen
2012-07-02 9:41 ` Lars-Peter Clausen
2012-07-02 9:55 ` Peter Meerwald
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).