Hi,

i had forgotten about this already, but this has existed for a while.
systat does consistently show "66 tick"(around 108 for stattick), while

root@av7marsb:~ # vmstat -i
interrupt                       total     rate
irq32/sximmc0                   35222       19
irq55/sxie0                      3860        2
irq1/com0                       17222        9
irq22/tick                     181011       99
irq23/stattick                 293512      162
Total                          530827      293

how come there is such consistent difference? guessing they must source
time differently, so does either use interface leading to rtc _gettime()
instead of timecounters _get_timecount()?

while trying to fix this(, before remembering about "vmstat -i"), i saw
this sillyness in sxitimer irq handlers, unconditional looping that can
be avoided w/slight reordering, and given how it doesn't support other
clocksources, i think it does make sense to avoid the useless bus_space
branching for TIMER_OSC24M-bit while restarting the timers too.

what i mean with above is, given how there is such _sync "price to pay",
for touching _INTV; timer needs to be stopped, and shouldn't be enabled
until 2*Tcycles has passed. it doesn't make sense to delay disabling
to the end right before it is to be restarted.

-Artturi



diff --git a/sys/arch/armv7/sunxi/sxitimer.c b/sys/arch/armv7/sunxi/sxitimer.c
index 65d455adaeb..8abd2137304 100644
--- a/sys/arch/armv7/sunxi/sxitimer.c
+++ b/sys/arch/armv7/sunxi/sxitimer.c
@@ -71,6 +71,9 @@
 #define        TIMER_CONTINOUS         (0 << 7)
 #define        TIMER_SINGLESHOT        (1 << 7)
 
+#define        TIMER_RESTART           (TIMER_ENABLE | TIMER_RELOAD | \
+                               TIMER_OSC24M | TIMER_SINGLESHOT)
+
 #define        TICKTIMER               0
 #define        STATTIMER               1
 #define        CNTRTIMER               2
@@ -84,8 +87,8 @@ int   sxitimer_statintr(void *);
 void   sxitimer_cpu_initclocks(void);
 void   sxitimer_setstatclockrate(int);
 uint64_t       sxitimer_readcnt64(void);
-uint32_t       sxitimer_readcnt32(void);
-void   sxitimer_sync(void);
+static inline uint32_t sxitimer_readcnt32(void);
+static inline void     sxitimer_sync(uint32_t);
 void   sxitimer_delay(u_int);
 
 u_int sxitimer_get_timecount(struct timecounter *);
@@ -290,7 +293,6 @@ int
 sxitimer_tickintr(void *frame)
 {
        uint32_t now, nextevent;
-       uint32_t val;
        int rc = 0;
 
        splassert(IPL_CLOCK);   
@@ -299,8 +301,11 @@ sxitimer_tickintr(void *frame)
        bus_space_write_4(sxitimer_iot, sxitimer_ioh,
            TIMER_ISR, TIMER_IRQ(TICKTIMER));
 
-       now = sxitimer_readcnt32();
+       /* stop/disable for sync */
+       bus_space_write_4(sxitimer_iot, sxitimer_ioh,
+           TIMER_CTRL(TICKTIMER), 0);
 
+       now = sxitimer_readcnt32();
        while ((int32_t)(now - sxitimer_tick_nextevt) < 0) {
                sxitimer_tick_nextevt -= sxitimer_tick_tpi;
                sxitimer_ticks_err_sum += sxitimer_ticks_err_cnt;
@@ -328,21 +333,13 @@ sxitimer_tickintr(void *frame)
                sxitimer_tick_nextevt = now;
        }
 
-       val = bus_space_read_4(sxitimer_iot, sxitimer_ioh,
-           TIMER_CTRL(TICKTIMER));
-       bus_space_write_4(sxitimer_iot, sxitimer_ioh,
-           TIMER_CTRL(TICKTIMER), val & ~TIMER_ENABLE);
-
-       sxitimer_sync();
+       sxitimer_sync(now);
 
        bus_space_write_4(sxitimer_iot, sxitimer_ioh,
            TIMER_INTV(TICKTIMER), nextevent);
 
-       val = bus_space_read_4(sxitimer_iot, sxitimer_ioh,
-           TIMER_CTRL(TICKTIMER));
        bus_space_write_4(sxitimer_iot, sxitimer_ioh,
-           TIMER_CTRL(TICKTIMER),
-           val | TIMER_ENABLE | TIMER_RELOAD | TIMER_SINGLESHOT);
+           TIMER_CTRL(TICKTIMER), TIMER_RESTART);
 
        return rc;
 }
@@ -351,7 +348,6 @@ int
 sxitimer_statintr(void *frame)
 {
        uint32_t now, nextevent, r;
-       uint32_t val;
        int rc = 0;
 
        splassert(IPL_STATCLOCK);       
@@ -360,6 +356,10 @@ sxitimer_statintr(void *frame)
        bus_space_write_4(sxitimer_iot, sxitimer_ioh,
            TIMER_ISR, TIMER_IRQ(STATTIMER));
 
+       /* stop/disable for sync */
+       bus_space_write_4(sxitimer_iot, sxitimer_ioh,
+           TIMER_CTRL(STATTIMER), 0);
+
        now = sxitimer_readcnt32();
        while ((int32_t)(now - sxitimer_stat_nextevt) < 0) {
                do {
@@ -386,21 +386,13 @@ sxitimer_statintr(void *frame)
                sxitimer_stat_nextevt = now;
        }
 
-       val = bus_space_read_4(sxitimer_iot, sxitimer_ioh,
-           TIMER_CTRL(STATTIMER));
-       bus_space_write_4(sxitimer_iot, sxitimer_ioh,
-           TIMER_CTRL(STATTIMER), val & ~TIMER_ENABLE);
-
-       sxitimer_sync();
+       sxitimer_sync(now);
 
        bus_space_write_4(sxitimer_iot, sxitimer_ioh,
            TIMER_INTV(STATTIMER), nextevent);
 
-       val = bus_space_read_4(sxitimer_iot, sxitimer_ioh,
-           TIMER_CTRL(STATTIMER));
        bus_space_write_4(sxitimer_iot, sxitimer_ioh,
-           TIMER_CTRL(STATTIMER),
-           val | TIMER_ENABLE | TIMER_RELOAD | TIMER_SINGLESHOT);
+           TIMER_CTRL(STATTIMER), TIMER_RESTART);
 
        return rc;
 }
@@ -434,10 +426,8 @@ sxitimer_readcnt32(void)
 }
 
 void
-sxitimer_sync(void)
+sxitimer_sync(uint32_t now)
 {
-       uint32_t now = sxitimer_readcnt32();
-
        while ((now - sxitimer_readcnt32()) < TIMER_SYNC)
                CPU_BUSY_CYCLE();
 }

Reply via email to