* Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support
2015-12-10 8:10 ` [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support Matt Ranostay
@ 2015-12-10 10:24 ` Adriana Reus
2015-12-11 4:50 ` Matt Ranostay
2015-12-10 11:04 ` Peter Meerwald-Stadler
2015-12-12 12:06 ` Jonathan Cameron
2 siblings, 1 reply; 14+ messages in thread
From: Adriana Reus @ 2015-12-10 10:24 UTC (permalink / raw)
To: Matt Ranostay, linux-iio, jic23
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 *) ®, sizeof(reg));
> + break;
> + case IIO_PH:
> + ret = atlas_read_ph_measurement(data, ®);
> + 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, ®, 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");
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support
2015-12-10 10:24 ` Adriana Reus
@ 2015-12-11 4:50 ` Matt Ranostay
2015-12-11 12:41 ` Adriana Reus
0 siblings, 1 reply; 14+ messages in thread
From: Matt Ranostay @ 2015-12-11 4:50 UTC (permalink / raw)
To: Adriana Reus; +Cc: linux-iio, Jonathan Cameron
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 *) ®, sizeof(reg));
>> + break;
>> + case IIO_PH:
>> + ret = atlas_read_ph_measurement(data, ®);
>> + 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, ®, 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");
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support
2015-12-11 4:50 ` Matt Ranostay
@ 2015-12-11 12:41 ` Adriana Reus
2015-12-12 4:06 ` Matt Ranostay
0 siblings, 1 reply; 14+ messages in thread
From: Adriana Reus @ 2015-12-11 12:41 UTC (permalink / raw)
To: Matt Ranostay; +Cc: linux-iio, Jonathan Cameron
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 *) ®, sizeof(reg));
>>> + break;
>>> + case IIO_PH:
>>> + ret = atlas_read_ph_measurement(data, ®);
>>> + 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, ®, 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");
>>>
>>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support
2015-12-11 12:41 ` Adriana Reus
@ 2015-12-12 4:06 ` Matt Ranostay
0 siblings, 0 replies; 14+ messages in thread
From: Matt Ranostay @ 2015-12-12 4:06 UTC (permalink / raw)
To: Adriana Reus; +Cc: linux-iio, Jonathan Cameron
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 *) ®, sizeof(reg));
>>>> + break;
>>>> + case IIO_PH:
>>>> + ret = atlas_read_ph_measurement(data, ®);
>>>> + 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, ®, 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");
>>>>
>>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support
2015-12-10 8:10 ` [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support Matt Ranostay
2015-12-10 10:24 ` Adriana Reus
@ 2015-12-10 11:04 ` Peter Meerwald-Stadler
2015-12-10 19:19 ` Matt Ranostay
2015-12-12 12:06 ` Jonathan Cameron
2 siblings, 1 reply; 14+ messages in thread
From: Peter Meerwald-Stadler @ 2015-12-10 11:04 UTC (permalink / raw)
To: Matt Ranostay; +Cc: linux-iio, jic23
> 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 *) ®, sizeof(reg));
zero on success...
> + break;
> + case IIO_PH:
> + ret = atlas_read_ph_measurement(data, ®);
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, ®, 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)
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support
2015-12-10 11:04 ` Peter Meerwald-Stadler
@ 2015-12-10 19:19 ` Matt Ranostay
0 siblings, 0 replies; 14+ messages in thread
From: Matt Ranostay @ 2015-12-10 19:19 UTC (permalink / raw)
To: Peter Meerwald-Stadler; +Cc: linux-iio, Jonathan Cameron
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 *) ®, sizeof(reg));
>
> zero on success...
>
>> + break;
>> + case IIO_PH:
>> + ret = atlas_read_ph_measurement(data, ®);
>
> 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, ®, 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)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support
2015-12-10 8:10 ` [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support Matt Ranostay
2015-12-10 10:24 ` Adriana Reus
2015-12-10 11:04 ` Peter Meerwald-Stadler
@ 2015-12-12 12:06 ` Jonathan Cameron
2015-12-13 5:50 ` Matt Ranostay
2 siblings, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2015-12-12 12:06 UTC (permalink / raw)
To: Matt Ranostay, linux-iio
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 *) ®, sizeof(reg));
> + break;
> + case IIO_PH:
> + ret = atlas_read_ph_measurement(data, ®);
> + 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, ®, 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");
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC v1 3/3] iio: chemical: add Atlas pH-SM sensor support
2015-12-12 12:06 ` Jonathan Cameron
@ 2015-12-13 5:50 ` Matt Ranostay
0 siblings, 0 replies; 14+ messages in thread
From: Matt Ranostay @ 2015-12-13 5:50 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio
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 *) ®, sizeof(reg));
>> + break;
>> + case IIO_PH:
>> + ret = atlas_read_ph_measurement(data, ®);
>> + 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, ®, 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");
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread