[PATCH 0/5] tty/serial: at91: fix atmel_serial_probe(), dma_sync_* and the Hardware Handshaking support

[PATCH 0/5] tty/serial: at91: fix atmel_serial_probe(), dma_sync_* and the Hardware Handshaking support

[Cyrille Pitchen]

This series of patches fix many bugs:
- a wrong enum type was used when calling dma_sync_*_for_cpu() and
  dma_sync_*_for_device()
- IO accesses to registers were done in atmel_serial_probe() after the
  peripheral clock had been disabled.
- the -EDEFER error was not handled properly in atmel_serial_probe()
- the Hardware Handshaking mode could never be enabled to control the RTS line

The patches were made from the next-20141205 tag of the linux-next repository.
They were tested on a SAMA5Dx FPGA platform.

Cyrille Pitchen (5):
  tty/serial: at91: use correct type for dma_sync_*_for_cpu() and
    dma_sync_*_for_device()
  tty/serial: at91: enable peripheral clock before accessing I/O
    registers
  tty/serial: at91: fix error handling in atmel_serial_probe()
  tty/serial: at91: fix RTS line management when hardware handshake is
    enabled
  tty/serial: at91: fix typo and indentation

 drivers/tty/serial/atmel_serial.c | 153 +++++++++++++++++++++++++-------------
 1 file changed, 101 insertions(+), 52 deletions(-)

-- 
1.8.2.2

[PATCH 1/5] tty/serial: at91: use correct type for dma_sync_*_for_cpu() and dma_sync_*_for_device()

[Cyrille Pitchen]

dma_sync_*_for_cpu() and dma_sync_*_for_device() use 'enum dma_data_direction',
not 'enum dma_transfer_direction'

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/tty/serial/atmel_serial.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 4d848a2..756f567 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -794,7 +794,7 @@ static void atmel_tx_dma(struct uart_port *port)
 			return;
 		}
 
-		dma_sync_sg_for_device(port->dev, sg, 1, DMA_MEM_TO_DEV);
+		dma_sync_sg_for_device(port->dev, sg, 1, DMA_TO_DEVICE);
 
 		atmel_port->desc_tx = desc;
 		desc->callback = atmel_complete_tx_dma;
@@ -927,7 +927,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
 	dma_sync_sg_for_cpu(port->dev,
 			    &atmel_port->sg_rx,
 			    1,
-			    DMA_DEV_TO_MEM);
+			    DMA_FROM_DEVICE);
 
 	/*
 	 * ring->head points to the end of data already written by the DMA.
@@ -974,7 +974,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
 	dma_sync_sg_for_device(port->dev,
 			       &atmel_port->sg_rx,
 			       1,
-			       DMA_DEV_TO_MEM);
+			       DMA_FROM_DEVICE);
 
 	/*
 	 * Drop the lock here since it might end up calling
-- 
1.8.2.2

[PATCH 2/5] tty/serial: at91: enable peripheral clock before accessing I/O registers

[Cyrille Pitchen]

atmel_serial_probe() calls atmel_init_port(). In turn, atmel_init_port() calls
clk_disable_unprepare() to disable the peripheral clock before returning.

Later atmel_serial_probe() accesses some I/O registers such as the Mode and
Control registers for RS485 support then the Name and Version registers, through a call to
atmel_get_ip_name(), but at that moment the peripheral clock was still
disabled.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/tty/serial/atmel_serial.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 756f567..c6621dc 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2596,6 +2596,12 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	device_init_wakeup(&pdev->dev, 1);
 	platform_set_drvdata(pdev, port);
 
+	/*
+	 * The peripheral clock has been disabled by atmel_init_port():
+	 * enable it before accessing I/O registers
+	 */
+	clk_prepare_enable(port->clk);
+
 	if (rs485_enabled) {
 		UART_PUT_MR(&port->uart, ATMEL_US_USMODE_NORMAL);
 		UART_PUT_CR(&port->uart, ATMEL_US_RTSEN);
@@ -2606,6 +2612,12 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	 */
 	atmel_get_ip_name(&port->uart);
 
+	/*
+	 * The peripheral clock can now safely be disabled till the port
+	 * is used
+	 */
+	clk_disable_unprepare(port->clk);
+
 	return 0;
 
 err_add_port:
-- 
1.8.2.2

[PATCH 3/5] tty/serial: at91: fix error handling in atmel_serial_probe()

[Cyrille Pitchen]

-EDEFER error wasn't handle properly by atmel_serial_probe().
As an example, when atmel_serial_probe() is called for the first time, we pass
the test_and_set_bit() test to check whether the port has already been
initalized. Then we call atmel_init_port(), which may return -EDEFER, possibly
returned before by clk_get(). Consequently atmel_serial_probe() used to return
this error code WITHOUT clearing the port bit in the "atmel_ports_in_use" mask.
When atmel_serial_probe() was called for the second time, it used to fail on
the test_and_set_bit() function then returning -EBUSY.

When atmel_serial_probe() fails, this patch make it clear the port bit in the
"atmel_ports_in_use" mask, if needed, before returning the error code.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/tty/serial/atmel_serial.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index c6621dc..92a8b26 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2565,7 +2565,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
 
 	ret = atmel_init_port(port, pdev);
 	if (ret)
-		goto err;
+		goto err_clear_bit;
 
 	if (!atmel_use_pdc_rx(&port->uart)) {
 		ret = -ENOMEM;
@@ -2628,6 +2628,8 @@ err_alloc_ring:
 		clk_put(port->clk);
 		port->clk = NULL;
 	}
+err_clear_bit:
+	clear_bit(port->uart.line, atmel_ports_in_use);
 err:
 	return ret;
 }
-- 
1.8.2.2

[PATCH 4/5] tty/serial: at91: fix RTS line management when hardware handshake is enabled

[Cyrille Pitchen]

This patch fixes many bugs in the code dealing with the hardware handshake.

As an example, in atmel_set_termios(), we used to test whether the CRTSCTS
c_cflag was set. If so, we selected the "Hardware Handshake" mode through the
Mode Register. However, few lines below the mode was reset to "Normal" (0).
So there was no way to select the "Hardware Handshake" mode. To fix this issue,
we moved the CRTSCRTS c_cflag test AFTER the mode has been reset to "Normal".

Also setting the RTSEN and RTSDIS bits in the Control Register has different
results whether the USART is set in "Normal" or "Hardware Handshake" mode:

1) "Normal" mode
- the RTSEN bit forces the RTS line to low level, which tells the remote peer
  that we are ready to received new data.
- the RTSDIS bit forces the RTS line to high level, which tells the remote peer
  to stop sending new data.

2) "Hardware Handshake" mode
- the RTSEN bit forces the RTS line to high level.
- the RTSDIS bit lets the hardware control the RTS line.

WARNING:
when FIFOs are not available or not enabled, the RTS line is controlled by the
PDC. This is why using the Hardware Handshake mode requires using the PDC
channel for reception. However the Hardware Handshake mode DOES NOT work with
DMA controller since it cannot control the RTS line.
Future designs with FIFOs will introduce a new feature: the RTS line will be
controlled by the RX FIFO using thresholds. This patch was tested with this new
design.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/tty/serial/atmel_serial.c | 91 ++++++++++++++++++++++++++-------------
 1 file changed, 61 insertions(+), 30 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 92a8b26..9e0c636 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -341,13 +341,37 @@ static u_int atmel_tx_empty(struct uart_port *port)
 static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 {
 	unsigned int control = 0;
-	unsigned int mode;
+	unsigned int mode = UART_GET_MR(port);
+	unsigned int rts_paused, rts_ready;
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
+	/* override mode to RS485 if needed, otherwise keep the current mode */
+	if (port->rs485.flags & SER_RS485_ENABLED) {
+		if ((port->rs485.delay_rts_after_send) > 0)
+			UART_PUT_TTGR(port, port->rs485.delay_rts_after_send);
+		mode &= ~ATMEL_US_USMODE;
+		mode |= ATMEL_US_USMODE_RS485;
+	}
+
+	/* set the RTS line state according to the mode */
+	if ((mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_HWHS) {
+		/* force RTS line to high level */
+		rts_paused = ATMEL_US_RTSEN;
+
+		/* give the control of the RTS line back to the hardware */
+		rts_ready = ATMEL_US_RTSDIS;
+	} else {
+		/* force RTS line to high level */
+		rts_paused = ATMEL_US_RTSDIS;
+
+		/* force RTS line to low level */
+		rts_ready = ATMEL_US_RTSEN;
+	}
+
 	if (mctrl & TIOCM_RTS)
-		control |= ATMEL_US_RTSEN;
+		control |= rts_ready;
 	else
-		control |= ATMEL_US_RTSDIS;
+		control |= rts_paused;
 
 	if (mctrl & TIOCM_DTR)
 		control |= ATMEL_US_DTREN;
@@ -359,23 +383,12 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 	mctrl_gpio_set(atmel_port->gpios, mctrl);
 
 	/* Local loopback mode? */
-	mode = UART_GET_MR(port) & ~ATMEL_US_CHMODE;
+	mode &= ~ATMEL_US_CHMODE;
 	if (mctrl & TIOCM_LOOP)
 		mode |= ATMEL_US_CHMODE_LOC_LOOP;
 	else
 		mode |= ATMEL_US_CHMODE_NORMAL;
 
-	/* Resetting serial mode to RS232 (0x0) */
-	mode &= ~ATMEL_US_USMODE;
-
-	if (port->rs485.flags & SER_RS485_ENABLED) {
-		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if ((port->rs485.delay_rts_after_send) > 0)
-			UART_PUT_TTGR(port, port->rs485.delay_rts_after_send);
-		mode |= ATMEL_US_USMODE_RS485;
-	} else {
-		dev_dbg(port->dev, "Setting UART to RS232\n");
-	}
 	UART_PUT_MR(port, mode);
 }
 
@@ -1921,12 +1934,14 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 			      struct ktermios *old)
 {
 	unsigned long flags;
-	unsigned int mode, imr, quot, baud;
+	unsigned int old_mode, mode, imr, quot, baud;
+
+	/* save the current mode register */
+	mode = old_mode = UART_GET_MR(port);
 
-	/* Get current mode register */
-	mode = UART_GET_MR(port) & ~(ATMEL_US_USCLKS | ATMEL_US_CHRL
-					| ATMEL_US_NBSTOP | ATMEL_US_PAR
-					| ATMEL_US_USMODE);
+	/* reset the mode, clock divisor, parity, stop bits and data size */
+	mode &= ~(ATMEL_US_USCLKS | ATMEL_US_CHRL | ATMEL_US_NBSTOP |
+		  ATMEL_US_PAR | ATMEL_US_USMODE);
 
 	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
 	quot = uart_get_divisor(port, baud);
@@ -1971,12 +1986,6 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 	} else
 		mode |= ATMEL_US_PAR_NONE;
 
-	/* hardware handshake (RTS/CTS) */
-	if (termios->c_cflag & CRTSCTS)
-		mode |= ATMEL_US_USMODE_HWHS;
-	else
-		mode |= ATMEL_US_USMODE_NORMAL;
-
 	spin_lock_irqsave(&port->lock, flags);
 
 	port->read_status_mask = ATMEL_US_OVRE;
@@ -2020,18 +2029,40 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 	/* disable receiver and transmitter */
 	UART_PUT_CR(port, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
 
-	/* Resetting serial mode to RS232 (0x0) */
-	mode &= ~ATMEL_US_USMODE;
-
+	/* mode */
 	if (port->rs485.flags & SER_RS485_ENABLED) {
 		if ((port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port, port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
+	} else if (termios->c_cflag & CRTSCTS) {
+		/* RS232 with hardware handshake (RTS/CTS) */
+		mode |= ATMEL_US_USMODE_HWHS;
+	} else {
+		/* RS232 without hadware handshake */
+		mode |= ATMEL_US_USMODE_NORMAL;
 	}
 
-	/* set the parity, stop bits and data size */
+	/* set the mode, clock divisor, parity, stop bits and data size */
 	UART_PUT_MR(port, mode);
 
+	/*
+	 * when switching the mode, set the RTS line state according to the
+	 * new mode, otherwise keep the former state
+	 */
+	if ((old_mode & ATMEL_US_USMODE) != (mode & ATMEL_US_USMODE)) {
+		unsigned int rts_state;
+
+		if ((mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_HWHS) {
+			/* let the hardware control the RTS line */
+			rts_state = ATMEL_US_RTSDIS;
+		} else {
+			/* force RTS line to low level */
+			rts_state = ATMEL_US_RTSEN;
+		}
+
+		UART_PUT_CR(port, rts_state);
+	}
+
 	/* set the baud rate */
 	UART_PUT_BRGR(port, quot);
 	UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
-- 
1.8.2.2

[PATCH 5/5] tty/serial: at91: fix typo and indentation

[Cyrille Pitchen]

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/tty/serial/atmel_serial.c | 40 +++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 9e0c636..846552b 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -738,7 +738,11 @@ static void atmel_complete_tx_dma(void *arg)
 	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
 
-	/* Do we really need this? */
+	/*
+	 * xmit is a circular buffer so, if we have just send data from
+	 * xmit->tail to the end of xmit->buf, now we have to transmit the
+	 * remaining data from the beginning of xmit->buf to xmit->head.
+	 */
 	if (!uart_circ_empty(xmit))
 		tasklet_schedule(&atmel_port->tasklet);
 
@@ -797,11 +801,11 @@ static void atmel_tx_dma(struct uart_port *port)
 		BUG_ON(!sg_dma_len(sg));
 
 		desc = dmaengine_prep_slave_sg(chan,
-						sg,
-						1,
-						DMA_MEM_TO_DEV,
-						DMA_PREP_INTERRUPT |
-						DMA_CTRL_ACK);
+					       sg,
+					       1,
+					       DMA_MEM_TO_DEV,
+					       DMA_PREP_INTERRUPT |
+					       DMA_CTRL_ACK);
 		if (!desc) {
 			dev_err(port->dev, "Failed to send via dma!\n");
 			return;
@@ -1025,13 +1029,13 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
 	/* UART circular rx buffer is an aligned page. */
 	BUG_ON((int)port->state->xmit.buf & ~PAGE_MASK);
 	sg_set_page(&atmel_port->sg_rx,
-			virt_to_page(ring->buf),
-			ATMEL_SERIAL_RINGSIZE,
-			(int)ring->buf & ~PAGE_MASK);
-			nent = dma_map_sg(port->dev,
-					&atmel_port->sg_rx,
-					1,
-					DMA_FROM_DEVICE);
+		    virt_to_page(ring->buf),
+		    ATMEL_SERIAL_RINGSIZE,
+		    (int)ring->buf & ~PAGE_MASK);
+	nent = dma_map_sg(port->dev,
+			  &atmel_port->sg_rx,
+			  1,
+			  DMA_FROM_DEVICE);
 
 	if (!nent) {
 		dev_dbg(port->dev, "need to release resource of dma\n");
@@ -1060,11 +1064,11 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
 	 * each one is half ring buffer size
 	 */
 	desc = dmaengine_prep_dma_cyclic(atmel_port->chan_rx,
-				sg_dma_address(&atmel_port->sg_rx),
-				sg_dma_len(&atmel_port->sg_rx),
-				sg_dma_len(&atmel_port->sg_rx)/2,
-				DMA_DEV_TO_MEM,
-				DMA_PREP_INTERRUPT);
+					 sg_dma_address(&atmel_port->sg_rx),
+					 sg_dma_len(&atmel_port->sg_rx),
+					 sg_dma_len(&atmel_port->sg_rx)/2,
+					 DMA_DEV_TO_MEM,
+					 DMA_PREP_INTERRUPT);
 	desc->callback = atmel_complete_rx_dma;
 	desc->callback_param = port;
 	atmel_port->desc_rx = desc;
-- 
1.8.2.2

Re: [PATCH 2/5] tty/serial: at91: enable peripheral clock before accessing I/O registers

[Nicolas Ferre]

Le 09/12/2014 14:31, Cyrille Pitchen a écrit :
> atmel_serial_probe() calls atmel_init_port(). In turn, atmel_init_port() calls
> clk_disable_unprepare() to disable the peripheral clock before returning.
> 
> Later atmel_serial_probe() accesses some I/O registers such as the Mode and
> Control registers for RS485 support then the Name and Version registers, through a call to
> atmel_get_ip_name(), but at that moment the peripheral clock was still
> disabled.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
and you can add also:
Cc: <stable@vger.kernel.org> # 3.12+

> ---
>  drivers/tty/serial/atmel_serial.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 756f567..c6621dc 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2596,6 +2596,12 @@ static int atmel_serial_probe(struct platform_device *pdev)
>  	device_init_wakeup(&pdev->dev, 1);
>  	platform_set_drvdata(pdev, port);
>  
> +	/*
> +	 * The peripheral clock has been disabled by atmel_init_port():
> +	 * enable it before accessing I/O registers
> +	 */
> +	clk_prepare_enable(port->clk);
> +
>  	if (rs485_enabled) {
>  		UART_PUT_MR(&port->uart, ATMEL_US_USMODE_NORMAL);
>  		UART_PUT_CR(&port->uart, ATMEL_US_RTSEN);
> @@ -2606,6 +2612,12 @@ static int atmel_serial_probe(struct platform_device *pdev)
>  	 */
>  	atmel_get_ip_name(&port->uart);
>  
> +	/*
> +	 * The peripheral clock can now safely be disabled till the port
> +	 * is used
> +	 */
> +	clk_disable_unprepare(port->clk);
> +
>  	return 0;
>  
>  err_add_port:
> 


-- 
Nicolas Ferre

Re: [PATCH 1/5] tty/serial: at91: use correct type for dma_sync_*_for_cpu() and dma_sync_*_for_device()

[Nicolas Ferre]

Le 09/12/2014 14:31, Cyrille Pitchen a écrit :
> dma_sync_*_for_cpu() and dma_sync_*_for_device() use 'enum dma_data_direction',
> not 'enum dma_transfer_direction'
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

Sure:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Here it's difficult to add a "stable" tag as the error was introduced by
several different patches.

Thanks.

> ---
>  drivers/tty/serial/atmel_serial.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 4d848a2..756f567 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -794,7 +794,7 @@ static void atmel_tx_dma(struct uart_port *port)
>  			return;
>  		}
>  
> -		dma_sync_sg_for_device(port->dev, sg, 1, DMA_MEM_TO_DEV);
> +		dma_sync_sg_for_device(port->dev, sg, 1, DMA_TO_DEVICE);
>  
>  		atmel_port->desc_tx = desc;
>  		desc->callback = atmel_complete_tx_dma;
> @@ -927,7 +927,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
>  	dma_sync_sg_for_cpu(port->dev,
>  			    &atmel_port->sg_rx,
>  			    1,
> -			    DMA_DEV_TO_MEM);
> +			    DMA_FROM_DEVICE);
>  
>  	/*
>  	 * ring->head points to the end of data already written by the DMA.
> @@ -974,7 +974,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
>  	dma_sync_sg_for_device(port->dev,
>  			       &atmel_port->sg_rx,
>  			       1,
> -			       DMA_DEV_TO_MEM);
> +			       DMA_FROM_DEVICE);
>  
>  	/*
>  	 * Drop the lock here since it might end up calling
> 


-- 
Nicolas Ferre

Re: [PATCH 3/5] tty/serial: at91: fix error handling in atmel_serial_probe()

[Nicolas Ferre]

Le 09/12/2014 14:31, Cyrille Pitchen a écrit :
> -EDEFER error wasn't handle properly by atmel_serial_probe().
> As an example, when atmel_serial_probe() is called for the first time, we pass
> the test_and_set_bit() test to check whether the port has already been
> initalized. Then we call atmel_init_port(), which may return -EDEFER, possibly
> returned before by clk_get(). Consequently atmel_serial_probe() used to return
> this error code WITHOUT clearing the port bit in the "atmel_ports_in_use" mask.
> When atmel_serial_probe() was called for the second time, it used to fail on
> the test_and_set_bit() function then returning -EBUSY.
> 
> When atmel_serial_probe() fails, this patch make it clear the port bit in the
> "atmel_ports_in_use" mask, if needed, before returning the error code.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
and you can also add:
Cc: <stable@vger.kernel.org> # 3.12+

Thanks.

> ---
>  drivers/tty/serial/atmel_serial.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index c6621dc..92a8b26 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2565,7 +2565,7 @@ static int atmel_serial_probe(struct platform_device *pdev)
>  
>  	ret = atmel_init_port(port, pdev);
>  	if (ret)
> -		goto err;
> +		goto err_clear_bit;
>  
>  	if (!atmel_use_pdc_rx(&port->uart)) {
>  		ret = -ENOMEM;
> @@ -2628,6 +2628,8 @@ err_alloc_ring:
>  		clk_put(port->clk);
>  		port->clk = NULL;
>  	}
> +err_clear_bit:
> +	clear_bit(port->uart.line, atmel_ports_in_use);
>  err:
>  	return ret;
>  }
> 


-- 
Nicolas Ferre

Re: [PATCH 4/5] tty/serial: at91: fix RTS line management when hardware handshake is enabled

[Nicolas Ferre]

Le 09/12/2014 14:31, Cyrille Pitchen a écrit :
> This patch fixes many bugs in the code dealing with the hardware handshake.
> 
> As an example, in atmel_set_termios(), we used to test whether the CRTSCTS
> c_cflag was set. If so, we selected the "Hardware Handshake" mode through the
> Mode Register. However, few lines below the mode was reset to "Normal" (0).
> So there was no way to select the "Hardware Handshake" mode. To fix this issue,
> we moved the CRTSCRTS c_cflag test AFTER the mode has been reset to "Normal".
> 
> Also setting the RTSEN and RTSDIS bits in the Control Register has different
> results whether the USART is set in "Normal" or "Hardware Handshake" mode:
> 
> 1) "Normal" mode
> - the RTSEN bit forces the RTS line to low level, which tells the remote peer
>   that we are ready to received new data.
> - the RTSDIS bit forces the RTS line to high level, which tells the remote peer
>   to stop sending new data.
> 
> 2) "Hardware Handshake" mode
> - the RTSEN bit forces the RTS line to high level.
> - the RTSDIS bit lets the hardware control the RTS line.
> 
> WARNING:
> when FIFOs are not available or not enabled, the RTS line is controlled by the
> PDC. This is why using the Hardware Handshake mode requires using the PDC
> channel for reception. However the Hardware Handshake mode DOES NOT work with
> DMA controller since it cannot control the RTS line.
> Future designs with FIFOs will introduce a new feature: the RTS line will be
> controlled by the RX FIFO using thresholds. This patch was tested with this new
> design.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

Indeed. It fixes a pretty important misbehavior.

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Thanks.

> ---
>  drivers/tty/serial/atmel_serial.c | 91 ++++++++++++++++++++++++++-------------
>  1 file changed, 61 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 92a8b26..9e0c636 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -341,13 +341,37 @@ static u_int atmel_tx_empty(struct uart_port *port)
>  static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  {
>  	unsigned int control = 0;
> -	unsigned int mode;
> +	unsigned int mode = UART_GET_MR(port);
> +	unsigned int rts_paused, rts_ready;
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>  
> +	/* override mode to RS485 if needed, otherwise keep the current mode */
> +	if (port->rs485.flags & SER_RS485_ENABLED) {
> +		if ((port->rs485.delay_rts_after_send) > 0)
> +			UART_PUT_TTGR(port, port->rs485.delay_rts_after_send);
> +		mode &= ~ATMEL_US_USMODE;
> +		mode |= ATMEL_US_USMODE_RS485;
> +	}
> +
> +	/* set the RTS line state according to the mode */
> +	if ((mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_HWHS) {
> +		/* force RTS line to high level */
> +		rts_paused = ATMEL_US_RTSEN;
> +
> +		/* give the control of the RTS line back to the hardware */
> +		rts_ready = ATMEL_US_RTSDIS;
> +	} else {
> +		/* force RTS line to high level */
> +		rts_paused = ATMEL_US_RTSDIS;
> +
> +		/* force RTS line to low level */
> +		rts_ready = ATMEL_US_RTSEN;
> +	}
> +
>  	if (mctrl & TIOCM_RTS)
> -		control |= ATMEL_US_RTSEN;
> +		control |= rts_ready;
>  	else
> -		control |= ATMEL_US_RTSDIS;
> +		control |= rts_paused;
>  
>  	if (mctrl & TIOCM_DTR)
>  		control |= ATMEL_US_DTREN;
> @@ -359,23 +383,12 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  	mctrl_gpio_set(atmel_port->gpios, mctrl);
>  
>  	/* Local loopback mode? */
> -	mode = UART_GET_MR(port) & ~ATMEL_US_CHMODE;
> +	mode &= ~ATMEL_US_CHMODE;
>  	if (mctrl & TIOCM_LOOP)
>  		mode |= ATMEL_US_CHMODE_LOC_LOOP;
>  	else
>  		mode |= ATMEL_US_CHMODE_NORMAL;
>  
> -	/* Resetting serial mode to RS232 (0x0) */
> -	mode &= ~ATMEL_US_USMODE;
> -
> -	if (port->rs485.flags & SER_RS485_ENABLED) {
> -		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if ((port->rs485.delay_rts_after_send) > 0)
> -			UART_PUT_TTGR(port, port->rs485.delay_rts_after_send);
> -		mode |= ATMEL_US_USMODE_RS485;
> -	} else {
> -		dev_dbg(port->dev, "Setting UART to RS232\n");
> -	}
>  	UART_PUT_MR(port, mode);
>  }
>  
> @@ -1921,12 +1934,14 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  			      struct ktermios *old)
>  {
>  	unsigned long flags;
> -	unsigned int mode, imr, quot, baud;
> +	unsigned int old_mode, mode, imr, quot, baud;
> +
> +	/* save the current mode register */
> +	mode = old_mode = UART_GET_MR(port);
>  
> -	/* Get current mode register */
> -	mode = UART_GET_MR(port) & ~(ATMEL_US_USCLKS | ATMEL_US_CHRL
> -					| ATMEL_US_NBSTOP | ATMEL_US_PAR
> -					| ATMEL_US_USMODE);
> +	/* reset the mode, clock divisor, parity, stop bits and data size */
> +	mode &= ~(ATMEL_US_USCLKS | ATMEL_US_CHRL | ATMEL_US_NBSTOP |
> +		  ATMEL_US_PAR | ATMEL_US_USMODE);
>  
>  	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
>  	quot = uart_get_divisor(port, baud);
> @@ -1971,12 +1986,6 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  	} else
>  		mode |= ATMEL_US_PAR_NONE;
>  
> -	/* hardware handshake (RTS/CTS) */
> -	if (termios->c_cflag & CRTSCTS)
> -		mode |= ATMEL_US_USMODE_HWHS;
> -	else
> -		mode |= ATMEL_US_USMODE_NORMAL;
> -
>  	spin_lock_irqsave(&port->lock, flags);
>  
>  	port->read_status_mask = ATMEL_US_OVRE;
> @@ -2020,18 +2029,40 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  	/* disable receiver and transmitter */
>  	UART_PUT_CR(port, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
>  
> -	/* Resetting serial mode to RS232 (0x0) */
> -	mode &= ~ATMEL_US_USMODE;
> -
> +	/* mode */
>  	if (port->rs485.flags & SER_RS485_ENABLED) {
>  		if ((port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port, port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
> +	} else if (termios->c_cflag & CRTSCTS) {
> +		/* RS232 with hardware handshake (RTS/CTS) */
> +		mode |= ATMEL_US_USMODE_HWHS;
> +	} else {
> +		/* RS232 without hadware handshake */
> +		mode |= ATMEL_US_USMODE_NORMAL;
>  	}
>  
> -	/* set the parity, stop bits and data size */
> +	/* set the mode, clock divisor, parity, stop bits and data size */
>  	UART_PUT_MR(port, mode);
>  
> +	/*
> +	 * when switching the mode, set the RTS line state according to the
> +	 * new mode, otherwise keep the former state
> +	 */
> +	if ((old_mode & ATMEL_US_USMODE) != (mode & ATMEL_US_USMODE)) {
> +		unsigned int rts_state;
> +
> +		if ((mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_HWHS) {
> +			/* let the hardware control the RTS line */
> +			rts_state = ATMEL_US_RTSDIS;
> +		} else {
> +			/* force RTS line to low level */
> +			rts_state = ATMEL_US_RTSEN;
> +		}
> +
> +		UART_PUT_CR(port, rts_state);
> +	}
> +
>  	/* set the baud rate */
>  	UART_PUT_BRGR(port, quot);
>  	UART_PUT_CR(port, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
> 


-- 
Nicolas Ferre

Re: [PATCH 5/5] tty/serial: at91: fix typo and indentation

[Nicolas Ferre]

Le 09/12/2014 14:31, Cyrille Pitchen a écrit :
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

thanks Cyrille. Bye.

> ---
>  drivers/tty/serial/atmel_serial.c | 40 +++++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 9e0c636..846552b 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -738,7 +738,11 @@ static void atmel_complete_tx_dma(void *arg)
>  	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
>  		uart_write_wakeup(port);
>  
> -	/* Do we really need this? */
> +	/*
> +	 * xmit is a circular buffer so, if we have just send data from
> +	 * xmit->tail to the end of xmit->buf, now we have to transmit the
> +	 * remaining data from the beginning of xmit->buf to xmit->head.
> +	 */
>  	if (!uart_circ_empty(xmit))
>  		tasklet_schedule(&atmel_port->tasklet);
>  
> @@ -797,11 +801,11 @@ static void atmel_tx_dma(struct uart_port *port)
>  		BUG_ON(!sg_dma_len(sg));
>  
>  		desc = dmaengine_prep_slave_sg(chan,
> -						sg,
> -						1,
> -						DMA_MEM_TO_DEV,
> -						DMA_PREP_INTERRUPT |
> -						DMA_CTRL_ACK);
> +					       sg,
> +					       1,
> +					       DMA_MEM_TO_DEV,
> +					       DMA_PREP_INTERRUPT |
> +					       DMA_CTRL_ACK);
>  		if (!desc) {
>  			dev_err(port->dev, "Failed to send via dma!\n");
>  			return;
> @@ -1025,13 +1029,13 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>  	/* UART circular rx buffer is an aligned page. */
>  	BUG_ON((int)port->state->xmit.buf & ~PAGE_MASK);
>  	sg_set_page(&atmel_port->sg_rx,
> -			virt_to_page(ring->buf),
> -			ATMEL_SERIAL_RINGSIZE,
> -			(int)ring->buf & ~PAGE_MASK);
> -			nent = dma_map_sg(port->dev,
> -					&atmel_port->sg_rx,
> -					1,
> -					DMA_FROM_DEVICE);
> +		    virt_to_page(ring->buf),
> +		    ATMEL_SERIAL_RINGSIZE,
> +		    (int)ring->buf & ~PAGE_MASK);
> +	nent = dma_map_sg(port->dev,
> +			  &atmel_port->sg_rx,
> +			  1,
> +			  DMA_FROM_DEVICE);
>  
>  	if (!nent) {
>  		dev_dbg(port->dev, "need to release resource of dma\n");
> @@ -1060,11 +1064,11 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>  	 * each one is half ring buffer size
>  	 */
>  	desc = dmaengine_prep_dma_cyclic(atmel_port->chan_rx,
> -				sg_dma_address(&atmel_port->sg_rx),
> -				sg_dma_len(&atmel_port->sg_rx),
> -				sg_dma_len(&atmel_port->sg_rx)/2,
> -				DMA_DEV_TO_MEM,
> -				DMA_PREP_INTERRUPT);
> +					 sg_dma_address(&atmel_port->sg_rx),
> +					 sg_dma_len(&atmel_port->sg_rx),
> +					 sg_dma_len(&atmel_port->sg_rx)/2,
> +					 DMA_DEV_TO_MEM,
> +					 DMA_PREP_INTERRUPT);
>  	desc->callback = atmel_complete_rx_dma;
>  	desc->callback_param = port;
>  	atmel_port->desc_rx = desc;
> 


-- 
Nicolas Ferre

Re: [PATCH 1/5] tty/serial: at91: use correct type for dma_sync_*_for_cpu() and dma_sync_*_for_device()

[Greg KH]

On Fri, Jan 09, 2015 at 04:13:59PM +0100, Nicolas Ferre wrote:
> Le 09/12/2014 14:31, Cyrille Pitchen a écrit :
> > dma_sync_*_for_cpu() and dma_sync_*_for_device() use 'enum dma_data_direction',
> > not 'enum dma_transfer_direction'
> > 
> > Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> 
> Sure:
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Here it's difficult to add a "stable" tag as the error was introduced by
> several different patches.

So that means it's not ok for older kernels?  That doesn't make sense...