Re: STM32H7 serial TX DMA issues

[Sebastien Lorquet]

Hello,

Yes, stm32h7 uart transmission has issues. You can easily test this in 
nsh with just an echo command and a very long string, eg > 64 ascii 
chars. At first I believed it was buffering problems.

This caused me some headaches 1.5 years ago, but the DMA serial driver 
is too complex for me to debug. I have disabled CONFIG_UARTn_TXDMA on 
relevant uarts of my board.

Please give the link to your PR when it's ready so I can follow this 
closely.

Thank you,

Sebastien


Le 08/03/2024 à 10:29, David Sidrane a écrit :
        Hi Kian,

        The Problem with the semaphore is it cause blocking when the port
is opened non blocking.

        Please do PR so we can review it.

David


-----Original Message-----
From: Kian Karas (KK) <k...@thrane.eu>
Sent: Friday, March 8, 2024 4:18 AM
To: dev@nuttx.apache.org
Subject: STM32H7 serial TX DMA issues

Hi community

The STM32H7 serial driver TX DMA logic is no longer working properly.

The issues started with commit 660ac63b. Subsequent attempts (f92a9068,
6c186b60) have failed to get it working again.

I think the original idea of 660ac63b is right, it just failed to restart
TX DMA upon TX DMA completion (if needed).

I would suggest reverting the following commits: 6c186b60 58f2a7b1
69a8b5b5. Then add the following patch as an amendment:

diff --git a/arch/arm/src/stm32h7/stm32_serial.c
b/arch/arm/src/stm32h7/stm32_serial.c
index 120ea0f3b5..fc90c5d521 100644
--- a/arch/arm/src/stm32h7/stm32_serial.c
+++ b/arch/arm/src/stm32h7/stm32_serial.c
@@ -3780,11 +3780,20 @@ static void up_dma_txcallback(DMA_HANDLE handle,
uint8_t status, void *arg)
          }
      }

-  nxsem_post(&priv->txdmasem);
-
    /* Adjust the pointers */

    uart_xmitchars_done(&priv->dev);
+
+  /* Initiate another transmit if data is ready */
+
+  if (priv->dev.xmit.tail != priv->dev.xmit.head)
+    {
+      uart_xmitchars_dma(&priv->dev);
+    }
+  else
+    {
+      nxsem_post(&priv->txdmasem);
+    }
  }
  #endif

@@ -3806,6 +3815,14 @@ static void up_dma_txavailable(struct uart_dev_s
*dev)
    int rv = nxsem_trywait(&priv->txdmasem);
    if (rv == OK)
      {
+      if (dev->xmit.head == dev->xmit.tail)
+        {
+          /* No data to transfer. Release semaphore. */
+
+          nxsem_post(&priv->txdmasem);
+          return;
+        }
+
        uart_xmitchars_dma(dev);
      }
  }


However, uart_xmitchars_dma() is currently not safe to call from an
interrupt service routine, so the following patch would also be required:

diff --git a/drivers/serial/serial_dma.c b/drivers/serial/serial_dma.c
index aa99e801ff..b2603953ad 100644
--- a/drivers/serial/serial_dma.c
+++ b/drivers/serial/serial_dma.c
@@ -97,26 +97,29 @@ void uart_xmitchars_dma(FAR uart_dev_t *dev)  {
    FAR struct uart_dmaxfer_s *xfer = &dev->dmatx;

-  if (dev->xmit.head == dev->xmit.tail)
+  size_t head = dev->xmit.head;
+  size_t tail = dev->xmit.tail;
+
+  if (head == tail)
      {
        /* No data to transfer. */

        return;
      }

-  if (dev->xmit.tail < dev->xmit.head)
+  if (tail < head)
      {
-      xfer->buffer  = &dev->xmit.buffer[dev->xmit.tail];
-      xfer->length  = dev->xmit.head - dev->xmit.tail;
+      xfer->buffer  = &dev->xmit.buffer[tail];
+      xfer->length  = head - tail;
        xfer->nbuffer = NULL;
        xfer->nlength = 0;
      }
    else
      {
-      xfer->buffer  = &dev->xmit.buffer[dev->xmit.tail];
-      xfer->length  = dev->xmit.size - dev->xmit.tail;
+      xfer->buffer  = &dev->xmit.buffer[tail];
+      xfer->length  = dev->xmit.size - tail;
        xfer->nbuffer = dev->xmit.buffer;
-      xfer->nlength = dev->xmit.head;
+      xfer->nlength = head;
      }

    dev->tx_count += xfer->length + xfer->nlength;


Any thoughts?

Regards
Kian