linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] HID: Support Varmilo Keyboards' media hotkeys
@ 2020-07-29 13:53 Frank Yang
  2020-08-17 10:03 ` Jiri Kosina
  2020-08-20 18:16 ` [PATCH v2] " Frank Yang
  0 siblings, 2 replies; 5+ messages in thread
From: Frank Yang @ 2020-07-29 13:53 UTC (permalink / raw)
  To: linux-input, Jiri Kosina, Benjamin Tissoires; +Cc: Frank Yang

The Varmilo VA104M Keyboard (04b4:07b1, reported as Varmilo Z104M)
exposes media control hotkeys as a USB HID consumer control device,
but these keys do not work in the current (5.8-rc1) kernel due to
the incorrect HID report descriptor. Fix the problem by modifying
the internal HID report descriptor.

More specifically, the keyboard report descriptor specifies the
logical boundary as 572~10754 (0x023c ~ 0x2a02) while the usage
boundary is specified as 0~10754 (0x00 ~ 0x2a02). This results in an
incorrect interpretation of input reports, causing inputs to be ignored.
By setting the Logical Minimum to zero, we align the logical boundary
with the Usage ID boundary.

Some notes:

* There seem to be multiple variants of the VA104M keyboard. This
  patch specifically targets 04b4:07b1 variant.

* The device works out-of-the-box on Windows platform with the generic
  consumer control device driver (hidserv.inf). This suggests that
  Windows either ignores the Logical Minimum/Logical Maximum or
  interprets the Usage ID assignment differently from the linux
  implementation; Maybe there are other devices out there that only
  works on Windows due to this problem?

Signed-off-by: Frank Yang <puilp0502@gmail.com>
---
 drivers/hid/Kconfig       |  6 ++++
 drivers/hid/Makefile      |  1 +
 drivers/hid/hid-ids.h     |  2 ++
 drivers/hid/hid-varmilo.c | 58 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+)
 create mode 100644 drivers/hid/hid-varmilo.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 443c5cbbde04..c9f0c9b79158 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -441,6 +441,12 @@ config HID_WALTOP
 	---help---
 	Support for Waltop tablets.
 
+config HID_VARMILO
+	tristate "Varmilo Keyboards"
+	depends on HID
+	help
+	  Support for Varmilo keyboards.
+
 config HID_VIEWSONIC
 	tristate "ViewSonic/Signotec"
 	depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index d8ea4b8c95af..e90a98090452 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -124,6 +124,7 @@ obj-$(CONFIG_HID_LED)		+= hid-led.o
 obj-$(CONFIG_HID_XINMO)		+= hid-xinmo.o
 obj-$(CONFIG_HID_ZEROPLUS)	+= hid-zpff.o
 obj-$(CONFIG_HID_ZYDACRON)	+= hid-zydacron.o
+obj-$(CONFIG_HID_VARMILO)	+= hid-varmilo.o
 obj-$(CONFIG_HID_VIEWSONIC)	+= hid-viewsonic.o
 
 wacom-objs			:= wacom_wac.o wacom_sys.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 874fc3791f3b..955be22fc69d 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1189,6 +1189,8 @@
 #define USB_DEVICE_ID_UNITEC_USB_TOUCH_0709	0x0709
 #define USB_DEVICE_ID_UNITEC_USB_TOUCH_0A19	0x0a19
 
+#define USB_DEVICE_ID_VARMILO_VA104M_07B1   0X07b1
+
 #define USB_VENDOR_ID_VELLEMAN		0x10cf
 #define USB_DEVICE_ID_VELLEMAN_K8055_FIRST	0x5500
 #define USB_DEVICE_ID_VELLEMAN_K8055_LAST	0x5503
diff --git a/drivers/hid/hid-varmilo.c b/drivers/hid/hid-varmilo.c
new file mode 100644
index 000000000000..10e50f2dca61
--- /dev/null
+++ b/drivers/hid/hid-varmilo.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *  HID report fixup for varmilo keyboards
+ *
+ *  Copyright (c) 2020 Frank Yang <puilp0502@gmail.com>
+ *
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+/*
+ * Varmilo VA104M with device ID of 07B1 incorrectly reports Logical Minimum as
+ * 572 (0x02 0x3c). We fix this by setting Logical Minimum to zero.
+ */
+static __u8 *varmilo_07b1_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+				       unsigned int *rsize)
+{
+	if (*rsize == 25 &&
+	    rdesc[0] == 0x05 && rdesc[1] == 0x0c &&
+	    rdesc[2] == 0x09 && rdesc[3] == 0x01 &&
+	    rdesc[6] == 0x19 && rdesc[7] == 0x00 &&
+	    rdesc[11] == 0x16 && rdesc[12] == 0x3c && rdesc[13] == 0x02) {
+		hid_info(hdev,
+			 "fixing up varmilo VA104M consumer control report descriptor\n");
+		rdesc[12] = 0x00;
+		rdesc[13] = 0x00;
+	}
+	return rdesc;
+}
+
+static __u8 *varmilo_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+				  unsigned int *rsize)
+{
+	if (hdev->product == USB_DEVICE_ID_VARMILO_VA104M_07B1 &&
+	    hdev->vendor == USB_VENDOR_ID_CYPRESS)
+		rdesc = varmilo_07b1_report_fixup(hdev, rdesc, rsize);
+	return rdesc;
+}
+
+static const struct hid_device_id varmilo_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_VARMILO_VA104M_07B1) },
+	{}
+};
+
+MODULE_DEVICE_TABLE(hid, varmilo_devices);
+
+static struct hid_driver varmilo_driver = {
+	.name = "varmilo",
+	.id_table = varmilo_devices,
+	.report_fixup = varmilo_report_fixup,
+};
+
+module_hid_driver(varmilo_driver);
+
+MODULE_LICENSE("GPL");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] HID: Support Varmilo Keyboards' media hotkeys
  2020-07-29 13:53 [PATCH] HID: Support Varmilo Keyboards' media hotkeys Frank Yang
@ 2020-08-17 10:03 ` Jiri Kosina
  2020-08-20 18:23   ` Frank Yang
  2020-08-20 18:16 ` [PATCH v2] " Frank Yang
  1 sibling, 1 reply; 5+ messages in thread
From: Jiri Kosina @ 2020-08-17 10:03 UTC (permalink / raw)
  To: Frank Yang; +Cc: linux-input, Benjamin Tissoires

On Wed, 29 Jul 2020, Frank Yang wrote:

> The Varmilo VA104M Keyboard (04b4:07b1, reported as Varmilo Z104M)
> exposes media control hotkeys as a USB HID consumer control device,
> but these keys do not work in the current (5.8-rc1) kernel due to
> the incorrect HID report descriptor. Fix the problem by modifying
> the internal HID report descriptor.
> 
> More specifically, the keyboard report descriptor specifies the
> logical boundary as 572~10754 (0x023c ~ 0x2a02) while the usage
> boundary is specified as 0~10754 (0x00 ~ 0x2a02). This results in an
> incorrect interpretation of input reports, causing inputs to be ignored.
> By setting the Logical Minimum to zero, we align the logical boundary
> with the Usage ID boundary.
> 
> Some notes:
> 
> * There seem to be multiple variants of the VA104M keyboard. This
>   patch specifically targets 04b4:07b1 variant.
> 
> * The device works out-of-the-box on Windows platform with the generic
>   consumer control device driver (hidserv.inf). This suggests that
>   Windows either ignores the Logical Minimum/Logical Maximum or
>   interprets the Usage ID assignment differently from the linux
>   implementation; Maybe there are other devices out there that only
>   works on Windows due to this problem?
> 
> Signed-off-by: Frank Yang <puilp0502@gmail.com>
> ---
>  drivers/hid/Kconfig       |  6 ++++
>  drivers/hid/Makefile      |  1 +
>  drivers/hid/hid-ids.h     |  2 ++
>  drivers/hid/hid-varmilo.c | 58 +++++++++++++++++++++++++++++++++++++++

Hi Frank,

thanks for the patch.

Given the fact that the device presents itself with CYPRESS VID (0x04b4, 
'officially' assigned to cypress), can we avoid creating extra driver, and 
rather extend hid-cypress.c with this quirk, please?

Thanks,

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] HID: Support Varmilo Keyboards' media hotkeys
  2020-07-29 13:53 [PATCH] HID: Support Varmilo Keyboards' media hotkeys Frank Yang
  2020-08-17 10:03 ` Jiri Kosina
@ 2020-08-20 18:16 ` Frank Yang
  2020-10-23 11:25   ` Jiri Kosina
  1 sibling, 1 reply; 5+ messages in thread
From: Frank Yang @ 2020-08-20 18:16 UTC (permalink / raw)
  To: linux-input, Jiri Kosina, Benjamin Tissoires; +Cc: Frank Yang

The Varmilo VA104M Keyboard (04b4:07b1, reported as Varmilo Z104M)
exposes media control hotkeys as a USB HID consumer control device, but
these keys do not work in the current (5.8-rc1) kernel due to the
incorrect HID report descriptor. Fix the problem by modifying the
internal HID report descriptor.

More specifically, the keyboard report descriptor specifies the
logical boundary as 572~10754 (0x023c ~ 0x2a02) while the usage
boundary is specified as 0~10754 (0x00 ~ 0x2a02). This results in an
incorrect interpretation of input reports, causing inputs to be ignored.
By setting the Logical Minimum to zero, we align the logical boundary
with the Usage ID boundary.

Some notes:

* There seem to be multiple variants of the VA104M keyboard. This
  patch specifically targets 04b4:07b1 variant.

* The device works out-of-the-box on Windows platform with the generic
  consumer control device driver (hidserv.inf). This suggests that
  Windows either ignores the Logical Minimum/Logical Maximum or
  interprets the Usage ID assignment differently from the linux
  implementation; Maybe there are other devices out there that only
  works on Windows due to this problem?

Signed-off-by: Frank Yang <puilp0502@gmail.com>
---
Changes since v1:
    - Extend hid-cypress.c instead of creating new drivers for varmilo,
      since the device reports the VID of Cypress (0x04b4).
      Suggested by: Jiri Kosina <jikos@kernel.org>

 drivers/hid/hid-cypress.c | 44 ++++++++++++++++++++++++++++++++++-----
 drivers/hid/hid-ids.h     |  2 ++
 2 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-cypress.c b/drivers/hid/hid-cypress.c
index a50ba4a4a1d7..b88f889b3932 100644
--- a/drivers/hid/hid-cypress.c
+++ b/drivers/hid/hid-cypress.c
@@ -23,19 +23,17 @@
 #define CP_2WHEEL_MOUSE_HACK		0x02
 #define CP_2WHEEL_MOUSE_HACK_ON		0x04
 
+#define VA_INVAL_LOGICAL_BOUNDARY	0x08
+
 /*
  * Some USB barcode readers from cypress have usage min and usage max in
  * the wrong order
  */
-static __u8 *cp_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+static __u8 *cp_rdesc_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
 {
-	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
 	unsigned int i;
 
-	if (!(quirks & CP_RDESC_SWAPPED_MIN_MAX))
-		return rdesc;
-
 	if (*rsize < 4)
 		return rdesc;
 
@@ -48,6 +46,40 @@ static __u8 *cp_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	return rdesc;
 }
 
+static __u8 *va_logical_boundary_fixup(struct hid_device *hdev, __u8 *rdesc,
+		unsigned int *rsize)
+{
+	/*
+	 * Varmilo VA104M (with VID Cypress and device ID 07B1) incorrectly
+	 * reports Logical Minimum of its Consumer Control device as 572
+	 * (0x02 0x3c). Fix this by setting its Logical Minimum to zero.
+	 */
+	if (*rsize == 25 &&
+			rdesc[0] == 0x05 && rdesc[1] == 0x0c &&
+			rdesc[2] == 0x09 && rdesc[3] == 0x01 &&
+			rdesc[6] == 0x19 && rdesc[7] == 0x00 &&
+			rdesc[11] == 0x16 && rdesc[12] == 0x3c && rdesc[13] == 0x02) {
+		hid_info(hdev,
+			 "fixing up varmilo VA104M consumer control report descriptor\n");
+		rdesc[12] = 0x00;
+		rdesc[13] = 0x00;
+	}
+	return rdesc;
+}
+
+static __u8 *cp_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+		unsigned int *rsize)
+{
+	unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);
+
+	if (quirks & CP_RDESC_SWAPPED_MIN_MAX)
+		rdesc = cp_rdesc_fixup(hdev, rdesc, rsize);
+	if (quirks & VA_INVAL_LOGICAL_BOUNDARY)
+		rdesc = va_logical_boundary_fixup(hdev, rdesc, rsize);
+
+	return rdesc;
+}
+
 static int cp_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
@@ -128,6 +160,8 @@ static const struct hid_device_id cp_devices[] = {
 		.driver_data = CP_RDESC_SWAPPED_MIN_MAX },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_MOUSE),
 		.driver_data = CP_2WHEEL_MOUSE_HACK },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_VARMILO_VA104M_07B1),
+		.driver_data = VA_INVAL_LOGICAL_BOUNDARY },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, cp_devices);
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 874fc3791f3b..dbca98da8aa5 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -331,6 +331,8 @@
 #define USB_DEVICE_ID_CYPRESS_BARCODE_4	0xed81
 #define USB_DEVICE_ID_CYPRESS_TRUETOUCH	0xc001
 
+#define USB_DEVICE_ID_CYPRESS_VARMILO_VA104M_07B1   0X07b1
+
 #define USB_VENDOR_ID_DATA_MODUL	0x7374
 #define USB_VENDOR_ID_DATA_MODUL_EASYMAXTOUCH	0x1201
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] HID: Support Varmilo Keyboards' media hotkeys
  2020-08-17 10:03 ` Jiri Kosina
@ 2020-08-20 18:23   ` Frank Yang
  0 siblings, 0 replies; 5+ messages in thread
From: Frank Yang @ 2020-08-20 18:23 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, Benjamin Tissoires

On Mon, Aug 17, 2020 at 12:03:26PM +0200, Jiri Kosina wrote:
> On Wed, 29 Jul 2020, Frank Yang wrote:
> 
> > The Varmilo VA104M Keyboard (04b4:07b1, reported as Varmilo Z104M)
> > exposes media control hotkeys as a USB HID consumer control device,
> > but these keys do not work in the current (5.8-rc1) kernel due to
> > the incorrect HID report descriptor. Fix the problem by modifying
> > the internal HID report descriptor.
> > 
> > More specifically, the keyboard report descriptor specifies the
> > logical boundary as 572~10754 (0x023c ~ 0x2a02) while the usage
> > boundary is specified as 0~10754 (0x00 ~ 0x2a02). This results in an
> > incorrect interpretation of input reports, causing inputs to be ignored.
> > By setting the Logical Minimum to zero, we align the logical boundary
> > with the Usage ID boundary.
> > 
> > Some notes:
> > 
> > * There seem to be multiple variants of the VA104M keyboard. This
> >   patch specifically targets 04b4:07b1 variant.
> > 
> > * The device works out-of-the-box on Windows platform with the generic
> >   consumer control device driver (hidserv.inf). This suggests that
> >   Windows either ignores the Logical Minimum/Logical Maximum or
> >   interprets the Usage ID assignment differently from the linux
> >   implementation; Maybe there are other devices out there that only
> >   works on Windows due to this problem?
> > 
> > Signed-off-by: Frank Yang <puilp0502@gmail.com>
> > ---
> >  drivers/hid/Kconfig       |  6 ++++
> >  drivers/hid/Makefile      |  1 +
> >  drivers/hid/hid-ids.h     |  2 ++
> >  drivers/hid/hid-varmilo.c | 58 +++++++++++++++++++++++++++++++++++++++
> 
> Hi Frank,
> 
> thanks for the patch.
> 
> Given the fact that the device presents itself with CYPRESS VID (0x04b4, 
> 'officially' assigned to cypress), can we avoid creating extra driver, and 
> rather extend hid-cypress.c with this quirk, please?
> 
> Thanks,
> 
> -- 
> Jiri Kosina
> SUSE Labs
> 
Hi Jiri,

thanks for your comment.

As you stated, I have sent the updated patch which extends hid-cypress.c 
instead of creating new driver.

Thanks,

Frank


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] HID: Support Varmilo Keyboards' media hotkeys
  2020-08-20 18:16 ` [PATCH v2] " Frank Yang
@ 2020-10-23 11:25   ` Jiri Kosina
  0 siblings, 0 replies; 5+ messages in thread
From: Jiri Kosina @ 2020-10-23 11:25 UTC (permalink / raw)
  To: Frank Yang; +Cc: linux-input, Benjamin Tissoires

On Fri, 21 Aug 2020, Frank Yang wrote:

> The Varmilo VA104M Keyboard (04b4:07b1, reported as Varmilo Z104M)
> exposes media control hotkeys as a USB HID consumer control device, but
> these keys do not work in the current (5.8-rc1) kernel due to the
> incorrect HID report descriptor. Fix the problem by modifying the
> internal HID report descriptor.
> 
> More specifically, the keyboard report descriptor specifies the
> logical boundary as 572~10754 (0x023c ~ 0x2a02) while the usage
> boundary is specified as 0~10754 (0x00 ~ 0x2a02). This results in an
> incorrect interpretation of input reports, causing inputs to be ignored.
> By setting the Logical Minimum to zero, we align the logical boundary
> with the Usage ID boundary.

Thanks, I have now applied the patch.

> Some notes:
> 
> * There seem to be multiple variants of the VA104M keyboard. This
>   patch specifically targets 04b4:07b1 variant.
> 
> * The device works out-of-the-box on Windows platform with the generic
>   consumer control device driver (hidserv.inf). This suggests that
>   Windows either ignores the Logical Minimum/Logical Maximum or
>   interprets the Usage ID assignment differently from the linux
>   implementation; Maybe there are other devices out there that only
>   works on Windows due to this problem?

This is not the first time I came across this issue, but I'd find it 
rather suprising if wondows HID driver would just be completely ignoring 
LogicalMin/LogicalMax values ... perhaps it treats some values as 
specific?

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-10-23 11:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-29 13:53 [PATCH] HID: Support Varmilo Keyboards' media hotkeys Frank Yang
2020-08-17 10:03 ` Jiri Kosina
2020-08-20 18:23   ` Frank Yang
2020-08-20 18:16 ` [PATCH v2] " Frank Yang
2020-10-23 11:25   ` Jiri Kosina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).