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

Working to impl embedded-dma for SPIM #234

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

Working to impl embedded-dma for SPIM #234

wants to merge 8 commits into from

Conversation

jamesmunns
Copy link
Member

CC @kalkyl, also fixed a SPIS item

@kalkyl
Copy link
Contributor

kalkyl commented Sep 28, 2020

Looks great to me!

Perhaps return the buffers aswell in dma_transfer_split Err?

mut rx_buffer: RxB,
) -> Result<TransferSplit<T, TxB, RxB>, (Self, Error)>
where
TxB: ReadBuffer<Word = TxW>,
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
TxB: ReadBuffer<Word = TxW>,
TxB: ReadBuffer<Word = TxW> + 'static,

Based on the matrix channel discussion today it looks like we need + 'static bounds on these buffers aswell

) -> Result<TransferSplit<T, TxB, RxB>, (Self, Error)>
where
TxB: ReadBuffer<Word = TxW>,
RxB: WriteBuffer<Word = RxW>,
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
RxB: WriteBuffer<Word = RxW>,
RxB: WriteBuffer<Word = RxW> + 'static,

//
// This event is triggered once both transmitting and receiving are
// done.
while !self.is_spi_dma_transfer_complete() {}

Choose a reason for hiding this comment

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

Is there an alternative to spinning here: could we setup an interrupt and go to sleep? I'm not an expert at all so apologies if this is a silly suggestion.

@derekdreery
Copy link

I'm trying to use this SPI master abstraction, but am struggling because power consumption is very important and I can't see a way to disable the SPI in the current API. This patch would solve the problem for me I think.

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.

None yet

3 participants