Skip to content

Clean up DMA code #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Jul 24, 2020
Merged

Clean up DMA code #260

merged 20 commits into from
Jul 24, 2020

Conversation

hannobraun
Copy link
Member

This PR is a meandering journey through DMA, cleaning up what needs cleaning, fixing some things, but not adding any new features. The most significant change is that a panic on LCP845 was fixed.

`Transfer` is the primary concept in this module, so it should come
first.
Those types need to be specified in multiple places (and I'm going to
add one more shortly) and the repetition is getting tedious.
The new name better reflects that it refers to the channel.
So far, only memory-peripheral transfers are supported. I see no reason
to require a mutable buffer for that.
I intend this to become the new style for DMA-based I/O. It's much more
visible than the method on `Channel` and it follows the convention in
other HALs.

Also, the method on `Channel` allows mixing channels and peripherals
without restrictions, and needs to be made crate-private anyway.
It's part of a `Result::Err`, so this is pretty important.
It's an internal type, so it won't do any user-facing harm. It's also
highly useful for a fix that I'm working on.
It is only relevant, if hardware triggers are enabled, so having it here
is misleading.
@hannobraun
Copy link
Member Author

Force-pushed to remove a few commits, make some other slight modifications, and to sneak in another minor commit.

The peripheral-to-memory transfers I'm currently working on seem to be taking a slightly different direction than expected, and some of the changes made here turned out to be counterproductive. @david-sawatzke, I figured making you review these changes, then making you review the changes undoing them in the next PR, would be inconsiderate, so I chose to clean up this pull request instead.

@david-sawatzke
Copy link
Member

(Also not a blocker and would necessitate some api changes, but the limit of 1024 should probably be enforced)

@hannobraun
Copy link
Member Author

(Also not a blocker and would necessitate some api changes, but the limit of 1024 should probably be enforced)

Thanks for the review!

I agree, but I'm going to merge now and address this in #261, to save myself the rebase.

@hannobraun hannobraun merged commit e1f79c1 into lpc-rs:master Jul 24, 2020
@hannobraun hannobraun deleted the dma-cleanup branch July 24, 2020 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants