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

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

[Aditya Dash]

The hid-lenovo-go driver exposes controller configuration attributes, but
several requests use incorrect fields or report the wrong result. Calibration
uses reversed command fields, FPS mode DPI includes an extra device byte,
firmware calibration failure appears as unknown, and two right-controller
attributes use the wrong selectors.

The first four patches correct those declarations and request layouts. The
last patch fixes the shared synchronous request path so that only the matching
reply completes a request and transport, firmware, interruption, and timeout
errors reach the caller. It uses the corrected calibration command pairs from
the first patch.

Calibration completion is asynchronous. The driver now submits those commands
without waiting for an immediate reply and continues to receive final state
through the existing completion report. Firmware does not provide a sequence
number, so a late reply with the same complete command tuple remains
indistinguishable from a newer reply.

Aditya Dash (5):
  HID: hid-lenovo-go: use the correct calibration commands
  HID: hid-lenovo-go: fix the FPS mode DPI request
  HID: hid-lenovo-go: record calibration completion state
  HID: hid-lenovo-go: use the right controller selectors
  HID: hid-lenovo-go: return configuration request errors

 drivers/hid/hid-lenovo-go.c | 165 +++++++++++++++++++++++++++---------
 1 file changed, 127 insertions(+), 38 deletions(-)


base-commit: 1292bca0f8d835d2ad96d309595b2e97f3106d3d
-- 
2.55.0

[PATCH 1/5] HID: hid-lenovo-go: use the correct calibration commands

[Aditya Dash]

The calibration attributes pass the calibration operation as the command
and the configuration command as the sub-command. This sends each request
with the two fields reversed.

Swap the command and sub-command values for all six calibration attributes.
For example, the malformed left joystick Start request is:

  05 00 04 0c 03 01 ...

The corrected request is:

  05 00 0c 04 03 01 ...

The bytes identify the output report, configuration group, command,
sub-command, left controller, and Start action, in that order.

Fixes: 995887a10da1 ("HID: hid-lenovo-go: Add Calibration Settings")
Assisted-by: Pi:gpt-5.6-sol
Signed-off-by: Aditya Dash <mradityadash@gmail.com>
---
 drivers/hid/hid-lenovo-go.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
index 318b1152ff8b..b8248bf8965c 100644
--- a/drivers/hid/hid-lenovo-go.c
+++ b/drivers/hid/hid-lenovo-go.c
@@ -1911,18 +1911,18 @@ LEGO_DEVICE_ATTR_RW(rumble_notification_left, "rumble_notification",
 static DEVICE_ATTR_RO_NAMED(rumble_notification_left_index,
 			    "rumble_notification_index");
 
-static struct go_cfg_attr cal_trigg_left = { TRIGGER_CALIBRATE };
-LEGO_CAL_DEVICE_ATTR(cal_trigg_left, "calibrate_trigger", SET_TRIGGER_CFG,
+static struct go_cfg_attr cal_trigg_left = { SET_TRIGGER_CFG };
+LEGO_CAL_DEVICE_ATTR(cal_trigg_left, "calibrate_trigger", TRIGGER_CALIBRATE,
 		     LEFT_CONTROLLER, index);
 static DEVICE_ATTR_RO_NAMED(cal_trigg_left_index, "calibrate_trigger_index");
 
-static struct go_cfg_attr cal_joy_left = { JOYSTICK_CALIBRATE };
-LEGO_CAL_DEVICE_ATTR(cal_joy_left, "calibrate_joystick", SET_JOYSTICK_CFG,
+static struct go_cfg_attr cal_joy_left = { SET_JOYSTICK_CFG };
+LEGO_CAL_DEVICE_ATTR(cal_joy_left, "calibrate_joystick", JOYSTICK_CALIBRATE,
 		     LEFT_CONTROLLER, index);
 static DEVICE_ATTR_RO_NAMED(cal_joy_left_index, "calibrate_joystick_index");
 
-static struct go_cfg_attr cal_gyro_left = { GYRO_CALIBRATE };
-LEGO_CAL_DEVICE_ATTR(cal_gyro_left, "calibrate_gyro", SET_GYRO_CFG,
+static struct go_cfg_attr cal_gyro_left = { SET_GYRO_CFG };
+LEGO_CAL_DEVICE_ATTR(cal_gyro_left, "calibrate_gyro", GYRO_CALIBRATE,
 		     LEFT_CONTROLLER, index);
 static DEVICE_ATTR_RO_NAMED(cal_gyro_left_index, "calibrate_gyro_index");
 
@@ -2018,18 +2018,18 @@ LEGO_DEVICE_ATTR_RW(rumble_notification_right, "rumble_notification",
 static DEVICE_ATTR_RO_NAMED(rumble_notification_right_index,
 			    "rumble_notification_index");
 
-static struct go_cfg_attr cal_trigg_right = { TRIGGER_CALIBRATE };
-LEGO_CAL_DEVICE_ATTR(cal_trigg_right, "calibrate_trigger", SET_TRIGGER_CFG,
+static struct go_cfg_attr cal_trigg_right = { SET_TRIGGER_CFG };
+LEGO_CAL_DEVICE_ATTR(cal_trigg_right, "calibrate_trigger", TRIGGER_CALIBRATE,
 		     RIGHT_CONTROLLER, index);
 static DEVICE_ATTR_RO_NAMED(cal_trigg_right_index, "calibrate_trigger_index");
 
-static struct go_cfg_attr cal_joy_right = { JOYSTICK_CALIBRATE };
-LEGO_CAL_DEVICE_ATTR(cal_joy_right, "calibrate_joystick", SET_JOYSTICK_CFG,
+static struct go_cfg_attr cal_joy_right = { SET_JOYSTICK_CFG };
+LEGO_CAL_DEVICE_ATTR(cal_joy_right, "calibrate_joystick", JOYSTICK_CALIBRATE,
 		     RIGHT_CONTROLLER, index);
 static DEVICE_ATTR_RO_NAMED(cal_joy_right_index, "calibrate_joystick_index");
 
-static struct go_cfg_attr cal_gyro_right = { GYRO_CALIBRATE };
-LEGO_CAL_DEVICE_ATTR(cal_gyro_right, "calibrate_gyro", SET_GYRO_CFG,
+static struct go_cfg_attr cal_gyro_right = { SET_GYRO_CFG };
+LEGO_CAL_DEVICE_ATTR(cal_gyro_right, "calibrate_gyro", GYRO_CALIBRATE,
 		     RIGHT_CONTROLLER, index);
 static DEVICE_ATTR_RO_NAMED(cal_gyro_right_index, "calibrate_gyro_index");
 
-- 
2.55.0

[PATCH 2/5] HID: hid-lenovo-go: fix the FPS mode DPI request

[Aditya Dash]

mcu_property_out() adds the five-byte request header used by most
configuration commands. For FPS mode DPI writes, this inserts a device byte
before the four-byte DPI value.

The FPS mode DPI request does not contain a device byte. Its little-endian
DPI value starts immediately after the command and sub-command.

Use the four-byte header only for FPS mode DPI writes. Keep the common
header for all other requests.

Fixes: f0bedee60607 ("HID: hid-lenovo-go: Add FPS Mode DPI settings")
Assisted-by: Pi:gpt-5.6-sol
Signed-off-by: Aditya Dash <mradityadash@gmail.com>
---
The corrected FPS mode DPI write was not tested on hardware.

 drivers/hid/hid-lenovo-go.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
index b8248bf8965c..a6e9a130ad41 100644
--- a/drivers/hid/hid-lenovo-go.c
+++ b/drivers/hid/hid-lenovo-go.c
@@ -725,6 +725,11 @@ static int mcu_property_out(struct hid_device *hdev, u8 id, u8 command,
 	int timeout = 50;
 	int ret;
 
+	/* The FPS mode DPI request does not contain a device byte. */
+	if (id == MCU_CONFIG_DATA && command == SET_DPI_CFG &&
+	    index == FPS_MODE_DPI)
+		header_size--;
+
 	if (header_size + len > GO_PACKET_SIZE)
 		return -EINVAL;
 
-- 
2.55.0

[PATCH 3/5] HID: hid-lenovo-go: record calibration completion state

[Aditya Dash]

The firmware uses result value 0 for failure and 1 for success. The driver
stores this value directly as a state index. Index 0 means unknown, so the
driver reports a firmware failure as unknown.

Map the result value to the matching failure or success state.

Fixes: 995887a10da1 ("HID: hid-lenovo-go: Add Calibration Settings")
Assisted-by: Pi:gpt-5.6-sol
Signed-off-by: Aditya Dash <mradityadash@gmail.com>
---
A successful calibration completion was tested on hardware. No final
failure completion was captured.

 drivers/hid/hid-lenovo-go.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
index a6e9a130ad41..d2d134873dd1 100644
--- a/drivers/hid/hid-lenovo-go.c
+++ b/drivers/hid/hid-lenovo-go.c
@@ -577,17 +577,19 @@ static int hid_go_light_event(struct command_report *cmd_rep)
 
 static int hid_go_device_status_event(struct command_report *cmd_rep)
 {
+	u8 status = cmd_rep->data[1] ? CAL_STAT_SUCCESS : CAL_STAT_FAILURE;
+
 	switch (cmd_rep->device_type) {
 	case LEFT_CONTROLLER:
 		switch (cmd_rep->data[0]) {
 		case CALDEV_GYROSCOPE:
-			drvdata.gp_left_gyro_cal_status = cmd_rep->data[1];
+			drvdata.gp_left_gyro_cal_status = status;
 			return 0;
 		case CALDEV_JOYSTICK:
-			drvdata.gp_left_joy_cal_status = cmd_rep->data[1];
+			drvdata.gp_left_joy_cal_status = status;
 			return 0;
 		case CALDEV_TRIGGER:
-			drvdata.gp_left_trigg_cal_status = cmd_rep->data[1];
+			drvdata.gp_left_trigg_cal_status = status;
 			return 0;
 		default:
 			return -EINVAL;
@@ -596,13 +598,13 @@ static int hid_go_device_status_event(struct command_report *cmd_rep)
 	case RIGHT_CONTROLLER:
 		switch (cmd_rep->data[0]) {
 		case CALDEV_GYROSCOPE:
-			drvdata.gp_right_gyro_cal_status = cmd_rep->data[1];
+			drvdata.gp_right_gyro_cal_status = status;
 			return 0;
 		case CALDEV_JOYSTICK:
-			drvdata.gp_right_joy_cal_status = cmd_rep->data[1];
+			drvdata.gp_right_joy_cal_status = status;
 			return 0;
 		case CALDEV_TRIGGER:
-			drvdata.gp_right_trigg_cal_status = cmd_rep->data[1];
+			drvdata.gp_right_trigg_cal_status = status;
 			return 0;
 		default:
 			return -EINVAL;
-- 
2.55.0

[PATCH 4/5] HID: hid-lenovo-go: use the right controller selectors

[Aditya Dash]

The right imu_enabled attribute selects FEATURE_IMU_BYPASS, and the right
reset attribute targets the left controller.

Select FEATURE_IMU_ENABLE for the right imu_enabled attribute. Target the
right controller for a right reset.

Fixes: 82cd9bc866e1 ("HID: hid-lenovo-go: Add Feature Status Attributes")
Assisted-by: Pi:gpt-5.6-sol
Signed-off-by: Aditya Dash <mradityadash@gmail.com>
---
The corrected right reset and right IMU writes were not tested on
hardware.

 drivers/hid/hid-lenovo-go.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
index d2d134873dd1..1e6915c969d8 100644
--- a/drivers/hid/hid-lenovo-go.c
+++ b/drivers/hid/hid-lenovo-go.c
@@ -2006,13 +2006,13 @@ LEGO_DEVICE_ATTR_RW(imu_bypass_right, "imu_bypass_enabled", RIGHT_CONTROLLER,
 		    index, feature_status);
 static DEVICE_ATTR_RO_NAMED(imu_bypass_right_index, "imu_bypass_enabled_index");
 
-static struct go_cfg_attr imu_enabled_right = { FEATURE_IMU_BYPASS };
+static struct go_cfg_attr imu_enabled_right = { FEATURE_IMU_ENABLE };
 LEGO_DEVICE_ATTR_RW(imu_enabled_right, "imu_enabled", RIGHT_CONTROLLER, index,
 		    feature_status);
 static DEVICE_ATTR_RO_NAMED(imu_enabled_right_index, "imu_enabled_index");
 
 static struct go_cfg_attr reset_right = { FEATURE_RESET_GAMEPAD };
-LEGO_DEVICE_ATTR_WO(reset_right, "reset", LEFT_CONTROLLER, feature_status);
+LEGO_DEVICE_ATTR_WO(reset_right, "reset", RIGHT_CONTROLLER, feature_status);
 
 static struct go_cfg_attr rumble_mode_right = { RUMBLE_MODE };
 LEGO_DEVICE_ATTR_RW(rumble_mode_right, "rumble_mode", RIGHT_CONTROLLER, index,
-- 
2.55.0

[PATCH 5/5] HID: hid-lenovo-go: return configuration request errors

[Aditya Dash]

The driver completes a pending request when it receives any decoded
configuration reply. It also discards the result of its interruptible wait.
A configuration operation can therefore report success after an unrelated
reply, firmware error, interrupted wait, or timeout.

Track one pending request and match replies by command ID, command,
sub-command, and device. Return output errors, firmware errors, interrupted
waits, and timeouts to the caller. Treat a timeout or interruption as final
if a matching reply arrives after the wait finishes.

Calibration completion is asynchronous, and some Start commands do not send
an immediate reply. Submit each exact calibration command and sub-command
pair once and return its HID output status without a synchronous wait.

The firmware does not include a sequence number. A late reply with the same
complete tuple can still match a newer request.

Fixes: d69ccfcbc955 ("HID: hid-lenovo-go: Add Lenovo Legion Go Series HID Driver")
Assisted-by: Pi:gpt-5.6-sol
Signed-off-by: Aditya Dash <mradityadash@gmail.com>
---
On an Original Legion Go, left-joystick Start did not return an
immediate reply in two controller sessions although firmware began
calibration. A focused target-kernel KUnit test covered timeout,
interruption, firmware error, and reply ownership. No natural firmware
rejection was reproduced.

 drivers/hid/hid-lenovo-go.c | 118 ++++++++++++++++++++++++++++++------
 1 file changed, 100 insertions(+), 18 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
index 1e6915c969d8..a1a255c28e2d 100644
--- a/drivers/hid/hid-lenovo-go.c
+++ b/drivers/hid/hid-lenovo-go.c
@@ -21,6 +21,7 @@
 #include <linux/led-class-multicolor.h>
 #include <linux/mutex.h>
 #include <linux/printk.h>
+#include <linux/spinlock.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
 #include <linux/unaligned.h>
@@ -31,13 +32,30 @@
 #include "hid-ids.h"
 
 #define GO_GP_INTF_IN		0x83
+#define GO_INPUT_REPORT_ID	0x04
 #define GO_OUTPUT_REPORT_ID	0x05
 #define GO_GP_RESET_SUCCESS	0x01
 #define GO_PACKET_SIZE		64
 
+/* Lenovo replies identify a command and a sub-command, but have no sequence. */
+struct hid_go_cmd {
+	struct completion done;
+	spinlock_t lock; /* protects fields below */
+	bool pending;
+	u8 id;
+	u8 command;
+	u8 sub_command;
+	u8 device;
+	int result;
+};
+
+static struct hid_go_cmd go_cmd = {
+	.done = COMPLETION_INITIALIZER(go_cmd.done),
+	.lock = __SPIN_LOCK_UNLOCKED(go_cmd.lock),
+};
+
 static struct hid_go_cfg {
 	struct delayed_work go_cfg_setup;
-	struct completion send_cmd_complete;
 	struct led_classdev *led_cdev;
 	struct hid_device *hdev;
 	struct mutex cfg_mutex; /*ensure single synchronous output report*/
@@ -331,6 +349,72 @@ static const char *const os_mode_text[] = {
 	[LINUX] = "linux",
 };
 
+static void hid_go_cmd_arm(u8 id, u8 command, u8 sub_command, u8 device)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&go_cmd.lock, flags);
+	reinit_completion(&go_cmd.done);
+	go_cmd.pending = true;
+	go_cmd.id = id;
+	go_cmd.command = command;
+	go_cmd.sub_command = sub_command;
+	go_cmd.device = device;
+	spin_unlock_irqrestore(&go_cmd.lock, flags);
+}
+
+static void hid_go_cmd_consume(const struct command_report *cmd_rep, int result)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&go_cmd.lock, flags);
+	if (go_cmd.pending && cmd_rep->id == go_cmd.id &&
+	    cmd_rep->cmd == go_cmd.command &&
+	    cmd_rep->sub_cmd == go_cmd.sub_command &&
+	    cmd_rep->device_type == go_cmd.device) {
+		go_cmd.pending = false;
+		go_cmd.result = result;
+		complete(&go_cmd.done);
+	}
+	spin_unlock_irqrestore(&go_cmd.lock, flags);
+}
+
+static int hid_go_cmd_finish(long wait_result)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&go_cmd.lock, flags);
+	if (wait_result <= 0) {
+		go_cmd.pending = false;
+		ret = wait_result < 0 ? wait_result : -ETIMEDOUT;
+	} else {
+		ret = go_cmd.result;
+	}
+	spin_unlock_irqrestore(&go_cmd.lock, flags);
+	return ret;
+}
+
+static int hid_go_cmd_cancel(int result)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&go_cmd.lock, flags);
+	go_cmd.pending = false;
+	spin_unlock_irqrestore(&go_cmd.lock, flags);
+	return result;
+}
+
+static int hid_go_send_output_report(struct hid_device *hdev, u8 *packet)
+{
+	int ret;
+
+	ret = hid_hw_output_report(hdev, packet, GO_PACKET_SIZE);
+	if (ret < 0)
+		return ret;
+	return ret == GO_PACKET_SIZE ? 0 : -EINVAL;
+}
+
 static int hid_go_version_event(struct command_report *cmd_rep)
 {
 	switch (cmd_rep->sub_cmd) {
@@ -658,6 +742,8 @@ static int hid_go_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	if (size != GO_PACKET_SIZE)
 		goto passthrough;
+	if (data[0] != GO_INPUT_REPORT_ID)
+		goto passthrough;
 
 	ep = get_endpoint_address(hdev);
 	if (ep != GO_GP_INTF_IN)
@@ -709,7 +795,7 @@ static int hid_go_raw_event(struct hid_device *hdev, struct hid_report *report,
 	dev_dbg(&hdev->dev, "Rx data as raw input report: [%*ph]\n",
 		GO_PACKET_SIZE, data);
 
-	complete(&drvdata.send_cmd_complete);
+	hid_go_cmd_consume(cmd_rep, ret);
 	return ret;
 
 passthrough:
@@ -725,6 +811,7 @@ static int mcu_property_out(struct hid_device *hdev, u8 id, u8 command,
 	u8 header[] = { GO_OUTPUT_REPORT_ID, id, command, index, device };
 	size_t header_size = ARRAY_SIZE(header);
 	int timeout = 50;
+	long wait_result;
 	int ret;
 
 	/* The FPS mode DPI request does not contain a device byte. */
@@ -747,22 +834,19 @@ static int mcu_property_out(struct hid_device *hdev, u8 id, u8 command,
 	dev_dbg(&hdev->dev, "Send data as raw output report: [%*ph]\n",
 		GO_PACKET_SIZE, dmabuf);
 
-	ret = hid_hw_output_report(hdev, dmabuf, GO_PACKET_SIZE);
-	if (ret < 0)
-		return ret;
+	if (id == MCU_CONFIG_DATA &&
+	    ((command == SET_TRIGGER_CFG && index == TRIGGER_CALIBRATE) ||
+	     (command == SET_JOYSTICK_CFG && index == JOYSTICK_CALIBRATE) ||
+	     (command == SET_GYRO_CFG && index == GYRO_CALIBRATE)))
+		return hid_go_send_output_report(hdev, dmabuf);
 
-	ret = ret == GO_PACKET_SIZE ? 0 : -EINVAL;
+	hid_go_cmd_arm(id, command, index, device);
+	ret = hid_go_send_output_report(hdev, dmabuf);
 	if (ret)
-		return ret;
-
-	ret = wait_for_completion_interruptible_timeout(&drvdata.send_cmd_complete,
-							msecs_to_jiffies(timeout));
-
-	if (ret == 0) /* timeout occurred */
-		ret = -EBUSY;
-
-	reinit_completion(&drvdata.send_cmd_complete);
-	return 0;
+		return hid_go_cmd_cancel(ret);
+	wait_result = wait_for_completion_interruptible_timeout(&go_cmd.done,
+								msecs_to_jiffies(timeout));
+	return hid_go_cmd_finish(wait_result);
 }
 
 static ssize_t version_show(struct device *dev, struct device_attribute *attr,
@@ -2395,8 +2479,6 @@ static int hid_go_cfg_probe(struct hid_device *hdev,
 
 	drvdata.led_cdev = &go_cdev_rgb.led_cdev;
 
-	init_completion(&drvdata.send_cmd_complete);
-
 	/* Executing calls prior to returning from probe will lock the MCU. Schedule
 	 * initial data call after probe has completed and MCU can accept calls.
 	 */
-- 
2.55.0

Re: [PATCH 5/5] HID: hid-lenovo-go: return configuration request errors

[Derek John Clark]

On Fri, Aug 21, 2026 at 2:48 PM Aditya Dash <mradityadash@gmail.com> wrote:
>
> The driver completes a pending request when it receives any decoded
> configuration reply. It also discards the result of its interruptible wait.
> A configuration operation can therefore report success after an unrelated
> reply, firmware error, interrupted wait, or timeout.
>
> Track one pending request and match replies by command ID, command,
> sub-command, and device. Return output errors, firmware errors, interrupted
> waits, and timeouts to the caller. Treat a timeout or interruption as final
> if a matching reply arrives after the wait finishes.
>
> Calibration completion is asynchronous, and some Start commands do not send
> an immediate reply. Submit each exact calibration command and sub-command
> pair once and return its HID output status without a synchronous wait.
>
> The firmware does not include a sequence number. A late reply with the same
> complete tuple can still match a newer request.
>
> Fixes: d69ccfcbc955 ("HID: hid-lenovo-go: Add Lenovo Legion Go Series HID Driver")
> Assisted-by: Pi:gpt-5.6-sol
> Signed-off-by: Aditya Dash <mradityadash@gmail.com>
> ---
> On an Original Legion Go, left-joystick Start did not return an
> immediate reply in two controller sessions although firmware began
> calibration. A focused target-kernel KUnit test covered timeout,
> interruption, firmware error, and reply ownership. No natural firmware
> rejection was reproduced.
>
>  drivers/hid/hid-lenovo-go.c | 118 ++++++++++++++++++++++++++++++------
>  1 file changed, 100 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
> index 1e6915c969d8..a1a255c28e2d 100644
> --- a/drivers/hid/hid-lenovo-go.c
> +++ b/drivers/hid/hid-lenovo-go.c
> @@ -21,6 +21,7 @@
>  #include <linux/led-class-multicolor.h>
>  #include <linux/mutex.h>
>  #include <linux/printk.h>
> +#include <linux/spinlock.h>
>  #include <linux/sysfs.h>
>  #include <linux/types.h>
>  #include <linux/unaligned.h>
> @@ -31,13 +32,30 @@
>  #include "hid-ids.h"
>
>  #define GO_GP_INTF_IN          0x83
> +#define GO_INPUT_REPORT_ID     0x04
>  #define GO_OUTPUT_REPORT_ID    0x05
>  #define GO_GP_RESET_SUCCESS    0x01
>  #define GO_PACKET_SIZE         64
>
> +/* Lenovo replies identify a command and a sub-command, but have no sequence. */
> +struct hid_go_cmd {
> +       struct completion done;
> +       spinlock_t lock; /* protects fields below */
> +       bool pending;
> +       u8 id;
> +       u8 command;
> +       u8 sub_command;
> +       u8 device;
> +       int result;
> +};
> +
> +static struct hid_go_cmd go_cmd = {
> +       .done = COMPLETION_INITIALIZER(go_cmd.done),
> +       .lock = __SPIN_LOCK_UNLOCKED(go_cmd.lock),
> +};
> +
Hi Aditya,

Something that will need to happen in the future is making drvdata
instantiated with devm_alloc rather than use a global. To avoid adding
more churn to that effort, can you make this a member of the drvdata
struct and instantiate its members in cfg_probe()?

>  static struct hid_go_cfg {
>         struct delayed_work go_cfg_setup;
> -       struct completion send_cmd_complete;
>         struct led_classdev *led_cdev;
>         struct hid_device *hdev;
>         struct mutex cfg_mutex; /*ensure single synchronous output report*/
> @@ -331,6 +349,72 @@ static const char *const os_mode_text[] = {
>         [LINUX] = "linux",
>  };
>
> +static void hid_go_cmd_arm(u8 id, u8 command, u8 sub_command, u8 device)
> +{
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&go_cmd.lock, flags);

Please use guard(spinlock_irqsave, &go_cmd.lock) and #include
<linux/cleanup.h> if it's not already in the list (applies to all
below instances)

> +       reinit_completion(&go_cmd.done);
> +       go_cmd.pending = true;
> +       go_cmd.id = id;
> +       go_cmd.command = command;
> +       go_cmd.sub_command = sub_command;
> +       go_cmd.device = device;
> +       spin_unlock_irqrestore(&go_cmd.lock, flags);
> +}
> +
> +static void hid_go_cmd_consume(const struct command_report *cmd_rep, int result)
> +{
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&go_cmd.lock, flags);
> +       if (go_cmd.pending && cmd_rep->id == go_cmd.id &&
> +           cmd_rep->cmd == go_cmd.command &&
> +           cmd_rep->sub_cmd == go_cmd.sub_command &&
> +           cmd_rep->device_type == go_cmd.device) {
> +               go_cmd.pending = false;
> +               go_cmd.result = result;
> +               complete(&go_cmd.done);
> +       }
> +       spin_unlock_irqrestore(&go_cmd.lock, flags);
> +}
> +
> +static int hid_go_cmd_finish(long wait_result)
> +{
> +       unsigned long flags;
> +       int ret;
> +
> +       spin_lock_irqsave(&go_cmd.lock, flags);
> +       if (wait_result <= 0) {
> +               go_cmd.pending = false;
> +               ret = wait_result < 0 ? wait_result : -ETIMEDOUT;
> +       } else {
> +               ret = go_cmd.result;
> +       }
> +       spin_unlock_irqrestore(&go_cmd.lock, flags);
> +       return ret;
> +}
> +
> +static int hid_go_cmd_cancel(int result)
> +{
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&go_cmd.lock, flags);
> +       go_cmd.pending = false;
> +       spin_unlock_irqrestore(&go_cmd.lock, flags);
> +       return result;
> +}
> +
> +static int hid_go_send_output_report(struct hid_device *hdev, u8 *packet)
> +{
> +       int ret;
> +
> +       ret = hid_hw_output_report(hdev, packet, GO_PACKET_SIZE);
> +       if (ret < 0)
> +               return ret;
> +       return ret == GO_PACKET_SIZE ? 0 : -EINVAL;
> +}
> +
>  static int hid_go_version_event(struct command_report *cmd_rep)
>  {
>         switch (cmd_rep->sub_cmd) {
> @@ -658,6 +742,8 @@ static int hid_go_raw_event(struct hid_device *hdev, struct hid_report *report,
>
>         if (size != GO_PACKET_SIZE)
>                 goto passthrough;
> +       if (data[0] != GO_INPUT_REPORT_ID)
use || instead of two ifs
> +               goto passthrough;
>
>         ep = get_endpoint_address(hdev);
>         if (ep != GO_GP_INTF_IN)
> @@ -709,7 +795,7 @@ static int hid_go_raw_event(struct hid_device *hdev, struct hid_report *report,
>         dev_dbg(&hdev->dev, "Rx data as raw input report: [%*ph]\n",
>                 GO_PACKET_SIZE, data);
>
> -       complete(&drvdata.send_cmd_complete);
> +       hid_go_cmd_consume(cmd_rep, ret);
>         return ret;
>
>  passthrough:
> @@ -725,6 +811,7 @@ static int mcu_property_out(struct hid_device *hdev, u8 id, u8 command,
>         u8 header[] = { GO_OUTPUT_REPORT_ID, id, command, index, device };
>         size_t header_size = ARRAY_SIZE(header);
>         int timeout = 50;
> +       long wait_result;
>         int ret;
>
>         /* The FPS mode DPI request does not contain a device byte. */
> @@ -747,22 +834,19 @@ static int mcu_property_out(struct hid_device *hdev, u8 id, u8 command,
>         dev_dbg(&hdev->dev, "Send data as raw output report: [%*ph]\n",
>                 GO_PACKET_SIZE, dmabuf);
>
> -       ret = hid_hw_output_report(hdev, dmabuf, GO_PACKET_SIZE);
> -       if (ret < 0)
> -               return ret;
> +       if (id == MCU_CONFIG_DATA &&
> +           ((command == SET_TRIGGER_CFG && index == TRIGGER_CALIBRATE) ||
> +            (command == SET_JOYSTICK_CFG && index == JOYSTICK_CALIBRATE) ||
> +            (command == SET_GYRO_CFG && index == GYRO_CALIBRATE)))
> +               return hid_go_send_output_report(hdev, dmabuf);

It might be simpler to add a variable that is passed to
mcu_property_out that triggers this path. That will avoid this list
expanding if additional attributes are added in the future that don't
wait on completions. In hid-msi I pass the completion timeout instead
of declaring it in the function and skip wait if it is 0. A bool would
also work.

Thanks,
Derek

> -       ret = ret == GO_PACKET_SIZE ? 0 : -EINVAL;
> +       hid_go_cmd_arm(id, command, index, device);
> +       ret = hid_go_send_output_report(hdev, dmabuf);
>         if (ret)
> -               return ret;
> -
> -       ret = wait_for_completion_interruptible_timeout(&drvdata.send_cmd_complete,
> -                                                       msecs_to_jiffies(timeout));
> -
> -       if (ret == 0) /* timeout occurred */
> -               ret = -EBUSY;
> -
> -       reinit_completion(&drvdata.send_cmd_complete);
> -       return 0;
> +               return hid_go_cmd_cancel(ret);
> +       wait_result = wait_for_completion_interruptible_timeout(&go_cmd.done,
> +                                                               msecs_to_jiffies(timeout));
> +       return hid_go_cmd_finish(wait_result);
>  }
>
>  static ssize_t version_show(struct device *dev, struct device_attribute *attr,
> @@ -2395,8 +2479,6 @@ static int hid_go_cfg_probe(struct hid_device *hdev,
>
>         drvdata.led_cdev = &go_cdev_rgb.led_cdev;
>
> -       init_completion(&drvdata.send_cmd_complete);
> -
>         /* Executing calls prior to returning from probe will lock the MCU. Schedule
>          * initial data call after probe has completed and MCU can accept calls.
>          */
> --
> 2.55.0
>

Re: [PATCH 1/5] HID: hid-lenovo-go: use the correct calibration commands

[Derek John Clark]

On Fri, Aug 21, 2026 at 2:48 PM Aditya Dash <mradityadash@gmail.com> wrote:
>
> The calibration attributes pass the calibration operation as the command
> and the configuration command as the sub-command. This sends each request
> with the two fields reversed.
>
> Swap the command and sub-command values for all six calibration attributes.
> For example, the malformed left joystick Start request is:
>
>   05 00 04 0c 03 01 ...
>
> The corrected request is:
>
>   05 00 0c 04 03 01 ...
>
> The bytes identify the output report, configuration group, command,
> sub-command, left controller, and Start action, in that order.
>
> Fixes: 995887a10da1 ("HID: hid-lenovo-go: Add Calibration Settings")
> Assisted-by: Pi:gpt-5.6-sol
> Signed-off-by: Aditya Dash <mradityadash@gmail.com>
> ---
>  drivers/hid/hid-lenovo-go.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
> index 318b1152ff8b..b8248bf8965c 100644
> --- a/drivers/hid/hid-lenovo-go.c
> +++ b/drivers/hid/hid-lenovo-go.c
> @@ -1911,18 +1911,18 @@ LEGO_DEVICE_ATTR_RW(rumble_notification_left, "rumble_notification",
>  static DEVICE_ATTR_RO_NAMED(rumble_notification_left_index,
>                             "rumble_notification_index");
>
> -static struct go_cfg_attr cal_trigg_left = { TRIGGER_CALIBRATE };
> -LEGO_CAL_DEVICE_ATTR(cal_trigg_left, "calibrate_trigger", SET_TRIGGER_CFG,
> +static struct go_cfg_attr cal_trigg_left = { SET_TRIGGER_CFG };
> +LEGO_CAL_DEVICE_ATTR(cal_trigg_left, "calibrate_trigger", TRIGGER_CALIBRATE,
>                      LEFT_CONTROLLER, index);
>  static DEVICE_ATTR_RO_NAMED(cal_trigg_left_index, "calibrate_trigger_index");
>
> -static struct go_cfg_attr cal_joy_left = { JOYSTICK_CALIBRATE };
> -LEGO_CAL_DEVICE_ATTR(cal_joy_left, "calibrate_joystick", SET_JOYSTICK_CFG,
> +static struct go_cfg_attr cal_joy_left = { SET_JOYSTICK_CFG };
> +LEGO_CAL_DEVICE_ATTR(cal_joy_left, "calibrate_joystick", JOYSTICK_CALIBRATE,
>                      LEFT_CONTROLLER, index);
>  static DEVICE_ATTR_RO_NAMED(cal_joy_left_index, "calibrate_joystick_index");
>
> -static struct go_cfg_attr cal_gyro_left = { GYRO_CALIBRATE };
> -LEGO_CAL_DEVICE_ATTR(cal_gyro_left, "calibrate_gyro", SET_GYRO_CFG,
> +static struct go_cfg_attr cal_gyro_left = { SET_GYRO_CFG };
> +LEGO_CAL_DEVICE_ATTR(cal_gyro_left, "calibrate_gyro", GYRO_CALIBRATE,
>                      LEFT_CONTROLLER, index);
>  static DEVICE_ATTR_RO_NAMED(cal_gyro_left_index, "calibrate_gyro_index");
>
> @@ -2018,18 +2018,18 @@ LEGO_DEVICE_ATTR_RW(rumble_notification_right, "rumble_notification",
>  static DEVICE_ATTR_RO_NAMED(rumble_notification_right_index,
>                             "rumble_notification_index");
>
> -static struct go_cfg_attr cal_trigg_right = { TRIGGER_CALIBRATE };
> -LEGO_CAL_DEVICE_ATTR(cal_trigg_right, "calibrate_trigger", SET_TRIGGER_CFG,
> +static struct go_cfg_attr cal_trigg_right = { SET_TRIGGER_CFG };
> +LEGO_CAL_DEVICE_ATTR(cal_trigg_right, "calibrate_trigger", TRIGGER_CALIBRATE,
>                      RIGHT_CONTROLLER, index);
>  static DEVICE_ATTR_RO_NAMED(cal_trigg_right_index, "calibrate_trigger_index");
>
> -static struct go_cfg_attr cal_joy_right = { JOYSTICK_CALIBRATE };
> -LEGO_CAL_DEVICE_ATTR(cal_joy_right, "calibrate_joystick", SET_JOYSTICK_CFG,
> +static struct go_cfg_attr cal_joy_right = { SET_JOYSTICK_CFG };
> +LEGO_CAL_DEVICE_ATTR(cal_joy_right, "calibrate_joystick", JOYSTICK_CALIBRATE,
>                      RIGHT_CONTROLLER, index);
>  static DEVICE_ATTR_RO_NAMED(cal_joy_right_index, "calibrate_joystick_index");
>
> -static struct go_cfg_attr cal_gyro_right = { GYRO_CALIBRATE };
> -LEGO_CAL_DEVICE_ATTR(cal_gyro_right, "calibrate_gyro", SET_GYRO_CFG,
> +static struct go_cfg_attr cal_gyro_right = { SET_GYRO_CFG };
> +LEGO_CAL_DEVICE_ATTR(cal_gyro_right, "calibrate_gyro", GYRO_CALIBRATE,
>                      RIGHT_CONTROLLER, index);
>  static DEVICE_ATTR_RO_NAMED(cal_gyro_right_index, "calibrate_gyro_index");
>

Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>
Tested-by: Derek J. Clark <derekjohn.clark@gmail.com>
> --
> 2.55.0
>

Re: [PATCH 2/5] HID: hid-lenovo-go: fix the FPS mode DPI request

[Derek John Clark]

On Fri, Aug 21, 2026 at 2:48 PM Aditya Dash <mradityadash@gmail.com> wrote:
>
> mcu_property_out() adds the five-byte request header used by most
> configuration commands. For FPS mode DPI writes, this inserts a device byte
> before the four-byte DPI value.
>
> The FPS mode DPI request does not contain a device byte. Its little-endian
> DPI value starts immediately after the command and sub-command.
>
> Use the four-byte header only for FPS mode DPI writes. Keep the common
> header for all other requests.
>
> Fixes: f0bedee60607 ("HID: hid-lenovo-go: Add FPS Mode DPI settings")
> Assisted-by: Pi:gpt-5.6-sol
> Signed-off-by: Aditya Dash <mradityadash@gmail.com>
> ---
> The corrected FPS mode DPI write was not tested on hardware.
>
>  drivers/hid/hid-lenovo-go.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
> index b8248bf8965c..a6e9a130ad41 100644
> --- a/drivers/hid/hid-lenovo-go.c
> +++ b/drivers/hid/hid-lenovo-go.c
> @@ -725,6 +725,11 @@ static int mcu_property_out(struct hid_device *hdev, u8 id, u8 command,
>         int timeout = 50;
>         int ret;
>
> +       /* The FPS mode DPI request does not contain a device byte. */
> +       if (id == MCU_CONFIG_DATA && command == SET_DPI_CFG &&
> +           index == FPS_MODE_DPI)
> +               header_size--;
> +

While testing this I go some strange results.

(deck@lego2pro device)$ echo 1800 | sudo tee fps_mode_dpi
1800
tee: fps_mode_dpi: Input/output error
(1)(deck@lego2pro device)$ cat fps_mode_dpi
500
(deck@lego2pro device)$ echo 1200 | sudo tee fps_mode_dpi
1200
(deck@lego2pro device)$ cat fps_mode_dpi
1800
(deck@lego2pro device)$ echo 1200 | sudo tee fps_mode_dpi
1200
(deck@lego2pro device)$ cat fps_mode_dpi
1800
(deck@lego2pro device)$ echo 800 | sudo tee fps_mode_dpi
800
(deck@lego2pro device)$ echo 500 | sudo tee fps_mode_dpi
500
(deck@lego2pro device)$ cat fps_mode_dpi
500

compared to current:
(deck@lego2pro device)$ echo 1800 | sudo tee fps_mode_dpi
1800
(deck@lego2pro device)$ cat fps_mode_dpi
1800
(deck@lego2pro device)$ echo 1200 | sudo tee fps_mode_dpi
1200
(deck@lego2pro device)$ cat fps_mode_dpi
1200
(deck@lego2pro device)$ echo 800 | sudo tee fps_mode_dpi
800
(deck@lego2pro device)$ cat fps_mode_dpi
800
(deck@lego2pro device)$ echo 500 | sudo tee fps_mode_dpi
500
(deck@lego2pro device)$ cat fps_mode_dpi
500

Thanks,
Derek
>         if (header_size + len > GO_PACKET_SIZE)
>                 return -EINVAL;
>
> --
> 2.55.0
>

Re: [PATCH 3/5] HID: hid-lenovo-go: record calibration completion state

[Derek John Clark]

On Fri, Aug 21, 2026 at 2:48 PM Aditya Dash <mradityadash@gmail.com> wrote:
>
> The firmware uses result value 0 for failure and 1 for success. The driver
> stores this value directly as a state index. Index 0 means unknown, so the
> driver reports a firmware failure as unknown.
This is false. Firmware reports 0x01 as success, 0x02 as failure. With
this change all calibration actions result in a success.

From the documentation:

1. Start Joystick calibration.
2. The user needs to rotate the joystick along the edge for two full
circles and then return it to
the center position.
3. Upon successful calibration, the firmware will automatically exit
the calibration mode after 2
seconds and report a successful calibration status. If the joystick
calibration is not completed
within 10 seconds, the firmware will automatically exit the
calibration mode and report a
failed calibration status.

Thanks,
Derek
> Map the result value to the matching failure or success state.
>
> Fixes: 995887a10da1 ("HID: hid-lenovo-go: Add Calibration Settings")
> Assisted-by: Pi:gpt-5.6-sol
> Signed-off-by: Aditya Dash <mradityadash@gmail.com>
> ---
> A successful calibration completion was tested on hardware. No final
> failure completion was captured.
>
>  drivers/hid/hid-lenovo-go.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
> index a6e9a130ad41..d2d134873dd1 100644
> --- a/drivers/hid/hid-lenovo-go.c
> +++ b/drivers/hid/hid-lenovo-go.c
> @@ -577,17 +577,19 @@ static int hid_go_light_event(struct command_report *cmd_rep)
>
>  static int hid_go_device_status_event(struct command_report *cmd_rep)
>  {
> +       u8 status = cmd_rep->data[1] ? CAL_STAT_SUCCESS : CAL_STAT_FAILURE;
> +
>         switch (cmd_rep->device_type) {
>         case LEFT_CONTROLLER:
>                 switch (cmd_rep->data[0]) {
>                 case CALDEV_GYROSCOPE:
> -                       drvdata.gp_left_gyro_cal_status = cmd_rep->data[1];
> +                       drvdata.gp_left_gyro_cal_status = status;
>                         return 0;
>                 case CALDEV_JOYSTICK:
> -                       drvdata.gp_left_joy_cal_status = cmd_rep->data[1];
> +                       drvdata.gp_left_joy_cal_status = status;
>                         return 0;
>                 case CALDEV_TRIGGER:
> -                       drvdata.gp_left_trigg_cal_status = cmd_rep->data[1];
> +                       drvdata.gp_left_trigg_cal_status = status;
>                         return 0;
>                 default:
>                         return -EINVAL;
> @@ -596,13 +598,13 @@ static int hid_go_device_status_event(struct command_report *cmd_rep)
>         case RIGHT_CONTROLLER:
>                 switch (cmd_rep->data[0]) {
>                 case CALDEV_GYROSCOPE:
> -                       drvdata.gp_right_gyro_cal_status = cmd_rep->data[1];
> +                       drvdata.gp_right_gyro_cal_status = status;
>                         return 0;
>                 case CALDEV_JOYSTICK:
> -                       drvdata.gp_right_joy_cal_status = cmd_rep->data[1];
> +                       drvdata.gp_right_joy_cal_status = status;
>                         return 0;
>                 case CALDEV_TRIGGER:
> -                       drvdata.gp_right_trigg_cal_status = cmd_rep->data[1];
> +                       drvdata.gp_right_trigg_cal_status = status;
>                         return 0;
>                 default:
>                         return -EINVAL;
> --
> 2.55.0
>

Re: [PATCH 4/5] HID: hid-lenovo-go: use the right controller selectors

[Derek John Clark]

On Fri, Aug 21, 2026 at 2:48 PM Aditya Dash <mradityadash@gmail.com> wrote:
>
> The right imu_enabled attribute selects FEATURE_IMU_BYPASS, and the right
> reset attribute targets the left controller.
>
> Select FEATURE_IMU_ENABLE for the right imu_enabled attribute. Target the
> right controller for a right reset.
>
> Fixes: 82cd9bc866e1 ("HID: hid-lenovo-go: Add Feature Status Attributes")
> Assisted-by: Pi:gpt-5.6-sol
> Signed-off-by: Aditya Dash <mradityadash@gmail.com>
> ---
> The corrected right reset and right IMU writes were not tested on
> hardware.
>
>  drivers/hid/hid-lenovo-go.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
> index d2d134873dd1..1e6915c969d8 100644
> --- a/drivers/hid/hid-lenovo-go.c
> +++ b/drivers/hid/hid-lenovo-go.c
> @@ -2006,13 +2006,13 @@ LEGO_DEVICE_ATTR_RW(imu_bypass_right, "imu_bypass_enabled", RIGHT_CONTROLLER,
>                     index, feature_status);
>  static DEVICE_ATTR_RO_NAMED(imu_bypass_right_index, "imu_bypass_enabled_index");
>
> -static struct go_cfg_attr imu_enabled_right = { FEATURE_IMU_BYPASS };
> +static struct go_cfg_attr imu_enabled_right = { FEATURE_IMU_ENABLE };
>  LEGO_DEVICE_ATTR_RW(imu_enabled_right, "imu_enabled", RIGHT_CONTROLLER, index,
>                     feature_status);
>  static DEVICE_ATTR_RO_NAMED(imu_enabled_right_index, "imu_enabled_index");
>
>  static struct go_cfg_attr reset_right = { FEATURE_RESET_GAMEPAD };
> -LEGO_DEVICE_ATTR_WO(reset_right, "reset", LEFT_CONTROLLER, feature_status);
> +LEGO_DEVICE_ATTR_WO(reset_right, "reset", RIGHT_CONTROLLER, feature_status);
>
>  static struct go_cfg_attr rumble_mode_right = { RUMBLE_MODE };
>  LEGO_DEVICE_ATTR_RW(rumble_mode_right, "rumble_mode", RIGHT_CONTROLLER, index,
> --
> 2.55.0
>

Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>
Tested-by: Derek J. Clark <derekjohn.clark@gmail.com>