[PATCH 0/6] tty: serial: fsl_lpuart: improve RXWATER setting and add imx8ulp support

[PATCH 0/6] tty: serial: fsl_lpuart: improve RXWATER setting and add imx8ulp support

[Sherry Sun]

This patch set improves RXWATER setting for lpuart, also adds support for
imx8ulp which has different rx_watermark value with imx7ulp.

Sherry Sun (6):
  tty: serial: fsl_lpuart: make rx_watermark configurable for different
    platforms
  tty: serial: fsl_lpuart: set receive watermark for imx8qxp platform
  tty: serial: fsl_lpuart: Fix the wrong RXWATER setting for rx dma case
  tty: serial: fsl_lpuart: Enable Receiver Idle Empty function for
    LPUART
  tty: serial: fsl_lpuart: set RTS watermark for lpuart
  tty: serial: fsl_lpuart: add imx8ulp support

 drivers/tty/serial/fsl_lpuart.c | 50 +++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 9 deletions(-)

-- 
2.17.1

[PATCH 1/6] tty: serial: fsl_lpuart: make rx_watermark configurable for different platforms

[Sherry Sun]

Add rx_watermark parameter for struct lpuart_port to make the receive
watermark configurable for different platforms.
No function changed.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index eac5243a9db0..e4aa161e61bf 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -262,6 +262,7 @@ struct lpuart_port {
 	unsigned int		txfifo_size;
 	unsigned int		rxfifo_size;
 
+	u8			rx_watermark;
 	bool			lpuart_dma_tx_use;
 	bool			lpuart_dma_rx_use;
 	struct dma_chan		*dma_tx_chan;
@@ -288,38 +289,45 @@ struct lpuart_soc_data {
 	enum lpuart_type devtype;
 	char iotype;
 	u8 reg_off;
+	u8 rx_watermark;
 };
 
 static const struct lpuart_soc_data vf_data = {
 	.devtype = VF610_LPUART,
 	.iotype = UPIO_MEM,
+	.rx_watermark = 1,
 };
 
 static const struct lpuart_soc_data ls1021a_data = {
 	.devtype = LS1021A_LPUART,
 	.iotype = UPIO_MEM32BE,
+	.rx_watermark = 1,
 };
 
 static const struct lpuart_soc_data ls1028a_data = {
 	.devtype = LS1028A_LPUART,
 	.iotype = UPIO_MEM32,
+	.rx_watermark = 1,
 };
 
 static struct lpuart_soc_data imx7ulp_data = {
 	.devtype = IMX7ULP_LPUART,
 	.iotype = UPIO_MEM32,
 	.reg_off = IMX_REG_OFF,
+	.rx_watermark = 1,
 };
 
 static struct lpuart_soc_data imx8qxp_data = {
 	.devtype = IMX8QXP_LPUART,
 	.iotype = UPIO_MEM32,
 	.reg_off = IMX_REG_OFF,
+	.rx_watermark = 1,
 };
 static struct lpuart_soc_data imxrt1050_data = {
 	.devtype = IMXRT1050_LPUART,
 	.iotype = UPIO_MEM32,
 	.reg_off = IMX_REG_OFF,
+	.rx_watermark = 1,
 };
 
 static const struct of_device_id lpuart_dt_ids[] = {
@@ -1520,7 +1528,7 @@ static void lpuart_setup_watermark(struct lpuart_port *sport)
 	}
 
 	writeb(0, sport->port.membase + UARTTWFIFO);
-	writeb(1, sport->port.membase + UARTRWFIFO);
+	writeb(sport->rx_watermark, sport->port.membase + UARTRWFIFO);
 
 	/* Restore cr2 */
 	writeb(cr2_saved, sport->port.membase + UARTCR2);
@@ -1555,7 +1563,8 @@ static void lpuart32_setup_watermark(struct lpuart_port *sport)
 	lpuart32_write(&sport->port, val, UARTFIFO);
 
 	/* set the watermark */
-	val = (0x1 << UARTWATER_RXWATER_OFF) | (0x0 << UARTWATER_TXWATER_OFF);
+	val = (sport->rx_watermark << UARTWATER_RXWATER_OFF) |
+	      (0x0 << UARTWATER_TXWATER_OFF);
 	lpuart32_write(&sport->port, val, UARTWATER);
 
 	/* Restore cr2 */
@@ -2731,6 +2740,7 @@ static int lpuart_probe(struct platform_device *pdev)
 	sport->port.dev = &pdev->dev;
 	sport->port.type = PORT_LPUART;
 	sport->devtype = sdata->devtype;
+	sport->rx_watermark = sdata->rx_watermark;
 	ret = platform_get_irq(pdev, 0);
 	if (ret < 0)
 		return ret;
-- 
2.17.1

[PATCH 2/6] tty: serial: fsl_lpuart: set receive watermark for imx8qxp platform

[Sherry Sun]

Since imx8qxp RX FIFO depth is 64 datawords, it will be better to set
the rx watermark as 31, which means when the number of datawords in the
receive FIFO(>= 32) is greater than the watermark, an interrupt or a DMA
request is generated.
Also keep the console rx watermark as 1 to make sure it responsive.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index e4aa161e61bf..868c2783d2c7 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -321,7 +321,7 @@ static struct lpuart_soc_data imx8qxp_data = {
 	.devtype = IMX8QXP_LPUART,
 	.iotype = UPIO_MEM32,
 	.reg_off = IMX_REG_OFF,
-	.rx_watermark = 1,
+	.rx_watermark = 31,
 };
 static struct lpuart_soc_data imxrt1050_data = {
 	.devtype = IMXRT1050_LPUART,
@@ -1527,6 +1527,8 @@ static void lpuart_setup_watermark(struct lpuart_port *sport)
 		writeb(UARTSFIFO_RXUF, sport->port.membase + UARTSFIFO);
 	}
 
+	if (uart_console(&sport->port))
+		sport->rx_watermark = 1;
 	writeb(0, sport->port.membase + UARTTWFIFO);
 	writeb(sport->rx_watermark, sport->port.membase + UARTRWFIFO);
 
@@ -1563,6 +1565,8 @@ static void lpuart32_setup_watermark(struct lpuart_port *sport)
 	lpuart32_write(&sport->port, val, UARTFIFO);
 
 	/* set the watermark */
+	if (uart_console(&sport->port))
+		sport->rx_watermark = 1;
 	val = (sport->rx_watermark << UARTWATER_RXWATER_OFF) |
 	      (0x0 << UARTWATER_TXWATER_OFF);
 	lpuart32_write(&sport->port, val, UARTWATER);
-- 
2.17.1

[PATCH 3/6] tty: serial: fsl_lpuart: Fix the wrong RXWATER setting for rx dma case

[Sherry Sun]

The RXWATER value must be greater than 0 according to the LPUART
reference manual. And when the number of datawords in the receive
FIFO is greater than RXWATER, an interrupt or a DMA request is
generated, so no need to set the different value for lpuart interrupt
case and dma case. Here delete the wrong RXWATER setting for dma case
directly.

Fixes: 42b68768e51b ("serial: fsl_lpuart: DMA support for 32-bit variant")
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 868c2783d2c7..ba6ade784ac5 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1729,12 +1729,6 @@ static void lpuart32_configure(struct lpuart_port *sport)
 {
 	unsigned long temp;
 
-	if (sport->lpuart_dma_rx_use) {
-		/* RXWATER must be 0 */
-		temp = lpuart32_read(&sport->port, UARTWATER);
-		temp &= ~(UARTWATER_WATER_MASK << UARTWATER_RXWATER_OFF);
-		lpuart32_write(&sport->port, temp, UARTWATER);
-	}
 	temp = lpuart32_read(&sport->port, UARTCTRL);
 	if (!sport->lpuart_dma_rx_use)
 		temp |= UARTCTRL_RIE | UARTCTRL_ILIE;
-- 
2.17.1

[PATCH 4/6] tty: serial: fsl_lpuart: Enable Receiver Idle Empty function for LPUART

[Sherry Sun]

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@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 2 ++
 1 file changed, 2 insertions(+)

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);
 
 	/* set the watermark */
-- 
2.17.1

[PATCH 5/6] tty: serial: fsl_lpuart: set RTS watermark for lpuart

[Sherry Sun]

Add RTS watermark support for LPUART. The RX RTS_B output negates when
the number of empty words in the receive FIFO is greater or equal to
RTSWATER. Here set the RTSWATER to half of the rxfifo_size.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 2789749d3d0d..c35e49a09bcc 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -201,6 +201,7 @@
 #define UARTDATA_MASK		0x3ff
 
 #define UARTMODIR_IREN		0x00020000
+#define UARTMODIR_RTSWATER	GENMASK(10, 8)
 #define UARTMODIR_TXCTSSRC	0x00000020
 #define UARTMODIR_TXCTSC	0x00000010
 #define UARTMODIR_RXRTSE	0x00000008
@@ -1573,6 +1574,13 @@ static void lpuart32_setup_watermark(struct lpuart_port *sport)
 	      (0x0 << UARTWATER_TXWATER_OFF);
 	lpuart32_write(&sport->port, val, UARTWATER);
 
+	/* set RTS watermark */
+	if (!uart_console(&sport->port)) {
+		val = lpuart32_read(&sport->port, UARTMODIR);
+		val |= FIELD_PREP(UARTMODIR_RTSWATER, sport->rxfifo_size >> 1);
+		lpuart32_write(&sport->port, val, UARTMODIR);
+	}
+
 	/* Restore cr2 */
 	lpuart32_write(&sport->port, ctrl_saved, UARTCTRL);
 }
-- 
2.17.1

Re: [PATCH 5/6] tty: serial: fsl_lpuart: set RTS watermark for lpuart

[Alexander Stein]

Am Montag, 30. Januar 2023, 07:44:48 CET schrieb Sherry Sun:
> Add RTS watermark support for LPUART. The RX RTS_B output negates when
> the number of empty words in the receive FIFO is greater or equal to
> RTSWATER. Here set the RTSWATER to half of the rxfifo_size.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c
> b/drivers/tty/serial/fsl_lpuart.c index 2789749d3d0d..c35e49a09bcc 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -201,6 +201,7 @@
>  #define UARTDATA_MASK		0x3ff
> 
>  #define UARTMODIR_IREN		0x00020000
> +#define UARTMODIR_RTSWATER	GENMASK(10, 8)
>  #define UARTMODIR_TXCTSSRC	0x00000020
>  #define UARTMODIR_TXCTSC	0x00000010
>  #define UARTMODIR_RXRTSE	0x00000008
> @@ -1573,6 +1574,13 @@ static void lpuart32_setup_watermark(struct
> lpuart_port *sport) (0x0 << UARTWATER_TXWATER_OFF);
>  	lpuart32_write(&sport->port, val, UARTWATER);
> 
> +	/* set RTS watermark */
> +	if (!uart_console(&sport->port)) {
> +		val = lpuart32_read(&sport->port, UARTMODIR);
> +		val |= FIELD_PREP(UARTMODIR_RTSWATER, sport->rxfifo_size 
>> 1);
> +		lpuart32_write(&sport->port, val, UARTMODIR);

On LS1021A these bits are reserved (please refer to QorIQ LS1021A Reference 
manual Rev 3.1 02/2020 section 27.3.6). Is it okay to write to them?

Best regards
Alexander

> +	}
> +
>  	/* Restore cr2 */
>  	lpuart32_write(&sport->port, ctrl_saved, UARTCTRL);
>  }

