diff --git a/src/display_service.cpp b/src/display_service.cpp index 734b88e..b9910a8 100644 --- a/src/display_service.cpp +++ b/src/display_service.cpp @@ -743,18 +743,16 @@ void cleanupPartialWriteOnDisconnect(void) { #define AXP2101_REG_POWER_WAKEUP_CTL 0x26 #define AXP2101_REG_ADC_CHANNEL_CTRL 0x30 #define AXP2101_REG_ADC_DATA_BAT_VOL_H 0x34 -#define AXP2101_REG_ADC_DATA_VBUS_VOL_H 0x36 -#define AXP2101_REG_ADC_DATA_SYS_VOL_H 0x38 +#define AXP2101_REG_ADC_DATA_VBUS_VOL_H 0x38 +#define AXP2101_REG_ADC_DATA_SYS_VOL_H 0x3A #define AXP2101_REG_BAT_PERCENT_DATA 0xA4 #define AXP2101_REG_PWRON_STATUS 0x20 #define AXP2101_REG_IRQ_ENABLE1 0x40 #define AXP2101_REG_IRQ_ENABLE2 0x41 #define AXP2101_REG_IRQ_ENABLE3 0x42 -#define AXP2101_REG_IRQ_ENABLE4 0x43 -#define AXP2101_REG_IRQ_STATUS1 0x44 -#define AXP2101_REG_IRQ_STATUS2 0x45 -#define AXP2101_REG_IRQ_STATUS3 0x46 -#define AXP2101_REG_IRQ_STATUS4 0x47 +#define AXP2101_REG_IRQ_STATUS1 0x48 +#define AXP2101_REG_IRQ_STATUS2 0x49 +#define AXP2101_REG_IRQ_STATUS3 0x4A #define AXP2101_REG_LDO_ONOFF_CTRL1 0x91 #define FONT_BASE_WIDTH 8 #define FONT_BASE_HEIGHT 8 @@ -1355,7 +1353,7 @@ void readAXP2101Data(){ } Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); Wire.write(AXP2101_REG_ADC_CHANNEL_CTRL); - Wire.write(0xFF); + Wire.write(0x3F); // all ADC channels; bits 7:6 are reserved error = Wire.endTransmission(); delay(10); Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); @@ -1368,9 +1366,9 @@ void readAXP2101Data(){ uint8_t status2 = Wire.read(); od_log_debug("Power Status 1: 0x%02X", status1); od_log_debug("Power Status 2: 0x%02X", status2); - bool batteryPresent = (status1 & 0x20) != 0; - bool charging = (status1 & 0x04) != 0; - bool vbusPresent = (status1 & 0x08) != 0; + bool batteryPresent = (status1 & 0x08) != 0; + bool charging = ((status2 >> 5) & 0x03) == 0x01; + bool vbusPresent = (status1 & 0x20) != 0; od_log_debug("Battery Present: %s", batteryPresent ? "Yes" : "No"); od_log_debug("Charging: %s", charging ? "Yes" : "No"); od_log_debug("VBUS Present: %s", vbusPresent ? "Yes" : "No"); @@ -1394,8 +1392,8 @@ void readAXP2101Data(){ if(Wire.available() >= 2){ uint8_t batVolH = Wire.read(); uint8_t batVolL = Wire.read(); - uint16_t batVolRaw = ((uint16_t)batVolH << 4) | (batVolL & 0x0F); - float batVoltage = batVolRaw * 0.5; + uint16_t batVolRaw = ((uint16_t)(batVolH & 0x3F) << 8) | batVolL; + float batVoltage = batVolRaw; od_log_debug("Battery Voltage: %.1f mV (%.2f V)", batVoltage, batVoltage / 1000.0); } } @@ -1407,8 +1405,8 @@ void readAXP2101Data(){ if(Wire.available() >= 2){ uint8_t vbusVolH = Wire.read(); uint8_t vbusVolL = Wire.read(); - uint16_t vbusVolRaw = ((uint16_t)vbusVolH << 4) | (vbusVolL & 0x0F); - float vbusVoltage = vbusVolRaw * 1.7; + uint16_t vbusVolRaw = ((uint16_t)(vbusVolH & 0x3F) << 8) | vbusVolL; + float vbusVoltage = vbusVolRaw; od_log_debug("VBUS Voltage: %.1f mV (%.2f V)", vbusVoltage, vbusVoltage / 1000.0); } } @@ -1420,8 +1418,8 @@ void readAXP2101Data(){ if(Wire.available() >= 2){ uint8_t sysVolH = Wire.read(); uint8_t sysVolL = Wire.read(); - uint16_t sysVolRaw = ((uint16_t)sysVolH << 4) | (sysVolL & 0x0F); - float sysVoltage = sysVolRaw * 1.4; + uint16_t sysVolRaw = ((uint16_t)(sysVolH & 0x3F) << 8) | sysVolL; + float sysVoltage = sysVolRaw; od_log_debug("System Voltage: %.1f mV (%.2f V)", sysVoltage, sysVoltage / 1000.0); } } @@ -1495,12 +1493,6 @@ void powerDownAXP2101(){ Wire.write(0x00); error = Wire.endTransmission(); } - if(error == 0){ - Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); - Wire.write(AXP2101_REG_IRQ_ENABLE4); - Wire.write(0x00); - error = Wire.endTransmission(); - } if(error == 0){ Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); Wire.write(AXP2101_REG_IRQ_STATUS1); @@ -1516,13 +1508,7 @@ void powerDownAXP2101(){ if(error == 0){ Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); Wire.write(AXP2101_REG_IRQ_STATUS3); - Wire.write(0xFF); - error = Wire.endTransmission(); - } - if(error == 0){ - Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); - Wire.write(AXP2101_REG_IRQ_STATUS4); - Wire.write(0xFF); + Wire.write(0xDF); // bit 5 is reserved error = Wire.endTransmission(); if(error == 0){ od_log_debug("All IRQs disabled and status cleared"); @@ -1553,29 +1539,18 @@ void powerDownAXP2101(){ Wire.write(0x00); error = Wire.endTransmission(); if(error == 0){ - od_log_debug("BLDO1-2, CPUSLDO, DLDO1-2 disabled"); + od_log_debug("DLDO2 disabled"); } else { - od_log_error("ERROR: Failed to disable BLDO/CPUSLDO/DLDO rails"); + od_log_error("ERROR: Failed to disable DLDO2"); } Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); Wire.write(AXP2101_REG_LDO_ONOFF_CTRL0); - error = Wire.endTransmission(); - uint8_t aldoEnable = 0x00; - if(error == 0){ - Wire.requestFrom(AXP2101_SLAVE_ADDRESS, (uint8_t)1); - if(Wire.available()){ - aldoEnable = Wire.read(); - } - } - aldoEnable &= ~0x0F; - Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); - Wire.write(AXP2101_REG_LDO_ONOFF_CTRL0); - Wire.write(aldoEnable); + Wire.write(0x00); error = Wire.endTransmission(); if(error == 0){ - od_log_debug("ALDO1-4 disabled"); + od_log_debug("ALDO1-4, BLDO1-2, CPUSLDO, DLDO1 disabled"); } else { - od_log_error("ERROR: Failed to disable ALDO rails"); + od_log_error("ERROR: Failed to disable ALDO/BLDO/CPUSLDO/DLDO1 rails"); } Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); Wire.write(AXP2101_REG_POWER_WAKEUP_CTL); @@ -1596,7 +1571,6 @@ void powerDownAXP2101(){ if(!(wakeupCtrl & 0x10)) { wakeupCtrl |= 0x10; } - wakeupCtrl |= 0x80; Wire.beginTransmission(AXP2101_SLAVE_ADDRESS); Wire.write(AXP2101_REG_POWER_WAKEUP_CTL); Wire.write(wakeupCtrl); diff --git a/src/main.h b/src/main.h index 430b3c1..2c3342e 100644 --- a/src/main.h +++ b/src/main.h @@ -312,8 +312,8 @@ static constexpr uint32_t DEFAULT_IDLE_HOLD_MS = 10000; #define AXP2101_SLAVE_ADDRESS 0x34 #define AXP2101_REG_POWER_STATUS 0x00 -#define AXP2101_REG_POWER_ON_STATUS 0x01 -#define AXP2101_REG_POWER_OFF_STATUS 0x02 +#define AXP2101_REG_POWER_ON_STATUS 0x20 +#define AXP2101_REG_POWER_OFF_STATUS 0x21 #define AXP2101_REG_DC_ONOFF_DVM_CTRL 0x80 #define AXP2101_REG_LDO_ONOFF_CTRL0 0x90 #define AXP2101_REG_DC_VOL0_CTRL 0x82 // DCDC1 voltage @@ -323,22 +323,20 @@ static constexpr uint32_t DEFAULT_IDLE_HOLD_MS = 10000; #define AXP2101_REG_ADC_CHANNEL_CTRL 0x30 #define AXP2101_REG_ADC_DATA_BAT_VOL_H 0x34 #define AXP2101_REG_ADC_DATA_BAT_VOL_L 0x35 -#define AXP2101_REG_ADC_DATA_VBUS_VOL_H 0x36 -#define AXP2101_REG_ADC_DATA_VBUS_VOL_L 0x37 -#define AXP2101_REG_ADC_DATA_SYS_VOL_H 0x38 -#define AXP2101_REG_ADC_DATA_SYS_VOL_L 0x39 +#define AXP2101_REG_ADC_DATA_VBUS_VOL_H 0x38 +#define AXP2101_REG_ADC_DATA_VBUS_VOL_L 0x39 +#define AXP2101_REG_ADC_DATA_SYS_VOL_H 0x3A +#define AXP2101_REG_ADC_DATA_SYS_VOL_L 0x3B #define AXP2101_REG_BAT_PERCENT_DATA 0xA4 #define AXP2101_REG_PWRON_STATUS 0x20 #define AXP2101_REG_BAT_DETECTION_CTRL 0x68 // Battery detection control #define AXP2101_REG_IRQ_ENABLE1 0x40 // IRQ enable register 1 #define AXP2101_REG_IRQ_ENABLE2 0x41 // IRQ enable register 2 #define AXP2101_REG_IRQ_ENABLE3 0x42 // IRQ enable register 3 -#define AXP2101_REG_IRQ_ENABLE4 0x43 // IRQ enable register 4 -#define AXP2101_REG_IRQ_STATUS1 0x44 // IRQ status register 1 -#define AXP2101_REG_IRQ_STATUS2 0x45 // IRQ status register 2 -#define AXP2101_REG_IRQ_STATUS3 0x46 // IRQ status register 3 -#define AXP2101_REG_IRQ_STATUS4 0x47 // IRQ status register 4 -#define AXP2101_REG_LDO_ONOFF_CTRL1 0x91 // LDO control register 1 (BLDO1-2, CPUSLDO, DLDO1-2) +#define AXP2101_REG_IRQ_STATUS1 0x48 // IRQ status register 1 +#define AXP2101_REG_IRQ_STATUS2 0x49 // IRQ status register 2 +#define AXP2101_REG_IRQ_STATUS3 0x4A // IRQ status register 3 +#define AXP2101_REG_LDO_ONOFF_CTRL1 0x91 // LDO control register 1 (DLDO2) // The deferred-work flags that used to be defined here are now file-static in // main.cpp: nothing outside it reads them, and the two that other translation diff --git a/src/sensor_axp2101.h b/src/sensor_axp2101.h index ab86e59..ff34b44 100644 --- a/src/sensor_axp2101.h +++ b/src/sensor_axp2101.h @@ -28,15 +28,14 @@ #define AXP2101_REG_VBAT_H_ADDR 0x34u // VBAT high byte; read as 2-byte block with 0x35 // Datasheet reg 0x00: bit 3 = battery presence, bit 5 = VBUS presence. -// The uncalled readAXP2101Data() diagnostic in display_service.cpp reads these -// the other way round (0x20 for battery, 0x08 for VBUS). That is a pre-existing -// defect in unreachable code, flagged in the pull request and left unchanged. #define AXP2101_POWER_STATUS_BATT_PRESENT_BIT (1u << 3) #define AXP2101_POWER_STATUS_VBUS_PRESENT_BIT (1u << 5) -// Datasheet reg 0x30: bit 0 enables the VBAT-voltage ADC channel. Every other -// bit is a different channel or an unrelated ADC control and must be preserved. +// Datasheet reg 0x30: bit 0 enables the VBAT-voltage ADC channel. Bits 5:1 are +// the other channel enables and must be preserved. Bits 7:6 are reserved (R0, +// default 0) and are written as 0. #define AXP2101_ADC_ENABLE_VBAT_BIT (1u << 0) +#define AXP2101_ADC_CTRL_RESERVED_MASK 0xC0u // Post-enable settling window before the first VBAT read is meaningful. Only // applied when the caller has just flipped bit 0 from 0 to 1. @@ -63,8 +62,9 @@ static inline bool axp2101_vbus_present(uint8_t status_reg) { return (status_reg & AXP2101_POWER_STATUS_VBUS_PRESENT_BIT) != 0u; } -// Returns the value to write back to reg 0x30 so bit 0 is set, with every other -// bit preserved. `out_channel_changed` receives `true` iff the caller has just +// Returns the value to write back to reg 0x30 so bit 0 is set, with the other +// channel enables preserved and the reserved bits 7:6 written as 0. +// `out_channel_changed` receives `true` iff the caller has just // flipped the channel from disabled to enabled -- the only case where the // caller must allow AXP2101_ADC_SETTLING_MS before the first VBAT sample. static inline uint8_t axp2101_adc_enable_bit0(uint8_t current, bool* out_channel_changed) { @@ -72,12 +72,13 @@ static inline uint8_t axp2101_adc_enable_bit0(uint8_t current, bool* out_channel if (out_channel_changed != NULL) { *out_channel_changed = was_off; } - return (uint8_t)(current | AXP2101_ADC_ENABLE_VBAT_BIT); + return (uint8_t)((current & (uint8_t)~AXP2101_ADC_CTRL_RESERVED_MASK) | + AXP2101_ADC_ENABLE_VBAT_BIT); } -// VBAT decoding. Reg 0x34 holds the six valid high bits (bits 7:6 are unused -// and must be masked), reg 0x35 holds the full low byte. Result is millivolts -// at 1 mV per count. +// VBAT decoding. Reg 0x34 bits 5:0 hold the high bits of the reading (bits 7:6 +// are the RW ch_dbg_en[1:0] debug field and must be masked), reg 0x35 holds the +// full low byte. Result is millivolts at 1 mV per count. static inline uint16_t axp2101_decode_vbat_mv(uint8_t hi, uint8_t lo) { const uint16_t high_bits = (uint16_t)(hi & 0x3Fu); return (uint16_t)((high_bits << 8) | (uint16_t)lo); diff --git a/tools/test_sensor_axp2101.cpp b/tools/test_sensor_axp2101.cpp index 50207ab..9d4721d 100644 --- a/tools/test_sensor_axp2101.cpp +++ b/tools/test_sensor_axp2101.cpp @@ -8,8 +8,8 @@ // Like tools/test_link_owner.cpp, this is as much a written-down statement of // the intended semantics as it is a test. It covers: // -// - VBAT decoding with upper unused bits clear and set -// - ADC bit-0 enable while preserving every other bit +// - VBAT decoding with reg 0x34 bits 7:6 (ch_dbg_en) clear and set +// - ADC bit-0 enable preserving the other channel enables, reserved 7:6 as 0 // - Battery-present status set and clear // - Default address selection for zero and 0xFF // - Configured address preservation @@ -87,7 +87,7 @@ static int fake_read_vbat_mv(FakeBus& bus, uint8_t addr) { } int main(void) { - // ---- VBAT decoding with upper unused bits clear ------------------------- + // ---- VBAT decoding with reg 0x34 bits 7:6 clear -------------------------- { // 0x0F B8 -- the ESPHome-observed sample -- must decode to 4024 mV. check(axp2101_decode_vbat_mv(0x0F, 0xB8) == 4024, "vbat decode 0x0FB8 == 4024 mV"); @@ -97,19 +97,19 @@ int main(void) { check(axp2101_decode_vbat_mv(0x3F, 0xFF) == 16383, "vbat decode 0x3FFF == 16383 mV"); } - // ---- VBAT decoding with upper unused bits SET --------------------------- + // ---- VBAT decoding with reg 0x34 bits 7:6 SET ---------------------------- { - // Bits 7:6 of reg 0x34 are unused; the decoder must mask them off so - // spurious garbage in that field cannot inflate the reading. + // Bits 7:6 of reg 0x34 are the ch_dbg_en debug field, not part of the + // reading; the decoder must mask them off so they cannot inflate it. check(axp2101_decode_vbat_mv(0xC0, 0x00) == 0, - "vbat unused bits 0xC0 masked -> 0 mV"); + "vbat ch_dbg_en bits 0xC0 masked -> 0 mV"); check(axp2101_decode_vbat_mv(0xCF, 0xB8) == 4024, - "vbat 0xCFB8 (unused bits set) decodes same as 0x0FB8"); + "vbat 0xCFB8 (ch_dbg_en set) decodes same as 0x0FB8"); check(axp2101_decode_vbat_mv(0xFF, 0xFF) == 16383, "vbat 0xFFFF decodes as 0x3FFF -> 16383 mV"); } - // ---- ADC bit-0 enable while preserving every other bit ------------------ + // ---- ADC bit-0 enable, preserving the other channel enables --------------- { // From all-zero: bit 0 flips 0->1, no other bit changes. bool changed = false; @@ -117,10 +117,11 @@ int main(void) { check(next == 0x01, "adc enable from 0x00 -> 0x01"); check(changed, "adc enable from 0x00 flags channel change"); - // From every other bit set: bit 0 flips 0->1, all other bits preserved. + // From every other bit set: bit 0 flips 0->1, bits 5..1 preserved, + // reserved bits 7:6 written as 0. changed = false; next = axp2101_adc_enable_bit0(0xFE, &changed); - check(next == 0xFF, "adc enable from 0xFE preserves 7..1, sets bit 0 -> 0xFF"); + check(next == 0x3F, "adc enable from 0xFE preserves 5..1, clears 7:6, sets bit 0 -> 0x3F"); check(changed, "adc enable from 0xFE flags channel change"); // Already enabled (0x01): value unchanged, no channel-change flag. @@ -129,10 +130,10 @@ int main(void) { check(next == 0x01, "adc enable from 0x01 stays 0x01"); check(!changed, "adc enable from 0x01 does not flag channel change"); - // Already enabled with siblings set: value unchanged, no flag. + // Already enabled with siblings set: no flag; reserved bit 7 written as 0. changed = true; next = axp2101_adc_enable_bit0(0xA5, &changed); - check(next == 0xA5, "adc enable from 0xA5 stays 0xA5 (bit 0 already set)"); + check(next == 0x25, "adc enable from 0xA5 keeps 5..0, clears reserved 7 -> 0x25"); check(!changed, "adc enable from 0xA5 does not flag channel change"); // NULL out-pointer must be tolerated.