All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate
@ 2021-02-07 15:45 Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 01/24] staging:iio:cdc:ad7150: use swapped reads for i2c rather than open coding Jonathan Cameron
                   ` (24 more replies)
  0 siblings, 25 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:45 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

This is an 'old' driver in IIO that has been in staging a while.
First submitted in October 2010.

I wanted to try and experiment and picked this driver to try it with.

The cleanup etc here was all tested against some basic emulation
added to QEMU rather than real hardware.  Once I've cleaned that up
a tiny bit I'll push it up to https://github.com/jic23/qemu
Note that for now I'm not proposing to upstream this to QEMU but
would be interested in hearing if people thing it is a good idea to
do so.

Whilst it's obviously hard to be absolutely sure that the emulation is
correct, the fact that the original driver worked as expected and the
cleaned up version still does is certainly encouraging.

Note however, that there were a few more significant changes in here than
basic cleanup.
1. Interrupts / events were handled in a rather esoteric fashion.
   (Always on, window modes represented as magnitudes).
   Note that for two channel devices there are separate lines. The original
   driver was not supporting this at all.
   They now look more like a standard IIO driver and reflect experience
   that we've gained over the years in dealing with devices where these
   aren't interrupt lines as such, but rather reporters of current status.
2. Timeouts were handled in a fashion that clearly wouldn't work.

Note that this moving out of staging makes a few bits of ABI 'official'
and so those are added to the main IIO ABI Docs.

Thanks in advance to anyone who has time to take a look.

Jonathan


Jonathan Cameron (24):
  staging:iio:cdc:ad7150: use swapped reads for i2c rather than open
    coding.
  staging:iio:cdc:ad7150: Remove magnitude adaptive events
  staging:iio:cdc:ad7150: Refactor event parameter update
  staging:iio:cdc:ad7150: Timeout register covers both directions so
    both need updating
  staging:iio:cdc:ad7150: Drop platform data support
  staging:iio:cdc:ad7150: Handle variation in chan_spec across device
    and irq present or not
  staging:iio:cdc:ad7150: Simplify event handling by only using rising
    direction.
  staging:iio:cdc:ad7150: Drop noisy print in probe
  staging:iio:cdc:ad7150: Add sampling_frequency support
  iio:event: Add timeout event info type
  staging:iio:cdc:ad7150: Change timeout units to seconds and use core
    support
  staging:iio:cdc:ad7150: Rework interrupt handling.
  staging:iio:cdc:ad7150: More consistent register and field naming
  staging:iio:cdc:ad7150: Reorganize headers.
  staging:iio:cdc:ad7150: Tidy up local variable positioning.
  staging:iio:cdc:ad7150: Drop unnecessary block comments.
  staging:iio:cdc:ad7150: Shift the _raw readings by 4 bits.
  staging:iio:cdc:ad7150: Add scale and offset to
    info_mask_shared_by_type
  staging:iio:cdc:ad7150: Really basic regulator support.
  staging:iio:cdc:ad7150: Add of_match_table
  dt-bindings:iio:cdc:adi,ad7150 binding doc
  iio:Documentation:ABI Add missing elements as used by the adi,ad7150
  staging:iio:cdc:ad7150: Add copyright notice given substantial
    changes.
  iio:cdc:ad7150: Move driver out of staging.

 Documentation/ABI/testing/sysfs-bus-iio       |  33 +
 .../bindings/iio/cdc/adi,ad7150.yaml          |  69 ++
 drivers/iio/Kconfig                           |   1 +
 drivers/iio/Makefile                          |   1 +
 drivers/iio/cdc/Kconfig                       |  17 +
 drivers/iio/cdc/Makefile                      |   6 +
 drivers/iio/cdc/ad7150.c                      | 678 ++++++++++++++++++
 drivers/iio/industrialio-event.c              |   1 +
 drivers/staging/iio/cdc/Kconfig               |  10 -
 drivers/staging/iio/cdc/Makefile              |   3 +-
 drivers/staging/iio/cdc/ad7150.c              | 655 -----------------
 include/linux/iio/types.h                     |   1 +
 12 files changed, 808 insertions(+), 667 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/cdc/adi,ad7150.yaml
 create mode 100644 drivers/iio/cdc/Kconfig
 create mode 100644 drivers/iio/cdc/Makefile
 create mode 100644 drivers/iio/cdc/ad7150.c
 delete mode 100644 drivers/staging/iio/cdc/ad7150.c

-- 
2.30.0


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

* [PATCH 01/24] staging:iio:cdc:ad7150: use swapped reads for i2c rather than open coding.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 02/24] staging:iio:cdc:ad7150: Remove magnitude adaptive events Jonathan Cameron
                   ` (23 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Reduces boilerplate and chances of getting the error handling wrong.
No functional change.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 48132ab157ef..c3ed88c5e0a5 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -109,18 +109,20 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = i2c_smbus_read_word_data(chip->client,
-					       ad7150_addresses[channel][0]);
+		ret = i2c_smbus_read_word_swapped(chip->client,
+						  ad7150_addresses[channel][0]);
 		if (ret < 0)
 			return ret;
-		*val = swab16(ret);
+		*val = ret;
+
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_AVERAGE_RAW:
-		ret = i2c_smbus_read_word_data(chip->client,
-					       ad7150_addresses[channel][1]);
+		ret = i2c_smbus_read_word_swapped(chip->client,
+						  ad7150_addresses[channel][1]);
 		if (ret < 0)
 			return ret;
-		*val = swab16(ret);
+		*val = ret;
+
 		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
@@ -188,9 +190,9 @@ static int ad7150_write_event_params(struct iio_dev *indio_dev,
 		/* Note completely different from the adaptive versions */
 	case IIO_EV_TYPE_THRESH:
 		value = chip->threshold[rising][chan];
-		return i2c_smbus_write_word_data(chip->client,
-						 ad7150_addresses[chan][3],
-						 swab16(value));
+		return i2c_smbus_write_word_swapped(chip->client,
+						    ad7150_addresses[chan][3],
+						    value);
 	case IIO_EV_TYPE_MAG_ADAPTIVE:
 		sens = chip->mag_sensitivity[rising][chan];
 		timeout = chip->mag_timeout[rising][chan];
-- 
2.30.0


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

* [PATCH 02/24] staging:iio:cdc:ad7150: Remove magnitude adaptive events
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 01/24] staging:iio:cdc:ad7150: use swapped reads for i2c rather than open coding Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 03/24] staging:iio:cdc:ad7150: Refactor event parameter update Jonathan Cameron
                   ` (22 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

The devices support window detection, but that corresponds to
being outside of a range defined by a lower an uppper bound rather
than being related to magnitude as such.   Hence drop this interface
in the interests of making the driver ABI compliant.

We may bring back support for the window mode at somepoint in the future
but it will be in an ABI compliant fashion.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index c3ed88c5e0a5..4dac4aaec0cf 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -57,14 +57,9 @@
  * @threshold: thresholds for simple capacitance value events
  * @thresh_sensitivity: threshold for simple capacitance offset
  *	from 'average' value.
- * @mag_sensitity: threshold for magnitude of capacitance offset from
- *	from 'average' value.
  * @thresh_timeout: a timeout, in samples from the moment an
  *	adaptive threshold event occurs to when the average
  *	value jumps to current value.
- * @mag_timeout: a timeout, in sample from the moment an
- *	adaptive magnitude event occurs to when the average
- *	value jumps to the current value.
  * @old_state: store state from previous event, allowing confirmation
  *	of new condition.
  * @conversion_mode: the current conversion mode.
@@ -76,9 +71,7 @@ struct ad7150_chip_info {
 	u64 current_event;
 	u16 threshold[2][2];
 	u8 thresh_sensitivity[2][2];
-	u8 mag_sensitivity[2][2];
 	u8 thresh_timeout[2][2];
-	u8 mag_timeout[2][2];
 	int old_state;
 	char *conversion_mode;
 	struct mutex state_lock;
@@ -149,10 +142,6 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
 	thrfixed = FIELD_GET(AD7150_CFG_FIX, ret);
 
 	switch (type) {
-	case IIO_EV_TYPE_MAG_ADAPTIVE:
-		if (dir == IIO_EV_DIR_RISING)
-			return !thrfixed && (threshtype == 0x1);
-		return !thrfixed && (threshtype == 0x0);
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 		if (dir == IIO_EV_DIR_RISING)
 			return !thrfixed && (threshtype == 0x3);
@@ -193,10 +182,6 @@ static int ad7150_write_event_params(struct iio_dev *indio_dev,
 		return i2c_smbus_write_word_swapped(chip->client,
 						    ad7150_addresses[chan][3],
 						    value);
-	case IIO_EV_TYPE_MAG_ADAPTIVE:
-		sens = chip->mag_sensitivity[rising][chan];
-		timeout = chip->mag_timeout[rising][chan];
-		break;
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 		sens = chip->thresh_sensitivity[rising][chan];
 		timeout = chip->thresh_timeout[rising][chan];
@@ -240,13 +225,6 @@ static int ad7150_write_event_config(struct iio_dev *indio_dev,
 	cfg = ret & ~((0x03 << 5) | BIT(7));
 
 	switch (type) {
-	case IIO_EV_TYPE_MAG_ADAPTIVE:
-		adaptive = 1;
-		if (rising)
-			thresh_type = 0x1;
-		else
-			thresh_type = 0x0;
-		break;
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 		adaptive = 1;
 		if (rising)
@@ -294,9 +272,6 @@ static int ad7150_read_event_value(struct iio_dev *indio_dev,
 
 	/* Complex register sharing going on here */
 	switch (type) {
-	case IIO_EV_TYPE_MAG_ADAPTIVE:
-		*val = chip->mag_sensitivity[rising][chan->channel];
-		return IIO_VAL_INT;
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 		*val = chip->thresh_sensitivity[rising][chan->channel];
 		return IIO_VAL_INT;
@@ -321,9 +296,6 @@ static int ad7150_write_event_value(struct iio_dev *indio_dev,
 
 	mutex_lock(&chip->state_lock);
 	switch (type) {
-	case IIO_EV_TYPE_MAG_ADAPTIVE:
-		chip->mag_sensitivity[rising][chan->channel] = val;
-		break;
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 		chip->thresh_sensitivity[rising][chan->channel] = val;
 		break;
@@ -358,9 +330,6 @@ static ssize_t ad7150_show_timeout(struct device *dev,
 		      == IIO_EV_DIR_RISING) ? 1 : 0;
 
 	switch (IIO_EVENT_CODE_EXTRACT_TYPE(this_attr->address)) {
-	case IIO_EV_TYPE_MAG_ADAPTIVE:
-		value = chip->mag_timeout[rising][chan];
-		break;
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 		value = chip->thresh_timeout[rising][chan];
 		break;
@@ -396,9 +365,6 @@ static ssize_t ad7150_store_timeout(struct device *dev,
 
 	mutex_lock(&chip->state_lock);
 	switch (type) {
-	case IIO_EV_TYPE_MAG_ADAPTIVE:
-		chip->mag_timeout[rising][chan] = data;
-		break;
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 		chip->thresh_timeout[rising][chan] = data;
 		break;
@@ -426,10 +392,6 @@ static ssize_t ad7150_store_timeout(struct device *dev,
 				     chan,				\
 				     IIO_EV_TYPE_##ev_type,		\
 				     IIO_EV_DIR_##ev_dir))
-static AD7150_TIMEOUT(0, mag_adaptive, rising, MAG_ADAPTIVE, RISING);
-static AD7150_TIMEOUT(0, mag_adaptive, falling, MAG_ADAPTIVE, FALLING);
-static AD7150_TIMEOUT(1, mag_adaptive, rising, MAG_ADAPTIVE, RISING);
-static AD7150_TIMEOUT(1, mag_adaptive, falling, MAG_ADAPTIVE, FALLING);
 static AD7150_TIMEOUT(0, thresh_adaptive, rising, THRESH_ADAPTIVE, RISING);
 static AD7150_TIMEOUT(0, thresh_adaptive, falling, THRESH_ADAPTIVE, FALLING);
 static AD7150_TIMEOUT(1, thresh_adaptive, rising, THRESH_ADAPTIVE, RISING);
@@ -456,16 +418,6 @@ static const struct iio_event_spec ad7150_events[] = {
 		.dir = IIO_EV_DIR_FALLING,
 		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
 			BIT(IIO_EV_INFO_ENABLE),
-	}, {
-		.type = IIO_EV_TYPE_MAG_ADAPTIVE,
-		.dir = IIO_EV_DIR_RISING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-			BIT(IIO_EV_INFO_ENABLE),
-	}, {
-		.type = IIO_EV_TYPE_MAG_ADAPTIVE,
-		.dir = IIO_EV_DIR_FALLING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-			BIT(IIO_EV_INFO_ENABLE),
 	},
 };
 
@@ -539,14 +491,6 @@ static irqreturn_t ad7150_event_handler(int irq, void *private)
 
 /* Timeouts not currently handled by core */
 static struct attribute *ad7150_event_attributes[] = {
-	&iio_dev_attr_in_capacitance0_mag_adaptive_rising_timeout
-	.dev_attr.attr,
-	&iio_dev_attr_in_capacitance0_mag_adaptive_falling_timeout
-	.dev_attr.attr,
-	&iio_dev_attr_in_capacitance1_mag_adaptive_rising_timeout
-	.dev_attr.attr,
-	&iio_dev_attr_in_capacitance1_mag_adaptive_falling_timeout
-	.dev_attr.attr,
 	&iio_dev_attr_in_capacitance0_thresh_adaptive_rising_timeout
 	.dev_attr.attr,
 	&iio_dev_attr_in_capacitance0_thresh_adaptive_falling_timeout
-- 
2.30.0


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

* [PATCH 03/24] staging:iio:cdc:ad7150: Refactor event parameter update
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 01/24] staging:iio:cdc:ad7150: use swapped reads for i2c rather than open coding Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 02/24] staging:iio:cdc:ad7150: Remove magnitude adaptive events Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 04/24] staging:iio:cdc:ad7150: Timeout register covers both directions so both need updating Jonathan Cameron
                   ` (21 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Original code was ordered in a fairly unituitive fashion with
the non adaptive threshold handling returning from the switch
statement, whilst the adapative path did the actual writes outside
the switch.   Make it more readable by bringing everything within
the switch statement cases and reducing scope of local variables
as appropriate.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 4dac4aaec0cf..d6a7bfd94f1c 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -163,9 +163,6 @@ static int ad7150_write_event_params(struct iio_dev *indio_dev,
 				     enum iio_event_type type,
 				     enum iio_event_direction dir)
 {
-	int ret;
-	u16 value;
-	u8 sens, timeout;
 	struct ad7150_chip_info *chip = iio_priv(indio_dev);
 	int rising = (dir == IIO_EV_DIR_RISING);
 	u64 event_code;
@@ -177,26 +174,31 @@ static int ad7150_write_event_params(struct iio_dev *indio_dev,
 
 	switch (type) {
 		/* Note completely different from the adaptive versions */
-	case IIO_EV_TYPE_THRESH:
-		value = chip->threshold[rising][chan];
+	case IIO_EV_TYPE_THRESH: {
+		u16 value = chip->threshold[rising][chan];
 		return i2c_smbus_write_word_swapped(chip->client,
 						    ad7150_addresses[chan][3],
 						    value);
-	case IIO_EV_TYPE_THRESH_ADAPTIVE:
+	}
+	case IIO_EV_TYPE_THRESH_ADAPTIVE: {
+		int ret;
+		u8 sens, timeout;
+
 		sens = chip->thresh_sensitivity[rising][chan];
+		ret = i2c_smbus_write_byte_data(chip->client,
+						ad7150_addresses[chan][4],
+						sens);
+		if (ret)
+			return ret;
+
 		timeout = chip->thresh_timeout[rising][chan];
-		break;
+		return i2c_smbus_write_byte_data(chip->client,
+						 ad7150_addresses[chan][5],
+						 timeout);
+	}
 	default:
 		return -EINVAL;
 	}
-	ret = i2c_smbus_write_byte_data(chip->client,
-					ad7150_addresses[chan][4],
-					sens);
-	if (ret)
-		return ret;
-	return i2c_smbus_write_byte_data(chip->client,
-					ad7150_addresses[chan][5],
-					timeout);
 }
 
 static int ad7150_write_event_config(struct iio_dev *indio_dev,
-- 
2.30.0


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

* [PATCH 04/24] staging:iio:cdc:ad7150: Timeout register covers both directions so both need updating
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (2 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 03/24] staging:iio:cdc:ad7150: Refactor event parameter update Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support Jonathan Cameron
                   ` (20 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

The timeout is treated as one single value, but the datasheet describes
it as two 4 bit values, one for each direction of event.
As such change the driver to support the separate directions.
Also add limit checking to ensure it fits within the 4 bits.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index d6a7bfd94f1c..0dce1b8ce76d 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -49,6 +49,8 @@
 /* AD7150 masks */
 #define AD7150_THRESHTYPE_MSK			GENMASK(6, 5)
 
+#define AD7150_CH_TIMEOUT_RECEDING		GENMASK(3, 0)
+#define AD7150_CH_TIMEOUT_APPROACHING		GENMASK(7, 4)
 /**
  * struct ad7150_chip_info - instance specific chip data
  * @client: i2c client for this device
@@ -59,7 +61,9 @@
  *	from 'average' value.
  * @thresh_timeout: a timeout, in samples from the moment an
  *	adaptive threshold event occurs to when the average
- *	value jumps to current value.
+ *	value jumps to current value.  Note made up of two fields,
+ *      3:0 are for timeout receding - applies if below lower threshold
+ *      7:4 are for timeout approaching - applies if above upper threshold
  * @old_state: store state from previous event, allowing confirmation
  *	of new condition.
  * @conversion_mode: the current conversion mode.
@@ -191,7 +195,14 @@ static int ad7150_write_event_params(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 
-		timeout = chip->thresh_timeout[rising][chan];
+		/*
+		 * Single timeout register contains timeouts for both
+		 * directions.
+		 */
+		timeout = FIELD_PREP(AD7150_CH_TIMEOUT_APPROACHING,
+				     chip->thresh_timeout[1][chan]);
+		timeout |= FIELD_PREP(AD7150_CH_TIMEOUT_RECEDING,
+				      chip->thresh_timeout[0][chan]);
 		return i2c_smbus_write_byte_data(chip->client,
 						 ad7150_addresses[chan][5],
 						 timeout);
@@ -365,6 +376,9 @@ static ssize_t ad7150_store_timeout(struct device *dev,
 	if (ret < 0)
 		return ret;
 
+	if (data > GENMASK(3, 0))
+		return -EINVAL;
+
 	mutex_lock(&chip->state_lock);
 	switch (type) {
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
-- 
2.30.0


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

* [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (3 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 04/24] staging:iio:cdc:ad7150: Timeout register covers both directions so both need updating Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-08  8:01   ` Song Bao Hua (Barry Song)
  2021-02-07 15:46 ` [PATCH 06/24] staging:iio:cdc:ad7150: Handle variation in chan_spec across device and irq present or not Jonathan Cameron
                   ` (19 subsequent siblings)
  24 siblings, 1 reply; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

There are no mainline board files using this driver so lets drop
the platform_data support in favour of devicetree and similar.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 0dce1b8ce76d..7ad9105e6b46 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -570,20 +570,6 @@ static int ad7150_probe(struct i2c_client *client,
 			return ret;
 	}
 
-	if (client->dev.platform_data) {
-		ret = devm_request_threaded_irq(&client->dev, *(unsigned int *)
-						client->dev.platform_data,
-						NULL,
-						&ad7150_event_handler,
-						IRQF_TRIGGER_RISING |
-						IRQF_TRIGGER_FALLING |
-						IRQF_ONESHOT,
-						"ad7150_irq2",
-						indio_dev);
-		if (ret)
-			return ret;
-	}
-
 	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 	if (ret)
 		return ret;
-- 
2.30.0


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

* [PATCH 06/24] staging:iio:cdc:ad7150: Handle variation in chan_spec across device and irq present or not
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (4 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 07/24] staging:iio:cdc:ad7150: Simplify event handling by only using rising direction Jonathan Cameron
                   ` (18 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

The driver supports devices with different numbers of channels and
also can function without provision of an IRQ (with reduced features),
so this patch handles this cleanly by having multiple chan_spec
arrays and iio_info structures to pick between depending on what we
have.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 7ad9105e6b46..539beed1a511 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -51,6 +51,12 @@
 
 #define AD7150_CH_TIMEOUT_RECEDING		GENMASK(3, 0)
 #define AD7150_CH_TIMEOUT_APPROACHING		GENMASK(7, 4)
+
+enum {
+	AD7150,
+	AD7151,
+};
+
 /**
  * struct ad7150_chip_info - instance specific chip data
  * @client: i2c client for this device
@@ -447,9 +453,30 @@ static const struct iio_event_spec ad7150_events[] = {
 		.num_event_specs = ARRAY_SIZE(ad7150_events),	\
 	}
 
+#define AD7150_CAPACITANCE_CHAN_NO_IRQ(_chan)	{		\
+		.type = IIO_CAPACITANCE,			\
+		.indexed = 1,					\
+		.channel = _chan,				\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+		BIT(IIO_CHAN_INFO_AVERAGE_RAW),			\
+	}
+
 static const struct iio_chan_spec ad7150_channels[] = {
 	AD7150_CAPACITANCE_CHAN(0),
-	AD7150_CAPACITANCE_CHAN(1)
+	AD7150_CAPACITANCE_CHAN(1),
+};
+
+static const struct iio_chan_spec ad7150_channels_no_irq[] = {
+	AD7150_CAPACITANCE_CHAN_NO_IRQ(0),
+	AD7150_CAPACITANCE_CHAN_NO_IRQ(1),
+};
+
+static const struct iio_chan_spec ad7151_channels[] = {
+	AD7150_CAPACITANCE_CHAN(0),
+};
+
+static const struct iio_chan_spec ad7151_channels_no_irq[] = {
+	AD7150_CAPACITANCE_CHAN_NO_IRQ(0),
 };
 
 static irqreturn_t ad7150_event_handler(int irq, void *private)
@@ -532,6 +559,10 @@ static const struct iio_info ad7150_info = {
 	.write_event_value = &ad7150_write_event_value,
 };
 
+static const struct iio_info ad7150_info_no_irq = {
+	.read_raw = &ad7150_read_raw,
+};
+
 static int ad7150_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -550,14 +581,24 @@ static int ad7150_probe(struct i2c_client *client,
 	chip->client = client;
 
 	indio_dev->name = id->name;
-	indio_dev->channels = ad7150_channels;
-	indio_dev->num_channels = ARRAY_SIZE(ad7150_channels);
-
-	indio_dev->info = &ad7150_info;
 
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	if (client->irq) {
+		indio_dev->info = &ad7150_info;
+		switch (id->driver_data) {
+		case AD7150:
+			indio_dev->channels = ad7150_channels;
+			indio_dev->num_channels = ARRAY_SIZE(ad7150_channels);
+			break;
+		case AD7151:
+			indio_dev->channels = ad7151_channels;
+			indio_dev->num_channels = ARRAY_SIZE(ad7151_channels);
+			break;
+		default:
+			return -EINVAL;
+		}
+
 		ret = devm_request_threaded_irq(&client->dev, client->irq,
 						NULL,
 						&ad7150_event_handler,
@@ -568,6 +609,20 @@ static int ad7150_probe(struct i2c_client *client,
 						indio_dev);
 		if (ret)
 			return ret;
+	} else {
+		indio_dev->info = &ad7150_info_no_irq;
+		switch (id->driver_data) {
+		case AD7150:
+			indio_dev->channels = ad7150_channels_no_irq;
+			indio_dev->num_channels = ARRAY_SIZE(ad7150_channels_no_irq);
+			break;
+		case AD7151:
+			indio_dev->channels = ad7151_channels_no_irq;
+			indio_dev->num_channels = ARRAY_SIZE(ad7151_channels_no_irq);
+			break;
+		default:
+			return -EINVAL;
+		}
 	}
 
 	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
@@ -581,9 +636,9 @@ static int ad7150_probe(struct i2c_client *client,
 }
 
 static const struct i2c_device_id ad7150_id[] = {
-	{ "ad7150", 0 },
-	{ "ad7151", 0 },
-	{ "ad7156", 0 },
+	{ "ad7150", AD7150 },
+	{ "ad7151", AD7151 },
+	{ "ad7156", AD7150 },
 	{}
 };
 
-- 
2.30.0


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

* [PATCH 07/24] staging:iio:cdc:ad7150: Simplify event handling by only using rising direction.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (5 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 06/24] staging:iio:cdc:ad7150: Handle variation in chan_spec across device and irq present or not Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 08/24] staging:iio:cdc:ad7150: Drop noisy print in probe Jonathan Cameron
                   ` (17 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

The event line is active high and not maskable within the device.
It indicates current state directly.

The device supports separate rising and falling thresholds so rather
than trying to using each bound to detect in both directions just use
IRQF_TRIGGER_RISING.  If a user wants to detect the value falling
back below the threshold, then set the falling threshold appropriately.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 539beed1a511..34e6afe52f0e 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -603,7 +603,6 @@ static int ad7150_probe(struct i2c_client *client,
 						NULL,
 						&ad7150_event_handler,
 						IRQF_TRIGGER_RISING |
-						IRQF_TRIGGER_FALLING |
 						IRQF_ONESHOT,
 						"ad7150_irq1",
 						indio_dev);
-- 
2.30.0


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

* [PATCH 08/24] staging:iio:cdc:ad7150: Drop noisy print in probe
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (6 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 07/24] staging:iio:cdc:ad7150: Simplify event handling by only using rising direction Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-08  8:02   ` Song Bao Hua (Barry Song)
  2021-02-07 15:46 ` [PATCH 09/24] staging:iio:cdc:ad7150: Add sampling_frequency support Jonathan Cameron
                   ` (16 subsequent siblings)
  24 siblings, 1 reply; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Also
* drop i2c_set_client_data() as now unused.
* white space cleanups

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 34e6afe52f0e..8f8e472e3240 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -573,11 +573,9 @@ static int ad7150_probe(struct i2c_client *client,
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (!indio_dev)
 		return -ENOMEM;
+
 	chip = iio_priv(indio_dev);
 	mutex_init(&chip->state_lock);
-	/* this is only used for device removal purposes */
-	i2c_set_clientdata(client, indio_dev);
-
 	chip->client = client;
 
 	indio_dev->name = id->name;
@@ -624,14 +622,7 @@ static int ad7150_probe(struct i2c_client *client,
 		}
 	}
 
-	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
-	if (ret)
-		return ret;
-
-	dev_info(&client->dev, "%s capacitive sensor registered,irq: %d\n",
-		 id->name, client->irq);
-
-	return 0;
+	return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 }
 
 static const struct i2c_device_id ad7150_id[] = {
-- 
2.30.0


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

* [PATCH 09/24] staging:iio:cdc:ad7150: Add sampling_frequency support
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (7 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 08/24] staging:iio:cdc:ad7150: Drop noisy print in probe Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 10/24] iio:event: Add timeout event info type Jonathan Cameron
                   ` (15 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Device uses a fixed sampling frequency. Let us expose it to userspace.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 8f8e472e3240..54f31aadc92a 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -126,6 +126,10 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		*val = ret;
 
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		/* Strangely same for both 1 and 2 chan parts */
+		*val = 100;
 		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
@@ -449,6 +453,7 @@ static const struct iio_event_spec ad7150_events[] = {
 		.channel = _chan,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
 		BIT(IIO_CHAN_INFO_AVERAGE_RAW),			\
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
 		.event_spec = ad7150_events,			\
 		.num_event_specs = ARRAY_SIZE(ad7150_events),	\
 	}
@@ -459,6 +464,7 @@ static const struct iio_event_spec ad7150_events[] = {
 		.channel = _chan,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
 		BIT(IIO_CHAN_INFO_AVERAGE_RAW),			\
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
 	}
 
 static const struct iio_chan_spec ad7150_channels[] = {
-- 
2.30.0


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

* [PATCH 10/24] iio:event: Add timeout event info type
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (8 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 09/24] staging:iio:cdc:ad7150: Add sampling_frequency support Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 11/24] staging:iio:cdc:ad7150: Change timeout units to seconds and use core support Jonathan Cameron
                   ` (14 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

For adaptive threshold events, the current value is compared with a
(typically) low pass filtered version of the same signal that slowly
tracks large scale changes.  However, sometimes a step change can
result in a large lag before the low pass filtered version begins
to track the signal again.  Timeouts can be used to made an
instantaneous 'correction'.  Documentation of this attribute
is added in a later patch.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/industrialio-event.c | 1 +
 include/linux/iio/types.h        | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index 7e532117ac55..ad01b4d84562 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -245,6 +245,7 @@ static const char * const iio_ev_info_text[] = {
 	[IIO_EV_INFO_PERIOD] = "period",
 	[IIO_EV_INFO_HIGH_PASS_FILTER_3DB] = "high_pass_filter_3db",
 	[IIO_EV_INFO_LOW_PASS_FILTER_3DB] = "low_pass_filter_3db",
+	[IIO_EV_INFO_TIMEOUT] = "timeout",
 };
 
 static enum iio_event_direction iio_ev_attr_dir(struct iio_dev_attr *attr)
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 1e3ed6f55bca..f2e7c96e6e81 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -16,6 +16,7 @@ enum iio_event_info {
 	IIO_EV_INFO_PERIOD,
 	IIO_EV_INFO_HIGH_PASS_FILTER_3DB,
 	IIO_EV_INFO_LOW_PASS_FILTER_3DB,
+	IIO_EV_INFO_TIMEOUT,
 };
 
 #define IIO_VAL_INT 1
-- 
2.30.0


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

* [PATCH 11/24] staging:iio:cdc:ad7150: Change timeout units to seconds and use core support
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (9 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 10/24] iio:event: Add timeout event info type Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 12/24] staging:iio:cdc:ad7150: Rework interrupt handling Jonathan Cameron
                   ` (13 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Now we have core support for timeouts related to adaptive events, let us
use it.  Note the units of that attribute are seconds, so we also need
to scale the cycles value by the period of each sample.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 54f31aadc92a..f8183c540d5c 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -294,13 +294,22 @@ static int ad7150_read_event_value(struct iio_dev *indio_dev,
 	int rising = (dir == IIO_EV_DIR_RISING);
 
 	/* Complex register sharing going on here */
-	switch (type) {
-	case IIO_EV_TYPE_THRESH_ADAPTIVE:
-		*val = chip->thresh_sensitivity[rising][chan->channel];
-		return IIO_VAL_INT;
-	case IIO_EV_TYPE_THRESH:
-		*val = chip->threshold[rising][chan->channel];
-		return IIO_VAL_INT;
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH_ADAPTIVE:
+			*val = chip->thresh_sensitivity[rising][chan->channel];
+			return IIO_VAL_INT;
+		case IIO_EV_TYPE_THRESH:
+			*val = chip->threshold[rising][chan->channel];
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_INFO_TIMEOUT:
+		*val = 0;
+		*val2 = chip->thresh_timeout[rising][chan->channel] * 10000;
+		return IIO_VAL_INT_PLUS_MICRO;
 	default:
 		return -EINVAL;
 	}
@@ -318,13 +327,36 @@ static int ad7150_write_event_value(struct iio_dev *indio_dev,
 	int rising = (dir == IIO_EV_DIR_RISING);
 
 	mutex_lock(&chip->state_lock);
-	switch (type) {
-	case IIO_EV_TYPE_THRESH_ADAPTIVE:
-		chip->thresh_sensitivity[rising][chan->channel] = val;
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH_ADAPTIVE:
+			chip->thresh_sensitivity[rising][chan->channel] = val;
+			break;
+		case IIO_EV_TYPE_THRESH:
+			chip->threshold[rising][chan->channel] = val;
+			break;
+		default:
+			ret = -EINVAL;
+			goto error_ret;
+		}
 		break;
-	case IIO_EV_TYPE_THRESH:
-		chip->threshold[rising][chan->channel] = val;
+	case IIO_EV_INFO_TIMEOUT: {
+		/*
+		 * Raw timeout is in cycles of 10 msecs as long as both
+		 * channels are enabled.
+		 * In terms of INT_PLUS_MICRO, that is in units of 10,000
+		 */
+		int timeout = val2 / 10000;
+
+		if (val != 0 || timeout < 0 || timeout > 15 || val2 % 10000) {
+			ret = -EINVAL;
+			goto error_ret;
+		}
+
+		chip->thresh_timeout[rising][chan->channel] = timeout;
 		break;
+	}
 	default:
 		ret = -EINVAL;
 		goto error_ret;
@@ -338,91 +370,6 @@ static int ad7150_write_event_value(struct iio_dev *indio_dev,
 	return ret;
 }
 
-static ssize_t ad7150_show_timeout(struct device *dev,
-				   struct device_attribute *attr,
-				   char *buf)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	u8 value;
-
-	/* use the event code for consistency reasons */
-	int chan = IIO_EVENT_CODE_EXTRACT_CHAN(this_attr->address);
-	int rising = (IIO_EVENT_CODE_EXTRACT_DIR(this_attr->address)
-		      == IIO_EV_DIR_RISING) ? 1 : 0;
-
-	switch (IIO_EVENT_CODE_EXTRACT_TYPE(this_attr->address)) {
-	case IIO_EV_TYPE_THRESH_ADAPTIVE:
-		value = chip->thresh_timeout[rising][chan];
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return sprintf(buf, "%d\n", value);
-}
-
-static ssize_t ad7150_store_timeout(struct device *dev,
-				    struct device_attribute *attr,
-				    const char *buf,
-				    size_t len)
-{
-	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	int chan = IIO_EVENT_CODE_EXTRACT_CHAN(this_attr->address);
-	enum iio_event_direction dir;
-	enum iio_event_type type;
-	int rising;
-	u8 data;
-	int ret;
-
-	type = IIO_EVENT_CODE_EXTRACT_TYPE(this_attr->address);
-	dir = IIO_EVENT_CODE_EXTRACT_DIR(this_attr->address);
-	rising = (dir == IIO_EV_DIR_RISING);
-
-	ret = kstrtou8(buf, 10, &data);
-	if (ret < 0)
-		return ret;
-
-	if (data > GENMASK(3, 0))
-		return -EINVAL;
-
-	mutex_lock(&chip->state_lock);
-	switch (type) {
-	case IIO_EV_TYPE_THRESH_ADAPTIVE:
-		chip->thresh_timeout[rising][chan] = data;
-		break;
-	default:
-		ret = -EINVAL;
-		goto error_ret;
-	}
-
-	ret = ad7150_write_event_params(indio_dev, chan, type, dir);
-error_ret:
-	mutex_unlock(&chip->state_lock);
-
-	if (ret < 0)
-		return ret;
-
-	return len;
-}
-
-#define AD7150_TIMEOUT(chan, type, dir, ev_type, ev_dir)		\
-	IIO_DEVICE_ATTR(in_capacitance##chan##_##type##_##dir##_timeout, \
-		0644,							\
-		&ad7150_show_timeout,					\
-		&ad7150_store_timeout,					\
-		IIO_UNMOD_EVENT_CODE(IIO_CAPACITANCE,			\
-				     chan,				\
-				     IIO_EV_TYPE_##ev_type,		\
-				     IIO_EV_DIR_##ev_dir))
-static AD7150_TIMEOUT(0, thresh_adaptive, rising, THRESH_ADAPTIVE, RISING);
-static AD7150_TIMEOUT(0, thresh_adaptive, falling, THRESH_ADAPTIVE, FALLING);
-static AD7150_TIMEOUT(1, thresh_adaptive, rising, THRESH_ADAPTIVE, RISING);
-static AD7150_TIMEOUT(1, thresh_adaptive, falling, THRESH_ADAPTIVE, FALLING);
-
 static const struct iio_event_spec ad7150_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -438,12 +385,14 @@ static const struct iio_event_spec ad7150_events[] = {
 		.type = IIO_EV_TYPE_THRESH_ADAPTIVE,
 		.dir = IIO_EV_DIR_RISING,
 		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-			BIT(IIO_EV_INFO_ENABLE),
+			BIT(IIO_EV_INFO_ENABLE) |
+			BIT(IIO_EV_INFO_TIMEOUT),
 	}, {
 		.type = IIO_EV_TYPE_THRESH_ADAPTIVE,
 		.dir = IIO_EV_DIR_FALLING,
 		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-			BIT(IIO_EV_INFO_ENABLE),
+			BIT(IIO_EV_INFO_ENABLE) |
+			BIT(IIO_EV_INFO_TIMEOUT),
 	},
 };
 
@@ -538,15 +487,11 @@ static irqreturn_t ad7150_event_handler(int irq, void *private)
 	return IRQ_HANDLED;
 }
 
-/* Timeouts not currently handled by core */
+static IIO_CONST_ATTR(in_capacitance_thresh_adaptive_timeout_available,
+		      "[0 0.01 0.15]");
+
 static struct attribute *ad7150_event_attributes[] = {
-	&iio_dev_attr_in_capacitance0_thresh_adaptive_rising_timeout
-	.dev_attr.attr,
-	&iio_dev_attr_in_capacitance0_thresh_adaptive_falling_timeout
-	.dev_attr.attr,
-	&iio_dev_attr_in_capacitance1_thresh_adaptive_rising_timeout
-	.dev_attr.attr,
-	&iio_dev_attr_in_capacitance1_thresh_adaptive_falling_timeout
+	&iio_const_attr_in_capacitance_thresh_adaptive_timeout_available
 	.dev_attr.attr,
 	NULL,
 };
-- 
2.30.0


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

* [PATCH 12/24] staging:iio:cdc:ad7150: Rework interrupt handling.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (10 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 11/24] staging:iio:cdc:ad7150: Change timeout units to seconds and use core support Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming Jonathan Cameron
                   ` (12 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Note this doesn't support everything the chip can do as we ignore
window mode for now (in window / out of window).

* Given the chip doesn't have any way of disabling the threshold
  pins, use disable_irq() etc to mask them except when we actually
  want them enabled (previously events were always enabled).
  Note there are race conditions, but using the current state from
  the status register and disabling interrupts across changes in
  type of event should mean those races result in interrupts,
  but no events to userspace.

* Correctly reflect that there is one threshold line per channel.

* Only take notice of rising edge. If anyone wants the other edge
  then they should set the other threshold (they are available for
  rising and falling directions).  This isn't perfect but it makes
  it a lot simpler.

* If insufficient interrupts are specified in firnware, don't support
  events.

* Adaptive events use the same pos/neg values of thrMD as non adaptive
  ones.

Tested against qemu based emulation.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index f8183c540d5c..24be97456c03 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 
 #include <linux/iio/iio.h>
@@ -60,8 +61,6 @@ enum {
 /**
  * struct ad7150_chip_info - instance specific chip data
  * @client: i2c client for this device
- * @current_event: device always has one type of event enabled.
- *	This element stores the event code of the current one.
  * @threshold: thresholds for simple capacitance value events
  * @thresh_sensitivity: threshold for simple capacitance offset
  *	from 'average' value.
@@ -70,21 +69,23 @@ enum {
  *	value jumps to current value.  Note made up of two fields,
  *      3:0 are for timeout receding - applies if below lower threshold
  *      7:4 are for timeout approaching - applies if above upper threshold
- * @old_state: store state from previous event, allowing confirmation
- *	of new condition.
- * @conversion_mode: the current conversion mode.
  * @state_lock: ensure consistent state of this structure wrt the
  *	hardware.
+ * @interrupts: one or two interrupt numbers depending on device type.
+ * @int_enabled: is a given interrupt currently enabled.
+ * @type: threshold type
+ * @dir: threshold direction
  */
 struct ad7150_chip_info {
 	struct i2c_client *client;
-	u64 current_event;
 	u16 threshold[2][2];
 	u8 thresh_sensitivity[2][2];
 	u8 thresh_timeout[2][2];
-	int old_state;
-	char *conversion_mode;
 	struct mutex state_lock;
+	int interrupts[2];
+	bool int_enabled[2];
+	enum iio_event_type type;
+	enum iio_event_direction dir;
 };
 
 /*
@@ -158,8 +159,8 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
 	switch (type) {
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 		if (dir == IIO_EV_DIR_RISING)
-			return !thrfixed && (threshtype == 0x3);
-		return !thrfixed && (threshtype == 0x2);
+			return !thrfixed && (threshtype == 0x1);
+		return !thrfixed && (threshtype == 0x0);
 	case IIO_EV_TYPE_THRESH:
 		if (dir == IIO_EV_DIR_RISING)
 			return thrfixed && (threshtype == 0x1);
@@ -179,11 +180,9 @@ static int ad7150_write_event_params(struct iio_dev *indio_dev,
 {
 	struct ad7150_chip_info *chip = iio_priv(indio_dev);
 	int rising = (dir == IIO_EV_DIR_RISING);
-	u64 event_code;
 
-	event_code = IIO_UNMOD_EVENT_CODE(IIO_CAPACITANCE, chan, type, dir);
-
-	if (event_code != chip->current_event)
+	/* Only update value live, if parameter is in use */
+	if ((type != chip->type) || (dir != chip->dir))
 		return 0;
 
 	switch (type) {
@@ -231,52 +230,91 @@ static int ad7150_write_event_config(struct iio_dev *indio_dev,
 	int ret;
 	struct ad7150_chip_info *chip = iio_priv(indio_dev);
 	int rising = (dir == IIO_EV_DIR_RISING);
-	u64 event_code;
-
-	/* Something must always be turned on */
-	if (!state)
-		return -EINVAL;
 
-	event_code = IIO_UNMOD_EVENT_CODE(chan->type, chan->channel, type, dir);
-	if (event_code == chip->current_event)
+	/*
+	 * There is only a single shared control and no on chip
+	 * interrupt disables for the two interrupt lines.
+	 * So, enabling will switch the events configured to enable
+	 * whatever was most recently requested and if necessary enable_irq()
+	 * the interrupt and any disable will disable_irq() for that
+	 * channels interrupt.
+	 */
+	if (!state) {
+		if ((chip->int_enabled[chan->channel]) &&
+		    (type == chip->type) && (dir == chip->dir)) {
+			disable_irq(chip->interrupts[chan->channel]);
+			chip->int_enabled[chan->channel] = false;
+		}
 		return 0;
+	}
+
 	mutex_lock(&chip->state_lock);
-	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
-	if (ret < 0)
-		goto error_ret;
+	if ((type != chip->type) || (dir != chip->dir)) {
 
-	cfg = ret & ~((0x03 << 5) | BIT(7));
+		/*
+		 * Need to temporarily disable both interrupts if
+		 * enabled - this is to avoid races around changing
+		 * config and thresholds.
+		 * Note enable/disable_irq() are reference counted so
+		 * no need to check if already enabled.
+		 */
+		disable_irq(chip->interrupts[0]);
+		disable_irq(chip->interrupts[1]);
 
-	switch (type) {
-	case IIO_EV_TYPE_THRESH_ADAPTIVE:
-		adaptive = 1;
-		if (rising)
-			thresh_type = 0x3;
-		else
-			thresh_type = 0x2;
-		break;
-	case IIO_EV_TYPE_THRESH:
-		adaptive = 0;
-		if (rising)
-			thresh_type = 0x1;
-		else
-			thresh_type = 0x0;
-		break;
-	default:
-		ret = -EINVAL;
-		goto error_ret;
-	}
+		ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
+		if (ret < 0)
+			goto error_ret;
 
-	cfg |= (!adaptive << 7) | (thresh_type << 5);
+		cfg = ret & ~((0x03 << 5) | BIT(7));
 
-	ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG, cfg);
-	if (ret < 0)
-		goto error_ret;
+		switch (type) {
+		case IIO_EV_TYPE_THRESH_ADAPTIVE:
+			adaptive = 1;
+			if (rising)
+				thresh_type = 0x1;
+			else
+				thresh_type = 0x0;
+			break;
+		case IIO_EV_TYPE_THRESH:
+			adaptive = 0;
+			if (rising)
+				thresh_type = 0x1;
+			else
+				thresh_type = 0x0;
+			break;
+		default:
+			ret = -EINVAL;
+			goto error_ret;
+		}
 
-	chip->current_event = event_code;
+		cfg |= (!adaptive << 7) | (thresh_type << 5);
+
+		ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG, cfg);
+		if (ret < 0)
+			goto error_ret;
+
+		/*
+		 * There is a potential race condition here, but not easy
+		 * to close given we can't disable the interrupt at the
+		 * chip side of things. Rely on the status bit.
+		 */
+		chip->type = type;
+		chip->dir = dir;
+
+		/* update control attributes */
+		ret = ad7150_write_event_params(indio_dev, chan->channel, type,
+						dir);
+		if (ret)
+			goto error_ret;
+		/* reenable any irq's we disabled whilst changing mode */
+		enable_irq(chip->interrupts[0]);
+		enable_irq(chip->interrupts[1]);
+	}
+	if (!chip->int_enabled[chan->channel]) {
+		enable_irq(chip->interrupts[chan->channel]);
+		chip->int_enabled[chan->channel] = true;
+	}
 
-	/* update control attributes */
-	ret = ad7150_write_event_params(indio_dev, chan->channel, type, dir);
 error_ret:
 	mutex_unlock(&chip->state_lock);
 
@@ -434,59 +472,44 @@ static const struct iio_chan_spec ad7151_channels_no_irq[] = {
 	AD7150_CAPACITANCE_CHAN_NO_IRQ(0),
 };
 
-static irqreturn_t ad7150_event_handler(int irq, void *private)
+static irqreturn_t __ad7150_event_handler(void *private, u8 status_mask,
+					  int channel)
 {
 	struct iio_dev *indio_dev = private;
 	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	u8 int_status;
 	s64 timestamp = iio_get_time_ns(indio_dev);
-	int ret;
+	int int_status;
 
-	ret = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS);
-	if (ret < 0)
+	int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS);
+	if (int_status < 0)
 		return IRQ_HANDLED;
 
-	int_status = ret;
-
-	if ((int_status & AD7150_STATUS_OUT1) &&
-	    !(chip->old_state & AD7150_STATUS_OUT1))
-		iio_push_event(indio_dev,
-			       IIO_UNMOD_EVENT_CODE(IIO_CAPACITANCE,
-						    0,
-						    IIO_EV_TYPE_THRESH,
-						    IIO_EV_DIR_RISING),
-				timestamp);
-	else if ((!(int_status & AD7150_STATUS_OUT1)) &&
-		 (chip->old_state & AD7150_STATUS_OUT1))
-		iio_push_event(indio_dev,
-			       IIO_UNMOD_EVENT_CODE(IIO_CAPACITANCE,
-						    0,
-						    IIO_EV_TYPE_THRESH,
-						    IIO_EV_DIR_FALLING),
-			       timestamp);
-
-	if ((int_status & AD7150_STATUS_OUT2) &&
-	    !(chip->old_state & AD7150_STATUS_OUT2))
-		iio_push_event(indio_dev,
-			       IIO_UNMOD_EVENT_CODE(IIO_CAPACITANCE,
-						    1,
-						    IIO_EV_TYPE_THRESH,
-						    IIO_EV_DIR_RISING),
-			       timestamp);
-	else if ((!(int_status & AD7150_STATUS_OUT2)) &&
-		 (chip->old_state & AD7150_STATUS_OUT2))
-		iio_push_event(indio_dev,
-			       IIO_UNMOD_EVENT_CODE(IIO_CAPACITANCE,
-						    1,
-						    IIO_EV_TYPE_THRESH,
-						    IIO_EV_DIR_FALLING),
-			       timestamp);
-	/* store the status to avoid repushing same events */
-	chip->old_state = int_status;
+	/*
+	 * There are race conditions around enabling and disabling that
+	 * could easily land us here with a spurious interrupt.
+	 * Just eat it if so.
+	 */
+	if (!(int_status & status_mask))
+		return IRQ_HANDLED;
+
+	iio_push_event(indio_dev,
+		       IIO_UNMOD_EVENT_CODE(IIO_CAPACITANCE, channel,
+					    chip->type, chip->dir),
+		       timestamp);
 
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t ad7150_event_handler_ch1(int irq, void *private)
+{
+	return __ad7150_event_handler(private, AD7150_STATUS_OUT1, 0);
+}
+
+static irqreturn_t ad7150_event_handler_ch2(int irq, void *private)
+{
+	return __ad7150_event_handler(private, AD7150_STATUS_OUT2, 1);
+}
+
 static IIO_CONST_ATTR(in_capacitance_thresh_adaptive_timeout_available,
 		      "[0 0.01 0.15]");
 
@@ -533,12 +556,44 @@ static int ad7150_probe(struct i2c_client *client,
 
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	if (client->irq) {
+	chip->interrupts[0] = fwnode_irq_get(dev_fwnode(&client->dev), 0);
+	if (chip->interrupts[0] < 0)
+		return chip->interrupts[0];
+	if (id->driver_data == AD7150) {
+		chip->interrupts[1] = fwnode_irq_get(dev_fwnode(&client->dev), 1);
+		if (chip->interrupts[1] < 0)
+			return chip->interrupts[1];
+	}
+	if (chip->interrupts[0] &&
+	    (id->driver_data == AD7151 || chip->interrupts[1])) {
+		irq_set_status_flags(chip->interrupts[0], IRQ_NOAUTOEN);
+		ret = devm_request_threaded_irq(&client->dev,
+						chip->interrupts[0],
+						NULL,
+						&ad7150_event_handler_ch1,
+						IRQF_TRIGGER_RISING |
+						IRQF_ONESHOT,
+						"ad7150_irq1",
+						indio_dev);
+		if (ret)
+			return ret;
+
 		indio_dev->info = &ad7150_info;
 		switch (id->driver_data) {
 		case AD7150:
 			indio_dev->channels = ad7150_channels;
 			indio_dev->num_channels = ARRAY_SIZE(ad7150_channels);
+			irq_set_status_flags(chip->interrupts[1], IRQ_NOAUTOEN);
+			ret = devm_request_threaded_irq(&client->dev,
+							chip->interrupts[1],
+							NULL,
+							&ad7150_event_handler_ch2,
+							IRQF_TRIGGER_RISING |
+							IRQF_ONESHOT,
+							"ad7150_irq2",
+							indio_dev);
+			if (ret)
+				return ret;
 			break;
 		case AD7151:
 			indio_dev->channels = ad7151_channels;
@@ -548,25 +603,18 @@ static int ad7150_probe(struct i2c_client *client,
 			return -EINVAL;
 		}
 
-		ret = devm_request_threaded_irq(&client->dev, client->irq,
-						NULL,
-						&ad7150_event_handler,
-						IRQF_TRIGGER_RISING |
-						IRQF_ONESHOT,
-						"ad7150_irq1",
-						indio_dev);
-		if (ret)
-			return ret;
 	} else {
 		indio_dev->info = &ad7150_info_no_irq;
 		switch (id->driver_data) {
 		case AD7150:
 			indio_dev->channels = ad7150_channels_no_irq;
-			indio_dev->num_channels = ARRAY_SIZE(ad7150_channels_no_irq);
+			indio_dev->num_channels =
+				ARRAY_SIZE(ad7150_channels_no_irq);
 			break;
 		case AD7151:
 			indio_dev->channels = ad7151_channels_no_irq;
-			indio_dev->num_channels = ARRAY_SIZE(ad7151_channels_no_irq);
+			indio_dev->num_channels =
+				ARRAY_SIZE(ad7151_channels_no_irq);
 			break;
 		default:
 			return -EINVAL;
-- 
2.30.0


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

* [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (11 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 12/24] staging:iio:cdc:ad7150: Rework interrupt handling Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-03-14 17:43   ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 14/24] staging:iio:cdc:ad7150: Reorganize headers Jonathan Cameron
                   ` (11 subsequent siblings)
  24 siblings, 1 reply; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Add _REG postfix to register addresses to avoid confusion with
fields.  Also add additional field defines and use throughout the
driver in place of magic numbers.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 24be97456c03..5d4783da7f98 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -21,37 +21,38 @@
  * AD7150 registers definition
  */
 
-#define AD7150_STATUS              0
-#define AD7150_STATUS_OUT1         BIT(3)
-#define AD7150_STATUS_OUT2         BIT(5)
-#define AD7150_CH1_DATA_HIGH       1
-#define AD7150_CH2_DATA_HIGH       3
-#define AD7150_CH1_AVG_HIGH        5
-#define AD7150_CH2_AVG_HIGH        7
-#define AD7150_CH1_SENSITIVITY     9
-#define AD7150_CH1_THR_HOLD_H      9
-#define AD7150_CH1_TIMEOUT         10
-#define AD7150_CH1_SETUP           11
-#define AD7150_CH2_SENSITIVITY     12
-#define AD7150_CH2_THR_HOLD_H      12
-#define AD7150_CH2_TIMEOUT         13
-#define AD7150_CH2_SETUP           14
-#define AD7150_CFG                 15
-#define AD7150_CFG_FIX             BIT(7)
-#define AD7150_PD_TIMER            16
-#define AD7150_CH1_CAPDAC          17
-#define AD7150_CH2_CAPDAC          18
-#define AD7150_SN3                 19
-#define AD7150_SN2                 20
-#define AD7150_SN1                 21
-#define AD7150_SN0                 22
-#define AD7150_ID                  23
-
-/* AD7150 masks */
-#define AD7150_THRESHTYPE_MSK			GENMASK(6, 5)
-
-#define AD7150_CH_TIMEOUT_RECEDING		GENMASK(3, 0)
-#define AD7150_CH_TIMEOUT_APPROACHING		GENMASK(7, 4)
+#define AD7150_STATUS_REG		0
+#define   AD7150_STATUS_OUT1		BIT(3)
+#define   AD7150_STATUS_OUT2		BIT(5)
+#define AD7150_CH1_DATA_HIGH_REG	1
+#define AD7150_CH2_DATA_HIGH_REG	3
+#define AD7150_CH1_AVG_HIGH_REG		5
+#define AD7150_CH2_AVG_HIGH_REG		7
+#define AD7150_CH1_SENSITIVITY_REG	9
+#define AD7150_CH1_THR_HOLD_H_REG	9
+#define AD7150_CH1_TIMEOUT_REG		10
+#define   AD7150_CH_TIMEOUT_RECEDING	GENMASK(3, 0)
+#define   AD7150_CH_TIMEOUT_APPROACHING	GENMASK(7, 4)
+#define AD7150_CH1_SETUP_REG		11
+#define AD7150_CH2_SENSITIVITY_REG	12
+#define AD7150_CH2_THR_HOLD_H_REG	12
+#define AD7150_CH2_TIMEOUT_REG		13
+#define AD7150_CH2_SETUP_REG		14
+#define AD7150_CFG_REG			15
+#define   AD7150_CFG_FIX		BIT(7)
+#define   AD7150_CFG_THRESHTYPE_MSK	GENMASK(6, 5)
+#define   AD7150_CFG_TT_NEG		0x0
+#define   AD7150_CFG_TT_POS		0x1
+#define   AD7150_CFG_TT_IN_WINDOW	0x2
+#define   AD7150_CFG_TT_OUT_WINDOW	0x3
+#define AD7150_PD_TIMER_REG		16
+#define AD7150_CH1_CAPDAC_REG		17
+#define AD7150_CH2_CAPDAC_REG		18
+#define AD7150_SN3_REG			19
+#define AD7150_SN2_REG			20
+#define AD7150_SN1_REG			21
+#define AD7150_SN0_REG			22
+#define AD7150_ID_REG			23
 
 enum {
 	AD7150,
@@ -93,12 +94,12 @@ struct ad7150_chip_info {
  */
 
 static const u8 ad7150_addresses[][6] = {
-	{ AD7150_CH1_DATA_HIGH, AD7150_CH1_AVG_HIGH,
-	  AD7150_CH1_SETUP, AD7150_CH1_THR_HOLD_H,
-	  AD7150_CH1_SENSITIVITY, AD7150_CH1_TIMEOUT },
-	{ AD7150_CH2_DATA_HIGH, AD7150_CH2_AVG_HIGH,
-	  AD7150_CH2_SETUP, AD7150_CH2_THR_HOLD_H,
-	  AD7150_CH2_SENSITIVITY, AD7150_CH2_TIMEOUT },
+	{ AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
+	  AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
+	  AD7150_CH1_SENSITIVITY_REG, AD7150_CH1_TIMEOUT_REG },
+	{ AD7150_CH2_DATA_HIGH_REG, AD7150_CH2_AVG_HIGH_REG,
+	  AD7150_CH2_SETUP_REG, AD7150_CH2_THR_HOLD_H_REG,
+	  AD7150_CH2_SENSITIVITY_REG, AD7150_CH2_TIMEOUT_REG },
 };
 
 static int ad7150_read_raw(struct iio_dev *indio_dev,
@@ -147,11 +148,11 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
 	bool thrfixed;
 	struct ad7150_chip_info *chip = iio_priv(indio_dev);
 
-	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
+	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
 	if (ret < 0)
 		return ret;
 
-	threshtype = FIELD_GET(AD7150_THRESHTYPE_MSK, ret);
+	threshtype = FIELD_GET(AD7150_CFG_THRESHTYPE_MSK, ret);
 
 	/*check if threshold mode is fixed or adaptive*/
 	thrfixed = FIELD_GET(AD7150_CFG_FIX, ret);
@@ -159,12 +160,12 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
 	switch (type) {
 	case IIO_EV_TYPE_THRESH_ADAPTIVE:
 		if (dir == IIO_EV_DIR_RISING)
-			return !thrfixed && (threshtype == 0x1);
-		return !thrfixed && (threshtype == 0x0);
+			return !thrfixed && (threshtype == AD7150_CFG_TT_POS);
+		return !thrfixed && (threshtype == AD7150_CFG_TT_NEG);
 	case IIO_EV_TYPE_THRESH:
 		if (dir == IIO_EV_DIR_RISING)
-			return thrfixed && (threshtype == 0x1);
-		return thrfixed && (threshtype == 0x0);
+			return thrfixed && (threshtype == AD7150_CFG_TT_POS);
+		return thrfixed && (threshtype == AD7150_CFG_TT_NEG);
 	default:
 		break;
 	}
@@ -261,35 +262,27 @@ static int ad7150_write_event_config(struct iio_dev *indio_dev,
 		disable_irq(chip->interrupts[0]);
 		disable_irq(chip->interrupts[1]);
 
-		ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
+		ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
 		if (ret < 0)
 			goto error_ret;
 
-		cfg = ret & ~((0x03 << 5) | BIT(7));
+		cfg = ret & ~(AD7150_CFG_THRESHTYPE_MSK | AD7150_CFG_FIX);
 
-		switch (type) {
-		case IIO_EV_TYPE_THRESH_ADAPTIVE:
+		if (type == IIO_EV_TYPE_THRESH_ADAPTIVE)
 			adaptive = 1;
-			if (rising)
-				thresh_type = 0x1;
-			else
-				thresh_type = 0x0;
-			break;
-		case IIO_EV_TYPE_THRESH:
+		else
 			adaptive = 0;
-			if (rising)
-				thresh_type = 0x1;
-			else
-				thresh_type = 0x0;
-			break;
-		default:
-			ret = -EINVAL;
-			goto error_ret;
-		}
 
-		cfg |= (!adaptive << 7) | (thresh_type << 5);
+		if (rising)
+			thresh_type = AD7150_CFG_TT_POS;
+		else
+			thresh_type = AD7150_CFG_TT_NEG;
+
+		cfg |= FIELD_PREP(AD7150_CFG_FIX, !adaptive) |
+			FIELD_PREP(AD7150_CFG_THRESHTYPE_MSK, thresh_type);
 
-		ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG, cfg);
+		ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG_REG,
+						cfg);
 		if (ret < 0)
 			goto error_ret;
 
@@ -480,7 +473,7 @@ static irqreturn_t __ad7150_event_handler(void *private, u8 status_mask,
 	s64 timestamp = iio_get_time_ns(indio_dev);
 	int int_status;
 
-	int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS);
+	int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS_REG);
 	if (int_status < 0)
 		return IRQ_HANDLED;
 
-- 
2.30.0


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

* [PATCH 14/24] staging:iio:cdc:ad7150: Reorganize headers.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (12 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-08  7:54   ` Song Bao Hua (Barry Song)
  2021-02-07 15:46 ` [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable positioning Jonathan Cameron
                   ` (10 subsequent siblings)
  24 siblings, 1 reply; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Whilst not important, it's nice to have the general headers in
alphabetical order.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 5d4783da7f98..9e88e774752f 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -6,13 +6,13 @@
  */
 
 #include <linux/bitfield.h>
-#include <linux/interrupt.h>
 #include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/slab.h>
-#include <linux/i2c.h>
+#include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
-- 
2.30.0


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

* [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable positioning.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (13 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 14/24] staging:iio:cdc:ad7150: Reorganize headers Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-08  7:54   ` Song Bao Hua (Barry Song)
  2021-02-07 15:46 ` [PATCH 16/24] staging:iio:cdc:ad7150: Drop unnecessary block comments Jonathan Cameron
                   ` (9 subsequent siblings)
  24 siblings, 1 reply; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Where there is no other basis on which to order declarations
let us prefer reverse xmas tree.  Also reduce scope where
sensible.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 9e88e774752f..d530b467d1b2 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -108,9 +108,9 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
 			   int *val2,
 			   long mask)
 {
-	int ret;
 	struct ad7150_chip_info *chip = iio_priv(indio_dev);
 	int channel = chan->channel;
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -143,10 +143,10 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
 				    enum iio_event_type type,
 				    enum iio_event_direction dir)
 {
-	int ret;
+	struct ad7150_chip_info *chip = iio_priv(indio_dev);
 	u8 threshtype;
 	bool thrfixed;
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
+	int ret;
 
 	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
 	if (ret < 0)
@@ -227,10 +227,8 @@ static int ad7150_write_event_config(struct iio_dev *indio_dev,
 				     enum iio_event_type type,
 				     enum iio_event_direction dir, int state)
 {
-	u8 thresh_type, cfg, adaptive;
-	int ret;
 	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	int rising = (dir == IIO_EV_DIR_RISING);
+	int ret;
 
 	/*
 	 * There is only a single shared control and no on chip
@@ -251,6 +249,8 @@ static int ad7150_write_event_config(struct iio_dev *indio_dev,
 
 	mutex_lock(&chip->state_lock);
 	if ((type != chip->type) || (dir != chip->dir)) {
+		int rising = (dir == IIO_EV_DIR_RISING);
+		u8 thresh_type, cfg, adaptive;
 
 		/*
 		 * Need to temporarily disable both interrupts if
@@ -533,9 +533,9 @@ static const struct iio_info ad7150_info_no_irq = {
 static int ad7150_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
-	int ret;
 	struct ad7150_chip_info *chip;
 	struct iio_dev *indio_dev;
+	int ret;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
 	if (!indio_dev)
-- 
2.30.0


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

* [PATCH 16/24] staging:iio:cdc:ad7150: Drop unnecessary block comments.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (14 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable positioning Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-08  7:52   ` Song Bao Hua (Barry Song)
  2021-02-07 15:46 ` [PATCH 17/24] staging:iio:cdc:ad7150: Shift the _raw readings by 4 bits Jonathan Cameron
                   ` (8 subsequent siblings)
  24 siblings, 1 reply; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

These have a habit of not getting updated with driver reorganizations
and don't add much info so drop them.

Also fix a minor comment syntax issue.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index d530b467d1b2..4c83e6e37c5a 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -17,9 +17,6 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/events.h>
-/*
- * AD7150 registers definition
- */
 
 #define AD7150_STATUS_REG		0
 #define   AD7150_STATUS_OUT1		BIT(3)
@@ -89,10 +86,6 @@ struct ad7150_chip_info {
 	enum iio_event_direction dir;
 };
 
-/*
- * sysfs nodes
- */
-
 static const u8 ad7150_addresses[][6] = {
 	{ AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
 	  AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
@@ -172,8 +165,7 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-/* state_lock should be held to ensure consistent state*/
-
+/* state_lock should be held to ensure consistent state */
 static int ad7150_write_event_params(struct iio_dev *indio_dev,
 				     unsigned int chan,
 				     enum iio_event_type type,
-- 
2.30.0


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

* [PATCH 17/24] staging:iio:cdc:ad7150: Shift the _raw readings by 4 bits.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (15 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 16/24] staging:iio:cdc:ad7150: Drop unnecessary block comments Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 18/24] staging:iio:cdc:ad7150: Add scale and offset to info_mask_shared_by_type Jonathan Cameron
                   ` (7 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Every other register related to raw value on the datasheet is
described as correpsonding to the 12MSB of the actual
data registers + the bottom 4 bits are 0.  So lets treat this
as what it actually is, which is a 12 bit value.
Note that we will have to be a little careful to compensate for
the offset and scale values.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 4c83e6e37c5a..97689625f26c 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -111,7 +111,7 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
 						  ad7150_addresses[channel][0]);
 		if (ret < 0)
 			return ret;
-		*val = ret;
+		*val = ret >> 4;
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_AVERAGE_RAW:
-- 
2.30.0


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

* [PATCH 18/24] staging:iio:cdc:ad7150: Add scale and offset to info_mask_shared_by_type
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (16 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 17/24] staging:iio:cdc:ad7150: Shift the _raw readings by 4 bits Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 19/24] staging:iio:cdc:ad7150: Really basic regulator support Jonathan Cameron
                   ` (6 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

The datasheet provides these two values on the assumption they are applied
to unshift raw value.  Hence shift both the offset and scale by 4
to compensate.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 97689625f26c..a2aae370c231 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -121,6 +121,18 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		*val = ret;
 
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * Base units for capacitance are nano farads and the value
+		 * calculated from the datasheet formula is in picofarad
+		 * so multiply by 1000
+		 */
+		*val = 1000;
+		*val2 = 40944 >> 4; /* To match shift in _RAW */
+		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = -(12288 >> 4); /* To match shift in _RAW */
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		/* Strangely same for both 1 and 2 chan parts */
@@ -425,6 +437,8 @@ static const struct iio_event_spec ad7150_events[] = {
 		.channel = _chan,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
 		BIT(IIO_CHAN_INFO_AVERAGE_RAW),			\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+			BIT(IIO_CHAN_INFO_OFFSET),		\
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
 		.event_spec = ad7150_events,			\
 		.num_event_specs = ARRAY_SIZE(ad7150_events),	\
@@ -436,6 +450,8 @@ static const struct iio_event_spec ad7150_events[] = {
 		.channel = _chan,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
 		BIT(IIO_CHAN_INFO_AVERAGE_RAW),			\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+			BIT(IIO_CHAN_INFO_OFFSET),		\
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
 	}
 
-- 
2.30.0


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

* [PATCH 19/24] staging:iio:cdc:ad7150: Really basic regulator support.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (17 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 18/24] staging:iio:cdc:ad7150: Add scale and offset to info_mask_shared_by_type Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table Jonathan Cameron
                   ` (5 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Given DT docs will include regulators, lets just turn them on and
off with driver probe() and remove().

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index a2aae370c231..0bc8c7a99883 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -12,6 +12,7 @@
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
 #include <linux/iio/iio.h>
@@ -538,11 +539,19 @@ static const struct iio_info ad7150_info_no_irq = {
 	.read_raw = &ad7150_read_raw,
 };
 
+static void ad7150_reg_disable(void *data)
+{
+	struct regulator *reg = data;
+
+	regulator_disable(reg);
+}
+
 static int ad7150_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	struct ad7150_chip_info *chip;
 	struct iio_dev *indio_dev;
+	struct regulator *reg;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
@@ -557,6 +566,18 @@ static int ad7150_probe(struct i2c_client *client,
 
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
+	reg = devm_regulator_get(&client->dev, "vdd");
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	ret = regulator_enable(reg);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&client->dev, ad7150_reg_disable, reg);
+	if (ret)
+		return ret;
+
 	chip->interrupts[0] = fwnode_irq_get(dev_fwnode(&client->dev), 0);
 	if (chip->interrupts[0] < 0)
 		return chip->interrupts[0];
-- 
2.30.0


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

* [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (18 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 19/24] staging:iio:cdc:ad7150: Really basic regulator support Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-08  7:11   ` Alexandru Ardelean
  2021-02-08  7:50   ` Song Bao Hua (Barry Song)
  2021-02-07 15:46 ` [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc Jonathan Cameron
                   ` (4 subsequent siblings)
  24 siblings, 2 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Rather than using the fallback path in the i2c subsystem and hoping
for no clashes across vendors, lets put in an explicit table for
matching.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 0bc8c7a99883..33c8a78c076f 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -12,6 +12,7 @@
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
@@ -655,9 +656,16 @@ static const struct i2c_device_id ad7150_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, ad7150_id);
 
+static const struct of_device_id ad7150_of_match[] = {
+	{ "adi,ad7150" },
+	{ "adi,ad7151" },
+	{ "adi,ad7156" },
+	{}
+};
 static struct i2c_driver ad7150_driver = {
 	.driver = {
 		.name = "ad7150",
+		.of_match_table = ad7150_of_match,
 	},
 	.probe = ad7150_probe,
 	.id_table = ad7150_id,
-- 
2.30.0


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

* [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (19 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 16:00   ` Lars-Peter Clausen
  2021-02-21 15:59   ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 22/24] iio:Documentation:ABI Add missing elements as used by the adi,ad7150 Jonathan Cameron
                   ` (3 subsequent siblings)
  24 siblings, 2 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron, Robh+dt, devicetree

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

Binding covering the ad7150, ad7151 and ad7156 capacitance to digital
convertors.  The only difference between these is how many channels they
have (1 or 2)

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Robh+dt@kernel.org
Cc: devicetree@vger.kernel.org
---
 .../bindings/iio/cdc/adi,ad7150.yaml          | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/cdc/adi,ad7150.yaml b/Documentation/devicetree/bindings/iio/cdc/adi,ad7150.yaml
new file mode 100644
index 000000000000..2155d3f5666c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/cdc/adi,ad7150.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/cdc/adi,ad7150.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog device AD7150 and similar capacitance to digital convertors.
+
+maintainers:
+  - Jonathan Cameron <jic23@kernel.org>
+
+properties:
+  compatible:
+    enum:
+      - adi,ad7150
+      - adi,ad7151
+      - adi,ad7156
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+
+  interrupts: true
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad7150
+              - adi,ad7156
+    then:
+      properties:
+        interrupts:
+          minItems: 2
+          maxItems: 2
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: adi,ad7151
+    then:
+      properties:
+        interrupts:
+          minItems: 1
+          maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        cdc@48 {
+            compatible = "adi,ad7150";
+            reg = <0x48>;
+            interrupts = <25 2>, <26 2>;
+            interrupt-parent = <&gpio>;
+        };
+    };
+...
-- 
2.30.0


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

* [PATCH 22/24] iio:Documentation:ABI Add missing elements as used by the adi,ad7150
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (20 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 23/24] staging:iio:cdc:ad7150: Add copyright notice given substantial changes Jonathan Cameron
                   ` (2 subsequent siblings)
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

Main additions are around thresh_adaptive.  This has been supported
by the core of IIO for a long time, but no driver that uses it has
previously graduated from staging, hence we are missing Docs.

Otherwise, just new entries in existing lists.

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

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 35289d47d6cb..e3b9878ea7f9 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -324,6 +324,7 @@ What:		/sys/bus/iio/devices/iio:deviceX/in_humidityrelative_offset
 What:		/sys/bus/iio/devices/iio:deviceX/in_magn_offset
 What:		/sys/bus/iio/devices/iio:deviceX/in_rot_offset
 What:		/sys/bus/iio/devices/iio:deviceX/in_angl_offset
+What:		/sys/bus/iio/devices/iio:deviceX/in_capacitanceX_offset
 KernelVersion:	2.6.35
 Contact:	linux-iio@vger.kernel.org
 Description:
@@ -655,6 +656,8 @@ What:		/sys/.../iio:deviceX/events/in_voltageY_thresh_falling_en
 What:		/sys/.../iio:deviceX/events/in_voltageY_thresh_either_en
 What:		/sys/.../iio:deviceX/events/in_tempY_thresh_rising_en
 What:		/sys/.../iio:deviceX/events/in_tempY_thresh_falling_en
+What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_rising_en
+What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_falling_en
 KernelVersion:	2.6.37
 Contact:	linux-iio@vger.kernel.org
 Description:
@@ -732,6 +735,32 @@ Description:
 		a given event type is enabled a future point (and not those for
 		whatever event was previously enabled).
 
+What:		/sys/.../events/in_capacitanceY_adaptive_thresh_rising_en
+What:		/sys/.../events/in_capacitanceY_adaptive_thresh_falling_en
+KernelVersion:	5.11
+Contact:	linux-iio@vger.kernel.org
+Descrption:
+		Adaptive thresholds are similar to normal fixed thresholds
+		but the value is expressed as an offset from a value which
+		provides a low frequency approximation of the channel itself.
+		Thus these detect if a rapid change occurs in the specified
+		direction which crosses tracking value + offset.
+		Tracking value calculation is devices specific.
+
+What:		/sys/.../in_capacitanceY_adaptive_thresh_rising_timeout
+What:		/sys/.../in_capacitanceY_adaptive_thresh_falling_timeout
+KernelVersion:	5.11
+Contact:	linux-iio@vger.kernel.org
+Descrption:
+		When adaptive thresholds are used, the tracking signal
+		may adjust too slowly to step changes in the raw signal.
+		*_timeout (in seconds) specifies a time for which the
+		difference between the slow tracking signal and the raw
+		signal is allowed to remain out-of-range before a reset
+		event occurs in which the tracking signal is made equal
+		to the raw signal, allowing slow tracking to resume and the
+		adaptive threshold event detection to function as expected.
+
 What:		/sys/.../events/in_accel_thresh_rising_value
 What:		/sys/.../events/in_accel_thresh_falling_value
 What:		/sys/.../events/in_accel_x_raw_thresh_rising_value
@@ -772,6 +801,10 @@ What:		/sys/.../events/in_proximity0_thresh_falling_value
 What:		/sys/.../events/in_proximity0_thresh_rising_value
 What:		/sys/.../events/in_illuminance_thresh_rising_value
 What:		/sys/.../events/in_illuminance_thresh_falling_value
+What:		/sys/.../events/in_capacitanceY_thresh_rising_value
+What:		/sys/.../events/in_capacitanceY_thresh_falling_value
+What:		/sys/.../events/in_capacitanceY_thresh_adaptive_rising_value
+What:		/sys/.../events/in_capacitanceY_thresh_falling_rising_value
 KernelVersion:	2.6.37
 Contact:	linux-iio@vger.kernel.org
 Description:
-- 
2.30.0


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

* [PATCH 23/24] staging:iio:cdc:ad7150: Add copyright notice given substantial changes.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (21 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 22/24] iio:Documentation:ABI Add missing elements as used by the adi,ad7150 Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 15:46 ` [PATCH 24/24] iio:cdc:ad7150: Move driver out of staging Jonathan Cameron
  2021-02-07 16:12 ` [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
  24 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

It seems to me that the changes made to get this ready to move out of
staging are substantial enough to warant a copyright notice addition.

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

diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 33c8a78c076f..9a6b55021fa7 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -3,6 +3,7 @@
  * AD7150 capacitive sensor driver supporting AD7150/1/6
  *
  * Copyright 2010-2011 Analog Devices Inc.
+ * Copyright 2021 Jonathan Cameron <Jonathan.Cameron@huawei.com>
  */
 
 #include <linux/bitfield.h>
-- 
2.30.0


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

* [PATCH 24/24] iio:cdc:ad7150: Move driver out of staging.
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (22 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 23/24] staging:iio:cdc:ad7150: Add copyright notice given substantial changes Jonathan Cameron
@ 2021-02-07 15:46 ` Jonathan Cameron
  2021-02-07 16:21   ` Jonathan Cameron
  2021-02-07 16:12 ` [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
  24 siblings, 1 reply; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 15:46 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

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

This capacitance to digital converter (CDC) driver is compliant with
the IIO ABI.  Note, not all features supported (e.g. window event modes)
but the driver should be in a useful functional state.

The cleanup was done against QEMU emulation of the device rather than
actual hardware.   Whilst this was a bit of an experiment, it made it
easy to confirm that the driver remained in a consistent working state
through the various refactors.  If it worked in the first place, it
should still be working after this cleanup.

Given some IIO drivers require expensive hardware setups, (not particularly
true with this one) the use of QEMU may provide a viable way forward
for providing testing during code changes where previously we'd had
to rely on sharp eyes and crossed fingers.

Note, no explicit MAINTAINERS entry as it will be covered by the
generic catch-alls for ADI and IIO drivers which are sufficient.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/Kconfig                    |  1 +
 drivers/iio/Makefile                   |  1 +
 drivers/iio/cdc/Kconfig                | 17 +++++++++++++++++
 drivers/iio/cdc/Makefile               |  6 ++++++
 drivers/{staging => }/iio/cdc/ad7150.c |  0
 drivers/staging/iio/cdc/Kconfig        | 10 ----------
 drivers/staging/iio/cdc/Makefile       |  3 +--
 7 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 267553386c71..a5c47300e5a0 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -72,6 +72,7 @@ source "drivers/iio/accel/Kconfig"
 source "drivers/iio/adc/Kconfig"
 source "drivers/iio/afe/Kconfig"
 source "drivers/iio/amplifiers/Kconfig"
+source "drivers/iio/cdc/Kconfig"
 source "drivers/iio/chemical/Kconfig"
 source "drivers/iio/common/Kconfig"
 source "drivers/iio/dac/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 1712011c0f4a..1cb1434b86b2 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -18,6 +18,7 @@ obj-y += adc/
 obj-y += afe/
 obj-y += amplifiers/
 obj-y += buffer/
+obj-y += cdc/
 obj-y += chemical/
 obj-y += common/
 obj-y += dac/
diff --git a/drivers/iio/cdc/Kconfig b/drivers/iio/cdc/Kconfig
new file mode 100644
index 000000000000..5e3319a3ff48
--- /dev/null
+++ b/drivers/iio/cdc/Kconfig
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# CDC drivers
+#
+menu "Capacitance to digital converters"
+
+config AD7150
+	tristate "Analog Devices ad7150/1/6 capacitive sensor driver"
+	depends on I2C
+	help
+	  Say yes here to build support for Analog Devices capacitive sensors.
+	  (ad7150, ad7151, ad7156) Provides direct access via sysfs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad7150.
+
+endmenu
diff --git a/drivers/iio/cdc/Makefile b/drivers/iio/cdc/Makefile
new file mode 100644
index 000000000000..ee490637b032
--- /dev/null
+++ b/drivers/iio/cdc/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for industrial I/O capacitance to digital converter (CDC) drivers
+#
+
+obj-$(CONFIG_AD7150) += ad7150.o
diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/iio/cdc/ad7150.c
similarity index 100%
rename from drivers/staging/iio/cdc/ad7150.c
rename to drivers/iio/cdc/ad7150.c
diff --git a/drivers/staging/iio/cdc/Kconfig b/drivers/staging/iio/cdc/Kconfig
index e0a5ce66a984..a7386bbbcb79 100644
--- a/drivers/staging/iio/cdc/Kconfig
+++ b/drivers/staging/iio/cdc/Kconfig
@@ -4,16 +4,6 @@
 #
 menu "Capacitance to digital converters"
 
-config AD7150
-	tristate "Analog Devices ad7150/1/6 capacitive sensor driver"
-	depends on I2C
-	help
-	  Say yes here to build support for Analog Devices capacitive sensors.
-	  (ad7150, ad7151, ad7156) Provides direct access via sysfs.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called ad7150.
-
 config AD7746
 	tristate "Analog Devices AD7745, AD7746 AD7747 capacitive sensor driver"
 	depends on I2C
diff --git a/drivers/staging/iio/cdc/Makefile b/drivers/staging/iio/cdc/Makefile
index ab8222579e7e..afb7499a7090 100644
--- a/drivers/staging/iio/cdc/Makefile
+++ b/drivers/staging/iio/cdc/Makefile
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 #
-# Makefile for industrial I/O DAC drivers
+# Makefile for industrial I/O CDC drivers
 #
 
-obj-$(CONFIG_AD7150) += ad7150.o
 obj-$(CONFIG_AD7746) += ad7746.o
-- 
2.30.0


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

* Re: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
  2021-02-07 15:46 ` [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc Jonathan Cameron
@ 2021-02-07 16:00   ` Lars-Peter Clausen
  2021-02-07 16:18     ` Jonathan Cameron
  2021-02-08  8:12     ` Song Bao Hua (Barry Song)
  2021-02-21 15:59   ` Jonathan Cameron
  1 sibling, 2 replies; 46+ messages in thread
From: Lars-Peter Clausen @ 2021-02-07 16:00 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Michael Hennerich, song.bao.hua, robh+dt, Jonathan Cameron, devicetree

On 2/7/21 4:46 PM, Jonathan Cameron wrote:
> +required:
> +  - compatible
> +  - reg

Is vdd-supply really optional the way it is implemented in the driver?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        cdc@48 {
> +            compatible = "adi,ad7150";
> +            reg = <0x48>;
> +            interrupts = <25 2>, <26 2>;

I wonder if we should use the symbolic constants for the IRQ type to 
make the example more clear. E.g.

interrupts = <25 IRQ_TYPE_EDGE_FALLING>, ...

> +            interrupt-parent = <&gpio>;
> +        };
> +    };
> +...



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

* Re: [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate
  2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
                   ` (23 preceding siblings ...)
  2021-02-07 15:46 ` [PATCH 24/24] iio:cdc:ad7150: Move driver out of staging Jonathan Cameron
@ 2021-02-07 16:12 ` Jonathan Cameron
  2021-02-08  7:20   ` Alexandru Ardelean
  24 siblings, 1 reply; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 16:12 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

On Sun,  7 Feb 2021 15:45:59 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This is an 'old' driver in IIO that has been in staging a while.
> First submitted in October 2010.
> 
> I wanted to try and experiment and picked this driver to try it with.
> 
> The cleanup etc here was all tested against some basic emulation
> added to QEMU rather than real hardware.  Once I've cleaned that up
> a tiny bit I'll push it up to https://github.com/jic23/qemu
Now up at:
https://github.com/jic23/qemu/tree/iio-ad7150

The device emulation is rather messy.
As arm / virt builds the DT tree alongside the instantiation of
devices it was a very convenient place to hack an i2c bus and the ad7150
in using the existing pl061 to provide suitable interrupt lines.

Note that I don't care about VM migration or anything like that
so the implementation is rather more minimal than would be needed
for normal QEMU device emulation.

Anyhow, gives an idea of what is needed for anyone who is curious.

qom-list /machine/unattached/device[9]
lists the various things that can be controlled including.
ch2_capacitance (int)
ch1_capacitance_avg (int)
ch2_event_state (int)
ch1_event_state (int)
ch2_capacitance_avg (int)
ch1_capacitance (int)

Event state is a bit simplistic in that you have direct control of
the gpio lines. I could have implemented a state machine so that the
actual thresholds etc were used, but the adaptive mode is sufficiently
complex that I haven't yet done so.

Jonathan

> Note that for now I'm not proposing to upstream this to QEMU but
> would be interested in hearing if people thing it is a good idea to
> do so.
> 
> Whilst it's obviously hard to be absolutely sure that the emulation is
> correct, the fact that the original driver worked as expected and the
> cleaned up version still does is certainly encouraging.
> 
> Note however, that there were a few more significant changes in here than
> basic cleanup.
> 1. Interrupts / events were handled in a rather esoteric fashion.
>    (Always on, window modes represented as magnitudes).
>    Note that for two channel devices there are separate lines. The original
>    driver was not supporting this at all.
>    They now look more like a standard IIO driver and reflect experience
>    that we've gained over the years in dealing with devices where these
>    aren't interrupt lines as such, but rather reporters of current status.
> 2. Timeouts were handled in a fashion that clearly wouldn't work.
> 
> Note that this moving out of staging makes a few bits of ABI 'official'
> and so those are added to the main IIO ABI Docs.
> 
> Thanks in advance to anyone who has time to take a look.
> 
> Jonathan
> 
> 
> Jonathan Cameron (24):
>   staging:iio:cdc:ad7150: use swapped reads for i2c rather than open
>     coding.
>   staging:iio:cdc:ad7150: Remove magnitude adaptive events
>   staging:iio:cdc:ad7150: Refactor event parameter update
>   staging:iio:cdc:ad7150: Timeout register covers both directions so
>     both need updating
>   staging:iio:cdc:ad7150: Drop platform data support
>   staging:iio:cdc:ad7150: Handle variation in chan_spec across device
>     and irq present or not
>   staging:iio:cdc:ad7150: Simplify event handling by only using rising
>     direction.
>   staging:iio:cdc:ad7150: Drop noisy print in probe
>   staging:iio:cdc:ad7150: Add sampling_frequency support
>   iio:event: Add timeout event info type
>   staging:iio:cdc:ad7150: Change timeout units to seconds and use core
>     support
>   staging:iio:cdc:ad7150: Rework interrupt handling.
>   staging:iio:cdc:ad7150: More consistent register and field naming
>   staging:iio:cdc:ad7150: Reorganize headers.
>   staging:iio:cdc:ad7150: Tidy up local variable positioning.
>   staging:iio:cdc:ad7150: Drop unnecessary block comments.
>   staging:iio:cdc:ad7150: Shift the _raw readings by 4 bits.
>   staging:iio:cdc:ad7150: Add scale and offset to
>     info_mask_shared_by_type
>   staging:iio:cdc:ad7150: Really basic regulator support.
>   staging:iio:cdc:ad7150: Add of_match_table
>   dt-bindings:iio:cdc:adi,ad7150 binding doc
>   iio:Documentation:ABI Add missing elements as used by the adi,ad7150
>   staging:iio:cdc:ad7150: Add copyright notice given substantial
>     changes.
>   iio:cdc:ad7150: Move driver out of staging.
> 
>  Documentation/ABI/testing/sysfs-bus-iio       |  33 +
>  .../bindings/iio/cdc/adi,ad7150.yaml          |  69 ++
>  drivers/iio/Kconfig                           |   1 +
>  drivers/iio/Makefile                          |   1 +
>  drivers/iio/cdc/Kconfig                       |  17 +
>  drivers/iio/cdc/Makefile                      |   6 +
>  drivers/iio/cdc/ad7150.c                      | 678 ++++++++++++++++++
>  drivers/iio/industrialio-event.c              |   1 +
>  drivers/staging/iio/cdc/Kconfig               |  10 -
>  drivers/staging/iio/cdc/Makefile              |   3 +-
>  drivers/staging/iio/cdc/ad7150.c              | 655 -----------------
>  include/linux/iio/types.h                     |   1 +
>  12 files changed, 808 insertions(+), 667 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/cdc/adi,ad7150.yaml
>  create mode 100644 drivers/iio/cdc/Kconfig
>  create mode 100644 drivers/iio/cdc/Makefile
>  create mode 100644 drivers/iio/cdc/ad7150.c
>  delete mode 100644 drivers/staging/iio/cdc/ad7150.c
> 


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

* Re: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
  2021-02-07 16:00   ` Lars-Peter Clausen
@ 2021-02-07 16:18     ` Jonathan Cameron
  2021-02-08  8:12     ` Song Bao Hua (Barry Song)
  1 sibling, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 16:18 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: linux-iio, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron, devicetree

On Sun, 7 Feb 2021 17:00:24 +0100
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 2/7/21 4:46 PM, Jonathan Cameron wrote:
> > +required:
> > +  - compatible
> > +  - reg  
> 
> Is vdd-supply really optional the way it is implemented in the driver?

Well sort of.  Obviously VDD isn't optional in the sense that the
device needs power, but it is in the binding because a stub regulator
should be fine.  For those regulator_enable() is a noop on assumption
they are already on.

> 
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        cdc@48 {
> > +            compatible = "adi,ad7150";
> > +            reg = <0x48>;
> > +            interrupts = <25 2>, <26 2>;  
> 
> I wonder if we should use the symbolic constants for the IRQ type to 
> make the example more clear. E.g.
> 
> interrupts = <25 IRQ_TYPE_EDGE_FALLING>, ...

Sure. I'll update in v2.

> 
> > +            interrupt-parent = <&gpio>;
> > +        };
> > +    };
> > +...  
> 
> 


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

* Re: [PATCH 24/24] iio:cdc:ad7150: Move driver out of staging.
  2021-02-07 15:46 ` [PATCH 24/24] iio:cdc:ad7150: Move driver out of staging Jonathan Cameron
@ 2021-02-07 16:21   ` Jonathan Cameron
  0 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-07 16:21 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

I forgot to disable rename detection, which is always a good idea for graduation
patches like this as it allows people to see the state of the whole driver.

Anyhow, see below.

From 634ca2e94ffdd7cf14f952263ee9f15fef782b70 Mon Sep 17 00:00:00 2001
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Date: Sun, 7 Feb 2021 15:07:17 +0000
Subject: [PATCH 24/24] iio:cdc:ad7150: Move driver out of staging.

This capacitance to digital converter (CDC) driver is compliant with
the IIO ABI.  Note, not all features supported (e.g. window event modes)
but the driver should be in a useful functional state.

The cleanup was done against QEMU emulation of the device rather than
actual hardware.   Whilst this was a bit of an experiment, it made it
easy to confirm that the driver remained in a consistent working state
through the various refactors.  If it worked in the first place, it
should still be working after this cleanup.

Given some IIO drivers require expensive hardware setups, (not particularly
true with this one) the use of QEMU may provide a viable way forward
for providing testing during code changes where previously we'd had
to rely on sharp eyes and crossed fingers.

Note, no explicit MAINTAINERS entry as it will be covered by the
generic catch-alls for ADI and IIO drivers which are sufficient.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/Kconfig              |   1 +
 drivers/iio/Makefile             |   1 +
 drivers/iio/cdc/Kconfig          |  17 +
 drivers/iio/cdc/Makefile         |   6 +
 drivers/iio/cdc/ad7150.c         | 678 +++++++++++++++++++++++++++++++
 drivers/staging/iio/cdc/Kconfig  |  10 -
 drivers/staging/iio/cdc/Makefile |   3 +-
 drivers/staging/iio/cdc/ad7150.c | 678 -------------------------------
 8 files changed, 704 insertions(+), 690 deletions(-)

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 267553386c71..a5c47300e5a0 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -72,6 +72,7 @@ source "drivers/iio/accel/Kconfig"
 source "drivers/iio/adc/Kconfig"
 source "drivers/iio/afe/Kconfig"
 source "drivers/iio/amplifiers/Kconfig"
+source "drivers/iio/cdc/Kconfig"
 source "drivers/iio/chemical/Kconfig"
 source "drivers/iio/common/Kconfig"
 source "drivers/iio/dac/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 1712011c0f4a..1cb1434b86b2 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -18,6 +18,7 @@ obj-y += adc/
 obj-y += afe/
 obj-y += amplifiers/
 obj-y += buffer/
+obj-y += cdc/
 obj-y += chemical/
 obj-y += common/
 obj-y += dac/
diff --git a/drivers/iio/cdc/Kconfig b/drivers/iio/cdc/Kconfig
new file mode 100644
index 000000000000..5e3319a3ff48
--- /dev/null
+++ b/drivers/iio/cdc/Kconfig
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# CDC drivers
+#
+menu "Capacitance to digital converters"
+
+config AD7150
+	tristate "Analog Devices ad7150/1/6 capacitive sensor driver"
+	depends on I2C
+	help
+	  Say yes here to build support for Analog Devices capacitive sensors.
+	  (ad7150, ad7151, ad7156) Provides direct access via sysfs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ad7150.
+
+endmenu
diff --git a/drivers/iio/cdc/Makefile b/drivers/iio/cdc/Makefile
new file mode 100644
index 000000000000..ee490637b032
--- /dev/null
+++ b/drivers/iio/cdc/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for industrial I/O capacitance to digital converter (CDC) drivers
+#
+
+obj-$(CONFIG_AD7150) += ad7150.o
diff --git a/drivers/iio/cdc/ad7150.c b/drivers/iio/cdc/ad7150.c
new file mode 100644
index 000000000000..9a6b55021fa7
--- /dev/null
+++ b/drivers/iio/cdc/ad7150.c
@@ -0,0 +1,678 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD7150 capacitive sensor driver supporting AD7150/1/6
+ *
+ * Copyright 2010-2011 Analog Devices Inc.
+ * Copyright 2021 Jonathan Cameron <Jonathan.Cameron@huawei.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/events.h>
+
+#define AD7150_STATUS_REG		0
+#define   AD7150_STATUS_OUT1		BIT(3)
+#define   AD7150_STATUS_OUT2		BIT(5)
+#define AD7150_CH1_DATA_HIGH_REG	1
+#define AD7150_CH2_DATA_HIGH_REG	3
+#define AD7150_CH1_AVG_HIGH_REG		5
+#define AD7150_CH2_AVG_HIGH_REG		7
+#define AD7150_CH1_SENSITIVITY_REG	9
+#define AD7150_CH1_THR_HOLD_H_REG	9
+#define AD7150_CH1_TIMEOUT_REG		10
+#define   AD7150_CH_TIMEOUT_RECEDING	GENMASK(3, 0)
+#define   AD7150_CH_TIMEOUT_APPROACHING	GENMASK(7, 4)
+#define AD7150_CH1_SETUP_REG		11
+#define AD7150_CH2_SENSITIVITY_REG	12
+#define AD7150_CH2_THR_HOLD_H_REG	12
+#define AD7150_CH2_TIMEOUT_REG		13
+#define AD7150_CH2_SETUP_REG		14
+#define AD7150_CFG_REG			15
+#define   AD7150_CFG_FIX		BIT(7)
+#define   AD7150_CFG_THRESHTYPE_MSK	GENMASK(6, 5)
+#define   AD7150_CFG_TT_NEG		0x0
+#define   AD7150_CFG_TT_POS		0x1
+#define   AD7150_CFG_TT_IN_WINDOW	0x2
+#define   AD7150_CFG_TT_OUT_WINDOW	0x3
+#define AD7150_PD_TIMER_REG		16
+#define AD7150_CH1_CAPDAC_REG		17
+#define AD7150_CH2_CAPDAC_REG		18
+#define AD7150_SN3_REG			19
+#define AD7150_SN2_REG			20
+#define AD7150_SN1_REG			21
+#define AD7150_SN0_REG			22
+#define AD7150_ID_REG			23
+
+enum {
+	AD7150,
+	AD7151,
+};
+
+/**
+ * struct ad7150_chip_info - instance specific chip data
+ * @client: i2c client for this device
+ * @threshold: thresholds for simple capacitance value events
+ * @thresh_sensitivity: threshold for simple capacitance offset
+ *	from 'average' value.
+ * @thresh_timeout: a timeout, in samples from the moment an
+ *	adaptive threshold event occurs to when the average
+ *	value jumps to current value.  Note made up of two fields,
+ *      3:0 are for timeout receding - applies if below lower threshold
+ *      7:4 are for timeout approaching - applies if above upper threshold
+ * @state_lock: ensure consistent state of this structure wrt the
+ *	hardware.
+ * @interrupts: one or two interrupt numbers depending on device type.
+ * @int_enabled: is a given interrupt currently enabled.
+ * @type: threshold type
+ * @dir: threshold direction
+ */
+struct ad7150_chip_info {
+	struct i2c_client *client;
+	u16 threshold[2][2];
+	u8 thresh_sensitivity[2][2];
+	u8 thresh_timeout[2][2];
+	struct mutex state_lock;
+	int interrupts[2];
+	bool int_enabled[2];
+	enum iio_event_type type;
+	enum iio_event_direction dir;
+};
+
+static const u8 ad7150_addresses[][6] = {
+	{ AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
+	  AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
+	  AD7150_CH1_SENSITIVITY_REG, AD7150_CH1_TIMEOUT_REG },
+	{ AD7150_CH2_DATA_HIGH_REG, AD7150_CH2_AVG_HIGH_REG,
+	  AD7150_CH2_SETUP_REG, AD7150_CH2_THR_HOLD_H_REG,
+	  AD7150_CH2_SENSITIVITY_REG, AD7150_CH2_TIMEOUT_REG },
+};
+
+static int ad7150_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val,
+			   int *val2,
+			   long mask)
+{
+	struct ad7150_chip_info *chip = iio_priv(indio_dev);
+	int channel = chan->channel;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = i2c_smbus_read_word_swapped(chip->client,
+						  ad7150_addresses[channel][0]);
+		if (ret < 0)
+			return ret;
+		*val = ret >> 4;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_AVERAGE_RAW:
+		ret = i2c_smbus_read_word_swapped(chip->client,
+						  ad7150_addresses[channel][1]);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * Base units for capacitance are nano farads and the value
+		 * calculated from the datasheet formula is in picofarad
+		 * so multiply by 1000
+		 */
+		*val = 1000;
+		*val2 = 40944 >> 4; /* To match shift in _RAW */
+		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = -(12288 >> 4); /* To match shift in _RAW */
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		/* Strangely same for both 1 and 2 chan parts */
+		*val = 100;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad7150_read_event_config(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir)
+{
+	struct ad7150_chip_info *chip = iio_priv(indio_dev);
+	u8 threshtype;
+	bool thrfixed;
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
+	if (ret < 0)
+		return ret;
+
+	threshtype = FIELD_GET(AD7150_CFG_THRESHTYPE_MSK, ret);
+
+	/*check if threshold mode is fixed or adaptive*/
+	thrfixed = FIELD_GET(AD7150_CFG_FIX, ret);
+
+	switch (type) {
+	case IIO_EV_TYPE_THRESH_ADAPTIVE:
+		if (dir == IIO_EV_DIR_RISING)
+			return !thrfixed && (threshtype == AD7150_CFG_TT_POS);
+		return !thrfixed && (threshtype == AD7150_CFG_TT_NEG);
+	case IIO_EV_TYPE_THRESH:
+		if (dir == IIO_EV_DIR_RISING)
+			return thrfixed && (threshtype == AD7150_CFG_TT_POS);
+		return thrfixed && (threshtype == AD7150_CFG_TT_NEG);
+	default:
+		break;
+	}
+	return -EINVAL;
+}
+
+/* state_lock should be held to ensure consistent state */
+static int ad7150_write_event_params(struct iio_dev *indio_dev,
+				     unsigned int chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir)
+{
+	struct ad7150_chip_info *chip = iio_priv(indio_dev);
+	int rising = (dir == IIO_EV_DIR_RISING);
+
+	/* Only update value live, if parameter is in use */
+	if ((type != chip->type) || (dir != chip->dir))
+		return 0;
+
+	switch (type) {
+		/* Note completely different from the adaptive versions */
+	case IIO_EV_TYPE_THRESH: {
+		u16 value = chip->threshold[rising][chan];
+		return i2c_smbus_write_word_swapped(chip->client,
+						    ad7150_addresses[chan][3],
+						    value);
+	}
+	case IIO_EV_TYPE_THRESH_ADAPTIVE: {
+		int ret;
+		u8 sens, timeout;
+
+		sens = chip->thresh_sensitivity[rising][chan];
+		ret = i2c_smbus_write_byte_data(chip->client,
+						ad7150_addresses[chan][4],
+						sens);
+		if (ret)
+			return ret;
+
+		/*
+		 * Single timeout register contains timeouts for both
+		 * directions.
+		 */
+		timeout = FIELD_PREP(AD7150_CH_TIMEOUT_APPROACHING,
+				     chip->thresh_timeout[1][chan]);
+		timeout |= FIELD_PREP(AD7150_CH_TIMEOUT_RECEDING,
+				      chip->thresh_timeout[0][chan]);
+		return i2c_smbus_write_byte_data(chip->client,
+						 ad7150_addresses[chan][5],
+						 timeout);
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad7150_write_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir, int state)
+{
+	struct ad7150_chip_info *chip = iio_priv(indio_dev);
+	int ret;
+
+	/*
+	 * There is only a single shared control and no on chip
+	 * interrupt disables for the two interrupt lines.
+	 * So, enabling will switch the events configured to enable
+	 * whatever was most recently requested and if necessary enable_irq()
+	 * the interrupt and any disable will disable_irq() for that
+	 * channels interrupt.
+	 */
+	if (!state) {
+		if ((chip->int_enabled[chan->channel]) &&
+		    (type == chip->type) && (dir == chip->dir)) {
+			disable_irq(chip->interrupts[chan->channel]);
+			chip->int_enabled[chan->channel] = false;
+		}
+		return 0;
+	}
+
+	mutex_lock(&chip->state_lock);
+	if ((type != chip->type) || (dir != chip->dir)) {
+		int rising = (dir == IIO_EV_DIR_RISING);
+		u8 thresh_type, cfg, adaptive;
+
+		/*
+		 * Need to temporarily disable both interrupts if
+		 * enabled - this is to avoid races around changing
+		 * config and thresholds.
+		 * Note enable/disable_irq() are reference counted so
+		 * no need to check if already enabled.
+		 */
+		disable_irq(chip->interrupts[0]);
+		disable_irq(chip->interrupts[1]);
+
+		ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
+		if (ret < 0)
+			goto error_ret;
+
+		cfg = ret & ~(AD7150_CFG_THRESHTYPE_MSK | AD7150_CFG_FIX);
+
+		if (type == IIO_EV_TYPE_THRESH_ADAPTIVE)
+			adaptive = 1;
+		else
+			adaptive = 0;
+
+		if (rising)
+			thresh_type = AD7150_CFG_TT_POS;
+		else
+			thresh_type = AD7150_CFG_TT_NEG;
+
+		cfg |= FIELD_PREP(AD7150_CFG_FIX, !adaptive) |
+			FIELD_PREP(AD7150_CFG_THRESHTYPE_MSK, thresh_type);
+
+		ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG_REG,
+						cfg);
+		if (ret < 0)
+			goto error_ret;
+
+		/*
+		 * There is a potential race condition here, but not easy
+		 * to close given we can't disable the interrupt at the
+		 * chip side of things. Rely on the status bit.
+		 */
+		chip->type = type;
+		chip->dir = dir;
+
+		/* update control attributes */
+		ret = ad7150_write_event_params(indio_dev, chan->channel, type,
+						dir);
+		if (ret)
+			goto error_ret;
+		/* reenable any irq's we disabled whilst changing mode */
+		enable_irq(chip->interrupts[0]);
+		enable_irq(chip->interrupts[1]);
+	}
+	if (!chip->int_enabled[chan->channel]) {
+		enable_irq(chip->interrupts[chan->channel]);
+		chip->int_enabled[chan->channel] = true;
+	}
+
+error_ret:
+	mutex_unlock(&chip->state_lock);
+
+	return ret;
+}
+
+static int ad7150_read_event_value(struct iio_dev *indio_dev,
+				   const struct iio_chan_spec *chan,
+				   enum iio_event_type type,
+				   enum iio_event_direction dir,
+				   enum iio_event_info info,
+				   int *val, int *val2)
+{
+	struct ad7150_chip_info *chip = iio_priv(indio_dev);
+	int rising = (dir == IIO_EV_DIR_RISING);
+
+	/* Complex register sharing going on here */
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH_ADAPTIVE:
+			*val = chip->thresh_sensitivity[rising][chan->channel];
+			return IIO_VAL_INT;
+		case IIO_EV_TYPE_THRESH:
+			*val = chip->threshold[rising][chan->channel];
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_INFO_TIMEOUT:
+		*val = 0;
+		*val2 = chip->thresh_timeout[rising][chan->channel] * 10000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad7150_write_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int val, int val2)
+{
+	int ret;
+	struct ad7150_chip_info *chip = iio_priv(indio_dev);
+	int rising = (dir == IIO_EV_DIR_RISING);
+
+	mutex_lock(&chip->state_lock);
+	switch (info) {
+	case IIO_EV_INFO_VALUE:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH_ADAPTIVE:
+			chip->thresh_sensitivity[rising][chan->channel] = val;
+			break;
+		case IIO_EV_TYPE_THRESH:
+			chip->threshold[rising][chan->channel] = val;
+			break;
+		default:
+			ret = -EINVAL;
+			goto error_ret;
+		}
+		break;
+	case IIO_EV_INFO_TIMEOUT: {
+		/*
+		 * Raw timeout is in cycles of 10 msecs as long as both
+		 * channels are enabled.
+		 * In terms of INT_PLUS_MICRO, that is in units of 10,000
+		 */
+		int timeout = val2 / 10000;
+
+		if (val != 0 || timeout < 0 || timeout > 15 || val2 % 10000) {
+			ret = -EINVAL;
+			goto error_ret;
+		}
+
+		chip->thresh_timeout[rising][chan->channel] = timeout;
+		break;
+	}
+	default:
+		ret = -EINVAL;
+		goto error_ret;
+	}
+
+	/* write back if active */
+	ret = ad7150_write_event_params(indio_dev, chan->channel, type, dir);
+
+error_ret:
+	mutex_unlock(&chip->state_lock);
+	return ret;
+}
+
+static const struct iio_event_spec ad7150_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+			BIT(IIO_EV_INFO_ENABLE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+			BIT(IIO_EV_INFO_ENABLE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH_ADAPTIVE,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+			BIT(IIO_EV_INFO_ENABLE) |
+			BIT(IIO_EV_INFO_TIMEOUT),
+	}, {
+		.type = IIO_EV_TYPE_THRESH_ADAPTIVE,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+			BIT(IIO_EV_INFO_ENABLE) |
+			BIT(IIO_EV_INFO_TIMEOUT),
+	},
+};
+
+#define AD7150_CAPACITANCE_CHAN(_chan)	{			\
+		.type = IIO_CAPACITANCE,			\
+		.indexed = 1,					\
+		.channel = _chan,				\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+		BIT(IIO_CHAN_INFO_AVERAGE_RAW),			\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+			BIT(IIO_CHAN_INFO_OFFSET),		\
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
+		.event_spec = ad7150_events,			\
+		.num_event_specs = ARRAY_SIZE(ad7150_events),	\
+	}
+
+#define AD7150_CAPACITANCE_CHAN_NO_IRQ(_chan)	{		\
+		.type = IIO_CAPACITANCE,			\
+		.indexed = 1,					\
+		.channel = _chan,				\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+		BIT(IIO_CHAN_INFO_AVERAGE_RAW),			\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
+			BIT(IIO_CHAN_INFO_OFFSET),		\
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
+	}
+
+static const struct iio_chan_spec ad7150_channels[] = {
+	AD7150_CAPACITANCE_CHAN(0),
+	AD7150_CAPACITANCE_CHAN(1),
+};
+
+static const struct iio_chan_spec ad7150_channels_no_irq[] = {
+	AD7150_CAPACITANCE_CHAN_NO_IRQ(0),
+	AD7150_CAPACITANCE_CHAN_NO_IRQ(1),
+};
+
+static const struct iio_chan_spec ad7151_channels[] = {
+	AD7150_CAPACITANCE_CHAN(0),
+};
+
+static const struct iio_chan_spec ad7151_channels_no_irq[] = {
+	AD7150_CAPACITANCE_CHAN_NO_IRQ(0),
+};
+
+static irqreturn_t __ad7150_event_handler(void *private, u8 status_mask,
+					  int channel)
+{
+	struct iio_dev *indio_dev = private;
+	struct ad7150_chip_info *chip = iio_priv(indio_dev);
+	s64 timestamp = iio_get_time_ns(indio_dev);
+	int int_status;
+
+	int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS_REG);
+	if (int_status < 0)
+		return IRQ_HANDLED;
+
+	/*
+	 * There are race conditions around enabling and disabling that
+	 * could easily land us here with a spurious interrupt.
+	 * Just eat it if so.
+	 */
+	if (!(int_status & status_mask))
+		return IRQ_HANDLED;
+
+	iio_push_event(indio_dev,
+		       IIO_UNMOD_EVENT_CODE(IIO_CAPACITANCE, channel,
+					    chip->type, chip->dir),
+		       timestamp);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ad7150_event_handler_ch1(int irq, void *private)
+{
+	return __ad7150_event_handler(private, AD7150_STATUS_OUT1, 0);
+}
+
+static irqreturn_t ad7150_event_handler_ch2(int irq, void *private)
+{
+	return __ad7150_event_handler(private, AD7150_STATUS_OUT2, 1);
+}
+
+static IIO_CONST_ATTR(in_capacitance_thresh_adaptive_timeout_available,
+		      "[0 0.01 0.15]");
+
+static struct attribute *ad7150_event_attributes[] = {
+	&iio_const_attr_in_capacitance_thresh_adaptive_timeout_available
+	.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group ad7150_event_attribute_group = {
+	.attrs = ad7150_event_attributes,
+	.name = "events",
+};
+
+static const struct iio_info ad7150_info = {
+	.event_attrs = &ad7150_event_attribute_group,
+	.read_raw = &ad7150_read_raw,
+	.read_event_config = &ad7150_read_event_config,
+	.write_event_config = &ad7150_write_event_config,
+	.read_event_value = &ad7150_read_event_value,
+	.write_event_value = &ad7150_write_event_value,
+};
+
+static const struct iio_info ad7150_info_no_irq = {
+	.read_raw = &ad7150_read_raw,
+};
+
+static void ad7150_reg_disable(void *data)
+{
+	struct regulator *reg = data;
+
+	regulator_disable(reg);
+}
+
+static int ad7150_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct ad7150_chip_info *chip;
+	struct iio_dev *indio_dev;
+	struct regulator *reg;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	chip = iio_priv(indio_dev);
+	mutex_init(&chip->state_lock);
+	chip->client = client;
+
+	indio_dev->name = id->name;
+
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	reg = devm_regulator_get(&client->dev, "vdd");
+	if (IS_ERR(reg))
+		return PTR_ERR(reg);
+
+	ret = regulator_enable(reg);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&client->dev, ad7150_reg_disable, reg);
+	if (ret)
+		return ret;
+
+	chip->interrupts[0] = fwnode_irq_get(dev_fwnode(&client->dev), 0);
+	if (chip->interrupts[0] < 0)
+		return chip->interrupts[0];
+	if (id->driver_data == AD7150) {
+		chip->interrupts[1] = fwnode_irq_get(dev_fwnode(&client->dev), 1);
+		if (chip->interrupts[1] < 0)
+			return chip->interrupts[1];
+	}
+	if (chip->interrupts[0] &&
+	    (id->driver_data == AD7151 || chip->interrupts[1])) {
+		irq_set_status_flags(chip->interrupts[0], IRQ_NOAUTOEN);
+		ret = devm_request_threaded_irq(&client->dev,
+						chip->interrupts[0],
+						NULL,
+						&ad7150_event_handler_ch1,
+						IRQF_TRIGGER_RISING |
+						IRQF_ONESHOT,
+						"ad7150_irq1",
+						indio_dev);
+		if (ret)
+			return ret;
+
+		indio_dev->info = &ad7150_info;
+		switch (id->driver_data) {
+		case AD7150:
+			indio_dev->channels = ad7150_channels;
+			indio_dev->num_channels = ARRAY_SIZE(ad7150_channels);
+			irq_set_status_flags(chip->interrupts[1], IRQ_NOAUTOEN);
+			ret = devm_request_threaded_irq(&client->dev,
+							chip->interrupts[1],
+							NULL,
+							&ad7150_event_handler_ch2,
+							IRQF_TRIGGER_RISING |
+							IRQF_ONESHOT,
+							"ad7150_irq2",
+							indio_dev);
+			if (ret)
+				return ret;
+			break;
+		case AD7151:
+			indio_dev->channels = ad7151_channels;
+			indio_dev->num_channels = ARRAY_SIZE(ad7151_channels);
+			break;
+		default:
+			return -EINVAL;
+		}
+
+	} else {
+		indio_dev->info = &ad7150_info_no_irq;
+		switch (id->driver_data) {
+		case AD7150:
+			indio_dev->channels = ad7150_channels_no_irq;
+			indio_dev->num_channels =
+				ARRAY_SIZE(ad7150_channels_no_irq);
+			break;
+		case AD7151:
+			indio_dev->channels = ad7151_channels_no_irq;
+			indio_dev->num_channels =
+				ARRAY_SIZE(ad7151_channels_no_irq);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
+}
+
+static const struct i2c_device_id ad7150_id[] = {
+	{ "ad7150", AD7150 },
+	{ "ad7151", AD7151 },
+	{ "ad7156", AD7150 },
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, ad7150_id);
+
+static const struct of_device_id ad7150_of_match[] = {
+	{ "adi,ad7150" },
+	{ "adi,ad7151" },
+	{ "adi,ad7156" },
+	{}
+};
+static struct i2c_driver ad7150_driver = {
+	.driver = {
+		.name = "ad7150",
+		.of_match_table = ad7150_of_match,
+	},
+	.probe = ad7150_probe,
+	.id_table = ad7150_id,
+};
+module_i2c_driver(ad7150_driver);
+
+MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
+MODULE_DESCRIPTION("Analog Devices AD7150/1/6 capacitive sensor driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/cdc/Kconfig b/drivers/staging/iio/cdc/Kconfig
index e0a5ce66a984..a7386bbbcb79 100644
--- a/drivers/staging/iio/cdc/Kconfig
+++ b/drivers/staging/iio/cdc/Kconfig
@@ -4,16 +4,6 @@
 #
 menu "Capacitance to digital converters"
 
-config AD7150
-	tristate "Analog Devices ad7150/1/6 capacitive sensor driver"
-	depends on I2C
-	help
-	  Say yes here to build support for Analog Devices capacitive sensors.
-	  (ad7150, ad7151, ad7156) Provides direct access via sysfs.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called ad7150.
-
 config AD7746
 	tristate "Analog Devices AD7745, AD7746 AD7747 capacitive sensor driver"
 	depends on I2C
diff --git a/drivers/staging/iio/cdc/Makefile b/drivers/staging/iio/cdc/Makefile
index ab8222579e7e..afb7499a7090 100644
--- a/drivers/staging/iio/cdc/Makefile
+++ b/drivers/staging/iio/cdc/Makefile
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 #
-# Makefile for industrial I/O DAC drivers
+# Makefile for industrial I/O CDC drivers
 #
 
-obj-$(CONFIG_AD7150) += ad7150.o
 obj-$(CONFIG_AD7746) += ad7746.o
diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
deleted file mode 100644
index 9a6b55021fa7..000000000000
--- a/drivers/staging/iio/cdc/ad7150.c
+++ /dev/null
@@ -1,678 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * AD7150 capacitive sensor driver supporting AD7150/1/6
- *
- * Copyright 2010-2011 Analog Devices Inc.
- * Copyright 2021 Jonathan Cameron <Jonathan.Cameron@huawei.com>
- */
-
-#include <linux/bitfield.h>
-#include <linux/device.h>
-#include <linux/interrupt.h>
-#include <linux/irq.h>
-#include <linux/i2c.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/mod_devicetable.h>
-#include <linux/regulator/consumer.h>
-#include <linux/slab.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-#include <linux/iio/events.h>
-
-#define AD7150_STATUS_REG		0
-#define   AD7150_STATUS_OUT1		BIT(3)
-#define   AD7150_STATUS_OUT2		BIT(5)
-#define AD7150_CH1_DATA_HIGH_REG	1
-#define AD7150_CH2_DATA_HIGH_REG	3
-#define AD7150_CH1_AVG_HIGH_REG		5
-#define AD7150_CH2_AVG_HIGH_REG		7
-#define AD7150_CH1_SENSITIVITY_REG	9
-#define AD7150_CH1_THR_HOLD_H_REG	9
-#define AD7150_CH1_TIMEOUT_REG		10
-#define   AD7150_CH_TIMEOUT_RECEDING	GENMASK(3, 0)
-#define   AD7150_CH_TIMEOUT_APPROACHING	GENMASK(7, 4)
-#define AD7150_CH1_SETUP_REG		11
-#define AD7150_CH2_SENSITIVITY_REG	12
-#define AD7150_CH2_THR_HOLD_H_REG	12
-#define AD7150_CH2_TIMEOUT_REG		13
-#define AD7150_CH2_SETUP_REG		14
-#define AD7150_CFG_REG			15
-#define   AD7150_CFG_FIX		BIT(7)
-#define   AD7150_CFG_THRESHTYPE_MSK	GENMASK(6, 5)
-#define   AD7150_CFG_TT_NEG		0x0
-#define   AD7150_CFG_TT_POS		0x1
-#define   AD7150_CFG_TT_IN_WINDOW	0x2
-#define   AD7150_CFG_TT_OUT_WINDOW	0x3
-#define AD7150_PD_TIMER_REG		16
-#define AD7150_CH1_CAPDAC_REG		17
-#define AD7150_CH2_CAPDAC_REG		18
-#define AD7150_SN3_REG			19
-#define AD7150_SN2_REG			20
-#define AD7150_SN1_REG			21
-#define AD7150_SN0_REG			22
-#define AD7150_ID_REG			23
-
-enum {
-	AD7150,
-	AD7151,
-};
-
-/**
- * struct ad7150_chip_info - instance specific chip data
- * @client: i2c client for this device
- * @threshold: thresholds for simple capacitance value events
- * @thresh_sensitivity: threshold for simple capacitance offset
- *	from 'average' value.
- * @thresh_timeout: a timeout, in samples from the moment an
- *	adaptive threshold event occurs to when the average
- *	value jumps to current value.  Note made up of two fields,
- *      3:0 are for timeout receding - applies if below lower threshold
- *      7:4 are for timeout approaching - applies if above upper threshold
- * @state_lock: ensure consistent state of this structure wrt the
- *	hardware.
- * @interrupts: one or two interrupt numbers depending on device type.
- * @int_enabled: is a given interrupt currently enabled.
- * @type: threshold type
- * @dir: threshold direction
- */
-struct ad7150_chip_info {
-	struct i2c_client *client;
-	u16 threshold[2][2];
-	u8 thresh_sensitivity[2][2];
-	u8 thresh_timeout[2][2];
-	struct mutex state_lock;
-	int interrupts[2];
-	bool int_enabled[2];
-	enum iio_event_type type;
-	enum iio_event_direction dir;
-};
-
-static const u8 ad7150_addresses[][6] = {
-	{ AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
-	  AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
-	  AD7150_CH1_SENSITIVITY_REG, AD7150_CH1_TIMEOUT_REG },
-	{ AD7150_CH2_DATA_HIGH_REG, AD7150_CH2_AVG_HIGH_REG,
-	  AD7150_CH2_SETUP_REG, AD7150_CH2_THR_HOLD_H_REG,
-	  AD7150_CH2_SENSITIVITY_REG, AD7150_CH2_TIMEOUT_REG },
-};
-
-static int ad7150_read_raw(struct iio_dev *indio_dev,
-			   struct iio_chan_spec const *chan,
-			   int *val,
-			   int *val2,
-			   long mask)
-{
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	int channel = chan->channel;
-	int ret;
-
-	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
-		ret = i2c_smbus_read_word_swapped(chip->client,
-						  ad7150_addresses[channel][0]);
-		if (ret < 0)
-			return ret;
-		*val = ret >> 4;
-
-		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_AVERAGE_RAW:
-		ret = i2c_smbus_read_word_swapped(chip->client,
-						  ad7150_addresses[channel][1]);
-		if (ret < 0)
-			return ret;
-		*val = ret;
-
-		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_SCALE:
-		/*
-		 * Base units for capacitance are nano farads and the value
-		 * calculated from the datasheet formula is in picofarad
-		 * so multiply by 1000
-		 */
-		*val = 1000;
-		*val2 = 40944 >> 4; /* To match shift in _RAW */
-		return IIO_VAL_FRACTIONAL;
-	case IIO_CHAN_INFO_OFFSET:
-		*val = -(12288 >> 4); /* To match shift in _RAW */
-		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_SAMP_FREQ:
-		/* Strangely same for both 1 and 2 chan parts */
-		*val = 100;
-		return IIO_VAL_INT;
-	default:
-		return -EINVAL;
-	}
-}
-
-static int ad7150_read_event_config(struct iio_dev *indio_dev,
-				    const struct iio_chan_spec *chan,
-				    enum iio_event_type type,
-				    enum iio_event_direction dir)
-{
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	u8 threshtype;
-	bool thrfixed;
-	int ret;
-
-	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
-	if (ret < 0)
-		return ret;
-
-	threshtype = FIELD_GET(AD7150_CFG_THRESHTYPE_MSK, ret);
-
-	/*check if threshold mode is fixed or adaptive*/
-	thrfixed = FIELD_GET(AD7150_CFG_FIX, ret);
-
-	switch (type) {
-	case IIO_EV_TYPE_THRESH_ADAPTIVE:
-		if (dir == IIO_EV_DIR_RISING)
-			return !thrfixed && (threshtype == AD7150_CFG_TT_POS);
-		return !thrfixed && (threshtype == AD7150_CFG_TT_NEG);
-	case IIO_EV_TYPE_THRESH:
-		if (dir == IIO_EV_DIR_RISING)
-			return thrfixed && (threshtype == AD7150_CFG_TT_POS);
-		return thrfixed && (threshtype == AD7150_CFG_TT_NEG);
-	default:
-		break;
-	}
-	return -EINVAL;
-}
-
-/* state_lock should be held to ensure consistent state */
-static int ad7150_write_event_params(struct iio_dev *indio_dev,
-				     unsigned int chan,
-				     enum iio_event_type type,
-				     enum iio_event_direction dir)
-{
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	int rising = (dir == IIO_EV_DIR_RISING);
-
-	/* Only update value live, if parameter is in use */
-	if ((type != chip->type) || (dir != chip->dir))
-		return 0;
-
-	switch (type) {
-		/* Note completely different from the adaptive versions */
-	case IIO_EV_TYPE_THRESH: {
-		u16 value = chip->threshold[rising][chan];
-		return i2c_smbus_write_word_swapped(chip->client,
-						    ad7150_addresses[chan][3],
-						    value);
-	}
-	case IIO_EV_TYPE_THRESH_ADAPTIVE: {
-		int ret;
-		u8 sens, timeout;
-
-		sens = chip->thresh_sensitivity[rising][chan];
-		ret = i2c_smbus_write_byte_data(chip->client,
-						ad7150_addresses[chan][4],
-						sens);
-		if (ret)
-			return ret;
-
-		/*
-		 * Single timeout register contains timeouts for both
-		 * directions.
-		 */
-		timeout = FIELD_PREP(AD7150_CH_TIMEOUT_APPROACHING,
-				     chip->thresh_timeout[1][chan]);
-		timeout |= FIELD_PREP(AD7150_CH_TIMEOUT_RECEDING,
-				      chip->thresh_timeout[0][chan]);
-		return i2c_smbus_write_byte_data(chip->client,
-						 ad7150_addresses[chan][5],
-						 timeout);
-	}
-	default:
-		return -EINVAL;
-	}
-}
-
-static int ad7150_write_event_config(struct iio_dev *indio_dev,
-				     const struct iio_chan_spec *chan,
-				     enum iio_event_type type,
-				     enum iio_event_direction dir, int state)
-{
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	int ret;
-
-	/*
-	 * There is only a single shared control and no on chip
-	 * interrupt disables for the two interrupt lines.
-	 * So, enabling will switch the events configured to enable
-	 * whatever was most recently requested and if necessary enable_irq()
-	 * the interrupt and any disable will disable_irq() for that
-	 * channels interrupt.
-	 */
-	if (!state) {
-		if ((chip->int_enabled[chan->channel]) &&
-		    (type == chip->type) && (dir == chip->dir)) {
-			disable_irq(chip->interrupts[chan->channel]);
-			chip->int_enabled[chan->channel] = false;
-		}
-		return 0;
-	}
-
-	mutex_lock(&chip->state_lock);
-	if ((type != chip->type) || (dir != chip->dir)) {
-		int rising = (dir == IIO_EV_DIR_RISING);
-		u8 thresh_type, cfg, adaptive;
-
-		/*
-		 * Need to temporarily disable both interrupts if
-		 * enabled - this is to avoid races around changing
-		 * config and thresholds.
-		 * Note enable/disable_irq() are reference counted so
-		 * no need to check if already enabled.
-		 */
-		disable_irq(chip->interrupts[0]);
-		disable_irq(chip->interrupts[1]);
-
-		ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
-		if (ret < 0)
-			goto error_ret;
-
-		cfg = ret & ~(AD7150_CFG_THRESHTYPE_MSK | AD7150_CFG_FIX);
-
-		if (type == IIO_EV_TYPE_THRESH_ADAPTIVE)
-			adaptive = 1;
-		else
-			adaptive = 0;
-
-		if (rising)
-			thresh_type = AD7150_CFG_TT_POS;
-		else
-			thresh_type = AD7150_CFG_TT_NEG;
-
-		cfg |= FIELD_PREP(AD7150_CFG_FIX, !adaptive) |
-			FIELD_PREP(AD7150_CFG_THRESHTYPE_MSK, thresh_type);
-
-		ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG_REG,
-						cfg);
-		if (ret < 0)
-			goto error_ret;
-
-		/*
-		 * There is a potential race condition here, but not easy
-		 * to close given we can't disable the interrupt at the
-		 * chip side of things. Rely on the status bit.
-		 */
-		chip->type = type;
-		chip->dir = dir;
-
-		/* update control attributes */
-		ret = ad7150_write_event_params(indio_dev, chan->channel, type,
-						dir);
-		if (ret)
-			goto error_ret;
-		/* reenable any irq's we disabled whilst changing mode */
-		enable_irq(chip->interrupts[0]);
-		enable_irq(chip->interrupts[1]);
-	}
-	if (!chip->int_enabled[chan->channel]) {
-		enable_irq(chip->interrupts[chan->channel]);
-		chip->int_enabled[chan->channel] = true;
-	}
-
-error_ret:
-	mutex_unlock(&chip->state_lock);
-
-	return ret;
-}
-
-static int ad7150_read_event_value(struct iio_dev *indio_dev,
-				   const struct iio_chan_spec *chan,
-				   enum iio_event_type type,
-				   enum iio_event_direction dir,
-				   enum iio_event_info info,
-				   int *val, int *val2)
-{
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	int rising = (dir == IIO_EV_DIR_RISING);
-
-	/* Complex register sharing going on here */
-	switch (info) {
-	case IIO_EV_INFO_VALUE:
-		switch (type) {
-		case IIO_EV_TYPE_THRESH_ADAPTIVE:
-			*val = chip->thresh_sensitivity[rising][chan->channel];
-			return IIO_VAL_INT;
-		case IIO_EV_TYPE_THRESH:
-			*val = chip->threshold[rising][chan->channel];
-			return IIO_VAL_INT;
-		default:
-			return -EINVAL;
-		}
-	case IIO_EV_INFO_TIMEOUT:
-		*val = 0;
-		*val2 = chip->thresh_timeout[rising][chan->channel] * 10000;
-		return IIO_VAL_INT_PLUS_MICRO;
-	default:
-		return -EINVAL;
-	}
-}
-
-static int ad7150_write_event_value(struct iio_dev *indio_dev,
-				    const struct iio_chan_spec *chan,
-				    enum iio_event_type type,
-				    enum iio_event_direction dir,
-				    enum iio_event_info info,
-				    int val, int val2)
-{
-	int ret;
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	int rising = (dir == IIO_EV_DIR_RISING);
-
-	mutex_lock(&chip->state_lock);
-	switch (info) {
-	case IIO_EV_INFO_VALUE:
-		switch (type) {
-		case IIO_EV_TYPE_THRESH_ADAPTIVE:
-			chip->thresh_sensitivity[rising][chan->channel] = val;
-			break;
-		case IIO_EV_TYPE_THRESH:
-			chip->threshold[rising][chan->channel] = val;
-			break;
-		default:
-			ret = -EINVAL;
-			goto error_ret;
-		}
-		break;
-	case IIO_EV_INFO_TIMEOUT: {
-		/*
-		 * Raw timeout is in cycles of 10 msecs as long as both
-		 * channels are enabled.
-		 * In terms of INT_PLUS_MICRO, that is in units of 10,000
-		 */
-		int timeout = val2 / 10000;
-
-		if (val != 0 || timeout < 0 || timeout > 15 || val2 % 10000) {
-			ret = -EINVAL;
-			goto error_ret;
-		}
-
-		chip->thresh_timeout[rising][chan->channel] = timeout;
-		break;
-	}
-	default:
-		ret = -EINVAL;
-		goto error_ret;
-	}
-
-	/* write back if active */
-	ret = ad7150_write_event_params(indio_dev, chan->channel, type, dir);
-
-error_ret:
-	mutex_unlock(&chip->state_lock);
-	return ret;
-}
-
-static const struct iio_event_spec ad7150_events[] = {
-	{
-		.type = IIO_EV_TYPE_THRESH,
-		.dir = IIO_EV_DIR_RISING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-			BIT(IIO_EV_INFO_ENABLE),
-	}, {
-		.type = IIO_EV_TYPE_THRESH,
-		.dir = IIO_EV_DIR_FALLING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-			BIT(IIO_EV_INFO_ENABLE),
-	}, {
-		.type = IIO_EV_TYPE_THRESH_ADAPTIVE,
-		.dir = IIO_EV_DIR_RISING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-			BIT(IIO_EV_INFO_ENABLE) |
-			BIT(IIO_EV_INFO_TIMEOUT),
-	}, {
-		.type = IIO_EV_TYPE_THRESH_ADAPTIVE,
-		.dir = IIO_EV_DIR_FALLING,
-		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
-			BIT(IIO_EV_INFO_ENABLE) |
-			BIT(IIO_EV_INFO_TIMEOUT),
-	},
-};
-
-#define AD7150_CAPACITANCE_CHAN(_chan)	{			\
-		.type = IIO_CAPACITANCE,			\
-		.indexed = 1,					\
-		.channel = _chan,				\
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
-		BIT(IIO_CHAN_INFO_AVERAGE_RAW),			\
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
-			BIT(IIO_CHAN_INFO_OFFSET),		\
-		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
-		.event_spec = ad7150_events,			\
-		.num_event_specs = ARRAY_SIZE(ad7150_events),	\
-	}
-
-#define AD7150_CAPACITANCE_CHAN_NO_IRQ(_chan)	{		\
-		.type = IIO_CAPACITANCE,			\
-		.indexed = 1,					\
-		.channel = _chan,				\
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
-		BIT(IIO_CHAN_INFO_AVERAGE_RAW),			\
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | \
-			BIT(IIO_CHAN_INFO_OFFSET),		\
-		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),\
-	}
-
-static const struct iio_chan_spec ad7150_channels[] = {
-	AD7150_CAPACITANCE_CHAN(0),
-	AD7150_CAPACITANCE_CHAN(1),
-};
-
-static const struct iio_chan_spec ad7150_channels_no_irq[] = {
-	AD7150_CAPACITANCE_CHAN_NO_IRQ(0),
-	AD7150_CAPACITANCE_CHAN_NO_IRQ(1),
-};
-
-static const struct iio_chan_spec ad7151_channels[] = {
-	AD7150_CAPACITANCE_CHAN(0),
-};
-
-static const struct iio_chan_spec ad7151_channels_no_irq[] = {
-	AD7150_CAPACITANCE_CHAN_NO_IRQ(0),
-};
-
-static irqreturn_t __ad7150_event_handler(void *private, u8 status_mask,
-					  int channel)
-{
-	struct iio_dev *indio_dev = private;
-	struct ad7150_chip_info *chip = iio_priv(indio_dev);
-	s64 timestamp = iio_get_time_ns(indio_dev);
-	int int_status;
-
-	int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS_REG);
-	if (int_status < 0)
-		return IRQ_HANDLED;
-
-	/*
-	 * There are race conditions around enabling and disabling that
-	 * could easily land us here with a spurious interrupt.
-	 * Just eat it if so.
-	 */
-	if (!(int_status & status_mask))
-		return IRQ_HANDLED;
-
-	iio_push_event(indio_dev,
-		       IIO_UNMOD_EVENT_CODE(IIO_CAPACITANCE, channel,
-					    chip->type, chip->dir),
-		       timestamp);
-
-	return IRQ_HANDLED;
-}
-
-static irqreturn_t ad7150_event_handler_ch1(int irq, void *private)
-{
-	return __ad7150_event_handler(private, AD7150_STATUS_OUT1, 0);
-}
-
-static irqreturn_t ad7150_event_handler_ch2(int irq, void *private)
-{
-	return __ad7150_event_handler(private, AD7150_STATUS_OUT2, 1);
-}
-
-static IIO_CONST_ATTR(in_capacitance_thresh_adaptive_timeout_available,
-		      "[0 0.01 0.15]");
-
-static struct attribute *ad7150_event_attributes[] = {
-	&iio_const_attr_in_capacitance_thresh_adaptive_timeout_available
-	.dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group ad7150_event_attribute_group = {
-	.attrs = ad7150_event_attributes,
-	.name = "events",
-};
-
-static const struct iio_info ad7150_info = {
-	.event_attrs = &ad7150_event_attribute_group,
-	.read_raw = &ad7150_read_raw,
-	.read_event_config = &ad7150_read_event_config,
-	.write_event_config = &ad7150_write_event_config,
-	.read_event_value = &ad7150_read_event_value,
-	.write_event_value = &ad7150_write_event_value,
-};
-
-static const struct iio_info ad7150_info_no_irq = {
-	.read_raw = &ad7150_read_raw,
-};
-
-static void ad7150_reg_disable(void *data)
-{
-	struct regulator *reg = data;
-
-	regulator_disable(reg);
-}
-
-static int ad7150_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
-{
-	struct ad7150_chip_info *chip;
-	struct iio_dev *indio_dev;
-	struct regulator *reg;
-	int ret;
-
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
-	if (!indio_dev)
-		return -ENOMEM;
-
-	chip = iio_priv(indio_dev);
-	mutex_init(&chip->state_lock);
-	chip->client = client;
-
-	indio_dev->name = id->name;
-
-	indio_dev->modes = INDIO_DIRECT_MODE;
-
-	reg = devm_regulator_get(&client->dev, "vdd");
-	if (IS_ERR(reg))
-		return PTR_ERR(reg);
-
-	ret = regulator_enable(reg);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(&client->dev, ad7150_reg_disable, reg);
-	if (ret)
-		return ret;
-
-	chip->interrupts[0] = fwnode_irq_get(dev_fwnode(&client->dev), 0);
-	if (chip->interrupts[0] < 0)
-		return chip->interrupts[0];
-	if (id->driver_data == AD7150) {
-		chip->interrupts[1] = fwnode_irq_get(dev_fwnode(&client->dev), 1);
-		if (chip->interrupts[1] < 0)
-			return chip->interrupts[1];
-	}
-	if (chip->interrupts[0] &&
-	    (id->driver_data == AD7151 || chip->interrupts[1])) {
-		irq_set_status_flags(chip->interrupts[0], IRQ_NOAUTOEN);
-		ret = devm_request_threaded_irq(&client->dev,
-						chip->interrupts[0],
-						NULL,
-						&ad7150_event_handler_ch1,
-						IRQF_TRIGGER_RISING |
-						IRQF_ONESHOT,
-						"ad7150_irq1",
-						indio_dev);
-		if (ret)
-			return ret;
-
-		indio_dev->info = &ad7150_info;
-		switch (id->driver_data) {
-		case AD7150:
-			indio_dev->channels = ad7150_channels;
-			indio_dev->num_channels = ARRAY_SIZE(ad7150_channels);
-			irq_set_status_flags(chip->interrupts[1], IRQ_NOAUTOEN);
-			ret = devm_request_threaded_irq(&client->dev,
-							chip->interrupts[1],
-							NULL,
-							&ad7150_event_handler_ch2,
-							IRQF_TRIGGER_RISING |
-							IRQF_ONESHOT,
-							"ad7150_irq2",
-							indio_dev);
-			if (ret)
-				return ret;
-			break;
-		case AD7151:
-			indio_dev->channels = ad7151_channels;
-			indio_dev->num_channels = ARRAY_SIZE(ad7151_channels);
-			break;
-		default:
-			return -EINVAL;
-		}
-
-	} else {
-		indio_dev->info = &ad7150_info_no_irq;
-		switch (id->driver_data) {
-		case AD7150:
-			indio_dev->channels = ad7150_channels_no_irq;
-			indio_dev->num_channels =
-				ARRAY_SIZE(ad7150_channels_no_irq);
-			break;
-		case AD7151:
-			indio_dev->channels = ad7151_channels_no_irq;
-			indio_dev->num_channels =
-				ARRAY_SIZE(ad7151_channels_no_irq);
-			break;
-		default:
-			return -EINVAL;
-		}
-	}
-
-	return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
-}
-
-static const struct i2c_device_id ad7150_id[] = {
-	{ "ad7150", AD7150 },
-	{ "ad7151", AD7151 },
-	{ "ad7156", AD7150 },
-	{}
-};
-
-MODULE_DEVICE_TABLE(i2c, ad7150_id);
-
-static const struct of_device_id ad7150_of_match[] = {
-	{ "adi,ad7150" },
-	{ "adi,ad7151" },
-	{ "adi,ad7156" },
-	{}
-};
-static struct i2c_driver ad7150_driver = {
-	.driver = {
-		.name = "ad7150",
-		.of_match_table = ad7150_of_match,
-	},
-	.probe = ad7150_probe,
-	.id_table = ad7150_id,
-};
-module_i2c_driver(ad7150_driver);
-
-MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices AD7150/1/6 capacitive sensor driver");
-MODULE_LICENSE("GPL v2");
-- 
2.30.0


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

* Re: [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table
  2021-02-07 15:46 ` [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table Jonathan Cameron
@ 2021-02-08  7:11   ` Alexandru Ardelean
  2021-02-11 15:51     ` Jonathan Cameron
  2021-02-08  7:50   ` Song Bao Hua (Barry Song)
  1 sibling, 1 reply; 46+ messages in thread
From: Alexandru Ardelean @ 2021-02-08  7:11 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Lars-Peter Clausen, Michael Hennerich, song.bao.hua,
	Rob Herring, Jonathan Cameron

On Sun, Feb 7, 2021 at 5:52 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Rather than using the fallback path in the i2c subsystem and hoping
> for no clashes across vendors, lets put in an explicit table for
> matching.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/staging/iio/cdc/ad7150.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
> index 0bc8c7a99883..33c8a78c076f 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -12,6 +12,7 @@
>  #include <linux/i2c.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>
> @@ -655,9 +656,16 @@ static const struct i2c_device_id ad7150_id[] = {
>
>  MODULE_DEVICE_TABLE(i2c, ad7150_id);
>
> +static const struct of_device_id ad7150_of_match[] = {
> +       { "adi,ad7150" },
> +       { "adi,ad7151" },
> +       { "adi,ad7156" },

Is this missing some match_driver_data logic?
Something like this:
https://patchwork.kernel.org/project/linux-iio/patch/20210207154623.433442-7-jic23@kernel.org/
?

> +       {}
> +};
>  static struct i2c_driver ad7150_driver = {
>         .driver = {
>                 .name = "ad7150",
> +               .of_match_table = ad7150_of_match,
>         },
>         .probe = ad7150_probe,
>         .id_table = ad7150_id,
> --
> 2.30.0
>

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

* Re: [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate
  2021-02-07 16:12 ` [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
@ 2021-02-08  7:20   ` Alexandru Ardelean
  0 siblings, 0 replies; 46+ messages in thread
From: Alexandru Ardelean @ 2021-02-08  7:20 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Lars-Peter Clausen, Michael Hennerich, song.bao.hua,
	Rob Herring, Jonathan Cameron

On Sun, Feb 7, 2021 at 6:13 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sun,  7 Feb 2021 15:45:59 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > This is an 'old' driver in IIO that has been in staging a while.
> > First submitted in October 2010.
> >
> > I wanted to try and experiment and picked this driver to try it with.
> >
> > The cleanup etc here was all tested against some basic emulation
> > added to QEMU rather than real hardware.  Once I've cleaned that up
> > a tiny bit I'll push it up to https://github.com/jic23/qemu
> Now up at:
> https://github.com/jic23/qemu/tree/iio-ad7150
>
> The device emulation is rather messy.
> As arm / virt builds the DT tree alongside the instantiation of
> devices it was a very convenient place to hack an i2c bus and the ad7150
> in using the existing pl061 to provide suitable interrupt lines.
>
> Note that I don't care about VM migration or anything like that
> so the implementation is rather more minimal than would be needed
> for normal QEMU device emulation.
>
> Anyhow, gives an idea of what is needed for anyone who is curious.
>
> qom-list /machine/unattached/device[9]
> lists the various things that can be controlled including.
> ch2_capacitance (int)
> ch1_capacitance_avg (int)
> ch2_event_state (int)
> ch1_event_state (int)
> ch2_capacitance_avg (int)
> ch1_capacitance (int)
>
> Event state is a bit simplistic in that you have direct control of
> the gpio lines. I could have implemented a state machine so that the
> actual thresholds etc were used, but the adaptive mode is sufficiently
> complex that I haven't yet done so.

I'm a little unsure about patch 12/24 " staging:iio:cdc:ad7150: Rework
interrupt handling"
And there's a minor issue with the of_match patch.
But for the rest [excluding the bindings ones and ABI ones [I'm not
great with ABI and bindings]]:

Reviewed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

This would be interesting to get into the chipmake BUs, as there
wouldn't be a need to create a board-farm for testing.

>
> Jonathan
>
> > Note that for now I'm not proposing to upstream this to QEMU but
> > would be interested in hearing if people thing it is a good idea to
> > do so.
> >
> > Whilst it's obviously hard to be absolutely sure that the emulation is
> > correct, the fact that the original driver worked as expected and the
> > cleaned up version still does is certainly encouraging.
> >
> > Note however, that there were a few more significant changes in here than
> > basic cleanup.
> > 1. Interrupts / events were handled in a rather esoteric fashion.
> >    (Always on, window modes represented as magnitudes).
> >    Note that for two channel devices there are separate lines. The original
> >    driver was not supporting this at all.
> >    They now look more like a standard IIO driver and reflect experience
> >    that we've gained over the years in dealing with devices where these
> >    aren't interrupt lines as such, but rather reporters of current status.
> > 2. Timeouts were handled in a fashion that clearly wouldn't work.
> >
> > Note that this moving out of staging makes a few bits of ABI 'official'
> > and so those are added to the main IIO ABI Docs.
> >
> > Thanks in advance to anyone who has time to take a look.
> >
> > Jonathan
> >
> >
> > Jonathan Cameron (24):
> >   staging:iio:cdc:ad7150: use swapped reads for i2c rather than open
> >     coding.
> >   staging:iio:cdc:ad7150: Remove magnitude adaptive events
> >   staging:iio:cdc:ad7150: Refactor event parameter update
> >   staging:iio:cdc:ad7150: Timeout register covers both directions so
> >     both need updating
> >   staging:iio:cdc:ad7150: Drop platform data support
> >   staging:iio:cdc:ad7150: Handle variation in chan_spec across device
> >     and irq present or not
> >   staging:iio:cdc:ad7150: Simplify event handling by only using rising
> >     direction.
> >   staging:iio:cdc:ad7150: Drop noisy print in probe
> >   staging:iio:cdc:ad7150: Add sampling_frequency support
> >   iio:event: Add timeout event info type
> >   staging:iio:cdc:ad7150: Change timeout units to seconds and use core
> >     support
> >   staging:iio:cdc:ad7150: Rework interrupt handling.
> >   staging:iio:cdc:ad7150: More consistent register and field naming
> >   staging:iio:cdc:ad7150: Reorganize headers.
> >   staging:iio:cdc:ad7150: Tidy up local variable positioning.
> >   staging:iio:cdc:ad7150: Drop unnecessary block comments.
> >   staging:iio:cdc:ad7150: Shift the _raw readings by 4 bits.
> >   staging:iio:cdc:ad7150: Add scale and offset to
> >     info_mask_shared_by_type
> >   staging:iio:cdc:ad7150: Really basic regulator support.
> >   staging:iio:cdc:ad7150: Add of_match_table
> >   dt-bindings:iio:cdc:adi,ad7150 binding doc
> >   iio:Documentation:ABI Add missing elements as used by the adi,ad7150
> >   staging:iio:cdc:ad7150: Add copyright notice given substantial
> >     changes.
> >   iio:cdc:ad7150: Move driver out of staging.
> >
> >  Documentation/ABI/testing/sysfs-bus-iio       |  33 +
> >  .../bindings/iio/cdc/adi,ad7150.yaml          |  69 ++
> >  drivers/iio/Kconfig                           |   1 +
> >  drivers/iio/Makefile                          |   1 +
> >  drivers/iio/cdc/Kconfig                       |  17 +
> >  drivers/iio/cdc/Makefile                      |   6 +
> >  drivers/iio/cdc/ad7150.c                      | 678 ++++++++++++++++++
> >  drivers/iio/industrialio-event.c              |   1 +
> >  drivers/staging/iio/cdc/Kconfig               |  10 -
> >  drivers/staging/iio/cdc/Makefile              |   3 +-
> >  drivers/staging/iio/cdc/ad7150.c              | 655 -----------------
> >  include/linux/iio/types.h                     |   1 +
> >  12 files changed, 808 insertions(+), 667 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/iio/cdc/adi,ad7150.yaml
> >  create mode 100644 drivers/iio/cdc/Kconfig
> >  create mode 100644 drivers/iio/cdc/Makefile
> >  create mode 100644 drivers/iio/cdc/ad7150.c
> >  delete mode 100644 drivers/staging/iio/cdc/ad7150.c
> >
>

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

* RE: [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table
  2021-02-07 15:46 ` [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table Jonathan Cameron
  2021-02-08  7:11   ` Alexandru Ardelean
@ 2021-02-08  7:50   ` Song Bao Hua (Barry Song)
  2021-02-08  8:01     ` Alexandru Ardelean
  1 sibling, 1 reply; 46+ messages in thread
From: Song Bao Hua (Barry Song) @ 2021-02-08  7:50 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, robh+dt, Jonathan Cameron



> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: Monday, February 8, 2021 4:46 AM
> To: linux-iio@vger.kernel.org
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Rather than using the fallback path in the i2c subsystem and hoping
> for no clashes across vendors, lets put in an explicit table for
> matching.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/staging/iio/cdc/ad7150.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index 0bc8c7a99883..33c8a78c076f 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -12,6 +12,7 @@
>  #include <linux/i2c.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
> 
> @@ -655,9 +656,16 @@ static const struct i2c_device_id ad7150_id[] = {
> 
>  MODULE_DEVICE_TABLE(i2c, ad7150_id);
> 
> +static const struct of_device_id ad7150_of_match[] = {
> +	{ "adi,ad7150" },
> +	{ "adi,ad7151" },
> +	{ "adi,ad7156" },
> +	{}
> +};

Does it compile if CONFIG_OF is not enabled?

>  static struct i2c_driver ad7150_driver = {
>  	.driver = {
>  		.name = "ad7150",
> +		.of_match_table = ad7150_of_match,

of_match_ptr(ad7150_of_match)?

Do we need dt-binding doc?


>  	},
>  	.probe = ad7150_probe,
>  	.id_table = ad7150_id,
> --
> 2.30.0

Thanks
Barry


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

* RE: [PATCH 16/24] staging:iio:cdc:ad7150: Drop unnecessary block comments.
  2021-02-07 15:46 ` [PATCH 16/24] staging:iio:cdc:ad7150: Drop unnecessary block comments Jonathan Cameron
@ 2021-02-08  7:52   ` Song Bao Hua (Barry Song)
  0 siblings, 0 replies; 46+ messages in thread
From: Song Bao Hua (Barry Song) @ 2021-02-08  7:52 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, robh+dt, Jonathan Cameron



> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: Monday, February 8, 2021 4:46 AM
> To: linux-iio@vger.kernel.org
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: [PATCH 16/24] staging:iio:cdc:ad7150: Drop unnecessary block
> comments.
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> These have a habit of not getting updated with driver reorganizations
> and don't add much info so drop them.
> 
> Also fix a minor comment syntax issue.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

Reviewed-by: Barry Song <song.bao.hua@hisilicon.com>

>  drivers/staging/iio/cdc/ad7150.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index d530b467d1b2..4c83e6e37c5a 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -17,9 +17,6 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/events.h>
> -/*
> - * AD7150 registers definition
> - */
> 
>  #define AD7150_STATUS_REG		0
>  #define   AD7150_STATUS_OUT1		BIT(3)
> @@ -89,10 +86,6 @@ struct ad7150_chip_info {
>  	enum iio_event_direction dir;
>  };
> 
> -/*
> - * sysfs nodes
> - */
> -
>  static const u8 ad7150_addresses[][6] = {
>  	{ AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
>  	  AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
> @@ -172,8 +165,7 @@ static int ad7150_read_event_config(struct iio_dev
> *indio_dev,
>  	return -EINVAL;
>  }
> 
> -/* state_lock should be held to ensure consistent state*/
> -
> +/* state_lock should be held to ensure consistent state */
>  static int ad7150_write_event_params(struct iio_dev *indio_dev,
>  				     unsigned int chan,
>  				     enum iio_event_type type,
> --
> 2.30.0


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

* RE: [PATCH 14/24] staging:iio:cdc:ad7150: Reorganize headers.
  2021-02-07 15:46 ` [PATCH 14/24] staging:iio:cdc:ad7150: Reorganize headers Jonathan Cameron
@ 2021-02-08  7:54   ` Song Bao Hua (Barry Song)
  0 siblings, 0 replies; 46+ messages in thread
From: Song Bao Hua (Barry Song) @ 2021-02-08  7:54 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, robh+dt, Jonathan Cameron



> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: Monday, February 8, 2021 4:46 AM
> To: linux-iio@vger.kernel.org
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: [PATCH 14/24] staging:iio:cdc:ad7150: Reorganize headers.
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Whilst not important, it's nice to have the general headers in
> alphabetical order.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

Reviewed-by: Barry Song <song.bao.hua@hisilicon.com>

>  drivers/staging/iio/cdc/ad7150.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index 5d4783da7f98..9e88e774752f 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -6,13 +6,13 @@
>   */
> 
>  #include <linux/bitfield.h>
> -#include <linux/interrupt.h>
>  #include <linux/device.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -#include <linux/i2c.h>
> +#include <linux/interrupt.h>
>  #include <linux/irq.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/slab.h>
> 
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> --
> 2.30.0


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

* RE: [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable positioning.
  2021-02-07 15:46 ` [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable positioning Jonathan Cameron
@ 2021-02-08  7:54   ` Song Bao Hua (Barry Song)
  2021-03-14 17:46     ` Jonathan Cameron
  0 siblings, 1 reply; 46+ messages in thread
From: Song Bao Hua (Barry Song) @ 2021-02-08  7:54 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, robh+dt, Jonathan Cameron



> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: Monday, February 8, 2021 4:46 AM
> To: linux-iio@vger.kernel.org
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable
> positioning.
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Where there is no other basis on which to order declarations
> let us prefer reverse xmas tree.  Also reduce scope where
> sensible.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Barry Song <song.bao.hua@hisilicon.com>

> ---
>  drivers/staging/iio/cdc/ad7150.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index 9e88e774752f..d530b467d1b2 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -108,9 +108,9 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
>  			   int *val2,
>  			   long mask)
>  {
> -	int ret;
>  	struct ad7150_chip_info *chip = iio_priv(indio_dev);
>  	int channel = chan->channel;
> +	int ret;
> 
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> @@ -143,10 +143,10 @@ static int ad7150_read_event_config(struct iio_dev
> *indio_dev,
>  				    enum iio_event_type type,
>  				    enum iio_event_direction dir)
>  {
> -	int ret;
> +	struct ad7150_chip_info *chip = iio_priv(indio_dev);
>  	u8 threshtype;
>  	bool thrfixed;
> -	struct ad7150_chip_info *chip = iio_priv(indio_dev);
> +	int ret;
> 
>  	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
>  	if (ret < 0)
> @@ -227,10 +227,8 @@ static int ad7150_write_event_config(struct iio_dev
> *indio_dev,
>  				     enum iio_event_type type,
>  				     enum iio_event_direction dir, int state)
>  {
> -	u8 thresh_type, cfg, adaptive;
> -	int ret;
>  	struct ad7150_chip_info *chip = iio_priv(indio_dev);
> -	int rising = (dir == IIO_EV_DIR_RISING);
> +	int ret;
> 
>  	/*
>  	 * There is only a single shared control and no on chip
> @@ -251,6 +249,8 @@ static int ad7150_write_event_config(struct iio_dev
> *indio_dev,
> 
>  	mutex_lock(&chip->state_lock);
>  	if ((type != chip->type) || (dir != chip->dir)) {
> +		int rising = (dir == IIO_EV_DIR_RISING);
> +		u8 thresh_type, cfg, adaptive;
> 
>  		/*
>  		 * Need to temporarily disable both interrupts if
> @@ -533,9 +533,9 @@ static const struct iio_info ad7150_info_no_irq = {
>  static int ad7150_probe(struct i2c_client *client,
>  			const struct i2c_device_id *id)
>  {
> -	int ret;
>  	struct ad7150_chip_info *chip;
>  	struct iio_dev *indio_dev;
> +	int ret;
> 
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
>  	if (!indio_dev)
> --
> 2.30.0


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

* RE: [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support
  2021-02-07 15:46 ` [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support Jonathan Cameron
@ 2021-02-08  8:01   ` Song Bao Hua (Barry Song)
  2021-02-08 11:12     ` Jonathan Cameron
  0 siblings, 1 reply; 46+ messages in thread
From: Song Bao Hua (Barry Song) @ 2021-02-08  8:01 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, robh+dt, Jonathan Cameron



> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: Monday, February 8, 2021 4:46 AM
> To: linux-iio@vger.kernel.org
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> There are no mainline board files using this driver so lets drop
> the platform_data support in favour of devicetree and similar.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/staging/iio/cdc/ad7150.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index 0dce1b8ce76d..7ad9105e6b46 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -570,20 +570,6 @@ static int ad7150_probe(struct i2c_client *client,
>  			return ret;
>  	}
> 
> -	if (client->dev.platform_data) {
> -		ret = devm_request_threaded_irq(&client->dev, *(unsigned int *)
> -						client->dev.platform_data,
> -						NULL,
> -						&ad7150_event_handler,
> -						IRQF_TRIGGER_RISING |
> -						IRQF_TRIGGER_FALLING |
> -						IRQF_ONESHOT,
> -						"ad7150_irq2",
> -						indio_dev);
> -		if (ret)
> -			return ret;
> -	}
> -

The original code looks ugly, I forget when ad7150 needs
the second interrupt.

>  	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
>  	if (ret)
>  		return ret;
> --
> 2.30.0

Thanks
Barry


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

* Re: [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table
  2021-02-08  7:50   ` Song Bao Hua (Barry Song)
@ 2021-02-08  8:01     ` Alexandru Ardelean
  0 siblings, 0 replies; 46+ messages in thread
From: Alexandru Ardelean @ 2021-02-08  8:01 UTC (permalink / raw)
  To: Song Bao Hua (Barry Song)
  Cc: Jonathan Cameron, linux-iio, Lars-Peter Clausen,
	Michael Hennerich, robh+dt, Jonathan Cameron

On Mon, Feb 8, 2021 at 9:52 AM Song Bao Hua (Barry Song)
<song.bao.hua@hisilicon.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Jonathan Cameron [mailto:jic23@kernel.org]
> > Sent: Monday, February 8, 2021 4:46 AM
> > To: linux-iio@vger.kernel.org
> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> > <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> > <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> > <jonathan.cameron@huawei.com>
> > Subject: [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Rather than using the fallback path in the i2c subsystem and hoping
> > for no clashes across vendors, lets put in an explicit table for
> > matching.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/staging/iio/cdc/ad7150.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/staging/iio/cdc/ad7150.c
> > b/drivers/staging/iio/cdc/ad7150.c
> > index 0bc8c7a99883..33c8a78c076f 100644
> > --- a/drivers/staging/iio/cdc/ad7150.c
> > +++ b/drivers/staging/iio/cdc/ad7150.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/i2c.h>
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/slab.h>
> >
> > @@ -655,9 +656,16 @@ static const struct i2c_device_id ad7150_id[] = {
> >
> >  MODULE_DEVICE_TABLE(i2c, ad7150_id);
> >
> > +static const struct of_device_id ad7150_of_match[] = {
> > +     { "adi,ad7150" },
> > +     { "adi,ad7151" },
> > +     { "adi,ad7156" },
> > +     {}
> > +};
>
> Does it compile if CONFIG_OF is not enabled?
>
> >  static struct i2c_driver ad7150_driver = {
> >       .driver = {
> >               .name = "ad7150",
> > +             .of_match_table = ad7150_of_match,
>
> of_match_ptr(ad7150_of_match)?

of_match_ptr() is not recommended anymore because of the ACPI PRP0001
thing/compat with OF;

>
> Do we need dt-binding doc?

Should be here:
https://lore.kernel.org/linux-iio/20210207161820.28abeb33@archlinux/T/#u

>
>
> >       },
> >       .probe = ad7150_probe,
> >       .id_table = ad7150_id,
> > --
> > 2.30.0
>
> Thanks
> Barry
>

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

* RE: [PATCH 08/24] staging:iio:cdc:ad7150: Drop noisy print in probe
  2021-02-07 15:46 ` [PATCH 08/24] staging:iio:cdc:ad7150: Drop noisy print in probe Jonathan Cameron
@ 2021-02-08  8:02   ` Song Bao Hua (Barry Song)
  0 siblings, 0 replies; 46+ messages in thread
From: Song Bao Hua (Barry Song) @ 2021-02-08  8:02 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, robh+dt, Jonathan Cameron



> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: Monday, February 8, 2021 4:46 AM
> To: linux-iio@vger.kernel.org
> Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: [PATCH 08/24] staging:iio:cdc:ad7150: Drop noisy print in probe
> 
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Also
> * drop i2c_set_client_data() as now unused.
> * white space cleanups
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Barry Song <song.bao.hua@hisilicon.com>

> ---
>  drivers/staging/iio/cdc/ad7150.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index 34e6afe52f0e..8f8e472e3240 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -573,11 +573,9 @@ static int ad7150_probe(struct i2c_client *client,
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
>  	if (!indio_dev)
>  		return -ENOMEM;
> +
>  	chip = iio_priv(indio_dev);
>  	mutex_init(&chip->state_lock);
> -	/* this is only used for device removal purposes */
> -	i2c_set_clientdata(client, indio_dev);
> -
>  	chip->client = client;
> 
>  	indio_dev->name = id->name;
> @@ -624,14 +622,7 @@ static int ad7150_probe(struct i2c_client *client,
>  		}
>  	}
> 
> -	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> -	if (ret)
> -		return ret;
> -
> -	dev_info(&client->dev, "%s capacitive sensor registered,irq: %d\n",
> -		 id->name, client->irq);
> -
> -	return 0;
> +	return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
>  }
> 
>  static const struct i2c_device_id ad7150_id[] = {
> --
> 2.30.0


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

* RE: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
  2021-02-07 16:00   ` Lars-Peter Clausen
  2021-02-07 16:18     ` Jonathan Cameron
@ 2021-02-08  8:12     ` Song Bao Hua (Barry Song)
  2021-02-08 11:20       ` Jonathan Cameron
  1 sibling, 1 reply; 46+ messages in thread
From: Song Bao Hua (Barry Song) @ 2021-02-08  8:12 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jonathan Cameron, linux-iio
  Cc: Michael Hennerich, robh+dt, Jonathan Cameron, devicetree



> -----Original Message-----
> From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> Sent: Monday, February 8, 2021 5:00 AM
> To: Jonathan Cameron <jic23@kernel.org>; linux-iio@vger.kernel.org
> Cc: Michael Hennerich <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> <jonathan.cameron@huawei.com>; devicetree@vger.kernel.org
> Subject: Re: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
> 
> On 2/7/21 4:46 PM, Jonathan Cameron wrote:
> > +required:
> > +  - compatible
> > +  - reg
> 
> Is vdd-supply really optional the way it is implemented in the driver?
> 
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        cdc@48 {
> > +            compatible = "adi,ad7150";
> > +            reg = <0x48>;
> > +            interrupts = <25 2>, <26 2>;

One question, here we have two interrupts, but the driver is reading
one interrupt only, do we need to call
of_irq_get(dev, index)
or
of_irq_get_byname()?


> 
> I wonder if we should use the symbolic constants for the IRQ type to
> make the example more clear. E.g.
> 
> interrupts = <25 IRQ_TYPE_EDGE_FALLING>, ...
> 
> > +            interrupt-parent = <&gpio>;
> > +        };
> > +    };
> > +...
> 
Thanks
Barry


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

* Re: [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support
  2021-02-08  8:01   ` Song Bao Hua (Barry Song)
@ 2021-02-08 11:12     ` Jonathan Cameron
  0 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-08 11:12 UTC (permalink / raw)
  To: Song Bao Hua (Barry Song)
  Cc: Jonathan Cameron, linux-iio, Lars-Peter Clausen,
	Michael Hennerich, robh+dt

On Mon, 8 Feb 2021 08:01:10 +0000
"Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron [mailto:jic23@kernel.org]
> > Sent: Monday, February 8, 2021 4:46 AM
> > To: linux-iio@vger.kernel.org
> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> > <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> > <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> > <jonathan.cameron@huawei.com>
> > Subject: [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support
> > 
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > There are no mainline board files using this driver so lets drop
> > the platform_data support in favour of devicetree and similar.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/staging/iio/cdc/ad7150.c | 14 --------------
> >  1 file changed, 14 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/cdc/ad7150.c
> > b/drivers/staging/iio/cdc/ad7150.c
> > index 0dce1b8ce76d..7ad9105e6b46 100644
> > --- a/drivers/staging/iio/cdc/ad7150.c
> > +++ b/drivers/staging/iio/cdc/ad7150.c
> > @@ -570,20 +570,6 @@ static int ad7150_probe(struct i2c_client *client,
> >  			return ret;
> >  	}
> > 
> > -	if (client->dev.platform_data) {
> > -		ret = devm_request_threaded_irq(&client->dev, *(unsigned int *)
> > -						client->dev.platform_data,
> > -						NULL,
> > -						&ad7150_event_handler,
> > -						IRQF_TRIGGER_RISING |
> > -						IRQF_TRIGGER_FALLING |
> > -						IRQF_ONESHOT,
> > -						"ad7150_irq2",
> > -						indio_dev);
> > -		if (ret)
> > -			return ret;
> > -	}
> > -  
> 
> The original code looks ugly, I forget when ad7150 needs
> the second interrupt.
It is one per channel.  That is events on a channel are only indicated on the
line associated with the channel.

I bring it back later in the series.

Jonathan

> 
> >  	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> >  	if (ret)
> >  		return ret;
> > --
> > 2.30.0  
> 
> Thanks
> Barry
> 


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

* Re: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
  2021-02-08  8:12     ` Song Bao Hua (Barry Song)
@ 2021-02-08 11:20       ` Jonathan Cameron
  0 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-08 11:20 UTC (permalink / raw)
  To: Song Bao Hua (Barry Song)
  Cc: Lars-Peter Clausen, Jonathan Cameron, linux-iio,
	Michael Hennerich, robh+dt, devicetree

On Mon, 8 Feb 2021 08:12:21 +0000
"Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com> wrote:

> > -----Original Message-----
> > From: Lars-Peter Clausen [mailto:lars@metafoo.de]
> > Sent: Monday, February 8, 2021 5:00 AM
> > To: Jonathan Cameron <jic23@kernel.org>; linux-iio@vger.kernel.org
> > Cc: Michael Hennerich <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> > <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> > <jonathan.cameron@huawei.com>; devicetree@vger.kernel.org
> > Subject: Re: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
> > 
> > On 2/7/21 4:46 PM, Jonathan Cameron wrote:  
> > > +required:
> > > +  - compatible
> > > +  - reg  
> > 
> > Is vdd-supply really optional the way it is implemented in the driver?
> >   
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    i2c {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        cdc@48 {
> > > +            compatible = "adi,ad7150";
> > > +            reg = <0x48>;
> > > +            interrupts = <25 2>, <26 2>;  
> 
> One question, here we have two interrupts, but the driver is reading
> one interrupt only, do we need to call
> of_irq_get(dev, index)
> or
> of_irq_get_byname()?

Driver reads both now (again).  It wasn't the cleanest transition in this series.
I dropped the second irq in patch 5 and brought it back in patch 12.

Whilst we could in theory support only one interrupt, that would make the driver
annoyingly fiddly for something I suspect no one would actually do.
We'd need to support events only on one of the two channels to make it work.

So I vote we take lazy option of saying it's either no interrupts, or both of
them for the two channel devices.  Nothing stops us relaxing that in future
and then using get_by_name.  If we do that and also require names are in order
INT1 INT2, INT1, INT2 but not INT2 INT1 and hence optional except in the case
of only one interrupt provided.

1) New DT, old driver - no interrupts but then there weren't any previously anyway.
2) New Driver old DT - fine, either both are specified or neither.

So there is a clean migration path if we ever find anyone who has wired up only
one of the interrupt lines and really does only want events on one or two channels
but still wants readings on both of them.

Jonathan


> 
> 
> > 
> > I wonder if we should use the symbolic constants for the IRQ type to
> > make the example more clear. E.g.
> > 
> > interrupts = <25 IRQ_TYPE_EDGE_FALLING>, ...
> >   
> > > +            interrupt-parent = <&gpio>;
> > > +        };
> > > +    };
> > > +...  
> >   
> Thanks
> Barry
> 


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

* Re: [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table
  2021-02-08  7:11   ` Alexandru Ardelean
@ 2021-02-11 15:51     ` Jonathan Cameron
  0 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-11 15:51 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: Jonathan Cameron, linux-iio, Lars-Peter Clausen,
	Michael Hennerich, song.bao.hua, Rob Herring

On Mon, 8 Feb 2021 09:11:21 +0200
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:

> On Sun, Feb 7, 2021 at 5:52 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Rather than using the fallback path in the i2c subsystem and hoping
> > for no clashes across vendors, lets put in an explicit table for
> > matching.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/staging/iio/cdc/ad7150.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
> > index 0bc8c7a99883..33c8a78c076f 100644
> > --- a/drivers/staging/iio/cdc/ad7150.c
> > +++ b/drivers/staging/iio/cdc/ad7150.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/i2c.h>
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/slab.h>
> >
> > @@ -655,9 +656,16 @@ static const struct i2c_device_id ad7150_id[] = {
> >
> >  MODULE_DEVICE_TABLE(i2c, ad7150_id);
> >
> > +static const struct of_device_id ad7150_of_match[] = {
> > +       { "adi,ad7150" },
> > +       { "adi,ad7151" },
> > +       { "adi,ad7156" },  
> 
> Is this missing some match_driver_data logic?

Odd though it may seem, we can still use id->driver_data
even though we matched against these ids from the point of view
of probe. 

https://elixir.bootlin.com/linux/latest/source/drivers/i2c/i2c-core-base.c#L528
In particular the call of i2c_match_id(driver->id_table, client)
which is passed (buried in client) via of_modalias_node() the
of_device_id string with vendor prefix stripped. 

We could do firmware specific match data logic, but we'd then need
to fallback to the i2c id->driver_data anyway if it failed.

So as things currently stand I think it is better to avoid duplication
of the data and just keep it in the one table.

The disadvantage is that we have to keep the two ID tables in sync.

Jonathan


> Something like this:
> https://patchwork.kernel.org/project/linux-iio/patch/20210207154623.433442-7-jic23@kernel.org/
> ?
> 
> > +       {}
> > +};
> >  static struct i2c_driver ad7150_driver = {
> >         .driver = {
> >                 .name = "ad7150",
> > +               .of_match_table = ad7150_of_match,
> >         },
> >         .probe = ad7150_probe,
> >         .id_table = ad7150_id,
> > --
> > 2.30.0
> >  


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

* Re: [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc
  2021-02-07 15:46 ` [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc Jonathan Cameron
  2021-02-07 16:00   ` Lars-Peter Clausen
@ 2021-02-21 15:59   ` Jonathan Cameron
  1 sibling, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-02-21 15:59 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron, devicetree

On Sun,  7 Feb 2021 15:46:20 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Binding covering the ad7150, ad7151 and ad7156 capacitance to digital
> convertors.  The only difference between these is how many channels they
> have (1 or 2)
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Robh+dt@kernel.org
> Cc: devicetree@vger.kernel.org
@Rob,

Any comments on this?  Lars requested that I use symbolic values
for the irq flags which I can do whilst applying - but otherwise
I don't plan to change anything else in here.

It's the only patch that needs tweaking and I don't really
want to repost all 24 just for that.

Thanks,

Jonathan

> ---
>  .../bindings/iio/cdc/adi,ad7150.yaml          | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/cdc/adi,ad7150.yaml b/Documentation/devicetree/bindings/iio/cdc/adi,ad7150.yaml
> new file mode 100644
> index 000000000000..2155d3f5666c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/cdc/adi,ad7150.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/cdc/adi,ad7150.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog device AD7150 and similar capacitance to digital convertors.
> +
> +maintainers:
> +  - Jonathan Cameron <jic23@kernel.org>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7150
> +      - adi,ad7151
> +      - adi,ad7156
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply: true
> +
> +  interrupts: true
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad7150
> +              - adi,ad7156
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 2
> +          maxItems: 2
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: adi,ad7151
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 1
> +          maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        cdc@48 {
> +            compatible = "adi,ad7150";
> +            reg = <0x48>;
> +            interrupts = <25 2>, <26 2>;
> +            interrupt-parent = <&gpio>;
> +        };
> +    };
> +...


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

* Re: [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming
  2021-02-07 15:46 ` [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming Jonathan Cameron
@ 2021-03-14 17:43   ` Jonathan Cameron
  2021-03-16 10:16     ` Alexandru Ardelean
  0 siblings, 1 reply; 46+ messages in thread
From: Jonathan Cameron @ 2021-03-14 17:43 UTC (permalink / raw)
  To: linux-iio
  Cc: Lars-Peter Clausen, Michael Hennerich, song.bao.hua, robh+dt,
	Jonathan Cameron

On Sun,  7 Feb 2021 15:46:12 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Add _REG postfix to register addresses to avoid confusion with
> fields.  Also add additional field defines and use throughout the
> driver in place of magic numbers.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Whilst applying this I got 
drivers/staging/iio/cdc/ad7150.c:281:24: warning: dubious: x & !y

which is related to the FIELD_PREP() 

To avoid any potential issues there, I flipped the logic to have a postive
'fixed' parameter rather than adaptive.

I'm going to assume that's trivial enough that Alex's tag stands and
apply this with that change.

Thanks,

Jonathan


> ---
>  drivers/staging/iio/cdc/ad7150.c | 123 +++++++++++++++----------------
>  1 file changed, 58 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
> index 24be97456c03..5d4783da7f98 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -21,37 +21,38 @@
>   * AD7150 registers definition
>   */
>  
> -#define AD7150_STATUS              0
> -#define AD7150_STATUS_OUT1         BIT(3)
> -#define AD7150_STATUS_OUT2         BIT(5)
> -#define AD7150_CH1_DATA_HIGH       1
> -#define AD7150_CH2_DATA_HIGH       3
> -#define AD7150_CH1_AVG_HIGH        5
> -#define AD7150_CH2_AVG_HIGH        7
> -#define AD7150_CH1_SENSITIVITY     9
> -#define AD7150_CH1_THR_HOLD_H      9
> -#define AD7150_CH1_TIMEOUT         10
> -#define AD7150_CH1_SETUP           11
> -#define AD7150_CH2_SENSITIVITY     12
> -#define AD7150_CH2_THR_HOLD_H      12
> -#define AD7150_CH2_TIMEOUT         13
> -#define AD7150_CH2_SETUP           14
> -#define AD7150_CFG                 15
> -#define AD7150_CFG_FIX             BIT(7)
> -#define AD7150_PD_TIMER            16
> -#define AD7150_CH1_CAPDAC          17
> -#define AD7150_CH2_CAPDAC          18
> -#define AD7150_SN3                 19
> -#define AD7150_SN2                 20
> -#define AD7150_SN1                 21
> -#define AD7150_SN0                 22
> -#define AD7150_ID                  23
> -
> -/* AD7150 masks */
> -#define AD7150_THRESHTYPE_MSK			GENMASK(6, 5)
> -
> -#define AD7150_CH_TIMEOUT_RECEDING		GENMASK(3, 0)
> -#define AD7150_CH_TIMEOUT_APPROACHING		GENMASK(7, 4)
> +#define AD7150_STATUS_REG		0
> +#define   AD7150_STATUS_OUT1		BIT(3)
> +#define   AD7150_STATUS_OUT2		BIT(5)
> +#define AD7150_CH1_DATA_HIGH_REG	1
> +#define AD7150_CH2_DATA_HIGH_REG	3
> +#define AD7150_CH1_AVG_HIGH_REG		5
> +#define AD7150_CH2_AVG_HIGH_REG		7
> +#define AD7150_CH1_SENSITIVITY_REG	9
> +#define AD7150_CH1_THR_HOLD_H_REG	9
> +#define AD7150_CH1_TIMEOUT_REG		10
> +#define   AD7150_CH_TIMEOUT_RECEDING	GENMASK(3, 0)
> +#define   AD7150_CH_TIMEOUT_APPROACHING	GENMASK(7, 4)
> +#define AD7150_CH1_SETUP_REG		11
> +#define AD7150_CH2_SENSITIVITY_REG	12
> +#define AD7150_CH2_THR_HOLD_H_REG	12
> +#define AD7150_CH2_TIMEOUT_REG		13
> +#define AD7150_CH2_SETUP_REG		14
> +#define AD7150_CFG_REG			15
> +#define   AD7150_CFG_FIX		BIT(7)
> +#define   AD7150_CFG_THRESHTYPE_MSK	GENMASK(6, 5)
> +#define   AD7150_CFG_TT_NEG		0x0
> +#define   AD7150_CFG_TT_POS		0x1
> +#define   AD7150_CFG_TT_IN_WINDOW	0x2
> +#define   AD7150_CFG_TT_OUT_WINDOW	0x3
> +#define AD7150_PD_TIMER_REG		16
> +#define AD7150_CH1_CAPDAC_REG		17
> +#define AD7150_CH2_CAPDAC_REG		18
> +#define AD7150_SN3_REG			19
> +#define AD7150_SN2_REG			20
> +#define AD7150_SN1_REG			21
> +#define AD7150_SN0_REG			22
> +#define AD7150_ID_REG			23
>  
>  enum {
>  	AD7150,
> @@ -93,12 +94,12 @@ struct ad7150_chip_info {
>   */
>  
>  static const u8 ad7150_addresses[][6] = {
> -	{ AD7150_CH1_DATA_HIGH, AD7150_CH1_AVG_HIGH,
> -	  AD7150_CH1_SETUP, AD7150_CH1_THR_HOLD_H,
> -	  AD7150_CH1_SENSITIVITY, AD7150_CH1_TIMEOUT },
> -	{ AD7150_CH2_DATA_HIGH, AD7150_CH2_AVG_HIGH,
> -	  AD7150_CH2_SETUP, AD7150_CH2_THR_HOLD_H,
> -	  AD7150_CH2_SENSITIVITY, AD7150_CH2_TIMEOUT },
> +	{ AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
> +	  AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
> +	  AD7150_CH1_SENSITIVITY_REG, AD7150_CH1_TIMEOUT_REG },
> +	{ AD7150_CH2_DATA_HIGH_REG, AD7150_CH2_AVG_HIGH_REG,
> +	  AD7150_CH2_SETUP_REG, AD7150_CH2_THR_HOLD_H_REG,
> +	  AD7150_CH2_SENSITIVITY_REG, AD7150_CH2_TIMEOUT_REG },
>  };
>  
>  static int ad7150_read_raw(struct iio_dev *indio_dev,
> @@ -147,11 +148,11 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
>  	bool thrfixed;
>  	struct ad7150_chip_info *chip = iio_priv(indio_dev);
>  
> -	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> +	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
>  	if (ret < 0)
>  		return ret;
>  
> -	threshtype = FIELD_GET(AD7150_THRESHTYPE_MSK, ret);
> +	threshtype = FIELD_GET(AD7150_CFG_THRESHTYPE_MSK, ret);
>  
>  	/*check if threshold mode is fixed or adaptive*/
>  	thrfixed = FIELD_GET(AD7150_CFG_FIX, ret);
> @@ -159,12 +160,12 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
>  	switch (type) {
>  	case IIO_EV_TYPE_THRESH_ADAPTIVE:
>  		if (dir == IIO_EV_DIR_RISING)
> -			return !thrfixed && (threshtype == 0x1);
> -		return !thrfixed && (threshtype == 0x0);
> +			return !thrfixed && (threshtype == AD7150_CFG_TT_POS);
> +		return !thrfixed && (threshtype == AD7150_CFG_TT_NEG);
>  	case IIO_EV_TYPE_THRESH:
>  		if (dir == IIO_EV_DIR_RISING)
> -			return thrfixed && (threshtype == 0x1);
> -		return thrfixed && (threshtype == 0x0);
> +			return thrfixed && (threshtype == AD7150_CFG_TT_POS);
> +		return thrfixed && (threshtype == AD7150_CFG_TT_NEG);
>  	default:
>  		break;
>  	}
> @@ -261,35 +262,27 @@ static int ad7150_write_event_config(struct iio_dev *indio_dev,
>  		disable_irq(chip->interrupts[0]);
>  		disable_irq(chip->interrupts[1]);
>  
> -		ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> +		ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
>  		if (ret < 0)
>  			goto error_ret;
>  
> -		cfg = ret & ~((0x03 << 5) | BIT(7));
> +		cfg = ret & ~(AD7150_CFG_THRESHTYPE_MSK | AD7150_CFG_FIX);
>  
> -		switch (type) {
> -		case IIO_EV_TYPE_THRESH_ADAPTIVE:
> +		if (type == IIO_EV_TYPE_THRESH_ADAPTIVE)
>  			adaptive = 1;
> -			if (rising)
> -				thresh_type = 0x1;
> -			else
> -				thresh_type = 0x0;
> -			break;
> -		case IIO_EV_TYPE_THRESH:
> +		else
>  			adaptive = 0;
> -			if (rising)
> -				thresh_type = 0x1;
> -			else
> -				thresh_type = 0x0;
> -			break;
> -		default:
> -			ret = -EINVAL;
> -			goto error_ret;
> -		}
>  
> -		cfg |= (!adaptive << 7) | (thresh_type << 5);
> +		if (rising)
> +			thresh_type = AD7150_CFG_TT_POS;
> +		else
> +			thresh_type = AD7150_CFG_TT_NEG;
> +
> +		cfg |= FIELD_PREP(AD7150_CFG_FIX, !adaptive) |
> +			FIELD_PREP(AD7150_CFG_THRESHTYPE_MSK, thresh_type);
>  
> -		ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG, cfg);
> +		ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG_REG,
> +						cfg);
>  		if (ret < 0)
>  			goto error_ret;
>  
> @@ -480,7 +473,7 @@ static irqreturn_t __ad7150_event_handler(void *private, u8 status_mask,
>  	s64 timestamp = iio_get_time_ns(indio_dev);
>  	int int_status;
>  
> -	int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS);
> +	int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS_REG);
>  	if (int_status < 0)
>  		return IRQ_HANDLED;
>  


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

* Re: [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable positioning.
  2021-02-08  7:54   ` Song Bao Hua (Barry Song)
@ 2021-03-14 17:46     ` Jonathan Cameron
  0 siblings, 0 replies; 46+ messages in thread
From: Jonathan Cameron @ 2021-03-14 17:46 UTC (permalink / raw)
  To: Song Bao Hua (Barry Song)
  Cc: linux-iio, Lars-Peter Clausen, Michael Hennerich, robh+dt,
	Jonathan Cameron

On Mon, 8 Feb 2021 07:54:59 +0000
"Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron [mailto:jic23@kernel.org]
> > Sent: Monday, February 8, 2021 4:46 AM
> > To: linux-iio@vger.kernel.org
> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Michael Hennerich
> > <Michael.Hennerich@analog.com>; Song Bao Hua (Barry Song)
> > <song.bao.hua@hisilicon.com>; robh+dt@kernel.org; Jonathan Cameron
> > <jonathan.cameron@huawei.com>
> > Subject: [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable
> > positioning.
> > 
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Where there is no other basis on which to order declarations
> > let us prefer reverse xmas tree.  Also reduce scope where
> > sensible.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> Reviewed-by: Barry Song <song.bao.hua@hisilicon.com>
This needed tweaking due to the earlier variable name change.

Applied with adaptive renamed to fixed and sense flipped.

Thanks,

Jonathan

> 
> > ---
> >  drivers/staging/iio/cdc/ad7150.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/cdc/ad7150.c
> > b/drivers/staging/iio/cdc/ad7150.c
> > index 9e88e774752f..d530b467d1b2 100644
> > --- a/drivers/staging/iio/cdc/ad7150.c
> > +++ b/drivers/staging/iio/cdc/ad7150.c
> > @@ -108,9 +108,9 @@ static int ad7150_read_raw(struct iio_dev *indio_dev,
> >  			   int *val2,
> >  			   long mask)
> >  {
> > -	int ret;
> >  	struct ad7150_chip_info *chip = iio_priv(indio_dev);
> >  	int channel = chan->channel;
> > +	int ret;
> > 
> >  	switch (mask) {
> >  	case IIO_CHAN_INFO_RAW:
> > @@ -143,10 +143,10 @@ static int ad7150_read_event_config(struct iio_dev
> > *indio_dev,
> >  				    enum iio_event_type type,
> >  				    enum iio_event_direction dir)
> >  {
> > -	int ret;
> > +	struct ad7150_chip_info *chip = iio_priv(indio_dev);
> >  	u8 threshtype;
> >  	bool thrfixed;
> > -	struct ad7150_chip_info *chip = iio_priv(indio_dev);
> > +	int ret;
> > 
> >  	ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
> >  	if (ret < 0)
> > @@ -227,10 +227,8 @@ static int ad7150_write_event_config(struct iio_dev
> > *indio_dev,
> >  				     enum iio_event_type type,
> >  				     enum iio_event_direction dir, int state)
> >  {
> > -	u8 thresh_type, cfg, adaptive;
> > -	int ret;
> >  	struct ad7150_chip_info *chip = iio_priv(indio_dev);
> > -	int rising = (dir == IIO_EV_DIR_RISING);
> > +	int ret;
> > 
> >  	/*
> >  	 * There is only a single shared control and no on chip
> > @@ -251,6 +249,8 @@ static int ad7150_write_event_config(struct iio_dev
> > *indio_dev,
> > 
> >  	mutex_lock(&chip->state_lock);
> >  	if ((type != chip->type) || (dir != chip->dir)) {
> > +		int rising = (dir == IIO_EV_DIR_RISING);
> > +		u8 thresh_type, cfg, adaptive;
> > 
> >  		/*
> >  		 * Need to temporarily disable both interrupts if
> > @@ -533,9 +533,9 @@ static const struct iio_info ad7150_info_no_irq = {
> >  static int ad7150_probe(struct i2c_client *client,
> >  			const struct i2c_device_id *id)
> >  {
> > -	int ret;
> >  	struct ad7150_chip_info *chip;
> >  	struct iio_dev *indio_dev;
> > +	int ret;
> > 
> >  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> >  	if (!indio_dev)
> > --
> > 2.30.0  
> 


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

* Re: [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming
  2021-03-14 17:43   ` Jonathan Cameron
@ 2021-03-16 10:16     ` Alexandru Ardelean
  0 siblings, 0 replies; 46+ messages in thread
From: Alexandru Ardelean @ 2021-03-16 10:16 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Lars-Peter Clausen, Michael Hennerich,
	Song Bao Hua (Barry Song),
	Rob Herring, Jonathan Cameron

On Sun, Mar 14, 2021 at 7:44 PM Jonathan Cameron
<jic23@jic23.retrosnub.co.uk> wrote:
>
> On Sun,  7 Feb 2021 15:46:12 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Add _REG postfix to register addresses to avoid confusion with
> > fields.  Also add additional field defines and use throughout the
> > driver in place of magic numbers.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Whilst applying this I got
> drivers/staging/iio/cdc/ad7150.c:281:24: warning: dubious: x & !y
>
> which is related to the FIELD_PREP()
>
> To avoid any potential issues there, I flipped the logic to have a postive
> 'fixed' parameter rather than adaptive.
>
> I'm going to assume that's trivial enough that Alex's tag stands and
> apply this with that change.

Yes.
It's fine from my side.

>
> Thanks,
>
> Jonathan
>
>
> > ---
> >  drivers/staging/iio/cdc/ad7150.c | 123 +++++++++++++++----------------
> >  1 file changed, 58 insertions(+), 65 deletions(-)
> >
> > diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
> > index 24be97456c03..5d4783da7f98 100644
> > --- a/drivers/staging/iio/cdc/ad7150.c
> > +++ b/drivers/staging/iio/cdc/ad7150.c
> > @@ -21,37 +21,38 @@
> >   * AD7150 registers definition
> >   */
> >
> > -#define AD7150_STATUS              0
> > -#define AD7150_STATUS_OUT1         BIT(3)
> > -#define AD7150_STATUS_OUT2         BIT(5)
> > -#define AD7150_CH1_DATA_HIGH       1
> > -#define AD7150_CH2_DATA_HIGH       3
> > -#define AD7150_CH1_AVG_HIGH        5
> > -#define AD7150_CH2_AVG_HIGH        7
> > -#define AD7150_CH1_SENSITIVITY     9
> > -#define AD7150_CH1_THR_HOLD_H      9
> > -#define AD7150_CH1_TIMEOUT         10
> > -#define AD7150_CH1_SETUP           11
> > -#define AD7150_CH2_SENSITIVITY     12
> > -#define AD7150_CH2_THR_HOLD_H      12
> > -#define AD7150_CH2_TIMEOUT         13
> > -#define AD7150_CH2_SETUP           14
> > -#define AD7150_CFG                 15
> > -#define AD7150_CFG_FIX             BIT(7)
> > -#define AD7150_PD_TIMER            16
> > -#define AD7150_CH1_CAPDAC          17
> > -#define AD7150_CH2_CAPDAC          18
> > -#define AD7150_SN3                 19
> > -#define AD7150_SN2                 20
> > -#define AD7150_SN1                 21
> > -#define AD7150_SN0                 22
> > -#define AD7150_ID                  23
> > -
> > -/* AD7150 masks */
> > -#define AD7150_THRESHTYPE_MSK                        GENMASK(6, 5)
> > -
> > -#define AD7150_CH_TIMEOUT_RECEDING           GENMASK(3, 0)
> > -#define AD7150_CH_TIMEOUT_APPROACHING                GENMASK(7, 4)
> > +#define AD7150_STATUS_REG            0
> > +#define   AD7150_STATUS_OUT1         BIT(3)
> > +#define   AD7150_STATUS_OUT2         BIT(5)
> > +#define AD7150_CH1_DATA_HIGH_REG     1
> > +#define AD7150_CH2_DATA_HIGH_REG     3
> > +#define AD7150_CH1_AVG_HIGH_REG              5
> > +#define AD7150_CH2_AVG_HIGH_REG              7
> > +#define AD7150_CH1_SENSITIVITY_REG   9
> > +#define AD7150_CH1_THR_HOLD_H_REG    9
> > +#define AD7150_CH1_TIMEOUT_REG               10
> > +#define   AD7150_CH_TIMEOUT_RECEDING GENMASK(3, 0)
> > +#define   AD7150_CH_TIMEOUT_APPROACHING      GENMASK(7, 4)
> > +#define AD7150_CH1_SETUP_REG         11
> > +#define AD7150_CH2_SENSITIVITY_REG   12
> > +#define AD7150_CH2_THR_HOLD_H_REG    12
> > +#define AD7150_CH2_TIMEOUT_REG               13
> > +#define AD7150_CH2_SETUP_REG         14
> > +#define AD7150_CFG_REG                       15
> > +#define   AD7150_CFG_FIX             BIT(7)
> > +#define   AD7150_CFG_THRESHTYPE_MSK  GENMASK(6, 5)
> > +#define   AD7150_CFG_TT_NEG          0x0
> > +#define   AD7150_CFG_TT_POS          0x1
> > +#define   AD7150_CFG_TT_IN_WINDOW    0x2
> > +#define   AD7150_CFG_TT_OUT_WINDOW   0x3
> > +#define AD7150_PD_TIMER_REG          16
> > +#define AD7150_CH1_CAPDAC_REG                17
> > +#define AD7150_CH2_CAPDAC_REG                18
> > +#define AD7150_SN3_REG                       19
> > +#define AD7150_SN2_REG                       20
> > +#define AD7150_SN1_REG                       21
> > +#define AD7150_SN0_REG                       22
> > +#define AD7150_ID_REG                        23
> >
> >  enum {
> >       AD7150,
> > @@ -93,12 +94,12 @@ struct ad7150_chip_info {
> >   */
> >
> >  static const u8 ad7150_addresses[][6] = {
> > -     { AD7150_CH1_DATA_HIGH, AD7150_CH1_AVG_HIGH,
> > -       AD7150_CH1_SETUP, AD7150_CH1_THR_HOLD_H,
> > -       AD7150_CH1_SENSITIVITY, AD7150_CH1_TIMEOUT },
> > -     { AD7150_CH2_DATA_HIGH, AD7150_CH2_AVG_HIGH,
> > -       AD7150_CH2_SETUP, AD7150_CH2_THR_HOLD_H,
> > -       AD7150_CH2_SENSITIVITY, AD7150_CH2_TIMEOUT },
> > +     { AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
> > +       AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
> > +       AD7150_CH1_SENSITIVITY_REG, AD7150_CH1_TIMEOUT_REG },
> > +     { AD7150_CH2_DATA_HIGH_REG, AD7150_CH2_AVG_HIGH_REG,
> > +       AD7150_CH2_SETUP_REG, AD7150_CH2_THR_HOLD_H_REG,
> > +       AD7150_CH2_SENSITIVITY_REG, AD7150_CH2_TIMEOUT_REG },
> >  };
> >
> >  static int ad7150_read_raw(struct iio_dev *indio_dev,
> > @@ -147,11 +148,11 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
> >       bool thrfixed;
> >       struct ad7150_chip_info *chip = iio_priv(indio_dev);
> >
> > -     ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> > +     ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
> >       if (ret < 0)
> >               return ret;
> >
> > -     threshtype = FIELD_GET(AD7150_THRESHTYPE_MSK, ret);
> > +     threshtype = FIELD_GET(AD7150_CFG_THRESHTYPE_MSK, ret);
> >
> >       /*check if threshold mode is fixed or adaptive*/
> >       thrfixed = FIELD_GET(AD7150_CFG_FIX, ret);
> > @@ -159,12 +160,12 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
> >       switch (type) {
> >       case IIO_EV_TYPE_THRESH_ADAPTIVE:
> >               if (dir == IIO_EV_DIR_RISING)
> > -                     return !thrfixed && (threshtype == 0x1);
> > -             return !thrfixed && (threshtype == 0x0);
> > +                     return !thrfixed && (threshtype == AD7150_CFG_TT_POS);
> > +             return !thrfixed && (threshtype == AD7150_CFG_TT_NEG);
> >       case IIO_EV_TYPE_THRESH:
> >               if (dir == IIO_EV_DIR_RISING)
> > -                     return thrfixed && (threshtype == 0x1);
> > -             return thrfixed && (threshtype == 0x0);
> > +                     return thrfixed && (threshtype == AD7150_CFG_TT_POS);
> > +             return thrfixed && (threshtype == AD7150_CFG_TT_NEG);
> >       default:
> >               break;
> >       }
> > @@ -261,35 +262,27 @@ static int ad7150_write_event_config(struct iio_dev *indio_dev,
> >               disable_irq(chip->interrupts[0]);
> >               disable_irq(chip->interrupts[1]);
> >
> > -             ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> > +             ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
> >               if (ret < 0)
> >                       goto error_ret;
> >
> > -             cfg = ret & ~((0x03 << 5) | BIT(7));
> > +             cfg = ret & ~(AD7150_CFG_THRESHTYPE_MSK | AD7150_CFG_FIX);
> >
> > -             switch (type) {
> > -             case IIO_EV_TYPE_THRESH_ADAPTIVE:
> > +             if (type == IIO_EV_TYPE_THRESH_ADAPTIVE)
> >                       adaptive = 1;
> > -                     if (rising)
> > -                             thresh_type = 0x1;
> > -                     else
> > -                             thresh_type = 0x0;
> > -                     break;
> > -             case IIO_EV_TYPE_THRESH:
> > +             else
> >                       adaptive = 0;
> > -                     if (rising)
> > -                             thresh_type = 0x1;
> > -                     else
> > -                             thresh_type = 0x0;
> > -                     break;
> > -             default:
> > -                     ret = -EINVAL;
> > -                     goto error_ret;
> > -             }
> >
> > -             cfg |= (!adaptive << 7) | (thresh_type << 5);
> > +             if (rising)
> > +                     thresh_type = AD7150_CFG_TT_POS;
> > +             else
> > +                     thresh_type = AD7150_CFG_TT_NEG;
> > +
> > +             cfg |= FIELD_PREP(AD7150_CFG_FIX, !adaptive) |
> > +                     FIELD_PREP(AD7150_CFG_THRESHTYPE_MSK, thresh_type);
> >
> > -             ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG, cfg);
> > +             ret = i2c_smbus_write_byte_data(chip->client, AD7150_CFG_REG,
> > +                                             cfg);
> >               if (ret < 0)
> >                       goto error_ret;
> >
> > @@ -480,7 +473,7 @@ static irqreturn_t __ad7150_event_handler(void *private, u8 status_mask,
> >       s64 timestamp = iio_get_time_ns(indio_dev);
> >       int int_status;
> >
> > -     int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS);
> > +     int_status = i2c_smbus_read_byte_data(chip->client, AD7150_STATUS_REG);
> >       if (int_status < 0)
> >               return IRQ_HANDLED;
> >
>

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

end of thread, other threads:[~2021-03-16 10:16 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-07 15:45 [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
2021-02-07 15:46 ` [PATCH 01/24] staging:iio:cdc:ad7150: use swapped reads for i2c rather than open coding Jonathan Cameron
2021-02-07 15:46 ` [PATCH 02/24] staging:iio:cdc:ad7150: Remove magnitude adaptive events Jonathan Cameron
2021-02-07 15:46 ` [PATCH 03/24] staging:iio:cdc:ad7150: Refactor event parameter update Jonathan Cameron
2021-02-07 15:46 ` [PATCH 04/24] staging:iio:cdc:ad7150: Timeout register covers both directions so both need updating Jonathan Cameron
2021-02-07 15:46 ` [PATCH 05/24] staging:iio:cdc:ad7150: Drop platform data support Jonathan Cameron
2021-02-08  8:01   ` Song Bao Hua (Barry Song)
2021-02-08 11:12     ` Jonathan Cameron
2021-02-07 15:46 ` [PATCH 06/24] staging:iio:cdc:ad7150: Handle variation in chan_spec across device and irq present or not Jonathan Cameron
2021-02-07 15:46 ` [PATCH 07/24] staging:iio:cdc:ad7150: Simplify event handling by only using rising direction Jonathan Cameron
2021-02-07 15:46 ` [PATCH 08/24] staging:iio:cdc:ad7150: Drop noisy print in probe Jonathan Cameron
2021-02-08  8:02   ` Song Bao Hua (Barry Song)
2021-02-07 15:46 ` [PATCH 09/24] staging:iio:cdc:ad7150: Add sampling_frequency support Jonathan Cameron
2021-02-07 15:46 ` [PATCH 10/24] iio:event: Add timeout event info type Jonathan Cameron
2021-02-07 15:46 ` [PATCH 11/24] staging:iio:cdc:ad7150: Change timeout units to seconds and use core support Jonathan Cameron
2021-02-07 15:46 ` [PATCH 12/24] staging:iio:cdc:ad7150: Rework interrupt handling Jonathan Cameron
2021-02-07 15:46 ` [PATCH 13/24] staging:iio:cdc:ad7150: More consistent register and field naming Jonathan Cameron
2021-03-14 17:43   ` Jonathan Cameron
2021-03-16 10:16     ` Alexandru Ardelean
2021-02-07 15:46 ` [PATCH 14/24] staging:iio:cdc:ad7150: Reorganize headers Jonathan Cameron
2021-02-08  7:54   ` Song Bao Hua (Barry Song)
2021-02-07 15:46 ` [PATCH 15/24] staging:iio:cdc:ad7150: Tidy up local variable positioning Jonathan Cameron
2021-02-08  7:54   ` Song Bao Hua (Barry Song)
2021-03-14 17:46     ` Jonathan Cameron
2021-02-07 15:46 ` [PATCH 16/24] staging:iio:cdc:ad7150: Drop unnecessary block comments Jonathan Cameron
2021-02-08  7:52   ` Song Bao Hua (Barry Song)
2021-02-07 15:46 ` [PATCH 17/24] staging:iio:cdc:ad7150: Shift the _raw readings by 4 bits Jonathan Cameron
2021-02-07 15:46 ` [PATCH 18/24] staging:iio:cdc:ad7150: Add scale and offset to info_mask_shared_by_type Jonathan Cameron
2021-02-07 15:46 ` [PATCH 19/24] staging:iio:cdc:ad7150: Really basic regulator support Jonathan Cameron
2021-02-07 15:46 ` [PATCH 20/24] staging:iio:cdc:ad7150: Add of_match_table Jonathan Cameron
2021-02-08  7:11   ` Alexandru Ardelean
2021-02-11 15:51     ` Jonathan Cameron
2021-02-08  7:50   ` Song Bao Hua (Barry Song)
2021-02-08  8:01     ` Alexandru Ardelean
2021-02-07 15:46 ` [PATCH 21/24] dt-bindings:iio:cdc:adi,ad7150 binding doc Jonathan Cameron
2021-02-07 16:00   ` Lars-Peter Clausen
2021-02-07 16:18     ` Jonathan Cameron
2021-02-08  8:12     ` Song Bao Hua (Barry Song)
2021-02-08 11:20       ` Jonathan Cameron
2021-02-21 15:59   ` Jonathan Cameron
2021-02-07 15:46 ` [PATCH 22/24] iio:Documentation:ABI Add missing elements as used by the adi,ad7150 Jonathan Cameron
2021-02-07 15:46 ` [PATCH 23/24] staging:iio:cdc:ad7150: Add copyright notice given substantial changes Jonathan Cameron
2021-02-07 15:46 ` [PATCH 24/24] iio:cdc:ad7150: Move driver out of staging Jonathan Cameron
2021-02-07 16:21   ` Jonathan Cameron
2021-02-07 16:12 ` [PATCH 00/24] staging:iio:cdc:ad7150: cleanup / fixup / graduate Jonathan Cameron
2021-02-08  7:20   ` Alexandru Ardelean

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.