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

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

[Richard Genoud]

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@vger.kernel.org
Reported-by: Gil Weber <webergil@gmail.com>
Tested-by: Gil Weber <webergil@gmail.com>
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 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 168b10cad47b..11c0117af80b 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -798,6 +798,11 @@ static void atmel_complete_tx_dma(void *arg)
 	 */
 	if (!uart_circ_empty(xmit))
 		atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
+	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);
 }
@@ -900,12 +905,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)

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

[Alexandre Belloni]

On 06/12/2016 at 13:05:33 +0100, Richard Genoud wrote :
> 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@vger.kernel.org
> Reported-by: Gil Weber <webergil@gmail.com>
> Tested-by: Gil Weber <webergil@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

[Richard Genoud]

2017-01-02 12:53 GMT+01:00 Alexandre Belloni
<alexandre.belloni@free-electrons.com>:
> On 06/12/2016 at 13:05:33 +0100, Richard Genoud wrote :
>> 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@vger.kernel.org
>> Reported-by: Gil Weber <webergil@gmail.com>
>> Tested-by: Gil Weber <webergil@gmail.com>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>
Greg, could you take this patch in your tree ?

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

[Greg KH]

On Mon, Jan 02, 2017 at 03:14:37PM +0100, Richard Genoud wrote:
> 2017-01-02 12:53 GMT+01:00 Alexandre Belloni
> <alexandre.belloni@free-electrons.com>:
> > On 06/12/2016 at 13:05:33 +0100, Richard Genoud wrote :
> >> 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@vger.kernel.org
> >> Reported-by: Gil Weber <webergil@gmail.com>
> >> Tested-by: Gil Weber <webergil@gmail.com>
> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> >
> > Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> >
> Greg, could you take this patch in your tree ?

Yes, will do so, thanks.

greg k-h

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

[Alexandre Belloni]

From: Richard Genoud <richard.genoud@gmail.com>

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@vger.kernel.org
Reported-by: Gil Weber <webergil@gmail.com>
Fixes: 0058f0871efe7b01c6
Tested-by: Gil Weber <webergil@gmail.com>
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---

Hi, this is b389f173aaa1204d6dc1f299082a162eb0491545 backported to v4.4 and v4.1

Note: this has only been build tested. Bryan, please test.

 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

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

[Bryan Evenson]

Alexandre,

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com]
> Sent: Tuesday, April 11, 2017 5:06 PM
> To: Bryan Evenson <bevenson@melinkcorp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Richard Genoud
> <richard.genoud@gmail.com>
> Cc: stable@vger.kernel.org; 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@gmail.com>
> 
> 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@vger.kernel.org
> Reported-by: Gil Weber <webergil@gmail.com>
> Fixes: 0058f0871efe7b01c6
> Tested-by: Gil Weber <webergil@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> 
> 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

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

[Bryan Evenson]

All,

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com]
> Sent: Tuesday, April 11, 2017 5:06 PM
> To: Bryan Evenson <bevenson@melinkcorp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Richard Genoud
> <richard.genoud@gmail.com>
> Cc: stable@vger.kernel.org; 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@gmail.com>
> 
> 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@vger.kernel.org
> Reported-by: Gil Weber <webergil@gmail.com>
> Fixes: 0058f0871efe7b01c6
> Tested-by: Gil Weber <webergil@gmail.com>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> 
> Hi, this is b389f173aaa1204d6dc1f299082a162eb0491545 backported to v4.4
> and v4.1
> 
> Note: this has only been build tested. Bryan, please test.

Tested with Atmel AT91SAM9G25 with nine devices connected to USART0 at 9600 baud, three devices connected to USART1 at 115200 baud, and two devices connected to USART3 at 115200 baud.  Verified with v4.4.61 that TX and RX DMA works on all three serial ports with the patch, and without the patch that data transactions fail when DMA is enabled.  Tested port open/close, power cycling the system and rebooting, and in all cases, all ports remained operational with the patch.

With v4.1.39, TX DMA works on all three serial ports with the patch, and without the patch all transactions fail with TX DMA enabled.  I was unable to use both RX and TX DMA with v4.1.39 even with the patch.  However, there are several other patches in v4.4 related to Atmel's serial DMA that are not in v4.1, which from my testing looks to be the reason why I can't operate with both RX and TX DMA with v4.1.39.  For today's purpose, the patch does fix the listed issue on v4.1.39.

Tested-by: Bryan Evenson <bevenson@melinkcorp.com>

> 
>  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