Re: [PATCH] ARM/ARM26: Enable arbitary speed tty ioctls and split input/output speed

Re: [PATCH] ARM/ARM26: Enable arbitary speed tty ioctls and split input/output speed

[Russell King]

On Wed, May 23, 2007 at 05:27:39PM +0100, Alan Cox wrote:
> Add the ioctls and values needed for this to the ARM26/ARM32 ports. The
> actual code has been in the base kernel for a while and automatically
> turns on when a port sets the required defines.
> 
> Signed-off-by: Alan Cox <alan@redhat.com>
> 
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/ioctls.h linux-2.6.22-rc1-mm1/include/asm-arm/ioctls.h
> --- linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/ioctls.h	2007-04-30 10:48:14.000000000 +0100
> +++ linux-2.6.22-rc1-mm1/include/asm-arm/ioctls.h	2007-05-23 16:19:48.010005480 +0100
> @@ -46,6 +46,10 @@
>  #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 */
>  
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/termbits.h linux-2.6.22-rc1-mm1/include/asm-arm/termbits.h
> --- linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/termbits.h	2007-04-30 10:48:14.000000000 +0100
> +++ linux-2.6.22-rc1-mm1/include/asm-arm/termbits.h	2007-05-23 16:23:05.765942008 +0100
> @@ -128,6 +128,7 @@
>  #define HUPCL	0002000
>  #define CLOCAL	0004000
>  #define CBAUDEX 0010000
> +#define    BOTHER 0010000
>  #define    B57600 0010001
>  #define   B115200 0010002
>  #define   B230400 0010003
> @@ -143,10 +144,12 @@
>  #define  B3000000 0010015
>  #define  B3500000 0010016
>  #define  B4000000 0010017
> -#define CIBAUD	  002003600000	/* input baud rate (not used) */
> +#define CIBAUD	  002003600000		/* input baud rate */
>  #define CMSPAR    010000000000		/* mark or space (stick) parity */
>  #define CRTSCTS	  020000000000		/* flow control */
>  
> +#define IBSHIFT	   16
> +
>  /* c_lflag bits */
>  #define ISIG	0000001
>  #define ICANON	0000002
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/termios.h linux-2.6.22-rc1-mm1/include/asm-arm/termios.h
> --- linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/termios.h	2007-04-30 11:00:07.000000000 +0100
> +++ linux-2.6.22-rc1-mm1/include/asm-arm/termios.h	2007-05-23 16:21:34.535811096 +0100
> @@ -82,8 +82,10 @@
>  	copy_to_user((termio)->c_cc, (termios)->c_cc, NCC); \
>  })
>  
> -#define user_termios_to_kernel_termios(k, u) copy_from_user(k, u, sizeof(struct termios))
> -#define kernel_termios_to_user_termios(u, k) copy_to_user(u, k, sizeof(struct termios))
> +#define user_termios_to_kernel_termios(k, u) copy_from_user(k, u, sizeof(struct termios2))
> +#define kernel_termios_to_user_termios(u, k) copy_to_user(u, k, sizeof(struct termios2))
> +#define user_termios_to_kernel_termios_1(k, u) copy_from_user(k, u, sizeof(struct termios))
> +#define kernel_termios_to_user_termios_1(u, k) copy_to_user(u, k, sizeof(struct termios))
>  
>  #endif	/* __KERNEL__ */
>  

Alan, thanks for this.

Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>

for the above.  The ARM26 stuff needs acking by Ian Molton, <spyro@f2s.com>

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

[PATCH] ARM/ARM26: Enable arbitary speed tty ioctls and split input/output speed

[Alan Cox]

Add the ioctls and values needed for this to the ARM26/ARM32 ports. The
actual code has been in the base kernel for a while and automatically
turns on when a port sets the required defines.

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

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/ioctls.h linux-2.6.22-rc1-mm1/include/asm-arm/ioctls.h
--- linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/ioctls.h	2007-04-30 10:48:14.000000000 +0100
+++ linux-2.6.22-rc1-mm1/include/asm-arm/ioctls.h	2007-05-23 16:19:48.010005480 +0100
@@ -46,6 +46,10 @@
 #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 */
 
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/termbits.h linux-2.6.22-rc1-mm1/include/asm-arm/termbits.h
--- linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/termbits.h	2007-04-30 10:48:14.000000000 +0100
+++ linux-2.6.22-rc1-mm1/include/asm-arm/termbits.h	2007-05-23 16:23:05.765942008 +0100
@@ -128,6 +128,7 @@
 #define HUPCL	0002000
 #define CLOCAL	0004000
 #define CBAUDEX 0010000
+#define    BOTHER 0010000
 #define    B57600 0010001
 #define   B115200 0010002
 #define   B230400 0010003
@@ -143,10 +144,12 @@
 #define  B3000000 0010015
 #define  B3500000 0010016
 #define  B4000000 0010017
-#define CIBAUD	  002003600000	/* input baud rate (not used) */
+#define CIBAUD	  002003600000		/* input baud rate */
 #define CMSPAR    010000000000		/* mark or space (stick) parity */
 #define CRTSCTS	  020000000000		/* flow control */
 
+#define IBSHIFT	   16
+
 /* c_lflag bits */
 #define ISIG	0000001
 #define ICANON	0000002
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/termios.h linux-2.6.22-rc1-mm1/include/asm-arm/termios.h
--- linux.vanilla-2.6.22-rc1-mm1/include/asm-arm/termios.h	2007-04-30 11:00:07.000000000 +0100
+++ linux-2.6.22-rc1-mm1/include/asm-arm/termios.h	2007-05-23 16:21:34.535811096 +0100
@@ -82,8 +82,10 @@
 	copy_to_user((termio)->c_cc, (termios)->c_cc, NCC); \
 })
 
