diff mbox series

Input: xpad - Add HyperX Clutch Gladiate support

Message ID 20230722010532.120280-1-hphyperxdev@gmail.com (mailing list archive)
State New, archived
Headers show
Series Input: xpad - Add HyperX Clutch Gladiate support | expand

Commit Message

Nguyen, Max July 22, 2023, 1:05 a.m. UTC 
Add HyperX controller support to xpad_device and xpad_table.

Reported-by: Chris Toledanes <chris.toledanes@hp.com>
Reviewed-by: Carl Ng <carl.ng@hp.com>
Signed-off-by: Max Nguyen <maxwell.nguyen@hp.com>
---
 drivers/input/joystick/xpad.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Rahul Rameshbabu July 22, 2023, 2:46 a.m. UTC | #1 
On Fri, 21 Jul, 2023 18:05:32 -0700 HP Dev <hphyperxdev@gmail.com> wrote:
> Add HyperX controller support to xpad_device and xpad_table.

This patch appears to be a resend of a previous patch but does not
follow the right practice for patch resends.

  https://docs.kernel.org/process/submitting-patches.html#don-t-get-discouraged-or-impatient

https://lore.kernel.org/linux-input/ZKxVBULWtM30ZJ7D@google.com/

>
> Reported-by: Chris Toledanes <chris.toledanes@hp.com>

I think this may not be an accurate use of the Reported-by: git trailer.
My guess is you would want to use the Suggested-by: trailer here. This
patch does not solve a "regression" in the xpad driver that was reported
by someone or some automation but rather adds a new controller to be
probed by the driver.

  https://docs.kernel.org/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

