Initial DMA support #86
Conversation
|
Thank you very much for this PR and providing so much extra information in #66. Now I have enough information to read into. I'll try to review this in the next days :) |
|
Thanks! Btw. I opened an issue about the use of Edit: Switched from |
|
Am I right, that this is on hold as long as this is still in progress? :) |
|
Not necessarily. The final goal of this project is to update the Embedonomicon, we don't need to wait for that. I think the discussion around the buffer type is most relevant for this PR and so far as I'm aware, the current proposal is sound. At least nobody has raised any issues with it yet. So I would suggest I update this PR to make use of the suggested traits and then we try to get it merged. That way we can also get some practical experience with these traits, which we should before we recommend them for everyone. |
|
I'd say it's ready for review now. |
|
Hey @Sh3Rm4n, I don't want to be a bother but that has been waiting for a while. Anything I can do to help move it forward? |
This only includes support for one-shot DMA transfers on STM32f303 MCUs for now.
This better reflects what that function is doing, namely reading as many bytes as there is space in the given buffer.
Using Pin for DMA buffers does not provide the necessary safety. See rust-embedded/embedonomicon#64
This commit migrates the current DMA implementation to the safe DMA buffer traits proposed by the "DMA API Documentation" focus project. See: https://github.com/ra-kete/dma-poc/blob/master/README.md
|
Oh, I forgot something, maybe we should have a method on the |
I looked into that, but it's not easy to implement nicely. The naive transformation would yield something like: /// Trait for defining DMA targets and their channels
pub unsafe trait Target<C: Channel> {
fn peripheral_address() -> u32;
}
macro_rules! targets {
(
$dma:ident,
$( $target:ty => ($C:ident, $reg:expr), )+
) => {
$(
unsafe impl Target<$dma::$C> for $target {
fn peripheral_address() -> u32 {
unsafe { &$reg as *const _ as u32 }
}
}
)+
};
}
#[cfg(feature = "stm32f303")]
targets!(dma1,
serial::Rx<stm32::USART1> => (C5, (*stm32::USART1::ptr()).rdr),
serial::Tx<stm32::USART1> => (C4, (*stm32::USART1::ptr()).tdr),
serial::Rx<stm32::USART2> => (C6, (*stm32::USART2::ptr()).rdr),
serial::Tx<stm32::USART2> => (C7, (*stm32::USART2::ptr()).tdr),
serial::Rx<stm32::USART3> => (C3, (*stm32::USART3::ptr()).rdr),
serial::Tx<stm32::USART3> => (C2, (*stm32::USART3::ptr()).tdr),
);Which I don't like much, since (aside from being ugly) I don't think the DMA module should concern itself with the details of specific USART registers. Such low-level knowledge is better kept in the serial module IMO is. Now, this could be resolved with an additional I think at some point, when DMA is supported for more than just serial, we'll want to move more setup logic into the |
Agreed. |
The ordering previously used here (Acquire) was not strong enough, since it only prevents memory reads after the fence to be moved before the last read before the fence. We want to prevent moving before the last *write* before the fence (the one that disables DMA). The SeqCst ordering does that for us.
Instead of always enabling DMA on the USART peripherals, we want to enable it only when we actually do a transfer. To this end, this commit renames the `Target` trait mapping DMA targets to their channels to `OnChannel`. The new `Target` trait now provides methods to start and stop DMA on the implementing target, which are called when a `Transfer` starts or stops DMA.
This PR is the first part of an effort to add DMA support to this crate (see issue #66). It adds initial support for DMA, including:
DmaExtextension trait that splits a DMA peripheral into individual channelsTransferabstraction that provides memory-safe one-shot DMA transfersThis works only on STM32F303 devices for now.