linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] IIO wrapper drivers, dpot-dac and envelope-detector
@ 2016-10-22 22:43 Peter Rosin
  2016-10-22 22:43 ` [PATCH v2 1/7] iio:core: add a callback to allow drivers to provide _available attributes Peter Rosin
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Peter Rosin @ 2016-10-22 22:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, linux-iio, devicetree

Hi!

These two drivers share the fact that they wrap another iio channel,
and I use the first in combination with the second, which is why I'm
submitting them as a pair.

The first driver is a simple wrapper converting an iio dpot into an
iio dac. It only changes the unit and scale. It also does not add any
fancy iio buffer support that I don't need. I suppose that can be
added. By someone else :-)

The second driver (the envelope detector) is more involved. It also
explains why I need the dpot-dac driver. I wanted the envelope
detector to be generic and work with any dac, but I had a dpot...

But before those two new drivers, there is some infrastructure added
to provide available values for a channel.

v1 -> v2
- provide out_resistanceX_raw_available channel attribute in mcp4531 dpot

dpot-dac:
- change Vref to vref
- the module will be called dpot-dac (in Kconfig help)
- removed a 'the'
- removed (s64) cast
- make the channel indexed, makes libiio find the channel (tested with 0.5)
- add a comment on how integer scale is converted to fractional scale
  and clarify the code a bit
- dig out max-ohms by looking at scale and maximum available raw value
  from the dpot channel and drop the 'dpot-dac,max-ohms' devicetree property
- provide out_voltageX_raw_available channel attribute

envelope-detector:
- change compatible from envelope-detector to axentia,tse850-envelope-detector
- remove envelope-detector,invert and envelope-detector,comp-interval-ms from
  devicetree and add them as iio device attributes instead
- make the channel indexed, makes libiio find the channel (tested with 0.5)
- reorder struct envelope to better indicate what is covered by read_lock
- add comment on interaction between envelope_detector_comp_latch (renamed
  from envelope_detector_latch) and envelope_detector_comp_isr (renamed
  from envelope_detector_isr)
- fixup a problem in envelope_detector_comp_latch where interrupts pending
  from when the interrupt has been disabled interferes with expected
  operation
- slight rewrite of the initial high/low assignments
- use a better name when requesting the interrupt
- dig out dac_max by looking at scale and maximum available raw value
  from the dac channel and drop the 'envelope-detector,dac-max' devicetree
  property

Ok, so the devicetree compatible string for the envelope-detector gained
the "axentia,tse850-" prefix, but at the same time the driver also gained
generality that in my mind puts it in about the same position as the
dpot-dac driver. The driver really has very little to do with the specifics
of the TSE-850. But at the same time there are more ways to build an
envelope detector compared to the obviousness of building a dac from a dpot...

One thing I don't like is that the irq needs to be changed for cases where
it is only ever interesting with the 'invert' variant, and it may not work
to set up the irq the wrong way first. For the TSE-850, this does not
matter, since we have a mux on the envelope detector input and can make use
of both 'invert' and 'normal' for different signals (we could have gotten
by with only 'invert' since the only signals we are measuring that are
'normal' are also DC signals and can thus be detected both from below and
from above), but it is nice to have it both ways.

Also, perhaps the new device attributes (invert and compare_interval_ms)
should really be channel attributes?

Cheers,
Peter

PS. I have not tried to forward port anything else from the thread with
the old thread with the initial iio:core patch.

Jonathan Cameron (1):
  iio:core: add a callback to allow drivers to provide _available
    attributes

Peter Rosin (6):
  iio: mcp4531: provide range of available raw values
  dt-bindings: add axentia to vendor-prefixes
  dt-bindings: iio: document dpot-dac bindings
  iio: dpot-dac: DAC driver based on a digital potentiometer
  dt-bindings: iio: document envelope-detector bindings
  iio: envelope-detector: ADC driver based on a DAC and a comparator

 .../testing/sysfs-bus-iio-adc-envelope-detector    |  36 ++
 .../bindings/iio/adc/envelope-detector.txt         |  55 +++
 .../devicetree/bindings/iio/dac/dpot-dac.txt       |  41 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 MAINTAINERS                                        |  15 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/envelope-detector.c                | 463 +++++++++++++++++++++
 drivers/iio/dac/Kconfig                            |  10 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/dpot-dac.c                         | 298 +++++++++++++
 drivers/iio/industrialio-core.c                    | 232 +++++++++--
 drivers/iio/potentiometer/mcp4531.c                | 104 +++--
 include/linux/iio/iio.h                            |  11 +
 include/linux/iio/types.h                          |   5 +
 15 files changed, 1216 insertions(+), 67 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
 create mode 100644 Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
 create mode 100644 Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
 create mode 100644 drivers/iio/adc/envelope-detector.c
 create mode 100644 drivers/iio/dac/dpot-dac.c

-- 
2.1.4

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

* [PATCH v2 1/7] iio:core: add a callback to allow drivers to provide _available attributes
  2016-10-22 22:43 [PATCH v2 0/7] IIO wrapper drivers, dpot-dac and envelope-detector Peter Rosin
@ 2016-10-22 22:43 ` Peter Rosin
  2016-10-23  9:33   ` Jonathan Cameron
  2016-10-22 22:43 ` [PATCH v2 2/7] iio: mcp4531: provide range of available raw values Peter Rosin
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Peter Rosin @ 2016-10-22 22:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Cameron, Peter Rosin, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, linux-iio, devicetree

From: Jonathan Cameron <jic23@kernel.org>

A large number of attributes can only take a limited range of values.
Currently in IIO this is handled by directly registering additional
*_available attributes thus providing this information to userspace.

It is desirable to provide this information via the core for much the same
reason this was done for the actual channel information attributes in the
first place.  If it isn't there, then it can only really be accessed from
userspace.  Other in kernel IIO consumers have no access to what valid
parameters are.

Two forms are currently supported:
* list of values in one particular IIO_VAL_* format.
	e.g. 1.300000 1.500000 1.730000
* range specification with a step size:
	e.g. [1.000000 0.500000 2.500000]
	equivalent to 1.000000 1.5000000 2.000000 2.500000

An addition set of masks are used to allow different sharing rules for the
*_available attributes generated.

This allows for example:

in_accel_x_offset
in_accel_y_offset
in_accel_offset_available.

We could have gone with having a specification for each and every
info_mask element but that would have meant changing the existing userspace
ABI.  This approach does not.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
[rather mindlessly forward ported from approx 3 years ago /peda]
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/iio/industrialio-core.c | 232 +++++++++++++++++++++++++++++++++++-----
 include/linux/iio/iio.h         |  11 ++
 include/linux/iio/types.h       |   5 +
 3 files changed, 223 insertions(+), 25 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index fc340ed3dca1..93c69ebeda1e 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -575,51 +575,41 @@ int of_iio_read_mount_matrix(const struct device *dev,
 #endif
 EXPORT_SYMBOL(of_iio_read_mount_matrix);
 
-/**
- * iio_format_value() - Formats a IIO value into its string representation
- * @buf:	The buffer to which the formatted value gets written
- * @type:	One of the IIO_VAL_... constants. This decides how the val
- *		and val2 parameters are formatted.
- * @size:	Number of IIO value entries contained in vals
- * @vals:	Pointer to the values, exact meaning depends on the
- *		type parameter.
- *
- * Return: 0 by default, a negative number on failure or the
- *	   total number of characters written for a type that belongs
- *	   to the IIO_VAL_... constant.
- */
-ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
+static ssize_t __iio_format_value(char *buf, unsigned int type,
+				  int size, const int *vals)
 {
 	unsigned long long tmp;
+	int tmp0, tmp1;
 	bool scale_db = false;
 
 	switch (type) {
 	case IIO_VAL_INT:
-		return sprintf(buf, "%d\n", vals[0]);
+		return sprintf(buf, "%d", vals[0]);
 	case IIO_VAL_INT_PLUS_MICRO_DB:
 		scale_db = true;
 	case IIO_VAL_INT_PLUS_MICRO:
 		if (vals[1] < 0)
-			return sprintf(buf, "-%d.%06u%s\n", abs(vals[0]),
+			return sprintf(buf, "-%d.%06u%s", abs(vals[0]),
 				       -vals[1], scale_db ? " dB" : "");
 		else
-			return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
+			return sprintf(buf, "%d.%06u%s", vals[0], vals[1],
 				scale_db ? " dB" : "");
 	case IIO_VAL_INT_PLUS_NANO:
 		if (vals[1] < 0)
-			return sprintf(buf, "-%d.%09u\n", abs(vals[0]),
+			return sprintf(buf, "-%d.%09u", abs(vals[0]),
 				       -vals[1]);
 		else
-			return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
+			return sprintf(buf, "%d.%09u", vals[0], vals[1]);
 	case IIO_VAL_FRACTIONAL:
 		tmp = div_s64((s64)vals[0] * 1000000000LL, vals[1]);
-		vals[0] = (int)div_s64_rem(tmp, 1000000000, &vals[1]);
-		return sprintf(buf, "%d.%09u\n", vals[0], abs(vals[1]));
+		tmp1 = vals[1];
+		tmp0 = (int)div_s64_rem(tmp, 1000000000, &tmp1);
+		return sprintf(buf, "%d.%09u", tmp0, abs(tmp1));
 	case IIO_VAL_FRACTIONAL_LOG2:
 		tmp = (s64)vals[0] * 1000000000LL >> vals[1];
-		vals[1] = do_div(tmp, 1000000000LL);
-		vals[0] = tmp;
-		return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
+		tmp1 = do_div(tmp, 1000000000LL);
+		tmp0 = tmp;
+		return sprintf(buf, "%d.%09u", tmp0, tmp1);
 	case IIO_VAL_INT_MULTIPLE:
 	{
 		int i;
@@ -628,13 +618,34 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
 		for (i = 0; i < size; ++i)
 			len += snprintf(&buf[len], PAGE_SIZE - len, "%d ",
 								vals[i]);
-		len += snprintf(&buf[len], PAGE_SIZE - len, "\n");
 		return len;
 	}
 	default:
 		return 0;
 	}
 }
+
+/**
+ * iio_format_value() - Formats a IIO value into its string representation
+ * @buf:	The buffer to which the formatted value gets written
+ * @type:	One of the IIO_VAL_... constants. This decides how the val
+ *		and val2 parameters are formatted.
+ * @size:	Number of IIO value entries contained in vals
+ * @vals:	Pointer to the values, exact meaning depends on the
+ *		type parameter.
+ *
+ * Return: 0 by default, a negative number on failure or the
+ *	   total number of characters written for a type that belongs
+ *	   to the IIO_VAL_... constant.
+ */
+ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
+{
+	ssize_t len;
+
+	len = __iio_format_value(buf, type, size, vals);
+
+	return len + sprintf(buf + len, "\n");
+}
 EXPORT_SYMBOL_GPL(iio_format_value);
 
 static ssize_t iio_read_channel_info(struct device *dev,
@@ -662,6 +673,113 @@ static ssize_t iio_read_channel_info(struct device *dev,
 	return iio_format_value(buf, ret, val_len, vals);
 }
 
+static ssize_t iio_format_avail_list(char *buf, const int *vals,
+				     int type, int length)
+{
+	int i;
+	ssize_t len = 0;
+
+	switch (type) {
+	case IIO_VAL_INT:
+		for (i = 0; i < length; i++) {
+			len += __iio_format_value(buf + len, type, 1, &vals[i]);
+			if (i < length - 1)
+				len += snprintf(buf + len,
+						PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len,
+						PAGE_SIZE - len,
+						"\n");
+		}
+		break;
+	default:
+		for (i = 0; i < length / 2; i++) {
+			len += __iio_format_value(buf + len,
+						  type,
+						  2,
+						  &vals[i * 2]);
+			if (i < length / 2 - 1)
+				len += snprintf(buf + len,
+						PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len,
+						PAGE_SIZE - len,
+						"\n");
+		}
+	};
+
+	return len;
+}
+
+static ssize_t iio_format_avail_range(char *buf, const int *vals, int type)
+{
+	int i;
+	ssize_t len;
+
+	len = snprintf(buf, PAGE_SIZE, "[");
+	switch (type) {
+	case IIO_VAL_INT:
+		for (i = 0; i < 3; i++) {
+			len += __iio_format_value(buf + len, type, 1, &vals[i]);
+			if (i < 2)
+				len += snprintf(buf + len,
+						PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len,
+						PAGE_SIZE - len,
+						"]\n");
+		}
+		break;
+	default:
+		for (i = 0; i < 3; i++) {
+			len += __iio_format_value(buf + len,
+						  type,
+						  2,
+						  &vals[i * 2]);
+			if (i < 2)
+				len += snprintf(buf + len,
+						PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len,
+						PAGE_SIZE - len,
+						"]\n");
+		}
+	};
+
+	return len;
+}
+
+static ssize_t iio_read_channel_info_avail(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+	const int *vals;
+	int ret;
+	int length;
+	int type;
+
+	ret = indio_dev->info->read_avail(indio_dev, this_attr->c,
+					  &vals, &type, &length,
+					  this_attr->address);
+
+	if (ret < 0)
+		return ret;
+	switch (ret) {
+	case IIO_AVAIL_LIST:
+		return iio_format_avail_list(buf, vals, type, length);
+	case IIO_AVAIL_RANGE:
+		return iio_format_avail_range(buf, vals, type);
+	default:
+		return -EINVAL;
+	}
+}
+
 /**
  * iio_str_to_fixpoint() - Parse a fixed-point number from a string
  * @str: The string to parse
@@ -978,6 +1096,40 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
 	return attrcount;
 }
 
+static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
+					       struct iio_chan_spec const *chan,
+					       enum iio_shared_by shared_by,
+					       const long *infomask)
+{
+	int i, ret, attrcount = 0;
+	char *avail_postfix;
+
+	for_each_set_bit(i, infomask, sizeof(infomask) * 8) {
+		avail_postfix = kasprintf(GFP_KERNEL,
+					  "%s_available",
+					  iio_chan_info_postfix[i]);
+		if (!avail_postfix)
+			return -ENOMEM;
+
+		ret = __iio_add_chan_devattr(avail_postfix,
+					     chan,
+					     &iio_read_channel_info_avail,
+					     NULL,
+					     i,
+					     shared_by,
+					     &indio_dev->dev,
+					     &indio_dev->channel_attr_list);
+		kfree(avail_postfix);
+		if ((ret == -EBUSY) && (shared_by != IIO_SEPARATE))
+			continue;
+		else if (ret < 0)
+			return ret;
+		attrcount++;
+	}
+
+	return attrcount;
+}
+
 static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 					struct iio_chan_spec const *chan)
 {
@@ -993,6 +1145,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 		return ret;
 	attrcount += ret;
 
+	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
+						  IIO_SEPARATE,
+						  &chan->
+						  info_mask_separate_available);
+	if (ret < 0)
+		return ret;
+	attrcount += ret;
+
 	ret = iio_device_add_info_mask_type(indio_dev, chan,
 					    IIO_SHARED_BY_TYPE,
 					    &chan->info_mask_shared_by_type);
@@ -1000,6 +1160,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 		return ret;
 	attrcount += ret;
 
+	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
+						  IIO_SHARED_BY_TYPE,
+						  &chan->
+						  info_mask_shared_by_type_available);
+	if (ret < 0)
+		return ret;
+	attrcount += ret;
+
 	ret = iio_device_add_info_mask_type(indio_dev, chan,
 					    IIO_SHARED_BY_DIR,
 					    &chan->info_mask_shared_by_dir);
@@ -1007,6 +1175,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 		return ret;
 	attrcount += ret;
 
+	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
+						  IIO_SHARED_BY_DIR,
+						  &chan->info_mask_shared_by_dir_available);
+	if (ret < 0)
+		return ret;
+	attrcount += ret;
+
 	ret = iio_device_add_info_mask_type(indio_dev, chan,
 					    IIO_SHARED_BY_ALL,
 					    &chan->info_mask_shared_by_all);
@@ -1014,6 +1189,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
 		return ret;
 	attrcount += ret;
 
+	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
+						  IIO_SHARED_BY_ALL,
+						  &chan->info_mask_shared_by_all_available);
+	if (ret < 0)
+		return ret;
+	attrcount += ret;
+
 	if (chan->ext_info) {
 		unsigned int i = 0;
 		for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index b4a0679e4a49..c0f897084620 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -269,9 +269,13 @@ struct iio_chan_spec {
 		enum iio_endian endianness;
 	} scan_type;
 	long			info_mask_separate;
+	long			info_mask_separate_available;
 	long			info_mask_shared_by_type;
+	long			info_mask_shared_by_type_available;
 	long			info_mask_shared_by_dir;
+	long			info_mask_shared_by_dir_available;
 	long			info_mask_shared_by_all;
+	long			info_mask_shared_by_all_available;
 	const struct iio_event_spec *event_spec;
 	unsigned int		num_event_specs;
 	const struct iio_chan_spec_ext_info *ext_info;
@@ -397,6 +401,13 @@ struct iio_info {
 			int *val_len,
 			long mask);
 
+	int (*read_avail)(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan,
+			  const int **vals,
+			  int *type,
+			  int *length,
+			  long mask_el);
+
 	int (*write_raw)(struct iio_dev *indio_dev,
 			 struct iio_chan_spec const *chan,
 			 int val,
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index 32b579525004..2aa7b6384d64 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -29,4 +29,9 @@ enum iio_event_info {
 #define IIO_VAL_FRACTIONAL 10
 #define IIO_VAL_FRACTIONAL_LOG2 11
 
+enum iio_available_type {
+	IIO_AVAIL_LIST,
+	IIO_AVAIL_RANGE,
+};
+
 #endif /* _IIO_TYPES_H_ */
-- 
2.1.4

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

* [PATCH v2 2/7] iio: mcp4531: provide range of available raw values
  2016-10-22 22:43 [PATCH v2 0/7] IIO wrapper drivers, dpot-dac and envelope-detector Peter Rosin
  2016-10-22 22:43 ` [PATCH v2 1/7] iio:core: add a callback to allow drivers to provide _available attributes Peter Rosin
@ 2016-10-22 22:43 ` Peter Rosin
  2016-10-23  9:36   ` Jonathan Cameron
  2016-10-22 22:43 ` [PATCH v2 3/7] dt-bindings: add axentia to vendor-prefixes Peter Rosin
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Peter Rosin @ 2016-10-22 22:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, linux-iio, devicetree

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/iio/potentiometer/mcp4531.c | 104 +++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/potentiometer/mcp4531.c b/drivers/iio/potentiometer/mcp4531.c
index 13b6ae2fcf7b..0d1bcf89ae17 100644
--- a/drivers/iio/potentiometer/mcp4531.c
+++ b/drivers/iio/potentiometer/mcp4531.c
@@ -38,7 +38,7 @@
 
 struct mcp4531_cfg {
 	int wipers;
-	int max_pos;
+	int avail[3];
 	int kohms;
 };
 
@@ -78,38 +78,38 @@ enum mcp4531_type {
 };
 
 static const struct mcp4531_cfg mcp4531_cfg[] = {
-	[MCP453x_502] = { .wipers = 1, .max_pos = 128, .kohms =   5, },
-	[MCP453x_103] = { .wipers = 1, .max_pos = 128, .kohms =  10, },
-	[MCP453x_503] = { .wipers = 1, .max_pos = 128, .kohms =  50, },
-	[MCP453x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
-	[MCP454x_502] = { .wipers = 1, .max_pos = 128, .kohms =   5, },
-	[MCP454x_103] = { .wipers = 1, .max_pos = 128, .kohms =  10, },
-	[MCP454x_503] = { .wipers = 1, .max_pos = 128, .kohms =  50, },
-	[MCP454x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
-	[MCP455x_502] = { .wipers = 1, .max_pos = 256, .kohms =   5, },
-	[MCP455x_103] = { .wipers = 1, .max_pos = 256, .kohms =  10, },
-	[MCP455x_503] = { .wipers = 1, .max_pos = 256, .kohms =  50, },
-	[MCP455x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
-	[MCP456x_502] = { .wipers = 1, .max_pos = 256, .kohms =   5, },
-	[MCP456x_103] = { .wipers = 1, .max_pos = 256, .kohms =  10, },
-	[MCP456x_503] = { .wipers = 1, .max_pos = 256, .kohms =  50, },
-	[MCP456x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
-	[MCP463x_502] = { .wipers = 2, .max_pos = 128, .kohms =   5, },
-	[MCP463x_103] = { .wipers = 2, .max_pos = 128, .kohms =  10, },
-	[MCP463x_503] = { .wipers = 2, .max_pos = 128, .kohms =  50, },
-	[MCP463x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
-	[MCP464x_502] = { .wipers = 2, .max_pos = 128, .kohms =   5, },
-	[MCP464x_103] = { .wipers = 2, .max_pos = 128, .kohms =  10, },
-	[MCP464x_503] = { .wipers = 2, .max_pos = 128, .kohms =  50, },
-	[MCP464x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
-	[MCP465x_502] = { .wipers = 2, .max_pos = 256, .kohms =   5, },
-	[MCP465x_103] = { .wipers = 2, .max_pos = 256, .kohms =  10, },
-	[MCP465x_503] = { .wipers = 2, .max_pos = 256, .kohms =  50, },
-	[MCP465x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
-	[MCP466x_502] = { .wipers = 2, .max_pos = 256, .kohms =   5, },
-	[MCP466x_103] = { .wipers = 2, .max_pos = 256, .kohms =  10, },
-	[MCP466x_503] = { .wipers = 2, .max_pos = 256, .kohms =  50, },
-	[MCP466x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
+	[MCP453x_502] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =   5, },
+	[MCP453x_103] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  10, },
+	[MCP453x_503] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  50, },
+	[MCP453x_104] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 100, },
+	[MCP454x_502] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =   5, },
+	[MCP454x_103] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  10, },
+	[MCP454x_503] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  50, },
+	[MCP454x_104] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 100, },
+	[MCP455x_502] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =   5, },
+	[MCP455x_103] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  10, },
+	[MCP455x_503] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  50, },
+	[MCP455x_104] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 100, },
+	[MCP456x_502] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =   5, },
+	[MCP456x_103] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  10, },
+	[MCP456x_503] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  50, },
+	[MCP456x_104] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 100, },
+	[MCP463x_502] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =   5, },
+	[MCP463x_103] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  10, },
+	[MCP463x_503] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  50, },
+	[MCP463x_104] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 100, },
+	[MCP464x_502] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =   5, },
+	[MCP464x_103] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  10, },
+	[MCP464x_503] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  50, },
+	[MCP464x_104] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 100, },
+	[MCP465x_502] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =   5, },
+	[MCP465x_103] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  10, },
+	[MCP465x_503] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  50, },
+	[MCP465x_104] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 100, },
+	[MCP466x_502] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =   5, },
+	[MCP466x_103] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  10, },
+	[MCP466x_503] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  50, },
+	[MCP466x_104] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 100, },
 };
 
 #define MCP4531_WRITE (0 << 2)
@@ -124,13 +124,14 @@ struct mcp4531_data {
 	const struct mcp4531_cfg *cfg;
 };
 
-#define MCP4531_CHANNEL(ch) {					\
-	.type = IIO_RESISTANCE,					\
-	.indexed = 1,						\
-	.output = 1,						\
-	.channel = (ch),					\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+#define MCP4531_CHANNEL(ch) {						\
+	.type = IIO_RESISTANCE,						\
+	.indexed = 1,							\
+	.output = 1,							\
+	.channel = (ch),						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_RAW),	\
 }
 
 static const struct iio_chan_spec mcp4531_channels[] = {
@@ -156,13 +157,31 @@ static int mcp4531_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		*val = 1000 * data->cfg->kohms;
-		*val2 = data->cfg->max_pos;
+		*val2 = data->cfg->avail[2];
 		return IIO_VAL_FRACTIONAL;
 	}
 
 	return -EINVAL;
 }
 
+static int mcp4531_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	struct mcp4531_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*length = ARRAY_SIZE(data->cfg->avail);
+		*vals = data->cfg->avail;
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_RANGE;
+	}
+
+	return -EINVAL;
+}
+
 static int mcp4531_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
@@ -172,7 +191,7 @@ static int mcp4531_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		if (val > data->cfg->max_pos || val < 0)
+		if (val > data->cfg->avail[2] || val < 0)
 			return -EINVAL;
 		break;
 	default:
