Re: [PATCH] HID: quirk for Saitek RAT7 and MMO7 mices' mode button

Sorry for the repost, my gmail decided to send HTML instead of plain
text... :(

On 06/01/14 12:24, Benjamin Tissoires wrote:

Hi Conan,

Peter asked me to have a look at a similar device in the X bugzilla [1],
so I will also comment here in the hope we will finally find an upstream
solution.


On Thu, Jan 2, 2014 at 2:56 PM, Conan <conan@braincalibration.de
<mailto:conan@braincalibration.de>> wrote:

    -----BEGIN PGP SIGNED MESSAGE-----
    Hash: SHA1

    Hi all,

    this patch adds support for some saitek mice which implement a tristate
    button (for switching button mappings in the official windows driver) by
    keeping one of three (non-physical) buttons constantly pressed. This
    breaks X and probably other userspace software.

    The patch below implements a quirk which instantly sends a release event
    for the affected buttons.

    Disclaimer: this is not entirely my own work, it is based on a patch for
    the R.A.T.7. which was posted here:
    http://us.generation-nt.com/answer/patch-2-6-38-mode-button-quirk-saitek-cyborg-t-7-help-202649672.html

    I merely added two usb ids and kept it up to date.


Please add your Signed-off-by line and use your real name (see
Documentation/SubmittingPatches in the kernel tree).

Also add Jiri in CC if you ever want it to be included in his tree
(which I assume you want).
 


    - --- a/drivers/hid/hid-ids.h   2013-10-08 15:20:58.781792791 +0200
    +++ b/drivers/hid/hid-ids.h     2013-10-08 15:22:17.815804729 +0200
    @@ -714,6 +714,9 @@
     #define USB_VENDOR_ID_SAITEK           0x06a3
     #define USB_DEVICE_ID_SAITEK_RUMBLEPAD 0xff17
     #define USB_DEVICE_ID_SAITEK_PS1000    0x0621
    +#define USB_DEVICE_ID_SAITEK_RAT7      0x0ccb
    +#define USB_DEVICE_ID_SAITEK_RAT7TOO   0x0cd7
    +#define USB_DEVICE_ID_SAITEK_MMO7      0x0cd0

     #define USB_VENDOR_ID_SAMSUNG          0x0419
     #define USB_DEVICE_ID_SAMSUNG_IR_REMOTE        0x0001
    - --- a/drivers/hid/hid-input.c 2013-01-31 19:09:35.058570485 +0100
    +++ b/drivers/hid/hid-input.c   2013-01-31 19:11:57.779833199 +0100


This should go into hid-saitek, not hid-input. It is really specific and
no other device will benefit from it, so move it where it belongs.
 

    @@ -1039,6 +1039,25 @@

            if ((field->flags & HID_MAIN_ITEM_RELATIVE) && (usage->type
    == EV_KEY))
                    input_event(input, usage->type, usage->code, 0);
    +
    +       /* hack for Saitek RAT mice which report release events for
    their
    +        * mode button on the NEXT press event: instant release
    +       */
    +       if ((*quirks & HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE) &&
    +               value && usage->type == EV_KEY &&
    +               /* RAT7 uses buttons 13,14,15 */
    +               ((usage->code >= BTN_MOUSE + 8 && usage->code <=
    BTN_MOUSE + 10) ||
    +               /* MMO7 uses button 20 */
    +               (usage->code == BTN_MOUSE + 15)) &&
    +               test_bit(usage->code, input->key)) {
    +               input_event(input, usage->type, usage->code, 0);
    +               /* we'll get a real release event from the mouse
    anyway, and
    +                * userspace should cope with the extra input-layer
    +                * button-up events anyway; just re-set the bit to stop
    +                * spurious button-down events
    +                */
    +               set_bit(usage->code, input->key);


This is just weird. input_event should take care of that. I bet you are
missing an input_sync event before releasing the key (or after).
 

    +       }
     }


     void hidinput_report_event(struct hid_device *hid, struct hid_report
    *report)
    - --- a/drivers/hid/usbhid/hid-quirks.c 2013-01-31
    19:09:35.058570485 +0100
    +++ b/drivers/hid/usbhid/hid-quirks.c   2013-01-31
    19:12:58.195232311 +0100
    @@ -82,6 +82,9 @@
            { USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3001,
    HID_QUIRK_NOGET },
            { USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3008,
    HID_QUIRK_NOGET },
            { USB_VENDOR_ID_REALTEK, USB_DEVICE_ID_REALTEK_READER,
    HID_QUIRK_NO_INIT_REPORTS },
    +       { USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7,
    HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE },
    +       { USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7TOO,
    HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE },
    +       { USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_MMO7,
    HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE },
            { USB_VENDOR_ID_SENNHEISER, USB_DEVICE_ID_SENNHEISER_BTD500USB,
    HID_QUIRK_NOGET },
            { USB_VENDOR_ID_SIGMATEL, USB_DEVICE_ID_SIGMATEL_STMP3780,
    HID_QUIRK_NOGET },
            { USB_VENDOR_ID_SUN, USB_DEVICE_ID_RARITAN_KVM_DONGLE,
    HID_QUIRK_NOGET },
    - --- a/include/linux/hid.h     2013-05-18 03:16:26.713052127 +0200
    +++ b/include/linux/hid.h       2013-05-18 03:17:23.407343351 +0200
    @@ -283,6 +283,7 @@
     #define HID_QUIRK_MULTI_INPUT                  0x00000040
     #define HID_QUIRK_HIDINPUT_FORCE               0x00000080
     #define HID_QUIRK_NO_EMPTY_INPUT               0x00000100
    +#define HID_QUIRK_RAT_BROKEN_BUTTON_RELEASE    0x00000200


Please do not add too specific fixup in the generic hid layer. (see the
hid-saitek comment above).
 

     #define HID_QUIRK_SKIP_OUTPUT_REPORTS          0x00010000
     #define HID_QUIRK_FULLSPEED_INTERVAL           0x10000000
     #define HID_QUIRK_NO_INIT_REPORTS              0x20000000

    - --
    Regards,
    C


Two more comments:
- given the Xorg bug, I think we should map the physical button to only
one input button, not three (the goal is to remove the state, so we
should do it correctly)
- I can give you a hand in developing/debugging if you sent me some
reports with hid-recorder [2]. Send some regular reports + the faulty
button presses, and I'll be able to test it on my laptop.

Cheers,
Benjamin

[1] https://bugs.freedesktop.org/show_bug.cgi?id=32200
[2] http://bentiss.github.io/hid-replay-docs/