Open chutz opened 4 months ago
Hi, thanks.
I'll take a look at this soon. I'm just busy at the moment with necessary changes that have resulted from testing the macbook plugin. Stay tuned please.
Sorry for the overly long delay. See my review for required changes.
@chutz due to your lack of response, i have created my own implementation -> https://github.com/linrunner/TLP/tree/feature/bat.d/framework
@Simerax can you test it?
FYI I wrote a generic ChromeOS EC charge control driver [0] While it won't probe by default on Framework (because the upstream CrOS EC commands are overridden by their downstream APIs) I hope to unify the Framework APIs so that driver will be used on Framework.
It would be great if TLP could be compatible with that driver. (I expect it to make it into 6.11)
[0] https://lore.kernel.org/lkml/20240616-cros_ec-charge-control-v4-0-74d649a9117d@weissschuh.net/T/#t
@t-8ch I'm very pleased to hear from you on this topic. I would have emailed you otherwise.
In principle, I would prefer to support your generic driver instead of _frameworklaptop, which is currently out-of-tree. I had my share of trouble with ThinkPad out-of-tree modules, I don't necessarily need that again.
However, the connections are not yet clear to me:
Does your driver provide the sysfiles chargecontrol{start,end}_threshold itself or is _frameworklaptop still needed for this?
Are both start and stop thresholds supported? _frameworklaptop provides stop only, but this article by the author suggests that the hardware can do both: https://www.howett.net/posts/2021-12-framework-ec/#3e03---charge-limit-control
What is the exact name your generated module, i.e. under /sys/module/?
The driver will (most likely) be called cros_charge-control
.
It supports start/stop thresholds and charge_behaviour
through the standard sysfs attributes.
On older hardware it only supports charge_behaviour
but no thresholds.
AFAIK this case is new and may need some changes to TLP.
OTOH it allows simulating thresholds in userspace by toggling between force-discharge
and auto
.
The hardware has all of the features, as proven by the upstream commands being supported and functional. For some reason Framework reinvented their own downstream commands.
I plan on writing some patches for the EC firmware to unify both command sets, so the vanilla CrOS EC driver can work on the Framework without user intervention.
@chutz due to your lack of response, i have created my own implementation -> https://github.com/linrunner/TLP/tree/feature/bat.d/framework
@Simerax can you test it?
Yes! Just checked out your branch and will now test it. :) I will probably comment my findings tomorrow or sometime over the weekend :)
Just realized it needs the out of tree module..would it maybe make more sense to wait for t-8ch driver in the kernel instead of now "quickly" getting the support with an out of tree module?
@Simerax you're knocking down open doors. However, I wouldn't mind if you could test the current plugin with the out-of-tree module.
@linrunner Okay I tested it with the current master of the framework_laptop
module.
Battery Care detects the plugin and the STOP_CHARGE_THRESH_BAT1
works. I'm guessing its expected that START_CHARGE_THRESH_BAT1
does not work right now since you said the framework_laptop
module does not support that.
Are there any other settings I should test? Do you want/need any stat outputs?
@Simerax Thanks so far.
I need the output of
sudo tlp-stat -s -b
Please also run the automatic test script and show the output: https://github.com/linrunner/TLP/blob/feature/bat.d/framework/unit-tests/charge-thresholds_framework
It requires the tool clitest to be installed. Should be available as a package in most distributions.
Clitest doesn't seem to be available on void. Is this the tool and repository you are referring to? https://github.com/aureliojargas/clitest
Output of tlp-stat -s -b
+++ System Info
System = Framework AA Laptop
BIOS = 03.17
OS Release = Void Linux
Kernel = 6.6.34_1 #1 SMP PREEMPT_DYNAMIC Mon Jun 17 11:50:08 UTC 2024 x86_64
/proc/cmdline = BOOT_IMAGE=/boot/vmlinuz-6.6.34_1 root=/dev/mapper/vvm-root ro loglevel=4 rd.lvm.vg=vvm rd.luks.uuid=XXX apparmor=1 security=apparmor
Init system = sysvinit
Boot mode = UEFI
Suspend mode = [s2idle] deep
+++ TLP Status
State = enabled
RDW state = not installed
Last run = 19:16:04, 4 sec(s) ago
Mode = AC
Power source = AC
+++ Battery Care
Plugin: framework
Supported features: charge threshold
Driver usage:
* natacpi (framework_laptop) = active (charge thresholds)
Parameter value ranges:
* STOP_CHARGE_THRESH_BAT0/1: 1..100(default)
+++ Battery Status: BAT1
/sys/class/power_supply/BAT1/manufacturer = NVT
/sys/class/power_supply/BAT1/model_name = Framewo
/sys/class/power_supply/BAT1/cycle_count = 101
/sys/class/power_supply/BAT1/charge_full_design = 35720 [mAh]
/sys/class/power_supply/BAT1/charge_full = 31970 [mAh]
/sys/class/power_supply/BAT1/charge_now = 14530 [mAh]
/sys/class/power_supply/BAT1/current_now = 1 [mA]
/sys/class/power_supply/BAT1/status = Charging
/sys/class/power_supply/BAT1/charge_control_end_threshold = 45 [%]
Charge = 45.4 [%]
Capacity = 89.5 [%]
Is this the tool and repository you are referring to? https://github.com/aureliojargas/clitest
Yes.
sorry for the delay. Output of clitest:
#1 # +++ Framework laptops +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
#2 #
#3 # --- tlp start
#4 sudo tlp start -- START_CHARGE_THRESH_BAT1= STOP_CHARGE_THRESH_BAT1= START_CHARGE_THRESH_BAT0= STOP_CHARGE_THRESH_BAT0=
Passwort:
#5 sudo tlp start -- START_CHARGE_THRESH_BAT1="60" STOP_CHARGE_THRESH_BAT1="100" START_CHARGE_THRESH_BAT0= STOP_CHARGE_THRESH_BAT0=
#6 sudo tlp start -- START_CHARGE_THRESH_BAT1="na" STOP_CHARGE_THRESH_BAT1="0" START_CHARGE_THRESH_BAT0= STOP_CHARGE_THRESH_BAT0=
#7 sudo tlp start -- START_CHARGE_THRESH_BAT1="0" STOP_CHARGE_THRESH_BAT1="101" START_CHARGE_THRESH_BAT$
#8 sudo tlp start -- START_CHARGE_THRESH_BAT1="na" STOP_CHARGE_THRESH_BAT1="86" START_CHARGE_THRESH_BAT0= STOP_CHARGE_THRESH_BAT0=
#9 sudo tlp start -- START_CHARGE_THRESH_BAT1="DEF" STOP_CHARGE_THRESH_BAT1="DEF" START_CHARGE_THRESH_BAT0= STOP_CHARGE_THRESH_BAT0=
#10 sudo tlp start -- NATACPI_ENABLE=0 START_CHARGE_THRESH_BAT1="DEF" STOP_CHARGE_THRESH_BAT1="DEF"
#11 #
#12 # --- tlp setcharge w/o arguments
#13 sudo tlp setcharge -- START_CHARGE_THRESH_BAT1="60" STOP_CHARGE_THRESH_BAT1="100"
#14 sudo tlp setcharge -- START_CHARGE_THRESH_BAT1="na" STOP_CHARGE_THRESH_BAT1="0"
#15 sudo tlp setcharge -- START_CHARGE_THRESH_BAT1="0" STOP_CHARGE_THRESH_BAT1="101"
#16 sudo tlp setcharge -- START_CHARGE_THRESH_BAT1="ABCDE" STOP_CHARGE_THRESH_BAT1="XYZZY"
#17 sudo tlp setcharge -- START_CHARGE_THRESH_BAT1="na" STOP_CHARGE_THRESH_BAT1="100"
#18 sudo tlp setcharge -- START_CHARGE_THRESH_BAT1="na" STOP_CHARGE_THRESH_BAT1="86"
#19 sudo tlp setcharge -- START_CHARGE_THRESH_BAT1="na" STOP_CHARGE_THRESH_BAT1="80"
#20 sudo tlp setcharge -- START_CHARGE_THRESH_BAT1="na" STOP_CHARGE_THRESH_BAT1="80"
#21 sudo tlp setcharge -- START_CHARGE_THRESH_BAT1="DEF" STOP_CHARGE_THRESH_BAT1="DEF"
#22 sudo tlp setcharge -- NATACPI_ENABLE=0 START_CHARGE_THRESH_BAT1="DEF" STOP_CHARGE_THRESH_BAT1="DEF"
#23 #
#24 # --- tlp setcharge w/ arguments
#25 sudo tlp setcharge 60 100 BAT1
#26 sudo tlp setcharge 0 0 BAT1
#27 sudo tlp setcharge 0 101
#28 sudo tlp setcharge ABCDE 0
#29 sudo tlp setcharge 0 XYZZY
#30 sudo tlp setcharge 97 100
#31 sudo tlp setcharge 0 66
#32 sudo tlp setcharge 0 60
#33 sudo tlp setcharge 0 60 -- X_THRESH_SIMULATE_START="35" X_THRESH_SIMULATE_STOP="100"
#34 sudo tlp setcharge 0 60
#35 sudo tlp setcharge DEF DEF
#36 sudo tlp setcharge BAT0
#37 sudo tlp setcharge 0 3 BAT0
#38 sudo tlp setcharge XYZZY ABCDE BAT0
#39 #
#40 # --- tlp-stat
#41 sudo tlp-stat -b -- | grep "charge_control_end_threshold"
#42 sudo tlp-stat -b -- X_THRESH_SIMULATE_READERR=1 | grep "charge_control_end_threshold"
#43 #
#44 # --- Reset test machine to configured thresholds
#45 sudo tlp setcharge BAT1 > /dev/null 2>&1
#46 #
OK: 46 of 46 tests passed
Wow. It actually fitted perfectly on the first try. Thank you very much.
Let's just save this in the back of our minds. Now I'll wait for the cros_charge-control
module be merged in mainline. I would be pleased if you could also test the corresponding plugin.
This requires the currently out of tree module for the framework laptop available at https://github.com/DHowett/framework-laptop-kmod
This started with the template and added support for the framework laptop.