[PATCH] platform: arm64: thinkpad-t14s-ec: don't send KEY_KBDILLUMTOGGLE on Fn+Space

[PATCH] platform: arm64: thinkpad-t14s-ec: don't send KEY_KBDILLUMTOGGLE on Fn+Space

[David Stephenson]

On the T14s, the EC already steps the keyboard backlight when you press
Fn+Space: off, then low, then high.

We report that change with t14s_kbd_bl_update() (brightness_hw_changed).
We then also send KEY_KBDILLUMTOGGLE.

The extra key is the problem. Desktops treat it as "please toggle the
light." GNOME calls Keyboard.Toggle() and changes the LED itself. That
fights the EC's three steps. The on-screen bar also stays empty,
because Toggle() always returns 0%.

thinkpad_acpi on x86 already skips the key when the firmware changed
the light. It only uses brightness_hw_changed. Do the same here: keep
the LED update, do not send KEY_KBDILLUMTOGGLE.

Tested on a Lenovo ThinkPad T14s Gen 6 (21N1, Snapdragon X Elite) with
GNOME. After this change the OSD bar tracks off / 50% / 100%.

Signed-off-by: David Stephenson <git@davidstephenson.net>
---
 drivers/platform/arm64/lenovo-thinkpad-t14s.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index 5590302a5..22d20a907 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -488,8 +488,14 @@ static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
 	case T14S_EC_EVT_NONE:
 		break;
 	case T14S_EC_EVT_KEY_FN_SPACE:
+		/*
+		 * Firmware already cycles the keyboard backlight
+		 * (off / low / high). Notify userspace through the LED
+		 * class only. Emitting KEY_KBDILLUMTOGGLE as well makes
+		 * desktop environments toggle the LED a second time.
+		 */
 		t14s_kbd_bl_update(ec);
-		fallthrough;
+		break;
 	case T14S_EC_EVT_KEY_FN_F4:
 	case T14S_EC_EVT_KEY_FN_F7:
 	case T14S_EC_EVT_KEY_FN_4:
-- 
2.55.0

Re: [PATCH] platform: arm64: thinkpad-t14s-ec: don't send KEY_KBDILLUMTOGGLE on Fn+Space

[Hans de Goede]

Hi,

On 25-Aug-26 21:36, David Stephenson wrote:
> On the T14s, the EC already steps the keyboard backlight when you press
> Fn+Space: off, then low, then high.
> 
> We report that change with t14s_kbd_bl_update() (brightness_hw_changed).
> We then also send KEY_KBDILLUMTOGGLE.
> 
> The extra key is the problem. Desktops treat it as "please toggle the
> light." GNOME calls Keyboard.Toggle() and changes the LED itself. That
> fights the EC's three steps. The on-screen bar also stays empty,
> because Toggle() always returns 0%.
> 
> thinkpad_acpi on x86 already skips the key when the firmware changed
> the light. It only uses brightness_hw_changed. Do the same here: keep
> the LED update, do not send KEY_KBDILLUMTOGGLE.
> 
> Tested on a Lenovo ThinkPad T14s Gen 6 (21N1, Snapdragon X Elite) with
> GNOME. After this change the OSD bar tracks off / 50% / 100%.
> 
> Signed-off-by: David Stephenson <git@davidstephenson.net>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans



> ---
>  drivers/platform/arm64/lenovo-thinkpad-t14s.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> index 5590302a5..22d20a907 100644
> --- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> +++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> @@ -488,8 +488,14 @@ static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
>  	case T14S_EC_EVT_NONE:
>  		break;
>  	case T14S_EC_EVT_KEY_FN_SPACE:
> +		/*
> +		 * Firmware already cycles the keyboard backlight
> +		 * (off / low / high). Notify userspace through the LED
> +		 * class only. Emitting KEY_KBDILLUMTOGGLE as well makes
> +		 * desktop environments toggle the LED a second time.
> +		 */
>  		t14s_kbd_bl_update(ec);
> -		fallthrough;
> +		break;
>  	case T14S_EC_EVT_KEY_FN_F4:
>  	case T14S_EC_EVT_KEY_FN_F7:
>  	case T14S_EC_EVT_KEY_FN_4: