On Sat, Dec 10 2022, Jeremie Courreges-Anglas <[email protected]> wrote:
> On Fri, Dec 09 2022, Scott Cheloha <[email protected]> wrote:
>> Next up for the armv7 clockintr switch: amptimer(4).
>>
>> - Remove amptimer-specific clock interrupt scheduling bits and
>>   randomized statclock bits.
>> - Remove debug evcounts.  The interrupt's evcount counts all clock
>>   interrupts from now on.
>> - Remove unused USE_GTIMER_CMP pieces.
>> - Wire up amptimer_intrclock.
>>
>> jca: You have a cubox with this device.  Does this compile?
>>      If so, does it boot?
>
> Builds but the machine didn't come up after reboot.  I'm not at home
> right now so it'll have to wait a day or two, unless someone else can
> test on their own machine.

Here's an updated diff with the fix found by cheloha (use 1 instead of
0 to trigger the timer ASAP).  A GENERIC kernel with this diff completed
a make build (minus some parts of llvm/clang), no clock drift visible so
far.  A bsd.rd including this diff successfully ran an upgrade over fec(4).

dmesg:
--8<--
OpenBSD 7.2-current (GENERIC) #9: Wed Dec 14 23:57:14 CET 2022
    [email protected]:/sys/arch/armv7/compile/GENERIC
real mem  = 3980587008 (3796MB)
avail mem = 3900547072 (3719MB)
random: good seed from bootblocks
mainbus0 at root: SolidRun Cubox-i Dual/Quad
cpu0 at mainbus0 mpidr 0: ARM Cortex-A9 r2p10
cpu0: 32KB 32b/line 4-way L1 VIPT I-cache, 32KB 32b/line 4-way L1 D-cache
cortex0 at mainbus0
amptimer0 at cortex0: 396000 kHz
armliicc0 at cortex0: rtl 7 waymask: 0x0000000f
simplebus0 at mainbus0: "soc"
ampintc0 at simplebus0 nirq 160, ncpu 4: "interrupt-controller"
"dma-apbh" at simplebus0 not configured
"hdmi" at simplebus0 not configured
"gpu" at simplebus0 not configured
"gpu" at simplebus0 not configured
"timer" at simplebus0 not configured
"cache-controller" at simplebus0 not configured
simplebus1 at simplebus0: "bus"
imxccm0 at simplebus1
imxanatop0 at simplebus1
syscon0 at simplebus1: "snvs"
imxrtc0 at syscon0
"snvs-lpgpr" at syscon0 not configured
imxsrc0 at simplebus1
syscon1 at simplebus1: "iomuxc-gpr"
"mux-controller" at syscon1 not configured
"ipu1_csi0_mux" at syscon1 not configured
"ipu2_csi1_mux" at syscon1 not configured
imxiomuxc0 at simplebus1
simplebus2 at simplebus1: "spba-bus"
"spdif" at simplebus2 not configured
imxuart0 at simplebus2: console
"asrc" at simplebus2 not configured
"vpu" at simplebus1 not configured
"pwm" at simplebus1 not configured
"timer" at simplebus1 not configured
imxgpio0 at simplebus1
imxgpio1 at simplebus1
imxgpio2 at simplebus1
imxgpio3 at simplebus1
imxgpio4 at simplebus1
imxgpio5 at simplebus1
imxgpio6 at simplebus1
imxdog0 at simplebus1
"usbphy" at simplebus1 not configured
"usbphy" at simplebus1 not configured
imxgpc0 at simplebus1
"sdma" at simplebus1 not configured
simplebus3 at simplebus0: "bus"
syscon2 at simplebus3: "efuse"
"crypto" at simplebus3 not configured
imxehci0 at simplebus3
usb0 at imxehci0: USB revision 2.0
uhub0 at usb0 configuration 1 interface 0 "i.MX EHCI root hub" rev 2.00/1.00 
addr 1
imxehci1 at simplebus3
usb1 at imxehci1: USB revision 2.0
uhub1 at usb1 configuration 1 interface 0 "i.MX EHCI root hub" rev 2.00/1.00 
addr 1
"usbmisc" at simplebus3 not configured
fec0 at simplebus3
fec0: address d0:63:b4:xx:xx:xx
atphy0 at fec0 phy 0: AR8035 10/100/1000 PHY, rev. 2
imxesdhc0 at simplebus3
imxesdhc0: 198 MHz base clock
sdmmc0 at imxesdhc0: 4-bit, sd high-speed, mmc high-speed, dma
imxesdhc1 at simplebus3
imxesdhc1: 198 MHz base clock
sdmmc1 at imxesdhc1: 4-bit, sd high-speed, mmc high-speed, dma
imxiic0 at simplebus3
iic0 at imxiic0
imxiic1 at simplebus3
iic1 at imxiic1
pcfrtc0 at iic1 addr 0x68: battery low
"memory-controller" at simplebus3 not configured
"vdoa" at simplebus3 not configured
imxuart1 at simplebus3
"ipu" at simplebus0 not configured
"sram" at simplebus0 not configured
imxahci0 at simplebus0: AHCI 1.3
scsibus0 at imxahci0: 32 targets
"gpu" at simplebus0 not configured
"ipu" at simplebus0 not configured
scsibus1 at sdmmc1: 2 targets, initiator 0
sd0 at scsibus1 targ 1 lun 0: <SD/MMC, SD32G, 0085> removable
sd0: 30436MB, 512 bytes/sector, 62333952 sectors
bwfm0 at sdmmc0 function 1
manufacturer 0x02d0, product 0x4330 at sdmmc0 function 2 not configured
vscsi0 at root
scsibus2 at vscsi0: 256 targets
softraid0 at root
scsibus3 at softraid0: 256 targets
bootfile: sd0a:/bsd
boot device: sd0
root on sd0a (a9d38e4f8011a0ce.a) swap on sd0b dump on sd0b
bwfm0: address 6c:ad:f8:xx:xx:xx
-->8--

