trouble with atrtc
Ian Lepore
freebsd at damnhippie.dyndns.org
Mon Jan 9 23:38:45 UTC 2012
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 -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));
More information about the freebsd-hackers
mailing list