[PATCH v1] 8250: add support for DTR/DSR hardware flow control

[PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Aristeu Rozanski]

This patch adds support for DTR/DSR hardware flow control on 8250 driver on x86
machines. It's done by adding a CDTRDSR flag to work just like CRTSCTS, which is
not done on other architectures on purpose (so each maintainer can allocate it).

This patch was tested with success with a serial printer configured with a small
buffer and DTR/DSR flow control.

This is based on the work of Michael Westermann (http://lkml.org/lkml/2007/8/31/133)

Comments more than welcome.

Signed-off-by: Aristeu Rozanski <arozansk@redhat.com>

---
 drivers/serial/8250.c        |   12 ++++++++--
 drivers/serial/serial_core.c |   42 ++++++++++++++++++++++++++++++++++-
 include/asm-x86/termbits.h   |    1 
 include/linux/serial_core.h  |   51 +++++++++++++++++++++++++++----------------
 include/linux/termios.h      |    5 ++++
 5 files changed, 90 insertions(+), 21 deletions(-)

--- linus-2.6.orig/drivers/serial/8250.c	2008-08-05 16:44:26.000000000 -0400
+++ linus-2.6/drivers/serial/8250.c	2008-08-05 18:45:01.000000000 -0400
@@ -1409,7 +1409,7 @@ static unsigned int check_modem_status(s
 		if (status & UART_MSR_TERI)
 			up->port.icount.rng++;
 		if (status & UART_MSR_DDSR)
-			up->port.icount.dsr++;
+			uart_handle_dsr_change(&up->port, status & UART_MSR_DSR);
 		if (status & UART_MSR_DDCD)
 			uart_handle_dcd_change(&up->port, status & UART_MSR_DCD);
 		if (status & UART_MSR_DCTS)
@@ -1739,9 +1739,17 @@ static inline void wait_for_xmitr(struct
 		unsigned int tmout;
 		for (tmout = 1000000; tmout; tmout--) {
 			unsigned int msr = serial_in(up, UART_MSR);
+			struct uart_info *info = up->port.info;
+
 			up->msr_saved_flags |= msr & MSR_SAVE_FLAGS;
-			if (msr & UART_MSR_CTS)
+
+			if ((info->flags & UIF_CTS_FLOW) &&
+			    (msr & UART_MSR_CTS))
 				break;
+			else if ((info->flags & UIF_DSR_FLOW) &&
+				 (msr & UART_MSR_DSR))
+ 				break;
+
 			udelay(1);
 			touch_nmi_watchdog();
 		}
--- linus-2.6.orig/drivers/serial/serial_core.c	2008-08-05 16:44:26.000000000 -0400
+++ linus-2.6/drivers/serial/serial_core.c	2008-08-05 18:45:01.000000000 -0400
@@ -194,6 +194,13 @@ static int uart_startup(struct uart_stat
 			spin_unlock_irq(&port->lock);
 		}
 
+		if (info->flags & UIF_DSR_FLOW) {
+			spin_lock_irq(&port->lock);
+			if (!(port->ops->get_mctrl(port) & TIOCM_DSR))
+				info->port.tty->hw_stopped = 1;
+			spin_unlock_irq(&port->lock);
+		}
+
 		info->flags |= UIF_INITIALIZED;
 
 		clear_bit(TTY_IO_ERROR, &info->port.tty->flags);
@@ -448,6 +455,11 @@ uart_change_speed(struct uart_state *sta
 	else
 		state->info->flags &= ~UIF_CTS_FLOW;
 
+	if (termios->c_cflag & CDTRDSR)
+		state->info->flags |= UIF_DSR_FLOW;
+	else
+		state->info->flags &= ~UIF_DSR_FLOW;
+
 	if (termios->c_cflag & CLOCAL)
 		state->info->flags &= ~UIF_CHECK_CD;
 	else
@@ -611,6 +623,8 @@ static void uart_throttle(struct tty_str
 
 	if (tty->termios->c_cflag & CRTSCTS)
 		uart_clear_mctrl(state->port, TIOCM_RTS);
+	if (tty->termios->c_cflag & CDTRDSR)
+		uart_clear_mctrl(state->port, TIOCM_DTR);
 }
 
 static void uart_unthrottle(struct tty_struct *tty)
@@ -627,6 +641,8 @@ static void uart_unthrottle(struct tty_s
 
 	if (tty->termios->c_cflag & CRTSCTS)
 		uart_set_mctrl(port, TIOCM_RTS);
+	if (tty->termios->c_cflag & CDTRDSR)
+		uart_set_mctrl(port, TIOCM_DTR);
 }
 
 static int uart_get_info(struct uart_state *state,
@@ -1212,6 +1228,9 @@ static void uart_set_termios(struct tty_
 		if (!(cflag & CRTSCTS) ||
 		    !test_bit(TTY_THROTTLED, &tty->flags))
 			mask |= TIOCM_RTS;
+		if (!(cflag & CDTRDSR) ||
+		    !test_bit(TTY_THROTTLED, &tty->flags))
+			mask &= ~TIOCM_DTR;
 		uart_set_mctrl(state->port, mask);
 	}
 
@@ -1232,6 +1251,24 @@ static void uart_set_termios(struct tty_
 		}
 		spin_unlock_irqrestore(&state->port->lock, flags);
 	}
+
+	/* Handle turning off CDTRDSR */
+	if ((old_termios->c_cflag & CDTRDSR) && !(cflag & CDTRDSR)) {
+		spin_lock_irqsave(&state->port->lock, flags);
+		tty->hw_stopped = 0;
+		__uart_start(tty);
+		spin_unlock_irqrestore(&state->port->lock, flags);
+	}
+
+	if (!(old_termios->c_cflag & CDTRDSR) && (cflag & CDTRDSR)) {
+		spin_lock_irqsave(&state->port->lock, flags);
+		if (!(state->port->ops->get_mctrl(state->port) & TIOCM_DSR)) {
+			tty->hw_stopped = 1;
+			state->port->ops->stop_tx(state->port);
+		}
+		spin_unlock_irqrestore(&state->port->lock, flags);
+	}
+
 #if 0
 	/*
 	 * No need to wake up processes in open wait, since they
@@ -1511,7 +1548,8 @@ uart_block_til_ready(struct file *filp, 
 		 * not set RTS here - we want to make sure we catch
 		 * the data from the modem.
 		 */
-		if (info->port.tty->termios->c_cflag & CBAUD)
+		if (info->port.tty->termios->c_cflag & CBAUD &&
+		    !(info->port.tty->termios->c_cflag & CDTRDSR))
 			uart_set_mctrl(port, TIOCM_DTR);
 
 		/*
@@ -1959,6 +1997,8 @@ uart_set_options(struct uart_port *port,
 
 	if (flow == 'r')
 		termios.c_cflag |= CRTSCTS;
+	if (flow == 'd')
+		termios.c_cflag |= CDTRDSR;
 
 	/*
 	 * some uarts on other side don't support no flow control.
--- linus-2.6.orig/include/asm-x86/termbits.h	2008-08-05 16:44:26.000000000 -0400
+++ linus-2.6/include/asm-x86/termbits.h	2008-08-06 13:33:57.000000000 -0400
@@ -157,6 +157,7 @@ struct ktermios {
 #define  B3500000 0010016
 #define  B4000000 0010017
 #define CIBAUD	  002003600000	/* input baud rate */
+#define CDTRDSR	  004000000000	/* DTR/DSR flow control */
 #define CMSPAR	  010000000000	/* mark or space (stick) parity */
 #define CRTSCTS	  020000000000	/* flow control */
 
--- linus-2.6.orig/include/linux/serial_core.h	2008-08-05 16:44:26.000000000 -0400
+++ linus-2.6/include/linux/serial_core.h	2008-08-05 18:45:01.000000000 -0400
@@ -351,6 +351,7 @@ struct uart_info {
  *
  * FIXME: use the ASY_ definitions
  */
+#define UIF_DSR_FLOW		((__force uif_t) (1 << 24))
 #define UIF_CHECK_CD		((__force uif_t) (1 << 25))
 #define UIF_CTS_FLOW		((__force uif_t) (1 << 26))
 #define UIF_NORMAL_ACTIVE	((__force uif_t) (1 << 29))
@@ -505,34 +506,48 @@ uart_handle_dcd_change(struct uart_port 
 }
 
 /**
- *	uart_handle_cts_change - handle a change of clear-to-send state
+ *	uart_handle_flow_control_change - handle a change of CTS or DSR
  *	@port: uart_port structure for the open port
- *	@status: new clear to send status, nonzero if active
+ *	@status: new CTS/DTR status, nonzero if active
  */
 static inline void
-uart_handle_cts_change(struct uart_port *port, unsigned int status)
+uart_handle_flow_control_change(struct uart_port *port, unsigned int status)
 {
 	struct uart_info *info = port->info;
 	struct tty_struct *tty = info->port.tty;
 
-	port->icount.cts++;
-
-	if (info->flags & UIF_CTS_FLOW) {
-		if (tty->hw_stopped) {
-			if (status) {
-				tty->hw_stopped = 0;
-				port->ops->start_tx(port);
-				uart_write_wakeup(port);
-			}
-		} else {
-			if (!status) {
-				tty->hw_stopped = 1;
-				port->ops->stop_tx(port);
-			}
+	if (tty->hw_stopped) {
+		if (status) {
+			tty->hw_stopped = 0;
+			port->ops->start_tx(port);
+			uart_write_wakeup(port);
+		}
+	} else {
+		if (!status) {
+			tty->hw_stopped = 1;
+			port->ops->stop_tx(port);
 		}
 	}
 }
 
+static inline void
+uart_handle_cts_change(struct uart_port *port, unsigned int status)
+{
+	struct uart_info *info = port->info;
+	port->icount.cts++;
+	if (info->flags & UIF_CTS_FLOW)
+		uart_handle_flow_control_change(port, status);
+}
+
+static inline void
+uart_handle_dsr_change(struct uart_port *port, unsigned int status)
+{
+	struct uart_info *info = port->info;
+	port->icount.dsr++;
+	if (info->flags & UIF_DSR_FLOW)
+		uart_handle_flow_control_change(port, status);
+}
+
 #include <linux/tty_flip.h>
 
 static inline void
@@ -556,7 +571,7 @@ uart_insert_char(struct uart_port *port,
  *	UART_ENABLE_MS - determine if port should enable modem status irqs
  */
 #define UART_ENABLE_MS(port,cflag)	((port)->flags & UPF_HARDPPS_CD || \
-					 (cflag) & CRTSCTS || \
+					 (cflag) & (CRTSCTS | CDTRDSR) || \
 					 !((cflag) & CLOCAL))
 
 #endif
--- linus-2.6.orig/include/linux/termios.h	2008-08-05 16:44:26.000000000 -0400
+++ linus-2.6/include/linux/termios.h	2008-08-05 18:45:01.000000000 -0400
@@ -4,4 +4,9 @@
 #include <linux/types.h>
 #include <asm/termios.h>
 
+#ifndef CDTRDSR
+#warning This architecture should implement CDTRDSR
+#define CDTRDSR 0 /* remove this when all architectures have a definition */
+#endif
+
 #endif

Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Laurent Pinchart]

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

Hi,

On Wednesday 06 August 2008, Aristeu Rozanski wrote:
> This patch adds support for DTR/DSR hardware flow control on 8250 driver on
> x86 machines. It's done by adding a CDTRDSR flag to work just like CRTSCTS,
> which is not done on other architectures on purpose (so each maintainer can
> allocate it). 

It's funny, serial flow control hasn't been discussed for a long time, and you're the third person to start a flow control related thread on this mailing list in a few days.

> This patch was tested with success with a serial printer configured with a
> small buffer and DTR/DSR flow control.
> 
> This is based on the work of Michael Westermann
> (http://lkml.org/lkml/2007/8/31/133) 
> 
> Comments more than welcome.

Please read the '[PATCH/RFC] 8250: Auto RS485 direction control' thread for background information. In a nutshell we need more than just CTS/RTS and DTR/DSR, and we don't have enough c_cflags bits. We will probably have to create a new ioctl.

Best regards,

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Alan Cox]

> Please read the '[PATCH/RFC] 8250: Auto RS485 direction control' thread for background information. In a nutshell we need more than just CTS/RTS and DTR/DSR, and we don't have enough c_cflags bits. We will probably have to create a new ioctl.

Maybe - fortunately the kernel and user termios structure are now
independent so we can extend ktermios easily enough.

RE: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Tosoni]

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

In my opinion, DSR/DTR is another (wrong) way to do RTS/CTS flow control, so
I would have it implemented in setserial as a modifier to the CRTSCTS flag.

About a RS485 ioctl: could you consider the attached files which are already
in the Linux kernel (in include/asm-cris).
They define a TIOCSERSETRS485 (ioctl.h), and the data structure (rs485.h)
with allows to specify timings. Sounds just like what we want ?

JP Tosoni

> -----Original Message-----
> From: linux-serial-owner@vger.kernel.org
> [mailto:linux-serial-owner@vger.kernel.org]On Behalf Of
> Laurent Pinchart
> Sent: Thursday, August 07, 2008 10:32 AM
> To: Aristeu Rozanski
> Cc: linux-serial@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow
> control
>
>
> Hi,
>
> On Wednesday 06 August 2008, Aristeu Rozanski wrote:
> > This patch adds support for DTR/DSR hardware flow control
> on 8250 driver on
> > x86 machines. It's done by adding a CDTRDSR flag to work
> just like CRTSCTS,
> > which is not done on other architectures on purpose (so
> each maintainer can
> > allocate it).
>
> It's funny, serial flow control hasn't been discussed for a
> long time, and you're the third person to start a flow
> control related thread on this mailing list in a few days.
>
> > This patch was tested with success with a serial printer
> configured with a
> > small buffer and DTR/DSR flow control.
> >
> > This is based on the work of Michael Westermann
> > (http://lkml.org/lkml/2007/8/31/133)
> >
> > Comments more than welcome.
>
> Please read the '[PATCH/RFC] 8250: Auto RS485 direction
> control' thread for background information. In a nutshell we
> need more than just CTS/RTS and DTR/DSR, and we don't have
> enough c_cflags bits. We will probably have to create a new ioctl.
>
> Best regards,
>
> --
> Laurent Pinchart
> CSE Semaphore Belgium
>
> Chaussee de Bruxelles, 732A
> B-1410 Waterloo
> Belgium
>
> T +32 (2) 387 42 59
> F +32 (2) 387 42 75
>

[-- Attachment #2: ioctls.h --]
[-- Type: text/plain, Size: 2955 bytes --]

#ifndef __ARCH_CRIS_IOCTLS_H__
#define __ARCH_CRIS_IOCTLS_H__

/* verbatim copy of asm-i386/ioctls.h */

#include <asm/ioctl.h>

/* 0x54 is just a magic number to make these relatively unique ('T') */

#define TCGETS		0x5401
#define TCSETS		0x5402
#define TCSETSW		0x5403
#define TCSETSF		0x5404
#define TCGETA		0x5405
#define TCSETA		0x5406
#define TCSETAW		0x5407
#define TCSETAF		0x5408
#define TCSBRK		0x5409
#define TCXONC		0x540A
#define TCFLSH		0x540B
#define TIOCEXCL	0x540C
#define TIOCNXCL	0x540D
#define TIOCSCTTY	0x540E
#define TIOCGPGRP	0x540F
#define TIOCSPGRP	0x5410
#define TIOCOUTQ	0x5411
#define TIOCSTI		0x5412
#define TIOCGWINSZ	0x5413
#define TIOCSWINSZ	0x5414
#define TIOCMGET	0x5415
#define TIOCMBIS	0x5416
#define TIOCMBIC	0x5417
#define TIOCMSET	0x5418
#define TIOCGSOFTCAR	0x5419
#define TIOCSSOFTCAR	0x541A
#define FIONREAD	0x541B
#define TIOCINQ		FIONREAD
#define TIOCLINUX	0x541C
#define TIOCCONS	0x541D
#define TIOCGSERIAL	0x541E
#define TIOCSSERIAL	0x541F
#define TIOCPKT		0x5420
#define FIONBIO		0x5421
#define TIOCNOTTY	0x5422
#define TIOCSETD	0x5423
#define TIOCGETD	0x5424
#define TCSBRKP		0x5425	/* Needed for POSIX tcsendbreak() */
#define TIOCSBRK	0x5427  /* BSD compatibility */
#define TIOCCBRK	0x5428  /* BSD compatibility */
#define TIOCGSID	0x5429  /* Return the session ID of FD */
#define TCGETS2		_IOR('T',0x2A, struct termios2)
#define TCSETS2		_IOW('T',0x2B, struct termios2)
#define TCSETSW2	_IOW('T',0x2C, struct termios2)
#define TCSETSF2	_IOW('T',0x2D, struct termios2)
#define TIOCGPTN	_IOR('T',0x30, unsigned int) /* Get Pty Number (of pty-mux device) */
#define TIOCSPTLCK	_IOW('T',0x31, int)  /* Lock/unlock Pty */

#define FIONCLEX	0x5450  /* these numbers need to be adjusted. */
#define FIOCLEX		0x5451
#define FIOASYNC	0x5452
#define TIOCSERCONFIG	0x5453
#define TIOCSERGWILD	0x5454
#define TIOCSERSWILD	0x5455
#define TIOCGLCKTRMIOS	0x5456
#define TIOCSLCKTRMIOS	0x5457
#define TIOCSERGSTRUCT	0x5458 /* For debugging only */
#define TIOCSERGETLSR   0x5459 /* Get line status register */
#define TIOCSERGETMULTI 0x545A /* Get multiport config  */
#define TIOCSERSETMULTI 0x545B /* Set multiport config */

#define TIOCMIWAIT	0x545C	/* wait for a change on serial input line(s) */
#define TIOCGICOUNT	0x545D	/* read serial port inline interrupt counts */
#define TIOCGHAYESESP   0x545E  /* Get Hayes ESP configuration */
#define TIOCSHAYESESP   0x545F  /* Set Hayes ESP configuration */
#define FIOQSIZE	0x5460

#define TIOCSERSETRS485 0x5461  /* enable rs-485 */
#define TIOCSERWRRS485  0x5462  /* write rs-485 */

/* Used for packet mode */
#define TIOCPKT_DATA		 0
#define TIOCPKT_FLUSHREAD	 1
#define TIOCPKT_FLUSHWRITE	 2
#define TIOCPKT_STOP		 4
#define TIOCPKT_START		 8
#define TIOCPKT_NOSTOP		16
#define TIOCPKT_DOSTOP		32

#define TIOCSER_TEMT    0x01	/* Transmitter physically empty */

#endif

[-- Attachment #3: rs485.h --]
[-- Type: text/plain, Size: 474 bytes --]

/* RS-485 structures */

/* RS-485 support */
/* Used with ioctl() TIOCSERSETRS485 */
struct rs485_control {
        unsigned short rts_on_send;
        unsigned short rts_after_sent;
        unsigned long delay_rts_before_send;
        unsigned short enabled;
#ifdef __KERNEL__
        int disable_serial_loopback;
#endif
};

/* Used with ioctl() TIOCSERWRRS485 */
struct rs485_write {
        unsigned short outc_size;
        unsigned char *outc;
};

Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Alan Cox]

> About a RS485 ioctl: could you consider the attached files which are already
> in the Linux kernel (in include/asm-cris).
> They define a TIOCSERSETRS485 (ioctl.h), and the data structure (rs485.h)
> with allows to specify timings. Sounds just like what we want ?

That would make sense for hardware assisted RS485 but we still have to
sort out DTR/DSR as well

Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Alan Cox]

On Thu, 7 Aug 2008 14:54:05 +0200
"Tosoni" <jp.tosoni@acksys.fr> wrote:

> About a RS485 ioctl: could you consider the attached files which are already
> in the Linux kernel (in include/asm-cris).
> They define a TIOCSERSETRS485 (ioctl.h), and the data structure (rs485.h)
> with allows to specify timings. Sounds just like what we want ?

I had a deeper look at this for RS485 and the answer is "sort of". I've
reworked the structure to keep it the same size irrespective of 32/64bit
systems, and to make stuff flags that can be, plus add some extra u32
words in case we need to (.. when we need to ;)) add stuff later.

Comments, thoughts - will this do what people in the RS485 world need ?

Alan
--------------

tty: Cris has a nice RS485 ioctl so we should steal it

From: Alan Cox <alan@redhat.com>

JP Tosoni observed:

"About a RS485 ioctl: could you consider the attached files which are
 already in the Linux kernel (in include/asm-cris).  They define a
 TIOCSERSETRS485 (ioctl.h), and the data structure (rs485.h)
 with allows to specify timings. Sounds just like what we want ?"

and he's right: sort of. Rework the structure to use flag bits and make the
time delay a fixed sized field so we don't get 32/64bit problems. Add the ioctls
to x86 so that people know what to add to their platform of choice.

Signed-off-by: Alan Cox <alan@redhat.com>
---

 include/asm-x86/ioctls.h |    2 ++
 include/linux/serial.h   |   16 ++++++++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)


diff --git a/include/asm-x86/ioctls.h b/include/asm-x86/ioctls.h
index c0c338b..2cd4775 100644
--- a/include/asm-x86/ioctls.h
+++ b/include/asm-x86/ioctls.h
@@ -51,6 +51,8 @@
 #define TCSETS2		_IOW('T', 0x2B, struct termios2)
 #define TCSETSW2	_IOW('T', 0x2C, struct termios2)
 #define TCSETSF2	_IOW('T', 0x2D, struct termios2)
+#define TIOCGRS485	0x542E
+#define TIOCSRS485	0x542F
 #define TIOCGPTN	_IOR('T', 0x30, unsigned int)
 				/* Get Pty Number (of pty-mux device) */
 #define TIOCSPTLCK	_IOW('T', 0x31, int)  /* Lock/unlock Pty */
diff --git a/include/linux/serial.h b/include/linux/serial.h
index deb7143..1ea8d92 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -173,6 +173,22 @@ struct serial_icounter_struct {
 	int reserved[9];
 };
 
+/*
+ * Serial interface for controlling RS485 settings on chips with suitable
+ * support. Set with TIOCSRS485 and get with TIOCGRS485 if supported by your
+ * platform. The set function returns the new state, with any unsupported bits
+ * reverted appropriately.
+ */
+
+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)
+	__u32	delay_rts_before_send;	/* Milliseconds */
+	__u32	padding[6];		/* Memory is cheap, new structs
+					   are a royal PITA .. */
+};
 
 #ifdef __KERNEL__
 #include <linux/compiler.h>

Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Aristeu Rozanski]

> > About a RS485 ioctl: could you consider the attached files which are already
> > in the Linux kernel (in include/asm-cris).
> > They define a TIOCSERSETRS485 (ioctl.h), and the data structure (rs485.h)
> > with allows to specify timings. Sounds just like what we want ?
> 
> I had a deeper look at this for RS485 and the answer is "sort of". I've
> reworked the structure to keep it the same size irrespective of 32/64bit
> systems, and to make stuff flags that can be, plus add some extra u32
> words in case we need to (.. when we need to ;)) add stuff later.
> 
> Comments, thoughts - will this do what people in the RS485 world need ?
as for DTR/DSR patch, will be used the same approach?

