pfeerick / pine64-scripts

Utility scripts for the pine64
GNU General Public License v3.0
39 stars 7 forks source link

RfE: Add battery capacity to rpimonitor install script #2

Closed ThomasKaiser closed 7 years ago

ThomasKaiser commented 7 years ago

Just a suggestion since I won't spend any more time on this... here's an RPi-Monitor template that also logs/displays battery information with legacy kernel: https://pastebin.com/JvxthdKS

TODO:

ThomasKaiser commented 7 years ago

in case it needs further calibration

Well, it does: /sys/class/power_supply/battery/capacity remaining on 7 since hours:

bildschirmfoto 2017-04-26 um 22 18 50
pfeerick commented 7 years ago

Firstly, thanks for make the inaugural issue post for this repo ;) It was initially where I was privately hosting the few scraps I don't want to lose between re-images, but then I thought I'd open it up because there isn't anything that creative here ;)

Now, that looks very interesting (and useful!)... and I will certainly look into in the next few weeks. I was hoping to work out how rpimonitor collects the different data points in order to add something like that, so it's great to have a template to work from. Thank you!

The capacity inaccuracy is to be expected because of a serious stuff up in the DTS settings for the PMU. It is being initialised with a battery capacity of 4800mah instead of 8000/10000mah, hence it can't even get close in doing the SoC measurement when coulomb counting. And I'm not sure if it's self-learning mechanism is turned on, meaning it can then self-calibrate to the true capacity of the battery once it is in the ballpark. The initial capacity initialisation value issue will be fixed soon, as the pertinent configuration parameters have finally been identified and the necessary changes discussed,

ThomasKaiser commented 7 years ago

Monitoring at least /sys/class/power_supply/battery/voltage_now should help understanding potential capacity calculation/calibration issues. And then I remembered that we went through all of this already some time ago (though with A20/AXP209 instead):

ThomasKaiser commented 7 years ago

New template: http://sprunge.us/iOFc

bildschirmfoto 2017-04-27 um 14 21 36
ThomasKaiser commented 7 years ago

And after exchanging the battery DT entries with those from Kamil now the Pinebook charges with ~15W and not 7W as before 👍

2520c2520
<           pmu_battery_rdc = <0x79>;
---
>           pmu_battery_rdc = <0x6e>;
2522c2522
<           pmu_runtime_chgcur = <0x708>;
---
>           pmu_runtime_chgcur = <0x1c2>;
2532c2532
<           pmu_chgled_func = <0x1>;
---
>           pmu_chgled_func = <0x0>;
2540,2564c2540,2564
<           pmu_bat_para7 = <0x1>;
<           pmu_bat_para8 = <0x1>;
<           pmu_bat_para9 = <0x2>;
<           pmu_bat_para10 = <0x2>;
<           pmu_bat_para11 = <0x3>;
<           pmu_bat_para12 = <0xa>;
<           pmu_bat_para13 = <0x12>;
<           pmu_bat_para14 = <0x1e>;
<           pmu_bat_para15 = <0x2c>;
<           pmu_bat_para16 = <0x32>;
<           pmu_bat_para17 = <0x36>;
<           pmu_bat_para18 = <0x3a>;
<           pmu_bat_para19 = <0x3d>;
<           pmu_bat_para20 = <0x42>;
<           pmu_bat_para21 = <0x48>;
<           pmu_bat_para22 = <0x4d>;
<           pmu_bat_para23 = <0x52>;
<           pmu_bat_para24 = <0x55>;
<           pmu_bat_para25 = <0x5b>;
<           pmu_bat_para26 = <0x60>;
<           pmu_bat_para27 = <0x62>;
<           pmu_bat_para28 = <0x64>;
<           pmu_bat_para29 = <0x64>;
<           pmu_bat_para30 = <0x64>;
<           pmu_bat_para31 = <0x64>;
---
>           pmu_bat_para7 = <0x4>;
>           pmu_bat_para8 = <0x5>;
>           pmu_bat_para9 = <0x7>;
>           pmu_bat_para10 = <0x8>;
>           pmu_bat_para11 = <0xa>;
>           pmu_bat_para12 = <0xc>;
>           pmu_bat_para13 = <0x10>;
>           pmu_bat_para14 = <0x17>;
>           pmu_bat_para15 = <0x2a>;
>           pmu_bat_para16 = <0x2d>;
>           pmu_bat_para17 = <0x31>;
>           pmu_bat_para18 = <0x34>;
>           pmu_bat_para19 = <0x37>;
>           pmu_bat_para20 = <0x3b>;
>           pmu_bat_para21 = <0x40>;
>           pmu_bat_para22 = <0x45>;
>           pmu_bat_para23 = <0x49>;
>           pmu_bat_para24 = <0x4d>;
>           pmu_bat_para25 = <0x50>;
>           pmu_bat_para26 = <0x54>;
>           pmu_bat_para27 = <0x56>;
>           pmu_bat_para28 = <0x57>;
>           pmu_bat_para29 = <0x5c>;
>           pmu_bat_para30 = <0x5f>;
>           pmu_bat_para31 = <0x62>;
ThomasKaiser commented 7 years ago

New template: http://sprunge.us/TEVO

bildschirmfoto 2017-04-28 um 08 31 40
ThomasKaiser commented 7 years ago

Since I realized that reported SoC temperature increases while charging and I remembered that both PMIC and SoC are both attached via thermal pads to the same metal plate I thought let's monitor PMIC temperature too:

bildschirmfoto 2017-04-28 um 11 19 15

New template: http://sprunge.us/PZUQ

Currently this requires a stupid background script called from /etc/rc.local:

root@pinebook:~# cat /usr/local/bin/monitor-charging.sh
#!/bin/bash

while true ; do
    echo 1 >/sys/class/axppower/regdebug
    dmesg | tail -n40 | awk -F" " '/charger->ic_temp/ {print $4}' | head -n1 >/tmp/pmictemp
    # dmesg | tail -n40 | awk -F" " '/charger->bat_temp/ {print $4}' | head -n1 >/tmp/batterytemp
    sleep 15
done

It should be easy to patch BSP kernel to provide PMIC thermal readouts through sysfs but I've no idea where to report/ask (@ayufan-pine64 or @longsleep ?). And unfortunately battery temperature readout seems to be either broken or not implemented. It's always the same 30 that are also available through /sys/devices/virtual/thermal/thermal_zone1/temp and /sys/devices/platform/axp81x_board/axp81x-supplyer.47/power_supply/battery/temp.

pfeerick commented 7 years ago

Right, trying your earlier prior config, I also get the working battery info, so I'll roll that in soon. Question though... On the pine64, running the Armbian (not longsleep like I initially said) xenial ubuntu image, I don't get the rooomage info, and memory / storage info seems to be b0rked. Any ideas?

For example, this is the contents of memory.conf, and whilst the total amount of memory is working, neither are free nor avail, and they are all coming from /proc/meminfo !!! http://sprunge.us/EGSR

image

pfeerick commented 7 years ago

It should be easy to patch BSP kernel to provide PMIC thermal readouts through sysfs but I've no idea where to report/ask (@ayufan-pine64 or @longsleep ?). And unfortunately battery temperature readout seems to be either broken or not implemented. It's always the same 30 that are also available through /sys/devices/virtual/thermal/thermal_zone1/temp and /sys/devices/platform/axp81x_board/axp81x-supplyer.47/power_supply/battery/temp.

You'll probably have better luck with @ayufan-pine64, unless you have a complete pull request that just makes it work. @longsleep seems to prefer taking a sit back and maintain role now that the kernel works and does what he wants.

ThomasKaiser commented 7 years ago

I don't get the rooomage info, and memory / storage info seems to be b0rked. Any ideas?

