(EV_LED, LED_MISC, #VALUE) can now be used to control leds on the
xpad gamepad (was only possible through /sys/class/leds/xpad0/brightness)
with permissions of /dev/input/xxx
To test the code (xpad can be compiled out-of-tree with some slight
tweaks):
```
import evdev
device = 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 <[email protected]>
---
drivers/input/joystick/xpad.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.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 = xpad;
@@ -1824,6 +1825,28 @@ static void xpad_deinit_input(struct usb_xpad *xpad)
}
}
+static int xpad_event(struct input_dev *dev, unsigned int type,
+ unsigned int code, int value)
+{
+ struct usb_xpad *xpad = input_get_drvdata(dev);
+
+ if (type != EV_LED || xpad->led == NULL)
+ return 0;
+ xpad_send_led_command(xpad, value);
+ xpad->led->led_cdev.brightness = 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 %= 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 = xpad_open;
input_dev->close = xpad_close;
}
+ input_dev->event = xpad_event;
if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
/* set up axes */
--
2.38.1
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 <[email protected]> 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/brightness)
> with permissions of /dev/input/xxx
>
> To test the code (xpad can be compiled out-of-tree with some slight
> tweaks):
>
> ```
> import evdev
> device = 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 <[email protected]>
> ---
> drivers/input/joystick/xpad.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.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 = xpad;
>
> @@ -1824,6 +1825,28 @@ static void xpad_deinit_input(struct usb_xpad *xpad)
> }
> }
>
> +static int xpad_event(struct input_dev *dev, unsigned int type,
> + unsigned int code, int value)
> +{
> + struct usb_xpad *xpad = input_get_drvdata(dev);
> +
> + if (type != EV_LED || xpad->led == NULL)
> + return 0;
> + xpad_send_led_command(xpad, value);
> + xpad->led->led_cdev.brightness = 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 %= 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 = xpad_open;
> input_dev->close = xpad_close;
> }
> + input_dev->event = xpad_event;
>
> if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
> /* set up axes */
> --
> 2.38.1
>
Hi Roderick,
Thanks for having a look. I'd understand if this is not a desirable direction,
I was not aware of this legacy-ish connotation of EV_LED (should have figured
it out with the hoops about the clear_bit thing) and it seemed convenient to
interact with the pad through a single /dev/input/xxx interface.
Let me know what you decide :)
Benoit
Le dim. 23 oct. 2022 à 06:00, Roderick Colenbrander
<[email protected]> a écrit :
>
> 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 <[email protected]> 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/brightness)
> > with permissions of /dev/input/xxx
> >
> > To test the code (xpad can be compiled out-of-tree with some slight
> > tweaks):
> >
> > ```
> > import evdev
> > device = 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 <[email protected]>
> > ---
> > drivers/input/joystick/xpad.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.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 = xpad;
> >
> > @@ -1824,6 +1825,28 @@ static void xpad_deinit_input(struct usb_xpad *xpad)
> > }
> > }
> >
> > +static int xpad_event(struct input_dev *dev, unsigned int type,
> > + unsigned int code, int value)
> > +{
> > + struct usb_xpad *xpad = input_get_drvdata(dev);
> > +
> > + if (type != EV_LED || xpad->led == NULL)
> > + return 0;
> > + xpad_send_led_command(xpad, value);
> > + xpad->led->led_cdev.brightness = 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 %= 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 = xpad_open;
> > input_dev->close = xpad_close;
> > }
> > + input_dev->event = xpad_event;
> >
> > if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
> > /* set up axes */
> > --
> > 2.38.1
> >