Re: [PATCH v2 0/6] HID: hid-lenovo-go: fix controller configuration

[Derek John Clark]

On Sun, Aug 30, 2026 at 7:57 AM Aditya Dash <mradityadash@xxxxxxxxx> wrote:
>
> This series fixes several issues in the controller configuration and
> calibration paths in hid-lenovo-go.
>
> 1/6 fixes the calibration attributes, where the command and sub-command
> are swapped. 2/6 fixes two right-controller attributes: imu_enabled
> selects the bypass feature, and reset targets the left controller.
>
> Before 3/6, any decoded configuration reply completes the shared
> completion. The driver then ignores the result of the wait, so an
> unrelated reply, a firmware error, an interrupted wait, or a timeout can
> still be reported as success.
>
> 3/6 tracks one pending request in the driver data and matches replies by
> command ID, command, sub-command, and device. It also returns request
> errors to the caller.
>
> The firmware has no sequence number, so a late reply with the same tuple
> can still match a newer request.
>
> The remaining patches fix calibration status handling.
>
> The calibration status table defines 0x00 as unknown, 0x01 as success,
> and 0x02 as failure. A Legion Go 1 returned 0x08 after a Stop was sent
> while no calibration was active. The driver stores the raw value as a
> status-text index, so a later read returns -EINVAL.
>
> 4/6 preserves the defined values and maps larger status values to
> failure.
>
> 5/6 rejects 'unknown' as an action. The action table contains it at index
> zero, but the options attribute advertises only 'start' and 'stop'.
>
> A previous result can also hide a new calibration attempt:
>
> failure -> start -> failure
>
> The status appears unchanged for the whole operation. 6/6 clears the
> selected status to unknown before submitting a Start. Stop leaves the
> status unchanged.
>
> ---
> Changes in v2:
> - Drop the FPS DPI patch; the existing request is correct.
> - Drop the v1 Boolean calibration-result mapping. Preserve values 0x00
>   through 0x02 and map only larger failure values.
> - Move the existing pending command state into the driver data and
>   initialize it in cfg_probe().
> - Use scoped IRQ-save guards and validate the report size and ID together.
> - Drop the v1 no-wait calibration path. Correct requests receive their
>   matching SET replies within the existing 50 ms timeout.
> - Reject 'unknown' and clear only the selected status on Start.
>
> v1: https://lore.kernel.org/all/20260821214810.87826-1-mradityadash@xxxxxxxxx/
>
Hi Aditya,

I tested the series and have no complaints, everything seems to work
well. Thank you for working on these bugs. You can ignore the existing
issues that sashiko-bot identified for your contribution, I'll take
care of those separately.
There were 2 new issues that need to be fixed:
- [High] The newly added spinlock and completion are accessed by the
raw event handler before they are initialized, causing a potential
kernel crash.
- [Low] The patch leaves behind dead code (`if (!val) size = 0;`)
after explicitly rejecting `CAL_UNKNOWN`.

The first one is more pressing, and will require a reordering of probe
in an additional patch to ensure drvdata is init before hid_hw_start()
and hid_hw_open() are called. The bug is admittedly very unlikely to
occur, but it is the kind of bug that could cause spurious crashes
during boot that would be very difficult to trace.

The second should be trivial to fix

Once fixed I'll add by review and tested by for the series.

Thanks,
Derek

> Aditya Dash (6):
>   HID: hid-lenovo-go: use the correct calibration commands
>   HID: hid-lenovo-go: use the right controller selectors
>   HID: hid-lenovo-go: return configuration request errors
>   HID: hid-lenovo-go: normalize calibration failure status
>   HID: hid-lenovo-go: reject unknown calibration action
>   HID: hid-lenovo-go: clear calibration status on start
>
>  drivers/hid/hid-lenovo-go.c | 179 +++++++++++++++++++++++++++---------
>  1 file changed, 136 insertions(+), 43 deletions(-)
>
>
> base-commit: 1292bca0f8d835d2ad96d309595b2e97f3106d3d
> --
> 2.55.0
>