From 02d221ee529e91ae1ede3147b6304d2710a74b5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 23 Sep 2024 17:24:27 +0200 Subject: [PATCH] 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 --- esp-hal/CHANGELOG.md | 4 +- esp-hal/src/gpio/mod.rs | 110 ++++--- esp-hal/src/soc/esp32/gpio.rs | 3 +- esp-hal/src/twai/mod.rs | 527 ++++++++++++++++------------------ examples/src/bin/twai.rs | 8 +- hil-test/tests/twai.rs | 18 +- 6 files changed, 314 insertions(+), 356 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 8fec8ff52..09abacb56 100644 --- a/esp-hal/CHANGELOG.md +++ b/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 diff --git a/esp-hal/src/gpio/mod.rs b/esp-hal/src/gpio/mod.rs index df03c5d67..02540005b 100644 --- a/esp-hal/src/gpio/mod.rs +++ b/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() - .clear_bit() - .dm_pullup() - .clear_bit() - .dm_pulldown() - .clear_bit() - .dp_pullup() - .clear_bit() - .dp_pulldown() - .clear_bit() + w.usb_pad_enable().clear_bit(); + w.dm_pullup().clear_bit(); + w.dm_pulldown().clear_bit(); + w.dp_pullup().clear_bit(); + w.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() - .bits(GPIO_FUNCTION as u8) - .fun_ie() - .set_bit() - .slp_sel() - .clear_bit() + w.mcu_sel().bits(GPIO_FUNCTION as u8); + w.fun_ie().set_bit(); + w.slp_sel().clear_bit() }); } @@ -936,14 +931,10 @@ where disable_usb_pads(GPIONUM); get_io_mux_reg(GPIONUM).modify(|_, w| unsafe { - w.mcu_sel() - .bits(alternate as u8) - .fun_ie() - .bit(open_drain) - .fun_drv() - .bits(DriveStrength::I20mA as u8) - .slp_sel() - .clear_bit() + w.mcu_sel().bits(alternate as u8); + w.fun_ie().bit(open_drain); + w.fun_drv().bits(DriveStrength::I20mA as u8); + w.slp_sel().clear_bit() }); } @@ -1231,10 +1222,10 @@ macro_rules! rtc_pins { // disable input paste::paste!{ - rtcio.$pin_reg.modify(|_,w| unsafe {w - .[<$prefix fun_ie>]().bit(input_enable) - .[<$prefix mux_sel>]().bit(mux) - .[<$prefix fun_sel>]().bits(func as u8) + rtcio.$pin_reg.modify(|_,w| unsafe { + w.[<$prefix fun_ie>]().bit(input_enable); + w.[<$prefix mux_sel>]().bit(mux); + 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) - .fun_ie().clear_bit() - .fun_wpu().clear_bit() - .fun_wpd().clear_bit() + w.mcu_sel().bits(1); + w.fun_ie().clear_bit(); + w.fun_wpu().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 - .xpd().set_bit() + w.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() - .rde().clear_bit() - .tie_opt().clear_bit() + w.rue().clear_bit(); + w.rde().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| - w - .xpd().set_bit() - .tie_opt().clear_bit() - ); + unsafe { &*RTC_IO::ptr() }.[< touch_pad $touch_num >]().write(|w| { + w.xpd().set_bit(); + w.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() - .bits(int_ena) - .int_type() - .bits(int_type) - .wakeup_enable() - .bit(wake_up_from_light_sleep) + w.int_ena().bits(int_ena); + w.int_type().bits(int_type); + w.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); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::HighLevel).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); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::LowLevel).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); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::RisingEdge).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); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::FallingEdge).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); - PinFuture::new(self.pin.number()).await + self.wait_for(Event::AnyEdge).await } } diff --git a/esp-hal/src/soc/esp32/gpio.rs b/esp-hal/src/soc/esp32/gpio.rs index 47dea4116..44395379c 100644 --- a/esp-hal/src/soc/esp32/gpio.rs +++ b/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, diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index f0e06ecda..6d9bd2ffa 100644 --- a/esp-hal/src/twai/mod.rs +++ b/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 = -//! SingleStandardFilter::new(b"xxxxxxxxxx0", b"x", [b"xxxxxxxx", -//! b"xxxxxxxx"]); can_config.set_filter(FILTER); +//! can_config.set_filter(const { SingleStandardFilter::new(b"xxxxxxxxxx0", +//! b"x", [b"xxxxxxxx", b"xxxxxxxx"]) }); //! //! // 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 = -//! SingleStandardFilter::new(b"xxxxxxxxxx0", b"x", -//! [b"xxxxxxxx", b"xxxxxxxx"]); -//! can_config.set_filter(FILTER); +//! can_config.set_filter(const { SingleStandardFilter::new(b"xxxxxxxxxx0", +//! b"x", [b"xxxxxxxx", b"xxxxxxxx"]) }); //! //! // 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(), -//! &[1, 2, 3]).unwrap(); -//! // Wait for a frame to be received. +//! let frame = EspTwaiFrame::new_self_reception(StandardId::ZERO, +//! &[1, 2, 3]).unwrap(); // 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 { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - match self { - Self::Overrun => write!(f, "The peripheral receive buffer was overrun"), - Self::Bit => write!( - f, - "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" - ), +macro_rules! impl_display { + ($($kind:ident => $msg:expr),* $(,)?) => { + impl core::fmt::Display for ErrorKind { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + $(Self::$kind => write!(f, $msg)),* + } + } } - } + + #[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 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 { + pub fn new(id: impl Into, data: &[u8]) -> Option { // 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()); - left.clone_from_slice(data); + d[..data.len()].copy_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 { + pub fn new_remote(id: impl Into, dlc: usize) -> Option { // 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 { + pub fn new_self_reception(id: impl Into, data: &[u8]) -> Option { if data.len() > 8 { return None; } let mut d: [u8; 8] = [0; 8]; - let (left, _unused) = d.split_at_mut(data.len()); - left.clone_from_slice(data); + d[..data.len()].copy_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, data: &[u8]) -> Option { - let id: embedded_hal_02::can::Id = id.into(); Self::new(id.into(), data) } fn new_remote(id: impl Into, dlc: usize) -> Option { - let id: embedded_hal_02::can::Id = id.into(); Self::new_remote(id.into(), dlc) } fn is_extended(&self) -> bool { - match self.id { - Id::Standard(_) => false, - Id::Extended(_) => true, - } + matches!(self.id, Id::Extended(_)) } 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, data: &[u8]) -> Option { - let id: embedded_can::Id = id.into(); Self::new(id.into(), data) } fn new_remote(id: impl Into, dlc: usize) -> Option { - let id: embedded_can::Id = id.into(); Self::new_remote(id.into(), dlc) } fn is_extended(&self) -> bool { - match self.id { - Id::Standard(_) => false, - Id::Extended(_) => true, - } + matches!(self.id, Id::Extended(_)) } 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, + 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); - rx_pin.connect_input_to_peripheral(T::INPUT_SIGNAL, crate::private::Internal); + #[cfg(esp32)] + { + // 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 - match mode { - TwaiMode::Normal => { - // Do nothing special, the default state is Normal mode. - } - TwaiMode::SelfTest => { - // Set the self-test mode (no acknowledgement required) - T::register_block() - .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()); - } - } + // Setting up RX pin later allows us to use a single pin in tests. + // `set_to_push_pull_output` disables input, here we re-enable it if rx_pin + // uses the same GPIO. + rx_pin.init_input(Pull::None, crate::private::Internal); + rx_pin.connect_input_to_peripheral(T::INPUT_SIGNAL, crate::private::Internal); + + // Freeze REC by changing to LOM mode + Self::set_mode(TwaiMode::ListenOnly); // 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::::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. - match frame.id { + // Assemble the identifier information of the packet and return where the data + // 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 { - match frame.id { - Id::Standard(_) => unsafe { - copy_to_data_register( - register_block.data_3().as_ptr(), - match frame.is_remote { - true => &[], - false => &frame.data[0..frame.dlc], - }, - ) + unsafe { + copy_to_data_register( + data_ptr, + match frame.is_remote { + true => &[], // RTR frame, so no data is included. + 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))] - register_block.cmd().write(|w| w.self_rx_req().set_bit()); - #[cfg(not(any(esp32, esp32c3, esp32s2, esp32s3)))] - register_block - .cmd() - .write(|w| w.self_rx_request().set_bit()); + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { + register_block.cmd().write(|w| w.self_rx_req().set_bit()); + } else { + register_block + .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(); - - 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() - } + let id = Id::from(StandardId::new(raw_id).unwrap()); + (id, register_block.data_3().as_ptr()) } 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 { - unsafe { - EspTwaiFrame::new_from_data_registers(id, register_block.data_5().as_ptr(), dlc) - } - } else { - EspTwaiFrame::new_remote(id.into(), dlc).unwrap() - } + let frame = if is_data_frame { + unsafe { EspTwaiFrame::new_from_data_registers(id, data_ptr, dlc) } + } else { + EspTwaiFrame::new_remote(id, 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; - const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI_TX; + cfg_if::cfg_if! { + 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() - .set_bit() - .tx_int_ena() - .set_bit() - .bus_err_int_ena() - .set_bit() - .arb_lost_int_ena() - .set_bit() - .err_passive_int_ena() - .set_bit() - }); + + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { + register_block.int_ena().modify(|_, w| { + w.rx_int_ena().set_bit(); + w.tx_int_ena().set_bit(); + w.bus_err_int_ena().set_bit(); + w.arb_lost_int_ena().set_bit(); + w.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))] -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)] +#[cfg(twai1)] 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() - .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() + 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(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 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 { @@ -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(); - let intr_status = register_block.int_raw().read(); + cfg_if::cfg_if! { + 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() - .modify(|_, w| w.bits(intr_enable.bits() & (!intr_status.bits() | 1))); + int_ena_reg.modify(|_, w| w.bits(intr_enable.bits() & (!intr_status.bits() | 1))); } } - #[cfg(any(esp32h2, esp32c6))] - #[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)] + #[cfg(twai1)] #[handler] pub(super) fn twai1() { let register_block = TWAI1::register_block(); diff --git a/examples/src/bin/twai.rs b/examples/src/bin/twai.rs index 14d690f4e..89e1e2ced 100644 --- a/examples/src/bin/twai.rs +++ b/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 = - SingleStandardFilter::new(b"00000000000", b"x", [b"xxxxxxxx", b"xxxxxxxx"]); - twai_config.set_filter(FILTER); + twai_config.set_filter( + const { SingleStandardFilter::new(b"00000000000", b"x", [b"xxxxxxxx", b"xxxxxxxx"]) }, + ); // 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"); } diff --git a/hil-test/tests/twai.rs b/hil-test/tests/twai.rs index 508c74d11..6bb6f9c1c 100644 --- a/hil-test/tests/twai.rs +++ b/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, - tx_pin, + loopback_pin.peripheral_input(), + loopback_pin, twai::BaudRate::B1000K, TwaiMode::SelfTest, ); - const FILTER: SingleStandardFilter = - SingleStandardFilter::new(b"00000000000", b"x", [b"xxxxxxxx", b"xxxxxxxx"]); - config.set_filter(FILTER); + config.set_filter(SingleStandardFilter::new( + b"00000000000", + 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();