Fix TWAI on ESP32 (#2207)

* Some more gpio cleanup

* Allow TWAI loopback using the same pin, enable ESP32

* Impl Format for ErrorKind

* Generic frame constructors

* More TWAI cleanups

* Fix signals

* Set self-reception bit

* Teach users to use const blocks

* Fix resetting TWAI

* Set opmode when starting

* Apply errata workaround

* Fix ESP32 baudrate

* Clean up read_frame a bit

* Changelog

* Clean up clippy

* Fix compile errors

---------

Co-authored-by: Jesse Braham <jessebraham@users.noreply.github.com>

esp-hal/CHANGELOG.md

@@ -44,13 +44,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Removed gpio pin generics from I8080 driver type. (#2171)
 - I8080 driver now decides bus width at transfer time rather than construction time. (#2171)
 - Replaced `AnyPin` with `InputSignal` and `OutputSignal` and renamed `ErasedPin` to `AnyPin` (#2128)
-- Replaced the `ErasedTimer` enum with the `AnyTimer` struct. (#?)
+- Replaced the `ErasedTimer` enum with the `AnyTimer` struct. (#2144)
 - Changed the parameters of `Spi::with_pins` to no longer be optional (#2133)
 - Renamed `DummyPin` to `NoPin` and removed all internal logic from it. (#2133)
 - The `NO_PIN` constant has been removed. (#2133)
 - MSRV bump to 1.79 (#2156)
 - Allow handling interrupts while trying to lock critical section on multi-core chips. (#2197)
 - Removed the PS-RAM related features, replaced by `quad-psram`/`octal-psram`, `init_psram` takes a configuration parameter, it's now possible to auto-detect PS-RAM size (#2178)
+- `EspTwaiFrame` constructors now accept any type that converts into `esp_hal::twai::Id` (#2207)
 
 ### Fixed
 
@@ -69,6 +70,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - SPI: Fixed an issue where repeated calls to `dma_transfer` may end up looping indefinitely (#2179)
 - SPI: Fixed an issue that prevented correctly reading the first byte in a transaction (#2179)
 - PARL_IO: Fixed an issue that caused garbage to be output at the start of some requests (#2211)
+- TWAI on ESP32 (#2207)
 
 ### Removed
 

esp-hal/src/gpio/mod.rs

@@ -761,16 +761,11 @@ fn disable_usb_pads(gpionum: u8) {
         unsafe { &*crate::peripherals::USB_DEVICE::PTR }
             .conf0()
             .modify(|_, w| {
-                w.usb_pad_enable()
+                w.usb_pad_enable().clear_bit();
-                    .clear_bit()
+                w.dm_pullup().clear_bit();
-                    .dm_pullup()
+                w.dm_pulldown().clear_bit();
-                    .clear_bit()
+                w.dp_pullup().clear_bit();
-                    .dm_pulldown()
+                w.dp_pulldown().clear_bit()
-                    .clear_bit()
-                    .dp_pullup()
-                    .clear_bit()
-                    .dp_pulldown()
-                    .clear_bit()
             });
     }
 }
@@ -799,7 +794,10 @@ where
         #[cfg(esp32)]
         crate::soc::gpio::errata36(GPIONUM, Some(pull_up), Some(pull_down));
 
-        get_io_mux_reg(GPIONUM).modify(|_, w| w.fun_wpd().bit(pull_down).fun_wpu().bit(pull_up));
+        get_io_mux_reg(GPIONUM).modify(|_, w| {
+            w.fun_wpd().bit(pull_down);
+            w.fun_wpu().bit(pull_up)
+        });
     }
 }
 
@@ -814,12 +812,9 @@ where
         disable_usb_pads(GPIONUM);
 
         get_io_mux_reg(GPIONUM).modify(|_, w| unsafe {
-            w.mcu_sel()
+            w.mcu_sel().bits(GPIO_FUNCTION as u8);
-                .bits(GPIO_FUNCTION as u8)
+            w.fun_ie().set_bit();
-                .fun_ie()
+            w.slp_sel().clear_bit()
-                .set_bit()
-                .slp_sel()
-                .clear_bit()
         });
     }
 
@@ -936,14 +931,10 @@ where
         disable_usb_pads(GPIONUM);
 
         get_io_mux_reg(GPIONUM).modify(|_, w| unsafe {
-            w.mcu_sel()
+            w.mcu_sel().bits(alternate as u8);
-                .bits(alternate as u8)
+            w.fun_ie().bit(open_drain);
-                .fun_ie()
+            w.fun_drv().bits(DriveStrength::I20mA as u8);
-                .bit(open_drain)
+            w.slp_sel().clear_bit()
-                .fun_drv()
-                .bits(DriveStrength::I20mA as u8)
-                .slp_sel()
-                .clear_bit()
         });
     }
 
@@ -1231,10 +1222,10 @@ macro_rules! rtc_pins {
 
                 // disable input
                 paste::paste!{
-                    rtcio.$pin_reg.modify(|_,w| unsafe {w
+                    rtcio.$pin_reg.modify(|_,w| unsafe {
-                        .[<$prefix fun_ie>]().bit(input_enable)
+                        w.[<$prefix fun_ie>]().bit(input_enable);
-                        .[<$prefix mux_sel>]().bit(mux)
+                        w.[<$prefix mux_sel>]().bit(mux);
-                        .[<$prefix fun_sel>]().bits(func as u8)
+                        w.[<$prefix fun_sel>]().bits(func as u8)
                     });
                 }
             }
@@ -1485,10 +1476,10 @@ macro_rules! analog {
                     use $crate::peripherals::{GPIO};
 
                     get_io_mux_reg($pin_num).modify(|_,w| unsafe {
-                        w.mcu_sel().bits(1)
+                        w.mcu_sel().bits(1);
-                            .fun_ie().clear_bit()
+                        w.fun_ie().clear_bit();
-                            .fun_wpu().clear_bit()
+                        w.fun_wpu().clear_bit();
-                            .fun_wpd().clear_bit()
+                        w.fun_wpd().clear_bit()
                     });
 
                     unsafe{ &*GPIO::PTR }.enable_w1tc().write(|w| unsafe { w.bits(1 << $pin_num) });
@@ -1505,29 +1496,27 @@ macro_rules! touch {
     (@pin_specific $touch_num:expr, true) => {
         paste::paste! {
             unsafe { &*RTC_IO::ptr() }.[< touch_pad $touch_num >]().write(|w| unsafe {
-                w
+                w.xpd().set_bit();
-                .xpd().set_bit()
                 // clear input_enable
-                .fun_ie().clear_bit()
+                w.fun_ie().clear_bit();
                 // Connect pin to analog / RTC module instead of standard GPIO
-                .mux_sel().set_bit()
+                w.mux_sel().set_bit();
                 // Disable pull-up and pull-down resistors on the pin
-                .rue().clear_bit()
+                w.rue().clear_bit();
-                .rde().clear_bit()
+                w.rde().clear_bit();
-                .tie_opt().clear_bit()
+                w.tie_opt().clear_bit();
                 // Select function "RTC function 1" (GPIO) for analog use
-                .fun_sel().bits(0b00)
+                w.fun_sel().bits(0b00)
             });
         }
     };
 
     (@pin_specific $touch_num:expr, false) => {
         paste::paste! {
-            unsafe { &*RTC_IO::ptr() }.[< touch_pad $touch_num >]().write(|w|
+            unsafe { &*RTC_IO::ptr() }.[< touch_pad $touch_num >]().write(|w| {
-                w
+                w.xpd().set_bit();
-                .xpd().set_bit()
+                w.tie_opt().clear_bit()
-                .tie_opt().clear_bit()
+            });
-            );
         }
     };
 
@@ -2444,12 +2433,9 @@ fn is_listening(pin_num: u8) -> bool {
 fn set_int_enable(gpio_num: u8, int_ena: u8, int_type: u8, wake_up_from_light_sleep: bool) {
     let gpio = unsafe { &*crate::peripherals::GPIO::PTR };
     gpio.pin(gpio_num as usize).modify(|_, w| unsafe {
-        w.int_ena()
+        w.int_ena().bits(int_ena);
-            .bits(int_ena)
+        w.int_type().bits(int_type);
-            .int_type()
+        w.wakeup_enable().bit(wake_up_from_light_sleep)
-            .bits(int_type)
-            .wakeup_enable()
-            .bit(wake_up_from_light_sleep)
     });
 }
 
@@ -2496,36 +2482,36 @@ mod asynch {
     where
         P: InputPin,
     {
+        async fn wait_for(&mut self, event: Event) {
+            self.listen(event);
+            PinFuture::new(self.pin.number()).await
+        }
+
         /// Wait until the pin is high. If it is already high, return
         /// immediately.
         pub async fn wait_for_high(&mut self) {
-            self.listen(Event::HighLevel);
+            self.wait_for(Event::HighLevel).await
-            PinFuture::new(self.pin.number()).await
         }
 
         /// Wait until the pin is low. If it is already low, return immediately.
         pub async fn wait_for_low(&mut self) {
-            self.listen(Event::LowLevel);
+            self.wait_for(Event::LowLevel).await
-            PinFuture::new(self.pin.number()).await
         }
 
         /// Wait for the pin to undergo a transition from low to high.
         pub async fn wait_for_rising_edge(&mut self) {
-            self.listen(Event::RisingEdge);
+            self.wait_for(Event::RisingEdge).await
-            PinFuture::new(self.pin.number()).await
         }
 
         /// Wait for the pin to undergo a transition from high to low.
         pub async fn wait_for_falling_edge(&mut self) {
-            self.listen(Event::FallingEdge);
+            self.wait_for(Event::FallingEdge).await
-            PinFuture::new(self.pin.number()).await
         }
 
         /// Wait for the pin to undergo any transition, i.e low to high OR high
         /// to low.
         pub async fn wait_for_any_edge(&mut self) {
-            self.listen(Event::AnyEdge);
+            self.wait_for(Event::AnyEdge).await
-            PinFuture::new(self.pin.number()).await
         }
     }
 

esp-hal/src/soc/esp32/gpio.rs

@@ -467,7 +467,8 @@ pub enum OutputSignal {
     PWM2_4H                  = 120,
     PWM2_4L                  = 121,
     TWAI_TX                  = 123,
-    CAN_BUS_OFF_ON           = 124,
+    TWAI_BUS_OFF_ON          = 124,
+    TWAI_CLKOUT              = 125,
     SPID4                    = 128,
     SPID5                    = 129,
     SPID6                    = 130,

esp-hal/src/twai/mod.rs

@@ -56,9 +56,8 @@
 //!
 //! // Partially filter the incoming messages to reduce overhead of receiving
 //! // undesired messages
-//! const FILTER: twai::filter::SingleStandardFilter =
+//! can_config.set_filter(const { SingleStandardFilter::new(b"xxxxxxxxxx0",
-//!     SingleStandardFilter::new(b"xxxxxxxxxx0", b"x", [b"xxxxxxxx",
+//! b"x", [b"xxxxxxxx", b"xxxxxxxx"]) });
-//! b"xxxxxxxx"]); can_config.set_filter(FILTER);
 //!
 //! // Start the peripheral. This locks the configuration settings of the
 //! // peripheral and puts it into operation mode, allowing packets to be sent
@@ -110,19 +109,16 @@
 //!
 //! // Partially filter the incoming messages to reduce overhead of receiving
 //! // undesired messages
-//! const FILTER: twai::filter::SingleStandardFilter =
+//! can_config.set_filter(const { SingleStandardFilter::new(b"xxxxxxxxxx0",
-//!     SingleStandardFilter::new(b"xxxxxxxxxx0", b"x",
+//! b"x", [b"xxxxxxxx", b"xxxxxxxx"]) });
-//!         [b"xxxxxxxx", b"xxxxxxxx"]);
-//! can_config.set_filter(FILTER);
 //!
 //! // Start the peripheral. This locks the configuration settings of the
 //! // peripheral and puts it into operation mode, allowing packets to be sent
 //! // and received.
 //! let mut can = can_config.start();
 //!
-//! let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO.into(),
+//! let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO,
-//!     &[1, 2, 3]).unwrap();
+//!     &[1, 2, 3]).unwrap(); // Wait for a frame to be received.
-//! // Wait for a frame to be received.
 //! let frame = block!(can.receive()).unwrap();
 //!
 //! # loop {}
@@ -175,27 +171,35 @@ pub enum ErrorKind {
     Other,
 }
 
-impl core::fmt::Display for ErrorKind {
+macro_rules! impl_display {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+    ($($kind:ident => $msg:expr),* $(,)?) => {
-        match self {
+        impl core::fmt::Display for ErrorKind {
-            Self::Overrun => write!(f, "The peripheral receive buffer was overrun"),
+            fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-            Self::Bit => write!(
+                match self {
-                f,
+                    $(Self::$kind => write!(f, $msg)),*
-                "Bit value that is monitored differs from the bit value sent"
+                }
-            ),
+            }
-            Self::Stuff => write!(f, "Sixth consecutive equal bits detected"),
-            Self::Crc => write!(f, "Calculated CRC sequence does not equal the received one"),
-            Self::Form => write!(
-                f,
-                "A fixed-form bit field contains one or more illegal bits"
-            ),
-            Self::Acknowledge => write!(f, "Transmitted frame was not acknowledged"),
-            Self::Other => write!(
-                f,
-                "A different error occurred. The original error may contain more information"
-            ),
         }
-    }
+
+        #[cfg(feature = "defmt")]
+        impl defmt::Format for ErrorKind {
+            fn format(&self, f: defmt::Formatter<'_>) {
+                match self {
+                    $(Self::$kind => defmt::write!(f, $msg)),*
+                }
+            }
+        }
+    };
+}
+
+impl_display! {
+    Overrun => "The peripheral receive buffer was overrun",
+    Bit => "Bit value that is monitored differs from the bit value sent",
+    Stuff => "Sixth consecutive equal bits detected",
+    Crc => "Calculated CRC sequence does not equal the received one",
+    Form => "A fixed-form bit field contains one or more illegal bits",
+    Acknowledge => "Transmitted frame was not acknowledged",
+    Other => "A different error occurred. The original error may contain more information",
 }
 
 impl From<ErrorKind> for embedded_hal_02::can::ErrorKind {
@@ -239,6 +243,7 @@ impl embedded_can::Error for ErrorKind {
 }
 
 /// Specifies in which mode the TWAI controller will operate.
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
 pub enum TwaiMode {
     /// Normal operating mode
     Normal,
@@ -456,18 +461,17 @@ pub struct EspTwaiFrame {
 
 impl EspTwaiFrame {
     /// Creates a new `EspTwaiFrame` with the specified ID and data payload.
-    pub fn new(id: Id, data: &[u8]) -> Option<Self> {
+    pub fn new(id: impl Into<Id>, data: &[u8]) -> Option<Self> {
         // TWAI frames cannot contain more than 8 bytes of data.
         if data.len() > 8 {
             return None;
         }
 
         let mut d: [u8; 8] = [0; 8];
-        let (left, _unused) = d.split_at_mut(data.len());
+        d[..data.len()].copy_from_slice(data);
-        left.clone_from_slice(data);
 
         Some(EspTwaiFrame {
-            id,
+            id: id.into(),
             data: d,
             dlc: data.len(),
             is_remote: false,
@@ -477,14 +481,14 @@ impl EspTwaiFrame {
 
     /// Creates a new `EspTwaiFrame` for a transmission request with the
     /// specified ID and data length (DLC).
-    pub fn new_remote(id: Id, dlc: usize) -> Option<Self> {
+    pub fn new_remote(id: impl Into<Id>, dlc: usize) -> Option<Self> {
         // TWAI frames cannot have more than 8 bytes.
         if dlc > 8 {
             return None;
         }
 
         Some(EspTwaiFrame {
-            id,
+            id: id.into(),
             data: [0; 8],
             dlc,
             is_remote: true,
@@ -494,17 +498,16 @@ impl EspTwaiFrame {
 
     /// Creates a new `EspTwaiFrame` ready for self-reception with the specified
     /// ID and data payload.
-    pub fn new_self_reception(id: Id, data: &[u8]) -> Option<Self> {
+    pub fn new_self_reception(id: impl Into<Id>, data: &[u8]) -> Option<Self> {
         if data.len() > 8 {
             return None;
         }
 
         let mut d: [u8; 8] = [0; 8];
-        let (left, _unused) = d.split_at_mut(data.len());
+        d[..data.len()].copy_from_slice(data);
-        left.clone_from_slice(data);
 
         Some(EspTwaiFrame {
-            id,
+            id: id.into(),
             data: d,
             dlc: data.len(),
             is_remote: false,
@@ -539,20 +542,15 @@ impl EspTwaiFrame {
 
 impl embedded_hal_02::can::Frame for EspTwaiFrame {
     fn new(id: impl Into<embedded_hal_02::can::Id>, data: &[u8]) -> Option<Self> {
-        let id: embedded_hal_02::can::Id = id.into();
         Self::new(id.into(), data)
     }
 
     fn new_remote(id: impl Into<embedded_hal_02::can::Id>, dlc: usize) -> Option<Self> {
-        let id: embedded_hal_02::can::Id = id.into();
         Self::new_remote(id.into(), dlc)
     }
 
     fn is_extended(&self) -> bool {
-        match self.id {
+        matches!(self.id, Id::Extended(_))
-            Id::Standard(_) => false,
-            Id::Extended(_) => true,
-        }
     }
 
     fn is_remote_frame(&self) -> bool {
@@ -579,20 +577,15 @@ impl embedded_hal_02::can::Frame for EspTwaiFrame {
 
 impl embedded_can::Frame for EspTwaiFrame {
     fn new(id: impl Into<embedded_can::Id>, data: &[u8]) -> Option<Self> {
-        let id: embedded_can::Id = id.into();
         Self::new(id.into(), data)
     }
 
     fn new_remote(id: impl Into<embedded_can::Id>, dlc: usize) -> Option<Self> {
-        let id: embedded_can::Id = id.into();
         Self::new_remote(id.into(), dlc)
     }
 
     fn is_extended(&self) -> bool {
-        match self.id {
+        matches!(self.id, Id::Extended(_))
-            Id::Standard(_) => false,
-            Id::Extended(_) => true,
-        }
     }
 
     fn is_remote_frame(&self) -> bool {
@@ -743,6 +736,7 @@ impl BaudRate {
 pub struct TwaiConfiguration<'d, T, DM: crate::Mode> {
     peripheral: PhantomData<&'d PeripheralRef<'d, T>>,
     phantom: PhantomData<DM>,
+    mode: TwaiMode,
 }
 
 impl<'d, T, DM> TwaiConfiguration<'d, T, DM>
@@ -762,16 +756,21 @@ where
         crate::into_ref!(tx_pin, rx_pin);
 
         // Enable the peripheral clock for the TWAI peripheral.
-        T::reset_peripheral();
         T::enable_peripheral();
+        T::reset_peripheral();
 
         // Set RESET bit to 1
         T::register_block()
             .mode()
             .write(|w| w.reset_mode().set_bit());
 
-        rx_pin.init_input(Pull::None, crate::private::Internal);
+        #[cfg(esp32)]
-        rx_pin.connect_input_to_peripheral(T::INPUT_SIGNAL, crate::private::Internal);
+        {
+            // Enable extended register layout
+            T::register_block()
+                .clock_divider()
+                .modify(|r, w| unsafe { w.bits(r.bits() | 0x80) });
+        }
 
         if no_transceiver {
             tx_pin.set_to_open_drain_output(crate::private::Internal);
@@ -780,24 +779,14 @@ where
         }
         tx_pin.connect_peripheral_to_output(T::OUTPUT_SIGNAL, crate::private::Internal);
 
-        // Set the operating mode based on provided option
+        // Setting up RX pin later allows us to use a single pin in tests.
-        match mode {
+        // `set_to_push_pull_output` disables input, here we re-enable it if rx_pin
-            TwaiMode::Normal => {
+        // uses the same GPIO.
-                // Do nothing special, the default state is Normal mode.
+        rx_pin.init_input(Pull::None, crate::private::Internal);
-            }
+        rx_pin.connect_input_to_peripheral(T::INPUT_SIGNAL, crate::private::Internal);
-            TwaiMode::SelfTest => {
+
-                // Set the self-test mode (no acknowledgement required)
+        // Freeze REC by changing to LOM mode
-                T::register_block()
+        Self::set_mode(TwaiMode::ListenOnly);
-                    .mode()
-                    .modify(|_, w| w.self_test_mode().set_bit());
-            }
-            TwaiMode::ListenOnly => {
-                // Set listen-only mode
-                T::register_block()
-                    .mode()
-                    .modify(|_, w| w.listen_only_mode().set_bit());
-            }
-        }
 
         // Set TEC to 0
         T::register_block()
@@ -817,6 +806,7 @@ where
         let mut cfg = TwaiConfiguration {
             peripheral: PhantomData,
             phantom: PhantomData,
+            mode,
         };
 
         cfg.set_baud_rate(baud_rate);
@@ -848,7 +838,26 @@ where
         // have 1 subtracted from them before being stored into the register.
         let timing = baud_rate.timing();
 
-        let prescale = (timing.baud_rate_prescaler / 2) - 1;
+        #[cfg_attr(not(esp32), allow(unused_mut))]
+        let mut prescaler = timing.baud_rate_prescaler;
+
+        #[cfg(esp32)]
+        {
+            if timing.baud_rate_prescaler > 128 {
+                // Enable /2 baudrate divider
+                T::register_block()
+                    .int_ena()
+                    .modify(|r, w| unsafe { w.bits(r.bits() | 0x08) });
+                prescaler = timing.baud_rate_prescaler / 2;
+            } else {
+                // Disable /2 baudrate divider
+                T::register_block()
+                    .int_ena()
+                    .modify(|r, w| unsafe { w.bits(r.bits() & !0xF7) });
+            }
+        }
+
+        let prescale = (prescaler / 2) - 1;
         let sjw = timing.sync_jump_width - 1;
         let tseg_1 = timing.tseg_1 - 1;
         let tseg_2 = timing.tseg_2 - 1;
@@ -875,6 +884,9 @@ where
     /// versa. Your application should check the id again once a frame has
     /// been received to make sure it is the expected value.
     ///
+    /// You may use a `const {}` block to ensure that the filter is parsed
+    /// during program compilation.
+    ///
     /// [ESP32C3 Reference Manual](https://www.espressif.com/sites/default/files/documentation/esp32-c3_technical_reference_manual_en.pdf#subsubsection.29.4.6)
     pub fn set_filter(&mut self, filter: impl Filter) {
         // Set or clear the rx filter mode bit depending on the filter type.
@@ -905,9 +917,61 @@ where
             .write(|w| unsafe { w.err_warning_limit().bits(limit) });
     }
 
+    fn mode() -> TwaiMode {
+        let mode = T::register_block().mode().read();
+
+        if mode.self_test_mode().bit_is_set() {
+            TwaiMode::SelfTest
+        } else if mode.listen_only_mode().bit_is_set() {
+            TwaiMode::ListenOnly
+        } else {
+            TwaiMode::Normal
+        }
+    }
+
+    /// Set the operating mode based on provided option
+    fn set_mode(mode: TwaiMode) {
+        T::register_block().mode().modify(|_, w| {
+            // self-test mode turns off acknowledgement requirement
+            w.self_test_mode().bit(mode == TwaiMode::SelfTest);
+            w.listen_only_mode().bit(mode == TwaiMode::ListenOnly)
+        });
+    }
+
     /// Put the peripheral into Operation Mode, allowing the transmission and
     /// reception of packets using the new object.
     pub fn start(self) -> Twai<'d, T, DM> {
+        Self::set_mode(self.mode);
+
+        // Clear the TEC and REC
+        T::register_block()
+            .tx_err_cnt()
+            .write(|w| unsafe { w.tx_err_cnt().bits(0) });
+
+        let rec =
+            if cfg!(any(esp32, esp32s2, esp32s3, esp32c3)) && self.mode == TwaiMode::ListenOnly {
+                // Errata workaround: Prevent transmission of dominant error frame while in
+                // listen only mode by setting REC to 128 before exiting reset mode.
+                // This forces the controller to be error passive (thus only transmits recessive
+                // bits). The TEC/REC remain frozen in listen only mode thus
+                // ensuring we remain error passive.
+                128
+            } else {
+                0
+            };
+        T::register_block()
+            .rx_err_cnt()
+            .write(|w| unsafe { w.rx_err_cnt().bits(rec) });
+
+        // Clear any interrupts by reading the status register
+        cfg_if::cfg_if! {
+            if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] {
+                let _ = T::register_block().int_raw().read();
+            } else {
+                let _ = T::register_block().interrupt().read();
+            }
+        }
+
         // Put the peripheral into operation mode by clearing the reset mode bit.
         T::register_block()
             .mode()
@@ -1020,7 +1084,7 @@ pub struct Twai<'d, T, DM: crate::Mode> {
 
 impl<'d, T, DM> Twai<'d, T, DM>
 where
-    T: OperationInstance,
+    T: Instance,
     DM: crate::Mode,
 {
     /// Stop the peripheral, putting it into reset mode and enabling
@@ -1035,6 +1099,7 @@ where
         TwaiConfiguration {
             peripheral: PhantomData,
             phantom: PhantomData,
+            mode: TwaiConfiguration::<T, DM>::mode(),
         }
     }
 
@@ -1108,7 +1173,7 @@ pub struct TwaiTx<'d, T, DM: crate::Mode> {
 
 impl<'d, T, DM> TwaiTx<'d, T, DM>
 where
-    T: OperationInstance,
+    T: Instance,
     DM: crate::Mode,
 {
     /// Transmit a frame.
@@ -1150,7 +1215,7 @@ pub struct TwaiRx<'d, T, DM: crate::Mode> {
 
 impl<'d, T, DM> TwaiRx<'d, T, DM>
 where
-    T: OperationInstance,
+    T: Instance,
     DM: crate::Mode,
 {
     /// Receive a frame
@@ -1247,7 +1312,7 @@ unsafe fn copy_to_data_register(dest: *mut u32, src: &[u8]) {
 
 impl<'d, T, DM> embedded_hal_02::can::Can for Twai<'d, T, DM>
 where
-    T: OperationInstance,
+    T: Instance,
     DM: crate::Mode,
 {
     type Frame = EspTwaiFrame;
@@ -1271,7 +1336,7 @@ where
 
 impl<'d, T, DM> embedded_can::nb::Can for Twai<'d, T, DM>
 where
-    T: OperationInstance,
+    T: Instance,
     DM: crate::Mode,
 {
     type Frame = EspTwaiFrame;
@@ -1321,11 +1386,7 @@ pub trait Instance: crate::private::Sealed {
 
     /// Enables interrupts for the TWAI peripheral.
     fn enable_interrupts();
-}
 
-/// An extension of the `Instance` trait that provides additional operations
-/// for managing and interacting with the TWAI peripheral.
-pub trait OperationInstance: Instance {
     /// Returns a reference to the asynchronous state for this TWAI instance.
     fn async_state() -> &'static asynch::TwaiAsyncState {
         &asynch::TWAI_STATE[Self::NUMBER]
@@ -1344,10 +1405,11 @@ pub trait OperationInstance: Instance {
     fn write_frame(frame: &EspTwaiFrame) {
         // Assemble the frame information into the data_0 byte.
         let frame_format: u8 = matches!(frame.id, Id::Extended(_)) as u8;
+        let self_reception: u8 = frame.self_reception as u8;
         let rtr_bit: u8 = frame.is_remote as u8;
         let dlc_bits: u8 = frame.dlc as u8 & 0b1111;
 
-        let data_0: u8 = frame_format << 7 | rtr_bit << 6 | dlc_bits;
+        let data_0: u8 = frame_format << 7 | rtr_bit << 6 | self_reception << 4 | dlc_bits;
 
         let register_block = Self::register_block();
 
@@ -1355,8 +1417,9 @@ pub trait OperationInstance: Instance {
             .data_0()
             .write(|w| unsafe { w.tx_byte_0().bits(data_0) });
 
-        // Assemble the identifier information of the packet.
+        // Assemble the identifier information of the packet and return where the data
-        match frame.id {
+        // buffer starts.
+        let data_ptr = match frame.id {
             Id::Standard(id) => {
                 let id = id.as_raw();
 
@@ -1367,6 +1430,8 @@ pub trait OperationInstance: Instance {
                 register_block
                     .data_2()
                     .write(|w| unsafe { w.tx_byte_2().bits((id << 5) as u8) });
+
+                register_block.data_3().as_ptr()
             }
             Id::Extended(id) => {
                 let id = id.as_raw();
@@ -1383,43 +1448,33 @@ pub trait OperationInstance: Instance {
                 register_block
                     .data_4()
                     .write(|w| unsafe { w.tx_byte_4().bits((id << 3) as u8) });
+
+                register_block.data_5().as_ptr()
             }
-        }
+        };
 
         // Store the data portion of the packet into the transmit buffer.
-        if !frame.is_remote {
+        unsafe {
-            match frame.id {
+            copy_to_data_register(
-                Id::Standard(_) => unsafe {
+                data_ptr,
-                    copy_to_data_register(
+                match frame.is_remote {
-                        register_block.data_3().as_ptr(),
+                    true => &[], // RTR frame, so no data is included.
-                        match frame.is_remote {
+                    false => &frame.data[0..frame.dlc],
-                            true => &[],
-                            false => &frame.data[0..frame.dlc],
-                        },
-                    )
                 },
-                Id::Extended(_) => unsafe {
+            )
-                    copy_to_data_register(
-                        register_block.data_5().as_ptr(),
-                        match frame.is_remote {
-                            true => &[],
-                            false => &frame.data[0..frame.dlc],
-                        },
-                    )
-                },
-            }
-        } else {
-            // Is RTR frame, so no data is included.
         }
 
         // Trigger the appropriate transmission request based on self_reception flag
         if frame.self_reception {
-            #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))]
+            cfg_if::cfg_if! {
-            register_block.cmd().write(|w| w.self_rx_req().set_bit());
+                if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] {
-            #[cfg(not(any(esp32, esp32c3, esp32s2, esp32s3)))]
+                    register_block.cmd().write(|w| w.self_rx_req().set_bit());
-            register_block
+                } else {
-                .cmd()
+                    register_block
-                .write(|w| w.self_rx_request().set_bit());
+                        .cmd()
+                        .write(|w| w.self_rx_request().set_bit());
+                }
+            }
         } else {
             // Set the transmit request command, this will lock the transmit buffer until
             // the transmission is complete or aborted.
@@ -1439,33 +1494,20 @@ pub trait OperationInstance: Instance {
         let dlc = (data_0 & 0b1111) as usize;
 
         // Read the payload from the packet and construct a frame.
-        let frame = if is_standard_format {
+        let (id, data_ptr) = if is_standard_format {
             // Frame uses standard 11 bit id.
             let data_1 = register_block.data_1().read().tx_byte_1().bits();
-
             let data_2 = register_block.data_2().read().tx_byte_2().bits();
 
             let raw_id: u16 = ((data_1 as u16) << 3) | ((data_2 as u16) >> 5);
 
-            let id = StandardId::new(raw_id).unwrap();
+            let id = Id::from(StandardId::new(raw_id).unwrap());
-
+            (id, register_block.data_3().as_ptr())
-            if is_data_frame {
-                // Create a new frame from the contents of the appropriate TWAI_DATA_x_REG
-                // registers.
-                unsafe {
-                    EspTwaiFrame::new_from_data_registers(id, register_block.data_3().as_ptr(), dlc)
-                }
-            } else {
-                EspTwaiFrame::new_remote(id.into(), dlc).unwrap()
-            }
         } else {
             // Frame uses extended 29 bit id.
             let data_1 = register_block.data_1().read().tx_byte_1().bits();
-
             let data_2 = register_block.data_2().read().tx_byte_2().bits();
-
             let data_3 = register_block.data_3().read().tx_byte_3().bits();
-
             let data_4 = register_block.data_4().read().tx_byte_4().bits();
 
             let raw_id: u32 = (data_1 as u32) << 21
@@ -1473,32 +1515,37 @@ pub trait OperationInstance: Instance {
                 | (data_3 as u32) << 5
                 | (data_4 as u32) >> 3;
 
-            let id = ExtendedId::new(raw_id).unwrap();
+            let id = Id::from(ExtendedId::new(raw_id).unwrap());
+            (id, register_block.data_5().as_ptr())
+        };
 
-            if is_data_frame {
+        let frame = if is_data_frame {
-                unsafe {
+            unsafe { EspTwaiFrame::new_from_data_registers(id, data_ptr, dlc) }
-                    EspTwaiFrame::new_from_data_registers(id, register_block.data_5().as_ptr(), dlc)
+        } else {
-                }
+            EspTwaiFrame::new_remote(id, dlc).unwrap()
-            } else {
-                EspTwaiFrame::new_remote(id.into(), dlc).unwrap()
-            }
         };
 
         // Release the packet we read from the FIFO, allowing the peripheral to prepare
         // the next packet.
-        register_block.cmd().write(|w| w.release_buf().set_bit());
+        Self::release_receive_fifo();
 
         frame
     }
 }
 
-#[cfg(any(esp32, esp32c3, esp32s2, esp32s3))]
 impl Instance for crate::peripherals::TWAI0 {
     const SYSTEM_PERIPHERAL: system::Peripheral = system::Peripheral::Twai0;
     const NUMBER: usize = 0;
 
-    const INPUT_SIGNAL: InputSignal = InputSignal::TWAI_RX;
+    cfg_if::cfg_if! {
-    const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI_TX;
+        if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] {
+            const INPUT_SIGNAL: InputSignal = InputSignal::TWAI_RX;
+            const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI_TX;
+        } else {
+            const INPUT_SIGNAL: InputSignal = InputSignal::TWAI0_RX;
+            const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI0_TX;
+        }
+    }
 
     const INTERRUPT: crate::peripherals::Interrupt = crate::peripherals::Interrupt::TWAI0;
 
@@ -1521,72 +1568,30 @@ impl Instance for crate::peripherals::TWAI0 {
 
     fn enable_interrupts() {
         let register_block = Self::register_block();
-        register_block.int_ena().modify(|_, w| {
+
-            w.rx_int_ena()
+        cfg_if::cfg_if! {
-                .set_bit()
+            if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] {
-                .tx_int_ena()
+                register_block.int_ena().modify(|_, w| {
-                .set_bit()
+                    w.rx_int_ena().set_bit();
-                .bus_err_int_ena()
+                    w.tx_int_ena().set_bit();
-                .set_bit()
+                    w.bus_err_int_ena().set_bit();
-                .arb_lost_int_ena()
+                    w.arb_lost_int_ena().set_bit();
-                .set_bit()
+                    w.err_passive_int_ena().set_bit()
-                .err_passive_int_ena()
+                });
-                .set_bit()
+            } else {
-        });
+                register_block.interrupt_enable().modify(|_, w| {
+                    w.ext_receive_int_ena().set_bit();
+                    w.ext_transmit_int_ena().set_bit();
+                    w.bus_err_int_ena().set_bit();
+                    w.arbitration_lost_int_ena().set_bit();
+                    w.err_passive_int_ena().set_bit()
+                });
+            }
+        }
     }
 }
 
-#[cfg(any(esp32, esp32c3, esp32s2, esp32s3))]
+#[cfg(twai1)]
-impl OperationInstance for crate::peripherals::TWAI0 {}
-
-#[cfg(any(esp32h2, esp32c6))]
-impl Instance for crate::peripherals::TWAI0 {
-    const SYSTEM_PERIPHERAL: system::Peripheral = system::Peripheral::Twai0;
-    const NUMBER: usize = 0;
-
-    const INPUT_SIGNAL: InputSignal = InputSignal::TWAI0_RX;
-    const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI0_TX;
-
-    const INTERRUPT: crate::peripherals::Interrupt = crate::peripherals::Interrupt::TWAI0;
-
-    fn async_handler() -> InterruptHandler {
-        asynch::twai0
-    }
-
-    #[inline(always)]
-    fn register_block() -> &'static RegisterBlock {
-        unsafe { &*crate::peripherals::TWAI0::PTR }
-    }
-
-    fn reset_peripheral() {
-        PeripheralClockControl::enable(crate::system::Peripheral::Twai0);
-    }
-
-    fn enable_peripheral() {
-        PeripheralClockControl::enable(crate::system::Peripheral::Twai0);
-    }
-
-    fn enable_interrupts() {
-        let register_block = Self::register_block();
-        register_block.interrupt_enable().modify(|_, w| {
-            w.ext_receive_int_ena()
-                .set_bit()
-                .ext_transmit_int_ena()
-                .set_bit()
-                .bus_err_int_ena()
-                .set_bit()
-                .arbitration_lost_int_ena()
-                .set_bit()
-                .err_passive_int_ena()
-                .set_bit()
-        });
-    }
-}
-
-#[cfg(any(esp32h2, esp32c6))]
-impl OperationInstance for crate::peripherals::TWAI0 {}
-
-#[cfg(esp32c6)]
 impl Instance for crate::peripherals::TWAI1 {
     const SYSTEM_PERIPHERAL: system::Peripheral = system::Peripheral::Twai1;
     const NUMBER: usize = 1;
@@ -1616,23 +1621,15 @@ impl Instance for crate::peripherals::TWAI1 {
     fn enable_interrupts() {
         let register_block = Self::register_block();
         register_block.interrupt_enable().modify(|_, w| {
-            w.ext_receive_int_ena()
+            w.ext_receive_int_ena().set_bit();
-                .set_bit()
+            w.ext_transmit_int_ena().set_bit();
-                .ext_transmit_int_ena()
+            w.bus_err_int_ena().set_bit();
-                .set_bit()
+            w.arbitration_lost_int_ena().set_bit();
-                .bus_err_int_ena()
+            w.err_passive_int_ena().set_bit()
-                .set_bit()
-                .arbitration_lost_int_ena()
-                .set_bit()
-                .err_passive_int_ena()
-                .set_bit()
         });
     }
 }
 
-#[cfg(esp32c6)]
-impl OperationInstance for crate::peripherals::TWAI1 {}
-
 mod asynch {
     use core::{future::poll_fn, task::Poll};
 
@@ -1645,7 +1642,7 @@ mod asynch {
 
     use super::*;
     use crate::peripherals::TWAI0;
-    #[cfg(esp32c6)]
+    #[cfg(twai1)]
     use crate::peripherals::TWAI1;
 
     pub struct TwaiAsyncState {
@@ -1676,7 +1673,7 @@ mod asynch {
 
     impl<T> Twai<'_, T, crate::Async>
     where
-        T: OperationInstance,
+        T: Instance,
     {
         /// Transmits an `EspTwaiFrame` asynchronously over the TWAI bus.
         pub async fn transmit_async(&mut self, frame: &EspTwaiFrame) -> Result<(), EspTwaiError> {
@@ -1690,7 +1687,7 @@ mod asynch {
 
     impl<'d, T> TwaiTx<'d, T, crate::Async>
     where
-        T: OperationInstance,
+        T: Instance,
     {
         /// Transmits an `EspTwaiFrame` asynchronously over the TWAI bus.
         pub async fn transmit_async(&mut self, frame: &EspTwaiFrame) -> Result<(), EspTwaiError> {
@@ -1720,7 +1717,7 @@ mod asynch {
 
     impl<'d, T> TwaiRx<'d, T, crate::Async>
     where
-        T: OperationInstance,
+        T: Instance,
     {
         /// Receives an `EspTwaiFrame` asynchronously over the TWAI bus.
         pub async fn receive_async(&mut self) -> Result<EspTwaiFrame, EspTwaiError> {
@@ -1729,7 +1726,7 @@ mod asynch {
                 T::async_state().err_waker.register(cx.waker());
 
                 if let Poll::Ready(result) = T::async_state().rx_queue.poll_receive(cx) {
-                    return Poll::Ready(result);
+                    Poll::Ready(result)
                 } else {
                     let register_block = T::register_block();
                     let status = register_block.status().read();
@@ -1743,29 +1740,45 @@ mod asynch {
                     if status.miss_st().bit_is_set() {
                         return Poll::Ready(Err(EspTwaiError::EmbeddedHAL(ErrorKind::Overrun)));
                     }
-                }
 
-                Poll::Pending
+                    Poll::Pending
+                }
             })
             .await
         }
     }
 
-    #[cfg(any(esp32c3, esp32, esp32s2, esp32s3))]
     #[handler]
     pub(super) fn twai0() {
         let register_block = TWAI0::register_block();
 
-        let intr_enable = register_block.int_ena().read();
+        cfg_if::cfg_if! {
-        let intr_status = register_block.int_raw().read();
+            if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] {
+                let intr_enable = register_block.int_ena().read();
+                let intr_status = register_block.int_raw().read();
+
+                let int_ena_reg = register_block.int_ena();
+
+                let tx_int_status = intr_status.tx_int_st();
+                let rx_int_status = intr_status.rx_int_st();
+            } else {
+                let intr_enable = register_block.interrupt_enable().read();
+                let intr_status = register_block.interrupt().read();
+
+                let int_ena_reg = register_block.interrupt_enable();
+
+                let tx_int_status = intr_status.transmit_int_st();
+                let rx_int_status = intr_status.receive_int_st();
+            }
+        }
 
         let async_state = TWAI0::async_state();
 
-        if intr_status.tx_int_st().bit_is_set() {
+        if tx_int_status.bit_is_set() {
             async_state.tx_waker.wake();
         }
 
-        if intr_status.rx_int_st().bit_is_set() {
+        if rx_int_status.bit_is_set() {
             let status = register_block.status().read();
 
             let rx_queue = &async_state.rx_queue;
@@ -1789,59 +1802,13 @@ mod asynch {
             async_state.err_waker.wake();
         }
 
+        // Clear interrupt request bits
         unsafe {
-            register_block
+            int_ena_reg.modify(|_, w| w.bits(intr_enable.bits() & (!intr_status.bits() | 1)));
-                .int_ena()
-                .modify(|_, w| w.bits(intr_enable.bits() & (!intr_status.bits() | 1)));
         }
     }
 
-    #[cfg(any(esp32h2, esp32c6))]
+    #[cfg(twai1)]
-    #[handler]
-    pub(super) fn twai0() {
-        let register_block = TWAI0::register_block();
-
-        let intr_enable = register_block.interrupt_enable().read();
-        let intr_status = register_block.interrupt().read();
-
-        let async_state = TWAI0::async_state();
-
-        if intr_status.transmit_int_st().bit_is_set() {
-            async_state.tx_waker.wake();
-        }
-
-        if intr_status.receive_int_st().bit_is_set() {
-            let status = register_block.status().read();
-
-            let rx_queue = &async_state.rx_queue;
-
-            if status.bus_off_st().bit_is_set() {
-                let _ = rx_queue.try_send(Err(EspTwaiError::BusOff));
-            }
-
-            if status.miss_st().bit_is_set() {
-                let _ = rx_queue.try_send(Err(EspTwaiError::EmbeddedHAL(ErrorKind::Overrun)));
-            }
-
-            let frame = TWAI0::read_frame();
-
-            let _ = rx_queue.try_send(Ok(frame));
-
-            register_block.cmd().write(|w| w.release_buf().set_bit());
-        }
-
-        if intr_status.bits() & 0b11111100 > 0 {
-            async_state.err_waker.wake();
-        }
-
-        unsafe {
-            register_block
-                .interrupt_enable()
-                .modify(|_, w| w.bits(intr_enable.bits() & (!intr_status.bits() | 1)));
-        }
-    }
-
-    #[cfg(esp32c6)]
     #[handler]
     pub(super) fn twai1() {
         let register_block = TWAI1::register_block();

examples/src/bin/twai.rs

@@ -65,9 +65,9 @@ fn main() -> ! {
     // be explicitly checked in the application instead of fully relying on
     // these partial acceptance filters to exactly match.
     // A filter that matches StandardId::ZERO.
-    const FILTER: SingleStandardFilter =
+    twai_config.set_filter(
-        SingleStandardFilter::new(b"00000000000", b"x", [b"xxxxxxxx", b"xxxxxxxx"]);
+        const { SingleStandardFilter::new(b"00000000000", b"x", [b"xxxxxxxx", b"xxxxxxxx"]) },
-    twai_config.set_filter(FILTER);
+    );
 
     // Start the peripheral. This locks the configuration settings of the peripheral
     // and puts it into operation mode, allowing packets to be sent and
@@ -77,7 +77,7 @@ fn main() -> ! {
     if IS_FIRST_SENDER {
         // Send a frame to the other ESP
         // Use `new_self_reception` if you want to use self-testing.
-        let frame = EspTwaiFrame::new(StandardId::ZERO.into(), &[1, 2, 3]).unwrap();
+        let frame = EspTwaiFrame::new(StandardId::ZERO, &[1, 2, 3]).unwrap();
         block!(can.transmit(&frame)).unwrap();
         println!("Sent a frame");
     }

hil-test/tests/twai.rs

@@ -1,6 +1,6 @@
 //! TWAI test
 
-//% CHIPS: esp32c3 esp32c6 esp32h2 esp32s2 esp32s3
+//% CHIPS: esp32 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3
 
 #![no_std]
 #![no_main]
@@ -33,19 +33,21 @@ mod tests {
 
         let io = Io::new(peripherals.GPIO, peripherals.IO_MUX);
 
-        let (tx_pin, rx_pin) = hil_test::common_test_pins!(io);
+        let (loopback_pin, _) = hil_test::common_test_pins!(io);
 
         let mut config = twai::TwaiConfiguration::new(
             peripherals.TWAI0,
-            rx_pin,
+            loopback_pin.peripheral_input(),
-            tx_pin,
+            loopback_pin,
             twai::BaudRate::B1000K,
             TwaiMode::SelfTest,
         );
 
-        const FILTER: SingleStandardFilter =
+        config.set_filter(SingleStandardFilter::new(
-            SingleStandardFilter::new(b"00000000000", b"x", [b"xxxxxxxx", b"xxxxxxxx"]);
+            b"00000000000",
-        config.set_filter(FILTER);
+            b"x",
+            [b"xxxxxxxx", b"xxxxxxxx"],
+        ));
 
         let twai = config.start();
 
@@ -55,7 +57,7 @@ mod tests {
     #[test]
     #[timeout(3)]
     fn test_send_receive(mut ctx: Context) {
-        let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO.into(), &[1, 2, 3]).unwrap();
+        let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO, &[1, 2, 3]).unwrap();
         block!(ctx.twai.transmit(&frame)).unwrap();
 
         let frame = block!(ctx.twai.receive()).unwrap();