FAILED: patch "[PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 16 words," failed to apply to 5.4-stable tree

FAILED: patch "[PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 16 words," failed to apply to 5.4-stable tree

[gregkh]

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From c97f2a6fb3dfbfbbc88edc8ea62ef2b944e18849 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Fri, 23 Oct 2020 04:34:29 +0300
Subject: [PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 16 words,
 like LS1028A
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 <michael@walle.cc>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Link: https://lore.kernel.org/r/20201023013429.3551026-1-vladimir.oltean@nxp.com
Reviewed-by：Fugang Duan <fugang.duan@nxp.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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;

Re: FAILED: patch "[PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 16 words," failed to apply to 5.4-stable tree

[Vladimir Oltean]

On Tue, Nov 03, 2020 at 03:23:20PM +0100, gregkh@linuxfoundation.org wrote:
> The patch below does not apply to the 5.4-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.

Greg,

For linux-5.4.y, could you please do the following so that I don't need
to explicitly resend these to linux-stable?

# tty: serial: fsl_lpuart: add LS1028A support
git cherry-pick -xs c2f448cff22a7ed09281f02bde084b0ce3bc61ed # this is dependency
# tty: serial: fsl_lpuart: LS1021A has a FIFO size of 16 words, like LS1028A
git cherry-pick -xs c97f2a6fb3dfbfbbc88edc8ea62ef2b944e18849 # this is the one that failed

Both sha1sums can be found in Linus' tree.

I have tested these 2 cherry-picks on top of linux-5.4.y on my board,
and it works just fine.

I was a bit concerned about backporting the LS1028A patch as a
dependency for my fix, but I have consulted
Documentation/process/stable-kernel-rules.rst and it says:

 - New device IDs and quirks are also accepted.

That patch also satisfies the following:

 - It must be obviously correct and tested. <- check
 - It cannot be bigger than 100 lines, with context. <- check

The patch does not apply because the fixes were discovered backwards.
LS1021A and LS1028A should be compatible with one another. However
when bringing up the LS1028A, Michael found the LPUART to be broken,
thought the LS1021A was working, and added the FIFO size quirk as a
LS1028A 'feature'.

Thanks,
-Vladimir

Re: FAILED: patch "[PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 16 words," failed to apply to 5.4-stable tree

[gregkh]

On Tue, Nov 03, 2020 at 10:48:26PM +0000, Vladimir Oltean wrote:
> On Tue, Nov 03, 2020 at 03:23:20PM +0100, gregkh@linuxfoundation.org wrote:
> > The patch below does not apply to the 5.4-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> 
> Greg,
> 
> For linux-5.4.y, could you please do the following so that I don't need
> to explicitly resend these to linux-stable?
> 
> # tty: serial: fsl_lpuart: add LS1028A support
> git cherry-pick -xs c2f448cff22a7ed09281f02bde084b0ce3bc61ed # this is dependency
> # tty: serial: fsl_lpuart: LS1021A has a FIFO size of 16 words, like LS1028A
> git cherry-pick -xs c97f2a6fb3dfbfbbc88edc8ea62ef2b944e18849 # this is the one that failed
> 
> Both sha1sums can be found in Linus' tree.
> 
> I have tested these 2 cherry-picks on top of linux-5.4.y on my board,
> and it works just fine.
> 
> I was a bit concerned about backporting the LS1028A patch as a
> dependency for my fix, but I have consulted
> Documentation/process/stable-kernel-rules.rst and it says:
> 
>  - New device IDs and quirks are also accepted.
> 
> That patch also satisfies the following:
> 
>  - It must be obviously correct and tested. <- check
>  - It cannot be bigger than 100 lines, with context. <- check
> 
> The patch does not apply because the fixes were discovered backwards.
> LS1021A and LS1028A should be compatible with one another. However
> when bringing up the LS1028A, Michael found the LPUART to be broken,
> thought the LS1021A was working, and added the FIFO size quirk as a
> LS1028A 'feature'.

That worked, now done, thanks.

greg k-h

Re: FAILED: patch "[PATCH] tty: serial: fsl_lpuart: LS1021A has a FIFO size of 16 words," failed to apply to 5.4-stable tree

[Vladimir Oltean]

On Mon, Nov 09, 2020 at 11:18:04AM +0100, gregkh@linuxfoundation.org wrote:
> That worked, now done, thanks.

Thanks a lot!