Re: [PATCH v2 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped

2019-03-26 Thread Richard Genoud
Le 19/03/2019 à 14:20, Razvan Stefanescu a écrit :
> In half-duplex operation, RX should be started after TX completes.
> 
> If DMA is used, there is a case when the DMA transfer completes but the
> TX FIFO is not emptied, so the RX cannot be restarted just yet.
> 
> Use a boolean variable to store this state and rearm TX interrupt mask
> to be signaled again that the transfer finished. In interrupt transmit
> handler this variable is used to start RX. A warning message is generated
> if RX is activated before TX fifo is cleared.
> 
> Fixes: b389f173aaa1 ("tty/serial: atmel: RS485 half duplex w/DMA: enable
> RX after TX is done")
> Signed-off-by: Razvan Stefanescu 
Acked-by: Richard Genoud 

NB: backport on kernel older than 4.20 will fail because of the iso7816
variables fidi_min/fidi_max.
> ---
> Changelog:
> v2:
>   - start RX and display warning in case of error
>   - add fix info
> 
>  drivers/tty/serial/atmel_serial.c | 25 ++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c 
> b/drivers/tty/serial/atmel_serial.c
> index b4b89a16a41b..5b2f859c327c 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -166,6 +166,8 @@ struct atmel_uart_port {
>   unsigned intpending_status;
>   spinlock_t  lock_suspended;
>  
> + boolhd_start_rx;/* can start RX during 
> half-duplex operation */
> +
>   /* ISO7816 */
>   unsigned intfidi_min;
>   unsigned intfidi_max;
> @@ -933,8 +935,13 @@ static void atmel_complete_tx_dma(void *arg)
>   if (!uart_circ_empty(xmit))
>   atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
>   else if (atmel_uart_is_half_duplex(port)) {
> - /* DMA done, stop TX, start RX for RS485 */
> - atmel_start_rx(port);
> + /*
> +  * DMA done, re-enable TXEMPTY and signal that we can stop
> +  * TX and start RX for RS485
> +  */
> + atmel_port->hd_start_rx = true;
> + atmel_uart_writel(port, ATMEL_US_IER,
> +   atmel_port->tx_done_mask);
>   }
>  
>   spin_unlock_irqrestore(&port->lock, flags);
> @@ -1378,9 +1385,20 @@ atmel_handle_transmit(struct uart_port *port, unsigned 
> int pending)
>   struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>  
>   if (pending & atmel_port->tx_done_mask) {
> - /* Either PDC or interrupt transmission */
>   atmel_uart_writel(port, ATMEL_US_IDR,
> atmel_port->tx_done_mask);
> +
> + /* Start RX if flag was set and FIFO is empty */
> + if (atmel_port->hd_start_rx) {
> + if (!(atmel_uart_readl(port, ATMEL_US_CSR)
> + & ATMEL_US_TXEMPTY))
> + dev_warn(port->dev, "Should start RX, but TX 
> fifo is not empty\n");
> +
> + atmel_port->hd_start_rx = false;
> + atmel_start_rx(port);
> + return;
> + }
> +
>   atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
>   }
>  }
> 



[PATCH v2 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped

2019-03-19 Thread Razvan Stefanescu
In half-duplex operation, RX should be started after TX completes.

If DMA is used, there is a case when the DMA transfer completes but the
TX FIFO is not emptied, so the RX cannot be restarted just yet.

Use a boolean variable to store this state and rearm TX interrupt mask
to be signaled again that the transfer finished. In interrupt transmit
handler this variable is used to start RX. A warning message is generated
if RX is activated before TX fifo is cleared.

Fixes: b389f173aaa1 ("tty/serial: atmel: RS485 half duplex w/DMA: enable
RX after TX is done")
Signed-off-by: Razvan Stefanescu 
---
Changelog:
v2:
  - start RX and display warning in case of error
  - add fix info

 drivers/tty/serial/atmel_serial.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c 
b/drivers/tty/serial/atmel_serial.c
index b4b89a16a41b..5b2f859c327c 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -166,6 +166,8 @@ struct atmel_uart_port {
unsigned intpending_status;
spinlock_t  lock_suspended;
 
+   boolhd_start_rx;/* can start RX during 
half-duplex operation */
+
/* ISO7816 */
unsigned intfidi_min;
unsigned intfidi_max;
@@ -933,8 +935,13 @@ static void atmel_complete_tx_dma(void *arg)
if (!uart_circ_empty(xmit))
atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
else if (atmel_uart_is_half_duplex(port)) {
-   /* DMA done, stop TX, start RX for RS485 */
-   atmel_start_rx(port);
+   /*
+* DMA done, re-enable TXEMPTY and signal that we can stop
+* TX and start RX for RS485
+*/
+   atmel_port->hd_start_rx = true;
+   atmel_uart_writel(port, ATMEL_US_IER,
+ atmel_port->tx_done_mask);
}
 
spin_unlock_irqrestore(&port->lock, flags);
@@ -1378,9 +1385,20 @@ atmel_handle_transmit(struct uart_port *port, unsigned 
int pending)
struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
if (pending & atmel_port->tx_done_mask) {
-   /* Either PDC or interrupt transmission */
atmel_uart_writel(port, ATMEL_US_IDR,
  atmel_port->tx_done_mask);
+
+   /* Start RX if flag was set and FIFO is empty */
+   if (atmel_port->hd_start_rx) {
+   if (!(atmel_uart_readl(port, ATMEL_US_CSR)
+   & ATMEL_US_TXEMPTY))
+   dev_warn(port->dev, "Should start RX, but TX 
fifo is not empty\n");
+
+   atmel_port->hd_start_rx = false;
+   atmel_start_rx(port);
+   return;
+   }
+
atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
}
 }
-- 
2.19.1