pkarashchenko commented on code in PR #6614: URL: https://github.com/apache/incubator-nuttx/pull/6614#discussion_r922177086
########## arch/arm/src/samv7/sam_xdmac.c: ########## @@ -1921,6 +1936,130 @@ int sam_dmarxsetup(DMA_HANDLE handle, uint32_t paddr, uint32_t maddr, return ret; } +/**************************************************************************** + * Name: sam_dmarxsetup_circular + * + * Description: + * Configure DMA for receipt of two circular buffers for peripheral to + * memory transfer. Function sam_dmastart_circular() needs to be called + * to start the transfer. Only peripheral to memory transfer is currently + * supported. + * + * Input Parameters: + * handle - DMA handler + * descr - array with DMA descriptors + * maddr - array of memory addresses (i.e. destination addresses) + * paddr - peripheral address (i.e. source address) + * nbytes - number of bytes to transfer + * ndescrs - number of descriptors (i.e. the lenght of descr array) + * + ****************************************************************************/ + +int sam_dmarxsetup_circular(DMA_HANDLE handle, + struct chnext_view1_s *descr[], + uint32_t maddr[], + uint32_t paddr, + size_t nbytes, + uint8_t ndescrs) +{ + struct sam_xdmach_s *xdmach = (struct sam_xdmach_s *)handle; + uint32_t cubc; + uint8_t nextdescr; + int i; + + /* Set circular as true */ + + xdmach->circular = true; + + xdmach->cc = sam_rxcc(xdmach); + + /* Calculate the number of transfers for CUBC */ + + cubc = sam_cubc(xdmach, nbytes); + cubc |= (CHNEXT_UBC_NDE | CHNEXT_UBC_NVIEW_1 | CHNEXT_UBC_NDEN | + CHNEXT_UBC_NSEN); + + nextdescr = 0; + + for (i = ndescrs - 1; i >= 0; i--) + { + descr[i]->cnda = (uint32_t)descr[nextdescr]; /* Next Descriptor Address */ + descr[i]->cubc = cubc; /* Channel Microblock Control Register */ + descr[i]->csa = paddr; /* Source address */ + descr[i]->cda = maddr[i]; /* Destination address */ + + /* Clean data cache */ + + up_clean_dcache((uintptr_t)descr[i], + (uintptr_t)descr[i] + + sizeof(struct chnext_view1_s)); + up_clean_dcache((uintptr_t)maddr[i], (uintptr_t)(maddr + nbytes)); Review Comment: This line is not ok. We need need ```suggestion up_clean_dcache((uintptr_t)maddr[i], (uintptr_t)(maddr[i] + nbytes)); ``` ########## arch/arm/src/samv7/sam_serial.c: ########## @@ -46,14 +46,83 @@ #include "arm_internal.h" #include "sam_config.h" + +#include "hardware/sam_pinmap.h" #include "hardware/sam_uart.h" +#include "hardware/sam_xdmac.h" +#include "sam_xdmac.h" +#include "sam_gpio.h" +#include "sam_serial.h" /**************************************************************************** * Pre-processor Definitions ****************************************************************************/ #ifdef USE_SERIALDRIVER +/* DMA Configuration */ + +#if defined(CONFIG_USART0_RXDMA) || defined(CONFIG_USART1_RXDMA) || \ + defined(CONFIG_USART2_RXDMA) +# define SERIAL_HAVE_RXDMA +#else +# undef SERIAL_HAVE_RXDMA +#endif + +#if defined(CONFIG_UART0_RXDMA) || defined(CONFIG_UART1_RXDMA) || \ + defined(CONFIG_UART2_RXDMA) || defined(CONFIG_UART3_RXDMA) || \ + defined(CONFIG_UART4_RXDMA) +# warning RX DMA is currently supported only for USART driver. +#endif + +#if defined(SERIAL_HAVE_RXDMA) && !defined(CONFIG_SAMV7_XDMAC) +# error SERIAL DMA requires CONFIG_SAMV7_XDMAC to be selected +#endif + +#ifdef SERIAL_HAVE_CONSOLE_RXDMA +# error RX DMA for serial console is currently not supported. +#endif + +#if defined(CONFIG_UART0_TXDMA) || defined(CONFIG_UART1_TXDMA) || \ + defined(CONFIG_UART2_TXDMA) || defined(CONFIG_UART3_TXDMA) || \ + defined(CONFIG_UART4_TXDMA) || defined(CONFIG_USART0_TXDMA) || \ + defined(CONFIG_USART1_TXDMA) || defined(CONFIG_USART2_TXDMA) +# warning TX DMA is currently not supported. +#endif + +#ifndef CONFIG_SAMV7_SERIAL_DMA_TIMEOUT +# define CONFIG_SAMV7_SERIAL_DMA_TIMEOUT 0 +#endif + +#ifdef SERIAL_HAVE_RXDMA + +# define DMA_RXFLAGS (DMACH_FLAG_FIFOCFG_LARGEST | \ + DMACH_FLAG_PERIPHH2SEL | DMACH_FLAG_PERIPHISPERIPH | \ + DMACH_FLAG_PERIPHWIDTH_32BITS | DMACH_FLAG_PERIPHCHUNKSIZE_1 | \ + DMACH_FLAG_MEMPID_MAX | DMACH_FLAG_MEMAHB_AHB_IF0 | \ + DMACH_FLAG_PERIPHAHB_AHB_IF1 | DMACH_FLAG_MEMWIDTH_32BITS | \ + DMACH_FLAG_MEMINCREMENT | DMACH_FLAG_MEMCHUNKSIZE_1 | \ + DMACH_FLAG_MEMBURST_1) + +/* The DMA buffer size when using RX DMA to emulate a FIFO. + * + * When streaming data, the generic serial layer will be called + * every time the FIFO receives half this number of bytes. + */ + +# ifndef CONFIG_SAMV7_SERIAL_RXDMA_BUFFER +# define CONFIG_SAMV7_SERIAL_RXDMA_BUFFER 128 +# endif +#define RXDMA_CACHE_SIZE ARMV7M_DCACHE_LINESIZE/sizeof(uint32_t) Review Comment: ```suggestion #define RXDMA_CACHE_SIZE (ARMV7M_DCACHE_LINESIZE/sizeof(uint32_t)) ``` ########## arch/arm/src/samv7/sam_serial.c: ########## @@ -1344,11 +1801,139 @@ static void sam_rxint(struct uart_dev_s *dev, bool enable) * ****************************************************************************/ +#ifdef SERIAL_HAVE_NODMA_OPS static bool sam_rxavailable(struct uart_dev_s *dev) { struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; return ((sam_serialin(priv, SAM_UART_SR_OFFSET) & UART_INT_RXRDY) != 0); } +#endif + +/**************************************************************************** + * Name: sam_dma_receive + * + * Description: + * Called (usually) from the interrupt level to receive one + * character from the USART. Error bits associated with the + * receipt are provided in the return 'status'. + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static int sam_dma_receive(struct uart_dev_s *dev, unsigned int *status) +{ + struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; + int c = 0; + + *status = priv->sr; + priv->sr = 0; + + /* Read character from the RX FIFO */ + + c = priv->rxbuf[priv->buf_idx][priv->rxdmanext] & UART_RHR_RXCHR_MASK; + priv->rxdmanext++; + + if ((priv->rxdmanext) == RXDMA_BUFFER_SIZE) + { + priv->rxdmanext = 0; + priv->nextcache = 0; + priv->odd = !priv->odd; + priv->buf_idx = 1 & ~(priv->buf_idx); + } + + return c; +} +#endif + +/**************************************************************************** + * Name: sam_dma_rxint + * + * Description: + * Call to enable or disable RX interrupts + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static void sam_dma_rxint(struct uart_dev_s *dev, bool enable) +{ + struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; + + /* En/disable DMA reception. + * + * Note that it is not safe to check for available bytes and immediately + * pass them to uart_recvchars as that could potentially recurse back + * to us again. Instead, bytes must wait until the next sam_dma_poll or + * DMA event. + */ + + if (priv->rxenable != enable) + { + priv->rxenable = enable; + + if (enable) + { + sam_serialout(priv, SAM_UART_IER_OFFSET, UART_INT_TIMEOUT); + } + else + { + sam_serialout(priv, SAM_UART_IDR_OFFSET, UART_INT_TIMEOUT); + } + } +} +#endif + +/**************************************************************************** + * Name: sam_dma_rxavailable + * + * Description: + * Return true if there are some data in the buffer we can read. Also takes + * care of invalidating data cache. + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static bool sam_dma_rxavailable(struct uart_dev_s *dev) +{ + struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; + uint32_t nextrx; + bool ret; + + ret = false; + + /* Get the current DMA pointer */ + + nextrx = sam_dma_nextrx(priv); + + /* Compare our receive pointer to the current DMA pointer, if they + * do match, then there are bytes to be received. + */ + + if ((nextrx != priv->rxdmanext) && priv->rxenable) + { + /* Invalidate data cache if necessary. This basically ensures + * we invalidate only that part of cache we need to. + */ + + if (priv->nextcache < nextrx) + { + up_invalidate_dcache((uintptr_t)priv->rxbuf[priv->buf_idx] \ + + (priv->nextcache << 2), + (uintptr_t)priv->rxbuf[priv->buf_idx] \ Review Comment: ditto ########## arch/arm/src/samv7/sam_serial.c: ########## @@ -879,10 +1168,109 @@ static int sam_setup(struct uart_dev_s *dev) /* Enable receiver & transmitter */ sam_serialout(priv, SAM_UART_CR_OFFSET, (UART_CR_RXEN | UART_CR_TXEN)); + +#ifdef SERIAL_HAVE_RS485 + if (priv->has_rs485) + { + sam_configgpio(priv->rs485_dir_gpio); + + regval = sam_serialin(priv, SAM_UART_MR_OFFSET); + regval |= UART_MR_MODE_RS485; + sam_serialout(priv, SAM_UART_MR_OFFSET, regval); + } +#endif +#endif + + return OK; +} + +/**************************************************************************** + * Name: sam_dma_setup + * + * Description: + * Configure the UART (USART) baud, bits, parity, etc. This method is + * called the first time that the serial port is opened. + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static int sam_dma_setup(struct uart_dev_s *dev) +{ + struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; + int result; + + /* Do the basic UART setup first, unless we are the console */ + + if (!dev->isconsole) + { + result = sam_setup(dev); + if (result != OK) + { + return result; + } + } + +#ifdef SERIAL_HAVE_RXDMA + /* Acquire the DMA channel. This should always succeed. */ + + if (priv->has_rxdma) + { + /* Do this only if this peripheral has RX DMA. This might not be + * neccessary at current state of the driver as sam_dma_setup() + * cannot be called if USARTn does not have RX DMA but it will be + * needed when TX DMA is implemented. The sam_dma_setup() can be + * called even when TX DMA is defined and RX DMA is not. + */ + + priv->rxdma = sam_dmachannel(0, DMA_RXFLAGS | \ Review Comment: not needed, but we can keep it ########## arch/arm/src/samv7/sam_serial.c: ########## @@ -1344,11 +1801,139 @@ static void sam_rxint(struct uart_dev_s *dev, bool enable) * ****************************************************************************/ +#ifdef SERIAL_HAVE_NODMA_OPS static bool sam_rxavailable(struct uart_dev_s *dev) { struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; return ((sam_serialin(priv, SAM_UART_SR_OFFSET) & UART_INT_RXRDY) != 0); } +#endif + +/**************************************************************************** + * Name: sam_dma_receive + * + * Description: + * Called (usually) from the interrupt level to receive one + * character from the USART. Error bits associated with the + * receipt are provided in the return 'status'. + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static int sam_dma_receive(struct uart_dev_s *dev, unsigned int *status) +{ + struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; + int c = 0; + + *status = priv->sr; + priv->sr = 0; + + /* Read character from the RX FIFO */ + + c = priv->rxbuf[priv->buf_idx][priv->rxdmanext] & UART_RHR_RXCHR_MASK; + priv->rxdmanext++; + + if ((priv->rxdmanext) == RXDMA_BUFFER_SIZE) Review Comment: optional ########## arch/arm/src/samv7/sam_serial.c: ########## @@ -715,6 +954,56 @@ static void sam_disableallints(struct sam_dev_s *priv, uint32_t *imr) leave_critical_section(flags); } +/**************************************************************************** + * Name: sam_dma_nextrx + * + * Description: + * Returns the index into the RX FIFO where the DMA will place the next + * byte that it receives. + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static int sam_dma_nextrx(struct sam_dev_s *priv) +{ + uint32_t cda; + uint32_t ret; + + /* Get the DMA destination address */ + + cda = sam_destaddr(priv->rxdma); + + if (!priv->odd) + { + ret = (cda - (uint32_t)priv->rxbuf[0]) / sizeof(uint32_t); + if (cda >= ((uint32_t)priv->rxbuf[0] + (RXDMA_BUFFER_SIZE << 2))) + { + /* We are already in another buffer so we need to read the + * whole RXDMA_BUFFER_SIZE. + */ + + ret = RXDMA_BUFFER_SIZE; + } + + return ret; + } + else + { + ret = (cda - (uint32_t)priv->rxbuf[1]) / sizeof(uint32_t); + if (cda < (uint32_t)priv->rxbuf[1]) Review Comment: ditto ########## arch/arm/src/samv7/sam_serial.c: ########## @@ -715,6 +954,56 @@ static void sam_disableallints(struct sam_dev_s *priv, uint32_t *imr) leave_critical_section(flags); } +/**************************************************************************** + * Name: sam_dma_nextrx + * + * Description: + * Returns the index into the RX FIFO where the DMA will place the next + * byte that it receives. + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static int sam_dma_nextrx(struct sam_dev_s *priv) +{ + uint32_t cda; + uint32_t ret; + + /* Get the DMA destination address */ + + cda = sam_destaddr(priv->rxdma); + + if (!priv->odd) + { + ret = (cda - (uint32_t)priv->rxbuf[0]) / sizeof(uint32_t); + if (cda >= ((uint32_t)priv->rxbuf[0] + (RXDMA_BUFFER_SIZE << 2))) Review Comment: My bad here, I thought that `rxbuf[0]` is already `uint32_t`, but it is `uint32_t *`. ########## arch/arm/src/samv7/sam_serial.c: ########## @@ -1344,11 +1801,139 @@ static void sam_rxint(struct uart_dev_s *dev, bool enable) * ****************************************************************************/ +#ifdef SERIAL_HAVE_NODMA_OPS static bool sam_rxavailable(struct uart_dev_s *dev) { struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; return ((sam_serialin(priv, SAM_UART_SR_OFFSET) & UART_INT_RXRDY) != 0); } +#endif + +/**************************************************************************** + * Name: sam_dma_receive + * + * Description: + * Called (usually) from the interrupt level to receive one + * character from the USART. Error bits associated with the + * receipt are provided in the return 'status'. + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static int sam_dma_receive(struct uart_dev_s *dev, unsigned int *status) +{ + struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; + int c = 0; + + *status = priv->sr; + priv->sr = 0; + + /* Read character from the RX FIFO */ + + c = priv->rxbuf[priv->buf_idx][priv->rxdmanext] & UART_RHR_RXCHR_MASK; + priv->rxdmanext++; + + if ((priv->rxdmanext) == RXDMA_BUFFER_SIZE) + { + priv->rxdmanext = 0; + priv->nextcache = 0; + priv->odd = !priv->odd; + priv->buf_idx = 1 & ~(priv->buf_idx); + } + + return c; +} +#endif + +/**************************************************************************** + * Name: sam_dma_rxint + * + * Description: + * Call to enable or disable RX interrupts + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static void sam_dma_rxint(struct uart_dev_s *dev, bool enable) +{ + struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; + + /* En/disable DMA reception. + * + * Note that it is not safe to check for available bytes and immediately + * pass them to uart_recvchars as that could potentially recurse back + * to us again. Instead, bytes must wait until the next sam_dma_poll or + * DMA event. + */ + + if (priv->rxenable != enable) + { + priv->rxenable = enable; + + if (enable) + { + sam_serialout(priv, SAM_UART_IER_OFFSET, UART_INT_TIMEOUT); + } + else + { + sam_serialout(priv, SAM_UART_IDR_OFFSET, UART_INT_TIMEOUT); + } + } +} +#endif + +/**************************************************************************** + * Name: sam_dma_rxavailable + * + * Description: + * Return true if there are some data in the buffer we can read. Also takes + * care of invalidating data cache. + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static bool sam_dma_rxavailable(struct uart_dev_s *dev) +{ + struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; + uint32_t nextrx; + bool ret; + + ret = false; + + /* Get the current DMA pointer */ + + nextrx = sam_dma_nextrx(priv); + + /* Compare our receive pointer to the current DMA pointer, if they + * do match, then there are bytes to be received. + */ + + if ((nextrx != priv->rxdmanext) && priv->rxenable) + { + /* Invalidate data cache if necessary. This basically ensures + * we invalidate only that part of cache we need to. + */ + + if (priv->nextcache < nextrx) + { + up_invalidate_dcache((uintptr_t)priv->rxbuf[priv->buf_idx] \ Review Comment: ditto ########## arch/arm/src/samv7/sam_serial.c: ########## @@ -1344,11 +1801,139 @@ static void sam_rxint(struct uart_dev_s *dev, bool enable) * ****************************************************************************/ +#ifdef SERIAL_HAVE_NODMA_OPS static bool sam_rxavailable(struct uart_dev_s *dev) { struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; return ((sam_serialin(priv, SAM_UART_SR_OFFSET) & UART_INT_RXRDY) != 0); } +#endif + +/**************************************************************************** + * Name: sam_dma_receive + * + * Description: + * Called (usually) from the interrupt level to receive one + * character from the USART. Error bits associated with the + * receipt are provided in the return 'status'. + * + ****************************************************************************/ + +#ifdef SERIAL_HAVE_RXDMA +static int sam_dma_receive(struct uart_dev_s *dev, unsigned int *status) +{ + struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv; + int c = 0; + + *status = priv->sr; + priv->sr = 0; + + /* Read character from the RX FIFO */ + + c = priv->rxbuf[priv->buf_idx][priv->rxdmanext] & UART_RHR_RXCHR_MASK; + priv->rxdmanext++; + + if ((priv->rxdmanext) == RXDMA_BUFFER_SIZE) + { + priv->rxdmanext = 0; + priv->nextcache = 0; + priv->odd = !priv->odd; + priv->buf_idx = 1 & ~(priv->buf_idx); Review Comment: optional -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org