Merge #420

420: Miscellaneous r=jordens a=jordens

* dds: name consistently
* dds: inline
* refactor the broker ip parsing to be panicing
* speed up qspi to 100 MHz (tested)
* tweak pounder qspi xfer assembly padding for speed
* fix some clippy lints
* try 32 byte max qspi xfer size instead of 16 but revert that until the memclr that's being pulled is can be avoided.

Co-authored-by: Robert Jördens <rj@quartiq.de>

ad9959/src/lib.rs

@@ -34,6 +34,7 @@ pub trait Interface {
 /// Indicates various communication modes of the DDS. The value of this enumeration is equivalent to
 /// the configuration bits of the DDS CSR register.
 #[derive(Copy, Clone, PartialEq)]
+#[repr(u8)]
 pub enum Mode {
     SingleBitTwoWire = 0b00,
     SingleBitThreeWire = 0b01,
@@ -320,7 +321,7 @@ impl<I: Interface> Ad9959<I> {
             .write(Register::CSR as u8, &[csr])
             .map_err(|_| Error::Interface)?;
 
-        self.write(register, &data)?;
+        self.write(register, data)?;
 
         Ok(())
     }
@@ -528,7 +529,7 @@ pub struct DdsConfig {
 impl DdsConfig {
     /// Create a serializer that can be used for generating a serialized DDS profile for writing to
     /// a QSPI stream.
-    pub fn builder(&self) -> ProfileSerializer {
+    pub fn serializer(&self) -> ProfileSerializer {
         ProfileSerializer::new(self.mode)
     }
 }
@@ -562,6 +563,7 @@ impl ProfileSerializer {
     /// * `acr` - If provided, indicates the amplitude control register for the channels. The ACR
     ///   should be stored in the 3 LSB of the word. Note that if amplitude scaling is to be used,
     ///   the "Amplitude multiplier enable" bit must be set.
+    #[inline]
     pub fn update_channels(
         &mut self,
         channels: &[Channel],
@@ -585,7 +587,7 @@ impl ProfileSerializer {
         }
 
         if let Some(acr) = acr {
-            self.add_write(Register::ACR, &acr.to_be_bytes()[1..=3]);
+            self.add_write(Register::ACR, &acr.to_be_bytes()[1..]);
         }
     }
 
@@ -597,20 +599,20 @@ impl ProfileSerializer {
         self.index += value.len() + 1;
     }
 
+    #[inline]
     fn pad(&mut self) {
         // Pad the buffer to 32-bit (4 byte) alignment by adding dummy writes to CSR and LSRR.
-        match self.index & 3 {
+        // In the case of 1 byte padding, this instead pads with 5 bytes as there is no
-            3 => {
+        // valid single-byte write that could be used.
-                // For a level of 3, we have to pad with 5 bytes to align things.
+        if self.index & 1 != 0 {
-                self.add_write(Register::CSR, &[(self.mode as u8) << 1]);
+            // Pad with 3 bytes
             self.add_write(Register::LSRR, &[0, 0]);
         }
-            2 => self.add_write(Register::CSR, &[(self.mode as u8) << 1]),
+        if self.index & 2 != 0 {
-            1 => self.add_write(Register::LSRR, &[0, 0]),
+            // Pad with 2 bytes
-            0 => {}
+            self.add_write(Register::CSR, &[(self.mode as u8) << 1]);
-
-            _ => unreachable!(),
         }
+        debug_assert_eq!(self.index & 3, 0);
     }
 
     /// Get the serialized profile as a slice of 32-bit words.
@@ -621,6 +623,7 @@ impl ProfileSerializer {
     ///
     /// # Returns
     /// A slice of `u32` words representing the serialized profile.
+    #[inline]
     pub fn finalize<'a>(&'a mut self) -> &'a [u32] {
         self.pad();
         unsafe {

src/bin/dual-iir.rs

@@ -47,7 +47,6 @@ use stabilizer::{
         DigitalInput0, DigitalInput1, AFE0, AFE1,
     },
     net::{
-        self,
         data_stream::{FrameGenerator, StreamFormat, StreamTarget},
         miniconf::Miniconf,
         serde::Deserialize,
@@ -200,7 +199,10 @@ const APP: () = {
             stabilizer.cycle_counter,
             env!("CARGO_BIN_NAME"),
             stabilizer.net.mac_address,
-            net::parse_or_default_broker(option_env!("BROKER")),
+            option_env!("BROKER")
+                .unwrap_or("10.34.16.10")
+                .parse()
+                .unwrap(),
         );
 
         let generator = network

src/bin/lockin.rs

@@ -50,7 +50,6 @@ use stabilizer::{
         DigitalInput0, DigitalInput1, AFE0, AFE1,
     },
     net::{
-        self,
         data_stream::{FrameGenerator, StreamFormat, StreamTarget},
         miniconf::Miniconf,
         serde::Deserialize,
@@ -241,7 +240,10 @@ const APP: () = {
             stabilizer.cycle_counter,
             env!("CARGO_BIN_NAME"),
             stabilizer.net.mac_address,
-            net::parse_or_default_broker(option_env!("BROKER")),
+            option_env!("BROKER")
+                .unwrap_or("10.34.16.10")
+                .parse()
+                .unwrap(),
         );
 
         let generator = network.configure_streaming(

src/hardware/design_parameters.rs

@@ -11,13 +11,13 @@ pub const ADC_DAC_SCK_MAX: MegaHertz = MegaHertz(50);
 pub const TIMER_FREQUENCY: MegaHertz = MegaHertz(100);
 
 /// The QSPI frequency for communicating with the pounder DDS.
-pub const POUNDER_QSPI_FREQUENCY: MegaHertz = MegaHertz(40);
+pub const POUNDER_QSPI_FREQUENCY: MegaHertz = MegaHertz(100);
 
 /// The delay after initiating a QSPI transfer before asserting the IO_Update for the pounder DDS.
-// Pounder Profile writes are always 16 bytes, with 2 cycles required per byte, coming out to a
+// Pounder Profile writes are up to 16 bytes, with 2 cycles required per byte, coming out to a
-// total of 32 QSPI clock cycles. The QSPI is configured for 40MHz, so this comes out to an offset
+// total of 32 QSPI clock cycles. The QSPI is configured for 100MHz, so this comes out to an offset
-// of 800nS. We use 900ns to be safe.
+// of 320 ns. We use 400 ns to be safe.
-pub const POUNDER_IO_UPDATE_DELAY: f32 = 900e-9;
+pub const POUNDER_IO_UPDATE_DELAY: f32 = 400e-9;
 
 /// The duration to assert IO_Update for the pounder DDS.
 // IO_Update should be latched for 4 SYNC_CLK cycles after the QSPI profile write. With pounder

src/hardware/pounder/dds_output.rs

@@ -76,15 +76,15 @@ impl DdsOutput {
     /// # Args
     /// * `qspi` - The QSPI interface to the run the stream on.
     /// * `io_update_trigger` - The HighResTimerE used to generate IO_Update pulses.
-    /// * `dds_config` - The frozen DDS configuration.
+    /// * `config` - The frozen DDS configuration.
     pub fn new(
         mut qspi: QspiInterface,
         io_update_trigger: HighResTimerE,
-        dds_config: DdsConfig,
+        config: DdsConfig,
     ) -> Self {
         qspi.start_stream().unwrap();
         Self {
-            config: dds_config,
+            config,
             _qspi: qspi,
             io_update_trigger,
         }
@@ -93,10 +93,10 @@ impl DdsOutput {
     /// Get a builder for serializing a Pounder DDS profile.
     #[allow(dead_code)]
     pub fn builder(&mut self) -> ProfileBuilder {
-        let builder = self.config.builder();
+        let serializer = self.config.serializer();
         ProfileBuilder {
-            dds_stream: self,
+            dds_output: self,
-            serializer: builder,
+            serializer,
         }
     }
 
@@ -135,7 +135,7 @@ impl DdsOutput {
 
 /// A temporary builder for serializing and writing profiles.
 pub struct ProfileBuilder<'a> {
-    dds_stream: &'a mut DdsOutput,
+    dds_output: &'a mut DdsOutput,
     serializer: ProfileSerializer,
 }
 
@@ -162,8 +162,9 @@ impl<'a> ProfileBuilder<'a> {
 
     /// Write the profile to the DDS asynchronously.
     #[allow(dead_code)]
+    #[inline]
     pub fn write_profile(mut self) {
         let profile = self.serializer.finalize();
-        self.dds_stream.write_profile(profile);
+        self.dds_output.write_profile(profile);
     }
 }

src/hardware/pounder/mod.rs

@@ -233,7 +233,7 @@ impl ad9959::Interface for QspiInterface {
                 };
 
                 self.qspi
-                    .write(encoded_address.into(), &encoded_payload)
+                    .write(encoded_address.into(), encoded_payload)
                     .map_err(|_| Error::Qspi)
             }
             ad9959::Mode::FourBitSerial => {

src/hardware/setup.rs

@@ -207,6 +207,8 @@ pub fn setup(
         .d2ccip1r
         .modify(|_, w| w.spi123sel().pll2_p().spi45sel().pll2_q());
 
+    device.RCC.d1ccipr.modify(|_, w| w.qspisel().rcc_hclk3());
+
     let rcc = device.RCC.constrain();
     let ccdr = rcc
         .use_hse(8.mhz())

src/hardware/timers.rs

@@ -297,21 +297,23 @@ macro_rules! timer_channels {
                     // Only atomic operations on completed on the timer registers.
                     let regs = unsafe { &*<$TY>::ptr() };
 
-                    let result = if regs.sr.read().[< cc $index if >]().bit_is_set() {
+                    if regs.sr.read().[< cc $index if >]().bit_is_set() {
                         // Read the capture value. Reading the captured value clears the flag in the
                         // status register automatically.
-                        Some(regs.[< ccr $index >].read().ccr().bits())
+                        let result = regs.[< ccr $index >].read().ccr().bits();
-                    } else {
-                        None
-                    };
 
-                    // Read SR again to check for a potential over-capture. If there is an
+                        // Read SR again to check for a potential over-capture. Return an error in
-                    // overcapture, return an error.
+                        // that case.
-                    if regs.sr.read().[< cc $index of >]().bit_is_set() {
+                        let sr = regs.sr.read();
-                        regs.sr.modify(|_, w| w.[< cc $index of >]().clear_bit());
+                        if sr.[< cc $index of >]().bit_is_set() {
-                        Err(result)
+                            // NOTE(unsafe) write-back is safe
+                            regs.sr.write(|w| unsafe { w.bits(sr.bits()) }.[< cc $index of >]().clear_bit());
+                            Err(Some(result))
                         } else {
-                        Ok(result)
+                            Ok(Some(result))
+                        }
+                    } else {
+                        Ok(None)
                     }
                 }
 

src/net/mod.rs

@@ -208,30 +208,3 @@ pub fn get_device_prefix(
 
     prefix
 }
-
-/// Determine the broker IP address
-///
-/// # Note
-/// If the broker IP is unspecified or unparseable, the default IP is returned.
-///
-/// # Args
-/// * `input` - The optionally-specified command-line broker IP address as a string.
-///
-/// # Returns
-/// The broker IP address.
-pub fn parse_or_default_broker(input: Option<&str>) -> IpAddr {
-    input.and_then(|data| {
-             data.parse::<minimq::embedded_nal::IpAddr>().map_or_else(
-                 |err| {
-                     log::error!(
-                         "{:?}: Failed to parse broker IP ({:?}) - Falling back to default",
-                         err,
-                         data
-                     );
-                     None
-                 },
-                 Some,
-             )
-         })
-         .unwrap_or_else(|| DEFAULT_MQTT_BROKER.into())
-}