[PATCH] HID: lenovo: Restore Fn-lock LED state on resume for tp10ubkbd devices

[PATCH] HID: lenovo: Restore Fn-lock LED state on resume for tp10ubkbd devices

[Kean]

The reset_resume callback only restores trackpoint settings for compact
USB keyboards (CUSBKBD, TPIIUSBKBD) but omits the tp10ubkbd device
family: X12_TAB, X12_TAB2, TP10UBKBD, X1_TAB, X1_TAB2, X1_TAB3.

If these devices lose power during suspend, the Fn-lock state reverts
to the hardware default but the driver's cached state and the keyboard
LED are not restored, causing the LED to show the wrong state after
resume.

Add the missing device cases to restore the Fn-lock LED on resume.

Tested:
This patch works on ThinkPad X12_TAB.

Signed-off-by: Kean <rh_king@163.com>
---
 drivers/hid/hid-lenovo.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index a6b73e03c16b..4c9fb163b6d6 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -1424,6 +1424,8 @@ static int lenovo_probe(struct hid_device *hdev,
 
 static int lenovo_reset_resume(struct hid_device *hdev)
 {
+	struct lenovo_drvdata *data;
+
 	switch (hdev->product) {
 	case USB_DEVICE_ID_LENOVO_CUSBKBD:
 	case USB_DEVICE_ID_LENOVO_TPIIUSBKBD:
@@ -1431,6 +1433,17 @@ static int lenovo_reset_resume(struct hid_device *hdev)
 			lenovo_features_set_cptkbd(hdev);
 
 		break;
+	case USB_DEVICE_ID_LENOVO_X12_TAB:
+	case USB_DEVICE_ID_LENOVO_X12_TAB2:
+	case USB_DEVICE_ID_LENOVO_TP10UBKBD:
+	case USB_DEVICE_ID_LENOVO_X1_TAB:
+	case USB_DEVICE_ID_LENOVO_X1_TAB2:
+	case USB_DEVICE_ID_LENOVO_X1_TAB3:
+		data = hid_get_drvdata(hdev);
+		if (data)
+			lenovo_led_set_tp10ubkbd(hdev, TP10UBKBD_FN_LOCK_LED,
+						 data->fn_lock ? LED_ON : LED_OFF);
+		break;
 	default:
 		break;
 	}
-- 
2.53.0

Re: [PATCH] HID: lenovo: Restore Fn-lock LED state on resume for tp10ubkbd devices

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unprotected state modifications / TOCTOU Race leading to Deadlock
--

commit 7c663fefe9bb0e01bc7e377e27b713f9cd82c9d8
Author: Kean <rh_king@163.com>

HID: lenovo: Restore Fn-lock LED state on resume for tp10ubkbd devices

This commit updates the reset_resume callback to restore the Fn-lock LED
state for several ThinkPad tablet keyboards. It does this by reading the
driver's cached state and calling lenovo_led_set_tp10ubkbd to update the
hardware.

> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c

[ ... ]

> @@ -1424,6 +1424,8 @@ static int lenovo_probe(struct hid_device *hdev,
>  
>  static int lenovo_reset_resume(struct hid_device *hdev)
>  {
> +	struct lenovo_drvdata *data;
> +
>  	switch (hdev->product) {
>  	case USB_DEVICE_ID_LENOVO_CUSBKBD:
>  	case USB_DEVICE_ID_LENOVO_TPIIUSBKBD:
> @@ -1431,6 +1433,17 @@ static int lenovo_reset_resume(struct hid_device *hdev)
>  			lenovo_features_set_cptkbd(hdev);
>  
>  		break;
> +	case USB_DEVICE_ID_LENOVO_X12_TAB:
> +	case USB_DEVICE_ID_LENOVO_X12_TAB2:
> +	case USB_DEVICE_ID_LENOVO_TP10UBKBD:
> +	case USB_DEVICE_ID_LENOVO_X1_TAB:
> +	case USB_DEVICE_ID_LENOVO_X1_TAB2:
> +	case USB_DEVICE_ID_LENOVO_X1_TAB3:
> +		data = hid_get_drvdata(hdev);
> +		if (data)
> +			lenovo_led_set_tp10ubkbd(hdev, TP10UBKBD_FN_LOCK_LED,
> +						 data->fn_lock ? LED_ON : LED_OFF);

Could this new call to lenovo_led_set_tp10ubkbd() cause a deadlock during
runtime resume?

When userspace writes to the fn_lock sysfs attribute, it eventually calls
lenovo_led_set_tp10ubkbd() which acquires led_report_mutex before issuing
the raw HID request:

lenovo_led_set_tp10ubkbd()
    mutex_lock(&data->led_report_mutex);
    ...
    hid_hw_raw_request(...)

If the device is runtime-suspended and requires a reset-resume,
hid_hw_raw_request() will trigger usb_autopm_get_interface() to
synchronously wake the device. This causes the USB core to execute
lenovo_reset_resume() in the same thread:

lenovo_reset_resume()
    lenovo_led_set_tp10ubkbd()
        mutex_lock(&data->led_report_mutex);

Since led_report_mutex is already held by the initial sysfs write call,
will this result in an immediate self-deadlock?

Additionally, if a resume is initiated by a remote wakeup via a PM worker
thread while a sysfs write is concurrently holding led_report_mutex, could
this lead to an ABBA deadlock between the sysfs thread and the PM worker
thread?

> +		break;
>  	default:
>  		break;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512142229.4098386-1-rh_king@163.com?part=1

Re: [PATCH] HID: lenovo: Restore Fn-lock LED state on resume for tp10ubkbd devices

[Kean]

Hi,

Thank you for the review.

Regarding the potential issue you raised:

> [High] Unprotected state modifications / TOCTOU Race leading to Deadlock
>
> Could this new call to lenovo_led_set_tp10ubkbd() cause a deadlock during
> runtime resume?

In my initial testing, I performed system suspend/resume (S3) cycles and the
behavior was normal — the Fn-lock LED state was correctly restored after
resume with no hangs or lockups observed. I tested two scenarios:

1. Normal suspend/resume: suspend the system with the keyboard attached,
   then resume normally.
2. Detach-reconnect: suspend the system, physically detach the keyboard
   during suspend, resume the system, then re-attach the keyboard.

Both scenarios appeared to work correctly without issues.

However, I realize my testing only covered system suspend/resume (S3), not
runtime PM resume (which may hold different locks in the HID core). Your
analysis about a potential deadlock is reasonable and worth investigating
thoroughly. I need to:

- Test runtime resume scenarios specifically (e.g., autosuspend of the USB
  HID device).
- Verify there is no lock ordering conflict between led_report_mutex (held
  inside lenovo_led_set_tp10ubkbd) and any HID core locks that may already
  be held when reset_resume is called.

I will run additional tests covering these cases. If I can reproduce any
issue, I will submit a V2 with the appropriate fix. If not, I will follow
up with a detailed description of my testing methodology and conclusions.

Thanks again for your guidance.

Regards,
Kean