[v2] usbserial: pl2303 tx xon/xoff flow control

[Johan Hovold <johan@kernel.org>]

On Sun, May 20, 2018 at 02:23:12AM +0200, Florian Zumbiehl wrote:
> Support hardware-level Xon/Xoff flow control in transmit direction with
> pl2303.
> 
> I only know how to get the hardware to do IXON/!IXANY with ^S/^Q as control
> characters, so I preserve the old behaviour for all other cases.
> 
> Signed-off-by: Florian Zumbiehl <florz@florz.de>
> ---
> --- linux-4.16.9/drivers/usb/serial/pl2303.c.orig	2018-05-20 00:51:34.580966124 +0200
> +++ linux-4.16.9/drivers/usb/serial/pl2303.c	2018-05-20 01:16:49.532730098 +0200
> @@ -539,12 +539,18 @@
>  	struct usb_serial *serial = port->serial;
>  	struct pl2303_serial_private *spriv = usb_get_serial_data(serial);
>  	struct pl2303_private *priv = usb_get_serial_port_data(port);
> +	bool termios_unchanged;
>  	unsigned long flags;
>  	unsigned char *buf;
>  	int ret;
>  	u8 control;
>  
> -	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
> +	termios_unchanged = old_termios &&
> +		!(tty_termios_hw_change(&tty->termios, old_termios) ||
> +		  ((tty->termios.c_iflag ^ old_termios->c_iflag) & (IXON | IXANY)) ||
> +		  tty->termios.c_cc[VSTOP] != old_termios->c_cc[VSTOP] ||
> +		  tty->termios.c_cc[VSTART] != old_termios->c_cc[VSTART]);
> +	if (termios_unchanged)

This is slightly more readable, but you still violate indentation rules
(e.g. two tabs for continuation lines) and the expression is just
generally hard to parse.

I played around with an ixon_changed intermediate (which what I had in
mind when I suggested an intermediate), but in the end I settled on a
helper function.

The result, which I've applied for -next, can be found below.

Thanks,
Johan


From 1f09807ed52bc530a1139c6ae6092b9176ded45f Mon Sep 17 00:00:00 2001
From: Florian Zumbiehl <florz@florz.de>
Date: Sun, 20 May 2018 02:23:12 +0200
Subject: [PATCH] USB: serial: pl2303: add support for tx xon/xoff flow control

Support hardware-level Xon/Xoff flow control in transmit direction with
pl2303.

I only know how to get the hardware to do IXON/!IXANY with ^S/^Q as control
characters, so I preserve the old behaviour for all other cases.

Signed-off-by: Florian Zumbiehl <florz@florz.de>
[ johan: rewrite logic using pl2303_termios_change() helper ]
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/pl2303.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 46dd09da2434..ac231cdf48a6 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -533,6 +533,17 @@ static int pl2303_set_line_request(struct usb_serial_port *port,
 	return 0;
 }
 
+static bool pl2303_termios_change(struct ktermios *a, struct ktermios *b)
+{
+	bool ixon_change;
+
+	ixon_change = ((a->c_iflag ^ b->c_iflag) & (IXON | IXANY)) ||
+			a->c_cc[VSTART] != b->c_cc[VSTART] ||
+			a->c_cc[VSTOP] != b->c_cc[VSTOP];
+
+	return tty_termios_hw_change(a, b) || ixon_change;
+}
+
 static void pl2303_set_termios(struct tty_struct *tty,
 		struct usb_serial_port *port, struct ktermios *old_termios)
 {
@@ -544,7 +555,7 @@ static void pl2303_set_termios(struct tty_struct *tty,
 	int ret;
 	u8 control;
 
-	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
+	if (old_termios && !pl2303_termios_change(&tty->termios, old_termios))
 		return;
 
 	buf = kzalloc(7, GFP_KERNEL);
@@ -662,6 +673,9 @@ static void pl2303_set_termios(struct tty_struct *tty,
 			pl2303_vendor_write(serial, 0x0, 0x41);
 		else
 			pl2303_vendor_write(serial, 0x0, 0x61);
+	} else if (I_IXON(tty) && !I_IXANY(tty) && START_CHAR(tty) == 0x11 &&
+			STOP_CHAR(tty) == 0x13) {
+		pl2303_vendor_write(serial, 0x0, 0xc0);
 	} else {
 		pl2303_vendor_write(serial, 0x0, 0x0);
 	}