Remove PeripheralMarker (#2468)

* Redo DMA compatibility check using DmaEligible * Remove PeripheralMarker
esp-rs · Nov 7, 2024 · ac819fb · ac819fb
1 parent 8782429
commit ac819fb
Show file tree

Hide file tree

Showing 19 changed files with 133 additions and 148 deletions.
diff --git a/esp-hal/src/dma/gdma.rs b/esp-hal/src/dma/gdma.rs
@@ -475,10 +475,7 @@ pub struct ChannelCreator<const N: u8> {}
 
 impl<CH: DmaChannel, M: Mode> Channel<'_, CH, M> {
     /// Asserts that the channel is compatible with the given peripheral.
-    pub fn runtime_ensure_compatible<P: PeripheralMarker + DmaEligible>(
-        &self,
-        _peripheral: &PeripheralRef<'_, P>,
-    ) {
+    pub fn runtime_ensure_compatible<P: DmaEligible>(&self, _peripheral: &PeripheralRef<'_, P>) {
         // No runtime checks; GDMA channels are compatible with any peripheral
     }
 }

diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs
@@ -949,12 +949,6 @@ macro_rules! impl_dma_eligible {
     };
 }
 
-/// Marker trait
-#[doc(hidden)]
-pub trait PeripheralMarker {
-    fn peripheral(&self) -> crate::system::Peripheral;
-}
-
 #[doc(hidden)]
 #[derive(Debug)]
 pub struct DescriptorChain {
@@ -2111,7 +2105,7 @@ pub trait RegisterAccess: crate::private::Sealed {
     fn set_ext_mem_block_size(&self, size: DmaExtMemBKSize);
 
     #[cfg(pdma)]
-    fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool;
+    fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool;
 
     /// Configure the channel.
     fn configure(&self, burst_mode: bool, priority: DmaPriority) {

diff --git a/esp-hal/src/dma/pdma.rs b/esp-hal/src/dma/pdma.rs
@@ -31,7 +31,7 @@ pub trait PdmaChannel: crate::private::Sealed {
     fn register_block(&self) -> &Self::RegisterBlock;
     fn tx_waker(&self) -> &'static AtomicWaker;
     fn rx_waker(&self) -> &'static AtomicWaker;
-    fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool;
+    fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool;
 }
 
 #[doc(hidden)]
@@ -92,7 +92,7 @@ impl<C: PdmaChannel<RegisterBlock = SpiRegisterBlock>> RegisterAccess for SpiDma
         }
     }
 
-    fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool {
+    fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool {
         self.0.is_compatible_with(peripheral)
     }
 }
@@ -236,7 +236,7 @@ impl<C: PdmaChannel<RegisterBlock = SpiRegisterBlock>> RegisterAccess for SpiDma
         }
     }
 
-    fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool {
+    fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool {
         self.0.is_compatible_with(peripheral)
     }
 }
@@ -390,8 +390,8 @@ macro_rules! ImplSpiChannel {
                     &WAKER
                 }
 
-                fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool {
-                    peripheral.peripheral() == crate::system::Peripheral::[<Spi $num>]
+                fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool {
+                    peripheral == DmaPeripheral::[<Spi $num>]
                 }
             }
 
@@ -497,7 +497,7 @@ impl<C: PdmaChannel<RegisterBlock = I2sRegisterBlock>> RegisterAccess for I2sDma
             .modify(|_, w| w.check_owner().bit(check_owner.unwrap_or(true)));
     }
 
-    fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool {
+    fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool {
         self.0.is_compatible_with(peripheral)
     }
 }
@@ -653,7 +653,7 @@ impl<C: PdmaChannel<RegisterBlock = I2sRegisterBlock>> RegisterAccess for I2sDma
             .modify(|_, w| w.check_owner().bit(check_owner.unwrap_or(true)));
     }
 
-    fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool {
+    fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool {
         self.0.is_compatible_with(peripheral)
     }
 }
