From ea40a796936da0203a60ca1bd6966a05210bc233 Mon Sep 17 00:00:00 2001 From: ainyan03 Date: Thu, 20 Aug 2026 05:42:51 +0000 Subject: [PATCH 1/2] Stop a low charge voltage request from raising the charge voltage Both setChargeVoltage implementations subtracted a bias from the argument before comparing it against their step table. The argument is unsigned, so a request below the lowest step wrapped around and then clamped to the opposite end of the table: on the AXP192 a request for 4.0V configured 4.36V, and on the AXP2101 the same request selected the reserved code 0. Asking for a gentler charge voltage gave a harsher one. The AXP2101 table also carried a 4.6V step that the part does not have - its constant-voltage register goes up to 4.4V - so a request that reached that entry wrapped the index back onto the reserved code as well. Compare in millivolts against the real steps instead, take the highest step that does not exceed the request, and fall back to the lowest step when the request is under all of them. Every request inside the supported range keeps its previous result. --- src/utility/power/AXP192_Class.cpp | 20 +++++++++----------- src/utility/power/AXP2101_Class.cpp | 28 ++++++++++++---------------- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/src/utility/power/AXP192_Class.cpp b/src/utility/power/AXP192_Class.cpp index 80299d6..06b2080 100644 --- a/src/utility/power/AXP192_Class.cpp +++ b/src/utility/power/AXP192_Class.cpp @@ -169,17 +169,15 @@ namespace m5 } void AXP192_Class::setChargeVoltage(std::uint16_t max_mV) - { - max_mV = (max_mV / 10) - 410; - if (max_mV > 436 - 410) { max_mV = 436 - 410; } - static constexpr std::uint8_t table[] = - { 415 - 410 /// 4150mV - , 420 - 410 /// 4200mV - , 436 - 410 /// 4360mV - , 255 - }; - size_t i = 0; - while (table[i] <= max_mV) { ++i; } + { /// reg 0x33 bit6:5 selects the target voltage. Compare in millivolts: + /// the earlier form subtracted a bias first, which underflowed the + /// unsigned argument for anything below the lowest step and then clamped + /// to the highest one - a request for less charge voltage produced more. + static constexpr std::uint16_t table[] = { 4100, 4150, 4200, 4360 }; + size_t i = (sizeof(table) / sizeof(table[0])) - 1; + /// pick the highest step that does not exceed the request, and the lowest + /// step when the request is under all of them. + while (i && table[i] > max_mV) { --i; } std::uint8_t val = 0; if (readRegister(0x33, &val, 1)) diff --git a/src/utility/power/AXP2101_Class.cpp b/src/utility/power/AXP2101_Class.cpp index 30a072e..4db15b8 100644 --- a/src/utility/power/AXP2101_Class.cpp +++ b/src/utility/power/AXP2101_Class.cpp @@ -125,22 +125,18 @@ namespace m5 } void AXP2101_Class::setChargeVoltage(std::uint16_t max_mV) - { - max_mV = (max_mV / 10) - 400; - if (max_mV > 460 - 400) { max_mV = 460 - 400; } - static constexpr std::uint8_t table[] = - { 410 - 400 /// 4100mV - , 420 - 400 /// 4200mV - , 435 - 400 /// 4350mV - , 440 - 400 /// 4400mV - , 460 - 400 /// 4600mV - , 255 - }; - size_t i = 0; - while (table[i] <= max_mV) { ++i; } - - if (++i >= 0b110) { i = 0; } - writeRegister8(0x64, i); + { /// reg 0x64 selects the constant-voltage target, 1 = 4.0V through 5 = 4.4V. + /// There is no step above 4.4V; the earlier table carried one, and a + /// request that reached it wrapped the index back to the reserved 0. + /// Requests below the lowest step underflowed the unsigned argument and + /// landed on the same 0. + static constexpr std::uint16_t table[] = { 4000, 4100, 4200, 4350, 4400 }; + size_t i = (sizeof(table) / sizeof(table[0])) - 1; + /// pick the highest step that does not exceed the request, and the lowest + /// step when the request is under all of them. + while (i && table[i] > max_mV) { --i; } + + writeRegister8(0x64, static_cast(i + 1)); } std::int8_t AXP2101_Class::getBatteryLevel(void) From c8b71115003c218b3ec3b083642073b33ced7bbc Mon Sep 17 00:00:00 2001 From: ainyan03 Date: Thu, 20 Aug 2026 05:42:51 +0000 Subject: [PATCH 2/2] Say what the charge voltage steps are, and why 4.6V is not one Review pointed out that the AXP2101 constant-voltage register is documented differently across datasheet revisions: the current ones reserve code 0 while an early one gave it to 4.6V. The change had asserted flatly that no step above 4.4V exists, which only holds for the revisions available now. State what every revision agrees on instead, and give the reason for holding a high request at 4.4V: no battery this library runs on charges to 4.6V, so the code whose meaning depends on the silicon is worth avoiding. Both headers now carry the supported steps and what happens on either side of them, which nothing stated before. --- src/utility/power/AXP192_Class.hpp | 5 +++++ src/utility/power/AXP2101_Class.cpp | 17 ++++++++++++----- src/utility/power/AXP2101_Class.hpp | 5 +++++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/utility/power/AXP192_Class.hpp b/src/utility/power/AXP192_Class.hpp index e423565..dd1691f 100644 --- a/src/utility/power/AXP192_Class.hpp +++ b/src/utility/power/AXP192_Class.hpp @@ -34,6 +34,11 @@ namespace m5 /// set battery charge voltage /// @param max_mV milli volt. (4100 - 4360). + /// set the constant-voltage charge target. + /// @param max_mV the highest step at or below this value is selected. + /// Supported steps are 4100 / 4150 / 4200 / 4360 mV; a request under the + /// lowest step selects that step, and one above the highest selects the + /// highest. void setChargeVoltage(std::uint16_t max_mV); /// Get whether the battery is currently charging or not. diff --git a/src/utility/power/AXP2101_Class.cpp b/src/utility/power/AXP2101_Class.cpp index 4db15b8..c46931b 100644 --- a/src/utility/power/AXP2101_Class.cpp +++ b/src/utility/power/AXP2101_Class.cpp @@ -125,11 +125,18 @@ namespace m5 } void AXP2101_Class::setChargeVoltage(std::uint16_t max_mV) - { /// reg 0x64 selects the constant-voltage target, 1 = 4.0V through 5 = 4.4V. - /// There is no step above 4.4V; the earlier table carried one, and a - /// request that reached it wrapped the index back to the reserved 0. - /// Requests below the lowest step underflowed the unsigned argument and - /// landed on the same 0. + { /// reg 0x64 selects the constant-voltage target: 1 = 4.0V through 5 = 4.4V, + /// with 0 reserved. An early revision of the datasheet also documented a + /// 4.6V setting, which later revisions dropped; nothing this library runs + /// on carries a cell that charges to 4.6V, so a request that high is held + /// at the highest step both revisions agree on rather than sent to a code + /// whose meaning depends on the silicon. + /// + /// The earlier form subtracted a bias from the argument before comparing. + /// The argument is unsigned, so a request under the lowest step wrapped + /// around and selected code 0, as did a request that reached the 4.6V + /// entry - asking for a gentler charge voltage produced either a reserved + /// code or the highest voltage, depending on the silicon. static constexpr std::uint16_t table[] = { 4000, 4100, 4200, 4350, 4400 }; size_t i = (sizeof(table) / sizeof(table[0])) - 1; /// pick the highest step that does not exceed the request, and the lowest diff --git a/src/utility/power/AXP2101_Class.hpp b/src/utility/power/AXP2101_Class.hpp index faf3736..e5aa66d 100644 --- a/src/utility/power/AXP2101_Class.hpp +++ b/src/utility/power/AXP2101_Class.hpp @@ -86,6 +86,11 @@ namespace m5 /// set battery charge voltage /// @param max_mV milli volt. (4100 - 4360). + /// set the constant-voltage charge target. + /// @param max_mV the highest step at or below this value is selected. + /// Supported steps are 4000 / 4100 / 4200 / 4350 / 4400 mV; a request under + /// the lowest step selects that step, and one above the highest selects + /// the highest. void setChargeVoltage(std::uint16_t max_mV); /// @return -1:discharge / 0:standby / 1:charge