trouble with atrtc

[John Baldwin]

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