Re: [PATCH] Re: [linux-usb-devel] 2.6.6-rc1: cdc-acm still (differently) broken

[Colin Leroy <colin@colino.net>]

[-- Attachment #1: Type: text/plain, Size: 187 bytes --]

On 16 Apr 2004 at 23h04, Colin Leroy wrote:

Hi, 

> Ok, here is the cosmetic patch following the previous one, then :)

Uhm, previous patch is broken.
Here are the two again.

-- 
Colin

[-- Attachment #2: 1_cdc-acm.patch --]
[-- Type: application/octet-stream, Size: 3474 bytes --]

--- drivers/usb/class/cdc-acm.c.orig	2004-04-15 20:04:47.000000000 +0200
+++ drivers/usb/class/cdc-acm.c	2004-04-16 12:21:40.120865512 +0200
@@ -581,45 +581,47 @@
 
 	dev = interface_to_usbdev (intf);
 
-		cfacm = dev->actconfig;
-
-		for (j = 0; j < cfacm->desc.bNumInterfaces - 1; j++) {
-		    
-			if (usb_interface_claimed(cfacm->interface[j]) ||
-			    usb_interface_claimed(cfacm->interface[j + 1]))
-			continue;
+			cfacm = dev->actconfig;
+	
+			/* We know we're probe()d with the control interface. */
+			ifcom = intf->cur_altsetting;
 
-			/* We know we're probe()d with the control interface.
-			 * FIXME ACM doesn't guarantee the data interface is
+			/* ACM doesn't guarantee the data interface is
 			 * adjacent to the control interface, or that if one
-			 * is there it's not for call management ... so use
-			 * the cdc union descriptor whenever there is one.
+			 * is there it's not for call management ... so find
+			 * it
 			 */
-			ifcom = intf->cur_altsetting;
-			if (intf == cfacm->interface[j]) {
-				ifdata = cfacm->interface[j + 1]->cur_altsetting;
-				data = cfacm->interface[j + 1];
-			} else if (intf == cfacm->interface[j + 1]) {
+			for (j = 0; j < cfacm->desc.bNumInterfaces; j++) {
 				ifdata = cfacm->interface[j]->cur_altsetting;
 				data = cfacm->interface[j];
-			} else
-				continue;
-
-			if (ifdata->desc.bInterfaceClass != 10 || ifdata->desc.bNumEndpoints < 2)
-				continue;
-
-			epctrl = &ifcom->endpoint[0].desc;
-			epread = &ifdata->endpoint[0].desc;
-			epwrite = &ifdata->endpoint[1].desc;
-
-			if ((epctrl->bEndpointAddress & 0x80) != 0x80 || (epctrl->bmAttributes & 3) != 3 ||
-			   (epread->bmAttributes & 3) != 2 || (epwrite->bmAttributes & 3) != 2 ||
-			   ((epread->bEndpointAddress & 0x80) ^ (epwrite->bEndpointAddress & 0x80)) != 0x80)
-				continue;
-
-			if ((epread->bEndpointAddress & 0x80) != 0x80) {
-				epread = &ifdata->endpoint[1].desc;
-				epwrite = &ifdata->endpoint[0].desc;
+
+				if (ifdata->desc.bInterfaceClass == 10 &&
+				    ifdata->desc.bNumEndpoints == 2) {
+					epctrl = &ifcom->endpoint[0].desc;
+					epread = &ifdata->endpoint[0].desc;
+					epwrite = &ifdata->endpoint[1].desc;
+
+					if ((epctrl->bEndpointAddress & 0x80) != 0x80 ||
+					    (epctrl->bmAttributes & 3) != 3 ||
+					    (epread->bmAttributes & 3) != 2 || 
+					    (epwrite->bmAttributes & 3) != 2 ||
+					    ((epread->bEndpointAddress & 0x80) ^ (epwrite->bEndpointAddress & 0x80)) != 0x80) 
+						goto next_interface;
+
+					dbg("found data interface at %d\n", j);
+					break;
+				} else {
+next_interface:
+					ifdata = NULL;
+					data = NULL;
+				}
+			}
+
+			/* there's been a problem */
+			if (!ifdata) {
+				dbg("interface not found (%p)\n", ifdata);
+				return -ENODEV;
+
 			}
 
 			for (minor = 0; minor < ACM_TTY_MINORS && acm_table[minor]; minor++);
@@ -696,16 +698,21 @@
 			acm->line.databits = 8;
 			acm_set_line(acm, &acm->line);
 
-			usb_driver_claim_interface(&acm_driver, data, acm);
+			if ( (j = usb_driver_claim_interface(&acm_driver, data, acm)) != 0) {
+				err("claim failed");
+				usb_free_urb(acm->ctrlurb);
+				usb_free_urb(acm->readurb);
+				usb_free_urb(acm->writeurb);
+				kfree(acm);
+				kfree(buf);
+				return j;
+			} 
 
 			tty_register_device(acm_tty_driver, minor, &intf->dev);
 
 			acm_table[minor] = acm;
 			usb_set_intfdata (intf, acm);
 			return 0;
-		}
-
-	return -EIO;
 }
 
 static void acm_disconnect(struct usb_interface *intf)

[-- Attachment #3: 2_cdc-acm-cosmetic.patch --]
[-- Type: application/octet-stream, Size: 7663 bytes --]

--- drivers/usb/class/cdc-acm.c.new	2004-04-16 23:44:14.208234976 +0200
+++ drivers/usb/class/cdc-acm.c	2004-04-16 23:44:45.442486648 +0200
@@ -581,138 +581,138 @@
 
 	dev = interface_to_usbdev (intf);
 
-			cfacm = dev->actconfig;
-	
-			/* We know we're probe()d with the control interface. */
-			ifcom = intf->cur_altsetting;
-
-			/* ACM doesn't guarantee the data interface is
-			 * adjacent to the control interface, or that if one
-			 * is there it's not for call management ... so find
-			 * it
-			 */
-			for (j = 0; j < cfacm->desc.bNumInterfaces; j++) {
-				ifdata = cfacm->interface[j]->cur_altsetting;
-				data = cfacm->interface[j];
-
-				if (ifdata->desc.bInterfaceClass == 10 &&
-				    ifdata->desc.bNumEndpoints == 2) {
-					epctrl = &ifcom->endpoint[0].desc;
-					epread = &ifdata->endpoint[0].desc;
-					epwrite = &ifdata->endpoint[1].desc;
-
-					if ((epctrl->bEndpointAddress & 0x80) != 0x80 ||
-					    (epctrl->bmAttributes & 3) != 3 ||
-					    (epread->bmAttributes & 3) != 2 || 
-					    (epwrite->bmAttributes & 3) != 2 ||
-					    ((epread->bEndpointAddress & 0x80) ^ (epwrite->bEndpointAddress & 0x80)) != 0x80) 
-						goto next_interface;
-
-					dbg("found data interface at %d\n", j);
-					break;
-				} else {
+	cfacm = dev->actconfig;
+
+	/* We know we're probe()d with the control interface. */
+	ifcom = intf->cur_altsetting;
+
+	/* ACM doesn't guarantee the data interface is
+	 * adjacent to the control interface, or that if one
+	 * is there it's not for call management ... so find
+	 * it
+	 */
+	for (j = 0; j < cfacm->desc.bNumInterfaces; j++) {
+		ifdata = cfacm->interface[j]->cur_altsetting;
+		data = cfacm->interface[j];
+
+		if (ifdata->desc.bInterfaceClass == 10 &&
+		    ifdata->desc.bNumEndpoints == 2) {
+			epctrl = &ifcom->endpoint[0].desc;
+			epread = &ifdata->endpoint[0].desc;
+			epwrite = &ifdata->endpoint[1].desc;
+
+			if ((epctrl->bEndpointAddress & 0x80) != 0x80 ||
+			    (epctrl->bmAttributes & 3) != 3 ||
+			    (epread->bmAttributes & 3) != 2 || 
+			    (epwrite->bmAttributes & 3) != 2 ||
+			    ((epread->bEndpointAddress & 0x80) ^ (epwrite->bEndpointAddress & 0x80)) != 0x80) 
+				goto next_interface;
+
+			dbg("found data interface at %d\n", j);
+			break;
+		} else {
 next_interface:
-					ifdata = NULL;
-					data = NULL;
-				}
-			}
-
-			/* there's been a problem */
-			if (!ifdata) {
-				dbg("interface not found (%p)\n", ifdata);
-				return -ENODEV;
-
-			}
-
-			for (minor = 0; minor < ACM_TTY_MINORS && acm_table[minor]; minor++);
-			if (acm_table[minor]) {
-				err("no more free acm devices");
-				return -ENODEV;
-			}
-
-			if (!(acm = kmalloc(sizeof(struct acm), GFP_KERNEL))) {
-				err("out of memory");
-				return -ENOMEM;
-			}
-			memset(acm, 0, sizeof(struct acm));
-
-			ctrlsize = epctrl->wMaxPacketSize;
-			readsize = epread->wMaxPacketSize;
-			acm->writesize = epwrite->wMaxPacketSize;
-			acm->control = intf;
-			acm->data = data;
-			acm->minor = minor;
-			acm->dev = dev;
-
-			acm->bh.func = acm_rx_tasklet;
-			acm->bh.data = (unsigned long) acm;
-			INIT_WORK(&acm->work, acm_softint, acm);
-
-			if (!(buf = kmalloc(ctrlsize + readsize + acm->writesize, GFP_KERNEL))) {
-				err("out of memory");
-				kfree(acm);
-				return -ENOMEM;
-			}
-
-			acm->ctrlurb = usb_alloc_urb(0, GFP_KERNEL);
-			if (!acm->ctrlurb) {
-				err("out of memory");
-				kfree(acm);
-				kfree(buf);
-				return -ENOMEM;
-			}
-			acm->readurb = usb_alloc_urb(0, GFP_KERNEL);
-			if (!acm->readurb) {
-				err("out of memory");
-				usb_free_urb(acm->ctrlurb);
-				kfree(acm);
-				kfree(buf);
-				return -ENOMEM;
-			}
-			acm->writeurb = usb_alloc_urb(0, GFP_KERNEL);
-			if (!acm->writeurb) {
-				err("out of memory");
-				usb_free_urb(acm->readurb);
-				usb_free_urb(acm->ctrlurb);
-				kfree(acm);
-				kfree(buf);
-				return -ENOMEM;
-			}
-
-			usb_fill_int_urb(acm->ctrlurb, dev, usb_rcvintpipe(dev, epctrl->bEndpointAddress),
-				buf, ctrlsize, acm_ctrl_irq, acm, epctrl->bInterval);
-
-			usb_fill_bulk_urb(acm->readurb, dev, usb_rcvbulkpipe(dev, epread->bEndpointAddress),
-				buf += ctrlsize, readsize, acm_read_bulk, acm);
-			acm->readurb->transfer_flags |= URB_NO_FSBR;
-
-			usb_fill_bulk_urb(acm->writeurb, dev, usb_sndbulkpipe(dev, epwrite->bEndpointAddress),
-				buf += readsize, acm->writesize, acm_write_bulk, acm);
-			acm->writeurb->transfer_flags |= URB_NO_FSBR;
-
-			dev_info(&intf->dev, "ttyACM%d: USB ACM device", minor);
-
-			acm_set_control(acm, acm->ctrlout);
-
-			acm->line.speed = cpu_to_le32(9600);
-			acm->line.databits = 8;
-			acm_set_line(acm, &acm->line);
-
-			if ( (j = usb_driver_claim_interface(&acm_driver, data, acm)) != 0) {
-				err("claim failed");
-				usb_free_urb(acm->ctrlurb);
-				usb_free_urb(acm->readurb);
-				usb_free_urb(acm->writeurb);
-				kfree(acm);
-				kfree(buf);
-				return j;
-			} 
-
-			tty_register_device(acm_tty_driver, minor, &intf->dev);
-
-			acm_table[minor] = acm;
-			usb_set_intfdata (intf, acm);
-			return 0;
+			ifdata = NULL;
+			data = NULL;
+		}
+	}
+
+	/* there's been a problem */
+	if (!ifdata) {
+		dbg("interface not found (%p)\n", ifdata);
+		return -ENODEV;
+
+	}
+
+	for (minor = 0; minor < ACM_TTY_MINORS && acm_table[minor]; minor++);
+	if (acm_table[minor]) {
+		err("no more free acm devices");
+		return -ENODEV;
+	}
+
+	if (!(acm = kmalloc(sizeof(struct acm), GFP_KERNEL))) {
+		err("out of memory");
+		return -ENOMEM;
+	}
+	memset(acm, 0, sizeof(struct acm));
+
+	ctrlsize = epctrl->wMaxPacketSize;
+	readsize = epread->wMaxPacketSize;
+	acm->writesize = epwrite->wMaxPacketSize;
+	acm->control = intf;
+	acm->data = data;
+	acm->minor = minor;
+	acm->dev = dev;
+
+	acm->bh.func = acm_rx_tasklet;
+	acm->bh.data = (unsigned long) acm;
+	INIT_WORK(&acm->work, acm_softint, acm);
+
+	if (!(buf = kmalloc(ctrlsize + readsize + acm->writesize, GFP_KERNEL))) {
+		err("out of memory");
+		kfree(acm);
+		return -ENOMEM;
+	}
+
+	acm->ctrlurb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!acm->ctrlurb) {
+		err("out of memory");
+		kfree(acm);
+		kfree(buf);
+		return -ENOMEM;
+	}
+	acm->readurb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!acm->readurb) {
+		err("out of memory");
+		usb_free_urb(acm->ctrlurb);
+		kfree(acm);
+		kfree(buf);
+		return -ENOMEM;
+	}
+	acm->writeurb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!acm->writeurb) {
+		err("out of memory");
+		usb_free_urb(acm->readurb);
+		usb_free_urb(acm->ctrlurb);
+		kfree(acm);
+		kfree(buf);
+		return -ENOMEM;
+	}
+
+	usb_fill_int_urb(acm->ctrlurb, dev, usb_rcvintpipe(dev, epctrl->bEndpointAddress),
+		buf, ctrlsize, acm_ctrl_irq, acm, epctrl->bInterval);
+
+	usb_fill_bulk_urb(acm->readurb, dev, usb_rcvbulkpipe(dev, epread->bEndpointAddress),
+		buf += ctrlsize, readsize, acm_read_bulk, acm);
+	acm->readurb->transfer_flags |= URB_NO_FSBR;
+
+	usb_fill_bulk_urb(acm->writeurb, dev, usb_sndbulkpipe(dev, epwrite->bEndpointAddress),
+		buf += readsize, acm->writesize, acm_write_bulk, acm);
+	acm->writeurb->transfer_flags |= URB_NO_FSBR;
+
+	dev_info(&intf->dev, "ttyACM%d: USB ACM device", minor);
+
+	acm_set_control(acm, acm->ctrlout);
+
+	acm->line.speed = cpu_to_le32(9600);
+	acm->line.databits = 8;
+	acm_set_line(acm, &acm->line);
+
+	if ( (j = usb_driver_claim_interface(&acm_driver, data, acm)) != 0) {
+		err("claim failed");
+		usb_free_urb(acm->ctrlurb);
+		usb_free_urb(acm->readurb);
+		usb_free_urb(acm->writeurb);
+		kfree(acm);
+		kfree(buf);
+		return j;
+	} 
+
+	tty_register_device(acm_tty_driver, minor, &intf->dev);
+
+	acm_table[minor] = acm;
+	usb_set_intfdata (intf, acm);
+	return 0;
 }
 
 static void acm_disconnect(struct usb_interface *intf)