[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

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

[Rong Zhang]

Hi Josh,

On Fri, 2026-11-06 at 15:48 -0800, Josh Triplett wrote:
> 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.

This is not in contradiction to the fact that blinking an LED is crucial.

When the EC "does that by some more effective means", it will have to
rely on the OS to ensure that the message is delivered. It's both
effective and ineffective.

> 
> I can't comment on what *every* ThinkPad EC would do, but at least on
> this X1 Carbon Gen 11, 
> 

That's the whole issue. The patch unlocks charging LEDs on all models,
while your ThinkPad can't speak on behalf of other models.

> 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.)

That's why relying on the OS to deliver the "effective means" can be
ineffective.

> 
> 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.

Again, "some other models" can't speak on behalf of all models.

> 
> > 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
> 

A tainted kernel isn't something really bad. It just provides a signal to
the (upstream and distribution) maintainers that something outstanding
has occurred.

OTOH, CONFIG_THINKPAD_ACPI_UNSAFE_LEDS should taint the kernel on the
very beginning.

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

That's why I proposed the debugfs approach. At the cost of tainting the
kernel, it can hopefully enter all major distributions, making it widely
available. Users with some sensible reasons (like yours) will be able to
unlock any LED without rebuilding the kernel on their own.

Thanks,
Rong

> 
> - Josh

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

[Henrique de Moraes Holschuh]

Eh, I better voice something about this...

1. The EC does use the battery led for alarms, and it also has specific high urgency events when it thinks the thing will catch fire or going to forcefully cut power very soon, although this was a relatively later enhancement.

Thinkpad ACPI is supposed to log a critical warning and it should attempt to initiate a system shutdown (power off) if it ever receives battery emergency alerts.  I was explicitly advised to do it this way by Lenovo bios engineering, in private, a long time ago.

2. Module parameters are the way I dealt with this kind of thing in the past, especially if they are not mutable at runtime.  It is vastly better than debugfs for this kind of thing IMHO and the fan control was implemented that way.

But the default must be the safer behavior.

> OTOH, CONFIG_THINKPAD_ACPI_UNSAFE_LEDS should taint the kernel on the
> very beginning.

I don't think tainting makes sense as long as it is something you either have to compile for and a distro would never do so as a default, or something you need to load the module with an explicit parameter set.  It will not cause system instability, unlike, say, poking around the EC registers directly.

A module parameter is a decent compromise IMO: 
Debugfs is too reachable for userspace to mess with without actually doing an informed opt-in.  People respect module parameters a lot more.

-- 
  Henrique Holschuh