| Message ID | 20220829134852.312548-1-hadess@hadess.net |
|---|---|
| State | Accepted |
| Commit | f7b7393cc3b04e288de85790b1c8e62af99799c2 |
| Headers | show |
| Series | [RFC,v1,1/2] HID: logitech-hidpp: Fix "Sw. Id." for HID++ 2.0 commands | expand |
On 8/29/22 09:48, Bastien Nocera wrote: > Some HID++ 2.0 commands had correctly set a non-zero software identifier > directly as part of their function identifiers, but it's more correct to > define the function identifier and the software identifier separately > before combined them when the command is sent. > > As this is now done in the previous commit, remove the hard-coded 0x1 > software identifiers in the function definitions. > > Signed-off-by: Bastien Nocera <hadess@hadess.net> > --- > drivers/hid/hid-logitech-hidpp.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c > index 02f8c99672c7..46b3e51cb854 100644 > --- a/drivers/hid/hid-logitech-hidpp.c > +++ b/drivers/hid/hid-logitech-hidpp.c > @@ -859,8 +859,8 @@ static int hidpp_unifying_init(struct hidpp_device *hidpp) > #define HIDPP_PAGE_ROOT 0x0000 > #define HIDPP_PAGE_ROOT_IDX 0x00 > > -#define CMD_ROOT_GET_FEATURE 0x01 > -#define CMD_ROOT_GET_PROTOCOL_VERSION 0x11 > +#define CMD_ROOT_GET_FEATURE 0x00 > +#define CMD_ROOT_GET_PROTOCOL_VERSION 0x10 > > static int hidpp_root_get_feature(struct hidpp_device *hidpp, u16 feature, > u8 *feature_index, u8 *feature_type) > @@ -937,9 +937,9 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp) > > #define HIDPP_PAGE_GET_DEVICE_NAME_TYPE 0x0005 > > -#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 > +#define CMD_GET_DEVICE_NAME_TYPE_GET_COUNT 0x00 > +#define CMD_GET_DEVICE_NAME_TYPE_GET_DEVICE_NAME 0x10 > +#define CMD_GET_DEVICE_NAME_TYPE_GET_TYPE 0x20 > > static int hidpp_devicenametype_get_count(struct hidpp_device *hidpp, > u8 feature_index, u8 *nameLength) > @@ -1969,8 +1969,8 @@ static int hidpp_touchpad_fw_items_set(struct hidpp_device *hidpp, > > #define HIDPP_PAGE_TOUCHPAD_RAW_XY 0x6100 > > -#define CMD_TOUCHPAD_GET_RAW_INFO 0x01 > -#define CMD_TOUCHPAD_SET_RAW_REPORT_STATE 0x21 > +#define CMD_TOUCHPAD_GET_RAW_INFO 0x00 > +#define CMD_TOUCHPAD_SET_RAW_REPORT_STATE 0x20 > > #define EVENT_TOUCHPAD_RAW_XY 0x00 > Looks good to me. peter
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 86e7a38d8a9a..02f8c99672c7 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 + #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);
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@hadess.net> --- drivers/hid/hid-logitech-hidpp.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)