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

[Rong Zhang <i@rong.moe>]

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