[PATCH 00/10] HID: hid-lenovo-go-s: Fix various bugs

[PATCH 00/10] HID: hid-lenovo-go-s: Fix various bugs

[Derek J. Clark]

This series fixes various bugs in the hid-lenovo-go-s driver

Patch 1 fixes a dropped error on return from mcu_property_out()

Patch 2 creates a mechanism to ignore stale responses if they timed out
in the previous command.

Patch 3 fixes a deadlock if the device is removed while cfg_setup() is
running.

Patch 4 adds missing pm_ptr to the reset_resume function pointer.

Patch 5 adds a kobject_uevent notify during reset-resume to ensure the
os_mode return is caught by udev.

Patch 6 adds a suspend callback to cancel delayed work and re-queue the
cfg_setup if the device was suspended prior to or during the work.

Patch 7 moves attribute init to the end of cfg_setup, ensuring the
attributes can only be used once the MCU is ready.

Patch 8 reorganizes hid_gos_cfg and renamed it to gos_cfg_drvdata in
preparation for later fixes.

Patch 9 moves the static led_classdev to become a member of the drvdata
struct.

Patch 10 moved drvdata from a static global to a device allocated
struct.

Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>

Derek J. Clark (10):
  HID: hid-lenovo-go-s: Return ret instead of 0 in mcu_property_out()
  HID: hid-lenovo-go-s: Bound stale reply window before reusing
    send_cmd_complete
  HID: hid-lenovo-go-s: Prevent deadlock if device removed during setup
  HID: hid-lenovo-go-s: Use pm_ptr for reset_resume callback
  HID: hid-lenovo-go-s: Add kobject_uevent notify during reset-resume
  HID: hid-lenovo-go-s: Add suspend function and gate access under bool.
  HID: hid-lenovo-go-s: Move attribute init to after device query
  HID: hid-lenovo-go-s: Reorganize and rename hid_gos_cfg
  HID: hid-lenovo-go-s: Move static led_classdev_mc to drvdata struct
  HID: hid-lenovo-go-s: Use devm_kzalloc for drvdata

 drivers/hid/hid-lenovo-go-s.c | 655 ++++++++++++++++++++++++----------
 1 file changed, 473 insertions(+), 182 deletions(-)

-- 
2.55.0

[PATCH 01/10] HID: hid-lenovo-go-s: Return ret instead of 0 in mcu_property_out()

[Derek J. Clark]

Currently mcu_property_out() blanket returns 0, discarding the ret
value and any errors with it. Wait completion returns a positive value
when it is not timed out, which would error on all successes, so if it
is positive return the ret value from the raw event handler. Only if
it is 0 set it to -EBUSY, otherwise pass the actual error.

Fixes: a23f3497bf208c59ad ("HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index a72f7f748cb5..68301d4c762a 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -38,6 +38,7 @@ static struct hid_gos_cfg {
 	struct led_classdev *led_cdev;
 	struct hid_device *hdev;
 	struct mutex cfg_mutex; /*ensure single synchronous output report*/
+	int cmd_status;
 	u8 gp_auto_sleep_time;
 	u8 gp_dpad_mode;
 	u8 gp_mode;
@@ -435,7 +436,9 @@ static int hid_gos_raw_event(struct hid_device *hdev, struct hid_report *report,
 	dev_dbg(&hdev->dev, "Rx data as raw input report: [%*ph]\n",
 		GO_S_PACKET_SIZE, data);
 
+	drvdata.cmd_status = ret;
 	complete(&drvdata.send_cmd_complete);
+
 	return ret;
 }
 
@@ -474,12 +477,10 @@ static int mcu_property_out(struct hid_device *hdev, u8 command, u8 index,
 	timeout = (command == GET_PL_TEST) ? 200 : 5;
 	ret = wait_for_completion_interruptible_timeout(&drvdata.send_cmd_complete,
 							msecs_to_jiffies(timeout));
-
-	if (ret == 0) /* timeout occurred */
-		ret = -EBUSY;
+	ret = ret > 0 ? drvdata.cmd_status : ret ?: -EBUSY;
 
 	reinit_completion(&drvdata.send_cmd_complete);
-	return 0;
+	return ret;
 }
 
 static ssize_t gamepad_property_store(struct device *dev,
-- 
2.55.0

[PATCH 02/10] HID: hid-lenovo-go-s: Bound stale reply window before reusing send_cmd_complete

[Derek J. Clark]

mcu_property_out() reinits send_cmd_complete immediately after a
timeout is detected, before the next command that reuses it is sent.
If the MCU's reply to the timed-out command arrives after this reinit
but before the next command's wait begins, it silently satisfies the
next, unrelated command's wait instead of the one it actually answers,
handing that caller stale data with no way to detect the mismatch.

Track when a command has timed out via cmd_orphaned. Before the next
command reuses the completion, wait a bounded 25ms for a stale reply
to arrive and be consumed, then unconditionally clear the flag and
reinit the completion. This does not fully eliminate the window in
which an unrelated reply could still be received, but bounds it to
a short interval right before a new command is sent. Behavior matches
the solution to the same problem in hid-msi.

Fixes: a23f3497bf208c59ad ("HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index 68301d4c762a..36505d8402ff 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -37,6 +37,7 @@ static struct hid_gos_cfg {
 	struct completion send_cmd_complete;
 	struct led_classdev *led_cdev;
 	struct hid_device *hdev;
+	bool orphan_ack_pending;
 	struct mutex cfg_mutex; /*ensure single synchronous output report*/
 	int cmd_status;
 	u8 gp_auto_sleep_time;
@@ -454,6 +455,19 @@ static int mcu_property_out(struct hid_device *hdev, u8 command, u8 index,
 		return -EINVAL;
 
 	guard(mutex)(&drvdata.cfg_mutex);
+
+	/*
+	 * A reply to the previous command may still be in flight. Give it a
+	 * short window to arrive and be consumed before this call reinits the
+	 * completion, so a late reply can't be mistaken for this command's.
+	 */
+	if (drvdata.orphan_ack_pending) {
+		wait_for_completion_timeout(&drvdata.send_cmd_complete, msecs_to_jiffies(25));
+		drvdata.orphan_ack_pending = false;
+		drvdata.cmd_status = -ETIMEDOUT;
+	}
+	reinit_completion(&drvdata.send_cmd_complete);
+
 	/* We can't use a devm_alloc reusable buffer without side effects during suspend */
 	dmabuf = kzalloc(GO_S_PACKET_SIZE, GFP_KERNEL);
 	if (!dmabuf)
@@ -479,7 +493,9 @@ static int mcu_property_out(struct hid_device *hdev, u8 command, u8 index,
 							msecs_to_jiffies(timeout));
 	ret = ret > 0 ? drvdata.cmd_status : ret ?: -EBUSY;
 
-	reinit_completion(&drvdata.send_cmd_complete);
+	if (ret)
+		drvdata.orphan_ack_pending = true;
+
 	return ret;
 }
 
-- 
2.55.0

[PATCH 03/10] HID: hid-lenovo-go-s: Prevent deadlock if device removed during setup

[Derek J. Clark]

hid_gos_cfg_remove() calls a mutex before canceling delayed work. That
delayed work function calls mcu_property_out(), which holds the same mutex.
Prevent deadlock my removing the mutex prior to stopping the work.
Additionally, since cancel_delayed_work has the potential to re-arm,
switch to disable_delayed_work.

Fixes: a23f3497bf208c59ad ("HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index 36505d8402ff..8b16fc7d151e 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -1442,8 +1442,7 @@ static int hid_gos_cfg_probe(struct hid_device *hdev,
 
 static void hid_gos_cfg_remove(struct hid_device *hdev)
 {
-	guard(mutex)(&drvdata.cfg_mutex);
-	cancel_delayed_work_sync(&drvdata.gos_cfg_setup);
+	disable_delayed_work_sync(&drvdata.gos_cfg_setup);
 	sysfs_remove_groups(&hdev->dev.kobj, top_level_attr_groups);
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
-- 
2.55.0

[PATCH 04/10] HID: hid-lenovo-go-s: Use pm_ptr for reset_resume callback

[Derek J. Clark]

Adds missing pm_ptr to the .reset_resume callback for hid-lenovo-go-s

Fixes: 3524900cc571bd922a ("HID: hid-lenovo-go-s: restore OS_TYPE after resume from s2idle")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index 8b16fc7d151e..b6850cf2c029 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -1556,7 +1556,7 @@ static struct hid_driver hid_lenovo_go_s = {
 	.probe = hid_gos_probe,
 	.remove = hid_gos_remove,
 	.raw_event = hid_gos_raw_event,
-	.reset_resume = hid_gos_reset_resume,
+	.reset_resume = pm_ptr(hid_gos_reset_resume),
 };
 module_hid_driver(hid_lenovo_go_s);
 
-- 
2.55.0

[PATCH 05/10] HID: hid-lenovo-go-s: Add kobject_uevent notify during reset-resume

[Derek J. Clark]

Adds kobject_uevent notification after setting os_mode attribute on resume.

Fixes: 3524900cc571bd922a ("HID: hid-lenovo-go-s: restore OS_TYPE after resume from s2idle")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index b6850cf2c029..712823e22bea 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -1467,6 +1467,7 @@ static int hid_gos_cfg_reset_resume(struct hid_device *hdev)
 	if (drvdata.os_mode != os_mode)
 		return -ENODEV;
 
+	kobject_uevent(&drvdata.hdev->dev.kobj, KOBJ_CHANGE);
 	return 0;
 }
 
-- 
2.55.0

[PATCH 06/10] HID: hid-lenovo-go-s: Add suspend function and gate access under bool.

[Derek J. Clark]

Currently a device can suspend while delayed work is pending or
executing. Cancel delayed work if it was pending and restart it if it
hasn't run yet. Gate access to show/store functions behind a bool to
prevent access prior to cfg_setup() being completed.

Fixes: a23f3497bf208c59ad ("HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 149 ++++++++++++++++++++++++++++++++++
 1 file changed, 149 insertions(+)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index 712823e22bea..6d98fff6d987 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -39,6 +39,8 @@ static struct hid_gos_cfg {
 	struct hid_device *hdev;
 	bool orphan_ack_pending;
 	struct mutex cfg_mutex; /*ensure single synchronous output report*/
+	bool gp_registered;
+	bool rgb_registered;
 	int cmd_status;
 	u8 gp_auto_sleep_time;
 	u8 gp_dpad_mode;
@@ -504,10 +506,22 @@ static ssize_t gamepad_property_store(struct device *dev,
 				      const char *buf, size_t count,
 				      enum feature_status_index index)
 {
+	bool dev_registered;
 	size_t size = 1;
 	u8 val = 0;
 	int ret;
 
+	/* rgb_enabled is attached to led_classdev, not hid_device */
+	if (index == FEATURE_RGB_ENABLE)
+		/* Pairs with smp_store_release from gos_cfg_setup */
+		dev_registered = smp_load_acquire(&drvdata.rgb_registered);
+	else
+		/* Pairs with smp_store_release from gos_cfg_setup */
+		dev_registered = smp_load_acquire(&drvdata.gp_registered);
+
+	if (!dev_registered)
+		return -ENODEV;
+
 	switch (index) {
 	case FEATURE_GAMEPAD_MODE:
 		ret = sysfs_match_string(gamepad_mode_text, buf);
@@ -588,9 +602,21 @@ static ssize_t gamepad_property_show(struct device *dev,
 				     struct device_attribute *attr, char *buf,
 				     enum feature_status_index index)
 {
+	bool dev_registered;
 	ssize_t count = 0;
 	u8 i;
 
+	/* rgb_enabled is attached to led_classdev, not hid_device */
+	if (index == FEATURE_RGB_ENABLE)
+		/* Pairs with smp_store_release from gos_cfg_setup */
+		dev_registered = smp_load_acquire(&drvdata.rgb_registered);
+	else
+		/* Pairs with smp_store_release from gos_cfg_setup */
+		dev_registered = smp_load_acquire(&drvdata.gp_registered);
+
+	if (!dev_registered)
+		return -ENODEV;
+
 	count = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, index, NULL, 0);
 	if (count < 0)
 		return count;
@@ -726,10 +752,16 @@ static ssize_t touchpad_property_store(struct device *dev,
 				       const char *buf, size_t count,
 				       enum touchpad_config_index index)
 {
+	bool gp_registered;
 	size_t size = 1;
 	u8 val = 0;
 	int ret;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (!gp_registered)
+		return -ENODEV;
+
 	switch (index) {
 	case CFG_WINDOWS_MODE:
 		ret = sysfs_match_string(touchpad_mode_text, buf);
@@ -760,9 +792,15 @@ static ssize_t touchpad_property_show(struct device *dev,
 				      struct device_attribute *attr, char *buf,
 				      enum touchpad_config_index index)
 {
+	bool gp_registered;
 	int ret = 0;
 	u8 i;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (!gp_registered)
+		return -ENODEV;
+
 	ret = mcu_property_out(drvdata.hdev, GET_TP_PARAM, index, NULL, 0);
 	if (ret < 0)
 		return ret;
@@ -789,9 +827,15 @@ static ssize_t touchpad_property_options(struct device *dev,
 					 char *buf,
 					 enum touchpad_config_index index)
 {
+	bool gp_registered;
 	size_t count = 0;
 	unsigned int i;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (!gp_registered)
+		return -ENODEV;
+
 	switch (index) {
 	case CFG_WINDOWS_MODE:
 	case CFG_LINUX_MODE:
@@ -814,9 +858,15 @@ static ssize_t test_property_show(struct device *dev,
 				  struct device_attribute *attr, char *buf,
 				  enum test_command_index index)
 {
+	bool gp_registered;
 	size_t count = 0;
 	u8 i;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (!gp_registered)
+		return -ENODEV;
+
 	switch (index) {
 	case TEST_TP_MFR:
 		i = drvdata.tp_manufacturer;
@@ -874,9 +924,15 @@ static ssize_t rgb_effect_store(struct device *dev,
 {
 	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(drvdata.led_cdev);
 	enum rgb_config_index index;
+	bool rgb_registered;
 	u8 effect;
 	int ret;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!rgb_registered)
+		return -ENODEV;
+
 	ret = sysfs_match_string(rgb_effect_text, buf);
 	if (ret < 0)
 		return ret;
@@ -901,8 +957,14 @@ static ssize_t rgb_effect_store(struct device *dev,
 static ssize_t rgb_effect_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
+	bool rgb_registered;
 	int ret;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!rgb_registered)
+		return -ENODEV;
+
 	ret = rgb_attr_show();
 	if (ret)
 		return ret;
@@ -934,9 +996,15 @@ static ssize_t rgb_speed_store(struct device *dev,
 {
 	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(drvdata.led_cdev);
 	enum rgb_config_index index;
+	bool rgb_registered;
 	int val = 0;
 	int ret;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!rgb_registered)
+		return -ENODEV;
+
 	ret = kstrtoint(buf, 10, &val);
 	if (ret)
 		return ret;
@@ -964,8 +1032,14 @@ static ssize_t rgb_speed_store(struct device *dev,
 static ssize_t rgb_speed_show(struct device *dev, struct device_attribute *attr,
 			      char *buf)
 {
+	bool rgb_registered;
 	int ret;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!rgb_registered)
+		return -ENODEV;
+
 	ret = rgb_attr_show();
 	if (ret)
 		return ret;
@@ -985,9 +1059,15 @@ static ssize_t rgb_speed_range_show(struct device *dev,
 static ssize_t rgb_mode_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
+	bool rgb_registered;
 	int ret;
 	u8 val;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!rgb_registered)
+		return -ENODEV;
+
 	ret = sysfs_match_string(rgb_mode_text, buf);
 	if (ret <= 0)
 		return ret;
@@ -1007,8 +1087,14 @@ static ssize_t rgb_mode_store(struct device *dev, struct device_attribute *attr,
 static ssize_t rgb_mode_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
+	bool rgb_registered;
 	int ret;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!rgb_registered)
+		return -ENODEV;
+
 	ret = rgb_cfg_call(drvdata.hdev, GET_RGB_CFG, LIGHT_MODE_SEL, NULL, 0);
 	if (ret)
 		return ret;
@@ -1038,10 +1124,16 @@ static ssize_t rgb_profile_store(struct device *dev,
 				 struct device_attribute *attr, const char *buf,
 				 size_t count)
 {
+	bool rgb_registered;
 	size_t size = 1;
 	int ret;
 	u8 val;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!rgb_registered)
+		return -ENODEV;
+
 	ret = kstrtou8(buf, 10, &val);
 	if (ret < 0)
 		return ret;
@@ -1061,8 +1153,14 @@ static ssize_t rgb_profile_store(struct device *dev,
 static ssize_t rgb_profile_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
+	bool rgb_registered;
 	int ret;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!rgb_registered)
+		return -ENODEV;
+
 	ret = rgb_cfg_call(drvdata.hdev, GET_RGB_CFG, LIGHT_PROFILE_SEL, NULL, 0);
 	if (ret)
 		return ret;
@@ -1084,8 +1182,14 @@ static void hid_gos_brightness_set(struct led_classdev *led_cdev,
 {
 	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(drvdata.led_cdev);
 	enum rgb_config_index index;
+	bool rgb_registered;
 	int ret;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!rgb_registered)
+		return;
+
 	if (brightness > led_cdev->max_brightness) {
 		dev_err(led_cdev->dev, "Invalid argument\n");
 		return;
@@ -1392,6 +1496,11 @@ static void cfg_setup(struct work_struct *work)
 			"Failed to retrieve OS Mode: %i\n", ret);
 		return;
 	}
+
+	/* Pairs with smp_load_acquire in attribute show/store functions */
+	smp_store_release(&drvdata.gp_registered, true);
+	/* Pairs with smp_load_acquire in attribute show/store functions */
+	smp_store_release(&drvdata.rgb_registered, true);
 }
 
 static int hid_gos_cfg_probe(struct hid_device *hdev,
@@ -1451,9 +1560,24 @@ static void hid_gos_cfg_remove(struct hid_device *hdev)
 
 static int hid_gos_cfg_reset_resume(struct hid_device *hdev)
 {
+	bool gp_registered, rgb_registered;
 	u8 os_mode = drvdata.os_mode;
 	int ret;
 
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+	/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!gp_registered || !rgb_registered) {
+		ret = schedule_delayed_work(&drvdata.gos_cfg_setup, msecs_to_jiffies(2));
+		if (!ret) {
+			dev_err(&hdev->dev, "Failed to schedule startup delayed work\n");
+			return -ENODEV;
+		}
+
+		return 0;
+	}
+
 	ret = mcu_property_out(drvdata.hdev, SET_GAMEPAD_CFG,
 			       FEATURE_OS_MODE, &os_mode, 1);
 	if (ret < 0)
@@ -1542,6 +1666,30 @@ static int hid_gos_reset_resume(struct hid_device *hdev)
 	return 0;
 }
 
+static int hid_gos_cfg_suspend(void)
+{
+	disable_delayed_work_sync(&drvdata.gos_cfg_setup);
+
+	return 0;
+}
+
+static int hid_gos_suspend(struct hid_device *hdev, pm_message_t msg)
+{
+	int ret;
+	u8 ep;
+
+	/* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+	ret = get_endpoint_address(hdev);
+	if (ret <= 0)
+		return 0;
+
+	ep = ret;
+	if (ep == GO_S_CFG_INTF_IN)
+		return hid_gos_cfg_suspend();
+
+	return 0;
+}
+
 static const struct hid_device_id hid_gos_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_QHE,
 			 USB_DEVICE_ID_LENOVO_LEGION_GO_S_XINPUT) },
@@ -1558,6 +1706,7 @@ static struct hid_driver hid_lenovo_go_s = {
 	.remove = hid_gos_remove,
 	.raw_event = hid_gos_raw_event,
 	.reset_resume = pm_ptr(hid_gos_reset_resume),
+	.suspend = pm_ptr(hid_gos_suspend),
 };
 module_hid_driver(hid_lenovo_go_s);
 
-- 
2.55.0

[PATCH 07/10] HID: hid-lenovo-go-s: Move attribute init to after device query

[Derek J. Clark]

Currently this driver registers attributes before the MCU is ready to
accept commands. This can lead to attribute access that blocks the MCU
in some rare cases.

Move attribute construction to gos_cfg_setup, after the device has been
queried. Gate creation to drvdata.gp_registered and
drvdata.rgb_registered so a resume before setup has run doesn't prevent
attribute creation or attempt to re-register the same attributes.

Fixes: a23f3497bf208c59ad ("HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 68 +++++++++++++++++++++++------------
 1 file changed, 45 insertions(+), 23 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index 6d98fff6d987..f1a0bcfcdb71 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -1452,6 +1452,7 @@ static struct led_classdev_mc gos_cdev_rgb = {
 
 static void cfg_setup(struct work_struct *work)
 {
+	bool gp_registered, rgb_registered;
 	int ret;
 
 	/* MCU */
@@ -1497,10 +1498,53 @@ static void cfg_setup(struct work_struct *work)
 		return;
 	}
 
+	/* Pairs with smp_store_release from below */
+	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (gp_registered)
+		goto try_rgb;
+
+	ret = sysfs_create_groups(&drvdata.hdev->dev.kobj, top_level_attr_groups);
+	if (ret) {
+		dev_err(&drvdata.hdev->dev,
+			"Failed to create gamepad configuration attributes: %i\n", ret);
+		goto try_rgb;
+	}
+
 	/* Pairs with smp_load_acquire in attribute show/store functions */
 	smp_store_release(&drvdata.gp_registered, true);
+	gp_registered = true;
+
+try_rgb:
+	/* Pairs with smp_store_release from below */
+	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (rgb_registered)
+		goto update_kobjects;
+
+	ret = devm_led_classdev_multicolor_register(&drvdata.hdev->dev, &gos_cdev_rgb);
+	if (ret) {
+		dev_err(&drvdata.hdev->dev,
+			"Failed to create RGB device: %i\n", ret);
+		goto update_kobjects;
+	}
+
+	ret = devm_device_add_group(gos_cdev_rgb.led_cdev.dev, &rgb_attr_group);
+	if (ret) {
+		dev_err(&drvdata.hdev->dev,
+			"Failed to create RGB configuration attributes: %i\n", ret);
+		goto update_kobjects;
+	}
+
+	drvdata.led_cdev = &gos_cdev_rgb.led_cdev;
+
 	/* Pairs with smp_load_acquire in attribute show/store functions */
 	smp_store_release(&drvdata.rgb_registered, true);
+	rgb_registered = true;
+
+update_kobjects:
+	if (gp_registered)
+		kobject_uevent(&drvdata.hdev->dev.kobj, KOBJ_CHANGE);
+	if (rgb_registered)
+		kobject_uevent(&drvdata.led_cdev->dev->kobj, KOBJ_CHANGE);
 }
 
 static int hid_gos_cfg_probe(struct hid_device *hdev,
@@ -1510,30 +1554,8 @@ static int hid_gos_cfg_probe(struct hid_device *hdev,
 
 	hid_set_drvdata(hdev, &drvdata);
 	drvdata.hdev = hdev;
-	mutex_init(&drvdata.cfg_mutex);
-
-	ret = sysfs_create_groups(&hdev->dev.kobj, top_level_attr_groups);
-	if (ret) {
-		dev_err_probe(&hdev->dev, ret,
-			      "Failed to create gamepad configuration attributes\n");
-		return ret;
-	}
-
-	ret = devm_led_classdev_multicolor_register(&hdev->dev, &gos_cdev_rgb);
-	if (ret) {
-		dev_err_probe(&hdev->dev, ret, "Failed to create RGB device\n");
-		return ret;
-	}
-
-	ret = devm_device_add_group(gos_cdev_rgb.led_cdev.dev, &rgb_attr_group);
-	if (ret) {
-		dev_err_probe(&hdev->dev, ret,
-			      "Failed to create RGB configuration attributes\n");
-		return ret;
-	}
-
-	drvdata.led_cdev = &gos_cdev_rgb.led_cdev;
 
+	mutex_init(&drvdata.cfg_mutex);
 	init_completion(&drvdata.send_cmd_complete);
 
 	/* Executing calls prior to returning from probe will lock the MCU. Schedule
-- 
2.55.0

[PATCH 08/10] HID: hid-lenovo-go-s: Reorganize and rename hid_gos_cfg

[Derek J. Clark]

Renames hid_gos_cfg struct to gos_cfg_drvdata in preparation for later
fixes.

This is a prerequisite for later fixes. No functional change intended.

Fixes: a23f3497bf208c59ad ("HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 39 ++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index f1a0bcfcdb71..e5c4104a3adb 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -32,36 +32,41 @@
 #define GO_S_CFG_INTF_IN	0x84
 #define GO_S_PACKET_SIZE	64
 
-static struct hid_gos_cfg {
-	struct delayed_work gos_cfg_setup;
+static struct gos_cfg_drvdata {
+	/* MCU General Variables */
 	struct completion send_cmd_complete;
-	struct led_classdev *led_cdev;
+	struct delayed_work gos_cfg_setup;
 	struct hid_device *hdev;
 	bool orphan_ack_pending;
 	struct mutex cfg_mutex; /*ensure single synchronous output report*/
-	bool gp_registered;
-	bool rgb_registered;
 	int cmd_status;
+
+	/* Gamepad Variables */
 	u8 gp_auto_sleep_time;
-	u8 gp_dpad_mode;
-	u8 gp_mode;
-	u8 gp_poll_rate;
-	u8 imu_bypass_en;
 	u8 imu_manufacturer;
+	bool gp_registered;
+	u8 tp_windows_mode;
+	u8 tp_manufacturer;
+	u8 tp_linux_mode;
+	u8 imu_bypass_en;
 	u8 imu_sensor_en;
+	u8 gp_dpad_mode;
+	u8 gp_poll_rate;
 	u8 mcu_id[12];
 	u8 mouse_step;
+	u8 tp_version;
+	u8 gp_mode;
 	u8 os_mode;
-	u8 rgb_effect;
-	u8 rgb_en;
-	u8 rgb_mode;
+	u8 tp_en;
+
+	/* RGB Variables */
+	struct led_classdev *led_cdev;
+	bool rgb_registered;
 	u8 rgb_profile;
+	u8 rgb_effect;
 	u8 rgb_speed;
-	u8 tp_en;
-	u8 tp_linux_mode;
-	u8 tp_windows_mode;
-	u8 tp_version;
-	u8 tp_manufacturer;
+	u8 rgb_mode;
+	u8 rgb_en;
 } drvdata;
 
 struct gos_cfg_attr {
-- 
2.55.0

[PATCH 09/10] HID: hid-lenovo-go-s: Move static led_classdev_mc to drvdata struct

[Derek J. Clark]

In the next patch I will switch drvdata from static global to a
devm_kzalloc struct. In preparation for that, move the led_classdev_mc
to be a member of the gos_cfg_drvdata struct. As part of that effort,
provide a constant name when accessing it and access the led_classdev
through the led_classdev_mc.

Fixes: be6d7dbb368ba840d7 ("HID: hid-lenovo-go-s: Add RGB LED control interface")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 73 +++++++++++++++++------------------
 1 file changed, 36 insertions(+), 37 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index e5c4104a3adb..c77c297a04cb 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -60,7 +60,7 @@ static struct gos_cfg_drvdata {
 	u8 tp_en;
 
 	/* RGB Variables */
-	struct led_classdev *led_cdev;
+	struct led_classdev_mc led_mc;
 	bool rgb_registered;
 	u8 rgb_profile;
 	u8 rgb_effect;
@@ -347,7 +347,7 @@ static int hid_gos_pl_test_event(struct command_report *cmd_rep)
 
 static int hid_gos_light_event(struct command_report *cmd_rep)
 {
-	struct led_classdev_mc *mc_cdev;
+	struct led_classdev_mc *led_mc;
 	int ret = 0;
 
 	switch (cmd_rep->sub_cmd) {
@@ -362,12 +362,12 @@ static int hid_gos_light_event(struct command_report *cmd_rep)
 	case USR_LIGHT_PROFILE_1:
 	case USR_LIGHT_PROFILE_2:
 	case USR_LIGHT_PROFILE_3:
-		mc_cdev = lcdev_to_mccdev(drvdata.led_cdev);
+		led_mc = &drvdata.led_mc;
 		drvdata.rgb_effect = cmd_rep->data[0];
-		mc_cdev->subled_info[0].intensity = cmd_rep->data[1];
-		mc_cdev->subled_info[1].intensity = cmd_rep->data[2];
-		mc_cdev->subled_info[2].intensity = cmd_rep->data[3];
-		drvdata.led_cdev->brightness = cmd_rep->data[4];
+		led_mc->subled_info[0].intensity = cmd_rep->data[1];
+		led_mc->subled_info[1].intensity = cmd_rep->data[2];
+		led_mc->subled_info[2].intensity = cmd_rep->data[3];
+		led_mc->led_cdev.brightness = cmd_rep->data[4];
 		drvdata.rgb_speed = cmd_rep->data[5];
 		ret = 0;
 		break;
@@ -927,7 +927,7 @@ static ssize_t rgb_effect_store(struct device *dev,
 				struct device_attribute *attr, const char *buf,
 				size_t count)
 {
-	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(drvdata.led_cdev);
+	struct led_classdev_mc *led_mc = &drvdata.led_mc;
 	enum rgb_config_index index;
 	bool rgb_registered;
 	u8 effect;
@@ -945,10 +945,10 @@ static ssize_t rgb_effect_store(struct device *dev,
 	effect = ret;
 	index = drvdata.rgb_profile + 2;
 	u8 rgb_profile[6] = { effect,
-			      mc_cdev->subled_info[0].intensity,
-			      mc_cdev->subled_info[1].intensity,
-			      mc_cdev->subled_info[2].intensity,
-			      drvdata.led_cdev->brightness,
+			      led_mc->subled_info[0].intensity,
+			      led_mc->subled_info[1].intensity,
+			      led_mc->subled_info[2].intensity,
+			      led_mc->led_cdev.brightness,
 			      drvdata.rgb_speed };
 
 	ret = rgb_cfg_call(drvdata.hdev, SET_RGB_CFG, index, rgb_profile, 6);
@@ -999,7 +999,7 @@ static ssize_t rgb_speed_store(struct device *dev,
 			       struct device_attribute *attr, const char *buf,
 			       size_t count)
 {
-	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(drvdata.led_cdev);
+	struct led_classdev_mc *led_mc = &drvdata.led_mc;
 	enum rgb_config_index index;
 	bool rgb_registered;
 	int val = 0;
@@ -1019,10 +1019,10 @@ static ssize_t rgb_speed_store(struct device *dev,
 
 	index = drvdata.rgb_profile + 2;
 	u8 rgb_profile[6] = { drvdata.rgb_effect,
-			      mc_cdev->subled_info[0].intensity,
-			      mc_cdev->subled_info[1].intensity,
-			      mc_cdev->subled_info[2].intensity,
-			      drvdata.led_cdev->brightness,
+			      led_mc->subled_info[0].intensity,
+			      led_mc->subled_info[1].intensity,
+			      led_mc->subled_info[2].intensity,
+			      led_mc->led_cdev.brightness,
 			      val };
 
 	ret = rgb_cfg_call(drvdata.hdev, SET_RGB_CFG, index, rgb_profile, 6);
@@ -1185,7 +1185,7 @@ static ssize_t rgb_profile_range_show(struct device *dev,
 static void hid_gos_brightness_set(struct led_classdev *led_cdev,
 				   enum led_brightness brightness)
 {
-	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(drvdata.led_cdev);
+	struct led_classdev_mc *led_mc = &drvdata.led_mc;
 	enum rgb_config_index index;
 	bool rgb_registered;
 	int ret;
@@ -1202,9 +1202,9 @@ static void hid_gos_brightness_set(struct led_classdev *led_cdev,
 
 	index = drvdata.rgb_profile + 2;
 	u8 rgb_profile[6] = { drvdata.rgb_effect,
-			      mc_cdev->subled_info[0].intensity,
-			      mc_cdev->subled_info[1].intensity,
-			      mc_cdev->subled_info[2].intensity,
+			      led_mc->subled_info[0].intensity,
+			      led_mc->subled_info[1].intensity,
+			      led_mc->subled_info[2].intensity,
 			      brightness,
 			      drvdata.rgb_speed };
 
@@ -1444,17 +1444,6 @@ static struct mc_subled gos_rgb_subled_info[] = {
 	},
 };
 
-static struct led_classdev_mc gos_cdev_rgb = {
-	.led_cdev = {
-		.name = "go_s:rgb:joystick_rings",
-		.brightness = 0x50,
-		.max_brightness = 0x64,
-		.brightness_set = hid_gos_brightness_set,
-	},
-	.num_colors = ARRAY_SIZE(gos_rgb_subled_info),
-	.subled_info = gos_rgb_subled_info,
-};
-
 static void cfg_setup(struct work_struct *work)
 {
 	bool gp_registered, rgb_registered;
@@ -1525,22 +1514,20 @@ static void cfg_setup(struct work_struct *work)
 	if (rgb_registered)
 		goto update_kobjects;
 
-	ret = devm_led_classdev_multicolor_register(&drvdata.hdev->dev, &gos_cdev_rgb);
+	ret = devm_led_classdev_multicolor_register(&drvdata.hdev->dev, &drvdata.led_mc);
 	if (ret) {
 		dev_err(&drvdata.hdev->dev,
 			"Failed to create RGB device: %i\n", ret);
 		goto update_kobjects;
 	}
 
-	ret = devm_device_add_group(gos_cdev_rgb.led_cdev.dev, &rgb_attr_group);
+	ret = devm_device_add_group(drvdata.led_mc.led_cdev.dev, &rgb_attr_group);
 	if (ret) {
 		dev_err(&drvdata.hdev->dev,
 			"Failed to create RGB configuration attributes: %i\n", ret);
 		goto update_kobjects;
 	}
 
-	drvdata.led_cdev = &gos_cdev_rgb.led_cdev;
-
 	/* Pairs with smp_load_acquire in attribute show/store functions */
 	smp_store_release(&drvdata.rgb_registered, true);
 	rgb_registered = true;
@@ -1549,7 +1536,7 @@ static void cfg_setup(struct work_struct *work)
 	if (gp_registered)
 		kobject_uevent(&drvdata.hdev->dev.kobj, KOBJ_CHANGE);
 	if (rgb_registered)
-		kobject_uevent(&drvdata.led_cdev->dev->kobj, KOBJ_CHANGE);
+		kobject_uevent(&drvdata.led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
 }
 
 static int hid_gos_cfg_probe(struct hid_device *hdev,
@@ -1563,6 +1550,18 @@ static int hid_gos_cfg_probe(struct hid_device *hdev,
 	mutex_init(&drvdata.cfg_mutex);
 	init_completion(&drvdata.send_cmd_complete);
 
+	/* Device is hardwired and name is guaranteed to be unique */
+	drvdata.led_mc.led_cdev.name = "go_s:rgb:joystick_rings";
+	drvdata.led_mc.led_cdev.brightness = 0x50;
+	drvdata.led_mc.led_cdev.max_brightness = 0x64;
+	drvdata.led_mc.led_cdev.color = LED_COLOR_ID_RGB;
+	drvdata.led_mc.led_cdev.brightness_set = hid_gos_brightness_set;
+	drvdata.led_mc.num_colors = 3;
+	drvdata.led_mc.subled_info = devm_kmemdup(&hdev->dev, gos_rgb_subled_info,
+						  sizeof(gos_rgb_subled_info), GFP_KERNEL);
+	if (!drvdata.led_mc.subled_info)
+		return -ENOMEM;
+
 	/* 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

[PATCH 10/10] HID: hid-lenovo-go-s: Use devm_kzalloc for drvdata

[Derek J. Clark]

Changes drvdata from static global to devm allocation per device.

Fixes: a23f3497bf208c59ad ("HID: hid-lenovo-go-s: Add Lenovo Legion Go S Series HID Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
 drivers/hid/hid-lenovo-go-s.c | 501 ++++++++++++++++++++--------------
 1 file changed, 300 insertions(+), 201 deletions(-)

diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
index c77c297a04cb..7102786ce124 100644
--- a/drivers/hid/hid-lenovo-go-s.c
+++ b/drivers/hid/hid-lenovo-go-s.c
@@ -10,6 +10,7 @@
 #include <linux/array_size.h>
 #include <linux/cleanup.h>
 #include <linux/completion.h>
+#include <linux/container_of.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/device.h>
@@ -32,7 +33,7 @@
 #define GO_S_CFG_INTF_IN	0x84
 #define GO_S_PACKET_SIZE	64
 
-static struct gos_cfg_drvdata {
+struct gos_cfg_drvdata {
 	/* MCU General Variables */
 	struct completion send_cmd_complete;
 	struct delayed_work gos_cfg_setup;
@@ -67,7 +68,7 @@ static struct gos_cfg_drvdata {
 	u8 rgb_speed;
 	u8 rgb_mode;
 	u8 rgb_en;
-} drvdata;
+};
 
 struct gos_cfg_attr {
 	u8 index;
@@ -243,56 +244,57 @@ static const char *const imu_manufacturer_text[] = {
 	[IMU_ST] = "ST",
 };
 
-static int hid_gos_version_event(u8 *data)
+static int hid_gos_version_event(struct gos_cfg_drvdata *drvdata, u8 *data)
 {
 	struct version_report *ver_rep = (struct version_report *)data;
 
-	drvdata.hdev->firmware_version = get_unaligned_le32(&ver_rep->version);
+	drvdata->hdev->firmware_version = get_unaligned_le32(&ver_rep->version);
 	return 0;
 }
 
-static int hid_gos_mcu_id_event(struct command_report *cmd_rep)
+static int hid_gos_mcu_id_event(struct gos_cfg_drvdata *drvdata, struct command_report *cmd_rep)
 {
-	drvdata.mcu_id[0] = cmd_rep->sub_cmd;
-	memcpy(&drvdata.mcu_id[1], cmd_rep->data, 11);
+	drvdata->mcu_id[0] = cmd_rep->sub_cmd;
+	memcpy(&drvdata->mcu_id[1], cmd_rep->data, 11);
 
 	return 0;
 }
 
-static int hid_gos_gamepad_cfg_event(struct command_report *cmd_rep)
+static int hid_gos_gamepad_cfg_event(struct gos_cfg_drvdata *drvdata,
+				     struct command_report *cmd_rep)
 {
 	int ret = 0;
 
 	switch (cmd_rep->sub_cmd) {
 	case FEATURE_GAMEPAD_MODE:
-		drvdata.gp_mode = cmd_rep->data[0];
+		drvdata->gp_mode = cmd_rep->data[0];
 		break;
 	case FEATURE_AUTO_SLEEP_TIME:
-		drvdata.gp_auto_sleep_time = cmd_rep->data[0];
+		drvdata->gp_auto_sleep_time = cmd_rep->data[0];
 		break;
 	case FEATURE_IMU_BYPASS:
-		drvdata.imu_bypass_en = cmd_rep->data[0];
+		drvdata->imu_bypass_en = cmd_rep->data[0];
 		break;
 	case FEATURE_RGB_ENABLE:
-		drvdata.rgb_en = cmd_rep->data[0];
+		drvdata->rgb_en = cmd_rep->data[0];
 		break;
 	case FEATURE_IMU_ENABLE:
-		drvdata.imu_sensor_en = cmd_rep->data[0];
+		drvdata->imu_sensor_en = cmd_rep->data[0];
 		break;
 	case FEATURE_TOUCHPAD_ENABLE:
-		drvdata.tp_en = cmd_rep->data[0];
+		drvdata->tp_en = cmd_rep->data[0];
 		break;
 	case FEATURE_OS_MODE:
-		drvdata.os_mode = cmd_rep->data[0];
+		drvdata->os_mode = cmd_rep->data[0];
 		break;
 	case FEATURE_POLL_RATE:
-		drvdata.gp_poll_rate = cmd_rep->data[0];
+		drvdata->gp_poll_rate = cmd_rep->data[0];
 		break;
 	case FEATURE_DPAD_MODE:
-		drvdata.gp_dpad_mode = cmd_rep->data[0];
+		drvdata->gp_dpad_mode = cmd_rep->data[0];
 		break;
 	case FEATURE_MOUSE_WHEEL_STEP:
-		drvdata.mouse_step = cmd_rep->data[0];
+		drvdata->mouse_step = cmd_rep->data[0];
 		break;
 	default:
 		ret = -EINVAL;
@@ -302,16 +304,16 @@ static int hid_gos_gamepad_cfg_event(struct command_report *cmd_rep)
 	return ret;
 }
 
-static int hid_gos_touchpad_event(struct command_report *cmd_rep)
+static int hid_gos_touchpad_event(struct gos_cfg_drvdata *drvdata, struct command_report *cmd_rep)
 {
 	int ret = 0;
 
 	switch (cmd_rep->sub_cmd) {
 	case CFG_LINUX_MODE:
-		drvdata.tp_linux_mode = cmd_rep->data[0];
+		drvdata->tp_linux_mode = cmd_rep->data[0];
 		break;
 	case CFG_WINDOWS_MODE:
-		drvdata.tp_windows_mode = cmd_rep->data[0];
+		drvdata->tp_windows_mode = cmd_rep->data[0];
 		break;
 	default:
 		ret = -EINVAL;
@@ -321,21 +323,21 @@ static int hid_gos_touchpad_event(struct command_report *cmd_rep)
 	return ret;
 }
 
-static int hid_gos_pl_test_event(struct command_report *cmd_rep)
+static int hid_gos_pl_test_event(struct gos_cfg_drvdata *drvdata, struct command_report *cmd_rep)
 {
 	int ret = 0;
 
 	switch (cmd_rep->sub_cmd) {
 	case TEST_TP_MFR:
-		drvdata.tp_manufacturer = cmd_rep->data[0];
+		drvdata->tp_manufacturer = cmd_rep->data[0];
 		ret = 0;
 		break;
 	case TEST_IMU_MFR:
-		drvdata.imu_manufacturer = cmd_rep->data[0];
+		drvdata->imu_manufacturer = cmd_rep->data[0];
 		ret = 0;
 		break;
 	case TEST_TP_VER:
-		drvdata.tp_version = cmd_rep->data[0];
+		drvdata->tp_version = cmd_rep->data[0];
 		ret = 0;
 		break;
 	default:
@@ -345,30 +347,30 @@ static int hid_gos_pl_test_event(struct command_report *cmd_rep)
 	return ret;
 }
 
-static int hid_gos_light_event(struct command_report *cmd_rep)
+static int hid_gos_light_event(struct gos_cfg_drvdata *drvdata, struct command_report *cmd_rep)
 {
 	struct led_classdev_mc *led_mc;
 	int ret = 0;
 
 	switch (cmd_rep->sub_cmd) {
 	case LIGHT_MODE_SEL:
-		drvdata.rgb_mode = cmd_rep->data[0];
+		drvdata->rgb_mode = cmd_rep->data[0];
 		ret = 0;
 		break;
 	case LIGHT_PROFILE_SEL:
-		drvdata.rgb_profile = cmd_rep->data[0];
+		drvdata->rgb_profile = cmd_rep->data[0];
 		ret = 0;
 		break;
 	case USR_LIGHT_PROFILE_1:
 	case USR_LIGHT_PROFILE_2:
 	case USR_LIGHT_PROFILE_3:
-		led_mc = &drvdata.led_mc;
-		drvdata.rgb_effect = cmd_rep->data[0];
+		led_mc = &drvdata->led_mc;
+		drvdata->rgb_effect = cmd_rep->data[0];
 		led_mc->subled_info[0].intensity = cmd_rep->data[1];
 		led_mc->subled_info[1].intensity = cmd_rep->data[2];
 		led_mc->subled_info[2].intensity = cmd_rep->data[3];
 		led_mc->led_cdev.brightness = cmd_rep->data[4];
-		drvdata.rgb_speed = cmd_rep->data[5];
+		drvdata->rgb_speed = cmd_rep->data[5];
 		ret = 0;
 		break;
 	default:
@@ -401,6 +403,7 @@ static int get_endpoint_address(struct hid_device *hdev)
 static int hid_gos_raw_event(struct hid_device *hdev, struct hid_report *report,
 			     u8 *data, int size)
 {
+	struct gos_cfg_drvdata *drvdata = hid_get_drvdata(hdev);
 	struct command_report *cmd_rep;
 	int ep, ret;
 
@@ -408,6 +411,9 @@ static int hid_gos_raw_event(struct hid_device *hdev, struct hid_report *report,
 	if (ep != GO_S_CFG_INTF_IN)
 		return 0;
 
+	if (!drvdata)
+		return -ENODEV;
+
 	if (size != GO_S_PACKET_SIZE)
 		return -EINVAL;
 
@@ -415,22 +421,22 @@ static int hid_gos_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	switch (cmd_rep->cmd) {
 	case GET_VERSION:
-		ret = hid_gos_version_event(data);
+		ret = hid_gos_version_event(drvdata, data);
 		break;
 	case GET_MCU_ID:
-		ret = hid_gos_mcu_id_event(cmd_rep);
+		ret = hid_gos_mcu_id_event(drvdata, cmd_rep);
 		break;
 	case GET_GAMEPAD_CFG:
-		ret = hid_gos_gamepad_cfg_event(cmd_rep);
+		ret = hid_gos_gamepad_cfg_event(drvdata, cmd_rep);
 		break;
 	case GET_TP_PARAM:
-		ret = hid_gos_touchpad_event(cmd_rep);
+		ret = hid_gos_touchpad_event(drvdata, cmd_rep);
 		break;
 	case GET_PL_TEST:
-		ret = hid_gos_pl_test_event(cmd_rep);
+		ret = hid_gos_pl_test_event(drvdata, cmd_rep);
 		break;
 	case GET_RGB_CFG:
-		ret = hid_gos_light_event(cmd_rep);
+		ret = hid_gos_light_event(drvdata, cmd_rep);
 		break;
 	case SET_GAMEPAD_CFG:
 	case SET_RGB_CFG:
@@ -444,8 +450,8 @@ static int hid_gos_raw_event(struct hid_device *hdev, struct hid_report *report,
 	dev_dbg(&hdev->dev, "Rx data as raw input report: [%*ph]\n",
 		GO_S_PACKET_SIZE, data);
 
-	drvdata.cmd_status = ret;
-	complete(&drvdata.send_cmd_complete);
+	drvdata->cmd_status = ret;
+	complete(&drvdata->send_cmd_complete);
 
 	return ret;
 }
@@ -453,27 +459,31 @@ static int hid_gos_raw_event(struct hid_device *hdev, struct hid_report *report,
 static int mcu_property_out(struct hid_device *hdev, u8 command, u8 index,
 			    u8 *data, size_t len)
 {
+	struct gos_cfg_drvdata *drvdata = hid_get_drvdata(hdev);
 	unsigned char *dmabuf __free(kfree) = NULL;
 	u8 header[] = { command, index };
 	size_t header_size = ARRAY_SIZE(header);
 	int timeout, ret;
 
+	if (!drvdata)
+		return -ENODEV;
+
 	if (header_size + len > GO_S_PACKET_SIZE)
 		return -EINVAL;
 
-	guard(mutex)(&drvdata.cfg_mutex);
+	guard(mutex)(&drvdata->cfg_mutex);
 
 	/*
 	 * A reply to the previous command may still be in flight. Give it a
 	 * short window to arrive and be consumed before this call reinits the
 	 * completion, so a late reply can't be mistaken for this command's.
 	 */
-	if (drvdata.orphan_ack_pending) {
-		wait_for_completion_timeout(&drvdata.send_cmd_complete, msecs_to_jiffies(25));
-		drvdata.orphan_ack_pending = false;
-		drvdata.cmd_status = -ETIMEDOUT;
+	if (drvdata->orphan_ack_pending) {
+		wait_for_completion_timeout(&drvdata->send_cmd_complete, msecs_to_jiffies(25));
+		drvdata->orphan_ack_pending = false;
+		drvdata->cmd_status = -ETIMEDOUT;
 	}
-	reinit_completion(&drvdata.send_cmd_complete);
+	reinit_completion(&drvdata->send_cmd_complete);
 
 	/* We can't use a devm_alloc reusable buffer without side effects during suspend */
 	dmabuf = kzalloc(GO_S_PACKET_SIZE, GFP_KERNEL);
@@ -496,12 +506,12 @@ static int mcu_property_out(struct hid_device *hdev, u8 command, u8 index,
 
 	/* PL_TEST commands can take longer because they go out to another device */
 	timeout = (command == GET_PL_TEST) ? 200 : 5;
-	ret = wait_for_completion_interruptible_timeout(&drvdata.send_cmd_complete,
+	ret = wait_for_completion_interruptible_timeout(&drvdata->send_cmd_complete,
 							msecs_to_jiffies(timeout));
-	ret = ret > 0 ? drvdata.cmd_status : ret ?: -EBUSY;
+	ret = ret > 0 ? drvdata->cmd_status : ret ?: -EBUSY;
 
 	if (ret)
-		drvdata.orphan_ack_pending = true;
+		drvdata->orphan_ack_pending = true;
 
 	return ret;
 }
@@ -511,18 +521,30 @@ static ssize_t gamepad_property_store(struct device *dev,
 				      const char *buf, size_t count,
 				      enum feature_status_index index)
 {
+	struct gos_cfg_drvdata *drvdata;
+	struct led_classdev_mc *led_mc;
+	struct led_classdev *led_cdev;
+	struct hid_device *hdev;
 	bool dev_registered;
 	size_t size = 1;
 	u8 val = 0;
 	int ret;
 
 	/* rgb_enabled is attached to led_classdev, not hid_device */
-	if (index == FEATURE_RGB_ENABLE)
-		/* Pairs with smp_store_release from gos_cfg_setup */
-		dev_registered = smp_load_acquire(&drvdata.rgb_registered);
-	else
-		/* Pairs with smp_store_release from gos_cfg_setup */
-		dev_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (index == FEATURE_RGB_ENABLE) {
+		led_cdev = dev_get_drvdata(dev);
+		led_mc = lcdev_to_mccdev(led_cdev);
+		drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
+		hdev = drvdata->hdev;
+	/* Pairs with smp_store_release from gos_cfg_setup */
+		dev_registered = smp_load_acquire(&drvdata->rgb_registered);
+
+	} else {
+		hdev = to_hid_device(dev);
+		drvdata = hid_get_drvdata(hdev);
+	/* Pairs with smp_store_release from gos_cfg_setup */
+		dev_registered = smp_load_acquire(&drvdata->gp_registered);
+	}
 
 	if (!dev_registered)
 		return -ENODEV;
@@ -595,7 +617,7 @@ static ssize_t gamepad_property_store(struct device *dev,
 	if (!val)
 		size = 0;
 
-	ret = mcu_property_out(drvdata.hdev, SET_GAMEPAD_CFG, index, &val,
+	ret = mcu_property_out(hdev, SET_GAMEPAD_CFG, index, &val,
 			       size);
 	if (ret < 0)
 		return ret;
@@ -607,79 +629,92 @@ static ssize_t gamepad_property_show(struct device *dev,
 				     struct device_attribute *attr, char *buf,
 				     enum feature_status_index index)
 {
+	struct gos_cfg_drvdata *drvdata;
+	struct led_classdev_mc *led_mc;
+	struct led_classdev *led_cdev;
+	struct hid_device *hdev;
 	bool dev_registered;
+
 	ssize_t count = 0;
 	u8 i;
 
 	/* rgb_enabled is attached to led_classdev, not hid_device */
-	if (index == FEATURE_RGB_ENABLE)
-		/* Pairs with smp_store_release from gos_cfg_setup */
-		dev_registered = smp_load_acquire(&drvdata.rgb_registered);
-	else
-		/* Pairs with smp_store_release from gos_cfg_setup */
-		dev_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (index == FEATURE_RGB_ENABLE) {
+		led_cdev = dev_get_drvdata(dev);
+		led_mc = lcdev_to_mccdev(led_cdev);
+		drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
+		hdev = drvdata->hdev;
+	/* Pairs with smp_store_release from gos_cfg_setup */
+		dev_registered = smp_load_acquire(&drvdata->rgb_registered);
+
+	} else {
+		hdev = to_hid_device(dev);
+		drvdata = hid_get_drvdata(hdev);
+	/* Pairs with smp_store_release from gos_cfg_setup */
+		dev_registered = smp_load_acquire(&drvdata->gp_registered);
+	}
 
 	if (!dev_registered)
 		return -ENODEV;
 
-	count = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, index, NULL, 0);
+	count = mcu_property_out(hdev, GET_GAMEPAD_CFG, index, NULL, 0);
 	if (count < 0)
 		return count;
 
 	switch (index) {
 	case FEATURE_GAMEPAD_MODE:
-		i = drvdata.gp_mode;
+		i = drvdata->gp_mode;
 		if (i >= ARRAY_SIZE(gamepad_mode_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", gamepad_mode_text[i]);
 		break;
 	case FEATURE_AUTO_SLEEP_TIME:
-		count = sysfs_emit(buf, "%u\n", drvdata.gp_auto_sleep_time);
+		count = sysfs_emit(buf, "%u\n", drvdata->gp_auto_sleep_time);
 		break;
 	case FEATURE_IMU_ENABLE:
-		i = drvdata.imu_sensor_en;
+		i = drvdata->imu_sensor_en;
 		if (i >= ARRAY_SIZE(feature_enabled_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", feature_enabled_text[i]);
 		break;
 	case FEATURE_IMU_BYPASS:
-		i = drvdata.imu_bypass_en;
+		i = drvdata->imu_bypass_en;
 		if (i >= ARRAY_SIZE(feature_enabled_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", feature_enabled_text[i]);
 		break;
 	case FEATURE_RGB_ENABLE:
-		i = drvdata.rgb_en;
+		i = drvdata->rgb_en;
 		if (i >= ARRAY_SIZE(feature_enabled_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", feature_enabled_text[i]);
 		break;
 	case FEATURE_TOUCHPAD_ENABLE:
-		i = drvdata.tp_en;
+		i = drvdata->tp_en;
 		if (i >= ARRAY_SIZE(feature_enabled_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", feature_enabled_text[i]);
 		break;
 	case FEATURE_OS_MODE:
-		i = drvdata.os_mode;
+		i = drvdata->os_mode;
 		if (i >= ARRAY_SIZE(os_type_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", os_type_text[i]);
 		break;
 	case FEATURE_POLL_RATE:
-		i = drvdata.gp_poll_rate;
+		i = drvdata->gp_poll_rate;
 		if (i >= ARRAY_SIZE(poll_rate_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", poll_rate_text[i]);
 		break;
 	case FEATURE_DPAD_MODE:
-		i = drvdata.gp_dpad_mode;
+		i = drvdata->gp_dpad_mode;
 		if (i >= ARRAY_SIZE(dpad_mode_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", dpad_mode_text[i]);
 		break;
 	case FEATURE_MOUSE_WHEEL_STEP:
-		i = drvdata.mouse_step;
+		i = drvdata->mouse_step;
 		if (i < 1 || i > 127)
 			return -EINVAL;
 		count = sysfs_emit(buf, "%u\n", i);
@@ -757,13 +792,18 @@ static ssize_t touchpad_property_store(struct device *dev,
 				       const char *buf, size_t count,
 				       enum touchpad_config_index index)
 {
+	struct hid_device *hdev = to_hid_device(dev);
+	struct gos_cfg_drvdata *drvdata = hid_get_drvdata(hdev);
 	bool gp_registered;
 	size_t size = 1;
 	u8 val = 0;
 	int ret;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (!drvdata)
+		return -ENODEV;
+
+/* Pairs with smp_store_release from gos_cfg_setup */
+	gp_registered = smp_load_acquire(&drvdata->gp_registered);
 	if (!gp_registered)
 		return -ENODEV;
 
@@ -786,7 +826,7 @@ static ssize_t touchpad_property_store(struct device *dev,
 	if (!val)
 		size = 0;
 
-	ret = mcu_property_out(drvdata.hdev, SET_TP_PARAM, index, &val, size);
+	ret = mcu_property_out(hdev, SET_TP_PARAM, index, &val, size);
 	if (ret < 0)
 		return ret;
 
@@ -797,25 +837,30 @@ static ssize_t touchpad_property_show(struct device *dev,
 				      struct device_attribute *attr, char *buf,
 				      enum touchpad_config_index index)
 {
+	struct hid_device *hdev = to_hid_device(dev);
+	struct gos_cfg_drvdata *drvdata = hid_get_drvdata(hdev);
 	bool gp_registered;
 	int ret = 0;
 	u8 i;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (!drvdata)
+		return -ENODEV;
+
+/* Pairs with smp_store_release from gos_cfg_setup */
+	gp_registered = smp_load_acquire(&drvdata->gp_registered);
 	if (!gp_registered)
 		return -ENODEV;
 
-	ret = mcu_property_out(drvdata.hdev, GET_TP_PARAM, index, NULL, 0);
+	ret = mcu_property_out(hdev, GET_TP_PARAM, index, NULL, 0);
 	if (ret < 0)
 		return ret;
 
 	switch (index) {
 	case CFG_WINDOWS_MODE:
-		i = drvdata.tp_windows_mode;
+		i = drvdata->tp_windows_mode;
 		break;
 	case CFG_LINUX_MODE:
-		i = drvdata.tp_linux_mode;
+		i = drvdata->tp_linux_mode;
 		break;
 	default:
 		return -EINVAL;
@@ -832,15 +877,9 @@ static ssize_t touchpad_property_options(struct device *dev,
 					 char *buf,
 					 enum touchpad_config_index index)
 {
-	bool gp_registered;
 	size_t count = 0;
 	unsigned int i;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	gp_registered = smp_load_acquire(&drvdata.gp_registered);
-	if (!gp_registered)
-		return -ENODEV;
-
 	switch (index) {
 	case CFG_WINDOWS_MODE:
 	case CFG_LINUX_MODE:
@@ -863,30 +902,35 @@ static ssize_t test_property_show(struct device *dev,
 				  struct device_attribute *attr, char *buf,
 				  enum test_command_index index)
 {
+	struct hid_device *hdev = to_hid_device(dev);
+	struct gos_cfg_drvdata *drvdata = hid_get_drvdata(hdev);
 	bool gp_registered;
 	size_t count = 0;
 	u8 i;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+	if (!drvdata)
+		return -ENODEV;
+
+/* Pairs with smp_store_release from gos_cfg_setup */
+	gp_registered = smp_load_acquire(&drvdata->gp_registered);
 	if (!gp_registered)
 		return -ENODEV;
 
 	switch (index) {
 	case TEST_TP_MFR:
-		i = drvdata.tp_manufacturer;
+		i = drvdata->tp_manufacturer;
 		if (i >= ARRAY_SIZE(touchpad_manufacturer_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", touchpad_manufacturer_text[i]);
 		break;
 	case TEST_IMU_MFR:
-		i = drvdata.imu_manufacturer;
+		i = drvdata->imu_manufacturer;
 		if (i >= ARRAY_SIZE(imu_manufacturer_text))
 			return -EINVAL;
 		count = sysfs_emit(buf, "%s\n", imu_manufacturer_text[i]);
 		break;
 	case TEST_TP_VER:
-		count = sysfs_emit(buf, "%u\n", drvdata.tp_version);
+		count = sysfs_emit(buf, "%u\n", drvdata->tp_version);
 		break;
 	default:
 		count = -EINVAL;
@@ -899,7 +943,13 @@ static ssize_t test_property_show(struct device *dev,
 static ssize_t mcu_id_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
-	return sysfs_emit(buf, "%*phN\n", 12, &drvdata.mcu_id);
+	struct hid_device *hdev = to_hid_device(dev);
+	struct gos_cfg_drvdata *drvdata = hid_get_drvdata(hdev);
+
+	if (!drvdata)
+		return -ENODEV;
+
+	return sysfs_emit(buf, "%*phN\n", 12, &drvdata->mcu_id);
 }
 
 static int rgb_cfg_call(struct hid_device *hdev, enum mcu_command_index cmd,
@@ -914,27 +964,29 @@ static int rgb_cfg_call(struct hid_device *hdev, enum mcu_command_index cmd,
 	return mcu_property_out(hdev, cmd, index, val, size);
 }
 
-static int rgb_attr_show(void)
+static int rgb_attr_show(struct gos_cfg_drvdata *drvdata)
 {
 	enum rgb_config_index index;
 
-	index = drvdata.rgb_profile + 2;
+	index = drvdata->rgb_profile + 2;
 
-	return rgb_cfg_call(drvdata.hdev, GET_RGB_CFG, index, NULL, 0);
+	return rgb_cfg_call(drvdata->hdev, GET_RGB_CFG, index, NULL, 0);
 };
 
 static ssize_t rgb_effect_store(struct device *dev,
 				struct device_attribute *attr, const char *buf,
 				size_t count)
 {
-	struct led_classdev_mc *led_mc = &drvdata.led_mc;
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_mc *led_mc = lcdev_to_mccdev(led_cdev);
+	struct gos_cfg_drvdata *drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
 	enum rgb_config_index index;
 	bool rgb_registered;
 	u8 effect;
 	int ret;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!rgb_registered)
 		return -ENODEV;
 
@@ -943,41 +995,44 @@ static ssize_t rgb_effect_store(struct device *dev,
 		return ret;
 
 	effect = ret;
-	index = drvdata.rgb_profile + 2;
+	index = drvdata->rgb_profile + 2;
 	u8 rgb_profile[6] = { effect,
 			      led_mc->subled_info[0].intensity,
 			      led_mc->subled_info[1].intensity,
 			      led_mc->subled_info[2].intensity,
 			      led_mc->led_cdev.brightness,
-			      drvdata.rgb_speed };
+			      drvdata->rgb_speed };
 
-	ret = rgb_cfg_call(drvdata.hdev, SET_RGB_CFG, index, rgb_profile, 6);
+	ret = rgb_cfg_call(drvdata->hdev, SET_RGB_CFG, index, rgb_profile, 6);
 	if (ret)
 		return ret;
 
-	drvdata.rgb_effect = effect;
+	drvdata->rgb_effect = effect;
 	return count;
 };
 
 static ssize_t rgb_effect_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_mc *led_mc = lcdev_to_mccdev(led_cdev);
+	struct gos_cfg_drvdata *drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
 	bool rgb_registered;
 	int ret;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!rgb_registered)
 		return -ENODEV;
 
-	ret = rgb_attr_show();
+	ret = rgb_attr_show(drvdata);
 	if (ret)
 		return ret;
 
-	if (drvdata.rgb_effect >= ARRAY_SIZE(rgb_effect_text))
+	if (drvdata->rgb_effect >= ARRAY_SIZE(rgb_effect_text))
 		return -EINVAL;
 
-	return sysfs_emit(buf, "%s\n", rgb_effect_text[drvdata.rgb_effect]);
+	return sysfs_emit(buf, "%s\n", rgb_effect_text[drvdata->rgb_effect]);
 }
 
 static ssize_t rgb_effect_index_show(struct device *dev,
@@ -999,14 +1054,16 @@ static ssize_t rgb_speed_store(struct device *dev,
 			       struct device_attribute *attr, const char *buf,
 			       size_t count)
 {
-	struct led_classdev_mc *led_mc = &drvdata.led_mc;
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_mc *led_mc = lcdev_to_mccdev(led_cdev);
+	struct gos_cfg_drvdata *drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
 	enum rgb_config_index index;
 	bool rgb_registered;
 	int val = 0;
 	int ret;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!rgb_registered)
 		return -ENODEV;
 
@@ -1017,19 +1074,19 @@ static ssize_t rgb_speed_store(struct device *dev,
 	if (val < 0 || val > 100)
 		return -EINVAL;
 
-	index = drvdata.rgb_profile + 2;
-	u8 rgb_profile[6] = { drvdata.rgb_effect,
+	index = drvdata->rgb_profile + 2;
+	u8 rgb_profile[6] = { drvdata->rgb_effect,
 			      led_mc->subled_info[0].intensity,
 			      led_mc->subled_info[1].intensity,
 			      led_mc->subled_info[2].intensity,
 			      led_mc->led_cdev.brightness,
 			      val };
 
-	ret = rgb_cfg_call(drvdata.hdev, SET_RGB_CFG, index, rgb_profile, 6);
+	ret = rgb_cfg_call(drvdata->hdev, SET_RGB_CFG, index, rgb_profile, 6);
 	if (ret)
 		return ret;
 
-	drvdata.rgb_speed = val;
+	drvdata->rgb_speed = val;
 
 	return count;
 };
@@ -1037,22 +1094,25 @@ static ssize_t rgb_speed_store(struct device *dev,
 static ssize_t rgb_speed_show(struct device *dev, struct device_attribute *attr,
 			      char *buf)
 {
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_mc *led_mc = lcdev_to_mccdev(led_cdev);
+	struct gos_cfg_drvdata *drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
 	bool rgb_registered;
 	int ret;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!rgb_registered)
 		return -ENODEV;
 
-	ret = rgb_attr_show();
+	ret = rgb_attr_show(drvdata);
 	if (ret)
 		return ret;
 
-	if (drvdata.rgb_speed > 100)
+	if (drvdata->rgb_speed > 100)
 		return -EINVAL;
 
-	return sysfs_emit(buf, "%hhu\n", drvdata.rgb_speed);
+	return sysfs_emit(buf, "%hhu\n", drvdata->rgb_speed);
 }
 
 static ssize_t rgb_speed_range_show(struct device *dev,
@@ -1064,12 +1124,15 @@ static ssize_t rgb_speed_range_show(struct device *dev,
 static ssize_t rgb_mode_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_mc *led_mc = lcdev_to_mccdev(led_cdev);
+	struct gos_cfg_drvdata *drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
 	bool rgb_registered;
 	int ret;
 	u8 val;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!rgb_registered)
 		return -ENODEV;
 
@@ -1079,12 +1142,12 @@ static ssize_t rgb_mode_store(struct device *dev, struct device_attribute *attr,
 
 	val = ret;
 
-	ret = rgb_cfg_call(drvdata.hdev, SET_RGB_CFG, LIGHT_MODE_SEL, &val,
+	ret = rgb_cfg_call(drvdata->hdev, SET_RGB_CFG, LIGHT_MODE_SEL, &val,
 			   1);
 	if (ret)
 		return ret;
 
-	drvdata.rgb_mode = val;
+	drvdata->rgb_mode = val;
 
 	return count;
 };
@@ -1092,22 +1155,25 @@ static ssize_t rgb_mode_store(struct device *dev, struct device_attribute *attr,
 static ssize_t rgb_mode_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_mc *led_mc = lcdev_to_mccdev(led_cdev);
+	struct gos_cfg_drvdata *drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
 	bool rgb_registered;
 	int ret;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!rgb_registered)
 		return -ENODEV;
 
-	ret = rgb_cfg_call(drvdata.hdev, GET_RGB_CFG, LIGHT_MODE_SEL, NULL, 0);
+	ret = rgb_cfg_call(drvdata->hdev, GET_RGB_CFG, LIGHT_MODE_SEL, NULL, 0);
 	if (ret)
 		return ret;
 
-	if (drvdata.rgb_mode >= ARRAY_SIZE(rgb_mode_text))
+	if (drvdata->rgb_mode >= ARRAY_SIZE(rgb_mode_text))
 		return -EINVAL;
 
-	return sysfs_emit(buf, "%s\n", rgb_mode_text[drvdata.rgb_mode]);
+	return sysfs_emit(buf, "%s\n", rgb_mode_text[drvdata->rgb_mode]);
 };
 
 static ssize_t rgb_mode_index_show(struct device *dev,
@@ -1129,13 +1195,16 @@ static ssize_t rgb_profile_store(struct device *dev,
 				 struct device_attribute *attr, const char *buf,
 				 size_t count)
 {
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_mc *led_mc = lcdev_to_mccdev(led_cdev);
+	struct gos_cfg_drvdata *drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
 	bool rgb_registered;
 	size_t size = 1;
 	int ret;
 	u8 val;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!rgb_registered)
 		return -ENODEV;
 
@@ -1146,11 +1215,11 @@ static ssize_t rgb_profile_store(struct device *dev,
 	if (val < 1 || val > 3)
 		return -EINVAL;
 
-	ret = rgb_cfg_call(drvdata.hdev, SET_RGB_CFG, LIGHT_PROFILE_SEL, &val, size);
+	ret = rgb_cfg_call(drvdata->hdev, SET_RGB_CFG, LIGHT_PROFILE_SEL, &val, size);
 	if (ret)
 		return ret;
 
-	drvdata.rgb_profile = val;
+	drvdata->rgb_profile = val;
 
 	return count;
 };
@@ -1158,22 +1227,25 @@ static ssize_t rgb_profile_store(struct device *dev,
 static ssize_t rgb_profile_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct led_classdev_mc *led_mc = lcdev_to_mccdev(led_cdev);
+	struct gos_cfg_drvdata *drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
 	bool rgb_registered;
 	int ret;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!rgb_registered)
 		return -ENODEV;
 
-	ret = rgb_cfg_call(drvdata.hdev, GET_RGB_CFG, LIGHT_PROFILE_SEL, NULL, 0);
+	ret = rgb_cfg_call(drvdata->hdev, GET_RGB_CFG, LIGHT_PROFILE_SEL, NULL, 0);
 	if (ret)
 		return ret;
 
-	if (drvdata.rgb_profile < 1 || drvdata.rgb_profile > 3)
+	if (drvdata->rgb_profile < 1 || drvdata->rgb_profile > 3)
 		return -EINVAL;
 
-	return sysfs_emit(buf, "%hhu\n", drvdata.rgb_profile);
+	return sysfs_emit(buf, "%hhu\n", drvdata->rgb_profile);
 };
 
 static ssize_t rgb_profile_range_show(struct device *dev,
@@ -1185,13 +1257,14 @@ static ssize_t rgb_profile_range_show(struct device *dev,
 static void hid_gos_brightness_set(struct led_classdev *led_cdev,
 				   enum led_brightness brightness)
 {
-	struct led_classdev_mc *led_mc = &drvdata.led_mc;
+	struct led_classdev_mc *led_mc = lcdev_to_mccdev(led_cdev);
+	struct gos_cfg_drvdata *drvdata = container_of(led_mc, struct gos_cfg_drvdata, led_mc);
 	enum rgb_config_index index;
 	bool rgb_registered;
 	int ret;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!rgb_registered)
 		return;
 
@@ -1200,15 +1273,15 @@ static void hid_gos_brightness_set(struct led_classdev *led_cdev,
 		return;
 	}
 
-	index = drvdata.rgb_profile + 2;
-	u8 rgb_profile[6] = { drvdata.rgb_effect,
+	index = drvdata->rgb_profile + 2;
+	u8 rgb_profile[6] = { drvdata->rgb_effect,
 			      led_mc->subled_info[0].intensity,
 			      led_mc->subled_info[1].intensity,
 			      led_mc->subled_info[2].intensity,
 			      brightness,
-			      drvdata.rgb_speed };
+			      drvdata->rgb_speed };
 
-	ret = rgb_cfg_call(drvdata.hdev, SET_RGB_CFG, index, rgb_profile, 6);
+	ret = rgb_cfg_call(drvdata->hdev, SET_RGB_CFG, index, rgb_profile, 6);
 	switch (ret) {
 	case 0:
 		led_cdev->brightness = brightness;
@@ -1446,127 +1519,138 @@ static struct mc_subled gos_rgb_subled_info[] = {
 
 static void cfg_setup(struct work_struct *work)
 {
+	struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+	struct gos_cfg_drvdata *drvdata = container_of(dwork, struct gos_cfg_drvdata,
+						       gos_cfg_setup);
 	bool gp_registered, rgb_registered;
 	int ret;
 
+	if (!drvdata)
+		return;
+
 	/* MCU */
-	ret = mcu_property_out(drvdata.hdev, GET_MCU_ID, FEATURE_NONE, NULL, 0);
+	ret = mcu_property_out(drvdata->hdev, GET_MCU_ID, FEATURE_NONE, NULL, 0);
 	if (ret) {
-		dev_err(&drvdata.hdev->dev, "Failed to retrieve MCU ID: %i\n",
+		dev_err(&drvdata->hdev->dev, "Failed to retrieve MCU ID: %i\n",
 			ret);
 		return;
 	}
 
-	ret = mcu_property_out(drvdata.hdev, GET_VERSION, FEATURE_NONE, NULL, 0);
+	ret = mcu_property_out(drvdata->hdev, GET_VERSION, FEATURE_NONE, NULL, 0);
 	if (ret) {
-		dev_err(&drvdata.hdev->dev, "Failed to retrieve MCU Version: %i\n", ret);
+		dev_err(&drvdata->hdev->dev, "Failed to retrieve MCU Version: %i\n", ret);
 		return;
 	}
 
-	ret = mcu_property_out(drvdata.hdev, GET_PL_TEST, TEST_TP_MFR, NULL, 0);
+	ret = mcu_property_out(drvdata->hdev, GET_PL_TEST, TEST_TP_MFR, NULL, 0);
 	if (ret) {
-		dev_err(&drvdata.hdev->dev,
+		dev_err(&drvdata->hdev->dev,
 			"Failed to retrieve Touchpad Manufacturer: %i\n", ret);
 		return;
 	}
 
-	ret = mcu_property_out(drvdata.hdev, GET_PL_TEST, TEST_TP_VER, NULL, 0);
+	ret = mcu_property_out(drvdata->hdev, GET_PL_TEST, TEST_TP_VER, NULL, 0);
 	if (ret) {
-		dev_err(&drvdata.hdev->dev,
+		dev_err(&drvdata->hdev->dev,
 			"Failed to retrieve Touchpad Firmware Version: %i\n", ret);
 		return;
 	}
 
-	ret = mcu_property_out(drvdata.hdev, GET_PL_TEST, TEST_IMU_MFR, NULL, 0);
+	ret = mcu_property_out(drvdata->hdev, GET_PL_TEST, TEST_IMU_MFR, NULL, 0);
 	if (ret) {
-		dev_err(&drvdata.hdev->dev,
+		dev_err(&drvdata->hdev->dev,
 			"Failed to retrieve IMU Manufacturer: %i\n", ret);
 		return;
 	}
 
-	ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, FEATURE_OS_MODE,
+	ret = mcu_property_out(drvdata->hdev, GET_GAMEPAD_CFG, FEATURE_OS_MODE,
 			       NULL, 0);
 	if (ret) {
-		dev_err(&drvdata.hdev->dev,
+		dev_err(&drvdata->hdev->dev,
 			"Failed to retrieve OS Mode: %i\n", ret);
 		return;
 	}
 
-	/* Pairs with smp_store_release from below */
-	gp_registered = smp_load_acquire(&drvdata.gp_registered);
+/* Pairs with smp_store_release from below */
+	gp_registered = smp_load_acquire(&drvdata->gp_registered);
 	if (gp_registered)
 		goto try_rgb;
 
-	ret = sysfs_create_groups(&drvdata.hdev->dev.kobj, top_level_attr_groups);
+	ret = sysfs_create_groups(&drvdata->hdev->dev.kobj, top_level_attr_groups);
 	if (ret) {
-		dev_err(&drvdata.hdev->dev,
+		dev_err(&drvdata->hdev->dev,
 			"Failed to create gamepad configuration attributes: %i\n", ret);
 		goto try_rgb;
 	}
 
-	/* Pairs with smp_load_acquire in attribute show/store functions */
-	smp_store_release(&drvdata.gp_registered, true);
+/* Pairs with smp_load_acquire in attribute show/store functions */
+	smp_store_release(&drvdata->gp_registered, true);
 	gp_registered = true;
 
 try_rgb:
-	/* Pairs with smp_store_release from below */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+/* Pairs with smp_store_release from below */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (rgb_registered)
 		goto update_kobjects;
 
-	ret = devm_led_classdev_multicolor_register(&drvdata.hdev->dev, &drvdata.led_mc);
+	ret = devm_led_classdev_multicolor_register(&drvdata->hdev->dev, &drvdata->led_mc);
 	if (ret) {
-		dev_err(&drvdata.hdev->dev,
+		dev_err(&drvdata->hdev->dev,
 			"Failed to create RGB device: %i\n", ret);
 		goto update_kobjects;
 	}
 
-	ret = devm_device_add_group(drvdata.led_mc.led_cdev.dev, &rgb_attr_group);
+	ret = devm_device_add_group(drvdata->led_mc.led_cdev.dev, &rgb_attr_group);
 	if (ret) {
-		dev_err(&drvdata.hdev->dev,
+		dev_err(&drvdata->hdev->dev,
 			"Failed to create RGB configuration attributes: %i\n", ret);
 		goto update_kobjects;
 	}
 
-	/* Pairs with smp_load_acquire in attribute show/store functions */
-	smp_store_release(&drvdata.rgb_registered, true);
+/* Pairs with smp_load_acquire in attribute show/store functions */
+	smp_store_release(&drvdata->rgb_registered, true);
 	rgb_registered = true;
 
 update_kobjects:
 	if (gp_registered)
-		kobject_uevent(&drvdata.hdev->dev.kobj, KOBJ_CHANGE);
+		kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
 	if (rgb_registered)
-		kobject_uevent(&drvdata.led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
+		kobject_uevent(&drvdata->led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
 }
 
 static int hid_gos_cfg_probe(struct hid_device *hdev,
 			     const struct hid_device_id *_id)
 {
+	struct gos_cfg_drvdata *drvdata;
 	int ret;
 
-	hid_set_drvdata(hdev, &drvdata);
-	drvdata.hdev = hdev;
+	drvdata = devm_kzalloc(&hdev->dev, sizeof(struct gos_cfg_drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	hid_set_drvdata(hdev, drvdata);
+	drvdata->hdev = hdev;
 
-	mutex_init(&drvdata.cfg_mutex);
-	init_completion(&drvdata.send_cmd_complete);
+	mutex_init(&drvdata->cfg_mutex);
+	init_completion(&drvdata->send_cmd_complete);
 
 	/* Device is hardwired and name is guaranteed to be unique */
-	drvdata.led_mc.led_cdev.name = "go_s:rgb:joystick_rings";
-	drvdata.led_mc.led_cdev.brightness = 0x50;
-	drvdata.led_mc.led_cdev.max_brightness = 0x64;
-	drvdata.led_mc.led_cdev.color = LED_COLOR_ID_RGB;
-	drvdata.led_mc.led_cdev.brightness_set = hid_gos_brightness_set;
-	drvdata.led_mc.num_colors = 3;
-	drvdata.led_mc.subled_info = devm_kmemdup(&hdev->dev, gos_rgb_subled_info,
-						  sizeof(gos_rgb_subled_info), GFP_KERNEL);
-	if (!drvdata.led_mc.subled_info)
+	drvdata->led_mc.led_cdev.name = "go_s:rgb:joystick_rings";
+	drvdata->led_mc.led_cdev.brightness = 0x50;
+	drvdata->led_mc.led_cdev.max_brightness = 0x64;
+	drvdata->led_mc.led_cdev.color = LED_COLOR_ID_RGB;
+	drvdata->led_mc.led_cdev.brightness_set = hid_gos_brightness_set;
+	drvdata->led_mc.num_colors = 3;
+	drvdata->led_mc.subled_info = devm_kmemdup(&hdev->dev, gos_rgb_subled_info,
+						   sizeof(gos_rgb_subled_info), GFP_KERNEL);
+	if (!drvdata->led_mc.subled_info)
 		return -ENOMEM;
 
 	/* Executing calls prior to returning from probe will lock the MCU. Schedule
 	 * initial data call after probe has completed and MCU can accept calls.
 	 */
-	INIT_DELAYED_WORK(&drvdata.gos_cfg_setup, &cfg_setup);
-	ret = schedule_delayed_work(&drvdata.gos_cfg_setup, msecs_to_jiffies(2));
+	INIT_DELAYED_WORK(&drvdata->gos_cfg_setup, &cfg_setup);
+	ret = schedule_delayed_work(&drvdata->gos_cfg_setup, msecs_to_jiffies(2));
 	if (!ret) {
 		dev_err(&hdev->dev, "Failed to schedule startup delayed work\n");
 		return -ENODEV;
@@ -1577,7 +1661,12 @@ static int hid_gos_cfg_probe(struct hid_device *hdev,
 
 static void hid_gos_cfg_remove(struct hid_device *hdev)
 {
-	disable_delayed_work_sync(&drvdata.gos_cfg_setup);
+	struct gos_cfg_drvdata *drvdata = hid_get_drvdata(hdev);
+
+	if (!drvdata)
+		return;
+
+	disable_delayed_work_sync(&drvdata->gos_cfg_setup);
 	sysfs_remove_groups(&hdev->dev.kobj, top_level_attr_groups);
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
@@ -1586,16 +1675,21 @@ static void hid_gos_cfg_remove(struct hid_device *hdev)
 
 static int hid_gos_cfg_reset_resume(struct hid_device *hdev)
 {
+	struct gos_cfg_drvdata *drvdata = hid_get_drvdata(hdev);
 	bool gp_registered, rgb_registered;
-	u8 os_mode = drvdata.os_mode;
+	u8 os_mode;
 	int ret;
 
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	gp_registered = smp_load_acquire(&drvdata.gp_registered);
-	/* Pairs with smp_store_release from gos_cfg_setup */
-	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
+	if (!drvdata)
+		return -ENODEV;
+
+	os_mode = drvdata->os_mode;
+/* Pairs with smp_store_release from gos_cfg_setup */
+	gp_registered = smp_load_acquire(&drvdata->gp_registered);
+/* Pairs with smp_store_release from gos_cfg_setup */
+	rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
 	if (!gp_registered || !rgb_registered) {
-		ret = schedule_delayed_work(&drvdata.gos_cfg_setup, msecs_to_jiffies(2));
+		ret = schedule_delayed_work(&drvdata->gos_cfg_setup, msecs_to_jiffies(2));
 		if (!ret) {
 			dev_err(&hdev->dev, "Failed to schedule startup delayed work\n");
 			return -ENODEV;
@@ -1604,20 +1698,20 @@ static int hid_gos_cfg_reset_resume(struct hid_device *hdev)
 		return 0;
 	}
 
-	ret = mcu_property_out(drvdata.hdev, SET_GAMEPAD_CFG,
+	ret = mcu_property_out(drvdata->hdev, SET_GAMEPAD_CFG,
 			       FEATURE_OS_MODE, &os_mode, 1);
 	if (ret < 0)
 		return ret;
 
-	ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG,
+	ret = mcu_property_out(drvdata->hdev, GET_GAMEPAD_CFG,
 			       FEATURE_OS_MODE, NULL, 0);
 	if (ret < 0)
 		return ret;
 
-	if (drvdata.os_mode != os_mode)
+	if (drvdata->os_mode != os_mode)
 		return -ENODEV;
 
-	kobject_uevent(&drvdata.hdev->dev.kobj, KOBJ_CHANGE);
+	kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
 	return 0;
 }
 
@@ -1692,9 +1786,14 @@ static int hid_gos_reset_resume(struct hid_device *hdev)
 	return 0;
 }
 
-static int hid_gos_cfg_suspend(void)
+static int hid_gos_cfg_suspend(struct hid_device *hdev)
 {
-	disable_delayed_work_sync(&drvdata.gos_cfg_setup);
+	struct gos_cfg_drvdata *drvdata = hid_get_drvdata(hdev);
+
+	if (!drvdata)
+		return -ENODEV;
+
+	disable_delayed_work_sync(&drvdata->gos_cfg_setup);
 
 	return 0;
 }
@@ -1711,7 +1810,7 @@ static int hid_gos_suspend(struct hid_device *hdev, pm_message_t msg)
 
 	ep = ret;
 	if (ep == GO_S_CFG_INTF_IN)
-		return hid_gos_cfg_suspend();
+		return hid_gos_cfg_suspend(hdev);
 
 	return 0;
 }
-- 
2.55.0

Re: [PATCH 02/10] HID: hid-lenovo-go-s: Bound stale reply window before reusing send_cmd_complete

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] hid: conflating MCU error with timeout in mcu_property_out()
--

commit b76b36bd4347a4e16200e7b5eadeaf196d8595bd
Author: Derek J. Clark <derekjohn.clark@gmail.com>
Subject: HID: hid-lenovo-go-s: Bound stale reply window before reusing send_cmd_complete

This patch mitigates an issue where stale replies to timed-out commands can
silently satisfy the wait for the next command. It introduces a short bounded
wait before reusing the completion structure and tracks when a command has
timed out using a new pending flag.

> diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
> index 68301d4c762a1..36505d8402ff1 100644
> --- a/drivers/hid/hid-lenovo-go-s.c
> +++ b/drivers/hid/hid-lenovo-go-s.c
> @@ -479,7 +493,9 @@ static int mcu_property_out(struct hid_device *hdev, u8 command, u8 index,
>  							msecs_to_jiffies(timeout));
>  	ret = ret > 0 ? drvdata.cmd_status : ret ?: -EBUSY;
>  
> -	reinit_completion(&drvdata.send_cmd_complete);
> +	if (ret)
> +		drvdata.orphan_ack_pending = true;

[Severity: Low]
Could this logic inadvertently flag a successfully received error reply as an
orphaned timeout?

When the MCU successfully replies with an error code (meaning drvdata.cmd_status
is negative), wait_for_completion_interruptible_timeout() returns a positive
value indicating the wait succeeded. The code then assigns ret =
drvdata.cmd_status, causing the if (ret) check to evaluate to true. 

Since the reply was successfully received and consumed, there doesn't seem to be
an orphaned ACK, but this will set orphan_ack_pending = true and penalize the
next unrelated command with an unnecessary 25ms delay. Is this the intended
behavior, or should it specifically check for the timeout condition instead?

> +
>  	return ret;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914225303.868569-1-derekjohn.clark@gmail.com?part=2

Re: [PATCH 03/10] HID: hid-lenovo-go-s: Prevent deadlock if device removed during setup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hid-lenovo-go-s: UAF in usbhid_output_report due to unprotected sysfs access during remove
--

commit 5d8a6b9ab527316bf05d3abf3dffc7d80baf351f
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-lenovo-go-s: Prevent deadlock if device removed during setup

This commit fixes a deadlock by removing the cfg_mutex guard in
hid_gos_cfg_remove() before stopping the delayed work. It also switches
from cancel_delayed_work_sync() to disable_delayed_work_sync().

> diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
> index 36505d8402ff1..8b16fc7d151e0 100644
> --- a/drivers/hid/hid-lenovo-go-s.c
> +++ b/drivers/hid/hid-lenovo-go-s.c
> @@ -1442,8 +1442,7 @@ static int hid_gos_cfg_probe(struct hid_device *hdev,
>  
>  static void hid_gos_cfg_remove(struct hid_device *hdev)
>  {
> -	guard(mutex)(&drvdata.cfg_mutex);
> -	cancel_delayed_work_sync(&drvdata.gos_cfg_setup);
> +	disable_delayed_work_sync(&drvdata.gos_cfg_setup);

[Severity: High]
Does removing this mutex guard expose hid_hw_stop() to concurrent sysfs
accesses, potentially leading to a use-after-free?

Because the LED class device is registered using devres, its sysfs
attributes remain active until after hid_gos_cfg_remove() returns. Without
this mutex, a concurrent sysfs write to the LED brightness or RGB effect
attributes can invoke mcu_property_out() and then hid_hw_output_report()
while hid_hw_stop() is executing.

When hid_hw_stop() calls down to usbhid_stop(), it frees usbhid->urbout
and sets it to NULL without locking against concurrent
usbhid_output_report() calls. A racing usbhid_output_report() could pass
the !usbhid->urbout check and dereference the freed URB in
usb_interrupt_msg(), leading to a kernel panic if a local user writes to
sysfs during device hot-unplug.

Could the mutex be reacquired after disable_delayed_work_sync() to protect
the remaining teardown sequence?

>  	sysfs_remove_groups(&hdev->dev.kobj, top_level_attr_groups);
>  	hid_hw_close(hdev);
>  	hid_hw_stop(hdev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914225303.868569-1-derekjohn.clark@gmail.com?part=3

Re: [PATCH 07/10] HID: hid-lenovo-go-s: Move attribute init to after device query

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] hid: hid-lenovo-go-s: double register of LED device corrupts leds_list
- [High] hid: hid-lenovo-go-s: permanently disabled workqueue breaks resume
- [Medium] sysfs: hid-lenovo-go-s: attributes return -ENODEV immediately after creation
--

commit 71c67b9719643790013b558cabadd6b89996f3ca
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-lenovo-go-s: Move attribute init to after device query

Currently this driver registers attributes before the MCU is ready to
accept commands, which can lead to attribute access that blocks the MCU
in some rare cases. Move attribute construction to gos_cfg_setup, after
the device has been queried.

> diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
> index 6d98fff6d987e..f1a0bcfcdb71a 100644
> --- a/drivers/hid/hid-lenovo-go-s.c
> +++ b/drivers/hid/hid-lenovo-go-s.c
[ ... ]
> @@ -1497,10 +1498,53 @@ static void cfg_setup(struct work_struct *work)
>  		return;
>  	}
>  
> +	/* Pairs with smp_store_release from below */
> +	gp_registered = smp_load_acquire(&drvdata.gp_registered);
> +	if (gp_registered)
> +		goto try_rgb;
> +
> +	ret = sysfs_create_groups(&drvdata.hdev->dev.kobj, top_level_attr_groups);
> +	if (ret) {
> +		dev_err(&drvdata.hdev->dev,
> +			"Failed to create gamepad configuration attributes: %i\n", ret);
> +		goto try_rgb;
> +	}
> +
>  	/* Pairs with smp_load_acquire in attribute show/store functions */
>  	smp_store_release(&drvdata.gp_registered, true);

[Severity: Medium]
After sysfs_create_groups() exposes the attributes to userspace, could a
fast userspace process read them and invoke gamepad_property_show() before
smp_store_release(&drvdata.gp_registered, true) executes?

If so, would this cause spurious -ENODEV errors for userspace, violating
the requirement that all private data needed by userspace callbacks must be
complete before publishing the interface?

> +	gp_registered = true;
> +
> +try_rgb:
> +	/* Pairs with smp_store_release from below */
> +	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
> +	if (rgb_registered)
> +		goto update_kobjects;
> +
> +	ret = devm_led_classdev_multicolor_register(&drvdata.hdev->dev, &gos_cdev_rgb);
> +	if (ret) {
> +		dev_err(&drvdata.hdev->dev,
> +			"Failed to create RGB device: %i\n", ret);
> +		goto update_kobjects;
> +	}
> +
> +	ret = devm_device_add_group(gos_cdev_rgb.led_cdev.dev, &rgb_attr_group);
> +	if (ret) {
> +		dev_err(&drvdata.hdev->dev,
> +			"Failed to create RGB configuration attributes: %i\n", ret);
> +		goto update_kobjects;
> +	}

[Severity: High]
If devm_device_add_group() fails, cfg_setup() jumps to update_kobjects without
setting rgb_registered = true, and without unregistering the LED device
successfully created by devm_led_classdev_multicolor_register().

If a USB reset occurs, hid_gos_cfg_reset_resume() sees !rgb_registered and
reschedules cfg_setup(). When it runs again, it will call
devm_led_classdev_multicolor_register() on gos_cdev_rgb which is already
actively registered.

Does this execute list_add_tail() on a node already in the subsystem list,
corrupting it?

> +
> +	drvdata.led_cdev = &gos_cdev_rgb.led_cdev;
> +
>  	/* Pairs with smp_load_acquire in attribute show/store functions */
>  	smp_store_release(&drvdata.rgb_registered, true);
> +	rgb_registered = true;
> +
> +update_kobjects:
> +	if (gp_registered)
> +		kobject_uevent(&drvdata.hdev->dev.kobj, KOBJ_CHANGE);
> +	if (rgb_registered)
> +		kobject_uevent(&drvdata.led_cdev->dev->kobj, KOBJ_CHANGE);
>  }

[Severity: High]
The commit message mentions that a resume before setup has run shouldn't
prevent attribute creation. However, can the device fail to resume and
become unusable after a suspend/resume cycle?

If the system suspends, hid_gos_cfg_suspend() permanently disables the work
item:

hid_gos_cfg_suspend() {
    disable_delayed_work_sync(&drvdata.gos_cfg_setup);
    ...
}

When the system wakes, if cfg_setup() hadn't finished (e.g. within the 2ms
delay) or failed, hid_gos_cfg_reset_resume() will retry scheduling the work:

hid_gos_cfg_reset_resume() {
    ...
    if (!gp_registered || !rgb_registered) {
        ret = schedule_delayed_work(&drvdata.gos_cfg_setup, msecs_to_jiffies(2));
        if (!ret) {
            dev_err(&hdev->dev, "Failed to schedule startup delayed work\n");
            return -ENODEV;
        }
        return 0;
    }
    ...
}

Because the work is permanently disabled and never re-enabled with
enable_delayed_work(), schedule_delayed_work() will return 0. Will this
cause hid_gos_cfg_reset_resume() to return -ENODEV, failing the resume
sequence entirely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914225303.868569-1-derekjohn.clark@gmail.com?part=7

Re: [PATCH 06/10] HID: hid-lenovo-go-s: Add suspend function and gate access under bool.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] hid: hid-lenovo-go-s: Suspend callback permanently disables setup work, breaking resume
- [Medium] hid: hid-lenovo-go-s: Transient MCU query failures permanently lock out sysfs configuration
--

commit 4cf4113a0e592276fc9988337678d1232b964a33
Author: Derek J. Clark <derekjohn.clark@gmail.com>

HID: hid-lenovo-go-s: Add suspend function and gate access under bool.

This commit adds a suspend function to cancel pending delayed work during
system sleep. It also introduces boolean flags to prevent sysfs attribute
access prior to the configuration setup completing.

> diff --git a/drivers/hid/hid-lenovo-go-s.c b/drivers/hid/hid-lenovo-go-s.c
> index 712823e22beae..6d98fff6d987e 100644
> --- a/drivers/hid/hid-lenovo-go-s.c
> +++ b/drivers/hid/hid-lenovo-go-s.c

[ ... ]

>  static void cfg_setup(struct work_struct *work)
>  {
>  	int ret;
>  
>  	/* MCU */
>  	ret = mcu_property_out(drvdata.hdev, GET_MCU_ID, FEATURE_NONE, NULL, 0);
>  	if (ret) {
>  		dev_err(&drvdata.hdev->dev, "Failed to retrieve MCU ID: %i\n",
>  			ret);
>  		return;

[Severity: Medium]
If one of these initial MCU queries fails due to a transient error or timeout,
does this early return prevent the gp_registered and rgb_registered flags from
ever being set? Since there is no retry mechanism here, this could leave the
sysfs configuration permanently returning -ENODEV until the device is
replugged.

>  	}

[ ... ]

>  static int hid_gos_cfg_reset_resume(struct hid_device *hdev)
>  {
> +	bool gp_registered, rgb_registered;
>  	u8 os_mode = drvdata.os_mode;
>  	int ret;
>  
> +	/* Pairs with smp_store_release from gos_cfg_setup */
> +	gp_registered = smp_load_acquire(&drvdata.gp_registered);
> +	/* Pairs with smp_store_release from gos_cfg_setup */
> +	rgb_registered = smp_load_acquire(&drvdata.rgb_registered);
> +	if (!gp_registered || !rgb_registered) {
> +		ret = schedule_delayed_work(&drvdata.gos_cfg_setup, msecs_to_jiffies(2));
> +		if (!ret) {
> +			dev_err(&hdev->dev, "Failed to schedule startup delayed work\n");
> +			return -ENODEV;
> +		}
> +
> +		return 0;
> +	}

[ ... ]

> +static int hid_gos_cfg_suspend(void)
> +{
> +	disable_delayed_work_sync(&drvdata.gos_cfg_setup);

[Severity: High]
Does using disable_delayed_work_sync() here permanently disable the work
queue since there is no matching enable_delayed_work() call?

If the work queue is permanently disabled, schedule_delayed_work() in
hid_gos_cfg_reset_resume() will return false. Would treating a false return
from schedule_delayed_work() as a fatal error cause the resume operation to
abort unconditionally with -ENODEV?

Could cancel_delayed_work_sync() be used instead to cancel the work without
disabling it?

> +
> +	return 0;
> +}

[ ... ]

While looking at this driver, I noticed a pre-existing issue in
hid_gos_probe():

>  static int hid_gos_probe(struct hid_device *hdev,
>  			 const struct hid_device_id *id)
>  {
[ ... ]
>  	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>  	if (ret) {
>  		hid_err(hdev, "Failed to start HID device\n");
>  		return ret;
>  	}
>  
>  	ret = hid_hw_open(hdev);
>  	if (ret) {
>  		hid_err(hdev, "Failed to open HID device\n");
>  		hid_hw_stop(hdev);
>  		return ret;
>  	}
>  
>  	ep = get_endpoint_address(hdev);
>  	if (ep != GO_S_CFG_INTF_IN) {
>  		dev_dbg(&hdev->dev, "Started interface %x as generic HID device.\n", ep);
>  		return 0;
>  	}
>  
>  	ret = hid_gos_cfg_probe(hdev, id);
>  	if (ret)
>  		dev_err_probe(&hdev->dev, ret, "Failed to start configuration interface");
>  
>  	dev_dbg(&hdev->dev, "Started interface %x as Go S configuration interface\n", ep);
>  	return ret;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but if hid_gos_cfg_probe() fails,
does this error path return without calling hid_hw_stop() and hid_hw_close()?
This would leave the hardware active and leak resources since the HID core
does not automatically invoke the .remove() callback on a probe failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914225303.868569-1-derekjohn.clark@gmail.com?part=6