From: Vladimir Oltean <vladimir.olt...@nxp.com> Sent: Friday, October 23, 2020 
9:34 AM
> Prior to the commit that this one fixes, the FIFO size was derived from the
> read-only register LPUARTx_FIFO[TXFIFOSIZE] using the following
> formula:
> 
> TX FIFO size = 2 ^ (LPUARTx_FIFO[TXFIFOSIZE] - 1)
> 
> The documentation for LS1021A is a mess. Under chapter 26.1.3 LS1021A
> LPUART module special consideration, it mentions TXFIFO_SZ and RXFIFO_SZ
> being equal to 4, and in the register description for LPUARTx_FIFO, it shows 
> the
> out-of-reset value of TXFIFOSIZE and RXFIFOSIZE fields as "011", even though
> these registers read as "101" in reality.
> 
> And when LPUART on LS1021A was working, the "101" value did correspond to
> "16 datawords", by applying the formula above, even though the
> documentation is wrong again (!!!!) and says that "101" means 64 datawords
> (hint: it doesn't).
> 
> So the "new" formula created by commit f77ebb241ce0 has all the premises of
> being wrong for LS1021A, because it relied only on false data and no actual
> experimentation.
> 
> Interestingly, in commit c2f448cff22a ("tty: serial: fsl_lpuart: add LS1028A
> support"), Michael Walle applied a workaround to this by manually setting the
> FIFO widths for LS1028A. It looks like the same values are used by LS1021A as
> well, in fact.
> 
> 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 pointing out where to look.
> 
> Fixes: f77ebb241ce0 ("tty: serial: fsl_lpuart: correct the FIFO depth size")
> Suggested-by: Michael Walle <mich...@walle.cc>
> Signed-off-by: Vladimir Oltean <vladimir.olt...@nxp.com>
> ---
> Changes in v2:
> Reworded commit message.

For the v2 with commit message change: 
Reviewed-by:Fugang Duan <fugang.d...@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

Reply via email to