2020-10-22 20:29:30

by Vladimir Oltean

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

From: Vladimir Oltean <[email protected]>

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 <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
---
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


2020-10-23 05:43:27

by Vladimir Oltean

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

On Thu, Oct 22, 2020 at 06:12:50PM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Vladimir Oltean <[email protected]>
> ---

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.

2020-10-23 07:36:57

by Andy Duan

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

From: Vladimir Oltean <[email protected]> Sent: Thursday, October 22, 2020 11:13 PM
> From: Vladimir Oltean <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Vladimir Oltean <[email protected]>
Layerscape has different define for the FIFO size.

Reviewed-by: Fugang Duan <[email protected]>
> ---
> 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