| Message ID | 20210602061253.5747-3-roderick@gaikai.com |
|---|---|
| State | New |
| Headers | show |
| Series | HID: playstation: add LED support. | expand |
On Tue, 1 Jun 2021, Roderick Colenbrander wrote: > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > Player LEDs are commonly found on game controllers from Nintendo and Sony > to indicate a player ID across a number of LEDs. For example, "Player 2" > might be indicated as "-x--" on a device with 4 LEDs where "x" means on. > > This patch introduces a new LED_FUNCTION_PLAYER to properly indicate > player LEDs from the kernel. Until now there was no good standard, which > resulted in inconsistent behavior across xpad, hid-sony, hid-wiimote and > other drivers. Moving forward new drivers should use LED_FUNCTION_PLAYER. > > Note: management of Player IDs is left to user space, though a kernel > driver may pick a default value. > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > --- > include/dt-bindings/leds/common.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h > index 52b619d44ba2..94999c250e4d 100644 > --- a/include/dt-bindings/leds/common.h > +++ b/include/dt-bindings/leds/common.h > @@ -60,6 +60,9 @@ > #define LED_FUNCTION_MICMUTE "micmute" > #define LED_FUNCTION_MUTE "mute" > > +/* Used for player LEDs as found on game controllers from e.g. Nintendo, Sony. */ > +#define LED_FUNCTION_PLAYER "player" > + > /* Miscelleaus functions. Use functions above if you can. */ > #define LED_FUNCTION_ACTIVITY "activity" > #define LED_FUNCTION_ALARM "alarm" Pavel, can I please get your Ack on this one, so that I can take it with the rest of the series? Thanks, -- Jiri Kosina SUSE Labs
Hi Pavel, Any feedback on this patch, which introduces a new player led type, which is common on game controllers? Thanks, Roderick Colenbrander On Thu, Jun 24, 2021 at 6:26 AM Jiri Kosina <jikos@kernel.org> wrote: > > On Tue, 1 Jun 2021, Roderick Colenbrander wrote: > > > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > > > Player LEDs are commonly found on game controllers from Nintendo and Sony > > to indicate a player ID across a number of LEDs. For example, "Player 2" > > might be indicated as "-x--" on a device with 4 LEDs where "x" means on. > > > > This patch introduces a new LED_FUNCTION_PLAYER to properly indicate > > player LEDs from the kernel. Until now there was no good standard, which > > resulted in inconsistent behavior across xpad, hid-sony, hid-wiimote and > > other drivers. Moving forward new drivers should use LED_FUNCTION_PLAYER. > > > > Note: management of Player IDs is left to user space, though a kernel > > driver may pick a default value. > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > > --- > > include/dt-bindings/leds/common.h | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h > > index 52b619d44ba2..94999c250e4d 100644 > > --- a/include/dt-bindings/leds/common.h > > +++ b/include/dt-bindings/leds/common.h > > @@ -60,6 +60,9 @@ > > #define LED_FUNCTION_MICMUTE "micmute" > > #define LED_FUNCTION_MUTE "mute" > > > > +/* Used for player LEDs as found on game controllers from e.g. Nintendo, Sony. */ > > +#define LED_FUNCTION_PLAYER "player" > > + > > /* Miscelleaus functions. Use functions above if you can. */ > > #define LED_FUNCTION_ACTIVITY "activity" > > #define LED_FUNCTION_ALARM "alarm" > > Pavel, can I please get your Ack on this one, so that I can take it with > the rest of the series? > > Thanks, > > -- > Jiri Kosina > SUSE Labs >
Hi Pavel, Any update on this topic? Thanks, Roderick On Mon, Jul 5, 2021 at 9:51 PM Roderick Colenbrander <thunderbird2k@gmail.com> wrote: > > Hi Pavel, > > Any feedback on this patch, which introduces a new player led type, > which is common on game controllers? > > Thanks, > Roderick Colenbrander > > On Thu, Jun 24, 2021 at 6:26 AM Jiri Kosina <jikos@kernel.org> wrote: > > > > On Tue, 1 Jun 2021, Roderick Colenbrander wrote: > > > > > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > > > > > Player LEDs are commonly found on game controllers from Nintendo and Sony > > > to indicate a player ID across a number of LEDs. For example, "Player 2" > > > might be indicated as "-x--" on a device with 4 LEDs where "x" means on. > > > > > > This patch introduces a new LED_FUNCTION_PLAYER to properly indicate > > > player LEDs from the kernel. Until now there was no good standard, which > > > resulted in inconsistent behavior across xpad, hid-sony, hid-wiimote and > > > other drivers. Moving forward new drivers should use LED_FUNCTION_PLAYER. > > > > > > Note: management of Player IDs is left to user space, though a kernel > > > driver may pick a default value. > > > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > > > --- > > > include/dt-bindings/leds/common.h | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h > > > index 52b619d44ba2..94999c250e4d 100644 > > > --- a/include/dt-bindings/leds/common.h > > > +++ b/include/dt-bindings/leds/common.h > > > @@ -60,6 +60,9 @@ > > > #define LED_FUNCTION_MICMUTE "micmute" > > > #define LED_FUNCTION_MUTE "mute" > > > > > > +/* Used for player LEDs as found on game controllers from e.g. Nintendo, Sony. */ > > > +#define LED_FUNCTION_PLAYER "player" > > > + > > > /* Miscelleaus functions. Use functions above if you can. */ > > > #define LED_FUNCTION_ACTIVITY "activity" > > > #define LED_FUNCTION_ALARM "alarm" > > > > Pavel, can I please get your Ack on this one, so that I can take it with > > the rest of the series? > > > > Thanks, > > > > -- > > Jiri Kosina > > SUSE Labs > >
Hi! > > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > > > Player LEDs are commonly found on game controllers from Nintendo and Sony > > to indicate a player ID across a number of LEDs. For example, "Player 2" > > might be indicated as "-x--" on a device with 4 LEDs where "x" means on. > > > > This patch introduces a new LED_FUNCTION_PLAYER to properly indicate > > player LEDs from the kernel. Until now there was no good standard, which > > resulted in inconsistent behavior across xpad, hid-sony, hid-wiimote and > > other drivers. Moving forward new drivers should use LED_FUNCTION_PLAYER. > > > > Note: management of Player IDs is left to user space, though a kernel > > driver may pick a default value. > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > > --- > > include/dt-bindings/leds/common.h | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h > > index 52b619d44ba2..94999c250e4d 100644 > > --- a/include/dt-bindings/leds/common.h > > +++ b/include/dt-bindings/leds/common.h > > @@ -60,6 +60,9 @@ > > #define LED_FUNCTION_MICMUTE "micmute" > > #define LED_FUNCTION_MUTE "mute" > > > > +/* Used for player LEDs as found on game controllers from e.g. Nintendo, Sony. */ > > +#define LED_FUNCTION_PLAYER "player" > > + > > /* Miscelleaus functions. Use functions above if you can. */ > > #define LED_FUNCTION_ACTIVITY "activity" > > #define LED_FUNCTION_ALARM "alarm" > > Pavel, can I please get your Ack on this one, so that I can take it with > the rest of the series? I'm sorry for delays. But no, player is not suitable function. Function would be "player1" AFAICT, right? I'm not sure "function" is suitable here, and we may want to create documentation like this... where it would be explained which functions apply to which devices and what they actually mean. Best regards, Pavel -*- org -*- It is somehow important to provide consistent interface to the userland. LED devices have one problem there, and that is naming of directories in /sys/class/leds. It would be nice if userland would just know right "name" for given LED function, but situation got more complex. Anyway, if backwards compatibility is not an issue, new code should use one of the "good" names from this list, and you should extend the list where applicable. Bad names are listed, too; in case you are writing application that wants to use particular feature, you should probe for good name, first, but then try the bad ones, too. * Keyboards Good: "input*:*:capslock" Good: "input*:*:scrolllock" Good: "input*:*:numlock" Bad: "shift-key-light" (Motorola Droid 4, capslock) Set of common keyboard LEDs, going back to PC AT or so. Good: "platform::kbd_backlight" Bad: "tpacpi::thinklight" (IBM/Lenovo Thinkpads) Bad: "lp5523:kb{1,2,3,4,5,6}" (Nokia N900) Frontlight/backlight of main keyboard. Bad: "button-backlight" (Motorola Droid 4) Some phones have touch buttons below screen; it is different from main keyboard. And this is their backlight. * Sound subsystem Good: "platform:*:mute" Good: "platform:*:micmute" LEDs on notebook body, indicating that sound input / output is muted. * System notification Good: "status-led:{red,green,blue}" (Motorola Droid 4) Bad: "lp5523:{r,g,b}" (Nokia N900) Phones usually have multi-color status LED. * Power management Good: "platform:*:charging" (allwinner sun50i) * Screen Good: ":backlight" (Motorola Droid 4) -- http://www.livejournal.com/~pavelmachek
Hi Pavel, See some quick comments inline. On Tue, Aug 3, 2021 at 3:39 PM Pavel Machek <pavel@ucw.cz> wrote: > > Hi! > > > > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > > > > > Player LEDs are commonly found on game controllers from Nintendo and Sony > > > to indicate a player ID across a number of LEDs. For example, "Player 2" > > > might be indicated as "-x--" on a device with 4 LEDs where "x" means on. > > > > > > This patch introduces a new LED_FUNCTION_PLAYER to properly indicate > > > player LEDs from the kernel. Until now there was no good standard, which > > > resulted in inconsistent behavior across xpad, hid-sony, hid-wiimote and > > > other drivers. Moving forward new drivers should use LED_FUNCTION_PLAYER. > > > > > > Note: management of Player IDs is left to user space, though a kernel > > > driver may pick a default value. > > > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > > > --- > > > include/dt-bindings/leds/common.h | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h > > > index 52b619d44ba2..94999c250e4d 100644 > > > --- a/include/dt-bindings/leds/common.h > > > +++ b/include/dt-bindings/leds/common.h > > > @@ -60,6 +60,9 @@ > > > #define LED_FUNCTION_MICMUTE "micmute" > > > #define LED_FUNCTION_MUTE "mute" > > > > > > +/* Used for player LEDs as found on game controllers from e.g. Nintendo, Sony. */ > > > +#define LED_FUNCTION_PLAYER "player" > > > + > > > /* Miscelleaus functions. Use functions above if you can. */ > > > #define LED_FUNCTION_ACTIVITY "activity" > > > #define LED_FUNCTION_ALARM "alarm" > > > > Pavel, can I please get your Ack on this one, so that I can take it with > > the rest of the series? > > I'm sorry for delays. > > But no, player is not suitable function. Function would be "player1" > AFAICT, right? A given gaming device such as Sony or Nintendo controllers have a multiple of these LEDs, which are meant to be configured with a player number. A typical device has like 4 of these LEDs, so a single controller would have: "player-1", "player-2", "player-3" and "player-4". It is up to userspace to configure the player number (a driver may set a default number across a number of LEDs). If player is not the right term (many people know it), what would it be? > > I'm not sure "function" is suitable here, and we may want to create > documentation like this... where it would be explained which functions > apply to which devices and what they actually mean. > > Best regards, > Pavel > > -*- org -*- > > It is somehow important to provide consistent interface to the > userland. LED devices have one problem there, and that is naming of > directories in /sys/class/leds. It would be nice if userland would > just know right "name" for given LED function, but situation got more > complex. > > Anyway, if backwards compatibility is not an issue, new code should > use one of the "good" names from this list, and you should extend the > list where applicable. > > Bad names are listed, too; in case you are writing application that > wants to use particular feature, you should probe for good name, first, > but then try the bad ones, too. > > * Keyboards > > Good: "input*:*:capslock" > Good: "input*:*:scrolllock" > Good: "input*:*:numlock" > Bad: "shift-key-light" (Motorola Droid 4, capslock) > > Set of common keyboard LEDs, going back to PC AT or so. > > Good: "platform::kbd_backlight" > Bad: "tpacpi::thinklight" (IBM/Lenovo Thinkpads) > Bad: "lp5523:kb{1,2,3,4,5,6}" (Nokia N900) > > Frontlight/backlight of main keyboard. > > Bad: "button-backlight" (Motorola Droid 4) > > Some phones have touch buttons below screen; it is different from main > keyboard. And this is their backlight. > > * Sound subsystem > > Good: "platform:*:mute" > Good: "platform:*:micmute" > > LEDs on notebook body, indicating that sound input / output is muted. > > * System notification > > Good: "status-led:{red,green,blue}" (Motorola Droid 4) > Bad: "lp5523:{r,g,b}" (Nokia N900) > > Phones usually have multi-color status LED. > > * Power management > > Good: "platform:*:charging" (allwinner sun50i) > > * Screen > > Good: ":backlight" (Motorola Droid 4) > > > -- > http://www.livejournal.com/~pavelmachek
Hi Pavel, Do you have any recommendations on what would be an appropriate function string for player indicator LEDs? Would some variant such as: "status-x" "player-status-x" "indicator-x" "player-indicator-x" be more suitable? It looks like the string "status" has been used for other existing LED names. I think we are pretty happy to use whatever naming scheme fits the standards of the led subsystem's userspace api for the Nintendo/Sony HID drivers, and any future game controller drivers featuring player LEDs could conform to that going forward. Thanks, Daniel On Tue, Aug 3, 2021 at 7:30 PM Roderick Colenbrander <thunderbird2k@gmail.com> wrote: > > Hi Pavel, > > See some quick comments inline. > > On Tue, Aug 3, 2021 at 3:39 PM Pavel Machek <pavel@ucw.cz> wrote: > > > > Hi! > > > > > > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > > > > > > > Player LEDs are commonly found on game controllers from Nintendo and Sony > > > > to indicate a player ID across a number of LEDs. For example, "Player 2" > > > > might be indicated as "-x--" on a device with 4 LEDs where "x" means on. > > > > > > > > This patch introduces a new LED_FUNCTION_PLAYER to properly indicate > > > > player LEDs from the kernel. Until now there was no good standard, which > > > > resulted in inconsistent behavior across xpad, hid-sony, hid-wiimote and > > > > other drivers. Moving forward new drivers should use LED_FUNCTION_PLAYER. > > > > > > > > Note: management of Player IDs is left to user space, though a kernel > > > > driver may pick a default value. > > > > > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > > > > --- > > > > include/dt-bindings/leds/common.h | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h > > > > index 52b619d44ba2..94999c250e4d 100644 > > > > --- a/include/dt-bindings/leds/common.h > > > > +++ b/include/dt-bindings/leds/common.h > > > > @@ -60,6 +60,9 @@ > > > > #define LED_FUNCTION_MICMUTE "micmute" > > > > #define LED_FUNCTION_MUTE "mute" > > > > > > > > +/* Used for player LEDs as found on game controllers from e.g. Nintendo, Sony. */ > > > > +#define LED_FUNCTION_PLAYER "player" > > > > + > > > > /* Miscelleaus functions. Use functions above if you can. */ > > > > #define LED_FUNCTION_ACTIVITY "activity" > > > > #define LED_FUNCTION_ALARM "alarm" > > > > > > Pavel, can I please get your Ack on this one, so that I can take it with > > > the rest of the series? > > > > I'm sorry for delays. > > > > But no, player is not suitable function. Function would be "player1" > > AFAICT, right? > > A given gaming device such as Sony or Nintendo controllers have a > multiple of these LEDs, which are meant to be configured with a player > number. A typical device has like 4 of these LEDs, so a single > controller would have: "player-1", "player-2", "player-3" and > "player-4". It is up to userspace to configure the player number (a > driver may set a default number across a number of LEDs). > > If player is not the right term (many people know it), what would it be? > > > > > I'm not sure "function" is suitable here, and we may want to create > > documentation like this... where it would be explained which functions > > apply to which devices and what they actually mean. > > > > Best regards, > > Pavel > > > > -*- org -*- > > > > It is somehow important to provide consistent interface to the > > userland. LED devices have one problem there, and that is naming of > > directories in /sys/class/leds. It would be nice if userland would > > just know right "name" for given LED function, but situation got more > > complex. > > > > Anyway, if backwards compatibility is not an issue, new code should > > use one of the "good" names from this list, and you should extend the > > list where applicable. > > > > Bad names are listed, too; in case you are writing application that > > wants to use particular feature, you should probe for good name, first, > > but then try the bad ones, too. > > > > * Keyboards > > > > Good: "input*:*:capslock" > > Good: "input*:*:scrolllock" > > Good: "input*:*:numlock" > > Bad: "shift-key-light" (Motorola Droid 4, capslock) > > > > Set of common keyboard LEDs, going back to PC AT or so. > > > > Good: "platform::kbd_backlight" > > Bad: "tpacpi::thinklight" (IBM/Lenovo Thinkpads) > > Bad: "lp5523:kb{1,2,3,4,5,6}" (Nokia N900) > > > > Frontlight/backlight of main keyboard. > > > > Bad: "button-backlight" (Motorola Droid 4) > > > > Some phones have touch buttons below screen; it is different from main > > keyboard. And this is their backlight. > > > > * Sound subsystem > > > > Good: "platform:*:mute" > > Good: "platform:*:micmute" > > > > LEDs on notebook body, indicating that sound input / output is muted. > > > > * System notification > > > > Good: "status-led:{red,green,blue}" (Motorola Droid 4) > > Bad: "lp5523:{r,g,b}" (Nokia N900) > > > > Phones usually have multi-color status LED. > > > > * Power management > > > > Good: "platform:*:charging" (allwinner sun50i) > > > > * Screen > > > > Good: ":backlight" (Motorola Droid 4) > > > > > > -- > > http://www.livejournal.com/~pavelmachek -- Daniel Ogorchock
On Fri, 13 Aug 2021, Daniel Ogorchock wrote: > Hi Pavel, > > Do you have any recommendations on what would be an appropriate > function string for player indicator LEDs? Would some variant such as: > "status-x" > "player-status-x" > "indicator-x" > "player-indicator-x" > be more suitable? It looks like the string "status" has been used for > other existing LED names. > > I think we are pretty happy to use whatever naming scheme fits the > standards of the led subsystem's userspace api for the Nintendo/Sony > HID drivers, and any future game controller drivers featuring player > LEDs could conform to that going forward. Pavel, could you please take a look here, so that we can proceed with the patchset? Thanks, -- Jiri Kosina SUSE Labs
Hi! > > Do you have any recommendations on what would be an appropriate > > function string for player indicator LEDs? Would some variant such as: > > "status-x" > > "player-status-x" > > "indicator-x" > > "player-indicator-x" > > be more suitable? It looks like the string "status" has been used for > > other existing LED names. I guess "player-x" would be suitable. > > I think we are pretty happy to use whatever naming scheme fits the > > standards of the led subsystem's userspace api for the Nintendo/Sony > > HID drivers, and any future game controller drivers featuring player > > LEDs could conform to that going forward. > > Pavel, could you please take a look here, so that we can proceed with the > patchset? So... leds tree has just been merged: > git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git/ tags/leds-5.15-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/a998a62be9cdb509491731ffe81575aa09943a32 It includes Documentation/leds/well-known-leds.txt file. Could a section describing proposed naming be added there (both device and function), with explanations what the LEDs do? Best regards, Pavel -- http://www.livejournal.com/~pavelmachek
Hi Pavel, On Tue, Aug 31, 2021 at 10:19 PM Pavel Machek <pavel@ucw.cz> wrote: > > Hi! > > > > Do you have any recommendations on what would be an appropriate > > > function string for player indicator LEDs? Would some variant such as: > > > "status-x" > > > "player-status-x" > > > "indicator-x" > > > "player-indicator-x" > > > be more suitable? It looks like the string "status" has been used for > > > other existing LED names. > > I guess "player-x" would be suitable. > > > > I think we are pretty happy to use whatever naming scheme fits the > > > standards of the led subsystem's userspace api for the Nintendo/Sony > > > HID drivers, and any future game controller drivers featuring player > > > LEDs could conform to that going forward. > > > > Pavel, could you please take a look here, so that we can proceed with the > > patchset? > > So... leds tree has just been merged: > > > git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git/ > tags/leds-5.15-rc1 > > has been merged into torvalds/linux.git: > https://git.kernel.org/torvalds/c/a998a62be9cdb509491731ffe81575aa09943a32 > > It includes Documentation/leds/well-known-leds.txt file. Could a > section describing proposed naming be added there (both device and > function), with explanations what the LEDs do? > Sure let me write add a few lines for that file and resubmit. I guess I should rebase based on Linus his tree then.. let me quickly start on that. (I'm technically on vacation and far from home, but luckily caught this and happen to have a break) Thanks, Roderick
diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h index 52b619d44ba2..94999c250e4d 100644 --- a/include/dt-bindings/leds/common.h +++ b/include/dt-bindings/leds/common.h @@ -60,6 +60,9 @@ #define LED_FUNCTION_MICMUTE "micmute" #define LED_FUNCTION_MUTE "mute" +/* Used for player LEDs as found on game controllers from e.g. Nintendo, Sony. */ +#define LED_FUNCTION_PLAYER "player" + /* Miscelleaus functions. Use functions above if you can. */ #define LED_FUNCTION_ACTIVITY "activity" #define LED_FUNCTION_ALARM "alarm"