RTC probe: identify RTCs by documented always-zero bits, stop skipping real ones (iteration branch)

zephcore/adapters/clock/ZephyrRTCDiscover.c

@@ -17,6 +17,8 @@
 #include <zephyr/drivers/i2c.h>
 #include <zephyr/logging/log.h>
 
+#include <string.h>
+
 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,63 +114,139 @@ 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;
 	}
 
 	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;

zephcore/adapters/clock/ZephyrRTCDiscover.h

@@ -23,18 +23,29 @@ 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);
 
 /*
  * 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);
 

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];
 };

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";

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.