Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[T440p Coreboot] False battery readings #657

Open
karstenes opened this issue Oct 25, 2022 · 20 comments
Open

[T440p Coreboot] False battery readings #657

karstenes opened this issue Oct 25, 2022 · 20 comments

Comments

@karstenes
Copy link

karstenes commented Oct 25, 2022

Describe the bug

TLP does not detect my original thinkpad T440p battery on open-rc artix on coreboot. I have made sure acpi_call and thinkpad_acpi are running and I am able to control the fans through thinkpad_acpi. TLP continues to use the generic driver for the battery however. I have tried running linux-lts (linux 5.15.74-1) but no luck with that either.

Expected behavior

TLP detects the battery and uses the modern thinkpad driver.

To Reproduce

The problem occurs on both battery and AC. I am not sure how to reproduce, but my system is the latest artix linux openrc edition. I am running this on the latest skulls coreboot release as well.

Here is my tlp-stat:
https://gist.github.com/karstenes/f8e1b6f220c1d6d12df2587338814bee

@linrunner
Copy link
Owner

Hi, your Gist link doesn't work.

@karstenes
Copy link
Author

Fixed the link

@linrunner
Copy link
Owner

linrunner commented Oct 26, 2022

Coreboot does not return the correct model string as the original BIOS via /sys/class/dmi/id/product_version.

As a workaround you can try to configure:

X_SIMULATE_MODEL="ThinkPad T440p"

Then show tlp-stat -s -b again please.

This information here is also wrong:

/sys/class/power_supply/BAT0/charge_full_design             =   5616 [mAh]
/sys/class/power_supply/BAT0/charge_full                    =   4561 [mAh]
/sys/class/power_supply/BAT0/charge_now                     =   1682 [mAh]
/sys/class/power_supply/BAT0/current_now                    =   2090 [mA]

These are not mA(h) values but mW(h) values missing the last digit (I can conclude that because 56,000 mWh is a typical battery size for the T440p). See #626 (comment) .

You should create a Coreboot bug report for both of your issues.

@karstenes
Copy link
Author

Yes, the battery is 57 Wh. Here is the tlp-stat on ac:
https://gist.github.com/karstenes/0b7eba367c1243a804762bcfc04ef920
and bat:
https://gist.github.com/karstenes/159be9ff50c1799662cec6dbcada21d2

The env variable did work to convince it it is a t440p.

Here's the battery variables you said were wrong (so TLP was just detecting the wrong unit?)

❯ cat /sys/class/power_supply/BAT0/charge_full_design 5616000

❯ cat /sys/class/power_supply/BAT0/charge_full 4561000

❯ cat /sys/class/power_supply/BAT0/charge_now 3210000

❯ cat /sys/class/power_supply/BAT0/current_now 2088000

@linrunner
Copy link
Owner

linrunner commented Oct 27, 2022

No, the kernel returns wrong values and I suppose it gets them from the BIOS via ACPI.

Actually there are two sets of battery sysfiles, one for W units and one for A units (I have added dots as thousand separators for clarity):

(1) W units scheme -- energy = J = Wh
This is what all stock ThinkPad BIOSes provide, example for a 57 Wh battery:

/sys/class/power_supply/BAT0/energy_full_design: 57.000.000 [µWh]
/sys/class/power_supply/BAT0/energy_full: 53.700.000 [µWh]
/sys/class/power_supply/BAT0/energy_now: 37.410.000 [µWh]
/sys/class/power_supply/BAT0/power_now: 0 [µW]

(2) A units scheme -- charge = C = Ah
This is what Coreboot provides for your T440p:

/sys/class/power_supply/BAT0/charge_full_design: 5.616.000 [µAh]
/sys/class/power_supply/BAT0/charge_full: 4.561.000 [µAh]
/sys/class/power_supply/BAT0/charge_now: 3.210.000 [µAh]
/sys/class/power_supply/BAT0/current_now: 2.088.000 [µA]

But your values are not plausible: the 6-cell with 57 Wh has a nominal voltage of ~11,5V resulting in ~4,96 Ah not 5,61 Ah.

The correct kernel response would be in your case:
/sys/class/power_supply/BAT0/energy_full_design: 56.160.000 [µWh]

So my conclusion based on your values is that Coreboot provides the battery values to the kernel in the wrong scheme.

TLP accepts only the W units scheme for ThinkPads and thus doesn't display the A units sysfiles. Including a workaround in TLP for Coreboot wouldn't make sense, because the A unit values provided are incorrect anyway.

