Merge pull request #4884 from xoviat/adc

Fix async adc reads on h5 and others

4 files changed, 80 insertions, 114 deletions

diff --git a/embassy-stm32/src/adc/c0.rs b/embassy-stm32/src/adc/c0.rs
index 3bdca7edb..d87bd1ed4 100644
--- a/embassy-stm32/src/adc/c0.rs
+++ b/embassy-stm32/src/adc/c0.rs
@@ -1,7 +1,7 @@
 #[allow(unused)]
 use pac::adc::vals::{Adstp, Align, Ckmode, Dmacfg, Exten, Ovrmod, Ovsr};
 use pac::adccommon::vals::Presc;
-use stm32_metapac::adc::vals::Scandir;
+use stm32_metapac::adc::vals::{SampleTime, Scandir};
 
 use super::{Adc, Instance, Resolution, blocking_delay_us};
 use crate::adc::{AnyInstance, ConversionMode};
@@ -17,7 +17,6 @@ const MAX_ADC_CLK_FREQ: Hertz = Hertz::mhz(25);
 
 const TIME_ADC_VOLTAGE_REGUALTOR_STARTUP_US: u32 = 20;
 
-const NUM_HW_CHANNELS: u8 = 22;
 const CHSELR_SQ_SIZE: usize = 8;
 const CHSELR_SQ_MAX_CHANNEL: u8 = 14;
 const CHSELR_SQ_SEQUENCE_END_MARKER: u8 = 0b1111;
@@ -100,82 +99,67 @@ impl<T: Instance> super::SealedAnyInstance for T {
         }
     }
 
