Date: Fri, 17 Aug 2012 08:41:07 -0600 (MDT) From: Ian Lepore <freebsd@damnhippie.dyndns.org> To: FreeBSD-gnats-submit@FreeBSD.org Subject: i386/170705: [patch] AT realtime clock support routines fail on some RTC hardware. Message-ID: <201208171441.q7HEf7IG015410@revolution.hippie.lan> Resent-Message-ID: <201208171450.q7HEo3Hj057371@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 170705 >Category: i386 >Synopsis: [patch] AT realtime clock support routines fail on some RTC hardware. >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-i386 >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Fri Aug 17 14:50:02 UTC 2012 >Closed-Date: >Last-Modified: >Originator: Ian Lepore <freebsd@damnhippie.dyndns.org> >Release: FreeBSD 10.0-CURRENT >Organization: Symmetricom, Inc. >Environment: 10.0-CURRENT FreeBSD 10.0-CURRENT #2 r239193M: Mon Aug 13 11:45:29 MDT 2012 >Description: On some RTC hardware, the RTC interrupt handler (rtcintr() in x86/isa/clock.c) can get stuck in its loop waiting for the RTCIR_PERIOD status bit to become de-asserted in the hardware register. To read RTC hardware registers you first set the index register at port 0x70 to select the register to read, then you read the value from port 0x71. On most hardware you can write to the index register once, then issue multiple subsequent reads to port 0x71 to continuously read new values from the selected register. At some point, the atrtc.c code was enhanced to avoid the expensive outb(0x70,) if it would re-select the same index as last time. It appears that on some hardware, the value in the selected register is latched at the time you select it with a write to 0x70, and then any number of subsequent reads of 0x71 will keep returning the same latched value forever. This is known to happen on systems based on the AMD Geode 500 and CS5536 companion chipset, used on a variety of single-board computers, including the Soekris net5501 series. The atrtc code also contains a delay (implemented by a dummy read of port 0x84) after each access to ports 0x70-71, which is expensive (about 1uS per read) and is not required by modern hardware. The attached patch eliminates this extra IO unless it is specifically enabled with hint.atrtc.0.use_iodelay=1. Finally, the attached patch adds a new routine, atrtc_nmi_enable(), which can be used by drivers which need to enable nmi interrupts. Without this routine, it's possible that a driver directly manipulating the nmi enable bit will conflict with the atrtc driver reading and writing the same port to change rtc index register. There was some mailing-list discussion of this in January 2012, when I first ran into the problem on an industrial single-board computer, and again in August 2012 when the attached patch was shown to fix a problem on a Soekris system. http://lists.freebsd.org/pipermail/freebsd-hackers/2012-January/037217.html http://lists.freebsd.org/pipermail/freebsd-current/2012-August/035868.html >How-To-Repeat: >Fix: Here is the patch for -current and 9. I can provide a patch to 8-stable as well; it's essentially the same patch with small context differences. I've tested this using -current on several systems, recent and old hardware, including manually bumping up the quality score for the rtc event timer to force it to get used, and it seems to work without trouble (and of course I've been testing the same patch at work in 8.2 for a while on a bunch of different hardware). --- atrtc.diff begins here --- Index: sys/isa/rtc.h =================================================================== RCS file: /local/base/FreeBSD-CVS/src/sys/isa/rtc.h,v retrieving revision 1.16.2.1 diff -u -p -r1.16.2.1 rtc.h --- sys/isa/rtc.h 23 Sep 2011 00:51:37 -0000 1.16.2.1 +++ sys/isa/rtc.h 9 Jan 2012 22:04:12 -0000 @@ -117,6 +117,7 @@ extern int atrtcclock_disable; int rtcin(int reg); void atrtc_restore(void); void writertc(int reg, u_char val); +void atrtc_nmi_enable(int enable); #endif #endif /* _I386_ISA_RTC_H_ */ Index: sys/x86/isa/atrtc.c =================================================================== RCS file: /local/base/FreeBSD-CVS/src/sys/x86/isa/atrtc.c,v retrieving revision 1.13.2.1 diff -u -p -r1.13.2.1 atrtc.c --- sys/x86/isa/atrtc.c 23 Sep 2011 00:51:37 -0000 1.13.2.1 +++ sys/x86/isa/atrtc.c 9 Jan 2012 22:04:12 -0000 @@ -55,28 +55,59 @@ __FBSDID("$FreeBSD: src/sys/x86/isa/atrt #define RTC_LOCK mtx_lock_spin(&clock_lock) #define RTC_UNLOCK mtx_unlock_spin(&clock_lock) +/* atrtcclock_disable is set to 1 by apm_attach() or by hint.atrtc.0.clock=0 */ int atrtcclock_disable = 0; -static int rtc_reg = -1; -static u_char rtc_statusa = RTCSA_DIVIDER | RTCSA_NOPROF; -static u_char rtc_statusb = RTCSB_24HR; +static int use_iodelay = 0; /* set from hint.atrtc.0.use_iodelay */ + +#define RTC_REINDEX_REQUIRED 0xffU +#define NMI_ENABLE_BIT 0x80U + +static u_char nmi_enable; +static u_char rtc_reg = RTC_REINDEX_REQUIRED; +static u_char rtc_statusa = RTCSA_DIVIDER | RTCSA_NOPROF; +static u_char rtc_statusb = RTCSB_24HR; + +/* + * Delay after writing to IO_RTC[+1] registers. Modern hardware doesn't + * require this expensive delay, so it's a tuneable that's disabled by default. + */ +static __inline void +rtc_iodelay(void) +{ + if (use_iodelay) + inb(0x84); +} /* * RTC support routines + * + * Most rtc chipsets let you write a value into the index register and then each + * read of the IO register obtains a new value from the indexed location. Others + * behave as if they latch the indexed value when you write to the index, and + * repeated reads keep returning the same value until you write to the index + * register again. atrtc_start() probes for this behavior and leaves rtc_reg + * set to RTC_REINDEX_REQUIRED if reads keep returning the same value. */ +static __inline void +rtcindex(u_char reg) +{ + if (rtc_reg != reg) { + if (rtc_reg != RTC_REINDEX_REQUIRED) + rtc_reg = reg; + outb(IO_RTC, reg | nmi_enable); + rtc_iodelay(); + } +} + int rtcin(int reg) { u_char val; RTC_LOCK; - if (rtc_reg != reg) { - inb(0x84); - outb(IO_RTC, reg); - rtc_reg = reg; - inb(0x84); - } + rtcindex(reg); val = inb(IO_RTC + 1); RTC_UNLOCK; return (val); @@ -87,14 +118,9 @@ writertc(int reg, u_char val) { RTC_LOCK; - if (rtc_reg != reg) { - inb(0x84); - outb(IO_RTC, reg); - rtc_reg = reg; - inb(0x84); - } + rtcindex(reg); outb(IO_RTC + 1, val); - inb(0x84); + rtc_iodelay(); RTC_UNLOCK; } @@ -104,12 +130,31 @@ readrtc(int port) return(bcd2bin(rtcin(port))); } +/* + * At start, probe read-without-reindex behavior. Reading RTC_INTR clears it; + * read until it has a non-zero value, then read it again without re-writing the + * index register. If 2nd read returns a different value it's safe to cache the + * current index with this chipset; enable by changing rtc_reg to current index. + */ static void atrtc_start(void) { + int status; - writertc(RTC_STATUSA, rtc_statusa); + writertc(RTC_STATUSA, RTCSA_DIVIDER | RTCSA_8192); writertc(RTC_STATUSB, RTCSB_24HR); + + RTC_LOCK; + do { + rtcindex(RTC_INTR); + status = inb(IO_RTC+1); + } while (status == 0); + + if (status != inb(IO_RTC+1)) + rtc_reg = RTC_INTR; + RTC_UNLOCK; + + writertc(RTC_STATUSA, RTCSA_DIVIDER | RTCSA_NOPROF); } static void @@ -139,6 +184,17 @@ atrtc_disable_intr(void) } void +atrtc_nmi_enable(int enable) +{ + /* Must not write to 0x70 without a following read or write of 0x71 on + * some chipsets, so do a read. Also, must access a register other than + * the current rtc_reg to force rtcin to push a change out to 0x70. + */ + nmi_enable = enable ? NMI_ENABLE_BIT : 0x00; + rtcin((rtc_reg == RTC_STATUSA) ? RTC_STATUSB : RTC_STATUSA); +} + +void atrtc_restore(void) { @@ -249,6 +305,7 @@ atrtc_attach(device_t dev) IO_RTC, IO_RTC + 1, 2, RF_ACTIVE); if (sc->port_res == NULL) device_printf(dev, "Warning: Couldn't map I/O.\n"); + resource_int_value("atrtc", 0, "use_iodelay", &use_iodelay); atrtc_start(); clock_register(dev, 1000000); bzero(&sc->et, sizeof(struct eventtimer)); --- atrtc.diff ends here --- >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208171441.q7HEf7IG015410>