-#define user_termios_to_kernel_termios(k, u) copy_from_user(k, u, sizeof(struct termios))
-#define kernel_termios_to_user_termios(u, k) copy_to_user(u, k, sizeof(struct termios))
+#define user_termios_to_kernel_termios(k, u) copy_from_user(k, u, sizeof(struct termios2))
+#define kernel_termios_to_user_termios(u, k) copy_to_user(u, k, sizeof(struct termios2))
+#define user_termios_to_kernel_termios_1(k, u) copy_from_user(k, u, sizeof(struct termios))
+#define kernel_termios_to_user_termios_1(u, k) copy_to_user(u, k, sizeof(struct termios))
 
 #endif	/* __KERNEL__ */
 
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc1-mm1/include/asm-arm26/ioctls.h linux-2.6.22-rc1-mm1/include/asm-arm26/ioctls.h
--- linux.vanilla-2.6.22-rc1-mm1/include/asm-arm26/ioctls.h	2007-04-30 10:48:14.000000000 +0100
+++ linux-2.6.22-rc1-mm1/include/asm-arm26/ioctls.h	2007-05-23 16:22:44.320202256 +0100
@@ -47,6 +47,10 @@
 #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 */
 
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc1-mm1/include/asm-arm26/termbits.h linux-2.6.22-rc1-mm1/include/asm-arm26/termbits.h
--- linux.vanilla-2.6.22-rc1-mm1/include/asm-arm26/termbits.h	2007-04-30 10:48:14.000000000 +0100
+++ linux-2.6.22-rc1-mm1/include/asm-arm26/termbits.h	2007-05-23 16:22:25.575051952 +0100
@@ -128,6 +128,7 @@
 #define HUPCL	0002000
 #define CLOCAL	0004000
 #define CBAUDEX 0010000
+#define   BOTHER  0010000
 #define    B57600 0010001
 #define   B115200 0010002
 #define   B230400 0010003
@@ -143,10 +144,12 @@
 #define  B3000000 0010015
 #define  B3500000 0010016
 #define  B4000000 0010017
-#define CIBAUD	  002003600000	/* input baud rate (not used) */
+#define CIBAUD	  002003600000		/* input baud rate */
 #define CMSPAR    010000000000		/* mark or space (stick) parity */
 #define CRTSCTS	  020000000000		/* flow control */
 
+#define IBSHIFT	  16		/* Shift from CBAUD to CIBAUD */
+
 /* c_lflag bits */
 #define ISIG	0000001
 #define ICANON	0000002
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc1-mm1/include/asm-arm26/termios.h linux-2.6.22-rc1-mm1/include/asm-arm26/termios.h
--- linux.vanilla-2.6.22-rc1-mm1/include/asm-arm26/termios.h	2007-04-30 11:00:07.000000000 +0100
+++ linux-2.6.22-rc1-mm1/include/asm-arm26/termios.h	2007-05-23 16:21:22.457647256 +0100
@@ -82,8 +82,10 @@
 	copy_to_user((termio)->c_cc, (termios)->c_cc, NCC); \
 })
 
-#define user_termios_to_kernel_termios(k, u) copy_from_user(k, u, sizeof(struct termios))
-#define kernel_termios_to_user_termios(u, k) copy_to_user(u, k, sizeof(struct termios))
+#define user_termios_to_kernel_termios(k, u) copy_from_user(k, u, sizeof(struct termios2))
+#define kernel_termios_to_user_termios(u, k) copy_to_user(u, k, sizeof(struct termios2))
+#define user_termios_to_kernel_termios_1(k, u) copy_from_user(k, u, sizeof(struct termios))
+#define kernel_termios_to_user_termios_1(u, k) copy_to_user(u, k, sizeof(struct termios))
 
 #endif	/* __KERNEL__ */
 

Re: [PATCH] $ARCH: Enable arbitary speed tty ioctls and split input/output speed

[David Woodhouse]

Is my mailbox (or brain) failing me, or did you just send out these
patches for every architecture _except_ PowerPC? :)

Presumably this is because of the mess in tty_termios_encode_baud_rate
when we fix its bogus assumptions about IBSHIFT always being defined?
There has to be a cleaner way to do that than this, but it shows the
problem...

diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
index fd471cb..f5f4568 100644
--- a/drivers/char/tty_ioctl.c
+++ b/drivers/char/tty_ioctl.c
@@ -215,6 +215,11 @@ speed_t tty_termios_input_baud_rate(struct ktermios *termios)
 	}
 	return baud_table[cbaud];
 #else
+#ifdef BOTHER
+	/* Magic token for arbitary speed via c_ispeed/c_ospeed */
+	if ((termios->c_cflag & CBAUD) == BOTHER)
+		return termios->c_ispeed;
+#endif
 	return tty_termios_baud_rate(termios);
 #endif
 }
@@ -252,16 +257,27 @@ void tty_termios_encode_baud_rate(struct ktermios *termios, speed_t ibaud, speed
 	termios->c_ispeed = ibaud;
 	termios->c_ospeed = obaud;
 
+#ifndef IBSHIFT
+	/* If the only way to express differing input and output baud rates
+	   is to use BOTHER, then that's what we have to do... */
+	if (ibaud != obaud) {
+		termios->c_cflag |= BOTHER;
+		return;
+	}
+#endif
+
 	/* If the user asked for a precise weird speed give a precise weird
 	   answer. If they asked for a Bfoo speed they many have problems
 	   digesting non-exact replies so fuzz a bit */
 
 	if ((termios->c_cflag & CBAUD) == BOTHER)
 		oclose = 0;
+#ifdef IBSHIFT
 	if (((termios->c_cflag >> IBSHIFT) & CBAUD) == BOTHER)
 		iclose = 0;
 	if ((termios->c_cflag >> IBSHIFT) & CBAUD)
 		ibinput = 1;	/* An input speed was specified */
+#endif
 
 	termios->c_cflag &= ~CBAUD;
 
@@ -270,20 +286,24 @@ void tty_termios_encode_baud_rate(struct ktermios *termios, speed_t ibaud, speed
 			termios->c_cflag |= baud_bits[i];
 			ofound = i;
 		}
+#ifdef IBSHIFT
 		if (ibaud - iclose >= baud_table[i] && ibaud + iclose <= baud_table[i]) {
 			/* For the case input == output don't set IBAUD bits if the user didn't do so */
 			if (ofound != i || ibinput)
 				termios->c_cflag |= (baud_bits[i] << IBSHIFT);
 			ifound = i;
 		}
-	}
-	while(++i < n_baud_table);
+#endif
+	} while(++i < n_baud_table);
+
 	if (ofound == -1)
 		termios->c_cflag |= BOTHER;
+#ifdef IBSHIFT
 	/* Set exact input bits only if the input and output differ or the
 	   user already did */
 	if (ifound == -1 && (ibaud != obaud  || ibinput))
 		termios->c_cflag |= (BOTHER << IBSHIFT);
+#endif
 }
 
 EXPORT_SYMBOL_GPL(tty_termios_encode_baud_rate);
diff --git a/include/asm-powerpc/ioctls.h b/include/asm-powerpc/ioctls.h
index 279a622..b6dfe7f 100644
--- a/include/asm-powerpc/ioctls.h
+++ b/include/asm-powerpc/ioctls.h
@@ -31,6 +31,11 @@
 #define TCXONC		_IO('t', 30)
 #define TCFLSH		_IO('t', 31)
 
+#define TCGETS2		_IOR('t', 32, struct termios2)
+#define TCSETS2		_IOW('t', 33, struct termios2)
+#define TCSETSW2	_IOW('t', 34, struct termios2)
+#define TCSETSF2	_IOW('t', 35, struct termios2)
+
 #define TIOCSWINSZ	_IOW('t', 103, struct winsize)
 #define TIOCGWINSZ	_IOR('t', 104, struct winsize)
 #define	TIOCSTART	_IO('t', 110)		/* start output, like ^Q */
diff --git a/include/asm-powerpc/termbits.h b/include/asm-powerpc/termbits.h
index 5e79198..c39877f 100644
--- a/include/asm-powerpc/termbits.h
+++ b/include/asm-powerpc/termbits.h
@@ -43,6 +43,19 @@ struct ktermios {
 	speed_t c_ospeed;		/* output speed */
 };
 
+/* Yay. A third identical definition of the same structure. */
+
+struct termios2 {
+	tcflag_t c_iflag;		/* input mode flags */
+	tcflag_t c_oflag;		/* output mode flags */
+	tcflag_t c_cflag;		/* control mode flags */
+	tcflag_t c_lflag;		/* local mode flags */
+	cc_t c_cc[NCCS];		/* control characters */
+	cc_t c_line;			/* line discipline (== c_cc[19]) */
+	speed_t c_ispeed;		/* input speed */
+	speed_t c_ospeed;		/* output speed */
+};
+
 /* c_cc characters */
 #define VINTR 	         0
 #define VQUIT 	         1
@@ -152,6 +165,7 @@ struct ktermios {
 #define B3000000  00034
 #define B3500000  00035
 #define B4000000  00036
+#define   BOTHER  00037
 
 #define CSIZE	00001400
 #define   CS5	00000000
diff --git a/include/asm-powerpc/termios.h b/include/asm-powerpc/termios.h
index 2c14fea..2841fb1 100644
--- a/include/asm-powerpc/termios.h
+++ b/include/asm-powerpc/termios.h
@@ -80,6 +80,9 @@ struct termio {
 
 #include <asm-generic/termios.h>
 
+#define user_termios_to_kernel_termios_1(k, u) copy_from_user(k, u, sizeof(struct termios))
+#define kernel_termios_to_user_termios_1(u, k) copy_to_user(u, k, sizeof(struct termios))
+
 #endif	/* __KERNEL__ */
 
 #endif	/* _ASM_POWERPC_TERMIOS_H */

-- 
dwmw2

Re: [PATCH] $ARCH: Enable arbitary speed tty ioctls and split input/output speed

[Alan Cox]

On Thu, 24 May 2007 09:08:55 -0400
David Woodhouse <dwmw2@infradead.org> wrote:

> Is my mailbox (or brain) failing me, or did you just send out these
> patches for every architecture _except_ PowerPC? :)

PowerPC is one of the main ones I've not touched because it needs work
itself to sort out the IBSHIFT value and bit allocations, also because
its one of several dependant upon the asm-generic stuff (which also means
your patch won't work)

Most people copied the x86 behaviour which makes it easy to transplant.
Some are just smoking something (see ioctls.h for sh-64 and weep), others
have slightly odd behaviour for historical compatibility reasons (Sparc)

> Presumably this is because of the mess in tty_termios_encode_baud_rate
> when we fix its bogus assumptions about IBSHIFT always being defined?

It's not a bogus assumption. The current code supports

	- The old way
	- BOTHER and IBSHIFT

When PPC wants to do arbitary baud rate it needs to resolve both of the
definitions together. IBSHIFT is simply the shift you apply to the baud
bits to get the input baud bits.

Alan

Re: [PATCH] $ARCH: Enable arbitary speed tty ioctls and split input/output speed

[David Woodhouse]

On Thu, 2007-05-24 at 14:41 +0100, Alan Cox wrote:
> Most people copied the x86 behaviour which makes it easy to transplant.
> Some are just smoking something (see ioctls.h for sh-64 and weep), others
> have slightly odd behaviour for historical compatibility reasons (Sparc)

Likewise, I assume the lack of IBSHIFT on PowerPC is because of AIX?

> > Presumably this is because of the mess in tty_termios_encode_baud_rate
> > when we fix its bogus assumptions about IBSHIFT always being defined?
> 
> It's not a bogus assumption. The current code supports
> 
> 	- The old way
> 	- BOTHER and IBSHIFT
> 
> When PPC wants to do arbitary baud rate it needs to resolve both of the
> definitions together. IBSHIFT is simply the shift you apply to the baud
> bits to get the input baud bits.

Why bother introducing new IBSHIFT stuff when it can be declared
obsolete already -- if you want different input and output baud rates,
just set BOTHER and have different values c_ispeed and c_ospeed.

That's what my patch attempts. It builds but I haven't actually tested
it. Why do you say it won't work?

-- 
dwmw2

Re: [PATCH] $ARCH: Enable arbitary speed tty ioctls and split input/output speed

[Alan Cox]

On Thu, 24 May 2007 09:45:37 -0400
David Woodhouse <dwmw2@infradead.org> wrote:

> On Thu, 2007-05-24 at 14:41 +0100, Alan Cox wrote:
> > Most people copied the x86 behaviour which makes it easy to transplant.
> > Some are just smoking something (see ioctls.h for sh-64 and weep), others
> > have slightly odd behaviour for historical compatibility reasons (Sparc)
> 
> Likewise, I assume the lack of IBSHIFT on PowerPC is because of AIX?

I assume nobody ever got around to it. CIBAUD is in the PowerPC System V
API supplement for example and is supported by other Power OS's.

> > When PPC wants to do arbitary baud rate it needs to resolve both of the
> > definitions together. IBSHIFT is simply the shift you apply to the baud
> > bits to get the input baud bits.
> 
> Why bother introducing new IBSHIFT stuff when it can be declared
> obsolete already -- if you want different input and output baud rates,
> just set BOTHER and have different values c_ispeed and c_ospeed.

BOTHER is for the output speed. BOTHER << IBSHIFT is for the input speed
if it different to the default (and you need B0 << IBSHIFT to know if an
input speed is being specifically set in the first place).

CIBAUD and IBSHIFT come from standards documents so it appears to make
sense at some level to support those standards. As everyone else in the
Power world supports it I don't see why Linux should be different.

Alan

Re: [PATCH] $ARCH: Enable arbitary speed tty ioctls and split input/output speed

[David Woodhouse]

On Thu, 2007-05-24 at 16:05 +0100, Alan Cox wrote:
> 
> > Likewise, I assume the lack of IBSHIFT on PowerPC is because of AIX?
> 
> I assume nobody ever got around to it. CIBAUD is in the PowerPC System V
> API supplement for example and is supported by other Power OS's.

Hm... what we have in asm-powerpc/termbits.h doesn't seem to match
what's in ppc-sysv-1995-09.ps.gz. Where _should_ I be looking?

Paulus?

-- 
dwmw2

Re: [PATCH] $ARCH: Enable arbitary speed tty ioctls and split input/output speed

[Paul Mackerras]

Alan Cox writes:

> On Thu, 24 May 2007 09:45:37 -0400
> David Woodhouse <dwmw2@infradead.org> wrote:
> 
> > On Thu, 2007-05-24 at 14:41 +0100, Alan Cox wrote:
> > > Most people copied the x86 behaviour which makes it easy to transplant.
> > > Some are just smoking something (see ioctls.h for sh-64 and weep), others
> > > have slightly odd behaviour for historical compatibility reasons (Sparc)
> > 
> > Likewise, I assume the lack of IBSHIFT on PowerPC is because of AIX?
> 
> I assume nobody ever got around to it. CIBAUD is in the PowerPC System V
> API supplement for example and is supported by other Power OS's.

The guys that did the original ppc linux port (mainly Gary Thomas and
Cort Dougan, I believe) tended to follow the alpha and x86 linux ports
rather than the System V ABI.  There aren't any SysV-based OSes for
PowerPC still extant so it doesn't really matter.

And yes, nobody has got around to implementing CIBAUD/IBSHIFT.

> > Why bother introducing new IBSHIFT stuff when it can be declared
> > obsolete already -- if you want different input and output baud rates,
> > just set BOTHER and have different values c_ispeed and c_ospeed.
> 
> BOTHER is for the output speed. BOTHER << IBSHIFT is for the input speed
> if it different to the default (and you need B0 << IBSHIFT to know if an
> input speed is being specifically set in the first place).
> 
> CIBAUD and IBSHIFT come from standards documents so it appears to make
> sense at some level to support those standards. As everyone else in the
> Power world supports it I don't see why Linux should be different.

Sure, we can add them.  I suggest IBSHIFT = 16 and CIBAUD = 0xff0000
(or 077600000 if we are going to continue the peculiar convention of
using octal for the C* constants).

Paul.

Re: [PATCH] ARM/ARM26: Enable arbitary speed tty ioctls and split input/output speed

[Russell King]

On Wed, May 23, 2007 at 05:27:39PM +0100, Alan Cox wrote:
> Add the ioctls and values needed for this to the ARM26/ARM32 ports. The
> actual code has been in the base kernel for a while and automatically
> turns on when a port sets the required defines.

Did you forget to provide a termios2 structure for ARM?  Hope you
remembered for the other arches:

drivers/char/tty_ioctl.c: In function `set_termios':
drivers/char/tty_ioctl.c:429: error: invalid application of `sizeof' to incomplete type `termios2'
drivers/char/tty_ioctl.c: In function `n_tty_ioctl':
drivers/char/tty_ioctl.c:732: error: invalid application of `sizeof' to incomplete type `termios2'

Will copy the i386 version into ARM's termbits.h

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

Re: [PATCH] ARM/ARM26: Enable arbitary speed tty ioctls and split input/output speed

[Andrew Morton]

On Mon, 28 May 2007 20:36:58 +0100 Russell King <rmk+lkml@arm.linux.org.uk> wrote:

> On Wed, May 23, 2007 at 05:27:39PM +0100, Alan Cox wrote:
> > Add the ioctls and values needed for this to the ARM26/ARM32 ports. The
> > actual code has been in the base kernel for a while and automatically
> > turns on when a port sets the required defines.
> 
> Did you forget to provide a termios2 structure for ARM?  Hope you
> remembered for the other arches:
> 
> drivers/char/tty_ioctl.c: In function `set_termios':
> drivers/char/tty_ioctl.c:429: error: invalid application of `sizeof' to incomplete type `termios2'
> drivers/char/tty_ioctl.c: In function `n_tty_ioctl':
> drivers/char/tty_ioctl.c:732: error: invalid application of `sizeof' to incomplete type `termios2'
> 
> Will copy the i386 version into ARM's termbits.h
> 

I think Alan's per-arch patch was dependent upon an earlier patch.  He
fooled everyone by sending them out in random order, without sequence
nnumbers and without telling anyone the dependencies.  And the add-termios2
patch was a single megapatch whereas the enablement patches were a per-arch
sprinkle.


Here's tha arm bit of
lots-of-architectures-enable-arbitary-speed-tty-support.patch: 

diff -puN include/asm-arm/termbits.h~lots-of-architectures-enable-arbitary-speed-tty-support include/asm-arm/termbits.h
--- a/include/asm-arm/termbits.h~lots-of-architectures-enable-arbitary-speed-tty-support
+++ a/include/asm-arm/termbits.h
@@ -15,6 +15,17 @@ struct termios {
 	cc_t c_cc[NCCS];		/* control characters */
 };
 
+struct termios_2 {
+	tcflag_t c_iflag;		/* input mode flags */
+	tcflag_t c_oflag;		/* output mode flags */
+	tcflag_t c_cflag;		/* control mode flags */
+	tcflag_t c_lflag;		/* local mode flags */
+	cc_t c_line;			/* line discipline */
+	cc_t c_cc[NCCS];		/* control characters */
+	speed_t c_ispeed;		/* input speed */
+	speed_t c_ospeed;		/* output speed */
+};
+
 struct ktermios {
 	tcflag_t c_iflag;		/* input mode flags */
 	tcflag_t c_oflag;		/* output mode flags */

Or you can just drop this patch and I'll resend it once
lots-of-architectures-enable-arbitary-speed-tty-support.patch is merged up.

Re: [PATCH] ARM/ARM26: Enable arbitary speed tty ioctls and split input/output speed

[Russell King]

On Mon, May 28, 2007 at 02:56:32PM -0700, Andrew Morton wrote:
> On Mon, 28 May 2007 20:36:58 +0100 Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> 
> > On Wed, May 23, 2007 at 05:27:39PM +0100, Alan Cox wrote:
> > > Add the ioctls and values needed for this to the ARM26/ARM32 ports. The
> > > actual code has been in the base kernel for a while and automatically
> > > turns on when a port sets the required defines.
> > 
> > Did you forget to provide a termios2 structure for ARM?  Hope you
> > remembered for the other arches:
> > 
> > drivers/char/tty_ioctl.c: In function `set_termios':
> > drivers/char/tty_ioctl.c:429: error: invalid application of `sizeof' to incomplete type `termios2'
> > drivers/char/tty_ioctl.c: In function `n_tty_ioctl':
> > drivers/char/tty_ioctl.c:732: error: invalid application of `sizeof' to incomplete type `termios2'
> > 
> > Will copy the i386 version into ARM's termbits.h
> > 
> 
> I think Alan's per-arch patch was dependent upon an earlier patch.  He
> fooled everyone by sending them out in random order, without sequence
> nnumbers and without telling anyone the dependencies.  And the add-termios2
> patch was a single megapatch whereas the enablement patches were a per-arch
> sprinkle.
> 
> 
> Here's tha arm bit of
> lots-of-architectures-enable-arbitary-speed-tty-support.patch: 

I hope that isn't because it's wrong.  It's "termios2" not "termios_2".

> Or you can just drop this patch and I'll resend it once
> lots-of-architectures-enable-arbitary-speed-tty-support.patch is merged up.

I think I'd prefer to commit my own version copied from x86, which at
least passes a build test.  Doing that now.

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

Re: [PATCH] ARM/ARM26: Enable arbitary speed tty ioctls and split input/output speed

[Alan Cox]

> I think Alan's per-arch patch was dependent upon an earlier patch.  He
> fooled everyone by sending them out in random order, without sequence

Sorry about that - I forgot to do it initially so it didn't get ordered
sanely

> +struct termios_2 {

struct termios2

so I also sneaked in a typo. Clearly I need a holiday


Alan

[SERIAL] Don't optimise away baud rate changes when BOTHER is used

[David Woodhouse]

The uart_set_termios() function will bail out early without bothering to
touch the hardware, if it decides that nothing "relevant" has changed.
Unfortunately, its idea of "relevant" doesn't include c_[io]speed. So if
the baud rate bits are BOTHER and you just change the speed, the change
gets optimised away.

This patch makes it ignore the old Bfoo bits in c_cflag and just check
whether c_ispeed and c_ospeed have changed. Those integers are always
set appropriately for us by set_termios().

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index e8be3b5..a81fc08 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1146,13 +1146,23 @@ static void uart_set_termios(struct tty_struct *tty, struct ktermios *old_termio
 
 	/*
 	 * These are the bits that are used to setup various
-	 * flags in the low level driver.
+	 * flags in the low level driver. We can ignore the Bfoo
+	 * bits in c_cflag; c_[io]speed will always be set
+	 * appropriately by set_termios() in tty_ioctl.c
 	 */
+#ifdef CIBAUD
+#define RELEVANT_CFLAG(cflag)	((cflag) & ~(CIBAUD|CBAUD)
+#else
+#define RELEVANT_CFLAG(cflag)	((cflag) & ~(CBAUD)
+#endif
 #define RELEVANT_IFLAG(iflag)	((iflag) & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK))
-
 	if ((cflag ^ old_termios->c_cflag) == 0 &&
-	    RELEVANT_IFLAG(tty->termios->c_iflag ^ old_termios->c_iflag) == 0)
+	    tty->termios->c_ospeed == old_termios->c_ospeed &&
+	    tty->termios->c_ispeed == old_termios->c_ispeed &&
+	    RELEVANT_IFLAG(tty->termios->c_iflag ^ old_termios->c_iflag) == 0) {
+		printk("No relevant change\n");
 		return;
+	}
 
 	uart_change_speed(state, old_termios);
 

-- 
dwmw2

Re: [PATCH] $ARCH: Enable arbitary speed tty ioctls and split input/output speed

[David Woodhouse]

On Mon, 2007-05-28 at 14:47 +1000, Paul Mackerras wrote:
> Sure, we can add them.  I suggest IBSHIFT = 16 and CIBAUD = 0xff0000
> (or 077600000 if we are going to continue the peculiar convention of
> using octal for the C* constants). 

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/include/asm-powerpc/ioctls.h b/include/asm-powerpc/ioctls.h
index 279a622..b6dfe7f 100644
--- a/include/asm-powerpc/ioctls.h
+++ b/include/asm-powerpc/ioctls.h
@@ -31,6 +31,11 @@
 #define TCXONC		_IO('t', 30)
 #define TCFLSH		_IO('t', 31)
 
+#define TCGETS2		_IOR('t', 32, struct termios2)
+#define TCSETS2		_IOW('t', 33, struct termios2)
+#define TCSETSW2	_IOW('t', 34, struct termios2)
+#define TCSETSF2	_IOW('t', 35, struct termios2)
+
 #define TIOCSWINSZ	_IOW('t', 103, struct winsize)
 #define TIOCGWINSZ	_IOR('t', 104, struct winsize)
 #define	TIOCSTART	_IO('t', 110)		/* start output, like ^Q */
diff --git a/include/asm-powerpc/termbits.h b/include/asm-powerpc/termbits.h
index 5e79198..a5a87f0 100644
--- a/include/asm-powerpc/termbits.h
+++ b/include/asm-powerpc/termbits.h
@@ -43,6 +43,19 @@ struct ktermios {
 	speed_t c_ospeed;		/* output speed */
 };
 
+/* Yay. A third identical definition of the same structure. */
+
+struct termios2 {
+	tcflag_t c_iflag;		/* input mode flags */
+	tcflag_t c_oflag;		/* output mode flags */
+	tcflag_t c_cflag;		/* control mode flags */
+	tcflag_t c_lflag;		/* local mode flags */
+	cc_t c_cc[NCCS];		/* control characters */
+	cc_t c_line;			/* line discipline (== c_cc[19]) */
+	speed_t c_ispeed;		/* input speed */
+	speed_t c_ospeed;		/* output speed */
+};
+
 /* c_cc characters */
 #define VINTR 	         0
 #define VQUIT 	         1
@@ -152,6 +165,10 @@ struct ktermios {
 #define B3000000  00034
 #define B3500000  00035
 #define B4000000  00036
+#define   BOTHER  00037
+
+#define CIBAUD	077600000
+#define IBSHIFT	16		/* Shift from CBAUD to CIBAUD */
 
 #define CSIZE	00001400
 #define   CS5	00000000
diff --git a/include/asm-powerpc/termios.h b/include/asm-powerpc/termios.h
index 2c14fea..2841fb1 100644
--- a/include/asm-powerpc/termios.h
+++ b/include/asm-powerpc/termios.h
@@ -80,6 +80,9 @@ struct termio {
 
 #include <asm-generic/termios.h>
 
+#define user_termios_to_kernel_termios_1(k, u) copy_from_user(k, u, sizeof(struct termios))
+#define kernel_termios_to_user_termios_1(u, k) copy_to_user(u, k, sizeof(struct termios))
+
 #endif	/* __KERNEL__ */
 
 #endif	/* _ASM_POWERPC_TERMIOS_H */


-- 
dwmw2

[PATCH] Minor cleanups in tty_ioctl.c

[David Woodhouse]

Cosmetic stuff which annoyed me while I was poking at it...

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
index fd471cb..79e9c17 100644
--- a/drivers/char/tty_ioctl.c
+++ b/drivers/char/tty_ioctl.c
@@ -276,8 +276,8 @@ void tty_termios_encode_baud_rate(struct ktermios *termios, speed_t ibaud, speed
 				termios->c_cflag |= (baud_bits[i] << IBSHIFT);
 			ifound = i;
 		}
-	}
-	while(++i < n_baud_table);
+	} while(++i < n_baud_table);
+
 	if (ofound == -1)
 		termios->c_cflag |= BOTHER;
 	/* Set exact input bits only if the input and output differ or the
@@ -437,7 +437,7 @@ static int set_termios(struct tty_struct * tty, void __user *arg, int opt)
 #endif
 
 	/* If old style Bfoo values are used then load c_ispeed/c_ospeed with the real speed
-	   so its unconditionally usable */
+	   so it's unconditionally usable */
 	tmp_termios.c_ispeed = tty_termios_input_baud_rate(&tmp_termios);
 	tmp_termios.c_ospeed = tty_termios_baud_rate(&tmp_termios);
 

-- 
dwmw2

Re: [PATCH] Minor cleanups in tty_ioctl.c

[Jiri Slaby]

David Woodhouse napsal(a):
> Cosmetic stuff which annoyed me while I was poking at it...
> 
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
> 
> diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
> index fd471cb..79e9c17 100644
> --- a/drivers/char/tty_ioctl.c
> +++ b/drivers/char/tty_ioctl.c
> @@ -276,8 +276,8 @@ void tty_termios_encode_baud_rate(struct ktermios *termios, speed_t ibaud, speed
>  				termios->c_cflag |= (baud_bits[i] << IBSHIFT);
>  			ifound = i;
>  		}
> -	}
> -	while(++i < n_baud_table);
> +	} while(++i < n_baud_table);
> +
>  	if (ofound == -1)
>  		termios->c_cflag |= BOTHER;
>  	/* Set exact input bits only if the input and output differ or the

I think, you use old version of tty_ioctl.c :)

http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc4/2.6.22-rc4-mm1/broken-out/char-tty_ioctl-little-whitespace-cleanup.patch

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[Alan Cox]

On Wed, 06 Jun 2007 13:30:10 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> The uart_set_termios() function will bail out early without bothering to
> touch the hardware, if it decides that nothing "relevant" has changed.
> Unfortunately, its idea of "relevant" doesn't include c_[io]speed. So if
> the baud rate bits are BOTHER and you just change the speed, the change
> gets optimised away.
> 
> This patch makes it ignore the old Bfoo bits in c_cflag and just check
> whether c_ispeed and c_ospeed have changed. Those integers are always
> set appropriately for us by set_termios().

Yep - and there are some other changes needed as well once everyone gets
their ports properly lined up (notably handing back the actual speed).

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

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[David Woodhouse]

On Wed, 2007-06-06 at 16:03 +0100, Alan Cox wrote:
> Yep - and there are some other changes needed as well once everyone
> gets their ports properly lined up (notably handing back the actual
> speed). 

Yeah, probably. This was was required just to get the speed thing to
pass basic testing though.

You earlier made a cryptic comment about asm-generic and said the
PowerPC patch wouldn't work -- I didn't understand, and it doesn't seem
to be empirically confirmed. Can you eludicate?

-- 
dwmw2

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[Alan Cox]

On Wed, 06 Jun 2007 18:29:58 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> On Wed, 2007-06-06 at 16:03 +0100, Alan Cox wrote:
> > Yep - and there are some other changes needed as well once everyone
> > gets their ports properly lined up (notably handing back the actual
> > speed). 
> 
> Yeah, probably. This was was required just to get the speed thing to
> pass basic testing though.
> 
> You earlier made a cryptic comment about asm-generic and said the
> PowerPC patch wouldn't work -- I didn't understand, and it doesn't seem
> to be empirically confirmed. Can you eludicate?

If your termios and termios2 structures differ in size then you need to
copy the right number of bytes or you won't get speed values into the
kernel. If they are the same size it wont matter.

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[David Woodhouse]

On Thu, 2007-06-07 at 16:38 +0100, Alan Cox wrote:
> If your termios and termios2 structures differ in size then you need to
> copy the right number of bytes or you won't get speed values into the
> kernel. If they are the same size it wont matter.

+/* Yay. A third identical definition of the same structure. */
+struct termios2 {

-- 
dwmw2

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[Alan Cox]

On Thu, 07 Jun 2007 16:50:21 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> On Thu, 2007-06-07 at 16:38 +0100, Alan Cox wrote:
> > If your termios and termios2 structures differ in size then you need to
> > copy the right number of bytes or you won't get speed values into the
> > kernel. If they are the same size it wont matter.
> 
> +/* Yay. A third identical definition of the same structure. */
> +struct termios2 {


Umm if your struct termios has the c_ispeed/c_ospeed fields then you
don't need to add the new ioctls to the PPC either - the Alpha is the
same here. 

Alan

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[David Woodhouse]

On Thu, 2007-06-07 at 22:55 +0100, Alan Cox wrote:
> Umm if your struct termios has the c_ispeed/c_ospeed fields then you
> don't need to add the new ioctls to the PPC either - the Alpha is the
> same here. 

Ah, OK. I hadn't previously noticed that setting TERMIOS_OLD only
actually affected the copy to/from user. I'll send a new patch in the
morning.

-- 
dwmw2

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[David Woodhouse]

On Thu, 2007-06-07 at 22:55 +0100, Alan Cox wrote:
> Umm if your struct termios has the c_ispeed/c_ospeed fields then you
> don't need to add the new ioctls to the PPC either - the Alpha is the
> same here. 

Well, OK -- if it only involves editing patch files I might _send_ a
patch tonight but I'll _test_ it in the morning... I think we only need
this one hunk of what I sent before.

diff --git a/include/asm-powerpc/termbits.h b/include/asm-powerpc/termbits.h
index 5e79198..a5a87f0 100644
--- a/include/asm-powerpc/termbits.h
+++ b/include/asm-powerpc/termbits.h
@@ -152,6 +165,10 @@ struct ktermios {
 #define B3000000  00034
 #define B3500000  00035
 #define B4000000  00036
+#define   BOTHER  00037
+
+#define CIBAUD	077600000
+#define IBSHIFT	16		/* Shift from CBAUD to CIBAUD */
 
 #define CSIZE	00001400
 #define   CS5	00000000

-- 
dwmw2

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[Alan Cox]

On Thu, 07 Jun 2007 23:15:17 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> On Thu, 2007-06-07 at 22:55 +0100, Alan Cox wrote:
> > Umm if your struct termios has the c_ispeed/c_ospeed fields then you
> > don't need to add the new ioctls to the PPC either - the Alpha is the
> > same here. 
> 
> Well, OK -- if it only involves editing patch files I might _send_ a
> patch tonight but I'll _test_ it in the morning... I think we only need
> this one hunk of what I sent before.

If it doesn't only involve editing the header files for this case (and
maybe needing a define to indicate old==new) then the tty layer wants
fixing to sort that out. Its on my todo list.

Alan

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[David Woodhouse]

On Thu, 2007-06-07 at 23:54 +0100, Alan Cox wrote:
> If it doesn't only involve editing the header files for this case (and
> maybe needing a define to indicate old==new) then the tty layer wants
> fixing to sort that out. Its on my todo list. 

It works fine. The only problem is that if I set a _standard_ baud rate
with BOTHER and then read it back with something that doesn't grok
BOTHER, I get it back just as I set it.

[root@pegasos ~]# ./testit1 
Get: c_flag 0x1f0b1f(31,31), ispeed 38400, ospeed 38400
[root@pegasos ~]# stty
speed 0 baud; line = 0;

It might be better if it was returning B38400, rather than BOTHER.
Should we be using tty_termios_encode_baud_rate() for TCGETS()?

-- 
dwmw2

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[David Woodhouse]

On Wed, 2007-06-06 at 13:30 +0100, David Woodhouse wrote:
> +               printk("No relevant change\n");

Oops, that wasn't supposed to sneak into the final patch. I'll send a
new one.

-- 
dwmw2

[SERIAL] Don't optimise away baud rate changes when BOTHER is used (v2)

[David Woodhouse]

The uart_set_termios() function will bail out early without bothering to
touch the hardware, if it decides that nothing "relevant" has changed.
Unfortunately, its idea of "relevant" doesn't include c_[io]speed. So if
the baud rate bits are BOTHER and you just change the speed, the change
gets optimised away.

This patch makes it ignore the old Bfoo bits in c_cflag and just check
whether c_ispeed and c_ospeed have changed. Those integers are always
set appropriately for us by set_termios().

This version of the patch lacks the debugging printk which I
accidentally left in the previous one.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index e8be3b5..4480990 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1146,11 +1146,19 @@ static void uart_set_termios(struct tty_struct *tty, struct ktermios *old_termio
 
 	/*
 	 * These are the bits that are used to setup various
-	 * flags in the low level driver.
+	 * flags in the low level driver. We can ignore the Bfoo
+	 * bits in c_cflag; c_[io]speed will always be set
+	 * appropriately by set_termios() in tty_ioctl.c
 	 */
+#ifdef CIBAUD
+#define RELEVANT_CFLAG(cflag)	((cflag) & ~(CIBAUD|CBAUD)
+#else
+#define RELEVANT_CFLAG(cflag)	((cflag) & ~(CIBAUD|CBAUD)
+#endif
 #define RELEVANT_IFLAG(iflag)	((iflag) & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK))
-
 	if ((cflag ^ old_termios->c_cflag) == 0 &&
+	    tty->termios->c_ospeed == old_termios->c_ospeed &&
+	    tty->termios->c_ispeed == old_termios->c_ispeed &&
 	    RELEVANT_IFLAG(tty->termios->c_iflag ^ old_termios->c_iflag) == 0)
 		return;
 

-- 
dwmw2

[PATCH] PowerPC: Enable arbitary speed tty ioctls and split input/output speed

[David Woodhouse]

Adding the defines/macros activates the existing code in the tty layer
and allows this platform to use the arbitary speed ioctl setting layer

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/include/asm-powerpc/termbits.h b/include/asm-powerpc/termbits.h
index 5e79198..6698188 100644
--- a/include/asm-powerpc/termbits.h
+++ b/include/asm-powerpc/termbits.h
@@ -152,6 +152,10 @@ struct ktermios {
 #define B3000000  00034
 #define B3500000  00035
 #define B4000000  00036
+#define   BOTHER  00037
+
+#define CIBAUD	077600000
+#define IBSHIFT	16		/* Shift from CBAUD to CIBAUD */
 
 #define CSIZE	00001400
 #define   CS5	00000000

-- 
dwmw2

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used (v2)

[Alan Cox]

On Fri, 08 Jun 2007 12:14:49 +0100
David Woodhouse <dwmw2@infradead.org> wrote:

> The uart_set_termios() function will bail out early without bothering to
> touch the hardware, if it decides that nothing "relevant" has changed.
> Unfortunately, its idea of "relevant" doesn't include c_[io]speed. So if
> the baud rate bits are BOTHER and you just change the speed, the change
> gets optimised away.
> 
> This patch makes it ignore the old Bfoo bits in c_cflag and just check
> whether c_ispeed and c_ospeed have changed. Those integers are always
> set appropriately for us by set_termios().
> 
> This version of the patch lacks the debugging printk which I
> accidentally left in the previous one.
> 
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>

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

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[David Woodhouse]

On Fri, 2007-06-08 at 12:48 +0100, Alan Cox wrote:
> > It works fine. The only problem is that if I set a _standard_ baud rate
> > with BOTHER and then read it back with something that doesn't grok
> > BOTHER, I get it back just as I set it.
> 
> That seemed to me to be the right thing to do.
> 
> > It might be better if it was returning B38400, rather than BOTHER.
> > Should we be using tty_termios_encode_baud_rate() for TCGETS()?
> 
> You can't really do that as you get weird behaviour then when people do
> 
> 	tcgetattr
> 		|= BOTHER;
> 		speed = 19200;
> 	tcsetattr
> 
> later in the same app
> 
> 	tcgetattr
> 		speed = 38400
> 	tcsetattr
> 
> knowing that they set BOTHER already.

Hm, true.

> I guess you could add both ioctl sets anyway but the plan longer term is
> for glibc tcsetattr/getattr to do the right thing with the new ioctls in
> all cases, as the glibc interface already provides speed fields.

Even with glibc helping, I'm not sure I see how to do the right thing
for both the 'old' stty and the case you describe. We'll just have to
update stty _if_ we set arbitrary baud rates and want it to display
them.

-- 
dwmw2

Re: [SERIAL] Don't optimise away baud rate changes when BOTHER is used

[Alan Cox]

> It works fine. The only problem is that if I set a _standard_ baud rate
> with BOTHER and then read it back with something that doesn't grok
> BOTHER, I get it back just as I set it.

That seemed to me to be the right thing to do.

> It might be better if it was returning B38400, rather than BOTHER.
> Should we be using tty_termios_encode_baud_rate() for TCGETS()?

You can't really do that as you get weird behaviour then when people do

	tcgetattr
		|= BOTHER;
		speed = 19200;
	tcsetattr

later in the same app

	tcgetattr
		speed = 38400
	tcsetattr

knowing that they set BOTHER already.

I guess you could add both ioctl sets anyway but the plan longer term is
for glibc tcsetattr/getattr to do the right thing with the new ioctls in
all cases, as the glibc interface already provides speed fields.