[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAO-hwJKdfAy9i28iFEKi5DWU0SPOopiEyjT_2HdpL7ahFhdGFg@mail.gmail.com>
Date: Mon, 1 Mar 2021 15:46:11 +0100
From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
To: Filipe Laíns <lains@...hlinux.org>
Cc: Jiri Kosina <jikos@...nel.org>,
"open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
lkml <linux-kernel@...r.kernel.org>,
Filipe Laíns <lains@...eup.net>
Subject: Re: [PATCH 1/2] HID: logitech-dj: add support for the new lightspeed
receiver iteration
On Sat, Jan 23, 2021 at 7:03 PM Filipe Laíns <lains@...hlinux.org> wrote:
>
> From: Filipe Laíns <lains@...eup.net>
>
> Tested with the G Pro X Superlight. libratbag sees the device, as
> expected, and input events are passing trough.
>
> https://github.com/libratbag/libratbag/pull/1122
>
> The receiver has a quirk where the moused interface doesn't have a
> report ID, I am not sure why, perhaps they forgot. All other interfaces
> have report IDs so I am left scratching my head.
> Since this driver doesn't have a quirk system, I simply implemented it
> as a different receiver type, which is true, it just wouldn't be the
> prefered approach :P
>
> Signed-off-by: Filipe Laíns <lains@...eup.net>
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-logitech-dj.c | 49 +++++++++++++++++++++++++----------
> 2 files changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 4c5f23640f9c..8eac3c93fa38 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -803,6 +803,7 @@
> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1 0xc539
> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1 0xc53f
> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY 0xc53a
> +#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2 0xc547
> #define USB_DEVICE_ID_SPACETRAVELLER 0xc623
> #define USB_DEVICE_ID_SPACENAVIGATOR 0xc626
> #define USB_DEVICE_ID_DINOVO_DESKTOP 0xc704
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 1401ee2067ca..6596c81947a8 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -114,6 +114,7 @@ enum recvr_type {
> recvr_type_dj,
> recvr_type_hidpp,
> recvr_type_gaming_hidpp,
> + recvr_type_gaming_hidpp_missing_mse_report_id, /* lightspeed receiver missing the mouse report ID */
> recvr_type_mouse_only,
> recvr_type_27mhz,
> recvr_type_bluetooth,
> @@ -1360,6 +1361,7 @@ static int logi_dj_ll_parse(struct hid_device *hid)
> dbg_hid("%s: sending a mouse descriptor, reports_supported: %llx\n",
> __func__, djdev->reports_supported);
> if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp ||
> + djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp_missing_mse_report_id ||
> djdev->dj_receiver_dev->type == recvr_type_mouse_only)
> rdcat(rdesc, &rsize, mse_high_res_descriptor,
> sizeof(mse_high_res_descriptor));
> @@ -1605,19 +1607,35 @@ static int logi_dj_raw_event(struct hid_device *hdev,
> data[0] = data[1];
> data[1] = 0;
> }
> - /*
> - * Mouse-only receivers send unnumbered mouse data. The 27 MHz
> - * receiver uses 6 byte packets, the nano receiver 8 bytes.
> - */
> - if (djrcv_dev->unnumbered_application == HID_GD_MOUSE &&
> - size <= 8) {
> - u8 mouse_report[9];
> -
> - /* Prepend report id */
> - mouse_report[0] = REPORT_TYPE_MOUSE;
> - memcpy(mouse_report + 1, data, size);
> - logi_dj_recv_forward_input_report(hdev, mouse_report,
> - size + 1);
> +
> +
> + if (djrcv_dev->unnumbered_application == HID_GD_MOUSE) {
> + /*
> + * Mouse-only receivers send unnumbered mouse data. The 27 MHz
> + * receiver uses 6 byte packets, the nano receiver 8 bytes.
> + */
> + if (size <= 8) {
> + u8 mouse_report[9];
Hmm, as stated above, the 27 MHz receiver already does the exact same thing.
Can't we just bump the array size and check above to the following:
if (size <= 16) {
u8 mouse_report[17];
The property "djrcv_dev->unnumbered_application" is based on the
report descriptor entirely, and they are just following the HID norm
here. So I think this should be enough.
Cheers,
Benjamin
> +
> + /* Prepend report id */
> + mouse_report[0] = REPORT_TYPE_MOUSE;
> + memcpy(mouse_report + 1, data, size);
> + logi_dj_recv_forward_input_report(hdev, mouse_report,
> + size + 1);
> +
> + /*
> + * A variant of the ligtpseed receivers is missing the mouse
> + * report ID.
> + */
> + } else if (djrcv_dev->type == recvr_type_gaming_hidpp_missing_mse_report_id) {
> + u8 mouse_report[17];
> +
> + /* Prepend report id */
> + mouse_report[0] = REPORT_TYPE_MOUSE;
> + memcpy(mouse_report + 1, data, size);
> + logi_dj_recv_forward_input_report(hdev, mouse_report,
> + size + 1);
> + }
> }
>
> return false;
> @@ -1688,6 +1706,7 @@ static int logi_dj_probe(struct hid_device *hdev,
> case recvr_type_dj: no_dj_interfaces = 3; break;
> case recvr_type_hidpp: no_dj_interfaces = 2; break;
> case recvr_type_gaming_hidpp: no_dj_interfaces = 3; break;
> + case recvr_type_gaming_hidpp_missing_mse_report_id: no_dj_interfaces = 3; break;
> case recvr_type_mouse_only: no_dj_interfaces = 2; break;
> case recvr_type_27mhz: no_dj_interfaces = 2; break;
> case recvr_type_bluetooth: no_dj_interfaces = 2; break;
> @@ -1886,6 +1905,10 @@ static const struct hid_device_id logi_dj_receivers[] = {
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1),
> .driver_data = recvr_type_gaming_hidpp},
> + { /* Logitech lightspeed receiver (0xc547) */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> + USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2),
> + .driver_data = recvr_type_gaming_hidpp_missing_mse_report_id},
> { /* Logitech 27 MHz HID++ 1.0 receiver (0xc513) */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER),
> .driver_data = recvr_type_27mhz},
> --
> 2.30.0
>
Powered by blists - more mailing lists