[v7,03/10] docs: Add Generic Counter interface documentation

Commit Message

William Breathitt Gray June 21, 2018, 9:07 p.m. UTC

This patch adds high-level documentation about the Generic Counter
interface.

Signed-off-by: William Breathitt Gray <[e-mail]>
---
 Documentation/driver-api/generic-counter.rst | 342 +++++++++++++++++++
 Documentation/driver-api/index.rst           |   1 +
 MAINTAINERS                                  |   1 +
 3 files changed, 344 insertions(+)
 create mode 100644 Documentation/driver-api/generic-counter.rst

Comments

Jonathan Cameron June 22, 2018, 4:51 p.m. UTC | #1

On Thu, 21 Jun 2018 17:07:30 -0400
William Breathitt Gray <[e-mail]> wrote:

> This patch adds high-level documentation about the Generic Counter
> interface.
> 
> Signed-off-by: William Breathitt Gray <[e-mail]>

Reviewed-by: Jonathan Cameron <[e-mail]>

> ---
>  Documentation/driver-api/generic-counter.rst | 342 +++++++++++++++++++
>  Documentation/driver-api/index.rst           |   1 +
>  MAINTAINERS                                  |   1 +
>  3 files changed, 344 insertions(+)
>  create mode 100644 Documentation/driver-api/generic-counter.rst
> 
> diff --git a/Documentation/driver-api/generic-counter.rst b/Documentation/driver-api/generic-counter.rst
> new file mode 100644
> index 000000000000..f51db893f595
> --- /dev/null
> +++ b/Documentation/driver-api/generic-counter.rst
> @@ -0,0 +1,342 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=========================
> +Generic Counter Interface
> +=========================
> +
> +Introduction
> +============
> +
> +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 driver API 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 tally counters, 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:
> +
> +* Count:
> +  Count data for a set of Signals.
> +
> +* Signal:
> +  Input data that is evaluated by the counter to determine the count
> +  data.
> +
> +* Synapse:
> +  The association of a Signal with a respective Count.
> +
> +COUNT
> +-----
> +A Count represents the count data for a set of Signals. The Generic
> +Counter interface provides the following available count data types:
> +
> +* COUNT_POSITION:
> +  Unsigned integer value representing position.
> +
> +A Count has a count function mode which represents the update behavior
> +for the count data. The Generic Counter interface provides the following
> +available count function modes:
> +
> +* Increase:
> +  Accumulated count is incremented.
> +
> +* Decrease:
> +  Accumulated count is decremented.
> +
> +* Pulse-Direction:
> +  Rising edges on signal A updates the respective count. The input level
> +  of signal B determines direction.
> +
> +* Quadrature:
> +  A pair of quadrature encoding signals are evaluated to determine
> +  position and direction. The following Quadrature modes are available:
> +
> +  - x1 A:
> +    If direction is forward, rising edges on quadrature pair signal A
> +    updates the respective count; if the direction is backward, falling
> +    edges on quadrature pair signal A updates the respective count.
> +    Quadrature encoding determines the direction.
> +
> +  - x1 B:
> +    If direction is forward, rising edges on quadrature pair signal B
> +    updates the respective count; if the direction is backward, falling
> +    edges on quadrature pair signal B updates the respective count.
> +    Quadrature encoding determines the direction.
> +
> +  - x2 A:
> +    Any state transition on quadrature pair signal A updates the
> +    respective count. Quadrature encoding determines the direction.
> +
> +  - x2 B:
> +    Any state transition on quadrature pair signal B updates the
> +    respective count. Quadrature encoding determines the direction.
> +
> +  - x4:
> +    Any state transition on either quadrature pair signals updates the
> +    respective count. Quadrature encoding determines the direction.
> +
> +A Count has a set of one or more associated Signals.
> +
> +SIGNAL
> +------
> +A Signal represents a counter input data; this is the input data that is
> +evaluated by the counter to determine the count data; e.g. a quadrature
> +signal output line of a rotary encoder. Not all counter devices provide
> +user access to the Signal data.
> +
> +The Generic Counter interface provides the following available signal
> +data types for when the Signal data is available for user access:
> +
> +* SIGNAL_LEVEL:
> +  Signal line state level. The following states are possible:
> +
> +  - SIGNAL_LEVEL_LOW:
> +    Signal line is in a low state.
> +
> +  - SIGNAL_LEVEL_HIGH:
> +    Signal line is in a high state.
> +
> +A Signal may be associated with one or more Counts.
> +
> +SYNAPSE
> +-------
> +A Synapse represents the association of a Signal with a respective
> +Count. Signal data affects respective Count data, and the Synapse
> +represents this relationship.
> +
> +The Synapse action mode specifies the Signal data condition which
> +triggers the respective Count's count function evaluation to update the
> +count data. The Generic Counter interface provides the following
> +available action modes:
> +
> +* None:
> +  Signal does not trigger the count function. In Pulse-Direction count
> +  function mode, this Signal is evaluated as Direction.
> +
> +* Rising Edge:
> +  Low state transitions to high state.
> +
> +* Falling Edge:
> +  High state transitions to low state.
> +
> +* Both Edges:
> +  Any state transition.
> +
> +A counter is defined as a set of input signals associated with 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 Counts
> +each associated with a set of Signals, whose respective Synapse
> +instances represent the count function update conditions for the
> +associated Counts.
> +
> +Paradigm
> +========
> +
> +The most basic counter device may be expressed as a single Count
> +associated with a single Signal via a single Synapse. Take for example
> +a counter device which simply accumulates a count of rising edges on a
> +source input line::
> +
> +                Count                Synapse        Signal
> +                -----                -------        ------
> +        +---------------------+
> +        | Data: Count         |    Rising Edge     ________
> +        | Function: Increase  |  <-------------   / Source \
> +        |                     |                  ____________
> +        +---------------------+
> +
> +In this example, the Signal is a source input line with a pulsing
> +voltage, while the Count is a persistent count value which is repeatedly
> +incremented. The Signal is associated with the respective Count via a
> +Synapse. The increase function is triggered by the Signal data condition
> +specified by the Synapse -- in this case a rising edge condition on the
> +voltage input line. In summary, the counter device existence and
> +behavior is aptly represented by respective Count, Signal, and Synapse
> +components: a rising edge condition triggers an increase function on an
> +accumulating count datum.
> +
> +A counter device is not limited to a single Signal; in fact, in theory
> +many Signals may be associated with even a single Count. For example, a
> +quadrature encoder counter device can keep track of position based on
> +the states of two input lines::
> +
> +                   Count                 Synapse     Signal
> +                   -----                 -------     ------
> +        +-------------------------+
> +        | Data: Position          |    Both Edges     ___
> +        | Function: Quadrature x4 |  <------------   / A \
> +        |                         |                 _______
> +        |                         |
> +        |                         |    Both Edges     ___
> +        |                         |  <------------   / B \
> +        |                         |                 _______
> +        +-------------------------+
> +
> +In this example, two Signals (quadrature encoder lines A and B) are
> +associated with a single Count: a rising or falling edge on either A or
> +B triggers the "Quadrature x4" function which determines the direction
> +of movement and updates the respective position data. The "Quadrature
> +x4" function is likely implemented in the hardware of the quadrature
> +encoder counter device; the Count, Signals, and Synapses simply
> +represent this hardware behavior and functionality.
> +
> +Signals associated with the same Count can have differing Synapse action
> +mode conditions. For example, a quadrature encoder counter device
> +operating in a non-quadrature Pulse-Direction mode could have one input
> +line dedicated for movement and a second input line dedicated for
> +direction::
> +
> +                   Count                   Synapse      Signal
> +                   -----                   -------      ------
> +        +---------------------------+
> +        | Data: Position            |    Rising Edge     ___
> +        | Function: Pulse-Direction |  <-------------   / A \ (Movement)
> +        |                           |                  _______
> +        |                           |
> +        |                           |       None         ___
> +        |                           |  <-------------   / B \ (Direction)
> +        |                           |                  _______
> +        +---------------------------+
> +
> +Only Signal A triggers the "Pulse-Direction" update function, but the
> +instantaneous state of Signal B is still required in order to know the
> +direction so that the position data may be properly updated. Ultimately,
> +both Signals are associated with the same Count via two respective
> +Synapses, but only one Synapse has an active action mode condition which
> +triggers the respective count function while the other is left with a
> +"None" condition action mode to indicate its respective Signal's
> +availability for state evaluation despite its non-triggering mode.
> +
> +Keep in mind that the Signal, Synapse, and Count are abstract
> +representations which do not need to be closely married to their
> +respective physical sources. This allows the user of a counter to
> +divorce themselves from the nuances of physical components (such as
> +whether an input line is differential or single-ended) and instead focus
> +on the core idea of what the data and process represent (e.g. position
> +as interpreted from quadrature encoding data).
> +
> +Userspace Interface
> +===================
> +
> +Several sysfs attributes are generated by the Generic Counter interface,
> +and reside under the /sys/bus/counter/devices/counterX directory, where
> +counterX refers to the respective counter device. Please see
> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
> +information on each Generic Counter interface sysfs attribute.
> +
> +Through these sysfs attributes, programs and scripts may interact with
> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
> +counter devices.
> +
> +Driver API
> +==========
> +
> +Driver authors may utilize the Generic Counter interface in their code
> +by including the include/linux/counter.h header file. This header file
> +provides several core data structures, function prototypes, and macros
> +for defining a counter device.
> +
> +.. kernel-doc:: include/linux/counter.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/counter/generic-counter.c
> +   :export:
> +
> +Implementation
> +==============
> +
> +To support a counter device, a driver must first allocate the available
> +Counter Signals via counter_signal structures. These Signals should
> +be stored as an array and set to the signals array member of an
> +allocated counter_device structure before the Counter is registered to
> +the system.
> +
> +Counter Counts may be allocated via counter_count structures, and
> +respective Counter Signal associations (Synapses) made via
> +counter_synapse structures. Associated counter_synapse structures are
> +stored as an array and set to the the synapses array member of the
> +respective counter_count structure. These counter_count structures are
> +set to the counts array member of an allocated counter_device structure
> +before the Counter is registered to the system.
> +
> +Driver callbacks should be provided to the counter_device structure via
> +a constant counter_ops structure in order to communicate with the
> +device: to read and write various Signals and Counts, and to set and get
> +the "action mode" and "function mode" for various Synapses and Counts
> +respectively.
> +
> +A defined counter_device structure may be registered to the system by
> +passing it to the counter_register function, and unregistered by passing
> +it to the counter_unregister function. Similarly, the
> +devm_counter_register and devm_counter_unregister functions may be used
> +if device memory-managed registration is desired.
> +
> +Extension sysfs attributes can be created for auxiliary functionality
> +and data by passing in defined counter_device_ext, counter_count_ext,
> +and counter_signal_ext structures. In these cases, the
> +counter_device_ext structure is used for global configuration of the
> +respective Counter device, while the counter_count_ext and
> +counter_signal_ext structures allow for auxiliary exposure and
> +configuration of a specific Count or Signal respectively.
> +
> +Architecture
> +============
> +
> +When the Generic Counter interface counter module is loaded, the
> +counter_init function is called which registers a bus_type named
> +"counter" to the system. Subsequently, when the module is unloaded, the
> +counter_exit function is called which unregisters the bus_type named
> +"counter" from the system.
> +
> +Counter devices are registered to the system via the counter_register
> +function, and later removed via the counter_unregister function. The
> +counter_register function establishes a unique ID for the Counter
> +device and creates a respective sysfs directory, where X is the
> +mentioned unique ID:
> +
> +    /sys/bus/counter/devices/counterX
> +
> +Sysfs attributes are created within the counterX directory to expose
> +functionality, configurations, and data relating to the Counts, Signals,
> +and Synapses of the Counter device, as well as options and information
> +for the Counter device itself.
> +
> +Each Signal has a directory created to house its relevant sysfs
> +attributes, where Y is the unique ID of the respective Signal:
> +
> +    /sys/bus/counter/devices/counterX/signalY
> +
> +Similarly, each Count has a directory created to house its relevant
> +sysfs attributes, where Y is the unique ID of the respective Count:
> +
> +    /sys/bus/counter/devices/counterX/countY
> +
> +For a more detailed breakdown of the available Generic Counter interface
> +sysfs attributes, please refer to the
> +Documentation/ABI/testing/sys-bus-counter file.
> +
> +The Signals and Counts associated with the Counter device are registered
> +to the system as well by the counter_register function. The
> +signal_read/signal_write driver callbacks are associated with their
> +respective Signal attributes, while the count_read/count_write and
> +function_get/function_set driver callbacks are associated with their
> +respective Count attributes; similarly, the same is true for the
> +action_get/action_set driver callbacks and their respective Synapse
> +attributes. If a driver callback is left undefined, then the respective
> +read/write permission is left disabled for the relevant attributes.
> +
> +Similarly, extension sysfs attributes are created for the defined
> +counter_device_ext, counter_count_ext, and counter_signal_ext
> +structures that are passed in.
> diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
> index f4180e7c7ed5..e39a9fd3d8c9 100644
> --- a/Documentation/driver-api/index.rst
> +++ b/Documentation/driver-api/index.rst
> @@ -52,6 +52,7 @@ available subsections can be seen below.
>     slimbus
>     soundwire/index
>     fpga/index
> +   generic-counter
>  
>  .. only::  subproject and html
>  
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f8a47fd197a1..c7fd36500635 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3693,6 +3693,7 @@ M:	William Breathitt Gray <[e-mail]>
>  L:	[e-mail]
>  S:	Maintained
>  F:	Documentation/ABI/testing/sysfs-bus-counter*
> +F:	Documentation/driver-api/generic-counter.rst
>  F:	drivers/counter/
>  F:	include/linux/counter.h
>