ok jca@ when you see fit.


Index: sys/arch/arm/include/cpu.h
===================================================================
RCS file: /cvs/src/sys/arch/arm/include/cpu.h,v
retrieving revision 1.61
diff -u -p -r1.61 cpu.h
--- sys/arch/arm/include/cpu.h  6 Jul 2021 09:34:06 -0000       1.61
+++ sys/arch/arm/include/cpu.h  14 Dec 2022 17:23:45 -0000
@@ -149,6 +149,7 @@ void        arm32_vector_init(vaddr_t, int);
  * Per-CPU information.  For now we assume one CPU.
  */
 
+#include <sys/clockintr.h>
 #include <sys/device.h>
 #include <sys/sched.h>
 #include <sys/srp.h>
@@ -198,7 +199,7 @@ struct cpu_info {
 #ifdef GPROF
        struct gmonparam *ci_gmon;
 #endif
-
+       struct clockintr_queue  ci_queue;
        char                    ci_panicbuf[512];
 };
 
Index: sys/arch/arm/include/_types.h
===================================================================
RCS file: /cvs/src/sys/arch/arm/include/_types.h,v
retrieving revision 1.19
diff -u -p -r1.19 _types.h
--- sys/arch/arm/include/_types.h       5 Mar 2018 01:15:25 -0000       1.19
+++ sys/arch/arm/include/_types.h       10 Dec 2022 17:09:55 -0000
@@ -35,6 +35,8 @@
 #ifndef _ARM__TYPES_H_
 #define _ARM__TYPES_H_
 
+#define        __HAVE_CLOCKINTR
+
 #if defined(_KERNEL)
 typedef struct label_t {
        long val[11];
Index: sys/arch/arm/cortex/amptimer.c
===================================================================
RCS file: /cvs/src/sys/arch/arm/cortex/amptimer.c,v
retrieving revision 1.14
diff -u -p -r1.14 amptimer.c
--- sys/arch/arm/cortex/amptimer.c      12 Mar 2022 14:40:41 -0000      1.14
+++ sys/arch/arm/cortex/amptimer.c      14 Dec 2022 21:51:20 -0000
@@ -17,8 +17,10 @@
 
 #include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/clockintr.h>
 #include <sys/device.h>
 #include <sys/kernel.h>
+#include <sys/stdint.h>
 #include <sys/timetc.h>
 
 #include <arm/cpufunc.h>
@@ -74,32 +76,25 @@ static struct timecounter amptimer_timec
        .tc_user = 0,
 };
 
-#define MAX_ARM_CPUS   8
+void amptimer_rearm(void *, uint64_t);
+void amptimer_trigger(void *);
 
