[PATCH] atmel_serial: RS485: receiving enabled when sending data

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Bernhard Roth]

Hello!

By default the atmel_serial driver in RS485 mode disables receiving data 
until all data in the send buffer has been sent. This flag allows to 
receive data even whilst sending data. This is very useful to

- check if the data has been sent correctly over the RS485 bus
- assure that no collision happened
- check for short circuits/termination issues on the RS485 bus

Usually this functionality is realized by hardware, wether controlling 
the RX-Enable pin of the RS485 transceiver with RTS (driver control 
signal) or pulling it LOW permanently. The present atmel_serial driver 
makes this impossible, thus requiring following patch.

Usage example:

     struct serial_rs485     rs485;

     memset(&rs485, 0, sizeof(rs485));
     rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
     ioctl(fd, TIOCSRS485, &rs485);




atmel_serial: RS485: receiving enabled when sending data

By default the atmel_serial driver in RS485 mode disables receiving data 
until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.

Signed-off-by: Bernhard Roth <br@pwrnet.de>
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
  drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
  include/linux/serial.h            |    1 +
  2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c 
b/drivers/tty/serial/atmel_serial.c
index af9b781..5f6c745 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
      /* Disable interrupts */
      UART_PUT_IDR(port, atmel_port->tx_done_mask);

-    if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-        atmel_start_rx(port);
+    if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+        !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+            atmel_start_rx(port);
  }

  /*
@@ -356,8 +357,9 @@ static void atmel_start_tx(struct uart_port *port)
                 really need this.*/
              return;

-        if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-            atmel_stop_rx(port);
+        if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+            !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+                atmel_stop_rx(port);

          /* re-enable PDC transmit */
          UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
@@ -680,9 +682,10 @@ static void atmel_tx_dma(struct uart_port *port)
          /* Enable interrupts */
          UART_PUT_IER(port, atmel_port->tx_done_mask);
      } else {
-        if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
-            /* DMA done, stop TX, start RX for RS485 */
-            atmel_start_rx(port);
+        if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+            !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+                /* DMA done, stop TX, start RX for RS485 */
+                atmel_start_rx(port);
          }
      }

diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
  #define SER_RS485_RTS_ON_SEND        (1 << 1)
  #define SER_RS485_RTS_AFTER_SEND    (1 << 2)
  #define SER_RS485_RTS_BEFORE_SEND    (1 << 3)
+#define SER_RS485_RX_DURING_TX        (1 << 4)
      __u32    delay_rts_before_send;    /* Milliseconds */
      __u32    delay_rts_after_send;    /* Milliseconds */
      __u32    padding[5];        /* Memory is cheap, new structs
-- 
1.7.1

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Bernhard Roth]

Hello!

By default the atmel_serial driver in RS485 mode disables receiving data 
until all data in the send buffer has been sent. This flag allows to 
receive data even whilst sending data. This is very useful to

- check if the data has been sent correctly over the RS485 bus
- assure that no collision happened
- check for short circuits/termination issues on the RS485 bus

Usually this functionality is realized by hardware, wether controlling 
the RX-Enable pin of the RS485 transceiver with RTS (driver control 
signal) or pulling it LOW permanently. The present atmel_serial driver 
makes this impossible, thus requiring following patch.

Usage example:

     struct serial_rs485     rs485;

     memset(&rs485, 0, sizeof(rs485));
     rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
     ioctl(fd, TIOCSRS485, &rs485);




atmel_serial: RS485: receiving enabled when sending data

By default the atmel_serial driver in RS485 mode disables receiving data 
until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.

Signed-off-by: Bernhard Roth <br@pwrnet.de>
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
  drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
  include/linux/serial.h            |    1 +
  2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c 
b/drivers/tty/serial/atmel_serial.c
index af9b781..5f6c745 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
      /* Disable interrupts */
      UART_PUT_IDR(port, atmel_port->tx_done_mask);

-    if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-        atmel_start_rx(port);
+    if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+        !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+            atmel_start_rx(port);
  }

  /*
@@ -356,8 +357,9 @@ static void atmel_start_tx(struct uart_port *port)
                 really need this.*/
              return;

-        if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-            atmel_stop_rx(port);
+        if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+            !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+                atmel_stop_rx(port);

          /* re-enable PDC transmit */
          UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
@@ -680,9 +682,10 @@ static void atmel_tx_dma(struct uart_port *port)
          /* Enable interrupts */
          UART_PUT_IER(port, atmel_port->tx_done_mask);
      } else {
-        if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
-            /* DMA done, stop TX, start RX for RS485 */
-            atmel_start_rx(port);
+        if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+            !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+                /* DMA done, stop TX, start RX for RS485 */
+                atmel_start_rx(port);
          }
      }

diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
  #define SER_RS485_RTS_ON_SEND        (1 << 1)
  #define SER_RS485_RTS_AFTER_SEND    (1 << 2)
  #define SER_RS485_RTS_BEFORE_SEND    (1 << 3)
+#define SER_RS485_RX_DURING_TX        (1 << 4)
      __u32    delay_rts_before_send;    /* Milliseconds */
      __u32    delay_rts_after_send;    /* Milliseconds */
      __u32    padding[5];        /* Memory is cheap, new structs
-- 
1.7.1

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Greg KH]

On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
> Hello!
> 
> By default the atmel_serial driver in RS485 mode disables receiving
> data until all data in the send buffer has been sent. This flag
> allows to receive data even whilst sending data. This is very useful
> to
> 
> - check if the data has been sent correctly over the RS485 bus
> - assure that no collision happened
> - check for short circuits/termination issues on the RS485 bus
> 
> Usually this functionality is realized by hardware, wether
> controlling the RX-Enable pin of the RS485 transceiver with RTS
> (driver control signal) or pulling it LOW permanently. The present
> atmel_serial driver makes this impossible, thus requiring following
> patch.
> 
> Usage example:
> 
>     struct serial_rs485     rs485;
> 
>     memset(&rs485, 0, sizeof(rs485));
>     rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
>     ioctl(fd, TIOCSRS485, &rs485);
> 
> 
> 
> 
> atmel_serial: RS485: receiving enabled when sending data
> 
> By default the atmel_serial driver in RS485 mode disables receiving
> data until
> all data in the send buffer has been sent. This flag allows to receive data
> even whilst sending data.
> 
> Signed-off-by: Bernhard Roth <br@pwrnet.de>
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> ---
>  drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
>  include/linux/serial.h            |    1 +
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index af9b781..5f6c745 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>      /* Disable interrupts */
>      UART_PUT_IDR(port, atmel_port->tx_done_mask);
> 
> -    if (atmel_port->rs485.flags & SER_RS485_ENABLED)
> -        atmel_start_rx(port);
> +    if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
> +        !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
> +            atmel_start_rx(port);

Can you fix your email client to not strip patches of tabs and resend
this so that I can apply it?

thanks,

greg k-h

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Greg KH]

On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
> Hello!
> 
> By default the atmel_serial driver in RS485 mode disables receiving
> data until all data in the send buffer has been sent. This flag
> allows to receive data even whilst sending data. This is very useful
> to
> 
> - check if the data has been sent correctly over the RS485 bus
> - assure that no collision happened
> - check for short circuits/termination issues on the RS485 bus
> 
> Usually this functionality is realized by hardware, wether
> controlling the RX-Enable pin of the RS485 transceiver with RTS
> (driver control signal) or pulling it LOW permanently. The present
> atmel_serial driver makes this impossible, thus requiring following
> patch.
> 
> Usage example:
> 
>     struct serial_rs485     rs485;
> 
>     memset(&rs485, 0, sizeof(rs485));
>     rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
>     ioctl(fd, TIOCSRS485, &rs485);
> 
> 
> 
> 
> atmel_serial: RS485: receiving enabled when sending data
> 
> By default the atmel_serial driver in RS485 mode disables receiving
> data until
> all data in the send buffer has been sent. This flag allows to receive data
> even whilst sending data.
> 
> Signed-off-by: Bernhard Roth <br@pwrnet.de>
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> ---
>  drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
>  include/linux/serial.h            |    1 +
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index af9b781..5f6c745 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>      /* Disable interrupts */
>      UART_PUT_IDR(port, atmel_port->tx_done_mask);
> 
> -    if (atmel_port->rs485.flags & SER_RS485_ENABLED)
> -        atmel_start_rx(port);
> +    if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
> +        !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
> +            atmel_start_rx(port);

Can you fix your email client to not strip patches of tabs and resend
this so that I can apply it?

thanks,

greg k-h

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Claudio Scordino]

Il 22/08/2011 23:18, Greg KH ha scritto:
> On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
>> Hello!
>>
>> By default the atmel_serial driver in RS485 mode disables receiving
>> data until all data in the send buffer has been sent. This flag
>> allows to receive data even whilst sending data. This is very useful
>> to
>>
>> - check if the data has been sent correctly over the RS485 bus
>> - assure that no collision happened
>> - check for short circuits/termination issues on the RS485 bus
>>
>> Usually this functionality is realized by hardware, wether
>> controlling the RX-Enable pin of the RS485 transceiver with RTS
>> (driver control signal) or pulling it LOW permanently. The present
>> atmel_serial driver makes this impossible, thus requiring following
>> patch.
>>
>> Usage example:
>>
>>      struct serial_rs485     rs485;
>>
>>      memset(&rs485, 0, sizeof(rs485));
>>      rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
>>      ioctl(fd, TIOCSRS485,&rs485);
>>
>>
>>
>>
>> atmel_serial: RS485: receiving enabled when sending data
>>
>> By default the atmel_serial driver in RS485 mode disables receiving
>> data until
>> all data in the send buffer has been sent. This flag allows to receive data
>> even whilst sending data.
>>
>> Signed-off-by: Bernhard Roth<br@pwrnet.de>
>> Signed-off-by: Claudio Scordino<claudio@evidence.eu.com>
>> ---
>>   drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
>>   include/linux/serial.h            |    1 +
>>   2 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c
>> b/drivers/tty/serial/atmel_serial.c
>> index af9b781..5f6c745 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>>       /* Disable interrupts */
>>       UART_PUT_IDR(port, atmel_port->tx_done_mask);
>>
>> -    if (atmel_port->rs485.flags&  SER_RS485_ENABLED)
>> -        atmel_start_rx(port);
>> +    if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
>> +        !(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
>> +            atmel_start_rx(port);
> 
> Can you fix your email client to not strip patches of tabs and resend
> this so that I can apply it?
> 
> thanks,
> 
> greg k-h

Hi Greg,

    please find below the patch with the right tabs.

Remind however that the main author of the patch is Bernhard, not me.

Best regards,

    		Claudio 

atmel_serial: RS485: receiving enabled when sending data

By default the atmel_serial driver in RS485 mode disables receiving data until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.

Signed-off-by: Bernhard Roth <br@pwrnet.de> 
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
 drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
 include/linux/serial.h            |    1 +
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index af9b781..5f6c745 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
 	/* Disable interrupts */
 	UART_PUT_IDR(port, atmel_port->tx_done_mask);
 
-	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-		atmel_start_rx(port);
+	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+			atmel_start_rx(port);
 }
 
 /*
@@ -356,8 +357,9 @@ static void atmel_start_tx(struct uart_port *port)
 			   really need this.*/
 			return;
 
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-			atmel_stop_rx(port);
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+			!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+				atmel_stop_rx(port);
 
 		/* re-enable PDC transmit */
 		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
@@ -680,9 +682,10 @@ static void atmel_tx_dma(struct uart_port *port)
 		/* Enable interrupts */
 		UART_PUT_IER(port, atmel_port->tx_done_mask);
 	} else {
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
-			/* DMA done, stop TX, start RX for RS485 */
-			atmel_start_rx(port);
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+			!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+				/* DMA done, stop TX, start RX for RS485 */
+				atmel_start_rx(port);
 		}
 	}
 
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
 #define SER_RS485_RTS_ON_SEND		(1 << 1)
 #define SER_RS485_RTS_AFTER_SEND	(1 << 2)
 #define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_RX_DURING_TX		(1 << 4)
 	__u32	delay_rts_before_send;	/* Milliseconds */
 	__u32	delay_rts_after_send;	/* Milliseconds */
 	__u32	padding[5];		/* Memory is cheap, new structs
-- 
1.7.1

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Claudio Scordino]

Il 22/08/2011 23:18, Greg KH ha scritto:
> On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
>> Hello!
>>
>> By default the atmel_serial driver in RS485 mode disables receiving
>> data until all data in the send buffer has been sent. This flag
>> allows to receive data even whilst sending data. This is very useful
>> to
>>
>> - check if the data has been sent correctly over the RS485 bus
>> - assure that no collision happened
>> - check for short circuits/termination issues on the RS485 bus
>>
>> Usually this functionality is realized by hardware, wether
>> controlling the RX-Enable pin of the RS485 transceiver with RTS
>> (driver control signal) or pulling it LOW permanently. The present
>> atmel_serial driver makes this impossible, thus requiring following
>> patch.
>>
>> Usage example:
>>
>>      struct serial_rs485     rs485;
>>
>>      memset(&rs485, 0, sizeof(rs485));
>>      rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
>>      ioctl(fd, TIOCSRS485,&rs485);
>>
>>
>>
>>
>> atmel_serial: RS485: receiving enabled when sending data
>>
>> By default the atmel_serial driver in RS485 mode disables receiving
>> data until
>> all data in the send buffer has been sent. This flag allows to receive data
>> even whilst sending data.
>>
>> Signed-off-by: Bernhard Roth<br@pwrnet.de>
>> Signed-off-by: Claudio Scordino<claudio@evidence.eu.com>
>> ---
>>   drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
>>   include/linux/serial.h            |    1 +
>>   2 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c
>> b/drivers/tty/serial/atmel_serial.c
>> index af9b781..5f6c745 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>>       /* Disable interrupts */
>>       UART_PUT_IDR(port, atmel_port->tx_done_mask);
>>
>> -    if (atmel_port->rs485.flags&  SER_RS485_ENABLED)
>> -        atmel_start_rx(port);
>> +    if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
>> +        !(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
>> +            atmel_start_rx(port);
> 
> Can you fix your email client to not strip patches of tabs and resend
> this so that I can apply it?
> 
> thanks,
> 
> greg k-h

Hi Greg,

    please find below the patch with the right tabs.

Remind however that the main author of the patch is Bernhard, not me.

Best regards,

    		Claudio 

atmel_serial: RS485: receiving enabled when sending data

By default the atmel_serial driver in RS485 mode disables receiving data until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.

Signed-off-by: Bernhard Roth <br@pwrnet.de> 
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
 drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
 include/linux/serial.h            |    1 +
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index af9b781..5f6c745 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
 	/* Disable interrupts */
 	UART_PUT_IDR(port, atmel_port->tx_done_mask);
 
-	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-		atmel_start_rx(port);
+	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+			atmel_start_rx(port);
 }
 
 /*
@@ -356,8 +357,9 @@ static void atmel_start_tx(struct uart_port *port)
 			   really need this.*/
 			return;
 
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-			atmel_stop_rx(port);
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+			!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+				atmel_stop_rx(port);
 
 		/* re-enable PDC transmit */
 		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
@@ -680,9 +682,10 @@ static void atmel_tx_dma(struct uart_port *port)
 		/* Enable interrupts */
 		UART_PUT_IER(port, atmel_port->tx_done_mask);
 	} else {
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
-			/* DMA done, stop TX, start RX for RS485 */
-			atmel_start_rx(port);
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+			!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+				/* DMA done, stop TX, start RX for RS485 */
+				atmel_start_rx(port);
 		}
 	}
 
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
 #define SER_RS485_RTS_ON_SEND		(1 << 1)
 #define SER_RS485_RTS_AFTER_SEND	(1 << 2)
 #define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_RX_DURING_TX		(1 << 4)
 	__u32	delay_rts_before_send;	/* Milliseconds */
 	__u32	delay_rts_after_send;	/* Milliseconds */
 	__u32	padding[5];		/* Memory is cheap, new structs
-- 
1.7.1

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Claudio Scordino]

Il 22/08/2011 23:18, Greg KH ha scritto:
> On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
>> Hello!
>>
>> By default the atmel_serial driver in RS485 mode disables receiving
>> data until all data in the send buffer has been sent. This flag
>> allows to receive data even whilst sending data. This is very useful
>> to
>>
>> - check if the data has been sent correctly over the RS485 bus
>> - assure that no collision happened
>> - check for short circuits/termination issues on the RS485 bus
>>
>> Usually this functionality is realized by hardware, wether
>> controlling the RX-Enable pin of the RS485 transceiver with RTS
>> (driver control signal) or pulling it LOW permanently. The present
>> atmel_serial driver makes this impossible, thus requiring following
>> patch.
>>
>> Usage example:
>>
>>      struct serial_rs485     rs485;
>>
>>      memset(&rs485, 0, sizeof(rs485));
>>      rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
>>      ioctl(fd, TIOCSRS485,&rs485);
>>
>>
>>
>>
>> atmel_serial: RS485: receiving enabled when sending data
>>
>> By default the atmel_serial driver in RS485 mode disables receiving
>> data until
>> all data in the send buffer has been sent. This flag allows to receive data
>> even whilst sending data.
>>
>> Signed-off-by: Bernhard Roth<br@pwrnet.de>
>> Signed-off-by: Claudio Scordino<claudio@evidence.eu.com>
>> ---
>>   drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
>>   include/linux/serial.h            |    1 +
>>   2 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c
>> b/drivers/tty/serial/atmel_serial.c
>> index af9b781..5f6c745 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>>       /* Disable interrupts */
>>       UART_PUT_IDR(port, atmel_port->tx_done_mask);
>>
>> -    if (atmel_port->rs485.flags&  SER_RS485_ENABLED)
>> -        atmel_start_rx(port);
>> +    if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
>> +        !(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
>> +            atmel_start_rx(port);
> 
> Can you fix your email client to not strip patches of tabs and resend
> this so that I can apply it?
> 
> thanks,
> 
> greg k-h

Hi Greg,

    please find below the patch with the right tabs.

Remind however that the main author of the patch is Bernhard, not me.

Best regards,

    		Claudio 

atmel_serial: RS485: receiving enabled when sending data

By default the atmel_serial driver in RS485 mode disables receiving data until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.

Signed-off-by: Bernhard Roth <br@pwrnet.de> 
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
 drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
 include/linux/serial.h            |    1 +
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index af9b781..5f6c745 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
 	/* Disable interrupts */
 	UART_PUT_IDR(port, atmel_port->tx_done_mask);
 
-	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-		atmel_start_rx(port);
+	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+			atmel_start_rx(port);
 }
 
 /*
@@ -356,8 +357,9 @@ static void atmel_start_tx(struct uart_port *port)
 			   really need this.*/
 			return;
 
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED)
-			atmel_stop_rx(port);
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+			!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
+				atmel_stop_rx(port);
 
 		/* re-enable PDC transmit */
 		UART_PUT_PTCR(port, ATMEL_PDC_TXTEN);
@@ -680,9 +682,10 @@ static void atmel_tx_dma(struct uart_port *port)
 		/* Enable interrupts */
 		UART_PUT_IER(port, atmel_port->tx_done_mask);
 	} else {
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
-			/* DMA done, stop TX, start RX for RS485 */
-			atmel_start_rx(port);
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+			!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+				/* DMA done, stop TX, start RX for RS485 */
+				atmel_start_rx(port);
 		}
 	}
 
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
 #define SER_RS485_RTS_ON_SEND		(1 << 1)
 #define SER_RS485_RTS_AFTER_SEND	(1 << 2)
 #define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_RX_DURING_TX		(1 << 4)
 	__u32	delay_rts_before_send;	/* Milliseconds */
 	__u32	delay_rts_after_send;	/* Milliseconds */
 	__u32	padding[5];		/* Memory is cheap, new structs
-- 
1.7.1

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Russell King - ARM Linux]

On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index af9b781..5f6c745 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>  	/* Disable interrupts */
>  	UART_PUT_IDR(port, atmel_port->tx_done_mask);
>  
> -	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
> -		atmel_start_rx(port);
> +	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
> +		!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
> +			atmel_start_rx(port);

However, this is incorrect formatting.  The code following the 'if' ends
up being indented by two tabs.

This illustrates why the whole idea that tabs should only be used for
indenting is wrong.  You end up with either what you have above, or:

	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
		!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
		atmel_start_rx(port);

both of which are dire for readability.  The only sane way to do this is
to ignore that stupid "indentation by tabs only" thing and do it as the
kernel code has _always_ been done over the last 19 years:

	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
	    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
		atmel_start_rx(port);

