Re: [PATCH] Input: xpad: Spelling fixes for "Xbox", improve and proofread the listed xpad device names

[Cameron Gutman <aicommander@gmail.com>]

On 7/30/20 12:06 PM, Ismael Ferreras Morezuelas wrote:
> The Linux kernel is notorious for misspelling X-Box, X-box, XBox or XBOX;
> the official spelling is actually just Xbox. Plain and simple.
> 
> After doing that I took the opportunity to review a bit the actual
> xpad gamepad name list to make it more factual and accurate.
> 
> Seems like a worthy enhancement. This commit covers:
> 
>  * Plain typos. (e.g. Mad Cats -> Mad Catz, SoulCaliber -> Soulcalibur)
>  * Wrong or missing parts. (e.g. PS2 -> PS/2, v2 -> S)
>  * Missing manufacturer. (i.e. only the marketing name)
>  * Appending of Xbox / Xbox 360 / Xbox One when needed.
>  * Normalization, use a common case scheme.
> 
> Tried to respect the existing notes and regional classification.
> 
> I mainly sourced the model names from the official driver packages;
> cross-checking against retailers, official websites and other resources.
> 
> The side effect is that this master list is reused in many other places,
> like Steam and SDL2 itself, so it's important to get them right:
> 
> https://support.steampowered.com/kb_article.php?ref=5199-TOKV-4426
> 
> While doing my research I also noticed that the 1532:0037 VID/PID seems to
> be used by the DeathAdder 2013, so that Razer Sabertooth instance looks
> wrong and very suspect to me, I created a separate patch for that.

The above sentence probably doesn't belong in the commit message.

> 
> No functional changes intended.
> 
> Signed-off-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com>
> ---
>  Documentation/input/gamepad.rst |   2 +-
>  drivers/input/joystick/Kconfig  |  14 +-
>  drivers/input/joystick/xpad.c   | 306 ++++++++++++++++----------------
>  3 files changed, 161 insertions(+), 161 deletions(-)
> 
> diff --git a/Documentation/input/gamepad.rst b/Documentation/input/gamepad.rst
> index 4d5e7fb80a84..968e07056552 100644
> --- a/Documentation/input/gamepad.rst
> +++ b/Documentation/input/gamepad.rst
> @@ -184,7 +184,7 @@ Gamepads report the following events:
>  
>    Many pads also have a third button which is branded or has a special symbol
>    and meaning. Such buttons are mapped as BTN_MODE. Examples are the Nintendo
> -  "HOME" button, the XBox "X"-button or Sony "PS" button.
> +  "HOME" button, the Xbox "X" button or the Sony PlayStation "PS" button.
>  
>  - Rumble:
>  
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index eb031b7a4866..45d220fd583c 100644
> --- a/drivers/input/joystick/Kconfig
> +++ b/drivers/input/joystick/Kconfig
> @@ -280,33 +280,33 @@ config JOYSTICK_JOYDUMP
>  	  module will be called joydump.
>  
>  config JOYSTICK_XPAD
> -	tristate "X-Box gamepad support"
> +	tristate "Xbox gamepad support"
>  	depends on USB_ARCH_HAS_HCD
>  	select USB
>  	help
> -	  Say Y here if you want to use the X-Box pad with your computer.
> +	  Say Y here if you want to use Xbox pads with your computer.
>  	  Make sure to say Y to "Joystick support" (CONFIG_INPUT_JOYDEV)
>  	  and/or "Event interface support" (CONFIG_INPUT_EVDEV) as well.
>  
> -	  For information about how to connect the X-Box pad to USB, see
> +	  For information about how to connect the Xbox pad to USB, see
>  	  <file:Documentation/input/devices/xpad.rst>.
>  
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called xpad.
>  
>  config JOYSTICK_XPAD_FF
> -	bool "X-Box gamepad rumble support"
> +	bool "Xbox gamepad rumble support"
>  	depends on JOYSTICK_XPAD && INPUT
>  	select INPUT_FF_MEMLESS
>  	help
> -	  Say Y here if you want to take advantage of xbox 360 rumble features.
> +	  Say Y here if you want to take advantage of Xbox 360 rumble features.
>  
>  config JOYSTICK_XPAD_LEDS
> -	bool "LED Support for Xbox360 controller 'BigX' LED"
> +	bool "LED Support for the Xbox 360 controller Guide button"
>  	depends on JOYSTICK_XPAD && (LEDS_CLASS=y || LEDS_CLASS=JOYSTICK_XPAD)
>  	help
>  	  This option enables support for the LED which surrounds the Big X on
> -	  XBox 360 controller.
> +	  Xbox 360 controllers.
>  
>  config JOYSTICK_WALKERA0701
>  	tristate "Walkera WK-0701 RC transmitter"
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index c77cdb3b62b5..814ca8d7978f 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - * X-Box gamepad driver
> + * Xbox gamepad driver
>   *
>   * Copyright (c) 2002 Marko Friedemann <mfr@bmx-chemnitz.de>
>   *               2004 Oliver Schwartz <Oliver.Schwartz@gmx.de>,
> @@ -23,7 +23,7 @@
>   *  - ITO Takayuki for providing essential xpad information on his website
>   *  - Vojtech Pavlik     - iforce driver / input subsystem
>   *  - Greg Kroah-Hartman - usb-skeleton driver
> - *  - XBOX Linux project - extra USB id's
> + *  - Xbox Linux project - extra USB ids
>   *  - Pekka Pöyry (quantus) - Xbox One controller reverse engineering
>   *
>   * TODO:
> @@ -52,7 +52,7 @@
>   * 2002-07-17 - 0.0.5 : simplified d-pad handling
>   *
>   * 2004-10-02 - 0.0.6 : DDR pad support
> - *  - borrowed from the XBOX linux kernel
> + *  - borrowed from the Xbox Linux kernel
>   *  - USB id's for commonly used dance pads are present
>   *  - dance pads will map D-PAD to buttons, not axes
>   *  - pass the module paramater 'dpad_to_buttons' to force

The docs and comment changes look fine to me.

> @@ -111,40 +111,40 @@ static const struct xpad_device {
>  	u8 mapping;
>  	u8 xtype;
>  } xpad_device[] = {
> -	{ 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
> -	{ 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
> -	{ 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
> -	{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
> +	{ 0x0079, 0x18d4, "GPD Win 2 Xbox Controller", 0, XTYPE_XBOX360 },
> +	{ 0x044f, 0x0f00, "Thrustmaster Steering Wheel for Xbox", 0, XTYPE_XBOX },
> +	{ 0x044f, 0x0f03, "Thrustmaster Steering Wheel for Xbox", 0, XTYPE_XBOX },
> +	{ 0x044f, 0x0f07, "Thrustmaster Controller for Xbox", 0, XTYPE_XBOX },
>  	{ 0x044f, 0x0f10, "Thrustmaster Modena GT Wheel", 0, XTYPE_XBOX },
>  	{ 0x044f, 0xb326, "Thrustmaster Gamepad GP XID", 0, XTYPE_XBOX360 },
> -	{ 0x045e, 0x0202, "Microsoft X-Box pad v1 (US)", 0, XTYPE_XBOX },
> -	{ 0x045e, 0x0285, "Microsoft X-Box pad (Japan)", 0, XTYPE_XBOX },
> +	{ 0x045e, 0x0202, "Microsoft Xbox Controller (US)", 0, XTYPE_XBOX },
> +	{ 0x045e, 0x0285, "Microsoft Xbox Controller S (Japan)", 0, XTYPE_XBOX },
>  	{ 0x045e, 0x0287, "Microsoft Xbox Controller S", 0, XTYPE_XBOX },
> -	{ 0x045e, 0x0288, "Microsoft Xbox Controller S v2", 0, XTYPE_XBOX },
> -	{ 0x045e, 0x0289, "Microsoft X-Box pad v2 (US)", 0, XTYPE_XBOX },
> -	{ 0x045e, 0x028e, "Microsoft X-Box 360 pad", 0, XTYPE_XBOX360 },
> -	{ 0x045e, 0x028f, "Microsoft X-Box 360 pad v2", 0, XTYPE_XBOX360 },

I'm somewhat concerned about the possibility of breaking userspace by changing
names. Some programs' gamepad mappings may be dependent on matching the device
names, rather than the VID+PID.

For example, Android did not expose the VID and PID for input devices until
Android 4.4. The device name was the only available attribute for matching
gamepads from Android 2.3 to 4.3. While these ancient Android version will
almost certainly never run a kernel with this patch, I worry about the
possibility of apps that haven't moved to VID+PID matching (and not just for
Android; I don't know if other libraries or frameworks have/had similar
limitations).

Perhaps my concerns are overblown, but If we aren't worried about changing
names, I'd really prefer to just drop the hardcoded names entirely and use the
manufacturer and product strings provided in the USB string descriptors. The
device list would turn into a quirk list where only device entries with a
special mapping flag like MAP_DPAD_TO_BUTTONS or MAP_TRIGGERS_TO_BUTTONS would
remain, and the device name strings would just become comments on each quirk
entry.

Thoughts?




Regards,
Cameron