You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Correct invalid AXP2101 register addresses and fields (follow-up to #155) #170
PR #155 flagged two follow-ups: the uncalled readAXP2101Data() diagnostic, and the AXP2101 handling in powerDownAXP2101(). Checking every AXP2101 register access against the datasheet found both share one cause: register maps and conventions carried over from other X-Powers PMICs. This issue corrects the addresses and fields and removes accesses that have no AXP2101 register or bit to land on.
This change is for register correctness only. It intentionally does not change the ALDO3 shutdown policy, so the post-shutdown I²C failure seen with #155's reader remains; that is a separate follow-up.
The Waveshare ESP32-S3-PhotoPainter is the only board in the opendisplay.org catalog with an AXP2101.
Problem
Checked against the AXP2101 datasheet (PDF page numbers) at upstream 3716925.
Shutdown (powerDownAXP2101(), runs at every panel shutdown):
IRQ status is cleared at the wrong addresses. It writes 0xFF to 0x44–0x46. The AXP2101's IRQ status registers are 0x48–0x4A (p.43–44), which are never written.
It writes a fourth IRQ-enable register (0x43) and a fourth IRQ-status register (0x47) that the AXP2101 does not have. It has three of each (0x40–0x42, 0x48–0x4A).
These writes are acknowledged on the bus, so the code logs All IRQs disabled and status cleared while the real status registers are not written. (Seen in captures on the PhotoPainter.)
REG26 reserved bit 7 is set (|= 0x80); bits 7:5 are reserved (p.37).
REG91 is logged as disabling "BLDO1-2, CPUSLDO, DLDO1-2", but REG91 holds only the DLDO2 enable (p.53). The BLDO1, BLDO2, CPUSLDO and DLDO1 enables are REG90 bits 4–7 (p.53), which the shutdown preserves, so those outputs are never turned off as the log states.
Battery reader (axp2101_adc_enable_bit0(), from #155, live): when enabling the VBAT ADC channel it writes REG30's reserved bits 7:6 back as read, instead of as 0 (p.39).
Diagnostic (readAXP2101Data(), uncalled):
VBUS is read from 0x36, which is the TS pin; VBUS is 0x38/0x39 (p.40–41).
VSYS is read from 0x38, which is VBUS; VSYS is 0x3A/0x3B (p.41).
All three voltages use a 12-bit decode with 0.5/1.7/1.4 mV steps (AXP192 conventions); the AXP2101 uses 14 bits, (H & 0x3F) << 8 | L, at 1 mV per count (p.40–41).
It writes 0xFF to REG30, enabling every channel and setting reserved bits 7:6 (p.39).
Battery presence and VBUS presence are swapped (REG00 bits 3 and 5, p.31).
REG00 bit 2 ("Battery in Active Mode") is logged as "Charging"; charge state is REG01 bits 6:5 (p.31).
Constants (src/main.h, unused): a duplicate AXP2101 block left behind when fd0d73a moved the AXP2101 functions to display_service.cpp. It carries VBUS/SYS at 0x36/0x38, POWER_ON_STATUS/POWER_OFF_STATUS at 0x01/0x02 (0x02 does not exist on the AXP2101), the same IRQ map as the shutdown path, and a REG91 comment naming rails it does not control.
Why the corrections are correct. Each register address and field follows the AXP2101 datasheet (page numbers above). ESPHome test-bed work on the PhotoPainter reads REG00 bits 3 and 5, REG01 bits 6:5, VBAT at 0x34/0x35 and VBUS at 0x38/0x39 with the 14-bit decode, REG30 bit 2, and battery percentage at 0xA4, and the datasheet matched the hardware on every one of them. VSYS (0x3A/0x3B), the IRQ status registers and REG26 could not be exercised on hardware here; those corrections follow the same datasheet, which has proven reliable on every register we could test.
Why 0x43 and 0x47 are removed, and where they came from. The AXP2101 has three IRQ-enable registers (0x40–0x42) and three IRQ-status registers (0x48–0x4A); 0x43–0x47 are not in its register map. I traced the firmware's four-enable, four-status layout to its sources. Its names and count (IRQ_ENABLE1–4, IRQ_STATUS1–4) match Allwinner's AXP8191 PMU definitions. Its status addresses (0x44–0x47) match the AXP192, where REG43 ("IRQ enable 4") enables N_OE, VBUS-session and APS low-voltage IRQs that the AXP2101 does not have. The diagnostic's 12-bit decode and 1.7/1.4 mV steps are AXP192 conventions too. So this is vestigial code from older PMIC support: on the AXP2101 these writes reach no register and cannot do what they were written to do. They are also non-erroring, because the bus acknowledges them and nothing reports a failure. That is how they went unnoticed.
Unchanged: the REG26 sleep/wake sequence apart from bit 7 (it matches the datasheet's Figure 6-2, with GPIO21 driving the IRQ-pin wake; checked against the datasheet, not hardware-tested), DCDC1 and ALDO3/ALDO4 voltage and enable setup, the IRQ enables 0x40–0x42, the REG80 rail disable, REG30 = 0x00 at shutdown, battery percentage (0xA4), power-on status (0x20), and #155's battery-present and VBAT reads.
Proposed change
Shutdown:
Clear IRQ status at 0x48–0x4A, writing 0xDF to 0x4A because its bit 5 is reserved.
Stop writing 0x43 and 0x47.
Stop setting REG26 bit 7.
Write REG90 = 0x00. This is a deliberate runtime behavior change: it newly disables BLDO1, BLDO2, CPUSLDO and DLDO1, which were previously preserved even though the shutdown log claimed they were disabled. The datasheet establishes where those enables live; turning them off follows the intent stated by the existing shutdown code. They are unconnected on the PhotoPainter, the board tested. Other AXP2101 hardware could observe the difference.
Make the two rail-disable logs state what each write did: DLDO2 disabled (REG91) and ALDO1-4, BLDO1-2, CPUSLDO, DLDO1 disabled (REG90).
Battery reader: write REG30 bits 7:6 as 0 when enabling the VBAT channel (defensive; live code).
Diagnostic: correct its registers, bit masks, and decodes in place. Write REG30 = 0x3F (all ADC channels, as it intended, without the reserved bits). It stays uncalled.
Constants: correct the main.h values to the datasheet, and remove the two IRQ constants that have no AXP2101 register.
The diagnostic and main.h changes do not affect the production binary's control flow. They keep retained AXP2101 code and duplicate definitions from preserving a second, incorrect register map that could be reused later.
Scope boundaries
Each of these will follow in its own issue and pull request:
Post-shutdown I²C lifecycle.powerDownAXP2101() still disables ALDO3. On the PhotoPainter this makes the shared I²C bus unavailable, so the battery reader added by Report battery voltage from a configured AXP2101 #155 fails after panel shutdown. A separate follow-up will keep ALDO3 enabled. This issue intentionally leaves that policy unchanged, including ALDO2 and REG80's control bits 5/6.
Fail-closed read/modify/write, the hardcoded 0x34 address and ambient bus, and when power state is committed. These need new code, so they will follow separately from this register-correctness change.
Validation
Implemented as commit 0fd1cd0 on upstream 3716925:
Report battery voltage from a configured AXP2101 #155's 49 host checks pass under -fsanitize=undefined,address -Wall -Wextra -Werror, with two expectations updated for the reserved-bit change. They cover the battery helper only.
Locally built all 12 environments from .github/firmware-targets.json, including esp32-wrover-e-N4R8, in production and diagnostic profiles, with no new warnings. RAM unchanged; flash smaller on every target.
Observed during boot-to-shutdown validation on the Waveshare ESP32-S3-PhotoPainter, four-state cycle (upstream, upstream + diagnostic call, fix + diagnostic call, fix):
the corrected shutdown completed without an I²C error and logged the expected lines (IRQ status was not read back);
the diagnostic-only commit read battery/VBUS presence, charge state, and VBAT (4171 mV against Report battery voltage from a configured AXP2101 #155's reader at 4169 mV) correctly. After a power-on reset it read VBUS and VSYS as 0 mV despite its existing 10 ms delay; the cause was not investigated. With the channels already enabled, the corrected addresses read VBUS 5191 mV and VSYS 5170 mV;
production builds followed the same boot, refresh, shutdown, and BLE-startup flow as upstream.
Seeed XIAO ESP32-C3 and Seeed EE04 (no AXP2101 configured): same control flow in every state, showing no regression on those boards.
Not tested: deep-sleep wake, BLE transfers, register readback, long-term behavior.
AI disclosure: researched and drafted with AI assistance (GitHub Copilot and other AI harnesses), with a human reviewing every stage. All hardware observations were made on physical devices by the contributor, and the raw serial, build, and flash logs are retained.
Context
PR #155 flagged two follow-ups: the uncalled
readAXP2101Data()diagnostic, and the AXP2101 handling inpowerDownAXP2101(). Checking every AXP2101 register access against the datasheet found both share one cause: register maps and conventions carried over from other X-Powers PMICs. This issue corrects the addresses and fields and removes accesses that have no AXP2101 register or bit to land on.This change is for register correctness only. It intentionally does not change the ALDO3 shutdown policy, so the post-shutdown I²C failure seen with #155's reader remains; that is a separate follow-up.
The Waveshare ESP32-S3-PhotoPainter is the only board in the opendisplay.org catalog with an AXP2101.
Problem
Checked against the AXP2101 datasheet (PDF page numbers) at upstream
3716925.Shutdown (
powerDownAXP2101(), runs at every panel shutdown):0xFFto0x44–0x46. The AXP2101's IRQ status registers are0x48–0x4A(p.43–44), which are never written.0x43) and a fourth IRQ-status register (0x47) that the AXP2101 does not have. It has three of each (0x40–0x42,0x48–0x4A).All IRQs disabled and status clearedwhile the real status registers are not written. (Seen in captures on the PhotoPainter.)|= 0x80); bits 7:5 are reserved (p.37).Battery reader (
axp2101_adc_enable_bit0(), from #155, live): when enabling the VBAT ADC channel it writes REG30's reserved bits 7:6 back as read, instead of as0(p.39).Diagnostic (
readAXP2101Data(), uncalled):0x36, which is the TS pin; VBUS is0x38/0x39(p.40–41).0x38, which is VBUS; VSYS is0x3A/0x3B(p.41).(H & 0x3F) << 8 | L, at 1 mV per count (p.40–41).0xFFto REG30, enabling every channel and setting reserved bits 7:6 (p.39).Constants (
src/main.h, unused): a duplicate AXP2101 block left behind whenfd0d73amoved the AXP2101 functions todisplay_service.cpp. It carries VBUS/SYS at0x36/0x38,POWER_ON_STATUS/POWER_OFF_STATUSat0x01/0x02(0x02does not exist on the AXP2101), the same IRQ map as the shutdown path, and a REG91 comment naming rails it does not control.Why the corrections are correct. Each register address and field follows the AXP2101 datasheet (page numbers above). ESPHome test-bed work on the PhotoPainter reads REG00 bits 3 and 5, REG01 bits 6:5, VBAT at
0x34/0x35and VBUS at0x38/0x39with the 14-bit decode, REG30 bit 2, and battery percentage at0xA4, and the datasheet matched the hardware on every one of them. VSYS (0x3A/0x3B), the IRQ status registers and REG26 could not be exercised on hardware here; those corrections follow the same datasheet, which has proven reliable on every register we could test.Why
0x43and0x47are removed, and where they came from. The AXP2101 has three IRQ-enable registers (0x40–0x42) and three IRQ-status registers (0x48–0x4A);0x43–0x47are not in its register map. I traced the firmware's four-enable, four-status layout to its sources. Its names and count (IRQ_ENABLE1–4,IRQ_STATUS1–4) match Allwinner's AXP8191 PMU definitions. Its status addresses (0x44–0x47) match the AXP192, where REG43 ("IRQ enable 4") enables N_OE, VBUS-session and APS low-voltage IRQs that the AXP2101 does not have. The diagnostic's 12-bit decode and 1.7/1.4 mV steps are AXP192 conventions too. So this is vestigial code from older PMIC support: on the AXP2101 these writes reach no register and cannot do what they were written to do. They are also non-erroring, because the bus acknowledges them and nothing reports a failure. That is how they went unnoticed.Unchanged: the REG26 sleep/wake sequence apart from bit 7 (it matches the datasheet's Figure 6-2, with GPIO21 driving the IRQ-pin wake; checked against the datasheet, not hardware-tested), DCDC1 and ALDO3/ALDO4 voltage and enable setup, the IRQ enables
0x40–0x42, the REG80 rail disable, REG30 =0x00at shutdown, battery percentage (0xA4), power-on status (0x20), and #155's battery-present and VBAT reads.Proposed change
0x48–0x4A, writing0xDFto0x4Abecause its bit 5 is reserved.0x43and0x47.0x00. This is a deliberate runtime behavior change: it newly disables BLDO1, BLDO2, CPUSLDO and DLDO1, which were previously preserved even though the shutdown log claimed they were disabled. The datasheet establishes where those enables live; turning them off follows the intent stated by the existing shutdown code. They are unconnected on the PhotoPainter, the board tested. Other AXP2101 hardware could observe the difference.DLDO2 disabled(REG91) andALDO1-4, BLDO1-2, CPUSLDO, DLDO1 disabled(REG90).0when enabling the VBAT channel (defensive; live code).0x3F(all ADC channels, as it intended, without the reserved bits). It stays uncalled.main.hvalues to the datasheet, and remove the two IRQ constants that have no AXP2101 register.The diagnostic and
main.hchanges do not affect the production binary's control flow. They keep retained AXP2101 code and duplicate definitions from preserving a second, incorrect register map that could be reused later.Scope boundaries
Each of these will follow in its own issue and pull request:
powerDownAXP2101()still disables ALDO3. On the PhotoPainter this makes the shared I²C bus unavailable, so the battery reader added by Report battery voltage from a configured AXP2101 #155 fails after panel shutdown. A separate follow-up will keep ALDO3 enabled. This issue intentionally leaves that policy unchanged, including ALDO2 and REG80's control bits 5/6.0x34address and ambient bus, and when power state is committed. These need new code, so they will follow separately from this register-correctness change.Validation
Implemented as commit
0fd1cd0on upstream3716925:-fsanitize=undefined,address -Wall -Wextra -Werror, with two expectations updated for the reserved-bit change. They cover the battery helper only..github/firmware-targets.json, includingesp32-wrover-e-N4R8, in production and diagnostic profiles, with no new warnings. RAM unchanged; flash smaller on every target.AI disclosure: researched and drafted with AI assistance (GitHub Copilot and other AI harnesses), with a human reviewing every stage. All hardware observations were made on physical devices by the contributor, and the raw serial, build, and flash logs are retained.