LinuxLists.cc - [PATCH 0/3] iio: Introduce the generic counter interface

2017-07-31 16:03:01

Subject: [PATCH 0/3] iio: Introduce the generic counter interface

Summary
=======

Counter devices are prevalent within a diverse spectrum of industries.
The ubiquitous presence of these devices necessitates a common interface
and standard of interaction and exposure. This patchset attempts to
resolve the issue of duplicate code found among existing counter device
drivers by introducing a generic counter interface for consumption. The
generic counter interface enables drivers to support and expose a common
set of components and functionality present in counter devices.

Theory
======

Counter devices can vary greatly in design, but regardless of whether
some devices are quadrature encoder counters or pedometers, all counter
devices consist of a core set of components. This core set of
components, shared by all counter devices, is what forms the essence of
the generic counter interface.

There are three core components to a counter:

VALUE
-----
A Value represents the count data for a set of Signals. A Value
has a count function mode (e.g. "increment" or "quadrature x4")
which respresents the update behavior for the count data. A
Value also has a set of one or more associated Signals.

SIGNAL
------
A Signal represents a count input line. A Signal may be
associated to one or more Values.

TRIGGER
-------
A Trigger represents a Value's count function trigger condition
mode (e.g. "rising edge" or "double pulse") for an associated
Signal. If a Signal is associated with a Value, a respective
Trigger instance for that association exists -- albeit perhaps
with a trigger condition mode of "none."

A counter is defined as a set of input signals associated to count data
that are generated by the evaluation of the state of the associated
input signals as defined by the respective count functions. Within the
context of the generic counter interface, a counter consists of Values
each associated to a set of Signals, whose respective Trigger instances
represent the count function update conditions for the associated
Values.

Implementation
==============

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.

Userspace Interface
===================

Several sysfs attributes are generated by the generic counter interface,
and reside under the /sys/bus/iio/devices/iio:deviceX directory.

Each counter has a respective set of countX-Y and signalX-Y prefixed
attributes, where X is the id set in the counter structure, and Y is the
id of the respective Value or Signal.

The generic counter interface sysfs attributes are as follows:

countX-Y_function: count function mode
countX-Y_function_available: available count function modes
countX-Y_name: Value name
countX-Y_raw: Value data
countX-Y_triggers: Value's associated Signals
countX-Y_trigger_signalX-Z: Value Y trigger mode for Signal Z
countX-Y_trigger_signalX-Z_available: available Value Y trigger
modes for Signal Z
signalX-Y_name: Signal name
signalX-Y_raw: Signal data

Future Development
==================

This patchset is focused on providing the core functionality required to
introduce a usable generic counter interface. Unfortunately, that means
nice features were left out for the sake of simplicity. It's probably
useful to include support for common counter functionality and
configurations such as preset values, min/max boundaries, and such in
future patches.

Take into consideration however that it is my intention for this generic
counter interface to serve as a base level of code to support counter
devices; that is to say: a feature should only be added to the generic
counter interface is it is pervasive to counter devices in general. If a
feature is specific to a subset of counter devices, then that feature
should be added to the respective subset counter interface; for example,
a "quadrature pair" atribute should be introduced to the quadrature
counter interface instead of the generic counter interface.

Managed memory functions such as devm_iio_counter_register should be
introduced in a future patch to provide support that aligns with the
current design of code in the kernel. This should help prevent memory
leaks and reduce a lot of boiler plate code in drivers.

I would like to see a character device node interface similar to the one
provided by the GPIO subsystem. I designed the generic counter interface
to allow for dynamic reconfiguration of the Value-Signal associations,
where a user may decide to attached or remove Signals to/from Values at
will; I don't believe a sysfs attribute system lends itself well to
these kinds of interactions.

Current Patchset Concerns
=========================

I'm piggybacking off the IIO core with this patchset, but I'm not sure
if its appropriate for the longterm. A lot of IIO core is ignored, and
I feel that I'm essentially just leveraging it for its sysfs code.
However, utilizing iio_device_register within iio_counter_register does
allow driver authors to pass in their own IIO channels to support
attributes and features not provide by the IIO generic counter interface
code -- so I'm a bit undecided whether to part completely either.

For what its worth, I'd like the generic counter interface to stay
piggybacked at least until the quadrature counter interface is release;
that should allow the driver authors to start making use of the
quadrature counter interface while the generic counter interface is
improved under the hood.

You may have noticed that Signals do not have associated
iio_counter_signal_register/iio_counter_signal_unregister functions.
This is because I encountered a deadlock scenario where unregistering a
Signal dynamically requires a search of all Values' Triggers to
determine is the Signal is associated to any Values and subsequently
remove that association; a lock of the counter structure
signal_list_lock is required at the iio_counter_signal_unregister call,
and then once more when performing the search through the Triggers
(in order to prevent a trigger structure signal member pointer being
dereferenced after the respective Signal is freed).

Since Signals are likely to be static for the lifetime of a driver
(since Signals typically represent physical lines on the device), I
decided to prevent drivers from dynamically registering and
unregistering Signals for the time being. I'm not sure whether this
design should stay however; I don't want to restrict a clever driver
or future interface that wants to dynamically register/unregister
Signals.

I put a dependency on CONFIG_IIO_COUNTER for the Kconfig IIO Counter
menu. Is this appropriate; should this menu simply select IIO_COUNTER
instead of a hard depends; or should drivers individually depend on
CONFIG_IIO_COUNTER?

This is more of a style nitpick: should I prefix static symbols with __?
I took on this coding convention when writing the generic counter
interface code, but I'm not sure if it is appropriate in this manner.

Finally, I'd like to actively encourage symbol renaming suggestions. I'm
afraid names such as "Value" and "Trigger" may be too easily confused
for other unrelated subsystem conventions (e.g. IIO Triggers). I mainly
stuck with the names and symbols used in this patchset so that I could
focus on developing the interface code. However, if the chance of
confusion is small, we can stick with the existing naming convention as
well.

William Breathitt Gray (3):
iio: Implement counter channel specification and IIO_SIGNAL constant
iio: Introduce the generic counter interface
iio: 104-quad-8: Add IIO generic counter interface support

MAINTAINERS | 7 +
drivers/iio/Kconfig | 8 +
drivers/iio/Makefile | 1 +
drivers/iio/counter/104-quad-8.c | 306 +++++++++-
drivers/iio/counter/Kconfig | 1 +
drivers/iio/industrialio-core.c | 14 +-
drivers/iio/industrialio-counter.c | 1157 ++++++++++++++++++++++++++++++++++++
include/linux/iio/counter.h | 221 +++++++
include/linux/iio/iio.h | 2 +
include/uapi/linux/iio/types.h | 1 +
10 files changed, 1700 insertions(+), 18 deletions(-)
create mode 100644 drivers/iio/industrialio-counter.c
create mode 100644 include/linux/iio/counter.h

--
2.13.3

2017-07-31 16:03:21

by William Breathitt Gray

[permalink] [raw]

Subject: [PATCH 1/3] iio: Implement counter channel specification and IIO_SIGNAL constant

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(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 17ec4cee51dc..70347ee7416d 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -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;

case IIO_SEPARATE:
- if (chan->indexed)
+ if (chan->counter) {
+ if (!chan->indexed) {
+ WARN(1, "Counter channels must be indexed\n");
+ ret = -EINVAL;
+ goto error_free_full_postfix;
+ }
+ name = kasprintf(GFP_KERNEL, "%s%d-%d_%s",
+ iio_chan_type_name_spec[chan->type],
+ chan->channel,
+ chan->channel2,
+ full_postfix);
+ } else if (chan->indexed)
name = kasprintf(GFP_KERNEL, "%s_%s%d_%s",
iio_direction[chan->output],
iio_chan_type_name_spec[chan->type],
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index d68bec297a45..6acce1016625 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -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;
};

diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index ffafd6c25a48..313899652ca7 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -43,6 +43,7 @@ enum iio_chan_type {
IIO_COUNT,
IIO_INDEX,
IIO_GRAVITY,
+ IIO_SIGNAL,
};

enum iio_modifier {
--
2.13.3

2017-07-31 16:03:34

by William Breathitt Gray

[permalink] [raw]

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

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 <[email protected]>
---
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 <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/iio/industrialio-counter.c
+F: include/linux/iio/counter.h
+
IIO SUBSYSTEM AND DRIVERS
M: Jonathan Cameron <[email protected]>
R: Hartmut Knaack <[email protected]>
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);
+ 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:
+ 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));
+ 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;
+ 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_ */
--
2.13.3

2017-07-31 16:03:55

by William Breathitt Gray

[permalink] [raw]

Subject: [PATCH 3/3] iio: 104-quad-8: Add IIO generic counter interface support

This patch adds support for the IIO generic counter interface to the
104-QUAD-8 driver. The existing 104-QUAD-8 device interface should not
be affected by this patch; all changes are intended as supplemental
additions as perceived by the user.

IIO Counter Signals are defined for all quadrature input pairs
(A and B), as well as index input lines. However, IIO Counter Triggers
are not created for the index input Signals. IIO Counter Values are
created for the eight quadrature channel counts, and their respective
Signals are associated via IIO Counter Triggers.

The new generic counter interface sysfs attributes expose the same
functionality and data available via the existing 104-QUAD-8 device
interface. Four IIO Counter Value function modes are available,
correlating to the four possible quadrature mode configurations:
"non-quadrature," "quadrature x1," "quadrature x2," and "quadrature x4."

A quad8_remove function is defined to call iio_counter_unregister. This
function can be eliminated once a devm_iio_counter_register function is
defined.

Signed-off-by: William Breathitt Gray <[email protected]>
---
drivers/iio/counter/104-quad-8.c | 306 ++++++++++++++++++++++++++++++++++++---
1 file changed, 289 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/counter/104-quad-8.c b/drivers/iio/counter/104-quad-8.c
index ba3d9030cd51..72b88b7de5b3 100644
--- a/drivers/iio/counter/104-quad-8.c
+++ b/drivers/iio/counter/104-quad-8.c
@@ -16,6 +16,7 @@
#include <linux/bitops.h>
#include <linux/device.h>
#include <linux/errno.h>
+#include <linux/iio/counter.h>
#include <linux/iio/iio.h>
#include <linux/iio/types.h>
#include <linux/io.h>
@@ -24,6 +25,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
+#include <linux/string.h>
#include <linux/types.h>

#define QUAD8_EXTENT 32
@@ -37,6 +39,7 @@ MODULE_PARM_DESC(base, "ACCES 104-QUAD-8 base addresses");

/**
* struct quad8_iio - IIO device private data structure
+ * @counter: instance of the iio_counter
* @preset: array of preset values
* @count_mode: array of count mode configurations
* @quadrature_mode: array of quadrature mode configurations
@@ -48,6 +51,7 @@ MODULE_PARM_DESC(base, "ACCES 104-QUAD-8 base addresses");
* @base: base port address of the IIO device
*/
struct quad8_iio {
+ struct iio_counter counter;
unsigned int preset[QUAD8_NUM_COUNTERS];
unsigned int count_mode[QUAD8_NUM_COUNTERS];
unsigned int quadrature_mode[QUAD8_NUM_COUNTERS];
@@ -528,33 +532,289 @@ static const struct iio_chan_spec quad8_channels[] = {
QUAD8_COUNT_CHAN(7), QUAD8_INDEX_CHAN(7)
};

+static int quad8_signal_read(struct iio_counter *counter,
+ struct iio_counter_signal *signal, int *val, int *val2)
+{
+ struct quad8_iio *const priv = counter->driver_data;
+
+ if (signal->id < 16)
+ return -EINVAL;
+
+ *val = !!(inb(priv->base + 0x16) & BIT(signal->id - 16));
+
+ return IIO_VAL_INT;
+}
+
+static int quad8_trigger_mode_get(struct iio_counter *counter,
+ struct iio_counter_value *value, struct iio_counter_trigger *trigger)
+{
+ struct quad8_iio *const priv = counter->driver_data;
+ const unsigned int mode = priv->quadrature_mode[value->id];
+ const unsigned int scale = priv->quadrature_scale[value->id];
+ unsigned int direction;
+ const unsigned int flag_addr = priv->base + 2 * value->id + 1;
+ const int signal_id = trigger->signal->id % 2;
+
+ if (mode)
+ switch (scale) {
+ case 0:
+ /* U/D flag: 1 = up, 0 = down */
+ /* direction: 0 = up, 1 = down */
+ direction = !(inb(flag_addr) & BIT(5));
+ if (!signal_id)
+ return direction + 1;
+ break;
+ case 1:
+ if (!signal_id)
+ return 3;
+ break;
+ case 2:
+ return 3;
+ }
+ else
+ if (!signal_id)
+ return 1;
+
+ return 0;
+}
+
+static int quad8_value_read(struct iio_counter *counter,
+ struct iio_counter_value *value, int *val, int *val2)
+{
+ struct quad8_iio *const priv = counter->driver_data;
+ const int base_offset = priv->base + 2 * value->id;
+ unsigned int flags;
+ unsigned int borrow;
+ unsigned int carry;
+ int i;
+
+ flags = inb(base_offset + 1);
+ borrow = flags & BIT(0);
+ carry = !!(flags & BIT(1));
+
+ /* Borrow XOR Carry effectively doubles count range */
+ *val = (borrow ^ carry) << 24;
+
+ /* Reset Byte Pointer; transfer Counter to Output Latch */
+ outb(0x11, base_offset + 1);
+
+ for (i = 0; i < 3; i++)
+ *val |= (unsigned int)inb(base_offset) << (8 * i);
+
+ return IIO_VAL_INT;
+}
+
+static int quad8_value_write(struct iio_counter *counter,
+ struct iio_counter_value *value, int val, int val2)
+{
+ struct quad8_iio *const priv = counter->driver_data;
+ const int base_offset = priv->base + 2 * value->id;
+ int i;
+
+ /* Only 24-bit values are supported */
+ if ((unsigned int)val > 0xFFFFFF)
+ return -EINVAL;
+
+ /* Reset Byte Pointer */
+ outb(0x01, base_offset + 1);
+
+ /* Counter can only be set via Preset Register */
+ for (i = 0; i < 3; i++)
+ outb(val >> (8 * i), base_offset);
+
+ /* Transfer Preset Register to Counter */
+ outb(0x08, base_offset + 1);
+
+ /* Reset Byte Pointer */
+ outb(0x01, base_offset + 1);
+
+ /* Set Preset Register back to original value */
+ val = priv->preset[value->id];
+ for (i = 0; i < 3; i++)
+ outb(val >> (8 * i), base_offset);
+
+ /* Reset Borrow, Carry, Compare, and Sign flags */
+ outb(0x02, base_offset + 1);
+ /* Reset Error flag */
+ outb(0x06, base_offset + 1);
+
+ return 0;
+}
+
+static int quad8_value_function_set(struct iio_counter *counter,
+ struct iio_counter_value *value, unsigned int mode)
+{
+ struct quad8_iio *const priv = counter->driver_data;
+ const unsigned int mode_cfg = mode << 3 |
+ priv->count_mode[value->id] << 1;
+ const unsigned int idr_cfg = priv->index_polarity[value->id] << 1;
+ const int base_offset = priv->base + 2 * value->id + 1;
+
+ if (mode)
+ priv->quadrature_scale[value->id] = mode - 1;
+ else {
+ /* Quadrature scaling only available in quadrature mode */
+ priv->quadrature_scale[value->id] = 0;
+
+ /* Synchronous function not supported in non-quadrature mode */
+ if (priv->synchronous_mode[value->id]) {
+ priv->synchronous_mode[value->id] = 0;
+ outb(0x60 | idr_cfg, base_offset);
+ }
+ }
+
+ priv->quadrature_mode[value->id] = !!mode;
+
+ /* Load mode configuration to Counter Mode Register */
+ outb(0x20 | mode_cfg, base_offset);
+
+ return 0;
+}
+
+static int quad8_value_function_get(struct iio_counter *counter,
+ struct iio_counter_value *value)
+{
+ struct quad8_iio *const priv = counter->driver_data;
+ unsigned int quadrature_mode = priv->quadrature_mode[value->id];
+
+ return (quadrature_mode) ? priv->quadrature_scale[value->id] + 1 : 0;
+}
+
+static const struct iio_counter_ops quad8_ops = {
+ .signal_read = quad8_signal_read,
+ .trigger_mode_get = quad8_trigger_mode_get,
+ .value_read = quad8_value_read,
+ .value_write = quad8_value_write,
+ .value_function_set = quad8_value_function_set,
+ .value_function_get = quad8_value_function_get
+};
+
+static const char *const quad8_function_modes[] = {
+ "non-quadrature",
+ "quadrature x1",
+ "quadrature x2",
+ "quadrature x4"
+};
+
+#define QUAD8_SIGNAL(_id, _name) { \
+ .id = _id, \
+ .name = _name \
+}
+
+static const struct iio_counter_signal quad8_signals[] = {
+ QUAD8_SIGNAL(0, "Channel 1 Quadrature A"),
+ QUAD8_SIGNAL(1, "Channel 1 Quadrature B"),
+ QUAD8_SIGNAL(2, "Channel 2 Quadrature A"),
+ QUAD8_SIGNAL(3, "Channel 2 Quadrature B"),
+ QUAD8_SIGNAL(4, "Channel 3 Quadrature A"),
+ QUAD8_SIGNAL(5, "Channel 3 Quadrature B"),
+ QUAD8_SIGNAL(6, "Channel 4 Quadrature A"),
+ QUAD8_SIGNAL(7, "Channel 4 Quadrature B"),
+ QUAD8_SIGNAL(8, "Channel 5 Quadrature A"),
+ QUAD8_SIGNAL(9, "Channel 5 Quadrature B"),
+ QUAD8_SIGNAL(10, "Channel 6 Quadrature A"),
+ QUAD8_SIGNAL(11, "Channel 6 Quadrature B"),
+ QUAD8_SIGNAL(12, "Channel 7 Quadrature A"),
+ QUAD8_SIGNAL(13, "Channel 7 Quadrature B"),
+ QUAD8_SIGNAL(14, "Channel 8 Quadrature A"),
+ QUAD8_SIGNAL(15, "Channel 8 Quadrature B"),
+ QUAD8_SIGNAL(16, "Channel 1 Index"),
+ QUAD8_SIGNAL(17, "Channel 2 Index"),
+ QUAD8_SIGNAL(18, "Channel 3 Index"),
+ QUAD8_SIGNAL(19, "Channel 4 Index"),
+ QUAD8_SIGNAL(20, "Channel 5 Index"),
+ QUAD8_SIGNAL(21, "Channel 6 Index"),
+ QUAD8_SIGNAL(22, "Channel 7 Index"),
+ QUAD8_SIGNAL(23, "Channel 8 Index")
+};
+
+#define QUAD8_VALUE(_id, _name) { \
+ .id = _id, \
+ .name = _name, \
+ .mode = 0, \
+ .function_modes = quad8_function_modes, \
+ .num_function_modes = ARRAY_SIZE(quad8_function_modes) \
+}
+
+static const struct iio_counter_value quad8_values[] = {
+ QUAD8_VALUE(0, "Channel 1 Count"), QUAD8_VALUE(1, "Channel 2 Count"),
+ QUAD8_VALUE(2, "Channel 3 Count"), QUAD8_VALUE(3, "Channel 4 Count"),
+ QUAD8_VALUE(4, "Channel 5 Count"), QUAD8_VALUE(5, "Channel 6 Count"),
+ QUAD8_VALUE(6, "Channel 7 Count"), QUAD8_VALUE(7, "Channel 8 Count")
+};
+
+static const char *const quad8_trigger_modes[] = {
+ "none",
+ "rising edge",
+ "falling edge",
+ "both edges"
+};
+
static int quad8_probe(struct device *dev, unsigned int id)
{
- struct iio_dev *indio_dev;
- struct quad8_iio *priv;
+ struct iio_counter_signal *init_signals;
+ const size_t num_init_signals = ARRAY_SIZE(quad8_signals);
+ struct iio_counter_value *init_values;
+ const size_t num_init_values = ARRAY_SIZE(quad8_values);
+ struct iio_counter_trigger *triggers;
+ struct quad8_iio *quad8iio;
int i, j;
unsigned int base_offset;

- indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
- if (!indio_dev)
- return -ENOMEM;
-
- if (!devm_request_region(dev, base[id], QUAD8_EXTENT,
- dev_name(dev))) {
+ if (!devm_request_region(dev, base[id], QUAD8_EXTENT, dev_name(dev))) {
dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n",
base[id], base[id] + QUAD8_EXTENT);
return -EBUSY;
}

- indio_dev->info = &quad8_info;
- indio_dev->modes = INDIO_DIRECT_MODE;
- indio_dev->num_channels = ARRAY_SIZE(quad8_channels);
- indio_dev->channels = quad8_channels;
- indio_dev->name = dev_name(dev);
- indio_dev->dev.parent = dev;
+ init_signals = devm_kmalloc(dev, sizeof(quad8_signals), GFP_KERNEL);
+ if (!init_signals)
+ return -ENOMEM;
+
+ memcpy(init_signals, quad8_signals, sizeof(quad8_signals));
+
+ init_values = devm_kmalloc(dev, sizeof(quad8_values), GFP_KERNEL);
+ if (!init_values)
+ return -ENOMEM;
+
+ memcpy(init_values, quad8_values, sizeof(quad8_values));
+
+ /* Associate values with their respective signals */
+ for (i = 0; i < num_init_values; i++) {
+ triggers = devm_kmalloc(dev, 2 * sizeof(*triggers), GFP_KERNEL);
+ if (!triggers)
+ return -ENOMEM;
+
+ /* Starts up in non-quadrature mode */
+ triggers[0].mode = 1;
+ triggers[0].trigger_modes = quad8_trigger_modes;
+ triggers[0].num_trigger_modes = ARRAY_SIZE(quad8_trigger_modes);
+ triggers[0].signal = &init_signals[2 * i];
+ triggers[1].mode = 0;
+ triggers[1].trigger_modes = quad8_trigger_modes;
+ triggers[1].num_trigger_modes = ARRAY_SIZE(quad8_trigger_modes);
+ triggers[1].signal = &init_signals[2 * i + 1];
+
+ init_values[i].init_triggers = triggers;
+ init_values[i].num_init_triggers = 2;
+ }
+
+ quad8iio = devm_kzalloc(dev, sizeof(*quad8iio), GFP_KERNEL);
+ if (!quad8iio)
+ return -ENOMEM;

- priv = iio_priv(indio_dev);
- priv->base = base[id];
+ quad8iio->counter.name = dev_name(dev);
+ quad8iio->counter.dev = dev;
+ quad8iio->counter.ops = &quad8_ops;
+ quad8iio->counter.init_signals = init_signals;
+ quad8iio->counter.num_init_signals = num_init_signals;
+ quad8iio->counter.init_values = init_values;
+ quad8iio->counter.num_init_values = num_init_values;
+ quad8iio->counter.channels = quad8_channels;
+ quad8iio->counter.num_channels = ARRAY_SIZE(quad8_channels);
+ quad8iio->counter.info = &quad8_info;
+ quad8iio->counter.driver_data = quad8iio;
+ quad8iio->base = base[id];

/* Reset all counters and disable interrupt function */
outb(0x01, base[id] + 0x11);
@@ -580,11 +840,23 @@ static int quad8_probe(struct device *dev, unsigned int id)
/* Enable all counters */
outb(0x00, base[id] + 0x11);

- return devm_iio_device_register(dev, indio_dev);
+ dev_set_drvdata(dev, &quad8iio->counter);
+
+ return iio_counter_register(&quad8iio->counter);
+}
+
+static int quad8_remove(struct device *dev, unsigned int id)
+{
+ struct iio_counter *counter = dev_get_drvdata(dev);
+
+ iio_counter_unregister(counter);
+
+ return 0;
}

