Skip to content

Split DMA RegisterAccess trait into RX/TX#2249

Merged
MabezDev merged 3 commits into
esp-rs:mainfrom
Dominaezzz:split_dma_trait
Sep 30, 2024
Merged

Split DMA RegisterAccess trait into RX/TX#2249
MabezDev merged 3 commits into
esp-rs:mainfrom
Dominaezzz:split_dma_trait

Conversation

@Dominaezzz
Copy link
Copy Markdown
Collaborator

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the Migration Guide.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

First two steps of #2248 and builds on top of #2247.

The RegisterAccess trait has been split into RxRegisterAccess, TxRegisterAccess and InterruptAccess.
And the new trait now take &self.

Testing

It builds, it's just an internal refactor.

@Dominaezzz Dominaezzz mentioned this pull request Sep 28, 2024
6 tasks
@Dominaezzz Dominaezzz added the skip-changelog No changelog modification needed label Sep 28, 2024
Copy link
Copy Markdown
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

Thanks!

@MabezDev MabezDev added this pull request to the merge queue Sep 30, 2024
Merged via the queue into esp-rs:main with commit f1bedbe Sep 30, 2024
@Dominaezzz Dominaezzz deleted the split_dma_trait branch September 30, 2024 14:52
Comment thread esp-hal/src/dma/mod.rs

self.rx_impl
.prepare_transfer_without_start(preparation.start, peri)
compiler_fence(core::sync::atomic::Ordering::SeqCst);
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.

Why do we need this fence? All operations around here are volatile, so non-reorderable

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Putting aside the "it was there before I touched it"

The fence is to make sure the descriptor and buffer setup (which is non-volatile) happens before the DMA starts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog No changelog modification needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants