From 5763a2e436784e9006958013ba6e756c22aa8f0f Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Wed, 16 Aug 2023 18:02:01 +0100 Subject: [PATCH 1/4] timer: Take &ReferenceClock rather than &ClocksManager ClocksManager may have been de-structured by the time a timer is created. The source of the Timer is the ref-clock (shared with the watchdog). --- rp2040-hal/examples/adc_fifo_dma.rs | 2 +- rp2040-hal/examples/adc_fifo_poll.rs | 2 +- rp2040-hal/examples/blinky.rs | 2 +- rp2040-hal/examples/vector_table.rs | 2 +- rp2040-hal/src/timer.rs | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rp2040-hal/examples/adc_fifo_dma.rs b/rp2040-hal/examples/adc_fifo_dma.rs index ad237df80..e6159c4ca 100644 --- a/rp2040-hal/examples/adc_fifo_dma.rs +++ b/rp2040-hal/examples/adc_fifo_dma.rs @@ -146,7 +146,7 @@ fn main() -> ! { adc_fifo.resume(); // initialize a timer, to measure the total sampling time (printed below) - let timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks); + let timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks.reference_clock); // NOTE: in a real-world program, instead of calling `wait` now, you would probably: // 1. Enable one of the DMA interrupts for the channel (e.g. `dma.ch0.enable_irq0()`) diff --git a/rp2040-hal/examples/adc_fifo_poll.rs b/rp2040-hal/examples/adc_fifo_poll.rs index 3f5a6fe2e..b61ca486c 100644 --- a/rp2040-hal/examples/adc_fifo_poll.rs +++ b/rp2040-hal/examples/adc_fifo_poll.rs @@ -133,7 +133,7 @@ fn main() -> ! { let mut i = 0; // initialize a timer, to measure the total sampling time (printed below) - let timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks); + let timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks.reference_clock); loop { // busy-wait until the FIFO contains at least two samples: diff --git a/rp2040-hal/examples/blinky.rs b/rp2040-hal/examples/blinky.rs index ac55a6f8d..1d14ff4d5 100644 --- a/rp2040-hal/examples/blinky.rs +++ b/rp2040-hal/examples/blinky.rs @@ -64,7 +64,7 @@ fn main() -> ! { .ok() .unwrap(); - let mut timer = rp2040_hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks); + let mut timer = rp2040_hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks.reference_clock); // The single-cycle I/O block controls our GPIO pins let sio = hal::Sio::new(pac.SIO); diff --git a/rp2040-hal/examples/vector_table.rs b/rp2040-hal/examples/vector_table.rs index b1c083069..9b3766e31 100644 --- a/rp2040-hal/examples/vector_table.rs +++ b/rp2040-hal/examples/vector_table.rs @@ -110,7 +110,7 @@ fn main() -> ! { // Configure GPIO25 as an output let led_pin = pins.gpio25.into_push_pull_output(); - let mut timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks); + let mut timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks.reference_clock); critical_section::with(|cs| { let mut alarm = timer.alarm_0().unwrap(); // Schedule an alarm in 1 second diff --git a/rp2040-hal/src/timer.rs b/rp2040-hal/src/timer.rs index ca6832a3c..4f8f9dff5 100644 --- a/rp2040-hal/src/timer.rs +++ b/rp2040-hal/src/timer.rs @@ -13,7 +13,7 @@ use fugit::{MicrosDurationU32, MicrosDurationU64, TimerInstantU64}; use crate::{ atomic_register_access::{write_bitmask_clear, write_bitmask_set}, - clocks::ClocksManager, + clocks::ReferenceClock, pac::{self, RESETS, TIMER}, resets::SubsystemReset, typelevel::Sealed, @@ -59,7 +59,7 @@ impl Timer { /// Make sure that clocks and watchdog are configured, so /// that timer ticks happen at a frequency of 1MHz. /// Otherwise, `Timer` won't work as expected. - pub fn new(timer: TIMER, resets: &mut RESETS, _clocks: &ClocksManager) -> Self { + pub fn new(timer: TIMER, resets: &mut RESETS, _clocks: &ReferenceClock) -> Self { timer.reset_bring_down(resets); timer.reset_bring_up(resets); Self { _private: () } From 4b9722c3efc900383b43d14c396deee82bc0604e Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Wed, 16 Aug 2023 18:02:30 +0100 Subject: [PATCH 2/4] timer: add an assert to the ref clock operating frequency --- rp2040-hal/src/timer.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rp2040-hal/src/timer.rs b/rp2040-hal/src/timer.rs index 4f8f9dff5..b922bad82 100644 --- a/rp2040-hal/src/timer.rs +++ b/rp2040-hal/src/timer.rs @@ -16,7 +16,7 @@ use crate::{ clocks::ReferenceClock, pac::{self, RESETS, TIMER}, resets::SubsystemReset, - typelevel::Sealed, + typelevel::Sealed, Clock, }; /// Instant type used by the Timer & Alarm methods. @@ -59,7 +59,8 @@ impl Timer { /// Make sure that clocks and watchdog are configured, so /// that timer ticks happen at a frequency of 1MHz. /// Otherwise, `Timer` won't work as expected. - pub fn new(timer: TIMER, resets: &mut RESETS, _clocks: &ReferenceClock) -> Self { + pub fn new(timer: TIMER, resets: &mut RESETS, clocks: &ReferenceClock) -> Self { + assert_eq!(clocks.freq().to_Hz(), 1_000_000); timer.reset_bring_down(resets); timer.reset_bring_up(resets); Self { _private: () } From f933b0cb975d8e53fe581d8ea6b14cf6b6fe5058 Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Wed, 16 Aug 2023 20:00:25 +0100 Subject: [PATCH 3/4] =?UTF-8?q?timer:=C2=A0Check=20the=20timer's=20source?= =?UTF-8?q?=20is=20ref=5Fclk=20=C3=B7=20watchdog=20tick.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- rp2040-hal/examples/adc_fifo_dma.rs | 7 ++++++- rp2040-hal/examples/adc_fifo_poll.rs | 7 ++++++- rp2040-hal/examples/blinky.rs | 7 ++++++- rp2040-hal/examples/vector_table.rs | 7 ++++++- rp2040-hal/src/clocks/mod.rs | 4 ++-- rp2040-hal/src/timer.rs | 18 ++++++++++++++---- rp2040-hal/src/watchdog.rs | 9 ++++++++- 7 files changed, 48 insertions(+), 11 deletions(-) diff --git a/rp2040-hal/examples/adc_fifo_dma.rs b/rp2040-hal/examples/adc_fifo_dma.rs index e6159c4ca..be3b287ee 100644 --- a/rp2040-hal/examples/adc_fifo_dma.rs +++ b/rp2040-hal/examples/adc_fifo_dma.rs @@ -146,7 +146,12 @@ fn main() -> ! { adc_fifo.resume(); // initialize a timer, to measure the total sampling time (printed below) - let timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks.reference_clock); + let timer = hal::Timer::new( + pac.TIMER, + &mut pac.RESETS, + &watchdog, + &clocks.reference_clock, + ); // NOTE: in a real-world program, instead of calling `wait` now, you would probably: // 1. Enable one of the DMA interrupts for the channel (e.g. `dma.ch0.enable_irq0()`) diff --git a/rp2040-hal/examples/adc_fifo_poll.rs b/rp2040-hal/examples/adc_fifo_poll.rs index b61ca486c..c2c011873 100644 --- a/rp2040-hal/examples/adc_fifo_poll.rs +++ b/rp2040-hal/examples/adc_fifo_poll.rs @@ -133,7 +133,12 @@ fn main() -> ! { let mut i = 0; // initialize a timer, to measure the total sampling time (printed below) - let timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks.reference_clock); + let timer = hal::Timer::new( + pac.TIMER, + &mut pac.RESETS, + &watchdog, + &clocks.reference_clock, + ); loop { // busy-wait until the FIFO contains at least two samples: diff --git a/rp2040-hal/examples/blinky.rs b/rp2040-hal/examples/blinky.rs index 1d14ff4d5..732a1af73 100644 --- a/rp2040-hal/examples/blinky.rs +++ b/rp2040-hal/examples/blinky.rs @@ -64,7 +64,12 @@ fn main() -> ! { .ok() .unwrap(); - let mut timer = rp2040_hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks.reference_clock); + let mut timer = rp2040_hal::Timer::new( + pac.TIMER, + &mut pac.RESETS, + &watchdog, + &clocks.reference_clock, + ); // The single-cycle I/O block controls our GPIO pins let sio = hal::Sio::new(pac.SIO); diff --git a/rp2040-hal/examples/vector_table.rs b/rp2040-hal/examples/vector_table.rs index 9b3766e31..8ed12fa1f 100644 --- a/rp2040-hal/examples/vector_table.rs +++ b/rp2040-hal/examples/vector_table.rs @@ -110,7 +110,12 @@ fn main() -> ! { // Configure GPIO25 as an output let led_pin = pins.gpio25.into_push_pull_output(); - let mut timer = hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks.reference_clock); + let mut timer = hal::Timer::new( + pac.TIMER, + &mut pac.RESETS, + &watchdog, + &clocks.reference_clock, + ); critical_section::with(|cs| { let mut alarm = timer.alarm_0().unwrap(); // Schedule an alarm in 1 second diff --git a/rp2040-hal/src/clocks/mod.rs b/rp2040-hal/src/clocks/mod.rs index 5f280a5c5..3a757fbbc 100644 --- a/rp2040-hal/src/clocks/mod.rs +++ b/rp2040-hal/src/clocks/mod.rs @@ -26,7 +26,7 @@ //! let xosc = setup_xosc_blocking(peripherals.XOSC, XOSC_CRYSTAL_FREQ.Hz()).map_err(InitError::XoscErr)?; //! //! // Start tick in watchdog -//! watchdog.enable_tick_generation((XOSC_CRYSTAL_FREQ / 1_000_000) as u8); +//! watchdog.enable_tick_generation((XOSC_CRYSTAL_FREQ / 1_000_000) as u16); //! //! let mut clocks = ClocksManager::new(peripherals.CLOCKS); //! @@ -330,7 +330,7 @@ pub fn init_clocks_and_plls( let xosc = setup_xosc_blocking(xosc_dev, xosc_crystal_freq.Hz()).map_err(InitError::XoscErr)?; // Configure watchdog tick generation to tick over every microsecond - watchdog.enable_tick_generation((xosc_crystal_freq / 1_000_000) as u8); + watchdog.enable_tick_generation((xosc_crystal_freq / 1_000_000) as u16); let mut clocks = ClocksManager::new(clocks_dev); diff --git a/rp2040-hal/src/timer.rs b/rp2040-hal/src/timer.rs index b922bad82..d55dff021 100644 --- a/rp2040-hal/src/timer.rs +++ b/rp2040-hal/src/timer.rs @@ -16,7 +16,8 @@ use crate::{ clocks::ReferenceClock, pac::{self, RESETS, TIMER}, resets::SubsystemReset, - typelevel::Sealed, Clock, + typelevel::Sealed, + Clock, Watchdog, }; /// Instant type used by the Timer & Alarm methods. @@ -59,8 +60,16 @@ impl Timer { /// Make sure that clocks and watchdog are configured, so /// that timer ticks happen at a frequency of 1MHz. /// Otherwise, `Timer` won't work as expected. - pub fn new(timer: TIMER, resets: &mut RESETS, clocks: &ReferenceClock) -> Self { - assert_eq!(clocks.freq().to_Hz(), 1_000_000); + pub fn new( + timer: TIMER, + resets: &mut RESETS, + watchdog: &Watchdog, + clocks: &ReferenceClock, + ) -> Self { + assert_eq!( + clocks.freq().to_Hz() / u32::from(watchdog.cycles_per_ticks()), + 1_000_000 + ); timer.reset_bring_down(resets); timer.reset_bring_up(resets); Self { _private: () } @@ -185,8 +194,9 @@ impl eh1_0_alpha::delay::DelayUs for Timer { /// // Make sure to initialize clocks, otherwise the timer wouldn't work /// // properly. Omitted here for terseness. /// let clocks: rp2040_hal::clocks::ClocksManager = todo!(); +/// let watchdog: rp2040_hal::Watchdog = todo!(); /// // Configure the Timer peripheral in count-down mode -/// let timer = rp2040_hal::Timer::new(pac.TIMER, &mut pac.RESETS, &clocks); +/// let timer = rp2040_hal::Timer::new(pac.TIMER, &mut pac.RESETS, &watchdog, &clocks.reference_clock); /// let mut count_down = timer.count_down(); /// // Create a count_down timer for 500 milliseconds /// count_down.start(500.millis()); diff --git a/rp2040-hal/src/watchdog.rs b/rp2040-hal/src/watchdog.rs index bef51cc35..d2da7d7aa 100644 --- a/rp2040-hal/src/watchdog.rs +++ b/rp2040-hal/src/watchdog.rs @@ -60,14 +60,21 @@ impl Watchdog { /// /// * `cycles` - Total number of tick cycles before the next tick is generated. /// It is expected to be the frequency in MHz of clk_ref. - pub fn enable_tick_generation(&mut self, cycles: u8) { + pub fn enable_tick_generation(&mut self, cycles: u16) { const WATCHDOG_TICK_ENABLE_BITS: u32 = 0x200; + assert_eq!(cycles & !0x1FF, 0); + self.watchdog .tick .write(|w| unsafe { w.bits(WATCHDOG_TICK_ENABLE_BITS | cycles as u32) }) } + /// Returns the number of `clk_ref` cycles between each watchdog (and Timer) ticks. + pub fn cycles_per_ticks(&self) -> u16 { + self.watchdog.tick.read().cycles().bits() + } + /// Defines whether or not the watchdog timer should be paused when processor(s) are in debug mode /// or when JTAG is accessing bus fabric /// From bc3add6ba39841df166d65cbede22bb663b3bab3 Mon Sep 17 00:00:00 2001 From: Wilfried Chauveau Date: Fri, 18 Aug 2023 17:41:12 +0100 Subject: [PATCH 4/4] Timer: Use multiplication in assert instead of division. Co-authored-by: Jan Niehusmann --- rp2040-hal/src/timer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rp2040-hal/src/timer.rs b/rp2040-hal/src/timer.rs index d55dff021..a4601ff29 100644 --- a/rp2040-hal/src/timer.rs +++ b/rp2040-hal/src/timer.rs @@ -67,8 +67,8 @@ impl Timer { clocks: &ReferenceClock, ) -> Self { assert_eq!( - clocks.freq().to_Hz() / u32::from(watchdog.cycles_per_ticks()), - 1_000_000 + clocks.freq().to_Hz(), + 1_000_000 * u32::from(watchdog.cycles_per_ticks()) ); timer.reset_bring_down(resets); timer.reset_bring_up(resets);