[PATCH v2 0/4] USB: serial: Fix coccinelle warnings

[PATCH v2 0/4] USB: serial: Fix coccinelle warnings

[Mathieu OTHACEHE]

This series of patches fixes all coccinelle warnings in
USB serial subsystem.

V2:
* Squash patches per type of issues reported by coccinelle

Mathieu OTHACEHE (4):
  USB: serial: fix compare_const_fl.cocci warnings
  USB: serial: fix returnvar.cocci warnings
  USB: serial: fix boolinit.cocci warnings
  USB: serial: fix semicolon.cocci warnings

 drivers/usb/serial/ch341.c       |  2 +-
 drivers/usb/serial/cyberjack.c   |  3 +--
 drivers/usb/serial/f81232.c      |  2 +-
 drivers/usb/serial/ftdi_sio.c    | 16 ++++++-------
 drivers/usb/serial/ftdi_sio.h    |  8 +++----
 drivers/usb/serial/garmin_gps.c  | 51 ++++++++++++++++++++--------------------
 drivers/usb/serial/iuu_phoenix.c |  4 ++--
 drivers/usb/serial/keyspan.c     |  2 +-
 drivers/usb/serial/kl5kusb105.c  |  3 ---
 drivers/usb/serial/mos7840.c     |  8 +++----
 drivers/usb/serial/quatech2.c    |  2 +-
 drivers/usb/serial/safe_serial.c | 11 +++------
 12 files changed, 51 insertions(+), 61 deletions(-)

-- 
2.6.4

[PATCH v2 1/4] USB: serial: fix compare_const_fl.cocci warnings

[Mathieu OTHACEHE]

Move constants to the right of binary operators.

Generated by: scripts/coccinelle/misc/compare_const_fl.cocci

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
Move constants to the right improve readability in my opinion.
But it's a matter of taste, and nothing is specified in CodingStyle.

 drivers/usb/serial/ch341.c      |  2 +-
 drivers/usb/serial/f81232.c     |  2 +-
 drivers/usb/serial/ftdi_sio.c   | 16 +++++++-------
 drivers/usb/serial/ftdi_sio.h   |  8 +++----
 drivers/usb/serial/garmin_gps.c | 48 ++++++++++++++++++++---------------------
 drivers/usb/serial/mos7840.c    |  4 ++--
 6 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index c73808f..f139488 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -370,7 +370,7 @@ static void ch341_set_termios(struct tty_struct *tty,
 static void ch341_break_ctl(struct tty_struct *tty, int break_state)
 {
 	const uint16_t ch341_break_reg =
-		CH341_REG_BREAK1 | ((uint16_t) CH341_REG_BREAK2 << 8);
+			((uint16_t) CH341_REG_BREAK2 << 8) | CH341_REG_BREAK1;
 	struct usb_serial_port *port = tty->driver_data;
 	int r;
 	uint16_t reg_contents;
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 972f5a5..589d856 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -190,7 +190,7 @@ static int f81232_set_mctrl(struct usb_serial_port *port,
 
 	/* force enable interrupt with OUT2 */
 	mutex_lock(&priv->lock);
-	val = UART_MCR_OUT2 | priv->modem_control;
+	val = priv->modem_control | UART_MCR_OUT2;
 
 	if (clear & TIOCM_DTR)
 		val &= ~UART_MCR_DTR;
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 8c660ae..07b0040 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1320,11 +1320,11 @@ static __u32 get_ftdi_divisor(struct tty_struct *tty,
 		if (baud <= 3000000) {
 			__u16 product_id = le16_to_cpu(
 				port->serial->dev->descriptor.idProduct);
-			if (((FTDI_NDI_HUC_PID == product_id) ||
-			     (FTDI_NDI_SPECTRA_SCU_PID == product_id) ||
-			     (FTDI_NDI_FUTURE_2_PID == product_id) ||
-			     (FTDI_NDI_FUTURE_3_PID == product_id) ||
-			     (FTDI_NDI_AURORA_SCU_PID == product_id)) &&
+			if (((product_id == FTDI_NDI_HUC_PID)		||
+			     (product_id == FTDI_NDI_SPECTRA_SCU_PID)	||
+			     (product_id == FTDI_NDI_FUTURE_2_PID)	||
+			     (product_id == FTDI_NDI_FUTURE_3_PID)	||
+			     (product_id == FTDI_NDI_AURORA_SCU_PID))	&&
 			    (baud == 19200)) {
 				baud = 1200000;
 			}
@@ -2325,7 +2325,7 @@ no_c_cflag_changes:
 				    usb_sndctrlpipe(dev, 0),
 				    FTDI_SIO_SET_FLOW_CTRL_REQUEST,
 				    FTDI_SIO_SET_FLOW_CTRL_REQUEST_TYPE,
-				    0 , (FTDI_SIO_RTS_CTS_HS | priv->interface),
+				    0, (priv->interface | FTDI_SIO_RTS_CTS_HS),
 				    NULL, 0, WDR_TIMEOUT) < 0) {
 			dev_err(ddev, "urb failed to set to rts/cts flow control\n");
 		}
@@ -2354,8 +2354,8 @@ no_c_cflag_changes:
 					    usb_sndctrlpipe(dev, 0),
 					    FTDI_SIO_SET_FLOW_CTRL_REQUEST,
 					    FTDI_SIO_SET_FLOW_CTRL_REQUEST_TYPE,
-					    urb_value , (FTDI_SIO_XON_XOFF_HS
-							 | priv->interface),
+					    urb_value, (priv->interface |
+						    FTDI_SIO_XON_XOFF_HS),
 					    NULL, 0, WDR_TIMEOUT) < 0) {
 				dev_err(&port->dev, "urb failed to set to "
 					"xon/xoff flow control\n");
diff --git a/drivers/usb/serial/ftdi_sio.h b/drivers/usb/serial/ftdi_sio.h
index ed58c6f..bbcc13df 100644
--- a/drivers/usb/serial/ftdi_sio.h
+++ b/drivers/usb/serial/ftdi_sio.h
@@ -239,11 +239,11 @@ enum ftdi_sio_baudrate {
  */
 
 #define FTDI_SIO_SET_DTR_MASK 0x1
-#define FTDI_SIO_SET_DTR_HIGH (1 | (FTDI_SIO_SET_DTR_MASK  << 8))
-#define FTDI_SIO_SET_DTR_LOW  (0 | (FTDI_SIO_SET_DTR_MASK  << 8))
+#define FTDI_SIO_SET_DTR_HIGH ((FTDI_SIO_SET_DTR_MASK  << 8) | 1)
+#define FTDI_SIO_SET_DTR_LOW  ((FTDI_SIO_SET_DTR_MASK  << 8) | 0)
 #define FTDI_SIO_SET_RTS_MASK 0x2
-#define FTDI_SIO_SET_RTS_HIGH (2 | (FTDI_SIO_SET_RTS_MASK << 8))
-#define FTDI_SIO_SET_RTS_LOW (0 | (FTDI_SIO_SET_RTS_MASK << 8))
+#define FTDI_SIO_SET_RTS_HIGH ((FTDI_SIO_SET_RTS_MASK << 8) | 2)
+#define FTDI_SIO_SET_RTS_LOW  ((FTDI_SIO_SET_RTS_MASK << 8) | 0)
 
 /*
  * ControlValue
diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index db591d1..c0bbe44 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -237,10 +237,10 @@ static inline int getDataLength(const __u8 *usbPacket)
  */
 static inline int isAbortTrfCmnd(const unsigned char *buf)
 {
-	if (0 == memcmp(buf, GARMIN_STOP_TRANSFER_REQ,
-					sizeof(GARMIN_STOP_TRANSFER_REQ)) ||
-	    0 == memcmp(buf, GARMIN_STOP_TRANSFER_REQ_V2,
-					sizeof(GARMIN_STOP_TRANSFER_REQ_V2)))
+	if (memcmp(buf, GARMIN_STOP_TRANSFER_REQ,
+			sizeof(GARMIN_STOP_TRANSFER_REQ)) == 0 ||
+	    memcmp(buf, GARMIN_STOP_TRANSFER_REQ_V2,
+			sizeof(GARMIN_STOP_TRANSFER_REQ_V2)) == 0)
 		return 1;
 	else
 		return 0;
@@ -350,7 +350,7 @@ static int gsp_send_ack(struct garmin_data *garmin_data_p, __u8 pkt_id)
 	unsigned  l = 0;
 
 	dev_dbg(&garmin_data_p->port->dev, "%s - pkt-id: 0x%X.\n", __func__,
-		0xFF & pkt_id);
+			pkt_id & 0xFF);
 
 	*ptr++ = DLE;
 	*ptr++ = ACK;
@@ -366,7 +366,7 @@ static int gsp_send_ack(struct garmin_data *garmin_data_p, __u8 pkt_id)
 		*ptr++ = DLE;
 
 	*ptr++ = 0;
-	*ptr++ = 0xFF & (-cksum);
+	*ptr++ = (-cksum) & 0xFF;
 	*ptr++ = DLE;
 	*ptr++ = ETX;
 
@@ -423,9 +423,9 @@ static int gsp_rec_packet(struct garmin_data *garmin_data_p, int count)
 		n++;
 	}
 
-	if ((0xff & (cksum + *recpkt)) != 0) {
+	if (((cksum + *recpkt) & 0xff) != 0) {
 		dev_dbg(dev, "%s - invalid checksum, expected %02x, got %02x\n",
-			__func__, 0xff & -cksum, 0xff & *recpkt);
+			__func__, -cksum & 0xff, *recpkt & 0xff);
 		return -EINVPKT;
 	}
 
@@ -528,7 +528,7 @@ static int gsp_receive(struct garmin_data *garmin_data_p,
 					dev_dbg(dev, "NAK packet complete.\n");
 				} else {
 					dev_dbg(dev, "packet complete - id=0x%X.\n",
-						0xFF & data);
+							data & 0xFF);
 					gsp_rec_packet(garmin_data_p, size);
 				}
 
@@ -636,7 +636,7 @@ static int gsp_send(struct garmin_data *garmin_data_p,
 
 	garmin_data_p->outsize = 0;
 
-	if (GARMIN_LAYERID_APPL != getLayerId(garmin_data_p->outbuffer)) {
+	if (getLayerId(garmin_data_p->outbuffer) != GARMIN_LAYERID_APPL) {
 		dev_dbg(dev, "not an application packet (%d)\n",
 				getLayerId(garmin_data_p->outbuffer));
 		return -1;
@@ -688,7 +688,7 @@ static int gsp_send(struct garmin_data *garmin_data_p,
 			*dst++ = DLE;
 	}
 
-	cksum = 0xFF & -cksum;
+	cksum = -cksum & 0xFF;
 	*dst++ = cksum;
 	if (cksum == DLE)
 		*dst++ = DLE;
@@ -970,7 +970,7 @@ static void garmin_write_bulk_callback(struct urb *urb)
 		struct garmin_data *garmin_data_p =
 					usb_get_serial_port_data(port);
 
-		if (GARMIN_LAYERID_APPL == getLayerId(urb->transfer_buffer)) {
+		if (getLayerId(urb->transfer_buffer) == GARMIN_LAYERID_APPL) {
 
 			if (garmin_data_p->mode == MODE_GARMIN_SERIAL) {
 				gsp_send_ack(garmin_data_p,
@@ -1025,7 +1025,7 @@ static int garmin_write_bulk(struct usb_serial_port *port,
 				dismiss_ack ? NULL : port);
 	urb->transfer_flags |= URB_ZERO_PACKET;
 
-	if (GARMIN_LAYERID_APPL == getLayerId(buffer)) {
+	if (getLayerId(buffer) == GARMIN_LAYERID_APPL) {
 
 		spin_lock_irqsave(&garmin_data_p->lock, flags);
 		garmin_data_p->flags |= APP_REQ_SEEN;
@@ -1077,9 +1077,9 @@ static int garmin_write(struct tty_struct *tty, struct usb_serial_port *port,
 		pktsiz = getDataLength(garmin_data_p->privpkt);
 		pktid  = getPacketId(garmin_data_p->privpkt);
 
-		if (count == (GARMIN_PKTHDR_LENGTH+pktsiz)
-		    && GARMIN_LAYERID_PRIVATE ==
-				getLayerId(garmin_data_p->privpkt)) {
+		if ((GARMIN_PKTHDR_LENGTH+pktsiz) == count &&
+			getLayerId(garmin_data_p->privpkt) ==
+				GARMIN_LAYERID_PRIVATE) {
 
 			dev_dbg(dev, "%s - processing private request %d\n",
 				__func__, pktid);
@@ -1192,7 +1192,7 @@ static void garmin_read_bulk_callback(struct urb *urb)
 	garmin_read_process(garmin_data_p, data, urb->actual_length, 1);
 
 	if (urb->actual_length == 0 &&
-			0 != (garmin_data_p->flags & FLAGS_BULK_IN_RESTART)) {
+			(garmin_data_p->flags & FLAGS_BULK_IN_RESTART) != 0) {
 		spin_lock_irqsave(&garmin_data_p->lock, flags);
 		garmin_data_p->flags &= ~FLAGS_BULK_IN_RESTART;
 		spin_unlock_irqrestore(&garmin_data_p->lock, flags);
@@ -1203,7 +1203,7 @@ static void garmin_read_bulk_callback(struct urb *urb)
 				__func__, retval);
 	} else if (urb->actual_length > 0) {
 		/* Continue trying to read until nothing more is received  */
-		if (0 == (garmin_data_p->flags & FLAGS_THROTTLED)) {
+		if ((garmin_data_p->flags & FLAGS_THROTTLED) == 0) {
 			retval = usb_submit_urb(port->read_urb, GFP_ATOMIC);
 			if (retval)
 				dev_err(&port->dev,
@@ -1249,12 +1249,12 @@ static void garmin_read_int_callback(struct urb *urb)
 			      urb->transfer_buffer);
 
 	if (urb->actual_length == sizeof(GARMIN_BULK_IN_AVAIL_REPLY) &&
-	    0 == memcmp(data, GARMIN_BULK_IN_AVAIL_REPLY,
-				sizeof(GARMIN_BULK_IN_AVAIL_REPLY))) {
+		memcmp(data, GARMIN_BULK_IN_AVAIL_REPLY,
+				sizeof(GARMIN_BULK_IN_AVAIL_REPLY)) == 0) {
 
 		dev_dbg(&port->dev, "%s - bulk data available.\n", __func__);
 
-		if (0 == (garmin_data_p->flags & FLAGS_BULK_IN_ACTIVE)) {
+		if ((garmin_data_p->flags & FLAGS_BULK_IN_ACTIVE) == 0) {
 
 			/* bulk data available */
 			retval = usb_submit_urb(port->read_urb, GFP_ATOMIC);
@@ -1276,8 +1276,8 @@ static void garmin_read_int_callback(struct urb *urb)
 		}
 
 	} else if (urb->actual_length == (4+sizeof(GARMIN_START_SESSION_REPLY))
-			 && 0 == memcmp(data, GARMIN_START_SESSION_REPLY,
-					sizeof(GARMIN_START_SESSION_REPLY))) {
+			 && memcmp(data, GARMIN_START_SESSION_REPLY,
+				 sizeof(GARMIN_START_SESSION_REPLY)) == 0) {
 
 		spin_lock_irqsave(&garmin_data_p->lock, flags);
 		garmin_data_p->flags |= FLAGS_SESSION_REPLY1_SEEN;
@@ -1356,7 +1356,7 @@ static void garmin_unthrottle(struct tty_struct *tty)
 	if (garmin_data_p->mode == MODE_NATIVE)
 		garmin_flush_queue(garmin_data_p);
 
-	if (0 != (garmin_data_p->flags & FLAGS_BULK_IN_ACTIVE)) {
+	if ((garmin_data_p->flags & FLAGS_BULK_IN_ACTIVE) != 0) {
 		status = usb_submit_urb(port->read_urb, GFP_KERNEL);
 		if (status)
 			dev_err(&port->dev,
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 2c69bfc..6a2fab8 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -2039,7 +2039,7 @@ static int mos7810_check(struct usb_serial *serial)
 		/* Send the 1-bit test pattern out to MCS7810 test pin */
 		usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
 			MCS_WRREQ, MCS_WR_RTYPE,
-			(0x0300 | (((test_pattern >> i) & 0x0001) << 1)),
+			((((test_pattern >> i) & 0x0001) << 1) | 0x0300),
 			MODEM_CONTROL_REGISTER, NULL, 0, MOS_WDR_TIMEOUT);
 
 		/* Read the test pattern back */
@@ -2059,7 +2059,7 @@ static int mos7810_check(struct usb_serial *serial)
 
 	/* Restore MCR setting */
 	usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0), MCS_WRREQ,
-		MCS_WR_RTYPE, 0x0300 | mcr_data, MODEM_CONTROL_REGISTER, NULL,
+		MCS_WR_RTYPE, mcr_data | 0x0300, MODEM_CONTROL_REGISTER, NULL,
 		0, MOS_WDR_TIMEOUT);
 
 	kfree(buf);
-- 
2.6.4

Re: [PATCH v2 1/4] USB: serial: fix compare_const_fl.cocci warnings

[Johan Hovold]

On Thu, Feb 04, 2016 at 07:01:27PM +0100, Mathieu OTHACEHE wrote:
> Move constants to the right of binary operators.
> 
> Generated by: scripts/coccinelle/misc/compare_const_fl.cocci
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---
> Move constants to the right improve readability in my opinion.
> But it's a matter of taste, and nothing is specified in CodingStyle.
> 
>  drivers/usb/serial/ch341.c      |  2 +-
>  drivers/usb/serial/f81232.c     |  2 +-
>  drivers/usb/serial/ftdi_sio.c   | 16 +++++++-------
>  drivers/usb/serial/ftdi_sio.h   |  8 +++----
>  drivers/usb/serial/garmin_gps.c | 48 ++++++++++++++++++++---------------------
>  drivers/usb/serial/mos7840.c    |  4 ++--
>  6 files changed, 40 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> index c73808f..f139488 100644
> --- a/drivers/usb/serial/ch341.c
> +++ b/drivers/usb/serial/ch341.c
> @@ -370,7 +370,7 @@ static void ch341_set_termios(struct tty_struct *tty,
>  static void ch341_break_ctl(struct tty_struct *tty, int break_state)
>  {
>  	const uint16_t ch341_break_reg =
> -		CH341_REG_BREAK1 | ((uint16_t) CH341_REG_BREAK2 << 8);
> +			((uint16_t) CH341_REG_BREAK2 << 8) | CH341_REG_BREAK1;
>  	struct usb_serial_port *port = tty->driver_data;
>  	int r;
>  	uint16_t reg_contents;
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index 972f5a5..589d856 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -190,7 +190,7 @@ static int f81232_set_mctrl(struct usb_serial_port *port,
>  
>  	/* force enable interrupt with OUT2 */
>  	mutex_lock(&priv->lock);
> -	val = UART_MCR_OUT2 | priv->modem_control;
> +	val = priv->modem_control | UART_MCR_OUT2;

I don't consider this an improvement as the modem-control bits are the
least significant bits of the register.

>  
>  	if (clear & TIOCM_DTR)
>  		val &= ~UART_MCR_DTR;
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 8c660ae..07b0040 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -1320,11 +1320,11 @@ static __u32 get_ftdi_divisor(struct tty_struct *tty,
>  		if (baud <= 3000000) {
>  			__u16 product_id = le16_to_cpu(
>  				port->serial->dev->descriptor.idProduct);
> -			if (((FTDI_NDI_HUC_PID == product_id) ||
> -			     (FTDI_NDI_SPECTRA_SCU_PID == product_id) ||
> -			     (FTDI_NDI_FUTURE_2_PID == product_id) ||
> -			     (FTDI_NDI_FUTURE_3_PID == product_id) ||
> -			     (FTDI_NDI_AURORA_SCU_PID == product_id)) &&
> +			if (((product_id == FTDI_NDI_HUC_PID)		||
> +			     (product_id == FTDI_NDI_SPECTRA_SCU_PID)	||
> +			     (product_id == FTDI_NDI_FUTURE_2_PID)	||
> +			     (product_id == FTDI_NDI_FUTURE_3_PID)	||
> +			     (product_id == FTDI_NDI_AURORA_SCU_PID))	&&
>  			    (baud == 19200)) {
>  				baud = 1200000;
>  			}
> @@ -2325,7 +2325,7 @@ no_c_cflag_changes:
>  				    usb_sndctrlpipe(dev, 0),
>  				    FTDI_SIO_SET_FLOW_CTRL_REQUEST,
>  				    FTDI_SIO_SET_FLOW_CTRL_REQUEST_TYPE,
> -				    0 , (FTDI_SIO_RTS_CTS_HS | priv->interface),
> +				    0, (priv->interface | FTDI_SIO_RTS_CTS_HS),

Similarly, here FTDI_SIO_RTS_CTS_HS is in the most significant byte.

>  				    NULL, 0, WDR_TIMEOUT) < 0) {
>  			dev_err(ddev, "urb failed to set to rts/cts flow control\n");
>  		}
> @@ -2354,8 +2354,8 @@ no_c_cflag_changes:
>  					    usb_sndctrlpipe(dev, 0),
>  					    FTDI_SIO_SET_FLOW_CTRL_REQUEST,
>  					    FTDI_SIO_SET_FLOW_CTRL_REQUEST_TYPE,
> -					    urb_value , (FTDI_SIO_XON_XOFF_HS
> -							 | priv->interface),
> +					    urb_value, (priv->interface |
> +						    FTDI_SIO_XON_XOFF_HS),

Same here.

>  					    NULL, 0, WDR_TIMEOUT) < 0) {
>  				dev_err(&port->dev, "urb failed to set to "
>  					"xon/xoff flow control\n");
> diff --git a/drivers/usb/serial/ftdi_sio.h b/drivers/usb/serial/ftdi_sio.h
> index ed58c6f..bbcc13df 100644
> --- a/drivers/usb/serial/ftdi_sio.h
> +++ b/drivers/usb/serial/ftdi_sio.h
> @@ -239,11 +239,11 @@ enum ftdi_sio_baudrate {
>   */
>  
>  #define FTDI_SIO_SET_DTR_MASK 0x1
> -#define FTDI_SIO_SET_DTR_HIGH (1 | (FTDI_SIO_SET_DTR_MASK  << 8))
> -#define FTDI_SIO_SET_DTR_LOW  (0 | (FTDI_SIO_SET_DTR_MASK  << 8))
> +#define FTDI_SIO_SET_DTR_HIGH ((FTDI_SIO_SET_DTR_MASK  << 8) | 1)
> +#define FTDI_SIO_SET_DTR_LOW  ((FTDI_SIO_SET_DTR_MASK  << 8) | 0)
>  #define FTDI_SIO_SET_RTS_MASK 0x2
> -#define FTDI_SIO_SET_RTS_HIGH (2 | (FTDI_SIO_SET_RTS_MASK << 8))
> -#define FTDI_SIO_SET_RTS_LOW (0 | (FTDI_SIO_SET_RTS_MASK << 8))
> +#define FTDI_SIO_SET_RTS_HIGH ((FTDI_SIO_SET_RTS_MASK << 8) | 2)
> +#define FTDI_SIO_SET_RTS_LOW  ((FTDI_SIO_SET_RTS_MASK << 8) | 0)
>  
>  /*
>   * ControlValue
> diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
> index db591d1..c0bbe44 100644
> --- a/drivers/usb/serial/garmin_gps.c
> +++ b/drivers/usb/serial/garmin_gps.c
> @@ -237,10 +237,10 @@ static inline int getDataLength(const __u8 *usbPacket)
>   */
>  static inline int isAbortTrfCmnd(const unsigned char *buf)
>  {
> -	if (0 == memcmp(buf, GARMIN_STOP_TRANSFER_REQ,
> -					sizeof(GARMIN_STOP_TRANSFER_REQ)) ||
> -	    0 == memcmp(buf, GARMIN_STOP_TRANSFER_REQ_V2,
> -					sizeof(GARMIN_STOP_TRANSFER_REQ_V2)))
> +	if (memcmp(buf, GARMIN_STOP_TRANSFER_REQ,
> +			sizeof(GARMIN_STOP_TRANSFER_REQ)) == 0 ||
> +	    memcmp(buf, GARMIN_STOP_TRANSFER_REQ_V2,
> +			sizeof(GARMIN_STOP_TRANSFER_REQ_V2)) == 0)
>  		return 1;
>  	else
>  		return 0;
> @@ -350,7 +350,7 @@ static int gsp_send_ack(struct garmin_data *garmin_data_p, __u8 pkt_id)
>  	unsigned  l = 0;
>  
>  	dev_dbg(&garmin_data_p->port->dev, "%s - pkt-id: 0x%X.\n", __func__,
> -		0xFF & pkt_id);
> +			pkt_id & 0xFF);

pkt_id is u8

>  
>  	*ptr++ = DLE;
>  	*ptr++ = ACK;
> @@ -366,7 +366,7 @@ static int gsp_send_ack(struct garmin_data *garmin_data_p, __u8 pkt_id)
>  		*ptr++ = DLE;
>  
>  	*ptr++ = 0;
> -	*ptr++ = 0xFF & (-cksum);
> +	*ptr++ = (-cksum) & 0xFF;
>  	*ptr++ = DLE;
>  	*ptr++ = ETX;
>  
> @@ -423,9 +423,9 @@ static int gsp_rec_packet(struct garmin_data *garmin_data_p, int count)
>  		n++;
>  	}
>  
> -	if ((0xff & (cksum + *recpkt)) != 0) {
> +	if (((cksum + *recpkt) & 0xff) != 0) {
>  		dev_dbg(dev, "%s - invalid checksum, expected %02x, got %02x\n",
> -			__func__, 0xff & -cksum, 0xff & *recpkt);
> +			__func__, -cksum & 0xff, *recpkt & 0xff);

and so is recpkt

>  		return -EINVPKT;
>  	}
>  
> @@ -528,7 +528,7 @@ static int gsp_receive(struct garmin_data *garmin_data_p,
>  					dev_dbg(dev, "NAK packet complete.\n");
>  				} else {
>  					dev_dbg(dev, "packet complete - id=0x%X.\n",
> -						0xFF & data);
> +							data & 0xFF);

and data

>  					gsp_rec_packet(garmin_data_p, size);
>  				}
>  
> @@ -636,7 +636,7 @@ static int gsp_send(struct garmin_data *garmin_data_p,
>  
>  	garmin_data_p->outsize = 0;
>  
> -	if (GARMIN_LAYERID_APPL != getLayerId(garmin_data_p->outbuffer)) {
> +	if (getLayerId(garmin_data_p->outbuffer) != GARMIN_LAYERID_APPL) {
>  		dev_dbg(dev, "not an application packet (%d)\n",
>  				getLayerId(garmin_data_p->outbuffer));
>  		return -1;
> @@ -688,7 +688,7 @@ static int gsp_send(struct garmin_data *garmin_data_p,
>  			*dst++ = DLE;
>  	}
>  
> -	cksum = 0xFF & -cksum;
> +	cksum = -cksum & 0xFF;
>  	*dst++ = cksum;
>  	if (cksum == DLE)
>  		*dst++ = DLE;
> @@ -970,7 +970,7 @@ static void garmin_write_bulk_callback(struct urb *urb)
>  		struct garmin_data *garmin_data_p =
>  					usb_get_serial_port_data(port);
>  
> -		if (GARMIN_LAYERID_APPL == getLayerId(urb->transfer_buffer)) {
> +		if (getLayerId(urb->transfer_buffer) == GARMIN_LAYERID_APPL) {
>  
>  			if (garmin_data_p->mode == MODE_GARMIN_SERIAL) {
>  				gsp_send_ack(garmin_data_p,
> @@ -1025,7 +1025,7 @@ static int garmin_write_bulk(struct usb_serial_port *port,
>  				dismiss_ack ? NULL : port);
>  	urb->transfer_flags |= URB_ZERO_PACKET;
>  
> -	if (GARMIN_LAYERID_APPL == getLayerId(buffer)) {
> +	if (getLayerId(buffer) == GARMIN_LAYERID_APPL) {
>  
>  		spin_lock_irqsave(&garmin_data_p->lock, flags);
>  		garmin_data_p->flags |= APP_REQ_SEEN;
> @@ -1077,9 +1077,9 @@ static int garmin_write(struct tty_struct *tty, struct usb_serial_port *port,
>  		pktsiz = getDataLength(garmin_data_p->privpkt);
>  		pktid  = getPacketId(garmin_data_p->privpkt);
>  
> -		if (count == (GARMIN_PKTHDR_LENGTH+pktsiz)
> -		    && GARMIN_LAYERID_PRIVATE ==
> -				getLayerId(garmin_data_p->privpkt)) {
> +		if ((GARMIN_PKTHDR_LENGTH+pktsiz) == count &&

Here I believe count should remain on the left side.

> +			getLayerId(garmin_data_p->privpkt) ==
> +				GARMIN_LAYERID_PRIVATE) {
>  
>  			dev_dbg(dev, "%s - processing private request %d\n",
>  				__func__, pktid);
> @@ -1192,7 +1192,7 @@ static void garmin_read_bulk_callback(struct urb *urb)
>  	garmin_read_process(garmin_data_p, data, urb->actual_length, 1);
>  
>  	if (urb->actual_length == 0 &&
> -			0 != (garmin_data_p->flags & FLAGS_BULK_IN_RESTART)) {
> +			(garmin_data_p->flags & FLAGS_BULK_IN_RESTART) != 0) {
>  		spin_lock_irqsave(&garmin_data_p->lock, flags);
>  		garmin_data_p->flags &= ~FLAGS_BULK_IN_RESTART;
>  		spin_unlock_irqrestore(&garmin_data_p->lock, flags);
> @@ -1203,7 +1203,7 @@ static void garmin_read_bulk_callback(struct urb *urb)
>  				__func__, retval);
>  	} else if (urb->actual_length > 0) {
>  		/* Continue trying to read until nothing more is received  */
> -		if (0 == (garmin_data_p->flags & FLAGS_THROTTLED)) {
> +		if ((garmin_data_p->flags & FLAGS_THROTTLED) == 0) {
>  			retval = usb_submit_urb(port->read_urb, GFP_ATOMIC);
>  			if (retval)
>  				dev_err(&port->dev,
> @@ -1249,12 +1249,12 @@ static void garmin_read_int_callback(struct urb *urb)
>  			      urb->transfer_buffer);
>  
>  	if (urb->actual_length == sizeof(GARMIN_BULK_IN_AVAIL_REPLY) &&
> -	    0 == memcmp(data, GARMIN_BULK_IN_AVAIL_REPLY,
> -				sizeof(GARMIN_BULK_IN_AVAIL_REPLY))) {
> +		memcmp(data, GARMIN_BULK_IN_AVAIL_REPLY,
> +				sizeof(GARMIN_BULK_IN_AVAIL_REPLY)) == 0) {
>  
>  		dev_dbg(&port->dev, "%s - bulk data available.\n", __func__);
>  
> -		if (0 == (garmin_data_p->flags & FLAGS_BULK_IN_ACTIVE)) {
> +		if ((garmin_data_p->flags & FLAGS_BULK_IN_ACTIVE) == 0) {
>  
>  			/* bulk data available */
>  			retval = usb_submit_urb(port->read_urb, GFP_ATOMIC);
> @@ -1276,8 +1276,8 @@ static void garmin_read_int_callback(struct urb *urb)
>  		}
>  
>  	} else if (urb->actual_length == (4+sizeof(GARMIN_START_SESSION_REPLY))
> -			 && 0 == memcmp(data, GARMIN_START_SESSION_REPLY,
> -					sizeof(GARMIN_START_SESSION_REPLY))) {
> +			 && memcmp(data, GARMIN_START_SESSION_REPLY,
> +				 sizeof(GARMIN_START_SESSION_REPLY)) == 0) {
>  
>  		spin_lock_irqsave(&garmin_data_p->lock, flags);
>  		garmin_data_p->flags |= FLAGS_SESSION_REPLY1_SEEN;
> @@ -1356,7 +1356,7 @@ static void garmin_unthrottle(struct tty_struct *tty)
>  	if (garmin_data_p->mode == MODE_NATIVE)
>  		garmin_flush_queue(garmin_data_p);
>  
> -	if (0 != (garmin_data_p->flags & FLAGS_BULK_IN_ACTIVE)) {
> +	if ((garmin_data_p->flags & FLAGS_BULK_IN_ACTIVE) != 0) {
>  		status = usb_submit_urb(port->read_urb, GFP_KERNEL);
>  		if (status)
>  			dev_err(&port->dev,
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index 2c69bfc..6a2fab8 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -2039,7 +2039,7 @@ static int mos7810_check(struct usb_serial *serial)
>  		/* Send the 1-bit test pattern out to MCS7810 test pin */
>  		usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
>  			MCS_WRREQ, MCS_WR_RTYPE,
> -			(0x0300 | (((test_pattern >> i) & 0x0001) << 1)),
> +			((((test_pattern >> i) & 0x0001) << 1) | 0x0300),

Again, I find it more natural to keep the most-significant bits to the
left.

>  			MODEM_CONTROL_REGISTER, NULL, 0, MOS_WDR_TIMEOUT);
>  
>  		/* Read the test pattern back */
> @@ -2059,7 +2059,7 @@ static int mos7810_check(struct usb_serial *serial)
>  
>  	/* Restore MCR setting */
>  	usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0), MCS_WRREQ,
> -		MCS_WR_RTYPE, 0x0300 | mcr_data, MODEM_CONTROL_REGISTER, NULL,
> +		MCS_WR_RTYPE, mcr_data | 0x0300, MODEM_CONTROL_REGISTER, NULL,

Ditto.

>  		0, MOS_WDR_TIMEOUT);
>  
>  	kfree(buf);

Ok, so maybe partially accepting this patch wasn't such a good idea, but
I do find that at least the "y == f()" => "f() == y" transformations
increase readability.

Thanks,
Johan

[PATCH v2 2/4] USB: serial: fix returnvar.cocci warnings

[Mathieu OTHACEHE]

Remove unneeded variables when "0" can be returned.

Generated by: scripts/coccinelle/misc/returnvar.cocci

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/cyberjack.c  | 3 +--
 drivers/usb/serial/garmin_gps.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c
index 2916dea..5f17a3b 100644
--- a/drivers/usb/serial/cyberjack.c
+++ b/drivers/usb/serial/cyberjack.c
@@ -140,7 +140,6 @@ static int  cyberjack_open(struct tty_struct *tty,
 {
 	struct cyberjack_private *priv;
 	unsigned long flags;
-	int result = 0;
 
 	dev_dbg(&port->dev, "%s - usb_clear_halt\n", __func__);
 	usb_clear_halt(port->serial->dev, port->write_urb->pipe);
@@ -152,7 +151,7 @@ static int  cyberjack_open(struct tty_struct *tty,
 	priv->wrsent = 0;
 	spin_unlock_irqrestore(&priv->lock, flags);
 
-	return result;
+	return 0;
 }
 
 static void cyberjack_close(struct usb_serial_port *port)
diff --git a/drivers/usb/serial/garmin_gps.c b/drivers/usb/serial/garmin_gps.c
index c0bbe44..91a9b55 100644
--- a/drivers/usb/serial/garmin_gps.c
+++ b/drivers/usb/serial/garmin_gps.c
@@ -860,7 +860,6 @@ static int process_resetdev_request(struct usb_serial_port *port)
 static int garmin_clear(struct garmin_data *garmin_data_p)
 {
 	unsigned long flags;
-	int status = 0;
 
 	/* flush all queued data */
 	pkt_clear(garmin_data_p);
@@ -870,7 +869,7 @@ static int garmin_clear(struct garmin_data *garmin_data_p)
 	garmin_data_p->outsize = 0;
 	spin_unlock_irqrestore(&garmin_data_p->lock, flags);
 
-	return status;
+	return 0;
 }
 
 
-- 
2.6.4

[PATCH v2 3/4] USB: serial: fix boolinit.cocci warnings

[Mathieu OTHACEHE]

Bool initializations should use true and false. Bool tests don't need
comparisons.

Also, use IS_ENABLED instead of ifdef.

Generated by: scripts/coccinelle/misc/boolinit.cocci

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/iuu_phoenix.c |  4 ++--
 drivers/usb/serial/mos7840.c     |  4 ++--
 drivers/usb/serial/quatech2.c    |  2 +-
 drivers/usb/serial/safe_serial.c | 11 +++--------
 4 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/serial/iuu_phoenix.c b/drivers/usb/serial/iuu_phoenix.c
index 5ad4a0f..344b4ee 100644
--- a/drivers/usb/serial/iuu_phoenix.c
+++ b/drivers/usb/serial/iuu_phoenix.c
@@ -360,7 +360,7 @@ static void iuu_led_activity_on(struct urb *urb)
 	int result;
 	char *buf_ptr = port->write_urb->transfer_buffer;
 	*buf_ptr++ = IUU_SET_LED;
-	if (xmas == 1) {
+	if (xmas) {
 		get_random_bytes(buf_ptr, 6);
 		*(buf_ptr+7) = 1;
 	} else {
@@ -380,7 +380,7 @@ static void iuu_led_activity_off(struct urb *urb)
 	struct usb_serial_port *port = urb->context;
 	int result;
 	char *buf_ptr = port->write_urb->transfer_buffer;
-	if (xmas == 1) {
+	if (xmas) {
 		iuu_rxcmd(urb);
 		return;
 	} else {
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 6a2fab8..982a3fb 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -1842,7 +1842,7 @@ static void mos7840_change_port_settings(struct tty_struct *tty,
 	Data = 0x0c;
 	mos7840_set_uart_reg(port, INTERRUPT_ENABLE_REGISTER, Data);
 
-	if (mos7840_port->read_urb_busy == false) {
+	if (!mos7840_port->read_urb_busy) {
 		mos7840_port->read_urb_busy = true;
 		status = usb_submit_urb(mos7840_port->read_urb, GFP_KERNEL);
 		if (status) {
@@ -1906,7 +1906,7 @@ static void mos7840_set_termios(struct tty_struct *tty,
 		return;
 	}
 
-	if (mos7840_port->read_urb_busy == false) {
+	if (!mos7840_port->read_urb_busy) {
 		mos7840_port->read_urb_busy = true;
 		status = usb_submit_urb(mos7840_port->read_urb, GFP_KERNEL);
 		if (status) {
diff --git a/drivers/usb/serial/quatech2.c b/drivers/usb/serial/quatech2.c
index 504f5bf..2df8ad5 100644
--- a/drivers/usb/serial/quatech2.c
+++ b/drivers/usb/serial/quatech2.c
@@ -973,7 +973,7 @@ static int qt2_write(struct tty_struct *tty,
 
 	data = write_urb->transfer_buffer;
 	spin_lock_irqsave(&port_priv->urb_lock, flags);
-	if (port_priv->urb_in_use == true) {
+	if (port_priv->urb_in_use) {
 		dev_err(&port->dev, "qt2_write - urb is in use\n");
 		goto write_out;
 	}
diff --git a/drivers/usb/serial/safe_serial.c b/drivers/usb/serial/safe_serial.c
index b2dff0f..93c6c9b 100644
--- a/drivers/usb/serial/safe_serial.c
+++ b/drivers/usb/serial/safe_serial.c
@@ -76,13 +76,8 @@
 #include <linux/usb.h>
 #include <linux/usb/serial.h>
 
-
-#ifndef CONFIG_USB_SERIAL_SAFE_PADDED
-#define CONFIG_USB_SERIAL_SAFE_PADDED 0
-#endif
-
-static bool safe = 1;
-static bool padded = CONFIG_USB_SERIAL_SAFE_PADDED;
+static bool safe = true;
+static bool padded = IS_ENABLED(CONFIG_USB_SERIAL_SAFE_PADDED);
 
 #define DRIVER_AUTHOR "sl@lineo.com, tbr@lineo.com, Johan Hovold <jhovold@gmail.com>"
 #define DRIVER_DESC "USB Safe Encapsulated Serial"
@@ -278,7 +273,7 @@ static int safe_startup(struct usb_serial *serial)
 	case LINEO_SAFESERIAL_CRC:
 		break;
 	case LINEO_SAFESERIAL_CRC_PADDED:
-		padded = 1;
+		padded = true;
 		break;
 	default:
 		return -EINVAL;
-- 
2.6.4

[PATCH v2 4/4] USB: serial: fix semicolon.cocci warnings

[Mathieu OTHACEHE]

Remove unneeded semicolons.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
---
 drivers/usb/serial/keyspan.c    | 2 +-
 drivers/usb/serial/kl5kusb105.c | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
index e07b15e..b6bd8e4 100644
--- a/drivers/usb/serial/keyspan.c
+++ b/drivers/usb/serial/keyspan.c
@@ -1963,7 +1963,7 @@ static int keyspan_usa49_send_setup(struct usb_serial *serial,
 	if (d_details->product_id == keyspan_usa49wg_product_id) {
 		dr = (void *)(s_priv->ctrl_buf);
 		dr->bRequestType = USB_TYPE_VENDOR | USB_DIR_OUT;
-		dr->bRequest = 0xB0;	/* 49wg control message */;
+		dr->bRequest = 0xB0;	/* 49wg control message */
 		dr->wValue = 0;
 		dr->wIndex = 0;
 		dr->wLength = cpu_to_le16(sizeof(msg));
diff --git a/drivers/usb/serial/kl5kusb105.c b/drivers/usb/serial/kl5kusb105.c
index e020ad2..fc5d3a7 100644
--- a/drivers/usb/serial/kl5kusb105.c
+++ b/drivers/usb/serial/kl5kusb105.c
@@ -472,7 +472,6 @@ static void klsi_105_set_termios(struct tty_struct *tty,
 		/* maybe this should be simulated by sending read
 		 * disable and read enable messages?
 		 */
-		;
 #if 0
 		priv->control_state &= ~(TIOCM_DTR | TIOCM_RTS);
 		mct_u232_set_modem_ctrl(serial, priv->control_state);
@@ -527,7 +526,6 @@ static void klsi_105_set_termios(struct tty_struct *tty,
 
 		mct_u232_set_line_ctrl(serial, priv->last_lcr);
 #endif
-		;
 	}
 	/*
 	 * Set flow control: well, I do not really now how to handle DTR/RTS.
@@ -546,7 +544,6 @@ static void klsi_105_set_termios(struct tty_struct *tty,
 			priv->control_state &= ~(TIOCM_DTR | TIOCM_RTS);
 		mct_u232_set_modem_ctrl(serial, priv->control_state);
 #endif
-		;
 	}
 	memcpy(cfg, &priv->cfg, sizeof(*cfg));
 	spin_unlock_irqrestore(&priv->lock, flags);
-- 
2.6.4

Re: [PATCH v2 4/4] USB: serial: fix semicolon.cocci warnings

[Johan Hovold]

On Thu, Feb 04, 2016 at 07:01:30PM +0100, Mathieu OTHACEHE wrote:
> Remove unneeded semicolons.
> 
> Generated by: scripts/coccinelle/misc/semicolon.cocci
> 
> Signed-off-by: Mathieu OTHACEHE <m.othacehe@gmail.com>
> ---

Rest of the series now applied as well.

Thanks,
Johan