Re: [PATCH 2/3] iio: Introduce the generic counter interface

[Jonathan Cameron]

On Mon, 31 Jul 2017 12:03:23 -0400
William Breathitt Gray <vilhelm.gray@xxxxxxxxx> wrote:

> 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 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 "counter signals" and "counter values," and set and
> get the "trigger mode" and "function mode" for various "counter signals"
> and "counter values" respectively.
> 
> To support a counter device, a driver must first allocate the available
> "counter signals" via iio_counter_signal structures. These "counter
> signals" should be stored as an array and set to the init_signals member
> of an allocated iio_counter structure before the counter is registered.
> 
> "Counter values" may be allocated via iio_counter_value structures, and
> respective "counter signal" associations made via iio_counter_trigger
> structures. Initial associated iio_counter_trigger structures may be
> stored as an array and set to the the init_triggers member of the
> respective iio_counter_value structure. These iio_counter_value
> structures may be set to the init_values member of an allocated
> iio_counter structure before the counter is registered if so desired.
> 
> 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.
> 
> Signed-off-by: William Breathitt Gray <vilhelm.gray@xxxxxxxxx>

Hi William,

A few minor points inline as a starting point.  I'm going to want to dig
into this in a lot more detail but don't have the time today (or possibly
for a few more weeks - sorry about that!)

Thanks,

Jonathan
> ---
>  MAINTAINERS                        |    7 +
>  drivers/iio/Kconfig                |    8 +
>  drivers/iio/Makefile               |    1 +
>  drivers/iio/counter/Kconfig        |    1 +
>  drivers/iio/industrialio-counter.c | 1157 ++++++++++++++++++++++++++++++++++++
>  include/linux/iio/counter.h        |  221 +++++++
>  6 files changed, 1395 insertions(+)
>  create mode 100644 drivers/iio/industrialio-counter.c
>  create mode 100644 include/linux/iio/counter.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f6ef3f3e5000..44345ef25066 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6480,6 +6480,13 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
>  F:	Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
>  F:	drivers/iio/adc/envelope-detector.c
>  
> +IIO GENERIC COUNTER INTERFACE
> +M:	William Breathitt Gray <vilhelm.gray@xxxxxxxxx>
> +L:	linux-iio@xxxxxxxxxxxxxxx
> +S:	Maintained
> +F:	drivers/iio/industrialio-counter.c
> +F:	include/linux/iio/counter.h
> +
>  IIO SUBSYSTEM AND DRIVERS
>  M:	Jonathan Cameron <jic23@xxxxxxxxxx>
>  R:	Hartmut Knaack <knaack.h@xxxxxx>
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index a918270d6f54..5ab8ed9ab7fc 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -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
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 33fa4026f92c..ca79f3860d1c 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -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
> diff --git a/drivers/iio/counter/Kconfig b/drivers/iio/counter/Kconfig
> index b37e5fc03149..3d46a790d8db 100644
> --- a/drivers/iio/counter/Kconfig
> +++ b/drivers/iio/counter/Kconfig
> @@ -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"
> diff --git a/drivers/iio/industrialio-counter.c b/drivers/iio/industrialio-counter.c
> new file mode 100644
> index 000000000000..27c20919af62
> --- /dev/null
> +++ b/drivers/iio/industrialio-counter.c
> @@ -0,0 +1,1157 @@
> +/*
> + * 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/err.h>
> +#include <linux/export.h>
> +#include <linux/gfp.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/mutex.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>
> +
> +static struct iio_counter_signal *__iio_counter_signal_find_by_id(
> +	const struct iio_counter *const counter, const int id)
> +{
> +	struct iio_counter_signal *iter;
> +
> +	list_for_each_entry(iter, &counter->signal_list, list)
> +		if (iter->id == id)
> +			return iter;
> +
> +	return NULL;
> +}
> +
> +static struct iio_counter_trigger *__iio_counter_trigger_find_by_id(
> +	const struct iio_counter_value *const value, const int id)
> +{
> +	struct iio_counter_trigger *iter;
> +
> +	list_for_each_entry(iter, &value->trigger_list, list)
> +		if (iter->signal->id == id)
> +			return iter;
> +
> +	return NULL;
> +}
> +
> +static struct iio_counter_value *__iio_counter_value_find_by_id(
> +	const struct iio_counter *const counter, const int id)
> +{
> +	struct iio_counter_value *iter;
> +
> +	list_for_each_entry(iter, &counter->value_list, list)
> +		if (iter->id == id)
> +			return iter;
> +
> +	return NULL;
> +}
> +
> +static void __iio_counter_trigger_unregister_all(
> +	struct iio_counter_value *const value)
> +{
> +	struct iio_counter_trigger *iter, *tmp_iter;
> +
> +	mutex_lock(&value->trigger_list_lock);
> +	list_for_each_entry_safe(iter, tmp_iter, &value->trigger_list, list)
> +		list_del(&iter->list);
> +	mutex_unlock(&value->trigger_list_lock);
> +}
> +
> +static void __iio_counter_signal_unregister_all(
> +	struct iio_counter *const counter)
> +{
> +	struct iio_counter_signal *iter, *tmp_iter;
> +
> +	mutex_lock(&counter->signal_list_lock);
> +	list_for_each_entry_safe(iter, tmp_iter, &counter->signal_list, list)
> +		list_del(&iter->list);
> +	mutex_unlock(&counter->signal_list_lock);
> +}
> +
> +static void __iio_counter_value_unregister_all(
> +	struct iio_counter *const counter)
> +{
> +	struct iio_counter_value *iter, *tmp_iter;
> +
> +	mutex_lock(&counter->value_list_lock);
> +	list_for_each_entry_safe(iter, tmp_iter, &counter->value_list, list) {
> +		__iio_counter_trigger_unregister_all(iter);
> +
> +		list_del(&iter->list);
> +	}
> +	mutex_unlock(&counter->value_list_lock);
> +}
> +
> +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(indio_dev);
> +	const struct iio_counter_signal *signal;
> +
> +	mutex_lock(&counter->signal_list_lock);
> +	signal = __iio_counter_signal_find_by_id(counter, chan->channel2);
> +	mutex_unlock(&counter->signal_list_lock);
> +	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,
> +	uintptr_t priv, const struct iio_chan_spec *chan, char *buf)
> +{
> +	struct iio_counter *const counter = iio_priv(indio_dev);
> +	const struct iio_counter_value *value;
> +
> +	mutex_lock(&counter->value_list_lock);
> +	value = __iio_counter_value_find_by_id(counter, chan->channel2);
> +	mutex_unlock(&counter->value_list_lock);
> +	if (!value)
> +		return -EINVAL;
> +
> +	return scnprintf(buf, PAGE_SIZE, "%s\n", value->name);
> +}
> +
> +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(indio_dev);
> +	struct iio_counter_value *value;
> +	const struct iio_counter_trigger *trigger;
> +	ssize_t len = 0;
> +
> +	mutex_lock(&counter->value_list_lock);
> +	value = __iio_counter_value_find_by_id(counter, chan->channel2);
> +	if (!value) {
> +		len = -EINVAL;
> +		goto err_find_value;
> +	}
> +
> +	mutex_lock(&value->trigger_list_lock);
> +	list_for_each_entry(trigger, &value->trigger_list, list) {
> +		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) {
> +			len = -ENOMEM;
> +			goto err_no_buffer_space;
> +		}
> +	}
> +err_no_buffer_space:
> +	mutex_unlock(&value->trigger_list_lock);
> +
> +err_find_value:
> +	mutex_unlock(&counter->value_list_lock);
> +
> +	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(indio_dev);
> +	struct iio_counter_value *value;
> +	ssize_t ret;
> +	struct iio_counter_trigger *trigger;
> +	const int signal_id = *((int *)priv);
> +	int mode;
> +
> +	if (!counter->ops->trigger_mode_get)
> +		return -EINVAL;
> +
> +	mutex_lock(&counter->value_list_lock);
> +
> +	value = __iio_counter_value_find_by_id(counter, chan->channel2);
> +	if (!value) {
> +		ret = -EINVAL;
> +		goto err_value;
> +	}
> +
> +	mutex_lock(&value->trigger_list_lock);
> +
> +	trigger = __iio_counter_trigger_find_by_id(value, signal_id);
> +	if (!trigger) {
> +		ret = -EINVAL;
> +		goto err_trigger;
> +	}
> +
> +	mode = counter->ops->trigger_mode_get(counter, value, trigger);
> +
> +	if (mode < 0) {
> +		ret = mode;
> +		goto err_trigger;
> +	} else if (mode >= trigger->num_trigger_modes) {
> +		ret = -EINVAL;
> +		goto err_trigger;
> +	}
> +
> +	trigger->mode = mode;
> +
> +	ret = scnprintf(buf, PAGE_SIZE, "%s\n", trigger->trigger_modes[mode]);
> +
> +	mutex_unlock(&value->trigger_list_lock);
> +
> +	mutex_unlock(&counter->value_list_lock);
> +
> +	return ret;
> +
> +err_trigger:
> +	mutex_unlock(&value->trigger_list_lock);
> +err_value:
> +	mutex_unlock(&counter->value_list_lock);
> +	return ret;
> +}
> +
> +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(indio_dev);
> +	struct iio_counter_value *value;
> +	ssize_t err;
> +	struct iio_counter_trigger *trigger;
> +	const int signal_id = *(int *)((void *)priv);
Given you don't go through the void * cast in _read I'm thinking it's not
needed here either.

> +	unsigned int mode;
> +
> +	if (!counter->ops->trigger_mode_set)
> +		return -EINVAL;
> +
> +	mutex_lock(&counter->value_list_lock);
> +
> +	value = __iio_counter_value_find_by_id(counter, chan->channel2);
> +	if (!value) {
> +		err = -EINVAL;
> +		goto err_value;
> +	}
> +
> +	mutex_lock(&value->trigger_list_lock);
> +
> +	trigger = __iio_counter_trigger_find_by_id(value, signal_id);
> +	if (!trigger) {
> +		err = -EINVAL;
> +		goto err_trigger;
> +	}
> +
> +	for (mode = 0; mode < trigger->num_trigger_modes; mode++)
> +		if (sysfs_streq(buf, trigger->trigger_modes[mode]))
> +			break;
> +
> +	if (mode >= trigger->num_trigger_modes) {
> +		err = -EINVAL;
> +		goto err_trigger;
> +	}
> +
> +	err = counter->ops->trigger_mode_set(counter, value, trigger, mode);
> +	if (err)
> +		goto err_trigger;
> +
> +	trigger->mode = mode;
> +
> +	mutex_unlock(&value->trigger_list_lock);
> +
> +	mutex_unlock(&counter->value_list_lock);
> +
> +	return len;
> +
> +err_trigger:
> +	mutex_unlock(&value->trigger_list_lock);
> +err_value:
> +	mutex_unlock(&counter->value_list_lock);
> +	return err;
> +}
> +
> +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(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;
> +
> +	mutex_lock(&counter->value_list_lock);
> +
> +	value = __iio_counter_value_find_by_id(counter, chan->channel2);
> +	if (!value) {
> +		len = -EINVAL;
> +		goto err_no_value;
> +	}
> +
> +	mutex_lock(&value->trigger_list_lock);
> +
> +	trigger = __iio_counter_trigger_find_by_id(value, signal_id);
> +	if (!trigger) {
> +		len = -EINVAL;
> +		goto err_no_trigger;
> +	}
> +
> +	for (i = 0; i < trigger->num_trigger_modes; i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len, "%s ",
> +			trigger->trigger_modes[i]);
> +
> +	mutex_unlock(&value->trigger_list_lock);
> +
> +	mutex_unlock(&counter->value_list_lock);
> +
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +
> +err_no_trigger:
> +	mutex_unlock(&value->trigger_list_lock);
> +err_no_value:
> +	mutex_unlock(&counter->value_list_lock);
> +	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(indio_dev);
> +	struct iio_counter_value *value;
> +	int err;
> +
> +	if (!counter->ops->trigger_mode_get)
> +		return -EINVAL;
> +
> +	mutex_lock(&counter->value_list_lock);
> +
> +	value = __iio_counter_value_find_by_id(counter, chan->channel2);
> +	if (!value) {
> +		err = -EINVAL;
> +		goto err_value;
> +	}
> +
> +	err = counter->ops->value_function_set(counter, value, mode);
> +	if (err)
> +		goto err_value;
> +
> +	value->mode = mode;
> +
> +err_value:
> +	mutex_unlock(&counter->value_list_lock);
> +
> +	return err;
> +}
> +
> +static int __iio_counter_value_function_get(struct iio_dev *indio_dev,
> +	const struct iio_chan_spec *chan)
> +{
> +	struct iio_counter *const counter = iio_priv(indio_dev);
> +	struct iio_counter_value *value;
> +	int retval;
> +
> +	if (!counter->ops->trigger_mode_get)
> +		return -EINVAL;
> +
> +	mutex_lock(&counter->value_list_lock);
> +
> +	value = __iio_counter_value_find_by_id(counter, chan->channel2);
> +	if (!value) {
> +		retval = -EINVAL;
> +		goto err_value;
> +	}
> +
> +	retval = counter->ops->value_function_get(counter, value);
> +	if (retval < 0)
> +		goto err_value;
> +	else if (retval >= value->num_function_modes) {
> +		retval = -EINVAL;
> +		goto err_value;
> +	}
> +
> +	value->mode = retval;
> +
> +err_value:
> +	mutex_unlock(&counter->value_list_lock);
> +
> +	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 = (uintptr_t)((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 struct list_head *pos;
> +	size_t num_triggers = 0;
> +	size_t num_triggers_ext_info;
> +	size_t num_ext_info;
> +	int err;
> +	struct iio_chan_spec_ext_info *ext_info;
> +	const struct iio_counter_trigger *trigger_pos;
> +	size_t i;
> +
> +	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;
> +
> +	mutex_lock(&value->trigger_list_lock);
> +
> +	list_for_each(pos, &value->trigger_list)
> +		num_triggers++;
> +
> +	num_triggers_ext_info = num_ext_info_trigger * num_triggers;
> +	num_ext_info = num_default + num_triggers_ext_info + 1;
> +
> +	ext_info = kmalloc_array(num_ext_info, sizeof(*ext_info), GFP_KERNEL);
> +	if (!ext_info) {
> +		err = -ENOMEM;
> +		goto err_ext_info_alloc;
> +	}
> +	ext_info[num_ext_info - 1].name = NULL;
> +
> +	memcpy(ext_info, ext_info_default, sizeof(ext_info_default));
> +	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));
> +
> +	i = num_default;
> +	list_for_each_entry(trigger_pos, &value->trigger_list, list) {
> +		ext_info[i].name = kasprintf(GFP_KERNEL, "trigger_signal%d-%d",
> +			chan->channel, trigger_pos->signal->id);
> +		if (!ext_info[i].name) {
> +			err = -ENOMEM;
> +			goto err_name_alloc;
> +		}
> +		ext_info[i].private = (void *)&trigger_pos->signal->id;
> +		i++;
> +
> +		ext_info[i].name = kasprintf(GFP_KERNEL,
> +			"trigger_signal%d-%d_available",
> +			chan->channel, trigger_pos->signal->id);
> +		if (!ext_info[i].name) {
> +			err = -ENOMEM;
> +			goto err_name_alloc;
> +		}
> +		ext_info[i].private = (void *)&trigger_pos->signal->id;
> +		i++;
> +	}
> +
> +	chan->ext_info = ext_info;
> +
> +	mutex_unlock(&value->trigger_list_lock);
> +
> +	return 0;
> +
> +err_name_alloc:
> +	while (i-- > num_default)
> +		kfree(ext_info[i].name);
> +	kfree(ext_info);
> +err_ext_info_alloc:
> +	mutex_unlock(&value->trigger_list_lock);
> +	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)
> +{
> +	const struct list_head *pos;
> +	size_t num_channels = 0;
> +	int err;
> +	struct iio_chan_spec *channels;
> +	struct iio_counter_value *value_pos;
> +	size_t i = counter->num_channels;
> +	const struct iio_counter_signal *signal_pos;
> +
> +	mutex_lock(&counter->signal_list_lock);
> +
> +	list_for_each(pos, &counter->signal_list)
> +		num_channels++;
> +
> +	if (!num_channels) {
> +		err = -EINVAL;
> +		goto err_no_signals;
> +	}
> +
> +	mutex_lock(&counter->value_list_lock);
> +
> +	list_for_each(pos, &counter->value_list)
> +		num_channels++;
> +
> +	num_channels += counter->num_channels;
> +
> +	channels = kcalloc(num_channels, sizeof(*channels), GFP_KERNEL);
> +	if (!channels) {
> +		err = -ENOMEM;
> +		goto err_channels_alloc;
> +	}
> +
> +	memcpy(channels, counter->channels,
> +		counter->num_channels * sizeof(*counter->channels));
> +
> +	list_for_each_entry(value_pos, &counter->value_list, list) {
> +		channels[i].type = IIO_COUNT;
> +		channels[i].channel = counter->id;
> +		channels[i].channel2 = value_pos->id;
> +		channels[i].info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> +		channels[i].indexed = 1;
> +		channels[i].counter = 1;
> +
> +		err = __iio_counter_value_ext_info_alloc(channels + i,
> +			value_pos);
> +		if (err)
> +			goto err_value_ext_info_alloc;
> +
> +		i++;
> +	}
> +
> +	mutex_unlock(&counter->value_list_lock);
> +
> +	list_for_each_entry(signal_pos, &counter->signal_list, list) {
> +		channels[i].type = IIO_SIGNAL;
> +		channels[i].channel = counter->id;
> +		channels[i].channel2 = signal_pos->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++;
> +	}
> +
> +	mutex_unlock(&counter->signal_list_lock);
> +
> +	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);
> +err_channels_alloc:
> +	mutex_unlock(&counter->value_list_lock);
> +err_no_signals:
> +	mutex_unlock(&counter->signal_list_lock);
> +	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)
> +		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(indio_dev);
> +	struct iio_counter_signal *signal;
> +	int retval;
> +	struct iio_counter_value *value;
> +
> +	if (mask != IIO_CHAN_INFO_RAW)
> +		return -EINVAL;
> +
> +	switch (chan->type) {
> +	case IIO_SIGNAL:
> +		if (!counter->ops->signal_read)
> +			return -EINVAL;
> +
> +		mutex_lock(&counter->signal_list_lock);
> +		signal = __iio_counter_signal_find_by_id(counter,
> +			chan->channel2);
> +		if (!signal) {
> +			mutex_unlock(&counter->signal_list_lock);
> +			return -EINVAL;
> +		}
> +
> +		retval = counter->ops->signal_read(counter, signal, val, val2);
> +		mutex_unlock(&counter->signal_list_lock);
> +
> +		return retval;
> +	case IIO_COUNT:
> +		if (!counter->ops->value_read)
> +			return -EINVAL;
> +
> +		mutex_lock(&counter->value_list_lock);
> +		value = __iio_counter_value_find_by_id(counter, chan->channel2);
> +		if (!value) {
> +			mutex_unlock(&counter->value_list_lock);
> +			return -EINVAL;
> +		}
> +
> +		retval = counter->ops->value_read(counter, value, val, val2);
> +		mutex_unlock(&counter->value_list_lock);
> +
> +		return retval;
> +	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(indio_dev);
> +	struct iio_counter_signal *signal;
> +	int retval;
> +	struct iio_counter_value *value;
> +
> +	if (mask != IIO_CHAN_INFO_RAW)
> +		return -EINVAL;
> +
> +	switch (chan->type) {
> +	case IIO_SIGNAL:
> +		if (!counter->ops->signal_write)
> +			return -EINVAL;
> +
> +		mutex_lock(&counter->signal_list_lock);
> +		signal = __iio_counter_signal_find_by_id(counter,
> +			chan->channel2);
> +		if (!signal) {
> +			mutex_unlock(&counter->signal_list_lock);
> +			return -EINVAL;
> +		}
> +
> +		retval = counter->ops->signal_write(counter, signal, val, val2);
> +		mutex_unlock(&counter->signal_list_lock);
> +
> +		return retval;
> +	case IIO_COUNT:
> +		if (!counter->ops->value_write)
> +			return -EINVAL;
> +
> +		mutex_lock(&counter->value_list_lock);
> +		value = __iio_counter_value_find_by_id(counter, chan->channel2);
> +		if (!value) {
> +			mutex_unlock(&counter->value_list_lock);
> +			return -EINVAL;
> +		}
> +
> +		retval = counter->ops->value_write(counter, value, val, val2);
> +		mutex_unlock(&counter->value_list_lock);
> +
> +		return retval;
> +	default:

This case definitely needs a comment!
I think you overwrite any write_raw callbacks passed in and hence this
is a infinite recursion...
Could be wrong though ;)


> +		if (counter->info && counter->info->write_raw)
> +			return counter->info->write_raw(indio_dev, chan, val,
> +				val2, mask);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int __iio_counter_signal_register(struct iio_counter *const counter,
> +	struct iio_counter_signal *const signal)
> +{
> +	int err;
> +
> +	if (!counter || !signal)
> +		return -EINVAL;
> +
> +	mutex_lock(&counter->signal_list_lock);
> +	if (__iio_counter_signal_find_by_id(counter, signal->id)) {
> +		pr_err("Duplicate counter signal ID '%d'\n", signal->id);
> +		err = -EEXIST;
> +		goto err_duplicate_id;
> +	}
> +	list_add_tail(&signal->list, &counter->signal_list);
> +	mutex_unlock(&counter->signal_list_lock);
> +
> +	return 0;
> +
> +err_duplicate_id:
> +	mutex_unlock(&counter->signal_list_lock);
> +	return err;
> +}
> +
> +static void __iio_counter_signal_unregister(struct iio_counter *const counter,
> +	struct iio_counter_signal *const signal)
> +{
> +	if (!counter || !signal)
> +		return;
> +
> +	mutex_lock(&counter->signal_list_lock);
> +	list_del(&signal->list);
> +	mutex_unlock(&counter->signal_list_lock);
> +}
> +
> +static int __iio_counter_signals_register(struct iio_counter *const counter,
> +	struct iio_counter_signal *const signals, const size_t num_signals)
> +{
> +	size_t i;
> +	int err;
> +
> +	if (!counter || !signals)
> +		return -EINVAL;
> +
> +	for (i = 0; i < num_signals; i++) {
> +		err = __iio_counter_signal_register(counter, signals + i);
> +		if (err)
> +			goto err_signal_register;
> +	}
> +
> +	return 0;
> +
> +err_signal_register:
> +	while (i--)
> +		__iio_counter_signal_unregister(counter, signals + i);
> +	return err;
> +}
> +
> +static void __iio_counter_signals_unregister(struct iio_counter *const counter,
> +	struct iio_counter_signal *signals, size_t num_signals)
> +{
> +	if (!counter || !signals)
> +		return;
> +
> +	while (num_signals--) {
> +		__iio_counter_signal_unregister(counter, signals);
> +		signals++;
> +	}
> +}
> +
> +/**
> + * iio_counter_trigger_register - register Trigger to Value
> + * @value: pointer to IIO Counter Value for association
> + * @trigger: pointer to IIO Counter Trigger to register
> + *
> + * The Trigger is added to the Value's trigger_list. A check is first performed
> + * to verify that the respective Signal is not already linked to the Value; if
> + * the respective Signal is already linked to the Value, the Trigger is not
> + * added to the Value's trigger_list.
> + *
> + * NOTE: This function will acquire and release the Value's trigger_list_lock
> + * during execution.
> + */
> +int iio_counter_trigger_register(struct iio_counter_value *const value,
> +	struct iio_counter_trigger *const trigger)
> +{
> +	if (!value || !trigger || !trigger->signal)
> +		return -EINVAL;
> +
> +	mutex_lock(&value->trigger_list_lock);
> +	if (__iio_counter_trigger_find_by_id(value, trigger->signal->id)) {
> +		pr_err("Signal%d is already linked to counter value%d\n",
> +			trigger->signal->id, value->id);
> +		return -EEXIST;
> +	}
> +	list_add_tail(&trigger->list, &value->trigger_list);
> +	mutex_unlock(&value->trigger_list_lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(iio_counter_trigger_register);
> +
> +/**
> + * iio_counter_trigger_unregister - unregister Trigger from Value
> + * @value: pointer to IIO Counter Value of association
> + * @trigger: pointer to IIO Counter Trigger to unregister
> + *
> + * The Trigger is removed from the Value's trigger_list.
> + *
> + * NOTE: This function will acquire and release the Value's trigger_list_lock
> + * during execution.
> + */
> +void iio_counter_trigger_unregister(struct iio_counter_value *const value,
> +	struct iio_counter_trigger *const trigger)
> +{
> +	if (!value || !trigger || !trigger->signal)
> +		return;
> +
> +	mutex_lock(&value->trigger_list_lock);
> +	list_del(&trigger->list);
> +	mutex_unlock(&value->trigger_list_lock);
> +}
> +EXPORT_SYMBOL(iio_counter_trigger_unregister);
> +
> +/**
> + * iio_counter_triggers_register - register an array of Triggers to Value
> + * @value: pointer to IIO Counter Value for association
> + * @triggers: array of pointers to IIO Counter Triggers to register
> + *
> + * The iio_counter_trigger_register function is called for each Trigger in the
> + * array. The @triggers array is traversed for the first @num_triggers Triggers.
> + *
> + * NOTE: @num_triggers must not be greater than the size of the @triggers array.
> + */
> +int iio_counter_triggers_register(struct iio_counter_value *const value,
> +	struct iio_counter_trigger *const triggers, const size_t num_triggers)
> +{
> +	size_t i;
> +	int err;
> +
> +	if (!value || !triggers)
> +		return -EINVAL;
> +
> +	for (i = 0; i < num_triggers; i++) {
> +		err = iio_counter_trigger_register(value, triggers + i);
> +		if (err)
> +			goto err_trigger_register;
> +	}
> +
> +	return 0;
> +
> +err_trigger_register:
> +	while (i--)
> +		iio_counter_trigger_unregister(value, triggers + i);
> +	return err;
> +}
> +EXPORT_SYMBOL(iio_counter_triggers_register);
> +
> +/**
> + * iio_counter_triggers_unregister - unregister Triggers from Value
> + * @value: pointer to IIO Counter Value of association
> + * @triggers: array of pointers to IIO Counter Triggers to unregister
> + *
> + * The iio_counter_trigger_unregister function is called for each Trigger in the
> + * array. The @triggers array is traversed for the first @num_triggers Triggers.
> + *
> + * NOTE: @num_triggers must not be greater than the size of the @triggers array.
> + */
> +void iio_counter_triggers_unregister(struct iio_counter_value *const value,
> +	struct iio_counter_trigger *triggers, size_t num_triggers)
> +{
> +	if (!value || !triggers)
> +		return;
> +
> +	while (num_triggers--) {
> +		iio_counter_trigger_unregister(value, triggers);
> +		triggers++;
> +	}
> +}
> +EXPORT_SYMBOL(iio_counter_triggers_unregister);
> +
> +/**
> + * iio_counter_value_register - register Value to Counter
> + * @counter: pointer to IIO Counter for association
> + * @value: pointer to IIO Counter Value to register
> + *
> + * The registration process occurs in two major steps. First, the Value is
> + * initialized: trigger_list_lock is initialized, trigger_list is initialized,
> + * and init_triggers if not NULL is passed to iio_counter_triggers_register.
> + * Second, the Value is added to the Counter's value_list. A check is first
> + * performed to verify that the Value is not already associated to the Counter
> + * (via the Value's unique ID); if the Value is already associated to the
> + * Counter, the Value is not added to the Counter's value_list and all of the
> + * Value's Triggers are unregistered.
> + *
> + * NOTE: This function will acquire and release the Counter's value_list_lock
> + * during execution.
> + */
> +int iio_counter_value_register(struct iio_counter *const counter,
> +	struct iio_counter_value *const value)
> +{
> +	int err;
> +
> +	if (!counter || !value)
> +		return -EINVAL;
> +
> +	mutex_init(&value->trigger_list_lock);
> +	INIT_LIST_HEAD(&value->trigger_list);
> +
> +	if (value->init_triggers) {
> +		err = iio_counter_triggers_register(value,
> +			value->init_triggers, value->num_init_triggers);
> +		if (err)
> +			return err;
> +	}
> +
> +	mutex_lock(&counter->value_list_lock);
> +	if (__iio_counter_value_find_by_id(counter, value->id)) {
> +		pr_err("Duplicate counter value ID '%d'\n", value->id);
> +		err = -EEXIST;
> +		goto err_duplicate_id;
> +	}
> +	list_add_tail(&value->list, &counter->value_list);
> +	mutex_unlock(&counter->value_list_lock);
> +
> +	return 0;
> +
> +err_duplicate_id:
> +	mutex_unlock(&counter->value_list_lock);
> +	__iio_counter_trigger_unregister_all(value);
> +	return err;
> +}
> +EXPORT_SYMBOL(iio_counter_value_register);
> +
> +/**
> + * iio_counter_value_unregister - unregister Value from Counter
> + * @counter: pointer to IIO Counter of association
> + * @value: pointer to IIO Counter Value to unregister
> + *
> + * The Value is removed from the Counter's value_list and all of the Value's
> + * Triggers are unregistered.
> + *
> + * NOTE: This function will acquire and release the Counter's value_list_lock
> + * during execution.
> + */
> +void iio_counter_value_unregister(struct iio_counter *const counter,
> +	struct iio_counter_value *const value)
> +{
> +	if (!counter || !value)
> +		return;
> +
> +	mutex_lock(&counter->value_list_lock);
> +	list_del(&value->list);
> +	mutex_unlock(&counter->value_list_lock);
> +
> +	__iio_counter_trigger_unregister_all(value);
> +}
> +EXPORT_SYMBOL(iio_counter_value_unregister);
> +
> +/**
> + * iio_counter_values_register - register an array of Values to Counter
> + * @counter: pointer to IIO Counter for association
> + * @values: array of pointers to IIO Counter Values to register
> + *
> + * The iio_counter_value_register function is called for each Value in the
> + * array. The @values array is traversed for the first @num_values Values.
> + *
> + * NOTE: @num_values must not be greater than the size of the @values array.
> + */
> +int iio_counter_values_register(struct iio_counter *const counter,
> +	struct iio_counter_value *const values, const size_t num_values)
> +{
> +	size_t i;
> +	int err;
> +
> +	if (!counter || !values)
> +		return -EINVAL;
> +
> +	for (i = 0; i < num_values; i++) {
> +		err = iio_counter_value_register(counter, values + i);
> +		if (err)
> +			goto err_values_register;
> +	}
> +
> +	return 0;
> +
> +err_values_register:
> +	while (i--)
> +		iio_counter_value_unregister(counter, values + i);
> +	return err;
> +}
> +EXPORT_SYMBOL(iio_counter_values_register);
> +
> +/**
> + * iio_counter_values_unregister - unregister Values from Counter
> + * @counter: pointer to IIO Counter of association
> + * @values: array of pointers to IIO Counter Values to unregister
> + *
> + * The iio_counter_value_unregister function is called for each Value in the
> + * array. The @values array is traversed for the first @num_values Values.
> + *
> + * NOTE: @num_values must not be greater than the size of the @values array.
> + */
> +void iio_counter_values_unregister(struct iio_counter *const counter,
> +	struct iio_counter_value *values, size_t num_values)
> +{
> +	if (!counter || !values)
> +		return;
> +
> +	while (num_values--) {
> +		iio_counter_value_unregister(counter, values);
> +		values++;
> +	}
> +}
> +EXPORT_SYMBOL(iio_counter_values_unregister);
> +
> +/**
> + * 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 initialized; if init_signals is not NULL it is passed to
> + * iio_counter_signals_register, and similarly if init_values is not NULL it is
> + * passed to iio_counter_values_register. 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 is copied 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 = {
> +		.driver_module = THIS_MODULE,
> +		.read_raw = __iio_counter_read_raw,
> +		.write_raw = __iio_counter_write_raw
> +	};
> +	int err;
> +	struct iio_info *info;
> +	struct iio_counter *priv;
> +
> +	if (!counter)
> +		return -EINVAL;
> +
> +	mutex_init(&counter->signal_list_lock);
> +	INIT_LIST_HEAD(&counter->signal_list);
> +
> +	if (counter->init_signals) {
> +		err = __iio_counter_signals_register(counter,
> +			counter->init_signals, counter->num_init_signals);
> +		if (err)
> +			return err;
> +	}
> +
> +	mutex_init(&counter->value_list_lock);
> +	INIT_LIST_HEAD(&counter->value_list);
> +
> +	if (counter->init_values) {
> +		err = iio_counter_values_register(counter,
> +			counter->init_values, counter->num_init_values);
> +		if (err)
> +			goto err_values_register;
> +	}
> +
> +	counter->indio_dev = iio_device_alloc(sizeof(*counter));

This is all getting a bit nasty and recursive.  You have a
counter containing an iio_dev containing a copy of the counter.

I'd just put a pointer to the outer counter in there instead.


> +	if (!counter->indio_dev) {
> +		err = -ENOMEM;
> +		goto err_iio_device_alloc;
> +	}
> +
> +	info = kmalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info) {
> +		err = -ENOMEM;
> +		goto err_info_alloc;
> +	}
> +	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;
> +
> +	err = __iio_counter_channels_alloc(counter);
> +	if (err)
> +		goto err_channels_alloc;
> +
> +	priv = iio_priv(counter->indio_dev);
> +	memcpy(priv, counter, sizeof(*priv));
> +
> +	err = iio_device_register(priv->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);
> +err_iio_device_alloc:
> +	iio_counter_values_unregister(counter, counter->init_values,
> +		counter->num_init_values);
> +err_values_register:
> +	__iio_counter_signals_unregister(counter, counter->init_signals,
> +		counter->num_init_signals);
> +	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,
> + * allocated memory is freed, and all associated Values and Signals are
> + * unregistered.
> + */
> +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);
> +
> +	__iio_counter_value_unregister_all(counter);
> +	__iio_counter_signal_unregister_all(counter);
> +}
> +EXPORT_SYMBOL(iio_counter_unregister);
> diff --git a/include/linux/iio/counter.h b/include/linux/iio/counter.h
> new file mode 100644
> index 000000000000..9a67c2a7cc46
> --- /dev/null
> +++ b/include/linux/iio/counter.h
> @@ -0,0 +1,221 @@
> +/*
> + * 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/mutex.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
> + * @list:	[INTERN] list of all signals currently registered to counter
> + */
> +struct iio_counter_signal {
> +	int		id;
> +	const char	*name;
> +
> +	struct list_head	list;
> +};
> +
> +/**
> + * 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 @trigger_modes
> + * @signal:		[DRIVER] pointer to associated signal
> + * @list:		[INTERN] list of all triggers currently registered to
> + *			value
> + */
> +struct iio_counter_trigger {
> +	unsigned int			mode;
> +	const char *const		*trigger_modes;
> +	unsigned int			num_trigger_modes;
> +	struct iio_counter_signal	*signal;
> +
> +	struct list_head		list;
> +};
> +
> +/**
> + * 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
> + * @init_triggers:	[DRIVER] array of triggers for initialization
> + * @num_init_triggers:	[DRIVER] number of triggers specified in @init_triggers
> + * @function_enum:	[INTERN] used internally to generate function attributes
> + * @trigger_list_lock:	[INTERN] lock for accessing @trigger_list
> + * @trigger_list:	[INTERN] list of all triggers currently registered to
> + *			value
> + * @list:		[INTERN] list of all values currently registered to
> + *			counter
> + */
> +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	*init_triggers;
> +	size_t				num_init_triggers;
> +
> +	struct iio_enum		function_enum;
> +	struct mutex		trigger_list_lock;
> +	struct list_head	trigger_list;
> +
> +	struct list_head	list;
> +};
> +
> +struct iio_counter;
> +
> +/**
> + * struct iio_counter_ops - IIO Counter related callbacks
> + * @signal_read:	function to request a signal 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. Note that the counter
> + *			signal_list_lock is acquired before this function is
> + *			called, and released after this function returns.
> + * @signal_write:	function to write a signal value to the device.
> + *			Parameters and locking behavior are the same as
> + *			signal_read.
> + * @trigger_mode_set:	function to set the trigger mode. mode is the index of
> + *			the requested mode from the value trigger_modes array.
> + *			Note that the counter value_list_lock and value
> + *			trigger_list_lock are acquired before this function is
> + *			called, and released after this function returns.
> + * @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. Locking behavior is the same
> + *			as trigger_mode_set.
> + * @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. Note that the counter
> + *			value_list_lock is acquired before this function is
> + *			called, and released after this function returns.
> + * @value_write:	function to write a value value to the device.
> + *			Parameters and locking behavior are 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. Note that the counter
> + *			value_list_lock is acquired before this function is
> + *			called, and released after this function returns.
> + * @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. Locking behavior is the
> + *			same as value_function_get.
> + */
> +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 for from driver
> + * @init_signals:	[DRIVER] array of signals for initialization
> + * @num_init_signals:	[DRIVER] number of signals specified in @init_signals
> + * @init_values:	[DRIVER] array of values for initialization
> + * @num_init_values:	[DRIVER] number of values specified in @init_values
> + * @signal_list_lock:	[INTERN] lock for accessing @signal_list
> + * @signal_list:	[INTERN] list of all signals currently registered to
> + *			counter
> + * @value_list_lock:	[INTERN] lock for accessing @value_list
> + * @value_list:		[INTERN] list of all values currently registered to
> + *			counter
> + * @channels:		[DRIVER] channel specification structure table
> + * @num_channels:	[DRIVER] number of channels specified in @channels
> + * @info:		[DRIVER] callbacks and constant info from driver
> + * @indio_dev:		[INTERN] industrial I/O device structure
> + * @driver_data:	[DRIVER] driver data
> + */
> +struct iio_counter {
> +	int				id;
> +	const char			*name;
> +	struct device			*dev;
> +	const struct iio_counter_ops	*ops;
> +
> +	struct iio_counter_signal	*init_signals;
> +	size_t				num_init_signals;
I'm missing if there is any chance of signals or values being
registered later?  If not then drop the init_.
> +	struct iio_counter_value	*init_values;
> +	size_t				num_init_values;
> +
> +	struct mutex		signal_list_lock;
> +	struct list_head	signal_list;
> +	struct mutex		value_list_lock;
> +	struct list_head	value_list;
> +
> +	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_trigger_register(struct iio_counter_value *const value,
> +	struct iio_counter_trigger *const trigger);
> +void iio_counter_trigger_unregister(struct iio_counter_value *const value,
> +	struct iio_counter_trigger *const trigger);
> +int iio_counter_triggers_register(struct iio_counter_value *const value,
> +	struct iio_counter_trigger *const triggers, const size_t num_triggers);
> +void iio_counter_triggers_unregister(struct iio_counter_value *const value,
> +	struct iio_counter_trigger *triggers, size_t num_triggers);
> +
> +int iio_counter_value_register(struct iio_counter *const counter,
> +	struct iio_counter_value *const value);
> +void iio_counter_value_unregister(struct iio_counter *const counter,
> +	struct iio_counter_value *const value);
> +int iio_counter_values_register(struct iio_counter *const counter,
> +	struct iio_counter_value *const values, const size_t num_values);
> +void iio_counter_values_unregister(struct iio_counter *const counter,
> +	struct iio_counter_value *values, size_t num_values);
> +
> +int iio_counter_register(struct iio_counter *const counter);
> +void iio_counter_unregister(struct iio_counter *const counter);
> +
> +#endif /* CONFIG_IIO_COUNTER */
> +
> +#endif /* _IIO_COUNTER_H_ */