Subject: Fixes for if_cdce.c; need wider testing
To: NetBSD Current Users <current-users@netbsd.org>
From: Barry Bouwsma <freebsd-misuser@remove-NOSPAM-to-reply.NOSPAM.dyndns.dk>
List: current-users
Date: 01/25/2005 21:03:19
Moin,

I've made a couple fixes to if_cdce.c (USB CDC Ethernet) to make it
work with my DOCSIS Cabal Modem, and I'd like to know if others have
any problems with my hacks.

Specifically, one should now be able to use a CDC Ethernet device such
as a cable modem, which probably did not work before.  So I'd like to
hear if this is the case, or if it used to work with any such devices
and no longer works with my changes.

In particular, I've had to make one change that may affect other PDA-
like devices which use this driver; the other changes I've had to make
to get the cable modem to work probably would not affect any other
devices, but it wouldn't hurt to verify this.

I've sent this message using this driver (as ported to FreeBSD), so it
seems to work.  I'd been pounding on it for quite a while.  The only
problem I notice, which also appears to be the case with the much-
cleaner NetBSD hacks, is that when I'm saturating the download, I'll
see the modem USB interface get in a wedged state, from which I can
recover by disconnecting and reconnecting the USB cable (and then
reconfiguring the interface after it reappears).  I can't say whether
this would be a problem in this CDC Ethernet driver, or elsewhere in
the USB (OHCI, haven't tried UHCI yet, modem doesn't talk EHCI) stack.
This is seen both with FreeBSD and NetBSD.  Up until I noticed this,
it worked swell for several weeks with no problems.

Here the diffs against a recent src/sys/dev/usb/if_cdce.c from -current:

--- if_cdce.c-DIST	Sun Oct 24 14:50:54 2004
+++ if_cdce.c	Tue Jan 25 20:50:13 2005
@@ -41,7 +41,13 @@
  */
 
 #include <sys/cdefs.h>
+#if defined(__NetBSD__)
 __KERNEL_RCSID(0, "$NetBSD: if_cdce.c,v 1.4 2004/10/24 12:50:54 augustss Exp $");
+#include "opt_inet.h"
+#include "opt_ns.h"
+#include "rnd.h"
+#endif
+
 #include "bpfilter.h"
 
 #include <sys/param.h>
@@ -163,8 +169,9 @@
 	usb_interface_descriptor_t	*id;
 	usb_endpoint_descriptor_t	*ed;
 	const usb_cdc_union_descriptor_t *ud;
+	usb_config_descriptor_t		*cd = usbd_get_config_descriptor(dev);
 	int				 data_ifcno;
-	int				 i;
+	int				 i, j, numalts;
 	u_char				 eaddr[ETHER_ADDR_LEN];
 	const usb_cdc_ethernet_descriptor_t *ue;
 	char				 eaddr_str[USB_MAX_STRING_LEN];
@@ -210,30 +217,56 @@
 		USB_ATTACH_ERROR_RETURN;
 	}
 
-	/* Find endpoints. */
+	/* In the case of my CDCEthernet modem, there are several alternate
+	 * interface possibilities -- the default first one does not work
+	 * (though in the original code it probably worked for those devices
+	 * which were supported), and in my case, the next alternate setting
+	 * gives me the desired interface.  There's another one that also
+	 * does not work, so we loop through the possibilities and take the
+	 * first one that works.
+	 */
 	id = usbd_get_interface_descriptor(sc->cdce_data_iface);
-	sc->cdce_bulkin_no = sc->cdce_bulkout_no = -1;
-	for (i = 0; i < id->bNumEndpoints; i++) {
-		ed = usbd_interface2endpoint_descriptor(sc->cdce_data_iface, i);
-		if (!ed) {
-			printf("%s: could not read endpoint descriptor\n",
-			    USBDEVNAME(sc->cdce_dev));
+	numalts = usbd_get_no_alts(cd, id->bInterfaceNumber);
+
+	for (j = 0; j < numalts; j++) {
+		if (usbd_set_interface(sc->cdce_data_iface, j)) {
+			printf("cdce%d: setting alternate interface failed\n",
+			    sc->cdce_unit);
 			USB_ATTACH_ERROR_RETURN;
 		}
-		if (UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_IN &&
-		    UE_GET_XFERTYPE(ed->bmAttributes) == UE_BULK) {
-			sc->cdce_bulkin_no = ed->bEndpointAddress;
-		} else if (UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_OUT &&
-		    UE_GET_XFERTYPE(ed->bmAttributes) == UE_BULK) {
-			sc->cdce_bulkout_no = ed->bEndpointAddress;
-		} else if (UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_IN &&
-		    UE_GET_XFERTYPE(ed->bmAttributes) == UE_INTERRUPT) {
-			/* XXX: CDC spec defines an interrupt pipe, but it is not
-			 * needed for simple host-to-host applications. */
-		} else {
-			printf("%s: unexpected endpoint\n",
-			    USBDEVNAME(sc->cdce_dev));
+
+		/* Find endpoints. */
+		id = usbd_get_interface_descriptor(sc->cdce_data_iface);
+		sc->cdce_bulkin_no = sc->cdce_bulkout_no = -1;
+		for (i = 0; i < id->bNumEndpoints; i++) {
+			ed = usbd_interface2endpoint_descriptor(sc->cdce_data_iface, i);
+			if (!ed) {
+				printf("%s: could not read endpoint descriptor\n",
+				    USBDEVNAME(sc->cdce_dev));
+				USB_ATTACH_ERROR_RETURN;
+			}
+			if (UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_IN &&
+			    UE_GET_XFERTYPE(ed->bmAttributes) == UE_BULK) {
+				sc->cdce_bulkin_no = ed->bEndpointAddress;
+			} else if (UE_GET_DIR(ed->bEndpointAddress) ==
+			    UE_DIR_OUT && UE_GET_XFERTYPE(ed->bmAttributes) ==
+			    UE_BULK) {
+				sc->cdce_bulkout_no = ed->bEndpointAddress;
+			} else if (UE_GET_DIR(ed->bEndpointAddress) ==
+			    UE_DIR_IN && UE_GET_XFERTYPE(ed->bmAttributes) ==
+			    UE_INTERRUPT) {
+				/* XXX: CDC spec defines an interrupt pipe,
+				 * but it is not needed for simple
+				 * host-to-host applications. */
+			} else {
+				printf("%s: unexpected endpoint\n",
+				    USBDEVNAME(sc->cdce_dev));
+			}
 		}
+
+		/* If we found something, try and use it... */
+		if ((sc->cdce_bulkin_no != -1) && (sc->cdce_bulkout_no != -1))
+		    break;
 	}
 
 	if (sc->cdce_bulkin_no == -1) {
@@ -248,7 +281,10 @@
 	}
 
 	ue = (usb_cdc_ethernet_descriptor_t *)usb_find_desc(dev,
-            UDESC_INTERFACE, UDESCSUB_CDC_ENF);
+/* XXX is this right?            UDESC_INTERFACE, UDESCSUB_CDC_ENF); */
+/* XXX With the below, I'm able to access my cabal modem.  Dunno if this
+ *     _CS_INTERFACE breaks anything else, and no word from anyone... */
+            UDESC_CS_INTERFACE, UDESCSUB_CDC_ENF);
 	if (!ue || usbd_get_string(dev, ue->iMacAddress, eaddr_str)) {
 		printf("%s: faking address\n", USBDEVNAME(sc->cdce_dev));
 		eaddr[0]= 0x2a;
@@ -266,7 +302,9 @@
 			else
 				c -= 'A' - 10;
 			c &= 0xf;
-			if (c%2 == 0)
+/* XXX This seems wrong			if (c%2 == 0) */
+/* XXX with the below instead, I can actually use the cabal modem */
+			if (i%2 == 0)
 				c <<= 4;
 			eaddr[i / 2] |= c;
 		}
@@ -274,7 +312,7 @@
 
 	s = splnet();
 
-	printf("%s: address %s\n", USBDEVNAME(sc->cdce_dev),
+	printf("%s: Ethernet address %s\n", USBDEVNAME(sc->cdce_dev),
 	    ether_sprintf(eaddr));
 
 	ifp = GET_IFP(sc);


Please eyeball and review this, and if you can, try it out, and
decide if it's a candidate for inclusion in -current.  Note that
I don't know what I'm doing.


thanks
barry bouwsma