iio: Introduce the generic counter interface

Counters are IIO devices that are exposed via a generic counter interface consisting of one or more counter signals (IIO_SIGNAL) linked to one or more counter values (IIO_COUNT).

This patch introduces the IIO_SIGNAL constant which represents a counter device signal line. Additionally, a new "counter" member is added to struct iio_chan_spec, with relevant support, to indicate that a channel is part of a counter.

Signed-off-by William Breathitt Gray <[email protected]>

---
 drivers/iio/industrialio-core.c | 14 +++++++++++++-
 include/linux/iio/iio.h         |  2 ++
 include/uapi/linux/iio/types.h  |  1 +
 3 files changed, 16 insertions(+), 1 deletion(-)

drivers/iio/industrialio-core.c +1 -0

index 7a5aa127c52e..ee508f2070a7 100644
@@ -85,6 +85,7 @@ static const char * const iio_chan_type_name_spec[] = {
 	[IIO_COUNT] = "count",
 	[IIO_INDEX] = "index",
 	[IIO_GRAVITY]  = "gravity",
+	[IIO_SIGNAL] = "signal",
 };
 
 static const char * const iio_modifier_names[] = {

@@ -989,7 +990,18 @@ int __iio_device_attr_init(struct device_attribute *dev_attr,
 			break;

drivers/iio/industrialio-core.c +0 -0

 				name = kasprintf(GFP_KERNEL, "%s_%s%d_%s",
 						    iio_direction[chan->output],
 						    iio_chan_type_name_spec[chan->type],

include/linux/iio/iio.h +2 -0

index 50cae8504256..9f949dd74b60 100644
@@ -263,6 +263,7 @@ struct iio_event_spec {
  *			attributes but not for event codes.
  * @output:		Channel is output.
  * @differential:	Channel is differential.
+ * @counter:		Channel is part of a counter.
  */
 struct iio_chan_spec {
 	enum iio_chan_type	type;

@@ -295,6 +296,7 @@ struct iio_chan_spec {
 	unsigned		indexed:1;
 	unsigned		output:1;
 	unsigned		differential:1;
+	unsigned		counter:1;
 };
 
 

include/uapi/linux/iio/types.h +1 -0

index ffafd6c25a48..313899652ca7 100644
@@ -43,6 +43,7 @@ enum iio_chan_type {
 	IIO_COUNT,
 	IIO_INDEX,
 	IIO_GRAVITY,
+	IIO_SIGNAL,
 };
 
 enum iio_modifier {

This patch introduces the IIO generic counter interface for supporting counter devices. The generic counter interface serves as a catch-all to enable rudimentary support for devices that qualify as counters. More specific and apt counter interfaces may be developed on top of the generic counter interface, and those interfaces should be used by drivers when possible rather than the generic counter interface.

In the context of the IIO generic counter interface, a counter is defined as a device that reports one or more "counter values" based on the state changes of one or more "counter signals" as evaluated by a defined "counter function."

The IIO generic counter interface piggybacks off of the IIO core. This is primarily used to leverage the existing sysfs setup: the IIO_COUNT channel attributes represent the Counter Value, while the IIO_SIGNAL channel attributes represent the Counter Signal; auxilary IIO_COUNT attributes represent the Counter Signal connections (Triggers) and their respective state change configurations which trigger an associated "counter function" evaluation.

The iio_counter_ops structure serves as a container for driver callbacks to communicate with the device; function callbacks are provided to read and write various Signals and Values, and set and get the "trigger mode" and "function mode" for various Triggers and Values respectively.

To support a counter device, a driver must first allocate the available Counter Signals via iio_counter_signal structures. These Signals should be stored as an array and set to the signals array member of an allocated iio_counter structure before the Counter is registered to the system.

Counter Values may be allocated via iio_counter_value structures, and respective Counter Signal associations (Triggers) made via iio_counter_trigger structures. Associated iio_counter_trigger structures are stored as an array and set to the the triggers array member of the respective iio_counter_value structure. These iio_counter_value structures are set to the values array member of an allocated iio_counter structure before the Counter is registered to the system.

A counter device is registered to the system by passing the respective initialized iio_counter structure to the iio_counter_register function; similarly, the iio_counter_unregister function unregisters the respective counter. The devm_iio_counter_register and iio_devm_iio_counter_unregister functions serve as device memory-managed versions of the iio_counter_register and iio_counter unregister functions respectively.

Signed-off-by William Breathitt Gray <[email protected]>

---
 MAINTAINERS                        |   7 +
 drivers/iio/Kconfig                |   8 +
 drivers/iio/Makefile               |   1 +
 drivers/iio/counter/Kconfig        |   1 +
 drivers/iio/industrialio-counter.c | 900 +++++++++++++++++++++++++++++++++++++
 include/linux/iio/counter.h        | 166 +++++++
 6 files changed, 1083 insertions(+)
 create mode 100644 drivers/iio/industrialio-counter.c
 create mode 100644 include/linux/iio/counter.h

MAINTAINERS +0 -0

index 2281af4b41b6..8b7c37bed252 100644

MAINTAINERS +1 -0

+
 IIO MULTIPLEXER
 M:	Peter Rosin <[email protected]>
 L:	[email protected]

drivers/iio/Kconfig +8 -0

index b3c8c6ef0dff..78e01f4f5937 100644
@@ -30,6 +30,14 @@ config IIO_CONFIGFS
 	  (e.g. software triggers). For more info see
 	  Documentation/iio/iio_configfs.txt.
 
+config IIO_COUNTER
+	bool "Enable IIO counter support"
+	help
+	  Provides IIO core support for counters. This API provides
+	  a generic interface that serves as the building blocks to
+	  create more complex counter interfaces. Rudimentary support
+	  for counters is enabled.
+
 config IIO_TRIGGER
 	bool "Enable triggered sampling support"
 	help

drivers/iio/Makefile +1 -0

index 93c769cd99bf..6427ff38f964 100644
@@ -5,6 +5,7 @@
 obj-$(CONFIG_IIO) += industrialio.o
 industrialio-y := industrialio-core.o industrialio-event.o inkern.o
 industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
+industrialio-$(CONFIG_IIO_COUNTER) += industrialio-counter.o
 industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
 
 obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o

drivers/iio/counter/Kconfig +1 -0

index 474e1ac4e7c0..c8becfe78e28 100644
@@ -4,6 +4,7 @@
 # When adding new entries keep the list in alphabetical order
 
 menu "Counters"
+	depends on IIO_COUNTER
 
 config 104_QUAD_8
 	tristate "ACCES 104-QUAD-8 driver"

drivers/iio/industrialio-counter.c new file +55 -0

new file mode 100644
index 000000000000..dfb982dae3a8
@@ -0,0 +1,900 @@
+/*
+ * Industrial I/O counter interface functions
+ * Copyright (C) 2017 William Breathitt Gray
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+
+#include <linux/iio/counter.h>
+
+#define iio_priv_get_counter(_indio_dev) \
+	(*(struct iio_counter **)iio_priv(_indio_dev))
+
+static struct iio_counter_signal *iio_counter_signal_find_by_id(
+	const struct iio_counter *const counter, const int id)
+{
+	size_t i;
+	struct iio_counter_signal *signal;
+
+	for (i = 0; i < counter->num_signals; i++) {
+		signal = counter->signals + i;
+		if (signal->id == id)
+			return signal;
+	}
+
+	return NULL;
+}
+
+static struct iio_counter_trigger *iio_counter_trigger_find_by_id(
+	const struct iio_counter_value *const value, const int id)
+{
+	size_t i;
+	struct iio_counter_trigger *trigger;
+
+	for (i = 0; i < value->num_triggers; i++) {
+		trigger = value->triggers + i;
+		if (trigger->signal->id == id)
+			return trigger;

drivers/iio/industrialio-counter.c new file +24 -0

+
+	for (i = 0; i < counter->num_values; i++) {
+		value = counter->values + i;
+		if (value->id == id)
+			return value;
+	}
+
+	return NULL;
+}
+
+static ssize_t iio_counter_signal_name_read(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
+{
+	struct iio_counter *const counter = iio_priv_get_counter(indio_dev);
+	const struct iio_counter_signal *signal;
+
+	signal = iio_counter_signal_find_by_id(counter, chan->channel2);
+	if (!signal)
+		return -EINVAL;
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n", signal->name);
+}
+
+static ssize_t iio_counter_value_name_read(struct iio_dev *indio_dev,

drivers/iio/industrialio-counter.c new file +153 -0

+}
+
+static ssize_t iio_counter_value_triggers_read(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
+{
+	struct iio_counter *const counter = iio_priv_get_counter(indio_dev);
+	struct iio_counter_value *value;
+	size_t i;
+	const struct iio_counter_trigger *trigger;
+	ssize_t len = 0;
+
+	value = iio_counter_value_find_by_id(counter, chan->channel2);
+	if (!value)
+		return -EINVAL;
+
+	/* Print out a list of every Signal association to Value */
+	for (i = 0; i < value->num_triggers; i++) {
+		trigger = value->triggers + i;
+		len += snprintf(buf + len, PAGE_SIZE - len, "%d\t%s\t%s\n",
+			trigger->signal->id, trigger->signal->name,
+			trigger->trigger_modes[trigger->mode]);
+		if (len >= PAGE_SIZE)
+			return -ENOMEM;
+	}
+
+	return len;
+}
+
+static ssize_t iio_counter_trigger_mode_read(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
+{
+	struct iio_counter *const counter = iio_priv_get_counter(indio_dev);
+	struct iio_counter_value *value;
+	struct iio_counter_trigger *trigger;
+	const int signal_id = *(int *)((void *)priv);
+	int mode;
+
+	if (!counter->ops->trigger_mode_get)
+		return -EINVAL;
+
+	value = iio_counter_value_find_by_id(counter, chan->channel2);
+	if (!value)
+		return -EINVAL;
+
+	trigger = iio_counter_trigger_find_by_id(value, signal_id);
+	if (!trigger)
+		return -EINVAL;
+
+	mode = counter->ops->trigger_mode_get(counter, value, trigger);
+
+	if (mode < 0)
+		return mode;
+	else if (mode >= trigger->num_trigger_modes)
+		return -EINVAL;
+
+	trigger->mode = mode;
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n", trigger->trigger_modes[mode]);
+}
+
+static ssize_t iio_counter_trigger_mode_write(struct iio_dev *indio_dev,
+	uintptr_t priv, const struct iio_chan_spec *chan, const char *buf,
+	size_t len)
+{
+	struct iio_counter *const counter = iio_priv_get_counter(indio_dev);
+	struct iio_counter_value *value;
+	ssize_t err;
+	struct iio_counter_trigger *trigger;
+	const int signal_id = *(int *)((void *)priv);
+	unsigned int mode;
+
+	if (!counter->ops->trigger_mode_set)
+		return -EINVAL;
+
+	value = iio_counter_value_find_by_id(counter, chan->channel2);
+	if (!value)
+		return -EINVAL;
+
+	trigger = iio_counter_trigger_find_by_id(value, signal_id);
+	if (!trigger)
+		return -EINVAL;
+
+	for (mode = 0; mode < trigger->num_trigger_modes; mode++)
+		if (sysfs_streq(buf, trigger->trigger_modes[mode]))
+			break;
+
+	if (mode >= trigger->num_trigger_modes)
+		return -EINVAL;
+
+	err = counter->ops->trigger_mode_set(counter, value, trigger, mode);
+	if (err)
+		return err;
+
+	trigger->mode = mode;
+
+	return len;
+}
+
+static ssize_t iio_counter_trigger_mode_available_read(
+	struct iio_dev *indio_dev, uintptr_t priv,
+	const struct iio_chan_spec *chan, char *buf)
+{
+	struct iio_counter *const counter = iio_priv_get_counter(indio_dev);
+	struct iio_counter_value *value;
+	ssize_t len = 0;
+	struct iio_counter_trigger *trigger;
+	const int signal_id = *(int *)((void *)priv);
+	unsigned int i;
+
+	value = iio_counter_value_find_by_id(counter, chan->channel2);
+	if (!value)
+		return -EINVAL;
+
+	trigger = iio_counter_trigger_find_by_id(value, signal_id);
+	if (!trigger)
+		return -EINVAL;
+
+	for (i = 0; i < trigger->num_trigger_modes; i++)
+		len += scnprintf(buf + len, PAGE_SIZE - len, "%s ",
+			trigger->trigger_modes[i]);
+
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
+static int iio_counter_value_function_set(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan, unsigned int mode)
+{
+	struct iio_counter *const counter = iio_priv_get_counter(indio_dev);
+	struct iio_counter_value *value;
+	int err;
+
+	if (!counter->ops->value_function_set)
+		return -EINVAL;
+
+	/* Find relevant Value */
+	value = iio_counter_value_find_by_id(counter, chan->channel2);
+	if (!value)
+		return -EINVAL;
+
+	/* Map IIO core function_set to Generic Counter value_function_set */
+	err = counter->ops->value_function_set(counter, value, mode);
+	if (err)
+		return err;
+
+	value->mode = mode;
+
+	return 0;
+}
+
+static int iio_counter_value_function_get(struct iio_dev *indio_dev,
+	const struct iio_chan_spec *chan)

drivers/iio/industrialio-counter.c new file +142 -0

+	if (!value)
+		return -EINVAL;
+
+	/* Map IIO core function_get to Generic Counter value_function_get */
+	retval = counter->ops->value_function_get(counter, value);
+	if (retval < 0)
+		return retval;
+	else if (retval >= value->num_function_modes)
+		return -EINVAL;
+
+	value->mode = retval;
+
+	return retval;
+}
+
+static int iio_counter_value_ext_info_alloc(struct iio_chan_spec *const chan,
+	struct iio_counter_value *const value)
+{
+	const struct iio_chan_spec_ext_info ext_info_default[] = {
+		{
+			.name = "name",
+			.shared = IIO_SEPARATE,
+			.read = iio_counter_value_name_read
+		},
+		IIO_ENUM("function", IIO_SEPARATE, &value->function_enum),
+		{
+			.name = "function_available",
+			.shared = IIO_SEPARATE,
+			.read = iio_enum_available_read,
+			.private = (void *)&value->function_enum
+		},
+		{
+			.name = "triggers",
+			.shared = IIO_SEPARATE,
+			.read = iio_counter_value_triggers_read
+		}
+	};
+	const size_t num_default = ARRAY_SIZE(ext_info_default);
+	const struct iio_chan_spec_ext_info ext_info_trigger[] = {
+		{
+			.shared = IIO_SEPARATE,
+			.read = iio_counter_trigger_mode_read,
+			.write = iio_counter_trigger_mode_write
+		},
+		{
+			.shared = IIO_SEPARATE,
+			.read = iio_counter_trigger_mode_available_read
+		}
+	};
+	const size_t num_ext_info_trigger = ARRAY_SIZE(ext_info_trigger);
+	const size_t num_triggers_ext_info = num_ext_info_trigger *
+		value->num_triggers;
+	const size_t num_ext_info = num_default + num_triggers_ext_info + 1;
+	int err;
+	struct iio_chan_spec_ext_info *ext_info;
+	const struct iio_counter_trigger *trigger;
+	size_t i;
+	size_t j;
+
+	/* Construct function_enum for Value */
+	value->function_enum.items = value->function_modes;
+	value->function_enum.num_items = value->num_function_modes;
+	value->function_enum.set = iio_counter_value_function_set;
+	value->function_enum.get = iio_counter_value_function_get;
+
+	ext_info = kmalloc_array(num_ext_info, sizeof(*ext_info), GFP_KERNEL);
+	if (!ext_info)
+		return -ENOMEM;
+	ext_info[num_ext_info - 1].name = NULL;
+
+	/* Add ext_info for the name, function, function_available, and triggers
+	 * attributes
+	 */
+	memcpy(ext_info, ext_info_default, sizeof(ext_info_default));
+	/* Add ext_info for the trigger_signalX-Z and
+	 * trigger_signalX-Z_available attributes for each Trigger
+	 */
+	for (i = 0; i < num_triggers_ext_info; i += num_ext_info_trigger)
+		memcpy(ext_info + num_default + i, ext_info_trigger,
+			sizeof(ext_info_trigger));
+
+	/* Set name for each trigger_signalX-Z and trigger_signalX-Z_available
+	 * attribute; store the respective Signal ID address in each ext_info
+	 * private member
+	 */
+	for (i = num_default, j = 0; j < value->num_triggers; i++, j++) {
+		trigger = value->triggers + j;
+
+		ext_info[i].name = kasprintf(GFP_KERNEL, "trigger_signal%d-%d",
+			chan->channel, trigger->signal->id);
+		if (!ext_info[i].name) {
+			err = -ENOMEM;
+			goto err_name_alloc;
+		}
+		ext_info[i].private = (void *)&trigger->signal->id;
+		i++;
+
+		ext_info[i].name = kasprintf(GFP_KERNEL,
+			"trigger_signal%d-%d_available",
+			chan->channel, trigger->signal->id);
+		if (!ext_info[i].name) {
+			err = -ENOMEM;
+			goto err_name_alloc;
+		}
+		ext_info[i].private = (void *)&trigger->signal->id;
+	}
+
+	chan->ext_info = ext_info;
+
+	return 0;
+
+err_name_alloc:
+	while (i-- > num_default)
+		kfree(ext_info[i].name);
+	kfree(ext_info);
+	return err;
+}
+
+static void iio_counter_value_ext_info_free(
+	const struct iio_chan_spec *const channel)
+{
+	size_t i;
+	const char *const prefix = "trigger_signal";
+	const size_t prefix_len = strlen(prefix);
+
+	for (i = 0; channel->ext_info[i].name; i++)
+		if (!strncmp(channel->ext_info[i].name, prefix, prefix_len))
+			kfree(channel->ext_info[i].name);
+	kfree(channel->ext_info);
+}
+
+static const struct iio_chan_spec_ext_info iio_counter_signal_ext_info[] = {
+	{
+		.name = "name",
+		.shared = IIO_SEPARATE,
+		.read = iio_counter_signal_name_read
+	},
+	{}
+};
+
+static int iio_counter_channels_alloc(struct iio_counter *const counter)
+{

drivers/iio/industrialio-counter.c new file +11 -0

+	if (!channels)
+		return -ENOMEM;
+
+	/* If any channels were supplied on Counter registration,
+	 * we add them here to the front of the array
+	 */
+	memcpy(channels, counter->channels,
+		counter->num_channels * sizeof(*counter->channels));
+
+	/* Add channel for each Value */
+	for (j = 0; j < counter->num_values; j++) {

drivers/iio/industrialio-counter.c new file +97 -0

+		err = iio_counter_value_ext_info_alloc(channels + i, value);
+		if (err)
+			goto err_value_ext_info_alloc;
+
+		i++;
+	}
+
+	/* Add channel for each Signal */
+	for (j = 0; j < counter->num_signals; j++) {
+		signal = counter->signals + j;
+
+		channels[i].type = IIO_SIGNAL;
+		channels[i].channel = counter->id;
+		channels[i].channel2 = signal->id;
+		channels[i].info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+		channels[i].indexed = 1;
+		channels[i].counter = 1;
+		channels[i].ext_info = iio_counter_signal_ext_info;
+
+		i++;
+	}
+
+	counter->indio_dev->num_channels = num_channels;
+	counter->indio_dev->channels = channels;
+
+	return 0;
+
+err_value_ext_info_alloc:
+	while (i-- > counter->num_channels)
+		iio_counter_value_ext_info_free(channels + i);
+	kfree(channels);
+	return err;
+}
+
+static void iio_counter_channels_free(const struct iio_counter *const counter)
+{
+	size_t i = counter->num_channels + counter->indio_dev->num_channels;
+	const struct iio_chan_spec *const chans = counter->indio_dev->channels;
+
+	while (i-- > counter->num_channels)
+		/* Only IIO_COUNT channels need to be freed here */
+		if (chans[i].type == IIO_COUNT)
+			iio_counter_value_ext_info_free(chans + i);
+
+	kfree(chans);
+}
+
+static int iio_counter_read_raw(struct iio_dev *indio_dev,
+	struct iio_chan_spec const *chan, int *val, int *val2, long mask)
+{
+	struct iio_counter *const counter = iio_priv_get_counter(indio_dev);
+	struct iio_counter_signal *signal;
+	struct iio_counter_value *value;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	switch (chan->type) {
+	/* Map read_raw to signal_read for Signal */
+	case IIO_SIGNAL:
+		if (!counter->ops->signal_read)
+			return -EINVAL;
+
+		signal = iio_counter_signal_find_by_id(counter, chan->channel2);
+		if (!signal)
+			return -EINVAL;
+
+		return counter->ops->signal_read(counter, signal, val, val2);
+	/* Map read_raw to value_read for Value */
+	case IIO_COUNT:
+		if (!counter->ops->value_read)
+			return -EINVAL;
+
+		value = iio_counter_value_find_by_id(counter, chan->channel2);
+		if (!value)
+			return -EINVAL;
+
+		return counter->ops->value_read(counter, value, val, val2);
+	/* Map read_raw to read_raw for non-counter channel */
+	default:
+		if (counter->info && counter->info->read_raw)
+			return counter->info->read_raw(indio_dev, chan, val,
+				val2, mask);
+	}
+
+	return -EINVAL;
+}
+
+static int iio_counter_write_raw(struct iio_dev *indio_dev,
+	struct iio_chan_spec const *chan, int val, int val2, long mask)
+{
+	struct iio_counter *const counter = iio_priv_get_counter(indio_dev);
+	struct iio_counter_signal *signal;
+	struct iio_counter_value *value;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;

drivers/iio/industrialio-counter.c new file +36 -0

+		if (!value)
+			return -EINVAL;
+
+		return counter->ops->value_write(counter, value, val, val2);
+	/* Map write_raw to write_raw for non-counter channel */
+	default:
+		if (counter->info && counter->info->write_raw)
+			return counter->info->write_raw(indio_dev, chan, val,
+				val2, mask);
+	}
+
+	return -EINVAL;
+}
+
+static int iio_counter_signals_validate(const struct iio_counter *const counter)
+{
+	size_t i;
+	const struct iio_counter_signal *signal;
+	size_t j;
+	int curr_id;
+
+	/* At least one Signal must be defined */
+	if (!counter->num_signals || !counter->signals) {
+		pr_err("Counter '%d' Signals undefined\n", counter->id);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < counter->num_signals; i++) {
+		signal = counter->signals + i;
+		/* No two Signals may share the same ID */
+		for (j = 0; j < i; j++) {
+			curr_id = counter->signals[j].id;
+			if (curr_id == signal->id) {
+				pr_err("Duplicate Counter '%d' Signal '%d'\n",
+						counter->id, signal->id);
+				return -EEXIST;

drivers/iio/industrialio-counter.c new file +12 -0

+	}
+
+	return 0;
+}
+
+static int iio_counter_triggers_validate(const int counter_id,
+	const struct iio_counter_value *const value)
+{
+	size_t i;
+	const struct iio_counter_trigger *trigger;
+	size_t j;
+	int curr_id;

drivers/iio/industrialio-counter.c new file +20 -0

+	 */
+	for (i = 0; i < value->num_triggers; i++)
+		if (!value->triggers[i].signal) {
+			pr_err("Counter '%d' Trigger '%zu' Signal undefined\n",
+				counter_id, i);
+			return -EINVAL;
+		}
+
+	/* Verify validity of each Trigger */
+	for (i = 0; i < value->num_triggers; i++) {
+		trigger = value->triggers + i;
+		/* No two Trigger Signals may share the same ID */
+		for (j = 0; j < i; j++) {
+			curr_id = value->triggers[j].signal->id;
+			if (curr_id == trigger->signal->id) {
+				pr_err("Signal '%d' is already linked to Counter '%d' Value '%d'\n",
+					trigger->signal->id, counter_id,
+					value->id);
+				return -EEXIST;
+			}

drivers/iio/industrialio-counter.c new file +4 -0

+
+		/* At least one trigger mode must be defined for each Trigger */
+		if (!trigger->num_trigger_modes || !trigger->trigger_modes) {
+			pr_err("Counter '%d' Signal '%d' trigger modes undefined\n",

drivers/iio/industrialio-counter.c new file +7 -0

+	size_t i;
+	const struct iio_counter_value *value;
+	size_t j;
+	int curr_id;
+	int err;
+
+	/* At least one Value must be defined */

drivers/iio/industrialio-counter.c new file +52 -0

+			curr_id = counter->values[j].id;
+			if (curr_id == value->id) {
+				pr_err("Duplicate Counter '%d' Value '%d'\n",
+					counter->id, value->id);
+				return -EEXIST;
+			}
+		}
+		for (j = i + 1; j < counter->num_values; j++) {
+			curr_id = counter->values[j].id;
+			if (curr_id == value->id) {
+				pr_err("Duplicate Counter '%d' Value '%d'\n",
+					counter->id, value->id);
+				return -EEXIST;
+			}
+		}
+
+		/* At least one function mode must be defined for each Value */
+		if (!value->num_function_modes || !value->function_modes) {
+			pr_err("Counter '%d' Value '%d' function modes undefined\n",
+				counter->id, value->id);
+			return -EINVAL;
+		}
+
+		/* Verify the Triggers associated with each Value */
+		err = iio_counter_triggers_validate(counter->id, value);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+/**
+ * iio_counter_register - register Counter to the system
+ * @counter: pointer to IIO Counter to register
+ *
+ * This function piggybacks off of iio_device_register. First, the relevant
+ * Counter members are validated; the signals and values members must be defined
+ * and populated by valid Signal and Value structures respectively. Next, a
+ * struct iio_dev is allocated by a call to iio_device_alloc and initialized for
+ * the Counter, IIO channels are allocated, the Counter address is stored as the
+ * private data, and finally iio_device_register is called.
+ */
+int iio_counter_register(struct iio_counter *const counter)
+{
+	const struct iio_info info_default = {
+		.read_raw = iio_counter_read_raw,
+		.write_raw = iio_counter_write_raw
+	};
+	int err;
+	struct iio_info *info;
+	struct iio_counter **priv;

drivers/iio/industrialio-counter.c new file +159 -0

+	 * Triggers for each Value are also verified for validity
+	 */
+	err = iio_counter_values_validate(counter);
+	if (err)
+		return err;
+
+	counter->indio_dev = iio_device_alloc(sizeof(counter));
+	if (!counter->indio_dev)
+		return -ENOMEM;
+
+	info = kmalloc(sizeof(*info), GFP_KERNEL);
+	if (!info) {
+		err = -ENOMEM;
+		goto err_info_alloc;
+	}
+	/* If an iio_info has been supplied than we use that,
+	 * otherwise we set all callbacks to NULL; iio_counter_read_raw
+	 * and iio_counter_write_raw is used for read_raw and write_raw
+	 * for either case in order to support counter functionality
+	 * (supplied read_raw/write_raw will be called from within
+	 * iio_counter_read_raw/iio_counter_write_raw for non-counter
+	 * channels)
+	 */
 
+	if (counter->info) {
+		memcpy(info, counter->info, sizeof(*counter->info));
+		info->read_raw = iio_counter_read_raw;
+		info->write_raw = iio_counter_write_raw;
+	} else {
+		memcpy(info, &info_default, sizeof(info_default));
+	}
+
+	counter->indio_dev->info = info;
+	counter->indio_dev->modes = INDIO_DIRECT_MODE;
+	counter->indio_dev->name = counter->name;
+	counter->indio_dev->dev.parent = counter->dev;
+
+	/* IIO channels are allocated and set for Signals, Values, and Triggers;
+	 * any auxiliary IIO channels provided in iio_counter are also set
+	 */
+	err = iio_counter_channels_alloc(counter);
+	if (err)
+		goto err_channels_alloc;
+
+	/* Pointer to the counter is stored in indio_dev as a way to refer
+	 * back to the counter from within various IIO callbacks
+	 */
+	priv = iio_priv(counter->indio_dev);
+	memcpy(priv, &counter, sizeof(*priv));
+
+	err = iio_device_register(counter->indio_dev);
+	if (err)
+		goto err_iio_device_register;
+
+	return 0;
+
+err_iio_device_register:
+	iio_counter_channels_free(counter);
+err_channels_alloc:
+	kfree(info);
+err_info_alloc:
+	iio_device_free(counter->indio_dev);
+	return err;
+}
+EXPORT_SYMBOL(iio_counter_register);
+
+/**
+ * iio_counter_unregister - unregister Counter from the system
+ * @counter: pointer to IIO Counter to unregister
+ *
+ * The Counter is unregistered from the system. The indio_dev is unregistered
+ * and all allocated memory is freed.
+ */
+void iio_counter_unregister(struct iio_counter *const counter)
+{
+	const struct iio_info *const info = counter->indio_dev->info;
+
+	if (!counter)
+		return;
+
+	iio_device_unregister(counter->indio_dev);
+
+	iio_counter_channels_free(counter);
+
+	kfree(info);
+	iio_device_free(counter->indio_dev);
+}
+EXPORT_SYMBOL(iio_counter_unregister);
+
+static void devm_iio_counter_unreg(struct device *dev, void *res)
+{
+	iio_counter_unregister(*(struct iio_counter **)res);
+}
+
+/**
+ * devm_iio_counter_register - Resource-managed iio_counter_register
+ * @dev: Device to allocate iio_counter for
+ * @counter: pointer to IIO Counter to register
+ *
+ * Managed iio_counter_register. The IIO counter registered with this
+ * function is automatically unregistered on driver detach. This function
+ * calls iio_counter_register internally. Refer to that function for more
+ * information.
+ *
+ * If an iio counter registered with this function needs to be unregistered
+ * separately, devm_iio_counter_unregister must be used.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.
+ */
+int devm_iio_counter_register(struct device *dev,
+	struct iio_counter *const counter)
+{
+	struct iio_counter **ptr;
+	int ret;
+
+	ptr = devres_alloc(devm_iio_counter_unreg, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return -ENOMEM;
+
+	*ptr = counter;
+	ret = iio_counter_register(counter);
+	if (!ret)
+		devres_add(dev, ptr);
+	else
+		devres_free(ptr);
+
+	return ret;
+}
+EXPORT_SYMBOL(devm_iio_counter_register);
+
+static int devm_iio_counter_match(struct device *dev, void *res, void *data)
+{
+	struct iio_counter **r = res;
+
+	if (!r || !*r) {
+		WARN_ON(!r || !*r);
+		return 0;
+	}
+
+	return *r == data;
+}
+
+/**
+ * devm_iio_counter_unregister - Resource-managed iio_counter_unregister
+ * @dev: Device this iio_counter belongs to
+ * @counter: the iio counter associated with the device
+ *
+ * Unregister iio counter registered with devm_iio_counter_register.
+ */
+void devm_iio_counter_unregister(struct device *dev,
+	struct iio_counter *const counter)
+{
+	int rc;
+
+	rc = devres_release(dev, devm_iio_counter_unreg,
+		devm_iio_counter_match, counter);
+	WARN_ON(rc);
+}
+EXPORT_SYMBOL(devm_iio_counter_unregister);

include/linux/iio/counter.h new file +35 -0

new file mode 100644
index 000000000000..35645406711a
@@ -0,0 +1,166 @@
+/*
+ * Industrial I/O counter interface
+ * Copyright (C) 2017 William Breathitt Gray
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+#ifndef _IIO_COUNTER_H_
+#define _IIO_COUNTER_H_
+
+#ifdef CONFIG_IIO_COUNTER
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+#include <linux/iio/iio.h>
+
+/**
+ * struct iio_counter_signal - IIO Counter Signal node
+ * @id:		[DRIVER] unique ID used to identify signal
+ * @name:	[DRIVER] device-specific signal name
+ */
+struct iio_counter_signal {
+	int		id;
+	const char	*name;
+};
+
+/**
+ * struct iio_counter_trigger - IIO Counter Trigger node

include/linux/iio/counter.h new file +24 -0

+};
+
+/**
+ * struct iio_counter_value - IIO Counter Value node
+ * @id:			[DRIVER] unique ID used to identify value
+ * @name:		[DRIVER] device-specific value name
+ * @mode:		[DRIVER] current function mode state
+ * @function_modes:	[DRIVER] available function modes
+ * @num_function_modes:	[DRIVER] number of modes specified in @function_modes
+ * @triggers:		[DRIVER] array of triggers for initialization
+ * @num_triggers:	[DRIVER] number of triggers specified in @triggers
+ * @function_enum:	[INTERN] used internally to generate function attributes
+ */
+struct iio_counter_value {
+	int			id;
+	const char		*name;
+	unsigned int		mode;
+	const char *const	*function_modes;
+	unsigned int		num_function_modes;
+
+	struct iio_counter_trigger	*triggers;
+	size_t				num_triggers;
+
+	struct iio_enum		function_enum;

include/linux/iio/counter.h new file +47 -0

+ * @trigger_mode_set:	function to set the trigger mode. mode is the index of
+ *			the requested mode from the value trigger_modes array.
+ * @trigger_mode_get:	function to get the current trigger mode. Return value
+ *			will specify the index of the current mode from the
+ *			value trigger_modes array.
+ * @value_read:		function to request a value value from the device.
+ *			Return value will specify the type of value returned by
+ *			the device. val and val2 will contain the elements
+ *			making up the returned value.
+ * @value_write:	function to write a value value to the device.
+ *                      Parameters are interpreted the same as value_read.
+ * @value_function_set: function to set the value function mode. mode is the
+ *			index of the requested mode from the value
+ *			function_modes array.
+ * @value_function_get: function to get the current value function mode. Return
+ *			value will specify the index of the current mode from
+ *			the value function_modes array.
+ */
+struct iio_counter_ops {
+	int (*signal_read)(struct iio_counter *counter,
+		struct iio_counter_signal *signal, int *val, int *val2);
+	int (*signal_write)(struct iio_counter *counter,
+		struct iio_counter_signal *signal, int val, int val2);
+	int (*trigger_mode_set)(struct iio_counter *counter,
+		struct iio_counter_value *value,
+		struct iio_counter_trigger *trigger, unsigned int mode);
+	int (*trigger_mode_get)(struct iio_counter *counter,
+		struct iio_counter_value *value,
+		struct iio_counter_trigger *trigger);
+	int (*value_read)(struct iio_counter *counter,
+		struct iio_counter_value *value, int *val, int *val2);
+	int (*value_write)(struct iio_counter *counter,
+		struct iio_counter_value *value, int val, int val2);
+	int (*value_function_set)(struct iio_counter *counter,
+		struct iio_counter_value *value, unsigned int mode);
+	int (*value_function_get)(struct iio_counter *counter,
+		struct iio_counter_value *value);
+};
+
+/**
+ * struct iio_counter - IIO Counter data structure
+ * @id:			[DRIVER] unique ID used to identify counter
+ * @name:		[DRIVER] name of the device
+ * @dev:		[DRIVER] device structure, should be assigned a parent
+ *			and owner
+ * @ops:		[DRIVER] callbacks from driver for counter components
+ * @signals:		[DRIVER] array of signals for initialization

include/linux/iio/counter.h new file +28 -0

+	int				id;
+	const char			*name;
+	struct device			*dev;
+	const struct iio_counter_ops	*ops;
+
+	struct iio_counter_signal	*signals;
+	size_t				num_signals;
+	struct iio_counter_value	*values;
+	size_t				num_values;
+
+	const struct iio_chan_spec	*channels;
+	size_t				num_channels;
+	const struct iio_info		*info;
+
+	struct iio_dev	*indio_dev;
+	void		*driver_data;
+};
+
+int iio_counter_register(struct iio_counter *const counter);
+void iio_counter_unregister(struct iio_counter *const counter);
+int devm_iio_counter_register(struct device *dev,
+	struct iio_counter *const counter);
+void devm_iio_counter_unregister(struct device *dev,
+	struct iio_counter *const counter);
+
+#endif /* CONFIG_IIO_COUNTER */
+
+#endif /* _IIO_COUNTER_H_ */

Quoted 6 lines · On Thu, 5 Oct 2017 14:13:44 -0400 William Breathitt Gray…

This patch introduces the IIO generic counter interface for supporting counter devices. The generic counter interface serves as a catch-all to enable rudimentary support for devices that qualify as counters. More specific and apt counter interfaces may be developed on top of the generic counter interface, and those interfaces should be used by drivers when possible rather than the generic counter interface.

I think you probably want to avoid any additional layering and try to push things down here (maybe with a few utility functions to simplify common cases), but that's a question for another day!

Quoted 34 lines

In the context of the IIO generic counter interface, a counter is defined as a device that reports one or more "counter values" based on the state changes of one or more "counter signals" as evaluated by a defined "counter function."

The IIO generic counter interface piggybacks off of the IIO core. This is primarily used to leverage the existing sysfs setup: the IIO_COUNT channel attributes represent the Counter Value, while the IIO_SIGNAL channel attributes represent the Counter Signal; auxilary IIO_COUNT attributes represent the Counter Signal connections (Triggers) and their respective state change configurations which trigger an associated "counter function" evaluation.

The iio_counter_ops structure serves as a container for driver callbacks to communicate with the device; function callbacks are provided to read and write various Signals and Values, and set and get the "trigger mode" and "function mode" for various Triggers and Values respectively.

To support a counter device, a driver must first allocate the available Counter Signals via iio_counter_signal structures. These Signals should be stored as an array and set to the signals array member of an allocated iio_counter structure before the Counter is registered to the system.

Counter Values may be allocated via iio_counter_value structures, and respective Counter Signal associations (Triggers) made via iio_counter_trigger structures. Associated iio_counter_trigger structures are stored as an array and set to the the triggers array member of the respective iio_counter_value structure. These iio_counter_value structures are set to the values array member of an allocated iio_counter structure before the Counter is registered to the system.

Quoted 7 lines

A counter device is registered to the system by passing the respective initialized iio_counter structure to the iio_counter_register function; similarly, the iio_counter_unregister function unregisters the respective counter. The devm_iio_counter_register and iio_devm_iio_counter_unregister functions serve as device memory-managed versions of the iio_counter_register and iio_counter unregister functions respectively.

All the find by ids are the main complexity introduced by layering this on IIO that you probably wouldn't have if it wasn't so layered. Hmm. I suggest inline that you could allow drivers to provide fast versions of that search given they can probably infer it directly from the index. Might be something to ignore for now though in the interests of initial simplicity.

Otherwise a few comments inline, but mostly this is coming together pretty well. I'm actually surprised the layering on top of IIO didn't end up more painful than it did. It's not horrendous - which is not to say you wouldn't be better breaking free of that entirely... (I'm not sure either way).

I think the big remaining questions are around naming more than anything else. I don't like the name value when a user will think they are lookin=

for a counter. If you use counter though the current use of counter need=

to change.

Trigger is also, as has been raised, a rather overloaded term. Not totall=

obvious what a better term is but we shouldn't reuse that one...

I'll try and think of something if you have no luck!

Jonathan

Quoted 8 lines

Signed-off-by William Breathitt Gray <[email protected]>

---
 MAINTAINERS                        |   7 +
 drivers/iio/Kconfig                |   8 +
 drivers/iio/Makefile               |   1 +
 drivers/iio/counter/Kconfig        |   1 +

drivers/iio/industrialio-counter.c | 900 ++++++++++++++++++++++++++++++=

MAINTAINERS F: Documentation/ABI/testing/sysfs-bus-iio-ad=

Quoted 10 lines

 include/linux/iio/counter.h        | 166 +++++++
 6 files changed, 1083 insertions(+)
 create mode 100644 drivers/iio/industrialio-counter.c
 create mode 100644 include/linux/iio/counter.h

MAINTAINERS +0 -0

index 2281af4b41b6..8b7c37bed252 100644
@@ -6693,6 +6693,13 @@ F:     Documentation/ABI/testing/sysfs-bus-iio-ad=

MAINTAINERS F: Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector · line 6701

Quoted 9 lines

MAINTAINERS +6 -0

 F:   Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
 F:   drivers/iio/adc/envelope-detector.c
 
+IIO GENERIC COUNTER INTERFACE
+M:   William Breathitt Gray <[email protected]>
+L:   [email protected]
+S:   Maintained
+F:   drivers/iio/industrialio-counter.c
+F:   include/linux/iio/counter.h

Don't forget the directory the drivers are going in ;) *laughs manically*

Quoted 42 lines

MAINTAINERS +1 -0

+
 IIO MULTIPLEXER
 M:   Peter Rosin <[email protected]>
 L:   [email protected]

drivers/iio/Kconfig +8 -0

index b3c8c6ef0dff..78e01f4f5937 100644
@@ -30,6 +30,14 @@ config IIO_CONFIGFS
        (e.g. software triggers). For more info see
        Documentation/iio/iio_configfs.txt.
 
+config IIO_COUNTER
+     bool "Enable IIO counter support"
+     help
+       Provides IIO core support for counters. This API provides
+       a generic interface that serves as the building blocks to
+       create more complex counter interfaces. Rudimentary support
+       for counters is enabled.
+
 config IIO_TRIGGER
      bool "Enable triggered sampling support"
      help

drivers/iio/Makefile +1 -0

index 93c769cd99bf..6427ff38f964 100644
@@ -5,6 +5,7 @@
 obj-$(CONFIG_IIO) +=3D industrialio.o
 industrialio-y :=3D industrialio-core.o industrialio-event.o inkern.o
 industrialio-$(CONFIG_IIO_BUFFER) +=3D industrialio-buffer.o
+industrialio-$(CONFIG_IIO_COUNTER) +=3D industrialio-counter.o
 industrialio-$(CONFIG_IIO_TRIGGER) +=3D industrialio-trigger.o
 
 obj-$(CONFIG_IIO_CONFIGFS) +=3D industrialio-configfs.o

drivers/iio/counter/Kconfig +0 -0

index 474e1ac4e7c0..c8becfe78e28 100644
@@ -4,6 +4,7 @@
 # When adding new entries keep the list in alphabetical order
 

drivers/iio/industrial=

Quoted 6 lines

drivers/iio/counter/Kconfig +1 -0

 menu "Counters"
+     depends on IIO_COUNTER
 
 config 104_QUAD_8
      tristate "ACCES 104-QUAD-8 driver"

drivers/iio/industrial= renamed from drivers/iio/industrialio-counter.c +0 -0

Quoted 60 lines

new file mode 100644
index 000000000000..dfb982dae3a8

drivers/iio/industrialio-counter.c new file +55 -0

@@ -0,0 +1,900 @@
+/*
+ * Industrial I/O counter interface functions
+ * Copyright (C) 2017 William Breathitt Gray
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+
+#include <linux/iio/counter.h>
+
+#define iio_priv_get_counter(_indio_dev) \
+     (*(struct iio_counter **)iio_priv(_indio_dev))
+
+static struct iio_counter_signal *iio_counter_signal_find_by_id(
+     const struct iio_counter *const counter, const int id)
+{
+     size_t i;
+     struct iio_counter_signal *signal;
+
+     for (i =3D 0; i < counter->num_signals; i++) {
+             signal =3D counter->signals + i;
+             if (signal->id =3D=3D id)
+                     return signal;
+     }
+
+     return NULL;
+}
+
+static struct iio_counter_trigger *iio_counter_trigger_find_by_id(
+     const struct iio_counter_value *const value, const int id)
+{
+     size_t i;
+     struct iio_counter_trigger *trigger;
+
+     for (i =3D 0; i < value->num_triggers; i++) {
+             trigger =3D value->triggers + i;
+             if (trigger->signal->id =3D=3D id)
+                     return trigger;

drivers/iio/industrialio-counter.c line 65

Quoted 10 lines

drivers/iio/industrialio-counter.c new file +10 -0

+     }
+
+     return NULL;
+}
+
+static struct iio_counter_value *iio_counter_value_find_by_id(
+     const struct iio_counter *const counter, const int id)
+{
+     size_t i;
+     struct iio_counter_value *value;

In most cases this mapping will be entirely obvious to the driver - perhaps provide an optional callback to let it provide it directly without any searching?

We could always add this later though if we start getting drivers with lots of instances of the various parts...

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+
+     for (i =3D 0; i < counter->num_values; i++) {
+             value =3D counter->values + i;
+             if (value->id =3D=3D id)

drivers/iio/industrialio-counter.c

Quoted 10 lines

drivers/iio/industrialio-counter.c new file +10 -0

+                     return value;
+     }
+
+     return NULL;
+}
+
+static ssize_t iio_counter_signal_name_read(struct iio_dev *indio_dev,
+     uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+     const struct iio_counter_signal *signal;
+
+     signal =3D iio_counter_signal_find_by_id(counter, chan->channel2);
+     if (!signal)
+             return -EINVAL;

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+     return scnprintf(buf, PAGE_SIZE, "%s\n", signal->name);
+}
+
+static ssize_t iio_counter_value_name_read(struct iio_dev *indio_dev,
+     uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

drivers/iio/industrialio-counter.c line 99

Quoted 7 lines

drivers/iio/industrialio-counter.c new file +7 -0

+     const struct iio_counter_value *value;
+
+     value =3D iio_counter_value_find_by_id(counter, chan->channel2);
+     if (!value)
+             return -EINVAL;
+
+     return scnprintf(buf, PAGE_SIZE, "%s\n", value->name);

I mentioned before but I wonder if we can just push name down into the chan spec itself in some fashion. Whether to associate it with existing datasheet_name or not is an open question however.

drivers/iio/industrialio-counter.c

Quoted 3 lines

drivers/iio/industrialio-counter.c new file +3 -0

+}
+
+static ssize_t iio_counter_value_triggers_read(struct iio_dev *indio_de=

drivers/iio/industrialio-counter.c

Quoted 3 lines

drivers/iio/industrialio-counter.c new file +3 -0

+     uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+     struct iio_counter_value *value;
+     size_t i;
+     const struct iio_counter_trigger *trigger;
+     ssize_t len =3D 0;
+

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     value =3D iio_counter_value_find_by_id(counter, chan->channel2);
+     if (!value)
+             return -EINVAL;
+
+     /* Print out a list of every Signal association to Value */
+     for (i =3D 0; i < value->num_triggers; i++) {
+             trigger =3D value->triggers + i;
+             len +=3D snprintf(buf + len, PAGE_SIZE - len, "%d\t%s\t%s\=

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+                     trigger->signal->id, trigger->signal->name,
+                     trigger->trigger_modes[trigger->mode]);
+             if (len >=3D PAGE_SIZE)
+                     return -ENOMEM;
+     }

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+     return len;
+}
+
+static ssize_t iio_counter_trigger_mode_read(struct iio_dev *indio_dev,
+     uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

Quoted 18 lines

drivers/iio/industrialio-counter.c new file +18 -0

+     struct iio_counter_value *value;
+     struct iio_counter_trigger *trigger;
+     const int signal_id =3D *(int *)((void *)priv);
+     int mode;
+
+     if (!counter->ops->trigger_mode_get)
+             return -EINVAL;
+
+     value =3D iio_counter_value_find_by_id(counter, chan->channel2);
+     if (!value)
+             return -EINVAL;
+
+     trigger =3D iio_counter_trigger_find_by_id(value, signal_id);
+     if (!trigger)
+             return -EINVAL;
+
+     mode =3D counter->ops->trigger_mode_get(counter, value, trigger);
+

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     if (mode < 0)
+             return mode;
+     else if (mode >=3D trigger->num_trigger_modes)
+             return -EINVAL;
+
+     trigger->mode =3D mode;
+
+     return scnprintf(buf, PAGE_SIZE, "%s\n", trigger->trigger_modes[mo=

drivers/iio/industrialio-counter.c

Quoted 3 lines

drivers/iio/industrialio-counter.c new file +3 -0

+}
+
+static ssize_t iio_counter_trigger_mode_write(struct iio_dev *indio_dev=

drivers/iio/industrialio-counter.c

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+     uintptr_t priv, const struct iio_chan_spec *chan, const char *buf,
+     size_t len)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

Quoted 17 lines

drivers/iio/industrialio-counter.c new file +17 -0

+     struct iio_counter_value *value;
+     ssize_t err;
+     struct iio_counter_trigger *trigger;
+     const int signal_id =3D *(int *)((void *)priv);
+     unsigned int mode;
+
+     if (!counter->ops->trigger_mode_set)
+             return -EINVAL;
+
+     value =3D iio_counter_value_find_by_id(counter, chan->channel2);
+     if (!value)
+             return -EINVAL;
+
+     trigger =3D iio_counter_trigger_find_by_id(value, signal_id);
+     if (!trigger)
+             return -EINVAL;
+

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     for (mode =3D 0; mode < trigger->num_trigger_modes; mode++)
+             if (sysfs_streq(buf, trigger->trigger_modes[mode]))
+                     break;
+
+     if (mode >=3D trigger->num_trigger_modes)
+             return -EINVAL;
+
+     err =3D counter->ops->trigger_mode_set(counter, value, trigger, mo=

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+     if (err)
+             return err;
+
+     trigger->mode =3D mode;
+

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     return len;
+}
+
+static ssize_t iio_counter_trigger_mode_available_read(
+     struct iio_dev *indio_dev, uintptr_t priv,
+     const struct iio_chan_spec *chan, char *buf)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

Quoted 19 lines

drivers/iio/industrialio-counter.c new file +19 -0

+     struct iio_counter_value *value;
+     ssize_t len =3D 0;
+     struct iio_counter_trigger *trigger;
+     const int signal_id =3D *(int *)((void *)priv);
+     unsigned int i;
+
+     value =3D iio_counter_value_find_by_id(counter, chan->channel2);
+     if (!value)
+             return -EINVAL;
+
+     trigger =3D iio_counter_trigger_find_by_id(value, signal_id);
+     if (!trigger)
+             return -EINVAL;
+
+     for (i =3D 0; i < trigger->num_trigger_modes; i++)
+             len +=3D scnprintf(buf + len, PAGE_SIZE - len, "%s ",
+                     trigger->trigger_modes[i]);
+
+     buf[len - 1] =3D '\n';

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+     return len;
+}
+
+static int iio_counter_value_function_set(struct iio_dev *indio_dev,
+     const struct iio_chan_spec *chan, unsigned int mode)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+     struct iio_counter_value *value;
+     int err;
+
+     if (!counter->ops->value_function_set)

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+             return -EINVAL;
+
+     /* Find relevant Value */
+     value =3D iio_counter_value_find_by_id(counter, chan->channel2);
+     if (!value)
+             return -EINVAL;
+
+     /* Map IIO core function_set to Generic Counter value_function_set=

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+     err =3D counter->ops->value_function_set(counter, value, mode);
+     if (err)
+             return err;
+
+     value->mode =3D mode;

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+     return 0;
+}
+
+static int iio_counter_value_function_get(struct iio_dev *indio_dev,
+     const struct iio_chan_spec *chan)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     struct iio_counter_value *value;
+     int retval;
+
+     if (!counter->ops->value_function_get)
+             return -EINVAL;
+
+     /* Find relevant Value */
+     value =3D iio_counter_value_find_by_id(counter, chan->channel2);

Same argument as below - would the driver not be able to do this better? Often it would know a simple transform to get the right one...

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+     if (!value)
+             return -EINVAL;
+
+     /* Map IIO core function_get to Generic Counter value_function_get=

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+     retval =3D counter->ops->value_function_get(counter, value);
+     if (retval < 0)
+             return retval;
+     else if (retval >=3D value->num_function_modes)

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+             return -EINVAL;
+
+     value->mode =3D retval;
+
+     return retval;
+}
+
+static int iio_counter_value_ext_info_alloc(struct iio_chan_spec *const=

Quoted 26 lines

drivers/iio/industrialio-counter.c new file +26 -0

+     struct iio_counter_value *const value)
+{
+     const struct iio_chan_spec_ext_info ext_info_default[] =3D {
+             {
+                     .name =3D "name",
+                     .shared =3D IIO_SEPARATE,
+                     .read =3D iio_counter_value_name_read
+             },
+             IIO_ENUM("function", IIO_SEPARATE, &value->function_enum),
+             {
+                     .name =3D "function_available",
+                     .shared =3D IIO_SEPARATE,
+                     .read =3D iio_enum_available_read,
+                     .private =3D (void *)&value->function_enum
+             },
+             {
+                     .name =3D "triggers",
+                     .shared =3D IIO_SEPARATE,
+                     .read =3D iio_counter_value_triggers_read
+             }
+     };
+     const size_t num_default =3D ARRAY_SIZE(ext_info_default);
+     const struct iio_chan_spec_ext_info ext_info_trigger[] =3D {
+             {
+                     .shared =3D IIO_SEPARATE,
+                     .read =3D iio_counter_trigger_mode_read,

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+                     .write =3D iio_counter_trigger_mode_write
+             },
+             {
+                     .shared =3D IIO_SEPARATE,
+                     .read =3D iio_counter_trigger_mode_available_read
+             }
+     };
+     const size_t num_ext_info_trigger =3D ARRAY_SIZE(ext_info_trigger)=

drivers/iio/industrialio-counter.c line 315

Quoted 3 lines

drivers/iio/industrialio-counter.c new file +3 -0

+     const size_t num_triggers_ext_info =3D num_ext_info_trigger *
+             value->num_triggers;
+     const size_t num_ext_info =3D num_default + num_triggers_ext_info =

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+     int err;
+     struct iio_chan_spec_ext_info *ext_info;
+     const struct iio_counter_trigger *trigger;
+     size_t i;
+     size_t j;

drivers/iio/industrialio-counter.c line 328

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+     /* Construct function_enum for Value */
+     value->function_enum.items =3D value->function_modes;
+     value->function_enum.num_items =3D value->num_function_modes;
+     value->function_enum.set =3D iio_counter_value_function_set;
+     value->function_enum.get =3D iio_counter_value_function_get;
+
+     ext_info =3D kmalloc_array(num_ext_info, sizeof(*ext_info), GFP_KE=

drivers/iio/industrialio-counter.c

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+     if (!ext_info)
+             return -ENOMEM;
+     ext_info[num_ext_info - 1].name =3D NULL;
+
+     /* Add ext_info for the name, function, function_available, and tr=

drivers/iio/industrialio-counter.c

Quoted 7 lines

drivers/iio/industrialio-counter.c new file +7 -0

+      * attributes
+      */
+     memcpy(ext_info, ext_info_default, sizeof(ext_info_default));
+     /* Add ext_info for the trigger_signalX-Z and
+      * trigger_signalX-Z_available attributes for each Trigger
+      */
+     for (i =3D 0; i < num_triggers_ext_info; i +=3D num_ext_info_trigg=

drivers/iio/industrialio-counter.c

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+             memcpy(ext_info + num_default + i, ext_info_trigger,
+                     sizeof(ext_info_trigger));
+
+     /* Set name for each trigger_signalX-Z and trigger_signalX-Z_avail=

drivers/iio/industrialio-counter.c line 345

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+      * attribute; store the respective Signal ID address in each ext_i=

drivers/iio/industrialio-counter.c

Quoted 3 lines

drivers/iio/industrialio-counter.c new file +3 -0

+      * private member
+      */
+     for (i =3D num_default, j =3D 0; j < value->num_triggers; i++, j++=

drivers/iio/industrialio-counter.c line 351

Quoted 3 lines

drivers/iio/industrialio-counter.c new file +3 -0

+             trigger =3D value->triggers + j;
+
+             ext_info[i].name =3D kasprintf(GFP_KERNEL, "trigger_signal=

Quoted 30 lines

drivers/iio/industrialio-counter.c new file +30 -0

+                     chan->channel, trigger->signal->id);
+             if (!ext_info[i].name) {
+                     err =3D -ENOMEM;
+                     goto err_name_alloc;
+             }
+             ext_info[i].private =3D (void *)&trigger->signal->id;
+             i++;
+
+             ext_info[i].name =3D kasprintf(GFP_KERNEL,
+                     "trigger_signal%d-%d_available",
+                     chan->channel, trigger->signal->id);
+             if (!ext_info[i].name) {
+                     err =3D -ENOMEM;
+                     goto err_name_alloc;
+             }
+             ext_info[i].private =3D (void *)&trigger->signal->id;
+     }
+
+     chan->ext_info =3D ext_info;
+
+     return 0;
+
+err_name_alloc:
+     while (i-- > num_default)
+             kfree(ext_info[i].name);
+     kfree(ext_info);
+     return err;
+}
+
+static void iio_counter_value_ext_info_free(

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     const struct iio_chan_spec *const channel)
+{
+     size_t i;
+     const char *const prefix =3D "trigger_signal";
+     const size_t prefix_len =3D strlen(prefix);
+
+     for (i =3D 0; channel->ext_info[i].name; i++)
+             if (!strncmp(channel->ext_info[i].name, prefix, prefix_len=

drivers/iio/industrialio-counter.c

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+                     kfree(channel->ext_info[i].name);
+     kfree(channel->ext_info);
+}
+
+static const struct iio_chan_spec_ext_info iio_counter_signal_ext_info[=

drivers/iio/industrialio-counter.c

Quoted 9 lines

drivers/iio/industrialio-counter.c new file +9 -0

+     {
+             .name =3D "name",
+             .shared =3D IIO_SEPARATE,
+             .read =3D iio_counter_signal_name_read
+     },
+     {}
+};
+
+static int iio_counter_channels_alloc(struct iio_counter *const counter=

drivers/iio/industrialio-counter.c line 405

Quoted 2 lines

drivers/iio/industrialio-counter.c new file +2 -0

+{
+     const size_t num_channels =3D counter->num_signals + counter->num_=

drivers/iio/industrialio-counter.c

Quoted 9 lines

drivers/iio/industrialio-counter.c new file +9 -0

+             counter->num_channels;
+     int err;
+     struct iio_chan_spec *channels;
+     size_t j;
+     struct iio_counter_value *value;
+     size_t i =3D counter->num_channels;
+     const struct iio_counter_signal *signal;
+
+     channels =3D kcalloc(num_channels, sizeof(*channels), GFP_KERNEL);

Bring the calculation of num_channels down here - it will make it more obvious that this allows space for both sources of channel.

Quoted 14 lines

drivers/iio/industrialio-counter.c new file +14 -0

+     if (!channels)
+             return -ENOMEM;
+
+     /* If any channels were supplied on Counter registration,
+      * we add them here to the front of the array
+      */
+     memcpy(channels, counter->channels,
+             counter->num_channels * sizeof(*counter->channels));
+
+     /* Add channel for each Value */
+     for (j =3D 0; j < counter->num_values; j++) {
+             value =3D counter->values + j;
+
+             channels[i].type =3D IIO_COUNT;

drivers/iio/industrialio-counter.c line 435

Quoted 7 lines

drivers/iio/industrialio-counter.c new file +7 -0

+             channels[i].channel =3D counter->id;
+             channels[i].channel2 =3D value->id;
+             channels[i].info_mask_separate =3D BIT(IIO_CHAN_INFO_RAW);
+             channels[i].indexed =3D 1;
+             channels[i].counter =3D 1;
+
+             /* Add channels for Triggers associated with Value */

Add channels? 'i' would need incrementing. Also that's not what this f=

is doing that I can see...

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+             err =3D iio_counter_value_ext_info_alloc(channels + i, val=

Quoted 26 lines

drivers/iio/industrialio-counter.c new file +26 -0

+             if (err)
+                     goto err_value_ext_info_alloc;
+
+             i++;
+     }
+
+     /* Add channel for each Signal */
+     for (j =3D 0; j < counter->num_signals; j++) {
+             signal =3D counter->signals + j;
+
+             channels[i].type =3D IIO_SIGNAL;
+             channels[i].channel =3D counter->id;
+             channels[i].channel2 =3D signal->id;
+             channels[i].info_mask_separate =3D BIT(IIO_CHAN_INFO_RAW);
+             channels[i].indexed =3D 1;
+             channels[i].counter =3D 1;
+             channels[i].ext_info =3D iio_counter_signal_ext_info;
+
+             i++;
+     }
+
+     counter->indio_dev->num_channels =3D num_channels;
+     counter->indio_dev->channels =3D channels;
+
+     return 0;
+

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+err_value_ext_info_alloc:
+     while (i-- > counter->num_channels)
+             iio_counter_value_ext_info_free(channels + i);
+     kfree(channels);
+     return err;
+}
+
+static void iio_counter_channels_free(const struct iio_counter *const c=

drivers/iio/industrialio-counter.c line 472

Quoted 2 lines

drivers/iio/industrialio-counter.c new file +2 -0

+{
+     size_t i =3D counter->num_channels + counter->indio_dev->num_chann=

drivers/iio/industrialio-counter.c line 473

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+     const struct iio_chan_spec *const chans =3D counter->indio_dev->ch=

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+
+     while (i-- > counter->num_channels)
+             /* Only IIO_COUNT channels need to be freed here */
+             if (chans[i].type =3D=3D IIO_COUNT)
+                     iio_counter_value_ext_info_free(chans + i);

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+     kfree(chans);
+}
+
+static int iio_counter_read_raw(struct iio_dev *indio_dev,
+     struct iio_chan_spec const *chan, int *val, int *val2, long mask)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+     struct iio_counter_signal *signal;
+     struct iio_counter_value *value;
+
+     if (mask !=3D IIO_CHAN_INFO_RAW)
+             return -EINVAL;

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+     switch (chan->type) {
+     /* Map read_raw to signal_read for Signal */
+     case IIO_SIGNAL:
+             if (!counter->ops->signal_read)
+                     return -EINVAL;
+
+             signal =3D iio_counter_signal_find_by_id(counter, chan->ch=

drivers/iio/industrialio-counter.c

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+             if (!signal)
+                     return -EINVAL;
+
+             return counter->ops->signal_read(counter, signal, val, val=

drivers/iio/industrialio-counter.c

Quoted 6 lines

drivers/iio/industrialio-counter.c new file +6 -0

+     /* Map read_raw to value_read for Value */
+     case IIO_COUNT:
+             if (!counter->ops->value_read)
+                     return -EINVAL;
+
+             value =3D iio_counter_value_find_by_id(counter, chan->chan=

drivers/iio/industrialio-counter.c

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+             if (!value)
+                     return -EINVAL;
+
+             return counter->ops->value_read(counter, value, val, val2)=

drivers/iio/industrialio-counter.c

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+     /* Map read_raw to read_raw for non-counter channel */
+     default:
+             if (counter->info && counter->info->read_raw)
+                     return counter->info->read_raw(indio_dev, chan, va=

drivers/iio/industrialio-counter.c

Quoted 10 lines

drivers/iio/industrialio-counter.c new file +10 -0

+                             val2, mask);
+     }
+
+     return -EINVAL;
+}
+
+static int iio_counter_write_raw(struct iio_dev *indio_dev,
+     struct iio_chan_spec const *chan, int val, int val2, long mask)
+{
+     struct iio_counter *const counter =3D iio_priv_get_counter(indio_d=

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+     struct iio_counter_signal *signal;
+     struct iio_counter_value *value;
+
+     if (mask !=3D IIO_CHAN_INFO_RAW)
+             return -EINVAL;

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+     switch (chan->type) {
+     /* Map write_raw to signal_write for Signal */
+     case IIO_SIGNAL:
+             if (!counter->ops->signal_write)
+                     return -EINVAL;
+
+             signal =3D iio_counter_signal_find_by_id(counter, chan->ch=

drivers/iio/industrialio-counter.c line 542

Quoted 2 lines

drivers/iio/industrialio-counter.c new file +2 -0

+             if (!signal)
+                     return -EINVAL;

Hmm. have to search is certainly a little ugly. Particularly as =

would be able to maintain this as a lookup. I'd be tempted to pa=

signal id down into the callback rather than finding the signal i=

Might turn out messier though... :)

Quoted 2 lines

drivers/iio/industrialio-counter.c new file +2 -0

+
+             return counter->ops->signal_write(counter, signal, val, va=

drivers/iio/industrialio-counter.c

Quoted 6 lines

drivers/iio/industrialio-counter.c new file +6 -0

+     /* Map write_raw to value_write for Value */
+     case IIO_COUNT:
+             if (!counter->ops->value_write)
+                     return -EINVAL;
+
+             value =3D iio_counter_value_find_by_id(counter, chan->chan=

Again, this mapping is a simple lookup in the driver I think, perhaps pus=

it down there?

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+             if (!value)
+                     return -EINVAL;
+
+             return counter->ops->value_write(counter, value, val, val2=

drivers/iio/industrialio-counter.c

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+     /* Map write_raw to write_raw for non-counter channel */
+     default:
+             if (counter->info && counter->info->write_raw)
+                     return counter->info->write_raw(indio_dev, chan, v=

drivers/iio/industrialio-counter.c

Quoted 7 lines

drivers/iio/industrialio-counter.c new file +7 -0

+                             val2, mask);
+     }
+
+     return -EINVAL;
+}
+
+static int iio_counter_signals_validate(const struct iio_counter *const=

Quoted 11 lines

drivers/iio/industrialio-counter.c new file +11 -0

+{
+     size_t i;
+     const struct iio_counter_signal *signal;
+     size_t j;
+     int curr_id;
+
+     /* At least one Signal must be defined */
+     if (!counter->num_signals || !counter->signals) {
+             pr_err("Counter '%d' Signals undefined\n", counter->id);
+             return -EINVAL;
+     }

drivers/iio/industrialio-counter.c line 584

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+     for (i =3D 0; i < counter->num_signals; i++) {
+             signal =3D counter->signals + i;
+             /* No two Signals may share the same ID */
+             for (j =3D 0; j < i; j++) {
+                     curr_id =3D counter->signals[j].id;
+                     if (curr_id =3D=3D signal->id) {
+                             pr_err("Duplicate Counter '%d' Signal '%d'=

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+                                             counter->id, signal->id);
+                             return -EEXIST;
+                     }
+             }
+             for (j =3D i + 1; j < counter->num_signals; j++) {
+                     curr_id =3D counter->signals[j].id;
+                     if (curr_id =3D=3D signal->id) {
+                             pr_err("Duplicate Counter '%d' Signal '%d'=

drivers/iio/industrialio-counter.c line 588

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+                                             counter->id, signal->id);
+                             return -EEXIST;
+                     }
+             }

Same as below.

Quoted 13 lines

drivers/iio/industrialio-counter.c new file +13 -0

+     }
+
+     return 0;
+}
+
+static int iio_counter_triggers_validate(const int counter_id,
+     const struct iio_counter_value *const value)
+{
+     size_t i;
+     const struct iio_counter_trigger *trigger;
+     size_t j;
+     int curr_id;
+

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     /* At least one Trigger must be defined */
+     if (!value->num_triggers || !value->triggers) {
+             pr_err("Counter '%d' Value '%d' Triggers undefined\n",
+                     counter_id, value->id);
+             return -EINVAL;
+     }
+
+     /* Ensure all Triggers have a defined Signal; this prevents a poss=

drivers/iio/industrialio-counter.c line 618

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+      * NULL pointer dereference when later validating Trigger Signal I=

Fix comment syntax throughout...

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+      */
+     for (i =3D 0; i < value->num_triggers; i++)
+             if (!value->triggers[i].signal) {
+                     pr_err("Counter '%d' Trigger '%zu' Signal undefine=

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+                             counter_id, i);
+                     return -EINVAL;
+             }
+

drivers/iio/industrialio-counter.c line 634

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     /* Verify validity of each Trigger */
+     for (i =3D 0; i < value->num_triggers; i++) {
+             trigger =3D value->triggers + i;
+             /* No two Trigger Signals may share the same ID */
+             for (j =3D 0; j < i; j++) {
+                     curr_id =3D value->triggers[j].signal->id;
+                     if (curr_id =3D=3D trigger->signal->id) {
+                             pr_err("Signal '%d' is already linked to C=

drivers/iio/industrialio-counter.c

Quoted 9 lines

drivers/iio/industrialio-counter.c new file +9 -0

+                                     trigger->signal->id, counter_id,
+                                     value->id);
+                             return -EEXIST;
+                     }
+             }
+             for (j =3D i + 1; j < value->num_triggers; j++) {
+                     curr_id =3D value->triggers[j].signal->id;
+                     if (curr_id =3D=3D trigger->signal->id) {
+                             pr_err("Signal '%d' is already linked to C=

drivers/iio/industrialio-counter.c line 639

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+                                     trigger->signal->id, counter_id,
+                                     value->id);
+                             return -EEXIST;
+                     }
+             }

Again, one loop with the condition changed so it doesn't fault on the i =

note I'm reviewing backwards...

Quoted 2 lines

drivers/iio/industrialio-counter.c new file +2 -0

+
+             /* At least one trigger mode must be defined for each Trig=

drivers/iio/industrialio-counter.c line 651

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+             if (!trigger->num_trigger_modes || !trigger->trigger_modes=

drivers/iio/industrialio-counter.c line 652

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+                     pr_err("Counter '%d' Signal '%d' trigger modes und=

drivers/iio/industrialio-counter.c

Quoted 9 lines

drivers/iio/industrialio-counter.c new file +9 -0

+                             counter_id, trigger->signal->id);
+                     return -EINVAL;
+             }
+     }
+
+     return 0;
+}
+
+static int iio_counter_values_validate(const struct iio_counter *const =

drivers/iio/industrialio-counter.c line 662

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+{

Hmm. Pretty heavy weight checking. Ah well, up to you.

Quoted 12 lines

drivers/iio/industrialio-counter.c new file +12 -0

+     size_t i;
+     const struct iio_counter_value *value;
+     size_t j;
+     int curr_id;
+     int err;
+
+     /* At least one Value must be defined */
+     if (!counter->num_values || !counter->values) {
+             pr_err("Counter '%d' Values undefined\n", counter->id);
+             return -EINVAL;
+     }
+

drivers/iio/industrialio-counter.c line 679

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+     /* Verify validity of each Value */
+     for (i =3D 0; i < counter->num_values; i++) {
+             value =3D counter->values + i;
+             /* No two Values may share the same ID */
+             for (j =3D 0; j < i; j++) {

Single loop with a slightly change to condition
if ((i !=3D j) and (curr_id =3D=3D value->id))

Quoted 3 lines

drivers/iio/industrialio-counter.c new file +3 -0

+                     curr_id =3D counter->values[j].id;
+                     if (curr_id =3D=3D value->id) {
+                             pr_err("Duplicate Counter '%d' Value '%d'\=

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+                                     counter->id, value->id);
+                             return -EEXIST;
+                     }
+             }
+             for (j =3D i + 1; j < counter->num_values; j++) {
+                     curr_id =3D counter->values[j].id;
+                     if (curr_id =3D=3D value->id) {
+                             pr_err("Duplicate Counter '%d' Value '%d'\=

drivers/iio/industrialio-counter.c

Quoted 6 lines

drivers/iio/industrialio-counter.c new file +6 -0

+                                     counter->id, value->id);
+                             return -EEXIST;
+                     }
+             }
+
+             /* At least one function mode must be defined for each Val=

drivers/iio/industrialio-counter.c line 697

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+             if (!value->num_function_modes || !value->function_modes) =

drivers/iio/industrialio-counter.c line 698

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+                     pr_err("Counter '%d' Value '%d' function modes und=

Quoted 10 lines

drivers/iio/industrialio-counter.c new file +10 -0

+                             counter->id, value->id);
+                     return -EINVAL;
+             }
+
+             /* Verify the Triggers associated with each Value */
+             err =3D iio_counter_triggers_validate(counter->id, value);
+             if (err)
+                     return err;
+     }
+

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     return 0;
+}
+
+/**
+ * iio_counter_register - register Counter to the system
+ * @counter: pointer to IIO Counter to register
+ *
+ * This function piggybacks off of iio_device_register. First, the rele=

drivers/iio/industrialio-counter.c line 717

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+ * Counter members are validated; the signals and values members must b=

drivers/iio/industrialio-counter.c line 718

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+ * and populated by valid Signal and Value structures respectively. Nex=

drivers/iio/industrialio-counter.c line 719

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+ * struct iio_dev is allocated by a call to iio_device_alloc and initia=

drivers/iio/industrialio-counter.c line 720

Quoted 1 line

drivers/iio/industrialio-counter.c new file +1 -0

+ * the Counter, IIO channels are allocated, the Counter address is stor=

Quoted 17 lines

drivers/iio/industrialio-counter.c new file +17 -0

+ * private data, and finally iio_device_register is called.
+ */
+int iio_counter_register(struct iio_counter *const counter)
+{
+     const struct iio_info info_default =3D {
+             .read_raw =3D iio_counter_read_raw,
+             .write_raw =3D iio_counter_write_raw
+     };
+     int err;
+     struct iio_info *info;
+     struct iio_counter **priv;
+
+     if (!counter)
+             return -EINVAL;
+
+     /* Verify that Signals are valid and IDs do not conflict */
+     err =3D iio_counter_signals_validate(counter);

drivers/iio/industrialio-counter.c line 741

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+     if (err)
+             return err;
+
+     /* Verify that Values are valid and IDs do not conflict;

Fix multiline comment syntax to standard kernel syntax. /*
* Verify
*/

Quoted 34 lines

drivers/iio/industrialio-counter.c new file +33 -0

+      * Triggers for each Value are also verified for validity
+      */
+     err =3D iio_counter_values_validate(counter);
+     if (err)
+             return err;
+
+     counter->indio_dev =3D iio_device_alloc(sizeof(counter));
+     if (!counter->indio_dev)
+             return -ENOMEM;
+
+     info =3D kmalloc(sizeof(*info), GFP_KERNEL);
+     if (!info) {
+             err =3D -ENOMEM;
+             goto err_info_alloc;
+     }
+     /* If an iio_info has been supplied than we use that,
+      * otherwise we set all callbacks to NULL; iio_counter_read_raw
+      * and iio_counter_write_raw is used for read_raw and write_raw
+      * for either case in order to support counter functionality
+      * (supplied read_raw/write_raw will be called from within
+      * iio_counter_read_raw/iio_counter_write_raw for non-counter
+      * channels)
+      */
 
+     if (counter->info) {
+             memcpy(info, counter->info, sizeof(*counter->info));
+             info->read_raw =3D iio_counter_read_raw;
+             info->write_raw =3D iio_counter_write_raw;
+     } else {
+             memcpy(info, &info_default, sizeof(info_default));
+     }
+
+     counter->indio_dev->info =3D info;
+     counter->indio_dev->modes =3D INDIO_DIRECT_MODE;

drivers/iio/industrialio-counter.c

Quoted 4 lines

drivers/iio/industrialio-counter.c new file +4 -0

+     counter->indio_dev->name =3D counter->name;
+     counter->indio_dev->dev.parent =3D counter->dev;
+
+     /* IIO channels are allocated and set for Signals, Values, and Tri=

Quoted 25 lines

drivers/iio/industrialio-counter.c new file +25 -0

+      * any auxiliary IIO channels provided in iio_counter are also set
+      */
+     err =3D iio_counter_channels_alloc(counter);
+     if (err)
+             goto err_channels_alloc;
+
+     /* Pointer to the counter is stored in indio_dev as a way to refer
+      * back to the counter from within various IIO callbacks
+      */
+     priv =3D iio_priv(counter->indio_dev);
+     memcpy(priv, &counter, sizeof(*priv));
+
+     err =3D iio_device_register(counter->indio_dev);
+     if (err)
+             goto err_iio_device_register;
+
+     return 0;
+
+err_iio_device_register:
+     iio_counter_channels_free(counter);
+err_channels_alloc:
+     kfree(info);
+err_info_alloc:
+     iio_device_free(counter->indio_dev);
+     return err;

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+}
+EXPORT_SYMBOL(iio_counter_register);
+
+/**
+ * iio_counter_unregister - unregister Counter from the system
+ * @counter: pointer to IIO Counter to unregister
+ *
+ * The Counter is unregistered from the system. The indio_dev is unregi=

Quoted 22 lines

+ * and all allocated memory is freed.
+ */

drivers/iio/industrialio-counter.c new file +20 -0

+void iio_counter_unregister(struct iio_counter *const counter)
+{
+     const struct iio_info *const info =3D counter->indio_dev->info;
+
+     if (!counter)
+             return;
+
+     iio_device_unregister(counter->indio_dev);
+
+     iio_counter_channels_free(counter);
+
+     kfree(info);
+     iio_device_free(counter->indio_dev);
+}
+EXPORT_SYMBOL(iio_counter_unregister);
+
+static void devm_iio_counter_unreg(struct device *dev, void *res)
+{
+     iio_counter_unregister(*(struct iio_counter **)res);
+}

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+
+/**
+ * devm_iio_counter_register - Resource-managed iio_counter_register
+ * @dev: Device to allocate iio_counter for
+ * @counter: pointer to IIO Counter to register
+ *
+ * Managed iio_counter_register. The IIO counter registered with this
+ * function is automatically unregistered on driver detach. This functi=

Quoted 1 line

+ * calls iio_counter_register internally. Refer to that function for mo=

Quoted 3 lines

+ * information.
+ *
+ * If an iio counter registered with this function needs to be unregist=

Quoted 4 lines

+ * separately, devm_iio_counter_unregister must be used.
+ *
+ * RETURNS:
+ * 0 on success, negative error number on failure.

drivers/iio/industrialio-counter.c

Quoted 8 lines

+ */

drivers/iio/industrialio-counter.c new file +7 -0

+int devm_iio_counter_register(struct device *dev,
+     struct iio_counter *const counter)
+{
+     struct iio_counter **ptr;
+     int ret;
+
+     ptr =3D devres_alloc(devm_iio_counter_unreg, sizeof(*ptr), GFP_KER=

Quoted 7 lines

drivers/iio/industrialio-counter.c new file +7 -0

+     if (!ptr)
+             return -ENOMEM;
+
+     *ptr =3D counter;
+     ret =3D iio_counter_register(counter);
+     if (!ret)
+             devres_add(dev, ptr);

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+     else
+             devres_free(ptr);
+
+     return ret;
+}
+EXPORT_SYMBOL(devm_iio_counter_register);
+
+static int devm_iio_counter_match(struct device *dev, void *res, void *=

Quoted 5 lines

drivers/iio/industrialio-counter.c new file +5 -0

+{
+     struct iio_counter **r =3D res;
+
+     if (!r || !*r) {
+             WARN_ON(!r || !*r);

drivers/iio/industrialio-counter.c

Quoted 8 lines

drivers/iio/industrialio-counter.c new file +8 -0

+             return 0;
+     }
+
+     return *r =3D=3D data;
+}
+
+/**
+ * devm_iio_counter_unregister - Resource-managed iio_counter_unregiste=

Quoted 51 lines

+ * @dev: Device this iio_counter belongs to
+ * @counter: the iio counter associated with the device
+ *
+ * Unregister iio counter registered with devm_iio_counter_register.
+ */

drivers/iio/industrialio-counter.c new file +10 -0

+void devm_iio_counter_unregister(struct device *dev,
+     struct iio_counter *const counter)
+{
+     int rc;
+
+     rc =3D devres_release(dev, devm_iio_counter_unreg,
+             devm_iio_counter_match, counter);
+     WARN_ON(rc);
+}
+EXPORT_SYMBOL(devm_iio_counter_unregister);

include/linux/iio/counter.h new file +30 -0

new file mode 100644
index 000000000000..35645406711a
@@ -0,0 +1,166 @@
+/*
+ * Industrial I/O counter interface
+ * Copyright (C) 2017 William Breathitt Gray
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+#ifndef _IIO_COUNTER_H_
+#define _IIO_COUNTER_H_
+
+#ifdef CONFIG_IIO_COUNTER
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+#include <linux/iio/iio.h>
+
+/**
+ * struct iio_counter_signal - IIO Counter Signal node
+ * @id:              [DRIVER] unique ID used to identify signal
+ * @name:    [DRIVER] device-specific signal name
+ */
+struct iio_counter_signal {
+     int             id;

include/linux/iio/counter.h

Quoted 8 lines

include/linux/iio/counter.h new file +8 -0

+     const char      *name;
+};
+
+/**
+ * struct iio_counter_trigger - IIO Counter Trigger node
+ * @mode:            [DRIVER] current trigger mode state
+ * @trigger_modes:   [DRIVER] available trigger modes
+ * @num_trigger_modes:       [DRIVER] number of modes specified in @tri=

include/linux/iio/counter.h line 45

Quoted 7 lines

include/linux/iio/counter.h new file +7 -0

+ * @signal:          [DRIVER] pointer to associated signal
+ */
+struct iio_counter_trigger {
+     unsigned int                    mode;
+     const char *const               *trigger_modes;
+     unsigned int                    num_trigger_modes;
+     struct iio_counter_signal       *signal;

Are signals actually changeable? - Or is that pointer constant?

include/linux/iio/counter.h

Quoted 9 lines

include/linux/iio/counter.h new file +9 -0

+};
+
+/**
+ * struct iio_counter_value - IIO Counter Value node
+ * @id:                      [DRIVER] unique ID used to identify value
+ * @name:            [DRIVER] device-specific value name
+ * @mode:            [DRIVER] current function mode state
+ * @function_modes:  [DRIVER] available function modes
+ * @num_function_modes:      [DRIVER] number of modes specified in @fun=

include/linux/iio/counter.h line 55

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ * @triggers:                [DRIVER] array of triggers for initializat=

include/linux/iio/counter.h line 57

Quoted 2 lines

include/linux/iio/counter.h new file +2 -0

+ * @num_triggers:    [DRIVER] number of triggers specified in @triggers
+ * @function_enum:   [INTERN] used internally to generate function attr=

Quoted 11 lines

include/linux/iio/counter.h new file +11 -0

+ */
+struct iio_counter_value {
+     int                     id;
+     const char              *name;
+     unsigned int            mode;
+     const char *const       *function_modes;
+     unsigned int            num_function_modes;
+
+     struct iio_counter_trigger      *triggers;
+     size_t                          num_triggers;
+

include/linux/iio/counter.h

Quoted 8 lines

include/linux/iio/counter.h new file +8 -0

+     struct iio_enum         function_enum;
+};
+
+struct iio_counter;
+
+/**
+ * struct iio_counter_ops - IIO Counter related callbacks
+ * @signal_read:     function to request a signal value from the device=

include/linux/iio/counter.h line 77

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ *                   Return value will specify the type of value return=

include/linux/iio/counter.h line 79

Quoted 2 lines

include/linux/iio/counter.h new file +2 -0

+ *                   the device. val and val2 will contain the elements
+ *                   making up the returned value.

Add some detail on the importance of the return value.

Quoted 2 lines

include/linux/iio/counter.h new file +2 -0

+ * @signal_write:    function to write a signal value to the device.
+ *                      Parameters are interpreted the same as signal_r=

You need a means of establishing the correct format for write (we needed =

in IIO fairly early on too)

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ * @trigger_mode_set:        function to set the trigger mode. mode is =

include/linux/iio/counter.h line 83

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ *                   the requested mode from the value trigger_modes ar=

include/linux/iio/counter.h line 84

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ * @trigger_mode_get:        function to get the current trigger mode. =

include/linux/iio/counter.h line 85

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ *                   will specify the index of the current mode from th=

include/linux/iio/counter.h line 87

Quoted 2 lines

include/linux/iio/counter.h new file +2 -0

+ *                   value trigger_modes array.
+ * @value_read:              function to request a value value from the=

include/linux/iio/counter.h line 88

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ *                   Return value will specify the type of value return=

include/linux/iio/counter.h

Quoted 4 lines

include/linux/iio/counter.h new file +4 -0

+ *                   the device. val and val2 will contain the elements
+ *                   making up the returned value.
+ * @value_write:     function to write a value value to the device.
+ *                      Parameters are interpreted the same as value_re=

include/linux/iio/counter.h line 93

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ * @value_function_set: function to set the value function mode. mode i=

include/linux/iio/counter.h

Quoted 3 lines

include/linux/iio/counter.h new file +3 -0

+ *                   index of the requested mode from the value
+ *                   function_modes array.
+ * @value_function_get: function to get the current value function mode=

include/linux/iio/counter.h line 97

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ *                   value will specify the index of the current mode f=

Quoted 18 lines

include/linux/iio/counter.h new file +18 -0

+ *                   the value function_modes array.
+ */
+struct iio_counter_ops {
+     int (*signal_read)(struct iio_counter *counter,
+             struct iio_counter_signal *signal, int *val, int *val2);
+     int (*signal_write)(struct iio_counter *counter,
+             struct iio_counter_signal *signal, int val, int val2);
+     int (*trigger_mode_set)(struct iio_counter *counter,
+             struct iio_counter_value *value,
+             struct iio_counter_trigger *trigger, unsigned int mode);
+     int (*trigger_mode_get)(struct iio_counter *counter,
+             struct iio_counter_value *value,
+             struct iio_counter_trigger *trigger);
+     int (*value_read)(struct iio_counter *counter,
+             struct iio_counter_value *value, int *val, int *val2);
+     int (*value_write)(struct iio_counter *counter,
+             struct iio_counter_value *value, int val, int val2);
+     int (*value_function_set)(struct iio_counter *counter,

include/linux/iio/counter.h

Quoted 8 lines

include/linux/iio/counter.h new file +8 -0

+             struct iio_counter_value *value, unsigned int mode);
+     int (*value_function_get)(struct iio_counter *counter,
+             struct iio_counter_value *value);
+};
+
+/**
+ * struct iio_counter - IIO Counter data structure
+ * @id:                      [DRIVER] unique ID used to identify counte=

include/linux/iio/counter.h line 125

Quoted 2 lines

include/linux/iio/counter.h new file +2 -0

+ * @name:            [DRIVER] name of the device
+ * @dev:             [DRIVER] device structure, should be assigned a pa=

include/linux/iio/counter.h line 127

Quoted 2 lines

include/linux/iio/counter.h new file +2 -0

+ *                   and owner
+ * @ops:             [DRIVER] callbacks from driver for counter compone=

include/linux/iio/counter.h

Quoted 4 lines

include/linux/iio/counter.h new file +4 -0

+ * @signals:         [DRIVER] array of signals for initialization
+ * @num_signals:     [DRIVER] number of signals specified in @signals
+ * @values:          [DRIVER] array of values for initialization
+ * @num_values:              [DRIVER] number of values specified in @va=

include/linux/iio/counter.h line 132

Quoted 1 line

include/linux/iio/counter.h new file +1 -0

+ * @channels:                [DRIVER] channel specification structure t=