Re: [PATCH v1 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: Dmitry Torokhov
Date: Sun Apr 26 2026 - 00:28:33 EST
Hi Svyatoslav,
On Sat, Apr 25, 2026 at 08:26:00PM +0300, Svyatoslav Ryhel wrote:
> Hello Dmitry!
>
> I have a question regarding this sashiko comment
>
> > +static void isa1200_play_work(struct work_struct *work)
> > +{
> > + struct isa1200 *isa =
> > + container_of(work, struct isa1200, play_work);
> > +
> > + if (isa->level)
> > + isa1200_start(isa);
> > + else
> > + isa1200_stop(isa);
> > +}
> Because the driver tracks isa->level locklessly, if an effect starts
> and quickly stops, the workqueue might only execute once. When it
> executes, it sees isa->level == 0 and calls isa1200_stop(). Since
> isa1200_start() was never called for this effect,
> clk_disable_unprepare(isa->clk) will be invoked on an un-enabled
> clock. Does this unbalance the clock reference count?
>
> This is a valid comment, but I cannot find how this should be handled
> among all available haptic drivers. Maybe you can point me in the
> right direction?
Maybe have a flag reflecting the true (committed) state of the
controller that is both checked and updated in the work entity?
Thanks.
--
Dmitry