[OLPC-devel] [PATCH] wireless/libertas: get rid of static global vars in if_usb.c

Dan Williams dcbw at redhat.com
Mon Jun 26 11:16:26 EDT 2006


- Get rid of problematic static global variables usb_cardp and pwlanpriv
- Protect usb suspend/resume handlers with CONFIG_PM like zd1201
- Move card reset on unregister to sbi_unregister_dev() to fix null
pointer dereference bug during rmmod if the card had already been
hot-unplugged

Signed-off-by: Dan Williams <dcbw at redhat.com>

--- a/drivers/net/wireless/libertas/if_usb.c	2006-06-25 19:26:45.000000000 -0400
+++ b/drivers/net/wireless/libertas/if_usb.c	2006-06-26 10:55:06.000000000 -0400
@@ -50,8 +50,8 @@ Change log:
 
 #define MESSAGE_HEADER_LEN	4
 
-static wlan_private *pwlanpriv;
-static struct usb_card_rec usb_cardp;
+static usb_notifier_fn_add wlan_card_add_cb = NULL;
+static usb_notifier_fn_remove wlan_card_remove_cb = NULL;
 const char usbdriver_name[] = "usb8xxx";
 static u8 usb_int_cause = 0;
 
@@ -75,8 +75,14 @@ static void if_usb_receive_fwload(struct
 static int if_usb_probe(struct usb_interface *intf,
 			const struct usb_device_id *id);
 static void if_usb_disconnect(struct usb_interface *intf);
+
+#ifdef CONFIG_PM
 static int if_usb_suspend(struct usb_interface *intf, pm_message_t message);
 static int if_usb_resume(struct usb_interface *intf);
+#else
+#define if_usb_suspend NULL
+#define if_usb_resume NULL
+#endif
 
 static int __if_usb_submit_rx_urb(wlan_private * priv,
 				  void (*callbackfn)
@@ -197,7 +203,6 @@ static void if_usb_free(struct usb_card_
 
 /** 
  *  @brief sets the configuration values
- *  @param udev		pointer to usb_device
  *  @param ifnum	interface number
  *  @param id		pointer to usb_device_id
  *  @return 	   	address of gloabl variable usb_cardp 
@@ -208,6 +213,8 @@ static int if_usb_probe(struct usb_inter
 	struct usb_device *udev;
 	struct usb_host_interface *iface_desc;
 	struct usb_endpoint_descriptor *endpoint;
+	wlan_private *pwlanpriv;
+	struct usb_card_rec *usb_cardp;
 	int i;
 
 	ENTER();
@@ -225,102 +232,116 @@ static int if_usb_probe(struct usb_inter
 		}
 	}
 
-	if (if_usb_table[i].idVendor) {
+	if (!if_usb_table[i].idVendor) {
+		PRINTM(INFO, "Discard the Probe request\n");
+		PRINTM(INFO, "VID = 0x%X PID = 0x%X\n",
+		       udev->descriptor.idVendor, udev->descriptor.idProduct);
+		LEAVE();
+		return -ENODEV;
+	}
 
-		usb_cardp.udev = udev;
-		iface_desc = intf->cur_altsetting;
+	usb_cardp = kzalloc(sizeof(struct usb_card_rec), GFP_KERNEL);
+	if (!usb_cardp) {
+		PRINTM(FATAL, "Out of memory allocating private data.\n");
+		goto error;
+	}
+
+	usb_cardp->add = wlan_card_add_cb;
+	usb_cardp->remove = wlan_card_remove_cb;
+	usb_cardp->udev = udev;
+	iface_desc = intf->cur_altsetting;
+
+	PRINTM(INFO, "bcdUSB = 0x%X bDeviceClass = 0x%X"
+	       " bDeviceSubClass = 0x%X, bDeviceProtocol = 0x%X\n",
+	       udev->descriptor.bcdUSB,
+	       udev->descriptor.bDeviceClass,
+	       udev->descriptor.bDeviceSubClass,
+	       udev->descriptor.bDeviceProtocol);
+
+	for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
+		endpoint = &iface_desc->endpoint[i].desc;
+		if ((endpoint->bEndpointAddress & BULK_ENDPOINT_PRESENT)
+		    && ((endpoint->bmAttributes & BULK_ENDPOINT_MASK) ==
+			BULK_ENDPOINT_FOUND)) {
+			/* we found a bulk in endpoint */
+			PRINTM(INFO, "Bulk in size is %d\n",
+			       endpoint->wMaxPacketSize);
+			if (!
+			    (usb_cardp->rx_urb =
+			     usb_alloc_urb(0, GFP_KERNEL))) {
+				PRINTM(INFO,
+				       "Rx URB allocation failed\n");
+				goto dealloc;
+			}
+			usb_cardp->rx_urb_recall = FALSE;
+
+			usb_cardp->bulk_in_size =
+			    endpoint->wMaxPacketSize;
+			usb_cardp->bulk_in_endpointAddr =
+			    (endpoint->
+			     bEndpointAddress & ENDPOINT_NUMBER_MASK);
+			PRINTM(INFO, "in_endpoint = %d\n",
+			       endpoint->bEndpointAddress);
+		}
 
-		PRINTM(INFO, "bcdUSB = 0x%X bDeviceClass = 0x%X"
-		       " bDeviceSubClass = 0x%X, bDeviceProtocol = 0x%X\n",
-		       udev->descriptor.bcdUSB,
-		       udev->descriptor.bDeviceClass,
-		       udev->descriptor.bDeviceSubClass,
-		       udev->descriptor.bDeviceProtocol);
-
-		for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
-			endpoint = &iface_desc->endpoint[i].desc;
-			if ((endpoint->bEndpointAddress & BULK_ENDPOINT_PRESENT)
-			    && ((endpoint->bmAttributes & BULK_ENDPOINT_MASK) ==
-				BULK_ENDPOINT_FOUND)) {
-				/* we found a bulk in endpoint */
-				PRINTM(INFO, "Bulk in size is %d\n",
-				       endpoint->wMaxPacketSize);
-				if (!
-				    (usb_cardp.rx_urb =
-				     usb_alloc_urb(0, GFP_KERNEL))) {
-					PRINTM(INFO,
-					       "Rx URB allocation failed\n");
-					goto error;
-				}
-				usb_cardp.rx_urb_recall = FALSE;
-
-				usb_cardp.bulk_in_size =
-				    endpoint->wMaxPacketSize;
-				usb_cardp.bulk_in_endpointAddr =
-				    (endpoint->
-				     bEndpointAddress & ENDPOINT_NUMBER_MASK);
-				PRINTM(INFO, "in_endpoint = %d\n",
-				       endpoint->bEndpointAddress);
+		if (((endpoint->
+		      bEndpointAddress & BULK_ENDPOINT_PRESENT) ==
+		     BULK_OUT_PRESENT)
+		    && ((endpoint->bmAttributes & BULK_ENDPOINT_MASK) ==
+			BULK_ENDPOINT_FOUND)) {
+			/* We found bulk out endpoint */
+			if (!
+			    (usb_cardp->tx_urb =
+			     usb_alloc_urb(0, GFP_KERNEL))) {
+				PRINTM(INFO,
+				       "Tx URB allocation failed\n");
+				goto dealloc;
 			}
+			usb_cardp->tx_urb_pending = FALSE;
 
-			if (((endpoint->
-			      bEndpointAddress & BULK_ENDPOINT_PRESENT) ==
-			     BULK_OUT_PRESENT)
-			    && ((endpoint->bmAttributes & BULK_ENDPOINT_MASK) ==
-				BULK_ENDPOINT_FOUND)) {
-				/* We found bulk out endpoint */
-				if (!
-				    (usb_cardp.tx_urb =
-				     usb_alloc_urb(0, GFP_KERNEL))) {
-					PRINTM(INFO,
-					       "Tx URB allocation failed\n");
-					goto error;
-				}
-				usb_cardp.tx_urb_pending = FALSE;
-
-				usb_cardp.bulk_out_size =
-				    endpoint->wMaxPacketSize;
-				PRINTM(INFO, "Bulk out size is %d\n",
-				       endpoint->wMaxPacketSize);
-				usb_cardp.bulk_out_endpointAddr =
-				    endpoint->bEndpointAddress;
-				PRINTM(INFO, "out_endpoint = %d\n",
-				       endpoint->bEndpointAddress);
-				usb_cardp.bulk_out_buffer =
-				    kmalloc(MRVDRV_ETH_TX_PACKET_BUFFER_SIZE,
-					    GFP_DMA);
-
-				if (!usb_cardp.bulk_out_buffer) {
-					PRINTM(INFO,
-					       "Could not allocate buffer\n");
-					goto error;
-				}
+			usb_cardp->bulk_out_size =
+			    endpoint->wMaxPacketSize;
+			PRINTM(INFO, "Bulk out size is %d\n",
+			       endpoint->wMaxPacketSize);
+			usb_cardp->bulk_out_endpointAddr =
+			    endpoint->bEndpointAddress;
+			PRINTM(INFO, "out_endpoint = %d\n",
+			       endpoint->bEndpointAddress);
+			usb_cardp->bulk_out_buffer =
+			    kmalloc(MRVDRV_ETH_TX_PACKET_BUFFER_SIZE,
+				    GFP_DMA);
+
+			if (!usb_cardp->bulk_out_buffer) {
+				PRINTM(INFO,
+				       "Could not allocate buffer\n");
+				goto dealloc;
 			}
 		}
+	}
 
-		/* At this point wlan_add_card() will be called */
-		if (!(pwlanpriv = usb_cardp.add(&usb_cardp))) {
-			goto error;
-		}
+	/* At this point wlan_add_card() will be called.  Don't worry
+	 * about keeping pwlanpriv around since it will be set on our
+	 * usb device data in -> add() -> sbi_register_dev().
+	 */
+	if (!(pwlanpriv = usb_cardp->add(usb_cardp))) {
+		goto dealloc;
+	}
 
-		usb_get_dev(udev);
-		usb_set_intfdata(intf, &usb_cardp);
-		LEAVE();
+	usb_get_dev(udev);
+	usb_set_intfdata(intf, usb_cardp);
+	LEAVE();
 
-		/* 
-		 * return card structure, which can be got back in the 
-		 * diconnect function as the ptr
-		 * argument.
-		 */
-		return 0;
-	} else {
-		PRINTM(INFO, "Discard the Probe request\n");
-		PRINTM(INFO, "VID = 0x%X PID = 0x%X\n",
-		       udev->descriptor.idVendor, udev->descriptor.idProduct);
-	}
-error:
-	if_usb_free(&usb_cardp);
+	/* 
+	 * return card structure, which can be got back in the 
+	 * diconnect function as the ptr
+	 * argument.
+	 */
+	return 0;
+
+dealloc:
+	if_usb_free(usb_cardp);
 
+error:
 	LEAVE();
 	return (-ENOMEM);
 }
@@ -328,7 +349,6 @@ error:
 /** 
  *  @brief free resource and cleanup
  *  @param udev		pointer to usb_device
- *  @param ptr		pointer to usb_cardp
  *  @return 	   	N/A
  */
 static void if_usb_disconnect(struct usb_interface *intf)
@@ -792,7 +812,19 @@ int sbi_get_cis_info(wlan_private * priv
  */
 int sbi_unregister_dev(wlan_private * priv)
 {
-	return WLAN_STATUS_SUCCESS;
+	int ret = WLAN_STATUS_SUCCESS;
+
+	// Need to send a Reset command to device before USB resources freed and wlan_remove_card() called,
+	// then device can handle FW download again.
+	if (priv) {
+		if (PrepareAndSendCommand(priv, HostCmd_CMD_802_11_RESET,
+					  HostCmd_ACT_HALT, 0, 0, NULL) !=
+				WLAN_STATUS_SUCCESS)
+			ret = WLAN_STATUS_FAILURE;
+		msleep_interruptible(10);		
+	}
+
+	return ret;
 }
 
 /**  
@@ -800,22 +832,15 @@ int sbi_unregister_dev(wlan_private * pr
  *  @param add		pointer to add_card callback function
  *  @param remove	pointer to remove card callback function
  *  @param arg		pointer to call back function parameter
- *  @return 	   	pointer to usb_cardp
+ *  @return 	   	dummy success variable
  */
 int *sbi_register(wlan_notifier_fn_add add, wlan_notifier_fn_remove remove,
 		  void *arg)
 {
 	ENTER();
 
-	/* 
-	 * Make use of the global variable cardp,
-	 * to assign the call backs
-	 */
-
-	PRINTM(INFO, "udev pointer is at %p\n", usb_cardp.udev);
-
-	usb_cardp.add = (usb_notifier_fn_add) add;
-	usb_cardp.remove = (usb_notifier_fn_remove) remove;
+	wlan_card_add_cb = (usb_notifier_fn_add) add;
+	wlan_card_remove_cb = (usb_notifier_fn_remove) remove;
 
 	/* 
 	 * API registers the Marvell USB driver
@@ -825,8 +850,8 @@ int *sbi_register(wlan_notifier_fn_add a
 
 	LEAVE();
 
-	/* Return the cardp structure to wlan layer */
-	return (int *)&usb_cardp;
+	/* Return success to wlan layer */
+	return (int *)0x1;
 }
 
 /**  
@@ -837,13 +862,8 @@ void sbi_unregister(void)
 {
 	ENTER();
 
-	// Need to send a Reset command to device before USB resources freed and wlan_remove_card() called,
-	// then device can handle FW download again.
-	if (pwlanpriv) {
-		PrepareAndSendCommand(pwlanpriv, HostCmd_CMD_802_11_RESET,
-				      HostCmd_ACT_HALT, 0, 0, NULL);
-		msleep_interruptible(10);
-	}
+	wlan_card_add_cb = NULL;
+	wlan_card_remove_cb = NULL;
 
 	/* API unregisters thedriver from USB subsystem */
 	usb_deregister(&if_usb_driver);
@@ -1062,6 +1082,7 @@ inline int sbi_resume(wlan_private * pri
 }
 #endif
 
+#ifdef CONFIG_PM
 static int if_usb_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct usb_card_rec *cardp = usb_get_intfdata(intf);
@@ -1099,3 +1120,4 @@ static int if_usb_resume(struct usb_inte
 	LEAVE();
 	return 0;
 }
+#endif





More information about the Devel mailing list