Introduce the Counter subsystem

This patch introduces the Generic Counter interface for supporting counter devices.

In the context of the Generic Counter interface, a counter is defined as a device that reports one or more "counts" based on the state changes of one or more "signals" as evaluated by a defined "count function."

Driver callbacks should be provided to communicate with the device: to read and write various Signals and Counts, and to set and get the "action mode" and "count function" for various Synapses and Counts respectively.

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.

A counter device is registered to the system by passing the respective initialized counter_device structure to the counter_register function; similarly, the counter_unregister function unregisters the respective Counter. The devm_counter_register and devm_counter_unregister functions serve as device memory-managed versions of the counter_register and counter_unregister functions respectively.

Reviewed-by Jonathan Cameron <[email protected]>

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

---
 MAINTAINERS                       |    7 +
 drivers/Kconfig                   |    2 +
 drivers/Makefile                  |    1 +
 drivers/counter/Kconfig           |   18 +
 drivers/counter/Makefile          |    8 +
 drivers/counter/generic-counter.c | 1519 +++++++++++++++++++++++++++++
 include/linux/counter.h           |  534 ++++++++++
 7 files changed, 2089 insertions(+)
 create mode 100644 drivers/counter/Kconfig
 create mode 100644 drivers/counter/Makefile
 create mode 100644 drivers/counter/generic-counter.c
 create mode 100644 include/linux/counter.h

drivers/counter/generic-counter.c new file +8 -0

+ */
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/fs.h>
+#include <linux/gfp.h>
+#include <linux/idr.h>
+#include <linux/init.h>

drivers/counter/generic-counter.c new file +17 -0

+
+	index = __sysfs_match_string(e->items, e->num_items, buf);
+	if (index < 0)
+		return index;
+
+	err = e->set(counter, signal, index);
+	if (err)
+		return err;
+
+	return len;
+}
+EXPORT_SYMBOL(counter_signal_enum_write);
+
+ssize_t counter_signal_enum_available_read(struct counter_device *counter,
+					   struct counter_signal *signal,
+					   void *priv, char *buf)
+{

drivers/counter/generic-counter.c new file +5 -0

+
+	return len;
+}
+EXPORT_SYMBOL(counter_signal_enum_available_read);
+

drivers/counter/generic-counter.c new file +25 -0

+{
+	struct counter_device_attr *counter_attr;
+	struct device_attribute *dev_attr;
+	int err;
+	struct list_head *const attr_list = &group->attr_list;
+
+	/* Allocate a Counter device attribute */
+	counter_attr = kzalloc(sizeof(*counter_attr), GFP_KERNEL);
+	if (!counter_attr)
+		return -ENOMEM;
+	dev_attr = &counter_attr->dev_attr;
+
+	sysfs_attr_init(&dev_attr->attr);
+
+	/* Configure device attribute */
+	dev_attr->attr.name = kasprintf(GFP_KERNEL, "%s%s", prefix, name);
+	if (!dev_attr->attr.name) {
+		err = -ENOMEM;
+		goto err_free_counter_attr;
+	}
+	if (show) {
+		dev_attr->attr.mode |= 0444;
+		dev_attr->show = show;
+	}
+	if (store) {

drivers/counter/generic-counter.c new file +11 -0

+	struct counter_signal	*signal;
+};
+
+static ssize_t counter_signal_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct counter_device *const counter = dev_get_drvdata(dev);
+	const struct counter_device_attr *const devattr = to_counter_attr(attr);
+	const struct signal_comp_t *const component = devattr->component;
+	struct counter_signal *const signal = component->signal;
+	int err;

Quoted 35 lines · William Breathitt Gray

This patch introduces the Generic Counter interface for supporting counter devices.

In the context of the Generic Counter interface, a counter is defined as a device that reports one or more "counts" based on the state changes of one or more "signals" as evaluated by a defined "count function."

Driver callbacks should be provided to communicate with the device: to read and write various Signals and Counts, and to set and get the "action mode" and "count function" for various Synapses and Counts respectively.

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.

A counter device is registered to the system by passing the respective initialized counter_device structure to the counter_register function; similarly, the counter_unregister function unregisters the respective Counter. The devm_counter_register and devm_counter_unregister functions serve as device memory-managed versions of the counter_register and counter_unregister functions respectively.

Reviewed-by Jonathan Cameron <[email protected]>

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

Quoted 13 lines

---
 MAINTAINERS                       |    7 +
 drivers/Kconfig                   |    2 +
 drivers/Makefile                  |    1 +
 drivers/counter/Kconfig           |   18 +
 drivers/counter/Makefile          |    8 +
 drivers/counter/generic-counter.c | 1519 +++++++++++++++++++++++++++++
 include/linux/counter.h           |  534 ++++++++++
 7 files changed, 2089 insertions(+)
 create mode 100644 drivers/counter/Kconfig
 create mode 100644 drivers/counter/Makefile
 create mode 100644 drivers/counter/generic-counter.c
 create mode 100644 include/linux/counter.h

First cut of review, does counter.h really have to be that big? It feels like some of the "internal" functions and structures could be local to drivers/counter/ right?

Quoted 10 lines

drivers/counter/Kconfig new file +10 -0

+menuconfig COUNTER
+	tristate "Counter support"
+	help
+	  Provides Generic Counter interface support for counter devices.
+
+	  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 providing a generic counter interface for

drivers/counter/Kconfig line 18

Quoted 3 lines

drivers/counter/Kconfig new file +3 -0

+	  consumption. The Generic Counter interface enables drivers to support
+	  and expose a common set of components and functionality present in
+	  counter devices.

No need to explain an "API" in a Kconfig help text, which is for a user of the kernel, not for someone writing kernel code. Odds are individual drivers will need to select this, or are you going to depend on this option?

Quoted 5 lines

drivers/counter/Makefile new file +0 -0

new file mode 100644
index 000000000000..ad1ba7109cdc

drivers/counter/Makefile line 5

Quoted 6 lines

drivers/counter/Makefile new file +5 -0

@@ -0,0 +1,8 @@
+#
+# Makefile for Counter devices
+#
+
+# When adding new entries keep the list in alphabetical order

Why does it matter? :)

drivers/counter/Makefile line 8

Quoted 3 lines

drivers/counter/Makefile new file +3 -0

+
+obj-$(CONFIG_COUNTER) += counter.o
+counter-y := generic-counter.o

Why not just call your .c file, "counter.c" to keep this simple?

drivers/counter/generic-counter.c line 1

Quoted 7 lines

drivers/counter/generic-counter.c new file +1 -0

new file mode 100644
index 000000000000..483826c8ce01
@@ -0,0 +1,1519 @@
+// SPDX-License-Identifier: GPL-2.0-only

Please use the normal SPDX identifiers we are using, as described in the kernel documentation. We aren't ready to use the "new" ones just yet.

drivers/counter/generic-counter.c line 4

Quoted 3 lines

drivers/counter/generic-counter.c new file +3 -0

+/*
+ * Generic Counter interface
+ * Copyright (C) 2017 William Breathitt Gray

It's 2018 now :)

Quoted 12 lines

drivers/counter/generic-counter.c new file +12 -0

+ */
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/fs.h>
+#include <linux/gfp.h>
+#include <linux/idr.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/printk.h>

drivers/counter/generic-counter.c line 22

Quoted 6 lines

drivers/counter/generic-counter.c new file +6 -0

+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#include <linux/counter.h>

Why a blank line?

drivers/counter/generic-counter.c line 28

Quoted 6 lines

drivers/counter/generic-counter.c new file +6 -0

+
+const char *const count_direction_str[2] = {
+	[COUNT_DIRECTION_FORWARD] = "forward",
+	[COUNT_DIRECTION_BACKWARD] = "backward"
+};
+EXPORT_SYMBOL(count_direction_str);

I have to ask, for all of these, why not EXPORT_SYMBOL_GPL()?

drivers/counter/generic-counter.c line 36

Quoted 8 lines

drivers/counter/generic-counter.c new file +8 -0

+
+const char *const count_mode_str[4] = {
+	[COUNT_MODE_NORMAL] = "normal",
+	[COUNT_MODE_RANGE_LIMIT] = "range limit",
+	[COUNT_MODE_NON_RECYCLE] = "non-recycle",
+	[COUNT_MODE_MODULO_N] = "modulo-n"
+};
+EXPORT_SYMBOL(count_mode_str);

And why do you need to export strings?

Quoted 6 lines

drivers/counter/generic-counter.c new file +6 -0

+
+ssize_t counter_signal_enum_read(struct counter_device *counter,
+				 struct counter_signal *signal, void *priv,
+				 char *buf)
+{
+	const struct counter_signal_enum_ext *const e = priv;

drivers/counter/generic-counter.c line 47

Quoted 5 lines

drivers/counter/generic-counter.c new file +5 -0

+	int err;
+	size_t index;
+
+	if (!e->get)
+		return -EINVAL;

How can e->get not be set? Shouldn't you just not have called into this if so?

Quoted 4 lines

drivers/counter/generic-counter.c new file +4 -0

+
+	err = e->get(counter, signal, &index);
+	if (err)
+		return err;

drivers/counter/generic-counter.c line 56

Quoted 5 lines

drivers/counter/generic-counter.c new file +5 -0

+
+	if (index >= e->num_items)
+		return -EINVAL;
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n", e->items[index]);

No need to do the scnprintf() crud, it's a sysfs file, a simple sprintf() works just fine. Yeah, it makes people nervous, and it should :)

drivers/counter/generic-counter.c line 58

Quoted 2 lines

drivers/counter/generic-counter.c new file +2 -0

+}
+EXPORT_SYMBOL(counter_signal_enum_read);

sysfs attribute files really should be EXPORT_SYMBOL_GPL() please.

drivers/counter/generic-counter.c line 69

Quoted 11 lines

drivers/counter/generic-counter.c new file +11 -0

