bat.d: add support for AppleSMC found on Apple Silicon devices #685
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Been using this code since December but I kept forgetting to upstream it 🙈 The only thing I have really tested is the charge threshold, so it may contain bugs in the rest of the code. |
|
Hi, thank you very much for this. I have merged it here for now: https://github.com/linrunner/TLP/tree/pr/685-applesmc Some questions:
|
|
@linrunner sorry for the delay, I forgot to reply 🤦
#define MIN_CHARGE_START 50
#define MAX_CHARGE_START 100
[...]
case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD:
if (val->intval < MIN_CHARGE_START || val->intval > MAX_CHARGE_START)
return -EINVAL;
outputAlso, marcan recently posted something about the EC firmware only supports 80% or 100% and that he might rewrite the threshold code to map to these two values so that the kernel doesn't have to be involved, which means the threshold would work even when the computer is suspended or even off, with the downside that it wouldn't support other values anymore, but these other values don't necessarily provide much anyway. |
|
Found For this PR, I guess the difference is simply that the |
|
(1) I suppose all those who use Linux on Apple Silicon already use the Asahi Linux kernel. I can therefore imagine to merge your PR. What I don't like to do to myself anymore are external kernel modules as a prerequisite. I'm tired of the ones need by ThinkPads. (2) OK (3) Fair enough :-) (4) Interesting. More devices than I thought. PCIe too.
The question is whether we should let the user configure the start threshold at all, or whether we should calculate it from the stop threshold instead. |
|
@linrunner do you think TLP could be reworked to enumerate power interfaces, instead of having to copy a boilerplate file just to change a couple of values in it? That way it will be automatically be compatible with all future devices that follow the spec :) |
|
@1ace The generic specification - i.e. charge_control_start/stop_threshold - is undoubtedly a great advance. Unfortunately, the user learns nothing from it about allowed value ranges and special cases of the respective hardware. In fact, there are a lot of variants, see https://linrunner.de/tlp/settings/bc-vendors.html. I deliberately chose the design with the vendor-specific plugins to spare the users frustrating trial and error which values really work. An approach to handle as many vendors as possible in a generic plugin with countless case distinctions was also discarded for reliability reasons. I would be happy to finish writing the plugin and then ask you to test it. |
linrunner
added a commit
that referenced
this pull request
Oct 7, 2023
* Requires applesmc kernel module * Start and stop threshold * Force discharge not supported * Requires Apple Silicon SoC (Macbook Mx) * Requires Apple firmware 13.0 or newer * Requires Asahi Linux kernel 6.3 or newer * charge_control_end_threshold: - Valid values: 80, 100 - Default value: 100 * charge_control_start_threshold: - Valid values: 75, 95 - Default value: 95 - Constraint: hardware/kernel enforces start = stop - 5 - Silently ignore start threshold if not meeting constraint Credits: * #685 Reference: * AsahiLinux/linux@107ed86 * https://social.treehouse.systems/@marcan/110219038891519815
|
@1ace I finally found the time to revise your plugin. It's now available in the main branch. May I ask you to test? |
linrunner
added a commit
that referenced
this pull request
Oct 10, 2023
* Requires applesmc kernel module * Start and stop threshold * Force discharge not supported * Requires Apple Silicon SoC (Macbook Mx) * Requires Apple firmware 13.0 or newer * Requires Asahi Linux kernel 6.3 or newer * charge_control_end_threshold: - Valid values: 80, 100 - Default value: 100 * charge_control_start_threshold: - Valid values: 75, 95 - Default value: 95 - Constraint: hardware/kernel enforces start = stop - 5 - Silently ignore start threshold if not meeting constraint Credits: * #685 Reference: * AsahiLinux/linux@107ed86 * https://social.treehouse.systems/@marcan/110219038891519815
linrunner
added a commit
that referenced
this pull request
Oct 13, 2023
* Requires applesmc kernel module * Start and stop threshold * Force discharge not supported * Requires Apple Silicon SoC (Macbook Mx) * Requires Apple firmware 13.0 or newer * Requires Asahi Linux kernel 6.3 or newer * charge_control_end_threshold: - Valid values: 80, 100 - Default value: 100 * charge_control_start_threshold: - Valid values: 75, 95 - Default value: 95 - Constraint: hardware/kernel enforces start = stop - 5 - Silently ignore start threshold if not meeting constraint Credits: * #685 Reference: * AsahiLinux/linux@107ed86 * https://social.treehouse.systems/@marcan/110219038891519815
linrunner
added a commit
that referenced
this pull request
Nov 10, 2023
* Requires applesmc kernel module * Start and stop threshold * Force discharge not supported * Requires Apple Silicon SoC (Macbook Mx) * Requires Apple firmware 13.0 or newer * Requires Asahi Linux kernel 6.3 or newer * charge_control_end_threshold: - Valid values: 80, 100 - Default value: 100 * charge_control_start_threshold: - Valid values: 75, 95 - Default value: 95 - Constraint: hardware/kernel enforces start = stop - 5 - Silently ignore start threshold if not meeting constraint Credits: * #685 Reference: * AsahiLinux/linux@107ed86 * https://social.treehouse.systems/@marcan/110219038891519815
|
@ALL: Testing Instructions Install
Test Install the tool clitest from your distribution's official repository 1.1 Execute the test script and post all output Manual 2.1 Execute the following test cases in a terminal and post all output Prerquisite: Connect the charger. 2.2 Run on battery until charge level falls below 70% 2.3 Connect the charger, check if charging stops at 80%. 2.4 Revert to stop threshold 100% 2.5 Check if charging commences and reaches 100% |
linrunner
added a commit
that referenced
this pull request
Nov 18, 2023
* Requires applesmc kernel module * Start and stop threshold * Force discharge not supported * Requires Apple Silicon SoC (Macbook Mx) * Requires Apple firmware 13.0 or newer * Requires Asahi Linux kernel 6.3 or newer * charge_control_end_threshold: - Valid values: 80, 100 - Default value: 100 * charge_control_start_threshold: - Valid values: 75, 95 - Default value: 95 - Constraint: hardware/kernel enforces start = stop - 5 - Silently ignore start threshold if not meeting constraint Credits: * #685 Reference: * AsahiLinux/linux@107ed86 * https://social.treehouse.systems/@marcan/110219038891519815
|
@ALL: Code improved once more. I'm still looking for a volunteer who owns the hardware and wants to test it! |
I am on M1 laptop, gentoo linux. I have no experience with TLP, but I can try it ;) UPD: oops, I found test instructions above. I'll post output today. |
test resultsLMK if i did something wrong. also, my laptop was full charged (acpi shows Full, 99%) when i launched tests, and with charger cable connected, as stated in test requirements |
|
@ZerdoX-x Thank you. I can see that the code and the test script need to be revised. The 1st battery is called Stay tuned please. |
|
Sure. Just ping me and I'll provide results ASAP 👀 |
|
@ZerdoX-x I noticed a concept problem during troubleshooting, which is why it took a while. Please update the plugin and test again with the (also updated) test script. Show the output of |
test results 2tlp-stat output83% capacity 😭 |
|
@ZerdoX-x you didn't test with the latest, corrected plugin nor with the corrected script. In the meantime I had to edit history. Use IMPORTANT: please test with the charger connected. The script is designed for AC mode only. too.
DMI data about the laptop is missing. May I see the output of |
|
Oops. I didn't save the file properly and forgot to connect the charger. Now only teststlp-stat -s -b
I do not have a dmi folder This is all folders I have in /sys/classMost of them - afair this is intended for now in asahi linux, I guess AsahiLinux/m1n1#378 should fix it? |
|
Very well. In #34, the expected result in the test script is incorrect. Otherwise perfect. Thank you very much for your contribution so far. What remains is to test whether the thresholds really stop the charging process. I suggest you carry out the manual test from above. I will add a "(not available)" for the missing DMI information. |
linrunner
added a commit
that referenced
this pull request
May 7, 2024
|
UPD: nvm, i'll report a bit later about manual tests. Set this in tlp.conf. The rest is defaults But now seems like I cannot charge above 75% Shouldn't my laptop get charged to 80 first? |
|
@ZerdoX-x This may well be correct, as the definition of the start charge threshold is "battery charge level below which charging will begin when connecting the charger". Have you tried discharging to 74% first? Incidentally, the output of any third-party tool is completely unsuitable for diagnosis. I always need to have |
|
Yes. Seems like everything is working for me correctly. I was confused about laptop not charging but it was due to some software compiling on machine at that time |
|
Can I still see the current state, please? |
charge oncharge offI thought it would stop charging at 80, drop to 75 and start charging till 80 again |
|
Oh my bad. It should be like that. So is everything alright? |
|
Yes. Thanks for testing. |
|
I wonder how this PR compares with https://github.com/AsahiLinux/asahi-scripts , which recently introduced udev rules and a systemd service for battery charge (see macsmc-battery folder). If I had to run a daemon, I'd prefer to just use tlp, which is battle-proven and I'm more familiar with. Cc @marcan @jannau |
|
@dkwo oh well, yet another conflict ... :-( |
|
I don't see a conflict. AsahiLinux/asahi-scripts#49 simply monitors |
|
@jannau Two different tools writing the same kernel tunable means unpredictable results = conflict. There can only be one. |
|
I don't see much risk. Udev will in most cases just write to it on system boot probably before other daemons managing are running. In situations where that's not the case it will only write the last written value so nothing should notice this. |
|
@jannau If incorrect values suddenly appear in the charge thresholds you never know for sure which tool caused it. I don't think that's desirable for support (to put it mildly). Fortunately, the value range is quite small :-). Btw: Which file contains the configuration for the udev rule? |
|
The beta test is complete, and TLP 1.7.0 is now available. Have fun! A huge thank you to all of our wonderful contributors, testers, and bug reporters! 👍 |
linrunner
added
released
fixed/completed
and removed
needs testing
work in progress
labels
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #665