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

[Curtis Vogt <curtis.vogt@gmail.com>]

On Thu, Sep 10, 2026 at 12:21:15PM -0500, Mario Limonciello wrote:
> 
> 
> On 9/10/26 12:17, sashiko-bot@kernel.org wrote:
> > 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 <mario.limonciello@amd.com>
> > 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
> > 
> > > 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
> > [ ... ]
> > > +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.
> > 
> > [ ... ]
> > > +	/* 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.
> > 
> > > +	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