From: Bernhard Roth <hidden> Date: 2011-08-15 14:37:55
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 <redacted>
Signed-off-by: Claudio Scordino <redacted>
---
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
On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
quoted hunk
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 <redacted>
Signed-off-by: Claudio Scordino <redacted>
---
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
On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
quoted
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<redacted>
Signed-off-by: Claudio Scordino<redacted>
---
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
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 <redacted>
Signed-off-by: Claudio Scordino <redacted>
---
drivers/tty/serial/atmel_serial.c | 17 ++++++++++-------
include/linux/serial.h | 1 +
2 files changed, 11 insertions(+), 7 deletions(-)
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.
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.
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 <redacted>
Signed-off-by: Claudio Scordino <redacted>
---
drivers/tty/serial/atmel_serial.c | 9 ++++++---
include/linux/serial.h | 1 +
2 files changed, 7 insertions(+), 3 deletions(-)
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 <redacted>
Signed-off-by: Claudio Scordino <redacted>
Acked-by: Alan Cox <redacted>
(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.
From: Russell King - ARM Linux <hidden> Date: 2011-08-23 10:24:55
On Tue, Aug 23, 2011 at 11:14:38AM +0100, Alan Cox wrote:
quoted
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 <redacted>
Signed-off-by: Claudio Scordino <redacted>
Acked-by: Alan Cox <redacted>
(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.
On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
Il 22/08/2011 23:18, Greg KH ha scritto:
quoted
On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
quoted
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<redacted>
Signed-off-by: Claudio Scordino<redacted>
---
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
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 <redacted>
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
On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
quoted
Il 22/08/2011 23:18, Greg KH ha scritto:
quoted
On Mon, Aug 15, 2011 at 04:28:15PM +0200, Bernhard Roth wrote:
quoted
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<redacted>
Signed-off-by: Claudio Scordino<redacted>
---
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
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<redacted>
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 <redacted>
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 <redacted>
Signed-off-by: Claudio Scordino <redacted>
Acked-by: Alan Cox <redacted>
---
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(-)
@@ -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. */ }
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 <redacted>
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 <redacted>
Signed-off-by: Darron Black <redacted>
---
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(-)
@@ -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 */
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.
Subject: RS485: fix inconsistencies in the meaning of some variables
From: Claudio Scordino <redacted>
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.
quoted
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 <redacted>
Signed-off-by: Darron Black <redacted>
---
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(-)
@@ -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 */
From: Nicolas Ferre <hidden> Date: 2011-11-08 09:31:52
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 <redacted>
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.
quoted
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.
@@ -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 */
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;
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...
quoted
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<redacted>
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.
quoted
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
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
On Tue, Nov 08, 2011 at 01:48:04PM +0000, Alan Cox wrote:
quoted
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
From: Nicolas Ferre <hidden> Date: 2011-11-08 15:03:26
On 11/08/2011 11:48 AM, Claudio Scordino :
Il 08/11/2011 10:30, Nicolas Ferre ha scritto:
quoted
On 11/04/2011 09:19 AM, Claudio Scordino :
quoted
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...
quoted
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<redacted>
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.
quoted
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
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...
quoted
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<redacted>
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.
quoted
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.
quoted
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
If you want, I can re-format the patch according to you suggestions,
Yes
quoted
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 <redacted>
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 <redacted>
Signed-off-by: Darron Black <redacted>
---
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(-)
@@ -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 */
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...
quoted
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 <redacted>
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.
quoted
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.
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 <redacted>
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
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 <redacted>
Signed-off-by: Darron Black <redacted>
On Tue, Nov 08, 2011 at 01:48:04PM +0000, Alan Cox wrote:
quoted
quoted
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 <redacted>
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 <redacted>
Signed-off-by: Darron Black <redacted>
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(-)
@@ -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 */
From: Wolfram Sang <hidden> Date: 2011-11-13 21:53:35
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?)
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/ |
From: Darron Black <hidden> Date: 2011-11-14 01:14:25
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:
quoted
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.
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
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
From: Nicolas Ferre <hidden> Date: 2011-11-14 11:13:00
On 11/14/2011 01:37 AM, Darron Black :
On 11/13/2011 03:53 PM, Wolfram Sang wrote:
quoted
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
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,
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
From: Nicolas Ferre <hidden> Date: 2011-11-14 12:19:41
On 11/09/2011 03:51 PM, Claudio Scordino :
Il 08/11/2011 15:24, Greg KH ha scritto:
quoted
On Tue, Nov 08, 2011 at 01:48:04PM +0000, Alan Cox wrote:
quoted
quoted
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 <redacted>
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
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 <redacted>
Signed-off-by: Darron Black <redacted>
Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>
Surely too late, but I add it for the record:
Acked-by: Nicolas Ferre <redacted>
@@ -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 */