From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: "Filipe Laíns" <lains@archlinux.org>
Cc: "Jiri Kosina" <jikos@kernel.org>,
"open list:HID CORE LAYER" <linux-input@vger.kernel.org>,
lkml <linux-kernel@vger.kernel.org>,
"Filipe Laíns" <lains@riseup.net>
Subject: Re: [PATCH 1/2] HID: logitech-dj: add support for the new lightspeed receiver iteration
Date: Mon, 13 Dec 2021 12:28:16 +0100 [thread overview]
Message-ID: <b9ca5d3b-abda-5195-4c17-fa3b49d37334@redhat.com> (raw)
In-Reply-To: <CAO-hwJKdfAy9i28iFEKi5DWU0SPOopiEyjT_2HdpL7ahFhdGFg@mail.gmail.com>
Hi Filipe,
On Mon, Mar 1, 2021 at 3:46 PM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
>
> On Sat, Jan 23, 2021 at 7:03 PM Filipe Laíns <lains@archlinux.org> wrote:
> >
> > From: Filipe Laíns <lains@riseup.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@riseup.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.
I've been pinged last week about the G Pro X superlight, and Peter found
that this patch never got into upstream.
AFAICT, compared to upstream the following code change should have the
same effect that what you proposed here:
---
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 7106b921b53c..f5cdfce272e7 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1691,11 +1691,12 @@ static int logi_dj_raw_event(struct hid_device *hdev,
}
/*
* Mouse-only receivers send unnumbered mouse data. The 27 MHz
- * receiver uses 6 byte packets, the nano receiver 8 bytes.
+ * receiver uses 6 byte packets, the nano receiver 8 bytes and
+ * some gaming ones are using 16 bytes.
*/
if (djrcv_dev->unnumbered_application == HID_GD_MOUSE &&
- size <= 8) {
- u8 mouse_report[9];
+ size <= 16) {
+ u8 mouse_report[17];
/* Prepend report id */
mouse_report[0] = REPORT_TYPE_MOUSE;
---
Would you mind sending a v2 of the patch updated to the current for-next so we can schedule this for 5.17?
Cheers,
Benjamin
>
> 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
> >
prev parent reply other threads:[~2021-12-13 11:28 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-23 18:03 [PATCH 1/2] HID: logitech-dj: add support for the new lightspeed receiver iteration Filipe Laíns
2021-01-23 18:03 ` [PATCH 2/2] HID: logitech-hidpp: add support for the G Pro X Superlight over USB Filipe Laíns
2021-02-05 15:14 ` [PATCH 1/2] HID: logitech-dj: add support for the new lightspeed receiver iteration Jiri Kosina
2021-02-26 14:08 ` Filipe Laíns
2021-03-01 14:46 ` Benjamin Tissoires
2021-12-13 11:28 ` Benjamin Tissoires [this message]
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=b9ca5d3b-abda-5195-4c17-fa3b49d37334@redhat.com \
--to=benjamin.tissoires@redhat.com \
--cc=jikos@kernel.org \
--cc=lains@archlinux.org \
--cc=lains@riseup.net \
--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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.