Linux-Serial Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3] serial: uartps: Add TACTIVE bit in cdns_uart_tx_empty function
@ 2020-01-31 11:04 Shubhrajyoti Datta
  2020-01-31 14:18 ` Johan Hovold
  2020-02-01 11:29 ` Maarten Brock
  0 siblings, 2 replies; 3+ messages in thread
From: Shubhrajyoti Datta @ 2020-01-31 11:04 UTC (permalink / raw)
  To: linux-serial
  Cc: gregkh, jslaby, michal.simek, linux-kernel, Raviteja Narayanam,
	Shubhrajyoti Datta

From: Raviteja Narayanam <raviteja.narayanam@xilinx.com>

Make sure that all the bytes are transmitted out of Uart by monitoring
TACTIVE bit as well.
Before setting up baud rate in set termios function, do not wait for
Tx empty as it is taken care by the tty layer if user specified.

Signed-off-by: Raviteja Narayanam <raviteja.narayanam@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
v3:
removed the wait from the set_termios and added the TACTIVE to cdns_uart_tx_empty
As suggested by Johan.

 drivers/tty/serial/xilinx_uartps.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index ed2f325..ebd0a74 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -32,7 +32,6 @@
 #define CDNS_UART_NAME		"xuartps"
 #define CDNS_UART_FIFO_SIZE	64	/* FIFO size */
 #define CDNS_UART_REGISTER_SPACE	0x1000
-#define TX_TIMEOUT		500000
 
 /* Rx Trigger level */
 static int rx_trigger_level = 56;
@@ -656,8 +655,9 @@ static unsigned int cdns_uart_tx_empty(struct uart_port *port)
 {
 	unsigned int status;
 
-	status = readl(port->membase + CDNS_UART_SR) &
-				CDNS_UART_SR_TXEMPTY;
+	status = ((readl(port->membase + CDNS_UART_SR) &
+				(CDNS_UART_SR_TXEMPTY |
+				CDNS_UART_SR_TACTIVE)) == CDNS_UART_SR_TXEMPTY);
 	return status ? TIOCSER_TEMT : 0;
 }
 
@@ -700,20 +700,8 @@ static void cdns_uart_set_termios(struct uart_port *port,
 	u32 cval = 0;
 	unsigned int baud, minbaud, maxbaud;
 	unsigned long flags;
-	unsigned int ctrl_reg, mode_reg, val;
-	int err;
-
-	/* Wait for the transmit FIFO to empty before making changes */
-	if (!(readl(port->membase + CDNS_UART_CR) &
-				CDNS_UART_CR_TX_DIS)) {
-		err = readl_poll_timeout(port->membase + CDNS_UART_SR,
-					 val, (val & CDNS_UART_SR_TXEMPTY),
-					 1000, TX_TIMEOUT);
-		if (err) {
-			dev_err(port->dev, "timed out waiting for tx empty");
-			return;
-		}
-	}
+	unsigned int ctrl_reg, mode_reg;
+
 	spin_lock_irqsave(&port->lock, flags);
 
 	/* Disable the TX and RX to set baud rate */
-- 
2.7.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] serial: uartps: Add TACTIVE bit in cdns_uart_tx_empty function
  2020-01-31 11:04 [PATCH v3] serial: uartps: Add TACTIVE bit in cdns_uart_tx_empty function Shubhrajyoti Datta
@ 2020-01-31 14:18 ` Johan Hovold
  2020-02-01 11:29 ` Maarten Brock
  1 sibling, 0 replies; 3+ messages in thread
From: Johan Hovold @ 2020-01-31 14:18 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: linux-serial, gregkh, jslaby, michal.simek, linux-kernel,
	Raviteja Narayanam

