[PATCH] tty: fix unused warning when TCGETX is not defined

[PATCH] tty: fix unused warning when TCGETX is not defined

[Mike Frysinger]

If TCGETX is not defined, we end up with this warning:
drivers/char/tty_ioctl.c: In function ‘tty_mode_ioctl’:
drivers/char/tty_ioctl.c:950: warning: unused variable ‘ktermx’

Since the variable is only used in one case statement, push it down to
the local case scope.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/char/tty_ioctl.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tty_ioctl.c b/drivers/char/tty_ioctl.c
index 8116bb1..b24f6c6 100644
--- a/drivers/char/tty_ioctl.c
+++ b/drivers/char/tty_ioctl.c
@@ -947,7 +947,6 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
 	void __user *p = (void __user *)arg;
 	int ret = 0;
 	struct ktermios kterm;
-	struct termiox ktermx;
 
 	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
 	    tty->driver->subtype == PTY_TYPE_MASTER)
@@ -1049,7 +1048,8 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
 		return ret;
 #endif
 #ifdef TCGETX
-	case TCGETX:
+	case TCGETX: {
+		struct termiox ktermx;
 		if (real_tty->termiox == NULL)
 			return -EINVAL;
 		mutex_lock(&real_tty->termios_mutex);
@@ -1058,6 +1058,7 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file,
 		if (copy_to_user(p, &ktermx, sizeof(struct termiox)))
 			ret = -EFAULT;
 		return ret;
+	}
 	case TCSETX:
 		return set_termiox(real_tty, p, 0);
 	case TCSETXW:
-- 
1.6.3.1

Re: [PATCH] tty: fix unused warning when TCGETX is not defined

[Alan Cox]

On Fri, 12 Jun 2009 06:27:48 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> If TCGETX is not defined, we end up with this warning:
> drivers/char/tty_ioctl.c: In function ‘tty_mode_ioctl’:
> drivers/char/tty_ioctl.c:950: warning: unused variable ‘ktermx’
> 
> Since the variable is only used in one case statement, push it down to
> the local case scope.

If I wasn't so nice I'd just make it and the lack of BOTHER definitions
on platforms error. Really there shouldn't be anyone without the features
defined ;)

Alan

Re: [PATCH] tty: fix unused warning when TCGETX is not defined

[Mike Frysinger]

On Fri, Jun 12, 2009 at 06:33, Alan Cox wrote:
> On Fri, 12 Jun 2009 06:27:48 -0400 Mike Frysinger wrote:
>> If TCGETX is not defined, we end up with this warning:
>> drivers/char/tty_ioctl.c: In function ‘tty_mode_ioctl’:
>> drivers/char/tty_ioctl.c:950: warning: unused variable ‘ktermx’
>>
>> Since the variable is only used in one case statement, push it down to
>> the local case scope.
>
> If I wasn't so nice I'd just make it and the lack of BOTHER definitions
> on platforms error. Really there shouldn't be anyone without the features
> defined ;)

if i knew a lick about these extended tty pieces, i'd look at hooking them up

are these really arch specific ?
-mike

Re: [PATCH] tty: fix unused warning when TCGETX is not defined

[Alan Cox]

> > If I wasn't so nice I'd just make it and the lack of BOTHER definitions
> > on platforms error. Really there shouldn't be anyone without the features
> > defined ;)
> 
> if i knew a lick about these extended tty pieces, i'd look at hooking them up
> 
> are these really arch specific ?

The ioctl numbers have to be (although most platforms use the same
values), and the "BOTHER" definition for arbitary baud rates depends on
the format of struct termios - which again varies by architecture. Its
usually the case that CBAUDEX|0 isn't used for anything so we use that
for BOTHER.

Other than the numbering they are not arch specific, so just pick the
constants for the platform.

Alan

Re: [PATCH] tty: fix unused warning when TCGETX is not defined

[Mike Frysinger]

On Fri, Jun 12, 2009 at 06:38, Alan Cox wrote:
>> > If I wasn't so nice I'd just make it and the lack of BOTHER definitions
>> > on platforms error. Really there shouldn't be anyone without the features
>> > defined ;)
>>
>> if i knew a lick about these extended tty pieces, i'd look at hooking them up
>>
>> are these really arch specific ?
>
> The ioctl numbers have to be (although most platforms use the same
> values), and the "BOTHER" definition for arbitary baud rates depends on
> the format of struct termios - which again varies by architecture. Its
> usually the case that CBAUDEX|0 isn't used for anything so we use that
> for BOTHER.

mmm BOTHER is used to get arbitrary speeds right ?  i recall testing
that on Blackfin already so i'm pretty sure that works ...

> Other than the numbering they are not arch specific, so just pick the
> constants for the platform.

the guys who did the original Blackfin arch port simply copied the x86
termios stuff (which actually kind of sucks because it means they
copied the termios2 wart)

wonder if people would get annoyed if i changed the Blackfin headers
to do #include <asm/../../../x86/include/asm/foo.h> ;)

doing a diff between x86 and Blackfin headers shows that
termbits/termios are exact copies (ignoring the #ifdef header
protection) and that the Blackfin ioctls.h is missing:
TIOCGRS485
TIOCSRS485
TCGETX
TCSETX
TCSETXF
TCSETXW
TIOCGHAYESESP
TIOCSHAYESESP
-mike

[PATCH] blackfin: update ioctls.h

[Arnd Bergmann]

The blackfin version of ioctls.h is a copy of the x86 version, just missing
some fixes that were not included in all copies. This change brings it back
in line with the latest version. Once my asm-generic tree gets merged, the
file can be replaced with an '#include <asm-generic/ioctls.h>'.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
On Friday 12 June 2009, Mike Frysinger wrote:
> On Fri, Jun 12, 2009 at 06:33, Alan Cox wrote:
> > On Fri, 12 Jun 2009 06:27:48 -0400 Mike Frysinger wrote:
> >> If TCGETX is not defined, we end up with this warning:
> >> drivers/char/tty_ioctl.c: In function ‘tty_mode_ioctl’:
> >> drivers/char/tty_ioctl.c:950: warning: unused variable ‘ktermx’
> >>
> >> Since the variable is only used in one case statement, push it down to
> >> the local case scope.
> >
> > If I wasn't so nice I'd just make it and the lack of BOTHER definitions
> > on platforms error. Really there shouldn't be anyone without the features
> > defined ;)
> 
> if i knew a lick about these extended tty pieces, i'd look at hooking them up
> 
> are these really arch specific ?

diff --git a/arch/blackfin/include/asm/ioctls.h b/arch/blackfin/include/asm/ioctls.h
index 895e317..3bcc071 100644
--- a/arch/blackfin/include/asm/ioctls.h
+++ b/arch/blackfin/include/asm/ioctls.h
@@ -43,7 +43,7 @@
 #define TIOCSETD	0x5423
 #define TIOCGETD	0x5424
 #define TCSBRKP		0x5425	/* Needed for POSIX tcsendbreak() */
-#define TIOCTTYGSTRUCT	0x5426	/* For debugging only */
+/* #define TIOCTTYGSTRUCT	0x5426	For debugging only */
 #define TIOCSBRK	0x5427	/* BSD compatibility */
 #define TIOCCBRK	0x5428	/* BSD compatibility */
 #define TIOCGSID	0x5429	/* Return the session ID of FD */
@@ -51,11 +51,17 @@
 #define TCSETS2		_IOW('T', 0x2B, struct termios2)
 #define TCSETSW2	_IOW('T', 0x2C, struct termios2)
 #define TCSETSF2	_IOW('T', 0x2D, struct termios2)
-/* Get Pty Number (of pty-mux device) */
-#define TIOCGPTN	_IOR('T', 0x30, unsigned int)
+#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 */
+#define TCGETX		0x5432 /* SYS5 TCGETX compatibility */
+#define TCSETX		0x5433
+#define TCSETXF		0x5434
+#define TCSETXW		0x5435
 
-#define FIONCLEX	0x5450	/* these numbers need to be adjusted. */
+
+#define FIONCLEX	0x5450
 #define FIOCLEX		0x5451
 #define FIOASYNC	0x5452
 #define TIOCSERCONFIG	0x5453

Re: [PATCH] tty: fix unused warning when TCGETX is not defined

[Arnd Bergmann]

On Friday 12 June 2009, Mike Frysinger wrote:
> TIOCGRS485
> TIOCSRS485
> TCGETX
> TCSETX
> TCSETXF
> TCSETXW

These can easily be added, as my patch does.

> TIOCGHAYESESP
> TIOCSHAYESESP

These two are better left out IMHO. TIOCGHAYESESP on half the architectures
has the same number as FIOQSIZE on the other half (including blackfin).
The only driver using it is ESPSERIAL, which is marked as broken and depending
on ISA, so you really don't care.

	Arnd <><

[PATCH] Blackfin: sync termios header changes with x86

[Mike Frysinger]

The Blackfin port copied the x86 termios interface, so pull updates from
it to get the latest termios ioctls and such.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
Arnd: if you don't mind, i'd rather merge this ... i can put it into my
next batch of Blackfin changes for 2.6.31

 arch/blackfin/include/asm/ioctls.h   |   35 ++++++++------
 arch/blackfin/include/asm/termbits.h |   54 +++++++++++-----------
 arch/blackfin/include/asm/termios.h  |   86 +++++++++++++++++++++-------------
 3 files changed, 101 insertions(+), 74 deletions(-)

diff --git a/arch/blackfin/include/asm/ioctls.h b/arch/blackfin/include/asm/ioctls.h
index 895e317..e22c400 100644
--- a/arch/blackfin/include/asm/ioctls.h
+++ b/arch/blackfin/include/asm/ioctls.h
@@ -6,7 +6,7 @@
 /* 0x54 is just a magic number to make these relatively unique ('T') */
 
 #define TCGETS		0x5401
-#define TCSETS		0x5402
+#define TCSETS		0x5402 /* Clashes with SNDCTL_TMR_START sound ioctl */
 #define TCSETSW		0x5403
 #define TCSETSF		0x5404
 #define TCGETA		0x5405
@@ -43,19 +43,25 @@
 #define TIOCSETD	0x5423
 #define TIOCGETD	0x5424
 #define TCSBRKP		0x5425	/* Needed for POSIX tcsendbreak() */
-#define TIOCTTYGSTRUCT	0x5426	/* For debugging only */
-#define TIOCSBRK	0x5427	/* BSD compatibility */
-#define TIOCCBRK	0x5428	/* BSD compatibility */
-#define TIOCGSID	0x5429	/* Return the session ID of FD */
+/* #define TIOCTTYGSTRUCT 0x5426 - Former debugging-only ioctl */
+#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)
-/* Get Pty Number (of pty-mux device) */
+#define TIOCGRS485	0x542E
+#define TIOCSRS485	0x542F
 #define TIOCGPTN	_IOR('T', 0x30, unsigned int)
-#define TIOCSPTLCK	_IOW('T', 0x31, int)	/* Lock/unlock Pty */
+				/* Get Pty Number (of pty-mux device) */
+#define TIOCSPTLCK	_IOW('T', 0x31, int)  /* Lock/unlock Pty */
+#define TCGETX		0x5432 /* SYS5 TCGETX compatibility */
+#define TCSETX		0x5433
+#define TCSETXF		0x5434
+#define TCSETXW		0x5435
 
-#define FIONCLEX	0x5450	/* these numbers need to be adjusted. */
+#define FIONCLEX	0x5450
 #define FIOCLEX		0x5451
 #define FIOASYNC	0x5452
 #define TIOCSERCONFIG	0x5453
@@ -63,15 +69,16 @@
 #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 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 FIOQSIZE	0x545E
+#define TIOCGHAYESESP   0x545E  /* Get Hayes ESP configuration */
+#define TIOCSHAYESESP   0x545F  /* Set Hayes ESP configuration */
+#define FIOQSIZE	0x5460
 
 /* Used for packet mode */
 #define TIOCPKT_DATA		 0
diff --git a/arch/blackfin/include/asm/termbits.h b/arch/blackfin/include/asm/termbits.h
index f37feb7..faa569f 100644
--- a/arch/blackfin/include/asm/termbits.h
+++ b/arch/blackfin/include/asm/termbits.h
@@ -3,40 +3,40 @@
 
 #include <linux/posix_types.h>
 
-typedef unsigned char cc_t;
-typedef unsigned int speed_t;
-typedef unsigned int tcflag_t;
+typedef unsigned char	cc_t;
+typedef unsigned int	speed_t;
+typedef unsigned int	tcflag_t;
 
 #define NCCS 19
 struct termios {
-	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 */
+	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 */
 };
 
 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_line;                    /* line discipline */
-	cc_t c_cc[NCCS];                /* control characters */
-	speed_t c_ispeed;               /* input speed */
-	speed_t c_ospeed;               /* output speed */
+	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 */
-	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 */
+	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 */
 };
 
 /* c_cc characters */
@@ -140,7 +140,7 @@ struct ktermios {
 #define HUPCL	0002000
 #define CLOCAL	0004000
 #define CBAUDEX 0010000
-#define BOTHER	0010000
+#define	   BOTHER 0010000		/* non standard rate */
 #define    B57600 0010001
 #define   B115200 0010002
 #define   B230400 0010003
@@ -160,7 +160,7 @@ struct ktermios {
 #define CMSPAR	  010000000000	/* mark or space (stick) parity */
 #define CRTSCTS	  020000000000	/* flow control */
 
-#define IBSHIFT	16	/* Shift from CBAUD to CIBAUD */
+#define IBSHIFT	  16		/* Shift from CBAUD to CIBAUD */
 
 /* c_lflag bits */
 #define ISIG	0000001
diff --git a/arch/blackfin/include/asm/termios.h b/arch/blackfin/include/asm/termios.h
index d50d063..a5f2f13 100644
--- a/arch/blackfin/include/asm/termios.h
+++ b/arch/blackfin/include/asm/termios.h
@@ -13,11 +13,11 @@ struct winsize {
 
 #define NCC 8
 struct termio {
-	unsigned short c_iflag;	/* input mode flags */
-	unsigned short c_oflag;	/* output mode flags */
-	unsigned short c_cflag;	/* control mode flags */
-	unsigned short c_lflag;	/* local mode flags */
-	unsigned char c_line;	/* line discipline */
+	unsigned short c_iflag;		/* input mode flags */
+	unsigned short c_oflag;		/* output mode flags */
+	unsigned short c_cflag;		/* control mode flags */
+	unsigned short c_lflag;		/* local mode flags */
+	unsigned char c_line;		/* line discipline */
 	unsigned char c_cc[NCC];	/* control characters */
 };
 
@@ -41,6 +41,8 @@ struct termio {
 
 #ifdef __KERNEL__
 
+#include <asm/uaccess.h>
+
 /*	intr=^C		quit=^\		erase=del	kill=^U
 	eof=^D		vtime=\0	vmin=\1		sxtc=\0
 	start=^Q	stop=^S		susp=^Z		eol=\0
@@ -58,37 +60,55 @@ struct termio {
 	*(unsigned short *) &(termios)->x = __tmp; \
 }
 
-#define user_termio_to_kernel_termios(termios, termio) \
-({ \
-	SET_LOW_TERMIOS_BITS(termios, termio, c_iflag); \
-	SET_LOW_TERMIOS_BITS(termios, termio, c_oflag); \
-	SET_LOW_TERMIOS_BITS(termios, termio, c_cflag); \
-	SET_LOW_TERMIOS_BITS(termios, termio, c_lflag); \
-	copy_from_user((termios)->c_cc, (termio)->c_cc, NCC); \
-})
+static inline int user_termio_to_kernel_termios(struct ktermios *termios,
+						struct termio __user *termio)
+{
+	SET_LOW_TERMIOS_BITS(termios, termio, c_iflag);
+	SET_LOW_TERMIOS_BITS(termios, termio, c_oflag);
+	SET_LOW_TERMIOS_BITS(termios, termio, c_cflag);
+	SET_LOW_TERMIOS_BITS(termios, termio, c_lflag);
+	get_user(termios->c_line, &termio->c_line);
+	return copy_from_user(termios->c_cc, termio->c_cc, NCC);
+}
 
 /*
  * Translate a "termios" structure into a "termio". Ugh.
  */
-#define kernel_termios_to_user_termio(termio, termios) \
-({ \
-	put_user((termios)->c_iflag, &(termio)->c_iflag); \
-	put_user((termios)->c_oflag, &(termio)->c_oflag); \
-	put_user((termios)->c_cflag, &(termio)->c_cflag); \
-	put_user((termios)->c_lflag, &(termio)->c_lflag); \
-	put_user((termios)->c_line,  &(termio)->c_line); \
-	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 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__ */
+static inline int kernel_termios_to_user_termio(struct termio __user *termio,
+					    struct ktermios *termios)
+{
+	put_user((termios)->c_iflag, &(termio)->c_iflag);
+	put_user((termios)->c_oflag, &(termio)->c_oflag);
+	put_user((termios)->c_cflag, &(termio)->c_cflag);
+	put_user((termios)->c_lflag, &(termio)->c_lflag);
+	put_user((termios)->c_line,  &(termio)->c_line);
+	return copy_to_user((termio)->c_cc, (termios)->c_cc, NCC);
+}
+
+static inline int user_termios_to_kernel_termios(struct ktermios *k,
+						 struct termios2 __user *u)
+{
+	return copy_from_user(k, u, sizeof(struct termios2));
+}
+
+static inline int kernel_termios_to_user_termios(struct termios2 __user *u,
+						 struct ktermios *k)
+{
+	return copy_to_user(u, k, sizeof(struct termios2));
+}
+
+static inline int user_termios_to_kernel_termios_1(struct ktermios *k,
+						   struct termios __user *u)
+{
+	return copy_from_user(k, u, sizeof(struct termios));
+}
+
+static inline int kernel_termios_to_user_termios_1(struct termios __user *u,
+						   struct ktermios *k)
+{
+	return copy_to_user(u, k, sizeof(struct termios));
+}
+
+#endif	/* __KERNEL__ */
 
 #endif				/* __BFIN_TERMIOS_H__ */
-- 
1.6.3.1

Re: [PATCH] Blackfin: sync termios header changes with x86

[Arnd Bergmann]

On Friday 12 June 2009, Mike Frysinger wrote:

> Arnd: if you don't mind, i'd rather merge this ... i can put it into my
> next batch of Blackfin changes for 2.6.31

I don't see how that helps, because:
 
>  #define TIOCMIWAIT	0x545C	/* wait for a change on serial input line(s) */
>  #define TIOCGICOUNT	0x545D	/* read serial port inline interrupt counts */
> -
> -#define FIOQSIZE	0x545E
> +#define TIOCGHAYESESP   0x545E  /* Get Hayes ESP configuration */
> +#define TIOCSHAYESESP   0x545F  /* Set Hayes ESP configuration */
> +#define FIOQSIZE	0x5460

This breaks existing applications using FIOQSIZE. You really shouldn't do that.

> diff --git a/arch/blackfin/include/asm/termbits.h b/arch/blackfin/include/asm/termbits.h
> index f37feb7..faa569f 100644
> --- a/arch/blackfin/include/asm/termbits.h
> +++ b/arch/blackfin/include/asm/termbits.h

This one only changes whitespace, what is the point?

> --- a/arch/blackfin/include/asm/termios.h
> +++ b/arch/blackfin/include/asm/termios.h

This change fixes one bug but not another:

> @@ -58,37 +60,55 @@ struct termio {
>  	*(unsigned short *) &(termios)->x = __tmp; \
>  }
>  
> -#define user_termio_to_kernel_termios(termios, termio) \
> -({ \
> -	SET_LOW_TERMIOS_BITS(termios, termio, c_iflag); \
> -	SET_LOW_TERMIOS_BITS(termios, termio, c_oflag); \
> -	SET_LOW_TERMIOS_BITS(termios, termio, c_cflag); \
> -	SET_LOW_TERMIOS_BITS(termios, termio, c_lflag); \
> -	copy_from_user((termios)->c_cc, (termio)->c_cc, NCC); \
> -})
> +static inline int user_termio_to_kernel_termios(struct ktermios *termios,
> +						struct termio __user *termio)
> +{
> +	SET_LOW_TERMIOS_BITS(termios, termio, c_iflag);
> +	SET_LOW_TERMIOS_BITS(termios, termio, c_oflag);
> +	SET_LOW_TERMIOS_BITS(termios, termio, c_cflag);
> +	SET_LOW_TERMIOS_BITS(termios, termio, c_lflag);
> +	get_user(termios->c_line, &termio->c_line);
> +	return copy_from_user(termios->c_cc, termio->c_cc, NCC);
> +}

You correctly read termios->c_line now, which was missing previously,
but you still don't check the return value of the get_user and
copy_from_user functions. The code also has a very ugly property
of working only on little-endian architectures (which includes
blackfin AFAICT) but should not serve as an example.

If you want a working version of this file, best copy it from avr32
or frv. Or just wait for the asm-generic version.

	Arnd <><

Re: [PATCH] Blackfin: sync termios header changes with x86

[Mike Frysinger]

On Fri, Jun 12, 2009 at 07:29, Arnd Bergmann wrote:
> On Friday 12 June 2009, Mike Frysinger wrote:
>>  #define TIOCMIWAIT   0x545C  /* wait for a change on serial input line(s) */
>>  #define TIOCGICOUNT  0x545D  /* read serial port inline interrupt counts */
>> -
>> -#define FIOQSIZE     0x545E
>> +#define TIOCGHAYESESP   0x545E  /* Get Hayes ESP configuration */
>> +#define TIOCSHAYESESP   0x545F  /* Set Hayes ESP configuration */
>> +#define FIOQSIZE     0x5460
>
> This breaks existing applications using FIOQSIZE. You really shouldn't do that.

meh, no one uses that anyways ;)

>> diff --git a/arch/blackfin/include/asm/termbits.h b/arch/blackfin/include/asm/termbits.h
>> index f37feb7..faa569f 100644
>> --- a/arch/blackfin/include/asm/termbits.h
>> +++ b/arch/blackfin/include/asm/termbits.h
>
> This one only changes whitespace, what is the point?

makes diffing easier

>> --- a/arch/blackfin/include/asm/termios.h
>> +++ b/arch/blackfin/include/asm/termios.h
>
> This change fixes one bug but not another:

better than none at all !

>> @@ -58,37 +60,55 @@ struct termio {
>>       *(unsigned short *) &(termios)->x = __tmp; \
>>  }
>>
>> -#define user_termio_to_kernel_termios(termios, termio) \
>> -({ \
>> -     SET_LOW_TERMIOS_BITS(termios, termio, c_iflag); \
>> -     SET_LOW_TERMIOS_BITS(termios, termio, c_oflag); \
>> -     SET_LOW_TERMIOS_BITS(termios, termio, c_cflag); \
>> -     SET_LOW_TERMIOS_BITS(termios, termio, c_lflag); \
>> -     copy_from_user((termios)->c_cc, (termio)->c_cc, NCC); \
>> -})
>> +static inline int user_termio_to_kernel_termios(struct ktermios *termios,
>> +                                             struct termio __user *termio)
>> +{
>> +     SET_LOW_TERMIOS_BITS(termios, termio, c_iflag);
>> +     SET_LOW_TERMIOS_BITS(termios, termio, c_oflag);
>> +     SET_LOW_TERMIOS_BITS(termios, termio, c_cflag);
>> +     SET_LOW_TERMIOS_BITS(termios, termio, c_lflag);
>> +     get_user(termios->c_line, &termio->c_line);
>> +     return copy_from_user(termios->c_cc, termio->c_cc, NCC);
>> +}
>
> You correctly read termios->c_line now, which was missing previously,
> but you still don't check the return value of the get_user and
> copy_from_user functions. The code also has a very ugly property
> of working only on little-endian architectures (which includes
> blackfin AFAICT) but should not serve as an example.

Blackfin is LE which means the assumption is fine by me

> If you want a working version of this file, best copy it from avr32
> or frv. Or just wait for the asm-generic version.

is asm-generic going to be in 2.6.31 ?  otherwise i'd prefer to have
Blackfin fixed now rather than 2.6.32+
-mike

Re: [PATCH] Blackfin: sync termios header changes with x86

[Alan Cox]

> -#define FIOQSIZE	0x545E
> +#define TIOCGHAYESESP   0x545E  /* Get Hayes ESP configuration */
> +#define TIOCSHAYESESP   0x545F  /* Set Hayes ESP configuration */
> +#define FIOQSIZE	0x5460

Umm.. probably better to leave FIOQSIZE where it is ;)

The HAYES ones are obsolete for a specific ancient ISA bus driver so can
probably just be left off

Re: [PATCH] tty: fix unused warning when TCGETX is not defined

[Alan Cox]

> mmm BOTHER is used to get arbitrary speeds right ?  i recall testing
> that on Blackfin already so i'm pretty sure that works ...

Blackfin isn't an offender on that one, but a few other people are ;)

Alan

Re: [PATCH] Blackfin: sync termios header changes with x86

[Arnd Bergmann]

On Friday 12 June 2009, Mike Frysinger wrote:
> > If you want a working version of this file, best copy it from avr32
> > or frv. Or just wait for the asm-generic version.
> 
> is asm-generic going to be in 2.6.31 ?  otherwise i'd prefer to have
> Blackfin fixed now rather than 2.6.32+

I certainly hope it does. I've sent the pull request yesterday,
so it's probably still in Linus' queue for trees to merge, unless
he dropped it.

	Arnd <><

Re: [PATCH] Blackfin: sync termios header changes with x86

[Mike Frysinger]

On Fri, Jun 12, 2009 at 17:41, Arnd Bergmann wrote:
> On Friday 12 June 2009, Mike Frysinger wrote:
>> > If you want a working version of this file, best copy it from avr32
>> > or frv. Or just wait for the asm-generic version.
>>
>> is asm-generic going to be in 2.6.31 ?  otherwise i'd prefer to have
>> Blackfin fixed now rather than 2.6.32+
>
> I certainly hope it does. I've sent the pull request yesterday,
> so it's probably still in Linus' queue for trees to merge, unless
> he dropped it.

then i'm fine with fixing the header post your pull by switching to
asm-generic.  thanks!
-mike