[PATCH v2 00/14] Move ad7746 driver out of staging

[PATCH v2 00/14] Move ad7746 driver out of staging

[Hernán Gonzalez]

Version 2 of the series trying to move ad7746 our of staging.

Changes in v2: (v1-> https://lkml.org/lkml/2018/3/21/406)
* Fix some issues pointed out by Jonathan
* Power down device on remove
* Add new ABI for the use case

Hernán Gonzalez (14):
  staging: iio: ad7746: Automatically swap values in readings/writings
  staging: iio: ad7746: Adjust arguments to match open parenthesis
  staging: iio: ad7746: Fix bound checkings
  staging: iio: ad7746: Fix multiple line dereference
  staging: iio: ad7746: Reorder includes alphabetically
  staging: iio: ad7746: Reorder variable declarations
  staging: iio: ad7746: Remove unused defines
  staging: iio: ad7746: Add dt-bindings
  staging: iio: ad7746: Add remove()
  staging: iio: ad7746: Add comments
  staging: iio: ad7746: Add devicetree bindings documentation
  staging: iio: ad7746: Add ABI documentation
  Move ad7746 out of staging
  staging: iio: Remove ad7746 from staging

 Documentation/ABI/testing/sysfs-bus-iio-ad7746     |  17 +++
 .../devicetree/bindings/iio/cdc/ad7746.txt         |  34 +++++
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/cdc/Kconfig                            |  16 ++
 drivers/iio/cdc/Makefile                           |   5 +
 drivers/{staging => }/iio/cdc/ad7746.c             | 162 ++++++++++++++++-----
 drivers/staging/iio/cdc/Kconfig                    |  10 --
 drivers/staging/iio/cdc/Makefile                   |   1 -
 .../staging => include/linux}/iio/cdc/ad7746.h     |   9 --
 10 files changed, 201 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-ad7746
 create mode 100644 Documentation/devicetree/bindings/iio/cdc/ad7746.txt
 create mode 100644 drivers/iio/cdc/Kconfig
 create mode 100644 drivers/iio/cdc/Makefile
 rename drivers/{staging => }/iio/cdc/ad7746.c (85%)
 rename {drivers/staging => include/linux}/iio/cdc/ad7746.h (70%)

-- 
2.7.4

[PATCH v2 01/14] staging: iio: ad7746: Automatically swap values in readings/writings

[Hernán Gonzalez]

Data to read or write was being handled with the swab16() macro instead
of using i2c_smbus_{read,write}_swapped.

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 4882dbc..53e28ae 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -451,7 +451,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 			goto out;
 		}
 
-		ret = i2c_smbus_write_word_data(chip->client, reg, swab16(val));
+		ret = i2c_smbus_write_word_swapped(chip->client, reg, val);
 		if (ret < 0)
 			goto out;
 
@@ -462,8 +462,8 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 			ret = -EINVAL;
 			goto out;
 		}
-		ret = i2c_smbus_write_word_data(chip->client,
-				AD7746_REG_CAP_OFFH, swab16(val));
+		ret = i2c_smbus_write_word_swapped(chip->client,
+						   AD7746_REG_CAP_OFFH, val);
 		if (ret < 0)
 			goto out;
 
@@ -594,21 +594,21 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 			goto out;
 		}
 
-		ret = i2c_smbus_read_word_data(chip->client, reg);
+		ret = i2c_smbus_read_word_swapped(chip->client, reg);
 		if (ret < 0)
 			goto out;
 		/* 1 + gain_val / 2^16 */
 		*val = 1;
-		*val2 = (15625 * swab16(ret)) / 1024;
+		*val2 = (15625 * ret) / 1024;
 
 		ret = IIO_VAL_INT_PLUS_MICRO;
 		break;
 	case IIO_CHAN_INFO_CALIBBIAS:
-		ret = i2c_smbus_read_word_data(chip->client,
-					       AD7746_REG_CAP_OFFH);
+		ret = i2c_smbus_read_word_swapped(chip->client,
+						  AD7746_REG_CAP_OFFH);
 		if (ret < 0)
 			goto out;
-		*val = swab16(ret);
+		*val = ret;
 
 		ret = IIO_VAL_INT;
 		break;
-- 
2.7.4

[PATCH v2 02/14] staging: iio: ad7746: Adjust arguments to match open parenthesis

[Hernán Gonzalez]

Clear a couple more checkpatch.pl CHECKS.

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 53e28ae..516aa93 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -556,7 +556,8 @@ 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, 3,
+						    &chip->data.d8[1]);
 
 		if (ret < 0)
 			goto out;
@@ -614,7 +615,7 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 		break;
 	case IIO_CHAN_INFO_OFFSET:
 		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
-			[chan->differential]) * 338646;
+					  [chan->differential]) * 338646;
 
 		ret = IIO_VAL_INT;
 		break;
-- 
2.7.4

[PATCH v2 03/14] staging: iio: ad7746: Fix bound checkings

[Hernán Gonzalez]

Also remove unnecessary parenthesis

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 516aa93..d793785 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -458,7 +458,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 		ret = 0;
 		break;
 	case IIO_CHAN_INFO_CALIBBIAS:
