From 8851aaf5b22836a55248cdc3d6503a2303e02ea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 16 Oct 2024 16:35:42 +0200 Subject: [PATCH 1/6] Swap order of generics --- esp-hal/src/twai/mod.rs | 46 ++++++++++++++++---------------- examples/src/bin/embassy_twai.rs | 2 +- hil-test/tests/twai.rs | 2 +- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index b99cf861ef7..3787523f67e 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -742,13 +742,13 @@ impl BaudRate { } /// An inactive TWAI peripheral in the "Reset"/configuration state. -pub struct TwaiConfiguration<'d, T, DM: crate::Mode> { +pub struct TwaiConfiguration<'d, DM: crate::Mode, T> { peripheral: PhantomData<&'d PeripheralRef<'d, T>>, phantom: PhantomData, mode: TwaiMode, } -impl<'d, T, DM> TwaiConfiguration<'d, T, DM> +impl<'d, DM, T> TwaiConfiguration<'d, DM, T> where T: Instance, DM: crate::Mode, @@ -961,7 +961,7 @@ where /// 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> { + pub fn start(self) -> Twai<'d, DM, T> { Self::set_mode(self.mode); // Clear the TEC and REC @@ -1012,7 +1012,7 @@ where } } -impl<'d, T> TwaiConfiguration<'d, T, crate::Blocking> +impl<'d, T> TwaiConfiguration<'d, crate::Blocking, T> where T: Instance, { @@ -1045,9 +1045,9 @@ where } } -impl<'d, T> crate::private::Sealed for TwaiConfiguration<'d, T, crate::Blocking> where T: Instance {} +impl crate::private::Sealed for TwaiConfiguration<'_, crate::Blocking, T> where T: Instance {} -impl<'d, T> InterruptConfigurable for TwaiConfiguration<'d, T, crate::Blocking> +impl InterruptConfigurable for TwaiConfiguration<'_, crate::Blocking, T> where T: Instance, { @@ -1056,7 +1056,7 @@ where } } -impl<'d, T> TwaiConfiguration<'d, T, crate::Async> +impl<'d, T> TwaiConfiguration<'d, crate::Async, T> where T: Instance, { @@ -1097,20 +1097,20 @@ where /// /// In this mode, the TWAI controller can transmit and receive messages /// including error signals (such as error and overload frames). -pub struct Twai<'d, T, DM: crate::Mode> { - tx: TwaiTx<'d, T, DM>, - rx: TwaiRx<'d, T, DM>, +pub struct Twai<'d, DM: crate::Mode, T> { + tx: TwaiTx<'d, DM, T>, + rx: TwaiRx<'d, DM, T>, phantom: PhantomData, } -impl<'d, T, DM> Twai<'d, T, DM> +impl<'d, T, DM> Twai<'d, DM, T> where T: Instance, DM: crate::Mode, { /// Stop the peripheral, putting it into reset mode and enabling /// reconfiguration. - pub fn stop(self) -> TwaiConfiguration<'d, T, DM> { + pub fn stop(self) -> TwaiConfiguration<'d, DM, T> { // Put the peripheral into reset/configuration mode by setting the reset mode // bit. T::register_block() @@ -1120,7 +1120,7 @@ where TwaiConfiguration { peripheral: PhantomData, phantom: PhantomData, - mode: TwaiConfiguration::::mode(), + mode: TwaiConfiguration::::mode(), } } @@ -1181,18 +1181,18 @@ where /// Consumes this `Twai` instance and splits it into transmitting and /// receiving halves. - pub fn split(self) -> (TwaiRx<'d, T, DM>, TwaiTx<'d, T, DM>) { + pub fn split(self) -> (TwaiRx<'d, DM, T>, TwaiTx<'d, DM, T>) { (self.rx, self.tx) } } /// Interface to the TWAI transmitter part. -pub struct TwaiTx<'d, T, DM: crate::Mode> { +pub struct TwaiTx<'d, DM: crate::Mode, T> { _peripheral: PhantomData<&'d T>, phantom: PhantomData, } -impl<'d, T, DM> TwaiTx<'d, T, DM> +impl TwaiTx<'_, DM, T> where T: Instance, DM: crate::Mode, @@ -1229,12 +1229,12 @@ where } /// Interface to the TWAI receiver part. -pub struct TwaiRx<'d, T, DM: crate::Mode> { +pub struct TwaiRx<'d, DM: crate::Mode, T> { _peripheral: PhantomData<&'d T>, phantom: PhantomData, } -impl<'d, T, DM> TwaiRx<'d, T, DM> +impl TwaiRx<'_, DM, T> where T: Instance, DM: crate::Mode, @@ -1331,7 +1331,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> +impl<'d, DM, T> embedded_hal_02::can::Can for Twai<'d, DM, T> where T: Instance, DM: crate::Mode, @@ -1355,7 +1355,7 @@ where } } -impl<'d, T, DM> embedded_can::nb::Can for Twai<'d, T, DM> +impl<'d, DM, T> embedded_can::nb::Can for Twai<'d, DM, T> where T: Instance, DM: crate::Mode, @@ -1695,7 +1695,7 @@ mod asynch { pub(crate) static TWAI_STATE: [TwaiAsyncState; NUM_TWAI] = [const { TwaiAsyncState::new() }; NUM_TWAI]; - impl Twai<'_, T, crate::Async> + impl Twai<'_, crate::Async, T> where T: Instance, { @@ -1709,7 +1709,7 @@ mod asynch { } } - impl<'d, T> TwaiTx<'d, T, crate::Async> + impl TwaiTx<'_, crate::Async, T> where T: Instance, { @@ -1739,7 +1739,7 @@ mod asynch { } } - impl<'d, T> TwaiRx<'d, T, crate::Async> + impl TwaiRx<'_, crate::Async, T> where T: Instance, { diff --git a/examples/src/bin/embassy_twai.rs b/examples/src/bin/embassy_twai.rs index 73983c5e3ef..de2a798ab7e 100644 --- a/examples/src/bin/embassy_twai.rs +++ b/examples/src/bin/embassy_twai.rs @@ -48,7 +48,7 @@ type TwaiOutbox = Channel; #[embassy_executor::task] async fn receiver( - mut rx: TwaiRx<'static, TWAI0, esp_hal::Async>, + mut rx: TwaiRx<'static, esp_hal::Async, TWAI0>, channel: &'static TwaiOutbox, ) -> ! { loop { diff --git a/hil-test/tests/twai.rs b/hil-test/tests/twai.rs index ba258c81e6a..ac30b564b8f 100644 --- a/hil-test/tests/twai.rs +++ b/hil-test/tests/twai.rs @@ -17,7 +17,7 @@ use hil_test as _; use nb::block; struct Context { - twai: twai::Twai<'static, TWAI0, Blocking>, + twai: twai::Twai<'static, Blocking, TWAI0>, } #[cfg(test)] From 4f647231629bae1a6e1f5c64e0c96a4a6979a89e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 16 Oct 2024 16:42:33 +0200 Subject: [PATCH 2/6] Don't refer to Instance in irq handler --- esp-hal/src/twai/mod.rs | 133 ++++++++++++++++++++-------------------- 1 file changed, 65 insertions(+), 68 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 3787523f67e..72de1ce77fc 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -1165,7 +1165,7 @@ where /// error states. pub fn clear_receive_fifo(&self) { while self.num_available_messages() > 0 { - T::release_receive_fifo(); + release_receive_fifo(T::register_block()); } } @@ -1261,7 +1261,7 @@ where ))); } - Ok(T::read_frame()?) + Ok(read_frame(register_block)?) } } @@ -1413,15 +1413,6 @@ pub trait Instance: crate::private::Sealed { &asynch::TWAI_STATE[Self::NUMBER] } - /// Release the message in the buffer. This will decrement the received - /// message counter and prepare the next message in the FIFO for - /// reading. - fn release_receive_fifo() { - Self::register_block() - .cmd() - .write(|w| w.release_buf().set_bit()); - } - /// Write a frame to the peripheral. fn write_frame(frame: &EspTwaiFrame) { // Assemble the frame information into the data_0 byte. @@ -1494,67 +1485,72 @@ pub trait Instance: crate::private::Sealed { register_block.cmd().write(|w| w.tx_req().set_bit()); } } +} - /// Read a frame from the peripheral. - fn read_frame() -> Result { - let register_block = Self::register_block(); +/// Read a frame from the peripheral. +fn read_frame(register_block: &RegisterBlock) -> Result { + // Read the frame information and extract the frame id format and dlc. + let data_0 = register_block.data_0().read().tx_byte_0().bits(); - // Read the frame information and extract the frame id format and dlc. - let data_0 = register_block.data_0().read().tx_byte_0().bits(); + let is_standard_format = data_0 & 0b1 << 7 == 0; + let is_data_frame = data_0 & 0b1 << 6 == 0; + let self_reception = data_0 & 0b1 << 4 != 0; + let dlc = data_0 & 0b1111; - let is_standard_format = data_0 & 0b1 << 7 == 0; - let is_data_frame = data_0 & 0b1 << 6 == 0; - let self_reception = data_0 & 0b1 << 4 != 0; - let dlc = data_0 & 0b1111; + if dlc > 8 { + // Release the packet we read from the FIFO, allowing the peripheral to prepare + // the next packet. + release_receive_fifo(register_block); - if dlc > 8 { - // Release the packet we read from the FIFO, allowing the peripheral to prepare - // the next packet. - Self::release_receive_fifo(); + return Err(EspTwaiError::NonCompliantDlc(dlc)); + } + let dlc = dlc as usize; - return Err(EspTwaiError::NonCompliantDlc(dlc)); - } - let dlc = dlc as usize; + // Read the payload from the packet and construct a frame. + 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(); - // Read the payload from the packet and construct a frame. - 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 raw_id: u16 = ((data_1 as u16) << 3) | ((data_2 as u16) >> 5); + 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 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 - | (data_2 as u32) << 13 - | (data_3 as u32) << 5 - | (data_4 as u32) >> 3; - - let id = Id::from(ExtendedId::new(raw_id).unwrap()); - (id, register_block.data_5().as_ptr()) - }; + let raw_id: u32 = (data_1 as u32) << 21 + | (data_2 as u32) << 13 + | (data_3 as u32) << 5 + | (data_4 as u32) >> 3; - let mut frame = if is_data_frame { - unsafe { EspTwaiFrame::new_from_data_registers(id, data_ptr, dlc) } - } else { - EspTwaiFrame::new_remote(id, dlc).unwrap() - }; - frame.self_reception = self_reception; + let id = Id::from(ExtendedId::new(raw_id).unwrap()); + (id, register_block.data_5().as_ptr()) + }; - // Release the packet we read from the FIFO, allowing the peripheral to prepare - // the next packet. - Self::release_receive_fifo(); + let mut frame = if is_data_frame { + unsafe { EspTwaiFrame::new_from_data_registers(id, data_ptr, dlc) } + } else { + EspTwaiFrame::new_remote(id, dlc).unwrap() + }; + frame.self_reception = self_reception; - Ok(frame) - } + // Release the packet we read from the FIFO, allowing the peripheral to prepare + // the next packet. + release_receive_fifo(register_block); + + Ok(frame) +} + +/// Release the message in the buffer. This will decrement the received +/// message counter and prepare the next message in the FIFO for +/// reading. +fn release_receive_fifo(register_block: &RegisterBlock) { + register_block.cmd().write(|w| w.release_buf().set_bit()); } impl Instance for crate::peripherals::TWAI0 { @@ -1772,8 +1768,7 @@ mod asynch { } } - fn handle_interrupt() { - let register_block = T::register_block(); + fn handle_interrupt(register_block: &RegisterBlock, async_state: &TwaiAsyncState) { cfg_if::cfg_if! { if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { let intr_enable = register_block.int_ena().read(); @@ -1794,8 +1789,6 @@ mod asynch { } } - let async_state = T::async_state(); - if tx_int_status.bit_is_set() { async_state.tx_waker.wake(); } @@ -1813,7 +1806,7 @@ mod asynch { let _ = rx_queue.try_send(Err(EspTwaiError::EmbeddedHAL(ErrorKind::Overrun))); } - match T::read_frame() { + match read_frame(register_block) { Ok(frame) => { let _ = rx_queue.try_send(Ok(frame)); } @@ -1833,12 +1826,16 @@ mod asynch { #[handler] pub(super) fn twai0() { - handle_interrupt::(); + let register_block = TWAI0::register_block(); + let async_state = TWAI0::async_state(); + handle_interrupt(register_block, async_state); } #[cfg(twai1)] #[handler] pub(super) fn twai1() { - handle_interrupt::(); + let register_block = TWAI1::register_block(); + let async_state = TWAI1::async_state(); + handle_interrupt(register_block, async_state); } } From 40675dc3cbe41608b0f65bb419d3691fc5677d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 16 Oct 2024 16:49:30 +0200 Subject: [PATCH 3/6] Deduplicate a bit --- esp-hal/src/soc/esp32/peripherals.rs | 2 +- esp-hal/src/soc/esp32c3/peripherals.rs | 2 +- esp-hal/src/soc/esp32c6/peripherals.rs | 4 +- esp-hal/src/soc/esp32h2/peripherals.rs | 2 +- esp-hal/src/soc/esp32s2/peripherals.rs | 2 +- esp-hal/src/soc/esp32s3/peripherals.rs | 2 +- esp-hal/src/twai/mod.rs | 98 ++++++++------------------ 7 files changed, 37 insertions(+), 75 deletions(-) diff --git a/esp-hal/src/soc/esp32/peripherals.rs b/esp-hal/src/soc/esp32/peripherals.rs index 6216b55bfac..e4aff4ea62e 100644 --- a/esp-hal/src/soc/esp32/peripherals.rs +++ b/esp-hal/src/soc/esp32/peripherals.rs @@ -67,7 +67,7 @@ crate::peripherals! { TIMG0 <= TIMG0, TIMG1 <= TIMG1, TOUCH <= virtual, - TWAI0 <= TWAI0, + [Twai0] TWAI0 <= TWAI0, UART0 <= UART0, UART1 <= UART1, UART2 <= UART2, diff --git a/esp-hal/src/soc/esp32c3/peripherals.rs b/esp-hal/src/soc/esp32c3/peripherals.rs index 5f5487711f1..1e5a345f273 100644 --- a/esp-hal/src/soc/esp32c3/peripherals.rs +++ b/esp-hal/src/soc/esp32c3/peripherals.rs @@ -53,7 +53,7 @@ crate::peripherals! { SW_INTERRUPT <= virtual, TIMG0 <= TIMG0, TIMG1 <= TIMG1, - TWAI0 <= TWAI0, + [Twai0] TWAI0 <= TWAI0, UART0 <= UART0, UART1 <= UART1, UHCI0 <= UHCI0, diff --git a/esp-hal/src/soc/esp32c6/peripherals.rs b/esp-hal/src/soc/esp32c6/peripherals.rs index dd44390e08c..babe3dc752c 100644 --- a/esp-hal/src/soc/esp32c6/peripherals.rs +++ b/esp-hal/src/soc/esp32c6/peripherals.rs @@ -81,8 +81,8 @@ crate::peripherals! { TIMG0 <= TIMG0, TIMG1 <= TIMG1, TRACE0 <= TRACE, - TWAI0 <= TWAI0, - TWAI1 <= TWAI1, + [Twai0] TWAI0 <= TWAI0, + [Twai1] TWAI1 <= TWAI1, UART0 <= UART0, UART1 <= UART1, UHCI0 <= UHCI0, diff --git a/esp-hal/src/soc/esp32h2/peripherals.rs b/esp-hal/src/soc/esp32h2/peripherals.rs index 52a8aac084c..81b1385c835 100644 --- a/esp-hal/src/soc/esp32h2/peripherals.rs +++ b/esp-hal/src/soc/esp32h2/peripherals.rs @@ -73,7 +73,7 @@ crate::peripherals! { TIMG0 <= TIMG0, TIMG1 <= TIMG1, TRACE0 <= TRACE, - TWAI0 <= TWAI0, + [Twai0] TWAI0 <= TWAI0, UART0 <= UART0, UART1 <= UART1, UHCI0 <= UHCI0, diff --git a/esp-hal/src/soc/esp32s2/peripherals.rs b/esp-hal/src/soc/esp32s2/peripherals.rs index c1a105da17d..d53431031f3 100644 --- a/esp-hal/src/soc/esp32s2/peripherals.rs +++ b/esp-hal/src/soc/esp32s2/peripherals.rs @@ -60,7 +60,7 @@ crate::peripherals! { SW_INTERRUPT <= virtual, TIMG0 <= TIMG0, TIMG1 <= TIMG1, - TWAI0 <= TWAI0, + [Twai0] TWAI0 <= TWAI0, UART0 <= UART0, UART1 <= UART1, UHCI0 <= UHCI0, diff --git a/esp-hal/src/soc/esp32s3/peripherals.rs b/esp-hal/src/soc/esp32s3/peripherals.rs index 9a1a0a0c2c0..5e8f04f35e4 100644 --- a/esp-hal/src/soc/esp32s3/peripherals.rs +++ b/esp-hal/src/soc/esp32s3/peripherals.rs @@ -66,7 +66,7 @@ crate::peripherals! { SW_INTERRUPT <= virtual, TIMG0 <= TIMG0, TIMG1 <= TIMG1, - TWAI0 <= TWAI0, + [Twai0] TWAI0 <= TWAI0, UART0 <= UART0, UART1 <= UART1, UART2 <= UART2, diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 72de1ce77fc..04de074e6dd 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -129,11 +129,12 @@ use core::marker::PhantomData; use self::filter::{Filter, FilterType}; use crate::{ + dma::PeripheralMarker, gpio::{InputSignal, OutputSignal, PeripheralInput, PeripheralOutput, Pull}, interrupt::InterruptHandler, peripheral::{Peripheral, PeripheralRef}, peripherals::twai0::RegisterBlock, - system::{self, PeripheralClockControl}, + system::PeripheralClockControl, InterruptConfigurable, }; @@ -754,7 +755,7 @@ where DM: crate::Mode, { fn new_internal( - _peripheral: impl Peripheral

+ 'd, + peripheral: impl Peripheral

+ 'd, rx_pin: impl Peripheral

+ 'd, tx_pin: impl Peripheral

+ 'd, baud_rate: BaudRate, @@ -762,11 +763,11 @@ where mode: TwaiMode, ) -> Self { // Set up the GPIO pins. - crate::into_ref!(tx_pin, rx_pin); + crate::into_ref!(peripheral, tx_pin, rx_pin); // Enable the peripheral clock for the TWAI peripheral. - T::enable_peripheral(); - T::reset_peripheral(); + PeripheralClockControl::enable(peripheral.peripheral()); + PeripheralClockControl::reset(peripheral.peripheral()); // Set RESET bit to 1 T::register_block() @@ -1380,9 +1381,7 @@ where } /// TWAI peripheral instance. -pub trait Instance: crate::private::Sealed { - /// The system peripheral associated with this TWAI instance. - const SYSTEM_PERIPHERAL: system::Peripheral; +pub trait Instance: crate::private::Sealed + PeripheralMarker { /// The identifier number for this TWAI instance. const NUMBER: usize; @@ -1399,14 +1398,30 @@ pub trait Instance: crate::private::Sealed { /// Returns a reference to the register block for TWAI instance. fn register_block() -> &'static RegisterBlock; - /// Enables the TWAI peripheral. - fn enable_peripheral(); - - /// Resets the TWAI peripheral. - fn reset_peripheral(); - /// Enables interrupts for the TWAI peripheral. - fn enable_interrupts(); + fn enable_interrupts() { + let register_block = Self::register_block(); + + 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() + }); + } + } + } /// Returns a reference to the asynchronous state for this TWAI instance. fn async_state() -> &'static asynch::TwaiAsyncState { @@ -1554,7 +1569,6 @@ fn release_receive_fifo(register_block: &RegisterBlock) { } impl Instance for crate::peripherals::TWAI0 { - const SYSTEM_PERIPHERAL: system::Peripheral = system::Peripheral::Twai0; const NUMBER: usize = 0; cfg_if::cfg_if! { @@ -1577,43 +1591,10 @@ impl Instance for crate::peripherals::TWAI0 { fn register_block() -> &'static RegisterBlock { unsafe { &*crate::peripherals::TWAI0::PTR } } - - fn reset_peripheral() { - PeripheralClockControl::reset(crate::system::Peripheral::Twai0); - } - - fn enable_peripheral() { - PeripheralClockControl::enable(crate::system::Peripheral::Twai0); - } - - fn enable_interrupts() { - let register_block = Self::register_block(); - - 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(twai1)] impl Instance for crate::peripherals::TWAI1 { - const SYSTEM_PERIPHERAL: system::Peripheral = system::Peripheral::Twai1; const NUMBER: usize = 1; const INPUT_SIGNAL: InputSignal = InputSignal::TWAI1_RX; @@ -1629,25 +1610,6 @@ impl Instance for crate::peripherals::TWAI1 { fn register_block() -> &'static RegisterBlock { unsafe { &*crate::peripherals::TWAI1::PTR } } - - fn reset_peripheral() { - PeripheralClockControl::enable(crate::system::Peripheral::Twai1); - } - - fn enable_peripheral() { - PeripheralClockControl::enable(crate::system::Peripheral::Twai1); - } - - fn enable_interrupts() { - let register_block = Self::register_block(); - 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() - }); - } } mod asynch { From e6b86a32715e192d036d1f881acd799aaf82b1f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 16 Oct 2024 17:07:45 +0200 Subject: [PATCH 4/6] Take &self --- esp-hal/src/twai/mod.rs | 449 +++++++++++++++++++++++----------------- 1 file changed, 256 insertions(+), 193 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 04de074e6dd..59629db6d9f 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -744,7 +744,7 @@ impl BaudRate { /// An inactive TWAI peripheral in the "Reset"/configuration state. pub struct TwaiConfiguration<'d, DM: crate::Mode, T> { - peripheral: PhantomData<&'d PeripheralRef<'d, T>>, + twai: PeripheralRef<'d, T>, phantom: PhantomData, mode: TwaiMode, } @@ -755,7 +755,7 @@ where DM: crate::Mode, { fn new_internal( - peripheral: impl Peripheral

