Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 1/2] mfd: cros: add "base attached" MKBP switch definition
From: Dmitry Torokhov @ 2018-10-01 21:39 UTC (permalink / raw)
  To: Jiri Kosina, Lee Jones
  Cc: Benjamin Tissoires, linux-input, linux-kernel, Nicolas Boichat

This adds a "base attached" switch definition to the MKBP protocol that
is used by Whiskers driver to properly determine device state (clamshell
vs tablet mode).

Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
---

Lee, I was wondering if it woudl be OK for cros_ec_commands.h to be
merged through HID tree.


 include/linux/mfd/cros_ec_commands.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 20ee71f10865..5fd0e429f472 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -2132,6 +2132,7 @@ struct ec_response_get_next_event_v1 {
 /* Switches */
 #define EC_MKBP_LID_OPEN	0
 #define EC_MKBP_TABLET_MODE	1
+#define EC_MKBP_BASE_ATTACHED	2
 
 /*****************************************************************************/
 /* Temperature sensor commands */
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* Re: [PATCH] input/psmouse: Don't hold the mutex while calling ->disconnect
From: Daniel Vetter @ 2018-10-01  9:23 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Stephen Lyons, intel-gfx, linux-input, Linux Kernel Mailing List,
	Benjamin Tissoires, Arvind Yadav, Daniel Vetter
In-Reply-To: <CAKMK7uFDwANOAF3j43g4S0jQ4V40b-_oiX+QSy8SRkL26Zwxzg@mail.gmail.com>

On Wed, May 2, 2018 at 11:06 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> On Mon, Apr 30, 2018 at 11:17 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Daniel,
> >
> > On Mon, Apr 30, 2018 at 09:56:49PM +0200, Daniel Vetter wrote:
> >> At least trackpoint_disconnect wants to remove some sysfs files, and
> >> we can't remove sysfs files while holding psmouse_mutex:
> >>
> >> ======================================================
> >> WARNING: possible circular locking dependency detected
> >> 4.16.0-rc5-g613eb885b69e-drmtip_1+ #1 Tainted: G     U
> >> ------------------------------------------------------
> >> kworker/0:3/102 is trying to acquire lock:
> >>  (kn->count#130){++++}, at: [<000000009679748b>] kernfs_remove_by_name_ns+0x3b/0x80
> >>
> >> but task is already holding lock:
> >>  (psmouse_mutex){+.+.}, at: [<0000000014f44bcc>] psmouse_disconnect+0x62/0x160
> >>
> >> which lock already depends on the new lock.
> >>
> >> the existing dependency chain (in reverse order) is:
> >>
> >> -> #1 (psmouse_mutex){+.+.}:
> >>        psmouse_attr_set_helper+0x28/0x140
> >>        kernfs_fop_write+0xfe/0x180
> >>        __vfs_write+0x1e/0x130
> >>        vfs_write+0xbd/0x1b0
> >>        SyS_write+0x40/0xa0
> >>        do_syscall_64+0x65/0x1a0
> >>        entry_SYSCALL_64_after_hwframe+0x42/0xb7
> >>
> >> -> #0 (kn->count#130){++++}:
> >>        __kernfs_remove+0x243/0x2b0
> >>        kernfs_remove_by_name_ns+0x3b/0x80
> >>        remove_files.isra.0+0x2b/0x60
> >>        sysfs_remove_group+0x38/0x80
> >>        sysfs_remove_groups+0x24/0x40
> >>        trackpoint_disconnect+0x2c/0x50
> >>        psmouse_disconnect+0x8f/0x160
> >>        serio_disconnect_driver+0x28/0x40
> >>        serio_driver_remove+0xc/0x10
> >>        device_release_driver_internal+0x15b/0x230
> >>        serio_handle_event+0x1c8/0x260
> >>        process_one_work+0x215/0x620
> >>        worker_thread+0x48/0x3a0
> >>        kthread+0xfb/0x130
> >>        ret_from_fork+0x3a/0x50
> >>
> >> other info that might help us debug this:
> >>
> >>  Possible unsafe locking scenario:
> >>
> >>        CPU0                    CPU1
> >>        ----                    ----
> >>   lock(psmouse_mutex);
> >>                                lock(kn->count#130);
> >>                                lock(psmouse_mutex);
> >>   lock(kn->count#130);
> >>
> >>  *** DEADLOCK ***
> >>
> >> 6 locks held by kworker/0:3/102:
> >>  #0:  ((wq_completion)"events_long"){+.+.}, at: [<000000002e408bfa>] process_one_work+0x191/0x620
> >>  #1:  (serio_event_work){+.+.}, at: [<000000002e408bfa>] process_one_work+0x191/0x620
> >>  #2:  (serio_mutex){+.+.}, at: [<00000000c8a49847>] serio_handle_event+0x23/0x260
> >>  #3:  (&dev->mutex){....}, at: [<00000000b55eee75>] device_release_driver_internal+0x2f/0x230
> >>  #4:  (&serio->drv_mutex){+.+.}, at: [<000000009719f997>] serio_disconnect_driver+0x16/0x40
> >>  #5:  (psmouse_mutex){+.+.}, at: [<0000000014f44bcc>] psmouse_disconnect+0x62/0x160
> >>
> >> stack backtrace:
> >> CPU: 0 PID: 102 Comm: kworker/0:3 Tainted: G     U           4.16.0-rc5-g613eb885b69e-drmtip_1+ #1
> >> Hardware name: LENOVO 74591P0/74591P0, BIOS 6DET28WW (1.05 ) 07/30/2008
> >> Workqueue: events_long serio_handle_event
> >> Call Trace:
> >>  dump_stack+0x5f/0x86
> >>  print_circular_bug.isra.18+0x1d0/0x2c0
> >>  __lock_acquire+0x14ae/0x1b60
> >>  ? kernfs_remove_by_name_ns+0x20/0x80
> >>  ? lock_acquire+0xaf/0x200
> >>  lock_acquire+0xaf/0x200
> >>  ? kernfs_remove_by_name_ns+0x3b/0x80
> >>  __kernfs_remove+0x243/0x2b0
> >>  ? kernfs_remove_by_name_ns+0x3b/0x80
> >>  ? kernfs_name_hash+0xd/0x70
> >>  ? kernfs_find_ns+0x7e/0x100
> >>  kernfs_remove_by_name_ns+0x3b/0x80
> >>  remove_files.isra.0+0x2b/0x60
> >>  sysfs_remove_group+0x38/0x80
> >>  sysfs_remove_groups+0x24/0x40
> >>  trackpoint_disconnect+0x2c/0x50
> >>  psmouse_disconnect+0x8f/0x160
> >>  serio_disconnect_driver+0x28/0x40
> >>  serio_driver_remove+0xc/0x10
> >>  device_release_driver_internal+0x15b/0x230
> >>  serio_handle_event+0x1c8/0x260
> >>  process_one_work+0x215/0x620
> >>  worker_thread+0x48/0x3a0
> >>  ? _raw_spin_unlock_irqrestore+0x4c/0x60
> >>  kthread+0xfb/0x130
> >>  ? process_one_work+0x620/0x620
> >>  ? _kthread_create_on_node+0x30/0x30
> >>  ret_from_fork+0x3a/0x50
> >>
> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
> >> Cc: Stephen Lyons <slysven@virginmedia.com>
> >> Cc: linux-input@vger.kernel.org
> >> ---
> >>  drivers/input/mouse/psmouse-base.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> >> index 8900c3166ebf..06ccd8e22f3c 100644
> >> --- a/drivers/input/mouse/psmouse-base.c
> >> +++ b/drivers/input/mouse/psmouse-base.c
> >> @@ -1484,8 +1484,10 @@ static void psmouse_disconnect(struct serio *serio)
> >>               psmouse_deactivate(parent);
> >>       }
> >>
> >> +     mutex_unlock(&psmouse_mutex);
> >>       if (psmouse->disconnect)
> >>               psmouse->disconnect(psmouse);
> >> +     mutex_lock(&psmouse_mutex);
> >
> > Why do you think it is proper to drop this mutex? It is introduced for a
> > reason.
> >
> > I think the trace you are seeing is due to:
> >
> > commit 988cd7afb3f37598891ca70b4c6eb914c338c46a
> > Author: Tejun Heo <tj@kernel.org>
> > Date:   Mon Feb 3 14:02:58 2014 -0500
> >
> >     kernfs: remove kernfs_addrm_cxt
> >
> > where we started taking kernfs_mutex when adding/removing sysfs files.
> > Ultimately I think we may have to change switching protocol to a work
> > item to be done asynchronously from writing to sysfs attribute.
>
> Well "holding a lock while adding/removing sysfs files and holding the
> same lock from sysfs read/write callbacks" is a classic deadlock. I've
> made lockdep shut up about it, I have no idea how to fix it properly.
> kernfs switching it's locking doesn't change anything here.
>
> Generally the fix is to untangle the locking: You need 1 set of locks
> to protect the datastructures exposed through sysfs, and another thing
> (maybe that's provided by serio already, I have no idea) to make sure
> you're ordering connect/disconnect handling correct and there's not
> concurrent calls to this hooks. Assuming serio does give that
> guarantee then there's no need to hold the lock while unregistering
> the sysfs files (we could perhaps push the lock dropping down into
> trackpoint_disconnect, but that's less maintainable imo since it's not
> obvious in psmouse_disconnect what's going on), and my patch is
> correct.
>
> But I didn't do a full locking audit of what exactly serio guarantees,
> and what exactly psmouse_mutex needs to protect (and where
> psmouse_mutex is only held because it's convenient).

Ping? Just noticed that I still have this bugfix hanging around.
bugfix as in "make lockdep more useful", not necessarily fixing the
locking here properly. I guess I could respin to add a FIXME comment,
but not sure how useful that would be.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH] [v3] HID: add support for Apple Magic Trackpad 2
From: Benjamin Tissoires @ 2018-10-01  8:43 UTC (permalink / raw)
  To: seobrien, Peter Hutterer
  Cc: marek.wyborski, lkml, Dmitry Torokhov, open list:HID CORE LAYER,
	Henrik Rydberg, Jiri Kosina, claudio
In-Reply-To: <20180920231308.237277-1-seobrien@chromium.org>

[adding Peter, for the libinput question]

On Fri, Sep 21, 2018 at 1:13 AM 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>
> ---
>

a few nitpcks:

>  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;

Peter, can you have a look?

To me, while adding this threshold, you are basically fooling
userspace, and we might want to come back to the raw values at some
point.
If there is a userspace problem, it has to be solved in userspace.

> +               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;
> -                */

Can you keep this comment from the original version? If your touchpad
doesn't have the timestamp bit, the old ones still have them, and
having the decoded information might help in the future.

Cheers,
Benjamin

> +               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
>

^ permalink raw reply

* Re: [PATCH] drivers: hid: Remove dead storage
From: Benjamin Tissoires @ 2018-10-01  8:33 UTC (permalink / raw)
  To: tapaswenipathak; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <5bafa31a.1c69fb81.ff018.dc43@mx.google.com>

On Sat, Sep 29, 2018 at 6:06 PM Tapasweni Pathak
<tapaswenipathak@gmail.com> wrote:
>
> Remove storing of return value of ntrig_version_string function
> to ret as it is a dead storage and not used afterwards.
>
> Found using Facebook's Infer. Build tested it.
>
> Signed-off-by: Tapasweni Pathak <tapaswenipathak@gmail.com>
> ---
> There is another option to instead check for non zero return
> value for ntrig_version_string() as a successful completion
> of the function. ntrig_version_string() returns the length
> of output printed by sprintf() after converting the raw firmware
> code into a 5 comma separated numbers.

If you really want to fix this, I would prefer the alternative above.

Just one nitpick: please make sure ntrig_version_string() returns
either 0 (success) or a negative error.
Then in ntrig_report_version(), if the returned value is non null,
output a hid_warn() instead of leaking a non allocated buffer.

Cheers,
Benjamin

>
>  drivers/hid/hid-ntrig.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
> index 43b1c72..22aebf4 100644
> --- a/drivers/hid/hid-ntrig.c
> +++ b/drivers/hid/hid-ntrig.c
> @@ -159,7 +159,7 @@ static void ntrig_report_version(struct hid_device *hdev)
>                               USB_CTRL_SET_TIMEOUT);
>
>         if (ret == 8) {
> -               ret = ntrig_version_string(&data[2], buf);
> +               ntrig_version_string(&data[2], buf);
>
>                 hid_info(hdev, "Firmware version: %s (%02x%02x %02x%02x)\n",
>                          buf, data[2], data[3], data[4], data[5]);
> --
> 2.7.4

^ permalink raw reply

* Re: [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device
From: Kai Heng Feng @ 2018-10-01  7:13 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Hans de Goede, open list:HID CORE LAYER, lkml
In-Reply-To: <CAO-hwJKhyVjfNsiEK-a16KGPdtfgrtnQ0XgxLyBqUhHVaS8AOA@mail.gmail.com>


> On Oct 1, 2018, at 3:00 PM, Benjamin Tissoires  
> <benjamin.tissoires@redhat.com> wrote:
>
> On Mon, Oct 1, 2018 at 5:53 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>> Raydium touchpanel (2386:4B33) sometimes does not workin desktop session
>> although it works in display manager.
>>
>> During user logging, the display manager exits, close the HID device,
>> then the device gets runtime suspended and powered off. The desktop
>> session begins shortly after, opens the HID device, then the device gets
>> runtime resumed and powered on.
>>
>> If the trasition from display manager to desktop sesesion is fast, the
>> touchpanel cannot switch from powered off to powered on in short
>> timeframe. So add a small delay to workaround the issue.
>
> I think you want something similar to
> https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git/commit/?h=for-next&id=807588ac92018bde88a1958f546438e840eb0158
>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>>  drivers/hid/i2c-hid/i2c-hid.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> index f3076659361a..ff5682cc1bce 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> @@ -409,6 +409,8 @@ static int i2c_hid_set_power(struct i2c_client  
>> *client, int power_state)
>>
>>         if (ret)
>>                 dev_err(&client->dev, "failed to change power setting.\n");
>> +       else
>> +               msleep(20);
>
> We shouldn't have this error in the first place, so adding a timeout
> will just add a band-aid on top of an other issue.
>
> I really think the solution from Anisse would work in your case and
> will be nicer.

The Hantick one requires 2.5 seconds delay, the Raydium one requires much  
less.
Also it doesn't have the input shift issue.

So disabling runtime PM entirely is a little overkill in this case.

Maybe there's a better approach?

Kai-Heng

>
> Cheers,
> Benjamin
>
>>  set_pwr_exit:
>>         return ret;
>> --
>> 2.17.1

^ permalink raw reply

* Re: [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device
From: Benjamin Tissoires @ 2018-10-01  7:00 UTC (permalink / raw)
  To: Kai Heng Feng; +Cc: Jiri Kosina, Hans de Goede, open list:HID CORE LAYER, lkml
In-Reply-To: <20181001035313.21453-1-kai.heng.feng@canonical.com>

On Mon, Oct 1, 2018 at 5:53 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> Raydium touchpanel (2386:4B33) sometimes does not workin desktop session
> although it works in display manager.
>
> During user logging, the display manager exits, close the HID device,
> then the device gets runtime suspended and powered off. The desktop
> session begins shortly after, opens the HID device, then the device gets
> runtime resumed and powered on.
>
> If the trasition from display manager to desktop sesesion is fast, the
> touchpanel cannot switch from powered off to powered on in short
> timeframe. So add a small delay to workaround the issue.

I think you want something similar to
https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git/commit/?h=for-next&id=807588ac92018bde88a1958f546438e840eb0158

>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/hid/i2c-hid/i2c-hid.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index f3076659361a..ff5682cc1bce 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -409,6 +409,8 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
>
>         if (ret)
>                 dev_err(&client->dev, "failed to change power setting.\n");
> +       else
> +               msleep(20);

We shouldn't have this error in the first place, so adding a timeout
will just add a band-aid on top of an other issue.

I really think the solution from Anisse would work in your case and
will be nicer.

Cheers,
Benjamin

>
>  set_pwr_exit:
>         return ret;
> --
> 2.17.1
>

^ permalink raw reply

* [PATCH] HID: i2c-hid: Add a small delay after powering on/off the device
From: Kai-Heng Feng @ 2018-10-01  3:53 UTC (permalink / raw)
  To: jikos
  Cc: benjamin.tissoires, hdegoede, linux-input, linux-kernel,
	Kai-Heng Feng

Raydium touchpanel (2386:4B33) sometimes does not workin desktop session
although it works in display manager.

During user logging, the display manager exits, close the HID device,
then the device gets runtime suspended and powered off. The desktop
session begins shortly after, opens the HID device, then the device gets
runtime resumed and powered on.

If the trasition from display manager to desktop sesesion is fast, the
touchpanel cannot switch from powered off to powered on in short
timeframe. So add a small delay to workaround the issue.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/hid/i2c-hid/i2c-hid.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index f3076659361a..ff5682cc1bce 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -409,6 +409,8 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
 
 	if (ret)
 		dev_err(&client->dev, "failed to change power setting.\n");
+	else
+		msleep(20);
 
 set_pwr_exit:
 	return ret;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] [v3] HID: add support for Apple Magic Trackpad 2
From: Henrik Rydberg @ 2018-09-30 11:34 UTC (permalink / raw)
  To: Sean O'Brien, Benjamin Tissoires
  Cc: marek.wyborski, linux-kernel, dmitry.torokhov, linux-input,
	Jiri Kosina, claudio
In-Reply-To: <CAOOzhkosyZpr+B1xwZcG3viNbWZFWr9WQtgGdP13i_+_Ocdgmg@mail.gmail.com>

Hi Sean,

> 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(-)

This version looks good.

Reviewed-by: Henrik Rydberg <rydberg@bitmath.org>

Thank you!

Henrik

^ permalink raw reply

* Re: [PATCH V2] hid: hid-core: Fix a sleep-in-atomic-context bug in __hid_request()
From: Jiri Kosina @ 2018-09-29 19:20 UTC (permalink / raw)
  To: Jia-Ju Bai; +Cc: benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <4c0096c6-3745-963c-5086-5f579792599b@gmail.com>

On Sat, 29 Sep 2018, Jia-Ju Bai wrote:

> >> picolcd_send_and_wait (acquire a spinlock)
> >>    hid_hw_request
> >>      __hid_request
> >>        hid_alloc_report_buf(GFP_KERNEL)
> >>
> >> picolcd_reset (acquire a spinlock)
> >>    hid_hw_request
> >>      __hid_request
> >>        hid_alloc_report_buf(GFP_KERNEL)
> >>
> >> lg4ff_play (acquire a spinlock)
> >>    hid_hw_request
> >>      __hid_request
> >>        hid_alloc_report_buf(GFP_KERNEL)
> >>
> >> lg4ff_set_autocenter_ffex (acquire a spinlock)
> >>    hid_hw_request
> >>      __hid_request
> >>        hid_alloc_report_buf(GFP_KERNEL)
> > Hm, so it's always drivers calling out into core in atomic context. So
> > either we take this, and put our bets on being able to allocate the buffer
> > without sleeping,
> 
> In my opinion, I prefer this way.

Why? Forcing all the report buffer to be limited to be non-sleeping 
allocations just because of two drivers, looks like an overkill, and 
actually calls for more issues (as GFP_ATOMIC is of course in principle 
less likely to succeed).

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* [PATCH] drivers: hid: Remove dead storage
From: Tapasweni Pathak @ 2018-09-29 16:06 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, linux-input, linux-kernel

Remove storing of return value of ntrig_version_string function
to ret as it is a dead storage and not used afterwards.

Found using Facebook's Infer. Build tested it.

Signed-off-by: Tapasweni Pathak <tapaswenipathak@gmail.com>
---
There is another option to instead check for non zero return
value for ntrig_version_string() as a successful completion
of the function. ntrig_version_string() returns the length
of output printed by sprintf() after converting the raw firmware
code into a 5 comma separated numbers.

 drivers/hid/hid-ntrig.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 43b1c72..22aebf4 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -159,7 +159,7 @@ static void ntrig_report_version(struct hid_device *hdev)
 			      USB_CTRL_SET_TIMEOUT);

 	if (ret == 8) {
-		ret = ntrig_version_string(&data[2], buf);
+		ntrig_version_string(&data[2], buf);

 		hid_info(hdev, "Firmware version: %s (%02x%02x %02x%02x)\n",
 			 buf, data[2], data[3], data[4], data[5]);
--
2.7.4

^ permalink raw reply related

* Re: [PATCH V2] hid: hid-core: Fix a sleep-in-atomic-context bug in __hid_request()
From: Jia-Ju Bai @ 2018-09-29  9:00 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <nycvar.YFH.7.76.1809241124480.15880@cbobk.fhfr.pm>



On 2018/9/24 17:26, Jiri Kosina wrote:
> On Thu, 13 Sep 2018, Jia-Ju Bai wrote:
>
>> hid_alloc_report_buf() has to be called with GFP_ATOMIC in
>> __hid_request(), because there are the following callchains
>> leading to __hid_request() being an atomic context:
>>
>> picolcd_send_and_wait (acquire a spinlock)
>>    hid_hw_request
>>      __hid_request
>>        hid_alloc_report_buf(GFP_KERNEL)
>>
>> picolcd_reset (acquire a spinlock)
>>    hid_hw_request
>>      __hid_request
>>        hid_alloc_report_buf(GFP_KERNEL)
>>
>> lg4ff_play (acquire a spinlock)
>>    hid_hw_request
>>      __hid_request
>>        hid_alloc_report_buf(GFP_KERNEL)
>>
>> lg4ff_set_autocenter_ffex (acquire a spinlock)
>>    hid_hw_request
>>      __hid_request
>>        hid_alloc_report_buf(GFP_KERNEL)
> Hm, so it's always drivers calling out into core in atomic context. So
> either we take this, and put our bets on being able to allocate the buffer
> without sleeping,

In my opinion, I prefer this way.


Best wishes,
Jia-Ju Bai

> or actually fix the few drivers (it's just lg4ff and
> picolcd at the end of the day) not to do that, and explicitly anotate
> __hid_request() with might_sleep().
>
> Hmm?
>
> Thanks,
>

^ permalink raw reply

* Re: [git pull] Input updates for v4.19-rc5
From: Greg Kroah-Hartman @ 2018-09-29  1:21 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-kernel, linux-input, Linus Torvalds
In-Reply-To: <20180928235751.GA182564@dtor-ws>

On Fri, Sep 28, 2018 at 04:57:51PM -0700, Dmitry Torokhov wrote:
> Hi Greg,
> 
> Please pull from:
> 
> 	git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
> 
> to receive updates for the input subsystem. Just a few driver fixes.

Now pulled, thanks.

greg k-h

^ permalink raw reply

* Re: [PATCH] Input: Support for Xbox1 PDP Camo series gamepad
From: Dmitry Torokhov @ 2018-09-28 23:59 UTC (permalink / raw)
  To: Ramses Ramírez
  Cc: Cameron Gutman, Mark Furneaux, Leo Sperling, Enno Boland,
	Marcus Folkesson, Francis Therien, Julia Lawall, linux-input,
	linux-kernel
In-Reply-To: <20180828054812.14428-1-ramzeto@gmail.com>

On Tue, Aug 28, 2018 at 12:48:11AM -0500, Ramses Ramírez wrote:
> The "Xbox One PDP Wired Controller - Camo series" has a different
> product-id than the regular PDP controller and the PDP stealth series,
> but it uses the same initialization sequence. This patch adds the
> product-id of the camo series to the structures that handle the other
> PDP Xbox One controllers.
> 
> Signed-off-by: Ramses Ramírez <ramzeto@gmail.com>

Applied, thank you.

> ---
>  drivers/input/joystick/xpad.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index cd620e009bad..d4b9db487b16 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -231,6 +231,7 @@ static const struct xpad_device {
>  	{ 0x0e6f, 0x0246, "Rock Candy Gamepad for Xbox One 2015", 0, XTYPE_XBOXONE },
>  	{ 0x0e6f, 0x02ab, "PDP Controller for Xbox One", 0, XTYPE_XBOXONE },
>  	{ 0x0e6f, 0x02a4, "PDP Wired Controller for Xbox One - Stealth Series", 0, XTYPE_XBOXONE },
> +	{ 0x0e6f, 0x02a6, "PDP Wired Controller for Xbox One - Camo Series", 0, XTYPE_XBOXONE },
>  	{ 0x0e6f, 0x0301, "Logic3 Controller", 0, XTYPE_XBOX360 },
>  	{ 0x0e6f, 0x0346, "Rock Candy Gamepad for Xbox One 2016", 0, XTYPE_XBOXONE },
>  	{ 0x0e6f, 0x0401, "Logic3 Controller", 0, XTYPE_XBOX360 },
> @@ -530,6 +531,8 @@ static const struct xboxone_init_packet xboxone_init_packets[] = {
>  	XBOXONE_INIT_PKT(0x0e6f, 0x02ab, xboxone_pdp_init2),
>  	XBOXONE_INIT_PKT(0x0e6f, 0x02a4, xboxone_pdp_init1),
>  	XBOXONE_INIT_PKT(0x0e6f, 0x02a4, xboxone_pdp_init2),
> +	XBOXONE_INIT_PKT(0x0e6f, 0x02a6, xboxone_pdp_init1),
> +	XBOXONE_INIT_PKT(0x0e6f, 0x02a6, xboxone_pdp_init2),
>  	XBOXONE_INIT_PKT(0x24c6, 0x541a, xboxone_rumblebegin_init),
>  	XBOXONE_INIT_PKT(0x24c6, 0x542a, xboxone_rumblebegin_init),
>  	XBOXONE_INIT_PKT(0x24c6, 0x543a, xboxone_rumblebegin_init),
> -- 
> 2.17.1
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] input: sun4i-lradc: Convert to using %pOFn instead of device_node.name
From: Dmitry Torokhov @ 2018-09-28 23:59 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-kernel, Hans de Goede, Maxime Ripard, linux-input
In-Reply-To: <20180828015252.28511-23-robh@kernel.org>

On Mon, Aug 27, 2018 at 08:52:24PM -0500, Rob Herring wrote:
> In preparation to remove the node name pointer from struct device_node,
> convert printf users to use the %pOFn format specifier.
> 
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>

Applied, thank you.

> ---
>  drivers/input/keyboard/sun4i-lradc-keys.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/keyboard/sun4i-lradc-keys.c b/drivers/input/keyboard/sun4i-lradc-keys.c
> index a37c172452e6..57272df34cd5 100644
> --- a/drivers/input/keyboard/sun4i-lradc-keys.c
> +++ b/drivers/input/keyboard/sun4i-lradc-keys.c
> @@ -185,19 +185,19 @@ static int sun4i_lradc_load_dt_keymap(struct device *dev,
>  
>  		error = of_property_read_u32(pp, "channel", &channel);
>  		if (error || channel != 0) {
> -			dev_err(dev, "%s: Inval channel prop\n", pp->name);
> +			dev_err(dev, "%pOFn: Inval channel prop\n", pp);
>  			return -EINVAL;
>  		}
>  
>  		error = of_property_read_u32(pp, "voltage", &map->voltage);
>  		if (error) {
> -			dev_err(dev, "%s: Inval voltage prop\n", pp->name);
> +			dev_err(dev, "%pOFn: Inval voltage prop\n", pp);
>  			return -EINVAL;
>  		}
>  
>  		error = of_property_read_u32(pp, "linux,code", &map->keycode);
>  		if (error) {
> -			dev_err(dev, "%s: Inval linux,code prop\n", pp->name);
> +			dev_err(dev, "%pOFn: Inval linux,code prop\n", pp);
>  			return -EINVAL;
>  		}
>  
> -- 
> 2.17.1
> 

-- 
Dmitry

^ permalink raw reply

* [git pull] Input updates for v4.19-rc5
From: Dmitry Torokhov @ 2018-09-28 23:57 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, linux-input, Linus Torvalds

Hi Greg,

Please pull from:

	git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus

to receive updates for the input subsystem. Just a few driver fixes.

Changelog:
---------

Aaron Ma (1):
      Input: elantech - enable middle button of touchpad on ThinkPad P72

Andreas Schwab (1):
      Input: atakbd - fix Atari keymap

Anson Huang (1):
      Input: egalax_ts - add system wakeup support

Michael Schmitz (1):
      Input: atakbd - fix Atari CapsLock behaviour

Peter Hutterer (1):
      Input: uinput - allow for max == min during input_absinfo validation

Song Qiang (1):
      Input: gpio-keys - fix a documentation index issue

Diffstat:
--------

 .../devicetree/bindings/input/gpio-keys.txt        |  2 +-
 drivers/input/keyboard/atakbd.c                    | 74 ++++++++--------------
 drivers/input/misc/uinput.c                        |  2 +-
 drivers/input/mouse/elantech.c                     |  2 +
 drivers/input/touchscreen/egalax_ts.c              |  6 ++
 5 files changed, 38 insertions(+), 48 deletions(-)

Thanks.


-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] input: sun4i-lradc: Convert to using %pOFn instead of device_node.name
From: Rob Herring @ 2018-09-28 21:54 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org, Dmitry Torokhov
  Cc: Hans de Goede, Maxime Ripard, linux-input
In-Reply-To: <20180828015252.28511-23-robh@kernel.org>

On Mon, Aug 27, 2018 at 8:54 PM Rob Herring <robh@kernel.org> wrote:
>
> In preparation to remove the node name pointer from struct device_node,
> convert printf users to use the %pOFn format specifier.
>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/input/keyboard/sun4i-lradc-keys.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Ping.

>
> diff --git a/drivers/input/keyboard/sun4i-lradc-keys.c b/drivers/input/keyboard/sun4i-lradc-keys.c
> index a37c172452e6..57272df34cd5 100644
> --- a/drivers/input/keyboard/sun4i-lradc-keys.c
> +++ b/drivers/input/keyboard/sun4i-lradc-keys.c
> @@ -185,19 +185,19 @@ static int sun4i_lradc_load_dt_keymap(struct device *dev,
>
>                 error = of_property_read_u32(pp, "channel", &channel);
>                 if (error || channel != 0) {
> -                       dev_err(dev, "%s: Inval channel prop\n", pp->name);
> +                       dev_err(dev, "%pOFn: Inval channel prop\n", pp);
>                         return -EINVAL;
>                 }
>
>                 error = of_property_read_u32(pp, "voltage", &map->voltage);
>                 if (error) {
> -                       dev_err(dev, "%s: Inval voltage prop\n", pp->name);
> +                       dev_err(dev, "%pOFn: Inval voltage prop\n", pp);
>                         return -EINVAL;
>                 }
>
>                 error = of_property_read_u32(pp, "linux,code", &map->keycode);
>                 if (error) {
> -                       dev_err(dev, "%s: Inval linux,code prop\n", pp->name);
> +                       dev_err(dev, "%pOFn: Inval linux,code prop\n", pp);
>                         return -EINVAL;
>                 }
>
> --
> 2.17.1
>

^ permalink raw reply

* Bad MAINTAINERS pattern in section 'D-LINK DIR-685 TOUCHKEYS DRIVER'
From: Joe Perches @ 2018-09-28 21:52 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Walleij, linux-input

Please fix this defect appropriately.

linux-next MAINTAINERS section:

	4010	D-LINK DIR-685 TOUCHKEYS DRIVER
	4011	M:	Linus Walleij <linus.walleij@linaro.org>
	4012	L:	linux-input@vger.kernel.org
	4013	S:	Supported
-->	4014	F:	drivers/input/dlink-dir685-touchkeys.c

Commit that introduced this:

commit 131b3de7016b73fca1aba8ffb528217ac95b2505
 Author: Linus Walleij <linus.walleij@linaro.org>
 Date:   Mon Jun 5 14:48:44 2017 -0700
 
     Input: add D-Link DIR-685 touchkeys driver
     
     This adds support for the D-Link DIR-685 touchkeys found in the
     router with this name.
     
     The vendor code calles this a "touchpad" but we are registering
     it here under its real name.
     
     Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
     Acked-by: Rob Herring <robh@kernel.org>
     Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
 
  .../bindings/input/dlink,dir685-touchkeys.txt      |  21 +++
  MAINTAINERS                                        |   6 +
  drivers/input/keyboard/Kconfig                     |  11 ++
  drivers/input/keyboard/Makefile                    |   1 +
  drivers/input/keyboard/dlink-dir685-touchkeys.c    | 155 +++++++++++++++++++++
  5 files changed, 194 insertions(+)

No commit with drivers/input/dlink-dir685-touchkeys.c found

^ permalink raw reply

* Re: [PATCH 0/5] ti_am335x_tsc: Enable wakeup capability
From: Vignesh R @ 2018-09-28  6:12 UTC (permalink / raw)
  To: Lee Jones
  Cc: Dmitry Torokhov, Jonathan Cameron, linux-iio, linux-omap,
	linux-kernel, linux-input
In-Reply-To: <20180725052621.GB5835@dell>

Hi Lee,

On Wednesday 25 July 2018 10:56 AM, Lee Jones wrote:
[...]
>>>>>>>
>>>>>>> Vignesh R (5):
>>>>>>>   mfd: ti_am335x_tscadc: Don't mark TSCADC MFD as wakeup capable
>>>>>>>   Input: ti_am335x_tsc: Mark TSC device as wakeup source
>>>>>>>   mfd: ti_am335x_tscadc: Keep ADC interface on if child is wakeup
>>>>>>>     capable
>>>>>>>   iio: adc: ti_am335x_adc: Disable ADC during suspend unconditionally
>>>>>>>   Input: ti_am335x_tsc: Mark IRQ as wakeup capable
>>>>>>>
>>>>>>>  drivers/iio/adc/ti_am335x_adc.c           | 12 ++++--------
>>>>>>>  drivers/input/touchscreen/ti_am335x_tsc.c | 22 +++++++++++++++++-----
>>>>>>>  drivers/mfd/ti_am335x_tscadc.c            | 14 +++++++++++++-
>>>>>>>  3 files changed, 34 insertions(+), 14 deletions(-)
>>>>>>>
>>>>>>
>>>>>> Gentle ping... Could you review/pick this series? MFD amd IIO bits are
>>>>>> already ACKed
>>>>>
>>>>> MFD patches are reviewed "for my own reference" meaning that we
>>>>> haven't yet agreed on a merge plan yet.
>>>>
>>>> I think this series makes sense to be pushed through a single tree as
>>>> opposed to being spread between 3, even if it could technically be
>>>> possible. It looks like Jonathan is fine with going it through either
>>>> his or some other tree, I am fine with it going through MFD.
>>>
>>> I'm happy either way.
>>>
>> Thanks Dmitry, Jonathan and Lee Jones!
>>
>> Could this be applied to one of the trees now? MFD perhaps?
> 
> It'll be applied, when it's applied. ;)

I see that this series was not picked up for v4.19. Could you consider
this series for v4.20? Patches apply cleanly against linux-next.


-- 
Regards
Vignesh

^ permalink raw reply

* Re: [PATCH] [v3] HID: add support for Apple Magic Trackpad 2
From: Sean O'Brien @ 2018-09-27 16:45 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: marek.wyborski, linux-kernel, dmitry.torokhov, linux-input,
	rydberg, Jiri Kosina, claudio
In-Reply-To: <20180920231308.237277-1-seobrien@chromium.org>

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
>

^ permalink raw reply

* Re: WARNING: kmalloc bug in input_mt_init_slots
From: Christopher Lameter @ 2018-09-27 15:47 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Dmitry Torokhov, syzbot+87829a10073277282ad1, Pekka Enberg,
	linux-input@vger.kernel.org, lkml, Henrik Rydberg, syzkaller-bugs,
	Linux-MM
In-Reply-To: <CACT4Y+aDFD6O48nV5J9UpXLiNpTPuSFoBQ4HVC+Kg1tM_KFEiQ@mail.gmail.com>

On Thu, 27 Sep 2018, Dmitry Vyukov wrote:

> > Please post on the mailing list
>
> It is on the  mailing lists:
> https://lkml.org/lkml/2018/9/27/802


Ok then lets continue the discussion there.

^ permalink raw reply

* Re: WARNING: kmalloc bug in input_mt_init_slots
From: Dmitry Vyukov @ 2018-09-27 15:29 UTC (permalink / raw)
  To: Christopher Lameter
  Cc: Dmitry Torokhov, syzbot+87829a10073277282ad1, Pekka Enberg,
	linux-input@vger.kernel.org, lkml, Henrik Rydberg, syzkaller-bugs,
	Linux-MM
In-Reply-To: <010001661b9fad1d-cdbfabdb-5553-446f-bcde-585e42837415-000000@email.amazonses.com>

On Thu, Sep 27, 2018 at 5:22 PM, Christopher Lameter <cl@linux.com> wrote:
> On Thu, 27 Sep 2018, Dmitry Vyukov wrote:
>
>> On Thu, Sep 27, 2018 at 4:16 PM, Christopher Lameter <cl@linux.com> wrote:
>> > On Thu, 27 Sep 2018, Dmitry Vyukov wrote:
>> >
>> >> On Tue, Sep 25, 2018 at 4:04 PM, Christopher Lameter <cl@linux.com> wrote:
>> >> > On Tue, 25 Sep 2018, Dmitry Vyukov wrote:
>> >> >
>> >> >> Assuming that the size is large enough to fail in all allocators, is
>> >> >> this warning still useful? How? Should we remove it?
>> >> >
>> >> > Remove it. It does not make sense because we check earlier if possible
>> >> > without the warn.
>> >>
>> >> Mailed "mm: don't warn about large allocations for slab" to remove the warning.
>> >>
>> >
>> > Hoe it arrives here at some point.
>>
>> It's here:
>> https://lore.kernel.org/patchwork/patch/992660/
>>
>
> Please post on the mailing list

It is on the  mailing lists:
https://lkml.org/lkml/2018/9/27/802

> and NAK to the patch. Testing against
> KMALLOC_MAX_CACHE_SIZE is not ok.

Why?

^ permalink raw reply

* Re: WARNING: kmalloc bug in input_mt_init_slots
From: Christopher Lameter @ 2018-09-27 15:22 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Dmitry Torokhov, syzbot+87829a10073277282ad1, Pekka Enberg,
	linux-input@vger.kernel.org, lkml, Henrik Rydberg, syzkaller-bugs,
	Linux-MM
In-Reply-To: <CACT4Y+biYtFUV7hK2ne2RfrbZjMt=4FK4deE0B6WykwT2qSt2g@mail.gmail.com>

On Thu, 27 Sep 2018, Dmitry Vyukov wrote:

> On Thu, Sep 27, 2018 at 4:16 PM, Christopher Lameter <cl@linux.com> wrote:
> > On Thu, 27 Sep 2018, Dmitry Vyukov wrote:
> >
> >> On Tue, Sep 25, 2018 at 4:04 PM, Christopher Lameter <cl@linux.com> wrote:
> >> > On Tue, 25 Sep 2018, Dmitry Vyukov wrote:
> >> >
> >> >> Assuming that the size is large enough to fail in all allocators, is
> >> >> this warning still useful? How? Should we remove it?
> >> >
> >> > Remove it. It does not make sense because we check earlier if possible
> >> > without the warn.
> >>
> >> Mailed "mm: don't warn about large allocations for slab" to remove the warning.
> >>
> >
> > Hoe it arrives here at some point.
>
> It's here:
> https://lore.kernel.org/patchwork/patch/992660/
>

Please post on the mailing list and NAK to the patch. Testing against
KMALLOC_MAX_CACHE_SIZE is not ok.

^ permalink raw reply

* Re: WARNING: kmalloc bug in input_mt_init_slots
From: Matthew Wilcox @ 2018-09-27 14:35 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Christopher Lameter, Dmitry Vyukov, syzbot+87829a10073277282ad1,
	Pekka Enberg, linux-input@vger.kernel.org, lkml, Henrik Rydberg,
	syzkaller-bugs, Linux-MM
In-Reply-To: <20180924184158.GA156847@dtor-ws>

On Mon, Sep 24, 2018 at 11:41:58AM -0700, Dmitry Torokhov wrote:
> > How large is the allocation? AFACIT nRequests larger than KMALLOC_MAX_SIZE
> > are larger than the maximum allowed by the page allocator. Thus the warning
> > and the NULL return.
> 
> The size in this particular case is being derived from a value passed
> from userspace. Input core does not care about any limits on size of
> memory kmalloc() can support and is perfectly happy with getting NULL
> and telling userspace to go away with their silly requests by returning
> -ENOMEM.
> 
> For the record: I definitely do not want to pre-sanitize size neither in
> uinput nor in input core.

Probably should be using kvzalloc then.

^ permalink raw reply

* Re: WARNING: kmalloc bug in input_mt_init_slots
From: Dmitry Vyukov @ 2018-09-27 14:28 UTC (permalink / raw)
  To: Christopher Lameter
  Cc: Dmitry Torokhov, syzbot+87829a10073277282ad1, Pekka Enberg,
	linux-input@vger.kernel.org, lkml, Henrik Rydberg, syzkaller-bugs,
	Linux-MM
In-Reply-To: <010001661b631a3e-f398fc0a-127c-4c6e-b6ca-b2bd63bc4a9a-000000@email.amazonses.com>

On Thu, Sep 27, 2018 at 4:16 PM, Christopher Lameter <cl@linux.com> wrote:
> On Thu, 27 Sep 2018, Dmitry Vyukov wrote:
>
>> On Tue, Sep 25, 2018 at 4:04 PM, Christopher Lameter <cl@linux.com> wrote:
>> > On Tue, 25 Sep 2018, Dmitry Vyukov wrote:
>> >
>> >> Assuming that the size is large enough to fail in all allocators, is
>> >> this warning still useful? How? Should we remove it?
>> >
>> > Remove it. It does not make sense because we check earlier if possible
>> > without the warn.
>>
>> Mailed "mm: don't warn about large allocations for slab" to remove the warning.
>>
>
> Hoe it arrives here at some point.

It's here:
https://lore.kernel.org/patchwork/patch/992660/

^ permalink raw reply

* Re: WARNING: kmalloc bug in input_mt_init_slots
From: Christopher Lameter @ 2018-09-27 14:16 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Dmitry Torokhov, syzbot+87829a10073277282ad1, Pekka Enberg,
	linux-input@vger.kernel.org, lkml, Henrik Rydberg, syzkaller-bugs,
	Linux-MM
In-Reply-To: <CACT4Y+aUdAmRmgiV5-KWXF-eGoCUCMhUC+ddLU-heQTQ53PhRA@mail.gmail.com>

On Thu, 27 Sep 2018, Dmitry Vyukov wrote:

> On Tue, Sep 25, 2018 at 4:04 PM, Christopher Lameter <cl@linux.com> wrote:
> > On Tue, 25 Sep 2018, Dmitry Vyukov wrote:
> >
> >> Assuming that the size is large enough to fail in all allocators, is
> >> this warning still useful? How? Should we remove it?
> >
> > Remove it. It does not make sense because we check earlier if possible
> > without the warn.
>
> Mailed "mm: don't warn about large allocations for slab" to remove the warning.
>

Hoe it arrives here at some point.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox