[PATCH V3 1/4] staging: iio: adc: ad7192: Add low_pass_3db_filter_frequency

[PATCH V3 1/4] staging: iio: adc: ad7192: Add low_pass_3db_filter_frequency

[Mircea Caprioru]

By adding this option we are able to remove the sync3 field and dt binding.
When setting the required cutoff frequency we also determine the ADC
configuration for chop and sync filter.

Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
---
Changelog V2:
- no changes here

Changelog V3:
- no changes here

 drivers/staging/iio/adc/ad7192.c | 148 +++++++++++++++++++++++++++----
 1 file changed, 132 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 81ea2639c67c..d58ce08f3693 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -143,6 +143,10 @@
 #define AD7192_EXT_FREQ_MHZ_MAX	5120000
 #define AD7192_INT_FREQ_MHZ	4915200
 
+#define AD7192_NO_SYNC_FILTER	1
+#define AD7192_SYNC3_FILTER	3
+#define AD7192_SYNC4_FILTER	4
+
 /* NOTE:
  * The AD7190/2/5 features a dual use data out ready DOUT/RDY output.
  * In order to avoid contentions on the SPI bus, it's therefore necessary
@@ -250,7 +254,7 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
 static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
 {
 	struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
-	bool rej60_en, sinc3_en, refin2_en, chop_en;
+	bool rej60_en, refin2_en;
 	bool buf_en, bipolar, burnout_curr_en;
 	unsigned long long scale_uv;
 	int i, ret, id;
@@ -282,24 +286,12 @@ static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
 	if (rej60_en)
 		st->mode |= AD7192_MODE_REJ60;
 
-	sinc3_en = of_property_read_bool(np, "adi,sinc3-filter-enable");
-	if (sinc3_en)
-		st->mode |= AD7192_MODE_SINC3;
-
 	refin2_en = of_property_read_bool(np, "adi,refin2-pins-enable");
 	if (refin2_en && st->devid != ID_AD7195)
 		st->conf |= AD7192_CONF_REFSEL;
 
-	chop_en = of_property_read_bool(np, "adi,chop-enable");
-	if (chop_en) {
-		st->conf |= AD7192_CONF_CHOP;
-		if (sinc3_en)
-			st->f_order = 3; /* SINC 3rd order */
-		else
-			st->f_order = 4; /* SINC 4th order */
-	} else {
-		st->f_order = 1;
-	}
+	st->conf &= ~AD7192_CONF_CHOP;
+	st->f_order = AD7192_NO_SYNC_FILTER;
 
 	buf_en = of_property_read_bool(np, "adi,buffer-enable");
 	if (buf_en)
@@ -311,7 +303,7 @@ static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
 
 	burnout_curr_en = of_property_read_bool(np,
 						"adi,burnout-currents-enable");
