Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221022220508.37489-1-maurinbe@gmail.com> <CAEc3jaDjGBUspf5zPUfjPKgP+nk4zVFqJ0pgR8LAmX35qmx+fA@mail.gmail.com>
In-Reply-To: <CAEc3jaDjGBUspf5zPUfjPKgP+nk4zVFqJ0pgR8LAmX35qmx+fA@mail.gmail.com>
From:   Benoit Maurin <maurinbe@gmail.com>
Date:   Sun, 23 Oct 2022 18:02:29 +0200
Message-ID: <CALamgT9ecqStP4YamvgHnbbZ9m9ux-fDr6UPYcfFfztJH=goRw@mail.gmail.com>
Subject: Re: [PATCH] input/xpad: LED controllable through input events
To:     Roderick Colenbrander <thunderbird2k@gmail.com>
Cc:     Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Roderick,

Thanks for having a look. I'd understand if this is not a desirable directi=
on,
I was not aware of this legacy-ish connotation of EV_LED (should have figur=
ed
it out with the hoops about the clear_bit thing) and it seemed convenient t=
o
interact with the pad through a single /dev/input/xxx interface.

Let me know what you decide :)
Benoit


Le dim. 23 oct. 2022 =C3=A0 06:00, Roderick Colenbrander
<thunderbird2k@gmail.com> a =C3=A9crit :
>
> Hi Benoit,
>
> Thanks for your patch. I'm thinking out loud whether this is a
> direction we want to go in regards to EV_LED. I vaguely recall some
> discussions a while ago that EV_LED was really from a different era
> for legacy reasons (keyboard LEDs). Since the introduction of the LED
> framework that was really the way to go.
>
> Thanks,
> Roderick
>
> On Sat, Oct 22, 2022 at 3:08 PM Benoit Maurin <maurinbe@gmail.com> wrote:
> >
> > (EV_LED, LED_MISC, #VALUE) can now be used to control leds on the
> > xpad gamepad (was only possible through /sys/class/leds/xpad0/brightnes=
s)
> > with permissions of /dev/input/xxx
> >
> > To test the code (xpad can be compiled out-of-tree with some slight
> > tweaks):
> >
> > ```
> > import evdev
> > device =3D evdev.InputDevice('/dev/input/event15') # not js0
> > device.set_led(8, 2)
> > device.set_led(8, 0) # this won't be delivered
> > device.set_led(8, 16) # must do this instead
> > device.set_led(8, 15)
> > ```
> >
> > Signed-off-by: Benoit Maurin <maurinbe@gmail.com>
> > ---
> >  drivers/input/joystick/xpad.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpa=
d.c
> > index 2959d80f7..fcf4d2c8f 100644
> > --- a/drivers/input/joystick/xpad.c
> > +++ b/drivers/input/joystick/xpad.c
> > @@ -1646,6 +1646,7 @@ static int xpad_led_probe(struct usb_xpad *xpad)
> >                 goto err_free_mem;
> >         }
> >
> > +       input_set_capability(xpad->dev, EV_LED, LED_MISC);
> >         snprintf(led->name, sizeof(led->name), "xpad%d", xpad->pad_nr);
> >         led->xpad =3D xpad;
> >
> > @@ -1824,6 +1825,28 @@ static void xpad_deinit_input(struct usb_xpad *x=
pad)
> >         }
> >  }
> >
> > +static int xpad_event(struct input_dev *dev, unsigned int type,
> > +                     unsigned int code, int value)
> > +{
> > +       struct usb_xpad *xpad =3D input_get_drvdata(dev);
> > +
> > +       if (type !=3D EV_LED || xpad->led =3D=3D NULL)
> > +               return 0;
> > +       xpad_send_led_command(xpad, value);
> > +       xpad->led->led_cdev.brightness =3D value;
> > +       /* Bit clearing is necessary otherwise two events with
> > +        * different non-null values will deliver only the first one.
> > +        * To work around this, we clear the bit to indicate that the
> > +        * current value is zero. The downside is that events with zero
> > +        * value won't be delivered. It's not a big deal since a value =
of
> > +        * 16 can be sent which is the same as 0
> > +        * See xpad_send_led_command, command %=3D 16
> > +        */
> > +
> > +       clear_bit(code, xpad->dev->led);
> > +       return 0;
> > +}
> > +
> >  static int xpad_init_input(struct usb_xpad *xpad)
> >  {
> >         struct input_dev *input_dev;
> > @@ -1851,6 +1874,7 @@ static int xpad_init_input(struct usb_xpad *xpad)
> >                 input_dev->open =3D xpad_open;
> >                 input_dev->close =3D xpad_close;
> >         }
> > +       input_dev->event =3D xpad_event;
> >
> >         if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
> >                 /* set up axes */
> > --
> > 2.38.1
> >