This is a multipart MIME message.

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


Checking what obstacles are in the way to loadable USB drivers
I found one implementation detail which makes it unnecessarily
hard -- the way "subdevices" are handled.

"subdevices" are logical devices (in terms of the NetBSD autoconf
framework) which are handling one physical USB device. There
is the possibility that one USB device provides multiple
"interfaces" (that's a USB term here) which different drivers
can attach to.

These are managed in a NULL-terminated list. That's bad because
with loadable drivers a device (in terms of autoconf) can disappear
when the driver is detached, independantly of hardware events.
We could make that an array with known length, or a linked list.
I've tried the former just to get an impression how invasive
this gets. Patch appended, just to give an impression -- there
is more to do unfortunately.
Comments?

And while we are here: is there any use of the udi_devnames[]
thing?

best regards
Matthias



--==_Exmh_7602827276590
Content-Type: text/plain ; name="usbsubdev.txt"; charset=us-ascii
Content-Description: usbsubdev.txt
Content-Disposition: attachment; filename="usbsubdev.txt"

# 
# patch "dev/usb/usb_subr.c"
#  from [1a53090eb6f197d0a5a4bfd6b313f6c6856ef39a]
#    to [08f6ce21de7a92ce5d0b4fd5c8af853b6ac4f01d]
# 
# patch "dev/usb/usbdivar.h"
#  from [8da871cf590c4ed2ebb4a8570123ff31d50131d4]
#    to [23ee345663ffa091c519c9a6a18afc365699359e]
# 
--- dev/usb/usb_subr.c
+++ dev/usb/usb_subr.c
@@ -851,11 +851,11 @@
 	DPRINTF(("usbd_probe_and_attach: trying device specific drivers\n"));
 	dv = USB_DO_ATTACH(dev, bdev, parent, &uaa, usbd_print, usbd_submatch);
 	if (dv) {
-		dev->subdevs = malloc(2 * sizeof dv, M_USB, M_NOWAIT);
+		dev->nsubdevs = 1;
+		dev->subdevs = malloc(sizeof dv, M_USB, M_NOWAIT);
 		if (dev->subdevs == NULL)
 			return (USBD_NOMEM);
 		dev->subdevs[0] = dv;
-		dev->subdevs[1] = 0;
 		return (USBD_NORMAL_COMPLETION);
 	}
 
@@ -889,7 +889,8 @@
 			ifaces[i] = &dev->ifaces[i];
 		uaa.ifaces = ifaces;
 		uaa.nifaces = nifaces;
-		dev->subdevs = malloc((nifaces+1) * sizeof dv, M_USB,M_NOWAIT);
+		dev->nsubdevs = nifaces;
+		dev->subdevs = malloc(nifaces * sizeof dv, M_USB, M_NOWAIT|M_ZERO);
 		if (dev->subdevs == NULL) {
 #if defined(__FreeBSD__)
 			device_delete_child(parent, bdev);
@@ -906,8 +907,8 @@
 			dv = USB_DO_ATTACH(dev, bdev, parent, &uaa, usbd_print,
 					   usbd_submatch);
 			if (dv != NULL) {
-				dev->subdevs[found++] = dv;
-				dev->subdevs[found] = 0;
+				found++;
+				dev->subdevs[i] = dv;
 				ifaces[i] = 0; /* consumed */
 
 #if defined(__FreeBSD__)
@@ -930,7 +931,7 @@
 			return (USBD_NORMAL_COMPLETION);
 		}
 		free(dev->subdevs, M_USB);
-		dev->subdevs = 0;
+		dev->subdevs = NULL;
 	}
 	/* No interfaces were attached in any of the configurations. */
 
@@ -946,11 +947,11 @@
 	uaa.ifaceno = UHUB_UNK_INTERFACE;
 	dv = USB_DO_ATTACH(dev, bdev, parent, &uaa, usbd_print, usbd_submatch);
 	if (dv != NULL) {
-		dev->subdevs = malloc(2 * sizeof dv, M_USB, M_NOWAIT);
-		if (dev->subdevs == 0)
+		dev->nsubdevs = 1;
+		dev->subdevs = malloc(sizeof dv, M_USB, M_NOWAIT);
+		if (dev->subdevs == NULL)
 			return (USBD_NOMEM);
 		dev->subdevs[0] = dv;
-		dev->subdevs[1] = 0;
 		return (USBD_NORMAL_COMPLETION);
 	}
 
@@ -1271,7 +1272,7 @@
 		     int usedev)
 {
 	struct usbd_port *p;
-	int i, err, s;
+	int i, j, err, s;
 
 	di->udi_bus = USBDEVUNIT(dev->bus->bdev);
 	di->udi_addr = dev->address;
@@ -1294,17 +1295,20 @@
 	di->udi_speed = dev->speed;
 
 	if (dev->subdevs != NULL) {
-		for (i = 0; dev->subdevs[i] &&
-			     i < USB_MAX_DEVNAMES; i++) {
-			strncpy(di->udi_devnames[i], USBDEVPTRNAME(dev->subdevs[i]),
+		for (i = 0, j = 0; i < dev->nsubdevs && j < USB_MAX_DEVNAMES;
+		     i++) {
+			if (!dev->subdevs[i])
+				continue;
+			strncpy(di->udi_devnames[j], USBDEVPTRNAME(dev->subdevs[i]),
 				USB_MAX_DEVNAMELEN);
-			di->udi_devnames[i][USB_MAX_DEVNAMELEN-1] = '\0';
+			di->udi_devnames[j][USB_MAX_DEVNAMELEN-1] = '\0';
+			j++;
                 }
         } else {
-                i = 0;
+                j = 0;
         }
-        for (/*i is set */; i < USB_MAX_DEVNAMES; i++)
-                di->udi_devnames[i][0] = 0;                 /* empty */
+        for (/*j is set */; j < USB_MAX_DEVNAMES; j++)
+                di->udi_devnames[j][0] = 0;                 /* empty */
 
 	if (dev->hub) {
 		for (i = 0;
@@ -1388,7 +1392,9 @@
 
 	if (dev->subdevs != NULL) {
 		DPRINTFN(3,("usb_disconnect_port: disconnect subdevs\n"));
-		for (i = 0; dev->subdevs[i]; i++) {
+		for (i = 0; i < dev->nsubdevs; i++) {
+			if (!dev->subdevs[i])
+				continue;
 			printf("%s: at %s", USBDEVPTRNAME(dev->subdevs[i]),
 			       hubname);
 			if (up->portno != 0)
--- dev/usb/usbdivar.h
+++ dev/usb/usbdivar.h
@@ -157,7 +157,8 @@
 	usb_config_descriptor_t *cdesc;	       /* full config descr */
 	const struct usbd_quirks     *quirks;  /* device quirks, always set */
 	struct usbd_hub	       *hub;           /* only if this is a hub */
-	device_ptr_t	       *subdevs;       /* sub-devices, 0 terminated */
+	int			nsubdevs;      /* length of subdevs array */
+	device_ptr_t	       *subdevs;       /* sub-devices */
 };
 
 struct usbd_interface {

--==_Exmh_7602827276590--