| Message ID | 20180907103450.13890-7-benjamin.tissoires@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | HID: logitech-hidpp: support non DJ devices | expand |
Hi Benjamin, On Fri, 7 Sep 2018 at 03:35, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > The G700 is using a non unifying receiver, so it's easy to add its support > in hid-logitech-hidpp now. > [snip] > @@ -3671,6 +3671,9 @@ static const struct hid_device_id hidpp_devices[] = { > { /* Solar Keyboard Logitech K750 */ > LDJ_DEVICE(0x4002), > .driver_data = HIDPP_QUIRK_CLASS_K750 }, > + { /* G700 over Wireless */ > + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G700_RECEIVER), > + .driver_data = HIDPP_QUIRK_RECEIVER | HIDPP_QUIRK_UNIFYING }, As someone who's new to the codebase, it seems rather confusing to me that HIDPP_QUIRK_UNIFYING would be present here for a device that doesn't use a Unifying receiver. Am I misunderstanding, or should we consider renaming the quirk or adding some clarifying comment? (Similarly for the G900 in the next patch.) > > { LDJ_DEVICE(HID_ANY_ID) }, > > -- > 2.14.3 > Thanks, Harry Cutts Chrome OS Touch/Input team
Hi Harry, [now that the series has been reverted and re-inserted, I am starting to have a look at this again] On Fri, Sep 7, 2018 at 7:33 PM Harry Cutts <hcutts@chromium.org> wrote: > > Hi Benjamin, > > On Fri, 7 Sep 2018 at 03:35, Benjamin Tissoires > <benjamin.tissoires@redhat.com> wrote: > > > > The G700 is using a non unifying receiver, so it's easy to add its support > > in hid-logitech-hidpp now. > > [snip] > > @@ -3671,6 +3671,9 @@ static const struct hid_device_id hidpp_devices[] = { > > { /* Solar Keyboard Logitech K750 */ > > LDJ_DEVICE(0x4002), > > .driver_data = HIDPP_QUIRK_CLASS_K750 }, > > + { /* G700 over Wireless */ > > + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G700_RECEIVER), > > + .driver_data = HIDPP_QUIRK_RECEIVER | HIDPP_QUIRK_UNIFYING }, > > As someone who's new to the codebase, it seems rather confusing to me > that HIDPP_QUIRK_UNIFYING would be present here for a device that > doesn't use a Unifying receiver. Am I misunderstanding, or should we > consider renaming the quirk or adding some clarifying comment? > (Similarly for the G900 in the next patch.) The initial reason is that the gaming receivers are Unifying but that are not normally allowed to connect to more that one device at a time. The reason is that those receiver are using a higher frequency and need all of the bandwidth to operate (so they can't multiplex the devices). But as I re-read the series, it is clear that HIDPP_QUIRK_RECEIVER and HIDPP_QUIRK_UNIFYING are never used separately, so it doesn't make sense to have 2 quirks. We should probably rename UNIFYING into RECEIVER for consistency and we will be good. Cheers, Benjamin > > > > > { LDJ_DEVICE(HID_ANY_ID) }, > > > > -- > > 2.14.3 > > > > Thanks, > > Harry Cutts > Chrome OS Touch/Input team
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 537223086d6b..b366976e928c 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -742,6 +742,7 @@ #define USB_DEVICE_ID_MX3000_RECEIVER 0xc513 #define USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER 0xc52b #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER 0xc52f +#define USB_DEVICE_ID_LOGITECH_G700_RECEIVER 0xc531 #define USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER_2 0xc532 #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_2 0xc534 #define USB_DEVICE_ID_SPACETRAVELLER 0xc623 diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index a09509fdb746..0ab7f8b66b83 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -3671,6 +3671,9 @@ static const struct hid_device_id hidpp_devices[] = { { /* Solar Keyboard Logitech K750 */ LDJ_DEVICE(0x4002), .driver_data = HIDPP_QUIRK_CLASS_K750 }, + { /* G700 over Wireless */ + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G700_RECEIVER), + .driver_data = HIDPP_QUIRK_RECEIVER | HIDPP_QUIRK_UNIFYING }, { LDJ_DEVICE(HID_ANY_ID) },
The G700 is using a non unifying receiver, so it's easy to add its support in hid-logitech-hidpp now. With this, the 2 input nodes attached to the mouse will be enumerated as "Logitech G700" and the battery power_supply will be exported (HID++ 1.0 battery type). Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/hid-ids.h | 1 + drivers/hid/hid-logitech-hidpp.c | 3 +++ 2 files changed, 4 insertions(+)