From 4b5e1c9d8c01bae217fd65538ae69c4f6bc2e9bf Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Thu, 4 Dec 2025 22:18:35 +0100 Subject: [PATCH 1/3] RMT: add HIL test for various clk dividers This is currently broken due to undocumented timing/ordering constraints of the peripheral, which leads to erroneous behavior when the sclk divider is large (i.e. presuamably when the RMT sclk is slow compared to some bus clock?). --- hil-test/src/bin/rmt.rs | 79 +++++++++++++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 15 deletions(-) diff --git a/hil-test/src/bin/rmt.rs b/hil-test/src/bin/rmt.rs index 6ffd3fb6128..c19130454b1 100644 --- a/hil-test/src/bin/rmt.rs +++ b/hil-test/src/bin/rmt.rs @@ -83,6 +83,8 @@ struct LoopbackConfig { length1_base: u16, length1_step: u16, length2: u16, + sclk_div: u16, // 1 - 256 + ch_div: u8, } impl Default for LoopbackConfig { @@ -100,14 +102,20 @@ impl Default for LoopbackConfig { length1_base: 100, length1_step: 10, length2: 50, + sclk_div: 1, + ch_div: DIV, } } } impl LoopbackConfig { + fn sclk_freq(&self) -> Rate { + FREQ / self.sclk_div as u32 + } + fn tx_config(&self) -> TxChannelConfig { TxChannelConfig::default() - .with_clk_divider(DIV) + .with_clk_divider(self.ch_div) .with_idle_output(self.idle_output) .with_idle_output_level(Level::Low) .with_memsize(self.tx_memsize) @@ -115,7 +123,7 @@ impl LoopbackConfig { fn rx_config(&self) -> RxChannelConfig { RxChannelConfig::default() - .with_clk_divider(DIV) + .with_clk_divider(self.ch_div) .with_idle_threshold(self.idle_threshold) .with_memsize(self.rx_memsize) } @@ -284,10 +292,13 @@ fn check_data_eq( struct BitbangTransmitter { pin: Flex<'static>, + cycles_per_us: u32, } impl BitbangTransmitter { - fn new(pin: impl Pin + 'static) -> Self { + const SCALE: u32 = 1024; + + fn new(pin: impl Pin + 'static, conf: &LoopbackConfig) -> Self { let mut pin = Flex::new(pin); pin.set_input_enable(true); pin.set_output_enable(true); @@ -296,7 +307,10 @@ impl BitbangTransmitter { pin.set_low(); - Self { pin } + Self { + pin, + cycles_per_us: conf.ch_div as u32 * 1000 * Self::SCALE / conf.sclk_freq().as_khz(), + } } fn rx_pin(&mut self) -> impl PeripheralInput<'static> { @@ -304,8 +318,6 @@ impl BitbangTransmitter { } fn transmit(&mut self, tx_data: &[PulseCode]) { - let cycles_per_us = DIV as u32 / FREQ.as_mhz(); - let delay = Delay::new(); for code in tx_data { @@ -313,13 +325,13 @@ impl BitbangTransmitter { break; } self.pin.set_level(code.level1()); - delay.delay_micros(code.length1() as u32 * cycles_per_us); + delay.delay_micros(code.length1() as u32 * self.cycles_per_us / Self::SCALE); if code.length2() == 0 { break; } self.pin.set_level(code.level2()); - delay.delay_micros(code.length2() as u32 * cycles_per_us); + delay.delay_micros(code.length2() as u32 * self.cycles_per_us / Self::SCALE); } } } @@ -482,7 +494,7 @@ impl Context { &mut self, conf: &LoopbackConfig, ) -> (Channel<'_, Blocking, Tx>, Channel<'_, Blocking, Rx>) { - let rmt = Rmt::new(self.rmt.reborrow(), FREQ).unwrap(); + let rmt = Rmt::new(self.rmt.reborrow(), conf.sclk_freq()).unwrap(); let (rx, tx) = pins!(self); Self::setup_impl(rmt, rx, tx, conf) } @@ -491,7 +503,9 @@ impl Context { &mut self, conf: &LoopbackConfig, ) -> (Channel<'_, Async, Tx>, Channel<'_, Async, Rx>) { - let rmt = Rmt::new(self.rmt.reborrow(), FREQ).unwrap().into_async(); + let rmt = Rmt::new(self.rmt.reborrow(), conf.sclk_freq()) + .unwrap() + .into_async(); let (rx, tx) = pins!(self); Self::setup_impl(rmt, rx, tx, conf) } @@ -696,7 +710,7 @@ mod tests { let conf = LoopbackConfig::default(); let (rx_pin, tx_pin) = pins!($ctx); - let mut rmt = Rmt::new($ctx.rmt.reborrow(), FREQ).unwrap(); + let mut rmt = Rmt::new($ctx.rmt.reborrow(), conf.sclk_freq()).unwrap(); let tx_channel = rmt .$tx_channel @@ -788,9 +802,9 @@ mod tests { ..Default::default() }; - let rmt = Rmt::new(ctx.rmt, FREQ).unwrap(); + let rmt = Rmt::new(ctx.rmt, conf.sclk_freq()).unwrap(); - let mut bitbang_tx = BitbangTransmitter::new(ctx.pin); + let mut bitbang_tx = BitbangTransmitter::new(ctx.pin, &conf); let (_, rx_channel) = Context::setup_impl(rmt, bitbang_tx.rx_pin(), NoPin, &conf); let tx_data = generate_tx_data(&conf); @@ -859,7 +873,7 @@ mod tests { // Test that dropping & recreating Rmt works for _ in 0..3 { - let mut rmt = Rmt::new(ctx.rmt.reborrow(), FREQ).unwrap(); + let mut rmt = Rmt::new(ctx.rmt.reborrow(), conf.sclk_freq()).unwrap(); // Test that dropping & recreating ChannelCreator works for _ in 0..3 { @@ -923,7 +937,9 @@ mod tests { // Test that dropping & recreating Rmt works for _ in 0..3 { - let mut rmt = Rmt::new(ctx.rmt.reborrow(), FREQ).unwrap().into_async(); + let mut rmt = Rmt::new(ctx.rmt.reborrow(), conf.sclk_freq()) + .unwrap() + .into_async(); // Test that dropping & recreating ChannelCreator works for _ in 0..3 { @@ -1082,4 +1098,37 @@ mod tests { "tx with loopcount 0 did not complete immediately" ); } + + #[test] + fn rmt_dividers(mut ctx: Context) { + for sclk_div in [1, 16, 256] { + for ch_div in [1, 16, 255] { + let div = sclk_div as u32 * ch_div as u32; + // the stop code is sent with length 2 * idle_threshold = 40 * step, cf. + // generate_tx_data + let step = (32767 / (41 * div)).max(5) as u16; + let conf = LoopbackConfig { + idle_output: true, + // write_stop_code: false, + sclk_div, + ch_div, + tx_len: 5, + length1_base: 10 * step, + length1_step: step, + length2: 2 * step, + idle_threshold: 20 * step, + ..Default::default() + }; + #[cfg(feature = "defmt")] + { + defmt::info!("sclk_div: {}, ch_div: {}, step: {}", sclk_div, ch_div, step); + defmt::flush(); + } + + let (tx_channel, rx_channel) = ctx.setup_loopback(&conf); + + do_rmt_loopback_inner(&conf, tx_channel, rx_channel); + } + } + } } From ae039140d8843208f365b414cb683cab88e205d9 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Wed, 12 Nov 2025 14:50:43 +0100 Subject: [PATCH 2/3] RMT: Workaround/fix driver for slow sclk Setting a large RMT SCLK divider breaks the driver in unexpected ways. There's nothing in the documentation hinting at relevant timing constraints (the only thing mentioned are limits on pulse code periods, but that's not the issue here), and the IDF code also doesn't seem to do anything special. It does order some register operations differently, first sets register and only then fills the hardware buffer, and also does more stuff (book keeping, locking, ...). This PR does the following: - match update() calls better to what IDF does (only before tx start, after tx stop, after rx start) - add some delays between configuring channels and actually starting transactions: It appears that adding a delay of about 1 SCLK period fixes all the weirdness. This suggests that some settings take a while to be properly propagated in the peripheral, and failing to provide that time results in weird glitches and hangs. It might be that IDF accidentally sidesteps this due to ordering things as follows (and due to overall doing more including locking, runtime checks, ...): 1. write RMT config registers 2. write data to RMT RAM 3. start tx whereas we switch steps 1 + 2. Thus, switching order might be a way to decrease the required delay, but that would require more research into whether that is really sufficient (e.g. when the data is short and writing it to RAM completes quickly). If there's no overall better way to fix this, maybe one could use cycle counters to just wait the minimal necessary time? In any case, this commit seems to be a minimally-invasive workaround/fix. --- esp-hal/src/rmt.rs | 22 ++++++++++++++-------- esp-hal/src/rmt/reader.rs | 2 ++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs index 8863d45d60f..d1667003f33 100644 --- a/esp-hal/src/rmt.rs +++ b/esp-hal/src/rmt.rs @@ -2146,9 +2146,7 @@ impl DynChannelAccess { #[cfg(rmt_has_tx_loop_count)] self.set_loopmode(loopmode); self.set_tx_wrap_mode(loopmode.is_none()); - self.update(); self.start_tx(); - self.update(); } #[inline(always)] @@ -2174,11 +2172,9 @@ impl DynChannelAccess { { self.set_rx_threshold((_memsize.codes() / 2) as u16); self.set_rx_wrap_mode(_wrap); - self.update(); } self.start_rx(); - self.update(); } #[inline(always)] @@ -2541,9 +2537,15 @@ mod chip_specific { rmt.ch_tx_conf0(ch_idx).modify(|_, w| { w.mem_rd_rst().set_bit(); - w.apb_mem_rst().set_bit(); - w.tx_start().set_bit() + w.apb_mem_rst().set_bit() }); + self.update(); + // Wait for configuration/data to be effective - there seem to be undocumented timing + // constraints when sclk_div is large. 4us corresponds to ~1 SCLK cycle when using the + // APB SCLK. + crate::rom::ets_delay_us(4); + rmt.ch_tx_conf0(self.channel().into()) + .modify(|_, w| w.tx_start().set_bit()); } // Return the first flag that is set of, in order of decreasing priority, @@ -2686,9 +2688,13 @@ mod chip_specific { rmt.ch_rx_conf1(ch_idx.into()).modify(|_, w| { w.mem_owner().set_bit(); w.mem_wr_rst().set_bit(); - w.apb_mem_rst().set_bit(); - w.rx_en().set_bit() + w.apb_mem_rst().set_bit() }); + self.update(); + crate::rom::ets_delay_us(4); + rmt.ch_rx_conf1(ch_idx.into()) + .modify(|_, w| w.rx_en().set_bit()); + self.update(); } // Return the first flag that is set of, in order of decreasing priority, diff --git a/esp-hal/src/rmt/reader.rs b/esp-hal/src/rmt/reader.rs index 674be47cdca..551c867b2b9 100644 --- a/esp-hal/src/rmt/reader.rs +++ b/esp-hal/src/rmt/reader.rs @@ -54,6 +54,8 @@ impl RmtReader { let memsize = raw.memsize().codes(); let offset = self.offset as usize; + // cf. comments on delay in ../rmt.rs + crate::rom::ets_delay_us(4); let max_count = if final_ { let hw_offset = raw.hw_offset(); From ff16db6583b3811ec3c683f7d0c9b36184a00d5c Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Tue, 9 Dec 2025 12:09:14 +0100 Subject: [PATCH 3/3] RMT: more helpful error for invalid input to generate_tx_data Give a more precise error when specifying timings that would make some of the pulses longer than the idle_threshold --- hil-test/src/bin/rmt.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/hil-test/src/bin/rmt.rs b/hil-test/src/bin/rmt.rs index c19130454b1..e2d9c7c9e58 100644 --- a/hil-test/src/bin/rmt.rs +++ b/hil-test/src/bin/rmt.rs @@ -136,17 +136,21 @@ impl LoopbackConfig { // buffer in loopback tests with buffer wrapping. If we did, that might hide bugs. // FIXME: Make a method on LoopbackConfig? fn generate_tx_data(conf: &LoopbackConfig) -> Vec { + const WRAP_COUNT: u16 = 23; + let mut tx_data: Vec<_> = (0..) .take(conf.tx_len) .map(|i| { - PulseCode::new( + PulseCode::try_new( Level::High, - conf.length1_base + conf.length1_step * (i % 23), + conf.length1_base + conf.length1_step * (i % WRAP_COUNT), Level::Low, conf.length2, ) + .expect("pulse code length out of range") }) .collect(); + let mut i_max = WRAP_COUNT.min(conf.tx_len as u16); let mut pos = conf.tx_len - 1; match conf.end_marker { @@ -154,19 +158,25 @@ fn generate_tx_data(conf: &LoopbackConfig) -> Vec { EndMarkerConfig::Field1 => { tx_data[pos] = PulseCode::end_marker(); pos -= 1; + i_max -= 1; } EndMarkerConfig::Field2 => { tx_data[pos] = tx_data[pos].with_length2(0).unwrap(); pos -= 1; + i_max -= 1; } } if conf.write_stop_code { - tx_data[pos] = PulseCode::new( + i_max -= 1; + assert!(conf.length1_base + i_max * conf.length1_step + conf.length2 < conf.idle_threshold); + + tx_data[pos] = PulseCode::try_new( Level::High, 2 * conf.idle_threshold, Level::Low, conf.length2, - ); + ) + .expect("idle_threshold out of range"); } tx_data