[PATCH] HID: add new gamepad LED constants

[PATCH] HID: add new gamepad LED constants

[Michael Wright]

Improve gamepad support by introducing new LED constants for player
LEDs and the mappings from their corresponding HID usages introduced
in HUTTR47: http://www.usb.org/developers/hidpage/HUTRR47.pdf

Change-Id: If25d8a8e2570dfab3a35f9be5b1d03ab662f6b1c
Signed-off-by: Michael Wright <michaelwr@google.com>
---
 drivers/hid/hid-input.c    | 4 ++++
 include/uapi/linux/input.h | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 2619f7f..a1cb3bf 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -633,6 +633,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 		case 0x4b:  map_led (LED_MISC);     break;    /*   "Generic Indicator"        */
 		case 0x19:  map_led (LED_MAIL);     break;    /*   "Message Waiting"          */
 		case 0x4d:  map_led (LED_CHARGING); break;    /*   "External Power Connected" */
+		case 0x4f:  map_led (LED_PLAYER_1); break;    /*   "Player 1" */
+		case 0x50:  map_led (LED_PLAYER_2); break;    /*   "Player 2" */
+		case 0x51:  map_led (LED_PLAYER_3); break;    /*   "Player 3" */
+		case 0x52:  map_led (LED_PLAYER_4); break;    /*   "Player 4" */
 
 		default: goto ignore;
 		}
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 1874ebe..bb43f20 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -908,6 +908,10 @@ struct input_keymap_entry {
 #define LED_MISC		0x08
 #define LED_MAIL		0x09
 #define LED_CHARGING		0x0a
+#define LED_PLAYER_1		0x0b
+#define LED_PLAYER_2		0x0c
+#define LED_PLAYER_3		0x0d
+#define LED_PLAYER_4		0x0e
 #define LED_MAX			0x0f
 #define LED_CNT			(LED_MAX+1)
 
-- 
2.1.0.rc2.206.gedb03e5

Re: [PATCH] HID: add new gamepad LED constants

[David Herrmann]

Hi

On Mon, Sep 15, 2014 at 5:13 AM, Michael Wright <michaelwr@android.com> wrote:
> Improve gamepad support by introducing new LED constants for player
> LEDs and the mappings from their corresponding HID usages introduced
> in HUTTR47: http://www.usb.org/developers/hidpage/HUTRR47.pdf

Why not introduce all 8 player LEDs as described in the document? You
should be safe increasing LED_MAX to 0x1f.

Otherwise, looks good to me. But please don't forgot to put maintainers on CC.

Thanks
David

> Change-Id: If25d8a8e2570dfab3a35f9be5b1d03ab662f6b1c
> Signed-off-by: Michael Wright <michaelwr@google.com>
> ---
>  drivers/hid/hid-input.c    | 4 ++++
>  include/uapi/linux/input.h | 4 ++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 2619f7f..a1cb3bf 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -633,6 +633,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>                 case 0x4b:  map_led (LED_MISC);     break;    /*   "Generic Indicator"        */
>                 case 0x19:  map_led (LED_MAIL);     break;    /*   "Message Waiting"          */
>                 case 0x4d:  map_led (LED_CHARGING); break;    /*   "External Power Connected" */
> +               case 0x4f:  map_led (LED_PLAYER_1); break;    /*   "Player 1" */
> +               case 0x50:  map_led (LED_PLAYER_2); break;    /*   "Player 2" */
> +               case 0x51:  map_led (LED_PLAYER_3); break;    /*   "Player 3" */
> +               case 0x52:  map_led (LED_PLAYER_4); break;    /*   "Player 4" */
>
>                 default: goto ignore;
>                 }
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 1874ebe..bb43f20 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -908,6 +908,10 @@ struct input_keymap_entry {
>  #define LED_MISC               0x08
>  #define LED_MAIL               0x09
>  #define LED_CHARGING           0x0a
> +#define LED_PLAYER_1           0x0b
> +#define LED_PLAYER_2           0x0c
> +#define LED_PLAYER_3           0x0d
> +#define LED_PLAYER_4           0x0e
>  #define LED_MAX                        0x0f
>  #define LED_CNT                        (LED_MAX+1)
>
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: add new gamepad LED constants

[Michael Wright]

> Why not introduce all 8 player LEDs as described in the document? You
> should be safe increasing LED_MAX to 0x1f.

Wasn't sure if it was safe to bump it since it could potentially break apps
until they recompile against the new API.

I'll send another patchset with all of them.

On Sun, Sep 14, 2014 at 10:07 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Mon, Sep 15, 2014 at 5:13 AM, Michael Wright <michaelwr@android.com> wrote:
>> Improve gamepad support by introducing new LED constants for player
>> LEDs and the mappings from their corresponding HID usages introduced
>> in HUTTR47: http://www.usb.org/developers/hidpage/HUTRR47.pdf
>
> Why not introduce all 8 player LEDs as described in the document? You
> should be safe increasing LED_MAX to 0x1f.
>
> Otherwise, looks good to me. But please don't forgot to put maintainers on CC.
>
> Thanks
> David
>
>> Change-Id: If25d8a8e2570dfab3a35f9be5b1d03ab662f6b1c
>> Signed-off-by: Michael Wright <michaelwr@google.com>
>> ---
>>  drivers/hid/hid-input.c    | 4 ++++
>>  include/uapi/linux/input.h | 4 ++++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>> index 2619f7f..a1cb3bf 100644
>> --- a/drivers/hid/hid-input.c
>> +++ b/drivers/hid/hid-input.c
>> @@ -633,6 +633,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>>                 case 0x4b:  map_led (LED_MISC);     break;    /*   "Generic Indicator"        */
>>                 case 0x19:  map_led (LED_MAIL);     break;    /*   "Message Waiting"          */
>>                 case 0x4d:  map_led (LED_CHARGING); break;    /*   "External Power Connected" */
>> +               case 0x4f:  map_led (LED_PLAYER_1); break;    /*   "Player 1" */
>> +               case 0x50:  map_led (LED_PLAYER_2); break;    /*   "Player 2" */
>> +               case 0x51:  map_led (LED_PLAYER_3); break;    /*   "Player 3" */
>> +               case 0x52:  map_led (LED_PLAYER_4); break;    /*   "Player 4" */
>>
>>                 default: goto ignore;
>>                 }
>> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
>> index 1874ebe..bb43f20 100644
>> --- a/include/uapi/linux/input.h
>> +++ b/include/uapi/linux/input.h
>> @@ -908,6 +908,10 @@ struct input_keymap_entry {
>>  #define LED_MISC               0x08
>>  #define LED_MAIL               0x09
>>  #define LED_CHARGING           0x0a
>> +#define LED_PLAYER_1           0x0b
>> +#define LED_PLAYER_2           0x0c
>> +#define LED_PLAYER_3           0x0d
>> +#define LED_PLAYER_4           0x0e
>>  #define LED_MAX                        0x0f
>>  #define LED_CNT                        (LED_MAX+1)
>>
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Michael Wright
Android Frameworks

Re: [PATCH] HID: add new gamepad LED constants

[Dmitry Torokhov]

On Mon, Sep 15, 2014 at 11:03:47AM -0700, Michael Wright wrote:
> > Why not introduce all 8 player LEDs as described in the document? You
> > should be safe increasing LED_MAX to 0x1f.
> 
> Wasn't sure if it was safe to bump it since it could potentially break apps
> until they recompile against the new API.

I'd rather we did not add any new input LED definitions but rather
looked into switching to LED subsystem, at least for new LEDs.

Thanks.

-- 
Dmitry

Re: [PATCH] HID: add new gamepad LED constants

[David Herrmann]

Hi

On Mon, Sep 15, 2014 at 11:57 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Sep 15, 2014 at 11:03:47AM -0700, Michael Wright wrote:
>> > Why not introduce all 8 player LEDs as described in the document? You
>> > should be safe increasing LED_MAX to 0x1f.
>>
>> Wasn't sure if it was safe to bump it since it could potentially break apps
>> until they recompile against the new API.
>
> I'd rather we did not add any new input LED definitions but rather
> looked into switching to LED subsystem, at least for new LEDs.

Why? The LED subsystem lacks any unprivileged API to control LEDs.
There's no cdev to control write-access to the LED API. /sys has never
been intended as unprivileged API so I'd really prefer if we add
proper input codes to control those. At least I cannot see the
advantage of the led subsystem over input EV_LED codes. Care to
elaborate?

Thanks
David

Re: [PATCH] HID: add new gamepad LED constants

[Dmitry Torokhov]

Hi David,

On Tue, Sep 16, 2014 at 12:15:08AM +0200, David Herrmann wrote:
> Hi
> 
> On Mon, Sep 15, 2014 at 11:57 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Mon, Sep 15, 2014 at 11:03:47AM -0700, Michael Wright wrote:
> >> > Why not introduce all 8 player LEDs as described in the document? You
> >> > should be safe increasing LED_MAX to 0x1f.
> >>
> >> Wasn't sure if it was safe to bump it since it could potentially break apps
> >> until they recompile against the new API.
> >
> > I'd rather we did not add any new input LED definitions but rather
> > looked into switching to LED subsystem, at least for new LEDs.
> 
> Why?

Because we have a proper subsystem that represents LEDs.

> The LED subsystem lacks any unprivileged API to control LEDs.

As far as I can see it is the same as for input devices. You just need
to make sure the device owner can access needed attributes, such as
brightness.

> There's no cdev to control write-access to the LED API. /sys has never
> been intended as unprivileged API

chmod/chown works just fine on sysfs attributes.

> so I'd really prefer if we add
> proper input codes to control those. At least I cannot see the
> advantage of the led subsystem over input EV_LED codes. Care to
> elaborate?
> 

LEDs should be controlled via LED subsystem. The fact that they happen
to be located on a given device does not mean they belong to input
subsystem. The same as LEds on network card are not part of network stack,
etc.

Thanks.

-- 
Dmitry

Re: [PATCH] HID: add new gamepad LED constants

[Michael Wright]

> LEDs should be controlled via LED subsystem. The fact that they happen
> to be located on a given device does not mean they belong to input
> subsystem. The same as LEds on network card are not part of network stack,
> etc.

I think the fundamental difference for me is that the primary input
standard does
have a specification for how to interact with LEDs, and right now the LED
subsystem has no knowledge of HID whatsoever.

Even if I did add the plumbing for HID over into the LED subsystem, is there an
easy way to correlate input devices and their LEDs from userspace?


On Mon, Sep 15, 2014 at 3:58 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi David,
>
> On Tue, Sep 16, 2014 at 12:15:08AM +0200, David Herrmann wrote:
>> Hi
>>
>> On Mon, Sep 15, 2014 at 11:57 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Mon, Sep 15, 2014 at 11:03:47AM -0700, Michael Wright wrote:
>> >> > Why not introduce all 8 player LEDs as described in the document? You
>> >> > should be safe increasing LED_MAX to 0x1f.
>> >>
>> >> Wasn't sure if it was safe to bump it since it could potentially break apps
>> >> until they recompile against the new API.
>> >
>> > I'd rather we did not add any new input LED definitions but rather
>> > looked into switching to LED subsystem, at least for new LEDs.
>>
>> Why?
>
> Because we have a proper subsystem that represents LEDs.
>
>> The LED subsystem lacks any unprivileged API to control LEDs.
>
> As far as I can see it is the same as for input devices. You just need
> to make sure the device owner can access needed attributes, such as
> brightness.
>
>> There's no cdev to control write-access to the LED API. /sys has never
>> been intended as unprivileged API
>
> chmod/chown works just fine on sysfs attributes.
>
>> so I'd really prefer if we add
>> proper input codes to control those. At least I cannot see the
>> advantage of the led subsystem over input EV_LED codes. Care to
>> elaborate?
>>
>
> LEDs should be controlled via LED subsystem. The fact that they happen
> to be located on a given device does not mean they belong to input
> subsystem. The same as LEds on network card are not part of network stack,
> etc.
>
> Thanks.
>
> --
> Dmitry



-- 
Michael Wright
Android Frameworks

Re: [PATCH] HID: add new gamepad LED constants

[Dmitry Torokhov]

On Mon, Sep 15, 2014 at 04:41:42PM -0700, Michael Wright wrote:
> > LEDs should be controlled via LED subsystem. The fact that they happen
> > to be located on a given device does not mean they belong to input
> > subsystem. The same as LEds on network card are not part of network stack,
> > etc.
> 
> I think the fundamental difference for me is that the primary input
> standard does
> have a specification for how to interact with LEDs,

... That predates LED subsystem. When we saw that it is not flexible enough LED
subsystem was created. Input, for better or worse, has "sound" support as well,
but I do not think anyone would want to to enhance it to support 7.1 surround
output ;)

