diff mbox series

[2/2] Input: xpad - add Logitech G923 Xbox variant

Message ID 20210527134104.217865-3-rickytaylor26@gmail.com (mailing list archive)
State New, archived
Headers show
Series G923 Racing Wheel support | expand

Commit Message

Erica Taylor May 27, 2021, 1:41 p.m. UTC 
This includes device information for the G923 as well as
code to perform a protocol change to HID++.

Many features do not work as-is with the xpad driver. Almost everything
apart from TrueForce works under the logitech-hidpp driver.

What works:
- Face buttons, D-pad, Xbox button, LB, RB, switch and menu
- Paddle shifters
- One pedal axis

What does not work:
- Force feedback
- TrueForce
- Shifter positions
- Dial and centre button
- +/- buttons
- Steering wheel rotation
- The other two pedal axes

Signed-off-by: Erica Taylor <rickytaylor26@gmail.com>
---
 drivers/input/joystick/xpad.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Erica Taylor May 27, 2021, 2:21 p.m. UTC | #1 
It looks like I sent a stale patch in my first e-mail. I'm very sorry.
I've included the correct patch below:

This includes device information for the G923 as well as
code to perform a protocol change to HID++.

Many features do not work as-is with the xpad driver. Almost everything
apart from TrueForce works under the logitech-hidpp driver.

What works:
- Face buttons, D-pad, Xbox button, LB, RB, switch and menu
- Paddle shifters
- One pedal axis

What does not work:
- Force feedback
- TrueForce
- Shifter positions
- Dial and centre button
- +/- buttons
- Steering wheel rotation
- The other two pedal axes

Signed-off-by: Erica Taylor <rickytaylor26@gmail.com>
---
 drivers/input/joystick/xpad.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index d69d7657ab12..a07fa92f1d6a 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -79,6 +79,7 @@
 #define MAP_DPAD_TO_BUTTONS		(1 << 0)
 #define MAP_TRIGGERS_TO_BUTTONS		(1 << 1)
 #define MAP_STICKS_TO_NULL		(1 << 2)
+#define HIDPP_CAPABLE			BIT(3)
 #define DANCEPAD_MAP_CONFIG	(MAP_DPAD_TO_BUTTONS |			\
 				MAP_TRIGGERS_TO_BUTTONS | MAP_STICKS_TO_NULL)
 
@@ -104,6 +105,10 @@ static bool auto_poweroff = true;
 module_param(auto_poweroff, bool, S_IWUSR | S_IRUGO);
 MODULE_PARM_DESC(auto_poweroff, "Power off wireless controllers on suspend");
 
