Merge #245 #252

245: Implement DMA TargetAddress for I2C[123] r=richardeoin a=richardeoin



252: Clamp scldel/sdadel to 4-bit and update divider calculations r=richardeoin a=richardeoin

Previously this code was calculating scldel/sdadel values > 15 which were correct, but could not be successfully used by the hardware.

Update divider calculations to generally pick a larger prescaler value, resulting in a slower scaled clock. This means that scldel/sdadel values up to 15 still result in the required delays.

Fixes #238

Co-authored-by: Richard Meadows <962920+richardeoin@users.noreply.github.com>

Author: bors[bot]

src/dma/bdma.rs

@@ -672,10 +672,8 @@ peripheral_target_address!(
         DMAReq::SPI6_RX_DMA,
         DMAReq::SPI6_TX_DMA
     ),
-    (pac::I2C4, rxdr, u8, P2M, DMAReq::I2C4_RX_DMA),
-    (pac::I2C4, txdr, u8, M2P, DMAReq::I2C4_TX_DMA),
-    (INNER: I2c<pac::I2C4>, rxdr, u8, P2M, DMAReq::I2C4_RX_DMA),
-    (INNER: I2c<pac::I2C4>, txdr, u8, M2P, DMAReq::I2C4_TX_DMA),
+    (HAL: I2c<pac::I2C4>, rxdr, u8, P2M, DMAReq::I2C4_RX_DMA),
+    (HAL: I2c<pac::I2C4>, txdr, u8, M2P, DMAReq::I2C4_TX_DMA),
 );
 
 #[cfg(not(feature = "rm0455"))]

src/dma/dma.rs

@@ -10,6 +10,7 @@ use super::{
 use core::marker::PhantomData;
 
 use crate::{
+    i2c::I2c,
     pac::{self, DMA1, DMA2, DMAMUX1},
     rcc::{rec, rec::ResetEnable},
     serial, spi,
@@ -923,6 +924,15 @@ peripheral_target_address!(
     ),
 );
 
+peripheral_target_address!(
+    (HAL: I2c<pac::I2C1>, rxdr, u8, P2M, DMAReq::I2C1_RX_DMA),
+    (HAL: I2c<pac::I2C1>, txdr, u8, M2P, DMAReq::I2C1_TX_DMA),
+    (HAL: I2c<pac::I2C2>, rxdr, u8, P2M, DMAReq::I2C2_RX_DMA),
+    (HAL: I2c<pac::I2C2>, txdr, u8, M2P, DMAReq::I2C2_TX_DMA),
+    (HAL: I2c<pac::I2C3>, rxdr, u8, P2M, DMAReq::I2C3_RX_DMA),
+    (HAL: I2c<pac::I2C3>, txdr, u8, M2P, DMAReq::I2C3_TX_DMA),
+);
+
 peripheral_target_address!(
     (pac::SAI1, cha.dr, u32, M2P, DMAReq::SAI1A_DMA),
     (pac::SAI1, chb.dr, u32, P2M, DMAReq::SAI1B_DMA),

src/dma/macros.rs

@@ -11,6 +11,7 @@ macro_rules! peripheral_target_address {
     };
 }
 macro_rules! peripheral_target_instance {
+    // PAC target
     (($peripheral:ty, $register:ident, $size:ty,
       $dir:ty $(, $mux:expr)*)) => {
         unsafe impl TargetAddress<$dir> for $peripheral {
@@ -26,8 +27,59 @@ macro_rules! peripheral_target_instance {
         }
     };
 
-    ((SPI: $peripheral:ty, $rxreg:ident, $txreg:ident, [$($size:ty),+], $rxmux:expr, $txmux:expr)) => {
-        // Access via PAC peripheral structures implies u8 sizing, as the sizing is unknown.
+    // PAC target where register is within a channel
+    (($peripheral:ty, $channel:ident.$register:ident, $size:ty,
+      $dir:ty $(, $mux:expr)*)) => {
+        unsafe impl TargetAddress<$dir> for $peripheral {
+            #[inline(always)]
+            fn address(&self) -> usize {
+                &self.$channel.$register as *const _ as usize
+            }
+
+            type MemSize = $size;
+            $(
+                const REQUEST_LINE: Option<u8> = Some($mux as u8);
+            )*
+        }
+    };
+
+    // PAC and HAL targets, generic
+    ((HAL: $hal:ident<$peripheral:ty>, $register:ident, $size:ty,
+      $dir:ty $(, $mux:expr)*)) => {
+
+        // PAC implementation
+        unsafe impl TargetAddress<$dir> for $peripheral {
+            #[inline(always)]
+            fn address(&self) -> usize {
+                &self.$register as *const _ as usize
+            }
+
+            type MemSize = $size;
+            $(
+                const REQUEST_LINE: Option<u8> = Some($mux as u8);
+            )*
+        }
+
+        // HAL implementation
+        unsafe impl TargetAddress<$dir> for $hal<$peripheral> {
+            #[inline(always)]
+            fn address(&self) -> usize {
+                &self.inner().$register as *const _ as usize
+            }
+
+            type MemSize = $size;
+            $(
+                const REQUEST_LINE: Option<u8> = Some($mux as u8);
+            )*
+        }
+    };
+
+    // PAC and HAL targets for SPI peripheral
+    ((SPI: $peripheral:ty, $rxreg:ident, $txreg:ident, [$($size:ty),+],
+      $rxmux:expr, $txmux:expr)) => {
+
+        // Access via PAC peripheral structures implies u8 sizing, as the sizing
+        // is unknown.
         unsafe impl TargetAddress<M2P> for $peripheral {
             #[inline(always)]
             fn address(&self) -> usize {
@@ -76,6 +128,7 @@ macro_rules! peripheral_target_instance {
         )+
     };
 
+    // PAC and HAL targets for Serial Peripherals
     ((SERIAL: $peripheral:ty, $rxreg:ident, $txreg:ident, $rxmux:expr, $txmux:expr)) => {
         unsafe impl TargetAddress<M2P> for $peripheral {
             #[inline(always)]
@@ -151,37 +204,4 @@ macro_rules! peripheral_target_instance {
             const TRBUFF: bool = true;
         }
     };
-
-    ((INNER: $peripheral:ty, $register:ident $(($TRBUFF:ident))*, $size:ty,
-      $dir:ty $(, $mux:expr)*)) => {
-        unsafe impl TargetAddress<$dir> for $peripheral {
-            #[inline(always)]
-            fn address(&self) -> usize {
-                &self.inner().$register as *const _ as usize
-            }
-
-            type MemSize = $size;
-            $(
-                const REQUEST_LINE: Option<u8> = Some($mux as u8);
-            )*
-                $(
-                    const $TRBUFF: bool = true;
-                )*
-        }
-    };
-
-    (($peripheral:ty, $channel:ident.$register:ident, $size:ty,
-      $dir:ty $(, $mux:expr)*)) => {
-        unsafe impl TargetAddress<$dir> for $peripheral {
-            #[inline(always)]
-            fn address(&self) -> usize {
-                &self.$channel.$register as *const _ as usize
-            }
-
-            type MemSize = $size;
-            $(
-                const REQUEST_LINE: Option<u8> = Some($mux as u8);
-            )*
-        }
-    };
 }

src/i2c.rs

@@ -184,8 +184,8 @@ macro_rules! i2c_timing {
             // Fast-mode (Fm) or Fast-mode Plus (Fm+)
             // here we pick SCLL + 1 = 2 * (SCLH + 1)
 
-            // Prescaler, 384 ticks for sclh/scll. Round up then subtract 1
-            let presc_reg = ((ratio - 1) / 384) as u8;
+            // Prescaler, 96 ticks for sclh/scll. Round up then subtract 1
+            let presc_reg = ((ratio - 1) / 96) as u8;
             // ratio < 1200 by pclk 120MHz max., therefore presc < 16
 
             // Actual precale value selected
@@ -206,8 +206,8 @@ macro_rules! i2c_timing {
                 // Fast-mode (Fm)
                 assert!($i2cclk >= 8_000_000); // See table in datsheet
 
-                let sdadel = $i2cclk / 4_000_000 / presc;
-                let scldel = $i2cclk / 2_000_000 / presc - 1;
+                let sdadel = $i2cclk / 3_000_000 / presc;
+                let scldel = $i2cclk / 1_000_000 / presc - 1;
 
                 (sdadel, scldel)
             };
@@ -224,10 +224,17 @@ macro_rules! i2c_timing {
             // here we pick SCLL = SCLH
             assert!($i2cclk >= 2_000_000); // See table in datsheet
 
-            // Prescaler, 512 ticks for sclh/scll. Round up then
+            // Prescaler, 128 or 256 ticks for sclh/scll. Round up then
             // subtract 1
-            let presc = (ratio - 1) / 512;
-            let presc_reg = cmp::min(presc, 15) as u8;
+            let presc_reg = (ratio - 1)
+                / if $freq < 8000 {
+                    256
+                } else if $freq < 80_000 {
+                    128
+                } else {
+                    64
+                };
+            let presc_reg = cmp::min(presc_reg, 15) as u8;
 
             // Actual prescale value selected
             let presc = (presc_reg + 1) as u32;
@@ -257,9 +264,12 @@ macro_rules! i2c_timing {
         assert!(presc_reg < 16);
 
         // Keep values within reasonable limits for fast per_ck
-        let sdadel = cmp::max(sdadel, 2);
+        let sdadel = cmp::max(sdadel, 1);
         let scldel = cmp::max(scldel, 4);
 
+        let sdadel = cmp::min(sdadel, 15);
+        let scldel = cmp::min(scldel, 15);
+
         (presc_reg, scll, sclh, sdadel, scldel)
     }};
 }
@@ -811,8 +821,8 @@ mod tests {
             );
 
             // We must generate a bus frequency less than or equal to that
-            // specified. Tolerate a 0.5% error
-            assert!(f_scl <= 1.005 * freq);
+            // specified. Tolerate a 2% error
+            assert!(f_scl <= 1.02 * freq);
 
             // But it should not be too much less than specified
             assert!(f_scl > 0.8 * freq);
@@ -884,6 +894,7 @@ mod tests {
                 "T SDA DELAY {:.2e} MINIMUM {:.2e}",
                 t_sdadel, t_sdadel_minimim
             );
+            assert!(sdadel <= 15);
             assert!(t_sdadel >= t_sdadel_minimim);
         });
     }
@@ -917,6 +928,7 @@ mod tests {
                 "T SDA DELAY {:.2e} MAXIMUM {:.2e}",
                 t_sdadel, t_sdadel_maximum
             );
+            assert!(sdadel <= 15);
             assert!(t_sdadel <= t_sdadel_maximum);
         });
     }
@@ -957,6 +969,7 @@ mod tests {
                 "T SCL DELAY {:.2e} MINIMUM {:.2e}",
                 t_scldel, t_scldel_minimum
             );
+            assert!(scldel <= 15);
             assert!(t_scldel >= t_scldel_minimum);
         });
     }