Hi,
On 19-04-19 17:52, Benjamin Tissoires wrote:
Hi Hans,
On Wed, Apr 10, 2019 at 4:55 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
Hi All,
Here is v2 of Benjamin's and mine series to enumerate the devices behind
various non DJ receivers instead of relying on their HID proxy mode.
Thanks for the re-spin. I spent a good amount of time today reviewing
the series and testing it, and I am happy with it.
There are a couple of nitpicks I'd like to see addressed, but
otherwise, I think this is ready for 5.2.
Checkpatch complains about 2 "ERROR: do not use assignment in if
condition" in patches 19/37 and 27/37. Though I can see the benefit of
having it that way, I think we should stick to checkpatch here as it
is too tempting to say "hey, there's a typo here, let's replace `=` by
`==`"
Checkpatch also complains about:
- trailing statements should be on next line: but I do not really mind
those, the blocks it complains are clearer that way
- lines over 80 characters -> these are fine in that context
- Block comments use a trailing */ on a separate line -> please fix
those (patches 7/37 and 8/37) as the rest of the code has the correct
block comments style
Heh, patches 7/37 and 8/37 are actually unmodified patches from you,
I was already wondering why I did not catch this myself.
Anyways I'm preparing a v3 series addressing the above comments now,
I will send this out shortly.
Regards,
Hans
There are 3 major changes in v2:
1) Add a new HID group for 27 MHz devices, this has 2 advantages:
1a) It allows the logitech-hidpp code to identity 27 Mhz devices and to
automatically enable some HID++ quirls for these, such as e.g. HID++
mouse-wheel handling (fixing some hwheel issues)
1b) Since the logitech-hidpp code can now identify these the logitech-dj
code for 27 MHz devices no longer needs separate 27 MHz descriptors for the
consumer and HID++ reports.
I am not entirely convinced we need a new group. But OTOH, being able
to quickly and reliably filter those legacy device is something that
is useful.
2) Add support Logitech Bluetooth receivers in HID proxy mode, tested with a
MX5000 keyboard + receiver. This allows e.g. battery monitoring of the kbd.
I played today with a diNovo mini which has a slightly different
Logitech BT receiver. This part works fine except for the integrated
touchapd which provides reports with an ID 5. I locally hacked
something to have the mouse working, but it's a hack.
The only problem I have with integrating the diNovo mini is that
hid-lg.c does an input_mapping for one key on the keyboard... And I
think we will need it too in hid-logitech-hidpp :/
I also added the G700 locally, and a small modification to support
unnumbered mouse reports of size 8 is required, but I can send that as
a followup patch.
Cheers,
Benjamin
3) Rework HID++ mouse-wheel and extra-buttons support making it more
generic and cleaner and add HID++ consumer-keys report. Using the HID++
consumer keys reports fixes several keys not working on some kbds.
Per patch info, unlisted patches are unchanged:
[PATCH 01/37] HID: quirks: do not blacklist Logitech devices
-New patch split of from a cleanup patch from Benjamin which was touching
many different drivers at once. This removes a dep on that patch.
[PATCH 12/37] HID: logitech-dj: support sharing struct dj_receiver_dev between USB-interfaces
-Fixed calling hid_err on a NULL hdev
[PATCH 16/37] HID: logitech-dj: add support for 27 MHz receivers
-Remove chunk to pick a better name for hidpp devices, see new patch below
-Add support for mouse on index 2, numeric-keypad on index 4
-Use a new group for 27MHz devices
[PATCH 20/37] HID: logitech-dj: pick a better name for non-unifying receivers
-New patch replacing the dropped chunk from patch 16 as well as replacing
"HID: logitech-hidpp: create a name based on the type if non available"
from v1
[PATCH 21/37] HID: logitech-dj: remove false-positive error ...
-New misc. fix patch
[PATCH 22/37] HID: logitech-dj: make appending of the HID++ ...
[PATCH 23/37] HID: logitech-dj: add support for Logitech Bluetooth
-2 new patches adding support for Logitech BT receivers in HID proxy mode
[PATCH 26/37] HID: logitech-hidpp: ignore very-short or empty
[PATCH 27/37] HID: logitech-hidpp: do not make failure to get the
[PATCH 28/37] HID: logitech-hidpp: remove double assignment from
[PATCH 29/37] HID: logitech-hidpp: remove unused
-4 new patches (misc. fixes / cleanups)
[PATCH 31/37] HID: logitech-hidpp: handle devices attached to
-Add support for the new 27 MHz lg devices hid group
[PATCH 32/37] HID: logitech-hidpp: do not hardcode very long
-This fixes logitech-hidpp rejecting the mx5000 keyboard when paired over BT
[PATCH 34/37] HID: logitech-hidpp: make hidpp10_set_register_bit a
[PATCH 35/37] HID: logitech-hidpp: add support for HID++ 1.0 wheel
[PATCH 36/37] HID: logitech-hidpp: add support for HID++ 1.0 extra
[PATCH 37/37] HID: logitech-hidpp: add support for HID++ 1.0
-Reworked HID++ mouse-wheel and extra-buttons support making it more
generic and add HID++ consumer-keys report support
For reference here is the cover letter of v1 of the series:
Here is a series Benjamin and I have been working on, before this series we
treat Logitech wireless mice / keyboards connected through a non-unifying
receiver as generic HID devices, using the generic HID emulation of the
receiver. This causes several problems:
-We cannot properly support some special keys / buttons on some keyboards /
mice since we cannot properly identify / talk to the device behind the recv.
-We cannot monitor the battery in these devices
This series addresses this by actually enumerating (and talking to) the
devices behind the receiver, rather then solely relying on the receiver's
generic HID emulation.
Note this series applies on top of the:
"[PATCH] HID: force setting drvdata to NULL when removing the driver"
patch which Benjamin send out 2 days ago.
Regards,
Hans
