[PATCH 0/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[PATCH 0/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Derya]

The MS Surface Pro 2 has a very annoying USB composite device on port 2.3.
It has 3 interfaces:
- Interface 0 is the sensor-hub
- Interface 1 is the wacom digitizer² (it's one without finger input, 
there is an atmel digitizer on port 2.4 for finger input)
- Interface 2 is the keyboard, if a keyboard cover is attached.

This USB composite device changes it product id depending if and which 
keyboard cover is attached.
Each of this hid devices contains several collections, this complicated 
everything.
I have uploaded a lsusb output to: http://pastebin.com/Jun5sa2t

I have to say, that I'm neither a developer nor a programmer, this means 
this things are beyond my ken, please excuse if my remarks aren't accurate.

The Touch and Type Covers (2) didn't work out of the box. Someone one 
the ubuntuforums made a patch which adds them to the microsoft special 
driver, but this leads to loading that driver for all the 3 interfaces 
and prevents loading of hid-sensor-hub for interface 0, if a keyboard 
cover is attached. Sadly this patch is submitted to the upstream kernel 
source.
Without that patch the keyboard covers are loading hid-multitouch 
instead of hid-generic. This misbehaviour is a result of the fancy hid 
collections, that the keyboards have. With hid-multitouch neither the 
keyboard nor the touchpad of the cover works². I added an if clause to 
hid_scan_input_usage to prevent loading of hid-multitouch for the 
keyboards. With hid-generic keyboard and touchpad are working (they come 
up as one input)

We also need the HID_QUIRK_NOGET for this usb composite device, without 
it hid-sensor-hub fails with a submit urb failure evertime a keyboard 
cover is (de)attached and it takes some seconds until the keyboard and 
wacom digitizer responds.

The second patch adds HID_SENSOR_HUB_ENUM_QUIRK for the Surface Pro 2's 
sensor-hub. There is still a bug with the sensors and the Surface Pro 2, 
but I didn't dig into it yet (hid-sensor-magn-3d fails to setup attributes)

Regards,

Derya



1 I'm also working to get the wacom driver working. At the moment the 
stylus works with hid-generic(if my patch is applied, without it use 
hid-microsoft). I got it working with wacom driver without disturbing 
the other interfaces, but the wacom interface contains also some fancy 
collection. The wacom driver doesn't care of them, this leads to losing 
the on device volume and left meta keys. With hid-generic they work, but 
the input events get distorted after the use of the eraser. But, this is 
another story...

2 @Benjamin Tissoires
I have tried your patches for fancy collection in hid-multitouch. It 
seems to be the right way to solve the problem with the keyboard (better 
than my approach to exclude this devices in hid_scan_input_usage), but 
it has the some drawbacks at the moment. It splits the input into 5 
pieces. 2 seperate keyboard inputs that leads into losing the caps lock 
led. The touchpad is mapped as a mouse. There is also a consumer device, 
which gives me no input and an unkown device, but no multitouch device. 
There is a HID_DG_INPUTMODE out of range error in dmesg. Please, contact 
me, if you need some logs.



--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Derya]

The Surface Pro 2 has a very annoying USB composite device on port 2.3.
It has 3 interfaces, interface 0 is the hid-sensor-hub, interface 1 the 
wacom
digitizer and if a keyboard is attached, then it is on interface 3. This USB
composite device changes it's product id, it depends on if and which 
keyboard
cover is attached. Adding the Type-/Touch Cover 2 to hid_have_special_driver
prevents loading the hid-sensor-hub driver for interface 0, if a cover is
attached (commit 117309c51dca42121f70cacec801511b76acf75c).
All 3 interfaces are loaded with the microsoft special driver. Interface 3,
the keyboard covers, has a HID_DG_CONTACTID. This leads to loading of
hid-multitouch for the keyboard, if we remove it from the special driver 
list.
Neither the keyboard not the touchpad works with hid-multitouch.
I add a check for vendor and product id in hid_scan_input_usage to prevent
the loading of hid-multitouch. By this way it loads hid-generic for
the keyboards without effecting the other two interfaces.
The HID_QUIRK_NOGET is needed for hid-sensor-hub, without it Submit urb 
fails
and it takes some seconds until the devices on this port get responsible
whenever a keyboard cover is (de)attached.

Signed-off-by: Derya <derya.kiran@yahoo.de>
---
  drivers/hid/hid-core.c          | 5 ++---
  drivers/hid/hid-ids.h           | 1 +
  drivers/hid/hid-microsoft.c     | 4 ----
  drivers/hid/usbhid/hid-quirks.c | 3 +++
  4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index cc32a6f..2d60a1a 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -685,7 +685,8 @@ static void hid_scan_input_usage(struct hid_parser 
*parser, u32 usage)
      struct hid_device *hid = parser->device;

      if (usage == HID_DG_CONTACTID)
-        hid->group = HID_GROUP_MULTITOUCH;
+        if (!(hid->vendor == USB_VENDOR_ID_MICROSOFT && (hid->product 
== USB_DEVICE_ID_MS_TYPE_COVER_2 || hid->product == 
USB_DEVICE_ID_MS_TOUCH_COVER_2)))
+            hid->group = HID_GROUP_MULTITOUCH;
  }

  static void hid_scan_feature_usage(struct hid_parser *parser, u32 usage)
@@ -1780,8 +1781,6 @@ static const struct hid_device_id 
hid_have_special_driver[] = {
      { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, 
USB_DEVICE_ID_MS_PRESENTER_8K_USB) },
      { 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_TYPE_COVER_2) },
-    { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, 
USB_DEVICE_ID_MS_TOUCH_COVER_2) },
      { HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, 
USB_DEVICE_ID_GENIUS_KB29E) },
      { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, 
USB_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
      { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, 
USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 22f28d6..0aa9f7e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -624,6 +624,7 @@
  #define USB_DEVICE_ID_MS_PRESENTER_8K_USB    0x0713
  #define USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K    0x0730
  #define USB_DEVICE_ID_MS_COMFORT_MOUSE_4500    0x076c
+#define USB_DEVICE_ID_MS_SURFACE_PRO_2    0x0799        //Surface Pro 
2's MICROSOFT SAM device without a keyboard cover attached, changes 
product id with cover
  #define USB_DEVICE_ID_MS_TOUCH_COVER_2    0x07a7
  #define USB_DEVICE_ID_MS_TYPE_COVER_2    0x07a9

diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index 404a3a8..c6ef6ee 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -208,10 +208,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_2),
-        .driver_data = 0 },
-    { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, 
USB_DEVICE_ID_MS_TOUCH_COVER_2),
-        .driver_data = 0 },

      { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 
USB_DEVICE_ID_MS_PRESENTER_8K_BT),
          .driver_data = MS_PRESENTER },
diff --git a/drivers/hid/usbhid/hid-quirks.c 
b/drivers/hid/usbhid/hid-quirks.c
index dbd8387..2b7fd92 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -73,6 +73,9 @@ static const struct hid_blacklist {
      { USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, 
HID_QUIRK_NO_INIT_REPORTS },
      { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, 
HID_QUIRK_NOGET },
      { USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
+    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_SURFACE_PRO_2, 
HID_QUIRK_NOGET },
+    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_2, 
HID_QUIRK_NOGET },
+    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TOUCH_COVER_2, 
HID_QUIRK_NOGET },
      { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, 
HID_QUIRK_NO_INIT_REPORTS },
      { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, 
HID_QUIRK_NO_INIT_REPORTS },
      { USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, 
HID_QUIRK_NO_INIT_REPORTS },
-- 
1.8.3.2

Re: [PATCH 1/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Benjamin Tissoires]

On Fri, Mar 7, 2014 at 10:56 AM, Derya <derya.kiran@yahoo.de> wrote:
> The Surface Pro 2 has a very annoying USB composite device on port 2.3.
> It has 3 interfaces, interface 0 is the hid-sensor-hub, interface 1 the
> wacom
> digitizer and if a keyboard is attached, then it is on interface 3. This USB
> composite device changes it's product id, it depends on if and which
> keyboard
> cover is attached. Adding the Type-/Touch Cover 2 to hid_have_special_driver
> prevents loading the hid-sensor-hub driver for interface 0, if a cover is
> attached (commit 117309c51dca42121f70cacec801511b76acf75c).

This patch obviously reverts this commit and add different features.
I would be happy if you split it in two:
- the first one would be the revert (given by "git revert" with a
clear commit message) so we will have a clear view that this patch was
broken
- then you do the additions (you will have to re-add things in
hid-ids.h, but it's not a big deal).

> All 3 interfaces are loaded with the microsoft special driver. Interface 3,
> the keyboard covers, has a HID_DG_CONTACTID. This leads to loading of
> hid-multitouch for the keyboard, if we remove it from the special driver
> list.
> Neither the keyboard not the touchpad works with hid-multitouch.
> I add a check for vendor and product id in hid_scan_input_usage to prevent
> the loading of hid-multitouch. By this way it loads hid-generic for
> the keyboards without effecting the other two interfaces.
> The HID_QUIRK_NOGET is needed for hid-sensor-hub, without it Submit urb
> fails
> and it takes some seconds until the devices on this port get responsible
> whenever a keyboard cover is (de)attached.
>
> Signed-off-by: Derya <derya.kiran@yahoo.de>
> ---
>  drivers/hid/hid-core.c          | 5 ++---
>  drivers/hid/hid-ids.h           | 1 +
>  drivers/hid/hid-microsoft.c     | 4 ----
>  drivers/hid/usbhid/hid-quirks.c | 3 +++
>  4 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index cc32a6f..2d60a1a 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -685,7 +685,8 @@ static void hid_scan_input_usage(struct hid_parser
> *parser, u32 usage)
>      struct hid_device *hid = parser->device;
>
>      if (usage == HID_DG_CONTACTID)
> -        hid->group = HID_GROUP_MULTITOUCH;
> +        if (!(hid->vendor == USB_VENDOR_ID_MICROSOFT && (hid->product ==
> USB_DEVICE_ID_MS_TYPE_COVER_2 || hid->product ==
> USB_DEVICE_ID_MS_TOUCH_COVER_2)))
> +            hid->group = HID_GROUP_MULTITOUCH;

Please don't do such vendor specific tests here.

I'd be happy if you put at the end of hid_scan_report()

    /*
    * Handle vendor specific handlings
    */
    if ((hid->vendor == USB_VENDOR_ID_MICROSOFT) &&
        (hid->product == USB_DEVICE_ID_MS_TYPE_COVER_2 ||
         hid->product == USB_DEVICE_ID_MS_TOUCH_COVER_2) &&
        (hid->group != HID_GROUP_MULTITOUCH))
        hid->group = HID_GROUP_GENERIC;

this way, we can add other vendors quirks (who said Wacom? :-P )

>  }
>
>  static void hid_scan_feature_usage(struct hid_parser *parser, u32 usage)
> @@ -1780,8 +1781,6 @@ static const struct hid_device_id
> hid_have_special_driver[] = {
>      { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> USB_DEVICE_ID_MS_PRESENTER_8K_USB) },
>      { 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_TYPE_COVER_2) },
> -    { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,

this should be in the revert commit as mentioned

> USB_DEVICE_ID_MS_TOUCH_COVER_2) },
>      { HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },
>      { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG, USB_DEVICE_ID_NTRIG_TOUCH_SCREEN)
> },
>      { HID_USB_DEVICE(USB_VENDOR_ID_NTRIG,
> USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 22f28d6..0aa9f7e 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -624,6 +624,7 @@
>  #define USB_DEVICE_ID_MS_PRESENTER_8K_USB    0x0713
>  #define USB_DEVICE_ID_MS_DIGITAL_MEDIA_3K    0x0730
>  #define USB_DEVICE_ID_MS_COMFORT_MOUSE_4500    0x076c
> +#define USB_DEVICE_ID_MS_SURFACE_PRO_2    0x0799        //Surface Pro 2's
> MICROSOFT SAM device without a keyboard cover attached, changes product id

please do not use "//" but /* ... */ for the comments.

Also, we are "limited" to 80 chars per line, so this comment should go
above the two declarations.

> with cover
>  #define USB_DEVICE_ID_MS_TOUCH_COVER_2    0x07a7
>  #define USB_DEVICE_ID_MS_TYPE_COVER_2    0x07a9
>
> diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
> index 404a3a8..c6ef6ee 100644
> --- a/drivers/hid/hid-microsoft.c
> +++ b/drivers/hid/hid-microsoft.c
> @@ -208,10 +208,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_2),
> -        .driver_data = 0 },
> -    { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT,
> USB_DEVICE_ID_MS_TOUCH_COVER_2),
> -        .driver_data = 0 },

revert patch

>
>      { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
> USB_DEVICE_ID_MS_PRESENTER_8K_BT),
>          .driver_data = MS_PRESENTER },
> diff --git a/drivers/hid/usbhid/hid-quirks.c
> b/drivers/hid/usbhid/hid-quirks.c
> index dbd8387..2b7fd92 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -73,6 +73,9 @@ static const struct hid_blacklist {
>      { USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER,
> HID_QUIRK_NO_INIT_REPORTS },
>      { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28,
> HID_QUIRK_NOGET },
>      { USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
> +    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_SURFACE_PRO_2,
> HID_QUIRK_NOGET },

please test HID_QUIRK_NO_INIT_INPUT_REPORTS

Good work!

Cheers,
Benjamin

> +    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TYPE_COVER_2,
> HID_QUIRK_NOGET },
> +    { USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TOUCH_COVER_2,
> HID_QUIRK_NOGET },
>      { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL,
> HID_QUIRK_NO_INIT_REPORTS },
>      { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750,
> HID_QUIRK_NO_INIT_REPORTS },
>      { USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE,
> HID_QUIRK_NO_INIT_REPORTS },
> --
> 1.8.3.2
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Derya]

Enumeration quirks for Surface Pro 2 sensor-hub

Signed-off-by: Derya <derya.kiran@yahoo.de>
---
  drivers/hid/hid-sensor-hub.c | 9 +++++++++
  1 file changed, 9 insertions(+)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 9c22e14..16f4bb8 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -668,6 +668,15 @@ static const struct hid_device_id 
sensor_hub_devices[] = {
      { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_STM_0,
              USB_DEVICE_ID_STM_HID_SENSOR),
              .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
+    { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, 
USB_VENDOR_ID_MICROSOFT,
+             USB_DEVICE_ID_MS_TOUCH_COVER_2),
+            .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
+    { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, 
USB_VENDOR_ID_MICROSOFT,
+             USB_DEVICE_ID_MS_TYPE_COVER_2),
+            .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
+    { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, 
USB_VENDOR_ID_MICROSOFT,
+             USB_DEVICE_ID_MS_SURFACE_PRO_2),
+            .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
      { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
               HID_ANY_ID) },
      { }
-- 
1.8.3.2

Re: [PATCH 2/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Benjamin Tissoires]

On Fri, Mar 7, 2014 at 10:58 AM, Derya <derya.kiran@yahoo.de> wrote:
> Enumeration quirks for Surface Pro 2 sensor-hub
>
> Signed-off-by: Derya <derya.kiran@yahoo.de>
> ---
>  drivers/hid/hid-sensor-hub.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 9c22e14..16f4bb8 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -668,6 +668,15 @@ static const struct hid_device_id sensor_hub_devices[]
> = {
>      { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_STM_0,
>              USB_DEVICE_ID_STM_HID_SENSOR),
>              .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> +    { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_MICROSOFT,
> +             USB_DEVICE_ID_MS_TOUCH_COVER_2),
> +            .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> +    { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_MICROSOFT,
> +             USB_DEVICE_ID_MS_TYPE_COVER_2),
> +            .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> +    { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_MICROSOFT,
> +             USB_DEVICE_ID_MS_SURFACE_PRO_2),
> +            .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},

this seems to be pretty heavy to have all three product id in the sensor hub.
Is it mandatory?

Cheers,
Benjamin

>      { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
>               HID_ANY_ID) },
>      { }
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Derya]

