[PATCH v3 00/17] staging/iio: Clean up AD7746 CDC driver and move from staging.

[PATCH v3 00/17] staging/iio: Clean up AD7746 CDC driver and move from staging.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Changes since v2:  Thanks to Andy Shevchenko for review
- Fix the horrible mess I made of a rebase that meant v2 didn't build
  mid patch set.
- Fix various build issues I'd missed due to wrong make file (fails to
  update variables after moving them, missing unaligned include,
  failure to cleanup staging driver Kconfig / Makefile)
- Change new ABI to _zeropoint and add more the description to hopefully
  add clarify to what this is. I don't think it's going to be a particularly
  common bit of ABI, but still good to get it right.

Note that the --no-renames was used to preserve full code listing
for the actual move out of staging to aid review. The ad7746 file is
moved with no changes.

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c
similarity index 100%
rename from drivers/staging/iio/cdc/ad7746.c
rename to drivers/iio/cdc/ad7746.c

This started out as an experiment with Vincent Whitchurch's roadtest
testing framework [1] and that worked well so I carried on cleaning up the
driver.  Note that if you are using roadtest rebased to current
kernel, add CONFIG_UML_RANDOM as suggested by Vincent in reply
to v2 of this series.  I still saw one freeze but with that config
element added roadtest runs the tests successfully the vast majority
of the time.

Mostly this is standard tidy up, move to new interfaces, then move the driver
out of staging, but there are a few other things in here.

1) Precision improvement for IIO_VAL_FRACTIONAL_LOG2.
   The ad7746 is a 24 bit sensor and this highlighted that 9 decimal
   places wasn't enough to keep the error introduced by
   _raw * _scale from growing too large over the whole range (I couldn't
   write a sensible test for it).  So I've proposed increasing the precision
   of the calculation used to provide the sysfs attributes with this value
   type and printing a few more decimal places (12), but only if overflow
   will not occur due to val[0] > ULLONG_MAX / PICO
2) _zeropoint ABI addition.  This driver had an odd use of _offset for
   the case of differential channels that applied the offset to both
   of the differential pair (hence userspace shouldn't not apply it when
   converting to the base units. That isn't inline with the existing
   documentation for _offset and it wasn't clear to me that it made sense
   at all.  To avoid confusion I've added this new ABI (_zeropoint) for this.
3) roadtest file - note this is not a complete test set for the driver and
   mainly focused on the main channel reads and places I thought I might
   have broken things whilst working on the driver.  Given roadtest isn't
   upstream yet this test will have to wait. That doesn't block merging
   the rest of the series.

My conclusion on roadtest - Very useful indeed. I'd encourage others to
consider developing some basic sanity tests for drivers they are working on.
Hopefully my python code isn't too hideous to understand at least!
Vincent, it might be worth thinking about some generic code to handle the
'variants' on correct ABI like I introduce here because I switched from
a shared by type scale to an individual one per channel for the voltages.
Both were ABI compliant so that sort of change is fine most of the time
though we have to be careful with it.

All comments welcome.  Note there may be changes that make more sense
to do after moving this out of staging as long as there are no ABI changes involved
etc.  Feel free to highlight those sorts of changes as well as anything more
significant.

[1] https://lore.kernel.org/all/20220311162445.346685-9-vincent.whitchurch@axis.com/

Jonathan Cameron (17):
  iio: core: Increase precision of IIO_VAL_FRACTIONAL_LOG2 when possible
  iio: ABI: Fix wrong format of differential capacitance channel ABI.
  staging: iio: cdc: ad7746: Use explicit be24 handling.
  staging: iio: cdc: ad7746: Push handling of supply voltage scale to
    userspace.
  staging: iio: cdc: ad7746: Use local buffer for multi byte reads.
  staging: iio: cdc: ad7746: Factor out ad7746_read_channel()
  staging: iio: cdc: ad7764: Push locking down into case statements in
    read/write_raw
  staging: iio: cdc: ad7746: Break up use of chan->address and use
    FIELD_PREP etc
  staging: iio: cdc: ad7746: Drop unused i2c_set_clientdata()
  staging: iio: cdc: ad7746: Use _raw and _scale for temperature
    channels.
  iio: core: Introduce _zeropoint for differential channels
  staging: iio: cdc: ad7746: Switch from _offset to _zeropoint for
    differential channels.
  staging: iio: cdc: ad7746: Use read_avail() rather than opencoding.
  staging: iio: ad7746: White space cleanup
  iio: cdc: ad7746: Add device specific ABI documentation.
  iio: cdc: ad7746: Move driver out of staging.
  RFC: iio: cdc: ad7746: Add roadtest

 Documentation/ABI/testing/sysfs-bus-iio       |  21 +-
 .../ABI/testing/sysfs-bus-iio-cdc-ad7746      |  11 +
 drivers/iio/cdc/Kconfig                       |  10 +
 drivers/iio/cdc/Makefile                      |   1 +
 drivers/iio/cdc/ad7746.c                      | 820 ++++++++++++++++++
 drivers/iio/industrialio-core.c               |  32 +-
 drivers/staging/iio/Kconfig                   |   1 -
 drivers/staging/iio/Makefile                  |   1 -
 drivers/staging/iio/cdc/Kconfig               |  17 -
 drivers/staging/iio/cdc/Makefile              |   6 -
 drivers/staging/iio/cdc/ad7746.c              | 767 ----------------
 include/linux/iio/types.h                     |   1 +
 .../roadtest/tests/iio/cdc/__init__.py        |   0
 .../roadtest/roadtest/tests/iio/cdc/config    |   1 +
 .../roadtest/tests/iio/cdc/test_ad7746.py     | 323 +++++++
 15 files changed, 1211 insertions(+), 801 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cdc-ad7746
 create mode 100644 drivers/iio/cdc/ad7746.c
 delete mode 100644 drivers/staging/iio/cdc/Kconfig
 delete mode 100644 drivers/staging/iio/cdc/Makefile
 delete mode 100644 drivers/staging/iio/cdc/ad7746.c
 create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/__init__.py
 create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/config
 create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/test_ad7746.py

-- 
2.36.1

[PATCH v3 01/17] iio: core: Increase precision of IIO_VAL_FRACTIONAL_LOG2 when possible

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

With some high resolution sensors such as the ad7746 the build up of error
when multiplying the _raw and _scale values together can be significant.
Reduce this affect by providing additional resolution in both calculation
and formatting of result. If overflow would occur with a 1e12 multiplier,
fall back to the 1e9 used before this patch and 9 decimal places.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/industrialio-core.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index dc3e1cb9bfbd..8225d0c43010 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -18,6 +18,7 @@
 #include <linux/poll.h>
 #include <linux/property.h>
 #include <linux/sched.h>
+#include <linux/units.h>
 #include <linux/wait.h>
 #include <linux/cdev.h>
 #include <linux/slab.h>
@@ -674,14 +675,28 @@ static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
 		else
 			return sysfs_emit_at(buf, offset, "%d.%09u", tmp0,
 					     abs(tmp1));