-    fn configure_sequence(mut sequence: impl ExactSizeIterator<Item = ((u8, bool), Self::SampleTime)>, blocking: bool) {
-        T::regs().cfgr1().modify(|reg| {
-            reg.set_chselrmod(!blocking);
-            reg.set_align(Align::RIGHT);
-        });
+    fn configure_sequence(sequence: impl ExactSizeIterator<Item = ((u8, bool), Self::SampleTime)>) {
+        let mut needs_hw = sequence.len() == 1 || sequence.len() > CHSELR_SQ_SIZE;
+        let mut is_ordered_up = true;
+        let mut is_ordered_down = true;
 
-        assert!(!blocking || sequence.len() == 1, "Sequence len must be 1 for blocking.");
-        if blocking {
-            let ((ch, _), sample_time) = sequence.next().unwrap();
-            // Set all channels to use SMP1 field as source.
-            T::regs().smpr().modify(|w| {
-                w.smpsel(0);
-                w.set_smp1(sample_time);
-            });
+        let sequence_len = sequence.len();
+        let mut hw_channel_selection: u32 = 0;
+        let mut last_channel: u8 = 0;
+        let mut sample_time: Self::SampleTime = SampleTime::CYCLES2_5;
+
+        T::regs().chselr_sq().write(|w| {
+            for (i, ((channel, _), _sample_time)) in sequence.enumerate() {
+                assert!(
+                    sample_time == _sample_time || i == 0,
+                    "C0 only supports one sample time for the sequence."
+                );
 
-            // write() because we want all other bits to be set to 0.
-            T::regs().chselr().write(|w| w.set_chsel(ch.into(), true));
-        } else {
-            let mut hw_channel_selection: u32 = 0;
-            let mut is_ordered_up = true;
-            let mut is_ordered_down = true;
-            let mut needs_hw = false;
+                sample_time = _sample_time;
+                needs_hw = needs_hw || channel > CHSELR_SQ_MAX_CHANNEL;
+                is_ordered_up = is_ordered_up && (channel > last_channel || i == 0);
+                is_ordered_down = is_ordered_down && (channel < last_channel || i == 0);
+                hw_channel_selection += 1 << channel;
+                last_channel = channel;
 
+                if !needs_hw {
+                    w.set_sq(i, channel);
+                }
+            }
+
+            for i in sequence_len..CHSELR_SQ_SIZE {
+                w.set_sq(i, CHSELR_SQ_SEQUENCE_END_MARKER);
+            }
+        });
+
+        if needs_hw {
             assert!(
-                sequence.len() <= CHSELR_SQ_SIZE,
-                "Sequence read set cannot be more than {} in size.",
+                sequence_len <= CHSELR_SQ_SIZE || is_ordered_up || is_ordered_down,
+                "Sequencer is required because of unordered channels, but read set cannot be more than {} in size.",
                 CHSELR_SQ_SIZE
             );
-            let mut last_sq_set: usize = 0;
-            let mut last_channel: u8 = 0;
-            T::regs().chselr_sq().write(|w| {
-                for (i, ((channel, _), _sample_time)) in sequence.enumerate() {
-                    needs_hw = needs_hw || channel > CHSELR_SQ_MAX_CHANNEL;
-                    last_sq_set = i;
-                    is_ordered_up = is_ordered_up && channel > last_channel;
-                    is_ordered_down = is_ordered_down && channel < last_channel;
-                    hw_channel_selection += 1 << channel;
-                    last_channel = channel;
-
-                    if !needs_hw {
-                        w.set_sq(i, channel);
-                    }
-                }
-
-                assert!(
-                    !needs_hw || is_ordered_up || is_ordered_down,
-                    "Sequencer is required because of unordered channels, but only support HW channels smaller than {}.",
-                    CHSELR_SQ_MAX_CHANNEL
-                );
+            assert!(
+                sequence_len > CHSELR_SQ_SIZE || is_ordered_up || is_ordered_down,
+                "Sequencer is required because of unordered channels, but only support HW channels smaller than {}.",
+                CHSELR_SQ_MAX_CHANNEL
+            );
 
-                if needs_hw {
-                    assert!(
-                        hw_channel_selection != 0,
-                        "Some bits in `hw_channel_selection` shall be set."
-                    );
-                    assert!(
-                        (hw_channel_selection >> NUM_HW_CHANNELS) == 0,
-                        "STM32C0 only have {} ADC channels. `hw_channel_selection` cannot have bits higher than this number set.",
-                        NUM_HW_CHANNELS
-                    );
-
-                    T::regs().cfgr1().modify(|reg| {
-                        reg.set_chselrmod(false);
-                        reg.set_scandir(if is_ordered_up { Scandir::UP} else { Scandir::BACK });
-                    });
-
-                    // Set required channels for multi-convert.
-                    unsafe { (T::regs().chselr().as_ptr() as *mut u32).write_volatile(hw_channel_selection) }
-                } else {
-                    for i in (last_sq_set + 1)..CHSELR_SQ_SIZE {
-                        w.set_sq(i, CHSELR_SQ_SEQUENCE_END_MARKER);
-                    }
-                }
-            });
+            // Set required channels for multi-convert.
+            unsafe { (T::regs().chselr().as_ptr() as *mut u32).write_volatile(hw_channel_selection) }
         }
 
+        T::regs().smpr().modify(|w| {
+            w.smpsel(0);
+            w.set_smp1(sample_time);
+        });
+
+        T::regs().cfgr1().modify(|reg| {
+            reg.set_chselrmod(!needs_hw);
+            reg.set_align(Align::RIGHT);
+            reg.set_scandir(if is_ordered_up { Scandir::UP } else { Scandir::BACK });
+        });
+
         // Trigger and wait for the channel selection procedure to complete.
         T::regs().isr().modify(|w| w.set_ccrdy(false));
         while !T::regs().isr().read().ccrdy() {}
diff --git a/embassy-stm32/src/adc/mod.rs b/embassy-stm32/src/adc/mod.rs
index 5ec08a22d..13f8a1544 100644
--- a/embassy-stm32/src/adc/mod.rs
+++ b/embassy-stm32/src/adc/mod.rs
@@ -111,10 +111,7 @@ pub(self) trait SealedAnyInstance: BasicAnyInstance {
     fn stop();
     fn convert() -> u16;
     fn configure_dma(conversion_mode: ConversionMode);
-    #[cfg(not(adc_c0))]
     fn configure_sequence(sequence: impl ExactSizeIterator<Item = ((u8, bool), Self::SampleTime)>);
-    #[cfg(adc_c0)]
-    fn configure_sequence(sequence: impl ExactSizeIterator<Item = ((u8, bool), Self::SampleTime)>, blocking: bool);
     #[allow(dead_code)]
     fn dr() -> *mut u16;
 }
@@ -197,13 +194,7 @@ impl<'d, T: AnyInstance> Adc<'d, T> {
 
         #[cfg(not(adc_v4))]
         T::enable();
-        #[cfg(not(adc_c0))]
         T::configure_sequence([((channel.channel(), channel.is_differential()), sample_time)].into_iter());
-        #[cfg(adc_c0)]
-        T::configure_sequence(
-            [((channel.channel(), channel.is_differential()), sample_time)].into_iter(),
-            true,
-        );
 
         T::convert()
     }
@@ -262,15 +253,8 @@ impl<'d, T: AnyInstance> Adc<'d, T> {
         T::stop();
         T::enable();
 
-        #[cfg(not(adc_c0))]
-        T::configure_sequence(
-            sequence.map(|(channel, sample_time)| ((channel.channel, channel.is_differential), sample_time)),
-        );
-
-        #[cfg(adc_c0)]
         T::configure_sequence(
             sequence.map(|(channel, sample_time)| ((channel.channel, channel.is_differential), sample_time)),
-            false,
         );
 
         T::configure_dma(ConversionMode::Singular);
diff --git a/embassy-stm32/src/adc/v3.rs b/embassy-stm32/src/adc/v3.rs
index 78b497727..288bd77ce 100644
--- a/embassy-stm32/src/adc/v3.rs
+++ b/embassy-stm32/src/adc/v3.rs
@@ -174,38 +174,31 @@ impl<T: Instance> super::SealedAnyInstance for T {
     }
 
     fn start() {
-        #[cfg(any(adc_v3, adc_g0, adc_u0))]
-        {
-            // Start adc conversion
-            T::regs().cr().modify(|reg| {
-                reg.set_adstart(true);
-            });
-        }
+        T::regs().cr().modify(|reg| {
+            reg.set_adstart(true);
+        });
     }
 
     fn stop() {
-        #[cfg(any(adc_v3, adc_g0, adc_u0))]
-        {
-            // Ensure conversions are finished.
-            if T::regs().cr().read().adstart() && !T::regs().cr().read().addis() {
-                T::regs().cr().modify(|reg| {
-                    reg.set_adstp(true);
-                });
-                while T::regs().cr().read().adstart() {}
-            }
-
-            // Reset configuration.
-            #[cfg(not(any(adc_g0, adc_u0)))]
-            T::regs().cfgr().modify(|reg| {
-                reg.set_cont(false);
-                reg.set_dmaen(false);
-            });
-            #[cfg(any(adc_g0, adc_u0))]
-            T::regs().cfgr1().modify(|reg| {
-                reg.set_cont(false);
-                reg.set_dmaen(false);
+        // Ensure conversions are finished.
+        if T::regs().cr().read().adstart() && !T::regs().cr().read().addis() {
+            T::regs().cr().modify(|reg| {
+                reg.set_adstp(true);
             });
+            while T::regs().cr().read().adstart() {}
         }
+
+        // Reset configuration.
+        #[cfg(not(any(adc_g0, adc_u0)))]
+        T::regs().cfgr().modify(|reg| {
+            reg.set_cont(false);
+            reg.set_dmaen(false);
+        });
+        #[cfg(any(adc_g0, adc_u0))]
+        T::regs().cfgr1().modify(|reg| {
+            reg.set_cont(false);
+            reg.set_dmaen(false);
+        });
     }
 
     /// Perform a single conversion.
diff --git a/examples/stm32h5/src/bin/adc_dma.rs b/examples/stm32h5/src/bin/adc_dma.rs
index fb9fcbc5c..2138257f7 100644
--- a/examples/stm32h5/src/bin/adc_dma.rs
+++ b/examples/stm32h5/src/bin/adc_dma.rs
@@ -6,7 +6,7 @@ use embassy_executor::Spawner;
 use embassy_stm32::adc::{self, Adc, AdcChannel, RxDma, SampleTime};
 use embassy_stm32::peripherals::{ADC1, ADC2, GPDMA1_CH0, GPDMA1_CH1, PA0, PA1, PA2, PA3};
 use embassy_stm32::{Config, Peri};
-use embassy_time::Instant;
+use embassy_time::{Duration, Instant, Ticker};
 use {defmt_rtt as _, panic_probe as _};
 
 #[embassy_executor::main]
@@ -76,6 +76,9 @@ async fn adc_task<'a, T: adc::Instance>(
     let mut pin1 = pin1.degrade_adc();
     let mut pin2 = pin2.degrade_adc();
 
+    info!("adc init");
+
+    let mut ticker = Ticker::every(Duration::from_millis(500));
     let mut tic = Instant::now();
     let mut buffer = [0u16; 512];
     loop {
@@ -84,11 +87,13 @@ async fn adc_task<'a, T: adc::Instance>(
         adc.read(
             dma.reborrow(),
             [(&mut pin1, SampleTime::CYCLES2_5), (&mut pin2, SampleTime::CYCLES2_5)].into_iter(),
-            &mut buffer,
+            &mut buffer[0..2],
         )
         .await;
         let toc = Instant::now();
         info!("\n adc1: {} dt = {}", buffer[0..16], (toc - tic).as_micros());
         tic = toc;
+
+        ticker.next().await;
     }
 }