Skip to content
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

impl waveform with TIM OC Channel DMA #2410

Merged
merged 3 commits into from
Feb 1, 2024

Conversation

eZioPan
Copy link
Contributor

@eZioPan eZioPan commented Jan 6, 2024

Notice:

  • Remove TIM6 DMA exclusion from build.rs, though a few chip still need extra setup to guaranty TIM DMA actually work, (won't implement until someone actually encounter this problem).
  • Rename .gen_waveform() to .waveform_up(), to match up with new .waveform_chx()

P.S.
I have to copy&paste™ entire "waveform for UP" implement, and do a few modification. It's too tricky to exclude some code with a $()? inside a MacroTranscriber:
.waveform_up() requires user input a timer::Channel, but .waveform_chx() don't. I don't know how to achieve this inside one macro.

@eZioPan eZioPan marked this pull request as ready for review January 6, 2024 14:51
@eZioPan eZioPan marked this pull request as draft January 8, 2024 03:07
@eZioPan eZioPan marked this pull request as ready for review January 8, 2024 11:30
Comment on lines +285 to +302
if !original_enable_state {
self.disable(cc_channel);
}

self.set_duty(cc_channel, original_duty_state);

// Since DMA is closed before timer Capture Compare Event trigger DMA is turn off,
// this can almost always trigger a DMA FIFO error.
//
// optional TODO:
// clean FEIF after disable UDE
if !original_cc_dma_enabled {
self.inner.set_cc_dma_enable_state(cc_channel, false);
}

if !original_cc_dma_on_update {
self.inner.set_cc_dma_selection(Ccds::ONCOMPARE)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This also would apply to your original PR, but I think this needs special logic to also apply on Drop: if one uses e.g. embassy_time::with_timeout, the Transfer will be canceled as part of Transfer::drop, but this will not. Rust's std often creates a simple struct that executes a bunch of code in its drop handler to solve this problem, but you could probably make one that takes a closure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, but we still need to wait dma::Transfer somehow expose a function to clean the flag.

@lights0123
Copy link
Contributor

And what's the point of the macro again? Are there some DMA channels that only work with some timer channels?

@eZioPan
Copy link
Contributor Author

eZioPan commented Jan 10, 2024

Are there some DMA channels that only work with some timer channels?

At least for F4 MCU, the answer is yes. ST seems "randomly pick" the DMA channel mapping for TIMx_CHy. Each TIM has one TIMx_UP DMA Channel, but (at most?) 4 TIMx_CHy Channel.

Copy link
Member

@Dirbaio Dirbaio left a comment

Choose a reason for hiding this comment

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

thanks!

@Dirbaio Dirbaio added this pull request to the merge queue Feb 1, 2024
Merged via the queue into embassy-rs:main with commit 7e02389 Feb 1, 2024
@eZioPan eZioPan deleted the waveform-on-CHx branch February 1, 2024 02:02
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.

3 participants