IOW, use spaces to align the wrapped 'if' statement.

> +		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
> +			!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
> +				/* DMA done, stop TX, start RX for RS485 */
> +				atmel_start_rx(port);

And here it makes it look like there's a missing brace.

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Russell King - ARM Linux]

On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index af9b781..5f6c745 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>  	/* Disable interrupts */
>  	UART_PUT_IDR(port, atmel_port->tx_done_mask);
>  
> -	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
> -		atmel_start_rx(port);
> +	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
> +		!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
> +			atmel_start_rx(port);

However, this is incorrect formatting.  The code following the 'if' ends
up being indented by two tabs.

This illustrates why the whole idea that tabs should only be used for
indenting is wrong.  You end up with either what you have above, or:

	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
		!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
		atmel_start_rx(port);

both of which are dire for readability.  The only sane way to do this is
to ignore that stupid "indentation by tabs only" thing and do it as the
kernel code has _always_ been done over the last 19 years:

	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
	    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
		atmel_start_rx(port);

IOW, use spaces to align the wrapped 'if' statement.

> +		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
> +			!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
> +				/* DMA done, stop TX, start RX for RS485 */
> +				atmel_start_rx(port);

And here it makes it look like there's a missing brace.

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Russell King - ARM Linux]

On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index af9b781..5f6c745 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>  	/* Disable interrupts */
>  	UART_PUT_IDR(port, atmel_port->tx_done_mask);
>  
> -	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
> -		atmel_start_rx(port);
> +	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
> +		!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
> +			atmel_start_rx(port);

However, this is incorrect formatting.  The code following the 'if' ends
up being indented by two tabs.

This illustrates why the whole idea that tabs should only be used for
indenting is wrong.  You end up with either what you have above, or:

	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
		!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
		atmel_start_rx(port);

both of which are dire for readability.  The only sane way to do this is
to ignore that stupid "indentation by tabs only" thing and do it as the
kernel code has _always_ been done over the last 19 years:

	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
	    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
		atmel_start_rx(port);

IOW, use spaces to align the wrapped 'if' statement.

> +		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
> +			!(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
> +				/* DMA done, stop TX, start RX for RS485 */
> +				atmel_start_rx(port);

And here it makes it look like there's a missing brace.

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Claudio Scordino]

Il 23/08/2011 11:30, Russell King - ARM Linux ha scritto:
> On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index af9b781..5f6c745 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>>   	/* Disable interrupts */
>>   	UART_PUT_IDR(port, atmel_port->tx_done_mask);
>>
>> -	if (atmel_port->rs485.flags&  SER_RS485_ENABLED)
>> -		atmel_start_rx(port);
>> +	if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
>> +		!(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
>> +			atmel_start_rx(port);
> 
> However, this is incorrect formatting.  The code following the 'if' ends
> up being indented by two tabs.
> 
> This illustrates why the whole idea that tabs should only be used for
> indenting is wrong.  You end up with either what you have above, or:
> 
> 	if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
> 		!(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
> 		atmel_start_rx(port);
> 
> both of which are dire for readability.  The only sane way to do this is
> to ignore that stupid "indentation by tabs only" thing and do it as the
> kernel code has _always_ been done over the last 19 years:
> 
> 	if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
> 	!(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
> 		atmel_start_rx(port);
> 
> IOW, use spaces to align the wrapped 'if' statement.
> 
>> +		if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
>> +			!(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX)) {
>> +				/* DMA done, stop TX, start RX for RS485 */
>> +				atmel_start_rx(port);
> 
> And here it makes it look like there's a missing brace.
> 

I understand. So the right patch should be the following one.

Best regards,

	Claudio

atmel_serial: RS485: receiving enabled when sending data

By default the atmel_serial driver in RS485 mode disables receiving data until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.

Signed-off-by: Bernhard Roth <br@pwrnet.de> 
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
 drivers/tty/serial/atmel_serial.c |    9 ++++++---
 include/linux/serial.h            |    1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index af9b781..c7232a9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,7 +339,8 @@ static void atmel_stop_tx(struct uart_port *port)
 	/* Disable interrupts */
 	UART_PUT_IDR(port, atmel_port->tx_done_mask);
 
-	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+	    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
 		atmel_start_rx(port);
 }
 
@@ -356,7 +357,8 @@ static void atmel_start_tx(struct uart_port *port)
 			   really need this.*/
 			return;
 
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
 			atmel_stop_rx(port);
 
 		/* re-enable PDC transmit */
@@ -680,7 +682,8 @@ static void atmel_tx_dma(struct uart_port *port)
 		/* Enable interrupts */
 		UART_PUT_IER(port, atmel_port->tx_done_mask);
 	} else {
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
 			/* DMA done, stop TX, start RX for RS485 */
 			atmel_start_rx(port);
 		}
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
 #define SER_RS485_RTS_ON_SEND		(1 << 1)
 #define SER_RS485_RTS_AFTER_SEND	(1 << 2)
 #define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_RX_DURING_TX		(1 << 4)
 	__u32	delay_rts_before_send;	/* Milliseconds */
 	__u32	delay_rts_after_send;	/* Milliseconds */
 	__u32	padding[5];		/* Memory is cheap, new structs
-- 
1.7.1

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Claudio Scordino]

Il 23/08/2011 11:30, Russell King - ARM Linux ha scritto:
> On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index af9b781..5f6c745 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>>   	/* Disable interrupts */
>>   	UART_PUT_IDR(port, atmel_port->tx_done_mask);
>>
>> -	if (atmel_port->rs485.flags&  SER_RS485_ENABLED)
>> -		atmel_start_rx(port);
>> +	if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
>> +		!(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
>> +			atmel_start_rx(port);
> 
> However, this is incorrect formatting.  The code following the 'if' ends
> up being indented by two tabs.
> 
> This illustrates why the whole idea that tabs should only be used for
> indenting is wrong.  You end up with either what you have above, or:
> 
> 	if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
> 		!(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
> 		atmel_start_rx(port);
> 
> both of which are dire for readability.  The only sane way to do this is
> to ignore that stupid "indentation by tabs only" thing and do it as the
> kernel code has _always_ been done over the last 19 years:
> 
> 	if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
> 	!(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
> 		atmel_start_rx(port);
> 
> IOW, use spaces to align the wrapped 'if' statement.
> 
>> +		if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
>> +			!(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX)) {
>> +				/* DMA done, stop TX, start RX for RS485 */
>> +				atmel_start_rx(port);
> 
> And here it makes it look like there's a missing brace.
> 

I understand. So the right patch should be the following one.

Best regards,

	Claudio

atmel_serial: RS485: receiving enabled when sending data

By default the atmel_serial driver in RS485 mode disables receiving data until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.

Signed-off-by: Bernhard Roth <br@pwrnet.de> 
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
 drivers/tty/serial/atmel_serial.c |    9 ++++++---
 include/linux/serial.h            |    1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index af9b781..c7232a9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,7 +339,8 @@ static void atmel_stop_tx(struct uart_port *port)
 	/* Disable interrupts */
 	UART_PUT_IDR(port, atmel_port->tx_done_mask);
 
-	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+	    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
 		atmel_start_rx(port);
 }
 
@@ -356,7 +357,8 @@ static void atmel_start_tx(struct uart_port *port)
 			   really need this.*/
 			return;
 
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
 			atmel_stop_rx(port);
 
 		/* re-enable PDC transmit */
@@ -680,7 +682,8 @@ static void atmel_tx_dma(struct uart_port *port)
 		/* Enable interrupts */
 		UART_PUT_IER(port, atmel_port->tx_done_mask);
 	} else {
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
 			/* DMA done, stop TX, start RX for RS485 */
 			atmel_start_rx(port);
 		}
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
 #define SER_RS485_RTS_ON_SEND		(1 << 1)
 #define SER_RS485_RTS_AFTER_SEND	(1 << 2)
 #define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_RX_DURING_TX		(1 << 4)
 	__u32	delay_rts_before_send;	/* Milliseconds */
 	__u32	delay_rts_after_send;	/* Milliseconds */
 	__u32	padding[5];		/* Memory is cheap, new structs
-- 
1.7.1

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Alan Cox]

> atmel_serial: RS485: receiving enabled when sending data
> 
> By default the atmel_serial driver in RS485 mode disables receiving data until
> all data in the send buffer has been sent. This flag allows to receive data
> even whilst sending data.
> 
> Signed-off-by: Bernhard Roth <br@pwrnet.de> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>

Acked-by: Alan Cox <alan@linux.intel.com>

(although when we have people having a serious meta discussion about four
 spaces, or a tab or neither rather than functionality I despair)

This is sensible functionality and relevant to various interfaces so
makes sense.

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Alan Cox]

> atmel_serial: RS485: receiving enabled when sending data
> 
> By default the atmel_serial driver in RS485 mode disables receiving data until
> all data in the send buffer has been sent. This flag allows to receive data
> even whilst sending data.
> 
> Signed-off-by: Bernhard Roth <br@pwrnet.de> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>

Acked-by: Alan Cox <alan@linux.intel.com>

(although when we have people having a serious meta discussion about four
 spaces, or a tab or neither rather than functionality I despair)

This is sensible functionality and relevant to various interfaces so
makes sense.

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Russell King - ARM Linux]

On Tue, Aug 23, 2011 at 11:14:38AM +0100, Alan Cox wrote:
> > atmel_serial: RS485: receiving enabled when sending data
> > 
> > By default the atmel_serial driver in RS485 mode disables receiving data until
> > all data in the send buffer has been sent. This flag allows to receive data
> > even whilst sending data.
> > 
> > Signed-off-by: Bernhard Roth <br@pwrnet.de> 
> > Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> 
> Acked-by: Alan Cox <alan@linux.intel.com>
> 
> (although when we have people having a serious meta discussion about four
>  spaces, or a tab or neither rather than functionality I despair)

We could ignore it, but then we end up with code being randomly
formatted throughout files, which makes it _more_ likely that the
code will be misread later.  That in turn makes the chances of
bugs introduced more likely in the future.

It also encourages 'cleanup' patches further down the line, so more
churn - which is something that Linus has been complaining about,
particularly in regard to ARM stuff.

It's far better to get things right the first time round than have
to keep touching stuff time and time again - either because of
cleanup patches or because of subtle bugs introduced by misreading
the code.

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Russell King - ARM Linux]

On Tue, Aug 23, 2011 at 11:14:38AM +0100, Alan Cox wrote:
> > atmel_serial: RS485: receiving enabled when sending data
> > 
> > By default the atmel_serial driver in RS485 mode disables receiving data until
> > all data in the send buffer has been sent. This flag allows to receive data
> > even whilst sending data.
> > 
> > Signed-off-by: Bernhard Roth <br@pwrnet.de> 
> > Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> 
> Acked-by: Alan Cox <alan@linux.intel.com>
> 
> (although when we have people having a serious meta discussion about four
>  spaces, or a tab or neither rather than functionality I despair)

We could ignore it, but then we end up with code being randomly
formatted throughout files, which makes it _more_ likely that the
code will be misread later.  That in turn makes the chances of
bugs introduced more likely in the future.

It also encourages 'cleanup' patches further down the line, so more
churn - which is something that Linus has been complaining about,
particularly in regard to ARM stuff.

It's far better to get things right the first time round than have
to keep touching stuff time and time again - either because of
cleanup patches or because of subtle bugs introduced by misreading
the code.

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Russell King - ARM Linux]

On Tue, Aug 23, 2011 at 11:14:38AM +0100, Alan Cox wrote:
> > atmel_serial: RS485: receiving enabled when sending data
> > 
> > By default the atmel_serial driver in RS485 mode disables receiving data until
> > all data in the send buffer has been sent. This flag allows to receive data
> > even whilst sending data.
> > 
> > Signed-off-by: Bernhard Roth <br@pwrnet.de> 
> > Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> 
> Acked-by: Alan Cox <alan@linux.intel.com>
> 
> (although when we have people having a serious meta discussion about four
>  spaces, or a tab or neither rather than functionality I despair)

We could ignore it, but then we end up with code being randomly
formatted throughout files, which makes it _more_ likely that the
code will be misread later.  That in turn makes the chances of
bugs introduced more likely in the future.

It also encourages 'cleanup' patches further down the line, so more
churn - which is something that Linus has been complaining about,
particularly in regard to ARM stuff.

It's far better to get things right the first time round than have
to keep touching stuff time and time again - either because of
cleanup patches or because of subtle bugs introduced by misreading
the code.

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Greg KH]

On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
> Il 22/08/2011 23:18, Greg KH ha scritto:
> > On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
> >> Hello!
> >>
> >> By default the atmel_serial driver in RS485 mode disables receiving
> >> data until all data in the send buffer has been sent. This flag
> >> allows to receive data even whilst sending data. This is very useful
> >> to
> >>
> >> - check if the data has been sent correctly over the RS485 bus
> >> - assure that no collision happened
> >> - check for short circuits/termination issues on the RS485 bus
> >>
> >> Usually this functionality is realized by hardware, wether
> >> controlling the RX-Enable pin of the RS485 transceiver with RTS
> >> (driver control signal) or pulling it LOW permanently. The present
> >> atmel_serial driver makes this impossible, thus requiring following
> >> patch.
> >>
> >> Usage example:
> >>
> >>      struct serial_rs485     rs485;
> >>
> >>      memset(&rs485, 0, sizeof(rs485));
> >>      rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
> >>      ioctl(fd, TIOCSRS485,&rs485);
> >>
> >>
> >>
> >>
> >> atmel_serial: RS485: receiving enabled when sending data
> >>
> >> By default the atmel_serial driver in RS485 mode disables receiving
> >> data until
> >> all data in the send buffer has been sent. This flag allows to receive data
> >> even whilst sending data.
> >>
> >> Signed-off-by: Bernhard Roth<br@pwrnet.de>
> >> Signed-off-by: Claudio Scordino<claudio@evidence.eu.com>
> >> ---
> >>   drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
> >>   include/linux/serial.h            |    1 +
> >>   2 files changed, 11 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/tty/serial/atmel_serial.c
> >> b/drivers/tty/serial/atmel_serial.c
> >> index af9b781..5f6c745 100644
> >> --- a/drivers/tty/serial/atmel_serial.c
> >> +++ b/drivers/tty/serial/atmel_serial.c
> >> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
> >>       /* Disable interrupts */
> >>       UART_PUT_IDR(port, atmel_port->tx_done_mask);
> >>
> >> -    if (atmel_port->rs485.flags&  SER_RS485_ENABLED)
> >> -        atmel_start_rx(port);
> >> +    if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
> >> +        !(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
> >> +            atmel_start_rx(port);
> > 
> > Can you fix your email client to not strip patches of tabs and resend
> > this so that I can apply it?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
>     please find below the patch with the right tabs.
> 
> Remind however that the main author of the patch is Bernhard, not me.

Then properly send the patch so that this is shown.  Please just add a:

From: foo <foo@foo.org>

as the first line of the patch changelog portion and git will fix things
up correctly.  Documentation/SubmittingPatches describes this in detail.

Care to resend your updated version, with this corrected author
information, so I get it right?

thanks,

greg k-h

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Greg KH]

On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
> Il 22/08/2011 23:18, Greg KH ha scritto:
> > On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
> >> Hello!
> >>
> >> By default the atmel_serial driver in RS485 mode disables receiving
> >> data until all data in the send buffer has been sent. This flag
> >> allows to receive data even whilst sending data. This is very useful
> >> to
> >>
> >> - check if the data has been sent correctly over the RS485 bus
> >> - assure that no collision happened
> >> - check for short circuits/termination issues on the RS485 bus
> >>
> >> Usually this functionality is realized by hardware, wether
> >> controlling the RX-Enable pin of the RS485 transceiver with RTS
> >> (driver control signal) or pulling it LOW permanently. The present
> >> atmel_serial driver makes this impossible, thus requiring following
> >> patch.
> >>
> >> Usage example:
> >>
> >>      struct serial_rs485     rs485;
> >>
> >>      memset(&rs485, 0, sizeof(rs485));
> >>      rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
> >>      ioctl(fd, TIOCSRS485,&rs485);
> >>
> >>
> >>
> >>
> >> atmel_serial: RS485: receiving enabled when sending data
> >>
> >> By default the atmel_serial driver in RS485 mode disables receiving
> >> data until
> >> all data in the send buffer has been sent. This flag allows to receive data
> >> even whilst sending data.
> >>
> >> Signed-off-by: Bernhard Roth<br@pwrnet.de>
> >> Signed-off-by: Claudio Scordino<claudio@evidence.eu.com>
> >> ---
> >>   drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
> >>   include/linux/serial.h            |    1 +
> >>   2 files changed, 11 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/tty/serial/atmel_serial.c
> >> b/drivers/tty/serial/atmel_serial.c
> >> index af9b781..5f6c745 100644
> >> --- a/drivers/tty/serial/atmel_serial.c
> >> +++ b/drivers/tty/serial/atmel_serial.c
> >> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
> >>       /* Disable interrupts */
> >>       UART_PUT_IDR(port, atmel_port->tx_done_mask);
> >>
> >> -    if (atmel_port->rs485.flags&  SER_RS485_ENABLED)
> >> -        atmel_start_rx(port);
> >> +    if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
> >> +        !(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
> >> +            atmel_start_rx(port);
> > 
> > Can you fix your email client to not strip patches of tabs and resend
> > this so that I can apply it?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
>     please find below the patch with the right tabs.
> 
> Remind however that the main author of the patch is Bernhard, not me.

Then properly send the patch so that this is shown.  Please just add a:

From: foo <foo@foo.org>

as the first line of the patch changelog portion and git will fix things
up correctly.  Documentation/SubmittingPatches describes this in detail.

Care to resend your updated version, with this corrected author
information, so I get it right?

thanks,

greg k-h

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Greg KH]

On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
> Il 22/08/2011 23:18, Greg KH ha scritto:
> > On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
> >> Hello!
> >>
> >> By default the atmel_serial driver in RS485 mode disables receiving
> >> data until all data in the send buffer has been sent. This flag
> >> allows to receive data even whilst sending data. This is very useful
> >> to
> >>
> >> - check if the data has been sent correctly over the RS485 bus
> >> - assure that no collision happened
> >> - check for short circuits/termination issues on the RS485 bus
> >>
> >> Usually this functionality is realized by hardware, wether
> >> controlling the RX-Enable pin of the RS485 transceiver with RTS
> >> (driver control signal) or pulling it LOW permanently. The present
> >> atmel_serial driver makes this impossible, thus requiring following
> >> patch.
> >>
> >> Usage example:
> >>
> >>      struct serial_rs485     rs485;
> >>
> >>      memset(&rs485, 0, sizeof(rs485));
> >>      rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
> >>      ioctl(fd, TIOCSRS485,&rs485);
> >>
> >>
> >>
> >>
> >> atmel_serial: RS485: receiving enabled when sending data
> >>
> >> By default the atmel_serial driver in RS485 mode disables receiving
> >> data until
> >> all data in the send buffer has been sent. This flag allows to receive data
> >> even whilst sending data.
> >>
> >> Signed-off-by: Bernhard Roth<br@pwrnet.de>
> >> Signed-off-by: Claudio Scordino<claudio@evidence.eu.com>
> >> ---
> >>   drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
> >>   include/linux/serial.h            |    1 +
> >>   2 files changed, 11 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/tty/serial/atmel_serial.c
> >> b/drivers/tty/serial/atmel_serial.c
> >> index af9b781..5f6c745 100644
> >> --- a/drivers/tty/serial/atmel_serial.c
> >> +++ b/drivers/tty/serial/atmel_serial.c
> >> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
> >>       /* Disable interrupts */
> >>       UART_PUT_IDR(port, atmel_port->tx_done_mask);
> >>
> >> -    if (atmel_port->rs485.flags&  SER_RS485_ENABLED)
> >> -        atmel_start_rx(port);
> >> +    if ((atmel_port->rs485.flags&  SER_RS485_ENABLED)&&
> >> +        !(atmel_port->rs485.flags&  SER_RS485_RX_DURING_TX))
> >> +            atmel_start_rx(port);
> > 
> > Can you fix your email client to not strip patches of tabs and resend
> > this so that I can apply it?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
>     please find below the patch with the right tabs.
> 
> Remind however that the main author of the patch is Bernhard, not me.

Then properly send the patch so that this is shown.  Please just add a:

From: foo <foo@foo.org>

as the first line of the patch changelog portion and git will fix things
up correctly.  Documentation/SubmittingPatches describes this in detail.

Care to resend your updated version, with this corrected author
information, so I get it right?

thanks,

greg k-h

Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data

[Claudio Scordino]

Il 23/08/2011 17:39, Greg KH ha scritto:
> On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
>> Il 22/08/2011 23:18, Greg KH ha scritto:
>>> On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
>>>> Hello!
>>>>
>>>> By default the atmel_serial driver in RS485 mode disables receiving
>>>> data until all data in the send buffer has been sent. This flag
>>>> allows to receive data even whilst sending data. This is very useful
>>>> to
>>>>
>>>> - check if the data has been sent correctly over the RS485 bus
>>>> - assure that no collision happened
>>>> - check for short circuits/termination issues on the RS485 bus
>>>>
>>>> Usually this functionality is realized by hardware, wether
>>>> controlling the RX-Enable pin of the RS485 transceiver with RTS
>>>> (driver control signal) or pulling it LOW permanently. The present
>>>> atmel_serial driver makes this impossible, thus requiring following
>>>> patch.
>>>>
>>>> Usage example:
>>>>
>>>>       struct serial_rs485     rs485;
>>>>
>>>>       memset(&rs485, 0, sizeof(rs485));
>>>>       rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
>>>>       ioctl(fd, TIOCSRS485,&rs485);
>>>>
>>>>
>>>>
>>>>
>>>> atmel_serial: RS485: receiving enabled when sending data
>>>>
>>>> By default the atmel_serial driver in RS485 mode disables receiving
>>>> data until
>>>> all data in the send buffer has been sent. This flag allows to receive data
>>>> even whilst sending data.
>>>>
>>>> Signed-off-by: Bernhard Roth<br@pwrnet.de>
>>>> Signed-off-by: Claudio Scordino<claudio@evidence.eu.com>
>>>> ---
>>>>    drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
>>>>    include/linux/serial.h            |    1 +
>>>>    2 files changed, 11 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/tty/serial/atmel_serial.c
>>>> b/drivers/tty/serial/atmel_serial.c
>>>> index af9b781..5f6c745 100644
>>>> --- a/drivers/tty/serial/atmel_serial.c
>>>> +++ b/drivers/tty/serial/atmel_serial.c
>>>> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>>>>        /* Disable interrupts */
>>>>        UART_PUT_IDR(port, atmel_port->tx_done_mask);
>>>>
>>>> -    if (atmel_port->rs485.flags&   SER_RS485_ENABLED)
>>>> -        atmel_start_rx(port);
>>>> +    if ((atmel_port->rs485.flags&   SER_RS485_ENABLED)&&
>>>> +        !(atmel_port->rs485.flags&   SER_RS485_RX_DURING_TX))
>>>> +            atmel_start_rx(port);
>>>
>>> Can you fix your email client to not strip patches of tabs and resend
>>> this so that I can apply it?
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> Hi Greg,
>>
>>      please find below the patch with the right tabs.
>>
>> Remind however that the main author of the patch is Bernhard, not me.
> 
> Then properly send the patch so that this is shown.  Please just add a:
> 
> From: foo<foo@foo.org>
> 
> as the first line of the patch changelog portion and git will fix things
> up correctly.  Documentation/SubmittingPatches describes this in detail.
> 
> Care to resend your updated version, with this corrected author
> information, so I get it right?

Here it is. I also added a few lines in the Documentation.

Many thanks,

	Claudio

Subject: atmel_serial: RS485: receiving enabled when sending data
From: Bernhard Roth <br@pwrnet.de> 

By default the atmel_serial driver in RS485 mode disables receiving data until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.

Signed-off-by: Bernhard Roth <br@pwrnet.de> 
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
Acked-by: Alan Cox <alan@linux.intel.com>
---
 Documentation/serial/serial-rs485.txt |    3 +++
 drivers/tty/serial/atmel_serial.c     |    9 ++++++---
 include/linux/serial.h                |    1 +
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
index a493238..c8878f8 100644
--- a/Documentation/serial/serial-rs485.txt
+++ b/Documentation/serial/serial-rs485.txt
@@ -104,6 +104,9 @@
 	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
 	rs485conf.delay_rts_after_send = ...;
 
+	/* Set this flag if you want to receive data even whilst sending data */
+	rs485conf.flags |= SER_RS485_RX_DURING_TX;
+
 	if (ioctl (fd, TIOCSRS485, &rs485conf) < 0) {
 		/* Error handling. See errno. */
 	}
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index af9b781..c7232a9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,7 +339,8 @@ static void atmel_stop_tx(struct uart_port *port)
 	/* Disable interrupts */
 	UART_PUT_IDR(port, atmel_port->tx_done_mask);
 
-	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+	    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
 		atmel_start_rx(port);
 }
 
@@ -356,7 +357,8 @@ static void atmel_start_tx(struct uart_port *port)
 			   really need this.*/
 			return;
 
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
 			atmel_stop_rx(port);
 
 		/* re-enable PDC transmit */
@@ -680,7 +682,8 @@ static void atmel_tx_dma(struct uart_port *port)
 		/* Enable interrupts */
 		UART_PUT_IER(port, atmel_port->tx_done_mask);
 	} else {
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
 			/* DMA done, stop TX, start RX for RS485 */
 			atmel_start_rx(port);
 		}
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
 #define SER_RS485_RTS_ON_SEND		(1 << 1)
 #define SER_RS485_RTS_AFTER_SEND	(1 << 2)
 #define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_RX_DURING_TX		(1 << 4)
 	__u32	delay_rts_before_send;	/* Milliseconds */
 	__u32	delay_rts_after_send;	/* Milliseconds */
 	__u32	padding[5];		/* Memory is cheap, new structs
-- 
1.7.1

[PATCH] atmel_serial: RS485: receiving enabled when sending data

[Claudio Scordino]

Il 23/08/2011 17:39, Greg KH ha scritto:
> On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
>> Il 22/08/2011 23:18, Greg KH ha scritto:
>>> On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
>>>> Hello!
>>>>
>>>> By default the atmel_serial driver in RS485 mode disables receiving
>>>> data until all data in the send buffer has been sent. This flag
>>>> allows to receive data even whilst sending data. This is very useful
>>>> to
>>>>
>>>> - check if the data has been sent correctly over the RS485 bus
>>>> - assure that no collision happened
>>>> - check for short circuits/termination issues on the RS485 bus
>>>>
>>>> Usually this functionality is realized by hardware, wether
>>>> controlling the RX-Enable pin of the RS485 transceiver with RTS
>>>> (driver control signal) or pulling it LOW permanently. The present
>>>> atmel_serial driver makes this impossible, thus requiring following
>>>> patch.
>>>>
>>>> Usage example:
>>>>
>>>>       struct serial_rs485     rs485;
>>>>
>>>>       memset(&rs485, 0, sizeof(rs485));
>>>>       rs485.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
>>>>       ioctl(fd, TIOCSRS485,&rs485);
>>>>
>>>>
>>>>
>>>>
>>>> atmel_serial: RS485: receiving enabled when sending data
>>>>
>>>> By default the atmel_serial driver in RS485 mode disables receiving
>>>> data until
>>>> all data in the send buffer has been sent. This flag allows to receive data
>>>> even whilst sending data.
>>>>
>>>> Signed-off-by: Bernhard Roth<br@pwrnet.de>
>>>> Signed-off-by: Claudio Scordino<claudio@evidence.eu.com>
>>>> ---
>>>>    drivers/tty/serial/atmel_serial.c |   17 ++++++++++-------
>>>>    include/linux/serial.h            |    1 +
>>>>    2 files changed, 11 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/tty/serial/atmel_serial.c
>>>> b/drivers/tty/serial/atmel_serial.c
>>>> index af9b781..5f6c745 100644
>>>> --- a/drivers/tty/serial/atmel_serial.c
>>>> +++ b/drivers/tty/serial/atmel_serial.c
>>>> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>>>>        /* Disable interrupts */
>>>>        UART_PUT_IDR(port, atmel_port->tx_done_mask);
>>>>
>>>> -    if (atmel_port->rs485.flags&   SER_RS485_ENABLED)
>>>> -        atmel_start_rx(port);
>>>> +    if ((atmel_port->rs485.flags&   SER_RS485_ENABLED)&&
>>>> +        !(atmel_port->rs485.flags&   SER_RS485_RX_DURING_TX))
>>>> +            atmel_start_rx(port);
>>>
>>> Can you fix your email client to not strip patches of tabs and resend
>>> this so that I can apply it?
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> Hi Greg,
>>
>>      please find below the patch with the right tabs.
>>
>> Remind however that the main author of the patch is Bernhard, not me.
> 
> Then properly send the patch so that this is shown.  Please just add a:
> 
> From: foo<foo@foo.org>
> 
> as the first line of the patch changelog portion and git will fix things
> up correctly.  Documentation/SubmittingPatches describes this in detail.
> 
> Care to resend your updated version, with this corrected author
> information, so I get it right?

Here it is. I also added a few lines in the Documentation.

Many thanks,

	Claudio

Subject: atmel_serial: RS485: receiving enabled when sending data
From: Bernhard Roth <br@pwrnet.de> 

By default the atmel_serial driver in RS485 mode disables receiving data until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.

Signed-off-by: Bernhard Roth <br@pwrnet.de> 
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
Acked-by: Alan Cox <alan@linux.intel.com>
---
 Documentation/serial/serial-rs485.txt |    3 +++
 drivers/tty/serial/atmel_serial.c     |    9 ++++++---
 include/linux/serial.h                |    1 +
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
index a493238..c8878f8 100644
--- a/Documentation/serial/serial-rs485.txt
+++ b/Documentation/serial/serial-rs485.txt
@@ -104,6 +104,9 @@
 	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
 	rs485conf.delay_rts_after_send = ...;
 
+	/* Set this flag if you want to receive data even whilst sending data */
+	rs485conf.flags |= SER_RS485_RX_DURING_TX;
+
 	if (ioctl (fd, TIOCSRS485, &rs485conf) < 0) {
 		/* Error handling. See errno. */
 	}
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index af9b781..c7232a9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,7 +339,8 @@ static void atmel_stop_tx(struct uart_port *port)
 	/* Disable interrupts */
 	UART_PUT_IDR(port, atmel_port->tx_done_mask);
 
-	if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+	if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+	    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
 		atmel_start_rx(port);
 }
 
@@ -356,7 +357,8 @@ static void atmel_start_tx(struct uart_port *port)
 			   really need this.*/
 			return;
 
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
 			atmel_stop_rx(port);
 
 		/* re-enable PDC transmit */
@@ -680,7 +682,8 @@ static void atmel_tx_dma(struct uart_port *port)
 		/* Enable interrupts */
 		UART_PUT_IER(port, atmel_port->tx_done_mask);
 	} else {
-		if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
+		if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+		    !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
 			/* DMA done, stop TX, start RX for RS485 */
 			atmel_start_rx(port);
 		}
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
 #define SER_RS485_RTS_ON_SEND		(1 << 1)
 #define SER_RS485_RTS_AFTER_SEND	(1 << 2)
 #define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_RX_DURING_TX		(1 << 4)
 	__u32	delay_rts_before_send;	/* Milliseconds */
 	__u32	delay_rts_after_send;	/* Milliseconds */
 	__u32	padding[5];		/* Memory is cheap, new structs
-- 
1.7.1

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Hi Alan, Hi Greg,

	it seems that the crisv10.c and the atmel_serial.c serial
drivers interpret the fields of the serial_rs485 structure in a different
way.

In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
atmel_serial.c, instead, uses these values to know if a _delay_ must be
set before and after sending.

This patch makes the usage of these variables consistent across all
drivers and fixes the Documentation as well.
In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
be used to set the logic value of the RTS pin (as in the crisv10.c
driver); the delay is understood by looking only at the value of
delay_rts_before_send and delay_rts_after_send.

Best regards,

	Claudio


Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <claudio@evidence.eu.com>

The crisv10.c and the atmel_serial.c serial drivers interpret the fields
of the serial_rs485 structure in a different way.
In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
uses these values to know if a delay must be set before and after sending.
This patch makes the usage of these variables consistent across all drivers and
fixes the Documentation as well.
>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
understood by looking only at the value of delay_rts_before_send and 
delay_rts_after_send.

Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
Signed-off-by: Darron Black <darron@griffin.net>
---
 Documentation/serial/serial-rs485.txt |   14 +++++++++++---
 drivers/tty/serial/atmel_serial.c     |   20 +++++---------------
 drivers/tty/serial/crisv10.c          |   10 ++--------
 include/linux/serial.h                |   14 ++++++++------
 4 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
index 079cb3d..d3a7388 100644
--- a/Documentation/serial/serial-rs485.txt
+++ b/Documentation/serial/serial-rs485.txt
@@ -97,15 +97,23 @@
 
 	struct serial_rs485 rs485conf;
 
-	/* Set RS485 mode: */
+	/* Enable RS485 mode: */
 	rs485conf.flags |= SER_RS485_ENABLED;
 
+	/* Set voltage value for RTS pin equal to 1 when sending: */
+	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
+	/* or, set voltage value for RTS pin equal to 0 when sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
+
+	/* Set voltage value for RTS pin equal to 1 after sending: */
+	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
+	/* or, set voltage value for RTS pin equal to 0 after sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
+
 	/* Set rts delay before send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
 	rs485conf.delay_rts_before_send = ...;
 
 	/* Set rts delay after send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
 	rs485conf.delay_rts_after_send = ...;
 
 	/* Set this flag if you want to receive data even whilst sending data */
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 4a0f86f..23aa677 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
 	if (rs485conf->flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
 		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
-		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
+		if ((rs485conf->delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
 	} else {
@@ -304,9 +304,9 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
-					atmel_port->rs485.delay_rts_after_send);
+			    atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
 	} else {
 		dev_dbg(port->dev, "Setting UART to RS232\n");
@@ -1228,9 +1228,9 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
-					atmel_port->rs485.delay_rts_after_send);
+			    atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
 	} else {
 		dev_dbg(port->dev, "Setting UART to RS232\n");
@@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
 		rs485conf->delay_rts_after_send = rs485_delay[1];
 		rs485conf->flags = 0;
 
-		if (rs485conf->delay_rts_before_send == 0 &&
-		    rs485conf->delay_rts_after_send == 0) {
-			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
-		} else {
-			if (rs485conf->delay_rts_before_send)
-				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
-			if (rs485conf->delay_rts_after_send)
-				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
-		}
-
 		if (of_get_property(np, "rs485-rx-during-tx", NULL))
 			rs485conf->flags |= SER_RS485_RX_DURING_TX;
 
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index b743504..1dfba7b 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
 		e100_disable_rx(info);
 		e100_enable_rx_irq(info);
 #endif
-		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
-			(info->rs485.delay_rts_before_send > 0))
-				msleep(info->rs485.delay_rts_before_send);
+		if (info->rs485.delay_rts_before_send > 0)
+			msleep(info->rs485.delay_rts_before_send);
 	}
 #endif /* CONFIG_ETRAX_RS485 */
 
@@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
 
 		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
 		rs485data.flags = 0;
-		if (rs485data.delay_rts_before_send != 0)
-			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
-		else
-			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 
 		if (rs485ctrl.enabled)
 			rs485data.flags |= SER_RS485_ENABLED;
@@ -4531,7 +4526,6 @@ static int __init rs_init(void)
 		/* Set sane defaults */
 		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
 		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
-		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 		info->rs485.delay_rts_before_send = 0;
 		info->rs485.flags &= ~(SER_RS485_ENABLED);
 #endif
diff --git a/include/linux/serial.h b/include/linux/serial.h
index 97ff8e2..5a9fd4a 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -207,13 +207,15 @@ struct serial_icounter_struct {
 
 struct serial_rs485 {
 	__u32	flags;			/* RS485 feature flags */
-#define SER_RS485_ENABLED		(1 << 0)
-#define SER_RS485_RTS_ON_SEND		(1 << 1)
-#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
-#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
+#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Voltage value for
+							   RTS pin when
+							   sending */
+#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Voltage value for
+							   RTS pin after sent*/
 #define SER_RS485_RX_DURING_TX		(1 << 4)
-	__u32	delay_rts_before_send;	/* Milliseconds */
-	__u32	delay_rts_after_send;	/* Milliseconds */
+	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
+	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
 	__u32	padding[5];		/* Memory is cheap, new structs
 					   are a royal PITA .. */
 };
-- 
1.7.1

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Hi Alan, Hi Greg,

	it seems that the crisv10.c and the atmel_serial.c serial
drivers interpret the fields of the serial_rs485 structure in a different
way.

In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
atmel_serial.c, instead, uses these values to know if a _delay_ must be
set before and after sending.

This patch makes the usage of these variables consistent across all
drivers and fixes the Documentation as well.
In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
be used to set the logic value of the RTS pin (as in the crisv10.c
driver); the delay is understood by looking only at the value of
delay_rts_before_send and delay_rts_after_send.

Best regards,

	Claudio


Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <claudio@evidence.eu.com>

The crisv10.c and the atmel_serial.c serial drivers interpret the fields
of the serial_rs485 structure in a different way.
In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
uses these values to know if a delay must be set before and after sending.
This patch makes the usage of these variables consistent across all drivers and
fixes the Documentation as well.

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Jesper Nilsson]

On Fri, Nov 04, 2011 at 09:19:21AM +0100, Claudio Scordino wrote:
> Hi Alan, Hi Greg,
> 
> 	it seems that the crisv10.c and the atmel_serial.c serial
> drivers interpret the fields of the serial_rs485 structure in a different
> way.
> 
> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
> atmel_serial.c, instead, uses these values to know if a _delay_ must be
> set before and after sending.
> 
> This patch makes the usage of these variables consistent across all
> drivers and fixes the Documentation as well.
> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
> be used to set the logic value of the RTS pin (as in the crisv10.c
> driver); the delay is understood by looking only at the value of
> delay_rts_before_send and delay_rts_after_send.

Looks good!

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

> 	Claudio

/Jesper

> 
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
> of the serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
> >From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and 
> delay_rts_after_send.
> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>
> ---
>  Documentation/serial/serial-rs485.txt |   14 +++++++++++---
>  drivers/tty/serial/atmel_serial.c     |   20 +++++---------------
>  drivers/tty/serial/crisv10.c          |   10 ++--------
>  include/linux/serial.h                |   14 ++++++++------
>  4 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
> index 079cb3d..d3a7388 100644
> --- a/Documentation/serial/serial-rs485.txt
> +++ b/Documentation/serial/serial-rs485.txt
> @@ -97,15 +97,23 @@
>  
>  	struct serial_rs485 rs485conf;
>  
> -	/* Set RS485 mode: */
> +	/* Enable RS485 mode: */
>  	rs485conf.flags |= SER_RS485_ENABLED;
>  
> +	/* Set voltage value for RTS pin equal to 1 when sending: */
> +	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 when sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
> +
> +	/* Set voltage value for RTS pin equal to 1 after sending: */
> +	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 after sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
> +
>  	/* Set rts delay before send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
>  	rs485conf.delay_rts_before_send = ...;
>  
>  	/* Set rts delay after send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
>  	rs485conf.delay_rts_after_send = ...;
>  
>  	/* Set this flag if you want to receive data even whilst sending data */
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 4a0f86f..23aa677 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
>  	if (rs485conf->flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
>  		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> -		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((rs485conf->delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
> @@ -304,9 +304,9 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1228,9 +1228,9 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
>  		rs485conf->delay_rts_after_send = rs485_delay[1];
>  		rs485conf->flags = 0;
>  
> -		if (rs485conf->delay_rts_before_send == 0 &&
> -		    rs485conf->delay_rts_after_send == 0) {
> -			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
> -		} else {
> -			if (rs485conf->delay_rts_before_send)
> -				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
> -			if (rs485conf->delay_rts_after_send)
> -				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
> -		}
> -
>  		if (of_get_property(np, "rs485-rx-during-tx", NULL))
>  			rs485conf->flags |= SER_RS485_RX_DURING_TX;
>  
> diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
> index b743504..1dfba7b 100644
> --- a/drivers/tty/serial/crisv10.c
> +++ b/drivers/tty/serial/crisv10.c
> @@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
>  		e100_disable_rx(info);
>  		e100_enable_rx_irq(info);
>  #endif
> -		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
> -			(info->rs485.delay_rts_before_send > 0))
> -				msleep(info->rs485.delay_rts_before_send);
> +		if (info->rs485.delay_rts_before_send > 0)
> +			msleep(info->rs485.delay_rts_before_send);
>  	}
>  #endif /* CONFIG_ETRAX_RS485 */
>  
> @@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
>  
>  		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
>  		rs485data.flags = 0;
> -		if (rs485data.delay_rts_before_send != 0)
> -			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
> -		else
> -			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  
>  		if (rs485ctrl.enabled)
>  			rs485data.flags |= SER_RS485_ENABLED;
> @@ -4531,7 +4526,6 @@ static int __init rs_init(void)
>  		/* Set sane defaults */
>  		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
>  		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
> -		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  		info->rs485.delay_rts_before_send = 0;
>  		info->rs485.flags &= ~(SER_RS485_ENABLED);
>  #endif
> diff --git a/include/linux/serial.h b/include/linux/serial.h
> index 97ff8e2..5a9fd4a 100644
> --- a/include/linux/serial.h
> +++ b/include/linux/serial.h
> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>  
>  struct serial_rs485 {
>  	__u32	flags;			/* RS485 feature flags */
> -#define SER_RS485_ENABLED		(1 << 0)
> -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> -#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
> +#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
> +#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Voltage value for
> +							   RTS pin when
> +							   sending */
> +#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Voltage value for
> +							   RTS pin after sent*/
>  #define SER_RS485_RX_DURING_TX		(1 << 4)
> -	__u32	delay_rts_before_send;	/* Milliseconds */
> -	__u32	delay_rts_after_send;	/* Milliseconds */
> +	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
> +	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
>  	__u32	padding[5];		/* Memory is cheap, new structs
>  					   are a royal PITA .. */
>  };
> -- 
> 1.7.1
/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Jesper Nilsson]

On Fri, Nov 04, 2011 at 09:19:21AM +0100, Claudio Scordino wrote:
> Hi Alan, Hi Greg,
> 
> 	it seems that the crisv10.c and the atmel_serial.c serial
> drivers interpret the fields of the serial_rs485 structure in a different
> way.
> 
> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
> atmel_serial.c, instead, uses these values to know if a _delay_ must be
> set before and after sending.
> 
> This patch makes the usage of these variables consistent across all
> drivers and fixes the Documentation as well.
> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
> be used to set the logic value of the RTS pin (as in the crisv10.c
> driver); the delay is understood by looking only at the value of
> delay_rts_before_send and delay_rts_after_send.

Looks good!

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

> 	Claudio

/Jesper

> 
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
> of the serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
> >From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and 
> delay_rts_after_send.
> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>
> ---
>  Documentation/serial/serial-rs485.txt |   14 +++++++++++---
>  drivers/tty/serial/atmel_serial.c     |   20 +++++---------------
>  drivers/tty/serial/crisv10.c          |   10 ++--------
>  include/linux/serial.h                |   14 ++++++++------
>  4 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
> index 079cb3d..d3a7388 100644
> --- a/Documentation/serial/serial-rs485.txt
> +++ b/Documentation/serial/serial-rs485.txt
> @@ -97,15 +97,23 @@
>  
>  	struct serial_rs485 rs485conf;
>  
> -	/* Set RS485 mode: */
> +	/* Enable RS485 mode: */
>  	rs485conf.flags |= SER_RS485_ENABLED;
>  
> +	/* Set voltage value for RTS pin equal to 1 when sending: */
> +	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 when sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
> +
> +	/* Set voltage value for RTS pin equal to 1 after sending: */
> +	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 after sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
> +
>  	/* Set rts delay before send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
>  	rs485conf.delay_rts_before_send = ...;
>  
>  	/* Set rts delay after send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
>  	rs485conf.delay_rts_after_send = ...;
>  
>  	/* Set this flag if you want to receive data even whilst sending data */
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 4a0f86f..23aa677 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
>  	if (rs485conf->flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
>  		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> -		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((rs485conf->delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
> @@ -304,9 +304,9 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1228,9 +1228,9 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
>  		rs485conf->delay_rts_after_send = rs485_delay[1];
>  		rs485conf->flags = 0;
>  
> -		if (rs485conf->delay_rts_before_send == 0 &&
> -		    rs485conf->delay_rts_after_send == 0) {
> -			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
> -		} else {
> -			if (rs485conf->delay_rts_before_send)
> -				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
> -			if (rs485conf->delay_rts_after_send)
> -				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
> -		}
> -
>  		if (of_get_property(np, "rs485-rx-during-tx", NULL))
>  			rs485conf->flags |= SER_RS485_RX_DURING_TX;
>  
> diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
> index b743504..1dfba7b 100644
> --- a/drivers/tty/serial/crisv10.c
> +++ b/drivers/tty/serial/crisv10.c
> @@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
>  		e100_disable_rx(info);
>  		e100_enable_rx_irq(info);
>  #endif
> -		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
> -			(info->rs485.delay_rts_before_send > 0))
> -				msleep(info->rs485.delay_rts_before_send);
> +		if (info->rs485.delay_rts_before_send > 0)
> +			msleep(info->rs485.delay_rts_before_send);
>  	}
>  #endif /* CONFIG_ETRAX_RS485 */
>  
> @@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
>  
>  		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
>  		rs485data.flags = 0;
> -		if (rs485data.delay_rts_before_send != 0)
> -			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
> -		else
> -			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  
>  		if (rs485ctrl.enabled)
>  			rs485data.flags |= SER_RS485_ENABLED;
> @@ -4531,7 +4526,6 @@ static int __init rs_init(void)
>  		/* Set sane defaults */
>  		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
>  		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
> -		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  		info->rs485.delay_rts_before_send = 0;
>  		info->rs485.flags &= ~(SER_RS485_ENABLED);
>  #endif
> diff --git a/include/linux/serial.h b/include/linux/serial.h
> index 97ff8e2..5a9fd4a 100644
> --- a/include/linux/serial.h
> +++ b/include/linux/serial.h
> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>  
>  struct serial_rs485 {
>  	__u32	flags;			/* RS485 feature flags */
> -#define SER_RS485_ENABLED		(1 << 0)
> -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> -#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
> +#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
> +#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Voltage value for
> +							   RTS pin when
> +							   sending */
> +#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Voltage value for
> +							   RTS pin after sent*/
>  #define SER_RS485_RX_DURING_TX		(1 << 4)
> -	__u32	delay_rts_before_send;	/* Milliseconds */
> -	__u32	delay_rts_after_send;	/* Milliseconds */
> +	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
> +	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
>  	__u32	padding[5];		/* Memory is cheap, new structs
>  					   are a royal PITA .. */
>  };
> -- 
> 1.7.1
/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Jesper Nilsson]

On Fri, Nov 04, 2011 at 09:19:21AM +0100, Claudio Scordino wrote:
> Hi Alan, Hi Greg,
> 
> 	it seems that the crisv10.c and the atmel_serial.c serial
> drivers interpret the fields of the serial_rs485 structure in a different
> way.
> 
> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
> atmel_serial.c, instead, uses these values to know if a _delay_ must be
> set before and after sending.
> 
> This patch makes the usage of these variables consistent across all
> drivers and fixes the Documentation as well.
> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
> be used to set the logic value of the RTS pin (as in the crisv10.c
> driver); the delay is understood by looking only at the value of
> delay_rts_before_send and delay_rts_after_send.

Looks good!

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

> 	Claudio

/Jesper

> 
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
> of the serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
> >From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and 
> delay_rts_after_send.
> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>
> ---
>  Documentation/serial/serial-rs485.txt |   14 +++++++++++---
>  drivers/tty/serial/atmel_serial.c     |   20 +++++---------------
>  drivers/tty/serial/crisv10.c          |   10 ++--------
>  include/linux/serial.h                |   14 ++++++++------
>  4 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
> index 079cb3d..d3a7388 100644
> --- a/Documentation/serial/serial-rs485.txt
> +++ b/Documentation/serial/serial-rs485.txt
> @@ -97,15 +97,23 @@
>  
>  	struct serial_rs485 rs485conf;
>  
> -	/* Set RS485 mode: */
> +	/* Enable RS485 mode: */
>  	rs485conf.flags |= SER_RS485_ENABLED;
>  
> +	/* Set voltage value for RTS pin equal to 1 when sending: */
> +	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 when sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
> +
> +	/* Set voltage value for RTS pin equal to 1 after sending: */
> +	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 after sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
> +
>  	/* Set rts delay before send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
>  	rs485conf.delay_rts_before_send = ...;
>  
>  	/* Set rts delay after send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
>  	rs485conf.delay_rts_after_send = ...;
>  
>  	/* Set this flag if you want to receive data even whilst sending data */
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 4a0f86f..23aa677 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
>  	if (rs485conf->flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
>  		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> -		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((rs485conf->delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
> @@ -304,9 +304,9 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1228,9 +1228,9 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
>  		rs485conf->delay_rts_after_send = rs485_delay[1];
>  		rs485conf->flags = 0;
>  
> -		if (rs485conf->delay_rts_before_send == 0 &&
> -		    rs485conf->delay_rts_after_send == 0) {
> -			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
> -		} else {
> -			if (rs485conf->delay_rts_before_send)
> -				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
> -			if (rs485conf->delay_rts_after_send)
> -				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
> -		}
> -
>  		if (of_get_property(np, "rs485-rx-during-tx", NULL))
>  			rs485conf->flags |= SER_RS485_RX_DURING_TX;
>  
> diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
> index b743504..1dfba7b 100644
> --- a/drivers/tty/serial/crisv10.c
> +++ b/drivers/tty/serial/crisv10.c
> @@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
>  		e100_disable_rx(info);
>  		e100_enable_rx_irq(info);
>  #endif
> -		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
> -			(info->rs485.delay_rts_before_send > 0))
> -				msleep(info->rs485.delay_rts_before_send);
> +		if (info->rs485.delay_rts_before_send > 0)
> +			msleep(info->rs485.delay_rts_before_send);
>  	}
>  #endif /* CONFIG_ETRAX_RS485 */
>  
> @@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
>  
>  		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
>  		rs485data.flags = 0;
> -		if (rs485data.delay_rts_before_send != 0)
> -			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
> -		else
> -			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  
>  		if (rs485ctrl.enabled)
>  			rs485data.flags |= SER_RS485_ENABLED;
> @@ -4531,7 +4526,6 @@ static int __init rs_init(void)
>  		/* Set sane defaults */
>  		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
>  		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
> -		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  		info->rs485.delay_rts_before_send = 0;
>  		info->rs485.flags &= ~(SER_RS485_ENABLED);
>  #endif
> diff --git a/include/linux/serial.h b/include/linux/serial.h
> index 97ff8e2..5a9fd4a 100644
> --- a/include/linux/serial.h
> +++ b/include/linux/serial.h
> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>  
>  struct serial_rs485 {
>  	__u32	flags;			/* RS485 feature flags */
> -#define SER_RS485_ENABLED		(1 << 0)
> -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> -#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
> +#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
> +#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Voltage value for
> +							   RTS pin when
> +							   sending */
> +#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Voltage value for
> +							   RTS pin after sent*/
>  #define SER_RS485_RX_DURING_TX		(1 << 4)
> -	__u32	delay_rts_before_send;	/* Milliseconds */
> -	__u32	delay_rts_after_send;	/* Milliseconds */
> +	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
> +	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
>  	__u32	padding[5];		/* Memory is cheap, new structs
>  					   are a royal PITA .. */
>  };
> -- 
> 1.7.1
/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson at axis.com

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Nicolas Ferre]

On 11/04/2011 09:19 AM, Claudio Scordino :
> Hi Alan, Hi Greg,
> 
> 	it seems that the crisv10.c and the atmel_serial.c serial
> drivers interpret the fields of the serial_rs485 structure in a different
> way.
> 
> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
> atmel_serial.c, instead, uses these values to know if a _delay_ must be
> set before and after sending.

It seems sensible, but, on the other hand, I fear that this is a big
change in the user interface: If people are already relying on this for
their application, this can be difficult to understand the change. Can't
we imagine an smoother migration path?

It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
another signal that RTS BTW)...

See comments online...

> This patch makes the usage of these variables consistent across all
> drivers and fixes the Documentation as well.
> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
> be used to set the logic value of the RTS pin (as in the crisv10.c
> driver); the delay is understood by looking only at the value of
> delay_rts_before_send and delay_rts_after_send.
> 
> Best regards,
> 
> 	Claudio
> 
> 
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
> of the serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
>>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and 
> delay_rts_after_send.

Ok, but don't you think that the flags names are not so much
self-explanatory for this new meaning?

What about:
SER_RS485_RTS_LEVEL_DURING_SEND
SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)

Moreover, can't we just use one property for this? I mean, if RTS
logical value is high during the sending of data, can it mean that RTS
will be low before and after? And the other way around: if the signal is
low during data send, will it be high before and after?

Here again, changing the user interface is not a good idea, so I fear
that it can be a show stopper.


> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>
> ---
>  Documentation/serial/serial-rs485.txt |   14 +++++++++++---
>  drivers/tty/serial/atmel_serial.c     |   20 +++++---------------
>  drivers/tty/serial/crisv10.c          |   10 ++--------
>  include/linux/serial.h                |   14 ++++++++------
>  4 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
> index 079cb3d..d3a7388 100644
> --- a/Documentation/serial/serial-rs485.txt
> +++ b/Documentation/serial/serial-rs485.txt
> @@ -97,15 +97,23 @@
>  
>  	struct serial_rs485 rs485conf;
>  
> -	/* Set RS485 mode: */
> +	/* Enable RS485 mode: */
>  	rs485conf.flags |= SER_RS485_ENABLED;
>  
> +	/* Set voltage value for RTS pin equal to 1 when sending: */
> +	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 when sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
> +
> +	/* Set voltage value for RTS pin equal to 1 after sending: */
> +	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 after sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
> +
>  	/* Set rts delay before send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
>  	rs485conf.delay_rts_before_send = ...;
>  
>  	/* Set rts delay after send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
>  	rs485conf.delay_rts_after_send = ...;
>  
>  	/* Set this flag if you want to receive data even whilst sending data */
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 4a0f86f..23aa677 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
>  	if (rs485conf->flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
>  		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> -		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((rs485conf->delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
> @@ -304,9 +304,9 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);

Is it just reformatting?


>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1228,9 +1228,9 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);

Here also?

>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
>  		rs485conf->delay_rts_after_send = rs485_delay[1];
>  		rs485conf->flags = 0;
>  
> -		if (rs485conf->delay_rts_before_send == 0 &&
> -		    rs485conf->delay_rts_after_send == 0) {
> -			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
> -		} else {
> -			if (rs485conf->delay_rts_before_send)
> -				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
> -			if (rs485conf->delay_rts_after_send)
> -				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
> -		}
> -

I agree to remove this, but definitively, you should add the new meaning
of properties in the device tree bindings and add a new way to retrieve
them here... I think that this should be part of this rework.

>  		if (of_get_property(np, "rs485-rx-during-tx", NULL))
>  			rs485conf->flags |= SER_RS485_RX_DURING_TX;
>  

[..]

> diff --git a/include/linux/serial.h b/include/linux/serial.h
> index 97ff8e2..5a9fd4a 100644
> --- a/include/linux/serial.h
> +++ b/include/linux/serial.h
> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>  
>  struct serial_rs485 {
>  	__u32	flags;			/* RS485 feature flags */
> -#define SER_RS485_ENABLED		(1 << 0)
> -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> -#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
> +#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
> +#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Voltage value for

Maybe name it "logical value" "signal logical LEVEL"....

> +							   RTS pin when
> +							   sending */
> +#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Voltage value for
> +							   RTS pin after sent*/
>  #define SER_RS485_RX_DURING_TX		(1 << 4)
> -	__u32	delay_rts_before_send;	/* Milliseconds */
> -	__u32	delay_rts_after_send;	/* Milliseconds */
> +	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
> +	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
>  	__u32	padding[5];		/* Memory is cheap, new structs
>  					   are a royal PITA .. */
>  };

Best regards,
-- 
Nicolas Ferre

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Nicolas Ferre]

On 11/04/2011 09:19 AM, Claudio Scordino :
> Hi Alan, Hi Greg,
> 
> 	it seems that the crisv10.c and the atmel_serial.c serial
> drivers interpret the fields of the serial_rs485 structure in a different
> way.
> 
> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
> atmel_serial.c, instead, uses these values to know if a _delay_ must be
> set before and after sending.

It seems sensible, but, on the other hand, I fear that this is a big
change in the user interface: If people are already relying on this for
their application, this can be difficult to understand the change. Can't
we imagine an smoother migration path?

It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
another signal that RTS BTW)...

See comments online...

> This patch makes the usage of these variables consistent across all
> drivers and fixes the Documentation as well.
> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
> be used to set the logic value of the RTS pin (as in the crisv10.c
> driver); the delay is understood by looking only at the value of
> delay_rts_before_send and delay_rts_after_send.
> 
> Best regards,
> 
> 	Claudio
> 
> 
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
> of the serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
>>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and 
> delay_rts_after_send.

Ok, but don't you think that the flags names are not so much
self-explanatory for this new meaning?

What about:
SER_RS485_RTS_LEVEL_DURING_SEND
SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)

Moreover, can't we just use one property for this? I mean, if RTS
logical value is high during the sending of data, can it mean that RTS
will be low before and after? And the other way around: if the signal is
low during data send, will it be high before and after?

Here again, changing the user interface is not a good idea, so I fear
that it can be a show stopper.


> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>
> ---
>  Documentation/serial/serial-rs485.txt |   14 +++++++++++---
>  drivers/tty/serial/atmel_serial.c     |   20 +++++---------------
>  drivers/tty/serial/crisv10.c          |   10 ++--------
>  include/linux/serial.h                |   14 ++++++++------
>  4 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
> index 079cb3d..d3a7388 100644
> --- a/Documentation/serial/serial-rs485.txt
> +++ b/Documentation/serial/serial-rs485.txt
> @@ -97,15 +97,23 @@
>  
>  	struct serial_rs485 rs485conf;
>  
> -	/* Set RS485 mode: */
> +	/* Enable RS485 mode: */
>  	rs485conf.flags |= SER_RS485_ENABLED;
>  
> +	/* Set voltage value for RTS pin equal to 1 when sending: */
> +	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 when sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
> +
> +	/* Set voltage value for RTS pin equal to 1 after sending: */
> +	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
> +	/* or, set voltage value for RTS pin equal to 0 after sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
> +
>  	/* Set rts delay before send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
>  	rs485conf.delay_rts_before_send = ...;
>  
>  	/* Set rts delay after send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
>  	rs485conf.delay_rts_after_send = ...;
>  
>  	/* Set this flag if you want to receive data even whilst sending data */
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 4a0f86f..23aa677 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
>  	if (rs485conf->flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
>  		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> -		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((rs485conf->delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
> @@ -304,9 +304,9 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);

Is it just reformatting?


>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1228,9 +1228,9 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
> -					atmel_port->rs485.delay_rts_after_send);
> +			    atmel_port->rs485.delay_rts_after_send);

Here also?

>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
>  		dev_dbg(port->dev, "Setting UART to RS232\n");
> @@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
>  		rs485conf->delay_rts_after_send = rs485_delay[1];
>  		rs485conf->flags = 0;
>  
> -		if (rs485conf->delay_rts_before_send == 0 &&
> -		    rs485conf->delay_rts_after_send == 0) {
> -			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
> -		} else {
> -			if (rs485conf->delay_rts_before_send)
> -				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
> -			if (rs485conf->delay_rts_after_send)
> -				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
> -		}
> -

I agree to remove this, but definitively, you should add the new meaning
of properties in the device tree bindings and add a new way to retrieve
them here... I think that this should be part of this rework.

>  		if (of_get_property(np, "rs485-rx-during-tx", NULL))
>  			rs485conf->flags |= SER_RS485_RX_DURING_TX;
>  

[..]

> diff --git a/include/linux/serial.h b/include/linux/serial.h
> index 97ff8e2..5a9fd4a 100644
> --- a/include/linux/serial.h
> +++ b/include/linux/serial.h
> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>  
>  struct serial_rs485 {
>  	__u32	flags;			/* RS485 feature flags */
> -#define SER_RS485_ENABLED		(1 << 0)
> -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> -#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
> +#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
> +#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Voltage value for

Maybe name it "logical value" "signal logical LEVEL"....

> +							   RTS pin when
> +							   sending */
> +#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Voltage value for
> +							   RTS pin after sent*/
>  #define SER_RS485_RX_DURING_TX		(1 << 4)
> -	__u32	delay_rts_before_send;	/* Milliseconds */
> -	__u32	delay_rts_after_send;	/* Milliseconds */
> +	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
> +	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
>  	__u32	padding[5];		/* Memory is cheap, new structs
>  					   are a royal PITA .. */
>  };

Best regards,
-- 
Nicolas Ferre

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:30 Tue 08 Nov     , Nicolas Ferre wrote:
> On 11/04/2011 09:19 AM, Claudio Scordino :
> > Hi Alan, Hi Greg,
> > 
> > 	it seems that the crisv10.c and the atmel_serial.c serial
> > drivers interpret the fields of the serial_rs485 structure in a different
> > way.
> > 
> > In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> > SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
> > atmel_serial.c, instead, uses these values to know if a _delay_ must be
> > set before and after sending.
> 
> It seems sensible, but, on the other hand, I fear that this is a big
> change in the user interface: If people are already relying on this for
> their application, this can be difficult to understand the change. Can't
> we imagine an smoother migration path?
> 
> It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
> another signal that RTS BTW)...
> 
> See comments online...
> 
> > This patch makes the usage of these variables consistent across all
> > drivers and fixes the Documentation as well.
> > In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
> > be used to set the logic value of the RTS pin (as in the crisv10.c
> > driver); the delay is understood by looking only at the value of
> > delay_rts_before_send and delay_rts_after_send.
> > 
> > Best regards,
> > 
> > 	Claudio
> > 
> > 
> > Subject: RS485: fix inconsistencies in the meaning of some variables
> > From: Claudio Scordino <claudio@evidence.eu.com>
> > 
> > The crisv10.c and the atmel_serial.c serial drivers interpret the fields
> > of the serial_rs485 structure in a different way.
> > In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> > SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> > uses these values to know if a delay must be set before and after sending.
> > This patch makes the usage of these variables consistent across all drivers and
> > fixes the Documentation as well.
> >>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> > set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> > understood by looking only at the value of delay_rts_before_send and 
> > delay_rts_after_send.
> 
> Ok, but don't you think that the flags names are not so much
> self-explanatory for this new meaning?
> 
> What about:
> SER_RS485_RTS_LEVEL_DURING_SEND
> SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
> SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)
> 
> Moreover, can't we just use one property for this? I mean, if RTS
> logical value is high during the sending of data, can it mean that RTS
> will be low before and after? And the other way around: if the signal is
> low during data send, will it be high before and after?
> 
> Here again, changing the user interface is not a good idea, so I fear
> that it can be a show stopper.
agreed with nicolas

Best Regards,
J.

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:30 Tue 08 Nov     , Nicolas Ferre wrote:
> On 11/04/2011 09:19 AM, Claudio Scordino :
> > Hi Alan, Hi Greg,
> > 
> > 	it seems that the crisv10.c and the atmel_serial.c serial
> > drivers interpret the fields of the serial_rs485 structure in a different
> > way.
> > 
> > In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> > SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
> > atmel_serial.c, instead, uses these values to know if a _delay_ must be
> > set before and after sending.
> 
> It seems sensible, but, on the other hand, I fear that this is a big
> change in the user interface: If people are already relying on this for
> their application, this can be difficult to understand the change. Can't
> we imagine an smoother migration path?
> 
> It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
> another signal that RTS BTW)...
> 
> See comments online...
> 
> > This patch makes the usage of these variables consistent across all
> > drivers and fixes the Documentation as well.
> > In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
> > be used to set the logic value of the RTS pin (as in the crisv10.c
> > driver); the delay is understood by looking only at the value of
> > delay_rts_before_send and delay_rts_after_send.
> > 
> > Best regards,
> > 
> > 	Claudio
> > 
> > 
> > Subject: RS485: fix inconsistencies in the meaning of some variables
> > From: Claudio Scordino <claudio@evidence.eu.com>
> > 
> > The crisv10.c and the atmel_serial.c serial drivers interpret the fields
> > of the serial_rs485 structure in a different way.
> > In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> > SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> > uses these values to know if a delay must be set before and after sending.
> > This patch makes the usage of these variables consistent across all drivers and
> > fixes the Documentation as well.
> >>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> > set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> > understood by looking only at the value of delay_rts_before_send and 
> > delay_rts_after_send.
> 
> Ok, but don't you think that the flags names are not so much
> self-explanatory for this new meaning?
> 
> What about:
> SER_RS485_RTS_LEVEL_DURING_SEND
> SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
> SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)
> 
> Moreover, can't we just use one property for this? I mean, if RTS
> logical value is high during the sending of data, can it mean that RTS
> will be low before and after? And the other way around: if the signal is
> low during data send, will it be high before and after?
> 
> Here again, changing the user interface is not a good idea, so I fear
> that it can be a show stopper.
agreed with nicolas

Best Regards,
J.

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:30 Tue 08 Nov     , Nicolas Ferre wrote:
> On 11/04/2011 09:19 AM, Claudio Scordino :
> > Hi Alan, Hi Greg,
> > 
> > 	it seems that the crisv10.c and the atmel_serial.c serial
> > drivers interpret the fields of the serial_rs485 structure in a different
> > way.
> > 
> > In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> > SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
> > atmel_serial.c, instead, uses these values to know if a _delay_ must be
> > set before and after sending.
> 
> It seems sensible, but, on the other hand, I fear that this is a big
> change in the user interface: If people are already relying on this for
> their application, this can be difficult to understand the change. Can't
> we imagine an smoother migration path?
> 
> It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
> another signal that RTS BTW)...
> 
> See comments online...
> 
> > This patch makes the usage of these variables consistent across all
> > drivers and fixes the Documentation as well.
> > In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
> > be used to set the logic value of the RTS pin (as in the crisv10.c
> > driver); the delay is understood by looking only at the value of
> > delay_rts_before_send and delay_rts_after_send.
> > 
> > Best regards,
> > 
> > 	Claudio
> > 
> > 
> > Subject: RS485: fix inconsistencies in the meaning of some variables
> > From: Claudio Scordino <claudio@evidence.eu.com>
> > 
> > The crisv10.c and the atmel_serial.c serial drivers interpret the fields
> > of the serial_rs485 structure in a different way.
> > In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> > SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> > uses these values to know if a delay must be set before and after sending.
> > This patch makes the usage of these variables consistent across all drivers and
> > fixes the Documentation as well.
> >>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> > set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> > understood by looking only at the value of delay_rts_before_send and 
> > delay_rts_after_send.
> 
> Ok, but don't you think that the flags names are not so much
> self-explanatory for this new meaning?
> 
> What about:
> SER_RS485_RTS_LEVEL_DURING_SEND
> SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
> SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)
> 
> Moreover, can't we just use one property for this? I mean, if RTS
> logical value is high during the sending of data, can it mean that RTS
> will be low before and after? And the other way around: if the signal is
> low during data send, will it be high before and after?
> 
> Here again, changing the user interface is not a good idea, so I fear
> that it can be a show stopper.
agreed with nicolas

Best Regards,
J.

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Il 08/11/2011 10:30, Nicolas Ferre ha scritto:
> On 11/04/2011 09:19 AM, Claudio Scordino :
>> Hi Alan, Hi Greg,
>>
>> 	it seems that the crisv10.c and the atmel_serial.c serial
>> drivers interpret the fields of the serial_rs485 structure in a different
>> way.
>>
>> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
>> atmel_serial.c, instead, uses these values to know if a _delay_ must be
>> set before and after sending.
>
> It seems sensible, but, on the other hand, I fear that this is a big
> change in the user interface: If people are already relying on this for
> their application, this can be difficult to understand the change. Can't
> we imagine an smoother migration path?
>
> It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
> another signal that RTS BTW)...
>
> See comments online...
>
>> This patch makes the usage of these variables consistent across all
>> drivers and fixes the Documentation as well.
>> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
>> be used to set the logic value of the RTS pin (as in the crisv10.c
>> driver); the delay is understood by looking only at the value of
>> delay_rts_before_send and delay_rts_after_send.
>>
>> Best regards,
>>
>> 	Claudio
>>
>>
>> Subject: RS485: fix inconsistencies in the meaning of some variables
>> From: Claudio Scordino<claudio@evidence.eu.com>
>>
>> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
>> of the serial_rs485 structure in a different way.
>> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
>> uses these values to know if a delay must be set before and after sending.
>> This patch makes the usage of these variables consistent across all drivers and
>> fixes the Documentation as well.
>> > From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
>> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
>> understood by looking only at the value of delay_rts_before_send and
>> delay_rts_after_send.
>
> Ok, but don't you think that the flags names are not so much
> self-explanatory for this new meaning?
>
> What about:
> SER_RS485_RTS_LEVEL_DURING_SEND
> SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
> SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)
>
> Moreover, can't we just use one property for this? I mean, if RTS
> logical value is high during the sending of data, can it mean that RTS
> will be low before and after? And the other way around: if the signal is
> low during data send, will it be high before and after?
>
> Here again, changing the user interface is not a good idea, so I fear
> that it can be a show stopper.

Hi Nicolas,

	I understand, but honestly I do not agree.

The current state is inconsistent, and leaving the status quo can only 
bring to more issues in the future (because it is not clear if the 
interface should be used either as in the Cris or in the Atmel driver). 
That's why I think it should be fixed ASAP (before further drivers start 
using it).

The modifications that I have proposed are very minimal, and most 
user-space code should continue to work without any difference. Any Cris 
user-space code will continue to work, because we didn't change the 
behavior of the driver. For Atmel user-space code, instead, the behavior 
of the driver changes only if flags are not set and delay variables 
contain a value different than 0 (which, hopefully, is not a very common 
situation). That's the reason why I preferred to not change the names of 
the variables, even if better names would be desirable.

If you want, I can re-format the patch according to you suggestions, 
remove formatted lines and changing the names of the variables. But 
unfortunately, I cannot undertake the device tree bindings at the moment.

Best regards,

	Claudio

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Il 08/11/2011 10:30, Nicolas Ferre ha scritto:
> On 11/04/2011 09:19 AM, Claudio Scordino :
>> Hi Alan, Hi Greg,
>>
>> 	it seems that the crisv10.c and the atmel_serial.c serial
>> drivers interpret the fields of the serial_rs485 structure in a different
>> way.
>>
>> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
>> atmel_serial.c, instead, uses these values to know if a _delay_ must be
>> set before and after sending.
>
> It seems sensible, but, on the other hand, I fear that this is a big
> change in the user interface: If people are already relying on this for
> their application, this can be difficult to understand the change. Can't
> we imagine an smoother migration path?
>
> It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
> another signal that RTS BTW)...
>
> See comments online...
>
>> This patch makes the usage of these variables consistent across all
>> drivers and fixes the Documentation as well.
>> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
>> be used to set the logic value of the RTS pin (as in the crisv10.c
>> driver); the delay is understood by looking only at the value of
>> delay_rts_before_send and delay_rts_after_send.
>>
>> Best regards,
>>
>> 	Claudio
>>
>>
>> Subject: RS485: fix inconsistencies in the meaning of some variables
>> From: Claudio Scordino<claudio@evidence.eu.com>
>>
>> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
>> of the serial_rs485 structure in a different way.
>> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
>> uses these values to know if a delay must be set before and after sending.
>> This patch makes the usage of these variables consistent across all drivers and
>> fixes the Documentation as well.
>> > From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
>> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
>> understood by looking only at the value of delay_rts_before_send and
>> delay_rts_after_send.
>
> Ok, but don't you think that the flags names are not so much
> self-explanatory for this new meaning?
>
> What about:
> SER_RS485_RTS_LEVEL_DURING_SEND
> SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
> SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)
>
> Moreover, can't we just use one property for this? I mean, if RTS
> logical value is high during the sending of data, can it mean that RTS
> will be low before and after? And the other way around: if the signal is
> low during data send, will it be high before and after?
>
> Here again, changing the user interface is not a good idea, so I fear
> that it can be a show stopper.

Hi Nicolas,

	I understand, but honestly I do not agree.

The current state is inconsistent, and leaving the status quo can only 
bring to more issues in the future (because it is not clear if the 
interface should be used either as in the Cris or in the Atmel driver). 
That's why I think it should be fixed ASAP (before further drivers start 
using it).

The modifications that I have proposed are very minimal, and most 
user-space code should continue to work without any difference. Any Cris 
user-space code will continue to work, because we didn't change the 
behavior of the driver. For Atmel user-space code, instead, the behavior 
of the driver changes only if flags are not set and delay variables 
contain a value different than 0 (which, hopefully, is not a very common 
situation). That's the reason why I preferred to not change the names of 
the variables, even if better names would be desirable.

If you want, I can re-format the patch according to you suggestions, 
remove formatted lines and changing the names of the variables. But 
unfortunately, I cannot undertake the device tree bindings at the moment.

Best regards,

	Claudio

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Alan Cox]

> The modifications that I have proposed are very minimal, and most 
> user-space code should continue to work without any difference. Any Cris 
> user-space code will continue to work, because we didn't change the 
> behavior of the driver. For Atmel user-space code, instead, the behavior 
> of the driver changes only if flags are not set and delay variables 
> contain a value different than 0 (which, hopefully, is not a very common 
> situation). That's the reason why I preferred to not change the names of 
> the variables, even if better names would be desirable.

We have inconsistency between implementations. We don't have a change in
implementation. There isn't any way to resolve that except by fixing the
deviating implementation and doing it promptly.

With my tty hat on I'm quite happy with this patch. The sooner it is
upstream the better.

Alan

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Alan Cox]

> The modifications that I have proposed are very minimal, and most 
> user-space code should continue to work without any difference. Any Cris 
> user-space code will continue to work, because we didn't change the 
> behavior of the driver. For Atmel user-space code, instead, the behavior 
> of the driver changes only if flags are not set and delay variables 
> contain a value different than 0 (which, hopefully, is not a very common 
> situation). That's the reason why I preferred to not change the names of 
> the variables, even if better names would be desirable.

We have inconsistency between implementations. We don't have a change in
implementation. There isn't any way to resolve that except by fixing the
deviating implementation and doing it promptly.

With my tty hat on I'm quite happy with this patch. The sooner it is
upstream the better.

Alan

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Greg KH]

On Tue, Nov 08, 2011 at 01:48:04PM +0000, Alan Cox wrote:
> > The modifications that I have proposed are very minimal, and most 
> > user-space code should continue to work without any difference. Any Cris 
> > user-space code will continue to work, because we didn't change the 
> > behavior of the driver. For Atmel user-space code, instead, the behavior 
> > of the driver changes only if flags are not set and delay variables 
> > contain a value different than 0 (which, hopefully, is not a very common 
> > situation). That's the reason why I preferred to not change the names of 
> > the variables, even if better names would be desirable.
> 
> We have inconsistency between implementations. We don't have a change in
> implementation. There isn't any way to resolve that except by fixing the
> deviating implementation and doing it promptly.
> 
> With my tty hat on I'm quite happy with this patch. The sooner it is
> upstream the better.

Ok, I'll push to get it to Linus for the next rc release.

greg k-h

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Greg KH]

On Tue, Nov 08, 2011 at 01:48:04PM +0000, Alan Cox wrote:
> > The modifications that I have proposed are very minimal, and most 
> > user-space code should continue to work without any difference. Any Cris 
> > user-space code will continue to work, because we didn't change the 
> > behavior of the driver. For Atmel user-space code, instead, the behavior 
> > of the driver changes only if flags are not set and delay variables 
> > contain a value different than 0 (which, hopefully, is not a very common 
> > situation). That's the reason why I preferred to not change the names of 
> > the variables, even if better names would be desirable.
> 
> We have inconsistency between implementations. We don't have a change in
> implementation. There isn't any way to resolve that except by fixing the
> deviating implementation and doing it promptly.
> 
> With my tty hat on I'm quite happy with this patch. The sooner it is
> upstream the better.

Ok, I'll push to get it to Linus for the next rc release.

greg k-h

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Nicolas Ferre]

On 11/08/2011 11:48 AM, Claudio Scordino :
> Il 08/11/2011 10:30, Nicolas Ferre ha scritto:
>> On 11/04/2011 09:19 AM, Claudio Scordino :
>>> Hi Alan, Hi Greg,
>>>
>>>     it seems that the crisv10.c and the atmel_serial.c serial
>>> drivers interpret the fields of the serial_rs485 structure in a
>>> different
>>> way.
>>>
>>> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>>> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
>>> atmel_serial.c, instead, uses these values to know if a _delay_ must be
>>> set before and after sending.
>>
>> It seems sensible, but, on the other hand, I fear that this is a big
>> change in the user interface: If people are already relying on this for
>> their application, this can be difficult to understand the change. Can't
>> we imagine an smoother migration path?
>>
>> It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
>> another signal that RTS BTW)...
>>
>> See comments online...
>>
>>> This patch makes the usage of these variables consistent across all
>>> drivers and fixes the Documentation as well.
>>> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
>>> be used to set the logic value of the RTS pin (as in the crisv10.c
>>> driver); the delay is understood by looking only at the value of
>>> delay_rts_before_send and delay_rts_after_send.
>>>
>>> Best regards,
>>>
>>>     Claudio
>>>
>>>
>>> Subject: RS485: fix inconsistencies in the meaning of some variables
>>> From: Claudio Scordino<claudio@evidence.eu.com>
>>>
>>> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
>>> of the serial_rs485 structure in a different way.
>>> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>>> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c,
>>> instead,
>>> uses these values to know if a delay must be set before and after
>>> sending.
>>> This patch makes the usage of these variables consistent across all
>>> drivers and
>>> fixes the Documentation as well.
>>> > From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND
>>> will be used to
>>> set the voltage of the RTS pin (as in the crisv10.c driver); the
>>> delay will be
>>> understood by looking only at the value of delay_rts_before_send and
>>> delay_rts_after_send.
>>
>> Ok, but don't you think that the flags names are not so much
>> self-explanatory for this new meaning?
>>
>> What about:
>> SER_RS485_RTS_LEVEL_DURING_SEND
>> SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
>> SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)
>>
>> Moreover, can't we just use one property for this? I mean, if RTS
>> logical value is high during the sending of data, can it mean that RTS
>> will be low before and after? And the other way around: if the signal is
>> low during data send, will it be high before and after?
>>
>> Here again, changing the user interface is not a good idea, so I fear
>> that it can be a show stopper.
> 
> Hi Nicolas,
> 
>     I understand, but honestly I do not agree.
> 
> The current state is inconsistent, and leaving the status quo can only
> bring to more issues in the future (because it is not clear if the
> interface should be used either as in the Cris or in the Atmel driver).
> That's why I think it should be fixed ASAP (before further drivers start
> using it).
> 
> The modifications that I have proposed are very minimal, and most
> user-space code should continue to work without any difference. Any Cris
> user-space code will continue to work, because we didn't change the
> behavior of the driver. For Atmel user-space code, instead, the behavior
> of the driver changes only if flags are not set and delay variables
> contain a value different than 0 (which, hopefully, is not a very common
> situation).

Ok then. I was fearing that, with Atmel driver, someone can set
SER_RS485_RTS_AFTER_SEND to tell that the delay is needed but not
necessarily that the signal level is "high".

But you are right telling that this inconstancy should be addressed.

> That's the reason why I preferred to not change the names of
> the variables, even if better names would be desirable.

100% agree with this.

> If you want, I can re-format the patch according to you suggestions,

Yes

> remove formatted lines

Yes

> and changing the names of the variables.

Well, this is proved to be not a good option at this stage of RS485
support in kernel. So as you said: no need to mess with this.

> But unfortunately, I cannot undertake the device tree bindings at the
moment.

Ah, ok... well we will manage this after your submitting of this patch.
But can you tell me if the signal wave is always like this:
__|--|__ and --|__|--

or if it can also be like this:
__|--|--  or --|--|__

This way, we will see if the hardware property can be addressed with
only one device tree biding or if tree of them are needed.

Best regards,
-- 
Nicolas Ferre

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Nicolas Ferre]

On 11/08/2011 11:48 AM, Claudio Scordino :
> Il 08/11/2011 10:30, Nicolas Ferre ha scritto:
>> On 11/04/2011 09:19 AM, Claudio Scordino :
>>> Hi Alan, Hi Greg,
>>>
>>>     it seems that the crisv10.c and the atmel_serial.c serial
>>> drivers interpret the fields of the serial_rs485 structure in a
>>> different
>>> way.
>>>
>>> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>>> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
>>> atmel_serial.c, instead, uses these values to know if a _delay_ must be
>>> set before and after sending.
>>
>> It seems sensible, but, on the other hand, I fear that this is a big
>> change in the user interface: If people are already relying on this for
>> their application, this can be difficult to understand the change. Can't
>> we imagine an smoother migration path?
>>
>> It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
>> another signal that RTS BTW)...
>>
>> See comments online...
>>
>>> This patch makes the usage of these variables consistent across all
>>> drivers and fixes the Documentation as well.
>>> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
>>> be used to set the logic value of the RTS pin (as in the crisv10.c
>>> driver); the delay is understood by looking only at the value of
>>> delay_rts_before_send and delay_rts_after_send.
>>>
>>> Best regards,
>>>
>>>     Claudio
>>>
>>>
>>> Subject: RS485: fix inconsistencies in the meaning of some variables
>>> From: Claudio Scordino<claudio@evidence.eu.com>
>>>
>>> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
>>> of the serial_rs485 structure in a different way.
>>> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>>> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c,
>>> instead,
>>> uses these values to know if a delay must be set before and after
>>> sending.
>>> This patch makes the usage of these variables consistent across all
>>> drivers and
>>> fixes the Documentation as well.
>>> > From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND
>>> will be used to
>>> set the voltage of the RTS pin (as in the crisv10.c driver); the
>>> delay will be
>>> understood by looking only at the value of delay_rts_before_send and
>>> delay_rts_after_send.
>>
>> Ok, but don't you think that the flags names are not so much
>> self-explanatory for this new meaning?
>>
>> What about:
>> SER_RS485_RTS_LEVEL_DURING_SEND
>> SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
>> SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)
>>
>> Moreover, can't we just use one property for this? I mean, if RTS
>> logical value is high during the sending of data, can it mean that RTS
>> will be low before and after? And the other way around: if the signal is
>> low during data send, will it be high before and after?
>>
>> Here again, changing the user interface is not a good idea, so I fear
>> that it can be a show stopper.
> 
> Hi Nicolas,
> 
>     I understand, but honestly I do not agree.
> 
> The current state is inconsistent, and leaving the status quo can only
> bring to more issues in the future (because it is not clear if the
> interface should be used either as in the Cris or in the Atmel driver).
> That's why I think it should be fixed ASAP (before further drivers start
> using it).
> 
> The modifications that I have proposed are very minimal, and most
> user-space code should continue to work without any difference. Any Cris
> user-space code will continue to work, because we didn't change the
> behavior of the driver. For Atmel user-space code, instead, the behavior
> of the driver changes only if flags are not set and delay variables
> contain a value different than 0 (which, hopefully, is not a very common
> situation).

Ok then. I was fearing that, with Atmel driver, someone can set
SER_RS485_RTS_AFTER_SEND to tell that the delay is needed but not
necessarily that the signal level is "high".

But you are right telling that this inconstancy should be addressed.

> That's the reason why I preferred to not change the names of
> the variables, even if better names would be desirable.

100% agree with this.

> If you want, I can re-format the patch according to you suggestions,

Yes

> remove formatted lines

Yes

> and changing the names of the variables.

Well, this is proved to be not a good option at this stage of RS485
support in kernel. So as you said: no need to mess with this.

> But unfortunately, I cannot undertake the device tree bindings at the
moment.

Ah, ok... well we will manage this after your submitting of this patch.
But can you tell me if the signal wave is always like this:
__|--|__ and --|__|--

or if it can also be like this:
__|--|--  or --|--|__

This way, we will see if the hardware property can be addressed with
only one device tree biding or if tree of them are needed.

Best regards,
-- 
Nicolas Ferre

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Il 08/11/2011 16:02, Nicolas Ferre ha scritto:
> On 11/08/2011 11:48 AM, Claudio Scordino :
>> Il 08/11/2011 10:30, Nicolas Ferre ha scritto:
>>> On 11/04/2011 09:19 AM, Claudio Scordino :
>>>> Hi Alan, Hi Greg,
>>>>
>>>>      it seems that the crisv10.c and the atmel_serial.c serial
>>>> drivers interpret the fields of the serial_rs485 structure in a
>>>> different
>>>> way.
>>>>
>>>> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>>>> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
>>>> atmel_serial.c, instead, uses these values to know if a _delay_ must be
>>>> set before and after sending.
>>>
>>> It seems sensible, but, on the other hand, I fear that this is a big
>>> change in the user interface: If people are already relying on this for
>>> their application, this can be difficult to understand the change. Can't
>>> we imagine an smoother migration path?
>>>
>>> It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
>>> another signal that RTS BTW)...
>>>
>>> See comments online...
>>>
>>>> This patch makes the usage of these variables consistent across all
>>>> drivers and fixes the Documentation as well.
>>>> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
>>>> be used to set the logic value of the RTS pin (as in the crisv10.c
>>>> driver); the delay is understood by looking only at the value of
>>>> delay_rts_before_send and delay_rts_after_send.
>>>>
>>>> Best regards,
>>>>
>>>>      Claudio
>>>>
>>>>
>>>> Subject: RS485: fix inconsistencies in the meaning of some variables
>>>> From: Claudio Scordino<claudio@evidence.eu.com>
>>>>
>>>> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
>>>> of the serial_rs485 structure in a different way.
>>>> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>>>> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c,
>>>> instead,
>>>> uses these values to know if a delay must be set before and after
>>>> sending.
>>>> This patch makes the usage of these variables consistent across all
>>>> drivers and
>>>> fixes the Documentation as well.
>>>>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND
>>>> will be used to
>>>> set the voltage of the RTS pin (as in the crisv10.c driver); the
>>>> delay will be
>>>> understood by looking only at the value of delay_rts_before_send and
>>>> delay_rts_after_send.
>>>
>>> Ok, but don't you think that the flags names are not so much
>>> self-explanatory for this new meaning?
>>>
>>> What about:
>>> SER_RS485_RTS_LEVEL_DURING_SEND
>>> SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
>>> SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)
>>>
>>> Moreover, can't we just use one property for this? I mean, if RTS
>>> logical value is high during the sending of data, can it mean that RTS
>>> will be low before and after? And the other way around: if the signal is
>>> low during data send, will it be high before and after?
>>>
>>> Here again, changing the user interface is not a good idea, so I fear
>>> that it can be a show stopper.
>>
>> Hi Nicolas,
>>
>>      I understand, but honestly I do not agree.
>>
>> The current state is inconsistent, and leaving the status quo can only
>> bring to more issues in the future (because it is not clear if the
>> interface should be used either as in the Cris or in the Atmel driver).
>> That's why I think it should be fixed ASAP (before further drivers start
>> using it).
>>
>> The modifications that I have proposed are very minimal, and most
>> user-space code should continue to work without any difference. Any Cris
>> user-space code will continue to work, because we didn't change the
>> behavior of the driver. For Atmel user-space code, instead, the behavior
>> of the driver changes only if flags are not set and delay variables
>> contain a value different than 0 (which, hopefully, is not a very common
>> situation).
> 
> Ok then. I was fearing that, with Atmel driver, someone can set
> SER_RS485_RTS_AFTER_SEND to tell that the delay is needed but not
> necessarily that the signal level is "high".
> 
> But you are right telling that this inconstancy should be addressed.
> 
>> That's the reason why I preferred to not change the names of
>> the variables, even if better names would be desirable.
> 
> 100% agree with this.
> 
>> If you want, I can re-format the patch according to you suggestions,
> 
> Yes
> 
>> remove formatted lines
> 
> Yes

Hi all,

	please find below a better formatted patch.

Best regards,

	Claudio



Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <claudio@evidence.eu.com>

The crisv10.c and the atmel_serial.c serial drivers intepret the fields of the
serial_rs485 structure in a different way.
In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
uses these values to know if a delay must be set before and after sending.
This patch makes the usage of these variables consistent across all drivers and
fixes the Documentation as well.
>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
understood by looking only at the value of delay_rts_before_send and 
delay_rts_after_send.

Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
Signed-off-by: Darron Black <darron@griffin.net>
---
 Documentation/serial/serial-rs485.txt |   14 +++++++++++---
 drivers/tty/serial/atmel_serial.c     |   16 +++-------------
 drivers/tty/serial/crisv10.c          |   10 ++--------
 include/linux/serial.h                |   14 ++++++++------
 4 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
index 079cb3d..41c8378 100644
--- a/Documentation/serial/serial-rs485.txt
+++ b/Documentation/serial/serial-rs485.txt
@@ -97,15 +97,23 @@
 
 	struct serial_rs485 rs485conf;
 
-	/* Set RS485 mode: */
+	/* Enable RS485 mode: */
 	rs485conf.flags |= SER_RS485_ENABLED;
 
+	/* Set logical level for RTS pin equal to 1 when sending: */
+	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
+	/* or, set logical level for RTS pin equal to 0 when sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
+
+	/* Set logical level for RTS pin equal to 1 after sending: */
+	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
+	/* or, set logical level for RTS pin equal to 0 after sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
+
 	/* Set rts delay before send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
 	rs485conf.delay_rts_before_send = ...;
 
 	/* Set rts delay after send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
 	rs485conf.delay_rts_after_send = ...;
 
 	/* Set this flag if you want to receive data even whilst sending data */
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 4a0f86f..4c823f3 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
 	if (rs485conf->flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
 		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
-		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
+		if ((rs485conf->delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
 	} else {
@@ -304,7 +304,7 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
 					atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
@@ -1228,7 +1228,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
 					atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
@@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
 		rs485conf->delay_rts_after_send = rs485_delay[1];
 		rs485conf->flags = 0;
 
-		if (rs485conf->delay_rts_before_send == 0 &&
-		    rs485conf->delay_rts_after_send == 0) {
-			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
-		} else {
-			if (rs485conf->delay_rts_before_send)
-				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
-			if (rs485conf->delay_rts_after_send)
-				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
-		}
-
 		if (of_get_property(np, "rs485-rx-during-tx", NULL))
 			rs485conf->flags |= SER_RS485_RX_DURING_TX;
 
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index b743504..1dfba7b 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
 		e100_disable_rx(info);
 		e100_enable_rx_irq(info);
 #endif
-		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
-			(info->rs485.delay_rts_before_send > 0))
-				msleep(info->rs485.delay_rts_before_send);
+		if (info->rs485.delay_rts_before_send > 0)
+			msleep(info->rs485.delay_rts_before_send);
 	}
 #endif /* CONFIG_ETRAX_RS485 */
 
@@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
 
 		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
 		rs485data.flags = 0;
-		if (rs485data.delay_rts_before_send != 0)
-			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
-		else
-			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 
 		if (rs485ctrl.enabled)
 			rs485data.flags |= SER_RS485_ENABLED;
@@ -4531,7 +4526,6 @@ static int __init rs_init(void)
 		/* Set sane defaults */
 		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
 		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
-		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 		info->rs485.delay_rts_before_send = 0;
 		info->rs485.flags &= ~(SER_RS485_ENABLED);
 #endif
diff --git a/include/linux/serial.h b/include/linux/serial.h
index 97ff8e2..3d86517 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -207,13 +207,15 @@ struct serial_icounter_struct {
 
 struct serial_rs485 {
 	__u32	flags;			/* RS485 feature flags */
-#define SER_RS485_ENABLED		(1 << 0)
-#define SER_RS485_RTS_ON_SEND		(1 << 1)
-#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
-#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
+#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Logical level for
+							   RTS pin when
+							   sending */
+#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
+							   RTS pin after sent*/
 #define SER_RS485_RX_DURING_TX		(1 << 4)
-	__u32	delay_rts_before_send;	/* Milliseconds */
-	__u32	delay_rts_after_send;	/* Milliseconds */
+	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
+	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
 	__u32	padding[5];		/* Memory is cheap, new structs
 					   are a royal PITA .. */
 };
-- 
1.7.1

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Il 08/11/2011 16:02, Nicolas Ferre ha scritto:
> On 11/08/2011 11:48 AM, Claudio Scordino :
>> Il 08/11/2011 10:30, Nicolas Ferre ha scritto:
>>> On 11/04/2011 09:19 AM, Claudio Scordino :
>>>> Hi Alan, Hi Greg,
>>>>
>>>>      it seems that the crisv10.c and the atmel_serial.c serial
>>>> drivers interpret the fields of the serial_rs485 structure in a
>>>> different
>>>> way.
>>>>
>>>> In particular, it seems that crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>>>> SER_RS485_RTS_ON_SEND for the _logic value_ of the RTS pin;
>>>> atmel_serial.c, instead, uses these values to know if a _delay_ must be
>>>> set before and after sending.
>>>
>>> It seems sensible, but, on the other hand, I fear that this is a big
>>> change in the user interface: If people are already relying on this for
>>> their application, this can be difficult to understand the change. Can't
>>> we imagine an smoother migration path?
>>>
>>> It seems from de6f86ce5 that 16C950 may also use rs485 mode (with
>>> another signal that RTS BTW)...
>>>
>>> See comments online...
>>>
>>>> This patch makes the usage of these variables consistent across all
>>>> drivers and fixes the Documentation as well.
>>>> In particular, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
>>>> be used to set the logic value of the RTS pin (as in the crisv10.c
>>>> driver); the delay is understood by looking only at the value of
>>>> delay_rts_before_send and delay_rts_after_send.
>>>>
>>>> Best regards,
>>>>
>>>>      Claudio
>>>>
>>>>
>>>> Subject: RS485: fix inconsistencies in the meaning of some variables
>>>> From: Claudio Scordino<claudio@evidence.eu.com>
>>>>
>>>> The crisv10.c and the atmel_serial.c serial drivers interpret the fields
>>>> of the serial_rs485 structure in a different way.
>>>> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and
>>>> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c,
>>>> instead,
>>>> uses these values to know if a delay must be set before and after
>>>> sending.
>>>> This patch makes the usage of these variables consistent across all
>>>> drivers and
>>>> fixes the Documentation as well.
>>>>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND
>>>> will be used to
>>>> set the voltage of the RTS pin (as in the crisv10.c driver); the
>>>> delay will be
>>>> understood by looking only at the value of delay_rts_before_send and
>>>> delay_rts_after_send.
>>>
>>> Ok, but don't you think that the flags names are not so much
>>> self-explanatory for this new meaning?
>>>
>>> What about:
>>> SER_RS485_RTS_LEVEL_DURING_SEND
>>> SER_RS485_RTS_VALUE_DURING_SEND (maybe too vague?)
>>> SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND (maybe too long?)
>>>
>>> Moreover, can't we just use one property for this? I mean, if RTS
>>> logical value is high during the sending of data, can it mean that RTS
>>> will be low before and after? And the other way around: if the signal is
>>> low during data send, will it be high before and after?
>>>
>>> Here again, changing the user interface is not a good idea, so I fear
>>> that it can be a show stopper.
>>
>> Hi Nicolas,
>>
>>      I understand, but honestly I do not agree.
>>
>> The current state is inconsistent, and leaving the status quo can only
>> bring to more issues in the future (because it is not clear if the
>> interface should be used either as in the Cris or in the Atmel driver).
>> That's why I think it should be fixed ASAP (before further drivers start
>> using it).
>>
>> The modifications that I have proposed are very minimal, and most
>> user-space code should continue to work without any difference. Any Cris
>> user-space code will continue to work, because we didn't change the
>> behavior of the driver. For Atmel user-space code, instead, the behavior
>> of the driver changes only if flags are not set and delay variables
>> contain a value different than 0 (which, hopefully, is not a very common
>> situation).
> 
> Ok then. I was fearing that, with Atmel driver, someone can set
> SER_RS485_RTS_AFTER_SEND to tell that the delay is needed but not
> necessarily that the signal level is "high".
> 
> But you are right telling that this inconstancy should be addressed.
> 
>> That's the reason why I preferred to not change the names of
>> the variables, even if better names would be desirable.
> 
> 100% agree with this.
> 
>> If you want, I can re-format the patch according to you suggestions,
> 
> Yes
> 
>> remove formatted lines
> 
> Yes

Hi all,

	please find below a better formatted patch.

Best regards,

	Claudio



Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <claudio@evidence.eu.com>

The crisv10.c and the atmel_serial.c serial drivers intepret the fields of the
serial_rs485 structure in a different way.
In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
uses these values to know if a delay must be set before and after sending.
This patch makes the usage of these variables consistent across all drivers and
fixes the Documentation as well.
>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
understood by looking only at the value of delay_rts_before_send and 
delay_rts_after_send.

Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
Signed-off-by: Darron Black <darron@griffin.net>
---
 Documentation/serial/serial-rs485.txt |   14 +++++++++++---
 drivers/tty/serial/atmel_serial.c     |   16 +++-------------
 drivers/tty/serial/crisv10.c          |   10 ++--------
 include/linux/serial.h                |   14 ++++++++------
 4 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
index 079cb3d..41c8378 100644
--- a/Documentation/serial/serial-rs485.txt
+++ b/Documentation/serial/serial-rs485.txt
@@ -97,15 +97,23 @@
 
 	struct serial_rs485 rs485conf;
 
-	/* Set RS485 mode: */
+	/* Enable RS485 mode: */
 	rs485conf.flags |= SER_RS485_ENABLED;
 
+	/* Set logical level for RTS pin equal to 1 when sending: */
+	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
+	/* or, set logical level for RTS pin equal to 0 when sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
+
+	/* Set logical level for RTS pin equal to 1 after sending: */
+	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
+	/* or, set logical level for RTS pin equal to 0 after sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
+
 	/* Set rts delay before send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
 	rs485conf.delay_rts_before_send = ...;
 
 	/* Set rts delay after send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
 	rs485conf.delay_rts_after_send = ...;
 
 	/* Set this flag if you want to receive data even whilst sending data */
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 4a0f86f..4c823f3 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
 	if (rs485conf->flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
 		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
-		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
+		if ((rs485conf->delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
 	} else {
@@ -304,7 +304,7 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
 					atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
@@ -1228,7 +1228,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
 					atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
@@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
 		rs485conf->delay_rts_after_send = rs485_delay[1];
 		rs485conf->flags = 0;
 
-		if (rs485conf->delay_rts_before_send == 0 &&
-		    rs485conf->delay_rts_after_send == 0) {
-			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
-		} else {
-			if (rs485conf->delay_rts_before_send)
-				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
-			if (rs485conf->delay_rts_after_send)
-				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
-		}
-
 		if (of_get_property(np, "rs485-rx-during-tx", NULL))
 			rs485conf->flags |= SER_RS485_RX_DURING_TX;
 
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index b743504..1dfba7b 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
 		e100_disable_rx(info);
 		e100_enable_rx_irq(info);
 #endif
-		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
-			(info->rs485.delay_rts_before_send > 0))
-				msleep(info->rs485.delay_rts_before_send);
+		if (info->rs485.delay_rts_before_send > 0)
+			msleep(info->rs485.delay_rts_before_send);
 	}
 #endif /* CONFIG_ETRAX_RS485 */
 
@@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
 
 		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
 		rs485data.flags = 0;
-		if (rs485data.delay_rts_before_send != 0)
-			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
-		else
-			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 
 		if (rs485ctrl.enabled)
 			rs485data.flags |= SER_RS485_ENABLED;
@@ -4531,7 +4526,6 @@ static int __init rs_init(void)
 		/* Set sane defaults */
 		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
 		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
-		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 		info->rs485.delay_rts_before_send = 0;
 		info->rs485.flags &= ~(SER_RS485_ENABLED);
 #endif
diff --git a/include/linux/serial.h b/include/linux/serial.h
index 97ff8e2..3d86517 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -207,13 +207,15 @@ struct serial_icounter_struct {
 
 struct serial_rs485 {
 	__u32	flags;			/* RS485 feature flags */
-#define SER_RS485_ENABLED		(1 << 0)
-#define SER_RS485_RTS_ON_SEND		(1 << 1)
-#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
-#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
+#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Logical level for
+							   RTS pin when
+							   sending */
+#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
+							   RTS pin after sent*/
 #define SER_RS485_RX_DURING_TX		(1 << 4)
-	__u32	delay_rts_before_send;	/* Milliseconds */
-	__u32	delay_rts_after_send;	/* Milliseconds */
+	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
+	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
 	__u32	padding[5];		/* Memory is cheap, new structs
 					   are a royal PITA .. */
 };
-- 
1.7.1

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Jesper Nilsson]

On Tue, Nov 08, 2011 at 04:45:46PM +0100, Claudio Scordino wrote:
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers intepret the fields of the
> serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
> >From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and
> delay_rts_after_send.
> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Jesper Nilsson]

On Tue, Nov 08, 2011 at 04:45:46PM +0100, Claudio Scordino wrote:
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers intepret the fields of the
> serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
> >From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and
> delay_rts_after_send.
> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Jesper Nilsson]

On Tue, Nov 08, 2011 at 04:45:46PM +0100, Claudio Scordino wrote:
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers intepret the fields of the
> serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
> >From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and
> delay_rts_after_send.
> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>

Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson at axis.com

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Il 08/11/2011 15:24, Greg KH ha scritto:
> On Tue, Nov 08, 2011 at 01:48:04PM +0000, Alan Cox wrote:
>>> The modifications that I have proposed are very minimal, and most
>>> user-space code should continue to work without any difference. Any Cris
>>> user-space code will continue to work, because we didn't change the
>>> behavior of the driver. For Atmel user-space code, instead, the behavior
>>> of the driver changes only if flags are not set and delay variables
>>> contain a value different than 0 (which, hopefully, is not a very common
>>> situation). That's the reason why I preferred to not change the names of
>>> the variables, even if better names would be desirable.
>>
>> We have inconsistency between implementations. We don't have a change in
>> implementation. There isn't any way to resolve that except by fixing the
>> deviating implementation and doing it promptly.
>>
>> With my tty hat on I'm quite happy with this patch. The sooner it is
>> upstream the better.
> 
> Ok, I'll push to get it to Linus for the next rc release.

Hi Greg,

	in case you didn't push it yet, this is the patch with also the ack by Jesper.

Best regards,

	Claudio


Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <claudio@evidence.eu.com>

The crisv10.c and the atmel_serial.c serial drivers intepret the fields of the
serial_rs485 structure in a different way.
In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
uses these values to know if a delay must be set before and after sending.
This patch makes the usage of these variables consistent across all drivers and
fixes the Documentation as well.
>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
understood by looking only at the value of delay_rts_before_send and 
delay_rts_after_send.

Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
Signed-off-by: Darron Black <darron@griffin.net>
Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>
---
 Documentation/serial/serial-rs485.txt |   14 +++++++++++---
 drivers/tty/serial/atmel_serial.c     |   16 +++-------------
 drivers/tty/serial/crisv10.c          |   10 ++--------
 include/linux/serial.h                |   14 ++++++++------
 4 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
index 079cb3d..41c8378 100644
--- a/Documentation/serial/serial-rs485.txt
+++ b/Documentation/serial/serial-rs485.txt
@@ -97,15 +97,23 @@
 
 	struct serial_rs485 rs485conf;
 
-	/* Set RS485 mode: */
+	/* Enable RS485 mode: */
 	rs485conf.flags |= SER_RS485_ENABLED;
 
+	/* Set logical level for RTS pin equal to 1 when sending: */
+	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
+	/* or, set logical level for RTS pin equal to 0 when sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
+
+	/* Set logical level for RTS pin equal to 1 after sending: */
+	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
+	/* or, set logical level for RTS pin equal to 0 after sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
+
 	/* Set rts delay before send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
 	rs485conf.delay_rts_before_send = ...;
 
 	/* Set rts delay after send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
 	rs485conf.delay_rts_after_send = ...;
 
 	/* Set this flag if you want to receive data even whilst sending data */
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 4a0f86f..4c823f3 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
 	if (rs485conf->flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
 		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
-		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
+		if ((rs485conf->delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
 	} else {
@@ -304,7 +304,7 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
 					atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
@@ -1228,7 +1228,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
 					atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
@@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
 		rs485conf->delay_rts_after_send = rs485_delay[1];
 		rs485conf->flags = 0;
 
-		if (rs485conf->delay_rts_before_send == 0 &&
-		    rs485conf->delay_rts_after_send == 0) {
-			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
-		} else {
-			if (rs485conf->delay_rts_before_send)
-				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
-			if (rs485conf->delay_rts_after_send)
-				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
-		}
-
 		if (of_get_property(np, "rs485-rx-during-tx", NULL))
 			rs485conf->flags |= SER_RS485_RX_DURING_TX;
 
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index b743504..1dfba7b 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
 		e100_disable_rx(info);
 		e100_enable_rx_irq(info);
 #endif
-		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
-			(info->rs485.delay_rts_before_send > 0))
-				msleep(info->rs485.delay_rts_before_send);
+		if (info->rs485.delay_rts_before_send > 0)
+			msleep(info->rs485.delay_rts_before_send);
 	}
 #endif /* CONFIG_ETRAX_RS485 */
 
@@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
 
 		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
 		rs485data.flags = 0;
-		if (rs485data.delay_rts_before_send != 0)
-			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
-		else
-			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 
 		if (rs485ctrl.enabled)
 			rs485data.flags |= SER_RS485_ENABLED;
@@ -4531,7 +4526,6 @@ static int __init rs_init(void)
 		/* Set sane defaults */
 		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
 		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
-		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 		info->rs485.delay_rts_before_send = 0;
 		info->rs485.flags &= ~(SER_RS485_ENABLED);
 #endif
diff --git a/include/linux/serial.h b/include/linux/serial.h
index 97ff8e2..3d86517 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -207,13 +207,15 @@ struct serial_icounter_struct {
 
 struct serial_rs485 {
 	__u32	flags;			/* RS485 feature flags */
-#define SER_RS485_ENABLED		(1 << 0)
-#define SER_RS485_RTS_ON_SEND		(1 << 1)
-#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
-#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
+#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Logical level for
+							   RTS pin when
+							   sending */
+#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
+							   RTS pin after sent*/
 #define SER_RS485_RX_DURING_TX		(1 << 4)
-	__u32	delay_rts_before_send;	/* Milliseconds */
-	__u32	delay_rts_after_send;	/* Milliseconds */
+	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
+	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
 	__u32	padding[5];		/* Memory is cheap, new structs
 					   are a royal PITA .. */
 };
-- 
1.7.1

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Il 08/11/2011 15:24, Greg KH ha scritto:
> On Tue, Nov 08, 2011 at 01:48:04PM +0000, Alan Cox wrote:
>>> The modifications that I have proposed are very minimal, and most
>>> user-space code should continue to work without any difference. Any Cris
>>> user-space code will continue to work, because we didn't change the
>>> behavior of the driver. For Atmel user-space code, instead, the behavior
>>> of the driver changes only if flags are not set and delay variables
>>> contain a value different than 0 (which, hopefully, is not a very common
>>> situation). That's the reason why I preferred to not change the names of
>>> the variables, even if better names would be desirable.
>>
>> We have inconsistency between implementations. We don't have a change in
>> implementation. There isn't any way to resolve that except by fixing the
>> deviating implementation and doing it promptly.
>>
>> With my tty hat on I'm quite happy with this patch. The sooner it is
>> upstream the better.
> 
> Ok, I'll push to get it to Linus for the next rc release.

Hi Greg,

	in case you didn't push it yet, this is the patch with also the ack by Jesper.

Best regards,

	Claudio


Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <claudio@evidence.eu.com>

The crisv10.c and the atmel_serial.c serial drivers intepret the fields of the
serial_rs485 structure in a different way.
In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
uses these values to know if a delay must be set before and after sending.
This patch makes the usage of these variables consistent across all drivers and
fixes the Documentation as well.
>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
understood by looking only at the value of delay_rts_before_send and 
delay_rts_after_send.

Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
Signed-off-by: Darron Black <darron@griffin.net>
Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>
---
 Documentation/serial/serial-rs485.txt |   14 +++++++++++---
 drivers/tty/serial/atmel_serial.c     |   16 +++-------------
 drivers/tty/serial/crisv10.c          |   10 ++--------
 include/linux/serial.h                |   14 ++++++++------
 4 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
index 079cb3d..41c8378 100644
--- a/Documentation/serial/serial-rs485.txt
+++ b/Documentation/serial/serial-rs485.txt
@@ -97,15 +97,23 @@
 
 	struct serial_rs485 rs485conf;
 
-	/* Set RS485 mode: */
+	/* Enable RS485 mode: */
 	rs485conf.flags |= SER_RS485_ENABLED;
 
+	/* Set logical level for RTS pin equal to 1 when sending: */
+	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
+	/* or, set logical level for RTS pin equal to 0 when sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
+
+	/* Set logical level for RTS pin equal to 1 after sending: */
+	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
+	/* or, set logical level for RTS pin equal to 0 after sending: */
+	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
+
 	/* Set rts delay before send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
 	rs485conf.delay_rts_before_send = ...;
 
 	/* Set rts delay after send, if needed: */
-	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
 	rs485conf.delay_rts_after_send = ...;
 
 	/* Set this flag if you want to receive data even whilst sending data */
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 4a0f86f..4c823f3 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
 	if (rs485conf->flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
 		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
-		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
+		if ((rs485conf->delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
 	} else {
@@ -304,7 +304,7 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
 					atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
@@ -1228,7 +1228,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
 		dev_dbg(port->dev, "Setting UART to RS485\n");
-		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
+		if ((atmel_port->rs485.delay_rts_after_send) > 0)
 			UART_PUT_TTGR(port,
 					atmel_port->rs485.delay_rts_after_send);
 		mode |= ATMEL_US_USMODE_RS485;
@@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
 		rs485conf->delay_rts_after_send = rs485_delay[1];
 		rs485conf->flags = 0;
 
-		if (rs485conf->delay_rts_before_send == 0 &&
-		    rs485conf->delay_rts_after_send == 0) {
-			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
-		} else {
-			if (rs485conf->delay_rts_before_send)
-				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
-			if (rs485conf->delay_rts_after_send)
-				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
-		}
-
 		if (of_get_property(np, "rs485-rx-during-tx", NULL))
 			rs485conf->flags |= SER_RS485_RX_DURING_TX;
 
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index b743504..1dfba7b 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
 		e100_disable_rx(info);
 		e100_enable_rx_irq(info);
 #endif
-		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
-			(info->rs485.delay_rts_before_send > 0))
-				msleep(info->rs485.delay_rts_before_send);
+		if (info->rs485.delay_rts_before_send > 0)
+			msleep(info->rs485.delay_rts_before_send);
 	}
 #endif /* CONFIG_ETRAX_RS485 */
 
@@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
 
 		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
 		rs485data.flags = 0;
-		if (rs485data.delay_rts_before_send != 0)
-			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
-		else
-			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 
 		if (rs485ctrl.enabled)
 			rs485data.flags |= SER_RS485_ENABLED;
@@ -4531,7 +4526,6 @@ static int __init rs_init(void)
 		/* Set sane defaults */
 		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
 		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
-		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
 		info->rs485.delay_rts_before_send = 0;
 		info->rs485.flags &= ~(SER_RS485_ENABLED);
 #endif
diff --git a/include/linux/serial.h b/include/linux/serial.h
index 97ff8e2..3d86517 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -207,13 +207,15 @@ struct serial_icounter_struct {
 
 struct serial_rs485 {
 	__u32	flags;			/* RS485 feature flags */
-#define SER_RS485_ENABLED		(1 << 0)
-#define SER_RS485_RTS_ON_SEND		(1 << 1)
-#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
-#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
+#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
+#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Logical level for
+							   RTS pin when
+							   sending */
+#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
+							   RTS pin after sent*/
 #define SER_RS485_RX_DURING_TX		(1 << 4)
-	__u32	delay_rts_before_send;	/* Milliseconds */
-	__u32	delay_rts_after_send;	/* Milliseconds */
+	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
+	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
 	__u32	padding[5];		/* Memory is cheap, new structs
 					   are a royal PITA .. */
 };
-- 
1.7.1

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1785 bytes --]

Hi,

I have been working on a patch series which adds hardware RS485 to the 8250
according to the latest developments. The series will be posted tomorrow after
some more tests. However, there is one thing I wondered about:

> From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and 
> delay_rts_after_send.

Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the inverted
signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it will
probably break even more users?)

> diff --git a/include/linux/serial.h b/include/linux/serial.h
> index 97ff8e2..3d86517 100644
> --- a/include/linux/serial.h
> +++ b/include/linux/serial.h
> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>  
>  struct serial_rs485 {
>  	__u32	flags;			/* RS485 feature flags */
> -#define SER_RS485_ENABLED		(1 << 0)
> -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> -#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
> +#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
> +#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Logical level for
> +							   RTS pin when
> +							   sending */
> +#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
> +							   RTS pin after sent*/

Nit: 80 char should be broken here, because that is not readable. Or put the
comment above the define.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Wolfram Sang]

Hi,

I have been working on a patch series which adds hardware RS485 to the 8250
according to the latest developments. The series will be posted tomorrow after
some more tests. However, there is one thing I wondered about:

> From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and 
> delay_rts_after_send.

Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the inverted
signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it will
probably break even more users?)

> diff --git a/include/linux/serial.h b/include/linux/serial.h
> index 97ff8e2..3d86517 100644
> --- a/include/linux/serial.h
> +++ b/include/linux/serial.h
> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>  
>  struct serial_rs485 {
>  	__u32	flags;			/* RS485 feature flags */
> -#define SER_RS485_ENABLED		(1 << 0)
> -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> -#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
> +#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
> +#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Logical level for
> +							   RTS pin when
> +							   sending */
> +#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
> +							   RTS pin after sent*/

Nit: 80 char should be broken here, because that is not readable. Or put the
comment above the define.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20111113/76fe5548/attachment.sig>

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Darron Black]

On 11/13/2011 03:53 PM, Wolfram Sang wrote:
> Hi,
>
> I have been working on a patch series which adds hardware RS485 to the 8250
> according to the latest developments. The series will be posted tomorrow after
> some more tests. However, there is one thing I wondered about:
>
>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
>> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
>> understood by looking only at the value of delay_rts_before_send and
>> delay_rts_after_send.
> Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the inverted
> signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
> SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it will
> probably break even more users?)
It allows the application to configure RTS to not toggle at all in one 
of those two scenarios.

Perhaps the RTS toggle after transmit delay needs to be large, and 
they'd rather do it in userspace than block in the driver. I also recall 
a protocol that would send a master assertion command and hold on to RTS 
afterwards. I can easily imagine needing to quickly transmit something, 
hold on to RTS for a while, then finish your transmit later.

However, I don't have any concrete examples of needing this outside that 
vague recollection of a master assertion sequence in an old embedded 
platform far away from Linux.

>> diff --git a/include/linux/serial.h b/include/linux/serial.h
>> index 97ff8e2..3d86517 100644
>> --- a/include/linux/serial.h
>> +++ b/include/linux/serial.h
>> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>>
>>   struct serial_rs485 {
>>   	__u32	flags;			/* RS485 feature flags */
>> -#define SER_RS485_ENABLED		(1<<  0)
>> -#define SER_RS485_RTS_ON_SEND		(1<<  1)
>> -#define SER_RS485_RTS_AFTER_SEND	(1<<  2)
>> -#define SER_RS485_RTS_BEFORE_SEND	(1<<  3)
>> +#define SER_RS485_ENABLED		(1<<  0)	/* If enabled */
>> +#define SER_RS485_RTS_ON_SEND		(1<<  1)	/* Logical level for
>> +							   RTS pin when
>> +							   sending */
>> +#define SER_RS485_RTS_AFTER_SEND	(1<<  2)	/* Logical level for
>> +							   RTS pin after sent*/
> Nit: 80 char should be broken here, because that is not readable. Or put the
> comment above the define.
>
> Thanks,
>
>     Wolfram
>

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Darron Black]

On 11/13/2011 03:53 PM, Wolfram Sang wrote:
> Hi,
>
> I have been working on a patch series which adds hardware RS485 to the 8250
> according to the latest developments. The series will be posted tomorrow after
> some more tests. However, there is one thing I wondered about:
>
>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
>> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
>> understood by looking only at the value of delay_rts_before_send and
>> delay_rts_after_send.
> Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the inverted
> signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
> SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it will
> probably break even more users?)
It allows the application to configure RTS to not toggle at all in one 
of those two scenarios.

Perhaps the RTS toggle after transmit delay needs to be large, and 
they'd rather do it in userspace than block in the driver. I also recall 
a protocol that would send a master assertion command and hold on to RTS 
afterwards. I can easily imagine needing to quickly transmit something, 
hold on to RTS for a while, then finish your transmit later.

However, I don't have any concrete examples of needing this outside that 
vague recollection of a master assertion sequence in an old embedded 
platform far away from Linux.

>> diff --git a/include/linux/serial.h b/include/linux/serial.h
>> index 97ff8e2..3d86517 100644
>> --- a/include/linux/serial.h
>> +++ b/include/linux/serial.h
>> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>>
>>   struct serial_rs485 {
>>   	__u32	flags;			/* RS485 feature flags */
>> -#define SER_RS485_ENABLED		(1<<  0)
>> -#define SER_RS485_RTS_ON_SEND		(1<<  1)
>> -#define SER_RS485_RTS_AFTER_SEND	(1<<  2)
>> -#define SER_RS485_RTS_BEFORE_SEND	(1<<  3)
>> +#define SER_RS485_ENABLED		(1<<  0)	/* If enabled */
>> +#define SER_RS485_RTS_ON_SEND		(1<<  1)	/* Logical level for
>> +							   RTS pin when
>> +							   sending */
>> +#define SER_RS485_RTS_AFTER_SEND	(1<<  2)	/* Logical level for
>> +							   RTS pin after sent*/
> Nit: 80 char should be broken here, because that is not readable. Or put the
> comment above the define.
>
> Thanks,
>
>     Wolfram
>

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Darron Black]

On 11/13/2011 03:53 PM, Wolfram Sang wrote:
> Hi,
>
> I have been working on a patch series which adds hardware RS485 to the 8250
> according to the latest developments. The series will be posted tomorrow after
> some more tests. However, there is one thing I wondered about:
>
>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
>> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
>> understood by looking only at the value of delay_rts_before_send and
>> delay_rts_after_send.
> Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the inverted
> signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
> SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it will
> probably break even more users?)
It allows the application to configure RTS to not toggle at all in one 
of those two scenarios.

Perhaps the RTS toggle after transmit delay needs to be large, and 
they'd rather do it in userspace than block in the driver. I also recall 
a protocol that would send a master assertion command and hold on to RTS 
afterwards. I can easily imagine needing to quickly transmit something, 
hold on to RTS for a while, then finish your transmit later.

However, I don't have any concrete examples of needing this outside that 
vague recollection of a master assertion sequence in an old embedded 
platform far away from Linux.

>> diff --git a/include/linux/serial.h b/include/linux/serial.h
>> index 97ff8e2..3d86517 100644
>> --- a/include/linux/serial.h
>> +++ b/include/linux/serial.h
>> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>>
>>   struct serial_rs485 {
>>   	__u32	flags;			/* RS485 feature flags */
>> -#define SER_RS485_ENABLED		(1<<  0)
>> -#define SER_RS485_RTS_ON_SEND		(1<<  1)
>> -#define SER_RS485_RTS_AFTER_SEND	(1<<  2)
>> -#define SER_RS485_RTS_BEFORE_SEND	(1<<  3)
>> +#define SER_RS485_ENABLED		(1<<  0)	/* If enabled */
>> +#define SER_RS485_RTS_ON_SEND		(1<<  1)	/* Logical level for
>> +							   RTS pin when
>> +							   sending */
>> +#define SER_RS485_RTS_AFTER_SEND	(1<<  2)	/* Logical level for
>> +							   RTS pin after sent*/
> Nit: 80 char should be broken here, because that is not readable. Or put the
> comment above the define.
>
> Thanks,
>
>     Wolfram
>

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Il 13/11/2011 22:53, Wolfram Sang ha scritto:
> Hi,
> 
> I have been working on a patch series which adds hardware RS485 to the 8250
> according to the latest developments. The series will be posted tomorrow after
> some more tests. However, there is one thing I wondered about:
> 
>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
>> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
>> understood by looking only at the value of delay_rts_before_send and
>> delay_rts_after_send.
> 
> Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the inverted
> signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
> SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it will
> probably break even more users?)

I think so, but I'm not sure since the original version of the Cris driver (prior than the
RS485 data structure) contained both values: a value for RTS during send and a value
for RTS after sent. That's why both vales have been reported inside the RS485 data structure...

Best regards,

	Claudio

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Claudio Scordino]

Il 13/11/2011 22:53, Wolfram Sang ha scritto:
> Hi,
> 
> I have been working on a patch series which adds hardware RS485 to the 8250
> according to the latest developments. The series will be posted tomorrow after
> some more tests. However, there is one thing I wondered about:
> 
>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
>> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
>> understood by looking only at the value of delay_rts_before_send and
>> delay_rts_after_send.
> 
> Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the inverted
> signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
> SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it will
> probably break even more users?)

I think so, but I'm not sure since the original version of the Cris driver (prior than the
RS485 data structure) contained both values: a value for RTS during send and a value
for RTS after sent. That's why both vales have been reported inside the RS485 data structure...

Best regards,

	Claudio

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Nicolas Ferre]

On 11/14/2011 01:37 AM, Darron Black :
> On 11/13/2011 03:53 PM, Wolfram Sang wrote:
>> Hi,
>>
>> I have been working on a patch series which adds hardware RS485 to the
>> 8250
>> according to the latest developments. The series will be posted
>> tomorrow after
>> some more tests. However, there is one thing I wondered about:
>>
>>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
>>> be used to
>>> set the voltage of the RTS pin (as in the crisv10.c driver); the
>>> delay will be
>>> understood by looking only at the value of delay_rts_before_send and
>>> delay_rts_after_send.
>> Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the
>> inverted
>> signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
>> SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it
>> will
>> probably break even more users?)
> It allows the application to configure RTS to not toggle at all in one
> of those two scenarios.
> 
> Perhaps the RTS toggle after transmit delay needs to be large, and
> they'd rather do it in userspace than block in the driver. I also recall
> a protocol that would send a master assertion command and hold on to RTS
> afterwards. I can easily imagine needing to quickly transmit something,
> hold on to RTS for a while, then finish your transmit later.
> 
> However, I don't have any concrete examples of needing this outside that
> vague recollection of a master assertion sequence in an old embedded
> platform far away from Linux.

Darron, Claudio,

This explanation makes sense. Thanks for this.

Bye,


>>> diff --git a/include/linux/serial.h b/include/linux/serial.h
>>> index 97ff8e2..3d86517 100644
>>> --- a/include/linux/serial.h
>>> +++ b/include/linux/serial.h
>>> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>>>
>>>   struct serial_rs485 {
>>>       __u32    flags;            /* RS485 feature flags */
>>> -#define SER_RS485_ENABLED        (1<<  0)
>>> -#define SER_RS485_RTS_ON_SEND        (1<<  1)
>>> -#define SER_RS485_RTS_AFTER_SEND    (1<<  2)
>>> -#define SER_RS485_RTS_BEFORE_SEND    (1<<  3)
>>> +#define SER_RS485_ENABLED        (1<<  0)    /* If enabled */
>>> +#define SER_RS485_RTS_ON_SEND        (1<<  1)    /* Logical level for
>>> +                               RTS pin when
>>> +                               sending */
>>> +#define SER_RS485_RTS_AFTER_SEND    (1<<  2)    /* Logical level for
>>> +                               RTS pin after sent*/
>> Nit: 80 char should be broken here, because that is not readable. Or
>> put the
>> comment above the define.
>>
>> Thanks,
>>
>>     Wolfram
>>
> 
> 


-- 
Nicolas Ferre

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Nicolas Ferre]

On 11/14/2011 01:37 AM, Darron Black :
> On 11/13/2011 03:53 PM, Wolfram Sang wrote:
>> Hi,
>>
>> I have been working on a patch series which adds hardware RS485 to the
>> 8250
>> according to the latest developments. The series will be posted
>> tomorrow after
>> some more tests. However, there is one thing I wondered about:
>>
>>>  From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will
>>> be used to
>>> set the voltage of the RTS pin (as in the crisv10.c driver); the
>>> delay will be
>>> understood by looking only at the value of delay_rts_before_send and
>>> delay_rts_after_send.
>> Do I overlook something or is SER_RS485_RTS_AFTER_SEND always the
>> inverted
>> signal of SER_RS485_RTS_ON_SEND. So why do we need both? (BTW
>> SER_RS485_RTS_ON_SEND is a non-obvious name, I think. But changing it
>> will
>> probably break even more users?)
> It allows the application to configure RTS to not toggle at all in one
> of those two scenarios.
> 
> Perhaps the RTS toggle after transmit delay needs to be large, and
> they'd rather do it in userspace than block in the driver. I also recall
> a protocol that would send a master assertion command and hold on to RTS
> afterwards. I can easily imagine needing to quickly transmit something,
> hold on to RTS for a while, then finish your transmit later.
> 
> However, I don't have any concrete examples of needing this outside that
> vague recollection of a master assertion sequence in an old embedded
> platform far away from Linux.

Darron, Claudio,

This explanation makes sense. Thanks for this.

Bye,


>>> diff --git a/include/linux/serial.h b/include/linux/serial.h
>>> index 97ff8e2..3d86517 100644
>>> --- a/include/linux/serial.h
>>> +++ b/include/linux/serial.h
>>> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>>>
>>>   struct serial_rs485 {
>>>       __u32    flags;            /* RS485 feature flags */
>>> -#define SER_RS485_ENABLED        (1<<  0)
>>> -#define SER_RS485_RTS_ON_SEND        (1<<  1)
>>> -#define SER_RS485_RTS_AFTER_SEND    (1<<  2)
>>> -#define SER_RS485_RTS_BEFORE_SEND    (1<<  3)
>>> +#define SER_RS485_ENABLED        (1<<  0)    /* If enabled */
>>> +#define SER_RS485_RTS_ON_SEND        (1<<  1)    /* Logical level for
>>> +                               RTS pin when
>>> +                               sending */
>>> +#define SER_RS485_RTS_AFTER_SEND    (1<<  2)    /* Logical level for
>>> +                               RTS pin after sent*/
>> Nit: 80 char should be broken here, because that is not readable. Or
>> put the
>> comment above the define.
>>
>> Thanks,
>>
>>     Wolfram
>>
> 
> 


-- 
Nicolas Ferre

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Alan Cox]

> Perhaps the RTS toggle after transmit delay needs to be large, and 
> they'd rather do it in userspace than block in the driver. I also
> recall a protocol that would send a master assertion command and hold
> on to RTS afterwards. I can easily imagine needing to quickly

Appletalk does this for one. Some protocols use it for priming low cpu
power receivers. You fire a header message at the target which fits
into its serial fifo, it then responds and can sit in a tight loop
getting the following frame data.

Alan

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Alan Cox]

> Perhaps the RTS toggle after transmit delay needs to be large, and 
> they'd rather do it in userspace than block in the driver. I also
> recall a protocol that would send a master assertion command and hold
> on to RTS afterwards. I can easily imagine needing to quickly

Appletalk does this for one. Some protocols use it for priming low cpu
power receivers. You fire a header message at the target which fits
into its serial fifo, it then responds and can sit in a tight loop
getting the following frame data.

Alan

Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables

[Nicolas Ferre]

On 11/09/2011 03:51 PM, Claudio Scordino :
> Il 08/11/2011 15:24, Greg KH ha scritto:
>> On Tue, Nov 08, 2011 at 01:48:04PM +0000, Alan Cox wrote:
>>>> The modifications that I have proposed are very minimal, and most
>>>> user-space code should continue to work without any difference. Any Cris
>>>> user-space code will continue to work, because we didn't change the
>>>> behavior of the driver. For Atmel user-space code, instead, the behavior
>>>> of the driver changes only if flags are not set and delay variables
>>>> contain a value different than 0 (which, hopefully, is not a very common
>>>> situation). That's the reason why I preferred to not change the names of
>>>> the variables, even if better names would be desirable.
>>>
>>> We have inconsistency between implementations. We don't have a change in
>>> implementation. There isn't any way to resolve that except by fixing the
>>> deviating implementation and doing it promptly.
>>>
>>> With my tty hat on I'm quite happy with this patch. The sooner it is
>>> upstream the better.
>>
>> Ok, I'll push to get it to Linus for the next rc release.
> 
> Hi Greg,
> 
> 	in case you didn't push it yet, this is the patch with also the ack by Jesper.
> 
> Best regards,
> 
> 	Claudio
> 
> 
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers intepret the fields of the
> serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
>>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and 
> delay_rts_after_send.
> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>
> Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

Surely too late, but I add it for the record:

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


> ---
>  Documentation/serial/serial-rs485.txt |   14 +++++++++++---
>  drivers/tty/serial/atmel_serial.c     |   16 +++-------------
>  drivers/tty/serial/crisv10.c          |   10 ++--------
>  include/linux/serial.h                |   14 ++++++++------
>  4 files changed, 24 insertions(+), 30 deletions(-)
> 
> diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
> index 079cb3d..41c8378 100644
> --- a/Documentation/serial/serial-rs485.txt
> +++ b/Documentation/serial/serial-rs485.txt
> @@ -97,15 +97,23 @@
>  
>  	struct serial_rs485 rs485conf;
>  
> -	/* Set RS485 mode: */
> +	/* Enable RS485 mode: */
>  	rs485conf.flags |= SER_RS485_ENABLED;
>  
> +	/* Set logical level for RTS pin equal to 1 when sending: */
> +	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
> +	/* or, set logical level for RTS pin equal to 0 when sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
> +
> +	/* Set logical level for RTS pin equal to 1 after sending: */
> +	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
> +	/* or, set logical level for RTS pin equal to 0 after sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
> +
>  	/* Set rts delay before send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
>  	rs485conf.delay_rts_before_send = ...;
>  
>  	/* Set rts delay after send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
>  	rs485conf.delay_rts_after_send = ...;
>  
>  	/* Set this flag if you want to receive data even whilst sending data */
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 4a0f86f..4c823f3 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
>  	if (rs485conf->flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
>  		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> -		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((rs485conf->delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
> @@ -304,7 +304,7 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
>  					atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
> @@ -1228,7 +1228,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
>  					atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
> @@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
>  		rs485conf->delay_rts_after_send = rs485_delay[1];
>  		rs485conf->flags = 0;
>  
> -		if (rs485conf->delay_rts_before_send == 0 &&
> -		    rs485conf->delay_rts_after_send == 0) {
> -			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
> -		} else {
> -			if (rs485conf->delay_rts_before_send)
> -				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
> -			if (rs485conf->delay_rts_after_send)
> -				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
> -		}
> -
>  		if (of_get_property(np, "rs485-rx-during-tx", NULL))
>  			rs485conf->flags |= SER_RS485_RX_DURING_TX;
>  
> diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
> index b743504..1dfba7b 100644
> --- a/drivers/tty/serial/crisv10.c
> +++ b/drivers/tty/serial/crisv10.c
> @@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
>  		e100_disable_rx(info);
>  		e100_enable_rx_irq(info);
>  #endif
> -		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
> -			(info->rs485.delay_rts_before_send > 0))
> -				msleep(info->rs485.delay_rts_before_send);
> +		if (info->rs485.delay_rts_before_send > 0)
> +			msleep(info->rs485.delay_rts_before_send);
>  	}
>  #endif /* CONFIG_ETRAX_RS485 */
>  
> @@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
>  
>  		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
>  		rs485data.flags = 0;
> -		if (rs485data.delay_rts_before_send != 0)
> -			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
> -		else
> -			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  
>  		if (rs485ctrl.enabled)
>  			rs485data.flags |= SER_RS485_ENABLED;
> @@ -4531,7 +4526,6 @@ static int __init rs_init(void)
>  		/* Set sane defaults */
>  		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
>  		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
> -		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  		info->rs485.delay_rts_before_send = 0;
>  		info->rs485.flags &= ~(SER_RS485_ENABLED);
>  #endif
> diff --git a/include/linux/serial.h b/include/linux/serial.h
> index 97ff8e2..3d86517 100644
> --- a/include/linux/serial.h
> +++ b/include/linux/serial.h
> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>  
>  struct serial_rs485 {
>  	__u32	flags;			/* RS485 feature flags */
> -#define SER_RS485_ENABLED		(1 << 0)
> -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> -#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
> +#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
> +#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Logical level for
> +							   RTS pin when
> +							   sending */
> +#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
> +							   RTS pin after sent*/
>  #define SER_RS485_RX_DURING_TX		(1 << 4)
> -	__u32	delay_rts_before_send;	/* Milliseconds */
> -	__u32	delay_rts_after_send;	/* Milliseconds */
> +	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
> +	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
>  	__u32	padding[5];		/* Memory is cheap, new structs
>  					   are a royal PITA .. */
>  };


-- 
Nicolas Ferre

[PATCH] RS485: fix inconsistencies in the meaning of some variables

[Nicolas Ferre]

On 11/09/2011 03:51 PM, Claudio Scordino :
> Il 08/11/2011 15:24, Greg KH ha scritto:
>> On Tue, Nov 08, 2011 at 01:48:04PM +0000, Alan Cox wrote:
>>>> The modifications that I have proposed are very minimal, and most
>>>> user-space code should continue to work without any difference. Any Cris
>>>> user-space code will continue to work, because we didn't change the
>>>> behavior of the driver. For Atmel user-space code, instead, the behavior
>>>> of the driver changes only if flags are not set and delay variables
>>>> contain a value different than 0 (which, hopefully, is not a very common
>>>> situation). That's the reason why I preferred to not change the names of
>>>> the variables, even if better names would be desirable.
>>>
>>> We have inconsistency between implementations. We don't have a change in
>>> implementation. There isn't any way to resolve that except by fixing the
>>> deviating implementation and doing it promptly.
>>>
>>> With my tty hat on I'm quite happy with this patch. The sooner it is
>>> upstream the better.
>>
>> Ok, I'll push to get it to Linus for the next rc release.
> 
> Hi Greg,
> 
> 	in case you didn't push it yet, this is the patch with also the ack by Jesper.
> 
> Best regards,
> 
> 	Claudio
> 
> 
> Subject: RS485: fix inconsistencies in the meaning of some variables
> From: Claudio Scordino <claudio@evidence.eu.com>
> 
> The crisv10.c and the atmel_serial.c serial drivers intepret the fields of the
> serial_rs485 structure in a different way.
> In particular, crisv10.c uses SER_RS485_RTS_AFTER_SEND and 
> SER_RS485_RTS_ON_SEND for the voltage of the RTS pin; atmel_serial.c, instead,
> uses these values to know if a delay must be set before and after sending.
> This patch makes the usage of these variables consistent across all drivers and
> fixes the Documentation as well.
>>From now on, SER_RS485_RTS_AFTER_SEND and SER_RS485_RTS_ON_SEND will be used to
> set the voltage of the RTS pin (as in the crisv10.c driver); the delay will be
> understood by looking only at the value of delay_rts_before_send and 
> delay_rts_after_send.
> 
> Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
> Signed-off-by: Darron Black <darron@griffin.net>
> Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>

Surely too late, but I add it for the record:

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


> ---
>  Documentation/serial/serial-rs485.txt |   14 +++++++++++---
>  drivers/tty/serial/atmel_serial.c     |   16 +++-------------
>  drivers/tty/serial/crisv10.c          |   10 ++--------
>  include/linux/serial.h                |   14 ++++++++------
>  4 files changed, 24 insertions(+), 30 deletions(-)
> 
> diff --git a/Documentation/serial/serial-rs485.txt b/Documentation/serial/serial-rs485.txt
> index 079cb3d..41c8378 100644
> --- a/Documentation/serial/serial-rs485.txt
> +++ b/Documentation/serial/serial-rs485.txt
> @@ -97,15 +97,23 @@
>  
>  	struct serial_rs485 rs485conf;
>  
> -	/* Set RS485 mode: */
> +	/* Enable RS485 mode: */
>  	rs485conf.flags |= SER_RS485_ENABLED;
>  
> +	/* Set logical level for RTS pin equal to 1 when sending: */
> +	rs485conf.flags |= SER_RS485_RTS_ON_SEND;
> +	/* or, set logical level for RTS pin equal to 0 when sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_ON_SEND);
> +
> +	/* Set logical level for RTS pin equal to 1 after sending: */
> +	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
> +	/* or, set logical level for RTS pin equal to 0 after sending: */
> +	rs485conf.flags &= ~(SER_RS485_RTS_AFTER_SEND);
> +
>  	/* Set rts delay before send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_BEFORE_SEND;
>  	rs485conf.delay_rts_before_send = ...;
>  
>  	/* Set rts delay after send, if needed: */
> -	rs485conf.flags |= SER_RS485_RTS_AFTER_SEND;
>  	rs485conf.delay_rts_after_send = ...;
>  
>  	/* Set this flag if you want to receive data even whilst sending data */
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 4a0f86f..4c823f3 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -228,7 +228,7 @@ void atmel_config_rs485(struct uart_port *port, struct serial_rs485 *rs485conf)
>  	if (rs485conf->flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
>  		atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
> -		if (rs485conf->flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((rs485conf->delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port, rs485conf->delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
>  	} else {
> @@ -304,7 +304,7 @@ static void atmel_set_mctrl(struct uart_port *port, u_int mctrl)
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
>  					atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
> @@ -1228,7 +1228,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
>  
>  	if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
>  		dev_dbg(port->dev, "Setting UART to RS485\n");
> -		if (atmel_port->rs485.flags & SER_RS485_RTS_AFTER_SEND)
> +		if ((atmel_port->rs485.delay_rts_after_send) > 0)
>  			UART_PUT_TTGR(port,
>  					atmel_port->rs485.delay_rts_after_send);
>  		mode |= ATMEL_US_USMODE_RS485;
> @@ -1447,16 +1447,6 @@ static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port,
>  		rs485conf->delay_rts_after_send = rs485_delay[1];
>  		rs485conf->flags = 0;
>  
> -		if (rs485conf->delay_rts_before_send == 0 &&
> -		    rs485conf->delay_rts_after_send == 0) {
> -			rs485conf->flags |= SER_RS485_RTS_ON_SEND;
> -		} else {
> -			if (rs485conf->delay_rts_before_send)
> -				rs485conf->flags |= SER_RS485_RTS_BEFORE_SEND;
> -			if (rs485conf->delay_rts_after_send)
> -				rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
> -		}
> -
>  		if (of_get_property(np, "rs485-rx-during-tx", NULL))
>  			rs485conf->flags |= SER_RS485_RX_DURING_TX;
>  
> diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
> index b743504..1dfba7b 100644
> --- a/drivers/tty/serial/crisv10.c
> +++ b/drivers/tty/serial/crisv10.c
> @@ -3234,9 +3234,8 @@ rs_write(struct tty_struct *tty,
>  		e100_disable_rx(info);
>  		e100_enable_rx_irq(info);
>  #endif
> -		if ((info->rs485.flags & SER_RS485_RTS_BEFORE_SEND) &&
> -			(info->rs485.delay_rts_before_send > 0))
> -				msleep(info->rs485.delay_rts_before_send);
> +		if (info->rs485.delay_rts_before_send > 0)
> +			msleep(info->rs485.delay_rts_before_send);
>  	}
>  #endif /* CONFIG_ETRAX_RS485 */
>  
> @@ -3693,10 +3692,6 @@ rs_ioctl(struct tty_struct *tty,
>  
>  		rs485data.delay_rts_before_send = rs485ctrl.delay_rts_before_send;
>  		rs485data.flags = 0;
> -		if (rs485data.delay_rts_before_send != 0)
> -			rs485data.flags |= SER_RS485_RTS_BEFORE_SEND;
> -		else
> -			rs485data.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  
>  		if (rs485ctrl.enabled)
>  			rs485data.flags |= SER_RS485_ENABLED;
> @@ -4531,7 +4526,6 @@ static int __init rs_init(void)
>  		/* Set sane defaults */
>  		info->rs485.flags &= ~(SER_RS485_RTS_ON_SEND);
>  		info->rs485.flags |= SER_RS485_RTS_AFTER_SEND;
> -		info->rs485.flags &= ~(SER_RS485_RTS_BEFORE_SEND);
>  		info->rs485.delay_rts_before_send = 0;
>  		info->rs485.flags &= ~(SER_RS485_ENABLED);
>  #endif
> diff --git a/include/linux/serial.h b/include/linux/serial.h
> index 97ff8e2..3d86517 100644
> --- a/include/linux/serial.h
> +++ b/include/linux/serial.h
> @@ -207,13 +207,15 @@ struct serial_icounter_struct {
>  
>  struct serial_rs485 {
>  	__u32	flags;			/* RS485 feature flags */
> -#define SER_RS485_ENABLED		(1 << 0)
> -#define SER_RS485_RTS_ON_SEND		(1 << 1)
> -#define SER_RS485_RTS_AFTER_SEND	(1 << 2)
> -#define SER_RS485_RTS_BEFORE_SEND	(1 << 3)
> +#define SER_RS485_ENABLED		(1 << 0)	/* If enabled */
> +#define SER_RS485_RTS_ON_SEND		(1 << 1)	/* Logical level for
> +							   RTS pin when
> +							   sending */
> +#define SER_RS485_RTS_AFTER_SEND	(1 << 2)	/* Logical level for
> +							   RTS pin after sent*/
>  #define SER_RS485_RX_DURING_TX		(1 << 4)
> -	__u32	delay_rts_before_send;	/* Milliseconds */
> -	__u32	delay_rts_after_send;	/* Milliseconds */
> +	__u32	delay_rts_before_send;	/* Delay before send (milliseconds) */
> +	__u32	delay_rts_after_send;	/* Delay after send (milliseconds) */
>  	__u32	padding[5];		/* Memory is cheap, new structs
>  					   are a royal PITA .. */
>  };


-- 
Nicolas Ferre