linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] iio: chemical: atlas-sensor: add DO support
@ 2020-02-06  6:13 Matt Ranostay
  2020-02-06  6:13 ` [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line Matt Ranostay
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Matt Ranostay @ 2020-02-06  6:13 UTC (permalink / raw)
  To: linux-iio; +Cc: jic23, Matt Ranostay

Changes from v1:
* combined dt binding docs
* added making interrupt line is optional patchset
* broke out dt changes from DO-SM module patchset

Change from v2:
* switch docs to YAML format

Change from v3:
* fix up issues reported with dt-schema

Matt Ranostay (3):
  iio: chemical: atlas-sensor: allow probe without interrupt line
  iio: chemical: atlas-sensor: add DO-SM module support
  dt-bindings: iio: chemical: consolidate atlas-sensor docs

 .../bindings/iio/chemical/atlas,ec-sm.txt     | 21 -----
 .../bindings/iio/chemical/atlas,orp-sm.txt    | 21 -----
 .../bindings/iio/chemical/atlas,ph-sm.txt     | 21 -----
 .../bindings/iio/chemical/atlas,sensor.yaml   | 53 +++++++++++
 drivers/iio/chemical/atlas-sensor.c           | 91 +++++++++++++++----
 5 files changed, 125 insertions(+), 82 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt
 delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
 delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml

-- 
2.20.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line
  2020-02-06  6:13 [PATCH v4 0/3] iio: chemical: atlas-sensor: add DO support Matt Ranostay
@ 2020-02-06  6:13 ` Matt Ranostay
  2020-02-07  0:53   ` Jeremy Fertic
  2020-02-08 16:39   ` Jonathan Cameron
  2020-02-06  6:13 ` [PATCH v4 2/3] iio: chemical: atlas-sensor: add DO-SM module support Matt Ranostay
  2020-02-06  6:13 ` [PATCH v4 3/3] dt-bindings: iio: chemical: consolidate atlas-sensor docs Matt Ranostay
  2 siblings, 2 replies; 13+ messages in thread
From: Matt Ranostay @ 2020-02-06  6:13 UTC (permalink / raw)
  To: linux-iio; +Cc: jic23, Matt Ranostay

Sensors don't actually need a interrupt line to give valid readings,
and can triggered with CONFIG_IIO_HRTIMER_TRIGGER as well. Remove
the required check for interrupt, and continue along in the probe
function.

Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
---
 drivers/iio/chemical/atlas-sensor.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
index 2f0a6fed2589..2e34c82cb65d 100644
--- a/drivers/iio/chemical/atlas-sensor.c
+++ b/drivers/iio/chemical/atlas-sensor.c
@@ -572,11 +572,6 @@ static int atlas_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	if (client->irq <= 0) {
-		dev_err(&client->dev, "no valid irq defined\n");
-		return -EINVAL;
-	}
-
 	ret = chip->calibration(data);
 	if (ret)
 		return ret;
@@ -596,16 +591,18 @@ static int atlas_probe(struct i2c_client *client,
 
 	init_irq_work(&data->work, atlas_work_handler);
 
-	/* interrupt pin toggles on new conversion */
-	ret = devm_request_threaded_irq(&client->dev, client->irq,
-					NULL, atlas_interrupt_handler,
-					IRQF_TRIGGER_RISING |
-					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					"atlas_irq",
-					indio_dev);
-	if (ret) {
-		dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
-		goto unregister_buffer;
+	if (client->irq <= 0) {
+		/* interrupt pin toggles on new conversion */
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+				NULL, atlas_interrupt_handler,
+				IRQF_TRIGGER_RISING |
+				IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+				"atlas_irq",
+				indio_dev);
+
+		if (ret)
+			dev_warn(&client->dev,
+				"request irq (%d) failed\n", client->irq);
 	}
 
 	ret = atlas_set_powermode(data, 1);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 2/3] iio: chemical: atlas-sensor: add DO-SM module support
  2020-02-06  6:13 [PATCH v4 0/3] iio: chemical: atlas-sensor: add DO support Matt Ranostay
  2020-02-06  6:13 ` [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line Matt Ranostay
@ 2020-02-06  6:13 ` Matt Ranostay
  2020-02-08 16:41   ` Jonathan Cameron
  2020-02-06  6:13 ` [PATCH v4 3/3] dt-bindings: iio: chemical: consolidate atlas-sensor docs Matt Ranostay
  2 siblings, 1 reply; 13+ messages in thread
From: Matt Ranostay @ 2020-02-06  6:13 UTC (permalink / raw)
  To: linux-iio; +Cc: jic23, Matt Ranostay

Atlas Scientific DO-SM OEM sensor reads disolved oxygen in
a solution. This is reported back as mg/L which maps direc
to ppm and so the IIO_CONCENTRATION channel type can be used.

Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
---
 drivers/iio/chemical/atlas-sensor.c | 64 +++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
index 2e34c82cb65d..919d408869be 100644
--- a/drivers/iio/chemical/atlas-sensor.c
+++ b/drivers/iio/chemical/atlas-sensor.c
@@ -48,6 +48,11 @@
 #define ATLAS_REG_EC_CALIB_STATUS_LOW		BIT(2)
 #define ATLAS_REG_EC_CALIB_STATUS_HIGH		BIT(3)
 
+#define ATLAS_REG_DO_CALIB_STATUS		0x09
+#define ATLAS_REG_DO_CALIB_STATUS_MASK		0x03
+#define ATLAS_REG_DO_CALIB_STATUS_PRESSURE	BIT(0)
+#define ATLAS_REG_DO_CALIB_STATUS_DO		BIT(1)
+
 #define ATLAS_REG_PH_TEMP_DATA		0x0e
 #define ATLAS_REG_PH_DATA		0x16
 
@@ -60,14 +65,19 @@
 #define ATLAS_REG_ORP_CALIB_STATUS	0x0d
 #define ATLAS_REG_ORP_DATA		0x0e
 
+#define ATLAS_REG_DO_TEMP_DATA		0x12
+#define ATLAS_REG_DO_DATA		0x22
+
 #define ATLAS_PH_INT_TIME_IN_MS		450
 #define ATLAS_EC_INT_TIME_IN_MS		650
 #define ATLAS_ORP_INT_TIME_IN_MS	450
+#define ATLAS_DO_INT_TIME_IN_MS		450
 
 enum {
 	ATLAS_PH_SM,
 	ATLAS_EC_SM,
 	ATLAS_ORP_SM,
+	ATLAS_DO_SM,
 };
 
 struct atlas_data {
@@ -121,7 +131,7 @@ static const struct iio_chan_spec atlas_ph_channels[] = {
 	},
 };
 
-#define ATLAS_EC_CHANNEL(_idx, _addr) \
+#define ATLAS_CONCENTRATION_CHANNEL(_idx, _addr) \
 	{\
 		.type = IIO_CONCENTRATION, \
 		.indexed = 1, \
@@ -152,8 +162,8 @@ static const struct iio_chan_spec atlas_ec_channels[] = {
 			.endianness = IIO_BE,
 		},
 	},
-	ATLAS_EC_CHANNEL(0, ATLAS_REG_TDS_DATA),
-	ATLAS_EC_CHANNEL(1, ATLAS_REG_PSS_DATA),
+	ATLAS_CONCENTRATION_CHANNEL(0, ATLAS_REG_TDS_DATA),
+	ATLAS_CONCENTRATION_CHANNEL(1, ATLAS_REG_PSS_DATA),
 	IIO_CHAN_SOFT_TIMESTAMP(3),
 	{
 		.type = IIO_TEMP,
@@ -182,6 +192,19 @@ static const struct iio_chan_spec atlas_orp_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(1),
 };
 
+static const struct iio_chan_spec atlas_do_channels[] = {
+	ATLAS_CONCENTRATION_CHANNEL(0, ATLAS_REG_DO_DATA),
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+	{
+		.type = IIO_TEMP,
+		.address = ATLAS_REG_DO_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;
@@ -262,7 +285,31 @@ static int atlas_check_orp_calibration(struct atlas_data *data)
 		dev_warn(dev, "device has not been calibrated\n");
 
 	return 0;
-};
+}
+
+static int atlas_check_do_calibration(struct atlas_data *data)
+{
+	struct device *dev = &data->client->dev;
+	int ret;
+	unsigned int val;
+
+	ret = regmap_read(data->regmap, ATLAS_REG_DO_CALIB_STATUS, &val);
+	if (ret)
+		return ret;
+
+	if (!(val & ATLAS_REG_DO_CALIB_STATUS_MASK)) {
+		dev_warn(dev, "device has not been calibrated\n");
+		return 0;
+	}
+
+	if (!(val & ATLAS_REG_DO_CALIB_STATUS_PRESSURE))
+		dev_warn(dev, "device missing atmospheric pressure calibration\n");
+
+	if (!(val & ATLAS_REG_DO_CALIB_STATUS_DO))
+		dev_warn(dev, "device missing dissolved oxygen calibration\n");
+
+	return 0;
+}
 
 struct atlas_device {
 	const struct iio_chan_spec *channels;
@@ -295,6 +342,13 @@ static struct atlas_device atlas_devices[] = {
 				.calibration = &atlas_check_orp_calibration,
 				.delay = ATLAS_ORP_INT_TIME_IN_MS,
 	},
+	[ATLAS_DO_SM] = {
+				.channels = atlas_do_channels,
+				.num_channels = 3,
+				.data_reg = ATLAS_REG_DO_DATA,
+				.calibration = &atlas_check_do_calibration,
+				.delay = ATLAS_DO_INT_TIME_IN_MS,
+	},
 };
 
 static int atlas_set_powermode(struct atlas_data *data, int on)
@@ -507,6 +561,7 @@ 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},
+	{ "atlas-do-sm", ATLAS_DO_SM},
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, atlas_id);
@@ -515,6 +570,7 @@ 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, },
+	{ .compatible = "atlas,do-sm", .data = (void *)ATLAS_DO_SM, },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, atlas_dt_ids);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 3/3] dt-bindings: iio: chemical: consolidate atlas-sensor docs
  2020-02-06  6:13 [PATCH v4 0/3] iio: chemical: atlas-sensor: add DO support Matt Ranostay
  2020-02-06  6:13 ` [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line Matt Ranostay
  2020-02-06  6:13 ` [PATCH v4 2/3] iio: chemical: atlas-sensor: add DO-SM module support Matt Ranostay
