From dde2618a136ea6627ab223032183d37bd0bd869d Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 10:33:46 -0700 Subject: [PATCH 01/10] rtc: select 24-hour mode before a time write, and take no time in 12-hour mode ZephCore assumed 24-hour mode and never selected it. If other firmware had left a chip in 12-hour mode, the read masked hours with 0x3F and took any BCD 00-23: PM 1-3 (21h-23h) were restored as 21:00-23:00, eight hours ahead, and PM 4-12 gave no time. The 24-hour hours byte a save wrote meant another hour to the chip. A new optional twelve-hour-bit descriptor property, , names the bit that reads set in 12-hour mode: Control 2 12_24 (10h bit 1) on the RV3028 (Application Manual Rev 1.4, p. 24), and hours bit 6 on the DS3231 node, which also covers the DS1307 (DS3231 19-5170 Rev 10, p. 12; DS1307 p. 8). The PCF8563, RX8130CE and RX8900 count 24 hours only. While the bit reads set, or cannot be read, the boot read takes no time, and a bit outside the time block is cleared; the RV3028 then converts its Hours register itself (02h, p. 15). Each time write selects 24-hour mode first, inside the RV3028's BSF bracket; on the DS3231 and DS1307 the write's own 24-hour hours byte clears the bit. Zephyr's rtc_rv3028 writes Control 2 = 00h at init, and MeshCore (meshcore-dev/MeshCore#3421) clears 12_24 on read and selects 24-hour mode before a write. Fixes ptr727/liquidraver-ZephCore#51. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 53 ++++++++++++++++++- zephcore/adapters/clock/ZephyrRTCDiscover.h | 10 ++-- zephcore/boards/common/rtc-i2c.dtsi | 3 ++ .../boards/nrf52840/rak4631/board.overlay | 1 + .../dts/bindings/rtc/zephcore,rtc-i2c.yaml | 10 ++++ 5 files changed, 71 insertions(+), 6 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index 7f0b6b561..d43118f56 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -40,6 +40,8 @@ struct rtc_desc { uint8_t status_mask; /* "time unreliable" bit within status_reg */ const uint8_t *zero; /* 7 bytes of bits the data sheet shows as 0, or NULL */ bool week_one_hot; /* weekday as one bit per day, not 0-6 */ + uint8_t h12_reg; /* register of the 12-hour mode bit */ + uint8_t h12_mask; /* that bit, or 0 if the chip has no 12-hour mode */ #if RTC_RV3028_CFG const uint8_t *cfg; /* rv3028-eeprom-config triplets, or NULL */ uint8_t cfg_len; @@ -91,6 +93,12 @@ DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_CFG_ARRAY) DT_NODE_HAS_PROP(node, zero_mask), \ (RTC_ZERO_NAME(node)), (NULL)), \ .week_one_hot = DT_PROP(node, weekday_one_hot), \ + .h12_reg = COND_CODE_1( \ + DT_NODE_HAS_PROP(node, twelve_hour_bit), \ + (DT_PROP_BY_IDX(node, twelve_hour_bit, 0)), (0)), \ + .h12_mask = COND_CODE_1( \ + DT_NODE_HAS_PROP(node, twelve_hour_bit), \ + (DT_PROP_BY_IDX(node, twelve_hour_bit, 1)), (0)), \ RTC_CFG_FIELDS(node) \ .name = DT_NODE_FULL_NAME(node), \ }, @@ -264,6 +272,36 @@ static void rtc_clear_power_flag(const struct rtc_desc *d) } } +/* 1 if the chip counts hours in 12-hour mode, 0 if in 24-hour mode or it has + * none, -1 if a mode bit outside the time block could not be read. */ +static int rtc_12h(const struct rtc_desc *d, const uint8_t blk[7]) +{ + uint8_t v; + + if (d->h12_mask == 0) { + return 0; + } + if (d->h12_reg >= d->time_reg && d->h12_reg < d->time_reg + 7) { + return (blk[d->h12_reg - d->time_reg] & d->h12_mask) != 0; + } + if (i2c_reg_read_byte(d->bus, d->addr, d->h12_reg, &v) != 0) { + return -1; + } + return (v & d->h12_mask) != 0; +} + +/* Select 24-hour mode. A mode bit outside the time block is cleared, and the + * RV3028 then converts its Hours register itself (02h, p. 15). One inside it + * (DS3231/DS1307 hours bit 6) is cleared by the time write's hours byte. */ +static bool rtc_select_24h(const struct rtc_desc *d) +{ + if (d->h12_mask == 0 || + (d->h12_reg >= d->time_reg && d->h12_reg < d->time_reg + 7)) { + return true; + } + return i2c_reg_update_byte(d->bus, d->addr, d->h12_reg, d->h12_mask, 0) == 0; +} + #if RTC_RV3028_CFG /* RV3028 registers for reading and writing its configuration EEPROM. */ @@ -287,7 +325,8 @@ static void rtc_clear_power_flag(const struct rtc_desc *d) * year 2000, so the next boot takes no time from it. */ static bool rv3028_write_time(const struct rtc_desc *d, const uint8_t blk[7]) { - return i2c_reg_write_byte(d->bus, d->addr, d->time_reg + 6, 0x00) == 0 && + return rtc_select_24h(d) && + i2c_reg_write_byte(d->bus, d->addr, d->time_reg + 6, 0x00) == 0 && i2c_burst_write(d->bus, d->addr, d->time_reg, blk, 6) == 0 && i2c_reg_write_byte(d->bus, d->addr, d->time_reg + 6, blk[6]) == 0; } @@ -616,6 +655,15 @@ static bool rtc_probe(uint32_t *epoch_out) "next GPS/app/CLI sync", d->name); continue; } + int h12 = rtc_12h(d, blk); + + if (h12 != 0) { + (void)rtc_select_24h(d); + LOG_WRN("%s present, %s — clock will be set on the next " + "GPS/app/CLI sync", d->name, + h12 > 0 ? "12-hour mode" : "hour mode unreadable"); + continue; + } int flag = rtc_power_flag(d, blk); if (flag != 0) { @@ -701,7 +749,8 @@ void zephcore_rtc_save(uint32_t epoch) } } else #endif - if (i2c_burst_write(d->bus, d->addr, d->time_reg, blk, sizeof(blk)) != 0) { + if (!rtc_select_24h(d) || + i2c_burst_write(d->bus, d->addr, d->time_reg, blk, sizeof(blk)) != 0) { LOG_WRN("RTC %s: time write failed", d->name); return; } diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.h b/zephcore/adapters/clock/ZephyrRTCDiscover.h index c2d22aebe..a3334e0db 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.h +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.h @@ -35,8 +35,10 @@ extern "C" { * while the power-loss flag does not read as set (an unreadable flag counts * as not set), or, on the second read, all 0xFF. A failed second read never * rules a device out. A time is taken only from a clean read whose fields, - * hours and year are each in range (12-hour mode is not decoded). Returns - * false if none present or no trustworthy time is held. + * hours and year are each in range, and never while the descriptor's + * twelve-hour-bit reads set or cannot be read; a set bit outside the time + * block is then cleared. Returns false if none present or no trustworthy + * time is held. * * At a descriptor with rv3028-eeprom-config, the time is read once more * with the chip's backup switch flag (BSF) cleared before and checked @@ -57,8 +59,8 @@ extern "C" { bool zephcore_rtc_restore(uint32_t *epoch_out); /* - * Persist an authoritative epoch to the discovered RTC chip and clear its - * power-loss flag. No-op if no RTC was discovered. Safe to call often, but + * Persist an authoritative epoch to the discovered RTC chip, selecting + * 24-hour mode first, and clear its power-loss flag. No-op if no RTC was discovered. Safe to call often, but * intended only for real syncs (GPS/app/CLI), not per-packet clock nudges. * If restore adopted nothing but skipped a candidate whose first read was all * 0xFF, the first save probes again, once per boot, in the caller's context. diff --git a/zephcore/boards/common/rtc-i2c.dtsi b/zephcore/boards/common/rtc-i2c.dtsi index 2dabd133a..caff4dad1 100644 --- a/zephcore/boards/common/rtc-i2c.dtsi +++ b/zephcore/boards/common/rtc-i2c.dtsi @@ -39,6 +39,8 @@ rtc_ds3231: rtc-ds3231@68 { date-index = <4>; status-reg = <0x0F>; /* control/status */ status-mask = <0x80>; /* OSF — oscillator stopped */ + /* Hours bit 6, 12-hour mode: DS3231 (p. 12), DS1307 (p. 8) */ + twelve-hour-bit = <0x02 0x40>; /* Bits shown as 0 in all three parts: DS3231 (19-5170 Rev 10, Figure 1, * p. 11) and DS3232 (19-5337 Rev 5, Figure 1, p. 11), where 05h bit 7 is * Century, and DS1307 (Rev 3/15, Table 2, p. 8, "always reads back as @@ -65,6 +67,7 @@ rtc_rv3028: rtc-rv3028@52 { date-index = <4>; status-reg = <0x0e>; /* status */ status-mask = <0x01>; /* PORF — power-on reset */ + twelve-hour-bit = <0x10 0x02>; /* Control 2 12_24, p. 24 */ /* RV-3028-C7 Application Manual Rev. 1.4, 3.2, p. 12 */ zero-mask = [80 80 c0 f8 c0 e0 00]; }; diff --git a/zephcore/boards/nrf52840/rak4631/board.overlay b/zephcore/boards/nrf52840/rak4631/board.overlay index 02ef9ed06..ea9360899 100644 --- a/zephcore/boards/nrf52840/rak4631/board.overlay +++ b/zephcore/boards/nrf52840/rak4631/board.overlay @@ -228,6 +228,7 @@ date-index = <4>; status-reg = <0x0e>; /* status */ status-mask = <0x01>; /* PORF — power-on reset */ + twelve-hour-bit = <0x10 0x02>; /* Control 2 12_24, p. 24 */ /* RV-3028-C7 Application Manual Rev. 1.4, 3.2, p. 12 */ zero-mask = [80 80 c0 f8 c0 e0 00]; /* The RAK12002's backup is a supercapacitor. As the RV-3028-C7 diff --git a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml index efde76778..8a6c90727 100644 --- a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml +++ b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml @@ -62,6 +62,16 @@ properties: bit 6) rather than a number 0-6. Set it for a chip whose data sheet says so, such as the RX8130CE. + twelve-hour-bit: + type: array + description: > + : the bit that reads set while the chip counts hours + in 12-hour mode. Leave it out for a chip with no 12-hour mode. While + it reads set, or cannot be read, no time is taken from the chip, and + each time write selects 24-hour mode first: a bit outside the time + block is cleared, and one inside it is cleared by the write's own + hours byte. + rv3028-eeprom-config: type: uint8-array description: > From 52d83b9902c81be268a3fe56cde57778548cf658 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 10:47:29 -0700 Subject: [PATCH 02/10] rtc: clear the 12-hour bit only on the adopted chip, and never from a bad read Local review: the boot-time clear ran on any present candidate the probe reached, so a device at the RTC's address that ZephCore never adopts could be written. It now runs on the adopted chip only, as the RV3028 config store does. The clear reads the register and refuses a read of all 1s rather than writing it back, since that is a transfer cut short: on the RV3028 it would set every other Control 2 bit, and RESET always reads 0 there. twelve-hour-bit is a two-byte uint8-array with a length assert, so a malformed node fails the build. The binding now says the boot clears the bit, and a header line the first edit left unwrapped is rewrapped. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 24 ++++++++++++++++--- zephcore/adapters/clock/ZephyrRTCDiscover.h | 7 +++--- zephcore/boards/common/rtc-i2c.dtsi | 4 ++-- .../boards/nrf52840/rak4631/board.overlay | 2 +- .../dts/bindings/rtc/zephcore,rtc-i2c.yaml | 13 +++++----- 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index d43118f56..562258406 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -60,6 +60,13 @@ struct rtc_desc { DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_ZERO_ARRAY) +#define RTC_H12_CHECK(node) \ + IF_ENABLED(DT_NODE_HAS_PROP(node, twelve_hour_bit), \ + (BUILD_ASSERT(DT_PROP_LEN(node, twelve_hour_bit) == 2, \ + "twelve-hour-bit is [register mask]");)) + +DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_H12_CHECK) + #if RTC_RV3028_CFG #define RTC_CFG_NAME(node) _CONCAT(rtc_cfg_, DT_DEP_ORD(node)) @@ -292,14 +299,23 @@ static int rtc_12h(const struct rtc_desc *d, const uint8_t blk[7]) /* Select 24-hour mode. A mode bit outside the time block is cleared, and the * RV3028 then converts its Hours register itself (02h, p. 15). One inside it - * (DS3231/DS1307 hours bit 6) is cleared by the time write's hours byte. */ + * (DS3231/DS1307 hours bit 6) is cleared by the time write's hours byte. A + * read of all 1s, a transfer cut short, is not written back: on the RV3028 + * it would set every other Control 2 bit, and RESET always reads 0 there. */ static bool rtc_select_24h(const struct rtc_desc *d) { + uint8_t v; + if (d->h12_mask == 0 || (d->h12_reg >= d->time_reg && d->h12_reg < d->time_reg + 7)) { return true; } - return i2c_reg_update_byte(d->bus, d->addr, d->h12_reg, d->h12_mask, 0) == 0; + if (i2c_reg_read_byte(d->bus, d->addr, d->h12_reg, &v) != 0 || v == 0xFF) { + return false; + } + return !(v & d->h12_mask) || + i2c_reg_write_byte(d->bus, d->addr, d->h12_reg, + v & (uint8_t)~d->h12_mask) == 0; } #if RTC_RV3028_CFG @@ -658,7 +674,9 @@ static bool rtc_probe(uint32_t *epoch_out) int h12 = rtc_12h(d, blk); if (h12 != 0) { - (void)rtc_select_24h(d); + if (d == s_active) { + (void)rtc_select_24h(d); /* only the adopted chip */ + } LOG_WRN("%s present, %s — clock will be set on the next " "GPS/app/CLI sync", d->name, h12 > 0 ? "12-hour mode" : "hour mode unreadable"); diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.h b/zephcore/adapters/clock/ZephyrRTCDiscover.h index a3334e0db..41745b6a7 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.h +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.h @@ -37,7 +37,7 @@ extern "C" { * rules a device out. A time is taken only from a clean read whose fields, * hours and year are each in range, and never while the descriptor's * twelve-hour-bit reads set or cannot be read; a set bit outside the time - * block is then cleared. Returns false if none present or no trustworthy + * block is then cleared on the adopted chip. Returns false if none present or no trustworthy * time is held. * * At a descriptor with rv3028-eeprom-config, the time is read once more @@ -60,8 +60,9 @@ bool zephcore_rtc_restore(uint32_t *epoch_out); /* * Persist an authoritative epoch to the discovered RTC chip, selecting - * 24-hour mode first, and clear its power-loss flag. No-op if no RTC was discovered. Safe to call often, but - * intended only for real syncs (GPS/app/CLI), not per-packet clock nudges. + * 24-hour mode first, and clear its power-loss flag. No-op if no RTC was + * discovered. Safe to call often, but intended only for real syncs + * (GPS/app/CLI), not per-packet clock nudges. * If restore adopted nothing but skipped a candidate whose first read was all * 0xFF, the first save probes again, once per boot, in the caller's context. * A device ruled out on its second read is not probed again for that reason. diff --git a/zephcore/boards/common/rtc-i2c.dtsi b/zephcore/boards/common/rtc-i2c.dtsi index caff4dad1..f3a206634 100644 --- a/zephcore/boards/common/rtc-i2c.dtsi +++ b/zephcore/boards/common/rtc-i2c.dtsi @@ -40,7 +40,7 @@ rtc_ds3231: rtc-ds3231@68 { status-reg = <0x0F>; /* control/status */ status-mask = <0x80>; /* OSF — oscillator stopped */ /* Hours bit 6, 12-hour mode: DS3231 (p. 12), DS1307 (p. 8) */ - twelve-hour-bit = <0x02 0x40>; + twelve-hour-bit = [02 40]; /* Bits shown as 0 in all three parts: DS3231 (19-5170 Rev 10, Figure 1, * p. 11) and DS3232 (19-5337 Rev 5, Figure 1, p. 11), where 05h bit 7 is * Century, and DS1307 (Rev 3/15, Table 2, p. 8, "always reads back as @@ -67,7 +67,7 @@ rtc_rv3028: rtc-rv3028@52 { date-index = <4>; status-reg = <0x0e>; /* status */ status-mask = <0x01>; /* PORF — power-on reset */ - twelve-hour-bit = <0x10 0x02>; /* Control 2 12_24, p. 24 */ + twelve-hour-bit = [10 02]; /* Control 2 12_24, p. 24 */ /* RV-3028-C7 Application Manual Rev. 1.4, 3.2, p. 12 */ zero-mask = [80 80 c0 f8 c0 e0 00]; }; diff --git a/zephcore/boards/nrf52840/rak4631/board.overlay b/zephcore/boards/nrf52840/rak4631/board.overlay index ea9360899..450f41f23 100644 --- a/zephcore/boards/nrf52840/rak4631/board.overlay +++ b/zephcore/boards/nrf52840/rak4631/board.overlay @@ -228,7 +228,7 @@ date-index = <4>; status-reg = <0x0e>; /* status */ status-mask = <0x01>; /* PORF — power-on reset */ - twelve-hour-bit = <0x10 0x02>; /* Control 2 12_24, p. 24 */ + twelve-hour-bit = [10 02]; /* Control 2 12_24, p. 24 */ /* RV-3028-C7 Application Manual Rev. 1.4, 3.2, p. 12 */ zero-mask = [80 80 c0 f8 c0 e0 00]; /* The RAK12002's backup is a supercapacitor. As the RV-3028-C7 diff --git a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml index 8a6c90727..c2a4da2ed 100644 --- a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml +++ b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml @@ -63,14 +63,15 @@ properties: says so, such as the RX8130CE. twelve-hour-bit: - type: array + type: uint8-array description: > - : the bit that reads set while the chip counts hours + [register mask]: the bit that reads set while the chip counts hours in 12-hour mode. Leave it out for a chip with no 12-hour mode. While - it reads set, or cannot be read, no time is taken from the chip, and - each time write selects 24-hour mode first: a bit outside the time - block is cleared, and one inside it is cleared by the write's own - hours byte. + it reads set, or cannot be read, no time is taken from the chip; a + set bit outside the time block is then cleared at boot on the + adopted chip. Each time write selects 24-hour mode first: a bit + outside the time block is cleared, and one inside it is cleared by + the write's own hours byte. rv3028-eeprom-config: type: uint8-array From e6d568b30949a2973985e32abb70b6c9ff2357a0 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 11:06:25 -0700 Subject: [PATCH 03/10] rtc: read an all-1s 12-hour mode register as unreadable Local review: rtc_12h() took a read of all 1s as 12-hour mode while rtc_select_24h() refuses the same read as a transfer cut short, so a switchover during the boot read of RV3028 Control 2, which never reads 0xFF since RESET always reads 0, logged "12-hour mode" rather than "hour mode unreadable". No time is taken either way. A header line the previous commit left unwrapped is reflowed. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 5 +++-- zephcore/adapters/clock/ZephyrRTCDiscover.h | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index 562258406..e7c4b5a5b 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -280,7 +280,8 @@ static void rtc_clear_power_flag(const struct rtc_desc *d) } /* 1 if the chip counts hours in 12-hour mode, 0 if in 24-hour mode or it has - * none, -1 if a mode bit outside the time block could not be read. */ + * none, -1 if a mode bit outside the time block could not be read, or read + * as all 1s, a transfer cut short (see rtc_select_24h()). */ static int rtc_12h(const struct rtc_desc *d, const uint8_t blk[7]) { uint8_t v; @@ -291,7 +292,7 @@ static int rtc_12h(const struct rtc_desc *d, const uint8_t blk[7]) if (d->h12_reg >= d->time_reg && d->h12_reg < d->time_reg + 7) { return (blk[d->h12_reg - d->time_reg] & d->h12_mask) != 0; } - if (i2c_reg_read_byte(d->bus, d->addr, d->h12_reg, &v) != 0) { + if (i2c_reg_read_byte(d->bus, d->addr, d->h12_reg, &v) != 0 || v == 0xFF) { return -1; } return (v & d->h12_mask) != 0; diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.h b/zephcore/adapters/clock/ZephyrRTCDiscover.h index 41745b6a7..ca252c2a2 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.h +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.h @@ -37,8 +37,8 @@ extern "C" { * rules a device out. A time is taken only from a clean read whose fields, * hours and year are each in range, and never while the descriptor's * twelve-hour-bit reads set or cannot be read; a set bit outside the time - * block is then cleared on the adopted chip. Returns false if none present or no trustworthy - * time is held. + * block is then cleared on the adopted chip. Returns false if none present + * or no trustworthy time is held. * * At a descriptor with rv3028-eeprom-config, the time is read once more * with the chip's backup switch flag (BSF) cleared before and checked From 235c3c00511ad1daeed3cedb75cbac4083fe2aa8 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 11:14:18 -0700 Subject: [PATCH 04/10] rtc: refuse a 12-hour mode register read that shows a bit reading 0 Local review: a one-byte read cut mid-byte by a switchover returns the real high bits and then 1s, with no I2C error and not always 0xFF, so the 0xFF check let a torn RV3028 Control 2 read through. If bit 1 was in the 1s tail, the clear wrote back RESET and the trailing enable bits. twelve-hour-bit gains a third byte naming the register's bits that always read 0 (RESET, Control 2 bit 0, p. 24); a read showing one is cut short, gives no time and is never written back. Every cut read ends in a 1 on bit 0, so this covers the 0xFF case too. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 28 +++++++++++++------ zephcore/boards/common/rtc-i2c.dtsi | 7 +++-- .../boards/nrf52840/rak4631/board.overlay | 2 +- .../dts/bindings/rtc/zephcore,rtc-i2c.yaml | 18 +++++++----- 4 files changed, 36 insertions(+), 19 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index e7c4b5a5b..6e4ed6339 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -42,6 +42,7 @@ struct rtc_desc { bool week_one_hot; /* weekday as one bit per day, not 0-6 */ uint8_t h12_reg; /* register of the 12-hour mode bit */ uint8_t h12_mask; /* that bit, or 0 if the chip has no 12-hour mode */ + uint8_t h12_zero; /* bits of h12_reg that always read 0 */ #if RTC_RV3028_CFG const uint8_t *cfg; /* rv3028-eeprom-config triplets, or NULL */ uint8_t cfg_len; @@ -62,8 +63,8 @@ DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_ZERO_ARRAY) #define RTC_H12_CHECK(node) \ IF_ENABLED(DT_NODE_HAS_PROP(node, twelve_hour_bit), \ - (BUILD_ASSERT(DT_PROP_LEN(node, twelve_hour_bit) == 2, \ - "twelve-hour-bit is [register mask]");)) + (BUILD_ASSERT(DT_PROP_LEN(node, twelve_hour_bit) == 3, \ + "twelve-hour-bit is [register mask zero]");)) DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_H12_CHECK) @@ -106,6 +107,9 @@ DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_CFG_ARRAY) .h12_mask = COND_CODE_1( \ DT_NODE_HAS_PROP(node, twelve_hour_bit), \ (DT_PROP_BY_IDX(node, twelve_hour_bit, 1)), (0)), \ + .h12_zero = COND_CODE_1( \ + DT_NODE_HAS_PROP(node, twelve_hour_bit), \ + (DT_PROP_BY_IDX(node, twelve_hour_bit, 2)), (0)), \ RTC_CFG_FIELDS(node) \ .name = DT_NODE_FULL_NAME(node), \ }, @@ -279,9 +283,18 @@ static void rtc_clear_power_flag(const struct rtc_desc *d) } } +/* Read the register holding an out-of-block 12-hour bit. False on a failed + * read, or one showing a bit that always reads 0: a read cut short ends in + * 1s (the RV3028's RESET, Control 2 bit 0, always reads 0, p. 24). */ +static bool rtc_read_h12_reg(const struct rtc_desc *d, uint8_t *v) +{ + return i2c_reg_read_byte(d->bus, d->addr, d->h12_reg, v) == 0 && + !(*v & d->h12_zero); +} + /* 1 if the chip counts hours in 12-hour mode, 0 if in 24-hour mode or it has - * none, -1 if a mode bit outside the time block could not be read, or read - * as all 1s, a transfer cut short (see rtc_select_24h()). */ + * none, -1 if a mode bit outside the time block could not be read cleanly + * (see rtc_read_h12_reg()). */ static int rtc_12h(const struct rtc_desc *d, const uint8_t blk[7]) { uint8_t v; @@ -292,7 +305,7 @@ static int rtc_12h(const struct rtc_desc *d, const uint8_t blk[7]) if (d->h12_reg >= d->time_reg && d->h12_reg < d->time_reg + 7) { return (blk[d->h12_reg - d->time_reg] & d->h12_mask) != 0; } - if (i2c_reg_read_byte(d->bus, d->addr, d->h12_reg, &v) != 0 || v == 0xFF) { + if (!rtc_read_h12_reg(d, &v)) { return -1; } return (v & d->h12_mask) != 0; @@ -301,8 +314,7 @@ static int rtc_12h(const struct rtc_desc *d, const uint8_t blk[7]) /* Select 24-hour mode. A mode bit outside the time block is cleared, and the * RV3028 then converts its Hours register itself (02h, p. 15). One inside it * (DS3231/DS1307 hours bit 6) is cleared by the time write's hours byte. A - * read of all 1s, a transfer cut short, is not written back: on the RV3028 - * it would set every other Control 2 bit, and RESET always reads 0 there. */ + * read that was not clean is never written back. */ static bool rtc_select_24h(const struct rtc_desc *d) { uint8_t v; @@ -311,7 +323,7 @@ static bool rtc_select_24h(const struct rtc_desc *d) (d->h12_reg >= d->time_reg && d->h12_reg < d->time_reg + 7)) { return true; } - if (i2c_reg_read_byte(d->bus, d->addr, d->h12_reg, &v) != 0 || v == 0xFF) { + if (!rtc_read_h12_reg(d, &v)) { return false; } return !(v & d->h12_mask) || diff --git a/zephcore/boards/common/rtc-i2c.dtsi b/zephcore/boards/common/rtc-i2c.dtsi index f3a206634..46614cd58 100644 --- a/zephcore/boards/common/rtc-i2c.dtsi +++ b/zephcore/boards/common/rtc-i2c.dtsi @@ -39,8 +39,9 @@ rtc_ds3231: rtc-ds3231@68 { date-index = <4>; status-reg = <0x0F>; /* control/status */ status-mask = <0x80>; /* OSF — oscillator stopped */ - /* Hours bit 6, 12-hour mode: DS3231 (p. 12), DS1307 (p. 8) */ - twelve-hour-bit = [02 40]; + /* Hours bit 6, 12-hour mode: DS3231 (p. 12), DS1307 (p. 8). In the + * time block, so the zero byte is unused. */ + twelve-hour-bit = [02 40 00]; /* Bits shown as 0 in all three parts: DS3231 (19-5170 Rev 10, Figure 1, * p. 11) and DS3232 (19-5337 Rev 5, Figure 1, p. 11), where 05h bit 7 is * Century, and DS1307 (Rev 3/15, Table 2, p. 8, "always reads back as @@ -67,7 +68,7 @@ rtc_rv3028: rtc-rv3028@52 { date-index = <4>; status-reg = <0x0e>; /* status */ status-mask = <0x01>; /* PORF — power-on reset */ - twelve-hour-bit = [10 02]; /* Control 2 12_24, p. 24 */ + twelve-hour-bit = [10 02 01]; /* Control 2 12_24; RESET reads 0, p. 24 */ /* RV-3028-C7 Application Manual Rev. 1.4, 3.2, p. 12 */ zero-mask = [80 80 c0 f8 c0 e0 00]; }; diff --git a/zephcore/boards/nrf52840/rak4631/board.overlay b/zephcore/boards/nrf52840/rak4631/board.overlay index 450f41f23..01a54d5c9 100644 --- a/zephcore/boards/nrf52840/rak4631/board.overlay +++ b/zephcore/boards/nrf52840/rak4631/board.overlay @@ -228,7 +228,7 @@ date-index = <4>; status-reg = <0x0e>; /* status */ status-mask = <0x01>; /* PORF — power-on reset */ - twelve-hour-bit = [10 02]; /* Control 2 12_24, p. 24 */ + twelve-hour-bit = [10 02 01]; /* Control 2 12_24; RESET reads 0, p. 24 */ /* RV-3028-C7 Application Manual Rev. 1.4, 3.2, p. 12 */ zero-mask = [80 80 c0 f8 c0 e0 00]; /* The RAK12002's backup is a supercapacitor. As the RV-3028-C7 diff --git a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml index c2a4da2ed..127c60d1f 100644 --- a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml +++ b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml @@ -65,13 +65,17 @@ properties: twelve-hour-bit: type: uint8-array description: > - [register mask]: the bit that reads set while the chip counts hours - in 12-hour mode. Leave it out for a chip with no 12-hour mode. While - it reads set, or cannot be read, no time is taken from the chip; a - set bit outside the time block is then cleared at boot on the - adopted chip. Each time write selects 24-hour mode first: a bit - outside the time block is cleared, and one inside it is cleared by - the write's own hours byte. + [register mask zero]: the bit that reads set while the chip counts + hours in 12-hour mode, and the bits of that register that always + read 0. Leave it out for a chip with no 12-hour mode. For a + register outside the time block, a read showing a zero bit set is + taken as cut short (it ends in 1s), so name at least bit 0 there; + it is never written back. While the mode bit reads set, or cannot + be read cleanly, no time is taken from the chip; a set bit outside + the time block is then cleared at boot on the adopted chip. Each + time write selects 24-hour mode first: a bit outside the time + block is cleared, and one inside it is cleared by the write's own + hours byte. rv3028-eeprom-config: type: uint8-array From d927e9330eed6c5cbbfd82f7226e18a545f58754 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 14:19:50 -0700 Subject: [PATCH 05/10] rtc: require a zero bit for an out-of-block twelve-hour-bit Local review: the binding asked for at least one always-zero bit when the mode register sits outside the time block, but nothing enforced it, and with none every read counts as clean, so a cut read could be written back. A BUILD_ASSERT now requires it (confirmed to fail on [10 02 00]). The binding says a read showing a zero bit set is never written back, rather than leaving "it" to refer to bit 0. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 7 ++++++- zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml | 16 ++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index c1fa29514..0c515e387 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -61,10 +61,15 @@ struct rtc_desc { DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_ZERO_ARRAY) +#define RTC_H12_REG(node) DT_PROP_BY_IDX(node, twelve_hour_bit, 0) #define RTC_H12_CHECK(node) \ IF_ENABLED(DT_NODE_HAS_PROP(node, twelve_hour_bit), \ (BUILD_ASSERT(DT_PROP_LEN(node, twelve_hour_bit) == 3, \ - "twelve-hour-bit is [register mask zero]");)) + "twelve-hour-bit is [register mask zero]"); \ + BUILD_ASSERT((RTC_H12_REG(node) >= DT_PROP(node, time_reg) && \ + RTC_H12_REG(node) < DT_PROP(node, time_reg) + 7) || \ + DT_PROP_BY_IDX(node, twelve_hour_bit, 2) != 0, \ + "twelve-hour-bit outside the time block needs a zero bit");)) DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_H12_CHECK) diff --git a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml index 127c60d1f..0395339b6 100644 --- a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml +++ b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml @@ -68,14 +68,14 @@ properties: [register mask zero]: the bit that reads set while the chip counts hours in 12-hour mode, and the bits of that register that always read 0. Leave it out for a chip with no 12-hour mode. For a - register outside the time block, a read showing a zero bit set is - taken as cut short (it ends in 1s), so name at least bit 0 there; - it is never written back. While the mode bit reads set, or cannot - be read cleanly, no time is taken from the chip; a set bit outside - the time block is then cleared at boot on the adopted chip. Each - time write selects 24-hour mode first: a bit outside the time - block is cleared, and one inside it is cleared by the write's own - hours byte. + register outside the time block the zero byte must name at least + one bit (the build checks): a read showing one set was cut short, + since a cut read ends in 1s, and such a read is never written + back. While the mode bit reads set, or cannot be read cleanly, no + time is taken from the chip; a set bit outside the time block is + then cleared at boot on the adopted chip. Each time write selects + 24-hour mode first: a bit outside the time block is cleared, and + one inside it is cleared by the write's own hours byte. rv3028-eeprom-config: type: uint8-array From 70def664439b3e6ee28e57aa8ed7f3ea3f2bbc8c Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 14:39:49 -0700 Subject: [PATCH 06/10] rtc: leave the 12-hour bit to the next save, and reject a zero byte over it Local review: the boot-time clear of an out-of-block 12-hour bit was the one write on the RV3028 boot path outside the BSF bracket, so a switchover during it could leave a corrupt Control 2 that no later read-modify-write repairs. It is removed: a chip in 12-hour mode gives no time until the next sync, whose write selects 24-hour mode inside the bracket. A BUILD_ASSERT also rejects a zero byte that overlaps the mode bit (confirmed to fail on [10 02 03]), which would refuse every read in 12-hour mode and fail every save. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 8 ++++---- zephcore/adapters/clock/ZephyrRTCDiscover.h | 3 +-- .../dts/bindings/rtc/zephcore,rtc-i2c.yaml | 18 +++++++++--------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index 0c515e387..0ac702aa9 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -69,7 +69,10 @@ DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_ZERO_ARRAY) BUILD_ASSERT((RTC_H12_REG(node) >= DT_PROP(node, time_reg) && \ RTC_H12_REG(node) < DT_PROP(node, time_reg) + 7) || \ DT_PROP_BY_IDX(node, twelve_hour_bit, 2) != 0, \ - "twelve-hour-bit outside the time block needs a zero bit");)) + "twelve-hour-bit outside the time block needs a zero bit"); \ + BUILD_ASSERT((DT_PROP_BY_IDX(node, twelve_hour_bit, 1) & \ + DT_PROP_BY_IDX(node, twelve_hour_bit, 2)) == 0, \ + "twelve-hour-bit zero bits overlap the mode bit");)) DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_H12_CHECK) @@ -677,9 +680,6 @@ static bool rtc_probe(uint32_t *epoch_out) int h12 = rtc_12h(d, blk); if (h12 != 0) { - if (d == s_active) { - (void)rtc_select_24h(d); /* only the adopted chip */ - } LOG_WRN("%s present, %s — clock will be set on the next " "GPS/app/CLI sync", d->name, h12 > 0 ? "12-hour mode" : "hour mode unreadable"); diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.h b/zephcore/adapters/clock/ZephyrRTCDiscover.h index d897e6c2c..77e3cf242 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.h +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.h @@ -28,8 +28,7 @@ extern "C" { * sane time (year >= 2025 and its power-loss flag clear): store its Unix * epoch in *epoch_out and return true. The first chip found (valid time or * not) is remembered as the write-back target. No time is taken while the - * descriptor's twelve-hour-bit reads set or cannot be read cleanly; a set - * bit outside the time block is then cleared on the adopted chip. Returns + * descriptor's twelve-hour-bit reads set or cannot be read cleanly. Returns * false if none is present or none holds a trustworthy time. * * A descriptor with rv3028-eeprom-config also has that config stored in the diff --git a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml index 0395339b6..a58814074 100644 --- a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml +++ b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml @@ -67,15 +67,15 @@ properties: description: > [register mask zero]: the bit that reads set while the chip counts hours in 12-hour mode, and the bits of that register that always - read 0. Leave it out for a chip with no 12-hour mode. For a - register outside the time block the zero byte must name at least - one bit (the build checks): a read showing one set was cut short, - since a cut read ends in 1s, and such a read is never written - back. While the mode bit reads set, or cannot be read cleanly, no - time is taken from the chip; a set bit outside the time block is - then cleared at boot on the adopted chip. Each time write selects - 24-hour mode first: a bit outside the time block is cleared, and - one inside it is cleared by the write's own hours byte. + read 0, which must not include the mode bit. Leave it out for a + chip with no 12-hour mode. For a register outside the time block + the zero byte must name at least one bit (the build checks both): + a read showing one set was cut short, since a cut read ends in 1s, + and such a read is never written back. While the mode bit reads + set, or cannot be read cleanly, no time is taken from the chip. + Each time write selects 24-hour mode first: a bit outside the time + block is cleared, and one inside it is cleared by the write's own + hours byte. rv3028-eeprom-config: type: uint8-array From 9a42030f3341eb1d66970a46aad0f741b0496e09 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 14:54:00 -0700 Subject: [PATCH 07/10] rtc: require bit 0 out of block, the hours register in block, and a mode bit Local review: only bit 0 reads 1 in every read cut short (I2C sends the MSB first), so an out-of-block zero byte naming another bit could let a torn read be written back; the build now requires bit 0. An in-block twelve-hour-bit must be the hours register, which the time write clears, and the mode mask must be nonzero. Each was confirmed to fail the build ([10 02 80], [04 10 00], [10 00 01]). The rv3028_write_time() comment and the binding no longer call year 00h the first write, since 24-hour mode is selected before it. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 17 +++++++---- .../dts/bindings/rtc/zephcore,rtc-i2c.yaml | 28 +++++++++---------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index 0ac702aa9..33dc86731 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -66,10 +66,16 @@ DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_ZERO_ARRAY) IF_ENABLED(DT_NODE_HAS_PROP(node, twelve_hour_bit), \ (BUILD_ASSERT(DT_PROP_LEN(node, twelve_hour_bit) == 3, \ "twelve-hour-bit is [register mask zero]"); \ + BUILD_ASSERT(DT_PROP_BY_IDX(node, twelve_hour_bit, 1) != 0, \ + "twelve-hour-bit needs a mode bit"); \ + BUILD_ASSERT(!(RTC_H12_REG(node) >= DT_PROP(node, time_reg) && \ + RTC_H12_REG(node) < DT_PROP(node, time_reg) + 7) || \ + RTC_H12_REG(node) == DT_PROP(node, time_reg) + 2, \ + "twelve-hour-bit in the time block must be in hours"); \ BUILD_ASSERT((RTC_H12_REG(node) >= DT_PROP(node, time_reg) && \ RTC_H12_REG(node) < DT_PROP(node, time_reg) + 7) || \ - DT_PROP_BY_IDX(node, twelve_hour_bit, 2) != 0, \ - "twelve-hour-bit outside the time block needs a zero bit"); \ + (DT_PROP_BY_IDX(node, twelve_hour_bit, 2) & 0x01), \ + "twelve-hour-bit outside the time block needs bit 0 as a zero bit"); \ BUILD_ASSERT((DT_PROP_BY_IDX(node, twelve_hour_bit, 1) & \ DT_PROP_BY_IDX(node, twelve_hour_bit, 2)) == 0, \ "twelve-hour-bit zero bits overlap the mode bit");)) @@ -348,9 +354,10 @@ static bool rtc_select_24h(const struct rtc_desc *d) #define RV3028_REG_CFG_LAST 0x37 /* EEPROM Backup */ #define RV3028_BACKUP_BSM 0x0C /* 37h switchover mode; 00 = disabled */ -/* Year 00h first and the real year last: a power loss, which no retry can - * repeat, cutting the write after the first byte and before the last leaves - * year 2000, so the next boot takes no time from it. */ +/* Year 00h before the other time bytes and the real year last (24-hour mode + * is selected before both): a power loss, which no retry can repeat, cutting + * the write after the first byte and before the last leaves year 2000, so + * the next boot takes no time from it. */ static bool rv3028_write_time(const struct rtc_desc *d, const uint8_t blk[7]) { return rtc_select_24h(d) && diff --git a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml index a58814074..46019325e 100644 --- a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml +++ b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml @@ -68,14 +68,14 @@ properties: [register mask zero]: the bit that reads set while the chip counts hours in 12-hour mode, and the bits of that register that always read 0, which must not include the mode bit. Leave it out for a - chip with no 12-hour mode. For a register outside the time block - the zero byte must name at least one bit (the build checks both): - a read showing one set was cut short, since a cut read ends in 1s, - and such a read is never written back. While the mode bit reads - set, or cannot be read cleanly, no time is taken from the chip. - Each time write selects 24-hour mode first: a bit outside the time - block is cleared, and one inside it is cleared by the write's own - hours byte. + chip with no 12-hour mode. Inside the time block it must be the + hours register. Outside it, the zero bits must include bit 0 (the + build checks these): a read cut short ends in 1s, so it always + shows bit 0 set, and such a read is never written back. While the + mode bit reads set, or cannot be read cleanly, no time is taken + from the chip. Each time write selects 24-hour mode first: a bit + outside the time block is cleared, and one in hours is cleared by + the write's own hours byte. rv3028-eeprom-config: type: uint8-array @@ -97,9 +97,9 @@ properties: identifying the chip again first; a boot whose read could not identify it cleanly waits for those retries. The time is read at boot and written on each sync with the chip's backup switch flag - cleared before and checked after. A write sets year 00h first and - the real year last, so a write cut by a power loss reads as "time - not yet set", and a write not confirmed is repeated every 5 s, up - to 12 times. Set it only on a board whose backup supply is known: - trickle charging suits a supercapacitor or rechargeable cell, - never a primary cell. + cleared before and checked after. A write sets year 00h before the + other time bytes and the real year last, so a write cut by a power + loss reads as "time not yet set", and a write not confirmed is + repeated every 5 s, up to 12 times. Set it only on a board whose + backup supply is known: trickle charging suits a supercapacitor or + rechargeable cell, never a primary cell. From 59ba354daf45b44fc3ed48c9df152fe7de24990b Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 15:24:32 -0700 Subject: [PATCH 08/10] rtc: honour twelve-hour-bit's zero bits in the time block too Copilot (PR 52): an in-block mode register checked only the mode bit, ignoring the declared always-zero bits, so the binding's "cannot be read cleanly" rule held only outside the time block. rtc_12h() now gives -1 for a set zero bit wherever the register is. In-tree this changes nothing: the DS3231 node declares no in-block zero bits, and its zero-mask already rules out a block with hours bit 7 set. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index 33dc86731..17ed78a00 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -309,9 +309,11 @@ static int rtc_12h(const struct rtc_desc *d, const uint8_t blk[7]) return 0; } if (d->h12_reg >= d->time_reg && d->h12_reg < d->time_reg + 7) { - return (blk[d->h12_reg - d->time_reg] & d->h12_mask) != 0; + v = blk[d->h12_reg - d->time_reg]; + } else if (!rtc_read_h12_reg(d, &v)) { + return -1; } - if (!rtc_read_h12_reg(d, &v)) { + if (v & d->h12_zero) { return -1; } return (v & d->h12_mask) != 0; From 74c7e0cf070a3825e9fd07838a724ee8e98a7db0 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 15:31:19 -0700 Subject: [PATCH 09/10] rtc: read back the 12-hour clear and repair a torn write Local review: the save's Control 2 read-modify-write was never checked against the value it meant to write. On an RV3028 without rv3028-eeprom-config there is no BSF bracket at all, and even inside the bracket a retry checks bit 1 alone, so a write torn by a switchover (5.10) could leave other Control 2 bits set for good. rtc_select_24h() now reads the register back and writes the intended value once more if it differs, failing the save if it still does. The DS3231 node's comment no longer calls its zero byte unused, which 59ba354 made untrue. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 21 ++++++++++++++++----- zephcore/boards/common/rtc-i2c.dtsi | 4 ++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index 17ed78a00..3285d4d7f 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -321,10 +321,11 @@ static int rtc_12h(const struct rtc_desc *d, const uint8_t blk[7]) /* Select 24-hour mode: clear an out-of-block bit (the RV3028 converts its * Hours itself, 02h); an in-block one (DS3231 hours bit 6) is cleared by the - * time write's hours byte. */ + * time write's hours byte. The clear is read back and written once more if + * it differs, which repairs a write torn by a switchover (5.10). */ static bool rtc_select_24h(const struct rtc_desc *d) { - uint8_t v; + uint8_t v, w, r; if (d->h12_mask == 0 || (d->h12_reg >= d->time_reg && d->h12_reg < d->time_reg + 7)) { @@ -333,9 +334,19 @@ static bool rtc_select_24h(const struct rtc_desc *d) if (!rtc_read_h12_reg(d, &v)) { return false; } - return !(v & d->h12_mask) || - i2c_reg_write_byte(d->bus, d->addr, d->h12_reg, - v & (uint8_t)~d->h12_mask) == 0; + if (!(v & d->h12_mask)) { + return true; + } + w = v & (uint8_t)~d->h12_mask; + for (int tries = 0; tries < 2; tries++) { + if (i2c_reg_write_byte(d->bus, d->addr, d->h12_reg, w) != 0) { + return false; + } + if (i2c_reg_read_byte(d->bus, d->addr, d->h12_reg, &r) == 0 && r == w) { + return true; + } + } + return false; } #if RTC_RV3028_CFG diff --git a/zephcore/boards/common/rtc-i2c.dtsi b/zephcore/boards/common/rtc-i2c.dtsi index 46614cd58..a9fc67a8a 100644 --- a/zephcore/boards/common/rtc-i2c.dtsi +++ b/zephcore/boards/common/rtc-i2c.dtsi @@ -39,8 +39,8 @@ rtc_ds3231: rtc-ds3231@68 { date-index = <4>; status-reg = <0x0F>; /* control/status */ status-mask = <0x80>; /* OSF — oscillator stopped */ - /* Hours bit 6, 12-hour mode: DS3231 (p. 12), DS1307 (p. 8). In the - * time block, so the zero byte is unused. */ + /* Hours bit 6, 12-hour mode: DS3231 (p. 12), DS1307 (p. 8). Hours + * bit 7 is left to zero-mask below. */ twelve-hour-bit = [02 40 00]; /* Bits shown as 0 in all three parts: DS3231 (19-5170 Rev 10, Figure 1, * p. 11) and DS3232 (19-5337 Rev 5, Figure 1, p. 11), where 05h bit 7 is From c18e1f05d2e217cf0e044502401d144d84abb516 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 6 Oct 2026 15:40:12 -0700 Subject: [PATCH 10/10] rtc: promise a sync only for the RTC it will write CodeRabbit (PR 52): a boot log for an RTC that is present but not the adopted write-back target said "clock will be set on the next GPS/app/CLI sync", which zephcore_rtc_save() never does for it. The clause is now added only for the adopted chip. Swept the three sibling log lines in the same loop (time unreadable twice, power-loss flag), which made the same promise. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index 3285d4d7f..efd209073 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -691,26 +691,26 @@ static bool rtc_probe(uint32_t *epoch_out) } #endif } + /* Only the write-back target is set by a later sync. */ + const char *then = d == s_active ? + " — clock will be set on the next GPS/app/CLI sync" : ""; if (v == RTC_FOUND_GARBLED) { - LOG_WRN("%s present, time unreadable — clock will be set on the " - "next GPS/app/CLI sync", d->name); + LOG_WRN("%s present, time unreadable%s", d->name, then); continue; } int h12 = rtc_12h(d, blk); if (h12 != 0) { - LOG_WRN("%s present, %s — clock will be set on the next " - "GPS/app/CLI sync", d->name, - h12 > 0 ? "12-hour mode" : "hour mode unreadable"); + LOG_WRN("%s present, %s%s", d->name, + h12 > 0 ? "12-hour mode" : "hour mode unreadable", then); continue; } int flag = rtc_power_flag(d, blk); if (flag != 0) { - LOG_WRN("%s present, power-loss flag %s — clock will be set " - "on the next GPS/app/CLI sync", d->name, - flag > 0 ? "set" : "unreadable"); + LOG_WRN("%s present, power-loss flag %s%s", d->name, + flag > 0 ? "set" : "unreadable", then); continue; } @@ -720,8 +720,7 @@ static bool rtc_probe(uint32_t *epoch_out) /* Identity leaves the hours and year unchecked, and may have let the * other fields pass on a flag read since; a time needs them all. */ if (!rtc_fields_ok(d, blk) || !bcd_field_ok(hb, 23) || !bcd_field_ok(yb, 99)) { - LOG_WRN("%s present, time unreadable — clock will be set on the " - "next GPS/app/CLI sync", d->name); + LOG_WRN("%s present, time unreadable%s", d->name, then); continue; }