From 111bb07cd8456e231afff92f1c03b27ddd0dbbf6 Mon Sep 17 00:00:00 2001 From: Finomnis Date: Fri, 22 Dec 2023 13:55:44 +0100 Subject: [PATCH 1/8] Ensure minimum frame size in LPSPI transactions Co-authored-by: Ian McIntyre --- CHANGELOG.md | 4 +++ src/common/lpspi.rs | 59 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05b41178..5170fa86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ Add embedded-hal 1 implementations for the following drivers: - GPIO +Introduce LPSPI improvements: + +- Add additional checks for LPSPI frame sizes. + ## [0.5.4] 2023-11-26 Add CCM APIs for configuring FlexIO clocks on 1000 targets. diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 802abe0b..d2a86657 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -199,7 +199,7 @@ pub enum LpspiError { pub struct Transaction { /// Enable byte swap. /// - /// When enabled (`true`), swap bytes with the `u32` word. This allows + /// When enabled (`true`), swap bytes within the `u32` word. This allows /// you to change the endianness of the 32-bit word transfer. The /// default is `false`. pub byte_swap: bool, @@ -256,7 +256,21 @@ impl Transaction { } fn new_words(data: &[W]) -> Result { - Transaction::new(8 * core::mem::size_of_val(data) as u16) + if let Ok(frame_size) = u16::try_from(8 * core::mem::size_of_val(data)) { + Transaction::new(frame_size) + } else { + Err(LpspiError::FrameSize) + } + } + + fn frame_size_valid(frame_size: u16) -> bool { + const MIN_FRAME_SIZE: u16 = 8; + const MAX_FRAME_SIZE: u16 = 1 << 12; + const WORD_SIZE: u16 = 32; + + let last_frame_size = frame_size % WORD_SIZE; + + (MIN_FRAME_SIZE..=MAX_FRAME_SIZE).contains(&frame_size) && (1 != last_frame_size) } /// Define a transaction by specifying the frame size, in bits. @@ -270,10 +284,9 @@ impl Transaction { /// - `frame_size` fits within 12 bits; the implementation enforces this maximum value. /// - The minimum value for `frame_size` is 8; the implementation enforces this minimum /// value. + /// - The last 32-bit word in the frame is at least 2 bits long. pub fn new(frame_size: u16) -> Result { - const MIN_FRAME_SIZE: u16 = 8; - const MAX_FRAME_SIZE: u16 = 1 << 12; - if (MIN_FRAME_SIZE..MAX_FRAME_SIZE).contains(&frame_size) { + if Self::frame_size_valid(frame_size) { Ok(Self { byte_swap: false, bit_order: Default::default(), @@ -1056,3 +1069,39 @@ impl Word for u16 { impl Word for u32 { const MAX: u32 = u32::MAX; } + +#[cfg(test)] +mod tests { + #[test] + fn transaction_frame_sizes() { + assert!(super::Transaction::new_words(&[1u8]).is_ok()); + assert!(super::Transaction::new_words(&[1u8, 2]).is_ok()); + assert!(super::Transaction::new_words(&[1u8, 2, 3]).is_ok()); + assert!(super::Transaction::new_words(&[1u8, 2, 3, 4]).is_ok()); + assert!(super::Transaction::new_words(&[1u8, 2, 3, 4, 5]).is_ok()); + + assert!(super::Transaction::new_words(&[1u16]).is_ok()); + assert!(super::Transaction::new_words(&[1u16, 2]).is_ok()); + assert!(super::Transaction::new_words(&[1u16, 2, 3]).is_ok()); + assert!(super::Transaction::new_words(&[1u16, 2, 3, 4]).is_ok()); + assert!(super::Transaction::new_words(&[1u16, 2, 3, 4, 5]).is_ok()); + + assert!(super::Transaction::new_words(&[1u32]).is_ok()); + assert!(super::Transaction::new_words(&[1u32, 2]).is_ok()); + assert!(super::Transaction::new_words(&[1u32, 2, 3]).is_ok()); + assert!(super::Transaction::new_words(&[1u32, 2, 3, 4]).is_ok()); + assert!(super::Transaction::new_words(&[1u32, 2, 3, 4, 5]).is_ok()); + + assert!(super::Transaction::new(7).is_err()); + assert!(super::Transaction::new(8).is_ok()); + assert!(super::Transaction::new(9).is_ok()); + assert!(super::Transaction::new(31).is_ok()); + assert!(super::Transaction::new(32).is_ok()); + assert!(super::Transaction::new(33).is_err()); + assert!(super::Transaction::new(34).is_ok()); + assert!(super::Transaction::new(95).is_ok()); + assert!(super::Transaction::new(96).is_ok()); + assert!(super::Transaction::new(97).is_err()); + assert!(super::Transaction::new(98).is_ok()); + } +} From 49d534f07a9973858b7f814d753905fcbced5ee7 Mon Sep 17 00:00:00 2001 From: Finomnis Date: Fri, 22 Dec 2023 13:31:19 +0100 Subject: [PATCH 2/8] Rework LPSPI clock settings - Make sure we never get a baud rate faster than specified (slower is pretty much always ok for peripherals) - Configure PCSSCK and SCKPCS properly for contiguous transfers --- CHANGELOG.md | 1 + src/common/lpspi.rs | 39 +++++++++++++++++++++------------------ 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5170fa86..5ecd9183 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Add embedded-hal 1 implementations for the following drivers: Introduce LPSPI improvements: - Add additional checks for LPSPI frame sizes. +- Rework LPSPI clock settings. ## [0.5.4] 2023-11-26 diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index d2a86657..d0a59d42 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -306,24 +306,27 @@ impl Transaction { /// /// This should only happen when the LPSPI peripheral is disabled. fn set_spi_clock(source_clock_hz: u32, spi_clock_hz: u32, reg: &ral::lpspi::RegisterBlock) { - let mut div = source_clock_hz / spi_clock_hz; - - if source_clock_hz / div > spi_clock_hz { - div += 1; - } - - // 0 <= div <= 255, and the true coefficient is really div + 2 - let div = div.saturating_sub(2).clamp(0, 255); - ral::write_reg!( - ral::lpspi, - reg, - CCR, - SCKDIV: div, - // Both of these delays are arbitrary choices, and they should - // probably be configurable by the end-user. - DBT: div / 2, - SCKPCS: 0x1F, - PCSSCK: 0x1F + // Round up, so we always get a resulting SPI clock that is + // equal or less than the requested frequency. + let half_div = + u32::try_from(1 + u64::from(source_clock_hz - 1) / (u64::from(spi_clock_hz) * 2)).unwrap(); + + // Make sure SCKDIV is between 0 and 255 + // For some reason SCK starts to misbehave in between frames + // if half_div is less than 3. + let half_div = half_div.clamp(3, 128); + // Because half_div is in range [3,128], sckdiv is in range [4, 254]. + let sckdiv = 2 * (half_div - 1); + + ral::write_reg!(ral::lpspi, reg, CCR, + // Delay between two clock transitions of two consecutive transfers + // is exactly sckdiv/2, which causes the transfer to be seamless. + DBT: half_div - 1, + // Add one sckdiv/2 setup and hold time before and after the transfer, + // to make sure the signal is stable at sample time + PCSSCK: half_div - 1, + SCKPCS: half_div - 1, + SCKDIV: sckdiv ); } From b7c4e97fb51fcc149ca2fb3a5c594dd6c8f97891 Mon Sep 17 00:00:00 2001 From: Finomnis Date: Fri, 22 Dec 2023 14:31:38 +0100 Subject: [PATCH 3/8] Add Lpspi::flush() Co-authored-by: Ian McIntyre --- CHANGELOG.md | 1 + src/common/lpspi.rs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ecd9183..fa0b31fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Introduce LPSPI improvements: - Add additional checks for LPSPI frame sizes. - Rework LPSPI clock settings. +- Add `flush()` method. ## [0.5.4] 2023-11-26 diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index d0a59d42..0308eae6 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -630,6 +630,37 @@ impl Lpspi { ); } + /// Wait for all ongoing transactions to be finished. + pub fn flush(&mut self) -> Result<(), LpspiError> { + loop { + let status = self.status(); + + if status.intersects(Status::RECEIVE_ERROR) { + return Err(LpspiError::Fifo(Direction::Rx)); + } + if status.intersects(Status::TRANSMIT_ERROR) { + return Err(LpspiError::Fifo(Direction::Tx)); + } + + // Contributor testing reveals that the busy flag may not set once + // the TX FIFO is filled. This means a sequence like + // + // lpspi.write(&[...])?; + // lpspi.flush()?; + // + // could pass through flush without observing busy. Therefore, we + // also check the FIFO contents. Even if the peripheral isn't + // busy, the FIFO should be non-empty. + // + // We can't just rely on the FIFO contents, since the FIFO could be + // empty while the transaction is completing. (There's data in the + // shift register, and PCS is still asserted.) + if !status.intersects(Status::BUSY) && self.fifo_status().is_empty(Direction::Tx) { + return Ok(()); + } + } + } + /// Exchanges data with the SPI device. /// /// This routine uses continuous transfers to perform the transaction, no matter the @@ -883,6 +914,14 @@ impl FifoStatus { }; count >= MAX_FIFO_SIZE } + /// Indicates if the FIFO is empty for the given direction. + #[inline] + const fn is_empty(self, direction: Direction) -> bool { + 0 == match direction { + Direction::Tx => self.txcount, + Direction::Rx => self.rxcount, + } + } } bitflags::bitflags! { From 36277e1541e62264961d1b2f242306e95bc066c7 Mon Sep 17 00:00:00 2001 From: Finomnis Date: Fri, 22 Dec 2023 14:14:49 +0100 Subject: [PATCH 4/8] Rework LPSPI initialization, disable Co-authored-by: Ian McIntyre --- CHANGELOG.md | 2 +- src/common/lpspi.rs | 38 ++++++++++++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa0b31fe..89a06722 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ Add embedded-hal 1 implementations for the following drivers: Introduce LPSPI improvements: - Add additional checks for LPSPI frame sizes. -- Rework LPSPI clock settings. +- Rework LPSPI clock settings, initialization, and disable. - Add `flush()` method. ## [0.5.4] 2023-11-26 diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 0308eae6..adc73eb0 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -425,8 +425,16 @@ impl Lpspi { pins, bit_order: BitOrder::default(), }; - ral::write_reg!(ral::lpspi, spi.lpspi, CR, RST: RST_1); - ral::write_reg!(ral::lpspi, spi.lpspi, CR, RST: RST_0); + + // Reset and disable + ral::modify_reg!(ral::lpspi, spi.lpspi, CR, MEN: MEN_0, RST: RST_1); + while spi.is_enabled() {} + ral::modify_reg!(ral::lpspi, spi.lpspi, CR, RST: RST_0); + + // Reset Fifos + ral::modify_reg!(ral::lpspi, spi.lpspi, CR, RTF: RTF_1, RRF: RRF_1); + + // Configure master mode ral::write_reg!( ral::lpspi, spi.lpspi, @@ -435,8 +443,16 @@ impl Lpspi { SAMPLE: SAMPLE_1 ); Disabled::new(&mut spi.lpspi).set_mode(MODE_0); - ral::write_reg!(ral::lpspi, spi.lpspi, FCR, RXWATER: 0xF, TXWATER: 0xF); + + let tx_fifo_size = spi.max_watermark(Direction::Tx); + // Configure watermarks + ral::write_reg!(ral::lpspi, spi.lpspi, FCR, + RXWATER: 0, // Notify when we have any data available + TXWATER: u32::from(tx_fifo_size) / 2 // Nofify when we have at least tx_fifo_size/2 space available + ); + ral::write_reg!(ral::lpspi, spi.lpspi, CR, MEN: MEN_1); + spi } @@ -490,10 +506,8 @@ impl Lpspi { /// Temporarily disable the LPSPI peripheral. /// /// The handle to a [`Disabled`](crate::lpspi::Disabled) driver lets you modify - /// LPSPI settings that require a fully disabled peripheral. This will clear the transmit - /// and receive FIFOs. + /// LPSPI settings that require a fully disabled peripheral. pub fn disabled(&mut self, func: impl FnOnce(&mut Disabled) -> R) -> R { - self.clear_fifos(); let mut disabled = Disabled::new(&mut self.lpspi); func(&mut disabled) } @@ -819,6 +833,13 @@ impl Lpspi { pub fn tdr(&self) -> *const ral::WORegister { core::ptr::addr_of!(self.lpspi.TDR) } + + fn max_watermark(&self, direction: Direction) -> u8 { + (match direction { + Direction::Rx => 1 << ral::read_reg!(ral::lpspi, self.lpspi, PARAM, RXFIFO), + Direction::Tx => 1 << ral::read_reg!(ral::lpspi, self.lpspi, PARAM, TXFIFO), + }) as u8 + } } bitflags::bitflags! { @@ -959,7 +980,12 @@ pub struct Disabled<'a, const N: u8> { impl<'a, const N: u8> Disabled<'a, N> { fn new(lpspi: &'a mut ral::lpspi::Instance) -> Self { let men = ral::read_reg!(ral::lpspi, lpspi, CR, MEN == MEN_1); + + // Request disable ral::modify_reg!(ral::lpspi, lpspi, CR, MEN: MEN_0); + // Wait for the driver to finish its current transfer + // and enter disabled state + while ral::read_reg!(ral::lpspi, lpspi, CR, MEN == MEN_1) {} Self { lpspi, men } } From 2039b4817d895079da3898d04b5ece40d73a9e6f Mon Sep 17 00:00:00 2001 From: Finomnis Date: Fri, 22 Dec 2023 13:31:19 +0100 Subject: [PATCH 5/8] Update LPSPI::set_enable documentation --- src/common/lpspi.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index adc73eb0..cf4b99d1 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -462,6 +462,11 @@ impl Lpspi { } /// Enable (`true`) or disable (`false`) the peripheral. + /// + /// Note that disabling does not take effect immediately; instead the + /// peripheral finishes the current transfer and then disables itself. + /// It is required to check [`is_enabled()`](Self::is_enabled) repeatedly until the + /// peripheral is actually disabled. pub fn set_enable(&mut self, enable: bool) { ral::modify_reg!(ral::lpspi, self.lpspi, CR, MEN: enable as u32) } From 1b721f513b533203a1df4ebb2524d16adcf2a8d4 Mon Sep 17 00:00:00 2001 From: Finomnis Date: Fri, 22 Dec 2023 14:31:38 +0100 Subject: [PATCH 6/8] Add LPSPI::soft_reset --- CHANGELOG.md | 1 + src/common/lpspi.rs | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89a06722..e62f250e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Introduce LPSPI improvements: - Add additional checks for LPSPI frame sizes. - Rework LPSPI clock settings, initialization, and disable. - Add `flush()` method. +- Add `soft_reset()` method. ## [0.5.4] 2023-11-26 diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index cf4b99d1..12e1ee6e 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -845,6 +845,45 @@ impl Lpspi { Direction::Tx => 1 << ral::read_reg!(ral::lpspi, self.lpspi, PARAM, TXFIFO), }) as u8 } + + /// Reset all internal logic while preserving the driver's configurations. + /// + /// Unlike [`reset()`](Self::reset), this preserves all peripheral registers. + pub fn soft_reset(&mut self) { + // Backup previous registers + let ier = ral::read_reg!(ral::lpspi, self.lpspi, IER); + let der = ral::read_reg!(ral::lpspi, self.lpspi, DER); + let cfgr0 = ral::read_reg!(ral::lpspi, self.lpspi, CFGR0); + let cfgr1 = ral::read_reg!(ral::lpspi, self.lpspi, CFGR1); + let dmr0 = ral::read_reg!(ral::lpspi, self.lpspi, DMR0); + let dmr1 = ral::read_reg!(ral::lpspi, self.lpspi, DMR1); + let ccr = ral::read_reg!(ral::lpspi, self.lpspi, CCR); + let fcr = ral::read_reg!(ral::lpspi, self.lpspi, FCR); + + // Backup enabled state + let enabled = self.is_enabled(); + + // Reset and disable + ral::modify_reg!(ral::lpspi, self.lpspi, CR, MEN: MEN_0, RST: RST_1); + while self.is_enabled() {} + ral::modify_reg!(ral::lpspi, self.lpspi, CR, RST: RST_0); + + // Reset fifos + ral::modify_reg!(ral::lpspi, self.lpspi, CR, RTF: RTF_1, RRF: RRF_1); + + // Restore settings + ral::write_reg!(ral::lpspi, self.lpspi, IER, ier); + ral::write_reg!(ral::lpspi, self.lpspi, DER, der); + ral::write_reg!(ral::lpspi, self.lpspi, CFGR0, cfgr0); + ral::write_reg!(ral::lpspi, self.lpspi, CFGR1, cfgr1); + ral::write_reg!(ral::lpspi, self.lpspi, DMR0, dmr0); + ral::write_reg!(ral::lpspi, self.lpspi, DMR1, dmr1); + ral::write_reg!(ral::lpspi, self.lpspi, CCR, ccr); + ral::write_reg!(ral::lpspi, self.lpspi, FCR, fcr); + + // Restore enabled state + self.set_enable(enabled); + } } bitflags::bitflags! { From 3a458140b100dffbc18d3a1c1efa6061e47d482d Mon Sep 17 00:00:00 2001 From: Finomnis Date: Sat, 27 Apr 2024 16:09:00 -0400 Subject: [PATCH 7/8] Enable LPSPI mode changes while enabled Currently non-breaking, and we encourage users towards the Lpspi::set_mode method. This allows us to avoid the RMW on TCR, which the reference manual says isn't best. Co-authored-by: Ian McIntyre --- CHANGELOG.md | 2 ++ src/common/lpspi.rs | 45 ++++++++++++++++++++++++++------------------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e62f250e..9df8a898 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ Introduce LPSPI improvements: - Rework LPSPI clock settings, initialization, and disable. - Add `flush()` method. - Add `soft_reset()` method. +- Allow users to change the mode while enabled. Deprecate the corresponding + method on the `Disabled` helper. ## [0.5.4] 2023-11-26 diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 12e1ee6e..72a2d33e 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -345,6 +345,7 @@ pub struct Lpspi { lpspi: ral::lpspi::Instance, pins: P, bit_order: BitOrder, + mode: Mode, } /// Pins for a LPSPI device. @@ -420,10 +421,11 @@ impl Lpspi { pub const N: u8 = N; fn init(lpspi: ral::lpspi::Instance, pins: P) -> Self { - let mut spi = Lpspi { + let spi = Lpspi { lpspi, pins, bit_order: BitOrder::default(), + mode: MODE_0, }; // Reset and disable @@ -442,7 +444,6 @@ impl Lpspi { MASTER: MASTER_1, SAMPLE: SAMPLE_1 ); - Disabled::new(&mut spi.lpspi).set_mode(MODE_0); let tx_fifo_size = spi.max_watermark(Direction::Tx); // Configure watermarks @@ -513,7 +514,7 @@ impl Lpspi { /// The handle to a [`Disabled`](crate::lpspi::Disabled) driver lets you modify /// LPSPI settings that require a fully disabled peripheral. pub fn disabled(&mut self, func: impl FnOnce(&mut Disabled) -> R) -> R { - let mut disabled = Disabled::new(&mut self.lpspi); + let mut disabled = Disabled::new(&mut self.lpspi, &mut self.mode); func(&mut disabled) } @@ -630,6 +631,14 @@ impl Lpspi { } } + /// Set the SPI mode for the peripheral. + /// + /// This only affects the next transfer; ongoing transfers + /// will not be influenced. + pub fn set_mode(&mut self, mode: Mode) { + self.mode = mode; + } + /// Place a transaction definition into the transmit FIFO. /// /// Once this definition is popped from the transmit FIFO, this may @@ -638,7 +647,12 @@ impl Lpspi { /// You're responsible for making sure there's space in the transmit /// FIFO for this transaction command. pub fn enqueue_transaction(&mut self, transaction: &Transaction) { - ral::modify_reg!(ral::lpspi, self.lpspi, TCR, + ral::write_reg!(ral::lpspi, self.lpspi, TCR, + CPOL: if self.mode.polarity == Polarity::IdleHigh { CPOL_1 } else { CPOL_0 }, + CPHA: if self.mode.phase == Phase::CaptureOnSecondTransition { CPHA_1 } else { CPHA_0 }, + PRESCALE: PRESCALE_0, + PCS: PCS_0, + WIDTH: WIDTH_0, LSBF: transaction.bit_order as u32, BYSW: transaction.byte_swap as u32, RXMSK: transaction.receive_data_mask as u32, @@ -1018,11 +1032,12 @@ bitflags::bitflags! { /// An LPSPI peripheral which is temporarily disabled. pub struct Disabled<'a, const N: u8> { lpspi: &'a ral::lpspi::Instance, + mode: &'a mut Mode, men: bool, } impl<'a, const N: u8> Disabled<'a, N> { - fn new(lpspi: &'a mut ral::lpspi::Instance) -> Self { + fn new(lpspi: &'a mut ral::lpspi::Instance, mode: &'a mut Mode) -> Self { let men = ral::read_reg!(ral::lpspi, lpspi, CR, MEN == MEN_1); // Request disable @@ -1030,24 +1045,16 @@ impl<'a, const N: u8> Disabled<'a, N> { // Wait for the driver to finish its current transfer // and enter disabled state while ral::read_reg!(ral::lpspi, lpspi, CR, MEN == MEN_1) {} - Self { lpspi, men } + Self { lpspi, mode, men } } /// Set the SPI mode for the peripheral + #[deprecated( + since = "0.5.5", + note = "Use Lpspi::set_mode to change modes while enabled." + )] pub fn set_mode(&mut self, mode: Mode) { - // This could probably be changed when we're not disabled. - // However, there's rules about when you can read TCR. - // Specifically, reading TCR while it's being loaded from - // the transmit FIFO could result in an incorrect reading. - // Only permitting this when we're disabled might help - // us avoid something troublesome. - ral::modify_reg!( - ral::lpspi, - self.lpspi, - TCR, - CPOL: ((mode.polarity == Polarity::IdleHigh) as u32), - CPHA: ((mode.phase == Phase::CaptureOnSecondTransition) as u32) - ); + *self.mode = mode; } /// Set the LPSPI clock speed (Hz). From b9a82455ffa967b5612ec5ecee52c2b11d45e6cc Mon Sep 17 00:00:00 2001 From: Finomnis Date: Sat, 27 Apr 2024 16:17:56 -0400 Subject: [PATCH 8/8] Enable LPSPI watermark change while enabled Unlike LPUART, there's no requirement to be disabled while changing this. Keep the existing API, and point users towards the new method. Co-authored-by: Ian McIntyre --- CHANGELOG.md | 2 ++ examples/rtic_spi.rs | 10 +++----- src/common/lpspi.rs | 58 +++++++++++++++++++++++++++++++------------- 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9df8a898..e1faed3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ Introduce LPSPI improvements: - Add `soft_reset()` method. - Allow users to change the mode while enabled. Deprecate the corresponding method on the `Disabled` helper. +- Allow users to change the watermark while enabled. Deprecate the corresponding + method on the `Disabled` helper. ## [0.5.4] 2023-11-26 diff --git a/examples/rtic_spi.rs b/examples/rtic_spi.rs index 29fe5faa..8a50c4c0 100644 --- a/examples/rtic_spi.rs +++ b/examples/rtic_spi.rs @@ -25,12 +25,10 @@ mod app { #[init] fn init(_: init::Context) -> (Shared, Local, init::Monotonics) { let (_, board::Specifics { mut spi, .. }) = board::new(); - spi.disabled(|spi| { - // Trigger when the TX FIFO is empty. - spi.set_watermark(Direction::Tx, 0); - // Wait to receive at least 2 u32s. - spi.set_watermark(Direction::Rx, 1); - }); + // Trigger when the TX FIFO is empty. + spi.set_watermark(Direction::Tx, 0); + // Wait to receive at least 2 u32s. + spi.set_watermark(Direction::Rx, 1); // Starts the I/O as soon as we're done initializing, since // the TX FIFO is empty. spi.set_interrupts(Interrupts::TRANSMIT_DATA); diff --git a/src/common/lpspi.rs b/src/common/lpspi.rs index 72a2d33e..c2c03115 100644 --- a/src/common/lpspi.rs +++ b/src/common/lpspi.rs @@ -898,6 +898,21 @@ impl Lpspi { // Restore enabled state self.set_enable(enabled); } + + /// Set the watermark level for a given direction. + /// + /// Returns the watermark level committed to the hardware. This may be different + /// than the supplied `watermark`, since it's limited by the hardware. + /// + /// When `direction == Direction::Rx`, the receive data flag is set whenever the + /// number of words in the receive FIFO is greater than `watermark`. + /// + /// When `direction == Direction::Tx`, the transmit data flag is set whenever the + /// the number of words in the transmit FIFO is less than, or equal, to `watermark`. + #[inline] + pub fn set_watermark(&mut self, direction: Direction, watermark: u8) -> u8 { + set_watermark(&self.lpspi, direction, watermark) + } } bitflags::bitflags! { @@ -1029,6 +1044,27 @@ bitflags::bitflags! { } } +#[inline] +fn set_watermark(lpspi: &ral::lpspi::RegisterBlock, direction: Direction, watermark: u8) -> u8 { + let max_watermark = match direction { + Direction::Rx => 1 << ral::read_reg!(ral::lpspi, lpspi, PARAM, RXFIFO), + Direction::Tx => 1 << ral::read_reg!(ral::lpspi, lpspi, PARAM, TXFIFO), + }; + + let watermark = watermark.min(max_watermark - 1); + + match direction { + Direction::Rx => { + ral::modify_reg!(ral::lpspi, lpspi, FCR, RXWATER: watermark as u32) + } + Direction::Tx => { + ral::modify_reg!(ral::lpspi, lpspi, FCR, TXWATER: watermark as u32) + } + } + + watermark +} + /// An LPSPI peripheral which is temporarily disabled. pub struct Disabled<'a, const N: u8> { lpspi: &'a ral::lpspi::Instance, @@ -1076,24 +1112,12 @@ impl<'a, const N: u8> Disabled<'a, N> { /// When `direction == Direction::Tx`, the transmit data flag is set whenever the /// the number of words in the transmit FIFO is less than, or equal, to `watermark`. #[inline] + #[deprecated( + since = "0.5.5", + note = "Use Lpspi::set_watermark to change watermark while enabled" + )] pub fn set_watermark(&mut self, direction: Direction, watermark: u8) -> u8 { - let max_watermark = match direction { - Direction::Rx => 1 << ral::read_reg!(ral::lpspi, self.lpspi, PARAM, RXFIFO), - Direction::Tx => 1 << ral::read_reg!(ral::lpspi, self.lpspi, PARAM, TXFIFO), - }; - - let watermark = watermark.min(max_watermark - 1); - - match direction { - Direction::Rx => { - ral::modify_reg!(ral::lpspi, self.lpspi, FCR, RXWATER: watermark as u32) - } - Direction::Tx => { - ral::modify_reg!(ral::lpspi, self.lpspi, FCR, TXWATER: watermark as u32) - } - } - - watermark + set_watermark(self.lpspi, direction, watermark) } /// Set the sampling point of the LPSPI peripheral.