Am 07.03.2014 18:01, schrieb Benjamin Tissoires:
> On Fri, Mar 7, 2014 at 10:58 AM, Derya <derya.kiran@yahoo.de> wrote:
>> Enumeration quirks for Surface Pro 2 sensor-hub
>>
>> Signed-off-by: Derya <derya.kiran@yahoo.de>
>> ---
>>   drivers/hid/hid-sensor-hub.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
>> index 9c22e14..16f4bb8 100644
>> --- a/drivers/hid/hid-sensor-hub.c
>> +++ b/drivers/hid/hid-sensor-hub.c
>> @@ -668,6 +668,15 @@ static const struct hid_device_id sensor_hub_devices[]
>> = {
>>       { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_STM_0,
>>               USB_DEVICE_ID_STM_HID_SENSOR),
>>               .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
>> +    { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
>> USB_VENDOR_ID_MICROSOFT,
>> +             USB_DEVICE_ID_MS_TOUCH_COVER_2),
>> +            .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
>> +    { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
>> USB_VENDOR_ID_MICROSOFT,
>> +             USB_DEVICE_ID_MS_TYPE_COVER_2),
>> +            .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
>> +    { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
>> USB_VENDOR_ID_MICROSOFT,
>> +             USB_DEVICE_ID_MS_SURFACE_PRO_2),
>> +            .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> this seems to be pretty heavy to have all three product id in the sensor hub.
> Is it mandatory?

We can post prune this until I have the time to have a closer look into 
the sensor stuff. The problem is the product id changes with the covers. 
We can use HID_ANY_ID instead of the product id, but that will apply to 
other MS devices, too. There are more MS Covers out there, I don't know 
how the others behave and I don't know, if the Surface Pro 1 needs it also.
>
> Cheers,
> Benjamin
>
>>       { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, HID_ANY_ID,
>>                HID_ANY_ID) },
>>       { }
>> --
>> 1.8.3.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Benjamin Tissoires]

Hi Derya,

On Fri, Mar 7, 2014 at 10:53 AM, Derya <derya.kiran@yahoo.de> wrote:
> The MS Surface Pro 2 has a very annoying USB composite device on port 2.3.
> It has 3 interfaces:
> - Interface 0 is the sensor-hub
> - Interface 1 is the wacom digitizer² (it's one without finger input, there
> is an atmel digitizer on port 2.4 for finger input)
> - Interface 2 is the keyboard, if a keyboard cover is attached.

Oh... yes, indeed, it's gona be tricky :)

>
> This USB composite device changes it product id depending if and which
> keyboard cover is attached.
> Each of this hid devices contains several collections, this complicated
> everything.
> I have uploaded a lsusb output to: http://pastebin.com/Jun5sa2t

Thanks

>
> I have to say, that I'm neither a developer nor a programmer, this means
> this things are beyond my ken, please excuse if my remarks aren't accurate.

But you still made the patches, so you are a programmer :)

>
> The Touch and Type Covers (2) didn't work out of the box. Someone one the
> ubuntuforums made a patch which adds them to the microsoft special driver,
> but this leads to loading that driver for all the 3 interfaces and prevents
> loading of hid-sensor-hub for interface 0, if a keyboard cover is attached.
> Sadly this patch is submitted to the upstream kernel source.

Well, if it breaks things, it's still time to revert it.

> Without that patch the keyboard covers are loading hid-multitouch instead of
> hid-generic. This misbehaviour is a result of the fancy hid collections,
> that the keyboards have. With hid-multitouch neither the keyboard nor the
> touchpad of the cover works². I added an if clause to hid_scan_input_usage
> to prevent loading of hid-multitouch for the keyboards. With hid-generic
> keyboard and touchpad are working (they come up as one input)

More comment on that in the patch

>
> We also need the HID_QUIRK_NOGET for this usb composite device, without it
> hid-sensor-hub fails with a submit urb failure evertime a keyboard cover is
> (de)attached and it takes some seconds until the keyboard and wacom
> digitizer responds.

Can you test if HID_QUIRK_NO_INIT_INPUT_REPORTS works. It's a little
bit less a hammer than NO_GET, and I am pretty sure that this is what
Windows does by default.

>
> The second patch adds HID_SENSOR_HUB_ENUM_QUIRK for the Surface Pro 2's
> sensor-hub. There is still a bug with the sensors and the Surface Pro 2, but
> I didn't dig into it yet (hid-sensor-magn-3d fails to setup attributes)
>
> Regards,
>
> Derya
>
>
>
> 1 I'm also working to get the wacom driver working. At the moment the stylus
> works with hid-generic(if my patch is applied, without it use
> hid-microsoft). I got it working with wacom driver without disturbing the
> other interfaces, but the wacom interface contains also some fancy
> collection. The wacom driver doesn't care of them, this leads to losing the
> on device volume and left meta keys. With hid-generic they work, but the
> input events get distorted after the use of the eraser. But, this is another
> story...

For the record, I am tempted (and some people at wacom too) to switch
the Wacom devices to use hid-wacom instead of wacom.ko. This may solve
your problems here.