@@ -186,6 +205,7 @@ static int mcp4531_write_raw(struct iio_dev *indio_dev,
 
 static const struct iio_info mcp4531_info = {
 	.read_raw = mcp4531_read_raw,
+	.read_avail = mcp4531_read_avail,
 	.write_raw = mcp4531_write_raw,
 	.driver_module = THIS_MODULE,
 };
-- 
2.1.4

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

* [PATCH v2 3/7] dt-bindings: add axentia to vendor-prefixes
  2016-10-22 22:43 [PATCH v2 0/7] IIO wrapper drivers, dpot-dac and envelope-detector Peter Rosin
  2016-10-22 22:43 ` [PATCH v2 1/7] iio:core: add a callback to allow drivers to provide _available attributes Peter Rosin
  2016-10-22 22:43 ` [PATCH v2 2/7] iio: mcp4531: provide range of available raw values Peter Rosin
@ 2016-10-22 22:43 ` Peter Rosin
  2016-10-22 22:43 ` [PATCH v2 4/7] dt-bindings: iio: document dpot-dac bindings Peter Rosin
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Peter Rosin @ 2016-10-22 22:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, linux-iio, devicetree

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index f0a48ea78659..a437120a7eee 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -39,6 +39,7 @@ auo	AU Optronics Corporation
 auvidea Auvidea GmbH
 avago	Avago Technologies
 avic	Shanghai AVIC Optoelectronics Co., Ltd.
+axentia	Axentia Technologies AB
 axis	Axis Communications AB
 boe	BOE Technology Group Co., Ltd.
 bosch	Bosch Sensortec GmbH
-- 
2.1.4

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

* [PATCH v2 4/7] dt-bindings: iio: document dpot-dac bindings
  2016-10-22 22:43 [PATCH v2 0/7] IIO wrapper drivers, dpot-dac and envelope-detector Peter Rosin
                   ` (2 preceding siblings ...)
  2016-10-22 22:43 ` [PATCH v2 3/7] dt-bindings: add axentia to vendor-prefixes Peter Rosin
@ 2016-10-22 22:43 ` Peter Rosin
  2016-10-23  8:02   ` Peter Meerwald-Stadler
  2016-10-22 22:43 ` [PATCH v2 5/7] iio: dpot-dac: DAC driver based on a digital potentiometer Peter Rosin
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Peter Rosin @ 2016-10-22 22:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, linux-iio, devicetree

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/iio/dac/dpot-dac.txt       | 41 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 ++++
 2 files changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/dpot-dac.txt

diff --git a/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt b/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
new file mode 100644
index 000000000000..4fd5d63cc2d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
@@ -0,0 +1,41 @@
+Bindings for DAC emulation using a digital potentiometer
+
+It is assumed the that the dpot is used as a voltage divider between the
+current dpot wiper setting and the maximum resistance of the dpot. The
+divided voltage is provided by a vref regulator.
+
+                  .------.
+   .-----------.  |      |
+   | vref      |--'    .---.
+   | regulator |--.    |   |
+   '-----------'  |    | d |
+                  |    | p |
+                  |    | o |  wiper
+                  |    | t |<---------+
+                  |    |   |
+                  |    '---'       dac output voltage
+                  |      |
+                  '------+------------+
+
+Required properties:
+- compatible: Should be "dpot-dac"
+- vref-supply: The regulator supplying the voltage divider.
+- io-channels: Channel node of the dpot to be used for the voltage division.
+- io-channel-names: Should be "dpot".
+
+Example:
+
+	&i2c {
+		dpot: mcp4651-503@28 {
+			compatible = "microchip,mcp4651-503";
+			reg = <0x28>;
+			#io-channel-cells = <1>;
+		};
+	};
+
+	dac {
+		compatible = "dpot-dac";
+		vref-supply = <&reg_3v3>;
+		io-channels = <&dpot 0>;
+		io-channel-names = "dpot";
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 1cd38a7e0064..c68b72088945 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6111,6 +6111,12 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	drivers/media/rc/iguanair.c
 
+IIO DIGITAL POTENTIOMETER DAC
+M:	Peter Rosin <peda@axentia.se>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
+
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23@kernel.org>
 R:	Hartmut Knaack <knaack.h@gmx.de>
-- 
2.1.4

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

* [PATCH v2 5/7] iio: dpot-dac: DAC driver based on a digital potentiometer
  2016-10-22 22:43 [PATCH v2 0/7] IIO wrapper drivers, dpot-dac and envelope-detector Peter Rosin
                   ` (3 preceding siblings ...)
  2016-10-22 22:43 ` [PATCH v2 4/7] dt-bindings: iio: document dpot-dac bindings Peter Rosin
@ 2016-10-22 22:43 ` Peter Rosin
  2016-10-23  9:42   ` Jonathan Cameron
  2016-10-22 22:43 ` [PATCH v2 6/7] dt-bindings: iio: document envelope-detector bindings Peter Rosin
  2016-10-22 22:43 ` [PATCH v2 7/7] iio: envelope-detector: ADC driver based on a DAC and a comparator Peter Rosin
  6 siblings, 1 reply; 15+ messages in thread
From: Peter Rosin @ 2016-10-22 22:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, linux-iio, devicetree

It is assumed the that the dpot is used as a voltage divider between the
current dpot wiper setting and the maximum resistance of the dpot. The
divided voltage is provided by a vref regulator.

                  .------.
   .-----------.  |      |
   | vref      |--'    .---.
   | regulator |--.    |   |
   '-----------'  |    | d |
                  |    | p |
                  |    | o |  wiper
                  |    | t |<---------+
                  |    |   |
                  |    '---'       dac output voltage
                  |      |
                  '------+------------+

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 MAINTAINERS                |   1 +
 drivers/iio/dac/Kconfig    |  10 ++
 drivers/iio/dac/Makefile   |   1 +
 drivers/iio/dac/dpot-dac.c | 298 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 310 insertions(+)
 create mode 100644 drivers/iio/dac/dpot-dac.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c68b72088945..8c8aae24b96b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6116,6 +6116,7 @@ M:	Peter Rosin <peda@axentia.se>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
+F:	drivers/iio/dac/dpot-dac.c
 
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23@kernel.org>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 120b24478469..d3084028905b 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -200,6 +200,16 @@ config AD8801
 	  To compile this driver as a module choose M here: the module will be called
 	  ad8801.
 
+config DPOT_DAC
+	tristate "DAC emulation using a DPOT"
+	depends on OF
+	help
+	  Say yes here to build support for DAC emulation using a digital
+	  potentiometer.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called dpot-dac.
+
 config LPC18XX_DAC
 	tristate "NXP LPC18xx DAC driver"
 	depends on ARCH_LPC18XX || COMPILE_TEST
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 27642bbf75f2..f01bf4a99867 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_AD5686) += ad5686.o
 obj-$(CONFIG_AD7303) += ad7303.o
 obj-$(CONFIG_AD8801) += ad8801.o
 obj-$(CONFIG_CIO_DAC) += cio-dac.o
+obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
 obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
 obj-$(CONFIG_M62332) += m62332.o
 obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/dpot-dac.c b/drivers/iio/dac/dpot-dac.c
new file mode 100644
index 000000000000..5613eae32347
--- /dev/null
+++ b/drivers/iio/dac/dpot-dac.c
@@ -0,0 +1,298 @@
+/*
+ * IIO DAC emulation driver using a digital potentiometer
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * It is assumed that the dpot is used as a voltage divider between the
+ * current dpot wiper setting and the maximum resistance of the dpot. The
+ * divided voltage is provided by a vref regulator.
+ *
+ *                   .------.
+ *    .-----------.  |      |
+ *    | vref      |--'    .---.
+ *    | regulator |--.    |   |
+ *    '-----------'  |    | d |
+ *                   |    | p |
+ *                   |    | o |  wiper
+ *                   |    | t |<---------+
+ *                   |    |   |
+ *                   |    '---'       dac output voltage
+ *                   |      |
+ *                   '------+------------+
+ */
+
+#include <linux/err.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+struct dpot_dac {
+	struct regulator *vref;
+	struct iio_channel *dpot;
+	u32 max_ohms;
+};
+
+static const struct iio_chan_spec dpot_dac_iio_channel = {
+	.type = IIO_VOLTAGE,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+			    | BIT(IIO_CHAN_INFO_SCALE),
+	.info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW),
+	.output = 1,
+	.indexed = 1,
+};
+
+static int dpot_dac_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct dpot_dac *dac = iio_priv(indio_dev);
+	int ret;
+	unsigned long long tmp;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		return iio_read_channel_raw(dac->dpot, val);
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = iio_read_channel_scale(dac->dpot, val, val2);
+		switch (ret) {
+		case IIO_VAL_FRACTIONAL_LOG2:
+			tmp = *val * 1000000000LL;
+			do_div(tmp, dac->max_ohms);
+			tmp *= regulator_get_voltage(dac->vref) / 1000;
+			do_div(tmp, 1000000000LL);
+			*val = tmp;
+			return ret;
+		case IIO_VAL_INT:
+			/*
+			 * Convert integer scale to fractional scale by
+			 * setting the denominator (val2) to one...
+			 */
+			*val2 = 1;
+			ret = IIO_VAL_FRACTIONAL;
+			/* ...and fall through. */
+		case IIO_VAL_FRACTIONAL:
+			*val *= regulator_get_voltage(dac->vref) / 1000;
+			*val2 *= dac->max_ohms;
+			break;
+		}
+
+		return ret;
+	}
+
+	return -EINVAL;
+}
+
+static int dpot_dac_read_avail(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       const int **vals, int *type, int *length,
+			       long mask)
+{
+	struct dpot_dac *dac = iio_priv(indio_dev);
+	struct iio_dev *dpot_dev = dac->dpot->indio_dev;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		return dpot_dev->info->read_avail(dpot_dev, dac->dpot->channel,
+						  vals, type, length, mask);
+	}
+
+	return -EINVAL;
+}
+
+static int dpot_dac_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
+{
+	struct dpot_dac *dac = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		return iio_write_channel_raw(dac->dpot, val);
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info dpot_dac_info = {
+	.read_raw = dpot_dac_read_raw,
+	.read_avail = dpot_dac_read_avail,
+	.write_raw = dpot_dac_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int dpot_dac_channel_max_ohms(struct iio_dev *indio_dev)
+{
+	struct device *dev = &indio_dev->dev;
+	struct dpot_dac *dac = iio_priv(indio_dev);
+	struct iio_channel *ch = dac->dpot;
+	struct iio_dev *iio_ch_dev = ch->indio_dev;
+	const int *vals;
+	unsigned long long tmp;
+	int type;
+	int len;
+	int ret;
+	int val1;
+	int val2;
+	int max;
+
+	if (!iio_ch_dev->info->read_avail) {
+		dev_err(dev, "dpot does not provide available raw values\n");
+		return -EINVAL;
+	}
+
+	ret = iio_ch_dev->info->read_avail(iio_ch_dev, ch->channel,
+					   &vals, &type, &len,
+					   IIO_CHAN_INFO_RAW);
+
+	if (ret < 0) {
+		dev_err(dev, "dpot failed to provide available raw values\n");
+		return ret;
+	}
+	if (type != IIO_VAL_INT) {
+		dev_err(dev, "dpot provides strange raw values\n");
+		return -EINVAL;
+	}
+
+	switch (ret) {
+	case IIO_AVAIL_RANGE:
+		if (len != 3) {
+			dev_err(dev, "need a range of available values\n");
+			return -EINVAL;
+		}
+		max = vals[2];
+		break;
+	default:
+		dev_err(dev, "dpot doesn't provide range of available values\n");
+		return -EINVAL;
+	}
+
+	switch (iio_read_channel_scale(dac->dpot, &val1, &val2)) {
+	case IIO_VAL_INT:
+		return max * val1;
+	case IIO_VAL_FRACTIONAL:
+		tmp = (unsigned long long)max * val1;
+		do_div(tmp, val2);
+		return tmp;
+	case IIO_VAL_FRACTIONAL_LOG2:
+		tmp = (s64)vals[0] * 1000000000LL * max >> vals[1];
+		do_div(tmp, 1000000000LL);
+		return tmp;
+	default:
+		dev_err(dev, "dpot has a scale that is too weird\n");
+	}
+
+	return -EINVAL;
+}
+
+static int dpot_dac_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iio_dev *indio_dev;
+	struct dpot_dac *dac;
+	enum iio_chan_type type;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*dac));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, indio_dev);
+	dac = iio_priv(indio_dev);
+
+	indio_dev->name = dev_name(dev);
+	indio_dev->dev.parent = dev;
+	indio_dev->info = &dpot_dac_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = &dpot_dac_iio_channel;
+	indio_dev->num_channels = 1;
+
+	dac->vref = devm_regulator_get(dev, "vref");
+	if (IS_ERR(dac->vref)) {
+		if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "failed to get vref regulator\n");
+		return PTR_ERR(dac->vref);
+	}
+
+	dac->dpot = devm_iio_channel_get(dev, "dpot");
+	if (IS_ERR(dac->dpot)) {
+		if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get dpot input channel\n");
+		return PTR_ERR(dac->dpot);
+	}
+
+	ret = iio_get_channel_type(dac->dpot, &type);
+	if (ret < 0)
+		return ret;
+
+	if (type != IIO_RESISTANCE) {
+		dev_err(dev, "dpot is of the wrong type\n");
+		return -EINVAL;
+	}
+
+	ret = dpot_dac_channel_max_ohms(indio_dev);
+	if (ret < 0)
+		return ret;
+	dac->max_ohms = ret;
+	dev_info(dev, "dpot max is %d\n", dac->max_ohms);
+
+	ret = regulator_enable(dac->vref);
+	if (ret) {
+		dev_err(dev, "failed to enable the vref regulator\n");
+		return ret;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "failed to register iio device\n");
+		goto disable_reg;
+	}
+
+	return 0;
+
+disable_reg:
+	regulator_disable(dac->vref);
+	return ret;
+}
+
+static int dpot_dac_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct dpot_dac *dac = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	regulator_disable(dac->vref);
+
+	return 0;
+}
+
+static const struct of_device_id dpot_dac_match[] = {
+	{ .compatible = "dpot-dac" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, dpot_dac_match);
+
+static struct platform_driver dpot_dac_driver = {
+	.probe = dpot_dac_probe,
+	.remove = dpot_dac_remove,
+	.driver = {
+		.name = "iio-dpot-dac",
+		.of_match_table = dpot_dac_match,
+	},
+};
+module_platform_driver(dpot_dac_driver);
+
+MODULE_DESCRIPTION("DAC emulation driver using a digital potentiometer");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* [PATCH v2 6/7] dt-bindings: iio: document envelope-detector bindings
  2016-10-22 22:43 [PATCH v2 0/7] IIO wrapper drivers, dpot-dac and envelope-detector Peter Rosin
                   ` (4 preceding siblings ...)
  2016-10-22 22:43 ` [PATCH v2 5/7] iio: dpot-dac: DAC driver based on a digital potentiometer Peter Rosin
@ 2016-10-22 22:43 ` Peter Rosin
  2016-10-23  9:01   ` Peter Rosin
  2016-10-22 22:43 ` [PATCH v2 7/7] iio: envelope-detector: ADC driver based on a DAC and a comparator Peter Rosin
  6 siblings, 1 reply; 15+ messages in thread
From: Peter Rosin @ 2016-10-22 22:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, linux-iio, devicetree

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../bindings/iio/adc/envelope-detector.txt         | 55 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/envelope-detector.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
new file mode 100644
index 000000000000..9099531cffff
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
@@ -0,0 +1,55 @@
+Bindings for ADC envelope detector using a DAC and a comparator
+
+The DAC is used to find the peak level of an alternating voltage input
+signal by a binary search using the output of a comparator wired to
+an interrupt pin. Like so:
+                          _
+                         | \
+    input +------>-------|+ \
+                         |   \
+           .-------.     |    }---.
+           |       |     |   /    |
+           |    dac|-->--|- /     |
+           |       |     |_/      |
+           |       |              |
+           |       |              |
+           |    irq|------<-------'
+           |       |
+           '-------'
+
+Required properties:
+- compatible: Should be "axentia,tse850-envelope-detector"
+- io-channels: Channel node of the dac to be used for comparator input.
+- io-channel-names: Should be "dac".
+- interrupt specification for one client interrupt,
+  see ../../interrupt-controller/interrupts.txt for details.
+- interrupt-names: Should be "comp".
+
+Example:
+
+	&i2c {
+		dpot: mcp4651-104@28 {
+			compatible = "microchip,mcp4651-104";
+			reg = <0x28>;
+			#io-channel-cells = <1>;
+		};
+	};
+
+	dac: dac {
+		compatible = "dpot-dac";
+		vref-supply = <&reg_3v3>;
+		io-channels = <&dpot 0>;
+		io-channel-names = "dpot";
+		dpot-dac,max-ohms = <100000>;
+		#io-channel-cells = <1>;
+	};
+
+	envelope-detector {
+		compatible = "axentia,tse850-envelope-detector";
+		io-channels = <&dac 0>;
+		io-channel-names = "dac";
+
+		interrupt-parent = <&gpio>;
+		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+		interrupt-names = "comp";
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 8c8aae24b96b..4b6f6ec1b703 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6118,6 +6118,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
 F:	drivers/iio/dac/dpot-dac.c
 
+IIO ENVELOPE DETECTOR
+M:	Peter Rosin <peda@axentia.se>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
+
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23@kernel.org>
 R:	Hartmut Knaack <knaack.h@gmx.de>
-- 
2.1.4

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

* [PATCH v2 7/7] iio: envelope-detector: ADC driver based on a DAC and a comparator
  2016-10-22 22:43 [PATCH v2 0/7] IIO wrapper drivers, dpot-dac and envelope-detector Peter Rosin
                   ` (5 preceding siblings ...)
  2016-10-22 22:43 ` [PATCH v2 6/7] dt-bindings: iio: document envelope-detector bindings Peter Rosin
@ 2016-10-22 22:43 ` Peter Rosin
  2016-10-23  9:48   ` Jonathan Cameron
  6 siblings, 1 reply; 15+ messages in thread
From: Peter Rosin @ 2016-10-22 22:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, linux-iio, devicetree

The DAC is used to find the peak level of an alternating voltage input
signal by a binary search using the output of a comparator wired to
an interrupt pin. Like so:
                          _
                         | \
    input +------>-------|+ \
                         |   \
           .-------.     |    }---.
           |       |     |   /    |
           |    dac|-->--|- /     |
           |       |     |_/      |
           |       |              |
           |       |              |
           |    irq|------<-------'
           |       |
           '-------'

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../testing/sysfs-bus-iio-adc-envelope-detector    |  36 ++
 MAINTAINERS                                        |   2 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/envelope-detector.c                | 463 +++++++++++++++++++++
 5 files changed, 512 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
 create mode 100644 drivers/iio/adc/envelope-detector.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector b/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
new file mode 100644
index 000000000000..8d890028a649
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
@@ -0,0 +1,36 @@
+What:		/sys/bus/iio/devices/iio:deviceX/invert
+Date:		October 2016
+KernelVersion:	4.9
+Contact:	Peter Rosin <peda@axentia.se>
+Description:
+		The DAC is used to find the peak level of an alternating
+		voltage input signal by a binary search using the output
+		of a comparator wired to an interrupt pin. Like so:
+		                           _
+		                          | \
+		     input +------>-------|+ \
+		                          |   \
+		            .-------.     |    }---.
+		            |       |     |   /    |
+		            |    dac|-->--|- /     |
+		            |       |     |_/      |
+		            |       |              |
+		            |       |              |
+		            |    irq|------<-------'
+		            |       |
+		            '-------'
+		The boolean invert attribute (0/1) should be set when the
+		input signal is centered around the maximum value of the
+		dac instead of zero. The envelope detector will search
+		from below in this case and will also invert the result.
+		The edge/level of the interrupt is also switched to its
+		opposite value.
+
+What:		/sys/bus/iio/devices/iio:deviceX/compare_interval_ms
+Date:		October 2016
+KernelVersion:	4.9
+Contact:	Peter Rosin <peda@axentia.se>
+Description:
+		Number of milliseconds to wait for the comparator in each
+		step of the binary search for the input peak level. Needs
+		to relate to the frequency of the input signal.
diff --git a/MAINTAINERS b/MAINTAINERS
index 4b6f6ec1b703..0d20f3561700 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6122,7 +6122,9 @@ IIO ENVELOPE DETECTOR
 M:	Peter Rosin <peda@axentia.se>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
 F:	Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
+F:	drivers/iio/adc/envelope-detector.c
 
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23@kernel.org>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7edcf3238620..d5c4a95855c2 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -195,6 +195,16 @@ config DA9150_GPADC
 	  To compile this driver as a module, choose M here: the module will be
 	  called berlin2-adc.
 
+config ENVELOPE_DETECTOR
+	tristate "Envelope detector using a DAC and a comparator"
+	depends on OF
+	help
+	  Say yes here to build support for an envelope detector using a DAC
+	  and a comparator.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called iio-envelope-detector.
+
 config EXYNOS_ADC
 	tristate "Exynos ADC driver support"
 	depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || (OF && COMPILE_TEST)
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7a40c04c311f..0d773c6a0578 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_BCM_IPROC_ADC) += bcm_iproc_adc.o
 obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
 obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
 obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
+obj-$(CONFIG_ENVELOPE_DETECTOR) += envelope-detector.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
 obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
 obj-$(CONFIG_HI8435) += hi8435.o
diff --git a/drivers/iio/adc/envelope-detector.c b/drivers/iio/adc/envelope-detector.c
new file mode 100644
index 000000000000..3842d1a72023
--- /dev/null
+++ b/drivers/iio/adc/envelope-detector.c
@@ -0,0 +1,463 @@
+/*
+ * Driver for an envelope detector using a DAC and a comparator
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * The DAC is used to find the peak level of an alternating voltage input
+ * signal by a binary search using the output of a comparator wired to
+ * an interrupt pin. Like so:
+ *                           _
+ *                          | \
+ *     input +------>-------|+ \
+ *                          |   \
+ *            .-------.     |    }---.
+ *            |       |     |   /    |
+ *            |    dac|-->--|- /     |
+ *            |       |     |_/      |
+ *            |       |              |
+ *            |       |              |
+ *            |    irq|------<-------'
+ *            |       |
+ *            '-------'
+ */
+
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+
+struct envelope {
+	spinlock_t comp_lock; /* protects comp */
+	int comp;
+
+	struct mutex read_lock; /* protects everything else */
+
+	int comp_irq;
+	u32 comp_irq_trigger;
+	u32 comp_irq_trigger_inv;
+
+	struct iio_channel *dac;
+	struct delayed_work comp_timeout;
+
+	unsigned int comp_interval;
+	bool invert;
+	u32 dac_max;
+
+	int high;
+	int level;
+	int low;
+
+	struct completion done;
+};
+
+/*
+ * The envelope_detector_comp_latch function works together with the compare
+ * interrupt service routine below (envelope_detector_comp_isr) as a latch
+ * (one-bit memory) for if the interrupt has triggered since last calling
+ * this function.
+ * The ..._comp_isr function disables the interrupt so that the cpu does not
+ * need to service a possible interrupt flood from the comparator when no-one
+ * cares anyway, and this ..._comp_latch function reenables them again if
+ * needed.
+ */
+static int envelope_detector_comp_latch(struct envelope *env)
+{
+	int comp;
+
+	spin_lock_irq(&env->comp_lock);
+	comp = env->comp;
+	env->comp = 0;
+	spin_unlock_irq(&env->comp_lock);
+
+	if (!comp)
+		return 0;
+
+	/*
+	 * The irq was disabled, and is reenabled just now.
+	 * But there might have been a pending irq that
+	 * happened while the irq was disabled that fires
+	 * just as the irq is reenabled. That is not what
+	 * is desired.
+	 */
+	enable_irq(env->comp_irq);
+
+	/* So, synchronize this possibly pending irq... */
+	synchronize_irq(env->comp_irq);
+
+	/* ...and redo the whole dance. */
+	spin_lock_irq(&env->comp_lock);
+	comp = env->comp;
+	env->comp = 0;
+	spin_unlock_irq(&env->comp_lock);
+
+	if (comp)
+		enable_irq(env->comp_irq);
+
+	return 1;
+}
+
+static irqreturn_t envelope_detector_comp_isr(int irq, void *ctx)
+{
+	struct envelope *env = ctx;
+
+	spin_lock(&env->comp_lock);
+	env->comp = 1;
+	disable_irq_nosync(env->comp_irq);
+	spin_unlock(&env->comp_lock);
+
+	return IRQ_HANDLED;
+}
+
+static void envelope_detector_setup_compare(struct envelope *env)
+{
+	int ret;
+
+	/*
+	 * Do a binary search for the peak input level, and stop
+	 * when that level is "trapped" between two adjacent DAC
+	 * values.
+	 * When invert is active, use the midpoint floor so that
+	 * env->level ends up as env->low when the termination
+	 * criteria below is fulfilled, and use the midpoint
+	 * ceiling when invert is not active so that env->level
+	 * ends up as env->high in that case.
+	 */
+	env->level = (env->high + env->low + !env->invert) / 2;
+
+	if (env->high == env->low + 1) {
+		complete(&env->done);
+		return;
+	}
+
+	/* Set a "safe" DAC level (if there is such a thing)... */
+	ret = iio_write_channel_raw(env->dac, env->invert ? 0 : env->dac_max);
+	if (ret < 0)
+		goto err;
+
+	/* ...clear the comparison result... */
+	envelope_detector_comp_latch(env);
+
+	/* ...set the real DAC level... */
+	ret = iio_write_channel_raw(env->dac, env->level);
+	if (ret < 0)
+		goto err;
+
+	/* ...and wait for a bit to see if the latch catches anything. */
+	schedule_delayed_work(&env->comp_timeout,
+			      msecs_to_jiffies(env->comp_interval));
+	return;
+
+err:
+	env->level = ret;
+	complete(&env->done);
+}
+
+static void envelope_detector_timeout(struct work_struct *work)
+{
+	struct envelope *env = container_of(work, struct envelope,
+					    comp_timeout.work);
+
+	/* Adjust low/high depending on the latch content... */
+	if (!envelope_detector_comp_latch(env) ^ !env->invert)
+		env->low = env->level;
+	else
+		env->high = env->level;
+
+	/* ...and continue the search. */
+	envelope_detector_setup_compare(env);
+}
+
+static int envelope_detector_read_raw(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan,
+				      int *val, int *val2, long mask)
+{
+	struct envelope *env = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		/*
+		 * When invert is active, start with high=max+1 and low=0
+		 * since we will end up with the low value when the
+		 * termination criteria is fulfilled (rounding down). And
+		 * start with high=max and low=-1 when invert is not active
+		 * since we will end up with the high value in that case.
+		 * This ensures that the returned value in both cases are
+		 * in the same range as the DAC and is a value that has not
+		 * triggered the comparator.
+		 */
+		mutex_lock(&env->read_lock);
+		env->high = env->dac_max + env->invert;
+		env->low = -1 + env->invert;
+		envelope_detector_setup_compare(env);
+		wait_for_completion(&env->done);
+		if (env->level < 0) {
+			ret = env->level;
+			goto err_unlock;
+		}
+		*val = env->invert ? env->dac_max - env->level : env->level;
+		mutex_unlock(&env->read_lock);
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		return iio_read_channel_scale(env->dac, val, val2);
+	}
+
+	return -EINVAL;
+
+err_unlock:
+	mutex_unlock(&env->read_lock);
+	return ret;
+}
+
+static int envelope_detector_channel_raw_max(struct iio_channel *ch)
+{
+	struct iio_dev *indio_dev = ch->indio_dev;
+	const int *vals;
+	int type;
+	int len;
+	int ret;
+
+	if (!ch->indio_dev->info->read_avail)
+		return -EINVAL;
+
+	ret = ch->indio_dev->info->read_avail(indio_dev, ch->channel,
+					      &vals, &type, &len,
+					      IIO_CHAN_INFO_RAW);
+
+	if (ret < 0)
+		return ret;
+	if (type != IIO_VAL_INT)
+		return -EINVAL;
+
+	switch (ret) {
+	case IIO_AVAIL_RANGE:
+		if (len != 3)
+			return -EINVAL;
+		return vals[2];
+	}
+	return -EINVAL;
+}
+
+static const struct iio_chan_spec envelope_detector_iio_channel = {
+	.type = IIO_ALTVOLTAGE,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+			    | BIT(IIO_CHAN_INFO_SCALE),
+	.indexed = 1,
+};
+
+static ssize_t envelope_show_invert(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct envelope *env = iio_priv(indio_dev);
+
+	return sprintf(buf, "%u\n", env->invert);
+}
+
+static ssize_t envelope_store_invert(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf,
+				     size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct envelope *env = iio_priv(indio_dev);
+	unsigned long invert;
+	int ret;
+	u32 trigger;
+
+	ret = kstrtoul(buf, 0, &invert);
+	if (ret < 0)
+		return ret;
+	if (invert > 1)
+		return -EINVAL;
+
+	trigger = invert ? env->comp_irq_trigger_inv : env->comp_irq_trigger;
+
+	mutex_lock(&env->read_lock);
+	if (invert != env->invert)
+		ret = irq_set_irq_type(env->comp_irq, trigger);
+	if (!ret) {
+		env->invert = invert;
+		ret = len;
+	}
+	mutex_unlock(&env->read_lock);
+
+	return ret;
+}
+
+static IIO_DEVICE_ATTR(invert, 0644,
+		       envelope_show_invert,
+		       envelope_store_invert, 0);
+
+static ssize_t envelope_show_comp_interval(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct envelope *env = iio_priv(indio_dev);
+
+	return sprintf(buf, "%u\n", env->comp_interval);
+}
+
+static ssize_t envelope_store_comp_interval(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf,
+					    size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct envelope *env = iio_priv(indio_dev);
+	unsigned long interval;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &interval);
+	if (ret < 0)
+		return ret;
+	if (interval > 1000)
+		return -EINVAL;
+
+	mutex_lock(&env->read_lock);
+	env->comp_interval = interval;
+	mutex_unlock(&env->read_lock);
+
+	return len;
+}
+
+static IIO_DEVICE_ATTR(compare_interval_ms, 0644,
+		       envelope_show_comp_interval,
+		       envelope_store_comp_interval, 0);
+
+static struct attribute *envelope_detector_attributes[] = {
+	&iio_dev_attr_invert.dev_attr.attr,
+	&iio_dev_attr_compare_interval_ms.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group envelope_detector_attribute_group = {
+	.attrs = envelope_detector_attributes,
+};
+
+static const struct iio_info envelope_detector_info = {
+	.read_raw = &envelope_detector_read_raw,
+	.driver_module = THIS_MODULE,
+	.attrs = &envelope_detector_attribute_group,
+};
+
+static int envelope_detector_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iio_dev *indio_dev;
+	struct envelope *env;
+	enum iio_chan_type type;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*env));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, indio_dev);
+	env = iio_priv(indio_dev);
+	env->comp_interval = 50; /* some sensible default? */
+
+	spin_lock_init(&env->comp_lock);
+	mutex_init(&env->read_lock);
+	init_completion(&env->done);
+	INIT_DELAYED_WORK(&env->comp_timeout, envelope_detector_timeout);
+
+	indio_dev->name = dev_name(dev);
+	indio_dev->dev.parent = dev;
+	indio_dev->dev.of_node = dev->of_node;
+	indio_dev->info = &envelope_detector_info;
+	indio_dev->channels = &envelope_detector_iio_channel;
+	indio_dev->num_channels = 1;
+
+	env->dac = devm_iio_channel_get(dev, "dac");
+	if (IS_ERR(env->dac)) {
+		if (PTR_ERR(env->dac) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get dac input channel\n");
+		return PTR_ERR(env->dac);
+	}
+
+	env->comp_irq = platform_get_irq_byname(pdev, "comp");
+	if (env->comp_irq < 0) {
+		if (env->comp_irq != -EPROBE_DEFER)
+			dev_err(dev, "failed to get compare interrupt\n");
+		return env->comp_irq;
+	}
+
+	ret = devm_request_irq(dev, env->comp_irq, envelope_detector_comp_isr,
+			       0, "envelope-detector", env);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to request interrupt\n");
+		return ret;
+	}
+	env->comp_irq_trigger = irq_get_trigger_type(env->comp_irq);
+	if (env->comp_irq_trigger & IRQF_TRIGGER_RISING)
+		env->comp_irq_trigger_inv |= IRQF_TRIGGER_FALLING;
+	if (env->comp_irq_trigger & IRQF_TRIGGER_FALLING)
+		env->comp_irq_trigger_inv |= IRQF_TRIGGER_RISING;
+	if (env->comp_irq_trigger & IRQF_TRIGGER_HIGH)
+		env->comp_irq_trigger_inv |= IRQF_TRIGGER_LOW;
+	if (env->comp_irq_trigger & IRQF_TRIGGER_LOW)
+		env->comp_irq_trigger_inv |= IRQF_TRIGGER_HIGH;
+
+	ret = iio_get_channel_type(env->dac, &type);
+	if (ret < 0)
+		return ret;
+
+	if (type != IIO_VOLTAGE) {
+		dev_err(dev, "dac is of the wrong type\n");
+		return -EINVAL;
+	}
+
+	ret = envelope_detector_channel_raw_max(env->dac);
+	if (ret < 0) {
+		dev_err(dev, "dac does not indicate its raw maximum value\n");
+		return ret;
+	}
+	env->dac_max = ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id envelope_detector_match[] = {
+	{ .compatible = "axentia,tse850-envelope-detector", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, envelope_detector_match);
+
+static struct platform_driver envelope_detector_driver = {
+	.probe = envelope_detector_probe,
+	.driver = {
+		.name = "iio-envelope-detector",
+		.of_match_table = envelope_detector_match,
+	},
+};
+module_platform_driver(envelope_detector_driver);
+
+MODULE_DESCRIPTION("Envelope detector using a DAC and a comparator");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* Re: [PATCH v2 4/7] dt-bindings: iio: document dpot-dac bindings
  2016-10-22 22:43 ` [PATCH v2 4/7] dt-bindings: iio: document dpot-dac bindings Peter Rosin
@ 2016-10-23  8:02   ` Peter Meerwald-Stadler
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Meerwald-Stadler @ 2016-10-23  8:02 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Rob Herring, Mark Rutland, linux-iio,
	devicetree


nitpicking below

> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  .../devicetree/bindings/iio/dac/dpot-dac.txt       | 41 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 ++++
>  2 files changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt b/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
> new file mode 100644
> index 000000000000..4fd5d63cc2d6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
> @@ -0,0 +1,41 @@
> +Bindings for DAC emulation using a digital potentiometer
> +
> +It is assumed the that the dpot is used as a voltage divider between the

delete 'the' before 'that'

> +current dpot wiper setting and the maximum resistance of the dpot. The
> +divided voltage is provided by a vref regulator.
> +
> +                  .------.
> +   .-----------.  |      |
> +   | vref      |--'    .---.
> +   | regulator |--.    |   |
> +   '-----------'  |    | d |
> +                  |    | p |
> +                  |    | o |  wiper
> +                  |    | t |<---------+
> +                  |    |   |
> +                  |    '---'       dac output voltage
> +                  |      |
> +                  '------+------------+
> +
> +Required properties:
> +- compatible: Should be "dpot-dac"
> +- vref-supply: The regulator supplying the voltage divider.
> +- io-channels: Channel node of the dpot to be used for the voltage division.
> +- io-channel-names: Should be "dpot".
> +
> +Example:
> +
> +	&i2c {
> +		dpot: mcp4651-503@28 {
> +			compatible = "microchip,mcp4651-503";
> +			reg = <0x28>;
> +			#io-channel-cells = <1>;
> +		};
> +	};
> +
> +	dac {
> +		compatible = "dpot-dac";
> +		vref-supply = <&reg_3v3>;
> +		io-channels = <&dpot 0>;
> +		io-channel-names = "dpot";
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1cd38a7e0064..c68b72088945 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6111,6 +6111,12 @@ L:	linux-media@vger.kernel.org
>  S:	Maintained
>  F:	drivers/media/rc/iguanair.c
>  
> +IIO DIGITAL POTENTIOMETER DAC
> +M:	Peter Rosin <peda@axentia.se>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
> +
>  IIO SUBSYSTEM AND DRIVERS
>  M:	Jonathan Cameron <jic23@kernel.org>
>  R:	Hartmut Knaack <knaack.h@gmx.de>
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

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

* Re: [PATCH v2 6/7] dt-bindings: iio: document envelope-detector bindings
  2016-10-22 22:43 ` [PATCH v2 6/7] dt-bindings: iio: document envelope-detector bindings Peter Rosin
@ 2016-10-23  9:01   ` Peter Rosin
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Rosin @ 2016-10-23  9:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland, linux-iio,
	devicetree

On 2016-10-23 00:43, Peter Rosin wrote:
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  .../bindings/iio/adc/envelope-detector.txt         | 55 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 +++
>  2 files changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> new file mode 100644
> index 000000000000..9099531cffff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> @@ -0,0 +1,55 @@
> +Bindings for ADC envelope detector using a DAC and a comparator
> +
> +The DAC is used to find the peak level of an alternating voltage input
> +signal by a binary search using the output of a comparator wired to
> +an interrupt pin. Like so:
> +                          _
> +                         | \
> +    input +------>-------|+ \
> +                         |   \
> +           .-------.     |    }---.
> +           |       |     |   /    |
> +           |    dac|-->--|- /     |
> +           |       |     |_/      |
> +           |       |              |
> +           |       |              |
> +           |    irq|------<-------'
> +           |       |
> +           '-------'
> +
> +Required properties:
> +- compatible: Should be "axentia,tse850-envelope-detector"
> +- io-channels: Channel node of the dac to be used for comparator input.
> +- io-channel-names: Should be "dac".
> +- interrupt specification for one client interrupt,
> +  see ../../interrupt-controller/interrupts.txt for details.
> +- interrupt-names: Should be "comp".
> +
> +Example:
> +
> +	&i2c {
> +		dpot: mcp4651-104@28 {
> +			compatible = "microchip,mcp4651-104";
> +			reg = <0x28>;
> +			#io-channel-cells = <1>;
> +		};
> +	};
> +
> +	dac: dac {
> +		compatible = "dpot-dac";
> +		vref-supply = <&reg_3v3>;
> +		io-channels = <&dpot 0>;
> +		io-channel-names = "dpot";
> +		dpot-dac,max-ohms = <100000>;

Just noticed that this line isn't needed anymore. I'll kill it for v3.

Cheers,
Peter

> +		#io-channel-cells = <1>;
> +	};
> +
> +	envelope-detector {
> +		compatible = "axentia,tse850-envelope-detector";
> +		io-channels = <&dac 0>;
> +		io-channel-names = "dac";
> +
> +		interrupt-parent = <&gpio>;
> +		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> +		interrupt-names = "comp";
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8c8aae24b96b..4b6f6ec1b703 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6118,6 +6118,12 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
>  F:	drivers/iio/dac/dpot-dac.c
>  
> +IIO ENVELOPE DETECTOR
> +M:	Peter Rosin <peda@axentia.se>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> +
>  IIO SUBSYSTEM AND DRIVERS
>  M:	Jonathan Cameron <jic23@kernel.org>
>  R:	Hartmut Knaack <knaack.h@gmx.de>
> 

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

* Re: [PATCH v2 1/7] iio:core: add a callback to allow drivers to provide _available attributes
  2016-10-22 22:43 ` [PATCH v2 1/7] iio:core: add a callback to allow drivers to provide _available attributes Peter Rosin
@ 2016-10-23  9:33   ` Jonathan Cameron
  2016-10-23 22:10     ` Peter Rosin
  0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2016-10-23  9:33 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, linux-iio, devicetree

On 22/10/16 23:43, Peter Rosin wrote:
> From: Jonathan Cameron <jic23@kernel.org>
> 
> A large number of attributes can only take a limited range of values.
> Currently in IIO this is handled by directly registering additional
> *_available attributes thus providing this information to userspace.
> 
> It is desirable to provide this information via the core for much the same
> reason this was done for the actual channel information attributes in the
> first place.  If it isn't there, then it can only really be accessed from
> userspace.  Other in kernel IIO consumers have no access to what valid
> parameters are.
> 
> Two forms are currently supported:
> * list of values in one particular IIO_VAL_* format.
> 	e.g. 1.300000 1.500000 1.730000
> * range specification with a step size:
> 	e.g. [1.000000 0.500000 2.500000]
> 	equivalent to 1.000000 1.5000000 2.000000 2.500000
> 
> An addition set of masks are used to allow different sharing rules for the
> *_available attributes generated.
> 
> This allows for example:
> 
> in_accel_x_offset
> in_accel_y_offset
> in_accel_offset_available.
> 
> We could have gone with having a specification for each and every
> info_mask element but that would have meant changing the existing userspace
> ABI.  This approach does not.
I'm wondering what I meant by this... where does it cause use ABI issues?
Do you have any idea?

I'd forgotten I'd even done it like this rather than just insisting on 
available for everything (which is more logical to me as we should know
the range of everything).

It's pretty low cost though and gives a way of adding this to drivers
piecemeal which is probably a good idea!
> 
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
> [rather mindlessly forward ported from approx 3 years ago /peda]
More importantly shepherding it through review!

Naturally it's perfect code so not comments inline ;)

Thanks again for picking this up!

So... I want to see lots of comments on this. If people are happy with
it (unlikely ;) then please say so.  It's at least a bit controversial
and adds a 'lot' of new ABI.


Jonathan
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/iio/industrialio-core.c | 232 +++++++++++++++++++++++++++++++++++-----
>  include/linux/iio/iio.h         |  11 ++
>  include/linux/iio/types.h       |   5 +
>  3 files changed, 223 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index fc340ed3dca1..93c69ebeda1e 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -575,51 +575,41 @@ int of_iio_read_mount_matrix(const struct device *dev,
>  #endif
>  EXPORT_SYMBOL(of_iio_read_mount_matrix);
>  
> -/**
> - * iio_format_value() - Formats a IIO value into its string representation
> - * @buf:	The buffer to which the formatted value gets written
> - * @type:	One of the IIO_VAL_... constants. This decides how the val
> - *		and val2 parameters are formatted.
> - * @size:	Number of IIO value entries contained in vals
> - * @vals:	Pointer to the values, exact meaning depends on the
> - *		type parameter.
> - *
> - * Return: 0 by default, a negative number on failure or the
> - *	   total number of characters written for a type that belongs
> - *	   to the IIO_VAL_... constant.
> - */
> -ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
> +static ssize_t __iio_format_value(char *buf, unsigned int type,
> +				  int size, const int *vals)
>  {
>  	unsigned long long tmp;
> +	int tmp0, tmp1;
>  	bool scale_db = false;
>  
>  	switch (type) {
>  	case IIO_VAL_INT:
> -		return sprintf(buf, "%d\n", vals[0]);
> +		return sprintf(buf, "%d", vals[0]);
>  	case IIO_VAL_INT_PLUS_MICRO_DB:
>  		scale_db = true;
>  	case IIO_VAL_INT_PLUS_MICRO:
>  		if (vals[1] < 0)
> -			return sprintf(buf, "-%d.%06u%s\n", abs(vals[0]),
> +			return sprintf(buf, "-%d.%06u%s", abs(vals[0]),
>  				       -vals[1], scale_db ? " dB" : "");
>  		else
> -			return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
> +			return sprintf(buf, "%d.%06u%s", vals[0], vals[1],
>  				scale_db ? " dB" : "");
>  	case IIO_VAL_INT_PLUS_NANO:
>  		if (vals[1] < 0)
> -			return sprintf(buf, "-%d.%09u\n", abs(vals[0]),
> +			return sprintf(buf, "-%d.%09u", abs(vals[0]),
>  				       -vals[1]);
>  		else
> -			return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
> +			return sprintf(buf, "%d.%09u", vals[0], vals[1]);
>  	case IIO_VAL_FRACTIONAL:
>  		tmp = div_s64((s64)vals[0] * 1000000000LL, vals[1]);
> -		vals[0] = (int)div_s64_rem(tmp, 1000000000, &vals[1]);
> -		return sprintf(buf, "%d.%09u\n", vals[0], abs(vals[1]));
> +		tmp1 = vals[1];
> +		tmp0 = (int)div_s64_rem(tmp, 1000000000, &tmp1);
> +		return sprintf(buf, "%d.%09u", tmp0, abs(tmp1));
>  	case IIO_VAL_FRACTIONAL_LOG2:
>  		tmp = (s64)vals[0] * 1000000000LL >> vals[1];
> -		vals[1] = do_div(tmp, 1000000000LL);
> -		vals[0] = tmp;
> -		return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
> +		tmp1 = do_div(tmp, 1000000000LL);
> +		tmp0 = tmp;
> +		return sprintf(buf, "%d.%09u", tmp0, tmp1);
>  	case IIO_VAL_INT_MULTIPLE:
>  	{
>  		int i;
> @@ -628,13 +618,34 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
>  		for (i = 0; i < size; ++i)
>  			len += snprintf(&buf[len], PAGE_SIZE - len, "%d ",
>  								vals[i]);
> -		len += snprintf(&buf[len], PAGE_SIZE - len, "\n");
>  		return len;
>  	}
>  	default:
>  		return 0;
>  	}
>  }
> +
> +/**
> + * iio_format_value() - Formats a IIO value into its string representation
> + * @buf:	The buffer to which the formatted value gets written
> + * @type:	One of the IIO_VAL_... constants. This decides how the val
> + *		and val2 parameters are formatted.
> + * @size:	Number of IIO value entries contained in vals
> + * @vals:	Pointer to the values, exact meaning depends on the
> + *		type parameter.
> + *
> + * Return: 0 by default, a negative number on failure or the
> + *	   total number of characters written for a type that belongs
> + *	   to the IIO_VAL_... constant.
> + */
> +ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
> +{
> +	ssize_t len;
> +
> +	len = __iio_format_value(buf, type, size, vals);
> +
> +	return len + sprintf(buf + len, "\n");
> +}
>  EXPORT_SYMBOL_GPL(iio_format_value);
>  
>  static ssize_t iio_read_channel_info(struct device *dev,
> @@ -662,6 +673,113 @@ static ssize_t iio_read_channel_info(struct device *dev,
>  	return iio_format_value(buf, ret, val_len, vals);
>  }
>  
> +static ssize_t iio_format_avail_list(char *buf, const int *vals,
> +				     int type, int length)
> +{
> +	int i;
> +	ssize_t len = 0;
> +
> +	switch (type) {
> +	case IIO_VAL_INT:
> +		for (i = 0; i < length; i++) {
> +			len += __iio_format_value(buf + len, type, 1, &vals[i]);
> +			if (i < length - 1)
> +				len += snprintf(buf + len,
> +						PAGE_SIZE - len,
> +						" ");
> +			else
> +				len += snprintf(buf + len,
> +						PAGE_SIZE - len,
> +						"\n");
> +		}
> +		break;
> +	default:
> +		for (i = 0; i < length / 2; i++) {
> +			len += __iio_format_value(buf + len,
> +						  type,
> +						  2,
> +						  &vals[i * 2]);
> +			if (i < length / 2 - 1)
> +				len += snprintf(buf + len,
> +						PAGE_SIZE - len,
> +						" ");
> +			else
> +				len += snprintf(buf + len,
> +						PAGE_SIZE - len,
> +						"\n");
> +		}
> +	};
> +
> +	return len;
> +}
> +
> +static ssize_t iio_format_avail_range(char *buf, const int *vals, int type)
> +{
> +	int i;
> +	ssize_t len;
> +
> +	len = snprintf(buf, PAGE_SIZE, "[");
> +	switch (type) {
> +	case IIO_VAL_INT:
> +		for (i = 0; i < 3; i++) {
> +			len += __iio_format_value(buf + len, type, 1, &vals[i]);
> +			if (i < 2)
> +				len += snprintf(buf + len,
> +						PAGE_SIZE - len,
> +						" ");
> +			else
> +				len += snprintf(buf + len,
> +						PAGE_SIZE - len,
> +						"]\n");
> +		}
> +		break;
> +	default:
> +		for (i = 0; i < 3; i++) {
> +			len += __iio_format_value(buf + len,
> +						  type,
> +						  2,
> +						  &vals[i * 2]);
> +			if (i < 2)
> +				len += snprintf(buf + len,
> +						PAGE_SIZE - len,
> +						" ");
> +			else
> +				len += snprintf(buf + len,
> +						PAGE_SIZE - len,
> +						"]\n");
> +		}
> +	};
> +
> +	return len;
> +}
> +
> +static ssize_t iio_read_channel_info_avail(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +	const int *vals;
> +	int ret;
> +	int length;
> +	int type;
> +
> +	ret = indio_dev->info->read_avail(indio_dev, this_attr->c,
> +					  &vals, &type, &length,
> +					  this_attr->address);
> +
> +	if (ret < 0)
> +		return ret;
> +	switch (ret) {
> +	case IIO_AVAIL_LIST:
> +		return iio_format_avail_list(buf, vals, type, length);
> +	case IIO_AVAIL_RANGE:
> +		return iio_format_avail_range(buf, vals, type);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  /**
>   * iio_str_to_fixpoint() - Parse a fixed-point number from a string
>   * @str: The string to parse
> @@ -978,6 +1096,40 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
>  	return attrcount;
>  }
>  
> +static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
> +					       struct iio_chan_spec const *chan,
> +					       enum iio_shared_by shared_by,
> +					       const long *infomask)
> +{
> +	int i, ret, attrcount = 0;
> +	char *avail_postfix;
> +
> +	for_each_set_bit(i, infomask, sizeof(infomask) * 8) {
> +		avail_postfix = kasprintf(GFP_KERNEL,
> +					  "%s_available",
> +					  iio_chan_info_postfix[i]);
> +		if (!avail_postfix)
> +			return -ENOMEM;
> +
> +		ret = __iio_add_chan_devattr(avail_postfix,
> +					     chan,
> +					     &iio_read_channel_info_avail,
> +					     NULL,
> +					     i,
> +					     shared_by,
> +					     &indio_dev->dev,
> +					     &indio_dev->channel_attr_list);
> +		kfree(avail_postfix);
> +		if ((ret == -EBUSY) && (shared_by != IIO_SEPARATE))
> +			continue;
> +		else if (ret < 0)
> +			return ret;
> +		attrcount++;
> +	}
> +
> +	return attrcount;
> +}
> +
>  static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>  					struct iio_chan_spec const *chan)
>  {
> @@ -993,6 +1145,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>  		return ret;
>  	attrcount += ret;
>  
> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
> +						  IIO_SEPARATE,
> +						  &chan->
> +						  info_mask_separate_available);
> +	if (ret < 0)
> +		return ret;
> +	attrcount += ret;
> +
>  	ret = iio_device_add_info_mask_type(indio_dev, chan,
>  					    IIO_SHARED_BY_TYPE,
>  					    &chan->info_mask_shared_by_type);
> @@ -1000,6 +1160,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>  		return ret;
>  	attrcount += ret;
>  
> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
> +						  IIO_SHARED_BY_TYPE,
> +						  &chan->
> +						  info_mask_shared_by_type_available);
> +	if (ret < 0)
> +		return ret;
> +	attrcount += ret;
> +
>  	ret = iio_device_add_info_mask_type(indio_dev, chan,
>  					    IIO_SHARED_BY_DIR,
>  					    &chan->info_mask_shared_by_dir);
> @@ -1007,6 +1175,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>  		return ret;
>  	attrcount += ret;
>  
> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
> +						  IIO_SHARED_BY_DIR,
> +						  &chan->info_mask_shared_by_dir_available);
> +	if (ret < 0)
> +		return ret;
> +	attrcount += ret;
> +
>  	ret = iio_device_add_info_mask_type(indio_dev, chan,
>  					    IIO_SHARED_BY_ALL,
>  					    &chan->info_mask_shared_by_all);
> @@ -1014,6 +1189,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>  		return ret;
>  	attrcount += ret;
>  
> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
> +						  IIO_SHARED_BY_ALL,
> +						  &chan->info_mask_shared_by_all_available);
> +	if (ret < 0)
> +		return ret;
> +	attrcount += ret;
> +
>  	if (chan->ext_info) {
>  		unsigned int i = 0;
>  		for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index b4a0679e4a49..c0f897084620 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -269,9 +269,13 @@ struct iio_chan_spec {
>  		enum iio_endian endianness;
>  	} scan_type;
>  	long			info_mask_separate;
> +	long			info_mask_separate_available;
>  	long			info_mask_shared_by_type;
> +	long			info_mask_shared_by_type_available;
>  	long			info_mask_shared_by_dir;
> +	long			info_mask_shared_by_dir_available;
>  	long			info_mask_shared_by_all;
> +	long			info_mask_shared_by_all_available;
>  	const struct iio_event_spec *event_spec;
>  	unsigned int		num_event_specs;
>  	const struct iio_chan_spec_ext_info *ext_info;
> @@ -397,6 +401,13 @@ struct iio_info {
>  			int *val_len,
>  			long mask);
>  
> +	int (*read_avail)(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan,
> +			  const int **vals,
> +			  int *type,
> +			  int *length,
> +			  long mask_el);
> +
>  	int (*write_raw)(struct iio_dev *indio_dev,
>  			 struct iio_chan_spec const *chan,
>  			 int val,
> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
> index 32b579525004..2aa7b6384d64 100644
> --- a/include/linux/iio/types.h
> +++ b/include/linux/iio/types.h
> @@ -29,4 +29,9 @@ enum iio_event_info {
>  #define IIO_VAL_FRACTIONAL 10
>  #define IIO_VAL_FRACTIONAL_LOG2 11
>  
> +enum iio_available_type {
> +	IIO_AVAIL_LIST,
> +	IIO_AVAIL_RANGE,
> +};
> +
>  #endif /* _IIO_TYPES_H_ */
> 

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

* Re: [PATCH v2 2/7] iio: mcp4531: provide range of available raw values
  2016-10-22 22:43 ` [PATCH v2 2/7] iio: mcp4531: provide range of available raw values Peter Rosin
@ 2016-10-23  9:36   ` Jonathan Cameron
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2016-10-23  9:36 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, linux-iio, devicetree

On 22/10/16 23:43, Peter Rosin wrote:
> Signed-off-by: Peter Rosin <peda@axentia.se>
Looks great to me.

Perhaps it's worth doing a cat *_available in the directory and pasting
the result here to show what the resulting ABI looks like.

Also new ABI so needs documenting! (you didn't think it would be
this easy did you? :)

J
> ---
>  drivers/iio/potentiometer/mcp4531.c | 104 +++++++++++++++++++++---------------
>  1 file changed, 62 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/iio/potentiometer/mcp4531.c b/drivers/iio/potentiometer/mcp4531.c
> index 13b6ae2fcf7b..0d1bcf89ae17 100644
> --- a/drivers/iio/potentiometer/mcp4531.c
> +++ b/drivers/iio/potentiometer/mcp4531.c
> @@ -38,7 +38,7 @@
>  
>  struct mcp4531_cfg {
>  	int wipers;
> -	int max_pos;
> +	int avail[3];
>  	int kohms;
>  };
>  
> @@ -78,38 +78,38 @@ enum mcp4531_type {
>  };
>  
>  static const struct mcp4531_cfg mcp4531_cfg[] = {
> -	[MCP453x_502] = { .wipers = 1, .max_pos = 128, .kohms =   5, },
> -	[MCP453x_103] = { .wipers = 1, .max_pos = 128, .kohms =  10, },
> -	[MCP453x_503] = { .wipers = 1, .max_pos = 128, .kohms =  50, },
> -	[MCP453x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
> -	[MCP454x_502] = { .wipers = 1, .max_pos = 128, .kohms =   5, },
> -	[MCP454x_103] = { .wipers = 1, .max_pos = 128, .kohms =  10, },
> -	[MCP454x_503] = { .wipers = 1, .max_pos = 128, .kohms =  50, },
> -	[MCP454x_104] = { .wipers = 1, .max_pos = 128, .kohms = 100, },
> -	[MCP455x_502] = { .wipers = 1, .max_pos = 256, .kohms =   5, },
> -	[MCP455x_103] = { .wipers = 1, .max_pos = 256, .kohms =  10, },
> -	[MCP455x_503] = { .wipers = 1, .max_pos = 256, .kohms =  50, },
> -	[MCP455x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
> -	[MCP456x_502] = { .wipers = 1, .max_pos = 256, .kohms =   5, },
> -	[MCP456x_103] = { .wipers = 1, .max_pos = 256, .kohms =  10, },
> -	[MCP456x_503] = { .wipers = 1, .max_pos = 256, .kohms =  50, },
> -	[MCP456x_104] = { .wipers = 1, .max_pos = 256, .kohms = 100, },
> -	[MCP463x_502] = { .wipers = 2, .max_pos = 128, .kohms =   5, },
> -	[MCP463x_103] = { .wipers = 2, .max_pos = 128, .kohms =  10, },
> -	[MCP463x_503] = { .wipers = 2, .max_pos = 128, .kohms =  50, },
> -	[MCP463x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
> -	[MCP464x_502] = { .wipers = 2, .max_pos = 128, .kohms =   5, },
> -	[MCP464x_103] = { .wipers = 2, .max_pos = 128, .kohms =  10, },
> -	[MCP464x_503] = { .wipers = 2, .max_pos = 128, .kohms =  50, },
> -	[MCP464x_104] = { .wipers = 2, .max_pos = 128, .kohms = 100, },
> -	[MCP465x_502] = { .wipers = 2, .max_pos = 256, .kohms =   5, },
> -	[MCP465x_103] = { .wipers = 2, .max_pos = 256, .kohms =  10, },
> -	[MCP465x_503] = { .wipers = 2, .max_pos = 256, .kohms =  50, },
> -	[MCP465x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
> -	[MCP466x_502] = { .wipers = 2, .max_pos = 256, .kohms =   5, },
> -	[MCP466x_103] = { .wipers = 2, .max_pos = 256, .kohms =  10, },
> -	[MCP466x_503] = { .wipers = 2, .max_pos = 256, .kohms =  50, },
> -	[MCP466x_104] = { .wipers = 2, .max_pos = 256, .kohms = 100, },
> +	[MCP453x_502] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =   5, },
> +	[MCP453x_103] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  10, },
> +	[MCP453x_503] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  50, },
> +	[MCP453x_104] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 100, },
> +	[MCP454x_502] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =   5, },
> +	[MCP454x_103] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  10, },
> +	[MCP454x_503] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms =  50, },
> +	[MCP454x_104] = { .wipers = 1, .avail = { 0, 1, 128 }, .kohms = 100, },
> +	[MCP455x_502] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =   5, },
> +	[MCP455x_103] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  10, },
> +	[MCP455x_503] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  50, },
> +	[MCP455x_104] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 100, },
> +	[MCP456x_502] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =   5, },
> +	[MCP456x_103] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  10, },
> +	[MCP456x_503] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms =  50, },
> +	[MCP456x_104] = { .wipers = 1, .avail = { 0, 1, 256 }, .kohms = 100, },
> +	[MCP463x_502] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =   5, },
> +	[MCP463x_103] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  10, },
> +	[MCP463x_503] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  50, },
> +	[MCP463x_104] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 100, },
> +	[MCP464x_502] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =   5, },
> +	[MCP464x_103] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  10, },
> +	[MCP464x_503] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms =  50, },
> +	[MCP464x_104] = { .wipers = 2, .avail = { 0, 1, 128 }, .kohms = 100, },
> +	[MCP465x_502] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =   5, },
> +	[MCP465x_103] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  10, },
> +	[MCP465x_503] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  50, },
> +	[MCP465x_104] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 100, },
> +	[MCP466x_502] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =   5, },
> +	[MCP466x_103] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  10, },
> +	[MCP466x_503] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms =  50, },
> +	[MCP466x_104] = { .wipers = 2, .avail = { 0, 1, 256 }, .kohms = 100, },
>  };
>  
>  #define MCP4531_WRITE (0 << 2)
> @@ -124,13 +124,14 @@ struct mcp4531_data {
>  	const struct mcp4531_cfg *cfg;
>  };
>  
> -#define MCP4531_CHANNEL(ch) {					\
> -	.type = IIO_RESISTANCE,					\
> -	.indexed = 1,						\
> -	.output = 1,						\
> -	.channel = (ch),					\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +#define MCP4531_CHANNEL(ch) {						\
> +	.type = IIO_RESISTANCE,						\
> +	.indexed = 1,							\
> +	.output = 1,							\
> +	.channel = (ch),						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_RAW),	\
>  }
>  
>  static const struct iio_chan_spec mcp4531_channels[] = {
> @@ -156,13 +157,31 @@ static int mcp4531_read_raw(struct iio_dev *indio_dev,
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 1000 * data->cfg->kohms;
> -		*val2 = data->cfg->max_pos;
> +		*val2 = data->cfg->avail[2];
>  		return IIO_VAL_FRACTIONAL;
>  	}
>  
>  	return -EINVAL;
>  }
>  
> +static int mcp4531_read_avail(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      const int **vals, int *type, int *length,
> +			      long mask)
> +{
> +	struct mcp4531_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*length = ARRAY_SIZE(data->cfg->avail);
> +		*vals = data->cfg->avail;
> +		*type = IIO_VAL_INT;
> +		return IIO_AVAIL_RANGE;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static int mcp4531_write_raw(struct iio_dev *indio_dev,
>  			     struct iio_chan_spec const *chan,
>  			     int val, int val2, long mask)
> @@ -172,7 +191,7 @@ static int mcp4531_write_raw(struct iio_dev *indio_dev,
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		if (val > data->cfg->max_pos || val < 0)
> +		if (val > data->cfg->avail[2] || val < 0)
>  			return -EINVAL;
>  		break;
>  	default:
> @@ -186,6 +205,7 @@ static int mcp4531_write_raw(struct iio_dev *indio_dev,
>  
>  static const struct iio_info mcp4531_info = {
>  	.read_raw = mcp4531_read_raw,
> +	.read_avail = mcp4531_read_avail,
>  	.write_raw = mcp4531_write_raw,
>  	.driver_module = THIS_MODULE,
>  };
> 

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

* Re: [PATCH v2 5/7] iio: dpot-dac: DAC driver based on a digital potentiometer
  2016-10-22 22:43 ` [PATCH v2 5/7] iio: dpot-dac: DAC driver based on a digital potentiometer Peter Rosin
@ 2016-10-23  9:42   ` Jonathan Cameron
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2016-10-23  9:42 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, linux-iio, devicetree

On 22/10/16 23:43, Peter Rosin wrote:
> It is assumed the that the dpot is used as a voltage divider between the
> current dpot wiper setting and the maximum resistance of the dpot. The
> divided voltage is provided by a vref regulator.
> 
>                   .------.
>    .-----------.  |      |
>    | vref      |--'    .---.
>    | regulator |--.    |   |
>    '-----------'  |    | d |
>                   |    | p |
>                   |    | o |  wiper
>                   |    | t |<---------+
>                   |    |   |
>                   |    '---'       dac output voltage
>                   |      |
>                   '------+------------+
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
Only suggstions is that some of the stuff for reading max value from
the channel wan't to get wrapped up in a utility function in iio/consumer.h
like the various channel value read and write functions.

As much of possible of internal IIO stuff should be opaque to consumers
(even when they happen to know about it as they are also IIO drivers ;)

Jonathan
> ---
>  MAINTAINERS                |   1 +
>  drivers/iio/dac/Kconfig    |  10 ++
>  drivers/iio/dac/Makefile   |   1 +
>  drivers/iio/dac/dpot-dac.c | 298 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 310 insertions(+)
>  create mode 100644 drivers/iio/dac/dpot-dac.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c68b72088945..8c8aae24b96b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6116,6 +6116,7 @@ M:	Peter Rosin <peda@axentia.se>
>  L:	linux-iio@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
> +F:	drivers/iio/dac/dpot-dac.c
>  
>  IIO SUBSYSTEM AND DRIVERS
>  M:	Jonathan Cameron <jic23@kernel.org>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 120b24478469..d3084028905b 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -200,6 +200,16 @@ config AD8801
>  	  To compile this driver as a module choose M here: the module will be called
>  	  ad8801.
>  
> +config DPOT_DAC
> +	tristate "DAC emulation using a DPOT"
> +	depends on OF
> +	help
> +	  Say yes here to build support for DAC emulation using a digital
> +	  potentiometer.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called dpot-dac.
> +
>  config LPC18XX_DAC
>  	tristate "NXP LPC18xx DAC driver"
>  	depends on ARCH_LPC18XX || COMPILE_TEST
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 27642bbf75f2..f01bf4a99867 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_AD5686) += ad5686.o
>  obj-$(CONFIG_AD7303) += ad7303.o
>  obj-$(CONFIG_AD8801) += ad8801.o
>  obj-$(CONFIG_CIO_DAC) += cio-dac.o
> +obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
>  obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
>  obj-$(CONFIG_M62332) += m62332.o
>  obj-$(CONFIG_MAX517) += max517.o
> diff --git a/drivers/iio/dac/dpot-dac.c b/drivers/iio/dac/dpot-dac.c
> new file mode 100644
> index 000000000000..5613eae32347
> --- /dev/null
> +++ b/drivers/iio/dac/dpot-dac.c
> @@ -0,0 +1,298 @@
> +/*
> + * IIO DAC emulation driver using a digital potentiometer
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +/*
> + * It is assumed that the dpot is used as a voltage divider between the
> + * current dpot wiper setting and the maximum resistance of the dpot. The
> + * divided voltage is provided by a vref regulator.
> + *
> + *                   .------.
> + *    .-----------.  |      |
> + *    | vref      |--'    .---.
> + *    | regulator |--.    |   |
> + *    '-----------'  |    | d |
> + *                   |    | p |
> + *                   |    | o |  wiper
> + *                   |    | t |<---------+
> + *                   |    |   |
> + *                   |    '---'       dac output voltage
> + *                   |      |
> + *                   '------+------------+
> + */
> +
> +#include <linux/err.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +
> +struct dpot_dac {
> +	struct regulator *vref;
> +	struct iio_channel *dpot;
> +	u32 max_ohms;
> +};
> +
> +static const struct iio_chan_spec dpot_dac_iio_channel = {
> +	.type = IIO_VOLTAGE,
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
> +			    | BIT(IIO_CHAN_INFO_SCALE),
> +	.info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW),
> +	.output = 1,
> +	.indexed = 1,
> +};
> +
> +static int dpot_dac_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	struct dpot_dac *dac = iio_priv(indio_dev);
> +	int ret;
> +	unsigned long long tmp;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		return iio_read_channel_raw(dac->dpot, val);
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = iio_read_channel_scale(dac->dpot, val, val2);
> +		switch (ret) {
> +		case IIO_VAL_FRACTIONAL_LOG2:
> +			tmp = *val * 1000000000LL;
> +			do_div(tmp, dac->max_ohms);
> +			tmp *= regulator_get_voltage(dac->vref) / 1000;
> +			do_div(tmp, 1000000000LL);
> +			*val = tmp;
> +			return ret;
> +		case IIO_VAL_INT:
> +			/*
> +			 * Convert integer scale to fractional scale by
> +			 * setting the denominator (val2) to one...
> +			 */
> +			*val2 = 1;
> +			ret = IIO_VAL_FRACTIONAL;
> +			/* ...and fall through. */
> +		case IIO_VAL_FRACTIONAL:
> +			*val *= regulator_get_voltage(dac->vref) / 1000;
> +			*val2 *= dac->max_ohms;
> +			break;
> +		}
> +
> +		return ret;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int dpot_dac_read_avail(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       const int **vals, int *type, int *length,
> +			       long mask)
> +{
> +	struct dpot_dac *dac = iio_priv(indio_dev);
> +	struct iio_dev *dpot_dev = dac->dpot->indio_dev;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		return dpot_dev->info->read_avail(dpot_dev, dac->dpot->channel,
> +						  vals, type, length, mask);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int dpot_dac_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int val, int val2, long mask)
> +{
> +	struct dpot_dac *dac = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		return iio_write_channel_raw(dac->dpot, val);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info dpot_dac_info = {
> +	.read_raw = dpot_dac_read_raw,
> +	.read_avail = dpot_dac_read_avail,
> +	.write_raw = dpot_dac_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int dpot_dac_channel_max_ohms(struct iio_dev *indio_dev)
> +{
> +	struct device *dev = &indio_dev->dev;
> +	struct dpot_dac *dac = iio_priv(indio_dev);
> +	struct iio_channel *ch = dac->dpot;
> +	struct iio_dev *iio_ch_dev = ch->indio_dev;
> +	const int *vals;
> +	unsigned long long tmp;
> +	int type;
> +	int len;
> +	int ret;
> +	int val1;
> +	int val2;
> +	int max;
> +
Wants a wrapper in iio/consumer.h like we do for the various reads.
Otherwise, we'll end up with this as cut and paste boiler plate all over
the place.

> +	if (!iio_ch_dev->info->read_avail) {
> +		dev_err(dev, "dpot does not provide available raw values\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = iio_ch_dev->info->read_avail(iio_ch_dev, ch->channel,
> +					   &vals, &type, &len,
> +					   IIO_CHAN_INFO_RAW);
> +
> +	if (ret < 0) {
> +		dev_err(dev, "dpot failed to provide available raw values\n");
> +		return ret;
> +	}
(wrapper down to here..)
Worth noting that for the consumers they shouldn't need to know anything
about internal IIO structures (obviously you do here for other reasons
but try not to use that knowledge - should all be opaque outside the
subsystem.)
> +	if (type != IIO_VAL_INT) {
> +		dev_err(dev, "dpot provides strange raw values\n");
> +		return -EINVAL;
> +	}
> +
> +	switch (ret) {
> +	case IIO_AVAIL_RANGE:
> +		if (len != 3) {
> +			dev_err(dev, "need a range of available values\n");
> +			return -EINVAL;
> +		}
> +		max = vals[2];
> +		break;
> +	default:
> +		dev_err(dev, "dpot doesn't provide range of available values\n");
You could handle the set of values but it would be more complex so fair
enough.  Any non linear dpots out there (with few enough values that we
can describe them in under page size)?
> +		return -EINVAL;
> +	}
> +
> +	switch (iio_read_channel_scale(dac->dpot, &val1, &val2)) {
> +	case IIO_VAL_INT:
> +		return max * val1;
> +	case IIO_VAL_FRACTIONAL:
> +		tmp = (unsigned long long)max * val1;
> +		do_div(tmp, val2);
> +		return tmp;
> +	case IIO_VAL_FRACTIONAL_LOG2:
> +		tmp = (s64)vals[0] * 1000000000LL * max >> vals[1];
> +		do_div(tmp, 1000000000LL);
> +		return tmp;
> +	default:
> +		dev_err(dev, "dpot has a scale that is too weird\n");
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int dpot_dac_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct iio_dev *indio_dev;
> +	struct dpot_dac *dac;
> +	enum iio_chan_type type;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*dac));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +	dac = iio_priv(indio_dev);
> +
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->dev.parent = dev;
> +	indio_dev->info = &dpot_dac_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = &dpot_dac_iio_channel;
> +	indio_dev->num_channels = 1;
> +
> +	dac->vref = devm_regulator_get(dev, "vref");
> +	if (IS_ERR(dac->vref)) {
> +		if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "failed to get vref regulator\n");
> +		return PTR_ERR(dac->vref);
> +	}
> +
> +	dac->dpot = devm_iio_channel_get(dev, "dpot");
> +	if (IS_ERR(dac->dpot)) {
> +		if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get dpot input channel\n");
> +		return PTR_ERR(dac->dpot);
> +	}
> +
> +	ret = iio_get_channel_type(dac->dpot, &type);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (type != IIO_RESISTANCE) {
> +		dev_err(dev, "dpot is of the wrong type\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = dpot_dac_channel_max_ohms(indio_dev);
> +	if (ret < 0)
> +		return ret;
> +	dac->max_ohms = ret;
> +	dev_info(dev, "dpot max is %d\n", dac->max_ohms);
> +
> +	ret = regulator_enable(dac->vref);
> +	if (ret) {
> +		dev_err(dev, "failed to enable the vref regulator\n");
> +		return ret;
> +	}
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(dev, "failed to register iio device\n");
> +		goto disable_reg;
> +	}
> +
> +	return 0;
> +
> +disable_reg:
> +	regulator_disable(dac->vref);
> +	return ret;
> +}
> +
> +static int dpot_dac_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct dpot_dac *dac = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	regulator_disable(dac->vref);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id dpot_dac_match[] = {
> +	{ .compatible = "dpot-dac" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, dpot_dac_match);
> +
> +static struct platform_driver dpot_dac_driver = {
> +	.probe = dpot_dac_probe,
> +	.remove = dpot_dac_remove,
> +	.driver = {
> +		.name = "iio-dpot-dac",
> +		.of_match_table = dpot_dac_match,
> +	},
> +};
> +module_platform_driver(dpot_dac_driver);
> +
> +MODULE_DESCRIPTION("DAC emulation driver using a digital potentiometer");
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
> +MODULE_LICENSE("GPL v2");
> 

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

* Re: [PATCH v2 7/7] iio: envelope-detector: ADC driver based on a DAC and a comparator
  2016-10-22 22:43 ` [PATCH v2 7/7] iio: envelope-detector: ADC driver based on a DAC and a comparator Peter Rosin
@ 2016-10-23  9:48   ` Jonathan Cameron
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2016-10-23  9:48 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, linux-iio, devicetree

On 22/10/16 23:43, Peter Rosin wrote:
> The DAC is used to find the peak level of an alternating voltage input
> signal by a binary search using the output of a comparator wired to
> an interrupt pin. Like so:
>                           _
>                          | \
>     input +------>-------|+ \
>                          |   \
>            .-------.     |    }---.
>            |       |     |   /    |
>            |    dac|-->--|- /     |
>            |       |     |_/      |
>            |       |              |
>            |       |              |
>            |    irq|------<-------'
>            |       |
>            '-------'
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
Again, just the need for opaque handling of the getting of the available
attribute.

Move your boiler plate into a core supplied function.

Looking very nice.

Jonathan
> ---
>  .../testing/sysfs-bus-iio-adc-envelope-detector    |  36 ++
>  MAINTAINERS                                        |   2 +
>  drivers/iio/adc/Kconfig                            |  10 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/envelope-detector.c                | 463 +++++++++++++++++++++
>  5 files changed, 512 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
>  create mode 100644 drivers/iio/adc/envelope-detector.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector b/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
> new file mode 100644
> index 000000000000..8d890028a649
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
> @@ -0,0 +1,36 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/invert
> +Date:		October 2016
> +KernelVersion:	4.9
> +Contact:	Peter Rosin <peda@axentia.se>
> +Description:
> +		The DAC is used to find the peak level of an alternating
> +		voltage input signal by a binary search using the output
> +		of a comparator wired to an interrupt pin. Like so:
> +		                           _
> +		                          | \
> +		     input +------>-------|+ \
> +		                          |   \
> +		            .-------.     |    }---.
> +		            |       |     |   /    |
> +		            |    dac|-->--|- /     |
> +		            |       |     |_/      |
> +		            |       |              |
> +		            |       |              |
> +		            |    irq|------<-------'
> +		            |       |
> +		            '-------'
> +		The boolean invert attribute (0/1) should be set when the
> +		input signal is centered around the maximum value of the
> +		dac instead of zero. The envelope detector will search
> +		from below in this case and will also invert the result.
> +		The edge/level of the interrupt is also switched to its
> +		opposite value.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/compare_interval_ms
> +Date:		October 2016
> +KernelVersion:	4.9
> +Contact:	Peter Rosin <peda@axentia.se>
> +Description:
> +		Number of milliseconds to wait for the comparator in each
> +		step of the binary search for the input peak level. Needs
> +		to relate to the frequency of the input signal.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4b6f6ec1b703..0d20f3561700 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6122,7 +6122,9 @@ IIO ENVELOPE DETECTOR
>  M:	Peter Rosin <peda@axentia.se>
>  L:	linux-iio@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
>  F:	Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> +F:	drivers/iio/adc/envelope-detector.c
>  
>  IIO SUBSYSTEM AND DRIVERS
>  M:	Jonathan Cameron <jic23@kernel.org>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 7edcf3238620..d5c4a95855c2 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -195,6 +195,16 @@ config DA9150_GPADC
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called berlin2-adc.
>  
> +config ENVELOPE_DETECTOR
> +	tristate "Envelope detector using a DAC and a comparator"
> +	depends on OF
> +	help
> +	  Say yes here to build support for an envelope detector using a DAC
> +	  and a comparator.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called iio-envelope-detector.
> +
>  config EXYNOS_ADC
>  	tristate "Exynos ADC driver support"
>  	depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || (OF && COMPILE_TEST)
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 7a40c04c311f..0d773c6a0578 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_BCM_IPROC_ADC) += bcm_iproc_adc.o
>  obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
>  obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
>  obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
> +obj-$(CONFIG_ENVELOPE_DETECTOR) += envelope-detector.o
>  obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
>  obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
>  obj-$(CONFIG_HI8435) += hi8435.o
> diff --git a/drivers/iio/adc/envelope-detector.c b/drivers/iio/adc/envelope-detector.c
> new file mode 100644
> index 000000000000..3842d1a72023
> --- /dev/null
> +++ b/drivers/iio/adc/envelope-detector.c
> @@ -0,0 +1,463 @@
> +/*
> + * Driver for an envelope detector using a DAC and a comparator
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +/*
> + * The DAC is used to find the peak level of an alternating voltage input
> + * signal by a binary search using the output of a comparator wired to
> + * an interrupt pin. Like so:
> + *                           _
> + *                          | \
> + *     input +------>-------|+ \
> + *                          |   \
> + *            .-------.     |    }---.
> + *            |       |     |   /    |
> + *            |    dac|-->--|- /     |
> + *            |       |     |_/      |
> + *            |       |              |
> + *            |       |              |
> + *            |    irq|------<-------'
> + *            |       |
> + *            '-------'
> + */
> +
> +#include <linux/completion.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/workqueue.h>
> +
> +struct envelope {
> +	spinlock_t comp_lock; /* protects comp */
> +	int comp;
> +
> +	struct mutex read_lock; /* protects everything else */
> +
> +	int comp_irq;
> +	u32 comp_irq_trigger;
> +	u32 comp_irq_trigger_inv;
> +
> +	struct iio_channel *dac;
> +	struct delayed_work comp_timeout;
> +
> +	unsigned int comp_interval;
> +	bool invert;
> +	u32 dac_max;
> +
> +	int high;
> +	int level;
> +	int low;
> +
> +	struct completion done;
> +};
> +
> +/*
> + * The envelope_detector_comp_latch function works together with the compare
> + * interrupt service routine below (envelope_detector_comp_isr) as a latch
> + * (one-bit memory) for if the interrupt has triggered since last calling
> + * this function.
> + * The ..._comp_isr function disables the interrupt so that the cpu does not
> + * need to service a possible interrupt flood from the comparator when no-one
> + * cares anyway, and this ..._comp_latch function reenables them again if
> + * needed.
> + */
> +static int envelope_detector_comp_latch(struct envelope *env)
> +{
> +	int comp;
> +
> +	spin_lock_irq(&env->comp_lock);
> +	comp = env->comp;
> +	env->comp = 0;
> +	spin_unlock_irq(&env->comp_lock);
> +
> +	if (!comp)
> +		return 0;
> +
> +	/*
> +	 * The irq was disabled, and is reenabled just now.
> +	 * But there might have been a pending irq that
> +	 * happened while the irq was disabled that fires
> +	 * just as the irq is reenabled. That is not what
> +	 * is desired.
> +	 */
> +	enable_irq(env->comp_irq);
> +
> +	/* So, synchronize this possibly pending irq... */
> +	synchronize_irq(env->comp_irq);
> +
> +	/* ...and redo the whole dance. */
> +	spin_lock_irq(&env->comp_lock);
> +	comp = env->comp;
> +	env->comp = 0;
> +	spin_unlock_irq(&env->comp_lock);
> +
> +	if (comp)
> +		enable_irq(env->comp_irq);
> +
> +	return 1;
> +}
> +
> +static irqreturn_t envelope_detector_comp_isr(int irq, void *ctx)
> +{
> +	struct envelope *env = ctx;
> +
> +	spin_lock(&env->comp_lock);
> +	env->comp = 1;
> +	disable_irq_nosync(env->comp_irq);
> +	spin_unlock(&env->comp_lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void envelope_detector_setup_compare(struct envelope *env)
> +{
> +	int ret;
> +
> +	/*
> +	 * Do a binary search for the peak input level, and stop
> +	 * when that level is "trapped" between two adjacent DAC
> +	 * values.
> +	 * When invert is active, use the midpoint floor so that
> +	 * env->level ends up as env->low when the termination
> +	 * criteria below is fulfilled, and use the midpoint
> +	 * ceiling when invert is not active so that env->level
> +	 * ends up as env->high in that case.
> +	 */
> +	env->level = (env->high + env->low + !env->invert) / 2;
> +
> +	if (env->high == env->low + 1) {
> +		complete(&env->done);
> +		return;
> +	}
> +
> +	/* Set a "safe" DAC level (if there is such a thing)... */
> +	ret = iio_write_channel_raw(env->dac, env->invert ? 0 : env->dac_max);
> +	if (ret < 0)
> +		goto err;
> +
> +	/* ...clear the comparison result... */
> +	envelope_detector_comp_latch(env);
> +
> +	/* ...set the real DAC level... */
> +	ret = iio_write_channel_raw(env->dac, env->level);
> +	if (ret < 0)
> +		goto err;
> +
> +	/* ...and wait for a bit to see if the latch catches anything. */
> +	schedule_delayed_work(&env->comp_timeout,
> +			      msecs_to_jiffies(env->comp_interval));
> +	return;
> +
> +err:
> +	env->level = ret;
> +	complete(&env->done);
> +}
> +
> +static void envelope_detector_timeout(struct work_struct *work)
> +{
> +	struct envelope *env = container_of(work, struct envelope,
> +					    comp_timeout.work);
> +
> +	/* Adjust low/high depending on the latch content... */
> +	if (!envelope_detector_comp_latch(env) ^ !env->invert)
> +		env->low = env->level;
> +	else
> +		env->high = env->level;
> +
> +	/* ...and continue the search. */
> +	envelope_detector_setup_compare(env);
> +}
> +
> +static int envelope_detector_read_raw(struct iio_dev *indio_dev,
> +				      struct iio_chan_spec const *chan,
> +				      int *val, int *val2, long mask)
> +{
> +	struct envelope *env = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		/*
> +		 * When invert is active, start with high=max+1 and low=0
> +		 * since we will end up with the low value when the
> +		 * termination criteria is fulfilled (rounding down). And
> +		 * start with high=max and low=-1 when invert is not active
> +		 * since we will end up with the high value in that case.
> +		 * This ensures that the returned value in both cases are
> +		 * in the same range as the DAC and is a value that has not
> +		 * triggered the comparator.
> +		 */
> +		mutex_lock(&env->read_lock);
> +		env->high = env->dac_max + env->invert;
> +		env->low = -1 + env->invert;
> +		envelope_detector_setup_compare(env);
> +		wait_for_completion(&env->done);
> +		if (env->level < 0) {
> +			ret = env->level;
> +			goto err_unlock;
> +		}
> +		*val = env->invert ? env->dac_max - env->level : env->level;
> +		mutex_unlock(&env->read_lock);
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		return iio_read_channel_scale(env->dac, val, val2);
> +	}
> +
> +	return -EINVAL;
> +
> +err_unlock:
> +	mutex_unlock(&env->read_lock);
> +	return ret;
> +}
> +
> +static int envelope_detector_channel_raw_max(struct iio_channel *ch)
> +{
> +	struct iio_dev *indio_dev = ch->indio_dev;
> +	const int *vals;
> +	int type;
> +	int len;
> +	int ret;
> +
Again, wrap this stuff up so that we have only

ret = iio_read_avail(ch, IIO_CHAN_INFO_RAW, &vals, &type, &len);

Could even have iio_read_avail_max returning just the max representation.

ABI wise might be cleaner to move the IIO type into the parameters as
well. It's no different from type realy in it's importance.  Hmm. Not obvious
so take your pick with which ever approach feels best to you.
> +	if (!ch->indio_dev->info->read_avail)
> +		return -EINVAL;
> +
> +	ret = ch->indio_dev->info->read_avail(indio_dev, ch->channel,
> +					      &vals, &type, &len,
> +					      IIO_CHAN_INFO_RAW);
> +
> +	if (ret < 0)
> +		return ret;
> +	if (type != IIO_VAL_INT)
> +		return -EINVAL;
> +
> +	switch (ret) {
> +	case IIO_AVAIL_RANGE:
> +		if (len != 3)
> +			return -EINVAL;
> +		return vals[2];
> +	}
> +	return -EINVAL;
> +}
> +
> +static const struct iio_chan_spec envelope_detector_iio_channel = {
> +	.type = IIO_ALTVOLTAGE,
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
> +			    | BIT(IIO_CHAN_INFO_SCALE),
> +	.indexed = 1,
> +};
> +
> +static ssize_t envelope_show_invert(struct device *dev,
> +				    struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct envelope *env = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%u\n", env->invert);
> +}
> +
> +static ssize_t envelope_store_invert(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf,
> +				     size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct envelope *env = iio_priv(indio_dev);
> +	unsigned long invert;
> +	int ret;
> +	u32 trigger;
> +
> +	ret = kstrtoul(buf, 0, &invert);
> +	if (ret < 0)
> +		return ret;
> +	if (invert > 1)
> +		return -EINVAL;
> +
> +	trigger = invert ? env->comp_irq_trigger_inv : env->comp_irq_trigger;
> +
> +	mutex_lock(&env->read_lock);
> +	if (invert != env->invert)
> +		ret = irq_set_irq_type(env->comp_irq, trigger);
> +	if (!ret) {
> +		env->invert = invert;
> +		ret = len;
> +	}
> +	mutex_unlock(&env->read_lock);
> +
> +	return ret;
> +}
> +
> +static IIO_DEVICE_ATTR(invert, 0644,
> +		       envelope_show_invert,
> +		       envelope_store_invert, 0);
> +
> +static ssize_t envelope_show_comp_interval(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct envelope *env = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%u\n", env->comp_interval);
> +}
> +
> +static ssize_t envelope_store_comp_interval(struct device *dev,
> +					    struct device_attribute *attr,
> +					    const char *buf,
> +					    size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct envelope *env = iio_priv(indio_dev);
> +	unsigned long interval;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &interval);
> +	if (ret < 0)
> +		return ret;
> +	if (interval > 1000)
> +		return -EINVAL;
> +
> +	mutex_lock(&env->read_lock);
> +	env->comp_interval = interval;
> +	mutex_unlock(&env->read_lock);
> +
> +	return len;
> +}
> +
> +static IIO_DEVICE_ATTR(compare_interval_ms, 0644,
> +		       envelope_show_comp_interval,
> +		       envelope_store_comp_interval, 0);
> +
> +static struct attribute *envelope_detector_attributes[] = {
> +	&iio_dev_attr_invert.dev_attr.attr,
> +	&iio_dev_attr_compare_interval_ms.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group envelope_detector_attribute_group = {
> +	.attrs = envelope_detector_attributes,
> +};
> +
> +static const struct iio_info envelope_detector_info = {
> +	.read_raw = &envelope_detector_read_raw,
> +	.driver_module = THIS_MODULE,
> +	.attrs = &envelope_detector_attribute_group,
> +};
> +
> +static int envelope_detector_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct iio_dev *indio_dev;
> +	struct envelope *env;
> +	enum iio_chan_type type;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*env));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +	env = iio_priv(indio_dev);
> +	env->comp_interval = 50; /* some sensible default? */
> +
> +	spin_lock_init(&env->comp_lock);
> +	mutex_init(&env->read_lock);
> +	init_completion(&env->done);
> +	INIT_DELAYED_WORK(&env->comp_timeout, envelope_detector_timeout);
> +
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->dev.parent = dev;
> +	indio_dev->dev.of_node = dev->of_node;
> +	indio_dev->info = &envelope_detector_info;
> +	indio_dev->channels = &envelope_detector_iio_channel;
> +	indio_dev->num_channels = 1;
> +
> +	env->dac = devm_iio_channel_get(dev, "dac");
> +	if (IS_ERR(env->dac)) {
> +		if (PTR_ERR(env->dac) != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get dac input channel\n");
> +		return PTR_ERR(env->dac);
> +	}
> +
> +	env->comp_irq = platform_get_irq_byname(pdev, "comp");
> +	if (env->comp_irq < 0) {
> +		if (env->comp_irq != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get compare interrupt\n");
> +		return env->comp_irq;
> +	}
> +
> +	ret = devm_request_irq(dev, env->comp_irq, envelope_detector_comp_isr,
> +			       0, "envelope-detector", env);
> +	if (ret) {
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to request interrupt\n");
> +		return ret;
> +	}
> +	env->comp_irq_trigger = irq_get_trigger_type(env->comp_irq);
> +	if (env->comp_irq_trigger & IRQF_TRIGGER_RISING)
> +		env->comp_irq_trigger_inv |= IRQF_TRIGGER_FALLING;
> +	if (env->comp_irq_trigger & IRQF_TRIGGER_FALLING)
> +		env->comp_irq_trigger_inv |= IRQF_TRIGGER_RISING;
> +	if (env->comp_irq_trigger & IRQF_TRIGGER_HIGH)
> +		env->comp_irq_trigger_inv |= IRQF_TRIGGER_LOW;
> +	if (env->comp_irq_trigger & IRQF_TRIGGER_LOW)
> +		env->comp_irq_trigger_inv |= IRQF_TRIGGER_HIGH;
> +
> +	ret = iio_get_channel_type(env->dac, &type);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (type != IIO_VOLTAGE) {
> +		dev_err(dev, "dac is of the wrong type\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = envelope_detector_channel_raw_max(env->dac);
> +	if (ret < 0) {
> +		dev_err(dev, "dac does not indicate its raw maximum value\n");
> +		return ret;
> +	}
> +	env->dac_max = ret;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static const struct of_device_id envelope_detector_match[] = {
> +	{ .compatible = "axentia,tse850-envelope-detector", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, envelope_detector_match);
> +
> +static struct platform_driver envelope_detector_driver = {
> +	.probe = envelope_detector_probe,
> +	.driver = {
> +		.name = "iio-envelope-detector",
> +		.of_match_table = envelope_detector_match,
> +	},
> +};
> +module_platform_driver(envelope_detector_driver);
> +
> +MODULE_DESCRIPTION("Envelope detector using a DAC and a comparator");
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
> +MODULE_LICENSE("GPL v2");
> 

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

* Re: [PATCH v2 1/7] iio:core: add a callback to allow drivers to provide _available attributes
  2016-10-23  9:33   ` Jonathan Cameron
@ 2016-10-23 22:10     ` Peter Rosin
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Rosin @ 2016-10-23 22:10 UTC (permalink / raw)
  To: Jonathan Cameron, linux-kernel
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, linux-iio, devicetree

On 2016-10-23 11:33, Jonathan Cameron wrote:
> On 22/10/16 23:43, Peter Rosin wrote:
>> From: Jonathan Cameron <jic23@kernel.org>
>>
>> A large number of attributes can only take a limited range of values.
>> Currently in IIO this is handled by directly registering additional
>> *_available attributes thus providing this information to userspace.
>>
>> It is desirable to provide this information via the core for much the same
>> reason this was done for the actual channel information attributes in the
>> first place.  If it isn't there, then it can only really be accessed from
>> userspace.  Other in kernel IIO consumers have no access to what valid
>> parameters are.
>>
>> Two forms are currently supported:
>> * list of values in one particular IIO_VAL_* format.
>> 	e.g. 1.300000 1.500000 1.730000
>> * range specification with a step size:
>> 	e.g. [1.000000 0.500000 2.500000]
>> 	equivalent to 1.000000 1.5000000 2.000000 2.500000
>>
>> An addition set of masks are used to allow different sharing rules for the
>> *_available attributes generated.
>>
>> This allows for example:
>>
>> in_accel_x_offset
>> in_accel_y_offset
>> in_accel_offset_available.
>>
>> We could have gone with having a specification for each and every
>> info_mask element but that would have meant changing the existing userspace
>> ABI.  This approach does not.
> I'm wondering what I meant by this... where does it cause use ABI issues?
> Do you have any idea?

Nope, sorry. My memory is probably just as blank as yours :-)

> I'd forgotten I'd even done it like this rather than just insisting on 
> available for everything (which is more logical to me as we should know
> the range of everything).
> 
> It's pretty low cost though and gives a way of adding this to drivers
> piecemeal which is probably a good idea!

Yes, that's always a good sign. Flag days are a pain.

>> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
>> [rather mindlessly forward ported from approx 3 years ago /peda]
> More importantly shepherding it through review!
> 
> Naturally it's perfect code so not comments inline ;)
> 
> Thanks again for picking this up!
> 
> So... I want to see lots of comments on this. If people are happy with
> it (unlikely ;) then please say so.  It's at least a bit controversial

Hmmm, maybe I should looking over the changes line-by-line, see inline
comments...

> and adds a 'lot' of new ABI.

I'd appreciate it if someone else wrote up the common stuff in the
testing/sysfs-bus/iio file. That file is a jungle to a newcomer...

> Jonathan
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  drivers/iio/industrialio-core.c | 232 +++++++++++++++++++++++++++++++++++-----
>>  include/linux/iio/iio.h         |  11 ++
>>  include/linux/iio/types.h       |   5 +
>>  3 files changed, 223 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
>> index fc340ed3dca1..93c69ebeda1e 100644
>> --- a/drivers/iio/industrialio-core.c
>> +++ b/drivers/iio/industrialio-core.c
>> @@ -575,51 +575,41 @@ int of_iio_read_mount_matrix(const struct device *dev,
>>  #endif
>>  EXPORT_SYMBOL(of_iio_read_mount_matrix);
>>  
>> -/**
>> - * iio_format_value() - Formats a IIO value into its string representation
>> - * @buf:	The buffer to which the formatted value gets written
>> - * @type:	One of the IIO_VAL_... constants. This decides how the val
>> - *		and val2 parameters are formatted.
>> - * @size:	Number of IIO value entries contained in vals
>> - * @vals:	Pointer to the values, exact meaning depends on the
>> - *		type parameter.
>> - *
>> - * Return: 0 by default, a negative number on failure or the
>> - *	   total number of characters written for a type that belongs
>> - *	   to the IIO_VAL_... constant.
>> - */
>> -ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
>> +static ssize_t __iio_format_value(char *buf, unsigned int type,
>> +				  int size, const int *vals)
>>  {
>>  	unsigned long long tmp;
>> +	int tmp0, tmp1;
>>  	bool scale_db = false;
>>  
>>  	switch (type) {
>>  	case IIO_VAL_INT:
>> -		return sprintf(buf, "%d\n", vals[0]);
>> +		return sprintf(buf, "%d", vals[0]);

This function was previously used to format one or a perhaps a few
values presumable at the start of a page. It doesn't bother to check
for buffer overflow. That is probably very bad now that it is used
to print arbitrary length lists of values...

I'll add a suggested fix in v3.

>>  	case IIO_VAL_INT_PLUS_MICRO_DB:
>>  		scale_db = true;
>>  	case IIO_VAL_INT_PLUS_MICRO:
>>  		if (vals[1] < 0)
>> -			return sprintf(buf, "-%d.%06u%s\n", abs(vals[0]),
>> +			return sprintf(buf, "-%d.%06u%s", abs(vals[0]),
>>  				       -vals[1], scale_db ? " dB" : "");
>>  		else
>> -			return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
>> +			return sprintf(buf, "%d.%06u%s", vals[0], vals[1],
>>  				scale_db ? " dB" : "");
>>  	case IIO_VAL_INT_PLUS_NANO:
>>  		if (vals[1] < 0)
>> -			return sprintf(buf, "-%d.%09u\n", abs(vals[0]),
>> +			return sprintf(buf, "-%d.%09u", abs(vals[0]),
>>  				       -vals[1]);
>>  		else
>> -			return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
>> +			return sprintf(buf, "%d.%09u", vals[0], vals[1]);
>>  	case IIO_VAL_FRACTIONAL:
>>  		tmp = div_s64((s64)vals[0] * 1000000000LL, vals[1]);
>> -		vals[0] = (int)div_s64_rem(tmp, 1000000000, &vals[1]);
>> -		return sprintf(buf, "%d.%09u\n", vals[0], abs(vals[1]));
>> +		tmp1 = vals[1];
>> +		tmp0 = (int)div_s64_rem(tmp, 1000000000, &tmp1);
>> +		return sprintf(buf, "%d.%09u", tmp0, abs(tmp1));
>>  	case IIO_VAL_FRACTIONAL_LOG2:
>>  		tmp = (s64)vals[0] * 1000000000LL >> vals[1];
>> -		vals[1] = do_div(tmp, 1000000000LL);
>> -		vals[0] = tmp;
>> -		return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
>> +		tmp1 = do_div(tmp, 1000000000LL);
>> +		tmp0 = tmp;
>> +		return sprintf(buf, "%d.%09u", tmp0, tmp1);
>>  	case IIO_VAL_INT_MULTIPLE:
>>  	{
>>  		int i;
>> @@ -628,13 +618,34 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
>>  		for (i = 0; i < size; ++i)
>>  			len += snprintf(&buf[len], PAGE_SIZE - len, "%d ",
>>  								vals[i]);

This seems like a dangerous thing to do, arg 2 of snprintf is size_t which
is unsigned, so if one call would have overflowed the buffer, the next
will see a negative available buffer, turned very large since unsigned.
*boom*

I'll add a suggested fix in v3.

>> -		len += snprintf(&buf[len], PAGE_SIZE - len, "\n");
>>  		return len;
>>  	}
>>  	default:
>>  		return 0;
>>  	}
>>  }
>> +
>> +/**
>> + * iio_format_value() - Formats a IIO value into its string representation
>> + * @buf:	The buffer to which the formatted value gets written
>> + * @type:	One of the IIO_VAL_... constants. This decides how the val
>> + *		and val2 parameters are formatted.
>> + * @size:	Number of IIO value entries contained in vals
>> + * @vals:	Pointer to the values, exact meaning depends on the
>> + *		type parameter.
>> + *
>> + * Return: 0 by default, a negative number on failure or the
>> + *	   total number of characters written for a type that belongs
>> + *	   to the IIO_VAL_... constant.
>> + */
>> +ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals)
>> +{
>> +	ssize_t len;
>> +
>> +	len = __iio_format_value(buf, type, size, vals);
>> +
>> +	return len + sprintf(buf + len, "\n");
>> +}
>>  EXPORT_SYMBOL_GPL(iio_format_value);
>>  
>>  static ssize_t iio_read_channel_info(struct device *dev,
>> @@ -662,6 +673,113 @@ static ssize_t iio_read_channel_info(struct device *dev,
>>  	return iio_format_value(buf, ret, val_len, vals);
>>  }
>>  
>> +static ssize_t iio_format_avail_list(char *buf, const int *vals,
>> +				     int type, int length)
>> +{
>> +	int i;
>> +	ssize_t len = 0;
>> +
>> +	switch (type) {
>> +	case IIO_VAL_INT:
>> +		for (i = 0; i < length; i++) {
>> +			len += __iio_format_value(buf + len, type, 1, &vals[i]);
>> +			if (i < length - 1)
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						" ");
>> +			else
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						"\n");
>> +		}
>> +		break;
>> +	default:
>> +		for (i = 0; i < length / 2; i++) {
>> +			len += __iio_format_value(buf + len,
>> +						  type,
>> +						  2,
>> +						  &vals[i * 2]);
>> +			if (i < length / 2 - 1)
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						" ");
>> +			else
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						"\n");
>> +		}
>> +	};
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t iio_format_avail_range(char *buf, const int *vals, int type)
>> +{
>> +	int i;
>> +	ssize_t len;
>> +
>> +	len = snprintf(buf, PAGE_SIZE, "[");
>> +	switch (type) {
>> +	case IIO_VAL_INT:
>> +		for (i = 0; i < 3; i++) {
>> +			len += __iio_format_value(buf + len, type, 1, &vals[i]);
>> +			if (i < 2)
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						" ");
>> +			else
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						"]\n");
>> +		}
>> +		break;
>> +	default:
>> +		for (i = 0; i < 3; i++) {
>> +			len += __iio_format_value(buf + len,
>> +						  type,
>> +						  2,
>> +						  &vals[i * 2]);
>> +			if (i < 2)
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						" ");
>> +			else
>> +				len += snprintf(buf + len,
>> +						PAGE_SIZE - len,
>> +						"]\n");
>> +		}
>> +	};
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t iio_read_channel_info_avail(struct device *dev,
>> +					   struct device_attribute *attr,
>> +					   char *buf)
>> +{
>> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> +	const int *vals;
>> +	int ret;
>> +	int length;
>> +	int type;
>> +
>> +	ret = indio_dev->info->read_avail(indio_dev, this_attr->c,
>> +					  &vals, &type, &length,
>> +					  this_attr->address);
>> +
>> +	if (ret < 0)
>> +		return ret;
>> +	switch (ret) {
>> +	case IIO_AVAIL_LIST:
>> +		return iio_format_avail_list(buf, vals, type, length);
>> +	case IIO_AVAIL_RANGE:
>> +		return iio_format_avail_range(buf, vals, type);
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>>  /**
>>   * iio_str_to_fixpoint() - Parse a fixed-point number from a string
>>   * @str: The string to parse
>> @@ -978,6 +1096,40 @@ static int iio_device_add_info_mask_type(struct iio_dev *indio_dev,
>>  	return attrcount;
>>  }
>>  
>> +static int iio_device_add_info_mask_type_avail(struct iio_dev *indio_dev,
>> +					       struct iio_chan_spec const *chan,
>> +					       enum iio_shared_by shared_by,
>> +					       const long *infomask)
>> +{
>> +	int i, ret, attrcount = 0;
>> +	char *avail_postfix;
>> +
>> +	for_each_set_bit(i, infomask, sizeof(infomask) * 8) {
>> +		avail_postfix = kasprintf(GFP_KERNEL,
>> +					  "%s_available",
>> +					  iio_chan_info_postfix[i]);
>> +		if (!avail_postfix)
>> +			return -ENOMEM;
>> +
>> +		ret = __iio_add_chan_devattr(avail_postfix,
>> +					     chan,
>> +					     &iio_read_channel_info_avail,
>> +					     NULL,
>> +					     i,
>> +					     shared_by,
>> +					     &indio_dev->dev,
>> +					     &indio_dev->channel_attr_list);
>> +		kfree(avail_postfix);
>> +		if ((ret == -EBUSY) && (shared_by != IIO_SEPARATE))
>> +			continue;
>> +		else if (ret < 0)
>> +			return ret;
>> +		attrcount++;
>> +	}
>> +
>> +	return attrcount;
>> +}
>> +
>>  static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  					struct iio_chan_spec const *chan)
>>  {
>> @@ -993,6 +1145,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  		return ret;
>>  	attrcount += ret;
>>  
>> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
>> +						  IIO_SEPARATE,
>> +						  &chan->
>> +						  info_mask_separate_available);
>> +	if (ret < 0)
>> +		return ret;
>> +	attrcount += ret;
>> +
>>  	ret = iio_device_add_info_mask_type(indio_dev, chan,
>>  					    IIO_SHARED_BY_TYPE,
>>  					    &chan->info_mask_shared_by_type);
>> @@ -1000,6 +1160,14 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  		return ret;
>>  	attrcount += ret;
>>  
>> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
>> +						  IIO_SHARED_BY_TYPE,
>> +						  &chan->
>> +						  info_mask_shared_by_type_available);
>> +	if (ret < 0)
>> +		return ret;
>> +	attrcount += ret;
>> +
>>  	ret = iio_device_add_info_mask_type(indio_dev, chan,
>>  					    IIO_SHARED_BY_DIR,
>>  					    &chan->info_mask_shared_by_dir);
>> @@ -1007,6 +1175,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  		return ret;
>>  	attrcount += ret;
>>  
>> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
>> +						  IIO_SHARED_BY_DIR,
>> +						  &chan->info_mask_shared_by_dir_available);
>> +	if (ret < 0)
>> +		return ret;
>> +	attrcount += ret;
>> +
>>  	ret = iio_device_add_info_mask_type(indio_dev, chan,
>>  					    IIO_SHARED_BY_ALL,
>>  					    &chan->info_mask_shared_by_all);
>> @@ -1014,6 +1189,13 @@ static int iio_device_add_channel_sysfs(struct iio_dev *indio_dev,
>>  		return ret;
>>  	attrcount += ret;
>>  
>> +	ret = iio_device_add_info_mask_type_avail(indio_dev, chan,
>> +						  IIO_SHARED_BY_ALL,
>> +						  &chan->info_mask_shared_by_all_available);
>> +	if (ret < 0)
>> +		return ret;
>> +	attrcount += ret;
>> +
>>  	if (chan->ext_info) {
>>  		unsigned int i = 0;
>>  		for (ext_info = chan->ext_info; ext_info->name; ext_info++) {
>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index b4a0679e4a49..c0f897084620 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -269,9 +269,13 @@ struct iio_chan_spec {
>>  		enum iio_endian endianness;
>>  	} scan_type;
>>  	long			info_mask_separate;
>> +	long			info_mask_separate_available;
>>  	long			info_mask_shared_by_type;
>> +	long			info_mask_shared_by_type_available;
>>  	long			info_mask_shared_by_dir;
>> +	long			info_mask_shared_by_dir_available;
>>  	long			info_mask_shared_by_all;
>> +	long			info_mask_shared_by_all_available;
>>  	const struct iio_event_spec *event_spec;
>>  	unsigned int		num_event_specs;
>>  	const struct iio_chan_spec_ext_info *ext_info;
>> @@ -397,6 +401,13 @@ struct iio_info {
>>  			int *val_len,
>>  			long mask);
>>  
>> +	int (*read_avail)(struct iio_dev *indio_dev,
>> +			  struct iio_chan_spec const *chan,
>> +			  const int **vals,
>> +			  int *type,
>> +			  int *length,
>> +			  long mask_el);
>> +
>>  	int (*write_raw)(struct iio_dev *indio_dev,
>>  			 struct iio_chan_spec const *chan,
>>  			 int val,
>> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
>> index 32b579525004..2aa7b6384d64 100644
>> --- a/include/linux/iio/types.h
>> +++ b/include/linux/iio/types.h
>> @@ -29,4 +29,9 @@ enum iio_event_info {
>>  #define IIO_VAL_FRACTIONAL 10
>>  #define IIO_VAL_FRACTIONAL_LOG2 11
>>  
>> +enum iio_available_type {
>> +	IIO_AVAIL_LIST,
>> +	IIO_AVAIL_RANGE,
>> +};
>> +
>>  #endif /* _IIO_TYPES_H_ */
>>
> 

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

end of thread, other threads:[~2016-10-23 22:27 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-22 22:43 [PATCH v2 0/7] IIO wrapper drivers, dpot-dac and envelope-detector Peter Rosin
2016-10-22 22:43 ` [PATCH v2 1/7] iio:core: add a callback to allow drivers to provide _available attributes Peter Rosin
2016-10-23  9:33   ` Jonathan Cameron
2016-10-23 22:10     ` Peter Rosin
2016-10-22 22:43 ` [PATCH v2 2/7] iio: mcp4531: provide range of available raw values Peter Rosin
2016-10-23  9:36   ` Jonathan Cameron
2016-10-22 22:43 ` [PATCH v2 3/7] dt-bindings: add axentia to vendor-prefixes Peter Rosin
2016-10-22 22:43 ` [PATCH v2 4/7] dt-bindings: iio: document dpot-dac bindings Peter Rosin
2016-10-23  8:02   ` Peter Meerwald-Stadler
2016-10-22 22:43 ` [PATCH v2 5/7] iio: dpot-dac: DAC driver based on a digital potentiometer Peter Rosin
2016-10-23  9:42   ` Jonathan Cameron
2016-10-22 22:43 ` [PATCH v2 6/7] dt-bindings: iio: document envelope-detector bindings Peter Rosin
2016-10-23  9:01   ` Peter Rosin
2016-10-22 22:43 ` [PATCH v2 7/7] iio: envelope-detector: ADC driver based on a DAC and a comparator Peter Rosin
2016-10-23  9:48   ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).