[PATCH 0/8] USB: serial: fix TIOCMBIS and TIOCMBIC

[PATCH 0/8] USB: serial: fix TIOCMBIS and TIOCMBIC

[Johan Hovold]

Asserting or deasserting a modem control line using TIOCMBIS or TIOCMBIC
should not deassert any lines that are not in the mask, but two drivers
got this wrong.

Included are also some related cleanups.

Johan


Johan Hovold (8):
  USB: serial: belkin_sa: fix TIOCMBIS and TIOCMBIC
  USB: serial: kobil_sct: fix TIOCMBIS and TIOCMBIC
  USB: serial: belkin_sa: clean up tiocmset()
  USB: serial: kobil_sct: clean up tiocmset()
  USB: serial: kobil_sct: clean up device type checks
  USB: serial: kobil_sct: add control request helpers
  USB: serial: kobil_sct: clean up set_termios()
  USB: serial: kobil_sct: drop unnecessary initialisations

 drivers/usb/serial/belkin_sa.c |  42 +++----
 drivers/usb/serial/kobil_sct.c | 210 ++++++++++++---------------------
 2 files changed, 95 insertions(+), 157 deletions(-)

-- 
2.49.1

[PATCH 1/8] USB: serial: belkin_sa: fix TIOCMBIS and TIOCMBIC

[Johan Hovold]

Asserting or deasserting a modem control line using TIOCMBIS or TIOCMBIC
should not deassert any lines that are not in the mask.

Fix this long-standing regression dating back to 2003 when the
tiocmset() callback was introduced.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/belkin_sa.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/serial/belkin_sa.c b/drivers/usb/serial/belkin_sa.c
index 44f5b58beec9..aa6b4c4ad5ec 100644
--- a/drivers/usb/serial/belkin_sa.c
+++ b/drivers/usb/serial/belkin_sa.c
@@ -435,7 +435,7 @@ static int belkin_sa_tiocmset(struct tty_struct *tty,
 	struct belkin_sa_private *priv = usb_get_serial_port_data(port);
 	unsigned long control_state;
 	unsigned long flags;
-	int retval;
+	int retval = 0;
 	int rts = 0;
 	int dtr = 0;
 
@@ -452,26 +452,32 @@ static int belkin_sa_tiocmset(struct tty_struct *tty,
 	}
 	if (clear & TIOCM_RTS) {
 		control_state &= ~TIOCM_RTS;
-		rts = 0;
+		rts = 1;
 	}
 	if (clear & TIOCM_DTR) {
 		control_state &= ~TIOCM_DTR;
-		dtr = 0;
+		dtr = 1;
 	}
 
 	priv->control_state = control_state;
 	spin_unlock_irqrestore(&priv->lock, flags);
 
-	retval = BSA_USB_CMD(BELKIN_SA_SET_RTS_REQUEST, rts);
-	if (retval < 0) {
-		dev_err(&port->dev, "Set RTS error %d\n", retval);
-		goto exit;
+	if (rts) {
+		retval = BSA_USB_CMD(BELKIN_SA_SET_RTS_REQUEST,
+					!!(control_state & TIOCM_RTS));
+		if (retval < 0) {
+			dev_err(&port->dev, "Set RTS error %d\n", retval);
+			goto exit;
+		}
 	}
 
-	retval = BSA_USB_CMD(BELKIN_SA_SET_DTR_REQUEST, dtr);
-	if (retval < 0) {
-		dev_err(&port->dev, "Set DTR error %d\n", retval);
-		goto exit;
+	if (dtr) {
+		retval = BSA_USB_CMD(BELKIN_SA_SET_DTR_REQUEST,
+					!!(control_state & TIOCM_DTR));
+		if (retval < 0) {
+			dev_err(&port->dev, "Set DTR error %d\n", retval);
+			goto exit;
+		}
 	}
 exit:
 	return retval;
-- 
2.49.1

[PATCH 2/8] USB: serial: kobil_sct: fix TIOCMBIS and TIOCMBIC

[Johan Hovold]

Asserting or deasserting a modem control line using TIOCMBIS or TIOCMBIC
should not deassert any lines that are not in the mask.

Fix this long-standing issue dating back to 2003 when the support for
these ioctls was added with the introduction of the tiocmset() callback.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/kobil_sct.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
index 464433be2034..96ea571c436a 100644
--- a/drivers/usb/serial/kobil_sct.c
+++ b/drivers/usb/serial/kobil_sct.c
@@ -418,7 +418,7 @@ static int kobil_tiocmset(struct tty_struct *tty,
 	struct usb_serial_port *port = tty->driver_data;
 	struct device *dev = &port->dev;
 	struct kobil_private *priv;
-	int result;
+	int result = 0;
 	int dtr = 0;
 	int rts = 0;
 
@@ -435,12 +435,12 @@ static int kobil_tiocmset(struct tty_struct *tty,
 	if (set & TIOCM_DTR)
 		dtr = 1;
 	if (clear & TIOCM_RTS)
-		rts = 0;
+		rts = 1;
 	if (clear & TIOCM_DTR)
-		dtr = 0;
+		dtr = 1;
 
-	if (priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID) {
-		if (dtr != 0)
+	if (dtr && priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID) {
+		if (set & TIOCM_DTR)
 			dev_dbg(dev, "%s - Setting DTR\n", __func__);
 		else
 			dev_dbg(dev, "%s - Clearing DTR\n", __func__);
@@ -448,13 +448,13 @@ static int kobil_tiocmset(struct tty_struct *tty,
 			  usb_sndctrlpipe(port->serial->dev, 0),
 			  SUSBCRequest_SetStatusLinesOrQueues,
 			  USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-			  ((dtr != 0) ? SUSBCR_SSL_SETDTR : SUSBCR_SSL_CLRDTR),
+			  ((set & TIOCM_DTR) ? SUSBCR_SSL_SETDTR : SUSBCR_SSL_CLRDTR),
 			  0,
 			  NULL,
 			  0,
 			  KOBIL_TIMEOUT);
-	} else {
-		if (rts != 0)
+	} else if (rts) {
+		if (set & TIOCM_RTS)
 			dev_dbg(dev, "%s - Setting RTS\n", __func__);
 		else
 			dev_dbg(dev, "%s - Clearing RTS\n", __func__);
@@ -462,7 +462,7 @@ static int kobil_tiocmset(struct tty_struct *tty,
 			usb_sndctrlpipe(port->serial->dev, 0),
 			SUSBCRequest_SetStatusLinesOrQueues,
 			USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-			((rts != 0) ? SUSBCR_SSL_SETRTS : SUSBCR_SSL_CLRRTS),
+			((set & TIOCM_RTS) ? SUSBCR_SSL_SETRTS : SUSBCR_SSL_CLRRTS),
 			0,
 			NULL,
 			0,
-- 
2.49.1

[PATCH 3/8] USB: serial: belkin_sa: clean up tiocmset()

[Johan Hovold]

Clean up the tiocmset() implementation by dropping the dtr and rts flags
to make the logic a little easier to follow.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/belkin_sa.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/serial/belkin_sa.c b/drivers/usb/serial/belkin_sa.c
index aa6b4c4ad5ec..5c41c1c82c3f 100644
--- a/drivers/usb/serial/belkin_sa.c
+++ b/drivers/usb/serial/belkin_sa.c
@@ -436,33 +436,23 @@ static int belkin_sa_tiocmset(struct tty_struct *tty,
 	unsigned long control_state;
 	unsigned long flags;
 	int retval = 0;
-	int rts = 0;
-	int dtr = 0;
 
 	spin_lock_irqsave(&priv->lock, flags);
 	control_state = priv->control_state;
 
-	if (set & TIOCM_RTS) {
+	if (set & TIOCM_RTS)
 		control_state |= TIOCM_RTS;
-		rts = 1;
-	}
-	if (set & TIOCM_DTR) {
+	if (set & TIOCM_DTR)
 		control_state |= TIOCM_DTR;
-		dtr = 1;
-	}
-	if (clear & TIOCM_RTS) {
+	if (clear & TIOCM_RTS)
 		control_state &= ~TIOCM_RTS;
-		rts = 1;
-	}
-	if (clear & TIOCM_DTR) {
+	if (clear & TIOCM_DTR)
 		control_state &= ~TIOCM_DTR;
-		dtr = 1;
-	}
 
 	priv->control_state = control_state;
 	spin_unlock_irqrestore(&priv->lock, flags);
 
-	if (rts) {
+	if ((set | clear) & TIOCM_RTS) {
 		retval = BSA_USB_CMD(BELKIN_SA_SET_RTS_REQUEST,
 					!!(control_state & TIOCM_RTS));
 		if (retval < 0) {
@@ -471,7 +461,7 @@ static int belkin_sa_tiocmset(struct tty_struct *tty,
 		}
 	}
 
-	if (dtr) {
+	if ((set | clear) & TIOCM_DTR) {
 		retval = BSA_USB_CMD(BELKIN_SA_SET_DTR_REQUEST,
 					!!(control_state & TIOCM_DTR));
 		if (retval < 0) {
-- 
2.49.1

[PATCH 4/8] USB: serial: kobil_sct: clean up tiocmset()

[Johan Hovold]

Clean up the tiocmset() implementation by simplifying the flag check,
dropping some dev_dbg(), logging errors using dev_err() and using a
common control message call for both DTR and RTS to make the existing
logic easier to follow.

Note that the modem control lines are currently only manipulated in this
function, which therefore does not require any locking.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/kobil_sct.c | 61 +++++++++++++++-------------------
 1 file changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
index 96ea571c436a..b8169783f6f0 100644
--- a/drivers/usb/serial/kobil_sct.c
+++ b/drivers/usb/serial/kobil_sct.c
@@ -418,11 +418,10 @@ static int kobil_tiocmset(struct tty_struct *tty,
 	struct usb_serial_port *port = tty->driver_data;
 	struct device *dev = &port->dev;
 	struct kobil_private *priv;
-	int result = 0;
-	int dtr = 0;
-	int rts = 0;
+	int dtr, rts;
+	int result;
+	u16 val = 0;
 
-	/* FIXME: locking ? */
 	priv = usb_get_serial_port_data(port);
 	if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID
 		|| priv->device_type == KOBIL_KAAN_SIM_PRODUCT_ID) {
@@ -430,46 +429,38 @@ static int kobil_tiocmset(struct tty_struct *tty,
 		return -EINVAL;
 	}
 
-	if (set & TIOCM_RTS)
-		rts = 1;
-	if (set & TIOCM_DTR)
-		dtr = 1;
-	if (clear & TIOCM_RTS)
-		rts = 1;
-	if (clear & TIOCM_DTR)
-		dtr = 1;
+	dtr = (set | clear) & TIOCM_DTR;
+	rts = (set | clear) & TIOCM_RTS;
 
 	if (dtr && priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID) {
 		if (set & TIOCM_DTR)
-			dev_dbg(dev, "%s - Setting DTR\n", __func__);
+			val = SUSBCR_SSL_SETDTR;
 		else
-			dev_dbg(dev, "%s - Clearing DTR\n", __func__);
-		result = usb_control_msg(port->serial->dev,
-			  usb_sndctrlpipe(port->serial->dev, 0),
-			  SUSBCRequest_SetStatusLinesOrQueues,
-			  USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-			  ((set & TIOCM_DTR) ? SUSBCR_SSL_SETDTR : SUSBCR_SSL_CLRDTR),
-			  0,
-			  NULL,
-			  0,
-			  KOBIL_TIMEOUT);
+			val = SUSBCR_SSL_CLRDTR;
 	} else if (rts) {
 		if (set & TIOCM_RTS)
-			dev_dbg(dev, "%s - Setting RTS\n", __func__);
+			val = SUSBCR_SSL_SETRTS;
 		else
-			dev_dbg(dev, "%s - Clearing RTS\n", __func__);
+			val = SUSBCR_SSL_CLRRTS;
+	}
+
+	if (val) {
 		result = usb_control_msg(port->serial->dev,
-			usb_sndctrlpipe(port->serial->dev, 0),
-			SUSBCRequest_SetStatusLinesOrQueues,
-			USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-			((set & TIOCM_RTS) ? SUSBCR_SSL_SETRTS : SUSBCR_SSL_CLRRTS),
-			0,
-			NULL,
-			0,
-			KOBIL_TIMEOUT);
+				usb_sndctrlpipe(port->serial->dev, 0),
+				SUSBCRequest_SetStatusLinesOrQueues,
+				USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
+				val,
+				0,
+				NULL,
+				0,
+				KOBIL_TIMEOUT);
+		if (result < 0) {
+			dev_err(dev, "failed to set status lines: %d\n", result);
+			return result;
+		}
 	}
-	dev_dbg(dev, "%s - Send set_status_line URB returns: %i\n", __func__, result);
-	return (result < 0) ? result : 0;
+
+	return 0;
 }
 
 static void kobil_set_termios(struct tty_struct *tty,
-- 
2.49.1

[PATCH 5/8] USB: serial: kobil_sct: clean up device type checks

[Johan Hovold]

Clean up the driver device type checks by moving logical operators to
the previous line and using consistent indentation of continuation
lines.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/kobil_sct.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
index b8169783f6f0..e1015cab2770 100644
--- a/drivers/usb/serial/kobil_sct.c
+++ b/drivers/usb/serial/kobil_sct.c
@@ -239,8 +239,8 @@ static int kobil_open(struct tty_struct *tty, struct usb_serial_port *port)
 		dev_dbg(dev, "%s - Send reset_all_queues URB returns: %i\n", __func__, result);
 	}
 	if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID ||
-	    priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID ||
-	    priv->device_type == KOBIL_KAAN_SIM_PRODUCT_ID) {
+			priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID ||
+			priv->device_type == KOBIL_KAAN_SIM_PRODUCT_ID) {
 		/* start reading (Adapter B 'cause PNP string) */
 		result = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
 		dev_dbg(dev, "%s - Send read URB returns: %i\n", __func__, result);
@@ -318,9 +318,10 @@ static int kobil_write(struct tty_struct *tty, struct usb_serial_port *port,
 	if (((priv->device_type != KOBIL_ADAPTER_B_PRODUCT_ID) && (priv->filled > 2) && (priv->filled >= (priv->buf[1] + 3))) ||
 	     ((priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID) && (priv->filled > 3) && (priv->filled >= (priv->buf[2] + 4)))) {
 		/* stop reading (except TWIN and KAAN SIM) */
-		if ((priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID)
-			|| (priv->device_type == KOBIL_ADAPTER_K_PRODUCT_ID))
+		if (priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID ||
+				priv->device_type == KOBIL_ADAPTER_K_PRODUCT_ID) {
 			usb_kill_urb(port->interrupt_in_urb);
+		}
 
 		todo = priv->filled - priv->cur_pos;
 
@@ -347,7 +348,7 @@ static int kobil_write(struct tty_struct *tty, struct usb_serial_port *port,
 
 		/* start reading (except TWIN and KAAN SIM) */
 		if (priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID ||
-			priv->device_type == KOBIL_ADAPTER_K_PRODUCT_ID) {
+				priv->device_type == KOBIL_ADAPTER_K_PRODUCT_ID) {
 			result = usb_submit_urb(port->interrupt_in_urb,
 					GFP_ATOMIC);
 			dev_dbg(&port->dev, "%s - Send read URB returns: %i\n", __func__, result);
@@ -373,8 +374,8 @@ static int kobil_tiocmget(struct tty_struct *tty)
 	int transfer_buffer_length = 8;
 
 	priv = usb_get_serial_port_data(port);
-	if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID
-			|| priv->device_type == KOBIL_KAAN_SIM_PRODUCT_ID) {
+	if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID ||
+			priv->device_type == KOBIL_KAAN_SIM_PRODUCT_ID) {
 		/* This device doesn't support ioctl calls */
 		return -EINVAL;
 	}
@@ -423,8 +424,8 @@ static int kobil_tiocmset(struct tty_struct *tty,
 	u16 val = 0;
 
 	priv = usb_get_serial_port_data(port);
-	if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID
-		|| priv->device_type == KOBIL_KAAN_SIM_PRODUCT_ID) {
+	if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID ||
+			priv->device_type == KOBIL_KAAN_SIM_PRODUCT_ID) {
 		/* This device doesn't support ioctl calls */
 		return -EINVAL;
 	}
-- 
2.49.1

[PATCH 6/8] USB: serial: kobil_sct: add control request helpers

[Johan Hovold]

Refactor by adding two control request helpers to make the code more
readable.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/kobil_sct.c | 108 +++++++++------------------------
 1 file changed, 27 insertions(+), 81 deletions(-)

diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
index e1015cab2770..3c13410520ec 100644
--- a/drivers/usb/serial/kobil_sct.c
+++ b/drivers/usb/serial/kobil_sct.c
@@ -109,6 +109,21 @@ struct kobil_private {
 	__u16 device_type;
 };
 
+static int kobil_ctrl_send(struct usb_serial_port *port, u8 req, u16 val)
+{
+	return usb_control_msg(port->serial->dev,
+			usb_sndctrlpipe(port->serial->dev, 0),
+			req, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
+			val, 0, NULL, 0, KOBIL_TIMEOUT);
+}
+
+static int kobil_ctrl_recv(struct usb_serial_port *port, u8 req, u16 val, void *buf, u16 size)
+{
+	return usb_control_msg(port->serial->dev,
+			usb_rcvctrlpipe(port->serial->dev, 0),
+			req, USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_IN,
+			val, 0, buf, size, KOBIL_TIMEOUT);
+}
 
 static int kobil_port_probe(struct usb_serial_port *port)
 {
@@ -176,16 +191,8 @@ static int kobil_open(struct tty_struct *tty, struct usb_serial_port *port)
 		return -ENOMEM;
 
 	/* get hardware version */
-	result = usb_control_msg(port->serial->dev,
-			  usb_rcvctrlpipe(port->serial->dev, 0),
-			  SUSBCRequest_GetMisc,
-			  USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_IN,
-			  SUSBCR_MSC_GetHWVersion,
-			  0,
-			  transfer_buffer,
-			  transfer_buffer_length,
-			  KOBIL_TIMEOUT
-	);
+	result = kobil_ctrl_recv(port, SUSBCRequest_GetMisc, SUSBCR_MSC_GetHWVersion,
+			transfer_buffer, transfer_buffer_length);
 	dev_dbg(dev, "%s - Send get_HW_version URB returns: %i\n", __func__, result);
 	if (result >= 3) {
 		dev_dbg(dev, "Hardware version: %i.%i.%i\n", transfer_buffer[0],
@@ -193,16 +200,8 @@ static int kobil_open(struct tty_struct *tty, struct usb_serial_port *port)
 	}
 
 	/* get firmware version */
-	result = usb_control_msg(port->serial->dev,
-			  usb_rcvctrlpipe(port->serial->dev, 0),
-			  SUSBCRequest_GetMisc,
-			  USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_IN,
-			  SUSBCR_MSC_GetFWVersion,
-			  0,
-			  transfer_buffer,
-			  transfer_buffer_length,
-			  KOBIL_TIMEOUT
-	);
+	result = kobil_ctrl_recv(port, SUSBCRequest_GetMisc, SUSBCR_MSC_GetFWVersion,
+			transfer_buffer, transfer_buffer_length);
 	dev_dbg(dev, "%s - Send get_FW_version URB returns: %i\n", __func__, result);
 	if (result >= 3) {
 		dev_dbg(dev, "Firmware version: %i.%i.%i\n", transfer_buffer[0],
@@ -212,30 +211,12 @@ static int kobil_open(struct tty_struct *tty, struct usb_serial_port *port)
 	if (priv->device_type == KOBIL_ADAPTER_B_PRODUCT_ID ||
 			priv->device_type == KOBIL_ADAPTER_K_PRODUCT_ID) {
 		/* Setting Baudrate, Parity and Stopbits */
-		result = usb_control_msg(port->serial->dev,
-			  usb_sndctrlpipe(port->serial->dev, 0),
-			  SUSBCRequest_SetBaudRateParityAndStopBits,
-			  USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-			  SUSBCR_SBR_9600 | SUSBCR_SPASB_EvenParity |
-							SUSBCR_SPASB_1StopBit,
-			  0,
-			  NULL,
-			  0,
-			  KOBIL_TIMEOUT
-		);
+		result = kobil_ctrl_send(port, SUSBCRequest_SetBaudRateParityAndStopBits,
+				SUSBCR_SBR_9600 | SUSBCR_SPASB_EvenParity | SUSBCR_SPASB_1StopBit);
 		dev_dbg(dev, "%s - Send set_baudrate URB returns: %i\n", __func__, result);
 
 		/* reset all queues */
-		result = usb_control_msg(port->serial->dev,
-			  usb_sndctrlpipe(port->serial->dev, 0),
-			  SUSBCRequest_Misc,
-			  USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-			  SUSBCR_MSC_ResetAllQueues,
-			  0,
-			  NULL,
-			  0,
-			  KOBIL_TIMEOUT
-		);
+		result = kobil_ctrl_send(port, SUSBCRequest_Misc, SUSBCR_MSC_ResetAllQueues);
 		dev_dbg(dev, "%s - Send reset_all_queues URB returns: %i\n", __func__, result);
 	}
 	if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID ||
@@ -385,16 +366,8 @@ static int kobil_tiocmget(struct tty_struct *tty)
 	if (!transfer_buffer)
 		return -ENOMEM;
 
-	result = usb_control_msg(port->serial->dev,
-			  usb_rcvctrlpipe(port->serial->dev, 0),
-			  SUSBCRequest_GetStatusLineState,
-			  USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_IN,
-			  0,
-			  0,
-			  transfer_buffer,
-			  transfer_buffer_length,
-			  KOBIL_TIMEOUT);
-
+	result = kobil_ctrl_recv(port, SUSBCRequest_GetStatusLineState, 0,
+			transfer_buffer, transfer_buffer_length);
 	dev_dbg(&port->dev, "Send get_status_line_state URB returns: %i\n",
 			result);
 	if (result < 1) {
@@ -446,15 +419,7 @@ static int kobil_tiocmset(struct tty_struct *tty,
 	}
 
 	if (val) {
-		result = usb_control_msg(port->serial->dev,
-				usb_sndctrlpipe(port->serial->dev, 0),
-				SUSBCRequest_SetStatusLinesOrQueues,
-				USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-				val,
-				0,
-				NULL,
-				0,
-				KOBIL_TIMEOUT);
+		result = kobil_ctrl_send(port, SUSBCRequest_SetStatusLinesOrQueues, val);
 		if (result < 0) {
 			dev_err(dev, "failed to set status lines: %d\n", result);
 			return result;
@@ -506,16 +471,7 @@ static void kobil_set_termios(struct tty_struct *tty,
 	tty->termios.c_cflag &= ~CMSPAR;
 	tty_encode_baud_rate(tty, speed, speed);
 
-	result = usb_control_msg(port->serial->dev,
-		  usb_sndctrlpipe(port->serial->dev, 0),
-		  SUSBCRequest_SetBaudRateParityAndStopBits,
-		  USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-		  urb_val,
-		  0,
-		  NULL,
-		  0,
-		  KOBIL_TIMEOUT
-		);
+	result = kobil_ctrl_send(port, SUSBCRequest_SetBaudRateParityAndStopBits, urb_val);
 	if (result) {
 		dev_err(&port->dev, "failed to update line settings: %d\n",
 				result);
@@ -536,17 +492,7 @@ static int kobil_ioctl(struct tty_struct *tty,
 
 	switch (cmd) {
 	case TCFLSH:
-		result = usb_control_msg(port->serial->dev,
-			  usb_sndctrlpipe(port->serial->dev, 0),
-			  SUSBCRequest_Misc,
-			  USB_TYPE_VENDOR | USB_RECIP_ENDPOINT | USB_DIR_OUT,
-			  SUSBCR_MSC_ResetAllQueues,
-			  0,
-			  NULL,
-			  0,
-			  KOBIL_TIMEOUT
-			);
-
+		result = kobil_ctrl_send(port, SUSBCRequest_Misc, SUSBCR_MSC_ResetAllQueues);
 		dev_dbg(&port->dev,
 			"%s - Send reset_all_queues (FLUSH) URB returns: %i\n",
 			__func__, result);
-- 
2.49.1

[PATCH 7/8] USB: serial: kobil_sct: clean up set_termios()

[Johan Hovold]

Clean up set_termios() by using a shorter identifier for the control
request value, replacing a ternary operator and adding some missing
braces to make it more readable.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/kobil_sct.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
index 3c13410520ec..cad3cfc63ce7 100644
--- a/drivers/usb/serial/kobil_sct.c
+++ b/drivers/usb/serial/kobil_sct.c
@@ -435,9 +435,9 @@ static void kobil_set_termios(struct tty_struct *tty,
 {
 	struct kobil_private *priv;
 	int result;
-	unsigned short urb_val = 0;
 	int c_cflag = tty->termios.c_cflag;
 	speed_t speed;
+	u16 val;
 
 	priv = usb_get_serial_port_data(port);
 	if (priv->device_type == KOBIL_USBTWIN_PRODUCT_ID ||
@@ -450,28 +450,34 @@ static void kobil_set_termios(struct tty_struct *tty,
 	speed = tty_get_baud_rate(tty);
 	switch (speed) {
 	case 1200:
-		urb_val = SUSBCR_SBR_1200;
+		val = SUSBCR_SBR_1200;
 		break;
 	default:
 		speed = 9600;
 		fallthrough;
 	case 9600:
-		urb_val = SUSBCR_SBR_9600;
+		val = SUSBCR_SBR_9600;
 		break;
 	}
-	urb_val |= (c_cflag & CSTOPB) ? SUSBCR_SPASB_2StopBits :
-							SUSBCR_SPASB_1StopBit;
+
+	if (c_cflag & CSTOPB)
+		val |= SUSBCR_SPASB_2StopBits;
+	else
+		val |= SUSBCR_SPASB_1StopBit;
+
 	if (c_cflag & PARENB) {
 		if  (c_cflag & PARODD)
-			urb_val |= SUSBCR_SPASB_OddParity;
+			val |= SUSBCR_SPASB_OddParity;
 		else
-			urb_val |= SUSBCR_SPASB_EvenParity;
-	} else
-		urb_val |= SUSBCR_SPASB_NoParity;
+			val |= SUSBCR_SPASB_EvenParity;
+	} else {
+		val |= SUSBCR_SPASB_NoParity;
+	}
+
 	tty->termios.c_cflag &= ~CMSPAR;
 	tty_encode_baud_rate(tty, speed, speed);
 
-	result = kobil_ctrl_send(port, SUSBCRequest_SetBaudRateParityAndStopBits, urb_val);
+	result = kobil_ctrl_send(port, SUSBCRequest_SetBaudRateParityAndStopBits, val);
 	if (result) {
 		dev_err(&port->dev, "failed to update line settings: %d\n",
 				result);
-- 
2.49.1

[PATCH 8/8] USB: serial: kobil_sct: drop unnecessary initialisations

[Johan Hovold]

Drop unnecessary initialisation of variables that are always assigned
before being used.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/kobil_sct.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
index cad3cfc63ce7..3a1343d88386 100644
--- a/drivers/usb/serial/kobil_sct.c
+++ b/drivers/usb/serial/kobil_sct.c
@@ -178,10 +178,10 @@ static void kobil_init_termios(struct tty_struct *tty)
 static int kobil_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
 	struct device *dev = &port->dev;
-	int result = 0;
 	struct kobil_private *priv;
 	unsigned char *transfer_buffer;
 	int transfer_buffer_length = 8;
+	int result;
 
 	priv = usb_get_serial_port_data(port);
 
@@ -272,10 +272,8 @@ static void kobil_write_int_callback(struct urb *urb)
 static int kobil_write(struct tty_struct *tty, struct usb_serial_port *port,
 			const unsigned char *buf, int count)
 {
-	int length = 0;
-	int result = 0;
-	int todo = 0;
 	struct kobil_private *priv;
+	int length, todo, result;
 
 	if (count == 0) {
 		dev_dbg(&port->dev, "%s - write request of 0 bytes\n", __func__);
-- 
2.49.1

Re: [PATCH 0/8] USB: serial: fix TIOCMBIS and TIOCMBIC

[Greg KH]

On Wed, Oct 22, 2025 at 05:26:32PM +0200, Johan Hovold wrote:
> Asserting or deasserting a modem control line using TIOCMBIS or TIOCMBIC
> should not deassert any lines that are not in the mask, but two drivers
> got this wrong.
> 
> Included are also some related cleanups.

Nice catch!

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 0/8] USB: serial: fix TIOCMBIS and TIOCMBIC

[Johan Hovold]

On Wed, Oct 22, 2025 at 07:14:34PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 22, 2025 at 05:26:32PM +0200, Johan Hovold wrote:
> > Asserting or deasserting a modem control line using TIOCMBIS or TIOCMBIC
> > should not deassert any lines that are not in the mask, but two drivers
> > got this wrong.
> > 
> > Included are also some related cleanups.
> 
> Nice catch!
> 
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks for reviewing. Now applied.

Johan