linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] SIIG 8-port serial boards support
@ 2006-01-24  8:25 Andrey Panin
  2006-01-24 21:01 ` Russell King
  0 siblings, 1 reply; 17+ messages in thread
From: Andrey Panin @ 2006-01-24  8:25 UTC (permalink / raw)
  To: Russell King; +Cc: linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 236 bytes --]

Hello,

This patch (against 2.6.16-rc1) adds support for SIIG 8 port serial boards.
Please consider applying.

Best regards.

-- 
Andrey Panin		| Linux and UNIX system administrator
pazke@donpac.ru		| PGP key: wwwkeys.pgp.net

[-- Attachment #1.2: patch-siig-8port-board --]
[-- Type: text/plain, Size: 2583 bytes --]


 drivers/serial/8250_pci.c |   25 ++++++++++++++++++++++++-
 include/linux/pci_ids.h   |    3 +++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff -urdpNX /usr/share/dontdiff linux-2.6.16-rc1.vanilla/drivers/serial/8250_pci.c linux-2.6.16-rc1/drivers/serial/8250_pci.c
--- linux-2.6.16-rc1.vanilla/drivers/serial/8250_pci.c	2006-01-24 10:14:24.000000000 +0300
+++ linux-2.6.16-rc1/drivers/serial/8250_pci.c	2006-01-24 10:16:56.000000000 +0300
@@ -439,6 +439,20 @@ static int pci_siig_init(struct pci_dev 
 	return -ENODEV;
 }
 
+static int pci_siig_setup(struct serial_private *priv,
+			  struct pciserial_board *board,
+			  struct uart_port *port, int idx)
+{
+	unsigned int bar = FL_GET_BASE(board->flags) + idx, offset = 0;
+
+	if (idx > 3) {
+		bar = 4;
+		offset = (idx - 4) * 8;
+	}
+
+	return setup_port(priv, port, bar, offset, 0);
+}
+
 /*
  * Timedia has an explosion of boards, and to avoid the PCI table from
  * growing *huge*, we use this function to collapse some 70 entries
@@ -748,7 +762,7 @@ static struct pci_serial_quirk pci_seria
 		.subvendor	= PCI_ANY_ID,
 		.subdevice	= PCI_ANY_ID,
 		.init		= pci_siig_init,
-		.setup		= pci_default_setup,
+		.setup		= pci_siig_setup,
 	},
 	/*
 	 * Titan cards
@@ -2134,6 +2148,15 @@ static struct pci_device_id serial_pci_t
 	{	PCI_VENDOR_ID_SIIG, PCI_DEVICE_ID_SIIG_4S_20x_850,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 		pbn_b0_bt_4_921600 },
+	{	PCI_VENDOR_ID_SIIG, PCI_DEVICE_ID_SIIG_8S_20x_550,
+		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+		pbn_b0_bt_8_921600 },
+	{	PCI_VENDOR_ID_SIIG, PCI_DEVICE_ID_SIIG_8S_20x_650,
+		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+		pbn_b0_bt_8_921600 },
+	{	PCI_VENDOR_ID_SIIG, PCI_DEVICE_ID_SIIG_8S_20x_850,
+		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+		pbn_b0_bt_8_921600 },
 
 	/*
 	 * Computone devices submitted by Doug McNash dmcnash@computone.com
diff -urdpNX /usr/share/dontdiff linux-2.6.16-rc1.vanilla/include/linux/pci_ids.h linux-2.6.16-rc1/include/linux/pci_ids.h
--- linux-2.6.16-rc1.vanilla/include/linux/pci_ids.h	2006-01-24 10:14:51.000000000 +0300
+++ linux-2.6.16-rc1/include/linux/pci_ids.h	2006-01-24 10:16:56.000000000 +0300
@@ -1677,6 +1677,9 @@
 #define PCI_DEVICE_ID_SIIG_2S1P_20x_550	0x2060
 #define PCI_DEVICE_ID_SIIG_2S1P_20x_650	0x2061
 #define PCI_DEVICE_ID_SIIG_2S1P_20x_850	0x2062
+#define PCI_DEVICE_ID_SIIG_8S_20x_550	0x2080
+#define PCI_DEVICE_ID_SIIG_8S_20x_650	0x2081
+#define PCI_DEVICE_ID_SIIG_8S_20x_850	0x2082
 #define PCI_SUBDEVICE_ID_SIIG_QUARTET_SERIAL	0x2050
 
 #define PCI_VENDOR_ID_RADISYS		0x1331

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-01-24  8:25 [PATCH] SIIG 8-port serial boards support Andrey Panin
@ 2006-01-24 21:01 ` Russell King
  2006-02-02 10:26   ` Russell King
  0 siblings, 1 reply; 17+ messages in thread
From: Russell King @ 2006-01-24 21:01 UTC (permalink / raw)
  To: linux-kernel

On Tue, Jan 24, 2006 at 11:25:38AM +0300, Andrey Panin wrote:
> This patch (against 2.6.16-rc1) adds support for SIIG 8 port serial boards.
> Please consider applying.

Could you supply a sign-off (and a description) for this patch please -
see Documentation/SubmittingPatches for more information.

Thanks.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-01-24 21:01 ` Russell King
@ 2006-02-02 10:26   ` Russell King
  2006-02-02 13:27     ` Andrey Panin
  0 siblings, 1 reply; 17+ messages in thread
From: Russell King @ 2006-02-02 10:26 UTC (permalink / raw)
  To: linux-kernel, Andrey Panin

On Tue, Jan 24, 2006 at 09:01:41PM +0000, Russell King wrote:
> On Tue, Jan 24, 2006 at 11:25:38AM +0300, Andrey Panin wrote:
> > This patch (against 2.6.16-rc1) adds support for SIIG 8 port serial boards.
> > Please consider applying.
> 
> Could you supply a sign-off (and a description) for this patch please -
> see Documentation/SubmittingPatches for more information.

Ping?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-02 10:26   ` Russell King
@ 2006-02-02 13:27     ` Andrey Panin
       [not found]       ` <20060202201734.GA17329@flint.arm.linux.org.uk>
  0 siblings, 1 reply; 17+ messages in thread
From: Andrey Panin @ 2006-02-02 13:27 UTC (permalink / raw)
  To: Russell King; +Cc: linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1143 bytes --]

n 033, 02 02, 2006 at 10:26:44AM +0000, Russell King wrote:
> On Tue, Jan 24, 2006 at 09:01:41PM +0000, Russell King wrote:
> > On Tue, Jan 24, 2006 at 11:25:38AM +0300, Andrey Panin wrote:
> > > This patch (against 2.6.16-rc1) adds support for SIIG 8 port serial boards.
> > > Please consider applying.
> > 
> > Could you supply a sign-off (and a description) for this patch please -
> > see Documentation/SubmittingPatches for more information.
> 
> Ping?

Pong :) Please see below. Does it look better now ?


BTW I have a question for you. I'm trying to add support for Advantech serial
cards into 8250_pci.c. These cards are based on Oxford Semiconductor 16C950
UARTs and some of then can work in RS485 mode. They use DTR to automatically
control direction of the RS485 transiever buffer and so need to set bits
3 and 4 in the ACR register. Unfortunately there is currently no way to set
different ACR value. Is it okay to use unused[] array in the struct uart_port
to pass ACR value from 8250_pci.c to 8250.c ?

-- 
Andrey Panin		| Linux and UNIX system administrator
pazke@donpac.ru		| PGP key: wwwkeys.pgp.net

[-- Attachment #1.2: patch-siig-8port-board.diff --]
[-- Type: text/plain, Size: 2957 bytes --]


This patch adds support for SIIG 8-port boards. These boards have 4 ports in
separate bars and another 4 ports in the single bar. Because of this strange
port arrangement these cards need special setup function. Fortunately no other
SIIG cards have more than 4 port, so this setup function could be used for them
too.

Signed-off-by: Andrey Panin <pazke@donpac.ru>

 drivers/serial/8250_pci.c |   25 ++++++++++++++++++++++++-
 include/linux/pci_ids.h   |    3 +++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff -urdpNX /usr/share/dontdiff linux-2.6.16-rc1.vanilla/drivers/serial/8250_pci.c linux-2.6.16-rc1/drivers/serial/8250_pci.c
--- linux-2.6.16-rc1.vanilla/drivers/serial/8250_pci.c	2006-01-24 10:14:24.000000000 +0300
+++ linux-2.6.16-rc1/drivers/serial/8250_pci.c	2006-01-24 10:16:56.000000000 +0300
@@ -439,6 +439,20 @@ static int pci_siig_init(struct pci_dev 
 	return -ENODEV;
 }
 
+static int pci_siig_setup(struct serial_private *priv,
+			  struct pciserial_board *board,
+			  struct uart_port *port, int idx)
+{
+	unsigned int bar = FL_GET_BASE(board->flags) + idx, offset = 0;
+
+	if (idx > 3) {
+		bar = 4;
+		offset = (idx - 4) * 8;
+	}
+
+	return setup_port(priv, port, bar, offset, 0);
+}
+
 /*
  * Timedia has an explosion of boards, and to avoid the PCI table from
  * growing *huge*, we use this function to collapse some 70 entries
@@ -748,7 +762,7 @@ static struct pci_serial_quirk pci_seria
 		.subvendor	= PCI_ANY_ID,
 		.subdevice	= PCI_ANY_ID,
 		.init		= pci_siig_init,
-		.setup		= pci_default_setup,
+		.setup		= pci_siig_setup,
 	},
 	/*
 	 * Titan cards
@@ -2134,6 +2148,15 @@ static struct pci_device_id serial_pci_t
 	{	PCI_VENDOR_ID_SIIG, PCI_DEVICE_ID_SIIG_4S_20x_850,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 		pbn_b0_bt_4_921600 },
+	{	PCI_VENDOR_ID_SIIG, PCI_DEVICE_ID_SIIG_8S_20x_550,
+		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+		pbn_b0_bt_8_921600 },
+	{	PCI_VENDOR_ID_SIIG, PCI_DEVICE_ID_SIIG_8S_20x_650,
+		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+		pbn_b0_bt_8_921600 },
+	{	PCI_VENDOR_ID_SIIG, PCI_DEVICE_ID_SIIG_8S_20x_850,
+		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+		pbn_b0_bt_8_921600 },
 
 	/*
 	 * Computone devices submitted by Doug McNash dmcnash@computone.com
diff -urdpNX /usr/share/dontdiff linux-2.6.16-rc1.vanilla/include/linux/pci_ids.h linux-2.6.16-rc1/include/linux/pci_ids.h
--- linux-2.6.16-rc1.vanilla/include/linux/pci_ids.h	2006-01-24 10:14:51.000000000 +0300
+++ linux-2.6.16-rc1/include/linux/pci_ids.h	2006-01-24 10:16:56.000000000 +0300
@@ -1677,6 +1677,9 @@
 #define PCI_DEVICE_ID_SIIG_2S1P_20x_550	0x2060
 #define PCI_DEVICE_ID_SIIG_2S1P_20x_650	0x2061
 #define PCI_DEVICE_ID_SIIG_2S1P_20x_850	0x2062
+#define PCI_DEVICE_ID_SIIG_8S_20x_550	0x2080
+#define PCI_DEVICE_ID_SIIG_8S_20x_650	0x2081
+#define PCI_DEVICE_ID_SIIG_8S_20x_850	0x2082
 #define PCI_SUBDEVICE_ID_SIIG_QUARTET_SERIAL	0x2050
 
 #define PCI_VENDOR_ID_RADISYS		0x1331

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
       [not found]       ` <20060202201734.GA17329@flint.arm.linux.org.uk>
@ 2006-02-03  9:13         ` Andrey Panin
  2006-02-03  9:24           ` Russell King
  0 siblings, 1 reply; 17+ messages in thread
From: Andrey Panin @ 2006-02-03  9:13 UTC (permalink / raw)
  To: Russell King; +Cc: linux-kernel

On 033, 02 02, 2006 at 08:17:35 +0000, Russell King wrote:
> On Thu, Feb 02, 2006 at 04:27:26PM +0300, Andrey Panin wrote:
> > Pong :) Please see below. Does it look better now ?
> 
> Thanks, applied.
> 
> > BTW I have a question for you. I'm trying to add support for Advantech serial
> > cards into 8250_pci.c. These cards are based on Oxford Semiconductor 16C950
> > UARTs and some of then can work in RS485 mode. They use DTR to automatically
> > control direction of the RS485 transiever buffer and so need to set bits
> > 3 and 4 in the ACR register. Unfortunately there is currently no way to set
> > different ACR value. Is it okay to use unused[] array in the struct uart_port
> > to pass ACR value from 8250_pci.c to 8250.c ?
> 
> As I've said many a time, we need a generic way to set different hand-
> shaking modes.  I've suggested using some spare bits in termios in the
> past, but nothing ever came of that - folk lose interest at that point.

IMHO there is no need to userspace visible changes to support RS485 on these cards,
because some of them are RS485 only and some have jumpers for individual ports. 
There is nothing that userspace can configure. We only need to set two bits in ACR
according to card type and jumper settings and UART will drive RS485 transiever
transparently.

Actually any program which want to talk with RS485 network should implement some
form of line discipline itself to prevent network collisions and kernel cannot
help with it much.

> So as far as I'm concerned, until folk start wanting to seriously
> discuss how to portably support different flow control methods, there's
> no real support for them.

For RS232 cace I agree, but for RS485 this is not needed IMHO.

-- 
Andrey Panin		| Linux and UNIX system administrator
pazke@donpac.ru		| PGP key: wwwkeys.pgp.net

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-03  9:13         ` Andrey Panin
@ 2006-02-03  9:24           ` Russell King
  2006-02-17 11:39             ` Andrey Panin
  0 siblings, 1 reply; 17+ messages in thread
From: Russell King @ 2006-02-03  9:24 UTC (permalink / raw)
  To: linux-kernel

On Fri, Feb 03, 2006 at 12:13:08PM +0300, Andrey Panin wrote:
> On 033, 02 02, 2006 at 08:17:35 +0000, Russell King wrote:
> > As I've said many a time, we need a generic way to set different hand-
> > shaking modes.  I've suggested using some spare bits in termios in the
> > past, but nothing ever came of that - folk lose interest at that point.
> 
> IMHO there is no need to userspace visible changes to support RS485 on
> these cards, because some of them are RS485 only and some have jumpers
> for individual ports.  There is nothing that userspace can configure.
> We only need to set two bits in ACR according to card type and jumper
> settings and UART will drive RS485 transiever transparently.

In this particular case you may be right, but I'm looking at the bigger
picture, where plain 16550's may be used for RS485.

There are drivers which want to implement their own private ioctl to
enable RS485 mode.  What I'm saying is that we should have one solution,
not multiple solutions to this problem.  When we have such a solution,
your RS485 card will be able to fit into that model.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-03  9:24           ` Russell King
@ 2006-02-17 11:39             ` Andrey Panin
  2006-02-17 20:02               ` Russell King
  0 siblings, 1 reply; 17+ messages in thread
From: Andrey Panin @ 2006-02-17 11:39 UTC (permalink / raw)
  To: Russell King; +Cc: linux-kernel

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

On 034, 02 03, 2006 at 09:24:36 +0000, Russell King wrote:
> On Fri, Feb 03, 2006 at 12:13:08PM +0300, Andrey Panin wrote:
> > On 033, 02 02, 2006 at 08:17:35 +0000, Russell King wrote:
> > > As I've said many a time, we need a generic way to set different hand-
> > > shaking modes.  I've suggested using some spare bits in termios in the
> > > past, but nothing ever came of that - folk lose interest at that point.

No wonder they do. Extra bits are not a problem, but for 8250.c we need some
way to glue subdrivers with serial8250_set_termios(). Callback in uart_port
structure ?

> > IMHO there is no need to userspace visible changes to support RS485 on
> > these cards, because some of them are RS485 only and some have jumpers
> > for individual ports.  There is nothing that userspace can configure.
> > We only need to set two bits in ACR according to card type and jumper
> > settings and UART will drive RS485 transiever transparently.
> 
> In this particular case you may be right, but I'm looking at the bigger
> picture, where plain 16550's may be used for RS485.

Common way to use plain 16550 for RS485 is to wire transiever to the RTS
and force userspace to use RTS/CTS flow control. I doubt there are many 
other sane way to do it.

> There are drivers which want to implement their own private ioctl to
> enable RS485 mode.  What I'm saying is that we should have one solution,
> not multiple solutions to this problem. When we have such a solution,
> your RS485 card will be able to fit into that model.

But it will need a way to pass ACR value anyway :)

-- 
Andrey Panin		| Linux and UNIX system administrator
pazke@donpac.ru		| PGP key: wwwkeys.pgp.net

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-17 11:39             ` Andrey Panin
@ 2006-02-17 20:02               ` Russell King
  2006-02-17 20:14                 ` Russell King
  2006-02-17 21:27                 ` Paul Fulghum
  0 siblings, 2 replies; 17+ messages in thread
From: Russell King @ 2006-02-17 20:02 UTC (permalink / raw)
  To: linux-kernel

On Fri, Feb 17, 2006 at 02:39:42PM +0300, Andrey Panin wrote:
> On 034, 02 03, 2006 at 09:24:36 +0000, Russell King wrote:
> > On Fri, Feb 03, 2006 at 12:13:08PM +0300, Andrey Panin wrote:
> > > On 033, 02 02, 2006 at 08:17:35 +0000, Russell King wrote:
> > > > As I've said many a time, we need a generic way to set different hand-
> > > > shaking modes.  I've suggested using some spare bits in termios in the
> > > > past, but nothing ever came of that - folk lose interest at that point.
> 
> No wonder they do. Extra bits are not a problem, but for 8250.c we need some
> way to glue subdrivers with serial8250_set_termios(). Callback in uart_port
> structure ?

They lose interest because they want to solve only their own small
little problem without looking at the bigger picture.  That's not
what I'm interested in, so as far as I'm concerned (and I hope this
is clear) I have _zero_ interest in solving their small little
problems.

By only solving the small little problem, we end up with lots of
drivers doing their own stupid implementation of the same feature,
which results in multiple differing ways to enable said same
feature from userspace.

Plus, there is more to handshaking than just the standard protocol
we implement today - yes there's RS485-using-RTS flavour, but there's
also an alternative interpretation of RTS to mean "I am requesting
to send something" rather than the conventional "it is okay for you
to send me something".

And yes, someone has already requested this alternative RTS
interpretation.

So, there are three distinct flow control scenarios:

- conventional RTS/CTS
- alternative RTS/CTS
- RS485

As I've said above, if folk wish to bury their heads and just address
RS485 in isolation, I'm just plain not interested.  Let's do the job
properly or not at all.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-17 20:02               ` Russell King
@ 2006-02-17 20:14                 ` Russell King
  2006-02-17 21:27                 ` Paul Fulghum
  1 sibling, 0 replies; 17+ messages in thread
From: Russell King @ 2006-02-17 20:14 UTC (permalink / raw)
  To: linux-kernel

On Fri, Feb 17, 2006 at 08:02:13PM +0000, Russell King wrote:
> On Fri, Feb 17, 2006 at 02:39:42PM +0300, Andrey Panin wrote:
> > On 034, 02 03, 2006 at 09:24:36 +0000, Russell King wrote:
> > > On Fri, Feb 03, 2006 at 12:13:08PM +0300, Andrey Panin wrote:
> > > > On 033, 02 02, 2006 at 08:17:35 +0000, Russell King wrote:
> > > > > As I've said many a time, we need a generic way to set different hand-
> > > > > shaking modes.  I've suggested using some spare bits in termios in the
> > > > > past, but nothing ever came of that - folk lose interest at that point.
> > 
> > No wonder they do. Extra bits are not a problem, but for 8250.c we need some
> > way to glue subdrivers with serial8250_set_termios(). Callback in uart_port
> > structure ?

Finally, let me explain why I favour the termios solution.  The biggest
(and most important) aspect is that it allows existing applications
such as minicom and gettys to work as expected - getting the correct
handshaking mode that they desire without having to change userspace.

I think that is an _extremely_ important property to have.  Without it,
you would end up with (eg) a security issue where a user changes the
handshaking mode of the dial-up connection they're on, rendering it
useless until the sysadmin "undoes" the damage.

Alternatively, every serial line getty at least needs to be taught
how to reset the handshaking mode, which at present means that it
needs to be taught about the hardware cards so it knows what silly
random ioctl call for some proprietary method to make.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-17 20:02               ` Russell King
  2006-02-17 20:14                 ` Russell King
@ 2006-02-17 21:27                 ` Paul Fulghum
  2006-02-17 21:39                   ` Russell King
  1 sibling, 1 reply; 17+ messages in thread
From: Paul Fulghum @ 2006-02-17 21:27 UTC (permalink / raw)
  To: Russell King; +Cc: linux-kernel

Russell King wrote:
> So, there are three distinct flow control scenarios:

So I'm clear on how you interpret these,
am I correct with the following?

> - conventional RTS/CTS
RTS active = ready to receive
CTS active = allowed to send

> - alternative RTS/CTS
RTS active = on before send, off after send
CTS active = allowed to send

> - RS485
RTS active = on before send, off after send (RTS enables driver)
CTS ignored (2 wire mode, no CTS)

So maybe the extra control fields would be:
CRTSONTX - RTS on before send, off after send
CTXONCTS - wait for CTS before sending

-- 
Paul Fulghum
Microgate Systems, Ltd.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-17 21:27                 ` Paul Fulghum
@ 2006-02-17 21:39                   ` Russell King
  2006-02-17 21:52                     ` Paul Fulghum
  0 siblings, 1 reply; 17+ messages in thread
From: Russell King @ 2006-02-17 21:39 UTC (permalink / raw)
  To: Paul Fulghum; +Cc: linux-kernel

On Fri, Feb 17, 2006 at 03:27:44PM -0600, Paul Fulghum wrote:
> Russell King wrote:
> >So, there are three distinct flow control scenarios:
> 
> So I'm clear on how you interpret these,
> am I correct with the following?
> 
> >- conventional RTS/CTS
> RTS active = ready to receive
> CTS active = allowed to send
> 
> >- alternative RTS/CTS
> RTS active = on before send, off after send
> CTS active = allowed to send

I'll have to dig through my archives to confirm this one.

> >- RS485
> RTS active = on before send, off after send (RTS enables driver)
> CTS ignored (2 wire mode, no CTS)
> 
> So maybe the extra control fields would be:
> CRTSONTX - RTS on before send, off after send
> CTXONCTS - wait for CTS before sending

That's a possibility, except that programs today expect CRTSCTS to
enable RTS/CTS flow control.

What I suggest is to use CRTSCTS to enable the chosen flow control
method.  Then we have a set of cflag bits which describe the flow
control mode, eg CFLOWRS485, CFLOWMODEM, CFLOWALT (probably needs
better names.)  CFLOWMODEM being the conventional mode should have
an all zeros value.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-17 21:39                   ` Russell King
@ 2006-02-17 21:52                     ` Paul Fulghum
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Fulghum @ 2006-02-17 21:52 UTC (permalink / raw)
  To: Russell King; +Cc: linux-kernel

Russell King wrote:
> What I suggest is to use CRTSCTS to enable the chosen flow control
> method.  Then we have a set of cflag bits which describe the flow
> control mode, eg CFLOWRS485, CFLOWMODEM, CFLOWALT (probably needs
> better names.)  CFLOWMODEM being the conventional mode should have
> an all zeros value.

That is better, as it enforces mutual exclusion of the different modes.

Thanks,
Paul

-- 
Paul Fulghum
Microgate Systems, Ltd.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-17 22:39 ` Russell King
@ 2006-02-17 23:11   ` Paul Fulghum
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Fulghum @ 2006-02-17 23:11 UTC (permalink / raw)
  To: Russell King; +Cc: linux, linux-kernel

Russell King wrote:
> On Fri, Feb 17, 2006 at 05:25:29PM -0500, linux@horizon.com wrote:
>>CRTSCTS	CRTSHDX	Handshaking
>>off	off	None.  (Computer might as well send RTS< but ignores CTS)
>>on	off	Full-duplex RTS/CTS
>>off	on	RS-485.  CTS ignored, RTS enables transmitter.
>>on	on	RS-232 half-duplex.  RTS is request, CTS is grant.
...
> Also, !CRTSCTS is most likely the state used by any existing userspace
> RS485 implementations which would be using TIOCMBIC/TIOCMBIS to
> manipulate the RTS signal, so having RTS manipulated in this state
> would be an undesirable change of behaviour.
> 
> Hence, I'm very much in favour of having the default flow control
> method to preserve in as many ways as possible existing behaviour
> for CRTSCTS.

It is important to maintain the "driver doesn't touch RTS/CTS"
semantics without regard to other (new) control flags.
An application might read the existing termios, and modify only
the bits it is aware of without verifying that new bits are zero.
CFLOWXXX also maintains a free setting for future flow modes, such as:
CFLOWZEN = alter RTS based on /dev/random

--
Paul Fulghum
Microgate Systems, Ltd

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-17 22:25 linux
@ 2006-02-17 22:39 ` Russell King
  2006-02-17 23:11   ` Paul Fulghum
  0 siblings, 1 reply; 17+ messages in thread
From: Russell King @ 2006-02-17 22:39 UTC (permalink / raw)
  To: linux; +Cc: paulkf, linux-kernel

On Fri, Feb 17, 2006 at 05:25:29PM -0500, linux@horizon.com wrote:
> >> - conventional RTS/CTS
> > RTS active = ready to receive
> > CTS active = allowed to send
> > 
> >> - alternative RTS/CTS
> > RTS active = on before send, off after send
> > CTS active = allowed to send
> > 
> >> - RS485
> > RTS active = on before send, off after send (RTS enables driver)
> > CTS ignored (2 wire mode, no CTS)
> > 
> > So maybe the extra control fields would be:
> > CRTSONTX - RTS on before send, off after send
> > CTXONCTS - wait for CTS before sending
> 
> As someone who's actually used both kinds of modems, here are
> the issues:
> 
> - For RS-485, you have a half-duplex wire, and the response is triggered
>   by the serial data.  And some of those industrial controllers respond in
>   less than a character time.  You MUST disable the transmitter promptly
>   when finished sending so you can hear the response.
> 
>   It's also helpful if the receiver is disabled while the transmitter
>   is enabled, but that's negotible.
> 
> - For Classic half-duplex RTS/CTS, the DTE (computer) must always accept
>   data, but raises RTS when it wants to send.  When it gets CTS, it's
>   allowed to actually send.  There are still single-frequency VHF radio
>   modems floating around that work this way.  When a modem receives
>   RTS and is not receiving a carrier (CD is deasserted), it enables its
>   transmitter, and waits a programmed receiver-acquisition delay before
>   asserting CTS.
> 
> Both of these are variants on the same theme, and I'd suggest expressing
> them with one additional bit along with the existing CRTSCTS.  I'll call
> it CRTSHDX (RTS half-duplex).  It means "assert RTS when we have data
> to send, and deassert it when we don't".  When it's not set, RTS is
> asserted when we can accept data and deasserted when we can't.
> 
> All four combinations are sensible:
> 
> CRTSCTS	CRTSHDX	Handshaking
> off	off	None.  (Computer might as well send RTS< but ignores CTS)
> on	off	Full-duplex RTS/CTS
> off	on	RS-485.  CTS ignored, RTS enables transmitter.
> on	on	RS-232 half-duplex.  RTS is request, CTS is grant.
> 
> The upshot is that CRTSCTS controls whether CTS is listened to, and the
> new CRTSHDX controls the interpretation of RTS.  For a three-wire hookup,
> CRTSRCS must be off and CRTSHDX has no real effect.

We need to preserve CRTSCTS off = no modem control activity - I'm
sure things like old serial mice do not take kindly to having the
modem control lines changing state, especially the ones they use to
power themselves.

Also, !CRTSCTS is most likely the state used by any existing userspace
RS485 implementations which would be using TIOCMBIC/TIOCMBIS to
manipulate the RTS signal, so having RTS manipulated in this state
would be an undesirable change of behaviour.

Hence, I'm very much in favour of having the default flow control
method to preserve in as many ways as possible existing behaviour
for CRTSCTS.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
@ 2006-02-17 22:25 linux
  2006-02-17 22:39 ` Russell King
  0 siblings, 1 reply; 17+ messages in thread
From: linux @ 2006-02-17 22:25 UTC (permalink / raw)
  To: paulkf, rmk+lkml; +Cc: linux-kernel

>> - conventional RTS/CTS
> RTS active = ready to receive
> CTS active = allowed to send
> 
>> - alternative RTS/CTS
> RTS active = on before send, off after send
> CTS active = allowed to send
> 
>> - RS485
> RTS active = on before send, off after send (RTS enables driver)
> CTS ignored (2 wire mode, no CTS)
> 
> So maybe the extra control fields would be:
> CRTSONTX - RTS on before send, off after send
> CTXONCTS - wait for CTS before sending

As someone who's actually used both kinds of modems, here are
the issues:

- For RS-485, you have a half-duplex wire, and the response is triggered
  by the serial data.  And some of those industrial controllers respond in
  less than a character time.  You MUST disable the transmitter promptly
  when finished sending so you can hear the response.

  It's also helpful if the receiver is disabled while the transmitter
  is enabled, but that's negotible.

- For Classic half-duplex RTS/CTS, the DTE (computer) must always accept
  data, but raises RTS when it wants to send.  When it gets CTS, it's
  allowed to actually send.  There are still single-frequency VHF radio
  modems floating around that work this way.  When a modem receives
  RTS and is not receiving a carrier (CD is deasserted), it enables its
  transmitter, and waits a programmed receiver-acquisition delay before
  asserting CTS.

Both of these are variants on the same theme, and I'd suggest expressing
them with one additional bit along with the existing CRTSCTS.  I'll call
it CRTSHDX (RTS half-duplex).  It means "assert RTS when we have data
to send, and deassert it when we don't".  When it's not set, RTS is
asserted when we can accept data and deasserted when we can't.

All four combinations are sensible:

CRTSCTS	CRTSHDX	Handshaking
off	off	None.  (Computer might as well send RTS< but ignores CTS)
on	off	Full-duplex RTS/CTS
off	on	RS-485.  CTS ignored, RTS enables transmitter.
on	on	RS-232 half-duplex.  RTS is request, CTS is grant.

The upshot is that CRTSCTS controls whether CTS is listened to, and the
new CRTSHDX controls the interpretation of RTS.  For a three-wire hookup,
CRTSRCS must be off and CRTSHDX has no real effect.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
  2006-02-17 20:32 Kilau, Scott
@ 2006-02-17 21:26 ` Russell King
  0 siblings, 0 replies; 17+ messages in thread
From: Russell King @ 2006-02-17 21:26 UTC (permalink / raw)
  To: Kilau, Scott; +Cc: linux-kernel

On Fri, Feb 17, 2006 at 02:32:03PM -0600, Kilau, Scott wrote:
> Hi everyone,
> (Sorry for the ugly copy/paste here, grabbing from a web browser to
> email)
> 
> On Fri, Feb 17, 2006 at 08:02:13PM +0000, Russell King wrote:
> > Finally, let me explain why I favour the termios solution.  The
> biggest
> > (and most important) aspect is that it allows existing applications
> > such as minicom and gettys to work as expected - getting the correct
> > handshaking mode that they desire without having to change userspace.
> 
> What about creating a "struct termiox".
> Yeah, it creates a new ioctl, but it is a pretty standard
> ioctl among Unix's.
> 
> I know adding termiox calls has been brought up before in
> the past, and of course, nothing ever gets added...

That still requires getty's and minicom etc to be modified, and as
I point out in my follow up mail, not having getty understand it
can be a security issue.

Since we do have spare bits in cflag, I see no reason not to use
them.  If we use these spare bits, we stand a good chance that we'll
have the desired behaviour without modifying userland.

I've seen the occasional alternative suggestion, but no one has yet
put forward a coherent argument against using termios's cflags to
control the handshake mode.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] SIIG 8-port serial boards support
@ 2006-02-17 20:32 Kilau, Scott
  2006-02-17 21:26 ` Russell King
  0 siblings, 1 reply; 17+ messages in thread
From: Kilau, Scott @ 2006-02-17 20:32 UTC (permalink / raw)
  To: linux-kernel; +Cc: rmk+lkml

Hi everyone,
(Sorry for the ugly copy/paste here, grabbing from a web browser to
email)

On Fri, Feb 17, 2006 at 08:02:13PM +0000, Russell King wrote:
> Finally, let me explain why I favour the termios solution.  The
biggest
> (and most important) aspect is that it allows existing applications
> such as minicom and gettys to work as expected - getting the correct
> handshaking mode that they desire without having to change userspace.

What about creating a "struct termiox".
Yeah, it creates a new ioctl, but it is a pretty standard
ioctl among Unix's.

I know adding termiox calls has been brought up before in
the past, and of course, nothing ever gets added...

Scott




^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2006-02-17 23:12 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-24  8:25 [PATCH] SIIG 8-port serial boards support Andrey Panin
2006-01-24 21:01 ` Russell King
2006-02-02 10:26   ` Russell King
2006-02-02 13:27     ` Andrey Panin
     [not found]       ` <20060202201734.GA17329@flint.arm.linux.org.uk>
2006-02-03  9:13         ` Andrey Panin
2006-02-03  9:24           ` Russell King
2006-02-17 11:39             ` Andrey Panin
2006-02-17 20:02               ` Russell King
2006-02-17 20:14                 ` Russell King
2006-02-17 21:27                 ` Paul Fulghum
2006-02-17 21:39                   ` Russell King
2006-02-17 21:52                     ` Paul Fulghum
2006-02-17 20:32 Kilau, Scott
2006-02-17 21:26 ` Russell King
2006-02-17 22:25 linux
2006-02-17 22:39 ` Russell King
2006-02-17 23:11   ` Paul Fulghum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).