static struct isa_driver quad8_driver = {
.probe = quad8_probe,
+ .remove = quad8_remove,
.driver = {
.name = "104-quad-8"
}
--
2.13.3

2017-07-31 23:15:34

On Mon, 28 Aug 2017 12:23:45 -0400
William Breathitt Gray <[email protected]> wrote:

> On Sun, Aug 20, 2017 at 01:11:18PM +0100, Jonathan Cameron wrote:
> >On Mon, 31 Jul 2017 12:03:46 -0400
> >William Breathitt Gray <[email protected]> wrote:
> >
> >> This patch adds support for the IIO generic counter interface to the
> >> 104-QUAD-8 driver. The existing 104-QUAD-8 device interface should not
> >> be affected by this patch; all changes are intended as supplemental
> >> additions as perceived by the user.
> >>
> >> IIO Counter Signals are defined for all quadrature input pairs
> >> (A and B), as well as index input lines. However, IIO Counter Triggers
> >> are not created for the index input Signals. IIO Counter Values are
> >> created for the eight quadrature channel counts, and their respective
> >> Signals are associated via IIO Counter Triggers.
> >>
> >> The new generic counter interface sysfs attributes expose the same
> >> functionality and data available via the existing 104-QUAD-8 device
> >> interface. Four IIO Counter Value function modes are available,
> >> correlating to the four possible quadrature mode configurations:
> >> "non-quadrature," "quadrature x1," "quadrature x2," and "quadrature x4."
> >>
> >> A quad8_remove function is defined to call iio_counter_unregister. This
> >> function can be eliminated once a devm_iio_counter_register function is
> >> defined.
> >>
> >> Signed-off-by: William Breathitt Gray <[email protected]>
> >
> >A good example.
> >
> >I think it does make it clear that we need to be very careful on how much of
> >the interface is defined by freeform strings. Even if we export other means
> >of establishing the associations to userspace, the moment there are strings
> >available giving them names, software will start to use those.
> >
> >May be fine but we need to be very careful.
>
> I would like to limit the amount of strings as well; the availability of
> freeform strings has an unfortunate tendency to create situations where
> different drivers form separate conventions for duplicate functionality.

Absolutely.

>
> The reason freeform strings are available for the generic counter
> interface is to provide the flexibility to support more complex classes
> of counters. More specific counter class interfaces such as the future
> quadrature counter interface will likely expose predefined constants
> rather than allow drivers to create their own strings. In general
> though, I believe your warning is a good word of caution and I'll keep
> an eye on reducing the amount of freeform strings we allow.

OK. That could work fine - we enforce the usage by review rather than
code.
>
> In truth, while this is a good example of how a driver would utilize the
> generic counter interface with real hardware, it's not a perfect case
> for quadrature counters in general. As you noticed, the dynamic aspects
> of the generic counter interface are not needed by the 104-QUAD-8. The
> future quadrature counter interface would be more fitting for the
> 104-QUAD-8.
>
> In addition, I may provide a dummy software counter driver in version 2
> of this patchset to showcase and exemplify the functionality of the
> generic counter interface more directly and aptly.

That could be very useful. An alternative would be to look at a simple
device (if we can find one) and implement a userspace fake for it
(similar to what Guenter has done with lots of hwmon devices).

That way we can play with a real driver against fake hardware and get
the best of all possible worlds. I've been meaning to look at doing this
for various IIO drivers for a while (most complete for i2c devices I think).

As you might imagine I don't actually have that many parts (and most of them
aren't connected to boards at any given time) so any form of emulation can be
very helpful.

I'll see if I can dig up any interesting devices beyond the ones we
already know are integrated in various SoCs.

I think we really need say 2+ devices to justify decisions in the
core code. I did the original IIO version against 3ish devices but
that was more varied (and that wasn't nearly enough with hindsight!)

Jonathan

2017-09-03 18:24:59

by Jonathan Cameron

[permalink] [raw]

Subject: Re: [PATCH 3/3] iio: 104-quad-8: Add IIO generic counter interface support

On Sun, 3 Sep 2017 18:50:01 +0100
Jonathan Cameron <[email protected]> wrote:

> On Mon, 28 Aug 2017 12:23:45 -0400
> William Breathitt Gray <[email protected]> wrote:
>
> > On Sun, Aug 20, 2017 at 01:11:18PM +0100, Jonathan Cameron wrote:
> > >On Mon, 31 Jul 2017 12:03:46 -0400
> > >William Breathitt Gray <[email protected]> wrote:
> > >
> > >> This patch adds support for the IIO generic counter interface to the
> > >> 104-QUAD-8 driver. The existing 104-QUAD-8 device interface should not
> > >> be affected by this patch; all changes are intended as supplemental
> > >> additions as perceived by the user.
> > >>
> > >> IIO Counter Signals are defined for all quadrature input pairs
> > >> (A and B), as well as index input lines. However, IIO Counter Triggers
> > >> are not created for the index input Signals. IIO Counter Values are
> > >> created for the eight quadrature channel counts, and their respective
> > >> Signals are associated via IIO Counter Triggers.
> > >>
> > >> The new generic counter interface sysfs attributes expose the same
> > >> functionality and data available via the existing 104-QUAD-8 device
> > >> interface. Four IIO Counter Value function modes are available,
> > >> correlating to the four possible quadrature mode configurations:
> > >> "non-quadrature," "quadrature x1," "quadrature x2," and "quadrature x4."
> > >>
> > >> A quad8_remove function is defined to call iio_counter_unregister. This
> > >> function can be eliminated once a devm_iio_counter_register function is
> > >> defined.
> > >>
> > >> Signed-off-by: William Breathitt Gray <[email protected]>
> > >
> > >A good example.
> > >
> > >I think it does make it clear that we need to be very careful on how much of
> > >the interface is defined by freeform strings. Even if we export other means
> > >of establishing the associations to userspace, the moment there are strings
> > >available giving them names, software will start to use those.
> > >
> > >May be fine but we need to be very careful.
> >
> > I would like to limit the amount of strings as well; the availability of
> > freeform strings has an unfortunate tendency to create situations where
> > different drivers form separate conventions for duplicate functionality.
>
> Absolutely.
>
> >
> > The reason freeform strings are available for the generic counter
> > interface is to provide the flexibility to support more complex classes
> > of counters. More specific counter class interfaces such as the future
> > quadrature counter interface will likely expose predefined constants
> > rather than allow drivers to create their own strings. In general
> > though, I believe your warning is a good word of caution and I'll keep
> > an eye on reducing the amount of freeform strings we allow.
>
> OK. That could work fine - we enforce the usage by review rather than
> code.
> >
> > In truth, while this is a good example of how a driver would utilize the
> > generic counter interface with real hardware, it's not a perfect case
> > for quadrature counters in general. As you noticed, the dynamic aspects
> > of the generic counter interface are not needed by the 104-QUAD-8. The
> > future quadrature counter interface would be more fitting for the
> > 104-QUAD-8.
> >
> > In addition, I may provide a dummy software counter driver in version 2
> > of this patchset to showcase and exemplify the functionality of the
> > generic counter interface more directly and aptly.
>
> That could be very useful. An alternative would be to look at a simple
> device (if we can find one) and implement a userspace fake for it
> (similar to what Guenter has done with lots of hwmon devices).
>
> That way we can play with a real driver against fake hardware and get
> the best of all possible worlds. I've been meaning to look at doing this
> for various IIO drivers for a while (most complete for i2c devices I think).
>
> As you might imagine I don't actually have that many parts (and most of them
> aren't connected to boards at any given time) so any form of emulation can be
> very helpful.
>
> I'll see if I can dig up any interesting devices beyond the ones we
> already know are integrated in various SoCs.

A quick search turned up a few parts that might do the job as additional
test parts.

lsi ls7366r - spi encoder to count chip.
http://www.lsisci.com - lots of parts made by them...

The broadcom (now - been various people) hctl-2032 - parallel output
but otherwise, does basic counting index etc.

https://www.tindie.com/products/Renbotics/tinyqed/
looks quite cute... Just does it using at little micro to do the hard work.
(long dead as a product by the look of it though..) Easy to emulate perhaps
but none of the links work..

Another thought would be to do a quick fpga version of whatever we
fancy playing with and use that. I can think of a few people to
ask if this would be useful (could do it myself, but low on time
for the next few months at least).

Jonathan

>
> I think we really need say 2+ devices to justify decisions in the
> core code. I did the original IIO version against 3ish devices but
> that was more varied (and that wasn't nearly enough with hindsight!)
>
> Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html