From 03f306b5b37fd1aad191fbf3fc439d106bd572cc Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 4 Oct 2026 11:48:55 -0700 Subject: [PATCH] rtc: identify RTCs by documented always-zero bits; stop skipping real ones rtc_probe() decided from one read whether the device at a declared address was the RTC its descriptor expects: a valid BCD block including hours and year, or the power-loss flag set or unreadable. That skipped real RTCs whose year byte or hours another firmware left out of range (a year byte of A0 or more; 12-hour mode reads PM hours with bit 5 set), ruled nothing out by identity, and adopted an erased EEPROM through its 0xFF "status". - An optional zero-mask descriptor property holds the time-block bits a part's data sheet shows as 0. rtc-i2c.dtsi sets it for 0x68 (DS3231 19-5170 Rev 10 and DS3232 19-5337 Rev 5, Figure 1, p. 11; DS1307 Rev 3/15, Table 2, p. 8; 00h bit 7 left out, the DS1307's clock-halt bit), the RV3028 (App Manual Rev 1.4, 3.2, p. 12) and the RX8130CE (ETM50E-10, 13.2.1 Table 12, p. 22). The PCF8563 gets none: its data sheet (Rev 11.1, Table 4, p. 10) marks unused bits "not relevant". - One read rules a device out if a masked bit is set, or if seconds, minutes, date or month are out of range while the power-loss flag does not read as set. The year and hours are no longer identity checks. - A device is passed over only when two reads each rule it out. A failed first read means nothing is there; a failed second read does not count, and with no clean read the chip is adopted for write-back but gives no time. An all-0xFF second read rules the device out. - An all-0xFF first read is skipped, and the first save probes once more, since a real RTC can power up that way. - A time is restored only from a clean read whose fields, hours and year are all valid. The decision is one function, rtc_identify(). The same rules, with a few agreed differences, are in meshcore-dev/MeshCore#3544. Co-Authored-By: Claude Opus 5.5 --- zephcore/adapters/clock/ZephyrRTCDiscover.c | 172 ++++++++++++++---- zephcore/adapters/clock/ZephyrRTCDiscover.h | 21 ++- zephcore/boards/common/rtc-i2c.dtsi | 12 ++ .../meshtracker_x1_nrf52840.dts | 6 +- .../dts/bindings/rtc/zephcore,rtc-i2c.yaml | 10 + 5 files changed, 178 insertions(+), 43 deletions(-) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index db37305..3fb8184 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -17,6 +17,8 @@ #include #include +#include + LOG_MODULE_REGISTER(zephcore_rtc, CONFIG_ZEPHCORE_DATASTORE_LOG_LEVEL); #define RTC_COMPAT zephcore_rtc_i2c @@ -33,9 +35,21 @@ struct rtc_desc { uint8_t date_index; /* day-of-month offset in the 7-byte block */ uint8_t status_reg; /* power-loss flag register, or RTC_STATUS_IN_SECONDS */ 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 */ const char *name; }; +#define RTC_ZERO_NAME(node) _CONCAT(rtc_zero_, DT_DEP_ORD(node)) + +#define RTC_ZERO_ARRAY(node) \ + IF_ENABLED(DT_NODE_HAS_PROP(node, zero_mask), \ + (static const uint8_t RTC_ZERO_NAME(node)[] = \ + DT_PROP(node, zero_mask); \ + BUILD_ASSERT(sizeof(RTC_ZERO_NAME(node)) == 7, \ + "zero-mask is one byte per time register");)) + +DT_FOREACH_STATUS_OKAY(RTC_COMPAT, RTC_ZERO_ARRAY) + #define RTC_DESC_ENTRY(node) \ { \ .bus = DEVICE_DT_GET(DT_BUS(node)), \ @@ -44,6 +58,9 @@ struct rtc_desc { .date_index = (uint8_t)DT_PROP(node, date_index), \ .status_reg = (uint8_t)DT_PROP(node, status_reg), \ .status_mask = (uint8_t)DT_PROP(node, status_mask), \ + .zero = COND_CODE_1( \ + DT_NODE_HAS_PROP(node, zero_mask), \ + (RTC_ZERO_NAME(node)), (NULL)), \ .name = DT_NODE_FULL_NAME(node), \ }, @@ -54,14 +71,16 @@ static const struct rtc_desc rtc_descs[] = { /* Chip we'll read/write going forward (first one found present). */ static const struct rtc_desc *s_active; static bool s_probed; +/* A candidate's first read was all 0xFF and it was skipped: if nothing was + * adopted, the first save probes again. */ +static bool s_skipped_ff; +static bool s_reprobed; #define BCD2BIN(x) ((((x) >> 4) & 0x0F) * 10 + ((x) & 0x0F)) #define BIN2BCD(x) ((((x) / 10) << 4) | ((x) % 10)) /* A byte is valid BCD if both nibbles are 0-9, and its decoded value fits the - * field. Used to tell a real RTC apart from an unrelated I2C chip that happens - * to share an address (e.g. an MPU-class IMU at 0x68, same as DS3231) — we must - * never adopt and write time into such a device. */ + * field. */ static bool bcd_field_ok(uint8_t v, unsigned max) { if ((v & 0x0F) > 9 || (v >> 4) > 9) { @@ -95,8 +114,8 @@ static void civil_from_days(int64_t z, int *y, unsigned *m, unsigned *d) *y = yy + (*m <= 2); } -/* Read the chip's power-loss flag. true => held time is unreliable. */ -static bool rtc_time_unreliable(const struct rtc_desc *d, const uint8_t blk[7]) +/* The chip's power-loss flag: 1 set, 0 clear, -1 the status read failed. */ +static int rtc_power_flag(const struct rtc_desc *d, const uint8_t blk[7]) { if (d->status_reg == RTC_STATUS_IN_SECONDS) { return (blk[0] & d->status_mask) != 0; @@ -104,54 +123,130 @@ static bool rtc_time_unreliable(const struct rtc_desc *d, const uint8_t blk[7]) uint8_t st; if (i2c_reg_read_byte(d->bus, d->addr, d->status_reg, &st) != 0) { - return true; /* can't confirm => don't trust it */ + return -1; } return (st & d->status_mask) != 0; } -/* Probe all chips once; cache the first present one in s_active. If a present - * chip holds a sane time, return it via epoch_out. */ +/* Seconds, minutes, date and month in range: the fields that read the same in + * 12- and 24-hour mode. */ +static bool rtc_fields_ok(const struct rtc_desc *d, const uint8_t blk[7]) +{ + uint8_t db = blk[d->date_index] & 0x3F, ob = blk[5] & 0x1F; + + return bcd_field_ok(blk[0] & 0x7F, 59) && bcd_field_ok(blk[1] & 0x7F, 59) && + bcd_field_ok(db, 31) && BCD2BIN(db) >= 1 && + bcd_field_ok(ob, 12) && BCD2BIN(ob) >= 1; +} + +/* True if one read shows the device is not this RTC: a bit the data sheet + * shows as 0 is set, or rtc_fields_ok() fails while the power-loss flag does + * not read as set. The year and hours are not checked: other firmware can + * leave a year byte past 99 or the chip in 12-hour mode, and the chip is + * still this RTC. */ +static bool rtc_ruled_out(const struct rtc_desc *d, const uint8_t blk[7]) +{ + for (size_t k = 0; d->zero != NULL && k < 7; k++) { + if (blk[k] & d->zero[k]) { + return true; + } + } + return !rtc_fields_ok(d, blk) && rtc_power_flag(d, blk) != 1; +} + +enum rtc_verdict { RTC_ABSENT, RTC_ERASED, RTC_NOT_THIS, RTC_FOUND, RTC_FOUND_GARBLED }; + +static bool rtc_all_ff(const uint8_t blk[7]) +{ + for (size_t k = 0; k < 7; k++) { + if (blk[k] != 0xFF) { + return false; + } + } + return true; +} + +/* Decide whether the device at d is the RTC it declares. A first read that + * fails means nothing is there, and one that is all 0xFF is an erased EEPROM, + * though a real RTC can power up that way, so it is skipped for now. + * Otherwise the device is ruled out only when two reads each rule it out: a + * failed second read does not, an all-0xFF second read does, and an + * unreadable power-loss flag cannot vouch for the fields. RTC_FOUND leaves + * the block to decode in blk; RTC_FOUND_GARBLED is this RTC with no clean + * read to take a time from. */ +static enum rtc_verdict rtc_identify(const struct rtc_desc *d, uint8_t blk[7]) +{ + uint8_t again[7]; + + if (!device_is_ready(d->bus) || + i2c_burst_read(d->bus, d->addr, d->time_reg, blk, 7) != 0) { + return RTC_ABSENT; + } + if (rtc_all_ff(blk)) { + return RTC_ERASED; + } + if (!rtc_ruled_out(d, blk)) { + return RTC_FOUND; + } + if (i2c_burst_read(d->bus, d->addr, d->time_reg, again, sizeof(again)) != 0) { + return RTC_FOUND_GARBLED; + } + if (rtc_all_ff(again) || rtc_ruled_out(d, again)) { + return RTC_NOT_THIS; + } + memcpy(blk, again, sizeof(again)); + return RTC_FOUND; +} + +/* Probe the chips in order; cache the first one found in s_active. Stop at + * the first that holds a sane time, returned via epoch_out. */ static bool rtc_probe(uint32_t *epoch_out) { + s_skipped_ff = false; + for (size_t i = 0; i < ARRAY_SIZE(rtc_descs); i++) { const struct rtc_desc *d = &rtc_descs[i]; uint8_t blk[7]; + enum rtc_verdict v = rtc_identify(d, blk); - if (!device_is_ready(d->bus)) { + if (v == RTC_ERASED) { + s_skipped_ff = true; continue; } - if (i2c_burst_read(d->bus, d->addr, d->time_reg, blk, sizeof(blk)) != 0) { - continue; /* no ACK => chip absent */ + if (v == RTC_NOT_THIS) { + LOG_INF("%s: device at the address is not this RTC, skipped", d->name); + continue; } - - /* Mask off flag/century bits. We trust this is a real RTC (vs. an - * unrelated chip sharing the address) if EITHER the block is valid - * BCD, OR the chip's power-loss flag is set — the latter is itself - * proof it's an RTC that lost power and whose time registers may be - * garbage. Adopting in the power-loss case is essential: otherwise a - * battery-depleted RTC (garbage registers, VL/OSF/PORF latched) would - * never become the write-back target, so a sync could never - * re-initialise it and the clock would stay blank forever. */ - uint8_t sb = blk[0] & 0x7F, mb = blk[1] & 0x7F, hb = blk[2] & 0x3F; - uint8_t db = blk[d->date_index] & 0x3F, ob = blk[5] & 0x1F, yb = blk[6]; - - bool bcd_ok = bcd_field_ok(sb, 59) && bcd_field_ok(mb, 59) && - bcd_field_ok(hb, 23) && bcd_field_ok(db, 31) && - bcd_field_ok(ob, 12) && bcd_field_ok(yb, 99) && - BCD2BIN(db) >= 1 && BCD2BIN(ob) >= 1; - bool unreliable = rtc_time_unreliable(d, blk); - - if (!bcd_ok && !unreliable) { - continue; /* neither valid time nor a lost-power RTC => skip */ + if (v == RTC_ABSENT) { + continue; } if (s_active == NULL) { s_active = d; /* RTC => our write-back target */ } - if (unreliable) { - LOG_WRN("%s present, power-loss flag set — clock will be set " - "on the next GPS/app/CLI sync", d->name); + if (v == RTC_FOUND_GARBLED) { + LOG_WRN("%s present, time unreadable — clock will be set on the " + "next GPS/app/CLI sync", d->name); + 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"); + continue; + } + + uint8_t sb = blk[0] & 0x7F, mb = blk[1] & 0x7F, hb = blk[2] & 0x3F; + uint8_t db = blk[d->date_index] & 0x3F, ob = blk[5] & 0x1F, yb = blk[6]; + + /* 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); continue; } @@ -172,9 +267,9 @@ static bool rtc_probe(uint32_t *epoch_out) hour * 3600 + min * 60 + sec; if (epoch_out) { *epoch_out = (uint32_t)e; + LOG_INF("RTC %s: restored %04u-%02u-%02u %02u:%02u:%02u UTC", + d->name, year, month, day, hour, min, sec); } - LOG_INF("RTC %s: restored %04u-%02u-%02u %02u:%02u:%02u UTC", - d->name, year, month, day, hour, min, sec); return true; } return false; @@ -192,6 +287,11 @@ void zephcore_rtc_save(uint32_t epoch) /* Restore wasn't run (unexpected) — discover now. */ (void)rtc_probe(NULL); s_probed = true; + } else if (s_active == NULL && s_skipped_ff && !s_reprobed) { + /* A running RTC may have counted off all 0xFF since boot; an + * erased EEPROM has not, and is skipped again. */ + s_reprobed = true; + (void)rtc_probe(NULL); } if (s_active == NULL) { return; diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.h b/zephcore/adapters/clock/ZephyrRTCDiscover.h index 4a33862..ba82da2 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.h +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.h @@ -23,11 +23,19 @@ extern "C" { #endif /* - * Probe all declared RTC chips. If one is present and holds a sane time - * (year >= 2025 and its power-loss flag is clear), store the Unix epoch in - * *epoch_out and return true. The present chip (valid time or not) is - * remembered as the write-back target. Returns false if none present or no - * trustworthy time is held. + * Probe the declared RTC chips in order, stopping at the first that holds a + * 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. A failed first read means + * nothing is there, and a first read of all 0xFF (an erased EEPROM) is + * skipped. Otherwise a device is passed over only if two reads each show it + * is not that RTC: a bit set that the data sheet shows as 0 (the + * descriptor's zero-mask), seconds, minutes, date or month out of range + * 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. */ bool zephcore_rtc_restore(uint32_t *epoch_out); @@ -35,6 +43,9 @@ 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 * 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. */ void zephcore_rtc_save(uint32_t epoch); diff --git a/zephcore/boards/common/rtc-i2c.dtsi b/zephcore/boards/common/rtc-i2c.dtsi index 1dc262c..b4edf1c 100644 --- a/zephcore/boards/common/rtc-i2c.dtsi +++ b/zephcore/boards/common/rtc-i2c.dtsi @@ -39,6 +39,12 @@ rtc_ds3231: rtc-ds3231@68 { date-index = <4>; status-reg = <0x0F>; /* control/status */ status-mask = <0x80>; /* OSF — oscillator stopped */ + /* 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 + * 0"). 00h bit 7 is left out: it is the DS1307's clock-halt bit, set on + * a new chip. On a DS1307, 0Fh is RAM, so status-reg reads a user byte. */ + zero-mask = [00 80 80 f8 c0 60 00]; }; rtc_pcf8563: rtc-pcf8563@51 { @@ -48,6 +54,8 @@ rtc_pcf8563: rtc-pcf8563@51 { date-index = <3>; /* PCF8563 puts date before weekday */ status-reg = <0xFF>; /* flag is in the seconds byte */ status-mask = <0x80>; /* VL — clock integrity not guaranteed */ + /* No zero-mask: the data sheet (Rev. 11.1, Table 4, p. 10) marks the + * unused bits "not relevant", not 0. */ }; rtc_rv3028: rtc-rv3028@52 { @@ -57,6 +65,8 @@ rtc_rv3028: rtc-rv3028@52 { date-index = <4>; status-reg = <0x0e>; /* status */ status-mask = <0x01>; /* PORF — power-on reset */ + /* RV-3028-C7 Application Manual Rev. 1.4, 3.2, p. 12 */ + zero-mask = [80 80 c0 f8 c0 e0 00]; }; rtc_rx8130ce: rtc-rx8130ce@32 { @@ -66,4 +76,6 @@ rtc_rx8130ce: rtc-rx8130ce@32 { date-index = <4>; status-reg = <0x1d>; /* flag */ status-mask = <0x02>; /* VLF — voltage low */ + /* RX8130CE Application Manual ETM50E-10, 13.2.1 Table 12, p. 22 */ + zero-mask = [80 80 c0 80 c0 e0 00]; }; diff --git a/zephcore/boards/nrf52840/meshtracker_x1/meshtracker_x1_nrf52840.dts b/zephcore/boards/nrf52840/meshtracker_x1/meshtracker_x1_nrf52840.dts index 3f7de1e..514578e 100644 --- a/zephcore/boards/nrf52840/meshtracker_x1/meshtracker_x1_nrf52840.dts +++ b/zephcore/boards/nrf52840/meshtracker_x1/meshtracker_x1_nrf52840.dts @@ -335,8 +335,10 @@ * The part is an RX8900 second-source (same 3.2x2.5x1.0 package, same * 32.768 kHz TCXO grades), so it uses the RX8900 register layout rather than * the RX8130CE one that rtc-i2c.dtsi assumes for address 0x32: time at 0x00, - * FLAG at 0x0E, VLF in bit 1. Boot-time discovery BCD-validates what it reads, - * so a wrong guess here is detected and skipped, not silently trusted. */ + * FLAG at 0x0E, VLF in bit 1. Boot-time discovery passes a device over when + * two reads both show seconds, minutes, date or month out of range with VLF + * not read as set (clear, or the FLAG read failed), so most wrong guesses + * are skipped; there is no zero-mask here. */ &i2c1 { compatible = "nordic,nrf-twim"; status = "okay"; diff --git a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml index 92f2e92..7e6643c 100644 --- a/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml +++ b/zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml @@ -41,3 +41,13 @@ properties: type: int required: true description: Bit mask within status-reg that means the held time is unreliable. + + zero-mask: + type: uint8-array + description: > + Seven bytes, one per register of the time block in block order: the + bits the chip's data sheet shows as reading 0. A device that + shows one of them, or seconds, minutes, date or month out of range + while the power-loss flag does not read as set, on two reads, is not + this RTC and is never adopted. Leave it out for a chip whose unused + bits are not documented as reading 0.