cvs commit: src/sys/i386/isa clock.c

Poul-Henning Kamp phk at phk.freebsd.dk
Wed Apr 13 11:12:15 PDT 2005


Could you do me the favour and run the diff again but with -b so all
the white-space only changes get ignored ?

In message <1113401913.838.0.camel at S010600deadc0de00.su.shawcable.net>, Adam Gregoire writes:
>
>--=-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 at 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--
>

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.


More information about the cvs-src mailing list