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

DMA Transfer consumes peripheral #282

Open
unpaid-bill opened this issue Mar 25, 2021 · 3 comments
Open

DMA Transfer consumes peripheral #282

unpaid-bill opened this issue Mar 25, 2021 · 3 comments

Comments

@unpaid-bill
Copy link

Right now there aren't really any examples of both DMA and peripherals using this HAL, so I might be doing something wrong.
I'm able to both build a DMA Transfer and initiate a peripheral such as the adc using this HAL, but not both at the same time.

// DMA init
let stream_0 = StreamsTuple::new(dp.DMA2).0;
let buf = singleton!(: [u16; 128] = [0; 128]).unwrap();
let dma_config = DmaConfig::default();

let adc_dma_transfer = Transfer::init(stream_0, dp.ADC1, buf, None, dma_config);  //<-- consumes dp.ADC1

// adc init
let adc_config = hal::adc::config::AdcConfig::default();
let adc_periph = Adc::adc1(dp.ADC1, true, adc_config); //<-- cannot init this peripheral. ADC1 has already been moved

I haven't dug too much into the DMA implementation but I don't think it would need ownership of the peripheral.

@unpaid-bill
Copy link
Author

Reading more into it, I understand I could instead hand the dma Transfer a copy of its address with something like

unsafe { &*ADC1::ptr() as *const _ as u32 }

But wouldn't it be more elegant for that to happen inside the dma module and not in the application code?
The stm32f1xx-hal has dma implementations of peripherals within their respective peripheral crate, so this de/referencing happens there. Is that an approach this HAL is looking to move toward?

@thalesfragoso
Copy link
Member

The idea is to implement the necessary abstraction and traits to the Adc HAL type, and pass it to the Transfer API instead of the pac type. It seems that this was done already for serial, but other peripherals are still missing.

@gernoteger
Copy link
Contributor

I just had a look: as of now (0.10.1) the Adc traits seem to be implemented for at least ADC1; nevertheless, the adc hal types are consumed themselves. This renders the calibration data inaccessible, as can be seen in the adc_dma example.
The peripheral in the Transfer structs is passed as &mut to the closures of start,pause, so I don't think we get around that.

A way to fix this special issue could be to expose a reference to the peripheral. This allows us to use for example adc.sample_to_millivolts() wherever the sampled values are consumed.

bors bot added a commit that referenced this issue Jan 10, 2022
396: adc improvements for dma r=burrbull a=gernoteger

This PR should fix adc with dma issues as mentioned in #282: "DMA Transfer consumes peripheral".

Changes:
- DMA Transfer exposes a reference to the peripheral, currently only for PeripheralToMemory
- the ADC provides a closure similar to sample_to_millivolts
- a demonstration of its use in the adc_dma_rtic example.

As of now this PR is not complete; I kindly ask to review the concept before I complete the implementation, documentation and squashing.



Co-authored-by: gernot <[email protected]>
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

No branches or pull requests

3 participants