@ 2020-02-06  6:13 ` Matt Ranostay
  2020-02-06 21:52   ` Rob Herring
  2020-02-08 16:34   ` Jonathan Cameron
  2 siblings, 2 replies; 13+ messages in thread
From: Matt Ranostay @ 2020-02-06  6:13 UTC (permalink / raw)
  To: linux-iio; +Cc: jic23, Matt Ranostay, devicetree

Since Atlas Scientific device support only varies from the compatible
string is ideal all the respective docs are merged into a single doc
named atlas,sensor.yaml

Cc: devicetree@vger.kernel.org
Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
---
 .../bindings/iio/chemical/atlas,ec-sm.txt     | 21 --------
 .../bindings/iio/chemical/atlas,orp-sm.txt    | 21 --------
 .../bindings/iio/chemical/atlas,ph-sm.txt     | 21 --------
 .../bindings/iio/chemical/atlas,sensor.yaml   | 53 +++++++++++++++++++
 4 files changed, 53 insertions(+), 63 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt
 delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
 delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml

diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt
deleted file mode 100644
index f4320595b851..000000000000
--- a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-* 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
-  - 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/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
deleted file mode 100644
index af1f5a9aa4da..000000000000
--- a/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-* 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
-  - 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/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
deleted file mode 100644
index 79d90f060327..000000000000
--- a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-* Atlas Scientific pH-SM OEM sensor
-
-http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
-
-Required properties:
-
-  - compatible: must be "atlas,ph-sm"
-  - reg: the I2C address of the sensor
-  - interrupts: the sole interrupt generated by the device
-
-  Refer to interrupt-controller/interrupts.txt for generic interrupt client
-  node bindings.
-
-Example:
-
-atlas@65 {
-	compatible = "atlas,ph-sm";
-	reg = <0x65>;
-	interrupt-parent = <&gpio1>;
-	interrupts = <16 2>;
-};
diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml b/Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml
new file mode 100644
index 000000000000..1b0c476838d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/chemical/atlas,sensor.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atlas Scientific OEM sensors
+
+maintainers:
+  - Matt Ranostay <matt.ranostay@konsulko.com>
+
+description: |
+  Atlas Scientific OEM sensors connected via I2C
+
+  Datasheets:
+    http://www.atlas-scientific.com/_files/_datasheets/_oem/DO_oem_datasheet.pdf
+    http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf
+    http://www.atlas-scientific.com/_files/_datasheets/_oem/ORP_oem_datasheet.pdf
+    http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
+
+properties:
+  compatible:
+    enum:
+      - atlas,do-sm
+      - atlas,ec-sm
+      - atlas,orp-sm
+      - atlas,ph-sm
+
+  reg:
+     maxItems: 1
+
+  interrupts:
+     maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      atlas@66 {
+        compatible = "atlas,orp-sm";
+        reg = <0x66>;
+        interrupt-parent = <&gpio1>;
+        interrupts = <16 2>;
+      };
+    };
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 3/3] dt-bindings: iio: chemical: consolidate atlas-sensor docs
  2020-02-06  6:13 ` [PATCH v4 3/3] dt-bindings: iio: chemical: consolidate atlas-sensor docs Matt Ranostay
@ 2020-02-06 21:52   ` Rob Herring
  2020-02-08 16:34   ` Jonathan Cameron
  1 sibling, 0 replies; 13+ messages in thread
From: Rob Herring @ 2020-02-06 21:52 UTC (permalink / raw)
  To: Matt Ranostay; +Cc: linux-iio, jic23, Matt Ranostay, devicetree

On Wed,  5 Feb 2020 22:13:32 -0800, Matt Ranostay wrote:
> Since Atlas Scientific device support only varies from the compatible
> string is ideal all the respective docs are merged into a single doc
> named atlas,sensor.yaml
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  .../bindings/iio/chemical/atlas,ec-sm.txt     | 21 --------
>  .../bindings/iio/chemical/atlas,orp-sm.txt    | 21 --------
>  .../bindings/iio/chemical/atlas,ph-sm.txt     | 21 --------
>  .../bindings/iio/chemical/atlas,sensor.yaml   | 53 +++++++++++++++++++
>  4 files changed, 53 insertions(+), 63 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt
>  delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
>  delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line
  2020-02-06  6:13 ` [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line Matt Ranostay
@ 2020-02-07  0:53   ` Jeremy Fertic
  2020-02-07  4:59     ` Matt Ranostay
  2020-02-08 16:39   ` Jonathan Cameron
  1 sibling, 1 reply; 13+ messages in thread
From: Jeremy Fertic @ 2020-02-07  0:53 UTC (permalink / raw)
  To: Matt Ranostay; +Cc: linux-iio, jic23

