RFC/patch 5/6: usb: split usb_new_device for clarity and refactoring

# HG changeset patch
# User Inaky Perez-Gonzalez <[email protected]>
# Date 1177458827 25200
# Node ID 79751b058ef127578c4830644d575d8f474d2f43
# Parent  a1f71724f48c863b6b84d2bc56d9035edc8284bc
usb: split usb_new_device for clarity and refactoring

This patch takes hub.c:usb_new_device() and splits it in three parts:

- The actual actions of adding a new device (quirk detection,
  announcement and autoresume tracking)

- Actual discovery and probing of the configuration and interfaces
  (split into __usb_configure_device())

- Configuration of the On-the-go parameters (split into
  __usb_configure_device_otg()).

The fundamental reasons for doing this split are clarity (smaller
functions are easier to maintain) and to allow part of the code to be
reused when authorizing devices to connect.

When a device is authorized connection, we need to run through the
hoops we didn't run when it was connected but not authorized, which is
basically parsing the configurations and probing
them. __usb_configure_device() will do that for us.

Signed-off-by: Inaky Perez-Gonzalez <[email protected]>

diff -r a1f71724f48c -r 79751b058ef1 drivers/usb/core/hub.c
--- a/drivers/usb/core/hub.c	Tue Apr 24 15:09:11 2007 -0700
+++ b/drivers/usb/core/hub.c	Tue Apr 24 16:53:47 2007 -0700
@@ -1257,65 +1257,17 @@ static int __usb_port_suspend(struct usb
 static int __usb_port_suspend(struct usb_device *, int port1);
 #endif
 
+
 /**
- * usb_new_device - perform initial device setup (usbcore-internal)
+ * __usb_configure_device_otg - FIXME (usbcore-internal)
  * @udev: newly addressed device (in ADDRESS state)
  *
- * This is called with devices which have been enumerated, but not yet
- * configured.  The device descriptor is available, but not descriptors
- * for any device configuration.  The caller must have locked either
- * the parent hub (if udev is a normal device) or else the
- * usb_bus_list_lock (if udev is a root hub).  The parent's pointer to
- * udev has already been installed, but udev is not yet visible through
- * sysfs or other filesystem code.
- *
- * It will return if the device is configured properly or not.  Zero if
- * the interface was registered with the driver core; else a negative
- * errno value.
- *
- * This call is synchronous, and may not be used in an interrupt context.
- *
- * Only the hub driver or root-hub registrar should ever call this.
- *
- * If the device is WUSB, we don't attempt to read the string
- * descriptors, as they will be errored out by the device. 
+ * Do configuration for On-The-Go devices
  */
-int usb_new_device(struct usb_device *udev)
-{
-	int err;
-
-	/* Determine quirks */
-	usb_detect_quirks(udev);
-
-	err = usb_get_configuration(udev);
-	if (err < 0) {
-		dev_err(&udev->dev, "can't read configurations, error %d\n",
-			err);
-		goto fail;
-	}
-
-	if (udev->wusb == 1 && udev->authorized == 0) {
-		udev->product = kstrdup("n/a (unauthorized)", GFP_KERNEL);
-		udev->manufacturer = kstrdup("n/a (unauthorized)", GFP_KERNEL);
-		udev->serial = kstrdup("n/a (unauthorized)", GFP_KERNEL);
-	}
-	else {
-		/* read the standard strings and cache them if present */
-		udev->product = usb_cache_string(udev, udev->descriptor.iProduct);
-		udev->manufacturer = usb_cache_string(udev,
-						      udev->descriptor.iManufacturer);
-		udev->serial = usb_cache_string(udev, udev->descriptor.iSerialNumber);		
-	}
-	/* Tell the world! */
-	dev_dbg(&udev->dev, "new device strings: Mfr=%d, Product=%d, "
-		"SerialNumber=%d\n",
-		udev->descriptor.iManufacturer,
-		udev->descriptor.iProduct,
-		udev->descriptor.iSerialNumber);
-	show_string(udev, "Product", udev->product);
-	show_string(udev, "Manufacturer", udev->manufacturer);
-	show_string(udev, "SerialNumber", udev->serial);
-
+static
+int __usb_configure_device_otg(struct usb_device *udev)
+{
+	int err = 0;
 #ifdef	CONFIG_USB_OTG
 	/*
 	 * OTG-aware devices on OTG-capable root hubs may be able to use SRP,
@@ -1376,28 +1328,109 @@ int usb_new_device(struct usb_device *ud
 		err = -ENODEV;
 		goto fail;
 	}
+fail:
 #endif
+	return err;
+}
+
+
+/**
+ * __usb_configure_device - Detect and probe device intfs/otg 
(usbcore-internal)
+ * @udev: newly addressed device (in ADDRESS state)
+ *
+ * This is only called by usb_new_device() and FIXME -- all comments
+ * that apply to them apply here wrt to environment. 
+ *
+ * If the device is WUSB and not authorized, we don't attempt to read
+ * the string descriptors, as they will be errored out by the device
+ * until it has been authorized.
+ */
+static
+int __usb_configure_device(struct usb_device *udev)
+{
+	int err;
+
+	err = usb_get_configuration(udev);
+	if (err < 0) {
+		dev_err(&udev->dev, "can't read configurations, error %d\n",
+			err);
+		goto fail;
+	}
+
+	if (udev->wusb == 1 && udev->authorized == 0) {
+		udev->product = kstrdup("n/a (unauthorized)", GFP_KERNEL);
+		udev->manufacturer = kstrdup("n/a (unauthorized)", GFP_KERNEL);
+		udev->serial = kstrdup("n/a (unauthorized)", GFP_KERNEL);
+	}
+	else {
+		/* read the standard strings and cache them if present */
+		udev->product = usb_cache_string(udev, udev->descriptor.iProduct);
+		udev->manufacturer = usb_cache_string(udev,
+						      udev->descriptor.iManufacturer);
+		udev->serial = usb_cache_string(udev, udev->descriptor.iSerialNumber);		
+	}
+	
+	err = __usb_configure_device_otg(udev);
+	if (err < 0)
+		goto fail;
 
 	/* Register the device.  The device driver is responsible
 	 * for adding the device files to usbfs and sysfs and for
 	 * configuring the device.
 	 */
 	err = device_add (&udev->dev);
-	if (err) {
+	if (err)
 		dev_err(&udev->dev, "can't device_add, error %d\n", err);
+fail:
+	return err;
+}
+
+
+/**
+ * usb_new_device - perform initial device setup (usbcore-internal)
+ * @udev: newly addressed device (in ADDRESS state)
+ *
+ * This is called with devices which have been enumerated, but not yet
+ * configured.  The device descriptor is available, but not descriptors
+ * for any device configuration.  The caller must have locked either
+ * the parent hub (if udev is a normal device) or else the
+ * usb_bus_list_lock (if udev is a root hub).  The parent's pointer to
+ * udev has already been installed, but udev is not yet visible through
+ * sysfs or other filesystem code.
+ *
+ * It will return if the device is configured properly or not.  Zero if
+ * the interface was registered with the driver core; else a negative
+ * errno value.
+ *
+ * This call is synchronous, and may not be used in an interrupt context.
+ *
+ * Only the hub driver or root-hub registrar should ever call this.
+ */
+int usb_new_device(struct usb_device *udev)
+{
+	int err;
+
+	usb_detect_quirks(udev);		/* Determine quirks */
+	err = __usb_configure_device(udev);	/* detect & probe dev/intfs */
+	if (err < 0)
 		goto fail;
-	}
-
+	/* Tell the world! */
+	dev_dbg(&udev->dev, "new device strings: Mfr=%d, Product=%d, "
+		"SerialNumber=%d\n",
+		udev->descriptor.iManufacturer,
+		udev->descriptor.iProduct,
+		udev->descriptor.iSerialNumber);
+	show_string(udev, "Product", udev->product);
+	show_string(udev, "Manufacturer", udev->manufacturer);
+	show_string(udev, "SerialNumber", udev->serial);
 	/* Increment the parent's count of unsuspended children */
 	if (udev->parent)
 		usb_autoresume_device(udev->parent);
-
-exit:
 	return err;
 
 fail:
 	usb_set_device_state(udev, USB_STATE_NOTATTACHED);
-	goto exit;
+	return err;
 }
 
 static int hub_port_status(struct usb_hub *hub, int port1,

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel