Input: xpad - add more Xbox one controller IDs

Input: xpad - add more Xbox one controller IDs

[Benjamin Valentin]

These are collected from issue reports on [0].

I wonder a bit why we need to maintain this list to begin with, when
the consoles (and Windows) can apparently identify any new third party
controller just fine.
I don't have *that* many controllers to do extensive tests, but I wonder
if there could be a better way.

[0] https://github.com/paroj/xpad/issues

Signed-off-by: Benjamin Valentin <benpicco@googlemail.com>
---
 drivers/input/joystick/xpad.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index e51f3a2bf06b..d99dd9aed137 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -150,6 +150,7 @@ static const struct xpad_device {
 	{ 0x06a3, 0x0200, "Saitek Racing Wheel", 0, XTYPE_XBOX },
 	{ 0x06a3, 0x0201, "Saitek Adrenalin", 0, XTYPE_XBOX },
 	{ 0x06a3, 0xf51a, "Saitek P3600", 0, XTYPE_XBOX360 },
+	{ 0x0738, 0x4503, "Mad Catz Racing Wheel", 0, XTYPE_XBOXONE },
 	{ 0x0738, 0x4506, "Mad Catz 4506 Wireless Controller", 0, XTYPE_XBOX },
 	{ 0x0738, 0x4516, "Mad Catz Control Pad", 0, XTYPE_XBOX },
 	{ 0x0738, 0x4520, "Mad Catz Control Pad Pro", 0, XTYPE_XBOX },
@@ -250,6 +251,7 @@ static const struct xpad_device {
 	{ 0x102c, 0xff0c, "Joytech Wireless Advanced Controller", 0, XTYPE_XBOX },
 	{ 0x1038, 0x1430, "SteelSeries Stratus Duo", 0, XTYPE_XBOX360 },
 	{ 0x1038, 0x1431, "SteelSeries Stratus Duo", 0, XTYPE_XBOX360 },
+	{ 0x10f5, 0x7005, "Turtle Beach Recon Controller", 0, XTYPE_XBOXONE },
 	{ 0x11c9, 0x55f0, "Nacon GC-100XF", 0, XTYPE_XBOX360 },
 	{ 0x1209, 0x2882, "Ardwiino Controller", 0, XTYPE_XBOX360 },
 	{ 0x12ab, 0x0004, "Honey Bee Xbox360 dancepad", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360 },
@@ -263,6 +265,7 @@ static const struct xpad_device {
 	{ 0x1532, 0x0037, "Razer Sabertooth", 0, XTYPE_XBOX360 },
 	{ 0x1532, 0x0a00, "Razer Atrox Arcade Stick", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOXONE },
 	{ 0x1532, 0x0a03, "Razer Wildcat", 0, XTYPE_XBOXONE },
+	{ 0x1532, 0x0a29, "Razer Wolverine v2", 0, XTYPE_XBOXONE },
 	{ 0x15e4, 0x3f00, "Power A Mini Pro Elite", 0, XTYPE_XBOX360 },
 	{ 0x15e4, 0x3f0a, "Xbox Airflo wired controller", 0, XTYPE_XBOX360 },
 	{ 0x15e4, 0x3f10, "Batarang Xbox 360 controller", 0, XTYPE_XBOX360 },
@@ -329,6 +332,7 @@ static const struct xpad_device {
 	{ 0x24c6, 0x550e, "Hori Real Arcade Pro V Kai 360", MAP_TRIGGERS_TO_BUTTONS, XTYPE_XBOX360 },
 	{ 0x24c6, 0x551a, "PowerA FUSION Pro Controller", 0, XTYPE_XBOXONE },
 	{ 0x24c6, 0x561a, "PowerA FUSION Controller", 0, XTYPE_XBOXONE },
+	{ 0x24c6, 0x581a, "ThrustMaster XB1 Classic Controller", 0, XTYPE_XBOXONE },
 	{ 0x24c6, 0x5b00, "ThrustMaster Ferrari 458 Racing Wheel", 0, XTYPE_XBOX360 },
 	{ 0x24c6, 0x5b02, "Thrustmaster, Inc. GPX Controller", 0, XTYPE_XBOX360 },
 	{ 0x24c6, 0x5b03, "Thrustmaster Ferrari 458 Racing Wheel", 0, XTYPE_XBOX360 },
@@ -336,6 +340,7 @@ static const struct xpad_device {
 	{ 0x24c6, 0xfafe, "Rock Candy Gamepad for Xbox 360", 0, XTYPE_XBOX360 },
 	{ 0x2563, 0x058d, "OneXPlayer Gamepad", 0, XTYPE_XBOX360 },
 	{ 0x3285, 0x0607, "Nacon GC-100", 0, XTYPE_XBOX360 },
+	{ 0x3285, 0x0614, "Nacon Pro Compact", 0, XTYPE_XBOXONE },
 	{ 0x3767, 0x0101, "Fanatec Speedster 3 Forceshock Wheel", 0, XTYPE_XBOX },
 	{ 0xffff, 0xffff, "Chinese-made Xbox Controller", 0, XTYPE_XBOX },
 	{ 0x0000, 0x0000, "Generic X-Box pad", 0, XTYPE_UNKNOWN }
@@ -435,6 +440,7 @@ static const struct usb_device_id xpad_table[] = {
 	XPAD_XBOX360_VENDOR(0x0f0d),		/* Hori Controllers */
 	XPAD_XBOXONE_VENDOR(0x0f0d),		/* Hori Controllers */
 	XPAD_XBOX360_VENDOR(0x1038),		/* SteelSeries Controllers */
+	XPAD_XBOXONE_VENDOR(0x10f5),		/* Turtle Beach */
 	XPAD_XBOX360_VENDOR(0x11c9),		/* Nacon GC100XF */
 	XPAD_XBOX360_VENDOR(0x1209),		/* Ardwiino Controllers */
 	XPAD_XBOX360_VENDOR(0x12ab),		/* X-Box 360 dance pads */
@@ -455,6 +461,7 @@ static const struct usb_device_id xpad_table[] = {
 	XPAD_XBOXONE_VENDOR(0x2e24),		/* Hyperkin Duke X-Box One pad */
 	XPAD_XBOX360_VENDOR(0x2f24),		/* GameSir Controllers */
 	XPAD_XBOX360_VENDOR(0x3285),		/* Nacon GC-100 */
+	XPAD_XBOXONE_VENDOR(0x3285),		/* Nacon XBOX Series S/X Controllers */
 	{ }
 };
 
-- 
2.32.0

[PATCH] Input: xpad - remove 'rumble support' from TODO

[Benjamin Valentin]

Rumble has been implemented on all controller types for a long while now.
Let's remove this ancient comment that just creates confusion.

Signed-off-by: Benjamin Valentin <benpicco@googlemail.com>
---
 drivers/input/joystick/xpad.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index d99dd9aed137..18caaf436ed4 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -29,7 +29,6 @@
  * TODO:
  *  - fine tune axes (especially trigger axes)
  *  - fix "analog" buttons (reported as digital now)
- *  - get rumble working
  *  - need USB IDs for other dance pads
  *
  * History:
-- 
2.32.0

Re: Input: xpad - add more Xbox one controller IDs

[Cameron Gutman]

On 11/13/21 06:17, Benjamin Valentin wrote:
> These are collected from issue reports on [0].
> 
> I wonder a bit why we need to maintain this list to begin with, when
> the consoles (and Windows) can apparently identify any new third party
> controller just fine.

There are 2 related problems here:

1) Detection of compatible gamepads
2) User-visible identification of gamepads and quirks

For 1, the way Windows does it is via the Microsoft OS descriptor [0].
AFAIK, the specific OS descriptor strings are "XUSB20" for Xbox 360 and
"XGIP10" for Xbox One.

That functionality is handled by xpad_table[] and the bInterfaceProtocol
detection logic in xpad_probe(). The xpad_device[] entry isn't required for
detection or functionality of devices, unless those devices need special
treatment like MAP_TRIGGERS_TO_BUTTONS or something.

Since the protocols are vendor-defined, we end up having to maintain the
vendor list. If we could detect that Microsoft OS descriptor, that would
avoid the need to maintain the xpad_table[] vendor list.

2 is the main reason xpad_device[] exists (other than special devices).
Most devices have pretty useless string descriptors which doesn't really
allow userspace to provide a human-friendly descriptor for the user. OTOH,
Windows totally abdicates this responsibility and just refers to XInput
gamepads by a player index, so maybe it's not something we have to solve.

[0]: https://docs.microsoft.com/en-us/windows-hardware/drivers/usbcon/microsoft-os-1-0-descriptors-specification

> I don't have *that* many controllers to do extensive tests, but I wonder
> if there could be a better way.
>> [0] https://github.com/paroj/xpad/issues
> 
> Signed-off-by: Benjamin Valentin <benpicco@googlemail.com>

Regards,
Cameron

Re: Input: xpad - add more Xbox one controller IDs

[Benjamin Valentin]

On Sat, 13 Nov 2021 19:34:54 -0600
Cameron Gutman <aicommander@gmail.com> wrote:

> For 1, the way Windows does it is via the Microsoft OS descriptor [0].
> AFAIK, the specific OS descriptor strings are "XUSB20" for Xbox 360
> and "XGIP10" for Xbox One.
> 
> That functionality is handled by xpad_table[] and the
> bInterfaceProtocol detection logic in xpad_probe(). The xpad_device[]
> entry isn't required for detection or functionality of devices,
> unless those devices need special treatment like
> MAP_TRIGGERS_TO_BUTTONS or something.

The problem is that the gamepad then gets assigned the type
XTYPE_UNKNOWN which excludes it from all run-time code path switches.

E.g. if I comment out

//    { 0x045e, 0x028e, "Microsoft X-Box 360 pad", 0, XTYPE_XBOX360 },

my X-Box 360 pad will not only lose any LED or rumble support, it will
also not report any input events with jstest because
xpad360_process_packet() is no longer called.

I'll try to check on how to read out the OS descriptor string, maybe
that allows us to detect the type on init and provide a better default
experience.

Best,
Benjamin

Re: Input: xpad - add more Xbox one controller IDs

[Benjamin Valentin]

On Sun, 14 Nov 2021 20:51:22 +0100
Benjamin Valentin <benpicco@googlemail.com> wrote:

> The problem is that the gamepad then gets assigned the type
> XTYPE_UNKNOWN which excludes it from all run-time code path switches.

Ah sorry for the noise. xpad_probe() already takes care of detecting
the gamepad type for the XTYPE_UNKNOWN case.
And this works for my Xbox 360 pad - not sure what's wrong with jstest,
but that is unrelated.

To avoid that confusion, how about

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 4c914f75a902..155ee644295d 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
 @@ -1783,14 +1785,19 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
 
 	if (xpad->xtype == XTYPE_UNKNOWN) {
 		if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
-			if (intf->cur_altsetting->desc.bInterfaceProtocol == 129)
+			if (intf->cur_altsetting->desc.bInterfaceProtocol == 129) {
 				xpad->xtype = XTYPE_XBOX360W;
-			else if (intf->cur_altsetting->desc.bInterfaceProtocol == 208)
+				xpad->name = "Generic Xbox 360 wireless pad";
+			} else if (intf->cur_altsetting->desc.bInterfaceProtocol == 208) {
 				xpad->xtype = XTYPE_XBOXONE;
-			else
+				xpad->name = "Generic Xbox One pad";
+			} else {
 				xpad->xtype = XTYPE_XBOX360;
+				xpad->name = "Generic Xbox 360 pad";
+			}
 		} else {
 			xpad->xtype = XTYPE_XBOX;
+			xpad->name = "Generic Xbox classic pad";
 		}
 
 		if (dpad_to_buttons)

Re: Input: xpad - add more Xbox one controller IDs

[Cameron Gutman]

On 11/14/21 13:51, Benjamin Valentin wrote:
> On Sat, 13 Nov 2021 19:34:54 -0600
> Cameron Gutman <aicommander@gmail.com> wrote:
> 
>> For 1, the way Windows does it is via the Microsoft OS descriptor [0].
>> AFAIK, the specific OS descriptor strings are "XUSB20" for Xbox 360
>> and "XGIP10" for Xbox One.
>>
>> That functionality is handled by xpad_table[] and the
>> bInterfaceProtocol detection logic in xpad_probe(). The xpad_device[]
>> entry isn't required for detection or functionality of devices,
>> unless those devices need special treatment like
>> MAP_TRIGGERS_TO_BUTTONS or something.
> 
> The problem is that the gamepad then gets assigned the type
> XTYPE_UNKNOWN which excludes it from all run-time code path switches.
> 
> E.g. if I comment out
> 
> //    { 0x045e, 0x028e, "Microsoft X-Box 360 pad", 0, XTYPE_XBOX360 },
> 
> my X-Box 360 pad will not only lose any LED or rumble support, it will
> also not report any input events with jstest because
> xpad360_process_packet() is no longer called.
> 

XTYPE_UNKNOWN is a special value that should always be overridden by
the check in xpad_probe() to be the proper XTYPE value at runtime:

	if (xpad->xtype == XTYPE_UNKNOWN) {
		if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
			if (intf->cur_altsetting->desc.bInterfaceProtocol == 129)
				xpad->xtype = XTYPE_XBOX360W;
			else if (intf->cur_altsetting->desc.bInterfaceProtocol == 208)
				xpad->xtype = XTYPE_XBOXONE;
			else
				xpad->xtype = XTYPE_XBOX360;
		} else {
			xpad->xtype = XTYPE_XBOX;
		}

	...


Those values should definitely be correct, because they're what we
use in the xpad_table[] itself. If they don't match, xpad_probe()
won't get called at all.

Can you see what's going on in xpad_probe() when you comment out
that line for your gamepad?

> I'll try to check on how to read out the OS descriptor string, maybe
> that allows us to detect the type on init and provide a better default
> experience.

That type derivation code above already handles determining the type for
unknown gamepads (assuming it actually works). The problem is having to
maintain the xpad_table[] list with every vendor that produces an Xbox
gamepad. That's where an extension to usb_device_id and something like
a USB_MS_OS_DESCRIPTOR("XUSB20") macro would come in, if the USB core
had first-class support for Microsoft OS descriptors.

The problem is that adding that Microsoft OS descriptor query may cause
compatibility issues for other devices. Windows actually keeps track of
whether the OS descriptor query fails to avoid querying it again [0].

[0]: https://docs.microsoft.com/en-us/windows-hardware/drivers/usbcon/microsoft-defined-usb-descriptors

> 
> Best,
> Benjamin
> 

Regards,
Cameron

Re: Input: xpad - add more Xbox one controller IDs

[Cameron Gutman]

On 11/14/21 14:37, Benjamin Valentin wrote:
> On Sun, 14 Nov 2021 20:51:22 +0100
> Benjamin Valentin <benpicco@googlemail.com> wrote:
> 
>> The problem is that the gamepad then gets assigned the type
>> XTYPE_UNKNOWN which excludes it from all run-time code path switches.
> 
> Ah sorry for the noise. xpad_probe() already takes care of detecting
> the gamepad type for the XTYPE_UNKNOWN case.
> And this works for my Xbox 360 pad - not sure what's wrong with jstest,
> but that is unrelated.

Ah okay good, that's what I was expecting.

> 
> To avoid that confusion, how about
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index 4c914f75a902..155ee644295d 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
>  @@ -1783,14 +1785,19 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
>  
>  	if (xpad->xtype == XTYPE_UNKNOWN) {
>  		if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
> -			if (intf->cur_altsetting->desc.bInterfaceProtocol == 129)
> +			if (intf->cur_altsetting->desc.bInterfaceProtocol == 129) {
>  				xpad->xtype = XTYPE_XBOX360W;
> -			else if (intf->cur_altsetting->desc.bInterfaceProtocol == 208)
> +				xpad->name = "Generic Xbox 360 wireless pad";
> +			} else if (intf->cur_altsetting->desc.bInterfaceProtocol == 208) {
>  				xpad->xtype = XTYPE_XBOXONE;
> -			else
> +				xpad->name = "Generic Xbox One pad";
> +			} else {
>  				xpad->xtype = XTYPE_XBOX360;
> +				xpad->name = "Generic Xbox 360 pad";
> +			}
>  		} else {
>  			xpad->xtype = XTYPE_XBOX;
> +			xpad->name = "Generic Xbox classic pad";
>  		}
>  
>  		if (dpad_to_buttons)
> 
> 

I like this idea. There's a small risk of causing userspace breakages for
games matching "Generic X-Box pad" by name, but we already run that risk
each time we update xpad_devices[] anyway. You might even consider dropping
"Generic" from the name and just call them "Xbox One gamepad" or similar.

Can you also replace "Generic X-Box pad" with NULL in xpad_devices[]? I
don't think it would ever be used anymore after this change.


Regards,
Cameron

Re: Input: xpad - add more Xbox one controller IDs

[Benjamin Valentin]

On Sun, 14 Nov 2021 15:14:38 -0600
Cameron Gutman <aicommander@gmail.com> wrote:

> I like this idea. There's a small risk of causing userspace breakages
> for games matching "Generic X-Box pad" by name, but we already run
> that risk each time we update xpad_devices[] anyway.

Since the button mappings already differ between the original Xbox
controller and the Xbox 360 controller, matching that string would have
always been broken.

> Can you also replace "Generic X-Box pad" with NULL in xpad_devices[]?
> I don't think it would ever be used anymore after this change.

Will do!

Best,
Benjamin