Re: [PATCH] hid: topre: Add driver fixing report descriptor

From: Benjamin Tissoires
Date: Tue Sep 20 2022 - 09:36:24 EST


On Sun, Sep 11, 2022 at 2:50 AM Harry Stern <harry@xxxxxxxxxxxxxx> wrote:
>
> The Topre REALFORCE R2 firmware incorrectly reports that interface
> descriptor number 1, input report descriptor 2's events are array events
> rather than variable events. That particular report descriptor is used
> to report keypresses when there are more than 6 keys held at a time.
> This bug prevents events from this interface from being registered
> properly, so only 6 keypresses (from a different interface) can be
> registered at once, rather than full n-key rollover.
>
> This commit fixes the bug by setting the correct value in a report_fixup
> function.
>
> The original bug report can be found here:
> Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/804
>
> Thanks to Benjamin Tissoires for diagnosing the issue with the report
> descriptor.
>
> Signed-off-by: Harry Stern <harry@xxxxxxxxxxxxxx>
> ---

Applied to for-6.1/topre in hid.git

Cheers,
Benjamin

> drivers/hid/Kconfig | 6 +++++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 3 +++
> drivers/hid/hid-topre.c | 49 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 59 insertions(+)
> create mode 100644 drivers/hid/hid-topre.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 6ce92830b..c4308d498 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -1141,6 +1141,12 @@ config HID_TOPSEED
> Say Y if you have a TopSeed Cyberlink or BTC Emprex or Conceptronic
> CLLRCMCE remote control.
>
> +config HID_TOPRE
> + tristate "Topre REALFORCE keyboards"
> + depends on HID
> + help
> + Say Y for N-key rollover support on Topre REALFORCE R2 108 key keyboards.
> +
> config HID_THINGM
> tristate "ThingM blink(1) USB RGB LED"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index b0bef8098..bccaec0d7 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -123,6 +123,7 @@ obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o
> obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o hid-thrustmaster.o
> obj-$(CONFIG_HID_TIVO) += hid-tivo.o
> obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
> +obj-$(CONFIG_HID_TOPRE) += hid-topre.o
> obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
> obj-$(CONFIG_HID_U2FZERO) += hid-u2fzero.o
> hid-uclogic-objs := hid-uclogic-core.o \
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index f80d6193f..50bab12d9 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -1231,6 +1231,9 @@
> #define USB_DEVICE_ID_TIVO_SLIDE 0x1201
> #define USB_DEVICE_ID_TIVO_SLIDE_PRO 0x1203
>
> +#define USB_VENDOR_ID_TOPRE 0x0853
> +#define USB_DEVICE_ID_TOPRE_REALFORCE_R2_108 0x0148
> +
> #define USB_VENDOR_ID_TOPSEED 0x0766
> #define USB_DEVICE_ID_TOPSEED_CYBERLINK 0x0204
>
> diff --git a/drivers/hid/hid-topre.c b/drivers/hid/hid-topre.c
> new file mode 100644
> index 000000000..88a91cdad
> --- /dev/null
> +++ b/drivers/hid/hid-topre.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * HID driver for Topre REALFORCE Keyboards
> + *
> + * Copyright (c) 2022 Harry Stern <harry@xxxxxxxxxxxxxx>
> + *
> + * Based on the hid-macally driver
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +MODULE_AUTHOR("Harry Stern <harry@xxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("REALFORCE R2 Keyboard driver");
> +MODULE_LICENSE("GPL");
> +
> +/*
> + * Fix the REALFORCE R2's non-boot interface's report descriptor to match the
> + * events it's actually sending. It claims to send array events but is instead
> + * sending variable events.
> + */
> +static __u8 *topre_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> + unsigned int *rsize)
> +{
> + if (*rsize >= 119 && rdesc[69] == 0x29 && rdesc[70] == 0xe7 &&
> + rdesc[71] == 0x81 && rdesc[72] == 0x00) {
> + hid_info(hdev,
> + "fixing up Topre REALFORCE keyboard report descriptor\n");
> + rdesc[72] = 0x02;
> + }
> + return rdesc;
> +}
> +
> +static const struct hid_device_id topre_id_table[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_TOPRE,
> + USB_DEVICE_ID_TOPRE_REALFORCE_R2_108) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, topre_id_table);
> +
> +static struct hid_driver topre_driver = {
> + .name = "topre",
> + .id_table = topre_id_table,
> + .report_fixup = topre_report_fixup,
> +};
> +
> +module_hid_driver(topre_driver);
> --
> 2.37.3
>