Re: [PATCH 2/3] leds: add new LED_FUNCTION_PLAYER for player LEDs for game controllers.

[Roderick Colenbrander <thunderbird2k@gmail.com>]

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