-
Notifications
You must be signed in to change notification settings - Fork 210
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
Allow DMA on both halves of a split Serial<USART> separately. #193
Conversation
079cdbf
to
c2035ee
Compare
@bobmcwhirter Mind rebasing this so we can have a look at the gist of this change? |
Will do shortly.
…On Fri, Jul 31, 2020 at 5:46 AM Daniel Egger ***@***.***> wrote:
@bobmcwhirter <https://github.com/bobmcwhirter> Mind rebasing this so we
can have a look at the gist of this change?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#193 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAD4TY3KQLM5DY3JXR5OJ3R6KHH5ANCNFSM4PK3JLKA>
.
|
c2035ee
to
9b44cf7
Compare
rebased/repushed. |
Hmm, seems like the first commit is undoing one of my last changes. |
I probably screwed up my rebase I guess. Will fix Monday.
…On Fri, Jul 31, 2020 at 7:39 PM Thales ***@***.***> wrote:
Hmm, seems like the first commit is undoing one of my last changes.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#193 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAD4T2GWJ2WWEZPQTQ6QO3R6NIZFANCNFSM4PK3JLKA>
.
|
Provide DmaConfig enum for setting up DMA on the Tx or Rx half of the serial during init.
9b44cf7
to
7d80205
Compare
Hopefully this looks better. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR, I just left a small nitpick, and another thing, have you tested this on hardware ?
src/serial.rs
Outdated
.dmat().enabled() | ||
}) | ||
} | ||
_ => {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be DmaConfig::None
instead ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point. I'll modify.
wrt hardware, yes, on an stm32f401re against my USART6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again! @therealprof I think this is good to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Thanks for the addtion @bobmcwhirter. Thanks for the review @thalesfragoso.
bors r+
Based upon and dependent upon #186