From 64037febdcb8ca3e2fc363b87fa711f4577f58a7 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Mon, 9 May 2016 19:51:46 +0200 Subject: [PATCH 1/6] hidpp20: fix return path for hidpp20_onboard_profiles_read A negative error means a communication error, while a positive one means a HID++ one. Make sure we abort early enough. Signed-off-by: Benjamin Tissoires --- src/hidpp20.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hidpp20.c b/src/hidpp20.c index 2b707a2b..b922ab8a 100644 --- a/src/hidpp20.c +++ b/src/hidpp20.c @@ -1571,7 +1571,7 @@ hidpp20_onboard_profiles_find_and_read_profile(struct hidpp20_device *device, index + 1, i * 0x10, data + i * 0x10); - if (rc < 0) + if (rc != 0) return rc; } @@ -1761,7 +1761,7 @@ int hidpp20_onboard_profiles_read(struct hidpp20_device *device, return -EINVAL; rc = hidpp20_onboard_profiles_find_and_read_profile(device, index, data); - if (rc < 0) + if (rc != 0) return rc; profile->report_rate = 1000 / max(1, pdata.profile.report_rate); From c14a0ca2a15338092a9a04e2dde56bf9bf9f9a8f Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Mon, 9 May 2016 19:52:56 +0200 Subject: [PATCH 2/6] hidpp20-drv: prevent an empty profile to try to read a null macro If the macro has not been retrieved, it's more likely that the button is not bound to anything. Signed-off-by: Benjamin Tissoires --- src/driver-hidpp20.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index a09c34de..62bbeb3a 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -117,7 +117,7 @@ hidpp20drv_read_macro_key_8100(struct ratbag_device *device, union hidpp20_macro return ratbag_hidraw_get_keycode_from_keyboard_usage(device, macro->key.key); } -static void +static int hidpp20drv_read_macro_8100(struct ratbag_button *button, struct hidpp20_profile *profile, union hidpp20_button_binding *binding) @@ -130,6 +130,9 @@ hidpp20drv_read_macro_8100(struct ratbag_button *button, macro = profile->macros[binding->macro.page]; + if (!macro) + return -EINVAL; + i = 0; m = ratbag_button_macro_new("macro"); @@ -175,6 +178,8 @@ hidpp20drv_read_macro_8100(struct ratbag_button *button, ratbag_button_copy_macro(button, m); ratbag_button_macro_unref(m); + + return 0; } static void @@ -183,6 +188,7 @@ hidpp20drv_read_button_8100(struct ratbag_button *button) struct ratbag_device *device = button->profile->device; struct hidpp20drv_data *drv_data = ratbag_get_drv_data(device); struct hidpp20_profile *profile; + int rc; if (!(drv_data->capabilities & HIDPP_CAP_ONBOARD_PROFILES_8100)) return; @@ -213,7 +219,9 @@ hidpp20drv_read_button_8100(struct ratbag_button *button) button->action.action.special = hidpp20_onboard_profiles_get_special(profile->buttons[button->index].special.special); break; case HIDPP20_BUTTON_MACRO: - hidpp20drv_read_macro_8100(button, profile, &profile->buttons[button->index]); + rc = hidpp20drv_read_macro_8100(button, profile, &profile->buttons[button->index]); + if (rc) + button->action.type = RATBAG_BUTTON_ACTION_TYPE_NONE; break; default: button->action.type = RATBAG_BUTTON_ACTION_TYPE_UNKNOWN; From fc076729837dfe780ee7d71f36d3d94064907e07 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Mon, 9 May 2016 20:08:18 +0200 Subject: [PATCH 3/6] hidpp20: prevent invalid ROM access The device can report how much rom it has through profile_count_oob. Make sure we have something in ROM before accessing it. Signed-off-by: Benjamin Tissoires --- src/hidpp20.c | 20 +++++++++++++++++--- src/hidpp20.h | 1 + 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/hidpp20.c b/src/hidpp20.c index b922ab8a..9eece6f5 100644 --- a/src/hidpp20.c +++ b/src/hidpp20.c @@ -1283,6 +1283,7 @@ hidpp20_onboard_profiles_initialize(struct hidpp20_device *device, info->profile_count * sizeof(struct hidpp20_profile)); profiles->num_profiles = info->profile_count; + profiles->num_rom_profiles = info->profile_count_oob; profiles->num_buttons = msg.msg.parameters[5] <= 16 ? msg.msg.parameters[5] : 16; profiles->num_modes = HIDPP20_DPI_COUNT; profiles->has_g_shift = (info->mechanical_layout & 0x03) == 2; @@ -1532,7 +1533,8 @@ hidpp20_onboard_profiles_destroy(struct hidpp20_device *device, static int hidpp20_onboard_profiles_find_and_read_profile(struct hidpp20_device *device, unsigned int index, - uint8_t *data) + uint8_t *data, + unsigned int num_rom_profiles) { int rc; unsigned i; @@ -1565,6 +1567,12 @@ hidpp20_onboard_profiles_find_and_read_profile(struct hidpp20_device *device, } /* something went wrong, the mouse is using the factory profile in ROM */ + if (num_rom_profiles == 0) + return -EINVAL; + + if (index >= num_rom_profiles) + index = num_rom_profiles - 1; + for (i = 0; i < HIDPP20_PROFILE_SIZE / 0x10; i++) { rc = hidpp20_onboard_profiles_read_memory(device, 1, @@ -1760,7 +1768,10 @@ int hidpp20_onboard_profiles_read(struct hidpp20_device *device, if (index >= profiles_list->num_profiles) return -EINVAL; - rc = hidpp20_onboard_profiles_find_and_read_profile(device, index, data); + rc = hidpp20_onboard_profiles_find_and_read_profile(device, + index, + data, + profiles_list->num_rom_profiles); if (rc != 0) return rc; @@ -1802,7 +1813,10 @@ int hidpp20_onboard_profiles_write(struct hidpp20_device *device, if (index >= profiles_list->num_profiles) return -EINVAL; - rc = hidpp20_onboard_profiles_find_and_read_profile(device, index, data); + rc = hidpp20_onboard_profiles_find_and_read_profile(device, + index, + data, + profiles_list->num_rom_profiles); if (rc < 0) return rc; diff --git a/src/hidpp20.h b/src/hidpp20.h index eb78480f..42f2db6a 100644 --- a/src/hidpp20.h +++ b/src/hidpp20.h @@ -362,6 +362,7 @@ struct hidpp20_profile { struct hidpp20_profiles { uint8_t num_profiles; + uint8_t num_rom_profiles; uint8_t num_buttons; uint8_t num_modes; uint8_t has_g_shift; From 27293a22df198b60900a19d222225d8f9a308866 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Mon, 9 May 2016 20:09:24 +0200 Subject: [PATCH 4/6] hidpp20: enable G900 profile type The profile is retro-compatible with the rest, so we can access it in the same way. Signed-off-by: Benjamin Tissoires --- src/hidpp20.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hidpp20.c b/src/hidpp20.c index 9eece6f5..0a534fc7 100644 --- a/src/hidpp20.c +++ b/src/hidpp20.c @@ -929,6 +929,7 @@ int hidpp20_adjustable_dpi_set_sensor_dpi(struct hidpp20_device *device, #define HIDPP20_ONBOARD_PROFILES_MEMORY_TYPE_G402 0x01 #define HIDPP20_ONBOARD_PROFILES_PROFILE_TYPE_G402 0x01 #define HIDPP20_ONBOARD_PROFILES_PROFILE_TYPE_G303 0x02 +#define HIDPP20_ONBOARD_PROFILES_PROFILE_TYPE_G900 0x03 #define HIDPP20_ONBOARD_PROFILES_MACRO_TYPE_G402 0x01 struct hidpp20_onboard_profiles_info { @@ -1242,7 +1243,8 @@ hidpp20_onboard_profiles_initialize(struct hidpp20_device *device, } if ((info->profile_format_id != HIDPP20_ONBOARD_PROFILES_PROFILE_TYPE_G402) && - (info->profile_format_id != HIDPP20_ONBOARD_PROFILES_PROFILE_TYPE_G303)) { + (info->profile_format_id != HIDPP20_ONBOARD_PROFILES_PROFILE_TYPE_G303) && + (info->profile_format_id != HIDPP20_ONBOARD_PROFILES_PROFILE_TYPE_G900)) { hidpp_log_error(&device->base, "Profile layout not supported: 0x%02x.\n", info->profile_format_id); @@ -1301,8 +1303,6 @@ hidpp20_onboard_profiles_initialize(struct hidpp20_device *device, break; } - - *profiles_list = profiles; return 0; From c1bf3be48ef3c5712844eb10e50ed05d725cb733 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 10 May 2016 11:13:34 +0200 Subject: [PATCH 5/6] hidpp20: read/write the whole dictionary, not just the first 16 bytes The G900 advertises 5 profiles, which do not fit in only 16 bytes. We need to read at least 32 bytes, so be it. Signed-off-by: Benjamin Tissoires --- src/hidpp20.c | 49 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/src/hidpp20.c b/src/hidpp20.c index 0a534fc7..c999fd52 100644 --- a/src/hidpp20.c +++ b/src/hidpp20.c @@ -1468,13 +1468,29 @@ hidpp20_onboard_profiles_parse_macro(struct hidpp20_device *device, return 0; } +static unsigned int +hidpp20_onboard_profiles_compute_dict_size(struct hidpp20_device *device, + struct hidpp20_profiles *profiles) +{ + unsigned p, num_offset; + + num_offset = 0; + p = profiles->num_profiles; + while (p) { + p >>= 2; + num_offset += 16; + } + + return num_offset; +} + int hidpp20_onboard_profiles_allocate(struct hidpp20_device *device, struct hidpp20_profiles **profiles_list) { - unsigned i; + unsigned i, offset, num_offset; int rc; - uint8_t data[16] = {0}; + uint8_t data[HIDPP20_PAGE_SIZE] = {0}; struct hidpp20_profiles *profiles = NULL; rc = hidpp20_onboard_profiles_initialize(device, @@ -1484,16 +1500,21 @@ hidpp20_onboard_profiles_allocate(struct hidpp20_device *device, assert(profiles); - rc = hidpp20_onboard_profiles_read_memory(device, - 0x00, - 0x00, - 0x00, - data); - if (rc < 0) - return rc; + num_offset = hidpp20_onboard_profiles_compute_dict_size(device, + profiles); + + for (offset = 0; offset < num_offset; offset += 16) { + rc = hidpp20_onboard_profiles_read_memory(device, + 0x00, + 0x00, + offset, + data + offset); + if (rc < 0) + return rc; + } for (i = 0; i < profiles->num_profiles; i++) { - uint8_t *d = data + 4 * i; + uint8_t *d = data + offset + 4 * i; if (d[0] == 0xFF && d[1] == 0xFF) break; @@ -1600,7 +1621,7 @@ hidpp20_onboard_profiles_enable_profile(struct hidpp20_device *device, profiles_list->profiles[index].index = index + 1; profiles_list->profiles[index].enabled = 0x01; - for (i = 0; i < 3; i++) { + for (i = 0; i < profiles_list->num_profiles; i++) { data[buffer_index++] = 0x00; data[buffer_index++] = i + 1; data[buffer_index++] = profiles_list->profiles[i].enabled; @@ -1615,7 +1636,11 @@ hidpp20_onboard_profiles_enable_profile(struct hidpp20_device *device, memset(data + buffer_index, 0xff, sizeof(data) - buffer_index); - hidpp_log_buf_error(&device->base, "dictionary: ", data, 16); + hidpp_log_buf_debug(&device->base, + "dictionary: ", + data, + hidpp20_onboard_profiles_compute_dict_size(device, + profiles_list)); return hidpp20_onboard_profiles_write_page(device, 0x00, data); } From 28438e638b7c3ff39728b3ba734d404381aff502 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 10 May 2016 11:35:17 +0200 Subject: [PATCH 6/6] hidpp20-drv: actually retrieve the current resolution value The onboard profile do not store the current value but the default. To retrieve the current value, we need to access the switchable resolution feature. Signed-off-by: Benjamin Tissoires --- src/driver-hidpp20.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index 62bbeb3a..2ff5e6bb 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -592,7 +592,7 @@ hidpp20drv_read_profile_8100(struct ratbag_profile *profile, unsigned int index) struct hidpp20drv_data *drv_data = ratbag_get_drv_data(device); struct ratbag_resolution *res; struct hidpp20_profile *p; - unsigned i, dpi; + unsigned i, dpi = 0; hidpp20drv_read_onboard_profile(device, profile->index); @@ -600,6 +600,10 @@ hidpp20drv_read_profile_8100(struct ratbag_profile *profile, unsigned int index) if ((int)index == hidpp20drv_current_profile(device)) profile->is_active = true; + /* retrieve the resolution through 220X as the profile doesn't has it */ + if (profile->is_active) + hidpp20drv_read_resolution_dpi(profile); + dpi = ratbag_resolution_get_dpi(profile->resolution.modes); p = &drv_data->profiles->profiles[index];