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

[Svyatoslav Ryhel]

пт, 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.

> Thanks.
>
> --
> Dmitry