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

[Dmitry Torokhov]

On Fri, Jun 26, 2026 at 10:45:20AM +0300, Svyatoslav Ryhel wrote:
> пт, 26 черв. 2026 р. о 02:45 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> пише:
> >
> > Hi Svyatoslav,
> >
> > On Wed, Jun 17, 2026 at 10:05:27AM +0300, Svyatoslav Ryhel wrote:
> > > From: Linus Walleij <linusw@xxxxxxxxxx>
> > >
> > > 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.
> > >
> > > Tested-by: Linus Walleij <linusw@xxxxxxxxxx> # GT-I9070 Janice
> > > Signed-off-by: Linus Walleij <linusw@xxxxxxxxxx>
> > > Co-developed-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> >
> >
> > Sashiko convinced me that using mutex_trylock() in the playback work
> > handler will result in dropping requests and that we are safe not taking
> > the lock there at all. Can you please try the following modification?
> >
> > diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
> > index c61adc4b605c..fb7f68fa0a2b 100644
> > --- a/drivers/input/misc/isa1200.c
> > +++ b/drivers/input/misc/isa1200.c
> > @@ -253,15 +253,12 @@ static void isa1200_stop(struct isa1200 *isa)
> >  static void isa1200_play_work(struct work_struct *work)
> >  {
> >         struct isa1200 *isa = container_of(work, struct isa1200, play_work);
> > -       struct input_dev *input = isa->input;
> > -
> > -       scoped_guard(mutex_try, &input->mutex) {
> > -               if (!isa->suspended) {
> > -                       if (isa->level)
> > -                               isa1200_start(isa);
> > -                       else
> > -                               isa1200_stop(isa);
> > -               }
> > +
> > +       if (!READ_ONCE(isa->suspended)) {
> > +               if (isa->level)
> > +                       isa1200_start(isa);
> > +               else
> > +                       isa1200_stop(isa);
> >         }
> >  }
> >
> >
> > If this works no need to resend, I'll fold on my side.
> >
> 
> It does work fine. Thank you.

Excelled, thank you. Applied both patches.

-- 
Dmitry