Hi,
here is the pull request:
https://github.com/apache/nuttx/pull/11871
My initial comments (and "fix") for uart_xmitchars_dma() are no longer
relevant. Hence, those changes are no longer included.
Regards
Kian
________________________________
From: Sebastien Lorquet <sebast...@lorquet.fr>
Sent: 08 March 2024 11:13
To: dev@nuttx.apache.org <dev@nuttx.apache.org>
Subject: Re: STM32H7 serial TX DMA issues
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