> and right now the LED
> subsystem has no knowledge of HID whatsoever.

Then that is something that needs to be changed.

> 
> Even if I did add the plumbing for HID over into the LED subsystem, is there an
> easy way to correlate input devices and their LEDs from userspace?

What does it mean "their led"? The led that happens to share the same physical
package?

Thanks.

-- 
Dmitry

Re: [PATCH] HID: add new gamepad LED constants

[Michael Wright]

On Mon, Sep 15, 2014 at 5:54 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> ... That predates LED subsystem. When we saw that it is not flexible enough LED
> subsystem was created. Input, for better or worse, has "sound" support as well,
> but I do not think anyone would want to to enhance it to support 7.1 surround
> output ;)

When I said standard I meant HID, not the input subsystem. I totally
understand wanting to create a better separation of responsibilities
internally.

> What does it mean "their led"? The led that happens to share the same physical
> package?

Sure, but in this case we really care that they're part of the same
physical package. We're lighting up the LED for the first player's
input device, so we *need* this ability to correlate.

LEDs also don't appear to have any uapi headers. Is the sysfs entry
considered stable API? Are we supposed to classify what they're for
(e.g. PLAYER_1 vs. PLAYER_2) by their name?

-- 
Michael Wright
Android Frameworks

Re: [PATCH] HID: add new gamepad LED constants

[simon]

> Even if I did add the plumbing for HID over into the LED subsystem, is
> there an
> easy way to correlate input devices and their LEDs from userspace?

You can follow the symlinks through the '/sys' filesystem. For example the
following all point to the same LED.
--
simon@womble:~$ ls
/sys/class/input/js0/device/device/leds/0003\:054C\:0268.0001\:\:sony1
brightness  device  max_brightness  power  subsystem  trigger  uevent
simon@womble:~$ ls /sys/class/leds/0003\:054C\:0268.0001\:\:sony1/
brightness  device  max_brightness  power  subsystem  trigger  uevent
simon@womble:~$ ls
/sys/bus/hid/devices/0003\:054C\:0268.0001/leds/0003\:054C\:0268.0001\:\:sony1/
brightness  device  max_brightness  power  subsystem  trigger  uevent
--

One problem that exists is that many HID devices don't have an (embedded)
serial number, so identification can be confused if you have more that one
identical device.

Simon.

Re: [PATCH] HID: add new gamepad LED constants

[Michael Wright]

On Mon, Sep 15, 2014 at 7:00 PM,  <simon@mungewell.org> wrote:
> One problem that exists is that many HID devices don't have an (embedded)
> serial number, so identification can be confused if you have more that one
> identical device.


That's a pretty serious problem since this is explicitly for multiple
gamepads connected to the same device, which will most likely be all
the same type.

-- 
Michael Wright
Android Frameworks

Re: [PATCH] HID: add new gamepad LED constants

[David Herrmann]

Hi

On Tue, Sep 16, 2014 at 12:58 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> The LED subsystem lacks any unprivileged API to control LEDs.
>
> As far as I can see it is the same as for input devices. You just need
> to make sure the device owner can access needed attributes, such as
> brightness.
>
>> There's no cdev to control write-access to the LED API. /sys has never
>> been intended as unprivileged API
>
> chmod/chown works just fine on sysfs attributes.

Sure it is. You can even use ACLs. But that doesn't mean it provides a
proper API environment. Udev maintainers have always said "sysfs is
only writable by root". Reasons mostly being the following:

* sysfs exists only once. Unlike char-devs, you cannot create multiple
independent entry-points to the device node. Instead, there's only a
single entry point which has to be shared between all users (across
sessions, across containers, across user-namespaces, across
pid-namespaces).

* sysfs does not provide per-file contexts. Unlike char-devs, sysfs
never knows the context of the user that writes into an attribute. It
cannot associate private data to the open file and it cannot track the
time the user releases the device again. It cannot multiplex across
multiple simultaneous users.

* sysfs is not "lightweight". People always say they don't want full
blown char-devs because sysfs is much lighter. Don't know where that
came from.. but looking at the amount of inodes and files needed for
sysfs APIs, it's definitely not faster nor smaller than char-devs.

Just look at IIO to see what happens if you write evdev-like APIs
based on sysfs attributes "because it's lightweight". It's a mess.

Sure, one solution would be to provide char-devs for LEDs. But then
one char-dev per LED sounds like overkill, so my proposal is to
include it in the input API like we always did. You're free to
disagree on that. I'm just saying that sysfs APIs to access LEDs on
GamePads (which this patch is mostly targeted at, I guess) is making
user-space life miserable. It's the same discussion we had with
backlights for years and we're trying eagerly to merge them into the
DRM char-devs.

Thanks
David

Re: [PATCH] HID: add new gamepad LED constants

[Pierre-Loup A. Griffais]

My issue with the LED subsystem is that to my knowledge, the only thing 
that it currently exposes is an opaque brightness value that has plenty 
of different meanings depending on what is actually being exposed. It 
would need to be overhauled to support the usecase at hand, which would 
be a lot more work than just applying this patch, on top of all the 
other problems already mentioned with finding and using the sysfs LED 
interface.

With this change all the gamepad drivers could easily plumb through 
their device-specific way of exposing player IDs, and a userspace 
component could easily ensure consistency between logical player IDs and 
their LEDs.

If such a thing was attempted currently with gamepads that are wired up 
to LED classes, the userspace component would need to have special 
knowledge of each driver to be able to know which special brightness 
value to set to get player N to light up on the controller. This isn't 
solvable without significantly expanding the interface to the LED subsystem.

Michael, I assume such a userspace component is what you're shooting for 
with this work?

In SteamOS the individual gamepad drivers are modified to automatically 
apply player IDs from the joydev device IDs, which in turn are the 
logical game controller IDs that are exposed by libraries such as SDL to 
games and as such are reflected through their UI.

This was deemed too ad-hoc to be upstreamable and your patch seems like 
the right first step towards a cleaner, albeit slightly more complex 
solution. Trying to leverage the LED system seems a lot more complex, 
without adding any value to this solution.

Thanks,
  - Pierre-Loup

On 09/15/2014 09:06 PM, Michael Wright wrote:
> On Mon, Sep 15, 2014 at 7:00 PM,  <simon@mungewell.org> wrote:
>> One problem that exists is that many HID devices don't have an (embedded)
>> serial number, so identification can be confused if you have more that one
>> identical device.
>
>
> That's a pretty serious problem since this is explicitly for multiple
> gamepads connected to the same device, which will most likely be all
> the same type.
>

Re: [PATCH] HID: add new gamepad LED constants

[Michael Wright]

Yes, pretty much exactly that.

Re: [PATCH] HID: add new gamepad LED constants

[Michael Wright]

Just so I can figure out the next steps, are you still convinced that
new LEDs should go in the led subsystem Dmitry?

Thanks,

-- 
Michael Wright
Android Frameworks

Re: [PATCH] HID: add new gamepad LED constants

[Dmitry Torokhov]

