Cal skew: use static must

use CSR instead of CSRStatus
add FSM to support CSR

src/gateware/si549.py

@@ -150,11 +150,9 @@ class ADPLLProgrammer(Module):
         fsm = FSM()
         self.submodules += fsm
 
-        adpll = Signal.like(self.adpll)
-
         fsm.act("IDLE",
             If(self.stb,
-                NextValue(adpll, self.adpll),
+                NextValue(self.nack, 0),
                 NextState("START")
             )
         )
@@ -174,47 +172,47 @@ class ADPLLProgrammer(Module):
                 If(master.ack,
                     NextState("DATA0")
                 ).Else(
-                    self.nack.eq(1),
+                    NextValue(self.nack, 1),
                     NextState("STOP")
                 )
             )
         )
         fsm.act("DATA0",
-            master.data.eq(adpll[0:8]),
+            master.data.eq(self.adpll[0:8]),
             master.write.eq(1),
             If(master.ready,
                 If(master.ack,
                     NextState("DATA1")
                 ).Else(
-                    self.nack.eq(1),
+                    NextValue(self.nack, 1),
                     NextState("STOP")
                 )
             )
         )
         fsm.act("DATA1",
-            master.data.eq(adpll[8:16]),
+            master.data.eq(self.adpll[8:16]),
             master.write.eq(1),
             If(master.ready,
                 If(master.ack,
                     NextState("DATA2")
                 ).Else(
-                    self.nack.eq(1),
+                    NextValue(self.nack, 1),
                     NextState("STOP")
                 )
             )
         )
         fsm.act("DATA2",
-            master.data.eq(adpll[16:24]),
+            master.data.eq(self.adpll[16:24]),
             master.write.eq(1),
             If(master.ready,
-                If(~master.ack, self.nack.eq(1)),
+                If(~master.ack, NextValue(self.nack, 1)),
                 NextState("STOP")
             )
         )
         fsm.act("STOP",
             master.stop.eq(1),
             If(master.ready,
-                If(~master.ack, self.nack.eq(1)),
+                If(~master.ack, NextValue(self.nack, 1)),
                 NextState("IDLE")
             )
         )
@@ -228,7 +226,7 @@ class Si549(Module, AutoCSR):
         self.i2c_address = CSRStorage(7)
 
         self.adpll = CSRStorage(24)
-        self.adpll_stb = CSRStorage()
+        self.adpll_stb = CSR()
         self.adpll_busy = CSRStatus()
         self.nack = CSRStatus()
 
@@ -244,11 +242,12 @@ class Si549(Module, AutoCSR):
 
         self.submodules.programmer = ADPLLProgrammer()
 
+        self.sync += self.programmer.stb.eq(self.adpll_stb.re)
+
         self.comb += [
             self.programmer.i2c_divider.eq(self.i2c_divider.storage),
             self.programmer.i2c_address.eq(self.i2c_address.storage),
             self.programmer.adpll.eq(self.adpll.storage),
-            self.programmer.stb.eq(self.adpll_stb.storage),
             self.adpll_busy.status.eq(self.programmer.busy),
             self.nack.status.eq(self.programmer.nack)
         ]

src/gateware/wrpll.py

@@ -7,42 +7,72 @@ from ddmtd import DDMTDSampler, DDMTD
 from si549 import Si549
 
 class FrequencyCounter(Module, AutoCSR):
-    def __init__(self, domains, counter_width=24):
+    def __init__(self, domains, counter_width=24, freq_div=2):
+        self.update = CSR()
+        self.busy = CSRStatus()
+
+        counter_reset = Signal()
+        counter_stb = Signal()
+        timer = Signal(counter_width)
+
+        # # #
+
+        fsm = FSM()
+        self.submodules += fsm
+
+        fsm.act("IDLE",
+            If(self.update.re,
+                counter_reset.eq(1),
+                NextValue(timer, 2**counter_width - 1),
+                NextState("COUNTING")
+            )
+        )
+        fsm.act("COUNTING",
+            self.busy.status.eq(1),
+            If(timer != 0,
+                NextValue(timer, timer - 1)
+            ).Else( 
+                counter_stb.eq(1),
+                NextState("IDLE")
+            )
+        )
+
         for domain in domains:
             name = "counter_" + domain
-            counter = CSRStatus(counter_width, name=name)
-            setattr(self, name, counter)
-        self.update_en = CSRStorage()
+            counter_csr = CSRStatus(counter_width, name=name)
+            setattr(self, name, counter_csr)
 
-        timer = Signal(counter_width)
-        timer_tick = Signal()
-        self.sync += Cat(timer, timer_tick).eq(timer + 1)
+            counter = Signal(max=freq_div)
+            result = Signal(counter_width)
 
-        for domain in domains:
-            sync_domain = getattr(self.sync, domain)
-            divider = Signal(2)
-            sync_domain += divider.eq(divider + 1)
-
-            divided = Signal()
-            sync_domain += divided.eq(divider[-1])
-            divided_sys = Signal()
-            self.specials += MultiReg(divided, divided_sys)
-
-            divided_sys_r = Signal()
-            divided_tick = Signal()
-            self.sync += divided_sys_r.eq(divided_sys)
-            self.comb += divided_tick.eq(divided_sys & ~divided_sys_r)
-
-            counter = Signal(counter_width)
-            counter_csr = getattr(self, "counter_" + domain)
-            self.sync += [
-                If(timer_tick,
-                    If(self.update_en.storage, counter_csr.status.eq(counter)),
-                    counter.eq(0),
-                ).Else(
-                    If(divided_tick, counter.eq(counter + 1))
-                )
+            # # #
+            
+            reset_ps = PulseSynchronizer("sys", domain)
+            stb_ps = PulseSynchronizer("sys", domain)
+            self.submodules +=[
+                reset_ps,
+                stb_ps
             ]
+            self.sync +=[
+                reset_ps.i.eq(counter_reset),
+                stb_ps.i.eq(counter_stb)
+            ]
+
+            sync_domain = getattr(self.sync, domain)
+            sync_domain += [
+                If(counter != 0,
+                    counter.eq(counter - 1)
+                ).Else(
+                    result.eq(result + 1),
+                    counter.eq(freq_div - 1)
+                ),
+                If(reset_ps.o, 
+                    counter.eq(0),
+                    result.eq(0)
+                ),
+                If(stb_ps.o, counter_csr.status.eq(result))
+            ]
+
 
 class SkewTester(Module, AutoCSR):
     def __init__(self, rx_synchronizer):
@@ -133,7 +163,7 @@ class WRPLL(Module, AutoCSR):
             )
         ]
 
-        # PL->PS interrupt
+        # EventMangers for firmware interrupt
 
         self.submodules.ref_tag_ev = EventManager()
         self.ref_tag_ev.stb = EventSourcePulse()
@@ -178,7 +208,7 @@ class SMAFrequencyMultiplier(Module, AutoCSR):
             Instance("IBUFDS",
                 i_I=sma_clkin.p, i_IB=sma_clkin.n,
                 o_O=sma_clkin_se),
-            # MMCME2 is capable to accept 10Mhz input while PLLE2 only support down to 19Mhz input (DS191)
+            # MMCME2 is capable to accept 10MHz input while PLLE2 only support down to 19MHz input (DS191)
             # The MMCME2 can be reconfiged during runtime using the Dynamic Reconfiguration Ports
             Instance("MMCME2_ADV",
                 p_BANDWIDTH="LOW",  # lower jitter
@@ -189,11 +219,11 @@ class SMAFrequencyMultiplier(Module, AutoCSR):
                 i_CLKIN1=sma_clkin_se,
                 i_CLKINSEL=1,  # 1=CLKIN1 0=CLKIN2
 
-                # VCO @ 1.25Ghz 
+                # VCO @ 1.25GHz 
                 p_CLKFBOUT_MULT_F=10, p_DIVCLK_DIVIDE=1,
                 i_CLKFBIN=mmcm_fb_clk, o_CLKFBOUT=mmcm_fb_clk, 
 
-                # 125Mhz for WRPLL
+                # 125MHz for WRPLL
                 p_CLKOUT0_DIVIDE_F=10, p_CLKOUT0_PHASE=0.0, o_CLKOUT0=ref_clk,
 
                 # Dynamic Reconfiguration Ports

src/libboard_artiq/src/si549.rs

@@ -284,8 +284,6 @@ pub fn helper_setup(timer: &mut GlobalTimer, settings: &FrequencySetting) -> Res
     Ok(())
 }
 
-/// set adpll using gateware i2c
-/// Note: disable main/helper i2c bitbang before using this function
 fn set_adpll(dcxo: i2c::DCXO, adpll: i32) -> Result<(), &'static str> {
     if adpll.abs() > ADPLL_MAX {
         return Err("adpll is too large");
@@ -298,15 +296,14 @@ fn set_adpll(dcxo: i2c::DCXO, adpll: i32) -> Result<(), &'static str> {
             }
 
             while csr::wrpll::main_dcxo_adpll_busy_read() == 1 {}
+            if csr::wrpll::main_dcxo_nack_read() == 1 {
+                return Err("Main si549 failed to ack adpll write");
+            }
+
             csr::wrpll::main_dcxo_i2c_address_write(ADDRESS);
             csr::wrpll::main_dcxo_adpll_write(adpll as u32);
 
             csr::wrpll::main_dcxo_adpll_stb_write(1);
-            csr::wrpll::main_dcxo_adpll_stb_write(0);
-
-            if csr::wrpll::main_dcxo_nack_read() == 1 {
-                return Err("Main si549 failed to ack adpll write");
-            }
         },
         i2c::DCXO::Helper => unsafe {
             if csr::wrpll::helper_dcxo_bitbang_enable_read() == 1 {
@@ -314,15 +311,14 @@ fn set_adpll(dcxo: i2c::DCXO, adpll: i32) -> Result<(), &'static str> {
             }
 
             while csr::wrpll::helper_dcxo_adpll_busy_read() == 1 {}
+            if csr::wrpll::helper_dcxo_nack_read() == 1 {
+                return Err("Helper si549 failed to ack adpll write");
+            }
+
             csr::wrpll::helper_dcxo_i2c_address_write(ADDRESS);
             csr::wrpll::helper_dcxo_adpll_write(adpll as u32);
 
             csr::wrpll::helper_dcxo_adpll_stb_write(1);
-            csr::wrpll::helper_dcxo_adpll_stb_write(0);
-
-            if csr::wrpll::helper_dcxo_nack_read() == 1 {
-                return Err("Helper si549 failed to ack adpll write");
-            }
         },
     };
 
@@ -332,21 +328,19 @@ fn set_adpll(dcxo: i2c::DCXO, adpll: i32) -> Result<(), &'static str> {
 #[cfg(has_wrpll)]
 pub mod wrpll {
 
-    use libcortex_a9::mutex::Mutex;
-
     use super::*;
 
     const BEATING_PERIOD: i32 = 0x8000;
     const BEATING_HALFPERIOD: i32 = 0x4000;
-    const TIMER_WIDTH: u32 = 24;
-    const COUNTER_DIV: u32 = 2;
+    const COUNTER_WIDTH: u32 = 24;
+    const FREQ_DIV: u32 = 2;
 
     const KP: i32 = 6;
     const KI: i32 = 2;
 
-    static BASE_ADPLL: Mutex<i32> = Mutex::new(0);
-    static H_INTEGRATOR: Mutex<i32> = Mutex::new(0);
-    static M_INTEGRATOR: Mutex<i32> = Mutex::new(0);
+    static mut BASE_ADPLL: i32 = 0;
+    static mut H_INTEGRATOR: i32 = 0;
+    static mut M_INTEGRATOR: i32 = 0;
 
     #[derive(Clone, Copy)]
     pub enum ISR {
@@ -358,55 +352,61 @@ pub mod wrpll {
         use super::*;
 
         #[cfg(wrpll_ref_clk = "GT_CDR")]
-        static TAG_OFFSET: Mutex<u32> = Mutex::new(19050);
+        static mut TAG_OFFSET: u32 = 19050;
         #[cfg(wrpll_ref_clk = "SMA_CLKIN")]
-        static TAG_OFFSET: Mutex<u32> = Mutex::new(0);
-        static REF_TAG: Mutex<u32> = Mutex::new(0);
-        static REF_TAG_READY: Mutex<bool> = Mutex::new(false);
-        static MAIN_TAG: Mutex<u32> = Mutex::new(0);
-        static MAIN_TAG_READY: Mutex<bool> = Mutex::new(false);
+        static mut TAG_OFFSET: u32 = 0;
+        static mut REF_TAG: u32 = 0;
+        static mut REF_TAG_READY: bool = false;
+        static mut MAIN_TAG: u32 = 0;
+        static mut MAIN_TAG_READY: bool = false;
 
         pub fn reset() {
             clear_phase_diff_ready();
-            *REF_TAG.lock() = 0;
-            *MAIN_TAG.lock() = 0;
+            unsafe {
+                REF_TAG = 0;
+                MAIN_TAG = 0;
+            }
         }
 
         pub fn clear_phase_diff_ready() {
-            *REF_TAG_READY.lock() = false;
-            *MAIN_TAG_READY.lock() = false;
+            unsafe {
+                REF_TAG_READY = false;
+                MAIN_TAG_READY = false;
+            }
         }
 
         pub fn collect_tags(interrupt: ISR) {
             match interrupt {
-                ISR::RefTag => {
-                    *REF_TAG.lock() = unsafe { csr::wrpll::ref_tag_read() };
-                    *REF_TAG_READY.lock() = true;
-                }
-                ISR::MainTag => {
-                    *MAIN_TAG.lock() = unsafe { csr::wrpll::main_tag_read() };
-                    *MAIN_TAG_READY.lock() = true;
-                }
+                ISR::RefTag => unsafe {
+                    REF_TAG = csr::wrpll::ref_tag_read();
+                    REF_TAG_READY = true;
+                },
+                ISR::MainTag => unsafe {
+                    MAIN_TAG = csr::wrpll::main_tag_read();
+                    MAIN_TAG_READY = true;
+                },
             }
         }
 
         pub fn phase_diff_ready() -> bool {
-            *REF_TAG_READY.lock() && *MAIN_TAG_READY.lock()
+            unsafe { REF_TAG_READY && MAIN_TAG_READY }
         }
 
         #[cfg(feature = "calibrate_wrpll_skew")]
         pub fn set_tag_offset(offset: u32) {
-            *TAG_OFFSET.lock() = offset;
+            unsafe {
+                TAG_OFFSET = offset;
+            }
         }
 
         #[cfg(feature = "calibrate_wrpll_skew")]
         pub fn get_tag_offset() -> u32 {
-            *TAG_OFFSET.lock()
+            unsafe { TAG_OFFSET }
         }
 
         pub fn get_period_error() -> i32 {
             // n * BEATING_PERIOD - REF_TAG(n) mod BEATING_PERIOD
-            let mut period_error = (*REF_TAG.lock()).overflowing_neg().0.rem_euclid(BEATING_PERIOD as u32) as i32;
+            let mut period_error = unsafe { REF_TAG.overflowing_neg().0.rem_euclid(BEATING_PERIOD as u32) as i32 };
 
             // mapping tags from [0, 2π] -> [-π, π]
             if period_error > BEATING_HALFPERIOD {
@@ -417,10 +417,12 @@ pub mod wrpll {
 
         pub fn get_phase_error() -> i32 {
             // MAIN_TAG(n) - REF_TAG(n) - TAG_OFFSET mod BEATING_PERIOD
-            let mut phase_error = (*MAIN_TAG.lock())
-                .overflowing_sub(*REF_TAG.lock() + *TAG_OFFSET.lock())
-                .0
-                .rem_euclid(BEATING_PERIOD as u32) as i32;
+            let mut phase_error = unsafe {
+                MAIN_TAG
+                    .overflowing_sub(REF_TAG + TAG_OFFSET)
+                    .0
+                    .rem_euclid(BEATING_PERIOD as u32) as i32
+            };
 
             // mapping tags from [0, 2π] -> [-π, π]
             if phase_error > BEATING_HALFPERIOD {
@@ -439,24 +441,23 @@ pub mod wrpll {
     }
 
     /// To get within capture range
-    fn set_base_adpll(timer: &mut GlobalTimer) -> Result<(), &'static str> {
+    fn set_base_adpll() -> Result<(), &'static str> {
         let count2adpll =
-            |error: i32| (((error) as f64 * 1e6) / (0.0001164 * (1 << (TIMER_WIDTH - COUNTER_DIV)) as f64)) as i32;
-
-        let (ref_count, main_count) = get_freq_counts(timer);
-        let mut base_adpll_lock = BASE_ADPLL.lock();
-        *base_adpll_lock = count2adpll(ref_count as i32 - main_count as i32);
-        set_adpll(i2c::DCXO::Main, *base_adpll_lock)?;
-        set_adpll(i2c::DCXO::Helper, *base_adpll_lock)?;
+            |error: i32| ((error as f64 * FREQ_DIV as f64 * 1e6) / (0.0001164 * (1 << COUNTER_WIDTH) as f64)) as i32;
 
+        let (ref_count, main_count) = get_freq_counts();
+        unsafe {
+            BASE_ADPLL = count2adpll(ref_count as i32 - main_count as i32);
+            set_adpll(i2c::DCXO::Main, BASE_ADPLL)?;
+            set_adpll(i2c::DCXO::Helper, BASE_ADPLL)?;
+        }
         Ok(())
     }
 
-    fn get_freq_counts(timer: &mut GlobalTimer) -> (u32, u32) {
+    fn get_freq_counts() -> (u32, u32) {
         unsafe {
-            csr::wrpll::frequency_counter_update_en_write(1);
-            timer.delay_us(150_000); // 8ns << TIMER_WIDTH
-            csr::wrpll::frequency_counter_update_en_write(0);
+            csr::wrpll::frequency_counter_update_write(1);
+            while csr::wrpll::frequency_counter_busy_read() == 1 {}
             #[cfg(wrpll_ref_clk = "GT_CDR")]
             let ref_count = csr::wrpll::frequency_counter_counter_rtio_rx0_read();
             #[cfg(wrpll_ref_clk = "SMA_CLKIN")]
@@ -467,11 +468,15 @@ pub mod wrpll {
         }
     }
 
-    fn reset_plls() -> Result<(), &'static str> {
-        *H_INTEGRATOR.lock() = 0;
-        *M_INTEGRATOR.lock() = 0;
+    fn reset_plls(timer: &mut GlobalTimer) -> Result<(), &'static str> {
+        unsafe {
+            H_INTEGRATOR = 0;
+            M_INTEGRATOR = 0;
+        }
         set_adpll(i2c::DCXO::Main, 0)?;
         set_adpll(i2c::DCXO::Helper, 0)?;
+        // wait for adpll to transfer and DCXO to settle
+        timer.delay_us(200);
         Ok(())
     }
 
@@ -510,27 +515,25 @@ pub mod wrpll {
 
     fn helper_pll() -> Result<(), &'static str> {
         let period_err = tag_collector::get_period_error();
-        let mut integrator_lock = H_INTEGRATOR.lock();
 
-        *integrator_lock += period_err * KI;
-        let mut h_adpll = *BASE_ADPLL.lock() + period_err * KP + *integrator_lock;
-
-        h_adpll = h_adpll.clamp(-ADPLL_MAX, ADPLL_MAX);
+        let h_adpll;
+        unsafe {
+            H_INTEGRATOR += period_err * KI;
+            h_adpll = (BASE_ADPLL + period_err * KP + H_INTEGRATOR).clamp(-ADPLL_MAX, ADPLL_MAX);
+        }
         set_adpll(i2c::DCXO::Helper, h_adpll)?;
-
         Ok(())
     }
 
     fn main_pll() -> Result<(), &'static str> {
         let phase_err = tag_collector::get_phase_error();
-        let mut integrator_lock = M_INTEGRATOR.lock();
 
-        *integrator_lock += phase_err * KI;
-        let mut m_adpll = *BASE_ADPLL.lock() + phase_err * KP + *integrator_lock;
-
-        m_adpll = m_adpll.clamp(-ADPLL_MAX, ADPLL_MAX);
+        let m_adpll;
+        unsafe {
+            M_INTEGRATOR += phase_err * KI;
+            m_adpll = (BASE_ADPLL + phase_err * KP + M_INTEGRATOR).clamp(-ADPLL_MAX, ADPLL_MAX);
+        }
         set_adpll(i2c::DCXO::Main, m_adpll)?;
-
         Ok(())
     }
 
@@ -646,12 +649,8 @@ pub mod wrpll {
 
         if rc {
             tag_collector::reset();
-            reset_plls().expect("failed to reset main and helper PLL");
-
-            info!("warming up refclk...");
-            // refclk need a couple seconds for freq counter to read it properly
-            timer.delay_us(20_000_000);
-            set_base_adpll(timer).expect("failed to set base adpll");
+            reset_plls(timer).expect("failed to reset main and helper PLL");
+            set_base_adpll().expect("failed to set base adpll");
 
             // clear gateware pending flag
             clear_pending(ISR::RefTag);
@@ -733,7 +732,7 @@ pub mod wrpll_refclk {
     fn read(address: u8) -> u16 {
         set_addr(address);
         set_enable(true);
-        // Set DADDR on the MMCM and assert DEN for one clock cycle
+        // Set DADDR on the mmcm and assert DEN for one clock cycle
         one_clock_cycle();
 
         set_enable(false);
@@ -749,7 +748,7 @@ pub mod wrpll_refclk {
         set_data(value);
         set_write_enable(true);
         set_enable(true);
-        // Set DADDR, DI on the MMCM and assert DWE, DEN for one clock cycle
+        // Set DADDR, DI on the mmcm and assert DWE, DEN for one clock cycle
         one_clock_cycle();
 
         set_write_enable(false);
@@ -799,7 +798,7 @@ pub mod wrpll_refclk {
 
             let locked = unsafe { csr::wrpll_refclk::mmcm_locked_read() == 1 };
             if !locked {
-                return Err("mmcm failed to generate 125Mhz ref clock from SMA CLKIN");
+                return Err("mmcm failed to generate 125MHz ref clock from SMA CLKIN");
             }
         }
 

src/runtime/src/rtio_clocking.rs

@@ -380,7 +380,7 @@ fn get_si549_setting(clk: RtioClock) -> si549::FrequencySetting {
                     fbdiv: 0x046C5F49797,
                 },
                 helper: si549::DividerConfig {
-                    // 100Mhz*32767/32768
+                    // 100MHz*32767/32768
                     hsdiv: 0x06C,
                     lsdiv: 0,
                     fbdiv: 0x046C5670BBD,
@@ -388,7 +388,7 @@ fn get_si549_setting(clk: RtioClock) -> si549::FrequencySetting {
             }
         }
         _ => {
-            // Everything else use 125Mhz
+            // Everything else use 125MHz
             si549::FrequencySetting {
                 main: si549::DividerConfig {
                     hsdiv: 0x058,
@@ -396,7 +396,7 @@ fn get_si549_setting(clk: RtioClock) -> si549::FrequencySetting {
                     fbdiv: 0x04815791F25,
                 },
                 helper: si549::DividerConfig {
-                    // 125Mhz*32767/32768
+                    // 125MHz*32767/32768
                     hsdiv: 0x058,
                     lsdiv: 0,
                     fbdiv: 0x04814E8F442,

src/satman/src/main.rs

@@ -838,7 +838,7 @@ const SI549_SETTINGS: si549::FrequencySetting = si549::FrequencySetting {
         fbdiv: 0x04815791F25,
     },
     helper: si549::DividerConfig {
-        // 125Mhz*32767/32768
+        // 125MHz*32767/32768
         hsdiv: 0x058,
         lsdiv: 0,
         fbdiv: 0x04814E8F442,
@@ -846,14 +846,14 @@ const SI549_SETTINGS: si549::FrequencySetting = si549::FrequencySetting {
 };
 
 #[cfg(all(has_si549, rtio_frequency = "100.0"))]
-pub const SI549_SETTINGS: si549::FrequencySetting = si549::FrequencySetting {
+const SI549_SETTINGS: si549::FrequencySetting = si549::FrequencySetting {
     main: si549::DividerConfig {
         hsdiv: 0x06C,
         lsdiv: 0,
         fbdiv: 0x046C5F49797,
     },
     helper: si549::DividerConfig {
-        // 100Mhz*32767/32768
+        // 100MHz*32767/32768
         hsdiv: 0x06C,
         lsdiv: 0,
         fbdiv: 0x046C5670BBD,