Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180920231308.237277-1-seobrien@chromium.org>
In-Reply-To: <20180920231308.237277-1-seobrien@chromium.org>
From:   "Sean O'Brien" <seobrien@chromium.org>
Date:   Thu, 27 Sep 2018 09:45:45 -0700
Message-ID: <CAOOzhkosyZpr+B1xwZcG3viNbWZFWr9WQtgGdP13i_+_Ocdgmg@mail.gmail.com>
Subject: Re: [PATCH] [v3] HID: add support for Apple Magic Trackpad 2
To:     Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc:     marek.wyborski@emwesoft.com, linux-kernel@vger.kernel.org,
        dmitry.torokhov@gmail.com, linux-input@vger.kernel.org,
        rydberg@bitmath.org, Jiri Kosina <jikos@kernel.org>,
        claudio@ponyfleisch.ch
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Gentle reminder, thank you!

On Thu, Sep 20, 2018 at 4:13 PM Sean O'Brien <seobrien@chromium.org> wrote:
>
> USB device
>         Vendor 05ac (Apple)
>         Device 0265 (Magic Trackpad 2)
> Bluetooth device
>         Vendor 004c (Apple)
>         Device 0265 (Magic Trackpad 2)
>
> Add support for Apple Magic Trackpad 2 over USB and bluetooth, putting
> the device in multi-touch mode.
>
> Signed-off-by: Claudio Mettler <claudio@ponyfleisch.ch>
> Signed-off-by: Marek Wyborski <marek.wyborski@emwesoft.com>
> Signed-off-by: Sean O'Brien <seobrien@chromium.org>
> ---
>
>  drivers/hid/hid-ids.h        |   1 +
>  drivers/hid/hid-magicmouse.c | 149 +++++++++++++++++++++++++++++++----
>  2 files changed, 134 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 5146ee029db4..bb0cd212c7cc 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -92,6 +92,7 @@
>  #define USB_DEVICE_ID_APPLE_MIGHTYMOUSE        0x0304
>  #define USB_DEVICE_ID_APPLE_MAGICMOUSE 0x030d
>  #define USB_DEVICE_ID_APPLE_MAGICTRACKPAD      0x030e
> +#define USB_DEVICE_ID_APPLE_MAGICTRACKPAD2     0x0265
>  #define USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI      0x020e
>  #define USB_DEVICE_ID_APPLE_FOUNTAIN_ISO       0x020f
>  #define USB_DEVICE_ID_APPLE_GEYSER_ANSI        0x0214
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index b454c4386157..6a3a6c83e509 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -54,6 +54,8 @@ module_param(report_undeciphered, bool, 0644);
>  MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state field using a MSC_RAW event");
>
>  #define TRACKPAD_REPORT_ID 0x28
> +#define TRACKPAD2_USB_REPORT_ID 0x02
> +#define TRACKPAD2_BT_REPORT_ID 0x31
>  #define MOUSE_REPORT_ID    0x29
>  #define DOUBLE_REPORT_ID   0xf7
>  /* These definitions are not precise, but they're close enough.  (Bits
> @@ -91,6 +93,17 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
>  #define TRACKPAD_RES_Y \
>         ((TRACKPAD_MAX_Y - TRACKPAD_MIN_Y) / (TRACKPAD_DIMENSION_Y / 100))
>
> +#define TRACKPAD2_DIMENSION_X (float)16000
> +#define TRACKPAD2_MIN_X -3678
> +#define TRACKPAD2_MAX_X 3934
> +#define TRACKPAD2_RES_X \
> +       ((TRACKPAD2_MAX_X - TRACKPAD2_MIN_X) / (TRACKPAD2_DIMENSION_X / 100))
> +#define TRACKPAD2_DIMENSION_Y (float)11490
> +#define TRACKPAD2_MIN_Y -2478
> +#define TRACKPAD2_MAX_Y 2587
> +#define TRACKPAD2_RES_Y \
> +       ((TRACKPAD2_MAX_Y - TRACKPAD2_MIN_Y) / (TRACKPAD2_DIMENSION_Y / 100))
> +
>  /**
>   * struct magicmouse_sc - Tracks Magic Mouse-specific data.
>   * @input: Input device through which we report events.
> @@ -183,6 +196,7 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
>  {
>         struct input_dev *input = msc->input;
>         int id, x, y, size, orientation, touch_major, touch_minor, state, down;
> +       int pressure = 0;
>
>         if (input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE) {
>                 id = (tdata[6] << 2 | tdata[5] >> 6) & 0xf;
> @@ -194,6 +208,20 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
>                 touch_minor = tdata[4];
>                 state = tdata[7] & TOUCH_STATE_MASK;
>                 down = state != TOUCH_STATE_NONE;
> +       } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
> +               id = tdata[8] & 0xf;
> +               x = (tdata[1] << 27 | tdata[0] << 19) >> 19;
> +               y = -((tdata[3] << 30 | tdata[2] << 22 | tdata[1] << 14) >> 19);
> +               size = tdata[6];
> +               orientation = (tdata[8] >> 5) - 4;
> +               touch_major = tdata[4];
> +               touch_minor = tdata[5];
> +               /* Add to pressure to prevent libraries such as libinput
> +                * from ignoring low pressure touches
> +                */
> +               pressure = tdata[7] + 30;
> +               state = tdata[3] & 0xC0;
> +               down = state == 0x80;
>         } else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
>                 id = (tdata[7] << 2 | tdata[6] >> 6) & 0xf;
>                 x = (tdata[1] << 27 | tdata[0] << 19) >> 19;
> @@ -215,7 +243,8 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
>         /* If requested, emulate a scroll wheel by detecting small
>          * vertical touch motions.
>          */
> -       if (emulate_scroll_wheel) {
> +       if (emulate_scroll_wheel && (input->id.product !=
> +                       USB_DEVICE_ID_APPLE_MAGICTRACKPAD2)) {
>                 unsigned long now = jiffies;
>                 int step_x = msc->touches[id].scroll_x - x;
>                 int step_y = msc->touches[id].scroll_y - y;
> @@ -269,10 +298,14 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
>                 input_report_abs(input, ABS_MT_POSITION_X, x);
>                 input_report_abs(input, ABS_MT_POSITION_Y, y);
>
> +               if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2)
> +                       input_report_abs(input, ABS_MT_PRESSURE, pressure);
> +
>                 if (report_undeciphered) {
>                         if (input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE)
>                                 input_event(input, EV_MSC, MSC_RAW, tdata[7]);
> -                       else /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
> +                       else if (input->id.product !=
> +                                       USB_DEVICE_ID_APPLE_MAGICTRACKPAD2)
>                                 input_event(input, EV_MSC, MSC_RAW, tdata[8]);
>                 }
>         }
> @@ -287,6 +320,7 @@ static int magicmouse_raw_event(struct hid_device *hdev,
>
>         switch (data[0]) {
>         case TRACKPAD_REPORT_ID:
> +       case TRACKPAD2_BT_REPORT_ID:
>                 /* Expect four bytes of prefix, and N*9 bytes of touch data. */
>                 if (size < 4 || ((size - 4) % 9) != 0)
>                         return 0;
> @@ -301,12 +335,22 @@ static int magicmouse_raw_event(struct hid_device *hdev,
>                         magicmouse_emit_touch(msc, ii, data + ii * 9 + 4);
>
>                 clicks = data[1];
> +               break;
> +       case TRACKPAD2_USB_REPORT_ID:
> +               /* Expect twelve bytes of prefix and N*9 bytes of touch data. */
> +               if (size < 12 || ((size - 12) % 9) != 0)
> +                       return 0;
> +               npoints = (size - 12) / 9;
> +               if (npoints > 15) {
> +                       hid_warn(hdev, "invalid size value (%d) for TRACKPAD2_USB_REPORT_ID\n",
> +                                       size);
> +                       return 0;
> +               }
> +               msc->ntouches = 0;
> +               for (ii = 0; ii < npoints; ii++)
> +                       magicmouse_emit_touch(msc, ii, data + ii * 9 + 12);
>
> -               /* The following bits provide a device specific timestamp. They
> -                * are unused here.
> -                *
> -                * ts = data[1] >> 6 | data[2] << 2 | data[3] << 10;
> -                */
> +               clicks = data[1];
>                 break;
>         case MOUSE_REPORT_ID:
>                 /* Expect six bytes of prefix, and N*8 bytes of touch data. */
> @@ -352,6 +396,9 @@ static int magicmouse_raw_event(struct hid_device *hdev,
>                 magicmouse_emit_buttons(msc, clicks & 3);
>                 input_report_rel(input, REL_X, x);
>                 input_report_rel(input, REL_Y, y);
> +       } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
> +               input_mt_sync_frame(input);
> +               input_report_key(input, BTN_MOUSE, clicks & 1);
>         } else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
>                 input_report_key(input, BTN_MOUSE, clicks & 1);
>                 input_mt_report_pointer_emulation(input, true);
> @@ -364,6 +411,7 @@ static int magicmouse_raw_event(struct hid_device *hdev,
>  static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hdev)
>  {
>         int error;
> +       int mt_flags = 0;
>
>         __set_bit(EV_KEY, input->evbit);
>
> @@ -380,6 +428,22 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd
>                         __set_bit(REL_WHEEL, input->relbit);
>                         __set_bit(REL_HWHEEL, input->relbit);
>                 }
> +       } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
> +               /* setting the device name to ensure the same driver settings
> +                * get loaded, whether connected through bluetooth or USB
> +                */
> +               input->name = "Apple Inc. Magic Trackpad 2";
> +
> +               __clear_bit(EV_MSC, input->evbit);
> +               __clear_bit(BTN_0, input->keybit);
> +               __clear_bit(BTN_RIGHT, input->keybit);
> +               __clear_bit(BTN_MIDDLE, input->keybit);
> +               __set_bit(BTN_MOUSE, input->keybit);
> +               __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> +               __set_bit(BTN_TOOL_FINGER, input->keybit);
> +
> +               mt_flags = INPUT_MT_POINTER | INPUT_MT_DROP_UNUSED |
> +                               INPUT_MT_TRACK;
>         } else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
>                 /* input->keybit is initialized with incorrect button info
>                  * for Magic Trackpad. There really is only one physical
> @@ -402,14 +466,13 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd
>
>         __set_bit(EV_ABS, input->evbit);
>
> -       error = input_mt_init_slots(input, 16, 0);
> +       error = input_mt_init_slots(input, 16, mt_flags);
>         if (error)
>                 return error;
>         input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255 << 2,
>                              4, 0);
>         input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 255 << 2,
>                              4, 0);
> -       input_set_abs_params(input, ABS_MT_ORIENTATION, -31, 32, 1, 0);
>
>         /* Note: Touch Y position from the device is inverted relative
>          * to how pointer motion is reported (and relative to how USB
> @@ -418,6 +481,7 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd
>          * inverse of the reported Y.
>          */
>         if (input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE) {
> +               input_set_abs_params(input, ABS_MT_ORIENTATION, -31, 32, 1, 0);
>                 input_set_abs_params(input, ABS_MT_POSITION_X,
>                                      MOUSE_MIN_X, MOUSE_MAX_X, 4, 0);
>                 input_set_abs_params(input, ABS_MT_POSITION_Y,
> @@ -427,7 +491,25 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd
>                                   MOUSE_RES_X);
>                 input_abs_set_res(input, ABS_MT_POSITION_Y,
>                                   MOUSE_RES_Y);
> +       } else if (input->id.product ==  USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
> +               input_set_abs_params(input, ABS_MT_PRESSURE, 0, 283, 0, 0);
> +               input_set_abs_params(input, ABS_PRESSURE, 0, 283, 0, 0);
> +               input_set_abs_params(input, ABS_MT_ORIENTATION, -3, 4, 0, 0);
> +               input_set_abs_params(input, ABS_X, TRACKPAD2_MIN_X,
> +                                    TRACKPAD2_MAX_X, 0, 0);
> +               input_set_abs_params(input, ABS_Y, TRACKPAD2_MIN_Y,
> +                                    TRACKPAD2_MAX_Y, 0, 0);
> +               input_set_abs_params(input, ABS_MT_POSITION_X,
> +                                    TRACKPAD2_MIN_X, TRACKPAD2_MAX_X, 0, 0);
> +               input_set_abs_params(input, ABS_MT_POSITION_Y,
> +                                    TRACKPAD2_MIN_Y, TRACKPAD2_MAX_Y, 0, 0);
> +
> +               input_abs_set_res(input, ABS_X, TRACKPAD2_RES_X);
> +               input_abs_set_res(input, ABS_Y, TRACKPAD2_RES_Y);
> +               input_abs_set_res(input, ABS_MT_POSITION_X, TRACKPAD2_RES_X);
> +               input_abs_set_res(input, ABS_MT_POSITION_Y, TRACKPAD2_RES_Y);
>         } else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
> +               input_set_abs_params(input, ABS_MT_ORIENTATION, -31, 32, 1, 0);
>                 input_set_abs_params(input, ABS_X, TRACKPAD_MIN_X,
>                                      TRACKPAD_MAX_X, 4, 0);
>                 input_set_abs_params(input, ABS_Y, TRACKPAD_MIN_Y,
> @@ -447,7 +529,8 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd
>
>         input_set_events_per_packet(input, 60);
>
> -       if (report_undeciphered) {
> +       if (report_undeciphered &&
> +           input->id.product != USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
>                 __set_bit(EV_MSC, input->evbit);
>                 __set_bit(MSC_RAW, input->mscbit);
>         }
> @@ -465,7 +548,8 @@ static int magicmouse_input_mapping(struct hid_device *hdev,
>                 msc->input = hi->input;
>
>         /* Magic Trackpad does not give relative data after switching to MT */
> -       if (hi->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD &&
> +       if ((hi->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD ||
> +            hi->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) &&
>             field->flags & HID_MAIN_ITEM_RELATIVE)
>                 return -1;
>
> @@ -494,11 +578,20 @@ static int magicmouse_input_configured(struct hid_device *hdev,
>  static int magicmouse_probe(struct hid_device *hdev,
>         const struct hid_device_id *id)
>  {
> -       const u8 feature[] = { 0xd7, 0x01 };
> +       const u8 *feature;
> +       const u8 feature_mt[] = { 0xD7, 0x01 };
> +       const u8 feature_mt_trackpad2_usb[] = { 0x02, 0x01 };
> +       const u8 feature_mt_trackpad2_bt[] = { 0xF1, 0x02, 0x01 };
>         u8 *buf;
>         struct magicmouse_sc *msc;
>         struct hid_report *report;
>         int ret;
> +       int feature_size;
> +
> +       if (id->vendor == USB_VENDOR_ID_APPLE &&
> +           id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 &&
> +           hdev->type != HID_TYPE_USBMOUSE)
> +               return 0;
>
>         msc = devm_kzalloc(&hdev->dev, sizeof(*msc), GFP_KERNEL);
>         if (msc == NULL) {
> @@ -532,7 +625,14 @@ static int magicmouse_probe(struct hid_device *hdev,
>         if (id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE)
>                 report = hid_register_report(hdev, HID_INPUT_REPORT,
>                         MOUSE_REPORT_ID, 0);
> -       else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
> +       else if (id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
> +               if (id->vendor == BT_VENDOR_ID_APPLE)
> +                       report = hid_register_report(hdev, HID_INPUT_REPORT,
> +                               TRACKPAD2_BT_REPORT_ID, 0);
> +               else /* USB_VENDOR_ID_APPLE */
> +                       report = hid_register_report(hdev, HID_INPUT_REPORT,
> +                               TRACKPAD2_USB_REPORT_ID, 0);
> +       } else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
>                 report = hid_register_report(hdev, HID_INPUT_REPORT,
>                         TRACKPAD_REPORT_ID, 0);
>                 report = hid_register_report(hdev, HID_INPUT_REPORT,
> @@ -546,7 +646,20 @@ static int magicmouse_probe(struct hid_device *hdev,
>         }
>         report->size = 6;
>
> -       buf = kmemdup(feature, sizeof(feature), GFP_KERNEL);
> +       if (id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) {
> +               if (id->vendor == BT_VENDOR_ID_APPLE) {
> +                       feature_size = sizeof(feature_mt_trackpad2_bt);
> +                       feature = feature_mt_trackpad2_bt;
> +               } else { /* USB_VENDOR_ID_APPLE */
> +                       feature_size = sizeof(feature_mt_trackpad2_usb);
> +                       feature = feature_mt_trackpad2_usb;
> +               }
> +       } else {
> +               feature_size = sizeof(feature_mt);
> +               feature = feature_mt;
> +       }
> +
> +       buf = kmemdup(feature, feature_size, GFP_KERNEL);
>         if (!buf) {
>                 ret = -ENOMEM;
>                 goto err_stop_hw;
> @@ -560,10 +673,10 @@ static int magicmouse_probe(struct hid_device *hdev,
>          * but there seems to be no other way of switching the mode.
>          * Thus the super-ugly hacky success check below.
>          */
> -       ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(feature),
> +       ret = hid_hw_raw_request(hdev, buf[0], buf, feature_size,
>                                 HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
>         kfree(buf);
> -       if (ret != -EIO && ret != sizeof(feature)) {
> +       if (ret != -EIO && ret != feature_size) {
>                 hid_err(hdev, "unable to request touch data (%d)\n", ret);
>                 goto err_stop_hw;
>         }
> @@ -579,6 +692,10 @@ static const struct hid_device_id magic_mice[] = {
>                 USB_DEVICE_ID_APPLE_MAGICMOUSE), .driver_data = 0 },
>         { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
>                 USB_DEVICE_ID_APPLE_MAGICTRACKPAD), .driver_data = 0 },
> +       { HID_BLUETOOTH_DEVICE(BT_VENDOR_ID_APPLE,
> +               USB_DEVICE_ID_APPLE_MAGICTRACKPAD2), .driver_data = 0 },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_APPLE,
> +               USB_DEVICE_ID_APPLE_MAGICTRACKPAD2), .driver_data = 0 },
>         { }
>  };
>  MODULE_DEVICE_TABLE(hid, magic_mice);
> --
> 2.19.0.444.g18242da7ef-goog
>