From owner-cvs-src@FreeBSD.ORG Wed Apr 13 18:12:14 2005 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D23D316A4CE; Wed, 13 Apr 2005 18:12:14 +0000 (GMT) Received: from critter.freebsd.dk (f170.freebsd.dk [212.242.86.170]) by mx1.FreeBSD.org (Postfix) with ESMTP id 94F8843D31; Wed, 13 Apr 2005 18:12:13 +0000 (GMT) (envelope-from phk@critter.freebsd.dk) Received: from critter.freebsd.dk (localhost [127.0.0.1]) by critter.freebsd.dk (8.13.3/8.13.1) with ESMTP id j3DICBOF021178; Wed, 13 Apr 2005 20:12:11 +0200 (CEST) (envelope-from phk@critter.freebsd.dk) To: Adam Gregoire From: "Poul-Henning Kamp" In-Reply-To: Your message of "Wed, 13 Apr 2005 10:18:33 EDT." <1113401913.838.0.camel@S010600deadc0de00.su.shawcable.net> Date: Wed, 13 Apr 2005 20:12:11 +0200 Message-ID: <21177.1113415931@critter.freebsd.dk> Sender: phk@critter.freebsd.dk cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org cc: "Matthew N. Dodd" Subject: Re: cvs commit: src/sys/i386/isa clock.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Apr 2005 18:12:15 -0000 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@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 > >--=-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@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.