RE: [PATCH 5/6] tty: serial: fsl_lpuart: set RTS watermark for lpuart

[Sherry Sun]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: 2023年2月2日 15:14
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: gregkh@linuxfoundation.org; jirislaby@kernel.org; linux-
> serial@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx <linux-
> imx@nxp.com>
> Subject: Re: [PATCH 5/6] tty: serial: fsl_lpuart: set RTS watermark for lpuart
> 
> Am Montag, 30. Januar 2023, 07:44:48 CET schrieb Sherry Sun:
> > Add RTS watermark support for LPUART. The RX RTS_B output negates
> when
> > the number of empty words in the receive FIFO is greater or equal to
> > RTSWATER. Here set the RTSWATER to half of the rxfifo_size.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> >  drivers/tty/serial/fsl_lpuart.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/tty/serial/fsl_lpuart.c
> > b/drivers/tty/serial/fsl_lpuart.c index 2789749d3d0d..c35e49a09bcc
> > 100644
> > --- a/drivers/tty/serial/fsl_lpuart.c
> > +++ b/drivers/tty/serial/fsl_lpuart.c
> > @@ -201,6 +201,7 @@
> >  #define UARTDATA_MASK		0x3ff
> >
> >  #define UARTMODIR_IREN		0x00020000
> > +#define UARTMODIR_RTSWATER	GENMASK(10, 8)
> >  #define UARTMODIR_TXCTSSRC	0x00000020
> >  #define UARTMODIR_TXCTSC	0x00000010
> >  #define UARTMODIR_RXRTSE	0x00000008
> > @@ -1573,6 +1574,13 @@ static void lpuart32_setup_watermark(struct
> > lpuart_port *sport) (0x0 << UARTWATER_TXWATER_OFF);
> >  	lpuart32_write(&sport->port, val, UARTWATER);
> >
> > +	/* set RTS watermark */
> > +	if (!uart_console(&sport->port)) {
> > +		val = lpuart32_read(&sport->port, UARTMODIR);
> > +		val |= FIELD_PREP(UARTMODIR_RTSWATER, sport-
> >rxfifo_size
> >> 1);
> > +		lpuart32_write(&sport->port, val, UARTMODIR);
> 
> On LS1021A these bits are reserved (please refer to QorIQ LS1021A Reference
> manual Rev 3.1 02/2020 section 27.3.6). Is it okay to write to them?
> 
Hi Alexander,

I checked the LS1021A RM, writing the reserved register bits have no impact, so it is okay here.
Also, nxp local linux repo add this patch for a long time, all imx and layerscape boards work well with this patch.

Best Regards
Sherry

[PATCH 6/6] tty: serial: fsl_lpuart: add imx8ulp support

[Sherry Sun]

The lpuart of imx8ulp is basically the same as imx7ulp, but it supports
some new features based on imx7ulp, such as it can assert the DMA
request on EOP(end-of-packet).

Here add lpuart support for imx8ulp, and rx_watermark is set to 3 as
imx8ulp RX FIFO depth is 8.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index c35e49a09bcc..644827e97cb0 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -252,6 +252,7 @@ enum lpuart_type {
 	LS1021A_LPUART,
 	LS1028A_LPUART,
 	IMX7ULP_LPUART,
+	IMX8ULP_LPUART,
 	IMX8QXP_LPUART,
 	IMXRT1050_LPUART,
 };
@@ -319,6 +320,13 @@ static struct lpuart_soc_data imx7ulp_data = {
 	.rx_watermark = 1,
 };
 
+static struct lpuart_soc_data imx8ulp_data = {
+	.devtype = IMX8ULP_LPUART,
+	.iotype = UPIO_MEM32,
+	.reg_off = IMX_REG_OFF,
+	.rx_watermark = 3,
+};
+
 static struct lpuart_soc_data imx8qxp_data = {
 	.devtype = IMX8QXP_LPUART,
 	.iotype = UPIO_MEM32,
@@ -337,6 +345,7 @@ static const struct of_device_id lpuart_dt_ids[] = {
 	{ .compatible = "fsl,ls1021a-lpuart",	.data = &ls1021a_data, },
 	{ .compatible = "fsl,ls1028a-lpuart",	.data = &ls1028a_data, },
 	{ .compatible = "fsl,imx7ulp-lpuart",	.data = &imx7ulp_data, },
+	{ .compatible = "fsl,imx8ulp-lpuart",	.data = &imx8ulp_data, },
 	{ .compatible = "fsl,imx8qxp-lpuart",	.data = &imx8qxp_data, },
 	{ .compatible = "fsl,imxrt1050-lpuart",	.data = &imxrt1050_data},
 	{ /* sentinel */ }
@@ -357,6 +366,11 @@ static inline bool is_imx7ulp_lpuart(struct lpuart_port *sport)
 	return sport->devtype == IMX7ULP_LPUART;
 }
 
+static inline bool is_imx8ulp_lpuart(struct lpuart_port *sport)
+{
+	return sport->devtype == IMX8ULP_LPUART;
+}
+
 static inline bool is_imx8qxp_lpuart(struct lpuart_port *sport)
 {
 	return sport->devtype == IMX8QXP_LPUART;
@@ -2691,7 +2705,7 @@ static int lpuart_global_reset(struct lpuart_port *sport)
 		return ret;
 	}
 
-	if (is_imx7ulp_lpuart(sport) || is_imx8qxp_lpuart(sport)) {
+	if (is_imx7ulp_lpuart(sport) || is_imx8ulp_lpuart(sport) || is_imx8qxp_lpuart(sport)) {
 		/*
 		 * If the transmitter is used by earlycon, wait for transmit engine to
 		 * complete and then reset.
-- 
2.17.1