diff mbox series

tty/serial: atmel: RS485 & ISO7816: wait for TXRDY before sending data

Message ID 20200107111656.26308-1-codrin.ciubotariu@microchip.com (mailing list archive)
State Mainlined
Commit 477b8383100023ea0769979cff67e9be3a720397
Headers show
Series tty/serial: atmel: RS485 & ISO7816: wait for TXRDY before sending data | expand

Commit Message

Codrin Ciubotariu Jan. 7, 2020, 11:17 a.m. UTC
At this moment, TXEMPTY is checked before sending data on RS485 and ISO7816
modes. However, TXEMPTY is risen when FIFO (if used) or the Transmit Shift
Register are empty, even though TXRDY might be up and controller is able to
receive data. Since the controller sends data only when TXEMPTY is ready,
on RS485, when DMA is not used, the RTS pin is driven low after each byte.
With this patch, the characters will be transmitted when TXRDY is up and
so, RTS pin will remain high between bytes.
The performance improvement on RS485 is about 8% with a baudrate of 300.

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
---
 drivers/tty/serial/atmel_serial.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

Comments

Richard Genoud Jan. 8, 2020, 1:45 p.m. UTC | #1
Le 07/01/2020 à 12:17, Codrin.Ciubotariu@microchip.com a écrit :
> At this moment, TXEMPTY is checked before sending data on RS485 and ISO7816
> modes. However, TXEMPTY is risen when FIFO (if used) or the Transmit Shift
> Register are empty, even though TXRDY might be up and controller is able to
> receive data. Since the controller sends data only when TXEMPTY is ready,
> on RS485, when DMA is not used, the RTS pin is driven low after each byte.
> With this patch, the characters will be transmitted when TXRDY is up and
> so, RTS pin will remain high between bytes.
> The performance improvement on RS485 is about 8% with a baudrate of 300.
> 
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
seems ok to me
Acked-by: Richard Genoud <richard.genoud@gmail.com>

