| Message ID | 20221022220508.37489-1-maurinbe@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | input/xpad: LED controllable through input events | expand |
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/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 <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/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 <thunderbird2k@gmail.com> 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 <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/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 <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/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 > >
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 */
(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 <maurinbe@gmail.com> --- drivers/input/joystick/xpad.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)