[PATCH 0/2] Microsoft Surface Type/Touch Cover 2 fixes

[PATCH 0/2] Microsoft Surface Type/Touch Cover 2 fixes

[Benjamin Tissoires]

Hi Jiri,

well, this is a series which should fix the current state of the
Surface 2 touch cover.
The first one has been reported by Derya and the second one fixes the
problem for this device and the future ones.

I added the CC: stable tags, but if you prefer not having them, feel
free to do so.

One last thing: the patches are on top of v3.14, because your for-next
branch does not contain the reverted commit for now. It will hopefully
be ok once you rebased your tree before sending it to Linus.
Patch 2/2 should apply on any kernel >3.12, so your for-next branch
should be fine with this one.

Cheers,
Benjamin


Benjamin Tissoires (2):
  Revert "HID: microsoft: Add ID's for Surface Type/Touch Cover 2"
  HID: core: do not scan constant input report

 drivers/hid/hid-core.c      | 5 +++--
 drivers/hid/hid-ids.h       | 2 --
 drivers/hid/hid-microsoft.c | 4 ----
 3 files changed, 3 insertions(+), 8 deletions(-)

-- 
1.9.0

[PATCH 1/2] Revert "HID: microsoft: Add ID's for Surface Type/Touch Cover 2"

[Benjamin Tissoires]

From: Derya <derya.kiran@yahoo.de>

This reverts commit 117309c51dca42121f70cacec801511b76acf75c.

The MS Surface Pro 2 has an USB composite device with 3 interfaces
- interface 0 - sensor hub
- interface 1 - wacom digitizer
- interface 2 - the keyboard cover, if one is attached
This USB composite device changes it product id dependent on if and which
keyboard cover is attached. Adding the covers to hid_have_special_driver
prevents loading the right hid drivers for the other two interfaces, all 3
get loaded with hid-microsoft. We don't even need hid-microsoft for the
keyboards. We have to revert this to load the right hid modules for each
interface.

CC: stable@vger.kernel.org # kernel 3.14 only
Signed-off-by: Derya <derya.kiran@yahoo.de>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-core.c      | 2 --
 drivers/hid/hid-ids.h       | 2 --
 drivers/hid/hid-microsoft.c | 4 ----
 3 files changed, 8 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index cc32a6f..bb5c494 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1780,8 +1780,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..07cd28c 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -624,8 +624,6 @@
 #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_TOUCH_COVER_2	0x07a7
-#define USB_DEVICE_ID_MS_TYPE_COVER_2	0x07a9
 
 #define USB_VENDOR_ID_MOJO		0x8282
 #define USB_DEVICE_ID_RETRO_ADAPTER	0x3201
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 },
-- 
1.9.0

[PATCH 2/2] HID: core: do not scan constant input report

[Benjamin Tissoires]

The Microsoft Surface Type/Touch Cover 2 is a fancy device which advertised
itself as a multitouch device but with constant input reports.
This way, hid_scan_report() gives the group MULTITOUCH to it, but
hid-multitouch can not handle it due to the constant collection ignored
by hid-input.

To prevent such crap in the future, and while we do not fix this particular
device, make the scan_report coherent with hid-input.c, and ignore constant
input reports.

CC: stable@vger.kernel.org # 3.12+
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index bb5c494..8a5384c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -718,6 +718,9 @@ static int hid_scan_main(struct hid_parser *parser, struct hid_item *item)
 	case HID_MAIN_ITEM_TAG_END_COLLECTION:
 		break;
 	case HID_MAIN_ITEM_TAG_INPUT:
+		/* ignore constant inputs, they will be ignored by hid-input */
+		if (data & HID_MAIN_ITEM_CONSTANT)
+			break;
 		for (i = 0; i < parser->local.usage_index; i++)
 			hid_scan_input_usage(parser, parser->local.usage[i]);
 		break;
-- 
1.9.0

Re: [PATCH 2/2] HID: core: do not scan constant input report

[Reyad Attiyat]

Tested on Surface Pro 2 Type Cover 2 on kernel 3.15/multitouch rebased
on 3.14. The device is now handled by hid-generic and hid-sensor-hub
as expected.
There is a newer UEFI and touch cover firmware that I have not tried,
as you need windows update to get drivers for this device. I will try
and update my device to make sure we don't need any
quirks.

> On Mon, Mar 31, 2014 at 12:27 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> The Microsoft Surface Type/Touch Cover 2 is a fancy device which advertised
>> itself as a multitouch device but with constant input reports.
>> This way, hid_scan_report() gives the group MULTITOUCH to it, but
>> hid-multitouch can not handle it due to the constant collection ignored
>> by hid-input.
>>
>> To prevent such crap in the future, and while we do not fix this particular
>> device, make the scan_report coherent with hid-input.c, and ignore constant
>> input reports.
>>
>> CC: stable@vger.kernel.org # 3.12+
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>>  drivers/hid/hid-core.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index bb5c494..8a5384c 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -718,6 +718,9 @@ static int hid_scan_main(struct hid_parser *parser, struct hid_item *item)
>>         case HID_MAIN_ITEM_TAG_END_COLLECTION:
>>                 break;
>>         case HID_MAIN_ITEM_TAG_INPUT:
>> +               /* ignore constant inputs, they will be ignored by hid-input */
>> +               if (data & HID_MAIN_ITEM_CONSTANT)
>> +                       break;
>>                 for (i = 0; i < parser->local.usage_index; i++)
>>                         hid_scan_input_usage(parser, parser->local.usage[i]);
>>                 break;
>> --
>> 1.9.0
>>

Re: [PATCH 0/2] Microsoft Surface Type/Touch Cover 2 fixes

[Jiri Kosina]

On Mon, 31 Mar 2014, Benjamin Tissoires wrote:

> well, this is a series which should fix the current state of the Surface 
> 2 touch cover. The first one has been reported by Derya and the second 
> one fixes the problem for this device and the future ones.

Thanks Benjamin, queuing this for 3.15 still.

-- 
Jiri Kosina
SUSE Labs

[Patch v2] Surface Pro 2 USB composite device handling

[Derya]

This patch set is a reworked version of the one, that I submitted on Friday.
Followed the Benjamin's suggestions (many thanks)

- The first patch reverses commit 117309c51dca42121f70cacec801511b76acf75c
- The second readds the product ids and ensures the load of hid-core 
instead of hid-multiouch

[PATCH 1/2] Revert "HID: microsoft: Add ID's for Surface Type/Touch, Cover 2"

[Derya]

This reverts commit 117309c51dca42121f70cacec801511b76acf75c.

The MS Surface Pro 2 has an USB composite device with 3 interfaces
- interface 0 - sensor hub
- interface 1 - wacom digitizer
- interface 2 - the keyboard cover, if one is attached
This USB composite device changes it product id dependent on if and which
keyboard cover is attached. Adding the covers to hid_have_special_driver
prevents loading the right hid drivers for the other two interfaces, all 3
get loaded with hid-microsoft. We don't even need hid-microsoft for the
keyboards. We have to revert this to load the right hid modules for each
interface.

Signed-off-by: Derya <derya.kiran@yahoo.de>
---
  drivers/hid/hid-core.c      | 2 --
  drivers/hid/hid-ids.h       | 2 --
  drivers/hid/hid-microsoft.c | 4 ----
  3 files changed, 8 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index cc32a6f..bb5c494 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1780,8 +1780,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..07cd28c 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -624,8 +624,6 @@
  #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_TOUCH_COVER_2    0x07a7
-#define USB_DEVICE_ID_MS_TYPE_COVER_2    0x07a9

  #define USB_VENDOR_ID_MOJO        0x8282
  #define USB_DEVICE_ID_RETRO_ADAPTER    0x3201
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 },
-- 
1.8.3.2

Re: [PATCH 1/2] Revert "HID: microsoft: Add ID's for Surface Type/Touch, Cover 2"

[Benjamin Tissoires]

On Mar 09 2014 or thereabouts, Derya wrote:
> This reverts commit 117309c51dca42121f70cacec801511b76acf75c.
> 
> The MS Surface Pro 2 has an USB composite device with 3 interfaces
> - interface 0 - sensor hub
> - interface 1 - wacom digitizer
> - interface 2 - the keyboard cover, if one is attached
> This USB composite device changes it product id dependent on if and which
> keyboard cover is attached. Adding the covers to hid_have_special_driver
> prevents loading the right hid drivers for the other two interfaces, all 3
> get loaded with hid-microsoft. We don't even need hid-microsoft for the
> keyboards. We have to revert this to load the right hid modules for each
> interface.
> 
> Signed-off-by: Derya <derya.kiran@yahoo.de>

Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Hoewever, Derya, you should reconsider re-sending the whole series
through "git send-email" or configuring your e-mail client according to
Documentation/email-clients.txt . This series has the lines wrapped and
tabs are converted to spaces, which means that Jiri ca not take the
patch without reformatting them to his tree.

Thanks,
Benjamin

> ---
>  drivers/hid/hid-core.c      | 2 --
>  drivers/hid/hid-ids.h       | 2 --
>  drivers/hid/hid-microsoft.c | 4 ----
>  3 files changed, 8 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index cc32a6f..bb5c494 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1780,8 +1780,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..07cd28c 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -624,8 +624,6 @@
>  #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_TOUCH_COVER_2    0x07a7
> -#define USB_DEVICE_ID_MS_TYPE_COVER_2    0x07a9
> 
>  #define USB_VENDOR_ID_MOJO        0x8282
>  #define USB_DEVICE_ID_RETRO_ADAPTER    0x3201
> 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 },
> -- 
> 1.8.3.2
> 

Re: [PATCH 1/2] Revert "HID: microsoft: Add ID's for Surface Type/Touch, Cover 2"

[Jiri Kosina]

On Mon, 10 Mar 2014, Benjamin Tissoires wrote:

> > The MS Surface Pro 2 has an USB composite device with 3 interfaces
> > - interface 0 - sensor hub
> > - interface 1 - wacom digitizer
> > - interface 2 - the keyboard cover, if one is attached
> > This USB composite device changes it product id dependent on if and which
> > keyboard cover is attached. Adding the covers to hid_have_special_driver
> > prevents loading the right hid drivers for the other two interfaces, all 3
> > get loaded with hid-microsoft. We don't even need hid-microsoft for the
> > keyboards. We have to revert this to load the right hid modules for each
> > interface.
> > 
> > Signed-off-by: Derya <derya.kiran@yahoo.de>
> 
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> Hoewever, Derya, you should reconsider re-sending the whole series
> through "git send-email" or configuring your e-mail client according to
> Documentation/email-clients.txt . This series has the lines wrapped and
> tabs are converted to spaces, which means that Jiri ca not take the
> patch without reformatting them to his tree.

Indeed, the patches have been mangled by your mail client beyond repair.

Could you please resend them (and include Benjamin's Ack), so that I can 
queue them?

Thanks!

-- 
Jiri Kosina
SUSE Labs