[RFC v1 0/3] iio: chemical: add Atlas pH-SM sensor support

[RFC v1 0/3] iio: chemical: add Atlas pH-SM sensor support

[Matt Ranostay]

* Initial revision of a driver that enables support for the Atlas
  Scientific pH-SM chemical sensor
* Add new IIO_PH iio_chan_type
* Add Atlas Scientific LLC devicetree vendor id

NOTE: Not tested on actual hardware yet. Hence the RFC prefix, just want
      early feedback.

Matt Ranostay (3):
  devicetree: add Atlas Scientific LLC vendor prefix
  iio: ph: add IIO_PH channel type
  iio: chemical: add Atlas pH-SM sensor support

 Documentation/ABI/testing/sysfs-bus-iio            |   6 +
 .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/iio/chemical/Kconfig                       |  13 +
 drivers/iio/chemical/Makefile                      |   1 +
 drivers/iio/chemical/atlas-ph-sensor.c             | 385 +++++++++++++++++++++
 drivers/iio/industrialio-core.c                    |   1 +
 include/uapi/linux/iio/types.h                     |   1 +
 8 files changed, 430 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
 create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c

-- 
1.9.1

[RFC v1 1/3] devicetree: add Atlas Scientific LLC vendor prefix

[Matt Ranostay]

Add the "atlas" vendor prefix for Atlas Scientific LLC

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 Documentation/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 4e5603b..2245178 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -28,6 +28,7 @@ arm	ARM Ltd.
 armadeus	ARMadeus Systems SARL
 artesyn	Artesyn Embedded Technologies Inc.
 asahi-kasei	Asahi Kasei Corp.
+atlas	Atlas Scientific LLC
 atmel	Atmel Corporation
 auo	AU Optronics Corporation
 avago	Avago Technologies
-- 
1.9.1

[RFC v1 2/3] iio: ph: add IIO_PH channel type

[Matt Ranostay]

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 6 ++++++
 drivers/iio/industrialio-core.c         | 1 +
 include/uapi/linux/iio/types.h          | 1 +
 3 files changed, 8 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 0439c2a..b15314e 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1491,3 +1491,9 @@ Description:
 		This ABI is especially applicable for humidity sensors
 		to heatup the device and get rid of any condensation
 		in some humidity environment
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_ph_raw
+KernelVersion:	4.5
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Raw (unscaled no offset etc.) pH reading of a substance.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index d42a8ff9..e1b9a9e 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -77,6 +77,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_VELOCITY] = "velocity",
 	[IIO_CONCENTRATION] = "concentration",
 	[IIO_RESISTANCE] = "resistance",
