RE: [PATCH] tty/serial: atmel: RS485 half duplex w/DMA: enable RX after TX is done

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Alexandre,

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@xxxxxxxxxxxxxxxxxx]
> Sent: Tuesday, April 11, 2017 5:06 PM
> To: Bryan Evenson <bevenson@xxxxxxxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>; Richard Genoud
> <richard.genoud@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx; Alexandre Belloni <alexandre.belloni@free-
> electrons.com>
> Subject: [PATCH] tty/serial: atmel: RS485 half duplex w/DMA: enable RX after
> TX is done
> 
> From: Richard Genoud <richard.genoud@xxxxxxxxx>
> 
> Commit b389f173aaa1204d6dc1f299082a162eb0491545 upstream.
> 
> When using RS485 in half duplex, RX should be enabled when TX is
> finished, and stopped when TX starts.
> 
> Before commit 0058f0871efe7b01c6 ("tty/serial: atmel: fix RS485 half
> duplex with DMA"), RX was not disabled in atmel_start_tx() if the DMA
> was used. So, collisions could happened.
> 
> But disabling RX in atmel_start_tx() uncovered another bug:
> RX was enabled again in the wrong place (in atmel_tx_dma) instead of
> being enabled when TX is finished (in atmel_complete_tx_dma), so the
> transmission simply stopped.
> 
> This bug was not triggered before commit 0058f0871efe7b01c6
> ("tty/serial: atmel: fix RS485 half duplex with DMA") because RX was
> never disabled before.
> 
> Moving atmel_start_rx() in atmel_complete_tx_dma() corrects the problem.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Gil Weber <webergil@xxxxxxxxx>
> Fixes: 0058f0871efe7b01c6
> Tested-by: Gil Weber <webergil@xxxxxxxxx>
> Signed-off-by: Richard Genoud <richard.genoud@xxxxxxxxx>
> Acked-by: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@xxxxxxxxxxxxxxxxxx>
> ---
> 
> Hi, this is b389f173aaa1204d6dc1f299082a162eb0491545 backported to v4.4
> and v4.1
> 
> Note: this has only been build tested. Bryan, please test.

Initially looks good, but I'm running a few more tests for completeness.  I should have a final answer later today.

Bryan

> 
>  drivers/tty/serial/atmel_serial.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index a15070a7fcd6..53e4d5056db7 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -810,6 +810,11 @@ static void atmel_complete_tx_dma(void *arg)
>  	 */
>  	if (!uart_circ_empty(xmit))
>  		tasklet_schedule(&atmel_port->tasklet);
> +	else if ((port->rs485.flags & SER_RS485_ENABLED) &&
> +		 !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
> +		/* DMA done, stop TX, start RX for RS485 */
> +		atmel_start_rx(port);
> +	}
> 
>  	spin_unlock_irqrestore(&port->lock, flags);
>  }
> @@ -912,12 +917,6 @@ static void atmel_tx_dma(struct uart_port *port)
>  		desc->callback = atmel_complete_tx_dma;
>  		desc->callback_param = atmel_port;
>  		atmel_port->cookie_tx = dmaengine_submit(desc);
> -
> -	} else {
> -		if (port->rs485.flags & SER_RS485_ENABLED) {
> -			/* DMA done, stop TX, start RX for RS485 */
> -			atmel_start_rx(port);
> -		}
>  	}
> 
>  	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> --
> 2.11.0





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]