Re: [PATCH v5 2/3] counter: add GPIO-based counter driver

[Wadim Mueller <wafgo01@gmail.com>]

On Wed, 17 Jun 2026 16:49:25 +0900
William Breathitt Gray <wbg@kernel.org> 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