bat.d/framework: Add initial support for framework laptop
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.
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/[email protected]/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 framework_laptop, 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 charge_control_{start,end}_threshold itself or is framework_laptop still needed for this?
-
Are both start and stop thresholds supported? framework_laptop 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.
Sorry about disappearing for awhile here.
I have updated this MR to handle the cros_charge-control module that has been merged into mainline starting at 6.11. With this module, the cros_charge_control module needs the probe_with_fwk_charge_control option passed to it for this to work. I added documentation for this to the comments at the start of the file, I am not sure if there is a better way to do this.
@chutz : thanks so far. I would prefer your PR to be based on my branch https://github.com/linrunner/TLP/tree/feature/bat.d/framework . It already includes my changes to the TEMPLATE for 1.7.
But before you do that, we need to clarify some points:
- Is your detection intentionally not
cros_charge-controlspecific?
case “$(read_sysf $BATDRV_FRAMEWORK_MD)” in
FRA*) return 0;;
The condition is also met by the framework_laptop module, refer to the above output from @Simerax. framework_laptop does not support a start threshold though.
What about checking for /sys/module/cros_charge_control?
-
On the one hand the code is detecting /sys/class/power_supply/BAT1, on the other hand the loops over BAT[01] are still present. Adjusting the detection to
cros_charge-controlwould resolve this contradiction. -
Threshold defaults don't make sense to me:
_bt_def_start=50
_bt_def_stop=60
They are used for tlp fullcharge, so at least it should be _bt_def_stop=100. What are the hardware (e.g. firmware) defaults if no thresholds are set?
@t-8ch
OTOH it allows simulating thresholds in userspace by toggling between
force-dischargeandauto.
Really? No way TLP will implement that :-D.
If the cros_charge_ctl module is used, should the TLP functionality still be called "framework"?
Really? No way TLP will implement that :-D.
Not saying that it's reasonable, only possible.