@linrunner
Copy link
Owner

Btw: you don't need acpi_call in conjunction with kernel 6.0 because it supports charge thresholds and recalibration natively. Unfortunately Coreboot does not support recalibration (refer to #626).

@linrunner
Copy link
Owner

linrunner commented Nov 9, 2022

@karstenes : Any news on this? Did you file a Coreboot bug yet?

@karstenes
Copy link
Author

@linrunner sorry, been busy and forgot to. Anything else you want in the bug report other than what has been discussed?

@linrunner
Copy link
Owner

linrunner commented Nov 10, 2022

@karstenes My key points are all in #657 (comment)

@linrunner
Copy link
Owner

linrunner commented Jan 9, 2023

@karstenes Anything new to report here?

@karstenes
Copy link
Author

@karstenes Anything new to report here?

Nothing as of now. The coreboot issue has not been responded to as of now.

@linrunner
Copy link
Owner

Do you mind posting the link to the coreboot issue here?

@karstenes
Copy link
Author

Coreboot Issue

@linrunner
Copy link
Owner

@diabl0w
Copy link

diabl0w commented Aug 6, 2023

apparently this was addressed some time ago? https://review.coreboot.org/c/coreboot/+/23178
maybe there was a breaking change or maybe this is different than what is being described
see also: https://www.coreboot.org/Board:lenovo/x200#Recalibrate_batteries

@linrunner
Copy link
Owner

linrunner commented Aug 7, 2023

@diabl0w The world does not only consist of charge thresholds. If you look at the output of @karstenes, you will see that the battery as well as the charge thresholds are recognized and thus should work.

/sys/class/power_supply/BAT0/charge_control_start_threshold = 0 [%]
/sys/class/power_supply/BAT0/charge_control_end_threshold = 100 [%]

The issue here is that false battery readings are provided by Coreboot. See my exhaustive explanation.

If you are affected by this, please contribute by updating the coreboot bug or creating your own. Just asking here does not bring us closer to a solution.

@linrunner linrunner changed the title TLP does not detect T440p Battery [T440p Coreboot] False battery data Aug 7, 2023
@linrunner linrunner changed the title [T440p Coreboot] False battery data [T440p Coreboot] False battery readings Aug 7, 2023
@diabl0w
Copy link

diabl0w commented Aug 16, 2023

@diabl0w The world does not only consist of charge thresholds. If you look at the output of @karstenes, you will see that the battery as well as the charge thresholds are recognized and thus should work.

/sys/class/power_supply/BAT0/charge_control_start_threshold = 0 [%]
/sys/class/power_supply/BAT0/charge_control_end_threshold = 100 [%]

The issue here is that false battery readings are provided by Coreboot. See my exhaustive explanation.

If you are affected by this, please contribute by updating the coreboot bug or creating your own. Just asking here does not bring us closer to a solution.

interesting, i guess i thought that was the only thing in the world. or more likely i made an innocemt error as i was naviagting a few things at once. who knows... anyways, i apologize for the false positive and i appreciate you looking into the feedback. i will go ahead and ping the current open coreboot report to being attention to it

@diabl0w
Copy link

diabl0w commented Aug 16, 2023

@linrunner, in fact, i do recall where that link came from. it came from Patrick Rudolph, a coreboot maintainer, who linked to that commit as a fix for the battery recalibration issue. perhaps he thinks thresholds are the only thing in the world as well: https://ticket.coreboot.org/issues/156

and, i obtained that link from the link i posted in my original comment which linked here

linrunner added a commit that referenced this issue Nov 9, 2023
…eboot

Rationale: Coreboot and Libreboot provide ACPI mW(h) data incorrectly
-  using sysfiles for mA(h)
-  one decimal place too little

Workaround: interpret values from /sys/class/power_supply/BAT[01]/
{charge_full*,charge_now,current_now} as mW(h)

References:
* #657
* https://thinkpad-forum.de/threads/projektvorstellung-tlp-%E2%80%93-linux-stromsparen.82441/page-118#post-2318015
* https://ticket.coreboot.org/issues/448
@linrunner
Copy link
Owner

@karstenes @ALL Since no one from the Core- or Libreboot developers feels obliged to fix the problem, I changed my mind and added a workaround for the missing battery data in tlp-stat -b.

Packages are available here: https://download.linrunner.de/packages/

Please show the output of

  sudo tlp-stat -s -b

@linrunner
Copy link
Owner

Initial testing happened here.

More tester are welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants