imxrt-rs / imxrt-hal

Rust for NXP i.MX RT
Apache License 2.0
133 stars 33 forks source link

DMA support for all chip variants #86

Closed mciantyre closed 3 years ago

mciantyre commented 3 years ago

The PR provides DMA support for all chip variants. DMA support is probably lower on #56's TODO list. However, the DMA implementation is more unique than the other peripherals: the RAL interface was hand-written. Hand-writing the RAL was necessary for providing a simpler API, since the svd2ral script generated a lot of symbols.

I arrived at this implementation by studying the reference manuals. In summary, the 1011 chip only has 16 DMA channels; chips that are 1015 and higher hav 32 DMA channels. When you have more than 16 DMA channels, you have the option for channel grouping. Today's DMA driver doesn't support channel grouping, so this is only a comment in the code.

The conditional compiles are hidden behind a new chip module, private to the DMA driver. We use the common symbols from the chip module throughout the implementation.

This is a backwards-compatible change, at least in the short term. We now export a new constant, CHANNEL_COUNT, which signals the number of DMA channels supported in the implementation. Crate users should favor that constant instead of hard-coded 32s.

mciantyre commented 3 years ago

Draft until we figure out

let mut dma_channels = peripherals.dma.clock(&mut peripherals.ccm.handle);

let tx_channel = dma_channels[9].take().unwrap();  // OK; 9 < 16
let mut rx_channel = dma_channels[25].take().unwrap(); // Compiler error, 25 > 16, won't work for 1011

I'm thinking that CHANNEL_COUNT is 32 by default. Since the 1011 is the only chip that has 16 channels, it'll be the exception.

For the second case, I propose that we always return an array of 32 DMA channels. But, only the lower CHANNEL_COUNT channels are initialized to Some(channel). If a user wants to remain portable across all i.MX RT variants, they will need to check for None in the upper half of the array.

We could also emit a compile-time configuration, named something like imxrt_hal_dma_limited_channels, that users could check for conditionally enabling code, or raise a compile-time error if they really need all the channels.

// In code that depends on imxrt-hal...

#[cfg(imxrt_hal_dma_limited_channels)]
compile_error!("This library requires the i.MX RT to have 32 DMA channels!");

fn do_dma_things() { /* ... */ }

I've not implemented that, because I've no easy way to test it at the moment. I'd also like to look up any best practices on this pattern. (I really want if constexpr and static_assert from C++, so users could check CHANNEL_COUNT for the same outcome...)

teburd commented 3 years ago

I think that makes sense