>
> 2 @Benjamin Tissoires
> I have tried your patches for fancy collection in hid-multitouch. It seems
> to be the right way to solve the problem with the keyboard (better than my
> approach to exclude this devices in hid_scan_input_usage), but it has the
> some drawbacks at the moment. It splits the input into 5 pieces. 2 seperate
> keyboard inputs that leads into losing the caps lock led. The touchpad is
> mapped as a mouse. There is also a consumer device, which gives me no input
> and an unkown device, but no multitouch device. There is a HID_DG_INPUTMODE
> out of range error in dmesg. Please, contact me, if you need some logs.

With the lsusb output I should be able to conduct more tests. But I
may not have time to do them.
Did you used the latest patch applied this week by Jiri?

Cheers,
Benjamin

>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Derya]

Am 07.03.2014 17:51, schrieb Benjamin Tissoires:
> Hi Derya,
>
> On Fri, Mar 7, 2014 at 10:53 AM, Derya <derya.kiran@yahoo.de> wrote:
>> The MS Surface Pro 2 has a very annoying USB composite device on port 2.3.
>> It has 3 interfaces:
>> - Interface 0 is the sensor-hub
>> - Interface 1 is the wacom digitizer² (it's one without finger input, there
>> is an atmel digitizer on port 2.4 for finger input)
>> - Interface 2 is the keyboard, if a keyboard cover is attached.
> Oh... yes, indeed, it's gona be tricky :)
>
>> This USB composite device changes it product id depending if and which
>> keyboard cover is attached.
>> Each of this hid devices contains several collections, this complicated
>> everything.
>> I have uploaded a lsusb output to: http://pastebin.com/Jun5sa2t
> Thanks
>
>> I have to say, that I'm neither a developer nor a programmer, this means
>> this things are beyond my ken, please excuse if my remarks aren't accurate.
> But you still made the patches, so you are a programmer :)
>
>> The Touch and Type Covers (2) didn't work out of the box. Someone one the
>> ubuntuforums made a patch which adds them to the microsoft special driver,
>> but this leads to loading that driver for all the 3 interfaces and prevents
>> loading of hid-sensor-hub for interface 0, if a keyboard cover is attached.
>> Sadly this patch is submitted to the upstream kernel source.
> Well, if it breaks things, it's still time to revert it.
>
>> Without that patch the keyboard covers are loading hid-multitouch instead of
>> hid-generic. This misbehaviour is a result of the fancy hid collections,
>> that the keyboards have. With hid-multitouch neither the keyboard nor the
>> touchpad of the cover works². I added an if clause to hid_scan_input_usage
>> to prevent loading of hid-multitouch for the keyboards. With hid-generic
>> keyboard and touchpad are working (they come up as one input)
> More comment on that in the patch

I will follow your comments and update the patch.
>
>> We also need the HID_QUIRK_NOGET for this usb composite device, without it
>> hid-sensor-hub fails with a submit urb failure evertime a keyboard cover is
>> (de)attached and it takes some seconds until the keyboard and wacom
>> digitizer responds.
> Can you test if HID_QUIRK_NO_INIT_INPUT_REPORTS works. It's a little
> bit less a hammer than NO_GET, and I am pretty sure that this is what
> Windows does by default.
>

Sure, I can give it a try :-)
>> The second patch adds HID_SENSOR_HUB_ENUM_QUIRK for the Surface Pro 2's
>> sensor-hub. There is still a bug with the sensors and the Surface Pro 2, but
>> I didn't dig into it yet (hid-sensor-magn-3d fails to setup attributes)
>>
>> Regards,
>>
>> Derya
>>
>>
>>
>> 1 I'm also working to get the wacom driver working. At the moment the stylus
>> works with hid-generic(if my patch is applied, without it use
>> hid-microsoft). I got it working with wacom driver without disturbing the
>> other interfaces, but the wacom interface contains also some fancy
>> collection. The wacom driver doesn't care of them, this leads to losing the
>> on device volume and left meta keys. With hid-generic they work, but the
>> input events get distorted after the use of the eraser. But, this is another
>> story...
> For the record, I am tempted (and some people at wacom too) to switch
> the Wacom devices to use hid-wacom instead of wacom.ko. This may solve
> your problems here.
I've looked into hid-wacom and thought, it would be better to use it 
instead of wacom.ko, but then I saw it contains only wireless device. It 
seemed to be the wrong place. The other thing is, if I use hid-wacom, 
then I have to add that USB composite device again to the special driver 
list, like with hid-microsoft, and this leads us to the same problem. It 
would load hid-wacom for all 3 interfaces. Or am I wrong? With wacom.ko 
I'm using the macro USB_DEVICE_INTERFACE_NUMBER (instead of USB_DEVICE 
and USB_DEVICE_AND_INTERFACE_INFO, which are used for other wacon 
devices) to applied the driver only to the wacom interface without 
disturbing the other ones.
I will try to use hid-wacom, but this will take some time.

>> 2 @Benjamin Tissoires
>> I have tried your patches for fancy collection in hid-multitouch. It seems
>> to be the right way to solve the problem with the keyboard (better than my
>> approach to exclude this devices in hid_scan_input_usage), but it has the
>> some drawbacks at the moment. It splits the input into 5 pieces. 2 seperate
>> keyboard inputs that leads into losing the caps lock led. The touchpad is
>> mapped as a mouse. There is also a consumer device, which gives me no input
>> and an unkown device, but no multitouch device. There is a HID_DG_INPUTMODE
>> out of range error in dmesg. Please, contact me, if you need some logs.
> With the lsusb output I should be able to conduct more tests. But I
> may not have time to do them.
> Did you used the latest patch applied this week by Jiri?

I tried it with this 
http://www.spinics.net/lists/linux-input/msg30018.html patch set.
>
> Cheers,
> Benjamin
>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Benjamin Tissoires]

On Fri, Mar 7, 2014 at 12:50 PM, Derya <derya.kiran@yahoo.de> wrote:
>
> Am 07.03.2014 17:51, schrieb Benjamin Tissoires:
>
>> Hi Derya,
>>
>> On Fri, Mar 7, 2014 at 10:53 AM, Derya <derya.kiran@yahoo.de> wrote:
>>>
>>> The MS Surface Pro 2 has a very annoying USB composite device on port
>>> 2.3.
>>> It has 3 interfaces:
>>> - Interface 0 is the sensor-hub
>>> - Interface 1 is the wacom digitizer² (it's one without finger input,
>>> there
>>> is an atmel digitizer on port 2.4 for finger input)
>>> - Interface 2 is the keyboard, if a keyboard cover is attached.
>>
>> Oh... yes, indeed, it's gona be tricky :)
>>
>>> This USB composite device changes it product id depending if and which
>>> keyboard cover is attached.
>>> Each of this hid devices contains several collections, this complicated
>>> everything.
>>> I have uploaded a lsusb output to: http://pastebin.com/Jun5sa2t
>>
>> Thanks
>>
>>> I have to say, that I'm neither a developer nor a programmer, this means
>>> this things are beyond my ken, please excuse if my remarks aren't
>>> accurate.
>>
>> But you still made the patches, so you are a programmer :)
>>
>>> The Touch and Type Covers (2) didn't work out of the box. Someone one the
>>> ubuntuforums made a patch which adds them to the microsoft special
>>> driver,
>>> but this leads to loading that driver for all the 3 interfaces and
>>> prevents
>>> loading of hid-sensor-hub for interface 0, if a keyboard cover is
>>> attached.
>>> Sadly this patch is submitted to the upstream kernel source.
>>
>> Well, if it breaks things, it's still time to revert it.
>>
>>> Without that patch the keyboard covers are loading hid-multitouch instead
>>> of
>>> hid-generic. This misbehaviour is a result of the fancy hid collections,
>>> that the keyboards have. With hid-multitouch neither the keyboard nor the
>>> touchpad of the cover works². I added an if clause to
>>> hid_scan_input_usage
>>> to prevent loading of hid-multitouch for the keyboards. With hid-generic
>>> keyboard and touchpad are working (they come up as one input)
>>
>> More comment on that in the patch
>
>
> I will follow your comments and update the patch.
>
>>
>>> We also need the HID_QUIRK_NOGET for this usb composite device, without
>>> it
>>> hid-sensor-hub fails with a submit urb failure evertime a keyboard cover
>>> is
>>> (de)attached and it takes some seconds until the keyboard and wacom
>>> digitizer responds.
>>
>> Can you test if HID_QUIRK_NO_INIT_INPUT_REPORTS works. It's a little
>> bit less a hammer than NO_GET, and I am pretty sure that this is what
>> Windows does by default.
>>
>
> Sure, I can give it a try :-)
>
>>> The second patch adds HID_SENSOR_HUB_ENUM_QUIRK for the Surface Pro 2's
>>> sensor-hub. There is still a bug with the sensors and the Surface Pro 2,
>>> but
>>> I didn't dig into it yet (hid-sensor-magn-3d fails to setup attributes)
>>>
>>> Regards,
>>>
>>> Derya
>>>
>>>
>>>
>>> 1 I'm also working to get the wacom driver working. At the moment the
>>> stylus
>>> works with hid-generic(if my patch is applied, without it use
>>> hid-microsoft). I got it working with wacom driver without disturbing the
>>> other interfaces, but the wacom interface contains also some fancy
>>> collection. The wacom driver doesn't care of them, this leads to losing
>>> the
>>> on device volume and left meta keys. With hid-generic they work, but the
>>> input events get distorted after the use of the eraser. But, this is
>>> another
>>> story...
>>
>> For the record, I am tempted (and some people at wacom too) to switch
>> the Wacom devices to use hid-wacom instead of wacom.ko. This may solve
>> your problems here.
>
> I've looked into hid-wacom and thought, it would be better to use it instead
> of wacom.ko, but then I saw it contains only wireless device. It seemed to
> be the wrong place. The other thing is, if I use hid-wacom, then I have to
> add that USB composite device again to the special driver list, like with
> hid-microsoft, and this leads us to the same problem. It would load
> hid-wacom for all 3 interfaces. Or am I wrong? With wacom.ko I'm using the
> macro USB_DEVICE_INTERFACE_NUMBER (instead of USB_DEVICE and
> USB_DEVICE_AND_INTERFACE_INFO, which are used for other wacon devices) to
> applied the driver only to the wacom interface without disturbing the other
> ones.
> I will try to use hid-wacom, but this will take some time.
>

