Skip to content

Conversation

@peauters
Copy link
Contributor

This is a first draft of what DMA support for SPI would look like.

I don't have a great opinion on how some of the error flags can/should be used in conjunction with DMA transfers and would love some advice.

Once #318 has been merged, I'll update the example to look at the extra error flags to show multiple transfers.

@peauters peauters force-pushed the spi-dma branch 3 times, most recently from 57a4319 to bf03f2a Compare June 19, 2021 13:14
Comment on lines 1109 to 760
pub struct Tx<SPI> {
spi: PhantomData<SPI>,
dr: u32,
}

pub struct Rx<SPI> {
spi: PhantomData<SPI>,
dr: u32,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you bound this SPI to an Instance trait (or other name) with a fn dr() -> u32 method, you could remove this dr field and save some ram. You could then implement this Instance trait for SPI1 (and others) with something like:

unsafe { &(&*SP1::ptr().dr) as *const _ as u32 }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

@peauters peauters marked this pull request as draft June 20, 2021 21:23
@peauters peauters force-pushed the spi-dma branch 2 times, most recently from 172a168 to 8242235 Compare June 21, 2021 00:20
@peauters peauters marked this pull request as ready for review June 21, 2021 00:20
@peauters
Copy link
Contributor Author

I've made the change suggested by Thales.

I'm having trouble getting the example to work with two transfers. It only transfers once. I have it working in my project using rtic, so my guess is that there is some concurrency going on that is whooshing above my head. Any got any ideas what I'm missing?

cortex_m::interrupt::free(|cs| *G_TRANSFER.borrow(cs).borrow_mut() = Some(transfer));
// Enable interrupt
unsafe {
cortex_m::peripheral::NVIC::unmask(stm32::Interrupt::DMA1_STREAM5);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cortex_m::peripheral::NVIC::unmask(stm32::Interrupt::DMA1_STREAM5);
cortex_m::peripheral::NVIC::unmask(stm32::Interrupt::DMA2_STREAM4);

src/spi.rs Outdated
}

fn new_tx(&self) -> Tx<SPI> {
self.spi.cr2.write(|w| w.txdmaen().enabled());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.spi.cr2.write(|w| w.txdmaen().enabled());
self.spi.cr2.modify(|_, w| w.txdmaen().enabled());

src/spi.rs Outdated
}

fn new_rx(self) -> Rx<SPI> {
self.spi.cr2.write(|w| w.rxdmaen().enabled());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.spi.cr2.write(|w| w.rxdmaen().enabled());
self.spi.cr2.modify(|_, w| w.rxdmaen().enabled());

... or else txrx() doesn't work because txdmaen is reset

@burrbull
Copy link
Member

cc @peauters

@peauters
Copy link
Contributor Author

Apologies for leaving this open for so long.

I got diverted off onto another project.

I'll look to get this in to shape in the next week or so.

@kalkyl
Copy link
Contributor

kalkyl commented Oct 5, 2021

Are there any showstoppers left for merging this one?

@burrbull
Copy link
Member

burrbull commented Oct 5, 2021

@kalkyl Can you test this?

@burrbull
Copy link
Member

bors r+

@bors bors bot merged commit b3aab7d into stm32-rs:master Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants