Port-i386 archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

change i8254 clock initialization?



Hi -
when I tried to find what might cause Hauke Fath's TSC
calibration problems (see the "machine losing time by the hour"
thread) I found some "initrtclock(0)" in the lapic code
which looks illogical. It doesn't do anything wrong - it just
relies on some non-obvious behavior of initrtclock() - but
my impression is that the clock initialization is somewhat
backwards.

As things are now, the timer is programmed the first time
it is touched (which is the first DELAY() call, or at latest
before autoconfiguration). It is programmed to wrap over
at HZ (100Hz usually), and to deliver interrupts at that rate.
In the process of autoconfiguration, it is decided which
timer gets to deliver the clock interrupt. On MP systems,
this would be the per-CPU lapic. In this case, the lapic driver
calls the initrtclock(0) mentioned above, which reprograms the
i8254 timer to use its full 16-bit width rather than wrapping
at 100Hz. (As I understand it, this is an optimization for
use as timecounter. Just an optimization, as the timecounter
needs to work either way. Correct me if I'm wrong.)

This first-time programming of the timer already needs the
exact TIMER_FREQ to be known, to deliver the correct rate
of clock interrupts. That frequency is usually well-known,
it didn't change since the original AT, but there is at least
one notable exception which is the AMD ELAN SC520. This means
that the frequency needs to be known at kernel compile time -
one needs a special kernel for such boards.

This is what I think is backwards: One really doesn't need
to know the exact TIMER_FREQ before clock interrupts are
actually enabled. If we program the timer to run the full
(16-bit) cycle initially, and only reprogram it for HZ
if it got selected as clock interrupt source, we gain two ways:
-The initrtclock(0) hack in the lapic driver can go away.
-We've got time until the end of autoconf until we need to
 decide on the timer frequency. This means we don't need
 a special NET4501 kernel.

One problem remaining is that DELAY() would still use the
standard frequency and thus be off by something less than
a percent in the SC520 case. This is hopefully negligible, but
in case it isn't one could find another way to calibrate it,
possible using the TOD (mc146818) clock.

I'm testing the appended patch on an MP laptop, and on an embedded
board with an ELAN SC520. The latter still doesn't make a great
timekeeper, the clock is still off by ~300ppm, but there must
be other reasons.

What do you think?

best regards
Matthias


------------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------------
Forschungszentrum Juelich GmbH
52425 Juelich
Sitz der Gesellschaft: Juelich
Eingetragen im Handelsregister des Amtsgerichts Dueren Nr. HR B 3498
Vorsitzende des Aufsichtsrats: MinDir'in Baerbel Brumme-Bothe
Geschaeftsfuehrung: Prof. Dr. Achim Bachem (Vorsitzender),
Dr. Ulrich Krafft (stellv. Vorsitzender), Prof. Dr.-Ing. Harald Bolt,
Prof. Dr. Sebastian M. Schmidt
------------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------------
#
# old_revision [06e7c3b3041ac05ff6feb399f6cd54fcebe801ee]
#
# patch "sys/arch/i386/i386/machdep.c"
#  from [a38a48e41c72b08031d9fde7070359492ce7047c]
#    to [52aa4f7d44d93f4c3e1d41fb4541e44a344cb728]
# 
# patch "sys/arch/i386/pci/elan520.c"
#  from [0442693b625fd1d45fd430cb4cc91375e3eaa19c]
#    to [7fdb0a424757739d7507fff21fc9924cf4b275ce]
# 
# patch "sys/arch/x86/include/cpu.h"
#  from [e9a04b37fd905160b0a370257d0199f7ef40b4cc]
#    to [70a130d83e897a0e587b92e0ed2591ad2d84d230]
# 
# patch "sys/arch/x86/isa/clock.c"
#  from [da21ca3541a4050ff2a15a7f30a35ec281860873]
#    to [33ecf034ed11e48e790b4ff7bfcd57a8d6c6736b]
# 
# patch "sys/arch/x86/x86/lapic.c"
#  from [797fe300d544be5387bf50ff279bc8a357d77c35]
#    to [068d29917e8f4b9bf7f180f104c3c4b6ae625645]
#
============================================================
--- sys/arch/i386/i386/machdep.c        a38a48e41c72b08031d9fde7070359492ce7047c
+++ sys/arch/i386/i386/machdep.c        52aa4f7d44d93f4c3e1d41fb4541e44a344cb728
@@ -1873,5 +1875,6 @@ cpu_initclocks(void)
 cpu_initclocks(void)
 {
 
+       i8254_inittc();
        (*initclock_func)();
 }
============================================================
--- sys/arch/i386/pci/elan520.c 0442693b625fd1d45fd430cb4cc91375e3eaa19c
+++ sys/arch/i386/pci/elan520.c 7fdb0a424757739d7507fff21fc9924cf4b275ce
@@ -118,6 +118,8 @@ int elansc_do_protect_pg0 = 1;
 int elansc_pcinmi = 1;
 int elansc_do_protect_pg0 = 1;
 
+extern u_long i8254_timerfreq; /* x86/isa/clock.c */
+
 #if NGPIO > 0
 static int     elansc_gpio_pin_read(void *, int);
 static void    elansc_gpio_pin_write(void *, int, int);
@@ -1281,6 +1283,8 @@ elansc_attach(device_t parent, device_t 
        aprint_naive(": System Controller\n");
        aprint_normal(": AMD Elan SC520 System Controller\n");
 
+       i8254_timerfreq = 1189200;
+
        sc->sc_iot = pba->pba_iot;
        sc->sc_memt = pba->pba_memt;
        if (bus_space_map(sc->sc_memt, MMCR_BASE_ADDR, PAGE_SIZE, 0,
============================================================
--- sys/arch/x86/include/cpu.h  e9a04b37fd905160b0a370257d0199f7ef40b4cc
+++ sys/arch/x86/include/cpu.h  70a130d83e897a0e587b92e0ed2591ad2d84d230
@@ -378,11 +378,12 @@ void      xen_initclocks(void);
 void   xen_initclocks(void);
 #else
 /* clock.c */
-void   initrtclock(u_long);
+void   initrtclock(void);
 void   startrtclock(void);
 void   i8254_delay(unsigned int);
 void   i8254_microtime(struct timeval *);
 void   i8254_initclocks(void);
+void   i8254_inittc(void);
 #endif
 
 /* cpu.c */
============================================================
--- sys/arch/x86/isa/clock.c    da21ca3541a4050ff2a15a7f30a35ec281860873
+++ sys/arch/x86/isa/clock.c    33ecf034ed11e48e790b4ff7bfcd57a8d6c6736b
@@ -198,6 +198,8 @@ static volatile int i8254_ticked;
 static volatile uint32_t i8254_offset;
 static volatile int i8254_ticked;
 
+u_long i8254_timerfreq = TIMER_FREQ;
+
 /* to protect TC timer variables */
 static __cpu_simple_lock_t tmr_lock = __SIMPLELOCK_UNLOCKED;
 
@@ -207,7 +209,7 @@ static struct timecounter i8254_timecoun
        i8254_get_timecount,    /* get_timecount */
        0,                      /* no poll_pps */
        ~0u,                    /* counter_mask */
-       TIMER_FREQ,             /* frequency */
+       0,                      /* frequency, will be filled in */
        "i8254",                /* name */
        100,                    /* quality */
        NULL,                   /* private data */
@@ -300,26 +302,15 @@ void
 
 /* minimal initialization, enough for delay() */
 void
-initrtclock(u_long freq)
+initrtclock()
 {
-       u_long tval;
 
-       /*
-        * Compute timer_count, the count-down count the timer will be
-        * set to.  Also, correctly round
-        * this by carrying an extra bit through the division.
-        */
-       tval = (freq * 2) / (u_long) hz;
-       tval = (tval / 2) + (tval & 0x1);
-
        /* initialize 8254 clock */
        outb(IO_TIMER1+TIMER_MODE, TIMER_SEL0|TIMER_RATEGEN|TIMER_16BIT);
+       outb(IO_TIMER1+TIMER_CNTR0, 0);
+       outb(IO_TIMER1+TIMER_CNTR0, 0);
 
-       /* Correct rounding will buy us a better precision in timekeeping */
-       outb(IO_TIMER1+TIMER_CNTR0, tval % 256);
-       outb(IO_TIMER1+TIMER_CNTR0, tval / 256);
-
-       rtclock_tval = tval ? tval : 0xFFFF;
+       rtclock_tval = 0xffff;
        rtclock_init = 1;
 }
 
@@ -329,7 +320,7 @@ startrtclock(void)
        int s;
 
        if (!rtclock_init)
-               initrtclock(TIMER_FREQ);
+               initrtclock();
 
        /* Check diagnostic status */
        if ((s = mc146818_read(NULL, NVRAM_DIAG)) != 0) { /* XXX softc */
@@ -338,10 +329,17 @@ startrtclock(void)
                printf("RTC BIOS diagnostic error %s\n", bits);
        }
 
-       tc_init(&i8254_timecounter);
        rtc_register();
 }
 
+void
+i8254_inittc()
+{
+
+       i8254_timecounter.tc_frequency = i8254_timerfreq;
+       tc_init(&i8254_timecounter);
+}
+
 /*
  * Must be called at splsched().
  */
@@ -455,7 +453,7 @@ i8254_delay(unsigned int n)
 
        /* allow DELAY() to be used before startrtclock() */
        if (!rtclock_init)
-               initrtclock(TIMER_FREQ);
+               initrtclock();
 
        /*
         * Read the counter first, so that the rest of the setup overhead is
@@ -548,8 +546,26 @@ i8254_initclocks(void)
 void
 i8254_initclocks(void)
 {
+       u_long tval;
 
+       KASSERT(rtclock_init);
+
        /*
+        * Compute timer_count, the count-down count the timer will be
+        * set to.  Also, correctly round
+        * this by carrying an extra bit through the division.
+        */
+       tval = (i8254_timerfreq * 2) / (u_long) hz;
+       tval = (tval / 2) + (tval & 0x1);
+
+       outb(IO_TIMER1+TIMER_MODE, TIMER_SEL0|TIMER_RATEGEN|TIMER_16BIT);
+       /* Correct rounding will buy us a better precision in timekeeping */
+       outb(IO_TIMER1+TIMER_CNTR0, tval % 256);
+       outb(IO_TIMER1+TIMER_CNTR0, tval / 256);
+
+       rtclock_tval = tval ? tval : 0xFFFF;
+
+       /*
         * XXX If you're doing strange things with multiple clocks, you might
         * want to keep track of clock handlers.
         */
============================================================
--- sys/arch/x86/x86/lapic.c    797fe300d544be5387bf50ff279bc8a357d77c35
+++ sys/arch/x86/x86/lapic.c    068d29917e8f4b9bf7f180f104c3c4b6ae625645
@@ -437,7 +432,6 @@ lapic_calibrate_timer(struct cpu_info *c
                 */
                delay_func = lapic_delay;
                initclock_func = lapic_initclocks;
-               initrtclock(0);
 
                if (lapic_timecounter.tc_frequency == 0) {
                        /*


Home | Main Index | Thread Index | Old Index