-		if ((val < 0) | (val > 0xFFFF)) {
+		if (val < 0 || val > 0xFFFF) {
 			ret = -EINVAL;
 			goto out;
 		}
@@ -470,7 +470,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 		ret = 0;
 		break;
 	case IIO_CHAN_INFO_OFFSET:
-		if ((val < 0) | (val > 43008000)) { /* 21pF */
+		if (val < 0 || val > 43008000) { /* 21pF */
 			ret = -EINVAL;
 			goto out;
 		}
-- 
2.7.4

[PATCH v2 04/14] staging: iio: ad7746: Fix multiple line dereference

[Hernán Gonzalez]

Clear checkpatch.pl WARNING about multiple line derefence but creates a
new one of line over 80 characters. In my opinion, it improves
readability.

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index d793785..82fac76 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -410,8 +410,7 @@ static struct attribute *ad7746_attributes[] = {
 	&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,
+	&iio_const_attr_in_capacitance_sampling_frequency_available.dev_attr.attr,
 	NULL,
 };
 
-- 
2.7.4

[PATCH v2 05/14] staging: iio: ad7746: Reorder includes alphabetically

[Hernán Gonzalez]

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 82fac76..9ef476a 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -6,15 +6,15 @@
  * Licensed under the GPL-2.
  */
 
-#include <linux/interrupt.h>
+#include <linux/delay.h>
 #include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/sysfs.h>
 #include <linux/i2c.h>
-#include <linux/delay.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 <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
-- 
2.7.4

[PATCH v2 06/14] staging: iio: ad7746: Reorder variable declarations

[Hernán Gonzalez]

Reorder some variable declarations in an inverse-pyramid scheme.

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 9ef476a..f53612a 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -220,8 +220,8 @@ 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);
-	int ret, delay, idx;
 	u8 vt_setup, cap_setup;
+	int ret, delay, idx;
 
 	switch (chan->type) {
 	case IIO_CAPACITANCE:
@@ -289,8 +289,8 @@ static inline ssize_t ad7746_start_calib(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad7746_chip_info *chip = iio_priv(indio_dev);
-	bool doit;
 	int ret, timeout = 10;
+	bool doit;
 
 	ret = strtobool(buf, &doit);
 	if (ret < 0)
@@ -680,8 +680,8 @@ static int ad7746_probe(struct i2c_client *client,
 	struct ad7746_platform_data *pdata = client->dev.platform_data;
 	struct ad7746_chip_info *chip;
 	struct iio_dev *indio_dev;
-	int ret = 0;
 	unsigned char regval = 0;
+	int ret = 0;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (!indio_dev)
-- 
2.7.4

[PATCH v2 07/14] staging: iio: ad7746: Remove unused defines

[Hernán Gonzalez]

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 7 -------
 drivers/staging/iio/cdc/ad7746.h | 5 -----
 2 files changed, 12 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index f53612a..d39ab34 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -25,7 +25,6 @@
  * 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
@@ -38,12 +37,6 @@
 #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 */
diff --git a/drivers/staging/iio/cdc/ad7746.h b/drivers/staging/iio/cdc/ad7746.h
index ea8572d..2fbcee8 100644
--- a/drivers/staging/iio/cdc/ad7746.h
+++ b/drivers/staging/iio/cdc/ad7746.h
@@ -13,11 +13,6 @@
  * TODO: struct ad7746_platform_data needs to go into include/linux/iio
  */
 
-#define AD7466_EXCLVL_0		0 /* +-VDD/8 */
-#define AD7466_EXCLVL_1		1 /* +-VDD/4 */
-#define AD7466_EXCLVL_2		2 /* +-VDD * 3/8 */
-#define AD7466_EXCLVL_3		3 /* +-VDD/2 */
-
 struct ad7746_platform_data {
 	unsigned char exclvl;	/*Excitation Voltage Level */
 	bool exca_en;		/* enables EXCA pin as the excitation output */
-- 
2.7.4

[PATCH v2 08/14] staging: iio: ad7746: Add dt-bindings

[Hernán Gonzalez]

This patch adds dt bindings by populating a pdata struct in order to
modify as little as possible the existing code. It supports both
platform_data and dt-bindings but uses only one depending on
CONFIG_OF's value.

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 54 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index d39ab34..c29a221 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -666,6 +666,43 @@ static const struct iio_info ad7746_info = {
 /*
  * device probe and remove
  */
+#ifdef CONFIG_OF
+static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct ad7746_platform_data *pdata;
+	unsigned int tmp;
+	int ret;
+
+	/* The default excitation outputs are not inverted, it should be stated
+	 * in the dt if needed.
+	 */
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	ret = of_property_read_u32(np, "adi,exclvl", &tmp);
+	if (ret || tmp > 3) {
+		dev_warn(dev, "Wrong exclvl value, using default\n");
+		pdata->exclvl = 3; /* default value */
+	} else {
+		pdata->exclvl = tmp;
+	}
+
+	pdata->exca_en = true;
+	pdata->excb_en = true;
+	pdata->exca_inv_en = of_property_read_bool(np, "adi,nexca_en");
+	pdata->excb_inv_en = of_property_read_bool(np, "adi,nexcb_en");
+
+	return pdata;
+}
+#else
+static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
+{
+	return NULL;
+}
+#endif
 
 static int ad7746_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
@@ -676,6 +713,11 @@ static int ad7746_probe(struct i2c_client *client,
 	unsigned char regval = 0;
 	int ret = 0;
 
+	if (client->dev.of_node)
+		pdata = ad7746_parse_dt(&client->dev);
+	else
+		pdata = client->dev.platform_data;
+
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -739,12 +781,22 @@ static const struct i2c_device_id ad7746_id[] = {
 	{ "ad7747", 7747 },
 	{}
 };
-
 MODULE_DEVICE_TABLE(i2c, ad7746_id);
 
+#ifdef CONFIG_OF
+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);
+#endif
+
 static struct i2c_driver ad7746_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
+		.of_match_table = of_match_ptr(ad7746_of_match),
 	},
 	.probe = ad7746_probe,
 	.id_table = ad7746_id,
-- 
2.7.4

[PATCH v2 09/14] staging: iio: ad7746: Add remove()

[Hernán Gonzalez]

This allows the driver to be probed and removed as a module powering it
down on remove().

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index c29a221..05506bf9 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -775,6 +775,31 @@ static int ad7746_probe(struct i2c_client *client,
 	return 0;
 }
 
+static int ad7746_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct ad7746_chip_info *chip = iio_priv(indio_dev);
+	unsigned char regval;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	regval = chip->config | AD7746_CONF_MODE_PWRDN;
+	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
+
+	mutex_unlock(&chip->lock);
+
+	if (ret < 0) {
+		dev_warn(&client->dev, "Could NOT Power Down!\n");
+		goto out;
+	}
+
+	iio_device_unregister(indio_dev);
+
+out:
+	return ret;
+}
+
 static const struct i2c_device_id ad7746_id[] = {
 	{ "ad7745", 7745 },
 	{ "ad7746", 7746 },
@@ -799,6 +824,7 @@ static struct i2c_driver ad7746_driver = {
 		.of_match_table = of_match_ptr(ad7746_of_match),
 	},
 	.probe = ad7746_probe,
+	.remove = ad7746_remove,
 	.id_table = ad7746_id,
 };
 module_i2c_driver(ad7746_driver);
-- 
2.7.4

[PATCH v2 10/14] staging: iio: ad7746: Add comments

[Hernán Gonzalez]

Add comments to clarify some of the calculations made, specially when
reading or writing values.

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 drivers/staging/iio/cdc/ad7746.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 05506bf9..ef0ebb5 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -429,6 +429,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
 			goto out;
 		}
 
+		/* 2^16 in micro */
 		val = (val2 * 1024) / 15625;
 
 		switch (chan->type) {
@@ -554,6 +555,13 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 		if (ret < 0)
 			goto out;
 
+		/*
+		 * Either for Capacitance, Voltage or Temperature,
+		 * the 0x000000 code represents negative full scale,
+		 * the 0x800000 code represents zero scale, and
+		 * the 0xFFFFFF code represents positive full scale.
+		 */
+
 		*val = (be32_to_cpu(chip->data.d32) & 0xFFFFFF) - 0x800000;
 
 		switch (chan->type) {
@@ -565,7 +573,13 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 			*val = (*val * 125) / 256;
 			break;
 		case IIO_VOLTAGE:
-			if (chan->channel == 1) /* supply_raw*/
+
+			/*
+			 * The voltage from the VDD pin is internally
+			 * attenuated by 6.
+			 */
+
+			if (chan->channel == 1) /* supply_raw */
 				*val = *val * 6;
 			break;
 		default:
@@ -606,6 +620,13 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 		ret = IIO_VAL_INT;
 		break;
 	case IIO_CHAN_INFO_OFFSET:
+
+		/*
+		 * CAPDAC Scale = 21pF_typ / 127
+		 * CIN Scale = 8.192pF / 2^24
+		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
+		 */
+
 		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
 					  [chan->differential]) * 338646;
 
@@ -614,13 +635,13 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_CAPACITANCE:
-			/* 8.192pf / 2^24 */
+			/* CIN Scale: 8.192pf / 2^24 */
 			*val =  0;
 			*val2 = 488;
 			ret = IIO_VAL_INT_PLUS_NANO;
 			break;
 		case IIO_VOLTAGE:
-			/* 1170mV / 2^23 */
+			/* VIN Scale: 1170mV / 2^23 */
 			*val = 1170;
 			*val2 = 23;
 			ret = IIO_VAL_FRACTIONAL_LOG2;
@@ -674,7 +695,8 @@ static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
 	unsigned int tmp;
 	int ret;
 
-	/* The default excitation outputs are not inverted, it should be stated
+	/*
+	 * The default excitation outputs are not inverted, it should be stated
 	 * in the dt if needed.
 	 */
 
@@ -685,7 +707,7 @@ static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
 	ret = of_property_read_u32(np, "adi,exclvl", &tmp);
 	if (ret || tmp > 3) {
 		dev_warn(dev, "Wrong exclvl value, using default\n");
-		pdata->exclvl = 3; /* default value */
+		pdata->exclvl = 3;
 	} else {
 		pdata->exclvl = tmp;
 	}
-- 
2.7.4

[PATCH v2 11/14] staging: iio: ad7746: Add devicetree bindings documentation

[Hernán Gonzalez]

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 .../devicetree/bindings/staging/iio/cdc/ad7746.txt | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt

diff --git a/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
new file mode 100644
index 0000000..7740f05
--- /dev/null
+++ b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
@@ -0,0 +1,34 @@
+Analog Devices AD7746/5/7 capacitive sensor driver
+
+Required properties:
+	- compatible: Should be one of
+		* "adi,ad7745"
+		* "adi,ad7746"
+	  * "adi,ad7747"
+	- reg: The 7-bits long I2c address of the device
+
+Optional properties:
+	- adi,exclvl: This property defines the excitation voltage level for the
+    capacitance to be measured. Possible values are:
+      * 0 = +-VDD/8
+      * 1 = +-VDD/4
+      * 2 = +-VDD * 3/8
+      * 3 = +-VDD/2 (Default)
+	- adi,nexca_en: Invert excitation output A.
+	- adi,nexcb_en:	Invert excitation output B.
+
+Example:
+Here exclvl would be 1 (VDD/4), Excitation pin A would be inverted and
+Excitation pin B would NOT be inverted.
+
+i2c2 {
+
+      < . . . >
+
+      ad7746: ad7746@60 {
+              compatible = "ad7746";
+              reg = <0x60>;
+              adi,exclvl = <1>;
+              adi,nexca_en;
+      };
+};
-- 
2.7.4

[PATCH v2 12/14] staging: iio: ad7746: Add ABI documentation

[Hernán Gonzalez]

The use cases for this driver don't comply with the current ABI. The
ad7746 and ad7152 need an external capacitance or voltage reference to
automatically calibrate themselves which is not the normal use case of
the calibscale and calibbias ABIs, a new ABI was needed.

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 Documentation/ABI/testing/sysfs-bus-iio-ad7746 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-ad7746

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-ad7746 b/Documentation/ABI/testing/sysfs-bus-iio-ad7746
new file mode 100644
index 0000000..96a41b7
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-ad7746
@@ -0,0 +1,17 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_capacitanceX_calibscale_calibration
+What:		/sys/bus/iio/devices/iio:deviceX/in_voltage0_calibscale_calibration
+KernelVersion:	4.17.0
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Enter gain calibration mode, in which a full-scale
+		{capacitance, voltage reference} must be connected to the
+		{capacitance, voltage} input beforehand to automatically
+		calibrate the device.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_capacitanceX_calibbias_calibration
+KernelVersion:	4.17.0
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Enter offset calibration mode, in which a zero-scale
+		capacitance must be connected to the capacitance input
+		beforehand to automatically calibrate the device.
-- 
2.7.4

[PATCH v2 13/14] Move ad7746 out of staging

[Hernán Gonzalez]

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 .../devicetree/bindings/iio/cdc/ad7746.txt         |  34 +
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/cdc/Kconfig                            |  16 +
 drivers/iio/cdc/Makefile                           |   5 +
 drivers/iio/cdc/ad7746.c                           | 855 +++++++++++++++++++++
 include/linux/iio/cdc/ad7746.h                     |  20 +
 7 files changed, 932 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/cdc/ad7746.txt
 create mode 100644 drivers/iio/cdc/Kconfig
 create mode 100644 drivers/iio/cdc/Makefile
 create mode 100644 drivers/iio/cdc/ad7746.c
 create mode 100644 include/linux/iio/cdc/ad7746.h

diff --git a/Documentation/devicetree/bindings/iio/cdc/ad7746.txt b/Documentation/devicetree/bindings/iio/cdc/ad7746.txt
new file mode 100644
index 0000000..7740f05
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/cdc/ad7746.txt
@@ -0,0 +1,34 @@
+Analog Devices AD7746/5/7 capacitive sensor driver
+
+Required properties:
+	- compatible: Should be one of
+		* "adi,ad7745"
+		* "adi,ad7746"
+	  * "adi,ad7747"
+	- reg: The 7-bits long I2c address of the device
+
+Optional properties:
+	- adi,exclvl: This property defines the excitation voltage level for the
+    capacitance to be measured. Possible values are:
+      * 0 = +-VDD/8
+      * 1 = +-VDD/4
+      * 2 = +-VDD * 3/8
+      * 3 = +-VDD/2 (Default)
+	- adi,nexca_en: Invert excitation output A.
+	- adi,nexcb_en:	Invert excitation output B.
+
+Example:
+Here exclvl would be 1 (VDD/4), Excitation pin A would be inverted and
+Excitation pin B would NOT be inverted.
+
+i2c2 {
+
+      < . . . >
+
+      ad7746: ad7746@60 {
+              compatible = "ad7746";
+              reg = <0x60>;
+              adi,exclvl = <1>;
+              adi,nexca_en;
+      };
+};
diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index b3c8c6e..d1c309b 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -71,6 +71,7 @@ config IIO_TRIGGERED_EVENT
 source "drivers/iio/accel/Kconfig"
 source "drivers/iio/adc/Kconfig"
 source "drivers/iio/amplifiers/Kconfig"
+source "drivers/iio/cdc/Kconfig"
 source "drivers/iio/chemical/Kconfig"
 source "drivers/iio/common/Kconfig"
 source "drivers/iio/counter/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index b16b2e9..18bea8d 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -17,6 +17,7 @@ obj-y += accel/
 obj-y += adc/
 obj-y += amplifiers/
 obj-y += buffer/
+obj-y += cdc/
 obj-y += chemical/
 obj-y += common/
 obj-y += counter/
diff --git a/drivers/iio/cdc/Kconfig b/drivers/iio/cdc/Kconfig
new file mode 100644
index 0000000..d3a8600
--- /dev/null
+++ b/drivers/iio/cdc/Kconfig
@@ -0,0 +1,16 @@
+#
+# 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/iio/cdc/Makefile b/drivers/iio/cdc/Makefile
new file mode 100644
index 0000000..1f71283
--- /dev/null
+++ b/drivers/iio/cdc/Makefile
@@ -0,0 +1,5 @@
+#
+#Makeefile for industrial I/O CDC drivers
+#
+
+obj-$(CONFIG_AD7746) += ad7746.o
diff --git a/drivers/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c
new file mode 100644
index 0000000..f283819
--- /dev/null
+++ b/drivers/iio/cdc/ad7746.c
@@ -0,0 +1,855 @@
+/*
+ * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
+ *
+ * Copyright 2011 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#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 <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/cdc/ad7746.h>
+
+/*
+ * AD7746 Register Definition
+ */
+
+#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
+
+/* 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		(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_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(x)	(((x) & 0x3) << 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)
+
+/* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
+#define AD7746_CAPDAC_DACEN		BIT(7)
+#define AD7746_CAPDAC_DACP(x)		((x) & 0x7F)
+
+/*
+ * struct ad7746_chip_info - chip specific information
+ */
+
+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;
+
+	union {
+		__be32 d32;
+		u8 d8[4];
+	} data ____cacheline_aligned;
+};
+
+enum ad7746_chan {
+	VIN,
+	VIN_VDD,
+	TEMP_INT,
+	TEMP_EXT,
+	CIN1,
+	CIN1_DIFF,
+	CIN2,
+	CIN2_DIFF,
+};
+
+static const struct iio_chan_spec ad7746_channels[] = {
+	[VIN] = {
+		.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),
+		.address = AD7746_REG_VT_DATA_HIGH << 8 |
+			AD7746_VTSETUP_VTMD_EXT_VIN,
+	},
+	[VIN_VDD] = {
+		.type = IIO_VOLTAGE,
+		.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),
+		.address = AD7746_REG_VT_DATA_HIGH << 8 |
+			AD7746_VTSETUP_VTMD_VDD_MON,
+	},
+	[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,
+	},
+	[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,
+	},
+	[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),
+		.address = AD7746_REG_CAP_DATA_HIGH << 8,
+	},
+	[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_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
+	},
+	[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),
+		.address = AD7746_REG_CAP_DATA_HIGH << 8 |
+			AD7746_CAPSETUP_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_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,
+	}
+};
+
+/* 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_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 = (chan->address & 0xFF) | AD7746_CAPSETUP_CAPEN;
+		vt_setup = chip->vt_setup & ~AD7746_VTSETUP_VTEN;
+		idx = (chip->config & AD7746_CONF_CAPFS_MASK) >>
+			AD7746_CONF_CAPFS_SHIFT;
+		delay = ad7746_cap_filter_rate_table[idx][1];
+
+		if (chip->capdac_set != chan->channel) {
+			ret = i2c_smbus_write_byte_data(chip->client,
+				AD7746_REG_CAPDACA,
+				chip->capdac[chan->channel][0]);
+			if (ret < 0)
+				return ret;
+			ret = i2c_smbus_write_byte_data(chip->client,
+				AD7746_REG_CAPDACB,
+				chip->capdac[chan->channel][1]);
+			if (ret < 0)
+				return ret;
+
+			chip->capdac_set = chan->channel;
+		}
+		break;
+	case IIO_VOLTAGE:
+	case IIO_TEMP:
+		vt_setup = (chan->address & 0xFF) | AD7746_VTSETUP_VTEN;
+		cap_setup = chip->cap_setup & ~AD7746_CAPSETUP_CAPEN;
+		idx = (chip->config & AD7746_CONF_VTFS_MASK) >>
+			AD7746_CONF_VTFS_SHIFT;
+		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 = strtobool(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,
+				  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,
+				  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 |= i << AD7746_CONF_CAPFS_SHIFT;
+
+	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 |= i << AD7746_CONF_VTFS_SHIFT;
+
+	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,
+};
+
+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;
+
+	mutex_lock(&chip->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		if (val != 1) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		/* 2^16 in micro */
+		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:
+			ret = -EINVAL;
+			goto out;
+		}
+
+		ret = i2c_smbus_write_word_swapped(chip->client, reg, val);
+		if (ret < 0)
+			goto out;
+
+		ret = 0;
+		break;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		if (val < 0 || val > 0xFFFF) {
+			ret = -EINVAL;
+			goto out;
+		}
+		ret = i2c_smbus_write_word_swapped(chip->client,
+						   AD7746_REG_CAP_OFFH, val);
+		if (ret < 0)
+			goto out;
+
+		ret = 0;
+		break;
+	case IIO_CHAN_INFO_OFFSET:
+		if (val < 0 || val > 43008000) { /* 21pF */
+			ret = -EINVAL;
+			goto out;
+		}
+
+		/*
+		 * CAPDAC Scale = 21pF_typ / 127
+		 * CIN Scale = 8.192pF / 2^24
+		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
+		 */
+
+		val /= 338646;
+
+		chip->capdac[chan->channel][chan->differential] = val > 0 ?
+			AD7746_CAPDAC_DACP(val) | AD7746_CAPDAC_DACEN : 0;
+
+		ret = i2c_smbus_write_byte_data(chip->client,
+						AD7746_REG_CAPDACA,
+						chip->capdac[chan->channel][0]);
+		if (ret < 0)
+			goto out;
+		ret = i2c_smbus_write_byte_data(chip->client,
+						AD7746_REG_CAPDACB,
+						chip->capdac[chan->channel][1]);
+		if (ret < 0)
+			goto out;
+
+		chip->capdac_set = chan->channel;
+
+		ret = 0;
+		break;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (val2) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		switch (chan->type) {
+		case IIO_CAPACITANCE:
+			ret = ad7746_store_cap_filter_rate_setup(chip, val);
+			break;
+		case IIO_VOLTAGE:
+			ret = ad7746_store_vt_filter_rate_setup(chip, val);
+			break;
+		default:
+			ret = -EINVAL;
+		}
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+out:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+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;
+
+	mutex_lock(&chip->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED:
+		ret = ad7746_select_channel(indio_dev, chan);
+		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, 3,
+						    &chip->data.d8[1]);
+
+		if (ret < 0)
+			goto out;
+
+		/*
+		 * Either for Capacitance, Voltage or Temperature,
+		 * the 0x000000 code represents negative full scale,
+		 * the 0x800000 code represents zero scale, and
+		 * the 0xFFFFFF code represents positive full scale.
+		 */
+
+		*val = (be32_to_cpu(chip->data.d32) & 0xFFFFFF) - 0x800000;
+
+		switch (chan->type) {
+		case IIO_TEMP:
+		/*
+		 * temperature in milli degrees Celsius
+		 * T = ((*val / 2048) - 4096) * 1000
+		 */
+			*val = (*val * 125) / 256;
+			break;
+		case IIO_VOLTAGE:
+
+			/*
+			 * The voltage from the VDD pin is internally
+			 * attenuated by 6.
+			 */
+
+			if (chan->channel == 1) /* supply_raw */
+				*val = *val * 6;
+			break;
+		default:
+			break;
+		}
+
+		ret = IIO_VAL_INT;
+		break;
+	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:
+			ret = -EINVAL;
+			goto out;
+		}
+
+		ret = i2c_smbus_read_word_swapped(chip->client, reg);
+		if (ret < 0)
+			goto out;
+		/* 1 + gain_val / 2^16 */
+		*val = 1;
+		*val2 = (15625 * ret) / 1024;
+
+		ret = IIO_VAL_INT_PLUS_MICRO;
+		break;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		ret = i2c_smbus_read_word_swapped(chip->client,
+						  AD7746_REG_CAP_OFFH);
+		if (ret < 0)
+			goto out;
+		*val = ret;
+
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_OFFSET:
+
+		/*
+		 * CAPDAC Scale = 21pF_typ / 127
+		 * CIN Scale = 8.192pF / 2^24
+		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
+		 */
+
+		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
+					  [chan->differential]) * 338646;
+
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_CAPACITANCE:
+			/* CIN Scale: 8.192pf / 2^24 */
+			*val =  0;
+			*val2 = 488;
+			ret = IIO_VAL_INT_PLUS_NANO;
+			break;
+		case IIO_VOLTAGE:
+			/* VIN Scale: 1170mV / 2^23 */
+			*val = 1170;
+			*val2 = 23;
+			ret = IIO_VAL_FRACTIONAL_LOG2;
+			break;
+		default:
+			ret = -EINVAL;
+			break;
+		}
+
+		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;
+		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;
+		default:
+			ret = -EINVAL;
+		}
+		break;
+	default:
+		ret = -EINVAL;
+	}
+out:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+static const struct iio_info ad7746_info = {
+	.attrs = &ad7746_attribute_group,
+	.read_raw = ad7746_read_raw,
+	.write_raw = ad7746_write_raw,
+};
+
+/*
+ * device probe and remove
+ */
+#ifdef CONFIG_OF
+static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct ad7746_platform_data *pdata;
+	unsigned int tmp;
+	int ret;
+
+	/*
+	 * The default excitation outputs are not inverted, it should be stated
+	 * in the dt if needed.
+	 */
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	ret = of_property_read_u32(np, "adi,exclvl", &tmp);
+	if (ret || tmp > 3) {
+		dev_warn(dev, "Wrong exclvl value, using default\n");
+		pdata->exclvl = 3;
+	} else {
+		pdata->exclvl = tmp;
+	}
+
+	pdata->exca_en = true;
+	pdata->excb_en = true;
+	pdata->exca_inv_en = of_property_read_bool(np, "adi,nexca_en");
+	pdata->excb_inv_en = of_property_read_bool(np, "adi,nexcb_en");
+
+	return pdata;
+}
+#else
+static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
+static int ad7746_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct ad7746_platform_data *pdata = client->dev.platform_data;
+	struct ad7746_chip_info *chip;
+	struct iio_dev *indio_dev;
+	unsigned char regval = 0;
+	int ret = 0;
+
+	if (client->dev.of_node)
+		pdata = ad7746_parse_dt(&client->dev);
+	else
+		pdata = client->dev.platform_data;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+	if (!indio_dev)
+		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;
+
+	/* Establish that the iio_dev is a child of the i2c device */
+	indio_dev->name = id->name;
+	indio_dev->dev.parent = &client->dev;
+	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->num_channels = ARRAY_SIZE(ad7746_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	if (pdata) {
+		if (pdata->exca_en) {
+			if (pdata->exca_inv_en)
+				regval |= AD7746_EXCSETUP_NEXCA;
+			else
+				regval |= AD7746_EXCSETUP_EXCA;
+		}
+
+		if (pdata->excb_en) {
+			if (pdata->excb_inv_en)
+				regval |= AD7746_EXCSETUP_NEXCB;
+			else
+				regval |= AD7746_EXCSETUP_EXCB;
+		}
+
+		regval |= AD7746_EXCSETUP_EXCLVL(pdata->exclvl);
+	} else {
+		dev_warn(&client->dev, "No platform data? using default\n");
+		regval = AD7746_EXCSETUP_EXCA | AD7746_EXCSETUP_EXCB |
+			AD7746_EXCSETUP_EXCLVL(3);
+	}
+
+	ret = i2c_smbus_write_byte_data(chip->client,
+					AD7746_REG_EXC_SETUP, regval);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ad7746_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct ad7746_chip_info *chip = iio_priv(indio_dev);
+	unsigned char regval;
+	int ret;
+
+	mutex_lock(&chip->lock);
+
+	regval = chip->config | AD7746_CONF_MODE_PWRDN;
+	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
+
+	mutex_unlock(&chip->lock);
+
+	if (ret < 0) {
+		dev_warn(&client->dev, "Could NOT Power Down!\n");
+		goto out;
+	}
+
+	iio_device_unregister(indio_dev);
+
+out:
+	return ret;
+}
+
+static const struct i2c_device_id ad7746_id[] = {
+	{ "ad7745", 7745 },
+	{ "ad7746", 7746 },
+	{ "ad7747", 7747 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, ad7746_id);
+
+#ifdef CONFIG_OF
+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);
+#endif
+
+static struct i2c_driver ad7746_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = of_match_ptr(ad7746_of_match),
+	},
+	.probe = ad7746_probe,
+	.remove = ad7746_remove,
+	.id_table = ad7746_id,
+};
+module_i2c_driver(ad7746_driver);
+
+MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
+MODULE_DESCRIPTION("Analog Devices AD7746/5/7 capacitive sensor driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/iio/cdc/ad7746.h b/include/linux/iio/cdc/ad7746.h
new file mode 100644
index 0000000..46ff25e
--- /dev/null
+++ b/include/linux/iio/cdc/ad7746.h
@@ -0,0 +1,20 @@
+/*
+ * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
+ *
+ * Copyright 2011 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ */
+
+#ifndef IIO_CDC_AD7746_H_
+#define IIO_CDC_AD7746_H_
+
+struct ad7746_platform_data {
+	unsigned char exclvl;	/*Excitation Voltage Level */
+	bool exca_en;		/* enables EXCA pin as the excitation output */
+	bool exca_inv_en;	/* enables /EXCA pin as the excitation output */
+	bool excb_en;		/* enables EXCB pin as the excitation output */
+	bool excb_inv_en;	/* enables /EXCB pin as the excitation output */
+};
+
+#endif /* IIO_CDC_AD7746_H_ */
-- 
2.7.4

[PATCH v2 14/14] staging: iio: Remove ad7746 from staging

[Hernán Gonzalez]

Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
---
 .../devicetree/bindings/staging/iio/cdc/ad7746.txt |  34 -
 drivers/staging/iio/cdc/Kconfig                    |  10 -
 drivers/staging/iio/cdc/Makefile                   |   1 -
 drivers/staging/iio/cdc/ad7746.c                   | 856 ---------------------
 drivers/staging/iio/cdc/ad7746.h                   |  24 -
 5 files changed, 925 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
 delete mode 100644 drivers/staging/iio/cdc/ad7746.c
 delete mode 100644 drivers/staging/iio/cdc/ad7746.h

diff --git a/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
deleted file mode 100644
index 7740f05..0000000
--- a/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-Analog Devices AD7746/5/7 capacitive sensor driver
-
-Required properties:
-	- compatible: Should be one of
-		* "adi,ad7745"
-		* "adi,ad7746"
-	  * "adi,ad7747"
-	- reg: The 7-bits long I2c address of the device
-
-Optional properties:
-	- adi,exclvl: This property defines the excitation voltage level for the
-    capacitance to be measured. Possible values are:
-      * 0 = +-VDD/8
-      * 1 = +-VDD/4
-      * 2 = +-VDD * 3/8
-      * 3 = +-VDD/2 (Default)
-	- adi,nexca_en: Invert excitation output A.
-	- adi,nexcb_en:	Invert excitation output B.
-
-Example:
-Here exclvl would be 1 (VDD/4), Excitation pin A would be inverted and
-Excitation pin B would NOT be inverted.
-
-i2c2 {
-
-      < . . . >
-
-      ad7746: ad7746@60 {
-              compatible = "ad7746";
-              reg = <0x60>;
-              adi,exclvl = <1>;
-              adi,nexca_en;
-      };
-};
diff --git a/drivers/staging/iio/cdc/Kconfig b/drivers/staging/iio/cdc/Kconfig
index 80211df..a170ab3 100644
--- a/drivers/staging/iio/cdc/Kconfig
+++ b/drivers/staging/iio/cdc/Kconfig
@@ -23,14 +23,4 @@ config AD7152
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7152.
 
-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
index a5fbabf..5db1acd 100644
--- a/drivers/staging/iio/cdc/Makefile
+++ b/drivers/staging/iio/cdc/Makefile
@@ -4,4 +4,3 @@
 
 obj-$(CONFIG_AD7150) += ad7150.o
 obj-$(CONFIG_AD7152) += ad7152.o
-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 ef0ebb5..0000000
--- a/drivers/staging/iio/cdc/ad7746.c
+++ /dev/null
@@ -1,856 +0,0 @@
-/*
- * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
- *
- * Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
- */
-
-#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 <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-
-#include "ad7746.h"
-
-/*
- * AD7746 Register Definition
- */
-
-#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
-
-/* 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		(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_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(x)	(((x) & 0x3) << 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)
-
-/* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
-#define AD7746_CAPDAC_DACEN		BIT(7)
-#define AD7746_CAPDAC_DACP(x)		((x) & 0x7F)
-
-/*
- * struct ad7746_chip_info - chip specific information
- */
-
-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;
-
-	union {
-		__be32 d32;
-		u8 d8[4];
-	} data ____cacheline_aligned;
-};
-
-enum ad7746_chan {
-	VIN,
-	VIN_VDD,
-	TEMP_INT,
-	TEMP_EXT,
-	CIN1,
-	CIN1_DIFF,
-	CIN2,
-	CIN2_DIFF,
-};
-
-static const struct iio_chan_spec ad7746_channels[] = {
-	[VIN] = {
-		.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),
-		.address = AD7746_REG_VT_DATA_HIGH << 8 |
-			AD7746_VTSETUP_VTMD_EXT_VIN,
-	},
-	[VIN_VDD] = {
-		.type = IIO_VOLTAGE,
-		.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),
-		.address = AD7746_REG_VT_DATA_HIGH << 8 |
-			AD7746_VTSETUP_VTMD_VDD_MON,
-	},
-	[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,
-	},
-	[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,
-	},
-	[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),
-		.address = AD7746_REG_CAP_DATA_HIGH << 8,
-	},
-	[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_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
-	},
-	[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),
-		.address = AD7746_REG_CAP_DATA_HIGH << 8 |
-			AD7746_CAPSETUP_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_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,
-	}
-};
-
-/* 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_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 = (chan->address & 0xFF) | AD7746_CAPSETUP_CAPEN;
-		vt_setup = chip->vt_setup & ~AD7746_VTSETUP_VTEN;
-		idx = (chip->config & AD7746_CONF_CAPFS_MASK) >>
-			AD7746_CONF_CAPFS_SHIFT;
-		delay = ad7746_cap_filter_rate_table[idx][1];
-
-		if (chip->capdac_set != chan->channel) {
-			ret = i2c_smbus_write_byte_data(chip->client,
-				AD7746_REG_CAPDACA,
-				chip->capdac[chan->channel][0]);
-			if (ret < 0)
-				return ret;
-			ret = i2c_smbus_write_byte_data(chip->client,
-				AD7746_REG_CAPDACB,
-				chip->capdac[chan->channel][1]);
-			if (ret < 0)
-				return ret;
-
-			chip->capdac_set = chan->channel;
-		}
-		break;
-	case IIO_VOLTAGE:
-	case IIO_TEMP:
-		vt_setup = (chan->address & 0xFF) | AD7746_VTSETUP_VTEN;
-		cap_setup = chip->cap_setup & ~AD7746_CAPSETUP_CAPEN;
-		idx = (chip->config & AD7746_CONF_VTFS_MASK) >>
-			AD7746_CONF_VTFS_SHIFT;
-		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 = strtobool(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,
-				  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,
-				  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 |= i << AD7746_CONF_CAPFS_SHIFT;
-
-	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 |= i << AD7746_CONF_VTFS_SHIFT;
-
-	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,
-};
-
-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;
-
-	mutex_lock(&chip->lock);
-
-	switch (mask) {
-	case IIO_CHAN_INFO_CALIBSCALE:
-		if (val != 1) {
-			ret = -EINVAL;
-			goto out;
-		}
-
-		/* 2^16 in micro */
-		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:
-			ret = -EINVAL;
-			goto out;
-		}
-
-		ret = i2c_smbus_write_word_swapped(chip->client, reg, val);
-		if (ret < 0)
-			goto out;
-
-		ret = 0;
-		break;
-	case IIO_CHAN_INFO_CALIBBIAS:
-		if (val < 0 || val > 0xFFFF) {
-			ret = -EINVAL;
-			goto out;
-		}
-		ret = i2c_smbus_write_word_swapped(chip->client,
-						   AD7746_REG_CAP_OFFH, val);
-		if (ret < 0)
-			goto out;
-
-		ret = 0;
-		break;
-	case IIO_CHAN_INFO_OFFSET:
-		if (val < 0 || val > 43008000) { /* 21pF */
-			ret = -EINVAL;
-			goto out;
-		}
-
-		/*
-		 * CAPDAC Scale = 21pF_typ / 127
-		 * CIN Scale = 8.192pF / 2^24
-		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
-		 */
-
-		val /= 338646;
-
-		chip->capdac[chan->channel][chan->differential] = val > 0 ?
-			AD7746_CAPDAC_DACP(val) | AD7746_CAPDAC_DACEN : 0;
-
-		ret = i2c_smbus_write_byte_data(chip->client,
-						AD7746_REG_CAPDACA,
-						chip->capdac[chan->channel][0]);
-		if (ret < 0)
-			goto out;
-		ret = i2c_smbus_write_byte_data(chip->client,
-						AD7746_REG_CAPDACB,
-						chip->capdac[chan->channel][1]);
-		if (ret < 0)
-			goto out;
-
-		chip->capdac_set = chan->channel;
-
-		ret = 0;
-		break;
-	case IIO_CHAN_INFO_SAMP_FREQ:
-		if (val2) {
-			ret = -EINVAL;
-			goto out;
-		}
-
-		switch (chan->type) {
-		case IIO_CAPACITANCE:
-			ret = ad7746_store_cap_filter_rate_setup(chip, val);
-			break;
-		case IIO_VOLTAGE:
-			ret = ad7746_store_vt_filter_rate_setup(chip, val);
-			break;
-		default:
-			ret = -EINVAL;
-		}
-		break;
-	default:
-		ret = -EINVAL;
-	}
-
-out:
-	mutex_unlock(&chip->lock);
-	return ret;
-}
-
-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;
-
-	mutex_lock(&chip->lock);
-
-	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
-	case IIO_CHAN_INFO_PROCESSED:
-		ret = ad7746_select_channel(indio_dev, chan);
-		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, 3,
-						    &chip->data.d8[1]);
-
-		if (ret < 0)
-			goto out;
-
-		/*
-		 * Either for Capacitance, Voltage or Temperature,
-		 * the 0x000000 code represents negative full scale,
-		 * the 0x800000 code represents zero scale, and
-		 * the 0xFFFFFF code represents positive full scale.
-		 */
-
-		*val = (be32_to_cpu(chip->data.d32) & 0xFFFFFF) - 0x800000;
-
-		switch (chan->type) {
-		case IIO_TEMP:
-		/*
-		 * temperature in milli degrees Celsius
-		 * T = ((*val / 2048) - 4096) * 1000
-		 */
-			*val = (*val * 125) / 256;
-			break;
-		case IIO_VOLTAGE:
-
-			/*
-			 * The voltage from the VDD pin is internally
-			 * attenuated by 6.
-			 */
-
-			if (chan->channel == 1) /* supply_raw */
-				*val = *val * 6;
-			break;
-		default:
-			break;
-		}
-
-		ret = IIO_VAL_INT;
-		break;
-	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:
-			ret = -EINVAL;
-			goto out;
-		}
-
-		ret = i2c_smbus_read_word_swapped(chip->client, reg);
-		if (ret < 0)
-			goto out;
-		/* 1 + gain_val / 2^16 */
-		*val = 1;
-		*val2 = (15625 * ret) / 1024;
-
-		ret = IIO_VAL_INT_PLUS_MICRO;
-		break;
-	case IIO_CHAN_INFO_CALIBBIAS:
-		ret = i2c_smbus_read_word_swapped(chip->client,
-						  AD7746_REG_CAP_OFFH);
-		if (ret < 0)
-			goto out;
-		*val = ret;
-
-		ret = IIO_VAL_INT;
-		break;
-	case IIO_CHAN_INFO_OFFSET:
-
-		/*
-		 * CAPDAC Scale = 21pF_typ / 127
-		 * CIN Scale = 8.192pF / 2^24
-		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
-		 */
-
-		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
-					  [chan->differential]) * 338646;
-
-		ret = IIO_VAL_INT;
-		break;
-	case IIO_CHAN_INFO_SCALE:
-		switch (chan->type) {
-		case IIO_CAPACITANCE:
-			/* CIN Scale: 8.192pf / 2^24 */
-			*val =  0;
-			*val2 = 488;
-			ret = IIO_VAL_INT_PLUS_NANO;
-			break;
-		case IIO_VOLTAGE:
-			/* VIN Scale: 1170mV / 2^23 */
-			*val = 1170;
-			*val2 = 23;
-			ret = IIO_VAL_FRACTIONAL_LOG2;
-			break;
-		default:
-			ret = -EINVAL;
-			break;
-		}
-
-		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;
-		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;
-		default:
-			ret = -EINVAL;
-		}
-		break;
-	default:
-		ret = -EINVAL;
-	}
-out:
-	mutex_unlock(&chip->lock);
-	return ret;
-}
-
-static const struct iio_info ad7746_info = {
-	.attrs = &ad7746_attribute_group,
-	.read_raw = ad7746_read_raw,
-	.write_raw = ad7746_write_raw,
-};
-
-/*
- * device probe and remove
- */
-#ifdef CONFIG_OF
-static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
-{
-	struct device_node *np = dev->of_node;
-	struct ad7746_platform_data *pdata;
-	unsigned int tmp;
-	int ret;
-
-	/*
-	 * The default excitation outputs are not inverted, it should be stated
-	 * in the dt if needed.
-	 */
-
-	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return NULL;
-
-	ret = of_property_read_u32(np, "adi,exclvl", &tmp);
-	if (ret || tmp > 3) {
-		dev_warn(dev, "Wrong exclvl value, using default\n");
-		pdata->exclvl = 3;
-	} else {
-		pdata->exclvl = tmp;
-	}
-
-	pdata->exca_en = true;
-	pdata->excb_en = true;
-	pdata->exca_inv_en = of_property_read_bool(np, "adi,nexca_en");
-	pdata->excb_inv_en = of_property_read_bool(np, "adi,nexcb_en");
-
-	return pdata;
-}
-#else
-static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
-{
-	return NULL;
-}
-#endif
-
-static int ad7746_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
-{
-	struct ad7746_platform_data *pdata = client->dev.platform_data;
-	struct ad7746_chip_info *chip;
-	struct iio_dev *indio_dev;
-	unsigned char regval = 0;
-	int ret = 0;
-
-	if (client->dev.of_node)
-		pdata = ad7746_parse_dt(&client->dev);
-	else
-		pdata = client->dev.platform_data;
-
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
-	if (!indio_dev)
-		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;
-
-	/* Establish that the iio_dev is a child of the i2c device */
-	indio_dev->name = id->name;
-	indio_dev->dev.parent = &client->dev;
-	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->num_channels = ARRAY_SIZE(ad7746_channels);
-	indio_dev->modes = INDIO_DIRECT_MODE;
-
-	if (pdata) {
-		if (pdata->exca_en) {
-			if (pdata->exca_inv_en)
-				regval |= AD7746_EXCSETUP_NEXCA;
-			else
-				regval |= AD7746_EXCSETUP_EXCA;
-		}
-
-		if (pdata->excb_en) {
-			if (pdata->excb_inv_en)
-				regval |= AD7746_EXCSETUP_NEXCB;
-			else
-				regval |= AD7746_EXCSETUP_EXCB;
-		}
-
-		regval |= AD7746_EXCSETUP_EXCLVL(pdata->exclvl);
-	} else {
-		dev_warn(&client->dev, "No platform data? using default\n");
-		regval = AD7746_EXCSETUP_EXCA | AD7746_EXCSETUP_EXCB |
-			AD7746_EXCSETUP_EXCLVL(3);
-	}
-
-	ret = i2c_smbus_write_byte_data(chip->client,
-					AD7746_REG_EXC_SETUP, regval);
-	if (ret < 0)
-		return ret;
-
-	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
-	if (ret)
-		return ret;
-
-	return 0;
-}
-
-static int ad7746_remove(struct i2c_client *client)
-{
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct ad7746_chip_info *chip = iio_priv(indio_dev);
-	unsigned char regval;
-	int ret;
-
-	mutex_lock(&chip->lock);
-
-	regval = chip->config | AD7746_CONF_MODE_PWRDN;
-	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
-
-	mutex_unlock(&chip->lock);
-
-	if (ret < 0) {
-		dev_warn(&client->dev, "Could NOT Power Down!\n");
-		goto out;
-	}
-
-	iio_device_unregister(indio_dev);
-
-out:
-	return ret;
-}
-
-static const struct i2c_device_id ad7746_id[] = {
-	{ "ad7745", 7745 },
-	{ "ad7746", 7746 },
-	{ "ad7747", 7747 },
-	{}
-};
-MODULE_DEVICE_TABLE(i2c, ad7746_id);
-
-#ifdef CONFIG_OF
-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);
-#endif
-
-static struct i2c_driver ad7746_driver = {
-	.driver = {
-		.name = KBUILD_MODNAME,
-		.of_match_table = of_match_ptr(ad7746_of_match),
-	},
-	.probe = ad7746_probe,
-	.remove = ad7746_remove,
-	.id_table = ad7746_id,
-};
-module_i2c_driver(ad7746_driver);
-
-MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
-MODULE_DESCRIPTION("Analog Devices AD7746/5/7 capacitive sensor driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/cdc/ad7746.h b/drivers/staging/iio/cdc/ad7746.h
deleted file mode 100644
index 2fbcee8..0000000
--- a/drivers/staging/iio/cdc/ad7746.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
- *
- * Copyright 2011 Analog Devices Inc.
- *
- * Licensed under the GPL-2.
- */
-
-#ifndef IIO_CDC_AD7746_H_
-#define IIO_CDC_AD7746_H_
-
-/*
- * TODO: struct ad7746_platform_data needs to go into include/linux/iio
- */
-
-struct ad7746_platform_data {
-	unsigned char exclvl;	/*Excitation Voltage Level */
-	bool exca_en;		/* enables EXCA pin as the excitation output */
-	bool exca_inv_en;	/* enables /EXCA pin as the excitation output */
-	bool excb_en;		/* enables EXCB pin as the excitation output */
-	bool excb_inv_en;	/* enables /EXCB pin as the excitation output */
-};
-
-#endif /* IIO_CDC_AD7746_H_ */
-- 
2.7.4

Re: [PATCH v2 01/14] staging: iio: ad7746: Automatically swap values in readings/writings

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:38 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Data to read or write was being handled with the swab16() macro instead
> of using i2c_smbus_{read,write}_swapped.
> 
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
Applied to the togreg branch of iio.git and pushed out as
testing for the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/cdc/ad7746.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index 4882dbc..53e28ae 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -451,7 +451,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
>  			goto out;
>  		}
>  
> -		ret = i2c_smbus_write_word_data(chip->client, reg, swab16(val));
> +		ret = i2c_smbus_write_word_swapped(chip->client, reg, val);
>  		if (ret < 0)
>  			goto out;
>  
> @@ -462,8 +462,8 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
>  			ret = -EINVAL;
>  			goto out;
>  		}
> -		ret = i2c_smbus_write_word_data(chip->client,
> -				AD7746_REG_CAP_OFFH, swab16(val));
> +		ret = i2c_smbus_write_word_swapped(chip->client,
> +						   AD7746_REG_CAP_OFFH, val);
>  		if (ret < 0)
>  			goto out;
>  
> @@ -594,21 +594,21 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
>  			goto out;
>  		}
>  
> -		ret = i2c_smbus_read_word_data(chip->client, reg);
> +		ret = i2c_smbus_read_word_swapped(chip->client, reg);
>  		if (ret < 0)
>  			goto out;
>  		/* 1 + gain_val / 2^16 */
>  		*val = 1;
> -		*val2 = (15625 * swab16(ret)) / 1024;
> +		*val2 = (15625 * ret) / 1024;
>  
>  		ret = IIO_VAL_INT_PLUS_MICRO;
>  		break;
>  	case IIO_CHAN_INFO_CALIBBIAS:
> -		ret = i2c_smbus_read_word_data(chip->client,
> -					       AD7746_REG_CAP_OFFH);
> +		ret = i2c_smbus_read_word_swapped(chip->client,
> +						  AD7746_REG_CAP_OFFH);
>  		if (ret < 0)
>  			goto out;
> -		*val = swab16(ret);
> +		*val = ret;
>  
>  		ret = IIO_VAL_INT;
>  		break;

Re: [PATCH v2 02/14] staging: iio: ad7746: Adjust arguments to match open parenthesis

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:39 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Clear a couple more checkpatch.pl CHECKS.
> 
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/cdc/ad7746.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index 53e28ae..516aa93 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -556,7 +556,8 @@ 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, 3,
> +						    &chip->data.d8[1]);
>  
>  		if (ret < 0)
>  			goto out;
> @@ -614,7 +615,7 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
>  		break;
>  	case IIO_CHAN_INFO_OFFSET:
>  		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
> -			[chan->differential]) * 338646;
> +					  [chan->differential]) * 338646;
>  
>  		ret = IIO_VAL_INT;
>  		break;

Re: [PATCH v2 03/14] staging: iio: ad7746: Fix bound checkings

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:40 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Also remove unnecessary parenthesis
I am probably missing something.  I'm not sure what you mean
by fix bound checking?   There are superfluous brackets, but
I don't see any functional change to indicate there was anything
wrong with the original checks.

> 
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> ---
>  drivers/staging/iio/cdc/ad7746.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index 516aa93..d793785 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -458,7 +458,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
>  		ret = 0;
>  		break;
>  	case IIO_CHAN_INFO_CALIBBIAS:
> -		if ((val < 0) | (val > 0xFFFF)) {
> +		if (val < 0 || val > 0xFFFF) {
>  			ret = -EINVAL;
>  			goto out;
>  		}
> @@ -470,7 +470,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
>  		ret = 0;
>  		break;
>  	case IIO_CHAN_INFO_OFFSET:
> -		if ((val < 0) | (val > 43008000)) { /* 21pF */
> +		if (val < 0 || val > 43008000) { /* 21pF */
>  			ret = -EINVAL;
>  			goto out;
>  		}

Re: [PATCH v2 04/14] staging: iio: ad7746: Fix multiple line dereference

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:41 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Clear checkpatch.pl WARNING about multiple line derefence but creates a
> new one of line over 80 characters. In my opinion, it improves
> readability.
> 
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
I wouldn't say I personally have strong views either way on this, but
your version is fine so I'll apply it.

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

Thanks,

Jonathan

> ---
>  drivers/staging/iio/cdc/ad7746.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index d793785..82fac76 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -410,8 +410,7 @@ static struct attribute *ad7746_attributes[] = {
>  	&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,
> +	&iio_const_attr_in_capacitance_sampling_frequency_available.dev_attr.attr,
>  	NULL,
>  };
>  

Re: [PATCH v2 05/14] staging: iio: ad7746: Reorder includes alphabetically

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:42 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/cdc/ad7746.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index 82fac76..9ef476a 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -6,15 +6,15 @@
>   * Licensed under the GPL-2.
>   */
>  
> -#include <linux/interrupt.h>
> +#include <linux/delay.h>
>  #include <linux/device.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/sysfs.h>
>  #include <linux/i2c.h>
> -#include <linux/delay.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 <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>

Re: [PATCH v2 06/14] staging: iio: ad7746: Reorder variable declarations

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:43 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Reorder some variable declarations in an inverse-pyramid scheme.
> 
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
Applied,

Thanks,

Jonathan
> ---
>  drivers/staging/iio/cdc/ad7746.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index 9ef476a..f53612a 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -220,8 +220,8 @@ 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);
> -	int ret, delay, idx;
>  	u8 vt_setup, cap_setup;
> +	int ret, delay, idx;
>  
>  	switch (chan->type) {
>  	case IIO_CAPACITANCE:
> @@ -289,8 +289,8 @@ static inline ssize_t ad7746_start_calib(struct device *dev,
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct ad7746_chip_info *chip = iio_priv(indio_dev);
> -	bool doit;
>  	int ret, timeout = 10;
> +	bool doit;
>  
>  	ret = strtobool(buf, &doit);
>  	if (ret < 0)
> @@ -680,8 +680,8 @@ static int ad7746_probe(struct i2c_client *client,
>  	struct ad7746_platform_data *pdata = client->dev.platform_data;
>  	struct ad7746_chip_info *chip;
>  	struct iio_dev *indio_dev;
> -	int ret = 0;
>  	unsigned char regval = 0;
> +	int ret = 0;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
>  	if (!indio_dev)

Re: [PATCH v2 07/14] staging: iio: ad7746: Remove unused defines

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:44 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> ---
>  drivers/staging/iio/cdc/ad7746.c | 7 -------
>  drivers/staging/iio/cdc/ad7746.h | 5 -----
>  2 files changed, 12 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index f53612a..d39ab34 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -25,7 +25,6 @@
>   * 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
> @@ -38,12 +37,6 @@
>  #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)
> -
There is a pretty strong argument that the driver 'should' be
checking the status register...

I would leave it the defines here.  When they are acting
as 'documentation' of the register layout of a device, they
do little harm and can be very useful.

>  /* Capacitive Channel Setup Register Bit Designations (AD7746_REG_CAP_SETUP) */
>  #define AD7746_CAPSETUP_CAPEN		BIT(7)
>  #define AD7746_CAPSETUP_CIN2		BIT(6) /* AD7746 only */
> diff --git a/drivers/staging/iio/cdc/ad7746.h b/drivers/staging/iio/cdc/ad7746.h
> index ea8572d..2fbcee8 100644
> --- a/drivers/staging/iio/cdc/ad7746.h
> +++ b/drivers/staging/iio/cdc/ad7746.h
> @@ -13,11 +13,6 @@
>   * TODO: struct ad7746_platform_data needs to go into include/linux/iio
>   */
>  
> -#define AD7466_EXCLVL_0		0 /* +-VDD/8 */
> -#define AD7466_EXCLVL_1		1 /* +-VDD/4 */
> -#define AD7466_EXCLVL_2		2 /* +-VDD * 3/8 */
> -#define AD7466_EXCLVL_3		3 /* +-VDD/2 */
> -

Think about what these are for.... They aren't unused
if you are actually using platform data on a given oard.

>  struct ad7746_platform_data {
>  	unsigned char exclvl;	/*Excitation Voltage Level */
>  	bool exca_en;		/* enables EXCA pin as the excitation output */

Re: [PATCH v2 08/14] staging: iio: ad7746: Add dt-bindings

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:45 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> This patch adds dt bindings by populating a pdata struct in order to
> modify as little as possible the existing code. It supports both
> platform_data and dt-bindings but uses only one depending on
> CONFIG_OF's value.
> 
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>

Please reorder the patches in the next version to put the bindings
patch next to this one (before preferably, but after is fine as
well.)

Hmm. I can see why you want to minimize the effect of the older
code, but given that we will probably (eventually) drop the
platform data option, I wonder if it wouldn't be better
to move the data to a sensible location rather than faking
platform_data.

The pdata is only used in probe and only two bits of
it at that so it would be fine to use some local variables and
fill them from platform data or device tree as appropriate.

Jonathan

> ---
>  drivers/staging/iio/cdc/ad7746.c | 54 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index d39ab34..c29a221 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -666,6 +666,43 @@ static const struct iio_info ad7746_info = {
>  /*
>   * device probe and remove
>   */
> +#ifdef CONFIG_OF
> +static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct ad7746_platform_data *pdata;
> +	unsigned int tmp;
> +	int ret;
> +
> +	/* The default excitation outputs are not inverted, it should be stated
Please use standard multiline comment syntax.

/*
 * The...
 */
> +	 * in the dt if needed.
> +	 */
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +
> +	ret = of_property_read_u32(np, "adi,exclvl", &tmp);
> +	if (ret || tmp > 3) {
> +		dev_warn(dev, "Wrong exclvl value, using default\n");

Seems a little odd to have the check in here rather than generic.

> +		pdata->exclvl = 3; /* default value */
> +	} else {
> +		pdata->exclvl = tmp;
> +	}
> +
> +	pdata->exca_en = true;
> +	pdata->excb_en = true;
> +	pdata->exca_inv_en = of_property_read_bool(np, "adi,nexca_en");
> +	pdata->excb_inv_en = of_property_read_bool(np, "adi,nexcb_en");
> +
> +	return pdata;
> +}
> +#else
> +static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
> +{
> +	return NULL;
> +}
> +#endif
>  
>  static int ad7746_probe(struct i2c_client *client,
>  			const struct i2c_device_id *id)
> @@ -676,6 +713,11 @@ static int ad7746_probe(struct i2c_client *client,
>  	unsigned char regval = 0;
>  	int ret = 0;
>  
> +	if (client->dev.of_node)
> +		pdata = ad7746_parse_dt(&client->dev);
> +	else
> +		pdata = client->dev.platform_data;
> +
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
>  	if (!indio_dev)
>  		return -ENOMEM;
> @@ -739,12 +781,22 @@ static const struct i2c_device_id ad7746_id[] = {
>  	{ "ad7747", 7747 },
>  	{}
>  };
> -
>  MODULE_DEVICE_TABLE(i2c, ad7746_id);
>  
> +#ifdef CONFIG_OF
> +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);
> +#endif
> +
>  static struct i2c_driver ad7746_driver = {
>  	.driver = {
>  		.name = KBUILD_MODNAME,
> +		.of_match_table = of_match_ptr(ad7746_of_match),
>  	},
>  	.probe = ad7746_probe,
>  	.id_table = ad7746_id,

Re: [PATCH v2 09/14] staging: iio: ad7746: Add remove()

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:46 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> This allows the driver to be probed and removed as a module powering it
> down on remove().
> 
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> ---
>  drivers/staging/iio/cdc/ad7746.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index c29a221..05506bf9 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -775,6 +775,31 @@ static int ad7746_probe(struct i2c_client *client,
>  	return 0;
>  }
>  
> +static int ad7746_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct ad7746_chip_info *chip = iio_priv(indio_dev);
> +	unsigned char regval;
> +	int ret;
> +
> +	mutex_lock(&chip->lock);
> +
> +	regval = chip->config | AD7746_CONF_MODE_PWRDN;
> +	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
As this is a one off operation, perhaps it would be better to factor
it out to a utility function then use devm_add_action_or_reset in
the probe?

Also, I am nervous about this change as there doesn't seem to be
path in probe by which this is deliberately reversed?
It seems to be 'accidentally' handled by the read_raw write to the
CFG register.

The data sheet doesn't really mention much about this register
at all.  It may have special requirements to exit from power down - I have
no idea, but if it is costless, why do we bother with idle mode?

Perhaps Michael can confirm if this is safe to do or not.


> +
> +	mutex_unlock(&chip->lock);
> +
> +	if (ret < 0) {
> +		dev_warn(&client->dev, "Could NOT Power Down!\n");
> +		goto out;
> +	}
> +
> +	iio_device_unregister(indio_dev);
You can't safely do this against the devm_iio_device_register.

> +
> +out:
> +	return ret;
> +}
> +
>  static const struct i2c_device_id ad7746_id[] = {
>  	{ "ad7745", 7745 },
>  	{ "ad7746", 7746 },
> @@ -799,6 +824,7 @@ static struct i2c_driver ad7746_driver = {
>  		.of_match_table = of_match_ptr(ad7746_of_match),
>  	},
>  	.probe = ad7746_probe,
> +	.remove = ad7746_remove,
>  	.id_table = ad7746_id,
>  };
>  module_i2c_driver(ad7746_driver);

Re: [PATCH v2 10/14] staging: iio: ad7746: Add comments

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:47 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Add comments to clarify some of the calculations made, specially when
> reading or writing values.
> 
Mostly good, but a few minor comments.

Jonathan

> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> ---
>  drivers/staging/iio/cdc/ad7746.c | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index 05506bf9..ef0ebb5 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -429,6 +429,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
>  			goto out;
>  		}
>  
> +		/* 2^16 in micro */
I'm not seeing the connection...  what is 2^16?

>  		val = (val2 * 1024) / 15625;
>  
>  		switch (chan->type) {
> @@ -554,6 +555,13 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
>  		if (ret < 0)
>  			goto out;
>  
> +		/*
> +		 * Either for Capacitance, Voltage or Temperature,
> +		 * the 0x000000 code represents negative full scale,
> +		 * the 0x800000 code represents zero scale, and
> +		 * the 0xFFFFFF code represents positive full scale.
> +		 */
> +
>  		*val = (be32_to_cpu(chip->data.d32) & 0xFFFFFF) - 0x800000;
>  
>  		switch (chan->type) {
> @@ -565,7 +573,13 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
>  			*val = (*val * 125) / 256;
>  			break;
>  		case IIO_VOLTAGE:
> -			if (chan->channel == 1) /* supply_raw*/
> +
> +			/*
> +			 * The voltage from the VDD pin is internally
> +			 * attenuated by 6.
> +			 */
> +
> +			if (chan->channel == 1) /* supply_raw */
>  				*val = *val * 6;
>  			break;
>  		default:
> @@ -606,6 +620,13 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
>  		ret = IIO_VAL_INT;
>  		break;
>  	case IIO_CHAN_INFO_OFFSET:
> +
> +		/*
> +		 * CAPDAC Scale = 21pF_typ / 127
> +		 * CIN Scale = 8.192pF / 2^24
> +		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
> +		 */
I don't think the following blank line does much other than make
it less clear what the comment is about.

Same in other locations. Allow the absence of a blank line on one
side of the comment to indicate it's association.


> +
>  		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
>  					  [chan->differential]) * 338646;
>  
> @@ -614,13 +635,13 @@ static int ad7746_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
>  		case IIO_CAPACITANCE:
> -			/* 8.192pf / 2^24 */
> +			/* CIN Scale: 8.192pf / 2^24 */
>  			*val =  0;
>  			*val2 = 488;
>  			ret = IIO_VAL_INT_PLUS_NANO;
>  			break;
>  		case IIO_VOLTAGE:
> -			/* 1170mV / 2^23 */
> +			/* VIN Scale: 1170mV / 2^23 */
>  			*val = 1170;
>  			*val2 = 23;
>  			ret = IIO_VAL_FRACTIONAL_LOG2;
> @@ -674,7 +695,8 @@ static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
>  	unsigned int tmp;
>  	int ret;
>  
> -	/* The default excitation outputs are not inverted, it should be stated
> +	/*
> +	 * The default excitation outputs are not inverted, it should be stated
>  	 * in the dt if needed.
>  	 */
>  
> @@ -685,7 +707,7 @@ static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
>  	ret = of_property_read_u32(np, "adi,exclvl", &tmp);
>  	if (ret || tmp > 3) {
>  		dev_warn(dev, "Wrong exclvl value, using default\n");
> -		pdata->exclvl = 3; /* default value */
> +		pdata->exclvl = 3;
>  	} else {
>  		pdata->exclvl = tmp;
>  	}

Re: [PATCH v2 11/14] staging: iio: ad7746: Add devicetree bindings documentation

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:48 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
A few comments inline.

Thanks,

Jonathan

> ---
>  .../devicetree/bindings/staging/iio/cdc/ad7746.txt | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
> 
> diff --git a/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
> new file mode 100644
> index 0000000..7740f05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
> @@ -0,0 +1,34 @@
> +Analog Devices AD7746/5/7 capacitive sensor driver
> +
> +Required properties:
> +	- compatible: Should be one of
> +		* "adi,ad7745"
> +		* "adi,ad7746"
> +	  * "adi,ad7747"
> +	- reg: The 7-bits long I2c address of the device
> +
> +Optional properties:
> +	- adi,exclvl: This property defines the excitation voltage level for the
> +    capacitance to be measured. Possible values are:
> +      * 0 = +-VDD/8
> +      * 1 = +-VDD/4
> +      * 2 = +-VDD * 3/8
> +      * 3 = +-VDD/2 (Default)
> +	- adi,nexca_en: Invert excitation output A.

The naming is rather odd.  Why _en?

> +	- adi,nexcb_en:	Invert excitation output B.
> +
> +Example:
> +Here exclvl would be 1 (VDD/4), Excitation pin A would be inverted and
> +Excitation pin B would NOT be inverted.
> +
> +i2c2 {
> +
> +      < . . . >
What does this spacer add to the example?
Sure, there can be other devices, but no need to illustrate that here.

> +
> +      ad7746: ad7746@60 {
> +              compatible = "ad7746";
> +              reg = <0x60>;
> +              adi,exclvl = <1>;
> +              adi,nexca_en;
> +      };
> +};

Re: [PATCH v2 12/14] staging: iio: ad7746: Add ABI documentation

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:49 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> The use cases for this driver don't comply with the current ABI. The
> ad7746 and ad7152 need an external capacitance or voltage reference to
> automatically calibrate themselves which is not the normal use case of
> the calibscale and calibbias ABIs, a new ABI was needed.

I agree with the general aim here.  Would appreciate input from Michael
in particular on these + ideally others!

One comment inline,

Thanks,

Jonathan

> 
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-ad7746 | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-ad7746
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-ad7746 b/Documentation/ABI/testing/sysfs-bus-iio-ad7746
> new file mode 100644
> index 0000000..96a41b7
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-ad7746
> @@ -0,0 +1,17 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/in_capacitanceX_calibscale_calibration
> +What:		/sys/bus/iio/devices/iio:deviceX/in_voltage0_calibscale_calibration

Hmm. I wonder if indicating that it causes calibration would be better done by
in_voltage0_calibscale_calibrate

> +KernelVersion:	4.17.0
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Enter gain calibration mode, in which a full-scale
> +		{capacitance, voltage reference} must be connected to the
> +		{capacitance, voltage} input beforehand to automatically
> +		calibrate the device.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/in_capacitanceX_calibbias_calibration
> +KernelVersion:	4.17.0
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Enter offset calibration mode, in which a zero-scale
> +		capacitance must be connected to the capacitance input
> +		beforehand to automatically calibrate the device.

Re: [PATCH v2 14/14] staging: iio: Remove ad7746 from staging

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:51 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
Please have a single patch (with move detection turned off) for this and the
previous.

Allows easy review by showing us the code but doesn't lead to a stage in which
two different versions will build and hence probably break bisectiblity.

Jonathan

> ---
>  .../devicetree/bindings/staging/iio/cdc/ad7746.txt |  34 -
>  drivers/staging/iio/cdc/Kconfig                    |  10 -
>  drivers/staging/iio/cdc/Makefile                   |   1 -
>  drivers/staging/iio/cdc/ad7746.c                   | 856 ---------------------
>  drivers/staging/iio/cdc/ad7746.h                   |  24 -
>  5 files changed, 925 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
>  delete mode 100644 drivers/staging/iio/cdc/ad7746.c
>  delete mode 100644 drivers/staging/iio/cdc/ad7746.h
> 
> diff --git a/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
> deleted file mode 100644
> index 7740f05..0000000
> --- a/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
> +++ /dev/null
> @@ -1,34 +0,0 @@
> -Analog Devices AD7746/5/7 capacitive sensor driver
> -
> -Required properties:
> -	- compatible: Should be one of
> -		* "adi,ad7745"
> -		* "adi,ad7746"
> -	  * "adi,ad7747"
> -	- reg: The 7-bits long I2c address of the device
> -
> -Optional properties:
> -	- adi,exclvl: This property defines the excitation voltage level for the
> -    capacitance to be measured. Possible values are:
> -      * 0 = +-VDD/8
> -      * 1 = +-VDD/4
> -      * 2 = +-VDD * 3/8
> -      * 3 = +-VDD/2 (Default)
> -	- adi,nexca_en: Invert excitation output A.
> -	- adi,nexcb_en:	Invert excitation output B.
> -
> -Example:
> -Here exclvl would be 1 (VDD/4), Excitation pin A would be inverted and
> -Excitation pin B would NOT be inverted.
> -
> -i2c2 {
> -
> -      < . . . >
> -
> -      ad7746: ad7746@60 {
> -              compatible = "ad7746";
> -              reg = <0x60>;
> -              adi,exclvl = <1>;
> -              adi,nexca_en;
> -      };
> -};
> diff --git a/drivers/staging/iio/cdc/Kconfig b/drivers/staging/iio/cdc/Kconfig
> index 80211df..a170ab3 100644
> --- a/drivers/staging/iio/cdc/Kconfig
> +++ b/drivers/staging/iio/cdc/Kconfig
> @@ -23,14 +23,4 @@ config AD7152
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ad7152.
>  
> -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
> index a5fbabf..5db1acd 100644
> --- a/drivers/staging/iio/cdc/Makefile
> +++ b/drivers/staging/iio/cdc/Makefile
> @@ -4,4 +4,3 @@
>  
>  obj-$(CONFIG_AD7150) += ad7150.o
>  obj-$(CONFIG_AD7152) += ad7152.o
> -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 ef0ebb5..0000000
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ /dev/null
> @@ -1,856 +0,0 @@
> -/*
> - * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
> - *
> - * Copyright 2011 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2.
> - */
> -
> -#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 <linux/iio/iio.h>
> -#include <linux/iio/sysfs.h>
> -
> -#include "ad7746.h"
> -
> -/*
> - * AD7746 Register Definition
> - */
> -
> -#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
> -
> -/* 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		(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_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(x)	(((x) & 0x3) << 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)
> -
> -/* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
> -#define AD7746_CAPDAC_DACEN		BIT(7)
> -#define AD7746_CAPDAC_DACP(x)		((x) & 0x7F)
> -
> -/*
> - * struct ad7746_chip_info - chip specific information
> - */
> -
> -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;
> -
> -	union {
> -		__be32 d32;
> -		u8 d8[4];
> -	} data ____cacheline_aligned;
> -};
> -
> -enum ad7746_chan {
> -	VIN,
> -	VIN_VDD,
> -	TEMP_INT,
> -	TEMP_EXT,
> -	CIN1,
> -	CIN1_DIFF,
> -	CIN2,
> -	CIN2_DIFF,
> -};
> -
> -static const struct iio_chan_spec ad7746_channels[] = {
> -	[VIN] = {
> -		.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),
> -		.address = AD7746_REG_VT_DATA_HIGH << 8 |
> -			AD7746_VTSETUP_VTMD_EXT_VIN,
> -	},
> -	[VIN_VDD] = {
> -		.type = IIO_VOLTAGE,
> -		.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),
> -		.address = AD7746_REG_VT_DATA_HIGH << 8 |
> -			AD7746_VTSETUP_VTMD_VDD_MON,
> -	},
> -	[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,
> -	},
> -	[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,
> -	},
> -	[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),
> -		.address = AD7746_REG_CAP_DATA_HIGH << 8,
> -	},
> -	[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_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
> -	},
> -	[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),
> -		.address = AD7746_REG_CAP_DATA_HIGH << 8 |
> -			AD7746_CAPSETUP_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_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,
> -	}
> -};
> -
> -/* 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_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 = (chan->address & 0xFF) | AD7746_CAPSETUP_CAPEN;
> -		vt_setup = chip->vt_setup & ~AD7746_VTSETUP_VTEN;
> -		idx = (chip->config & AD7746_CONF_CAPFS_MASK) >>
> -			AD7746_CONF_CAPFS_SHIFT;
> -		delay = ad7746_cap_filter_rate_table[idx][1];
> -
> -		if (chip->capdac_set != chan->channel) {
> -			ret = i2c_smbus_write_byte_data(chip->client,
> -				AD7746_REG_CAPDACA,
> -				chip->capdac[chan->channel][0]);
> -			if (ret < 0)
> -				return ret;
> -			ret = i2c_smbus_write_byte_data(chip->client,
> -				AD7746_REG_CAPDACB,
> -				chip->capdac[chan->channel][1]);
> -			if (ret < 0)
> -				return ret;
> -
> -			chip->capdac_set = chan->channel;
> -		}
> -		break;
> -	case IIO_VOLTAGE:
> -	case IIO_TEMP:
> -		vt_setup = (chan->address & 0xFF) | AD7746_VTSETUP_VTEN;
> -		cap_setup = chip->cap_setup & ~AD7746_CAPSETUP_CAPEN;
> -		idx = (chip->config & AD7746_CONF_VTFS_MASK) >>
> -			AD7746_CONF_VTFS_SHIFT;
> -		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 = strtobool(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,
> -				  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,
> -				  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 |= i << AD7746_CONF_CAPFS_SHIFT;
> -
> -	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 |= i << AD7746_CONF_VTFS_SHIFT;
> -
> -	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,
> -};
> -
> -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;
> -
> -	mutex_lock(&chip->lock);
> -
> -	switch (mask) {
> -	case IIO_CHAN_INFO_CALIBSCALE:
> -		if (val != 1) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -
> -		/* 2^16 in micro */
> -		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:
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -
> -		ret = i2c_smbus_write_word_swapped(chip->client, reg, val);
> -		if (ret < 0)
> -			goto out;
> -
> -		ret = 0;
> -		break;
> -	case IIO_CHAN_INFO_CALIBBIAS:
> -		if (val < 0 || val > 0xFFFF) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -		ret = i2c_smbus_write_word_swapped(chip->client,
> -						   AD7746_REG_CAP_OFFH, val);
> -		if (ret < 0)
> -			goto out;
> -
> -		ret = 0;
> -		break;
> -	case IIO_CHAN_INFO_OFFSET:
> -		if (val < 0 || val > 43008000) { /* 21pF */
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -
> -		/*
> -		 * CAPDAC Scale = 21pF_typ / 127
> -		 * CIN Scale = 8.192pF / 2^24
> -		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
> -		 */
> -
> -		val /= 338646;
> -
> -		chip->capdac[chan->channel][chan->differential] = val > 0 ?
> -			AD7746_CAPDAC_DACP(val) | AD7746_CAPDAC_DACEN : 0;
> -
> -		ret = i2c_smbus_write_byte_data(chip->client,
> -						AD7746_REG_CAPDACA,
> -						chip->capdac[chan->channel][0]);
> -		if (ret < 0)
> -			goto out;
> -		ret = i2c_smbus_write_byte_data(chip->client,
> -						AD7746_REG_CAPDACB,
> -						chip->capdac[chan->channel][1]);
> -		if (ret < 0)
> -			goto out;
> -
> -		chip->capdac_set = chan->channel;
> -
> -		ret = 0;
> -		break;
> -	case IIO_CHAN_INFO_SAMP_FREQ:
> -		if (val2) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -
> -		switch (chan->type) {
> -		case IIO_CAPACITANCE:
> -			ret = ad7746_store_cap_filter_rate_setup(chip, val);
> -			break;
> -		case IIO_VOLTAGE:
> -			ret = ad7746_store_vt_filter_rate_setup(chip, val);
> -			break;
> -		default:
> -			ret = -EINVAL;
> -		}
> -		break;
> -	default:
> -		ret = -EINVAL;
> -	}
> -
> -out:
> -	mutex_unlock(&chip->lock);
> -	return ret;
> -}
> -
> -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;
> -
> -	mutex_lock(&chip->lock);
> -
> -	switch (mask) {
> -	case IIO_CHAN_INFO_RAW:
> -	case IIO_CHAN_INFO_PROCESSED:
> -		ret = ad7746_select_channel(indio_dev, chan);
> -		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, 3,
> -						    &chip->data.d8[1]);
> -
> -		if (ret < 0)
> -			goto out;
> -
> -		/*
> -		 * Either for Capacitance, Voltage or Temperature,
> -		 * the 0x000000 code represents negative full scale,
> -		 * the 0x800000 code represents zero scale, and
> -		 * the 0xFFFFFF code represents positive full scale.
> -		 */
> -
> -		*val = (be32_to_cpu(chip->data.d32) & 0xFFFFFF) - 0x800000;
> -
> -		switch (chan->type) {
> -		case IIO_TEMP:
> -		/*
> -		 * temperature in milli degrees Celsius
> -		 * T = ((*val / 2048) - 4096) * 1000
> -		 */
> -			*val = (*val * 125) / 256;
> -			break;
> -		case IIO_VOLTAGE:
> -
> -			/*
> -			 * The voltage from the VDD pin is internally
> -			 * attenuated by 6.
> -			 */
> -
> -			if (chan->channel == 1) /* supply_raw */
> -				*val = *val * 6;
> -			break;
> -		default:
> -			break;
> -		}
> -
> -		ret = IIO_VAL_INT;
> -		break;
> -	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:
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -
> -		ret = i2c_smbus_read_word_swapped(chip->client, reg);
> -		if (ret < 0)
> -			goto out;
> -		/* 1 + gain_val / 2^16 */
> -		*val = 1;
> -		*val2 = (15625 * ret) / 1024;
> -
> -		ret = IIO_VAL_INT_PLUS_MICRO;
> -		break;
> -	case IIO_CHAN_INFO_CALIBBIAS:
> -		ret = i2c_smbus_read_word_swapped(chip->client,
> -						  AD7746_REG_CAP_OFFH);
> -		if (ret < 0)
> -			goto out;
> -		*val = ret;
> -
> -		ret = IIO_VAL_INT;
> -		break;
> -	case IIO_CHAN_INFO_OFFSET:
> -
> -		/*
> -		 * CAPDAC Scale = 21pF_typ / 127
> -		 * CIN Scale = 8.192pF / 2^24
> -		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
> -		 */
> -
> -		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
> -					  [chan->differential]) * 338646;
> -
> -		ret = IIO_VAL_INT;
> -		break;
> -	case IIO_CHAN_INFO_SCALE:
> -		switch (chan->type) {
> -		case IIO_CAPACITANCE:
> -			/* CIN Scale: 8.192pf / 2^24 */
> -			*val =  0;
> -			*val2 = 488;
> -			ret = IIO_VAL_INT_PLUS_NANO;
> -			break;
> -		case IIO_VOLTAGE:
> -			/* VIN Scale: 1170mV / 2^23 */
> -			*val = 1170;
> -			*val2 = 23;
> -			ret = IIO_VAL_FRACTIONAL_LOG2;
> -			break;
> -		default:
> -			ret = -EINVAL;
> -			break;
> -		}
> -
> -		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;
> -		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;
> -		default:
> -			ret = -EINVAL;
> -		}
> -		break;
> -	default:
> -		ret = -EINVAL;
> -	}
> -out:
> -	mutex_unlock(&chip->lock);
> -	return ret;
> -}
> -
> -static const struct iio_info ad7746_info = {
> -	.attrs = &ad7746_attribute_group,
> -	.read_raw = ad7746_read_raw,
> -	.write_raw = ad7746_write_raw,
> -};
> -
> -/*
> - * device probe and remove
> - */
> -#ifdef CONFIG_OF
> -static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
> -{
> -	struct device_node *np = dev->of_node;
> -	struct ad7746_platform_data *pdata;
> -	unsigned int tmp;
> -	int ret;
> -
> -	/*
> -	 * The default excitation outputs are not inverted, it should be stated
> -	 * in the dt if needed.
> -	 */
> -
> -	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> -	if (!pdata)
> -		return NULL;
> -
> -	ret = of_property_read_u32(np, "adi,exclvl", &tmp);
> -	if (ret || tmp > 3) {
> -		dev_warn(dev, "Wrong exclvl value, using default\n");
> -		pdata->exclvl = 3;
> -	} else {
> -		pdata->exclvl = tmp;
> -	}
> -
> -	pdata->exca_en = true;
> -	pdata->excb_en = true;
> -	pdata->exca_inv_en = of_property_read_bool(np, "adi,nexca_en");
> -	pdata->excb_inv_en = of_property_read_bool(np, "adi,nexcb_en");
> -
> -	return pdata;
> -}
> -#else
> -static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
> -{
> -	return NULL;
> -}
> -#endif
> -
> -static int ad7746_probe(struct i2c_client *client,
> -			const struct i2c_device_id *id)
> -{
> -	struct ad7746_platform_data *pdata = client->dev.platform_data;
> -	struct ad7746_chip_info *chip;
> -	struct iio_dev *indio_dev;
> -	unsigned char regval = 0;
> -	int ret = 0;
> -
> -	if (client->dev.of_node)
> -		pdata = ad7746_parse_dt(&client->dev);
> -	else
> -		pdata = client->dev.platform_data;
> -
> -	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> -	if (!indio_dev)
> -		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;
> -
> -	/* Establish that the iio_dev is a child of the i2c device */
> -	indio_dev->name = id->name;
> -	indio_dev->dev.parent = &client->dev;
> -	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->num_channels = ARRAY_SIZE(ad7746_channels);
> -	indio_dev->modes = INDIO_DIRECT_MODE;
> -
> -	if (pdata) {
> -		if (pdata->exca_en) {
> -			if (pdata->exca_inv_en)
> -				regval |= AD7746_EXCSETUP_NEXCA;
> -			else
> -				regval |= AD7746_EXCSETUP_EXCA;
> -		}
> -
> -		if (pdata->excb_en) {
> -			if (pdata->excb_inv_en)
> -				regval |= AD7746_EXCSETUP_NEXCB;
> -			else
> -				regval |= AD7746_EXCSETUP_EXCB;
> -		}
> -
> -		regval |= AD7746_EXCSETUP_EXCLVL(pdata->exclvl);
> -	} else {
> -		dev_warn(&client->dev, "No platform data? using default\n");
> -		regval = AD7746_EXCSETUP_EXCA | AD7746_EXCSETUP_EXCB |
> -			AD7746_EXCSETUP_EXCLVL(3);
> -	}
> -
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					AD7746_REG_EXC_SETUP, regval);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> -	if (ret)
> -		return ret;
> -
> -	return 0;
> -}
> -
> -static int ad7746_remove(struct i2c_client *client)
> -{
> -	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> -	struct ad7746_chip_info *chip = iio_priv(indio_dev);
> -	unsigned char regval;
> -	int ret;
> -
> -	mutex_lock(&chip->lock);
> -
> -	regval = chip->config | AD7746_CONF_MODE_PWRDN;
> -	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
> -
> -	mutex_unlock(&chip->lock);
> -
> -	if (ret < 0) {
> -		dev_warn(&client->dev, "Could NOT Power Down!\n");
> -		goto out;
> -	}
> -
> -	iio_device_unregister(indio_dev);
> -
> -out:
> -	return ret;
> -}
> -
> -static const struct i2c_device_id ad7746_id[] = {
> -	{ "ad7745", 7745 },
> -	{ "ad7746", 7746 },
> -	{ "ad7747", 7747 },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(i2c, ad7746_id);
> -
> -#ifdef CONFIG_OF
> -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);
> -#endif
> -
> -static struct i2c_driver ad7746_driver = {
> -	.driver = {
> -		.name = KBUILD_MODNAME,
> -		.of_match_table = of_match_ptr(ad7746_of_match),
> -	},
> -	.probe = ad7746_probe,
> -	.remove = ad7746_remove,
> -	.id_table = ad7746_id,
> -};
> -module_i2c_driver(ad7746_driver);
> -
> -MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
> -MODULE_DESCRIPTION("Analog Devices AD7746/5/7 capacitive sensor driver");
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/staging/iio/cdc/ad7746.h b/drivers/staging/iio/cdc/ad7746.h
> deleted file mode 100644
> index 2fbcee8..0000000
> --- a/drivers/staging/iio/cdc/ad7746.h
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/*
> - * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
> - *
> - * Copyright 2011 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2.
> - */
> -
> -#ifndef IIO_CDC_AD7746_H_
> -#define IIO_CDC_AD7746_H_
> -
> -/*
> - * TODO: struct ad7746_platform_data needs to go into include/linux/iio
> - */
> -
> -struct ad7746_platform_data {
> -	unsigned char exclvl;	/*Excitation Voltage Level */
> -	bool exca_en;		/* enables EXCA pin as the excitation output */
> -	bool exca_inv_en;	/* enables /EXCA pin as the excitation output */
> -	bool excb_en;		/* enables EXCB pin as the excitation output */
> -	bool excb_inv_en;	/* enables /EXCB pin as the excitation output */
> -};
> -
> -#endif /* IIO_CDC_AD7746_H_ */

Re: [PATCH v2 13/14] Move ad7746 out of staging

[Jonathan Cameron]

On Fri, 13 Apr 2018 13:36:50 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
A few comments inline.

Also nice to have a bit of a description of the driver in the patch
moving it out of staging.  It's a patch people might actually notice
as to many people it is some new hardware support (as they won't run
staging drivers).

A few comments inline, including some I missed in earlier patches that
could do with tidying up.  Ideally I'd like to see if Michael can
sanity check that the driver actually works.  If that is going to
take a while, I don't mind moving it on the basis we'll fix up any
issues that show up at a later date.

Thanks for your hard work on this.

Jonathan

> ---
>  .../devicetree/bindings/iio/cdc/ad7746.txt         |  34 +

Hmm. Thinking about it, there is no actual need for the driver to be out of staging
to put the devicetree bindings in place outside staging.
I would do that in the first place rather than introducing them then moving 
them a few patches later.


>  drivers/iio/Kconfig                                |   1 +
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/cdc/Kconfig                            |  16 +
>  drivers/iio/cdc/Makefile                           |   5 +
>  drivers/iio/cdc/ad7746.c                           | 855 +++++++++++++++++++++
>  include/linux/iio/cdc/ad7746.h                     |  20 +
>  7 files changed, 932 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/cdc/ad7746.txt
>  create mode 100644 drivers/iio/cdc/Kconfig
>  create mode 100644 drivers/iio/cdc/Makefile
>  create mode 100644 drivers/iio/cdc/ad7746.c
>  create mode 100644 include/linux/iio/cdc/ad7746.h
> 
> diff --git a/Documentation/devicetree/bindings/iio/cdc/ad7746.txt b/Documentation/devicetree/bindings/iio/cdc/ad7746.txt
> new file mode 100644
> index 0000000..7740f05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/cdc/ad7746.txt
> @@ -0,0 +1,34 @@
> +Analog Devices AD7746/5/7 capacitive sensor driver
> +
> +Required properties:
> +	- compatible: Should be one of
> +		* "adi,ad7745"
> +		* "adi,ad7746"
> +	  * "adi,ad7747"
> +	- reg: The 7-bits long I2c address of the device
> +
> +Optional properties:
> +	- adi,exclvl: This property defines the excitation voltage level for the
> +    capacitance to be measured. Possible values are:
> +      * 0 = +-VDD/8
> +      * 1 = +-VDD/4
> +      * 2 = +-VDD * 3/8
> +      * 3 = +-VDD/2 (Default)
> +	- adi,nexca_en: Invert excitation output A.
> +	- adi,nexcb_en:	Invert excitation output B.
> +
> +Example:
> +Here exclvl would be 1 (VDD/4), Excitation pin A would be inverted and
> +Excitation pin B would NOT be inverted.
> +
> +i2c2 {
> +
> +      < . . . >
> +
> +      ad7746: ad7746@60 {
> +              compatible = "ad7746";
> +              reg = <0x60>;
> +              adi,exclvl = <1>;
> +              adi,nexca_en;
> +      };
> +};
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index b3c8c6e..d1c309b 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -71,6 +71,7 @@ config IIO_TRIGGERED_EVENT
>  source "drivers/iio/accel/Kconfig"
>  source "drivers/iio/adc/Kconfig"
>  source "drivers/iio/amplifiers/Kconfig"
> +source "drivers/iio/cdc/Kconfig"
>  source "drivers/iio/chemical/Kconfig"
>  source "drivers/iio/common/Kconfig"
>  source "drivers/iio/counter/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index b16b2e9..18bea8d 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -17,6 +17,7 @@ obj-y += accel/
>  obj-y += adc/
>  obj-y += amplifiers/
>  obj-y += buffer/
> +obj-y += cdc/
>  obj-y += chemical/
>  obj-y += common/
>  obj-y += counter/
> diff --git a/drivers/iio/cdc/Kconfig b/drivers/iio/cdc/Kconfig
> new file mode 100644
> index 0000000..d3a8600
> --- /dev/null
> +++ b/drivers/iio/cdc/Kconfig
> @@ -0,0 +1,16 @@
> +#
> +# 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/iio/cdc/Makefile b/drivers/iio/cdc/Makefile
> new file mode 100644
> index 0000000..1f71283
> --- /dev/null
> +++ b/drivers/iio/cdc/Makefile
> @@ -0,0 +1,5 @@
> +#
> +#Makeefile for industrial I/O CDC drivers
> +#
> +
> +obj-$(CONFIG_AD7746) += ad7746.o
> diff --git a/drivers/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c
> new file mode 100644
> index 0000000..f283819
> --- /dev/null
> +++ b/drivers/iio/cdc/ad7746.c
> @@ -0,0 +1,855 @@
> +/*
> + * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
> + *
> + * Copyright 2011 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#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 <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/cdc/ad7746.h>
There is a standard location for all platform data headers these days and
this isn't it.  This changed since some of the IIO drivers were written
so not surprising if some are still in the wrong place.


linux/platform_data/
> +
> +/*
> + * AD7746 Register Definition
Somewhat stating the obvious and also wrong comment syntax..
I'd get rid of this comment.

> + */
> +
> +#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
> +
> +/* 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		(1 << 7)
The one above looks like it could be a BIT(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_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(x)	(((x) & 0x3) << 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)
> +
> +/* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
> +#define AD7746_CAPDAC_DACEN		BIT(7)
> +#define AD7746_CAPDAC_DACP(x)		((x) & 0x7F)
> +
> +/*
> + * struct ad7746_chip_info - chip specific information
Name of the structure makes this pretty obvious :)  Perhaps
this comment (which should be in single line style anyway,
could be dropped).
> + */
> +
> +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;
> +
> +	union {
> +		__be32 d32;
> +		u8 d8[4];
> +	} data ____cacheline_aligned;
> +};
> +
> +enum ad7746_chan {
> +	VIN,
> +	VIN_VDD,
> +	TEMP_INT,
> +	TEMP_EXT,
> +	CIN1,
> +	CIN1_DIFF,
> +	CIN2,
> +	CIN2_DIFF,
> +};
> +
> +static const struct iio_chan_spec ad7746_channels[] = {
> +	[VIN] = {
> +		.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),
> +		.address = AD7746_REG_VT_DATA_HIGH << 8 |
> +			AD7746_VTSETUP_VTMD_EXT_VIN,
Hmm. I never like to see a single location used to hold two different things.
I would suggest perhaps having address be an enum then have have a lookup
into an array of structures that have the two elements separately.
(use the ad7746_chan enum again for this?)

> +	},
> +	[VIN_VDD] = {
> +		.type = IIO_VOLTAGE,
> +		.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),
> +		.address = AD7746_REG_VT_DATA_HIGH << 8 |
> +			AD7746_VTSETUP_VTMD_VDD_MON,
> +	},
> +	[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,
> +	},
> +	[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,
> +	},
> +	[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),
> +		.address = AD7746_REG_CAP_DATA_HIGH << 8,
> +	},
> +	[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_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
> +	},
> +	[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),
> +		.address = AD7746_REG_CAP_DATA_HIGH << 8 |
> +			AD7746_CAPSETUP_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_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,
> +	}
> +};
> +
> +/* 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_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 = (chan->address & 0xFF) | AD7746_CAPSETUP_CAPEN;
> +		vt_setup = chip->vt_setup & ~AD7746_VTSETUP_VTEN;
> +		idx = (chip->config & AD7746_CONF_CAPFS_MASK) >>
> +			AD7746_CONF_CAPFS_SHIFT;
> +		delay = ad7746_cap_filter_rate_table[idx][1];
> +
> +		if (chip->capdac_set != chan->channel) {
> +			ret = i2c_smbus_write_byte_data(chip->client,
> +				AD7746_REG_CAPDACA,
> +				chip->capdac[chan->channel][0]);
> +			if (ret < 0)
> +				return ret;
> +			ret = i2c_smbus_write_byte_data(chip->client,
> +				AD7746_REG_CAPDACB,
> +				chip->capdac[chan->channel][1]);
> +			if (ret < 0)
> +				return ret;
> +
> +			chip->capdac_set = chan->channel;
> +		}
> +		break;
> +	case IIO_VOLTAGE:
> +	case IIO_TEMP:
> +		vt_setup = (chan->address & 0xFF) | AD7746_VTSETUP_VTEN;
> +		cap_setup = chip->cap_setup & ~AD7746_CAPSETUP_CAPEN;
> +		idx = (chip->config & AD7746_CONF_VTFS_MASK) >>
> +			AD7746_CONF_VTFS_SHIFT;
> +		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 = strtobool(buf, &doit);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!doit)
> +		return 0;
> +
> +	mutex_lock(&chip->lock);
This feels like the wrong level to be taking the lock at.  We aren't
preventing race problems (see below).

> +	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,
> +				  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);

No race possibilities in here? Nothing stops two instances of this running
at once, with different channels...  It is worth checking for
similar problems elsewhere in the driver.


> +	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,
> +				  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);
> +

Seems a little odd to have this next block of code here, before the
use of the above attrs?  Perhaps a bit of reorganization is needed?

> +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 |= i << AD7746_CONF_CAPFS_SHIFT;
> +
> +	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 |= i << AD7746_CONF_VTFS_SHIFT;
> +
> +	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,
> +};
> +
> +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;
> +
> +	mutex_lock(&chip->lock);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		if (val != 1) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		/* 2^16 in micro */
> +		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:
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		ret = i2c_smbus_write_word_swapped(chip->client, reg, val);
> +		if (ret < 0)
> +			goto out;
> +
> +		ret = 0;
> +		break;
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		if (val < 0 || val > 0xFFFF) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		ret = i2c_smbus_write_word_swapped(chip->client,
> +						   AD7746_REG_CAP_OFFH, val);
> +		if (ret < 0)
> +			goto out;
> +
> +		ret = 0;
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (val < 0 || val > 43008000) { /* 21pF */
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		/*
> +		 * CAPDAC Scale = 21pF_typ / 127
> +		 * CIN Scale = 8.192pF / 2^24
> +		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
> +		 */
> +
> +		val /= 338646;
> +
> +		chip->capdac[chan->channel][chan->differential] = val > 0 ?
> +			AD7746_CAPDAC_DACP(val) | AD7746_CAPDAC_DACEN : 0;
> +
> +		ret = i2c_smbus_write_byte_data(chip->client,
> +						AD7746_REG_CAPDACA,
> +						chip->capdac[chan->channel][0]);
> +		if (ret < 0)
> +			goto out;
> +		ret = i2c_smbus_write_byte_data(chip->client,
> +						AD7746_REG_CAPDACB,
> +						chip->capdac[chan->channel][1]);
> +		if (ret < 0)
> +			goto out;
> +
> +		chip->capdac_set = chan->channel;
> +
> +		ret = 0;
> +		break;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		if (val2) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		switch (chan->type) {
> +		case IIO_CAPACITANCE:
> +			ret = ad7746_store_cap_filter_rate_setup(chip, val);
> +			break;
> +		case IIO_VOLTAGE:
> +			ret = ad7746_store_vt_filter_rate_setup(chip, val);
> +			break;
> +		default:
> +			ret = -EINVAL;
> +		}
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +out:
> +	mutex_unlock(&chip->lock);
As with the raw_read below, this lock is held over lots of things
that don't need it. I would push it up to limit it's scope.

> +	return ret;
> +}
> +
> +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;
> +
> +	mutex_lock(&chip->lock);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +	case IIO_CHAN_INFO_PROCESSED:
> +		ret = ad7746_select_channel(indio_dev, chan);
> +		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, 3,
> +						    &chip->data.d8[1]);
> +
> +		if (ret < 0)
> +			goto out;
> +
> +		/*
> +		 * Either for Capacitance, Voltage or Temperature,
> +		 * the 0x000000 code represents negative full scale,
> +		 * the 0x800000 code represents zero scale, and
> +		 * the 0xFFFFFF code represents positive full scale.
> +		 */
> +
> +		*val = (be32_to_cpu(chip->data.d32) & 0xFFFFFF) - 0x800000;
> +
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +		/*
> +		 * temperature in milli degrees Celsius
> +		 * T = ((*val / 2048) - 4096) * 1000
> +		 */
I missed this earlier, but the comment should probably be indented one step more.