@@ -802,8 +802,8 @@ macro_rules! ImplI2sChannel {
                     static WAKER: embassy_sync::waitqueue::AtomicWaker = embassy_sync::waitqueue::AtomicWaker::new();
                     &WAKER
                 }
-                fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool {
-                    peripheral.peripheral() == crate::system::Peripheral::[<I2s $num>]
+                fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool {
+                    peripheral == DmaPeripheral::[<I2s $num>]
                 }
             }
 
@@ -909,11 +909,13 @@ where
     C: DmaChannel,
 {
     /// Asserts that the channel is compatible with the given peripheral.
-    pub fn runtime_ensure_compatible(&self, peripheral: &PeripheralRef<'_, impl PeripheralMarker>) {
+    pub fn runtime_ensure_compatible(&self, peripheral: &PeripheralRef<'_, impl DmaEligible>) {
         assert!(
-            self.tx.tx_impl.is_compatible_with(&**peripheral),
+            self.tx
+                .tx_impl
+                .is_compatible_with(peripheral.dma_peripheral()),
             "This DMA channel is not compatible with {:?}",
-            peripheral.peripheral()
+            peripheral.dma_peripheral()
         );
     }
 }
@@ -963,7 +965,7 @@ impl PdmaChannel for AnySpiDmaChannelInner {
             fn register_block(&self) -> &SpiRegisterBlock;
             fn tx_waker(&self) -> &'static AtomicWaker;
             fn rx_waker(&self) -> &'static AtomicWaker;
-            fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool;
+            fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool;
         }
     }
 }
@@ -1017,7 +1019,7 @@ impl PdmaChannel for AnyI2sDmaChannelInner {
             fn register_block(&self) -> &I2sRegisterBlock;
             fn tx_waker(&self) -> &'static AtomicWaker;
             fn rx_waker(&self) -> &'static AtomicWaker;
-            fn is_compatible_with(&self, peripheral: &impl PeripheralMarker) -> bool;
+            fn is_compatible_with(&self, peripheral: DmaPeripheral) -> bool;
         }
     }
 }
