[PATCH 00/15] ad799x cleanup

[PATCH 00/15] ad799x cleanup

[Peter Meerwald]

Hello,

I ran into some issues with IIO events using the ad799x ADC driver with an
ad7997 chip; the following series has cleanups and fixes

patches 1-6, 8 are cleanup

patch 7 exposes the IIO event interface of the driver only if an IRQ has been configured
for the device; no IRQ -> no events

patch 9 shifts out the last two bits when reading the event value on ad7993 and ad7997
(which have 10-bit ADCs); same as read_raw()

patch 10 checks the range of the event value on write and shifts the value into place;
similar to the previous patch

patch 11 adds helper function to read/write the chip's config register (16-bit on 
ad7997/ad7998, 8-bit everywhere else)

patch 12 actually writes the default config to the chip and keeps a copy of
the config register in the driver's state

patch 13 changes update_scan_mode() to store the enabled channels in the config
register, on ad7992/ad7993/ad7994/ad7997/ad7998, hence the rename

patch 14 reports only those events as enabled which actually are

patch 15 allows to write the event config, previously events could only be
set implicitly via the buffer scan mode

regards, p.

Peter Meerwald (15):
  staging:iio: Update iio_event_monitor program
  staging:iio: Fix iio_utils.h function prototypes
  iio:adc:ad799x: Fix ad799x_chip_info kerneldoc
  iio:adc:ad799x: Drop I2C access helper functions
  iio:adc:ad799x: Save some lines in ad7997_8_update_scan_mode() exit
    handling
  iio:adc:ad799x: Use BIT() and GENMASK()
  iio:adc:ad799x: Only expose event interface when IRQ is available
  iio:adc:ad799x: Make chan_spec const in ad799x_chip_config struct
  iio:adc:ad799x: Fix reported event values, apply shift
  iio:adc:ad799x: Check event value range on write
  iio:adc:ad799x: Add helper function to read/write config register
  iio:adc:ad799x: Write default config on probe and reset alert status
    on probe
  iio:adc:ad799x: Rename ad7997_8_update_scan_mode() to
    ad799x_update_scan_mode()
  iio:adc:ad799x: Return more meaningful event enabled state
  iio:adc:ad799x: Allow to write event config

 drivers/iio/adc/ad799x.c                           | 507 ++++++++++++---------
 .../staging/iio/Documentation/iio_event_monitor.c  |  10 +
 drivers/staging/iio/Documentation/iio_utils.h      |   6 +-
 3 files changed, 317 insertions(+), 206 deletions(-)

-- 
1.9.1

[PATCH 01/15] staging:iio: Update iio_event_monitor program

[Peter Meerwald]

add types recently added

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/Documentation/iio_event_monitor.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/iio/Documentation/iio_event_monitor.c b/drivers/staging/iio/Documentation/iio_event_monitor.c
index 3a9b000..cb35a97 100644
--- a/drivers/staging/iio/Documentation/iio_event_monitor.c
+++ b/drivers/staging/iio/Documentation/iio_event_monitor.c
@@ -46,6 +46,9 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_TIMESTAMP] = "timestamp",
 	[IIO_CAPACITANCE] = "capacitance",
 	[IIO_ALTVOLTAGE] = "altvoltage",
+	[IIO_CCT] = "cct",
+	[IIO_PRESSURE] = "pressure",
+	[IIO_HUMIDITYRELATIVE] = "humidityrelative",
 };
 
 static const char * const iio_ev_type_text[] = {
@@ -70,6 +73,8 @@ static const char * const iio_modifier_names[] = {
 	[IIO_MOD_LIGHT_IR] = "ir",
 	[IIO_MOD_ROOT_SUM_SQUARED_X_Y] = "sqrt(x^2+y^2)",
 	[IIO_MOD_SUM_SQUARED_X_Y_Z] = "x^2+y^2+z^2",
+	[IIO_MOD_LIGHT_BOTH] = "both",
+	[IIO_MOD_LIGHT_IR] = "ir",
 	[IIO_MOD_LIGHT_CLEAR] = "clear",
 	[IIO_MOD_LIGHT_RED] = "red",
 	[IIO_MOD_LIGHT_GREEN] = "green",
@@ -100,6 +105,9 @@ static bool event_is_known(struct iio_event_data *event)
 	case IIO_TIMESTAMP:
 	case IIO_CAPACITANCE:
 	case IIO_ALTVOLTAGE:
+	case IIO_CCT:
+	case IIO_PRESSURE:
+	case IIO_HUMIDITYRELATIVE:
 		break;
 	default:
 		return false;
@@ -114,6 +122,8 @@ static bool event_is_known(struct iio_event_data *event)
 	case IIO_MOD_LIGHT_IR:
 	case IIO_MOD_ROOT_SUM_SQUARED_X_Y:
 	case IIO_MOD_SUM_SQUARED_X_Y_Z:
+	case IIO_MOD_LIGHT_BOTH:
+	case IIO_MOD_LIGHT_IR:
 	case IIO_MOD_LIGHT_CLEAR:
 	case IIO_MOD_LIGHT_RED:
 	case IIO_MOD_LIGHT_GREEN:
-- 
1.9.1

[PATCH 02/15] staging:iio: Fix iio_utils.h function prototypes

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/staging/iio/Documentation/iio_utils.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/Documentation/iio_utils.h b/drivers/staging/iio/Documentation/iio_utils.h
index a9cfc06..0973a09 100644
--- a/drivers/staging/iio/Documentation/iio_utils.h
+++ b/drivers/staging/iio/Documentation/iio_utils.h
@@ -633,7 +633,7 @@ error_free:
 
 int read_sysfs_float(char *filename, char *basedir, float *val)
 {
-	float ret = 0;
+	int ret = 0;
 	FILE  *sysfsfp;
 	char *temp = malloc(strlen(basedir) + strlen(filename) + 2);
 	if (temp == NULL) {
@@ -653,9 +653,9 @@ error_free:
 	return ret;
 }
 
-read_sysfs_string(const char *filename, const char *basedir, char *str)
+int read_sysfs_string(const char *filename, const char *basedir, char *str)
 {
-	float ret = 0;
+	int ret = 0;
 	FILE  *sysfsfp;
 	char *temp = malloc(strlen(basedir) + strlen(filename) + 2);
 	if (temp == NULL) {
-- 
1.9.1

[PATCH 03/15] iio:adc:ad799x: Fix ad799x_chip_info kerneldoc

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 39b4cb4..7e08c60 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -105,9 +105,8 @@ enum {
  * struct ad799x_chip_info - chip specific information
  * @channel:		channel specification
  * @num_channels:	number of channels
- * @monitor_mode:	whether the chip supports monitor interrupts
  * @default_config:	device default configuration
- * @event_attrs:	pointer to the monitor event attribute group
+ * @info:		pointer to iio_info struct
  */
 struct ad799x_chip_info {
 	struct iio_chan_spec		channel[9];
-- 
1.9.1

[PATCH 04/15] iio:adc:ad799x: Drop I2C access helper functions

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 120 ++++++++++-------------------------------------
 1 file changed, 26 insertions(+), 94 deletions(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 7e08c60..c03406e 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -175,65 +175,6 @@ out:
 	return IRQ_HANDLED;
 }
 
-/*
- * ad799x register access by I2C
- */
-static int ad799x_i2c_read16(struct ad799x_state *st, u8 reg, u16 *data)
-{
-	struct i2c_client *client = st->client;
-	int ret = 0;
-
-	ret = i2c_smbus_read_word_swapped(client, reg);
-	if (ret < 0) {
-		dev_err(&client->dev, "I2C read error\n");
-		return ret;
-	}
-
-	*data = (u16)ret;
-
-	return 0;
-}
-
-static int ad799x_i2c_read8(struct ad799x_state *st, u8 reg, u8 *data)
-{
-	struct i2c_client *client = st->client;
-	int ret = 0;
-
-	ret = i2c_smbus_read_byte_data(client, reg);
-	if (ret < 0) {
-		dev_err(&client->dev, "I2C read error\n");
-		return ret;
-	}
-
-	*data = (u8)ret;
-
-	return 0;
-}
-
-static int ad799x_i2c_write16(struct ad799x_state *st, u8 reg, u16 data)
-{
-	struct i2c_client *client = st->client;
-	int ret = 0;
-
-	ret = i2c_smbus_write_word_swapped(client, reg, data);
-	if (ret < 0)
-		dev_err(&client->dev, "I2C write error\n");
-
-	return ret;
-}
-
-static int ad799x_i2c_write8(struct ad799x_state *st, u8 reg, u8 data)
-{
-	struct i2c_client *client = st->client;
-	int ret = 0;
-
-	ret = i2c_smbus_write_byte_data(client, reg, data);
-	if (ret < 0)
-		dev_err(&client->dev, "I2C write error\n");
-
-	return ret;
-}
-
 static int ad7997_8_update_scan_mode(struct iio_dev *indio_dev,
 	const unsigned long *scan_mask)
 {
@@ -249,7 +190,7 @@ static int ad7997_8_update_scan_mode(struct iio_dev *indio_dev,
 	switch (st->id) {
 	case ad7997:
 	case ad7998:
-		return ad799x_i2c_write16(st, AD7998_CONF_REG,
+		return i2c_smbus_write_word_swapped(st->client, AD7998_CONF_REG,
 			st->config | (*scan_mask << AD799X_CHANNEL_SHIFT));
 	default:
 		break;
@@ -260,9 +201,7 @@ static int ad7997_8_update_scan_mode(struct iio_dev *indio_dev,
 
 static int ad799x_scan_direct(struct ad799x_state *st, unsigned ch)
 {
-	u16 rxbuf;
 	u8 cmd;
-	int ret;
 
 	switch (st->id) {
 	case ad7991:
@@ -283,11 +222,7 @@ static int ad799x_scan_direct(struct ad799x_state *st, unsigned ch)
 		return -EINVAL;
 	}
 
-	ret = ad799x_i2c_read16(st, cmd, &rxbuf);
-	if (ret < 0)
-		return ret;
-
-	return rxbuf;
+	return i2c_smbus_read_word_swapped(st->client, cmd);
 }
 
 static int ad799x_read_raw(struct iio_dev *indio_dev,
@@ -332,6 +267,7 @@ static const unsigned int ad7998_frequencies[] = {
 	[AD7998_CYC_TCONF_1024]	= 488,
 	[AD7998_CYC_TCONF_2048]	= 244,
 };
+
 static ssize_t ad799x_read_frequency(struct device *dev,
 					struct device_attribute *attr,
 					char *buf)
@@ -339,15 +275,11 @@ static ssize_t ad799x_read_frequency(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad799x_state *st = iio_priv(indio_dev);
 
-	int ret;
-	u8 val;
-	ret = ad799x_i2c_read8(st, AD7998_CYCLE_TMR_REG, &val);
-	if (ret)
+	int ret = i2c_smbus_read_byte_data(st->client, AD7998_CYCLE_TMR_REG);
+	if (ret < 0)
 		return ret;
 
-	val &= AD7998_CYC_MASK;
-
-	return sprintf(buf, "%u\n", ad7998_frequencies[val]);
+	return sprintf(buf, "%u\n", ad7998_frequencies[ret & AD7998_CYC_MASK]);
 }
 
 static ssize_t ad799x_write_frequency(struct device *dev,
@@ -360,18 +292,17 @@ static ssize_t ad799x_write_frequency(struct device *dev,
 
 	long val;
 	int ret, i;
-	u8 t;
 
 	ret = kstrtol(buf, 10, &val);
 	if (ret)
 		return ret;
 
 	mutex_lock(&indio_dev->mlock);
-	ret = ad799x_i2c_read8(st, AD7998_CYCLE_TMR_REG, &t);
-	if (ret)
+	ret = i2c_smbus_read_byte_data(st->client, AD7998_CYCLE_TMR_REG);
+	if (ret < 0)
 		goto error_ret_mutex;
 	/* Wipe the bits clean */
-	t &= ~AD7998_CYC_MASK;
+	ret &= ~AD7998_CYC_MASK;
 
 	for (i = 0; i < ARRAY_SIZE(ad7998_frequencies); i++)
 		if (val == ad7998_frequencies[i])
@@ -380,13 +311,17 @@ static ssize_t ad799x_write_frequency(struct device *dev,
 		ret = -EINVAL;
 		goto error_ret_mutex;
 	}
-	t |= i;
-	ret = ad799x_i2c_write8(st, AD7998_CYCLE_TMR_REG, t);
+
+	ret = i2c_smbus_write_byte_data(st->client, AD7998_CYCLE_TMR_REG,
+		ret | i);
+	if (ret < 0)
+		goto error_ret_mutex;
+	ret = len;
 
 error_ret_mutex:
 	mutex_unlock(&indio_dev->mlock);
 
-	return ret ? ret : len;
+	return ret;
 }
 
 static int ad799x_read_event_config(struct iio_dev *indio_dev,
@@ -427,8 +362,8 @@ static int ad799x_write_event_value(struct iio_dev *indio_dev,
 	struct ad799x_state *st = iio_priv(indio_dev);
 
 	mutex_lock(&indio_dev->mlock);
-	ret = ad799x_i2c_write16(st, ad799x_threshold_reg(chan, dir, info),
-		val);
+	ret = i2c_smbus_write_word_swapped(st->client,
+		ad799x_threshold_reg(chan, dir, info), val);
 	mutex_unlock(&indio_dev->mlock);
 
 	return ret;
@@ -443,15 +378,14 @@ static int ad799x_read_event_value(struct iio_dev *indio_dev,
 {
 	int ret;
 	struct ad799x_state *st = iio_priv(indio_dev);
-	u16 valin;
 
 	mutex_lock(&indio_dev->mlock);
-	ret = ad799x_i2c_read16(st, ad799x_threshold_reg(chan, dir, info),
-		&valin);
+	ret = i2c_smbus_read_word_swapped(st->client,
+		ad799x_threshold_reg(chan, dir, info));
 	mutex_unlock(&indio_dev->mlock);
 	if (ret < 0)
 		return ret;
-	*val = valin;
+	*val = ret;
 
 	return IIO_VAL_INT;
 }
@@ -460,20 +394,18 @@ static irqreturn_t ad799x_event_handler(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
 	struct ad799x_state *st = iio_priv(private);
-	u8 status;
 	int i, ret;
 
-	ret = ad799x_i2c_read8(st, AD7998_ALERT_STAT_REG, &status);
-	if (ret)
+	ret = i2c_smbus_read_byte_data(st->client, AD7998_ALERT_STAT_REG);
+	if (ret <= 0)
 		goto done;
 
-	if (!status)
+	if (i2c_smbus_write_byte_data(st->client, AD7998_ALERT_STAT_REG,
+		AD7998_ALERT_STAT_CLEAR) < 0)
 		goto done;
 
-	ad799x_i2c_write8(st, AD7998_ALERT_STAT_REG, AD7998_ALERT_STAT_CLEAR);
-
 	for (i = 0; i < 8; i++) {
-		if (status & (1 << i))
+		if (ret & (1 << i))
 			iio_push_event(indio_dev,
 				       i & 0x1 ?
 				       IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE,
-- 
1.9.1

[PATCH 05/15] iio:adc:ad799x: Save some lines in ad7997_8_update_scan_mode() exit handling

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index c03406e..24eca2f 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -193,10 +193,8 @@ static int ad7997_8_update_scan_mode(struct iio_dev *indio_dev,
 		return i2c_smbus_write_word_swapped(st->client, AD7998_CONF_REG,
 			st->config | (*scan_mask << AD799X_CHANNEL_SHIFT));
 	default:
-		break;
+		return 0;
 	}
-
-	return 0;
 }
 
 static int ad799x_scan_direct(struct ad799x_state *st, unsigned ch)
-- 
1.9.1

[PATCH 06/15] iio:adc:ad799x: Use BIT() and GENMASK()

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 24eca2f..572c32a 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -32,6 +32,7 @@
 #include <linux/types.h>
 #include <linux/err.h>
 #include <linux/module.h>
+#include <linux/bitops.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -41,7 +42,7 @@
 #include <linux/iio/triggered_buffer.h>
 
 #define AD799X_CHANNEL_SHIFT			4
-#define AD799X_STORAGEBITS			16
+
 /*
  * AD7991, AD7995 and AD7999 defines
  */
@@ -55,10 +56,10 @@
  * AD7992, AD7993, AD7994, AD7997 and AD7998 defines
  */
 
-#define AD7998_FLTR				0x08
-#define AD7998_ALERT_EN				0x04
-#define AD7998_BUSY_ALERT			0x02
-#define AD7998_BUSY_ALERT_POL			0x01
+#define AD7998_FLTR				BIT(3)
+#define AD7998_ALERT_EN				BIT(2)
+#define AD7998_BUSY_ALERT			BIT(1)
+#define AD7998_BUSY_ALERT_POL			BIT(0)
 
 #define AD7998_CONV_RES_REG			0x0
 #define AD7998_ALERT_STAT_REG			0x1
@@ -69,7 +70,7 @@
 #define AD7998_DATAHIGH_REG(x)			((x) * 3 + 0x5)
 #define AD7998_HYST_REG(x)			((x) * 3 + 0x6)
 
-#define AD7998_CYC_MASK				0x7
+#define AD7998_CYC_MASK				GENMASK(2, 0)
 #define AD7998_CYC_DIS				0x0
 #define AD7998_CYC_TCONF_32			0x1
 #define AD7998_CYC_TCONF_64			0x2
@@ -85,10 +86,8 @@
  * AD7997 and AD7997 defines
  */
 
-#define AD7997_8_READ_SINGLE			0x80
-#define AD7997_8_READ_SEQUENCE			0x70
-/* TODO: move this into a common header */
-#define RES_MASK(bits)	((1 << (bits)) - 1)
+#define AD7997_8_READ_SINGLE			BIT(7)
+#define AD7997_8_READ_SEQUENCE			(BIT(6) | BIT(5) | BIT(4))
 
 enum {
 	ad7991,
@@ -205,12 +204,12 @@ static int ad799x_scan_direct(struct ad799x_state *st, unsigned ch)
 	case ad7991:
 	case ad7995:
 	case ad7999:
-		cmd = st->config | ((1 << ch) << AD799X_CHANNEL_SHIFT);
+		cmd = st->config | (BIT(ch) << AD799X_CHANNEL_SHIFT);
 		break;
 	case ad7992:
 	case ad7993:
 	case ad7994:
-		cmd = (1 << ch) << AD799X_CHANNEL_SHIFT;
+		cmd = BIT(ch) << AD799X_CHANNEL_SHIFT;
 		break;
 	case ad7997:
 	case ad7998:
@@ -244,7 +243,7 @@ static int ad799x_read_raw(struct iio_dev *indio_dev,
 		if (ret < 0)
 			return ret;
 		*val = (ret >> chan->scan_type.shift) &
-			RES_MASK(chan->scan_type.realbits);
+			GENMASK(chan->scan_type.realbits - 1, 0);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		ret = regulator_get_voltage(st->vref);
@@ -403,7 +402,7 @@ static irqreturn_t ad799x_event_handler(int irq, void *private)
 		goto done;
 
 	for (i = 0; i < 8; i++) {
-		if (ret & (1 << i))
+		if (ret & BIT(i))
 			iio_push_event(indio_dev,
 				       i & 0x1 ?
 				       IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE,
-- 
1.9.1

[PATCH 07/15] iio:adc:ad799x: Only expose event interface when IRQ is available

[Peter Meerwald]

an IRQ is necessary to handle the ALERT condition; without
IRQ, the IIO event interface serves no purpose

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 263 +++++++++++++++++++++++++++++++----------------
 1 file changed, 177 insertions(+), 86 deletions(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 572c32a..eb49ac1 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -101,22 +101,32 @@ enum {
 };
 
 /**
- * struct ad799x_chip_info - chip specific information
+ * struct ad799x_chip_config - chip specific information
  * @channel:		channel specification
- * @num_channels:	number of channels
  * @default_config:	device default configuration
  * @info:		pointer to iio_info struct
  */
-struct ad799x_chip_info {
+struct ad799x_chip_config {
 	struct iio_chan_spec		channel[9];
-	int				num_channels;
 	u16				default_config;
 	const struct iio_info		*info;
 };
 
+/**
+ * struct ad799x_chip_info - chip specific information
+ * @num_channels:	number of channels
+ * @noirq_config:	device configuration w/o IRQ
+ * @irq_config:		device configuration w/IRQ
+ */
+struct ad799x_chip_info {
+	int				num_channels;
+	const struct ad799x_chip_config	noirq_config;
+	const struct ad799x_chip_config	irq_config;
+};
+
 struct ad799x_state {
 	struct i2c_client		*client;
-	const struct ad799x_chip_info	*chip_info;
+	const struct ad799x_chip_config	*chip_config;
 	struct regulator		*reg;
 	struct regulator		*vref;
 	unsigned			id;
@@ -441,7 +451,13 @@ static const struct iio_info ad7991_info = {
 	.driver_module = THIS_MODULE,
 };
 
-static const struct iio_info ad7993_4_7_8_info = {
+static const struct iio_info ad7993_4_7_8_noirq_info = {
+	.read_raw = &ad799x_read_raw,
+	.driver_module = THIS_MODULE,
+	.update_scan_mode = ad7997_8_update_scan_mode,
+};
+
+static const struct iio_info ad7993_4_7_8_irq_info = {
 	.read_raw = &ad799x_read_raw,
 	.event_attrs = &ad799x_event_attrs_group,
 	.read_event_config = &ad799x_read_event_config,
@@ -496,103 +512,175 @@ static const struct iio_event_spec ad799x_events[] = {
 
 static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
 	[ad7991] = {
-		.channel = {
-			AD799X_CHANNEL(0, 12),
-			AD799X_CHANNEL(1, 12),
-			AD799X_CHANNEL(2, 12),
-			AD799X_CHANNEL(3, 12),
-			IIO_CHAN_SOFT_TIMESTAMP(4),
-		},
 		.num_channels = 5,
-		.info = &ad7991_info,
+		.noirq_config = {
+			.channel = {
+				AD799X_CHANNEL(0, 12),
+				AD799X_CHANNEL(1, 12),
+				AD799X_CHANNEL(2, 12),
+				AD799X_CHANNEL(3, 12),
+				IIO_CHAN_SOFT_TIMESTAMP(4),
+			},
+			.info = &ad7991_info,
+		},
 	},
 	[ad7995] = {
-		.channel = {
-			AD799X_CHANNEL(0, 10),
-			AD799X_CHANNEL(1, 10),
-			AD799X_CHANNEL(2, 10),
-			AD799X_CHANNEL(3, 10),
-			IIO_CHAN_SOFT_TIMESTAMP(4),
-		},
 		.num_channels = 5,
-		.info = &ad7991_info,
+		.noirq_config = {
+			.channel = {
+				AD799X_CHANNEL(0, 10),
+				AD799X_CHANNEL(1, 10),
+				AD799X_CHANNEL(2, 10),
+				AD799X_CHANNEL(3, 10),
+				IIO_CHAN_SOFT_TIMESTAMP(4),
+			},
+			.info = &ad7991_info,
+		},
 	},
 	[ad7999] = {
-		.channel = {
-			AD799X_CHANNEL(0, 8),
-			AD799X_CHANNEL(1, 8),
-			AD799X_CHANNEL(2, 8),
-			AD799X_CHANNEL(3, 8),
-			IIO_CHAN_SOFT_TIMESTAMP(4),
-		},
 		.num_channels = 5,
-		.info = &ad7991_info,
+		.noirq_config = {
+			.channel = {
+				AD799X_CHANNEL(0, 8),
+				AD799X_CHANNEL(1, 8),
+				AD799X_CHANNEL(2, 8),
+				AD799X_CHANNEL(3, 8),
+				IIO_CHAN_SOFT_TIMESTAMP(4),
+			},
+			.info = &ad7991_info,
+		},
 	},
 	[ad7992] = {
-		.channel = {
-			AD799X_CHANNEL_WITH_EVENTS(0, 12),
-			AD799X_CHANNEL_WITH_EVENTS(1, 12),
-			IIO_CHAN_SOFT_TIMESTAMP(3),
-		},
 		.num_channels = 3,
-		.default_config = AD7998_ALERT_EN,
-		.info = &ad7993_4_7_8_info,
+		.noirq_config = {
+			.channel = {
+				AD799X_CHANNEL(0, 12),
+				AD799X_CHANNEL(1, 12),
+				IIO_CHAN_SOFT_TIMESTAMP(3),
+			},
+			.info = &ad7993_4_7_8_noirq_info,
+		},
+		.irq_config = {
+			.channel = {
+				AD799X_CHANNEL_WITH_EVENTS(0, 12),
+				AD799X_CHANNEL_WITH_EVENTS(1, 12),
+				IIO_CHAN_SOFT_TIMESTAMP(3),
+			},
+			.default_config = AD7998_ALERT_EN,
+			.info = &ad7993_4_7_8_irq_info,
+		},
 	},
 	[ad7993] = {
-		.channel = {
-			AD799X_CHANNEL_WITH_EVENTS(0, 10),
-			AD799X_CHANNEL_WITH_EVENTS(1, 10),
-			AD799X_CHANNEL_WITH_EVENTS(2, 10),
-			AD799X_CHANNEL_WITH_EVENTS(3, 10),
-			IIO_CHAN_SOFT_TIMESTAMP(4),
-		},
 		.num_channels = 5,
-		.default_config = AD7998_ALERT_EN,
-		.info = &ad7993_4_7_8_info,
+		.noirq_config = {
+			.channel = {
+				AD799X_CHANNEL(0, 10),
+				AD799X_CHANNEL(1, 10),
+				AD799X_CHANNEL(2, 10),
+				AD799X_CHANNEL(3, 10),
+				IIO_CHAN_SOFT_TIMESTAMP(4),
+			},
+			.info = &ad7993_4_7_8_noirq_info,
+		},
+		.irq_config = {
+			.channel = {
+				AD799X_CHANNEL_WITH_EVENTS(0, 10),
+				AD799X_CHANNEL_WITH_EVENTS(1, 10),
+				AD799X_CHANNEL_WITH_EVENTS(2, 10),
+				AD799X_CHANNEL_WITH_EVENTS(3, 10),
+				IIO_CHAN_SOFT_TIMESTAMP(4),
+			},
+			.default_config = AD7998_ALERT_EN,
+			.info = &ad7993_4_7_8_irq_info,
+		},
 	},
 	[ad7994] = {
-		.channel = {
-			AD799X_CHANNEL_WITH_EVENTS(0, 12),
-			AD799X_CHANNEL_WITH_EVENTS(1, 12),
-			AD799X_CHANNEL_WITH_EVENTS(2, 12),
-			AD799X_CHANNEL_WITH_EVENTS(3, 12),
-			IIO_CHAN_SOFT_TIMESTAMP(4),
-		},
 		.num_channels = 5,
-		.default_config = AD7998_ALERT_EN,
-		.info = &ad7993_4_7_8_info,
+		.noirq_config = {
+			.channel = {
+				AD799X_CHANNEL(0, 12),
+				AD799X_CHANNEL(1, 12),
+				AD799X_CHANNEL(2, 12),
+				AD799X_CHANNEL(3, 12),
+				IIO_CHAN_SOFT_TIMESTAMP(4),
+			},
+			.info = &ad7993_4_7_8_noirq_info,
+		},
+		.irq_config = {
+			.channel = {
+				AD799X_CHANNEL_WITH_EVENTS(0, 12),
+				AD799X_CHANNEL_WITH_EVENTS(1, 12),
+				AD799X_CHANNEL_WITH_EVENTS(2, 12),
+				AD799X_CHANNEL_WITH_EVENTS(3, 12),
+				IIO_CHAN_SOFT_TIMESTAMP(4),
+			},
+			.default_config = AD7998_ALERT_EN,
+			.info = &ad7993_4_7_8_irq_info,
+		},
 	},
 	[ad7997] = {
-		.channel = {
-			AD799X_CHANNEL_WITH_EVENTS(0, 10),
-			AD799X_CHANNEL_WITH_EVENTS(1, 10),
-			AD799X_CHANNEL_WITH_EVENTS(2, 10),
-			AD799X_CHANNEL_WITH_EVENTS(3, 10),
-			AD799X_CHANNEL(4, 10),
-			AD799X_CHANNEL(5, 10),
-			AD799X_CHANNEL(6, 10),
-			AD799X_CHANNEL(7, 10),
-			IIO_CHAN_SOFT_TIMESTAMP(8),
-		},
 		.num_channels = 9,
-		.default_config = AD7998_ALERT_EN,
-		.info = &ad7993_4_7_8_info,
+		.noirq_config = {
+			.channel = {
+				AD799X_CHANNEL(0, 10),
+				AD799X_CHANNEL(1, 10),
+				AD799X_CHANNEL(2, 10),
+				AD799X_CHANNEL(3, 10),
+				AD799X_CHANNEL(4, 10),
+				AD799X_CHANNEL(5, 10),
+				AD799X_CHANNEL(6, 10),
+				AD799X_CHANNEL(7, 10),
+				IIO_CHAN_SOFT_TIMESTAMP(8),
+			},
+			.info = &ad7993_4_7_8_noirq_info,
+		},
+		.irq_config = {
+			.channel = {
+				AD799X_CHANNEL_WITH_EVENTS(0, 10),
+				AD799X_CHANNEL_WITH_EVENTS(1, 10),
+				AD799X_CHANNEL_WITH_EVENTS(2, 10),
+				AD799X_CHANNEL_WITH_EVENTS(3, 10),
+				AD799X_CHANNEL(4, 10),
+				AD799X_CHANNEL(5, 10),
+				AD799X_CHANNEL(6, 10),
+				AD799X_CHANNEL(7, 10),
+				IIO_CHAN_SOFT_TIMESTAMP(8),
+			},
+			.default_config = AD7998_ALERT_EN,
+			.info = &ad7993_4_7_8_irq_info,
+		},
 	},
 	[ad7998] = {
-		.channel = {
-			AD799X_CHANNEL_WITH_EVENTS(0, 12),
-			AD799X_CHANNEL_WITH_EVENTS(1, 12),
-			AD799X_CHANNEL_WITH_EVENTS(2, 12),
-			AD799X_CHANNEL_WITH_EVENTS(3, 12),
-			AD799X_CHANNEL(4, 12),
-			AD799X_CHANNEL(5, 12),
-			AD799X_CHANNEL(6, 12),
-			AD799X_CHANNEL(7, 12),
-			IIO_CHAN_SOFT_TIMESTAMP(8),
-		},
 		.num_channels = 9,
-		.default_config = AD7998_ALERT_EN,
-		.info = &ad7993_4_7_8_info,
+		.noirq_config = {
+			.channel = {
+				AD799X_CHANNEL(0, 12),
+				AD799X_CHANNEL(1, 12),
+				AD799X_CHANNEL(2, 12),
+				AD799X_CHANNEL(3, 12),
+				AD799X_CHANNEL(4, 12),
+				AD799X_CHANNEL(5, 12),
+				AD799X_CHANNEL(6, 12),
+				AD799X_CHANNEL(7, 12),
+				IIO_CHAN_SOFT_TIMESTAMP(8),
+			},
+			.info = &ad7993_4_7_8_noirq_info,
+		},
+		.irq_config = {
+			.channel = {
+				AD799X_CHANNEL_WITH_EVENTS(0, 12),
+				AD799X_CHANNEL_WITH_EVENTS(1, 12),
+				AD799X_CHANNEL_WITH_EVENTS(2, 12),
+				AD799X_CHANNEL_WITH_EVENTS(3, 12),
+				AD799X_CHANNEL(4, 12),
+				AD799X_CHANNEL(5, 12),
+				AD799X_CHANNEL(6, 12),
+				AD799X_CHANNEL(7, 12),
+				IIO_CHAN_SOFT_TIMESTAMP(8),
+			},
+			.default_config = AD7998_ALERT_EN,
+			.info = &ad7993_4_7_8_irq_info,
+		},
 	},
 };
 
@@ -602,6 +690,8 @@ static int ad799x_probe(struct i2c_client *client,
 	int ret;
 	struct ad799x_state *st;
 	struct iio_dev *indio_dev;
+	const struct ad799x_chip_info *chip_info =
+		&ad799x_chip_info_tbl[id->driver_data];
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
 	if (indio_dev == NULL)
@@ -612,8 +702,9 @@ static int ad799x_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, indio_dev);
 
 	st->id = id->driver_data;
-	st->chip_info = &ad799x_chip_info_tbl[st->id];
-	st->config = st->chip_info->default_config;
+	st->chip_config = (client->irq > 0 && chip_info->irq_config.info) ?
+		&chip_info->irq_config : &chip_info->noirq_config;
+	st->config = st->chip_config->default_config;
 
 	/* TODO: Add pdata options for filtering and bit delay */
 
@@ -636,11 +727,11 @@ static int ad799x_probe(struct i2c_client *client,
 
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->name = id->name;
-	indio_dev->info = st->chip_info->info;
+	indio_dev->info = st->chip_config->info;
 
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = st->chip_info->channel;
-	indio_dev->num_channels = st->chip_info->num_channels;
+	indio_dev->channels = st->chip_config->channel;
+	indio_dev->num_channels = chip_info->num_channels;
 
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 		&ad799x_trigger_handler, NULL);
-- 
1.9.1

[PATCH 08/15] iio:adc:ad799x: Make chan_spec const in ad799x_chip_config struct

[Peter Meerwald]

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index eb49ac1..0f97cd2 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -107,7 +107,7 @@ enum {
  * @info:		pointer to iio_info struct
  */
 struct ad799x_chip_config {
-	struct iio_chan_spec		channel[9];
+	const struct iio_chan_spec	channel[9];
 	u16				default_config;
 	const struct iio_info		*info;
 };
-- 
1.9.1

[PATCH 09/15] iio:adc:ad799x: Fix reported event values, apply shift

[Peter Meerwald]

last two bits of ADC and limit valuse are zero and not reported (ad7993,
ad7997); compare with read_raw()

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 0f97cd2..0d8e950 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -392,7 +392,8 @@ static int ad799x_read_event_value(struct iio_dev *indio_dev,
 	mutex_unlock(&indio_dev->mlock);
 	if (ret < 0)
 		return ret;
-	*val = ret;
+	*val = (ret >> chan->scan_type.shift) &
+		GENMASK(chan->scan_type.realbits - 1, 0);
 
 	return IIO_VAL_INT;
 }
-- 
1.9.1

[PATCH 10/15] iio:adc:ad799x: Check event value range on write

[Peter Meerwald]

event values are 10 or 12 bit max., and need to be left-shifted
on ad7993, ad7997

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 0d8e950..dbc7c2b 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -368,9 +368,13 @@ static int ad799x_write_event_value(struct iio_dev *indio_dev,
 	int ret;
 	struct ad799x_state *st = iio_priv(indio_dev);
 
+	if (val > GENMASK(chan->scan_type.realbits - 1, 0))
+		return -EINVAL;
+
 	mutex_lock(&indio_dev->mlock);
 	ret = i2c_smbus_write_word_swapped(st->client,
-		ad799x_threshold_reg(chan, dir, info), val);
+		ad799x_threshold_reg(chan, dir, info),
+		val << chan->scan_type.shift);
 	mutex_unlock(&indio_dev->mlock);
 
 	return ret;
-- 
1.9.1

[PATCH 11/15] iio:adc:ad799x: Add helper function to read/write config register

[Peter Meerwald]

16-bit on ad7997/ad7998, 8-bit elsewhere

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index dbc7c2b..0219970 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -136,6 +136,30 @@ struct ad799x_state {
 	unsigned int			transfer_size;
 };
 
+static int ad799x_write_config(struct ad799x_state *st, u16 val)
+{
+	switch (st->id) {
+	case ad7997:
+	case ad7998:
+		return i2c_smbus_write_word_swapped(st->client, AD7998_CONF_REG,
+			val);
+	default:
+		return i2c_smbus_write_byte_data(st->client, AD7998_CONF_REG,
+			val);
+	}
+}
+
+static int ad799x_read_config(struct ad799x_state *st)
+{
+	switch (st->id) {
+	case ad7997:
+	case ad7998:
+		return i2c_smbus_read_word_swapped(st->client, AD7998_CONF_REG);
+	default:
+		return i2c_smbus_read_byte_data(st->client, AD7998_CONF_REG);
+	}
+}
+
 /**
  * ad799x_trigger_handler() bh of trigger launched polling to ring buffer
  *
-- 
1.9.1

[PATCH 12/15] iio:adc:ad799x: Write default config on probe and reset alert status on probe

[Peter Meerwald]

writing ALERT_EN and BUSY_ALERT to the chip config register clears
pending alerts, BUSY_ALERT is cleared when reading back the register

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 0219970..f8e029d 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -595,7 +595,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
 				AD799X_CHANNEL_WITH_EVENTS(1, 12),
 				IIO_CHAN_SOFT_TIMESTAMP(3),
 			},
-			.default_config = AD7998_ALERT_EN,
+			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
 			.info = &ad7993_4_7_8_irq_info,
 		},
 	},
@@ -619,7 +619,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
 				AD799X_CHANNEL_WITH_EVENTS(3, 10),
 				IIO_CHAN_SOFT_TIMESTAMP(4),
 			},
-			.default_config = AD7998_ALERT_EN,
+			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
 			.info = &ad7993_4_7_8_irq_info,
 		},
 	},
@@ -643,7 +643,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
 				AD799X_CHANNEL_WITH_EVENTS(3, 12),
 				IIO_CHAN_SOFT_TIMESTAMP(4),
 			},
-			.default_config = AD7998_ALERT_EN,
+			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
 			.info = &ad7993_4_7_8_irq_info,
 		},
 	},
@@ -675,7 +675,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
 				AD799X_CHANNEL(7, 10),
 				IIO_CHAN_SOFT_TIMESTAMP(8),
 			},
-			.default_config = AD7998_ALERT_EN,
+			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
 			.info = &ad7993_4_7_8_irq_info,
 		},
 	},
@@ -707,7 +707,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
 				AD799X_CHANNEL(7, 12),
 				IIO_CHAN_SOFT_TIMESTAMP(8),
 			},
-			.default_config = AD7998_ALERT_EN,
+			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
 			.info = &ad7993_4_7_8_irq_info,
 		},
 	},
@@ -733,7 +733,6 @@ static int ad799x_probe(struct i2c_client *client,
 	st->id = id->driver_data;
 	st->chip_config = (client->irq > 0 && chip_info->irq_config.info) ?
 		&chip_info->irq_config : &chip_info->noirq_config;
-	st->config = st->chip_config->default_config;
 
 	/* TODO: Add pdata options for filtering and bit delay */
 
@@ -762,6 +761,14 @@ static int ad799x_probe(struct i2c_client *client,
 	indio_dev->channels = st->chip_config->channel;
 	indio_dev->num_channels = chip_info->num_channels;
 
+	ret = ad799x_write_config(st, st->chip_config->default_config);
+	if (ret < 0)
+		goto error_disable_reg;
+	ret = ad799x_read_config(st);
+	if (ret < 0)
+		goto error_disable_reg;
+	st->config = ret;
+
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 		&ad799x_trigger_handler, NULL);
 	if (ret)
-- 
1.9.1

[PATCH 13/15] iio:adc:ad799x: Rename ad7997_8_update_scan_mode() to ad799x_update_scan_mode()

[Peter Meerwald]

function is used by all chips with ALERT pin

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index f8e029d..5c9445e 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -208,7 +208,7 @@ out:
 	return IRQ_HANDLED;
 }
 
-static int ad7997_8_update_scan_mode(struct iio_dev *indio_dev,
+static int ad799x_update_scan_mode(struct iio_dev *indio_dev,
 	const unsigned long *scan_mask)
 {
 	struct ad799x_state *st = iio_priv(indio_dev);
@@ -221,10 +221,14 @@ static int ad7997_8_update_scan_mode(struct iio_dev *indio_dev,
 	st->transfer_size = bitmap_weight(scan_mask, indio_dev->masklength) * 2;
 
 	switch (st->id) {
+	case ad7992:
+	case ad7993:
+	case ad7994:
 	case ad7997:
 	case ad7998:
-		return i2c_smbus_write_word_swapped(st->client, AD7998_CONF_REG,
-			st->config | (*scan_mask << AD799X_CHANNEL_SHIFT));
+		st->config &= ~(GENMASK(7, 0) << AD799X_CHANNEL_SHIFT);
+		st->config |= (*scan_mask << AD799X_CHANNEL_SHIFT);
+		return ad799x_write_config(st, st->config);
 	default:
 		return 0;
 	}
@@ -483,7 +487,7 @@ static const struct iio_info ad7991_info = {
 static const struct iio_info ad7993_4_7_8_noirq_info = {
 	.read_raw = &ad799x_read_raw,
 	.driver_module = THIS_MODULE,
-	.update_scan_mode = ad7997_8_update_scan_mode,
+	.update_scan_mode = ad799x_update_scan_mode,
 };
 
 static const struct iio_info ad7993_4_7_8_irq_info = {
@@ -493,7 +497,7 @@ static const struct iio_info ad7993_4_7_8_irq_info = {
 	.read_event_value = &ad799x_read_event_value,
 	.write_event_value = &ad799x_write_event_value,
 	.driver_module = THIS_MODULE,
-	.update_scan_mode = ad7997_8_update_scan_mode,
+	.update_scan_mode = ad799x_update_scan_mode,
 };
 
 static const struct iio_event_spec ad799x_events[] = {
-- 
1.9.1

[PATCH 14/15] iio:adc:ad799x: Return more meaningful event enabled state

[Peter Meerwald]

only report an event as enabled if it actually is enabled

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 5c9445e..b8191f1 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -364,7 +364,15 @@ static int ad799x_read_event_config(struct iio_dev *indio_dev,
 				    enum iio_event_type type,
 				    enum iio_event_direction dir)
 {
-	return 1;
+	struct ad799x_state *st = iio_priv(indio_dev);
+
+	if (!(st->config & AD7998_ALERT_EN))
+		return 0;
+
+	if ((st->config >> AD799X_CHANNEL_SHIFT) & BIT(chan->scan_index))
+		return 1;
+
+	return 0;
 }
 
 static unsigned int ad799x_threshold_reg(const struct iio_chan_spec *chan,
-- 
1.9.1

[PATCH 15/15] iio:adc:ad799x: Allow to write event config

[Peter Meerwald]

allow to enable events

previously, events were always reported as enabled, but actually only
implicitly enabled when updating the buffer scan mode

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
 drivers/iio/adc/ad799x.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index b8191f1..f8bfbcb 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -375,6 +375,39 @@ static int ad799x_read_event_config(struct iio_dev *indio_dev,
 	return 0;
 }
 
+static int ad799x_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 ad799x_state *st = iio_priv(indio_dev);
+	int ret;
+
+	mutex_lock(&indio_dev->mlock);
+	if (iio_buffer_enabled(indio_dev)) {
+		ret = -EBUSY;
+		goto done;
+	}
+
+	if (state)
+		st->config |= BIT(chan->scan_index) << AD799X_CHANNEL_SHIFT;
+	else
+		st->config &= ~(BIT(chan->scan_index) << AD799X_CHANNEL_SHIFT);
+
+	if (st->config >> AD799X_CHANNEL_SHIFT)
+		st->config |= AD7998_ALERT_EN;
+	else
+		st->config &= ~AD7998_ALERT_EN;
+
+	ret = ad799x_write_config(st, st->config);
+
+done:
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+
 static unsigned int ad799x_threshold_reg(const struct iio_chan_spec *chan,
 					 enum iio_event_direction dir,
 					 enum iio_event_info info)
@@ -502,6 +535,7 @@ static const struct iio_info ad7993_4_7_8_irq_info = {
 	.read_raw = &ad799x_read_raw,
 	.event_attrs = &ad799x_event_attrs_group,
 	.read_event_config = &ad799x_read_event_config,
+	.write_event_config = &ad799x_write_event_config,
 	.read_event_value = &ad799x_read_event_value,
 	.write_event_value = &ad799x_write_event_value,
 	.driver_module = THIS_MODULE,
-- 
1.9.1

Re: [PATCH 03/15] iio:adc:ad799x: Fix ad799x_chip_info kerneldoc

[Lars-Peter Clausen]

On 06/04/2014 12:42 AM, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

> ---
>   drivers/iio/adc/ad799x.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index 39b4cb4..7e08c60 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -105,9 +105,8 @@ enum {
>    * struct ad799x_chip_info - chip specific information
>    * @channel:		channel specification
>    * @num_channels:	number of channels
> - * @monitor_mode:	whether the chip supports monitor interrupts
>    * @default_config:	device default configuration
> - * @event_attrs:	pointer to the monitor event attribute group
> + * @info:		pointer to iio_info struct
>    */
>   struct ad799x_chip_info {
>   	struct iio_chan_spec		channel[9];
>

Re: [PATCH 10/15] iio:adc:ad799x: Check event value range on write

[Lars-Peter Clausen]

On 06/04/2014 12:42 AM, Peter Meerwald wrote:
> event values are 10 or 12 bit max., and need to be left-shifted
> on ad7993, ad7997
>

This should probably be squashed in the previous patch. As that one updates 
how the write value is computed and this one how the read value is computed.

> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> ---
>   drivers/iio/adc/ad799x.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index 0d8e950..dbc7c2b 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -368,9 +368,13 @@ static int ad799x_write_event_value(struct iio_dev *indio_dev,
>   	int ret;
>   	struct ad799x_state *st = iio_priv(indio_dev);
>
> +	if (val > GENMASK(chan->scan_type.realbits - 1, 0))
> +		return -EINVAL;
> +
>   	mutex_lock(&indio_dev->mlock);
>   	ret = i2c_smbus_write_word_swapped(st->client,
> -		ad799x_threshold_reg(chan, dir, info), val);
> +		ad799x_threshold_reg(chan, dir, info),
> +		val << chan->scan_type.shift);
>   	mutex_unlock(&indio_dev->mlock);
>
>   	return ret;
>

Re: [PATCH 15/15] iio:adc:ad799x: Allow to write event config

[Lars-Peter Clausen]

On 06/04/2014 12:42 AM, Peter Meerwald wrote:
> allow to enable events
>
> previously, events were always reported as enabled, but actually only
> implicitly enabled when updating the buffer scan mode
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> ---
>   drivers/iio/adc/ad799x.c | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index b8191f1..f8bfbcb 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -375,6 +375,39 @@ static int ad799x_read_event_config(struct iio_dev *indio_dev,
>   	return 0;
>   }
>
> +static int ad799x_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 ad799x_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&indio_dev->mlock);
> +	if (iio_buffer_enabled(indio_dev)) {
> +		ret = -EBUSY;
> +		goto done;
> +	}
> +
> +	if (state)
> +		st->config |= BIT(chan->scan_index) << AD799X_CHANNEL_SHIFT;
> +	else
> +		st->config &= ~(BIT(chan->scan_index) << AD799X_CHANNEL_SHIFT);
> +
> +	if (st->config >> AD799X_CHANNEL_SHIFT)
> +		st->config |= AD7998_ALERT_EN;
> +	else
> +		st->config &= ~AD7998_ALERT_EN;
> +
> +	ret = ad799x_write_config(st, st->config);

If I understand this correctly the enabled channels will be overwritten 
again as soon as the scan mode is updated. I think that is a bit unexpected. 
I'm not quite sure how to implement being able to independently enable a 
channel for sampling and for event monitoring in a proper way though.

Re: [PATCH 05/15] iio:adc:ad799x: Save some lines in ad7997_8_update_scan_mode() exit handling

[Lars-Peter Clausen]

On 06/04/2014 12:42 AM, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

well, ok

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

> ---
>   drivers/iio/adc/ad799x.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index c03406e..24eca2f 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -193,10 +193,8 @@ static int ad7997_8_update_scan_mode(struct iio_dev *indio_dev,
>   		return i2c_smbus_write_word_swapped(st->client, AD7998_CONF_REG,
>   			st->config | (*scan_mask << AD799X_CHANNEL_SHIFT));
>   	default:
> -		break;
> +		return 0;
>   	}
> -
> -	return 0;
>   }
>
>   static int ad799x_scan_direct(struct ad799x_state *st, unsigned ch)
>

Re: [PATCH 08/15] iio:adc:ad799x: Make chan_spec const in ad799x_chip_config struct

[Lars-Peter Clausen]

On 06/04/2014 12:42 AM, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

> ---
>   drivers/iio/adc/ad799x.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index eb49ac1..0f97cd2 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -107,7 +107,7 @@ enum {
>    * @info:		pointer to iio_info struct
>    */
>   struct ad799x_chip_config {
> -	struct iio_chan_spec		channel[9];
> +	const struct iio_chan_spec	channel[9];
>   	u16				default_config;
>   	const struct iio_info		*info;
>   };
>

Re: [PATCH 07/15] iio:adc:ad799x: Only expose event interface when IRQ is available

[Lars-Peter Clausen]

On 06/04/2014 12:42 AM, Peter Meerwald wrote:
> an IRQ is necessary to handle the ALERT condition; without
> IRQ, the IIO event interface serves no purpose
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

Makes sense. We should probably at some point investigate if it is possible 
to implement something to ignore certain channel attributes when the channel 
is created, rather than having to have two almost identical declarations of 
the channel arrays.

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

[...]
> @@ -612,8 +702,9 @@ static int ad799x_probe(struct i2c_client *client,
>   	i2c_set_clientdata(client, indio_dev);
>
>   	st->id = id->driver_data;
> -	st->chip_info = &ad799x_chip_info_tbl[st->id];
> -	st->config = st->chip_info->default_config;
> +	st->chip_config = (client->irq > 0 && chip_info->irq_config.info) ?
> +		&chip_info->irq_config : &chip_info->noirq_config;

Nitpick: should be

	if (...)
		st->chip_config = ...;
	else
		st->chip_config = ...;

> +	st->config = st->chip_config->default_config;
>
>   	/* TODO: Add pdata options for filtering and bit delay */
>
[...]

Re: [PATCH 11/15] iio:adc:ad799x: Add helper function to read/write config register

[Lars-Peter Clausen]

On 06/04/2014 12:42 AM, Peter Meerwald wrote:
> 16-bit on ad7997/ad7998, 8-bit elsewhere
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

Should this and patch 13 be considered a bug fix?

> ---
>   drivers/iio/adc/ad799x.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index dbc7c2b..0219970 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -136,6 +136,30 @@ struct ad799x_state {
>   	unsigned int			transfer_size;
>   };
>
> +static int ad799x_write_config(struct ad799x_state *st, u16 val)
> +{
> +	switch (st->id) {
> +	case ad7997:
> +	case ad7998:
> +		return i2c_smbus_write_word_swapped(st->client, AD7998_CONF_REG,
> +			val);
> +	default:
> +		return i2c_smbus_write_byte_data(st->client, AD7998_CONF_REG,
> +			val);
> +	}
> +}
> +
> +static int ad799x_read_config(struct ad799x_state *st)
> +{
> +	switch (st->id) {
> +	case ad7997:
> +	case ad7998:
> +		return i2c_smbus_read_word_swapped(st->client, AD7998_CONF_REG);
> +	default:
> +		return i2c_smbus_read_byte_data(st->client, AD7998_CONF_REG);
> +	}
> +}
> +
>   /**
>    * ad799x_trigger_handler() bh of trigger launched polling to ring buffer
>    *
>

Re: [PATCH 12/15] iio:adc:ad799x: Write default config on probe and reset alert status on probe

[Lars-Peter Clausen]

On 06/04/2014 12:42 AM, Peter Meerwald wrote:
> writing ALERT_EN and BUSY_ALERT to the chip config register clears
> pending alerts, BUSY_ALERT is cleared when reading back the register
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

> ---
>   drivers/iio/adc/ad799x.c | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index 0219970..f8e029d 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -595,7 +595,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
>   				AD799X_CHANNEL_WITH_EVENTS(1, 12),
>   				IIO_CHAN_SOFT_TIMESTAMP(3),
>   			},
> -			.default_config = AD7998_ALERT_EN,
> +			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
>   			.info = &ad7993_4_7_8_irq_info,
>   		},
>   	},
> @@ -619,7 +619,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
>   				AD799X_CHANNEL_WITH_EVENTS(3, 10),
>   				IIO_CHAN_SOFT_TIMESTAMP(4),
>   			},
> -			.default_config = AD7998_ALERT_EN,
> +			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
>   			.info = &ad7993_4_7_8_irq_info,
>   		},
>   	},
> @@ -643,7 +643,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
>   				AD799X_CHANNEL_WITH_EVENTS(3, 12),
>   				IIO_CHAN_SOFT_TIMESTAMP(4),
>   			},
> -			.default_config = AD7998_ALERT_EN,
> +			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
>   			.info = &ad7993_4_7_8_irq_info,
>   		},
>   	},
> @@ -675,7 +675,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
>   				AD799X_CHANNEL(7, 10),
>   				IIO_CHAN_SOFT_TIMESTAMP(8),
>   			},
> -			.default_config = AD7998_ALERT_EN,
> +			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
>   			.info = &ad7993_4_7_8_irq_info,
>   		},
>   	},
> @@ -707,7 +707,7 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
>   				AD799X_CHANNEL(7, 12),
>   				IIO_CHAN_SOFT_TIMESTAMP(8),
>   			},
> -			.default_config = AD7998_ALERT_EN,
> +			.default_config = AD7998_ALERT_EN | AD7998_BUSY_ALERT,
>   			.info = &ad7993_4_7_8_irq_info,
>   		},
>   	},
> @@ -733,7 +733,6 @@ static int ad799x_probe(struct i2c_client *client,
>   	st->id = id->driver_data;
>   	st->chip_config = (client->irq > 0 && chip_info->irq_config.info) ?
>   		&chip_info->irq_config : &chip_info->noirq_config;
> -	st->config = st->chip_config->default_config;
>
>   	/* TODO: Add pdata options for filtering and bit delay */
>
> @@ -762,6 +761,14 @@ static int ad799x_probe(struct i2c_client *client,
>   	indio_dev->channels = st->chip_config->channel;
>   	indio_dev->num_channels = chip_info->num_channels;
>
> +	ret = ad799x_write_config(st, st->chip_config->default_config);
> +	if (ret < 0)
> +		goto error_disable_reg;
> +	ret = ad799x_read_config(st);
> +	if (ret < 0)
> +		goto error_disable_reg;
> +	st->config = ret;
> +
>   	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>   		&ad799x_trigger_handler, NULL);
>   	if (ret)
>

Re: [PATCH 13/15] iio:adc:ad799x: Rename ad7997_8_update_scan_mode() to ad799x_update_scan_mode()

[Lars-Peter Clausen]

On 06/04/2014 12:42 AM, Peter Meerwald wrote:
> function is used by all chips with ALERT pin

The subject is a bit misleading, this does a bit more than just renaming the 
function.

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> ---
>   drivers/iio/adc/ad799x.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index f8e029d..5c9445e 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -208,7 +208,7 @@ out:
>   	return IRQ_HANDLED;
>   }
>
> -static int ad7997_8_update_scan_mode(struct iio_dev *indio_dev,
> +static int ad799x_update_scan_mode(struct iio_dev *indio_dev,
>   	const unsigned long *scan_mask)
>   {
>   	struct ad799x_state *st = iio_priv(indio_dev);
> @@ -221,10 +221,14 @@ static int ad7997_8_update_scan_mode(struct iio_dev *indio_dev,
>   	st->transfer_size = bitmap_weight(scan_mask, indio_dev->masklength) * 2;
>
>   	switch (st->id) {
> +	case ad7992:
> +	case ad7993:
> +	case ad7994:
>   	case ad7997:
>   	case ad7998:
> -		return i2c_smbus_write_word_swapped(st->client, AD7998_CONF_REG,
> -			st->config | (*scan_mask << AD799X_CHANNEL_SHIFT));
> +		st->config &= ~(GENMASK(7, 0) << AD799X_CHANNEL_SHIFT);
> +		st->config |= (*scan_mask << AD799X_CHANNEL_SHIFT);
> +		return ad799x_write_config(st, st->config);
>   	default:
>   		return 0;
>   	}
> @@ -483,7 +487,7 @@ static const struct iio_info ad7991_info = {
>   static const struct iio_info ad7993_4_7_8_noirq_info = {
>   	.read_raw = &ad799x_read_raw,
>   	.driver_module = THIS_MODULE,
> -	.update_scan_mode = ad7997_8_update_scan_mode,
> +	.update_scan_mode = ad799x_update_scan_mode,
>   };
>
>   static const struct iio_info ad7993_4_7_8_irq_info = {
> @@ -493,7 +497,7 @@ static const struct iio_info ad7993_4_7_8_irq_info = {
>   	.read_event_value = &ad799x_read_event_value,
>   	.write_event_value = &ad799x_write_event_value,
>   	.driver_module = THIS_MODULE,
> -	.update_scan_mode = ad7997_8_update_scan_mode,
> +	.update_scan_mode = ad799x_update_scan_mode,
>   };
>
>   static const struct iio_event_spec ad799x_events[] = {
>

Re: [PATCH 15/15] iio:adc:ad799x: Allow to write event config

[Peter Meerwald]

Hello Lars-Peter,

thank you for reviewing this series! reply below

> > previously, events were always reported as enabled, but actually only
> > implicitly enabled when updating the buffer scan mode
> > 
> > Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> > ---
> >   drivers/iio/adc/ad799x.c | 34 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 34 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> > index b8191f1..f8bfbcb 100644
> > --- a/drivers/iio/adc/ad799x.c
> > +++ b/drivers/iio/adc/ad799x.c
> > @@ -375,6 +375,39 @@ static int ad799x_read_event_config(struct iio_dev
> > *indio_dev,
> >   	return 0;
> >   }
> > 
> > +static int ad799x_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 ad799x_state *st = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	mutex_lock(&indio_dev->mlock);
> > +	if (iio_buffer_enabled(indio_dev)) {
> > +		ret = -EBUSY;
> > +		goto done;
> > +	}
> > +
> > +	if (state)
> > +		st->config |= BIT(chan->scan_index) << AD799X_CHANNEL_SHIFT;
> > +	else
> > +		st->config &= ~(BIT(chan->scan_index) <<
> > AD799X_CHANNEL_SHIFT);
> > +
> > +	if (st->config >> AD799X_CHANNEL_SHIFT)
> > +		st->config |= AD7998_ALERT_EN;
> > +	else
> > +		st->config &= ~AD7998_ALERT_EN;
> > +
> > +	ret = ad799x_write_config(st, st->config);
> 
> If I understand this correctly the enabled channels will be overwritten again
> as soon as the scan mode is updated. I think that is a bit unexpected. I'm not
> quite sure how to implement being able to independently enable a channel for
> sampling and for event monitoring in a proper way though.

yes, this is a problem; I have no good solution
at least the enabled events show their correct status now :)

one could turn on the union set of event and scan mode channels and ignore
events or measurement when not in the event or or scan mode set, resp.

sysfs-bus-iio documentation says 
"So if you want to be sure you have set what you think you have, check the 
contents of these attributes after everything is configured. Drivers 
may have to buffer any parameters so that they are consistent 
when a given event type is enabled at a future point (and not 
those for whatever event was previously enabled)."

so there already is a warning :)


another question:
is an event supposed to occur on crossing the threshold or whenever and 
as long as the ADC value exceeds the threshold?

the ad7997 does the later, the documentation is not very clear what is 
to be expected

p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 11/15] iio:adc:ad799x: Add helper function to read/write config register

[Peter Meerwald]

> Should this and patch 13 be considered a bug fix?

patch 11 is not a bug fix; the ad799x driver did update the CONF_REG on 
ad7997 and ad7998 only

patch 13 is a bug fix I think; on ad7992/3/4 no channels were ever 
enabled, hence no events

> > ---
> >   drivers/iio/adc/ad799x.c | 24 ++++++++++++++++++++++++
> >   1 file changed, 24 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> > index dbc7c2b..0219970 100644
> > --- a/drivers/iio/adc/ad799x.c
> > +++ b/drivers/iio/adc/ad799x.c
> > @@ -136,6 +136,30 @@ struct ad799x_state {
> >   	unsigned int			transfer_size;
> >   };
> > 
> > +static int ad799x_write_config(struct ad799x_state *st, u16 val)
> > +{
> > +	switch (st->id) {
> > +	case ad7997:
> > +	case ad7998:
> > +		return i2c_smbus_write_word_swapped(st->client,
> > AD7998_CONF_REG,
> > +			val);
> > +	default:
> > +		return i2c_smbus_write_byte_data(st->client, AD7998_CONF_REG,
> > +			val);
> > +	}
> > +}
> > +
> > +static int ad799x_read_config(struct ad799x_state *st)
> > +{
> > +	switch (st->id) {
> > +	case ad7997:
> > +	case ad7998:
> > +		return i2c_smbus_read_word_swapped(st->client,
> > AD7998_CONF_REG);
> > +	default:
> > +		return i2c_smbus_read_byte_data(st->client, AD7998_CONF_REG);
> > +	}
> > +}
> > +
> >   /**
> >    * ad799x_trigger_handler() bh of trigger launched polling to ring buffer
> >    *
> > 
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 15/15] iio:adc:ad799x: Allow to write event config

[Lars-Peter Clausen]

On 06/04/2014 10:35 AM, Peter Meerwald wrote:
> Hello Lars-Peter,
>
> thank you for reviewing this series! reply below
>
>>> previously, events were always reported as enabled, but actually only
>>> implicitly enabled when updating the buffer scan mode
>>>
>>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
>>> ---
>>>    drivers/iio/adc/ad799x.c | 34 ++++++++++++++++++++++++++++++++++
>>>    1 file changed, 34 insertions(+)
>>>
>>> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
>>> index b8191f1..f8bfbcb 100644
>>> --- a/drivers/iio/adc/ad799x.c
>>> +++ b/drivers/iio/adc/ad799x.c
>>> @@ -375,6 +375,39 @@ static int ad799x_read_event_config(struct iio_dev
>>> *indio_dev,
>>>    	return 0;
>>>    }
>>>
>>> +static int ad799x_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 ad799x_state *st = iio_priv(indio_dev);
>>> +	int ret;
>>> +
>>> +	mutex_lock(&indio_dev->mlock);
>>> +	if (iio_buffer_enabled(indio_dev)) {
>>> +		ret = -EBUSY;
>>> +		goto done;
>>> +	}
>>> +
>>> +	if (state)
>>> +		st->config |= BIT(chan->scan_index) << AD799X_CHANNEL_SHIFT;
>>> +	else
>>> +		st->config &= ~(BIT(chan->scan_index) <<
>>> AD799X_CHANNEL_SHIFT);
>>> +
>>> +	if (st->config >> AD799X_CHANNEL_SHIFT)
>>> +		st->config |= AD7998_ALERT_EN;
>>> +	else
>>> +		st->config &= ~AD7998_ALERT_EN;
>>> +
>>> +	ret = ad799x_write_config(st, st->config);
>>
>> If I understand this correctly the enabled channels will be overwritten again
>> as soon as the scan mode is updated. I think that is a bit unexpected. I'm not
>> quite sure how to implement being able to independently enable a channel for
>> sampling and for event monitoring in a proper way though.
>
> yes, this is a problem; I have no good solution
> at least the enabled events show their correct status now :)
>
> one could turn on the union set of event and scan mode channels and ignore
> events or measurement when not in the event or or scan mode set, resp.

I that would be better, although it will probably require a custom demux for 
the read data.

>
> sysfs-bus-iio documentation says
> "So if you want to be sure you have set what you think you have, check the
> contents of these attributes after everything is configured. Drivers
> may have to buffer any parameters so that they are consistent
> when a given event type is enabled at a future point (and not
> those for whatever event was previously enabled)."
>
> so there already is a warning :)
>
>
> another question:
> is an event supposed to occur on crossing the threshold or whenever and
> as long as the ADC value exceeds the threshold?
>
> the ad7997 does the later, the documentation is not very clear what is
> to be expected

It should probably only generate an event when it crosses the threshold 
everything else makes little sense since we'd just generate a IRQ storm. 
E.g. you can't make a over-temperature event go away in the interrupt 
handler. I think in some drivers we temporarily disable the interrupt and 
then start polling and wait for the value to go back below the threshold and 
then re-enable it.

- Lars

Re: [PATCH 00/15] ad799x cleanup

[Jonathan Cameron]

On 03/06/14 23:42, Peter Meerwald wrote:
> Hello,
>
> I ran into some issues with IIO events using the ad799x ADC driver with an
> ad7997 chip; the following series has cleanups and fixes
Hi Peter,

A useful looking set.  Will take a look once Lars is happy with them all
(I'm lazy).

Putting my maintainer hat on, I'm trying to work out what if anything in
here would want to go to stable.

We are rather two interlinked between clean ups and fixes though I can
see why you did it like this.  If possible to reorder to have any fixes at
the top that would be great.
>
> patches 1-6, 8 are cleanup
>
> patch 7 exposes the IIO event interface of the driver only if an IRQ has been configured
> for the device; no IRQ -> no events
While unexpected behaviour (and good to fix) I don't think this one is stable material.
>
> patch 9 shifts out the last two bits when reading the event value on ad7993 and ad7997
> (which have 10-bit ADCs); same as read_raw()
This looks like stable material.
>
> patch 10 checks the range of the event value on write and shifts the value into place;
> similar to the previous patch
this one as well (as Lars suggested, combining these makes sense...)
>
> patch 11 adds helper function to read/write the chip's config register (16-bit on
> ad7997/ad7998, 8-bit everywhere else)
You've already stated this is a cleanup rather than a bug fix.
>
> patch 12 actually writes the default config to the chip and keeps a copy of
> the config register in the driver's state
Another unexpected behaviour case, I think...
>
> patch 13 changes update_scan_mode() to store the enabled channels in the config
> register, on ad7992/ad7993/ad7994/ad7997/ad7998, hence the rename
Ouch - this would be stable material, but is rather more involved with earlier
patches than ideal...
>
> patch 14 reports only those events as enabled which actually are
Unexpected behaviour again, though I'd hope any userspace code would cope with this.
In theory any event can result in any other even being enabled, but then userspace
should be able to see what is enabled by reading back the relevant attributes.
It's clearly a bug fix, but stable material?
>
> patch 15 allows to write the event config, previously events could only be
> set implicitly via the buffer scan mode
Clearly still under discussion but sounds like more a case of unusual behaviour
(in need of fixing) than a bug that wants patching in stable?

Sorry for being a pain on this.  Until fairly recently I'd just have been
cynical and applied this lot to the togreg branch and ignored the fact
it would be left broken in earlier versions!  I'm trying to do this better
hence this email.

J
>
> regards, p.
>
> Peter Meerwald (15):
>    staging:iio: Update iio_event_monitor program
>    staging:iio: Fix iio_utils.h function prototypes
>    iio:adc:ad799x: Fix ad799x_chip_info kerneldoc
>    iio:adc:ad799x: Drop I2C access helper functions
>    iio:adc:ad799x: Save some lines in ad7997_8_update_scan_mode() exit
>      handling
>    iio:adc:ad799x: Use BIT() and GENMASK()
>    iio:adc:ad799x: Only expose event interface when IRQ is available
>    iio:adc:ad799x: Make chan_spec const in ad799x_chip_config struct
>    iio:adc:ad799x: Fix reported event values, apply shift
>    iio:adc:ad799x: Check event value range on write
>    iio:adc:ad799x: Add helper function to read/write config register
>    iio:adc:ad799x: Write default config on probe and reset alert status
>      on probe
>    iio:adc:ad799x: Rename ad7997_8_update_scan_mode() to
>      ad799x_update_scan_mode()
>    iio:adc:ad799x: Return more meaningful event enabled state
>    iio:adc:ad799x: Allow to write event config
>
>   drivers/iio/adc/ad799x.c                           | 507 ++++++++++++---------
>   .../staging/iio/Documentation/iio_event_monitor.c  |  10 +
>   drivers/staging/iio/Documentation/iio_utils.h      |   6 +-
>   3 files changed, 317 insertions(+), 206 deletions(-)
>

Re: [PATCH 00/15] ad799x cleanup

[Peter Meerwald]

Hello,

> > I ran into some issues with IIO events using the ad799x ADC driver with an
> > ad7997 chip; the following series has cleanups and fixes

> A useful looking set.  Will take a look once Lars is happy with them all
> (I'm lazy).

patches are mostly ack'd; I resend a v2

> Putting my maintainer hat on, I'm trying to work out what if anything in
> here would want to go to stable.
> 
> We are rather two interlinked between clean ups and fixes though I can
> see why you did it like this.  If possible to reorder to have any fixes at
> the top that would be great.
> > 
> > patches 1-6, 8 are cleanup
> > 
> > patch 7 exposes the IIO event interface of the driver only if an IRQ has
> > been configured
> > for the device; no IRQ -> no events
> While unexpected behaviour (and good to fix) I don't think this one is stable
> material.
> > 
> > patch 9 shifts out the last two bits when reading the event value on ad7993
> > and ad7997
> > (which have 10-bit ADCs); same as read_raw()
> This looks like stable material.

the ad799x was recently moved from staging (for 3.16)
we'd need separate patches to fix staging in stable -- not sure if it's 
worth it: those who are using the staging driver in a stable kernel 
probably prefer to keep things as-is (otherwise they'd have sent a patch)
 
> > patch 10 checks the range of the event value on write and shifts the value
> > into place;
> > similar to the previous patch
> this one as well (as Lars suggested, combining these makes sense...)
> > 
> > patch 11 adds helper function to read/write the chip's config register
> > (16-bit on
> > ad7997/ad7998, 8-bit everywhere else)
> You've already stated this is a cleanup rather than a bug fix.
> > 
> > patch 12 actually writes the default config to the chip and keeps a copy of
> > the config register in the driver's state
> Another unexpected behaviour case, I think...
> > 
> > patch 13 changes update_scan_mode() to store the enabled channels in the
> > config
> > register, on ad7992/ad7993/ad7994/ad7997/ad7998, hence the rename
> Ouch - this would be stable material, but is rather more involved with earlier
> patches than ideal...
> > 
> > patch 14 reports only those events as enabled which actually are
> Unexpected behaviour again, though I'd hope any userspace code would cope with
> this.
> In theory any event can result in any other even being enabled, but then
> userspace
> should be able to see what is enabled by reading back the relevant attributes.
> It's clearly a bug fix, but stable material?
> > 
> > patch 15 allows to write the event config, previously events could only be
> > set implicitly via the buffer scan mode
> Clearly still under discussion but sounds like more a case of unusual
> behaviour
> (in need of fixing) than a bug that wants patching in stable?
> 
> Sorry for being a pain on this.  Until fairly recently I'd just have been
> cynical and applied this lot to the togreg branch and ignored the fact
> it would be left broken in earlier versions!  I'm trying to do this better
> hence this email.

I think the rules for -stable are to have a patch in -current and then 
backport; I've put the reading/writing event values (shifting) patch in 
front

patches for 3.14, 3.12, 3.10, 3.4, 3.2 would be extra work (due to moving 
and renaming)

regards, p.

> > Peter Meerwald (15):
> >    staging:iio: Update iio_event_monitor program
> >    staging:iio: Fix iio_utils.h function prototypes
> >    iio:adc:ad799x: Fix ad799x_chip_info kerneldoc
> >    iio:adc:ad799x: Drop I2C access helper functions
> >    iio:adc:ad799x: Save some lines in ad7997_8_update_scan_mode() exit
> >      handling
> >    iio:adc:ad799x: Use BIT() and GENMASK()
> >    iio:adc:ad799x: Only expose event interface when IRQ is available
> >    iio:adc:ad799x: Make chan_spec const in ad799x_chip_config struct
> >    iio:adc:ad799x: Fix reported event values, apply shift
> >    iio:adc:ad799x: Check event value range on write
> >    iio:adc:ad799x: Add helper function to read/write config register
> >    iio:adc:ad799x: Write default config on probe and reset alert status
> >      on probe
> >    iio:adc:ad799x: Rename ad7997_8_update_scan_mode() to
> >      ad799x_update_scan_mode()
> >    iio:adc:ad799x: Return more meaningful event enabled state
> >    iio:adc:ad799x: Allow to write event config
> > 
> >   drivers/iio/adc/ad799x.c                           | 507
> > ++++++++++++---------
> >   .../staging/iio/Documentation/iio_event_monitor.c  |  10 +
> >   drivers/staging/iio/Documentation/iio_utils.h      |   6 +-
> >   3 files changed, 317 insertions(+), 206 deletions(-)
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)