2015-12-14 13:01:49

by Akihiko Odaki

[permalink] [raw]
Subject: [PATCH 1/2] HID: Use multitouch driver for Type Covers

Use multitouch driver instead of microsoft one for Microsoft Surface
Type Covers.

By using MT_CLS_EXPORT_ALL_INPUTS, the keyboards function as well as
the multitouch pads do.

Signed-off-by: Akihiko Odaki <[email protected]>
---
drivers/hid/hid-core.c | 14 --------------
drivers/hid/hid-microsoft.c | 10 ----------
drivers/hid/hid-multitouch.c | 17 +++++++++++++++++
3 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index c6f7a69..43afc55 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -723,15 +723,6 @@ static void hid_scan_collection(struct hid_parser *parser, unsigned type)
type == HID_COLLECTION_PHYSICAL)
hid->group = HID_GROUP_SENSOR_HUB;

- if (hid->vendor == USB_VENDOR_ID_MICROSOFT &&
- (hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3 ||
- hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2 ||
- hid->product == USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP ||
- hid->product == USB_DEVICE_ID_MS_TYPE_COVER_3 ||
- hid->product == USB_DEVICE_ID_MS_POWER_COVER) &&
- hid->group == HID_GROUP_MULTITOUCH)
- hid->group = HID_GROUP_GENERIC;
-
if ((parser->global.usage_page << 16) == HID_UP_GENDESK)
for (i = 0; i < parser->local.usage_index; i++)
if (parser->local.usage[i] == HID_GD_POINTER)
@@ -1933,11 +1924,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_WIRELESS_OPTICAL_DESKTOP_3_0) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_OFFICE_KB) },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3) },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3) },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index 77a2cf3..0dbc2a0 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -276,16 +276,6 @@ static const struct hid_device_id ms_devices[] = {
.driver_data = MS_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_COMFORT_MOUSE_4500),
.driver_data = MS_DUPLICATE_USAGES },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3),
- .driver_data = MS_HIDINPUT },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2),
- .driver_data = MS_HIDINPUT },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP),
- .driver_data = MS_HIDINPUT },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3),
- .driver_data = MS_HIDINPUT },
- { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER),
- .driver_data = MS_HIDINPUT },

{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_PRESENTER_8K_BT),
.driver_data = MS_PRESENTER },
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3d664d0..dfc2698 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1330,6 +1330,23 @@ static const struct hid_device_id mt_devices[] = {
MT_USB_DEVICE(USB_VENDOR_ID_ILITEK,
USB_DEVICE_ID_ILITEK_MULTITOUCH) },

