Skip to contentSkip to conversation

tty: drop broken alt-speed handling

PATCH In review Johan Hovold 4 participants 18 messages Jun 6, 2017 – Jun 13, 2017

Subsystems (4)
Networking USB TTY layer and serial drivers USB serial
lore
Share & tools
lore

Johan Hovold

author maintainer
#0 lore raw

cover tty: drop broken alt-speed handling

Setting an alt-speed using TIOCSSERIAL and SPD flags has been deprecated since v2.1.69 and has been broken for all tty drivers but serial-core since v3.10 and commit 6865ff222cca ("TTY: do not warn about setting speed via SPD_*") without anyone noticing (for four years).

Instead of reverting the offending commit, lets get rid of this legacy code while adding back a warning about the SPD flags being deprecated to the drivers that once implemented it. Note that drivers implementing SPD_CUST will continue to support using a custom divisor.

Also note that serial-core did not rely on the TTY layer for SPD handling and continues to support it while warning about deprecation (since 2003 at least).

Greg, I suggest you take it all trough the TTY tree even if merging through multiple trees and applying the final patch once everything is upstream would be an option. Also the irda clean up does not depend on the rest of series as the code implementing SPD handling was ifdefed out.

Johan

Johan Hovold (9):
serial: rate limit custom-speed deprecation notice
USB: serial: ftdi_sio: simplify TIOCSSERIAL flag logic
USB: serial: ftdi_sio: remove broken alt-speed handling
tty: simserial: drop unused alt_speed handling
tty: amiserial: drop broken alt-speed support
tty: cyclades: drop broken alt-speed support
tty: rocket: drop broken alt-speed support
tty: ircomm: remove dead and broken ioctl code
tty: drop unused alt_speed from tty_struct

 arch/ia64/hp/sim/simserial.c       |  13 -----
 drivers/tty/amiserial.c            |  23 ++------
 drivers/tty/cyclades.c             |  21 ++++----
 drivers/tty/rocket.c               |  27 +++-------
 drivers/tty/serial/serial_core.c   |   5 +-
 drivers/usb/serial/ftdi_sio.c      |  67 +++++------------------
 include/linux/tty.h                |   1 -
 net/irda/ircomm/ircomm_tty_ioctl.c | 107 +------------------------------------
 8 files changed, 34 insertions(+), 230 deletions(-)
Signature · 1 line
2.13.0
author maintainer
#1 lore raw

PATCH 1/9 serial: rate limit custom-speed deprecation notice

