Subject: Re: Belkin Bluetooth vs aue vs ubt
To: Iain Hibbert <plunky@rya-online.net>
From: Matthias Drochner <M.Drochner@fz-juelich.de>
List: tech-kern
Date: 03/12/2007 14:32:00
This is a multipart MIME message.

--==_Exmh_19411231074000
Content-Type: text/plain; charset=us-ascii


plunky@rya-online.net said:
> This means then, that as-is**, it should really match against device
> class (as attached)? 

As said, the bluetooth spec (it was "core_10_b.pdf" what I looked at)
suggests to do so.
Looking at the USB audio class spec I found however that it is
unavoidable to match against one interface and claim others later.
So we'll need some API extension to do that cleanly. Bur ubt should
be converted regardlessly.

Your patch works -- by some luck. The configuration index should
be zero -- it is just the first and only configuration.
(While symbolic constants are good, in these cases this causes
more confusion than readability: Someone looking at the code
later does not know whether the intent was "just use the only
possible configuration" or an explicite device specific selection.)
It seems that that device does not check the index argument of
the "get descriptor" request. If I add a consistency check to
usbd_set_config_index() another problem shows up: The "detach"
function will work on uninitialized data, leading to a crash.
I'll append a modified patch which works for me.

best regards
Matthias



--==_Exmh_19411231074000
Content-Type: text/plain ; name="ubt.diff"; charset=us-ascii
Content-Description: ubt.diff
Content-Disposition: attachment; filename="ubt.diff"

# 
# old_revision [16989f6b7c6b8053ec12557b87bc618f23007dfe]
# 
# patch "sys/dev/usb/ubt.c"
#  from [a3a92287fc64d0915219a19fd45284865e3aa45e]
#    to [cc49acaba3d0bdb12cd9e521d2a030847754e421]
# 
============================================================
--- sys/dev/usb/ubt.c	a3a92287fc64d0915219a19fd45284865e3aa45e
+++ sys/dev/usb/ubt.c	cc49acaba3d0bdb12cd9e521d2a030847754e421
@@ -230,6 +230,9 @@ struct ubt_softc {
 
 	/* Protocol structure */
 	struct hci_unit		 sc_unit;
+
+	/* Successfully attached */
+	int			 sc_ok;
 };
 
 /*******************************************************************************
@@ -292,22 +295,22 @@ USB_MATCH(ubt)
 USB_MATCH(ubt)
 {
 	USB_MATCH_START(ubt, uaa);
-	usb_interface_descriptor_t *id;
+	usb_device_descriptor_t *dd;
 
 	DPRINTFN(50, "ubt_match\n");
 
-	if (uaa->iface == NULL)
+	if (uaa->iface != NULL)
 		return UMATCH_NONE;
 
 	if (usb_lookup(ubt_ignore, uaa->vendor, uaa->product))
 		return UMATCH_NONE;
 
-	id = usbd_get_interface_descriptor(uaa->iface);
-	if (id != NULL
-	    && id->bInterfaceClass == UICLASS_WIRELESS
-	    && id->bInterfaceSubClass == UISUBCLASS_RF
-	    && id->bInterfaceProtocol == UIPROTO_BLUETOOTH)
-		return UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO;
+	dd = usbd_get_device_descriptor(uaa->device);
+	if (dd != NULL
+	    && dd->bDeviceClass == UDCLASS_WIRELESS
+	    && dd->bDeviceSubClass == UDSUBCLASS_RF
+	    && dd->bDeviceProtocol == UDPROTO_BLUETOOTH)
+		return UMATCH_DEVCLASS_DEVSUBCLASS_DEVPROTO;
 
 	return UMATCH_NONE;
 }
@@ -315,7 +318,6 @@ USB_ATTACH(ubt)
 USB_ATTACH(ubt)
 {
 	USB_ATTACH_START(ubt, sc, uaa);
-	usbd_interface_handle iface;
 	usb_config_descriptor_t *cd;
 	usb_endpoint_descriptor_t *ed;
 	const struct sysctlnode *node;
@@ -333,11 +335,12 @@ USB_ATTACH(ubt)
 	usbd_devinfo_free(devinfop);
 
 	/*
-	 * We must have at least 2 interfaces.
+	 * Move the device into the configured state
 	 */
-	if (uaa->nifaces < 2) {
-		aprint_error("%s: need 2 interfaces (got %d)\n",
-			USBDEVNAME(sc->sc_dev), uaa->nifaces);
+	err = usbd_set_config_index(sc->sc_udev, 0, 1);
+	if (err) {
+		aprint_error("%s: failed to set configuration idx 0: %s\n",
+		    USBDEVNAME(sc->sc_dev), usbd_errstr(err));
 
 		USB_ATTACH_ERROR_RETURN;
 	}
@@ -348,7 +351,7 @@ USB_ATTACH(ubt)
 	 *	2) Bulk IN endpoint to receive ACL data
 	 *	3) Bulk OUT endpoint to send ACL data
 	 */