+	[IIO_PH] = "ph",
 };
 
 static const char * const iio_modifier_names[] = {
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 7c63bd6..c077617 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -37,6 +37,7 @@ enum iio_chan_type {
 	IIO_VELOCITY,
 	IIO_CONCENTRATION,
 	IIO_RESISTANCE,
+	IIO_PH,
 };
 
 enum iio_modifier {
-- 
1.9.1

[RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support

[Matt Ranostay]

Add support for the Atlas Scientific pH-SM chemical sensor that can
detect pH levels of solutions in the range of 0-14.

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
 drivers/iio/chemical/Kconfig                       |  13 +
 drivers/iio/chemical/Makefile                      |   1 +
 drivers/iio/chemical/atlas-ph-sensor.c             | 385 +++++++++++++++++++++
 4 files changed, 421 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
 create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c

diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
new file mode 100644
index 0000000..a060fd1
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
@@ -0,0 +1,22 @@
+* Atlas Scientific pH-SM OEM sensor
+
+http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
+
+Required properties:
+
+  - compatible: must be "atlas,ph-sm"
+  - reg: the I2C address of the sensor
+  - interrupt-parent: should be the phandle for the interrupt controller
+  - interrupts : the sole interrupt generated by the device
+
+  Refer to interrupt-controller/interrupts.txt for generic interrupt client
+  node bindings.
+
+Example:
+
+atlas@65 {
+	compatible = "atlas,ph-sm";
+	reg = <0x65>;
+	interrupt-parent = <&gpio1>;
+	interrupts = <16 2>;
+};
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 3061b72..753eab6 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -4,6 +4,19 @@
 
 menu "Chemical Sensors"
 
+config ATLAS_PH_SENSOR
+	tristate "Atlas Scientific OEM pH-SM sensor"
+	depends on I2C
+	select REGMAP_I2C
+	select IIO_BUFFER
+	select IIO_KFIFO_BUF
+	help
+	 Say Y here to build I2C interface support for the Atlas
+	 Scientific OEM pH-SM sensor.
+
+	 To compile this driver as module, choose M here: the
+	 module will be called atlas-ph-sensor
+
 config VZ89X
 	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index 7292f2d..e9eb852 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -3,4 +3,5 @@
 #
 
 # When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_ATLAS_PH_SENSOR)	+= atlas-ph-sensor.o
 obj-$(CONFIG_VZ89X)		+= vz89x.o
diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
new file mode 100644
index 0000000..6c6f261
--- /dev/null
+++ b/drivers/iio/chemical/atlas-ph-sensor.c
@@ -0,0 +1,385 @@
+/*
+ * atlas-ph-sensor.c - Support for Atlas Scientific OEM pH-SM sensor
+ *
+ * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/err.h>
+#include <linux/irq.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/pm_runtime.h>
+
+#define ATLAS_REGMAP_NAME	"atlas_ph_regmap"
+#define	ATLAS_DRV_NAME		"atlas_ph"
+
+#define ATLAS_REG_DEV_TYPE		0x00
+#define ATLAS_REG_DEV_VERSION		0x01
+
+#define ATLAS_REG_INT_CONTROL		0x04
+#define ATLAS_REG_INT_CONTROL_EN	BIT(2)
+
+#define ATLAS_REG_PWR_CONTROL		0x06
+
+#define ATLAS_REG_TEMP_DATA		0x0e
+#define ATLAS_REG_PH_DATA		0x16
+
+#define ATLAS_PH_INT_TIME_IN_US		450000
+
+struct atlas_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+
+	__be32 buffer[3]; /* 32-bit pH data + 64-bit timestamp */
+};
+
+static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	if (reg == ATLAS_REG_INT_CONTROL)
+		return true;
+
+	return false;
+}
+
+static const struct regmap_config atlas_regmap_config = {
+	.name = ATLAS_REGMAP_NAME,
+
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.volatile_reg = max30100_is_volatile_reg,
+	.max_register = ATLAS_REG_PH_DATA + 4,
+	.cache_type = REGCACHE_FLAT,
+
+};
+
+static const struct iio_chan_spec atlas_channels[] = {
+	{
+		.type = IIO_PH,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_BE,
+		},
+	},
+	{
+		.type = IIO_TEMP,
+		.address = ATLAS_REG_TEMP_DATA,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+
+		.output = 1,
+		.scan_index = -1
+	},
+};
+
+#ifdef CONFIG_PM
+static int atlas_set_powermode(struct atlas_data *data, int on)
+{
+	return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on);
+}
+#else
+static int atlas_set_powermode(struct atlas_data *data, int on)
+{
+	return 0;
+}
+#endif
+
+static int atlas_set_interrupt(struct atlas_data *data, bool state)
+{
+	return regmap_update_bits(data->regmap, ATLAS_REG_INT_CONTROL,
+				  ATLAS_REG_INT_CONTROL_EN,
+				  state ? ATLAS_REG_INT_CONTROL_EN : 0);
+}
+
+static int atlas_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct atlas_data *data = iio_priv(indio_dev);
+
+	pm_runtime_get_sync(&data->client->dev);
+
+	return atlas_set_interrupt(data, true);
+}
+
+static int atlas_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct atlas_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = atlas_set_interrupt(data, false);
+	if (ret)
+		return ret;
+
+	pm_runtime_mark_last_busy(&data->client->dev);
+	return pm_runtime_put_autosuspend(&data->client->dev);
+}
+
+static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
+	.postenable = atlas_buffer_postenable,
+	.predisable = atlas_buffer_predisable,
+};
+
+static irqreturn_t atlas_interrupt_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct atlas_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
+				sizeof(data->buffer[0]), (u8 *) &data->buffer);
+	if (!ret)
+		iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
+				iio_get_time_ns());
+
+	/* re-enable interrupt */
+	atlas_set_interrupt(data, true);
+
+	return IRQ_HANDLED;
+}
+
+static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val)
+{
+	struct device *dev = &data->client->dev;
+	int suspended = pm_runtime_suspended(dev);
+	int ret;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret)
+		return ret;
+
+	if (suspended)
+		usleep_range(ATLAS_PH_INT_TIME_IN_US,
+			     ATLAS_PH_INT_TIME_IN_US + 100000);
+
+	ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
+					    sizeof(*val), (u8 *) val);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
+static int atlas_read_raw(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan,
+			  int *val, int *val2, long mask)
+{
+	struct atlas_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW: {
+		int ret;
+		__be32 reg;
+
+		mutex_lock(&indio_dev->mlock);
+
+		if (iio_buffer_enabled(indio_dev)) {
+			ret = -EINVAL;
+			goto error_busy;
+		}
+
+		switch (chan->type) {
+		case IIO_TEMP:
+			ret = regmap_bulk_read(data->regmap, chan->address,
+					      (u8 *) &reg, sizeof(reg));
+			break;
+		case IIO_PH:
+			ret = atlas_read_ph_measurement(data, &reg);
+			break;
+		default:
+			ret = -EINVAL;
+		}
+
+		if (!ret) {
+			*val = be32_to_cpu(reg);
+			ret = IIO_VAL_INT;
+		}
+
+error_busy:
+		mutex_unlock(&indio_dev->mlock);
+		return ret;
+	}
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0; /* 0.001 */
+		*val2 = 1000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+
+	return -EINVAL;
+}
+
+static int atlas_write_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int val, int val2, long mask)
+{
+	struct atlas_data *data = iio_priv(indio_dev);
+	__be32 reg = cpu_to_be32(val);
+
+	if (val2 != 0)
+		return -EINVAL;
+
+	if (mask != IIO_CHAN_INFO_RAW || chan->type != IIO_TEMP)
+		return -EINVAL;
+
+	return regmap_bulk_write(data->regmap, chan->address, &reg, 4);
+}
+
+static const struct iio_info atlas_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = atlas_read_raw,
+	.write_raw = atlas_write_raw,
+};
+
+static int atlas_probe(struct i2c_client *client,
+		       const struct i2c_device_id *id)
+{
+	struct atlas_data *data;
+	struct iio_buffer *buffer;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	buffer = devm_iio_kfifo_allocate(&client->dev);
+	if (!buffer)
+		return -ENOMEM;
+
+	iio_device_attach_buffer(indio_dev, buffer);
+
+	indio_dev->info = &atlas_info;
+	indio_dev->name = ATLAS_DRV_NAME;
+	indio_dev->channels = atlas_channels;
+	indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);
+	indio_dev->setup_ops = &atlas_buffer_setup_ops;
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+
+	i2c_set_clientdata(client, indio_dev);
+
+	data->regmap = devm_regmap_init_i2c(client, &atlas_regmap_config);
+	if (IS_ERR(data->regmap)) {
+		dev_err(&client->dev, "regmap initialization failed\n");
+		return PTR_ERR(data->regmap);
+	}
+
+	ret = pm_runtime_set_active(&client->dev);
+	if (ret)
+		return ret;
+
+	if (client->irq <= 0) {
+		dev_err(&client->dev, "no valid irq defined\n");
+		return -EINVAL;
+	}
+
+	ret = devm_request_threaded_irq(&client->dev, client->irq,
+					NULL, atlas_interrupt_handler,
+					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					"atlas_event",
+					indio_dev);
+	if (ret) {
+		dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
+		return ret;
+	}
+
+	pm_runtime_enable(&client->dev);
+	pm_runtime_set_autosuspend_delay(&client->dev, 2500);
+	pm_runtime_use_autosuspend(&client->dev);
+
+	ret = atlas_set_powermode(data, 0);
+	if (ret)
+		return ret;
+
+	return iio_device_register(indio_dev);
+}
+
+static int atlas_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct atlas_data *data = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+
+	return atlas_set_powermode(data, 0);
+}
+
+#ifdef CONFIG_PM
+static int atlas_runtime_suspend(struct device *dev)
+{
+	struct atlas_data *data =
+		     iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+
+	return atlas_set_powermode(data, 0);
+}
+
+static int atlas_runtime_resume(struct device *dev)
+{
+	struct atlas_data *data =
+		     iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
+
+	return atlas_set_powermode(data, 1);
+}
+#endif
+
+static const struct dev_pm_ops atlas_pm_ops = {
+	SET_RUNTIME_PM_OPS(atlas_runtime_suspend,
+			   atlas_runtime_resume, NULL)
+};
+
+static const struct i2c_device_id atlas_id[] = {
+	{ "atlas-ph-sm", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, atlas_id);
+
+static const struct of_device_id atlas_dt_ids[] = {
+	{ .compatible = "atlas,ph-sm" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, atlas_dt_ids);
+
+static struct i2c_driver atlas_driver = {
+	.driver = {
+		.name	= ATLAS_DRV_NAME,
+		.of_match_table	= of_match_ptr(atlas_dt_ids),
+		.pm	= &atlas_pm_ops,
+	},
+	.probe		= atlas_probe,
+	.remove		= atlas_remove,
+	.id_table	= atlas_id,
+};
+module_i2c_driver(atlas_driver);
+
+MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
+MODULE_DESCRIPTION("Atlas Scientific pH-SM sensor");
+MODULE_LICENSE("GPL");
-- 
1.9.1

Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support

[Adriana Reus]

A couple of notes inline...

On 10.12.2015 10:10, Matt Ranostay wrote:
> Add support for the Atlas Scientific pH-SM chemical sensor that can
> detect pH levels of solutions in the range of 0-14.
>
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>   .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
>   drivers/iio/chemical/Kconfig                       |  13 +
>   drivers/iio/chemical/Makefile                      |   1 +
>   drivers/iio/chemical/atlas-ph-sensor.c             | 385 +++++++++++++++++++++
>   4 files changed, 421 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>   create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c
>
> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
> new file mode 100644
> index 0000000..a060fd1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
> @@ -0,0 +1,22 @@
> +* Atlas Scientific pH-SM OEM sensor
> +
> +http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
> +
> +Required properties:
> +
> +  - compatible: must be "atlas,ph-sm"
> +  - reg: the I2C address of the sensor
> +  - interrupt-parent: should be the phandle for the interrupt controller
> +  - interrupts : the sole interrupt generated by the device
> +
> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
> +  node bindings.
> +
> +Example:
> +
> +atlas@65 {
> +	compatible = "atlas,ph-sm";
> +	reg = <0x65>;
> +	interrupt-parent = <&gpio1>;
> +	interrupts = <16 2>;
> +};
> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> index 3061b72..753eab6 100644
> --- a/drivers/iio/chemical/Kconfig
> +++ b/drivers/iio/chemical/Kconfig
> @@ -4,6 +4,19 @@
>
>   menu "Chemical Sensors"
>
> +config ATLAS_PH_SENSOR
> +	tristate "Atlas Scientific OEM pH-SM sensor"
> +	depends on I2C
> +	select REGMAP_I2C
> +	select IIO_BUFFER
> +	select IIO_KFIFO_BUF
> +	help
> +	 Say Y here to build I2C interface support for the Atlas
> +	 Scientific OEM pH-SM sensor.
> +
> +	 To compile this driver as module, choose M here: the
> +	 module will be called atlas-ph-sensor
> +
>   config VZ89X
>   	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>   	depends on I2C
> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> index 7292f2d..e9eb852 100644
> --- a/drivers/iio/chemical/Makefile
> +++ b/drivers/iio/chemical/Makefile
> @@ -3,4 +3,5 @@
>   #
>
>   # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_ATLAS_PH_SENSOR)	+= atlas-ph-sensor.o
>   obj-$(CONFIG_VZ89X)		+= vz89x.o
> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
> new file mode 100644
> index 0000000..6c6f261
> --- /dev/null
> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
> @@ -0,0 +1,385 @@
> +/*
> + * atlas-ph-sensor.c - Support for Atlas Scientific OEM pH-SM sensor
> + *
> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/irq.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/pm_runtime.h>
> +
> +#define ATLAS_REGMAP_NAME	"atlas_ph_regmap"
> +#define	ATLAS_DRV_NAME		"atlas_ph"
> +
> +#define ATLAS_REG_DEV_TYPE		0x00
> +#define ATLAS_REG_DEV_VERSION		0x01
> +
> +#define ATLAS_REG_INT_CONTROL		0x04
> +#define ATLAS_REG_INT_CONTROL_EN	BIT(2)
> +
> +#define ATLAS_REG_PWR_CONTROL		0x06
> +
> +#define ATLAS_REG_TEMP_DATA		0x0e
> +#define ATLAS_REG_PH_DATA		0x16
> +
> +#define ATLAS_PH_INT_TIME_IN_US		450000
> +
> +struct atlas_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +
> +	__be32 buffer[3]; /* 32-bit pH data + 64-bit timestamp */
> +};
> +
> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	if (reg == ATLAS_REG_INT_CONTROL)
> +		return true;
> +
> +	return false;
> +}
> +
> +static const struct regmap_config atlas_regmap_config = {
> +	.name = ATLAS_REGMAP_NAME,
> +
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.volatile_reg = max30100_is_volatile_reg,
> +	.max_register = ATLAS_REG_PH_DATA + 4,
> +	.cache_type = REGCACHE_FLAT,
> +
> +};
> +
> +static const struct iio_chan_spec atlas_channels[] = {
> +	{
> +		.type = IIO_PH,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 32,
> +			.storagebits = 32,
> +			.endianness = IIO_BE,
> +		},
> +	},
> +	{
> +		.type = IIO_TEMP,
> +		.address = ATLAS_REG_TEMP_DATA,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +
> +		.output = 1,
> +		.scan_index = -1
> +	},
> +};
> +
> +#ifdef CONFIG_PM
> +static int atlas_set_powermode(struct atlas_data *data, int on)
> +{
> +	return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on);
> +}
> +#else
> +static int atlas_set_powermode(struct atlas_data *data, int on)
> +{
> +	return 0;
> +}
> +#endif
^ Why does this need to depend on CONFIG_PM? You may want to use it
if CONFIG_PM is not defined also (See my comment in the remove
funcion).
> +
> +static int atlas_set_interrupt(struct atlas_data *data, bool state)
> +{
> +	return regmap_update_bits(data->regmap, ATLAS_REG_INT_CONTROL,
> +				  ATLAS_REG_INT_CONTROL_EN,
> +				  state ? ATLAS_REG_INT_CONTROL_EN : 0);
> +}
> +
> +static int atlas_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +
> +	pm_runtime_get_sync(&data->client->dev);
> +
> +	return atlas_set_interrupt(data, true);
> +}
> +
> +static int atlas_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = atlas_set_interrupt(data, false);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_mark_last_busy(&data->client->dev);
> +	return pm_runtime_put_autosuspend(&data->client->dev);
> +}
> +
> +static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
> +	.postenable = atlas_buffer_postenable,
> +	.predisable = atlas_buffer_predisable,
> +};
> +
> +static irqreturn_t atlas_interrupt_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct atlas_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
> +				sizeof(data->buffer[0]), (u8 *) &data->buffer);
> +	if (!ret)
> +		iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> +				iio_get_time_ns());
> +
> +	/* re-enable interrupt */
> +	atlas_set_interrupt(data, true);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val)
> +{
> +	struct device *dev = &data->client->dev;
> +	int suspended = pm_runtime_suspended(dev);
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret)
> +		return ret;
> +
> +	if (suspended)
> +		usleep_range(ATLAS_PH_INT_TIME_IN_US,
> +			     ATLAS_PH_INT_TIME_IN_US + 100000);
> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
> +					    sizeof(*val), (u8 *) val);
> +
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
> +
> +	return ret;
> +}
> +
> +static int atlas_read_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan,
> +			  int *val, int *val2, long mask)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW: {
> +		int ret;
> +		__be32 reg;
> +
> +		mutex_lock(&indio_dev->mlock);
> +
> +		if (iio_buffer_enabled(indio_dev)) {
> +			ret = -EINVAL;
> +			goto error_busy;
> +		}
> +
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			ret = regmap_bulk_read(data->regmap, chan->address,
> +					      (u8 *) &reg, sizeof(reg));
> +			break;
> +		case IIO_PH:
> +			ret = atlas_read_ph_measurement(data, &reg);
> +			break;
> +		default:
> +			ret = -EINVAL;
> +		}
> +
> +		if (!ret) {
> +			*val = be32_to_cpu(reg);
> +			ret = IIO_VAL_INT;
> +		}
> +
> +error_busy:
> +		mutex_unlock(&indio_dev->mlock);
> +		return ret;
> +	}
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0; /* 0.001 */
> +		*val2 = 1000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int atlas_write_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int val, int val2, long mask)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +	__be32 reg = cpu_to_be32(val);
> +
> +	if (val2 != 0)
> +		return -EINVAL;
> +
> +	if (mask != IIO_CHAN_INFO_RAW || chan->type != IIO_TEMP)
> +		return -EINVAL;
> +
> +	return regmap_bulk_write(data->regmap, chan->address, &reg, 4);
> +}
> +
> +static const struct iio_info atlas_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = atlas_read_raw,
> +	.write_raw = atlas_write_raw,
> +};
> +
> +static int atlas_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id)
> +{
> +	struct atlas_data *data;
> +	struct iio_buffer *buffer;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	buffer = devm_iio_kfifo_allocate(&client->dev);
> +	if (!buffer)
> +		return -ENOMEM;
> +
> +	iio_device_attach_buffer(indio_dev, buffer);
> +
> +	indio_dev->info = &atlas_info;
> +	indio_dev->name = ATLAS_DRV_NAME;
> +	indio_dev->channels = atlas_channels;
> +	indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);
> +	indio_dev->setup_ops = &atlas_buffer_setup_ops;
> +
> +	data = iio_priv(indio_dev);
> +	data->client = client;
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	data->regmap = devm_regmap_init_i2c(client, &atlas_regmap_config);
> +	if (IS_ERR(data->regmap)) {
> +		dev_err(&client->dev, "regmap initialization failed\n");
> +		return PTR_ERR(data->regmap);
> +	}
> +
> +	ret = pm_runtime_set_active(&client->dev);
> +	if (ret)
> +		return ret;
> +
> +	if (client->irq <= 0) {
> +		dev_err(&client->dev, "no valid irq defined\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = devm_request_threaded_irq(&client->dev, client->irq,
> +					NULL, atlas_interrupt_handler,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					"atlas_event",
> +					indio_dev);
> +	if (ret) {
> +		dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
> +		return ret;
> +	}
> +
> +	pm_runtime_enable(&client->dev);
> +	pm_runtime_set_autosuspend_delay(&client->dev, 2500);
> +	pm_runtime_use_autosuspend(&client->dev);
> +
> +	ret = atlas_set_powermode(data, 0);
> +	if (ret)
> +		return ret;
Unless I am missing something I don't think you need to explicitly
set power off here ^. Runtime pm will call your suspend callback in
some seconds anyway, so it seems redundant.
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static int atlas_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct atlas_data *data = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	pm_runtime_disable(&client->dev);
> +	pm_runtime_set_suspended(&client->dev);
> +
> +	return atlas_set_powermode(data, 0);
If you don't have CONFIG_PM defined, set_powermode will be no-op,
I don't think that's what you want here.
> +}
> +
> +#ifdef CONFIG_PM
> +static int atlas_runtime_suspend(struct device *dev)
> +{
> +	struct atlas_data *data =
> +		     iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return atlas_set_powermode(data, 0);
> +}
> +
> +static int atlas_runtime_resume(struct device *dev)
> +{
> +	struct atlas_data *data =
> +		     iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return atlas_set_powermode(data, 1);
> +}
> +#endif
> +
> +static const struct dev_pm_ops atlas_pm_ops = {
> +	SET_RUNTIME_PM_OPS(atlas_runtime_suspend,
> +			   atlas_runtime_resume, NULL)
> +};
> +
> +static const struct i2c_device_id atlas_id[] = {
> +	{ "atlas-ph-sm", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, atlas_id);
> +
> +static const struct of_device_id atlas_dt_ids[] = {
> +	{ .compatible = "atlas,ph-sm" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, atlas_dt_ids);
> +
> +static struct i2c_driver atlas_driver = {
> +	.driver = {
> +		.name	= ATLAS_DRV_NAME,
> +		.of_match_table	= of_match_ptr(atlas_dt_ids),
> +		.pm	= &atlas_pm_ops,
> +	},
> +	.probe		= atlas_probe,
> +	.remove		= atlas_remove,
> +	.id_table	= atlas_id,
> +};
> +module_i2c_driver(atlas_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("Atlas Scientific pH-SM sensor");
> +MODULE_LICENSE("GPL");
>

Re: [RFC v1 2/3] iio: ph: add IIO_PH channel type

[Peter Meerwald-Stadler]

> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 6 ++++++
>  drivers/iio/industrialio-core.c         | 1 +
>  include/uapi/linux/iio/types.h          | 1 +
>  3 files changed, 8 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 0439c2a..b15314e 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1491,3 +1491,9 @@ Description:
>  		This ABI is especially applicable for humidity sensors
>  		to heatup the device and get rid of any condensation
>  		in some humidity environment
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_ph_raw
> +KernelVersion:	4.5
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Raw (unscaled no offset etc.) pH reading of a substance.

what is the unit (after application of offset and scale?
or otherwise state that this is unit-less

> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index d42a8ff9..e1b9a9e 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -77,6 +77,7 @@ static const char * const iio_chan_type_name_spec[] = {
>  	[IIO_VELOCITY] = "velocity",
>  	[IIO_CONCENTRATION] = "concentration",
>  	[IIO_RESISTANCE] = "resistance",
> +	[IIO_PH] = "ph",
>  };
>  
>  static const char * const iio_modifier_names[] = {
> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> index 7c63bd6..c077617 100644
> --- a/include/uapi/linux/iio/types.h
> +++ b/include/uapi/linux/iio/types.h
> @@ -37,6 +37,7 @@ enum iio_chan_type {
>  	IIO_VELOCITY,
>  	IIO_CONCENTRATION,
>  	IIO_RESISTANCE,
> +	IIO_PH,
>  };
>  
>  enum iio_modifier {
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support

[Peter Meerwald-Stadler]

> Add support for the Atlas Scientific pH-SM chemical sensor that can
> detect pH levels of solutions in the range of 0-14.

nitpicking below
 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
>  drivers/iio/chemical/Kconfig                       |  13 +
>  drivers/iio/chemical/Makefile                      |   1 +
>  drivers/iio/chemical/atlas-ph-sensor.c             | 385 +++++++++++++++++++++
>  4 files changed, 421 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>  create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
> new file mode 100644
> index 0000000..a060fd1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
> @@ -0,0 +1,22 @@
> +* Atlas Scientific pH-SM OEM sensor
> +
> +http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
> +
> +Required properties:
> +
> +  - compatible: must be "atlas,ph-sm"
> +  - reg: the I2C address of the sensor
> +  - interrupt-parent: should be the phandle for the interrupt controller
> +  - interrupts : the sole interrupt generated by the device

delete space before colon, so "interrupts: the sole"

> +
> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
> +  node bindings.
> +
> +Example:
> +
> +atlas@65 {
> +	compatible = "atlas,ph-sm";
> +	reg = <0x65>;
> +	interrupt-parent = <&gpio1>;
> +	interrupts = <16 2>;
> +};
> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> index 3061b72..753eab6 100644
> --- a/drivers/iio/chemical/Kconfig
> +++ b/drivers/iio/chemical/Kconfig
> @@ -4,6 +4,19 @@
>  
>  menu "Chemical Sensors"
>  
> +config ATLAS_PH_SENSOR
> +	tristate "Atlas Scientific OEM pH-SM sensor"
> +	depends on I2C
> +	select REGMAP_I2C
> +	select IIO_BUFFER
> +	select IIO_KFIFO_BUF
> +	help
> +	 Say Y here to build I2C interface support for the Atlas
> +	 Scientific OEM pH-SM sensor.
> +
> +	 To compile this driver as module, choose M here: the
> +	 module will be called atlas-ph-sensor

full stop

> +
>  config VZ89X
>  	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>  	depends on I2C
> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> index 7292f2d..e9eb852 100644
> --- a/drivers/iio/chemical/Makefile
> +++ b/drivers/iio/chemical/Makefile
> @@ -3,4 +3,5 @@
>  #
>  
>  # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_ATLAS_PH_SENSOR)	+= atlas-ph-sensor.o
>  obj-$(CONFIG_VZ89X)		+= vz89x.o
> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
> new file mode 100644
> index 0000000..6c6f261
> --- /dev/null
> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
> @@ -0,0 +1,385 @@
> +/*
> + * atlas-ph-sensor.c - Support for Atlas Scientific OEM pH-SM sensor
> + *
> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/irq.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/pm_runtime.h>
> +
> +#define ATLAS_REGMAP_NAME	"atlas_ph_regmap"
> +#define	ATLAS_DRV_NAME		"atlas_ph"
> +
> +#define ATLAS_REG_DEV_TYPE		0x00
> +#define ATLAS_REG_DEV_VERSION		0x01
> +
> +#define ATLAS_REG_INT_CONTROL		0x04
> +#define ATLAS_REG_INT_CONTROL_EN	BIT(2)
> +
> +#define ATLAS_REG_PWR_CONTROL		0x06
> +
> +#define ATLAS_REG_TEMP_DATA		0x0e
> +#define ATLAS_REG_PH_DATA		0x16
> +
> +#define ATLAS_PH_INT_TIME_IN_US		450000
> +
> +struct atlas_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +
> +	__be32 buffer[3]; /* 32-bit pH data + 64-bit timestamp */

32-bit pH data + padding + 64-bit timestamp
4 bytes + 4 bytes + 8 bytes

hence, should be __be32 buffer[4]

> +};
> +
> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg)

uups, max30100 stuff sneaked in :)

why is reg unsigned int when register values seems to be 8 bits?

> +{
> +	if (reg == ATLAS_REG_INT_CONTROL)
> +		return true;
> +
> +	return false;
> +}
> +
> +static const struct regmap_config atlas_regmap_config = {
> +	.name = ATLAS_REGMAP_NAME,
> +
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.volatile_reg = max30100_is_volatile_reg,
> +	.max_register = ATLAS_REG_PH_DATA + 4,
> +	.cache_type = REGCACHE_FLAT,
> +
> +};
> +
> +static const struct iio_chan_spec atlas_channels[] = {
> +	{
> +		.type = IIO_PH,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +

drop newline
no .address here?

> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 32,
> +			.storagebits = 32,
> +			.endianness = IIO_BE,
> +		},
> +	},
> +	{
> +		.type = IIO_TEMP,
> +		.address = ATLAS_REG_TEMP_DATA,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +
> +		.output = 1,

_TEMP as an output, wow!

> +		.scan_index = -1
> +	},
> +};
> +
> +#ifdef CONFIG_PM
> +static int atlas_set_powermode(struct atlas_data *data, int on)
> +{
> +	return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on);
> +}
> +#else
> +static int atlas_set_powermode(struct atlas_data *data, int on)
> +{
> +	return 0;
> +}
> +#endif
> +
> +static int atlas_set_interrupt(struct atlas_data *data, bool state)
> +{
> +	return regmap_update_bits(data->regmap, ATLAS_REG_INT_CONTROL,
> +				  ATLAS_REG_INT_CONTROL_EN,
> +				  state ? ATLAS_REG_INT_CONTROL_EN : 0);
> +}
> +
> +static int atlas_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +
> +	pm_runtime_get_sync(&data->client->dev);
> +
> +	return atlas_set_interrupt(data, true);
> +}
> +
> +static int atlas_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = atlas_set_interrupt(data, false);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_mark_last_busy(&data->client->dev);
> +	return pm_runtime_put_autosuspend(&data->client->dev);
> +}
> +
> +static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
> +	.postenable = atlas_buffer_postenable,
> +	.predisable = atlas_buffer_predisable,
> +};
> +
> +static irqreturn_t atlas_interrupt_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct atlas_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
> +				sizeof(data->buffer[0]), (u8 *) &data->buffer);
> +	if (!ret)

i2c_smbus_read_i2c_block_data() return the number of bytes read, so should be 
if (ret > 0)

> +		iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> +				iio_get_time_ns());
> +
> +	/* re-enable interrupt */
> +	atlas_set_interrupt(data, true);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val)
> +{
> +	struct device *dev = &data->client->dev;
> +	int suspended = pm_runtime_suspended(dev);
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret)
> +		return ret;
> +
> +	if (suspended)
> +		usleep_range(ATLAS_PH_INT_TIME_IN_US,
> +			     ATLAS_PH_INT_TIME_IN_US + 100000);
> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
> +					    sizeof(*val), (u8 *) val);
> +
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
> +
> +	return ret;
> +}
> +
> +static int atlas_read_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan,
> +			  int *val, int *val2, long mask)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW: {
> +		int ret;
> +		__be32 reg;
> +
> +		mutex_lock(&indio_dev->mlock);
> +
> +		if (iio_buffer_enabled(indio_dev)) {
> +			ret = -EINVAL;

-EBUSY?

> +			goto error_busy;
> +		}
> +
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			ret = regmap_bulk_read(data->regmap, chan->address,
> +					      (u8 *) &reg, sizeof(reg));

zero on success...

> +			break;
> +		case IIO_PH:
> +			ret = atlas_read_ph_measurement(data, &reg);

number of bytes on success...

> +			break;
> +		default:
> +			ret = -EINVAL;
> +		}
> +
> +		if (!ret) {

... won't work for both

> +			*val = be32_to_cpu(reg);
> +			ret = IIO_VAL_INT;
> +		}
> +
> +error_busy:
> +		mutex_unlock(&indio_dev->mlock);
> +		return ret;
> +	}
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0; /* 0.001 */
> +		*val2 = 1000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int atlas_write_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int val, int val2, long mask)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +	__be32 reg = cpu_to_be32(val);
> +
> +	if (val2 != 0)
> +		return -EINVAL;
> +
> +	if (mask != IIO_CHAN_INFO_RAW || chan->type != IIO_TEMP)
> +		return -EINVAL;
> +
> +	return regmap_bulk_write(data->regmap, chan->address, &reg, 4);
> +}
> +
> +static const struct iio_info atlas_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = atlas_read_raw,
> +	.write_raw = atlas_write_raw,
> +};
> +
> +static int atlas_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id)
> +{
> +	struct atlas_data *data;
> +	struct iio_buffer *buffer;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	buffer = devm_iio_kfifo_allocate(&client->dev);
> +	if (!buffer)
> +		return -ENOMEM;
> +
> +	iio_device_attach_buffer(indio_dev, buffer);
> +
> +	indio_dev->info = &atlas_info;
> +	indio_dev->name = ATLAS_DRV_NAME;
> +	indio_dev->channels = atlas_channels;
> +	indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);

parenthesis not needed

> +	indio_dev->setup_ops = &atlas_buffer_setup_ops;
> +
> +	data = iio_priv(indio_dev);
> +	data->client = client;
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	data->regmap = devm_regmap_init_i2c(client, &atlas_regmap_config);
> +	if (IS_ERR(data->regmap)) {
> +		dev_err(&client->dev, "regmap initialization failed\n");
> +		return PTR_ERR(data->regmap);
> +	}
> +
> +	ret = pm_runtime_set_active(&client->dev);
> +	if (ret)
> +		return ret;
> +
> +	if (client->irq <= 0) {
> +		dev_err(&client->dev, "no valid irq defined\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = devm_request_threaded_irq(&client->dev, client->irq,
> +					NULL, atlas_interrupt_handler,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					"atlas_event",

how about "atlas_irq"?

> +					indio_dev);
> +	if (ret) {
> +		dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
> +		return ret;
> +	}
> +
> +	pm_runtime_enable(&client->dev);
> +	pm_runtime_set_autosuspend_delay(&client->dev, 2500);
> +	pm_runtime_use_autosuspend(&client->dev);
> +
> +	ret = atlas_set_powermode(data, 0);
> +	if (ret)
> +		return ret;
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static int atlas_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct atlas_data *data = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	pm_runtime_disable(&client->dev);
> +	pm_runtime_set_suspended(&client->dev);
> +
> +	return atlas_set_powermode(data, 0);
> +}
> +
> +#ifdef CONFIG_PM
> +static int atlas_runtime_suspend(struct device *dev)
> +{
> +	struct atlas_data *data =
> +		     iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return atlas_set_powermode(data, 0);
> +}
> +
> +static int atlas_runtime_resume(struct device *dev)
> +{
> +	struct atlas_data *data =
> +		     iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return atlas_set_powermode(data, 1);
> +}
> +#endif
> +
> +static const struct dev_pm_ops atlas_pm_ops = {
> +	SET_RUNTIME_PM_OPS(atlas_runtime_suspend,
> +			   atlas_runtime_resume, NULL)
> +};
> +
> +static const struct i2c_device_id atlas_id[] = {
> +	{ "atlas-ph-sm", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, atlas_id);
> +
> +static const struct of_device_id atlas_dt_ids[] = {
> +	{ .compatible = "atlas,ph-sm" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, atlas_dt_ids);
> +
> +static struct i2c_driver atlas_driver = {
> +	.driver = {
> +		.name	= ATLAS_DRV_NAME,
> +		.of_match_table	= of_match_ptr(atlas_dt_ids),
> +		.pm	= &atlas_pm_ops,
> +	},
> +	.probe		= atlas_probe,
> +	.remove		= atlas_remove,
> +	.id_table	= atlas_id,
> +};
> +module_i2c_driver(atlas_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("Atlas Scientific pH-SM sensor");
> +MODULE_LICENSE("GPL");
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support

[Matt Ranostay]

On Thu, Dec 10, 2015 at 3:04 AM, Peter Meerwald-Stadler
<pmeerw@pmeerw.net> wrote:
>
>> Add support for the Atlas Scientific pH-SM chemical sensor that can
>> detect pH levels of solutions in the range of 0-14.
>
> nitpicking below
>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> ---
>>  .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
>>  drivers/iio/chemical/Kconfig                       |  13 +
>>  drivers/iio/chemical/Makefile                      |   1 +
>>  drivers/iio/chemical/atlas-ph-sensor.c             | 385 +++++++++++++++++++++
>>  4 files changed, 421 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>  create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c
>>
>> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>> new file mode 100644
>> index 0000000..a060fd1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>> @@ -0,0 +1,22 @@
>> +* Atlas Scientific pH-SM OEM sensor
>> +
>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
>> +
>> +Required properties:
>> +
>> +  - compatible: must be "atlas,ph-sm"
>> +  - reg: the I2C address of the sensor
>> +  - interrupt-parent: should be the phandle for the interrupt controller
>> +  - interrupts : the sole interrupt generated by the device
>
> delete space before colon, so "interrupts: the sole"
>
>> +
>> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
>> +  node bindings.
>> +
>> +Example:
>> +
>> +atlas@65 {
>> +     compatible = "atlas,ph-sm";
>> +     reg = <0x65>;
>> +     interrupt-parent = <&gpio1>;
>> +     interrupts = <16 2>;
>> +};
>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
>> index 3061b72..753eab6 100644
>> --- a/drivers/iio/chemical/Kconfig
>> +++ b/drivers/iio/chemical/Kconfig
>> @@ -4,6 +4,19 @@
>>
>>  menu "Chemical Sensors"
>>
>> +config ATLAS_PH_SENSOR
>> +     tristate "Atlas Scientific OEM pH-SM sensor"
>> +     depends on I2C
>> +     select REGMAP_I2C
>> +     select IIO_BUFFER
>> +     select IIO_KFIFO_BUF
>> +     help
>> +      Say Y here to build I2C interface support for the Atlas
>> +      Scientific OEM pH-SM sensor.
>> +
>> +      To compile this driver as module, choose M here: the
>> +      module will be called atlas-ph-sensor
>
> full stop
>
>> +
>>  config VZ89X
>>       tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>>       depends on I2C
>> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
>> index 7292f2d..e9eb852 100644
>> --- a/drivers/iio/chemical/Makefile
>> +++ b/drivers/iio/chemical/Makefile
>> @@ -3,4 +3,5 @@
>>  #
>>
>>  # When adding new entries keep the list in alphabetical order
>> +obj-$(CONFIG_ATLAS_PH_SENSOR)        += atlas-ph-sensor.o
>>  obj-$(CONFIG_VZ89X)          += vz89x.o
>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
>> new file mode 100644
>> index 0000000..6c6f261
>> --- /dev/null
>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
>> @@ -0,0 +1,385 @@
>> +/*
>> + * atlas-ph-sensor.c - Support for Atlas Scientific OEM pH-SM sensor
>> + *
>> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/delay.h>
>> +#include <linux/mutex.h>
>> +#include <linux/err.h>
>> +#include <linux/irq.h>
>> +#include <linux/gpio.h>
>> +#include <linux/i2c.h>
>> +#include <linux/regmap.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/kfifo_buf.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#define ATLAS_REGMAP_NAME    "atlas_ph_regmap"
>> +#define      ATLAS_DRV_NAME          "atlas_ph"
>> +
>> +#define ATLAS_REG_DEV_TYPE           0x00
>> +#define ATLAS_REG_DEV_VERSION                0x01
>> +
>> +#define ATLAS_REG_INT_CONTROL                0x04
>> +#define ATLAS_REG_INT_CONTROL_EN     BIT(2)
>> +
>> +#define ATLAS_REG_PWR_CONTROL                0x06
>> +
>> +#define ATLAS_REG_TEMP_DATA          0x0e
>> +#define ATLAS_REG_PH_DATA            0x16
>> +
>> +#define ATLAS_PH_INT_TIME_IN_US              450000
>> +
>> +struct atlas_data {
>> +     struct i2c_client *client;
>> +     struct regmap *regmap;
>> +
>> +     __be32 buffer[3]; /* 32-bit pH data + 64-bit timestamp */
>
> 32-bit pH data + padding + 64-bit timestamp
> 4 bytes + 4 bytes + 8 bytes
>
> hence, should be __be32 buffer[4]
>
>> +};
>> +
>> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg)
>
> uups, max30100 stuff sneaked in :)
Oops :)

>
> why is reg unsigned int when register values seems to be 8 bits?
Well because .volatile_reg expects - >    bool (*volatile_reg)(struct
device *dev, unsigned int reg);

>
>> +{
>> +     if (reg == ATLAS_REG_INT_CONTROL)
>> +             return true;
>> +
>> +     return false;
>> +}
>> +
>> +static const struct regmap_config atlas_regmap_config = {
>> +     .name = ATLAS_REGMAP_NAME,
>> +
>> +     .reg_bits = 8,
>> +     .val_bits = 8,
>> +
>> +     .volatile_reg = max30100_is_volatile_reg,
>> +     .max_register = ATLAS_REG_PH_DATA + 4,
>> +     .cache_type = REGCACHE_FLAT,
>> +
>> +};
>> +
>> +static const struct iio_chan_spec atlas_channels[] = {
>> +     {
>> +             .type = IIO_PH,
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +
>
> drop newline
> no .address here?
Could add it, but wouldn't use it anywhere.

>
>> +             .scan_index = 0,
>> +             .scan_type = {
>> +                     .sign = 'u',
>> +                     .realbits = 32,
>> +                     .storagebits = 32,
>> +                     .endianness = IIO_BE,
>> +             },
>> +     },
>> +     {
>> +             .type = IIO_TEMP,
>> +             .address = ATLAS_REG_TEMP_DATA,
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +
>> +             .output = 1,
>
> _TEMP as an output, wow!

Yeah needs an external probe to note the temp of the liquid being
measured. Since pH is affected by temperature.

>
>> +             .scan_index = -1
>> +     },
>> +};
>> +
>> +#ifdef CONFIG_PM
>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>> +{
>> +     return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on);
>> +}
>> +#else
>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>> +{
>> +     return 0;
>> +}
>> +#endif
>> +
>> +static int atlas_set_interrupt(struct atlas_data *data, bool state)
>> +{
>> +     return regmap_update_bits(data->regmap, ATLAS_REG_INT_CONTROL,
>> +                               ATLAS_REG_INT_CONTROL_EN,
>> +                               state ? ATLAS_REG_INT_CONTROL_EN : 0);
>> +}
>> +
>> +static int atlas_buffer_postenable(struct iio_dev *indio_dev)
>> +{
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +
>> +     pm_runtime_get_sync(&data->client->dev);
>> +
>> +     return atlas_set_interrupt(data, true);
>> +}
>> +
>> +static int atlas_buffer_predisable(struct iio_dev *indio_dev)
>> +{
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +     int ret;
>> +
>> +     ret = atlas_set_interrupt(data, false);
>> +     if (ret)
>> +             return ret;
>> +
>> +     pm_runtime_mark_last_busy(&data->client->dev);
>> +     return pm_runtime_put_autosuspend(&data->client->dev);
>> +}
>> +
>> +static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
>> +     .postenable = atlas_buffer_postenable,
>> +     .predisable = atlas_buffer_predisable,
>> +};
>> +
>> +static irqreturn_t atlas_interrupt_handler(int irq, void *private)
>> +{
>> +     struct iio_dev *indio_dev = private;
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +     int ret;
>> +
>> +     ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
>> +                             sizeof(data->buffer[0]), (u8 *) &data->buffer);
>> +     if (!ret)
>
> i2c_smbus_read_i2c_block_data() return the number of bytes read, so should be
> if (ret > 0)
Ah good catch!

