On Fri, Jan 15, 2021 at 02:32:39AM +0000, zhangqiumiao wrote:

On Thu, Jan 14, 2021 at 08:28:30PM +0800, zhangqiumiao1@huawei.com wrote:

quoted

From: Qiumiao Zhang <redacted>

make pl011 serial port support 485 mode full duplex communication

Signed-off-by: Qiumiao Zhang <redacted>
---
Changes in v3:
  -Fix busy loop forever in pl011_tx_empty
  -Move the definition of cr into uart_amba_port
  -run checkpatch with no error or warning

Changes in v2:
  -Fix two compilation errors

 drivers/tty/serial/amba-pl011.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c 

b/drivers/tty/serial/amba-pl011.c index c255476..9da10a4 100644

--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c

@@ -44,6 +44,7 @@

 #include "amba-pl011.h"

+#define ISEMPTY			1
 #define UART_NR			14

 #define SERIAL_AMBA_MAJOR	204

@@ -264,6 +265,7 @@ struct uart_amba_port {
 	unsigned int		fifosize;	/* vendor-specific */
 	unsigned int		old_cr;		/* state during shutdown */
 	unsigned int		fixed_baud;	/* vendor-set fixed baud rate */
+	unsigned int		cr;
 	char			type[12];
 #ifdef CONFIG_DMA_ENGINE
 	/* DMA stuff */

@@ -1284,6 +1286,8 @@ static inline bool pl011_dma_rx_running(struct uart_amba_port *uap)
 #define pl011_dma_flush_buffer	NULL
 #endif

+static unsigned int pl011_tx_empty(struct uart_port *port);
+
 static void pl011_stop_tx(struct uart_port *port)  {
 	struct uart_amba_port *uap =

@@ -1292,6 +1296,17 @@ static void pl011_stop_tx(struct uart_port *port)
 	uap->im &= ~UART011_TXIM;
 	pl011_write(uap->im, uap, REG_IMSC);
 	pl011_dma_tx_stop(uap);
+	if (port->rs485.flags & SER_RS485_ENABLED) {
+		while(pl011_tx_empty(port) != ISEMPTY) ;

I intend to change this to:
        while(pl011_tx_empty(port) != ISEMPTY)
			cpu_relax();
Is this ok?

See other comments on this same list (linux-serial) for a patch for a
different driver a few days ago about why doing that would not be a good
idea.  Did you not review that change when it was submitted?

quoted

+
+		uap->cr = pl011_read(uap, REG_CR);
+		if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) {
+			uap->cr |= UART011_CR_RTS;
+		} else {
+			uap->cr &= ~UART011_CR_RTS;
+		}

Checkpatch doesn't find the problem here. Please tell me what's wrong here?

No braces should be needed, have you read the coding style rules?

thanks,

greg k-h