diff mbox series

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

Message ID 20210901223037.2964665-3-roderick.colenbrander@sony.com (mailing list archive)
State Superseded
Headers show
Series HID: playstation: add LED support | expand

Commit Message

Roderick Colenbrander Sept. 1, 2021, 10:30 p.m. UTC
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>
---
 Documentation/leds/well-known-leds.txt | 14 ++++++++++++++
 include/dt-bindings/leds/common.h      |  3 +++
 2 files changed, 17 insertions(+)

Comments

Pavel Machek Sept. 3, 2021, 4:17 p.m. UTC | #1
Hi!

> 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>
> ---
>  Documentation/leds/well-known-leds.txt | 14 ++++++++++++++
>  include/dt-bindings/leds/common.h      |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/leds/well-known-leds.txt b/Documentation/leds/well-known-leds.txt
> index 4a8b9dc4bf52..2160382c86be 100644
> --- a/Documentation/leds/well-known-leds.txt
> +++ b/Documentation/leds/well-known-leds.txt
> @@ -16,6 +16,20 @@ but then try the legacy ones, too.
>  
>  Notice there's a list of functions in include/dt-bindings/leds/common.h .
>  
> +* Gamepads and joysticks
> +
> +Game controllers may feature LEDs to indicate a player number. This is commonly
> +used on game consoles in which multiple controllers can be connected to a system.
> +The "player LEDs" are then programmed with a pattern to indicate a particular
> +player. For example, a game controller with 4 LEDs, may be programmed with "x---"
> +to indicate player 1, "-x--" to indicate player 2 etcetera where "x" means on.
> +Input drivers can utilize the LED class to expose the individual player LEDs
> +of a game controller using the function "player".

Thank you.

> +Note: tracking and management of Player IDs is the responsibility of user space,
> +though drivers may pick a default value.

I'm not sure we want kernel to do that.

> +Good: "input*:*:player-{1,2,3,4,5}

This goes to the top.

> +++ 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"
> +

Let's not add this. For consistency we'd need defines player-1, player-2, ... We don't
need the define at all.

I guess this should go through my tree?

Best regards,
									Pavel
Jiri Kosina Sept. 8, 2021, 10:23 a.m. UTC | #2
On Fri, 3 Sep 2021, Pavel Machek wrote:

> > 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>
> > ---
> >  Documentation/leds/well-known-leds.txt | 14 ++++++++++++++
> >  include/dt-bindings/leds/common.h      |  3 +++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/Documentation/leds/well-known-leds.txt b/Documentation/leds/well-known-leds.txt
> > index 4a8b9dc4bf52..2160382c86be 100644
> > --- a/Documentation/leds/well-known-leds.txt
> > +++ b/Documentation/leds/well-known-leds.txt
> > @@ -16,6 +16,20 @@ but then try the legacy ones, too.
> >  
> >  Notice there's a list of functions in include/dt-bindings/leds/common.h .
> >  
> > +* Gamepads and joysticks
> > +
> > +Game controllers may feature LEDs to indicate a player number. This is commonly
> > +used on game consoles in which multiple controllers can be connected to a system.
> > +The "player LEDs" are then programmed with a pattern to indicate a particular
> > +player. For example, a game controller with 4 LEDs, may be programmed with "x---"
> > +to indicate player 1, "-x--" to indicate player 2 etcetera where "x" means on.
> > +Input drivers can utilize the LED class to expose the individual player LEDs
> > +of a game controller using the function "player".
> 
> Thank you.
> 
> > +Note: tracking and management of Player IDs is the responsibility of user space,
> > +though drivers may pick a default value.
> 
> I'm not sure we want kernel to do that.
> 
> > +Good: "input*:*:player-{1,2,3,4,5}
> 
> This goes to the top.
> 
> > +++ 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"
> > +
> 
> Let's not add this. For consistency we'd need defines player-1, player-2, ... We don't
> need the define at all.
> 
> I guess this should go through my tree?

Once you provide your Reviewed/Acked-by, I can take it through my tree 
with the rest of the series.

Thanks,
diff mbox series

Patch

diff --git a/Documentation/leds/well-known-leds.txt b/Documentation/leds/well-known-leds.txt
index 4a8b9dc4bf52..2160382c86be 100644
--- a/Documentation/leds/well-known-leds.txt
+++ b/Documentation/leds/well-known-leds.txt
@@ -16,6 +16,20 @@  but then try the legacy ones, too.
 
 Notice there's a list of functions in include/dt-bindings/leds/common.h .
 
+* Gamepads and joysticks
+
+Game controllers may feature LEDs to indicate a player number. This is commonly
+used on game consoles in which multiple controllers can be connected to a system.
+The "player LEDs" are then programmed with a pattern to indicate a particular
+player. For example, a game controller with 4 LEDs, may be programmed with "x---"
+to indicate player 1, "-x--" to indicate player 2 etcetera where "x" means on.
+Input drivers can utilize the LED class to expose the individual player LEDs
+of a game controller using the function "player".
+Note: tracking and management of Player IDs is the responsibility of user space,
+though drivers may pick a default value.
+
+Good: "input*:*:player-{1,2,3,4,5}
+
 * Keyboards
   
 Good: "input*:*:capslock"
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"