After some input from bde@ and rereading the datasheet use a MTX_SPIN

mutex instead of a MTX_DEF one in order to defer preemption while
reading the date and time registers. If we don't manage to read them
within the time slot where we are guaranteed that no updates occur we
might actually read them during an update in which case the output is
undefined.

3 files changed, 13 insertions, 12 deletions

diff --git a/sys/dev/mc146818/mc146818.c b/sys/dev/mc146818/mc146818.c
index 6e6d478..3bd4ce3 100644
--- a/sys/dev/mc146818/mc146818.c
+++ b/sys/dev/mc146818/mc146818.c
@@ -80,9 +80,9 @@ mc146818_attach(device_t dev)
 			sc->sc_setcent = mc146818_def_setcent;
 	}
 
-	mtx_lock(&sc->sc_mtx);
+	mtx_lock_spin(&sc->sc_mtx);
 	if (!(*sc->sc_mcread)(dev, MC_REGD) & MC_REGD_VRT) {
-		mtx_unlock(&sc->sc_mtx);
+		mtx_unlock_spin(&sc->sc_mtx);
 		device_printf(dev, "%s: battery low\n", __func__);
 		return (ENXIO);
 	}
@@ -94,7 +94,7 @@ mc146818_attach(device_t dev)
 	sc->sc_regb |= (sc->sc_flag & MC146818_BCD) ? 0 : MC_REGB_BINARY;
 	sc->sc_regb |= (sc->sc_flag & MC146818_12HR) ? 0 : MC_REGB_24HR;
 	(*sc->sc_mcwrite)(dev, MC_REGB, sc->sc_regb);
-	mtx_unlock(&sc->sc_mtx);
+	mtx_unlock_spin(&sc->sc_mtx);
 
 	clock_register(dev, 1000000);	/* 1 second resolution. */
 
@@ -116,16 +116,16 @@ mc146818_gettime(device_t dev, struct timespec *ts)
 
 	timeout = 1000000;	/* XXX how long should we wait? */
 
-	mtx_lock(&sc->sc_mtx);
 	/*
 	 * If MC_REGA_UIP is 0 we have at least 244us before the next
 	 * update. If it's 1 an update is imminent.
 	 */
 	for (;;) {
+		mtx_lock_spin(&sc->sc_mtx);
 		if (!((*sc->sc_mcread)(dev, MC_REGA) & MC_REGA_UIP))
 			break;
+		mtx_unlock_spin(&sc->sc_mtx);
 		if (--timeout < 0) {
-			mtx_unlock(&sc->sc_mtx);
 			device_printf(dev, "%s: timeout\n", __func__);
 			return (EBUSY);
 		}
@@ -148,7 +148,7 @@ mc146818_gettime(device_t dev, struct timespec *ts)
 		year += cent * 100;
 	} else if (year < POSIX_BASE_YEAR)
 		year += 100;
-	mtx_unlock(&sc->sc_mtx);
+	mtx_unlock_spin(&sc->sc_mtx);
 
 	ct.year = year;
 
@@ -166,19 +166,19 @@ mc146818_getsecs(device_t dev, int *secp)
 
 	timeout = 1000000;	/* XXX how long should we wait? */
 
-	mtx_lock(&sc->sc_mtx);
 	for (;;) {
+		mtx_lock_spin(&sc->sc_mtx);
 		if (!((*sc->sc_mcread)(dev, MC_REGA) & MC_REGA_UIP)) {
 			sec = FROMREG((*sc->sc_mcread)(dev, MC_SEC));
+			mtx_unlock_spin(&sc->sc_mtx);
 			break;
 		}
+		mtx_unlock_spin(&sc->sc_mtx);
 		if (--timeout == 0) {
-			mtx_unlock(&sc->sc_mtx);
 			device_printf(dev, "%s: timeout\n", __func__);
 			return (EBUSY);
 		}
 	}
-	mtx_unlock(&sc->sc_mtx);
 
 #undef FROMREG
 
@@ -206,7 +206,7 @@ mc146818_settime(device_t dev, struct timespec *ts)
 	ts->tv_nsec = 0;
 	clock_ts_to_ct(ts, &ct);
 
-	mtx_lock(&sc->sc_mtx);
+	mtx_lock_spin(&sc->sc_mtx);
 	/* Disable RTC updates and interrupts (if enabled). */
 	(*sc->sc_mcwrite)(dev, MC_REGB,
 	    ((sc->sc_regb & (MC_REGB_BINARY | MC_REGB_24HR)) | MC_REGB_SET));
@@ -232,7 +232,7 @@ mc146818_settime(device_t dev, struct timespec *ts)
 
 	/* Reenable RTC updates and interrupts. */
 	(*sc->sc_mcwrite)(dev, MC_REGB, sc->sc_regb);
-	mtx_unlock(&sc->sc_mtx);
+	mtx_unlock_spin(&sc->sc_mtx);
 
 #undef TOREG
 
diff --git a/sys/kern/subr_witness.c b/sys/kern/subr_witness.c
index 7c71f53..082c8b8 100644
--- a/sys/kern/subr_witness.c
+++ b/sys/kern/subr_witness.c
@@ -375,6 +375,7 @@ static struct witness_order_list_entry order_lists[] = {
 #endif
 #ifdef __sparc64__
 	{ "ipi", &lock_class_mtx_spin },
+	{ "rtc_mtx", &lock_class_mtx_spin },
 #endif
 #endif
 	{ "clk", &lock_class_mtx_spin },
diff --git a/sys/sparc64/sparc64/rtc.c b/sys/sparc64/sparc64/rtc.c
index 234d1b7..be8e009 100644
--- a/sys/sparc64/sparc64/rtc.c
+++ b/sys/sparc64/sparc64/rtc.c
@@ -149,7 +149,7 @@ rtc_attach(device_t dev)
 	sc = device_get_softc(dev);
 	bzero(sc, sizeof(struct mc146818_softc));
 
-	mtx_init(&sc->sc_mtx, "rtc_mtx", NULL, MTX_DEF);
+	mtx_init(&sc->sc_mtx, "rtc_mtx", NULL, MTX_SPIN);
 
 	if (strcmp(device_get_name(device_get_parent(dev)), "isa") == 0)
 		rtype = SYS_RES_IOPORT;