Re: [PATCH v1 2/2] Input: isa1200 - new driver for Imagis ISA1200

Svyatoslav Ryhel
2026-04-25T17:26:13Z

пт, 24 квіт. 2026 р. о 19:01 Svyatoslav Ryhel <clamor95@gmail.com> пише:
>
> пт, 24 квіт. 2026 р. о 18:31 Dmitry Torokhov <dmitry.torokhov@gmail.com> пише:
> >
> > On Fri, Apr 24, 2026 at 10:13:05AM +0300, Svyatoslav Ryhel wrote:
> > > From: Linus Walleij <linusw@kernel.org>
> > >
> > > The ISA1200 is a haptic feedback unit from Imagis Technology using two
> > > motors for haptic feedback in mobile phones. Used in many mobile devices
> > > c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
> > > GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.
> > >
> > > The exact datasheet for the ISA1200 is not available; all data was modeled
> > > based on available downstream kernel sources for various devices and
> > > fragments of information scattered across the internet.
> > >
> > > Signed-off-by: Linus Walleij <linusw@kernel.org>
> > > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> >
> > There are a bunch of valid sashiko comments, please address them:
> >
> > https://sashiko.dev/#/patchset/20260424071305.89503-1-clamor95%40gmail.com
> >
> > Thanks.
> >
>
> Acknowledged, thank you.
>

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?

> > --
> > Dmitry