-struct amptimer_pcpu_softc {
-       uint64_t                pc_nexttickevent;
-       uint64_t                pc_nextstatevent;
-       u_int32_t               pc_ticks_err_sum;
+struct intrclock amptimer_intrclock = {
+       .ic_rearm = amptimer_rearm,
+       .ic_trigger = amptimer_trigger
 };
 
+#define MAX_ARM_CPUS   8
+
 struct amptimer_softc {
        struct device           sc_dev;
        bus_space_tag_t         sc_iot;
        bus_space_handle_t      sc_ioh;
        bus_space_handle_t      sc_pioh;
 
-       struct amptimer_pcpu_softc sc_pstat[MAX_ARM_CPUS];
-
-       u_int32_t               sc_ticks_err_cnt;
        u_int32_t               sc_ticks_per_second;
-       u_int32_t               sc_ticks_per_intr;
-       u_int32_t               sc_statvar;
-       u_int32_t               sc_statmin;
-
-#ifdef AMPTIMER_DEBUG
-       struct evcount          sc_clk_count;
-       struct evcount          sc_stat_count;
-#endif
+       u_int64_t               sc_nsec_cycle_ratio;
+       u_int64_t               sc_nsec_max;
 };
 
 int            amptimer_match(struct device *, void *, void *);
@@ -173,6 +168,9 @@ amptimer_attach(struct device *parent, s
                panic("amptimer_attach: bus_space_map priv timer failed!");
 
        sc->sc_ticks_per_second = amptimer_frequency;
+       sc->sc_nsec_cycle_ratio =
+           sc->sc_ticks_per_second * (1ULL << 32) / 1000000000;
+       sc->sc_nsec_max = UINT64_MAX / sc->sc_nsec_cycle_ratio;
        printf(": %d kHz\n", sc->sc_ticks_per_second / 1000);
 
        sc->sc_ioh = ioh;
@@ -188,21 +186,10 @@ amptimer_attach(struct device *parent, s
        /* enable global timer */
        bus_space_write_4(sc->sc_iot, ioh, GTIMER_CTRL, GTIMER_CTRL_TIMER);
 
-#if defined(USE_GTIMER_CMP)
-
-       /* clear event */
-       bus_space_write_4(sc->sc_iot, sc->sc_ioh, GTIMER_STATUS, 1);
-#else
+       /* Disable private timer, clear event flag. */
        bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_CTRL, 0);
        bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_STATUS,
            PTIMER_STATUS_EVENT);
-#endif
-
-
-#ifdef AMPTIMER_DEBUG
-       evcount_attach(&sc->sc_clk_count, "clock", NULL);
-       evcount_attach(&sc->sc_stat_count, "stat", NULL);
-#endif
 
        /*
         * private timer and interrupts not enabled until
@@ -214,8 +201,9 @@ amptimer_attach(struct device *parent, s
 
        amptimer_timecounter.tc_frequency = sc->sc_ticks_per_second;
        amptimer_timecounter.tc_priv = sc;
-
        tc_init(&amptimer_timecounter);
+
+       amptimer_intrclock.ic_cookie = sc;
 }
 
 u_int
@@ -225,102 +213,51 @@ amptimer_get_timecount(struct timecounte
        return bus_space_read_4(sc->sc_iot, sc->sc_ioh, GTIMER_CNT_LOW);
 }
 
-int
-amptimer_intr(void *frame)
+void
+amptimer_rearm(void *cookie, uint64_t nsecs)
 {
-       struct amptimer_softc   *sc = amptimer_cd.cd_devs[0];
-       struct amptimer_pcpu_softc *pc = &sc->sc_pstat[CPU_INFO_UNIT(curcpu())];
-       uint64_t                 now;
-       uint64_t                 nextevent;
-       uint32_t                 r, reg;
-#if defined(USE_GTIMER_CMP)
-       int                      skip = 1;
-#else
-       int64_t                  delay;
-#endif
-       int                      rc = 0;
+       struct amptimer_softc *sc = cookie;
+       uint32_t cycles, reg;
 
-       /*
-        * DSR - I know that the tick timer is 64 bits, but the following
-        * code deals with rollover, so there is no point in dealing
-        * with the 64 bit math, just let the 32 bit rollover
-        * do the right thing
-        */
+       if (nsecs > sc->sc_nsec_max)
+               nsecs = sc->sc_nsec_max;
+       cycles = (nsecs * sc->sc_nsec_cycle_ratio) >> 32;
+       if (cycles == 0)
+               cycles = 1;
 
-       now = amptimer_readcnt64(sc);
-
-       while (pc->pc_nexttickevent <= now) {
-               pc->pc_nexttickevent += sc->sc_ticks_per_intr;
-               pc->pc_ticks_err_sum += sc->sc_ticks_err_cnt;
-
-               /* looping a few times is faster than divide */
-               while (pc->pc_ticks_err_sum  > hz) {
-                       pc->pc_nexttickevent += 1;
-                       pc->pc_ticks_err_sum -= hz;
-               }
-
-#ifdef AMPTIMER_DEBUG
-               sc->sc_clk_count.ec_count++;
-#endif
-               rc = 1;
-               hardclock(frame);
-       }
-       while (pc->pc_nextstatevent <= now) {
-               do {
-                       r = random() & (sc->sc_statvar -1);
-               } while (r == 0); /* random == 0 not allowed */
-               pc->pc_nextstatevent += sc->sc_statmin + r;
-
-               /* XXX - correct nextstatevent? */
-#ifdef AMPTIMER_DEBUG
-               sc->sc_stat_count.ec_count++;
-#endif
-               rc = 1;
-               statclock(frame);
-       }
-
-       if (pc->pc_nexttickevent < pc->pc_nextstatevent)
-               nextevent = pc->pc_nexttickevent;
-       else
-               nextevent = pc->pc_nextstatevent;
-
-#if defined(USE_GTIMER_CMP)
-again:
-       reg = bus_space_read_4(sc->sc_iot, sc->sc_ioh, GTIMER_CTRL);
-       reg &= ~GTIMER_CTRL_COMP;
-       bus_space_write_4(sc->sc_iot, sc->sc_ioh, GTIMER_CTRL, reg);
-       bus_space_write_4(sc->sc_iot, sc->sc_ioh, GTIMER_CMP_LOW,
-           nextevent & 0xffffffff);
-       bus_space_write_4(sc->sc_iot, sc->sc_ioh, GTIMER_CMP_HIGH,
-           nextevent >> 32);
-       reg |= GTIMER_CTRL_COMP;
-       bus_space_write_4(sc->sc_iot, sc->sc_ioh, GTIMER_CTRL, reg);
-
-       now = amptimer_readcnt64(sc);
-       if (now >= nextevent) {
-               nextevent = now + skip;
-               skip += 1;
-               goto again;
-       }
-#else
        /* clear old status */
        bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_STATUS,
            PTIMER_STATUS_EVENT);
 
-       delay = nextevent - now;
-       if (delay < 0)
-               delay = 1;
-
+       /*
+        * Make sure the private timer counter and interrupts are enabled.
+        * XXX Why wouldn't they be?
+        */
        reg = bus_space_read_4(sc->sc_iot, sc->sc_pioh, PTIMER_CTRL);
        if ((reg & (PTIMER_CTRL_ENABLE | PTIMER_CTRL_IRQEN)) !=
            (PTIMER_CTRL_ENABLE | PTIMER_CTRL_IRQEN))
                bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_CTRL,
                    (PTIMER_CTRL_ENABLE | PTIMER_CTRL_IRQEN));
 
-       bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_LOAD, delay);
-#endif
+       /* Start the downcounter. */
+       bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_LOAD, cycles);
+}
 
-       return (rc);
+void
+amptimer_trigger(void *cookie)
+{
+       struct amptimer_softc *sc = cookie;
+
+       /* Clear event flag, then arm counter to fire immediately. */
+       bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_STATUS,
+           PTIMER_STATUS_EVENT);
+       bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_LOAD, 1);
+}
+
+int
+amptimer_intr(void *frame)
+{
+       return clockintr_dispatch(frame);
 }
 
 void
@@ -334,7 +271,12 @@ amptimer_set_clockrate(int32_t new_frequ
                return;
 
        sc->sc_ticks_per_second = amptimer_frequency;
+       sc->sc_nsec_cycle_ratio =
+           sc->sc_ticks_per_second * (1ULL << 32) / 1000000000;
+       sc->sc_nsec_max = UINT64_MAX / sc->sc_nsec_cycle_ratio;
+
        amptimer_timecounter.tc_frequency = sc->sc_ticks_per_second;
+
        printf("amptimer0: adjusting clock: new rate %d kHz\n",
            sc->sc_ticks_per_second / 1000);
 }
@@ -343,54 +285,27 @@ void
 amptimer_cpu_initclocks(void)
 {
        struct amptimer_softc   *sc = amptimer_cd.cd_devs[0];
-       struct amptimer_pcpu_softc *pc = &sc->sc_pstat[CPU_INFO_UNIT(curcpu())];
-       uint64_t                 next;
-#if defined(USE_GTIMER_CMP)
-       uint32_t                 reg;
-#endif
 
        stathz = hz;
        profhz = hz * 10;
+       clockintr_init(CL_RNDSTAT);
 
        if (sc->sc_ticks_per_second != amptimer_frequency) {
                amptimer_set_clockrate(amptimer_frequency);
        }
 
-       amptimer_setstatclockrate(stathz);
-
-       sc->sc_ticks_per_intr = sc->sc_ticks_per_second / hz;
-       sc->sc_ticks_err_cnt = sc->sc_ticks_per_second % hz;
-       pc->pc_ticks_err_sum = 0;
-
        /* establish interrupts */
        /* XXX - irq */
-#if defined(USE_GTIMER_CMP)
-       ampintc_intr_establish(27, IST_EDGE_RISING, IPL_CLOCK,
-           NULL, amptimer_intr, NULL, "tick");
-#else
        ampintc_intr_establish(29, IST_EDGE_RISING, IPL_CLOCK,
            NULL, amptimer_intr, NULL, "tick");
-#endif
 
-       next = amptimer_readcnt64(sc) + sc->sc_ticks_per_intr;
-       pc->pc_nexttickevent = pc->pc_nextstatevent = next;
-
-#if defined(USE_GTIMER_CMP)
-       reg = bus_space_read_4(sc->sc_iot, sc->sc_ioh, GTIMER_CTRL);
-       reg &= ~GTIMER_CTRL_COMP;
-       bus_space_write_4(sc->sc_iot, sc->sc_ioh, GTIMER_CTRL, reg);
-       bus_space_write_4(sc->sc_iot, sc->sc_ioh, GTIMER_CMP_LOW,
-           next & 0xffffffff);
-       bus_space_write_4(sc->sc_iot, sc->sc_ioh, GTIMER_CMP_HIGH,
-           next >> 32);
-       reg |= GTIMER_CTRL_COMP | GTIMER_CTRL_IRQ;
-       bus_space_write_4(sc->sc_iot, sc->sc_ioh, GTIMER_CTRL, reg);
-#else
+       /* Enable private timer counter and interrupt. */
        bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_CTRL,
            (PTIMER_CTRL_ENABLE | PTIMER_CTRL_IRQEN));
-       bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_LOAD,
-           sc->sc_ticks_per_intr);
-#endif
+
+       /* Start the clock interrupt cycle. */
+       clockintr_cpu_init(&amptimer_intrclock);
+       clockintr_trigger();
 }
 
 void
@@ -429,39 +344,12 @@ amptimer_delay(u_int usecs)
 void
 amptimer_setstatclockrate(int newhz)
 {
-       struct amptimer_softc   *sc = amptimer_cd.cd_devs[0];
-       int                      minint, statint;
-       int                      s;
-
-       s = splclock();
-
-       statint = sc->sc_ticks_per_second / newhz;
-       /* calculate largest 2^n which is smaller that just over half statint */
-       sc->sc_statvar = 0x40000000; /* really big power of two */
-       minint = statint / 2 + 100;
-       while (sc->sc_statvar > minint)
-               sc->sc_statvar >>= 1;
-
-       sc->sc_statmin = statint - (sc->sc_statvar >> 1);
-
-       splx(s);
-
-       /*
-        * XXX this allows the next stat timer to occur then it switches
-        * to the new frequency. Rather than switching instantly.
-        */
+       clockintr_setstatclockrate(newhz);
 }
 
 void
 amptimer_startclock(void)
 {
-       struct amptimer_softc   *sc = amptimer_cd.cd_devs[0];
-       struct amptimer_pcpu_softc *pc = &sc->sc_pstat[CPU_INFO_UNIT(curcpu())];
-       uint64_t nextevent;
-
-       nextevent = amptimer_readcnt64(sc) + sc->sc_ticks_per_intr;
-       pc->pc_nexttickevent = pc->pc_nextstatevent = nextevent;
-       
-       bus_space_write_4(sc->sc_iot, sc->sc_pioh, PTIMER_LOAD,
-               sc->sc_ticks_per_intr);
+       clockintr_cpu_init(&amptimer_intrclock);
+       clockintr_trigger();
 }

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to