-	case IIO_VAL_FRACTIONAL_LOG2:
-		tmp2 = shift_right((s64)vals[0] * 1000000000LL, vals[1]);
-		tmp0 = (int)div_s64_rem(tmp2, 1000000000LL, &tmp1);
-		if (tmp0 == 0 && tmp2 < 0)
-			return sysfs_emit_at(buf, offset, "-0.%09u", abs(tmp1));
-		else
-			return sysfs_emit_at(buf, offset, "%d.%09u", tmp0,
-					     abs(tmp1));
+	case IIO_VAL_FRACTIONAL_LOG2: {
+		u64 t1, t2, mult;
+		int integer, precision;
+		bool neg = vals[0] < 0;
+
+		if (vals[0] > ULLONG_MAX / PICO) {
+			mult = NANO;
+			precision = 9;
+		} else {
+			mult = PICO;
+			precision = 12;
+		}
+		t1 = shift_right((u64)abs(vals[0]) * mult, vals[1]);
+		integer = (int)div64_u64_rem(t1, mult, &t2);
+		if (integer == 0 && neg)
+			return sysfs_emit_at(buf, offset, "-0.%0*llu",
+					     precision, abs(t2));
+		if (neg)
+			integer *= -1;
+		return sysfs_emit_at(buf, offset, "%d.%0*llu", integer,
+				     precision, abs(t2));
+	}
 	case IIO_VAL_INT_MULTIPLE:
 	{
 		int i;
-- 
2.36.1

Re: [PATCH v3 01/17] iio: core: Increase precision of IIO_VAL_FRACTIONAL_LOG2 when possible

[Jonathan Cameron]

On Sun, 26 Jun 2022 13:29:22 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> With some high resolution sensors such as the ad7746 the build up of error
> when multiplying the _raw and _scale values together can be significant.
> Reduce this affect by providing additional resolution in both calculation
> and formatting of result. If overflow would occur with a 1e12 multiplier,
> fall back to the 1e9 used before this patch and 9 decimal places.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
I want to leave this patch on the list a little longer on basis it may have
side effects in other drivers (hopefully not bad ones!)

However, the only patch that needs this in the series is the roadtest tests
in the final patch.  Those are obviously dependent on roadtest going upstream
so no rush their.  As such I'm going to see if I can pick up the reset of the
series (2-16 - minor mods as per Andy's comments).

Thanks,

Jonathan

> ---
>  drivers/iio/industrialio-core.c | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index dc3e1cb9bfbd..8225d0c43010 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -18,6 +18,7 @@
>  #include <linux/poll.h>
>  #include <linux/property.h>
>  #include <linux/sched.h>
> +#include <linux/units.h>
>  #include <linux/wait.h>
>  #include <linux/cdev.h>
>  #include <linux/slab.h>
> @@ -674,14 +675,28 @@ static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
>  		else
>  			return sysfs_emit_at(buf, offset, "%d.%09u", tmp0,
>  					     abs(tmp1));
> -	case IIO_VAL_FRACTIONAL_LOG2:
> -		tmp2 = shift_right((s64)vals[0] * 1000000000LL, vals[1]);
> -		tmp0 = (int)div_s64_rem(tmp2, 1000000000LL, &tmp1);
> -		if (tmp0 == 0 && tmp2 < 0)
> -			return sysfs_emit_at(buf, offset, "-0.%09u", abs(tmp1));
> -		else
> -			return sysfs_emit_at(buf, offset, "%d.%09u", tmp0,
> -					     abs(tmp1));
> +	case IIO_VAL_FRACTIONAL_LOG2: {
> +		u64 t1, t2, mult;
> +		int integer, precision;
> +		bool neg = vals[0] < 0;
> +
> +		if (vals[0] > ULLONG_MAX / PICO) {
> +			mult = NANO;
> +			precision = 9;
> +		} else {
> +			mult = PICO;
> +			precision = 12;
> +		}
> +		t1 = shift_right((u64)abs(vals[0]) * mult, vals[1]);
> +		integer = (int)div64_u64_rem(t1, mult, &t2);
> +		if (integer == 0 && neg)
> +			return sysfs_emit_at(buf, offset, "-0.%0*llu",
> +					     precision, abs(t2));
> +		if (neg)
> +			integer *= -1;
> +		return sysfs_emit_at(buf, offset, "%d.%0*llu", integer,
> +				     precision, abs(t2));
> +	}
>  	case IIO_VAL_INT_MULTIPLE:
>  	{
>  		int i;

[PATCH v3 02/17] iio: ABI: Fix wrong format of differential capacitance channel ABI.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

in_ only occurs once in these attributes.

Fixes: 0baf29d658c7 ("staging:iio:documentation Add abi docs for capacitance adcs.")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 3e00d7f7ee22..d3a0c0ef8948 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -193,7 +193,7 @@ Description:
 		Raw capacitance measurement from channel Y. Units after
 		application of scale and offset are nanofarads.
 
-What:		/sys/.../iio:deviceX/in_capacitanceY-in_capacitanceZ_raw
+What:		/sys/.../iio:deviceX/in_capacitanceY-capacitanceZ_raw
 KernelVersion:	3.2
 Contact:	linux-iio@vger.kernel.org
 Description:
-- 
2.36.1

[PATCH v3 03/17] staging: iio: cdc: ad7746: Use explicit be24 handling.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Chance from fiddly local implementation of be24 to cpu endian conversion
by reading into a 3 byte buffer and using get_unaligned_be24()

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 52b8957c19c9..08f73be5797a 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -15,6 +15,8 @@
 #include <linux/stat.h>
 #include <linux/sysfs.h>
 
+#include <asm/unaligned.h>
+
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
@@ -95,10 +97,7 @@ struct ad7746_chip_info {
 	u8	capdac[2][2];
 	s8	capdac_set;
 
-	union {
-		__be32 d32;
-		u8 d8[4];
-	} data ____cacheline_aligned;
+	u8 data[3] ____cacheline_aligned;
 };
 
 enum ad7746_chan {
@@ -546,13 +545,14 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 		/* Now read the actual register */
 
 		ret = i2c_smbus_read_i2c_block_data(chip->client,
-						    chan->address >> 8, 3,
-						    &chip->data.d8[1]);
+						    chan->address >> 8,
+						    sizeof(chip->data),
+						    chip->data);
 
 		if (ret < 0)
 			goto out;
 
-		*val = (be32_to_cpu(chip->data.d32) & 0xFFFFFF) - 0x800000;
+		*val = get_unaligned_be24(chip->data) - 0x800000;
 
 		switch (chan->type) {
 		case IIO_TEMP:
-- 
2.36.1

[PATCH v3 04/17] staging: iio: cdc: ad7746: Push handling of supply voltage scale to userspace.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

The supply voltage is attenuated by 6 before being fed to the ADC.
Handle this explicitly rather than pre-multiplying the _raw value by 6.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 08f73be5797a..0b5cb788abee 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -116,9 +116,8 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		.type = IIO_VOLTAGE,
 		.indexed = 1,
 		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
-			BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = AD7746_REG_VT_DATA_HIGH << 8 |
 			AD7746_VTSETUP_VTMD_EXT_VIN,
 	},
@@ -127,9 +126,8 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		.indexed = 1,
 		.channel = 1,
 		.extend_name = "supply",
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |
-			BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = AD7746_REG_VT_DATA_HIGH << 8 |
 			AD7746_VTSETUP_VTMD_VDD_MON,
 	},
@@ -562,10 +560,6 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 			 */
 			*val = (*val * 125) / 256;
 			break;
-		case IIO_VOLTAGE:
-			if (chan->channel == 1) /* supply_raw*/
-				*val = *val * 6;
-			break;
 		default:
 			break;
 		}
@@ -620,6 +614,8 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 		case IIO_VOLTAGE:
 			/* 1170mV / 2^23 */
 			*val = 1170;
+			if (chan->channel == 1)
+				*val *= 6;
 			*val2 = 23;
 			ret = IIO_VAL_FRACTIONAL_LOG2;
 			break;
-- 
2.36.1

[PATCH v3 05/17] staging: iio: cdc: ad7746: Use local buffer for multi byte reads.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

I2C does not require DMA safe buffers so there is no need to ensure
the buffers are in their own cacheline. Hence simplify things by
using a local variable instead of embedding the buffer in the chip
info structure.

Includes a trivial whitespace cleanup to drop a line between function
and error handling.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 0b5cb788abee..496e90f559c7 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -96,8 +96,6 @@ struct ad7746_chip_info {
 	u8	vt_setup;
 	u8	capdac[2][2];
 	s8	capdac_set;
-
-	u8 data[3] ____cacheline_aligned;
 };
 
 enum ad7746_chan {
@@ -522,6 +520,7 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 	struct ad7746_chip_info *chip = iio_priv(indio_dev);
 	int ret, delay, idx;
 	u8 regval, reg;
+	u8 data[3];
 
 	mutex_lock(&chip->lock);
 
@@ -544,13 +543,11 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 
 		ret = i2c_smbus_read_i2c_block_data(chip->client,
 						    chan->address >> 8,
-						    sizeof(chip->data),
-						    chip->data);
-
+						    sizeof(data), data);
 		if (ret < 0)
 			goto out;
 
-		*val = get_unaligned_be24(chip->data) - 0x800000;
+		*val = get_unaligned_be24(data) - 0x800000;
 
 		switch (chan->type) {
 		case IIO_TEMP:
-- 
2.36.1

[PATCH v3 06/17] staging: iio: cdc: ad7746: Factor out ad7746_read_channel()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reduce deep indenting and simplify the locking cleanup that follows.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 80 ++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 496e90f559c7..8052ac2696d3 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -512,54 +512,66 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 	return ret;
 }
 
+static int ad7746_read_channel(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int *val)
+{
+	struct ad7746_chip_info *chip = iio_priv(indio_dev);
+	int ret, delay;
+	u8 data[3];
+	u8 regval;
+
+	ret = ad7746_select_channel(indio_dev, chan);
+	if (ret < 0)
+		return ret;
+	delay = ret;
+
+	regval = chip->config | AD7746_CONF_MODE_SINGLE_CONV;
+	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
+	if (ret < 0)
+		return ret;
+
+	msleep(delay);
+	/* Now read the actual register */
+	ret = i2c_smbus_read_i2c_block_data(chip->client,  chan->address >> 8,
+					    sizeof(data), data);
+	if (ret < 0)
+		return ret;
+
+	*val = get_unaligned_be24(data) - 0x800000;
+
+	switch (chan->type) {
+	case IIO_TEMP:
+		/*
+		 * temperature in milli degrees Celsius
+		 * T = ((*val / 2048) - 4096) * 1000
+		 */
+		*val = (*val  * 125) / 256;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 static int ad7746_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2,
 			   long mask)
 {
 	struct ad7746_chip_info *chip = iio_priv(indio_dev);
-	int ret, delay, idx;
-	u8 regval, reg;
-	u8 data[3];
+	int ret, idx;
+	u8 reg;
 
 	mutex_lock(&chip->lock);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 	case IIO_CHAN_INFO_PROCESSED:
-		ret = ad7746_select_channel(indio_dev, chan);
+		ret = ad7746_read_channel(indio_dev, chan, val);
 		if (ret < 0)
 			goto out;
-		delay = ret;
-
-		regval = chip->config | AD7746_CONF_MODE_SINGLE_CONV;
-		ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG,
-						regval);
-		if (ret < 0)
-			goto out;
-
-		msleep(delay);
-		/* Now read the actual register */
-
-		ret = i2c_smbus_read_i2c_block_data(chip->client,
-						    chan->address >> 8,
-						    sizeof(data), data);
-		if (ret < 0)
-			goto out;
-
-		*val = get_unaligned_be24(data) - 0x800000;
-
-		switch (chan->type) {
-		case IIO_TEMP:
-			/*
-			 * temperature in milli degrees Celsius
-			 * T = ((*val / 2048) - 4096) * 1000
-			 */
-			*val = (*val * 125) / 256;
-			break;
-		default:
-			break;
-		}
 
 		ret = IIO_VAL_INT;
 		break;
-- 
2.36.1

[PATCH v3 07/17] staging: iio: cdc: ad7764: Push locking down into case statements in read/write_raw

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Not all paths require any locking at all. So to simplify the
removal of such locking push the locks down into the individual
case statements.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 127 +++++++++++++------------------
 1 file changed, 54 insertions(+), 73 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 8052ac2696d3..bc03760e44e0 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -420,14 +420,10 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 	struct ad7746_chip_info *chip = iio_priv(indio_dev);
 	int ret, reg;
 
-	mutex_lock(&chip->lock);
-
 	switch (mask) {
 	case IIO_CHAN_INFO_CALIBSCALE:
-		if (val != 1) {
-			ret = -EINVAL;
-			goto out;
-		}
+		if (val != 1)
+			return -EINVAL;
 
 		val = (val2 * 1024) / 15625;
 
@@ -439,33 +435,31 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 			reg = AD7746_REG_VOLT_GAINH;
 			break;
 		default:
-			ret = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
 
+		mutex_lock(&chip->lock);
 		ret = i2c_smbus_write_word_swapped(chip->client, reg, val);
+		mutex_unlock(&chip->lock);
 		if (ret < 0)
-			goto out;
+			return ret;
 
-		ret = 0;
-		break;
+		return 0;
 	case IIO_CHAN_INFO_CALIBBIAS:
-		if (val < 0 || val > 0xFFFF) {
-			ret = -EINVAL;
-			goto out;
-		}
+		if (val < 0 || val > 0xFFFF)
+			return -EINVAL;
+
+		mutex_lock(&chip->lock);
 		ret = i2c_smbus_write_word_swapped(chip->client,
 						   AD7746_REG_CAP_OFFH, val);
+		mutex_unlock(&chip->lock);
 		if (ret < 0)
-			goto out;
+			return ret;
 
-		ret = 0;
-		break;
+		return 0;
 	case IIO_CHAN_INFO_OFFSET:
-		if (val < 0 || val > 43008000) { /* 21pF */
-			ret = -EINVAL;
-			goto out;
-		}
+		if (val < 0 || val > 43008000) /* 21pF */
+			return -EINVAL;
 
 		/*
 		 * CAPDAC Scale = 21pF_typ / 127
@@ -474,42 +468,41 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 		 */
 
 		val /= 338646;
-
+		mutex_lock(&chip->lock);
 		chip->capdac[chan->channel][chan->differential] = val > 0 ?
 			AD7746_CAPDAC_DACP(val) | AD7746_CAPDAC_DACEN : 0;
 
 		ret = ad7746_set_capdac(chip, chan->channel);
-		if (ret < 0)
-			goto out;
+		if (ret < 0) {
+			mutex_unlock(&chip->lock);
+			return ret;
+		}
 
 		chip->capdac_set = chan->channel;
+		mutex_unlock(&chip->lock);
 
-		ret = 0;
-		break;
+		return 0;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		if (val2) {
-			ret = -EINVAL;
-			goto out;
-		}
+		if (val2)
+			return -EINVAL;
 
 		switch (chan->type) {
 		case IIO_CAPACITANCE:
+			mutex_lock(&chip->lock);
 			ret = ad7746_store_cap_filter_rate_setup(chip, val);
-			break;
+			mutex_unlock(&chip->lock);
+			return ret;
 		case IIO_VOLTAGE:
+			mutex_lock(&chip->lock);
 			ret = ad7746_store_vt_filter_rate_setup(chip, val);
-			break;
+			mutex_unlock(&chip->lock);
+			return ret;
 		default:
-			ret = -EINVAL;
+			return -EINVAL;
 		}
-		break;
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-
-out:
-	mutex_unlock(&chip->lock);
-	return ret;
 }
 
 static int ad7746_read_channel(struct iio_dev *indio_dev,
@@ -564,17 +557,16 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 	int ret, idx;
 	u8 reg;
 
-	mutex_lock(&chip->lock);
-
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 	case IIO_CHAN_INFO_PROCESSED:
+		mutex_lock(&chip->lock);
 		ret = ad7746_read_channel(indio_dev, chan, val);
+		mutex_unlock(&chip->lock);
 		if (ret < 0)
-			goto out;
+			return ret;
 
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_CALIBSCALE:
 		switch (chan->type) {
 		case IIO_CAPACITANCE:
@@ -584,80 +576,69 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 			reg = AD7746_REG_VOLT_GAINH;
 			break;
 		default:
-			ret = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
 
+		mutex_lock(&chip->lock);
 		ret = i2c_smbus_read_word_swapped(chip->client, reg);
+		mutex_unlock(&chip->lock);
 		if (ret < 0)
-			goto out;
+			return ret;
 		/* 1 + gain_val / 2^16 */
 		*val = 1;
 		*val2 = (15625 * ret) / 1024;
 
-		ret = IIO_VAL_INT_PLUS_MICRO;
-		break;
+		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_CALIBBIAS:
+		mutex_lock(&chip->lock);
 		ret = i2c_smbus_read_word_swapped(chip->client,
 						  AD7746_REG_CAP_OFFH);
+		mutex_unlock(&chip->lock);
 		if (ret < 0)
-			goto out;
+			return ret;
 		*val = ret;
 
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_OFFSET:
 		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
 					  [chan->differential]) * 338646;
 
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_CAPACITANCE:
 			/* 8.192pf / 2^24 */
 			*val =  0;
 			*val2 = 488;
-			ret = IIO_VAL_INT_PLUS_NANO;
-			break;
+			return IIO_VAL_INT_PLUS_NANO;
 		case IIO_VOLTAGE:
 			/* 1170mV / 2^23 */
 			*val = 1170;
 			if (chan->channel == 1)
 				*val *= 6;
 			*val2 = 23;
-			ret = IIO_VAL_FRACTIONAL_LOG2;
-			break;
+			return IIO_VAL_FRACTIONAL_LOG2;
 		default:
-			ret = -EINVAL;
-			break;
+			return -EINVAL;
 		}
-
-		break;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		switch (chan->type) {
 		case IIO_CAPACITANCE:
 			idx = (chip->config & AD7746_CONF_CAPFS_MASK) >>
 				AD7746_CONF_CAPFS_SHIFT;
 			*val = ad7746_cap_filter_rate_table[idx][0];
-			ret = IIO_VAL_INT;
-			break;
+			return IIO_VAL_INT;
 		case IIO_VOLTAGE:
 			idx = (chip->config & AD7746_CONF_VTFS_MASK) >>
 				AD7746_CONF_VTFS_SHIFT;
 			*val = ad7746_vt_filter_rate_table[idx][0];
-			ret = IIO_VAL_INT;
-			break;
+			return IIO_VAL_INT;
 		default:
-			ret = -EINVAL;
+			return -EINVAL;
 		}
-		break;
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-out:
-	mutex_unlock(&chip->lock);
-	return ret;
 }
 
 static const struct iio_info ad7746_info = {
-- 
2.36.1

[PATCH v3 08/17] staging: iio: cdc: ad7746: Break up use of chan->address and use FIELD_PREP etc

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Instead of encoding several different fields into chan->address use
an indirection to a separate per channel structure where the various
fields can be expressed in a more readable form.  This also allows
the register values to be constructed at runtime using FIELD_PREP()

Drop the now redundant _SHIFT macros.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 152 ++++++++++++++++++++-----------
 1 file changed, 99 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index bc03760e44e0..fa16522221aa 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -5,6 +5,7 @@
  * Copyright 2011 Analog Devices Inc.
  */
 
+#include <linux/bitfield.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
@@ -50,11 +51,12 @@
 #define AD7746_CAPSETUP_CACHOP		BIT(0)
 
 /* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) */
-#define AD7746_VTSETUP_VTEN		(1 << 7)
-#define AD7746_VTSETUP_VTMD_INT_TEMP	(0 << 5)
-#define AD7746_VTSETUP_VTMD_EXT_TEMP	(1 << 5)
-#define AD7746_VTSETUP_VTMD_VDD_MON	(2 << 5)
-#define AD7746_VTSETUP_VTMD_EXT_VIN	(3 << 5)
+#define AD7746_VTSETUP_VTEN		BIT(7)
+#define AD7746_VTSETUP_VTMD_MASK	GENMASK(6, 5)
+#define AD7746_VTSETUP_VTMD_INT_TEMP	0
+#define AD7746_VTSETUP_VTMD_EXT_TEMP	1
+#define AD7746_VTSETUP_VTMD_VDD_MON	2
+#define AD7746_VTSETUP_VTMD_EXT_VIN	3
 #define AD7746_VTSETUP_EXTREF		BIT(4)
 #define AD7746_VTSETUP_VTSHORT		BIT(1)
 #define AD7746_VTSETUP_VTCHOP		BIT(0)
@@ -66,23 +68,22 @@
 #define AD7746_EXCSETUP_NEXCB		BIT(4)
 #define AD7746_EXCSETUP_EXCA		BIT(3)
 #define AD7746_EXCSETUP_NEXCA		BIT(2)
-#define AD7746_EXCSETUP_EXCLVL(x)	(((x) & 0x3) << 0)
+#define AD7746_EXCSETUP_EXCLVL_MASK	GENMASK(1, 0)
 
 /* Config Register Bit Designations (AD7746_REG_CFG) */
-#define AD7746_CONF_VTFS_SHIFT		6
-#define AD7746_CONF_CAPFS_SHIFT		3
 #define AD7746_CONF_VTFS_MASK		GENMASK(7, 6)
 #define AD7746_CONF_CAPFS_MASK		GENMASK(5, 3)
-#define AD7746_CONF_MODE_IDLE		(0 << 0)
-#define AD7746_CONF_MODE_CONT_CONV	(1 << 0)
-#define AD7746_CONF_MODE_SINGLE_CONV	(2 << 0)
-#define AD7746_CONF_MODE_PWRDN		(3 << 0)
-#define AD7746_CONF_MODE_OFFS_CAL	(5 << 0)
-#define AD7746_CONF_MODE_GAIN_CAL	(6 << 0)
+#define AD7746_CONF_MODE_MASK		GENMASK(2, 0)
+#define AD7746_CONF_MODE_IDLE		0
+#define AD7746_CONF_MODE_CONT_CONV	1
+#define AD7746_CONF_MODE_SINGLE_CONV	2
+#define AD7746_CONF_MODE_PWRDN		3
+#define AD7746_CONF_MODE_OFFS_CAL	5
+#define AD7746_CONF_MODE_GAIN_CAL	6
 
 /* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
 #define AD7746_CAPDAC_DACEN		BIT(7)
-#define AD7746_CAPDAC_DACP(x)		((x) & 0x7F)
+#define AD7746_CAPDAC_DACP_MASK		0x7F
 
 struct ad7746_chip_info {
 	struct i2c_client *client;
@@ -109,6 +110,52 @@ enum ad7746_chan {
 	CIN2_DIFF,
 };
 
+struct ad7746_chan_info {
+	u8 addr;
+	union {
+		u8 vtmd;
+		struct { /* CAP SETUP fields */
+			unsigned int cin2 : 1;
+			unsigned int capdiff : 1;
+		};
+	};
+};
+
+static const struct ad7746_chan_info ad7746_chan_info[] = {
+	[VIN] = {
+		.addr = AD7746_REG_VT_DATA_HIGH,
+		.vtmd = AD7746_VTSETUP_VTMD_EXT_VIN,
+	},
+	[VIN_VDD] = {
+		.addr = AD7746_REG_VT_DATA_HIGH,
+		.vtmd = AD7746_VTSETUP_VTMD_VDD_MON,
+	},
+	[TEMP_INT] = {
+		.addr = AD7746_REG_VT_DATA_HIGH,
+		.vtmd = AD7746_VTSETUP_VTMD_INT_TEMP,
+	},
+	[TEMP_EXT] = {
+		.addr = AD7746_REG_VT_DATA_HIGH,
+		.vtmd = AD7746_VTSETUP_VTMD_EXT_TEMP,
+	},
+	[CIN1] = {
+		.addr = AD7746_REG_CAP_DATA_HIGH,
+	},
+	[CIN1_DIFF] = {
+		.addr =  AD7746_REG_CAP_DATA_HIGH,
+		.capdiff = 1,
+	},
+	[CIN2] = {
+		.addr = AD7746_REG_CAP_DATA_HIGH,
+		.cin2 = 1,
+	},
+	[CIN2_DIFF] = {
+		.addr = AD7746_REG_CAP_DATA_HIGH,
+		.cin2 = 1,
+		.capdiff = 1,
+	},
+};
+
 static const struct iio_chan_spec ad7746_channels[] = {
 	[VIN] = {
 		.type = IIO_VOLTAGE,
@@ -116,8 +163,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		.channel = 0,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = AD7746_REG_VT_DATA_HIGH << 8 |
-			AD7746_VTSETUP_VTMD_EXT_VIN,
+		.address = VIN,
 	},
 	[VIN_VDD] = {
 		.type = IIO_VOLTAGE,
@@ -126,24 +172,21 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		.extend_name = "supply",
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = AD7746_REG_VT_DATA_HIGH << 8 |
-			AD7746_VTSETUP_VTMD_VDD_MON,
+		.address = VIN_VDD,
 	},
 	[TEMP_INT] = {
 		.type = IIO_TEMP,
 		.indexed = 1,
 		.channel = 0,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
-		.address = AD7746_REG_VT_DATA_HIGH << 8 |
-			AD7746_VTSETUP_VTMD_INT_TEMP,
+		.address = TEMP_INT,
 	},
 	[TEMP_EXT] = {
 		.type = IIO_TEMP,
 		.indexed = 1,
 		.channel = 1,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
-		.address = AD7746_REG_VT_DATA_HIGH << 8 |
-			AD7746_VTSETUP_VTMD_EXT_TEMP,
+		.address = TEMP_EXT,
 	},
 	[CIN1] = {
 		.type = IIO_CAPACITANCE,
@@ -153,7 +196,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = AD7746_REG_CAP_DATA_HIGH << 8,
+		.address = CIN1,
 	},
 	[CIN1_DIFF] = {
 		.type = IIO_CAPACITANCE,
@@ -165,8 +208,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = AD7746_REG_CAP_DATA_HIGH << 8 |
-			AD7746_CAPSETUP_CAPDIFF
+		.address = CIN1_DIFF,
 	},
 	[CIN2] = {
 		.type = IIO_CAPACITANCE,
@@ -176,8 +218,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = AD7746_REG_CAP_DATA_HIGH << 8 |
-			AD7746_CAPSETUP_CIN2,
+		.address = CIN2,
 	},
 	[CIN2_DIFF] = {
 		.type = IIO_CAPACITANCE,
@@ -189,8 +230,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = AD7746_REG_CAP_DATA_HIGH << 8 |
-			AD7746_CAPSETUP_CAPDIFF | AD7746_CAPSETUP_CIN2,
+		.address = CIN2_DIFF,
 	}
 };
 
