Re: [PATCH] Input: xpad - add support for XBOX One Elite paddles

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 18, 2022 at 12:46:53PM +0200, Pavel Rojtberg wrote:
> 
> 
> Am 18.04.22 um 08:20 schrieb Greg KH:
> > On Sun, Apr 17, 2022 at 06:19:08PM +0200, Pavel Rojtberg wrote:
> >> From: Christopher Crockett <chaorace@xxxxxxxxx>
> >>
> >> An effort has been made to support every official model and firmware
> >> version I could track down info on. The following controllers _should_
> >> have working paddles with this PR:
> >> - Xbox Elite (**untested**)
> >> - Xbox Elite Series 2 on early firmwares (**untested**)
> >> - Xbox Elite Series 2 on v4 firmwares (Tested v4.8.1908.0)
> >> - Xbox Elite Series 2 on v5 pre-BLE firmwares (**untested**)
> >> - Xbox Elite Series 2 on v5 post-BLE firmwares (Tested v5.13.3143.0)
> >>
> >> This patch also introduces correct handling for the Elite 1 controller
> >> and properly suppresses paddle inputs when using a custom profile slot.
> >>
> >> Starting in v5.11, certain inputs for the Elite 2 were moved to an extra
> >> packet that is not enabled by default.
> > 
> > why does 5.11 matter here?
> 
> This refers to the gamepad firmware, not to Linux :)

Ah, you should make that obvious :)

> >>
> >> We must first manually enable this extra packet in order to correctly
> >> process paddle input data with these later firmwares.
> >>
> >> For further details see: https://github.com/paroj/xpad/pull/195
> > 
> > don't like to random web sites, summarize in here properly.
> 
> The summary here should be complete. Do you have any specific questions?

If the summary is fine, no need to link to the github location, right?

> >> Signed-off-by: Fmstrat <nospam@xxxxxxxxxx>
> > 
> > I doubt that is a correct email address and valid name :(
> 
> Unfortunately this is all I get at github. The only alternative would be no attribution at all.

Did you ask?

> >> Signed-off-by: Pavel Rojtberg <rojtberg@xxxxxxxxx>
> >> ---
> >>  drivers/input/joystick/xpad.c | 239 ++++++++++++++++++++++++++++++++++++++++++---------------
> >>  1 file changed, 176 insertions(+), 63 deletions(-)
> >>
> >> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> >> index 53126d9..0746813 100644
> >> --- a/drivers/input/joystick/xpad.c
> >> +++ b/drivers/input/joystick/xpad.c
> >> @@ -80,6 +80,7 @@
> >>  #define MAP_TRIGGERS_TO_BUTTONS		(1 << 1)
> >>  #define MAP_STICKS_TO_NULL		(1 << 2)
> >>  #define MAP_SELECT_BUTTON		(1 << 3)
> >> +#define MAP_PADDLES				(1 << 4)
> >>  #define DANCEPAD_MAP_CONFIG	(MAP_DPAD_TO_BUTTONS |			\
> >>  				MAP_TRIGGERS_TO_BUTTONS | MAP_STICKS_TO_NULL)
> >>  
> >> @@ -89,6 +90,12 @@
> >>  #define XTYPE_XBOXONE     3
> >>  #define XTYPE_UNKNOWN     4
> >>  
> >> +#define PKT_XB              0
> >> +#define PKT_XBE1            1
> >> +#define PKT_XBE2_FW_OLD     2
> >> +#define PKT_XBE2_FW_5_EARLY 3
> >> +#define PKT_XBE2_FW_5_11    4
> >> +
> >>  static bool dpad_to_buttons;
> >>  module_param(dpad_to_buttons, bool, S_IRUGO);
> >>  MODULE_PARM_DESC(dpad_to_buttons, "Map D-PAD to buttons rather than axes for unknown pads");
> >> @@ -111,6 +118,7 @@ static const struct xpad_device {
> >>  	char *name;
> >>  	u8 mapping;
> >>  	u8 xtype;
> >> +	u8 pktType;
> > 
> > Please use proper Linux kernel naming schemes.
> 
> that would be pkt_type, right?

You have vowels, "packet_type" is nicer.

> >>  } xpad_device[] = {
> >>  	{ 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
> >>  	{ 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
> >> @@ -128,7 +136,8 @@ static const struct xpad_device {
> >>  	{ 0x045e, 0x0291, "Xbox 360 Wireless Receiver (XBOX)", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
> >>  	{ 0x045e, 0x02d1, "Microsoft X-Box One pad", 0, XTYPE_XBOXONE },
> >>  	{ 0x045e, 0x02dd, "Microsoft X-Box One pad (Firmware 2015)", 0, XTYPE_XBOXONE },
> >> -	{ 0x045e, 0x02e3, "Microsoft X-Box One Elite pad", 0, XTYPE_XBOXONE },
> >> +	{ 0x045e, 0x02e3, "Microsoft X-Box One Elite pad", MAP_PADDLES, XTYPE_XBOXONE },
> >> +	{ 0x045e, 0x0b00, "Microsoft X-Box One Elite 2 pad", MAP_PADDLES, XTYPE_XBOXONE },
> >>  	{ 0x045e, 0x02ea, "Microsoft X-Box One S pad", 0, XTYPE_XBOXONE },
> >>  	{ 0x045e, 0x0719, "Xbox 360 Wireless Receiver", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360W },
> >>  	{ 0x045e, 0x0b12, "Microsoft Xbox Series S|X Controller", MAP_SELECT_BUTTON, XTYPE_XBOXONE },
> >> @@ -390,6 +399,13 @@ static const signed short xpad_abs_triggers[] = {
> >>  	-1
> >>  };
> >>  
> >> +/* used when the controller has extra paddle buttons */
> >> +static const signed short xpad_btn_paddles[] = {
> >> +	BTN_TRIGGER_HAPPY5, BTN_TRIGGER_HAPPY6, /* paddle upper right, lower right */
> >> +	BTN_TRIGGER_HAPPY7, BTN_TRIGGER_HAPPY8, /* paddle upper left, lower left */
> >> +	-1						/* terminating entry */
> > 
> > 0 should be the terminator, right?
> 
> while 0 would probably do, the other arrays in this file also use -1.

Ah, ok, nevermind.

> Do I read correctly, that your comments merely aiming at style mean that
> you are ok with the code in general?

Yes, the logic seems fine if it has been tested by others.

thanks,

greg k-h



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux