Re: [PATCH v5 2/3] counter: add GPIO-based counter driver
From: Wadim Mueller
Date: Mon Jun 22 2026 - 16:53:35 EST
On Wed, 17 Jun 2026 16:49:25 +0900
William Breathitt Gray <wbg@xxxxxxxxxx> wrote:
Hi William,
thanks for the review. Three things before I spin v6.
> One change I consider is whether to make Signal B optional. [...]
> I wonder whether this is substantially different enough from
> simply using the interrupt-cnt module on the respective IRQ?
> I'm CCing Oleksij and the Pengutronix team in case they wish to
> comment.
I want to keep signal-b mandatory in v6 (if no concerns from Oleksij). The single-line case is
already covered by interrupt-cnt.
> In such a configuration, we would have two Counts: Count 1 [...]
> Count 2 supports only increase/decrease modes with a Synapse for
> Signal B.
Just to confirm, plan for v6 is:
Count 0 "AB Count": A + B + optional index0, all 8 functions
Count 1 "B Count": B + optional index1, increase/decrease only
One counter_ops, dispatch on count->id. Per-count state in
struct gpio_counter_count_priv (value, ceiling, preset, preset_enabled,
enabled, function, direction), held in priv->count_priv[2] as you
suggested. prev_a/prev_b stay on priv (they describe the wire, not
the count).
For the second Index in DT I would just let index-gpios take 1..2
entries (first = count0, second = count1), no new property. Ok for
you and Conor?
> Hmm, is it a problem that priv->enabled is changed to a false state
> before the IRQs are actually disabled? Do any issues arise if an IRQ
> is handled during that brief period of time?
I guess it is a race. In v6 I will reorder:
enable=1: enable_irq(); lock; enabled = true; unlock;
enable=0: lock; enabled = false; unlock; disable_irq();
Plus a mutex around enable_write so two writers can not interleave
(disable_irq() can not run under the spinlock).
All other points from your review (kill *_delta, STATE_CHANGED for
all quadrature modes, INC/DEC both edges, drop prev_a check in
pulse-direction, ...) go into v6 too.
action_write and floor in a follow-up, as you suggested.
Thanks,
Wadim