[PATCH v4] HID: vivaldi: avoid redundant feature report reads per usage

Zhengping Jiang posted 1 patch 1 week, 2 days ago
drivers/hid/hid-vivaldi-common.c | 33 +++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
[PATCH v4] HID: vivaldi: avoid redundant feature report reads per usage
Posted by Zhengping Jiang 1 week, 2 days ago
vivaldi_feature_mapping() is invoked by report_features() in ascending
usage_index order for each usage in the top-row physical keymap field.
Because fetching the feature report at usage_index == 0 already reads
the entire report from hardware, calling hid_hw_raw_request() again for
subsequent usages redundantly reads the same report N times (e.g., 12
sequential BLE GATT reads taking ~2s on Vivaldi BLE keyboards).

Bail out immediately when usage->usage_index != 0, fetch the feature
report once at usage_index == 0, and iterate over all usages in the
field to populate data->function_row_physmap[].

Fixes: 33bbe04a15f2 ("HID: google: extract Vivaldi hid feature mapping for use in hid-hammer")
Signed-off-by: Zhengping Jiang <jiangzp@google.com>
---
Based on "[PATCH v2] HID: vivaldi: Extract feature report value directly in vivaldi_feature_mapping()":
https://lore.kernel.org/r/20261013-upstream-vivaldi-fix-v2-1-ecc20329d24c@chromium.org

Changes in v4:
- Rebase on top of Shashikirana's patch using hid_field_extract() and
  __free(kfree), as suggested by Dmitry Torokhov.
- Return immediately when usage->usage_index != 0, fetch the report once
  at usage_index == 0, and loop over all usages in the field so failed
  reads do not update num_function_row_keys or expose an all-zero keymap.
- Fix caller name in commit description (report_features()).

Changes in v3:
- Add comment explaining why the report is only read at usage_index == 0
  and how failed reads rely on zero-initialized field->value[].
- Jump/fall through to parse_usage on read failure.

Changes in v2:
- Move per-usage validation (HID_UP_ORDINAL and fn_key bounds) after the
  usage_index == 0 hardware read.

 drivers/hid/hid-vivaldi-common.c | 33 +++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-vivaldi-common.c b/drivers/hid/hid-vivaldi-common.c
index 1359a107e63b..973393d09356 100644
--- a/drivers/hid/hid-vivaldi-common.c
+++ b/drivers/hid/hid-vivaldi-common.c
@@ -39,19 +39,11 @@ void vivaldi_feature_mapping(struct hid_device *hdev,
 	unsigned int offset;
 	u32 report_len;
 	int ret;
+	int i;
 
-	if (field->logical != HID_USAGE_FN_ROW_PHYSMAP ||
-	    (usage->hid & HID_USAGE_PAGE) != HID_UP_ORDINAL ||
-	    usage->usage_index >= field->report_count)
+	if (field->logical != HID_USAGE_FN_ROW_PHYSMAP || usage->usage_index)
 		return;
 
-	fn_key = usage->hid & HID_USAGE;
-	if (fn_key < MIN_FN_ROW_KEY || fn_key > MAX_FN_ROW_KEY)
-		return;
-
-	if (fn_key > data->num_function_row_keys)
-		data->num_function_row_keys = fn_key;
-
 	u8 *buf __free(kfree) = hid_alloc_report_buf(report, GFP_KERNEL);
 	if (!buf)
 		return;
@@ -74,9 +66,24 @@ void vivaldi_feature_mapping(struct hid_device *hdev,
 		return;
 	}
 
-	offset = field->report_offset + usage->usage_index * field->report_size;
-	data->function_row_physmap[fn_key - MIN_FN_ROW_KEY] =
-		hid_field_extract(hdev, buf + 1, offset, field->report_size);
+	for (i = 0; i < field->maxusage && i < field->report_count; i++) {
+		usage = &field->usage[i];
+
+		if ((usage->hid & HID_USAGE_PAGE) != HID_UP_ORDINAL)
+			continue;
+
+		fn_key = usage->hid & HID_USAGE;
+		if (fn_key < MIN_FN_ROW_KEY || fn_key > MAX_FN_ROW_KEY)
+			continue;
+
+		if (fn_key > data->num_function_row_keys)
+			data->num_function_row_keys = fn_key;
+
+		offset = field->report_offset + i * field->report_size;
+		data->function_row_physmap[fn_key - MIN_FN_ROW_KEY] =
+			hid_field_extract(hdev, buf + 1, offset,
+					  field->report_size);
+	}
 }
 EXPORT_SYMBOL_GPL(vivaldi_feature_mapping);
 
-- 
2.56.0.385.gd3acb90ef8-goog