Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 13 Apr 2005 10:18:33 -0400
From:      Adam Gregoire <ebola@psychoholics.org>
To:        "Matthew N. Dodd" <mdodd@FreeBSD.org>
Cc:        cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/sys/i386/isa clock.c
Message-ID:  <1113401913.838.0.camel@S010600deadc0de00.su.shawcable.net>
In-Reply-To: <200504122049.j3CKnV9v071004@repoman.freebsd.org>
References:  <200504122049.j3CKnV9v071004@repoman.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--=-D2UC1HCBeWdBxtbsjtTR
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

>  Modified files:
>     sys/i386/isa         clock.c 
>   Log:
>   Replace spl protection in rtcin() and writertc() with spinlocks
>   using the existing clock_lock mutex.
>   
>   Revision  Changes    Path
>   1.219     +6 -6      src/sys/i386/isa/clock.c

Hello, I have made a patch that brings these changes into amd64's
isa/clock.c, and it also contains changes for style consistancy,
whitespace, and use of prototypes on both amd64 and i386. It should even
reduce diffs a bit. I have tested this on amd64 kernel as of an hour
ago. All seems to be fine here.

Hopefully this is deemed useful. If not are you able to just get the
amd64 changes for locking?

Thank you,

-- 
Adam Gregoire <ebola@psychoholics.org>

--=-D2UC1HCBeWdBxtbsjtTR
Content-Disposition: attachment; filename=clock.diff
Content-Type: text/x-patch; name=clock.diff; charset=UTF-8
Content-Transfer-Encoding: 7bit

--- sys/amd64/isa/clock.c.orig	Wed Apr 13 01:16:47 2005
+++ sys/amd64/isa/clock.c	Wed Apr 13 01:55:42 2005
@@ -84,10 +84,10 @@
  * 32-bit time_t's can't reach leap years before 1904 or after 2036, so we
  * can use a simple formula for leap years.
  */
-#define	LEAPYEAR(y) (((u_int)(y) % 4 == 0) ? 1 : 0)
-#define DAYSPERYEAR   (31+28+31+30+31+30+31+31+30+31+30+31)
+#define	LEAPYEAR(y)	(((u_int)(y) % 4 == 0) ? 1 : 0)
+#define	DAYSPERYEAR	(31+28+31+30+31+30+31+31+30+31+30+31)
 
-#define	TIMER_DIV(x) ((timer_freq + (x) / 2) / (x))
+#define	TIMER_DIV(x)	((timer_freq + (x) / 2) / (x))
 
 int	adjkerntz;		/* local offset from GMT in seconds */
 int	clkintr_pending;
@@ -96,12 +96,14 @@
 int	psdiv = 1;
 int	statclock_disable;
 #ifndef TIMER_FREQ
-#define TIMER_FREQ   1193182
+#define	TIMER_FREQ	1193182
 #endif
 u_int	timer_freq = TIMER_FREQ;
 int	timer0_max_count;
 int	wall_cmos_clock;	/* wall CMOS clock assumed if != 0 */
 struct mtx clock_lock;
+#define	RTC_LOCK	mtx_lock_spin(&clock_lock)
+#define	RTC_UNLOCK	mtx_unlock_spin(&clock_lock)
 
 static	int	beeping = 0;
 static	const u_char daysinmonth[] = {31,28,31,30,31,30,31,31,30,31,30,31};
@@ -175,7 +177,7 @@
 }
 
 int
-release_timer2()
+release_timer2(void)
 {
 
 	if (timer2_state != ACQUIRED)
@@ -228,9 +230,9 @@
 DB_SHOW_COMMAND(rtc, rtc)
 {
 	printf("%02x/%02x/%02x %02x:%02x:%02x, A = %02x, B = %02x, C = %02x\n",
-	       rtcin(RTC_YEAR), rtcin(RTC_MONTH), rtcin(RTC_DAY),
-	       rtcin(RTC_HRS), rtcin(RTC_MIN), rtcin(RTC_SEC),
-	       rtcin(RTC_STATUSA), rtcin(RTC_STATUSB), rtcin(RTC_INTR));
+	    rtcin(RTC_YEAR), rtcin(RTC_MONTH), rtcin(RTC_DAY),
+	    rtcin(RTC_HRS), rtcin(RTC_MIN), rtcin(RTC_SEC),
+	    rtcin(RTC_STATUSA), rtcin(RTC_STATUSB), rtcin(RTC_INTR));
 }
 #endif /* DDB */
 
@@ -323,7 +325,7 @@
 		 * before the delay is up (unless we're interrupted).
 		 */
 		ticks_left = ((u_int)n * (long long)timer_freq + 999999)
-			     / 1000000;
+		    / 1000000;
 
 	while (ticks_left > 0) {
 #ifdef KDB
@@ -356,7 +358,7 @@
 #ifdef DELAYDEBUG
 	if (state == 1)
 		printf(" %d calls to getit() at %d usec each\n",
-		       getit_calls, (n + 5) / getit_calls);
+		    getit_calls, (n + 5) / getit_calls);
 #endif
 }
 
@@ -398,33 +400,30 @@
  */
 
 int
-rtcin(reg)
-	int reg;
+rtcin(int reg)
 {
-	int s;
 	u_char val;
 
-	s = splhigh();
+	RTC_LOCK;
 	outb(IO_RTC, reg);
 	inb(0x84);
 	val = inb(IO_RTC + 1);
 	inb(0x84);
-	splx(s);
+	RTC_UNLOCK;
 	return (val);
 }
 
 static __inline void
 writertc(u_char reg, u_char val)
 {
-	int s;
 
-	s = splhigh();
+	RTC_LOCK;
 	inb(0x84);
 	outb(IO_RTC, reg);
 	inb(0x84);
 	outb(IO_RTC + 1, val);
 	inb(0x84);		/* XXX work around wrong order in rtcin() */
-	splx(s);
+	RTC_UNLOCK;
 }
 
 static __inline int
@@ -501,15 +500,14 @@
 			goto fail;
 	}
 
-	if (bootverbose) {
+	if (bootverbose)
 	        printf("i8254 clock: %u Hz\n", tot_count);
-	}
 	return (tot_count);
 
 fail:
 	if (bootverbose)
 	        printf("failed, using default i8254 clock of %u Hz\n",
-		       timer_freq);
+		    timer_freq);
 	return (timer_freq);
 }
 
@@ -535,7 +533,7 @@
  * XXX initialization of other timers is unintentionally left blank.
  */
 void
-startrtclock()
+startrtclock(void)
 {
 	u_int delta, freq;
 
@@ -547,7 +545,7 @@
 #ifdef CLK_CALIBRATION_LOOP
 	if (bootverbose) {
 		printf(
-		"Press a key on the console to abort clock calibration\n");
+		    "Press a key on the console to abort clock calibration\n");
 		while (cncheckc() == -1)
 			calibrate_clocks();
 	}
@@ -562,18 +560,16 @@
 	if (delta < timer_freq / 100) {
 #ifndef CLK_USE_I8254_CALIBRATION
 		if (bootverbose)
-			printf(
-"CLK_USE_I8254_CALIBRATION not specified - using default frequency\n");
+			printf("CLK_USE_I8254_CALIBRATION not specified - "
+			    "using default frequency\n");
 		freq = timer_freq;
 #endif
 		timer_freq = freq;
-	} else {
+	} else
 		if (bootverbose)
-			printf(
-		    "%d Hz differs from default of %d Hz by more than 1%%\n",
-			       freq, timer_freq);
-	}
-
+			printf("%d Hz differs from default of %d Hz by more "
+			    "than 1%%\n", freq, timer_freq);
+	
 	set_timer_freq(timer_freq, hz);
 	i8254_timecounter.tc_frequency = timer_freq;
 	tc_init(&i8254_timecounter);
@@ -633,10 +629,8 @@
 	days += readrtc(RTC_DAY) - 1;
 	for (y = 1970; y < year; y++)
 		days += DAYSPERYEAR + LEAPYEAR(y);
-	sec = ((( days * 24 +
-		  readrtc(RTC_HRS)) * 60 +
-		  readrtc(RTC_MIN)) * 60 +
-		  readrtc(RTC_SEC));
+	sec = (((days * 24 +
+	    readrtc(RTC_HRS)) * 60 + readrtc(RTC_MIN)) * 60 + readrtc(RTC_SEC));
 	/* sec now contains the number of seconds, since Jan 1 1970,
 	   in the local time zone */
 
@@ -653,18 +647,19 @@
 	return;
 
 wrong_time:
-	printf("Invalid time in real time clock.\n");
-	printf("Check and reset the date immediately!\n");
+	printf("Invalid time in real time clock.\n"
+	    "Check and reset the date immediately!\n");
 }
 
 /*
  * Write system time back to RTC
  */
 void
-resettodr()
+resettodr(void)
 {
 	unsigned long	tm;
 	int		y, m, s;
+	int		ml;
 
 	if (disable_rtc_set)
 		return;
@@ -697,8 +692,6 @@
 	writertc(RTC_CENTURY, bin2bcd(y/100));		/* ... and Century    */
 #endif
 	for (m = 0; ; m++) {
-		int ml;
-
 		ml = daysinmonth[m];
 		if (m == 1 && LEAPYEAR(y))
 			ml++;
@@ -720,7 +713,7 @@
  * Start both clocks running.
  */
 void
-cpu_initclocks()
+cpu_initclocks(void)
 {
 	int diag;
 
@@ -752,9 +745,9 @@
 
 	/* Don't bother enabling the statistics clock. */
 	if (!statclock_disable && !using_lapic_timer) {
-		diag = rtcin(RTC_DIAG);
-		if (diag != 0)
-			printf("RTC BIOS diagnostic error %b\n", diag, RTCDG_BITS);
+		if ((diag = rtcin(RTC_DIAG)) != 0)
+			printf("RTC BIOS diagnostic error %b\n",
+			    diag, RTCDG_BITS);
 
 		intr_add_handler("rtc", 8, (driver_intr_t *)rtcintr, NULL,
 		    INTR_TYPE_CLK | INTR_FAST, NULL);
@@ -772,8 +765,7 @@
 
 	if (using_lapic_timer)
 		return;
-	rtc_statusa = RTCSA_DIVIDER | RTCSA_PROF;
-	writertc(RTC_STATUSA, rtc_statusa);
+	writertc(RTC_STATUSA, RTCSA_DIVIDER | RTCSA_PROF);
 	psdiv = pscnt = psratio;
 }
 
@@ -783,8 +775,7 @@
 
 	if (using_lapic_timer)
 		return;
-	rtc_statusa = RTCSA_DIVIDER | RTCSA_NOPROF;
-	writertc(RTC_STATUSA, rtc_statusa);
+	writertc(RTC_STATUSA, RTCSA_DIVIDER | RTCSA_NOPROF);
 	psdiv = pscnt = 1;
 }
 
@@ -799,25 +790,23 @@
 	 * is is too generic.  Should use it everywhere.
 	 */
 	freq = timer_freq;
-	error = sysctl_handle_int(oidp, &freq, sizeof(freq), req);
-	if (error == 0 && req->newptr != NULL) {
+	if ((error = sysctl_handle_int(oidp, &freq, sizeof(freq), req)) == 0 &&
+	    req->newptr != NULL) {
 		set_timer_freq(freq, hz);
 		i8254_timecounter.tc_frequency = freq;
 	}
 	return (error);
 }
 
-SYSCTL_PROC(_machdep, OID_AUTO, i8254_freq, CTLTYPE_INT | CTLFLAG_RW,
-    0, sizeof(u_int), sysctl_machdep_i8254_freq, "IU", "");
+SYSCTL_PROC(_machdep, OID_AUTO, i8254_freq, CTLTYPE_INT | CTLFLAG_RW, 0,
+    sizeof(u_int), sysctl_machdep_i8254_freq, "IU", "");
 
 static unsigned
 i8254_get_timecount(struct timecounter *tc)
 {
 	u_int count;
 	u_int high, low;
-	u_long rflags;
 
-	rflags = read_rflags();
 	mtx_lock_spin(&clock_lock);
 
 	/* Select timer0 and latch counter value. */
@@ -826,10 +815,10 @@
 	low = inb(TIMER_CNTR0);
 	high = inb(TIMER_CNTR0);
 	count = timer0_max_count - ((high << 8) | low);
-	if (count < i8254_lastcount ||
-	    (!i8254_ticked && (clkintr_pending ||
-	    ((count < 20 || (!(rflags & PSL_I) && count < timer0_max_count / 2u)) &&
-	    i8254_pending != NULL && i8254_pending(i8254_intsrc))))) {
+	if (count < i8254_lastcount || (!i8254_ticked && (clkintr_pending ||
+	    ((count < 20 || (!(read_rflags() & PSL_I) &&
+	    count < timer0_max_count / 2u)) && i8254_pending != NULL &&
+	    i8254_pending(i8254_intsrc))))) {
 		i8254_ticked = 1;
 		i8254_offset += timer0_max_count;
 	}
@@ -852,11 +841,12 @@
 static int
 attimer_probe(device_t dev)
 {
-	int result;
+	int res;
 	
-	if ((result = ISA_PNP_PROBE(device_get_parent(dev), dev, attimer_ids)) <= 0)
+	if ((res =
+	    ISA_PNP_PROBE(device_get_parent(dev), dev, attimer_ids) <= 0))
 		device_quiet(dev);
-	return(result);
+	return(res);
 }
 
 static int
--- sys/i386/isa/clock.c.orig	Wed Apr 13 01:16:51 2005
+++ sys/i386/isa/clock.c	Wed Apr 13 01:55:54 2005
@@ -93,10 +93,10 @@
  * 32-bit time_t's can't reach leap years before 1904 or after 2036, so we
  * can use a simple formula for leap years.
  */
-#define	LEAPYEAR(y) (((u_int)(y) % 4 == 0) ? 1 : 0)
-#define DAYSPERYEAR   (31+28+31+30+31+30+31+31+30+31+30+31)
+#define	LEAPYEAR(y)	(((u_int)(y) % 4 == 0) ? 1 : 0)
+#define	DAYSPERYEAR	(31+28+31+30+31+30+31+31+30+31+30+31)
 
-#define	TIMER_DIV(x) ((timer_freq + (x) / 2) / (x))
+#define	TIMER_DIV(x)	((timer_freq + (x) / 2) / (x))
 
 int	adjkerntz;		/* local offset from GMT in seconds */
 int	clkintr_pending;
@@ -105,7 +105,7 @@
 int	psdiv = 1;
 int	statclock_disable;
 #ifndef TIMER_FREQ
-#define TIMER_FREQ   1193182
+#define	TIMER_FREQ	1193182
 #endif
 u_int	timer_freq = TIMER_FREQ;
 int	timer0_max_count;
@@ -191,7 +191,7 @@
 }
 
 int
-release_timer2()
+release_timer2(void)
 {
 
 	if (timer2_state != ACQUIRED)
@@ -244,9 +244,9 @@
 DB_SHOW_COMMAND(rtc, rtc)
 {
 	printf("%02x/%02x/%02x %02x:%02x:%02x, A = %02x, B = %02x, C = %02x\n",
-	       rtcin(RTC_YEAR), rtcin(RTC_MONTH), rtcin(RTC_DAY),
-	       rtcin(RTC_HRS), rtcin(RTC_MIN), rtcin(RTC_SEC),
-	       rtcin(RTC_STATUSA), rtcin(RTC_STATUSB), rtcin(RTC_INTR));
+	    rtcin(RTC_YEAR), rtcin(RTC_MONTH), rtcin(RTC_DAY),
+	    rtcin(RTC_HRS), rtcin(RTC_MIN), rtcin(RTC_SEC),
+	    rtcin(RTC_STATUSA), rtcin(RTC_STATUSB), rtcin(RTC_INTR));
 }
 #endif /* DDB */
 
@@ -339,7 +339,7 @@
 		 * before the delay is up (unless we're interrupted).
 		 */
 		ticks_left = ((u_int)n * (long long)timer_freq + 999999)
-			     / 1000000;
+		    / 1000000;
 
 	while (ticks_left > 0) {
 #ifdef KDB
@@ -372,7 +372,7 @@
 #ifdef DELAYDEBUG
 	if (state == 1)
 		printf(" %d calls to getit() at %d usec each\n",
-		       getit_calls, (n + 5) / getit_calls);
+		    getit_calls, (n + 5) / getit_calls);
 #endif
 }
 
@@ -414,8 +414,7 @@
  */
 
 int
-rtcin(reg)
-	int reg;
+rtcin(int reg)
 {
 	u_char val;
 
@@ -515,15 +514,14 @@
 			goto fail;
 	}
 
-	if (bootverbose) {
+	if (bootverbose)
 	        printf("i8254 clock: %u Hz\n", tot_count);
-	}
 	return (tot_count);
 
 fail:
 	if (bootverbose)
 	        printf("failed, using default i8254 clock of %u Hz\n",
-		       timer_freq);
+		    timer_freq);
 	return (timer_freq);
 }
 
@@ -587,7 +585,7 @@
  * XXX initialization of other timers is unintentionally left blank.
  */
 void
-startrtclock()
+startrtclock(void)
 {
 	u_int delta, freq;
 
@@ -599,7 +597,7 @@
 #ifdef CLK_CALIBRATION_LOOP
 	if (bootverbose) {
 		printf(
-		"Press a key on the console to abort clock calibration\n");
+		    "Press a key on the console to abort clock calibration\n");
 		while (cncheckc() == -1)
 			calibrate_clocks();
 	}
@@ -614,17 +612,15 @@
 	if (delta < timer_freq / 100) {
 #ifndef CLK_USE_I8254_CALIBRATION
 		if (bootverbose)
-			printf(
-"CLK_USE_I8254_CALIBRATION not specified - using default frequency\n");
+			printf("CLK_USE_I8254_CALIBRATION not specified - "
+			    "using default frequency\n");
 		freq = timer_freq;
 #endif
 		timer_freq = freq;
-	} else {
+	} else
 		if (bootverbose)
-			printf(
-		    "%d Hz differs from default of %d Hz by more than 1%%\n",
-			       freq, timer_freq);
-	}
+			printf("%d Hz differs from default of %d Hz by more "
+			    "than 1%%\n", freq, timer_freq);
 
 	set_timer_freq(timer_freq, hz);
 	i8254_timecounter.tc_frequency = timer_freq;
@@ -705,18 +701,19 @@
 	return;
 
 wrong_time:
-	printf("Invalid time in real time clock.\n");
-	printf("Check and reset the date immediately!\n");
+	printf("Invalid time in real time clock.\n"
+	    "Check and reset the date immediately!\n");
 }
 
 /*
  * Write system time back to RTC
  */
 void
-resettodr()
+resettodr(void)
 {
 	unsigned long	tm;
 	int		y, m, s;
+	int		ml;
 
 	if (disable_rtc_set)
 		return;
@@ -749,8 +746,6 @@
 	writertc(RTC_CENTURY, bin2bcd(y/100));		/* ... and Century    */
 #endif
 	for (m = 0; ; m++) {
-		int ml;
-
 		ml = daysinmonth[m];
 		if (m == 1 && LEAPYEAR(y))
 			ml++;
@@ -772,7 +767,7 @@
  * Start both clocks running.
  */
 void
-cpu_initclocks()
+cpu_initclocks(void)
 {
 	int diag;
 
@@ -804,9 +799,9 @@
 	 * drive statclock() and profclock().
 	 */
 	if (!statclock_disable && !using_lapic_timer) {
-		diag = rtcin(RTC_DIAG);
-		if (diag != 0)
-			printf("RTC BIOS diagnostic error %b\n", diag, RTCDG_BITS);
+		if ((diag = rtcin(RTC_DIAG)) != 0)
+			printf("RTC BIOS diagnostic error %b\n",
+			    diag, RTCDG_BITS);
 
 	        /* Setting stathz to nonzero early helps avoid races. */
 		stathz = RTC_NOPROFRATE;
@@ -830,8 +825,7 @@
 
 	if (using_lapic_timer)
 		return;
-	rtc_statusa = RTCSA_DIVIDER | RTCSA_PROF;
-	writertc(RTC_STATUSA, rtc_statusa);
+	writertc(RTC_STATUSA, RTCSA_DIVIDER | RTCSA_PROF);
 	psdiv = pscnt = psratio;
 }
 
@@ -841,8 +835,7 @@
 
 	if (using_lapic_timer)
 		return;
-	rtc_statusa = RTCSA_DIVIDER | RTCSA_NOPROF;
-	writertc(RTC_STATUSA, rtc_statusa);
+	writertc(RTC_STATUSA, RTCSA_DIVIDER | RTCSA_NOPROF);
 	psdiv = pscnt = 1;
 }
 
@@ -857,25 +850,23 @@
 	 * is is too generic.  Should use it everywhere.
 	 */
 	freq = timer_freq;
-	error = sysctl_handle_int(oidp, &freq, sizeof(freq), req);
-	if (error == 0 && req->newptr != NULL) {
+	if ((error = sysctl_handle_int(oidp, &freq, sizeof(freq), req) == 0) &&
+	    req->newptr != NULL) {
 		set_timer_freq(freq, hz);
 		i8254_timecounter.tc_frequency = freq;
 	}
 	return (error);
 }
 
-SYSCTL_PROC(_machdep, OID_AUTO, i8254_freq, CTLTYPE_INT | CTLFLAG_RW,
-    0, sizeof(u_int), sysctl_machdep_i8254_freq, "IU", "");
+SYSCTL_PROC(_machdep, OID_AUTO, i8254_freq, CTLTYPE_INT | CTLFLAG_RW, 0,
+    sizeof(u_int), sysctl_machdep_i8254_freq, "IU", "");
 
 static unsigned
 i8254_get_timecount(struct timecounter *tc)
 {
 	u_int count;
 	u_int high, low;
-	u_int eflags;
 
-	eflags = read_eflags();
 	mtx_lock_spin(&clock_lock);
 
 	/* Select timer0 and latch counter value. */
@@ -884,10 +875,10 @@
 	low = inb(TIMER_CNTR0);
 	high = inb(TIMER_CNTR0);
 	count = timer0_max_count - ((high << 8) | low);
-	if (count < i8254_lastcount ||
-	    (!i8254_ticked && (clkintr_pending ||
-	    ((count < 20 || (!(eflags & PSL_I) && count < timer0_max_count / 2u)) &&
-	    i8254_pending != NULL && i8254_pending(i8254_intsrc))))) {
+	if (count < i8254_lastcount || (!i8254_ticked && (clkintr_pending ||
+	    ((count < 20 || (!(read_eflags() & PSL_I) && 
+	    count < timer0_max_count / 2u)) && i8254_pending != NULL &&
+	    i8254_pending(i8254_intsrc))))) {
 		i8254_ticked = 1;
 		i8254_offset += timer0_max_count;
 	}
@@ -910,11 +901,12 @@
 static int
 attimer_probe(device_t dev)
 {
-	int result;
+	int res;
 	
-	if ((result = ISA_PNP_PROBE(device_get_parent(dev), dev, attimer_ids)) <= 0)
+	if ((res =
+	    ISA_PNP_PROBE(device_get_parent(dev), dev, attimer_ids)) <= 0)
 		device_quiet(dev);
-	return(result);
+	return(res);
 }
 
 static int

--=-D2UC1HCBeWdBxtbsjtTR--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1113401913.838.0.camel>