Linux Input/HID development
 help / color / mirror / Atom feed
From: Zhengping Jiang <jiangzp@google.com>
To: Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <bentiss@kernel.org>,
	 Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: "Sean O'Brien" <seobrien@chromium.org>,
	Shashikirana H K <shashikiranah@chromium.org>,
	 linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Zhengping Jiang <jiangzp@google.com>
Subject: [PATCH v4] HID: vivaldi: avoid redundant feature report reads per usage
Date: Thu, 15 Oct 2026 06:42:23 +0000	[thread overview]
Message-ID: <20261015064223.1939367-1-jiangzp@google.com> (raw)
In-Reply-To: <01c8cf7d695261ba8270ff57aa8f2a6f@gmail.com>

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


      reply	other threads:[~2026-10-15  6:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-12 21:29 [PATCH] HID: vivaldi: avoid redundant feature report reads per usage Zhengping Jiang
2026-10-12 21:37 ` sashiko-bot
2026-10-12 22:02 ` [PATCH v2] " Zhengping Jiang
2026-10-13 19:23   ` [PATCH v3] " Zhengping Jiang
2026-10-15  5:32     ` Dmitry Torokhov
2026-10-15  6:42       ` Zhengping Jiang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261015064223.1939367-1-jiangzp@google.com \
    --to=jiangzp@google.com \
    --cc=bentiss@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seobrien@chromium.org \
    --cc=shashikiranah@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox