Message-Id: <cover.1501516678.git.vilhelm.gray@gmail.com>
Date:   Mon, 31 Jul 2017 12:02:45 -0400
From:   William Breathitt Gray <vilhelm.gray@...il.com>
To:     jic23@...nel.org, knaack.h@....de, lars@...afoo.de,
        pmeerw@...erw.net, benjamin.gaignard@...aro.org
Cc:     linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
        William Breathitt Gray <vilhelm.gray@...il.com>
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

Powered by blists - more mailing lists