You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "michallenc (via GitHub)" <gi...@apache.org> on 2024/03/04 14:56:17 UTC

[PR] samv7: add support for USART TX DMA transfers [nuttx]

michallenc opened a new pull request, #11836:
URL: https://github.com/apache/nuttx/pull/11836

   ## Summary
   This commit adds support for TX DMA transfers for USART peripheral. Code refactor in `sam_serial.h` was also required in order to have correct defines for all possible cases (both RX and TX DMA used, just one used, none used).
   
   ## Impact
   SAMv7 MCUs only
   
   ## Testing
   Tested on SAMv7 evaluation kit and on custom board with SAMv7 MCU.
   
   


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] samv7: add support for USART TX DMA transfers [nuttx]

Posted by "michallenc (via GitHub)" <gi...@apache.org>.
michallenc commented on code in PR #11836:
URL: https://github.com/apache/nuttx/pull/11836#discussion_r1511564483


##########
arch/arm/src/samv7/sam_serial.c:
##########
@@ -2037,11 +2181,196 @@ static void sam_dma_rxcallback(DMA_HANDLE handle, void *arg, int status)
  *
  ****************************************************************************/
 
+#ifdef SERIAL_HAVE_NODMA_OPS
 static bool sam_txempty(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_TXEMPTY) != 0);
 }
+#endif
+
+/****************************************************************************
+ * Name: sam_dma_send
+ *
+ * Description:
+ *   This method will initiates DMA transfer for sending bytes over UART.
+ *
+ ****************************************************************************/
+
+#ifdef SERIAL_HAVE_TXDMA
+static void sam_dma_send(struct uart_dev_s *dev)
+{
+  struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv;
+  uint32_t maddr;
+  uint32_t paddr;
+  size_t buflen;
+
+  /* We need to stop DMA before reconfiguration */
+
+  sam_dmastop(priv->txdma);
+
+  /* Flush the contents of the TX buffer into physical memory */
+
+  up_flush_dcache((uintptr_t)dev->dmatx.buffer,
+                  (uintptr_t)dev->dmatx.buffer + dev->dmatx.length);
+
+  /* Setup first half */
+
+  paddr = priv->usartbase + SAM_UART_THR_OFFSET;
+
+  maddr = (uint32_t) dev->dmatx.buffer;
+  buflen = (size_t) dev->dmatx.length;
+
+  /* Is this a split transfer? */
+
+  sam_dmatxsetup(priv->txdma, paddr, maddr, buflen);
+
+  /* Start transmission with the callback on DMA completion */
+
+  sam_dmastart(priv->txdma, sam_dma_txcallback, (void *)dev);
+}
+
+/****************************************************************************
+ * Name: sam_dma_txint
+ *
+ * Description:
+ *   This method handles TX interrupt if DMA is enabled.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txint(struct uart_dev_s *dev, bool enable)
+{
+  /* Nothing to do. */
+
+  /* In case of DMA transfer we do not want to make use of UART interrupts.
+   * Instead, we use DMA interrupts that are activated once during boot
+   * sequence. Furthermore we can use sam_dma_txcallback() to handle staff at
+   * half DMA transfer or after transfer completion (depending configuration,
+   * see stm32_dmastart(...) ).
+   */
+}
+
+/****************************************************************************
+ * Name: sam_dma_txavailable
+ *
+ * Description:
+ *   This method waits on semaphore until DMA is available.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txavailable(struct uart_dev_s *dev)
+{
+  struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv;
+  int rv;
+
+  /* Only send when the DMA is idle */
+
+  if (sam_dmaresidual(priv->txdma) == 0)
+    {
+      uart_xmitchars_dma(dev);
+    }
+}
+
+/****************************************************************************
+ * Name: sam_dma_txavailable_worker
+ *
+ * Description:
+ *   This method waits on semaphore until DMA is available.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txavailable_worker(void *arg)
+{
+  struct uart_dev_s *dev = (struct uart_dev_s *)arg;
+
+  /* Just call sam_dma_txavailable. We cannot have sam_dma_txavailable
+   * directly as a worker because the function is defined with
+   * struct uart_dev_s *dev parameter.
+   */
+
+  sam_dma_txavailable(dev);
+}
+
+/****************************************************************************
+ * Name: sam_dma_txcallback
+ *
+ * Description:
+ *   This method implements callback after DMA is done.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txcallback(DMA_HANDLE handle, void *arg, int status)
+{
+  struct uart_dev_s *dev = (struct uart_dev_s *)arg;
+  struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv;
+  uint32_t maddr;
+  uint32_t paddr;
+  size_t buflen;
+
+  if (status != OK)
+    {
+      /* This means some error occured during DMA transfer. This is most
+       * likely just rare error so schedule work again. Note that this is not
+       * ideal and we could end in an infinite loop. Better approach would be
+       * to use some error counter and report error to serial driver if
+       * counter exceeds some limit. However there currently is no way how to
+       * report DMA error to upper layer driver.
+       */
+
+      DEBUGASSERT(work_available(&priv->work));
+      DEBUGVERIFY(work_queue(HPWORK, &priv->work, sam_dma_txavailable_worker,
+                        (void *)dev, 0));
+      return;
+    }
+
+  /* Update 'nbytes' indicating number of bytes actually transferred by DMA.
+   * This is important to free TX buffer space by 'uart_xmitchars_done'.
+   */
+
+  dev->dmatx.nbytes = dev->dmatx.length;
+  if (priv->dev.dmatx.nlength)
+    {
+      /* Set up DMA on next buffer */
+
+      up_flush_dcache((uintptr_t)dev->dmatx.nbuffer,
+                      (uintptr_t)dev->dmatx.nbuffer + dev->dmatx.nlength);
+
+      paddr = priv->usartbase + SAM_UART_THR_OFFSET;
+      maddr = (uint32_t) dev->dmatx.nbuffer;
+      buflen = (size_t) dev->dmatx.nlength;

Review Comment:
   All should be done. Thanks.



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] samv7: add support for USART TX DMA transfers [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #11836:
URL: https://github.com/apache/nuttx/pull/11836


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] samv7: add support for USART TX DMA transfers [nuttx]

Posted by "michallenc (via GitHub)" <gi...@apache.org>.
michallenc commented on PR #11836:
URL: https://github.com/apache/nuttx/pull/11836#issuecomment-1976789551

   > @michallenc could you please the coding style issue (probably they already existed in the file before you modify it)
   
   Yes thanks, it was my fault. I have written one if statement in clang format.


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] samv7: add support for USART TX DMA transfers [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #11836:
URL: https://github.com/apache/nuttx/pull/11836#discussion_r1511479775


##########
arch/arm/src/samv7/sam_serial.c:
##########
@@ -80,15 +80,31 @@
 # 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.
+#if defined(CONFIG_USART0_TXDMA) || defined(CONFIG_USART1_TXDMA) || \
+    defined(CONFIG_USART2_TXDMA)
+#  define SERIAL_HAVE_TXDMA
+#else
+# undef SERIAL_HAVE_TXDMA
 #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.
+    defined(CONFIG_UART4_TXDMA)
+# warning TX DMA is currently supported only for USART driver.

Review Comment:
   ALL prepreccess intent is TWO spaces, let's fix all places



##########
arch/arm/src/samv7/sam_serial.c:
##########
@@ -80,15 +80,31 @@
 # 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.
+#if defined(CONFIG_USART0_TXDMA) || defined(CONFIG_USART1_TXDMA) || \
+    defined(CONFIG_USART2_TXDMA)
+#  define SERIAL_HAVE_TXDMA
+#else
+# undef SERIAL_HAVE_TXDMA

Review Comment:
   ```suggestion
   #  undef SERIAL_HAVE_TXDMA
   ```



##########
arch/arm/src/samv7/sam_serial.c:
##########
@@ -2037,11 +2181,196 @@ static void sam_dma_rxcallback(DMA_HANDLE handle, void *arg, int status)
  *
  ****************************************************************************/
 
+#ifdef SERIAL_HAVE_NODMA_OPS
 static bool sam_txempty(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_TXEMPTY) != 0);
 }
+#endif
+
+/****************************************************************************
+ * Name: sam_dma_send
+ *
+ * Description:
+ *   This method will initiates DMA transfer for sending bytes over UART.
+ *
+ ****************************************************************************/
+
+#ifdef SERIAL_HAVE_TXDMA
+static void sam_dma_send(struct uart_dev_s *dev)
+{
+  struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv;
+  uint32_t maddr;
+  uint32_t paddr;
+  size_t buflen;
+
+  /* We need to stop DMA before reconfiguration */
+
+  sam_dmastop(priv->txdma);
+
+  /* Flush the contents of the TX buffer into physical memory */
+
+  up_flush_dcache((uintptr_t)dev->dmatx.buffer,
+                  (uintptr_t)dev->dmatx.buffer + dev->dmatx.length);
+
+  /* Setup first half */
+
+  paddr = priv->usartbase + SAM_UART_THR_OFFSET;
+
+  maddr = (uint32_t) dev->dmatx.buffer;
+  buflen = (size_t) dev->dmatx.length;
+
+  /* Is this a split transfer? */
+
+  sam_dmatxsetup(priv->txdma, paddr, maddr, buflen);
+
+  /* Start transmission with the callback on DMA completion */
+
+  sam_dmastart(priv->txdma, sam_dma_txcallback, (void *)dev);
+}
+
+/****************************************************************************
+ * Name: sam_dma_txint
+ *
+ * Description:
+ *   This method handles TX interrupt if DMA is enabled.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txint(struct uart_dev_s *dev, bool enable)
+{
+  /* Nothing to do. */
+
+  /* In case of DMA transfer we do not want to make use of UART interrupts.
+   * Instead, we use DMA interrupts that are activated once during boot
+   * sequence. Furthermore we can use sam_dma_txcallback() to handle staff at
+   * half DMA transfer or after transfer completion (depending configuration,
+   * see stm32_dmastart(...) ).
+   */
+}
+
+/****************************************************************************
+ * Name: sam_dma_txavailable
+ *
+ * Description:
+ *   This method waits on semaphore until DMA is available.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txavailable(struct uart_dev_s *dev)
+{
+  struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv;
+  int rv;
+
+  /* Only send when the DMA is idle */
+
+  if (sam_dmaresidual(priv->txdma) == 0)
+    {
+      uart_xmitchars_dma(dev);
+    }
+}
+
+/****************************************************************************
+ * Name: sam_dma_txavailable_worker
+ *
+ * Description:
+ *   This method waits on semaphore until DMA is available.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txavailable_worker(void *arg)
+{
+  struct uart_dev_s *dev = (struct uart_dev_s *)arg;
+
+  /* Just call sam_dma_txavailable. We cannot have sam_dma_txavailable
+   * directly as a worker because the function is defined with
+   * struct uart_dev_s *dev parameter.
+   */
+
+  sam_dma_txavailable(dev);
+}
+
+/****************************************************************************
+ * Name: sam_dma_txcallback
+ *
+ * Description:
+ *   This method implements callback after DMA is done.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txcallback(DMA_HANDLE handle, void *arg, int status)
+{
+  struct uart_dev_s *dev = (struct uart_dev_s *)arg;
+  struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv;
+  uint32_t maddr;
+  uint32_t paddr;
+  size_t buflen;
+
+  if (status != OK)
+    {
+      /* This means some error occured during DMA transfer. This is most
+       * likely just rare error so schedule work again. Note that this is not
+       * ideal and we could end in an infinite loop. Better approach would be
+       * to use some error counter and report error to serial driver if
+       * counter exceeds some limit. However there currently is no way how to
+       * report DMA error to upper layer driver.
+       */
+
+      DEBUGASSERT(work_available(&priv->work));
+      DEBUGVERIFY(work_queue(HPWORK, &priv->work, sam_dma_txavailable_worker,
+                        (void *)dev, 0));
+      return;
+    }
+
+  /* Update 'nbytes' indicating number of bytes actually transferred by DMA.
+   * This is important to free TX buffer space by 'uart_xmitchars_done'.
+   */
+
+  dev->dmatx.nbytes = dev->dmatx.length;
+  if (priv->dev.dmatx.nlength)
+    {
+      /* Set up DMA on next buffer */
+
+      up_flush_dcache((uintptr_t)dev->dmatx.nbuffer,
+                      (uintptr_t)dev->dmatx.nbuffer + dev->dmatx.nlength);
+
+      paddr = priv->usartbase + SAM_UART_THR_OFFSET;
+      maddr = (uint32_t) dev->dmatx.nbuffer;
+      buflen = (size_t) dev->dmatx.nlength;

Review Comment:
   ```suggestion
         buflen = (size_t)dev->dmatx.nlength;
   ```



##########
arch/arm/src/samv7/sam_serial.c:
##########
@@ -2037,11 +2181,196 @@ static void sam_dma_rxcallback(DMA_HANDLE handle, void *arg, int status)
  *
  ****************************************************************************/
 
+#ifdef SERIAL_HAVE_NODMA_OPS
 static bool sam_txempty(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_TXEMPTY) != 0);
 }