> Reviewed-by: Carl Ng <carl.ng@hp.com>
> Signed-off-by: Max Nguyen <maxwell.nguyen@hp.com>
> ---
>  drivers/input/joystick/xpad.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index cdb193317c3b..dfddb0bba8d8 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -130,6 +130,7 @@ static const struct xpad_device {
>  	{ 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
>  	{ 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
>  	{ 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
> +	{ 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },
>  	{ 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
>  	{ 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
>  	{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
> @@ -457,6 +458,8 @@ static const struct usb_device_id xpad_table[] = {
>  	{ USB_INTERFACE_INFO('X', 'B', 0) },	/* Xbox USB-IF not-approved class */
>  	XPAD_XBOX360_VENDOR(0x0079),		/* GPD Win 2 controller */
>  	XPAD_XBOX360_VENDOR(0x03eb),		/* Wooting Keyboards (Legacy) */
> +	XPAD_XBOX360_VENDOR(0x03f0),		/* HP HyperX XBox 360 Controllers */

There are no HP HyperX Xbox 360 controllers previously supported or
added in this patch. I think the above line should be removed since you
only have a XTYPE_XBOXONE type controller. You probably do not need to
match against Xbox 360 vendor-specific class since you only implement an
Xbox One controller.

> +	XPAD_XBOXONE_VENDOR(0x03f0),		/* HP HyperX Xbox One Controllers */
>  	XPAD_XBOX360_VENDOR(0x044f),		/* Thrustmaster Xbox 360 controllers */
>  	XPAD_XBOX360_VENDOR(0x045e),		/* Microsoft Xbox 360 controllers */
>  	XPAD_XBOXONE_VENDOR(0x045e),		/* Microsoft Xbox One controllers */

-- Rahul Rameshbabu
Rahul Rameshbabu July 24, 2023, 5:02 p.m. UTC | #2 
On Mon, 24 Jul, 2023 16:40:14 +0000 "Nguyen, Max" <maxwell.nguyen@hyperx.com> wrote:
> Hi Rahul,
>
>  
>
> My apologies for the resend.  We resent due to white space issues in the previous patches.  I will resend again and address the comments you
> mentioned below.
>
No worries. The point on resending was minor. If you make the changes to
the patch, be sure to make it a v2 rather than a resend.
>  
>
> The purpose of adding Xbox 360 specified class is to cover future products that will need this protocol support ahead of project launch date. 
> This is purely for testing during development.
>
Yeah, lets omit this from submission to the mainline kernel till the
future products with Xbox 360 support are implemented. Even then if they
support Xbox One devices, you do not need Xbox 360 vendor-specific class
support since you can use Xbox One class support. I have little knobs
like this too that I keep internal for testing purposes.
>  
>
> Regards,
>
> Max
>
In general, this type of reply is considered top-posting and is looked
down upon in the kernel community. I see you are likely using Outlook
which is not the best mailing option for interacting with the kernel
mailing lists.

* https://people.kernel.org/tglx/
* https://www.kernel.org/doc/html/latest/process/email-clients.html

-- Rahul Rameshbabu

>  
>
> From: Rahul Rameshbabu <rrameshbabu@nvidia.com> 
> Sent: Friday, July 21, 2023 7:46 PM
> To: HP Dev <hphyperxdev@gmail.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>; linux-input@vger.kernel.org; TOLEDANES, CHRIS <chris.toledanes@hp.com>; Ng, Carl
> <carl.ng@hp.com>; Nguyen, Max <maxwell.nguyen@hp.com>
> Subject: Re: [PATCH] Input: xpad - Add HyperX Clutch Gladiate support
>
>  
>
> CAUTION: External Email 
>
> On Fri, 21 Jul, 2023 18:05:32 -0700 HP Dev <hphyperxdev@gmail.com> wrote:
>> Add HyperX controller support to xpad_device and xpad_table.
>
> This patch appears to be a resend of a previous patch but does not
> follow the right practice for patch resends.
>
> https://docs.kernel.org/process/submitting-patches.html#don-t-get-discouraged-or-impatient
>
> https://lore.kernel.org/linux-input/ZKxVBULWtM30ZJ7D@google.com/
>
>>
>> Reported-by: Chris Toledanes <chris.toledanes@hp.com>
>
> I think this may not be an accurate use of the Reported-by: git trailer.
> My guess is you would want to use the Suggested-by: trailer here. This
> patch does not solve a "regression" in the xpad driver that was reported
> by someone or some automation but rather adds a new controller to be
> probed by the driver.
>
> https://docs.kernel.org/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
>
>> Reviewed-by: Carl Ng <carl.ng@hp.com>
>> Signed-off-by: Max Nguyen <maxwell.nguyen@hp.com>
>> ---
>> drivers/input/joystick/xpad.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
>> index cdb193317c3b..dfddb0bba8d8 100644
>> --- a/drivers/input/joystick/xpad.c
>> +++ b/drivers/input/joystick/xpad.c
>> @@ -130,6 +130,7 @@ static const struct xpad_device {
>> { 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
>> { 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
>> { 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
>> + { 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },
>> { 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
>> { 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
>> { 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
>> @@ -457,6 +458,8 @@ static const struct usb_device_id xpad_table[] = {
>> { USB_INTERFACE_INFO('X', 'B', 0) }, /* Xbox USB-IF not-approved class */
>> XPAD_XBOX360_VENDOR(0x0079), /* GPD Win 2 controller */
>> XPAD_XBOX360_VENDOR(0x03eb), /* Wooting Keyboards (Legacy) */
>> + XPAD_XBOX360_VENDOR(0x03f0), /* HP HyperX XBox 360 Controllers */
>
> There are no HP HyperX Xbox 360 controllers previously supported or
> added in this patch. I think the above line should be removed since you
> only have a XTYPE_XBOXONE type controller. You probably do not need to
> match against Xbox 360 vendor-specific class since you only implement an
> Xbox One controller.
>
>> + XPAD_XBOXONE_VENDOR(0x03f0), /* HP HyperX Xbox One Controllers */
>> XPAD_XBOX360_VENDOR(0x044f), /* Thrustmaster Xbox 360 controllers */
>> XPAD_XBOX360_VENDOR(0x045e), /* Microsoft Xbox 360 controllers */
>> XPAD_XBOXONE_VENDOR(0x045e), /* Microsoft Xbox One controllers */
>
> -- Rahul Rameshbabu
diff mbox series

Patch

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index cdb193317c3b..dfddb0bba8d8 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -130,6 +130,7 @@  static const struct xpad_device {
 	{ 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
 	{ 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
 	{ 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
+	{ 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },
 	{ 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
 	{ 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
 	{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
@@ -457,6 +458,8 @@  static const struct usb_device_id xpad_table[] = {
 	{ USB_INTERFACE_INFO('X', 'B', 0) },	/* Xbox USB-IF not-approved class */
 	XPAD_XBOX360_VENDOR(0x0079),		/* GPD Win 2 controller */
 	XPAD_XBOX360_VENDOR(0x03eb),		/* Wooting Keyboards (Legacy) */
+	XPAD_XBOX360_VENDOR(0x03f0),		/* HP HyperX XBox 360 Controllers */
+	XPAD_XBOXONE_VENDOR(0x03f0),		/* HP HyperX Xbox One Controllers */
 	XPAD_XBOX360_VENDOR(0x044f),		/* Thrustmaster Xbox 360 controllers */
 	XPAD_XBOX360_VENDOR(0x045e),		/* Microsoft Xbox 360 controllers */
 	XPAD_XBOXONE_VENDOR(0x045e),		/* Microsoft Xbox One controllers */