> +			*val = (*val * 125) / 256;
> +			break;
> +		case IIO_VOLTAGE:
> +
> +			/*
> +			 * The voltage from the VDD pin is internally
> +			 * attenuated by 6.
> +			 */
> +
> +			if (chan->channel == 1) /* supply_raw */
> +				*val = *val * 6;
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		ret = IIO_VAL_INT;
> +		break;
> +	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:
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		ret = i2c_smbus_read_word_swapped(chip->client, reg);
> +		if (ret < 0)
> +			goto out;
> +		/* 1 + gain_val / 2^16 */
> +		*val = 1;
> +		*val2 = (15625 * ret) / 1024;
> +
> +		ret = IIO_VAL_INT_PLUS_MICRO;
> +		break;
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		ret = i2c_smbus_read_word_swapped(chip->client,
> +						  AD7746_REG_CAP_OFFH);
> +		if (ret < 0)
> +			goto out;
> +		*val = ret;
> +
> +		ret = IIO_VAL_INT;
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +
> +		/*
> +		 * CAPDAC Scale = 21pF_typ / 127
> +		 * CIN Scale = 8.192pF / 2^24
> +		 * Offset Scale = CAPDAC Scale / CIN Scale = 338646
> +		 */
> +
> +		*val = AD7746_CAPDAC_DACP(chip->capdac[chan->channel]
> +					  [chan->differential]) * 338646;
> +
> +		ret = IIO_VAL_INT;
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_CAPACITANCE:
Why are these inside the lock?
Seems odd given they are constant.
I would pull the lock in so it only covers cases that need locking.
It may well make sense to pull a few of the case statements' contents out
as utility functions to simply the unlocking in error paths etc.

