On Tue, 2022-08-30 at 15:19 +0200, Benjamin Tissoires wrote: > On Tue, Aug 30, 2022 at 1:39 PM Bastien Nocera <hadess@xxxxxxxxxx> > wrote: > > > > Always set a non-zero "Sw. Id." in the lower nibble of the > > Function/ASE > > and Software Identifier byte in HID++ 2.0 commands. > > > > As per the "Protocol HID++2.0 essential features" section in > > https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf > > " > > Software identifier (4 bits, unsigned) > > > > A number uniquely defining the software that sends a request. The > > firmware must copy the software identifier in the response but does > > not use it in any other ways. > > > > 0 Do not use (allows to distinguish a notification from a > > response). > > " > > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215699 > > Signed-off-by: Bastien Nocera <hadess@xxxxxxxxxx> > > --- > > drivers/hid/hid-logitech-hidpp.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid- > > logitech-hidpp.c > > index 98ebedb73d98..9c8088d8879e 100644 > > --- a/drivers/hid/hid-logitech-hidpp.c > > +++ b/drivers/hid/hid-logitech-hidpp.c > > @@ -41,6 +41,9 @@ module_param(disable_tap_to_click, bool, 0644); > > MODULE_PARM_DESC(disable_tap_to_click, > > "Disable Tap-To-Click mode reporting for touchpads (only on > > the K400 currently)."); > > > > +/* Define a non-zero software ID to identify our own requests */ > > +#define LINUX_KERNEL_SW_ID 0x06 > > For consistency, and as Peter already asked, please use 0x01 instead > of 0x06. > > The simple reason is that it was well known that the kernel used 0x01 > from day one, and so we might have userspace application that uses > 0x06, and in this case you are walking on their toes. Done in v4, thanks. > > Cheers, > Benjamin > > > + > > #define REPORT_ID_HIDPP_SHORT 0x10 > > #define REPORT_ID_HIDPP_LONG 0x11 > > #define REPORT_ID_HIDPP_VERY_LONG 0x12 > > @@ -343,7 +346,7 @@ static int hidpp_send_fap_command_sync(struct > > hidpp_device *hidpp, > > else > > message->report_id = REPORT_ID_HIDPP_LONG; > > message->fap.feature_index = feat_index; > > - message->fap.funcindex_clientid = funcindex_clientid; > > + message->fap.funcindex_clientid = funcindex_clientid | > > LINUX_KERNEL_SW_ID; > > memcpy(&message->fap.params, params, param_count); > > > > ret = hidpp_send_message_sync(hidpp, message, response); > > -- > > 2.37.2 > > >
