[PATCH] platform/x86: tpacpi: Mark charging LEDs as safe

[PATCH] platform/x86: tpacpi: Mark charging LEDs as safe

[Josh Triplett]

tpacpi can expose the charging LEDs as `tpacpi:orange:batt` (LED 1) and
`tpacpi:green:batt` (LED 2). However, at least on my X1 Carbon Gen 11,
these LEDs get hidden because `TPACPI_SAFE_LEDS` doesn't allow them.

Add these to `TPACPI_SAFE_LEDS`, so that tpacpi can expose them.

This was previously proposed in
https://lore.kernel.org/lkml/20250403040846.7834-1-martin@arp242.net/ .

I'm currently toggling these using write_support via ec_sys, and would
prefer to handle these via the normal LED mechanism.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 drivers/platform/x86/lenovo/thinkpad_acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
index 4f81855ad721..50a8cff5b374 100644
--- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
+++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
@@ -5608,7 +5608,7 @@ static const char * const tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
 	"tpacpi::unknown_led3",
 	"tpacpi::thinkvantage",
 };
-#define TPACPI_SAFE_LEDS	0x1481U
+#define TPACPI_SAFE_LEDS	0x1487U
 
 static inline bool tpacpi_is_led_restricted(const unsigned int led)
 {
-- 
2.55.0

Re: [PATCH] platform/x86: tpacpi: Mark charging LEDs as safe

[Rong Zhang]

Hi Josh, Mark,

On Thu, 2026-11-05 at 11:24 -0800, Josh Triplett wrote:
> tpacpi can expose the charging LEDs as `tpacpi:orange:batt` (LED 1) and
> `tpacpi:green:batt` (LED 2). However, at least on my X1 Carbon Gen 11,
> these LEDs get hidden because `TPACPI_SAFE_LEDS` doesn't allow them.
> 
> Add these to `TPACPI_SAFE_LEDS`, so that tpacpi can expose them.

Please see the commit message of commit a4d5effcc737 ("thinkpad-acpi:
restrict access to some firmware LEDs").

The term "unsafe LED" exists because LEDs are write-only for software and
fire-and-forget for the EC. By taking the control of an unsafe LED, e.g.,
attaching an LED trigger, any warning signal from the EC can no longer be
displayed by the LED.

When it comes to battery LEDs, they are definitely unsafe to override.
After all, batteries are dangerous things when they malfuction. Thus, I
don't think it's suitable to enable it for all users.

> 
> This was previously proposed in
> https://lore.kernel.org/lkml/20250403040846.7834-1-martin@arp242.net/ .
> 
> I'm currently toggling these using write_support via ec_sys, and would
> prefer to handle these via the normal LED mechanism.

Since not providing them via the LED core does not really prevents users
to control them, I am also thinking about adding a debugfs attribute that
unlocks unsafe LEDs and taints the kernel.

I believe tainting the kernel when overriding unsafe LEDs would be a
better approach. This is also one of the reason why taint flags exist,
i.e., indicating that something unsupported has happened. Multiple
drivers have taken the approach (hint: grep TAINT_USER).

We can also make the overriding procedure tedious to hopefully block
users who don't know what they are doing:

- Drop CONFIG_THINKPAD_ACPI_UNSAFE_LEDS, so there is no longer a
permanent switch
- Unsafe LEDs are also registered, but with LED_SYSFS_DISABLE set to
disable write access
- Add a debugfs attribute unlock_unsafe_led to every unsafe LED, which
can be written to unlock the LED's write access
- When unlocking, taints the kernel with TAINT_USER

Users must write to the attribute to unlock an unsafe LED. They can't
unlock all unsafe LEDs at once. Instead, they must unlock whatever they
desire one by one.

From the said commit message, it seems that the reason why
CONFIG_THINKPAD_ACPI_UNSAFE_LEDS was introduced was that Lenovo didn't
want to support the use case of overriding unsafe LEDs. So...

Mark, what do you think of the approach?

I will send a patch implementing my proposal soon. You can reply there.

Thanks,
Rong

> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
>  drivers/platform/x86/lenovo/thinkpad_acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/lenovo/thinkpad_acpi.c b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> index 4f81855ad721..50a8cff5b374 100644
> --- a/drivers/platform/x86/lenovo/thinkpad_acpi.c
> +++ b/drivers/platform/x86/lenovo/thinkpad_acpi.c
> @@ -5608,7 +5608,7 @@ static const char * const tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
>  	"tpacpi::unknown_led3",
>  	"tpacpi::thinkvantage",
>  };
> -#define TPACPI_SAFE_LEDS	0x1481U
> +#define TPACPI_SAFE_LEDS	0x1487U
>  
>  static inline bool tpacpi_is_led_restricted(const unsigned int led)
>  {

Re: [PATCH] platform/x86: tpacpi: Mark charging LEDs as safe

[Josh Triplett]

On Sat, Nov 07, 2026 at 04:48:44AM +0800, Rong Zhang wrote:
> The term "unsafe LED" exists because LEDs are write-only for software and
> fire-and-forget for the EC. By taking the control of an unsafe LED, e.g.,
> attaching an LED trigger, any warning signal from the EC can no longer be
> displayed by the LED.
> 
> When it comes to battery LEDs, they are definitely unsafe to override.
> After all, batteries are dangerous things when they malfuction. Thus, I
> don't think it's suitable to enable it for all users.

Choosing to not report "charging" vs "full" vs "not charging" via LEDs
(as opposed to on-screen indicators) is not in itself dangerous. If the
EC has something like "battery is damaged/overheating/exploding" to
report, it had better do that by some more effective means than blinking
an LED, or the message is going to be lost.

I can't comment on what *every* ThinkPad EC would do, but at least on
this X1 Carbon Gen 11, based on some (LLM-assisted, but all of this mail
is 100% human-written) disassembly of both the ACPI tables and the EC
firmware (EC version 1.22 specifically), the EC does not appear to
report any battery states via the battery LEDs other than:
- AC not present (LEDs off)
- AC present and battery <90% (LEDs yellow)
- AC present and battery >90% (LEDs white).

Critical battery diagnostics can get reported through ACPI, and if the
firmware reports one of those, it appears to lead to tpacpi reporting
"THERMAL EMERGENCY" and sending a userspace netlink message. (As an
aside, AFAICT a system running upower and GNOME doesn't display that,
unlike low-battery warnings. It really should, in a hard-to-ignore way.)

Some other models (e.g. the X1 Carbon Gen 12) can emit other kinds of
diagnostics via LED blink codes (
https://download.lenovo.com/manual/x1_carbon_g12/user_guide/en/help_troubleshoot_or_diagnose_problems.html#t3
). But those primarily appear to represent pre-boot failure states where you
won't get to an OS, anyway.

> Since not providing them via the LED core does not really prevents users
> to control them, I am also thinking about adding a debugfs attribute that
> unlocks unsafe LEDs and taints the kernel.

I'd really love to not have a tainted kernel just because I'd like to
not have a bright LED in my hotel room when I'm trying to sleep.

- Josh