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
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 <[email protected]>
---
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
Clear a couple more checkpatch.pl CHECKS.
Signed-off-by: Hernán Gonzalez <[email protected]>
---
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
Signed-off-by: Hernán Gonzalez <[email protected]>
---
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
Reorder some variable declarations in an inverse-pyramid scheme.
Signed-off-by: Hernán Gonzalez <[email protected]>
---
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
Signed-off-by: Hernán Gonzalez <[email protected]>
---
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
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 <[email protected]>
---
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
Add comments to clarify some of the calculations made, specially when
reading or writing values.
Signed-off-by: Hernán Gonzalez <[email protected]>
---
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
Cc: Rob Herring <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: [email protected]
Signed-off-by: Hernán Gonzalez <[email protected]>
---
.../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
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 <[email protected]>
---
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: [email protected]
+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: [email protected]
+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
Signed-off-by: Hernán Gonzalez <[email protected]>
---
.../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 <[email protected]>");
+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
Signed-off-by: Hernán Gonzalez <[email protected]>
---
.../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 <[email protected]>");
-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
Also remove unnecessary parenthesis
Signed-off-by: Hernán Gonzalez <[email protected]>
---
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
This allows the driver to be probed and removed as a module powering it
down on remove().
Signed-off-by: Hernán Gonzalez <[email protected]>
---
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
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 <[email protected]>
---
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
On Fri, 13 Apr 2018 13:36:38 -0300
Hernán Gonzalez <[email protected]> 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 <[email protected]>
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;
On Fri, 13 Apr 2018 13:36:39 -0300
Hernán Gonzalez <[email protected]> wrote:
> Clear a couple more checkpatch.pl CHECKS.
>
> Signed-off-by: Hernán Gonzalez <[email protected]>
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;
On Fri, 13 Apr 2018 13:36:40 -0300
Hernán Gonzalez <[email protected]> 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 <[email protected]>
> ---
> 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;
> }
On Fri, 13 Apr 2018 13:36:41 -0300
Hernán Gonzalez <[email protected]> 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 <[email protected]>
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,
> };
>
On Fri, 13 Apr 2018 13:36:42 -0300
Hernán Gonzalez <[email protected]> wrote:
> Signed-off-by: Hernán Gonzalez <[email protected]>
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>
On Fri, 13 Apr 2018 13:36:43 -0300
Hernán Gonzalez <[email protected]> wrote:
> Reorder some variable declarations in an inverse-pyramid scheme.
>
> Signed-off-by: Hernán Gonzalez <[email protected]>
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)
On Fri, 13 Apr 2018 13:36:44 -0300
Hernán Gonzalez <[email protected]> wrote:
> Signed-off-by: Hernán Gonzalez <[email protected]>
> ---
> 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 */
On Fri, 13 Apr 2018 13:36:45 -0300
Hernán Gonzalez <[email protected]> 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 <[email protected]>
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,
On Fri, 13 Apr 2018 13:36:46 -0300
Hernán Gonzalez <[email protected]> wrote:
> This allows the driver to be probed and removed as a module powering it
> down on remove().
>
> Signed-off-by: Hernán Gonzalez <[email protected]>
> ---
> 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);
On Fri, 13 Apr 2018 13:36:47 -0300
Hernán Gonzalez <[email protected]> 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 <[email protected]>
> ---
> 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;
> }
On Fri, 13 Apr 2018 13:36:48 -0300
Hernán Gonzalez <[email protected]> wrote:
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: [email protected]
> Signed-off-by: Hernán Gonzalez <[email protected]>
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;
> + };
> +};
On Fri, 13 Apr 2018 13:36:49 -0300
Hernán Gonzalez <[email protected]> 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 <[email protected]>
> ---
> 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: [email protected]
> +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: [email protected]
> +Description:
> + Enter offset calibration mode, in which a zero-scale
> + capacitance must be connected to the capacitance input
> + beforehand to automatically calibrate the device.
On Fri, 13 Apr 2018 13:36:51 -0300
Hernán Gonzalez <[email protected]> wrote:
> Signed-off-by: Hernán Gonzalez <[email protected]>
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 <[email protected]>");
> -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_ */
On Fri, 13 Apr 2018 13:36:50 -0300
Hernán Gonzalez <[email protected]> wrote:
> Signed-off-by: Hernán Gonzalez <[email protected]>
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 <[email protected]>");
> +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_ */
On Sun, 2018-04-15 at 18:04 +0100, Jonathan Cameron wrote:
> On Fri, 13 Apr 2018 13:36:50 -0300
> Hern?n Gonzalez <[email protected]> wrote:
>
> > Signed-off-by: Hern?n Gonzalez <[email protected]>
>
> 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...
On Sun, 2018-04-15 at 16:43 +0100, Jonathan Cameron wrote:
> On Fri, 13 Apr 2018 13:36:51 -0300
> Hern?n Gonzalez <[email protected]> wrote:
>
> > Signed-off-by: Hern?n Gonzalez <[email protected]>
>
> 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.
On Sun, 15 Apr 2018 12:24:23 -0700
Joe Perches <[email protected]> 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 <[email protected]> wrote:
> >
> > > Signed-off-by: Hernán Gonzalez <[email protected]>
> >
> > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 15 Apr 2018 11:46:09 -0700
Joe Perches <[email protected]> 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 <[email protected]> wrote:
> >
> > > Signed-off-by: Hernán Gonzalez <[email protected]>
> >
> > 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...
>
>
On Sun, Apr 15, 2018 at 12:05 PM, Jonathan Cameron <[email protected]> wrote:
> On Fri, 13 Apr 2018 13:36:40 -0300
> Hernán Gonzalez <[email protected]> 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 <[email protected]>
>> ---
>> 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;
>> }
>
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 <[email protected]> wrote:
> On Fri, 13 Apr 2018 13:36:44 -0300
> Hernán Gonzalez <[email protected]> wrote:
>
>> Signed-off-by: Hernán Gonzalez <[email protected]>
>> ---
>> 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 */
>
On Sun, Apr 15, 2018 at 12:37 PM, Jonathan Cameron <[email protected]> wrote:
> On Fri, 13 Apr 2018 13:36:48 -0300
> Hernán Gonzalez <[email protected]> wrote:
>
>> Cc: Rob Herring <[email protected]>
>> Cc: Mark Rutland <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: Hernán Gonzalez <[email protected]>
> 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;
>> + };
>> +};
>
On Fri, Apr 13, 2018 at 01:36:48PM -0300, Hern?n Gonzalez wrote:
Commit msg?
> Cc: Rob Herring <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: [email protected]
> Signed-off-by: Hern?n Gonzalez <[email protected]>
> ---
> .../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
>
On Mon, 16 Apr 2018 11:47:05 -0300
Hern?n Gonzalez <[email protected]> wrote:
> On Sun, Apr 15, 2018 at 12:05 PM, Jonathan Cameron <[email protected]> wrote:
> > On Fri, 13 Apr 2018 13:36:40 -0300
> > Hern?n Gonzalez <[email protected]> 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 <[email protected]>
> >> ---
> >> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Apr 15, 2018 at 12:31 PM, Jonathan Cameron <[email protected]> wrote:
> On Fri, 13 Apr 2018 13:36:46 -0300
> Hernán Gonzalez <[email protected]> wrote:
>
>> This allows the driver to be probed and removed as a module powering it
>> down on remove().
>>
>> Signed-off-by: Hernán Gonzalez <[email protected]>
>> ---
>> 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);
>
On 18.04.2018 21:25, Hernán Gonzalez wrote:
> On Sun, Apr 15, 2018 at 12:31 PM, Jonathan Cameron <[email protected]> wrote:
>> On Fri, 13 Apr 2018 13:36:46 -0300
>> Hernán Gonzalez <[email protected]> wrote:
>>
>>> This allows the driver to be probed and removed as a module powering it
>>> down on remove().
>>>
>>> Signed-off-by: Hernán Gonzalez <[email protected]>
>>> ---
>>> 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
On Wed, 18 Apr 2018 10:39:48 +0100
Jonathan Cameron <[email protected]> wrote:
> On Mon, 16 Apr 2018 11:47:05 -0300
> Hernán Gonzalez <[email protected]> wrote:
>
> > On Sun, Apr 15, 2018 at 12:05 PM, Jonathan Cameron <[email protected]> wrote:
> > > On Fri, 13 Apr 2018 13:36:40 -0300
> > > Hernán Gonzalez <[email protected]> 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 <[email protected]>
> > >> ---
> > >> 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 [email protected]
> > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html