On Fri, Jan 31, 2020 at 04:34:45PM +0530, Shubhrajyoti Datta wrote:
> From: Raviteja Narayanam <raviteja.narayanam@xilinx.com>
> 
> Make sure that all the bytes are transmitted out of Uart by monitoring
> TACTIVE bit as well.
> Before setting up baud rate in set termios function, do not wait for
> Tx empty as it is taken care by the tty layer if user specified.

Since this is two logical changes I suggest you split it in two patches;
the first removing the wait from set_termios, and the second adding the
TCACTIVE check.

> Signed-off-by: Raviteja Narayanam <raviteja.narayanam@xilinx.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v3:
> removed the wait from the set_termios and added the TACTIVE to cdns_uart_tx_empty
> As suggested by Johan.
> 
>  drivers/tty/serial/xilinx_uartps.c | 22 +++++-----------------
>  1 file changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index ed2f325..ebd0a74 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -32,7 +32,6 @@
>  #define CDNS_UART_NAME		"xuartps"
>  #define CDNS_UART_FIFO_SIZE	64	/* FIFO size */
>  #define CDNS_UART_REGISTER_SPACE	0x1000
> -#define TX_TIMEOUT		500000
>  
>  /* Rx Trigger level */
>  static int rx_trigger_level = 56;
> @@ -656,8 +655,9 @@ static unsigned int cdns_uart_tx_empty(struct uart_port *port)
>  {
>  	unsigned int status;
>  
> -	status = readl(port->membase + CDNS_UART_SR) &
> -				CDNS_UART_SR_TXEMPTY;
> +	status = ((readl(port->membase + CDNS_UART_SR) &
> +				(CDNS_UART_SR_TXEMPTY |
> +				CDNS_UART_SR_TACTIVE)) == CDNS_UART_SR_TXEMPTY);
>  	return status ? TIOCSER_TEMT : 0;

Please rewrite this to avoid some of the line breaks (e.g. split readl()
from the test).

Johan

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] serial: uartps: Add TACTIVE bit in cdns_uart_tx_empty function
  2020-01-31 11:04 [PATCH v3] serial: uartps: Add TACTIVE bit in cdns_uart_tx_empty function Shubhrajyoti Datta
  2020-01-31 14:18 ` Johan Hovold
@ 2020-02-01 11:29 ` Maarten Brock
  1 sibling, 0 replies; 3+ messages in thread
From: Maarten Brock @ 2020-02-01 11:29 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: linux-serial, gregkh, jslaby, michal.simek, linux-kernel,
	Raviteja Narayanam, linux-serial-owner

On 2020-01-31 12:04, Shubhrajyoti Datta wrote:
>  drivers/tty/serial/xilinx_uartps.c | 22 +++++-----------------
>  1 file changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c
> b/drivers/tty/serial/xilinx_uartps.c
> index ed2f325..ebd0a74 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -656,8 +655,9 @@ static unsigned int cdns_uart_tx_empty(struct
> uart_port *port)
>  {
>  	unsigned int status;
> 
> -	status = readl(port->membase + CDNS_UART_SR) &
> -				CDNS_UART_SR_TXEMPTY;
> +	status = ((readl(port->membase + CDNS_UART_SR) &
> +				(CDNS_UART_SR_TXEMPTY |
> +				CDNS_UART_SR_TACTIVE)) == CDNS_UART_SR_TXEMPTY);
>  	return status ? TIOCSER_TEMT : 0;
>  }

These lines look pretty incomprehensible.
How about rewriting it like this?

     status = readl(port->membase + CDNS_UART_SR) &
              (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE);
     return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0;

Maarten


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-31 11:04 [PATCH v3] serial: uartps: Add TACTIVE bit in cdns_uart_tx_empty function Shubhrajyoti Datta
2020-01-31 14:18 ` Johan Hovold
2020-02-01 11:29 ` Maarten Brock

Linux-Serial Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-serial/0 linux-serial/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-serial linux-serial/ https://lore.kernel.org/linux-serial \
		linux-serial@vger.kernel.org
	public-inbox-index linux-serial

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-serial


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git