diff mbox

keyboard not working with Thinkpad Helix (2nd gen)

Message ID 553FF022.6050708@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Benjamin Tissoires April 28, 2015, 8:40 p.m. UTC
Hi,

On 04/28/2015 02:46 PM, John Reid wrote:
> Hi all,
> 
> To me the following page in the keyboard interface 0 descriptor (from
> "sudo usbhid-dump -a1:8 -i0| grep -v : | xxd -r -p | hidrd-convert -o")
> seems suspicious. I'm guessing Logical Maximum should not be -1 if
> Logical Minimum is 0, and a Usage Maximum of FFFFh might break
> include/linux/hid.h #define HID_MAX_USAGES    12288

Yep, that's what prevents the hid subsystem to handle the keyboard
(though the logical min has nothing to do here).
Below is a patch that fixes the report descriptor and will hopefully
make the keyboard working.
Can you apply it on top of Jiri's tree? (or you might want to cherry
pick https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-4.2/lenovo&id=d92189ebbdcd0eb180317d8cd6d46c57ac9a3dc0
on top of a clean v4.0)

And then, tell us if hid-lenovo handles the keyboard?

> 
> Usage Page (88h),                   ; 88h, reserved
> Usage (01h),
> Collection (Application),
>      Report ID (4),
>      Usage Minimum (00h),
>      Usage Maximum (FFFFh),
>      Logical Minimum (0),
>      Logical Maximum (-1),
>      Report Size (8),
>      Report Count (2),
>      Input (Variable),
> End Collection,
> 
> Cheers,
> John
> 

FWIW, I'll track down the other problems in the other threads.

Cheers,
Benjamin


From abc7922e4972910a31eb54c673bc3b261fd919f7 Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date: Tue, 28 Apr 2015 16:31:56 -0400
Subject: [PATCH] HID: lenovo: add support for Lenovo ThinkPad Keyboard Pro
 unit

This dock is used with the Thinkpad Helix 2 but suffers from an error
in the report descriptor where an usage max is 65535.

Add a report fixup for it and make the keyboard working.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-core.c   |  1 +
 drivers/hid/hid-ids.h    |  1 +
 drivers/hid/hid-lenovo.c | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+)

Comments

jono April 30, 2015, 8:42 p.m. UTC | #1
I'm writing this email from the Helix keyboard! The stylus work as
well, although users might need to remap the eraser tip (imagine this
is an Xorg or Xournal issue rather than a kernel one). Thanks so much
Benjamin and the linux-input crew! I've fallen in love with linux all
over again. Really appreciate all your great work.
Jonathan

On Tue, Apr 28, 2015 at 9:40 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> Hi,
>
> On 04/28/2015 02:46 PM, John Reid wrote:
>> Hi all,
>>
>> To me the following page in the keyboard interface 0 descriptor (from
>> "sudo usbhid-dump -a1:8 -i0| grep -v : | xxd -r -p | hidrd-convert -o")
>> seems suspicious. I'm guessing Logical Maximum should not be -1 if
>> Logical Minimum is 0, and a Usage Maximum of FFFFh might break
>> include/linux/hid.h #define HID_MAX_USAGES    12288
>
> Yep, that's what prevents the hid subsystem to handle the keyboard
> (though the logical min has nothing to do here).
> Below is a patch that fixes the report descriptor and will hopefully
> make the keyboard working.
> Can you apply it on top of Jiri's tree? (or you might want to cherry
> pick https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/commit/?h=for-4.2/lenovo&id=d92189ebbdcd0eb180317d8cd6d46c57ac9a3dc0
> on top of a clean v4.0)
>
> And then, tell us if hid-lenovo handles the keyboard?
>
>>
>> Usage Page (88h),                   ; 88h, reserved
>> Usage (01h),
>> Collection (Application),
>>      Report ID (4),
>>      Usage Minimum (00h),
>>      Usage Maximum (FFFFh),
>>      Logical Minimum (0),
>>      Logical Maximum (-1),
>>      Report Size (8),
>>      Report Count (2),
>>      Input (Variable),
>> End Collection,
>>
>> Cheers,
>> John
>>
>
> FWIW, I'll track down the other problems in the other threads.
>
> Cheers,
> Benjamin
>
>
> From abc7922e4972910a31eb54c673bc3b261fd919f7 Mon Sep 17 00:00:00 2001
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Date: Tue, 28 Apr 2015 16:31:56 -0400
> Subject: [PATCH] HID: lenovo: add support for Lenovo ThinkPad Keyboard Pro
>  unit
>
> This dock is used with the Thinkpad Helix 2 but suffers from an error
> in the report descriptor where an usage max is 65535.
>
> Add a report fixup for it and make the keyboard working.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>  drivers/hid/hid-core.c   |  1 +
>  drivers/hid/hid-ids.h    |  1 +
>  drivers/hid/hid-lenovo.c | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 34 insertions(+)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 722a925..c2baf8c 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1851,6 +1851,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
>         { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
>         { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPPRODOCK) },
>  #endif
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b24caf8..e5b86e0 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -585,6 +585,7 @@
>  #define USB_DEVICE_ID_LENOVO_TPKBD     0x6009
>  #define USB_DEVICE_ID_LENOVO_CUSBKBD   0x6047
>  #define USB_DEVICE_ID_LENOVO_CBTKBD    0x6048
> +#define USB_DEVICE_ID_LENOVO_TPPRODOCK 0x6067
>
>  #define USB_VENDOR_ID_LG               0x1fd2
>  #define USB_DEVICE_ID_LG_MULTITOUCH    0x0064
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 78608d6..c6c81ff 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -43,6 +43,36 @@ struct lenovo_drvdata_cptkbd {
>
>  #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
>
> +static const __u8 lenovo_pro_dock_need_fixup_collection[] = {
> +       0x05, 0x88,             /* Usage Page (Vendor Usage Page 0x88)  */
> +       0x09, 0x01,             /* Usage (Vendor Usage 0x01)            */
> +       0xa1, 0x01,             /* Collection (Application)             */
> +       0x85, 0x04,             /*  Report ID (4)                       */
> +       0x19, 0x00,             /*  Usage Minimum (0)                   */
> +       0x2a, 0xff, 0xff,       /*  Usage Maximum (65535)               */
> +};
> +
> +static __u8 *lenovo_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> +               unsigned int *rsize)
> +{
> +       switch (hdev->product) {
> +       case USB_DEVICE_ID_LENOVO_TPPRODOCK:
> +               /* the fixups that need to be done:
> +                *   - get a reasonable usage max for the vendor collection
> +                *     0x8801 from the report ID 4
> +                */
> +               pr_err("%s %14ph %s:%d\n", __func__, &rdesc[140], __FILE__, __LINE__);
> +               if (*rsize >= 153 &&
> +                   memcmp(&rdesc[140], lenovo_pro_dock_need_fixup_collection,
> +                         sizeof(lenovo_pro_dock_need_fixup_collection)) == 0) {
> +                       rdesc[151] = 0x01;
> +                       rdesc[152] = 0x00;
> +               }
> +               break;
> +       }
> +       return rdesc;
> +}
> +
>  static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
>                 struct hid_input *hi, struct hid_field *field,
>                 struct hid_usage *usage, unsigned long **bit, int *max)
> @@ -784,6 +814,7 @@ static const struct hid_device_id lenovo_devices[] = {
>         { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
>         { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPPRODOCK) },
>         { }
>  };
>
> @@ -797,6 +828,7 @@ static struct hid_driver lenovo_driver = {
>         .probe = lenovo_probe,
>         .remove = lenovo_remove,
>         .raw_event = lenovo_raw_event,
> +       .report_fixup = lenovo_report_fixup,
>  };
>  module_hid_driver(lenovo_driver);
>
> --
> 2.3.5
--
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
diff mbox

Patch

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 722a925..c2baf8c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1851,6 +1851,7 @@  static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPPRODOCK) },
 #endif
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b24caf8..e5b86e0 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -585,6 +585,7 @@ 
 #define USB_DEVICE_ID_LENOVO_TPKBD	0x6009
 #define USB_DEVICE_ID_LENOVO_CUSBKBD	0x6047
 #define USB_DEVICE_ID_LENOVO_CBTKBD	0x6048
+#define USB_DEVICE_ID_LENOVO_TPPRODOCK	0x6067
 
 #define USB_VENDOR_ID_LG		0x1fd2
 #define USB_DEVICE_ID_LG_MULTITOUCH	0x0064
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 78608d6..c6c81ff 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -43,6 +43,36 @@  struct lenovo_drvdata_cptkbd {
 
 #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
 
+static const __u8 lenovo_pro_dock_need_fixup_collection[] = {
+	0x05, 0x88,		/* Usage Page (Vendor Usage Page 0x88)	*/
+	0x09, 0x01,		/* Usage (Vendor Usage 0x01)		*/
+	0xa1, 0x01,		/* Collection (Application)		*/
+	0x85, 0x04,		/*  Report ID (4)			*/
+	0x19, 0x00,		/*  Usage Minimum (0)			*/
+	0x2a, 0xff, 0xff,	/*  Usage Maximum (65535)		*/
+};
+
+static __u8 *lenovo_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+		unsigned int *rsize)
+{
+	switch (hdev->product) {
+	case USB_DEVICE_ID_LENOVO_TPPRODOCK:
+		/* the fixups that need to be done:
+		 *   - get a reasonable usage max for the vendor collection
+		 *     0x8801 from the report ID 4
+		 */
+		pr_err("%s %14ph %s:%d\n", __func__, &rdesc[140], __FILE__, __LINE__);
+		if (*rsize >= 153 &&
+		    memcmp(&rdesc[140], lenovo_pro_dock_need_fixup_collection,
+			  sizeof(lenovo_pro_dock_need_fixup_collection)) == 0) {
+			rdesc[151] = 0x01;
+			rdesc[152] = 0x00;
+		}
+		break;
+	}
+	return rdesc;
+}
+
 static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
 		struct hid_input *hi, struct hid_field *field,
 		struct hid_usage *usage, unsigned long **bit, int *max)
@@ -784,6 +814,7 @@  static const struct hid_device_id lenovo_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPPRODOCK) },
 	{ }
 };
 
@@ -797,6 +828,7 @@  static struct hid_driver lenovo_driver = {
 	.probe = lenovo_probe,
 	.remove = lenovo_remove,
 	.raw_event = lenovo_raw_event,
+	.report_fixup = lenovo_report_fixup,
 };
 module_hid_driver(lenovo_driver);