[PATCH 1/3] iio: core: implement devm_iio_device_alloc/devm_iio_device_free

[PATCH 1/3] iio: core: implement devm_iio_device_alloc/devm_iio_device_free

[Oleksandr Kravchenko]

From: Grygorii Strashko <grygorii.strashko@ti.com>

Add a resource managed devm_iio_device_alloc()/devm_iio_device_free()
to automatically clean up any allocations made by IIO drivers,
thus leading to simplified IIO drivers code.

In addition, this will allow IIO drivers to use other devm_*() API
(like devm_request_irq) and don't care about the race between
iio_device_free() and the release of resources by Device core
during driver removing.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
[o.v.kravchenko@globallogic.com: fix return value ib devm_iio_device_alloc
in case if devres_alloc failed, remove unused variable "rc"]
Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
Tested-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
---
 drivers/iio/industrialio-core.c |   47 +++++++++++++++++++++++++++++++++++++++
 include/linux/iio/iio.h         |   25 +++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index e145931..d56d122 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -912,6 +912,53 @@ void iio_device_free(struct iio_dev *dev)
 }
 EXPORT_SYMBOL(iio_device_free);
 
+static void devm_iio_device_release(struct device *dev, void *res)
+{
+	iio_device_free(*(struct iio_dev **)res);
+}
+
+static int devm_iio_device_match(struct device *dev, void *res, void *data)
+{
+	struct iio_dev **r = res;
+	if (!r || !*r) {
+		WARN_ON(!r || !*r);
+		return 0;
+	}
+	return *r == data;
+}
+
+struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
+{
+	struct iio_dev **ptr, *iio_dev;
+
+	ptr = devres_alloc(devm_iio_device_release, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	/* use raw alloc_dr for kmalloc caller tracing */
+	iio_dev = iio_device_alloc(sizeof_priv);
+	if (iio_dev) {
+		*ptr = iio_dev;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return iio_dev;
+}
+EXPORT_SYMBOL_GPL(devm_iio_device_alloc);
+
+void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev)
+{
+	int rc;
+
+	rc = devres_release(dev, devm_iio_device_release,
+			    devm_iio_device_match, iio_dev);
+	WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_iio_device_free);
+
 /**
  * iio_chrdev_open() - chrdev file open for buffer access and ioctls
  **/
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 8d171f4..f1d99f6 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -532,6 +532,31 @@ static inline struct iio_dev *iio_priv_to_dev(void *priv)
 void iio_device_free(struct iio_dev *indio_dev);
 
 /**
+ * devm_iio_device_alloc - Resource-managed iio_device_alloc()
+ * @dev: 		Device to allocate iio_dev for
+ * @sizeof_priv: 	Space to allocate for private structure.
+ *
+ * Managed iio_device_alloc.  iio_dev allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an iio_dev allocated with this function needs to be freed separately,
+ * devm_iio_device_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated iio_dev on success, NULL on failure.
+ */
+struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv);
+
+/**
+ * devm_iio_device_free - Resource-managed iio_device_free()
+ * @dev:		Device this iio_dev belongs to
+ * @indio_dev: 		the iio_dev associated with the device
+ *
+ * Free indio_dev allocated with devm_iio_device_alloc().
+ */
+void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev);
+
+/**
  * iio_buffer_enabled() - helper function to test if the buffer is enabled
  * @indio_dev:		IIO device structure for device
  **/
-- 
1.7.9.5

[PATCH 1/3] iio: core: implement devm_iio_device_alloc/devm_iio_device_free

[Oleksandr Kravchenko]

From: Grygorii Strashko <grygorii.strashko@ti.com>

Add a resource managed devm_iio_device_alloc()/devm_iio_device_free()
to automatically clean up any allocations made by IIO drivers,
thus leading to simplified IIO drivers code.

In addition, this will allow IIO drivers to use other devm_*() API
(like devm_request_irq) and don't care about the race between
iio_device_free() and the release of resources by Device core
during driver removing.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
[o.v.kravchenko@globallogic.com: fix return value ib devm_iio_device_alloc
in case if devres_alloc failed, remove unused variable "rc"]
Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
Tested-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
---
 drivers/iio/industrialio-core.c |   47 +++++++++++++++++++++++++++++++++++++++
 include/linux/iio/iio.h         |   25 +++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index e145931..d56d122 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -912,6 +912,53 @@ void iio_device_free(struct iio_dev *dev)
 }
 EXPORT_SYMBOL(iio_device_free);
 
+static void devm_iio_device_release(struct device *dev, void *res)
+{
+	iio_device_free(*(struct iio_dev **)res);
+}
+
+static int devm_iio_device_match(struct device *dev, void *res, void *data)
+{
+	struct iio_dev **r = res;
+	if (!r || !*r) {
+		WARN_ON(!r || !*r);
+		return 0;
+	}
+	return *r == data;
+}
+
+struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
+{
+	struct iio_dev **ptr, *iio_dev;
+
+	ptr = devres_alloc(devm_iio_device_release, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	/* use raw alloc_dr for kmalloc caller tracing */
+	iio_dev = iio_device_alloc(sizeof_priv);
+	if (iio_dev) {
+		*ptr = iio_dev;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return iio_dev;
+}
+EXPORT_SYMBOL_GPL(devm_iio_device_alloc);
+
+void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev)
+{
+	int rc;
+
+	rc = devres_release(dev, devm_iio_device_release,
+			    devm_iio_device_match, iio_dev);
+	WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_iio_device_free);
+
 /**
  * iio_chrdev_open() - chrdev file open for buffer access and ioctls
  **/
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 8d171f4..f1d99f6 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -532,6 +532,31 @@ static inline struct iio_dev *iio_priv_to_dev(void *priv)
 void iio_device_free(struct iio_dev *indio_dev);
 
 /**
+ * devm_iio_device_alloc - Resource-managed iio_device_alloc()
+ * @dev: 		Device to allocate iio_dev for
+ * @sizeof_priv: 	Space to allocate for private structure.
+ *
+ * Managed iio_device_alloc.  iio_dev allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an iio_dev allocated with this function needs to be freed separately,
+ * devm_iio_device_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated iio_dev on success, NULL on failure.
+ */
+struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv);
+
+/**
+ * devm_iio_device_free - Resource-managed iio_device_free()
+ * @dev:		Device this iio_dev belongs to
+ * @indio_dev: 		the iio_dev associated with the device
+ *
+ * Free indio_dev allocated with devm_iio_device_alloc().
+ */
+void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev);
+
+/**
  * iio_buffer_enabled() - helper function to test if the buffer is enabled
  * @indio_dev:		IIO device structure for device
  **/
-- 
1.7.9.5

[PATCH 2/3] of: Add Avago Technologies vendor prefix

[Oleksandr Kravchenko]

From: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>

This commit adds a device tree vendor prefix for Avago Technologies.

Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
---
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d5a79ca..8aab9c6 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -11,6 +11,7 @@ amcc	Applied Micro Circuits Corporation (APM, formally AMCC)
 apm	Applied Micro Circuits Corporation (APM)
 arm	ARM Ltd.
 atmel	Atmel Corporation
+avago	Avago Technologies
 bosch	Bosch Sensortec GmbH
 brcm	Broadcom Corporation
 cavium	Cavium, Inc.
-- 
1.7.9.5

[PATCH 2/3] of: Add Avago Technologies vendor prefix

[Oleksandr Kravchenko]

From: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>

This commit adds a device tree vendor prefix for Avago Technologies.

Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
---
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d5a79ca..8aab9c6 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -11,6 +11,7 @@ amcc	Applied Micro Circuits Corporation (APM, formally AMCC)
 apm	Applied Micro Circuits Corporation (APM)
 arm	ARM Ltd.
 atmel	Atmel Corporation
+avago	Avago Technologies
 bosch	Bosch Sensortec GmbH
 brcm	Broadcom Corporation
 cavium	Cavium, Inc.
-- 
1.7.9.5

[PATCH 3/3] iio: add APDS9300 ambilent light sensor driver

[Oleksandr Kravchenko]

From: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>

This patch adds IIO driver for APDS9300 ambient light sensor (ALS).
http://www.avagotech.com/docs/AV02-1077EN

The driver allows to read raw data from ADC registers or calculate
lux value. It also can handle threshold interrupt.

Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
---
 .../devicetree/bindings/iio/light/apds9300.txt     |   22 +
 drivers/iio/light/Kconfig                          |   10 +
 drivers/iio/light/Makefile                         |    1 +
 drivers/iio/light/apds9300.c                       |  517 ++++++++++++++++++++
 4 files changed, 550 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/apds9300.txt
 create mode 100644 drivers/iio/light/apds9300.c

diff --git a/Documentation/devicetree/bindings/iio/light/apds9300.txt b/Documentation/devicetree/bindings/iio/light/apds9300.txt
new file mode 100644
index 0000000..d6f66c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/apds9300.txt
@@ -0,0 +1,22 @@
+* Avago APDS9300 ambient light sensor
+
+http://www.avagotech.com/docs/AV02-1077EN
+
+Required properties:
+
+  - compatible : should be "avago,apds9300"
+  - reg : the I2C address of the sensor
+
+Optional properties:
+
+  - interrupt-parent : should be the phandle for the interrupt controller
+  - interrupts : interrupt mapping for GPIO IRQ
+
+Example:
+
+apds9300@39 {
+	compatible = "avago,apds9300";
+	reg = <0x39>;
+	interrupt-parent = <&gpio2>;
+	interrupts = <29 8>;
+};
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 5ef1a39..fb6af1b 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -15,6 +15,16 @@ config ADJD_S311
 	 This driver can also be built as a module.  If so, the module
 	 will be called adjd_s311.
 
+config APDS9300
+	tristate "APDS9300 ambient light sensor"
+	depends on I2C
+	help
+	 Say Y here if you want to build a driver for the Avago APDS9300
+	 ambient light sensor.
+
+	 To compile this driver as a module, choose M here: the
+	 module will be called apds9300.
+
 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 040d9c7..da58e12 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -6,4 +6,5 @@ obj-$(CONFIG_ADJD_S311)		+= adjd_s311.o
 obj-$(CONFIG_SENSORS_LM3533)	+= lm3533-als.o
 obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
 obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
+obj-$(CONFIG_APDS9300)		+= apds9300.o
 obj-$(CONFIG_HID_SENSOR_ALS)	+= hid-sensor-als.o
diff --git a/drivers/iio/light/apds9300.c b/drivers/iio/light/apds9300.c
new file mode 100644
index 0000000..c06a59e
--- /dev/null
+++ b/drivers/iio/light/apds9300.c
@@ -0,0 +1,517 @@
+/*
+ * apds9300.c - IIO driver for Avago APDS9300 ambient light sensor
+ *
+ * Copyright 2013 Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/pm.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
+
+#define ALS_DRV_NAME "apds9300"
+#define ALS_IRQ_NAME "apds9300_event"
+
+/* Command register bits */
+#define ALS_CMD		BIT(7) /* Select command register. Must write as 1 */
+#define ALS_WORD	BIT(5) /* I2C write/read: if 1 word, if 0 byte */
+#define ALS_CLEAR	BIT(6) /* Interrupt clear. Clears pending interrupt */
+
+/* Register set */
+#define ALS_CONTROL		0x00 /* Control of basic functions */
+#define ALS_THRESHLOWLOW	0x02 /* Low byte of low interrupt threshold */
+#define ALS_THRESHHIGHLOW	0x04 /* Low byte of high interrupt threshold */
+#define ALS_INTERRUPT		0x06 /* Interrupt control */
+#define ALS_DATA0LOW		0x0c /* Low byte of ADC channel 0 */
+#define ALS_DATA1LOW		0x0e /* Low byte of ADC channel 1 */
+
+/* Power on/off value for ALS_CONTROL register */
+#define ALS_POWER_ON		0x03
+#define ALS_POWER_OFF		0x00
+
+/* Interrupts */
+#define ALS_INTR_ENABLE		0x10
+/* Interrupt Persist Function: Any value outside of threshold range */
+#define ALS_THRESH_INTR		0x01
+
+#define ALS_THRESH_MAX		0xffff /* Max threshold value */
+
+struct als_data {
+	struct i2c_client *client;
+	struct mutex mutex;
+	int power_state;
+	int thresh_low;
+	int thresh_hi;
+	int intr_en;
+};
+
+/* Lux calculation */
+
+/* Calculated values 1000 * (CH1/CH0)^1.4 for CH1/CH0 from 0 to 0.52 */
+static const u16 lux_ratio[] = {
+	0, 2, 4, 7, 11, 15, 19, 24, 29, 34, 40, 45, 51, 57, 64, 70, 77, 84, 91,
+	98, 105, 112, 120, 128, 136, 144, 152, 160, 168, 177, 185, 194, 203,
+	212, 221, 230, 239, 249, 258, 268, 277, 287, 297, 307, 317, 327, 337,
+	347, 358, 368, 379, 390, 400,
+};
+
+static unsigned long als_calculate_lux(u16 ch0, u16 ch1)
+{
+	unsigned long lux, tmp;
+
+	/* avoid division by zero */
+	if (ch0 == 0)
+		return 0;
+
+	tmp = DIV_ROUND_UP(ch1 * 100, ch0);
+	if (tmp <= 52) {
+		/*
+		 * Variable tmp64 need to avoid overflow of this part of lux
+		 * calculation formula.
+		 */
+		lux = 3150 * ch0 - (unsigned long)DIV_ROUND_UP_ULL(ch0
+				* lux_ratio[tmp] * 5930ull, 1000);
+	} else if (tmp <= 65) {
+		lux = 2290 * ch0 - 2910 * ch1;
+	} else if (tmp <= 80) {
+		lux = 1570 * ch0 - 1800 * ch1;
+	} else if (tmp <= 130) {
+		lux = 338 * ch0 - 260 * ch1;
+	} else {
+		lux = 0;
+	}
+
+	return lux / 100000;
+}
+
+/* I2C I/O operations */
+
+static int als_set_power_state(struct als_data *data, int state)
+{
+	int ret;
+	u8 cmd;
+
+	cmd = state ? ALS_POWER_ON : ALS_POWER_OFF;
+	ret = i2c_smbus_write_byte_data(data->client,
+			ALS_CONTROL | ALS_CMD, cmd);
+	if (!ret)
+		data->power_state = state;
+	else
+		dev_err(&data->client->dev,
+			"failed to set power state %d\n", state);
+
+	return ret;
+}
+
+static int als_get_adc_val(struct als_data *data, int adc_number)
+{
+	int ret;
+	u8 flags = ALS_CMD | ALS_WORD;
+
+	if (!data->power_state)
+		return -EBUSY;
+
+	/* Select ADC0 or ADC1 data register */
+	flags |= adc_number ? ALS_DATA1LOW : ALS_DATA0LOW;
+
+	ret = i2c_smbus_read_word_data(data->client, flags);
+	if (ret < 0)
+		dev_err(&data->client->dev,
+			"failed to read ADC%d value\n", adc_number);
+
+	return ret;
+}
+
+static int als_set_thresh_low(struct als_data *data, int value)
+{
+	int ret;
+
+	if (!data->power_state)
+		return -EBUSY;
+
+	if (value > ALS_THRESH_MAX || value > data->thresh_hi)
+		return -EINVAL;
+
+	ret = i2c_smbus_write_word_data(data->client,
+			ALS_THRESHLOWLOW | ALS_CMD | ALS_WORD, value);
+	if (!ret)
+		data->thresh_low = value;
+	else
+		dev_err(&data->client->dev,
+			"failed to set thresh_low\n");
+
+	return ret;
+}
+
+static int als_set_thresh_hi(struct als_data *data, int value)
+{
+	int ret;
+
+	if (!data->power_state)
+		return -EBUSY;
+
+	if (value > ALS_THRESH_MAX || value < data->thresh_low)
+		return -EINVAL;
+
+	ret = i2c_smbus_write_word_data(data->client,
+			ALS_THRESHHIGHLOW | ALS_CMD | ALS_WORD, value);
+	if (!ret)
+		data->thresh_hi = value;
+	else
+		dev_err(&data->client->dev,
+			"failed to set thresh_hi\n");
+
+	return ret;
+}
+
+static int als_set_intr_state(struct als_data *data, int state)
+{
+	int ret;
+	u8 cmd;
+
+	if (!data->power_state)
+		return -EBUSY;
+
+	cmd = state ? ALS_INTR_ENABLE | ALS_THRESH_INTR : 0x00;
+	ret = i2c_smbus_write_byte_data(data->client,
+			ALS_INTERRUPT | ALS_CMD, cmd);
+	if (!ret)
+		data->intr_en = state;
+	else
+		dev_err(&data->client->dev,
+			"failed to set interrupt state %d\n", state);
+
+	return ret;
+}
+
+static void als_clear_intr(struct als_data *data)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte(data->client, ALS_CLEAR | ALS_CMD);
+	if (ret < 0)
+		dev_err(&data->client->dev,
+			"failed to clear interrupt\n");
+}
+
+static int als_chip_init(struct als_data *data)
+{
+	int ret;
+
+	/* Need to set power off to ensure that the chip is off */
+	ret = als_set_power_state(data, 0);
+	if (ret < 0)
+		goto err;
+	/*
+	 * Probe the chip. To do so we try to power up the device and then to
+	 * read back the 0x03 code
+	 */
+	ret = als_set_power_state(data, 1);
+	if (ret < 0)
+		goto err;
+	ret = i2c_smbus_read_byte_data(data->client, ALS_CONTROL | ALS_CMD);
+	if (ret != ALS_POWER_ON) {
+		ret = -ENODEV;
+		goto err;
+	}
+	/*
+	 * Disable interrupt to ensure thai it is doesn't enable
+	 * i.e. after device soft reset
+	 */
+	ret = als_set_intr_state(data, 0);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+
+err:
+	dev_err(&data->client->dev, "failed to init the chip\n");
+	return ret;
+}
+
+/* Industrial I/O data and functions */
+
+static int als_read_raw(struct iio_dev *indio_dev,
+		struct iio_chan_spec const *chan, int *val, int *val2,
+		long mask)
+{
+	int ch0, ch1, ret = -EINVAL;
+	struct als_data *data = iio_priv(indio_dev);
+
+	mutex_lock(&data->mutex);
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			ch0 = als_get_adc_val(data, 0);
+			if (ch0 < 0) {
+				ret = ch0;
+				break;
+			}
+			ch1 = als_get_adc_val(data, 1);
+			if (ch1 < 0) {
+				ret = ch1;
+				break;
+			}
+			*val = als_calculate_lux(ch0, ch1);
+			ret = IIO_VAL_INT;
+			break;
+		case IIO_INTENSITY:
+			ret = als_get_adc_val(data, chan->channel);
+			if (ret < 0)
+				break;
+			*val = ret;
+			ret = IIO_VAL_INT;
+			break;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int als_read_thresh(struct iio_dev *indio_dev, u64 event_code, int *val)
+{
+	struct als_data *data = iio_priv(indio_dev);
+
+	switch (IIO_EVENT_CODE_EXTRACT_DIR(event_code)) {
+	case IIO_EV_DIR_RISING:
+		*val = data->thresh_hi;
+		break;
+	case IIO_EV_DIR_FALLING:
+		*val = data->thresh_low;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int als_write_thresh(struct iio_dev *indio_dev, u64 event_code, int val)
+{
+	struct als_data *data = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&data->mutex);
+	if (IIO_EVENT_CODE_EXTRACT_DIR(event_code) == IIO_EV_DIR_RISING)
+		ret = als_set_thresh_hi(data, val);
+	else
+		ret = als_set_thresh_low(data, val);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int als_read_interrupt_config(struct iio_dev *indio_dev,
+		u64 event_code)
+{
+	struct als_data *data = iio_priv(indio_dev);
+	return data->intr_en;
+}
+
+static int als_write_interrupt_config(struct iio_dev *indio_dev,
+		u64 event_code, int state)
+{
+	struct als_data *data = iio_priv(indio_dev);
+	int ret = 0;
+
+	mutex_lock(&data->mutex);
+	ret = als_set_intr_state(data, state);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static const struct iio_info als_info_no_irq = {
+	.driver_module	= THIS_MODULE,
+	.read_raw	= &als_read_raw,
+};
+
+static const struct iio_info als_info = {
+	.driver_module		= THIS_MODULE,
+	.read_raw		= als_read_raw,
+	.read_event_value	= &als_read_thresh,
+	.write_event_value	= &als_write_thresh,
+	.read_event_config	= &als_read_interrupt_config,
+	.write_event_config	= &als_write_interrupt_config,
+};
+
+static const struct iio_chan_spec als_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.channel = 0,
+		.indexed = true,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+	}, {
+		.type = IIO_INTENSITY,
+		.channel = 0,
+		.indexed = true,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.event_mask = (IIO_EV_BIT(IIO_EV_TYPE_THRESH,
+					  IIO_EV_DIR_RISING) |
+			       IIO_EV_BIT(IIO_EV_TYPE_THRESH,
+					  IIO_EV_DIR_FALLING)),
+	}, {
+		.type = IIO_INTENSITY,
+		.channel = 1,
+		.indexed = true,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+	},
+};
+
+static irqreturn_t als_interrupt_handler(int irq, void *private)
+{
+	struct iio_dev *dev_info = private;
+	struct als_data *data = iio_priv(dev_info);
+
+	iio_push_event(dev_info,
+		       IIO_UNMOD_EVENT_CODE(IIO_INTENSITY, 0,
+					    IIO_EV_TYPE_THRESH,
+					    IIO_EV_DIR_EITHER),
+		       iio_get_time_ns());
+
+	als_clear_intr(data);
+
+	return IRQ_HANDLED;
+}
+
+static int als_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	struct als_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	ret = als_chip_init(data);
+	if (ret < 0)
+		goto err;
+
+	mutex_init(&data->mutex);
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->channels = als_channels;
+	indio_dev->num_channels = ARRAY_SIZE(als_channels);
+	indio_dev->name = ALS_DRV_NAME;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	if (client->irq)
+		indio_dev->info = &als_info;
+	else
+		indio_dev->info = &als_info_no_irq;
+
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+				NULL, als_interrupt_handler,
+				IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+				ALS_IRQ_NAME, indio_dev);
+		if (ret) {
+			dev_err(&client->dev, "irq request error %d\n", -ret);
+			goto err;
+		}
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+
+err:
+	/* Ensure that power off in case of error */
+	als_set_power_state(data, 0);
+	return ret;
+}
+
+static int als_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct als_data *data = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+
+	/* Ensure that power off and interrupts are disabled */
+	als_set_intr_state(data, 0);
+	als_set_power_state(data, 0);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int als_suspend(struct device *dev)
+{
+	struct als_data *data = i2c_get_clientdata(to_i2c_client(dev));
+	int ret;
+
+	mutex_lock(&data->mutex);
+	ret = als_set_power_state(data, 0);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int als_resume(struct device *dev)
+{
+	struct als_data *data = i2c_get_clientdata(to_i2c_client(dev));
+	int ret;
+
+	mutex_lock(&data->mutex);
+	ret = als_set_power_state(data, 1);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(als_pm_ops, als_suspend, als_resume);
+#define ALS_PM_OPS (&als_pm_ops)
+#else
+#define ALS_PM_OPS NULL
+#endif
+
+static struct i2c_device_id als_id[] = {
+	{ ALS_DRV_NAME, 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, als_id);
+
+static struct i2c_driver als_driver = {
+	.driver = {
+		.name	= ALS_DRV_NAME,
+		.owner	= THIS_MODULE,
+		.pm	= ALS_PM_OPS,
+	},
+	.probe		= als_probe,
+	.remove		= als_remove,
+	.id_table	= als_id,
+};
+
+module_i2c_driver(als_driver);
+
+MODULE_AUTHOR("Kravchenko Oleksandr <o.v.kravchenko@globallogic.com>");
+MODULE_AUTHOR("GlobalLogic inc.");
+MODULE_DESCRIPTION("APDS9300 ambient light photo sensor driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

[PATCH 3/3] iio: add APDS9300 ambilent light sensor driver

[Oleksandr Kravchenko]

From: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>

This patch adds IIO driver for APDS9300 ambient light sensor (ALS).
http://www.avagotech.com/docs/AV02-1077EN

The driver allows to read raw data from ADC registers or calculate
lux value. It also can handle threshold interrupt.

Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
---
 .../devicetree/bindings/iio/light/apds9300.txt     |   22 +
 drivers/iio/light/Kconfig                          |   10 +
 drivers/iio/light/Makefile                         |    1 +
 drivers/iio/light/apds9300.c                       |  517 ++++++++++++++++++++
 4 files changed, 550 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/apds9300.txt
 create mode 100644 drivers/iio/light/apds9300.c

diff --git a/Documentation/devicetree/bindings/iio/light/apds9300.txt b/Documentation/devicetree/bindings/iio/light/apds9300.txt
new file mode 100644
index 0000000..d6f66c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/apds9300.txt
@@ -0,0 +1,22 @@
+* Avago APDS9300 ambient light sensor
+
+http://www.avagotech.com/docs/AV02-1077EN
+
+Required properties:
+
+  - compatible : should be "avago,apds9300"
+  - reg : the I2C address of the sensor
+
+Optional properties:
+
+  - interrupt-parent : should be the phandle for the interrupt controller
+  - interrupts : interrupt mapping for GPIO IRQ
+
+Example:
+
+apds9300@39 {
+	compatible = "avago,apds9300";
+	reg = <0x39>;
+	interrupt-parent = <&gpio2>;
+	interrupts = <29 8>;
+};
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 5ef1a39..fb6af1b 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -15,6 +15,16 @@ config ADJD_S311
 	 This driver can also be built as a module.  If so, the module
 	 will be called adjd_s311.
 
+config APDS9300
+	tristate "APDS9300 ambient light sensor"
+	depends on I2C
+	help
+	 Say Y here if you want to build a driver for the Avago APDS9300
+	 ambient light sensor.
+
+	 To compile this driver as a module, choose M here: the
+	 module will be called apds9300.
+
 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 040d9c7..da58e12 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -6,4 +6,5 @@ obj-$(CONFIG_ADJD_S311)		+= adjd_s311.o
 obj-$(CONFIG_SENSORS_LM3533)	+= lm3533-als.o
 obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
 obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
+obj-$(CONFIG_APDS9300)		+= apds9300.o
 obj-$(CONFIG_HID_SENSOR_ALS)	+= hid-sensor-als.o
diff --git a/drivers/iio/light/apds9300.c b/drivers/iio/light/apds9300.c
new file mode 100644
index 0000000..c06a59e
--- /dev/null
+++ b/drivers/iio/light/apds9300.c
@@ -0,0 +1,517 @@
+/*
+ * apds9300.c - IIO driver for Avago APDS9300 ambient light sensor
+ *
+ * Copyright 2013 Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/pm.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
+
+#define ALS_DRV_NAME "apds9300"
+#define ALS_IRQ_NAME "apds9300_event"
+
+/* Command register bits */
+#define ALS_CMD		BIT(7) /* Select command register. Must write as 1 */
+#define ALS_WORD	BIT(5) /* I2C write/read: if 1 word, if 0 byte */
+#define ALS_CLEAR	BIT(6) /* Interrupt clear. Clears pending interrupt */
+
+/* Register set */
+#define ALS_CONTROL		0x00 /* Control of basic functions */
+#define ALS_THRESHLOWLOW	0x02 /* Low byte of low interrupt threshold */
+#define ALS_THRESHHIGHLOW	0x04 /* Low byte of high interrupt threshold */
+#define ALS_INTERRUPT		0x06 /* Interrupt control */
+#define ALS_DATA0LOW		0x0c /* Low byte of ADC channel 0 */
+#define ALS_DATA1LOW		0x0e /* Low byte of ADC channel 1 */
+
+/* Power on/off value for ALS_CONTROL register */
+#define ALS_POWER_ON		0x03
+#define ALS_POWER_OFF		0x00
+
+/* Interrupts */
+#define ALS_INTR_ENABLE		0x10
+/* Interrupt Persist Function: Any value outside of threshold range */
+#define ALS_THRESH_INTR		0x01
+
+#define ALS_THRESH_MAX		0xffff /* Max threshold value */
+
+struct als_data {
+	struct i2c_client *client;
+	struct mutex mutex;
+	int power_state;
+	int thresh_low;
+	int thresh_hi;
+	int intr_en;
+};
+
+/* Lux calculation */
+
+/* Calculated values 1000 * (CH1/CH0)^1.4 for CH1/CH0 from 0 to 0.52 */
+static const u16 lux_ratio[] = {
+	0, 2, 4, 7, 11, 15, 19, 24, 29, 34, 40, 45, 51, 57, 64, 70, 77, 84, 91,
+	98, 105, 112, 120, 128, 136, 144, 152, 160, 168, 177, 185, 194, 203,
+	212, 221, 230, 239, 249, 258, 268, 277, 287, 297, 307, 317, 327, 337,
+	347, 358, 368, 379, 390, 400,
+};
+
+static unsigned long als_calculate_lux(u16 ch0, u16 ch1)
+{
+	unsigned long lux, tmp;
+
+	/* avoid division by zero */
+	if (ch0 == 0)
+		return 0;
+
+	tmp = DIV_ROUND_UP(ch1 * 100, ch0);
+	if (tmp <= 52) {
+		/*
+		 * Variable tmp64 need to avoid overflow of this part of lux
+		 * calculation formula.
+		 */
+		lux = 3150 * ch0 - (unsigned long)DIV_ROUND_UP_ULL(ch0
+				* lux_ratio[tmp] * 5930ull, 1000);
+	} else if (tmp <= 65) {
+		lux = 2290 * ch0 - 2910 * ch1;
+	} else if (tmp <= 80) {
+		lux = 1570 * ch0 - 1800 * ch1;
+	} else if (tmp <= 130) {
+		lux = 338 * ch0 - 260 * ch1;
+	} else {
+		lux = 0;
+	}
+
+	return lux / 100000;
+}
+
+/* I2C I/O operations */
+
+static int als_set_power_state(struct als_data *data, int state)
+{
+	int ret;
+	u8 cmd;
+
+	cmd = state ? ALS_POWER_ON : ALS_POWER_OFF;
+	ret = i2c_smbus_write_byte_data(data->client,
+			ALS_CONTROL | ALS_CMD, cmd);
+	if (!ret)
+		data->power_state = state;
+	else
+		dev_err(&data->client->dev,
+			"failed to set power state %d\n", state);
+
+	return ret;
+}
+
+static int als_get_adc_val(struct als_data *data, int adc_number)
+{
+	int ret;
+	u8 flags = ALS_CMD | ALS_WORD;
+
+	if (!data->power_state)
+		return -EBUSY;
+
+	/* Select ADC0 or ADC1 data register */
+	flags |= adc_number ? ALS_DATA1LOW : ALS_DATA0LOW;
+
+	ret = i2c_smbus_read_word_data(data->client, flags);
+	if (ret < 0)
+		dev_err(&data->client->dev,
+			"failed to read ADC%d value\n", adc_number);
+
+	return ret;
+}
+
+static int als_set_thresh_low(struct als_data *data, int value)
+{
+	int ret;
+
+	if (!data->power_state)
+		return -EBUSY;
+
+	if (value > ALS_THRESH_MAX || value > data->thresh_hi)
+		return -EINVAL;
+
+	ret = i2c_smbus_write_word_data(data->client,
+			ALS_THRESHLOWLOW | ALS_CMD | ALS_WORD, value);
+	if (!ret)
+		data->thresh_low = value;
+	else
+		dev_err(&data->client->dev,
+			"failed to set thresh_low\n");
+
+	return ret;
+}
+
+static int als_set_thresh_hi(struct als_data *data, int value)
+{
+	int ret;
+
+	if (!data->power_state)
+		return -EBUSY;
+
+	if (value > ALS_THRESH_MAX || value < data->thresh_low)
+		return -EINVAL;
+
+	ret = i2c_smbus_write_word_data(data->client,
+			ALS_THRESHHIGHLOW | ALS_CMD | ALS_WORD, value);
+	if (!ret)
+		data->thresh_hi = value;
+	else
+		dev_err(&data->client->dev,
+			"failed to set thresh_hi\n");
+
+	return ret;
+}
+
+static int als_set_intr_state(struct als_data *data, int state)
+{
+	int ret;
+	u8 cmd;
+
+	if (!data->power_state)
+		return -EBUSY;
+
+	cmd = state ? ALS_INTR_ENABLE | ALS_THRESH_INTR : 0x00;
+	ret = i2c_smbus_write_byte_data(data->client,
+			ALS_INTERRUPT | ALS_CMD, cmd);
+	if (!ret)
+		data->intr_en = state;
+	else
+		dev_err(&data->client->dev,
+			"failed to set interrupt state %d\n", state);
+
+	return ret;
+}
+
+static void als_clear_intr(struct als_data *data)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte(data->client, ALS_CLEAR | ALS_CMD);
+	if (ret < 0)
+		dev_err(&data->client->dev,
+			"failed to clear interrupt\n");
+}
+
+static int als_chip_init(struct als_data *data)
+{
+	int ret;
+
+	/* Need to set power off to ensure that the chip is off */
+	ret = als_set_power_state(data, 0);
+	if (ret < 0)
+		goto err;
+	/*
+	 * Probe the chip. To do so we try to power up the device and then to
+	 * read back the 0x03 code
+	 */
+	ret = als_set_power_state(data, 1);
+	if (ret < 0)
+		goto err;
+	ret = i2c_smbus_read_byte_data(data->client, ALS_CONTROL | ALS_CMD);
+	if (ret != ALS_POWER_ON) {
+		ret = -ENODEV;
+		goto err;
+	}
+	/*
+	 * Disable interrupt to ensure thai it is doesn't enable
+	 * i.e. after device soft reset
+	 */
+	ret = als_set_intr_state(data, 0);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+
+err:
+	dev_err(&data->client->dev, "failed to init the chip\n");
+	return ret;
+}
+
+/* Industrial I/O data and functions */
+
+static int als_read_raw(struct iio_dev *indio_dev,
+		struct iio_chan_spec const *chan, int *val, int *val2,
+		long mask)
+{
+	int ch0, ch1, ret = -EINVAL;
+	struct als_data *data = iio_priv(indio_dev);
+
+	mutex_lock(&data->mutex);
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED:
+		switch (chan->type) {
+		case IIO_LIGHT:
+			ch0 = als_get_adc_val(data, 0);
+			if (ch0 < 0) {
+				ret = ch0;
+				break;
+			}
+			ch1 = als_get_adc_val(data, 1);
+			if (ch1 < 0) {
+				ret = ch1;
+				break;
+			}
+			*val = als_calculate_lux(ch0, ch1);
+			ret = IIO_VAL_INT;
+			break;
+		case IIO_INTENSITY:
+			ret = als_get_adc_val(data, chan->channel);
+			if (ret < 0)
+				break;
+			*val = ret;
+			ret = IIO_VAL_INT;
+			break;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int als_read_thresh(struct iio_dev *indio_dev, u64 event_code, int *val)
+{
+	struct als_data *data = iio_priv(indio_dev);
+
+	switch (IIO_EVENT_CODE_EXTRACT_DIR(event_code)) {
+	case IIO_EV_DIR_RISING:
+		*val = data->thresh_hi;
+		break;
+	case IIO_EV_DIR_FALLING:
+		*val = data->thresh_low;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int als_write_thresh(struct iio_dev *indio_dev, u64 event_code, int val)
+{
+	struct als_data *data = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&data->mutex);
+	if (IIO_EVENT_CODE_EXTRACT_DIR(event_code) == IIO_EV_DIR_RISING)
+		ret = als_set_thresh_hi(data, val);
+	else
+		ret = als_set_thresh_low(data, val);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int als_read_interrupt_config(struct iio_dev *indio_dev,
+		u64 event_code)
+{
+	struct als_data *data = iio_priv(indio_dev);
+	return data->intr_en;
+}
+
+static int als_write_interrupt_config(struct iio_dev *indio_dev,
+		u64 event_code, int state)
+{
+	struct als_data *data = iio_priv(indio_dev);
+	int ret = 0;
+
+	mutex_lock(&data->mutex);
+	ret = als_set_intr_state(data, state);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static const struct iio_info als_info_no_irq = {
+	.driver_module	= THIS_MODULE,
+	.read_raw	= &als_read_raw,
+};
+
+static const struct iio_info als_info = {
+	.driver_module		= THIS_MODULE,
+	.read_raw		= als_read_raw,
+	.read_event_value	= &als_read_thresh,
+	.write_event_value	= &als_write_thresh,
+	.read_event_config	= &als_read_interrupt_config,
+	.write_event_config	= &als_write_interrupt_config,
+};
+
+static const struct iio_chan_spec als_channels[] = {
+	{
+		.type = IIO_LIGHT,
+		.channel = 0,
+		.indexed = true,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+	}, {
+		.type = IIO_INTENSITY,
+		.channel = 0,
+		.indexed = true,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.event_mask = (IIO_EV_BIT(IIO_EV_TYPE_THRESH,
+					  IIO_EV_DIR_RISING) |
+			       IIO_EV_BIT(IIO_EV_TYPE_THRESH,
+					  IIO_EV_DIR_FALLING)),
+	}, {
+		.type = IIO_INTENSITY,
+		.channel = 1,
+		.indexed = true,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+	},
+};
+
+static irqreturn_t als_interrupt_handler(int irq, void *private)
+{
+	struct iio_dev *dev_info = private;
+	struct als_data *data = iio_priv(dev_info);
+
+	iio_push_event(dev_info,
+		       IIO_UNMOD_EVENT_CODE(IIO_INTENSITY, 0,
+					    IIO_EV_TYPE_THRESH,
+					    IIO_EV_DIR_EITHER),
+		       iio_get_time_ns());
+
+	als_clear_intr(data);
+
+	return IRQ_HANDLED;
+}
+
+static int als_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+	struct als_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+
+	ret = als_chip_init(data);
+	if (ret < 0)
+		goto err;
+
+	mutex_init(&data->mutex);
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->channels = als_channels;
+	indio_dev->num_channels = ARRAY_SIZE(als_channels);
+	indio_dev->name = ALS_DRV_NAME;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	if (client->irq)
+		indio_dev->info = &als_info;
+	else
+		indio_dev->info = &als_info_no_irq;
+
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+				NULL, als_interrupt_handler,
+				IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+				ALS_IRQ_NAME, indio_dev);
+		if (ret) {
+			dev_err(&client->dev, "irq request error %d\n", -ret);
+			goto err;
+		}
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+
+err:
+	/* Ensure that power off in case of error */
+	als_set_power_state(data, 0);
+	return ret;
+}
+
+static int als_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct als_data *data = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+
+	/* Ensure that power off and interrupts are disabled */
+	als_set_intr_state(data, 0);
+	als_set_power_state(data, 0);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int als_suspend(struct device *dev)
+{
+	struct als_data *data = i2c_get_clientdata(to_i2c_client(dev));
+	int ret;
+
+	mutex_lock(&data->mutex);
+	ret = als_set_power_state(data, 0);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int als_resume(struct device *dev)
+{
+	struct als_data *data = i2c_get_clientdata(to_i2c_client(dev));
+	int ret;
+
+	mutex_lock(&data->mutex);
+	ret = als_set_power_state(data, 1);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static SIMPLE_DEV_PM_OPS(als_pm_ops, als_suspend, als_resume);
+#define ALS_PM_OPS (&als_pm_ops)
+#else
+#define ALS_PM_OPS NULL
+#endif
+
+static struct i2c_device_id als_id[] = {
+	{ ALS_DRV_NAME, 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, als_id);
+
+static struct i2c_driver als_driver = {
+	.driver = {
+		.name	= ALS_DRV_NAME,
+		.owner	= THIS_MODULE,
+		.pm	= ALS_PM_OPS,
+	},
+	.probe		= als_probe,
+	.remove		= als_remove,
+	.id_table	= als_id,
+};
+
+module_i2c_driver(als_driver);
+
+MODULE_AUTHOR("Kravchenko Oleksandr <o.v.kravchenko@globallogic.com>");
+MODULE_AUTHOR("GlobalLogic inc.");
+MODULE_DESCRIPTION("APDS9300 ambient light photo sensor driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

Re: [PATCH 3/3] iio: add APDS9300 ambilent light sensor driver

[Jonathan Cameron]

On 07/18/2013 11:19 AM, Oleksandr Kravchenko wrote:
> From: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
>
> This patch adds IIO driver for APDS9300 ambient light sensor (ALS).
> http://www.avagotech.com/docs/AV02-1077EN
>
> The driver allows to read raw data from ADC registers or calculate
> lux value. It also can handle threshold interrupt.
>
> Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>

Mostly looking very nice, couple of points though.

1) ALS_ is not a specific enough prefix to my mind.  I'd like them
replaced with APDS9300_ for all the defines.  Makes it pretty implausible
that a namespace clash will occur wherase ALS is too generic (as everyone
calls their ambient light sensors that - we even proposed a subsystem
under that name at one point a few years ago)

2) Suspend and resume look to have a bug with getting the wrong type
of pointer.

3) Modifiers for the two intensity channels give more info than simply
channel 0 and channel 1.  If you don't want to use them then argue
against it ;)

Anyhow, not much really so looking forward to the next revision!

Thanks,

Jonathan
> ---
>  .../devicetree/bindings/iio/light/apds9300.txt     |   22 +
>  drivers/iio/light/Kconfig                          |   10 +
>  drivers/iio/light/Makefile                         |    1 +
>  drivers/iio/light/apds9300.c                       |  517 ++++++++++++++++++++
>  4 files changed, 550 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/apds9300.txt
>  create mode 100644 drivers/iio/light/apds9300.c
>
> diff --git a/Documentation/devicetree/bindings/iio/light/apds9300.txt b/Documentation/devicetree/bindings/iio/light/apds9300.txt
> new file mode 100644
> index 0000000..d6f66c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/apds9300.txt
> @@ -0,0 +1,22 @@
> +* Avago APDS9300 ambient light sensor
> +
> +http://www.avagotech.com/docs/AV02-1077EN
> +
> +Required properties:
> +
> +  - compatible : should be "avago,apds9300"
> +  - reg : the I2C address of the sensor
> +
> +Optional properties:
> +
> +  - interrupt-parent : should be the phandle for the interrupt controller
> +  - interrupts : interrupt mapping for GPIO IRQ
> +
> +Example:
> +
> +apds9300@39 {
> +	compatible = "avago,apds9300";
> +	reg = <0x39>;
> +	interrupt-parent = <&gpio2>;
> +	interrupts = <29 8>;
> +};
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 5ef1a39..fb6af1b 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -15,6 +15,16 @@ config ADJD_S311
>  	 This driver can also be built as a module.  If so, the module
>  	 will be called adjd_s311.
>
> +config APDS9300
> +	tristate "APDS9300 ambient light sensor"
> +	depends on I2C
> +	help
> +	 Say Y here if you want to build a driver for the Avago APDS9300
> +	 ambient light sensor.
> +
> +	 To compile this driver as a module, choose M here: the
> +	 module will be called apds9300.
> +
>  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 040d9c7..da58e12 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_ADJD_S311)		+= adjd_s311.o
>  obj-$(CONFIG_SENSORS_LM3533)	+= lm3533-als.o
>  obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
>  obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
> +obj-$(CONFIG_APDS9300)		+= apds9300.o
>  obj-$(CONFIG_HID_SENSOR_ALS)	+= hid-sensor-als.o
> diff --git a/drivers/iio/light/apds9300.c b/drivers/iio/light/apds9300.c
> new file mode 100644
> index 0000000..c06a59e
> --- /dev/null
> +++ b/drivers/iio/light/apds9300.c
> @@ -0,0 +1,517 @@
> +/*
> + * apds9300.c - IIO driver for Avago APDS9300 ambient light sensor
> + *
> + * Copyright 2013 Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/pm.h>
> +#include <linux/i2c.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/interrupt.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
> +
I'm not happy that ALS is sufficient to avoid
possible namespace clashes in future.
Please prefix these with APDS9300_

> +#define ALS_DRV_NAME "apds9300"
> +#define ALS_IRQ_NAME "apds9300_event"
> +
> +/* Command register bits */
> +#define ALS_CMD		BIT(7) /* Select command register. Must write as 1 */
> +#define ALS_WORD	BIT(5) /* I2C write/read: if 1 word, if 0 byte */
> +#define ALS_CLEAR	BIT(6) /* Interrupt clear. Clears pending interrupt */
> +
> +/* Register set */
> +#define ALS_CONTROL		0x00 /* Control of basic functions */
> +#define ALS_THRESHLOWLOW	0x02 /* Low byte of low interrupt threshold */
> +#define ALS_THRESHHIGHLOW	0x04 /* Low byte of high interrupt threshold */
> +#define ALS_INTERRUPT		0x06 /* Interrupt control */
> +#define ALS_DATA0LOW		0x0c /* Low byte of ADC channel 0 */
> +#define ALS_DATA1LOW		0x0e /* Low byte of ADC channel 1 */
> +
> +/* Power on/off value for ALS_CONTROL register */
> +#define ALS_POWER_ON		0x03
> +#define ALS_POWER_OFF		0x00
> +
> +/* Interrupts */
> +#define ALS_INTR_ENABLE		0x10
> +/* Interrupt Persist Function: Any value outside of threshold range */
> +#define ALS_THRESH_INTR		0x01
> +
> +#define ALS_THRESH_MAX		0xffff /* Max threshold value */
> +
> +struct als_data {
> +	struct i2c_client *client;
> +	struct mutex mutex;
> +	int power_state;
> +	int thresh_low;
> +	int thresh_hi;
> +	int intr_en;
> +};
> +
> +/* Lux calculation */
> +
> +/* Calculated values 1000 * (CH1/CH0)^1.4 for CH1/CH0 from 0 to 0.52 */
> +static const u16 lux_ratio[] = {
> +	0, 2, 4, 7, 11, 15, 19, 24, 29, 34, 40, 45, 51, 57, 64, 70, 77, 84, 91,
> +	98, 105, 112, 120, 128, 136, 144, 152, 160, 168, 177, 185, 194, 203,
> +	212, 221, 230, 239, 249, 258, 268, 277, 287, 297, 307, 317, 327, 337,
> +	347, 358, 368, 379, 390, 400,
> +};
> +
*laughs* even by normal ALS standards this is one ugly calculation ;)

> +static unsigned long als_calculate_lux(u16 ch0, u16 ch1)
> +{
> +	unsigned long lux, tmp;
> +
> +	/* avoid division by zero */
> +	if (ch0 == 0)
> +		return 0;
> +
> +	tmp = DIV_ROUND_UP(ch1 * 100, ch0);
> +	if (tmp <= 52) {
> +		/*
> +		 * Variable tmp64 need to avoid overflow of this part of lux
> +		 * calculation formula.
> +		 */
> +		lux = 3150 * ch0 - (unsigned long)DIV_ROUND_UP_ULL(ch0
> +				* lux_ratio[tmp] * 5930ull, 1000);
> +	} else if (tmp <= 65) {
> +		lux = 2290 * ch0 - 2910 * ch1;
> +	} else if (tmp <= 80) {
> +		lux = 1570 * ch0 - 1800 * ch1;
> +	} else if (tmp <= 130) {
> +		lux = 338 * ch0 - 260 * ch1;
> +	} else {
> +		lux = 0;
> +	}
> +
> +	return lux / 100000;
> +}
> +
Definitely drop this boilerplate - it's not even all that accurate
as I wouldn't count power state setting as such an operation.
> +/* I2C I/O operations */
> +
> +static int als_set_power_state(struct als_data *data, int state)
> +{
> +	int ret;
> +	u8 cmd;
> +
> +	cmd = state ? ALS_POWER_ON : ALS_POWER_OFF;
> +	ret = i2c_smbus_write_byte_data(data->client,
> +			ALS_CONTROL | ALS_CMD, cmd);
As for the one below, I'd prefer the logic flipped here even a the
cost of a couple more lines of code. (I review drivers backwards ;)
> +	if (!ret)
> +		data->power_state = state;
> +	else
> +		dev_err(&data->client->dev,
> +			"failed to set power state %d\n", state);
> +
> +	return ret;
> +}
> +
> +static int als_get_adc_val(struct als_data *data, int adc_number)
> +{
> +	int ret;
> +	u8 flags = ALS_CMD | ALS_WORD;
> +
> +	if (!data->power_state)
> +		return -EBUSY;
> +
> +	/* Select ADC0 or ADC1 data register */
> +	flags |= adc_number ? ALS_DATA1LOW : ALS_DATA0LOW;
> +
> +	ret = i2c_smbus_read_word_data(data->client, flags);
> +	if (ret < 0)
> +		dev_err(&data->client->dev,
> +			"failed to read ADC%d value\n", adc_number);
> +
> +	return ret;
> +}
> +
> +static int als_set_thresh_low(struct als_data *data, int value)
> +{
> +	int ret;
> +
> +	if (!data->power_state)
> +		return -EBUSY;
> +
Now this is a nasty one as to move your range to an non overlapping one
you have to move the right limit first.  I'd be tempted to be
more nefarious and if necessary 'push' the other limit along.

Note IIO never guarantees that writing to one attribute in sysfs
will not change any others.

The reason I suggest this is to make life easier for userspace
code which can then write the values in any order it likes though
I guess the trick I've suggested might in some case trigger an unexpected
interrupt.

Maybe your way was better in the first place.  I'll leave the decision
on this up to you (assuming no one else comments).
> +	if (value > ALS_THRESH_MAX || value > data->thresh_hi)
> +		return -EINVAL;
> +
> +	ret = i2c_smbus_write_word_data(data->client,
> +			ALS_THRESHLOWLOW | ALS_CMD | ALS_WORD, value);
Whilst you clearly save a couple of lines of code here, I'd prefer
the easier to read inverted option.

if (ret) {
   dev_err(...)
   return ret;
}
data->thresh_low = value;

return 0;
> +	if (!ret)
> +		data->thresh_low = value;
> +	else
> +		dev_err(&data->client->dev,
> +			"failed to set thresh_low\n");
> +
> +	return ret;
> +}
> +
> +static int als_set_thresh_hi(struct als_data *data, int value)
> +{
> +	int ret;
> +
> +	if (!data->power_state)
> +		return -EBUSY;
> +
> +	if (value > ALS_THRESH_MAX || value < data->thresh_low)
> +		return -EINVAL;
> +
> +	ret = i2c_smbus_write_word_data(data->client,
> +			ALS_THRESHHIGHLOW | ALS_CMD | ALS_WORD, value);
> +	if (!ret)
> +		data->thresh_hi = value;
> +	else
> +		dev_err(&data->client->dev,
> +			"failed to set thresh_hi\n");
> +
> +	return ret;
> +}
> +
> +static int als_set_intr_state(struct als_data *data, int state)
> +{
> +	int ret;
> +	u8 cmd;
> +
> +	if (!data->power_state)
> +		return -EBUSY;
> +
> +	cmd = state ? ALS_INTR_ENABLE | ALS_THRESH_INTR : 0x00;
> +	ret = i2c_smbus_write_byte_data(data->client,
> +			ALS_INTERRUPT | ALS_CMD, cmd);
> +	if (!ret)
> +		data->intr_en = state;
> +	else
> +		dev_err(&data->client->dev,
> +			"failed to set interrupt state %d\n", state);
> +
> +	return ret;
> +}
> +
> +static void als_clear_intr(struct als_data *data)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte(data->client, ALS_CLEAR | ALS_CMD);
> +	if (ret < 0)
> +		dev_err(&data->client->dev,
> +			"failed to clear interrupt\n");
> +}
> +
> +static int als_chip_init(struct als_data *data)
> +{
> +	int ret;
> +
> +	/* Need to set power off to ensure that the chip is off */
> +	ret = als_set_power_state(data, 0);
> +	if (ret < 0)
> +		goto err;
> +	/*
> +	 * Probe the chip. To do so we try to power up the device and then to
> +	 * read back the 0x03 code
> +	 */
> +	ret = als_set_power_state(data, 1);
> +	if (ret < 0)
> +		goto err;
> +	ret = i2c_smbus_read_byte_data(data->client, ALS_CONTROL | ALS_CMD);
> +	if (ret != ALS_POWER_ON) {
> +		ret = -ENODEV;
> +		goto err;
> +	}
> +	/*
> +	 * Disable interrupt to ensure thai it is doesn't enable
> +	 * i.e. after device soft reset
> +	 */
> +	ret = als_set_intr_state(data, 0);
> +	if (ret < 0)
> +		goto err;
> +
> +	return 0;
> +
> +err:
> +	dev_err(&data->client->dev, "failed to init the chip\n");
> +	return ret;
> +}
> +
Don't bother with boilerplate comments like this.
The only possible purpose is to tell anyone editting the driver
in the future where to put their changes.   For that we
rely on reviewers like me moaning at them ;)

> +/* Industrial I/O data and functions */
> +
> +static int als_read_raw(struct iio_dev *indio_dev,
> +		struct iio_chan_spec const *chan, int *val, int *val2,
> +		long mask)
> +{
> +	int ch0, ch1, ret = -EINVAL;
> +	struct als_data *data = iio_priv(indio_dev);
> +
> +	mutex_lock(&data->mutex);

The cynical coder (e.g. me) would just not bother checking
the mask at all.  Afterall we know that no read requests
for anything else can come in as the driver doesn't claim
to provide them.

> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +	case IIO_CHAN_INFO_PROCESSED:
> +		switch (chan->type) {
> +		case IIO_LIGHT:
> +			ch0 = als_get_adc_val(data, 0);
> +			if (ch0 < 0) {
> +				ret = ch0;
> +				break;
> +			}
> +			ch1 = als_get_adc_val(data, 1);
> +			if (ch1 < 0) {
> +				ret = ch1;
> +				break;
> +			}
> +			*val = als_calculate_lux(ch0, ch1);
> +			ret = IIO_VAL_INT;
> +			break;
> +		case IIO_INTENSITY:
> +			ret = als_get_adc_val(data, chan->channel);
> +			if (ret < 0)
> +				break;
> +			*val = ret;
> +			ret = IIO_VAL_INT;
> +			break;
> +		default:
> +			break;
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static int als_read_thresh(struct iio_dev *indio_dev, u64 event_code, int *val)
> +{
> +	struct als_data *data = iio_priv(indio_dev);
> +
> +	switch (IIO_EVENT_CODE_EXTRACT_DIR(event_code)) {
> +	case IIO_EV_DIR_RISING:
> +		*val = data->thresh_hi;
> +		break;
> +	case IIO_EV_DIR_FALLING:
> +		*val = data->thresh_low;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int als_write_thresh(struct iio_dev *indio_dev, u64 event_code, int val)
> +{
> +	struct als_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&data->mutex);
> +	if (IIO_EVENT_CODE_EXTRACT_DIR(event_code) == IIO_EV_DIR_RISING)
> +		ret = als_set_thresh_hi(data, val);
> +	else
> +		ret = als_set_thresh_low(data, val);
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static int als_read_interrupt_config(struct iio_dev *indio_dev,
> +		u64 event_code)
> +{
> +	struct als_data *data = iio_priv(indio_dev);
blank line here to keep it the same as the rest of your driver
(which incidentally is very nicely formatted ;)
> +	return data->intr_en;
> +}
> +
> +static int als_write_interrupt_config(struct iio_dev *indio_dev,
> +		u64 event_code, int state)
> +{
> +	struct als_data *data = iio_priv(indio_dev);
> +	int ret = 0;
I'm surprised sparse or checkpatch didn't point out that the = 0;
is pointless given it is always set 2 lines below.
Hence drop it.
> +
> +	mutex_lock(&data->mutex);
> +	ret = als_set_intr_state(data, state);
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static const struct iio_info als_info_no_irq = {
> +	.driver_module	= THIS_MODULE,
> +	.read_raw	= &als_read_raw,
> +};
> +
> +static const struct iio_info als_info = {
> +	.driver_module		= THIS_MODULE,
> +	.read_raw		= als_read_raw,
> +	.read_event_value	= &als_read_thresh,
> +	.write_event_value	= &als_write_thresh,
> +	.read_event_config	= &als_read_interrupt_config,
> +	.write_event_config	= &als_write_interrupt_config,
> +};
> +
> +static const struct iio_chan_spec als_channels[] = {
> +	{
> +		.type = IIO_LIGHT,
> +		.channel = 0,
> +		.indexed = true,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +	}, {
> +		.type = IIO_INTENSITY,
> +		.channel = 0,
> +		.indexed = true,
We did define some 'axis modifiers' for the different sensitive ranges
for these sensors. IIO_MOD_IR and IIO_MOD_BOTH.  Now those
are arguably not some of our better interfaces, but they do
give more information than we get here from a simple channel number.
(note I'm assuming that this part has the usual approach or
 one unfiltered sensor and one with an infrared pass filter?)


> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.event_mask = (IIO_EV_BIT(IIO_EV_TYPE_THRESH,
> +					  IIO_EV_DIR_RISING) |
> +			       IIO_EV_BIT(IIO_EV_TYPE_THRESH,
> +					  IIO_EV_DIR_FALLING)),
> +	}, {
> +		.type = IIO_INTENSITY,
> +		.channel = 1,
> +		.indexed = true,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +	},
> +};
> +
> +static irqreturn_t als_interrupt_handler(int irq, void *private)
> +{
> +	struct iio_dev *dev_info = private;
> +	struct als_data *data = iio_priv(dev_info);
> +
> +	iio_push_event(dev_info,
> +		       IIO_UNMOD_EVENT_CODE(IIO_INTENSITY, 0,
> +					    IIO_EV_TYPE_THRESH,
> +					    IIO_EV_DIR_EITHER),
> +		       iio_get_time_ns());
> +
> +	als_clear_intr(data);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int als_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> +	struct als_data *data;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +
> +	ret = als_chip_init(data);
> +	if (ret < 0)
> +		goto err;
> +
> +	mutex_init(&data->mutex);
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->channels = als_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(als_channels);
> +	indio_dev->name = ALS_DRV_NAME;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	if (client->irq)
> +		indio_dev->info = &als_info;
> +	else
> +		indio_dev->info = &als_info_no_irq;
> +
> +	if (client->irq) {
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +				NULL, als_interrupt_handler,
> +				IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +				ALS_IRQ_NAME, indio_dev);
> +		if (ret) {
> +			dev_err(&client->dev, "irq request error %d\n", -ret);
> +			goto err;
> +		}
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0)
> +		goto err;
> +
> +	return 0;
> +
> +err:
> +	/* Ensure that power off in case of error */
> +	als_set_power_state(data, 0);
> +	return ret;
> +}
> +
> +static int als_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct als_data *data = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	/* Ensure that power off and interrupts are disabled */
> +	als_set_intr_state(data, 0);
> +	als_set_power_state(data, 0);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int als_suspend(struct device *dev)
> +{
> +	struct als_data *data = i2c_get_clientdata(to_i2c_client(dev));

i2c_get_client_data elsewhere gives the pointer to the
struct iio_dev not the iio_priv(indio_dev) that you are using here.

> +	int ret;
> +
> +	mutex_lock(&data->mutex);
> +	ret = als_set_power_state(data, 0);
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static int als_resume(struct device *dev)
> +{
> +	struct als_data *data = i2c_get_clientdata(to_i2c_client(dev));
> +	int ret;
same here.  Incidentally this is the sort of thing that makes me
glad most IIO drivers now have roughly the same form.  I spotted this
simply because I'd normally expect i2c_get_clientdata to return a
struct iio_dev * and it didn't.
> +
> +	mutex_lock(&data->mutex);
> +	ret = als_set_power_state(data, 1);
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(als_pm_ops, als_suspend, als_resume);
> +#define ALS_PM_OPS (&als_pm_ops)
> +#else
> +#define ALS_PM_OPS NULL
> +#endif
> +
> +static struct i2c_device_id als_id[] = {
> +	{ ALS_DRV_NAME, 0 },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, als_id);
> +
> +static struct i2c_driver als_driver = {
> +	.driver = {
> +		.name	= ALS_DRV_NAME,
> +		.owner	= THIS_MODULE,
> +		.pm	= ALS_PM_OPS,
> +	},
> +	.probe		= als_probe,
> +	.remove		= als_remove,
> +	.id_table	= als_id,
> +};
> +
> +module_i2c_driver(als_driver);
> +
> +MODULE_AUTHOR("Kravchenko Oleksandr <o.v.kravchenko@globallogic.com>");
> +MODULE_AUTHOR("GlobalLogic inc.");
> +MODULE_DESCRIPTION("APDS9300 ambient light photo sensor driver");
> +MODULE_LICENSE("GPL");
>

Re: [PATCH 1/3] iio: core: implement devm_iio_device_alloc/devm_iio_device_free

[Lars-Peter Clausen]

On 07/18/2013 12:19 PM, Oleksandr Kravchenko wrote:
> From: Grygorii Strashko <grygorii.strashko@ti.com>
>
> Add a resource managed devm_iio_device_alloc()/devm_iio_device_free()
> to automatically clean up any allocations made by IIO drivers,
> thus leading to simplified IIO drivers code.
>
> In addition, this will allow IIO drivers to use other devm_*() API
> (like devm_request_irq) and don't care about the race between
> iio_device_free() and the release of resources by Device core
> during driver removing.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> [o.v.kravchenko@globallogic.com: fix return value ib devm_iio_device_alloc
> in case if devres_alloc failed, remove unused variable "rc"]
> Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
> Tested-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>

Very nice, thanks for taking care of this.

Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>


> ---
>   drivers/iio/industrialio-core.c |   47 +++++++++++++++++++++++++++++++++++++++
>   include/linux/iio/iio.h         |   25 +++++++++++++++++++++
>   2 files changed, 72 insertions(+)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index e145931..d56d122 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -912,6 +912,53 @@ void iio_device_free(struct iio_dev *dev)
>   }
>   EXPORT_SYMBOL(iio_device_free);
>
> +static void devm_iio_device_release(struct device *dev, void *res)
> +{
> +	iio_device_free(*(struct iio_dev **)res);
> +}
> +
> +static int devm_iio_device_match(struct device *dev, void *res, void *data)
> +{
> +	struct iio_dev **r = res;
> +	if (!r || !*r) {
> +		WARN_ON(!r || !*r);
> +		return 0;
> +	}
> +	return *r == data;
> +}
> +
> +struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
> +{
> +	struct iio_dev **ptr, *iio_dev;
> +
> +	ptr = devres_alloc(devm_iio_device_release, sizeof(*ptr),
> +			   GFP_KERNEL);
> +	if (!ptr)
> +		return NULL;
> +
> +	/* use raw alloc_dr for kmalloc caller tracing */
> +	iio_dev = iio_device_alloc(sizeof_priv);
> +	if (iio_dev) {
> +		*ptr = iio_dev;
> +		devres_add(dev, ptr);
> +	} else {
> +		devres_free(ptr);
> +	}
> +
> +	return iio_dev;
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_device_alloc);
> +
> +void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev)
> +{
> +	int rc;
> +
> +	rc = devres_release(dev, devm_iio_device_release,
> +			    devm_iio_device_match, iio_dev);
> +	WARN_ON(rc);
> +}
> +EXPORT_SYMBOL_GPL(devm_iio_device_free);
> +
>   /**
>    * iio_chrdev_open() - chrdev file open for buffer access and ioctls
>    **/
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 8d171f4..f1d99f6 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -532,6 +532,31 @@ static inline struct iio_dev *iio_priv_to_dev(void *priv)
>   void iio_device_free(struct iio_dev *indio_dev);
>
>   /**
> + * devm_iio_device_alloc - Resource-managed iio_device_alloc()
> + * @dev: 		Device to allocate iio_dev for
> + * @sizeof_priv: 	Space to allocate for private structure.
> + *
> + * Managed iio_device_alloc.  iio_dev allocated with this function is
> + * automatically freed on driver detach.
> + *
> + * If an iio_dev allocated with this function needs to be freed separately,
> + * devm_iio_device_free() must be used.
> + *
> + * RETURNS:
> + * Pointer to allocated iio_dev on success, NULL on failure.
> + */
> +struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv);
> +
> +/**
> + * devm_iio_device_free - Resource-managed iio_device_free()
> + * @dev:		Device this iio_dev belongs to
> + * @indio_dev: 		the iio_dev associated with the device
> + *
> + * Free indio_dev allocated with devm_iio_device_alloc().
> + */
> +void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev);
> +
> +/**
>    * iio_buffer_enabled() - helper function to test if the buffer is enabled
>    * @indio_dev:		IIO device structure for device
>    **/
>

Re: [PATCH 1/3] iio: core: implement devm_iio_device_alloc/devm_iio_device_free

[Jonathan Cameron]

On 07/20/2013 02:05 PM, Lars-Peter Clausen wrote:
> On 07/18/2013 12:19 PM, Oleksandr Kravchenko wrote:
>> From: Grygorii Strashko <grygorii.strashko@ti.com>
>>
>> Add a resource managed devm_iio_device_alloc()/devm_iio_device_free()
>> to automatically clean up any allocations made by IIO drivers,
>> thus leading to simplified IIO drivers code.
>>
>> In addition, this will allow IIO drivers to use other devm_*() API
>> (like devm_request_irq) and don't care about the race between
>> iio_device_free() and the release of resources by Device core
>> during driver removing.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> [o.v.kravchenko@globallogic.com: fix return value ib devm_iio_device_alloc
>> in case if devres_alloc failed, remove unused variable "rc"]
>> Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
>> Tested-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
> 
> Very nice, thanks for taking care of this.
> 
> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>

Indeed.  Thanks for this,

I did drop the [] bit in the sign off as you had a perfect right
to sign off on it anyway and that really cluttered it up with
stuff that isn't in a standard form for that block as far as I know.

Applied to the togreg branch of iio.git.  I suspect we may get a
reasonable flood of patches using this over the next month or two.

Thanks,

Jonathan
> 
> 
>> ---
>>   drivers/iio/industrialio-core.c |   47 +++++++++++++++++++++++++++++++++++++++
>>   include/linux/iio/iio.h         |   25 +++++++++++++++++++++
>>   2 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index e145931..d56d122 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -912,6 +912,53 @@ void iio_device_free(struct iio_dev *dev)
>>   }
>>   EXPORT_SYMBOL(iio_device_free);
>>
>> +static void devm_iio_device_release(struct device *dev, void *res)
>> +{
>> +    iio_device_free(*(struct iio_dev **)res);
>> +}
>> +
>> +static int devm_iio_device_match(struct device *dev, void *res, void *data)
>> +{
>> +    struct iio_dev **r = res;
>> +    if (!r || !*r) {
>> +        WARN_ON(!r || !*r);
>> +        return 0;
>> +    }
>> +    return *r == data;
>> +}
>> +
>> +struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
>> +{
>> +    struct iio_dev **ptr, *iio_dev;
>> +
>> +    ptr = devres_alloc(devm_iio_device_release, sizeof(*ptr),
>> +               GFP_KERNEL);
>> +    if (!ptr)
>> +        return NULL;
>> +
>> +    /* use raw alloc_dr for kmalloc caller tracing */
>> +    iio_dev = iio_device_alloc(sizeof_priv);
>> +    if (iio_dev) {
>> +        *ptr = iio_dev;
>> +        devres_add(dev, ptr);
>> +    } else {
>> +        devres_free(ptr);
>> +    }
>> +
>> +    return iio_dev;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_iio_device_alloc);
>> +
>> +void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev)
>> +{
>> +    int rc;
>> +
>> +    rc = devres_release(dev, devm_iio_device_release,
>> +                devm_iio_device_match, iio_dev);
>> +    WARN_ON(rc);
>> +}
>> +EXPORT_SYMBOL_GPL(devm_iio_device_free);
>> +
>>   /**
>>    * iio_chrdev_open() - chrdev file open for buffer access and ioctls
>>    **/
>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index 8d171f4..f1d99f6 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -532,6 +532,31 @@ static inline struct iio_dev *iio_priv_to_dev(void *priv)
>>   void iio_device_free(struct iio_dev *indio_dev);
>>
>>   /**
>> + * devm_iio_device_alloc - Resource-managed iio_device_alloc()
>> + * @dev:         Device to allocate iio_dev for
>> + * @sizeof_priv:     Space to allocate for private structure.
>> + *
>> + * Managed iio_device_alloc.  iio_dev allocated with this function is
>> + * automatically freed on driver detach.
>> + *
>> + * If an iio_dev allocated with this function needs to be freed separately,
>> + * devm_iio_device_free() must be used.
>> + *
>> + * RETURNS:
>> + * Pointer to allocated iio_dev on success, NULL on failure.
>> + */
>> +struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv);
>> +
>> +/**
>> + * devm_iio_device_free - Resource-managed iio_device_free()
>> + * @dev:        Device this iio_dev belongs to
>> + * @indio_dev:         the iio_dev associated with the device
>> + *
>> + * Free indio_dev allocated with devm_iio_device_alloc().
>> + */
>> +void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev);
>> +
>> +/**
>>    * iio_buffer_enabled() - helper function to test if the buffer is enabled
>>    * @indio_dev:        IIO device structure for device
>>    **/
>>
> 
> -- 
> 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: [PATCH 1/3] iio: core: implement devm_iio_device_alloc/devm_iio_device_free

[Jonathan Cameron]

On 07/20/2013 02:05 PM, Lars-Peter Clausen wrote:
> On 07/18/2013 12:19 PM, Oleksandr Kravchenko wrote:
>> From: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>>
>> Add a resource managed devm_iio_device_alloc()/devm_iio_device_free()
>> to automatically clean up any allocations made by IIO drivers,
>> thus leading to simplified IIO drivers code.
>>
>> In addition, this will allow IIO drivers to use other devm_*() API
>> (like devm_request_irq) and don't care about the race between
>> iio_device_free() and the release of resources by Device core
>> during driver removing.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>> [o.v.kravchenko-hExfYMNmJl/Cnp4W7fqMDg@public.gmane.org: fix return value ib devm_iio_device_alloc
>> in case if devres_alloc failed, remove unused variable "rc"]
>> Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko-hExfYMNmJl/Cnp4W7fqMDg@public.gmane.org>
>> Tested-by: Oleksandr Kravchenko <o.v.kravchenko-hExfYMNmJl/Cnp4W7fqMDg@public.gmane.org>
> 
> Very nice, thanks for taking care of this.
> 
> Reviewed-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>

Indeed.  Thanks for this,

I did drop the [] bit in the sign off as you had a perfect right
to sign off on it anyway and that really cluttered it up with
stuff that isn't in a standard form for that block as far as I know.

Applied to the togreg branch of iio.git.  I suspect we may get a
reasonable flood of patches using this over the next month or two.

Thanks,

Jonathan
> 
> 
>> ---
>>   drivers/iio/industrialio-core.c |   47 +++++++++++++++++++++++++++++++++++++++
>>   include/linux/iio/iio.h         |   25 +++++++++++++++++++++
>>   2 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index e145931..d56d122 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -912,6 +912,53 @@ void iio_device_free(struct iio_dev *dev)
>>   }
>>   EXPORT_SYMBOL(iio_device_free);
>>
>> +static void devm_iio_device_release(struct device *dev, void *res)
>> +{
>> +    iio_device_free(*(struct iio_dev **)res);
>> +}
>> +
>> +static int devm_iio_device_match(struct device *dev, void *res, void *data)
>> +{
>> +    struct iio_dev **r = res;
>> +    if (!r || !*r) {
>> +        WARN_ON(!r || !*r);
>> +        return 0;
>> +    }
>> +    return *r == data;
>> +}
>> +
>> +struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
>> +{
>> +    struct iio_dev **ptr, *iio_dev;
>> +
>> +    ptr = devres_alloc(devm_iio_device_release, sizeof(*ptr),
>> +               GFP_KERNEL);
>> +    if (!ptr)
>> +        return NULL;
>> +
>> +    /* use raw alloc_dr for kmalloc caller tracing */
>> +    iio_dev = iio_device_alloc(sizeof_priv);
>> +    if (iio_dev) {
>> +        *ptr = iio_dev;
>> +        devres_add(dev, ptr);
>> +    } else {
>> +        devres_free(ptr);
>> +    }
>> +
>> +    return iio_dev;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_iio_device_alloc);
>> +
>> +void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev)
>> +{
>> +    int rc;
>> +
>> +    rc = devres_release(dev, devm_iio_device_release,
>> +                devm_iio_device_match, iio_dev);
>> +    WARN_ON(rc);
>> +}
>> +EXPORT_SYMBOL_GPL(devm_iio_device_free);
>> +
>>   /**
>>    * iio_chrdev_open() - chrdev file open for buffer access and ioctls
>>    **/
>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index 8d171f4..f1d99f6 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -532,6 +532,31 @@ static inline struct iio_dev *iio_priv_to_dev(void *priv)
>>   void iio_device_free(struct iio_dev *indio_dev);
>>
>>   /**
>> + * devm_iio_device_alloc - Resource-managed iio_device_alloc()
>> + * @dev:         Device to allocate iio_dev for
>> + * @sizeof_priv:     Space to allocate for private structure.
>> + *
>> + * Managed iio_device_alloc.  iio_dev allocated with this function is
>> + * automatically freed on driver detach.
>> + *
>> + * If an iio_dev allocated with this function needs to be freed separately,
>> + * devm_iio_device_free() must be used.
>> + *
>> + * RETURNS:
>> + * Pointer to allocated iio_dev on success, NULL on failure.
>> + */
>> +struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv);
>> +
>> +/**
>> + * devm_iio_device_free - Resource-managed iio_device_free()
>> + * @dev:        Device this iio_dev belongs to
>> + * @indio_dev:         the iio_dev associated with the device
>> + *
>> + * Free indio_dev allocated with devm_iio_device_alloc().
>> + */
>> +void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev);
>> +
>> +/**
>>    * iio_buffer_enabled() - helper function to test if the buffer is enabled
>>    * @indio_dev:        IIO device structure for device
>>    **/
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] iio: core: implement devm_iio_device_alloc/devm_iio_device_free

[Jonathan Cameron]

[-- Attachment #1: Type: text/plain, Size: 4184 bytes --]

Just noticed that we need do document this in devres.txt....

Oleksandr Kravchenko <x0199363@ti.com> wrote:
>From: Grygorii Strashko <grygorii.strashko@ti.com>
>
>Add a resource managed devm_iio_device_alloc()/devm_iio_device_free()
>to automatically clean up any allocations made by IIO drivers,
>thus leading to simplified IIO drivers code.
>
>In addition, this will allow IIO drivers to use other devm_*() API
>(like devm_request_irq) and don't care about the race between
>iio_device_free() and the release of resources by Device core
>during driver removing.
>
>Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>[o.v.kravchenko@globallogic.com: fix return value ib
>devm_iio_device_alloc
>in case if devres_alloc failed, remove unused variable "rc"]
>Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
>Tested-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
>---
>drivers/iio/industrialio-core.c |   47
>+++++++++++++++++++++++++++++++++++++++
> include/linux/iio/iio.h         |   25 +++++++++++++++++++++
> 2 files changed, 72 insertions(+)
>
>diff --git a/drivers/iio/industrialio-core.c
>b/drivers/iio/industrialio-core.c
>index e145931..d56d122 100644
>--- a/drivers/iio/industrialio-core.c
>+++ b/drivers/iio/industrialio-core.c
>@@ -912,6 +912,53 @@ void iio_device_free(struct iio_dev *dev)
> }
> EXPORT_SYMBOL(iio_device_free);
> 
>+static void devm_iio_device_release(struct device *dev, void *res)
>+{
>+	iio_device_free(*(struct iio_dev **)res);
>+}
>+
>+static int devm_iio_device_match(struct device *dev, void *res, void
>*data)
>+{
>+	struct iio_dev **r = res;
>+	if (!r || !*r) {
>+		WARN_ON(!r || !*r);
>+		return 0;
>+	}
>+	return *r == data;
>+}
>+
>+struct iio_dev *devm_iio_device_alloc(struct device *dev, int
>sizeof_priv)
>+{
>+	struct iio_dev **ptr, *iio_dev;
>+
>+	ptr = devres_alloc(devm_iio_device_release, sizeof(*ptr),
>+			   GFP_KERNEL);
>+	if (!ptr)
>+		return NULL;
>+
>+	/* use raw alloc_dr for kmalloc caller tracing */
>+	iio_dev = iio_device_alloc(sizeof_priv);
>+	if (iio_dev) {
>+		*ptr = iio_dev;
>+		devres_add(dev, ptr);
>+	} else {
>+		devres_free(ptr);
>+	}
>+
>+	return iio_dev;
>+}
>+EXPORT_SYMBOL_GPL(devm_iio_device_alloc);
>+
>+void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev)
>+{
>+	int rc;
>+
>+	rc = devres_release(dev, devm_iio_device_release,
>+			    devm_iio_device_match, iio_dev);
>+	WARN_ON(rc);
>+}
>+EXPORT_SYMBOL_GPL(devm_iio_device_free);
>+
> /**
>  * iio_chrdev_open() - chrdev file open for buffer access and ioctls
>  **/
>diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>index 8d171f4..f1d99f6 100644
>--- a/include/linux/iio/iio.h
>+++ b/include/linux/iio/iio.h
>@@ -532,6 +532,31 @@ static inline struct iio_dev *iio_priv_to_dev(void
>*priv)
> void iio_device_free(struct iio_dev *indio_dev);
> 
> /**
>+ * devm_iio_device_alloc - Resource-managed iio_device_alloc()
>+ * @dev: 		Device to allocate iio_dev for
>+ * @sizeof_priv: 	Space to allocate for private structure.
>+ *
>+ * Managed iio_device_alloc.  iio_dev allocated with this function is
>+ * automatically freed on driver detach.
>+ *
>+ * If an iio_dev allocated with this function needs to be freed
>separately,
>+ * devm_iio_device_free() must be used.
>+ *
>+ * RETURNS:
>+ * Pointer to allocated iio_dev on success, NULL on failure.
>+ */
>+struct iio_dev *devm_iio_device_alloc(struct device *dev, int
>sizeof_priv);
>+
>+/**
>+ * devm_iio_device_free - Resource-managed iio_device_free()
>+ * @dev:		Device this iio_dev belongs to
>+ * @indio_dev: 		the iio_dev associated with the device
>+ *
>+ * Free indio_dev allocated with devm_iio_device_alloc().
>+ */
>+void devm_iio_device_free(struct device *dev, struct iio_dev
>*iio_dev);
>+
>+/**
>* iio_buffer_enabled() - helper function to test if the buffer is
>enabled
>  * @indio_dev:		IIO device structure for device
>  **/
>-- 
>1.7.9.5
>
>--
>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

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

[-- Attachment #2: Type: text/html, Size: 4680 bytes --]

[PATCH 1/3] iio: core: implement devm_iio_device_alloc/devm_iio_device_free

[Oleksandr Kravchenko]

From: Grygorii Strashko <grygorii.strashko@ti.com>

Add a resource managed devm_iio_device_alloc()/devm_iio_device_free()
to automatically clean up any allocations made by IIO drivers,
thus leading to simplified IIO drivers code.

In addition, this will allow IIO drivers to use other devm_*() API
(like devm_request_irq) and don't care about the race between
iio_device_free() and the release of resources by Device core
during driver removing.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
[o.v.kravchenko@globallogic.com: fix return value ib devm_iio_device_alloc
in case if devres_alloc failed, remove unused variable "rc"]
Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
Tested-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
---
 drivers/iio/industrialio-core.c |   47 +++++++++++++++++++++++++++++++++++++++
 include/linux/iio/iio.h         |   25 +++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index e145931..d56d122 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -912,6 +912,53 @@ void iio_device_free(struct iio_dev *dev)
 }
 EXPORT_SYMBOL(iio_device_free);
 
+static void devm_iio_device_release(struct device *dev, void *res)
+{
+	iio_device_free(*(struct iio_dev **)res);
+}
+
+static int devm_iio_device_match(struct device *dev, void *res, void *data)
+{
+	struct iio_dev **r = res;
+	if (!r || !*r) {
+		WARN_ON(!r || !*r);
+		return 0;
+	}
+	return *r == data;
+}
+
+struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
+{
+	struct iio_dev **ptr, *iio_dev;
+
+	ptr = devres_alloc(devm_iio_device_release, sizeof(*ptr),
+			   GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	/* use raw alloc_dr for kmalloc caller tracing */
+	iio_dev = iio_device_alloc(sizeof_priv);
+	if (iio_dev) {
+		*ptr = iio_dev;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return iio_dev;
+}
+EXPORT_SYMBOL_GPL(devm_iio_device_alloc);
+
+void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev)
+{
+	int rc;
+
+	rc = devres_release(dev, devm_iio_device_release,
+			    devm_iio_device_match, iio_dev);
+	WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_iio_device_free);
+
 /**
  * iio_chrdev_open() - chrdev file open for buffer access and ioctls
  **/
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 8d171f4..f1d99f6 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -532,6 +532,31 @@ static inline struct iio_dev *iio_priv_to_dev(void *priv)
 void iio_device_free(struct iio_dev *indio_dev);
 
 /**
+ * devm_iio_device_alloc - Resource-managed iio_device_alloc()
+ * @dev: 		Device to allocate iio_dev for
+ * @sizeof_priv: 	Space to allocate for private structure.
+ *
+ * Managed iio_device_alloc.  iio_dev allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an iio_dev allocated with this function needs to be freed separately,
+ * devm_iio_device_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated iio_dev on success, NULL on failure.
+ */
+struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv);
+
+/**
+ * devm_iio_device_free - Resource-managed iio_device_free()
+ * @dev:		Device this iio_dev belongs to
+ * @indio_dev: 		the iio_dev associated with the device
+ *
+ * Free indio_dev allocated with devm_iio_device_alloc().
+ */
+void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev);
+
+/**
  * iio_buffer_enabled() - helper function to test if the buffer is enabled
  * @indio_dev:		IIO device structure for device
  **/