On Wed, Feb 05, 2020 at 10:13:30PM -0800, Matt Ranostay wrote:
> Sensors don't actually need a interrupt line to give valid readings,
> and can triggered with CONFIG_IIO_HRTIMER_TRIGGER as well. Remove
> the required check for interrupt, and continue along in the probe
> function.
> 
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  drivers/iio/chemical/atlas-sensor.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
> index 2f0a6fed2589..2e34c82cb65d 100644
> --- a/drivers/iio/chemical/atlas-sensor.c
> +++ b/drivers/iio/chemical/atlas-sensor.c
> @@ -572,11 +572,6 @@ static int atlas_probe(struct i2c_client *client,
>  	if (ret)
>  		return ret;
>  
> -	if (client->irq <= 0) {
> -		dev_err(&client->dev, "no valid irq defined\n");
> -		return -EINVAL;
> -	}
> -
>  	ret = chip->calibration(data);
>  	if (ret)
>  		return ret;
> @@ -596,16 +591,18 @@ static int atlas_probe(struct i2c_client *client,
>  
>  	init_irq_work(&data->work, atlas_work_handler);
>  
> -	/* interrupt pin toggles on new conversion */
> -	ret = devm_request_threaded_irq(&client->dev, client->irq,
> -					NULL, atlas_interrupt_handler,
> -					IRQF_TRIGGER_RISING |
> -					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -					"atlas_irq",
> -					indio_dev);
> -	if (ret) {
> -		dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
> -		goto unregister_buffer;
> +	if (client->irq <= 0) {

Should this be > 0 ?

Jeremy

> +		/* interrupt pin toggles on new conversion */
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +				NULL, atlas_interrupt_handler,
> +				IRQF_TRIGGER_RISING |
> +				IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +				"atlas_irq",
> +				indio_dev);
> +
> +		if (ret)
> +			dev_warn(&client->dev,
> +				"request irq (%d) failed\n", client->irq);
>  	}
>  
>  	ret = atlas_set_powermode(data, 1);
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line
  2020-02-07  0:53   ` Jeremy Fertic
@ 2020-02-07  4:59     ` Matt Ranostay
  0 siblings, 0 replies; 13+ messages in thread
From: Matt Ranostay @ 2020-02-07  4:59 UTC (permalink / raw)
  To: Jeremy Fertic; +Cc: open list:IIO SUBSYSTEM AND DRIVERS, Jonathan Cameron

On Thu, Feb 6, 2020 at 4:53 PM Jeremy Fertic <jeremyfertic@gmail.com> wrote:
>
> On Wed, Feb 05, 2020 at 10:13:30PM -0800, Matt Ranostay wrote:
> > Sensors don't actually need a interrupt line to give valid readings,
> > and can triggered with CONFIG_IIO_HRTIMER_TRIGGER as well. Remove
> > the required check for interrupt, and continue along in the probe
> > function.
> >
> > Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> > ---
> >  drivers/iio/chemical/atlas-sensor.c | 27 ++++++++++++---------------
> >  1 file changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
> > index 2f0a6fed2589..2e34c82cb65d 100644
> > --- a/drivers/iio/chemical/atlas-sensor.c
> > +++ b/drivers/iio/chemical/atlas-sensor.c
> > @@ -572,11 +572,6 @@ static int atlas_probe(struct i2c_client *client,
> >       if (ret)
> >               return ret;
> >
> > -     if (client->irq <= 0) {
> > -             dev_err(&client->dev, "no valid irq defined\n");
> > -             return -EINVAL;
> > -     }
> > -
> >       ret = chip->calibration(data);
> >       if (ret)
> >               return ret;
> > @@ -596,16 +591,18 @@ static int atlas_probe(struct i2c_client *client,
> >
> >       init_irq_work(&data->work, atlas_work_handler);
> >
> > -     /* interrupt pin toggles on new conversion */
> > -     ret = devm_request_threaded_irq(&client->dev, client->irq,
> > -                                     NULL, atlas_interrupt_handler,
> > -                                     IRQF_TRIGGER_RISING |
> > -                                     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> > -                                     "atlas_irq",
> > -                                     indio_dev);
> > -     if (ret) {
> > -             dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
> > -             goto unregister_buffer;
> > +     if (client->irq <= 0) {
>
> Should this be > 0 ?
>

Ah yes good catch :/

- Matt

> Jeremy
>
> > +             /* interrupt pin toggles on new conversion */
> > +             ret = devm_request_threaded_irq(&client->dev, client->irq,
> > +                             NULL, atlas_interrupt_handler,
> > +                             IRQF_TRIGGER_RISING |
> > +                             IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> > +                             "atlas_irq",
> > +                             indio_dev);
> > +
> > +             if (ret)
> > +                     dev_warn(&client->dev,
> > +                             "request irq (%d) failed\n", client->irq);
> >       }
> >
> >       ret = atlas_set_powermode(data, 1);
> > --
> > 2.20.1
> >

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 3/3] dt-bindings: iio: chemical: consolidate atlas-sensor docs
  2020-02-06  6:13 ` [PATCH v4 3/3] dt-bindings: iio: chemical: consolidate atlas-sensor docs Matt Ranostay
  2020-02-06 21:52   ` Rob Herring
@ 2020-02-08 16:34   ` Jonathan Cameron
  1 sibling, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2020-02-08 16:34 UTC (permalink / raw)
  To: Matt Ranostay; +Cc: linux-iio, devicetree

On Wed,  5 Feb 2020 22:13:32 -0800
Matt Ranostay <matt.ranostay@konsulko.com> wrote:

> Since Atlas Scientific device support only varies from the compatible
> string is ideal all the respective docs are merged into a single doc
> named atlas,sensor.yaml
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  .../bindings/iio/chemical/atlas,ec-sm.txt     | 21 --------
>  .../bindings/iio/chemical/atlas,orp-sm.txt    | 21 --------
>  .../bindings/iio/chemical/atlas,ph-sm.txt     | 21 --------
>  .../bindings/iio/chemical/atlas,sensor.yaml   | 53 +++++++++++++++++++
>  4 files changed, 53 insertions(+), 63 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt
>  delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
>  delete mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt
> deleted file mode 100644
> index f4320595b851..000000000000
> --- a/Documentation/devicetree/bindings/iio/chemical/atlas,ec-sm.txt
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -* 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
> -  - 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/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
> deleted file mode 100644
> index af1f5a9aa4da..000000000000
> --- a/Documentation/devicetree/bindings/iio/chemical/atlas,orp-sm.txt
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -* 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
> -  - 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/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt b/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
> deleted file mode 100644
> index 79d90f060327..000000000000
> --- a/Documentation/devicetree/bindings/iio/chemical/atlas,ph-sm.txt
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -* Atlas Scientific pH-SM OEM sensor
> -
> -http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
> -
> -Required properties:
> -
> -  - compatible: must be "atlas,ph-sm"
> -  - reg: the I2C address of the sensor
> -  - interrupts: the sole interrupt generated by the device
> -
> -  Refer to interrupt-controller/interrupts.txt for generic interrupt client
> -  node bindings.
> -
> -Example:
> -
> -atlas@65 {
> -	compatible = "atlas,ph-sm";
> -	reg = <0x65>;
> -	interrupt-parent = <&gpio1>;
> -	interrupts = <16 2>;
> -};
> diff --git a/Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml b/Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml
> new file mode 100644
> index 000000000000..1b0c476838d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/atlas,sensor.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: GPL-2.0

Given this is entirely your binding, would you mind using the dual
license with GPL and BSD that is preferred for new bindings?

Otherwise looks good to me.

Jonathan

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/chemical/atlas,sensor.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atlas Scientific OEM sensors
> +
> +maintainers:
> +  - Matt Ranostay <matt.ranostay@konsulko.com>
> +
> +description: |
> +  Atlas Scientific OEM sensors connected via I2C
> +
> +  Datasheets:
> +    http://www.atlas-scientific.com/_files/_datasheets/_oem/DO_oem_datasheet.pdf
> +    http://www.atlas-scientific.com/_files/_datasheets/_oem/EC_oem_datasheet.pdf
> +    http://www.atlas-scientific.com/_files/_datasheets/_oem/ORP_oem_datasheet.pdf
> +    http://www.atlas-scientific.com/_files/_datasheets/_oem/pH_oem_datasheet.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - atlas,do-sm
> +      - atlas,ec-sm
> +      - atlas,orp-sm
> +      - atlas,ph-sm
> +
> +  reg:
> +     maxItems: 1
> +
> +  interrupts:
> +     maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      atlas@66 {
> +        compatible = "atlas,orp-sm";
> +        reg = <0x66>;
> +        interrupt-parent = <&gpio1>;
> +        interrupts = <16 2>;
> +      };
> +    };


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line
  2020-02-06  6:13 ` [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line Matt Ranostay
  2020-02-07  0:53   ` Jeremy Fertic
@ 2020-02-08 16:39   ` Jonathan Cameron
  2020-02-09  2:12     ` Matt Ranostay
  1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2020-02-08 16:39 UTC (permalink / raw)
  To: Matt Ranostay; +Cc: linux-iio

On Wed,  5 Feb 2020 22:13:30 -0800
Matt Ranostay <matt.ranostay@konsulko.com> wrote:

> Sensors don't actually need a interrupt line to give valid readings,
> and can triggered with CONFIG_IIO_HRTIMER_TRIGGER as well. Remove
> the required check for interrupt, and continue along in the probe
> function.

Basic aim is good, but if you don't have an interrupt doesn't make
sense to register the trigger either.

The interrupt enable / disable is also tied up with the buffer whereas
it should probably be done via the trigger enable callback or am I
missing something?

Jonathan

> 
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> ---
>  drivers/iio/chemical/atlas-sensor.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
> index 2f0a6fed2589..2e34c82cb65d 100644
> --- a/drivers/iio/chemical/atlas-sensor.c
> +++ b/drivers/iio/chemical/atlas-sensor.c
> @@ -572,11 +572,6 @@ static int atlas_probe(struct i2c_client *client,
>  	if (ret)
>  		return ret;
>  
> -	if (client->irq <= 0) {
> -		dev_err(&client->dev, "no valid irq defined\n");
> -		return -EINVAL;
> -	}
> -
>  	ret = chip->calibration(data);
>  	if (ret)
>  		return ret;
> @@ -596,16 +591,18 @@ static int atlas_probe(struct i2c_client *client,
>  
>  	init_irq_work(&data->work, atlas_work_handler);
>  
> -	/* interrupt pin toggles on new conversion */
> -	ret = devm_request_threaded_irq(&client->dev, client->irq,
> -					NULL, atlas_interrupt_handler,
> -					IRQF_TRIGGER_RISING |
> -					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> -					"atlas_irq",
> -					indio_dev);
> -	if (ret) {
> -		dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
> -		goto unregister_buffer;
> +	if (client->irq <= 0) {
> +		/* interrupt pin toggles on new conversion */
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +				NULL, atlas_interrupt_handler,
> +				IRQF_TRIGGER_RISING |
> +				IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +				"atlas_irq",
> +				indio_dev);
> +
> +		if (ret)
> +			dev_warn(&client->dev,
> +				"request irq (%d) failed\n", client->irq);
>  	}
>  
>  	ret = atlas_set_powermode(data, 1);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 2/3] iio: chemical: atlas-sensor: add DO-SM module support
  2020-02-06  6:13 ` [PATCH v4 2/3] iio: chemical: atlas-sensor: add DO-SM module support Matt Ranostay
@ 2020-02-08 16:41   ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2020-02-08 16:41 UTC (permalink / raw)
  To: Matt Ranostay; +Cc: linux-iio

On Wed,  5 Feb 2020 22:13:31 -0800
Matt Ranostay <matt.ranostay@konsulko.com> wrote:

> Atlas Scientific DO-SM OEM sensor reads disolved oxygen in
> a solution. This is reported back as mg/L which maps direc
> to ppm and so the IIO_CONCENTRATION channel type can be used.
> 
> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
Hi Matt,

This looks good to me. Will pick up once we've resolved the other
bits in the series.

Also want to give time for Rob to take a look a the yaml.

Thanks,

Jonathan

> ---
>  drivers/iio/chemical/atlas-sensor.c | 64 +++++++++++++++++++++++++++--
>  1 file changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
> index 2e34c82cb65d..919d408869be 100644
> --- a/drivers/iio/chemical/atlas-sensor.c
> +++ b/drivers/iio/chemical/atlas-sensor.c
> @@ -48,6 +48,11 @@
>  #define ATLAS_REG_EC_CALIB_STATUS_LOW		BIT(2)
>  #define ATLAS_REG_EC_CALIB_STATUS_HIGH		BIT(3)
>  
> +#define ATLAS_REG_DO_CALIB_STATUS		0x09
> +#define ATLAS_REG_DO_CALIB_STATUS_MASK		0x03
> +#define ATLAS_REG_DO_CALIB_STATUS_PRESSURE	BIT(0)
> +#define ATLAS_REG_DO_CALIB_STATUS_DO		BIT(1)
> +
>  #define ATLAS_REG_PH_TEMP_DATA		0x0e
>  #define ATLAS_REG_PH_DATA		0x16
>  
> @@ -60,14 +65,19 @@
>  #define ATLAS_REG_ORP_CALIB_STATUS	0x0d
>  #define ATLAS_REG_ORP_DATA		0x0e
>  
> +#define ATLAS_REG_DO_TEMP_DATA		0x12
> +#define ATLAS_REG_DO_DATA		0x22
> +
>  #define ATLAS_PH_INT_TIME_IN_MS		450
>  #define ATLAS_EC_INT_TIME_IN_MS		650
>  #define ATLAS_ORP_INT_TIME_IN_MS	450
> +#define ATLAS_DO_INT_TIME_IN_MS		450
>  
>  enum {
>  	ATLAS_PH_SM,
>  	ATLAS_EC_SM,
>  	ATLAS_ORP_SM,
> +	ATLAS_DO_SM,
>  };
>  
>  struct atlas_data {
> @@ -121,7 +131,7 @@ static const struct iio_chan_spec atlas_ph_channels[] = {
>  	},
>  };
>  
> -#define ATLAS_EC_CHANNEL(_idx, _addr) \
> +#define ATLAS_CONCENTRATION_CHANNEL(_idx, _addr) \
>  	{\
>  		.type = IIO_CONCENTRATION, \
>  		.indexed = 1, \
> @@ -152,8 +162,8 @@ static const struct iio_chan_spec atlas_ec_channels[] = {
>  			.endianness = IIO_BE,
>  		},
>  	},
> -	ATLAS_EC_CHANNEL(0, ATLAS_REG_TDS_DATA),
> -	ATLAS_EC_CHANNEL(1, ATLAS_REG_PSS_DATA),
> +	ATLAS_CONCENTRATION_CHANNEL(0, ATLAS_REG_TDS_DATA),
> +	ATLAS_CONCENTRATION_CHANNEL(1, ATLAS_REG_PSS_DATA),
>  	IIO_CHAN_SOFT_TIMESTAMP(3),
>  	{
>  		.type = IIO_TEMP,
> @@ -182,6 +192,19 @@ static const struct iio_chan_spec atlas_orp_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(1),
>  };
>  
> +static const struct iio_chan_spec atlas_do_channels[] = {
> +	ATLAS_CONCENTRATION_CHANNEL(0, ATLAS_REG_DO_DATA),
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +	{
> +		.type = IIO_TEMP,
> +		.address = ATLAS_REG_DO_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;
> @@ -262,7 +285,31 @@ static int atlas_check_orp_calibration(struct atlas_data *data)
>  		dev_warn(dev, "device has not been calibrated\n");
>  
>  	return 0;
> -};
> +}

Unrelated change so really should have been in a separate patch, but meh
it's trivial so I won't fuss too much about it being here ;)

> +
> +static int atlas_check_do_calibration(struct atlas_data *data)
> +{
> +	struct device *dev = &data->client->dev;
> +	int ret;
> +	unsigned int val;
> +
> +	ret = regmap_read(data->regmap, ATLAS_REG_DO_CALIB_STATUS, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (!(val & ATLAS_REG_DO_CALIB_STATUS_MASK)) {
> +		dev_warn(dev, "device has not been calibrated\n");
> +		return 0;
> +	}
> +
> +	if (!(val & ATLAS_REG_DO_CALIB_STATUS_PRESSURE))
> +		dev_warn(dev, "device missing atmospheric pressure calibration\n");
> +
> +	if (!(val & ATLAS_REG_DO_CALIB_STATUS_DO))
> +		dev_warn(dev, "device missing dissolved oxygen calibration\n");
> +
> +	return 0;
> +}
>  
>  struct atlas_device {
>  	const struct iio_chan_spec *channels;
> @@ -295,6 +342,13 @@ static struct atlas_device atlas_devices[] = {
>  				.calibration = &atlas_check_orp_calibration,
>  				.delay = ATLAS_ORP_INT_TIME_IN_MS,
>  	},
> +	[ATLAS_DO_SM] = {
> +				.channels = atlas_do_channels,
> +				.num_channels = 3,
> +				.data_reg = ATLAS_REG_DO_DATA,
> +				.calibration = &atlas_check_do_calibration,
> +				.delay = ATLAS_DO_INT_TIME_IN_MS,
> +	},
>  };
>  
>  static int atlas_set_powermode(struct atlas_data *data, int on)
> @@ -507,6 +561,7 @@ 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},
> +	{ "atlas-do-sm", ATLAS_DO_SM},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(i2c, atlas_id);
> @@ -515,6 +570,7 @@ 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, },
> +	{ .compatible = "atlas,do-sm", .data = (void *)ATLAS_DO_SM, },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, atlas_dt_ids);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line
  2020-02-08 16:39   ` Jonathan Cameron
@ 2020-02-09  2:12     ` Matt Ranostay
  2020-02-09  2:16       ` Matt Ranostay
  2020-02-14 13:27       ` Jonathan Cameron
  0 siblings, 2 replies; 13+ messages in thread
From: Matt Ranostay @ 2020-02-09  2:12 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio



> On Feb 8, 2020, at 08:39, Jonathan Cameron <jic23@kernel.org> wrote:
> 
> On Wed,  5 Feb 2020 22:13:30 -0800
> Matt Ranostay <matt.ranostay@konsulko.com> wrote:
> 
>> Sensors don't actually need a interrupt line to give valid readings,
>> and can triggered with CONFIG_IIO_HRTIMER_TRIGGER as well. Remove
>> the required check for interrupt, and continue along in the probe
>> function.
> 
> Basic aim is good, but if you don't have an interrupt doesn't make
> sense to register the trigger either.
> 
> The interrupt enable / disable is also tied up with the buffer whereas
> it should probably be done via the trigger enable callback or am I
> missing something?
 
It was for allowing sysfs and hrtimer triggers. But just remembered most these sensors have a low refresh rate. I can go either way on having it or not. Thoughts?

- Matt


> 
> Jonathan
> 
>> 
>> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
>> ---
>> drivers/iio/chemical/atlas-sensor.c | 27 ++++++++++++---------------
>> 1 file changed, 12 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
>> index 2f0a6fed2589..2e34c82cb65d 100644
>> --- a/drivers/iio/chemical/atlas-sensor.c
>> +++ b/drivers/iio/chemical/atlas-sensor.c
>> @@ -572,11 +572,6 @@ static int atlas_probe(struct i2c_client *client,
>>    if (ret)
>>        return ret;
>> 
>> -    if (client->irq <= 0) {
>> -        dev_err(&client->dev, "no valid irq defined\n");
>> -        return -EINVAL;
>> -    }
>> -
>>    ret = chip->calibration(data);
>>    if (ret)
>>        return ret;
>> @@ -596,16 +591,18 @@ static int atlas_probe(struct i2c_client *client,
>> 
>>    init_irq_work(&data->work, atlas_work_handler);
>> 
>> -    /* interrupt pin toggles on new conversion */
>> -    ret = devm_request_threaded_irq(&client->dev, client->irq,
>> -                    NULL, atlas_interrupt_handler,
>> -                    IRQF_TRIGGER_RISING |
>> -                    IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>> -                    "atlas_irq",
>> -                    indio_dev);
>> -    if (ret) {
>> -        dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
>> -        goto unregister_buffer;
>> +    if (client->irq <= 0) {
>> +        /* interrupt pin toggles on new conversion */
>> +        ret = devm_request_threaded_irq(&client->dev, client->irq,
>> +                NULL, atlas_interrupt_handler,
>> +                IRQF_TRIGGER_RISING |
>> +                IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>> +                "atlas_irq",
>> +                indio_dev);
>> +
>> +        if (ret)
>> +            dev_warn(&client->dev,
>> +                "request irq (%d) failed\n", client->irq);
>>    }
>> 
>>    ret = atlas_set_powermode(data, 1);
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line
  2020-02-09  2:12     ` Matt Ranostay
@ 2020-02-09  2:16       ` Matt Ranostay
  2020-02-14 13:27       ` Jonathan Cameron
  1 sibling, 0 replies; 13+ messages in thread
From: Matt Ranostay @ 2020-02-09  2:16 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio



> On Feb 8, 2020, at 18:12, Matt Ranostay <matt.ranostay@konsulko.com> wrote:
> 
> 
> 
>>> On Feb 8, 2020, at 08:39, Jonathan Cameron <jic23@kernel.org> wrote:
>>> 
>>> On Wed,  5 Feb 2020 22:13:30 -0800
>>> Matt Ranostay <matt.ranostay@konsulko.com> wrote:
>>> 
>>> Sensors don't actually need a interrupt line to give valid readings,
>>> and can triggered with CONFIG_IIO_HRTIMER_TRIGGER as well. Remove
>>> the required check for interrupt, and continue along in the probe
>>> function.
>> 
>> Basic aim is good, but if you don't have an interrupt doesn't make
>> sense to register the trigger either.
>> 
>> The interrupt enable / disable is also tied up with the buffer whereas
>> it should probably be done via the trigger enable callback or am I
>> missing something?
> 
> It was for allowing sysfs and hrtimer triggers. But just remembered most these sensors have a low refresh rate. I can go either way on having it or not. Thoughts?
> 
> - Matt
> 
 
Also issue one issue was fixed in v5 related to this.

- Matt
> 
>> 
>> Jonathan
>> 
>>> 
>>> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
>>> ---
>>> drivers/iio/chemical/atlas-sensor.c | 27 ++++++++++++---------------
>>> 1 file changed, 12 insertions(+), 15 deletions(-)
>>> 
>>> diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
>>> index 2f0a6fed2589..2e34c82cb65d 100644
>>> --- a/drivers/iio/chemical/atlas-sensor.c
>>> +++ b/drivers/iio/chemical/atlas-sensor.c
>>> @@ -572,11 +572,6 @@ static int atlas_probe(struct i2c_client *client,
>>>   if (ret)
>>>       return ret;
>>> 
>>> -    if (client->irq <= 0) {
>>> -        dev_err(&client->dev, "no valid irq defined\n");
>>> -        return -EINVAL;
>>> -    }
>>> -
>>>   ret = chip->calibration(data);
>>>   if (ret)
>>>       return ret;
>>> @@ -596,16 +591,18 @@ static int atlas_probe(struct i2c_client *client,
>>> 
>>>   init_irq_work(&data->work, atlas_work_handler);
>>> 
>>> -    /* interrupt pin toggles on new conversion */
>>> -    ret = devm_request_threaded_irq(&client->dev, client->irq,
>>> -                    NULL, atlas_interrupt_handler,
>>> -                    IRQF_TRIGGER_RISING |
>>> -                    IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>>> -                    "atlas_irq",
>>> -                    indio_dev);
>>> -    if (ret) {
>>> -        dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
>>> -        goto unregister_buffer;
>>> +    if (client->irq <= 0) {
>>> +        /* interrupt pin toggles on new conversion */
>>> +        ret = devm_request_threaded_irq(&client->dev, client->irq,
>>> +                NULL, atlas_interrupt_handler,
>>> +                IRQF_TRIGGER_RISING |
>>> +                IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>>> +                "atlas_irq",
>>> +                indio_dev);
>>> +
>>> +        if (ret)
>>> +            dev_warn(&client->dev,
>>> +                "request irq (%d) failed\n", client->irq);
>>>   }
>>> 
>>>   ret = atlas_set_powermode(data, 1);
>> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line
  2020-02-09  2:12     ` Matt Ranostay
  2020-02-09  2:16       ` Matt Ranostay
@ 2020-02-14 13:27       ` Jonathan Cameron
  1 sibling, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2020-02-14 13:27 UTC (permalink / raw)
  To: Matt Ranostay; +Cc: linux-iio

On Sat, 8 Feb 2020 18:12:03 -0800
Matt Ranostay <matt.ranostay@konsulko.com> wrote:

> > On Feb 8, 2020, at 08:39, Jonathan Cameron <jic23@kernel.org> wrote:
> > 
> > On Wed,  5 Feb 2020 22:13:30 -0800
> > Matt Ranostay <matt.ranostay@konsulko.com> wrote:
> >   
> >> Sensors don't actually need a interrupt line to give valid readings,
> >> and can triggered with CONFIG_IIO_HRTIMER_TRIGGER as well. Remove
> >> the required check for interrupt, and continue along in the probe
> >> function.  
> > 
> > Basic aim is good, but if you don't have an interrupt doesn't make
> > sense to register the trigger either.
> > 
> > The interrupt enable / disable is also tied up with the buffer whereas
> > it should probably be done via the trigger enable callback or am I
> > missing something?  
>  
> It was for allowing sysfs and hrtimer triggers. But just remembered most these sensors have a low refresh rate. I can go either way on having it or not. Thoughts?
I'm a bit confused.   With sysfs and hrtimer triggers why would we
need the interrupt enabled?  As things stand it will be as it's done
in the buffer setup.  I was suggesting moving it to the trigger
setup so we only deal with the interrupt if we are actually using
the data ready trigger.

So move it the atlas_set_interrupt call from pre / post enable to
the trigger state callback.

Jonathan
> 
> - Matt
> 
> 
> > 
> > Jonathan
> >   
> >> 
> >> Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
> >> ---
> >> drivers/iio/chemical/atlas-sensor.c | 27 ++++++++++++---------------
> >> 1 file changed, 12 insertions(+), 15 deletions(-)
> >> 
> >> diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
> >> index 2f0a6fed2589..2e34c82cb65d 100644
> >> --- a/drivers/iio/chemical/atlas-sensor.c
> >> +++ b/drivers/iio/chemical/atlas-sensor.c
> >> @@ -572,11 +572,6 @@ static int atlas_probe(struct i2c_client *client,
> >>    if (ret)
> >>        return ret;
> >> 
> >> -    if (client->irq <= 0) {
> >> -        dev_err(&client->dev, "no valid irq defined\n");
> >> -        return -EINVAL;
> >> -    }
> >> -
> >>    ret = chip->calibration(data);
> >>    if (ret)
> >>        return ret;
> >> @@ -596,16 +591,18 @@ static int atlas_probe(struct i2c_client *client,
> >> 
> >>    init_irq_work(&data->work, atlas_work_handler);
> >> 
> >> -    /* interrupt pin toggles on new conversion */
> >> -    ret = devm_request_threaded_irq(&client->dev, client->irq,
> >> -                    NULL, atlas_interrupt_handler,
> >> -                    IRQF_TRIGGER_RISING |
> >> -                    IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> >> -                    "atlas_irq",
> >> -                    indio_dev);
> >> -    if (ret) {
> >> -        dev_err(&client->dev, "request irq (%d) failed\n", client->irq);
> >> -        goto unregister_buffer;
> >> +    if (client->irq <= 0) {
> >> +        /* interrupt pin toggles on new conversion */
> >> +        ret = devm_request_threaded_irq(&client->dev, client->irq,
> >> +                NULL, atlas_interrupt_handler,
> >> +                IRQF_TRIGGER_RISING |
> >> +                IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> >> +                "atlas_irq",
> >> +                indio_dev);
> >> +
> >> +        if (ret)
> >> +            dev_warn(&client->dev,
> >> +                "request irq (%d) failed\n", client->irq);
> >>    }
> >> 
> >>    ret = atlas_set_powermode(data, 1);  
> >   


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-02-14 13:27 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-06  6:13 [PATCH v4 0/3] iio: chemical: atlas-sensor: add DO support Matt Ranostay
2020-02-06  6:13 ` [PATCH v4 1/3] iio: chemical: atlas-sensor: allow probe without interrupt line Matt Ranostay
2020-02-07  0:53   ` Jeremy Fertic
2020-02-07  4:59     ` Matt Ranostay
2020-02-08 16:39   ` Jonathan Cameron
2020-02-09  2:12     ` Matt Ranostay
2020-02-09  2:16       ` Matt Ranostay
2020-02-14 13:27       ` Jonathan Cameron
2020-02-06  6:13 ` [PATCH v4 2/3] iio: chemical: atlas-sensor: add DO-SM module support Matt Ranostay
2020-02-08 16:41   ` Jonathan Cameron
2020-02-06  6:13 ` [PATCH v4 3/3] dt-bindings: iio: chemical: consolidate atlas-sensor docs Matt Ranostay
2020-02-06 21:52   ` Rob Herring
2020-02-08 16:34   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).