You can, but you will loose your time I think. I just mentioned that
because I mainly wanted to warn Jiri and others on the list that some
people are thinking of it.
But As you said there is not the thing you need currently for this
device, so if you can manage to use wacom.ko, that's fine.

>
>>> 2 @Benjamin Tissoires
>>> I have tried your patches for fancy collection in hid-multitouch. It
>>> seems
>>> to be the right way to solve the problem with the keyboard (better than
>>> my
>>> approach to exclude this devices in hid_scan_input_usage), but it has the
>>> some drawbacks at the moment. It splits the input into 5 pieces. 2
>>> seperate
>>> keyboard inputs that leads into losing the caps lock led. The touchpad is
>>> mapped as a mouse. There is also a consumer device, which gives me no
>>> input
>>> and an unkown device, but no multitouch device. There is a
>>> HID_DG_INPUTMODE
>>> out of range error in dmesg. Please, contact me, if you need some logs.
>>
>> With the lsusb output I should be able to conduct more tests. But I
>> may not have time to do them.
>> Did you used the latest patch applied this week by Jiri?
>
>
> I tried it with this http://www.spinics.net/lists/linux-input/msg30018.html
> patch set.

Alright, this is the last one which has been applied by Jiri.
I'll definitively have to check this out them :(

Cheers,
Benjamin

>
>>
>> Cheers,
>> Benjamin
>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2 ] Surface Pro 2 HID sensor, wacom, keyboard/multitouch composite device

[Tolga Cakir]

2014-03-07 16:53 GMT+01:00 Derya <derya.kiran@yahoo.de>:
> The MS Surface Pro 2 has a very annoying USB composite device on port 2.3.
> It has 3 interfaces:
> - Interface 0 is the sensor-hub
> - Interface 1 is the wacom digitizer² (it's one without finger input, there
> is an atmel digitizer on port 2.4 for finger input)
> - Interface 2 is the keyboard, if a keyboard cover is attached.
>
> This USB composite device changes it product id depending if and which
> keyboard cover is attached.
> Each of this hid devices contains several collections, this complicated
> everything.
> I have uploaded a lsusb output to: http://pastebin.com/Jun5sa2t
>
> We also need the HID_QUIRK_NOGET for this usb composite device, without it
> hid-sensor-hub fails with a submit urb failure evertime a keyboard cover is
> (de)attached and it takes some seconds until the keyboard and wacom
> digitizer responds.

I had a similar issue while implementing Sidewinder X4 support. Can
you check the output of hid->type for each of these interfaces? I was
able to differentiate between the interfaces on my devices by checking
"hid->type", this might help here, too.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html