[PATCH v4 0/8] IIO wrapper drivers, dpot-dac and envelope-detector

[PATCH v4 0/8] IIO wrapper drivers, dpot-dac and envelope-detector

[Peter Rosin]

Hi!

This is a respin with a handful of nitpicks fixed from v3. No major
changes. And a couple of acks was added too, thanks! I also added
Thomas Gleixner as Cc, since Jonathan was hoping for some comments
on the somewhat odd interrupt handling in patch 8/8 (but perhaps
plumbers-week isn't the best week to hope for that).


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.

One thing I still 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. The only way out of this
is a devicetree thing. I suppose it will have to be added by whomever needs
it whenever that is...

I also wonder if the "new" *_available ABI should perhaps be documented
for all variants directly in sysfs-bus-iio instead of doing it in a driver
specific maner that I did? But that can be fixed later by someone more
capable than me :-)

v3 -> v4
- gained acks from Rob for the three bindings patches
- gained ack from Daniel for the core _available patch (1/8)
- dropped the type argument from iio_read_avail_channel_raw(), since
  raw values are assumed to be of type IIO_VAL_INT anyway
- add some more words about what iio_read_avail_channel_raw() does
- rebased onto v4.9-rc4

dpot-dac:
- adjust to changed signature of iio_read_avail_channel_raw()
- one instance of s/the channels supports/the channel supports/
- drop surplus s64 cast in dpot_dac_channel_max_ohms()

envelope-detector
- the envelope-detector module is called envelope-detector


v2 -> v3
- add some missing @foo comments in iio.h in the initial forward ported patch
- killed some buffer overflow problems in the initial forward ported patch
- add inkern.c helpers for the new available attributes to avoid viral
  boilerplating in the future (also add support for finding max of
  IIO_AVAIL_LISTs of IIO_VAL_INTs)
- add ABI docs for new ABI of mcp4531 (out_resistance_raw_available)
  and an example in the commit message

dpot-dac:
- two more counts of s/assumed the that the/assumed that the/   *blush*
- add ABI docs for new ABI (out_voltageY_raw_available)

envelope detector:
- move device attributes 'compare_interval_ms' and 'invert' to extended
  channel attributes (out_altvoltage0_compare_interval and
  out_altvoltage0_invert) as they really are about the channel and not
  the device
- kill "dpot-dac,max-ohms = <100000>;" in the devicetree example


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

Cheers,
Peter

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

Peter Rosin (7):
  iio: inkern: add helpers to query available values from channels
  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 ++
 .../ABI/testing/sysfs-bus-iio-dac-dpot-dac         |   8 +
 .../testing/sysfs-bus-iio-potentiometer-mcp4531    |   8 +
 .../bindings/iio/adc/envelope-detector.txt         |  54 +++
 .../devicetree/bindings/iio/dac/dpot-dac.txt       |  41 ++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 MAINTAINERS                                        |  17 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/envelope-detector.c                | 422 +++++++++++++++++++++
 drivers/iio/dac/Kconfig                            |  10 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/dpot-dac.c                         | 266 +++++++++++++
 drivers/iio/industrialio-core.c                    | 259 +++++++++++--
 drivers/iio/inkern.c                               | 104 +++++
 drivers/iio/potentiometer/mcp4531.c                | 104 +++--
 include/linux/iio/consumer.h                       |  28 ++
 include/linux/iio/iio.h                            |  46 +++
 include/linux/iio/types.h                          |   5 +
 19 files changed, 1346 insertions(+), 75 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
 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

[PATCH v4 1/8] iio:core: add a callback to allow drivers to provide _available attributes

[Peter Rosin]

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>
[forward ported, added some docs and fixed buffer overflows /peda]
Acked-by: Daniel Baluta <daniel.baluta@intel.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/iio/industrialio-core.c | 259 +++++++++++++++++++++++++++++++++++-----
 include/linux/iio/iio.h         |  29 +++++
 include/linux/iio/types.h       |   5 +
 3 files changed, 260 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index fc340ed3dca1..b35c61a31fa6 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -575,66 +575,82 @@ 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, size_t len, 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 snprintf(buf, len, "%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]),
-				       -vals[1], scale_db ? " dB" : "");
+			return snprintf(buf, len, "-%d.%06u%s", abs(vals[0]),
+					-vals[1], scale_db ? " dB" : "");
 		else
-			return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
-				scale_db ? " dB" : "");
+			return snprintf(buf, len, "%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]),
-				       -vals[1]);
+			return snprintf(buf, len, "-%d.%09u", abs(vals[0]),
+					-vals[1]);
 		else
-			return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
+			return snprintf(buf, len, "%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 snprintf(buf, len, "%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 snprintf(buf, len, "%d.%09u", tmp0, tmp1);
 	case IIO_VAL_INT_MULTIPLE:
 	{
 		int i;
-		int len = 0;
+		int l = 0;
 
-		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;
+		for (i = 0; i < size; ++i) {
+			l += snprintf(&buf[l], len - l, "%d ", vals[i]);
+			if (l >= len)
+				break;
+		}
+		return l;
 	}
 	default:
 		return 0;
 	}
 }
