USB: serial: W=1 related cleanups

PATCH In review Johan Hovold 2 participants 7 messages Jul 1, 2020

Subsystems (2)

USB USB serial

lore

Share & tools

lore

Johan Hovold

author maintainer

2020-07-01 16:54 UTC #0 lore raw

cover USB: serial: W=1 related cleanups

People have started submitting patches to "fix" W=1 warning (some even reported by bots). Let's just address the remaining ones in USB serial to avoid having to spend more time on this.

Johan

Johan Hovold (5):
USB: serial: garmin_gps: don't compile unused packet definitions
USB: serial: iuu_phoenix: drop unused URB submission results
USB: serial: keyspan_pda: drop unused firmware reset status
USB: serial: kobil_sct: log failure to update line settings
USB: serial: quatech2: drop two stub functions

 drivers/usb/serial/garmin_gps.c  | 13 ++++++++-----
 drivers/usb/serial/iuu_phoenix.c | 12 ++++--------
 drivers/usb/serial/keyspan_pda.c |  3 +--
 drivers/usb/serial/kobil_sct.c   |  4 ++++
 drivers/usb/serial/quatech2.c    | 18 +-----------------
 5 files changed, 18 insertions(+), 32 deletions(-)

Signature · 1 line

2.26.2

Johan Hovold

author maintainer

2020-07-01 16:54 UTC #1 lore raw

PATCH 1/5 USB: serial: garmin_gps: don't compile unused packet definitions

Don't compile the four unused packet definitions but keep them around for documentation purposes.

This avoids the corresponding W=1 (-Wunused-const-variable) warnings.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/garmin_gps.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

drivers/usb/serial/garmin_gps.c +8 -5

index d63072fee099..c02c19bb1183 100644
@@ -179,19 +179,22 @@ static unsigned char const GARMIN_START_SESSION_REPLY[]
 	= { 0, 0, 0, 0,  6, 0, 0, 0, 4, 0, 0, 0 };
 static unsigned char const GARMIN_BULK_IN_AVAIL_REPLY[]
 	= { 0, 0, 0, 0,  2, 0, 0, 0, 0, 0, 0, 0 };
+static unsigned char const GARMIN_STOP_TRANSFER_REQ[]
+	= { 20, 0, 0, 0,  10, 0, 0, 0, 2, 0, 0, 0, 0, 0 };
+static unsigned char const GARMIN_STOP_TRANSFER_REQ_V2[]
+	= { 20, 0, 0, 0,  10, 0, 0, 0, 1, 0, 0, 0, 0 };
+
+/* packets currently unused, left as documentation */
+#if 0
 static unsigned char const GARMIN_APP_LAYER_REPLY[]
 	= { 0x14, 0, 0, 0 };
 static unsigned char const GARMIN_START_PVT_REQ[]
 	= { 20, 0, 0, 0,  10, 0, 0, 0, 2, 0, 0, 0, 49, 0 };
 static unsigned char const GARMIN_STOP_PVT_REQ[]
 	= { 20, 0, 0, 0,  10, 0, 0, 0, 2, 0, 0, 0, 50, 0 };
-static unsigned char const GARMIN_STOP_TRANSFER_REQ[]
-	= { 20, 0, 0, 0,  10, 0, 0, 0, 2, 0, 0, 0, 0, 0 };
-static unsigned char const GARMIN_STOP_TRANSFER_REQ_V2[]
-	= { 20, 0, 0, 0,  10, 0, 0, 0, 1, 0, 0, 0, 0 };
 static unsigned char const PRIVATE_REQ[]
 	=    { 0x4B, 0x6E, 0x10, 0x01,  0xFF, 0, 0, 0, 0xFF, 0, 0, 0 };
-
+#endif
 
 
 static const struct usb_device_id id_table[] = {

Signature · 1 line

2.26.2

Johan Hovold

author maintainer

2020-07-01 16:54 UTC #2 lore raw

PATCH 2/5 USB: serial: iuu_phoenix: drop unused URB submission results

In reply to Johan Hovold

The driver is submitting URBs in various completion callbacks without bothering to log errors yet still assigned the return value to temporary variables. Let's drop those temporaries.

This suppresses the corresponding W=1 (-Wunused-but-set-variable) warnings.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/iuu_phoenix.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

drivers/usb/serial/iuu_phoenix.c +4 -8

index d5bff69b1769..6336616fee49 100644
@@ -158,7 +158,6 @@ static int iuu_tiocmget(struct tty_struct *tty)
 static void iuu_rxcmd(struct urb *urb)
 {
 	struct usb_serial_port *port = urb->context;
-	int result;
 	int status = urb->status;
 
 	if (status) {

@@ -174,7 +173,7 @@ static void iuu_rxcmd(struct urb *urb)
 					  port->bulk_out_endpointAddress),
 			  port->write_urb->transfer_buffer, 1,
 			  read_rxcmd_callback, port);
-	result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
+	usb_submit_urb(port->write_urb, GFP_ATOMIC);
 }
 
 static int iuu_reset(struct usb_serial_port *port, u8 wt)

@@ -241,7 +240,6 @@ static void iuu_update_status_callback(struct urb *urb)
 static void iuu_status_callback(struct urb *urb)
 {
 	struct usb_serial_port *port = urb->context;
-	int result;
 	int status = urb->status;
 
 	dev_dbg(&port->dev, "%s - status = %d\n", __func__, status);

@@ -250,7 +248,7 @@ static void iuu_status_callback(struct urb *urb)
 					  port->bulk_in_endpointAddress),
 			  port->read_urb->transfer_buffer, 256,
 			  iuu_update_status_callback, port);
-	result = usb_submit_urb(port->read_urb, GFP_ATOMIC);
+	usb_submit_urb(port->read_urb, GFP_ATOMIC);
 }
 
 static int iuu_status(struct usb_serial_port *port)

@@ -351,7 +349,6 @@ static void iuu_rgbf_fill_buffer(u8 *buf, u8 r1, u8 r2, u8 g1, u8 g2, u8 b1,
 static void iuu_led_activity_on(struct urb *urb)
 {
 	struct usb_serial_port *port = urb->context;
-	int result;
 	char *buf_ptr = port->write_urb->transfer_buffer;
 	*buf_ptr++ = IUU_SET_LED;
 	if (xmas) {

@@ -366,13 +363,12 @@ static void iuu_led_activity_on(struct urb *urb)
 					  port->bulk_out_endpointAddress),
 			  port->write_urb->transfer_buffer, 8 ,
 			  iuu_rxcmd, port);
-	result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
+	usb_submit_urb(port->write_urb, GFP_ATOMIC);
 }
 
 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) {
 		iuu_rxcmd(urb);

@@ -386,7 +382,7 @@ static void iuu_led_activity_off(struct urb *urb)
 					  port->bulk_out_endpointAddress),
 			  port->write_urb->transfer_buffer, 8 ,
 			  iuu_rxcmd, port);
-	result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
+	usb_submit_urb(port->write_urb, GFP_ATOMIC);
 }

Signature · 1 line

2.26.2

Johan Hovold

author maintainer

2020-07-01 16:54 UTC #3 lore raw

PATCH 3/5 USB: serial: keyspan_pda: drop unused firmware reset status

In reply to Johan Hovold

Drop the unused firmware reset status which would already have been logged.

This suppresses the corresponding W=1 (-Wunused-but-set-variable) warning.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/keyspan_pda.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/usb/serial/keyspan_pda.c +1 -2

index bf988f77d400..c1333919716b 100644
@@ -664,11 +664,10 @@ static void keyspan_pda_close(struct usb_serial_port *port)
 /* download the firmware to a "fake" device (pre-renumeration) */
 static int keyspan_pda_fake_startup(struct usb_serial *serial)
 {
-	int response;
 	const char *fw_name;
 
 	/* download the firmware here ... */
-	response = ezusb_fx1_set_reset(serial->dev, 1);
+	ezusb_fx1_set_reset(serial->dev, 1);
 
 	if (0) { ; }
 #ifdef KEYSPAN

Signature · 1 line

2.26.2

Johan Hovold

author maintainer

2020-07-01 16:54 UTC #4 lore raw

PATCH 4/5 USB: serial: kobil_sct: log failure to update line settings

In reply to Johan Hovold

Log failure to update the line settings in set_termios().

This also avoids a W=1 (-Wunused-but-set-variable) warning.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/kobil_sct.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/usb/serial/kobil_sct.c +4 -0

index e9882ba20933..79ce0219fdde 100644
@@ -526,6 +526,10 @@ static void kobil_set_termios(struct tty_struct *tty,
 		  0,
 		  KOBIL_TIMEOUT
 		);
+	if (result) {
+		dev_err(&port->dev, "failed to update line settings: %d\n",
+				result);
+	}
 }
 
 static int kobil_ioctl(struct tty_struct *tty,

Signature · 1 line

2.26.2

Johan Hovold

author maintainer

2020-07-01 16:54 UTC #5 lore raw

PATCH 5/5 USB: serial: quatech2: drop two stub functions

In reply to Johan Hovold

Drop two unused stub functions which only served as documentation.

This also avoids a W=1 (-Wunused-but-set-variable) warning.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/quatech2.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

drivers/usb/serial/quatech2.c +1 ~~-17~~

index f93b81a297d6..872d1bc86ab4 100644
@@ -480,21 +480,6 @@ static void qt2_process_status(struct usb_serial_port *port, unsigned char *ch)
 	}
 }
 
-/* not needed, kept to document functionality */
-static void qt2_process_xmit_empty(struct usb_serial_port *port,
-				   unsigned char *ch)
-{
-	int bytes_written;
-
-	bytes_written = (int)(*ch) + (int)(*(ch + 1) << 4);
-}
-
-/* not needed, kept to document functionality */
-static void qt2_process_flush(struct usb_serial_port *port, unsigned char *ch)
-{
-	return;
-}
-
 static void qt2_process_read_urb(struct urb *urb)
 {
 	struct usb_serial *serial;

@@ -540,7 +525,7 @@ static void qt2_process_read_urb(struct urb *urb)
 						 __func__);
 					break;
 				}
-				qt2_process_xmit_empty(port, ch + 3);
+				/* bytes_written = (ch[1] << 4) + ch[0]; */
 				i += 4;
 				escapeflag = true;
 				break;

@@ -569,7 +554,6 @@ static void qt2_process_read_urb(struct urb *urb)
 				break;
 			case QT2_REC_FLUSH:
 			case QT2_XMIT_FLUSH:
-				qt2_process_flush(port, ch + 2);
 				i += 2;
 				escapeflag = true;
 				break;

Signature · 1 line

2.26.2

Greg KH

Reviewed-by Greg Kroah-Hartman on the v1 series

2020-07-01 17:23 UTC #6 lore raw

In reply to Johan Hovold

Quoted 7 lines · Johan Hovold

People have started submitting patches to "fix" W=1 warning (some even reported by bots). Let's just address the remaining ones in USB serial to avoid having to spend more time on this.
Johan

Quoted 6 lines

Johan Hovold (5):
USB: serial: garmin_gps: don't compile unused packet definitions
USB: serial: iuu_phoenix: drop unused URB submission results
USB: serial: keyspan_pda: drop unused firmware reset status
USB: serial: kobil_sct: log failure to update line settings
USB: serial: quatech2: drop two stub functions

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