Re: [PATCH v3 2/3] counter: add GPIO-based quadrature encoder driver

Wadim Mueller
2026-05-04T21:15:50Z

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