public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver
@ 2026-05-01 20:07 Wadim Mueller
  2026-05-01 20:07 ` [PATCH v3 1/3] dt-bindings: counter: add gpio-quadrature-encoder binding Wadim Mueller
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Wadim Mueller @ 2026-05-01 20:07 UTC (permalink / raw)
  To: wbg
  Cc: conor+dt, krzk+dt, robh, conor.dooley, linux-iio, devicetree,
	linux-kernel

This series adds a new counter subsystem driver that implements
quadrature encoder position tracking using plain GPIO pins with
edge-triggered interrupts.

The driver is intended for low to medium speed rotary encoders where
hardware counter peripherals (eQEP, FTM, etc.) are unavailable or
already in use. It targets the same use-cases as interrupt-cnt.c but
provides full quadrature decoding instead of simple pulse counting.

Features:
  - X1, X2, X4 quadrature decoding and pulse-direction mode
  - Optional index signal for zero-reset
  - Configurable ceiling (position clamping)
  - Standard counter subsystem sysfs + chrdev interface
  - Enable/disable via sysfs with IRQ gating

Tested on TI AM64x (Cortex-A53) with a motor-driven rotary encoder
at up to 2 kHz quadrature edge rate.

Changes in v3:
  - Pick up Acked-by: Conor Dooley on the DT binding patch.
  - No code changes.

Changes in v2:
  - DT binding: rephrase description to describe hardware, not
    driver/sysfs behaviour (Conor Dooley)
  - DT binding: drop redundant example without index GPIO (Conor Dooley)

Wadim Mueller (3):
  dt-bindings: counter: add gpio-quadrature-encoder binding
  counter: add GPIO-based quadrature encoder driver
  MAINTAINERS: add entry for GPIO quadrature encoder counter driver

 .../counter/gpio-quadrature-encoder.yaml      |  60 ++
 MAINTAINERS                                   |   7 +
 drivers/counter/Kconfig                       |  15 +
 drivers/counter/Makefile                      |   1 +
 drivers/counter/gpio-quadrature-encoder.c     | 710 ++++++++++++++++++
 5 files changed, 793 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/counter/gpio-quadrature-encoder.yaml
 create mode 100644 drivers/counter/gpio-quadrature-encoder.c

-- 
2.52.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/3] dt-bindings: counter: add gpio-quadrature-encoder binding
  2026-05-01 20:07 [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver Wadim Mueller
@ 2026-05-01 20:07 ` Wadim Mueller
  2026-05-01 20:07 ` [PATCH v3 2/3] counter: add GPIO-based quadrature encoder driver Wadim Mueller
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Wadim Mueller @ 2026-05-01 20:07 UTC (permalink / raw)
  To: wbg
  Cc: conor+dt, krzk+dt, robh, conor.dooley, linux-iio, devicetree,
	linux-kernel

Add devicetree binding documentation for the GPIO-based quadrature
encoder counter driver. The driver reads A/B quadrature signals and
an optional index pulse via edge-triggered GPIO interrupts, supporting
X1, X2, X4 quadrature decoding and pulse-direction mode.

This is useful on SoCs that lack a dedicated hardware quadrature
decoder or where the encoder is wired to generic GPIO pins.

Signed-off-by: Wadim Mueller <wafgo01@gmail.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../counter/gpio-quadrature-encoder.yaml      | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/counter/gpio-quadrature-encoder.yaml

diff --git a/Documentation/devicetree/bindings/counter/gpio-quadrature-encoder.yaml b/Documentation/devicetree/bindings/counter/gpio-quadrature-encoder.yaml
new file mode 100644
index 000000000..741396b29
--- /dev/null
+++ b/Documentation/devicetree/bindings/counter/gpio-quadrature-encoder.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/counter/gpio-quadrature-encoder.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO-based Quadrature Encoder
+
+maintainers:
+  - Wadim Mueller <wadim.mueller@cmblu.de>
+
+description: |
+  A generic GPIO-based quadrature encoder counter.  Reads A/B quadrature
+  signals and an optional index pulse via edge-triggered GPIO interrupts.
+  Supports X1, X2, X4 quadrature decoding and pulse-direction mode.
+
+  This is useful on SoCs that lack a dedicated hardware quadrature
+  decoder (eQEP, QEI, etc.) or where the encoder is wired to generic
+  GPIO pins rather than to a dedicated peripheral.
+
+properties:
+  compatible:
+    const: gpio-quadrature-encoder
+
+  encoder-a-gpios:
+    maxItems: 1
+    description:
+      GPIO connected to the encoder's A (phase A) output.
+
+  encoder-b-gpios:
+    maxItems: 1
+    description:
+      GPIO connected to the encoder's B (phase B) output.
+
+  encoder-index-gpios:
+    maxItems: 1
+    description:
+      Optional GPIO connected to the encoder's index (Z) output.
+      The index signal pulses once per revolution and can be used
+      as a reference point for absolute position tracking.
+
+required:
+  - compatible
+  - encoder-a-gpios
+  - encoder-b-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    quadrature-encoder {
+        compatible = "gpio-quadrature-encoder";
+        encoder-a-gpios = <&gpio0 10 GPIO_ACTIVE_LOW>;
+        encoder-b-gpios = <&gpio0 11 GPIO_ACTIVE_LOW>;
+        encoder-index-gpios = <&gpio0 12 GPIO_ACTIVE_LOW>;
+    };
+
+...
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/3] counter: add GPIO-based quadrature encoder driver
  2026-05-01 20:07 [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver Wadim Mueller
  2026-05-01 20:07 ` [PATCH v3 1/3] dt-bindings: counter: add gpio-quadrature-encoder binding Wadim Mueller
@ 2026-05-01 20:07 ` Wadim Mueller
  2026-05-04 20:54   ` Krzysztof Kozlowski
  2026-05-01 20:07 ` [PATCH v3 3/3] MAINTAINERS: add entry for GPIO quadrature encoder counter driver Wadim Mueller
  2026-05-04  9:36 ` [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver William Breathitt Gray
  3 siblings, 1 reply; 8+ messages in thread
From: Wadim Mueller @ 2026-05-01 20:07 UTC (permalink / raw)
  To: wbg
  Cc: conor+dt, krzk+dt, robh, conor.dooley, linux-iio, devicetree,
	linux-kernel

Add a platform driver that turns ordinary GPIOs into a quadrature
encoder counter device.  The driver requests edge-triggered interrupts
on the A and B (and optional Index) GPIOs and decodes the quadrature
signal in software using a classic state-table approach.

Supported counting modes:
  - Quadrature X1 (count on A rising edge only)
  - Quadrature X2 (count on both A edges)
  - Quadrature X4 (count on every A and B edge)
  - Pulse-direction (A = pulse, B = direction)

An optional index signal resets the count to zero on its rising edge
when enabled through sysfs.  A configurable ceiling clamps the count
to [0, ceiling].

Signed-off-by: Wadim Mueller <wafgo01@gmail.com>
---
 drivers/counter/Kconfig                   |  15 +
 drivers/counter/Makefile                  |   1 +
 drivers/counter/gpio-quadrature-encoder.c | 710 ++++++++++++++++++++++
 3 files changed, 726 insertions(+)
 create mode 100644 drivers/counter/gpio-quadrature-encoder.c

diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
index d30d22dfe..72c5c8159 100644
--- a/drivers/counter/Kconfig
+++ b/drivers/counter/Kconfig
@@ -68,6 +68,21 @@ config INTEL_QEP
 	  To compile this driver as a module, choose M here: the module
 	  will be called intel-qep.
 
+config GPIO_QUADRATURE_ENCODER
+	tristate "GPIO-based quadrature encoder counter driver"
+	depends on GPIOLIB
+	help
+	  Select this option to enable the GPIO-based quadrature encoder
+	  counter driver.  It reads A/B quadrature signals and an optional
+	  index pulse via edge-triggered GPIO interrupts, supporting X1, X2,
+	  X4 quadrature decoding and pulse-direction mode.
+
+	  This is useful on SoCs that lack a dedicated hardware quadrature
+	  decoder or where the encoder is wired to generic GPIO pins.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called gpio-quadrature-encoder.
+
 config INTERRUPT_CNT
 	tristate "Interrupt counter driver"
 	depends on GPIOLIB
diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
index fa3c1d08f..2bef64d10 100644
--- a/drivers/counter/Makefile
+++ b/drivers/counter/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_STM32_TIMER_CNT)	+= stm32-timer-cnt.o
 obj-$(CONFIG_STM32_LPTIMER_CNT)	+= stm32-lptimer-cnt.o
 obj-$(CONFIG_TI_EQEP)		+= ti-eqep.o
 obj-$(CONFIG_FTM_QUADDEC)	+= ftm-quaddec.o
+obj-$(CONFIG_GPIO_QUADRATURE_ENCODER)	+= gpio-quadrature-encoder.o
 obj-$(CONFIG_MICROCHIP_TCB_CAPTURE)	+= microchip-tcb-capture.o
 obj-$(CONFIG_INTEL_QEP)		+= intel-qep.o
 obj-$(CONFIG_TI_ECAP_CAPTURE)	+= ti-ecap-capture.o
diff --git a/drivers/counter/gpio-quadrature-encoder.c b/drivers/counter/gpio-quadrature-encoder.c
new file mode 100644
index 000000000..0822f0a8a
--- /dev/null
+++ b/drivers/counter/gpio-quadrature-encoder.c
@@ -0,0 +1,710 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPIO-based Quadrature Encoder Counter Driver
+ *
+ * Reads quadrature encoder signals (A, B, and optional Index) via GPIOs.
+ * Supports X1, X2, X4 quadrature decoding and pulse-direction mode.
+ *
+ * Copyright (C) 2026 CMBlu Energy AG
+ * Author: Wadim Mueller <wafgo01@gmail.com>
+ */
+
+#include <linux/counter.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+enum gpio_qenc_function {
+	GPIO_QENC_FUNC_QUAD_X1 = 0,
+	GPIO_QENC_FUNC_QUAD_X2,
+	GPIO_QENC_FUNC_QUAD_X4,
+	GPIO_QENC_FUNC_PULSE_DIR,
+};
+
+enum gpio_qenc_signal_id {
+	GPIO_QENC_SIGNAL_A = 0,
+	GPIO_QENC_SIGNAL_B,
+	GPIO_QENC_SIGNAL_INDEX,
+};
+
+struct gpio_qenc_priv {
+	struct gpio_desc *gpio_a;
+	struct gpio_desc *gpio_b;
+	struct gpio_desc *gpio_index;
+
+	int irq_a;
+	int irq_b;
+	int irq_index;
+
+	spinlock_t lock;
+
+	s64 count;
+	u64 ceiling;
+	bool enabled;
+	enum counter_count_direction direction;
+	enum gpio_qenc_function function;
+
+	int prev_a;
+	int prev_b;
+
+	bool index_enabled;
+
+	struct counter_signal signals[3];
+	struct counter_synapse synapses[3];
+	struct counter_count cnts;
+};
+
+/*
+ * Quadrature state table for X4 decoding.
+ * Rows = previous state (A<<1 | B), Columns = new state (A<<1 | B).
+ * Values: 0 = no change, +1 = forward, -1 = backward, 2 = error (skip).
+ */
+static const int quad_table[4][4] = {
+	/*          00  01  10  11  <- new */
+	/* 00 */ {  0, -1,  1,  2 },
+	/* 01 */ {  1,  0,  2, -1 },
+	/* 10 */ { -1,  2,  0,  1 },
+	/* 11 */ {  2,  1, -1,  0 },
+};
+
+static void gpio_qenc_update_count(struct gpio_qenc_priv *priv, int delta)
+{
+	s64 new_count;
+
+	if (!delta)
+		return;
+
+	new_count = priv->count + delta;
+
+	if (priv->ceiling) {
+		if (new_count < 0)
+			new_count = 0;
+		else if (new_count > (s64)priv->ceiling)
+			new_count = priv->ceiling;
+	}
+
+	priv->count = new_count;
+	priv->direction = (delta > 0) ? COUNTER_COUNT_DIRECTION_FORWARD
+				      : COUNTER_COUNT_DIRECTION_BACKWARD;
+}
+
+static irqreturn_t gpio_qenc_a_isr(int irq, void *dev_id)
+{
+	struct counter_device *counter = dev_id;
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+	int a, b, prev_state, new_state, delta;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	if (!priv->enabled)
+		goto out;
+
+	a = gpiod_get_value(priv->gpio_a);
+	b = gpiod_get_value(priv->gpio_b);
+
+	prev_state = (priv->prev_a << 1) | priv->prev_b;
+	new_state = (a << 1) | b;
+
+	switch (priv->function) {
+	case GPIO_QENC_FUNC_QUAD_X4:
+		delta = quad_table[prev_state][new_state];
+		if (delta == 2)
+			delta = 0;
+		gpio_qenc_update_count(priv, delta);
+		break;
+
+	case GPIO_QENC_FUNC_QUAD_X2:
+		delta = quad_table[prev_state][new_state];
+		if (delta == 2)
+			delta = 0;
+		gpio_qenc_update_count(priv, delta);
+		break;
+
+	case GPIO_QENC_FUNC_QUAD_X1:
+		if (!priv->prev_a && a) {
+			delta = b ? -1 : 1;
+			gpio_qenc_update_count(priv, delta);
+		}
+		break;
+
+	case GPIO_QENC_FUNC_PULSE_DIR:
+		if (!priv->prev_a && a) {
+			delta = b ? -1 : 1;
+			gpio_qenc_update_count(priv, delta);
+		}
+		break;
+	}
+
+	priv->prev_a = a;
+	priv->prev_b = b;
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	counter_push_event(counter, COUNTER_EVENT_CHANGE_OF_STATE, 0);
+
+	return IRQ_HANDLED;
+
+out:
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t gpio_qenc_b_isr(int irq, void *dev_id)
+{
+	struct counter_device *counter = dev_id;
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+	int a, b, prev_state, new_state, delta;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	if (!priv->enabled)
+		goto out;
+
+	a = gpiod_get_value(priv->gpio_a);
+	b = gpiod_get_value(priv->gpio_b);
+
+	prev_state = (priv->prev_a << 1) | priv->prev_b;
+	new_state = (a << 1) | b;
+
+	switch (priv->function) {
+	case GPIO_QENC_FUNC_QUAD_X4:
+		delta = quad_table[prev_state][new_state];
+		if (delta == 2)
+			delta = 0;
+		gpio_qenc_update_count(priv, delta);
+		break;
+
+	case GPIO_QENC_FUNC_QUAD_X2:
+		/* X2: only A-channel edges update count */
+		break;
+
+	case GPIO_QENC_FUNC_QUAD_X1:
+	case GPIO_QENC_FUNC_PULSE_DIR:
+		break;
+	}
+
+	priv->prev_a = a;
+	priv->prev_b = b;
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return IRQ_HANDLED;
+
+out:
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t gpio_qenc_index_isr(int irq, void *dev_id)
+{
+	struct counter_device *counter = dev_id;
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	if (priv->enabled && priv->index_enabled)
+		priv->count = 0;
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	counter_push_event(counter, COUNTER_EVENT_INDEX, 0);
+
+	return IRQ_HANDLED;
+}
+
+static int gpio_qenc_count_read(struct counter_device *counter,
+				struct counter_count *count, u64 *val)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	*val = (u64)priv->count;
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int gpio_qenc_count_write(struct counter_device *counter,
+				 struct counter_count *count, const u64 val)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	if (priv->ceiling && val > priv->ceiling) {
+		spin_unlock_irqrestore(&priv->lock, flags);
+		return -EINVAL;
+	}
+
+	priv->count = (s64)val;
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static const enum counter_function gpio_qenc_functions[] = {
+	COUNTER_FUNCTION_QUADRATURE_X1_A,
+	COUNTER_FUNCTION_QUADRATURE_X2_A,
+	COUNTER_FUNCTION_QUADRATURE_X4,
+	COUNTER_FUNCTION_PULSE_DIRECTION,
+};
+
+static int gpio_qenc_function_read(struct counter_device *counter,
+				   struct counter_count *count,
+				   enum counter_function *function)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	switch (priv->function) {
+	case GPIO_QENC_FUNC_QUAD_X1:
+		*function = COUNTER_FUNCTION_QUADRATURE_X1_A;
+		break;
+	case GPIO_QENC_FUNC_QUAD_X2:
+		*function = COUNTER_FUNCTION_QUADRATURE_X2_A;
+		break;
+	case GPIO_QENC_FUNC_QUAD_X4:
+		*function = COUNTER_FUNCTION_QUADRATURE_X4;
+		break;
+	case GPIO_QENC_FUNC_PULSE_DIR:
+		*function = COUNTER_FUNCTION_PULSE_DIRECTION;
+		break;
+	}
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return 0;
+}
+
+static int gpio_qenc_function_write(struct counter_device *counter,
+				    struct counter_count *count,
+				    enum counter_function function)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	switch (function) {
+	case COUNTER_FUNCTION_QUADRATURE_X1_A:
+		priv->function = GPIO_QENC_FUNC_QUAD_X1;
+		break;
+	case COUNTER_FUNCTION_QUADRATURE_X2_A:
+		priv->function = GPIO_QENC_FUNC_QUAD_X2;
+		break;
+	case COUNTER_FUNCTION_QUADRATURE_X4:
+		priv->function = GPIO_QENC_FUNC_QUAD_X4;
+		break;
+	case COUNTER_FUNCTION_PULSE_DIRECTION:
+		priv->function = GPIO_QENC_FUNC_PULSE_DIR;
+		break;
+	default:
+		spin_unlock_irqrestore(&priv->lock, flags);
+		return -EINVAL;
+	}
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+	return 0;
+}
+
+static const enum counter_synapse_action gpio_qenc_synapse_actions[] = {
+	COUNTER_SYNAPSE_ACTION_BOTH_EDGES,
+	COUNTER_SYNAPSE_ACTION_RISING_EDGE,
+	COUNTER_SYNAPSE_ACTION_NONE,
+};
+
+static int gpio_qenc_action_read(struct counter_device *counter,
+				 struct counter_count *count,
+				 struct counter_synapse *synapse,
+				 enum counter_synapse_action *action)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	enum gpio_qenc_signal_id signal_id = synapse->signal->id;
+
+	switch (priv->function) {
+	case GPIO_QENC_FUNC_QUAD_X4:
+		if (signal_id == GPIO_QENC_SIGNAL_A ||
+		    signal_id == GPIO_QENC_SIGNAL_B)
+			*action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;
+		else
+			*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
+		return 0;
+
+	case GPIO_QENC_FUNC_QUAD_X2:
+		if (signal_id == GPIO_QENC_SIGNAL_A)
+			*action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;
+		else if (signal_id == GPIO_QENC_SIGNAL_B)
+			*action = COUNTER_SYNAPSE_ACTION_NONE;
+		else
+			*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
+		return 0;
+
+	case GPIO_QENC_FUNC_QUAD_X1:
+		if (signal_id == GPIO_QENC_SIGNAL_A)
+			*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
+		else if (signal_id == GPIO_QENC_SIGNAL_B)
+			*action = COUNTER_SYNAPSE_ACTION_NONE;
+		else
+			*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
+		return 0;
+
+	case GPIO_QENC_FUNC_PULSE_DIR:
+		if (signal_id == GPIO_QENC_SIGNAL_A)
+			*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
+		else
+			*action = COUNTER_SYNAPSE_ACTION_NONE;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int gpio_qenc_signal_read(struct counter_device *counter,
+				 struct counter_signal *signal,
+				 enum counter_signal_level *level)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	struct gpio_desc *gpio;
+	int ret;
+
+	switch (signal->id) {
+	case GPIO_QENC_SIGNAL_A:
+		gpio = priv->gpio_a;
+		break;
+	case GPIO_QENC_SIGNAL_B:
+		gpio = priv->gpio_b;
+		break;
+	case GPIO_QENC_SIGNAL_INDEX:
+		gpio = priv->gpio_index;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (!gpio)
+		return -EINVAL;
+
+	ret = gpiod_get_value(gpio);
+	if (ret < 0)
+		return ret;
+
+	*level = ret ? COUNTER_SIGNAL_LEVEL_HIGH : COUNTER_SIGNAL_LEVEL_LOW;
+	return 0;
+}
+
+static int gpio_qenc_events_configure(struct counter_device *counter)
+{
+	return 0;
+}
+
+static int gpio_qenc_watch_validate(struct counter_device *counter,
+				    const struct counter_watch *watch)
+{
+	if (watch->channel != 0)
+		return -EINVAL;
+
+	switch (watch->event) {
+	case COUNTER_EVENT_CHANGE_OF_STATE:
+	case COUNTER_EVENT_INDEX:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct counter_ops gpio_qenc_ops = {
+	.count_read	= gpio_qenc_count_read,
+	.count_write	= gpio_qenc_count_write,
+	.function_read	= gpio_qenc_function_read,
+	.function_write	= gpio_qenc_function_write,
+	.action_read	= gpio_qenc_action_read,
+	.signal_read	= gpio_qenc_signal_read,
+	.events_configure = gpio_qenc_events_configure,
+	.watch_validate	= gpio_qenc_watch_validate,
+};
+
+static int gpio_qenc_ceiling_read(struct counter_device *counter,
+				  struct counter_count *count, u64 *val)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	*val = priv->ceiling;
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int gpio_qenc_ceiling_write(struct counter_device *counter,
+				   struct counter_count *count, const u64 val)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	priv->ceiling = val;
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int gpio_qenc_enable_read(struct counter_device *counter,
+				 struct counter_count *count, u8 *enable)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+
+	*enable = priv->enabled;
+	return 0;
+}
+
+static int gpio_qenc_enable_write(struct counter_device *counter,
+				  struct counter_count *count, u8 enable)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	if (priv->enabled == !!enable) {
+		spin_unlock_irqrestore(&priv->lock, flags);
+		return 0;
+	}
+
+	if (enable) {
+		priv->enabled = true;
+		spin_unlock_irqrestore(&priv->lock, flags);
+		enable_irq(priv->irq_a);
+		enable_irq(priv->irq_b);
+		if (priv->irq_index)
+			enable_irq(priv->irq_index);
+	} else {
+		priv->enabled = false;
+		spin_unlock_irqrestore(&priv->lock, flags);
+		disable_irq(priv->irq_a);
+		disable_irq(priv->irq_b);
+		if (priv->irq_index)
+			disable_irq(priv->irq_index);
+	}
+
+	return 0;
+}
+
+static int gpio_qenc_direction_read(struct counter_device *counter,
+				    struct counter_count *count, u32 *direction)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	*direction = priv->direction;
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int gpio_qenc_index_enable_read(struct counter_device *counter,
+				       struct counter_count *count, u8 *val)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+
+	*val = priv->index_enabled;
+	return 0;
+}
+
+static int gpio_qenc_index_enable_write(struct counter_device *counter,
+					struct counter_count *count, u8 val)
+{
+	struct gpio_qenc_priv *priv = counter_priv(counter);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	priv->index_enabled = !!val;
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static struct counter_comp gpio_qenc_count_ext[] = {
+	COUNTER_COMP_CEILING(gpio_qenc_ceiling_read, gpio_qenc_ceiling_write),
+	COUNTER_COMP_ENABLE(gpio_qenc_enable_read, gpio_qenc_enable_write),
+	COUNTER_COMP_DIRECTION(gpio_qenc_direction_read),
+	COUNTER_COMP_COUNT_BOOL("index_enabled",
+				gpio_qenc_index_enable_read,
+				gpio_qenc_index_enable_write),
+};
+
+static int gpio_qenc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct counter_device *counter;
+	struct gpio_qenc_priv *priv;
+	bool has_index;
+	int num_signals;
+	int num_synapses;
+	int ret;
+
+	counter = devm_counter_alloc(dev, sizeof(*priv));
+	if (!counter)
+		return -ENOMEM;
+
+	priv = counter_priv(counter);
+	spin_lock_init(&priv->lock);
+
+	priv->gpio_a = devm_gpiod_get(dev, "encoder-a", GPIOD_IN);
+	if (IS_ERR(priv->gpio_a))
+		return dev_err_probe(dev, PTR_ERR(priv->gpio_a),
+				     "failed to get encoder-a GPIO\n");
+
+	priv->gpio_b = devm_gpiod_get(dev, "encoder-b", GPIOD_IN);
+	if (IS_ERR(priv->gpio_b))
+		return dev_err_probe(dev, PTR_ERR(priv->gpio_b),
+				     "failed to get encoder-b GPIO\n");
+
+	priv->gpio_index = devm_gpiod_get_optional(dev, "encoder-index",
+						    GPIOD_IN);
+	if (IS_ERR(priv->gpio_index))
+		return dev_err_probe(dev, PTR_ERR(priv->gpio_index),
+				     "failed to get encoder-index GPIO\n");
+
+	has_index = !!priv->gpio_index;
+
+	priv->irq_a = gpiod_to_irq(priv->gpio_a);
+	if (priv->irq_a < 0)
+		return dev_err_probe(dev, priv->irq_a,
+				     "failed to get IRQ for encoder-a\n");
+
+	priv->irq_b = gpiod_to_irq(priv->gpio_b);
+	if (priv->irq_b < 0)
+		return dev_err_probe(dev, priv->irq_b,
+				     "failed to get IRQ for encoder-b\n");
+
+	if (has_index) {
+		priv->irq_index = gpiod_to_irq(priv->gpio_index);
+		if (priv->irq_index < 0)
+			return dev_err_probe(dev, priv->irq_index,
+					     "failed to get IRQ for encoder-index\n");
+	}
+
+	priv->prev_a = gpiod_get_value(priv->gpio_a);
+	priv->prev_b = gpiod_get_value(priv->gpio_b);
+
+	priv->function = GPIO_QENC_FUNC_QUAD_X4;
+	priv->direction = COUNTER_COUNT_DIRECTION_FORWARD;
+
+	num_signals = has_index ? 3 : 2;
+
+	priv->signals[GPIO_QENC_SIGNAL_A].id = GPIO_QENC_SIGNAL_A;
+	priv->signals[GPIO_QENC_SIGNAL_A].name = "Signal A";
+
+	priv->signals[GPIO_QENC_SIGNAL_B].id = GPIO_QENC_SIGNAL_B;
+	priv->signals[GPIO_QENC_SIGNAL_B].name = "Signal B";
+
+	if (has_index) {
+		priv->signals[GPIO_QENC_SIGNAL_INDEX].id =
+			GPIO_QENC_SIGNAL_INDEX;
+		priv->signals[GPIO_QENC_SIGNAL_INDEX].name = "Index";
+	}
+
+	num_synapses = num_signals;
+
+	priv->synapses[0].actions_list = gpio_qenc_synapse_actions;
+	priv->synapses[0].num_actions = ARRAY_SIZE(gpio_qenc_synapse_actions);
+	priv->synapses[0].signal = &priv->signals[GPIO_QENC_SIGNAL_A];
+
+	priv->synapses[1].actions_list = gpio_qenc_synapse_actions;
+	priv->synapses[1].num_actions = ARRAY_SIZE(gpio_qenc_synapse_actions);
+	priv->synapses[1].signal = &priv->signals[GPIO_QENC_SIGNAL_B];
+
+	if (has_index) {
+		priv->synapses[2].actions_list = gpio_qenc_synapse_actions;
+		priv->synapses[2].num_actions =
+			ARRAY_SIZE(gpio_qenc_synapse_actions);
+		priv->synapses[2].signal =
+			&priv->signals[GPIO_QENC_SIGNAL_INDEX];
+	}
+
+	priv->cnts.id = 0;
+	priv->cnts.name = "Position";
+	priv->cnts.functions_list = gpio_qenc_functions;
+	priv->cnts.num_functions = ARRAY_SIZE(gpio_qenc_functions);
+	priv->cnts.synapses = priv->synapses;
+	priv->cnts.num_synapses = num_synapses;
+	priv->cnts.ext = gpio_qenc_count_ext;
+	priv->cnts.num_ext = ARRAY_SIZE(gpio_qenc_count_ext);
+
+	counter->name = dev_name(dev);
+	counter->parent = dev;
+	counter->ops = &gpio_qenc_ops;
+	counter->signals = priv->signals;
+	counter->num_signals = num_signals;
+	counter->counts = &priv->cnts;
+	counter->num_counts = 1;
+
+	irq_set_status_flags(priv->irq_a, IRQ_NOAUTOEN);
+	ret = devm_request_irq(dev, priv->irq_a, gpio_qenc_a_isr,
+			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			       "gpio-qenc-a", counter);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to request IRQ for encoder-a\n");
+
+	irq_set_status_flags(priv->irq_b, IRQ_NOAUTOEN);
+	ret = devm_request_irq(dev, priv->irq_b, gpio_qenc_b_isr,
+			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			       "gpio-qenc-b", counter);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to request IRQ for encoder-b\n");
+
+	if (has_index) {
+		irq_set_status_flags(priv->irq_index, IRQ_NOAUTOEN);
+		ret = devm_request_irq(dev, priv->irq_index,
+				       gpio_qenc_index_isr,
+				       IRQF_TRIGGER_RISING,
+				       "gpio-qenc-index", counter);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to request IRQ for encoder-index\n");
+	}
+
+	ret = devm_counter_add(dev, counter);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to add counter\n");
+
+	dev_info(dev, "GPIO quadrature encoder registered (signals: A, B%s)\n",
+		 has_index ? ", Index" : "");
+
+	return 0;
+}
+
+static const struct of_device_id gpio_qenc_of_match[] = {
+	{ .compatible = "gpio-quadrature-encoder" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, gpio_qenc_of_match);
+
+static struct platform_driver gpio_qenc_driver = {
+	.probe = gpio_qenc_probe,
+	.driver = {
+		.name = "gpio-quadrature-encoder",
+		.of_match_table = gpio_qenc_of_match,
+	},
+};
+module_platform_driver(gpio_qenc_driver);
+
+MODULE_ALIAS("platform:gpio-quadrature-encoder");
+MODULE_AUTHOR("Wadim Mueller <wafgo01@gmail.com>");
+MODULE_DESCRIPTION("GPIO-based quadrature encoder counter driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("COUNTER");
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] MAINTAINERS: add entry for GPIO quadrature encoder counter driver
  2026-05-01 20:07 [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver Wadim Mueller
  2026-05-01 20:07 ` [PATCH v3 1/3] dt-bindings: counter: add gpio-quadrature-encoder binding Wadim Mueller
  2026-05-01 20:07 ` [PATCH v3 2/3] counter: add GPIO-based quadrature encoder driver Wadim Mueller
@ 2026-05-01 20:07 ` Wadim Mueller
  2026-05-04  9:36 ` [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver William Breathitt Gray
  3 siblings, 0 replies; 8+ messages in thread
From: Wadim Mueller @ 2026-05-01 20:07 UTC (permalink / raw)
  To: wbg
  Cc: conor+dt, krzk+dt, robh, conor.dooley, linux-iio, devicetree,
	linux-kernel

Add myself as maintainer for the new gpio-quadrature-encoder counter
driver and its devicetree binding.

Signed-off-by: Wadim Mueller <wafgo01@gmail.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 06a8c7457..fca62baa7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11018,6 +11018,13 @@ F:	Documentation/dev-tools/gpio-sloppy-logic-analyzer.rst
 F:	drivers/gpio/gpio-sloppy-logic-analyzer.c
 F:	tools/gpio/gpio-sloppy-logic-analyzer.sh
 
+GPIO QUADRATURE ENCODER COUNTER DRIVER
+M:	Wadim Mueller <wafgo01@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/counter/gpio-quadrature-encoder.yaml
+F:	drivers/counter/gpio-quadrature-encoder.c
+
 GPIO SUBSYSTEM
 M:	Linus Walleij <linusw@kernel.org>
 M:	Bartosz Golaszewski <brgl@kernel.org>
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver
  2026-05-01 20:07 [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver Wadim Mueller
                   ` (2 preceding siblings ...)
  2026-05-01 20:07 ` [PATCH v3 3/3] MAINTAINERS: add entry for GPIO quadrature encoder counter driver Wadim Mueller
@ 2026-05-04  9:36 ` William Breathitt Gray
  2026-05-04 19:37   ` Wadim Mueller
  3 siblings, 1 reply; 8+ messages in thread
From: William Breathitt Gray @ 2026-05-04  9:36 UTC (permalink / raw)
  To: Wadim Mueller
  Cc: William Breathitt Gray, linusw, brgl, conor+dt, krzk+dt, robh,
	conor.dooley, linux-iio, devicetree, linux-kernel

On Fri, May 01, 2026 at 10:07:46PM +0200, Wadim Mueller wrote:
> This series adds a new counter subsystem driver that implements
> quadrature encoder position tracking using plain GPIO pins with
> edge-triggered interrupts.
> 
> The driver is intended for low to medium speed rotary encoders where
> hardware counter peripherals (eQEP, FTM, etc.) are unavailable or
> already in use. It targets the same use-cases as interrupt-cnt.c but
> provides full quadrature decoding instead of simple pulse counting.
> 
> Features:
>   - X1, X2, X4 quadrature decoding and pulse-direction mode
>   - Optional index signal for zero-reset
>   - Configurable ceiling (position clamping)
>   - Standard counter subsystem sysfs + chrdev interface
>   - Enable/disable via sysfs with IRQ gating
> 
> Tested on TI AM64x (Cortex-A53) with a motor-driven rotary encoder
> at up to 2 kHz quadrature edge rate.

Hello Wadim,

This is certainly a neat idea! :-) Several times I have wished for a
convenient way to just plug in a quadrature encoder to the GPIO lines of
my system and immediately start reading position data. However, I want
to be sure this makes sense as a Counter subsystem driver before I
proceed with a full review.

If I understand correctly from my brief overview, the core approach in
the gpio-quadrature-encoder module is to take two GPIO lines (A and B),
setup interrupt service routines for them, compare their GPIO values on
each interrupt, and respectively update a persistent count based on the
quadrature relationship.

From that description, I don't immediately see a need for this to occur
in kernelspace. Couldn't the same design be accomplished effectively in
userspace via the libgpiod API[^1]? I believe that library allows you
to watch for GPIO edge events and request GPIO line values. (I'm CCing
the GPIO subsystem maintainers in case I'm missing something obvious
here.)

Although the Counter subsystem does provide an established user
interface for counter devices, I'm not sure that alone justifies a
kernel driver when the same can be achieved by an equivalent userspace
application. If you can argue for why this should exist in the kernel,
I'll feel more comfortable with accepting the Counter subsystem as the
right home for the gpio-quadrature-encoder module.

> Changes in v3:
>   - Pick up Acked-by: Conor Dooley on the DT binding patch.
>   - No code changes.

As an aside, you don't need to resend the patchset if there are no code
changes, I'll make sure to pick up the tags in the mail threads when the
patches are accepted. This helps reduce the amount the messages we need
to parse on the mailing list.

Thanks,

William Breathitt Gray

[^1] https://libgpiod.readthedocs.io/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver
  2026-05-04  9:36 ` [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver William Breathitt Gray
@ 2026-05-04 19:37   ` Wadim Mueller
  0 siblings, 0 replies; 8+ messages in thread
From: Wadim Mueller @ 2026-05-04 19:37 UTC (permalink / raw)
  To: William Breathitt Gray
  Cc: linusw, brgl, conor+dt, krzk+dt, robh, conor.dooley, linux-iio,
	devicetree, linux-kernel

On 2026-05-04 18:36, William Breathitt Gray wrote:
> On Fri, May 01, 2026 at 10:07:46PM +0200, Wadim Mueller wrote:
> > This series adds a new counter subsystem driver that implements
> > quadrature encoder position tracking using plain GPIO pins with
> > edge-triggered interrupts.
> > 
> > The driver is intended for low to medium speed rotary encoders where
> > hardware counter peripherals (eQEP, FTM, etc.) are unavailable or
> > already in use. It targets the same use-cases as interrupt-cnt.c but
> > provides full quadrature decoding instead of simple pulse counting.
> > 
> > Features:
> >   - X1, X2, X4 quadrature decoding and pulse-direction mode
> >   - Optional index signal for zero-reset
> >   - Configurable ceiling (position clamping)
> >   - Standard counter subsystem sysfs + chrdev interface
> >   - Enable/disable via sysfs with IRQ gating
> > 
> > Tested on TI AM64x (Cortex-A53) with a motor-driven rotary encoder
> > at up to 2 kHz quadrature edge rate.
> 
> Hello Wadim,
> 
> This is certainly a neat idea! :-) Several times I have wished for a
> convenient way to just plug in a quadrature encoder to the GPIO lines of
> my system and immediately start reading position data. However, I want
> to be sure this makes sense as a Counter subsystem driver before I
> proceed with a full review.
> 
> If I understand correctly from my brief overview, the core approach in
> the gpio-quadrature-encoder module is to take two GPIO lines (A and B),
> setup interrupt service routines for them, compare their GPIO values on
> each interrupt, and respectively update a persistent count based on the
> quadrature relationship.
> 
> From that description, I don't immediately see a need for this to occur
> in kernelspace. Couldn't the same design be accomplished effectively in
> userspace via the libgpiod API[^1]? I believe that library allows you
> to watch for GPIO edge events and request GPIO line values. (I'm CCing
> the GPIO subsystem maintainers in case I'm missing something obvious
> here.)
> 
> Although the Counter subsystem does provide an established user
> interface for counter devices, I'm not sure that alone justifies a
> kernel driver when the same can be achieved by an equivalent userspace
> application. If you can argue for why this should exist in the kernel,
> I'll feel more comfortable with accepting the Counter subsystem as the
> right home for the gpio-quadrature-encoder module.
>

Hello William,

thanks for the look -- fair question. Let me try.

Even with A and B in a single line request (so one event fd, no extra
get_value() between edges) the libgpiod path is:

  edge IRQ -> kernel edge event -> chardev fifo -> scheduler wakeup
    -> read() -> decode -> update count

The wake-to-update delay is bounded by the scheduler, not by the IRQ.
At 2 kHz X4 the inter-edge spacing is ~125 us, which is firmly inside
scheduler-jitter territory on a stock kernel under load -- and a single
missed edge is a permanent position error rather than a transient
glitch. In the kernel the same work is essentially
spin_lock_irqsave / gpiod_get_value / count update / unlock in
interrupt context, which is what gives quadrature decoding the
atomicity it needs.

To be honest I haven't put real numbers on this yet -- the argument
above is from experience, not measurement. I'm going to set up a
proper back-to-back test (libgpiod v2 decoder vs the in-kernel driver,
same hardware, deterministic edge source via eHRPWM so the generator
side doesn't pollute the result) and follow up on this thread with
the results once they're in. If userspace turns out to hold up under
load on this SoC I'll happily drop the series.

The other angle: interrupt-cnt.c is the same shape of driver -- one
GPIO IRQ feeding a Counter -- and quadrature off two GPIOs feels more
like a sibling of that than something to push out to userspace.
Keeping both behind the Counter ABI also means downstream tooling
doesn't need a parallel libgpiod path next to the existing Counter one.

> > Changes in v3:
> >   - Pick up Acked-by: Conor Dooley on the DT binding patch.
> >   - No code changes.
> 
> As an aside, you don't need to resend the patchset if there are no code
> changes, I'll make sure to pick up the tags in the mail threads when the
> patches are accepted. This helps reduce the amount the messages we need
> to parse on the mailing list.

Got it on the resend, thanks. I'll just collect tags in-thread next time.
Thanks,

Wadim

> 
> Thanks,
> 
> William Breathitt Gray
> 
> [^1] https://libgpiod.readthedocs.io/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/3] counter: add GPIO-based quadrature encoder driver
  2026-05-01 20:07 ` [PATCH v3 2/3] counter: add GPIO-based quadrature encoder driver Wadim Mueller
@ 2026-05-04 20:54   ` Krzysztof Kozlowski
  2026-05-04 21:15     ` Wadim Mueller
  0 siblings, 1 reply; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-05-04 20:54 UTC (permalink / raw)
  To: Wadim Mueller, wbg
  Cc: conor+dt, krzk+dt, robh, conor.dooley, linux-iio, devicetree,
	linux-kernel

On 01/05/2026 22:07, Wadim Mueller wrote:
> +
> +static int gpio_qenc_count_read(struct counter_device *counter,
> +				struct counter_count *count, u64 *val)
> +{
> +	struct gpio_qenc_priv *priv = counter_priv(counter);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	*val = (u64)priv->count;
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int gpio_qenc_count_write(struct counter_device *counter,
> +				 struct counter_count *count, const u64 val)

Please don't continue this broken 'const scalar' pattern. You probably
copied this code, but no such new code should be ever added.

It's not necessary - compiler/preprocessor does not care from function
signature point of view. It's not helping - it's scalar and no sane code
modifies such argument, thus there is no single need to protect it. It's
not making code easier to read. Quite opposite: raises eyebrows for no
real reason.

Same in few other places.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/3] counter: add GPIO-based quadrature encoder driver
  2026-05-04 20:54   ` Krzysztof Kozlowski
@ 2026-05-04 21:15     ` Wadim Mueller
  0 siblings, 0 replies; 8+ messages in thread
From: Wadim Mueller @ 2026-05-04 21:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: wbg, conor+dt, krzk+dt, robh, conor.dooley, linux-iio, devicetree,
	linux-kernel

On 2026-05-04 22:54, Krzysztof Kozlowski wrote:
> On 01/05/2026 22:07, Wadim Mueller wrote:
> > +
> > +static int gpio_qenc_count_read(struct counter_device *counter,
> > +				struct counter_count *count, u64 *val)
> > +{
> > +	struct gpio_qenc_priv *priv = counter_priv(counter);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&priv->lock, flags);
> > +	*val = (u64)priv->count;
> > +	spin_unlock_irqrestore(&priv->lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int gpio_qenc_count_write(struct counter_device *counter,
> > +				 struct counter_count *count, const u64 val)
> 
> Please don't continue this broken 'const scalar' pattern. You probably
> copied this code, but no such new code should be ever added.
> 
> It's not necessary - compiler/preprocessor does not care from function
> signature point of view. It's not helping - it's scalar and no sane code
> modifies such argument, thus there is no single need to protect it. It's
> not making code easier to read. Quite opposite: raises eyebrows for no
> real reason.
> 
> Same in few other places.
> 

You're right -- I picked it up from the existing counter callbacks
without thinking. I'll drop the const from the two scalar callbacks
(count_write and ceiling_write) for v4.

> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-04 21:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-01 20:07 [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver Wadim Mueller
2026-05-01 20:07 ` [PATCH v3 1/3] dt-bindings: counter: add gpio-quadrature-encoder binding Wadim Mueller
2026-05-01 20:07 ` [PATCH v3 2/3] counter: add GPIO-based quadrature encoder driver Wadim Mueller
2026-05-04 20:54   ` Krzysztof Kozlowski
2026-05-04 21:15     ` Wadim Mueller
2026-05-01 20:07 ` [PATCH v3 3/3] MAINTAINERS: add entry for GPIO quadrature encoder counter driver Wadim Mueller
2026-05-04  9:36 ` [PATCH v3 0/3] counter: add GPIO-based quadrature encoder driver William Breathitt Gray
2026-05-04 19:37   ` Wadim Mueller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox