Skip to content

Make in-progress DMA transfers potentially fallible #665

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.

Sign up for GitHub

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

Merged
MabezDev merged 1 commit into from
Jul 22, 2023
Merged

Make in-progress DMA transfers potentially fallible #665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Remove the `allow-opt-level-z` feature from `esp32c3-hal` (#654)

### Breaking

- `DmaTransfer::wait` and `I2sReadDmaTransfer::wait_receive` now return `Result` (#665)

## [0.10.0] - 2023-06-04

### Added
Expand Down
16 changes: 14 additions & 2 deletions esp-hal-common/src/dma/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,8 @@ pub trait RxPrivate {

fn drain_buffer(&mut self, dst: &mut [u8]) -> Result<usize, DmaError>;

fn has_error(&self) -> bool;

#[cfg(feature = "async")]
fn waker() -> &'static embassy_sync::waitqueue::AtomicWaker;
}
Expand Down Expand Up @@ -474,6 +476,10 @@ where
R::unlisten_in_eof()
}

fn has_error(&self) -> bool {
R::has_in_descriptor_error()
}

#[cfg(feature = "async")]
fn waker() -> &'static embassy_sync::waitqueue::AtomicWaker {
T::waker()
Expand Down Expand Up @@ -504,6 +510,8 @@ pub trait TxPrivate {

fn available(&mut self) -> usize;

fn has_error(&self) -> bool;

fn push(&mut self, data: &[u8]) -> Result<usize, DmaError>;

#[cfg(feature = "async")]
Expand Down Expand Up @@ -790,6 +798,10 @@ where
R::unlisten_out_eof()
}

fn has_error(&self) -> bool {
R::has_out_descriptor_error()
}

#[cfg(feature = "async")]
fn waker() -> &'static embassy_sync::waitqueue::AtomicWaker {
T::waker()
Expand Down Expand Up @@ -850,7 +862,7 @@ where
#[allow(drop_bounds)]
pub trait DmaTransfer<B, T>: Drop {
/// Wait for the transfer to finish.
fn wait(self) -> (B, T);
fn wait(self) -> Result<(B, T), (DmaError, B, T)>;
/// Check if the transfer is finished.
fn is_done(&self) -> bool;
}
Expand All @@ -859,7 +871,7 @@ pub trait DmaTransfer<B, T>: Drop {
#[allow(drop_bounds)]
pub trait DmaTransferRxTx<BR, BT, T>: Drop {
/// Wait for the transfer to finish.
fn wait(self) -> (BR, BT, T);
fn wait(self) -> Result<(BR, BT, T), (DmaError, BR, BT, T)>;
/// Check if the transfer is finished.
fn is_done(&self) -> bool;
}
Expand Down
35 changes: 29 additions & 6 deletions esp-hal-common/src/i2s.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,9 @@ where
{
/// Wait for the DMA transfer to complete and return the buffers and the
/// I2sTx instance.
fn wait(self) -> (BUFFER, I2sTx<'d, T, P, CH>) {
fn wait(
self,
) -> Result<(BUFFER, I2sTx<'d, T, P, CH>), (DmaError, BUFFER, I2sTx<'d, T, P, CH>)> {
self.i2s_tx.wait_tx_dma_done().ok(); // waiting for the DMA transfer is not enough

// `DmaTransfer` needs to have a `Drop` implementation, because we accept
Expand All @@ -331,8 +333,13 @@ where
unsafe {
let buffer = core::ptr::read(&self.buffer);
let payload = core::ptr::read(&self.i2s_tx);
let err = (&self).i2s_tx.tx_channel.has_error();
core::mem::forget(self);
(buffer, payload)
if err {
Err((DmaError::DescriptorError, buffer, payload))
} else {
Ok((buffer, payload))
}
}
}

Expand Down Expand Up @@ -412,7 +419,11 @@ where
/// I2sTx instance after copying the read data to the given buffer.
/// Length of the received data is returned at the third element of the
/// tuple.
pub fn wait_receive(mut self, dst: &mut [u8]) -> (BUFFER, I2sRx<'d, T, P, CH>, usize) {
pub fn wait_receive(
mut self,
dst: &mut [u8],
) -> Result<(BUFFER, I2sRx<'d, T, P, CH>, usize), (DmaError, BUFFER, I2sRx<'d, T, P, CH>, usize)>
{
self.i2s_rx.wait_rx_dma_done().ok(); // waiting for the DMA transfer is not enough

let len = self.i2s_rx.rx_channel.drain_buffer(dst).unwrap();
Expand All @@ -427,8 +438,13 @@ where
unsafe {
let buffer = core::ptr::read(&self.buffer);
let payload = core::ptr::read(&self.i2s_rx);
let err = (&self).i2s_rx.rx_channel.has_error();
core::mem::forget(self);
(buffer, payload, len)
if err {
Err((DmaError::DescriptorError, buffer, payload, len))
} else {
Ok((buffer, payload, len))
}
}
}
}
Expand All @@ -442,7 +458,9 @@ where
{
/// Wait for the DMA transfer to complete and return the buffers and the
/// I2sTx instance.
fn wait(self) -> (BUFFER, I2sRx<'d, T, P, CH>) {
fn wait(
self,
) -> Result<(BUFFER, I2sRx<'d, T, P, CH>), (DmaError, BUFFER, I2sRx<'d, T, P, CH>)> {
self.i2s_rx.wait_rx_dma_done().ok(); // waiting for the DMA transfer is not enough

// `DmaTransfer` needs to have a `Drop` implementation, because we accept
Expand All @@ -455,8 +473,13 @@ where
unsafe {
let buffer = core::ptr::read(&self.buffer);
let payload = core::ptr::read(&self.i2s_rx);
let err = (&self).i2s_rx.rx_channel.has_error();
core::mem::forget(self);
(buffer, payload)
if err {
Err((DmaError::DescriptorError, buffer, payload))
} else {
Ok((buffer, payload))
}
}
}

Expand Down
40 changes: 36 additions & 4 deletions esp-hal-common/src/spi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,7 @@ pub mod dma {
dma::{
Channel,
ChannelTypes,
DmaError,
DmaTransfer,
DmaTransferRxTx,
RxPrivate,
Expand Down Expand Up @@ -846,7 +847,12 @@ pub mod dma {
{
/// Wait for the DMA transfer to complete and return the buffers and the
/// SPI instance.
fn wait(mut self) -> (RXBUF, TXBUF, SpiDma<'d, T, C, M>) {
fn wait(
mut self,
) -> Result<
(RXBUF, TXBUF, SpiDma<'d, T, C, M>),
(DmaError, RXBUF, TXBUF, SpiDma<'d, T, C, M>),
> {
self.spi_dma.spi.flush().ok(); // waiting for the DMA transfer is not enough

// `DmaTransfer` needs to have a `Drop` implementation, because we accept
Expand All @@ -860,8 +866,14 @@ pub mod dma {
let rbuffer = core::ptr::read(&self.rbuffer);
let tbuffer = core::ptr::read(&self.tbuffer);
let payload = core::ptr::read(&self.spi_dma);
let err = (&self).spi_dma.channel.rx.has_error()
|| (&self).spi_dma.channel.tx.has_error();
mem::forget(self);
(rbuffer, tbuffer, payload)
if err {
Err((DmaError::DescriptorError, rbuffer, tbuffer, payload))
} else {
Ok((rbuffer, tbuffer, payload))
}
}
}

Expand Down Expand Up @@ -906,7 +918,10 @@ pub mod dma {
{
/// Wait for the DMA transfer to complete and return the buffers and the
/// SPI instance.
fn wait(mut self) -> (BUFFER, SpiDma<'d, T, C, M>) {
fn wait(
mut self,
) -> Result<(BUFFER, SpiDma<'d, T, C, M>), (DmaError, BUFFER, SpiDma<'d, T, C, M>)>
{
self.spi_dma.spi.flush().ok(); // waiting for the DMA transfer is not enough

// `DmaTransfer` needs to have a `Drop` implementation, because we accept
Expand All @@ -919,8 +934,14 @@ pub mod dma {
unsafe {
let buffer = core::ptr::read(&self.buffer);
let payload = core::ptr::read(&self.spi_dma);
let err = (&self).spi_dma.channel.rx.has_error()
|| (&self).spi_dma.channel.tx.has_error();
mem::forget(self);
(buffer, payload)
if err {
Err((DmaError::DescriptorError, buffer, payload))
} else {
Ok((buffer, payload))
}
}
}

Expand Down Expand Up @@ -955,6 +976,17 @@ pub mod dma {
_mode: PhantomData<M>,
}

impl<'d, T, C, M> core::fmt::Debug for SpiDma<'d, T, C, M>
where
C: ChannelTypes,
C::P: SpiPeripheral,
M: DuplexMode,
{
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("SpiDma").finish()
}
}

impl<'d, T, C, M> SpiDma<'d, T, C, M>
where
T: InstanceDma<C::Tx<'d>, C::Rx<'d>>,
Expand Down
10 changes: 5 additions & 5 deletions esp32-hal/examples/qspi_flash.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ fn main() -> ! {
zero_buf,
)
.unwrap();
(zero_buf, spi) = transfer.wait();
(zero_buf, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

// erase sector
Expand All @@ -122,7 +122,7 @@ fn main() -> ! {
zero_buf,
)
.unwrap();
(zero_buf, spi) = transfer.wait();
(zero_buf, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

// write enable
Expand All @@ -135,7 +135,7 @@ fn main() -> ! {
zero_buf,
)
.unwrap();
(_, spi) = transfer.wait();
(_, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

// write data / program page
Expand All @@ -150,7 +150,7 @@ fn main() -> ! {
send,
)
.unwrap();
(_, spi) = transfer.wait();
(_, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

loop {
Expand All @@ -168,7 +168,7 @@ fn main() -> ! {
// here we could do something else while DMA transfer is in progress
// the buffers and spi is moved into the transfer and we can get it back via
// `wait`
(receive, spi) = transfer.wait();
(receive, spi) = transfer.wait().unwrap();

println!("{:x?}", &receive);
for b in &mut receive.iter() {
Expand Down
2 changes: 1 addition & 1 deletion esp32-hal/examples/spi_loopback_dma.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ fn main() -> ! {
}
// the buffers and spi is moved into the transfer and we can get it back via
// `wait`
(receive, send, spi) = transfer.wait();
(receive, send, spi) = transfer.wait().unwrap();
println!(
"{:x?} .. {:x?}",
&receive[..10],
Expand Down
10 changes: 5 additions & 5 deletions esp32c2-hal/examples/qspi_flash.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ fn main() -> ! {
zero_buf,
)
.unwrap();
(zero_buf, spi) = transfer.wait();
(zero_buf, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

// erase sector
Expand All @@ -116,7 +116,7 @@ fn main() -> ! {
zero_buf,
)
.unwrap();
(zero_buf, spi) = transfer.wait();
(zero_buf, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

// write enable
Expand All @@ -129,7 +129,7 @@ fn main() -> ! {
zero_buf,
)
.unwrap();
(_, spi) = transfer.wait();
(_, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

// write data / program page
Expand All @@ -144,7 +144,7 @@ fn main() -> ! {
send,
)
.unwrap();
(_, spi) = transfer.wait();
(_, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

loop {
Expand All @@ -162,7 +162,7 @@ fn main() -> ! {
// here we could do something else while DMA transfer is in progress
// the buffers and spi is moved into the transfer and we can get it back via
// `wait`
(receive, spi) = transfer.wait();
(receive, spi) = transfer.wait().unwrap();

println!("{:x?}", &receive);
for b in &mut receive.iter() {
Expand Down
2 changes: 1 addition & 1 deletion esp32c2-hal/examples/spi_loopback_dma.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ fn main() -> ! {
}
// the buffers and spi is moved into the transfer and we can get it back via
// `wait`
(receive, send, spi) = transfer.wait();
(receive, send, spi) = transfer.wait().unwrap();
println!(
"{:x?} .. {:x?}",
&receive[..10],
Expand Down
10 changes: 5 additions & 5 deletions esp32c3-hal/examples/qspi_flash.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn main() -> ! {
zero_buf,
)
.unwrap();
(zero_buf, spi) = transfer.wait();
(zero_buf, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

// erase sector
Expand All @@ -123,7 +123,7 @@ fn main() -> ! {
zero_buf,
)
.unwrap();
(zero_buf, spi) = transfer.wait();
(zero_buf, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

// write enable
Expand All @@ -136,7 +136,7 @@ fn main() -> ! {
zero_buf,
)
.unwrap();
(_, spi) = transfer.wait();
(_, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

// write data / program page
Expand All @@ -151,7 +151,7 @@ fn main() -> ! {
send,
)
.unwrap();
(_, spi) = transfer.wait();
(_, spi) = transfer.wait().unwrap();
delay.delay_ms(250u32);

loop {
Expand All @@ -169,7 +169,7 @@ fn main() -> ! {
// here we could do something else while DMA transfer is in progress
// the buffers and spi is moved into the transfer and we can get it back via
// `wait`
(receive, spi) = transfer.wait();
(receive, spi) = transfer.wait().unwrap();

println!("{:x?}", &receive);
for b in &mut receive.iter() {
Expand Down
2 changes: 1 addition & 1 deletion esp32c3-hal/examples/spi_loopback_dma.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ fn main() -> ! {
}
// the buffers and spi is moved into the transfer and we can get it back via
// `wait`
(receive, send, spi) = transfer.wait();
(receive, send, spi) = transfer.wait().unwrap();
println!(
"{:x?} .. {:x?}",
&receive[..10],
Expand Down
Loading