+
+ssize_t counter_signal_enum_write(struct counter_device *counter,
+				  struct counter_signal *signal, void *priv,
+				  const char *buf, size_t len)
+{
+	const struct counter_signal_enum_ext *const e = priv;
+	ssize_t index;
+	int err;
+
+	if (!e->set)
+		return -EINVAL;

Again, how can this be true?

Quoted 21 lines

drivers/counter/generic-counter.c new file +21 -0

+
+	index = __sysfs_match_string(e->items, e->num_items, buf);
+	if (index < 0)
+		return index;
+
+	err = e->set(counter, signal, index);
+	if (err)
+		return err;
+
+	return len;
+}
+EXPORT_SYMBOL(counter_signal_enum_write);
+
+ssize_t counter_signal_enum_available_read(struct counter_device *counter,
+					   struct counter_signal *signal,
+					   void *priv, char *buf)
+{
+	const struct counter_signal_enum_ext *const e = priv;
+	size_t i;
+	size_t len = 0;
+

drivers/counter/generic-counter.c line 96

Quoted 6 lines

drivers/counter/generic-counter.c new file +6 -0

+	if (!e->num_items)
+		return 0;
+
+	for (i = 0; i < e->num_items; i++)
+		len += scnprintf(buf + len, PAGE_SIZE - len, "%s\n",
+			e->items[i]);

a list of items? In sysfs? Are you _SURE_ you want to do that?

Quoted 5 lines

drivers/counter/generic-counter.c new file +5 -0

+
+	return len;
+}
+EXPORT_SYMBOL(counter_signal_enum_available_read);
+

drivers/counter/generic-counter.c line 111

Quoted 10 lines

drivers/counter/generic-counter.c new file +10 -0

+ssize_t counter_count_enum_read(struct counter_device *counter,
+				struct counter_count *count, void *priv,
+				char *buf)
+{
+	const struct counter_count_enum_ext *const e = priv;
+	int err;
+	size_t index;
+
+	if (!e->get)
+		return -EINVAL;

Same comment everywhere for this...

let's skip lower in the files...

drivers/counter/generic-counter.c line 317

Quoted 8 lines

drivers/counter/generic-counter.c new file +8 -0

+static int counter_attribute_create(
+	struct counter_device_attr_group *const group,
+	const char *const prefix,
+	const char *const name,
+	ssize_t (*show)(struct device *dev, struct device_attribute *attr,
+			char *buf),
+	ssize_t (*store)(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t len),

Typedefs for these function pointers are good to have.

drivers/counter/generic-counter.c line 318

Quoted 1 line

drivers/counter/generic-counter.c new file +1 -0

+	void *const component)

That's a crazy list of parameters...

Quoted 31 lines

drivers/counter/generic-counter.c new file +31 -0

+{
+	struct counter_device_attr *counter_attr;
+	struct device_attribute *dev_attr;
+	int err;
+	struct list_head *const attr_list = &group->attr_list;
+
+	/* Allocate a Counter device attribute */
+	counter_attr = kzalloc(sizeof(*counter_attr), GFP_KERNEL);
+	if (!counter_attr)
+		return -ENOMEM;
+	dev_attr = &counter_attr->dev_attr;
+
+	sysfs_attr_init(&dev_attr->attr);
+
+	/* Configure device attribute */
+	dev_attr->attr.name = kasprintf(GFP_KERNEL, "%s%s", prefix, name);
+	if (!dev_attr->attr.name) {
+		err = -ENOMEM;
+		goto err_free_counter_attr;
+	}
+	if (show) {
+		dev_attr->attr.mode |= 0444;
+		dev_attr->show = show;
+	}
+	if (store) {
+		dev_attr->attr.mode |= 0200;
+		dev_attr->store = store;
+	}
+
+	/* Store associated Counter component with attribute */
+	counter_attr->component = component;

drivers/counter/generic-counter.c line 353

Quoted 4 lines

drivers/counter/generic-counter.c new file +4 -0

+
+	/* Keep track of the attribute for later cleanup */
+	list_add(&counter_attr->l, attr_list);
+	group->num_attr++;

So you are dynamically creating sysfs attributes? Why not just have one big group and only enable them if needed? That would make things a lot simpler, right?

drivers/counter/generic-counter.c line 365

Quoted 1 line

drivers/counter/generic-counter.c new file +1 -0

+struct signal_comp_t {

"_t"???

Quoted 11 lines

drivers/counter/generic-counter.c new file +11 -0

+	struct counter_signal	*signal;
+};
+
+static ssize_t counter_signal_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct counter_device *const counter = dev_get_drvdata(dev);
+	const struct counter_device_attr *const devattr = to_counter_attr(attr);
+	const struct signal_comp_t *const component = devattr->component;
+	struct counter_signal *const signal = component->signal;
+	int err;

drivers/counter/generic-counter.c line 386

Quoted 10 lines

drivers/counter/generic-counter.c new file +10 -0

+	struct signal_read_value val = { .buf = buf };
+
+	err = counter->ops->signal_read(counter, signal, &val);
+	if (err)
+		return err;
+
+	return val.len;
+}
+
+struct name_comp_t {

"_t"???

Same for the rest of this file...

include/linux/counter.h line 1

Quoted 7 lines

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

new file mode 100644
index 000000000000..88fc82ee30a7
@@ -0,0 +1,534 @@
+/* SPDX-License-Identifier: GPL-2.0-only */

Same identifier issue.

include/linux/counter.h line 4

Quoted 3 lines

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

+/*
+ * Counter interface
+ * Copyright (C) 2017 William Breathitt Gray

2018!

And again, it looks like this file can be a lot smaller, but I don't see a user of it just yet, so I don't really know...

thanks,

greg k-h

I'll make the updates noted in a version 8 submission, but I'll wait to submit it until you have a chance to review the rest of this current patchset. The counter device drivers in this directory (104-quad-8.c, stm32-lptimer-cnt.c, stm32-timer-cnt.c) should give you an idea of how the counter.h file is used at the moment.

Quoted 5 lines · William Breathitt Gray

I'll make the updates noted in a version 8 submission, but I'll wait to submit it until you have a chance to review the rest of this current patchset. The counter device drivers in this directory (104-quad-8.c, stm32-lptimer-cnt.c, stm32-timer-cnt.c) should give you an idea of how the counter.h file is used at the moment.

Please do not wait for me, I will not have a chance to review the other patches anytime soon.

greg k-h

This patch introduces the Generic Counter interface for supporting counter devices.

Quoted 3 lines · On Thu, 21 Jun 2018 17:07:08 -0400 William Breathitt Gray…

This patch introduces the Generic Counter interface for supporting counter devices.

drivers/counter/generic-counter.c new file +18 -0

+EXPORT_SYMBOL(count_direction_str);
+EXPORT_SYMBOL(count_mode_str);
+EXPORT_SYMBOL(counter_signal_enum_read);
+EXPORT_SYMBOL(counter_signal_enum_write);
+EXPORT_SYMBOL(counter_signal_enum_available_read);
+EXPORT_SYMBOL(counter_count_enum_read);
+EXPORT_SYMBOL(counter_count_enum_write);
+EXPORT_SYMBOL(counter_count_enum_available_read);
+EXPORT_SYMBOL(counter_device_enum_read);
+EXPORT_SYMBOL(counter_device_enum_write);
+EXPORT_SYMBOL(counter_device_enum_available_read);
+EXPORT_SYMBOL(signal_read_value_set);
+EXPORT_SYMBOL(count_read_value_set);
+EXPORT_SYMBOL(count_write_value_get);
+EXPORT_SYMBOL(counter_register);
+EXPORT_SYMBOL(counter_unregister);
+EXPORT_SYMBOL(devm_counter_register);
+EXPORT_SYMBOL(devm_counter_unregister);

The naming is a bit chaotic. Most of the symbols start with counter_, which is good. But a handful do not.

Also, symbols called signal_* make my head spin - Linux already has a firmly ingrained notion of what a signal is, and this ain't it ;) Although the kernel tends to use sig_ for signals-as-an-IPC-thing.

Also, many many drivers deal with signals-as-an-electrical-thing - is it appropriate for this particular driver to take that namespace?

Quoted 2 lines

Also, many many drivers deal with signals-as-an-electrical-thing - is it appropriate for this particular driver to take that namespace?

In the context of the Generic Counter paradigm, a "Signal" is an abstraction for the stream of data that is fed to the counter device for evaluation (triggering updates for the readable "Count"). In many cases a "Signal" correlates with a physical electrical line (for example the A and B electrical lines for a quadrature encoder), but this isn't a hard requirement as the paradigm permits more abstract data streams.

I decided on "Signal" to match the naming convention that appears in the datasheets of many counter devices, but "Line" may be a decent alternative name we could use to indicate a counter device input data stream.

I'd like to get some other opinions as well before I make a naming change to "Signal" -- whether to stay with "Signal," switch to "Line," or rename to something else. For what it's worth, I think it's unlikely for a counter device driver author to confuse a Counter Signal with the Linux OS signal within the context of the Generic Counter paradigm and their respective counter device datasheet.

This patch adds standard documentation for the userspace sysfs attributes of the Generic Counter interface.

Reviewed-by Jonathan Cameron <[email protected]>

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

---
  Documentation/ABI/testing/sysfs-bus-counter | 230 ++++++++++++++++++++
  MAINTAINERS                                 |   1 +
  2 files changed, 231 insertions(+)

create mode 100644 Documentation/ABI/testing/sysfs-bus-counter

Documentation/ABI/testing/sysfs-bus-counter new file +46 -0

new file mode 100644
index 000000000000..0bb0dce67bf8
@@ -0,0 +1,230 @@
+What:		/sys/bus/counter/devices/counterX/countY/count
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Count data of Count Y represented as a string.
+
+What:		/sys/bus/counter/devices/counterX/countY/ceiling
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Count value ceiling for Count Y. This is the upper limit for the
+		respective counter.
+
+What:		/sys/bus/counter/devices/counterX/countY/floor
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Count value floor for Count Y. This is the lower limit for the
+		respective counter.
+
+What:		/sys/bus/counter/devices/counterX/countY/count_mode
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Count mode for channel Y. The ceiling and floor values for
+		Count Y are used by the count mode where required. The following
+		count modes are available:
+
+		Normal:
+			Counting is continuous in either direction.
+
+		Range Limit:
+			An upper or lower limit is set, mimicking limit switches
+			in the mechanical counterpart. The upper limit is set to
+			the Count Y ceiling value, while the lower limit is set
+			to the Count Y floor value. The counter freezes at
+			count = ceiling when counting up, and at count = floor
+			when counting down. At either of these limits, the
+			counting is resumed only when the count direction is
+			reversed.
+
+		Non-Recycle:
+			The counter is disabled whenever a counter overflow or
+			underflow takes place. The counter is re-enabled when a
+			new count value is loaded to the counter via a preset
+			operation or direct write.

Documentation/ABI/testing/sysfs-bus-counter new file +82 -0

+What:		/sys/bus/counter/devices/counterX/countY/count_mode_available
+What:		/sys/bus/counter/devices/counterX/countY/error_noise_available
+What:		/sys/bus/counter/devices/counterX/countY/function_available
+What:		/sys/bus/counter/devices/counterX/countY/signalZ_action_available
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Discrete set of available values for the respective Count Y
+		configuration are listed in this file. Values are delineated by
+		newline characters.
+
+What:		/sys/bus/counter/devices/counterX/countY/direction
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Read-only attribute that indicates the count direction of Count
+		Y. Two count directions are available: forward and backward.
+
+		Some counter devices are able to determine the direction of
+		their counting. For example, quadrature encoding counters can
+		determine the direction of movement by evaluating the leading
+		phase of the respective A and B quadrature encoding signals.
+		This attribute exposes such count directions.
+
+What:		/sys/bus/counter/devices/counterX/countY/enable
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Whether channel Y counter is enabled. Valid attribute values are
+		boolean.
+
+		This attribute is intended to serve as a pause/unpause mechanism
+		for Count Y. Suppose a counter device is used to count the total
+		movement of a conveyor belt: this attribute allows an operator
+		to temporarily pause the counter, service the conveyor belt,
+		and then finally unpause the counter to continue where it had
+		left off.
+
+What:		/sys/bus/counter/devices/counterX/countY/error_noise
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Read-only attribute that indicates whether excessive noise is
+		present at the channel Y counter inputs.
+
+What:		/sys/bus/counter/devices/counterX/countY/function
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Count function mode of Count Y; count function evaluation is
+		triggered by conditions specified by the Count Y signalZ_action
+		attributes. The following count functions are available:
+
+		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 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.
+
+		Quadrature 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.
+
+		Quadrature x2 A:
+			Any state transition on quadrature pair signal A updates
+			the respective count. Quadrature encoding determines the
+			direction.
+

Documentation/ABI/testing/sysfs-bus-counter new file +6 -0

+What:		/sys/bus/counter/devices/counterX/countY/name
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Read-only attribute that indicates the device-specific name of
+		Count Y. If possible, this should match the name of the

Documentation/ABI/testing/sysfs-bus-counter new file +20 -0

+
+What:		/sys/bus/counter/devices/counterX/countY/preset_enable
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Whether channel Y counter preset operation is enabled. Valid
+		attribute values are boolean.
+
+What:		/sys/bus/counter/devices/counterX/countY/signalZ_action
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Action mode of Count Y for Signal Z. This attribute indicates
+		the condition of Signal Z that triggers the count function
+		evaluation for Count Y. The following action modes are
+		available:
+
+		None:
+			Signal does not trigger the count function. In
+			Pulse-Direction count function mode, this Signal is

Documentation/ABI/testing/sysfs-bus-counter new file +18 -0

+
+What:		/sys/bus/counter/devices/counterX/name
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Read-only attribute that indicates the device-specific name of
+		the Counter. This should match the name of the device as it
+		appears in its respective datasheet.
+
+What:		/sys/bus/counter/devices/counterX/num_counts
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Read-only attribute that indicates the total number of Counts
+		belonging to the Counter.
+
+What:		/sys/bus/counter/devices/counterX/num_signals
+KernelVersion:	4.19

Documentation/ABI/testing/sysfs-bus-counter new file +8 -0

+
+What:		/sys/bus/counter/devices/counterX/signalY/name
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Read-only attribute that indicates the device-specific name of
+		Signal Y. If possible, this should match the name of the
+		respective signal as it appears in the device datasheet.

MAINTAINERS +1 -0

index f6e995c142ed..f8a47fd197a1 100644
@@ -3692,6 +3692,7 @@ COUNTER SUBSYSTEM
  M:	William Breathitt Gray <[email protected]>
  L:	linux-iio at vger.kernel.org
  S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-counter*
  F:	drivers/counter/
  F:	include/linux/counter.h
 

Quoted 31 lines · William Breathitt Gray

Documentation/ABI/testing/sysfs-bus-counter new file +31 -0

+What:		/sys/bus/counter/devices/counterX/countY/count_mode
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Count mode for channel Y. The ceiling and floor values for
+		Count Y are used by the count mode where required. The following
+		count modes are available:
+
+		Normal:
+			Counting is continuous in either direction.
+
+		Range Limit:
+			An upper or lower limit is set, mimicking limit switches
+			in the mechanical counterpart. The upper limit is set to
+			the Count Y ceiling value, while the lower limit is set
+			to the Count Y floor value. The counter freezes at
+			count = ceiling when counting up, and at count = floor
+			when counting down. At either of these limits, the
+			counting is resumed only when the count direction is
+			reversed.
+
+		Non-Recycle:
+			The counter is disabled whenever a counter overflow or
+			underflow takes place. The counter is re-enabled when a
+			new count value is loaded to the counter via a preset
+			operation or direct write.
+
+		Modulo-N:
+			A count value boundary is set between the Count Y floor
+			value and the Count Y ceiling value. The counter is
+			reset to the Cunt Y floor value at count = ceiling when

Documentation/ABI/testing/sysfs-bus-counter line 56

Quoted 5 lines

Documentation/ABI/testing/sysfs-bus-counter new file +5 -0

+			counting up, while the counter is set to the Count Y
+			ceiling value at count = floor when counting down; the
+			counter does not freeze at the boundary points, but
+			counts continuously throughout.
+

This might be a bit misleading since the values returned are all lower case, e.g. "rising edge", whereas they are listed here in Title Case.

Documentation/ABI/testing/sysfs-bus-counter line 164

Quoted 8 lines

Documentation/ABI/testing/sysfs-bus-counter new file +8 -0

+What:		/sys/bus/counter/devices/counterX/countY/preset
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		If the counter device supports preset registers -- registers
+		used to load counter channels to a set count upon device-defined
+		preset operation trigger events -- the preset count for channel
+		Y is provided by this attribute.

Should this be presetZ in case a device has more than one preset?

Documentation/ABI/testing/sysfs-bus-counter line 222

Quoted 5 lines

Documentation/ABI/testing/sysfs-bus-counter new file +5 -0

+What:		/sys/bus/counter/devices/counterX/signalY/signal
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Signal data of Signal Y represented as a string.

I guess the actual value returned depends on the type of signal? Would we need /sys/bus/counter/devices/counterX/signalY/type to indicate this so we know how to interpret the value?

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

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

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

Documentation/driver-api/generic-counter.rst new file +342 -0

new file mode 100644
index 000000000000..f51db893f595
@@ -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.

Documentation/driver-api/index.rst +1 -0

index f4180e7c7ed5..e39a9fd3d8c9 100644
@@ -52,6 +52,7 @@ available subsections can be seen below.
    slimbus
    soundwire/index
    fpga/index
+   generic-counter
 
 .. only::  subproject and html
 

MAINTAINERS +1 -0

index f8a47fd197a1..c7fd36500635 100644
@@ -3693,6 +3693,7 @@ M:	William Breathitt Gray <[email protected]>
 L:	linux-iio at vger.kernel.org
 S:	Maintained
 F:	Documentation/ABI/testing/sysfs-bus-counter*
+F:	Documentation/driver-api/generic-counter.rst
 F:	drivers/counter/
 F:	include/linux/counter.h
 

Documentation/driver-api/generic-counter.rst line 242

Quoted 13 lines · William Breathitt Gray

Documentation/driver-api/generic-counter.rst new file +13 -0

+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

Quoted 28 lines · David Lechner

Documentation/driver-api/generic-counter.rst line 242

Quoted 13 lines · William Breathitt Gray

Documentation/driver-api/generic-counter.rst new file +13 -0

+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.

Quoted 1 line

[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.

Quoted 18 lines · William Breathitt Gray

Quoted 17 lines · David Lechner

Documentation/driver-api/generic-counter.rst line 242

Quoted 13 lines · William Breathitt Gray

Documentation/driver-api/generic-counter.rst new file +13 -0

+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?

Quoted 5 lines

Quoted 5 lines

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.

Quoted 2 lines

Quoted 2 lines

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

Quoted 1 line

Quoted 1 line

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

Quoted 2 lines

Quoted 2 lines

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

Quoted 7 lines

Quoted 2 lines

[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.

Quoted 5 lines

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
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 20 lines · On Wed, 4 Jul 2018 19:23:26 +0200 Linus Walleij <[email protected]>…

Quoted 18 lines · William Breathitt Gray

Quoted 17 lines · David Lechner

Documentation/driver-api/generic-counter.rst line 242

Quoted 13 lines · William Breathitt Gray

Documentation/driver-api/generic-counter.rst new file +13 -0

+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?

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

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

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

- 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.

Quoted 1 line

Quoted 1 line

Quoted 1 line

- 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.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

- 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.

Quoted 11 lines

Quoted 7 lines

Quoted 2 lines

[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.

Quoted 5 lines

Quoted 5 lines

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!

Documentation/driver-api/generic-counter.rst line 241

Quoted 12 lines · William Breathitt Gray

Documentation/driver-api/generic-counter.rst new file +12 -0

+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.

This patch adds support for the 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.

Generic Counter Counts are created for the eight quadrature channel counts, as well as their respective quadrature A and B Signals (which are associated via respective Synapse structures) and respective index Signals.

The new Generic Counter interface sysfs attributes are intended to expose the same functionality and data available via the existing 104-QUAD-8 IIO device interface; the Generic Counter interface serves to provide the respective functionality and data in a standard way expected of counter devices.

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

---
 MAINTAINERS                            |   4 +-
 drivers/{iio => }/counter/104-quad-8.c | 782 ++++++++++++++++++++++++-
 drivers/counter/Kconfig                |  21 +
 drivers/counter/Makefile               |   2 +
 drivers/iio/counter/Kconfig            |  17 -
 drivers/iio/counter/Makefile           |   1 -
 6 files changed, 784 insertions(+), 43 deletions(-)
 rename drivers/{iio => }/counter/104-quad-8.c (44%)

MAINTAINERS +2 -2

index c7fd36500635..4083523699f3 100644
@@ -266,12 +266,12 @@ L:	linux-gpio at vger.kernel.org
 S:	Maintained
 F:	drivers/gpio/gpio-104-idio-16.c
 
-ACCES 104-QUAD-8 IIO DRIVER
+ACCES 104-QUAD-8 DRIVER
 M:	William Breathitt Gray <[email protected]>
 L:	linux-iio at vger.kernel.org
 S:	Maintained
 F:	Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8
-F:	drivers/iio/counter/104-quad-8.c
+F:	drivers/counter/104-quad-8.c
 
 ACCES PCI-IDIO-16 GPIO DRIVER
 M:	William Breathitt Gray <[email protected]>

drivers/counter/104-quad-8.c renamed from drivers/iio/counter/104-quad-8.c +2 -1

similarity index 44%
rename from drivers/iio/counter/104-quad-8.c
rename to drivers/counter/104-quad-8.c
index 92be8d0f7735..bbd1640f3a51 100644
@@ -1,19 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
- * IIO driver for the ACCES 104-QUAD-8
+ * Counter driver for the ACCES 104-QUAD-8
  * Copyright (C) 2016 William Breathitt Gray

drivers/counter/104-quad-8.c renamed from drivers/iio/counter/104-quad-8.c +757 -13

  * This driver supports the ACCES 104-QUAD-8 and ACCES 104-QUAD-4.
  */
 #include <linux/bitops.h>
+#include <linux/counter.h>
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/iio/iio.h>

@@ -37,6 +30,7 @@ MODULE_PARM_DESC(base, "ACCES 104-QUAD-8 base addresses");
 
 /**
  * struct quad8_iio - IIO device private data structure
+ * @counter:		instance of the counter_device
  * @preset:		array of preset values
  * @count_mode:		array of count mode configurations
  * @quadrature_mode:	array of quadrature mode configurations

@@ -48,6 +42,7 @@ MODULE_PARM_DESC(base, "ACCES 104-QUAD-8 base addresses");
  * @base:		base port address of the IIO device
  */
 struct quad8_iio {
+	struct counter_device counter;
 	unsigned int preset[QUAD8_NUM_COUNTERS];
 	unsigned int count_mode[QUAD8_NUM_COUNTERS];
 	unsigned int quadrature_mode[QUAD8_NUM_COUNTERS];

@@ -91,6 +86,10 @@ struct quad8_iio {
 #define QUAD8_RLD_CNTR_OUT 0x10
 #define QUAD8_CHAN_OP_ENABLE_COUNTERS 0x00
 #define QUAD8_CHAN_OP_RESET_COUNTERS 0x01
+#define QUAD8_CMR_QUADRATURE_X1 0x08
+#define QUAD8_CMR_QUADRATURE_X2 0x10
+#define QUAD8_CMR_QUADRATURE_X4 0x18
+
 
 static int quad8_read_raw(struct iio_dev *indio_dev,
 	struct iio_chan_spec const *chan, int *val, int *val2, long mask)

@@ -346,13 +345,13 @@ static const char *const quad8_count_modes[] = {
 };
 
 static int quad8_set_count_mode(struct iio_dev *indio_dev,
-	const struct iio_chan_spec *chan, unsigned int count_mode)
+	const struct iio_chan_spec *chan, unsigned int cnt_mode)
 {
 	struct quad8_iio *const priv = iio_priv(indio_dev);
-	unsigned int mode_cfg = count_mode << 1;
+	unsigned int mode_cfg = cnt_mode << 1;
 	const int base_offset = priv->base + 2 * chan->channel + 1;
 
-	priv->count_mode[chan->channel] = count_mode;
+	priv->count_mode[chan->channel] = cnt_mode;
 
 	/* Add quadrature mode configuration */
 	if (priv->quadrature_mode[chan->channel])

@@ -562,24 +561,746 @@ static const struct iio_chan_spec quad8_channels[] = {
 	QUAD8_COUNT_CHAN(7), QUAD8_INDEX_CHAN(7)
 };
 
+static int quad8_signal_read(struct counter_device *counter,
+	struct counter_signal *signal, struct signal_read_value *val)
+{
+	const struct quad8_iio *const priv = counter->priv;
+	unsigned int state;
+	enum signal_level level;
+
+	/* Only Index signal levels can be read */
+	if (signal->id < 16)
+		return -EINVAL;
+
+	state = inb(priv->base + QUAD8_REG_INDEX_INPUT_LEVELS)
+		& BIT(signal->id - 16);
+
+	level = (state) ? SIGNAL_LEVEL_HIGH : SIGNAL_LEVEL_LOW;
+
+	signal_read_value_set(val, SIGNAL_LEVEL, &level);
+
+	return 0;
+}
+
+static int quad8_count_read(struct counter_device *counter,
+	struct counter_count *count, struct count_read_value *val)
+{
+	const struct quad8_iio *const priv = counter->priv;
+	const int base_offset = priv->base + 2 * count->id;
+	unsigned int flags;
+	unsigned int borrow;
+	unsigned int carry;
+	unsigned long position;
+	int i;
+
+	flags = inb(base_offset + 1);
+	borrow = flags & QUAD8_FLAG_BT;
+	carry = !!(flags & QUAD8_FLAG_CT);
+
+	/* Borrow XOR Carry effectively doubles count range */
+	position = (unsigned long)(borrow ^ carry) << 24;
+
+	/* Reset Byte Pointer; transfer Counter to Output Latch */
+	outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_BP | QUAD8_RLD_CNTR_OUT,
+	     base_offset + 1);
+
+	for (i = 0; i < 3; i++)
+		position |= (unsigned long)inb(base_offset) << (8 * i);
+
+	count_read_value_set(val, COUNT_POSITION, &position);
+
+	return 0;
+}
+
+static int quad8_count_write(struct counter_device *counter,
+	struct counter_count *count, struct count_write_value *val)
+{
+	const struct quad8_iio *const priv = counter->priv;
+	const int base_offset = priv->base + 2 * count->id;
+	int err;
+	unsigned long position;
+	int i;
+
+	err = count_write_value_get(&position, COUNT_POSITION, val);
+	if (err)
+		return err;
+
+	/* Only 24-bit values are supported */
+	if (position > 0xFFFFFF)
+		return -EINVAL;
+
+	/* Reset Byte Pointer */
+	outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_BP, base_offset + 1);
+
+	/* Counter can only be set via Preset Register */
+	for (i = 0; i < 3; i++)
+		outb(position >> (8 * i), base_offset);
+
+	/* Transfer Preset Register to Counter */
+	outb(QUAD8_CTR_RLD | QUAD8_RLD_PRESET_CNTR, base_offset + 1);
+
+	/* Reset Byte Pointer */
+	outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_BP, base_offset + 1);
+
+	/* Set Preset Register back to original value */
+	position = priv->preset[count->id];
+	for (i = 0; i < 3; i++)
+		outb(position >> (8 * i), base_offset);
+
+	/* Reset Borrow, Carry, Compare, and Sign flags */
+	outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_FLAGS, base_offset + 1);
+	/* Reset Error flag */
+	outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_E, base_offset + 1);
+
+	return 0;
+}
+
+enum quad8_count_function {
+	QUAD8_COUNT_FUNCTION_PULSE_DIRECTION = 0,
+	QUAD8_COUNT_FUNCTION_QUADRATURE_X1,
+	QUAD8_COUNT_FUNCTION_QUADRATURE_X2,
+	QUAD8_COUNT_FUNCTION_QUADRATURE_X4
+};
+
+static enum count_function quad8_count_functions_list[] = {
+	[QUAD8_COUNT_FUNCTION_PULSE_DIRECTION] = COUNT_FUNCTION_PULSE_DIRECTION,
+	[QUAD8_COUNT_FUNCTION_QUADRATURE_X1] = COUNT_FUNCTION_QUADRATURE_X1_A,
+	[QUAD8_COUNT_FUNCTION_QUADRATURE_X2] = COUNT_FUNCTION_QUADRATURE_X2_A,
+	[QUAD8_COUNT_FUNCTION_QUADRATURE_X4] = COUNT_FUNCTION_QUADRATURE_X4
+};
+
+static int quad8_function_get(struct counter_device *counter,
+	struct counter_count *count, size_t *function)
+{
+	const struct quad8_iio *const priv = counter->priv;
+	const int id = count->id;
+	const unsigned int quadrature_mode = priv->quadrature_mode[id];
+	const unsigned int scale = priv->quadrature_scale[id];
+
+	if (quadrature_mode)
+		switch (scale) {
+		case 0:
+			*function = QUAD8_COUNT_FUNCTION_QUADRATURE_X1;
+			break;
+		case 1:
+			*function = QUAD8_COUNT_FUNCTION_QUADRATURE_X2;
+			break;
+		case 2:
+			*function = QUAD8_COUNT_FUNCTION_QUADRATURE_X4;
+			break;
+		}
+	else
+		*function = QUAD8_COUNT_FUNCTION_PULSE_DIRECTION;
+
+	return 0;
+}
+
+static int quad8_function_set(struct counter_device *counter,
+	struct counter_count *count, size_t function)
+{
+	struct quad8_iio *const priv = counter->priv;
+	const int id = count->id;
+	unsigned int *const quadrature_mode = priv->quadrature_mode + id;
+	unsigned int *const scale = priv->quadrature_scale + id;
+	unsigned int mode_cfg = priv->count_mode[id] << 1;
+	unsigned int *const synchronous_mode = priv->synchronous_mode + id;
+	const unsigned int idr_cfg = priv->index_polarity[id] << 1;
+	const int base_offset = priv->base + 2 * id + 1;
+
+	if (function == QUAD8_COUNT_FUNCTION_PULSE_DIRECTION) {
+		*quadrature_mode = 0;
+
+		/* Quadrature scaling only available in quadrature mode */
+		*scale = 0;
+
+		/* Synchronous function not supported in non-quadrature mode */
+		if (*synchronous_mode) {
+			*synchronous_mode = 0;
+			/* Disable synchronous function mode */
+			outb(QUAD8_CTR_IDR | idr_cfg, base_offset);
+		}
+	} else {
+		*quadrature_mode = 1;
+
+		switch (function) {
+		case QUAD8_COUNT_FUNCTION_QUADRATURE_X1:
+			*scale = 0;
+			mode_cfg |= QUAD8_CMR_QUADRATURE_X1;
+			break;
+		case QUAD8_COUNT_FUNCTION_QUADRATURE_X2:
+			*scale = 1;
+			mode_cfg |= QUAD8_CMR_QUADRATURE_X2;
+			break;
+		case QUAD8_COUNT_FUNCTION_QUADRATURE_X4:
+			*scale = 2;
+			mode_cfg |= QUAD8_CMR_QUADRATURE_X4;
+			break;
+		}
+	}
+
+	/* Load mode configuration to Counter Mode Register */
+	outb(QUAD8_CTR_CMR | mode_cfg, base_offset);
+
+	return 0;
+}
+
+static void quad8_direction_get(struct counter_device *counter,
+	struct counter_count *count, enum count_direction *direction)
+{
+	const struct quad8_iio *const priv = counter->priv;
+	unsigned int ud_flag;
+	const unsigned int flag_addr = priv->base + 2 * count->id + 1;
+
+	/* U/D flag: nonzero = up, zero = down */
+	ud_flag = inb(flag_addr) & QUAD8_FLAG_UD;
+
+	*direction = (ud_flag) ? COUNT_DIRECTION_FORWARD :
+		COUNT_DIRECTION_BACKWARD;
+}
+
+enum quad8_synapse_action {
+	QUAD8_SYNAPSE_ACTION_NONE = 0,
+	QUAD8_SYNAPSE_ACTION_RISING_EDGE,
+	QUAD8_SYNAPSE_ACTION_FALLING_EDGE,
+	QUAD8_SYNAPSE_ACTION_BOTH_EDGES
+};
+
+static enum synapse_action quad8_index_actions_list[] = {
+	[QUAD8_SYNAPSE_ACTION_NONE] = SYNAPSE_ACTION_NONE,
+	[QUAD8_SYNAPSE_ACTION_RISING_EDGE] = SYNAPSE_ACTION_RISING_EDGE
+};
+
+static enum synapse_action quad8_synapse_actions_list[] = {
+	[QUAD8_SYNAPSE_ACTION_NONE] = SYNAPSE_ACTION_NONE,
+	[QUAD8_SYNAPSE_ACTION_RISING_EDGE] = SYNAPSE_ACTION_RISING_EDGE,
+	[QUAD8_SYNAPSE_ACTION_FALLING_EDGE] = SYNAPSE_ACTION_FALLING_EDGE,
+	[QUAD8_SYNAPSE_ACTION_BOTH_EDGES] = SYNAPSE_ACTION_BOTH_EDGES
+};
+
+static int quad8_action_get(struct counter_device *counter,
+	struct counter_count *count, struct counter_synapse *synapse,
+	size_t *action)
+{
+	struct quad8_iio *const priv = counter->priv;
+	int err;
+	size_t function = 0;
+	const size_t signal_a_id = count->synapses[0].signal->id;
+	enum count_direction direction;
+
+	/* Handle Index signals */
+	if (synapse->signal->id >= 16) {
+		if (priv->preset_enable[count->id])
+			*action = QUAD8_SYNAPSE_ACTION_RISING_EDGE;
+		else
+			*action = QUAD8_SYNAPSE_ACTION_NONE;
+
+		return 0;
+	}
+
+	err = quad8_function_get(counter, count, &function);
+	if (err)
+		return err;
+
+	/* Default action mode */
+	*action = QUAD8_SYNAPSE_ACTION_NONE;
+
+	/* Determine action mode based on current count function mode */
+	switch (function) {
+	case QUAD8_COUNT_FUNCTION_PULSE_DIRECTION:
+		if (synapse->signal->id == signal_a_id)
+			*action = QUAD8_SYNAPSE_ACTION_RISING_EDGE;
+		break;
+	case QUAD8_COUNT_FUNCTION_QUADRATURE_X1:
+		if (synapse->signal->id == signal_a_id) {
+			quad8_direction_get(counter, count, &direction);
+
+			if (direction == COUNT_DIRECTION_FORWARD)
+				*action = QUAD8_SYNAPSE_ACTION_RISING_EDGE;
+			else
+				*action = QUAD8_SYNAPSE_ACTION_FALLING_EDGE;
+		}
+		break;
+	case QUAD8_COUNT_FUNCTION_QUADRATURE_X2:
+		if (synapse->signal->id == signal_a_id)
+			*action = QUAD8_SYNAPSE_ACTION_BOTH_EDGES;
+		break;
+	case QUAD8_COUNT_FUNCTION_QUADRATURE_X4:
+		*action = QUAD8_SYNAPSE_ACTION_BOTH_EDGES;
+		break;
+	}
+
+	return 0;
+}
+
+const struct counter_ops quad8_ops = {
+	.signal_read = quad8_signal_read,
+	.count_read = quad8_count_read,
+	.count_write = quad8_count_write,
+	.function_get = quad8_function_get,
+	.function_set = quad8_function_set,
+	.action_get = quad8_action_get
+};
+
+static int quad8_index_polarity_get(struct counter_device *counter,
+	struct counter_signal *signal, size_t *index_polarity)
+{
+	const struct quad8_iio *const priv = counter->priv;
+	const size_t channel_id = signal->id - 16;
+
+	*index_polarity = priv->index_polarity[channel_id];
+
+	return 0;
+}
+
+static int quad8_index_polarity_set(struct counter_device *counter,
+	struct counter_signal *signal, size_t index_polarity)
+{
+	struct quad8_iio *const priv = counter->priv;
+	const size_t channel_id = signal->id - 16;
+	const unsigned int idr_cfg = priv->synchronous_mode[channel_id] |
+		index_polarity << 1;
+	const int base_offset = priv->base + 2 * channel_id + 1;
+
+	priv->index_polarity[channel_id] = index_polarity;
+
+	/* Load Index Control configuration to Index Control Register */
+	outb(QUAD8_CTR_IDR | idr_cfg, base_offset);
+
+	return 0;
+}
+
+static struct counter_signal_enum_ext quad8_index_pol_enum = {
+	.items = quad8_index_polarity_modes,
+	.num_items = ARRAY_SIZE(quad8_index_polarity_modes),
+	.get = quad8_index_polarity_get,
+	.set = quad8_index_polarity_set
+};
+
+static int quad8_synchronous_mode_get(struct counter_device *counter,
+	struct counter_signal *signal, size_t *synchronous_mode)
+{
+	const struct quad8_iio *const priv = counter->priv;
+	const size_t channel_id = signal->id - 16;
+
+	*synchronous_mode = priv->synchronous_mode[channel_id];
+
+	return 0;
+}
+
+static int quad8_synchronous_mode_set(struct counter_device *counter,
+	struct counter_signal *signal, size_t synchronous_mode)
+{
+	struct quad8_iio *const priv = counter->priv;
+	const size_t channel_id = signal->id - 16;
+	const unsigned int idr_cfg = synchronous_mode |
+		priv->index_polarity[channel_id] << 1;
+	const int base_offset = priv->base + 2 * channel_id + 1;
+
+	/* Index function must be non-synchronous in non-quadrature mode */
+	if (synchronous_mode && !priv->quadrature_mode[channel_id])
+		return -EINVAL;
+
+	priv->synchronous_mode[channel_id] = synchronous_mode;
+
+	/* Load Index Control configuration to Index Control Register */
+	outb(QUAD8_CTR_IDR | idr_cfg, base_offset);
+
+	return 0;
+}
+
+static struct counter_signal_enum_ext quad8_syn_mode_enum = {
+	.items = quad8_synchronous_modes,
+	.num_items = ARRAY_SIZE(quad8_synchronous_modes),
+	.get = quad8_synchronous_mode_get,
+	.set = quad8_synchronous_mode_set
+};
+
+static ssize_t quad8_count_floor_read(struct counter_device *counter,
+	struct counter_count *count, void *private, char *buf)
+{
+	/* Only a floor of 0 is supported */
+	return snprintf(buf, PAGE_SIZE, "0\n");
+}
+
+static int quad8_count_mode_get(struct counter_device *counter,
+	struct counter_count *count, size_t *cnt_mode)
+{
+	const struct quad8_iio *const priv = counter->priv;
+
+	/* Map 104-QUAD-8 count mode to Generic Counter count mode */
+	switch (priv->count_mode[count->id]) {
+	case 0:
+		*cnt_mode = COUNT_MODE_NORMAL;
+		break;
+	case 1:
+		*cnt_mode = COUNT_MODE_RANGE_LIMIT;
+		break;
+	case 2:
+		*cnt_mode = COUNT_MODE_NON_RECYCLE;
+		break;
+	case 3:
+		*cnt_mode = COUNT_MODE_MODULO_N;
+		break;
+	}
+
+	return 0;
+}
+
+static int quad8_count_mode_set(struct counter_device *counter,
+	struct counter_count *count, size_t cnt_mode)
+{
+	struct quad8_iio *const priv = counter->priv;
+	unsigned int mode_cfg;
+	const int base_offset = priv->base + 2 * count->id + 1;
+
+	/* Map Generic Counter count mode to 104-QUAD-8 count mode */
+	switch (cnt_mode) {
+	case COUNT_MODE_NORMAL:
+		cnt_mode = 0;
+		break;
+	case COUNT_MODE_RANGE_LIMIT:
+		cnt_mode = 1;
+		break;
+	case COUNT_MODE_NON_RECYCLE:
+		cnt_mode = 2;
+		break;
+	case COUNT_MODE_MODULO_N:
+		cnt_mode = 3;
+		break;
+	}
+
+	priv->count_mode[count->id] = cnt_mode;
+
+	/* Set count mode configuration value */
+	mode_cfg = cnt_mode << 1;
+
+	/* Add quadrature mode configuration */
+	if (priv->quadrature_mode[count->id])
+		mode_cfg |= (priv->quadrature_scale[count->id] + 1) << 3;
+
+	/* Load mode configuration to Counter Mode Register */
+	outb(QUAD8_CTR_CMR | mode_cfg, base_offset);
+
+	return 0;
+}
+
+static struct counter_count_enum_ext quad8_cnt_mode_enum = {
+	.items = count_mode_str,
+	.num_items = ARRAY_SIZE(count_mode_str),
+	.get = quad8_count_mode_get,
+	.set = quad8_count_mode_set
+};
+
+static ssize_t quad8_count_direction_read(struct counter_device *counter,
+	struct counter_count *count, void *priv, char *buf)
+{
+	enum count_direction dir;
+
+	quad8_direction_get(counter, count, &dir);
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n", count_direction_str[dir]);
+}
+
+static ssize_t quad8_count_enable_read(struct counter_device *counter,
+	struct counter_count *count, void *private, char *buf)
+{
+	const struct quad8_iio *const priv = counter->priv;
+
+	return scnprintf(buf, PAGE_SIZE, "%u\n", priv->ab_enable[count->id]);
+}
+
+static ssize_t quad8_count_enable_write(struct counter_device *counter,
+	struct counter_count *count, void *private, const char *buf, size_t len)
+{
+	struct quad8_iio *const priv = counter->priv;
+	const int base_offset = priv->base + 2 * count->id;
+	int err;
+	bool ab_enable;
+	unsigned int ior_cfg;
+
+	err = kstrtobool(buf, &ab_enable);
+	if (err)
+		return err;
+
+	priv->ab_enable[count->id] = ab_enable;
+
+	ior_cfg = ab_enable | priv->preset_enable[count->id] << 1;
+
+	/* Load I/O control configuration */
+	outb(QUAD8_CTR_IOR | ior_cfg, base_offset + 1);
+
+	return len;
+}
+
+static int quad8_error_noise_get(struct counter_device *counter,
+	struct counter_count *count, size_t *noise_error)
+{
+	const struct quad8_iio *const priv = counter->priv;
+	const int base_offset = priv->base + 2 * count->id + 1;
+
+	*noise_error = !!(inb(base_offset) & QUAD8_FLAG_E);
+
+	return 0;
+}
+
+static struct counter_count_enum_ext quad8_error_noise_enum = {
+	.items = quad8_noise_error_states,
+	.num_items = ARRAY_SIZE(quad8_noise_error_states),
+	.get = quad8_error_noise_get
+};
+
+static ssize_t quad8_count_preset_read(struct counter_device *counter,
+	struct counter_count *count, void *private, char *buf)
+{
+	const struct quad8_iio *const priv = counter->priv;
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", priv->preset[count->id]);
+}
+
+static ssize_t quad8_count_preset_write(struct counter_device *counter,
+	struct counter_count *count, void *private, const char *buf, size_t len)
+{
+	struct quad8_iio *const priv = counter->priv;
+	const int base_offset = priv->base + 2 * count->id;
+	unsigned int preset;
+	int ret;
+	int i;
+
+	ret = kstrtouint(buf, 0, &preset);
+	if (ret)
+		return ret;
+
+	/* Only 24-bit values are supported */
+	if (preset > 0xFFFFFF)
+		return -EINVAL;
+
+	priv->preset[count->id] = preset;
+
+	/* Reset Byte Pointer */
+	outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_BP, base_offset + 1);
+
+	/* Set Preset Register */
+	for (i = 0; i < 3; i++)
+		outb(preset >> (8 * i), base_offset);
+
+	return len;
+}
+
+static ssize_t quad8_count_ceiling_read(struct counter_device *counter,
+	struct counter_count *count, void *private, char *buf)
+{
+	const struct quad8_iio *const priv = counter->priv;
+
+	/* Range Limit and Modulo-N count modes use preset value as ceiling */
+	switch (priv->count_mode[count->id]) {
+	case 1:
+	case 3:
+		return quad8_count_preset_read(counter, count, private, buf);
+	}
+
+	/* By default 0x1FFFFFF (25 bits unsigned) is maximum count */
+	return snprintf(buf, PAGE_SIZE, "33554431\n");
+}
+
+static ssize_t quad8_count_ceiling_write(struct counter_device *counter,
+	struct counter_count *count, void *private, const char *buf, size_t len)
+{
+	struct quad8_iio *const priv = counter->priv;
+
+	/* Range Limit and Modulo-N count modes use preset value as ceiling */
+	switch (priv->count_mode[count->id]) {
+	case 1:
+	case 3:
+		return quad8_count_preset_write(counter, count, private, buf,
+						len);
+	}
+
+	return len;
+}
+
+static ssize_t quad8_count_preset_enable_read(struct counter_device *counter,
+	struct counter_count *count, void *private, char *buf)
+{
+	const struct quad8_iio *const priv = counter->priv;
+
+	return snprintf(buf, PAGE_SIZE, "%u\n",
+		!priv->preset_enable[count->id]);
+}
+
+static ssize_t quad8_count_preset_enable_write(struct counter_device *counter,
+	struct counter_count *count, void *private, const char *buf, size_t len)
+{
+	struct quad8_iio *const priv = counter->priv;
+	const int base_offset = priv->base + 2 * count->id + 1;
+	bool preset_enable;
+	int ret;
+	unsigned int ior_cfg;
+
+	ret = kstrtobool(buf, &preset_enable);
+	if (ret)
+		return ret;
+
+	/* Preset enable is active low in Input/Output Control register */
+	preset_enable = !preset_enable;
+
+	priv->preset_enable[count->id] = preset_enable;
+
+	ior_cfg = priv->ab_enable[count->id] | (unsigned int)preset_enable << 1;
+
+	/* Load I/O control configuration to Input / Output Control Register */
+	outb(QUAD8_CTR_IOR | ior_cfg, base_offset);
+
+	return len;
+}
+
+static const struct counter_signal_ext quad8_index_ext[] = {
+	COUNTER_SIGNAL_ENUM("index_polarity", &quad8_index_pol_enum),
+	COUNTER_SIGNAL_ENUM_AVAILABLE("index_polarity",	&quad8_index_pol_enum),
+	COUNTER_SIGNAL_ENUM("synchronous_mode", &quad8_syn_mode_enum),
+	COUNTER_SIGNAL_ENUM_AVAILABLE("synchronous_mode", &quad8_syn_mode_enum)
+};
+
+#define	QUAD8_QUAD_SIGNAL(_id, _name) {	\
+	.id = (_id),			\
+	.name = (_name)			\
+}
+
+#define	QUAD8_INDEX_SIGNAL(_id, _name) {	\
+	.id = (_id),				\
+	.name = (_name),			\
+	.ext = quad8_index_ext,			\
+	.num_ext = ARRAY_SIZE(quad8_index_ext)	\
+}
+
+static struct counter_signal quad8_signals[] = {
+	QUAD8_QUAD_SIGNAL(0, "Channel 1 Quadrature A"),
+	QUAD8_QUAD_SIGNAL(1, "Channel 1 Quadrature B"),
+	QUAD8_QUAD_SIGNAL(2, "Channel 2 Quadrature A"),
+	QUAD8_QUAD_SIGNAL(3, "Channel 2 Quadrature B"),
+	QUAD8_QUAD_SIGNAL(4, "Channel 3 Quadrature A"),
+	QUAD8_QUAD_SIGNAL(5, "Channel 3 Quadrature B"),
+	QUAD8_QUAD_SIGNAL(6, "Channel 4 Quadrature A"),
+	QUAD8_QUAD_SIGNAL(7, "Channel 4 Quadrature B"),
+	QUAD8_QUAD_SIGNAL(8, "Channel 5 Quadrature A"),
+	QUAD8_QUAD_SIGNAL(9, "Channel 5 Quadrature B"),
+	QUAD8_QUAD_SIGNAL(10, "Channel 6 Quadrature A"),
+	QUAD8_QUAD_SIGNAL(11, "Channel 6 Quadrature B"),
+	QUAD8_QUAD_SIGNAL(12, "Channel 7 Quadrature A"),
+	QUAD8_QUAD_SIGNAL(13, "Channel 7 Quadrature B"),
+	QUAD8_QUAD_SIGNAL(14, "Channel 8 Quadrature A"),
+	QUAD8_QUAD_SIGNAL(15, "Channel 8 Quadrature B"),
+	QUAD8_INDEX_SIGNAL(16, "Channel 1 Index"),
+	QUAD8_INDEX_SIGNAL(17, "Channel 2 Index"),
+	QUAD8_INDEX_SIGNAL(18, "Channel 3 Index"),
+	QUAD8_INDEX_SIGNAL(19, "Channel 4 Index"),
+	QUAD8_INDEX_SIGNAL(20, "Channel 5 Index"),
+	QUAD8_INDEX_SIGNAL(21, "Channel 6 Index"),
+	QUAD8_INDEX_SIGNAL(22, "Channel 7 Index"),
+	QUAD8_INDEX_SIGNAL(23, "Channel 8 Index")
+};
+
+#define QUAD8_COUNT_SYNAPSES(_id) {					\
+	{								\
+		.actions_list = quad8_synapse_actions_list,		\
+		.num_actions = ARRAY_SIZE(quad8_synapse_actions_list),	\
+		.signal = quad8_signals + 2 * (_id)			\
+	},								\
+	{								\
+		.actions_list = quad8_synapse_actions_list,		\
+		.num_actions = ARRAY_SIZE(quad8_synapse_actions_list),	\
+		.signal = quad8_signals + 2 * (_id) + 1			\
+	},								\
+	{								\
+		.actions_list = quad8_index_actions_list,		\
+		.num_actions = ARRAY_SIZE(quad8_index_actions_list),	\
+		.signal = quad8_signals + 2 * (_id) + 16		\
+	}								\
+}
+
+static struct counter_synapse quad8_count_synapses[][3] = {
+	QUAD8_COUNT_SYNAPSES(0), QUAD8_COUNT_SYNAPSES(1),
+	QUAD8_COUNT_SYNAPSES(2), QUAD8_COUNT_SYNAPSES(3),
+	QUAD8_COUNT_SYNAPSES(4), QUAD8_COUNT_SYNAPSES(5),
+	QUAD8_COUNT_SYNAPSES(6), QUAD8_COUNT_SYNAPSES(7)
+};
+
+static const struct counter_count_ext quad8_count_ext[] = {
+	{
+		.name = "ceiling",
+		.read = quad8_count_ceiling_read,
+		.write = quad8_count_ceiling_write
+	},
+	{
+		.name = "floor",
+		.read = quad8_count_floor_read
+	},
+	COUNTER_COUNT_ENUM("count_mode", &quad8_cnt_mode_enum),
+	COUNTER_COUNT_ENUM_AVAILABLE("count_mode", &quad8_cnt_mode_enum),
+	{
+		.name = "direction",
+		.read = quad8_count_direction_read
+	},
+	{
+		.name = "enable",
+		.read = quad8_count_enable_read,
+		.write = quad8_count_enable_write
+	},
+	COUNTER_COUNT_ENUM("error_noise", &quad8_error_noise_enum),
+	COUNTER_COUNT_ENUM_AVAILABLE("error_noise", &quad8_error_noise_enum),
+	{
+		.name = "preset",
+		.read = quad8_count_preset_read,
+		.write = quad8_count_preset_write
+	},
+	{
+		.name = "preset_enable",
+		.read = quad8_count_preset_enable_read,
+		.write = quad8_count_preset_enable_write
+	}
+};
+
+#define QUAD8_COUNT(_id, _cntname) {					\
+	.id = (_id),							\
+	.name = (_cntname),						\
+	.functions_list = quad8_count_functions_list,			\
+	.num_functions = ARRAY_SIZE(quad8_count_functions_list),	\
+	.synapses = quad8_count_synapses[(_id)],			\
+	.num_synapses =	2,						\
+	.ext = quad8_count_ext,						\
+	.num_ext = ARRAY_SIZE(quad8_count_ext)				\
+}
+
+static struct counter_count quad8_counts[] = {
+	QUAD8_COUNT(0, "Channel 1 Count"),
+	QUAD8_COUNT(1, "Channel 2 Count"),
+	QUAD8_COUNT(2, "Channel 3 Count"),
+	QUAD8_COUNT(3, "Channel 4 Count"),
+	QUAD8_COUNT(4, "Channel 5 Count"),
+	QUAD8_COUNT(5, "Channel 6 Count"),
+	QUAD8_COUNT(6, "Channel 7 Count"),
+	QUAD8_COUNT(7, "Channel 8 Count")
+};
+
 static int quad8_probe(struct device *dev, unsigned int id)
 {
 	struct iio_dev *indio_dev;
-	struct quad8_iio *priv;
+	struct quad8_iio *quad8iio;
 	int i, j;
 	unsigned int base_offset;
+	int err;
 
-	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;
 	}
 
+	/* Allocate IIO device; this also allocates driver data structure */
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*quad8iio));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	/* Initialize IIO device */
 	indio_dev->info = &quad8_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->num_channels = ARRAY_SIZE(quad8_channels);

@@ -587,8 +1308,17 @@ static int quad8_probe(struct device *dev, unsigned int id)
 	indio_dev->name = dev_name(dev);
 	indio_dev->dev.parent = dev;
 
-	priv = iio_priv(indio_dev);
-	priv->base = base[id];
+	/* Initialize Counter device and driver data */
+	quad8iio = iio_priv(indio_dev);
+	quad8iio->counter.name = dev_name(dev);
+	quad8iio->counter.parent = dev;
+	quad8iio->counter.ops = &quad8_ops;
+	quad8iio->counter.counts = quad8_counts;
+	quad8iio->counter.num_counts = ARRAY_SIZE(quad8_counts);
+	quad8iio->counter.signals = quad8_signals;
+	quad8iio->counter.num_signals = ARRAY_SIZE(quad8_signals);
+	quad8iio->counter.priv = quad8iio;
+	quad8iio->base = base[id];
 
 	/* Reset all counters and disable interrupt function */
 	outb(QUAD8_CHAN_OP_RESET_COUNTERS, base[id] + QUAD8_REG_CHAN_OP);

@@ -614,7 +1344,13 @@ static int quad8_probe(struct device *dev, unsigned int id)
 	/* Enable all counters */
 	outb(QUAD8_CHAN_OP_ENABLE_COUNTERS, base[id] + QUAD8_REG_CHAN_OP);
 
-	return devm_iio_device_register(dev, indio_dev);
+	/* Register IIO device */
+	err = devm_iio_device_register(dev, indio_dev);
+	if (err)
+		return err;
+
+	/* Register Counter device */
+	return devm_counter_register(dev, &quad8iio->counter);
 }
 
 static struct isa_driver quad8_driver = {

drivers/counter/Kconfig +21 -0

index 65fa92abd5a4..7646f5df76f3 100644
@@ -16,3 +16,24 @@ menuconfig COUNTER
 	  consumption. The Generic Counter interface enables drivers to support
 	  and expose a common set of components and functionality present in
 	  counter devices.
+
+if COUNTER
+
+config 104_QUAD_8
+	tristate "ACCES 104-QUAD-8 driver"
+	depends on PC104 && X86 && IIO
+	select ISA_BUS_API
+	help
+	  Say yes here to build support for the ACCES 104-QUAD-8 quadrature
+	  encoder counter/interface device family (104-QUAD-8, 104-QUAD-4).
+
+	  A counter's respective error flag may be cleared by performing a write
+	  operation on the respective count value attribute. Although the
+	  104-QUAD-8 counters have a 25-bit range, only the lower 24 bits may be
+	  set, either directly or via the counter's preset attribute. Interrupts
+	  are not supported by this driver.
+
+	  The base port addresses for the devices may be configured via the base
+	  array module parameter.
+
+endif # COUNTER

drivers/counter/Makefile +2 -0

index ad1ba7109cdc..23a4f6263e45 100644
@@ -6,3 +6,5 @@
 
 obj-$(CONFIG_COUNTER) += counter.o
 counter-y := generic-counter.o
+
+obj-$(CONFIG_104_QUAD_8)	+= 104-quad-8.o

drivers/iio/counter/Kconfig +0 -17

index bf1e559ad7cd..eeb358122cbe 100644
@@ -5,23 +5,6 @@
 
 menu "Counters"
 
-config 104_QUAD_8
-	tristate "ACCES 104-QUAD-8 driver"
-	depends on PC104 && X86
-	select ISA_BUS_API
-	help
-	  Say yes here to build support for the ACCES 104-QUAD-8 quadrature
-	  encoder counter/interface device family (104-QUAD-8, 104-QUAD-4).
-
-	  Performing a write to a counter's IIO_CHAN_INFO_RAW sets the counter and
-	  also clears the counter's respective error flag. Although the counters
-	  have a 25-bit range, only the lower 24 bits may be set, either directly
-	  or via a counter's preset attribute. Interrupts are not supported by
-	  this driver.
-
-	  The base port addresses for the devices may be configured via the base
-	  array module parameter.
-
 config STM32_LPTIMER_CNT
 	tristate "STM32 LP Timer encoder counter driver"
 	depends on MFD_STM32_LPTIMER || COMPILE_TEST

drivers/iio/counter/Makefile +0 -1

index 1b9a896eb488..93933ba49280 100644
@@ -4,5 +4,4 @@
 
 # When adding new entries keep the list in alphabetical order
 
-obj-$(CONFIG_104_QUAD_8)	+= 104-quad-8.o
 obj-$(CONFIG_STM32_LPTIMER_CNT)	+= stm32-lptimer-cnt.o

This patch adds standard documentation for the Generic Counter interface userspace sysfs attributes of the 104-QUAD-8 driver.

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

---
 .../ABI/testing/sysfs-bus-counter-104-quad-8  | 36 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 37 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-counter-104-quad-8

Documentation/ABI/testing/sysfs-bus-counter-104-quad-8 new file +36 -0

new file mode 100644
index 000000000000..062637df3c78
@@ -0,0 +1,36 @@
+What:		/sys/bus/counter/devices/counterX/signalY/index_polarity
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Active level of index input Signal Y; irrelevant in
+		non-synchronous load mode.
+
+What:		/sys/bus/counter/devices/counterX/signalY/index_polarity_available
+What:		/sys/bus/counter/devices/counterX/signalY/synchronous_mode_available
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Discrete set of available values for the respective Signal Y
+		configuration are listed in this file.
+
+What:		/sys/bus/counter/devices/counterX/signalY/synchronous_mode
+KernelVersion:	4.19
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Configure the counter associated with Signal Y for
+		non-synchronous or synchronous load mode. Synchronous load mode
+		cannot be selected in non-quadrature (Pulse-Direction) clock
+		mode.
+
+		Non-synchronous:
+			A logic low level is the active level at this index
+			input. The index function (as enabled via preset_enable)
+			is performed directly on the active level of the index
+			input.
+
+		Synchronous:
+			Intended for interfacing with encoder Index output in
+			quadrature clock mode. The active level is configured
+			via index_polarity. The index function (as enabled via
+			preset_enable) is performed synchronously with the
+			quadrature clock on the active level of the index input.

MAINTAINERS +1 -0

index 4083523699f3..a06fd762f5d9 100644
@@ -270,6 +270,7 @@ ACCES 104-QUAD-8 DRIVER
 M:	William Breathitt Gray <[email protected]>
 L:	linux-iio at vger.kernel.org
 S:	Maintained
+F:	Documentation/ABI/testing/sysfs-bus-counter-104-quad-8
 F:	Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8
 F:	drivers/counter/104-quad-8.c
 

From: Benjamin Gaignard <[email protected]>

Implement counter part of the STM32 timer hardware block by using counter API. Hardware only supports X2 and X4 quadrature modes. A ceiling value can be set to define the maximum value reachable by the counter.

Signed-off-by Benjamin Gaignard <[email protected]>

Co-authored-by Fabrice Gasnier <[email protected]>

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

---
 drivers/counter/Kconfig           |  10 +
 drivers/counter/Makefile          |   1 +
 drivers/counter/stm32-timer-cnt.c | 390 ++++++++++++++++++++++++++++++
 3 files changed, 401 insertions(+)
 create mode 100644 drivers/counter/stm32-timer-cnt.c

drivers/counter/Kconfig +10 -0

index 7646f5df76f3..fcfbaa935122 100644
@@ -36,4 +36,14 @@ config 104_QUAD_8
 	  The base port addresses for the devices may be configured via the base
 	  array module parameter.
 
+config STM32_TIMER_CNT
+	tristate "STM32 Timer encoder counter driver"
+	depends on MFD_STM32_TIMERS || COMPILE_TEST
+	help
+	  Select this option to enable STM32 Timer quadrature encoder
+	  and counter driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called stm32-timer-cnt.
+
 endif # COUNTER

drivers/counter/Makefile +1 -0

index 23a4f6263e45..ff024259fb46 100644
@@ -8,3 +8,4 @@ obj-$(CONFIG_COUNTER) += counter.o
 counter-y := generic-counter.o
 
 obj-$(CONFIG_104_QUAD_8)	+= 104-quad-8.o
+obj-$(CONFIG_STM32_TIMER_CNT)	+= stm32-timer-cnt.o

drivers/counter/stm32-timer-cnt.c new file +0 -0

new file mode 100644
index 000000000000..0b15e729798b

drivers/counter/stm32-timer-cnt.c new file +382 -0

+ */
+#include <linux/counter.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+#include <linux/mfd/stm32-timers.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define TIM_CCMR_CCXS	(BIT(8) | BIT(0))
+#define TIM_CCMR_MASK	(TIM_CCMR_CC1S | TIM_CCMR_CC2S | \
+			 TIM_CCMR_IC1F | TIM_CCMR_IC2F)
+#define TIM_CCER_MASK	(TIM_CCER_CC1P | TIM_CCER_CC1NP | \
+			 TIM_CCER_CC2P | TIM_CCER_CC2NP)
+
+struct stm32_timer_cnt {
+	struct counter_device counter;
+	struct regmap *regmap;
+	struct clk *clk;
+	u32 ceiling;
+};
+
+/**
+ * stm32_count_function - enumerates stm32 timer counter encoder modes
+ * @STM32_COUNT_SLAVE_MODE_DISABLED: counts on internal clock when CEN=1
+ * @STM32_COUNT_ENCODER_MODE_1: counts TI1FP1 edges, depending on TI2FP2 level
+ * @STM32_COUNT_ENCODER_MODE_2: counts TI2FP2 edges, depending on TI1FP1 level
+ * @STM32_COUNT_ENCODER_MODE_3: counts on both TI1FP1 and TI2FP2 edges
+ */
+enum stm32_count_function {
+	STM32_COUNT_SLAVE_MODE_DISABLED = -1,
+	STM32_COUNT_ENCODER_MODE_1,
+	STM32_COUNT_ENCODER_MODE_2,
+	STM32_COUNT_ENCODER_MODE_3,
+};
+
+static enum count_function stm32_count_functions[] = {
+	[STM32_COUNT_ENCODER_MODE_1] = COUNT_FUNCTION_QUADRATURE_X2_A,
+	[STM32_COUNT_ENCODER_MODE_2] = COUNT_FUNCTION_QUADRATURE_X2_B,
+	[STM32_COUNT_ENCODER_MODE_3] = COUNT_FUNCTION_QUADRATURE_X4,
+};
+
+static int stm32_count_read(struct counter_device *counter,
+			    struct counter_count *count,
+			    struct count_read_value *val)
+{
+	struct stm32_timer_cnt *const priv = counter->priv;
+	u32 cnt;
+
+	regmap_read(priv->regmap, TIM_CNT, &cnt);
+	count_read_value_set(val, COUNT_POSITION, &cnt);
+
+	return 0;
+}
+
+static int stm32_count_write(struct counter_device *counter,
+			     struct counter_count *count,
+			     struct count_write_value *val)
+{
+	struct stm32_timer_cnt *const priv = counter->priv;
+	u32 cnt;
+	int err;
+
+	err = count_write_value_get(&cnt, COUNT_POSITION, val);
+	if (err)
+		return err;
+
+	if (cnt > priv->ceiling)
+		return -EINVAL;
+
+	return regmap_write(priv->regmap, TIM_CNT, cnt);
+}
+
+static int stm32_count_function_get(struct counter_device *counter,
+				    struct counter_count *count,
+				    size_t *function)
+{
+	struct stm32_timer_cnt *const priv = counter->priv;
+	u32 smcr;
+
+	regmap_read(priv->regmap, TIM_SMCR, &smcr);
+
+	switch (smcr & TIM_SMCR_SMS) {
+	case 1:
+		*function = STM32_COUNT_ENCODER_MODE_1;
+		return 0;
+	case 2:
+		*function = STM32_COUNT_ENCODER_MODE_2;
+		return 0;
+	case 3:
+		*function = STM32_COUNT_ENCODER_MODE_3;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int stm32_count_function_set(struct counter_device *counter,
+				    struct counter_count *count,
+				    size_t function)
+{
+	struct stm32_timer_cnt *const priv = counter->priv;
+	u32 cr1, sms;
+
+	switch (function) {
+	case STM32_COUNT_ENCODER_MODE_1:
+		sms = 1;
+		break;
+	case STM32_COUNT_ENCODER_MODE_2:
+		sms = 2;
+		break;
+	case STM32_COUNT_ENCODER_MODE_3:
+		sms = 3;
+		break;
+	default:
+		sms = 0;
+		break;
+	}
+
+	/* Store enable status */
+	regmap_read(priv->regmap, TIM_CR1, &cr1);
+
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+
+	/* TIMx_ARR register shouldn't be buffered (ARPE=0) */
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, 0);
+	regmap_write(priv->regmap, TIM_ARR, priv->ceiling);
+
+	regmap_update_bits(priv->regmap, TIM_SMCR, TIM_SMCR_SMS, sms);
+
+	/* Make sure that registers are updated */
+	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+
+	/* Restore the enable status */
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, cr1);
+
+	return 0;
+}
+
+static ssize_t stm32_count_direction_read(struct counter_device *counter,
+				      struct counter_count *count,
+				      void *private, char *buf)
+{
+	struct stm32_timer_cnt *const priv = counter->priv;
+	const char *direction;
+	u32 cr1;
+
+	regmap_read(priv->regmap, TIM_CR1, &cr1);
+	direction = (cr1 & TIM_CR1_DIR) ? "backward" : "forward";
+
+	return scnprintf(buf, PAGE_SIZE, "%s\n", direction);
+}
+
+static ssize_t stm32_count_ceiling_read(struct counter_device *counter,
+					struct counter_count *count,
+					void *private, char *buf)
+{
+	struct stm32_timer_cnt *const priv = counter->priv;
+	u32 arr;
+
+	regmap_read(priv->regmap, TIM_ARR, &arr);
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", arr);
+}
+
+static ssize_t stm32_count_ceiling_write(struct counter_device *counter,
+					 struct counter_count *count,
+					 void *private,
+					 const char *buf, size_t len)
+{
+	struct stm32_timer_cnt *const priv = counter->priv;
+	unsigned int ceiling;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &ceiling);
+	if (ret)
+		return ret;
+
+	/* TIMx_ARR register shouldn't be buffered (ARPE=0) */
+	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, 0);
+	regmap_write(priv->regmap, TIM_ARR, ceiling);
+
+	priv->ceiling = ceiling;
+	return len;
+}
+
+static ssize_t stm32_count_enable_read(struct counter_device *counter,
+				       struct counter_count *count,
+				       void *private, char *buf)
+{
+	struct stm32_timer_cnt *const priv = counter->priv;
+	u32 cr1;
+
+	regmap_read(priv->regmap, TIM_CR1, &cr1);
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n", (bool)(cr1 & TIM_CR1_CEN));
+}
+
+static ssize_t stm32_count_enable_write(struct counter_device *counter,
+					struct counter_count *count,
+					void *private,
+					const char *buf, size_t len)
+{
+	struct stm32_timer_cnt *const priv = counter->priv;
+	int err;
+	u32 cr1;
+	bool enable;
+
+	err = kstrtobool(buf, &enable);
+	if (err)
+		return err;
+
+	if (enable) {
+		regmap_read(priv->regmap, TIM_CR1, &cr1);
+			if (!(cr1 & TIM_CR1_CEN))
+				clk_enable(priv->clk);
+
+		regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN,
+				   TIM_CR1_CEN);
+	} else {
+		regmap_read(priv->regmap, TIM_CR1, &cr1);
+		regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+		if (cr1 & TIM_CR1_CEN)
+			clk_disable(priv->clk);
+	}
+
+	return len;
+}
+
+static const struct counter_count_ext stm32_count_ext[] = {
+	{
+		.name = "direction",
+		.read = stm32_count_direction_read,
+	},
+	{
+		.name = "enable",
+		.read = stm32_count_enable_read,
+		.write = stm32_count_enable_write
+	},
+	{
+		.name = "ceiling",
+		.read = stm32_count_ceiling_read,
+		.write = stm32_count_ceiling_write
+	},
+};
+
+enum stm32_synapse_action {
+	STM32_SYNAPSE_ACTION_NONE,
+	STM32_SYNAPSE_ACTION_BOTH_EDGES
+};
+
+static enum synapse_action stm32_synapse_actions[] = {
+	[STM32_SYNAPSE_ACTION_NONE] = SYNAPSE_ACTION_NONE,
+	[STM32_SYNAPSE_ACTION_BOTH_EDGES] = SYNAPSE_ACTION_BOTH_EDGES
+};
+
+static int stm32_action_get(struct counter_device *counter,
+			    struct counter_count *count,
+			    struct counter_synapse *synapse,
+			    size_t *action)
+{
+	size_t function;
+	int err;
+
+	/* Default action mode (e.g. STM32_COUNT_SLAVE_MODE_DISABLED) */
+	*action = STM32_SYNAPSE_ACTION_NONE;
+
+	err = stm32_count_function_get(counter, count, &function);
+	if (err)
+		return 0;
+
+	switch (function) {
+	case STM32_COUNT_ENCODER_MODE_1:
+		/* counts up/down on TI1FP1 edge depending on TI2FP2 level */
+		if (synapse->signal->id == count->synapses[0].signal->id)
+			*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
+		break;
+	case STM32_COUNT_ENCODER_MODE_2:
+		/* counts up/down on TI2FP2 edge depending on TI1FP1 level */
+		if (synapse->signal->id == count->synapses[1].signal->id)
+			*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
+		break;
+	case STM32_COUNT_ENCODER_MODE_3:
+		/* counts up/down on both TI1FP1 and TI2FP2 edges */
+		*action = STM32_SYNAPSE_ACTION_BOTH_EDGES;
+		break;
+	}
+
+	return 0;
+}
+
+static const struct counter_ops stm32_timer_cnt_ops = {
+	.count_read = stm32_count_read,
+	.count_write = stm32_count_write,
+	.function_get = stm32_count_function_get,
+	.function_set = stm32_count_function_set,
+	.action_get = stm32_action_get,
+};
+
+static struct counter_signal stm32_signals[] = {
+	{
+		.id = 0,
+		.name = "Channel 1 Quadrature A"
+	},
+	{
+		.id = 1,
+		.name = "Channel 1 Quadrature B"
+	}
+};
+
+static struct counter_synapse stm32_count_synapses[] = {
+	{
+		.actions_list = stm32_synapse_actions,
+		.num_actions = ARRAY_SIZE(stm32_synapse_actions),
+		.signal = &stm32_signals[0]
+	},
+	{
+		.actions_list = stm32_synapse_actions,
+		.num_actions = ARRAY_SIZE(stm32_synapse_actions),
+		.signal = &stm32_signals[1]
+	}
+};
+
+static struct counter_count stm32_counts = {
+	.id = 0,
+	.name = "Channel 1 Count",
+	.functions_list = stm32_count_functions,
+	.num_functions = ARRAY_SIZE(stm32_count_functions),
+	.synapses = stm32_count_synapses,
+	.num_synapses = ARRAY_SIZE(stm32_count_synapses),
+	.ext = stm32_count_ext,
+	.num_ext = ARRAY_SIZE(stm32_count_ext)
+};
+
+static int stm32_timer_cnt_probe(struct platform_device *pdev)
+{
+	struct stm32_timers *ddata = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct stm32_timer_cnt *priv;
+
+	if (IS_ERR_OR_NULL(ddata))
+		return -EINVAL;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->regmap = ddata->regmap;
+	priv->clk = ddata->clk;
+	priv->ceiling = ddata->max_arr;
+
+	priv->counter.name = dev_name(dev);
+	priv->counter.parent = dev;
+	priv->counter.ops = &stm32_timer_cnt_ops;
+	priv->counter.counts = &stm32_counts;
+	priv->counter.num_counts = 1;
+	priv->counter.signals = stm32_signals;
+	priv->counter.num_signals = ARRAY_SIZE(stm32_signals);
+	priv->counter.priv = priv;
+
+	/* Register Counter device */
+	return devm_counter_register(dev, &priv->counter);
+}
+
+static const struct of_device_id stm32_timer_cnt_of_match[] = {
+	{ .compatible = "st,stm32-timer-counter", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stm32_timer_cnt_of_match);
+
+static struct platform_driver stm32_timer_cnt_driver = {
+	.probe = stm32_timer_cnt_probe,
+	.driver = {
+		.name = "stm32-timer-counter",
+		.of_match_table = stm32_timer_cnt_of_match,
+	},
+};
+module_platform_driver(stm32_timer_cnt_driver);
+
+MODULE_AUTHOR("Benjamin Gaignard <[email protected]>");
+MODULE_ALIAS("platform:stm32-timer-counter");
+MODULE_DESCRIPTION("STMicroelectronics STM32 TIMER counter driver");
+MODULE_LICENSE("GPL v2");