@@ -226,10 +266,13 @@ static int ad7746_select_channel(struct iio_dev *indio_dev,
 
 	switch (chan->type) {
 	case IIO_CAPACITANCE:
-		cap_setup = (chan->address & 0xFF) | AD7746_CAPSETUP_CAPEN;
+		cap_setup = FIELD_PREP(AD7746_CAPSETUP_CIN2,
+				       ad7746_chan_info[chan->address].cin2) |
+			FIELD_PREP(AD7746_CAPSETUP_CAPDIFF,
+				   ad7746_chan_info[chan->address].capdiff) |
+			FIELD_PREP(AD7746_CAPSETUP_CAPEN, 1);
 		vt_setup = chip->vt_setup & ~AD7746_VTSETUP_VTEN;
-		idx = (chip->config & AD7746_CONF_CAPFS_MASK) >>
-			AD7746_CONF_CAPFS_SHIFT;
+		idx = FIELD_GET(AD7746_CONF_CAPFS_MASK, chip->config);
 		delay = ad7746_cap_filter_rate_table[idx][1];
 
 		ret = ad7746_set_capdac(chip, chan->channel);
@@ -241,10 +284,11 @@ static int ad7746_select_channel(struct iio_dev *indio_dev,
 		break;
 	case IIO_VOLTAGE:
 	case IIO_TEMP:
-		vt_setup = (chan->address & 0xFF) | AD7746_VTSETUP_VTEN;
+		vt_setup = FIELD_PREP(AD7746_VTSETUP_VTMD_MASK,
+				      ad7746_chan_info[chan->address].vtmd) |
+			FIELD_PREP(AD7746_VTSETUP_VTEN, 1);
 		cap_setup = chip->cap_setup & ~AD7746_CAPSETUP_CAPEN;
-		idx = (chip->config & AD7746_CONF_VTFS_MASK) >>
-			AD7746_CONF_VTFS_SHIFT;
+		idx = FIELD_GET(AD7746_CONF_VTFS_MASK, chip->config);
 		delay = ad7746_cap_filter_rate_table[idx][1];
 		break;
 	default:
@@ -327,7 +371,8 @@ static ssize_t ad7746_start_offset_calib(struct device *dev,
 		return ret;
 
 	return ad7746_start_calib(dev, attr, buf, len,
-				  AD7746_CONF_MODE_OFFS_CAL);
+				  FIELD_PREP(AD7746_CONF_MODE_MASK,
+					     AD7746_CONF_MODE_OFFS_CAL));
 }
 
 static ssize_t ad7746_start_gain_calib(struct device *dev,
@@ -342,7 +387,8 @@ static ssize_t ad7746_start_gain_calib(struct device *dev,
 		return ret;
 
 	return ad7746_start_calib(dev, attr, buf, len,
-				  AD7746_CONF_MODE_GAIN_CAL);
+				  FIELD_PREP(AD7746_CONF_MODE_MASK,
+					     AD7746_CONF_MODE_GAIN_CAL));
 }
 
 static IIO_DEVICE_ATTR(in_capacitance0_calibbias_calibration,
@@ -369,7 +415,7 @@ static int ad7746_store_cap_filter_rate_setup(struct ad7746_chip_info *chip,
 		i = ARRAY_SIZE(ad7746_cap_filter_rate_table) - 1;
 
 	chip->config &= ~AD7746_CONF_CAPFS_MASK;
-	chip->config |= i << AD7746_CONF_CAPFS_SHIFT;
+	chip->config |= FIELD_PREP(AD7746_CONF_CAPFS_MASK, i);
 
 	return 0;
 }
@@ -387,7 +433,7 @@ static int ad7746_store_vt_filter_rate_setup(struct ad7746_chip_info *chip,
 		i = ARRAY_SIZE(ad7746_vt_filter_rate_table) - 1;
 
 	chip->config &= ~AD7746_CONF_VTFS_MASK;
-	chip->config |= i << AD7746_CONF_VTFS_SHIFT;
+	chip->config |= FIELD_PREP(AD7746_CONF_VTFS_MASK, i);
 
 	return 0;
 }
@@ -470,7 +516,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 		val /= 338646;
 		mutex_lock(&chip->lock);
 		chip->capdac[chan->channel][chan->differential] = val > 0 ?
-			AD7746_CAPDAC_DACP(val) | AD7746_CAPDAC_DACEN : 0;
+			FIELD_PREP(AD7746_CAPDAC_DACP_MASK, val) | AD7746_CAPDAC_DACEN : 0;
 
 		ret = ad7746_set_capdac(chip, chan->channel);
 		if (ret < 0) {
@@ -519,14 +565,16 @@ static int ad7746_read_channel(struct iio_dev *indio_dev,
 		return ret;
 	delay = ret;
 
-	regval = chip->config | AD7746_CONF_MODE_SINGLE_CONV;
+	regval = chip->config | FIELD_PREP(AD7746_CONF_MODE_MASK,
+					   AD7746_CONF_MODE_SINGLE_CONV);
 	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
 	if (ret < 0)
 		return ret;
 
 	msleep(delay);
 	/* Now read the actual register */
-	ret = i2c_smbus_read_i2c_block_data(chip->client,  chan->address >> 8,
+	ret = i2c_smbus_read_i2c_block_data(chip->client,
+					    ad7746_chan_info[chan->address].addr,
 					    sizeof(data), data);
 	if (ret < 0)
 		return ret;
@@ -600,8 +648,8 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_OFFSET:
-		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
-					  [chan->differential]) * 338646;
+		*val = FIELD_GET(AD7746_CAPDAC_DACP_MASK,
+				 chip->capdac[chan->channel][chan->differential]) * 338646;
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
@@ -624,13 +672,11 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		switch (chan->type) {
 		case IIO_CAPACITANCE:
-			idx = (chip->config & AD7746_CONF_CAPFS_MASK) >>
-				AD7746_CONF_CAPFS_SHIFT;
+			idx = FIELD_GET(AD7746_CONF_CAPFS_MASK, chip->config);
 			*val = ad7746_cap_filter_rate_table[idx][0];
 			return IIO_VAL_INT;
 		case IIO_VOLTAGE:
-			idx = (chip->config & AD7746_CONF_VTFS_MASK) >>
-				AD7746_CONF_VTFS_SHIFT;
+			idx = FIELD_GET(AD7746_CONF_VTFS_MASK, chip->config);
 			*val = ad7746_vt_filter_rate_table[idx][0];
 			return IIO_VAL_INT;
 		default:
@@ -696,16 +742,16 @@ static int ad7746_probe(struct i2c_client *client,
 	if (!ret) {
 		switch (vdd_permille) {
 		case 125:
-			regval |= AD7746_EXCSETUP_EXCLVL(0);
+			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 0);
 			break;
 		case 250:
-			regval |= AD7746_EXCSETUP_EXCLVL(1);
+			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 1);
 			break;
 		case 375:
-			regval |= AD7746_EXCSETUP_EXCLVL(2);
+			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 2);
 			break;
 		case 500:
-			regval |= AD7746_EXCSETUP_EXCLVL(3);
+			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 3);
 			break;
 		default:
 			break;
-- 
2.36.1

Re: [PATCH v3 08/17] staging: iio: cdc: ad7746: Break up use of chan->address and use FIELD_PREP etc

[Andy Shevchenko]

On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Instead of encoding several different fields into chan->address use
> an indirection to a separate per channel structure where the various
> fields can be expressed in a more readable form.  This also allows
> the register values to be constructed at runtime using FIELD_PREP()

FIELD_PREP().

(Note period)

> Drop the now redundant _SHIFT macros.

...

>  /* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
>  #define AD7746_CAPDAC_DACEN            BIT(7)
> -#define AD7746_CAPDAC_DACP(x)          ((x) & 0x7F)
> +#define AD7746_CAPDAC_DACP_MASK                0x7F

GENMASK() ?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 08/17] staging: iio: cdc: ad7746: Break up use of chan->address and use FIELD_PREP etc

[Jonathan Cameron]

On Tue, 28 Jun 2022 14:15:57 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Instead of encoding several different fields into chan->address use
> > an indirection to a separate per channel structure where the various
> > fields can be expressed in a more readable form.  This also allows
> > the register values to be constructed at runtime using FIELD_PREP()  
> 
> FIELD_PREP().
> 
> (Note period)
> 
> > Drop the now redundant _SHIFT macros.  
> 
> ...
> 
> >  /* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
> >  #define AD7746_CAPDAC_DACEN            BIT(7)
> > -#define AD7746_CAPDAC_DACP(x)          ((x) & 0x7F)
> > +#define AD7746_CAPDAC_DACP_MASK                0x7F  
> 
> GENMASK() ?
> 

Both fixed whilst applying the patch.  Thanks!

[PATCH v3 09/17] staging: iio: cdc: ad7746: Drop unused i2c_set_clientdata()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

As the comment states, this was only used in remove() and now
there is no explicit remove() function to make use of it.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index fa16522221aa..c574bc1c7d3d 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -708,8 +708,6 @@ static int ad7746_probe(struct i2c_client *client,
 		return -ENOMEM;
 	chip = iio_priv(indio_dev);
 	mutex_init(&chip->lock);
-	/* this is only used for device removal purposes */
-	i2c_set_clientdata(client, indio_dev);
 
 	chip->client = client;
 	chip->capdac_set = -1;
-- 
2.36.1

[PATCH v3 10/17] staging: iio: cdc: ad7746: Use _raw and _scale for temperature channels.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Performing the maths to rescale a 24 bit raw reading within the driver
was resulting in precision losses.  So make that userspace's problem
by exporting the scale and letting the maths be done in userspace with
appropriate precision.  Issue identified using roadtester testing
framework.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index c574bc1c7d3d..2b27a2cb807c 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -178,14 +178,16 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		.type = IIO_TEMP,
 		.indexed = 1,
 		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
 		.address = TEMP_INT,
 	},
 	[TEMP_EXT] = {
 		.type = IIO_TEMP,
 		.indexed = 1,
 		.channel = 1,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
 		.address = TEMP_EXT,
 	},
 	[CIN1] = {
@@ -581,18 +583,6 @@ static int ad7746_read_channel(struct iio_dev *indio_dev,
 
 	*val = get_unaligned_be24(data) - 0x800000;
 
-	switch (chan->type) {
-	case IIO_TEMP:
-		/*
-		 * temperature in milli degrees Celsius
-		 * T = ((*val / 2048) - 4096) * 1000
-		 */
-		*val = (*val  * 125) / 256;
-		break;
-	default:
-		break;
-	}
-
 	return 0;
 }
 
@@ -607,7 +597,6 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-	case IIO_CHAN_INFO_PROCESSED:
 		mutex_lock(&chip->lock);
 		ret = ad7746_read_channel(indio_dev, chan, val);
 		mutex_unlock(&chip->lock);
@@ -666,6 +655,10 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 				*val *= 6;
 			*val2 = 23;
 			return IIO_VAL_FRACTIONAL_LOG2;
+		case IIO_TEMP:
+			*val = 125;
+			*val2 = 8;
+			return IIO_VAL_FRACTIONAL_LOG2;
 		default:
 			return -EINVAL;
 		}
-- 
2.36.1

[PATCH v3 11/17] iio: core: Introduce _zeropoint for differential channels

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Address an ABI gap for device where the offset of both lines in a
differential pair may be controlled so as to allow a wider range of
inputs, but without having any direct effect of the differential
measurement.

_offset cannot be used as to remain in line with existing usage,
userspace would be expected to apply it as (_raw + _offset) * _scale
whereas _zeropoint is not. i.e. If we were computing the differential
in software it would be.
((postive_raw + _zeropoint) - (negative_raw + zeropoint) + _offset) * _scale
= ((postive_raw - negative_raw) + _offset) * _scale
= (differential_raw + _offset) * _scale

Similarly calibbias is expected to tweak the measurement seen, not
the adjust the two lines of the differential pair.

Needed for in_capacitanceX-capacitanceY_zeropoint for the
AD7746 CDC driver.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 19 +++++++++++++++++++
 drivers/iio/industrialio-core.c         |  1 +
 include/linux/iio/types.h               |  1 +
 3 files changed, 21 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index d3a0c0ef8948..1dfd6c3f2bd0 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -204,6 +204,25 @@ Description:
 		is required is a consistent labeling.  Units after application
 		of scale and offset are nanofarads.
 
+What:		/sys/.../iio:deviceX/in_capacitanceY-capacitanceZ_zeropoint
+KernelVersion:	5.19
+Contact:	linux-iio@vger.kernel.org
+Description:
+		For differential channels, this an offset that is applied
+		equally to both inputs. As the reading is of the difference
+		between the two inputs, this should not be applied to the _raw
+		reading by userspace (unlike _offset) and unlike calibbias
+		it does not affect the differential value measured because
+		the effect of _zeropoint cancels out across the two inputs
+		that make up the differential pair. It's purpose is to bring
+		the individual signals, before the differential is measured,
+		within the measurement range of the device. The naming is
+		chosen because if the separate inputs that make the
+		differential pair are drawn on a graph in their
+		_raw  units, this is the value that the zero point on the
+		measurement axis represents. It is expressed with the
+		same scaling as _raw.
+
 What:		/sys/bus/iio/devices/iio:deviceX/in_temp_raw
 What:		/sys/bus/iio/devices/iio:deviceX/in_tempX_raw
 What:		/sys/bus/iio/devices/iio:deviceX/in_temp_x_raw
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 8225d0c43010..fbd7d6bd6f98 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -169,6 +169,7 @@ static const char * const iio_chan_info_postfix[] = {
 	[IIO_CHAN_INFO_OVERSAMPLING_RATIO] = "oversampling_ratio",
 	[IIO_CHAN_INFO_THERMOCOUPLE_TYPE] = "thermocouple_type",
 	[IIO_CHAN_INFO_CALIBAMBIENT] = "calibambient",
+	[IIO_CHAN_INFO_ZEROPOINT] = "zeropoint",
 };
 /**
  * iio_device_id() - query the unique ID for the device
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index a7aa91f3a8dc..27143b03909d 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -63,6 +63,7 @@ enum iio_chan_info_enum {
 	IIO_CHAN_INFO_OVERSAMPLING_RATIO,
 	IIO_CHAN_INFO_THERMOCOUPLE_TYPE,
 	IIO_CHAN_INFO_CALIBAMBIENT,
+	IIO_CHAN_INFO_ZEROPOINT,
 };
 
 #endif /* _IIO_TYPES_H_ */
-- 
2.36.1

Re: [PATCH v3 11/17] iio: core: Introduce _zeropoint for differential channels

[Andy Shevchenko]

On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Address an ABI gap for device where the offset of both lines in a
> differential pair may be controlled so as to allow a wider range of
> inputs, but without having any direct effect of the differential
> measurement.
>
> _offset cannot be used as to remain in line with existing usage,
> userspace would be expected to apply it as (_raw + _offset) * _scale
> whereas _zeropoint is not. i.e. If we were computing the differential
> in software it would be.
> ((postive_raw + _zeropoint) - (negative_raw + zeropoint) + _offset) * _scale
> = ((postive_raw - negative_raw) + _offset) * _scale
> = (differential_raw + _offset) * _scale
>
> Similarly calibbias is expected to tweak the measurement seen, not
> the adjust the two lines of the differential pair.
>
> Needed for in_capacitanceX-capacitanceY_zeropoint for the
> AD7746 CDC driver.

...

> +What:          /sys/.../iio:deviceX/in_capacitanceY-capacitanceZ_zeropoint
> +KernelVersion: 5.19

5.20?

> +Contact:       linux-iio@vger.kernel.org
> +Description:
> +               For differential channels, this an offset that is applied
> +               equally to both inputs. As the reading is of the difference
> +               between the two inputs, this should not be applied to the _raw
> +               reading by userspace (unlike _offset) and unlike calibbias
> +               it does not affect the differential value measured because
> +               the effect of _zeropoint cancels out across the two inputs
> +               that make up the differential pair. It's purpose is to bring

makes

> +               the individual signals, before the differential is measured,
> +               within the measurement range of the device. The naming is
> +               chosen because if the separate inputs that make the
> +               differential pair are drawn on a graph in their
> +               _raw  units, this is the value that the zero point on the
> +               measurement axis represents. It is expressed with the
> +               same scaling as _raw.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 11/17] iio: core: Introduce _zeropoint for differential channels

[Jonathan Cameron]

On Tue, 28 Jun 2022 14:17:33 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Address an ABI gap for device where the offset of both lines in a
> > differential pair may be controlled so as to allow a wider range of
> > inputs, but without having any direct effect of the differential
> > measurement.
> >
> > _offset cannot be used as to remain in line with existing usage,
> > userspace would be expected to apply it as (_raw + _offset) * _scale
> > whereas _zeropoint is not. i.e. If we were computing the differential
> > in software it would be.
> > ((postive_raw + _zeropoint) - (negative_raw + zeropoint) + _offset) * _scale
> > = ((postive_raw - negative_raw) + _offset) * _scale
> > = (differential_raw + _offset) * _scale
> >
> > Similarly calibbias is expected to tweak the measurement seen, not
> > the adjust the two lines of the differential pair.
> >
> > Needed for in_capacitanceX-capacitanceY_zeropoint for the
> > AD7746 CDC driver.  
> 
> ...
> 
> > +What:          /sys/.../iio:deviceX/in_capacitanceY-capacitanceZ_zeropoint
> > +KernelVersion: 5.19  
> 
> 5.20?

:) Probably 5.21 as I'm not going to rush this in now, but good point none the less.

> 
> > +Contact:       linux-iio@vger.kernel.org
> > +Description:
> > +               For differential channels, this an offset that is applied
> > +               equally to both inputs. As the reading is of the difference
> > +               between the two inputs, this should not be applied to the _raw
> > +               reading by userspace (unlike _offset) and unlike calibbias
> > +               it does not affect the differential value measured because
> > +               the effect of _zeropoint cancels out across the two inputs
> > +               that make up the differential pair. It's purpose is to bring  
> 
> makes

No.  make is correct.
That's indeed an odd corner of English and honestly I'm not sure I could successfully
argue why it should be make :)

> 
> > +               the individual signals, before the differential is measured,
> > +               within the measurement range of the device. The naming is
> > +               chosen because if the separate inputs that make the
> > +               differential pair are drawn on a graph in their
> > +               _raw  units, this is the value that the zero point on the
> > +               measurement axis represents. It is expressed with the
> > +               same scaling as _raw.  
> 

Re: [PATCH v3 11/17] iio: core: Introduce _zeropoint for differential channels

[Jonathan Cameron]

On Mon, 18 Jul 2022 19:02:35 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 28 Jun 2022 14:17:33 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> > On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@kernel.org> wrote:  
> > >
> > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > >
> > > Address an ABI gap for device where the offset of both lines in a
> > > differential pair may be controlled so as to allow a wider range of
> > > inputs, but without having any direct effect of the differential
> > > measurement.
> > >
> > > _offset cannot be used as to remain in line with existing usage,
> > > userspace would be expected to apply it as (_raw + _offset) * _scale
> > > whereas _zeropoint is not. i.e. If we were computing the differential
> > > in software it would be.
> > > ((postive_raw + _zeropoint) - (negative_raw + zeropoint) + _offset) * _scale
> > > = ((postive_raw - negative_raw) + _offset) * _scale
> > > = (differential_raw + _offset) * _scale
> > >
> > > Similarly calibbias is expected to tweak the measurement seen, not
> > > the adjust the two lines of the differential pair.
> > >
> > > Needed for in_capacitanceX-capacitanceY_zeropoint for the
> > > AD7746 CDC driver.    
> > 
> > ...
> >   
> > > +What:          /sys/.../iio:deviceX/in_capacitanceY-capacitanceZ_zeropoint
> > > +KernelVersion: 5.19    
> > 
> > 5.20?  
> 
> :) Probably 5.21 as I'm not going to rush this in now, but good point none the less.
6.0 I guess :)  Anyhow I've applied this with Andy's tag on the assumption that the below
English quirk is fine by Andy.

Jonathan

> 
> >   
> > > +Contact:       linux-iio@vger.kernel.org
> > > +Description:
> > > +               For differential channels, this an offset that is applied
> > > +               equally to both inputs. As the reading is of the difference
> > > +               between the two inputs, this should not be applied to the _raw
> > > +               reading by userspace (unlike _offset) and unlike calibbias
> > > +               it does not affect the differential value measured because
> > > +               the effect of _zeropoint cancels out across the two inputs
> > > +               that make up the differential pair. It's purpose is to bring    
> > 
> > makes  
> 
> No.  make is correct.
> That's indeed an odd corner of English and honestly I'm not sure I could successfully
> argue why it should be make :)
> 
> >   
> > > +               the individual signals, before the differential is measured,
> > > +               within the measurement range of the device. The naming is
> > > +               chosen because if the separate inputs that make the
> > > +               differential pair are drawn on a graph in their
> > > +               _raw  units, this is the value that the zero point on the
> > > +               measurement axis represents. It is expressed with the
> > > +               same scaling as _raw.    
> >   
> 

Re: [PATCH v3 11/17] iio: core: Introduce _zeropoint for differential channels

[Jonathan Cameron]

On Sun, 7 Aug 2022 14:46:51 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 18 Jul 2022 19:02:35 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Tue, 28 Jun 2022 14:17:33 +0200
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >   
> > > On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@kernel.org> wrote:    
> > > >
> > > > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > >
> > > > Address an ABI gap for device where the offset of both lines in a
> > > > differential pair may be controlled so as to allow a wider range of
> > > > inputs, but without having any direct effect of the differential
> > > > measurement.
> > > >
> > > > _offset cannot be used as to remain in line with existing usage,
> > > > userspace would be expected to apply it as (_raw + _offset) * _scale
> > > > whereas _zeropoint is not. i.e. If we were computing the differential
> > > > in software it would be.
> > > > ((postive_raw + _zeropoint) - (negative_raw + zeropoint) + _offset) * _scale
> > > > = ((postive_raw - negative_raw) + _offset) * _scale
> > > > = (differential_raw + _offset) * _scale
> > > >
> > > > Similarly calibbias is expected to tweak the measurement seen, not
> > > > the adjust the two lines of the differential pair.
> > > >
> > > > Needed for in_capacitanceX-capacitanceY_zeropoint for the
> > > > AD7746 CDC driver.      
> > > 
> > > ...
> > >     
> > > > +What:          /sys/.../iio:deviceX/in_capacitanceY-capacitanceZ_zeropoint
> > > > +KernelVersion: 5.19      
> > > 
> > > 5.20?    
> > 
> > :) Probably 5.21 as I'm not going to rush this in now, but good point none the less.  
> 6.0 I guess :)  Anyhow I've applied this with Andy's tag on the assumption that the below
> English quirk is fine by Andy.
6.1.  As sounds like Linus will tag this merge window as 6.0 and this is being queued for the
next one. oops.
> 
> Jonathan
> 
> >   
> > >     
> > > > +Contact:       linux-iio@vger.kernel.org
> > > > +Description:
> > > > +               For differential channels, this an offset that is applied
> > > > +               equally to both inputs. As the reading is of the difference
> > > > +               between the two inputs, this should not be applied to the _raw
> > > > +               reading by userspace (unlike _offset) and unlike calibbias
> > > > +               it does not affect the differential value measured because
> > > > +               the effect of _zeropoint cancels out across the two inputs
> > > > +               that make up the differential pair. It's purpose is to bring      
> > > 
> > > makes    
> > 
> > No.  make is correct.
> > That's indeed an odd corner of English and honestly I'm not sure I could successfully
> > argue why it should be make :)
> >   
> > >     
> > > > +               the individual signals, before the differential is measured,
> > > > +               within the measurement range of the device. The naming is
> > > > +               chosen because if the separate inputs that make the
> > > > +               differential pair are drawn on a graph in their
> > > > +               _raw  units, this is the value that the zero point on the
> > > > +               measurement axis represents. It is expressed with the
> > > > +               same scaling as _raw.      
> > >     
> >   
> 

[PATCH v3 12/17] staging: iio: cdc: ad7746: Switch from _offset to _zeropoint for differential channels.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

As this offset is applied equally to both lines of the differential
pair, _ofset should not be used. Use the new ABI _zeropoint instead
to avoid userspace software applying this value when calculating
real value = (_raw + _offset) * _scale

Also add a comment to explain why an offset of 0x800000 is applied
within the driver rather than exposed to userspace.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 2b27a2cb807c..d8fbb8a85bbf 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -207,7 +207,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		.channel = 0,
 		.channel2 = 2,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
+		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_ZEROPOINT),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = CIN1_DIFF,
@@ -229,7 +229,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		.channel = 1,
 		.channel2 = 3,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