+
+/**
+ * iio_format_value() - Formats a IIO value into its string representation
+ * @buf:	The buffer to which the formatted value gets written
+ *		which is assumed to be big enough (i.e. PAGE_SIZE).
+ * @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, PAGE_SIZE, type, size, vals);
+	if (len >= PAGE_SIZE - 1)
+		return -EFBIG;
+
+	return len + sprintf(buf + len, "\n");
+}
 EXPORT_SYMBOL_GPL(iio_format_value);
 
 static ssize_t iio_read_channel_info(struct device *dev,
@@ -662,6 +678,119 @@ 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, PAGE_SIZE - len,
+						  type, 1, &vals[i]);
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+			if (i < length - 1)
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						"\n");
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+		}
+		break;
+	default:
+		for (i = 0; i < length / 2; i++) {
+			len += __iio_format_value(buf + len, PAGE_SIZE - len,
+						  type, 2, &vals[i * 2]);
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+			if (i < length / 2 - 1)
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						"\n");
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+		}
+	};
+
+	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, PAGE_SIZE - len,
+						  type, 1, &vals[i]);
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+			if (i < 2)
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						"]\n");
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+		}
+		break;
+	default:
+		for (i = 0; i < 3; i++) {
+			len += __iio_format_value(buf + len, PAGE_SIZE - len,
+						  type, 2, &vals[i * 2]);
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+			if (i < 2)
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						" ");
+			else
+				len += snprintf(buf + len, PAGE_SIZE - len,
+						"]\n");
+			if (len >= PAGE_SIZE)
+				return -EFBIG;
+		}
+	};
+
+	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 +1107,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 +1156,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 +1171,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 +1186,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 +1200,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..45b781084a4b 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -225,12 +225,22 @@ struct iio_event_spec {
  *			endianness:	little or big endian
  * @info_mask_separate: What information is to be exported that is specific to
  *			this channel.
+ * @info_mask_separate_available: What availability information is to be
+ *			exported that is specific to this channel.
  * @info_mask_shared_by_type: What information is to be exported that is shared
  *			by all channels of the same type.
+ * @info_mask_shared_by_type_available: What availability information is to be
+ *			exported that is shared by all channels of the same
+ *			type.
  * @info_mask_shared_by_dir: What information is to be exported that is shared
  *			by all channels of the same direction.
+ * @info_mask_shared_by_dir_available: What availability information is to be
+ *			exported that is shared by all channels of the same
+ *			direction.
  * @info_mask_shared_by_all: What information is to be exported that is shared
  *			by all channels.
+ * @info_mask_shared_by_all_available: What availability information is to be
+ *			exported that is shared by all channels.
  * @event_spec:		Array of events which should be registered for this
  *			channel.
  * @num_event_specs:	Size of the event_spec array.
@@ -269,9 +279,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;
@@ -349,6 +363,14 @@ struct iio_dev;
  *			max_len specifies maximum number of elements
  *			vals pointer can contain. val_len is used to return
  *			length of valid elements in vals.
+ * @read_avail:		function to return the available values from the device.
+ *			mask specifies which value. Note 0 means the available
+ *			values for the channel in question.  Return value
+ *			specifies if a IIO_AVAIL_LIST or a IIO_AVAIL_RANGE is
+ *			returned in vals. The type of the vals are returned in
+ *			type and the number of vals is returned in length. For
+ *			ranges, there are always three vals returned; min, step
+ *			and max. For lists, all possible values are enumerated.
  * @write_raw:		function to write a value to the device.
  *			Parameters are the same as for read_raw.
  * @write_raw_get_fmt:	callback function to query the expected
@@ -397,6 +419,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);
+
 	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

[PATCH v4 2/8] iio: inkern: add helpers to query available values from channels

[Peter Rosin]

Specifically a helper for reading the available maximum raw value of a
channel and a helper for forwarding read_avail requests for raw values
from one iio driver to an iio channel that is consumed.

These rather specific helpers are in turn built with generic helpers
making it easy to build more helpers for available values as needed.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/iio/inkern.c         | 104 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/consumer.h |  28 ++++++++++++
 include/linux/iio/iio.h      |  17 +++++++
 3 files changed, 149 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c4757e6367e7..cfca17ba2535 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -703,6 +703,110 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val, int *val2)
 }
 EXPORT_SYMBOL_GPL(iio_read_channel_scale);
 
+static int iio_channel_read_avail(struct iio_channel *chan,
+				  const int **vals, int *type, int *length,
+				  enum iio_chan_info_enum info)
+{
+	if (!iio_channel_has_available(chan->channel, info))
+		return -EINVAL;
+
+	return chan->indio_dev->info->read_avail(chan->indio_dev, chan->channel,
+						 vals, type, length, info);
+}
+
+int iio_read_avail_channel_raw(struct iio_channel *chan,
+			       const int **vals, int *length)
+{
+	int ret;
+	int type;
+
+	mutex_lock(&chan->indio_dev->info_exist_lock);
+	if (!chan->indio_dev->info) {
+		ret = -ENODEV;
+		goto err_unlock;
+	}
+
+	ret = iio_channel_read_avail(chan,
+				     vals, &type, length, IIO_CHAN_INFO_RAW);
+err_unlock:
+	mutex_unlock(&chan->indio_dev->info_exist_lock);
+
+	if (ret >= 0 && type != IIO_VAL_INT) {
+		/* raw values are assumed to be IIO_VAL_INT */
+		ret = -EINVAL;
+		goto err_unlock;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iio_read_avail_channel_raw);
+
+static int iio_channel_read_max(struct iio_channel *chan,
+				int *val, int *val2, int *type,
+				enum iio_chan_info_enum info)
+{
+	int unused;
+	const int *vals;
+	int length;
+	int ret;
+
+	if (!val2)
+		val2 = &unused;
+
+	ret = iio_channel_read_avail(chan, &vals, type, &length, info);
+	switch (ret) {
+	case IIO_AVAIL_RANGE:
+		switch (*type) {
+		case IIO_VAL_INT:
+			*val = vals[2];
+			break;
+		default:
+			*val = vals[4];
+			*val2 = vals[5];
+		}
+		return 0;
+
+	case IIO_AVAIL_LIST:
+		if (length <= 0)
+			return -EINVAL;
+		switch (*type) {
+		case IIO_VAL_INT:
+			*val = vals[--length];
+			while (length) {
+				if (vals[--length] > *val)
+					*val = vals[length];
+			}
+			break;
+		default:
+			/* FIXME: learn about max for other iio values */
+			return -EINVAL;
+		}
+		return 0;
+
+	default:
+		return ret;
+	}
+}
+
+int iio_read_max_channel_raw(struct iio_channel *chan, int *val)
+{
+	int ret;
+	int type;
+
+	mutex_lock(&chan->indio_dev->info_exist_lock);
+	if (!chan->indio_dev->info) {
+		ret = -ENODEV;
+		goto err_unlock;
+	}
+
+	ret = iio_channel_read_max(chan, val, NULL, &type, IIO_CHAN_INFO_RAW);
+err_unlock:
+	mutex_unlock(&chan->indio_dev->info_exist_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iio_read_max_channel_raw);
+
 int iio_get_channel_type(struct iio_channel *chan, enum iio_chan_type *type)
 {
 	int ret = 0;
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 9edccfba1ffb..9a4f336d8b4a 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -226,6 +226,34 @@ int iio_read_channel_processed(struct iio_channel *chan, int *val);
 int iio_write_channel_raw(struct iio_channel *chan, int val);
 
 /**
+ * iio_read_max_channel_raw() - read maximum available raw value from a given
+ *				channel, i.e. the maximum possible value.
+ * @chan:		The channel being queried.
+ * @val:		Value read back.
+ *
+ * Note raw reads from iio channels are in adc counts and hence
+ * scale will need to be applied if standard units are required.
+ */
+int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
+
+/**
+ * iio_read_avail_channel_raw() - read available raw values from a given channel
+ * @chan:		The channel being queried.
+ * @vals:		Available values read back.
+ * @length:		Number of entries in vals.
+ *
+ * Returns an error code, IIO_AVAIL_RANGE or IIO_AVAIL_LIST.
+ *
+ * For ranges, three vals are always returned; min, step and max.
+ * For lists, all the possible values are enumerated.
+ *
+ * Note raw available values from iio channels are in adc counts and
+ * hence scale will need to be applied if standard units are required.
+ */
+int iio_read_avail_channel_raw(struct iio_channel *chan,
+			       const int **vals, int *length);
+
+/**
  * iio_get_channel_type() - get the type of a channel
  * @channel:		The channel being queried.
  * @type:		The type of the channel.
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 45b781084a4b..f9be8836ef23 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -315,6 +315,23 @@ static inline bool iio_channel_has_info(const struct iio_chan_spec *chan,
 		(chan->info_mask_shared_by_all & BIT(type));
 }
 
+/**
+ * iio_channel_has_available() - Checks if a channel has an available attribute
+ * @chan: The channel to be queried
+ * @type: Type of the available attribute to be checked
+ *
+ * Returns true if the channel supports reporting available values for the
+ * given attribute type, false otherwise.
+ */
+static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
+					     enum iio_chan_info_enum type)
+{
+	return (chan->info_mask_separate_available & BIT(type)) |
+		(chan->info_mask_shared_by_type_available & BIT(type)) |
+		(chan->info_mask_shared_by_dir_available & BIT(type)) |
+		(chan->info_mask_shared_by_all_available & BIT(type));
+}
+
 #define IIO_CHAN_SOFT_TIMESTAMP(_si) {					\
 	.type = IIO_TIMESTAMP,						\
 	.channel = -1,							\
-- 
2.1.4

[PATCH v4 3/8] iio: mcp4531: provide range of available raw values

[Peter Rosin]

Example:

$ cat '/sys/bus/iio/devices/iio:device0/out_resistance_raw_available'
[0 1 256]

Meaning: min 0, step 1 and max 256.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../testing/sysfs-bus-iio-potentiometer-mcp4531    |   8 ++
 MAINTAINERS                                        |   1 +
 drivers/iio/potentiometer/mcp4531.c                | 104 ++++++++++++---------
 3 files changed, 71 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531 b/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
new file mode 100644
index 000000000000..2a91fbe394fc
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
@@ -0,0 +1,8 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_resistance_raw_available
+Date:		October 2016
+KernelVersion:	4.9
+Contact:	Peter Rosin <peda@axentia.se>
+Description:
+		The range of available values represented as the minimum value,
+		the step and the maximum value, all enclosed in square brackets.
+		Example: [0 1 256]
diff --git a/MAINTAINERS b/MAINTAINERS
index 411e3b87b8c2..2409e1d79cb6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7707,6 +7707,7 @@ MCP4531 MICROCHIP DIGITAL POTENTIOMETER DRIVER
 M:	Peter Rosin <peda@axentia.se>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
 F:	drivers/iio/potentiometer/mcp4531.c
 
 MEASUREMENT COMPUTING CIO-DAC IIO DRIVER
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

[PATCH v4 4/8] dt-bindings: add axentia to vendor-prefixes

[Peter Rosin]

Acked-by: Rob Herring <robh@kernel.org>
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

[PATCH v4 5/8] dt-bindings: iio: document dpot-dac bindings

[Peter Rosin]

Acked-by: Rob Herring <robh@kernel.org>
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..fdf47a01bfef
--- /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 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 2409e1d79cb6..4ff9c12b1313 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6124,6 +6124,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

[PATCH v4 6/8] iio: dpot-dac: DAC driver based on a digital potentiometer

[Peter Rosin]

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
                  |      |
                  '------+------------+

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../ABI/testing/sysfs-bus-iio-dac-dpot-dac         |   8 +
 MAINTAINERS                                        |   2 +
 drivers/iio/dac/Kconfig                            |  10 +
 drivers/iio/dac/Makefile                           |   1 +
 drivers/iio/dac/dpot-dac.c                         | 266 +++++++++++++++++++++
 5 files changed, 287 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
 create mode 100644 drivers/iio/dac/dpot-dac.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
new file mode 100644
index 000000000000..580e93f373f6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
@@ -0,0 +1,8 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_raw_available
+Date:		October 2016
+KernelVersion:	4.9
+Contact:	Peter Rosin <peda@axentia.se>
+Description:
+		The range of available values represented as the minimum value,
+		the step and the maximum value, all enclosed in square brackets.
+		Example: [0 1 256]
diff --git a/MAINTAINERS b/MAINTAINERS
index 4ff9c12b1313..583c6c93b6f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6128,7 +6128,9 @@ IIO DIGITAL POTENTIOMETER DAC
 M:	Peter Rosin <peda@axentia.se>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
 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..eb1f0509a3ec
--- /dev/null
+++ b/drivers/iio/dac/dpot-dac.c
@@ -0,0 +1,266 @@
+/*
+ * 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);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*type = IIO_VAL_INT;
+		return iio_read_avail_channel_raw(dac->dpot, vals, length);
+	}
+
+	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);
+	unsigned long long tmp;
+	int ret;
+	int val;
+	int val2;
+	int max;
+
+	ret = iio_read_max_channel_raw(dac->dpot, &max);
+	if (ret < 0) {
+		dev_err(dev, "dpot does not indicate its raw maximum value\n");
+		return ret;
+	}
+
+	switch (iio_read_channel_scale(dac->dpot, &val, &val2)) {
+	case IIO_VAL_INT:
+		return max * val;
+	case IIO_VAL_FRACTIONAL:
+		tmp = (unsigned long long)max * val;
+		do_div(tmp, val2);
+		return tmp;
+	case IIO_VAL_FRACTIONAL_LOG2:
+		tmp = val * 1000000000LL * max >> val2;
+		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;
+
+	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

[PATCH v4 7/8] dt-bindings: iio: document envelope-detector bindings

[Peter Rosin]

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../bindings/iio/adc/envelope-detector.txt         | 54 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 60 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..27544bdd4478
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
@@ -0,0 +1,54 @@
+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";
+		#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 583c6c93b6f3..0e13066ca3a2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6132,6 +6132,12 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
 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

[PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Peter Rosin]

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                | 422 +++++++++++++++++++++
 5 files changed, 471 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..2071f9bcfaa5
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
@@ -0,0 +1,36 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_altvoltageY_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/in_altvoltageY_compare_interval
+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 0e13066ca3a2..539b20baf791 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6136,7 +6136,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 99c051490eff..de4030894f23 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 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..fef15c0d7c9c
--- /dev/null
+++ b/drivers/iio/adc/envelope-detector.c
@@ -0,0 +1,422 @@
+/*
+ * 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 ssize_t envelope_show_invert(struct iio_dev *indio_dev,
+				    uintptr_t private,
+				    struct iio_chan_spec const *ch, char *buf)
+{
+	struct envelope *env = iio_priv(indio_dev);
+
+	return sprintf(buf, "%u\n", env->invert);
+}
+
+static ssize_t envelope_store_invert(struct iio_dev *indio_dev,
+				     uintptr_t private,
+				     struct iio_chan_spec const *ch,
+				     const char *buf, size_t len)
+{
+	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 ssize_t envelope_show_comp_interval(struct iio_dev *indio_dev,
+					   uintptr_t private,
+					   struct iio_chan_spec const *ch,
+					   char *buf)
+{
+	struct envelope *env = iio_priv(indio_dev);
+
+	return sprintf(buf, "%u\n", env->comp_interval);
+}
+
+static ssize_t envelope_store_comp_interval(struct iio_dev *indio_dev,
+					    uintptr_t private,
+					    struct iio_chan_spec const *ch,
+					    const char *buf, size_t len)
+{
+	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 const struct iio_chan_spec_ext_info envelope_detector_ext_info[] = {
+	{ .name = "invert",
+	  .read = envelope_show_invert,
+	  .write = envelope_store_invert, },
+	{ .name = "compare_interval",
+	  .read = envelope_show_comp_interval,
+	  .write = envelope_store_comp_interval, },
+	{ /* sentinel */ }
+};
+
+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),
+	.ext_info = envelope_detector_ext_info,
+	.indexed = 1,
+};
+
+static const struct iio_info envelope_detector_info = {
+	.read_raw = &envelope_detector_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+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 = iio_read_max_channel_raw(env->dac, &env->dac_max);
+	if (ret < 0) {
+		dev_err(dev, "dac does not indicate its raw maximum value\n");
+		return 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

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Thomas Gleixner]

On Tue, 8 Nov 2016, Peter Rosin wrote:
> +/*
> + * 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);

So you need that whole dance including the delayed work because you cannot
call iio_write_channel_raw() from hard interrupt context, right?

So you might just register a threaded interrupt handler, which should make
this whole thing way simpler.

     devm_request_threaded_irq(dev, irq, NULL, your_isr, IRQF_ONESHOT, ...);

The core will mask the interrupt line until the threaded handler is
finished. The threaded handler is invoked with preemption enabled, so you
can sleep there as long as you want. So you can do everything in your
handler and the above dance is just not required.

Thanks,

	tglx

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Peter Rosin]

On 2016-11-08 16:59, Thomas Gleixner wrote:
> On Tue, 8 Nov 2016, Peter Rosin wrote:
>> +/*
>> + * 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);
> 
> So you need that whole dance including the delayed work because you cannot
> call iio_write_channel_raw() from hard interrupt context, right?

It's not the "cannot call from hard irq context" that made me do that, it's...

> So you might just register a threaded interrupt handler, which should make
> this whole thing way simpler.
> 
>      devm_request_threaded_irq(dev, irq, NULL, your_isr, IRQF_ONESHOT, ...);
> 
> The core will mask the interrupt line until the threaded handler is
> finished. The threaded handler is invoked with preemption enabled, so you
> can sleep there as long as you want. So you can do everything in your
> handler and the above dance is just not required.

...that I couldn't work out how to reenable a oneshot irq once it had fired,
short of freeing the irq and requesting it again. That seemed entirely
bogus, the driver shouldn't risk losing a resource like that so I don't know
what I didn't see? Or maybe it was that I had a hard time resolving the race
between the irq and the timeout in a nice way. I honestly don't remember
why exactly I abandoned oneshot irqs, but this enable/sync/enable dance
was much nicer than what I came up with for the oneshot irq solution I
originally worked on.

Or maybe I had problems with the possibly pending irq also when using a
oneshot irq, but didn't realize it? That was something I discovered quite
late in the process, some time after moving away from oneshot irqs. Are
pending irqs cleared when requesting (or reenabling, however that is done)
a oneshot irq?

Anyway, I do not want the interrupt to be serviced when no one is interested,
since I'm afraid that nasty input might generate a flood of interrupts that
might disturb other things that the cpu is doing. Which means that I need
to enable/disable the interrupt as needed.

However, what *I* thought Jonathan wanted input on was the part where the
interrupt edge/level is flipped when requesting "inverted" signals in
envelope_store_invert(). That could perhaps be seen as unorthodox and in
need of more eyes?

Cheers,
Peter

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Thomas Gleixner]

On Tue, 8 Nov 2016, Peter Rosin wrote:
> On 2016-11-08 16:59, Thomas Gleixner wrote:

> > So you need that whole dance including the delayed work because you cannot
> > call iio_write_channel_raw() from hard interrupt context, right?
> 
> It's not the "cannot call from hard irq context" that made me do that, it's..

Well, what guarantees you that the DAC is writeable from IRQ context? It
might be hanging off an i2c/spi bus as well....

> > The core will mask the interrupt line until the threaded handler is
> > finished. The threaded handler is invoked with preemption enabled, so you
> > can sleep there as long as you want. So you can do everything in your
> > handler and the above dance is just not required.
> 
> ...that I couldn't work out how to reenable a oneshot irq once it had fired,
> short of freeing the irq and requesting it again. That seemed entirely
> bogus, the driver shouldn't risk losing a resource like that so I don't know
> what I didn't see? Or maybe it was that I had a hard time resolving the race
> between the irq and the timeout in a nice way. I honestly don't remember
> why exactly I abandoned oneshot irqs, but this enable/sync/enable dance
> was much nicer than what I came up with for the oneshot irq solution I
> originally worked on.

Threaded ONESHOT irqs work this way:

 interrupt fires
   mask interrupt
   handler thread is woken
 
 handler thread runs
   invokes isr
   unmask interrupt

So if you rewrite the DAC to the new value in your ISR, then you should not
get any spurious interrupt.

Note, that this only works for level type interrupts.

We do not mask edge type interrupts as we might lose an edge, but if that
helps the cause of your problem it's simple enough to make it conditionally
doing so in the core.

> Or maybe I had problems with the possibly pending irq also when using a
> oneshot irq, but didn't realize it? That was something I discovered quite
> late in the process, some time after moving away from oneshot irqs. Are
> pending irqs cleared when requesting (or reenabling, however that is done)
> a oneshot irq?

Pending irqs are only replayed, when you reenable an interrupt via
enable_irq(). That can happen either by software or by hardware.

> Anyway, I do not want the interrupt to be serviced when no one is interested,
> since I'm afraid that nasty input might generate a flood of interrupts that
> might disturb other things that the cpu is doing. Which means that I need
> to enable/disable the interrupt as needed.

So the main issue I'm seing here, is that your comparator does not have
means to prevent it from firing interrupts.

> However, what *I* thought Jonathan wanted input on was the part where the
> interrupt edge/level is flipped when requesting "inverted" signals in
> envelope_store_invert(). That could perhaps be seen as unorthodox and in
> need of more eyes?

Flipping the dectection level of the interrupt is fine, but what's the
guarantee that it is correct in the first place? I don't see anything which
makes that sure at all. Aside of that this bit does not makes sense:

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

What's the |= about?

> +	if (env->comp_irq_trigger & IRQF_TRIGGER_FALLING)

and this should be 'else if'. If the interrupt is configured for both
edges, which is possible with some interrupt controllers then the whole
thing does not work at all.

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

Thanks,

	tglx

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Peter Rosin]

On 2016-11-08 19:38, Thomas Gleixner wrote:
> On Tue, 8 Nov 2016, Peter Rosin wrote:
>> On 2016-11-08 16:59, Thomas Gleixner wrote:
> 
>>> So you need that whole dance including the delayed work because you cannot
>>> call iio_write_channel_raw() from hard interrupt context, right?
>>
>> It's not the "cannot call from hard irq context" that made me do that, it's..
> 
> Well, what guarantees you that the DAC is writeable from IRQ context? It
> might be hanging off an i2c/spi bus as well....

Right, the DAC is actually on an i2c-bus so it's not possible to call
iio_write_channel_raw in hard irq context, I did not disagree with that.
But that was not the reason for not calling it in (hard or threaded) irq
context. The reason is that this was the simplest way to kill the race
with the timeout, and I wasn't in that much of a hurry to start the next
step (in the binary search) and was completely happy with just recording
if any interrupt did happen and then continue when the timeout eventually
happened. It's not like the delayed work connected to the timeout can be
left out anyway, since the design is to not get an interrupt before the
timeout in about half the cases. In some cases there might not be any
interrupts in eons...

>>> The core will mask the interrupt line until the threaded handler is
>>> finished. The threaded handler is invoked with preemption enabled, so you
>>> can sleep there as long as you want. So you can do everything in your
>>> handler and the above dance is just not required.
>>
>> ...that I couldn't work out how to reenable a oneshot irq once it had fired,
>> short of freeing the irq and requesting it again. That seemed entirely
>> bogus, the driver shouldn't risk losing a resource like that so I don't know
>> what I didn't see? Or maybe it was that I had a hard time resolving the race
>> between the irq and the timeout in a nice way. I honestly don't remember
>> why exactly I abandoned oneshot irqs, but this enable/sync/enable dance
>> was much nicer than what I came up with for the oneshot irq solution I
>> originally worked on.
> 
> Threaded ONESHOT irqs work this way:
> 
>  interrupt fires
>    mask interrupt
>    handler thread is woken
>  
>  handler thread runs
>    invokes isr
>    unmask interrupt
> 
> So if you rewrite the DAC to the new value in your ISR, then you should not
> get any spurious interrupt.
> 
> Note, that this only works for level type interrupts.
> 
> We do not mask edge type interrupts as we might lose an edge, but if that
> helps the cause of your problem it's simple enough to make it conditionally
> doing so in the core.
> 
>> Or maybe I had problems with the possibly pending irq also when using a
>> oneshot irq, but didn't realize it? That was something I discovered quite
>> late in the process, some time after moving away from oneshot irqs. Are
>> pending irqs cleared when requesting (or reenabling, however that is done)
>> a oneshot irq?
> 
> Pending irqs are only replayed, when you reenable an interrupt via
> enable_irq(). That can happen either by software or by hardware.

Ah, of course, the interrupt core does its best to not lose interrupts, but
in this case it is actually desired to lose the interrupts that happen
while the irq is disabled. Which means that the enable/sync/enable-dance is
needed for oneshot interrupts as well. Either that or make tweaks to the
core (yes, irqs are edge-triggered interrupts in my case).

>> Anyway, I do not want the interrupt to be serviced when no one is interested,
>> since I'm afraid that nasty input might generate a flood of interrupts that
>> might disturb other things that the cpu is doing. Which means that I need
>> to enable/disable the interrupt as needed.
> 
> So the main issue I'm seing here, is that your comparator does not have
> means to prevent it from firing interrupts.

Right, it's just a discrete op-amp. Only way to "turn it off", is to feed
it input that makes it silent. But I'd rather not have code in this driver
that knows how to do that, since then the driver mutates from something
fairly generic to something that is very specific with hairy dependencies.
It would probably also require code to handle trailing interrupts caused
by setting up the silent state. It just sounds horrible compared to
simply disabling the interrupt (and doing a dance when enabling them).

>> However, what *I* thought Jonathan wanted input on was the part where the
>> interrupt edge/level is flipped when requesting "inverted" signals in
>> envelope_store_invert(). That could perhaps be seen as unorthodox and in
>> need of more eyes?
> 
> Flipping the dectection level of the interrupt is fine, but what's the
> guarantee that it is correct in the first place? I don't see anything which
> makes that sure at all. Aside of that this bit does not makes sense:

That "guarantee" comes from devicetree. I.e. the person writing the
dts.

>> +	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;
> 
> What's the |= about?

env->comp_irq_trigger_inv is zero from the start, the idea is to

- set FALLING for ..._trigger_inv if RISING is set for ..._trigger
- set RISING for ..._trigger_inv if FALLING is set for ..._trigger
- set LOW for ..._trigger_inv if HIGH is set for ..._trigger
- set HIGH for ..._trigger_inv if LOW is set for ..._trigger

That way, ..._trigger_inv will be the opposite of ..._trigger at all
times, whichever way ..._trigger is set up from devicetree.

>> +	if (env->comp_irq_trigger & IRQF_TRIGGER_FALLING)
> 
> and this should be 'else if'. If the interrupt is configured for both
> edges, which is possible with some interrupt controllers then the whole
> thing does not work at all.

Right, if you do stupid thing in the devicetree, you get garbage. And
if ..._trigger is triggering on both edges, ..._trigger_inv might just
do the same stupid thing. That's only fair, methinks?

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

So, to sum up, in order for this to work with threaded oneshot
interrupts, I still need to either keep the enable/sync/enable-dance
or tweak the irq core to handle my case better. The only gain would
be that I could fire the next step of the search from the threaded
irq handler directly (but it needs some new race-killing code).
Or am I missing something? If not, there's no pressing reason to
switch to threaded oneshot interrupts, right?

Cheers,
Peter

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Thomas Gleixner]

On Tue, 8 Nov 2016, Peter Rosin wrote:
> So, to sum up, in order for this to work with threaded oneshot
> interrupts, I still need to either keep the enable/sync/enable-dance
> or tweak the irq core to handle my case better. The only gain would
> be that I could fire the next step of the search from the threaded
> irq handler directly (but it needs some new race-killing code).
> Or am I missing something? If not, there's no pressing reason to
> switch to threaded oneshot interrupts, right?

There is no pressing reason, but that misfire prevention dance looks
fragile and overly complex to me.

The completely untested patch below should block the replay for edge
interrupts from the core code. It also makes sure that the edge interrupt
is masked until the thread handler returns. All you have to do is
requesting your threaded handler with IRQF_ONESHOT | IRQF_NO_REPLAY.

I don't think you need extra race handling with that, but I might be wrong
as usual.

Thanks,

	tglx

8<------------------
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -74,6 +74,7 @@
 #define IRQF_NO_THREAD		0x00010000
 #define IRQF_EARLY_RESUME	0x00020000
 #define IRQF_COND_SUSPEND	0x00040000
+#define IRQF_NO_REPLAY		0x00080000
 
 #define IRQF_TIMER		(__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD)
 
--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -57,6 +57,7 @@ enum {
 	IRQS_WAITING		= 0x00000080,
 	IRQS_PENDING		= 0x00000200,
 	IRQS_SUSPENDED		= 0x00000800,
+	IRQS_NO_REPLAY		= 0x00001000,
 };
 
 #include "debug.h"
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1212,7 +1212,8 @@ static int
 		 */
 		if (!((old->flags & new->flags) & IRQF_SHARED) ||
 		    ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) ||
-		    ((old->flags ^ new->flags) & IRQF_ONESHOT))
+		    ((old->flags ^ new->flags) & IRQF_ONESHOT) ||
+		    ((old->flags ^ new->flags) & IRQF_NO_REPLAY))
 			goto mismatch;
 
 		/* All handlers must agree on per-cpuness */
@@ -1324,6 +1325,9 @@ static int
 		if (new->flags & IRQF_ONESHOT)
 			desc->istate |= IRQS_ONESHOT;
 
+		if (new->flags & IRQF_NO_REPLAY)
+			desc->istate |= IRQS_NO_REPLAY;
+
 		if (irq_settings_can_autoenable(desc))
 			irq_startup(desc, true);
 		else
--- a/kernel/irq/resend.c
+++ b/kernel/irq/resend.c
@@ -56,12 +56,12 @@ static DECLARE_TASKLET(resend_tasklet, r
 void check_irq_resend(struct irq_desc *desc)
 {
 	/*
-	 * We do not resend level type interrupts. Level type
-	 * interrupts are resent by hardware when they are still
-	 * active. Clear the pending bit so suspend/resume does not
-	 * get confused.
+	 * We do not resend level type interrupts. Level type interrupts
+	 * are resent by hardware when they are still active. Also prevent
+	 * resend when the user requested so.  Clear the pending bit so
+	 * suspend/resume does not get confused.
 	 */
-	if (irq_settings_is_level(desc)) {
+	if (irq_settings_is_level(desc) || (desc->istate & IRQS_NO_REPLAY)) {
 		desc->istate &= ~IRQS_PENDING;
 		return;
 	}
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -643,7 +643,10 @@ void handle_edge_irq(struct irq_desc *de
 	kstat_incr_irqs_this_cpu(desc);
 
 	/* Start handling the irq */
-	desc->irq_data.chip->irq_ack(&desc->irq_data);
+	if (!(desc->istate & (IRQS_NO_REPLAY | IRQS_ONESHOT))
+		desc->irq_data.chip->irq_ack(&desc->irq_data);
+	else
+		mask_ack_irq(desc);
 
 	do {
 		if (unlikely(!desc->action)) {

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Peter Rosin]

On 2016-11-08 22:47, Thomas Gleixner wrote:
> On Tue, 8 Nov 2016, Peter Rosin wrote:
>> So, to sum up, in order for this to work with threaded oneshot
>> interrupts, I still need to either keep the enable/sync/enable-dance
>> or tweak the irq core to handle my case better. The only gain would
>> be that I could fire the next step of the search from the threaded
>> irq handler directly (but it needs some new race-killing code).
>> Or am I missing something? If not, there's no pressing reason to
>> switch to threaded oneshot interrupts, right?
> 
> There is no pressing reason, but that misfire prevention dance looks
> fragile and overly complex to me.

I don't see any fragility? But I wouldn't complain if there was some
simpler way to enable an irq in such a way that only new and fresh
irqs are considered.

> The completely untested patch below should block the replay for edge

[it is missing a ')', see inline comment below]

> interrupts from the core code. It also makes sure that the edge interrupt
> is masked until the thread handler returns. All you have to do is
> requesting your threaded handler with IRQF_ONESHOT | IRQF_NO_REPLAY.

It doesn't appear to work. I still get irqs that reasonably should
only have happened *during* the handling of a previous irq. And I
still need to dance when enabling the irq, otherwise I get spurious
hits.

> I don't think you need extra race handling with that, but I might be wrong
> as usual.

There's obviously no way to determine which of the timeout or the
interrupt that happens first without some race handling, so I don't
know what you mean? If the timeout happens first, there is also a
need to handle late hits from the irq that might come in during the
preparation for the next step in the binary search. It gets messy
quickly compared to the simplicity of the current implementation.

Cheers,
Peter

> 8<------------------
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -74,6 +74,7 @@
>  #define IRQF_NO_THREAD		0x00010000
>  #define IRQF_EARLY_RESUME	0x00020000
>  #define IRQF_COND_SUSPEND	0x00040000
> +#define IRQF_NO_REPLAY		0x00080000
>  
>  #define IRQF_TIMER		(__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD)
>  
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -57,6 +57,7 @@ enum {
>  	IRQS_WAITING		= 0x00000080,
>  	IRQS_PENDING		= 0x00000200,
>  	IRQS_SUSPENDED		= 0x00000800,
> +	IRQS_NO_REPLAY		= 0x00001000,
>  };
>  
>  #include "debug.h"
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1212,7 +1212,8 @@ static int
>  		 */
>  		if (!((old->flags & new->flags) & IRQF_SHARED) ||
>  		    ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) ||
> -		    ((old->flags ^ new->flags) & IRQF_ONESHOT))
> +		    ((old->flags ^ new->flags) & IRQF_ONESHOT) ||
> +		    ((old->flags ^ new->flags) & IRQF_NO_REPLAY))
>  			goto mismatch;
>  
>  		/* All handlers must agree on per-cpuness */
> @@ -1324,6 +1325,9 @@ static int
>  		if (new->flags & IRQF_ONESHOT)
>  			desc->istate |= IRQS_ONESHOT;
>  
> +		if (new->flags & IRQF_NO_REPLAY)
> +			desc->istate |= IRQS_NO_REPLAY;
> +
>  		if (irq_settings_can_autoenable(desc))
>  			irq_startup(desc, true);
>  		else
> --- a/kernel/irq/resend.c
> +++ b/kernel/irq/resend.c
> @@ -56,12 +56,12 @@ static DECLARE_TASKLET(resend_tasklet, r
>  void check_irq_resend(struct irq_desc *desc)
>  {
>  	/*
> -	 * We do not resend level type interrupts. Level type
> -	 * interrupts are resent by hardware when they are still
> -	 * active. Clear the pending bit so suspend/resume does not
> -	 * get confused.
> +	 * We do not resend level type interrupts. Level type interrupts
> +	 * are resent by hardware when they are still active. Also prevent
> +	 * resend when the user requested so.  Clear the pending bit so
> +	 * suspend/resume does not get confused.
>  	 */
> -	if (irq_settings_is_level(desc)) {
> +	if (irq_settings_is_level(desc) || (desc->istate & IRQS_NO_REPLAY)) {
>  		desc->istate &= ~IRQS_PENDING;
>  		return;
>  	}
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -643,7 +643,10 @@ void handle_edge_irq(struct irq_desc *de
>  	kstat_incr_irqs_this_cpu(desc);
>  
>  	/* Start handling the irq */
> -	desc->irq_data.chip->irq_ack(&desc->irq_data);
> +	if (!(desc->istate & (IRQS_NO_REPLAY | IRQS_ONESHOT))

Missing ')' at the end.

> +		desc->irq_data.chip->irq_ack(&desc->irq_data);
> +	else
> +		mask_ack_irq(desc);
>  
>  	do {
>  		if (unlikely(!desc->action)) {
> 
> 

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Thomas Gleixner]

On Wed, 9 Nov 2016, Peter Rosin wrote:
> On 2016-11-08 22:47, Thomas Gleixner wrote:
> > I don't think you need extra race handling with that, but I might be wrong
> > as usual.
> 
> There's obviously no way to determine which of the timeout or the
> interrupt that happens first without some race handling, so I don't
> know what you mean? If the timeout happens first, there is also a
> need to handle late hits from the irq that might come in during the
> preparation for the next step in the binary search. It gets messy
> quickly compared to the simplicity of the current implementation.

Gah, forgot about that timeout thingy. Fair enough.

Feel free to add an 

Acked-by: Thomas Gleixner <tglx@linutronix.de>

Thanks,

	tglx

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Peter Rosin]

On 2016-11-09 16:06, Thomas Gleixner wrote:
> On Wed, 9 Nov 2016, Peter Rosin wrote:
>> On 2016-11-08 22:47, Thomas Gleixner wrote:
>>> I don't think you need extra race handling with that, but I might be wrong
>>> as usual.
>>
>> There's obviously no way to determine which of the timeout or the
>> interrupt that happens first without some race handling, so I don't
>> know what you mean? If the timeout happens first, there is also a
>> need to handle late hits from the irq that might come in during the
>> preparation for the next step in the binary search. It gets messy
>> quickly compared to the simplicity of the current implementation.
> 
> Gah, forgot about that timeout thingy. Fair enough.
> 
> Feel free to add an 
> 
> Acked-by: Thomas Gleixner <tglx@linutronix.de>

Thanks for looking!

Cheers,
Peter

Re: [PATCH v4 0/8] IIO wrapper drivers, dpot-dac and envelope-detector

[Jonathan Cameron]

On 08/11/16 11:58, Peter Rosin wrote:
> Hi!
> 
> This is a respin with a handful of nitpicks fixed from v3. No major
> changes. And a couple of acks was added too, thanks! I also added
> Thomas Gleixner as Cc, since Jonathan was hoping for some comments
> on the somewhat odd interrupt handling in patch 8/8 (but perhaps
> plumbers-week isn't the best week to hope for that).
> 
> 
> 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 :-)
I'll probably get to it sooner or later if no one else jumps on it.
> 
> 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.
> 
> One thing I still 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. The only way out of this
> is a devicetree thing. I suppose it will have to be added by whomever needs
> it whenever that is...
> 
> I also wonder if the "new" *_available ABI should perhaps be documented
> for all variants directly in sysfs-bus-iio instead of doing it in a driver
> specific maner that I did? But that can be fixed later by someone more
> capable than me :-)
You doubt yourself too much ;)  Some one with fewer inhibitions you mean!

Anyhow, just thought I'd add that I like this series very much.
It's a nice interesting use of the infrastructures.  Good to see people
are getting more adventurous all the time.

Also always nice when someone else picks up a patch I dropped years ago
and does the remaining hard work to get it in ;)

There is a limited window left obviously if we want to adjust that ABI
so shout soon or it'll be there for ever *muhahahaha*

Jonathan
> 
> v3 -> v4
> - gained acks from Rob for the three bindings patches
> - gained ack from Daniel for the core _available patch (1/8)
> - dropped the type argument from iio_read_avail_channel_raw(), since
>   raw values are assumed to be of type IIO_VAL_INT anyway
> - add some more words about what iio_read_avail_channel_raw() does
> - rebased onto v4.9-rc4
> 
> dpot-dac:
> - adjust to changed signature of iio_read_avail_channel_raw()
> - one instance of s/the channels supports/the channel supports/
> - drop surplus s64 cast in dpot_dac_channel_max_ohms()
> 
> envelope-detector
> - the envelope-detector module is called envelope-detector
> 
> 
> v2 -> v3
> - add some missing @foo comments in iio.h in the initial forward ported patch
> - killed some buffer overflow problems in the initial forward ported patch
> - add inkern.c helpers for the new available attributes to avoid viral
>   boilerplating in the future (also add support for finding max of
>   IIO_AVAIL_LISTs of IIO_VAL_INTs)
> - add ABI docs for new ABI of mcp4531 (out_resistance_raw_available)
>   and an example in the commit message
> 
> dpot-dac:
> - two more counts of s/assumed the that the/assumed that the/   *blush*
> - add ABI docs for new ABI (out_voltageY_raw_available)
> 
> envelope detector:
> - move device attributes 'compare_interval_ms' and 'invert' to extended
>   channel attributes (out_altvoltage0_compare_interval and
>   out_altvoltage0_invert) as they really are about the channel and not
>   the device
> - kill "dpot-dac,max-ohms = <100000>;" in the devicetree example
> 
> 
> 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
> 
> Cheers,
> Peter
> 
> Jonathan Cameron (1):
>   iio:core: add a callback to allow drivers to provide _available
>     attributes
> 
> Peter Rosin (7):
>   iio: inkern: add helpers to query available values from channels
>   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 ++
>  .../ABI/testing/sysfs-bus-iio-dac-dpot-dac         |   8 +
>  .../testing/sysfs-bus-iio-potentiometer-mcp4531    |   8 +
>  .../bindings/iio/adc/envelope-detector.txt         |  54 +++
>  .../devicetree/bindings/iio/dac/dpot-dac.txt       |  41 ++
>  .../devicetree/bindings/vendor-prefixes.txt        |   1 +
>  MAINTAINERS                                        |  17 +
>  drivers/iio/adc/Kconfig                            |  10 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/envelope-detector.c                | 422 +++++++++++++++++++++
>  drivers/iio/dac/Kconfig                            |  10 +
>  drivers/iio/dac/Makefile                           |   1 +
>  drivers/iio/dac/dpot-dac.c                         | 266 +++++++++++++
>  drivers/iio/industrialio-core.c                    | 259 +++++++++++--
>  drivers/iio/inkern.c                               | 104 +++++
>  drivers/iio/potentiometer/mcp4531.c                | 104 +++--
>  include/linux/iio/consumer.h                       |  28 ++
>  include/linux/iio/iio.h                            |  46 +++
>  include/linux/iio/types.h                          |   5 +
>  19 files changed, 1346 insertions(+), 75 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
>  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
> 

Re: [PATCH v4 1/8] iio:core: add a callback to allow drivers to provide _available attributes

[Jonathan Cameron]

On 08/11/16 11:58, 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.
> 
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
> [forward ported, added some docs and fixed buffer overflows /peda]
> Acked-by: Daniel Baluta <daniel.baluta@intel.com>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied to the togreg branch of iio.git.

Lars, if you get a chance I'd still appreciate any feedback you have
in the ABI in particular.  We still have a window to modify it but that
will be running out fast. If you are happy with it then great!

Jonathan
> ---
>  drivers/iio/industrialio-core.c | 259 +++++++++++++++++++++++++++++++++++-----
>  include/linux/iio/iio.h         |  29 +++++
>  include/linux/iio/types.h       |   5 +
>  3 files changed, 260 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index fc340ed3dca1..b35c61a31fa6 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -575,66 +575,82 @@ 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, size_t len, 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 snprintf(buf, len, "%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]),
> -				       -vals[1], scale_db ? " dB" : "");
> +			return snprintf(buf, len, "-%d.%06u%s", abs(vals[0]),
> +					-vals[1], scale_db ? " dB" : "");
>  		else
> -			return sprintf(buf, "%d.%06u%s\n", vals[0], vals[1],
> -				scale_db ? " dB" : "");
> +			return snprintf(buf, len, "%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]),
> -				       -vals[1]);
> +			return snprintf(buf, len, "-%d.%09u", abs(vals[0]),
> +					-vals[1]);
>  		else
> -			return sprintf(buf, "%d.%09u\n", vals[0], vals[1]);
> +			return snprintf(buf, len, "%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 snprintf(buf, len, "%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 snprintf(buf, len, "%d.%09u", tmp0, tmp1);
>  	case IIO_VAL_INT_MULTIPLE:
>  	{
>  		int i;
> -		int len = 0;
> +		int l = 0;
>  
> -		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;
> +		for (i = 0; i < size; ++i) {
> +			l += snprintf(&buf[l], len - l, "%d ", vals[i]);
> +			if (l >= len)
> +				break;
> +		}
> +		return l;
>  	}
>  	default:
>  		return 0;
>  	}
>  }
> +
> +/**
> + * iio_format_value() - Formats a IIO value into its string representation
> + * @buf:	The buffer to which the formatted value gets written
> + *		which is assumed to be big enough (i.e. PAGE_SIZE).
> + * @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, PAGE_SIZE, type, size, vals);
> +	if (len >= PAGE_SIZE - 1)
> +		return -EFBIG;
> +
> +	return len + sprintf(buf + len, "\n");
> +}
>  EXPORT_SYMBOL_GPL(iio_format_value);
>  
>  static ssize_t iio_read_channel_info(struct device *dev,
> @@ -662,6 +678,119 @@ 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, PAGE_SIZE - len,
> +						  type, 1, &vals[i]);
> +			if (len >= PAGE_SIZE)
> +				return -EFBIG;
> +			if (i < length - 1)
> +				len += snprintf(buf + len, PAGE_SIZE - len,
> +						" ");
> +			else
> +				len += snprintf(buf + len, PAGE_SIZE - len,
> +						"\n");
> +			if (len >= PAGE_SIZE)
> +				return -EFBIG;
> +		}
> +		break;
> +	default:
> +		for (i = 0; i < length / 2; i++) {
> +			len += __iio_format_value(buf + len, PAGE_SIZE - len,
> +						  type, 2, &vals[i * 2]);
> +			if (len >= PAGE_SIZE)
> +				return -EFBIG;
> +			if (i < length / 2 - 1)
> +				len += snprintf(buf + len, PAGE_SIZE - len,
> +						" ");
> +			else
> +				len += snprintf(buf + len, PAGE_SIZE - len,
> +						"\n");
> +			if (len >= PAGE_SIZE)
> +				return -EFBIG;
> +		}
> +	};
> +
> +	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, PAGE_SIZE - len,
> +						  type, 1, &vals[i]);
> +			if (len >= PAGE_SIZE)
> +				return -EFBIG;
> +			if (i < 2)
> +				len += snprintf(buf + len, PAGE_SIZE - len,
> +						" ");
> +			else
> +				len += snprintf(buf + len, PAGE_SIZE - len,
> +						"]\n");
> +			if (len >= PAGE_SIZE)
> +				return -EFBIG;
> +		}
> +		break;
> +	default:
> +		for (i = 0; i < 3; i++) {
> +			len += __iio_format_value(buf + len, PAGE_SIZE - len,
> +						  type, 2, &vals[i * 2]);
> +			if (len >= PAGE_SIZE)
> +				return -EFBIG;
> +			if (i < 2)
> +				len += snprintf(buf + len, PAGE_SIZE - len,
> +						" ");
> +			else
> +				len += snprintf(buf + len, PAGE_SIZE - len,
> +						"]\n");
> +			if (len >= PAGE_SIZE)
> +				return -EFBIG;
> +		}
> +	};
> +
> +	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 +1107,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 +1156,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 +1171,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 +1186,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 +1200,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..45b781084a4b 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -225,12 +225,22 @@ struct iio_event_spec {
>   *			endianness:	little or big endian
>   * @info_mask_separate: What information is to be exported that is specific to
>   *			this channel.
> + * @info_mask_separate_available: What availability information is to be
> + *			exported that is specific to this channel.
>   * @info_mask_shared_by_type: What information is to be exported that is shared
>   *			by all channels of the same type.
> + * @info_mask_shared_by_type_available: What availability information is to be
> + *			exported that is shared by all channels of the same
> + *			type.
>   * @info_mask_shared_by_dir: What information is to be exported that is shared
>   *			by all channels of the same direction.
> + * @info_mask_shared_by_dir_available: What availability information is to be
> + *			exported that is shared by all channels of the same
> + *			direction.
>   * @info_mask_shared_by_all: What information is to be exported that is shared
>   *			by all channels.
> + * @info_mask_shared_by_all_available: What availability information is to be
> + *			exported that is shared by all channels.
>   * @event_spec:		Array of events which should be registered for this
>   *			channel.
>   * @num_event_specs:	Size of the event_spec array.
> @@ -269,9 +279,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;
> @@ -349,6 +363,14 @@ struct iio_dev;
>   *			max_len specifies maximum number of elements
>   *			vals pointer can contain. val_len is used to return
>   *			length of valid elements in vals.
> + * @read_avail:		function to return the available values from the device.
> + *			mask specifies which value. Note 0 means the available
> + *			values for the channel in question.  Return value
> + *			specifies if a IIO_AVAIL_LIST or a IIO_AVAIL_RANGE is
> + *			returned in vals. The type of the vals are returned in
> + *			type and the number of vals is returned in length. For
> + *			ranges, there are always three vals returned; min, step
> + *			and max. For lists, all possible values are enumerated.
>   * @write_raw:		function to write a value to the device.
>   *			Parameters are the same as for read_raw.
>   * @write_raw_get_fmt:	callback function to query the expected
> @@ -397,6 +419,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);
> +
>  	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_ */
> 

Re: [PATCH v4 2/8] iio: inkern: add helpers to query available values from channels

[Jonathan Cameron]

On 08/11/16 11:58, Peter Rosin wrote:
> Specifically a helper for reading the available maximum raw value of a
> channel and a helper for forwarding read_avail requests for raw values
> from one iio driver to an iio channel that is consumed.
> 
> These rather specific helpers are in turn built with generic helpers
> making it easy to build more helpers for available values as needed.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied.
> ---
>  drivers/iio/inkern.c         | 104 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/consumer.h |  28 ++++++++++++
>  include/linux/iio/iio.h      |  17 +++++++
>  3 files changed, 149 insertions(+)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index c4757e6367e7..cfca17ba2535 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -703,6 +703,110 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val, int *val2)
>  }
>  EXPORT_SYMBOL_GPL(iio_read_channel_scale);
>  
> +static int iio_channel_read_avail(struct iio_channel *chan,
> +				  const int **vals, int *type, int *length,
> +				  enum iio_chan_info_enum info)
> +{
> +	if (!iio_channel_has_available(chan->channel, info))
> +		return -EINVAL;
> +
> +	return chan->indio_dev->info->read_avail(chan->indio_dev, chan->channel,
> +						 vals, type, length, info);
> +}
> +
> +int iio_read_avail_channel_raw(struct iio_channel *chan,
> +			       const int **vals, int *length)
> +{
> +	int ret;
> +	int type;
> +
> +	mutex_lock(&chan->indio_dev->info_exist_lock);
> +	if (!chan->indio_dev->info) {
> +		ret = -ENODEV;
> +		goto err_unlock;
> +	}
> +
> +	ret = iio_channel_read_avail(chan,
> +				     vals, &type, length, IIO_CHAN_INFO_RAW);
> +err_unlock:
> +	mutex_unlock(&chan->indio_dev->info_exist_lock);
> +
> +	if (ret >= 0 && type != IIO_VAL_INT) {
> +		/* raw values are assumed to be IIO_VAL_INT */
> +		ret = -EINVAL;
> +		goto err_unlock;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_read_avail_channel_raw);
> +
> +static int iio_channel_read_max(struct iio_channel *chan,
> +				int *val, int *val2, int *type,
> +				enum iio_chan_info_enum info)
> +{
> +	int unused;
> +	const int *vals;
> +	int length;
> +	int ret;
> +
> +	if (!val2)
> +		val2 = &unused;
> +
> +	ret = iio_channel_read_avail(chan, &vals, type, &length, info);
> +	switch (ret) {
> +	case IIO_AVAIL_RANGE:
> +		switch (*type) {
> +		case IIO_VAL_INT:
> +			*val = vals[2];
> +			break;
> +		default:
> +			*val = vals[4];
> +			*val2 = vals[5];
> +		}
> +		return 0;
> +
> +	case IIO_AVAIL_LIST:
> +		if (length <= 0)
> +			return -EINVAL;
> +		switch (*type) {
> +		case IIO_VAL_INT:
> +			*val = vals[--length];
> +			while (length) {
> +				if (vals[--length] > *val)
> +					*val = vals[length];
> +			}
> +			break;
> +		default:
> +			/* FIXME: learn about max for other iio values */
> +			return -EINVAL;
> +		}
> +		return 0;
> +
> +	default:
> +		return ret;
> +	}
> +}
> +
> +int iio_read_max_channel_raw(struct iio_channel *chan, int *val)
> +{
> +	int ret;
> +	int type;
> +
> +	mutex_lock(&chan->indio_dev->info_exist_lock);
> +	if (!chan->indio_dev->info) {
> +		ret = -ENODEV;
> +		goto err_unlock;
> +	}
> +
> +	ret = iio_channel_read_max(chan, val, NULL, &type, IIO_CHAN_INFO_RAW);
> +err_unlock:
> +	mutex_unlock(&chan->indio_dev->info_exist_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_read_max_channel_raw);
> +
>  int iio_get_channel_type(struct iio_channel *chan, enum iio_chan_type *type)
>  {
>  	int ret = 0;
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 9edccfba1ffb..9a4f336d8b4a 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -226,6 +226,34 @@ int iio_read_channel_processed(struct iio_channel *chan, int *val);
>  int iio_write_channel_raw(struct iio_channel *chan, int val);
>  
>  /**
> + * iio_read_max_channel_raw() - read maximum available raw value from a given
> + *				channel, i.e. the maximum possible value.
> + * @chan:		The channel being queried.
> + * @val:		Value read back.
> + *
> + * Note raw reads from iio channels are in adc counts and hence
> + * scale will need to be applied if standard units are required.
> + */
> +int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
> +
> +/**
> + * iio_read_avail_channel_raw() - read available raw values from a given channel
> + * @chan:		The channel being queried.
> + * @vals:		Available values read back.
> + * @length:		Number of entries in vals.
> + *
> + * Returns an error code, IIO_AVAIL_RANGE or IIO_AVAIL_LIST.
> + *
> + * For ranges, three vals are always returned; min, step and max.
> + * For lists, all the possible values are enumerated.
> + *
> + * Note raw available values from iio channels are in adc counts and
> + * hence scale will need to be applied if standard units are required.
> + */
> +int iio_read_avail_channel_raw(struct iio_channel *chan,
> +			       const int **vals, int *length);
> +
> +/**
>   * iio_get_channel_type() - get the type of a channel
>   * @channel:		The channel being queried.
>   * @type:		The type of the channel.
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 45b781084a4b..f9be8836ef23 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -315,6 +315,23 @@ static inline bool iio_channel_has_info(const struct iio_chan_spec *chan,
>  		(chan->info_mask_shared_by_all & BIT(type));
>  }
>  
> +/**
> + * iio_channel_has_available() - Checks if a channel has an available attribute
> + * @chan: The channel to be queried
> + * @type: Type of the available attribute to be checked
> + *
> + * Returns true if the channel supports reporting available values for the
> + * given attribute type, false otherwise.
> + */
> +static inline bool iio_channel_has_available(const struct iio_chan_spec *chan,
> +					     enum iio_chan_info_enum type)
> +{
> +	return (chan->info_mask_separate_available & BIT(type)) |
> +		(chan->info_mask_shared_by_type_available & BIT(type)) |
> +		(chan->info_mask_shared_by_dir_available & BIT(type)) |
> +		(chan->info_mask_shared_by_all_available & BIT(type));
> +}
> +
>  #define IIO_CHAN_SOFT_TIMESTAMP(_si) {					\
>  	.type = IIO_TIMESTAMP,						\
>  	.channel = -1,							\
> 

Re: [PATCH v4 3/8] iio: mcp4531: provide range of available raw values

[Jonathan Cameron]

On 08/11/16 11:58, Peter Rosin wrote:
> Example:
> 
> $ cat '/sys/bus/iio/devices/iio:device0/out_resistance_raw_available'
> [0 1 256]
> 
> Meaning: min 0, step 1 and max 256.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied.
> ---
>  .../testing/sysfs-bus-iio-potentiometer-mcp4531    |   8 ++
>  MAINTAINERS                                        |   1 +
>  drivers/iio/potentiometer/mcp4531.c                | 104 ++++++++++++---------
>  3 files changed, 71 insertions(+), 42 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531 b/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
> new file mode 100644
> index 000000000000..2a91fbe394fc
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
> @@ -0,0 +1,8 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/out_resistance_raw_available
> +Date:		October 2016
> +KernelVersion:	4.9
> +Contact:	Peter Rosin <peda@axentia.se>
> +Description:
> +		The range of available values represented as the minimum value,
> +		the step and the maximum value, all enclosed in square brackets.
> +		Example: [0 1 256]
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 411e3b87b8c2..2409e1d79cb6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7707,6 +7707,7 @@ MCP4531 MICROCHIP DIGITAL POTENTIOMETER DRIVER
>  M:	Peter Rosin <peda@axentia.se>
>  L:	linux-iio@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/ABI/testing/sysfs-bus-iio-potentiometer-mcp4531
>  F:	drivers/iio/potentiometer/mcp4531.c
>  
>  MEASUREMENT COMPUTING CIO-DAC IIO DRIVER
> 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,
>  };
> 

Re: [PATCH v4 4/8] dt-bindings: add axentia to vendor-prefixes

[Jonathan Cameron]

On 08/11/16 11:58, Peter Rosin wrote:
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied.
> ---
>  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
> 

Re: [PATCH v4 6/8] iio: dpot-dac: DAC driver based on a digital potentiometer

[Jonathan Cameron]

On 08/11/16 11:58, Peter Rosin wrote:
> 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
>                   |      |
>                   '------+------------+
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied to the togreg branch of iio.git - shortly to be pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  .../ABI/testing/sysfs-bus-iio-dac-dpot-dac         |   8 +
>  MAINTAINERS                                        |   2 +
>  drivers/iio/dac/Kconfig                            |  10 +
>  drivers/iio/dac/Makefile                           |   1 +
>  drivers/iio/dac/dpot-dac.c                         | 266 +++++++++++++++++++++
>  5 files changed, 287 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
>  create mode 100644 drivers/iio/dac/dpot-dac.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
> new file mode 100644
> index 000000000000..580e93f373f6
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
> @@ -0,0 +1,8 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/out_voltageY_raw_available
> +Date:		October 2016
> +KernelVersion:	4.9
> +Contact:	Peter Rosin <peda@axentia.se>
> +Description:
> +		The range of available values represented as the minimum value,
> +		the step and the maximum value, all enclosed in square brackets.
> +		Example: [0 1 256]
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4ff9c12b1313..583c6c93b6f3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6128,7 +6128,9 @@ IIO DIGITAL POTENTIOMETER DAC
>  M:	Peter Rosin <peda@axentia.se>
>  L:	linux-iio@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
>  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..eb1f0509a3ec
> --- /dev/null
> +++ b/drivers/iio/dac/dpot-dac.c
> @@ -0,0 +1,266 @@
> +/*
> + * 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);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*type = IIO_VAL_INT;
> +		return iio_read_avail_channel_raw(dac->dpot, vals, length);
> +	}
> +
> +	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);
> +	unsigned long long tmp;
> +	int ret;
> +	int val;
> +	int val2;
> +	int max;
> +
> +	ret = iio_read_max_channel_raw(dac->dpot, &max);
> +	if (ret < 0) {
> +		dev_err(dev, "dpot does not indicate its raw maximum value\n");
> +		return ret;
> +	}
> +
> +	switch (iio_read_channel_scale(dac->dpot, &val, &val2)) {
> +	case IIO_VAL_INT:
> +		return max * val;
> +	case IIO_VAL_FRACTIONAL:
> +		tmp = (unsigned long long)max * val;
> +		do_div(tmp, val2);
> +		return tmp;
> +	case IIO_VAL_FRACTIONAL_LOG2:
> +		tmp = val * 1000000000LL * max >> val2;
> +		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;
> +
> +	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");
> 

Re: [PATCH v4 7/8] dt-bindings: iio: document envelope-detector bindings

[Jonathan Cameron]

On 08/11/16 11:58, Peter Rosin wrote:
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied.
> ---
>  .../bindings/iio/adc/envelope-detector.txt         | 54 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 +++
>  2 files changed, 60 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..27544bdd4478
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
> @@ -0,0 +1,54 @@
> +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";
> +		#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 583c6c93b6f3..0e13066ca3a2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6132,6 +6132,12 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
>  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>
> 

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Jonathan Cameron]

On 08/11/16 17:03, Peter Rosin wrote:
> On 2016-11-08 16:59, Thomas Gleixner wrote:
>> On Tue, 8 Nov 2016, Peter Rosin wrote:
>>> +/*
>>> + * 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);
>>
>> So you need that whole dance including the delayed work because you cannot
>> call iio_write_channel_raw() from hard interrupt context, right?
> 
> It's not the "cannot call from hard irq context" that made me do that, it's...
> 
>> So you might just register a threaded interrupt handler, which should make
>> this whole thing way simpler.
>>
>>      devm_request_threaded_irq(dev, irq, NULL, your_isr, IRQF_ONESHOT, ...);
>>
>> The core will mask the interrupt line until the threaded handler is
>> finished. The threaded handler is invoked with preemption enabled, so you
>> can sleep there as long as you want. So you can do everything in your
>> handler and the above dance is just not required.
> 
> ...that I couldn't work out how to reenable a oneshot irq once it had fired,
> short of freeing the irq and requesting it again. That seemed entirely
> bogus, the driver shouldn't risk losing a resource like that so I don't know
> what I didn't see? Or maybe it was that I had a hard time resolving the race
> between the irq and the timeout in a nice way. I honestly don't remember
> why exactly I abandoned oneshot irqs, but this enable/sync/enable dance
> was much nicer than what I came up with for the oneshot irq solution I
> originally worked on.
> 
> Or maybe I had problems with the possibly pending irq also when using a
> oneshot irq, but didn't realize it? That was something I discovered quite
> late in the process, some time after moving away from oneshot irqs. Are
> pending irqs cleared when requesting (or reenabling, however that is done)
> a oneshot irq?
> 
> Anyway, I do not want the interrupt to be serviced when no one is interested,
> since I'm afraid that nasty input might generate a flood of interrupts that
> might disturb other things that the cpu is doing. Which means that I need
> to enable/disable the interrupt as needed.
> 
> However, what *I* thought Jonathan wanted input on was the part where the
> interrupt edge/level is flipped when requesting "inverted" signals in
> envelope_store_invert(). That could perhaps be seen as unorthodox and in
> need of more eyes?
Nope, as far as I can recall it was precisely this dance that was 
I wanted Thomas to comment on :)  The inverted bit isn't as novel as
this ;)

Anyhow, thread ended up with a good conclusion so I'm happy.

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

Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator

[Jonathan Cameron]

On 11/11/16 11:37, Peter Rosin wrote:
> On 2016-11-09 16:06, Thomas Gleixner wrote:
>> On Wed, 9 Nov 2016, Peter Rosin wrote:
>>> On 2016-11-08 22:47, Thomas Gleixner wrote:
>>>> I don't think you need extra race handling with that, but I might be wrong
>>>> as usual.
>>>
>>> There's obviously no way to determine which of the timeout or the
>>> interrupt that happens first without some race handling, so I don't
>>> know what you mean? If the timeout happens first, there is also a
>>> need to handle late hits from the irq that might come in during the
>>> preparation for the next step in the binary search. It gets messy
>>> quickly compared to the simplicity of the current implementation.
>>
>> Gah, forgot about that timeout thingy. Fair enough.
>>
>> Feel free to add an 
>>
>> Acked-by: Thomas Gleixner <tglx@linutronix.de>
> 
> Thanks for looking!
Applied to the togreg branch of iio.git and pushed out
as testing for the autobuilders to play with.

Excellent patch set.

Thanks,

Jonathan
> 
> Cheers,
> Peter
> 

Re: [PATCH v4 0/8] IIO wrapper drivers, dpot-dac and envelope-detector

[Peter Rosin]

On 2016-11-12 18:15, Jonathan Cameron wrote:
> On 08/11/16 11:58, Peter Rosin wrote:
>> I also wonder if the "new" *_available ABI should perhaps be documented
>> for all variants directly in sysfs-bus-iio instead of doing it in a driver
>> specific maner that I did? But that can be fixed later by someone more
>> capable than me :-)
> You doubt yourself too much ;)  Some one with fewer inhibitions you mean!

Maybe so, I just find that file long and confusing. I do not have a mental
picture of what fits where...

> Anyhow, just thought I'd add that I like this series very much.
> It's a nice interesting use of the infrastructures.  Good to see people
> are getting more adventurous all the time.

Thanks for the confidence boost! And also thanks for taking care of the
bot fallout...

Cheers,
Peter