roomage is only interesting if you experiment with throttling settings (obviously no one is doing) since you see what strategy currently applies (Allwinner's kernel allows either throttling or killing of CPU cores, then you see not a 4 at the end but lower numbers and that's also the reason why I added 'Active CPU cores' to the template to monitor such stuff). You should keep in mind that Armbian has a kernel patch to bring back killed CPU cores, IIRC longsleep implemented a shell helper script and it seems that what's now 'official Pinebook software support' simply ignores all of this stuff. To be honest, after visiting uk.pine64.xyz:9090 twice I'm really impressed by the level of ignorance the small 'Pine64 community' suffers from (ignoring most if not all of the stuff that has been solved long ago by much larger linux-sunxi community).

I never touched memory.conf since reporting of 'free' and 'available' memory is for clueless people only (RPi-Monitor originated in the Raspberry world). If you're concerned about free memory on a Linux/Unix/*BSD (implementing virtual memory) better visit http://www.linuxatemyram.com instead. Reporting of swap used is important and that works.

I'm soon going to play with zram soon and then I'll touch memory.conf the first time to be able to monitor efficiency of zram vs swap (looking at available memory). :)

pfeerick commented 7 years ago

Right, I've done some poking around, and I have rpi-monitor doing this stuff on the pinebook also. For the background script for the PMIC temperature, it's actually the /sys/class/axppower/axpdebug element that needs to have a 1 poked at it to trigger the debug info to dmesg, not sure what the regdebug one actually does.

For the dmesg | tail -n40 | awk -F" " '/charger->ic_temp/ {print $4}' | head -n1 >/tmp/pmictemp line... I'm not that comfortable with awk... do you have any suggestions as to what I do to get [ 2168.675260] charger->ic_temp = 51 processed properly... I changed the print $4 to $5 and it worked, so I realised it's the number of elements... but won't that fail once the system uptime goes beyond 9999 seconds?

And I presume I can use a sed type line to insert a reference to the script into /etc/rc.local before exit 0, but any pointers as to what it would look like. Is something like sed -i -e '$i /usr/local/bin/monitor-pmic.sh &' /etc/rc.local a safe enough option? Perhaps which a test to make sure the last line isn't blank instead of exit 0.

As far as memory, wasn't particularly bothered... just confused as to why it and the storage space readouts was broken when didn't appear to be a reason. I was wondering if perhaps it's that annoying thing where free reports differently on recent builds or is different between jessie and ubuntu... there was something different around there somewhere.

longsleep commented 7 years ago

You should keep in mind that Armbian has a kernel patch to bring back killed CPU cores, IIRC longsleep implemented a shell helper script and it seems that what's now 'official Pinebook software support' simply ignores all of this stuff.

I am going to merge the Kernel cpukeeper implementation eventually - just did not feel the need yet :)

longsleep commented 7 years ago

For the dmesg | tail -n40 | awk -F" " '/charger->ic_temp/ {print $4}' | head -n1 >/tmp/pmictemp line... I'm not that comfortable with awk... do you have any suggestions as to what I do to get [ 2168.675260] charger->ic_temp = 51 processed properly... I changed the print $4 to $5 and it worked,

That is terrible :)

ThomasKaiser commented 7 years ago

I am going to merge the Kernel cpukeeper implementation eventually - just did not feel the need yet :)

@longsleep I made the mistake twice within the last 24 hours to join http://irc.pine64.uk -- today someone reported ending up with only 3 CPU cores left so obviously whoever is responsible for tweaking DT managed to f*ck up cpu_budget_cool. In other words: it would really help Pinebook users if you implement this fix in the kernel.

That is terrible :)

Totally agree and it's just mad to fire up this debug stuff every few seconds. I still hope you or ayufan simply take this register value and put it at /sys/devices/virtual/thermal/thermal_zone2/temp so it can easily be monitored from user space 👍

BTW: After unloading the camera/video drivers I get the battery drain below 3W on Pinebook with blanked display: https://forum.armbian.com/index.php?/topic/4133-quick-pinebook-preview-review/#comment-30534

ThomasKaiser commented 7 years ago

but won't that fail once the system uptime goes beyond 9999 seconds?

You were totally right, an attempt that seems to work regardless of uptime might be:

#!/bin/bash
while true ; do
    echo 1 >/sys/class/axppower/axpdebug
    dmesg | tail -n40 | awk -F"= " '/charger->ic_temp/ {print $2}' | head -n1 >/tmp/pmictemp
    dmesg | tail -n40 | awk -F"= " '/power_sply/ {print $2}' | head -n1 >/tmp/powersupply
    sleep 15
done
pfeerick commented 7 years ago

Oh duh! That makes more sense now that I rather stupidly realised that F was specifying the field delimiter pattern. I had instead changed it to {print $NF}, but will use this since you've also added power_sply to the mix. Until now, I've managed to stay well away from regex and the likes of grep and awk as I have reverted to being a dumb user instead of a programmer :-O

I've started on a new branch for the rpimonitor for pinebook, and will incorporate your IP address fix there btw. I'll accept the PR if I can get it to change to the new branch and folder (pinebook stuff will go in a subfolder), and I'll move the pine64 stuff into a subfolder also so I can use the root for common scripts). It will be a day or two before I have time to finish putting this together, but it is looking good so far.

ThomasKaiser commented 7 years ago

Well, IMO it's wrong to do such differentiation in different branches or with different scripts since device differentiation should be done based on DT entries. Unfortunately upstream in every .dts currently used for all A64 Pine devices it reads:

model = "sun50iw1p1";

And that's what we get when we now read from /proc/device-tree/model. This can only be fixed 'upstream' by @longsleep or @ayufan-pine64 or whoever is responsible for the 'official' Pine OS builds.

ThomasKaiser commented 7 years ago

Otherwise in scripts you could do

case $(cat /proc/device-tree/model) in
    pinebook) # do this
    pine64) # do that
    sopine64) # do something else
esac
ThomasKaiser commented 7 years ago

FYI: https://github.com/armbian/u-boot-pine64-armbian/commit/c751ad4f6a3867da84a1c57b82b6484f24295470 (and both ayufan and longsleep agreed to do it the same way, now let's hope this doesn't break anything with BSP drivers -- I made just a few tests but that's not sufficient ;) )

pfeerick commented 7 years ago

Thanks for that... just have to wait for that to go live, then I won't have to have separate scripts. BTW, the branch is only whilst I work on developing it... might as well as called it testing as that is all it is, as the stuff in that branch isn't guaranteed to work at all (yet).

longsleep commented 7 years ago

And that's what we get when we now read from /proc/device-tree/model. This can only be fixed 'upstream' by @longsleep or @ayufan-pine64 or whoever is responsible for the 'official' Pine OS builds.

I implemented something different now. The model and compatible values are used as is form the device tree sources and should not be modified automatically.

instead, there will be the /chosen/pine64-model property, holding the U-Boot generated "pine64_model" value. See https://github.com/longsleep/u-boot-pine64/commit/cf0f2a7c4088131f35804e03c9a59d788c09246b for details and implementation.

ThomasKaiser commented 7 years ago

And from now on the ugly hack isn't necessary any more since @ayufan added the necessary bits to sysfs. Final template (for now ;) ): http://sprunge.us/KNcT

ThomasKaiser commented 7 years ago

And sent as a PR upstream: https://github.com/ayufan-pine64/linux-build/pull/2

ThomasKaiser commented 7 years ago

https://github.com/armbian/build/commit/9f60c423cee447ff2f315c17a2a382aa35575d9f

If you come up with ideas regarding those templates please inform me, I'll adopt useful changes in both Armbian repo and will update the /usr/local/sbin/install_rpi_monitor.sh script that's now part of Pinebook (and other?) community builds.