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