diff mbox series

[v3,6/7] USB: serial: ftdi_sio: Fix custom_divisor for TIOCGSERIAL and c_*speed for TCGETS2

Message ID 20220924102718.2984-7-pali@kernel.org (mailing list archive)
State New, archived
Headers show
Series ftdi_sio driver improvements | expand

Commit Message

Pali Rohár Sept. 24, 2022, 10:27 a.m. UTC 
When ASYNC_SPD_CUST is used, update custom_divisor field for TIOCGSERIAL
and c_*speed fields for TCGETS2 so that they correspond to the newly set
baud rate value.

So userspace TCGETS2 ioctls in new c_*speed fields will see the true baud
rate that is being used.

This is needed for switching userspace applications to use TCGETS2 API as
currently new c_*speed fields does not report correct values. Without this
change userspace applications still have to use old deprecated TIOCGSERIAL
to retrieve current baud rate.

Signed-off-by: Pali Rohár <pali@kernel.org>
Tested-by: Marek Behún <kabel@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
Please note that if c_*speed fields of TCGETS2 are not going to be fixed
then userspace application cannot be switched to use this new TCGETS2 API
due to this issue.
---
 drivers/usb/serial/ftdi_sio.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Johan Hovold Nov. 28, 2022, 5:05 p.m. UTC | #1 
On Sat, Sep 24, 2022 at 12:27:17PM +0200, Pali Rohár wrote:
> When ASYNC_SPD_CUST is used, update custom_divisor field for TIOCGSERIAL
> and c_*speed fields for TCGETS2 so that they correspond to the newly set
> baud rate value.
> 
> So userspace TCGETS2 ioctls in new c_*speed fields will see the true baud
> rate that is being used.
> 
> This is needed for switching userspace applications to use TCGETS2 API as
> currently new c_*speed fields does not report correct values. Without this
> change userspace applications still have to use old deprecated TIOCGSERIAL
> to retrieve current baud rate.

Still no. Not happening.

We should not try to determine the rates used when setting (hardware)
divisors directly through the deprecated SPD_CUST hack. Serial core
does not do so for a reason, including that you can set 38400 once and
then fiddle around with setserial all you want without the magic value
changing.

USB serial regressed at one point by starting to report back the rate it
would try to set. I left it in place because it took a fair while before
anyone noticed and no one should be using this interface anyway.

But if you try to generalise this, I'd rather fix that bug or just rip
this out completely.

Johan
diff mbox series

Patch

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 718c86db2900..79b00912c81c 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1319,6 +1319,7 @@  static u32 get_ftdi_divisor(struct tty_struct *tty,
 {
 	struct ftdi_private *priv = usb_get_serial_port_data(port);
 	struct device *dev = &port->dev;
+	int fix_custom_divisor = 0;
 	u32 div_value = 0;
 	int div_okay = 1;
 	int baud;
@@ -1333,6 +1334,7 @@  static u32 get_ftdi_divisor(struct tty_struct *tty,
 	if (baud == 38400 &&
 	    ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST) &&
 	     (priv->custom_divisor)) {
+		fix_custom_divisor = 1;
 		baud = DIV_ROUND_CLOSEST(priv->baud_base, priv->custom_divisor);
 		dev_dbg(dev, "%s - custom divisor %d sets baud rate to %d\n",
 			__func__, priv->custom_divisor, baud);
@@ -1426,7 +1428,19 @@  static u32 get_ftdi_divisor(struct tty_struct *tty,
 			ftdi_chip_name[priv->chip_type]);
 	}
 
+	/* Fix deprecated async-compatible custom_divisor hack and update tty baud rate */
+	if (fix_custom_divisor) {
+		priv->custom_divisor = DIV_ROUND_CLOSEST(priv->baud_base, baud);
+		old_baud = baud;
+		baud = 38400;
+	}
+
 	tty_encode_baud_rate(tty, baud, baud);
+
+	/* For async-compatible custom_divisor store into TCGETS2 c_*speed fields real baud rate */
+	if (fix_custom_divisor)
+		tty->termios.c_ispeed = tty->termios.c_ospeed = old_baud;
+
 	return div_value;
 }
 
@@ -2699,6 +2713,8 @@  static void ftdi_set_termios(struct tty_struct *tty,
 		dev_dbg(ddev, "%s: forcing baud rate for this device\n", __func__);
 		tty_encode_baud_rate(tty, priv->force_baud,
 					priv->force_baud);
+		termios->c_ispeed = termios->c_ospeed =
+			DIV_ROUND_CLOSEST(priv->baud_base, priv->custom_divisor);
 	}
 
 	/* Force RTS-CTS if this device requires it. */