Skip to content

Commit

Permalink
cpu/stm32/periph/dma: fix dma_resume
Browse files Browse the repository at this point in the history
As implmented, dma_resume assumed that transfers widths were 1 byte and
that the memory address incrmenting was always on and periphial address
incrementing always off. This resulted in memory corruption anytime
these assumptions were not true and a dma was resumed. The DMA module
allows intitiating transfers that did not meet these assumption.

This patch adds proper handling inside dma_resume to safely resume any
transfer. Clearifications and errors are added/fixed in the module's
header file. Also, a few constants are removed from the gobal namespace.
  • Loading branch information
Enoch247 committed Oct 7, 2022
1 parent d6978a9 commit a42f4be
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 12 deletions.
21 changes: 10 additions & 11 deletions cpu/stm32/include/periph/cpu_dma.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*
* @author Hauke Petersen <hauke.petersen@fu-berlin.de>
* @author Vincent Dupont <vincent@otakeys.com>
* @author Joshua DeWeese <jdeweese@primecontrols.com>
*/

#ifndef PERIPH_CPU_DMA_H
Expand Down Expand Up @@ -88,10 +89,8 @@ typedef enum {
* @{
*/
#define DMA_DATA_WIDTH_BYTE (0x00) /**< Byte width */
#define DMA_DATA_WIDTH_HALF_WORD (0x01) /**< Half word width */
#define DMA_DATA_WIDTH_WORD (0x02) /**< Word width */
#define DMA_DATA_WIDTH_MASK (0x03) /**< Width mask */
#define DMA_DATA_WIDTH_SHIFT (0) /**< Width position */
#define DMA_DATA_WIDTH_HALF_WORD (0x01) /**< Half word width (2 bytes)*/
#define DMA_DATA_WIDTH_WORD (0x02) /**< Word width (4 bytes)*/
/** @} */

#ifdef MODULE_PERIPH_DMA
Expand All @@ -115,7 +114,7 @@ void dma_init(void);
* @param[in] chan DMA channel (on stm32f2/4/7, CxS or unused on others)
* @param[in] src source buffer
* @param[out] dst destination buffer
* @param[in] len length to transfer
* @param[in] len number of transfers to perform
* @param[in] mode DMA mode
* @param[in] flags DMA configuration
*
Expand Down Expand Up @@ -153,15 +152,15 @@ void dma_start(dma_t dma);
*
* @param[in] dma logical DMA stream
*
* @return the remaining number of bytes to transfer
* @return the remaining number of transfers to perform
*/
uint16_t dma_suspend(dma_t dma);

/**
* @brief Resume a suspended DMA transfer on a stream
*
* @param[in] dma logical DMA stream
* @param[in] reamaining the remaining number of bytes to transfer
* @param[in] reamaining the remaining number of transfers to perform
*/
void dma_resume(dma_t dma, uint16_t remaining);

Expand All @@ -186,7 +185,7 @@ void dma_wait(dma_t dma);
* @param[in] chan DMA channel (on stm32f2/4/7, CxS or unused on others)
* @param[in] src source buffer
* @param[out] dst destination buffer
* @param[in] len length to transfer
* @param[in] len number of transfers to perform
* @param[in] mode DMA mode
* @param[in] flags DMA configuration
*
Expand All @@ -206,7 +205,7 @@ int dma_configure(dma_t dma, int chan, const volatile void *src, volatile void *
* @param[in] chan DMA channel (on stm32f2/4/7, CxS or unused on others)
* @param[in] periph_addr Peripheral register address
* @param[in] mode DMA direction mode
* @param[in] width DMA transfer width
* @param[in] width DMA transfer width (one of DMA_DATA_WIDTH_*)
* @param[in] inc_periph Increment peripheral address after read/write
*/
void dma_setup(dma_t dma, int chan, void *periph_addr, dma_mode_t mode,
Expand All @@ -217,8 +216,8 @@ void dma_setup(dma_t dma, int chan, void *periph_addr, dma_mode_t mode,
*
* @param[in] dma Logical DMA stream
* @param[in] mem Memory address
* @param[in] len Transfer length
* @param[in] inc_mem Increment the memory address after read/write
* @param[in] len Number of transfers to perform
* @param[in] inc_mem Increment the memory address (by the transfer width) after read/write
*/
void dma_prepare(dma_t dma, void *mem, size_t len, bool incr_mem);

Expand Down
26 changes: 25 additions & 1 deletion cpu/stm32/periph/dma.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* @brief Low-level DMA driver implementation
*
* @author Vincent Dupont <vincent@otakeys.com>
* @author Joshua DeWeese <jdeweese@primecontrols.com>
*
* @}
*/
Expand Down Expand Up @@ -84,6 +85,9 @@
#endif
#endif /* CPU_FAM_STM32F2 || CPU_FAM_STM32F4 || CPU_FAM_STM32F7 */

#define DMA_DATA_WIDTH_MASK (0x03)
#define DMA_DATA_WIDTH_SHIFT (0)

struct dma_ctx {
STM32_DMA_Stream_Type *stream;
mutex_t conf_lock;
Expand Down Expand Up @@ -482,10 +486,30 @@ void dma_resume(dma_t dma, uint16_t remaining)
int stream_n = dma_config[dma].stream;
STM32_DMA_Stream_Type *stream = dma_ctx[dma].stream;

#if CPU_FAM_STM32F2 || CPU_FAM_STM32F4 || CPU_FAM_STM32F7
const bool mem_inc = stream->CONTROL_REG & DMA_SxCR_MINC;
const bool periph_inc = stream->CONTROL_REG & DMA_SxCR_PINC;
const int msize_reg =
(stream->CONTROL_REG & DMA_SxCR_MSIZE) >> DMA_SxCR_MSIZE_Pos;
const int psize_reg =
(stream->CONTROL_REG & DMA_SxCR_MSIZE) >> DMA_SxCR_MSIZE_Pos;
#else
const bool mem_inc = stream->CONTROL_REG & DMA_CCR_MINC;
const bool periph_inc = stream->CONTROL_REG & DMA_CCR_PINC;
const int msize_reg =
(stream->CONTROL_REG & DMA_CCR_MSIZE) >> DMA_CCR_MSIZE_Pos;
const int psize_reg =
(stream->CONTROL_REG & DMA_CCR_PSIZE) >> DMA_CCR_PSIZE_Pos;
#endif

const int mpitch = (mem_inc) ? msize_reg + 1 : 0;
const int ppitch = (periph_inc) ? psize_reg + 1 : 0;

if (remaining > 0) {
dma_isr_enable(stream_n);
stream->NDTR_REG = remaining;
stream->MEM_ADDR += dma_ctx[dma].len - remaining;
stream->MEM_ADDR += mpitch * (dma_ctx[dma].len - remaining);
stream->PERIPH_ADDR += ppitch * (dma_ctx[dma].len - remaining);
dma_ctx[dma].len = remaining;
stream->CONTROL_REG |= DMA_EN;
}
Expand Down

0 comments on commit a42f4be

Please sign in to comment.