On Tue, Sep 16, 2014 at 12:00:26PM +0200, David Herrmann wrote:
> Hi
> 
> On Tue, Sep 16, 2014 at 12:58 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >> The LED subsystem lacks any unprivileged API to control LEDs.
> >
> > As far as I can see it is the same as for input devices. You just need
> > to make sure the device owner can access needed attributes, such as
> > brightness.
> >
> >> There's no cdev to control write-access to the LED API. /sys has never
> >> been intended as unprivileged API
> >
> > chmod/chown works just fine on sysfs attributes.
> 
> Sure it is. You can even use ACLs. But that doesn't mean it provides a
> proper API environment. Udev maintainers have always said "sysfs is
> only writable by root". Reasons mostly being the following:
> 
> * sysfs exists only once. Unlike char-devs, you cannot create multiple
> independent entry-points to the device node. Instead, there's only a
> single entry point which has to be shared between all users (across
> sessions, across containers, across user-namespaces, across
> pid-namespaces).
> 
> * sysfs does not provide per-file contexts. Unlike char-devs, sysfs
> never knows the context of the user that writes into an attribute. It
> cannot associate private data to the open file and it cannot track the
> time the user releases the device again. It cannot multiplex across
> multiple simultaneous users.
> 
> * sysfs is not "lightweight". People always say they don't want full
> blown char-devs because sysfs is much lighter. Don't know where that
> came from.. but looking at the amount of inodes and files needed for
> sysfs APIs, it's definitely not faster nor smaller than char-devs.

I do not recall anyone saying char dev is super heavy. I was only saying
that stuffing everything in input device does not make much sense and
that full-blown input device structure is quite heavy in itself, without
even talking about all interface drivers that can attach to it.

> 
> Just look at IIO to see what happens if you write evdev-like APIs
> based on sysfs attributes "because it's lightweight". It's a mess.
> 
> Sure, one solution would be to provide char-devs for LEDs.

Exactly. If something like chardev is better API for LEDs then do it.
LED subsystem allows us to create many different "triggers".

> But then
> one char-dev per LED sounds like overkill, so my proposal is to

Not really. I think it would be fine.

>
> include it in the input API like we always did. You're free to
> disagree on that. I'm just saying that sysfs APIs to access LEDs on
> GamePads (which this patch is mostly targeted at, I guess) is making
> user-space life miserable.

How can we make life not miserable, but still not merge this into input?
I see the draw of saying "this is only for gamepads", "this is only for
keyboards", etc... But then I see generic-purpose LEDs getting way into
(we already had LED_MAIL, etc creep in), we have LEDs on network cards
(which are incidentally not part of network stack), istorage enclosures
have their LEDs, and all other LEds everywhere.

> It's the same discussion we had with
> backlights for years and we're trying eagerly to merge them into the
> DRM char-devs.
> 

So keyboard backlight is going to be controlled by display chardev? Or
we are fragmenting backlights into different classes?

Thanks.

-- 
Dmitry

Re: [PATCH] HID: add new gamepad LED constants

[Dmitry Torokhov]

On Mon, Sep 22, 2014 at 02:36:57PM -0700, Michael Wright wrote:
> Just so I can figure out the next steps, are you still convinced that
> new LEDs should go in the led subsystem Dmitry?

At this point I would really prefer them not be part of input_dev
structure. Actually there are patches to convert input core to use
generic LEDs for the standard keyboard leds, sadly there is an open
question of how to handle access via legacy and the new API together so
I was not able to merge it [yet].

Thanks.

-- 
Dmitry

Re: [PATCH] HID: add new gamepad LED constants

[David Herrmann]

Hi

On Tue, Sep 23, 2014 at 6:37 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> It's the same discussion we had with
>> backlights for years and we're trying eagerly to merge them into the
>> DRM char-devs.
>>
>
> So keyboard backlight is going to be controlled by display chardev? Or
> we are fragmenting backlights into different classes?

The idea is to make DRM property changes propagate into a linked
backlight. Applied to input-LEDs, it would mean setting LED_XYZ via
input would cause an kernel-internal call into the linked led_class
set_brightness(). This allows us to register an led-class per LED as
you request, but at the same time allow easy access via input devices
where it makes sense.

Thanks
David