+#endif
+
+/****************************************************************************
+ * Name: sam_dma_send
+ *
+ * Description:
+ *   This method will initiates DMA transfer for sending bytes over UART.
+ *
+ ****************************************************************************/
+
+#ifdef SERIAL_HAVE_TXDMA
+static void sam_dma_send(struct uart_dev_s *dev)
+{
+  struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv;
+  uint32_t maddr;
+  uint32_t paddr;
+  size_t buflen;
+
+  /* We need to stop DMA before reconfiguration */
+
+  sam_dmastop(priv->txdma);
+
+  /* Flush the contents of the TX buffer into physical memory */
+
+  up_flush_dcache((uintptr_t)dev->dmatx.buffer,
+                  (uintptr_t)dev->dmatx.buffer + dev->dmatx.length);
+
+  /* Setup first half */
+
+  paddr = priv->usartbase + SAM_UART_THR_OFFSET;
+
+  maddr = (uint32_t) dev->dmatx.buffer;
+  buflen = (size_t) dev->dmatx.length;
+
+  /* Is this a split transfer? */
+
+  sam_dmatxsetup(priv->txdma, paddr, maddr, buflen);
+
+  /* Start transmission with the callback on DMA completion */
+
+  sam_dmastart(priv->txdma, sam_dma_txcallback, (void *)dev);
+}
+
+/****************************************************************************
+ * Name: sam_dma_txint
+ *
+ * Description:
+ *   This method handles TX interrupt if DMA is enabled.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txint(struct uart_dev_s *dev, bool enable)
+{
+  /* Nothing to do. */
+
+  /* In case of DMA transfer we do not want to make use of UART interrupts.
+   * Instead, we use DMA interrupts that are activated once during boot
+   * sequence. Furthermore we can use sam_dma_txcallback() to handle staff at
+   * half DMA transfer or after transfer completion (depending configuration,
+   * see stm32_dmastart(...) ).
+   */
+}
+
+/****************************************************************************
+ * Name: sam_dma_txavailable
+ *
+ * Description:
+ *   This method waits on semaphore until DMA is available.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txavailable(struct uart_dev_s *dev)
+{
+  struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv;
+  int rv;
+
+  /* Only send when the DMA is idle */
+
+  if (sam_dmaresidual(priv->txdma) == 0)
+    {
+      uart_xmitchars_dma(dev);
+    }
+}
+
+/****************************************************************************
+ * Name: sam_dma_txavailable_worker
+ *
+ * Description:
+ *   This method waits on semaphore until DMA is available.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txavailable_worker(void *arg)
+{
+  struct uart_dev_s *dev = (struct uart_dev_s *)arg;
+
+  /* Just call sam_dma_txavailable. We cannot have sam_dma_txavailable
+   * directly as a worker because the function is defined with
+   * struct uart_dev_s *dev parameter.
+   */
+
+  sam_dma_txavailable(dev);
+}
+
+/****************************************************************************
+ * Name: sam_dma_txcallback
+ *
+ * Description:
+ *   This method implements callback after DMA is done.
+ *
+ ****************************************************************************/
+
+static void sam_dma_txcallback(DMA_HANDLE handle, void *arg, int status)
+{
+  struct uart_dev_s *dev = (struct uart_dev_s *)arg;
+  struct sam_dev_s *priv = (struct sam_dev_s *)dev->priv;
+  uint32_t maddr;
+  uint32_t paddr;
+  size_t buflen;
+
+  if (status != OK)
+    {
+      /* This means some error occured during DMA transfer. This is most
+       * likely just rare error so schedule work again. Note that this is not
+       * ideal and we could end in an infinite loop. Better approach would be
+       * to use some error counter and report error to serial driver if
+       * counter exceeds some limit. However there currently is no way how to
+       * report DMA error to upper layer driver.
+       */
+
+      DEBUGASSERT(work_available(&priv->work));
+      DEBUGVERIFY(work_queue(HPWORK, &priv->work, sam_dma_txavailable_worker,
+                        (void *)dev, 0));
+      return;
+    }
+
+  /* Update 'nbytes' indicating number of bytes actually transferred by DMA.
+   * This is important to free TX buffer space by 'uart_xmitchars_done'.
+   */
+
+  dev->dmatx.nbytes = dev->dmatx.length;
+  if (priv->dev.dmatx.nlength)
+    {
+      /* Set up DMA on next buffer */
+
+      up_flush_dcache((uintptr_t)dev->dmatx.nbuffer,
+                      (uintptr_t)dev->dmatx.nbuffer + dev->dmatx.nlength);
+
+      paddr = priv->usartbase + SAM_UART_THR_OFFSET;
+      maddr = (uint32_t) dev->dmatx.nbuffer;

Review Comment:
   ```suggestion
         maddr = (uint32_t)dev->dmatx.nbuffer;
   ```



##########
arch/arm/src/samv7/sam_xdmac.c:
##########
@@ -2118,6 +2118,25 @@ size_t sam_destaddr(DMA_HANDLE handle)
   return sam_getdmach(xdmach, SAM_XDMACH_CDA_OFFSET);
 }
 
+/****************************************************************************
+ * Name: sam_dmaresidual
+ *
+ * Description:
+ *   Returns the number of bytes remaining to be transferred
+ *
+ * Assumptions:
+ *   - DMA handle allocated by sam_dmachannel()
+ *
+ ****************************************************************************/
+
+size_t sam_dmaresidual(DMA_HANDLE handle)
+{
+  struct sam_xdmach_s *xdmach = (struct sam_xdmach_s *)handle;
+  uint32_t cubc = sam_getdmach(xdmach, SAM_XDMACH_CUBC_OFFSET);
+
+  return (cubc & XDMACH_CUBC_UBLEN_MASK);

Review Comment:
   ```suggestion
     return cubc & XDMACH_CUBC_UBLEN_MASK;
   ```



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] samv7: add support for USART TX DMA transfers [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #11836:
URL: https://github.com/apache/nuttx/pull/11836#issuecomment-1976778316

   @michallenc could you please the coding style issue (probably they already existed in the file before you modify it)


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org