unnecessary use of set_bit

unnecessary use of set_bit

[Paul Mackerras]

The ohci_hub_status_data() procedure in drivers/usb/host/ohci-hub.c in
2.5.11 is broken in a couple of ways: it uses set_bit on a char *
address and it assumes little-endian byte order in the bitmap.

Here is a patch to fix both problems.  As a bonus, the file ends up
one line shorter. :)

Please, people, don't use set_bit when you don't need an atomic
operation.  set_bit(&x, n) is slower than x |= 1 << n on a lot of
platforms, including SMP x86 I believe.  (If the bitmap is more than
one word you can use __set_bit if you don't require atomicity.)

Paul.

diff -urN linux-2.5/drivers/usb/host/ohci-hub.c pmac-2.5/drivers/usb/host/ohci-hub.c
--- linux-2.5/drivers/usb/host/ohci-hub.c	Sat Apr 27 20:51:31 2002
+++ pmac-2.5/drivers/usb/host/ohci-hub.c	Sat Apr 27 15:03:06 2002
@@ -67,7 +67,7 @@
 ohci_hub_status_data (struct usb_hcd *hcd, char *buf)
 {
 	struct ohci_hcd	*ohci = hcd_to_ohci (hcd);
-	int		ports, i, changed = 0, length = 1;
+	int		ports, i, mask = 0, length = 1;
 
 	ports = roothub_a (ohci) & RH_A_NDP; 
 	if (ports > MAX_ROOT_PORTS) {
@@ -80,13 +80,7 @@
 
 	/* init status */
 	if (roothub_status (ohci) & (RH_HS_LPSC | RH_HS_OCIC))
-		buf [0] = changed = 1;
-	else
-		buf [0] = 0;
-	if (ports > 7) {
-		buf [1] = 0;
-		length++;
-	}
+		mask = 1;
 
 	/* look at each port */
 	for (i = 0; i < ports; i++) {
@@ -94,12 +88,17 @@
 
 		status &= RH_PS_CSC | RH_PS_PESC | RH_PS_PSSC
 				| RH_PS_OCIC | RH_PS_PRSC;
-		if (status) {
-			changed = 1;
-			set_bit (i + 1, buf);
-		}
+		if (status)
+			mask |= 1 << (i + 1);
+	}
+	if (!mask)
+		return 0;
+	buf[0] = mask;
+	if (ports > 7) {
+		length++;
+		buf[1] = mask >> 8;
 	}
-	return changed ? length : 0;
+	return length;
 }
 
 /*-------------------------------------------------------------------------*/

Re: unnecessary use of set_bit

[David Brownell]

> The ohci_hub_status_data() procedure in drivers/usb/host/ohci-hub.c in
> 2.5.11 is broken in a couple of ways: it uses set_bit on a char *
> address and it assumes little-endian byte order in the bitmap.

Greg already submitted my patch to Linus, but I guess it didn't
make it in yet.  You'll be glad to know it stopped using set_bit().

Looks like your patch also preserves the API requirement that
the result be in little-endian byte order.

- Dave