> -----Original Message-----
> From: Robert Hodaszi <robert.hodaszi@xxxxxxxx>
> Sent: 2023年6月7日 22:05
> To: Sherry Sun <sherry.sun@xxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx;
> jirislaby@xxxxxxxxxx; linux-serial@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] Revert "tty: serial: fsl_lpuart: Fix the wrong RXWATER
> setting for rx dma case"
>
> On 2023. 06. 07. 14:39, Sherry Sun wrote:
> > Hi Robert,
> >
> > No, please don’t do that.
> > I agree with you that the LPUART RM has a false statement, the Receive
> Watermark actually can be greater than or equal to 0, but this doesn’t mean
> that it must be 0.
> > And the false statement has nothing to do with the code here. The code
> here aims to set the different values for lpuart interrupt case and dma case,
> and force the rx watermark for dma case to 0, which is unreasonable.
> > We have already set the watermark in lpuart32_setup_watermark(), it
> works for both interrupt and dma case, you can set the rx_watermark value
> for different platforms according to your requirements, from 0 ~
> FIFO[RXFIFOSIZE]-1.
> >
> > Best Regards
> > Sherry
> >
> I'm working on an LS1028A-based (actually LS1027A) unit. Having the
> watermark set to 1 is basically making the LPUART unusable. DMA is enabled
> on this platform. The last character always gets stuck in the receive buffer,
> and I receive only the previous one. So the WATER register's content after
> sending one character to the unit (but not receiving it in Linux on the unit) is
> the following:
>
> 0x0226001c: 01010000
>
> Which means, watermark is set to 1, there's one character waiting in the FIFO,
> and there was no DMA transaction to move out that character from the FIFO
> into the DMA RX buffer.
>
> The RM says: "When the number of datawords in the receive FIFO/buffer is
> greater than the value in this register field, an interrupt or a DMA request is
> generated." So if I get it right, having the watermark set to 1, will never
> generate a DMA request when only a single character is sent. The RX timeout
> function with DMA will help nothing here, as there will be nothing in the RX
> buffer.
>
> But if DMA is NOT used (interrupt based receive), we can set whatever we
> want to watermark, as the timeout function will collect the received bytes
> from the FIFO.
Hi Robert,
I understand your concern, fortunately, LPUART IP provides the Receiver Idle Empty function, which can avoid the receive data being trapped in the RX FIFO. For more details you can check my patch below.
commit 96f54fd4894711b0dce6a1c8c26c882295dc9234
Author: Sherry Sun <sherry.sun@xxxxxxx>
Date: Mon Jan 30 14:44:47 2023 +0800
tty: serial: fsl_lpuart: Enable Receiver Idle Empty function for LPUART
With the growth of rx watermark, it's useful to enable the Receiver Idle
Empty function, it can assert the RDRF(Receive Data Register Full Flag)
when the receiver is idle for a number of idle characters and the FIFO
is not empty. It will generate a DMA request or interrupt, which can
avoid receive data being trapped in the RX FIFO since the number of
words received is less than the watermark.
Here set the RXIDEN as 0x3 which enable the RDRF assertion due to
partially filled FIFO when receiver is idle for 4 characters.
Signed-off-by: Sherry Sun <sherry.sun@xxxxxxx>
Link: https://lore.kernel.org/r/20230130064449.9564-5-sherry.sun@xxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index ba6ade784ac5..2789749d3d0d 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -214,6 +214,7 @@
#define UARTFIFO_RXUF 0x00010000
#define UARTFIFO_TXFLUSH 0x00008000
#define UARTFIFO_RXFLUSH 0x00004000
+#define UARTFIFO_RXIDEN GENMASK(12, 10)
#define UARTFIFO_TXOFE 0x00000200
#define UARTFIFO_RXUFE 0x00000100
#define UARTFIFO_TXFE 0x00000080
@@ -1562,6 +1563,7 @@ static void lpuart32_setup_watermark(struct lpuart_port *sport)
val = lpuart32_read(&sport->port, UARTFIFO);
val |= UARTFIFO_TXFE | UARTFIFO_RXFE;
val |= UARTFIFO_TXFLUSH | UARTFIFO_RXFLUSH;
+ val |= FIELD_PREP(UARTFIFO_RXIDEN, 0x3);
lpuart32_write(&sport->port, val, UARTFIFO);
Best Regards
Sherry
