* [PATCH 0/6] HID: Add Legion Go S Driver
@ 2025-07-03 0:49 Derek J. Clark
2025-07-03 0:49 ` [PATCH 1/6] HID: Include firmware version in the uevent Derek J. Clark
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Derek J. Clark @ 2025-07-03 0:49 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Mario Limonciello, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson,
Pierre-Loup A . Griffais, Derek J . Clark, linux-input, linux-doc,
linux-kernel
This series adds initial support for the Legion Go S's built-in
controller HID configuration interface. In the first patch a new HID
uevent property is added, HID_FIRMWARE_VERSION, so as to permit fwupd
to read the firmware version of the HID interface without detaching the
kernel driver. The second patch adds the ability for an hid_driver to
assign new/arbitrary uevent properties for static data that doesn't
benefit from having a sysfs entry. The third patch adds the VID and PID
for the Lenovo Legion Go S MCU. The fourth patch adds ABI documentation
for the config interface introduced in the final patch. The fifth patch
introduces the core lenovo-legos-hid driver which acts as a routing
interface for the different endpoints. The sixth path introduces the
config lenovo-legos-hid driver wich uses both the HID_FIRMWARE_VERSION
as well as arbitrary uevent properties. Additional interfaces and config
properties are planned to be added in a future series.
Patch 6 introduces a checkpatch WARNING that I'm unable to resolve:
WARNING: ENOSYS means 'invalid syscall nr' and nothing else
1292: FILE: drivers/hid/lenovo-legos-hid/lenovo-legos-hid-config.c:1085:
+ case -ENOSYS: /* during rmmod -ENOSYS is expected */
This error handling case was added as it is experienced in the real world
when the driver is rmmod. The LED subsystem produces this error code in
its legacy code and this is not a new novel use of -ENOSYS, we are simply
catching the case to avoid spurious errors in dmesg when the driver is
removed. If there is a way to prevent this error from being triggered by
checkpatch in the first place, that would be an ideal remedy, but I'm not
aware how that can be done at this time.
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
Derek J. Clark (4):
HID: Add Legion Go S ID's
HID: Add documentation for lenovo-legos-hid driver
HID: Add lenovo-legos-hid core
HID: Add lenovo-legos-hid configuration endpoint interface
Mario Limonciello (2):
HID: Include firmware version in the uevent
HID: Allow HID drivers to add more uevent variables
.../ABI/testing/sysfs-driver-lenovo-legos-hid | 269 +++
MAINTAINERS | 7 +
drivers/hid/Kconfig | 2 +
drivers/hid/Makefile | 2 +
drivers/hid/hid-core.c | 11 +
drivers/hid/hid-ids.h | 4 +
drivers/hid/lenovo-legos-hid/Kconfig | 11 +
drivers/hid/lenovo-legos-hid/Makefile | 6 +
drivers/hid/lenovo-legos-hid/config.c | 1518 +++++++++++++++++
drivers/hid/lenovo-legos-hid/config.h | 19 +
drivers/hid/lenovo-legos-hid/core.c | 122 ++
drivers/hid/lenovo-legos-hid/core.h | 25 +
include/linux/hid.h | 2 +
13 files changed, 1998 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid
create mode 100644 drivers/hid/lenovo-legos-hid/Kconfig
create mode 100644 drivers/hid/lenovo-legos-hid/Makefile
create mode 100644 drivers/hid/lenovo-legos-hid/config.c
create mode 100644 drivers/hid/lenovo-legos-hid/config.h
create mode 100644 drivers/hid/lenovo-legos-hid/core.c
create mode 100644 drivers/hid/lenovo-legos-hid/core.h
--
2.50.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/6] HID: Include firmware version in the uevent 2025-07-03 0:49 [PATCH 0/6] HID: Add Legion Go S Driver Derek J. Clark @ 2025-07-03 0:49 ` Derek J. Clark 2025-07-03 0:49 ` [PATCH 2/6] HID: Allow HID drivers to add more uevent variables Derek J. Clark ` (5 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Derek J. Clark @ 2025-07-03 0:49 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires Cc: Mario Limonciello, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, Derek J . Clark, linux-input, linux-doc, linux-kernel, Mario Limonciello, Richard Hughes From: Mario Limonciello <mario.limonciello@amd.com> Userspace software fwupd probes some HID devices when the daemon starts up to determine the current firmware version in order to be able to offer updated firmware if the manufacturer has made it available. In order to do this fwupd will detach the existing kernel driver if one is present, send a HID command and then reattach the kernel driver. This can be problematic if the user is using the HID device at the time that fwupd probes the hardware and can cause a few frames of input to be dropped. In some cases HID drivers already have a command to look up the firmware version, and so if that is exported to userspace fwupd can discover it and avoid needing to detach the kernel driver until it's time to update the device. Introduce a new member in the struct hid_device for the version and export a new uevent variable HID_FIRMWARE_VERSION that will display the version that HID drivers obtained. Cc: Richard Hughes <hughsient@gmail.com> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/hid/hid-core.c | 5 +++++ include/linux/hid.h | 1 + 2 files changed, 6 insertions(+) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 46d552b1d250..1b18e0dadbac 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2839,6 +2839,11 @@ static int hid_uevent(const struct device *dev, struct kobj_uevent_env *env) if (add_uevent_var(env, "MODALIAS=hid:b%04Xg%04Xv%08Xp%08X", hdev->bus, hdev->group, hdev->vendor, hdev->product)) return -ENOMEM; + if (hdev->firmware_version) { + if (add_uevent_var(env, "HID_FIRMWARE_VERSION=0x%04llX", + hdev->firmware_version)) + return -ENOMEM; + } return 0; } diff --git a/include/linux/hid.h b/include/linux/hid.h index 7f260e0e2049..ffc81a8c7a49 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -667,6 +667,7 @@ struct hid_device { char name[128]; /* Device name */ char phys[64]; /* Device physical location */ char uniq[64]; /* Device unique identifier (serial #) */ + u64 firmware_version; /* Firmware version */ void *driver_data; -- 2.50.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/6] HID: Allow HID drivers to add more uevent variables 2025-07-03 0:49 [PATCH 0/6] HID: Add Legion Go S Driver Derek J. Clark 2025-07-03 0:49 ` [PATCH 1/6] HID: Include firmware version in the uevent Derek J. Clark @ 2025-07-03 0:49 ` Derek J. Clark 2025-07-03 0:49 ` [PATCH 3/6] HID: Add Legion Go S ID's Derek J. Clark ` (4 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Derek J. Clark @ 2025-07-03 0:49 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires Cc: Mario Limonciello, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, Derek J . Clark, linux-input, linux-doc, linux-kernel, Mario Limonciello, Richard Hughes From: Mario Limonciello <mario.limonciello@amd.com> Some drivers have static information that can be useful for userspace to have, but maintaining a sysfs file is overkill. Add an optional callback for drivers to be able to add their own uevent variables. Cc: Richard Hughes <hughsient@gmail.com> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/hid/hid-core.c | 6 ++++++ include/linux/hid.h | 1 + 2 files changed, 7 insertions(+) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 1b18e0dadbac..de95470066d9 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2844,6 +2844,12 @@ static int hid_uevent(const struct device *dev, struct kobj_uevent_env *env) hdev->firmware_version)) return -ENOMEM; } + if (hdev->uevent) { + int ret = hdev->uevent(dev, env); + + if (ret) + return ret; + } return 0; } diff --git a/include/linux/hid.h b/include/linux/hid.h index ffc81a8c7a49..36e3c167c7ff 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -680,6 +680,7 @@ struct hid_device { void (*hiddev_hid_event) (struct hid_device *, struct hid_field *field, struct hid_usage *, __s32); void (*hiddev_report_event) (struct hid_device *, struct hid_report *); + int (*uevent)(const struct device *dev, struct kobj_uevent_env *env); /* debugging support via debugfs */ unsigned short debug; -- 2.50.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/6] HID: Add Legion Go S ID's 2025-07-03 0:49 [PATCH 0/6] HID: Add Legion Go S Driver Derek J. Clark 2025-07-03 0:49 ` [PATCH 1/6] HID: Include firmware version in the uevent Derek J. Clark 2025-07-03 0:49 ` [PATCH 2/6] HID: Allow HID drivers to add more uevent variables Derek J. Clark @ 2025-07-03 0:49 ` Derek J. Clark 2025-07-03 0:49 ` [PATCH 4/6] HID: Add documentation for lenovo-legos-hid driver Derek J. Clark ` (3 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Derek J. Clark @ 2025-07-03 0:49 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires Cc: Mario Limonciello, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, Derek J . Clark, linux-input, linux-doc, linux-kernel Adds QHE vendor ID for Legion Go S Controller Adds xinput and dinput mode Product ID for Legion Go S controller Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> --- drivers/hid/hid-ids.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 8cdc789fbf2b..d6c096a20eaf 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -716,6 +716,10 @@ #define USB_DEVICE_ID_ITE8595 0x8595 #define USB_DEVICE_ID_ITE_MEDION_E1239T 0xce50 +#define USB_VENDOR_ID_QHE 0x1a86 +#define USB_DEVICE_ID_LENOVO_LEGION_GO_S_XINPUT 0xe310 +#define USB_DEVICE_ID_LENOVO_LEGION_GO_S_DINPUT 0xe311 + #define USB_VENDOR_ID_JABRA 0x0b0e #define USB_DEVICE_ID_JABRA_SPEAK_410 0x0412 #define USB_DEVICE_ID_JABRA_SPEAK_510 0x0420 -- 2.50.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/6] HID: Add documentation for lenovo-legos-hid driver 2025-07-03 0:49 [PATCH 0/6] HID: Add Legion Go S Driver Derek J. Clark ` (2 preceding siblings ...) 2025-07-03 0:49 ` [PATCH 3/6] HID: Add Legion Go S ID's Derek J. Clark @ 2025-07-03 0:49 ` Derek J. Clark 2025-07-03 0:49 ` [PATCH 5/6] HID: Add lenovo-legos-hid core Derek J. Clark ` (2 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Derek J. Clark @ 2025-07-03 0:49 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires Cc: Mario Limonciello, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, Derek J . Clark, linux-input, linux-doc, linux-kernel Adds ABI documentation for the lenovo-legos-hid driver Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> --- .../ABI/testing/sysfs-driver-lenovo-legos-hid | 270 ++++++++++++++++++ MAINTAINERS | 6 + 2 files changed, 276 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid diff --git a/Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid b/Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid new file mode 100644 index 000000000000..af99df79843d --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid @@ -0,0 +1,270 @@ +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/leds/go_s:rgb:joystick_rings/effect +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls the display effect of the RGB interface. + + Values are monocolor, breathe, chroma, or rainbow. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/leds/go_s:rgb:joystick_rings/effect_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the effect attribute. + + Values are monocolor, breathe, chroma, or rainbow. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/leds/go_s:rgb:joystick_rings/enable +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls enabling or disabling the RGB interface. + + Values are true or false. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/class/leds/go_s:rgb:joystick_rings/enable_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the enable attribute. + + Values are true or false. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/class/leds/go_s:rgb:joystick_rings/mode +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls the operating mode of the RGB interface. + + Values are dynamic or custom. Custom allows setting the RGB effect and color. + Dynamic is a Windows mode for syncing Lenovo RGB interfaces not currently + supported under Linux. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/class/leds/go_s:rgb:joystick_rings/mode_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the mode attribute. + + Values are dynamic or custom. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/class/leds/go_s:rgb:joystick_rings/profile +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls selecting the configured RGB profile. + + Values are 1-3. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/class/leds/go_s:rgb:joystick_rings/profile_range +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the profile attribute. + + Values are 1-3. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/class/leds/go_s:rgb:joystick_rings/speed +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls the change rate for the breathe, chroma, and rainbow effects. + + Values are 0-100. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/class/leds/go_s:rgb:joystick_rings/speed_range +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the speed attribute. + + Values are 0-100. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/gamepad_config/auto_sleep_time +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls the sleep timer due to inactivity for the built-in controller. + + Values are 0-255. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/gamepad_config/auto_sleep_time_range +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the gamepad_config/auto_sleep_time attribute. + + Values are 0-255. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/gamepad_config/dpad_mode +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls the operating mode of the built-in controllers D-pad. + + Values are 4-way or 8-way. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/gamepad_config/dpad_mode_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the gamepad_config/dpad_mode attribute. + + Values are 4-way or 8-way. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/gamepad_config/mode +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls the operating mode of the built-in controller. + + Values are xinput or dinput. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/gamepad_config/mode_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the gamepad_config/mode attribute. + + Values are xinput or dinput. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/gamepad_config/poll_rate +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls the poll rate in Hz of the built-in controller. + + Values are 125, 250, 500, or 1000. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/gamepad_config/poll_rate_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the gamepad_config/poll_rate attribute. + + Values are 125, 250, 500, or 1000. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/imu_config/bypass_enable +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls enabling or disabling the IMU bypass function. When enabled the IMU data is directly reported to the OS through +an HIDRAW interface. + + Values are true or false. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/imu_config/bypass_enable_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the imu_config/bypass_enable attribute. + + Values are true or false. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/imu_config/sensor_enable +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls enabling or disabling the IMU. + + Values are true, false, or wake-2s. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/imu_config/sensor_enable_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the imu_config/sensor_enable attribute. + + Values are true, false, or wake-2s. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/os_mode +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls which value is used for the touchpads operating mode. + + Values are windows or linux. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/os_mode_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the os_mode attribute. + + Values are windows or linux. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/touchpad_config/enable +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls enabling or disabling the built-in touchpad. + + Values are true or false. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/touchpad_config/enable_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the touchpad_config/enable attribute. + + Values are true or false. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/touchpad_config/linux_mode +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls behavior of the touchpad events when os_mode is set to linux. + + Values are absolute or relative. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/touchpad_config/linux_mode_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the touchpad_config/linux_mode attribute. + + Values are absolute or relative. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/touchpad_config/windows_mode +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This controls behavior of the touchpad events when os_mode is set to windows. + + Values are absolute or relative. + + Applies to Lenovo Legion Go S line of handheld devices. + +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/touchpad_config/windows_mode_index +Date: July 2025 +Contact: linux-input@vger.kernel.org +Description: This displays the available options for the touchpad_config/windows_mode attribute. + + Values are absolute or relative. + + Applies to Lenovo Legion Go S line of handheld devices. diff --git a/MAINTAINERS b/MAINTAINERS index 5bdae246605d..68211d6eb236 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13746,6 +13746,12 @@ L: platform-driver-x86@vger.kernel.org S: Maintained F: drivers/platform/x86/lenovo/wmi-hotkey-utilities.c +LENOVO LEGION GO S HID +M: Derek J. Clark <derekjohn.clark@gmail.com> +L: linux-input@vger.kernel.org +S: Maintained +F: Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid + LETSKETCH HID TABLET DRIVER M: Hans de Goede <hansg@kernel.org> L: linux-input@vger.kernel.org -- 2.50.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/6] HID: Add lenovo-legos-hid core 2025-07-03 0:49 [PATCH 0/6] HID: Add Legion Go S Driver Derek J. Clark ` (3 preceding siblings ...) 2025-07-03 0:49 ` [PATCH 4/6] HID: Add documentation for lenovo-legos-hid driver Derek J. Clark @ 2025-07-03 0:49 ` Derek J. Clark 2025-07-03 0:49 ` [PATCH 6/6] HID: Add lenovo-legos-hid configuration endpoint interface Derek J. Clark 2025-07-03 13:48 ` [PATCH 0/6] HID: Add Legion Go S Driver Benjamin Tissoires 6 siblings, 0 replies; 11+ messages in thread From: Derek J. Clark @ 2025-07-03 0:49 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires Cc: Mario Limonciello, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, Derek J . Clark, linux-input, linux-doc, linux-kernel Adds core interface for the lenovo-legos-hid driver. The purpose of core is to identify each available endpoint for the MCU in the Lenovo Legion Go S and route each one to the appropriate initialization and event handling functions. Endpoint specific logic will be implemented in subsequent patches. Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> --- MAINTAINERS | 1 + drivers/hid/Kconfig | 2 + drivers/hid/Makefile | 2 + drivers/hid/lenovo-legos-hid/Kconfig | 11 +++ drivers/hid/lenovo-legos-hid/Makefile | 6 ++ drivers/hid/lenovo-legos-hid/core.c | 113 ++++++++++++++++++++++++++ drivers/hid/lenovo-legos-hid/core.h | 25 ++++++ 7 files changed, 160 insertions(+) create mode 100644 drivers/hid/lenovo-legos-hid/Kconfig create mode 100644 drivers/hid/lenovo-legos-hid/Makefile create mode 100644 drivers/hid/lenovo-legos-hid/core.c create mode 100644 drivers/hid/lenovo-legos-hid/core.h diff --git a/MAINTAINERS b/MAINTAINERS index 68211d6eb236..aa61be9e5bc1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13751,6 +13751,7 @@ M: Derek J. Clark <derekjohn.clark@gmail.com> L: linux-input@vger.kernel.org S: Maintained F: Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid +F: drivers/hid/lenovo-legos-hid/* LETSKETCH HID TABLET DRIVER M: Hans de Goede <hansg@kernel.org> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index a57901203aeb..494e8386b598 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -1436,4 +1436,6 @@ endif # HID source "drivers/hid/usbhid/Kconfig" +source "drivers/hid/lenovo-legos-hid/Kconfig" + endif # HID_SUPPORT diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index 10ae5dedbd84..bdf3ebaf11e5 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -175,3 +175,5 @@ obj-$(CONFIG_AMD_SFH_HID) += amd-sfh-hid/ obj-$(CONFIG_SURFACE_HID_CORE) += surface-hid/ obj-$(CONFIG_INTEL_THC_HID) += intel-thc-hid/ + +obj-$(CONFIG_LENOVO_LEGOS_HID) += lenovo-legos-hid/ diff --git a/drivers/hid/lenovo-legos-hid/Kconfig b/drivers/hid/lenovo-legos-hid/Kconfig new file mode 100644 index 000000000000..6918b25e191c --- /dev/null +++ b/drivers/hid/lenovo-legos-hid/Kconfig @@ -0,0 +1,11 @@ +config LENOVO_LEGOS_HID + tristate "Lenovo Legion Go S HID" + depends on USB_HID + depends on LEDS_CLASS + depends on LEDS_CLASS_MULTICOLOR + help + Say Y here to include support for the Lenovo Legion Go S Handheld + Console Controller. + + To compile this driver as a module, choose M here: the module will + be called lenovo-legos-hid. diff --git a/drivers/hid/lenovo-legos-hid/Makefile b/drivers/hid/lenovo-legos-hid/Makefile new file mode 100644 index 000000000000..707f1be80c78 --- /dev/null +++ b/drivers/hid/lenovo-legos-hid/Makefile @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Makefile - Lenovo Legion Go S Handheld Console Controller driver +# +lenovo-legos-hid-y := core.o +obj-$(CONFIG_LENOVO_LEGOS_HID) := lenovo-legos-hid.o diff --git a/drivers/hid/lenovo-legos-hid/core.c b/drivers/hid/lenovo-legos-hid/core.c new file mode 100644 index 000000000000..9049cbb8bd6c --- /dev/null +++ b/drivers/hid/lenovo-legos-hid/core.c @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * HID driver for Lenovo Legion Go S series gamepad. + * + * Copyright (c) 2025 Derek J. Clark <derekjohn.clark@gmail.com> + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/hid.h> +#include <linux/types.h> +#include <linux/usb.h> + +#include "core.h" +#include "../hid-ids.h" + +u8 get_endpoint_address(struct hid_device *hdev) +{ + struct usb_interface *intf = to_usb_interface(hdev->dev.parent); + struct usb_host_endpoint *ep; + + if (intf) { + ep = intf->cur_altsetting->endpoint; + if (ep) + return ep->desc.bEndpointAddress; + } + + return -ENODEV; +} + +static int lenovo_legos_raw_event(struct hid_device *hdev, + struct hid_report *report, u8 *data, int size) +{ + int ep; + + ep = get_endpoint_address(hdev); + + switch (ep) { + default: + break; + } + return 0; +} + +static int lenovo_legos_hid_probe(struct hid_device *hdev, + const struct hid_device_id *id) +{ + int ret, ep; + + ep = get_endpoint_address(hdev); + if (ep <= 0) + return ep; + + ret = hid_parse(hdev); + if (ret) { + hid_err(hdev, "Parse failed\n"); + return ret; + } + + 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; + } + + switch (ep) { + default: + break; + } + + return ret; +} + +static void lenovo_legos_hid_remove(struct hid_device *hdev) +{ + int ep = get_endpoint_address(hdev); + + switch (ep) { + default: + hid_hw_close(hdev); + hid_hw_stop(hdev); + + break; + } +} + +static const struct hid_device_id lenovo_legos_devices[] = { + { HID_USB_DEVICE(USB_VENDOR_ID_QHE, + USB_DEVICE_ID_LENOVO_LEGION_GO_S_XINPUT) }, + { HID_USB_DEVICE(USB_VENDOR_ID_QHE, + USB_DEVICE_ID_LENOVO_LEGION_GO_S_DINPUT) }, + {} +}; + +MODULE_DEVICE_TABLE(hid, lenovo_legos_devices); +static struct hid_driver lenovo_legos_hid = { + .name = "lenovo-legos-hid", + .id_table = lenovo_legos_devices, + .probe = lenovo_legos_hid_probe, + .remove = lenovo_legos_hid_remove, + .raw_event = lenovo_legos_raw_event, +}; +module_hid_driver(lenovo_legos_hid); + +MODULE_AUTHOR("Derek J. Clark"); +MODULE_DESCRIPTION("HID Driver for Lenovo Legion Go S Series gamepad."); +MODULE_LICENSE("GPL"); diff --git a/drivers/hid/lenovo-legos-hid/core.h b/drivers/hid/lenovo-legos-hid/core.h new file mode 100644 index 000000000000..efbc50896536 --- /dev/null +++ b/drivers/hid/lenovo-legos-hid/core.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +/* Copyright(C) 2025 Derek J. Clark <derekjohn.clark@gmail.com> */ + +#ifndef _LENOVO_LEGOS_HID_CORE_ +#define _LENOVO_LEGOS_HID_CORE_ + +#include <linux/types.h> + +#define GO_S_PACKET_SIZE 64 + +struct hid_device; + +enum legos_interface { + LEGION_GO_S_IAP_INTF_IN = 0x81, + LEGION_GO_S_TP_INTF_IN = 0x83, + LEGION_GO_S_CFG_INTF_IN, + LEGION_GO_S_IMU_INTF_IN, + LEGION_GO_S_GP_INFT_IN, + LEGION_GO_S_UNK_INTF_IN, +}; + +u8 get_endpoint_address(struct hid_device *hdev); + +#endif /* !_LENOVO_LEGOS_HID_CORE_*/ -- 2.50.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/6] HID: Add lenovo-legos-hid configuration endpoint interface 2025-07-03 0:49 [PATCH 0/6] HID: Add Legion Go S Driver Derek J. Clark ` (4 preceding siblings ...) 2025-07-03 0:49 ` [PATCH 5/6] HID: Add lenovo-legos-hid core Derek J. Clark @ 2025-07-03 0:49 ` Derek J. Clark 2025-07-03 13:48 ` [PATCH 0/6] HID: Add Legion Go S Driver Benjamin Tissoires 6 siblings, 0 replies; 11+ messages in thread From: Derek J. Clark @ 2025-07-03 0:49 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires Cc: Mario Limonciello, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, Derek J . Clark, linux-input, linux-doc, linux-kernel, Mario Limonciello Adds the logical interface for the Lenovo Legion Go S configuration endpoint. This endpoint provides settings for the gamepad, trackpad, joystick RGB, and MCU information. The MCU only responds to raw data through hid_raw_output_report. Returns from this interface take ~800usec and multiple requests will lock the MCU, so a mutex is used to block additional calls until it can return. After 5ms the return call is aborted for normal MCU commands. The touchpad is queried through the MCU over I2C and as such takes longer to respond. The response for those calls aborts after 200ms. Co-developed-by: Mario Limonciello <mario.limonciello@amd.com> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> --- drivers/hid/lenovo-legos-hid/Makefile | 2 +- drivers/hid/lenovo-legos-hid/config.c | 1509 +++++++++++++++++++++++++ drivers/hid/lenovo-legos-hid/config.h | 19 + drivers/hid/lenovo-legos-hid/core.c | 9 + 4 files changed, 1538 insertions(+), 1 deletion(-) create mode 100644 drivers/hid/lenovo-legos-hid/config.c create mode 100644 drivers/hid/lenovo-legos-hid/config.h diff --git a/drivers/hid/lenovo-legos-hid/Makefile b/drivers/hid/lenovo-legos-hid/Makefile index 707f1be80c78..ded6158bcbf2 100644 --- a/drivers/hid/lenovo-legos-hid/Makefile +++ b/drivers/hid/lenovo-legos-hid/Makefile @@ -2,5 +2,5 @@ # # Makefile - Lenovo Legion Go S Handheld Console Controller driver # -lenovo-legos-hid-y := core.o +lenovo-legos-hid-y := core.o config.o obj-$(CONFIG_LENOVO_LEGOS_HID) := lenovo-legos-hid.o diff --git a/drivers/hid/lenovo-legos-hid/config.c b/drivers/hid/lenovo-legos-hid/config.c new file mode 100644 index 000000000000..bd148f26c8c7 --- /dev/null +++ b/drivers/hid/lenovo-legos-hid/config.c @@ -0,0 +1,1509 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * HID driver for Lenovo Legion Go S devices. + * + * Copyright (c) 2025 Derek J. Clark <derekjohn.clark@gmail.com> + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#define BOOL_TO_STR(x) x ? "true" : "false" + +#include <linux/array_size.h> +#include <linux/cleanup.h> +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/dev_printk.h> +#include <linux/device.h> +#include <linux/hid.h> +#include <linux/jiffies.h> +#include <linux/kstrtox.h> +#include <linux/led-class-multicolor.h> +#include <linux/mutex.h> +#include <linux/printk.h> +#include <linux/stddef.h> +#include <linux/string.h> +#include <linux/sysfs.h> +#include <linux/types.h> +#include <linux/usb.h> +#include <linux/workqueue.h> +#include <linux/workqueue_types.h> + +#include "core.h" +#include "config.h" + +struct legos_cfg { + struct delayed_work legos_cfg_setup; + struct completion send_cmd_complete; + struct led_classdev *led_cdev; + struct hid_device *hdev; + struct mutex cfg_mutex; /* Avoid Send/Rx MCU locking */ + u8 gp_auto_sleep_time; + u8 gp_dpad_mode; + u8 gp_mode; + u8 gp_poll_rate; + bool imu_bypass_en; + u8 imu_manufacturer; + u8 imu_sensor_en; + u8 mcu_id[12]; + u8 mouse_step; + u8 os_mode; + u8 rgb_effect; + bool rgb_en; + u8 rgb_mode; + u8 rgb_profile; + u8 rgb_speed; + bool tp_en; + u8 tp_linux_mode; + u8 tp_manufacturer; + u8 tp_version; + u8 tp_windows_mode; +} drvdata; + +/* GET/SET_GAMEPAD_CFG */ +enum GAMEPAD_MODE { + XINPUT, + DINPUT, +}; + +static const char *const GAMEPAD_MODE_TEXT[] = { + [XINPUT] = "xinput", + [DINPUT] = "dinput", +}; + +enum IMU_ENABLED { + IMU_OFF, + IMU_ON, + IMU_OFF_2S, +}; + +static const char *const IMU_ENABLED_TEXT[] = { + [IMU_OFF] = "false", + [IMU_ON] = "true", + [IMU_OFF_2S] = "wake-2s", +}; + +enum OS_TYPE { + WINDOWS, + LINUX, +}; + +static const char *const OS_TYPE_TEXT[] = { + [WINDOWS] = "windows", + [LINUX] = "linux", +}; + +enum POLL_RATE { + HZ125, + HZ250, + HZ500, + HZ1000, +}; + +static const char *const POLL_RATE_TEXT[] = { + [HZ125] = "125", + [HZ250] = "250", + [HZ500] = "500", + [HZ1000] = "1000", +}; + +enum DPAD_MODE { + DIR8, + DIR4, +}; + +static const char *const DPAD_MODE_TEXT[] = { + [DIR8] = "8-way", + [DIR4] = "4-way", +}; + +enum GAMEPAD_CFG_INDEX { + NONE = 0x00, + CFG_GAMEPAD_MODE, // GAMEPAD_MODE + CFG_AUTO_SLP_TIME = 0x04, // 1-255 + CFG_PASS_ENABLE, // FEATURE_ENABLED + CFG_LIGHT_ENABLE, // FEATURE_ENABLED + CFG_IMU_ENABLE, // FEATURE_ENABLED + CFG_TP_ENABLE, // FEATURE_ENABLED + CFG_OS_TYPE = 0x0A, // OS_TYPE + CFG_POLL_RATE = 0x10, // POLL_RATE + CFG_DPAD_MODE, // DPAD_MODE + CFG_MS_WHEEL_STEP, // 1-127 +}; + +/* GET/SET_TP_PARAM */ +enum TOUCHPAD_MODE { + TP_REL, + TP_ABS, +}; + +static const char *const TOUCHPAD_MODE_TEXT[] = { + [TP_REL] = "relative", + [TP_ABS] = "absolute", +}; + +enum TOUCHPAD_CFG_INDEX { + CFG_WINDOWS_MODE = 0x03, // TOUCHPAD_MODE + CFG_LINUX_MODE, // TOUCHPAD_MODE + +}; + +enum RGB_MODE { + RGB_MODE_DYNAMIC, + RGB_MODE_CUSTOM, +}; + +static const char *const RGB_MODE_TEXT[] = { + [RGB_MODE_DYNAMIC] = "dynamic", + [RGB_MODE_CUSTOM] = "custom", +}; + +enum RGB_EFFECT { + RGB_EFFECT_MONO, + RGB_EFFECT_BREATHE, + RGB_EFFECT_CHROMA, + RGB_EFFECT_RAINBOW, +}; + +static const char *const RGB_EFFECT_TEXT[] = { + [RGB_EFFECT_MONO] = "monocolor", + [RGB_EFFECT_BREATHE] = "breathe", + [RGB_EFFECT_CHROMA] = "chroma", + [RGB_EFFECT_RAINBOW] = "rainbow", +}; + +/* GET/SET_LIGHT_CFG */ +enum LIGHT_CFG_INDEX { + LIGHT_MODE_SEL = 0x01, + LIGHT_PROFILE_SEL, + USR_LIGHT_PROFILE_1, + USR_LIGHT_PROFILE_2, + USR_LIGHT_PROFILE_3, +}; + +enum MCU_COMMAND { + SEND_HEARTBEAT, + GET_VERSION, + GET_MCU_ID, + GET_GAMEPAD_CFG, + SET_GAMEPAD_CFG, + GET_TP_PARAM, + SET_TP_PARAM, + GET_MOTOR_CFG, + SET_MOTOR_CFG, + GET_TRIGGER_CFG, + SET_TRIGGER_CFG, + GET_STICK_CFG, + SET_STICK_CFG, + GET_GYRO_CFG, + SET_GYRO_CFG, + GET_LIGHT_CFG, + SET_LIGHT_CFG, + GET_KEY_MAP, + SET_KEY_MAP, + INT_EVENT_REPORT = 0xc0, + INT_EVENT_CLEAR, + GET_PL_TEST = 0xdf, + SET_PL_TEST, + START_IAP_UPGRADE, + DBG_CTRL, + PL_TP_TEST, + RESTORE_FACTORY, + IC_RESET, +}; + +/*GET/SET_PL_TEST */ +enum TEST_INDEX { + TEST_EN = 0x01, + TEST_TP_MFR, // TP_MANUFACTURER + TEST_IMU_MFR, // IMU_MANUFACTURER + TEST_TP_VER, // u8 + MOTOR_F0_CALI = 0x10, + READ_MOTOR_F0, + SAVE_MOTOR_F0, + TEST_LED_L = 0x20, + TEST_LED_R, + LED_COLOR_CALI, + STICK_CALI_TH = 0x30, + TRIGGER_CALI_TH, + STICK_CALI_DEAD, + TRIGGER_CALI_DEAD, + STICK_CALI_POLARITY, + TRIGGER_CALI_POLARITY, + GYRO_CALI_CFG, + STICK_CALI_TOUT, + TRIGGER_CALI_TOUT, +}; + +enum TP_MANUFACTURER { + TP_NONE, + TP_BETTERLIFE, + TP_SIPO, +}; + +static const char *const TP_MANUFACTURER_TEXT[] = { + [TP_NONE] = "None", + [TP_BETTERLIFE] = "BetterLife", + [TP_SIPO] = "SIPO", +}; + +enum IMU_MANUFACTURER { + IMU_NONE, + IMU_BOSCH, + IMU_ST, +}; + +static const char *const IMU_MANUFACTURER_TEXT[] = { + [IMU_NONE] = "None", + [IMU_BOSCH] = "Bosch", + [IMU_ST] = "ST", +}; + +struct command_report { + u8 cmd; + u8 sub_cmd; + u8 data[63]; +} __packed; + +struct version_report { + u8 cmd; + u32 version; + u8 reserved[59]; +} __packed; + +struct legos_cfg_rw_attr { + u8 index; +}; + +int legos_cfg_raw_event(u8 *data, int size) +{ + struct led_classdev_mc *mc_cdev; + struct command_report *cmd_rep; + struct version_report *ver_rep; + int ret; + + print_hex_dump_debug("", DUMP_PREFIX_NONE, 16, 1, data, size, false); + + if (size != GO_S_PACKET_SIZE) + return -EINVAL; + + cmd_rep = (struct command_report *)data; + switch (cmd_rep->cmd) { + case GET_VERSION: + ver_rep = (struct version_report *)data; + drvdata.hdev->firmware_version = + __cpu_to_le32(ver_rep->version); + ret = 0; + break; + case GET_MCU_ID: + drvdata.mcu_id[0] = cmd_rep->sub_cmd; + memcpy(&drvdata.mcu_id[1], cmd_rep->data, 11); + ret = 0; + break; + case GET_GAMEPAD_CFG: + switch (cmd_rep->sub_cmd) { + case CFG_GAMEPAD_MODE: + drvdata.gp_mode = cmd_rep->data[0]; + ret = 0; + break; + case CFG_AUTO_SLP_TIME: + drvdata.gp_auto_sleep_time = cmd_rep->data[0]; + ret = 0; + break; + case CFG_PASS_ENABLE: + drvdata.imu_bypass_en = cmd_rep->data[0]; + ret = 0; + break; + case CFG_LIGHT_ENABLE: + drvdata.rgb_en = cmd_rep->data[0]; + ret = 0; + break; + case CFG_IMU_ENABLE: + drvdata.imu_sensor_en = cmd_rep->data[0]; + ret = 0; + break; + case CFG_TP_ENABLE: + drvdata.tp_en = cmd_rep->data[0]; + ret = 0; + break; + case CFG_OS_TYPE: + drvdata.os_mode = cmd_rep->data[0]; + ret = 0; + break; + case CFG_POLL_RATE: + drvdata.gp_poll_rate = cmd_rep->data[0]; + ret = 0; + break; + case CFG_DPAD_MODE: + drvdata.gp_dpad_mode = cmd_rep->data[0]; + ret = 0; + break; + case CFG_MS_WHEEL_STEP: + drvdata.mouse_step = cmd_rep->data[0]; + ret = 0; + break; + default: + ret = -EINVAL; + break; + } + break; + case GET_TP_PARAM: + switch (cmd_rep->sub_cmd) { + case CFG_LINUX_MODE: + drvdata.tp_linux_mode = cmd_rep->data[0]; + ret = 0; + break; + case CFG_WINDOWS_MODE: + drvdata.tp_windows_mode = cmd_rep->data[0]; + ret = 0; + break; + default: + ret = -EINVAL; + break; + } + break; + case GET_PL_TEST: + switch (cmd_rep->sub_cmd) { + case TEST_TP_MFR: + drvdata.tp_manufacturer = cmd_rep->data[0]; + ret = 0; + break; + case TEST_IMU_MFR: + drvdata.imu_manufacturer = cmd_rep->data[0]; + ret = 0; + break; + case TEST_TP_VER: + drvdata.tp_version = cmd_rep->data[0]; + ret = 0; + break; + default: + ret = -EINVAL; + break; + } + break; + case GET_LIGHT_CFG: + switch (cmd_rep->sub_cmd) { + case LIGHT_MODE_SEL: + drvdata.rgb_mode = cmd_rep->data[0]; + ret = 0; + break; + case LIGHT_PROFILE_SEL: + 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: + mc_cdev = lcdev_to_mccdev(drvdata.led_cdev); + 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]; + drvdata.rgb_speed = cmd_rep->data[5]; + ret = 0; + break; + default: + ret = -EINVAL; + break; + } + break; + case GET_GYRO_CFG: + case GET_KEY_MAP: + case GET_MOTOR_CFG: + case GET_STICK_CFG: + case GET_TRIGGER_CFG: + ret = -EINVAL; + break; + case SET_GAMEPAD_CFG: + case SET_GYRO_CFG: + case SET_KEY_MAP: + case SET_LIGHT_CFG: + case SET_MOTOR_CFG: + case SET_STICK_CFG: + case SET_TP_PARAM: + case SET_TRIGGER_CFG: + ret = -cmd_rep->data[0]; + break; + default: + ret = -EINVAL; + break; + }; + + if (ret && cmd_rep->cmd != START_IAP_UPGRADE) + dev_err(&drvdata.hdev->dev, + "Command %u with index %u failed with error code: %x\n", + cmd_rep->cmd, cmd_rep->sub_cmd, ret); + + pr_debug("Last command: %u, sub_cmd: %u, ret: %u, val: [%ph]\n", + cmd_rep->cmd, cmd_rep->sub_cmd, ret, cmd_rep->data); + + complete(&drvdata.send_cmd_complete); + return ret; +} + +static int legos_cfg_send_cmd(struct hid_device *hdev, u8 *buf, int ep) +{ + unsigned char *dmabuf __free(kfree) = NULL; + size_t size = GO_S_PACKET_SIZE; + int ret; + + pr_debug("Send data as raw output report: [%*ph]\n", GO_S_PACKET_SIZE, + buf); + + dmabuf = kmemdup(buf, size, GFP_KERNEL); + if (!dmabuf) + return -ENOMEM; + + ret = hid_hw_output_report(hdev, dmabuf, size); + if (ret < 0) + return ret; + + return ret == size ? 0 : -EINVAL; +} + +static int mcu_property_out(struct hid_device *hdev, enum MCU_COMMAND command, + u8 index, u8 *val, size_t size) +{ + u8 outbuf[GO_S_PACKET_SIZE] = { command, index }; + int ep = get_endpoint_address(hdev); + unsigned int i; + int timeout = 5; + int ret; + + if (ep != LEGION_GO_S_CFG_INTF_IN) + return -ENODEV; + + for (i = 0; i < size; i++) + outbuf[i + 2] = val[i]; + + guard(mutex)(&drvdata.cfg_mutex); + ret = legos_cfg_send_cmd(hdev, outbuf, ep); + if (ret) + return ret; + + /* PL_TEST commands can take longer because they go out to another device */ + if (command == GET_PL_TEST) + timeout = 200; + + ret = wait_for_completion_interruptible_timeout(&drvdata.send_cmd_complete, + msecs_to_jiffies(timeout)); + + if (ret == 0) /* timeout occurred */ + ret = -EBUSY; + if (ret > 0) /* timeout/interrupt didn't occur */ + ret = 0; + + reinit_completion(&drvdata.send_cmd_complete); + return ret; +} + +/* Read-Write Attributes */ +static ssize_t gamepad_property_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count, + enum GAMEPAD_CFG_INDEX index) +{ + size_t size = 1; + u8 val = 0; + bool res; + int ret; + + switch (index) { + case CFG_GAMEPAD_MODE: { + ret = sysfs_match_string(GAMEPAD_MODE_TEXT, buf); + if (ret < 0) + return ret; + val = ret; + drvdata.gp_mode = val; + break; + } + case CFG_AUTO_SLP_TIME: + ret = kstrtou8(buf, 10, &val); + if (ret) + return ret; + + if (val < 0 || val > 255) + return -EINVAL; + drvdata.gp_auto_sleep_time = val; + break; + case CFG_IMU_ENABLE: + ret = sysfs_match_string(IMU_ENABLED_TEXT, buf); + if (ret < 0) + return ret; + val = ret; + drvdata.imu_sensor_en = val; + break; + case CFG_PASS_ENABLE: + ret = kstrtobool(buf, &res); + if (ret < 0) + return ret; + drvdata.imu_bypass_en = res; + break; + case CFG_LIGHT_ENABLE: + ret = kstrtobool(buf, &res); + if (ret < 0) + return ret; + drvdata.rgb_en = res; + break; + case CFG_TP_ENABLE: + ret = kstrtobool(buf, &res); + if (ret < 0) + return ret; + drvdata.tp_en = res; + break; + case CFG_OS_TYPE: + ret = sysfs_match_string(OS_TYPE_TEXT, buf); + if (ret < 0) + return ret; + val = ret; + drvdata.os_mode = val; + break; + case CFG_POLL_RATE: + ret = sysfs_match_string(POLL_RATE_TEXT, buf); + if (ret < 0) + return ret; + val = ret; + drvdata.gp_poll_rate = val; + break; + case CFG_DPAD_MODE: + ret = sysfs_match_string(DPAD_MODE_TEXT, buf); + if (ret < 0) + return ret; + val = ret; + drvdata.gp_dpad_mode = val; + break; + case CFG_MS_WHEEL_STEP: + ret = kstrtou8(buf, 10, &val); + if (ret) + return ret; + if (val < 1 || val > 127) + return -EINVAL; + drvdata.mouse_step = val; + break; + default: + return -EINVAL; + } + + if (!val) + size = 0; + + ret = mcu_property_out(drvdata.hdev, SET_GAMEPAD_CFG, index, &val, + size); + if (ret < 0) + return ret; + + return count; +} + +static ssize_t gamepad_property_show(struct device *dev, + struct device_attribute *attr, char *buf, + enum GAMEPAD_CFG_INDEX index) +{ + size_t count = 0; + char *res; + u8 i; + + count = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, index, 0, 0); + if (count < 0) + return count; + + switch (index) { + case CFG_GAMEPAD_MODE: + i = drvdata.gp_mode; + if (i > DINPUT) + count = -EINVAL; + else + count = sysfs_emit(buf, "%s\n", GAMEPAD_MODE_TEXT[i]); + break; + case CFG_AUTO_SLP_TIME: + count = sysfs_emit(buf, "%u\n", drvdata.gp_auto_sleep_time); + break; + case CFG_IMU_ENABLE: + i = drvdata.imu_sensor_en; + if (i > IMU_OFF_2S) + count = -EINVAL; + else + count = sysfs_emit(buf, "%s\n", IMU_ENABLED_TEXT[i]); + break; + case CFG_PASS_ENABLE: + res = BOOL_TO_STR(drvdata.imu_bypass_en); + count = sysfs_emit(buf, "%s\n", res); + break; + case CFG_LIGHT_ENABLE: + res = BOOL_TO_STR(drvdata.rgb_en); + count = sysfs_emit(buf, "%s\n", res); + break; + case CFG_TP_ENABLE: + res = BOOL_TO_STR(drvdata.tp_en); + count = sysfs_emit(buf, "%s\n", res); + break; + case CFG_OS_TYPE: + i = drvdata.os_mode; + if (i > LINUX) + count = -EINVAL; + else + count = sysfs_emit(buf, "%s\n", OS_TYPE_TEXT[i]); + break; + case CFG_POLL_RATE: + i = drvdata.gp_poll_rate; + if (i > HZ1000) + count = -EINVAL; + else + count = sysfs_emit(buf, "%s\n", POLL_RATE_TEXT[i]); + break; + case CFG_DPAD_MODE: + i = drvdata.gp_dpad_mode; + if (i > DIR4) + count = -EINVAL; + else + count = sysfs_emit(buf, "%s\n", DPAD_MODE_TEXT[i]); + break; + case CFG_MS_WHEEL_STEP: + i = drvdata.mouse_step; + if (i < 1 || i > 127) + count = -EINVAL; + else + count = sysfs_emit(buf, "%u\n", i); + break; + default: + count = -EINVAL; + break; + } + + return count; +} + +static ssize_t gamepad_property_options(struct device *dev, + struct device_attribute *attr, + char *buf, enum GAMEPAD_CFG_INDEX index) +{ + size_t count = 0; + unsigned int i; + + switch (index) { + case CFG_GAMEPAD_MODE: + for (i = 0; i < ARRAY_SIZE(GAMEPAD_MODE_TEXT); i++) + count += sysfs_emit_at(buf, count, "%s ", GAMEPAD_MODE_TEXT[i]); + break; + case CFG_AUTO_SLP_TIME: + return sysfs_emit(buf, "0-255\n"); + case CFG_IMU_ENABLE: + for (i = 0; i < ARRAY_SIZE(IMU_ENABLED_TEXT); i++) + count += sysfs_emit_at(buf, count, "%s ", IMU_ENABLED_TEXT[i]); + break; + case CFG_PASS_ENABLE: + case CFG_LIGHT_ENABLE: + case CFG_TP_ENABLE: + return sysfs_emit(buf, "true false\n"); + case CFG_OS_TYPE: + for (i = 0; i < ARRAY_SIZE(OS_TYPE_TEXT); i++) + count += sysfs_emit_at(buf, count, "%s ", OS_TYPE_TEXT[i]); + break; + case CFG_POLL_RATE: + for (i = 0; i < ARRAY_SIZE(POLL_RATE_TEXT); i++) + count += sysfs_emit_at(buf, count, "%s ", POLL_RATE_TEXT[i]); + break; + case CFG_DPAD_MODE: + for (i = 0; i < ARRAY_SIZE(DPAD_MODE_TEXT); i++) + count += sysfs_emit_at(buf, count, "%s ", DPAD_MODE_TEXT[i]); + break; + case CFG_MS_WHEEL_STEP: + return sysfs_emit(buf, "1-127\n"); + default: + return count; + } + + if (count) + buf[count - 1] = '\n'; + + return count; +} + +static ssize_t touchpad_property_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count, + enum TOUCHPAD_CFG_INDEX index) +{ + size_t size = 1; + u8 val = 0; + int ret; + + switch (index) { + case CFG_WINDOWS_MODE: + ret = sysfs_match_string(TOUCHPAD_MODE_TEXT, buf); + if (ret < 0) + return ret; + val = ret; + drvdata.tp_windows_mode = val; + break; + case CFG_LINUX_MODE: + ret = sysfs_match_string(TOUCHPAD_MODE_TEXT, buf); + if (ret < 0) + return ret; + val = ret; + drvdata.tp_linux_mode = val; + break; + default: + return -EINVAL; + } + if (!val) + size = 0; + + ret = mcu_property_out(drvdata.hdev, SET_TP_PARAM, index, &val, size); + if (ret < 0) + return ret; + + return count; +} + +static ssize_t touchpad_property_show(struct device *dev, + struct device_attribute *attr, char *buf, + enum TOUCHPAD_CFG_INDEX index) +{ + int ret = 0; + u8 i; + + ret = mcu_property_out(drvdata.hdev, GET_TP_PARAM, index, 0, 0); + if (ret < 0) + return ret; + + switch (index) { + case CFG_WINDOWS_MODE: + i = drvdata.tp_windows_mode; + break; + case CFG_LINUX_MODE: + i = drvdata.tp_linux_mode; + break; + default: + return -EINVAL; + } + + if (i > TP_ABS) + return -EINVAL; + + return sysfs_emit(buf, "%s\n", TOUCHPAD_MODE_TEXT[i]); +} + +static ssize_t touchpad_property_options(struct device *dev, + struct device_attribute *attr, + char *buf, + enum TOUCHPAD_CFG_INDEX index) +{ + size_t count = 0; + unsigned int i; + + switch (index) { + case CFG_WINDOWS_MODE: + case CFG_LINUX_MODE: + for (i = 0; i < ARRAY_SIZE(TOUCHPAD_MODE_TEXT); i++) { + count += sysfs_emit_at(buf, count, "%s ", + TOUCHPAD_MODE_TEXT[i]); + } + break; + default: + return count; + } + + if (count) + buf[count - 1] = '\n'; + + return count; +} + +/* RGB LED */ +static int rgb_cfg_call(struct hid_device *hdev, enum MCU_COMMAND cmd, + enum LIGHT_CFG_INDEX index, u8 *val, size_t size) +{ + if (cmd != SET_LIGHT_CFG && cmd != GET_LIGHT_CFG) + return -EINVAL; + + if (index < LIGHT_MODE_SEL || index > USR_LIGHT_PROFILE_3) + return -EINVAL; + + return mcu_property_out(hdev, cmd, index, val, size); +} + +static int rgb_profile_call(enum MCU_COMMAND cmd, u8 *rgb_profile, size_t size) +{ + enum LIGHT_CFG_INDEX index; + + index = drvdata.rgb_profile + 2; + + return rgb_cfg_call(drvdata.hdev, cmd, index, rgb_profile, size); +} + +static int rgb_write_profile(void) +{ + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(drvdata.led_cdev); + + 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, + drvdata.rgb_speed }; + + return rgb_profile_call(SET_LIGHT_CFG, rgb_profile, 6); +} + +static int rgb_attr_show(void) +{ + return rgb_profile_call(GET_LIGHT_CFG, 0, 0); +}; + +static int rgb_attr_store(void) +{ + if (drvdata.rgb_mode != RGB_MODE_CUSTOM) + return -EINVAL; + + return rgb_write_profile(); +} + +static ssize_t rgb_effect_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + int ret; + + ret = rgb_attr_show(); + if (ret) + return ret; + + return sysfs_emit(buf, "%s\n", RGB_EFFECT_TEXT[drvdata.rgb_effect]); +} + +static ssize_t rgb_effect_store(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t count) +{ + int ret; + + ret = sysfs_match_string(RGB_EFFECT_TEXT, buf); + if (ret < 0) + return ret; + + drvdata.rgb_effect = ret; + + ret = rgb_attr_store(); + if (ret) + return ret; + + return count; +}; + +static ssize_t rgb_effect_index_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + size_t count = 0; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(RGB_EFFECT_TEXT); i++) + count += sysfs_emit_at(buf, count, "%s ", RGB_EFFECT_TEXT[i]); + + if (count) + buf[count - 1] = '\n'; + + return count; +} + +static ssize_t rgb_speed_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + int ret; + + ret = rgb_attr_show(); + if (ret) + return ret; + + return sysfs_emit(buf, "%hhu\n", drvdata.rgb_speed); +} + +static ssize_t rgb_speed_store(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t count) +{ + int val = 0; + int ret; + + ret = kstrtoint(buf, 10, &val); + if (ret) + return ret; + + if (val > 100 || val < 0) + return -EINVAL; + + drvdata.rgb_speed = val; + + ret = rgb_attr_store(); + if (ret) + return ret; + + return count; +}; + +static ssize_t rgb_speed_range_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "0-100\n"); +} + +static ssize_t rgb_mode_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + return sysfs_emit(buf, "%s\n", RGB_MODE_TEXT[drvdata.rgb_mode]); +}; + +static ssize_t rgb_mode_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + size_t size = 1; + int ret; + + ret = sysfs_match_string(RGB_MODE_TEXT, buf); + if (ret < 0) + return ret; + + drvdata.rgb_mode = ret; + + if (!drvdata.rgb_mode) + size = 0; + + ret = rgb_cfg_call(drvdata.hdev, SET_LIGHT_CFG, LIGHT_MODE_SEL, + &drvdata.rgb_mode, size); + if (ret < 0) + return ret; + + return count; +}; + +static ssize_t rgb_mode_index_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + size_t count = 0; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(RGB_MODE_TEXT); i++) + count += sysfs_emit_at(buf, count, "%s ", RGB_MODE_TEXT[i]); + + if (count) + buf[count - 1] = '\n'; + + return count; +} + +static ssize_t rgb_profile_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%hhu\n", drvdata.rgb_profile); +}; + +static ssize_t rgb_profile_store(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t count) +{ + size_t size = 1; + int ret; + u8 val; + + ret = kstrtou8(buf, 10, &val); + if (ret < 0) + return ret; + + if (val > 3 || val < 1) + return -EINVAL; + + drvdata.rgb_profile = val; + + if (!drvdata.rgb_profile) + size = 0; + + ret = rgb_cfg_call(drvdata.hdev, SET_LIGHT_CFG, LIGHT_PROFILE_SEL, + &drvdata.rgb_profile, size); + if (ret < 0) + return ret; + + return count; +}; + +static ssize_t rgb_profile_range_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "1-3\n"); +} + +static enum led_brightness legos_rgb_color_get(struct led_classdev *led_cdev) +{ + return led_cdev->brightness; +}; + +static void legos_rgb_color_set(struct led_classdev *led_cdev, + enum led_brightness brightness) +{ + int ret; + + led_cdev->brightness = brightness; + + ret = rgb_attr_store(); + switch (ret) { + case 0: + break; + case -ENODEV: /* during switch to IAP -ENODEV is expected */ + case -ENOSYS: /* during rmmod -ENOSYS is expected */ + dev_dbg(led_cdev->dev, "Failed to write RGB profile: %i\n", + ret); + break; + default: + dev_err(led_cdev->dev, "Failed to write RGB profile: %i\n", + ret); + }; +} + +#define DEVICE_ATTR_RO_NAMED(_name, _attrname) \ + struct device_attribute dev_attr_##_name = { \ + .attr = { .name = _attrname, .mode = 0444 }, \ + .show = _name##_show, \ + } + +#define DEVICE_ATTR_RW_NAMED(_name, _attrname) \ + struct device_attribute dev_attr_##_name = { \ + .attr = { .name = _attrname, .mode = 0644 }, \ + .show = _name##_show, \ + .store = _name##_store, \ + } + +#define ATTR_LEGOS_GAMEPAD_RW(_name, _attrname, _rtype) \ + static ssize_t _name##_store(struct device *dev, \ + struct device_attribute *attr, \ + const char *buf, size_t count) \ + { \ + return gamepad_property_store(dev, attr, buf, count, \ + _name.index); \ + } \ + static ssize_t _name##_show(struct device *dev, \ + struct device_attribute *attr, char *buf) \ + { \ + return gamepad_property_show(dev, attr, buf, _name.index); \ + } \ + static ssize_t _name##_##_rtype##_show( \ + struct device *dev, struct device_attribute *attr, char *buf) \ + { \ + return gamepad_property_options(dev, attr, buf, _name.index); \ + } \ + DEVICE_ATTR_RW_NAMED(_name, _attrname) + +#define ATTR_LEGOS_TOUCHPAD_RW(_name, _attrname, _rtype) \ + static ssize_t _name##_store(struct device *dev, \ + struct device_attribute *attr, \ + const char *buf, size_t count) \ + { \ + return touchpad_property_store(dev, attr, buf, count, \ + _name.index); \ + } \ + static ssize_t _name##_show(struct device *dev, \ + struct device_attribute *attr, char *buf) \ + { \ + return touchpad_property_show(dev, attr, buf, _name.index); \ + } \ + static ssize_t _name##_##_rtype##_show( \ + struct device *dev, struct device_attribute *attr, char *buf) \ + { \ + return touchpad_property_options(dev, attr, buf, _name.index); \ + } \ + DEVICE_ATTR_RW_NAMED(_name, _attrname) + +/* Gamepad */ +struct legos_cfg_rw_attr auto_sleep_time = { CFG_AUTO_SLP_TIME }; +struct legos_cfg_rw_attr dpad_mode = { CFG_DPAD_MODE }; +struct legos_cfg_rw_attr gamepad_mode = { CFG_GAMEPAD_MODE }; +struct legos_cfg_rw_attr gamepad_poll_rate = { CFG_POLL_RATE }; + +ATTR_LEGOS_GAMEPAD_RW(auto_sleep_time, "auto_sleep_time", range); +ATTR_LEGOS_GAMEPAD_RW(dpad_mode, "dpad_mode", index); +ATTR_LEGOS_GAMEPAD_RW(gamepad_mode, "mode", index); +ATTR_LEGOS_GAMEPAD_RW(gamepad_poll_rate, "poll_rate", index); +static DEVICE_ATTR_RO(auto_sleep_time_range); +static DEVICE_ATTR_RO(dpad_mode_index); +static DEVICE_ATTR_RO_NAMED(gamepad_mode_index, "mode_index"); +static DEVICE_ATTR_RO_NAMED(gamepad_poll_rate_index, "poll_rate_index"); + +static struct attribute *legos_gamepad_attrs[] = { + &dev_attr_auto_sleep_time.attr, + &dev_attr_auto_sleep_time_range.attr, + &dev_attr_dpad_mode.attr, + &dev_attr_dpad_mode_index.attr, + &dev_attr_gamepad_mode.attr, + &dev_attr_gamepad_mode_index.attr, + &dev_attr_gamepad_poll_rate.attr, + &dev_attr_gamepad_poll_rate_index.attr, + NULL, +}; + +/* IMU */ +struct legos_cfg_rw_attr imu_bypass_enabled = { CFG_PASS_ENABLE }; +struct legos_cfg_rw_attr imu_manufacturer = { TEST_IMU_MFR }; +struct legos_cfg_rw_attr imu_sensor_enabled = { CFG_IMU_ENABLE }; + +ATTR_LEGOS_GAMEPAD_RW(imu_bypass_enabled, "bypass_enabled", index); +ATTR_LEGOS_GAMEPAD_RW(imu_sensor_enabled, "sensor_enabled", index); +static DEVICE_ATTR_RO_NAMED(imu_bypass_enabled_index, "bypass_enabled_index"); +static DEVICE_ATTR_RO_NAMED(imu_sensor_enabled_index, "sensor_enabled_index"); + +static struct attribute *legos_imu_attrs[] = { + &dev_attr_imu_bypass_enabled.attr, + &dev_attr_imu_bypass_enabled_index.attr, + &dev_attr_imu_sensor_enabled.attr, + &dev_attr_imu_sensor_enabled_index.attr, + NULL, +}; + +/* MCU */ +struct legos_cfg_rw_attr os_mode = { CFG_OS_TYPE }; + +ATTR_LEGOS_GAMEPAD_RW(os_mode, "os_mode", index); +static DEVICE_ATTR_RO(os_mode_index); + +static struct attribute *legos_mcu_attrs[] = { + &dev_attr_os_mode.attr, + &dev_attr_os_mode_index.attr, + NULL, +}; + +/* RGB */ +struct legos_cfg_rw_attr rgb_enabled = { CFG_LIGHT_ENABLE }; + +ATTR_LEGOS_GAMEPAD_RW(rgb_enabled, "enabled", index); +static DEVICE_ATTR_RO_NAMED(rgb_effect_index, "effect_index"); +static DEVICE_ATTR_RO_NAMED(rgb_enabled_index, "enabled_index"); +static DEVICE_ATTR_RO_NAMED(rgb_mode_index, "mode_index"); +static DEVICE_ATTR_RO_NAMED(rgb_profile_range, "profile_range"); +static DEVICE_ATTR_RO_NAMED(rgb_speed_range, "speed_range"); +static DEVICE_ATTR_RW_NAMED(rgb_effect, "effect"); +static DEVICE_ATTR_RW_NAMED(rgb_mode, "mode"); +static DEVICE_ATTR_RW_NAMED(rgb_profile, "profile"); +static DEVICE_ATTR_RW_NAMED(rgb_speed, "speed"); + +static struct attribute *legos_rgb_attrs[] = { + &dev_attr_rgb_effect.attr, + &dev_attr_rgb_effect_index.attr, + &dev_attr_rgb_speed.attr, + &dev_attr_rgb_speed_range.attr, + &dev_attr_rgb_mode.attr, + &dev_attr_rgb_mode_index.attr, + &dev_attr_rgb_profile.attr, + &dev_attr_rgb_profile_range.attr, + &dev_attr_rgb_enabled.attr, + &dev_attr_rgb_enabled_index.attr, + NULL, +}; + +/* Touchpad */ +struct legos_cfg_rw_attr touchpad_enabled = { CFG_TP_ENABLE }; +struct legos_cfg_rw_attr touchpad_linux_mode = { CFG_LINUX_MODE }; +struct legos_cfg_rw_attr touchpad_manufacturer = { TEST_TP_MFR }; +struct legos_cfg_rw_attr touchpad_version = { TEST_TP_VER }; +struct legos_cfg_rw_attr touchpad_windows_mode = { CFG_WINDOWS_MODE }; + +ATTR_LEGOS_GAMEPAD_RW(touchpad_enabled, "enabled", index); +ATTR_LEGOS_TOUCHPAD_RW(touchpad_linux_mode, "linux_mode", index); +ATTR_LEGOS_TOUCHPAD_RW(touchpad_windows_mode, "windows_mode", index); +static DEVICE_ATTR_RO_NAMED(touchpad_enabled_index, "enabled_index"); +static DEVICE_ATTR_RO_NAMED(touchpad_linux_mode_index, "linux_mode_index"); +static DEVICE_ATTR_RO_NAMED(touchpad_windows_mode_index, "windows_mode_index"); + +static struct attribute *legos_touchpad_attrs[] = { + &dev_attr_touchpad_enabled.attr, + &dev_attr_touchpad_enabled_index.attr, + &dev_attr_touchpad_linux_mode.attr, + &dev_attr_touchpad_linux_mode_index.attr, + &dev_attr_touchpad_windows_mode.attr, + &dev_attr_touchpad_windows_mode_index.attr, + NULL, +}; + +static const struct attribute_group gamepad_attr_group = { + .name = "gamepad", + .attrs = legos_gamepad_attrs, +}; + +static const struct attribute_group imu_attr_group = { + .name = "imu", + .attrs = legos_imu_attrs, +}; + +static const struct attribute_group mcu_attr_group = { + .attrs = legos_mcu_attrs, +}; + +static struct attribute_group rgb_attr_group = { + .attrs = legos_rgb_attrs, +}; + +static const struct attribute_group touchpad_attr_group = { + .name = "touchpad", + .attrs = legos_touchpad_attrs, +}; + +static const struct attribute_group *legos_top_level_attr_groups[] = { + &gamepad_attr_group, + &imu_attr_group, + &mcu_attr_group, + &touchpad_attr_group, + NULL, +}; + +struct mc_subled legos_rgb_subled_info[] = { + { + .color_index = LED_COLOR_ID_RED, + .brightness = 0x50, + .intensity = 0x24, + .channel = 0x1, + }, + { + .color_index = LED_COLOR_ID_GREEN, + .brightness = 0x50, + .intensity = 0x22, + .channel = 0x2, + }, + { + .color_index = LED_COLOR_ID_BLUE, + .brightness = 0x50, + .intensity = 0x99, + .channel = 0x3, + }, +}; + +struct led_classdev_mc legos_cdev_rgb = { + .led_cdev = { + .name = "go_s:rgb:joystick_rings", + .brightness = 0x50, + .max_brightness = 0x64, + .brightness_set = legos_rgb_color_set, + .brightness_get = legos_rgb_color_get, + }, + .num_colors = ARRAY_SIZE(legos_rgb_subled_info), + .subled_info = legos_rgb_subled_info, +}; + +void cfg_setup(struct work_struct *work) +{ + int ret; + + /* Gamepad */ + ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, CFG_AUTO_SLP_TIME, + 0, 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve gamepad auto sleep time: %i\n", + ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, CFG_DPAD_MODE, 0, + 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve gamepad dpad mode: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, CFG_GAMEPAD_MODE, + 0, 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve gamepad mode: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, CFG_POLL_RATE, 0, + 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve gamepad poll rate: %i\n", ret); + return; + } + + /* IMU */ + ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, CFG_PASS_ENABLE, + 0, 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve IMU bypass enabled: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_PL_TEST, TEST_IMU_MFR, 0, 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve IMU Manufacturer: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, CFG_IMU_ENABLE, 0, + 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve IMU enabled: %i\n", ret); + return; + } + + /* MCU */ + ret = mcu_property_out(drvdata.hdev, GET_MCU_ID, NONE, 0, 0); + if (ret) { + dev_err(&drvdata.hdev->dev, "Failed to retrieve MCU ID: %i\n", + ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, CFG_OS_TYPE, 0, + 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve MCU OS Mode: %i\n", ret); + return; + } + + /* RGB */ + ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, CFG_LIGHT_ENABLE, + 0, 0); + if (ret < 0) { + dev_err(drvdata.led_cdev->dev, + "Failed to retrieve RGB enabled: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_LIGHT_CFG, LIGHT_MODE_SEL, 0, + 0); + if (ret < 0) { + dev_err(drvdata.led_cdev->dev, + "Failed to retrieve RGB Mode: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_LIGHT_CFG, LIGHT_PROFILE_SEL, + 0, 0); + if (ret < 0) { + dev_err(drvdata.led_cdev->dev, + "Failed to retrieve RGB Profile: %i\n", ret); + return; + } + + ret = rgb_attr_show(); + if (ret < 0) { + dev_err(drvdata.led_cdev->dev, + "Failed to retrieve RGB Profile Data: %i\n", ret); + return; + } + + /* Touchpad */ + ret = mcu_property_out(drvdata.hdev, GET_GAMEPAD_CFG, CFG_TP_ENABLE, 0, + 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve touchpad enabled: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_TP_PARAM, CFG_LINUX_MODE, 0, + 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve touchpad Linux mode: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_PL_TEST, TEST_TP_MFR, 0, 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve touchpad manufacturer: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_TP_PARAM, CFG_WINDOWS_MODE, 0, + 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve touchpad Windows mode: %i\n", ret); + return; + } + + ret = mcu_property_out(drvdata.hdev, GET_PL_TEST, TEST_TP_VER, 0, 0); + if (ret) { + dev_err(&drvdata.hdev->dev, + "Failed to retrieve touchpad Version: %i\n", ret); + return; + } +} + +static int legos_cfg_uevent(const struct device *dev, + struct kobj_uevent_env *env) +{ + if (add_uevent_var(env, "LEGOS_TP_MANUFACTURER=%s", + TP_MANUFACTURER_TEXT[drvdata.tp_manufacturer])) + return -ENOMEM; + if (add_uevent_var(env, "LEGOS_TP_VERSION=%u", drvdata.tp_version)) + return -ENOMEM; + if (add_uevent_var(env, "LEGOS_IMU_MANUFACTURER=%s", + IMU_MANUFACTURER_TEXT[drvdata.imu_manufacturer])) + return -ENOMEM; + if (add_uevent_var(env, "LEGOS_MCU_ID=%*phN", 12, &drvdata.mcu_id)) + return -ENOMEM; + return 0; +} + +int legos_cfg_probe(struct hid_device *hdev, const struct hid_device_id *_id) +{ + int ret; + + mutex_init(&drvdata.cfg_mutex); + + hid_set_drvdata(hdev, &drvdata); + + drvdata.hdev = hdev; + hdev->uevent = legos_cfg_uevent; + + ret = sysfs_create_groups(&hdev->dev.kobj, legos_top_level_attr_groups); + if (ret) { + dev_err(&hdev->dev, + "Failed to create gamepad configuration attributes: %i\n", + ret); + return ret; + } + + ret = devm_led_classdev_multicolor_register(&hdev->dev, + &legos_cdev_rgb); + if (ret) { + dev_err(&hdev->dev, "Failed to create RGB device: %i\n", ret); + return ret; + } + + ret = devm_device_add_group(legos_cdev_rgb.led_cdev.dev, + &rgb_attr_group); + if (ret) { + dev_err(&hdev->dev, + "Failed to create RGB configuratiion attributes: %i\n", + ret); + return ret; + } + + drvdata.led_cdev = &legos_cdev_rgb.led_cdev; + drvdata.led_cdev->color = LED_COLOR_ID_RGB; + + 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. + */ + INIT_DELAYED_WORK(&drvdata.legos_cfg_setup, &cfg_setup); + schedule_delayed_work(&drvdata.legos_cfg_setup, msecs_to_jiffies(2)); + + return 0; +} + +void legos_cfg_remove(struct hid_device *hdev) +{ + guard(mutex)(&drvdata.cfg_mutex); + cancel_delayed_work_sync(&drvdata.legos_cfg_setup); + sysfs_remove_groups(&hdev->dev.kobj, legos_top_level_attr_groups); + hid_hw_close(hdev); + hid_hw_stop(hdev); + hdev->uevent = NULL; + hid_set_drvdata(hdev, NULL); +} diff --git a/drivers/hid/lenovo-legos-hid/config.h b/drivers/hid/lenovo-legos-hid/config.h new file mode 100644 index 000000000000..3d13744e2692 --- /dev/null +++ b/drivers/hid/lenovo-legos-hid/config.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +/* Copyright(C) 2025 Derek J. Clark <derekjohn.clark@gmail.com> */ + +#ifndef _LENOVO_LEGOS_HID_CONFIG_ +#define _LENOVO_LEGOS_HID_CONFIG_ + +#include <linux/types.h> + +struct hid_device; +struct hid_device_id; +struct work_struct; + +int legos_cfg_raw_event(u8 *data, int size); +void cfg_setup(struct work_struct *work); +int legos_cfg_probe(struct hid_device *hdev, const struct hid_device_id *_id); +void legos_cfg_remove(struct hid_device *hdev); + +#endif /* !_LENOVO_LEGOS_HID_CONFIG_*/ diff --git a/drivers/hid/lenovo-legos-hid/core.c b/drivers/hid/lenovo-legos-hid/core.c index 9049cbb8bd6c..1a5d5396ea6d 100644 --- a/drivers/hid/lenovo-legos-hid/core.c +++ b/drivers/hid/lenovo-legos-hid/core.c @@ -11,6 +11,7 @@ #include <linux/usb.h> #include "core.h" +#include "config.h" #include "../hid-ids.h" u8 get_endpoint_address(struct hid_device *hdev) @@ -35,6 +36,8 @@ static int lenovo_legos_raw_event(struct hid_device *hdev, ep = get_endpoint_address(hdev); switch (ep) { + case LEGION_GO_S_CFG_INTF_IN: + return legos_cfg_raw_event(data, size); default: break; } @@ -70,6 +73,9 @@ static int lenovo_legos_hid_probe(struct hid_device *hdev, } switch (ep) { + case LEGION_GO_S_CFG_INTF_IN: + ret = legos_cfg_probe(hdev, id); + break; default: break; } @@ -82,6 +88,9 @@ static void lenovo_legos_hid_remove(struct hid_device *hdev) int ep = get_endpoint_address(hdev); switch (ep) { + case LEGION_GO_S_CFG_INTF_IN: + legos_cfg_remove(hdev); + break; default: hid_hw_close(hdev); hid_hw_stop(hdev); -- 2.50.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] HID: Add Legion Go S Driver 2025-07-03 0:49 [PATCH 0/6] HID: Add Legion Go S Driver Derek J. Clark ` (5 preceding siblings ...) 2025-07-03 0:49 ` [PATCH 6/6] HID: Add lenovo-legos-hid configuration endpoint interface Derek J. Clark @ 2025-07-03 13:48 ` Benjamin Tissoires 2025-07-04 1:57 ` Mario Limonciello 2025-07-10 15:15 ` Derek J. Clark 6 siblings, 2 replies; 11+ messages in thread From: Benjamin Tissoires @ 2025-07-03 13:48 UTC (permalink / raw) To: Derek J. Clark Cc: Jiri Kosina, Mario Limonciello, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, linux-input, linux-doc, linux-kernel Hi Derek, [I'll answer to this email with a very high level overview of it, as I'm not sure I'll have time to dig much deeper in 6/6 today.] On Jul 02 2025, Derek J. Clark wrote: > This series adds initial support for the Legion Go S's built-in > controller HID configuration interface. In the first patch a new HID > uevent property is added, HID_FIRMWARE_VERSION, so as to permit fwupd > to read the firmware version of the HID interface without detaching the > kernel driver. That immediately raise red flags on my side. HID_FIRMWARE_VERSION will likely be used only for this new driver, and that means a special case in each and every client. We had to deal with firmware versions in the past in the HID drivers, and we ended up relying on the uniq field of the hid_device (because the serial+firmware version uniquely identify the device). > The second patch adds the ability for an hid_driver to > assign new/arbitrary uevent properties for static data that doesn't > benefit from having a sysfs entry. That, in my mind, is even worse (for the reasons above). > The third patch adds the VID and PID > for the Lenovo Legion Go S MCU. Which shouldn't be in its own patch, but part of the driver initial patch. > The fourth patch adds ABI documentation > for the config interface introduced in the final patch. The fifth patch > introduces the core lenovo-legos-hid driver which acts as a routing > interface for the different endpoints. That "core" patch is IMO useless. All it does is: - check for the USB endpoint (but in the wrong way, because if you insert a device through uhid with the same PID/VID it will crash) - replace the HID-core core functions with the same code Really, this should be squashed into the next patch (with 3/6 then). Also, why adding a new subdirectory? All the hid drivers are flat in the drivers/hid/ directory, and the subdirs are for transport layers. There is one exception for the surface driver but I don't see why you need such an exception (yeah, the code is big, but what's the difference in having a 1500 lines of code source in its own subdir vs at the root?) > The sixth path introduces the > config lenovo-legos-hid driver wich uses both the HID_FIRMWARE_VERSION > as well as arbitrary uevent properties. Additional interfaces and config > properties are planned to be added in a future series. That one is too big for my liking. Generally speaking, a commit descrition which says "this does this and that" can be split into 2 patches at least :) What kind of future interfaces and config properties are you planning? > > Patch 6 introduces a checkpatch WARNING that I'm unable to resolve: > WARNING: ENOSYS means 'invalid syscall nr' and nothing else > 1292: FILE: drivers/hid/lenovo-legos-hid/lenovo-legos-hid-config.c:1085: > + case -ENOSYS: /* during rmmod -ENOSYS is expected */ We can losely waive those while merging. We do it quite often actually. But trying to minimize checkpatch warnings is a good thing, so thanks for that. > > This error handling case was added as it is experienced in the real world > when the driver is rmmod. The LED subsystem produces this error code in > its legacy code and this is not a new novel use of -ENOSYS, we are simply > catching the case to avoid spurious errors in dmesg when the driver is > removed. If there is a way to prevent this error from being triggered by > checkpatch in the first place, that would be an ideal remedy, but I'm not > aware how that can be done at this time. Again, nothing to worry about. Cheers, Benjamin > > Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> > > > Derek J. Clark (4): > HID: Add Legion Go S ID's > HID: Add documentation for lenovo-legos-hid driver > HID: Add lenovo-legos-hid core > HID: Add lenovo-legos-hid configuration endpoint interface > > Mario Limonciello (2): > HID: Include firmware version in the uevent > HID: Allow HID drivers to add more uevent variables > > .../ABI/testing/sysfs-driver-lenovo-legos-hid | 269 +++ > MAINTAINERS | 7 + > drivers/hid/Kconfig | 2 + > drivers/hid/Makefile | 2 + > drivers/hid/hid-core.c | 11 + > drivers/hid/hid-ids.h | 4 + > drivers/hid/lenovo-legos-hid/Kconfig | 11 + > drivers/hid/lenovo-legos-hid/Makefile | 6 + > drivers/hid/lenovo-legos-hid/config.c | 1518 +++++++++++++++++ > drivers/hid/lenovo-legos-hid/config.h | 19 + > drivers/hid/lenovo-legos-hid/core.c | 122 ++ > drivers/hid/lenovo-legos-hid/core.h | 25 + > include/linux/hid.h | 2 + > 13 files changed, 1998 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid > create mode 100644 drivers/hid/lenovo-legos-hid/Kconfig > create mode 100644 drivers/hid/lenovo-legos-hid/Makefile > create mode 100644 drivers/hid/lenovo-legos-hid/config.c > create mode 100644 drivers/hid/lenovo-legos-hid/config.h > create mode 100644 drivers/hid/lenovo-legos-hid/core.c > create mode 100644 drivers/hid/lenovo-legos-hid/core.h > > -- > 2.50.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] HID: Add Legion Go S Driver 2025-07-03 13:48 ` [PATCH 0/6] HID: Add Legion Go S Driver Benjamin Tissoires @ 2025-07-04 1:57 ` Mario Limonciello 2025-07-04 16:08 ` Richard Hughes 2025-07-10 15:15 ` Derek J. Clark 1 sibling, 1 reply; 11+ messages in thread From: Mario Limonciello @ 2025-07-04 1:57 UTC (permalink / raw) To: Benjamin Tissoires, Derek J. Clark, Richard Hughes Cc: Jiri Kosina, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, linux-input, linux-doc, linux-kernel On 7/3/25 09:48, Benjamin Tissoires wrote: > Hi Derek, > > [I'll answer to this email with a very high level overview of it, as I'm > not sure I'll have time to dig much deeper in 6/6 today.] I'll touch on my two patches at the front of the series and let Derek get to the questions/comments on the later ones. > > On Jul 02 2025, Derek J. Clark wrote: >> This series adds initial support for the Legion Go S's built-in >> controller HID configuration interface. In the first patch a new HID >> uevent property is added, HID_FIRMWARE_VERSION, so as to permit fwupd >> to read the firmware version of the HID interface without detaching the >> kernel driver. > > That immediately raise red flags on my side. HID_FIRMWARE_VERSION will > likely be used only for this new driver, and that means a special case > in each and every client. Actually Richard and I had envisioned that all updatable HID devices would start exporting their firmware version through this HID property. lenovo-legos-hid was just the first. The idea would then be that userspace software like fwupd would know to parse this property to show the current version and never need to interrogate the device directly unless it was actually being updated. > > We had to deal with firmware versions in the past in the HID drivers, > and we ended up relying on the uniq field of the hid_device (because the > serial+firmware version uniquely identify the device). I think this is a different case. We don't care so much about the unique identification of the device as much as we care about the stream of firmware applied to the device. If HID_UNIQ is the right way to get the firmware version but some drivers encode a serial+firmware and others encode firmware that's going to make for a very messy "generic" property to read the firmware version of a device. > >> The second patch adds the ability for an hid_driver to >> assign new/arbitrary uevent properties for static data that doesn't >> benefit from having a sysfs entry. > > That, in my mind, is even worse (for the reasons above). Do clients actually need to know about all the properties? My thought was that if a client encounters a property it doesn't care about it can just ignore it. If that's misplaced; what would you prefer for all this static information? A pile of sysfs files? > >> The third patch adds the VID and PID >> for the Lenovo Legion Go S MCU. > > Which shouldn't be in its own patch, but part of the driver initial > patch. > >> The fourth patch adds ABI documentation >> for the config interface introduced in the final patch. The fifth patch >> introduces the core lenovo-legos-hid driver which acts as a routing >> interface for the different endpoints. > > That "core" patch is IMO useless. All it does is: > - check for the USB endpoint (but in the wrong way, because if you > insert a device through uhid with the same PID/VID it will crash) > - replace the HID-core core functions with the same code > > Really, this should be squashed into the next patch (with 3/6 then). > > Also, why adding a new subdirectory? All the hid drivers are flat in the > drivers/hid/ directory, and the subdirs are for transport layers. There > is one exception for the surface driver but I don't see why you need > such an exception (yeah, the code is big, but what's the difference in > having a 1500 lines of code source in its own subdir vs at the root?) > >> The sixth path introduces the >> config lenovo-legos-hid driver wich uses both the HID_FIRMWARE_VERSION >> as well as arbitrary uevent properties. Additional interfaces and config >> properties are planned to be added in a future series. > > That one is too big for my liking. Generally speaking, a commit > descrition which says "this does this and that" can be split into 2 > patches at least :) > > What kind of future interfaces and config properties are you planning? > >> >> Patch 6 introduces a checkpatch WARNING that I'm unable to resolve: >> WARNING: ENOSYS means 'invalid syscall nr' and nothing else >> 1292: FILE: drivers/hid/lenovo-legos-hid/lenovo-legos-hid-config.c:1085: >> + case -ENOSYS: /* during rmmod -ENOSYS is expected */ > > We can losely waive those while merging. We do it quite often actually. > > But trying to minimize checkpatch warnings is a good thing, so thanks > for that. > >> >> This error handling case was added as it is experienced in the real world >> when the driver is rmmod. The LED subsystem produces this error code in >> its legacy code and this is not a new novel use of -ENOSYS, we are simply >> catching the case to avoid spurious errors in dmesg when the driver is >> removed. If there is a way to prevent this error from being triggered by >> checkpatch in the first place, that would be an ideal remedy, but I'm not >> aware how that can be done at this time. > > Again, nothing to worry about. > > Cheers, > Benjamin > >> >> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> >> >> >> Derek J. Clark (4): >> HID: Add Legion Go S ID's >> HID: Add documentation for lenovo-legos-hid driver >> HID: Add lenovo-legos-hid core >> HID: Add lenovo-legos-hid configuration endpoint interface >> >> Mario Limonciello (2): >> HID: Include firmware version in the uevent >> HID: Allow HID drivers to add more uevent variables >> >> .../ABI/testing/sysfs-driver-lenovo-legos-hid | 269 +++ >> MAINTAINERS | 7 + >> drivers/hid/Kconfig | 2 + >> drivers/hid/Makefile | 2 + >> drivers/hid/hid-core.c | 11 + >> drivers/hid/hid-ids.h | 4 + >> drivers/hid/lenovo-legos-hid/Kconfig | 11 + >> drivers/hid/lenovo-legos-hid/Makefile | 6 + >> drivers/hid/lenovo-legos-hid/config.c | 1518 +++++++++++++++++ >> drivers/hid/lenovo-legos-hid/config.h | 19 + >> drivers/hid/lenovo-legos-hid/core.c | 122 ++ >> drivers/hid/lenovo-legos-hid/core.h | 25 + >> include/linux/hid.h | 2 + >> 13 files changed, 1998 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid >> create mode 100644 drivers/hid/lenovo-legos-hid/Kconfig >> create mode 100644 drivers/hid/lenovo-legos-hid/Makefile >> create mode 100644 drivers/hid/lenovo-legos-hid/config.c >> create mode 100644 drivers/hid/lenovo-legos-hid/config.h >> create mode 100644 drivers/hid/lenovo-legos-hid/core.c >> create mode 100644 drivers/hid/lenovo-legos-hid/core.h >> >> -- >> 2.50.0 >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] HID: Add Legion Go S Driver 2025-07-04 1:57 ` Mario Limonciello @ 2025-07-04 16:08 ` Richard Hughes 0 siblings, 0 replies; 11+ messages in thread From: Richard Hughes @ 2025-07-04 16:08 UTC (permalink / raw) To: Mario Limonciello Cc: Benjamin Tissoires, Derek J. Clark, Richard Hughes, Jiri Kosina, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, linux-input, linux-doc, linux-kernel On Friday, July 4th, 2025 at 2:58 AM, Mario Limonciello <superm1@kernel.org> wrote: > If HID_UNIQ is the right way to get the firmware version but some > drivers encode a serial+firmware and others encode firmware that's going > to make for a very messy "generic" property to read the firmware version > of a device. I think I've also changed my mind on HID_UNIQ -- IIUC the whole point of HID_UNIQ is to be unique comparing devices with the same VID&PID. If I plug in two identical devices with the same firmware version then HID_UNIQ would be the same -- so I think HID_UNIQ should probably always be the serial number. I think HID_UNIQ should also stay the same during the firmware update too, so we don't want to jam two things into one property like "HID_UNIQ=serial:1234,fwver=12.34" Certainly exposing the firmware version as a HID property makes enumerating devices much easier in fwupd; the kernel driver often just knows the firmware version already and for userspace to re-query the firmware version using a per-device custom protocol seems pointless at best and dangerous at worse -- given we're typically doing a SetReport and GetReport with no sequence number available. I don't have much to comment on wrt the implementation, but providing a way for the hidraw kernel driver to export the current firmware version up to userspace makes a lot of sense to me. Richard ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] HID: Add Legion Go S Driver 2025-07-03 13:48 ` [PATCH 0/6] HID: Add Legion Go S Driver Benjamin Tissoires 2025-07-04 1:57 ` Mario Limonciello @ 2025-07-10 15:15 ` Derek J. Clark 1 sibling, 0 replies; 11+ messages in thread From: Derek J. Clark @ 2025-07-10 15:15 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jiri Kosina, Mario Limonciello, Xino Ni, Zhixin Zhang, Mia Shao, Mark Pearson, Pierre-Loup A . Griffais, linux-input, linux-doc, linux-kernel On July 3, 2025 6:48:00 AM PDT, Benjamin Tissoires <bentiss@kernel.org> wrote: >Hi Derek, > >[I'll answer to this email with a very high level overview of it, as I'm >not sure I'll have time to dig much deeper in 6/6 today.] > >On Jul 02 2025, Derek J. Clark wrote: >> This series adds initial support for the Legion Go S's built-in >> controller HID configuration interface. In the first patch a new HID >> uevent property is added, HID_FIRMWARE_VERSION, so as to permit fwupd >> to read the firmware version of the HID interface without detaching the >> kernel driver. > >That immediately raise red flags on my side. HID_FIRMWARE_VERSION will >likely be used only for this new driver, and that means a special case >in each and every client. > >We had to deal with firmware versions in the past in the HID drivers, >and we ended up relying on the uniq field of the hid_device (because the >serial+firmware version uniquely identify the device). > >> The second patch adds the ability for an hid_driver to >> assign new/arbitrary uevent properties for static data that doesn't >> benefit from having a sysfs entry. > >That, in my mind, is even worse (for the reasons above). > Hi Benjamin, Sorry abtthe late reply. Travel & holidays have me a bit behind. I'll let Mario and Richard hash out the specifics on these points. I'll just add a bit of context to why they're in this series. Prior to this, the fwupd plugin would disconnect this driver to query this information to see if there was an available update. As this can be triggered by a system daemon during gameplay that's not a reasonable expectation. Originally we had these as sysfs entries, and returning to them, would be simple enough, but we felt like this is a fairly standard piece of information that should be available. I wasn't aware of the uniq property being used for this historically, but from an outside looking in perspective this seems a bit convoluted. Apart from just being unintuitive if you're not familiar, userspace is going to need bespoke ways to interpret this anyway as serial numbers and firmware formatting are not consistent between manufacturers. I'll wait to adjust this until a more thorough discussion has taken place. >> The third patch adds the VID and PID >> for the Lenovo Legion Go S MCU. > >Which shouldn't be in its own patch, but part of the driver initial >patch. I can do that. My reasoning was simply that if another patch becomes reliant on the VID and you needed to revert the other patches for any reason there would be a conflict. >> The fourth patch adds ABI documentation >> for the config interface introduced in the final patch. The fifth patch >> introduces the core lenovo-legos-hid driver which acts as a routing >> interface for the different endpoints. > >That "core" patch is IMO useless. All it does is: >- check for the USB endpoint (but in the wrong way, because if you > insert a device through uhid with the same PID/VID it will crash) >- replace the HID-core core functions with the same code Can you point me to a better way? This series only implements the config endpoint. ATM the gamepad, touchpad, and IMU are combined into a single Steam Deck like interface in root level userspace as a uhid device. I do have some plans for how to do this in the kernel instead, but the proposal isn't ready yet so I need to keep the hidraw devices available to userspace that aren't implemented yet for backwards compatibility. >Really, this should be squashed into the next patch (with 3/6 then). > >Also, why adding a new subdirectory? All the hid drivers are flat in the >drivers/hid/ directory, and the subdirs are for transport layers. There >is one exception for the surface driver but I don't see why you need >such an exception (yeah, the code is big, but what's the difference in >having a 1500 lines of code source in its own subdir vs at the root?) Sure, I can change it to a single file if that's preferable in this subsystem. This is my first foray into kernel HID drivers so I'm not super familiar with the style preferences yet. Breaking everything up by logical subset made sense to me but I'm not married to it. There >> The sixth path introduces the >> config lenovo-legos-hid driver wich uses both the HID_FIRMWARE_VERSION >> as well as arbitrary uevent properties. Additional interfaces and config >> properties are planned to be added in a future series. > >That one is too big for my liking. Generally speaking, a commit >descrition which says "this does this and that" can be split into 2 >patches at least :) I figured, but I wasn't sure how you'd prefer I break it up. I was thinking that one patch per attribute group would make sense but wanted some feedback before I did that to avoid going down the wrong path with them. >What kind of future interfaces and config properties are you planning? The MCU has a gamepad interface that is more complete than what the xpad driver uses, which includes some back paddles as well as native gyro data which is passed through from the IMU. There's also a separate IMU endpoint so there are a couple options how this can be used. My thoughts are that a sysfs attribute could toggle if gyro is added to one of the joysticks using the IMU data included in the gamepad report, and the external one could be exposed as a motion sensor with the same uniq. Then userspace can determine how to use it. The touchpad works in both abs and rel modes with the default kernel implementations but additional functionality can be gained through Steam Input if this is integrated into the gamepad. As far as additional attributes for the config interface, there is hardware level button remapping and calibration for all axes that need to be added but weren't considered critical for initial support from Lenovo. >> >> Patch 6 introduces a checkpatch WARNING that I'm unable to resolve: >> WARNING: ENOSYS means 'invalid syscall nr' and nothing else >> 1292: FILE: drivers/hid/lenovo-legos-hid/lenovo-legos-hid-config.c:1085: >> + case -ENOSYS: /* during rmmod -ENOSYS is expected */ > >We can losely waive those while merging. We do it quite often actually. > >But trying to minimize checkpatch warnings is a good thing, so thanks >for that. Cool, I'll keep a brief note for posterity. Thanks, Derek >> >> This error handling case was added as it is experienced in the real world >> when the driver is rmmod. The LED subsystem produces this error code in >> its legacy code and this is not a new novel use of -ENOSYS, we are simply >> catching the case to avoid spurious errors in dmesg when the driver is >> removed. If there is a way to prevent this error from being triggered by >> checkpatch in the first place, that would be an ideal remedy, but I'm not >> aware how that can be done at this time. > >Again, nothing to worry about. > >Cheers, >Benjamin > >> >> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com> >> >> >> Derek J. Clark (4): >> HID: Add Legion Go S ID's >> HID: Add documentation for lenovo-legos-hid driver >> HID: Add lenovo-legos-hid core >> HID: Add lenovo-legos-hid configuration endpoint interface >> >> Mario Limonciello (2): >> HID: Include firmware version in the uevent >> HID: Allow HID drivers to add more uevent variables >> >> .../ABI/testing/sysfs-driver-lenovo-legos-hid | 269 +++ >> MAINTAINERS | 7 + >> drivers/hid/Kconfig | 2 + >> drivers/hid/Makefile | 2 + >> drivers/hid/hid-core.c | 11 + >> drivers/hid/hid-ids.h | 4 + >> drivers/hid/lenovo-legos-hid/Kconfig | 11 + >> drivers/hid/lenovo-legos-hid/Makefile | 6 + >> drivers/hid/lenovo-legos-hid/config.c | 1518 +++++++++++++++++ >> drivers/hid/lenovo-legos-hid/config.h | 19 + >> drivers/hid/lenovo-legos-hid/core.c | 122 ++ >> drivers/hid/lenovo-legos-hid/core.h | 25 + >> include/linux/hid.h | 2 + >> 13 files changed, 1998 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-driver-lenovo-legos-hid >> create mode 100644 drivers/hid/lenovo-legos-hid/Kconfig >> create mode 100644 drivers/hid/lenovo-legos-hid/Makefile >> create mode 100644 drivers/hid/lenovo-legos-hid/config.c >> create mode 100644 drivers/hid/lenovo-legos-hid/config.h >> create mode 100644 drivers/hid/lenovo-legos-hid/core.c >> create mode 100644 drivers/hid/lenovo-legos-hid/core.h >> >> -- >> 2.50.0 >> ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-07-10 15:15 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-03 0:49 [PATCH 0/6] HID: Add Legion Go S Driver Derek J. Clark 2025-07-03 0:49 ` [PATCH 1/6] HID: Include firmware version in the uevent Derek J. Clark 2025-07-03 0:49 ` [PATCH 2/6] HID: Allow HID drivers to add more uevent variables Derek J. Clark 2025-07-03 0:49 ` [PATCH 3/6] HID: Add Legion Go S ID's Derek J. Clark 2025-07-03 0:49 ` [PATCH 4/6] HID: Add documentation for lenovo-legos-hid driver Derek J. Clark 2025-07-03 0:49 ` [PATCH 5/6] HID: Add lenovo-legos-hid core Derek J. Clark 2025-07-03 0:49 ` [PATCH 6/6] HID: Add lenovo-legos-hid configuration endpoint interface Derek J. Clark 2025-07-03 13:48 ` [PATCH 0/6] HID: Add Legion Go S Driver Benjamin Tissoires 2025-07-04 1:57 ` Mario Limonciello 2025-07-04 16:08 ` Richard Hughes 2025-07-10 15:15 ` Derek J. Clark
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).