[PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 32 datawords

[PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 32 datawords

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Similar to the workaround applied by Michael Walle in commit
c2f448cff22a ("tty: serial: fsl_lpuart: add LS1028A support"), it turns
out that the LPUARTx_FIFO encoding for fields TXFIFOSIZE and RXFIFOSIZE
is the same for LS1028A as for LS1021A.

The RXFIFOSIZE in the Layerscape SoCs is fixed at this value:
101 Receive FIFO/Buffer depth = 32 datawords.

When Andy Duan wrote the commit in Fixes: below, he assumed that the 101
encoding means 64 datawords. But this is not true for Layerscape. So
that commit broke LS1021A, and this patch is extending the workaround
for LS1028A which appeared in the meantime, to fix that breakage.

When the driver thinks that it has a deeper FIFO than it really has,
getty (user space) output gets truncated.

Many thanks to Michael for suggesting this!

Fixes: f77ebb241ce0 ("tty: serial: fsl_lpuart: correct the FIFO depth size")
Suggested-by: Michael Walle <michael@walle.cc>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index ff4b88c637d0..bd047e1f9bea 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -314,9 +314,10 @@ MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
 /* Forward declare this for the dma callbacks*/
 static void lpuart_dma_tx_complete(void *arg);
 
-static inline bool is_ls1028a_lpuart(struct lpuart_port *sport)
+static inline bool is_layerscape_lpuart(struct lpuart_port *sport)
 {
-	return sport->devtype == LS1028A_LPUART;
+	return (sport->devtype == LS1021A_LPUART ||
+		sport->devtype == LS1028A_LPUART);
 }
 
 static inline bool is_imx8qxp_lpuart(struct lpuart_port *sport)
@@ -1701,11 +1702,11 @@ static int lpuart32_startup(struct uart_port *port)
 					    UARTFIFO_FIFOSIZE_MASK);
 
 	/*
-	 * The LS1028A has a fixed length of 16 words. Although it supports the
-	 * RX/TXSIZE fields their encoding is different. Eg the reference manual
-	 * states 0b101 is 16 words.
+	 * The LS1021A and LS1028A have a fixed FIFO depth of 16 words.
+	 * Although they support the RX/TXSIZE fields, their encoding is
+	 * different. Eg the reference manual states 0b101 is 16 words.
 	 */
-	if (is_ls1028a_lpuart(sport)) {
+	if (is_layerscape_lpuart(sport)) {
 		sport->rxfifo_size = 16;
 		sport->txfifo_size = 16;
 		sport->port.fifosize = sport->txfifo_size;
-- 
2.25.1

Re: [PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 32 datawords

[Vladimir Oltean]

On Thu, Oct 22, 2020 at 06:12:50PM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Similar to the workaround applied by Michael Walle in commit
> c2f448cff22a ("tty: serial: fsl_lpuart: add LS1028A support"), it turns
> out that the LPUARTx_FIFO encoding for fields TXFIFOSIZE and RXFIFOSIZE
> is the same for LS1028A as for LS1021A.
> 
> The RXFIFOSIZE in the Layerscape SoCs is fixed at this value:
> 101 Receive FIFO/Buffer depth = 32 datawords.
> 
> When Andy Duan wrote the commit in Fixes: below, he assumed that the 101
> encoding means 64 datawords. But this is not true for Layerscape. So
> that commit broke LS1021A, and this patch is extending the workaround
> for LS1028A which appeared in the meantime, to fix that breakage.
> 
> When the driver thinks that it has a deeper FIFO than it really has,
> getty (user space) output gets truncated.
> 
> Many thanks to Michael for suggesting this!
> 
> Fixes: f77ebb241ce0 ("tty: serial: fsl_lpuart: correct the FIFO depth size")
> Suggested-by: Michael Walle <michael@walle.cc>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---

Please don't merge this yet. The patch works, but the commit message is
a mess. Right now I suspect there might be some issues in the documentation.
I'll return with a v2 when I get that clarified.

RE: [EXT] [PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 32 datawords

[Andy Duan]

From: Vladimir Oltean <olteanv@gmail.com> Sent: Thursday, October 22, 2020 11:13 PM
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Similar to the workaround applied by Michael Walle in commit c2f448cff22a
> ("tty: serial: fsl_lpuart: add LS1028A support"), it turns out that the
> LPUARTx_FIFO encoding for fields TXFIFOSIZE and RXFIFOSIZE is the same for
> LS1028A as for LS1021A.
> 
> The RXFIFOSIZE in the Layerscape SoCs is fixed at this value:
> 101 Receive FIFO/Buffer depth = 32 datawords.
> 
> When Andy Duan wrote the commit in Fixes: below, he assumed that the 101
> encoding means 64 datawords. But this is not true for Layerscape. So that
> commit broke LS1021A, and this patch is extending the workaround for LS1028A
> which appeared in the meantime, to fix that breakage.
> 
> When the driver thinks that it has a deeper FIFO than it really has, getty (user
> space) output gets truncated.
> 
> Many thanks to Michael for suggesting this!
> 
> Fixes: f77ebb241ce0 ("tty: serial: fsl_lpuart: correct the FIFO depth size")
> Suggested-by: Michael Walle <michael@walle.cc>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Layerscape has different define for the FIFO size.

Reviewed-by: Fugang Duan <fugang.duan@nxp.com>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index
> ff4b88c637d0..bd047e1f9bea 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -314,9 +314,10 @@ MODULE_DEVICE_TABLE(of, lpuart_dt_ids);
>  /* Forward declare this for the dma callbacks*/  static void
> lpuart_dma_tx_complete(void *arg);
> 
> -static inline bool is_ls1028a_lpuart(struct lpuart_port *sport)
> +static inline bool is_layerscape_lpuart(struct lpuart_port *sport)
>  {
> -       return sport->devtype == LS1028A_LPUART;
> +       return (sport->devtype == LS1021A_LPUART ||
> +               sport->devtype == LS1028A_LPUART);
>  }
> 
>  static inline bool is_imx8qxp_lpuart(struct lpuart_port *sport) @@ -1701,11
> +1702,11 @@ static int lpuart32_startup(struct uart_port *port)
> 
> UARTFIFO_FIFOSIZE_MASK);
> 
>         /*
> -        * The LS1028A has a fixed length of 16 words. Although it supports
> the
> -        * RX/TXSIZE fields their encoding is different. Eg the reference
> manual
> -        * states 0b101 is 16 words.
> +        * The LS1021A and LS1028A have a fixed FIFO depth of 16 words.
> +        * Although they support the RX/TXSIZE fields, their encoding is
> +        * different. Eg the reference manual states 0b101 is 16 words.
>          */
> -       if (is_ls1028a_lpuart(sport)) {
> +       if (is_layerscape_lpuart(sport)) {
>                 sport->rxfifo_size = 16;
>                 sport->txfifo_size = 16;
>                 sport->port.fifosize = sport->txfifo_size;
> --
> 2.25.1