+		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_ZEROPOINT),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = CIN2_DIFF,
@@ -506,6 +506,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 
 		return 0;
 	case IIO_CHAN_INFO_OFFSET:
+	case IIO_CHAN_INFO_ZEROPOINT:
 		if (val < 0 || val > 43008000) /* 21pF */
 			return -EINVAL;
 
@@ -581,6 +582,10 @@ static int ad7746_read_channel(struct iio_dev *indio_dev,
 	if (ret < 0)
 		return ret;
 
+	/*
+	 * Offset applied internally becaue the _offset userspace interface is
+	 * needed for the CAP DACs which apply a controllable offset.
+	 */
 	*val = get_unaligned_be24(data) - 0x800000;
 
 	return 0;
@@ -637,6 +642,7 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_OFFSET:
+	case IIO_CHAN_INFO_ZEROPOINT:
 		*val = FIELD_GET(AD7746_CAPDAC_DACP_MASK,
 				 chip->capdac[chan->channel][chan->differential]) * 338646;
 
-- 
2.36.1

[PATCH v3 13/17] staging: iio: cdc: ad7746: Use read_avail() rather than opencoding.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switch over to the IIO core handling for _available attributes
making them available for in kernel users and enforcing correct
naming etc automatically.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 39 +++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index d8fbb8a85bbf..034054b09b6f 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -163,6 +163,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		.channel = 0,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = VIN,
 	},
 	[VIN_VDD] = {
@@ -172,6 +173,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		.extend_name = "supply",
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = VIN_VDD,
 	},
 	[TEMP_INT] = {
@@ -198,6 +200,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = CIN1,
 	},
 	[CIN1_DIFF] = {
@@ -210,6 +213,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_ZEROPOINT),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = CIN1_DIFF,
 	},
 	[CIN2] = {
@@ -220,6 +224,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = CIN2,
 	},
 	[CIN2_DIFF] = {
@@ -232,6 +237,7 @@ static const struct iio_chan_spec ad7746_channels[] = {
 		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_ZEROPOINT),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
 		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.address = CIN2_DIFF,
 	}
 };
@@ -440,18 +446,12 @@ static int ad7746_store_vt_filter_rate_setup(struct ad7746_chip_info *chip,
 	return 0;
 }
 
-static IIO_CONST_ATTR(in_voltage_sampling_frequency_available, "50 31 16 8");
-static IIO_CONST_ATTR(in_capacitance_sampling_frequency_available,
-		       "91 84 50 26 16 13 11 9");
-
 static struct attribute *ad7746_attributes[] = {
 	&iio_dev_attr_in_capacitance0_calibbias_calibration.dev_attr.attr,
 	&iio_dev_attr_in_capacitance0_calibscale_calibration.dev_attr.attr,
 	&iio_dev_attr_in_capacitance1_calibscale_calibration.dev_attr.attr,
 	&iio_dev_attr_in_capacitance1_calibbias_calibration.dev_attr.attr,
 	&iio_dev_attr_in_voltage0_calibscale_calibration.dev_attr.attr,
-	&iio_const_attr_in_voltage_sampling_frequency_available.dev_attr.attr,
-	&iio_const_attr_in_capacitance_sampling_frequency_available.dev_attr.attr,
 	NULL,
 };
 
@@ -554,6 +554,32 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 	}
 }
 
+static const int ad7746_v_samp_freq[] = { 50, 31, 16, 8, };
+static const int ad7746_cap_samp_freq[] = { 91, 84, 50, 26, 16, 13, 11, 9, };
+
+static int ad7746_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, const int **vals,
+			     int *type, int *length, long mask)
+{
+	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
+		return -EINVAL;
+
+	switch (chan->type) {
+	case IIO_VOLTAGE:
+		*vals = ad7746_v_samp_freq;
+		*length = ARRAY_SIZE(ad7746_v_samp_freq);
+		break;
+	case IIO_CAPACITANCE:
+		*vals = ad7746_cap_samp_freq;
+		*length = ARRAY_SIZE(ad7746_cap_samp_freq);
+		break;
+	default:
+		return -EINVAL;
+	}
+	*type = IIO_VAL_INT;
+	return IIO_AVAIL_LIST;
+}
+
 static int ad7746_read_channel(struct iio_dev *indio_dev,
 			       struct iio_chan_spec const *chan,
 			       int *val)
@@ -689,6 +715,7 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 static const struct iio_info ad7746_info = {
 	.attrs = &ad7746_attribute_group,
 	.read_raw = ad7746_read_raw,
+	.read_avail = ad7746_read_avail,
 	.write_raw = ad7746_write_raw,
 };
 
-- 
2.36.1

[PATCH v3 14/17] staging: iio: ad7746: White space cleanup

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Tidy up some trivial whitespace issues.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/staging/iio/cdc/ad7746.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 034054b09b6f..9ef14405a260 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -21,9 +21,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-/*
- * AD7746 Register Definition
- */
+/* AD7746 Register Definition */
 
 #define AD7746_REG_STATUS		0
 #define AD7746_REG_CAP_DATA_HIGH	1
@@ -244,12 +242,12 @@ static const struct iio_chan_spec ad7746_channels[] = {
 
 /* Values are Update Rate (Hz), Conversion Time (ms) + 1*/
 static const unsigned char ad7746_vt_filter_rate_table[][2] = {
-	{50, 20 + 1}, {31, 32 + 1}, {16, 62 + 1}, {8, 122 + 1},
+	{ 50, 20 + 1 }, { 31, 32 + 1 }, { 16, 62 + 1 }, { 8, 122 + 1 },
 };
 
 static const unsigned char ad7746_cap_filter_rate_table[][2] = {
-	{91, 11 + 1}, {84, 12 + 1}, {50, 20 + 1}, {26, 38 + 1},
-	{16, 62 + 1}, {13, 77 + 1}, {11, 92 + 1}, {9, 110 + 1},
+	{ 91, 11 + 1 }, { 84, 12 + 1 }, { 50, 20 + 1 }, { 26, 38 + 1 },
+	{ 16, 62 + 1 }, { 13, 77 + 1 }, { 11, 92 + 1 }, { 9, 110 + 1 },
 };
 
 static int ad7746_set_capdac(struct ad7746_chip_info *chip, int channel)
@@ -732,6 +730,7 @@ static int ad7746_probe(struct i2c_client *client,
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (!indio_dev)
 		return -ENOMEM;
+
 	chip = iio_priv(indio_dev);
 	mutex_init(&chip->lock);
 
@@ -782,8 +781,8 @@ static int ad7746_probe(struct i2c_client *client,
 		}
 	}
 
-	ret = i2c_smbus_write_byte_data(chip->client,
-					AD7746_REG_EXC_SETUP, regval);
+	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_EXC_SETUP,
+					regval);
 	if (ret < 0)
 		return ret;
 
@@ -796,7 +795,6 @@ static const struct i2c_device_id ad7746_id[] = {
 	{ "ad7747", 7747 },
 	{}
 };
-
 MODULE_DEVICE_TABLE(i2c, ad7746_id);
 
 static const struct of_device_id ad7746_of_match[] = {
@@ -805,7 +803,6 @@ static const struct of_device_id ad7746_of_match[] = {
 	{ .compatible = "adi,ad7747" },
 	{ },
 };
-
 MODULE_DEVICE_TABLE(of, ad7746_of_match);
 
 static struct i2c_driver ad7746_driver = {
-- 
2.36.1

[PATCH v3 15/17] iio: cdc: ad7746: Add device specific ABI documentation.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

The datasheet description of offset calibration is complex, so for that
on just refer the reader to the device datasheet.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 Documentation/ABI/testing/sysfs-bus-iio-cdc-ad7746 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cdc-ad7746 b/Documentation/ABI/testing/sysfs-bus-iio-cdc-ad7746
new file mode 100644
index 000000000000..6db81725b5d2
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-cdc-ad7746
@@ -0,0 +1,11 @@
+What:		/sys/.../iio:deviceX/in_capacitableY_calibbias_calibration
+What:		/sys/.../iio:deviceX/in_capacitableY_calibscale_calibration
+KernelVersion:	5.19
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Write 1 to trigger a calibration of the calibbias or
+		calibscale. For calibscale, a fullscale capacitance should
+		be connected to the capacitance input and a
+		calibscale_calibration then started.  For calibbias see
+		the device datasheet section on "capacitive system offset
+		calibration".
-- 
2.36.1

Re: [PATCH v3 15/17] iio: cdc: ad7746: Add device specific ABI documentation.

[Andy Shevchenko]

On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> The datasheet description of offset calibration is complex, so for that
> on just refer the reader to the device datasheet.

...

> +What:          /sys/.../iio:deviceX/in_capacitableY_calibbias_calibration
> +What:          /sys/.../iio:deviceX/in_capacitableY_calibscale_calibration
> +KernelVersion: 5.19

5.20 ?

> +Contact:       linux-iio@vger.kernel.org
> +Description:
> +               Write 1 to trigger a calibration of the calibbias or
> +               calibscale. For calibscale, a fullscale capacitance should

full scale

> +               be connected to the capacitance input and a
> +               calibscale_calibration then started.  For calibbias see
> +               the device datasheet section on "capacitive system offset
> +               calibration".


-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 16/17] iio: cdc: ad7746: Move driver out of staging.

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

All known major issues with this driver resolved so time to move
it out of staging. This also allows us to remove the now empty
staging/iio/cdc directory and build files.

Note this cleanup work was done using the roadtest framework.
https://lore.kernel.org/all/20220311162445.346685-1-vincent.whitchurch@axis.com/

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---

Note that I've deliberately used --no-renames to generate this patch.
The intent is to allow reviewers to easily see the driver as proposed
for staging graduation which would otherwise be hidden.

The ad7746.c file is moved with no changes.

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c
similarity index 100%
rename from drivers/staging/iio/cdc/ad7746.c
rename to drivers/iio/cdc/ad7746.c

 drivers/iio/cdc/Kconfig          |  10 +
 drivers/iio/cdc/Makefile         |   1 +
 drivers/iio/cdc/ad7746.c         | 820 +++++++++++++++++++++++++++++++
 drivers/staging/iio/Kconfig      |   1 -
 drivers/staging/iio/Makefile     |   1 -
 drivers/staging/iio/cdc/Kconfig  |  17 -
 drivers/staging/iio/cdc/Makefile |   6 -
 drivers/staging/iio/cdc/ad7746.c | 820 -------------------------------
 8 files changed, 831 insertions(+), 845 deletions(-)

diff --git a/drivers/iio/cdc/Kconfig b/drivers/iio/cdc/Kconfig
index 5e3319a3ff48..e0a5ce66a984 100644
--- a/drivers/iio/cdc/Kconfig
+++ b/drivers/iio/cdc/Kconfig
@@ -14,4 +14,14 @@ config AD7150
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7150.
 
+config AD7746
+	tristate "Analog Devices AD7745, AD7746 AD7747 capacitive sensor driver"
+	depends on I2C
+	help
+	  Say yes here to build support for Analog Devices capacitive sensors.
+	  (AD7745, AD7746, AD7747) Provides direct access via sysfs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad7746.
+
 endmenu
diff --git a/drivers/iio/cdc/Makefile b/drivers/iio/cdc/Makefile
index ee490637b032..41db756d8020 100644
--- a/drivers/iio/cdc/Makefile
+++ b/drivers/iio/cdc/Makefile
@@ -4,3 +4,4 @@
 #
 
 obj-$(CONFIG_AD7150) += ad7150.o
+obj-$(CONFIG_AD7746) += ad7746.o
diff --git a/drivers/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c
new file mode 100644
index 000000000000..9ef14405a260
--- /dev/null
+++ b/drivers/iio/cdc/ad7746.c
@@ -0,0 +1,820 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
+ *
+ * Copyright 2011 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/stat.h>
+#include <linux/sysfs.h>
+
+#include <asm/unaligned.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+/* AD7746 Register Definition */
+
+#define AD7746_REG_STATUS		0
+#define AD7746_REG_CAP_DATA_HIGH	1
+#define AD7746_REG_VT_DATA_HIGH		4
+#define AD7746_REG_CAP_SETUP		7
+#define AD7746_REG_VT_SETUP		8
+#define AD7746_REG_EXC_SETUP		9
+#define AD7746_REG_CFG			10
+#define AD7746_REG_CAPDACA		11
+#define AD7746_REG_CAPDACB		12
+#define AD7746_REG_CAP_OFFH		13
+#define AD7746_REG_CAP_GAINH		15
+#define AD7746_REG_VOLT_GAINH		17
+
+/* Status Register Bit Designations (AD7746_REG_STATUS) */
+#define AD7746_STATUS_EXCERR		BIT(3)
+#define AD7746_STATUS_RDY		BIT(2)
+#define AD7746_STATUS_RDYVT		BIT(1)
+#define AD7746_STATUS_RDYCAP		BIT(0)
+
+/* Capacitive Channel Setup Register Bit Designations (AD7746_REG_CAP_SETUP) */
+#define AD7746_CAPSETUP_CAPEN		BIT(7)
+#define AD7746_CAPSETUP_CIN2		BIT(6) /* AD7746 only */
+#define AD7746_CAPSETUP_CAPDIFF		BIT(5)
+#define AD7746_CAPSETUP_CACHOP		BIT(0)
+
+/* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) */
+#define AD7746_VTSETUP_VTEN		BIT(7)
+#define AD7746_VTSETUP_VTMD_MASK	GENMASK(6, 5)
+#define AD7746_VTSETUP_VTMD_INT_TEMP	0
+#define AD7746_VTSETUP_VTMD_EXT_TEMP	1
+#define AD7746_VTSETUP_VTMD_VDD_MON	2
+#define AD7746_VTSETUP_VTMD_EXT_VIN	3
+#define AD7746_VTSETUP_EXTREF		BIT(4)
+#define AD7746_VTSETUP_VTSHORT		BIT(1)
+#define AD7746_VTSETUP_VTCHOP		BIT(0)
+
+/* Excitation Setup Register Bit Designations (AD7746_REG_EXC_SETUP) */
+#define AD7746_EXCSETUP_CLKCTRL		BIT(7)
+#define AD7746_EXCSETUP_EXCON		BIT(6)
+#define AD7746_EXCSETUP_EXCB		BIT(5)
+#define AD7746_EXCSETUP_NEXCB		BIT(4)
+#define AD7746_EXCSETUP_EXCA		BIT(3)
+#define AD7746_EXCSETUP_NEXCA		BIT(2)
+#define AD7746_EXCSETUP_EXCLVL_MASK	GENMASK(1, 0)
+
+/* Config Register Bit Designations (AD7746_REG_CFG) */
+#define AD7746_CONF_VTFS_MASK		GENMASK(7, 6)
+#define AD7746_CONF_CAPFS_MASK		GENMASK(5, 3)
+#define AD7746_CONF_MODE_MASK		GENMASK(2, 0)
+#define AD7746_CONF_MODE_IDLE		0
+#define AD7746_CONF_MODE_CONT_CONV	1
+#define AD7746_CONF_MODE_SINGLE_CONV	2
+#define AD7746_CONF_MODE_PWRDN		3
+#define AD7746_CONF_MODE_OFFS_CAL	5
+#define AD7746_CONF_MODE_GAIN_CAL	6
+
+/* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
+#define AD7746_CAPDAC_DACEN		BIT(7)
+#define AD7746_CAPDAC_DACP_MASK		0x7F
+
+struct ad7746_chip_info {
+	struct i2c_client *client;
+	struct mutex lock; /* protect sensor state */
+	/*
+	 * Capacitive channel digital filter setup;
+	 * conversion time/update rate setup per channel
+	 */
+	u8	config;
+	u8	cap_setup;
+	u8	vt_setup;
+	u8	capdac[2][2];
+	s8	capdac_set;
+};
+
+enum ad7746_chan {
+	VIN,
+	VIN_VDD,
+	TEMP_INT,
+	TEMP_EXT,
+	CIN1,
+	CIN1_DIFF,
+	CIN2,
+	CIN2_DIFF,
+};
+
+struct ad7746_chan_info {
+	u8 addr;
+	union {
+		u8 vtmd;
+		struct { /* CAP SETUP fields */
+			unsigned int cin2 : 1;
+			unsigned int capdiff : 1;
+		};
+	};
+};
+
+static const struct ad7746_chan_info ad7746_chan_info[] = {
+	[VIN] = {
+		.addr = AD7746_REG_VT_DATA_HIGH,
+		.vtmd = AD7746_VTSETUP_VTMD_EXT_VIN,
+	},
+	[VIN_VDD] = {
+		.addr = AD7746_REG_VT_DATA_HIGH,
+		.vtmd = AD7746_VTSETUP_VTMD_VDD_MON,
+	},
+	[TEMP_INT] = {
+		.addr = AD7746_REG_VT_DATA_HIGH,
+		.vtmd = AD7746_VTSETUP_VTMD_INT_TEMP,
+	},
+	[TEMP_EXT] = {
+		.addr = AD7746_REG_VT_DATA_HIGH,
+		.vtmd = AD7746_VTSETUP_VTMD_EXT_TEMP,
+	},
+	[CIN1] = {
+		.addr = AD7746_REG_CAP_DATA_HIGH,
+	},
+	[CIN1_DIFF] = {
+		.addr =  AD7746_REG_CAP_DATA_HIGH,
+		.capdiff = 1,
+	},
+	[CIN2] = {
+		.addr = AD7746_REG_CAP_DATA_HIGH,
+		.cin2 = 1,
+	},
+	[CIN2_DIFF] = {
+		.addr = AD7746_REG_CAP_DATA_HIGH,
+		.cin2 = 1,
+		.capdiff = 1,
+	},
+};
+
+static const struct iio_chan_spec ad7746_channels[] = {
+	[VIN] = {
+		.type = IIO_VOLTAGE,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.address = VIN,
+	},
+	[VIN_VDD] = {
+		.type = IIO_VOLTAGE,
+		.indexed = 1,
+		.channel = 1,
+		.extend_name = "supply",
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.address = VIN_VDD,
+	},
+	[TEMP_INT] = {
+		.type = IIO_TEMP,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+		.address = TEMP_INT,
+	},
+	[TEMP_EXT] = {
+		.type = IIO_TEMP,
+		.indexed = 1,
+		.channel = 1,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+		.address = TEMP_EXT,
+	},
+	[CIN1] = {
+		.type = IIO_CAPACITANCE,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
+		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.address = CIN1,
+	},
+	[CIN1_DIFF] = {
+		.type = IIO_CAPACITANCE,
+		.differential = 1,
+		.indexed = 1,
+		.channel = 0,
+		.channel2 = 2,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_ZEROPOINT),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
+		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.address = CIN1_DIFF,
+	},
+	[CIN2] = {
+		.type = IIO_CAPACITANCE,
+		.indexed = 1,
+		.channel = 1,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
+		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.address = CIN2,
+	},
+	[CIN2_DIFF] = {
+		.type = IIO_CAPACITANCE,
+		.differential = 1,
+		.indexed = 1,
+		.channel = 1,
+		.channel2 = 3,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_ZEROPOINT),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
+		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.address = CIN2_DIFF,
+	}
+};
+
+/* Values are Update Rate (Hz), Conversion Time (ms) + 1*/
+static const unsigned char ad7746_vt_filter_rate_table[][2] = {
+	{ 50, 20 + 1 }, { 31, 32 + 1 }, { 16, 62 + 1 }, { 8, 122 + 1 },
+};
+
+static const unsigned char ad7746_cap_filter_rate_table[][2] = {
+	{ 91, 11 + 1 }, { 84, 12 + 1 }, { 50, 20 + 1 }, { 26, 38 + 1 },
+	{ 16, 62 + 1 }, { 13, 77 + 1 }, { 11, 92 + 1 }, { 9, 110 + 1 },
+};
+
+static int ad7746_set_capdac(struct ad7746_chip_info *chip, int channel)
+{
+	int ret = i2c_smbus_write_byte_data(chip->client,
+					    AD7746_REG_CAPDACA,
+					    chip->capdac[channel][0]);
+	if (ret < 0)
+		return ret;
+
+	return i2c_smbus_write_byte_data(chip->client,
+					  AD7746_REG_CAPDACB,
+					  chip->capdac[channel][1]);
+}
+
+static int ad7746_select_channel(struct iio_dev *indio_dev,
+				 struct iio_chan_spec const *chan)
+{
+	struct ad7746_chip_info *chip = iio_priv(indio_dev);
+	u8 vt_setup, cap_setup;
+	int ret, delay, idx;
+
+	switch (chan->type) {
+	case IIO_CAPACITANCE:
+		cap_setup = FIELD_PREP(AD7746_CAPSETUP_CIN2,
+				       ad7746_chan_info[chan->address].cin2) |
+			FIELD_PREP(AD7746_CAPSETUP_CAPDIFF,
+				   ad7746_chan_info[chan->address].capdiff) |
+			FIELD_PREP(AD7746_CAPSETUP_CAPEN, 1);
+		vt_setup = chip->vt_setup & ~AD7746_VTSETUP_VTEN;
+		idx = FIELD_GET(AD7746_CONF_CAPFS_MASK, chip->config);
+		delay = ad7746_cap_filter_rate_table[idx][1];
+
+		ret = ad7746_set_capdac(chip, chan->channel);
+		if (ret < 0)
+			return ret;
+
+		if (chip->capdac_set != chan->channel)
+			chip->capdac_set = chan->channel;
+		break;
+	case IIO_VOLTAGE:
+	case IIO_TEMP:
+		vt_setup = FIELD_PREP(AD7746_VTSETUP_VTMD_MASK,
+				      ad7746_chan_info[chan->address].vtmd) |
+			FIELD_PREP(AD7746_VTSETUP_VTEN, 1);
+		cap_setup = chip->cap_setup & ~AD7746_CAPSETUP_CAPEN;
+		idx = FIELD_GET(AD7746_CONF_VTFS_MASK, chip->config);
+		delay = ad7746_cap_filter_rate_table[idx][1];
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (chip->cap_setup != cap_setup) {
+		ret = i2c_smbus_write_byte_data(chip->client,
+						AD7746_REG_CAP_SETUP,
+						cap_setup);
+		if (ret < 0)
+			return ret;
+
+		chip->cap_setup = cap_setup;
+	}
+
+	if (chip->vt_setup != vt_setup) {
+		ret = i2c_smbus_write_byte_data(chip->client,
+						AD7746_REG_VT_SETUP,
+						vt_setup);
+		if (ret < 0)
+			return ret;
+
+		chip->vt_setup = vt_setup;
+	}
+
+	return delay;
+}
+
+static inline ssize_t ad7746_start_calib(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf,
+					 size_t len,
+					 u8 regval)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct ad7746_chip_info *chip = iio_priv(indio_dev);
+	int ret, timeout = 10;
+	bool doit;
+
+	ret = kstrtobool(buf, &doit);
+	if (ret < 0)
+		return ret;
+
+	if (!doit)
+		return 0;
+
+	mutex_lock(&chip->lock);
+	regval |= chip->config;
+	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
+	if (ret < 0)
+		goto unlock;
+
+	do {
+		msleep(20);
+		ret = i2c_smbus_read_byte_data(chip->client, AD7746_REG_CFG);
+		if (ret < 0)
+			goto unlock;
+
+	} while ((ret == regval) && timeout--);
+
+	mutex_unlock(&chip->lock);
+
+	return len;
+
+unlock:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+static ssize_t ad7746_start_offset_calib(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf,
+					 size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	int ret = ad7746_select_channel(indio_dev,
+			      &ad7746_channels[to_iio_dev_attr(attr)->address]);
+	if (ret < 0)
+		return ret;
+
+	return ad7746_start_calib(dev, attr, buf, len,
+				  FIELD_PREP(AD7746_CONF_MODE_MASK,
+					     AD7746_CONF_MODE_OFFS_CAL));
+}
+
+static ssize_t ad7746_start_gain_calib(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf,
+				       size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	int ret = ad7746_select_channel(indio_dev,
+			      &ad7746_channels[to_iio_dev_attr(attr)->address]);
+	if (ret < 0)
+		return ret;
+
+	return ad7746_start_calib(dev, attr, buf, len,
+				  FIELD_PREP(AD7746_CONF_MODE_MASK,
+					     AD7746_CONF_MODE_GAIN_CAL));
+}
+
+static IIO_DEVICE_ATTR(in_capacitance0_calibbias_calibration,
+		       0200, NULL, ad7746_start_offset_calib, CIN1);
+static IIO_DEVICE_ATTR(in_capacitance1_calibbias_calibration,
+		       0200, NULL, ad7746_start_offset_calib, CIN2);
+static IIO_DEVICE_ATTR(in_capacitance0_calibscale_calibration,
+		       0200, NULL, ad7746_start_gain_calib, CIN1);
+static IIO_DEVICE_ATTR(in_capacitance1_calibscale_calibration,
+		       0200, NULL, ad7746_start_gain_calib, CIN2);
+static IIO_DEVICE_ATTR(in_voltage0_calibscale_calibration,
+		       0200, NULL, ad7746_start_gain_calib, VIN);
+
+static int ad7746_store_cap_filter_rate_setup(struct ad7746_chip_info *chip,
+					      int val)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ad7746_cap_filter_rate_table); i++)
+		if (val >= ad7746_cap_filter_rate_table[i][0])
+			break;
+
+	if (i >= ARRAY_SIZE(ad7746_cap_filter_rate_table))
+		i = ARRAY_SIZE(ad7746_cap_filter_rate_table) - 1;
+
+	chip->config &= ~AD7746_CONF_CAPFS_MASK;
+	chip->config |= FIELD_PREP(AD7746_CONF_CAPFS_MASK, i);
+
+	return 0;
+}
+
+static int ad7746_store_vt_filter_rate_setup(struct ad7746_chip_info *chip,
+					     int val)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ad7746_vt_filter_rate_table); i++)
+		if (val >= ad7746_vt_filter_rate_table[i][0])
+			break;
+
+	if (i >= ARRAY_SIZE(ad7746_vt_filter_rate_table))
+		i = ARRAY_SIZE(ad7746_vt_filter_rate_table) - 1;
+
+	chip->config &= ~AD7746_CONF_VTFS_MASK;
+	chip->config |= FIELD_PREP(AD7746_CONF_VTFS_MASK, i);
+
+	return 0;
+}
+
+static struct attribute *ad7746_attributes[] = {
+	&iio_dev_attr_in_capacitance0_calibbias_calibration.dev_attr.attr,
+	&iio_dev_attr_in_capacitance0_calibscale_calibration.dev_attr.attr,
+	&iio_dev_attr_in_capacitance1_calibscale_calibration.dev_attr.attr,
+	&iio_dev_attr_in_capacitance1_calibbias_calibration.dev_attr.attr,
+	&iio_dev_attr_in_voltage0_calibscale_calibration.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group ad7746_attribute_group = {
+	.attrs = ad7746_attributes,
+};
+
+static int ad7746_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val,
+			    int val2,
+			    long mask)
+{
+	struct ad7746_chip_info *chip = iio_priv(indio_dev);
+	int ret, reg;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		if (val != 1)
+			return -EINVAL;
+
+		val = (val2 * 1024) / 15625;
+
+		switch (chan->type) {
+		case IIO_CAPACITANCE:
+			reg = AD7746_REG_CAP_GAINH;
+			break;
+		case IIO_VOLTAGE:
+			reg = AD7746_REG_VOLT_GAINH;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		mutex_lock(&chip->lock);
+		ret = i2c_smbus_write_word_swapped(chip->client, reg, val);
+		mutex_unlock(&chip->lock);
+		if (ret < 0)
+			return ret;
+
+		return 0;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		if (val < 0 || val > 0xFFFF)
+			return -EINVAL;
+
+		mutex_lock(&chip->lock);
+		ret = i2c_smbus_write_word_swapped(chip->client,
+						   AD7746_REG_CAP_OFFH, val);
+		mutex_unlock(&chip->lock);
+		if (ret < 0)
+			return ret;
+
+		return 0;
+	case IIO_CHAN_INFO_OFFSET:
+	case IIO_CHAN_INFO_ZEROPOINT:
+		if (val < 0 || val > 43008000) /* 21pF */
+			return -EINVAL;
+
+		/*
+		 * CAPDAC Scale = 21pF_typ / 127
+		 * CIN Scale = 8.192pF / 2^24
+		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
+		 */
+
+		val /= 338646;
+		mutex_lock(&chip->lock);
+		chip->capdac[chan->channel][chan->differential] = val > 0 ?
+			FIELD_PREP(AD7746_CAPDAC_DACP_MASK, val) | AD7746_CAPDAC_DACEN : 0;
+
+		ret = ad7746_set_capdac(chip, chan->channel);
+		if (ret < 0) {
+			mutex_unlock(&chip->lock);
+			return ret;
+		}
+
+		chip->capdac_set = chan->channel;
+		mutex_unlock(&chip->lock);
+
+		return 0;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (val2)
+			return -EINVAL;
+
+		switch (chan->type) {
+		case IIO_CAPACITANCE:
+			mutex_lock(&chip->lock);
+			ret = ad7746_store_cap_filter_rate_setup(chip, val);
+			mutex_unlock(&chip->lock);
+			return ret;
+		case IIO_VOLTAGE:
+			mutex_lock(&chip->lock);
+			ret = ad7746_store_vt_filter_rate_setup(chip, val);
+			mutex_unlock(&chip->lock);
+			return ret;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static const int ad7746_v_samp_freq[] = { 50, 31, 16, 8, };
+static const int ad7746_cap_samp_freq[] = { 91, 84, 50, 26, 16, 13, 11, 9, };
+
+static int ad7746_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, const int **vals,
+			     int *type, int *length, long mask)
+{
+	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
+		return -EINVAL;
+
+	switch (chan->type) {
+	case IIO_VOLTAGE:
+		*vals = ad7746_v_samp_freq;
+		*length = ARRAY_SIZE(ad7746_v_samp_freq);
+		break;
+	case IIO_CAPACITANCE:
+		*vals = ad7746_cap_samp_freq;
+		*length = ARRAY_SIZE(ad7746_cap_samp_freq);
+		break;
+	default:
+		return -EINVAL;
+	}
+	*type = IIO_VAL_INT;
+	return IIO_AVAIL_LIST;
+}
+
+static int ad7746_read_channel(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int *val)
+{
+	struct ad7746_chip_info *chip = iio_priv(indio_dev);
+	int ret, delay;
+	u8 data[3];
+	u8 regval;
+
+	ret = ad7746_select_channel(indio_dev, chan);
+	if (ret < 0)
+		return ret;
+	delay = ret;
+
+	regval = chip->config | FIELD_PREP(AD7746_CONF_MODE_MASK,
+					   AD7746_CONF_MODE_SINGLE_CONV);
+	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
+	if (ret < 0)
+		return ret;
+
+	msleep(delay);
+	/* Now read the actual register */
+	ret = i2c_smbus_read_i2c_block_data(chip->client,
+					    ad7746_chan_info[chan->address].addr,
+					    sizeof(data), data);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Offset applied internally becaue the _offset userspace interface is
+	 * needed for the CAP DACs which apply a controllable offset.
+	 */
+	*val = get_unaligned_be24(data) - 0x800000;
+
+	return 0;
+}
+
+static int ad7746_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2,
+			   long mask)
+{
+	struct ad7746_chip_info *chip = iio_priv(indio_dev);
+	int ret, idx;
+	u8 reg;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&chip->lock);
+		ret = ad7746_read_channel(indio_dev, chan, val);
+		mutex_unlock(&chip->lock);
+		if (ret < 0)
+			return ret;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		switch (chan->type) {
+		case IIO_CAPACITANCE:
+			reg = AD7746_REG_CAP_GAINH;
+			break;
+		case IIO_VOLTAGE:
+			reg = AD7746_REG_VOLT_GAINH;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		mutex_lock(&chip->lock);
+		ret = i2c_smbus_read_word_swapped(chip->client, reg);
+		mutex_unlock(&chip->lock);
+		if (ret < 0)
+			return ret;
+		/* 1 + gain_val / 2^16 */
+		*val = 1;
+		*val2 = (15625 * ret) / 1024;
+
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		mutex_lock(&chip->lock);
+		ret = i2c_smbus_read_word_swapped(chip->client,
+						  AD7746_REG_CAP_OFFH);
+		mutex_unlock(&chip->lock);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_OFFSET:
+	case IIO_CHAN_INFO_ZEROPOINT:
+		*val = FIELD_GET(AD7746_CAPDAC_DACP_MASK,
+				 chip->capdac[chan->channel][chan->differential]) * 338646;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_CAPACITANCE:
+			/* 8.192pf / 2^24 */
+			*val =  0;
+			*val2 = 488;
+			return IIO_VAL_INT_PLUS_NANO;
+		case IIO_VOLTAGE:
+			/* 1170mV / 2^23 */
+			*val = 1170;
+			if (chan->channel == 1)
+				*val *= 6;
+			*val2 = 23;
+			return IIO_VAL_FRACTIONAL_LOG2;
+		case IIO_TEMP:
+			*val = 125;
+			*val2 = 8;
+			return IIO_VAL_FRACTIONAL_LOG2;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		switch (chan->type) {
+		case IIO_CAPACITANCE:
+			idx = FIELD_GET(AD7746_CONF_CAPFS_MASK, chip->config);
+			*val = ad7746_cap_filter_rate_table[idx][0];
+			return IIO_VAL_INT;
+		case IIO_VOLTAGE:
+			idx = FIELD_GET(AD7746_CONF_VTFS_MASK, chip->config);
+			*val = ad7746_vt_filter_rate_table[idx][0];
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info ad7746_info = {
+	.attrs = &ad7746_attribute_group,
+	.read_raw = ad7746_read_raw,
+	.read_avail = ad7746_read_avail,
+	.write_raw = ad7746_write_raw,
+};
+
+static int ad7746_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct ad7746_chip_info *chip;
+	struct iio_dev *indio_dev;
+	unsigned char regval = 0;
+	unsigned int vdd_permille;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	chip = iio_priv(indio_dev);
+	mutex_init(&chip->lock);
+
+	chip->client = client;
+	chip->capdac_set = -1;
+
+	indio_dev->name = id->name;
+	indio_dev->info = &ad7746_info;
+	indio_dev->channels = ad7746_channels;
+	if (id->driver_data == 7746)
+		indio_dev->num_channels = ARRAY_SIZE(ad7746_channels);
+	else
+		indio_dev->num_channels =  ARRAY_SIZE(ad7746_channels) - 2;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	if (device_property_read_bool(dev, "adi,exca-output-en")) {
+		if (device_property_read_bool(dev, "adi,exca-output-invert"))
+			regval |= AD7746_EXCSETUP_NEXCA;
+		else
+			regval |= AD7746_EXCSETUP_EXCA;
+	}
+
+	if (device_property_read_bool(dev, "adi,excb-output-en")) {
+		if (device_property_read_bool(dev, "adi,excb-output-invert"))
+			regval |= AD7746_EXCSETUP_NEXCB;
+		else
+			regval |= AD7746_EXCSETUP_EXCB;
+	}
+
+	ret = device_property_read_u32(dev, "adi,excitation-vdd-permille",
+				       &vdd_permille);
+	if (!ret) {
+		switch (vdd_permille) {
+		case 125:
+			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 0);
+			break;
+		case 250:
+			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 1);
+			break;
+		case 375:
+			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 2);
+			break;
+		case 500:
+			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 3);
+			break;
+		default:
+			break;
+		}
+	}
+
+	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_EXC_SETUP,
+					regval);
+	if (ret < 0)
+		return ret;
+
+	return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
+}
+
+static const struct i2c_device_id ad7746_id[] = {
+	{ "ad7745", 7745 },
+	{ "ad7746", 7746 },
+	{ "ad7747", 7747 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ad7746_id);
+
+static const struct of_device_id ad7746_of_match[] = {
+	{ .compatible = "adi,ad7745" },
+	{ .compatible = "adi,ad7746" },
+	{ .compatible = "adi,ad7747" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ad7746_of_match);
+
+static struct i2c_driver ad7746_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = ad7746_of_match,
+	},
+	.probe = ad7746_probe,
+	.id_table = ad7746_id,
+};
+module_i2c_driver(ad7746_driver);
+
+MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD7746/5/7 capacitive sensor driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
index a8e970db179d..afd05bf3345e 100644
--- a/drivers/staging/iio/Kconfig
+++ b/drivers/staging/iio/Kconfig
@@ -8,7 +8,6 @@ menu "IIO staging drivers"
 source "drivers/staging/iio/accel/Kconfig"
 source "drivers/staging/iio/adc/Kconfig"
 source "drivers/staging/iio/addac/Kconfig"
-source "drivers/staging/iio/cdc/Kconfig"
 source "drivers/staging/iio/frequency/Kconfig"
 source "drivers/staging/iio/impedance-analyzer/Kconfig"
 source "drivers/staging/iio/meter/Kconfig"
diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
index b15904b99581..5ed56fe57e14 100644
--- a/drivers/staging/iio/Makefile
+++ b/drivers/staging/iio/Makefile
@@ -6,7 +6,6 @@
 obj-y += accel/
 obj-y += adc/
 obj-y += addac/
-obj-y += cdc/
 obj-y += frequency/
 obj-y += impedance-analyzer/
 obj-y += meter/
diff --git a/drivers/staging/iio/cdc/Kconfig b/drivers/staging/iio/cdc/Kconfig
deleted file mode 100644
index a7386bbbcb79..000000000000
--- a/drivers/staging/iio/cdc/Kconfig
+++ /dev/null
@@ -1,17 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-#
-# CDC drivers
-#
-menu "Capacitance to digital converters"
-
-config AD7746
-	tristate "Analog Devices AD7745, AD7746 AD7747 capacitive sensor driver"
-	depends on I2C
-	help
-	  Say yes here to build support for Analog Devices capacitive sensors.
-	  (AD7745, AD7746, AD7747) Provides direct access via sysfs.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called ad7746.
-
-endmenu
diff --git a/drivers/staging/iio/cdc/Makefile b/drivers/staging/iio/cdc/Makefile
deleted file mode 100644
index afb7499a7090..000000000000
--- a/drivers/staging/iio/cdc/Makefile
+++ /dev/null
@@ -1,6 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-#
-# Makefile for industrial I/O CDC drivers
-#
-
-obj-$(CONFIG_AD7746) += ad7746.o
diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
deleted file mode 100644
index 9ef14405a260..000000000000
--- a/drivers/staging/iio/cdc/ad7746.c
+++ /dev/null
@@ -1,820 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
- *
- * Copyright 2011 Analog Devices Inc.
- */
-
-#include <linux/bitfield.h>
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/i2c.h>
-#include <linux/interrupt.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/stat.h>
-#include <linux/sysfs.h>
-
-#include <asm/unaligned.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-
-/* AD7746 Register Definition */
-
-#define AD7746_REG_STATUS		0
-#define AD7746_REG_CAP_DATA_HIGH	1
-#define AD7746_REG_VT_DATA_HIGH		4
-#define AD7746_REG_CAP_SETUP		7
-#define AD7746_REG_VT_SETUP		8
-#define AD7746_REG_EXC_SETUP		9
-#define AD7746_REG_CFG			10
-#define AD7746_REG_CAPDACA		11
-#define AD7746_REG_CAPDACB		12
-#define AD7746_REG_CAP_OFFH		13
-#define AD7746_REG_CAP_GAINH		15
-#define AD7746_REG_VOLT_GAINH		17
-
-/* Status Register Bit Designations (AD7746_REG_STATUS) */
-#define AD7746_STATUS_EXCERR		BIT(3)
-#define AD7746_STATUS_RDY		BIT(2)
-#define AD7746_STATUS_RDYVT		BIT(1)
-#define AD7746_STATUS_RDYCAP		BIT(0)
-
-/* Capacitive Channel Setup Register Bit Designations (AD7746_REG_CAP_SETUP) */
-#define AD7746_CAPSETUP_CAPEN		BIT(7)
-#define AD7746_CAPSETUP_CIN2		BIT(6) /* AD7746 only */
-#define AD7746_CAPSETUP_CAPDIFF		BIT(5)
-#define AD7746_CAPSETUP_CACHOP		BIT(0)
-
-/* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) */
-#define AD7746_VTSETUP_VTEN		BIT(7)
-#define AD7746_VTSETUP_VTMD_MASK	GENMASK(6, 5)
-#define AD7746_VTSETUP_VTMD_INT_TEMP	0
-#define AD7746_VTSETUP_VTMD_EXT_TEMP	1
-#define AD7746_VTSETUP_VTMD_VDD_MON	2
-#define AD7746_VTSETUP_VTMD_EXT_VIN	3
-#define AD7746_VTSETUP_EXTREF		BIT(4)
-#define AD7746_VTSETUP_VTSHORT		BIT(1)
-#define AD7746_VTSETUP_VTCHOP		BIT(0)
-
-/* Excitation Setup Register Bit Designations (AD7746_REG_EXC_SETUP) */
-#define AD7746_EXCSETUP_CLKCTRL		BIT(7)
-#define AD7746_EXCSETUP_EXCON		BIT(6)
-#define AD7746_EXCSETUP_EXCB		BIT(5)
-#define AD7746_EXCSETUP_NEXCB		BIT(4)
-#define AD7746_EXCSETUP_EXCA		BIT(3)
-#define AD7746_EXCSETUP_NEXCA		BIT(2)
-#define AD7746_EXCSETUP_EXCLVL_MASK	GENMASK(1, 0)
-
-/* Config Register Bit Designations (AD7746_REG_CFG) */
-#define AD7746_CONF_VTFS_MASK		GENMASK(7, 6)
-#define AD7746_CONF_CAPFS_MASK		GENMASK(5, 3)
-#define AD7746_CONF_MODE_MASK		GENMASK(2, 0)
-#define AD7746_CONF_MODE_IDLE		0
-#define AD7746_CONF_MODE_CONT_CONV	1
-#define AD7746_CONF_MODE_SINGLE_CONV	2
-#define AD7746_CONF_MODE_PWRDN		3
-#define AD7746_CONF_MODE_OFFS_CAL	5
-#define AD7746_CONF_MODE_GAIN_CAL	6
-
-/* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
-#define AD7746_CAPDAC_DACEN		BIT(7)
-#define AD7746_CAPDAC_DACP_MASK		0x7F
-
-struct ad7746_chip_info {
-	struct i2c_client *client;
-	struct mutex lock; /* protect sensor state */
-	/*
-	 * Capacitive channel digital filter setup;
-	 * conversion time/update rate setup per channel
-	 */
-	u8	config;
-	u8	cap_setup;
-	u8	vt_setup;
-	u8	capdac[2][2];
-	s8	capdac_set;
-};
-
-enum ad7746_chan {
-	VIN,
-	VIN_VDD,
-	TEMP_INT,
-	TEMP_EXT,
-	CIN1,
-	CIN1_DIFF,
-	CIN2,
-	CIN2_DIFF,
-};
-
-struct ad7746_chan_info {
-	u8 addr;
-	union {
-		u8 vtmd;
-		struct { /* CAP SETUP fields */
-			unsigned int cin2 : 1;
-			unsigned int capdiff : 1;
-		};
-	};
-};
-
-static const struct ad7746_chan_info ad7746_chan_info[] = {
-	[VIN] = {
-		.addr = AD7746_REG_VT_DATA_HIGH,
-		.vtmd = AD7746_VTSETUP_VTMD_EXT_VIN,
-	},
-	[VIN_VDD] = {
-		.addr = AD7746_REG_VT_DATA_HIGH,
-		.vtmd = AD7746_VTSETUP_VTMD_VDD_MON,
-	},
-	[TEMP_INT] = {
-		.addr = AD7746_REG_VT_DATA_HIGH,
-		.vtmd = AD7746_VTSETUP_VTMD_INT_TEMP,
-	},
-	[TEMP_EXT] = {
-		.addr = AD7746_REG_VT_DATA_HIGH,
-		.vtmd = AD7746_VTSETUP_VTMD_EXT_TEMP,
-	},
-	[CIN1] = {
-		.addr = AD7746_REG_CAP_DATA_HIGH,
-	},
-	[CIN1_DIFF] = {
-		.addr =  AD7746_REG_CAP_DATA_HIGH,
-		.capdiff = 1,
-	},
-	[CIN2] = {
-		.addr = AD7746_REG_CAP_DATA_HIGH,
-		.cin2 = 1,
-	},
-	[CIN2_DIFF] = {
-		.addr = AD7746_REG_CAP_DATA_HIGH,
-		.cin2 = 1,
-		.capdiff = 1,
-	},
-};
-
-static const struct iio_chan_spec ad7746_channels[] = {
-	[VIN] = {
-		.type = IIO_VOLTAGE,
-		.indexed = 1,
-		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = VIN,
-	},
-	[VIN_VDD] = {
-		.type = IIO_VOLTAGE,
-		.indexed = 1,
-		.channel = 1,
-		.extend_name = "supply",
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = VIN_VDD,
-	},
-	[TEMP_INT] = {
-		.type = IIO_TEMP,
-		.indexed = 1,
-		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
-		.address = TEMP_INT,
-	},
-	[TEMP_EXT] = {
-		.type = IIO_TEMP,
-		.indexed = 1,
-		.channel = 1,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
-		.address = TEMP_EXT,
-	},
-	[CIN1] = {
-		.type = IIO_CAPACITANCE,
-		.indexed = 1,
-		.channel = 0,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
-		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = CIN1,
-	},
-	[CIN1_DIFF] = {
-		.type = IIO_CAPACITANCE,
-		.differential = 1,
-		.indexed = 1,
-		.channel = 0,
-		.channel2 = 2,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_ZEROPOINT),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
-		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = CIN1_DIFF,
-	},
-	[CIN2] = {
-		.type = IIO_CAPACITANCE,
-		.indexed = 1,
-		.channel = 1,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_OFFSET),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
-		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = CIN2,
-	},
-	[CIN2_DIFF] = {
-		.type = IIO_CAPACITANCE,
-		.differential = 1,
-		.indexed = 1,
-		.channel = 1,
-		.channel2 = 3,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-		BIT(IIO_CHAN_INFO_CALIBSCALE) | BIT(IIO_CHAN_INFO_ZEROPOINT),
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_CALIBBIAS) |
-		BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
-		.address = CIN2_DIFF,
-	}
-};
-
-/* Values are Update Rate (Hz), Conversion Time (ms) + 1*/
-static const unsigned char ad7746_vt_filter_rate_table[][2] = {
-	{ 50, 20 + 1 }, { 31, 32 + 1 }, { 16, 62 + 1 }, { 8, 122 + 1 },
-};
-
-static const unsigned char ad7746_cap_filter_rate_table[][2] = {
-	{ 91, 11 + 1 }, { 84, 12 + 1 }, { 50, 20 + 1 }, { 26, 38 + 1 },
-	{ 16, 62 + 1 }, { 13, 77 + 1 }, { 11, 92 + 1 }, { 9, 110 + 1 },
-};
-
-static int ad7746_set_capdac(struct ad7746_chip_info *chip, int channel)
-{
-	int ret = i2c_smbus_write_byte_data(chip->client,
-					    AD7746_REG_CAPDACA,
-					    chip->capdac[channel][0]);
-	if (ret < 0)
-		return ret;
-
-	return i2c_smbus_write_byte_data(chip->client,
-					  AD7746_REG_CAPDACB,
-					  chip->capdac[channel][1]);
-}
-
-static int ad7746_select_channel(struct iio_dev *indio_dev,
-				 struct iio_chan_spec const *chan)
-{
-	struct ad7746_chip_info *chip = iio_priv(indio_dev);
-	u8 vt_setup, cap_setup;
-	int ret, delay, idx;
-
-	switch (chan->type) {
-	case IIO_CAPACITANCE:
-		cap_setup = FIELD_PREP(AD7746_CAPSETUP_CIN2,
-				       ad7746_chan_info[chan->address].cin2) |
-			FIELD_PREP(AD7746_CAPSETUP_CAPDIFF,
-				   ad7746_chan_info[chan->address].capdiff) |
-			FIELD_PREP(AD7746_CAPSETUP_CAPEN, 1);
-		vt_setup = chip->vt_setup & ~AD7746_VTSETUP_VTEN;
-		idx = FIELD_GET(AD7746_CONF_CAPFS_MASK, chip->config);
-		delay = ad7746_cap_filter_rate_table[idx][1];
-
-		ret = ad7746_set_capdac(chip, chan->channel);
-		if (ret < 0)
-			return ret;
-
-		if (chip->capdac_set != chan->channel)
-			chip->capdac_set = chan->channel;
-		break;
-	case IIO_VOLTAGE:
-	case IIO_TEMP:
-		vt_setup = FIELD_PREP(AD7746_VTSETUP_VTMD_MASK,
-				      ad7746_chan_info[chan->address].vtmd) |
-			FIELD_PREP(AD7746_VTSETUP_VTEN, 1);
-		cap_setup = chip->cap_setup & ~AD7746_CAPSETUP_CAPEN;
-		idx = FIELD_GET(AD7746_CONF_VTFS_MASK, chip->config);
-		delay = ad7746_cap_filter_rate_table[idx][1];
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	if (chip->cap_setup != cap_setup) {
-		ret = i2c_smbus_write_byte_data(chip->client,
-						AD7746_REG_CAP_SETUP,
-						cap_setup);
-		if (ret < 0)
-			return ret;
-
-		chip->cap_setup = cap_setup;
-	}
-
-	if (chip->vt_setup != vt_setup) {
-		ret = i2c_smbus_write_byte_data(chip->client,
-						AD7746_REG_VT_SETUP,
-						vt_setup);
-		if (ret < 0)
-			return ret;
-
-		chip->vt_setup = vt_setup;
-	}
-
-	return delay;
-}
-
-static inline ssize_t ad7746_start_calib(struct device *dev,
-					 struct device_attribute *attr,
-					 const char *buf,
-					 size_t len,
-					 u8 regval)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7746_chip_info *chip = iio_priv(indio_dev);
-	int ret, timeout = 10;
-	bool doit;
-
-	ret = kstrtobool(buf, &doit);
-	if (ret < 0)
-		return ret;
-
-	if (!doit)
-		return 0;
-
-	mutex_lock(&chip->lock);
-	regval |= chip->config;
-	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
-	if (ret < 0)
-		goto unlock;
-
-	do {
-		msleep(20);
-		ret = i2c_smbus_read_byte_data(chip->client, AD7746_REG_CFG);
-		if (ret < 0)
-			goto unlock;
-
-	} while ((ret == regval) && timeout--);
-
-	mutex_unlock(&chip->lock);
-
-	return len;
-
-unlock:
-	mutex_unlock(&chip->lock);
-	return ret;
-}
-
-static ssize_t ad7746_start_offset_calib(struct device *dev,
-					 struct device_attribute *attr,
-					 const char *buf,
-					 size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	int ret = ad7746_select_channel(indio_dev,
-			      &ad7746_channels[to_iio_dev_attr(attr)->address]);
-	if (ret < 0)
-		return ret;
-
-	return ad7746_start_calib(dev, attr, buf, len,
-				  FIELD_PREP(AD7746_CONF_MODE_MASK,
-					     AD7746_CONF_MODE_OFFS_CAL));
-}
-
-static ssize_t ad7746_start_gain_calib(struct device *dev,
-				       struct device_attribute *attr,
-				       const char *buf,
-				       size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	int ret = ad7746_select_channel(indio_dev,
-			      &ad7746_channels[to_iio_dev_attr(attr)->address]);
-	if (ret < 0)
-		return ret;
-
-	return ad7746_start_calib(dev, attr, buf, len,
-				  FIELD_PREP(AD7746_CONF_MODE_MASK,
-					     AD7746_CONF_MODE_GAIN_CAL));
-}
-
-static IIO_DEVICE_ATTR(in_capacitance0_calibbias_calibration,
-		       0200, NULL, ad7746_start_offset_calib, CIN1);
-static IIO_DEVICE_ATTR(in_capacitance1_calibbias_calibration,
-		       0200, NULL, ad7746_start_offset_calib, CIN2);
-static IIO_DEVICE_ATTR(in_capacitance0_calibscale_calibration,
-		       0200, NULL, ad7746_start_gain_calib, CIN1);
-static IIO_DEVICE_ATTR(in_capacitance1_calibscale_calibration,
-		       0200, NULL, ad7746_start_gain_calib, CIN2);
-static IIO_DEVICE_ATTR(in_voltage0_calibscale_calibration,
-		       0200, NULL, ad7746_start_gain_calib, VIN);
-
-static int ad7746_store_cap_filter_rate_setup(struct ad7746_chip_info *chip,
-					      int val)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(ad7746_cap_filter_rate_table); i++)
-		if (val >= ad7746_cap_filter_rate_table[i][0])
-			break;
-
-	if (i >= ARRAY_SIZE(ad7746_cap_filter_rate_table))
-		i = ARRAY_SIZE(ad7746_cap_filter_rate_table) - 1;
-
-	chip->config &= ~AD7746_CONF_CAPFS_MASK;
-	chip->config |= FIELD_PREP(AD7746_CONF_CAPFS_MASK, i);
-
-	return 0;
-}
-
-static int ad7746_store_vt_filter_rate_setup(struct ad7746_chip_info *chip,
-					     int val)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(ad7746_vt_filter_rate_table); i++)
-		if (val >= ad7746_vt_filter_rate_table[i][0])
-			break;
-
-	if (i >= ARRAY_SIZE(ad7746_vt_filter_rate_table))
-		i = ARRAY_SIZE(ad7746_vt_filter_rate_table) - 1;
-
-	chip->config &= ~AD7746_CONF_VTFS_MASK;
-	chip->config |= FIELD_PREP(AD7746_CONF_VTFS_MASK, i);
-
-	return 0;
-}
-
-static struct attribute *ad7746_attributes[] = {
-	&iio_dev_attr_in_capacitance0_calibbias_calibration.dev_attr.attr,
-	&iio_dev_attr_in_capacitance0_calibscale_calibration.dev_attr.attr,
-	&iio_dev_attr_in_capacitance1_calibscale_calibration.dev_attr.attr,
-	&iio_dev_attr_in_capacitance1_calibbias_calibration.dev_attr.attr,
-	&iio_dev_attr_in_voltage0_calibscale_calibration.dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group ad7746_attribute_group = {
-	.attrs = ad7746_attributes,
-};
-
-static int ad7746_write_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan,
-			    int val,
-			    int val2,
-			    long mask)
-{
-	struct ad7746_chip_info *chip = iio_priv(indio_dev);
-	int ret, reg;
-
-	switch (mask) {
-	case IIO_CHAN_INFO_CALIBSCALE:
-		if (val != 1)
-			return -EINVAL;
-
-		val = (val2 * 1024) / 15625;
-
-		switch (chan->type) {
-		case IIO_CAPACITANCE:
-			reg = AD7746_REG_CAP_GAINH;
-			break;
-		case IIO_VOLTAGE:
-			reg = AD7746_REG_VOLT_GAINH;
-			break;
-		default:
-			return -EINVAL;
-		}
-
-		mutex_lock(&chip->lock);
-		ret = i2c_smbus_write_word_swapped(chip->client, reg, val);
-		mutex_unlock(&chip->lock);
-		if (ret < 0)
-			return ret;
-
-		return 0;
-	case IIO_CHAN_INFO_CALIBBIAS:
-		if (val < 0 || val > 0xFFFF)
-			return -EINVAL;
-
-		mutex_lock(&chip->lock);
-		ret = i2c_smbus_write_word_swapped(chip->client,
-						   AD7746_REG_CAP_OFFH, val);
-		mutex_unlock(&chip->lock);
-		if (ret < 0)
-			return ret;
-
-		return 0;
-	case IIO_CHAN_INFO_OFFSET:
-	case IIO_CHAN_INFO_ZEROPOINT:
-		if (val < 0 || val > 43008000) /* 21pF */
-			return -EINVAL;
-
-		/*
-		 * CAPDAC Scale = 21pF_typ / 127
-		 * CIN Scale = 8.192pF / 2^24
-		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
-		 */
-
-		val /= 338646;
-		mutex_lock(&chip->lock);
-		chip->capdac[chan->channel][chan->differential] = val > 0 ?
-			FIELD_PREP(AD7746_CAPDAC_DACP_MASK, val) | AD7746_CAPDAC_DACEN : 0;
-
-		ret = ad7746_set_capdac(chip, chan->channel);
-		if (ret < 0) {
-			mutex_unlock(&chip->lock);
-			return ret;
-		}
-
-		chip->capdac_set = chan->channel;
-		mutex_unlock(&chip->lock);
-
-		return 0;
-	case IIO_CHAN_INFO_SAMP_FREQ:
-		if (val2)
-			return -EINVAL;
-
-		switch (chan->type) {
-		case IIO_CAPACITANCE:
-			mutex_lock(&chip->lock);
-			ret = ad7746_store_cap_filter_rate_setup(chip, val);
-			mutex_unlock(&chip->lock);
-			return ret;
-		case IIO_VOLTAGE:
-			mutex_lock(&chip->lock);
-			ret = ad7746_store_vt_filter_rate_setup(chip, val);
-			mutex_unlock(&chip->lock);
-			return ret;
-		default:
-			return -EINVAL;
-		}
-	default:
-		return -EINVAL;
-	}
-}
-
-static const int ad7746_v_samp_freq[] = { 50, 31, 16, 8, };
-static const int ad7746_cap_samp_freq[] = { 91, 84, 50, 26, 16, 13, 11, 9, };
-
-static int ad7746_read_avail(struct iio_dev *indio_dev,
-			     struct iio_chan_spec const *chan, const int **vals,
-			     int *type, int *length, long mask)
-{
-	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
-		return -EINVAL;
-
-	switch (chan->type) {
-	case IIO_VOLTAGE:
-		*vals = ad7746_v_samp_freq;
-		*length = ARRAY_SIZE(ad7746_v_samp_freq);
-		break;
-	case IIO_CAPACITANCE:
-		*vals = ad7746_cap_samp_freq;
-		*length = ARRAY_SIZE(ad7746_cap_samp_freq);
-		break;
-	default:
-		return -EINVAL;
-	}
-	*type = IIO_VAL_INT;
-	return IIO_AVAIL_LIST;
-}
-
-static int ad7746_read_channel(struct iio_dev *indio_dev,
-			       struct iio_chan_spec const *chan,
-			       int *val)
-{
-	struct ad7746_chip_info *chip = iio_priv(indio_dev);
-	int ret, delay;
-	u8 data[3];
-	u8 regval;
-
-	ret = ad7746_select_channel(indio_dev, chan);
-	if (ret < 0)
-		return ret;
-	delay = ret;
-
-	regval = chip->config | FIELD_PREP(AD7746_CONF_MODE_MASK,
-					   AD7746_CONF_MODE_SINGLE_CONV);
-	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
-	if (ret < 0)
-		return ret;
-
-	msleep(delay);
-	/* Now read the actual register */
-	ret = i2c_smbus_read_i2c_block_data(chip->client,
-					    ad7746_chan_info[chan->address].addr,
-					    sizeof(data), data);
-	if (ret < 0)
-		return ret;
-
-	/*
-	 * Offset applied internally becaue the _offset userspace interface is
-	 * needed for the CAP DACs which apply a controllable offset.
-	 */
-	*val = get_unaligned_be24(data) - 0x800000;
-
-	return 0;
-}
-
-static int ad7746_read_raw(struct iio_dev *indio_dev,
-			   struct iio_chan_spec const *chan,
-			   int *val, int *val2,
-			   long mask)
-{
-	struct ad7746_chip_info *chip = iio_priv(indio_dev);
-	int ret, idx;
-	u8 reg;
-
-	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
-		mutex_lock(&chip->lock);
-		ret = ad7746_read_channel(indio_dev, chan, val);
-		mutex_unlock(&chip->lock);
-		if (ret < 0)
-			return ret;
-
-		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_CALIBSCALE:
-		switch (chan->type) {
-		case IIO_CAPACITANCE:
-			reg = AD7746_REG_CAP_GAINH;
-			break;
-		case IIO_VOLTAGE:
-			reg = AD7746_REG_VOLT_GAINH;
-			break;
-		default:
-			return -EINVAL;
-		}
-
-		mutex_lock(&chip->lock);
-		ret = i2c_smbus_read_word_swapped(chip->client, reg);
-		mutex_unlock(&chip->lock);
-		if (ret < 0)
-			return ret;
-		/* 1 + gain_val / 2^16 */
-		*val = 1;
-		*val2 = (15625 * ret) / 1024;
-
-		return IIO_VAL_INT_PLUS_MICRO;
-	case IIO_CHAN_INFO_CALIBBIAS:
-		mutex_lock(&chip->lock);
-		ret = i2c_smbus_read_word_swapped(chip->client,
-						  AD7746_REG_CAP_OFFH);
-		mutex_unlock(&chip->lock);
-		if (ret < 0)
-			return ret;
-		*val = ret;
-
-		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_OFFSET:
-	case IIO_CHAN_INFO_ZEROPOINT:
-		*val = FIELD_GET(AD7746_CAPDAC_DACP_MASK,
-				 chip->capdac[chan->channel][chan->differential]) * 338646;
-
-		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_SCALE:
-		switch (chan->type) {
-		case IIO_CAPACITANCE:
-			/* 8.192pf / 2^24 */
-			*val =  0;
-			*val2 = 488;
-			return IIO_VAL_INT_PLUS_NANO;
-		case IIO_VOLTAGE:
-			/* 1170mV / 2^23 */
-			*val = 1170;
-			if (chan->channel == 1)
-				*val *= 6;
-			*val2 = 23;
-			return IIO_VAL_FRACTIONAL_LOG2;
-		case IIO_TEMP:
-			*val = 125;
-			*val2 = 8;
-			return IIO_VAL_FRACTIONAL_LOG2;
-		default:
-			return -EINVAL;
-		}
-	case IIO_CHAN_INFO_SAMP_FREQ:
-		switch (chan->type) {
-		case IIO_CAPACITANCE:
-			idx = FIELD_GET(AD7746_CONF_CAPFS_MASK, chip->config);
-			*val = ad7746_cap_filter_rate_table[idx][0];
-			return IIO_VAL_INT;
-		case IIO_VOLTAGE:
-			idx = FIELD_GET(AD7746_CONF_VTFS_MASK, chip->config);
-			*val = ad7746_vt_filter_rate_table[idx][0];
-			return IIO_VAL_INT;
-		default:
-			return -EINVAL;
-		}
-	default:
-		return -EINVAL;
-	}
-}
-
-static const struct iio_info ad7746_info = {
-	.attrs = &ad7746_attribute_group,
-	.read_raw = ad7746_read_raw,
-	.read_avail = ad7746_read_avail,
-	.write_raw = ad7746_write_raw,
-};
-
-static int ad7746_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
-{
-	struct device *dev = &client->dev;
-	struct ad7746_chip_info *chip;
-	struct iio_dev *indio_dev;
-	unsigned char regval = 0;
-	unsigned int vdd_permille;
-	int ret;
-
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
-	if (!indio_dev)
-		return -ENOMEM;
-
-	chip = iio_priv(indio_dev);
-	mutex_init(&chip->lock);
-
-	chip->client = client;
-	chip->capdac_set = -1;
-
-	indio_dev->name = id->name;
-	indio_dev->info = &ad7746_info;
-	indio_dev->channels = ad7746_channels;
-	if (id->driver_data == 7746)
-		indio_dev->num_channels = ARRAY_SIZE(ad7746_channels);
-	else
-		indio_dev->num_channels =  ARRAY_SIZE(ad7746_channels) - 2;
-	indio_dev->modes = INDIO_DIRECT_MODE;
-
-	if (device_property_read_bool(dev, "adi,exca-output-en")) {
-		if (device_property_read_bool(dev, "adi,exca-output-invert"))
-			regval |= AD7746_EXCSETUP_NEXCA;
-		else
-			regval |= AD7746_EXCSETUP_EXCA;
-	}
-
-	if (device_property_read_bool(dev, "adi,excb-output-en")) {
-		if (device_property_read_bool(dev, "adi,excb-output-invert"))
-			regval |= AD7746_EXCSETUP_NEXCB;
-		else
-			regval |= AD7746_EXCSETUP_EXCB;
-	}
-
-	ret = device_property_read_u32(dev, "adi,excitation-vdd-permille",
-				       &vdd_permille);
-	if (!ret) {
-		switch (vdd_permille) {
-		case 125:
-			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 0);
-			break;
-		case 250:
-			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 1);
-			break;
-		case 375:
-			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 2);
-			break;
-		case 500:
-			regval |= FIELD_PREP(AD7746_EXCSETUP_EXCLVL_MASK, 3);
-			break;
-		default:
-			break;
-		}
-	}
-
-	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_EXC_SETUP,
-					regval);
-	if (ret < 0)
-		return ret;
-
-	return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
-}
-
-static const struct i2c_device_id ad7746_id[] = {
-	{ "ad7745", 7745 },
-	{ "ad7746", 7746 },
-	{ "ad7747", 7747 },
-	{}
-};
-MODULE_DEVICE_TABLE(i2c, ad7746_id);
-
-static const struct of_device_id ad7746_of_match[] = {
-	{ .compatible = "adi,ad7745" },
-	{ .compatible = "adi,ad7746" },
-	{ .compatible = "adi,ad7747" },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, ad7746_of_match);
-
-static struct i2c_driver ad7746_driver = {
-	.driver = {
-		.name = KBUILD_MODNAME,
-		.of_match_table = ad7746_of_match,
-	},
-	.probe = ad7746_probe,
-	.id_table = ad7746_id,
-};
-module_i2c_driver(ad7746_driver);
-
-MODULE_AUTHOR("Michael Hennerich <michael.hennerich@analog.com>");
-MODULE_DESCRIPTION("Analog Devices AD7746/5/7 capacitive sensor driver");
-MODULE_LICENSE("GPL v2");
-- 
2.36.1

[PATCH v3 17/17] RFC: iio: cdc: ad7746: Add roadtest

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

The recent work to cleanup and migrate this driver out of staging
relied on using roadtest to verify that the various refactoring
and fixes did not cause any regressions.

Note I am far from an experienced python coder so a large part of
this exercise was to establish if roadtest was useful / usable
without that particular skill set.  Conclusion yes it is :)

Apologies for the code!

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 .../roadtest/tests/iio/cdc/__init__.py        |   0
 .../roadtest/roadtest/tests/iio/cdc/config    |   1 +
 .../roadtest/tests/iio/cdc/test_ad7746.py     | 323 ++++++++++++++++++
 3 files changed, 324 insertions(+)

diff --git a/tools/testing/roadtest/roadtest/tests/iio/cdc/__init__.py b/tools/testing/roadtest/roadtest/tests/iio/cdc/__init__.py
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tools/testing/roadtest/roadtest/tests/iio/cdc/config b/tools/testing/roadtest/roadtest/tests/iio/cdc/config
new file mode 100644
index 000000000000..6e9397325918
--- /dev/null
+++ b/tools/testing/roadtest/roadtest/tests/iio/cdc/config
@@ -0,0 +1 @@
+CONFIG_AD7746=m
diff --git a/tools/testing/roadtest/roadtest/tests/iio/cdc/test_ad7746.py b/tools/testing/roadtest/roadtest/tests/iio/cdc/test_ad7746.py
new file mode 100644
index 000000000000..60d75c0f4fe8
--- /dev/null
+++ b/tools/testing/roadtest/roadtest/tests/iio/cdc/test_ad7746.py
@@ -0,0 +1,323 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# Based on test_vncl4010 Copyright Axis Communications AB
+# TODO
+# - Test read temperatures
+# - Test read capacitance
+#
+
+from typing import Any, Final
+
+from roadtest.backend.i2c import SMBusModel
+from roadtest.core.devicetree import DtFragment, DtVar
+from roadtest.core.hardware import Hardware
+from roadtest.core.suite import UMLTestCase
+from roadtest.core.modules import insmod, rmmod
+from roadtest.core.sysfs import (
+    I2CDriver,
+    read_float,
+    read_int,
+    read_str,
+    write_int,
+    write_str,
+)
+
+REG_STATUS: Final = 0x00
+REG_CAP_DATA_H: Final = 0x01
+REG_CAP_DATA_M: Final = 0x02
+REG_CAP_DATA_L: Final = 0x03
+REG_VT_DATA_H: Final = 0x04
+REG_VT_DATA_M: Final = 0x05
+REG_VT_DATA_L: Final = 0x06
+REG_CAP_SETUP: Final = 0x07
+REG_VT_SETUP: Final = 0x08
+REG_EXC_SETUP: Final = 0x09
+REG_CONFIG: Final = 0x0a
+REG_CAP_DAC_A: Final = 0x0b
+REG_CAP_DAC_B: Final = 0x0c
+REG_CAP_OFFSET_H: Final = 0x0d
+REG_CAP_OFFSET_L: Final = 0x0e
+REG_CAP_GAIN_H: Final = 0x0f
+REG_CAP_GAIN_L: Final = 0x10
+REG_VOLT_GAIN_H: Final = 0x11
+REG_VOLT_GAIN_L: Final = 0x12
+
+class AD7746(SMBusModel):
+    def __init__(self, **kwargs: Any) -> None:
+        super().__init__(regbytes=1, **kwargs)
+        self.regs = {
+            REG_STATUS: 0x07,
+            REG_CAP_DATA_H: 0x00,
+            REG_CAP_DATA_M: 0x00,
+            REG_CAP_DATA_L: 0x00,
+            REG_VT_DATA_H: 0x00,
+            REG_VT_DATA_M: 0x00,
+            REG_VT_DATA_L: 0x00,
+            REG_CAP_SETUP: 0x00,
+            REG_VT_SETUP: 0x00,
+            REG_EXC_SETUP: 0x03,
+            REG_CONFIG: 0xa0,
+            REG_CAP_DAC_A: 0x00,
+            REG_CAP_DAC_B: 0x00,
+            REG_CAP_OFFSET_H: 0x80,
+            REG_CAP_OFFSET_L: 0x00,
+            REG_CAP_GAIN_H: 0x00,
+            REG_CAP_GAIN_L: 0x00,
+            REG_VOLT_GAIN_H: 0x00,
+            REG_VOLT_GAIN_L: 0x00,
+        }
+
+    def reg_read(self, addr: int) -> int:
+        val = self.regs[addr]
+        return val;
+
+    def reg_write(self, addr: int, val: int) -> None:
+        assert addr in self.regs
+        assert addr not in ( REG_STATUS,
+                             REG_CAP_DATA_H, REG_CAP_DATA_M, REG_CAP_DATA_L,
+                             REG_VT_DATA_H, REG_VT_DATA_M, REG_VT_DATA_L )
+
+        self.regs[addr] = val
+
+    def inject(self, addr: int, val: int, mask : int = ~0) -> None:
+        old = self.regs[addr] & ~mask
+        new = old | (val & mask)
+        self.regs[addr] = new
+
+class TestAD7746(UMLTestCase):
+    dts = DtFragment(
+        src="""
+&i2c {
+    cdc@$addr$ {
+        compatible = "adi,ad7746";
+        reg = <0x$addr$>;
+    };
+};
+        """,
+        variables={
+            "addr": DtVar.I2C_ADDR,
+        },
+    )
+
+    @classmethod
+    def setUpClass(cls) -> None:
+        insmod("ad7746")
+
+    @classmethod
+    def tearDownClass(cls) -> None:
+        rmmod("ad7746")
+
+    def setUp(self) -> None:
+        self.driver = I2CDriver("ad7746")
+        self.hw = Hardware("i2c")
+        self.hw.load_model(AD7746)
+
+    def tearDown(self) -> None:
+        self.hw.close()
+
+    def read_attr(self, dev, type, attr, index, extend_name=None, default_val=0) -> float:
+        name_start = "iio:device0/in_" + type;
+        if extend_name is None:
+            try:
+                scale = read_float(dev.path / (name_start + str(index) + "_" + attr))
+            except:
+                try:
+                    scale = read_float(dev.path / (name_start + "_" + attr))
+                except:
+                    scale = default_val
+        if extend_name is not None:
+            try:
+                scale = read_float(dev.path / (name_start + str(index) + "_" + extend_name + "_" + attr))
+            except:
+                try:
+                    scale = read_float(dev.path / (name_start + str(index) +  "_" + attr))
+                except:
+                    try:
+                        scale = read_float(dev.path / (name_start + "_" + attr))
+                    except:
+                        scale = default_val
+
+        return scale
+
+    def test_scales(self) -> None:
+        with self.driver.bind(self.dts["addr"]) as dev:
+            scale = self.read_attr(dev, "voltage", "scale", 0, default_val=1.0)
+            self.assertAlmostEqual(scale, 1170 / (1 << 23))
+            scale = self.read_attr(dev, "voltage", "scale", 1, "supply", default_val=1.0)
+            self.assertAlmostEqual(scale, 1170 * 6 / ( 1 << 23))
+            scale = self.read_attr(dev, "capacitance", "scale", 0, default_val=1.0)
+            self.assertEqual(scale, 0.000000488)
+
+    def test_read_avail(self) -> None:
+        with self.driver.bind(self.dts["addr"]) as dev:
+            test = read_str(dev.path / "iio:device0/in_capacitance_sampling_frequency_available")
+            assert test == "91 84 50 26 16 13 11 9"
+            test = read_str(dev.path / "iio:device0/in_voltage_sampling_frequency_available")
+            assert test == "50 31 16 8", test
+
+    def test_read_channels(self) -> None:
+        with self.driver.bind(self.dts["addr"]) as dev:
+            testscale = 1170.0 / (1 << 23)
+            # Test endpoints and each of the bytes
+            testvals = (( 0x0, 0x0, 0x0, (-0x800000) * testscale),
+                        ( 0x0, 0x40, 0x0, (-0x800000 + 0x004000) * testscale),
+                        ( 0x80, 0x0, 0x0, (0x0) * testscale),
+                        ( 0x80, 0x0, 0x1, (0x1) * testscale),
+                        ( 0xff, 0xff, 0xff, (0x0 + 0x7fffff) * testscale),
+                        )
+            scale = self.read_attr(dev, "voltage", "scale", 0, default_val=1.0)
+            offset = self.read_attr(dev, "voltage", "offset", 0, default_val=0.0)
+
+            # Should really be testing that the voltage after application of
+            # scale and offset is correct, not raw values.
+            for h, m, l, testval in testvals:
+                # Sequence matters as we will only write registers if the change.
+                # Hence always proceed a vt read with a cap read and visa versa
+                read_int(dev.path / "iio:device0/in_capacitance0_raw")
+                # value is 24 bit and driver offsets it by -0x800000.
+                self.hw.inject(REG_VT_DATA_H, h)
+                self.hw.inject(REG_VT_DATA_M, m)
+                self.hw.inject(REG_VT_DATA_L, l)
+                value = read_int(dev.path / "iio:device0/in_voltage0_raw")
+                self.assertAlmostEqual((value + offset) * scale, testval, 4)
+                mock = self.hw.update_mock()
+                mock.assert_last_reg_write(self, REG_VT_SETUP, 0x80 | 0x60)
+                #verify capacitance read disabled
+                cap_setup = mock.get_last_reg_write(REG_CAP_SETUP)
+                assert(not (cap_setup & 0x80))
+                mock.reset_mock()
+            testscale = 1170 * 6 / (1 << 23)
+            # Test endpoints and each of the bytes
+            testvals = (( 0x0, 0x0, 0x0, (-0x800000) * testscale),
+                        ( 0x0, 0x40, 0x0, (-0x800000 + 0x004000) * testscale),
+                        ( 0x80, 0x0, 0x0, (0x0) * testscale),
+                        ( 0x80, 0x0, 0x1, (0x1) * testscale),
+                        ( 0xff, 0xff, 0xff, (0x0 + 0x7fffff) * testscale),
+                        )
+            scale = self.read_attr(dev, "voltage", "scale", 1, "supply", default_val = 1.0)
+            for h, m, l, testval in testvals:
+                read_int(dev.path / "iio:device0/in_capacitance0_raw")
+                self.hw.inject(REG_VT_DATA_H, h)
+                self.hw.inject(REG_VT_DATA_M, m)
+                self.hw.inject(REG_VT_DATA_L, l)
+                value = read_int(dev.path/ "iio:device0/in_voltage1_supply_raw")
+                # Supply voltage channel is attenuated by x6
+                self.assertAlmostEqual((value + offset) * scale, testval, 4)
+                mock = self.hw.update_mock()
+                mock.assert_last_reg_write(self, REG_VT_SETUP, 0x80 | 0x40)
+                #verify capacitance read disabled
+                cap_setup = mock.get_last_reg_write(REG_CAP_SETUP)
+                assert(not (cap_setup & 0x80))
+                mock.reset_mock()
+
+            # Only bother testing one of the temperature channels. They are very similar.
+            testvals = (( 0x0, 0x0, 0x0, (0/2048.0 - 4096) * 1000),
+                        ( 0x0, 0x40, 0x0, (0x004000/2048.0 - 4096) * 1000),
+                        ( 0x80, 0x0, 0x0, (0x800000/2048.0 - 4096) * 1000),
+                        ( 0x80, 0x0, 0x1, (0x800001/2048.0 - 4096) * 1000),
+                        ( 0xff, 0xff, 0xff, (0xffffff/2048.0 - 4096) * 1000),
+                        )
+            scale = self.read_attr(dev, "temp", "scale", 0, default_val = 1.0)
+            for h, m, l, testval in testvals:
+                read_int(dev.path / "iio:device0/in_capacitance0_raw")
+                self.hw.inject(REG_VT_DATA_H, h)
+                self.hw.inject(REG_VT_DATA_M, m)
+                self.hw.inject(REG_VT_DATA_L, l)
+                value = read_int(dev.path/ "iio:device0/in_temp0_raw")
+                self.assertAlmostEqual(value * scale, testval, 4)
+                mock = self.hw.update_mock()
+                mock.assert_last_reg_write(self, REG_VT_SETUP, 0x80 | 0x00)
+                #verify capacitance read disabled
+                cap_setup = mock.get_last_reg_write(REG_CAP_SETUP)
+                assert(not (cap_setup & 0x80))
+                mock.reset_mock()
+
+            testscale = 0.000000488
+            # Test endpoints and each of the bytes
+            testvals = (( 0x0, 0x0, 0x0, (-0x800000) * testscale),
+                        ( 0x0, 0x40, 0x0, (-0x800000 + 0x004000) * testscale),
+                        ( 0x80, 0x0, 0x0, (0x0) * testscale),
+                        ( 0x80, 0x0, 0x1, (0x1) * testscale),
+                        ( 0xff, 0xff, 0xff, (0x0 + 0x7fffff) * testscale),
+                        )
+
+            scale = self.read_attr(dev, "capacitance", "scale", 0, default_val=1.0)
+            offset = 0.0 #self.read_attr(dev, "capacitance", "offset", 0, default_val=0.0)
+
+            # Should really be testing that the voltage after application of
+            # scale and offset is correct, not raw values.
+            for h, m, l, testval in testvals:
+                # Sequence matters as we will only write registers if the change.
+                # Hence always proceed a vt read with a cap read and visa versa
+                read_int(dev.path / "iio:device0/in_voltage0_raw")
+                # value is 24 bit and driver offsets it by -0x800000.
+                self.hw.inject(REG_CAP_DATA_H, h)
+                self.hw.inject(REG_CAP_DATA_M, m)
+                self.hw.inject(REG_CAP_DATA_L, l)
+                value = read_int(dev.path / "iio:device0/in_capacitance0_raw")
+                self.assertAlmostEqual((value + offset) * scale, testval, 9)
+                mock = self.hw.update_mock()
+                mock.assert_last_reg_write(self, REG_CAP_SETUP, 0x80)
+                #verify capacitance read disabled
+                vt_setup = mock.get_last_reg_write(REG_VT_SETUP)
+                assert(not (vt_setup & 0x80))
+                mock.reset_mock()
+
+            scale = self.read_attr(dev, "capacitance", "scale", 1, default_val=1.0)
+            offset = 0.0 #self.read_attr(dev, "capacitance", "offset", 0, default_val=0.0)
+
+            # Should really be testing that the voltage after application of
+            # scale and offset is correct, not raw values.
+            for h, m, l, testval in testvals:
+                # Sequence matters as we will only write registers if the change.
+                # Hence always proceed a vt read with a cap read and visa versa
+                read_int(dev.path / "iio:device0/in_voltage0_raw")
+                # value is 24 bit and driver offsets it by -0x800000.
+                self.hw.inject(REG_CAP_DATA_H, h)
+                self.hw.inject(REG_CAP_DATA_M, m)
+                self.hw.inject(REG_CAP_DATA_L, l)
+                value = read_int(dev.path / "iio:device0/in_capacitance1_raw")
+                self.assertAlmostEqual((value + offset) * scale, testval, 9)
+                mock = self.hw.update_mock()
+                mock.assert_last_reg_write(self, REG_CAP_SETUP, 0x80 | 0x40)
+                #verify capacitance read disabled
+                vt_setup = mock.get_last_reg_write(REG_VT_SETUP)
+                assert(not (vt_setup & 0x80))
+                mock.reset_mock()
+
+            # Should really be testing that the voltage after application of
+            # scale and offset is correct, not raw values.
+            for h, m, l, testval in testvals:
+                # Sequence matters as we will only write registers if the change.
+                # Hence always proceed a vt read with a cap read and visa versa
+                read_int(dev.path / "iio:device0/in_voltage0_raw")
+                # value is 24 bit and driver offsets it by -0x800000.
+                self.hw.inject(REG_CAP_DATA_H, h)
+                self.hw.inject(REG_CAP_DATA_M, m)
+                self.hw.inject(REG_CAP_DATA_L, l)
+                value = read_int(dev.path / "iio:device0/in_capacitance0-capacitance2_raw")
+                self.assertAlmostEqual((value + offset) * scale, testval, 9)
+                mock = self.hw.update_mock()
+                mock.assert_last_reg_write(self, REG_CAP_SETUP, 0x80 | 0x20)
+                #verify capacitance read disabled
+                vt_setup = mock.get_last_reg_write(REG_VT_SETUP)
+                assert(not (vt_setup & 0x80))
+                mock.reset_mock()
+
+            # Should really be testing that the voltage after application of
+            # scale and offset is correct, not raw values.
+            for h, m, l, testval in testvals:
+                # Sequence matters as we will only write registers if the change.
+                # Hence always proceed a vt read with a cap read and visa versa
+                read_int(dev.path / "iio:device0/in_voltage0_raw")
+                # value is 24 bit and driver offsets it by -0x800000.
+                self.hw.inject(REG_CAP_DATA_H, h)
+                self.hw.inject(REG_CAP_DATA_M, m)
+                self.hw.inject(REG_CAP_DATA_L, l)
+                value = read_int(dev.path / "iio:device0/in_capacitance1-capacitance3_raw")
+                self.assertAlmostEqual((value + offset) * scale, testval, 9)
+                mock = self.hw.update_mock()
+                mock.assert_last_reg_write(self, REG_CAP_SETUP, 0x80 | 0x60)
+                #verify capacitance read disabled
+                vt_setup = mock.get_last_reg_write(REG_VT_SETUP)
+                assert(not (vt_setup & 0x80))
+                mock.reset_mock()
-- 
2.36.1

Re: [PATCH v3 00/17] staging/iio: Clean up AD7746 CDC driver and move from staging.

[Andy Shevchenko]

On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Changes since v2:  Thanks to Andy Shevchenko for review
> - Fix the horrible mess I made of a rebase that meant v2 didn't build
>   mid patch set.
> - Fix various build issues I'd missed due to wrong make file (fails to
>   update variables after moving them, missing unaligned include,
>   failure to cleanup staging driver Kconfig / Makefile)
> - Change new ABI to _zeropoint and add more the description to hopefully
>   add clarify to what this is. I don't think it's going to be a particularly
>   common bit of ABI, but still good to get it right.


For non-commented patches (except 17th):
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
For commented, in case you address the comments I suggested, you may
also add a tag.

> Note that the --no-renames was used to preserve full code listing
> for the actual move out of staging to aid review. The ad7746 file is
> moved with no changes.
>
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c
> similarity index 100%
> rename from drivers/staging/iio/cdc/ad7746.c
> rename to drivers/iio/cdc/ad7746.c
>
> This started out as an experiment with Vincent Whitchurch's roadtest
> testing framework [1] and that worked well so I carried on cleaning up the
> driver.  Note that if you are using roadtest rebased to current
> kernel, add CONFIG_UML_RANDOM as suggested by Vincent in reply
> to v2 of this series.  I still saw one freeze but with that config
> element added roadtest runs the tests successfully the vast majority
> of the time.
>
> Mostly this is standard tidy up, move to new interfaces, then move the driver
> out of staging, but there are a few other things in here.
>
> 1) Precision improvement for IIO_VAL_FRACTIONAL_LOG2.
>    The ad7746 is a 24 bit sensor and this highlighted that 9 decimal
>    places wasn't enough to keep the error introduced by
>    _raw * _scale from growing too large over the whole range (I couldn't
>    write a sensible test for it).  So I've proposed increasing the precision
>    of the calculation used to provide the sysfs attributes with this value
>    type and printing a few more decimal places (12), but only if overflow
>    will not occur due to val[0] > ULLONG_MAX / PICO
> 2) _zeropoint ABI addition.  This driver had an odd use of _offset for
>    the case of differential channels that applied the offset to both
>    of the differential pair (hence userspace shouldn't not apply it when
>    converting to the base units. That isn't inline with the existing
>    documentation for _offset and it wasn't clear to me that it made sense
>    at all.  To avoid confusion I've added this new ABI (_zeropoint) for this.
> 3) roadtest file - note this is not a complete test set for the driver and
>    mainly focused on the main channel reads and places I thought I might
>    have broken things whilst working on the driver.  Given roadtest isn't
>    upstream yet this test will have to wait. That doesn't block merging
>    the rest of the series.
>
> My conclusion on roadtest - Very useful indeed. I'd encourage others to
> consider developing some basic sanity tests for drivers they are working on.
> Hopefully my python code isn't too hideous to understand at least!
> Vincent, it might be worth thinking about some generic code to handle the
> 'variants' on correct ABI like I introduce here because I switched from
> a shared by type scale to an individual one per channel for the voltages.
> Both were ABI compliant so that sort of change is fine most of the time
> though we have to be careful with it.
>
> All comments welcome.  Note there may be changes that make more sense
> to do after moving this out of staging as long as there are no ABI changes involved
> etc.  Feel free to highlight those sorts of changes as well as anything more
> significant.
>
> [1] https://lore.kernel.org/all/20220311162445.346685-9-vincent.whitchurch@axis.com/
>
> Jonathan Cameron (17):
>   iio: core: Increase precision of IIO_VAL_FRACTIONAL_LOG2 when possible
>   iio: ABI: Fix wrong format of differential capacitance channel ABI.
>   staging: iio: cdc: ad7746: Use explicit be24 handling.
>   staging: iio: cdc: ad7746: Push handling of supply voltage scale to
>     userspace.
>   staging: iio: cdc: ad7746: Use local buffer for multi byte reads.
>   staging: iio: cdc: ad7746: Factor out ad7746_read_channel()
>   staging: iio: cdc: ad7764: Push locking down into case statements in
>     read/write_raw
>   staging: iio: cdc: ad7746: Break up use of chan->address and use
>     FIELD_PREP etc
>   staging: iio: cdc: ad7746: Drop unused i2c_set_clientdata()
>   staging: iio: cdc: ad7746: Use _raw and _scale for temperature
>     channels.
>   iio: core: Introduce _zeropoint for differential channels
>   staging: iio: cdc: ad7746: Switch from _offset to _zeropoint for
>     differential channels.
>   staging: iio: cdc: ad7746: Use read_avail() rather than opencoding.
>   staging: iio: ad7746: White space cleanup
>   iio: cdc: ad7746: Add device specific ABI documentation.
>   iio: cdc: ad7746: Move driver out of staging.
>   RFC: iio: cdc: ad7746: Add roadtest
>
>  Documentation/ABI/testing/sysfs-bus-iio       |  21 +-
>  .../ABI/testing/sysfs-bus-iio-cdc-ad7746      |  11 +
>  drivers/iio/cdc/Kconfig                       |  10 +
>  drivers/iio/cdc/Makefile                      |   1 +
>  drivers/iio/cdc/ad7746.c                      | 820 ++++++++++++++++++
>  drivers/iio/industrialio-core.c               |  32 +-
>  drivers/staging/iio/Kconfig                   |   1 -
>  drivers/staging/iio/Makefile                  |   1 -
>  drivers/staging/iio/cdc/Kconfig               |  17 -
>  drivers/staging/iio/cdc/Makefile              |   6 -
>  drivers/staging/iio/cdc/ad7746.c              | 767 ----------------
>  include/linux/iio/types.h                     |   1 +
>  .../roadtest/tests/iio/cdc/__init__.py        |   0
>  .../roadtest/roadtest/tests/iio/cdc/config    |   1 +
>  .../roadtest/tests/iio/cdc/test_ad7746.py     | 323 +++++++
>  15 files changed, 1211 insertions(+), 801 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cdc-ad7746
>  create mode 100644 drivers/iio/cdc/ad7746.c
>  delete mode 100644 drivers/staging/iio/cdc/Kconfig
>  delete mode 100644 drivers/staging/iio/cdc/Makefile
>  delete mode 100644 drivers/staging/iio/cdc/ad7746.c
>  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/__init__.py
>  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/config
>  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/test_ad7746.py
>
> --
> 2.36.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 00/17] staging/iio: Clean up AD7746 CDC driver and move from staging.

[Jonathan Cameron]

On Tue, 28 Jun 2022 14:21:06 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Changes since v2:  Thanks to Andy Shevchenko for review
> > - Fix the horrible mess I made of a rebase that meant v2 didn't build
> >   mid patch set.
> > - Fix various build issues I'd missed due to wrong make file (fails to
> >   update variables after moving them, missing unaligned include,
> >   failure to cleanup staging driver Kconfig / Makefile)
> > - Change new ABI to _zeropoint and add more the description to hopefully
> >   add clarify to what this is. I don't think it's going to be a particularly
> >   common bit of ABI, but still good to get it right.  
> 
> 
> For non-commented patches (except 17th):
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> For commented, in case you address the comments I suggested, you may
> also add a tag.

Thanks Andy!

Applied, patches 2-16 with suggested modifications (except the one where
I disagreed on make vs makes). 

I'll pick up patch 1 in a few weeks if no feedback to suggest otherwise.
Patch 17 needs the roadtest framework to go upstream.

Given how useful roadtest was I'm hopeful that will progress!

Thanks,

Jonathan

> 
> > Note that the --no-renames was used to preserve full code listing
> > for the actual move out of staging to aid review. The ad7746 file is
> > moved with no changes.
> >
> > diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c
> > similarity index 100%
> > rename from drivers/staging/iio/cdc/ad7746.c
> > rename to drivers/iio/cdc/ad7746.c
> >
> > This started out as an experiment with Vincent Whitchurch's roadtest
> > testing framework [1] and that worked well so I carried on cleaning up the
> > driver.  Note that if you are using roadtest rebased to current
> > kernel, add CONFIG_UML_RANDOM as suggested by Vincent in reply
> > to v2 of this series.  I still saw one freeze but with that config
> > element added roadtest runs the tests successfully the vast majority
> > of the time.
> >
> > Mostly this is standard tidy up, move to new interfaces, then move the driver
> > out of staging, but there are a few other things in here.
> >
> > 1) Precision improvement for IIO_VAL_FRACTIONAL_LOG2.
> >    The ad7746 is a 24 bit sensor and this highlighted that 9 decimal
> >    places wasn't enough to keep the error introduced by
> >    _raw * _scale from growing too large over the whole range (I couldn't
> >    write a sensible test for it).  So I've proposed increasing the precision
> >    of the calculation used to provide the sysfs attributes with this value
> >    type and printing a few more decimal places (12), but only if overflow
> >    will not occur due to val[0] > ULLONG_MAX / PICO
> > 2) _zeropoint ABI addition.  This driver had an odd use of _offset for
> >    the case of differential channels that applied the offset to both
> >    of the differential pair (hence userspace shouldn't not apply it when
> >    converting to the base units. That isn't inline with the existing
> >    documentation for _offset and it wasn't clear to me that it made sense
> >    at all.  To avoid confusion I've added this new ABI (_zeropoint) for this.
> > 3) roadtest file - note this is not a complete test set for the driver and
> >    mainly focused on the main channel reads and places I thought I might
> >    have broken things whilst working on the driver.  Given roadtest isn't
> >    upstream yet this test will have to wait. That doesn't block merging
> >    the rest of the series.
> >
> > My conclusion on roadtest - Very useful indeed. I'd encourage others to
> > consider developing some basic sanity tests for drivers they are working on.
> > Hopefully my python code isn't too hideous to understand at least!
> > Vincent, it might be worth thinking about some generic code to handle the
> > 'variants' on correct ABI like I introduce here because I switched from
> > a shared by type scale to an individual one per channel for the voltages.
> > Both were ABI compliant so that sort of change is fine most of the time
> > though we have to be careful with it.
> >
> > All comments welcome.  Note there may be changes that make more sense
> > to do after moving this out of staging as long as there are no ABI changes involved
> > etc.  Feel free to highlight those sorts of changes as well as anything more
> > significant.
> >
> > [1] https://lore.kernel.org/all/20220311162445.346685-9-vincent.whitchurch@axis.com/
> >
> > Jonathan Cameron (17):
> >   iio: core: Increase precision of IIO_VAL_FRACTIONAL_LOG2 when possible
> >   iio: ABI: Fix wrong format of differential capacitance channel ABI.
> >   staging: iio: cdc: ad7746: Use explicit be24 handling.
> >   staging: iio: cdc: ad7746: Push handling of supply voltage scale to
> >     userspace.
> >   staging: iio: cdc: ad7746: Use local buffer for multi byte reads.
> >   staging: iio: cdc: ad7746: Factor out ad7746_read_channel()
> >   staging: iio: cdc: ad7764: Push locking down into case statements in
> >     read/write_raw
> >   staging: iio: cdc: ad7746: Break up use of chan->address and use
> >     FIELD_PREP etc
> >   staging: iio: cdc: ad7746: Drop unused i2c_set_clientdata()
> >   staging: iio: cdc: ad7746: Use _raw and _scale for temperature
> >     channels.
> >   iio: core: Introduce _zeropoint for differential channels
> >   staging: iio: cdc: ad7746: Switch from _offset to _zeropoint for
> >     differential channels.
> >   staging: iio: cdc: ad7746: Use read_avail() rather than opencoding.
> >   staging: iio: ad7746: White space cleanup
> >   iio: cdc: ad7746: Add device specific ABI documentation.
> >   iio: cdc: ad7746: Move driver out of staging.
> >   RFC: iio: cdc: ad7746: Add roadtest
> >
> >  Documentation/ABI/testing/sysfs-bus-iio       |  21 +-
> >  .../ABI/testing/sysfs-bus-iio-cdc-ad7746      |  11 +
> >  drivers/iio/cdc/Kconfig                       |  10 +
> >  drivers/iio/cdc/Makefile                      |   1 +
> >  drivers/iio/cdc/ad7746.c                      | 820 ++++++++++++++++++
> >  drivers/iio/industrialio-core.c               |  32 +-
> >  drivers/staging/iio/Kconfig                   |   1 -
> >  drivers/staging/iio/Makefile                  |   1 -
> >  drivers/staging/iio/cdc/Kconfig               |  17 -
> >  drivers/staging/iio/cdc/Makefile              |   6 -
> >  drivers/staging/iio/cdc/ad7746.c              | 767 ----------------
> >  include/linux/iio/types.h                     |   1 +
> >  .../roadtest/tests/iio/cdc/__init__.py        |   0
> >  .../roadtest/roadtest/tests/iio/cdc/config    |   1 +
> >  .../roadtest/tests/iio/cdc/test_ad7746.py     | 323 +++++++
> >  15 files changed, 1211 insertions(+), 801 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cdc-ad7746
> >  create mode 100644 drivers/iio/cdc/ad7746.c
> >  delete mode 100644 drivers/staging/iio/cdc/Kconfig
> >  delete mode 100644 drivers/staging/iio/cdc/Makefile
> >  delete mode 100644 drivers/staging/iio/cdc/ad7746.c
> >  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/__init__.py
> >  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/config
> >  create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/test_ad7746.py
> >
> > --
> > 2.36.1
> >  
> 
>