| Message ID | 20190410145459.11430-17-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | HID: logitech: Handling of non DJ receivers in hid-logitech-dj | expand |
On Wed, Apr 10, 2019 at 4:55 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Most Logitech wireless keyboard and mice using the 27 MHz are hidpp10 > devices, add support to logitech-dj for their receivers. > > Doing so leads to 2 improvements: > > 1) All these devices share the same USB product-id for their receiver, > making it impossible to properly map some special keys / buttons > which differ from device to device. Adding support to logitech-dj to > see these as hidpp10 devices allows us to get the actual device-id > from the keyboard / mouse. > > 2) It enables battery-monitoring of these devices > > This patch uses a new HID group for 27Mhz devices, since the logitech-hidpp > code needs to be able to differentiate them from other devices instantiated > by the logitech-dj code. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > -Remove chunk to pick a better name for hidpp devices, this is replaced > with a more generic fix in a separate patch > -Add support for mouse on index 2, numeric-keypad on index 4 > -Use a new group for 27MHz, so that logitech-hidpp.c can check for this to > enable 27MHz specific behavior, rather then have it guess based on the > descriptors. This also allows removing some 27MHz descriptor hacks from > logitech-dj.c, so it simplifies things on both ends > --- > drivers/hid/hid-lg.c | 2 - > drivers/hid/hid-logitech-dj.c | 94 ++++++++++++++++++++++++++++++++++- > drivers/hid/hid-quirks.c | 1 - > include/linux/hid.h | 1 + > 4 files changed, 94 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c > index 5d419a95b6c2..36d725fdb199 100644 > --- a/drivers/hid/hid-lg.c > +++ b/drivers/hid/hid-lg.c > @@ -876,8 +876,6 @@ static const struct hid_device_id lg_devices[] = { > .driver_data = LG_RDESC | LG_WIRELESS }, > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER), > .driver_data = LG_RDESC | LG_WIRELESS }, > - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2), > - .driver_data = LG_RDESC | LG_WIRELESS }, > > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER), > .driver_data = LG_BAD_RELATIVE_KEYS }, > diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c > index a994d2e3dd9e..9d5db242e326 100644 > --- a/drivers/hid/hid-logitech-dj.c > +++ b/drivers/hid/hid-logitech-dj.c > @@ -106,6 +106,7 @@ > #define HIDPP_PARAM_DEVICE_INFO 0x01 > #define HIDPP_PARAM_EQUAD_LSB 0x02 > #define HIDPP_PARAM_EQUAD_MSB 0x03 > +#define HIDPP_PARAM_27MHZ_DEVID 0x03 > #define HIDPP_DEVICE_TYPE_MASK GENMASK(3, 0) > #define HIDPP_LINK_STATUS_MASK BIT(6) > > @@ -120,6 +121,7 @@ enum recvr_type { > recvr_type_dj, > recvr_type_hidpp, > recvr_type_gaming_hidpp, > + recvr_type_27mhz, > }; > > struct dj_report { > @@ -248,6 +250,44 @@ static const char mse_descriptor[] = { > 0xC0, /* END_COLLECTION */ > }; > > +/* Mouse descriptor (2) for 27 MHz receiver, only 8 buttons */ > +static const char mse_27mhz_descriptor[] = { > + 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ > + 0x09, 0x02, /* USAGE (Mouse) */ > + 0xA1, 0x01, /* COLLECTION (Application) */ > + 0x85, 0x02, /* REPORT_ID = 2 */ > + 0x09, 0x01, /* USAGE (pointer) */ > + 0xA1, 0x00, /* COLLECTION (physical) */ > + 0x05, 0x09, /* USAGE_PAGE (buttons) */ > + 0x19, 0x01, /* USAGE_MIN (1) */ > + 0x29, 0x08, /* USAGE_MAX (8) */ > + 0x15, 0x00, /* LOGICAL_MIN (0) */ > + 0x25, 0x01, /* LOGICAL_MAX (1) */ > + 0x95, 0x08, /* REPORT_COUNT (8) */ > + 0x75, 0x01, /* REPORT_SIZE (1) */ > + 0x81, 0x02, /* INPUT (data var abs) */ > + 0x05, 0x01, /* USAGE_PAGE (generic desktop) */ > + 0x16, 0x01, 0xF8, /* LOGICAL_MIN (-2047) */ > + 0x26, 0xFF, 0x07, /* LOGICAL_MAX (2047) */ > + 0x75, 0x0C, /* REPORT_SIZE (12) */ > + 0x95, 0x02, /* REPORT_COUNT (2) */ > + 0x09, 0x30, /* USAGE (X) */ > + 0x09, 0x31, /* USAGE (Y) */ > + 0x81, 0x06, /* INPUT */ > + 0x15, 0x81, /* LOGICAL_MIN (-127) */ > + 0x25, 0x7F, /* LOGICAL_MAX (127) */ > + 0x75, 0x08, /* REPORT_SIZE (8) */ > + 0x95, 0x01, /* REPORT_COUNT (1) */ > + 0x09, 0x38, /* USAGE (wheel) */ > + 0x81, 0x06, /* INPUT */ > + 0x05, 0x0C, /* USAGE_PAGE(consumer) */ > + 0x0A, 0x38, 0x02, /* USAGE(AC Pan) */ > + 0x95, 0x01, /* REPORT_COUNT (1) */ > + 0x81, 0x06, /* INPUT */ > + 0xC0, /* END_COLLECTION */ > + 0xC0, /* END_COLLECTION */ > +}; > + > /* Gaming Mouse descriptor (2) */ > static const char mse_high_res_descriptor[] = { > 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ > @@ -596,7 +636,10 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, > "Logitech Unifying Device. Wireless PID:%04x", > dj_hiddev->product); > > - dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE; > + if (djrcv_dev->type == recvr_type_27mhz) > + dj_hiddev->group = HID_GROUP_LOGITECH_27MHZ_DEVICE; > + else > + dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE; > > memcpy(dj_hiddev->phys, djrcv_hdev->phys, sizeof(djrcv_hdev->phys)); > snprintf(tmpstr, sizeof(tmpstr), ":%d", device_index); > @@ -782,6 +825,28 @@ static void logi_hidpp_dev_conn_notif_equad(struct hidpp_event *hidpp_report, > } > } > > +static void logi_hidpp_dev_conn_notif_27mhz(struct hid_device *hdev, > + struct hidpp_event *hidpp_report, > + struct dj_workitem *workitem) > +{ > + workitem->type = WORKITEM_TYPE_PAIRED; > + workitem->quad_id_lsb = hidpp_report->params[HIDPP_PARAM_27MHZ_DEVID]; > + switch (hidpp_report->device_index) { > + case 1: /* Index 1 is always a mouse */ IIRC we are supposed to mark the fall through statements as such. Not sure why checkpatch doesn't complain here. > + case 2: /* Index 2 is always a mouse */ > + workitem->reports_supported |= STD_MOUSE; > + break; > + case 3: /* Index 3 is always the keyboard */ Same here, please mark this as a fall-through. Cheers, Benjamin > + case 4: /* Index 4 is used for an optional separate numpad */ > + workitem->reports_supported |= STD_KEYBOARD | MULTIMEDIA | > + POWER_KEYS; > + break; > + default: > + hid_warn(hdev, "%s: unexpected device-index %d", __func__, > + hidpp_report->device_index); > + } > +} > + > static void logi_hidpp_recv_queue_notif(struct hid_device *hdev, > struct hidpp_event *hidpp_report) > { > @@ -799,6 +864,7 @@ static void logi_hidpp_recv_queue_notif(struct hid_device *hdev, > break; > case 0x02: > device_type = "27 Mhz"; > + logi_hidpp_dev_conn_notif_27mhz(hdev, hidpp_report, &workitem); > break; > case 0x03: > device_type = "QUAD or eQUAD"; > @@ -1186,6 +1252,9 @@ static int logi_dj_ll_parse(struct hid_device *hid) > if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp) > rdcat(rdesc, &rsize, mse_high_res_descriptor, > sizeof(mse_high_res_descriptor)); > + else if (djdev->dj_receiver_dev->type == recvr_type_27mhz) > + rdcat(rdesc, &rsize, mse_27mhz_descriptor, > + sizeof(mse_27mhz_descriptor)); > else > rdcat(rdesc, &rsize, mse_descriptor, > sizeof(mse_descriptor)); > @@ -1357,6 +1426,25 @@ static int logi_dj_hidpp_event(struct hid_device *hdev, > spin_lock_irqsave(&djrcv_dev->lock, flags); > > dj_dev = djrcv_dev->paired_dj_devices[device_index]; > + > + /* > + * With 27 MHz receivers, we do not get an explicit unpair event, > + * remove the old device if the user has paired a *different* device. > + */ > + if (djrcv_dev->type == recvr_type_27mhz && dj_dev && > + hidpp_report->sub_id == REPORT_TYPE_NOTIF_DEVICE_CONNECTED && > + hidpp_report->params[HIDPP_PARAM_PROTO_TYPE] == 0x02 && > + hidpp_report->params[HIDPP_PARAM_27MHZ_DEVID] != > + dj_dev->hdev->product) { > + struct dj_workitem workitem = { > + .device_index = hidpp_report->device_index, > + .type = WORKITEM_TYPE_UNPAIRED, > + }; > + kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem)); > + /* logi_hidpp_recv_queue_notif will queue the work */ > + dj_dev = NULL; > + } > + > if (dj_dev) { > logi_dj_recv_forward_report(dj_dev, data, size); > } else { > @@ -1624,6 +1712,10 @@ static const struct hid_device_id logi_dj_receivers[] = { > HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, > USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_GAMING), > .driver_data = recvr_type_gaming_hidpp}, > + { /* Logitech 27 MHz HID++ 1.0 receiver (0xc517) */ > + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, > + USB_DEVICE_ID_S510_RECEIVER_2), > + .driver_data = recvr_type_27mhz}, > {} > }; > > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c > index 56559f2b8334..5decf16aeb4b 100644 > --- a/drivers/hid/hid-quirks.c > +++ b/drivers/hid/hid-quirks.c > @@ -432,7 +432,6 @@ static const struct hid_device_id hid_have_special_driver[] = { > #if IS_ENABLED(CONFIG_HID_LOGITECH) > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) }, > - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) }, > diff --git a/include/linux/hid.h b/include/linux/hid.h > index f9707d1dcb58..9f161fa5cbd4 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -382,6 +382,7 @@ struct hid_item { > #define HID_GROUP_WACOM 0x0101 > #define HID_GROUP_LOGITECH_DJ_DEVICE 0x0102 > #define HID_GROUP_STEAM 0x0103 > +#define HID_GROUP_LOGITECH_27MHZ_DEVICE 0x0104 > > /* > * HID protocol status > -- > 2.21.0 >
Hi, On 4/19/19 5:54 PM, Benjamin Tissoires wrote: > On Wed, Apr 10, 2019 at 4:55 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> Most Logitech wireless keyboard and mice using the 27 MHz are hidpp10 >> devices, add support to logitech-dj for their receivers. >> >> Doing so leads to 2 improvements: >> >> 1) All these devices share the same USB product-id for their receiver, >> making it impossible to properly map some special keys / buttons >> which differ from device to device. Adding support to logitech-dj to >> see these as hidpp10 devices allows us to get the actual device-id >> from the keyboard / mouse. >> >> 2) It enables battery-monitoring of these devices >> >> This patch uses a new HID group for 27Mhz devices, since the logitech-hidpp >> code needs to be able to differentiate them from other devices instantiated >> by the logitech-dj code. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v2: >> -Remove chunk to pick a better name for hidpp devices, this is replaced >> with a more generic fix in a separate patch >> -Add support for mouse on index 2, numeric-keypad on index 4 >> -Use a new group for 27MHz, so that logitech-hidpp.c can check for this to >> enable 27MHz specific behavior, rather then have it guess based on the >> descriptors. This also allows removing some 27MHz descriptor hacks from >> logitech-dj.c, so it simplifies things on both ends >> --- >> drivers/hid/hid-lg.c | 2 - >> drivers/hid/hid-logitech-dj.c | 94 ++++++++++++++++++++++++++++++++++- >> drivers/hid/hid-quirks.c | 1 - >> include/linux/hid.h | 1 + >> 4 files changed, 94 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c >> index 5d419a95b6c2..36d725fdb199 100644 >> --- a/drivers/hid/hid-lg.c >> +++ b/drivers/hid/hid-lg.c >> @@ -876,8 +876,6 @@ static const struct hid_device_id lg_devices[] = { >> .driver_data = LG_RDESC | LG_WIRELESS }, >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER), >> .driver_data = LG_RDESC | LG_WIRELESS }, >> - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2), >> - .driver_data = LG_RDESC | LG_WIRELESS }, >> >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER), >> .driver_data = LG_BAD_RELATIVE_KEYS }, >> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c >> index a994d2e3dd9e..9d5db242e326 100644 >> --- a/drivers/hid/hid-logitech-dj.c >> +++ b/drivers/hid/hid-logitech-dj.c >> @@ -106,6 +106,7 @@ >> #define HIDPP_PARAM_DEVICE_INFO 0x01 >> #define HIDPP_PARAM_EQUAD_LSB 0x02 >> #define HIDPP_PARAM_EQUAD_MSB 0x03 >> +#define HIDPP_PARAM_27MHZ_DEVID 0x03 >> #define HIDPP_DEVICE_TYPE_MASK GENMASK(3, 0) >> #define HIDPP_LINK_STATUS_MASK BIT(6) >> >> @@ -120,6 +121,7 @@ enum recvr_type { >> recvr_type_dj, >> recvr_type_hidpp, >> recvr_type_gaming_hidpp, >> + recvr_type_27mhz, >> }; >> >> struct dj_report { >> @@ -248,6 +250,44 @@ static const char mse_descriptor[] = { >> 0xC0, /* END_COLLECTION */ >> }; >> >> +/* Mouse descriptor (2) for 27 MHz receiver, only 8 buttons */ >> +static const char mse_27mhz_descriptor[] = { >> + 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ >> + 0x09, 0x02, /* USAGE (Mouse) */ >> + 0xA1, 0x01, /* COLLECTION (Application) */ >> + 0x85, 0x02, /* REPORT_ID = 2 */ >> + 0x09, 0x01, /* USAGE (pointer) */ >> + 0xA1, 0x00, /* COLLECTION (physical) */ >> + 0x05, 0x09, /* USAGE_PAGE (buttons) */ >> + 0x19, 0x01, /* USAGE_MIN (1) */ >> + 0x29, 0x08, /* USAGE_MAX (8) */ >> + 0x15, 0x00, /* LOGICAL_MIN (0) */ >> + 0x25, 0x01, /* LOGICAL_MAX (1) */ >> + 0x95, 0x08, /* REPORT_COUNT (8) */ >> + 0x75, 0x01, /* REPORT_SIZE (1) */ >> + 0x81, 0x02, /* INPUT (data var abs) */ >> + 0x05, 0x01, /* USAGE_PAGE (generic desktop) */ >> + 0x16, 0x01, 0xF8, /* LOGICAL_MIN (-2047) */ >> + 0x26, 0xFF, 0x07, /* LOGICAL_MAX (2047) */ >> + 0x75, 0x0C, /* REPORT_SIZE (12) */ >> + 0x95, 0x02, /* REPORT_COUNT (2) */ >> + 0x09, 0x30, /* USAGE (X) */ >> + 0x09, 0x31, /* USAGE (Y) */ >> + 0x81, 0x06, /* INPUT */ >> + 0x15, 0x81, /* LOGICAL_MIN (-127) */ >> + 0x25, 0x7F, /* LOGICAL_MAX (127) */ >> + 0x75, 0x08, /* REPORT_SIZE (8) */ >> + 0x95, 0x01, /* REPORT_COUNT (1) */ >> + 0x09, 0x38, /* USAGE (wheel) */ >> + 0x81, 0x06, /* INPUT */ >> + 0x05, 0x0C, /* USAGE_PAGE(consumer) */ >> + 0x0A, 0x38, 0x02, /* USAGE(AC Pan) */ >> + 0x95, 0x01, /* REPORT_COUNT (1) */ >> + 0x81, 0x06, /* INPUT */ >> + 0xC0, /* END_COLLECTION */ >> + 0xC0, /* END_COLLECTION */ >> +}; >> + >> /* Gaming Mouse descriptor (2) */ >> static const char mse_high_res_descriptor[] = { >> 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ >> @@ -596,7 +636,10 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, >> "Logitech Unifying Device. Wireless PID:%04x", >> dj_hiddev->product); >> >> - dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE; >> + if (djrcv_dev->type == recvr_type_27mhz) >> + dj_hiddev->group = HID_GROUP_LOGITECH_27MHZ_DEVICE; >> + else >> + dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE; >> >> memcpy(dj_hiddev->phys, djrcv_hdev->phys, sizeof(djrcv_hdev->phys)); >> snprintf(tmpstr, sizeof(tmpstr), ":%d", device_index); >> @@ -782,6 +825,28 @@ static void logi_hidpp_dev_conn_notif_equad(struct hidpp_event *hidpp_report, >> } >> } >> >> +static void logi_hidpp_dev_conn_notif_27mhz(struct hid_device *hdev, >> + struct hidpp_event *hidpp_report, >> + struct dj_workitem *workitem) >> +{ >> + workitem->type = WORKITEM_TYPE_PAIRED; >> + workitem->quad_id_lsb = hidpp_report->params[HIDPP_PARAM_27MHZ_DEVID]; >> + switch (hidpp_report->device_index) { >> + case 1: /* Index 1 is always a mouse */ > > IIRC we are supposed to mark the fall through statements as such. Not > sure why checkpatch doesn't complain here. Using multiple labels behind one each other without any statements in between is a common designpattern and -Wimplicit-fallthrough allows this without warning. I've added a patch to my personal tree to enable -Wimplicit-fallthrough to make sure that I'm not introducing new warnings: https://github.com/jwrdegoede/linux-sunxi/commit/c10090f844bbcb8c646ca9afec269c8242526255 So adding fall through comments to consecutive case labels without any statements in between is not necessary and is quite ugly IMHO. Regards, Hans > >> + case 2: /* Index 2 is always a mouse */ >> + workitem->reports_supported |= STD_MOUSE; >> + break; >> + case 3: /* Index 3 is always the keyboard */ > > Same here, please mark this as a fall-through. > > Cheers, > Benjamin > >> + case 4: /* Index 4 is used for an optional separate numpad */ >> + workitem->reports_supported |= STD_KEYBOARD | MULTIMEDIA | >> + POWER_KEYS; >> + break; >> + default: >> + hid_warn(hdev, "%s: unexpected device-index %d", __func__, >> + hidpp_report->device_index); >> + } >> +} >> + >> static void logi_hidpp_recv_queue_notif(struct hid_device *hdev, >> struct hidpp_event *hidpp_report) >> { >> @@ -799,6 +864,7 @@ static void logi_hidpp_recv_queue_notif(struct hid_device *hdev, >> break; >> case 0x02: >> device_type = "27 Mhz"; >> + logi_hidpp_dev_conn_notif_27mhz(hdev, hidpp_report, &workitem); >> break; >> case 0x03: >> device_type = "QUAD or eQUAD"; >> @@ -1186,6 +1252,9 @@ static int logi_dj_ll_parse(struct hid_device *hid) >> if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp) >> rdcat(rdesc, &rsize, mse_high_res_descriptor, >> sizeof(mse_high_res_descriptor)); >> + else if (djdev->dj_receiver_dev->type == recvr_type_27mhz) >> + rdcat(rdesc, &rsize, mse_27mhz_descriptor, >> + sizeof(mse_27mhz_descriptor)); >> else >> rdcat(rdesc, &rsize, mse_descriptor, >> sizeof(mse_descriptor)); >> @@ -1357,6 +1426,25 @@ static int logi_dj_hidpp_event(struct hid_device *hdev, >> spin_lock_irqsave(&djrcv_dev->lock, flags); >> >> dj_dev = djrcv_dev->paired_dj_devices[device_index]; >> + >> + /* >> + * With 27 MHz receivers, we do not get an explicit unpair event, >> + * remove the old device if the user has paired a *different* device. >> + */ >> + if (djrcv_dev->type == recvr_type_27mhz && dj_dev && >> + hidpp_report->sub_id == REPORT_TYPE_NOTIF_DEVICE_CONNECTED && >> + hidpp_report->params[HIDPP_PARAM_PROTO_TYPE] == 0x02 && >> + hidpp_report->params[HIDPP_PARAM_27MHZ_DEVID] != >> + dj_dev->hdev->product) { >> + struct dj_workitem workitem = { >> + .device_index = hidpp_report->device_index, >> + .type = WORKITEM_TYPE_UNPAIRED, >> + }; >> + kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem)); >> + /* logi_hidpp_recv_queue_notif will queue the work */ >> + dj_dev = NULL; >> + } >> + >> if (dj_dev) { >> logi_dj_recv_forward_report(dj_dev, data, size); >> } else { >> @@ -1624,6 +1712,10 @@ static const struct hid_device_id logi_dj_receivers[] = { >> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, >> USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_GAMING), >> .driver_data = recvr_type_gaming_hidpp}, >> + { /* Logitech 27 MHz HID++ 1.0 receiver (0xc517) */ >> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, >> + USB_DEVICE_ID_S510_RECEIVER_2), >> + .driver_data = recvr_type_27mhz}, >> {} >> }; >> >> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c >> index 56559f2b8334..5decf16aeb4b 100644 >> --- a/drivers/hid/hid-quirks.c >> +++ b/drivers/hid/hid-quirks.c >> @@ -432,7 +432,6 @@ static const struct hid_device_id hid_have_special_driver[] = { >> #if IS_ENABLED(CONFIG_HID_LOGITECH) >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) }, >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) }, >> - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2) }, >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) }, >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) }, >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) }, >> diff --git a/include/linux/hid.h b/include/linux/hid.h >> index f9707d1dcb58..9f161fa5cbd4 100644 >> --- a/include/linux/hid.h >> +++ b/include/linux/hid.h >> @@ -382,6 +382,7 @@ struct hid_item { >> #define HID_GROUP_WACOM 0x0101 >> #define HID_GROUP_LOGITECH_DJ_DEVICE 0x0102 >> #define HID_GROUP_STEAM 0x0103 >> +#define HID_GROUP_LOGITECH_27MHZ_DEVICE 0x0104 >> >> /* >> * HID protocol status >> -- >> 2.21.0 >>
On Sat, Apr 20, 2019 at 12:34 AM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 4/19/19 5:54 PM, Benjamin Tissoires wrote: > > On Wed, Apr 10, 2019 at 4:55 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> > >> Most Logitech wireless keyboard and mice using the 27 MHz are hidpp10 > >> devices, add support to logitech-dj for their receivers. > >> > >> Doing so leads to 2 improvements: > >> > >> 1) All these devices share the same USB product-id for their receiver, > >> making it impossible to properly map some special keys / buttons > >> which differ from device to device. Adding support to logitech-dj to > >> see these as hidpp10 devices allows us to get the actual device-id > >> from the keyboard / mouse. > >> > >> 2) It enables battery-monitoring of these devices > >> > >> This patch uses a new HID group for 27Mhz devices, since the logitech-hidpp > >> code needs to be able to differentiate them from other devices instantiated > >> by the logitech-dj code. > >> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> Changes in v2: > >> -Remove chunk to pick a better name for hidpp devices, this is replaced > >> with a more generic fix in a separate patch > >> -Add support for mouse on index 2, numeric-keypad on index 4 > >> -Use a new group for 27MHz, so that logitech-hidpp.c can check for this to > >> enable 27MHz specific behavior, rather then have it guess based on the > >> descriptors. This also allows removing some 27MHz descriptor hacks from > >> logitech-dj.c, so it simplifies things on both ends > >> --- > >> drivers/hid/hid-lg.c | 2 - > >> drivers/hid/hid-logitech-dj.c | 94 ++++++++++++++++++++++++++++++++++- > >> drivers/hid/hid-quirks.c | 1 - > >> include/linux/hid.h | 1 + > >> 4 files changed, 94 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c > >> index 5d419a95b6c2..36d725fdb199 100644 > >> --- a/drivers/hid/hid-lg.c > >> +++ b/drivers/hid/hid-lg.c > >> @@ -876,8 +876,6 @@ static const struct hid_device_id lg_devices[] = { > >> .driver_data = LG_RDESC | LG_WIRELESS }, > >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER), > >> .driver_data = LG_RDESC | LG_WIRELESS }, > >> - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2), > >> - .driver_data = LG_RDESC | LG_WIRELESS }, > >> > >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER), > >> .driver_data = LG_BAD_RELATIVE_KEYS }, > >> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c > >> index a994d2e3dd9e..9d5db242e326 100644 > >> --- a/drivers/hid/hid-logitech-dj.c > >> +++ b/drivers/hid/hid-logitech-dj.c > >> @@ -106,6 +106,7 @@ > >> #define HIDPP_PARAM_DEVICE_INFO 0x01 > >> #define HIDPP_PARAM_EQUAD_LSB 0x02 > >> #define HIDPP_PARAM_EQUAD_MSB 0x03 > >> +#define HIDPP_PARAM_27MHZ_DEVID 0x03 > >> #define HIDPP_DEVICE_TYPE_MASK GENMASK(3, 0) > >> #define HIDPP_LINK_STATUS_MASK BIT(6) > >> > >> @@ -120,6 +121,7 @@ enum recvr_type { > >> recvr_type_dj, > >> recvr_type_hidpp, > >> recvr_type_gaming_hidpp, > >> + recvr_type_27mhz, > >> }; > >> > >> struct dj_report { > >> @@ -248,6 +250,44 @@ static const char mse_descriptor[] = { > >> 0xC0, /* END_COLLECTION */ > >> }; > >> > >> +/* Mouse descriptor (2) for 27 MHz receiver, only 8 buttons */ > >> +static const char mse_27mhz_descriptor[] = { > >> + 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ > >> + 0x09, 0x02, /* USAGE (Mouse) */ > >> + 0xA1, 0x01, /* COLLECTION (Application) */ > >> + 0x85, 0x02, /* REPORT_ID = 2 */ > >> + 0x09, 0x01, /* USAGE (pointer) */ > >> + 0xA1, 0x00, /* COLLECTION (physical) */ > >> + 0x05, 0x09, /* USAGE_PAGE (buttons) */ > >> + 0x19, 0x01, /* USAGE_MIN (1) */ > >> + 0x29, 0x08, /* USAGE_MAX (8) */ > >> + 0x15, 0x00, /* LOGICAL_MIN (0) */ > >> + 0x25, 0x01, /* LOGICAL_MAX (1) */ > >> + 0x95, 0x08, /* REPORT_COUNT (8) */ > >> + 0x75, 0x01, /* REPORT_SIZE (1) */ > >> + 0x81, 0x02, /* INPUT (data var abs) */ > >> + 0x05, 0x01, /* USAGE_PAGE (generic desktop) */ > >> + 0x16, 0x01, 0xF8, /* LOGICAL_MIN (-2047) */ > >> + 0x26, 0xFF, 0x07, /* LOGICAL_MAX (2047) */ > >> + 0x75, 0x0C, /* REPORT_SIZE (12) */ > >> + 0x95, 0x02, /* REPORT_COUNT (2) */ > >> + 0x09, 0x30, /* USAGE (X) */ > >> + 0x09, 0x31, /* USAGE (Y) */ > >> + 0x81, 0x06, /* INPUT */ > >> + 0x15, 0x81, /* LOGICAL_MIN (-127) */ > >> + 0x25, 0x7F, /* LOGICAL_MAX (127) */ > >> + 0x75, 0x08, /* REPORT_SIZE (8) */ > >> + 0x95, 0x01, /* REPORT_COUNT (1) */ > >> + 0x09, 0x38, /* USAGE (wheel) */ > >> + 0x81, 0x06, /* INPUT */ > >> + 0x05, 0x0C, /* USAGE_PAGE(consumer) */ > >> + 0x0A, 0x38, 0x02, /* USAGE(AC Pan) */ > >> + 0x95, 0x01, /* REPORT_COUNT (1) */ > >> + 0x81, 0x06, /* INPUT */ > >> + 0xC0, /* END_COLLECTION */ > >> + 0xC0, /* END_COLLECTION */ > >> +}; > >> + > >> /* Gaming Mouse descriptor (2) */ > >> static const char mse_high_res_descriptor[] = { > >> 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ > >> @@ -596,7 +636,10 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, > >> "Logitech Unifying Device. Wireless PID:%04x", > >> dj_hiddev->product); > >> > >> - dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE; > >> + if (djrcv_dev->type == recvr_type_27mhz) > >> + dj_hiddev->group = HID_GROUP_LOGITECH_27MHZ_DEVICE; > >> + else > >> + dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE; > >> > >> memcpy(dj_hiddev->phys, djrcv_hdev->phys, sizeof(djrcv_hdev->phys)); > >> snprintf(tmpstr, sizeof(tmpstr), ":%d", device_index); > >> @@ -782,6 +825,28 @@ static void logi_hidpp_dev_conn_notif_equad(struct hidpp_event *hidpp_report, > >> } > >> } > >> > >> +static void logi_hidpp_dev_conn_notif_27mhz(struct hid_device *hdev, > >> + struct hidpp_event *hidpp_report, > >> + struct dj_workitem *workitem) > >> +{ > >> + workitem->type = WORKITEM_TYPE_PAIRED; > >> + workitem->quad_id_lsb = hidpp_report->params[HIDPP_PARAM_27MHZ_DEVID]; > >> + switch (hidpp_report->device_index) { > >> + case 1: /* Index 1 is always a mouse */ > > > > IIRC we are supposed to mark the fall through statements as such. Not > > sure why checkpatch doesn't complain here. > > Using multiple labels behind one each other without any statements in > between is a common designpattern and -Wimplicit-fallthrough allows this > without warning. I've added a patch to my personal tree to enable > -Wimplicit-fallthrough to make sure that I'm not introducing new warnings: > https://github.com/jwrdegoede/linux-sunxi/commit/c10090f844bbcb8c646ca9afec269c8242526255 > > So adding fall through comments to consecutive case labels without > any statements in between is not necessary and is quite ugly IMHO. OK, that's good news. I also thought this would be ugly but I do not like receiving a message from a bot telling me we need this right after I applied a patch. Cheers, Benjamin > > Regards, > > Hans > > > > > > >> + case 2: /* Index 2 is always a mouse */ > >> + workitem->reports_supported |= STD_MOUSE; > >> + break; > >> + case 3: /* Index 3 is always the keyboard */ > > > > Same here, please mark this as a fall-through. > > > > Cheers, > > Benjamin > > > >> + case 4: /* Index 4 is used for an optional separate numpad */ > >> + workitem->reports_supported |= STD_KEYBOARD | MULTIMEDIA | > >> + POWER_KEYS; > >> + break; > >> + default: > >> + hid_warn(hdev, "%s: unexpected device-index %d", __func__, > >> + hidpp_report->device_index); > >> + } > >> +} > >> + > >> static void logi_hidpp_recv_queue_notif(struct hid_device *hdev, > >> struct hidpp_event *hidpp_report) > >> { > >> @@ -799,6 +864,7 @@ static void logi_hidpp_recv_queue_notif(struct hid_device *hdev, > >> break; > >> case 0x02: > >> device_type = "27 Mhz"; > >> + logi_hidpp_dev_conn_notif_27mhz(hdev, hidpp_report, &workitem); > >> break; > >> case 0x03: > >> device_type = "QUAD or eQUAD"; > >> @@ -1186,6 +1252,9 @@ static int logi_dj_ll_parse(struct hid_device *hid) > >> if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp) > >> rdcat(rdesc, &rsize, mse_high_res_descriptor, > >> sizeof(mse_high_res_descriptor)); > >> + else if (djdev->dj_receiver_dev->type == recvr_type_27mhz) > >> + rdcat(rdesc, &rsize, mse_27mhz_descriptor, > >> + sizeof(mse_27mhz_descriptor)); > >> else > >> rdcat(rdesc, &rsize, mse_descriptor, > >> sizeof(mse_descriptor)); > >> @@ -1357,6 +1426,25 @@ static int logi_dj_hidpp_event(struct hid_device *hdev, > >> spin_lock_irqsave(&djrcv_dev->lock, flags); > >> > >> dj_dev = djrcv_dev->paired_dj_devices[device_index]; > >> + > >> + /* > >> + * With 27 MHz receivers, we do not get an explicit unpair event, > >> + * remove the old device if the user has paired a *different* device. > >> + */ > >> + if (djrcv_dev->type == recvr_type_27mhz && dj_dev && > >> + hidpp_report->sub_id == REPORT_TYPE_NOTIF_DEVICE_CONNECTED && > >> + hidpp_report->params[HIDPP_PARAM_PROTO_TYPE] == 0x02 && > >> + hidpp_report->params[HIDPP_PARAM_27MHZ_DEVID] != > >> + dj_dev->hdev->product) { > >> + struct dj_workitem workitem = { > >> + .device_index = hidpp_report->device_index, > >> + .type = WORKITEM_TYPE_UNPAIRED, > >> + }; > >> + kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem)); > >> + /* logi_hidpp_recv_queue_notif will queue the work */ > >> + dj_dev = NULL; > >> + } > >> + > >> if (dj_dev) { > >> logi_dj_recv_forward_report(dj_dev, data, size); > >> } else { > >> @@ -1624,6 +1712,10 @@ static const struct hid_device_id logi_dj_receivers[] = { > >> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, > >> USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_GAMING), > >> .driver_data = recvr_type_gaming_hidpp}, > >> + { /* Logitech 27 MHz HID++ 1.0 receiver (0xc517) */ > >> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, > >> + USB_DEVICE_ID_S510_RECEIVER_2), > >> + .driver_data = recvr_type_27mhz}, > >> {} > >> }; > >> > >> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c > >> index 56559f2b8334..5decf16aeb4b 100644 > >> --- a/drivers/hid/hid-quirks.c > >> +++ b/drivers/hid/hid-quirks.c > >> @@ -432,7 +432,6 @@ static const struct hid_device_id hid_have_special_driver[] = { > >> #if IS_ENABLED(CONFIG_HID_LOGITECH) > >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) }, > >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) }, > >> - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2) }, > >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) }, > >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) }, > >> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) }, > >> diff --git a/include/linux/hid.h b/include/linux/hid.h > >> index f9707d1dcb58..9f161fa5cbd4 100644 > >> --- a/include/linux/hid.h > >> +++ b/include/linux/hid.h > >> @@ -382,6 +382,7 @@ struct hid_item { > >> #define HID_GROUP_WACOM 0x0101 > >> #define HID_GROUP_LOGITECH_DJ_DEVICE 0x0102 > >> #define HID_GROUP_STEAM 0x0103 > >> +#define HID_GROUP_LOGITECH_27MHZ_DEVICE 0x0104 > >> > >> /* > >> * HID protocol status > >> -- > >> 2.21.0 > >>
diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c index 5d419a95b6c2..36d725fdb199 100644 --- a/drivers/hid/hid-lg.c +++ b/drivers/hid/hid-lg.c @@ -876,8 +876,6 @@ static const struct hid_device_id lg_devices[] = { .driver_data = LG_RDESC | LG_WIRELESS }, { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER), .driver_data = LG_RDESC | LG_WIRELESS }, - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2), - .driver_data = LG_RDESC | LG_WIRELESS }, { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER), .driver_data = LG_BAD_RELATIVE_KEYS }, diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index a994d2e3dd9e..9d5db242e326 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -106,6 +106,7 @@ #define HIDPP_PARAM_DEVICE_INFO 0x01 #define HIDPP_PARAM_EQUAD_LSB 0x02 #define HIDPP_PARAM_EQUAD_MSB 0x03 +#define HIDPP_PARAM_27MHZ_DEVID 0x03 #define HIDPP_DEVICE_TYPE_MASK GENMASK(3, 0) #define HIDPP_LINK_STATUS_MASK BIT(6) @@ -120,6 +121,7 @@ enum recvr_type { recvr_type_dj, recvr_type_hidpp, recvr_type_gaming_hidpp, + recvr_type_27mhz, }; struct dj_report { @@ -248,6 +250,44 @@ static const char mse_descriptor[] = { 0xC0, /* END_COLLECTION */ }; +/* Mouse descriptor (2) for 27 MHz receiver, only 8 buttons */ +static const char mse_27mhz_descriptor[] = { + 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ + 0x09, 0x02, /* USAGE (Mouse) */ + 0xA1, 0x01, /* COLLECTION (Application) */ + 0x85, 0x02, /* REPORT_ID = 2 */ + 0x09, 0x01, /* USAGE (pointer) */ + 0xA1, 0x00, /* COLLECTION (physical) */ + 0x05, 0x09, /* USAGE_PAGE (buttons) */ + 0x19, 0x01, /* USAGE_MIN (1) */ + 0x29, 0x08, /* USAGE_MAX (8) */ + 0x15, 0x00, /* LOGICAL_MIN (0) */ + 0x25, 0x01, /* LOGICAL_MAX (1) */ + 0x95, 0x08, /* REPORT_COUNT (8) */ + 0x75, 0x01, /* REPORT_SIZE (1) */ + 0x81, 0x02, /* INPUT (data var abs) */ + 0x05, 0x01, /* USAGE_PAGE (generic desktop) */ + 0x16, 0x01, 0xF8, /* LOGICAL_MIN (-2047) */ + 0x26, 0xFF, 0x07, /* LOGICAL_MAX (2047) */ + 0x75, 0x0C, /* REPORT_SIZE (12) */ + 0x95, 0x02, /* REPORT_COUNT (2) */ + 0x09, 0x30, /* USAGE (X) */ + 0x09, 0x31, /* USAGE (Y) */ + 0x81, 0x06, /* INPUT */ + 0x15, 0x81, /* LOGICAL_MIN (-127) */ + 0x25, 0x7F, /* LOGICAL_MAX (127) */ + 0x75, 0x08, /* REPORT_SIZE (8) */ + 0x95, 0x01, /* REPORT_COUNT (1) */ + 0x09, 0x38, /* USAGE (wheel) */ + 0x81, 0x06, /* INPUT */ + 0x05, 0x0C, /* USAGE_PAGE(consumer) */ + 0x0A, 0x38, 0x02, /* USAGE(AC Pan) */ + 0x95, 0x01, /* REPORT_COUNT (1) */ + 0x81, 0x06, /* INPUT */ + 0xC0, /* END_COLLECTION */ + 0xC0, /* END_COLLECTION */ +}; + /* Gaming Mouse descriptor (2) */ static const char mse_high_res_descriptor[] = { 0x05, 0x01, /* USAGE_PAGE (Generic Desktop) */ @@ -596,7 +636,10 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev, "Logitech Unifying Device. Wireless PID:%04x", dj_hiddev->product); - dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE; + if (djrcv_dev->type == recvr_type_27mhz) + dj_hiddev->group = HID_GROUP_LOGITECH_27MHZ_DEVICE; + else + dj_hiddev->group = HID_GROUP_LOGITECH_DJ_DEVICE; memcpy(dj_hiddev->phys, djrcv_hdev->phys, sizeof(djrcv_hdev->phys)); snprintf(tmpstr, sizeof(tmpstr), ":%d", device_index); @@ -782,6 +825,28 @@ static void logi_hidpp_dev_conn_notif_equad(struct hidpp_event *hidpp_report, } } +static void logi_hidpp_dev_conn_notif_27mhz(struct hid_device *hdev, + struct hidpp_event *hidpp_report, + struct dj_workitem *workitem) +{ + workitem->type = WORKITEM_TYPE_PAIRED; + workitem->quad_id_lsb = hidpp_report->params[HIDPP_PARAM_27MHZ_DEVID]; + switch (hidpp_report->device_index) { + case 1: /* Index 1 is always a mouse */ + case 2: /* Index 2 is always a mouse */ + workitem->reports_supported |= STD_MOUSE; + break; + case 3: /* Index 3 is always the keyboard */ + case 4: /* Index 4 is used for an optional separate numpad */ + workitem->reports_supported |= STD_KEYBOARD | MULTIMEDIA | + POWER_KEYS; + break; + default: + hid_warn(hdev, "%s: unexpected device-index %d", __func__, + hidpp_report->device_index); + } +} + static void logi_hidpp_recv_queue_notif(struct hid_device *hdev, struct hidpp_event *hidpp_report) { @@ -799,6 +864,7 @@ static void logi_hidpp_recv_queue_notif(struct hid_device *hdev, break; case 0x02: device_type = "27 Mhz"; + logi_hidpp_dev_conn_notif_27mhz(hdev, hidpp_report, &workitem); break; case 0x03: device_type = "QUAD or eQUAD"; @@ -1186,6 +1252,9 @@ static int logi_dj_ll_parse(struct hid_device *hid) if (djdev->dj_receiver_dev->type == recvr_type_gaming_hidpp) rdcat(rdesc, &rsize, mse_high_res_descriptor, sizeof(mse_high_res_descriptor)); + else if (djdev->dj_receiver_dev->type == recvr_type_27mhz) + rdcat(rdesc, &rsize, mse_27mhz_descriptor, + sizeof(mse_27mhz_descriptor)); else rdcat(rdesc, &rsize, mse_descriptor, sizeof(mse_descriptor)); @@ -1357,6 +1426,25 @@ static int logi_dj_hidpp_event(struct hid_device *hdev, spin_lock_irqsave(&djrcv_dev->lock, flags); dj_dev = djrcv_dev->paired_dj_devices[device_index]; + + /* + * With 27 MHz receivers, we do not get an explicit unpair event, + * remove the old device if the user has paired a *different* device. + */ + if (djrcv_dev->type == recvr_type_27mhz && dj_dev && + hidpp_report->sub_id == REPORT_TYPE_NOTIF_DEVICE_CONNECTED && + hidpp_report->params[HIDPP_PARAM_PROTO_TYPE] == 0x02 && + hidpp_report->params[HIDPP_PARAM_27MHZ_DEVID] != + dj_dev->hdev->product) { + struct dj_workitem workitem = { + .device_index = hidpp_report->device_index, + .type = WORKITEM_TYPE_UNPAIRED, + }; + kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem)); + /* logi_hidpp_recv_queue_notif will queue the work */ + dj_dev = NULL; + } + if (dj_dev) { logi_dj_recv_forward_report(dj_dev, data, size); } else { @@ -1624,6 +1712,10 @@ static const struct hid_device_id logi_dj_receivers[] = { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_GAMING), .driver_data = recvr_type_gaming_hidpp}, + { /* Logitech 27 MHz HID++ 1.0 receiver (0xc517) */ + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, + USB_DEVICE_ID_S510_RECEIVER_2), + .driver_data = recvr_type_27mhz}, {} }; diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index 56559f2b8334..5decf16aeb4b 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -432,7 +432,6 @@ static const struct hid_device_id hid_have_special_driver[] = { #if IS_ENABLED(CONFIG_HID_LOGITECH) { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) }, { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) }, - { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2) }, { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) }, { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) }, { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) }, diff --git a/include/linux/hid.h b/include/linux/hid.h index f9707d1dcb58..9f161fa5cbd4 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -382,6 +382,7 @@ struct hid_item { #define HID_GROUP_WACOM 0x0101 #define HID_GROUP_LOGITECH_DJ_DEVICE 0x0102 #define HID_GROUP_STEAM 0x0103 +#define HID_GROUP_LOGITECH_27MHZ_DEVICE 0x0104 /* * HID protocol status
Most Logitech wireless keyboard and mice using the 27 MHz are hidpp10 devices, add support to logitech-dj for their receivers. Doing so leads to 2 improvements: 1) All these devices share the same USB product-id for their receiver, making it impossible to properly map some special keys / buttons which differ from device to device. Adding support to logitech-dj to see these as hidpp10 devices allows us to get the actual device-id from the keyboard / mouse. 2) It enables battery-monitoring of these devices This patch uses a new HID group for 27Mhz devices, since the logitech-hidpp code needs to be able to differentiate them from other devices instantiated by the logitech-dj code. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: -Remove chunk to pick a better name for hidpp devices, this is replaced with a more generic fix in a separate patch -Add support for mouse on index 2, numeric-keypad on index 4 -Use a new group for 27MHz, so that logitech-hidpp.c can check for this to enable 27MHz specific behavior, rather then have it guess based on the descriptors. This also allows removing some 27MHz descriptor hacks from logitech-dj.c, so it simplifies things on both ends --- drivers/hid/hid-lg.c | 2 - drivers/hid/hid-logitech-dj.c | 94 ++++++++++++++++++++++++++++++++++- drivers/hid/hid-quirks.c | 1 - include/linux/hid.h | 1 + 4 files changed, 94 insertions(+), 4 deletions(-)