[PATCH 0/2] Add keyboard LED support for Lenovo IdeaPad 83RR/83SR

[PATCH 0/2] Add keyboard LED support for Lenovo IdeaPad 83RR/83SR

[Rodnei Cilto]

This series adds support for the internal keyboard of Lenovo IdeaPad
laptops based on Wildcat Lake SoC (models 83RR worldwide and 83SR
Brazil regional variant).

The EC PS/2 emulation on these models does not fully support the AT
protocol. Sending the SETLEDS command (0xED) after initialization
causes the EC to return corrupted scancodes ('**' in i8042.debug),
rendering the keyboard non-functional.

Patch 1 adds a softleds DMI quirk to atkbd that suppresses 0xED
while keeping EV_LED capabilities visible to userspace.

Patch 2 adds physical CapsLock/NumLock LED control in ideapad-laptop
via direct EC register access (offset 0xA1) and ACPI _QDF method,
discovered via DSDT analysis.

Rodnei Cilto (2):
  input: atkbd: add softleds quirk for broken EC PS/2 emulation
  platform/x86: ideapad-laptop: add CapsLock/NumLock LED via EC

Signed-off-by: Rodnei Cilto <rodnei.cilto@gmail.com>
---
Rodnei Cilto (2):
      input: atkbd: add softleds quirk for broken EC PS/2 emulation
      platform/x86: ideapad-laptop: add CapsLock/NumLock LED via EC

 drivers/input/keyboard/atkbd.c               |  46 ++++++++-
 drivers/input/serio/i8042-acpipnpio.h        |   3 +
 drivers/platform/x86/lenovo/ideapad-laptop.c | 143 +++++++++++++++++++++++++++
 3 files changed, 191 insertions(+), 1 deletion(-)
---
base-commit: 03e2778d1f11de9260543f969e9e888a1c2bf830
change-id: 20260628-ideapad-kbd-leds-16f587e9ab97

Best regards,
--  
Rodnei Cilto <rodnei.cilto@gmail.com>

[PATCH 1/2] input: atkbd: add softleds quirk for broken EC PS/2 emulation

[Rodnei Cilto]

Some Lenovo IdeaPad laptops (e.g. 83RR/83SR, Wildcat Lake) implement
PS/2 keyboard emulation via the Embedded Controller (EC) but do not
fully support the AT protocol. Specifically, sending the SETLEDS
command (0xED) after initialization causes the EC to return corrupted
scancodes (reported as '**' in i8042.debug), rendering the keyboard
non-functional.

The existing SERIO_QUIRK_DUMBKBD resolves scancode corruption by
zeroing serio->write, preventing AT commands. However, LED registration
in atkbd_set_device_attrs() depends on atkbd->write being set, so
dumbkbd mode loses EV_LED capabilities entirely.

Note: serio->id.extra is __u8 (8 bits only) and cannot be used to
pass new quirk flags from i8042 to atkbd. The quirk is detected
directly in atkbd via its DMI quirk table.

Introduce atkbd_softleds: a DMI-detected mode that combines dumbkbd
behaviour (serio->write = NULL, no 0xED sent) with EV_LED registration
so that CapsLock/NumLock/ScrollLock state remains visible to userspace
via the input subsystem.

Add DMI entries for Lenovo IdeaPad 83RR (Wildcat Lake) and its Brazil
regional variant 83SR.

Signed-off-by: Rodnei Cilto <rodnei.cilto@gmail.com>
---
 drivers/input/keyboard/atkbd.c        | 46 ++++++++++++++++++++++++++++++++++-
 drivers/input/serio/i8042-acpipnpio.h |  3 +++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 8cb4dc6fb165..826a21dc016a 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -212,6 +212,7 @@ struct atkbd {
 	bool softrepeat;
 	bool softraw;
 	bool scroll;
+	bool softleds;		/* suppress 0xED, register EV_LED in software */
 	bool enabled;
 
 	/* Accessed only from interrupt */
@@ -245,6 +246,7 @@ static unsigned int (*atkbd_platform_scancode_fixup)(struct atkbd *, unsigned in
  * to many commands until full reset (ATKBD_CMD_RESET_BAT) is performed.
  */
 static bool atkbd_skip_deactivate;
+static bool atkbd_softleds;
 
 static ssize_t atkbd_attr_show_helper(struct device *dev, char *buf,
 				      ssize_t (*handler)(struct atkbd *, char *));
@@ -600,6 +602,14 @@ static int atkbd_set_leds(struct atkbd *atkbd)
 	struct input_dev *dev = atkbd->dev;
 	u8 param[2];
 
+	/*
+	 * softleds: EC PS/2 emulation does not support AT commands
+	 * after initialization. Accept LED state from userspace but
+	 * never send SETLEDS (0xED) to avoid scancode corruption.
+	 */
+	if (atkbd->softleds)
+		return 0;
+
 	param[0] = (test_bit(LED_SCROLLL, dev->led) ? 1 : 0)
 		 | (test_bit(LED_NUML,    dev->led) ? 2 : 0)
 		 | (test_bit(LED_CAPSL,   dev->led) ? 4 : 0);
@@ -1193,7 +1203,7 @@ static void atkbd_set_device_attrs(struct atkbd *atkbd)
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) |
 		BIT_MASK(EV_MSC);
 
-	if (atkbd->write) {
+	if (atkbd->write || atkbd->softleds) {
 		input_dev->evbit[0] |= BIT_MASK(EV_LED);
 		input_dev->ledbit[0] = BIT_MASK(LED_NUML) |
 			BIT_MASK(LED_CAPSL) | BIT_MASK(LED_SCROLLL);
@@ -1291,6 +1301,12 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv)
 	if (atkbd->softrepeat)
 		atkbd->softraw = true;
 
+	if (atkbd_softleds) {
+		serio->write = NULL;
+		atkbd->write = false;
+		atkbd->softleds = true;
+	}
+
 	serio_set_drvdata(serio, atkbd);
 
 	err = serio_open(serio, drv);
@@ -1767,6 +1783,12 @@ static int __init atkbd_deactivate_fixup(const struct dmi_system_id *id)
 	return 1;
 }
 
+static int __init atkbd_setup_softleds(const struct dmi_system_id *id)
+{
+	atkbd_softleds = true;
+	return 1;
+}
+
 /*
  * NOTE: do not add any more "force release" quirks to this table.  The
  * task of adjusting list of keys that should be "released" automatically
@@ -1938,6 +1960,28 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
 		},
 		.callback = atkbd_deactivate_fixup,
 	},
+	{
+		/*
+		 * Lenovo IdeaPad 83RR (Wildcat Lake) - EC PS/2 emulation
+		 * returns corrupted scancodes ('**' in i8042.debug) when
+		 * receiving AT SETLEDS (0xED) after keyboard initialization.
+		 * Enable softleds mode: suppress 0xED to hardware while
+		 * keeping CapsLock/NumLock/ScrollLock visible to userspace.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "83RR"),
+		},
+		.callback = atkbd_setup_softleds,
+	},
+	{
+		/* Lenovo IdeaPad 83SR (83RR Brazil regional variant) */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "83SR"),
+		},
+		.callback = atkbd_setup_softleds,
+	},
 	{ }
 };
 
diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 8ebdf4fb9030..d233544ebac9 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -79,6 +79,9 @@ static inline void i8042_write_command(int val)
 #define SERIO_QUIRK_DIRECT		BIT(8)
 #define SERIO_QUIRK_DUMBKBD		BIT(9)
 #define SERIO_QUIRK_NOLOOP		BIT(10)
+/* SERIO_QUIRK_DUMBKBD_LEDS handled via atkbd DMI quirk table.
+ * serio->id.extra is __u8 (8 bits only), cannot carry this flag.
+ */
 #define SERIO_QUIRK_NOTIMEOUT		BIT(11)
 #define SERIO_QUIRK_KBDRESET		BIT(12)
 #define SERIO_QUIRK_DRITEK		BIT(13)

-- 
2.51.0

Re: [PATCH 1/2] input: atkbd: add softleds quirk for broken EC PS/2 emulation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The `atkbd` driver mutates the parent `serio` port's `write` function pointer to `NULL`, which is a driver model violation and permanently corrupts the bus state.
- [Medium] The newly added early return `if (atkbd->softleds) return 0;` inside `atkbd_set_leds()` is completely unreachable dead code.

Pre-existing issues:
- [High] A pre-existing race condition in `atkbd_disconnect()` can lead to a Use-After-Free of the `input_dev` structure in `atkbd_event_work()`.
--

commit 449f71d6f3b7e8daf08e55bc0da16d0f59475df4
Author: Rodnei Cilto <rodnei.cilto@gmail.com>

input: atkbd: add softleds quirk for broken EC PS/2 emulation

This commit introduces the atkbd_softleds mode to handle broken EC PS/2
emulation on some Lenovo IdeaPad laptops. It combines dumbkbd behaviour with
EV_LED registration to keep LED states visible to userspace without sending
the SETLEDS command. DMI entries are added for the Lenovo IdeaPad 83RR and
83SR.

> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
> index 8cb4dc6fb1658..826a21dc016ab 100644
> --- a/drivers/input/keyboard/atkbd.c
> +++ b/drivers/input/keyboard/atkbd.c
> @@ -600,6 +602,14 @@ static int atkbd_set_leds(struct atkbd *atkbd)
>  	struct input_dev *dev = atkbd->dev;
>  	u8 param[2];
>  
> +	/*
> +	 * softleds: EC PS/2 emulation does not support AT commands
> +	 * after initialization. Accept LED state from userspace but
> +	 * never send SETLEDS (0xED) to avoid scancode corruption.
> +	 */
> +	if (atkbd->softleds)
> +		return 0;
> +

[Severity: Medium]
Is this early return reachable? When atkbd_softleds is active, atkbd_connect()
sets atkbd->write = false.

When an EV_LED event occurs, atkbd_event() checks atkbd->write and immediately
returns -1, dropping the event before atkbd_schedule_event_work() is called.

Does this mean atkbd_set_leds() is never executed by the workqueue in this
mode, and this check will never be triggered?

[ ... ]

> @@ -1291,6 +1301,12 @@ static int atkbd_connect(struct serio *serio, struct serio_driver *drv)
>  	if (atkbd->softrepeat)
>  		atkbd->softraw = true;
>  
> +	if (atkbd_softleds) {
> +		serio->write = NULL;
> +		atkbd->write = false;
> +		atkbd->softleds = true;
> +	}
> +

[Severity: High]
Does this permanently modify the parent bus provider's function pointer
(serio->write)?

If atkbd is unbound and another driver (e.g., serio_raw) binds to the same
port, or if atkbd is rebound, would the port remain crippled and unable to
transmit due to serio->write still being NULL?

[Severity: High]
This is a pre-existing issue, but can this sequence lead to a use-after-free
if a privileged user unbinds the driver while LED or repeat rate events are
being processed?

In atkbd_disconnect(), input_unregister_device() is called before
cancel_delayed_work_sync():

drivers/input/keyboard/atkbd.c:atkbd_disconnect() {
    ...
    input_unregister_device(atkbd->dev);

    /*
     * Make sure we don't have a command in flight.
     ...
     */
    cancel_delayed_work_sync(&atkbd->event_work);
    ...
}

If atkbd_event_work() starts executing on another CPU, it acquires
atkbd->mutex and checks atkbd->enabled. If preempted immediately after this
check, atkbd_disconnect() can proceed to set atkbd->enabled = false,
unregister and free the input device, and then block on
cancel_delayed_work_sync().

When the worker thread resumes, it calls atkbd_set_leds():