Contrary to what a comment claimed, the ASYNC_SPD flags and custom divisor can be set by a non-privileged user so rate limit the deprecation notice as was intended.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/tty/serial/serial_core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/tty/serial/serial_core.c +2 -3
index 13bfd5dcffce..f534a40aebde 100644
@@ -954,11 +954,10 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
 		    old_custom_divisor != uport->custom_divisor) {
 			/*
 			 * If they're setting up a custom divisor or speed,
-			 * instead of clearing it, then bitch about it. No
-			 * need to rate-limit; it's CAP_SYS_ADMIN only.
+			 * instead of clearing it, then bitch about it.
 			 */
 			if (uport->flags & UPF_SPD_MASK) {
-				dev_notice(uport->dev,
+				dev_notice_ratelimited(uport->dev,
 				       "%s sets custom speed on %s. This is deprecated.\n",
 				      current->comm,
 				      tty_name(port->tty));
Signature · 1 line
2.13.0
author maintainer
#2 lore raw

PATCH 2/9 USB: serial: ftdi_sio: simplify TIOCSSERIAL flag logic

In reply to Johan Hovold

Simplify TIOCSSERIAL flag logic somewhat.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/ftdi_sio.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)
drivers/usb/serial/ftdi_sio.c +5 -8
index aba74f817dc6..df5c45a4b1d7 100644
@@ -1505,8 +1505,7 @@ static int set_serial_info(struct tty_struct *tty,
 	/* Do error checking and permission checking */
 
 	if (!capable(CAP_SYS_ADMIN)) {
-		if (((new_serial.flags & ~ASYNC_USR_MASK) !=
-		     (priv->flags & ~ASYNC_USR_MASK))) {
+		if ((new_serial.flags ^ priv->flags) & ~ASYNC_USR_MASK) {
 			mutex_unlock(&priv->cfg_lock);
 			return -EPERM;
 		}

@@ -1530,8 +1529,7 @@ static int set_serial_info(struct tty_struct *tty,
 check_and_exit:
 	write_latency_timer(port);
 
-	if ((old_priv.flags & ASYNC_SPD_MASK) !=
-	     (priv->flags & ASYNC_SPD_MASK)) {
+	if ((priv->flags ^ old_priv.flags) & ASYNC_SPD_MASK) {
 		if ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_HI)
 			tty->alt_speed = 57600;
 		else if ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_VHI)

@@ -1543,10 +1541,9 @@ static int set_serial_info(struct tty_struct *tty,
 		else
 			tty->alt_speed = 0;
 	}
-	if (((old_priv.flags & ASYNC_SPD_MASK) !=
-	     (priv->flags & ASYNC_SPD_MASK)) ||
-	    (((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST) &&
-	     (old_priv.custom_divisor != priv->custom_divisor))) {
+	if ((priv->flags ^ old_priv.flags) & ASYNC_SPD_MASK ||
+			((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
+			 priv->custom_divisor != old_priv.custom_divisor)) {
 		change_speed(tty, port);
 		mutex_unlock(&priv->cfg_lock);
 	}
Signature · 1 line
2.13.0
author maintainer
#3 lore raw

PATCH 3/9 USB: serial: ftdi_sio: remove broken alt-speed handling

In reply to Johan Hovold

Remove the broken alt_speed code, and warn when trying to set the line speed using TIOCSSERIAL and SPD flags.

The use of SPD flags to set the line speed has been deprecated since v2.1.69 and support for alt_speed (e.g. "warp") has even been removed from TTY core in v3.10 by commit 6865ff222cca ("TTY: do not warn about setting speed via SPD_*"), effectively breaking all driver implementations of this except for serial core.

Also remove the verbose and outdated comment on how to set baud rates.

Note that setting a custom divisor will continue to work with the caveat that 38400 must again be selected every time the divisor is changed since v2.6.24 and commit 669a6db1037e ("USB: ftd_sio: cleanups and updates for new termios work") which started reporting back the actual baud rate used.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/ftdi_sio.c | 56 +++++++------------------------------------
 1 file changed, 9 insertions(+), 47 deletions(-)
drivers/usb/serial/ftdi_sio.c +9 -47
index df5c45a4b1d7..1cec03799cdf 100644
@@ -1244,42 +1244,13 @@ static __u32 get_ftdi_divisor(struct tty_struct *tty,
 	int div_okay = 1;
 	int baud;
 
-	/*
-	 * The logic involved in setting the baudrate can be cleanly split into
-	 * 3 steps.
-	 * 1. Standard baud rates are set in tty->termios->c_cflag
-	 * 2. If these are not enough, you can set any speed using alt_speed as
-	 * follows:
-	 *    - set tty->termios->c_cflag speed to B38400
-	 *    - set your real speed in tty->alt_speed; it gets ignored when
-	 *      alt_speed==0, (or)
-	 *    - call TIOCSSERIAL ioctl with (struct serial_struct) set as
-	 *	follows:
-	 *      flags & ASYNC_SPD_MASK == ASYNC_SPD_[HI, VHI, SHI, WARP],
-	 *	this just sets alt_speed to (HI: 57600, VHI: 115200,
-	 *	SHI: 230400, WARP: 460800)
-	 * ** Steps 1, 2 are done courtesy of tty_get_baud_rate
-	 * 3. You can also set baud rate by setting custom divisor as follows
-	 *    - set tty->termios->c_cflag speed to B38400
-	 *    - call TIOCSSERIAL ioctl with (struct serial_struct) set as
-	 *	follows:
-	 *      o flags & ASYNC_SPD_MASK == ASYNC_SPD_CUST
-	 *      o custom_divisor set to baud_base / your_new_baudrate
-	 * ** Step 3 is done courtesy of code borrowed from serial.c
-	 *    I should really spend some time and separate + move this common
-	 *    code to serial.c, it is replicated in nearly every serial driver
-	 *    you see.
-	 */
-
-	/* 1. Get the baud rate from the tty settings, this observes
-	      alt_speed hack */
-
 	baud = tty_get_baud_rate(tty);
 	dev_dbg(dev, "%s - tty_get_baud_rate reports speed %d\n", __func__, baud);
 
-	/* 2. Observe async-compatible custom_divisor hack, update baudrate
-	   if needed */
-
+	/*
+	 * Observe deprecated async-compatible custom_divisor hack, update
+	 * baudrate if needed.
+	 */
 	if (baud == 38400 &&
 	    ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST) &&
 	     (priv->custom_divisor)) {

@@ -1288,8 +1259,6 @@ static __u32 get_ftdi_divisor(struct tty_struct *tty,
 			__func__, priv->custom_divisor, baud);
 	}
 
-	/* 3. Convert baudrate to device-specific divisor */
-
 	if (!baud)
 		baud = 9600;
 	switch (priv->chip_type) {

@@ -1529,21 +1498,14 @@ static int set_serial_info(struct tty_struct *tty,
 check_and_exit:
 	write_latency_timer(port);
 
-	if ((priv->flags ^ old_priv.flags) & ASYNC_SPD_MASK) {
-		if ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_HI)
-			tty->alt_speed = 57600;
-		else if ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_VHI)
-			tty->alt_speed = 115200;
-		else if ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_SHI)
-			tty->alt_speed = 230400;
-		else if ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_WARP)
-			tty->alt_speed = 460800;
-		else
-			tty->alt_speed = 0;
-	}
 	if ((priv->flags ^ old_priv.flags) & ASYNC_SPD_MASK ||
 			((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
 			 priv->custom_divisor != old_priv.custom_divisor)) {
+
+		/* warn about deprecation unless clearing */
+		if (priv->flags & ASYNC_SPD_MASK)
+			dev_warn_ratelimited(&port->dev, "use of SPD flags is deprecated\n");
+
 		change_speed(tty, port);
 		mutex_unlock(&priv->cfg_lock);
 	}
Signature · 1 line
2.13.0
author maintainer
#4 lore raw

PATCH 4/9 tty: simserial: drop unused alt_speed handling

In reply to Johan Hovold

This driver was setting the deprecated and broken alt_speed based on port flags, but never provided a means to change the flags or to actually change the speed.

Signed-off-by Johan Hovold <[email protected]>
---
 arch/ia64/hp/sim/simserial.c | 13 -------------
 1 file changed, 13 deletions(-)
arch/ia64/hp/sim/simserial.c +0 -13
index de8cba121013..70d52e9bb575 100644
@@ -387,19 +387,6 @@ static int activate(struct tty_port *port, struct tty_struct *tty)
 	}
 
 	state->xmit.head = state->xmit.tail = 0;
-
-	/*
-	 * Set up the tty->alt_speed kludge
-	 */
-	if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_HI)
-		tty->alt_speed = 57600;
-	if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_VHI)
-		tty->alt_speed = 115200;
-	if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_SHI)
-		tty->alt_speed = 230400;
-	if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_WARP)
-		tty->alt_speed = 460800;
-
 errout:
 	local_irq_restore(flags);
 	return retval;
Signature · 1 line
2.13.0
author maintainer
#5 lore raw

PATCH 5/9 tty: amiserial: drop broken alt-speed support

In reply to Johan Hovold

Setting an alt_speed using the ASYNC_SPD flags has been deprecated since v2.1.69, and has been broken since v3.10 and commit 6865ff222cca ("TTY:
do not warn about setting speed via SPD_*") without anyone noticing.

Drop the broken alt-speed handling altogether, and add a ratelimited warning about using TIOCCSERIAL to change speed as being deprecated.

Note that using ASYNC_SPD_CUST is still supported.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/tty/amiserial.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)
drivers/tty/amiserial.c +3 -20
index dea16bb8c46a..9820e20993db 100644
@@ -570,18 +570,6 @@ static int startup(struct tty_struct *tty, struct serial_state *info)
 	info->xmit.head = info->xmit.tail = 0;
 
 	/*
-	 * Set up the tty->alt_speed kludge
-	 */
-	if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_HI)
-		tty->alt_speed = 57600;
-	if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_VHI)
-		tty->alt_speed = 115200;
-	if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_SHI)
-		tty->alt_speed = 230400;
-	if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_WARP)
-		tty->alt_speed = 460800;
-
-	/*
 	 * and set the speed of the serial port
 	 */
 	change_speed(tty, info, NULL);

@@ -1084,14 +1072,9 @@ static int set_serial_info(struct tty_struct *tty, struct serial_state *state,
 check_and_exit:
 	if (tty_port_initialized(port)) {
 		if (change_spd) {
-			if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_HI)
-				tty->alt_speed = 57600;
-			if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_VHI)
-				tty->alt_speed = 115200;
-			if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_SHI)
-				tty->alt_speed = 230400;
-			if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_WARP)
-				tty->alt_speed = 460800;
+			/* warn about deprecation unless clearing */
+			if (new_serial.flags & ASYNC_SPD_MASK)
+				dev_warn_ratelimited(tty->dev, "use of SPD flags is deprecated\n");
 			change_speed(tty, state, NULL);
 		}
 	} else
Signature · 1 line
2.13.0
author maintainer
#6 lore raw

PATCH 6/9 tty: cyclades: drop broken alt-speed support

In reply to Johan Hovold

Setting an alt_speed using the ASYNC_SPD flags has been deprecated since v2.1.69, and has been broken since v3.10 and commit 6865ff222cca ("TTY:
do not warn about setting speed via SPD_*") without anyone noticing.

Drop the broken alt-speed handling altogether, and add a ratelimited warning about using TIOCCSERIAL to to change speed as being deprecated.

Note that using ASYNC_SPD_CUST is still supported.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/tty/cyclades.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)
drivers/tty/cyclades.c +9 -12
index 104f09c58163..d272bc4e7fb5 100644
@@ -1975,18 +1975,6 @@ static void cy_set_line_char(struct cyclades_port *info, struct tty_struct *tty)
 	cflag = tty->termios.c_cflag;
 	iflag = tty->termios.c_iflag;
 
-	/*
-	 * Set up the tty->alt_speed kludge
-	 */
-	if ((info->port.flags & ASYNC_SPD_MASK) == ASYNC_SPD_HI)
-		tty->alt_speed = 57600;
-	if ((info->port.flags & ASYNC_SPD_MASK) == ASYNC_SPD_VHI)
-		tty->alt_speed = 115200;
-	if ((info->port.flags & ASYNC_SPD_MASK) == ASYNC_SPD_SHI)
-		tty->alt_speed = 230400;
-	if ((info->port.flags & ASYNC_SPD_MASK) == ASYNC_SPD_WARP)
-		tty->alt_speed = 460800;
-
 	card = info->card;
 	channel = info->line - card->first_line;
 

@@ -2295,12 +2283,16 @@ cy_set_serial_info(struct cyclades_port *info, struct tty_struct *tty,
 		struct serial_struct __user *new_info)
 {
 	struct serial_struct new_serial;
+	int old_flags;
 	int ret;
 
 	if (copy_from_user(&new_serial, new_info, sizeof(new_serial)))
 		return -EFAULT;
 
 	mutex_lock(&info->port.mutex);
+
+	old_flags = info->port.flags;
+
 	if (!capable(CAP_SYS_ADMIN)) {
 		if (new_serial.close_delay != info->port.close_delay ||
 				new_serial.baud_base != info->baud ||

@@ -2332,6 +2324,11 @@ cy_set_serial_info(struct cyclades_port *info, struct tty_struct *tty,
 
 check_and_exit:
 	if (tty_port_initialized(&info->port)) {
+		if ((new_serial.flags ^ old_flags) & ASYNC_SPD_MASK) {
+			/* warn about deprecation unless clearing */
+			if (new_serial.flags & ASYNC_SPD_MASK)
+				dev_warn_ratelimited(tty->dev, "use of SPD flags is deprecated\n");
+		}
 		cy_set_line_char(info, tty);
 		ret = 0;
 	} else {
Signature · 1 line
2.13.0
author maintainer
#7 lore raw

PATCH 7/9 tty: rocket: drop broken alt-speed support

In reply to Johan Hovold

Setting an alt_speed using the ROCKET_SPD flags has been deprecated since v2.1.69, and has been broken since commit 6865ff222cca ("TTY: do not warn about setting speed via SPD_*") without anyone noticing.

To make things worse commit 6df3526b6649 ("rocket: first pass at termios reporting") in v2.6.25 started reporting back the actual baud rate used, something which also required 38400 to again be set whenever changing a SPD flag.

Drop the broken alt-speed handling altogether, and add a ratelimited warning about using TIOCCSERIAL to change speed as being deprecated.

Note that the rocket driver has never supported using a custom divisor (ASYNC_SPD_CUST equivalent).

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/tty/rocket.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)
drivers/tty/rocket.c +6 -21
index b51a877da986..20d79a6007d5 100644
@@ -947,18 +947,6 @@ static int rp_open(struct tty_struct *tty, struct file *filp)
 
 		tty_port_set_initialized(&info->port, 1);
 
-		/*
-		 * Set up the tty->alt_speed kludge
-		 */
-		if ((info->flags & ROCKET_SPD_MASK) == ROCKET_SPD_HI)
-			tty->alt_speed = 57600;
-		if ((info->flags & ROCKET_SPD_MASK) == ROCKET_SPD_VHI)
-			tty->alt_speed = 115200;
-		if ((info->flags & ROCKET_SPD_MASK) == ROCKET_SPD_SHI)
-			tty->alt_speed = 230400;
-		if ((info->flags & ROCKET_SPD_MASK) == ROCKET_SPD_WARP)
-			tty->alt_speed = 460800;
-
 		configure_r_port(tty, info, NULL);
 		if (C_BAUD(tty)) {
 			sSetDTR(cp);

@@ -1219,23 +1207,20 @@ static int set_config(struct tty_struct *tty, struct r_port *info,
 			return -EPERM;
 		}
 		info->flags = ((info->flags & ~ROCKET_USR_MASK) | (new_serial.flags & ROCKET_USR_MASK));
-		configure_r_port(tty, info, NULL);
 		mutex_unlock(&info->port.mutex);
 		return 0;
 	}
 
+	if ((new_serial.flags ^ info->flags) & ROCKET_SPD_MASK) {
+		/* warn about deprecation, unless clearing */
+		if (new_serial.flags & ROCKET_SPD_MASK)
+			dev_warn_ratelimited(tty->dev, "use of SPD flags is deprecated\n");
+	}
+
 	info->flags = ((info->flags & ~ROCKET_FLAGS) | (new_serial.flags & ROCKET_FLAGS));
 	info->port.close_delay = new_serial.close_delay;
 	info->port.closing_wait = new_serial.closing_wait;
 
-	if ((info->flags & ROCKET_SPD_MASK) == ROCKET_SPD_HI)
-		tty->alt_speed = 57600;
-	if ((info->flags & ROCKET_SPD_MASK) == ROCKET_SPD_VHI)
-		tty->alt_speed = 115200;
-	if ((info->flags & ROCKET_SPD_MASK) == ROCKET_SPD_SHI)
-		tty->alt_speed = 230400;
-	if ((info->flags & ROCKET_SPD_MASK) == ROCKET_SPD_WARP)
-		tty->alt_speed = 460800;
 	mutex_unlock(&info->port.mutex);
 
 	configure_r_port(tty, info, NULL);
Signature · 1 line
2.13.0
author maintainer
#8 lore raw

PATCH 8/9 tty: ircomm: remove dead and broken ioctl code

In reply to Johan Hovold

Remove three ifdefed and broken implementations of TIOCSSERIAL and TIOCGICOUNT, and parity handling in set_termios which had suffered severe bit rot.

Signed-off-by Johan Hovold <[email protected]>
---
 net/irda/ircomm/ircomm_tty_ioctl.c | 107 +------------------------------------
 1 file changed, 1 insertion(+), 106 deletions(-)
net/irda/ircomm/ircomm_tty_ioctl.c +1 -106
index f18070118d05..171c3dee760e 100644
@@ -97,33 +97,7 @@ static void ircomm_tty_change_speed(struct ircomm_tty_cb *self,
 		self->settings.flow_control &= ~IRCOMM_RTS_CTS_IN;
 	}
 	tty_port_set_check_carrier(&self->port, ~cflag & CLOCAL);
-#if 0
-	/*
-	 * Set up parity check flag
-	 */
-
-	if (I_INPCK(self->tty))
-		driver->read_status_mask |= LSR_FE | LSR_PE;
-	if (I_BRKINT(driver->tty) || I_PARMRK(driver->tty))
-		driver->read_status_mask |= LSR_BI;
-
-	/*
-	 * Characters to ignore
-	 */
-	driver->ignore_status_mask = 0;
-	if (I_IGNPAR(driver->tty))
-		driver->ignore_status_mask |= LSR_PE | LSR_FE;
-
-	if (I_IGNBRK(self->tty)) {
-		self->ignore_status_mask |= LSR_BI;
-		/*
-		 * If we're ignore parity and break indicators, ignore
-		 * overruns too. (For real raw support).
-		 */
-		if (I_IGNPAR(self->tty))
-			self->ignore_status_mask |= LSR_OE;
-	}
-#endif
+
 	self->settings.data_format = cval;
 
 	ircomm_param_request(self, IRCOMM_DATA_FORMAT, FALSE);

@@ -271,67 +245,6 @@ static int ircomm_tty_get_serial_info(struct ircomm_tty_cb *self,
 static int ircomm_tty_set_serial_info(struct ircomm_tty_cb *self,
 				      struct serial_struct __user *new_info)
 {
-#if 0
-	struct serial_struct new_serial;
-	struct ircomm_tty_cb old_state, *state;
-
-	if (copy_from_user(&new_serial,new_info,sizeof(new_serial)))
-		return -EFAULT;
-
-
-	state = self
-	old_state = *self;
-
-	if (!capable(CAP_SYS_ADMIN)) {
-		if ((new_serial.baud_base != state->settings.data_rate) ||
-		    (new_serial.close_delay != state->close_delay) ||
-		    ((new_serial.flags & ~ASYNC_USR_MASK) !=
-		     (self->flags & ~ASYNC_USR_MASK)))
-			return -EPERM;
-		state->flags = ((state->flags & ~ASYNC_USR_MASK) |
-				 (new_serial.flags & ASYNC_USR_MASK));
-		self->flags = ((self->flags & ~ASYNC_USR_MASK) |
-			       (new_serial.flags & ASYNC_USR_MASK));
-		/* self->custom_divisor = new_serial.custom_divisor; */
-		goto check_and_exit;
-	}
-
-	/*
-	 * OK, past this point, all the error checking has been done.
-	 * At this point, we start making changes.....
-	 */
-
-	if (self->settings.data_rate != new_serial.baud_base) {
-		self->settings.data_rate = new_serial.baud_base;
-		ircomm_param_request(self, IRCOMM_DATA_RATE, TRUE);
-	}
-
-	self->close_delay = new_serial.close_delay * HZ/100;
-	self->closing_wait = new_serial.closing_wait * HZ/100;
-	/* self->custom_divisor = new_serial.custom_divisor; */
-
-	self->flags = ((self->flags & ~ASYNC_FLAGS) |
-		       (new_serial.flags & ASYNC_FLAGS));
-	self->tty->low_latency = (self->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
-
- check_and_exit:
-
-	if (tty_port_initialized(self)) {
-		if (((old_state.flags & ASYNC_SPD_MASK) !=
-		     (self->flags & ASYNC_SPD_MASK)) ||
-		    (old_driver.custom_divisor != driver->custom_divisor)) {
-			if ((driver->flags & ASYNC_SPD_MASK) == ASYNC_SPD_HI)
-				driver->tty->alt_speed = 57600;
-			if ((driver->flags & ASYNC_SPD_MASK) == ASYNC_SPD_VHI)
-				driver->tty->alt_speed = 115200;
-			if ((driver->flags & ASYNC_SPD_MASK) == ASYNC_SPD_SHI)
-				driver->tty->alt_speed = 230400;
-			if ((driver->flags & ASYNC_SPD_MASK) == ASYNC_SPD_WARP)
-				driver->tty->alt_speed = 460800;
-			ircomm_tty_change_speed(driver);
-		}
-	}
-#endif
 	return 0;
 }
 

@@ -367,24 +280,6 @@ int ircomm_tty_ioctl(struct tty_struct *tty,
 
 	case TIOCGICOUNT:
 		pr_debug("%s(), TIOCGICOUNT not impl!\n", __func__);
-#if 0
-		save_flags(flags); cli();
-		cnow = driver->icount;
-		restore_flags(flags);
-		p_cuser = (struct serial_icounter_struct __user *) arg;
-		if (put_user(cnow.cts, &p_cuser->cts) ||
-		    put_user(cnow.dsr, &p_cuser->dsr) ||
-		    put_user(cnow.rng, &p_cuser->rng) ||
-		    put_user(cnow.dcd, &p_cuser->dcd) ||
-		    put_user(cnow.rx, &p_cuser->rx) ||
-		    put_user(cnow.tx, &p_cuser->tx) ||
-		    put_user(cnow.frame, &p_cuser->frame) ||
-		    put_user(cnow.overrun, &p_cuser->overrun) ||
-		    put_user(cnow.parity, &p_cuser->parity) ||
-		    put_user(cnow.brk, &p_cuser->brk) ||
-		    put_user(cnow.buf_overrun, &p_cuser->buf_overrun))
-			return -EFAULT;
-#endif
 		return 0;
 	default:
 		ret = -ENOIOCTLCMD;  /* ioctls which we must ignore */
Signature · 1 line
2.13.0
#9 lore raw

Re: [PATCH 8/9] tty: ircomm: remove dead and broken ioctl code

Quoted 3 lines · On Tue, 6 Jun 2017 12:54:40 +0200 Johan Hovold <[email protected]> wrote:

Remove three ifdefed and broken implementations of TIOCSSERIAL and TIOCGICOUNT, and parity handling in set_termios which had suffered severe bit rot.

I would be amazed if the IRDA code still works. It's not been tested properly for years and it never followed the tty rules properly in the first place - so this looks good, although moving IRDA into staging and /dev/null would IMHO be far better.

IRDA is dead, and IR remotes are handled via completely different code.

Alan

maintainer
#10 lore raw

Re: [PATCH 8/9] tty: ircomm: remove dead and broken ioctl code

Quoted 7 lines · Alan Cox
Quoted 3 lines · On Tue, 6 Jun 2017 12:54:40 +0200 Johan Hovold <[email protected]> wrote:

Remove three ifdefed and broken implementations of TIOCSSERIAL and TIOCGICOUNT, and parity handling in set_termios which had suffered severe bit rot.

Quoted 6 lines

I would be amazed if the IRDA code still works. It's not been tested properly for years and it never followed the tty rules properly in the first place - so this looks good, although moving IRDA into staging and /dev/null would IMHO be far better.

IRDA is dead, and IR remotes are handled via completely different code.

I agree, and we should move it into staging and out of the tree, I'll send netdev some patches for that...

thanks,

greg k-h

author maintainer
#11 lore raw

Re: [PATCH 8/9] tty: ircomm: remove dead and broken ioctl code

Quoted 13 lines · Greg Kroah-Hartman
Quoted 12 lines · Alan Cox
Quoted 3 lines · On Tue, 6 Jun 2017 12:54:40 +0200 Johan Hovold <[email protected]> wrote:

Remove three ifdefed and broken implementations of TIOCSSERIAL and TIOCGICOUNT, and parity handling in set_termios which had suffered severe bit rot.

I would be amazed if the IRDA code still works. It's not been tested properly for years and it never followed the tty rules properly in the first place - so this looks good, although moving IRDA into staging and /dev/null would IMHO be far better.

Quoted 4 lines
Quoted 1 line

IRDA is dead, and IR remotes are handled via completely different code.

I agree, and we should move it into staging and out of the tree, I'll send netdev some patches for that...

Sounds good, but please consider applying this patch before moving to staging as people have been modifying this dead code as part of kernel-wide updates even though hasn't even compiled for quite some time.

Thanks,
Johan

author maintainer
#12 lore raw

Re: [PATCH 8/9] tty: ircomm: remove dead and broken ioctl code

Quoted 16 lines · Johan Hovold
Quoted 15 lines · Greg Kroah-Hartman
Quoted 13 lines · Alan Cox
Quoted 3 lines · On Tue, 6 Jun 2017 12:54:40 +0200 Johan Hovold <[email protected]> wrote:

Remove three ifdefed and broken implementations of TIOCSSERIAL and TIOCGICOUNT, and parity handling in set_termios which had suffered severe bit rot.

I would be amazed if the IRDA code still works. It's not been tested properly for years and it never followed the tty rules properly in the first place - so this looks good, although moving IRDA into staging and /dev/null would IMHO be far better.

IRDA is dead, and IR remotes are handled via completely different code.

Quoted 7 lines
Quoted 2 lines

I agree, and we should move it into staging and out of the tree, I'll send netdev some patches for that...

Sounds good, but please consider applying this patch before moving to staging as people have been modifying this dead code as part of kernel-wide updates even though hasn't even compiled for quite some time.

That is, just that which you ended up doing. :)

Thanks,
Johan

maintainer
#13 lore raw

Re: [PATCH 8/9] tty: ircomm: remove dead and broken ioctl code

In reply to Johan Hovold
Quoted 16 lines · Johan Hovold
Quoted 15 lines · Greg Kroah-Hartman
Quoted 13 lines · Alan Cox
Quoted 3 lines · On Tue, 6 Jun 2017 12:54:40 +0200 Johan Hovold <[email protected]> wrote:

Remove three ifdefed and broken implementations of TIOCSSERIAL and TIOCGICOUNT, and parity handling in set_termios which had suffered severe bit rot.

I would be amazed if the IRDA code still works. It's not been tested properly for years and it never followed the tty rules properly in the first place - so this looks good, although moving IRDA into staging and /dev/null would IMHO be far better.

IRDA is dead, and IR remotes are handled via completely different code.

Quoted 7 lines
Quoted 2 lines

I agree, and we should move it into staging and out of the tree, I'll send netdev some patches for that...

Sounds good, but please consider applying this patch before moving to staging as people have been modifying this dead code as part of kernel-wide updates even though hasn't even compiled for quite some time.

I already applied it :)

I'll wait for 4.13-rc1 before doing it (if I remember...)

thanks,

greg k-h

author maintainer
#14 lore raw

PATCH 9/9 tty: drop unused alt_speed from tty_struct

In reply to Johan Hovold

Drop the now unused alt_speed field from struct tty_struct.

Setting an alt_speed using the ASYNC_SPD flags has been deprecated since v2.1.69, and has been broken for all tty drivers but serial-core since v3.10 and commit 6865ff222cca ("TTY: do not warn about setting speed via SPD_*") without anyone noticing.

Note that serial-core still supports changing speed using TIOCSSERIAL and SPD flags (including "alt-speeds"), but also warns about it being deprecated since pre-git.

Signed-off-by Johan Hovold <[email protected]>
---
 include/linux/tty.h | 1 -
 1 file changed, 1 deletion(-)
include/linux/tty.h +0 -1
index eccb4ec30a8a..585cf2b5ca94 100644
@@ -316,7 +316,6 @@ struct tty_struct {
 
 	struct tty_struct *link;
 	struct fasync_struct *fasync;
-	int alt_speed;		/* For magic substitution of 38400 bps */
 	wait_queue_head_t write_wait;
 	wait_queue_head_t read_wait;
 	struct work_struct hangup_work;
Signature · 1 line
2.13.0
Reviewed-by on the v1 series
#15 lore raw
In reply to Johan Hovold
Quoted 4 lines · On Tue, 6 Jun 2017 12:54:32 +0200 Johan Hovold <[email protected]> wrote:

Setting an alt-speed using TIOCSSERIAL and SPD flags has been deprecated since v2.1.69 and has been broken for all tty drivers but serial-core since v3.10 and commit 6865ff222cca ("TTY: do not warn about setting speed via SPD_*") without anyone noticing (for four years).

We have the ability to set the speed directly via termios so I doubt anyone is using the remaining hacks either but I guess someone may. All looks good to me.

Reviewed-by Alan Cox <[email protected]>

Alan

Reviewed-by on the v1 series
#16 lore raw
In reply to Johan Hovold
Quoted 14 lines · Johan Hovold

Setting an alt-speed using TIOCSSERIAL and SPD flags has been deprecated since v2.1.69 and has been broken for all tty drivers but serial-core since v3.10 and commit 6865ff222cca ("TTY: do not warn about setting speed via SPD_*") without anyone noticing (for four years).

Instead of reverting the offending commit, lets get rid of this legacy code while adding back a warning about the SPD flags being deprecated to the drivers that once implemented it. Note that drivers implementing SPD_CUST will continue to support using a custom divisor.

Also note that serial-core did not rely on the TTY layer for SPD handling and continues to support it while warning about deprecation (since 2003 at least).

Quoted 5 lines

Greg, I suggest you take it all trough the TTY tree even if merging through multiple trees and applying the final patch once everything is upstream would be an option. Also the irda clean up does not depend on the rest of series as the code implementing SPD handling was ifdefed out.

What a nice clean up!

Reviewed-by Andy Shevchenko <[email protected]>
Quoted 28 lines

Johan

Johan Hovold (9):
serial: rate limit custom-speed deprecation notice
USB: serial: ftdi_sio: simplify TIOCSSERIAL flag logic
USB: serial: ftdi_sio: remove broken alt-speed handling
tty: simserial: drop unused alt_speed handling
tty: amiserial: drop broken alt-speed support
tty: cyclades: drop broken alt-speed support
tty: rocket: drop broken alt-speed support
tty: ircomm: remove dead and broken ioctl code
tty: drop unused alt_speed from tty_struct

 arch/ia64/hp/sim/simserial.c       |  13 -----
 drivers/tty/amiserial.c            |  23 ++------
 drivers/tty/cyclades.c             |  21 ++++----
 drivers/tty/rocket.c               |  27 +++-------
 drivers/tty/serial/serial_core.c   |   5 +-
 drivers/usb/serial/ftdi_sio.c      |  67 +++++------------------
 include/linux/tty.h                |   1 -
 net/irda/ircomm/ircomm_tty_ioctl.c | 107 +------------------------------------
 8 files changed, 34 insertions(+), 230 deletions(-)

--
2.13.0

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#17 lore raw
In reply to Johan Hovold
Quoted 14 lines · Johan Hovold

Setting an alt-speed using TIOCSSERIAL and SPD flags has been deprecated since v2.1.69 and has been broken for all tty drivers but serial-core since v3.10 and commit 6865ff222cca ("TTY: do not warn about setting speed via SPD_*") without anyone noticing (for four years).

Instead of reverting the offending commit, lets get rid of this legacy code while adding back a warning about the SPD flags being deprecated to the drivers that once implemented it. Note that drivers implementing SPD_CUST will continue to support using a custom divisor.

Also note that serial-core did not rely on the TTY layer for SPD handling and continues to support it while warning about deprecation (since 2003 at least).

Quoted 5 lines

Greg, I suggest you take it all trough the TTY tree even if merging through multiple trees and applying the final patch once everything is upstream would be an option. Also the irda clean up does not depend on the rest of series as the code implementing SPD handling was ifdefed out.

I've taken these all through my tty tree, nice work, thanks for the cleanups.

greg k-h