Hi,

> You really need to update the “should skip” logic for this.  It should 
> probably be changed to something along the lines of:
> 
> - If todr handle has NULL dev, consider it to be the authoritative TODR (it 
> is likely backed by a CPU register [vax] or a firmware call [like EFI]).
> 
> - Otherwise, If the DEVICE_IS_SYSTEM_TODR device call returns true, the 
> consider it to be the authoritative TODR.
> 
> - The authoiritative TODR should be at the head of the list, everything else 
> added in disovered order after.
> 
> IMO, it would be better to not dynamically allocate memory for this.  Just 
> but the list linkage in the todr handle and bump the kernel version?

Thanks for the comments!  How about something closer to the attached?

I replaced the "should skip" logic with a todr_is_system() check instead.
If an RTC is probed after the first, and this returns true, then that RTC
is moved to the head of the list.

I don't have any machines with no device backing the RTC, nor with a
binding for DEVICE_IS_SYSTEM_TODR to test against though.  Looking at the
code for DEVICE_IS_SYSTEM_TODR, I wonder about extending it for machines
which don't use device tree.  However, this would probably have to be
something in the individual drivers configured from a kernel flag or based
on a firmware property.  Would that make sense?

Does it need a kernel version bump?  None of the externally visible
functions have changed signatures.

With this patch, dmesg from a Sun V245 with 2 RTC's looks like:

  [     1.000000] rtc0 at ebus0 addr 70-73: mc146818 compatible time-of-day 
clock: m5823
    ...
  [     1.000000] dsrtc0 at iic0 addr 0x68: DS1307 Real-time Clock/NVRAM
  [     1.000000] dsrtc0: secondary clock

Regards,

Julian

 - - - - - 8< - - - - - - - - - - - - - - - - - - - - - - - - - 8< - - - - - -

Index: src/sys/kern/kern_todr.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_todr.c,v
retrieving revision 1.52
diff -u -r1.52 kern_todr.c
--- src/sys/kern/kern_todr.c    4 Jan 2026 02:09:03 -0000       1.52
+++ src/sys/kern/kern_todr.c    22 Sep 2026 09:50:02 -0000
@@ -78,6 +78,7 @@
 #include <sys/device_calls.h>
 #include <sys/intr.h>
 #include <sys/kernel.h>
+#include <sys/kmem.h>
 #include <sys/mutex.h>
 #include <sys/rndsource.h>
 #include <sys/sdt.h>
@@ -86,11 +87,18 @@
 
 #include <dev/clock_subr.h>    /* hmm.. this should probably move to sys */
 
+struct todr_list {
+       todr_chip_handle_t todr_handle;
+       struct todr_list *todr_next;
+};
+static const char default_name[] = "internal ";
+
 static int todr_gettime(todr_chip_handle_t, struct timeval *);
 static int todr_settime(todr_chip_handle_t, struct timeval *);
 
 static kmutex_t todr_mutex;
-static todr_chip_handle_t todr_handle;
+static struct todr_list todr_head =
+       { .todr_handle = NULL, .todr_next = NULL };
 static bool todr_initialized;
 
 /* The minimum reasonable RTC date before preposterousness */
@@ -143,33 +151,32 @@
 }
 
 /*
- * todr_should_skip:
- *     Evaluate if we should skip attaching the clock device
- *     specified by the todr handle.
+ * todr_is_system:
+ *     Evaluate if the clock device specified by the todr handle
+ *     is the system clock.
  */
 static bool
-todr_should_skip(todr_chip_handle_t todr)
+todr_is_system(todr_chip_handle_t todr)
 {
        device_t dev = todr->todr_dev;
        struct device_is_system_todr_args args = { };
 
-       /* No basis for evaluation if no device backs the TODR. */
+       /* Assume this is the system clock if no device backs the TODR. */
        if (dev == NULL) {
-               return false;
+               return true;
        }
 
        if (device_call(dev, DEVICE_IS_SYSTEM_TODR(&args)) != 0) {
-               /* Call is not supported; proceed as if we should attach. */
+               /* Call is not supported. */
                return false;
        }
 
        if (args.result) {
-               /* This is the system TODR; proceed. */
-               return false;
+               /* This is the system TODR. */
+               return true;
        }
 
-       aprint_normal_dev(dev, "disabled (not system TODR)\n");
-       return true;
+       return false;
 }
 
 /*
@@ -179,6 +186,9 @@
 void
 todr_attach(todr_chip_handle_t todr)
 {
+       struct todr_list *todr_newp, *todr_iterp;
+       bool is_system;
+       const char *dev_name;
 
        /*
         * todr_init() is called very early in main(), but this is
@@ -187,18 +197,40 @@
         */
        KASSERT(todr_initialized);
 
-       if (todr_should_skip(todr)) {
-               return;
-       }
+       is_system = todr_is_system(todr);
 
-       todr_lock();
-       if (todr_handle) {
+       /* Initial entry is static, others allocated. */
+       if (todr_head.todr_handle == NULL) {
+               todr_lock();
+               todr_head.todr_handle = todr;
                todr_unlock();
-               printf("todr_attach: TOD already configured\n");
-               return;
+       } else {
+               todr_newp = kmem_alloc(sizeof(struct todr_list),
+                   KM_SLEEP);
+               todr_newp->todr_next = NULL;
+               todr_lock();
+               /* Move the system TODR to the head of the list */
+               if (is_system) {
+                       todr_newp->todr_handle = todr_head.todr_handle;
+                       todr_head.todr_handle = todr;
+               } else {
+                       todr_newp->todr_handle = todr;
+               }
+               todr_iterp = &todr_head;
+               while (todr_iterp->todr_next != NULL)
+                       todr_iterp = todr_iterp->todr_next;
+               todr_iterp->todr_next = todr_newp;
+               todr_unlock();
+                       
+               if (todr->todr_dev == NULL)
+                       dev_name = default_name;
+               else
+                       dev_name = device_xname(todr->todr_dev);
+               if (is_system)
+                       printf("%s: system clock\n", dev_name);
+               else
+                       printf("%s: secondary clock\n", dev_name);
        }
-       todr_handle = todr;
-       todr_unlock();
 }
 
 static bool timeset = false;
@@ -217,7 +249,11 @@
        bool goodtime = false;
        bool badrtc = false;
        struct timespec ts;
-       struct timeval tv;
+       struct timeval tv, tv2;
+       struct todr_list *todr_iterp;
+       device_t t_dev;
+       const char *dev_name;
+       time_t deltat;
 
        KASSERT(todr_lock_owned());
 
@@ -246,22 +282,23 @@
        /*
         * Some ports need to be supplied base in order to fabricate a time_t.
         */
-       if (todr_handle)
-               todr_handle->todr_base_time = base;
+       if (todr_head.todr_handle != NULL)
+               todr_head.todr_handle->todr_base_time = base;
 
        memset(&tv, 0, sizeof(tv));
 
-       if ((todr_handle == NULL) ||
-           (todr_gettime(todr_handle, &tv) != 0) ||
+       /* Note, that we use the first RTC for system time. */
+       if ((todr_head.todr_handle == NULL) ||
+           (todr_gettime(todr_head.todr_handle, &tv) != 0) ||
            (tv.tv_sec < (PREPOSTEROUS_YEARS * SECS_PER_COMMON_YEAR))) {
 
-               if (todr_handle != NULL)
+               if (todr_head.todr_handle != NULL)
                        printf("WARNING: preposterous TOD clock time\n");
                else
                        printf("WARNING: no TOD clock present\n");
                badrtc = true;
        } else {
-               time_t deltat = tv.tv_sec - base;
+               deltat = tv.tv_sec - base;
 
                if (deltat < 0)
                        deltat = -deltat;
@@ -301,6 +338,33 @@
                tv.tv_usec = 0;
        }
 
+       /* Check other clocks, if any, against our time */
+       todr_iterp = &todr_head;
+       while ((todr_iterp = todr_iterp->todr_next) != NULL) {
+               if ((todr_iterp->todr_handle == NULL) || 
+                   (todr_gettime(todr_iterp->todr_handle, &tv2) != 0))
+                       continue;
+               t_dev = todr_iterp->todr_handle->todr_dev;
+               if (t_dev == NULL)
+                       dev_name = default_name;
+               else
+                       dev_name = device_xname(t_dev);
+               if (tv2.tv_sec < (PREPOSTEROUS_YEARS * SECS_PER_COMMON_YEAR)) {
+                       printf("WARNING: preposterous %s clock time\n",
+                           dev_name);
+                       deltat = 0;
+               } else {
+                       deltat = tv.tv_sec - tv2.tv_sec;
+                       rnd_add_data(NULL, &tv, sizeof(tv), 0);
+               }
+               if (deltat < -10)
+                       printf("WARNING: %s clock lost %" PRId64 " seconds\n",
+                           dev_name, deltat);
+               if (deltat > 10)
+                       printf("WARNING: %s clock gained %" PRId64 " seconds\n",
+                           dev_name, deltat);
+       }
+
        timeset = true;
 
        ts.tv_sec = tv.tv_sec;
@@ -315,13 +379,16 @@
 
 /*
  * todr_save_systime:
- *     Save the current system time back to the TOD clock.
+ *     Save the current system time back to the TOD clock(s).
  *     Must be called with the TODR lock held.
  */
 void
 todr_save_systime(void)
 {
        struct timeval tv;
+       struct todr_list *todr_iterp;
+       device_t t_dev;
+       const char *dev_name;
 
        KASSERT(todr_lock_owned());
 
@@ -338,9 +405,19 @@
        if (tv.tv_sec == 0)
                return;
 
-       if (todr_handle)
-               if (todr_settime(todr_handle, &tv) != 0)
-                       printf("Cannot set TOD clock time\n");
+       todr_iterp = &todr_head;
+       do {
+               if ((todr_iterp->todr_handle != NULL) &&
+                   (todr_settime(todr_iterp->todr_handle, &tv) != 0)) {
+                       t_dev = todr_iterp->todr_handle->todr_dev;
+                       if (t_dev == NULL)
+                               dev_name = default_name;
+                       else
+                               dev_name = device_xname(t_dev);
+                       printf("Cannot set %s TOD clock time\n", dev_name);
+               }
+               todr_iterp = todr_iterp->todr_next;
+       } while (todr_iterp != NULL);
 }
 
 /*

-- 

Reply via email to