Re: [PATCH] Add HID's to hid-microsoft driver of Surface Type/Touch Cover 2 to fix bug

Hi Reyad,

On Tue, Jan 21, 2014 at 2:33 AM, Reyad Attiyat [off-list ref] wrote:

On Tue, Jan 21, 2014 at 12:47 AM, Jiri Kosina [off-list ref] wrote:

quoted

On Mon, 20 Jan 2014, Reyad Attiyat wrote:

quoted

The below patch fixes a bug 64811
(https://bugzilla.kernel.org/show_bug.cgi?id=64811) of the Microsoft
Surface Type/Touch cover 2 devices being detected as a multitouch
device.
The fix adds the HID of the two devices to hid-microsoft driver. This
ensures that hid-input will eventually be used for the device and not
hid-multitouch.

Hi,

your patch is missing hid_have_special_driver[] entry, therefore correct
driver binding is not guaranteed.

--
Jiri Kosina
SUSE Labs

Hi,

Thanks for reminding me of hid_have_special_driver[]. I noticed that
this device has the HID_DG_CONTACTID and in the comment of the
hid_have_sepcial_driver[]

* Please note that for multitouch devices (driven by hid-multitouch driver),
* there is a proper autodetection and autoloading in place (based on presence
* of HID_DG_CONTACTID), so those devices don't need to be added to this list,
* as we are doing the right thing in hid_scan_usage().

This device should not be driven by hid-multitouch as it does not
handle keyboard/mouse input devices.
I submitted a new patch below with it added. I believe it should still
be part of this array, in case this kind of implementation is
fixed/updated.

This implementation is perfectly fine (I am referring to the "fixed/updated"):
- if your device should be driven by hid-multitouch, then you _don't_
add it to hid_have_special_driver
- if your device should not be driven by hid-multitouch, then you
_need_ to add it to hid_have_special_driver.

Adding the device to hid_have_special_driver prevents the detection of
the group HID_GRP_MULTITOUCH, so you will not end with a race between
hid-multitouch and your special hid driver.

From 291742873dcf181faf9657b41279487f31302c73 Mon Sep 17 00:00:00 2001
From: Reyad Attiyat <redacted>
Date: Tue, 21 Jan 2014 01:22:25 -0600
Subject: [PATCH 1/1] Added in HID's for Microsoft Surface Type/Touch cover 2.
 This is to fix bug 64811 where this device is detected as a multitouch device

You are missing a commit message here (the first message you sent
would fit perfectly here).

Other than that, I played a little with the report descriptor pointed
in the bugzilla.

I think I will be able to handle this touch cover in hid-multitouch,
but that would require more testings/debugging. Microsoft seems to
have implemented an indirect (dual) touchpad here, but until we know
which mode we should put it into, it's going to be tricky to set it up
correctly.

One last thing, in the bugzilla, in the comment 2 you say: "I still
have issues with the type cover 2 even with this fix". Are you still
experiencing those disconnection? If so, maybe we should switch to
hid-multitouch at some point.


So to sum up, with the commit message amended:
Reviewed-by: Benjamin Tissoires <redacted>

Cheers,
Benjamin