Msp432: DMA-support

[lebakassemmerl]

Pull Request Overview

This PR adds DMA-support, which will be used with UART TX and RX and later with SPI, ADC, etc.
This is just a first version where only UART TX is supported.

Testing Strategy

Testing with UART on eval-board

Help Wanted

• It would be nice to get some feedback if the interface-definitions make sense how they are now implemented
• Any recommendations/ideas how to better align the DMA_CONFIG struct, I just wanted to make a standard array and no struct, but I didn't find anything to align arrays in rust.

TODO

• implement transfer_mem_to_peripheral() for UART TX
• implement transfer_peripheral_to_mem() for UART RX
• implement transfer_mem_to_mem()

Documentation Updated

n/a

Formatting

Ran make ci-nosetup

[ppannuto]

The changes in these files look pretty separate from the DMA stuff. In general, we try to keep PRs focused to make them easier to review. To the extent reasonable, more, smaller PRs are better than big ones. These changes also look like stuff that can/should sail through quickly as they're general platform bugfixes, while the DMA will probably take some time to review. Can you pull these out?

[lebakassemmerl]

The changes in these files look pretty separate from the DMA stuff. In general, we try to keep PRs focused to make them easier to review. To the extent reasonable, more, smaller PRs are better than big ones. These changes also look like stuff that can/should sail through quickly as they're general platform bugfixes, while the DMA will probably take some time to review. Can you pull these out?

Sure!
So then I guess it's the best to just create an own platform-fix PR with these changes, right?

[hudson-ayers]

So then I guess it's the best to just create an own platform-fix PR with these changes, right?

Yep!

[alistair23]

Ack.

I didn't check with the datasheet, but overall this looks good.

[lebakassemmerl]

I also implemented already an abort_transfer() function, which stops any active DMA transfers. My plan was to use in the transmit_abort() and receive_abort() function from the UART traits. Should I add these changes here? Or should I create another PR with them?

An do you have any recommendations how to test the UART abort-functions? I actually don't really know how to trigger them.

[alistair23]

It should probably be a separate PR.

I have no idea how to test it though

[lebakassemmerl]

Am I supposed to change something or add any missing features here? Or is it just necessary to wait for some more feedback? (:

[ppannuto]

There is a lot of unsafe code in this driver. I think that all traces back to the static mut DMA_CONFIG region. I think it would be more appropriate to model this using the InMemoryRegister abstraction (see chips/sam4l/src/usbc/mod.rs for an example).

This comes with several advantages:

• It should encapsulate unsafe to the single instantiation of the 'register-like' memory, rather than every access.
• It will treat the memory accesses as volatile, which in the case of status registers is important since I believe the DMA engine itself can write to the memory addresses
• It will remove all of the manually-authored bit manipulation code in favor of the register abstraction

[ppannuto]

(sorry for the slow-ish review; summer distractions :) )

[lebakassemmerl]

There is a lot of unsafe code in this driver. I think that all traces back to the static mut DMA_CONFIG region. I think it would be more appropriate to model this using the InMemoryRegister abstraction (see chips/sam4l/src/usbc/mod.rs for an example).

This comes with several advantages:

* It should encapsulate `unsafe` to the single instantiation of the 'register-like' memory, rather than every access.

* It will treat the memory accesses as volatile, which in the case of status registers is important since I believe the DMA engine itself can write to the memory addresses

* It will remove all of the manually-authored bit manipulation code in favor of the register abstraction

Ok, thanks for the info! I was actually searching for something like that but I didn't know what I had to look for (:
Then I'll convert the code to use the InMemoryRegister model.

[hudson-ayers]

I haven't looked at the datasheet, but I reviewed the unsafe uses and skimmed most of the code and feel reasonably comfortable with it.

Making DMA_CHANNELS a struct with a handle_interrupt() method, rather than relying on a global function, might be a little more in line with what we do elsewhere (GPIO_PORT, etc.), but I am not sure we need to block on that.

[ppannuto]

Thanks for iterating on this a few times with us! Really exciting to have DMA on a new platform 👍

[lebakassemmerl]

Thanks for iterating on this a few times with us! Really exciting to have DMA on a new platform +1

Of course, no problem! I'm also very excited to see new features being accepted in the main-tree (:

[ppannuto]

bors r+

[bors]

Build succeeded:

• ci-build (macos-latest)
• ci-build (ubuntu-latest)
• ci-format (macos-latest)
• ci-format (ubuntu-latest)
• ci-qemu
• ci-tests (macos-latest)
• ci-tests (ubuntu-latest)