* [PATCH 0/2] iio: chemical: atlas-ph-sensor: add conductivity sensor support @ 2016-04-24 20:43 Matt Ranostay 2016-04-24 20:43 ` [PATCH 1/2] iio: chemical: atlas-ph-sensor: reorg driver to allow multiple chips Matt Ranostay 2016-04-24 20:43 ` [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature Matt Ranostay 0 siblings, 2 replies; 10+ messages in thread From: Matt Ranostay @ 2016-04-24 20:43 UTC (permalink / raw) To: linux-iio; +Cc: jic23, Matt Ranostay This patchset series adds the following functionality to the atlas-ph-sensor driver: * Allow other sensor chips to share the core driver functionality * Add support for the Atlas EC-SM OEM conductivity sensor Matt Ranostay (2): iio: chemical: atlas-ph-sensor: reorg driver to allow multiple chips iio: chemical: atlas-ph-sensor: add EC feature .../bindings/iio/chemical/atlas,ec-sm.txt | 22 ++ drivers/iio/chemical/Kconfig | 8 +- drivers/iio/chemical/atlas-ph-sensor.c | 244 ++++++++++++++++----- 3 files changed, 221 insertions(+), 53 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt -- 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] iio: chemical: atlas-ph-sensor: reorg driver to allow multiple chips 2016-04-24 20:43 [PATCH 0/2] iio: chemical: atlas-ph-sensor: add conductivity sensor support Matt Ranostay @ 2016-04-24 20:43 ` Matt Ranostay 2016-05-04 10:31 ` Jonathan Cameron 2016-04-24 20:43 ` [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature Matt Ranostay 1 sibling, 1 reply; 10+ messages in thread From: Matt Ranostay @ 2016-04-24 20:43 UTC (permalink / raw) To: linux-iio; +Cc: jic23, Matt Ranostay Signed-off-by: Matt Ranostay <mranostay@gmail.com> --- drivers/iio/chemical/atlas-ph-sensor.c | 131 +++++++++++++++++++++------------ 1 file changed, 83 insertions(+), 48 deletions(-) diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c index 62b37cd..c57f9c2 100644 --- a/drivers/iio/chemical/atlas-ph-sensor.c +++ b/drivers/iio/chemical/atlas-ph-sensor.c @@ -24,6 +24,7 @@ #include <linux/irq_work.h> #include <linux/gpio.h> #include <linux/i2c.h> +#include <linux/of_device.h> #include <linux/regmap.h> #include <linux/iio/iio.h> #include <linux/iio/buffer.h> @@ -43,20 +44,25 @@ #define ATLAS_REG_PWR_CONTROL 0x06 -#define ATLAS_REG_CALIB_STATUS 0x0d -#define ATLAS_REG_CALIB_STATUS_MASK 0x07 -#define ATLAS_REG_CALIB_STATUS_LOW BIT(0) -#define ATLAS_REG_CALIB_STATUS_MID BIT(1) -#define ATLAS_REG_CALIB_STATUS_HIGH BIT(2) +#define ATLAS_REG_PH_CALIB_STATUS 0x0d +#define ATLAS_REG_PH_CALIB_STATUS_MASK 0x07 +#define ATLAS_REG_PH_CALIB_STATUS_LOW BIT(0) +#define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) +#define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) -#define ATLAS_REG_TEMP_DATA 0x0e +#define ATLAS_REG_PH_TEMP_DATA 0x0e #define ATLAS_REG_PH_DATA 0x16 #define ATLAS_PH_INT_TIME_IN_US 450000 +enum { + ATLAS_PH_SM, +}; + struct atlas_data { struct i2c_client *client; struct iio_trigger *trig; + struct atlas_device *chip; struct regmap *regmap; struct irq_work work; @@ -84,9 +90,10 @@ static const struct regmap_config atlas_regmap_config = { .cache_type = REGCACHE_RBTREE, }; -static const struct iio_chan_spec atlas_channels[] = { +static const struct iio_chan_spec atlas_ph_channels[] = { { .type = IIO_PH, + .address = ATLAS_REG_PH_DATA, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), .scan_index = 0, @@ -100,7 +107,7 @@ static const struct iio_chan_spec atlas_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(1), { .type = IIO_TEMP, - .address = ATLAS_REG_TEMP_DATA, + .address = ATLAS_REG_PH_TEMP_DATA, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), .output = 1, @@ -108,6 +115,52 @@ static const struct iio_chan_spec atlas_channels[] = { }, }; +static int atlas_check_ph_calibration(struct atlas_data *data) +{ + struct device *dev = &data->client->dev; + int ret; + unsigned int val; + + ret = regmap_read(data->regmap, ATLAS_REG_PH_CALIB_STATUS, &val); + if (ret) + return ret; + + if (!(val & ATLAS_REG_PH_CALIB_STATUS_MASK)) { + dev_warn(dev, "device has not been calibrated\n"); + return 0; + } + + if (!(val & ATLAS_REG_PH_CALIB_STATUS_LOW)) + dev_warn(dev, "device missing low point calibration\n"); + + if (!(val & ATLAS_REG_PH_CALIB_STATUS_MID)) + dev_warn(dev, "device missing mid point calibration\n"); + + if (!(val & ATLAS_REG_PH_CALIB_STATUS_HIGH)) + dev_warn(dev, "device missing high point calibration\n"); + + return 0; +} + +struct atlas_device { + const struct iio_chan_spec *channels; + int num_channels; + int data_reg; + + int (*calibration)(struct atlas_data *data); + int delay; +}; + +static struct atlas_device atlas_devices[] = { + [ATLAS_PH_SM] = { + .channels = atlas_ph_channels, + .num_channels = 3, + .data_reg = ATLAS_REG_PH_DATA, + .calibration = &atlas_check_ph_calibration, + .delay = ATLAS_PH_INT_TIME_IN_US, + }, +}; + static int atlas_set_powermode(struct atlas_data *data, int on) { return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on); @@ -178,8 +231,9 @@ static irqreturn_t atlas_trigger_handler(int irq, void *private) struct atlas_data *data = iio_priv(indio_dev); int ret; - ret = regmap_bulk_read(data->regmap, ATLAS_REG_PH_DATA, - (u8 *) &data->buffer, sizeof(data->buffer[0])); + ret = regmap_bulk_read(data->regmap, data->chip->data_reg, + (u8 *) &data->buffer, + sizeof(__be32) * (data->chip->num_channels - 2)); if (!ret) iio_push_to_buffers_with_timestamp(indio_dev, data->buffer, @@ -200,7 +254,7 @@ static irqreturn_t atlas_interrupt_handler(int irq, void *private) return IRQ_HANDLED; } -static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val) +static int atlas_read_measurement(struct atlas_data *data, int reg, __be32 *val) { struct device *dev = &data->client->dev; int suspended = pm_runtime_suspended(dev); @@ -213,11 +267,9 @@ static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val) } if (suspended) - usleep_range(ATLAS_PH_INT_TIME_IN_US, - ATLAS_PH_INT_TIME_IN_US + 100000); + usleep_range(data->chip->delay, data->chip->delay + 100000); - ret = regmap_bulk_read(data->regmap, ATLAS_REG_PH_DATA, - (u8 *) val, sizeof(*val)); + ret = regmap_bulk_read(data->regmap, reg, (u8 *) val, sizeof(*val)); pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); @@ -247,7 +299,8 @@ static int atlas_read_raw(struct iio_dev *indio_dev, if (iio_buffer_enabled(indio_dev)) ret = -EBUSY; else - ret = atlas_read_ph_measurement(data, ®); + ret = atlas_read_measurement(data, + chan->address, ®); mutex_unlock(&indio_dev->mlock); break; @@ -303,37 +356,12 @@ static const struct iio_info atlas_info = { .write_raw = atlas_write_raw, }; -static int atlas_check_calibration(struct atlas_data *data) -{ - struct device *dev = &data->client->dev; - int ret; - unsigned int val; - - ret = regmap_read(data->regmap, ATLAS_REG_CALIB_STATUS, &val); - if (ret) - return ret; - - if (!(val & ATLAS_REG_CALIB_STATUS_MASK)) { - dev_warn(dev, "device has not been calibrated\n"); - return 0; - } - - if (!(val & ATLAS_REG_CALIB_STATUS_LOW)) - dev_warn(dev, "device missing low point calibration\n"); - - if (!(val & ATLAS_REG_CALIB_STATUS_MID)) - dev_warn(dev, "device missing mid point calibration\n"); - - if (!(val & ATLAS_REG_CALIB_STATUS_HIGH)) - dev_warn(dev, "device missing high point calibration\n"); - - return 0; -}; - static int atlas_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct atlas_data *data; + struct atlas_device *chip; + const struct of_device_id *of_id; struct iio_trigger *trig; struct iio_dev *indio_dev; int ret; @@ -342,10 +370,16 @@ static int atlas_probe(struct i2c_client *client, if (!indio_dev) return -ENOMEM; + of_id = of_match_device(atlas_dt_ids, &client->dev); + if (!of_id) + chip = &atlas_devices[id->driver_data]; + else + chip = &atlas_devices[(unsigned long)of_id->data]; + indio_dev->info = &atlas_info; indio_dev->name = ATLAS_DRV_NAME; - indio_dev->channels = atlas_channels; - indio_dev->num_channels = ARRAY_SIZE(atlas_channels); + indio_dev->channels = chip->channels; + indio_dev->num_channels = chip->num_channels; indio_dev->modes = INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE; indio_dev->dev.parent = &client->dev; @@ -358,6 +392,7 @@ static int atlas_probe(struct i2c_client *client, data = iio_priv(indio_dev); data->client = client; data->trig = trig; + data->chip = chip; trig->dev.parent = indio_dev->dev.parent; trig->ops = &atlas_interrupt_trigger_ops; iio_trigger_set_drvdata(trig, indio_dev); @@ -379,7 +414,7 @@ static int atlas_probe(struct i2c_client *client, return -EINVAL; } - ret = atlas_check_calibration(data); + ret = chip->calibration(data); if (ret) return ret; @@ -481,13 +516,13 @@ static const struct dev_pm_ops atlas_pm_ops = { }; static const struct i2c_device_id atlas_id[] = { - { "atlas-ph-sm", 0 }, + { "atlas-ph-sm", ATLAS_PH_SM}, {} }; MODULE_DEVICE_TABLE(i2c, atlas_id); static const struct of_device_id atlas_dt_ids[] = { - { .compatible = "atlas,ph-sm" }, + { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, { } }; MODULE_DEVICE_TABLE(of, atlas_dt_ids); -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] iio: chemical: atlas-ph-sensor: reorg driver to allow multiple chips 2016-04-24 20:43 ` [PATCH 1/2] iio: chemical: atlas-ph-sensor: reorg driver to allow multiple chips Matt Ranostay @ 2016-05-04 10:31 ` Jonathan Cameron 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:31 UTC (permalink / raw) To: Matt Ranostay, linux-iio On 24/04/16 21:43, Matt Ranostay wrote: > Signed-off-by: Matt Ranostay <mranostay@gmail.com> I was half way through reviewing this when by battery ran out on Sunday so fingers crossed this time! (it claims I have an hour left and 2 more hours on this train) Anyhow did make it through - straight forward refactor. Will apply if no issues in next patch. Jonathan > --- > drivers/iio/chemical/atlas-ph-sensor.c | 131 +++++++++++++++++++++------------ > 1 file changed, 83 insertions(+), 48 deletions(-) > > diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c > index 62b37cd..c57f9c2 100644 > --- a/drivers/iio/chemical/atlas-ph-sensor.c > +++ b/drivers/iio/chemical/atlas-ph-sensor.c > @@ -24,6 +24,7 @@ > #include <linux/irq_work.h> > #include <linux/gpio.h> > #include <linux/i2c.h> > +#include <linux/of_device.h> > #include <linux/regmap.h> > #include <linux/iio/iio.h> > #include <linux/iio/buffer.h> > @@ -43,20 +44,25 @@ > > #define ATLAS_REG_PWR_CONTROL 0x06 > > -#define ATLAS_REG_CALIB_STATUS 0x0d > -#define ATLAS_REG_CALIB_STATUS_MASK 0x07 > -#define ATLAS_REG_CALIB_STATUS_LOW BIT(0) > -#define ATLAS_REG_CALIB_STATUS_MID BIT(1) > -#define ATLAS_REG_CALIB_STATUS_HIGH BIT(2) > +#define ATLAS_REG_PH_CALIB_STATUS 0x0d > +#define ATLAS_REG_PH_CALIB_STATUS_MASK 0x07 > +#define ATLAS_REG_PH_CALIB_STATUS_LOW BIT(0) > +#define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) > +#define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) > > -#define ATLAS_REG_TEMP_DATA 0x0e > +#define ATLAS_REG_PH_TEMP_DATA 0x0e > #define ATLAS_REG_PH_DATA 0x16 > > #define ATLAS_PH_INT_TIME_IN_US 450000 > > +enum { > + ATLAS_PH_SM, > +}; > + > struct atlas_data { > struct i2c_client *client; > struct iio_trigger *trig; > + struct atlas_device *chip; > struct regmap *regmap; > struct irq_work work; > > @@ -84,9 +90,10 @@ static const struct regmap_config atlas_regmap_config = { > .cache_type = REGCACHE_RBTREE, > }; > > -static const struct iio_chan_spec atlas_channels[] = { > +static const struct iio_chan_spec atlas_ph_channels[] = { > { > .type = IIO_PH, > + .address = ATLAS_REG_PH_DATA, > .info_mask_separate = > BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), > .scan_index = 0, > @@ -100,7 +107,7 @@ static const struct iio_chan_spec atlas_channels[] = { > IIO_CHAN_SOFT_TIMESTAMP(1), > { > .type = IIO_TEMP, > - .address = ATLAS_REG_TEMP_DATA, > + .address = ATLAS_REG_PH_TEMP_DATA, > .info_mask_separate = > BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), > .output = 1, > @@ -108,6 +115,52 @@ static const struct iio_chan_spec atlas_channels[] = { > }, > }; > > +static int atlas_check_ph_calibration(struct atlas_data *data) > +{ > + struct device *dev = &data->client->dev; > + int ret; > + unsigned int val; > + > + ret = regmap_read(data->regmap, ATLAS_REG_PH_CALIB_STATUS, &val); > + if (ret) > + return ret; > + > + if (!(val & ATLAS_REG_PH_CALIB_STATUS_MASK)) { > + dev_warn(dev, "device has not been calibrated\n"); > + return 0; > + } > + > + if (!(val & ATLAS_REG_PH_CALIB_STATUS_LOW)) > + dev_warn(dev, "device missing low point calibration\n"); > + > + if (!(val & ATLAS_REG_PH_CALIB_STATUS_MID)) > + dev_warn(dev, "device missing mid point calibration\n"); > + > + if (!(val & ATLAS_REG_PH_CALIB_STATUS_HIGH)) > + dev_warn(dev, "device missing high point calibration\n"); > + > + return 0; > +} > + > +struct atlas_device { > + const struct iio_chan_spec *channels; > + int num_channels; > + int data_reg; > + > + int (*calibration)(struct atlas_data *data); > + int delay; > +}; > + > +static struct atlas_device atlas_devices[] = { > + [ATLAS_PH_SM] = { > + .channels = atlas_ph_channels, > + .num_channels = 3, > + .data_reg = ATLAS_REG_PH_DATA, > + .calibration = &atlas_check_ph_calibration, > + .delay = ATLAS_PH_INT_TIME_IN_US, > + }, > +}; > + > static int atlas_set_powermode(struct atlas_data *data, int on) > { > return regmap_write(data->regmap, ATLAS_REG_PWR_CONTROL, on); > @@ -178,8 +231,9 @@ static irqreturn_t atlas_trigger_handler(int irq, void *private) > struct atlas_data *data = iio_priv(indio_dev); > int ret; > > - ret = regmap_bulk_read(data->regmap, ATLAS_REG_PH_DATA, > - (u8 *) &data->buffer, sizeof(data->buffer[0])); > + ret = regmap_bulk_read(data->regmap, data->chip->data_reg, > + (u8 *) &data->buffer, > + sizeof(__be32) * (data->chip->num_channels - 2)); > > if (!ret) > iio_push_to_buffers_with_timestamp(indio_dev, data->buffer, > @@ -200,7 +254,7 @@ static irqreturn_t atlas_interrupt_handler(int irq, void *private) > return IRQ_HANDLED; > } > > -static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val) > +static int atlas_read_measurement(struct atlas_data *data, int reg, __be32 *val) > { > struct device *dev = &data->client->dev; > int suspended = pm_runtime_suspended(dev); > @@ -213,11 +267,9 @@ static int atlas_read_ph_measurement(struct atlas_data *data, __be32 *val) > } > > if (suspended) > - usleep_range(ATLAS_PH_INT_TIME_IN_US, > - ATLAS_PH_INT_TIME_IN_US + 100000); > + usleep_range(data->chip->delay, data->chip->delay + 100000); > > - ret = regmap_bulk_read(data->regmap, ATLAS_REG_PH_DATA, > - (u8 *) val, sizeof(*val)); > + ret = regmap_bulk_read(data->regmap, reg, (u8 *) val, sizeof(*val)); > > pm_runtime_mark_last_busy(dev); > pm_runtime_put_autosuspend(dev); > @@ -247,7 +299,8 @@ static int atlas_read_raw(struct iio_dev *indio_dev, > if (iio_buffer_enabled(indio_dev)) > ret = -EBUSY; > else > - ret = atlas_read_ph_measurement(data, ®); > + ret = atlas_read_measurement(data, > + chan->address, ®); > > mutex_unlock(&indio_dev->mlock); > break; > @@ -303,37 +356,12 @@ static const struct iio_info atlas_info = { > .write_raw = atlas_write_raw, > }; > > -static int atlas_check_calibration(struct atlas_data *data) > -{ > - struct device *dev = &data->client->dev; > - int ret; > - unsigned int val; > - > - ret = regmap_read(data->regmap, ATLAS_REG_CALIB_STATUS, &val); > - if (ret) > - return ret; > - > - if (!(val & ATLAS_REG_CALIB_STATUS_MASK)) { > - dev_warn(dev, "device has not been calibrated\n"); > - return 0; > - } > - > - if (!(val & ATLAS_REG_CALIB_STATUS_LOW)) > - dev_warn(dev, "device missing low point calibration\n"); > - > - if (!(val & ATLAS_REG_CALIB_STATUS_MID)) > - dev_warn(dev, "device missing mid point calibration\n"); > - > - if (!(val & ATLAS_REG_CALIB_STATUS_HIGH)) > - dev_warn(dev, "device missing high point calibration\n"); > - > - return 0; > -}; > - > static int atlas_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > struct atlas_data *data; > + struct atlas_device *chip; > + const struct of_device_id *of_id; > struct iio_trigger *trig; > struct iio_dev *indio_dev; > int ret; > @@ -342,10 +370,16 @@ static int atlas_probe(struct i2c_client *client, > if (!indio_dev) > return -ENOMEM; > > + of_id = of_match_device(atlas_dt_ids, &client->dev); > + if (!of_id) > + chip = &atlas_devices[id->driver_data]; > + else > + chip = &atlas_devices[(unsigned long)of_id->data]; > + > indio_dev->info = &atlas_info; > indio_dev->name = ATLAS_DRV_NAME; > - indio_dev->channels = atlas_channels; > - indio_dev->num_channels = ARRAY_SIZE(atlas_channels); > + indio_dev->channels = chip->channels; > + indio_dev->num_channels = chip->num_channels; > indio_dev->modes = INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE; > indio_dev->dev.parent = &client->dev; > > @@ -358,6 +392,7 @@ static int atlas_probe(struct i2c_client *client, > data = iio_priv(indio_dev); > data->client = client; > data->trig = trig; > + data->chip = chip; > trig->dev.parent = indio_dev->dev.parent; > trig->ops = &atlas_interrupt_trigger_ops; > iio_trigger_set_drvdata(trig, indio_dev); > @@ -379,7 +414,7 @@ static int atlas_probe(struct i2c_client *client, > return -EINVAL; > } > > - ret = atlas_check_calibration(data); > + ret = chip->calibration(data); > if (ret) > return ret; > > @@ -481,13 +516,13 @@ static const struct dev_pm_ops atlas_pm_ops = { > }; > > static const struct i2c_device_id atlas_id[] = { > - { "atlas-ph-sm", 0 }, > + { "atlas-ph-sm", ATLAS_PH_SM}, > {} > }; > MODULE_DEVICE_TABLE(i2c, atlas_id); > > static const struct of_device_id atlas_dt_ids[] = { > - { .compatible = "atlas,ph-sm" }, > + { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, > { } > }; > MODULE_DEVICE_TABLE(of, atlas_dt_ids); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature 2016-04-24 20:43 [PATCH 0/2] iio: chemical: atlas-ph-sensor: add conductivity sensor support Matt Ranostay 2016-04-24 20:43 ` [PATCH 1/2] iio: chemical: atlas-ph-sensor: reorg driver to allow multiple chips Matt Ranostay @ 2016-04-24 20:43 ` Matt Ranostay 2016-05-04 10:39 ` Jonathan Cameron 1 sibling, 1 reply; 10+ messages in thread From: Matt Ranostay @ 2016-04-24 20:43 UTC (permalink / raw) To: linux-iio; +Cc: jic23, Matt Ranostay Signed-off-by: Matt Ranostay <mranostay@gmail.com> --- .../bindings/iio/chemical/atlas,ec-sm.txt | 22 ++++ drivers/iio/chemical/Kconfig | 8 +- drivers/iio/chemical/atlas-ph-sensor.c | 113 ++++++++++++++++++++- 3 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt new file mode 100644 index 0000000..2962bd9 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt @@ -0,0 +1,22 @@ +* Atlas Scientific EC-SM OEM sensor + +http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf + +Required properties: + + - compatible: must be "atlas,ec-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@64 { + compatible = "atlas,ec-sm"; + reg = <0x64>; + interrupt-parent = <&gpio1>; + interrupts = <16 2>; +}; diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig index f73290f..4bcc025 100644 --- a/drivers/iio/chemical/Kconfig +++ b/drivers/iio/chemical/Kconfig @@ -5,15 +5,17 @@ menu "Chemical Sensors" config ATLAS_PH_SENSOR - tristate "Atlas Scientific OEM pH-SM sensor" + tristate "Atlas Scientific OEM SM sensors" depends on I2C select REGMAP_I2C select IIO_BUFFER select IIO_TRIGGERED_BUFFER select IRQ_WORK help - Say Y here to build I2C interface support for the Atlas - Scientific OEM pH-SM sensor. + Say Y here to build I2C interface support for the following + Atlas Scientific OEM SM sensors: + * pH SM sensor + * EC SM sensor To compile this driver as module, choose M here: the module will be called atlas-ph-sensor. diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c index c57f9c2..6dddd75 100644 --- a/drivers/iio/chemical/atlas-ph-sensor.c +++ b/drivers/iio/chemical/atlas-ph-sensor.c @@ -50,13 +50,28 @@ #define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) #define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) +#define ATLAS_REG_EC_CALIB_STATUS 0x0f +#define ATLAS_REG_EC_CALIB_STATUS_MASK 0x0f +#define ATLAS_REG_EC_CALIB_STATUS_DRY BIT(0) +#define ATLAS_REG_EC_CALIB_STATUS_SINGLE BIT(1) +#define ATLAS_REG_EC_CALIB_STATUS_LOW BIT(2) +#define ATLAS_REG_EC_CALIB_STATUS_HIGH BIT(3) + #define ATLAS_REG_PH_TEMP_DATA 0x0e #define ATLAS_REG_PH_DATA 0x16 +#define ATLAS_REG_EC_PROBE 0x08 +#define ATLAS_REG_EC_TEMP_DATA 0x10 +#define ATLAS_REG_EC_DATA 0x18 +#define ATLAS_REG_TDS_DATA 0x1c +#define ATLAS_REG_PSS_DATA 0x20 + #define ATLAS_PH_INT_TIME_IN_US 450000 +#define ATLAS_EC_INT_TIME_IN_US 650000 enum { ATLAS_PH_SM, + ATLAS_EC_SM, }; struct atlas_data { @@ -66,12 +81,13 @@ struct atlas_data { struct regmap *regmap; struct irq_work work; - __be32 buffer[4]; /* 32-bit pH data + 32-bit pad + 64-bit timestamp */ + __be32 buffer[6]; /* 96-bit data + 32-bit pad + 64-bit timestamp */ }; static const struct regmap_range atlas_volatile_ranges[] = { regmap_reg_range(ATLAS_REG_INT_CONTROL, ATLAS_REG_INT_CONTROL), regmap_reg_range(ATLAS_REG_PH_DATA, ATLAS_REG_PH_DATA + 4), + regmap_reg_range(ATLAS_REG_EC_DATA, ATLAS_REG_PSS_DATA + 4), }; static const struct regmap_access_table atlas_volatile_table = { @@ -86,7 +102,7 @@ static const struct regmap_config atlas_regmap_config = { .val_bits = 8, .volatile_table = &atlas_volatile_table, - .max_register = ATLAS_REG_PH_DATA + 4, + .max_register = ATLAS_REG_PSS_DATA + 4, .cache_type = REGCACHE_RBTREE, }; @@ -115,6 +131,38 @@ static const struct iio_chan_spec atlas_ph_channels[] = { }, }; +#define ATLAS_EC_CHANNEL(idx, addr) \ + {\ + .type = IIO_CONCENTRATION, \ + .indexed = 1, \ + .channel = idx, \ + .address = addr, \ + .info_mask_separate = \ + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = idx, \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_BE, \ + }, \ + } + +static const struct iio_chan_spec atlas_ec_channels[] = { + ATLAS_EC_CHANNEL(0, ATLAS_REG_EC_DATA), + ATLAS_EC_CHANNEL(1, ATLAS_REG_TDS_DATA), + ATLAS_EC_CHANNEL(2, ATLAS_REG_PSS_DATA), + IIO_CHAN_SOFT_TIMESTAMP(3), + { + .type = IIO_TEMP, + .address = ATLAS_REG_EC_TEMP_DATA, + .info_mask_separate = + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), + .output = 1, + .scan_index = -1 + }, +}; + static int atlas_check_ph_calibration(struct atlas_data *data) { struct device *dev = &data->client->dev; @@ -142,6 +190,44 @@ static int atlas_check_ph_calibration(struct atlas_data *data) return 0; } +static int atlas_check_ec_calibration(struct atlas_data *data) +{ + struct device *dev = &data->client->dev; + int ret; + unsigned int val; + + ret = regmap_bulk_read(data->regmap, ATLAS_REG_EC_PROBE, &val, 2); + if (ret) + return ret; + + dev_info(dev, "probe set to K = %d.%.2d", be16_to_cpu(val) / 100, + be16_to_cpu(val) % 100); + + ret = regmap_read(data->regmap, ATLAS_REG_EC_CALIB_STATUS, &val); + if (ret) + return ret; + + if (!(val & ATLAS_REG_EC_CALIB_STATUS_MASK)) { + dev_warn(dev, "device has not been calibrated\n"); + return 0; + } + + if (!(val & ATLAS_REG_EC_CALIB_STATUS_DRY)) + dev_warn(dev, "device missing dry point calibration\n"); + + if (val & ATLAS_REG_EC_CALIB_STATUS_SINGLE) { + dev_warn(dev, "device using single point calibration\n"); + } else { + if (!(val & ATLAS_REG_EC_CALIB_STATUS_LOW)) + dev_warn(dev, "device missing low point calibration\n"); + + if (!(val & ATLAS_REG_EC_CALIB_STATUS_HIGH)) + dev_warn(dev, "device missing high point calibration\n"); + } + + return 0; +} + struct atlas_device { const struct iio_chan_spec *channels; int num_channels; @@ -159,6 +245,14 @@ static struct atlas_device atlas_devices[] = { .calibration = &atlas_check_ph_calibration, .delay = ATLAS_PH_INT_TIME_IN_US, }, + [ATLAS_EC_SM] = { + .channels = atlas_ec_channels, + .num_channels = 5, + .data_reg = ATLAS_REG_EC_DATA, + .calibration = &atlas_check_ec_calibration, + .delay = ATLAS_EC_INT_TIME_IN_US, + }, + }; static int atlas_set_powermode(struct atlas_data *data, int on) @@ -294,6 +388,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev, (u8 *) ®, sizeof(reg)); break; case IIO_PH: + case IIO_CONCENTRATION: mutex_lock(&indio_dev->mlock); if (iio_buffer_enabled(indio_dev)) @@ -324,6 +419,18 @@ static int atlas_read_raw(struct iio_dev *indio_dev, *val = 1; /* 0.001 */ *val2 = 1000; break; + case IIO_CONCENTRATION: + switch (chan->address) { + case ATLAS_REG_EC_DATA: + *val = 0; /* 0.00000000064 */ + *val2 = 640; + return IIO_VAL_INT_PLUS_NANO; + default: + *val = 0; /* 0.000000001 */ + *val2 = 1000; + return IIO_VAL_INT_PLUS_NANO; + } + break; default: return -EINVAL; } @@ -517,12 +624,14 @@ static const struct dev_pm_ops atlas_pm_ops = { static const struct i2c_device_id atlas_id[] = { { "atlas-ph-sm", ATLAS_PH_SM}, + { "atlas-ec-sm", ATLAS_EC_SM}, {} }; MODULE_DEVICE_TABLE(i2c, atlas_id); static const struct of_device_id atlas_dt_ids[] = { { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, + { .compatible = "atlas,ec-sm", .data = (void *)ATLAS_EC_SM, }, { } }; MODULE_DEVICE_TABLE(of, atlas_dt_ids); -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature 2016-04-24 20:43 ` [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature Matt Ranostay @ 2016-05-04 10:39 ` Jonathan Cameron 2016-05-05 8:17 ` Matt Ranostay 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Cameron @ 2016-05-04 10:39 UTC (permalink / raw) To: Matt Ranostay, linux-iio On 24/04/16 21:43, Matt Ranostay wrote: Give us a bit more description - what is it, what is it for, what interface is supplied. Err what does EC standard for? > Signed-off-by: Matt Ranostay <mranostay@gmail.com> I know next to nothing about these sensors and don't have the datasheet to hand. This is measuring 3 different types of concentration (I think) and reporting them simple as channels 0-2. What are they are should we not be distinguishing between them in some more informative fashion? Code itself is mostly fine... Jonathan > --- > .../bindings/iio/chemical/atlas,ec-sm.txt | 22 ++++ > drivers/iio/chemical/Kconfig | 8 +- > drivers/iio/chemical/atlas-ph-sensor.c | 113 ++++++++++++++++++++- > 3 files changed, 138 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt > > diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt > new file mode 100644 > index 0000000..2962bd9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt > @@ -0,0 +1,22 @@ > +* Atlas Scientific EC-SM OEM sensor > + > +http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf > + > +Required properties: > + > + - compatible: must be "atlas,ec-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@64 { > + compatible = "atlas,ec-sm"; > + reg = <0x64>; > + interrupt-parent = <&gpio1>; > + interrupts = <16 2>; > +}; > diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig > index f73290f..4bcc025 100644 > --- a/drivers/iio/chemical/Kconfig > +++ b/drivers/iio/chemical/Kconfig > @@ -5,15 +5,17 @@ > menu "Chemical Sensors" > > config ATLAS_PH_SENSOR > - tristate "Atlas Scientific OEM pH-SM sensor" > + tristate "Atlas Scientific OEM SM sensors" > depends on I2C > select REGMAP_I2C > select IIO_BUFFER > select IIO_TRIGGERED_BUFFER > select IRQ_WORK > help > - Say Y here to build I2C interface support for the Atlas > - Scientific OEM pH-SM sensor. > + Say Y here to build I2C interface support for the following > + Atlas Scientific OEM SM sensors: > + * pH SM sensor > + * EC SM sensor > > To compile this driver as module, choose M here: the > module will be called atlas-ph-sensor. > diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c > index c57f9c2..6dddd75 100644 > --- a/drivers/iio/chemical/atlas-ph-sensor.c > +++ b/drivers/iio/chemical/atlas-ph-sensor.c > @@ -50,13 +50,28 @@ > #define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) > #define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) > > +#define ATLAS_REG_EC_CALIB_STATUS 0x0f > +#define ATLAS_REG_EC_CALIB_STATUS_MASK 0x0f > +#define ATLAS_REG_EC_CALIB_STATUS_DRY BIT(0) > +#define ATLAS_REG_EC_CALIB_STATUS_SINGLE BIT(1) > +#define ATLAS_REG_EC_CALIB_STATUS_LOW BIT(2) > +#define ATLAS_REG_EC_CALIB_STATUS_HIGH BIT(3) > + > #define ATLAS_REG_PH_TEMP_DATA 0x0e > #define ATLAS_REG_PH_DATA 0x16 > > +#define ATLAS_REG_EC_PROBE 0x08 > +#define ATLAS_REG_EC_TEMP_DATA 0x10 > +#define ATLAS_REG_EC_DATA 0x18 > +#define ATLAS_REG_TDS_DATA 0x1c > +#define ATLAS_REG_PSS_DATA 0x20 > + > #define ATLAS_PH_INT_TIME_IN_US 450000 > +#define ATLAS_EC_INT_TIME_IN_US 650000 > > enum { > ATLAS_PH_SM, > + ATLAS_EC_SM, > }; > > struct atlas_data { > @@ -66,12 +81,13 @@ struct atlas_data { > struct regmap *regmap; > struct irq_work work; > > - __be32 buffer[4]; /* 32-bit pH data + 32-bit pad + 64-bit timestamp */ > + __be32 buffer[6]; /* 96-bit data + 32-bit pad + 64-bit timestamp */ > }; > > static const struct regmap_range atlas_volatile_ranges[] = { > regmap_reg_range(ATLAS_REG_INT_CONTROL, ATLAS_REG_INT_CONTROL), > regmap_reg_range(ATLAS_REG_PH_DATA, ATLAS_REG_PH_DATA + 4), > + regmap_reg_range(ATLAS_REG_EC_DATA, ATLAS_REG_PSS_DATA + 4), > }; > > static const struct regmap_access_table atlas_volatile_table = { > @@ -86,7 +102,7 @@ static const struct regmap_config atlas_regmap_config = { > .val_bits = 8, > > .volatile_table = &atlas_volatile_table, > - .max_register = ATLAS_REG_PH_DATA + 4, > + .max_register = ATLAS_REG_PSS_DATA + 4, > .cache_type = REGCACHE_RBTREE, > }; > > @@ -115,6 +131,38 @@ static const struct iio_chan_spec atlas_ph_channels[] = { > }, > }; > > +#define ATLAS_EC_CHANNEL(idx, addr) \ > + {\ > + .type = IIO_CONCENTRATION, \ > + .indexed = 1, \ > + .channel = idx, \ > + .address = addr, \ > + .info_mask_separate = \ > + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \ > + .scan_index = idx, \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = 32, \ > + .storagebits = 32, \ > + .endianness = IIO_BE, \ > + }, \ > + } > + > +static const struct iio_chan_spec atlas_ec_channels[] = { > + ATLAS_EC_CHANNEL(0, ATLAS_REG_EC_DATA), > + ATLAS_EC_CHANNEL(1, ATLAS_REG_TDS_DATA), > + ATLAS_EC_CHANNEL(2, ATLAS_REG_PSS_DATA), This needs some explanation. 3 concentration channels of different types? What are they... > + IIO_CHAN_SOFT_TIMESTAMP(3), > + { > + .type = IIO_TEMP, > + .address = ATLAS_REG_EC_TEMP_DATA, > + .info_mask_separate = > + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), > + .output = 1, > + .scan_index = -1 > + }, > +}; > + > static int atlas_check_ph_calibration(struct atlas_data *data) > { > struct device *dev = &data->client->dev; > @@ -142,6 +190,44 @@ static int atlas_check_ph_calibration(struct atlas_data *data) > return 0; > } > > +static int atlas_check_ec_calibration(struct atlas_data *data) > +{ > + struct device *dev = &data->client->dev; > + int ret; > + unsigned int val; > + > + ret = regmap_bulk_read(data->regmap, ATLAS_REG_EC_PROBE, &val, 2); > + if (ret) > + return ret; > + > + dev_info(dev, "probe set to K = %d.%.2d", be16_to_cpu(val) / 100, > + be16_to_cpu(val) % 100); > + > + ret = regmap_read(data->regmap, ATLAS_REG_EC_CALIB_STATUS, &val); > + if (ret) > + return ret; > + > + if (!(val & ATLAS_REG_EC_CALIB_STATUS_MASK)) { > + dev_warn(dev, "device has not been calibrated\n"); > + return 0; > + } > + > + if (!(val & ATLAS_REG_EC_CALIB_STATUS_DRY)) > + dev_warn(dev, "device missing dry point calibration\n"); > + > + if (val & ATLAS_REG_EC_CALIB_STATUS_SINGLE) { > + dev_warn(dev, "device using single point calibration\n"); > + } else { > + if (!(val & ATLAS_REG_EC_CALIB_STATUS_LOW)) > + dev_warn(dev, "device missing low point calibration\n"); > + > + if (!(val & ATLAS_REG_EC_CALIB_STATUS_HIGH)) > + dev_warn(dev, "device missing high point calibration\n"); > + } > + > + return 0; > +} > + > struct atlas_device { > const struct iio_chan_spec *channels; > int num_channels; > @@ -159,6 +245,14 @@ static struct atlas_device atlas_devices[] = { > .calibration = &atlas_check_ph_calibration, > .delay = ATLAS_PH_INT_TIME_IN_US, > }, > + [ATLAS_EC_SM] = { > + .channels = atlas_ec_channels, > + .num_channels = 5, Use array_size - obviously correct that way so I don't have to go count... > + .data_reg = ATLAS_REG_EC_DATA, > + .calibration = &atlas_check_ec_calibration, > + .delay = ATLAS_EC_INT_TIME_IN_US, > + }, > + > }; > > static int atlas_set_powermode(struct atlas_data *data, int on) > @@ -294,6 +388,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev, > (u8 *) ®, sizeof(reg)); I guess it's not complex enough to justify splitting read_raw for the two devices. Close though so you may want to if you support a third type in here. > break; > case IIO_PH: > + case IIO_CONCENTRATION: > mutex_lock(&indio_dev->mlock); > > if (iio_buffer_enabled(indio_dev)) > @@ -324,6 +419,18 @@ static int atlas_read_raw(struct iio_dev *indio_dev, > *val = 1; /* 0.001 */ > *val2 = 1000; > break; > + case IIO_CONCENTRATION: > + switch (chan->address) { > + case ATLAS_REG_EC_DATA: > + *val = 0; /* 0.00000000064 */ > + *val2 = 640; > + return IIO_VAL_INT_PLUS_NANO; > + default: > + *val = 0; /* 0.000000001 */ > + *val2 = 1000; > + return IIO_VAL_INT_PLUS_NANO; > + } > + break; > default: > return -EINVAL; > } > @@ -517,12 +624,14 @@ static const struct dev_pm_ops atlas_pm_ops = { > > static const struct i2c_device_id atlas_id[] = { > { "atlas-ph-sm", ATLAS_PH_SM}, > + { "atlas-ec-sm", ATLAS_EC_SM}, > {} > }; > MODULE_DEVICE_TABLE(i2c, atlas_id); > > static const struct of_device_id atlas_dt_ids[] = { > { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, > + { .compatible = "atlas,ec-sm", .data = (void *)ATLAS_EC_SM, }, > { } > }; > MODULE_DEVICE_TABLE(of, atlas_dt_ids); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature 2016-05-04 10:39 ` Jonathan Cameron @ 2016-05-05 8:17 ` Matt Ranostay 2016-05-08 19:29 ` Jonathan Cameron 0 siblings, 1 reply; 10+ messages in thread From: Matt Ranostay @ 2016-05-05 8:17 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio On Wed, May 4, 2016 at 3:39 AM, Jonathan Cameron <jic23@kernel.org> wrote: > On 24/04/16 21:43, Matt Ranostay wrote: > Give us a bit more description - what is it, what is it for, what interface is > supplied. Err what does EC standard for? >> Signed-off-by: Matt Ranostay <mranostay@gmail.com> > I know next to nothing about these sensors and don't have the datasheet to hand. > > This is measuring 3 different types of concentration (I think) and reporting them > simple as channels 0-2. What are they are should we not be distinguishing between > them in some more informative fashion? Should I add some .extend_name for each channel? Horribly I have to admit my pH sensor based board doesn't give valid values for this sensor till I resign the ground plane under the IC :/. But I can confirm it does report some "values"... > > Code itself is mostly fine... > > Jonathan >> --- >> .../bindings/iio/chemical/atlas,ec-sm.txt | 22 ++++ >> drivers/iio/chemical/Kconfig | 8 +- >> drivers/iio/chemical/atlas-ph-sensor.c | 113 ++++++++++++++++++++- >> 3 files changed, 138 insertions(+), 5 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >> >> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >> new file mode 100644 >> index 0000000..2962bd9 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >> @@ -0,0 +1,22 @@ >> +* Atlas Scientific EC-SM OEM sensor >> + >> +http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf >> + >> +Required properties: >> + >> + - compatible: must be "atlas,ec-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@64 { >> + compatible = "atlas,ec-sm"; >> + reg = <0x64>; >> + interrupt-parent = <&gpio1>; >> + interrupts = <16 2>; >> +}; >> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig >> index f73290f..4bcc025 100644 >> --- a/drivers/iio/chemical/Kconfig >> +++ b/drivers/iio/chemical/Kconfig >> @@ -5,15 +5,17 @@ >> menu "Chemical Sensors" >> >> config ATLAS_PH_SENSOR >> - tristate "Atlas Scientific OEM pH-SM sensor" >> + tristate "Atlas Scientific OEM SM sensors" >> depends on I2C >> select REGMAP_I2C >> select IIO_BUFFER >> select IIO_TRIGGERED_BUFFER >> select IRQ_WORK >> help >> - Say Y here to build I2C interface support for the Atlas >> - Scientific OEM pH-SM sensor. >> + Say Y here to build I2C interface support for the following >> + Atlas Scientific OEM SM sensors: >> + * pH SM sensor >> + * EC SM sensor >> >> To compile this driver as module, choose M here: the >> module will be called atlas-ph-sensor. >> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c >> index c57f9c2..6dddd75 100644 >> --- a/drivers/iio/chemical/atlas-ph-sensor.c >> +++ b/drivers/iio/chemical/atlas-ph-sensor.c >> @@ -50,13 +50,28 @@ >> #define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) >> #define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) >> >> +#define ATLAS_REG_EC_CALIB_STATUS 0x0f >> +#define ATLAS_REG_EC_CALIB_STATUS_MASK 0x0f >> +#define ATLAS_REG_EC_CALIB_STATUS_DRY BIT(0) >> +#define ATLAS_REG_EC_CALIB_STATUS_SINGLE BIT(1) >> +#define ATLAS_REG_EC_CALIB_STATUS_LOW BIT(2) >> +#define ATLAS_REG_EC_CALIB_STATUS_HIGH BIT(3) >> + >> #define ATLAS_REG_PH_TEMP_DATA 0x0e >> #define ATLAS_REG_PH_DATA 0x16 >> >> +#define ATLAS_REG_EC_PROBE 0x08 >> +#define ATLAS_REG_EC_TEMP_DATA 0x10 >> +#define ATLAS_REG_EC_DATA 0x18 >> +#define ATLAS_REG_TDS_DATA 0x1c >> +#define ATLAS_REG_PSS_DATA 0x20 >> + >> #define ATLAS_PH_INT_TIME_IN_US 450000 >> +#define ATLAS_EC_INT_TIME_IN_US 650000 >> >> enum { >> ATLAS_PH_SM, >> + ATLAS_EC_SM, >> }; >> >> struct atlas_data { >> @@ -66,12 +81,13 @@ struct atlas_data { >> struct regmap *regmap; >> struct irq_work work; >> >> - __be32 buffer[4]; /* 32-bit pH data + 32-bit pad + 64-bit timestamp */ >> + __be32 buffer[6]; /* 96-bit data + 32-bit pad + 64-bit timestamp */ >> }; >> >> static const struct regmap_range atlas_volatile_ranges[] = { >> regmap_reg_range(ATLAS_REG_INT_CONTROL, ATLAS_REG_INT_CONTROL), >> regmap_reg_range(ATLAS_REG_PH_DATA, ATLAS_REG_PH_DATA + 4), >> + regmap_reg_range(ATLAS_REG_EC_DATA, ATLAS_REG_PSS_DATA + 4), >> }; >> >> static const struct regmap_access_table atlas_volatile_table = { >> @@ -86,7 +102,7 @@ static const struct regmap_config atlas_regmap_config = { >> .val_bits = 8, >> >> .volatile_table = &atlas_volatile_table, >> - .max_register = ATLAS_REG_PH_DATA + 4, >> + .max_register = ATLAS_REG_PSS_DATA + 4, >> .cache_type = REGCACHE_RBTREE, >> }; >> >> @@ -115,6 +131,38 @@ static const struct iio_chan_spec atlas_ph_channels[] = { >> }, >> }; >> >> +#define ATLAS_EC_CHANNEL(idx, addr) \ >> + {\ >> + .type = IIO_CONCENTRATION, \ >> + .indexed = 1, \ >> + .channel = idx, \ >> + .address = addr, \ >> + .info_mask_separate = \ >> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \ >> + .scan_index = idx, \ >> + .scan_type = { \ >> + .sign = 'u', \ >> + .realbits = 32, \ >> + .storagebits = 32, \ >> + .endianness = IIO_BE, \ >> + }, \ >> + } >> + >> +static const struct iio_chan_spec atlas_ec_channels[] = { >> + ATLAS_EC_CHANNEL(0, ATLAS_REG_EC_DATA), >> + ATLAS_EC_CHANNEL(1, ATLAS_REG_TDS_DATA), >> + ATLAS_EC_CHANNEL(2, ATLAS_REG_PSS_DATA), > This needs some explanation. 3 concentration channels of different types? > What are they... >> + IIO_CHAN_SOFT_TIMESTAMP(3), >> + { >> + .type = IIO_TEMP, >> + .address = ATLAS_REG_EC_TEMP_DATA, >> + .info_mask_separate = >> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >> + .output = 1, >> + .scan_index = -1 >> + }, >> +}; >> + >> static int atlas_check_ph_calibration(struct atlas_data *data) >> { >> struct device *dev = &data->client->dev; >> @@ -142,6 +190,44 @@ static int atlas_check_ph_calibration(struct atlas_data *data) >> return 0; >> } >> >> +static int atlas_check_ec_calibration(struct atlas_data *data) >> +{ >> + struct device *dev = &data->client->dev; >> + int ret; >> + unsigned int val; >> + >> + ret = regmap_bulk_read(data->regmap, ATLAS_REG_EC_PROBE, &val, 2); >> + if (ret) >> + return ret; >> + >> + dev_info(dev, "probe set to K = %d.%.2d", be16_to_cpu(val) / 100, >> + be16_to_cpu(val) % 100); >> + >> + ret = regmap_read(data->regmap, ATLAS_REG_EC_CALIB_STATUS, &val); >> + if (ret) >> + return ret; >> + >> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_MASK)) { >> + dev_warn(dev, "device has not been calibrated\n"); >> + return 0; >> + } >> + >> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_DRY)) >> + dev_warn(dev, "device missing dry point calibration\n"); >> + >> + if (val & ATLAS_REG_EC_CALIB_STATUS_SINGLE) { >> + dev_warn(dev, "device using single point calibration\n"); >> + } else { >> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_LOW)) >> + dev_warn(dev, "device missing low point calibration\n"); >> + >> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_HIGH)) >> + dev_warn(dev, "device missing high point calibration\n"); >> + } >> + >> + return 0; >> +} >> + >> struct atlas_device { >> const struct iio_chan_spec *channels; >> int num_channels; >> @@ -159,6 +245,14 @@ static struct atlas_device atlas_devices[] = { >> .calibration = &atlas_check_ph_calibration, >> .delay = ATLAS_PH_INT_TIME_IN_US, >> }, >> + [ATLAS_EC_SM] = { >> + .channels = atlas_ec_channels, >> + .num_channels = 5, > Use array_size - obviously correct that way so I don't have to go count... >> + .data_reg = ATLAS_REG_EC_DATA, >> + .calibration = &atlas_check_ec_calibration, >> + .delay = ATLAS_EC_INT_TIME_IN_US, >> + }, >> + >> }; >> >> static int atlas_set_powermode(struct atlas_data *data, int on) >> @@ -294,6 +388,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >> (u8 *) ®, sizeof(reg)); > I guess it's not complex enough to justify splitting read_raw for the two > devices. Close though so you may want to if you support a third type in here. > >> break; >> case IIO_PH: >> + case IIO_CONCENTRATION: >> mutex_lock(&indio_dev->mlock); >> >> if (iio_buffer_enabled(indio_dev)) >> @@ -324,6 +419,18 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >> *val = 1; /* 0.001 */ >> *val2 = 1000; >> break; >> + case IIO_CONCENTRATION: >> + switch (chan->address) { >> + case ATLAS_REG_EC_DATA: >> + *val = 0; /* 0.00000000064 */ >> + *val2 = 640; >> + return IIO_VAL_INT_PLUS_NANO; >> + default: >> + *val = 0; /* 0.000000001 */ >> + *val2 = 1000; >> + return IIO_VAL_INT_PLUS_NANO; >> + } >> + break; >> default: >> return -EINVAL; >> } >> @@ -517,12 +624,14 @@ static const struct dev_pm_ops atlas_pm_ops = { >> >> static const struct i2c_device_id atlas_id[] = { >> { "atlas-ph-sm", ATLAS_PH_SM}, >> + { "atlas-ec-sm", ATLAS_EC_SM}, >> {} >> }; >> MODULE_DEVICE_TABLE(i2c, atlas_id); >> >> static const struct of_device_id atlas_dt_ids[] = { >> { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, >> + { .compatible = "atlas,ec-sm", .data = (void *)ATLAS_EC_SM, }, >> { } >> }; >> MODULE_DEVICE_TABLE(of, atlas_dt_ids); >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature 2016-05-05 8:17 ` Matt Ranostay @ 2016-05-08 19:29 ` Jonathan Cameron 2016-05-11 2:35 ` Matt Ranostay 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Cameron @ 2016-05-08 19:29 UTC (permalink / raw) To: Matt Ranostay; +Cc: linux-iio On 05/05/16 09:17, Matt Ranostay wrote: > On Wed, May 4, 2016 at 3:39 AM, Jonathan Cameron <jic23@kernel.org> wrote: >> On 24/04/16 21:43, Matt Ranostay wrote: >> Give us a bit more description - what is it, what is it for, what interface is >> supplied. Err what does EC standard for? >>> Signed-off-by: Matt Ranostay <mranostay@gmail.com> >> I know next to nothing about these sensors and don't have the datasheet to hand. >> >> This is measuring 3 different types of concentration (I think) and reporting them >> simple as channels 0-2. What are they are should we not be distinguishing between >> them in some more informative fashion? > > Should I add some .extend_name for each channel? probably not. What are these 3 channels actually measuring? As I understand it from search the datasheet for tds... We have electric conductivity (that's not a concentration channel to my mind)... Perhaps have it as a new channel type in of itself... total dissolved solids (TDS) which kind of sounds concentration like to me... However, we might want to consider an appropriate modifier to say 'of what?'. PSS is salinity? Looks like this is a temperature adjusted concentration scale? (I'm reading wikipedia and not really following it... It might make sense to define a whole new channel type for this... I'm not keen. on having modifiers effect the units so channel type may be the only way to do it coherently.. I have this nasty feeling chemical measurements are going to introduce all sorts of weird ways of measuring 'concentrations' according to what actually happens to work for a given chemical.. Ah well. 27 channel types so far - bit of room left before we discover allowing 256 in the event descriptors wasn't thinking forwards enough... > Horribly I have to admit my pH sensor based board doesn't give valid > values for this sensor till I resign the ground plane under the IC :/. > But I can confirm it does report some "values"... :) > >> >> Code itself is mostly fine... >> >> Jonathan >>> --- >>> .../bindings/iio/chemical/atlas,ec-sm.txt | 22 ++++ >>> drivers/iio/chemical/Kconfig | 8 +- >>> drivers/iio/chemical/atlas-ph-sensor.c | 113 ++++++++++++++++++++- >>> 3 files changed, 138 insertions(+), 5 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>> >>> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>> new file mode 100644 >>> index 0000000..2962bd9 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>> @@ -0,0 +1,22 @@ >>> +* Atlas Scientific EC-SM OEM sensor >>> + >>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf >>> + >>> +Required properties: >>> + >>> + - compatible: must be "atlas,ec-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@64 { >>> + compatible = "atlas,ec-sm"; >>> + reg = <0x64>; >>> + interrupt-parent = <&gpio1>; >>> + interrupts = <16 2>; >>> +}; >>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig >>> index f73290f..4bcc025 100644 >>> --- a/drivers/iio/chemical/Kconfig >>> +++ b/drivers/iio/chemical/Kconfig >>> @@ -5,15 +5,17 @@ >>> menu "Chemical Sensors" >>> >>> config ATLAS_PH_SENSOR >>> - tristate "Atlas Scientific OEM pH-SM sensor" >>> + tristate "Atlas Scientific OEM SM sensors" >>> depends on I2C >>> select REGMAP_I2C >>> select IIO_BUFFER >>> select IIO_TRIGGERED_BUFFER >>> select IRQ_WORK >>> help >>> - Say Y here to build I2C interface support for the Atlas >>> - Scientific OEM pH-SM sensor. >>> + Say Y here to build I2C interface support for the following >>> + Atlas Scientific OEM SM sensors: >>> + * pH SM sensor >>> + * EC SM sensor >>> >>> To compile this driver as module, choose M here: the >>> module will be called atlas-ph-sensor. >>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c >>> index c57f9c2..6dddd75 100644 >>> --- a/drivers/iio/chemical/atlas-ph-sensor.c >>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c >>> @@ -50,13 +50,28 @@ >>> #define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) >>> #define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) >>> >>> +#define ATLAS_REG_EC_CALIB_STATUS 0x0f >>> +#define ATLAS_REG_EC_CALIB_STATUS_MASK 0x0f >>> +#define ATLAS_REG_EC_CALIB_STATUS_DRY BIT(0) >>> +#define ATLAS_REG_EC_CALIB_STATUS_SINGLE BIT(1) >>> +#define ATLAS_REG_EC_CALIB_STATUS_LOW BIT(2) >>> +#define ATLAS_REG_EC_CALIB_STATUS_HIGH BIT(3) >>> + >>> #define ATLAS_REG_PH_TEMP_DATA 0x0e >>> #define ATLAS_REG_PH_DATA 0x16 >>> >>> +#define ATLAS_REG_EC_PROBE 0x08 >>> +#define ATLAS_REG_EC_TEMP_DATA 0x10 >>> +#define ATLAS_REG_EC_DATA 0x18 >>> +#define ATLAS_REG_TDS_DATA 0x1c >>> +#define ATLAS_REG_PSS_DATA 0x20 >>> + >>> #define ATLAS_PH_INT_TIME_IN_US 450000 >>> +#define ATLAS_EC_INT_TIME_IN_US 650000 >>> >>> enum { >>> ATLAS_PH_SM, >>> + ATLAS_EC_SM, >>> }; >>> >>> struct atlas_data { >>> @@ -66,12 +81,13 @@ struct atlas_data { >>> struct regmap *regmap; >>> struct irq_work work; >>> >>> - __be32 buffer[4]; /* 32-bit pH data + 32-bit pad + 64-bit timestamp */ >>> + __be32 buffer[6]; /* 96-bit data + 32-bit pad + 64-bit timestamp */ >>> }; >>> >>> static const struct regmap_range atlas_volatile_ranges[] = { >>> regmap_reg_range(ATLAS_REG_INT_CONTROL, ATLAS_REG_INT_CONTROL), >>> regmap_reg_range(ATLAS_REG_PH_DATA, ATLAS_REG_PH_DATA + 4), >>> + regmap_reg_range(ATLAS_REG_EC_DATA, ATLAS_REG_PSS_DATA + 4), >>> }; >>> >>> static const struct regmap_access_table atlas_volatile_table = { >>> @@ -86,7 +102,7 @@ static const struct regmap_config atlas_regmap_config = { >>> .val_bits = 8, >>> >>> .volatile_table = &atlas_volatile_table, >>> - .max_register = ATLAS_REG_PH_DATA + 4, >>> + .max_register = ATLAS_REG_PSS_DATA + 4, >>> .cache_type = REGCACHE_RBTREE, >>> }; >>> >>> @@ -115,6 +131,38 @@ static const struct iio_chan_spec atlas_ph_channels[] = { >>> }, >>> }; >>> >>> +#define ATLAS_EC_CHANNEL(idx, addr) \ >>> + {\ >>> + .type = IIO_CONCENTRATION, \ >>> + .indexed = 1, \ >>> + .channel = idx, \ >>> + .address = addr, \ >>> + .info_mask_separate = \ >>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \ >>> + .scan_index = idx, \ >>> + .scan_type = { \ >>> + .sign = 'u', \ >>> + .realbits = 32, \ >>> + .storagebits = 32, \ >>> + .endianness = IIO_BE, \ >>> + }, \ >>> + } >>> + >>> +static const struct iio_chan_spec atlas_ec_channels[] = { >>> + ATLAS_EC_CHANNEL(0, ATLAS_REG_EC_DATA), >>> + ATLAS_EC_CHANNEL(1, ATLAS_REG_TDS_DATA), >>> + ATLAS_EC_CHANNEL(2, ATLAS_REG_PSS_DATA), >> This needs some explanation. 3 concentration channels of different types? >> What are they... >>> + IIO_CHAN_SOFT_TIMESTAMP(3), >>> + { >>> + .type = IIO_TEMP, >>> + .address = ATLAS_REG_EC_TEMP_DATA, >>> + .info_mask_separate = >>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >>> + .output = 1, >>> + .scan_index = -1 >>> + }, >>> +}; >>> + >>> static int atlas_check_ph_calibration(struct atlas_data *data) >>> { >>> struct device *dev = &data->client->dev; >>> @@ -142,6 +190,44 @@ static int atlas_check_ph_calibration(struct atlas_data *data) >>> return 0; >>> } >>> >>> +static int atlas_check_ec_calibration(struct atlas_data *data) >>> +{ >>> + struct device *dev = &data->client->dev; >>> + int ret; >>> + unsigned int val; >>> + >>> + ret = regmap_bulk_read(data->regmap, ATLAS_REG_EC_PROBE, &val, 2); >>> + if (ret) >>> + return ret; >>> + >>> + dev_info(dev, "probe set to K = %d.%.2d", be16_to_cpu(val) / 100, >>> + be16_to_cpu(val) % 100); >>> + >>> + ret = regmap_read(data->regmap, ATLAS_REG_EC_CALIB_STATUS, &val); >>> + if (ret) >>> + return ret; >>> + >>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_MASK)) { >>> + dev_warn(dev, "device has not been calibrated\n"); >>> + return 0; >>> + } >>> + >>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_DRY)) >>> + dev_warn(dev, "device missing dry point calibration\n"); >>> + >>> + if (val & ATLAS_REG_EC_CALIB_STATUS_SINGLE) { >>> + dev_warn(dev, "device using single point calibration\n"); >>> + } else { >>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_LOW)) >>> + dev_warn(dev, "device missing low point calibration\n"); >>> + >>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_HIGH)) >>> + dev_warn(dev, "device missing high point calibration\n"); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> struct atlas_device { >>> const struct iio_chan_spec *channels; >>> int num_channels; >>> @@ -159,6 +245,14 @@ static struct atlas_device atlas_devices[] = { >>> .calibration = &atlas_check_ph_calibration, >>> .delay = ATLAS_PH_INT_TIME_IN_US, >>> }, >>> + [ATLAS_EC_SM] = { >>> + .channels = atlas_ec_channels, >>> + .num_channels = 5, >> Use array_size - obviously correct that way so I don't have to go count... >>> + .data_reg = ATLAS_REG_EC_DATA, >>> + .calibration = &atlas_check_ec_calibration, >>> + .delay = ATLAS_EC_INT_TIME_IN_US, >>> + }, >>> + >>> }; >>> >>> static int atlas_set_powermode(struct atlas_data *data, int on) >>> @@ -294,6 +388,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>> (u8 *) ®, sizeof(reg)); >> I guess it's not complex enough to justify splitting read_raw for the two >> devices. Close though so you may want to if you support a third type in here. >> >>> break; >>> case IIO_PH: >>> + case IIO_CONCENTRATION: >>> mutex_lock(&indio_dev->mlock); >>> >>> if (iio_buffer_enabled(indio_dev)) >>> @@ -324,6 +419,18 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>> *val = 1; /* 0.001 */ >>> *val2 = 1000; >>> break; >>> + case IIO_CONCENTRATION: >>> + switch (chan->address) { >>> + case ATLAS_REG_EC_DATA: >>> + *val = 0; /* 0.00000000064 */ >>> + *val2 = 640; >>> + return IIO_VAL_INT_PLUS_NANO; >>> + default: >>> + *val = 0; /* 0.000000001 */ >>> + *val2 = 1000; >>> + return IIO_VAL_INT_PLUS_NANO; >>> + } >>> + break; >>> default: >>> return -EINVAL; >>> } >>> @@ -517,12 +624,14 @@ static const struct dev_pm_ops atlas_pm_ops = { >>> >>> static const struct i2c_device_id atlas_id[] = { >>> { "atlas-ph-sm", ATLAS_PH_SM}, >>> + { "atlas-ec-sm", ATLAS_EC_SM}, >>> {} >>> }; >>> MODULE_DEVICE_TABLE(i2c, atlas_id); >>> >>> static const struct of_device_id atlas_dt_ids[] = { >>> { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, >>> + { .compatible = "atlas,ec-sm", .data = (void *)ATLAS_EC_SM, }, >>> { } >>> }; >>> MODULE_DEVICE_TABLE(of, atlas_dt_ids); >>> >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature 2016-05-08 19:29 ` Jonathan Cameron @ 2016-05-11 2:35 ` Matt Ranostay 2016-05-14 17:02 ` Jonathan Cameron 0 siblings, 1 reply; 10+ messages in thread From: Matt Ranostay @ 2016-05-11 2:35 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio On Sun, May 8, 2016 at 12:29 PM, Jonathan Cameron <jic23@kernel.org> wrote: > On 05/05/16 09:17, Matt Ranostay wrote: >> On Wed, May 4, 2016 at 3:39 AM, Jonathan Cameron <jic23@kernel.org> wrote: >>> On 24/04/16 21:43, Matt Ranostay wrote: >>> Give us a bit more description - what is it, what is it for, what interface is >>> supplied. Err what does EC standard for? >>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com> >>> I know next to nothing about these sensors and don't have the datasheet to hand. >>> >>> This is measuring 3 different types of concentration (I think) and reporting them >>> simple as channels 0-2. What are they are should we not be distinguishing between >>> them in some more informative fashion? >> >> Should I add some .extend_name for each channel? > probably not. What are these 3 channels actually measuring? > As I understand it from search the datasheet for tds... > We have > > electric conductivity (that's not a concentration channel to my mind)... > Perhaps have it as a new channel type in of itself... Ok no problem of doing that. Just that I was using a known conversion value of EC (salts) to ppm. I have no issue with adding yet another weird iio chan type :) Would a IIO_EC make more sense than IIO_US_PER_CM (microseimens per centimeter)? The latter could be used by some meters in the future. > > total dissolved solids (TDS) which kind of sounds concentration like to me... > However, we might want to consider an appropriate modifier to say 'of what?'. > > PSS is salinity? Looks like this is a temperature adjusted concentration scale? > (I'm reading wikipedia and not really following it... Yeah that is correct.. like the pH sensor temperature affects the reading greatly. > > It might make sense to define a whole new channel type for this... I'm not keen. on having modifiers effect the units so channel type may be the only way to do it > coherently.. > > I have this nasty feeling chemical measurements are going to introduce all sorts > of weird ways of measuring 'concentrations' according to what actually happens > to work for a given chemical.. > > Ah well. 27 channel types so far - bit of room left before we discover allowing > 256 in the event descriptors wasn't thinking forwards enough... > >> Horribly I have to admit my pH sensor based board doesn't give valid >> values for this sensor till I resign the ground plane under the IC :/. >> But I can confirm it does report some "values"... > :) >> >>> >>> Code itself is mostly fine... >>> >>> Jonathan >>>> --- >>>> .../bindings/iio/chemical/atlas,ec-sm.txt | 22 ++++ >>>> drivers/iio/chemical/Kconfig | 8 +- >>>> drivers/iio/chemical/atlas-ph-sensor.c | 113 ++++++++++++++++++++- >>>> 3 files changed, 138 insertions(+), 5 deletions(-) >>>> create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>> new file mode 100644 >>>> index 0000000..2962bd9 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>> @@ -0,0 +1,22 @@ >>>> +* Atlas Scientific EC-SM OEM sensor >>>> + >>>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf >>>> + >>>> +Required properties: >>>> + >>>> + - compatible: must be "atlas,ec-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@64 { >>>> + compatible = "atlas,ec-sm"; >>>> + reg = <0x64>; >>>> + interrupt-parent = <&gpio1>; >>>> + interrupts = <16 2>; >>>> +}; >>>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig >>>> index f73290f..4bcc025 100644 >>>> --- a/drivers/iio/chemical/Kconfig >>>> +++ b/drivers/iio/chemical/Kconfig >>>> @@ -5,15 +5,17 @@ >>>> menu "Chemical Sensors" >>>> >>>> config ATLAS_PH_SENSOR >>>> - tristate "Atlas Scientific OEM pH-SM sensor" >>>> + tristate "Atlas Scientific OEM SM sensors" >>>> depends on I2C >>>> select REGMAP_I2C >>>> select IIO_BUFFER >>>> select IIO_TRIGGERED_BUFFER >>>> select IRQ_WORK >>>> help >>>> - Say Y here to build I2C interface support for the Atlas >>>> - Scientific OEM pH-SM sensor. >>>> + Say Y here to build I2C interface support for the following >>>> + Atlas Scientific OEM SM sensors: >>>> + * pH SM sensor >>>> + * EC SM sensor >>>> >>>> To compile this driver as module, choose M here: the >>>> module will be called atlas-ph-sensor. >>>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c >>>> index c57f9c2..6dddd75 100644 >>>> --- a/drivers/iio/chemical/atlas-ph-sensor.c >>>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c >>>> @@ -50,13 +50,28 @@ >>>> #define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) >>>> #define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) >>>> >>>> +#define ATLAS_REG_EC_CALIB_STATUS 0x0f >>>> +#define ATLAS_REG_EC_CALIB_STATUS_MASK 0x0f >>>> +#define ATLAS_REG_EC_CALIB_STATUS_DRY BIT(0) >>>> +#define ATLAS_REG_EC_CALIB_STATUS_SINGLE BIT(1) >>>> +#define ATLAS_REG_EC_CALIB_STATUS_LOW BIT(2) >>>> +#define ATLAS_REG_EC_CALIB_STATUS_HIGH BIT(3) >>>> + >>>> #define ATLAS_REG_PH_TEMP_DATA 0x0e >>>> #define ATLAS_REG_PH_DATA 0x16 >>>> >>>> +#define ATLAS_REG_EC_PROBE 0x08 >>>> +#define ATLAS_REG_EC_TEMP_DATA 0x10 >>>> +#define ATLAS_REG_EC_DATA 0x18 >>>> +#define ATLAS_REG_TDS_DATA 0x1c >>>> +#define ATLAS_REG_PSS_DATA 0x20 >>>> + >>>> #define ATLAS_PH_INT_TIME_IN_US 450000 >>>> +#define ATLAS_EC_INT_TIME_IN_US 650000 >>>> >>>> enum { >>>> ATLAS_PH_SM, >>>> + ATLAS_EC_SM, >>>> }; >>>> >>>> struct atlas_data { >>>> @@ -66,12 +81,13 @@ struct atlas_data { >>>> struct regmap *regmap; >>>> struct irq_work work; >>>> >>>> - __be32 buffer[4]; /* 32-bit pH data + 32-bit pad + 64-bit timestamp */ >>>> + __be32 buffer[6]; /* 96-bit data + 32-bit pad + 64-bit timestamp */ >>>> }; >>>> >>>> static const struct regmap_range atlas_volatile_ranges[] = { >>>> regmap_reg_range(ATLAS_REG_INT_CONTROL, ATLAS_REG_INT_CONTROL), >>>> regmap_reg_range(ATLAS_REG_PH_DATA, ATLAS_REG_PH_DATA + 4), >>>> + regmap_reg_range(ATLAS_REG_EC_DATA, ATLAS_REG_PSS_DATA + 4), >>>> }; >>>> >>>> static const struct regmap_access_table atlas_volatile_table = { >>>> @@ -86,7 +102,7 @@ static const struct regmap_config atlas_regmap_config = { >>>> .val_bits = 8, >>>> >>>> .volatile_table = &atlas_volatile_table, >>>> - .max_register = ATLAS_REG_PH_DATA + 4, >>>> + .max_register = ATLAS_REG_PSS_DATA + 4, >>>> .cache_type = REGCACHE_RBTREE, >>>> }; >>>> >>>> @@ -115,6 +131,38 @@ static const struct iio_chan_spec atlas_ph_channels[] = { >>>> }, >>>> }; >>>> >>>> +#define ATLAS_EC_CHANNEL(idx, addr) \ >>>> + {\ >>>> + .type = IIO_CONCENTRATION, \ >>>> + .indexed = 1, \ >>>> + .channel = idx, \ >>>> + .address = addr, \ >>>> + .info_mask_separate = \ >>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \ >>>> + .scan_index = idx, \ >>>> + .scan_type = { \ >>>> + .sign = 'u', \ >>>> + .realbits = 32, \ >>>> + .storagebits = 32, \ >>>> + .endianness = IIO_BE, \ >>>> + }, \ >>>> + } >>>> + >>>> +static const struct iio_chan_spec atlas_ec_channels[] = { >>>> + ATLAS_EC_CHANNEL(0, ATLAS_REG_EC_DATA), >>>> + ATLAS_EC_CHANNEL(1, ATLAS_REG_TDS_DATA), >>>> + ATLAS_EC_CHANNEL(2, ATLAS_REG_PSS_DATA), >>> This needs some explanation. 3 concentration channels of different types? >>> What are they... >>>> + IIO_CHAN_SOFT_TIMESTAMP(3), >>>> + { >>>> + .type = IIO_TEMP, >>>> + .address = ATLAS_REG_EC_TEMP_DATA, >>>> + .info_mask_separate = >>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >>>> + .output = 1, >>>> + .scan_index = -1 >>>> + }, >>>> +}; >>>> + >>>> static int atlas_check_ph_calibration(struct atlas_data *data) >>>> { >>>> struct device *dev = &data->client->dev; >>>> @@ -142,6 +190,44 @@ static int atlas_check_ph_calibration(struct atlas_data *data) >>>> return 0; >>>> } >>>> >>>> +static int atlas_check_ec_calibration(struct atlas_data *data) >>>> +{ >>>> + struct device *dev = &data->client->dev; >>>> + int ret; >>>> + unsigned int val; >>>> + >>>> + ret = regmap_bulk_read(data->regmap, ATLAS_REG_EC_PROBE, &val, 2); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + dev_info(dev, "probe set to K = %d.%.2d", be16_to_cpu(val) / 100, >>>> + be16_to_cpu(val) % 100); >>>> + >>>> + ret = regmap_read(data->regmap, ATLAS_REG_EC_CALIB_STATUS, &val); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_MASK)) { >>>> + dev_warn(dev, "device has not been calibrated\n"); >>>> + return 0; >>>> + } >>>> + >>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_DRY)) >>>> + dev_warn(dev, "device missing dry point calibration\n"); >>>> + >>>> + if (val & ATLAS_REG_EC_CALIB_STATUS_SINGLE) { >>>> + dev_warn(dev, "device using single point calibration\n"); >>>> + } else { >>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_LOW)) >>>> + dev_warn(dev, "device missing low point calibration\n"); >>>> + >>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_HIGH)) >>>> + dev_warn(dev, "device missing high point calibration\n"); >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> struct atlas_device { >>>> const struct iio_chan_spec *channels; >>>> int num_channels; >>>> @@ -159,6 +245,14 @@ static struct atlas_device atlas_devices[] = { >>>> .calibration = &atlas_check_ph_calibration, >>>> .delay = ATLAS_PH_INT_TIME_IN_US, >>>> }, >>>> + [ATLAS_EC_SM] = { >>>> + .channels = atlas_ec_channels, >>>> + .num_channels = 5, >>> Use array_size - obviously correct that way so I don't have to go count... >>>> + .data_reg = ATLAS_REG_EC_DATA, >>>> + .calibration = &atlas_check_ec_calibration, >>>> + .delay = ATLAS_EC_INT_TIME_IN_US, >>>> + }, >>>> + >>>> }; >>>> >>>> static int atlas_set_powermode(struct atlas_data *data, int on) >>>> @@ -294,6 +388,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>>> (u8 *) ®, sizeof(reg)); >>> I guess it's not complex enough to justify splitting read_raw for the two >>> devices. Close though so you may want to if you support a third type in here. >>> >>>> break; >>>> case IIO_PH: >>>> + case IIO_CONCENTRATION: >>>> mutex_lock(&indio_dev->mlock); >>>> >>>> if (iio_buffer_enabled(indio_dev)) >>>> @@ -324,6 +419,18 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>>> *val = 1; /* 0.001 */ >>>> *val2 = 1000; >>>> break; >>>> + case IIO_CONCENTRATION: >>>> + switch (chan->address) { >>>> + case ATLAS_REG_EC_DATA: >>>> + *val = 0; /* 0.00000000064 */ >>>> + *val2 = 640; >>>> + return IIO_VAL_INT_PLUS_NANO; >>>> + default: >>>> + *val = 0; /* 0.000000001 */ >>>> + *val2 = 1000; >>>> + return IIO_VAL_INT_PLUS_NANO; >>>> + } >>>> + break; >>>> default: >>>> return -EINVAL; >>>> } >>>> @@ -517,12 +624,14 @@ static const struct dev_pm_ops atlas_pm_ops = { >>>> >>>> static const struct i2c_device_id atlas_id[] = { >>>> { "atlas-ph-sm", ATLAS_PH_SM}, >>>> + { "atlas-ec-sm", ATLAS_EC_SM}, >>>> {} >>>> }; >>>> MODULE_DEVICE_TABLE(i2c, atlas_id); >>>> >>>> static const struct of_device_id atlas_dt_ids[] = { >>>> { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, >>>> + { .compatible = "atlas,ec-sm", .data = (void *)ATLAS_EC_SM, }, >>>> { } >>>> }; >>>> MODULE_DEVICE_TABLE(of, atlas_dt_ids); >>>> >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature 2016-05-11 2:35 ` Matt Ranostay @ 2016-05-14 17:02 ` Jonathan Cameron 2016-05-19 5:43 ` Matt Ranostay 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Cameron @ 2016-05-14 17:02 UTC (permalink / raw) To: Matt Ranostay; +Cc: linux-iio On 11/05/16 03:35, Matt Ranostay wrote: > On Sun, May 8, 2016 at 12:29 PM, Jonathan Cameron <jic23@kernel.org> wrote: >> On 05/05/16 09:17, Matt Ranostay wrote: >>> On Wed, May 4, 2016 at 3:39 AM, Jonathan Cameron <jic23@kernel.org> wrote: >>>> On 24/04/16 21:43, Matt Ranostay wrote: >>>> Give us a bit more description - what is it, what is it for, what interface is >>>> supplied. Err what does EC standard for? >>>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com> >>>> I know next to nothing about these sensors and don't have the datasheet to hand. >>>> >>>> This is measuring 3 different types of concentration (I think) and reporting them >>>> simple as channels 0-2. What are they are should we not be distinguishing between >>>> them in some more informative fashion? >>> >>> Should I add some .extend_name for each channel? >> probably not. What are these 3 channels actually measuring? >> As I understand it from search the datasheet for tds... >> We have >> >> electric conductivity (that's not a concentration channel to my mind)... >> Perhaps have it as a new channel type in of itself... > Ok no problem of doing that. Just that I was using a known conversion > value of EC (salts) to ppm. > I have no issue with adding yet another weird iio chan type :) > > Would a IIO_EC make more sense than IIO_US_PER_CM (microseimens per > centimeter)? The latter could be used by some meters in the future. Not a clue! Unless someone else chimes in I'm happy to go with whatever you recommend - the more generic the better. Though seimens/m would probably be better. The places we let in non 'base' units were generally to match hwmon - a decision I now regret as it means people have to read the docs rather than 'knowing' what the units will be. > >> >> total dissolved solids (TDS) which kind of sounds concentration like to me... >> However, we might want to consider an appropriate modifier to say 'of what?'. >> >> PSS is salinity? Looks like this is a temperature adjusted concentration scale? >> (I'm reading wikipedia and not really following it... > Yeah that is correct.. like the pH sensor temperature affects the > reading greatly. > >> >> It might make sense to define a whole new channel type for this... I'm not keen. on having modifiers effect the units so channel type may be the only way to do it >> coherently.. >> >> I have this nasty feeling chemical measurements are going to introduce all sorts >> of weird ways of measuring 'concentrations' according to what actually happens >> to work for a given chemical.. >> >> Ah well. 27 channel types so far - bit of room left before we discover allowing >> 256 in the event descriptors wasn't thinking forwards enough... >> >>> Horribly I have to admit my pH sensor based board doesn't give valid >>> values for this sensor till I resign the ground plane under the IC :/. >>> But I can confirm it does report some "values"... >> :) >>> >>>> >>>> Code itself is mostly fine... >>>> >>>> Jonathan >>>>> --- >>>>> .../bindings/iio/chemical/atlas,ec-sm.txt | 22 ++++ >>>>> drivers/iio/chemical/Kconfig | 8 +- >>>>> drivers/iio/chemical/atlas-ph-sensor.c | 113 ++++++++++++++++++++- >>>>> 3 files changed, 138 insertions(+), 5 deletions(-) >>>>> create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>>> new file mode 100644 >>>>> index 0000000..2962bd9 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>>> @@ -0,0 +1,22 @@ >>>>> +* Atlas Scientific EC-SM OEM sensor >>>>> + >>>>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf >>>>> + >>>>> +Required properties: >>>>> + >>>>> + - compatible: must be "atlas,ec-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@64 { >>>>> + compatible = "atlas,ec-sm"; >>>>> + reg = <0x64>; >>>>> + interrupt-parent = <&gpio1>; >>>>> + interrupts = <16 2>; >>>>> +}; >>>>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig >>>>> index f73290f..4bcc025 100644 >>>>> --- a/drivers/iio/chemical/Kconfig >>>>> +++ b/drivers/iio/chemical/Kconfig >>>>> @@ -5,15 +5,17 @@ >>>>> menu "Chemical Sensors" >>>>> >>>>> config ATLAS_PH_SENSOR >>>>> - tristate "Atlas Scientific OEM pH-SM sensor" >>>>> + tristate "Atlas Scientific OEM SM sensors" >>>>> depends on I2C >>>>> select REGMAP_I2C >>>>> select IIO_BUFFER >>>>> select IIO_TRIGGERED_BUFFER >>>>> select IRQ_WORK >>>>> help >>>>> - Say Y here to build I2C interface support for the Atlas >>>>> - Scientific OEM pH-SM sensor. >>>>> + Say Y here to build I2C interface support for the following >>>>> + Atlas Scientific OEM SM sensors: >>>>> + * pH SM sensor >>>>> + * EC SM sensor >>>>> >>>>> To compile this driver as module, choose M here: the >>>>> module will be called atlas-ph-sensor. >>>>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c >>>>> index c57f9c2..6dddd75 100644 >>>>> --- a/drivers/iio/chemical/atlas-ph-sensor.c >>>>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c >>>>> @@ -50,13 +50,28 @@ >>>>> #define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) >>>>> #define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) >>>>> >>>>> +#define ATLAS_REG_EC_CALIB_STATUS 0x0f >>>>> +#define ATLAS_REG_EC_CALIB_STATUS_MASK 0x0f >>>>> +#define ATLAS_REG_EC_CALIB_STATUS_DRY BIT(0) >>>>> +#define ATLAS_REG_EC_CALIB_STATUS_SINGLE BIT(1) >>>>> +#define ATLAS_REG_EC_CALIB_STATUS_LOW BIT(2) >>>>> +#define ATLAS_REG_EC_CALIB_STATUS_HIGH BIT(3) >>>>> + >>>>> #define ATLAS_REG_PH_TEMP_DATA 0x0e >>>>> #define ATLAS_REG_PH_DATA 0x16 >>>>> >>>>> +#define ATLAS_REG_EC_PROBE 0x08 >>>>> +#define ATLAS_REG_EC_TEMP_DATA 0x10 >>>>> +#define ATLAS_REG_EC_DATA 0x18 >>>>> +#define ATLAS_REG_TDS_DATA 0x1c >>>>> +#define ATLAS_REG_PSS_DATA 0x20 >>>>> + >>>>> #define ATLAS_PH_INT_TIME_IN_US 450000 >>>>> +#define ATLAS_EC_INT_TIME_IN_US 650000 >>>>> >>>>> enum { >>>>> ATLAS_PH_SM, >>>>> + ATLAS_EC_SM, >>>>> }; >>>>> >>>>> struct atlas_data { >>>>> @@ -66,12 +81,13 @@ struct atlas_data { >>>>> struct regmap *regmap; >>>>> struct irq_work work; >>>>> >>>>> - __be32 buffer[4]; /* 32-bit pH data + 32-bit pad + 64-bit timestamp */ >>>>> + __be32 buffer[6]; /* 96-bit data + 32-bit pad + 64-bit timestamp */ >>>>> }; >>>>> >>>>> static const struct regmap_range atlas_volatile_ranges[] = { >>>>> regmap_reg_range(ATLAS_REG_INT_CONTROL, ATLAS_REG_INT_CONTROL), >>>>> regmap_reg_range(ATLAS_REG_PH_DATA, ATLAS_REG_PH_DATA + 4), >>>>> + regmap_reg_range(ATLAS_REG_EC_DATA, ATLAS_REG_PSS_DATA + 4), >>>>> }; >>>>> >>>>> static const struct regmap_access_table atlas_volatile_table = { >>>>> @@ -86,7 +102,7 @@ static const struct regmap_config atlas_regmap_config = { >>>>> .val_bits = 8, >>>>> >>>>> .volatile_table = &atlas_volatile_table, >>>>> - .max_register = ATLAS_REG_PH_DATA + 4, >>>>> + .max_register = ATLAS_REG_PSS_DATA + 4, >>>>> .cache_type = REGCACHE_RBTREE, >>>>> }; >>>>> >>>>> @@ -115,6 +131,38 @@ static const struct iio_chan_spec atlas_ph_channels[] = { >>>>> }, >>>>> }; >>>>> >>>>> +#define ATLAS_EC_CHANNEL(idx, addr) \ >>>>> + {\ >>>>> + .type = IIO_CONCENTRATION, \ >>>>> + .indexed = 1, \ >>>>> + .channel = idx, \ >>>>> + .address = addr, \ >>>>> + .info_mask_separate = \ >>>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \ >>>>> + .scan_index = idx, \ >>>>> + .scan_type = { \ >>>>> + .sign = 'u', \ >>>>> + .realbits = 32, \ >>>>> + .storagebits = 32, \ >>>>> + .endianness = IIO_BE, \ >>>>> + }, \ >>>>> + } >>>>> + >>>>> +static const struct iio_chan_spec atlas_ec_channels[] = { >>>>> + ATLAS_EC_CHANNEL(0, ATLAS_REG_EC_DATA), >>>>> + ATLAS_EC_CHANNEL(1, ATLAS_REG_TDS_DATA), >>>>> + ATLAS_EC_CHANNEL(2, ATLAS_REG_PSS_DATA), >>>> This needs some explanation. 3 concentration channels of different types? >>>> What are they... >>>>> + IIO_CHAN_SOFT_TIMESTAMP(3), >>>>> + { >>>>> + .type = IIO_TEMP, >>>>> + .address = ATLAS_REG_EC_TEMP_DATA, >>>>> + .info_mask_separate = >>>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >>>>> + .output = 1, >>>>> + .scan_index = -1 >>>>> + }, >>>>> +}; >>>>> + >>>>> static int atlas_check_ph_calibration(struct atlas_data *data) >>>>> { >>>>> struct device *dev = &data->client->dev; >>>>> @@ -142,6 +190,44 @@ static int atlas_check_ph_calibration(struct atlas_data *data) >>>>> return 0; >>>>> } >>>>> >>>>> +static int atlas_check_ec_calibration(struct atlas_data *data) >>>>> +{ >>>>> + struct device *dev = &data->client->dev; >>>>> + int ret; >>>>> + unsigned int val; >>>>> + >>>>> + ret = regmap_bulk_read(data->regmap, ATLAS_REG_EC_PROBE, &val, 2); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + dev_info(dev, "probe set to K = %d.%.2d", be16_to_cpu(val) / 100, >>>>> + be16_to_cpu(val) % 100); >>>>> + >>>>> + ret = regmap_read(data->regmap, ATLAS_REG_EC_CALIB_STATUS, &val); >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_MASK)) { >>>>> + dev_warn(dev, "device has not been calibrated\n"); >>>>> + return 0; >>>>> + } >>>>> + >>>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_DRY)) >>>>> + dev_warn(dev, "device missing dry point calibration\n"); >>>>> + >>>>> + if (val & ATLAS_REG_EC_CALIB_STATUS_SINGLE) { >>>>> + dev_warn(dev, "device using single point calibration\n"); >>>>> + } else { >>>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_LOW)) >>>>> + dev_warn(dev, "device missing low point calibration\n"); >>>>> + >>>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_HIGH)) >>>>> + dev_warn(dev, "device missing high point calibration\n"); >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> struct atlas_device { >>>>> const struct iio_chan_spec *channels; >>>>> int num_channels; >>>>> @@ -159,6 +245,14 @@ static struct atlas_device atlas_devices[] = { >>>>> .calibration = &atlas_check_ph_calibration, >>>>> .delay = ATLAS_PH_INT_TIME_IN_US, >>>>> }, >>>>> + [ATLAS_EC_SM] = { >>>>> + .channels = atlas_ec_channels, >>>>> + .num_channels = 5, >>>> Use array_size - obviously correct that way so I don't have to go count... >>>>> + .data_reg = ATLAS_REG_EC_DATA, >>>>> + .calibration = &atlas_check_ec_calibration, >>>>> + .delay = ATLAS_EC_INT_TIME_IN_US, >>>>> + }, >>>>> + >>>>> }; >>>>> >>>>> static int atlas_set_powermode(struct atlas_data *data, int on) >>>>> @@ -294,6 +388,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>>>> (u8 *) ®, sizeof(reg)); >>>> I guess it's not complex enough to justify splitting read_raw for the two >>>> devices. Close though so you may want to if you support a third type in here. >>>> >>>>> break; >>>>> case IIO_PH: >>>>> + case IIO_CONCENTRATION: >>>>> mutex_lock(&indio_dev->mlock); >>>>> >>>>> if (iio_buffer_enabled(indio_dev)) >>>>> @@ -324,6 +419,18 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>>>> *val = 1; /* 0.001 */ >>>>> *val2 = 1000; >>>>> break; >>>>> + case IIO_CONCENTRATION: >>>>> + switch (chan->address) { >>>>> + case ATLAS_REG_EC_DATA: >>>>> + *val = 0; /* 0.00000000064 */ >>>>> + *val2 = 640; >>>>> + return IIO_VAL_INT_PLUS_NANO; >>>>> + default: >>>>> + *val = 0; /* 0.000000001 */ >>>>> + *val2 = 1000; >>>>> + return IIO_VAL_INT_PLUS_NANO; >>>>> + } >>>>> + break; >>>>> default: >>>>> return -EINVAL; >>>>> } >>>>> @@ -517,12 +624,14 @@ static const struct dev_pm_ops atlas_pm_ops = { >>>>> >>>>> static const struct i2c_device_id atlas_id[] = { >>>>> { "atlas-ph-sm", ATLAS_PH_SM}, >>>>> + { "atlas-ec-sm", ATLAS_EC_SM}, >>>>> {} >>>>> }; >>>>> MODULE_DEVICE_TABLE(i2c, atlas_id); >>>>> >>>>> static const struct of_device_id atlas_dt_ids[] = { >>>>> { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, >>>>> + { .compatible = "atlas,ec-sm", .data = (void *)ATLAS_EC_SM, }, >>>>> { } >>>>> }; >>>>> MODULE_DEVICE_TABLE(of, atlas_dt_ids); >>>>> >>>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature 2016-05-14 17:02 ` Jonathan Cameron @ 2016-05-19 5:43 ` Matt Ranostay 0 siblings, 0 replies; 10+ messages in thread From: Matt Ranostay @ 2016-05-19 5:43 UTC (permalink / raw) To: Jonathan Cameron; +Cc: linux-iio On Sat, May 14, 2016 at 10:02 AM, Jonathan Cameron <jic23@kernel.org> wrote: > On 11/05/16 03:35, Matt Ranostay wrote: >> On Sun, May 8, 2016 at 12:29 PM, Jonathan Cameron <jic23@kernel.org> wrote: >>> On 05/05/16 09:17, Matt Ranostay wrote: >>>> On Wed, May 4, 2016 at 3:39 AM, Jonathan Cameron <jic23@kernel.org> wrote: >>>>> On 24/04/16 21:43, Matt Ranostay wrote: >>>>> Give us a bit more description - what is it, what is it for, what interface is >>>>> supplied. Err what does EC standard for? >>>>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com> >>>>> I know next to nothing about these sensors and don't have the datasheet to hand. >>>>> >>>>> This is measuring 3 different types of concentration (I think) and reporting them >>>>> simple as channels 0-2. What are they are should we not be distinguishing between >>>>> them in some more informative fashion? >>>> >>>> Should I add some .extend_name for each channel? >>> probably not. What are these 3 channels actually measuring? >>> As I understand it from search the datasheet for tds... >>> We have >>> >>> electric conductivity (that's not a concentration channel to my mind)... >>> Perhaps have it as a new channel type in of itself... >> Ok no problem of doing that. Just that I was using a known conversion >> value of EC (salts) to ppm. >> I have no issue with adding yet another weird iio chan type :) >> >> Would a IIO_EC make more sense than IIO_US_PER_CM (microseimens per >> centimeter)? The latter could be used by some meters in the future. > Not a clue! Unless someone else chimes in I'm happy to go with whatever you > recommend - the more generic the better. Though seimens/m would probably > be better. The places we let in non 'base' units were generally to match > hwmon - a decision I now regret as it means people have to read the docs > rather than 'knowing' what the units will be. Make one mistake in the ABI and you be supporting it for years. :) >> >>> >>> total dissolved solids (TDS) which kind of sounds concentration like to me... >>> However, we might want to consider an appropriate modifier to say 'of what?'. >>> >>> PSS is salinity? Looks like this is a temperature adjusted concentration scale? >>> (I'm reading wikipedia and not really following it... >> Yeah that is correct.. like the pH sensor temperature affects the >> reading greatly. >> >>> >>> It might make sense to define a whole new channel type for this... I'm not keen. on having modifiers effect the units so channel type may be the only way to do it >>> coherently.. >>> >>> I have this nasty feeling chemical measurements are going to introduce all sorts >>> of weird ways of measuring 'concentrations' according to what actually happens >>> to work for a given chemical.. >>> >>> Ah well. 27 channel types so far - bit of room left before we discover allowing >>> 256 in the event descriptors wasn't thinking forwards enough... >>> >>>> Horribly I have to admit my pH sensor based board doesn't give valid >>>> values for this sensor till I resign the ground plane under the IC :/. >>>> But I can confirm it does report some "values"... >>> :) >>>> >>>>> >>>>> Code itself is mostly fine... >>>>> >>>>> Jonathan >>>>>> --- >>>>>> .../bindings/iio/chemical/atlas,ec-sm.txt | 22 ++++ >>>>>> drivers/iio/chemical/Kconfig | 8 +- >>>>>> drivers/iio/chemical/atlas-ph-sensor.c | 113 ++++++++++++++++++++- >>>>>> 3 files changed, 138 insertions(+), 5 deletions(-) >>>>>> create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>>>> new file mode 100644 >>>>>> index 0000000..2962bd9 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt >>>>>> @@ -0,0 +1,22 @@ >>>>>> +* Atlas Scientific EC-SM OEM sensor >>>>>> + >>>>>> +http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf >>>>>> + >>>>>> +Required properties: >>>>>> + >>>>>> + - compatible: must be "atlas,ec-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@64 { >>>>>> + compatible = "atlas,ec-sm"; >>>>>> + reg = <0x64>; >>>>>> + interrupt-parent = <&gpio1>; >>>>>> + interrupts = <16 2>; >>>>>> +}; >>>>>> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig >>>>>> index f73290f..4bcc025 100644 >>>>>> --- a/drivers/iio/chemical/Kconfig >>>>>> +++ b/drivers/iio/chemical/Kconfig >>>>>> @@ -5,15 +5,17 @@ >>>>>> menu "Chemical Sensors" >>>>>> >>>>>> config ATLAS_PH_SENSOR >>>>>> - tristate "Atlas Scientific OEM pH-SM sensor" >>>>>> + tristate "Atlas Scientific OEM SM sensors" >>>>>> depends on I2C >>>>>> select REGMAP_I2C >>>>>> select IIO_BUFFER >>>>>> select IIO_TRIGGERED_BUFFER >>>>>> select IRQ_WORK >>>>>> help >>>>>> - Say Y here to build I2C interface support for the Atlas >>>>>> - Scientific OEM pH-SM sensor. >>>>>> + Say Y here to build I2C interface support for the following >>>>>> + Atlas Scientific OEM SM sensors: >>>>>> + * pH SM sensor >>>>>> + * EC SM sensor >>>>>> >>>>>> To compile this driver as module, choose M here: the >>>>>> module will be called atlas-ph-sensor. >>>>>> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c >>>>>> index c57f9c2..6dddd75 100644 >>>>>> --- a/drivers/iio/chemical/atlas-ph-sensor.c >>>>>> +++ b/drivers/iio/chemical/atlas-ph-sensor.c >>>>>> @@ -50,13 +50,28 @@ >>>>>> #define ATLAS_REG_PH_CALIB_STATUS_MID BIT(1) >>>>>> #define ATLAS_REG_PH_CALIB_STATUS_HIGH BIT(2) >>>>>> >>>>>> +#define ATLAS_REG_EC_CALIB_STATUS 0x0f >>>>>> +#define ATLAS_REG_EC_CALIB_STATUS_MASK 0x0f >>>>>> +#define ATLAS_REG_EC_CALIB_STATUS_DRY BIT(0) >>>>>> +#define ATLAS_REG_EC_CALIB_STATUS_SINGLE BIT(1) >>>>>> +#define ATLAS_REG_EC_CALIB_STATUS_LOW BIT(2) >>>>>> +#define ATLAS_REG_EC_CALIB_STATUS_HIGH BIT(3) >>>>>> + >>>>>> #define ATLAS_REG_PH_TEMP_DATA 0x0e >>>>>> #define ATLAS_REG_PH_DATA 0x16 >>>>>> >>>>>> +#define ATLAS_REG_EC_PROBE 0x08 >>>>>> +#define ATLAS_REG_EC_TEMP_DATA 0x10 >>>>>> +#define ATLAS_REG_EC_DATA 0x18 >>>>>> +#define ATLAS_REG_TDS_DATA 0x1c >>>>>> +#define ATLAS_REG_PSS_DATA 0x20 >>>>>> + >>>>>> #define ATLAS_PH_INT_TIME_IN_US 450000 >>>>>> +#define ATLAS_EC_INT_TIME_IN_US 650000 >>>>>> >>>>>> enum { >>>>>> ATLAS_PH_SM, >>>>>> + ATLAS_EC_SM, >>>>>> }; >>>>>> >>>>>> struct atlas_data { >>>>>> @@ -66,12 +81,13 @@ struct atlas_data { >>>>>> struct regmap *regmap; >>>>>> struct irq_work work; >>>>>> >>>>>> - __be32 buffer[4]; /* 32-bit pH data + 32-bit pad + 64-bit timestamp */ >>>>>> + __be32 buffer[6]; /* 96-bit data + 32-bit pad + 64-bit timestamp */ >>>>>> }; >>>>>> >>>>>> static const struct regmap_range atlas_volatile_ranges[] = { >>>>>> regmap_reg_range(ATLAS_REG_INT_CONTROL, ATLAS_REG_INT_CONTROL), >>>>>> regmap_reg_range(ATLAS_REG_PH_DATA, ATLAS_REG_PH_DATA + 4), >>>>>> + regmap_reg_range(ATLAS_REG_EC_DATA, ATLAS_REG_PSS_DATA + 4), >>>>>> }; >>>>>> >>>>>> static const struct regmap_access_table atlas_volatile_table = { >>>>>> @@ -86,7 +102,7 @@ static const struct regmap_config atlas_regmap_config = { >>>>>> .val_bits = 8, >>>>>> >>>>>> .volatile_table = &atlas_volatile_table, >>>>>> - .max_register = ATLAS_REG_PH_DATA + 4, >>>>>> + .max_register = ATLAS_REG_PSS_DATA + 4, >>>>>> .cache_type = REGCACHE_RBTREE, >>>>>> }; >>>>>> >>>>>> @@ -115,6 +131,38 @@ static const struct iio_chan_spec atlas_ph_channels[] = { >>>>>> }, >>>>>> }; >>>>>> >>>>>> +#define ATLAS_EC_CHANNEL(idx, addr) \ >>>>>> + {\ >>>>>> + .type = IIO_CONCENTRATION, \ >>>>>> + .indexed = 1, \ >>>>>> + .channel = idx, \ >>>>>> + .address = addr, \ >>>>>> + .info_mask_separate = \ >>>>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), \ >>>>>> + .scan_index = idx, \ >>>>>> + .scan_type = { \ >>>>>> + .sign = 'u', \ >>>>>> + .realbits = 32, \ >>>>>> + .storagebits = 32, \ >>>>>> + .endianness = IIO_BE, \ >>>>>> + }, \ >>>>>> + } >>>>>> + >>>>>> +static const struct iio_chan_spec atlas_ec_channels[] = { >>>>>> + ATLAS_EC_CHANNEL(0, ATLAS_REG_EC_DATA), >>>>>> + ATLAS_EC_CHANNEL(1, ATLAS_REG_TDS_DATA), >>>>>> + ATLAS_EC_CHANNEL(2, ATLAS_REG_PSS_DATA), >>>>> This needs some explanation. 3 concentration channels of different types? >>>>> What are they... >>>>>> + IIO_CHAN_SOFT_TIMESTAMP(3), >>>>>> + { >>>>>> + .type = IIO_TEMP, >>>>>> + .address = ATLAS_REG_EC_TEMP_DATA, >>>>>> + .info_mask_separate = >>>>>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >>>>>> + .output = 1, >>>>>> + .scan_index = -1 >>>>>> + }, >>>>>> +}; >>>>>> + >>>>>> static int atlas_check_ph_calibration(struct atlas_data *data) >>>>>> { >>>>>> struct device *dev = &data->client->dev; >>>>>> @@ -142,6 +190,44 @@ static int atlas_check_ph_calibration(struct atlas_data *data) >>>>>> return 0; >>>>>> } >>>>>> >>>>>> +static int atlas_check_ec_calibration(struct atlas_data *data) >>>>>> +{ >>>>>> + struct device *dev = &data->client->dev; >>>>>> + int ret; >>>>>> + unsigned int val; >>>>>> + >>>>>> + ret = regmap_bulk_read(data->regmap, ATLAS_REG_EC_PROBE, &val, 2); >>>>>> + if (ret) >>>>>> + return ret; >>>>>> + >>>>>> + dev_info(dev, "probe set to K = %d.%.2d", be16_to_cpu(val) / 100, >>>>>> + be16_to_cpu(val) % 100); >>>>>> + >>>>>> + ret = regmap_read(data->regmap, ATLAS_REG_EC_CALIB_STATUS, &val); >>>>>> + if (ret) >>>>>> + return ret; >>>>>> + >>>>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_MASK)) { >>>>>> + dev_warn(dev, "device has not been calibrated\n"); >>>>>> + return 0; >>>>>> + } >>>>>> + >>>>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_DRY)) >>>>>> + dev_warn(dev, "device missing dry point calibration\n"); >>>>>> + >>>>>> + if (val & ATLAS_REG_EC_CALIB_STATUS_SINGLE) { >>>>>> + dev_warn(dev, "device using single point calibration\n"); >>>>>> + } else { >>>>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_LOW)) >>>>>> + dev_warn(dev, "device missing low point calibration\n"); >>>>>> + >>>>>> + if (!(val & ATLAS_REG_EC_CALIB_STATUS_HIGH)) >>>>>> + dev_warn(dev, "device missing high point calibration\n"); >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> struct atlas_device { >>>>>> const struct iio_chan_spec *channels; >>>>>> int num_channels; >>>>>> @@ -159,6 +245,14 @@ static struct atlas_device atlas_devices[] = { >>>>>> .calibration = &atlas_check_ph_calibration, >>>>>> .delay = ATLAS_PH_INT_TIME_IN_US, >>>>>> }, >>>>>> + [ATLAS_EC_SM] = { >>>>>> + .channels = atlas_ec_channels, >>>>>> + .num_channels = 5, >>>>> Use array_size - obviously correct that way so I don't have to go count... >>>>>> + .data_reg = ATLAS_REG_EC_DATA, >>>>>> + .calibration = &atlas_check_ec_calibration, >>>>>> + .delay = ATLAS_EC_INT_TIME_IN_US, >>>>>> + }, >>>>>> + >>>>>> }; >>>>>> >>>>>> static int atlas_set_powermode(struct atlas_data *data, int on) >>>>>> @@ -294,6 +388,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>>>>> (u8 *) ®, sizeof(reg)); >>>>> I guess it's not complex enough to justify splitting read_raw for the two >>>>> devices. Close though so you may want to if you support a third type in here. >>>>> >>>>>> break; >>>>>> case IIO_PH: >>>>>> + case IIO_CONCENTRATION: >>>>>> mutex_lock(&indio_dev->mlock); >>>>>> >>>>>> if (iio_buffer_enabled(indio_dev)) >>>>>> @@ -324,6 +419,18 @@ static int atlas_read_raw(struct iio_dev *indio_dev, >>>>>> *val = 1; /* 0.001 */ >>>>>> *val2 = 1000; >>>>>> break; >>>>>> + case IIO_CONCENTRATION: >>>>>> + switch (chan->address) { >>>>>> + case ATLAS_REG_EC_DATA: >>>>>> + *val = 0; /* 0.00000000064 */ >>>>>> + *val2 = 640; >>>>>> + return IIO_VAL_INT_PLUS_NANO; >>>>>> + default: >>>>>> + *val = 0; /* 0.000000001 */ >>>>>> + *val2 = 1000; >>>>>> + return IIO_VAL_INT_PLUS_NANO; >>>>>> + } >>>>>> + break; >>>>>> default: >>>>>> return -EINVAL; >>>>>> } >>>>>> @@ -517,12 +624,14 @@ static const struct dev_pm_ops atlas_pm_ops = { >>>>>> >>>>>> static const struct i2c_device_id atlas_id[] = { >>>>>> { "atlas-ph-sm", ATLAS_PH_SM}, >>>>>> + { "atlas-ec-sm", ATLAS_EC_SM}, >>>>>> {} >>>>>> }; >>>>>> MODULE_DEVICE_TABLE(i2c, atlas_id); >>>>>> >>>>>> static const struct of_device_id atlas_dt_ids[] = { >>>>>> { .compatible = "atlas,ph-sm", .data = (void *)ATLAS_PH_SM, }, >>>>>> + { .compatible = "atlas,ec-sm", .data = (void *)ATLAS_EC_SM, }, >>>>>> { } >>>>>> }; >>>>>> MODULE_DEVICE_TABLE(of, atlas_dt_ids); >>>>>> >>>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>> > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-05-19 5:44 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-24 20:43 [PATCH 0/2] iio: chemical: atlas-ph-sensor: add conductivity sensor support Matt Ranostay 2016-04-24 20:43 ` [PATCH 1/2] iio: chemical: atlas-ph-sensor: reorg driver to allow multiple chips Matt Ranostay 2016-05-04 10:31 ` Jonathan Cameron 2016-04-24 20:43 ` [PATCH 2/2] iio: chemical: atlas-ph-sensor: add EC feature Matt Ranostay 2016-05-04 10:39 ` Jonathan Cameron 2016-05-05 8:17 ` Matt Ranostay 2016-05-08 19:29 ` Jonathan Cameron 2016-05-11 2:35 ` Matt Ranostay 2016-05-14 17:02 ` Jonathan Cameron 2016-05-19 5:43 ` Matt Ranostay
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.