>
>> +             iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>> +                             iio_get_time_ns());
>> +
>> +     /* re-enable interrupt */
>> +     atlas_set_interrupt(data, true);
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val)
>> +{
>> +     struct device *dev = &data->client->dev;
>> +     int suspended = pm_runtime_suspended(dev);
>> +     int ret;
>> +
>> +     ret = pm_runtime_get_sync(dev);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (suspended)
>> +             usleep_range(ATLAS_PH_INT_TIME_IN_US,
>> +                          ATLAS_PH_INT_TIME_IN_US + 100000);
>> +
>> +     ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
>> +                                         sizeof(*val), (u8 *) val);
>> +
>> +     pm_runtime_mark_last_busy(dev);
>> +     pm_runtime_put_autosuspend(dev);
>> +
>> +     return ret;
>> +}
>> +
>> +static int atlas_read_raw(struct iio_dev *indio_dev,
>> +                       struct iio_chan_spec const *chan,
>> +                       int *val, int *val2, long mask)
>> +{
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_RAW: {
>> +             int ret;
>> +             __be32 reg;
>> +
>> +             mutex_lock(&indio_dev->mlock);
>> +
>> +             if (iio_buffer_enabled(indio_dev)) {
>> +                     ret = -EINVAL;
>
> -EBUSY?
>
Noted.

>> +                     goto error_busy;
>> +             }
>> +
>> +             switch (chan->type) {
>> +             case IIO_TEMP:
>> +                     ret = regmap_bulk_read(data->regmap, chan->address,
>> +                                           (u8 *) &reg, sizeof(reg));
>
> zero on success...
>
>> +                     break;
>> +             case IIO_PH:
>> +                     ret = atlas_read_ph_measurement(data, &reg);
>
> number of bytes on success...
>
>> +                     break;
>> +             default:
>> +                     ret = -EINVAL;
>> +             }
>> +
>> +             if (!ret) {
>
> ... won't work for both
>
>> +                     *val = be32_to_cpu(reg);
>> +                     ret = IIO_VAL_INT;
>> +             }
>> +
>> +error_busy:
>> +             mutex_unlock(&indio_dev->mlock);
>> +             return ret;
>> +     }
>> +     case IIO_CHAN_INFO_SCALE:
>> +             *val = 0; /* 0.001 */
>> +             *val2 = 1000;
>> +             return IIO_VAL_INT_PLUS_MICRO;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static int atlas_write_raw(struct iio_dev *indio_dev,
>> +                        struct iio_chan_spec const *chan,
>> +                        int val, int val2, long mask)
>> +{
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +     __be32 reg = cpu_to_be32(val);
>> +
>> +     if (val2 != 0)
>> +             return -EINVAL;
>> +
>> +     if (mask != IIO_CHAN_INFO_RAW || chan->type != IIO_TEMP)
>> +             return -EINVAL;
>> +
>> +     return regmap_bulk_write(data->regmap, chan->address, &reg, 4);
>> +}
>> +
>> +static const struct iio_info atlas_info = {
>> +     .driver_module = THIS_MODULE,
>> +     .read_raw = atlas_read_raw,
>> +     .write_raw = atlas_write_raw,
>> +};
>> +
>> +static int atlas_probe(struct i2c_client *client,
>> +                    const struct i2c_device_id *id)
>> +{
>> +     struct atlas_data *data;
>> +     struct iio_buffer *buffer;
>> +     struct iio_dev *indio_dev;
>> +     int ret;
>> +
>> +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> +     if (!indio_dev)
>> +             return -ENOMEM;
>> +
>> +     buffer = devm_iio_kfifo_allocate(&client->dev);
>> +     if (!buffer)
>> +             return -ENOMEM;
>> +
>> +     iio_device_attach_buffer(indio_dev, buffer);
>> +
>> +     indio_dev->info = &atlas_info;
>> +     indio_dev->name = ATLAS_DRV_NAME;
>> +     indio_dev->channels = atlas_channels;
>> +     indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);
>
> parenthesis not needed
>
>> +     indio_dev->setup_ops = &atlas_buffer_setup_ops;
>> +
>> +     data = iio_priv(indio_dev);
>> +     data->client = client;
>> +
>> +     i2c_set_clientdata(client, indio_dev);
>> +
>> +     data->regmap = devm_regmap_init_i2c(client, &atlas_regmap_config);
>> +     if (IS_ERR(data->regmap)) {
>> +             dev_err(&client->dev, "regmap initialization failed\n");
>> +             return PTR_ERR(data->regmap);
>> +     }
>> +
>> +     ret = pm_runtime_set_active(&client->dev);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (client->irq <= 0) {
>> +             dev_err(&client->dev, "no valid irq defined\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     ret = devm_request_threaded_irq(&client->dev, client->irq,
>> +                                     NULL, atlas_interrupt_handler,
>> +                                     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>> +                                     "atlas_event",
>
> how about "atlas_irq"?
>
>> +                                     indio_dev);
>> +     if (ret) {
>> +             dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
>> +             return ret;
>> +     }
>> +
>> +     pm_runtime_enable(&client->dev);
>> +     pm_runtime_set_autosuspend_delay(&client->dev, 2500);
>> +     pm_runtime_use_autosuspend(&client->dev);
>> +
>> +     ret = atlas_set_powermode(data, 0);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return iio_device_register(indio_dev);
>> +}
>> +
>> +static int atlas_remove(struct i2c_client *client)
>> +{
>> +     struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +
>> +     iio_device_unregister(indio_dev);
>> +
>> +     pm_runtime_disable(&client->dev);
>> +     pm_runtime_set_suspended(&client->dev);
>> +
>> +     return atlas_set_powermode(data, 0);
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int atlas_runtime_suspend(struct device *dev)
>> +{
>> +     struct atlas_data *data =
>> +                  iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +     return atlas_set_powermode(data, 0);
>> +}
>> +
>> +static int atlas_runtime_resume(struct device *dev)
>> +{
>> +     struct atlas_data *data =
>> +                  iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +     return atlas_set_powermode(data, 1);
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops atlas_pm_ops = {
>> +     SET_RUNTIME_PM_OPS(atlas_runtime_suspend,
>> +                        atlas_runtime_resume, NULL)
>> +};
>> +
>> +static const struct i2c_device_id atlas_id[] = {
>> +     { "atlas-ph-sm", 0 },
>> +     {}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, atlas_id);
>> +
>> +static const struct of_device_id atlas_dt_ids[] = {
>> +     { .compatible = "atlas,ph-sm" },
>> +     { }
>> +};
>> +MODULE_DEVICE_TABLE(of, atlas_dt_ids);
>> +
>> +static struct i2c_driver atlas_driver = {
>> +     .driver = {
>> +             .name   = ATLAS_DRV_NAME,
>> +             .of_match_table = of_match_ptr(atlas_dt_ids),
>> +             .pm     = &atlas_pm_ops,
>> +     },
>> +     .probe          = atlas_probe,
>> +     .remove         = atlas_remove,
>> +     .id_table       = atlas_id,
>> +};
>> +module_i2c_driver(atlas_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> +MODULE_DESCRIPTION("Atlas Scientific pH-SM sensor");
>> +MODULE_LICENSE("GPL");
>>
>
> --
>
> Peter Meerwald-Stadler
> +43-664-2444418 (mobile)

Re: [RFC v1 2/3] iio: ph: add IIO_PH channel type

[Matt Ranostay]

On Thu, Dec 10, 2015 at 2:40 AM, Peter Meerwald-Stadler
<pmeerw@pmeerw.net> wrote:
>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio | 6 ++++++
>>  drivers/iio/industrialio-core.c         | 1 +
>>  include/uapi/linux/iio/types.h          | 1 +
>>  3 files changed, 8 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index 0439c2a..b15314e 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -1491,3 +1491,9 @@ Description:
>>               This ABI is especially applicable for humidity sensors
>>               to heatup the device and get rid of any condensation
>>               in some humidity environment
>> +
>> +What:                /sys/bus/iio/devices/iio:deviceX/in_ph_raw
>> +KernelVersion:       4.5
>> +Contact:     linux-iio@vger.kernel.org
>> +Description:
>> +             Raw (unscaled no offset etc.) pH reading of a substance.
>
> what is the unit (after application of offset and scale?
> or otherwise state that this is unit-less

It will be floating point 0.0 to 14.0 with the pH level.  Which is the
log scale of hydronium (per litre) in a solution.

>
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index d42a8ff9..e1b9a9e 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -77,6 +77,7 @@ static const char * const iio_chan_type_name_spec[] = {
>>       [IIO_VELOCITY] = "velocity",
>>       [IIO_CONCENTRATION] = "concentration",
>>       [IIO_RESISTANCE] = "resistance",
>> +     [IIO_PH] = "ph",
>>  };
>>
>>  static const char * const iio_modifier_names[] = {
>> diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
>> index 7c63bd6..c077617 100644
>> --- a/include/uapi/linux/iio/types.h
>> +++ b/include/uapi/linux/iio/types.h
>> @@ -37,6 +37,7 @@ enum iio_chan_type {
>>       IIO_VELOCITY,
>>       IIO_CONCENTRATION,
>>       IIO_RESISTANCE,
>> +     IIO_PH,
>>  };
>>
>>  enum iio_modifier {
>>
>
> --
>
> Peter Meerwald-Stadler
> +43-664-2444418 (mobile)

Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support

[Matt Ranostay]

On Thu, Dec 10, 2015 at 2:24 AM, Adriana Reus <adriana.reus@intel.com> wrote:
> A couple of notes inline...
>
>
> On 10.12.2015 10:10, Matt Ranostay wrote:
>>
>> Add support for the Atlas Scientific pH-SM chemical sensor that can
>> detect pH levels of solutions in the range of 0-14.
>>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> ---
>>   .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
>>   drivers/iio/chemical/Kconfig                       |  13 +
>>   drivers/iio/chemical/Makefile                      |   1 +
>>   drivers/iio/chemical/atlas-ph-sensor.c             | 385
>> +++++++++++++++++++++
>>   4 files changed, 421 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>   create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c
>>
>> diff --git
>> a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>> b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>> new file mode 100644
>> index 0000000..a060fd1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>> @@ -0,0 +1,22 @@
>> +* Atlas Scientific pH-SM OEM sensor
>> +
>>
>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
>> +
>> +Required properties:
>> +
>> +  - compatible: must be "atlas,ph-sm"
>> +  - reg: the I2C address of the sensor
>> +  - interrupt-parent: should be the phandle for the interrupt controller
>> +  - interrupts : the sole interrupt generated by the device
>> +
>> +  Refer to interrupt-controller/interrupts.txt for generic interrupt
>> client
>> +  node bindings.
>> +
>> +Example:
>> +
>> +atlas@65 {
>> +       compatible = "atlas,ph-sm";
>> +       reg = <0x65>;
>> +       interrupt-parent = <&gpio1>;
>> +       interrupts = <16 2>;
>> +};
>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
>> index 3061b72..753eab6 100644
>> --- a/drivers/iio/chemical/Kconfig
>> +++ b/drivers/iio/chemical/Kconfig
>> @@ -4,6 +4,19 @@
>>
>>   menu "Chemical Sensors"
>>
>> +config ATLAS_PH_SENSOR
>> +       tristate "Atlas Scientific OEM pH-SM sensor"
>> +       depends on I2C
>> +       select REGMAP_I2C
>> +       select IIO_BUFFER
>> +       select IIO_KFIFO_BUF
>> +       help
>> +        Say Y here to build I2C interface support for the Atlas
>> +        Scientific OEM pH-SM sensor.
>> +
>> +        To compile this driver as module, choose M here: the
>> +        module will be called atlas-ph-sensor
>> +
>>   config VZ89X
>>         tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>>         depends on I2C
>> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
>> index 7292f2d..e9eb852 100644
>> --- a/drivers/iio/chemical/Makefile
>> +++ b/drivers/iio/chemical/Makefile
>> @@ -3,4 +3,5 @@
>>   #
>>
>>   # When adding new entries keep the list in alphabetical order
>> +obj-$(CONFIG_ATLAS_PH_SENSOR)  += atlas-ph-sensor.o
>>   obj-$(CONFIG_VZ89X)           += vz89x.o
>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c
>> b/drivers/iio/chemical/atlas-ph-sensor.c
>> new file mode 100644
>> index 0000000..6c6f261
>> --- /dev/null
>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
>> @@ -0,0 +1,385 @@
>> +/*
>> + * atlas-ph-sensor.c - Support for Atlas Scientific OEM pH-SM sensor
>> + *
>> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/delay.h>
>> +#include <linux/mutex.h>
>> +#include <linux/err.h>
>> +#include <linux/irq.h>
>> +#include <linux/gpio.h>
>> +#include <linux/i2c.h>
>> +#include <linux/regmap.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/kfifo_buf.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#define ATLAS_REGMAP_NAME      "atlas_ph_regmap"
>> +#define        ATLAS_DRV_NAME          "atlas_ph"
>> +
>> +#define ATLAS_REG_DEV_TYPE             0x00
>> +#define ATLAS_REG_DEV_VERSION          0x01
>> +
>> +#define ATLAS_REG_INT_CONTROL          0x04
>> +#define ATLAS_REG_INT_CONTROL_EN       BIT(2)
>> +
>> +#define ATLAS_REG_PWR_CONTROL          0x06
>> +
>> +#define ATLAS_REG_TEMP_DATA            0x0e
>> +#define ATLAS_REG_PH_DATA              0x16
>> +
>> +#define ATLAS_PH_INT_TIME_IN_US                450000
>> +
>> +struct atlas_data {
>> +       struct i2c_client *client;
>> +       struct regmap *regmap;
>> +
>> +       __be32 buffer[3]; /* 32-bit pH data + 64-bit timestamp */
>> +};
>> +
>> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int
>> reg)
>> +{
>> +       if (reg == ATLAS_REG_INT_CONTROL)
>> +               return true;
>> +
>> +       return false;
>> +}
>> +
>> +static const struct regmap_config atlas_regmap_config = {
>> +       .name = ATLAS_REGMAP_NAME,
>> +
>> +       .reg_bits = 8,
>> +       .val_bits = 8,
>> +
>> +       .volatile_reg = max30100_is_volatile_reg,
>> +       .max_register = ATLAS_REG_PH_DATA + 4,
>> +       .cache_type = REGCACHE_FLAT,
>> +
>> +};
>> +
>> +static const struct iio_chan_spec atlas_channels[] = {
>> +       {
>> +               .type = IIO_PH,
>> +               .info_mask_separate =
>> +                       BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +
>> +               .scan_index = 0,
>> +               .scan_type = {
>> +                       .sign = 'u',
>> +                       .realbits = 32,
>> +                       .storagebits = 32,
>> +                       .endianness = IIO_BE,
>> +               },
>> +       },
>> +       {
>> +               .type = IIO_TEMP,
>> +               .address = ATLAS_REG_TEMP_DATA,
>> +               .info_mask_separate =
>> +                       BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +
>> +               .output = 1,
>> +               .scan_index = -1
>> +       },
>> +};
>> +
>> +#ifdef CONFIG_PM
>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>> +{
>> +       return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on);
>> +}
>> +#else
>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>> +{
>> +       return 0;
>> +}
>> +#endif
>
> ^ Why does this need to depend on CONFIG_PM? You may want to use it
> if CONFIG_PM is not defined also (See my comment in the remove
> funcion).
>
>> +
>> +static int atlas_set_interrupt(struct atlas_data *data, bool state)
>> +{
>> +       return regmap_update_bits(data->regmap, ATLAS_REG_INT_CONTROL,
>> +                                 ATLAS_REG_INT_CONTROL_EN,
>> +                                 state ? ATLAS_REG_INT_CONTROL_EN : 0);
>> +}
>> +
>> +static int atlas_buffer_postenable(struct iio_dev *indio_dev)
>> +{
>> +       struct atlas_data *data = iio_priv(indio_dev);
>> +
>> +       pm_runtime_get_sync(&data->client->dev);
>> +
>> +       return atlas_set_interrupt(data, true);
>> +}
>> +
>> +static int atlas_buffer_predisable(struct iio_dev *indio_dev)
>> +{
>> +       struct atlas_data *data = iio_priv(indio_dev);
>> +       int ret;
>> +
>> +       ret = atlas_set_interrupt(data, false);
>> +       if (ret)
>> +               return ret;
>> +
>> +       pm_runtime_mark_last_busy(&data->client->dev);
>> +       return pm_runtime_put_autosuspend(&data->client->dev);
>> +}
>> +
>> +static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
>> +       .postenable = atlas_buffer_postenable,
>> +       .predisable = atlas_buffer_predisable,
>> +};
>> +
>> +static irqreturn_t atlas_interrupt_handler(int irq, void *private)
>> +{
>> +       struct iio_dev *indio_dev = private;
>> +       struct atlas_data *data = iio_priv(indio_dev);
>> +       int ret;
>> +
>> +       ret = i2c_smbus_read_i2c_block_data(data->client,
>> ATLAS_REG_PH_DATA,
>> +                               sizeof(data->buffer[0]), (u8 *)
>> &data->buffer);
>> +       if (!ret)
>> +               iio_push_to_buffers_with_timestamp(indio_dev,
>> data->buffer,
>> +                               iio_get_time_ns());
>> +
>> +       /* re-enable interrupt */
>> +       atlas_set_interrupt(data, true);
>> +
>> +       return IRQ_HANDLED;
>> +}
>> +
>> +static int atlas_read_ph_measurement(struct atlas_data *data, __be32
>> *val)
>> +{
>> +       struct device *dev = &data->client->dev;
>> +       int suspended = pm_runtime_suspended(dev);
>> +       int ret;
>> +
>> +       ret = pm_runtime_get_sync(dev);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (suspended)
>> +               usleep_range(ATLAS_PH_INT_TIME_IN_US,
>> +                            ATLAS_PH_INT_TIME_IN_US + 100000);
>> +
>> +       ret = i2c_smbus_read_i2c_block_data(data->client,
>> ATLAS_REG_PH_DATA,
>> +                                           sizeof(*val), (u8 *) val);
>> +
>> +       pm_runtime_mark_last_busy(dev);
>> +       pm_runtime_put_autosuspend(dev);
>> +
>> +       return ret;
>> +}
>> +
>> +static int atlas_read_raw(struct iio_dev *indio_dev,
>> +                         struct iio_chan_spec const *chan,
>> +                         int *val, int *val2, long mask)
>> +{
>> +       struct atlas_data *data = iio_priv(indio_dev);
>> +
>> +       switch (mask) {
>> +       case IIO_CHAN_INFO_RAW: {
>> +               int ret;
>> +               __be32 reg;
>> +
>> +               mutex_lock(&indio_dev->mlock);
>> +
>> +               if (iio_buffer_enabled(indio_dev)) {
>> +                       ret = -EINVAL;
>> +                       goto error_busy;
>> +               }
>> +
>> +               switch (chan->type) {
>> +               case IIO_TEMP:
>> +                       ret = regmap_bulk_read(data->regmap,
>> chan->address,
>> +                                             (u8 *) &reg, sizeof(reg));
>> +                       break;
>> +               case IIO_PH:
>> +                       ret = atlas_read_ph_measurement(data, &reg);
>> +                       break;
>> +               default:
>> +                       ret = -EINVAL;
>> +               }
>> +
>> +               if (!ret) {
>> +                       *val = be32_to_cpu(reg);
>> +                       ret = IIO_VAL_INT;
>> +               }
>> +
>> +error_busy:
>> +               mutex_unlock(&indio_dev->mlock);
>> +               return ret;
>> +       }
>> +       case IIO_CHAN_INFO_SCALE:
>> +               *val = 0; /* 0.001 */
>> +               *val2 = 1000;
>> +               return IIO_VAL_INT_PLUS_MICRO;
>> +       }
>> +
>> +       return -EINVAL;
>> +}
>> +
>> +static int atlas_write_raw(struct iio_dev *indio_dev,
>> +                          struct iio_chan_spec const *chan,
>> +                          int val, int val2, long mask)
>> +{
>> +       struct atlas_data *data = iio_priv(indio_dev);
>> +       __be32 reg = cpu_to_be32(val);
>> +
>> +       if (val2 != 0)
>> +               return -EINVAL;
>> +
>> +       if (mask != IIO_CHAN_INFO_RAW || chan->type != IIO_TEMP)
>> +               return -EINVAL;
>> +
>> +       return regmap_bulk_write(data->regmap, chan->address, &reg, 4);
>> +}
>> +
>> +static const struct iio_info atlas_info = {
>> +       .driver_module = THIS_MODULE,
>> +       .read_raw = atlas_read_raw,
>> +       .write_raw = atlas_write_raw,
>> +};
>> +
>> +static int atlas_probe(struct i2c_client *client,
>> +                      const struct i2c_device_id *id)
>> +{
>> +       struct atlas_data *data;
>> +       struct iio_buffer *buffer;
>> +       struct iio_dev *indio_dev;
>> +       int ret;
>> +
>> +       indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> +       if (!indio_dev)
>> +               return -ENOMEM;
>> +
>> +       buffer = devm_iio_kfifo_allocate(&client->dev);
>> +       if (!buffer)
>> +               return -ENOMEM;
>> +
>> +       iio_device_attach_buffer(indio_dev, buffer);
>> +
>> +       indio_dev->info = &atlas_info;
>> +       indio_dev->name = ATLAS_DRV_NAME;
>> +       indio_dev->channels = atlas_channels;
>> +       indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);
>> +       indio_dev->setup_ops = &atlas_buffer_setup_ops;
>> +
>> +       data = iio_priv(indio_dev);
>> +       data->client = client;
>> +
>> +       i2c_set_clientdata(client, indio_dev);
>> +
>> +       data->regmap = devm_regmap_init_i2c(client, &atlas_regmap_config);
>> +       if (IS_ERR(data->regmap)) {
>> +               dev_err(&client->dev, "regmap initialization failed\n");
>> +               return PTR_ERR(data->regmap);
>> +       }
>> +
>> +       ret = pm_runtime_set_active(&client->dev);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (client->irq <= 0) {
>> +               dev_err(&client->dev, "no valid irq defined\n");
>> +               return -EINVAL;
>> +       }
>> +
>> +       ret = devm_request_threaded_irq(&client->dev, client->irq,
>> +                                       NULL, atlas_interrupt_handler,
>> +                                       IRQF_TRIGGER_FALLING |
>> IRQF_ONESHOT,
>> +                                       "atlas_event",
>> +                                       indio_dev);
>> +       if (ret) {
>> +               dev_err(&client->dev, "request irq (%d) failed\n",
>> client->irq);
>> +               return ret;
>> +       }
>> +
>> +       pm_runtime_enable(&client->dev);
>> +       pm_runtime_set_autosuspend_delay(&client->dev, 2500);
>> +       pm_runtime_use_autosuspend(&client->dev);
>> +
>> +       ret = atlas_set_powermode(data, 0);
>> +       if (ret)
>> +               return ret;
>
> Unless I am missing something I don't think you need to explicitly
> set power off here ^. Runtime pm will call your suspend callback in
> some seconds anyway, so it seems redundant.
Ah true. Would pm_runtime_mark_last_busy be required to be called here as well?

Although doing more thought on this.. that it should be enabling power
up in case CONFIG_PM/runtime pm isn't enabled... since .remove() could
power off the device on a reset.
>>
>> +
>> +       return iio_device_register(indio_dev);
>> +}
>> +
>> +static int atlas_remove(struct i2c_client *client)
>> +{
>> +       struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> +       struct atlas_data *data = iio_priv(indio_dev);
>> +
>> +       iio_device_unregister(indio_dev);
>> +
>> +       pm_runtime_disable(&client->dev);
>> +       pm_runtime_set_suspended(&client->dev);
>> +
>> +       return atlas_set_powermode(data, 0);
>
> If you don't have CONFIG_PM defined, set_powermode will be no-op,
> I don't think that's what you want here.
>
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int atlas_runtime_suspend(struct device *dev)
>> +{
>> +       struct atlas_data *data =
>> +                    iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +       return atlas_set_powermode(data, 0);
>> +}
>> +
>> +static int atlas_runtime_resume(struct device *dev)
>> +{
>> +       struct atlas_data *data =
>> +                    iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +       return atlas_set_powermode(data, 1);
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops atlas_pm_ops = {
>> +       SET_RUNTIME_PM_OPS(atlas_runtime_suspend,
>> +                          atlas_runtime_resume, NULL)
>> +};
>> +
>> +static const struct i2c_device_id atlas_id[] = {
>> +       { "atlas-ph-sm", 0 },
>> +       {}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, atlas_id);
>> +
>> +static const struct of_device_id atlas_dt_ids[] = {
>> +       { .compatible = "atlas,ph-sm" },
>> +       { }
>> +};
>> +MODULE_DEVICE_TABLE(of, atlas_dt_ids);
>> +
>> +static struct i2c_driver atlas_driver = {
>> +       .driver = {
>> +               .name   = ATLAS_DRV_NAME,
>> +               .of_match_table = of_match_ptr(atlas_dt_ids),
>> +               .pm     = &atlas_pm_ops,
>> +       },
>> +       .probe          = atlas_probe,
>> +       .remove         = atlas_remove,
>> +       .id_table       = atlas_id,
>> +};
>> +module_i2c_driver(atlas_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> +MODULE_DESCRIPTION("Atlas Scientific pH-SM sensor");
>> +MODULE_LICENSE("GPL");
>>
>

Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support

[Adriana Reus]

On 11.12.2015 06:50, Matt Ranostay wrote:
> On Thu, Dec 10, 2015 at 2:24 AM, Adriana Reus <adriana.reus@intel.com> wrote:
>> A couple of notes inline...
>>
>>
>> On 10.12.2015 10:10, Matt Ranostay wrote:
>>>
>>> Add support for the Atlas Scientific pH-SM chemical sensor that can
>>> detect pH levels of solutions in the range of 0-14.
>>>
>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>>> ---
>>>    .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
>>>    drivers/iio/chemical/Kconfig                       |  13 +
>>>    drivers/iio/chemical/Makefile                      |   1 +
>>>    drivers/iio/chemical/atlas-ph-sensor.c             | 385
>>> +++++++++++++++++++++
>>>    4 files changed, 421 insertions(+)
>>>    create mode 100644
>>> Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>>    create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>> b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>> new file mode 100644
>>> index 0000000..a060fd1
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>> @@ -0,0 +1,22 @@
>>> +* Atlas Scientific pH-SM OEM sensor
>>> +
>>>
>>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
>>> +
>>> +Required properties:
>>> +
>>> +  - compatible: must be "atlas,ph-sm"
>>> +  - reg: the I2C address of the sensor
>>> +  - interrupt-parent: should be the phandle for the interrupt controller
>>> +  - interrupts : the sole interrupt generated by the device
>>> +
>>> +  Refer to interrupt-controller/interrupts.txt for generic interrupt
>>> client
>>> +  node bindings.
>>> +
>>> +Example:
>>> +
>>> +atlas@65 {
>>> +       compatible = "atlas,ph-sm";
>>> +       reg = <0x65>;
>>> +       interrupt-parent = <&gpio1>;
>>> +       interrupts = <16 2>;
>>> +};
>>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
>>> index 3061b72..753eab6 100644
>>> --- a/drivers/iio/chemical/Kconfig
>>> +++ b/drivers/iio/chemical/Kconfig
>>> @@ -4,6 +4,19 @@
>>>
>>>    menu "Chemical Sensors"
>>>
>>> +config ATLAS_PH_SENSOR
>>> +       tristate "Atlas Scientific OEM pH-SM sensor"
>>> +       depends on I2C
>>> +       select REGMAP_I2C
>>> +       select IIO_BUFFER
>>> +       select IIO_KFIFO_BUF
>>> +       help
>>> +        Say Y here to build I2C interface support for the Atlas
>>> +        Scientific OEM pH-SM sensor.
>>> +
>>> +        To compile this driver as module, choose M here: the
>>> +        module will be called atlas-ph-sensor
>>> +
>>>    config VZ89X
>>>          tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>>>          depends on I2C
>>> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
>>> index 7292f2d..e9eb852 100644
>>> --- a/drivers/iio/chemical/Makefile
>>> +++ b/drivers/iio/chemical/Makefile
>>> @@ -3,4 +3,5 @@
>>>    #
>>>
>>>    # When adding new entries keep the list in alphabetical order
>>> +obj-$(CONFIG_ATLAS_PH_SENSOR)  += atlas-ph-sensor.o
>>>    obj-$(CONFIG_VZ89X)           += vz89x.o
>>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c
>>> b/drivers/iio/chemical/atlas-ph-sensor.c
>>> new file mode 100644
>>> index 0000000..6c6f261
>>> --- /dev/null
>>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
>>> @@ -0,0 +1,385 @@
>>> +/*
>>> + * atlas-ph-sensor.c - Support for Atlas Scientific OEM pH-SM sensor
>>> + *
>>> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/init.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/err.h>
>>> +#include <linux/irq.h>
>>> +#include <linux/gpio.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/buffer.h>
>>> +#include <linux/iio/kfifo_buf.h>
>>> +#include <linux/pm_runtime.h>
>>> +
>>> +#define ATLAS_REGMAP_NAME      "atlas_ph_regmap"
>>> +#define        ATLAS_DRV_NAME          "atlas_ph"
>>> +
>>> +#define ATLAS_REG_DEV_TYPE             0x00
>>> +#define ATLAS_REG_DEV_VERSION          0x01
>>> +
>>> +#define ATLAS_REG_INT_CONTROL          0x04
>>> +#define ATLAS_REG_INT_CONTROL_EN       BIT(2)
>>> +
>>> +#define ATLAS_REG_PWR_CONTROL          0x06
>>> +
>>> +#define ATLAS_REG_TEMP_DATA            0x0e
>>> +#define ATLAS_REG_PH_DATA              0x16
>>> +
>>> +#define ATLAS_PH_INT_TIME_IN_US                450000
>>> +
>>> +struct atlas_data {
>>> +       struct i2c_client *client;
>>> +       struct regmap *regmap;
>>> +
>>> +       __be32 buffer[3]; /* 32-bit pH data + 64-bit timestamp */
>>> +};
>>> +
>>> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int
>>> reg)
>>> +{
>>> +       if (reg == ATLAS_REG_INT_CONTROL)
>>> +               return true;
>>> +
>>> +       return false;
>>> +}
>>> +
>>> +static const struct regmap_config atlas_regmap_config = {
>>> +       .name = ATLAS_REGMAP_NAME,
>>> +
>>> +       .reg_bits = 8,
>>> +       .val_bits = 8,
>>> +
>>> +       .volatile_reg = max30100_is_volatile_reg,
>>> +       .max_register = ATLAS_REG_PH_DATA + 4,
>>> +       .cache_type = REGCACHE_FLAT,
>>> +
>>> +};
>>> +
>>> +static const struct iio_chan_spec atlas_channels[] = {
>>> +       {
>>> +               .type = IIO_PH,
>>> +               .info_mask_separate =
>>> +                       BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>> +
>>> +               .scan_index = 0,
>>> +               .scan_type = {
>>> +                       .sign = 'u',
>>> +                       .realbits = 32,
>>> +                       .storagebits = 32,
>>> +                       .endianness = IIO_BE,
>>> +               },
>>> +       },
>>> +       {
>>> +               .type = IIO_TEMP,
>>> +               .address = ATLAS_REG_TEMP_DATA,
>>> +               .info_mask_separate =
>>> +                       BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>> +
>>> +               .output = 1,
>>> +               .scan_index = -1
>>> +       },
>>> +};
>>> +
>>> +#ifdef CONFIG_PM
>>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>>> +{
>>> +       return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on);
>>> +}
>>> +#else
>>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>>> +{
>>> +       return 0;
>>> +}
>>> +#endif
>>
>> ^ Why does this need to depend on CONFIG_PM? You may want to use it
>> if CONFIG_PM is not defined also (See my comment in the remove
>> funcion).
>>
>>> +
>>> +static int atlas_set_interrupt(struct atlas_data *data, bool state)
>>> +{
>>> +       return regmap_update_bits(data->regmap, ATLAS_REG_INT_CONTROL,
>>> +                                 ATLAS_REG_INT_CONTROL_EN,
>>> +                                 state ? ATLAS_REG_INT_CONTROL_EN : 0);
>>> +}
>>> +
>>> +static int atlas_buffer_postenable(struct iio_dev *indio_dev)
>>> +{
>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>> +
>>> +       pm_runtime_get_sync(&data->client->dev);
>>> +
>>> +       return atlas_set_interrupt(data, true);
>>> +}
>>> +
>>> +static int atlas_buffer_predisable(struct iio_dev *indio_dev)
>>> +{
>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>> +       int ret;
>>> +
>>> +       ret = atlas_set_interrupt(data, false);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       pm_runtime_mark_last_busy(&data->client->dev);
>>> +       return pm_runtime_put_autosuspend(&data->client->dev);
>>> +}
>>> +
>>> +static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
>>> +       .postenable = atlas_buffer_postenable,
>>> +       .predisable = atlas_buffer_predisable,
>>> +};
>>> +
>>> +static irqreturn_t atlas_interrupt_handler(int irq, void *private)
>>> +{
>>> +       struct iio_dev *indio_dev = private;
>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>> +       int ret;
>>> +
>>> +       ret = i2c_smbus_read_i2c_block_data(data->client,
>>> ATLAS_REG_PH_DATA,
>>> +                               sizeof(data->buffer[0]), (u8 *)
>>> &data->buffer);
>>> +       if (!ret)
>>> +               iio_push_to_buffers_with_timestamp(indio_dev,
>>> data->buffer,
>>> +                               iio_get_time_ns());
>>> +
>>> +       /* re-enable interrupt */
>>> +       atlas_set_interrupt(data, true);
>>> +
>>> +       return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int atlas_read_ph_measurement(struct atlas_data *data, __be32
>>> *val)
>>> +{
>>> +       struct device *dev = &data->client->dev;
>>> +       int suspended = pm_runtime_suspended(dev);
>>> +       int ret;
>>> +
>>> +       ret = pm_runtime_get_sync(dev);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       if (suspended)
>>> +               usleep_range(ATLAS_PH_INT_TIME_IN_US,
>>> +                            ATLAS_PH_INT_TIME_IN_US + 100000);
>>> +
>>> +       ret = i2c_smbus_read_i2c_block_data(data->client,
>>> ATLAS_REG_PH_DATA,
>>> +                                           sizeof(*val), (u8 *) val);
>>> +
>>> +       pm_runtime_mark_last_busy(dev);
>>> +       pm_runtime_put_autosuspend(dev);
>>> +
>>> +       return ret;
>>> +}
>>> +
>>> +static int atlas_read_raw(struct iio_dev *indio_dev,
>>> +                         struct iio_chan_spec const *chan,
>>> +                         int *val, int *val2, long mask)
>>> +{
>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>> +
>>> +       switch (mask) {
>>> +       case IIO_CHAN_INFO_RAW: {
>>> +               int ret;
>>> +               __be32 reg;
>>> +
>>> +               mutex_lock(&indio_dev->mlock);
>>> +
>>> +               if (iio_buffer_enabled(indio_dev)) {
>>> +                       ret = -EINVAL;
>>> +                       goto error_busy;
>>> +               }
>>> +
>>> +               switch (chan->type) {
>>> +               case IIO_TEMP:
>>> +                       ret = regmap_bulk_read(data->regmap,
>>> chan->address,
>>> +                                             (u8 *) &reg, sizeof(reg));
>>> +                       break;
>>> +               case IIO_PH:
>>> +                       ret = atlas_read_ph_measurement(data, &reg);
>>> +                       break;
>>> +               default:
>>> +                       ret = -EINVAL;
>>> +               }
>>> +
>>> +               if (!ret) {
>>> +                       *val = be32_to_cpu(reg);
>>> +                       ret = IIO_VAL_INT;
>>> +               }
>>> +
>>> +error_busy:
>>> +               mutex_unlock(&indio_dev->mlock);
>>> +               return ret;
>>> +       }
>>> +       case IIO_CHAN_INFO_SCALE:
>>> +               *val = 0; /* 0.001 */
>>> +               *val2 = 1000;
>>> +               return IIO_VAL_INT_PLUS_MICRO;
>>> +       }
>>> +
>>> +       return -EINVAL;
>>> +}
>>> +
>>> +static int atlas_write_raw(struct iio_dev *indio_dev,
>>> +                          struct iio_chan_spec const *chan,
>>> +                          int val, int val2, long mask)
>>> +{
>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>> +       __be32 reg = cpu_to_be32(val);
>>> +
>>> +       if (val2 != 0)
>>> +               return -EINVAL;
>>> +
>>> +       if (mask != IIO_CHAN_INFO_RAW || chan->type != IIO_TEMP)
>>> +               return -EINVAL;
>>> +
>>> +       return regmap_bulk_write(data->regmap, chan->address, &reg, 4);
>>> +}
>>> +
>>> +static const struct iio_info atlas_info = {
>>> +       .driver_module = THIS_MODULE,
>>> +       .read_raw = atlas_read_raw,
>>> +       .write_raw = atlas_write_raw,
>>> +};
>>> +
>>> +static int atlas_probe(struct i2c_client *client,
>>> +                      const struct i2c_device_id *id)
>>> +{
>>> +       struct atlas_data *data;
>>> +       struct iio_buffer *buffer;
>>> +       struct iio_dev *indio_dev;
>>> +       int ret;
>>> +
>>> +       indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>>> +       if (!indio_dev)
>>> +               return -ENOMEM;
>>> +
>>> +       buffer = devm_iio_kfifo_allocate(&client->dev);
>>> +       if (!buffer)
>>> +               return -ENOMEM;
>>> +
>>> +       iio_device_attach_buffer(indio_dev, buffer);
>>> +
>>> +       indio_dev->info = &atlas_info;
>>> +       indio_dev->name = ATLAS_DRV_NAME;
>>> +       indio_dev->channels = atlas_channels;
>>> +       indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);
>>> +       indio_dev->setup_ops = &atlas_buffer_setup_ops;
>>> +
>>> +       data = iio_priv(indio_dev);
>>> +       data->client = client;
>>> +
>>> +       i2c_set_clientdata(client, indio_dev);
>>> +
>>> +       data->regmap = devm_regmap_init_i2c(client, &atlas_regmap_config);
>>> +       if (IS_ERR(data->regmap)) {
>>> +               dev_err(&client->dev, "regmap initialization failed\n");
>>> +               return PTR_ERR(data->regmap);
>>> +       }
>>> +
>>> +       ret = pm_runtime_set_active(&client->dev);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       if (client->irq <= 0) {
>>> +               dev_err(&client->dev, "no valid irq defined\n");
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       ret = devm_request_threaded_irq(&client->dev, client->irq,
>>> +                                       NULL, atlas_interrupt_handler,
>>> +                                       IRQF_TRIGGER_FALLING |
>>> IRQF_ONESHOT,
>>> +                                       "atlas_event",
>>> +                                       indio_dev);
>>> +       if (ret) {
>>> +               dev_err(&client->dev, "request irq (%d) failed\n",
>>> client->irq);
>>> +               return ret;
>>> +       }
>>> +
>>> +       pm_runtime_enable(&client->dev);
>>> +       pm_runtime_set_autosuspend_delay(&client->dev, 2500);
>>> +       pm_runtime_use_autosuspend(&client->dev);
>>> +
>>> +       ret = atlas_set_powermode(data, 0);
>>> +       if (ret)
>>> +               return ret;
>>
>> Unless I am missing something I don't think you need to explicitly
>> set power off here ^. Runtime pm will call your suspend callback in
>> some seconds anyway, so it seems redundant.
> Ah true. Would pm_runtime_mark_last_busy be required to be called here as well?
I don't think it is. You're setting a positive delay and instructing it
to use autosuspend, so by quickly glancing at the implementation in
runtime.c it would seem that runtime pm will eventually call
rpm_suspend (rpm_idle -> rpm_suspend) - which in turn calls your
registered suspend function. Don't take my word for it though,
you may want to check and experiment. (Errata at my above comment:
I now think it's right away, not in "some seconds")

>
> Although doing more thought on this.. that it should be enabling power
> up in case CONFIG_PM/runtime pm isn't enabled... since .remove() could
> power off the device on a reset.
You don't need to power up either.. if CONFIG_PM is not defined, your
rpm callbacks will not be acknowledged and won't be ever called. So
you'll start with the chip enabled, the pm stuff will have no effect
and the chip will still be enabled when you exit probe. At remove
you should power it off regardless of CONFIG_PM (Note my previous
comment in the .remove() function).
>>>
>>> +
>>> +       return iio_device_register(indio_dev);
>>> +}
>>> +
>>> +static int atlas_remove(struct i2c_client *client)
>>> +{
>>> +       struct iio_dev *indio_dev = i2c_get_clientdata(client);
>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>> +
>>> +       iio_device_unregister(indio_dev);
>>> +
>>> +       pm_runtime_disable(&client->dev);
>>> +       pm_runtime_set_suspended(&client->dev);
>>> +
>>> +       return atlas_set_powermode(data, 0);
>>
>> If you don't have CONFIG_PM defined, set_powermode will be no-op,
>> I don't think that's what you want here.
>>
>>> +}
>>> +
>>> +#ifdef CONFIG_PM
>>> +static int atlas_runtime_suspend(struct device *dev)
>>> +{
>>> +       struct atlas_data *data =
>>> +                    iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>>> +
>>> +       return atlas_set_powermode(data, 0);
>>> +}
>>> +
>>> +static int atlas_runtime_resume(struct device *dev)
>>> +{
>>> +       struct atlas_data *data =
>>> +                    iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>>> +
>>> +       return atlas_set_powermode(data, 1);
>>> +}
>>> +#endif
>>> +
>>> +static const struct dev_pm_ops atlas_pm_ops = {
>>> +       SET_RUNTIME_PM_OPS(atlas_runtime_suspend,
>>> +                          atlas_runtime_resume, NULL)
>>> +};
>>> +
>>> +static const struct i2c_device_id atlas_id[] = {
>>> +       { "atlas-ph-sm", 0 },
>>> +       {}
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, atlas_id);
>>> +
>>> +static const struct of_device_id atlas_dt_ids[] = {
>>> +       { .compatible = "atlas,ph-sm" },
>>> +       { }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, atlas_dt_ids);
>>> +
>>> +static struct i2c_driver atlas_driver = {
>>> +       .driver = {
>>> +               .name   = ATLAS_DRV_NAME,
>>> +               .of_match_table = of_match_ptr(atlas_dt_ids),
>>> +               .pm     = &atlas_pm_ops,
>>> +       },
>>> +       .probe          = atlas_probe,
>>> +       .remove         = atlas_remove,
>>> +       .id_table       = atlas_id,
>>> +};
>>> +module_i2c_driver(atlas_driver);
>>> +
>>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>>> +MODULE_DESCRIPTION("Atlas Scientific pH-SM sensor");
>>> +MODULE_LICENSE("GPL");
>>>
>>

Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support

[Matt Ranostay]

On Fri, Dec 11, 2015 at 4:41 AM, Adriana Reus <adriana.reus@intel.com> wrote:
>
>
> On 11.12.2015 06:50, Matt Ranostay wrote:
>>
>> On Thu, Dec 10, 2015 at 2:24 AM, Adriana Reus <adriana.reus@intel.com>
>> wrote:
>>>
>>> A couple of notes inline...
>>>
>>>
>>> On 10.12.2015 10:10, Matt Ranostay wrote:
>>>>
>>>>
>>>> Add support for the Atlas Scientific pH-SM chemical sensor that can
>>>> detect pH levels of solutions in the range of 0-14.
>>>>
>>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>>>> ---
>>>>    .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
>>>>    drivers/iio/chemical/Kconfig                       |  13 +
>>>>    drivers/iio/chemical/Makefile                      |   1 +
>>>>    drivers/iio/chemical/atlas-ph-sensor.c             | 385
>>>> +++++++++++++++++++++
>>>>    4 files changed, 421 insertions(+)
>>>>    create mode 100644
>>>> Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>>>    create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>>> b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>>> new file mode 100644
>>>> index 0000000..a060fd1
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>>> @@ -0,0 +1,22 @@
>>>> +* Atlas Scientific pH-SM OEM sensor
>>>> +
>>>>
>>>>
>>>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
>>>> +
>>>> +Required properties:
>>>> +
>>>> +  - compatible: must be "atlas,ph-sm"
>>>> +  - reg: the I2C address of the sensor
>>>> +  - interrupt-parent: should be the phandle for the interrupt
>>>> controller
>>>> +  - interrupts : the sole interrupt generated by the device
>>>> +
>>>> +  Refer to interrupt-controller/interrupts.txt for generic interrupt
>>>> client
>>>> +  node bindings.
>>>> +
>>>> +Example:
>>>> +
>>>> +atlas@65 {
>>>> +       compatible = "atlas,ph-sm";
>>>> +       reg = <0x65>;
>>>> +       interrupt-parent = <&gpio1>;
>>>> +       interrupts = <16 2>;
>>>> +};
>>>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
>>>> index 3061b72..753eab6 100644
>>>> --- a/drivers/iio/chemical/Kconfig
>>>> +++ b/drivers/iio/chemical/Kconfig
>>>> @@ -4,6 +4,19 @@
>>>>
>>>>    menu "Chemical Sensors"
>>>>
>>>> +config ATLAS_PH_SENSOR
>>>> +       tristate "Atlas Scientific OEM pH-SM sensor"
>>>> +       depends on I2C
>>>> +       select REGMAP_I2C
>>>> +       select IIO_BUFFER
>>>> +       select IIO_KFIFO_BUF
>>>> +       help
>>>> +        Say Y here to build I2C interface support for the Atlas
>>>> +        Scientific OEM pH-SM sensor.
>>>> +
>>>> +        To compile this driver as module, choose M here: the
>>>> +        module will be called atlas-ph-sensor
>>>> +
>>>>    config VZ89X
>>>>          tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>>>>          depends on I2C
>>>> diff --git a/drivers/iio/chemical/Makefile
>>>> b/drivers/iio/chemical/Makefile
>>>> index 7292f2d..e9eb852 100644
>>>> --- a/drivers/iio/chemical/Makefile
>>>> +++ b/drivers/iio/chemical/Makefile
>>>> @@ -3,4 +3,5 @@
>>>>    #
>>>>
>>>>    # When adding new entries keep the list in alphabetical order
>>>> +obj-$(CONFIG_ATLAS_PH_SENSOR)  += atlas-ph-sensor.o
>>>>    obj-$(CONFIG_VZ89X)           += vz89x.o
>>>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c
>>>> b/drivers/iio/chemical/atlas-ph-sensor.c
>>>> new file mode 100644
>>>> index 0000000..6c6f261
>>>> --- /dev/null
>>>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
>>>> @@ -0,0 +1,385 @@
>>>> +/*
>>>> + * atlas-ph-sensor.c - Support for Atlas Scientific OEM pH-SM sensor
>>>> + *
>>>> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License as published by
>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>> + * (at your option) any later version.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/interrupt.h>
>>>> +#include <linux/delay.h>
>>>> +#include <linux/mutex.h>
>>>> +#include <linux/err.h>
>>>> +#include <linux/irq.h>
>>>> +#include <linux/gpio.h>
>>>> +#include <linux/i2c.h>
>>>> +#include <linux/regmap.h>
>>>> +#include <linux/iio/iio.h>
>>>> +#include <linux/iio/buffer.h>
>>>> +#include <linux/iio/kfifo_buf.h>
>>>> +#include <linux/pm_runtime.h>
>>>> +
>>>> +#define ATLAS_REGMAP_NAME      "atlas_ph_regmap"
>>>> +#define        ATLAS_DRV_NAME          "atlas_ph"
>>>> +
>>>> +#define ATLAS_REG_DEV_TYPE             0x00
>>>> +#define ATLAS_REG_DEV_VERSION          0x01
>>>> +
>>>> +#define ATLAS_REG_INT_CONTROL          0x04
>>>> +#define ATLAS_REG_INT_CONTROL_EN       BIT(2)
>>>> +
>>>> +#define ATLAS_REG_PWR_CONTROL          0x06
>>>> +
>>>> +#define ATLAS_REG_TEMP_DATA            0x0e
>>>> +#define ATLAS_REG_PH_DATA              0x16
>>>> +
>>>> +#define ATLAS_PH_INT_TIME_IN_US                450000
>>>> +
>>>> +struct atlas_data {
>>>> +       struct i2c_client *client;
>>>> +       struct regmap *regmap;
>>>> +
>>>> +       __be32 buffer[3]; /* 32-bit pH data + 64-bit timestamp */
>>>> +};
>>>> +
>>>> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int
>>>> reg)
>>>> +{
>>>> +       if (reg == ATLAS_REG_INT_CONTROL)
>>>> +               return true;
>>>> +
>>>> +       return false;
>>>> +}
>>>> +
>>>> +static const struct regmap_config atlas_regmap_config = {
>>>> +       .name = ATLAS_REGMAP_NAME,
>>>> +
>>>> +       .reg_bits = 8,
>>>> +       .val_bits = 8,
>>>> +
>>>> +       .volatile_reg = max30100_is_volatile_reg,
>>>> +       .max_register = ATLAS_REG_PH_DATA + 4,
>>>> +       .cache_type = REGCACHE_FLAT,
>>>> +
>>>> +};
>>>> +
>>>> +static const struct iio_chan_spec atlas_channels[] = {
>>>> +       {
>>>> +               .type = IIO_PH,
>>>> +               .info_mask_separate =
>>>> +                       BIT(IIO_CHAN_INFO_RAW) |
>>>> BIT(IIO_CHAN_INFO_SCALE),
>>>> +
>>>> +               .scan_index = 0,
>>>> +               .scan_type = {
>>>> +                       .sign = 'u',
>>>> +                       .realbits = 32,
>>>> +                       .storagebits = 32,
>>>> +                       .endianness = IIO_BE,
>>>> +               },
>>>> +       },
>>>> +       {
>>>> +               .type = IIO_TEMP,
>>>> +               .address = ATLAS_REG_TEMP_DATA,
>>>> +               .info_mask_separate =
>>>> +                       BIT(IIO_CHAN_INFO_RAW) |
>>>> BIT(IIO_CHAN_INFO_SCALE),
>>>> +
>>>> +               .output = 1,
>>>> +               .scan_index = -1
>>>> +       },
>>>> +};
>>>> +
>>>> +#ifdef CONFIG_PM
>>>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>>>> +{
>>>> +       return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on);
>>>> +}
>>>> +#else
>>>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>>>> +{
>>>> +       return 0;
>>>> +}
>>>> +#endif
>>>
>>>
>>> ^ Why does this need to depend on CONFIG_PM? You may want to use it
>>> if CONFIG_PM is not defined also (See my comment in the remove
>>> funcion).
>>>
>>>> +
>>>> +static int atlas_set_interrupt(struct atlas_data *data, bool state)
>>>> +{
>>>> +       return regmap_update_bits(data->regmap, ATLAS_REG_INT_CONTROL,
>>>> +                                 ATLAS_REG_INT_CONTROL_EN,
>>>> +                                 state ? ATLAS_REG_INT_CONTROL_EN : 0);
>>>> +}
>>>> +
>>>> +static int atlas_buffer_postenable(struct iio_dev *indio_dev)
>>>> +{
>>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>>> +
>>>> +       pm_runtime_get_sync(&data->client->dev);
>>>> +
>>>> +       return atlas_set_interrupt(data, true);
>>>> +}
>>>> +
>>>> +static int atlas_buffer_predisable(struct iio_dev *indio_dev)
>>>> +{
>>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>>> +       int ret;
>>>> +
>>>> +       ret = atlas_set_interrupt(data, false);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       pm_runtime_mark_last_busy(&data->client->dev);
>>>> +       return pm_runtime_put_autosuspend(&data->client->dev);
>>>> +}
>>>> +
>>>> +static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
>>>> +       .postenable = atlas_buffer_postenable,
>>>> +       .predisable = atlas_buffer_predisable,
>>>> +};
>>>> +
>>>> +static irqreturn_t atlas_interrupt_handler(int irq, void *private)
>>>> +{
>>>> +       struct iio_dev *indio_dev = private;
>>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>>> +       int ret;
>>>> +
>>>> +       ret = i2c_smbus_read_i2c_block_data(data->client,
>>>> ATLAS_REG_PH_DATA,
>>>> +                               sizeof(data->buffer[0]), (u8 *)
>>>> &data->buffer);
>>>> +       if (!ret)
>>>> +               iio_push_to_buffers_with_timestamp(indio_dev,
>>>> data->buffer,
>>>> +                               iio_get_time_ns());
>>>> +
>>>> +       /* re-enable interrupt */
>>>> +       atlas_set_interrupt(data, true);
>>>> +
>>>> +       return IRQ_HANDLED;
>>>> +}
>>>> +
>>>> +static int atlas_read_ph_measurement(struct atlas_data *data, __be32
>>>> *val)
>>>> +{
>>>> +       struct device *dev = &data->client->dev;
>>>> +       int suspended = pm_runtime_suspended(dev);
>>>> +       int ret;
>>>> +
>>>> +       ret = pm_runtime_get_sync(dev);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       if (suspended)
>>>> +               usleep_range(ATLAS_PH_INT_TIME_IN_US,
>>>> +                            ATLAS_PH_INT_TIME_IN_US + 100000);
>>>> +
>>>> +       ret = i2c_smbus_read_i2c_block_data(data->client,
>>>> ATLAS_REG_PH_DATA,
>>>> +                                           sizeof(*val), (u8 *) val);
>>>> +
>>>> +       pm_runtime_mark_last_busy(dev);
>>>> +       pm_runtime_put_autosuspend(dev);
>>>> +
>>>> +       return ret;
>>>> +}
>>>> +
>>>> +static int atlas_read_raw(struct iio_dev *indio_dev,
>>>> +                         struct iio_chan_spec const *chan,
>>>> +                         int *val, int *val2, long mask)
>>>> +{
>>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>>> +
>>>> +       switch (mask) {
>>>> +       case IIO_CHAN_INFO_RAW: {
>>>> +               int ret;
>>>> +               __be32 reg;
>>>> +
>>>> +               mutex_lock(&indio_dev->mlock);
>>>> +
>>>> +               if (iio_buffer_enabled(indio_dev)) {
>>>> +                       ret = -EINVAL;
>>>> +                       goto error_busy;
>>>> +               }
>>>> +
>>>> +               switch (chan->type) {
>>>> +               case IIO_TEMP:
>>>> +                       ret = regmap_bulk_read(data->regmap,
>>>> chan->address,
>>>> +                                             (u8 *) &reg, sizeof(reg));
>>>> +                       break;
>>>> +               case IIO_PH:
>>>> +                       ret = atlas_read_ph_measurement(data, &reg);
>>>> +                       break;
>>>> +               default:
>>>> +                       ret = -EINVAL;
>>>> +               }
>>>> +
>>>> +               if (!ret) {
>>>> +                       *val = be32_to_cpu(reg);
>>>> +                       ret = IIO_VAL_INT;
>>>> +               }
>>>> +
>>>> +error_busy:
>>>> +               mutex_unlock(&indio_dev->mlock);
>>>> +               return ret;
>>>> +       }
>>>> +       case IIO_CHAN_INFO_SCALE:
>>>> +               *val = 0; /* 0.001 */
>>>> +               *val2 = 1000;
>>>> +               return IIO_VAL_INT_PLUS_MICRO;
>>>> +       }
>>>> +
>>>> +       return -EINVAL;
>>>> +}
>>>> +
>>>> +static int atlas_write_raw(struct iio_dev *indio_dev,
>>>> +                          struct iio_chan_spec const *chan,
>>>> +                          int val, int val2, long mask)
>>>> +{
>>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>>> +       __be32 reg = cpu_to_be32(val);
>>>> +
>>>> +       if (val2 != 0)
>>>> +               return -EINVAL;
>>>> +
>>>> +       if (mask != IIO_CHAN_INFO_RAW || chan->type != IIO_TEMP)
>>>> +               return -EINVAL;
>>>> +
>>>> +       return regmap_bulk_write(data->regmap, chan->address, &reg, 4);
>>>> +}
>>>> +
>>>> +static const struct iio_info atlas_info = {
>>>> +       .driver_module = THIS_MODULE,
>>>> +       .read_raw = atlas_read_raw,
>>>> +       .write_raw = atlas_write_raw,
>>>> +};
>>>> +
>>>> +static int atlas_probe(struct i2c_client *client,
>>>> +                      const struct i2c_device_id *id)
>>>> +{
>>>> +       struct atlas_data *data;
>>>> +       struct iio_buffer *buffer;
>>>> +       struct iio_dev *indio_dev;
>>>> +       int ret;
>>>> +
>>>> +       indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>>>> +       if (!indio_dev)
>>>> +               return -ENOMEM;
>>>> +
>>>> +       buffer = devm_iio_kfifo_allocate(&client->dev);
>>>> +       if (!buffer)
>>>> +               return -ENOMEM;
>>>> +
>>>> +       iio_device_attach_buffer(indio_dev, buffer);
>>>> +
>>>> +       indio_dev->info = &atlas_info;
>>>> +       indio_dev->name = ATLAS_DRV_NAME;
>>>> +       indio_dev->channels = atlas_channels;
>>>> +       indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);
>>>> +       indio_dev->setup_ops = &atlas_buffer_setup_ops;
>>>> +
>>>> +       data = iio_priv(indio_dev);
>>>> +       data->client = client;
>>>> +
>>>> +       i2c_set_clientdata(client, indio_dev);
>>>> +
>>>> +       data->regmap = devm_regmap_init_i2c(client,
>>>> &atlas_regmap_config);
>>>> +       if (IS_ERR(data->regmap)) {
>>>> +               dev_err(&client->dev, "regmap initialization failed\n");
>>>> +               return PTR_ERR(data->regmap);
>>>> +       }
>>>> +
>>>> +       ret = pm_runtime_set_active(&client->dev);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       if (client->irq <= 0) {
>>>> +               dev_err(&client->dev, "no valid irq defined\n");
>>>> +               return -EINVAL;
>>>> +       }
>>>> +
>>>> +       ret = devm_request_threaded_irq(&client->dev, client->irq,
>>>> +                                       NULL, atlas_interrupt_handler,
>>>> +                                       IRQF_TRIGGER_FALLING |
>>>> IRQF_ONESHOT,
>>>> +                                       "atlas_event",
>>>> +                                       indio_dev);
>>>> +       if (ret) {
>>>> +               dev_err(&client->dev, "request irq (%d) failed\n",
>>>> client->irq);
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       pm_runtime_enable(&client->dev);
>>>> +       pm_runtime_set_autosuspend_delay(&client->dev, 2500);
>>>> +       pm_runtime_use_autosuspend(&client->dev);
>>>> +
>>>> +       ret = atlas_set_powermode(data, 0);
>>>> +       if (ret)
>>>> +               return ret;
>>>
>>>
>>> Unless I am missing something I don't think you need to explicitly
>>> set power off here ^. Runtime pm will call your suspend callback in
>>> some seconds anyway, so it seems redundant.
>>
>> Ah true. Would pm_runtime_mark_last_busy be required to be called here as
>> well?
>
> I don't think it is. You're setting a positive delay and instructing it
> to use autosuspend, so by quickly glancing at the implementation in
> runtime.c it would seem that runtime pm will eventually call
> rpm_suspend (rpm_idle -> rpm_suspend) - which in turn calls your
> registered suspend function. Don't take my word for it though,
> you may want to check and experiment. (Errata at my above comment:
> I now think it's right away, not in "some seconds")
>
>>
>> Although doing more thought on this.. that it should be enabling power
>> up in case CONFIG_PM/runtime pm isn't enabled... since .remove() could
>> power off the device on a reset.
>
> You don't need to power up either.. if CONFIG_PM is not defined, your
> rpm callbacks will not be acknowledged and won't be ever called. So
> you'll start with the chip enabled, the pm stuff will have no effect
> and the chip will still be enabled when you exit probe. At remove
> you should power it off regardless of CONFIG_PM (Note my previous
> comment in the .remove() function).
>

But if CONFIG_PM is disabled wouldn't the corner case of loading the
module, unloading, and re-loading cause a device that is powered off?

>>>>
>>>> +
>>>> +       return iio_device_register(indio_dev);
>>>> +}
>>>> +
>>>> +static int atlas_remove(struct i2c_client *client)
>>>> +{
>>>> +       struct iio_dev *indio_dev = i2c_get_clientdata(client);
>>>> +       struct atlas_data *data = iio_priv(indio_dev);
>>>> +
>>>> +       iio_device_unregister(indio_dev);
>>>> +
>>>> +       pm_runtime_disable(&client->dev);
>>>> +       pm_runtime_set_suspended(&client->dev);
>>>> +
>>>> +       return atlas_set_powermode(data, 0);
>>>
>>>
>>> If you don't have CONFIG_PM defined, set_powermode will be no-op,
>>> I don't think that's what you want here.
>>>
>>>> +}
>>>> +
>>>> +#ifdef CONFIG_PM
>>>> +static int atlas_runtime_suspend(struct device *dev)
>>>> +{
>>>> +       struct atlas_data *data =
>>>> +                    iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>>>> +
>>>> +       return atlas_set_powermode(data, 0);
>>>> +}
>>>> +
>>>> +static int atlas_runtime_resume(struct device *dev)
>>>> +{
>>>> +       struct atlas_data *data =
>>>> +                    iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>>>> +
>>>> +       return atlas_set_powermode(data, 1);
>>>> +}
>>>> +#endif
>>>> +
>>>> +static const struct dev_pm_ops atlas_pm_ops = {
>>>> +       SET_RUNTIME_PM_OPS(atlas_runtime_suspend,
>>>> +                          atlas_runtime_resume, NULL)
>>>> +};
>>>> +
>>>> +static const struct i2c_device_id atlas_id[] = {
>>>> +       { "atlas-ph-sm", 0 },
>>>> +       {}
>>>> +};
>>>> +MODULE_DEVICE_TABLE(i2c, atlas_id);
>>>> +
>>>> +static const struct of_device_id atlas_dt_ids[] = {
>>>> +       { .compatible = "atlas,ph-sm" },
>>>> +       { }
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, atlas_dt_ids);
>>>> +
>>>> +static struct i2c_driver atlas_driver = {
>>>> +       .driver = {
>>>> +               .name   = ATLAS_DRV_NAME,
>>>> +               .of_match_table = of_match_ptr(atlas_dt_ids),
>>>> +               .pm     = &atlas_pm_ops,
>>>> +       },
>>>> +       .probe          = atlas_probe,
>>>> +       .remove         = atlas_remove,
>>>> +       .id_table       = atlas_id,
>>>> +};
>>>> +module_i2c_driver(atlas_driver);
>>>> +
>>>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>>>> +MODULE_DESCRIPTION("Atlas Scientific pH-SM sensor");
>>>> +MODULE_LICENSE("GPL");
>>>>
>>>
>

Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support

[Jonathan Cameron]

On 10/12/15 08:10, Matt Ranostay wrote:
> Add support for the Atlas Scientific pH-SM chemical sensor that can
> detect pH levels of solutions in the range of 0-14.
> 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
This looks like it has a fairly standard dataready interrupt and no
hardware buffering.  Hence it should be using a trigger to allow other
sensors to be captured synced with it.  Skipping a trigger is fine
if there is a hardware buffer involved or some other reason why the trigger
is less than meaningful.  Here I think you should have one as it expands
possible usecases and makes things more flexible.

Interestingly I can't find any reference in the (admittedly very pretty)
datasheet to how fast thing this runs.

Otherwise, looks like you got some thorough reviews already so I'll
take a proper look at a new version.

Jonathan
> ---
>  .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
>  drivers/iio/chemical/Kconfig                       |  13 +
>  drivers/iio/chemical/Makefile                      |   1 +
>  drivers/iio/chemical/atlas-ph-sensor.c             | 385 +++++++++++++++++++++
>  4 files changed, 421 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>  create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
> new file mode 100644
> index 0000000..a060fd1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
> @@ -0,0 +1,22 @@
> +* Atlas Scientific pH-SM OEM sensor
> +
> +http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
> +
> +Required properties:
> +
> +  - compatible: must be "atlas,ph-sm"
> +  - reg: the I2C address of the sensor
> +  - interrupt-parent: should be the phandle for the interrupt controller
> +  - interrupts : the sole interrupt generated by the device
> +
> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
> +  node bindings.
> +
> +Example:
> +
> +atlas@65 {
> +	compatible = "atlas,ph-sm";
> +	reg = <0x65>;
> +	interrupt-parent = <&gpio1>;
> +	interrupts = <16 2>;
> +};
> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> index 3061b72..753eab6 100644
> --- a/drivers/iio/chemical/Kconfig
> +++ b/drivers/iio/chemical/Kconfig
> @@ -4,6 +4,19 @@
>  
>  menu "Chemical Sensors"
>  
> +config ATLAS_PH_SENSOR
> +	tristate "Atlas Scientific OEM pH-SM sensor"
> +	depends on I2C
> +	select REGMAP_I2C
> +	select IIO_BUFFER
> +	select IIO_KFIFO_BUF
> +	help
> +	 Say Y here to build I2C interface support for the Atlas
> +	 Scientific OEM pH-SM sensor.
> +
> +	 To compile this driver as module, choose M here: the
> +	 module will be called atlas-ph-sensor
> +
>  config VZ89X
>  	tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>  	depends on I2C
> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
> index 7292f2d..e9eb852 100644
> --- a/drivers/iio/chemical/Makefile
> +++ b/drivers/iio/chemical/Makefile
> @@ -3,4 +3,5 @@
>  #
>  
>  # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_ATLAS_PH_SENSOR)	+= atlas-ph-sensor.o
>  obj-$(CONFIG_VZ89X)		+= vz89x.o
> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
> new file mode 100644
> index 0000000..6c6f261
> --- /dev/null
> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
> @@ -0,0 +1,385 @@
> +/*
> + * atlas-ph-sensor.c - Support for Atlas Scientific OEM pH-SM sensor
> + *
> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/irq.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <linux/pm_runtime.h>
> +
> +#define ATLAS_REGMAP_NAME	"atlas_ph_regmap"
> +#define	ATLAS_DRV_NAME		"atlas_ph"
> +
> +#define ATLAS_REG_DEV_TYPE		0x00
> +#define ATLAS_REG_DEV_VERSION		0x01
> +
> +#define ATLAS_REG_INT_CONTROL		0x04
> +#define ATLAS_REG_INT_CONTROL_EN	BIT(2)
> +
> +#define ATLAS_REG_PWR_CONTROL		0x06
> +
> +#define ATLAS_REG_TEMP_DATA		0x0e
> +#define ATLAS_REG_PH_DATA		0x16
> +
> +#define ATLAS_PH_INT_TIME_IN_US		450000
> +
> +struct atlas_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +
> +	__be32 buffer[3]; /* 32-bit pH data + 64-bit timestamp */
> +};
> +
> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	if (reg == ATLAS_REG_INT_CONTROL)
> +		return true;
> +
> +	return false;
> +}
> +
> +static const struct regmap_config atlas_regmap_config = {
> +	.name = ATLAS_REGMAP_NAME,
> +
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.volatile_reg = max30100_is_volatile_reg,
> +	.max_register = ATLAS_REG_PH_DATA + 4,
> +	.cache_type = REGCACHE_FLAT,
> +
> +};
> +
> +static const struct iio_chan_spec atlas_channels[] = {
> +	{
> +		.type = IIO_PH,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 32,
> +			.storagebits = 32,
> +			.endianness = IIO_BE,
> +		},
> +	},
> +	{
> +		.type = IIO_TEMP,
> +		.address = ATLAS_REG_TEMP_DATA,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +
> +		.output = 1,
> +		.scan_index = -1
> +	},
> +};
> +
> +#ifdef CONFIG_PM
> +static int atlas_set_powermode(struct atlas_data *data, int on)
> +{
> +	return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on);
> +}
> +#else
> +static int atlas_set_powermode(struct atlas_data *data, int on)
> +{
> +	return 0;
> +}
> +#endif
> +
> +static int atlas_set_interrupt(struct atlas_data *data, bool state)
> +{
> +	return regmap_update_bits(data->regmap, ATLAS_REG_INT_CONTROL,
> +				  ATLAS_REG_INT_CONTROL_EN,
> +				  state ? ATLAS_REG_INT_CONTROL_EN : 0);
> +}
> +
> +static int atlas_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +
> +	pm_runtime_get_sync(&data->client->dev);
> +
> +	return atlas_set_interrupt(data, true);
> +}
> +
> +static int atlas_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = atlas_set_interrupt(data, false);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_mark_last_busy(&data->client->dev);
> +	return pm_runtime_put_autosuspend(&data->client->dev);
> +}
> +
> +static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
> +	.postenable = atlas_buffer_postenable,
> +	.predisable = atlas_buffer_predisable,
> +};
> +
> +static irqreturn_t atlas_interrupt_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct atlas_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
> +				sizeof(data->buffer[0]), (u8 *) &data->buffer);
> +	if (!ret)
> +		iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> +				iio_get_time_ns());
> +
> +	/* re-enable interrupt */
> +	atlas_set_interrupt(data, true);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val)
> +{
> +	struct device *dev = &data->client->dev;
> +	int suspended = pm_runtime_suspended(dev);
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret)
> +		return ret;
> +
> +	if (suspended)
> +		usleep_range(ATLAS_PH_INT_TIME_IN_US,
> +			     ATLAS_PH_INT_TIME_IN_US + 100000);
> +
> +	ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
> +					    sizeof(*val), (u8 *) val);
> +
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
> +
> +	return ret;
> +}
> +
> +static int atlas_read_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan,
> +			  int *val, int *val2, long mask)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW: {
> +		int ret;
> +		__be32 reg;
> +
> +		mutex_lock(&indio_dev->mlock);
> +
> +		if (iio_buffer_enabled(indio_dev)) {
> +			ret = -EINVAL;
> +			goto error_busy;
> +		}
> +
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			ret = regmap_bulk_read(data->regmap, chan->address,
> +					      (u8 *) &reg, sizeof(reg));
> +			break;
> +		case IIO_PH:
> +			ret = atlas_read_ph_measurement(data, &reg);
> +			break;
> +		default:
> +			ret = -EINVAL;
> +		}
> +
> +		if (!ret) {
> +			*val = be32_to_cpu(reg);
> +			ret = IIO_VAL_INT;
> +		}
> +
> +error_busy:
> +		mutex_unlock(&indio_dev->mlock);
> +		return ret;
> +	}
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0; /* 0.001 */
> +		*val2 = 1000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int atlas_write_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int val, int val2, long mask)
> +{
> +	struct atlas_data *data = iio_priv(indio_dev);
> +	__be32 reg = cpu_to_be32(val);
> +
> +	if (val2 != 0)
> +		return -EINVAL;
> +
> +	if (mask != IIO_CHAN_INFO_RAW || chan->type != IIO_TEMP)
> +		return -EINVAL;
> +
> +	return regmap_bulk_write(data->regmap, chan->address, &reg, 4);
> +}
> +
> +static const struct iio_info atlas_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = atlas_read_raw,
> +	.write_raw = atlas_write_raw,
> +};
> +
> +static int atlas_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id)
> +{
> +	struct atlas_data *data;
> +	struct iio_buffer *buffer;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	buffer = devm_iio_kfifo_allocate(&client->dev);
> +	if (!buffer)
> +		return -ENOMEM;
> +
> +	iio_device_attach_buffer(indio_dev, buffer);
> +
> +	indio_dev->info = &atlas_info;
> +	indio_dev->name = ATLAS_DRV_NAME;
> +	indio_dev->channels = atlas_channels;
> +	indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);
> +	indio_dev->setup_ops = &atlas_buffer_setup_ops;
> +
> +	data = iio_priv(indio_dev);
> +	data->client = client;
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	data->regmap = devm_regmap_init_i2c(client, &atlas_regmap_config);
> +	if (IS_ERR(data->regmap)) {
> +		dev_err(&client->dev, "regmap initialization failed\n");
> +		return PTR_ERR(data->regmap);
> +	}
> +
> +	ret = pm_runtime_set_active(&client->dev);
> +	if (ret)
> +		return ret;
> +
> +	if (client->irq <= 0) {
> +		dev_err(&client->dev, "no valid irq defined\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = devm_request_threaded_irq(&client->dev, client->irq,
> +					NULL, atlas_interrupt_handler,
> +					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +					"atlas_event",
> +					indio_dev);
> +	if (ret) {
> +		dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
> +		return ret;
> +	}
> +
> +	pm_runtime_enable(&client->dev);
> +	pm_runtime_set_autosuspend_delay(&client->dev, 2500);
> +	pm_runtime_use_autosuspend(&client->dev);
> +
> +	ret = atlas_set_powermode(data, 0);
> +	if (ret)
> +		return ret;
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static int atlas_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct atlas_data *data = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	pm_runtime_disable(&client->dev);
> +	pm_runtime_set_suspended(&client->dev);
> +
> +	return atlas_set_powermode(data, 0);
> +}
> +
> +#ifdef CONFIG_PM
> +static int atlas_runtime_suspend(struct device *dev)
> +{
> +	struct atlas_data *data =
> +		     iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return atlas_set_powermode(data, 0);
> +}
> +
> +static int atlas_runtime_resume(struct device *dev)
> +{
> +	struct atlas_data *data =
> +		     iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
> +
> +	return atlas_set_powermode(data, 1);
> +}
> +#endif
> +
> +static const struct dev_pm_ops atlas_pm_ops = {
> +	SET_RUNTIME_PM_OPS(atlas_runtime_suspend,
> +			   atlas_runtime_resume, NULL)
> +};
> +
> +static const struct i2c_device_id atlas_id[] = {
> +	{ "atlas-ph-sm", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, atlas_id);
> +
> +static const struct of_device_id atlas_dt_ids[] = {
> +	{ .compatible = "atlas,ph-sm" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, atlas_dt_ids);
> +
> +static struct i2c_driver atlas_driver = {
> +	.driver = {
> +		.name	= ATLAS_DRV_NAME,
> +		.of_match_table	= of_match_ptr(atlas_dt_ids),
> +		.pm	= &atlas_pm_ops,
> +	},
> +	.probe		= atlas_probe,
> +	.remove		= atlas_remove,
> +	.id_table	= atlas_id,
> +};
> +module_i2c_driver(atlas_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("Atlas Scientific pH-SM sensor");
> +MODULE_LICENSE("GPL");
> 

Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support

[Matt Ranostay]

On Sat, Dec 12, 2015 at 4:06 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 10/12/15 08:10, Matt Ranostay wrote:
>> Add support for the Atlas Scientific pH-SM chemical sensor that can
>> detect pH levels of solutions in the range of 0-14.
>>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> This looks like it has a fairly standard dataready interrupt and no
> hardware buffering.  Hence it should be using a trigger to allow other
> sensors to be captured synced with it.  Skipping a trigger is fine
> if there is a hardware buffer involved or some other reason why the trigger
> is less than meaningful.  Here I think you should have one as it expands
> possible usecases and makes things more flexible.
>
> Interestingly I can't find any reference in the (admittedly very pretty)
> datasheet to how fast thing this runs.

It is in there.. albeit hidden. It should update the samples every 420
milliseconds.

>
> Otherwise, looks like you got some thorough reviews already so I'll
> take a proper look at a new version.
>
> Jonathan
>> ---
>>  .../bindings/iio/chemical/atlas,ph-sm.txt          |  22 ++
>>  drivers/iio/chemical/Kconfig                       |  13 +
>>  drivers/iio/chemical/Makefile                      |   1 +
>>  drivers/iio/chemical/atlas-ph-sensor.c             | 385 +++++++++++++++++++++
>>  4 files changed, 421 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>>  create mode 100644 drivers/iio/chemical/atlas-ph-sensor.c
>>
>> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>> new file mode 100644
>> index 0000000..a060fd1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>> @@ -0,0 +1,22 @@
>> +* Atlas Scientific pH-SM OEM sensor
>> +
>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
>> +
>> +Required properties:
>> +
>> +  - compatible: must be "atlas,ph-sm"
>> +  - reg: the I2C address of the sensor
>> +  - interrupt-parent: should be the phandle for the interrupt controller
>> +  - interrupts : the sole interrupt generated by the device
>> +
>> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
>> +  node bindings.
>> +
>> +Example:
>> +
>> +atlas@65 {
>> +     compatible = "atlas,ph-sm";
>> +     reg = <0x65>;
>> +     interrupt-parent = <&gpio1>;
>> +     interrupts = <16 2>;
>> +};
>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
>> index 3061b72..753eab6 100644
>> --- a/drivers/iio/chemical/Kconfig
>> +++ b/drivers/iio/chemical/Kconfig
>> @@ -4,6 +4,19 @@
>>
>>  menu "Chemical Sensors"
>>
>> +config ATLAS_PH_SENSOR
>> +     tristate "Atlas Scientific OEM pH-SM sensor"
>> +     depends on I2C
>> +     select REGMAP_I2C
>> +     select IIO_BUFFER
>> +     select IIO_KFIFO_BUF
>> +     help
>> +      Say Y here to build I2C interface support for the Atlas
>> +      Scientific OEM pH-SM sensor.
>> +
>> +      To compile this driver as module, choose M here: the
>> +      module will be called atlas-ph-sensor
>> +
>>  config VZ89X
>>       tristate "SGX Sensortech MiCS VZ89X VOC sensor"
>>       depends on I2C
>> diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
>> index 7292f2d..e9eb852 100644
>> --- a/drivers/iio/chemical/Makefile
>> +++ b/drivers/iio/chemical/Makefile
>> @@ -3,4 +3,5 @@
>>  #
>>
>>  # When adding new entries keep the list in alphabetical order
>> +obj-$(CONFIG_ATLAS_PH_SENSOR)        += atlas-ph-sensor.o
>>  obj-$(CONFIG_VZ89X)          += vz89x.o
>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
>> new file mode 100644
>> index 0000000..6c6f261
>> --- /dev/null
>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
>> @@ -0,0 +1,385 @@
>> +/*
>> + * atlas-ph-sensor.c - Support for Atlas Scientific OEM pH-SM sensor
>> + *
>> + * Copyright (C) 2015 Matt Ranostay <mranostay@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/delay.h>
>> +#include <linux/mutex.h>
>> +#include <linux/err.h>
>> +#include <linux/irq.h>
>> +#include <linux/gpio.h>
>> +#include <linux/i2c.h>
>> +#include <linux/regmap.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/kfifo_buf.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#define ATLAS_REGMAP_NAME    "atlas_ph_regmap"
>> +#define      ATLAS_DRV_NAME          "atlas_ph"
>> +
>> +#define ATLAS_REG_DEV_TYPE           0x00
>> +#define ATLAS_REG_DEV_VERSION                0x01
>> +
>> +#define ATLAS_REG_INT_CONTROL                0x04
>> +#define ATLAS_REG_INT_CONTROL_EN     BIT(2)
>> +
>> +#define ATLAS_REG_PWR_CONTROL                0x06
>> +
>> +#define ATLAS_REG_TEMP_DATA          0x0e
>> +#define ATLAS_REG_PH_DATA            0x16
>> +
>> +#define ATLAS_PH_INT_TIME_IN_US              450000
>> +
>> +struct atlas_data {
>> +     struct i2c_client *client;
>> +     struct regmap *regmap;
>> +
>> +     __be32 buffer[3]; /* 32-bit pH data + 64-bit timestamp */
>> +};
>> +
>> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg)
>> +{
>> +     if (reg == ATLAS_REG_INT_CONTROL)
>> +             return true;
>> +
>> +     return false;
>> +}
>> +
>> +static const struct regmap_config atlas_regmap_config = {
>> +     .name = ATLAS_REGMAP_NAME,
>> +
>> +     .reg_bits = 8,
>> +     .val_bits = 8,
>> +
>> +     .volatile_reg = max30100_is_volatile_reg,
>> +     .max_register = ATLAS_REG_PH_DATA + 4,
>> +     .cache_type = REGCACHE_FLAT,
>> +
>> +};
>> +
>> +static const struct iio_chan_spec atlas_channels[] = {
>> +     {
>> +             .type = IIO_PH,
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +
>> +             .scan_index = 0,
>> +             .scan_type = {
>> +                     .sign = 'u',
>> +                     .realbits = 32,
>> +                     .storagebits = 32,
>> +                     .endianness = IIO_BE,
>> +             },
>> +     },
>> +     {
>> +             .type = IIO_TEMP,
>> +             .address = ATLAS_REG_TEMP_DATA,
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +
>> +             .output = 1,
>> +             .scan_index = -1
>> +     },
>> +};
>> +
>> +#ifdef CONFIG_PM
>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>> +{
>> +     return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on);
>> +}
>> +#else
>> +static int atlas_set_powermode(struct atlas_data *data, int on)
>> +{
>> +     return 0;
>> +}
>> +#endif
>> +
>> +static int atlas_set_interrupt(struct atlas_data *data, bool state)
>> +{
>> +     return regmap_update_bits(data->regmap, ATLAS_REG_INT_CONTROL,
>> +                               ATLAS_REG_INT_CONTROL_EN,
>> +                               state ? ATLAS_REG_INT_CONTROL_EN : 0);
>> +}
>> +
>> +static int atlas_buffer_postenable(struct iio_dev *indio_dev)
>> +{
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +
>> +     pm_runtime_get_sync(&data->client->dev);
>> +
>> +     return atlas_set_interrupt(data, true);
>> +}
>> +
>> +static int atlas_buffer_predisable(struct iio_dev *indio_dev)
>> +{
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +     int ret;
>> +
>> +     ret = atlas_set_interrupt(data, false);
>> +     if (ret)
>> +             return ret;
>> +
>> +     pm_runtime_mark_last_busy(&data->client->dev);
>> +     return pm_runtime_put_autosuspend(&data->client->dev);
>> +}
>> +
>> +static const struct iio_buffer_setup_ops atlas_buffer_setup_ops = {
>> +     .postenable = atlas_buffer_postenable,
>> +     .predisable = atlas_buffer_predisable,
>> +};
>> +
>> +static irqreturn_t atlas_interrupt_handler(int irq, void *private)
>> +{
>> +     struct iio_dev *indio_dev = private;
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +     int ret;
>> +
>> +     ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
>> +                             sizeof(data->buffer[0]), (u8 *) &data->buffer);
>> +     if (!ret)
>> +             iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>> +                             iio_get_time_ns());
>> +
>> +     /* re-enable interrupt */
>> +     atlas_set_interrupt(data, true);
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val)
>> +{
>> +     struct device *dev = &data->client->dev;
>> +     int suspended = pm_runtime_suspended(dev);
>> +     int ret;
>> +
>> +     ret = pm_runtime_get_sync(dev);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (suspended)
>> +             usleep_range(ATLAS_PH_INT_TIME_IN_US,
>> +                          ATLAS_PH_INT_TIME_IN_US + 100000);
>> +
>> +     ret = i2c_smbus_read_i2c_block_data(data->client, ATLAS_REG_PH_DATA,
>> +                                         sizeof(*val), (u8 *) val);
>> +
>> +     pm_runtime_mark_last_busy(dev);
>> +     pm_runtime_put_autosuspend(dev);
>> +
>> +     return ret;
>> +}
>> +
>> +static int atlas_read_raw(struct iio_dev *indio_dev,
>> +                       struct iio_chan_spec const *chan,
>> +                       int *val, int *val2, long mask)
>> +{
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_RAW: {
>> +             int ret;
>> +             __be32 reg;
>> +
>> +             mutex_lock(&indio_dev->mlock);
>> +
>> +             if (iio_buffer_enabled(indio_dev)) {
>> +                     ret = -EINVAL;
>> +                     goto error_busy;
>> +             }
>> +
>> +             switch (chan->type) {
>> +             case IIO_TEMP:
>> +                     ret = regmap_bulk_read(data->regmap, chan->address,
>> +                                           (u8 *) &reg, sizeof(reg));
>> +                     break;
>> +             case IIO_PH:
>> +                     ret = atlas_read_ph_measurement(data, &reg);
>> +                     break;
>> +             default:
>> +                     ret = -EINVAL;
>> +             }
>> +
>> +             if (!ret) {
>> +                     *val = be32_to_cpu(reg);
>> +                     ret = IIO_VAL_INT;
>> +             }
>> +
>> +error_busy:
>> +             mutex_unlock(&indio_dev->mlock);
>> +             return ret;
>> +     }
>> +     case IIO_CHAN_INFO_SCALE:
>> +             *val = 0; /* 0.001 */
>> +             *val2 = 1000;
>> +             return IIO_VAL_INT_PLUS_MICRO;
>> +     }
>> +
>> +     return -EINVAL;
>> +}
>> +
>> +static int atlas_write_raw(struct iio_dev *indio_dev,
>> +                        struct iio_chan_spec const *chan,
>> +                        int val, int val2, long mask)
>> +{
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +     __be32 reg = cpu_to_be32(val);
>> +
>> +     if (val2 != 0)
>> +             return -EINVAL;
>> +
>> +     if (mask != IIO_CHAN_INFO_RAW || chan->type != IIO_TEMP)
>> +             return -EINVAL;
>> +
>> +     return regmap_bulk_write(data->regmap, chan->address, &reg, 4);
>> +}
>> +
>> +static const struct iio_info atlas_info = {
>> +     .driver_module = THIS_MODULE,
>> +     .read_raw = atlas_read_raw,
>> +     .write_raw = atlas_write_raw,
>> +};
>> +
>> +static int atlas_probe(struct i2c_client *client,
>> +                    const struct i2c_device_id *id)
>> +{
>> +     struct atlas_data *data;
>> +     struct iio_buffer *buffer;
>> +     struct iio_dev *indio_dev;
>> +     int ret;
>> +
>> +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> +     if (!indio_dev)
>> +             return -ENOMEM;
>> +
>> +     buffer = devm_iio_kfifo_allocate(&client->dev);
>> +     if (!buffer)
>> +             return -ENOMEM;
>> +
>> +     iio_device_attach_buffer(indio_dev, buffer);
>> +
>> +     indio_dev->info = &atlas_info;
>> +     indio_dev->name = ATLAS_DRV_NAME;
>> +     indio_dev->channels = atlas_channels;
>> +     indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);
>> +     indio_dev->setup_ops = &atlas_buffer_setup_ops;
>> +
>> +     data = iio_priv(indio_dev);
>> +     data->client = client;
>> +
>> +     i2c_set_clientdata(client, indio_dev);
>> +
>> +     data->regmap = devm_regmap_init_i2c(client, &atlas_regmap_config);
>> +     if (IS_ERR(data->regmap)) {
>> +             dev_err(&client->dev, "regmap initialization failed\n");
>> +             return PTR_ERR(data->regmap);
>> +     }
>> +
>> +     ret = pm_runtime_set_active(&client->dev);
>> +     if (ret)
>> +             return ret;
>> +
>> +     if (client->irq <= 0) {
>> +             dev_err(&client->dev, "no valid irq defined\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     ret = devm_request_threaded_irq(&client->dev, client->irq,
>> +                                     NULL, atlas_interrupt_handler,
>> +                                     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>> +                                     "atlas_event",
>> +                                     indio_dev);
>> +     if (ret) {
>> +             dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
>> +             return ret;
>> +     }
>> +
>> +     pm_runtime_enable(&client->dev);
>> +     pm_runtime_set_autosuspend_delay(&client->dev, 2500);
>> +     pm_runtime_use_autosuspend(&client->dev);
>> +
>> +     ret = atlas_set_powermode(data, 0);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return iio_device_register(indio_dev);
>> +}
>> +
>> +static int atlas_remove(struct i2c_client *client)
>> +{
>> +     struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> +     struct atlas_data *data = iio_priv(indio_dev);
>> +
>> +     iio_device_unregister(indio_dev);
>> +
>> +     pm_runtime_disable(&client->dev);
>> +     pm_runtime_set_suspended(&client->dev);
>> +
>> +     return atlas_set_powermode(data, 0);
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +static int atlas_runtime_suspend(struct device *dev)
>> +{
>> +     struct atlas_data *data =
>> +                  iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +     return atlas_set_powermode(data, 0);
>> +}
>> +
>> +static int atlas_runtime_resume(struct device *dev)
>> +{
>> +     struct atlas_data *data =
>> +                  iio_priv(i2c_get_clientdata(to_i2c_client(dev)));
>> +
>> +     return atlas_set_powermode(data, 1);
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops atlas_pm_ops = {
>> +     SET_RUNTIME_PM_OPS(atlas_runtime_suspend,
>> +                        atlas_runtime_resume, NULL)
>> +};
>> +
>> +static const struct i2c_device_id atlas_id[] = {
>> +     { "atlas-ph-sm", 0 },
>> +     {}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, atlas_id);
>> +
>> +static const struct of_device_id atlas_dt_ids[] = {
>> +     { .compatible = "atlas,ph-sm" },
>> +     { }
>> +};
>> +MODULE_DEVICE_TABLE(of, atlas_dt_ids);
>> +
>> +static struct i2c_driver atlas_driver = {
>> +     .driver = {
>> +             .name   = ATLAS_DRV_NAME,
>> +             .of_match_table = of_match_ptr(atlas_dt_ids),
>> +             .pm     = &atlas_pm_ops,
>> +     },
>> +     .probe          = atlas_probe,
>> +     .remove         = atlas_remove,
>> +     .id_table       = atlas_id,
>> +};
>> +module_i2c_driver(atlas_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> +MODULE_DESCRIPTION("Atlas Scientific pH-SM sensor");
>> +MODULE_LICENSE("GPL");
>>
>