USB: serial: TIOCSSERIAL fixes and generic support

PATCH In review Johan Hovold 2 participants 26 messages Apr 7, 2021

Subsystems (3)

Stable branch USB USB serial

lore

Share & tools

lore

Johan Hovold

author maintainer

2021-04-07 10:39 UTC #0 lore raw

cover USB: serial: TIOCSSERIAL fixes and generic support

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

This series fixes up the various USB serial driver implementations, before adding generic support to core for the benefit of all USB serial drivers.

Johan

Johan Hovold (24):
USB: serial: ark3116: fix TIOCGSERIAL implementation
USB: serial: f81232: fix TIOCGSERIAL implementation
USB: serial: f81534: fix TIOCGSERIAL implementation
USB: serial: ftdi_sio: fix TIOCGSERIAL implementation
USB: serial: io_edgeport: fix TIOCGSERIAL implementation
USB: serial: io_ti: fix TIOCGSERIAL implementation
USB: serial: mos7720: fix TIOCGSERIAL implementation
USB: serial: mos7840: fix TIOCGSERIAL implementation
USB: serial: opticon: fix TIOCGSERIAL implementation
USB: serial: pl2303: fix TIOCGSERIAL implementation
USB: serial: quatech2: fix TIOCGSERIAL implementation
USB: serial: ssu100: fix TIOCGSERIAL implementation
USB: serial: ti_usb_3410_5052: fix TIOCGSERIAL implementation
USB: serial: ti_usb_3410_5052: fix TIOCSSERIAL permission check
USB: serial: usb_wwan: fix TIOCSSERIAL jiffies conversions
USB: serial: usb_wwan: fix unprivileged TIOCCSERIAL
USB: serial: usb_wwan: fix TIOCGSERIAL implementation
USB: serial: whiteheat: fix TIOCGSERIAL implementation
USB: serial: fix return value for unsupported ioctls
USB: serial: add generic support for TIOCSSERIAL
USB: serial: stop reporting legacy UART types
USB: serial: ftdi_sio: ignore baud_base changes
USB: serial: ftdi_sio: simplify TIOCGSERIAL permission check
USB: serial: ftdi_sio: clean up TIOCSSERIAL

 drivers/usb/serial/ark3116.c          | 13 ------
 drivers/usb/serial/f81232.c           | 11 ++---
 drivers/usb/serial/f81534.c           |  7 +---
 drivers/usb/serial/ftdi_sio.c         | 35 ++++------------
 drivers/usb/serial/io_edgeport.c      | 22 ----------
 drivers/usb/serial/io_ti.c            | 24 -----------
 drivers/usb/serial/mos7720.c          | 18 --------
 drivers/usb/serial/mos7840.c          | 23 -----------
 drivers/usb/serial/opticon.c          | 18 --------
 drivers/usb/serial/option.c           |  2 -
 drivers/usb/serial/pl2303.c           | 13 ------
 drivers/usb/serial/quatech2.c         | 16 --------
 drivers/usb/serial/ssu100.c           | 16 --------
 drivers/usb/serial/ti_usb_3410_5052.c | 38 +----------------
 drivers/usb/serial/usb-serial.c       | 59 +++++++++++++++++++++++----
 drivers/usb/serial/usb-wwan.h         |  4 --
 drivers/usb/serial/usb_wwan.c         | 45 --------------------
 drivers/usb/serial/whiteheat.c        | 17 +-------
 include/linux/usb/serial.h            |  2 +-
 19 files changed, 69 insertions(+), 314 deletions(-)

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:39 UTC #1 lore raw

PATCH 1/24 USB: serial: ark3116: fix TIOCGSERIAL implementation

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

The baud_base parameter could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known.

The close_delay and closing_wait parameters returned by TIOCGSERIAL are specified in centiseconds. The driver does not yet support changing these, but let's report back the default values actually used (0.5 and 30 seconds, respectively).

Fixes 2f430b4bbae7 ("USB: ark3116: Add TIOCGSERIAL and TIOCSSERIAL ioctl calls.")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ark3116.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/usb/serial/ark3116.c +3 -2

index b9bedfe9bd09..957cdd694b1f 100644
@@ -392,8 +392,9 @@ static int ark3116_get_serial_info(struct tty_struct *tty,
 
 	ss->type = PORT_16654;
 	ss->line = port->minor;
-	ss->port = port->port_number;
-	ss->baud_base = 460800;
+	ss->close_delay = 50;
+	ss->closing_wait = 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:39 UTC #2 lore raw

PATCH 2/24 USB: serial: f81232: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

Fixes aac1fc386fa1 ("USB: serial: add Fintek F81232 usb to serial driver")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/f81232.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/serial/f81232.c +3 -1

index 6a8f39147d8e..af0fe2a82eb2 100644
@@ -828,8 +828,10 @@ static int f81232_get_serial_info(struct tty_struct *tty,
 
 	ss->type = PORT_16550A;
 	ss->line = port->minor;
-	ss->port = port->port_number;
 	ss->baud_base = priv->baud_base;
+	ss->close_delay = 50;
+	ss->closing_wait = 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #3 lore raw

PATCH 3/24 USB: serial: f81534: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

Fixes aac1fc386fa1 ("USB: serial: add Fintek F81232 usb to serial driver")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/f81534.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/serial/f81534.c +3 -1

index a763b362f081..c9f90d437e3a 100644
@@ -1149,9 +1149,11 @@ static int f81534_get_serial_info(struct tty_struct *tty,
 	port_priv = usb_get_serial_port_data(port);
 
 	ss->type = PORT_16550A;
-	ss->port = port->port_number;
 	ss->line = port->minor;
 	ss->baud_base = port_priv->baud_base;
+	ss->close_delay = 50;
+	ss->closing_wait = 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #4 lore raw

PATCH 4/24 USB: serial: ftdi_sio: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The FTDI driver is the only USB serial driver supporting the deprecated ASYNC_SPD flags, which are reported back as they should by TIOCGSERIAL, but the returned parameters did not include the line number.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/usb/serial/ftdi_sio.c +4 -0

index c867592477c9..f8a0911f90ea 100644
@@ -1483,9 +1483,13 @@ static int get_serial_info(struct tty_struct *tty,
 	struct usb_serial_port *port = tty->driver_data;
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
 
+	ss->line = port->minor;
 	ss->flags = priv->flags;
 	ss->baud_base = priv->baud_base;
 	ss->custom_divisor = priv->custom_divisor;
+	ss->close_delay = 50;
+	ss->closing_wait = 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #5 lore raw

PATCH 5/24 USB: serial: io_edgeport: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

The xmit_fifo_size parameter could be used to set the hardware transmit fifo size of a legacy UART when it could not be detected, but the interface is limited to eight bits and should be left unset when not used.

Similarly, baud_base could be used to set the uart base clock when it could not be detected, but might as well be left unset when it is not known.

The close_delay and closing_wait parameters returned by TIOCGSERIAL are specified in centiseconds (not jiffies). The driver does not yet support changing these, but let's report back the default values actually used (0.5 and 30 seconds, respectively).

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/io_edgeport.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

drivers/usb/serial/io_edgeport.c +4 -8

index 68401adcffde..471a1a04c9c3 100644
@@ -1641,16 +1641,12 @@ static int get_serial_info(struct tty_struct *tty,
 				struct serial_struct *ss)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	struct edgeport_port *edge_port = usb_get_serial_port_data(port);
 
 	ss->type		= PORT_16550A;
-	ss->line		= edge_port->port->minor;
-	ss->port		= edge_port->port->port_number;
-	ss->irq			= 0;
-	ss->xmit_fifo_size	= edge_port->maxTxCredits;
-	ss->baud_base		= 9600;
-	ss->close_delay		= 5*HZ;
-	ss->closing_wait	= 30*HZ;
+	ss->line		= port->minor;
+	ss->close_delay		= 50;
+	ss->closing_wait	= 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:39 UTC #6 lore raw

PATCH 6/24 USB: serial: io_ti: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

Similarly, baud_base could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known.

The close_delay and closing_wait parameters returned by TIOCGSERIAL are specified in centiseconds (not jiffies). The driver does not yet support changing close_delay, but let's report back the default value actually used (0.5 seconds).

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/io_ti.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

drivers/usb/serial/io_ti.c +4 -8

index e800547be9e0..f5aab570fd05 100644
@@ -2437,21 +2437,17 @@ static int get_serial_info(struct tty_struct *tty,
 				struct serial_struct *ss)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	struct edgeport_port *edge_port = usb_get_serial_port_data(port);
 	unsigned cwait;
 
-	cwait = edge_port->port->port.closing_wait;
+	cwait = port->port.closing_wait;
 	if (cwait != ASYNC_CLOSING_WAIT_NONE)
 		cwait = jiffies_to_msecs(cwait) / 10;
 
 	ss->type		= PORT_16550A;
-	ss->line		= edge_port->port->minor;
-	ss->port		= edge_port->port->port_number;
-	ss->irq			= 0;
-	ss->xmit_fifo_size	= edge_port->port->bulk_out_size;
-	ss->baud_base		= 9600;
-	ss->close_delay		= 5*HZ;
+	ss->line		= port->minor;
+	ss->close_delay		= 50;
 	ss->closing_wait	= cwait;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:39 UTC #7 lore raw

PATCH 7/24 USB: serial: mos7720: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

Similarly, baud_base could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known.

Fixes 0f64478cbc7a ("USB: add USB serial mos7720 driver")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/mos7720.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

drivers/usb/serial/mos7720.c +4 -8

index bb3d39307d93..db90ce560d42 100644
@@ -1638,16 +1638,12 @@ static int get_serial_info(struct tty_struct *tty,
 			   struct serial_struct *ss)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	struct moschip_port *mos7720_port = usb_get_serial_port_data(port);
 
 	ss->type		= PORT_16550A;
-	ss->line		= mos7720_port->port->minor;
-	ss->port		= mos7720_port->port->port_number;
-	ss->irq			= 0;
-	ss->xmit_fifo_size	= NUM_URBS * URB_TRANSFER_BUFFER_SIZE;
-	ss->baud_base		= 9600;
-	ss->close_delay		= 5*HZ;
-	ss->closing_wait	= 30*HZ;
+	ss->line		= port->minor;
+	ss->close_delay		= 50;
+	ss->closing_wait	= 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #8 lore raw

PATCH 8/24 USB: serial: mos7840: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

Similarly, baud_base could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known.

Fixes 3f5429746d91 ("USB: Moschip 7840 USB-Serial Driver")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/mos7840.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

drivers/usb/serial/mos7840.c +4 -8

index 1bf0d066f55a..77cbe18a1629 100644
@@ -1392,16 +1392,12 @@ static int mos7840_get_serial_info(struct tty_struct *tty,
 				   struct serial_struct *ss)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	struct moschip_port *mos7840_port = usb_get_serial_port_data(port);
 
 	ss->type = PORT_16550A;
-	ss->line = mos7840_port->port->minor;
-	ss->port = mos7840_port->port->port_number;
-	ss->irq = 0;
-	ss->xmit_fifo_size = NUM_URBS * URB_TRANSFER_BUFFER_SIZE;
-	ss->baud_base = 9600;
-	ss->close_delay = 5 * HZ;
-	ss->closing_wait = 30 * HZ;
+	ss->line = port->minor;
+	ss->close_delay = 50;
+	ss->closing_wait = 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #9 lore raw

PATCH 9/24 USB: serial: opticon: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

Similarly, baud_base could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known.

Fixes faac64ad9c7b ("USB: serial: opticon: add serial line ioctls")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/opticon.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

drivers/usb/serial/opticon.c +3 -6

index eecb72aef83e..1c7e5dc2c272 100644
@@ -360,12 +360,9 @@ static int get_serial_info(struct tty_struct *tty,
 	/* fake emulate a 16550 uart to make userspace code happy */
 	ss->type		= PORT_16550A;
 	ss->line		= port->minor;
-	ss->port		= 0;
-	ss->irq			= 0;
-	ss->xmit_fifo_size	= 1024;
-	ss->baud_base		= 9600;
-	ss->close_delay		= 5*HZ;
-	ss->closing_wait	= 30*HZ;
+	ss->close_delay		= 50;
+	ss->closing_wait	= 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #10 lore raw

PATCH 10/24 USB: serial: pl2303: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

The baud_base parameter could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/pl2303.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/usb/serial/pl2303.c +3 -2

index eed9acd1ae08..0455add8593a 100644
@@ -946,8 +946,9 @@ static int pl2303_get_serial(struct tty_struct *tty,
 
 	ss->type = PORT_16654;
 	ss->line = port->minor;
-	ss->port = port->port_number;
-	ss->baud_base = 460800;
+	ss->close_delay = 50;
+	ss->closing_wait = 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #11 lore raw

PATCH 11/24 USB: serial: quatech2: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

Similarly, baud_base could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known.

Fixes f7a33e608d9a ("USB: serial: add quatech2 usb to serial driver")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/quatech2.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

drivers/usb/serial/quatech2.c +3 -6

index 599dcb2e374d..0d23e565e0d2 100644
@@ -459,12 +459,9 @@ static int get_serial_info(struct tty_struct *tty,
 	struct usb_serial_port *port = tty->driver_data;
 
 	ss->line		= port->minor;
-	ss->port		= 0;
-	ss->irq			= 0;
-	ss->xmit_fifo_size	= port->bulk_out_size;
-	ss->baud_base		= 9600;
-	ss->close_delay		= 5*HZ;
-	ss->closing_wait	= 30*HZ;
+	ss->close_delay		= 50;
+	ss->closing_wait	= 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #12 lore raw

PATCH 12/24 USB: serial: ssu100: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

Similarly, baud_base could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known.

Fixes 52af95459939 ("USB: add USB serial ssu100 driver")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ssu100.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

drivers/usb/serial/ssu100.c +3 -6

index 89fdc5c19285..c4616c37f33f 100644
@@ -337,12 +337,9 @@ static int get_serial_info(struct tty_struct *tty,
 	struct usb_serial_port *port = tty->driver_data;
 
 	ss->line		= port->minor;
-	ss->port		= 0;
-	ss->irq			= 0;
-	ss->xmit_fifo_size	= port->bulk_out_size;
-	ss->baud_base		= 9600;
-	ss->close_delay		= 5*HZ;
-	ss->closing_wait	= 30*HZ;
+	ss->close_delay		= 50;
+	ss->closing_wait	= 3000;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #13 lore raw

PATCH 13/24 USB: serial: ti_usb_3410_5052: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

The close_delay and closing_wait parameters returned by TIOCGSERIAL are specified in centiseconds. The driver does not yet support changing close_delay, but let's report back the default value actually used (0.5 seconds).

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ti_usb_3410_5052.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/usb/serial/ti_usb_3410_5052.c +2 -2

index 7252b0ce75a6..4b497c1e850b 100644
@@ -1406,10 +1406,10 @@ static int ti_get_serial_info(struct tty_struct *tty,
 
 	ss->type = PORT_16550A;
 	ss->line = port->minor;
-	ss->port = port->port_number;
-	ss->xmit_fifo_size = kfifo_size(&port->write_fifo);
 	ss->baud_base = tport->tp_tdev->td_is_3410 ? 921600 : 460800;
+	ss->close_delay = 50;
 	ss->closing_wait = cwait;
+
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:39 UTC #14 lore raw

PATCH 14/24 USB: serial: ti_usb_3410_5052: fix TIOCSSERIAL permission check

In reply to Johan Hovold

Changing the port closing-wait parameter is a privileged operation so make sure to return -EPERM if a regular user tries to change it.

Cc [email protected]

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ti_usb_3410_5052.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

drivers/usb/serial/ti_usb_3410_5052.c +7 -2

index 4b497c1e850b..bb50098a0ce6 100644
@@ -1418,14 +1418,19 @@ static int ti_set_serial_info(struct tty_struct *tty,
 	struct serial_struct *ss)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	struct ti_port *tport = usb_get_serial_port_data(port);
+	struct tty_port *tport = &port->port;
 	unsigned cwait;
 
 	cwait = ss->closing_wait;
 	if (cwait != ASYNC_CLOSING_WAIT_NONE)
 		cwait = msecs_to_jiffies(10 * ss->closing_wait);
 
-	tport->tp_port->port.closing_wait = cwait;
+	if (!capable(CAP_SYS_ADMIN)) {
+		if (cwait != tport->closing_wait)
+			return -EPERM;
+	}
+
+	tport->closing_wait = cwait;
 
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:39 UTC #15 lore raw

PATCH 15/24 USB: serial: usb_wwan: fix TIOCSSERIAL jiffies conversions

In reply to Johan Hovold

The port close_delay and closing_wait parameters set by TIOCSSERIAL are specified in jiffies and not milliseconds.

Add the missing conversions so that the TIOCSSERIAL works as expected also when HZ is not 1000.

Fixes 02303f73373a ("usb-wwan: implement TIOCGSERIAL and TIOCSSERIAL to avoid blocking close(2)")

Cc [email protected] # 2.6.38

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/usb_wwan.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

drivers/usb/serial/usb_wwan.c +5 -4

index 46d46a4f99c9..4e9c994a972a 100644
@@ -140,10 +140,10 @@ int usb_wwan_get_serial_info(struct tty_struct *tty,
 	ss->line            = port->minor;
 	ss->port            = port->port_number;
 	ss->baud_base       = tty_get_baud_rate(port->port.tty);
-	ss->close_delay	    = port->port.close_delay / 10;
+	ss->close_delay	    = jiffies_to_msecs(port->port.close_delay) / 10;
 	ss->closing_wait    = port->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
 				 ASYNC_CLOSING_WAIT_NONE :
-				 port->port.closing_wait / 10;
+				 jiffies_to_msecs(port->port.closing_wait) / 10;
 	return 0;
 }
 EXPORT_SYMBOL(usb_wwan_get_serial_info);

@@ -155,9 +155,10 @@ int usb_wwan_set_serial_info(struct tty_struct *tty,
 	unsigned int closing_wait, close_delay;
 	int retval = 0;
 
-	close_delay = ss->close_delay * 10;
+	close_delay = msecs_to_jiffies(ss->close_delay * 10);
 	closing_wait = ss->closing_wait == ASYNC_CLOSING_WAIT_NONE ?
-			ASYNC_CLOSING_WAIT_NONE : ss->closing_wait * 10;
+			ASYNC_CLOSING_WAIT_NONE :
+			msecs_to_jiffies(ss->closing_wait * 10);
 
 	mutex_lock(&port->port.mutex);

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #16 lore raw

PATCH 16/24 USB: serial: usb_wwan: fix unprivileged TIOCCSERIAL

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

A non-privileged user has only ever been able to set the since long deprecated ASYNC_SPD flags and trying to change any other *supported* feature should result in -EPERM being returned. Setting the current values for any supported features should return success.

Fix the usb_wwan implementation which instead indicated that the TIOCSSERIAL ioctl was not even implemented when a non-privileged user set the current values.

Fixes 02303f73373a ("usb-wwan: implement TIOCGSERIAL and TIOCSSERIAL to avoid blocking close(2)")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/usb_wwan.c | 2 --
 1 file changed, 2 deletions(-)

drivers/usb/serial/usb_wwan.c +0 -2

index 4e9c994a972a..e71c828682f5 100644
@@ -166,8 +166,6 @@ int usb_wwan_set_serial_info(struct tty_struct *tty,
 		if ((close_delay != port->port.close_delay) ||
 		    (closing_wait != port->port.closing_wait))
 			retval = -EPERM;
-		else
-			retval = -EOPNOTSUPP;
 	} else {
 		port->port.close_delay  = close_delay;
 		port->port.closing_wait = closing_wait;

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #17 lore raw

PATCH 17/24 USB: serial: usb_wwan: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

The baud_base parameter could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known.

Fix the usb_wwan TIOCGSERIAL implementation by dropping its custom interpretation of the unused port and baud_base fields, which were set to the port index and current line speed, respectively.

Fixes 02303f73373a ("usb-wwan: implement TIOCGSERIAL and TIOCSSERIAL to avoid blocking close(2)")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/usb_wwan.c | 2 --
 1 file changed, 2 deletions(-)

drivers/usb/serial/usb_wwan.c +0 -2

index e71c828682f5..4ea315e5e69b 100644
@@ -138,8 +138,6 @@ int usb_wwan_get_serial_info(struct tty_struct *tty,
 	struct usb_serial_port *port = tty->driver_data;
 
 	ss->line            = port->minor;
-	ss->port            = port->port_number;
-	ss->baud_base       = tty_get_baud_rate(port->port.tty);
 	ss->close_delay	    = jiffies_to_msecs(port->port.close_delay) / 10;
 	ss->closing_wait    = port->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
 				 ASYNC_CLOSING_WAIT_NONE :

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #18 lore raw

PATCH 18/24 USB: serial: whiteheat: fix TIOCGSERIAL implementation

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/whiteheat.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

drivers/usb/serial/whiteheat.c +2 -6

index ccfd5ed652cd..c8b10faa2ff8 100644
@@ -170,7 +170,6 @@ static int firm_report_tx_done(struct usb_serial_port *port);
 #define COMMAND_PORT		4
 #define COMMAND_TIMEOUT		(2*HZ)	/* 2 second timeout for a command */
 #define	COMMAND_TIMEOUT_MS	2000
-#define CLOSING_DELAY		(30 * HZ)
 
 
 /*****************************************************************************

@@ -447,12 +446,9 @@ static int whiteheat_get_serial(struct tty_struct *tty,
 
 	ss->type = PORT_16654;
 	ss->line = port->minor;
-	ss->port = port->port_number;
-	ss->xmit_fifo_size = kfifo_size(&port->write_fifo);
-	ss->custom_divisor = 0;
 	ss->baud_base = 460800;
-	ss->close_delay = CLOSING_DELAY;
-	ss->closing_wait = CLOSING_DELAY;
+	ss->close_delay = 50;
+	ss->closing_wait = 3000;
 
 	return 0;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #19 lore raw

PATCH 19/24 USB: serial: fix return value for unsupported ioctls

In reply to Johan Hovold

Drivers should return -ENOTTY ("Inappropriate I/O control operation") when an ioctl isn't supported, while -EINVAL is used for invalid arguments.

Fix up the TIOCMGET, TIOCMSET and TIOCGICOUNT helpers which returned -EINVAL when a USB serial driver did not implement the corresponding methods.

Note that the TIOCMGET and TIOCMSET helpers predate git and do not get a corresponding Fixes tag below.

Fixes d281da7ff6f7 ("tty: Make tiocgicount a handler")

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/usb-serial.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/usb/serial/usb-serial.c +3 -3

index 27e3bb58c872..c311cc4fabee 100644
@@ -502,7 +502,7 @@ static int serial_tiocmget(struct tty_struct *tty)
 
 	if (port->serial->type->tiocmget)
 		return port->serial->type->tiocmget(tty);
-	return -EINVAL;
+	return -ENOTTY;
 }
 
 static int serial_tiocmset(struct tty_struct *tty,

@@ -514,7 +514,7 @@ static int serial_tiocmset(struct tty_struct *tty,
 
 	if (port->serial->type->tiocmset)
 		return port->serial->type->tiocmset(tty, set, clear);
-	return -EINVAL;
+	return -ENOTTY;
 }
 
 static int serial_get_icount(struct tty_struct *tty,

@@ -526,7 +526,7 @@ static int serial_get_icount(struct tty_struct *tty,
 
 	if (port->serial->type->get_icount)
 		return port->serial->type->get_icount(tty, icount);
-	return -EINVAL;
+	return -ENOTTY;
 }
 
 /*

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #20 lore raw

PATCH 20/24 USB: serial: add generic support for TIOCSSERIAL

In reply to Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.

The closing_wait parameter determines how long to wait for the transfer buffers to drain during close and the default timeout of 30 seconds may not be sufficient at low line speeds. In other cases, when for example flow is stopped, the default timeout may instead be too long.

Add generic support for TIOCSSERIAL and TIOCGSERIAL with handling of the three common parameters close_delay, closing_wait and line for the benefit of all USB serial drivers while still allowing drivers to implement further functionality through the existing callbacks.

This currently includes a few drivers that report their base baud clock rate even if that is really only of interest when setting custom divisors through the deprecated ASYNC_SPD_CUST interface; an interface which only the FTDI driver actually implements.

Some drivers have also been reporting back a fake UART type, something which should no longer be needed and will be dropped by a follow-on patch.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ark3116.c          |  9 +----
 drivers/usb/serial/f81232.c           | 12 ++----
 drivers/usb/serial/f81534.c           |  8 +---
 drivers/usb/serial/ftdi_sio.c         | 11 +-----
 drivers/usb/serial/io_edgeport.c      | 12 +-----
 drivers/usb/serial/io_ti.c            | 17 +--------
 drivers/usb/serial/mos7720.c          | 12 +-----
 drivers/usb/serial/mos7840.c          | 10 +----
 drivers/usb/serial/opticon.c          | 12 +-----
 drivers/usb/serial/option.c           |  2 -
 drivers/usb/serial/pl2303.c           | 10 +----
 drivers/usb/serial/quatech2.c         | 13 -------
 drivers/usb/serial/ssu100.c           | 13 -------
 drivers/usb/serial/ti_usb_3410_5052.c | 42 +--------------------
 drivers/usb/serial/usb-serial.c       | 53 ++++++++++++++++++++++++---
 drivers/usb/serial/usb-wwan.h         |  4 --
 drivers/usb/serial/usb_wwan.c         | 42 ---------------------
 drivers/usb/serial/whiteheat.c        | 12 +-----
 include/linux/usb/serial.h            |  2 +-
 19 files changed, 70 insertions(+), 226 deletions(-)

drivers/usb/serial/ark3116.c +1 -8

index 957cdd694b1f..c0cf60e9273d 100644
@@ -385,17 +385,10 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port)
 	return result;
 }
 
-static int ark3116_get_serial_info(struct tty_struct *tty,
+static void ark3116_get_serial_info(struct tty_struct *tty,
 			struct serial_struct *ss)
 {
-	struct usb_serial_port *port = tty->driver_data;
-
 	ss->type = PORT_16654;
-	ss->line = port->minor;
-	ss->close_delay = 50;
-	ss->closing_wait = 3000;
-
-	return 0;
 }
 
 static int ark3116_tiocmget(struct tty_struct *tty)

drivers/usb/serial/f81232.c +3 -9

index af0fe2a82eb2..5e34b364d94d 100644
@@ -820,19 +820,13 @@ static int f81232_carrier_raised(struct usb_serial_port *port)
 	return 0;
 }
 
-static int f81232_get_serial_info(struct tty_struct *tty,
-		struct serial_struct *ss)
+static void f81232_get_serial(struct tty_struct *tty, struct serial_struct *ss)
 {
 	struct usb_serial_port *port = tty->driver_data;
 	struct f81232_private *priv = usb_get_serial_port_data(port);
 
 	ss->type = PORT_16550A;
-	ss->line = port->minor;
 	ss->baud_base = priv->baud_base;
-	ss->close_delay = 50;
-	ss->closing_wait = 3000;
-
-	return 0;
 }
 
 static void  f81232_interrupt_work(struct work_struct *work)

@@ -1023,7 +1017,7 @@ static struct usb_serial_driver f81232_device = {
 	.close =		f81232_close,
 	.dtr_rts =		f81232_dtr_rts,
 	.carrier_raised =	f81232_carrier_raised,
-	.get_serial =		f81232_get_serial_info,
+	.get_serial =		f81232_get_serial,
 	.break_ctl =		f81232_break_ctl,
 	.set_termios =		f81232_set_termios,
 	.tiocmget =		f81232_tiocmget,

@@ -1048,7 +1042,7 @@ static struct usb_serial_driver f81534a_device = {
 	.close =		f81232_close,
 	.dtr_rts =		f81232_dtr_rts,
 	.carrier_raised =	f81232_carrier_raised,
-	.get_serial =		f81232_get_serial_info,
+	.get_serial =		f81232_get_serial,
 	.break_ctl =		f81232_break_ctl,
 	.set_termios =		f81232_set_termios,
 	.tiocmget =		f81232_tiocmget,

drivers/usb/serial/f81534.c +1 -7

index c9f90d437e3a..633de52feaad 100644
@@ -1140,8 +1140,7 @@ static void f81534_close(struct usb_serial_port *port)
 	mutex_unlock(&serial_priv->urb_mutex);
 }
 
-static int f81534_get_serial_info(struct tty_struct *tty,
-				  struct serial_struct *ss)
+static void f81534_get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 {
 	struct usb_serial_port *port = tty->driver_data;
 	struct f81534_port_private *port_priv;

@@ -1149,12 +1148,7 @@ static int f81534_get_serial_info(struct tty_struct *tty,
 	port_priv = usb_get_serial_port_data(port);
 
 	ss->type = PORT_16550A;
-	ss->line = port->minor;
 	ss->baud_base = port_priv->baud_base;
-	ss->close_delay = 50;
-	ss->closing_wait = 3000;
-
-	return 0;
 }
 
 static void f81534_process_per_serial_block(struct usb_serial_port *port,

drivers/usb/serial/ftdi_sio.c +2 -9

index f8a0911f90ea..16d3e50487e6 100644
@@ -1082,8 +1082,7 @@ static int  ftdi_tiocmset(struct tty_struct *tty,
 			unsigned int set, unsigned int clear);
 static int  ftdi_ioctl(struct tty_struct *tty,
 			unsigned int cmd, unsigned long arg);
-static int get_serial_info(struct tty_struct *tty,
-				struct serial_struct *ss);
+static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss);
 static int set_serial_info(struct tty_struct *tty,
 				struct serial_struct *ss);
 static void ftdi_break_ctl(struct tty_struct *tty, int break_state);

@@ -1477,20 +1476,14 @@ static int read_latency_timer(struct usb_serial_port *port)
 	return 0;
 }
 
-static int get_serial_info(struct tty_struct *tty,
-				struct serial_struct *ss)
+static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 {
 	struct usb_serial_port *port = tty->driver_data;
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
 
-	ss->line = port->minor;
 	ss->flags = priv->flags;
 	ss->baud_base = priv->baud_base;
 	ss->custom_divisor = priv->custom_divisor;
-	ss->close_delay = 50;
-	ss->closing_wait = 3000;
-
-	return 0;
 }
 
 static int set_serial_info(struct tty_struct *tty,

drivers/usb/serial/io_edgeport.c +2 ~~-10~~

index 471a1a04c9c3..f669b065dc42 100644
@@ -1637,17 +1637,9 @@ static int edge_tiocmget(struct tty_struct *tty)
 	return result;
 }
 
-static int get_serial_info(struct tty_struct *tty,
-				struct serial_struct *ss)
+static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 {
-	struct usb_serial_port *port = tty->driver_data;
-
-	ss->type		= PORT_16550A;
-	ss->line		= port->minor;
-	ss->close_delay		= 50;
-	ss->closing_wait	= 3000;
-
-	return 0;
+	ss->type = PORT_16550A;
 }

drivers/usb/serial/io_ti.c +2 ~~-15~~

index f5aab570fd05..dce994c29afe 100644
@@ -2433,22 +2433,9 @@ static int edge_tiocmget(struct tty_struct *tty)
 	return result;
 }
 
-static int get_serial_info(struct tty_struct *tty,
-				struct serial_struct *ss)
+static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 {
-	struct usb_serial_port *port = tty->driver_data;
-	unsigned cwait;
-
-	cwait = port->port.closing_wait;
-	if (cwait != ASYNC_CLOSING_WAIT_NONE)
-		cwait = jiffies_to_msecs(cwait) / 10;
-
-	ss->type		= PORT_16550A;
-	ss->line		= port->minor;
-	ss->close_delay		= 50;
-	ss->closing_wait	= cwait;
-
-	return 0;
+	ss->type = PORT_16550A;
 }
 
 static void edge_break(struct tty_struct *tty, int break_state)

drivers/usb/serial/mos7720.c +2 ~~-10~~

index db90ce560d42..ccb1291c94a2 100644
@@ -1634,17 +1634,9 @@ static int mos7720_tiocmset(struct tty_struct *tty,
 	return 0;
 }
 
-static int get_serial_info(struct tty_struct *tty,
-			   struct serial_struct *ss)
+static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 {
-	struct usb_serial_port *port = tty->driver_data;
-
-	ss->type		= PORT_16550A;
-	ss->line		= port->minor;
-	ss->close_delay		= 50;
-	ss->closing_wait	= 3000;
-
-	return 0;
+	ss->type = PORT_16550A;
 }
 
 static int mos7720_ioctl(struct tty_struct *tty,

drivers/usb/serial/mos7840.c +1 -9

index 77cbe18a1629..d20fb0a678dc 100644
@@ -1388,17 +1388,9 @@ static int mos7840_get_lsr_info(struct tty_struct *tty,
  *      function to get information about serial port
  *****************************************************************************/
 
-static int mos7840_get_serial_info(struct tty_struct *tty,
-				   struct serial_struct *ss)
+static void mos7840_get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 {
-	struct usb_serial_port *port = tty->driver_data;
-
 	ss->type = PORT_16550A;
-	ss->line = port->minor;
-	ss->close_delay = 50;
-	ss->closing_wait = 3000;
-
-	return 0;
 }
 
 /*****************************************************************************

drivers/usb/serial/opticon.c +2 ~~-10~~

index 1c7e5dc2c272..db84afcf7f1a 100644
@@ -352,18 +352,10 @@ static int opticon_tiocmset(struct tty_struct *tty,
 	return 0;
 }
 
-static int get_serial_info(struct tty_struct *tty,
-			   struct serial_struct *ss)
+static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 {
-	struct usb_serial_port *port = tty->driver_data;
-
 	/* fake emulate a 16550 uart to make userspace code happy */
-	ss->type		= PORT_16550A;
-	ss->line		= port->minor;
-	ss->close_delay		= 50;
-	ss->closing_wait	= 3000;
-
-	return 0;
+	ss->type = PORT_16550A;
 }
 
 static int opticon_port_probe(struct usb_serial_port *port)

drivers/usb/serial/option.c +0 -2

index c6969ca72839..3e79a543d3e7 100644
@@ -2095,8 +2095,6 @@ static struct usb_serial_driver option_1port_device = {
 	.chars_in_buffer   = usb_wwan_chars_in_buffer,
 	.tiocmget          = usb_wwan_tiocmget,
 	.tiocmset          = usb_wwan_tiocmset,
-	.get_serial        = usb_wwan_get_serial_info,
-	.set_serial        = usb_wwan_set_serial_info,
 	.attach            = option_attach,
 	.release           = option_release,
 	.port_probe        = usb_wwan_port_probe,

drivers/usb/serial/pl2303.c +1 -9

index 0455add8593a..67d598b1a0f7 100644
@@ -939,17 +939,9 @@ static int pl2303_carrier_raised(struct usb_serial_port *port)
 	return 0;
 }
 
-static int pl2303_get_serial(struct tty_struct *tty,
-			struct serial_struct *ss)
+static void pl2303_get_serial(struct tty_struct *tty, struct serial_struct *ss)
 {
-	struct usb_serial_port *port = tty->driver_data;
-
 	ss->type = PORT_16654;
-	ss->line = port->minor;
-	ss->close_delay = 50;
-	ss->closing_wait = 3000;
-
-	return 0;
 }
 
 static void pl2303_set_break(struct usb_serial_port *port, bool enable)

drivers/usb/serial/quatech2.c +0 ~~-13~~

index 0d23e565e0d2..5f2e7f668e68 100644
@@ -453,18 +453,6 @@ static void qt2_disconnect(struct usb_serial *serial)
 	usb_kill_urb(serial_priv->read_urb);
 }
 
-static int get_serial_info(struct tty_struct *tty,
-			   struct serial_struct *ss)
-{
-	struct usb_serial_port *port = tty->driver_data;
-
-	ss->line		= port->minor;
-	ss->close_delay		= 50;
-	ss->closing_wait	= 3000;
-
-	return 0;
-}
-
 static void qt2_process_status(struct usb_serial_port *port, unsigned char *ch)
 {
 	switch (*ch) {

@@ -975,7 +963,6 @@ static struct usb_serial_driver qt2_device = {
 	.tiocmset            = qt2_tiocmset,
 	.tiocmiwait          = usb_serial_generic_tiocmiwait,
 	.get_icount	     = usb_serial_generic_get_icount,
-	.get_serial          = get_serial_info,
 	.set_termios         = qt2_set_termios,
 };

drivers/usb/serial/ssu100.c +0 ~~-13~~

index c4616c37f33f..3baf7c0f5a98 100644
@@ -331,18 +331,6 @@ static int ssu100_open(struct tty_struct *tty, struct usb_serial_port *port)
 	return usb_serial_generic_open(tty, port);
 }
 
-static int get_serial_info(struct tty_struct *tty,
-			   struct serial_struct *ss)
-{
-	struct usb_serial_port *port = tty->driver_data;
-
-	ss->line		= port->minor;
-	ss->close_delay		= 50;
-	ss->closing_wait	= 3000;
-
-	return 0;
-}
-
 static int ssu100_attach(struct usb_serial *serial)
 {
 	return ssu100_initdevice(serial->dev);

@@ -542,7 +530,6 @@ static struct usb_serial_driver ssu100_device = {
 	.tiocmset            = ssu100_tiocmset,
 	.tiocmiwait          = usb_serial_generic_tiocmiwait,
 	.get_icount	     = usb_serial_generic_get_icount,
-	.get_serial          = get_serial_info,
 	.set_termios         = ssu100_set_termios,
 };

drivers/usb/serial/ti_usb_3410_5052.c +2 ~~-40~~

72 lines not on this page · raw message

drivers/usb/serial/usb-serial.c +48 -5

67 lines not on this page · raw message

drivers/usb/serial/usb-wwan.h +0 -4

index 79dafd98e0a1..b5331d03092f 100644
@@ -15,10 +15,6 @@ extern int usb_wwan_write_room(struct tty_struct *tty);
 extern int usb_wwan_tiocmget(struct tty_struct *tty);
 extern int usb_wwan_tiocmset(struct tty_struct *tty,
 			     unsigned int set, unsigned int clear);
-extern int usb_wwan_get_serial_info(struct tty_struct *tty,
-			   struct serial_struct *ss);
-extern int usb_wwan_set_serial_info(struct tty_struct *tty,
-			   struct serial_struct *ss);
 extern int usb_wwan_write(struct tty_struct *tty, struct usb_serial_port *port,
 			  const unsigned char *buf, int count);
 extern int usb_wwan_chars_in_buffer(struct tty_struct *tty);

drivers/usb/serial/usb_wwan.c +0 ~~-42~~

48 lines not on this page · raw message

drivers/usb/serial/whiteheat.c +2 ~~-10~~

index c8b10faa2ff8..6a95c5a0056f 100644
@@ -83,7 +83,7 @@ static void whiteheat_port_remove(struct usb_serial_port *port);
 static int  whiteheat_open(struct tty_struct *tty,
 			struct usb_serial_port *port);
 static void whiteheat_close(struct usb_serial_port *port);
-static int  whiteheat_get_serial(struct tty_struct *tty,
+static void whiteheat_get_serial(struct tty_struct *tty,
 			struct serial_struct *ss);
 static void whiteheat_set_termios(struct tty_struct *tty,
 			struct usb_serial_port *port, struct ktermios *old);

@@ -439,18 +439,10 @@ static int whiteheat_tiocmset(struct tty_struct *tty,
 }
 
 
-static int whiteheat_get_serial(struct tty_struct *tty,
-				struct serial_struct *ss)
+static void whiteheat_get_serial(struct tty_struct *tty, struct serial_struct *ss)
 {
-	struct usb_serial_port *port = tty->driver_data;
-
 	ss->type = PORT_16654;
-	ss->line = port->minor;
 	ss->baud_base = 460800;
-	ss->close_delay = 50;
-	ss->closing_wait = 3000;
-
-	return 0;
 }

include/linux/usb/serial.h +1 -1

index 952272002e48..6a6ac06fe1e6 100644
@@ -276,7 +276,7 @@ struct usb_serial_driver {
 	int  (*write_room)(struct tty_struct *tty);
 	int  (*ioctl)(struct tty_struct *tty,
 		      unsigned int cmd, unsigned long arg);
-	int  (*get_serial)(struct tty_struct *tty, struct serial_struct *ss);
+	void (*get_serial)(struct tty_struct *tty, struct serial_struct *ss);
 	int  (*set_serial)(struct tty_struct *tty, struct serial_struct *ss);
 	void (*set_termios)(struct tty_struct *tty,
 			struct usb_serial_port *port, struct ktermios *old);

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #21 lore raw

PATCH 21/24 USB: serial: stop reporting legacy UART types

In reply to Johan Hovold

The TIOCGSERIAL ioctl can be used to set and retrieve the UART type for legacy UARTs, but some USB serial drivers have been reporting back random types in order to "make user-space happy".

Some applications have historically expected TIOCGSERIAL to be implemented, but judging from the Debian sources, the port type not being PORT_UNKNOWN is only used to check for the existence of legacy serial ports (ttySn).

Drivers like ftdi_sio have been using PORT_UNKNOWN for twenty years (and option for 10 years) without anyone complaining so let's stop reporting back anything else.

In the unlikely event that this do cause problems, this should be fixed tree-wide anyway (e.g. for all USB serial drivers and also CDC-ACM).

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ark3116.c          |  7 -------
 drivers/usb/serial/f81232.c           |  1 -
 drivers/usb/serial/f81534.c           |  1 -
 drivers/usb/serial/io_edgeport.c      | 10 ----------
 drivers/usb/serial/io_ti.c            |  7 -------
 drivers/usb/serial/mos7720.c          |  6 ------
 drivers/usb/serial/mos7840.c          | 11 -----------
 drivers/usb/serial/opticon.c          |  7 -------
 drivers/usb/serial/pl2303.c           |  6 ------
 drivers/usb/serial/ti_usb_3410_5052.c |  1 -
 drivers/usb/serial/whiteheat.c        |  1 -
 11 files changed, 58 deletions(-)

drivers/usb/serial/ark3116.c +0 -7

index c0cf60e9273d..5dd710e9fe7d 100644
@@ -385,12 +385,6 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port)
 	return result;
 }
 
-static void ark3116_get_serial_info(struct tty_struct *tty,
-			struct serial_struct *ss)
-{
-	ss->type = PORT_16654;
-}
-
 static int ark3116_tiocmget(struct tty_struct *tty)
 {
 	struct usb_serial_port *port = tty->driver_data;

@@ -627,7 +621,6 @@ static struct usb_serial_driver ark3116_device = {
 	.port_probe =		ark3116_port_probe,
 	.port_remove =		ark3116_port_remove,
 	.set_termios =		ark3116_set_termios,
-	.get_serial =		ark3116_get_serial_info,
 	.tiocmget =		ark3116_tiocmget,
 	.tiocmset =		ark3116_tiocmset,
 	.tiocmiwait =		usb_serial_generic_tiocmiwait,

drivers/usb/serial/f81232.c +0 -1

index 5e34b364d94d..b4b847dce4bc 100644
@@ -825,7 +825,6 @@ static void f81232_get_serial(struct tty_struct *tty, struct serial_struct *ss)
 	struct usb_serial_port *port = tty->driver_data;
 	struct f81232_private *priv = usb_get_serial_port_data(port);
 
-	ss->type = PORT_16550A;
 	ss->baud_base = priv->baud_base;
 }

drivers/usb/serial/f81534.c +0 -1

index 633de52feaad..c0bca52ef92a 100644
@@ -1147,7 +1147,6 @@ static void f81534_get_serial_info(struct tty_struct *tty, struct serial_struct
 
 	port_priv = usb_get_serial_port_data(port);
 
-	ss->type = PORT_16550A;
 	ss->baud_base = port_priv->baud_base;
 }

drivers/usb/serial/io_edgeport.c +0 ~~-10~~

index f669b065dc42..4f9fb1d96380 100644
@@ -1637,12 +1637,6 @@ static int edge_tiocmget(struct tty_struct *tty)
 	return result;
 }
 
-static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
-{
-	ss->type = PORT_16550A;
-}
-
-
 /*****************************************************************************
  * SerialIoctl
  *	this function handles any ioctl calls to the driver

@@ -3104,7 +3098,6 @@ static struct usb_serial_driver edgeport_2port_device = {
 	.set_termios		= edge_set_termios,
 	.tiocmget		= edge_tiocmget,
 	.tiocmset		= edge_tiocmset,
-	.get_serial		= get_serial_info,
 	.tiocmiwait		= usb_serial_generic_tiocmiwait,
 	.get_icount		= usb_serial_generic_get_icount,
 	.write			= edge_write,

@@ -3140,7 +3133,6 @@ static struct usb_serial_driver edgeport_4port_device = {
 	.set_termios		= edge_set_termios,
 	.tiocmget		= edge_tiocmget,
 	.tiocmset		= edge_tiocmset,
-	.get_serial		= get_serial_info,
 	.tiocmiwait		= usb_serial_generic_tiocmiwait,
 	.get_icount		= usb_serial_generic_get_icount,
 	.write			= edge_write,

@@ -3176,7 +3168,6 @@ static struct usb_serial_driver edgeport_8port_device = {
 	.set_termios		= edge_set_termios,
 	.tiocmget		= edge_tiocmget,
 	.tiocmset		= edge_tiocmset,
-	.get_serial		= get_serial_info,
 	.tiocmiwait		= usb_serial_generic_tiocmiwait,
 	.get_icount		= usb_serial_generic_get_icount,
 	.write			= edge_write,

@@ -3212,7 +3203,6 @@ static struct usb_serial_driver epic_device = {
 	.set_termios		= edge_set_termios,
 	.tiocmget		= edge_tiocmget,
 	.tiocmset		= edge_tiocmset,
-	.get_serial		= get_serial_info,
 	.tiocmiwait		= usb_serial_generic_tiocmiwait,
 	.get_icount		= usb_serial_generic_get_icount,
 	.write			= edge_write,

drivers/usb/serial/io_ti.c +0 -7

index dce994c29afe..f548cdbf0a51 100644
@@ -2433,11 +2433,6 @@ static int edge_tiocmget(struct tty_struct *tty)
 	return result;
 }
 
-static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
-{
-	ss->type = PORT_16550A;
-}
-
 static void edge_break(struct tty_struct *tty, int break_state)
 {
 	struct usb_serial_port *port = tty->driver_data;

@@ -2696,7 +2691,6 @@ static struct usb_serial_driver edgeport_1port_device = {
 	.release		= edge_release,
 	.port_probe		= edge_port_probe,
 	.port_remove		= edge_port_remove,
-	.get_serial		= get_serial_info,
 	.set_termios		= edge_set_termios,
 	.tiocmget		= edge_tiocmget,
 	.tiocmset		= edge_tiocmset,

@@ -2735,7 +2729,6 @@ static struct usb_serial_driver edgeport_2port_device = {
 	.release		= edge_release,
 	.port_probe		= edge_port_probe,
 	.port_remove		= edge_port_remove,
-	.get_serial		= get_serial_info,
 	.set_termios		= edge_set_termios,
 	.tiocmget		= edge_tiocmget,
 	.tiocmset		= edge_tiocmset,

drivers/usb/serial/mos7720.c +0 -6

index ccb1291c94a2..6ee83886e2c9 100644
@@ -1634,11 +1634,6 @@ static int mos7720_tiocmset(struct tty_struct *tty,
 	return 0;
 }
 
-static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
-{
-	ss->type = PORT_16550A;
-}
-
 static int mos7720_ioctl(struct tty_struct *tty,
 			 unsigned int cmd, unsigned long arg)
 {

@@ -1778,7 +1773,6 @@ static struct usb_serial_driver moschip7720_2port_driver = {
 	.ioctl			= mos7720_ioctl,
 	.tiocmget		= mos7720_tiocmget,
 	.tiocmset		= mos7720_tiocmset,
-	.get_serial		= get_serial_info,
 	.set_termios		= mos7720_set_termios,
 	.write			= mos7720_write,
 	.write_room		= mos7720_write_room,

drivers/usb/serial/mos7840.c +0 ~~-11~~

index d20fb0a678dc..28e4093794e0 100644
@@ -1383,16 +1383,6 @@ static int mos7840_get_lsr_info(struct tty_struct *tty,
 	return 0;
 }
 
-/*****************************************************************************
- * mos7840_get_serial_info
- *      function to get information about serial port
- *****************************************************************************/
-
-static void mos7840_get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
-{
-	ss->type = PORT_16550A;
-}
-
 /*****************************************************************************
  * SerialIoctl
  *	this function handles any ioctl calls to the driver

@@ -1771,7 +1761,6 @@ static struct usb_serial_driver moschip7840_4port_device = {
 	.probe = mos7840_probe,
 	.attach = mos7840_attach,
 	.ioctl = mos7840_ioctl,
-	.get_serial = mos7840_get_serial_info,
 	.set_termios = mos7840_set_termios,
 	.break_ctl = mos7840_break,
 	.tiocmget = mos7840_tiocmget,

drivers/usb/serial/opticon.c +0 -7

index db84afcf7f1a..40c713fae0c3 100644
@@ -352,12 +352,6 @@ static int opticon_tiocmset(struct tty_struct *tty,
 	return 0;
 }
 
-static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
-{
-	/* fake emulate a 16550 uart to make userspace code happy */
-	ss->type = PORT_16550A;
-}
-
 static int opticon_port_probe(struct usb_serial_port *port)
 {
 	struct opticon_private *priv;

@@ -399,7 +393,6 @@ static struct usb_serial_driver opticon_device = {
 	.chars_in_buffer =	opticon_chars_in_buffer,
 	.throttle =		usb_serial_generic_throttle,
 	.unthrottle =		usb_serial_generic_unthrottle,
-	.get_serial =		get_serial_info,
 	.tiocmget =		opticon_tiocmget,
 	.tiocmset =		opticon_tiocmset,
 	.process_read_urb =	opticon_process_read_urb,

drivers/usb/serial/pl2303.c +0 -6

index 67d598b1a0f7..10f92a60f5fb 100644
@@ -939,11 +939,6 @@ static int pl2303_carrier_raised(struct usb_serial_port *port)
 	return 0;
 }
 
-static void pl2303_get_serial(struct tty_struct *tty, struct serial_struct *ss)
-{
-	ss->type = PORT_16654;
-}
-
 static void pl2303_set_break(struct usb_serial_port *port, bool enable)
 {
 	struct usb_serial *serial = port->serial;

@@ -1127,7 +1122,6 @@ static struct usb_serial_driver pl2303_device = {
 	.close =		pl2303_close,
 	.dtr_rts =		pl2303_dtr_rts,
 	.carrier_raised =	pl2303_carrier_raised,
-	.get_serial =		pl2303_get_serial,
 	.break_ctl =		pl2303_break_ctl,
 	.set_termios =		pl2303_set_termios,
 	.tiocmget =		pl2303_tiocmget,

drivers/usb/serial/ti_usb_3410_5052.c +0 -1

index 6df316bdb40f..c312d0cce5fb 100644
@@ -1393,7 +1393,6 @@ static void ti_get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 	struct usb_serial_port *port = tty->driver_data;
 	struct ti_port *tport = usb_get_serial_port_data(port);
 
-	ss->type = PORT_16550A;
 	ss->baud_base = tport->tp_tdev->td_is_3410 ? 921600 : 460800;
 }

drivers/usb/serial/whiteheat.c +0 -1

index 6a95c5a0056f..5116ed9db3eb 100644
@@ -441,7 +441,6 @@ static int whiteheat_tiocmset(struct tty_struct *tty,
 
 static void whiteheat_get_serial(struct tty_struct *tty, struct serial_struct *ss)
 {
-	ss->type = PORT_16654;
 	ss->baud_base = 460800;
 }

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #22 lore raw

PATCH 22/24 USB: serial: ftdi_sio: ignore baud_base changes

In reply to Johan Hovold

The TIOCSSERIAL error handling is inconsistent at best, but drivers tend to ignore requests to change parameters which cannot be changed rather than return an error.

The FTDI driver ignores change requests for all immutable parameters but baud_base so return success also in this case for consistency.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 4 ----
 1 file changed, 4 deletions(-)

drivers/usb/serial/ftdi_sio.c +0 -4

index 16d3e50487e6..3fd7875200b9 100644
@@ -1509,10 +1509,6 @@ static int set_serial_info(struct tty_struct *tty,
 		goto check_and_exit;
 	}
 
-	if (ss->baud_base != priv->baud_base) {
-		mutex_unlock(&priv->cfg_lock);
-		return -EINVAL;
-	}
 
 	/* Make the changes - these are privileged changes! */

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #23 lore raw

PATCH 23/24 USB: serial: ftdi_sio: simplify TIOCGSERIAL permission check

In reply to Johan Hovold

Changing the deprecated custom_divisor field is an unprivileged operation so after verifying that flag field does not contain any privileged changes both updates can be carried out by any user.

Combine the two branches and drop the erroneous comment.

Note that private flags field is only used for ASYNC flags so there's no need to try to retain any other bits when updating the flags.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

drivers/usb/serial/ftdi_sio.c +1 ~~-12~~

index 3fd7875200b9..9228e56a91c0 100644
@@ -1496,27 +1496,16 @@ static int set_serial_info(struct tty_struct *tty,
 	mutex_lock(&priv->cfg_lock);
 	old_priv = *priv;
 
-	/* Do error checking and permission checking */
-
 	if (!capable(CAP_SYS_ADMIN)) {
 		if ((ss->flags ^ priv->flags) & ~ASYNC_USR_MASK) {
 			mutex_unlock(&priv->cfg_lock);
 			return -EPERM;
 		}
-		priv->flags = ((priv->flags & ~ASYNC_USR_MASK) |
-			       (ss->flags & ASYNC_USR_MASK));
-		priv->custom_divisor = ss->custom_divisor;
-		goto check_and_exit;
 	}
 
-
-	/* Make the changes - these are privileged changes! */
-
-	priv->flags = ((priv->flags & ~ASYNC_FLAGS) |
-					(ss->flags & ASYNC_FLAGS));
+	priv->flags = ss->flags & ASYNC_FLAGS;
 	priv->custom_divisor = ss->custom_divisor;
 
-check_and_exit:
 	write_latency_timer(port);
 
 	if ((priv->flags ^ old_priv.flags) & ASYNC_SPD_MASK ||

Signature · 1 line

2.26.3

Johan Hovold

author maintainer

2021-04-07 10:40 UTC #24 lore raw

PATCH 24/24 USB: serial: ftdi_sio: clean up TIOCSSERIAL

In reply to Johan Hovold

The TIOCSSERIAL implementation needs to compare the old flag and divisor settings with the new to detect ASYNC_SPD changes, but there's no need to copy all driver state to the stack for that.

While at it, unbreak the function parameter list.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/ftdi_sio.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

drivers/usb/serial/ftdi_sio.c +7 -6

index 9228e56a91c0..6f2659e59b2e 100644
@@ -1486,15 +1486,13 @@ static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 	ss->custom_divisor = priv->custom_divisor;
 }
 
-static int set_serial_info(struct tty_struct *tty,
-	struct serial_struct *ss)
+static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)
 {
 	struct usb_serial_port *port = tty->driver_data;
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
-	struct ftdi_private old_priv;
+	int old_flags, old_divisor;
 
 	mutex_lock(&priv->cfg_lock);
-	old_priv = *priv;
 
 	if (!capable(CAP_SYS_ADMIN)) {
 		if ((ss->flags ^ priv->flags) & ~ASYNC_USR_MASK) {

@@ -1503,14 +1501,17 @@ static int set_serial_info(struct tty_struct *tty,
 		}
 	}
 
+	old_flags = priv->flags;
+	old_divisor = priv->custom_divisor;
+
 	priv->flags = ss->flags & ASYNC_FLAGS;
 	priv->custom_divisor = ss->custom_divisor;
 
 	write_latency_timer(port);
 
-	if ((priv->flags ^ old_priv.flags) & ASYNC_SPD_MASK ||
+	if ((priv->flags ^ old_flags) & ASYNC_SPD_MASK ||
 			((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
-			 priv->custom_divisor != old_priv.custom_divisor)) {
+			 priv->custom_divisor != old_divisor)) {
 
 		/* warn about deprecation unless clearing */
 		if (priv->flags & ASYNC_SPD_MASK)

Signature · 1 line

2.26.3

Greg KH

maintainer Reviewed-by on the v1 series

2021-04-07 15:23 UTC #25 lore raw

In reply to Johan Hovold

Quoted 7 lines · Johan Hovold

TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters.
This series fixes up the various USB serial driver implementations, before adding generic support to core for the benefit of all USB serial drivers.

Nice fixes!

Reviewed-by Greg Kroah-Hartman <[email protected]>