David Lechner July 2, 2018, 7:37 p.m. UTC | #2

On 06/21/2018 04:07 PM, William Breathitt Gray wrote:
> +Userspace Interface
> +===================
> +
> +Several sysfs attributes are generated by the Generic Counter interface,
> +and reside under the /sys/bus/counter/devices/counterX directory, where
> +counterX refers to the respective counter device. Please see
> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
> +information on each Generic Counter interface sysfs attribute.
> +
> +Through these sysfs attributes, programs and scripts may interact with
> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
> +counter devices.
> +

Have you considered a character device in addition to the sysfs interface?

I basically have many of the same concerns that resulted in a char dev for
gpio[1].

- With sysfs, you *can* technically poll for events, but then you have to
   seek and read or re-open the file.
- File permissions are annoying if you want a non root user to be able to
   use the device.
- A single program can't claim exclusive access to a device.
- There is no automatic cleanup if a userspace program accessing the device
    crashes.

[1]: https://www.elinux.org/images/7/74/Elce2017_new_GPIO_interface.pdf

David Lechner July 2, 2018, 7:42 p.m. UTC | #3

On 06/21/2018 04:07 PM, William Breathitt Gray wrote:
> +Userspace Interface
> +===================
> +
> +Several sysfs attributes are generated by the Generic Counter interface,
> +and reside under the /sys/bus/counter/devices/counterX directory, where
> +counterX refers to the respective counter device. Please see
> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
> +information on each Generic Counter interface sysfs attribute.
> +
> +Through these sysfs attributes, programs and scripts may interact with
> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
> +counter devices.

I don't actually see any mention of the Synapses in the sysfs documentation.
But configfs my be better suited to configuring Synapses anyway, e.g. one
would symlink a Signal to a Counter to make the connection.

William Breathitt Gray July 3, 2018, 2:16 p.m. UTC | #4

On Mon, Jul 02, 2018 at 02:37:53PM -0500, David Lechner wrote:
>On 06/21/2018 04:07 PM, William Breathitt Gray wrote:
>> +Userspace Interface
>> +===================
>> +
>> +Several sysfs attributes are generated by the Generic Counter interface,
>> +and reside under the /sys/bus/counter/devices/counterX directory, where
>> +counterX refers to the respective counter device. Please see
>> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
>> +information on each Generic Counter interface sysfs attribute.
>> +
>> +Through these sysfs attributes, programs and scripts may interact with
>> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
>> +counter devices.
>> +
>
>Have you considered a character device in addition to the sysfs interface?
>
>I basically have many of the same concerns that resulted in a char dev for
>gpio[1].
>
>- With sysfs, you *can* technically poll for events, but then you have to
>   seek and read or re-open the file.
>- File permissions are annoying if you want a non root user to be able to
>   use the device.
>- A single program can't claim exclusive access to a device.
>- There is no automatic cleanup if a userspace program accessing the device
>    crashes.
>
>[1]: https://www.elinux.org/images/7/74/Elce2017_new_GPIO_interface.pdf

Those look like good technical reasons for implementing a character
device for the Generic Counter interface. I chose to implement the sysfs
interface because I was using the IIO code as a reference, but I
personally don't have much opposition to a character device in addition.

I'd like to get Jonathan's opinion on this as well if possible -- I
vaguely recall us considering this option briefly last year when the
Generic Counter interface was still in its beginnings. I've CC'd Linus
Walleij as well for input as the GPIO maintainer.

William Breathitt Gray

William Breathitt Gray July 3, 2018, 2:21 p.m. UTC | #5

On Mon, Jul 02, 2018 at 02:42:06PM -0500, David Lechner wrote:
>On 06/21/2018 04:07 PM, William Breathitt Gray wrote:
>> +Userspace Interface
>> +===================
>> +
>> +Several sysfs attributes are generated by the Generic Counter interface,
>> +and reside under the /sys/bus/counter/devices/counterX directory, where
>> +counterX refers to the respective counter device. Please see
>> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
>> +information on each Generic Counter interface sysfs attribute.
>> +
>> +Through these sysfs attributes, programs and scripts may interact with
>> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
>> +counter devices.
>
>I don't actually see any mention of the Synapses in the sysfs documentation.
>But configfs my be better suited to configuring Synapses anyway, e.g. one
>would symlink a Signal to a Counter to make the connection.

I expect in a future patch to add a synapses directory to the count
directories which will have symlinks to the respective associated
signals. I've postponed this development for now to keep the
introduction patchset simple, and also because the current users have
little use for this feature at the moment, but it is indeed planned for
the future.

William Breathitt Gray

Linus Walleij July 4, 2018, 5:23 p.m. UTC | #6

On Tue, Jul 3, 2018 at 4:16 PM William Breathitt Gray
<[e-mail]> wrote:
> On Mon, Jul 02, 2018 at 02:37:53PM -0500, David Lechner wrote:
> >On 06/21/2018 04:07 PM, William Breathitt Gray wrote:
> >> +Userspace Interface
> >> +===================
> >> +
> >> +Several sysfs attributes are generated by the Generic Counter interface,
> >> +and reside under the /sys/bus/counter/devices/counterX directory, where
> >> +counterX refers to the respective counter device. Please see
> >> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
> >> +information on each Generic Counter interface sysfs attribute.
> >> +
> >> +Through these sysfs attributes, programs and scripts may interact with
> >> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
> >> +counter devices.
> >> +
> >
> >Have you considered a character device in addition to the sysfs interface?
> >
> >I basically have many of the same concerns that resulted in a char dev for
> >gpio[1].
> >
> >- With sysfs, you *can* technically poll for events, but then you have to
> >   seek and read or re-open the file.
> >- File permissions are annoying if you want a non root user to be able to
> >   use the device.
> >- A single program can't claim exclusive access to a device.
> >- There is no automatic cleanup if a userspace program accessing the device
> >    crashes.
> >
> >[1]: https://www.elinux.org/images/7/74/Elce2017_new_GPIO_interface.pdf
>
> Those look like good technical reasons for implementing a character
> device for the Generic Counter interface. I chose to implement the sysfs
> interface because I was using the IIO code as a reference, but I
> personally don't have much opposition to a character device in addition.

IIO is also using a character device for outputting events and sensor
data. In IIO sysfs is only used for configuring what events and
data should come out of the character device.

> I'd like to get Jonathan's opinion on this as well if possible -- I
> vaguely recall us considering this option briefly last year when the
> Generic Counter interface was still in its beginnings. I've CC'd Linus
> Walleij as well for input as the GPIO maintainer.

The GPIO character device was based on the IIO character device.

We have one TODO item: to merge the timestamping format
between GPIO and IIO and use the same sysfs file for configuring
the time base.

Yours,
Linus Walleij

Jonathan Cameron July 6, 2018, 5:15 p.m. UTC | #7

On Wed, 4 Jul 2018 19:23:26 +0200
Linus Walleij <[e-mail]> wrote:

> On Tue, Jul 3, 2018 at 4:16 PM William Breathitt Gray
> <[e-mail]> wrote:
> > On Mon, Jul 02, 2018 at 02:37:53PM -0500, David Lechner wrote:  
> > >On 06/21/2018 04:07 PM, William Breathitt Gray wrote:  
> > >> +Userspace Interface
> > >> +===================
> > >> +
> > >> +Several sysfs attributes are generated by the Generic Counter interface,
> > >> +and reside under the /sys/bus/counter/devices/counterX directory, where
> > >> +counterX refers to the respective counter device. Please see
> > >> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
> > >> +information on each Generic Counter interface sysfs attribute.
> > >> +
> > >> +Through these sysfs attributes, programs and scripts may interact with
> > >> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
> > >> +counter devices.
> > >> +  
> > >
> > >Have you considered a character device in addition to the sysfs interface?
> > >
> > >I basically have many of the same concerns that resulted in a char dev for
> > >gpio[1].
> > >
> > >- With sysfs, you *can* technically poll for events, but then you have to
> > >   seek and read or re-open the file.

For this to be relevant we need some type of self clocking sampling of a counter,
so far this hasn't really been true for William's devices - they tend to have
internal monitoring and you just 'ask' them when you want to know the rotation.
Sure if we have 'events' such as soft limit switches in the hardware, then
we'd want some sort of event chrdev (personally I think these should be separate
from the main data flow - but that's a detail).

> > >- File permissions are annoying if you want a non root user to be able to
> > >   use the device.

They aren't a whole lot different for a chrdev.  In both cases you can allow
a non root user write access if you want to.

> > >- A single program can't claim exclusive access to a device.

Agreed.  Though that only matters for control, why do you care if someone
else can read.  In IIO we get around this by 'generally' blocking settings
changes that will a process that is sampling data via the chrdev.
It's not a hard and fast rule though.  I really don't like configuration
via chrdevs as for complex devices you end up with a non self describing
interface with a lot of complexity.

The exceptions are things like the media controller frameworks, but they
are very very heavyweight for an simple devices like counters.

> > >- There is no automatic cleanup if a userspace program accessing the device
> > >    crashes.

For these devices, the question is what sort of cleanup makes sense?

Often they are freerunning so the most you could do is power down if you knew
no one cared, but for an encoder you may well want to continue tracking even
if no one is looking right now.

I can think of reasons you 'might' want to tidy up, but we'd need real
driver code to show the necessity of this one.

> > >
> > >[1]: https://www.elinux.org/images/7/74/Elce2017_new_GPIO_interface.pdf  
> >
> > Those look like good technical reasons for implementing a character
> > device for the Generic Counter interface. I chose to implement the sysfs
> > interface because I was using the IIO code as a reference, but I
> > personally don't have much opposition to a character device in addition.  
> 
> IIO is also using a character device for outputting events and sensor
> data. In IIO sysfs is only used for configuring what events and
> data should come out of the character device.

Yes, with the addition that we typically provide data readback as well.
For some simple devices which are slow and are actually polled to get
a reading, there is not a lot of point in implementing the chrdev route
so in IIO it is optional.
> 
> > I'd like to get Jonathan's opinion on this as well if possible -- I
> > vaguely recall us considering this option briefly last year when the
> > Generic Counter interface was still in its beginnings. I've CC'd Linus
> > Walleij as well for input as the GPIO maintainer.  

I'm not against it, but I do want to see use cases that are not
satisfied by sysfs first.

So far we've no seen them but sounds like you might have one David!

Jonathan

> 
> The GPIO character device was based on the IIO character device.
> 
> We have one TODO item: to merge the timestamping format
> between GPIO and IIO and use the same sysfs file for configuring
> the time base.
> 
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [e-mail]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

David Lechner July 6, 2018, 6:25 p.m. UTC | #8

On 07/06/2018 12:15 PM, Jonathan Cameron wrote:
> On Wed, 4 Jul 2018 19:23:26 +0200
> Linus Walleij <[e-mail]> wrote:
> 
>> On Tue, Jul 3, 2018 at 4:16 PM William Breathitt Gray
>> <[e-mail]> wrote:
>>> On Mon, Jul 02, 2018 at 02:37:53PM -0500, David Lechner wrote:
>>>> On 06/21/2018 04:07 PM, William Breathitt Gray wrote:
>>>>> +Userspace Interface
>>>>> +===================
>>>>> +
>>>>> +Several sysfs attributes are generated by the Generic Counter interface,
>>>>> +and reside under the /sys/bus/counter/devices/counterX directory, where
>>>>> +counterX refers to the respective counter device. Please see
>>>>> +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
>>>>> +information on each Generic Counter interface sysfs attribute.
>>>>> +
>>>>> +Through these sysfs attributes, programs and scripts may interact with
>>>>> +the Generic Counter paradigm Counts, Signals, and Synapses of respective
>>>>> +counter devices.
>>>>> +
>>>>
>>>> Have you considered a character device in addition to the sysfs interface?
>>>>
>>>> I basically have many of the same concerns that resulted in a char dev for
>>>> gpio[1].
>>>>
>>>> - With sysfs, you *can* technically poll for events, but then you have to
>>>>    seek and read or re-open the file.
> 
> For this to be relevant we need some type of self clocking sampling of a counter,
> so far this hasn't really been true for William's devices - they tend to have
> internal monitoring and you just 'ask' them when you want to know the rotation.
> Sure if we have 'events' such as soft limit switches in the hardware, then
> we'd want some sort of event chrdev (personally I think these should be separate
> from the main data flow - but that's a detail).
> 
>>>> - File permissions are annoying if you want a non root user to be able to
>>>>    use the device.
> 
> They aren't a whole lot different for a chrdev.  In both cases you can allow
> a non root user write access if you want to.
> 
>>>> - A single program can't claim exclusive access to a device.
> 
> Agreed.  Though that only matters for control, why do you care if someone
> else can read.  In IIO we get around this by 'generally' blocking settings
> changes that will a process that is sampling data via the chrdev.
> It's not a hard and fast rule though.  I really don't like configuration
> via chrdevs as for complex devices you end up with a non self describing
> interface with a lot of complexity.
> 
> The exceptions are things like the media controller frameworks, but they
> are very very heavyweight for an simple devices like counters.
> 
>>>> - There is no automatic cleanup if a userspace program accessing the device
>>>>     crashes.
> 
> For these devices, the question is what sort of cleanup makes sense?
> 
> Often they are freerunning so the most you could do is power down if you knew
> no one cared, but for an encoder you may well want to continue tracking even
> if no one is looking right now.
> 
> I can think of reasons you 'might' want to tidy up, but we'd need real
> driver code to show the necessity of this one.
> 
>>>>
>>>> [1]: https://www.elinux.org/images/7/74/Elce2017_new_GPIO_interface.pdf
>>>
>>> Those look like good technical reasons for implementing a character
>>> device for the Generic Counter interface. I chose to implement the sysfs
>>> interface because I was using the IIO code as a reference, but I
>>> personally don't have much opposition to a character device in addition.
>>
>> IIO is also using a character device for outputting events and sensor
>> data. In IIO sysfs is only used for configuring what events and
>> data should come out of the character device.
> 
> Yes, with the addition that we typically provide data readback as well.
> For some simple devices which are slow and are actually polled to get
> a reading, there is not a lot of point in implementing the chrdev route
> so in IIO it is optional.
>>
>>> I'd like to get Jonathan's opinion on this as well if possible -- I
>>> vaguely recall us considering this option briefly last year when the
>>> Generic Counter interface was still in its beginnings. I've CC'd Linus
>>> Walleij as well for input as the GPIO maintainer.
> 
> I'm not against it, but I do want to see use cases that are not
> satisfied by sysfs first.
> 
> So far we've no seen them but sounds like you might have one David!
> 

Basically, we are implementing a counter in the PRU on TI Sitara, so
we can make it do just about whatever we want. Although, I'm trying to
keep it similar to the eQEP.

Patch

diff --git a/Documentation/driver-api/generic-counter.rst b/Documentation/driver-api/generic-counter.rst
new file mode 100644
index 000000000000..f51db893f595
--- /dev/null
+++ b/Documentation/driver-api/generic-counter.rst
@@ -0,0 +1,342 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+=========================
+Generic Counter Interface
+=========================
+
+Introduction
+============
+
+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 driver API 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 tally counters, 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:
+
+* Count:
+  Count data for a set of Signals.
+
+* Signal:
+  Input data that is evaluated by the counter to determine the count
+  data.
+
+* Synapse:
+  The association of a Signal with a respective Count.
+
+COUNT
+-----
+A Count represents the count data for a set of Signals. The Generic
+Counter interface provides the following available count data types:
+
+* COUNT_POSITION:
+  Unsigned integer value representing position.
+
+A Count has a count function mode which represents the update behavior
+for the count data. The Generic Counter interface provides the following
+available count function modes:
+
+* Increase:
+  Accumulated count is incremented.
+
+* Decrease:
+  Accumulated count is decremented.
+
+* Pulse-Direction:
+  Rising edges on signal A updates the respective count. The input level
+  of signal B determines direction.
+
+* Quadrature:
+  A pair of quadrature encoding signals are evaluated to determine
+  position and direction. The following Quadrature modes are available:
+
+  - x1 A:
+    If direction is forward, rising edges on quadrature pair signal A
+    updates the respective count; if the direction is backward, falling
+    edges on quadrature pair signal A updates the respective count.
+    Quadrature encoding determines the direction.
+
+  - x1 B:
+    If direction is forward, rising edges on quadrature pair signal B
+    updates the respective count; if the direction is backward, falling
+    edges on quadrature pair signal B updates the respective count.
+    Quadrature encoding determines the direction.
+
+  - x2 A:
+    Any state transition on quadrature pair signal A updates the
+    respective count. Quadrature encoding determines the direction.
+
+  - x2 B:
+    Any state transition on quadrature pair signal B updates the
+    respective count. Quadrature encoding determines the direction.
+
+  - x4:
+    Any state transition on either quadrature pair signals updates the
+    respective count. Quadrature encoding determines the direction.
+
+A Count has a set of one or more associated Signals.
+
+SIGNAL
+------
+A Signal represents a counter input data; this is the input data that is
+evaluated by the counter to determine the count data; e.g. a quadrature
+signal output line of a rotary encoder. Not all counter devices provide
+user access to the Signal data.
+
+The Generic Counter interface provides the following available signal
+data types for when the Signal data is available for user access:
+
+* SIGNAL_LEVEL:
+  Signal line state level. The following states are possible:
+
+  - SIGNAL_LEVEL_LOW:
+    Signal line is in a low state.
+
+  - SIGNAL_LEVEL_HIGH:
+    Signal line is in a high state.
+
+A Signal may be associated with one or more Counts.
+
+SYNAPSE
+-------
+A Synapse represents the association of a Signal with a respective
+Count. Signal data affects respective Count data, and the Synapse
+represents this relationship.
+
+The Synapse action mode specifies the Signal data condition which
+triggers the respective Count's count function evaluation to update the
+count data. The Generic Counter interface provides the following
+available action modes:
+
+* None:
+  Signal does not trigger the count function. In Pulse-Direction count
+  function mode, this Signal is evaluated as Direction.
+
+* Rising Edge:
+  Low state transitions to high state.
+
+* Falling Edge:
+  High state transitions to low state.
+
+* Both Edges:
+  Any state transition.
+
+A counter is defined as a set of input signals associated with 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 Counts
+each associated with a set of Signals, whose respective Synapse
+instances represent the count function update conditions for the
+associated Counts.
+
+Paradigm
+========
+
+The most basic counter device may be expressed as a single Count
+associated with a single Signal via a single Synapse. Take for example
+a counter device which simply accumulates a count of rising edges on a
+source input line::
+
+                Count                Synapse        Signal
+                -----                -------        ------
+        +---------------------+
+        | Data: Count         |    Rising Edge     ________
+        | Function: Increase  |  <-------------   / Source \
+        |                     |                  ____________
+        +---------------------+
+
+In this example, the Signal is a source input line with a pulsing
+voltage, while the Count is a persistent count value which is repeatedly
+incremented. The Signal is associated with the respective Count via a
+Synapse. The increase function is triggered by the Signal data condition
+specified by the Synapse -- in this case a rising edge condition on the
+voltage input line. In summary, the counter device existence and
+behavior is aptly represented by respective Count, Signal, and Synapse
+components: a rising edge condition triggers an increase function on an
+accumulating count datum.
+
+A counter device is not limited to a single Signal; in fact, in theory
+many Signals may be associated with even a single Count. For example, a
+quadrature encoder counter device can keep track of position based on
+the states of two input lines::
+
+                   Count                 Synapse     Signal
+                   -----                 -------     ------
+        +-------------------------+
+        | Data: Position          |    Both Edges     ___
+        | Function: Quadrature x4 |  <------------   / A \
+        |                         |                 _______
+        |                         |
+        |                         |    Both Edges     ___
+        |                         |  <------------   / B \
+        |                         |                 _______
+        +-------------------------+
+
+In this example, two Signals (quadrature encoder lines A and B) are
+associated with a single Count: a rising or falling edge on either A or
+B triggers the "Quadrature x4" function which determines the direction
+of movement and updates the respective position data. The "Quadrature
+x4" function is likely implemented in the hardware of the quadrature
+encoder counter device; the Count, Signals, and Synapses simply
+represent this hardware behavior and functionality.
+
+Signals associated with the same Count can have differing Synapse action
+mode conditions. For example, a quadrature encoder counter device
+operating in a non-quadrature Pulse-Direction mode could have one input
+line dedicated for movement and a second input line dedicated for
+direction::
+
+                   Count                   Synapse      Signal
+                   -----                   -------      ------
+        +---------------------------+
+        | Data: Position            |    Rising Edge     ___
+        | Function: Pulse-Direction |  <-------------   / A \ (Movement)
+        |                           |                  _______
+        |                           |
+        |                           |       None         ___
+        |                           |  <-------------   / B \ (Direction)
+        |                           |                  _______
+        +---------------------------+
+
+Only Signal A triggers the "Pulse-Direction" update function, but the
+instantaneous state of Signal B is still required in order to know the
+direction so that the position data may be properly updated. Ultimately,
+both Signals are associated with the same Count via two respective
+Synapses, but only one Synapse has an active action mode condition which
+triggers the respective count function while the other is left with a
+"None" condition action mode to indicate its respective Signal's
+availability for state evaluation despite its non-triggering mode.
+
+Keep in mind that the Signal, Synapse, and Count are abstract
+representations which do not need to be closely married to their
+respective physical sources. This allows the user of a counter to
+divorce themselves from the nuances of physical components (such as
+whether an input line is differential or single-ended) and instead focus
+on the core idea of what the data and process represent (e.g. position
+as interpreted from quadrature encoding data).
+
+Userspace Interface
+===================
+
+Several sysfs attributes are generated by the Generic Counter interface,
+and reside under the /sys/bus/counter/devices/counterX directory, where
+counterX refers to the respective counter device. Please see
+Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed
+information on each Generic Counter interface sysfs attribute.
+
+Through these sysfs attributes, programs and scripts may interact with
+the Generic Counter paradigm Counts, Signals, and Synapses of respective
+counter devices.
+
+Driver API
+==========
+
+Driver authors may utilize the Generic Counter interface in their code
+by including the include/linux/counter.h header file. This header file
+provides several core data structures, function prototypes, and macros
+for defining a counter device.
+
+.. kernel-doc:: include/linux/counter.h
+   :internal:
+
+.. kernel-doc:: drivers/counter/generic-counter.c
+   :export:
+
+Implementation
+==============
+
+To support a counter device, a driver must first allocate the available
+Counter Signals via counter_signal structures. These Signals should
+be stored as an array and set to the signals array member of an
+allocated counter_device structure before the Counter is registered to
+the system.
+
+Counter Counts may be allocated via counter_count structures, and
+respective Counter Signal associations (Synapses) made via
+counter_synapse structures. Associated counter_synapse structures are
+stored as an array and set to the the synapses array member of the
+respective counter_count structure. These counter_count structures are
+set to the counts array member of an allocated counter_device structure
+before the Counter is registered to the system.
+
+Driver callbacks should be provided to the counter_device structure via
+a constant counter_ops structure in order to communicate with the
+device: to read and write various Signals and Counts, and to set and get
+the "action mode" and "function mode" for various Synapses and Counts
+respectively.
+
+A defined counter_device structure may be registered to the system by
+passing it to the counter_register function, and unregistered by passing
+it to the counter_unregister function. Similarly, the
+devm_counter_register and devm_counter_unregister functions may be used
+if device memory-managed registration is desired.
+
+Extension sysfs attributes can be created for auxiliary functionality
+and data by passing in defined counter_device_ext, counter_count_ext,
+and counter_signal_ext structures. In these cases, the
+counter_device_ext structure is used for global configuration of the
+respective Counter device, while the counter_count_ext and
+counter_signal_ext structures allow for auxiliary exposure and
+configuration of a specific Count or Signal respectively.
+
+Architecture
+============
+
+When the Generic Counter interface counter module is loaded, the
+counter_init function is called which registers a bus_type named
+"counter" to the system. Subsequently, when the module is unloaded, the
+counter_exit function is called which unregisters the bus_type named
+"counter" from the system.
+
+Counter devices are registered to the system via the counter_register
+function, and later removed via the counter_unregister function. The
+counter_register function establishes a unique ID for the Counter
+device and creates a respective sysfs directory, where X is the
+mentioned unique ID:
+
+    /sys/bus/counter/devices/counterX
+
+Sysfs attributes are created within the counterX directory to expose
+functionality, configurations, and data relating to the Counts, Signals,
+and Synapses of the Counter device, as well as options and information
+for the Counter device itself.
+
+Each Signal has a directory created to house its relevant sysfs
+attributes, where Y is the unique ID of the respective Signal:
+
+    /sys/bus/counter/devices/counterX/signalY
+
+Similarly, each Count has a directory created to house its relevant
+sysfs attributes, where Y is the unique ID of the respective Count:
+
+    /sys/bus/counter/devices/counterX/countY
+
+For a more detailed breakdown of the available Generic Counter interface
+sysfs attributes, please refer to the
+Documentation/ABI/testing/sys-bus-counter file.
+
+The Signals and Counts associated with the Counter device are registered
+to the system as well by the counter_register function. The
+signal_read/signal_write driver callbacks are associated with their
+respective Signal attributes, while the count_read/count_write and
+function_get/function_set driver callbacks are associated with their
+respective Count attributes; similarly, the same is true for the
+action_get/action_set driver callbacks and their respective Synapse
+attributes. If a driver callback is left undefined, then the respective
+read/write permission is left disabled for the relevant attributes.
+
+Similarly, extension sysfs attributes are created for the defined
+counter_device_ext, counter_count_ext, and counter_signal_ext
+structures that are passed in.
diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index f4180e7c7ed5..e39a9fd3d8c9 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -52,6 +52,7 @@  available subsections can be seen below.
    slimbus
    soundwire/index
    fpga/index
+   generic-counter
 
 .. only::  subproject and html
 
diff --git a/MAINTAINERS b/MAINTAINERS
index f8a47fd197a1..c7fd36500635 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3693,6 +3693,7 @@  M:	William Breathitt Gray <[e-mail]>
 L:	[e-mail]
 S:	Maintained
 F:	Documentation/ABI/testing/sysfs-bus-counter*
+F:	Documentation/driver-api/generic-counter.rst
 F:	drivers/counter/
 F:	include/linux/counter.h