Skip to contentSkip to conversation

USB: serial: fix TIOCMBIS and TIOCMBIC

PATCH In review Johan Hovold 2 participants 11 messages Oct 22, 2025 – Oct 27, 2025

Subsystems (3)
Stable branch USB USB serial
lore
Share & tools
lore

Johan Hovold

author maintainer
#0 lore raw

cover USB: serial: fix TIOCMBIS and TIOCMBIC

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(-)
Signature · 1 line
2.49.1
author maintainer
#1 lore raw

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

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 [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/belkin_sa.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)
drivers/usb/serial/belkin_sa.c +17 -11
index 44f5b58beec9..aa6b4c4ad5ec 100644
@@ -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;
Signature · 1 line
2.49.1
author maintainer
#2 lore raw

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

In reply to 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 [email protected]
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/kobil_sct.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/usb/serial/kobil_sct.c +9 -9
index 464433be2034..96ea571c436a 100644
@@ -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,
Signature · 1 line
2.49.1
author maintainer
#3 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/usb/serial/belkin_sa.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)
drivers/usb/serial/belkin_sa.c +6 -16
index aa6b4c4ad5ec..5c41c1c82c3f 100644
@@ -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) {
Signature · 1 line
2.49.1
author maintainer
#4 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/usb/serial/kobil_sct.c | 61 +++++++++++++++-------------------
 1 file changed, 26 insertions(+), 35 deletions(-)
drivers/usb/serial/kobil_sct.c +26 -35
index 96ea571c436a..b8169783f6f0 100644
@@ -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,
Signature · 1 line
2.49.1
author maintainer
#5 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/usb/serial/kobil_sct.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)
drivers/usb/serial/kobil_sct.c +10 -9
index b8169783f6f0..e1015cab2770 100644
@@ -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;
 	}
Signature · 1 line
2.49.1
author maintainer
#6 lore raw

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

In reply to Johan Hovold

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

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/kobil_sct.c | 108 +++++++++------------------------
 1 file changed, 27 insertions(+), 81 deletions(-)
drivers/usb/serial/kobil_sct.c +27 -81
index e1015cab2770..3c13410520ec 100644
@@ -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);
Signature · 1 line
2.49.1
author maintainer
#7 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/usb/serial/kobil_sct.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/usb/serial/kobil_sct.c +16 -10
index 3c13410520ec..cad3cfc63ce7 100644
@@ -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);
Signature · 1 line
2.49.1
author maintainer
#8 lore raw

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

In reply to Johan Hovold

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

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/kobil_sct.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/usb/serial/kobil_sct.c +2 -4
index cad3cfc63ce7..3a1343d88386 100644
@@ -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__);
Signature · 1 line
2.49.1
Reviewed-by Greg Kroah-Hartman on the v1 series
#9 lore raw
In reply to Johan Hovold
Quoted 5 lines · 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.

Nice catch!

Reviewed-by Greg Kroah-Hartman <[email protected]>
author maintainer
#10 lore raw
Quoted 10 lines · Greg Kroah-Hartman
Quoted 5 lines · 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.

Nice catch!

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

Thanks for reviewing. Now applied.

Johan