-	err = usbd_device2interface_handle(sc->sc_udev, 0, &iface);
+	err = usbd_device2interface_handle(sc->sc_udev, 0, &sc->sc_iface0);
 	if (err) {
 		aprint_error("%s: Could not get interface 0 handle %s (%d)\n",
 				USBDEVNAME(sc->sc_dev), usbd_errstr(err), err);
@@ -361,12 +364,12 @@ USB_ATTACH(ubt)
 	sc->sc_aclwr_addr = -1;
 
 	count = 0;
-	(void)usbd_endpoint_count(iface, &count);
+	(void)usbd_endpoint_count(sc->sc_iface0, &count);
 
 	for (i = 0 ; i < count ; i++) {
 		int dir, type;
 
-		ed = usbd_interface2endpoint_descriptor(iface, i);
+		ed = usbd_interface2endpoint_descriptor(sc->sc_iface0, i);
 		if (ed == NULL) {
 			aprint_error("%s: could not read endpoint descriptor %d\n",
 			    USBDEVNAME(sc->sc_dev), i);
@@ -404,10 +407,6 @@ USB_ATTACH(ubt)
 		USB_ATTACH_ERROR_RETURN;
 	}
 
-	/* Interface 0 Ok */
-	sc->sc_iface0 = iface;
-	uaa->ifaces[0] = NULL;
-
 	/*
 	 * Interface 1 must have 2 endpoints
 	 *	1) Isochronous IN endpoint to receive SCO data
@@ -417,7 +416,7 @@ USB_ATTACH(ubt)
 	 * via a sysctl variable. We select config 0 to start, which
 	 * means that no SCO data will be available.
 	 */
-	err = usbd_device2interface_handle(sc->sc_udev, 1, &iface);
+	err = usbd_device2interface_handle(sc->sc_udev, 1, &sc->sc_iface1);
 	if (err) {
 		aprint_error("%s: Could not get interface 1 handle %s (%d)\n",
 				USBDEVNAME(sc->sc_dev), usbd_errstr(err), err);
@@ -435,10 +434,6 @@ USB_ATTACH(ubt)
 
 	sc->sc_alt_config = usbd_get_no_alts(cd, 1);
 
-	/* Interface 1 Ok */
-	sc->sc_iface1 = iface;
-	uaa->ifaces[1] = NULL;
-
 	/* set initial config */
 	err = ubt_set_isoc_config(sc);
 	if (err) {
@@ -518,6 +513,7 @@ USB_ATTACH(ubt)
 			CTL_CREATE, CTL_EOL);
 	}
 
+	sc->sc_ok = 1;
 	USB_ATTACH_SUCCESS_RETURN;
 }
 
@@ -530,6 +526,9 @@ USB_DETACH(ubt)
 
 	sc->sc_dying = 1;
 
+	if (!sc->sc_ok)
+		return 0;
+
 	/* delete sysctl nodes */
 	sysctl_teardown(&sc->sc_log);
 

--==_Exmh_19411231074000--