atmel_serial: RS485: receiving enabled when sending data
Subsystems (4)
PATCH 1/1 atmel_serial: RS485: receiving enabled when sending data
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.
--- drivers/tty/serial/atmel_serial.c | 17 ++++++++++------- include/linux/serial.h | 1 + 2 files changed, 11 insertions(+), 7 deletions(-)
(quoted diff) +0 -0
b/drivers/tty/serial/atmel_serial.c
index af9b781..5f6c745 100644
drivers/tty/serial/atmel_serial.c +10 -7
@@ -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); } }
include/linux/serial.h +1 -0
@@ -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
Signature · 1 line
1.7.1
Quoted 48 lines · Bernhard Roth
Hello!
By default the
atmel_serialdriver 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 busUsually 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_serialdriver 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 dataBy default the
atmel_serialdriver 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 <[email protected]>Signed-off-by Claudio Scordino <[email protected]>--- drivers/tty/serial/atmel_serial.c | 17 ++++++++++------- include/linux/serial.h | 1 + 2 files changed, 11 insertions(+), 7 deletions(-)drivers/tty/serial/atmel_serial.c +0
-0
drivers/tty/serial/atmel_serial.c static void atmel_stop_tx(struct uart_port *port) · line 344
Quoted 8 lines
drivers/tty/serial/atmel_serial.c +3
-2@@ -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
Il 22/08/2011 23:18, Greg KH ha scritto:
Quoted 61 lines · Bernhard Roth
Quoted 48 lines
Hello!
By default the
atmel_serialdriver 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 busUsually 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_serialdriver 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 dataBy default the
atmel_serialdriver 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<[email protected]>Signed-off-by Claudio Scordino<[email protected]>--- drivers/tty/serial/atmel_serial.c | 17 ++++++++++------- include/linux/serial.h | 1 + 2 files changed, 11 insertions(+), 7 deletions(-)drivers/tty/serial/atmel_serial.c +0
-0drivers/tty/serial/atmel_serial.c static void atmel_stop_tx(struct uart_port *port) · line 344
Quoted 8 lines
drivers/tty/serial/atmel_serial.c +3
-2@@ -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?
Quoted 3 lines
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.
--- drivers/tty/serial/atmel_serial.c | 17 ++++++++++------- include/linux/serial.h | 1 + 2 files changed, 11 insertions(+), 7 deletions(-)
drivers/tty/serial/atmel_serial.c +10 -7
@@ -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); } }
include/linux/serial.h +1 -0
@@ -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
Signature · 1 line
1.7.1
drivers/tty/serial/atmel_serial.c static void atmel_stop_tx(struct uart_port *port) · line 344
Quoted 13 lines · Claudio Scordino
drivers/tty/serial/atmel_serial.c +3
-2@@ -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.
drivers/tty/serial/atmel_serial.c static void atmel_tx_dma(struct uart_port *port) · line 688
Quoted 4 lines
drivers/tty/serial/atmel_serial.c +4
-0+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.
Il 23/08/2011 11:30, Russell King - ARM Linux ha scritto:
Quoted 33 lines · Claudio Scordino
drivers/tty/serial/atmel_serial.c static void atmel_stop_tx(struct uart_port *port) · line 344
Quoted 13 lines
drivers/tty/serial/atmel_serial.c +3
-2@@ -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);
Quoted 9 lines
IOW, use spaces to align the wrapped 'if' statement.
drivers/tty/serial/atmel_serial.c static void atmel_stop_tx(struct uart_port *port)
Quoted 4 lines
drivers/tty/serial/atmel_serial.c +4
-0+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.
--- drivers/tty/serial/atmel_serial.c | 9 ++++++--- include/linux/serial.h | 1 + 2 files changed, 7 insertions(+), 3 deletions(-)
drivers/tty/serial/atmel_serial.c +6 -3
@@ -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); }
include/linux/serial.h +1 -0
@@ -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
Signature · 1 line
1.7.1
Alan Cox
Acked-byQuoted 8 lines
atmel_serial: RS485: receiving enabled when sending dataBy default the
atmel_serialdriver 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 <[email protected]>Signed-off-by Claudio Scordino <[email protected]>
(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.
Quoted 6 lines · Alan Cox
Quoted 6 lines
atmel_serial: RS485: receiving enabled when sending dataBy default the
atmel_serialdriver 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.
Quoted 7 lines
Quoted 2 lines
Signed-off-by Bernhard Roth <[email protected]>Signed-off-by Claudio Scordino <[email protected]>Acked-by Alan Cox <[email protected]>(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.
Quoted 64 lines · Claudio Scordino
Il 22/08/2011 23:18, Greg KH ha scritto:
Quoted 63 lines · Bernhard Roth
Quoted 48 lines
Hello!
By default the
atmel_serialdriver 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 busUsually 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_serialdriver 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 dataBy default the
atmel_serialdriver 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<[email protected]>Signed-off-by Claudio Scordino<[email protected]>--- drivers/tty/serial/atmel_serial.c | 17 ++++++++++------- include/linux/serial.h | 1 + 2 files changed, 11 insertions(+), 7 deletions(-)drivers/tty/serial/atmel_serial.c +0
-0drivers/tty/serial/atmel_serial.c static void atmel_stop_tx(struct uart_port *port) · line 344
Quoted 8 lines
drivers/tty/serial/atmel_serial.c +3
-2@@ -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,
Quoted 7 lines
Quoted 1 line
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 <[email protected]>
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
Claudio Scordino
Acked-by Alan CoxIl 23/08/2011 17:39, Greg KH ha scritto:
Quoted 75 lines · Claudio Scordino
Quoted 64 lines
Il 22/08/2011 23:18, Greg KH ha scritto:
Quoted 63 lines · Bernhard Roth
Quoted 48 lines
Hello!
By default the
atmel_serialdriver 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 busUsually 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_serialdriver 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 dataBy default the
atmel_serialdriver 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<[email protected]>Signed-off-by Claudio Scordino<[email protected]>--- drivers/tty/serial/atmel_serial.c | 17 ++++++++++------- include/linux/serial.h | 1 + 2 files changed, 11 insertions(+), 7 deletions(-)drivers/tty/serial/atmel_serial.c +0
-0drivers/tty/serial/atmel_serial.c static void atmel_stop_tx(struct uart_port *port) · line 344
Quoted 8 lines
drivers/tty/serial/atmel_serial.c +3
-2@@ -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,
Quoted 7 lines
Quoted 1 line
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:
Quoted 7 lines
From: foo<[email protected]>
as the first line of the patch changelog portion and git will fix things up correctly.
Documentation/SubmittingPatchesdescribes 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 <[email protected]>
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.
--- 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(-)
Documentation/serial/serial-rs485.txt +3 -0
@@ -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. */ }
drivers/tty/serial/atmel_serial.c +6 -3
@@ -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); }
include/linux/serial.h +1 -0
@@ -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
Signature · 1 line
1.7.1
PATCH 1/1 RS485: fix inconsistencies in the meaning of some variables
In reply to Greg KHHi 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 <[email protected]>
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
Quoted 11 lines · Claudio Scordino
Hi Alan, Hi Greg,
it seems that the
crisv10.cand theatmel_serial.cserial
drivers interpret the fields of theserial_rs485structure in a different way.In particular, it seems that
crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor the_logicvalue_of the RTS pin;atmel_serial.c, instead, uses these values to know if a _delay_ must be set before and after sending.
Quoted 6 lines
This patch makes the usage of these variables consistent across all drivers and fixes the Documentation as well.
In particular,SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to set the logic value of the RTS pin (as in thecrisv10.cdriver); the delay is understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.
Looks good!
Quoted 1 line
Claudio
/Jesper
Quoted 173 lines
Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <[email protected]>The
crisv10.cand theatmel_serial.cserial drivers interpret the fields of theserial_rs485structure in a different way.
In particular,crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor 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.Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used toset the voltage of the RTS pin (as in the
crisv10.cdriver); the delay will be understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Signed-off-by Claudio Scordino <[email protected]>Signed-off-by Darron Black <[email protected]>--- 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(-)Documentation/serial/serial-rs485.txt +11
-3@@ -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 */drivers/tty/serial/atmel_serial.c +5
-15@@ -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 ;drivers/tty/serial/crisv10.c +2
-8@@ -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 include/linux/serial.h +8
-6@@ -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 .. */ };Signature · 1 line
1.7.1
/^JN - Jesper Nilsson
Signature · 1 line
Jesper Nilsson -- jesper.nilsson at axis.com
Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables
In reply to Claudio ScordinoOn 11/04/2011 09:19 AM, Claudio Scordino :
Quoted 10 lines
Hi Alan, Hi Greg,
it seems that the
crisv10.cand theatmel_serial.cserial
drivers interpret the fields of theserial_rs485structure in a different way.In particular, it seems that
crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor the_logicvalue_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...
Quoted 22 lines
This patch makes the usage of these variables consistent across all drivers and fixes the Documentation as well.
In particular,SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to set the logic value of the RTS pin (as in thecrisv10.cdriver); the delay is understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Best regards,
Claudio
Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <[email protected]>The
crisv10.cand theatmel_serial.cserial drivers interpret the fields of theserial_rs485structure in a different way.
In particular,crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor 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.
Quoted 4 lines
Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used toset the voltage of the RTS pin (as in the
crisv10.cdriver); the delay will be understood by looking only at the value ofdelay_rts_before_sendanddelay_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.
Quoted 54 lines
Signed-off-by Claudio Scordino <[email protected]>Signed-off-by Darron Black <[email protected]>--- 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(-)Documentation/serial/serial-rs485.txt +11
-3@@ -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 */drivers/tty/serial/atmel_serial.c +1
-1@@ -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 {
drivers/tty/serial/atmel_serial.c static void atmel_set_mctrl(struct uart_port *port, u_int mctrl) · line 309
Quoted 8 lines
drivers/tty/serial/atmel_serial.c +2
-2@@ -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?
drivers/tty/serial/atmel_serial.c static void atmel_set_termios(struct uart_port *port, struct ktermios *termios, · line 1233
Quoted 12 lines
drivers/tty/serial/atmel_serial.c +2
-2mode |=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?
Quoted 7 lines
drivers/tty/serial/atmel_serial.c +0
-0mode |=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 ;
drivers/tty/serial/atmel_serial.c static void __devinit atmel_of_init_port(struct atmel_uart_port *atmel_port, · line 1459
Quoted 10 lines
drivers/tty/serial/atmel_serial.c +0
-10-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.
Quoted 3 lines
drivers/tty/serial/atmel_serial.c +0
-0if (of_get_property (np,"rs485-rx-during-tx" ,NULL )) rs485conf->flags |=SER_RS485_RX_DURING_TX ;
[..]
Quoted 5 lines
include/linux/serial.h +0
-0
include/linux/serial.h struct serial_icounter_struct { · line 211
Quoted 9 lines
include/linux/serial.h +2
-4@@ -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"....
include/linux/serial.h struct serial_icounter_struct { · line 221
Quoted 12 lines
include/linux/serial.h +6
-2+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,
Signature · 1 line
Nicolas Ferre
Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables
Quoted 57 lines · Nicolas Ferre
On 11/04/2011 09:19 AM, Claudio Scordino :
Quoted 10 lines
Hi Alan, Hi Greg,
it seems that the
crisv10.cand theatmel_serial.cserial
drivers interpret the fields of theserial_rs485structure in a different way.In particular, it seems that
crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor the_logicvalue_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
de6f86ce5that 16C950 may also use rs485 mode (with another signal that RTS BTW)...See comments online...
Quoted 22 lines
This patch makes the usage of these variables consistent across all drivers and fixes the Documentation as well.
In particular,SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to set the logic value of the RTS pin (as in thecrisv10.cdriver); the delay is understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Best regards,
Claudio
Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <[email protected]>The
crisv10.cand theatmel_serial.cserial drivers interpret the fields of theserial_rs485structure in a different way.
In particular,crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor 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.Quoted 4 lines
Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used toset the voltage of the RTS pin (as in the
crisv10.cdriver); the delay will be understood by looking only at the value ofdelay_rts_before_sendanddelay_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_SENDSER_RS485_RTS_VALUE_DURING_SEND(maybe too vague?)SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND(maybe too long?)
Quoted 7 lines
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
In reply to Nicolas FerreIl 08/11/2011 10:30, Nicolas Ferre ha scritto:
Quoted 57 lines
On 11/04/2011 09:19 AM, Claudio Scordino :
Quoted 10 lines
Hi Alan, Hi Greg,
it seems that the
crisv10.cand theatmel_serial.cserial
drivers interpret the fields of theserial_rs485structure in a different way.In particular, it seems that
crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor the_logicvalue_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
de6f86ce5that 16C950 may also use rs485 mode (with another signal that RTS BTW)...See comments online...
Quoted 22 lines
This patch makes the usage of these variables consistent across all drivers and fixes the Documentation as well.
In particular,SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to set the logic value of the RTS pin (as in thecrisv10.cdriver); the delay is understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Best regards,
Claudio
Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino<[email protected]>The
crisv10.cand theatmel_serial.cserial drivers interpret the fields of theserial_rs485structure in a different way.
In particular,crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor 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.Quoted 4 lines
Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used toset the voltage of the RTS pin (as in the
crisv10.cdriver); the delay will be understood by looking only at the value ofdelay_rts_before_sendanddelay_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_SENDSER_RS485_RTS_VALUE_DURING_SEND(maybe too vague?)SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND(maybe too long?)
Quoted 7 lines
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
Quoted 8 lines
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
Quoted 9 lines · Alan Cox
Quoted 8 lines
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.
Quoted 6 lines
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
Claudio Scordino
Acked-by Jesper NilssonRe: [PATCH] RS485: fix inconsistencies in the meaning of some variables
Il 08/11/2011 15:24, Greg KH ha scritto:
Quoted 10 lines · Alan Cox
Quoted 9 lines
Quoted 8 lines
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.
Quoted 8 lines
Quoted 6 lines
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 <[email protected]>
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.
Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill 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.
--- 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(-)
Documentation/serial/serial-rs485.txt +11 -3
@@ -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 */
drivers/tty/serial/atmel_serial.c +3 -13
@@ -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 ;
drivers/tty/serial/crisv10.c +2 -8
@@ -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
include/linux/serial.h +8 -6
@@ -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 .. */ };
Signature · 1 line
1.7.1
Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables
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:
Quoted 4 lines
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to set the voltage of the RTS pin (as in thecrisv10.cdriver); the delay will be understood by looking only at the value ofdelay_rts_before_sendanddelay_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?)
Quoted 8 lines
include/linux/serial.h +0
-0@@ -207,13 +207,15 @@ struct serial_icounter_struct {struct serial_rs485 {__u32 flags;/* RS485 feature flags */
include/linux/serial.h struct serial_icounter_struct { · line 215
Quoted 10 lines
include/linux/serial.h +6
-4-#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
Signature · 9 lines
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
Quoted 6 lines · 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:
Quoted 8 lines
Quoted 4 lines
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to
set the voltage of the RTS pin (as in thecrisv10.cdriver); the delay will be understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Do I overlook something or is
SER_RS485_RTS_AFTER_SENDalways the inverted signal ofSER_RS485_RTS_ON_SEND. So why do we need both? (BTWSER_RS485_RTS_ON_SENDis 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.
Quoted 25 lines
Quoted 8 lines
include/linux/serial.h +0
-0@@ -207,13 +207,15 @@ struct serial_icounter_struct {struct serial_rs485 {__u32 flags;/* RS485 feature flags */ include/linux/serial.h struct serial_icounter_struct { · line 215
Quoted 10 lines
include/linux/serial.h +6
-4-#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
On 11/14/2011 01:37 AM, Darron Black :
Quoted 30 lines · Wolfram Sang
Quoted 14 lines
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:Quoted 6 lines
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill
be used to
set the voltage of the RTS pin (as in thecrisv10.cdriver); the delay will be
understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Quoted 6 lines
Do I overlook something or is
SER_RS485_RTS_AFTER_SENDalways the inverted
signal ofSER_RS485_RTS_ON_SEND. So why do we need both? (BTWSER_RS485_RTS_ON_SENDis 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.
Quoted 3 lines
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,
Quoted 28 lines
Quoted 26 lines
Quoted 8 lines
include/linux/serial.h +0
-0@@ -207,13 +207,15 @@ struct serial_icounter_struct {struct serial_rs485 {__u32 flags;/* RS485 feature flags */ include/linux/serial.h struct serial_icounter_struct { · line 215
Quoted 10 lines
include/linux/serial.h +6
-4-#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
Signature · 1 line
Nicolas Ferre
Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables
In reply to Darron BlackQuoted 4 lines
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
In reply to Wolfram SangIl 13/11/2011 22:53, Wolfram Sang ha scritto:
Quoted 11 lines
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:
Quoted 4 lines
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to
set the voltage of the RTS pin (as in thecrisv10.cdriver); the delay will be understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.
Quoted 4 lines
Do I overlook something or is
SER_RS485_RTS_AFTER_SENDalways the inverted signal ofSER_RS485_RTS_ON_SEND. So why do we need both? (BTWSER_RS485_RTS_ON_SENDis 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
Nicolas Ferre
Acked-byRe: [PATCH] RS485: fix inconsistencies in the meaning of some variables
In reply to Claudio ScordinoOn 11/09/2011 03:51 PM, Claudio Scordino :
Quoted 39 lines
Il 08/11/2011 15:24, Greg KH ha scritto:
Quoted 10 lines · Alan Cox
Quoted 9 lines
Quoted 8 lines
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.
Quoted 8 lines
Quoted 6 lines
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 <[email protected]>The
crisv10.cand theatmel_serial.cserial drivers intepret the fields of theserial_rs485structure in a different way.
In particular,crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor 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.
Quoted 8 lines
Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used toset the voltage of the RTS pin (as in the
crisv10.cdriver); the delay will be understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Signed-off-by Claudio Scordino <[email protected]>Signed-off-by Darron Black <[email protected]>Acked-by Jesper Nilsson <[email protected]>
Surely too late, but I add it for the record:
Quoted 147 lines
--- 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(-)Documentation/serial/serial-rs485.txt +11
-3@@ -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 */drivers/tty/serial/atmel_serial.c +3
-13@@ -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 ;drivers/tty/serial/crisv10.c +2
-8@@ -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 include/linux/serial.h +8
-6@@ -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 .. */ };
Signature · 1 line
Nicolas Ferre
Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables
In reply to Claudio ScordinoOn 11/08/2011 11:48 AM, Claudio Scordino :
Quoted 82 lines
Il 08/11/2011 10:30, Nicolas Ferre ha scritto:
Quoted 63 lines
On 11/04/2011 09:19 AM, Claudio Scordino :
Quoted 11 lines
Hi Alan, Hi Greg,
it seems that the
crisv10.cand theatmel_serial.cserial
drivers interpret the fields of theserial_rs485structure in a different
way.In particular, it seems that
crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor the_logicvalue_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
de6f86ce5that 16C950 may also use rs485 mode (with another signal that RTS BTW)...See comments online...
Quoted 25 lines
This patch makes the usage of these variables consistent across all drivers and fixes the Documentation as well.
In particular,SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to set the logic value of the RTS pin (as in thecrisv10.cdriver); the delay is understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Best regards,
Claudio
Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino<[email protected]>The
crisv10.cand theatmel_serial.cserial drivers interpret the fields of theserial_rs485structure in a different way.
In particular,crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor 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.Quoted 6 lines
Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to
set the voltage of the RTS pin (as in thecrisv10.cdriver); the delay will be
understood by looking only at the value ofdelay_rts_before_sendanddelay_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_SENDSER_RS485_RTS_VALUE_DURING_SEND(maybe too vague?)SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND(maybe too long?)Quoted 7 lines
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).
Quoted 7 lines
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.
Quoted 2 lines
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.
Quoted 1 line
If you want, I can re-format the patch according to you suggestions,
Yes
Quoted 1 line
remove formatted lines
Yes
Quoted 1 line
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.
Quoted 1 line
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,
Signature · 1 line
Nicolas Ferre
Re: [PATCH] RS485: fix inconsistencies in the meaning of some variables
Il 08/11/2011 16:02, Nicolas Ferre ha scritto:
Quoted 102 lines
On 11/08/2011 11:48 AM, Claudio Scordino :
Quoted 82 lines
Il 08/11/2011 10:30, Nicolas Ferre ha scritto:
Quoted 63 lines
On 11/04/2011 09:19 AM, Claudio Scordino :
Quoted 11 lines
Hi Alan, Hi Greg,
it seems that the
crisv10.cand theatmel_serial.cserial
drivers interpret the fields of theserial_rs485structure in a different
way.In particular, it seems that
crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor the_logicvalue_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
de6f86ce5that 16C950 may also use rs485 mode (with another signal that RTS BTW)...See comments online...
Quoted 25 lines
This patch makes the usage of these variables consistent across all drivers and fixes the Documentation as well.
In particular,SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to set the logic value of the RTS pin (as in thecrisv10.cdriver); the delay is understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Best regards,
Claudio
Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino<[email protected]>The
crisv10.cand theatmel_serial.cserial drivers interpret the fields of theserial_rs485structure in a different way.
In particular,crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor 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.Quoted 6 lines
Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used to
set the voltage of the RTS pin (as in thecrisv10.cdriver); the delay will be
understood by looking only at the value ofdelay_rts_before_sendanddelay_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_SENDSER_RS485_RTS_VALUE_DURING_SEND(maybe too vague?)SER_RS485_RTS_LOGICAL_VALUE_DURING_SEND(maybe too long?)Quoted 7 lines
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).
Quoted 7 lines
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_SENDto 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.
Quoted 2 lines
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.
Quoted 7 lines
Quoted 1 line
If you want, I can re-format the patch according to you suggestions,
Yes
Quoted 1 line
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 <[email protected]>
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.
Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill 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.
--- 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(-)
Documentation/serial/serial-rs485.txt +11 -3
@@ -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 */
drivers/tty/serial/atmel_serial.c +3 -13
@@ -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 ;
drivers/tty/serial/crisv10.c +2 -8
@@ -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
include/linux/serial.h +8 -6
@@ -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 .. */ };
Signature · 1 line
1.7.1
Jesper Nilsson
Acked-byRe: [PATCH] RS485: fix inconsistencies in the meaning of some variables
Quoted 10 lines · Claudio Scordino
Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <[email protected]>The
crisv10.cand theatmel_serial.cserial drivers intepret the fields of theserial_rs485structure in a different way.
In particular,crisv10.cusesSER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDfor 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.
Quoted 7 lines
Quoted 1 line
From now on,
SER_RS485_RTS_AFTER_SENDandSER_RS485_RTS_ON_SENDwill be used toset the voltage of the RTS pin (as in the
crisv10.cdriver); the delay will be understood by looking only at the value ofdelay_rts_before_sendanddelay_rts_after_send.Signed-off-by Claudio Scordino <[email protected]>Signed-off-by Darron Black <[email protected]>
/^JN - Jesper Nilsson
Signature · 1 line
Jesper Nilsson -- jesper.nilsson at axis.com