drivers/input/keyboard/atkbd.c:atkbd_event_work() {
    ...
    if (!atkbd->enabled) {
        ...
    } else {
        if (test_and_clear_bit(ATKBD_LED_EVENT_BIT, &atkbd->event_mask))
            atkbd_set_leds(atkbd);
    ...
}

Would atkbd_set_leds() then dereference the now-freed atkbd->dev?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628-ideapad-kbd-leds-v1-0-50b421d94927@gmail.com?part=1

Re: [PATCH 1/2] input: atkbd: add softleds quirk for broken EC PS/2 emulation

[Dmitry Torokhov]

Hi Rodnei,

On Sun, Jun 28, 2026 at 10:57:03PM -0300, Rodnei Cilto wrote:
> Some Lenovo IdeaPad laptops (e.g. 83RR/83SR, Wildcat Lake) implement
> PS/2 keyboard emulation via the Embedded Controller (EC) but do not
> fully support the AT protocol. Specifically, sending the SETLEDS
> command (0xED) after initialization causes the EC to return corrupted
> scancodes (reported as '**' in i8042.debug), rendering the keyboard
> non-functional.

"**" do not represent corrupted scancodes, they are safety measure to
not disclose your password on accident.

Use i8042.debug=1 i8042.unmask_kbd_data=1 to unveil real data.

Thanks.

-- 
Dmitry

[PATCH 2/2] platform/x86: ideapad-laptop: add CapsLock/NumLock LED via EC

[Rodnei Cilto]

Some Lenovo IdeaPad laptops (e.g. 83RR/83SR, Wildcat Lake) have
physical CapsLock and NumLock LEDs controlled via the EC.

The EC exposes CAPL (bit 5) and NUML (bit 4) at offset 0xA1.
Writing these bits via ec_write() and evaluating _QDF via
acpi_evaluate_object() causes the firmware to sync EC state to the
GPIO lines that drive the physical LEDs.

Discovery via DSDT analysis on Lenovo IdeaPad 83RR (Wildcat Lake):
  - CAPL/NUML at EC offset 0xA1 (bits 5 and 4)
  - _QDF (_SB.PC00.LPCB.EC0._QDF) reads CAPL/NUML -> SGOV()
  - GPIO 0x001A1087 -> CapsLock LED physical pin
  - GPIO 0x001A0485 -> NumLock LED physical pin
  - ec_read/ec_write exported via EXPORT_SYMBOL in drivers/acpi/ec.c
    and declared in <linux/acpi.h>

Add two led_classdev entries (input::capslock, input::numlock)
guarded by DMI match (features.kbd_leds) for 83RR and its Brazil
regional variant 83SR.

Signed-off-by: Rodnei Cilto <rodnei.cilto@gmail.com>
---
 drivers/platform/x86/lenovo/ideapad-laptop.c | 143 +++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
index 4fbc904f1fc3..03e3f234067c 100644
--- a/drivers/platform/x86/lenovo/ideapad-laptop.c
+++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
@@ -42,6 +42,17 @@
 
 #include <dt-bindings/leds/common.h>
 
+/* EC keyboard LED control (IdeaPad EC PS/2 emulation).
+ * Validated on Lenovo IdeaPad 83RR (Wildcat Lake):
+ *   EC offset 0xA1 bit4=NUML, bit5=CAPL
+ *   _QDF syncs EC state to GPIO -> physical LED
+ * ec_read/ec_write declared in <linux/acpi.h>
+ */
+#define IDEAPAD_EC_KBD_LED_OFFSET	0xA1
+#define IDEAPAD_EC_KBD_LED_NUML_BIT	BIT(4)
+#define IDEAPAD_EC_KBD_LED_CAPL_BIT	BIT(5)
+#define IDEAPAD_ACPI_EC0_QDF_PATH	"\\_SB.PC00.LPCB.EC0._QDF"
+
 #define IDEAPAD_RFKILL_DEV_NUM	3
 
 enum {
@@ -198,6 +209,7 @@ struct ideapad_private {
 		bool ctrl_ps2_aux_port    : 1;
 		bool usb_charging         : 1;
 		bool ymc_ec_trigger       : 1;
+		bool kbd_leds             : 1;
 	} features;
 	struct {
 		bool initialized;
@@ -210,6 +222,11 @@ struct ideapad_private {
 		struct led_classdev led;
 		unsigned int last_brightness;
 	} fn_lock;
+	struct {
+		bool initialized;
+		struct led_classdev capslock;
+		struct led_classdev numlock;
+	} kbd_leds;
 };
 
 static bool no_bt_rfkill;
@@ -1587,6 +1604,99 @@ static void ideapad_backlight_notify_brightness(struct ideapad_private *priv)
 /*
  * keyboard backlight
  */
+static int ideapad_kbd_led_ec_set(u8 bit, bool on)
+{
+	u8 val;
+	int err;
+
+	err = ec_read(IDEAPAD_EC_KBD_LED_OFFSET, &val);
+	if (err)
+		return err;
+	if (on)
+		val |= bit;
+	else
+		val &= ~bit;
+	err = ec_write(IDEAPAD_EC_KBD_LED_OFFSET, val);
+	if (err)
+		return err;
+	acpi_evaluate_object(NULL, IDEAPAD_ACPI_EC0_QDF_PATH, NULL, NULL);
+	return 0;
+}
+
+static void ideapad_capslock_led_set(struct led_classdev *led_cdev,
+				     enum led_brightness brightness)
+{
+	ideapad_kbd_led_ec_set(IDEAPAD_EC_KBD_LED_CAPL_BIT, brightness != LED_OFF);
+}
+
+static enum led_brightness ideapad_capslock_led_get(struct led_classdev *led_cdev)
+{
+	u8 val;
+
+	if (ec_read(IDEAPAD_EC_KBD_LED_OFFSET, &val))
+		return LED_OFF;
+	return (val & IDEAPAD_EC_KBD_LED_CAPL_BIT) ? LED_ON : LED_OFF;
+}
+
+static void ideapad_numlock_led_set(struct led_classdev *led_cdev,
+				    enum led_brightness brightness)
+{
+	ideapad_kbd_led_ec_set(IDEAPAD_EC_KBD_LED_NUML_BIT, brightness != LED_OFF);
+}
+
+static enum led_brightness ideapad_numlock_led_get(struct led_classdev *led_cdev)
+{
+	u8 val;
+
+	if (ec_read(IDEAPAD_EC_KBD_LED_OFFSET, &val))
+		return LED_OFF;
+	return (val & IDEAPAD_EC_KBD_LED_NUML_BIT) ? LED_ON : LED_OFF;
+}
+
+static int ideapad_kbd_leds_init(struct ideapad_private *priv)
+{
+	int err;
+
+	if (WARN_ON(priv->kbd_leds.initialized))
+		return -EEXIST;
+
+	priv->kbd_leds.capslock.name           = "input::capslock";
+	priv->kbd_leds.capslock.max_brightness = 1;
+	priv->kbd_leds.capslock.brightness_set = ideapad_capslock_led_set;
+	priv->kbd_leds.capslock.brightness_get = ideapad_capslock_led_get;
+	priv->kbd_leds.capslock.flags          = LED_RETAIN_AT_SHUTDOWN;
+
+	err = led_classdev_register(&priv->platform_device->dev,
+				    &priv->kbd_leds.capslock);
+	if (err)
+		return err;
+
+	priv->kbd_leds.numlock.name            = "input::numlock";
+	priv->kbd_leds.numlock.max_brightness  = 1;
+	priv->kbd_leds.numlock.brightness_set  = ideapad_numlock_led_set;
+	priv->kbd_leds.numlock.brightness_get  = ideapad_numlock_led_get;
+	priv->kbd_leds.numlock.flags           = LED_RETAIN_AT_SHUTDOWN;
+
+	err = led_classdev_register(&priv->platform_device->dev,
+				    &priv->kbd_leds.numlock);
+	if (err) {
+		led_classdev_unregister(&priv->kbd_leds.capslock);
+		return err;
+	}
+
+	priv->kbd_leds.initialized = true;
+	return 0;
+}
+
+static void ideapad_kbd_leds_exit(struct ideapad_private *priv)
+{
+	if (!priv->kbd_leds.initialized)
+		return;
+	priv->kbd_leds.initialized = false;
+	led_classdev_unregister(&priv->kbd_leds.numlock);
+	led_classdev_unregister(&priv->kbd_leds.capslock);
+}
+
 static int ideapad_kbd_bl_check_tristate(int type)
 {
 	return (type == KBD_BL_TRISTATE) || (type == KBD_BL_TRISTATE_AUTO);
@@ -1832,6 +1942,29 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv, bool send_
 	priv->r_touchpad_val = value;
 }
 
+static const struct dmi_system_id ideapad_kbd_leds_dmi_table[] = {
+	{
+		/*
+		 * Lenovo IdeaPad 83RR (Wildcat Lake) - EC PS/2 emulation
+		 * controls CapsLock/NumLock LEDs via EC offset 0xA1 + _QDF.
+		 * CAPL=bit5 (0x20), NUML=bit4 (0x10).
+		 * _QDF drives GPIO via SGOV() to physical LED pins.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "83RR"),
+		},
+	},
+	{
+		/* Lenovo IdeaPad 83SR (83RR Brazil regional variant) */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "83SR"),
+		},
+	},
+	{ }
+};
+
 static const struct dmi_system_id ymc_ec_trigger_quirk_dmi_table[] = {
 	{
 		/* Lenovo Yoga 7 14ARB7 */
@@ -2178,6 +2311,8 @@ static int ideapad_check_features(struct ideapad_private *priv)
 	priv->features.touchpad_ctrl_via_ec = touchpad_ctrl_via_ec;
 	priv->features.ymc_ec_trigger =
 		ymc_ec_trigger || dmi_check_system(ymc_ec_trigger_quirk_dmi_table);
+	priv->features.kbd_leds =
+		dmi_check_system(ideapad_kbd_leds_dmi_table);
 
 	if (!read_ec_data(handle, VPCCMD_R_FAN, &val))
 		priv->features.fan_mode = true;
@@ -2418,6 +2553,12 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 			dev_info(&pdev->dev, "FnLock control not available\n");
 	}
 
+	if (priv->features.kbd_leds) {
+		err = ideapad_kbd_leds_init(priv);
+		if (err)
+			dev_warn(&pdev->dev, "Could not set up kbd LEDs: %d\n", err);
+	}
+
 	/*
 	 * On some models without a hw-switch (the yoga 2 13 at least)
 	 * VPCCMD_W_RF must be explicitly set to 1 for the wifi to work.
@@ -2477,6 +2618,7 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 		ideapad_unregister_rfkill(priv, i);
 
 	ideapad_fn_lock_led_exit(priv);
+	ideapad_kbd_leds_exit(priv);
 	ideapad_kbd_bl_exit(priv);
 	ideapad_input_exit(priv);
 
@@ -2506,6 +2648,7 @@ static void ideapad_acpi_remove(struct platform_device *pdev)
 		ideapad_unregister_rfkill(priv, i);
 
 	ideapad_fn_lock_led_exit(priv);
+	ideapad_kbd_leds_exit(priv);
 	ideapad_kbd_bl_exit(priv);
 	ideapad_input_exit(priv);
 	ideapad_debugfs_exit(priv);

-- 
2.51.0

Re: [PATCH 2/2] platform/x86: ideapad-laptop: add CapsLock/NumLock LED via EC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Sleeping in atomic context due to improper LED brightness callback assignment.
- [Medium] Unsynchronized read-modify-write on the shared EC keyboard LED register.
- [Medium] Missing default triggers for CapsLock and NumLock LEDs.
--

commit 3bd8490b486c6791548a82ecc890f8f432c92886
Author: Rodnei Cilto <rodnei.cilto@gmail.com>

platform/x86: ideapad-laptop: add CapsLock/NumLock LED via EC

This commit adds support for physical CapsLock and NumLock LEDs on specific
Lenovo IdeaPad laptops (83RR/83SR). It maps the EC's offset 0xA1 bits 5 and
4 to CAPL and NUML and exposes them as led_classdev entries.

> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c

[ ... ]

> +static int ideapad_kbd_led_ec_set(u8 bit, bool on)
> +{
> +	u8 val;
> +	int err;
> +
> +	err = ec_read(IDEAPAD_EC_KBD_LED_OFFSET, &val);
> +	if (err)
> +		return err;
> +	if (on)
> +		val |= bit;
> +	else
> +		val &= ~bit;
> +	err = ec_write(IDEAPAD_EC_KBD_LED_OFFSET, val);

[Severity: Medium]
Is this read-modify-write sequence safe against concurrent updates?

Since CapsLock and NumLock are exposed as separate led_classdev devices, the
LED subsystem protects their state with independent locks. If both LEDs are
toggled simultaneously, could two threads concurrently enter
ideapad_kbd_led_ec_set(), read the same initial value, and write it back,
causing one modification to overwrite and lose the other?

> +	if (err)
> +		return err;
> +	acpi_evaluate_object(NULL, IDEAPAD_ACPI_EC0_QDF_PATH, NULL, NULL);
> +	return 0;
> +}

[ ... ]

> +static int ideapad_kbd_leds_init(struct ideapad_private *priv)
> +{
> +	int err;
> +
> +	if (WARN_ON(priv->kbd_leds.initialized))
> +		return -EEXIST;
> +
> +	priv->kbd_leds.capslock.name           = "input::capslock";
> +	priv->kbd_leds.capslock.max_brightness = 1;
> +	priv->kbd_leds.capslock.brightness_set = ideapad_capslock_led_set;

[Severity: Medium]
Will this LED automatically synchronize with the system's actual keyboard lock
state? Without setting .default_trigger = "kbd-capslock", the LED subsystem
might not automatically link the keyboard input state to this LED, requiring
manual sysfs configuration.

[Severity: High]
Does this risk a scheduling while atomic panic? The LED subsystem often calls
the non-blocking brightness_set callback from atomic contexts, like softirq
for keyboard input triggers.

Since ideapad_capslock_led_set() calls ideapad_kbd_led_ec_set(), which uses
ec_read() and ec_write() that acquire mutexes and sleep, should this use
brightness_set_blocking instead to safely defer execution to a workqueue?

> +	priv->kbd_leds.capslock.brightness_get = ideapad_capslock_led_get;
> +	priv->kbd_leds.capslock.flags          = LED_RETAIN_AT_SHUTDOWN;
> +
> +	err = led_classdev_register(&priv->platform_device->dev,
> +				    &priv->kbd_leds.capslock);
> +	if (err)
> +		return err;
> +
> +	priv->kbd_leds.numlock.name            = "input::numlock";
> +	priv->kbd_leds.numlock.max_brightness  = 1;
> +	priv->kbd_leds.numlock.brightness_set  = ideapad_numlock_led_set;

[Severity: Medium]
Similarly, should this have .default_trigger = "kbd-numlock" set to ensure
the NumLock LED automatically tracks the physical keyboard state?

[Severity: High]
As with the CapsLock LED, does this need to use brightness_set_blocking to
avoid sleeping in atomic context when the trigger updates the LED?

> +	priv->kbd_leds.numlock.brightness_get  = ideapad_numlock_led_get;
> +	priv->kbd_leds.numlock.flags           = LED_RETAIN_AT_SHUTDOWN;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628-ideapad-kbd-leds-v1-0-50b421d94927@gmail.com?part=2