Message ID | 20220408140902.15966-1-dbdaniel42@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Delegated to: | Jiri Kosina |
Headers | show |
Series | HID: microsoft: Fix button/axis mapping for Xbox One S Controller | expand |
On 4/8/22 09:09, Daniel Bomar wrote: > Remaps several buttons and axes to match how these are mapped in the > xpad driver (same controller over USB). > > This is also how they are documented to be mapped in > Documentation/input/gamepad.rst > --- Hi Daniel, I don't think this is a good idea. Remapping this is going to cause problems for userspace applications and libraries that already have mappings for this gamepad (such as SDL [0]). At the very least, you should set a bit in the version like hid-sony.c does so SDL can tell the new and old mappings apart. However, I still don't think it's worth making this change at all. In the _best_ case, you break a ton of existing games and applications that have support for this gamepad already and achieve very little practical gain. I believe this change will break Android's mapping too [1]. Using GAS/BRAKE for triggers is fairly common for gamepads designed for Android, so it's likely that Microsoft did it this way on purpose. I don't think we should be overruling their conscious design decisions without very compelling reasons. It's definitely not ideal that the mappings between USB and BT differ, but breaking existing applications by changing it is much worse. [0]: https://github.com/libsdl-org/SDL/blob/505d6a4a052592b2676f87456c1f564daa8d2c50/src/joystick/SDL_gamecontrollerdb.h#L795 [1]: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/data/keyboards/Vendor_045e_Product_02fd.kl Regards, Cameron > drivers/hid/hid-microsoft.c | 73 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 72 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c > index 071fd093a5f4..903e09a3d898 100644 > --- a/drivers/hid/hid-microsoft.c > +++ b/drivers/hid/hid-microsoft.c > @@ -27,6 +27,7 @@ > #define MS_DUPLICATE_USAGES BIT(5) > #define MS_SURFACE_DIAL BIT(6) > #define MS_QUIRK_FF BIT(7) > +#define MS_XBOX BIT(8) > > struct ms_data { > unsigned long quirks; > @@ -179,6 +180,70 @@ static int ms_surface_dial_quirk(struct hid_input *hi, struct hid_field *field, > return 0; > } > > +#define ms_map_abs_clear(c) hid_map_usage_clear(hi, usage, bit, max, \ > + EV_ABS, (c)) > +/* > + * Remap buttons and axes on Xbox controllers over bluetooth so they match > + * with the xpad driver (USB interface) and with mapping specified in > + * Documentation/input/gamepad.rst > +*/ > +static int ms_xbox_quirk(struct hid_input *hi, struct hid_usage *usage, > + unsigned long **bit, int *max) > +{ > + int code; > + switch (usage->hid & HID_USAGE_PAGE) { > + /* > + * Remap "Xbox" and Select buttons from consumer page to gamepad buttons. > + * This allows these buttons to show up on the /dev/input/js* interface. > + */ > + case HID_UP_CONSUMER: > + switch (usage->hid & HID_USAGE) { > + case 0x223: > + ms_map_key_clear(BTN_MODE); > + return 1; > + case 0x224: > + ms_map_key_clear(BTN_SELECT); > + return 1; > + } > + break; > + /* These buttons do not physically exist on the controller. Ignore them. */ > + case HID_UP_BUTTON: > + code = ((usage->hid - 1) & HID_USAGE) + BTN_GAMEPAD; > + switch (code) { > + case BTN_C: > + case BTN_Z: > + case BTN_TL2: > + case BTN_TR2: > + return -1; > + } > + break; > + /* Remap right joystick to RX/RY */ > + case HID_UP_GENDESK: > + switch (usage->hid) { > + case HID_GD_Z: > + ms_map_abs_clear(ABS_RX); > + return 1; > + case HID_GD_RZ: > + ms_map_abs_clear(ABS_RY); > + return 1; > + } > + break; > + /* Remap left and right triggers from "gas" and "break" to RZ/Z */ > + case HID_UP_SIMULATION: > + switch (usage->hid & HID_USAGE) { > + case 0xc4: > + ms_map_abs_clear(ABS_RZ); > + return 1; > + case 0xc5: > + ms_map_abs_clear(ABS_Z); > + return 1; > + } > + break; > + } > + > + return 0; > +} > + > static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi, > struct hid_field *field, struct hid_usage *usage, > unsigned long **bit, int *max) > @@ -203,6 +268,12 @@ static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi, > return ret; > } > > + if (quirks & MS_XBOX) { > + int ret = ms_xbox_quirk(hi, usage, bit, max); > + if (ret) > + return ret; > + } > + > return 0; > } > > @@ -447,7 +518,7 @@ static const struct hid_device_id ms_devices[] = { > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 0x091B), > .driver_data = MS_SURFACE_DIAL }, > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER), > - .driver_data = MS_QUIRK_FF }, > + .driver_data = MS_QUIRK_FF | MS_XBOX }, > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS), > .driver_data = MS_QUIRK_FF }, > { } >
diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c index 071fd093a5f4..903e09a3d898 100644 --- a/drivers/hid/hid-microsoft.c +++ b/drivers/hid/hid-microsoft.c @@ -27,6 +27,7 @@ #define MS_DUPLICATE_USAGES BIT(5) #define MS_SURFACE_DIAL BIT(6) #define MS_QUIRK_FF BIT(7) +#define MS_XBOX BIT(8) struct ms_data { unsigned long quirks; @@ -179,6 +180,70 @@ static int ms_surface_dial_quirk(struct hid_input *hi, struct hid_field *field, return 0; } +#define ms_map_abs_clear(c) hid_map_usage_clear(hi, usage, bit, max, \ + EV_ABS, (c)) +/* + * Remap buttons and axes on Xbox controllers over bluetooth so they match + * with the xpad driver (USB interface) and with mapping specified in + * Documentation/input/gamepad.rst +*/ +static int ms_xbox_quirk(struct hid_input *hi, struct hid_usage *usage, + unsigned long **bit, int *max) +{ + int code; + switch (usage->hid & HID_USAGE_PAGE) { + /* + * Remap "Xbox" and Select buttons from consumer page to gamepad buttons. + * This allows these buttons to show up on the /dev/input/js* interface. + */ + case HID_UP_CONSUMER: + switch (usage->hid & HID_USAGE) { + case 0x223: + ms_map_key_clear(BTN_MODE); + return 1; + case 0x224: + ms_map_key_clear(BTN_SELECT); + return 1; + } + break; + /* These buttons do not physically exist on the controller. Ignore them. */ + case HID_UP_BUTTON: + code = ((usage->hid - 1) & HID_USAGE) + BTN_GAMEPAD; + switch (code) { + case BTN_C: + case BTN_Z: + case BTN_TL2: + case BTN_TR2: + return -1; + } + break; + /* Remap right joystick to RX/RY */ + case HID_UP_GENDESK: + switch (usage->hid) { + case HID_GD_Z: + ms_map_abs_clear(ABS_RX); + return 1; + case HID_GD_RZ: + ms_map_abs_clear(ABS_RY); + return 1; + } + break; + /* Remap left and right triggers from "gas" and "break" to RZ/Z */ + case HID_UP_SIMULATION: + switch (usage->hid & HID_USAGE) { + case 0xc4: + ms_map_abs_clear(ABS_RZ); + return 1; + case 0xc5: + ms_map_abs_clear(ABS_Z); + return 1; + } + break; + } + + return 0; +} + static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi, struct hid_field *field, struct hid_usage *usage, unsigned long **bit, int *max) @@ -203,6 +268,12 @@ static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi, return ret; } + if (quirks & MS_XBOX) { + int ret = ms_xbox_quirk(hi, usage, bit, max); + if (ret) + return ret; + } + return 0; } @@ -447,7 +518,7 @@ static const struct hid_device_id ms_devices[] = { { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 0x091B), .driver_data = MS_SURFACE_DIAL }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER), - .driver_data = MS_QUIRK_FF }, + .driver_data = MS_QUIRK_FF | MS_XBOX }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS), .driver_data = MS_QUIRK_FF }, { }