Skip to contentSkip to conversation

USB: xhci: unsigned char never equals -1

PATCH In review Dan Carpenter 2 participants 4 messages Mar 17, 2011

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 USB: xhci: unsigned char never equals -1

There were some places that compared port_speed = -1 where port_speed is a u8. This doesn't work unless we cast the -1 to u8. Some places did it correctly.

Instead of using -1 directly, I've created a DUPLICATE_ENTRY define which does the cast and is more descriptive as well.

---
I'm not super familiar with this code... Compile tested only.

drivers/usb/host/xhci-mem.c +4 -4
index a003e79..ab7fc2b 100644
@@ -846,7 +846,7 @@ static u32 xhci_find_real_port_number(struct xhci_hcd *xhci,
 		 * Skip ports that don't have known speeds, or have duplicate
 		 * Extended Capabilities port speed entries.
 		 */
-		if (port_speed = 0 || port_speed = -1)
+		if (port_speed = 0 || port_speed = DUPLICATE_ENTRY)
 			continue;
 
 		/*
@@ -1727,12 +1727,12 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
 			 * found a similar duplicate.
 			 */
 			if (xhci->port_array[i] != major_revision &&
-				xhci->port_array[i] != (u8) -1) {
+				xhci->port_array[i] != DUPLICATE_ENTRY) {
 				if (xhci->port_array[i] = 0x03)
 					xhci->num_usb3_ports--;
 				else
 					xhci->num_usb2_ports--;
-				xhci->port_array[i] = (u8) -1;
+				xhci->port_array[i] = DUPLICATE_ENTRY;
 			}
 			/* FIXME: Should we disable the port? */
 			continue;
@@ -1831,7 +1831,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
 		for (i = 0; i < num_ports; i++) {
 			if (xhci->port_array[i] = 0x03 ||
 					xhci->port_array[i] = 0 ||
-					xhci->port_array[i] = -1)
+					xhci->port_array[i] = DUPLICATE_ENTRY)
 				continue;
 
 			xhci->usb2_ports[port_index] diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c

index cfc1ad9..c6d1462 100644

drivers/usb/host/xhci-ring.c +2 -2
@@ -1209,7 +1209,7 @@ static unsigned int find_faked_portnum_from_hw_portnum(struct usb_hcd *hcd,
 		 * Skip ports that don't have known speeds, or have duplicate
 		 * Extended Capabilities port speed entries.
 		 */
-		if (port_speed = 0 || port_speed = -1)
+		if (port_speed = 0 || port_speed = DUPLICATE_ENTRY)
 			continue;
 
 		/*
@@ -1260,7 +1260,7 @@ static void handle_port_status(struct xhci_hcd *xhci,
 				port_id);
 		goto cleanup;
 	}
-	if (major_revision = (u8) -1) {
+	if (major_revision = DUPLICATE_ENTRY) {
 		xhci_warn(xhci, "Event for port %u duplicated in"
 				"Extended Capabilities, ignoring.\n",
 				port_id);
drivers/usb/host/xhci.h +3 -0
index 711de25..45bd5a2 100644
@@ -348,6 +348,9 @@ struct xhci_op_regs {
 /* Initiate a warm port reset - complete when PORT_WRC is '1' */
 #define PORT_WR		(1 << 31)
 
+/* We mark duplicate entries with -1 */
+#define DUPLICATE_ENTRY ((u8)(-1))
+
 /* Port Power Management Status and Control - port_power_base bitmasks */
 /* Inactivity timer value for transitions into U1, in microseconds.
  * Timeout can be up to 127us.  0xFF means an infinite timeout.
#1 lore raw

Btw, xhci-ring.c causes some valid looking gcc warnings:

C [M] drivers/usb/host/xhci-ring.o
drivers/usb/host/xhci-ring.c: In function ‘handle_port_status’:
drivers/usb/host/xhci-ring.c:1229: warning: ‘hcd’ may be used uninitialized in this function drivers/usb/host/xhci-ring.c: In function ‘handle_tx_event’:
drivers/usb/host/xhci-ring.c:1895: warning: ‘event_trb’ may be used uninitialized in this function

regards,
dan carpenter

#2 lore raw
In reply to Dan Carpenter
Quoted 1 line · Dan Carpenter

Btw, xhci-ring.c causes some valid looking gcc warnings:

Which git commit are you working off of?

Quoted 5 lines

C [M] drivers/usb/host/xhci-ring.o
drivers/usb/host/xhci-ring.c: In function ‘handle_port_status’:
drivers/usb/host/xhci-ring.c:1229: warning: ‘hcd’ may be used uninitialized in this function drivers/usb/host/xhci-ring.c: In function ‘handle_tx_event’:
drivers/usb/host/xhci-ring.c:1895: warning: ‘event_trb’ may be used uninitialized in this function

I think the second warning is just the compiler getting confused, but I'll look into the first warning. What gcc version are you using? I had compile-tested the patches I recently sent to Greg, but on an older (un-updateable, thanks Ubuntu) box with an older gcc.

Sarah Sharp

#3 lore raw
In reply to Dan Carpenter
Quoted 5 lines · Sarah Sharp
Quoted 1 line · Dan Carpenter

Btw, xhci-ring.c causes some valid looking gcc warnings:

Which git commit are you working off of?

This is linux-next for 20110316.

commit b3694afad1864eaa7b59a1a5bcf576ed00d4089a
Merge: 37f454a 500132a
Author: Stephen Rothwell <[email protected]>
Date: Wed Mar 16 16:53:13 2011 +1100

Merge remote-tracking branch 'usb/usb-next'

Conflicts:
arch/arm/mach-omap2/board-omap3evm.c
arch/arm/mach-omap2/board-overo.c
arch/arm/mach-omap2/clock3xxx_data.c
arch/arm/mach-omap2/usb-musb.c
arch/arm/plat-omap/include/plat/usb.h
drivers/usb/gadget/Kconfig
drivers/usb/gadget/fsl_mxc_udc.c
drivers/usb/musb/musb_core.h

commit ba0a4d9aaae789a6a632968b27c21d49b858b13a

Author: Sarah Sharp <[email protected]>
Date:   Wed Feb 23 18:13:43 2011 -0800

xhci: Clean up cycle bit math used during stalls.

Quoted 11 lines
Quoted 5 lines

C [M] drivers/usb/host/xhci-ring.o
drivers/usb/host/xhci-ring.c: In function ‘handle_port_status’:
drivers/usb/host/xhci-ring.c:1229: warning: ‘hcd’ may be used uninitialized in this function drivers/usb/host/xhci-ring.c: In function ‘handle_tx_event’:
drivers/usb/host/xhci-ring.c:1895: warning: ‘event_trb’ may be used uninitialized in this function

I think the second warning is just the compiler getting confused, but I'll look into the first warning. What gcc version are you using? I had compile-tested the patches I recently sent to Greg, but on an older (un-updateable, thanks Ubuntu) box with an older gcc.

I'm using gcc 4.4.3-4ubuntu5.

regards,
dan carpenter