Bug 205939 - Thinkpad battery: status=Unknown in sysfs when charge_*_threshold in effect
Summary: Thinkpad battery: status=Unknown in sysfs when charge_*_threshold in effect
Status: CLOSED DOCUMENTED
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Battery (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: acpi_power-battery
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-12-21 16:18 UTC by haarp
Modified: 2020-06-29 11:24 UTC (History)
3 users (show)

See Also:
Kernel Version: 5.4.3
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description haarp 2019-12-21 16:18:17 UTC
Hello,

on Thinkpads (tested on a W530 and X1 Extreme Gen 2), charge thresholds can be set under /sys/class/power_supply/BAT0/charge_{start,stop}_threshold.

When a stop threshold is set to e.g. 90, the battery will stop charging when it reaches 90%. However, /sys/class/power_supply/BAT0/status will now show "Unknown", when it should be something else. Probably "Full", but one might argue that it's not really "full"...

The "Unknown" status confuses userspace. e.g. upower will make wrong assumptions about the battery state. It will assume an idle battery is still charging (when below 90%).

Thanks!
Comment 1 Zhang Rui 2019-12-29 12:19:00 UTC
power_supply sysfs I/F can not reflect this status clearly. the ACPI _BST method can not reflect this neither, there is no such definition in the ACPI spec for this. I'm not sure why we should fix this in Linux kernel.
Further more, as upower knows the user setting by checking /sys/class/power_supply/BAT0/charge_{start,stop}_threshold, why not have such a logic in upower instead?
Comment 2 haarp 2019-12-29 14:13:49 UTC
Thanks for your reply.

I had hoped the kernel would have a reliable source to read the status from. Since there isn't, I understand why "Unknown" is used.

As for "why", IMO it's the kernel's job to handle hardware abstraction and to provide accurate information. As userspace, I would want to read from sysfs and gain the info I need, without even needing knowledge of special cases such as charge thresholds.

Cheers
Comment 3 Zhang Rui 2019-12-30 03:55:33 UTC
please attach the output of "cat /sys/class/power_supply/BAT0/device/path", I'd like to confirm if this is an ACPI Battery device or not. Now I doubt this because ACPI Battery does not support charge_stop_threshold. But instead drivers/platform/x86/thinkpad_acpi.c supports this.
Comment 4 haarp 2019-12-30 10:21:48 UTC
> cat /sys/class/power_supply/BAT0/device/path
\_SB_.PCI0.LPCB.EC__.BAT0

Which indicates an ACPI battery, if I'm not mistaken.

But I can verify that charge_{start,stop}_threshold are indeed provided by thinkpad_acpi. I should probably move this issue to the relevant channels, but I could not find a Product/Component for thinkpad_apci on this Bugzilla.
Comment 5 Zhang Rui 2019-12-31 02:38:29 UTC
Hah, this becomes more complex.
Now we have an ACPI battery device, and its charge_{start,stop}_threshold attributes are created by thinkpad_acpi.

acpi battery driver knows the current capacity but it does not know the threshold, the same thing applies to thinkpad_acpi driver.

power_supply class is the only place that knows all the information to do this trick.
1. calculate charge_end_capacity from charge_end_threshold
2. compare the current capacity and the expected charge_end_capacity, and override the battery status to something like "Full"?

I will get some feedback of this idea and come back later.
Comment 6 haarp 2019-12-31 12:31:41 UTC
Sounds good, thanks for your efforts!

I do wonder how accurate calculating capacities based on percentages is. I suspect the capacity comparison might end up needing some fuzz to work reliably.
Comment 7 Zhang Rui 2020-01-02 05:36:32 UTC
CC Sebastian Reichel <sre@kernel.org>

Hi, Sebastian, what do you think of this issue?
Comment 8 Sebastian Reichel 2020-01-03 21:12:39 UTC
Hi,

We have POWER_SUPPLY_STATUS_NOT_CHARGING, which basically means, that the battery is idle and should fit well for this case. It should be possible to handle this in `drivers/acpi/battery.c`

-- Sebastian
Comment 9 Zhang Rui 2020-06-29 11:24:40 UTC
No, we should not handle this in ACPI battery driver because it has no idea of charge_{start,stop}_threshold. We have no such definition in ACPI spec.
userspace is the only place that knows both the current_capacity and charge_threshold, and then know that the battery is not charging because of charge threshold.
Checked with Rafael, and we won't do anything in ACPI battery driver. sorry.

Note You need to log in before you can comment on or make changes to this bug.