“This release modifies the battery charge limit functionality to add a 5% float range. This allows us to reduce the number of microcycles on the battery when the CPU turbos.”
Will the rest of the FL13 lineup see this fix coming down the line?
Ideally, it would be, IMO, better if the float range can be user-defined. (The “Start Charging Threshold”…mentioned / requested in May 2022)
ac = 1
chg_voltage = 17600mV
chg_current = 2740mA
chg_input_current = 4500mA
batt_state_of_charge = 44%
No battery temperature display, and the chg_voltage is incorrect, should be 17800mV, that’s not flipping but still related to EC and it also has negative impact on the battery.
Neither do thermalget
sensor warn high halt fan_off fan_max name
0 343 353 393 313 343 local_f75303@4d
1 343 353 393 319 327 cpu_f75303@4d
2 343 353 393 401 401 ddr_f75303@4d
3 381 388 400 376 378 cpu@4c
(all temps in degrees Kelvin)
What am I missing, does the EC code make it show the temperature?
I took a look at the common/charge_state_v2.c code in the Framework EC firmware Github defining the charge_request function.
Most of the logic is over my head, but I think the charge_control calls set the buck/boost voltage level to some small offset above whatever the EC thinks the current battery voltage is. If the CPU spikes cause a momentary battery voltage sag, that would explain why the charger voltage shown by ec-syslog gets lower with each function call.
Here is what I think is the relevant section of code.
#ifdef CONFIG_CHARGER_NARROW_VDC
current = 0;
/*
* With NVDC charger, keep VSYS voltage higher than battery,
* otherwise the BGATE FET body diode would conduct and
* discharge the battery.
*/
voltage = charger_closest_voltage(
curr.batt.voltage + charger_get_info()->voltage_step);
/* If the battery is full, request the max voltage. */
if (is_full)
voltage = battery_get_info()->voltage_max;
/* And handle dead battery case */
voltage = MAX(voltage, battery_get_info()->voltage_normal);
#else
voltage = current = 0;
#endif
I also found this page with mainboard schematics for different FW13 laptops. Looking at the schematic for the AMD 7040 series laptops specifically, it appears they use a model ISL9241 buck/boost converter. Apologies if the existence of these schematics were already common knowledge, they were new to me.
Well, that is what the EC firmware source code says, but the question is, is that the way it is supposed to work if it was doing NVDC correctly, according to the standards and appropriate datasheets. How much should one really trust that code snippet is doing it correctly.
As it turns out, that code snippet is doing it wrongly or it has got a bug in it.
“curr.batt.voltage” is the current battery voltage, as measured by the Battery BMS.
The ISL9241 datasheets say that should use the charger VBAT measured value.
While those two values might be similar, I think it is fair to assume that the charger’s own ADCs that measure VSYS and VBAT will be calibrated so that a comparison of the two makes sense, rather than using the value from the battery BMS, that has unknown calibration accuracy.
Now, what I have just said here, if fixed by a one-line fix, might actually fix this problem for everyone, as it is an obvious bug in the EC firmware currently.
As I have said before, I don’t really think the EC firmware code is particularly high quality, with yet another example of a bug here.
According to git blame, that line of code was written/last changed in 2015-10-28.
So, its fair to say, that is probably before FW existed!!!
I am not sure the command and the logic is even correct.
One of the application notes for the ISL9241 says otherwise:
https://www.renesas.com/en/document/apn/r16an0002-isl9241-operation-modes-application-note?r=507701
In the section:
2.2 Transitioning from Bypass to NVDC or NVDC + CHRG
“… The ISL9241 output and system are ramped down close to the battery voltage so that
charging can be enabled safely any time…”
But. In step 3: “Set MaxsysV = VADP (from ADC or PD controller) - 2V. For > 20V adapter, set to max of 18.304V”. So this starts the MaxSysVoltage close to the adapter voltage so bypass can be switched off, and it then is ramped down in step 5.
For step 5 it has “Set MaxSysVoltage = Battery full charge voltage after 1ms”
So, “battery voltage” here means “Battery full charge voltage”, not the actual current battery voltage or VBAT right now.
So, these instructions basically start the MaxsysV at 20V - 2 = 18V.
These instructions then suggest it ramps down to what the battery full charge voltage is (read from the battery) is, which is about 17.6V.
This is not the same as the code above that is ramping it down the the current charge level + 8mV, which is commonly about 15.8V.
There is nothing in the ISL9241 datasheets that suggest NVDC mode should have MaxsysV set to current_batteryV + 8mV.
So, one could say that the datasheets are also ambiguous in this area.
FYI, all the ISL9241 datasheets are here:
I would be interested to hear what other people think should be happening (ignoring the EC Firmware source code), and just basing it on the datasheets.