+static bool switch_to_hidpp = true;
+module_param(switch_to_hidpp, bool, 0444);
+MODULE_PARM_DESC(switch_to_hidpp, "Switch appropriate devices over to the HID++ protocol");
+
 static const struct xpad_device {
 	u16 idVendor;
 	u16 idProduct;
@@ -333,6 +338,7 @@ static const struct xpad_device {
 	{ 0x24c6, 0x5d04, "Razer Sabertooth", 0, XTYPE_XBOX360 },
 	{ 0x24c6, 0xfafe, "Rock Candy Gamepad for Xbox 360", 0, XTYPE_XBOX360 },
 	{ 0x3767, 0x0101, "Fanatec Speedster 3 Forceshock Wheel", 0, XTYPE_XBOX },
+	{ 0x046d, 0xc26d, "Logitech G923 Wheel (Xbox Mode)", HIDPP_CAPABLE, XTYPE_XBOXONE },
 	{ 0xffff, 0xffff, "Chinese-made Xbox Controller", 0, XTYPE_XBOX },
 	{ 0x0000, 0x0000, "Generic X-Box pad", 0, XTYPE_UNKNOWN }
 };
@@ -420,6 +426,7 @@ static const struct usb_device_id xpad_table[] = {
 	XPAD_XBOX360_VENDOR(0x045e),		/* Microsoft X-Box 360 controllers */
 	XPAD_XBOXONE_VENDOR(0x045e),		/* Microsoft X-Box One controllers */
 	XPAD_XBOX360_VENDOR(0x046d),		/* Logitech X-Box 360 style controllers */
+	XPAD_XBOXONE_VENDOR(0x046d),		/* Logitech X-Box One style controllers */
 	XPAD_XBOX360_VENDOR(0x056e),		/* Elecom JC-U3613M */
 	XPAD_XBOX360_VENDOR(0x06a3),		/* Saitek P3600 */
 	XPAD_XBOX360_VENDOR(0x0738),		/* Mad Catz X-Box 360 controllers */
@@ -558,6 +565,17 @@ static const struct xboxone_init_packet xboxone_init_packets[] = {
 	XBOXONE_INIT_PKT(0x24c6, 0x543a, xboxone_rumbleend_init),
 };
 
+/*
+ * A magic packet sent to Logitech devices to tell them to change to the HID++
+ * protocol. This is preferred when in use on a PC.
+ *
+ * After receiving this packet, the device will disconnect and reappear with
+ * a different productId, which will be picked up by the Logitech HID++ driver.
+ */
+static const u8 switch_to_hidpp_cmd[] = {
+	0x0f, 0x00, 0x01, 0x01, 0x42
+};
+
 struct xpad_output_packet {
 	u8 data[XPAD_PKT_LEN];
 	u8 len;
@@ -998,6 +1016,14 @@ static bool xpad_prepare_next_init_packet(struct usb_xpad *xpad)
 		return true;
 	}
 
+	if (switch_to_hidpp && xpad->mapping & HIDPP_CAPABLE) {
+		dev_dbg(&xpad->intf->dev, "%s - switching to HID++", __func__);
+		memcpy(xpad->odata, switch_to_hidpp_cmd, ARRAY_SIZE(switch_to_hidpp_cmd));
+		xpad->irq_out->transfer_buffer_length = ARRAY_SIZE(switch_to_hidpp_cmd);
+		xpad->odata[2] = xpad->odata_serial++;
+		return true;
+	}
+
 	return false;
 }
kernel test robot May 27, 2021, 2:54 p.m. UTC | #2 
Hi Erica,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on hid/for-next]
[also build test ERROR on input/next v5.13-rc3]
[cannot apply to jikos-hid/for-next next-20210527]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Erica-Taylor/G923-Racing-Wheel-support/20210527-214315
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
config: alpha-randconfig-r023-20210526 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ae500b9fb85860a6590531cecf51be335880aa67
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Erica-Taylor/G923-Racing-Wheel-support/20210527-214315
        git checkout ae500b9fb85860a6590531cecf51be335880aa67
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/input/joystick/xpad.c: In function 'xpad_prepare_next_init_packet':
>> drivers/input/joystick/xpad.c:1020:23: error: 'g923_hidpp_init' undeclared (first use in this function)
    1020 |   memcpy(xpad->odata, g923_hidpp_init, ARRAY_SIZE(g923_hidpp_init));
         |                       ^~~~~~~~~~~~~~~
   drivers/input/joystick/xpad.c:1020:23: note: each undeclared identifier is reported only once for each function it appears in
   In file included from include/linux/bits.h:22,
                    from include/linux/bitops.h:6,
                    from include/linux/kernel.h:11,
                    from drivers/input/joystick/xpad.c:64:
>> include/linux/build_bug.h:16:51: error: bit-field '<anonymous>' width not an integer constant
      16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
         |                                                   ^
   include/linux/compiler.h:240:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
     240 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
         |                            ^~~~~~~~~~~~~~~~~
   include/linux/kernel.h:49:59: note: in expansion of macro '__must_be_array'
      49 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                                                           ^~~~~~~~~~~~~~~
   drivers/input/joystick/xpad.c:1020:40: note: in expansion of macro 'ARRAY_SIZE'
    1020 |   memcpy(xpad->odata, g923_hidpp_init, ARRAY_SIZE(g923_hidpp_init));
         |                                        ^~~~~~~~~~
>> include/linux/build_bug.h:16:51: error: bit-field '<anonymous>' width not an integer constant
      16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
         |                                                   ^
   include/linux/compiler.h:240:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
     240 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
         |                            ^~~~~~~~~~~~~~~~~
   include/linux/kernel.h:49:59: note: in expansion of macro '__must_be_array'
      49 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                                                           ^~~~~~~~~~~~~~~
   drivers/input/joystick/xpad.c:1021:43: note: in expansion of macro 'ARRAY_SIZE'
    1021 |   xpad->irq_out->transfer_buffer_length = ARRAY_SIZE(g923_hidpp_init);
         |                                           ^~~~~~~~~~
   At top level:
   drivers/input/joystick/xpad.c:574:17: warning: 'switch_to_hidpp_cmd' defined but not used [-Wunused-const-variable=]
     574 | static const u8 switch_to_hidpp_cmd[] = {
         |                 ^~~~~~~~~~~~~~~~~~~


vim +/g923_hidpp_init +1020 drivers/input/joystick/xpad.c

   988	
   989	/* Callers must hold xpad->odata_lock spinlock */
   990	static bool xpad_prepare_next_init_packet(struct usb_xpad *xpad)
   991	{
   992		const struct xboxone_init_packet *init_packet;
   993	
   994		if (xpad->xtype != XTYPE_XBOXONE)
   995			return false;
   996	
   997		/* Perform initialization sequence for Xbox One pads that require it */
   998		while (xpad->init_seq < ARRAY_SIZE(xboxone_init_packets)) {
   999			init_packet = &xboxone_init_packets[xpad->init_seq++];
  1000	
  1001			if (init_packet->idVendor != 0 &&
  1002			    init_packet->idVendor != xpad->dev->id.vendor)
  1003				continue;
  1004	
  1005			if (init_packet->idProduct != 0 &&
  1006			    init_packet->idProduct != xpad->dev->id.product)
  1007				continue;
  1008	
  1009			/* This packet applies to our device, so prepare to send it */
  1010			memcpy(xpad->odata, init_packet->data, init_packet->len);
  1011			xpad->irq_out->transfer_buffer_length = init_packet->len;
  1012	
  1013			/* Update packet with current sequence number */
  1014			xpad->odata[2] = xpad->odata_serial++;
  1015			return true;
  1016		}
  1017	
  1018		if (switch_to_hidpp && xpad->mapping & HIDPP_CAPABLE) {
  1019			dev_dbg(&xpad->intf->dev, "%s - switching to HID++", __func__);
> 1020			memcpy(xpad->odata, g923_hidpp_init, ARRAY_SIZE(g923_hidpp_init));
  1021			xpad->irq_out->transfer_buffer_length = ARRAY_SIZE(g923_hidpp_init);
  1022			xpad->odata[2] = xpad->odata_serial++;
  1023			return true;
  1024		}
  1025	
  1026		return false;
  1027	}
  1028	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot May 27, 2021, 6:22 p.m. UTC | #3 
Hi Erica,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on hid/for-next]
[also build test ERROR on input/next v5.13-rc3]
[cannot apply to next-20210527]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Erica-Taylor/G923-Racing-Wheel-support/20210527-214315
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
config: x86_64-randconfig-r025-20210526 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 6505c630407c5feec818f0bb1c284f9eeebf2071)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/ae500b9fb85860a6590531cecf51be335880aa67
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Erica-Taylor/G923-Racing-Wheel-support/20210527-214315
        git checkout ae500b9fb85860a6590531cecf51be335880aa67
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/input/joystick/xpad.c:1020:51: error: use of undeclared identifier 'g923_hidpp_init'
                   memcpy(xpad->odata, g923_hidpp_init, ARRAY_SIZE(g923_hidpp_init));
                                                                   ^
>> drivers/input/joystick/xpad.c:1020:51: error: use of undeclared identifier 'g923_hidpp_init'
>> drivers/input/joystick/xpad.c:1020:51: error: use of undeclared identifier 'g923_hidpp_init'
   drivers/input/joystick/xpad.c:1020:23: error: use of undeclared identifier 'g923_hidpp_init'
                   memcpy(xpad->odata, g923_hidpp_init, ARRAY_SIZE(g923_hidpp_init));
                                       ^
   drivers/input/joystick/xpad.c:1021:54: error: use of undeclared identifier 'g923_hidpp_init'
                   xpad->irq_out->transfer_buffer_length = ARRAY_SIZE(g923_hidpp_init);
                                                                      ^
   drivers/input/joystick/xpad.c:1021:54: error: use of undeclared identifier 'g923_hidpp_init'
   drivers/input/joystick/xpad.c:1021:54: error: use of undeclared identifier 'g923_hidpp_init'
   7 errors generated.


vim +/g923_hidpp_init +1020 drivers/input/joystick/xpad.c

   988	
   989	/* Callers must hold xpad->odata_lock spinlock */
   990	static bool xpad_prepare_next_init_packet(struct usb_xpad *xpad)
   991	{
   992		const struct xboxone_init_packet *init_packet;
   993	
   994		if (xpad->xtype != XTYPE_XBOXONE)
   995			return false;
   996	
   997		/* Perform initialization sequence for Xbox One pads that require it */
   998		while (xpad->init_seq < ARRAY_SIZE(xboxone_init_packets)) {
   999			init_packet = &xboxone_init_packets[xpad->init_seq++];
  1000	
  1001			if (init_packet->idVendor != 0 &&
  1002			    init_packet->idVendor != xpad->dev->id.vendor)
  1003				continue;
  1004	
  1005			if (init_packet->idProduct != 0 &&
  1006			    init_packet->idProduct != xpad->dev->id.product)
  1007				continue;
  1008	
  1009			/* This packet applies to our device, so prepare to send it */
  1010			memcpy(xpad->odata, init_packet->data, init_packet->len);
  1011			xpad->irq_out->transfer_buffer_length = init_packet->len;
  1012	
  1013			/* Update packet with current sequence number */
  1014			xpad->odata[2] = xpad->odata_serial++;
  1015			return true;
  1016		}
  1017	
  1018		if (switch_to_hidpp && xpad->mapping & HIDPP_CAPABLE) {
  1019			dev_dbg(&xpad->intf->dev, "%s - switching to HID++", __func__);
> 1020			memcpy(xpad->odata, g923_hidpp_init, ARRAY_SIZE(g923_hidpp_init));
  1021			xpad->irq_out->transfer_buffer_length = ARRAY_SIZE(g923_hidpp_init);
  1022			xpad->odata[2] = xpad->odata_serial++;
  1023			return true;
  1024		}
  1025	
  1026		return false;
  1027	}
  1028	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index d69d7657ab12..930283433615 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -79,6 +79,7 @@ 
 #define MAP_DPAD_TO_BUTTONS		(1 << 0)
 #define MAP_TRIGGERS_TO_BUTTONS		(1 << 1)
 #define MAP_STICKS_TO_NULL		(1 << 2)
+#define HIDPP_CAPABLE			BIT(3)
 #define DANCEPAD_MAP_CONFIG	(MAP_DPAD_TO_BUTTONS |			\
 				MAP_TRIGGERS_TO_BUTTONS | MAP_STICKS_TO_NULL)
 
@@ -104,6 +105,10 @@  static bool auto_poweroff = true;
 module_param(auto_poweroff, bool, S_IWUSR | S_IRUGO);
 MODULE_PARM_DESC(auto_poweroff, "Power off wireless controllers on suspend");
 
+static bool switch_to_hidpp = true;
+module_param(switch_to_hidpp, bool, 0444);
+MODULE_PARM_DESC(switch_to_hidpp, "Switch appropriate devices over to the HID++ protocol");
+
 static const struct xpad_device {
 	u16 idVendor;
 	u16 idProduct;
@@ -333,6 +338,8 @@  static const struct xpad_device {
 	{ 0x24c6, 0x5d04, "Razer Sabertooth", 0, XTYPE_XBOX360 },
 	{ 0x24c6, 0xfafe, "Rock Candy Gamepad for Xbox 360", 0, XTYPE_XBOX360 },
 	{ 0x3767, 0x0101, "Fanatec Speedster 3 Forceshock Wheel", 0, XTYPE_XBOX },
+	{ 0x046d, 0xc262, "Logitech G920 Wheel (Xbox Mode)", HIDPP_CAPABLE, XTYPE_XBOXONE },
+	{ 0x046d, 0xc26d, "Logitech G923 Wheel (Xbox Mode)", HIDPP_CAPABLE, XTYPE_XBOXONE },
 	{ 0xffff, 0xffff, "Chinese-made Xbox Controller", 0, XTYPE_XBOX },
 	{ 0x0000, 0x0000, "Generic X-Box pad", 0, XTYPE_UNKNOWN }
 };
@@ -420,6 +427,7 @@  static const struct usb_device_id xpad_table[] = {
 	XPAD_XBOX360_VENDOR(0x045e),		/* Microsoft X-Box 360 controllers */
 	XPAD_XBOXONE_VENDOR(0x045e),		/* Microsoft X-Box One controllers */
 	XPAD_XBOX360_VENDOR(0x046d),		/* Logitech X-Box 360 style controllers */
+	XPAD_XBOXONE_VENDOR(0x046d),		/* Logitech X-Box One style controllers */
 	XPAD_XBOX360_VENDOR(0x056e),		/* Elecom JC-U3613M */
 	XPAD_XBOX360_VENDOR(0x06a3),		/* Saitek P3600 */
 	XPAD_XBOX360_VENDOR(0x0738),		/* Mad Catz X-Box 360 controllers */
@@ -558,6 +566,17 @@  static const struct xboxone_init_packet xboxone_init_packets[] = {
 	XBOXONE_INIT_PKT(0x24c6, 0x543a, xboxone_rumbleend_init),
 };
 
+/*
+ * A magic packet sent to Logitech devices to tell them to change to the HID++
+ * protocol. This is preferred when in use on a PC.
+ *
+ * After receiving this packet, the device will disconnect and reappear with
+ * a different productId, which will be picked up by the Logitech HID++ driver.
+ */
+static const u8 switch_to_hidpp_cmd[] = {
+	0x0f, 0x00, 0x01, 0x01, 0x42
+};
+
 struct xpad_output_packet {
 	u8 data[XPAD_PKT_LEN];
 	u8 len;
@@ -998,6 +1017,14 @@  static bool xpad_prepare_next_init_packet(struct usb_xpad *xpad)
 		return true;
 	}
 
+	if (switch_to_hidpp && xpad->mapping & HIDPP_CAPABLE) {
+		dev_dbg(&xpad->intf->dev, "%s - switching to HID++", __func__);
+		memcpy(xpad->odata, g923_hidpp_init, ARRAY_SIZE(g923_hidpp_init));
+		xpad->irq_out->transfer_buffer_length = ARRAY_SIZE(g923_hidpp_init);
+		xpad->odata[2] = xpad->odata_serial++;
+		return true;
+	}
+
 	return false;
 }