All of lore.kernel.org
 help / color / mirror / Atom feed
* [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, &reg);
+				ret = atlas_read_measurement(data,
+							chan->address, &reg);
 
 			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

* [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 *) &reg, 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 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, &reg);
> +				ret = atlas_read_measurement(data,
> +							chan->address, &reg);
>  
>  			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

* 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 *) &reg, 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 *) &reg, 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 *) &reg, 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 *) &reg, 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 *) &reg, 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 *) &reg, 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.