diff --git a/esp-hal/src/i2c.rs b/esp-hal/src/i2c.rs
@@ -66,7 +66,6 @@ use procmacros::handler;
 
 use crate::{
     clock::Clocks,
-    dma::PeripheralMarker,
     gpio::{interconnect::PeripheralOutput, InputSignal, OutputSignal, Pull},
     interrupt::InterruptHandler,
     peripheral::{Peripheral, PeripheralRef},
@@ -1176,7 +1175,9 @@ pub(super) fn i2c1_handler() {
 
 /// I2C Peripheral Instance
 #[doc(hidden)]
-pub trait Instance: Peripheral<P = Self> + PeripheralMarker + Into<AnyI2c> + 'static {
+pub trait Instance: Peripheral<P = Self> + Into<AnyI2c> + 'static {
+    fn peripheral(&self) -> crate::system::Peripheral;
+
     /// Returns the interrupt associated with this I2C peripheral.
     fn interrupt(&self) -> crate::peripherals::Interrupt;
 
@@ -2204,14 +2205,12 @@ fn write_fifo(register_block: &RegisterBlock, data: u8) {
     }
 }
 
-impl PeripheralMarker for crate::peripherals::I2C0 {
+impl Instance for crate::peripherals::I2C0 {
     #[inline(always)]
     fn peripheral(&self) -> crate::system::Peripheral {
         crate::system::Peripheral::I2cExt0
     }
-}
 
-impl Instance for crate::peripherals::I2C0 {
     #[inline(always)]
     fn async_handler(&self) -> InterruptHandler {
         i2c0_handler
@@ -2254,15 +2253,12 @@ impl Instance for crate::peripherals::I2C0 {
 }
 
 #[cfg(i2c1)]
-impl PeripheralMarker for crate::peripherals::I2C1 {
+impl Instance for crate::peripherals::I2C1 {
     #[inline(always)]
     fn peripheral(&self) -> crate::system::Peripheral {
         crate::system::Peripheral::I2cExt1
     }
-}
 
-#[cfg(i2c1)]
-impl Instance for crate::peripherals::I2C1 {
     #[inline(always)]
     fn async_handler(&self) -> InterruptHandler {
         i2c1_handler
@@ -2322,6 +2318,7 @@ impl Instance for AnyI2c {
             AnyI2cInner::I2c1(i2c) => i2c,
         } {
             fn interrupt(&self) -> crate::peripherals::Interrupt;
+            fn peripheral(&self) -> crate::system::Peripheral;
             fn async_handler(&self) -> InterruptHandler;
             fn scl_output_signal(&self) -> OutputSignal;
             fn scl_input_signal(&self) -> InputSignal;

diff --git a/esp-hal/src/i2s/master.rs b/esp-hal/src/i2s/master.rs
@@ -773,14 +773,7 @@ mod private {
     #[cfg(i2s1)]
     use crate::peripherals::{i2s1::RegisterBlock, I2S1};
     use crate::{
-        dma::{
-            ChannelRx,
-            ChannelTx,
-            DescriptorChain,
-            DmaDescriptor,
-            DmaEligible,
-            PeripheralMarker,
-        },
+        dma::{ChannelRx, ChannelTx, DescriptorChain, DmaDescriptor, DmaEligible},
         gpio::{
             interconnect::{PeripheralInput, PeripheralOutput},
             InputSignal,
@@ -911,10 +904,9 @@ mod private {
         }
     }
 
-    pub trait RegBlock:
-        Peripheral<P = Self> + PeripheralMarker + DmaEligible + Into<super::AnyI2s> + 'static
-    {
+    pub trait RegBlock: Peripheral<P = Self> + DmaEligible + Into<super::AnyI2s> + 'static {
         fn register_block(&self) -> &RegisterBlock;
+        fn peripheral(&self) -> crate::system::Peripheral;
     }
 
     pub trait Signals: RegBlock {
@@ -1609,6 +1601,10 @@ mod private {
         fn register_block(&self) -> &RegisterBlock {
             unsafe { &*I2S0::PTR.cast::<RegisterBlock>() }
         }
+
+        fn peripheral(&self) -> crate::system::Peripheral {
+            crate::system::Peripheral::I2s0
+        }
     }
 
     impl RegisterAccessPrivate for I2S0 {
@@ -1713,6 +1709,10 @@ mod private {
         fn register_block(&self) -> &RegisterBlock {
             unsafe { &*I2S1::PTR.cast::<RegisterBlock>() }
         }
+
+        fn peripheral(&self) -> crate::system::Peripheral {
+            crate::system::Peripheral::I2s1
+        }
     }
 
     #[cfg(i2s1)]
@@ -1786,6 +1786,7 @@ mod private {
                 super::AnyI2sInner::I2s1(i2s) => i2s,
             } {
                 fn register_block(&self) -> &RegisterBlock;
+                fn peripheral(&self) -> crate::system::Peripheral;
             }
         }
     }

diff --git a/esp-hal/src/i2s/parallel.rs b/esp-hal/src/i2s/parallel.rs
@@ -52,7 +52,6 @@ use crate::{
         DmaError,
         DmaPeripheral,
         DmaTxBuffer,
-        PeripheralMarker,
         Tx,
     },
     gpio::{
@@ -424,10 +423,9 @@ fn calculate_clock(sample_rate: impl Into<fugit::HertzU32>, data_bits: u8) -> I2
 }
 
 #[doc(hidden)]
-pub trait Instance:
-    Peripheral<P = Self> + PeripheralMarker + DmaEligible + Into<AnyI2s> + 'static
-{
+pub trait Instance: Peripheral<P = Self> + DmaEligible + Into<AnyI2s> + 'static {
     fn register_block(&self) -> &RegisterBlock;
+    fn peripheral(&self) -> crate::system::Peripheral;
     fn ws_signal(&self) -> OutputSignal;
     fn data_out_signal(&self, i: usize, bits: u8) -> OutputSignal;
 
@@ -614,6 +612,10 @@ impl Instance for I2S0 {
         unsafe { &*I2S0::PTR.cast::<RegisterBlock>() }
     }
 
+    fn peripheral(&self) -> crate::system::Peripheral {
+        crate::system::Peripheral::I2s0
+    }
+
     fn ws_signal(&self) -> OutputSignal {
         OutputSignal::I2S0O_WS
     }
@@ -661,6 +663,10 @@ impl Instance for I2S1 {
         unsafe { &*I2S1::PTR.cast::<RegisterBlock>() }
     }
 
+    fn peripheral(&self) -> crate::system::Peripheral {
+        crate::system::Peripheral::I2s1
+    }
+
     fn ws_signal(&self) -> OutputSignal {
         OutputSignal::I2S1O_WS
     }
@@ -729,6 +735,7 @@ impl Instance for AnyI2s {
             AnyI2sInner::I2s1(i2s) => i2s,
         } {
             fn register_block(&self) -> &RegisterBlock;
+            fn peripheral(&self) -> crate::system::Peripheral;
             fn ws_signal(&self) -> OutputSignal;
             fn data_out_signal(&self, i: usize, bits: u8) -> OutputSignal ;
         }

diff --git a/esp-hal/src/macros.rs b/esp-hal/src/macros.rs
@@ -87,18 +87,6 @@ macro_rules! any_peripheral {
             $vis struct $name([< $name Inner >]);
             impl $crate::private::Sealed for $name {}
 
-            impl $crate::dma::PeripheralMarker for $name {
-                #[inline(always)]
-                fn peripheral(&self) -> $crate::system::Peripheral {
-                    match &self.0 {
-                        $(
-                            $(#[cfg($variant_meta)])*
-                            [<$name Inner>]::$variant(inner) => inner.peripheral(),
-                        )*
-                    }
-                }
-            }
-
             impl $crate::peripheral::Peripheral for $name {
                 type P = $name;
 

diff --git a/esp-hal/src/peripheral.rs b/esp-hal/src/peripheral.rs
@@ -210,10 +210,7 @@ mod peripheral_macros {
     /// Creates a new `Peripherals` struct and its associated methods.
     ///
     /// The macro has a few fields doing different things, in the form of
-    /// `[first] second <= third (fourth)`.
-    /// - The first field is the name of the `Peripherals` enum variant. This is
-    ///   optional and used to create `PeripheralMarker` implementations for
-    ///   DMA-eligible peripherals.
+    /// `second <= third (fourth)`.
     /// - The second field is the name of the peripheral, as it appears in the
     ///   `Peripherals` struct.
     /// - The third field is the name of the peripheral as it appears in the
@@ -226,15 +223,15 @@ mod peripheral_macros {
     macro_rules! peripherals {
         (
             $(
-                $([$enum_variant:ident])? $name:ident <= $from_pac:tt $(($($interrupt:ident),*))?
+                $name:ident <= $from_pac:tt $(($($interrupt:ident),*))?
             ), *$(,)?
         ) => {
 
             /// Contains the generated peripherals which implement [`Peripheral`]
             mod peripherals {
                 pub use super::pac::*;
                 $(
-                    $crate::create_peripheral!($([$enum_variant])? $name <= $from_pac);
+                    $crate::create_peripheral!($name <= $from_pac);
                 )*
             }
 
@@ -327,7 +324,7 @@ mod peripheral_macros {
     #[macro_export]
     /// Macro to create a peripheral structure.
     macro_rules! create_peripheral {
-        ($([$enum_variant:ident])? $name:ident <= virtual) => {
+        ($name:ident <= virtual) => {
             #[derive(Debug)]
             #[allow(non_camel_case_types)]
             /// Represents a virtual peripheral with no associated hardware.
@@ -358,15 +355,6 @@ mod peripheral_macros {
             }
 
             impl $crate::private::Sealed for $name {}
-
-            $(
-                impl $crate::dma::PeripheralMarker for $crate::peripherals::$name {
-                    #[inline(always)]
-                    fn peripheral(&self) -> $crate::system::Peripheral {
-                        $crate::system::Peripheral::$enum_variant
-                    }
-                }
-            )?
         };
 
         ($([$enum_variant:ident])? $name:ident <= $base:ident) => {