+ /* Microsoft Surface Typecover */
+ { .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+ MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+ USB_DEVICE_ID_MS_TYPE_COVER_PRO_3) },
+ { .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+ MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+ USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2) },
+ { .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+ MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+ USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
+ { .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+ MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+ USB_DEVICE_ID_MS_TYPE_COVER_3) },
+ { .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+ MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+ USB_DEVICE_ID_MS_POWER_COVER) },
+
/* MosArt panels */
{ .driver_data = MT_CLS_CONFIDENCE_MINUS_ONE,
MT_USB_DEVICE(USB_VENDOR_ID_ASUS,
--
2.6.4


2015-12-14 12:58:47

by Akihiko Odaki

[permalink] [raw]
Subject: [PATCH 2/2] HID: Add new Japanese Surface 3 Pro Type Cover

Add support for the new Japanese version of Microsoft Surface 3 Pro
Type Cover.

Signed-off-by: Akihiko Odaki <[email protected]>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-multitouch.c | 3 +++
drivers/hid/usbhid/hid-quirks.c | 1 +
3 files changed, 5 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 8b78a7f..a12e392 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -683,6 +683,7 @@
#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_3 0x07dc
#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2 0x07e2
#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP 0x07dd
+#define USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2_JP 0x07e3
#define USB_DEVICE_ID_MS_TYPE_COVER_3 0x07de
#define USB_DEVICE_ID_MS_POWER_COVER 0x07da

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index dfc2698..b5e040d 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1342,6 +1342,9 @@ static const struct hid_device_id mt_devices[] = {
USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP) },
{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
+ USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2_JP) },
+ { .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+ MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
USB_DEVICE_ID_MS_TYPE_COVER_3) },
{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
MT_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 7dd0953..50a57d0 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -91,6 +91,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_JP, HID_QUIRK_NO_INIT_REPORTS },
+ { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_PRO_3_2_JP, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_3, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER, HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
--
2.6.4

2015-12-14 13:22:55

by Bastien Nocera

[permalink] [raw]
Subject: Re: [PATCH 1/2] HID: Use multitouch driver for Type Covers

On Mon, 2015-12-14 at 21:50 +0900, Akihiko Odaki wrote:
> Use multitouch driver instead of microsoft one for Microsoft Surface
> Type Covers.
>
> By using MT_CLS_EXPORT_ALL_INPUTS, the keyboards function as well as
> the multitouch pads do.

I've discussed this a couple of weeks back with Benjamin Tissoires, and
this patch would break the special keys (mute, brightness up/down,
keyboard backlight up/down and play/pause).

The recommended way to fix this was to move multi-touch processing into
the Microsoft driver, so that it would handle the trackpad's multi-
touch events.

You should be able to do this by carefully picking up the handling code
from hid-multitouch, or do something similar to what's done in hid-
wacom, which has the same problem as the Type Cover handling.

Can you confirm that this does indeed break those special keys? If it
does, it's a NAK from my side.

Cheers

2015-12-14 16:39:47

by Donavan Lance

[permalink] [raw]
Subject: Re: [PATCH 1/2] HID: Use multitouch driver for Type Covers

On Mon, Dec 14, 2015 at 8:22 AM, Bastien Nocera <[email protected]> wrote:
> On Mon, 2015-12-14 at 21:50 +0900, Akihiko Odaki wrote:
>> Use multitouch driver instead of microsoft one for Microsoft Surface
>> Type Covers.
>>
>> By using MT_CLS_EXPORT_ALL_INPUTS, the keyboards function as well as
>> the multitouch pads do.
>
> I've discussed this a couple of weeks back with Benjamin Tissoires, and
> this patch would break the special keys (mute, brightness up/down,
> keyboard backlight up/down and play/pause).
>
> The recommended way to fix this was to move multi-touch processing into
> the Microsoft driver, so that it would handle the trackpad's multi-
> touch events.
>
> You should be able to do this by carefully picking up the handling code
> from hid-multitouch, or do something similar to what's done in hid-
> wacom, which has the same problem as the Type Cover handling.
>
> Can you confirm that this does indeed break those special keys? If it
> does, it's a NAK from my side.

For what it's worth the special keys on my keyboard work fine when
using this patch. I'm using a Surface Pro 3, Type Cover 3, running
GNOME and Fedora 23. The search, share, connect(?), and settings keys
are the only ones not mapped to anything out of the box, but they are
recognized by xev.

2015-12-18 06:28:34

by Akihiko Odaki

[permalink] [raw]
Subject: Re: [PATCH 1/2] HID: Use multitouch driver for Type Covers

> The search, share, connect(?), and settings keys
I tested the patch again with xev and found that those "charm" keys
don't respond both on hid-microsoft and hid-multitouch, while other
keys respond. I'll have a further look.

Anyway, keys working with hid-microsoft also work with hid-multitouch,
so It's ready for merging, I think.

On 12/15/2015 01:39 AM, Donavan Lance wrote:
> On Mon, Dec 14, 2015 at 8:22 AM, Bastien Nocera
> <[email protected]>
wrote:
>> On Mon, 2015-12-14 at 21:50 +0900, Akihiko Odaki wrote:
>>> Use multitouch driver instead of microsoft one for Microsoft
>>> Surface Type Covers.
>>>
>>> By using MT_CLS_EXPORT_ALL_INPUTS, the keyboards function as
>>> well as the multitouch pads do.
>>
>> I've discussed this a couple of weeks back with Benjamin
>> Tissoires, and this patch would break the special keys (mute,
>> brightness up/down, keyboard backlight up/down and play/pause).
>>
>> The recommended way to fix this was to move multi-touch
>> processing into the Microsoft driver, so that it would handle the
>> trackpad's multi- touch events.
>>
>> You should be able to do this by carefully picking up the
>> handling code from hid-multitouch, or do something similar to
>> what's done in hid- wacom, which has the same problem as the Type
>> Cover handling.
>>
>> Can you confirm that this does indeed break those special keys?
>> If it does, it's a NAK from my side.
>
> For what it's worth the special keys on my keyboard work fine when
> using this patch. I'm using a Surface Pro 3, Type Cover 3, running
> GNOME and Fedora 23. The search, share, connect(?), and settings
> keys are the only ones not mapped to anything out of the box, but
> they are recognized by xev.
>

2015-12-18 09:14:56

by Akihiko Odaki

[permalink] [raw]
Subject: Re: [PATCH 1/2] HID: Use multitouch driver for Type Covers

The "Charm" keys seem to issue short-cut key combinations.
I also found Microsoft's document by searching for the combinations.

https://msdn.microsoft.com/en-us/library/windows/hardware/dn614461(v=vs.85).aspx

So the behavior of hid-multitouch is completely fine.

On 12/18/2015 03:28 PM, Akihiko Odaki wrote:
>> The search, share, connect(?), and settings keys
> I tested the patch again with xev and found that those "charm"
> keys don't respond both on hid-microsoft and hid-multitouch, while
> other keys respond. I'll have a further look.
>
> Anyway, keys working with hid-microsoft also work with
> hid-multitouch, so It's ready for merging, I think.
>
> On 12/15/2015 01:39 AM, Donavan Lance wrote:
>> On Mon, Dec 14, 2015 at 8:22 AM, Bastien Nocera
>> <[email protected]>
> wrote:
>>> On Mon, 2015-12-14 at 21:50 +0900, Akihiko Odaki wrote:
>>>> Use multitouch driver instead of microsoft one for Microsoft
>>>> Surface Type Covers.
>>>>
>>>> By using MT_CLS_EXPORT_ALL_INPUTS, the keyboards function as
>>>> well as the multitouch pads do.
>>>
>>> I've discussed this a couple of weeks back with Benjamin
>>> Tissoires, and this patch would break the special keys (mute,
>>> brightness up/down, keyboard backlight up/down and
>>> play/pause).
>>>
>>> The recommended way to fix this was to move multi-touch
>>> processing into the Microsoft driver, so that it would handle
>>> the trackpad's multi- touch events.
>>>
>>> You should be able to do this by carefully picking up the
>>> handling code from hid-multitouch, or do something similar to
>>> what's done in hid- wacom, which has the same problem as the
>>> Type Cover handling.
>>>
>>> Can you confirm that this does indeed break those special
>>> keys? If it does, it's a NAK from my side.
>>
>> For what it's worth the special keys on my keyboard work fine
>> when using this patch. I'm using a Surface Pro 3, Type Cover 3,
>> running GNOME and Fedora 23. The search, share, connect(?), and
>> settings keys are the only ones not mapped to anything out of the
>> box, but they are recognized by xev.
>>

2015-12-18 10:41:36

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH 1/2] HID: Use multitouch driver for Type Covers

On Fri, Dec 18, 2015 at 10:12 AM, Akihiko Odaki
<[email protected]> wrote:
> The "Charm" keys seem to issue short-cut key combinations.
> I also found Microsoft's document by searching for the combinations.
>
> https://msdn.microsoft.com/en-us/library/windows/hardware/dn614461(v=vs.85).aspx
>
> So the behavior of hid-multitouch is completely fine.

OK, so it looks like there was no need to use hid-microsoft for these covers.

However, I'd rather not add a full batch of the PIDs, but rather a
completed tested environment before migrating these covers from
hid-microsoft to hid-multitouch.

Can you restrict the patch to the covers you actually have and were
able to test?

Cheers,
Benjamin

>
> On 12/18/2015 03:28 PM, Akihiko Odaki wrote:
>>> The search, share, connect(?), and settings keys
>> I tested the patch again with xev and found that those "charm"
>> keys don't respond both on hid-microsoft and hid-multitouch, while
>> other keys respond. I'll have a further look.
>>
>> Anyway, keys working with hid-microsoft also work with
>> hid-multitouch, so It's ready for merging, I think.
>>
>> On 12/15/2015 01:39 AM, Donavan Lance wrote:
>>> On Mon, Dec 14, 2015 at 8:22 AM, Bastien Nocera
>>> <[email protected]>
>> wrote:
>>>> On Mon, 2015-12-14 at 21:50 +0900, Akihiko Odaki wrote:
>>>>> Use multitouch driver instead of microsoft one for Microsoft
>>>>> Surface Type Covers.
>>>>>
>>>>> By using MT_CLS_EXPORT_ALL_INPUTS, the keyboards function as
>>>>> well as the multitouch pads do.
>>>>
>>>> I've discussed this a couple of weeks back with Benjamin
>>>> Tissoires, and this patch would break the special keys (mute,
>>>> brightness up/down, keyboard backlight up/down and
>>>> play/pause).
>>>>
>>>> The recommended way to fix this was to move multi-touch
>>>> processing into the Microsoft driver, so that it would handle
>>>> the trackpad's multi- touch events.
>>>>
>>>> You should be able to do this by carefully picking up the
>>>> handling code from hid-multitouch, or do something similar to
>>>> what's done in hid- wacom, which has the same problem as the
>>>> Type Cover handling.
>>>>
>>>> Can you confirm that this does indeed break those special
>>>> keys? If it does, it's a NAK from my side.
>>>
>>> For what it's worth the special keys on my keyboard work fine
>>> when using this patch. I'm using a Surface Pro 3, Type Cover 3,
>>> running GNOME and Fedora 23. The search, share, connect(?), and
>>> settings keys are the only ones not mapped to anything out of the
>>> box, but they are recognized by xev.
>>>

2015-12-18 15:06:52

by Bastien Nocera

[permalink] [raw]
Subject: Re: [PATCH 1/2] HID: Use multitouch driver for Type Covers

On Mon, 2015-12-14 at 21:50 +0900, Akihiko Odaki wrote:
> Use multitouch driver instead of microsoft one for Microsoft Surface
> Type Covers.
>
> By using MT_CLS_EXPORT_ALL_INPUTS, the keyboards function as well as
> the multitouch pads do.
>
> Signed-off-by: Akihiko Odaki <[email protected]>

All the multimedia keys work, and MT support also works, on my Surface
3 cover (045e:07de).

*But* the Caps-Lock key's LED doesn't light up anymore. Can you verify
it does on yours as well?

Cheers

2015-12-19 01:34:28

by Akihiko Odaki

[permalink] [raw]
Subject: Re: [PATCH 1/2] HID: Use multitouch driver for Type Covers

No, it doesn't work.

On 12/19/2015 12:06 AM, Bastien Nocera wrote:
> On Mon, 2015-12-14 at 21:50 +0900, Akihiko Odaki wrote:
>> Use multitouch driver instead of microsoft one for Microsoft Surface
>> Type Covers.
>>
>> By using MT_CLS_EXPORT_ALL_INPUTS, the keyboards function as well as
>> the multitouch pads do.
>>
>> Signed-off-by: Akihiko Odaki <[email protected]>
>
> All the multimedia keys work, and MT support also works, on my Surface
> 3 cover (045e:07de).
>
> *But* the Caps-Lock key's LED doesn't light up anymore. Can you verify
> it does on yours as well?
>
> Cheers
>

2016-04-05 12:48:12

by Bastien Nocera

[permalink] [raw]
Subject: Re: [PATCH 1/2] HID: Use multitouch driver for Type Covers

On Sat, 2015-12-19 at 10:34 +0900, Akihiko Odaki wrote:
> No, it doesn't work.

According to:
https://github.com/jimdigriz/debian-mssp4

Running "kbd_mode -u" afterwards would fix the Caps-Lock key not
working. I have no idea what the wider consequence of this would be
though[1].

Anything we can do in the kernel to have that working by default?

[1]: kbd_mode calls ioctl(fd, KDSKBMODE, K_UNICODE) with fd being the
console's fd.

> On 12/19/2015 12:06 AM, Bastien Nocera wrote:
> >
> > On Mon, 2015-12-14 at 21:50 +0900, Akihiko Odaki wrote:
> > >
> > > Use multitouch driver instead of microsoft one for Microsoft
> > > Surface
> > > Type Covers.
> > >
> > > By using MT_CLS_EXPORT_ALL_INPUTS, the keyboards function as well
> > > as
> > > the multitouch pads do.
> > >
> > > Signed-off-by: Akihiko Odaki <[email protected]>
> > All the multimedia keys work, and MT support also works, on my
> > Surface
> > 3 cover (045e:07de).
> >
> > *But* the Caps-Lock key's LED doesn't light up anymore. Can you
> > verify
> > it does on yours as well?
> >
> > Cheers
> >