Linux Input Archive on lore.kernel.org
 help / color / Atom feed
From: Frank Yang <puilp0502@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	linux-input@vger.kernel.org
Subject: Re: [PATCH] HID: Support Varmilo Keyboards' media hotkeys
Date: Tue, 21 Jul 2020 05:24:34 +0900
Message-ID: <CA+rSvc7goy-h-Hu7MruaryngBs39_8oE1AijFAnybuDSiSRHhw@mail.gmail.com> (raw)
In-Reply-To: <20200720200916.31082-1-puilp0502@gmail.com>

On Tue, 2020-07-21 at 05:10 +0900, Frank Yang <puilp0502@gmail.com> 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 +++++++++++++++++++++++++++++++++++++++
>  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
>
CCing linux-input mailing list.

Cc: linux-input@vger.kernel.org

       reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200720200916.31082-1-puilp0502@gmail.com>
2020-07-20 20:24 ` Frank Yang [this message]
2020-07-29 13:53 Frank Yang
2020-08-17 10:03 ` Jiri Kosina
2020-08-20 18:23   ` Frank Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+rSvc7goy-h-Hu7MruaryngBs39_8oE1AijFAnybuDSiSRHhw@mail.gmail.com \
    --to=puilp0502@gmail.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux Input Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-input/0 linux-input/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-input linux-input/ https://lore.kernel.org/linux-input \
		linux-input@vger.kernel.org
	public-inbox-index linux-input

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-input


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git