-- 
1.7.9.5

[PATCH 2/3] of: Add Avago Technologies vendor prefix

[Oleksandr Kravchenko]

From: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>

This commit adds a device tree vendor prefix for Avago Technologies.

Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
---
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d5a79ca..8aab9c6 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -11,6 +11,7 @@ amcc	Applied Micro Circuits Corporation (APM, formally AMCC)
 apm	Applied Micro Circuits Corporation (APM)
 arm	ARM Ltd.
 atmel	Atmel Corporation
+avago	Avago Technologies
 bosch	Bosch Sensortec GmbH
 brcm	Broadcom Corporation
 cavium	Cavium, Inc.
-- 
1.7.9.5

[PATCH 2/3] of: Add Avago Technologies vendor prefix

[Oleksandr Kravchenko]

From: Oleksandr Kravchenko <o.v.kravchenko-hExfYMNmJl/Cnp4W7fqMDg@public.gmane.org>

This commit adds a device tree vendor prefix for Avago Technologies.

Signed-off-by: Oleksandr Kravchenko <o.v.kravchenko-hExfYMNmJl/Cnp4W7fqMDg@public.gmane.org>
---
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d5a79ca..8aab9c6 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -11,6 +11,7 @@ amcc	Applied Micro Circuits Corporation (APM, formally AMCC)
 apm	Applied Micro Circuits Corporation (APM)
 arm	ARM Ltd.
 atmel	Atmel Corporation
+avago	Avago Technologies
 bosch	Bosch Sensortec GmbH
 brcm	Broadcom Corporation
 cavium	Cavium, Inc.
-- 
1.7.9.5