PERFORCE change 166156 for review

[Hans Petter Selasky]

http://perforce.freebsd.org/chv.cgi?CH=166156

Change 166156 by hselasky at hselasky_laptop001 on 2009/07/16 08:27:42

	
	USB core:
	 - improve handling of alternate settings in device side mode.

Affected files ...

.. //depot/projects/usb/src/sys/dev/usb/usb_device.c#43 edit
.. //depot/projects/usb/src/sys/dev/usb/usb_handle_request.c#17 edit
.. //depot/projects/usb/src/sys/dev/usb/usb_parse.c#7 edit

Differences ...

==== //depot/projects/usb/src/sys/dev/usb/usb_device.c#43 (text+ko) ====

@@ -833,18 +833,13 @@
 		err = USB_ERR_INVAL;
 		goto done;
 	}
-	if (udev->flags.usb_mode == USB_MODE_DEVICE) {
-		usb_detach_device(udev, iface_index,
-		    USB_UNCFG_FLAG_FREE_SUBDEV);
-	} else {
-		if (iface->alt_index == alt_index) {
-			/* 
-			 * Optimise away duplicate setting of
-			 * alternate setting in USB Host Mode!
-			 */
-			err = 0;
-			goto done;
-		}
+	if (iface->alt_index == alt_index) {
+		/* 
+		 * Optimise away duplicate setting of
+		 * alternate setting in USB Host Mode!
+		 */
+		err = 0;
+		goto done;
 	}
 #if USB_HAVE_UGEN
 	/*
@@ -858,6 +853,12 @@
 	if (err) {
 		goto done;
 	}
+	if (iface->alt_index != alt_index) {
+		/* the alternate setting does not exist */
+		err = USB_ERR_INVAL;
+		goto done;
+	}
+
 	err = usbd_req_set_alt_interface_no(udev, NULL, iface_index,
 	    iface->idesc->bAlternateSetting);
 
@@ -959,7 +960,6 @@
 {
 	struct usb_endpoint *ep;
 	struct usb_endpoint *ep_end;
-	usb_error_t err;
 
 	ep = udev->endpoints;
 	ep_end = udev->endpoints + udev->endpoints_max;
@@ -971,10 +971,7 @@
 			continue;
 		}
 		/* simulate a clear stall from the peer */
-		err = usbd_set_endpoint_stall(udev, ep, 0);
-		if (err) {
-			/* just ignore */
-		}
+		usbd_set_endpoint_stall(udev, ep, 0);
 	}
 	return (0);
 }

==== //depot/projects/usb/src/sys/dev/usb/usb_handle_request.c#17 (text+ko) ====

@@ -46,6 +46,7 @@
 
 #include <dev/usb/usb.h>
 #include <dev/usb/usbdi.h>
+#include <dev/usb/usbdi_util.h>
 #include "usb_if.h"
 
 #define	USB_DEBUG_VAR usb_debug
@@ -181,6 +182,30 @@
 	return (err);
 }
 
+static usb_error_t
+usb_check_alt_setting(struct usb_device *udev, 
+     struct usb_interface *iface, uint8_t alt_index)
+{
+	uint8_t do_unlock;
+	usb_error_t err = 0;
+
+	/* automatic locking */
+	if (sx_xlocked(udev->default_sx + 1)) {
+		do_unlock = 0;
+	} else {
+		do_unlock = 1;
+		sx_xlock(udev->default_sx + 1);
+	}
+
+	if (alt_index >= usbd_get_no_alts(udev->cdesc, iface->idesc))
+		err = USB_ERR_INVAL;
+
+	if (do_unlock) {
+		sx_unlock(udev->default_sx + 1);
+	}
+	return (err);
+}
+
 /*------------------------------------------------------------------------*
  *	usb_handle_iface_request
  *
@@ -285,42 +310,29 @@
 		switch (req.bRequest) {
 		case UR_SET_INTERFACE:
 			/*
-			 * Handle special case. If we have parent interface
-			 * we just reset the endpoints, because this is a
-			 * multi interface device and re-attaching only a
-			 * part of the device is not possible. Also if the
-			 * alternate setting is the same like before we just
-			 * reset the interface endoints.
+			 * We assume that the endpoints are the same
+			 * accross the alternate settings.
+			 *
+			 * Reset the endpoints, because re-attaching
+			 * only a part of the device is not possible.
 			 */
-			if ((iface_parent != NULL) ||
-			    (iface->alt_index == req.wValue[0])) {
-				error = usb_reset_iface_endpoints(udev,
-				    iface_index);
-				if (error) {
-					DPRINTF("alt setting failed %s\n",
-					    usbd_errstr(error));
-					goto tr_stalled;
-				}
-				break;
-			}
-			/* 
-			 * Doing the alternate setting will detach the
-			 * interface aswell:
-			 */
-			error = usbd_set_alt_interface_index(udev,
-			    iface_index, req.wValue[0]);
+			error = usb_check_alt_setting(udev,
+			    iface, req.wValue[0]);
 			if (error) {
-				DPRINTF("alt setting failed %s\n",
+				DPRINTF("alt setting does not exist %s\n",
 				    usbd_errstr(error));
 				goto tr_stalled;
 			}
-			error = usb_probe_and_attach(udev,
-			    iface_index);
+			error = usb_reset_iface_endpoints(udev, iface_index);
 			if (error) {
-				DPRINTF("alt setting probe failed\n");
+				DPRINTF("alt setting failed %s\n",
+				    usbd_errstr(error));
 				goto tr_stalled;
 			}
+			/* update the current alternate setting */
+			iface->alt_index = req.wValue[0];
 			break;
+
 		default:
 			goto tr_stalled;
 		}

==== //depot/projects/usb/src/sys/dev/usb/usb_parse.c#7 (text+ko) ====

@@ -222,11 +222,22 @@
     struct usb_interface_descriptor *id)
 {
 	struct usb_descriptor *desc;
-	uint8_t n = 0;
+	uint8_t n;
 	uint8_t ifaceno;
 
+	/* 
+	 * We always have at least one interface setting, due to the
+	 * fact that the usb_desc_foreach() call below will skip the
+	 * first interface descriptor.
+	 */
+	n = 1;
+
+	/* Get the interface number */
+
 	ifaceno = id->bInterfaceNumber;
 
+	/* Get the interface descriptor */
+
 	desc = (struct usb_descriptor *)id;
 
 	while ((desc = usb_desc_foreach(cd, desc))) {