Re: trouble with atrtc
On Thu, 2012-01-05 at 10:33 -0500, John Baldwin wrote:
On Wednesday, January 04, 2012 5:22:29 pm Ian Lepore wrote:
[...]
Because atrtc.c has a long and rich history of modifcations, some of
them fairly recent, I thought it would be a good idea to toss out my
ideas for changes here and solicit feedback up front, rather than just
blindly posting a PR with a patch...
It turns out to be very easy to probe for the latched-read behavior with
just a few lines of code in atrtc_start(), so I'd propose doing that and
setting a flag that the in/out code can use to disable the caching of
the current register number on hardware that needs it.
I'd like to add a new public function, atrtc_nmi_enable(int enable) that
drivers can use to manipulate the NMI flag safely under clock_lock and
playing nicely with the register number caching code.
Completely unrelated but nice to have: I'd like to add a tuneable to
control the use of inb(0x84) ops to insert delays after writing to 0x70
and 0x71. Modern hardware doesn't need this, so I think it should
default to not inserting delays.
I've done all these things in our local 8.2 code base and tested them on
all the hardware I've got on hand. If these changes sound acceptable
I'll prepare patches to -current as well.
These changes all sound good to me.
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 in 8.2 for a
while on a bunch of different hardware).
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 - 1.16.2.1
+++ sys/isa/rtc.h 9 Jan 2012 22:04:12 -
@@ -117,6 +117,7 @@ extern int atrtcclock_disable;
intrtcin(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 - 1.13.2.1
+++ sys/x86/isa/atrtc.c 9 Jan 2012 22:04:12 -
@@ -55,28 +55,59 @@ __FBSDID($FreeBSD: src/sys/x86/isa/atrt
#defineRTC_LOCKmtx_lock_spin(clock_lock)
#defineRTC_UNLOCK mtx_unlock_spin(clock_lock)
+/* atrtcclock_disable is set to 1 by apm_attach() or by hint.atrtc.0.clock=0 */
intatrtcclock_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_BIT0x80U
+
+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 =
Re: trouble with atrtc
You'll probably get a better response from freebsd-stable@. Good luck, Doug -- You can observe a lot just by watching. -- Yogi Berra Breadth of IT experience, and depth of knowledge in the DNS. Yours for the right price. :) http://SupersetSolutions.com/ ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: trouble with atrtc
On Wednesday, January 04, 2012 5:22:29 pm Ian Lepore wrote: I'm in the process of updating a bunch of old systems from FreeBSD 6 to 8.2, and I ran into trouble with some old hardware. Long story short, I eventually realized it came down to statclock not ticking, and the rtc interrupt handler was stuck in its while-loop. The specific problem was the new(-ish) logic that caches the currently-selected RTC register number and avoids an outb(0x70) if the access is to the same register as last time. Commenting out that optimization makes the problem go away. Testing on about a dozen systems with hardware in the 2-10 years old range, this problem happens on two of them, and both are based on Cyrix MediaGX chipsets with the CS5530 southbridge. On these systems the code behaves as if the act of doing outb(0x70, rtc_reg) latches the value of the indexed location into some read buffer, and then all subsequent inb(0x71) just keep returning the same latched value over and over until a new write to 0x70 is done. A separate issue, tangentially related, is that we have device drivers that enable and disable NMI interrupts on the fly. They can't safely do so with the new logic that caches the last value written to 0x70. (I'm not sure what they were doing before was all that safe, but that's yet another story.) Because atrtc.c has a long and rich history of modifcations, some of them fairly recent, I thought it would be a good idea to toss out my ideas for changes here and solicit feedback up front, rather than just blindly posting a PR with a patch... It turns out to be very easy to probe for the latched-read behavior with just a few lines of code in atrtc_start(), so I'd propose doing that and setting a flag that the in/out code can use to disable the caching of the current register number on hardware that needs it. I'd like to add a new public function, atrtc_nmi_enable(int enable) that drivers can use to manipulate the NMI flag safely under clock_lock and playing nicely with the register number caching code. Completely unrelated but nice to have: I'd like to add a tuneable to control the use of inb(0x84) ops to insert delays after writing to 0x70 and 0x71. Modern hardware doesn't need this, so I think it should default to not inserting delays. I've done all these things in our local 8.2 code base and tested them on all the hardware I've got on hand. If these changes sound acceptable I'll prepare patches to -current as well. These changes all sound good to me. -- John Baldwin ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
trouble with atrtc
I'm in the process of updating a bunch of old systems from FreeBSD 6 to 8.2, and I ran into trouble with some old hardware. Long story short, I eventually realized it came down to statclock not ticking, and the rtc interrupt handler was stuck in its while-loop. The specific problem was the new(-ish) logic that caches the currently-selected RTC register number and avoids an outb(0x70) if the access is to the same register as last time. Commenting out that optimization makes the problem go away. Testing on about a dozen systems with hardware in the 2-10 years old range, this problem happens on two of them, and both are based on Cyrix MediaGX chipsets with the CS5530 southbridge. On these systems the code behaves as if the act of doing outb(0x70, rtc_reg) latches the value of the indexed location into some read buffer, and then all subsequent inb(0x71) just keep returning the same latched value over and over until a new write to 0x70 is done. A separate issue, tangentially related, is that we have device drivers that enable and disable NMI interrupts on the fly. They can't safely do so with the new logic that caches the last value written to 0x70. (I'm not sure what they were doing before was all that safe, but that's yet another story.) Because atrtc.c has a long and rich history of modifcations, some of them fairly recent, I thought it would be a good idea to toss out my ideas for changes here and solicit feedback up front, rather than just blindly posting a PR with a patch... It turns out to be very easy to probe for the latched-read behavior with just a few lines of code in atrtc_start(), so I'd propose doing that and setting a flag that the in/out code can use to disable the caching of the current register number on hardware that needs it. I'd like to add a new public function, atrtc_nmi_enable(int enable) that drivers can use to manipulate the NMI flag safely under clock_lock and playing nicely with the register number caching code. Completely unrelated but nice to have: I'd like to add a tuneable to control the use of inb(0x84) ops to insert delays after writing to 0x70 and 0x71. Modern hardware doesn't need this, so I think it should default to not inserting delays. I've done all these things in our local 8.2 code base and tested them on all the hardware I've got on hand. If these changes sound acceptable I'll prepare patches to -current as well. -- Ian ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