-	if (burnout_curr_en && buf_en && !chop_en) {
+	if (burnout_curr_en && buf_en) {
 		st->conf |= AD7192_CONF_BURN;
 	} else if (burnout_curr_en) {
 		dev_warn(&st->sd.spi->dev,
@@ -409,6 +401,49 @@ static ssize_t ad7192_set(struct device *dev,
 	return ret ? ret : len;
 }
 
+static void ad7192_get_available_filter_freq(struct ad7192_state *st,
+						    int *freq)
+{
+	unsigned int fadc;
+
+	/* Formulas for filter at page 25 of the datasheet */
+	fadc = DIV_ROUND_CLOSEST(st->fclk,
+				 AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
+	freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
+
+	fadc = DIV_ROUND_CLOSEST(st->fclk,
+				 AD7192_SYNC3_FILTER * AD7192_MODE_RATE(st->mode));
+	freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
+
+	fadc = DIV_ROUND_CLOSEST(st->fclk, AD7192_MODE_RATE(st->mode));
+	freq[2] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
+	freq[3] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
+}
+
+static int ad7192_show_filter_avail(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct ad7192_state *st = iio_priv(indio_dev);
+	unsigned int freq_avail[4], i;
+	size_t len = 0;
+
+	ad7192_get_available_filter_freq(st, freq_avail);
+
+	for (i = 0; i < ARRAY_SIZE(freq_avail); i++)
+		len += scnprintf(buf + len, PAGE_SIZE - len,
+				 "%d.%d ", freq_avail[i] / 1000,
+				 freq_avail[i] % 1000);
+
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(filter_low_pass_3db_frequency_available,
+		       0444, ad7192_show_filter_avail, NULL, 0);
+
 static IIO_DEVICE_ATTR(bridge_switch_en, 0644,
 		       ad7192_show_bridge_switch, ad7192_set,
 		       AD7192_REG_GPOCON);
@@ -418,6 +453,7 @@ static IIO_DEVICE_ATTR(ac_excitation_en, 0644,
 		       AD7192_REG_MODE);
 
 static struct attribute *ad7192_attributes[] = {
+	&iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
 	&iio_dev_attr_bridge_switch_en.dev_attr.attr,
 	&iio_dev_attr_ac_excitation_en.dev_attr.attr,
 	NULL
@@ -428,6 +464,7 @@ static const struct attribute_group ad7192_attribute_group = {
 };
 
 static struct attribute *ad7195_attributes[] = {
+	&iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
 	&iio_dev_attr_bridge_switch_en.dev_attr.attr,
 	NULL
 };
@@ -441,6 +478,74 @@ static unsigned int ad7192_get_temp_scale(bool unipolar)
 	return unipolar ? 2815 * 2 : 2815;
 }
 
+static int ad7192_set_3db_filter_freq(struct ad7192_state *st,
+				      int val, int val2)
+{
+	int freq_avail[4], i, ret, idx, freq;
+	unsigned int diff_new, diff_old;
+
+	diff_old = U32_MAX;
+	freq = val * 1000 + val2;
+
+	ad7192_get_available_filter_freq(st, freq_avail);
+
+	for (i = 0; i < ARRAY_SIZE(freq_avail); i++) {
+		diff_new = abs(freq - freq_avail[i]);
+		if (diff_new < diff_old) {
+			diff_old = diff_new;
+			idx = i;
+		}
+	}
+
+	switch (idx) {
+	case 0:
+		st->f_order = AD7192_SYNC4_FILTER;
+		st->mode &= ~AD7192_MODE_SINC3;
+
+		st->conf |= AD7192_CONF_CHOP;
+		break;
+	case 1:
+		st->f_order = AD7192_SYNC3_FILTER;
+		st->mode |= AD7192_MODE_SINC3;
+
+		st->conf |= AD7192_CONF_CHOP;
+		break;
+	case 2:
+		st->f_order = AD7192_NO_SYNC_FILTER;
+		st->mode &= ~AD7192_MODE_SINC3;
+
+		st->conf &= ~AD7192_CONF_CHOP;
+		break;
+	case 3:
+		st->f_order = AD7192_NO_SYNC_FILTER;
+		st->mode |= AD7192_MODE_SINC3;
+
+		st->conf &= ~AD7192_CONF_CHOP;
+		break;
+	}
+
+	ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
+	if (ret < 0)
+		return ret;
+
+	return ad_sd_write_reg(&st->sd, AD7192_REG_CONF, 3, st->conf);
+}
+
+static int ad7192_get_3db_filter_freq(struct ad7192_state *st)
+{
+	unsigned int fadc;
+
+	fadc = DIV_ROUND_CLOSEST(st->fclk,
+				 st->f_order * AD7192_MODE_RATE(st->mode));
+
+	if (st->conf & AD7192_CONF_CHOP)
+		return DIV_ROUND_CLOSEST(fadc * 240, 1024);
+	if (st->mode & AD7192_MODE_SINC3)
+		return DIV_ROUND_CLOSEST(fadc * 272, 1024);
+	else
+		return DIV_ROUND_CLOSEST(fadc * 230, 1024);
+}
+
 static int ad7192_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val,
@@ -481,6 +586,10 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
 		*val = st->fclk /
 			(st->f_order * 1024 * AD7192_MODE_RATE(st->mode));
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		*val = ad7192_get_3db_filter_freq(st);
+		*val2 = 1000;
+		return IIO_VAL_FRACTIONAL;
 	}
 
 	return -EINVAL;
@@ -535,6 +644,9 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
 		st->mode |= AD7192_MODE_RATE(div);
 		ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
 		break;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		ret = ad7192_set_3db_filter_freq(st, val, val2 / 1000);
+		break;
 	default:
 		ret = -EINVAL;
 	}
@@ -553,6 +665,8 @@ static int ad7192_write_raw_get_fmt(struct iio_dev *indio_dev,
 		return IIO_VAL_INT_PLUS_NANO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}
@@ -653,6 +767,8 @@ static int ad7192_channels_config(struct iio_dev *indio_dev)
 
 	for (i = 0; i < indio_dev->num_channels; i++) {
 		*chan = channels[i];
+		chan->info_mask_shared_by_all |=
+			BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY);
 		if (chan->type != IIO_TEMP)
 			chan->info_mask_shared_by_type_available |=
 				BIT(IIO_CHAN_INFO_SCALE);
-- 
2.17.1

[PATCH V3 2/4] iio: adc: ad_sigma_delta: Export ad_sd_calibrate

[Mircea Caprioru]

This patch exports the ad_sd_calibrate function in order to be able to
call it from outside ad_sigma_delta.

There are cases where the option to calibrate one channel at a time is
necessary (ex. system calibration for zero scale and full scale).

Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
---
Changelog V2:
- no changes here

Changelog V3:
- no changes here

 drivers/iio/adc/ad_sigma_delta.c       | 3 ++-
 include/linux/iio/adc/ad_sigma_delta.h | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 2640b75fb774..8ba90486c787 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -205,7 +205,7 @@ int ad_sd_reset(struct ad_sigma_delta *sigma_delta,
 }
 EXPORT_SYMBOL_GPL(ad_sd_reset);
 
-static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
+int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
 	unsigned int mode, unsigned int channel)
 {
 	int ret;
@@ -242,6 +242,7 @@ static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ad_sd_calibrate);
 
 /**
  * ad_sd_calibrate_all() - Performs channel calibration
diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
index 7716fa0c9fce..8a4e25a7080c 100644
--- a/include/linux/iio/adc/ad_sigma_delta.h
+++ b/include/linux/iio/adc/ad_sigma_delta.h
@@ -119,6 +119,8 @@ int ad_sd_reset(struct ad_sigma_delta *sigma_delta,
 
 int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
 	const struct iio_chan_spec *chan, int *val);
+int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
+	unsigned int mode, unsigned int channel);
 int ad_sd_calibrate_all(struct ad_sigma_delta *sigma_delta,
 	const struct ad_sd_calib_data *cd, unsigned int n);
 int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
-- 
2.17.1

[PATCH V3 3/4] staging: iio: adc: ad7192: Add system calibration support

[Mircea Caprioru]

This patch will add a system calibration attribute for each channel. Using
this option the user will have the ability to calibrate each channel for
zero scale and full scale. It uses the iio_chan_spec_ext_info and IIO_ENUM
to implement the functionality.

Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
---
Changelog V2:
- no changes here

Changelog V3:
- no changes here

 drivers/staging/iio/adc/ad7192.c | 55 +++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index d58ce08f3693..731072830f30 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -155,6 +155,11 @@
  * The DOUT/RDY output must also be wired to an interrupt capable GPIO.
  */
 
+enum {
+   AD7192_SYSCALIB_ZERO_SCALE,
+   AD7192_SYSCALIB_FULL_SCALE,
+};
+
 struct ad7192_state {
 	struct regulator		*avdd;
 	struct regulator		*dvdd;
@@ -169,10 +174,56 @@ struct ad7192_state {
 	u8				devid;
 	u8				clock_sel;
 	struct mutex			lock;	/* protect sensor state */
+	u8				syscalib_mode[8];
 
 	struct ad_sigma_delta		sd;
 };
 
+static const char * const ad7192_syscalib_modes[] = {
+	[AD7192_SYSCALIB_ZERO_SCALE] = "zero_scale",
+	[AD7192_SYSCALIB_FULL_SCALE] = "full_scale",
+};
+
+static int ad7192_set_syscalib_mode(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    unsigned int mode)
+{
+	struct ad7192_state *st = iio_priv(indio_dev);
+	int ret;
+
+	st->syscalib_mode[chan->channel] = mode;
+
+	if (mode == AD7192_SYSCALIB_ZERO_SCALE)
+		ret = ad_sd_calibrate(&st->sd, AD7192_MODE_CAL_SYS_ZERO,
+				      chan->address);
+	else
+		ret = ad_sd_calibrate(&st->sd, AD7192_MODE_CAL_SYS_FULL,
+				      chan->address);
+
+	return ret;
+}
+
+static int ad7192_get_syscalib_mode(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan)
+{
+	struct ad7192_state *st = iio_priv(indio_dev);
+
+	return st->syscalib_mode[chan->channel];
+}
+
+static const struct iio_enum ad7192_syscalib_mode_enum = {
+	.items = ad7192_syscalib_modes,
+	.num_items = ARRAY_SIZE(ad7192_syscalib_modes),
+	.set = ad7192_set_syscalib_mode,
+	.get = ad7192_get_syscalib_mode
+};
+
+static const struct iio_chan_spec_ext_info ad7192_calibsys_ext_info[] = {
+	IIO_ENUM("system_calibration", IIO_SEPARATE, &ad7192_syscalib_mode_enum),
+	IIO_ENUM_AVAILABLE("system_calibration", &ad7192_syscalib_mode_enum),
+	{}
+};
+
 static struct ad7192_state *ad_sigma_delta_to_ad7192(struct ad_sigma_delta *sd)
 {
 	return container_of(sd, struct ad7192_state, sd);
@@ -769,9 +820,11 @@ static int ad7192_channels_config(struct iio_dev *indio_dev)
 		*chan = channels[i];
 		chan->info_mask_shared_by_all |=
 			BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY);
-		if (chan->type != IIO_TEMP)
+		if (chan->type != IIO_TEMP) {
 			chan->info_mask_shared_by_type_available |=
 				BIT(IIO_CHAN_INFO_SCALE);
+			chan->ext_info = ad7192_calibsys_ext_info;
+		}
 		chan++;
 	}
 
-- 
2.17.1

[PATCH V3 4/4] dt-bindings: iio: adc: ad7192: Add binding documentation for AD7192

[Mircea Caprioru]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 3928 bytes --]

This patch add device tree binding documentation for AD7192 adc in YAML
format.

Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
---
Changelog V2:
- remove description from spi and interrupt properties
- changed the name of the device from ad7192 to adc in the example

Changelog V3:
- added semicolon at the end of the dt example

 .../bindings/iio/adc/adi,ad7192.yaml          | 121 ++++++++++++++++++
 1 file changed, 121 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
new file mode 100644
index 000000000000..676ec42e1438
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
@@ -0,0 +1,121 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/iio/adc/adi,ad7192.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD7192 ADC device driver
+
+maintainers:
+  - Michael Hennerich <michael.hennerich@analog.com>
+
+description: |
+  Bindings for the Analog Devices AD7192 ADC device. Datasheet can be
+  found here:
+  https://www.analog.com/media/en/technical-documentation/data-sheets/AD7192.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7190
+      - adi,ad7192
+      - adi,ad7193
+      - adi,ad7195
+
+  reg:
+    maxItems: 1
+
+  spi-cpol: true
+
+  spi-cpha: true
+
+  clocks:
+    maxItems: 1
+    description: phandle to the master clock (mclk)
+
+  clock-names:
+    items:
+      - const: mclk
+
+  interrupts:
+    maxItems: 1
+
+  dvdd-supply:
+    description: DVdd voltage supply
+    items:
+      - const: dvdd
+
+  avdd-supply:
+    description: AVdd voltage supply
+    items:
+      - const: avdd
+
+  adi,rejection-60-Hz-enable:
+    description: |
+      This bit enables a notch at 60 Hz when the first notch of the sinc
+      filter is at 50 Hz. When REJ60 is set, a filter notch is placed at
+      60 Hz when the sinc filter first notch is at 50 Hz. This allows
+      simultaneous 50 Hz/ 60 Hz rejection.
+    type: boolean
+
+  adi,refin2-pins-enable:
+    description: |
+      External reference applied between the P1/REFIN2(+) and P0/REFIN2(−) pins.
+    type: boolean
+
+  adi,buffer-enable:
+    description: |
+      Enables the buffer on the analog inputs. If cleared, the analog inputs
+      are unbuffered, lowering the power consumption of the device. If this
+      bit is set, the analog inputs are buffered, allowing the user to place
+      source impedances on the front end without contributing gain errors to
+      the system.
+    type: boolean
+
+  adi,burnout-currents-enable:
+    description: |
+      When this bit is set to 1, the 500 nA current sources in the signal
+      path are enabled. When BURN = 0, the burnout currents are disabled.
+      The burnout currents can be enabled only when the buffer is active
+      and when chop is disabled.
+    type: boolean
+
+  bipolar:
+    description: see Documentation/devicetree/bindings/iio/adc/adc.txt
+    type: boolean
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - dvdd-supply
+  - avdd-supply
+  - spi-cpol
+  - spi-cpha
+
+examples:
+  - |
+    spi0 {
+      adc@0 {
+        compatible = "adi,ad7192";
+        reg = <0>;
+        spi-max-frequency = <1000000>;
+        spi-cpol;
+        spi-cpha;
+        clocks = <&ad7192_mclk>;
+        clock-names = "mclk";
+        #interrupt-cells = <2>;
+        interrupts = <25 0x2>;
+        interrupt-parent = <&gpio>;
+        dvdd-supply = <&dvdd>;
+        avdd-supply = <&avdd>;
+
+        adi,refin2-pins-enable;
+        adi,rejection-60-Hz-enable;
+        adi,buffer-enable;
+        adi,burnout-currents-enable;
+        };
+    };
-- 
2.17.1

Re: [PATCH V3 4/4] dt-bindings: iio: adc: ad7192: Add binding documentation for AD7192

[Rob Herring]

On Wed, Aug 14, 2019 at 1:32 AM Mircea Caprioru
<mircea.caprioru@analog.com> wrote:
>
> This patch add device tree binding documentation for AD7192 adc in YAML
> format.
>
> Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
> ---
> Changelog V2:
> - remove description from spi and interrupt properties
> - changed the name of the device from ad7192 to adc in the example
>
> Changelog V3:
> - added semicolon at the end of the dt example
>
>  .../bindings/iio/adc/adi,ad7192.yaml          | 121 ++++++++++++++++++
>  1 file changed, 121 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml

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

Re: [PATCH V3 1/4] staging: iio: adc: ad7192: Add low_pass_3db_filter_frequency

[Jonathan Cameron]

On Wed, 14 Aug 2019 10:31:47 +0300
Mircea Caprioru <mircea.caprioru@analog.com> wrote:

> By adding this option we are able to remove the sync3 field and dt binding.
> When setting the required cutoff frequency we also determine the ADC
> configuration for chop and sync filter.
> 
> Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
A few things turned up in my x86_64 test build for this one.

I've fixed them up as below and applied to the togreg branch of iio.git
and pushed out as testing for the autobuilders to see what else I missed ;)

Thanks,

Jonathan

> ---
> Changelog V2:
> - no changes here
> 
> Changelog V3:
> - no changes here
> 
>  drivers/staging/iio/adc/ad7192.c | 148 +++++++++++++++++++++++++++----
>  1 file changed, 132 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index 81ea2639c67c..d58ce08f3693 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -143,6 +143,10 @@
>  #define AD7192_EXT_FREQ_MHZ_MAX	5120000
>  #define AD7192_INT_FREQ_MHZ	4915200
>  
> +#define AD7192_NO_SYNC_FILTER	1
> +#define AD7192_SYNC3_FILTER	3
> +#define AD7192_SYNC4_FILTER	4
> +
>  /* NOTE:
>   * The AD7190/2/5 features a dual use data out ready DOUT/RDY output.
>   * In order to avoid contentions on the SPI bus, it's therefore necessary
> @@ -250,7 +254,7 @@ static int ad7192_of_clock_select(struct ad7192_state *st)
>  static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
>  {
>  	struct iio_dev *indio_dev = spi_get_drvdata(st->sd.spi);
> -	bool rej60_en, sinc3_en, refin2_en, chop_en;
> +	bool rej60_en, refin2_en;
>  	bool buf_en, bipolar, burnout_curr_en;
>  	unsigned long long scale_uv;
>  	int i, ret, id;
> @@ -282,24 +286,12 @@ static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
>  	if (rej60_en)
>  		st->mode |= AD7192_MODE_REJ60;
>  
> -	sinc3_en = of_property_read_bool(np, "adi,sinc3-filter-enable");
> -	if (sinc3_en)
> -		st->mode |= AD7192_MODE_SINC3;
> -
>  	refin2_en = of_property_read_bool(np, "adi,refin2-pins-enable");
>  	if (refin2_en && st->devid != ID_AD7195)
>  		st->conf |= AD7192_CONF_REFSEL;
>  
> -	chop_en = of_property_read_bool(np, "adi,chop-enable");
> -	if (chop_en) {
> -		st->conf |= AD7192_CONF_CHOP;
> -		if (sinc3_en)
> -			st->f_order = 3; /* SINC 3rd order */
> -		else
> -			st->f_order = 4; /* SINC 4th order */
> -	} else {
> -		st->f_order = 1;
> -	}
> +	st->conf &= ~AD7192_CONF_CHOP;
> +	st->f_order = AD7192_NO_SYNC_FILTER;
>  
>  	buf_en = of_property_read_bool(np, "adi,buffer-enable");
>  	if (buf_en)
> @@ -311,7 +303,7 @@ static int ad7192_setup(struct ad7192_state *st, struct device_node *np)
>  
>  	burnout_curr_en = of_property_read_bool(np,
>  						"adi,burnout-currents-enable");
> -	if (burnout_curr_en && buf_en && !chop_en) {
> +	if (burnout_curr_en && buf_en) {
>  		st->conf |= AD7192_CONF_BURN;
>  	} else if (burnout_curr_en) {
>  		dev_warn(&st->sd.spi->dev,
> @@ -409,6 +401,49 @@ static ssize_t ad7192_set(struct device *dev,
>  	return ret ? ret : len;
>  }
>  
> +static void ad7192_get_available_filter_freq(struct ad7192_state *st,
> +						    int *freq)
> +{
> +	unsigned int fadc;
> +
> +	/* Formulas for filter at page 25 of the datasheet */
> +	fadc = DIV_ROUND_CLOSEST(st->fclk,
> +				 AD7192_SYNC4_FILTER * AD7192_MODE_RATE(st->mode));
> +	freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
> +
> +	fadc = DIV_ROUND_CLOSEST(st->fclk,
> +				 AD7192_SYNC3_FILTER * AD7192_MODE_RATE(st->mode));
> +	freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
> +
> +	fadc = DIV_ROUND_CLOSEST(st->fclk, AD7192_MODE_RATE(st->mode));
> +	freq[2] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
> +	freq[3] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
> +}
> +
> +static int ad7192_show_filter_avail(struct device *dev,

ssize_t for a show function return type.

> +				    struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct ad7192_state *st = iio_priv(indio_dev);
> +	unsigned int freq_avail[4], i;
> +	size_t len = 0;
> +
> +	ad7192_get_available_filter_freq(st, freq_avail);
> +
> +	for (i = 0; i < ARRAY_SIZE(freq_avail); i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len,
> +				 "%d.%d ", freq_avail[i] / 1000,
> +				 freq_avail[i] % 1000);
> +
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
> +static IIO_DEVICE_ATTR(filter_low_pass_3db_frequency_available,
> +		       0444, ad7192_show_filter_avail, NULL, 0);
> +
>  static IIO_DEVICE_ATTR(bridge_switch_en, 0644,
>  		       ad7192_show_bridge_switch, ad7192_set,
>  		       AD7192_REG_GPOCON);
> @@ -418,6 +453,7 @@ static IIO_DEVICE_ATTR(ac_excitation_en, 0644,
>  		       AD7192_REG_MODE);
>  
>  static struct attribute *ad7192_attributes[] = {
> +	&iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
>  	&iio_dev_attr_bridge_switch_en.dev_attr.attr,
>  	&iio_dev_attr_ac_excitation_en.dev_attr.attr,
>  	NULL
> @@ -428,6 +464,7 @@ static const struct attribute_group ad7192_attribute_group = {
>  };
>  
>  static struct attribute *ad7195_attributes[] = {
> +	&iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
>  	&iio_dev_attr_bridge_switch_en.dev_attr.attr,
>  	NULL
>  };
> @@ -441,6 +478,74 @@ static unsigned int ad7192_get_temp_scale(bool unipolar)
>  	return unipolar ? 2815 * 2 : 2815;
>  }
>  
> +static int ad7192_set_3db_filter_freq(struct ad7192_state *st,
> +				      int val, int val2)
> +{
> +	int freq_avail[4], i, ret, idx, freq;
> +	unsigned int diff_new, diff_old;
> +
At least some compilers are unable to tell idx is always intialized.

> +	diff_old = U32_MAX;
> +	freq = val * 1000 + val2;
> +
> +	ad7192_get_available_filter_freq(st, freq_avail);
> +
> +	for (i = 0; i < ARRAY_SIZE(freq_avail); i++) {
> +		diff_new = abs(freq - freq_avail[i]);
> +		if (diff_new < diff_old) {
> +			diff_old = diff_new;
> +			idx = i;
> +		}
> +	}
> +
> +	switch (idx) {
> +	case 0:
> +		st->f_order = AD7192_SYNC4_FILTER;
> +		st->mode &= ~AD7192_MODE_SINC3;
> +
> +		st->conf |= AD7192_CONF_CHOP;
> +		break;
> +	case 1:
> +		st->f_order = AD7192_SYNC3_FILTER;
> +		st->mode |= AD7192_MODE_SINC3;
> +
> +		st->conf |= AD7192_CONF_CHOP;
> +		break;
> +	case 2:
> +		st->f_order = AD7192_NO_SYNC_FILTER;
> +		st->mode &= ~AD7192_MODE_SINC3;
> +
> +		st->conf &= ~AD7192_CONF_CHOP;
> +		break;
> +	case 3:
> +		st->f_order = AD7192_NO_SYNC_FILTER;
> +		st->mode |= AD7192_MODE_SINC3;
> +
> +		st->conf &= ~AD7192_CONF_CHOP;
> +		break;
> +	}
> +
> +	ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
> +	if (ret < 0)
> +		return ret;
> +
> +	return ad_sd_write_reg(&st->sd, AD7192_REG_CONF, 3, st->conf);
> +}
> +
> +static int ad7192_get_3db_filter_freq(struct ad7192_state *st)
> +{
> +	unsigned int fadc;
> +
> +	fadc = DIV_ROUND_CLOSEST(st->fclk,
> +				 st->f_order * AD7192_MODE_RATE(st->mode));
> +
> +	if (st->conf & AD7192_CONF_CHOP)
> +		return DIV_ROUND_CLOSEST(fadc * 240, 1024);
> +	if (st->mode & AD7192_MODE_SINC3)
> +		return DIV_ROUND_CLOSEST(fadc * 272, 1024);
> +	else
> +		return DIV_ROUND_CLOSEST(fadc * 230, 1024);
> +}
> +
>  static int ad7192_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val,
> @@ -481,6 +586,10 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
>  		*val = st->fclk /
>  			(st->f_order * 1024 * AD7192_MODE_RATE(st->mode));
>  		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		*val = ad7192_get_3db_filter_freq(st);
> +		*val2 = 1000;
> +		return IIO_VAL_FRACTIONAL;
>  	}
>  
>  	return -EINVAL;
> @@ -535,6 +644,9 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
>  		st->mode |= AD7192_MODE_RATE(div);
>  		ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
>  		break;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		ret = ad7192_set_3db_filter_freq(st, val, val2 / 1000);
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> @@ -553,6 +665,8 @@ static int ad7192_write_raw_get_fmt(struct iio_dev *indio_dev,
>  		return IIO_VAL_INT_PLUS_NANO;
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		return IIO_VAL_INT_PLUS_MICRO;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -653,6 +767,8 @@ static int ad7192_channels_config(struct iio_dev *indio_dev)
>  
>  	for (i = 0; i < indio_dev->num_channels; i++) {
>  		*chan = channels[i];
> +		chan->info_mask_shared_by_all |=
> +			BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY);
>  		if (chan->type != IIO_TEMP)
>  			chan->info_mask_shared_by_type_available |=
>  				BIT(IIO_CHAN_INFO_SCALE);

Re: [PATCH V3 2/4] iio: adc: ad_sigma_delta: Export ad_sd_calibrate

[Jonathan Cameron]

On Wed, 14 Aug 2019 10:31:48 +0300
Mircea Caprioru <mircea.caprioru@analog.com> wrote:

> This patch exports the ad_sd_calibrate function in order to be able to
> call it from outside ad_sigma_delta.
> 
> There are cases where the option to calibrate one channel at a time is
> necessary (ex. system calibration for zero scale and full scale).
> 
> Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
Applied,

Thanks,

Jonathan

> ---
> Changelog V2:
> - no changes here
> 
> Changelog V3:
> - no changes here
> 
>  drivers/iio/adc/ad_sigma_delta.c       | 3 ++-
>  include/linux/iio/adc/ad_sigma_delta.h | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
> index 2640b75fb774..8ba90486c787 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -205,7 +205,7 @@ int ad_sd_reset(struct ad_sigma_delta *sigma_delta,
>  }
>  EXPORT_SYMBOL_GPL(ad_sd_reset);
>  
> -static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
> +int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
>  	unsigned int mode, unsigned int channel)
>  {
>  	int ret;
> @@ -242,6 +242,7 @@ static int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(ad_sd_calibrate);
>  
>  /**
>   * ad_sd_calibrate_all() - Performs channel calibration
> diff --git a/include/linux/iio/adc/ad_sigma_delta.h b/include/linux/iio/adc/ad_sigma_delta.h
> index 7716fa0c9fce..8a4e25a7080c 100644
> --- a/include/linux/iio/adc/ad_sigma_delta.h
> +++ b/include/linux/iio/adc/ad_sigma_delta.h
> @@ -119,6 +119,8 @@ int ad_sd_reset(struct ad_sigma_delta *sigma_delta,
>  
>  int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
>  	const struct iio_chan_spec *chan, int *val);
> +int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
> +	unsigned int mode, unsigned int channel);
>  int ad_sd_calibrate_all(struct ad_sigma_delta *sigma_delta,
>  	const struct ad_sd_calib_data *cd, unsigned int n);
>  int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,

Re: [PATCH V3 3/4] staging: iio: adc: ad7192: Add system calibration support

[Jonathan Cameron]

On Wed, 14 Aug 2019 10:31:49 +0300
Mircea Caprioru <mircea.caprioru@analog.com> wrote:

> This patch will add a system calibration attribute for each channel. Using
> this option the user will have the ability to calibrate each channel for
> zero scale and full scale. It uses the iio_chan_spec_ext_info and IIO_ENUM
> to implement the functionality.
> 
> Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>

Hi, 

This introduces new ABI so needs documentation in
Documentation/ABI/testing/...

I'm not particularly keen on a write to what might be considered
a mode select register resulting in a calibration starting.
That is rather non obvious, I'd prefer to either two attributes
to trigger the two modes, or a mode attr and some sort of calibrate
now attribute.

I'll back out patch 2 for now, as it was there to support this.

Thanks,

Jonathan


> ---
> Changelog V2:
> - no changes here
> 
> Changelog V3:
> - no changes here
> 
>  drivers/staging/iio/adc/ad7192.c | 55 +++++++++++++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index d58ce08f3693..731072830f30 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -155,6 +155,11 @@
>   * The DOUT/RDY output must also be wired to an interrupt capable GPIO.
>   */
>  
> +enum {
> +   AD7192_SYSCALIB_ZERO_SCALE,
> +   AD7192_SYSCALIB_FULL_SCALE,
> +};
> +
>  struct ad7192_state {
>  	struct regulator		*avdd;
>  	struct regulator		*dvdd;
> @@ -169,10 +174,56 @@ struct ad7192_state {
>  	u8				devid;
>  	u8				clock_sel;
>  	struct mutex			lock;	/* protect sensor state */
> +	u8				syscalib_mode[8];
>  
>  	struct ad_sigma_delta		sd;
>  };
>  
> +static const char * const ad7192_syscalib_modes[] = {
> +	[AD7192_SYSCALIB_ZERO_SCALE] = "zero_scale",
> +	[AD7192_SYSCALIB_FULL_SCALE] = "full_scale",
> +};
> +
> +static int ad7192_set_syscalib_mode(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    unsigned int mode)
> +{
> +	struct ad7192_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	st->syscalib_mode[chan->channel] = mode;
> +
> +	if (mode == AD7192_SYSCALIB_ZERO_SCALE)
> +		ret = ad_sd_calibrate(&st->sd, AD7192_MODE_CAL_SYS_ZERO,
> +				      chan->address);
> +	else
> +		ret = ad_sd_calibrate(&st->sd, AD7192_MODE_CAL_SYS_FULL,
> +				      chan->address);
> +
> +	return ret;
> +}
> +
> +static int ad7192_get_syscalib_mode(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan)
> +{
> +	struct ad7192_state *st = iio_priv(indio_dev);
> +
> +	return st->syscalib_mode[chan->channel];
> +}
> +
> +static const struct iio_enum ad7192_syscalib_mode_enum = {
> +	.items = ad7192_syscalib_modes,
> +	.num_items = ARRAY_SIZE(ad7192_syscalib_modes),
> +	.set = ad7192_set_syscalib_mode,
> +	.get = ad7192_get_syscalib_mode
> +};
> +
> +static const struct iio_chan_spec_ext_info ad7192_calibsys_ext_info[] = {
> +	IIO_ENUM("system_calibration", IIO_SEPARATE, &ad7192_syscalib_mode_enum),
> +	IIO_ENUM_AVAILABLE("system_calibration", &ad7192_syscalib_mode_enum),
> +	{}
> +};
> +
>  static struct ad7192_state *ad_sigma_delta_to_ad7192(struct ad_sigma_delta *sd)
>  {
>  	return container_of(sd, struct ad7192_state, sd);
> @@ -769,9 +820,11 @@ static int ad7192_channels_config(struct iio_dev *indio_dev)
>  		*chan = channels[i];
>  		chan->info_mask_shared_by_all |=
>  			BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY);
> -		if (chan->type != IIO_TEMP)
> +		if (chan->type != IIO_TEMP) {
>  			chan->info_mask_shared_by_type_available |=
>  				BIT(IIO_CHAN_INFO_SCALE);
> +			chan->ext_info = ad7192_calibsys_ext_info;
> +		}
>  		chan++;
>  	}
>  

Re: [PATCH V3 4/4] dt-bindings: iio: adc: ad7192: Add binding documentation for AD7192

[Jonathan Cameron]

On Wed, 14 Aug 2019 20:39:04 -0600
Rob Herring <robh+dt@kernel.org> wrote:

> On Wed, Aug 14, 2019 at 1:32 AM Mircea Caprioru
> <mircea.caprioru@analog.com> wrote:
> >
> > This patch add device tree binding documentation for AD7192 adc in YAML
> > format.
> >
> > Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
> > ---
> > Changelog V2:
> > - remove description from spi and interrupt properties
> > - changed the name of the device from ad7192 to adc in the example
> >
> > Changelog V3:
> > - added semicolon at the end of the dt example
> >
> >  .../bindings/iio/adc/adi,ad7192.yaml          | 121 ++++++++++++++++++
> >  1 file changed, 121 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml  
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

For some reason, this patch gave me a git error based on encoding. 
I applied it by hand instead and all seemed fine.  Not sure why
that happened!

Applied to the togreg branch of iio.git and pushed out as testing
so the autobuilders can play with it.

Thanks,

Jonathan

Re: [PATCH V3 4/4] dt-bindings: iio: adc: ad7192: Add binding documentation for AD7192

[Jonathan Cameron]

On Sun, 18 Aug 2019 19:46:27 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed, 14 Aug 2019 20:39:04 -0600
> Rob Herring <robh+dt@kernel.org> wrote:
> 
> > On Wed, Aug 14, 2019 at 1:32 AM Mircea Caprioru
> > <mircea.caprioru@analog.com> wrote:  
> > >
> > > This patch add device tree binding documentation for AD7192 adc in YAML
> > > format.
> > >
> > > Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
> > > ---
> > > Changelog V2:
> > > - remove description from spi and interrupt properties
> > > - changed the name of the device from ad7192 to adc in the example
> > >
> > > Changelog V3:
> > > - added semicolon at the end of the dt example
> > >
> > >  .../bindings/iio/adc/adi,ad7192.yaml          | 121 ++++++++++++++++++
> > >  1 file changed, 121 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml    
> > 
> > Reviewed-by: Rob Herring <robh@kernel.org>  
> 
> For some reason, this patch gave me a git error based on encoding. 
> I applied it by hand instead and all seemed fine.  Not sure why
> that happened!
> 
> Applied to the togreg branch of iio.git and pushed out as testing
> so the autobuilders can play with it.
> 
> Thanks,
> 
> Jonathan
> 
I spoke a bit soon as the build test was still running.

you have const values for the regulators - that doesn't make much sense
to my mind and means your example gives warnings...

    items:
      - const: dvdd
/iio/Documentation/devicetree/bindings/iio/adc/adi,ad7192.example.dt.yaml: adc@0: dvdd-supply:0: 'dvdd' was expected

I've dropped this and will pick up in v4.

thanks,

Jonathan