2007-05-23 16:23:43

by Alan

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

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 <[email protected]>

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__ */


2007-05-23 16:27:26

by Russell King

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

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 <[email protected]>
>
> 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 <[email protected]>

for the above. The ARM26 stuff needs acking by Ian Molton, <[email protected]>

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

2007-05-24 13:08:57

by David Woodhouse

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

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

2007-05-24 13:36:38

by Alan

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

On Thu, 24 May 2007 09:08:55 -0400
David Woodhouse <[email protected]> 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

2007-05-24 13:45:36

by David Woodhouse

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

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

2007-05-24 15:01:50

by Alan

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

On Thu, 24 May 2007 09:45:37 -0400
David Woodhouse <[email protected]> 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

2007-05-24 15:18:21

by David Woodhouse

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

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

2007-05-28 04:47:57

by Paul Mackerras

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

Alan Cox writes:

> On Thu, 24 May 2007 09:45:37 -0400
> David Woodhouse <[email protected]> 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.

2007-05-28 19:37:23

by Russell King

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

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:

2007-05-28 21:56:55

by Andrew Morton

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

On Mon, 28 May 2007 20:36:58 +0100 Russell King <[email protected]> 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.

2007-05-28 22:01:18

by Russell King

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

On Mon, May 28, 2007 at 02:56:32PM -0700, Andrew Morton wrote:
> On Mon, 28 May 2007 20:36:58 +0100 Russell King <[email protected]> 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:

2007-05-29 08:26:39

by Alan

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

> 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

2007-06-06 12:30:29

by David Woodhouse

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

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 <[email protected]>

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

2007-06-06 12:33:21

by David Woodhouse

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

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 <[email protected]>

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

2007-06-06 12:34:44

by David Woodhouse

[permalink] [raw]
Subject: [PATCH] Minor cleanups in tty_ioctl.c

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

Signed-off-by: David Woodhouse <[email protected]>

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

2007-06-06 13:09:49

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] Minor cleanups in tty_ioctl.c

David Woodhouse napsal(a):
> Cosmetic stuff which annoyed me while I was poking at it...
>
> Signed-off-by: David Woodhouse <[email protected]>
>
> 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

2007-06-06 14:58:20

by Alan

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

On Wed, 06 Jun 2007 13:30:10 +0100
David Woodhouse <[email protected]> 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 <[email protected]>

2007-06-06 17:30:31

by David Woodhouse

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

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

2007-06-07 15:33:17

by Alan

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

On Wed, 06 Jun 2007 18:29:58 +0100
David Woodhouse <[email protected]> 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.

2007-06-07 15:51:44

by David Woodhouse

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

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

2007-06-07 21:50:34

by Alan

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

On Thu, 07 Jun 2007 16:50:21 +0100
David Woodhouse <[email protected]> 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

2007-06-07 22:07:21

by David Woodhouse

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

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

2007-06-07 22:15:33

by David Woodhouse

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

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

2007-06-07 22:49:53

by Alan

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

On Thu, 07 Jun 2007 23:15:17 +0100
David Woodhouse <[email protected]> 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

2007-06-08 11:01:22

by David Woodhouse

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

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

2007-06-08 11:14:01

by David Woodhouse

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

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

2007-06-08 11:15:07

by David Woodhouse

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

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 <[email protected]>

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

2007-06-08 11:19:07

by David Woodhouse

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

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 <[email protected]>

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

2007-06-08 11:38:13

by Alan

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

On Fri, 08 Jun 2007 12:14:49 +0100
David Woodhouse <[email protected]> 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 <[email protected]>

Acked-by: Alan Cox <[email protected]>

2007-06-08 11:43:51

by Alan

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

> 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.

2007-06-08 11:48:46

by David Woodhouse

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

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