[PATCH v2 0/3] iio: chemical: atlas-ph-sensor: add ORP feature

[PATCH v2 0/3] iio: chemical: atlas-ph-sensor: add ORP feature

[Matt Ranostay]

Add support for the Oxidation-Reduction Potential SM OEM module from Atlas
Scientific part, and some regmap + locking cleanups.

Changes from v1:
* remove double locking of the mlock mutex

Matt Ranostay (3):
  iio: chemical: atlas-ph-sensor: switch to iio_device_*_direct_mode
    helpers
  iio: chemical: atlas-ph-sensor: switch to REGCACHE_NONE regmap tree
  iio: chemical: atlas-ph-sensor: add ORP feature

 .../bindings/iio/chemical/atlas,orp-sm.txt         | 22 ++++++
 drivers/iio/chemical/Kconfig                       |  1 +
 drivers/iio/chemical/atlas-ph-sensor.c             | 81 +++++++++++++++-------
 3 files changed, 80 insertions(+), 24 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt

-- 
2.7.4

[PATCH v2 1/3] iio: chemical: atlas-ph-sensor: switch to iio_device_*_direct_mode helpers

[Matt Ranostay]

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 drivers/iio/chemical/atlas-ph-sensor.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
index ae038a59d256..696875413d10 100644
--- a/drivers/iio/chemical/atlas-ph-sensor.c
+++ b/drivers/iio/chemical/atlas-ph-sensor.c
@@ -402,15 +402,13 @@ static int atlas_read_raw(struct iio_dev *indio_dev,
 		case IIO_PH:
 		case IIO_CONCENTRATION:
 		case IIO_ELECTRICALCONDUCTIVITY:
-			mutex_lock(&indio_dev->mlock);
+			ret = iio_device_claim_direct_mode(indio_dev);
+			if (ret)
+				return ret;
 
-			if (iio_buffer_enabled(indio_dev))
-				ret = -EBUSY;
-			else
-				ret = atlas_read_measurement(data,
-							chan->address, &reg);
+			ret = atlas_read_measurement(data, chan->address, &reg);
 
-			mutex_unlock(&indio_dev->mlock);
+			iio_device_release_direct_mode(indio_dev);
 			break;
 		default:
 			ret = -EINVAL;
-- 
2.7.4

[PATCH v2 2/3] iio: chemical: atlas-ph-sensor: switch to REGCACHE_NONE regmap tree

[Matt Ranostay]

Since there are overlapping volatile regions between parts, and only
register that isn't volatile is the temperature compensation provided
from userspace.

Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
 drivers/iio/chemical/atlas-ph-sensor.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
index 696875413d10..c0a0ebbbd54a 100644
--- a/drivers/iio/chemical/atlas-ph-sensor.c
+++ b/drivers/iio/chemical/atlas-ph-sensor.c
@@ -84,26 +84,10 @@ struct atlas_data {
 	__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 = {
-	.yes_ranges	= atlas_volatile_ranges,
-	.n_yes_ranges	= ARRAY_SIZE(atlas_volatile_ranges),
-};
-
 static const struct regmap_config atlas_regmap_config = {
 	.name = ATLAS_REGMAP_NAME,
-
 	.reg_bits = 8,
 	.val_bits = 8,
-
-	.volatile_table = &atlas_volatile_table,
-	.max_register = ATLAS_REG_PSS_DATA + 4,
-	.cache_type = REGCACHE_RBTREE,
 };
 
 static const struct iio_chan_spec atlas_ph_channels[] = {
-- 
2.7.4

[PATCH v2 3/3] iio: chemical: atlas-ph-sensor: add ORP feature

[Matt Ranostay]

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

diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
new file mode 100644
index 000000000000..5d8b687d5edc
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
@@ -0,0 +1,22 @@
+* Atlas Scientific ORP-SM OEM sensor
+
+https://www.atlas-scientific.com/_files/_datasheets/_oem/ORP_oem_datasheet.pdf
+
+Required properties:
+
+  - compatible: must be "atlas,orp-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@66 {
+	compatible = "atlas,orp-sm";
+	reg = <0x66>;
+	interrupt-parent = <&gpio1>;
+	interrupts = <16 2>;
+};
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 4bcc025e8c8a..cea7f9857a1f 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -16,6 +16,7 @@ config ATLAS_PH_SENSOR
 	 Atlas Scientific OEM SM sensors:
 	    * pH SM sensor
 	    * EC SM sensor
+	    * ORP 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 c0a0ebbbd54a..84fbff32b96d 100644
--- a/drivers/iio/chemical/atlas-ph-sensor.c
+++ b/drivers/iio/chemical/atlas-ph-sensor.c
@@ -66,12 +66,17 @@
 #define ATLAS_REG_TDS_DATA		0x1c
 #define ATLAS_REG_PSS_DATA		0x20
 
+#define ATLAS_REG_ORP_CALIB_STATUS	0x0d
+#define ATLAS_REG_ORP_DATA		0x0e
+
 #define ATLAS_PH_INT_TIME_IN_US		450000
 #define ATLAS_EC_INT_TIME_IN_US		650000
+#define ATLAS_ORP_INT_TIME_IN_US	450000
 
 enum {
 	ATLAS_PH_SM,
 	ATLAS_EC_SM,
+	ATLAS_ORP_SM,
 };
 
 struct atlas_data {
@@ -159,6 +164,23 @@ static const struct iio_chan_spec atlas_ec_channels[] = {
 	},
 };
 
+static const struct iio_chan_spec atlas_orp_channels[] = {
+	{
+		.type = IIO_VOLTAGE,
+		.address = ATLAS_REG_ORP_DATA,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_BE,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
 static int atlas_check_ph_calibration(struct atlas_data *data)
 {
 	struct device *dev = &data->client->dev;
@@ -224,6 +246,22 @@ static int atlas_check_ec_calibration(struct atlas_data *data)
 	return 0;
 }
 
+static int atlas_check_orp_calibration(struct atlas_data *data)
+{
+	struct device *dev = &data->client->dev;
+	int ret;
+	unsigned int val;
+
+	ret = regmap_read(data->regmap, ATLAS_REG_ORP_CALIB_STATUS, &val);
+	if (ret)
+		return ret;
+
+	if (!val)
+		dev_warn(dev, "device has not been calibrated\n");
+
+	return 0;
+};
+
 struct atlas_device {
 	const struct iio_chan_spec *channels;
 	int num_channels;
@@ -248,7 +286,13 @@ static struct atlas_device atlas_devices[] = {
 				.calibration = &atlas_check_ec_calibration,
 				.delay = ATLAS_EC_INT_TIME_IN_US,
 	},
-
+	[ATLAS_ORP_SM] = {
+				.channels = atlas_orp_channels,
+				.num_channels = 2,
+				.data_reg = ATLAS_REG_ORP_DATA,
+				.calibration = &atlas_check_orp_calibration,
+				.delay = ATLAS_ORP_INT_TIME_IN_US,
+	},
 };
 
 static int atlas_set_powermode(struct atlas_data *data, int on)
@@ -386,6 +430,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev,
 		case IIO_PH:
 		case IIO_CONCENTRATION:
 		case IIO_ELECTRICALCONDUCTIVITY:
+		case IIO_VOLTAGE:
 			ret = iio_device_claim_direct_mode(indio_dev);
 			if (ret)
 				return ret;
@@ -422,6 +467,10 @@ static int atlas_read_raw(struct iio_dev *indio_dev,
 			*val = 0; /* 0.000000001 */
 			*val2 = 1000;
 			return IIO_VAL_INT_PLUS_NANO;
+		case IIO_VOLTAGE:
+			*val = 1; /* 0.1 */
+			*val2 = 10;
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -457,6 +506,7 @@ static const struct iio_info atlas_info = {
 static const struct i2c_device_id atlas_id[] = {
 	{ "atlas-ph-sm", ATLAS_PH_SM},
 	{ "atlas-ec-sm", ATLAS_EC_SM},
+	{ "atlas-orp-sm", ATLAS_ORP_SM},
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, atlas_id);
@@ -464,6 +514,7 @@ 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, },
+	{ .compatible = "atlas,orp-sm", .data = (void *)ATLAS_ORP_SM, },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, atlas_dt_ids);
-- 
2.7.4

Re: [PATCH v2 1/3] iio: chemical: atlas-ph-sensor: switch to iio_device_*_direct_mode helpers

[Jonathan Cameron]

On 19/08/16 21:36, Matt Ranostay wrote:
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
Applied to the togreg branch of iio.git - initially pushed out
as testing for the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  drivers/iio/chemical/atlas-ph-sensor.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
> index ae038a59d256..696875413d10 100644
> --- a/drivers/iio/chemical/atlas-ph-sensor.c
> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
> @@ -402,15 +402,13 @@ static int atlas_read_raw(struct iio_dev *indio_dev,
>  		case IIO_PH:
>  		case IIO_CONCENTRATION:
>  		case IIO_ELECTRICALCONDUCTIVITY:
> -			mutex_lock(&indio_dev->mlock);
> +			ret = iio_device_claim_direct_mode(indio_dev);
> +			if (ret)
> +				return ret;
>  
> -			if (iio_buffer_enabled(indio_dev))
> -				ret = -EBUSY;
> -			else
> -				ret = atlas_read_measurement(data,
> -							chan->address, &reg);
> +			ret = atlas_read_measurement(data, chan->address, &reg);
>  
> -			mutex_unlock(&indio_dev->mlock);
> +			iio_device_release_direct_mode(indio_dev);
>  			break;
>  		default:
>  			ret = -EINVAL;
> 

Re: [PATCH v2 2/3] iio: chemical: atlas-ph-sensor: switch to REGCACHE_NONE regmap tree

[Jonathan Cameron]

On 19/08/16 21:36, Matt Ranostay wrote:
> Since there are overlapping volatile regions between parts, and only
> register that isn't volatile is the temperature compensation provided
> from userspace.
This does make me wonder if a shared regmap across the different parts
makes sense. (assuming I've understood the issue).

However, not many registers, not a quick device so who really cares ;)

Applied.

Thanks,

Jonathan
> 
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
>  drivers/iio/chemical/atlas-ph-sensor.c | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/drivers/iio/chemical/atlas-ph-sensor.c b/drivers/iio/chemical/atlas-ph-sensor.c
> index 696875413d10..c0a0ebbbd54a 100644
> --- a/drivers/iio/chemical/atlas-ph-sensor.c
> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
> @@ -84,26 +84,10 @@ struct atlas_data {
>  	__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 = {
> -	.yes_ranges	= atlas_volatile_ranges,
> -	.n_yes_ranges	= ARRAY_SIZE(atlas_volatile_ranges),
> -};
> -
>  static const struct regmap_config atlas_regmap_config = {
>  	.name = ATLAS_REGMAP_NAME,
> -
>  	.reg_bits = 8,
>  	.val_bits = 8,
> -
> -	.volatile_table = &atlas_volatile_table,
> -	.max_register = ATLAS_REG_PSS_DATA + 4,
> -	.cache_type = REGCACHE_RBTREE,
>  };
>  
>  static const struct iio_chan_spec atlas_ph_channels[] = {
> 

Re: [PATCH v2 3/3] iio: chemical: atlas-ph-sensor: add ORP feature

[Jonathan Cameron]

On 19/08/16 21:36, Matt Ranostay wrote:
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
Applied.

thanks,

Jonathan
> ---
>  .../bindings/iio/chemical/atlas,orp-sm.txt         | 22 +++++++++
>  drivers/iio/chemical/Kconfig                       |  1 +
>  drivers/iio/chemical/atlas-ph-sensor.c             | 53 +++++++++++++++++++++-
>  3 files changed, 75 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
> new file mode 100644
> index 000000000000..5d8b687d5edc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
> @@ -0,0 +1,22 @@
> +* Atlas Scientific ORP-SM OEM sensor
> +
> +https://www.atlas-scientific.com/_files/_datasheets/_oem/ORP_oem_datasheet.pdf
> +
> +Required properties:
> +
> +  - compatible: must be "atlas,orp-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@66 {
> +	compatible = "atlas,orp-sm";
> +	reg = <0x66>;
> +	interrupt-parent = <&gpio1>;
> +	interrupts = <16 2>;
> +};
> diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
> index 4bcc025e8c8a..cea7f9857a1f 100644
> --- a/drivers/iio/chemical/Kconfig
> +++ b/drivers/iio/chemical/Kconfig
> @@ -16,6 +16,7 @@ config ATLAS_PH_SENSOR
>  	 Atlas Scientific OEM SM sensors:
>  	    * pH SM sensor
>  	    * EC SM sensor
> +	    * ORP 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 c0a0ebbbd54a..84fbff32b96d 100644
> --- a/drivers/iio/chemical/atlas-ph-sensor.c
> +++ b/drivers/iio/chemical/atlas-ph-sensor.c
> @@ -66,12 +66,17 @@
>  #define ATLAS_REG_TDS_DATA		0x1c
>  #define ATLAS_REG_PSS_DATA		0x20
>  
> +#define ATLAS_REG_ORP_CALIB_STATUS	0x0d
> +#define ATLAS_REG_ORP_DATA		0x0e
> +
>  #define ATLAS_PH_INT_TIME_IN_US		450000
>  #define ATLAS_EC_INT_TIME_IN_US		650000
> +#define ATLAS_ORP_INT_TIME_IN_US	450000
>  
>  enum {
>  	ATLAS_PH_SM,
>  	ATLAS_EC_SM,
> +	ATLAS_ORP_SM,
>  };
>  
>  struct atlas_data {
> @@ -159,6 +164,23 @@ static const struct iio_chan_spec atlas_ec_channels[] = {
>  	},
>  };
>  
> +static const struct iio_chan_spec atlas_orp_channels[] = {
> +	{
> +		.type = IIO_VOLTAGE,
> +		.address = ATLAS_REG_ORP_DATA,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 's',
> +			.realbits = 32,
> +			.storagebits = 32,
> +			.endianness = IIO_BE,
> +		},
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
>  static int atlas_check_ph_calibration(struct atlas_data *data)
>  {
>  	struct device *dev = &data->client->dev;
> @@ -224,6 +246,22 @@ static int atlas_check_ec_calibration(struct atlas_data *data)
>  	return 0;
>  }
>  
> +static int atlas_check_orp_calibration(struct atlas_data *data)
> +{
> +	struct device *dev = &data->client->dev;
> +	int ret;
> +	unsigned int val;
> +
> +	ret = regmap_read(data->regmap, ATLAS_REG_ORP_CALIB_STATUS, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (!val)
> +		dev_warn(dev, "device has not been calibrated\n");
> +
> +	return 0;
> +};
> +
>  struct atlas_device {
>  	const struct iio_chan_spec *channels;
>  	int num_channels;
> @@ -248,7 +286,13 @@ static struct atlas_device atlas_devices[] = {
>  				.calibration = &atlas_check_ec_calibration,
>  				.delay = ATLAS_EC_INT_TIME_IN_US,
>  	},
> -
> +	[ATLAS_ORP_SM] = {
> +				.channels = atlas_orp_channels,
> +				.num_channels = 2,
> +				.data_reg = ATLAS_REG_ORP_DATA,
> +				.calibration = &atlas_check_orp_calibration,
> +				.delay = ATLAS_ORP_INT_TIME_IN_US,
> +	},
>  };
>  
>  static int atlas_set_powermode(struct atlas_data *data, int on)
> @@ -386,6 +430,7 @@ static int atlas_read_raw(struct iio_dev *indio_dev,
>  		case IIO_PH:
>  		case IIO_CONCENTRATION:
>  		case IIO_ELECTRICALCONDUCTIVITY:
> +		case IIO_VOLTAGE:
>  			ret = iio_device_claim_direct_mode(indio_dev);
>  			if (ret)
>  				return ret;
> @@ -422,6 +467,10 @@ static int atlas_read_raw(struct iio_dev *indio_dev,
>  			*val = 0; /* 0.000000001 */
>  			*val2 = 1000;
>  			return IIO_VAL_INT_PLUS_NANO;
> +		case IIO_VOLTAGE:
> +			*val = 1; /* 0.1 */
> +			*val2 = 10;
> +			break;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -457,6 +506,7 @@ static const struct iio_info atlas_info = {
>  static const struct i2c_device_id atlas_id[] = {
>  	{ "atlas-ph-sm", ATLAS_PH_SM},
>  	{ "atlas-ec-sm", ATLAS_EC_SM},
> +	{ "atlas-orp-sm", ATLAS_ORP_SM},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(i2c, atlas_id);
> @@ -464,6 +514,7 @@ 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, },
> +	{ .compatible = "atlas,orp-sm", .data = (void *)ATLAS_ORP_SM, },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, atlas_dt_ids);
>