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

Aditya Dash posted 6 patches 3 weeks, 6 days ago
There is a newer version of this series
drivers/hid/hid-lenovo-go.c | 179 +++++++++++++++++++++++++++---------
1 file changed, 136 insertions(+), 43 deletions(-)
[PATCH v2 0/6] HID: hid-lenovo-go: fix controller configuration
Posted by Aditya Dash 3 weeks, 6 days ago
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@gmail.com/


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
Re: [PATCH v2 0/6] HID: hid-lenovo-go: fix controller configuration
Posted by Derek John Clark 3 weeks, 4 days ago
On Sun, Aug 30, 2026 at 7:57 AM Aditya Dash <mradityadash@gmail.com> 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@gmail.com/
>
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
>