NB: MS exchange has added some =3D and =20 here and there, but git am
doesn't seems to be bothered by them.
> ---
>  drivers/tty/serial/atmel_serial.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index a8dc8af83f39..19c8fb9faa36 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -313,7 +313,11 @@ static int atmel_config_rs485(struct uart_port *port,
>  
>  	if (rs485conf->flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> +		if (port->rs485.flags & SER_RS485_RX_DURING_TX)
> +			atmel_port->tx_done_mask = ATMEL_US_TXRDY;
> +		else
> +			atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> +
>  		atmel_uart_writel(port, ATMEL_US_TTGR,
>  				  rs485conf->delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
> @@ -831,7 +835,7 @@ static void atmel_tx_chars(struct uart_port *port)
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>  
>  	if (port->x_char &&
> -	    (atmel_uart_readl(port, ATMEL_US_CSR) & atmel_port->tx_done_mask)) {
> +	    (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY)) {
>  		atmel_uart_write_char(port, port->x_char);
>  		port->icount.tx++;
>  		port->x_char = 0;
> @@ -839,8 +843,7 @@ static void atmel_tx_chars(struct uart_port *port)
>  	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
>  		return;
>  
> -	while (atmel_uart_readl(port, ATMEL_US_CSR) &
> -	       atmel_port->tx_done_mask) {
> +	while (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY) {
>  		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
>  		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
>  		port->icount.tx++;
> @@ -851,10 +854,20 @@ static void atmel_tx_chars(struct uart_port *port)
>  	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
>  		uart_write_wakeup(port);
>  
> -	if (!uart_circ_empty(xmit))
> +	if (!uart_circ_empty(xmit)) {
> +		/* we still have characters to transmit, so we should continue
> +		 * transmitting them when TX is ready, regardless of
> +		 * mode or duplexity
> +		 */
> +		atmel_port->tx_done_mask |= ATMEL_US_TXRDY;
> +
>  		/* Enable interrupts */
>  		atmel_uart_writel(port, ATMEL_US_IER,
>  				  atmel_port->tx_done_mask);
> +	} else {
> +		if (atmel_uart_is_half_duplex(port))
> +			atmel_port->tx_done_mask &= ~ATMEL_US_TXRDY;
> +	}
>  }
>  
>  static void atmel_complete_tx_dma(void *arg)
> @@ -2525,8 +2538,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
>  	 * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
>  	 * ENDTX|TXBUFE
>  	 */
> -	if (port->rs485.flags & SER_RS485_ENABLED ||
> -	    port->iso7816.flags & SER_ISO7816_ENABLED)
> +	if (atmel_uart_is_half_duplex(port))
>  		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
>  	else if (atmel_use_pdc_tx(port)) {
>  		port->fifosize = PDC_BUFFER_SIZE;
>
Codrin Ciubotariu Jan. 8, 2020, 4:03 p.m. UTC | #2
On 08.01.2020 15:45, Richard Genoud wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Le 07/01/2020 à 12:17, Codrin.Ciubotariu@microchip.com a écrit :
>> At this moment, TXEMPTY is checked before sending data on RS485 and ISO7816
>> modes. However, TXEMPTY is risen when FIFO (if used) or the Transmit Shift
>> Register are empty, even though TXRDY might be up and controller is able to
>> receive data. Since the controller sends data only when TXEMPTY is ready,
>> on RS485, when DMA is not used, the RTS pin is driven low after each byte.
>> With this patch, the characters will be transmitted when TXRDY is up and
>> so, RTS pin will remain high between bytes.
>> The performance improvement on RS485 is about 8% with a baudrate of 300.
>>
>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> seems ok to me
> Acked-by: Richard Genoud <richard.genoud@gmail.com>
> 
> NB: MS exchange has added some =3D and =20 here and there, but git am
> doesn't seems to be bothered by them.

We are struggling with some e-mail changes, sorry for that. Thanks for 
letting me know about this.

>> ---
>>   drivers/tty/serial/atmel_serial.c | 26 +++++++++++++++++++-------
>>   1 file changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index a8dc8af83f39..19c8fb9faa36 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -313,7 +313,11 @@ static int atmel_config_rs485(struct uart_port *port,
>>
>>        if (rs485conf->flags & SER_RS485_ENABLED) {
>>                dev_dbg(port->dev, "Setting UART to RS485\n");
>> -             atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
>> +             if (port->rs485.flags & SER_RS485_RX_DURING_TX)
>> +                     atmel_port->tx_done_mask = ATMEL_US_TXRDY;
>> +             else
>> +                     atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
>> +
>>                atmel_uart_writel(port, ATMEL_US_TTGR,
>>                                  rs485conf->delay_rts_after_send);
>>                mode |= ATMEL_US_USMODE_RS485;
>> @@ -831,7 +835,7 @@ static void atmel_tx_chars(struct uart_port *port)
>>        struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>>
>>        if (port->x_char &&
>> -         (atmel_uart_readl(port, ATMEL_US_CSR) & atmel_port->tx_done_mask)) {
>> +         (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY)) {
>>                atmel_uart_write_char(port, port->x_char);
>>                port->icount.tx++;
>>                port->x_char = 0;
>> @@ -839,8 +843,7 @@ static void atmel_tx_chars(struct uart_port *port)
>>        if (uart_circ_empty(xmit) || uart_tx_stopped(port))
>>                return;
>>
>> -     while (atmel_uart_readl(port, ATMEL_US_CSR) &
>> -            atmel_port->tx_done_mask) {
>> +     while (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY) {
>>                atmel_uart_write_char(port, xmit->buf[xmit->tail]);
>>                xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
>>                port->icount.tx++;
>> @@ -851,10 +854,20 @@ static void atmel_tx_chars(struct uart_port *port)
>>        if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
>>                uart_write_wakeup(port);
>>
>> -     if (!uart_circ_empty(xmit))
>> +     if (!uart_circ_empty(xmit)) {
>> +             /* we still have characters to transmit, so we should continue
>> +              * transmitting them when TX is ready, regardless of
>> +              * mode or duplexity
>> +              */
>> +             atmel_port->tx_done_mask |= ATMEL_US_TXRDY;
>> +
>>                /* Enable interrupts */
>>                atmel_uart_writel(port, ATMEL_US_IER,
>>                                  atmel_port->tx_done_mask);
>> +     } else {
>> +             if (atmel_uart_is_half_duplex(port))
>> +                     atmel_port->tx_done_mask &= ~ATMEL_US_TXRDY;
>> +     }
>>   }
>>
>>   static void atmel_complete_tx_dma(void *arg)
>> @@ -2525,8 +2538,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
>>         * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
>>         * ENDTX|TXBUFE
>>         */
>> -     if (port->rs485.flags & SER_RS485_ENABLED ||
>> -         port->iso7816.flags & SER_ISO7816_ENABLED)
>> +     if (atmel_uart_is_half_duplex(port))
>>                atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
>>        else if (atmel_use_pdc_tx(port)) {
>>                port->fifosize = PDC_BUFFER_SIZE;
>>
>
Uwe Kleine-König Jan. 9, 2020, 7:33 a.m. UTC | #3
Hello,

On Wed, Jan 08, 2020 at 02:45:05PM +0100, Richard Genoud wrote:
> NB: MS exchange has added some =3D and =20 here and there, but git am
> doesn't seems to be bothered by them.

Unless I missed something I cannot confirm. In mutt I don't see any =3D
or =20.

Best regards
Uwe
Richard Genoud Jan. 9, 2020, 8:52 a.m. UTC | #4
Le 09/01/2020 à 08:33, Uwe Kleine-König a écrit :
> Hello,
> 
> On Wed, Jan 08, 2020 at 02:45:05PM +0100, Richard Genoud wrote:
>> NB: MS exchange has added some =3D and =20 here and there, but git am
>> doesn't seems to be bothered by them.
> 
> Unless I missed something I cannot confirm. In mutt I don't see any =3D
> or =20.
It's strange indeed. I double check under thunderbird and directly on my
webmail (posteo.de), there're =3D lying around in the message source.
But not on the other patches from linux-serial.
weird...

> 
> Best regards
> Uwe
>
diff mbox series

Patch

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index a8dc8af83f39..19c8fb9faa36 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -313,7 +313,11 @@  static int atmel_config_rs485(struct uart_port *port,
 
 	if (rs485conf->flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
+		if (port->rs485.flags & SER_RS485_RX_DURING_TX)
+			atmel_port->tx_done_mask = ATMEL_US_TXRDY;
+		else
+			atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
+
 		atmel_uart_writel(port, ATMEL_US_TTGR,
 				  rs485conf->delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
@@ -831,7 +835,7 @@  static void atmel_tx_chars(struct uart_port *port)
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
 	if (port->x_char &&
-	    (atmel_uart_readl(port, ATMEL_US_CSR) & atmel_port->tx_done_mask)) {
+	    (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY)) {
 		atmel_uart_write_char(port, port->x_char);
 		port->icount.tx++;
 		port->x_char = 0;
@@ -839,8 +843,7 @@  static void atmel_tx_chars(struct uart_port *port)
 	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
 		return;
 
-	while (atmel_uart_readl(port, ATMEL_US_CSR) &
-	       atmel_port->tx_done_mask) {
+	while (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY) {
 		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
 		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
 		port->icount.tx++;
@@ -851,10 +854,20 @@  static void atmel_tx_chars(struct uart_port *port)
 	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
 		uart_write_wakeup(port);
 
-	if (!uart_circ_empty(xmit))
+	if (!uart_circ_empty(xmit)) {
+		/* we still have characters to transmit, so we should continue
+		 * transmitting them when TX is ready, regardless of
+		 * mode or duplexity
+		 */
+		atmel_port->tx_done_mask |= ATMEL_US_TXRDY;
+
 		/* Enable interrupts */
 		atmel_uart_writel(port, ATMEL_US_IER,
 				  atmel_port->tx_done_mask);
+	} else {
+		if (atmel_uart_is_half_duplex(port))
+			atmel_port->tx_done_mask &= ~ATMEL_US_TXRDY;
+	}
 }
 
 static void atmel_complete_tx_dma(void *arg)
@@ -2525,8 +2538,7 @@  static int atmel_init_port(struct atmel_uart_port *atmel_port,
 	 * Use TXEMPTY for interrupt when rs485 or ISO7816 else TXRDY or
 	 * ENDTX|TXBUFE
 	 */
-	if (port->rs485.flags & SER_RS485_ENABLED ||
-	    port->iso7816.flags & SER_ISO7816_ENABLED)
+	if (atmel_uart_is_half_duplex(port))
 		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
 	else if (atmel_use_pdc_tx(port)) {
 		port->fifosize = PDC_BUFFER_SIZE;