+ 'd, + twai: impl Peripheral

+ 'd, rx_pin: impl Peripheral

+ 'd, tx_pin: impl Peripheral

+ 'd, baud_rate: BaudRate, @@ -763,21 +763,29 @@ where mode: TwaiMode, ) -> Self { // Set up the GPIO pins. - crate::into_ref!(peripheral, tx_pin, rx_pin); + crate::into_ref!(twai, tx_pin, rx_pin); // Enable the peripheral clock for the TWAI peripheral. - PeripheralClockControl::enable(peripheral.peripheral()); - PeripheralClockControl::reset(peripheral.peripheral()); + PeripheralClockControl::enable(twai.peripheral()); + PeripheralClockControl::reset(twai.peripheral()); + + let mut this = TwaiConfiguration { + twai, + phantom: PhantomData, + mode, + }; // Set RESET bit to 1 - T::register_block() + this.twai + .register_block() .mode() .write(|w| w.reset_mode().set_bit()); #[cfg(esp32)] { // Enable extended register layout - T::register_block() + this.twai + .register_block() .clock_divider() .modify(|_, w| w.ext_mode().set_bit()); } @@ -790,46 +798,43 @@ where tx_pin.set_to_push_pull_output(crate::private::Internal); Pull::None }; - tx_pin.connect_peripheral_to_output(T::OUTPUT_SIGNAL, crate::private::Internal); + tx_pin.connect_peripheral_to_output(this.twai.output_signal(), crate::private::Internal); // 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(rx_pull, crate::private::Internal); - rx_pin.connect_input_to_peripheral(T::INPUT_SIGNAL, crate::private::Internal); + rx_pin.connect_input_to_peripheral(this.twai.input_signal(), crate::private::Internal); // Freeze REC by changing to LOM mode - Self::set_mode(TwaiMode::ListenOnly); + this.set_mode(TwaiMode::ListenOnly); // Set TEC to 0 - T::register_block() + this.twai + .register_block() .tx_err_cnt() .write(|w| unsafe { w.tx_err_cnt().bits(0) }); // Set REC to 0 - T::register_block() + this.twai + .register_block() .rx_err_cnt() .write(|w| unsafe { w.rx_err_cnt().bits(0) }); // Set EWL to 96 - T::register_block() + this.twai + .register_block() .err_warning_limit() .write(|w| unsafe { w.err_warning_limit().bits(96) }); - let mut cfg = TwaiConfiguration { - peripheral: PhantomData, - phantom: PhantomData, - mode, - }; - - cfg.set_baud_rate(baud_rate); - cfg + this.set_baud_rate(baud_rate); + this } fn internal_set_interrupt_handler(&mut self, handler: InterruptHandler) { unsafe { - crate::interrupt::bind_interrupt(T::INTERRUPT, handler.handler()); - crate::interrupt::enable(T::INTERRUPT, handler.priority()).unwrap(); + crate::interrupt::bind_interrupt(self.twai.interrupt(), handler.handler()); + crate::interrupt::enable(self.twai.interrupt(), handler.priority()).unwrap(); } } @@ -862,13 +867,15 @@ where // Enable /2 baudrate divider by setting `brp_div`. // `brp_div` is not an interrupt, it will prescale BRP by 2. Only available on // ESP32 Revision 2 or later. Reserved otherwise. - T::register_block() + self.twai + .register_block() .int_ena() .modify(|_, w| w.brp_div().set_bit()); prescaler = timing.baud_rate_prescaler / 2; } else { // Disable /2 baudrate divider by clearing brp_div. - T::register_block() + self.twai + .register_block() .int_ena() .modify(|_, w| w.brp_div().clear_bit()); } @@ -881,20 +888,27 @@ where let triple_sample = timing.triple_sample; // Set up the prescaler and sync jump width. - T::register_block().bus_timing_0().modify(|_, w| unsafe { - w.baud_presc().bits(prescale as _); - w.sync_jump_width().bits(sjw) - }); + self.twai + .register_block() + .bus_timing_0() + .modify(|_, w| unsafe { + w.baud_presc().bits(prescale as _); + w.sync_jump_width().bits(sjw) + }); // Set up the time segment 1, time segment 2, and triple sample. - T::register_block().bus_timing_1().modify(|_, w| unsafe { - w.time_seg1().bits(tseg_1); - w.time_seg2().bits(tseg_2); - w.time_samp().bit(triple_sample) - }); + self.twai + .register_block() + .bus_timing_1() + .modify(|_, w| unsafe { + w.time_seg1().bits(tseg_1); + w.time_seg2().bits(tseg_2); + w.time_samp().bit(triple_sample) + }); // disable CLKOUT - T::register_block() + self.twai + .register_block() .clock_divider() .modify(|_, w| w.clock_off().set_bit()); } @@ -913,7 +927,8 @@ where pub fn set_filter(&mut self, filter: impl Filter) { // Set or clear the rx filter mode bit depending on the filter type. let filter_mode_bit = filter.filter_type() == FilterType::Single; - T::register_block() + self.twai + .register_block() .mode() .modify(|_, w| w.rx_filter_mode().bit(filter_mode_bit)); @@ -923,7 +938,7 @@ where // Copy the filter to the peripheral. unsafe { - copy_to_data_register(T::register_block().data_0().as_ptr(), ®isters); + copy_to_data_register(self.twai.register_block().data_0().as_ptr(), ®isters); } } @@ -934,26 +949,15 @@ where /// warning interrupt will be triggered (given the enable signal is /// valid). pub fn set_error_warning_limit(&mut self, limit: u8) { - T::register_block() + self.twai + .register_block() .err_warning_limit() .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| { + fn set_mode(&self, mode: TwaiMode) { + self.twai.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) @@ -963,10 +967,11 @@ where /// Put the peripheral into Operation Mode, allowing the transmission and /// reception of packets using the new object. pub fn start(self) -> Twai<'d, DM, T> { - Self::set_mode(self.mode); + self.set_mode(self.mode); // Clear the TEC and REC - T::register_block() + self.twai + .register_block() .tx_err_cnt() .write(|w| unsafe { w.tx_err_cnt().bits(0) }); @@ -981,33 +986,36 @@ where } else { 0 }; - T::register_block() + self.twai + .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(); + let _ = self.twai.register_block().int_raw().read(); } else { - let _ = T::register_block().interrupt().read(); + let _ = self.twai.register_block().interrupt().read(); } } // Put the peripheral into operation mode by clearing the reset mode bit. - T::register_block() + self.twai + .register_block() .mode() .modify(|_, w| w.reset_mode().clear_bit()); Twai { rx: TwaiRx { - _peripheral: PhantomData, + twai: unsafe { self.twai.clone_unchecked() }, phantom: PhantomData, }, tx: TwaiTx { - _peripheral: PhantomData, + twai: unsafe { self.twai.clone_unchecked() }, phantom: PhantomData, }, + twai: unsafe { self.twai.clone_unchecked() }, phantom: PhantomData, } } @@ -1072,7 +1080,7 @@ where mode: TwaiMode, ) -> Self { let mut this = Self::new_internal(peripheral, rx_pin, tx_pin, baud_rate, false, mode); - this.internal_set_interrupt_handler(T::async_handler()); + this.internal_set_interrupt_handler(this.twai.async_handler()); this } @@ -1089,7 +1097,7 @@ where mode: TwaiMode, ) -> Self { let mut this = Self::new_internal(peripheral, rx_pin, tx_pin, baud_rate, true, mode); - this.internal_set_interrupt_handler(T::async_handler()); + this.internal_set_interrupt_handler(this.twai.async_handler()); this } } @@ -1099,6 +1107,7 @@ where /// In this mode, the TWAI controller can transmit and receive messages /// including error signals (such as error and overload frames). pub struct Twai<'d, DM: crate::Mode, T> { + twai: PeripheralRef<'d, T>, tx: TwaiTx<'d, DM, T>, rx: TwaiRx<'d, DM, T>, phantom: PhantomData, @@ -1109,35 +1118,61 @@ where T: Instance, DM: crate::Mode, { + fn mode(&self) -> TwaiMode { + let mode = self.twai.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 + } + } + /// Stop the peripheral, putting it into reset mode and enabling /// reconfiguration. pub fn stop(self) -> TwaiConfiguration<'d, DM, T> { // Put the peripheral into reset/configuration mode by setting the reset mode // bit. - T::register_block() + self.twai + .register_block() .mode() .modify(|_, w| w.reset_mode().set_bit()); + let mode = self.mode(); + TwaiConfiguration { - peripheral: PhantomData, + twai: self.twai, phantom: PhantomData, - mode: TwaiConfiguration::::mode(), + mode, } } /// Returns the value of the receive error counter. pub fn receive_error_count(&self) -> u8 { - T::register_block().rx_err_cnt().read().rx_err_cnt().bits() + self.twai + .register_block() + .rx_err_cnt() + .read() + .rx_err_cnt() + .bits() } /// Returns the value of the transmit error counter. pub fn transmit_error_count(&self) -> u8 { - T::register_block().tx_err_cnt().read().tx_err_cnt().bits() + self.twai + .register_block() + .tx_err_cnt() + .read() + .tx_err_cnt() + .bits() } /// Check if the controller is in a bus off state. pub fn is_bus_off(&self) -> bool { - T::register_block() + self.twai + .register_block() .status() .read() .bus_off_st() @@ -1150,7 +1185,8 @@ where /// Note that this may not be the number of valid messages in the receive /// FIFO due to fifo overflow/overrun. pub fn num_available_messages(&self) -> u8 { - T::register_block() + self.twai + .register_block() .rx_message_cnt() .read() .rx_message_counter() @@ -1166,7 +1202,7 @@ where /// error states. pub fn clear_receive_fifo(&self) { while self.num_available_messages() > 0 { - release_receive_fifo(T::register_block()); + release_receive_fifo(self.twai.register_block()); } } @@ -1189,7 +1225,7 @@ where /// Interface to the TWAI transmitter part. pub struct TwaiTx<'d, DM: crate::Mode, T> { - _peripheral: PhantomData<&'d T>, + twai: PeripheralRef<'d, T>, phantom: PhantomData, } @@ -1211,7 +1247,7 @@ where /// functionality. See notes 1 and 2 in the "Frame Identifier" section /// of the reference manual. pub fn transmit(&mut self, frame: &EspTwaiFrame) -> nb::Result<(), EspTwaiError> { - let register_block = T::register_block(); + let register_block = self.twai.register_block(); let status = register_block.status().read(); // Check that the peripheral is not in a bus off state. @@ -1223,7 +1259,7 @@ where return nb::Result::Err(nb::Error::WouldBlock); } - T::write_frame(frame); + write_frame(register_block, frame); Ok(()) } @@ -1231,7 +1267,7 @@ where /// Interface to the TWAI receiver part. pub struct TwaiRx<'d, DM: crate::Mode, T> { - _peripheral: PhantomData<&'d T>, + twai: PeripheralRef<'d, T>, phantom: PhantomData, } @@ -1242,7 +1278,7 @@ where { /// Receive a frame pub fn receive(&mut self) -> nb::Result { - let register_block = T::register_block(); + let register_block = self.twai.register_block(); let status = register_block.status().read(); // Check that the peripheral is not in a bus off state. @@ -1381,26 +1417,26 @@ where } /// TWAI peripheral instance. -pub trait Instance: crate::private::Sealed + PeripheralMarker { +pub trait Instance: Peripheral

+ PeripheralMarker + 'static { /// The identifier number for this TWAI instance. - const NUMBER: usize; + fn number(&self) -> usize; /// Input signal. - const INPUT_SIGNAL: InputSignal; + fn input_signal(&self) -> InputSignal; /// Output signal. - const OUTPUT_SIGNAL: OutputSignal; + fn output_signal(&self) -> OutputSignal; /// The interrupt associated with this TWAI instance. - const INTERRUPT: crate::peripherals::Interrupt; + fn interrupt(&self) -> crate::peripherals::Interrupt; /// Provides an asynchronous interrupt handler for TWAI instance. - fn async_handler() -> InterruptHandler; + fn async_handler(&self) -> InterruptHandler; /// Returns a reference to the register block for TWAI instance. - fn register_block() -> &'static RegisterBlock; + fn register_block(&self) -> &RegisterBlock; /// Enables interrupts for the TWAI peripheral. - fn enable_interrupts() { - let register_block = Self::register_block(); + fn enable_interrupts(&self) { + let register_block = self.register_block(); cfg_if::cfg_if! { if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { @@ -1424,82 +1460,7 @@ pub trait Instance: crate::private::Sealed + PeripheralMarker { } /// Returns a reference to the asynchronous state for this TWAI instance. - fn async_state() -> &'static asynch::TwaiAsyncState { - &asynch::TWAI_STATE[Self::NUMBER] - } - - /// Write a frame to the peripheral. - 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 | self_reception << 4 | dlc_bits; - - let register_block = Self::register_block(); - - register_block - .data_0() - .write(|w| unsafe { w.tx_byte_0().bits(data_0) }); - - // 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(); - - register_block - .data_1() - .write(|w| unsafe { w.tx_byte_1().bits((id >> 3) as u8) }); - - 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(); - - register_block - .data_1() - .write(|w| unsafe { w.tx_byte_1().bits((id >> 21) as u8) }); - register_block - .data_2() - .write(|w| unsafe { w.tx_byte_2().bits((id >> 13) as u8) }); - register_block - .data_3() - .write(|w| unsafe { w.tx_byte_3().bits((id >> 5) as u8) }); - 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. - 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], - }, - ) - } - - // Trigger the appropriate transmission request based on self_reception flag - if frame.self_reception { - register_block.cmd().write(|w| w.self_rx_req().set_bit()); - } else { - // Set the transmit request command, this will lock the transmit buffer until - // the transmission is complete or aborted. - register_block.cmd().write(|w| w.tx_req().set_bit()); - } - } + fn async_state(&self) -> &asynch::TwaiAsyncState; } /// Read a frame from the peripheral. @@ -1568,48 +1529,152 @@ fn release_receive_fifo(register_block: &RegisterBlock) { register_block.cmd().write(|w| w.release_buf().set_bit()); } +/// Write a frame to the peripheral. +fn write_frame(register_block: &RegisterBlock, 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 | self_reception << 4 | dlc_bits; + + register_block + .data_0() + .write(|w| unsafe { w.tx_byte_0().bits(data_0) }); + + // 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(); + + register_block + .data_1() + .write(|w| unsafe { w.tx_byte_1().bits((id >> 3) as u8) }); + + 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(); + + register_block + .data_1() + .write(|w| unsafe { w.tx_byte_1().bits((id >> 21) as u8) }); + register_block + .data_2() + .write(|w| unsafe { w.tx_byte_2().bits((id >> 13) as u8) }); + register_block + .data_3() + .write(|w| unsafe { w.tx_byte_3().bits((id >> 5) as u8) }); + 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. + 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], + }, + ) + } + + // Trigger the appropriate transmission request based on self_reception flag + if frame.self_reception { + register_block.cmd().write(|w| w.self_rx_req().set_bit()); + } else { + // Set the transmit request command, this will lock the transmit buffer until + // the transmission is complete or aborted. + register_block.cmd().write(|w| w.tx_req().set_bit()); + } +} + impl Instance for crate::peripherals::TWAI0 { - const NUMBER: usize = 0; + fn number(&self) -> usize { + 0 + } - 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; + fn input_signal(&self) -> InputSignal { + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { + InputSignal::TWAI_RX + } else { + InputSignal::TWAI0_RX + } } } - const INTERRUPT: crate::peripherals::Interrupt = crate::peripherals::Interrupt::TWAI0; + fn output_signal(&self) -> OutputSignal { + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32c3, esp32s2, esp32s3))] { + OutputSignal::TWAI_TX + } else { + OutputSignal::TWAI0_TX + } + } + } - fn async_handler() -> InterruptHandler { + fn interrupt(&self) -> crate::peripherals::Interrupt { + crate::peripherals::Interrupt::TWAI0 + } + + fn async_handler(&self) -> InterruptHandler { asynch::twai0 } #[inline(always)] - fn register_block() -> &'static RegisterBlock { + fn register_block(&self) -> &RegisterBlock { unsafe { &*crate::peripherals::TWAI0::PTR } } + + fn async_state(&self) -> &asynch::TwaiAsyncState { + static STATE: asynch::TwaiAsyncState = asynch::TwaiAsyncState::new(); + &STATE + } } #[cfg(twai1)] impl Instance for crate::peripherals::TWAI1 { - const NUMBER: usize = 1; + fn number(&self) -> usize { + 1 + } + + fn input_signal(&self) -> InputSignal { + InputSignal::TWAI1_RX + } - const INPUT_SIGNAL: InputSignal = InputSignal::TWAI1_RX; - const OUTPUT_SIGNAL: OutputSignal = OutputSignal::TWAI1_TX; + fn output_signal(&self) -> OutputSignal { + OutputSignal::TWAI1_TX + } - const INTERRUPT: crate::peripherals::Interrupt = crate::peripherals::Interrupt::TWAI1; + fn interrupt(&self) -> crate::peripherals::Interrupt { + crate::peripherals::Interrupt::TWAI1 + } - fn async_handler() -> InterruptHandler { + fn async_handler(&self) -> InterruptHandler { asynch::twai1 } #[inline(always)] - fn register_block() -> &'static RegisterBlock { + fn register_block(&self) -> &RegisterBlock { unsafe { &*crate::peripherals::TWAI1::PTR } } + + fn async_state(&self) -> &asynch::TwaiAsyncState { + static STATE: asynch::TwaiAsyncState = asynch::TwaiAsyncState::new(); + &STATE + } } mod asynch { @@ -1649,10 +1714,6 @@ mod asynch { } } - const NUM_TWAI: usize = 1 + cfg!(twai1) as usize; - pub(crate) static TWAI_STATE: [TwaiAsyncState; NUM_TWAI] = - [const { TwaiAsyncState::new() }; NUM_TWAI]; - impl Twai<'_, crate::Async, T> where T: Instance, @@ -1673,11 +1734,11 @@ mod asynch { { /// Transmits an `EspTwaiFrame` asynchronously over the TWAI bus. pub async fn transmit_async(&mut self, frame: &EspTwaiFrame) -> Result<(), EspTwaiError> { - T::enable_interrupts(); + self.twai.enable_interrupts(); poll_fn(|cx| { - T::async_state().tx_waker.register(cx.waker()); + self.twai.async_state().tx_waker.register(cx.waker()); - let register_block = T::register_block(); + let register_block = self.twai.register_block(); let status = register_block.status().read(); // Check that the peripheral is not in a bus off state. @@ -1689,7 +1750,7 @@ mod asynch { return Poll::Pending; } - T::write_frame(frame); + write_frame(register_block, frame); Poll::Ready(Ok(())) }) @@ -1703,14 +1764,14 @@ mod asynch { { /// Receives an `EspTwaiFrame` asynchronously over the TWAI bus. pub async fn receive_async(&mut self) -> Result { - T::enable_interrupts(); + self.twai.enable_interrupts(); poll_fn(|cx| { - T::async_state().err_waker.register(cx.waker()); + self.twai.async_state().err_waker.register(cx.waker()); - if let Poll::Ready(result) = T::async_state().rx_queue.poll_receive(cx) { + if let Poll::Ready(result) = self.twai.async_state().rx_queue.poll_receive(cx) { Poll::Ready(result) } else { - let register_block = T::register_block(); + let register_block = self.twai.register_block(); let status = register_block.status().read(); // Check that the peripheral is not in a bus off state. @@ -1788,16 +1849,18 @@ mod asynch { #[handler] pub(super) fn twai0() { - let register_block = TWAI0::register_block(); - let async_state = TWAI0::async_state(); + let twai = unsafe { TWAI0::steal() }; + let register_block = twai.register_block(); + let async_state = twai.async_state(); handle_interrupt(register_block, async_state); } #[cfg(twai1)] #[handler] pub(super) fn twai1() { - let register_block = TWAI1::register_block(); - let async_state = TWAI1::async_state(); + let twai = unsafe { TWAI1::steal() }; + let register_block = twai.register_block(); + let async_state = twai.async_state(); handle_interrupt(register_block, async_state); } } From ef1307033ee8b8101169170ff3ef437496b9563f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 16 Oct 2024 17:08:21 +0200 Subject: [PATCH 5/6] Reduce nesting --- esp-hal/src/twai/mod.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index 59629db6d9f..a323dc599fd 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -1769,23 +1769,23 @@ mod asynch { self.twai.async_state().err_waker.register(cx.waker()); if let Poll::Ready(result) = self.twai.async_state().rx_queue.poll_receive(cx) { - Poll::Ready(result) - } else { - let register_block = self.twai.register_block(); - let status = register_block.status().read(); - - // Check that the peripheral is not in a bus off state. - if status.bus_off_st().bit_is_set() { - return Poll::Ready(Err(EspTwaiError::BusOff)); - } - - // Check if the packet in the receive buffer is valid or overrun. - if status.miss_st().bit_is_set() { - return Poll::Ready(Err(EspTwaiError::EmbeddedHAL(ErrorKind::Overrun))); - } - - Poll::Pending + return Poll::Ready(result); } + + let register_block = self.twai.register_block(); + let status = register_block.status().read(); + + // Check that the peripheral is not in a bus off state. + if status.bus_off_st().bit_is_set() { + return Poll::Ready(Err(EspTwaiError::BusOff)); + } + + // Check if the packet in the receive buffer is valid or overrun. + if status.miss_st().bit_is_set() { + return Poll::Ready(Err(EspTwaiError::EmbeddedHAL(ErrorKind::Overrun))); + } + + Poll::Pending }) .await } From 0c61001a756f858cae9d1da1a02c065baa2b4452 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 23 Oct 2024 15:46:42 +0200 Subject: [PATCH 6/6] Erase TWAI instance --- esp-hal/CHANGELOG.md | 2 + esp-hal/MIGRATING-0.21.md | 1 + esp-hal/src/twai/mod.rs | 147 ++++++++++++++++++++++++++----- examples/src/bin/embassy_twai.rs | 18 +--- hil-test/tests/twai.rs | 3 +- 5 files changed, 129 insertions(+), 42 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 149b466d88b..abf3eb35f65 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `AnyPin` now implements `From>`. (#2326) - Added `AnySpi` and `AnySpiDmaChannel`. (#2334) - Added `AnyI2s` and `AnyI2sDmaChannel`. (#2367) +- Added `AnyTwai`. (#2359) - `Pins::steal()` to unsafely obtain GPIO. (#2335) - `I2c::with_timeout` (#2361) @@ -23,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Peripheral type erasure for SPI (#2334) - Peripheral type erasure for I2S (#2367) - Peripheral type erasure for I2C (#2361) +- Peripheral type erasure for TWAI (#2359) ### Fixed diff --git a/esp-hal/MIGRATING-0.21.md b/esp-hal/MIGRATING-0.21.md index b26ed414e06..ec24809372c 100644 --- a/esp-hal/MIGRATING-0.21.md +++ b/esp-hal/MIGRATING-0.21.md @@ -31,6 +31,7 @@ peripherals: - SPI (both master and slave) - I2S - I2C +- TWAI ```diff -Spi<'static, SPI2, FullDuplexMode> diff --git a/esp-hal/src/twai/mod.rs b/esp-hal/src/twai/mod.rs index a323dc599fd..69114044ec5 100644 --- a/esp-hal/src/twai/mod.rs +++ b/esp-hal/src/twai/mod.rs @@ -135,6 +135,8 @@ use crate::{ peripheral::{Peripheral, PeripheralRef}, peripherals::twai0::RegisterBlock, system::PeripheralClockControl, + Async, + Blocking, InterruptConfigurable, }; @@ -743,7 +745,7 @@ impl BaudRate { } /// An inactive TWAI peripheral in the "Reset"/configuration state. -pub struct TwaiConfiguration<'d, DM: crate::Mode, T> { +pub struct TwaiConfiguration<'d, DM: crate::Mode, T = AnyTwai> { twai: PeripheralRef<'d, T>, phantom: PhantomData, mode: TwaiMode, @@ -1019,16 +1021,57 @@ where phantom: PhantomData, } } + + /// Convert the configuration into an async configuration. + fn into_async(self) -> TwaiConfiguration<'d, Async, T> { + let mut this = TwaiConfiguration { + twai: self.twai, + phantom: PhantomData, + mode: self.mode, + }; + this.internal_set_interrupt_handler(this.twai.async_handler()); + this + } +} + +impl<'d> TwaiConfiguration<'d, Blocking> { + /// Create a new instance of [TwaiConfiguration] + /// + /// You will need to use a transceiver to connect to the TWAI bus + pub fn new( + peripheral: impl Peripheral

+ 'd, + rx_pin: impl Peripheral

+ 'd, + tx_pin: impl Peripheral

+ 'd, + baud_rate: BaudRate, + mode: TwaiMode, + ) -> Self { + Self::new_typed(peripheral.map_into(), rx_pin, tx_pin, baud_rate, mode) + } + + /// Create a new instance of [TwaiConfiguration] meant to connect two ESP32s + /// directly + /// + /// You don't need a transceiver by following the description in the + /// `twai.rs` example + pub fn new_no_transceiver( + peripheral: impl Peripheral

+ 'd, + rx_pin: impl Peripheral

+ 'd, + tx_pin: impl Peripheral

+ 'd, + baud_rate: BaudRate, + mode: TwaiMode, + ) -> Self { + Self::new_no_transceiver_typed(peripheral.map_into(), rx_pin, tx_pin, baud_rate, mode) + } } -impl<'d, T> TwaiConfiguration<'d, crate::Blocking, T> +impl<'d, T> TwaiConfiguration<'d, Blocking, T> where T: Instance, { /// Create a new instance of [TwaiConfiguration] /// /// You will need to use a transceiver to connect to the TWAI bus - pub fn new( + pub fn new_typed( peripheral: impl Peripheral

+ 'd, rx_pin: impl Peripheral

+ 'd, tx_pin: impl Peripheral

+ 'd, @@ -1043,7 +1086,7 @@ where /// /// You don't need a transceiver by following the description in the /// `twai.rs` example - pub fn new_no_transceiver( + pub fn new_no_transceiver_typed( peripheral: impl Peripheral

+ 'd, rx_pin: impl Peripheral

+ 'd, tx_pin: impl Peripheral

+ 'd, @@ -1054,34 +1097,51 @@ where } } -impl crate::private::Sealed for TwaiConfiguration<'_, crate::Blocking, T> where T: Instance {} +impl<'d> TwaiConfiguration<'d, Async> { + /// Create a new instance of [TwaiConfiguration] + /// + /// You will need to use a transceiver to connect to the TWAI bus + pub fn new_async( + peripheral: impl Peripheral

+ 'd, + rx_pin: impl Peripheral

+ 'd, + tx_pin: impl Peripheral

+ 'd, + baud_rate: BaudRate, + mode: TwaiMode, + ) -> Self { + Self::new_async_typed(peripheral.map_into(), rx_pin, tx_pin, baud_rate, mode) + } -impl InterruptConfigurable for TwaiConfiguration<'_, crate::Blocking, T> -where - T: Instance, -{ - fn set_interrupt_handler(&mut self, handler: crate::interrupt::InterruptHandler) { - self.internal_set_interrupt_handler(handler); + /// Create a new instance of [TwaiConfiguration] meant to connect two ESP32s + /// directly + /// + /// You don't need a transceiver by following the description in the + /// `twai.rs` example + pub fn new_async_no_transceiver( + peripheral: impl Peripheral

+ 'd, + rx_pin: impl Peripheral

+ 'd, + tx_pin: impl Peripheral

+ 'd, + baud_rate: BaudRate, + mode: TwaiMode, + ) -> Self { + Self::new_async_no_transceiver_typed(peripheral.map_into(), rx_pin, tx_pin, baud_rate, mode) } } -impl<'d, T> TwaiConfiguration<'d, crate::Async, T> +impl<'d, T> TwaiConfiguration<'d, Async, T> where T: Instance, { /// Create a new instance of [TwaiConfiguration] in async mode /// /// You will need to use a transceiver to connect to the TWAI bus - pub fn new_async( + pub fn new_async_typed( peripheral: impl Peripheral

+ 'd, rx_pin: impl Peripheral

+ 'd, tx_pin: impl Peripheral

+ 'd, baud_rate: BaudRate, mode: TwaiMode, ) -> Self { - let mut this = Self::new_internal(peripheral, rx_pin, tx_pin, baud_rate, false, mode); - this.internal_set_interrupt_handler(this.twai.async_handler()); - this + TwaiConfiguration::new_typed(peripheral, rx_pin, tx_pin, baud_rate, mode).into_async() } /// Create a new instance of [TwaiConfiguration] meant to connect two ESP32s @@ -1089,16 +1149,26 @@ where /// /// You don't need a transceiver by following the description in the /// `twai.rs` example - pub fn new_async_no_transceiver( + pub fn new_async_no_transceiver_typed( peripheral: impl Peripheral

+ 'd, rx_pin: impl Peripheral

+ 'd, tx_pin: impl Peripheral

+ 'd, baud_rate: BaudRate, mode: TwaiMode, ) -> Self { - let mut this = Self::new_internal(peripheral, rx_pin, tx_pin, baud_rate, true, mode); - this.internal_set_interrupt_handler(this.twai.async_handler()); - this + TwaiConfiguration::new_no_transceiver_typed(peripheral, rx_pin, tx_pin, baud_rate, mode) + .into_async() + } +} + +impl crate::private::Sealed for TwaiConfiguration<'_, Blocking, T> where T: Instance {} + +impl InterruptConfigurable for TwaiConfiguration<'_, Blocking, T> +where + T: Instance, +{ + fn set_interrupt_handler(&mut self, handler: crate::interrupt::InterruptHandler) { + self.internal_set_interrupt_handler(handler); } } @@ -1106,7 +1176,7 @@ where /// /// In this mode, the TWAI controller can transmit and receive messages /// including error signals (such as error and overload frames). -pub struct Twai<'d, DM: crate::Mode, T> { +pub struct Twai<'d, DM: crate::Mode, T = AnyTwai> { twai: PeripheralRef<'d, T>, tx: TwaiTx<'d, DM, T>, rx: TwaiRx<'d, DM, T>, @@ -1224,7 +1294,7 @@ where } /// Interface to the TWAI transmitter part. -pub struct TwaiTx<'d, DM: crate::Mode, T> { +pub struct TwaiTx<'d, DM: crate::Mode, T = AnyTwai> { twai: PeripheralRef<'d, T>, phantom: PhantomData, } @@ -1266,7 +1336,7 @@ where } /// Interface to the TWAI receiver part. -pub struct TwaiRx<'d, DM: crate::Mode, T> { +pub struct TwaiRx<'d, DM: crate::Mode, T = AnyTwai> { twai: PeripheralRef<'d, T>, phantom: PhantomData, } @@ -1417,7 +1487,7 @@ where } /// TWAI peripheral instance. -pub trait Instance: Peripheral

+ PeripheralMarker + 'static { +pub trait Instance: Peripheral

+ PeripheralMarker + Into + 'static { /// The identifier number for this TWAI instance. fn number(&self) -> usize; @@ -1677,6 +1747,35 @@ impl Instance for crate::peripherals::TWAI1 { } } +crate::any_peripheral! { + /// Any TWAI peripheral. + pub peripheral AnyTwai { + #[cfg(twai0)] + Twai0(crate::peripherals::TWAI0), + #[cfg(twai1)] + Twai1(crate::peripherals::TWAI1), + } +} + +impl Instance for AnyTwai { + delegate::delegate! { + to match &self.0 { + #[cfg(twai0)] + AnyTwaiInner::Twai0(twai) => twai, + #[cfg(twai1)] + AnyTwaiInner::Twai1(twai) => twai, + } { + fn number(&self) -> usize; + fn input_signal(&self) -> InputSignal; + fn output_signal(&self) -> OutputSignal; + fn interrupt(&self) -> crate::peripherals::Interrupt; + fn async_handler(&self) -> InterruptHandler; + fn register_block(&self) -> &RegisterBlock; + fn async_state(&self) -> &asynch::TwaiAsyncState; + } + } +} + mod asynch { use core::{future::poll_fn, task::Poll}; diff --git a/examples/src/bin/embassy_twai.rs b/examples/src/bin/embassy_twai.rs index de2a798ab7e..40128f04abd 100644 --- a/examples/src/bin/embassy_twai.rs +++ b/examples/src/bin/embassy_twai.rs @@ -36,8 +36,6 @@ use embedded_can::{Frame, Id}; use esp_backtrace as _; use esp_hal::{ gpio::Io, - interrupt, - peripherals::{self, TWAI0}, timer::timg::TimerGroup, twai::{self, EspTwaiFrame, StandardId, TwaiMode, TwaiRx, TwaiTx}, }; @@ -47,10 +45,7 @@ use static_cell::StaticCell; type TwaiOutbox = Channel; #[embassy_executor::task] -async fn receiver( - mut rx: TwaiRx<'static, esp_hal::Async, TWAI0>, - channel: &'static TwaiOutbox, -) -> ! { +async fn receiver(mut rx: TwaiRx<'static, esp_hal::Async>, channel: &'static TwaiOutbox) -> ! { loop { let frame = rx.receive_async().await; @@ -72,10 +67,7 @@ async fn receiver( } #[embassy_executor::task] -async fn transmitter( - mut tx: TwaiTx<'static, TWAI0, esp_hal::Async>, - channel: &'static TwaiOutbox, -) -> ! { +async fn transmitter(mut tx: TwaiTx<'static, esp_hal::Async>, channel: &'static TwaiOutbox) -> ! { loop { let frame = channel.receive().await; let result = tx.transmit_async(&frame).await; @@ -141,12 +133,6 @@ async fn main(spawner: Spawner) { // Get separate transmit and receive halves of the peripheral. let (rx, tx) = twai.split(); - interrupt::enable( - peripherals::Interrupt::TWAI0, - interrupt::Priority::Priority1, - ) - .unwrap(); - static CHANNEL: StaticCell = StaticCell::new(); let channel = &*CHANNEL.init(Channel::new()); diff --git a/hil-test/tests/twai.rs b/hil-test/tests/twai.rs index ac30b564b8f..da41f1dc258 100644 --- a/hil-test/tests/twai.rs +++ b/hil-test/tests/twai.rs @@ -8,7 +8,6 @@ use embedded_hal_02::can::Frame; use esp_hal::{ gpio::Io, - peripherals::TWAI0, prelude::*, twai::{self, filter::SingleStandardFilter, EspTwaiFrame, StandardId, TwaiMode}, Blocking, @@ -17,7 +16,7 @@ use hil_test as _; use nb::block; struct Context { - twai: twai::Twai<'static, Blocking, TWAI0>, + twai: twai::Twai<'static, Blocking>, } #[cfg(test)]