Thread (21 messages) 21 messages, 5 authors, 16d ago

Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions

From: Curtis Vogt <hidden>
Date: 2026-09-11 04:55:40
Also in: sashiko-reviews

On Thu, Sep 10, 2026 at 12:21:15PM -0500, Mario Limonciello wrote:

On 9/10/26 12:17, sashiko-bot@kernel.org wrote:
quoted
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect buffer formatting and report ID targeting in `hid_hw_raw_request` fallback for an unnumbered HID report.
--

commit 4526fbce2701e5a40089fba6ae186ded76bc0864
Author: Mario Limonciello [off-list ref]
Subject: HID: valve-index: Reboot headset on system power transitions

This commit introduces a new HID driver for the Valve Index headset to
reboot it on system power transitions. It sends a specific 64-byte report
to recover the EDID service after a DisplayPort link shutdown.

Link: https://github.com/ValveSoftware/SteamVR-for-Linux/issues/939
quoted
diff --git a/drivers/hid/hid-valve-index.c b/drivers/hid/hid-valve-index.c
new file mode 100644
--- /dev/null
+++ b/drivers/hid/hid-valve-index.c
[ ... ]
quoted
+static void valve_index_reboot(struct hid_device *hdev, bool wake)
+{
+	u8 *report;
+	int ret;
+
+	if (!valve_index_has_reboot_report(hdev))
+		return;
+
+	/* USB transfer buffers must be DMA-able, so not on the stack. */
+	report = kzalloc(VALVE_INDEX_REPORT_SIZE, GFP_KERNEL);
+	if (!report)
+		return;
+	report[0] = VALVE_INDEX_REBOOT_REPORT_ID;
[Severity: Medium]
Is the report buffer sized correctly for an unnumbered report fallback?

The HID core and usbhid transport expect unnumbered report buffers to be
prefixed with a dummy 0x00 byte (so payload size + 1) for control transfers.
Allocating exactly 64 bytes and placing the payload command at index 0 might
break control transfer formatting in usbhid_set_raw_report if the fallback
path below is taken.

[ ... ]
quoted
+	/* Use the same interrupt-out then SET_REPORT fallback as hidraw. */
+	ret = hid_hw_output_report(hdev, report, VALVE_INDEX_REPORT_SIZE);
+	if (ret == -ENOSYS)
+		ret = hid_hw_raw_request(hdev, report[0], report,
+					 VALVE_INDEX_REPORT_SIZE,
+					 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
[Severity: Medium]
Does this properly target the unnumbered report during the fallback?

By passing report[0] (which contains 0x16) as the reportnum argument, the
fallback control transfer will target Output Report ID 0x16 instead of the
required unnumbered report (ID 0). If hid_hw_output_report returns -ENOSYS
and this fallback is taken, the headset might reject the transfer, causing
the reboot command to fail.
quoted
+	if (ret < 0)
+		hid_warn(hdev, "failed to reboot headset: %d\n", ret);
[ ... ]
Curtis - can you check how the headset handles those cases?
I'll dig into this
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help