-- 
Aristeu

Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Alan Cox]

On Wed, 20 Aug 2008 17:43:36 -0400
Aristeu Rozanski <aris@ruivo.org> wrote:

> > > About a RS485 ioctl: could you consider the attached files which are already
> > > in the Linux kernel (in include/asm-cris).
> > > They define a TIOCSERSETRS485 (ioctl.h), and the data structure (rs485.h)
> > > with allows to specify timings. Sounds just like what we want ?
> > 
> > I had a deeper look at this for RS485 and the answer is "sort of". I've
> > reworked the structure to keep it the same size irrespective of 32/64bit
> > systems, and to make stuff flags that can be, plus add some extra u32
> > words in case we need to (.. when we need to ;)) add stuff later.
> > 
> > Comments, thoughts - will this do what people in the RS485 world need ?
> as for DTR/DSR patch, will be used the same approach?

I'm still trying to get a sensible answer on how other Unixes handle it

Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

['Aristeu Rozanski']

> > > > About a RS485 ioctl: could you consider the attached files which are already
> > > > in the Linux kernel (in include/asm-cris).
> > > > They define a TIOCSERSETRS485 (ioctl.h), and the data structure (rs485.h)
> > > > with allows to specify timings. Sounds just like what we want ?
> > > 
> > > I had a deeper look at this for RS485 and the answer is "sort of". I've
> > > reworked the structure to keep it the same size irrespective of 32/64bit
> > > systems, and to make stuff flags that can be, plus add some extra u32
> > > words in case we need to (.. when we need to ;)) add stuff later.
> > > 
> > > Comments, thoughts - will this do what people in the RS485 world need ?
> > as for DTR/DSR patch, will be used the same approach?
> 
> I'm still trying to get a sensible answer on how other Unixes handle it
I did some research on that:
Solaris and AIX:
	TC{G,S}ETX for extended options and only input flow control (DTRXOFF)
SCO:
	{S,G}ETFLOW for configuring flow control, TXHARD, RXHARD for DTRDSR
FreeBSD:
	cflags has 'dtrflow' and 'dsrflow'

Having the option to set individually which pins to use for input and output
flow control and which ones should be on/off all the time seem to be a powerful
way to do it, instead of having a "CDTRDSR".

-- 
Aristeu

Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Christopher Gibson]

I've had a look at the asm-cris rs485 implementation, as JP Tosoni
suggested, and say that that interface would satisfy my requirements,
and would also provide a suitable interface for controlling out dated
radio modems.  I would say though that some additional consideration
should be given to the interface.

I had assumed that the rts_on_send, and rts_after_send were time delay
figures until I read the suggested rework by Alan Cox, then read through
the crisv10.c code.  In the crisv10 serial driver code the rts_on_send
specifies the state that the RTS line should change to when a call to
the TIOCSERWRRS485 ioctl is made the rts_after_send is the state that
RTS is changed to after the transmission has completed.  So in two of
the four possible combinations user space control of RTS is still
required in order to get any directional control activity out of the RTS
line.  I would be so bold as to say that this combination of user /
automatic direction control would be unlikely to be ever used by anyone.
If this is correct then the two flags could be replaced with one
rts_tx_level indicating if RTS should be high or low during a
transmission, and pre-transmission delay.  Would this be clearer?

The other observation about the crsv10.c implementation is that the call
to TIOCSERWRRS485 ioctl, or write() if the enabled flag is set, does not
return until the transmission has completed and the line has been turned
around.  This functionality could be restrictive in some situations
(single threaded user applications that wish to do other stuff during
transmission).

My suggestion would be to scrap the TIOCSERWRRS485 ioctl, in favour of
using the standard write(fd) and drain(fd) function calls, but to stick
with the TIOCSERSETRS485 ioctl for automatic direction control port
setup.  I would be suggesting a control structure (based on Alan Cox's
suggested modifications of the include/asm-cris/rs485.h) as follows:

/*
 * Serial interface for controlling RS485 settings on chips with suitable
 * support. Set with TIOCSRS485 and get with TIOCGRS485 if supported by your
 * platform. The set function returns the new state, with any unsupported bits
 * reverted appropriately.
 */

struct serial_rs485 {
	__u32	flags;			/* RS485 feature flags */
#define SER_RS485_MODES			(7 << 0)	/* Mask for mode bits. */
#define SER_RS485_MODE_DISABLED		(0 << 0)
#define SER_RS485_MODE_RTS_TX_HIGH	(2 << 0)
#define SER_RS485_MODE_RTS_TX_LOW	(3 << 0)
#define SER_RS485_MODE_DTR_TX_HIGH	(4 << 0)
#define SER_RS485_MODE_DTR_TX_LOW	(5 << 0)
	__u16	delay_rts_before_send;	/* Milliseconds */
	__u16	delay_rts_after_send;	/* Milliseconds */
	__u32	padding[6];		/* Memory is cheap, new structs
					   are a royal PITA .. */
};

Any comments?

The above would allow for RTS and DTR direction control as well as
settings for lead in and lead out timing.

-- 
Christopher Gibson <chris@toftronix.com.au>

RE: [PATCH v1] 8250: add support for DTR/DSR hardware flow control

[Tosoni]

Your analysis and the new struct are fine to me.

Maybe 'delay_rts_before_send' should be named simply 'delay_before_send'
since it applies to dtr as well. Same for delay_rts_after_send.

Jean-Pierre Tosoni

> -----Original Message-----
> From: linux-serial-owner@vger.kernel.org
> [mailto:linux-serial-owner@vger.kernel.org]On Behalf Of Christopher
> Gibson
> Sent: Friday, August 29, 2008 5:34 PM
> To: linux-serial
> Subject: Re: [PATCH v1] 8250: add support for DTR/DSR hardware flow
> control
>
>
> I've had a look at the asm-cris rs485 implementation, as JP Tosoni
> suggested, and say that that interface would satisfy my requirements,
> and would also provide a suitable interface for controlling out dated
> radio modems.  I would say though that some additional consideration
> should be given to the interface.
>
> I had assumed that the rts_on_send, and rts_after_send were time delay
> figures until I read the suggested rework by Alan Cox, then
> read through
> the crisv10.c code.  In the crisv10 serial driver code the rts_on_send
> specifies the state that the RTS line should change to when a call to
> the TIOCSERWRRS485 ioctl is made the rts_after_send is the state that
> RTS is changed to after the transmission has completed.  So in two of
> the four possible combinations user space control of RTS is still
> required in order to get any directional control activity out
> of the RTS
> line.  I would be so bold as to say that this combination of user /
> automatic direction control would be unlikely to be ever used
> by anyone.
> If this is correct then the two flags could be replaced with one
> rts_tx_level indicating if RTS should be high or low during a
> transmission, and pre-transmission delay.  Would this be clearer?
>
> The other observation about the crsv10.c implementation is
> that the call
> to TIOCSERWRRS485 ioctl, or write() if the enabled flag is
> set, does not
> return until the transmission has completed and the line has
> been turned
> around.  This functionality could be restrictive in some situations
> (single threaded user applications that wish to do other stuff during
> transmission).
>
> My suggestion would be to scrap the TIOCSERWRRS485 ioctl, in favour of
> using the standard write(fd) and drain(fd) function calls,
> but to stick
> with the TIOCSERSETRS485 ioctl for automatic direction control port
> setup.  I would be suggesting a control structure (based on Alan Cox's
> suggested modifications of the include/asm-cris/rs485.h) as follows:
>
> /*
>  * Serial interface for controlling RS485 settings on chips
> with suitable
>  * support. Set with TIOCSRS485 and get with TIOCGRS485 if
> supported by your
>  * platform. The set function returns the new state, with any
> unsupported bits
>  * reverted appropriately.
>  */
>
> struct serial_rs485 {
> 	__u32	flags;			/* RS485 feature flags */
> #define SER_RS485_MODES			(7 << 0)
> /* Mask for mode bits. */
> #define SER_RS485_MODE_DISABLED		(0 << 0)
> #define SER_RS485_MODE_RTS_TX_HIGH	(2 << 0)
> #define SER_RS485_MODE_RTS_TX_LOW	(3 << 0)
> #define SER_RS485_MODE_DTR_TX_HIGH	(4 << 0)
> #define SER_RS485_MODE_DTR_TX_LOW	(5 << 0)
> 	__u16	delay_rts_before_send;	/* Milliseconds */
> 	__u16	delay_rts_after_send;	/* Milliseconds */
> 	__u32	padding[6];		/* Memory is cheap, new structs
> 					   are a royal PITA .. */
> };
>
> Any comments?
>
> The above would allow for RTS and DTR direction control as well as
> settings for lead in and lead out timing.
>
> --
> Christopher Gibson <chris@toftronix.com.au>
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-serial" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>