Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor SPI MISO setup #2557

Merged
merged 7 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions esp-hal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Updated `esp-pacs` with support for Wi-Fi on the ESP32 and made the peripheral non virtual
- `SpiBitOrder`, `SpiDataMode`, `SpiMode` were renamed to `BitOder`, `DataMode` and `Mode` (#2828)
- `crate::Mode` was renamed to `crate::DriverMode` (#2828)
- `Spi::with_miso` has been overloaded into `Spi::with_miso` and `Spi::with_sio1` (#2557)
- Renamed some I2C error variants (#2844)
- I2C: Replaced potential panics with errors. (#2831)
- UART: Make `AtCmdConfig` and `ConfigError` non-exhaustive (#2851)
Expand Down
8 changes: 8 additions & 0 deletions esp-hal/MIGRATING-0.22.md
Original file line number Diff line number Diff line change
Expand Up @@ -386,3 +386,11 @@ e.g.)
The previous blocking implementation of `read_bytes` has been removed, and the non-blocking `drain_fifo` has instead been renamed to `read_bytes` in its place.

Any code which was previously using `read_bytes` to fill a buffer in a blocking manner will now need to implement the necessary logic to block until the buffer is filled in their application instead.

## Spi `with_miso` has been split

Previously, `with_miso` set up the provided pin as an input and output, which was necessary for half duplex.
Full duplex does not require this, and it also creates an artificial restriction.

If you were using half duplex SPI with `with_miso`,
you should now use `with_sio1` instead to get the previous behavior.
24 changes: 22 additions & 2 deletions esp-hal/src/spi/master.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,12 @@ use crate::{
asynch::AtomicWaker,
clock::Clocks,
dma::{DmaChannelFor, DmaEligible, DmaRxBuffer, DmaTxBuffer, Rx, Tx},
gpio::{interconnect::PeripheralOutput, InputSignal, NoPin, OutputSignal},
gpio::{
interconnect::{PeripheralInput, PeripheralOutput},
InputSignal,
NoPin,
OutputSignal,
},
interrupt::{InterruptConfigurable, InterruptHandler},
peripheral::{Peripheral, PeripheralRef},
peripherals::spi2::RegisterBlock,
Expand Down Expand Up @@ -676,9 +681,24 @@ where

/// Assign the MISO (Master In Slave Out) pin for the SPI instance.
///
/// Enables input functionality for the pin, and connects it to the MISO
/// signal.
pub fn with_miso<MISO: PeripheralInput>(self, miso: impl Peripheral<P = MISO> + 'd) -> Self {
crate::into_mapped_ref!(miso);
miso.enable_input(true, private::Internal);

self.driver().info.miso.connect_to(&mut miso);

self
}

/// Assign the SIO1/MISO pin for the SPI instance.
///
/// Enables both input and output functionality for the pin, and connects it
/// to the MISO signal and SIO1 input signal.
pub fn with_miso<MISO: PeripheralOutput>(self, miso: impl Peripheral<P = MISO> + 'd) -> Self {
///
/// Note: You do not need to call [Self::with_miso] when this is used.
pub fn with_sio1<SIO1: PeripheralOutput>(self, miso: impl Peripheral<P = SIO1> + 'd) -> Self {
MabezDev marked this conversation as resolved.
Show resolved Hide resolved
crate::into_mapped_ref!(miso);
miso.enable_input(true, private::Internal);
miso.enable_output(true, private::Internal);
Expand Down
4 changes: 2 additions & 2 deletions hil-test/tests/qspi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ mod tests {
let [pin, pin_mirror, _] = ctx.gpios;
let pin_mirror = Output::new(pin_mirror, Level::High);

let spi = ctx.spi.with_miso(pin).with_dma(ctx.dma_channel);
let spi = ctx.spi.with_sio1(pin).with_dma(ctx.dma_channel);

super::execute_write_read(spi, pin_mirror, 0b0010_0010);
}
Expand Down Expand Up @@ -355,7 +355,7 @@ mod tests {
let spi = ctx
.spi
.with_mosi(mosi)
.with_miso(gpio)
.with_sio1(gpio)
.with_dma(ctx.dma_channel);

super::execute_write(unit0, unit1, spi, 0b0000_0010, true);
Expand Down
6 changes: 4 additions & 2 deletions hil-test/tests/spi_full_duplex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,16 @@ mod tests {
}
}

let (miso, mosi) = mosi.split();
#[cfg(pcnt)]
let (mosi_loopback_pcnt, mosi) = mosi.split();
let mosi_loopback_pcnt = miso.clone();

// Need to set miso first so that mosi can overwrite the
// output connection (because we are using the same pin to loop back)
let spi = Spi::new(peripherals.SPI2, Config::default().with_frequency(10.MHz()))
.unwrap()
.with_sck(sclk)
.with_miso(unsafe { mosi.clone_unchecked() })
.with_miso(miso)
.with_mosi(mosi);

let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(32000);
Expand Down
Loading