Skip to content

Add DMAMUX support #26

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 3 commits into from
Aug 11, 2020
Merged

Add DMAMUX support #26

merged 3 commits into from
Aug 11, 2020

Conversation

ijager
Copy link
Contributor

@ijager ijager commented Aug 3, 2020

WIP

This PR will address #12. Integrate support for DMAMUX into the DMA driver.

Steps to use DMA:

DMA peripheral init

Only needs RCC

  • reset peripheral

  • enable DMA clock


DMA Channel configure

  • Pick DMA channel (1..5, 1..7) (Note: DMAMUX starts at zero)

  • Configure DMAMUX dmareq_id with correct index value.

  • Configure DMA channel (dma ccr register):

  • Write peripheral register in DMA

  • Configure memory buffer address (cparx) and length (cndtrx)

  • Enable DMA mode in peripheral


start transfer

  • (update buffer addresses)
  • enable dma channel (ccr reg -> enable bit)

Let's model the DMA driver on stm32f3xx-hal.

Perhaps the DMA Target trait implementations should also configure the DMAMUX, since the peripheral implementation knows which DMAMUX index to set.

/// Trait implemented by DMA targets.
pub trait Target {
    /// Enable DMA on the target
    fn enable_dma(&mut self) {}
    /// Disable DMA on the target
    fn disable_dma(&mut self) {}
}

@andresv
Copy link
Member

andresv commented Aug 3, 2020

Super!

Yes, I also think that Target should configure DMAMUX because index is known.

@ijager
Copy link
Contributor Author

ijager commented Aug 5, 2020

I have something working, but it depends on stm32-rs/stm32-rs#408

I added the link_dma method in the dma::Target trait

/// Trait implemented by DMA targets.
pub trait Target {

    /// Connect dmamux channel to dma channel and peripheral
    ///
    /// The implementation should call
    /// dma_ch.select_peripheral(DmaMuxIndex::XXX)
    /// with the correct DmaMuxIndex for XXX
    fn link_dma<C: Channel>(&mut self, dma_ch: &mut C);

    /// Enable DMA on the target
    fn enable_dma(&mut self) {}
    /// Disable DMA on the target
    fn disable_dma(&mut self) {}
}

Which needs to be implemented by the peripheral driver that wants to use DMA:

impl<Config> dma::Target for Rx<$USARTX, Config> {

    fn link_dma<C: dma::Channel>(&mut self, dma_ch: &mut C) {
        dma_ch.select_peripheral(DmaMuxIndex::USART1_RX)
     }
}

Example of how to use it:

    let (mut tx, _rx) = usart1.split();

    let mut dma = dp.DMA.split(&mut rcc, dp.DMAMUX);

    let usart = unsafe { &(*stm32::USART1::ptr()) };
    let tx_data_register_addr = &usart.tdr as *const _ as u32;
    let tx_dma_buf_addr : u32 = tx_buffer.as_ptr() as u32;

    dma.ch1.set_direction(dma::Direction::FromMemory);
    dma.ch1.set_memory_address(tx_dma_buf_addr, true);
    dma.ch1.set_peripheral_address(tx_data_register_addr, false);
    dma.ch1.set_transfer_length(tx_buffer.len() as u16);

    tx.link_dma(&mut dma.ch1);
    tx.enable_dma();
    dma.ch1.enable();

What do you think?

@andresv
Copy link
Member

andresv commented Aug 5, 2020

I think it is okay. My only concern is about method link_dma name.
Maybe this is more intuitive:

 tx.use_channel(&mut dma.ch1);
 tx.enable_dma();
 dma.ch1.enable();

Or actually it is other way around, we do not want to use channel, channel wants to use peripheral:

 tx.select_peripheral(&mut dma.ch1);
 tx.enable_dma();
 dma.ch1.enable()

But this is also misleading. Basically we would like to configure which peripheral a channel should use, tx has other methods so actually yes _dma should be in this name.

 tx.assign_to_dma(&mut dma.ch1);
// or
 tx.configure_dma(&mut dma.ch1);
 tx.enable_dma();
 dma.ch1.enable();

I think this method name should be more clear, but I do not have a good proposal either. Maybe you have some more ideas how to name it?

@ijager
Copy link
Contributor Author

ijager commented Aug 5, 2020

perhaps

tx.use_dma_channel(&mut dma.ch1);

@andresv
Copy link
Member

andresv commented Aug 5, 2020

What if:

dma.ch1.set_transfer_length(tx_buffer.len() as u16);
dma.ch1.select_peripheral(tx.dmamux());
tx.enable_dma();
dma.ch1.enable();

Implement for USART and add test.
@ijager
Copy link
Contributor Author

ijager commented Aug 6, 2020

So it is

tx.use_dma_channel(&mut dma.ch1);
// vs
dma.ch1.select_peripheral(tx.dmamux());

I like your suggestion since we're configuring dma. Let me try it in the next commit.


writeln!(usart1, "Hello without DMA\r\n").unwrap();

let mut tx_buffer = [0u8; 16];
Copy link
Member

@andresv andresv Aug 6, 2020

Choose a reason for hiding this comment

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

You can do this:

let mut tx_buffer: [u8; 16] = *b"Hello with DMA!\n";

This reverts control flow. The peripheral implements dmamux() that
returns the correct index to configure the channel multiplexing.

Usage: dma.ch1.select_peripheral(tx.dmamux());
@ijager
Copy link
Contributor Author

ijager commented Aug 11, 2020

I think this can be merged, unless there are other issues?

@andresv andresv merged commit ed4a74e into stm32-rs:master Aug 11, 2020
@ijager ijager deleted the dma branch April 13, 2023 12:50
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