This is particularly true for _RAW and _PROCESSED block above.


> +			/* CIN Scale: 8.192pf / 2^24 */
> +			*val =  0;
> +			*val2 = 488;
> +			ret = IIO_VAL_INT_PLUS_NANO;
> +			break;
> +		case IIO_VOLTAGE:
> +			/* VIN Scale: 1170mV / 2^23 */
> +			*val = 1170;
> +			*val2 = 23;
> +			ret = IIO_VAL_FRACTIONAL_LOG2;
> +			break;
> +		default:
> +			ret = -EINVAL;
> +			break;
> +		}
> +
> +		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;
> +		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;
> +		default:
> +			ret = -EINVAL;
> +		}
> +		break;
> +	default:
> +		ret = -EINVAL;
Comment above is partly about avoiding this tree of default statements.
There is little point in having a lock for these error cases.

> +	}
> +out:
> +	mutex_unlock(&chip->lock);
> +	return ret;
> +}
> +
> +static const struct iio_info ad7746_info = {
> +	.attrs = &ad7746_attribute_group,
> +	.read_raw = ad7746_read_raw,
> +	.write_raw = ad7746_write_raw,
> +};
> +
> +/*
> + * device probe and remove
> + */
> +#ifdef CONFIG_OF
> +static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct ad7746_platform_data *pdata;
> +	unsigned int tmp;
> +	int ret;
> +
> +	/*
> +	 * The default excitation outputs are not inverted, it should be stated
> +	 * in the dt if needed.
> +	 */
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +
> +	ret = of_property_read_u32(np, "adi,exclvl", &tmp);
> +	if (ret || tmp > 3) {
> +		dev_warn(dev, "Wrong exclvl value, using default\n");
> +		pdata->exclvl = 3;
> +	} else {
> +		pdata->exclvl = tmp;
> +	}
> +
> +	pdata->exca_en = true;
> +	pdata->excb_en = true;
> +	pdata->exca_inv_en = of_property_read_bool(np, "adi,nexca_en");
> +	pdata->excb_inv_en = of_property_read_bool(np, "adi,nexcb_en");
> +
> +	return pdata;
> +}
> +#else
> +static struct ad7746_platform_data *ad7746_parse_dt(struct device *dev)
> +{
> +	return NULL;
> +}
> +#endif
> +
> +static int ad7746_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct ad7746_platform_data *pdata = client->dev.platform_data;
> +	struct ad7746_chip_info *chip;
> +	struct iio_dev *indio_dev;
> +	unsigned char regval = 0;
> +	int ret = 0;

There doesn't seem to be a path on which ret isn't set to something else.

> +
> +	if (client->dev.of_node)
> +		pdata = ad7746_parse_dt(&client->dev);
> +	else
> +		pdata = client->dev.platform_data;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> +	if (!indio_dev)
> +		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;
> +
> +	/* Establish that the iio_dev is a child of the i2c device */
> +	indio_dev->name = id->name;
> +	indio_dev->dev.parent = &client->dev;
> +	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->num_channels = ARRAY_SIZE(ad7746_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	if (pdata) {
> +		if (pdata->exca_en) {
> +			if (pdata->exca_inv_en)
> +				regval |= AD7746_EXCSETUP_NEXCA;
> +			else
> +				regval |= AD7746_EXCSETUP_EXCA;
> +		}
> +
> +		if (pdata->excb_en) {
> +			if (pdata->excb_inv_en)
> +				regval |= AD7746_EXCSETUP_NEXCB;
> +			else
> +				regval |= AD7746_EXCSETUP_EXCB;
> +		}
> +
> +		regval |= AD7746_EXCSETUP_EXCLVL(pdata->exclvl);
> +	} else {
> +		dev_warn(&client->dev, "No platform data? using default\n");
> +		regval = AD7746_EXCSETUP_EXCA | AD7746_EXCSETUP_EXCB |
> +			AD7746_EXCSETUP_EXCLVL(3);
> +	}
> +
> +	ret = i2c_smbus_write_byte_data(chip->client,
> +					AD7746_REG_EXC_SETUP, regval);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
If it is worth turning the power off in the remove, then it is worth
turning it off on error in probe.

Also, return devm_iio_device_register would other otherwise be neater.

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int ad7746_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct ad7746_chip_info *chip = iio_priv(indio_dev);
> +	unsigned char regval;
> +	int ret;
> +
> +	mutex_lock(&chip->lock);
> +
> +	regval = chip->config | AD7746_CONF_MODE_PWRDN;
> +	ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
> +
> +	mutex_unlock(&chip->lock);
> +
> +	if (ret < 0) {
> +		dev_warn(&client->dev, "Could NOT Power Down!\n");
> +		goto out;
> +	}
> +
> +	iio_device_unregister(indio_dev);
> +
> +out:
> +	return ret;
> +}
> +
> +static const struct i2c_device_id ad7746_id[] = {
> +	{ "ad7745", 7745 },
> +	{ "ad7746", 7746 },
> +	{ "ad7747", 7747 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, ad7746_id);
> +
> +#ifdef CONFIG_OF
Mark it __maybe_unused and drop the ifdef (I missed this
in the patch that introduced it).  
The size cost is tiny, for a gain in readability.

> +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);
> +#endif
> +
> +static struct i2c_driver ad7746_driver = {
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = of_match_ptr(ad7746_of_match),
> +	},
> +	.probe = ad7746_probe,
> +	.remove = ad7746_remove,
> +	.id_table = ad7746_id,
> +};
> +module_i2c_driver(ad7746_driver);
> +
> +MODULE_AUTHOR("Michael Hennerich <hennerich@blackfin.uclinux.org>");
> +MODULE_DESCRIPTION("Analog Devices AD7746/5/7 capacitive sensor driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/iio/cdc/ad7746.h b/include/linux/iio/cdc/ad7746.h
> new file mode 100644
> index 0000000..46ff25e
> --- /dev/null
> +++ b/include/linux/iio/cdc/ad7746.h
> @@ -0,0 +1,20 @@
> +/*
> + * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
> + *
> + * Copyright 2011 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#ifndef IIO_CDC_AD7746_H_
> +#define IIO_CDC_AD7746_H_
> +
> +struct ad7746_platform_data {
> +	unsigned char exclvl;	/*Excitation Voltage Level */
> +	bool exca_en;		/* enables EXCA pin as the excitation output */
> +	bool exca_inv_en;	/* enables /EXCA pin as the excitation output */
> +	bool excb_en;		/* enables EXCB pin as the excitation output */
> +	bool excb_inv_en;	/* enables /EXCB pin as the excitation output */
> +};
> +
> +#endif /* IIO_CDC_AD7746_H_ */

Re: [PATCH v2 13/14] Move ad7746 out of staging

[Joe Perches]

On Sun, 2018-04-15 at 18:04 +0100, Jonathan Cameron wrote:
> On Fri, 13 Apr 2018 13:36:50 -0300
> Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
> 
> > Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> 
> A few comments inline.

And a trivial typo and other bits

> > diff --git a/drivers/iio/cdc/Makefile b/drivers/iio/cdc/Makefile
[]
> > @@ -0,0 +1,5 @@
> > +#
> > +#Makeefile for industrial I/O CDC drivers

Makefile

> > diff --git a/drivers/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c
[]
> > @@ -0,0 +1,855 @@

Perhaps use the SPDX tags

> > +/*
> > + * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
> > + *
> > + * Copyright 2011 Analog Devices Inc.
> > + *
> > + * Licensed under the GPL-2.
> > + */

[]

> > +static const struct iio_chan_spec ad7746_channels[] = {
> > +	[VIN] = {
> > +		.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),
> > +		.address = AD7746_REG_VT_DATA_HIGH << 8 |
> > +			AD7746_VTSETUP_VTMD_EXT_VIN,
> 
> Hmm. I never like to see a single location used to hold two different things.
> I would suggest perhaps having address be an enum then have have a lookup
> into an array of structures that have the two elements separately.
> (use the ad7746_chan enum again for this?)

And perhaps it's nicer to align the multiple
BIT(identifier) uses like

		.info_mask_shared_by_type = (BIT(IIO_CHAN_INFO_SCALE) |
 					     BIT(IIO_CHAN_INFO_SAMP_FREQ)),

The extra unnecessary parentheses allow at least emacs
to align the BIT uses properly.

[]

> > +	[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),
> > +		.address = AD7746_REG_CAP_DATA_HIGH << 8,
> > +	},

So this one could be:

		.info_mask_shared_by_type = (BIT(IIO_CHAN_INFO_CALIBBIAS) |
					     BIT(IIO_CHAN_INFO_SCALE) |
					     BIT(IIO_CHAN_INFO_SAMP_FREQ)),

etc...

Re: [PATCH v2 14/14] staging: iio: Remove ad7746 from staging

[Joe Perches]

On Sun, 2018-04-15 at 16:43 +0100, Jonathan Cameron wrote:
> On Fri, 13 Apr 2018 13:36:51 -0300
> Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
> 
> > Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> 
> Please have a single patch (with move detection turned off) for this and the
> previous.

> Allows easy review by showing us the code but doesn't lead to a stage in which
> two different versions will build and hence probably break bisectiblity.

Moving both files in a single patch is fine but
move detection off is much harder to review.

Re: [PATCH v2 13/14] Move ad7746 out of staging

[Jonathan Cameron]

On Sun, 15 Apr 2018 11:46:09 -0700
Joe Perches <joe@perches.com> wrote:

> On Sun, 2018-04-15 at 18:04 +0100, Jonathan Cameron wrote:
> > On Fri, 13 Apr 2018 13:36:50 -0300
> > Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
> >   
> > > Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>  
> > 
> > A few comments inline.  
> 
> And a trivial typo and other bits
> 
> > > diff --git a/drivers/iio/cdc/Makefile b/drivers/iio/cdc/Makefile  
> []
> > > @@ -0,0 +1,5 @@
> > > +#
> > > +#Makeefile for industrial I/O CDC drivers  
> 
> Makefile
> 
> > > diff --git a/drivers/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c  
> []
> > > @@ -0,0 +1,855 @@  
> 
> Perhaps use the SPDX tags
There was some resistance around this so we dropped it from suggested changes
before moving these drivers out of staging.  Can sort it out later.

> 
> > > +/*
> > > + * AD7746 capacitive sensor driver supporting AD7745, AD7746 and AD7747
> > > + *
> > > + * Copyright 2011 Analog Devices Inc.
> > > + *
> > > + * Licensed under the GPL-2.
> > > + */  
> 
> []
> 
> > > +static const struct iio_chan_spec ad7746_channels[] = {
> > > +	[VIN] = {
> > > +		.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),
> > > +		.address = AD7746_REG_VT_DATA_HIGH << 8 |
> > > +			AD7746_VTSETUP_VTMD_EXT_VIN,  
> > 
> > Hmm. I never like to see a single location used to hold two different things.
> > I would suggest perhaps having address be an enum then have have a lookup
> > into an array of structures that have the two elements separately.
> > (use the ad7746_chan enum again for this?)  
> 
> And perhaps it's nicer to align the multiple
> BIT(identifier) uses like
> 
> 		.info_mask_shared_by_type = (BIT(IIO_CHAN_INFO_SCALE) |
>  					     BIT(IIO_CHAN_INFO_SAMP_FREQ)),
> 
> The extra unnecessary parentheses allow at least emacs
> to align the BIT uses properly.
> 
> []
> 
> > > +	[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),
> > > +		.address = AD7746_REG_CAP_DATA_HIGH << 8,
> > > +	},  
> 
> So this one could be:
> 
> 		.info_mask_shared_by_type = (BIT(IIO_CHAN_INFO_CALIBBIAS) |
> 					     BIT(IIO_CHAN_INFO_SCALE) |
> 					     BIT(IIO_CHAN_INFO_SAMP_FREQ)),
> 
> etc...
> 
> 

Re: [PATCH v2 14/14] staging: iio: Remove ad7746 from staging

[Jonathan Cameron]

On Sun, 15 Apr 2018 12:24:23 -0700
Joe Perches <joe@perches.com> wrote:

> On Sun, 2018-04-15 at 16:43 +0100, Jonathan Cameron wrote:
> > On Fri, 13 Apr 2018 13:36:51 -0300
> > Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
> >   
> > > Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>  
> > 
> > Please have a single patch (with move detection turned off) for this and the
> > previous.  
> 
> > Allows easy review by showing us the code but doesn't lead to a stage in which
> > two different versions will build and hence probably break bisectiblity.  
> 
> Moving both files in a single patch is fine but
> move detection off is much harder to review.
> 
Not in this one case.   Here we want explicitly to review the code,
not the move.   The point is to put the whole driver code in front
of people as part of the proposal to move it out of staging.  If we
leave move detection on then there is a barrier to review as only
someone who has actually applied all the patches will be able to
see what state the driver is in.

Sure, side effect is we loose the check on whether it is a 'pure'
move or not, but that is easy to check when I finally apply the patches.

I haven't come up with a better way of reviewing a staging driver
on the mailing lists in a fashion similar to how we would review
a newly submitted driver.

Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 03/14] staging: iio: ad7746: Fix bound checkings

[Hernán Gonzalez]

On Sun, Apr 15, 2018 at 12:05 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 13 Apr 2018 13:36:40 -0300
> Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
>
>> Also remove unnecessary parenthesis
> I am probably missing something.  I'm not sure what you mean
> by fix bound checking?   There are superfluous brackets, but
> I don't see any functional change to indicate there was anything
> wrong with the original checks.
>

Maybe I'm wrong but | is a bitwise operator while || is a logical one.
There are no functional changes as you said but, from K&R, "One must
distinguish the bitwise operators & and | from the logical operators
&& and II, which imply left-to-right evaluation of a truth value. For
example, if x is 1 and y is 2, then x & y is zero while x && y is one"
so it'd be slightly faster if the first condition is true, and it
would be the "correct" operator to use in this case, even though it
doesn't affect the result.

>>
>> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
>> ---
>>  drivers/staging/iio/cdc/ad7746.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
>> index 516aa93..d793785 100644
>> --- a/drivers/staging/iio/cdc/ad7746.c
>> +++ b/drivers/staging/iio/cdc/ad7746.c
>> @@ -458,7 +458,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
>>               ret = 0;
>>               break;
>>       case IIO_CHAN_INFO_CALIBBIAS:
>> -             if ((val < 0) | (val > 0xFFFF)) {
>> +             if (val < 0 || val > 0xFFFF) {
>>                       ret = -EINVAL;
>>                       goto out;
>>               }
>> @@ -470,7 +470,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
>>               ret = 0;
>>               break;
>>       case IIO_CHAN_INFO_OFFSET:
>> -             if ((val < 0) | (val > 43008000)) { /* 21pF */
>> +             if (val < 0 || val > 43008000) { /* 21pF */
>>                       ret = -EINVAL;
>>                       goto out;
>>               }
>

Re: [PATCH v2 07/14] staging: iio: ad7746: Remove unused defines

[Hernán Gonzalez]

You're right, got confused from the macro defined in the .c file. I'll
leave this alone on the next series
Thanks!

On Sun, Apr 15, 2018 at 12:12 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 13 Apr 2018 13:36:44 -0300
> Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
>
>> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
>> ---
>>  drivers/staging/iio/cdc/ad7746.c | 7 -------
>>  drivers/staging/iio/cdc/ad7746.h | 5 -----
>>  2 files changed, 12 deletions(-)
>>
>> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
>> index f53612a..d39ab34 100644
>> --- a/drivers/staging/iio/cdc/ad7746.c
>> +++ b/drivers/staging/iio/cdc/ad7746.c
>> @@ -25,7 +25,6 @@
>>   * 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
>> @@ -38,12 +37,6 @@
>>  #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)
>> -
> There is a pretty strong argument that the driver 'should' be
> checking the status register...
>
> I would leave it the defines here.  When they are acting
> as 'documentation' of the register layout of a device, they
> do little harm and can be very useful.
>
>>  /* Capacitive Channel Setup Register Bit Designations (AD7746_REG_CAP_SETUP) */
>>  #define AD7746_CAPSETUP_CAPEN                BIT(7)
>>  #define AD7746_CAPSETUP_CIN2         BIT(6) /* AD7746 only */
>> diff --git a/drivers/staging/iio/cdc/ad7746.h b/drivers/staging/iio/cdc/ad7746.h
>> index ea8572d..2fbcee8 100644
>> --- a/drivers/staging/iio/cdc/ad7746.h
>> +++ b/drivers/staging/iio/cdc/ad7746.h
>> @@ -13,11 +13,6 @@
>>   * TODO: struct ad7746_platform_data needs to go into include/linux/iio
>>   */
>>
>> -#define AD7466_EXCLVL_0              0 /* +-VDD/8 */
>> -#define AD7466_EXCLVL_1              1 /* +-VDD/4 */
>> -#define AD7466_EXCLVL_2              2 /* +-VDD * 3/8 */
>> -#define AD7466_EXCLVL_3              3 /* +-VDD/2 */
>> -
>
> Think about what these are for.... They aren't unused
> if you are actually using platform data on a given oard.
>
>>  struct ad7746_platform_data {
>>       unsigned char exclvl;   /*Excitation Voltage Level */
>>       bool exca_en;           /* enables EXCA pin as the excitation output */
>

Re: [PATCH v2 11/14] staging: iio: ad7746: Add devicetree bindings documentation

[Hernán Gonzalez]

On Sun, Apr 15, 2018 at 12:37 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 13 Apr 2018 13:36:48 -0300
> Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> A few comments inline.
>
> Thanks,
>
> Jonathan
>
>> ---
>>  .../devicetree/bindings/staging/iio/cdc/ad7746.txt | 34 ++++++++++++++++++++++
>>  1 file changed, 34 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
>>
>> diff --git a/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
>> new file mode 100644
>> index 0000000..7740f05
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
>> @@ -0,0 +1,34 @@
>> +Analog Devices AD7746/5/7 capacitive sensor driver
>> +
>> +Required properties:
>> +     - compatible: Should be one of
>> +             * "adi,ad7745"
>> +             * "adi,ad7746"
>> +       * "adi,ad7747"
>> +     - reg: The 7-bits long I2c address of the device
>> +
>> +Optional properties:
>> +     - adi,exclvl: This property defines the excitation voltage level for the
>> +    capacitance to be measured. Possible values are:
>> +      * 0 = +-VDD/8
>> +      * 1 = +-VDD/4
>> +      * 2 = +-VDD * 3/8
>> +      * 3 = +-VDD/2 (Default)
>> +     - adi,nexca_en: Invert excitation output A.
>
> The naming is rather odd.  Why _en?
>

Just sticking to the original pdata naming. I'll rename it on next series.

>> +     - adi,nexcb_en: Invert excitation output B.
>> +
>> +Example:
>> +Here exclvl would be 1 (VDD/4), Excitation pin A would be inverted and
>> +Excitation pin B would NOT be inverted.
>> +
>> +i2c2 {
>> +
>> +      < . . . >
> What does this spacer add to the example?
> Sure, there can be other devices, but no need to illustrate that here.
>
>> +
>> +      ad7746: ad7746@60 {
>> +              compatible = "ad7746";
>> +              reg = <0x60>;
>> +              adi,exclvl = <1>;
>> +              adi,nexca_en;
>> +      };
>> +};
>

Re: [PATCH v2 11/14] staging: iio: ad7746: Add devicetree bindings documentation

[Rob Herring]

On Fri, Apr 13, 2018 at 01:36:48PM -0300, Hernán Gonzalez wrote:

Commit msg?

> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> ---
>  .../devicetree/bindings/staging/iio/cdc/ad7746.txt | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
> 
> diff --git a/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
> new file mode 100644
> index 0000000..7740f05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/staging/iio/cdc/ad7746.txt
> @@ -0,0 +1,34 @@
> +Analog Devices AD7746/5/7 capacitive sensor driver
> +
> +Required properties:
> +	- compatible: Should be one of
> +		* "adi,ad7745"
> +		* "adi,ad7746"
> +	  * "adi,ad7747"

Alignment.

> +	- reg: The 7-bits long I2c address of the device
> +
> +Optional properties:
> +	- adi,exclvl: This property defines the excitation voltage level for the
> +    capacitance to be measured. Possible values are:
> +      * 0 = +-VDD/8
> +      * 1 = +-VDD/4
> +      * 2 = +-VDD * 3/8
> +      * 3 = +-VDD/2 (Default)
> +	- adi,nexca_en: Invert excitation output A.
> +	- adi,nexcb_en:	Invert excitation output B.

s/_/-/

> +
> +Example:
> +Here exclvl would be 1 (VDD/4), Excitation pin A would be inverted and
> +Excitation pin B would NOT be inverted.
> +
> +i2c2 {
> +
> +      < . . . >
> +
> +      ad7746: ad7746@60 {
> +              compatible = "ad7746";
> +              reg = <0x60>;
> +              adi,exclvl = <1>;
> +              adi,nexca_en;
> +      };
> +};
> -- 
> 2.7.4
> 

Re: [PATCH v2 03/14] staging: iio: ad7746: Fix bound checkings

[Jonathan Cameron]

On Mon, 16 Apr 2018 11:47:05 -0300
Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:

> On Sun, Apr 15, 2018 at 12:05 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> > On Fri, 13 Apr 2018 13:36:40 -0300
> > Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
> >  
> >> Also remove unnecessary parenthesis  
> > I am probably missing something.  I'm not sure what you mean
> > by fix bound checking?   There are superfluous brackets, but
> > I don't see any functional change to indicate there was anything
> > wrong with the original checks.
> >  
> 
> Maybe I'm wrong but | is a bitwise operator while || is a logical one.
> There are no functional changes as you said but, from K&R, "One must
> distinguish the bitwise operators & and | from the logical operators
> && and II, which imply left-to-right evaluation of a truth value. For
> example, if x is 1 and y is 2, then x & y is zero while x && y is one"
> so it'd be slightly faster if the first condition is true, and it
> would be the "correct" operator to use in this case, even though it
> doesn't affect the result.
Got you, I missed the operator change entirely. Doh.

Jonathan

> 
> >>
> >> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> >> ---
> >>  drivers/staging/iio/cdc/ad7746.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> >> index 516aa93..d793785 100644
> >> --- a/drivers/staging/iio/cdc/ad7746.c
> >> +++ b/drivers/staging/iio/cdc/ad7746.c
> >> @@ -458,7 +458,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
> >>               ret = 0;
> >>               break;
> >>       case IIO_CHAN_INFO_CALIBBIAS:
> >> -             if ((val < 0) | (val > 0xFFFF)) {
> >> +             if (val < 0 || val > 0xFFFF) {
> >>                       ret = -EINVAL;
> >>                       goto out;
> >>               }
> >> @@ -470,7 +470,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
> >>               ret = 0;
> >>               break;
> >>       case IIO_CHAN_INFO_OFFSET:
> >> -             if ((val < 0) | (val > 43008000)) { /* 21pF */
> >> +             if (val < 0 || val > 43008000) { /* 21pF */
> >>                       ret = -EINVAL;
> >>                       goto out;
> >>               }  
> >  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 09/14] staging: iio: ad7746: Add remove()

[Hernán Gonzalez]

On Sun, Apr 15, 2018 at 12:31 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 13 Apr 2018 13:36:46 -0300
> Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
>
>> This allows the driver to be probed and removed as a module powering it
>> down on remove().
>>
>> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
>> ---
>>  drivers/staging/iio/cdc/ad7746.c | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
>> index c29a221..05506bf9 100644
>> --- a/drivers/staging/iio/cdc/ad7746.c
>> +++ b/drivers/staging/iio/cdc/ad7746.c
>> @@ -775,6 +775,31 @@ static int ad7746_probe(struct i2c_client *client,
>>       return 0;
>>  }
>>
>> +static int ad7746_remove(struct i2c_client *client)
>> +{
>> +     struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> +     struct ad7746_chip_info *chip = iio_priv(indio_dev);
>> +     unsigned char regval;
>> +     int ret;
>> +
>> +     mutex_lock(&chip->lock);
>> +
>> +     regval = chip->config | AD7746_CONF_MODE_PWRDN;
>> +     ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
> As this is a one off operation, perhaps it would be better to factor
> it out to a utility function then use devm_add_action_or_reset in
> the probe?
>
> Also, I am nervous about this change as there doesn't seem to be
> path in probe by which this is deliberately reversed?
> It seems to be 'accidentally' handled by the read_raw write to the
> CFG register.
>
> The data sheet doesn't really mention much about this register
> at all.  It may have special requirements to exit from power down - I have
> no idea, but if it is costless, why do we bother with idle mode?
>
> Perhaps Michael can confirm if this is safe to do or not.
>
>

I guess it'll be better to just drop this until Michael answers. I've
been trying to get a hold of the hw but with no success (or I have to
pay 3 times its cost in shipping), will keep searching though.

>> +
>> +     mutex_unlock(&chip->lock);
>> +
>> +     if (ret < 0) {
>> +             dev_warn(&client->dev, "Could NOT Power Down!\n");
>> +             goto out;
>> +     }
>> +
>> +     iio_device_unregister(indio_dev);
> You can't safely do this against the devm_iio_device_register.
>
>> +
>> +out:
>> +     return ret;
>> +}
>> +
>>  static const struct i2c_device_id ad7746_id[] = {
>>       { "ad7745", 7745 },
>>       { "ad7746", 7746 },
>> @@ -799,6 +824,7 @@ static struct i2c_driver ad7746_driver = {
>>               .of_match_table = of_match_ptr(ad7746_of_match),
>>       },
>>       .probe = ad7746_probe,
>> +     .remove = ad7746_remove,
>>       .id_table = ad7746_id,
>>  };
>>  module_i2c_driver(ad7746_driver);
>

Re: [PATCH v2 09/14] staging: iio: ad7746: Add remove()

[Michael Hennerich]

On 18.04.2018 21:25, Hernán Gonzalez wrote:
> On Sun, Apr 15, 2018 at 12:31 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On Fri, 13 Apr 2018 13:36:46 -0300
>> Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
>>
>>> This allows the driver to be probed and removed as a module powering it
>>> down on remove().
>>>
>>> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
>>> ---
>>>   drivers/staging/iio/cdc/ad7746.c | 26 ++++++++++++++++++++++++++
>>>   1 file changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
>>> index c29a221..05506bf9 100644
>>> --- a/drivers/staging/iio/cdc/ad7746.c
>>> +++ b/drivers/staging/iio/cdc/ad7746.c
>>> @@ -775,6 +775,31 @@ static int ad7746_probe(struct i2c_client *client,
>>>        return 0;
>>>   }
>>>
>>> +static int ad7746_remove(struct i2c_client *client)
>>> +{
>>> +     struct iio_dev *indio_dev = i2c_get_clientdata(client);
>>> +     struct ad7746_chip_info *chip = iio_priv(indio_dev);
>>> +     unsigned char regval;
>>> +     int ret;
>>> +
>>> +     mutex_lock(&chip->lock);
>>> +
>>> +     regval = chip->config | AD7746_CONF_MODE_PWRDN;
>>> +     ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
>> As this is a one off operation, perhaps it would be better to factor
>> it out to a utility function then use devm_add_action_or_reset in
>> the probe?
>>
>> Also, I am nervous about this change as there doesn't seem to be
>> path in probe by which this is deliberately reversed?
>> It seems to be 'accidentally' handled by the read_raw write to the
>> CFG register.
>>
>> The data sheet doesn't really mention much about this register
>> at all.  It may have special requirements to exit from power down - I have
>> no idea, but if it is costless, why do we bother with idle mode?
>>
>> Perhaps Michael can confirm if this is safe to do or not.
>>
>>
> 
> I guess it'll be better to just drop this until Michael answers. I've
> been trying to get a hold of the hw but with no success (or I have to
> pay 3 times its cost in shipping), will keep searching though.


It's some time since I last worked with the device. I think it would be 
safe to restore the power on default in the configuration register upon 
probe. Which would be idle state. Besides a unspecified delay, I don't 
think there is anything else to handle here. Due to fact it's not 
specified it might not be required at all.
If your planning to do further cleanup on this driver, I ship you an 
eval board free of charge. Feel free to contact me off-list.


> 
>>> +
>>> +     mutex_unlock(&chip->lock);
>>> +
>>> +     if (ret < 0) {
>>> +             dev_warn(&client->dev, "Could NOT Power Down!\n");
>>> +             goto out;
>>> +     }
>>> +
>>> +     iio_device_unregister(indio_dev);
>> You can't safely do this against the devm_iio_device_register.
>>
>>> +
>>> +out:
>>> +     return ret;
>>> +}
>>> +
>>>   static const struct i2c_device_id ad7746_id[] = {
>>>        { "ad7745", 7745 },
>>>        { "ad7746", 7746 },
>>> @@ -799,6 +824,7 @@ static struct i2c_driver ad7746_driver = {
>>>                .of_match_table = of_match_ptr(ad7746_of_match),
>>>        },
>>>        .probe = ad7746_probe,
>>> +     .remove = ad7746_remove,
>>>        .id_table = ad7746_id,
>>>   };
>>>   module_i2c_driver(ad7746_driver);
>>
> 


-- 
Greetings,
Michael

--
Analog Devices GmbH      Otl-Aicher Strasse 60-64      80807 München
Sitz der Gesellschaft München, Registergericht München HRB 40368,
Geschäftsführer: Peter Kolberg, Ali Raza Husain, Eileen Wynne

Re: [PATCH v2 03/14] staging: iio: ad7746: Fix bound checkings

[Jonathan Cameron]

On Wed, 18 Apr 2018 10:39:48 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> On Mon, 16 Apr 2018 11:47:05 -0300
> Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
> 
> > On Sun, Apr 15, 2018 at 12:05 PM, Jonathan Cameron <jic23@kernel.org> wrote:  
> > > On Fri, 13 Apr 2018 13:36:40 -0300
> > > Hernán Gonzalez <hernan@vanguardiasur.com.ar> wrote:
> > >    
> > >> Also remove unnecessary parenthesis    
> > > I am probably missing something.  I'm not sure what you mean
> > > by fix bound checking?   There are superfluous brackets, but
> > > I don't see any functional change to indicate there was anything
> > > wrong with the original checks.
> > >    
> > 
> > Maybe I'm wrong but | is a bitwise operator while || is a logical one.
> > There are no functional changes as you said but, from K&R, "One must
> > distinguish the bitwise operators & and | from the logical operators
> > && and II, which imply left-to-right evaluation of a truth value. For
> > example, if x is 1 and y is 2, then x & y is zero while x && y is one"
> > so it'd be slightly faster if the first condition is true, and it
> > would be the "correct" operator to use in this case, even though it
> > doesn't affect the result.  
> Got you, I missed the operator change entirely. Doh.
> 
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> Jonathan
> 
> >   
> > >>
> > >> Signed-off-by: Hernán Gonzalez <hernan@vanguardiasur.com.ar>
> > >> ---
> > >>  drivers/staging/iio/cdc/ad7746.c | 4 ++--
> > >>  1 file changed, 2 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> > >> index 516aa93..d793785 100644
> > >> --- a/drivers/staging/iio/cdc/ad7746.c
> > >> +++ b/drivers/staging/iio/cdc/ad7746.c
> > >> @@ -458,7 +458,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
> > >>               ret = 0;
> > >>               break;
> > >>       case IIO_CHAN_INFO_CALIBBIAS:
> > >> -             if ((val < 0) | (val > 0xFFFF)) {
> > >> +             if (val < 0 || val > 0xFFFF) {
> > >>                       ret = -EINVAL;
> > >>                       goto out;
> > >>               }
> > >> @@ -470,7 +470,7 @@ static int ad7746_write_raw(struct iio_dev *indio_dev,
> > >>               ret = 0;
> > >>               break;
> > >>       case IIO_CHAN_INFO_OFFSET:
> > >> -             if ((val < 0) | (val > 43008000)) { /* 21pF */
> > >> +             if (val < 0 || val > 43008000) { /* 21pF */
> > >>                       ret = -EINVAL;
> > >>                       goto out;
> > >>               }    
> > >    
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html