This patch will cause more use of a part of the driver that constructs messages that do not conform to the HID++ 2.0 specification. This makes now a good time to fix the parts of the driver that construct non-conforming messages. More information follows. The Logitech HID++2.0 Draft Specification at https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf specifies (on pages 2 and 3) that the low-order nibble of the function (command) byte is non-zero. The HID++ driver at https://github.com/torvalds/linux/blob/master/drivers/hid/hid-logitech-hidpp.c has 1 in that nibble for some commands, #define CMD_ROOT_GET_FEATURE 0x01 #define CMD_ROOT_GET_PROTOCOL_VERSION 0x11 #define CMD_GET_DEVICE_NAME_TYPE_GET_COUNT 0x01 #define CMD_GET_DEVICE_NAME_TYPE_GET_DEVICE_NAME 0x11 #define CMD_GET_DEVICE_NAME_TYPE_GET_TYPE 0x21 But other commands have zero in that nibble, namely #define CMD_BATTERY_LEVEL_STATUS_GET_BATTERY_LEVEL_STATUS 0x00 #define CMD_BATTERY_LEVEL_STATUS_GET_BATTERY_CAPABILITY 0x10 #define CMD_BATTERY_VOLTAGE_GET_BATTERY_VOLTAGE 0x00 #define CMD_UNIFIED_BATTERY_GET_CAPABILITIES 0x00 #define CMD_UNIFIED_BATTERY_GET_STATUS 0x10 #define CMD_HI_RESOLUTION_SCROLLING_SET_HIGHRES_SCROLLING_MODE 0x10 #define CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY 0x00 #define CMD_HIRES_WHEEL_SET_WHEEL_MODE 0x20 #define CMD_SOLAR_SET_LIGHT_MEASURE 0x00 #define CMD_TOUCHPAD_FW_ITEMS_SET 0x10 This results in non-conforming messages being sent to devices. As devices are obligated to return this nibble intact they produce non- conforming responses as well. (Their other option would be to reject the messages.) This confuses other software that correctly uses this nibble to distinguish between device response messages and device event messages. In particular, the response to the unified battery command to get the capabilities comes back with a 0x00 function byte which is indistinguishable from a spontaneous notification message from the device for a battery status event. Other software trying to communicate with the device (e.g., Solaar) sees a unified battery status notification and will generally end up with incorrect information about the device. I suspect that this is actually happening and is the cause of the Solaar bug report https://github.com/pwr-Solaar/Solaar/issues/1718 There is also the possibility that the driver confuses a notification from the device as the response to a command that it sent. When this happens it would be likely that the actual response would be treated as a notification. The fix is to modify all the CMD definitions in the code to have 1 in their low-order nibble. peter PS: There is another HID++ 2.0 feature that reports battery information, 0x1F20 ADC Measurement, but that is not in the driver code. On Thu, 2022-08-25 at 17:59 +0200, Nestor Lopez Casado wrote: > Adding Peter, as he maintains Solaar, one popular application that > supports Logitech bluetooth devices. > @Peter F. Patel-Schneider > -nestor > > On Thu, Aug 25, 2022 at 2:29 PM Bastien Nocera <hadess@xxxxxxxxxx> > wrote: > > > > Probe for HID++ support over Bluetooth for all the Logitech > > Bluetooth > > devices. As Logitech doesn't have a list of Bluetooth devices that > > support HID++ over Bluetooth, probe every device. The HID++ driver > > will fall back to plain HID if the device does not support HID++. > > > > Note that this change might cause upower to export 2 batteries for > > certain Bluetooth LE devices which export their battery information > > through the Bluetooth BATT profile. This particular bug is tracked > > at: > > https://gitlab.freedesktop.org/upower/upower/-/issues/166 > > > > Tested with a Logitech Signature M650 mouse, over Bluetooth > > > > Signed-off-by: Bastien Nocera <hadess@xxxxxxxxxx> > > --- > > > > Note that I could not test whether the Harmony PS3 (handled in hid- > > sony.c) > > or DiNovo Edge keyboard (handled in hid-input.c) devices would > > correctly fallback > > to those drivers in that case. > > > > Ways to test this would be appreciated (or merge this, and wait for > > feedback...) > > > > drivers/hid/hid-logitech-hidpp.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid- > > logitech-hidpp.c > > index 81de88ab2ecc..86e7a38d8a9a 100644 > > --- a/drivers/hid/hid-logitech-hidpp.c > > +++ b/drivers/hid/hid-logitech-hidpp.c > > @@ -4347,6 +4347,9 @@ static const struct hid_device_id > > hidpp_devices[] = { > > { /* MX Master 3 mouse over Bluetooth */ > > HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb023), > > .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, > > + > > + { /* And try to enable HID++ for all the Logitech Bluetooth > > devices */ > > + HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_ANY, > > USB_VENDOR_ID_LOGITECH, HID_ANY_ID) }, > > {} > > }; > > > > -- > > 2.37.2 > >
