nrf52: Implement EasyDMA-based SPI peripheral implemenation#14057
Merged
benpicco merged 16 commits intoRIOT-OS:masterfrom May 18, 2020
Merged
nrf52: Implement EasyDMA-based SPI peripheral implemenation#14057benpicco merged 16 commits intoRIOT-OS:masterfrom
benpicco merged 16 commits intoRIOT-OS:masterfrom
Conversation
bergzand
added
Type: enhancement
bergzand
requested review from
MrKevinWeiss,
aabadie and
haukepetersen
as code owners
bergzand
force-pushed
the
pr/nrf52/dma_spi
branch
3 times, most recently
from
b520aca to
48d72ef
Compare
benpicco
reviewed
May 11, 2020
Contributor
benpicco
left a comment
benpicco
left a comment
There was a problem hiding this comment.
Looking good - some small comments.
bergzand
commented
May 12, 2020
benpicco
reviewed
May 12, 2020
benpicco
reviewed
May 12, 2020
Member
Author
|
@benpicco I've modified the code to minimize the number of ifdefs and let the compiler handle removing any resulting dead code. |
benpicco
reviewed
May 12, 2020
benpicco
reviewed
May 12, 2020
benpicco
reviewed
May 12, 2020
bergzand
removed
the
CI: ready for build
bergzand
force-pushed
the
pr/nrf52/dma_spi
branch
from
ed6e7f5 to
4f2db36
Compare
Member
Author
|
Rebased on top of #14061 (and now waiting on that PR) |
bergzand
added
the
State: waiting for other PR
benpicco
removed
the
State: waiting for other PR
bergzand
force-pushed
the
pr/nrf52/dma_spi
branch
from
1b5c543 to
ce8c795
Compare
Member
Author
|
Rebased! |
bergzand
added
the
CI: ready for build
benpicco
reviewed
May 12, 2020
benpicco
reviewed
May 12, 2020
benpicco
reviewed
May 18, 2020
Contributor
benpicco
left a comment
benpicco
left a comment
There was a problem hiding this comment.
Looks good to me - just some minor nits.
You can squash directly.
bergzand
force-pushed
the
pr/nrf52/dma_spi
branch
from
4b6920e to
d0047dd
Compare
Member
Author
|
Squashed! |
Member
Author
|
Ho, That rebase is not how it should be, give me a second for a next attempt |
bergzand
force-pushed
the
pr/nrf52/dma_spi
branch
from
d0047dd to
e76ee57
Compare
Member
Author
|
Should be good now, at least the content of the commits now matches their description |
Member
Author
|
Thanks for the review @benpicco! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
This PR splits the common nrf5x SPI peripheral into the old (,and for the nrf52 deprecated peripheral,) nRF51 driver and a new EasyDMA-based driver for the nRF52. The new driver is significantly faster than the old one as the bytes can be send back-to-back without overhead from the CPU.
I decided to split the nrf5x common peripheral driver, it became too much of an ifdef hell to keep them merged.
The peripheral driver requires a GPIO interrupt on the nrf52832 due to errata 58. The proposed driver uses an adapted form of the workaround presented by Nordic. This anomaly also exists on engineering sample A of the nrf52840. I decided not to support that to shave off some additional flash usage and runtime checks. This might be an issue on the nrf52840-pdk boards.
Testing procedure
Compilation should verify that all nrf52 boards have been adapted, the board config requires modification from
NRF_SPI#toNRF_SPIM#. Maybe @MrKevinWeiss can give this a try on the RobotFW tests to verify the runtime behaviour.Issues/PRs references
None