Skip to content

cpu/nrf52840: fix UART DMA when data is in ROM#11170

Merged
jcarrano merged 1 commit intoRIOT-OS:masterfrom
haukepetersen:fix_nrf52840_dmafromrom
Mar 13, 2019
Merged

cpu/nrf52840: fix UART DMA when data is in ROM#11170
jcarrano merged 1 commit intoRIOT-OS:masterfrom
haukepetersen:fix_nrf52840_dmafromrom

Conversation

@haukepetersen
Copy link
Copy Markdown
Contributor

Contribution description

When testing #11096, we discovered a bug in the nrf5240s uart implementation: Nordic's EasyDMA is only capable of transferring data from RAM, but not from ROM. So whenever passing a ROM location to the UART driver (e.g. some const char *), the UART would only transfer 0x00 bytes. This made e.g. tests/fmt_print fail for all targets using the nrf5280 cpu.

This PR fixes this issue by chunk-wise copying the TX data into RAM before sending it in case the data is coming from ROM.

Testing procedure

Run e.g. the tests/fmt_print test on the nrf52840dk or the reel or similar. Without this PR the test will fail, with this PR it will succeed.

Issues/PRs references

issue found in #11096

@haukepetersen haukepetersen added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: cpu Area: CPU/MCU ports labels Mar 13, 2019
@haukepetersen haukepetersen requested a review from aabadie March 13, 2019 12:06
@haukepetersen haukepetersen requested a review from jcarrano March 13, 2019 12:07
Copy link
Copy Markdown
Contributor

@jcarrano jcarrano left a comment

Choose a reason for hiding this comment

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

Solves the bug.

* So if the given `data` buffer resides in ROM, we need to copy it to RAM
* before being able to transfer it. To make sure the stack does not
* overflow, we do this chunk-wise. */
if (!((uint32_t)data & RAM_MASK)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!((uint32_t)data & RAM_MASK)) {
if (!((intptr_t)data & RAM_MASK)) {

@jcarrano jcarrano merged commit 4af3548 into RIOT-OS:master Mar 13, 2019
@haukepetersen haukepetersen deleted the fix_nrf52840_dmafromrom branch March 13, 2019 17:05
@danpetry danpetry added this to the Release 2019.04 milestone Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants