linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] USBATM: summary
@ 2006-01-12 16:29 Duncan Sands
  2006-01-12 16:43 ` [PATCH 01/13] USBATM: trivial modifications Duncan Sands
                   ` (13 more replies)
  0 siblings, 14 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-12 16:29 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-usb-devel, linux-kernel, usbatm

Hi Greg, here are some fixes and improvements to the USB ATM
modem drivers, in thirteen patches:

01: trivial modifications (formatting, changes to variable names,
comments, log level changes, printk rate limiting).

02: have minidrivers tell the core about special requirements
using a flags field.

03: remove the unused .owner field in struct usbatm_driver.

04: use kzalloc rather than kmalloc + memset.

05: make xusbatm useable.

06: sternly tell open connections that the game is up when the
modem is disconnected.

07: return the correct error code when out of memory.

08: use dev_kfree_skb_any rather than dev_kfree_skb.

09: specify buffer sizes in bytes, rather than in ATM cells.

10: add mechanism for turning on isochronous urb support.

11: remove the assumption that incoming urbs contain
complete ATM cells.

12: bump some version numbers.

13: EILSEQ hack needed by the ueagle.

All the best,

Duncan.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/13] USBATM: trivial modifications
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
@ 2006-01-12 16:43 ` Duncan Sands
  2006-01-12 17:40   ` Duncan Sands
  2006-01-12 17:48 ` [PATCH 02/13] USBATM: add flags field Duncan Sands
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Duncan Sands @ 2006-01-12 16:43 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

Formatting, changes to variable names, comments, log level changes,
printk rate limiting.

Signed-off-by:	Duncan Sands <baldrick@free.fr>

[-- Attachment #2: trivial --]
[-- Type: text/x-diff, Size: 30174 bytes --]

Index: kernel/cxacru.c
===================================================================
--- kernel.orig/cxacru.c	2006-01-10 08:42:02.000000000 +0100
+++ kernel/cxacru.c	2006-01-12 16:08:35.000000000 +0100
@@ -352,7 +352,6 @@
 		struct atm_dev *atm_dev)
 {
 	struct cxacru_data *instance = usbatm_instance->driver_data;
-	struct device *dev = &usbatm_instance->usb_intf->dev;
 	/*
 	struct atm_dev *atm_dev = usbatm_instance->atm_dev;
 	*/
@@ -364,14 +363,14 @@
 	ret = cxacru_cm(instance, CM_REQUEST_CARD_GET_MAC_ADDRESS, NULL, 0,
 			atm_dev->esi, sizeof(atm_dev->esi));
 	if (ret < 0) {
-		dev_err(dev, "cxacru_atm_start: CARD_GET_MAC_ADDRESS returned %d\n", ret);
+		atm_err(usbatm_instance, "cxacru_atm_start: CARD_GET_MAC_ADDRESS returned %d\n", ret);
 		return ret;
 	}
 
 	/* start ADSL */
 	ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0);
 	if (ret < 0) {
-		dev_err(dev, "cxacru_atm_start: CHIP_ADSL_LINE_START returned %d\n", ret);
+		atm_err(usbatm_instance, "cxacru_atm_start: CHIP_ADSL_LINE_START returned %d\n", ret);
 		return ret;
 	}
 
@@ -383,13 +382,13 @@
 static void cxacru_poll_status(struct cxacru_data *instance)
 {
 	u32 buf[CXINF_MAX] = {};
-	struct device *dev = &instance->usbatm->usb_intf->dev;
-	struct atm_dev *atm_dev = instance->usbatm->atm_dev;
+	struct usbatm_data *usbatm = instance->usbatm;
+	struct atm_dev *atm_dev = usbatm->atm_dev;
 	int ret;
 
 	ret = cxacru_cm_get_array(instance, CM_REQUEST_CARD_INFO_GET, buf, CXINF_MAX);
 	if (ret < 0) {
-		dev_warn(dev, "poll status: error %d\n", ret);
+		atm_warn(usbatm, "poll status: error %d\n", ret);
 		goto reschedule;
 	}
 
@@ -400,50 +399,50 @@
 	switch (instance->line_status) {
 	case 0:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: down\n");
+		atm_info(usbatm, "ADSL line: down\n");
 		break;
 
 	case 1:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: attemtping to activate\n");
+		atm_info(usbatm, "ADSL line: attempting to activate\n");
 		break;
 
 	case 2:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: training\n");
+		atm_info(usbatm, "ADSL line: training\n");
 		break;
 
 	case 3:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: channel analysis\n");
+		atm_info(usbatm, "ADSL line: channel analysis\n");
 		break;
 
 	case 4:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: exchange\n");
+		atm_info(usbatm, "ADSL line: exchange\n");
 		break;
 
 	case 5:
 		atm_dev->link_rate = buf[CXINF_DOWNSTREAM_RATE] * 1000 / 424;
 		atm_dev->signal = ATM_PHY_SIG_FOUND;
 
-		dev_info(dev, "ADSL line: up (%d kb/s down | %d kb/s up)\n",
+		atm_info(usbatm, "ADSL line: up (%d kb/s down | %d kb/s up)\n",
 		     buf[CXINF_DOWNSTREAM_RATE], buf[CXINF_UPSTREAM_RATE]);
 		break;
 
 	case 6:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: waiting\n");
+		atm_info(usbatm, "ADSL line: waiting\n");
 		break;
 
 	case 7:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: initializing\n");
+		atm_info(usbatm, "ADSL line: initializing\n");
 		break;
 
 	default:
 		atm_dev->signal = ATM_PHY_SIG_UNKNOWN;
-		dev_info(dev, "Unknown line state %02x\n", instance->line_status);
+		atm_info(usbatm, "Unknown line state %02x\n", instance->line_status);
 		break;
 	}
 reschedule:
@@ -504,8 +503,8 @@
 {
 	int ret;
 	int off;
-	struct usb_device *usb_dev = instance->usbatm->usb_dev;
-	struct device *dev = &instance->usbatm->usb_intf->dev;
+	struct usbatm_data *usbatm = instance->usbatm;
+	struct usb_device *usb_dev = usbatm->usb_dev;
 	u16 signature[] = { usb_dev->descriptor.idVendor, usb_dev->descriptor.idProduct };
 	u32 val;
 
@@ -515,7 +514,7 @@
 	val = cpu_to_le32(instance->modem_type->pll_f_clk);
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, PLLFCLK_ADDR, (u8 *) &val, 4);
 	if (ret) {
-		dev_err(dev, "FirmwarePllFClkValue failed: %d\n", ret);
+		usb_err(usbatm, "FirmwarePllFClkValue failed: %d\n", ret);
 		return;
 	}
 
@@ -523,7 +522,7 @@
 	val = cpu_to_le32(instance->modem_type->pll_b_clk);
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, PLLBCLK_ADDR, (u8 *) &val, 4);
 	if (ret) {
-		dev_err(dev, "FirmwarePllBClkValue failed: %d\n", ret);
+		usb_err(usbatm, "FirmwarePllBClkValue failed: %d\n", ret);
 		return;
 	}
 
@@ -531,14 +530,14 @@
 	val = cpu_to_le32(SDRAM_ENA);
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, SDRAMEN_ADDR, (u8 *) &val, 4);
 	if (ret) {
-		dev_err(dev, "Enable SDRAM failed: %d\n", ret);
+		usb_err(usbatm, "Enable SDRAM failed: %d\n", ret);
 		return;
 	}
 
 	/* Firmware */
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, FW_ADDR, fw->data, fw->size);
 	if (ret) {
-		dev_err(dev, "Firmware upload failed: %d\n", ret);
+		usb_err(usbatm, "Firmware upload failed: %d\n", ret);
 		return;
 	}
 
@@ -546,7 +545,7 @@
 	if (instance->modem_type->boot_rom_patch) {
 		ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, BR_ADDR, bp->data, bp->size);
 		if (ret) {
-			dev_err(dev, "Boot ROM patching failed: %d\n", ret);
+			usb_err(usbatm, "Boot ROM patching failed: %d\n", ret);
 			return;
 		}
 	}
@@ -554,7 +553,7 @@
 	/* Signature */
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, SIG_ADDR, (u8 *) signature, 4);
 	if (ret) {
-		dev_err(dev, "Signature storing failed: %d\n", ret);
+		usb_err(usbatm, "Signature storing failed: %d\n", ret);
 		return;
 	}
 
@@ -566,7 +565,7 @@
 		ret = cxacru_fw(usb_dev, FW_GOTO_MEM, 0x0, 0x0, FW_ADDR, NULL, 0);
 	}
 	if (ret) {
-		dev_err(dev, "Passing control to firmware failed: %d\n", ret);
+		usb_err(usbatm, "Passing control to firmware failed: %d\n", ret);
 		return;
 	}
 
@@ -580,7 +579,7 @@
 
 	ret = cxacru_cm(instance, CM_REQUEST_CARD_GET_STATUS, NULL, 0, NULL, 0);
 	if (ret < 0) {
-		dev_err(dev, "modem failed to initialize: %d\n", ret);
+		usb_err(usbatm, "modem failed to initialize: %d\n", ret);
 		return;
 	}
 
@@ -597,7 +596,7 @@
 			ret = cxacru_cm(instance, CM_REQUEST_CARD_DATA_SET,
 					(u8 *) buf, len, NULL, 0);
 			if (ret < 0) {
-				dev_err(dev, "load config data failed: %d\n", ret);
+				usb_err(usbatm, "load config data failed: %d\n", ret);
 				return;
 			}
 		}
@@ -608,18 +607,19 @@
 static int cxacru_find_firmware(struct cxacru_data *instance,
 				char* phase, const struct firmware **fw_p)
 {
-	struct device *dev = &instance->usbatm->usb_intf->dev;
+	struct usbatm_data *usbatm = instance->usbatm;
+	struct device *dev = &usbatm->usb_intf->dev;
 	char buf[16];
 
 	sprintf(buf, "cxacru-%s.bin", phase);
 	dbg("cxacru_find_firmware: looking for %s", buf);
 
 	if (request_firmware(fw_p, buf, dev)) {
-		dev_dbg(dev, "no stage %s firmware found\n", phase);
+		usb_dbg(usbatm, "no stage %s firmware found\n", phase);
 		return -ENOENT;
 	}
 
-	dev_info(dev, "found firmware %s\n", buf);
+	usb_info(usbatm, "found firmware %s\n", buf);
 
 	return 0;
 }
@@ -627,20 +627,19 @@
 static int cxacru_heavy_init(struct usbatm_data *usbatm_instance,
 			     struct usb_interface *usb_intf)
 {
-	struct device *dev = &usbatm_instance->usb_intf->dev;
 	const struct firmware *fw, *bp, *cf;
 	struct cxacru_data *instance = usbatm_instance->driver_data;
 
 	int ret = cxacru_find_firmware(instance, "fw", &fw);
 	if (ret) {
-		dev_warn(dev, "firmware (cxacru-fw.bin) unavailable (hotplug misconfiguration?)\n");
+		usb_warn(usbatm_instance, "firmware (cxacru-fw.bin) unavailable (system misconfigured?)\n");
 		return ret;
 	}
 
 	if (instance->modem_type->boot_rom_patch) {
 		ret = cxacru_find_firmware(instance, "bp", &bp);
 		if (ret) {
-			dev_warn(dev, "boot ROM patch (cxacru-bp.bin) unavailable (hotplug misconfiguration?)\n");
+			usb_warn(usbatm_instance, "boot ROM patch (cxacru-bp.bin) unavailable (system misconfigured?)\n");
 			release_firmware(fw);
 			return ret;
 		}
@@ -787,12 +786,12 @@
 	{ /* V = Conexant			P = ADSL modem (Hasbani project)	*/
 		USB_DEVICE(0x0572, 0xcb00),	.driver_info = (unsigned long) &cxacru_cb00
 	},
-	{ /* V = Conexant             P = ADSL modem (Well PTI-800 */
-		USB_DEVICE(0x0572, 0xcb02),	.driver_info = (unsigned long) &cxacru_cb00
-	},
 	{ /* V = Conexant			P = ADSL modem				*/
 		USB_DEVICE(0x0572, 0xcb01),	.driver_info = (unsigned long) &cxacru_cb00
 	},
+	{ /* V = Conexant			P = ADSL modem (Well PTI-800) */
+		USB_DEVICE(0x0572, 0xcb02),	.driver_info = (unsigned long) &cxacru_cb00
+	},
 	{ /* V = Conexant			P = ADSL modem				*/
 		USB_DEVICE(0x0572, 0xcb06),	.driver_info = (unsigned long) &cxacru_cb00
 	},
Index: kernel/speedtch.c
===================================================================
--- kernel.orig/speedtch.c	2006-01-10 18:48:26.000000000 +0100
+++ kernel/speedtch.c	2006-01-12 16:08:35.000000000 +0100
@@ -205,7 +205,7 @@
 				   buffer, 0x200, &actual_length, 2000);
 
 		if (ret < 0 && ret != -ETIMEDOUT)
-			usb_dbg(usbatm, "%s: read BLOCK0 from modem failed (%d)!\n", __func__, ret);
+			usb_warn(usbatm, "%s: read BLOCK0 from modem failed (%d)!\n", __func__, ret);
 		else
 			usb_dbg(usbatm, "%s: BLOCK0 downloaded (%d bytes)\n", __func__, ret);
 	}
@@ -219,7 +219,7 @@
 				   buffer, thislen, &actual_length, DATA_TIMEOUT);
 
 		if (ret < 0) {
-			usb_dbg(usbatm, "%s: write BLOCK1 to modem failed (%d)!\n", __func__, ret);
+			usb_err(usbatm, "%s: write BLOCK1 to modem failed (%d)!\n", __func__, ret);
 			goto out_free;
 		}
 		usb_dbg(usbatm, "%s: BLOCK1 uploaded (%zu bytes)\n", __func__, fw1->size);
@@ -232,7 +232,7 @@
 			   buffer, 0x200, &actual_length, DATA_TIMEOUT);
 
 	if (ret < 0) {
-		usb_dbg(usbatm, "%s: read BLOCK2 from modem failed (%d)!\n", __func__, ret);
+		usb_err(usbatm, "%s: read BLOCK2 from modem failed (%d)!\n", __func__, ret);
 		goto out_free;
 	}
 	usb_dbg(usbatm, "%s: BLOCK2 downloaded (%d bytes)\n", __func__, actual_length);
@@ -246,7 +246,7 @@
 				   buffer, thislen, &actual_length, DATA_TIMEOUT);
 
 		if (ret < 0) {
-			usb_dbg(usbatm, "%s: write BLOCK3 to modem failed (%d)!\n", __func__, ret);
+			usb_err(usbatm, "%s: write BLOCK3 to modem failed (%d)!\n", __func__, ret);
 			goto out_free;
 		}
 	}
@@ -259,7 +259,7 @@
 			   buffer, 0x200, &actual_length, DATA_TIMEOUT);
 
 	if (ret < 0) {
-		usb_dbg(usbatm, "%s: read BLOCK4 from modem failed (%d)!\n", __func__, ret);
+		usb_err(usbatm, "%s: read BLOCK4 from modem failed (%d)!\n", __func__, ret);
 		goto out_free;
 	}
 
@@ -285,8 +285,8 @@
 	return ret;
 }
 
-static int speedtch_find_firmware(struct usb_interface *intf, int phase,
-				  const struct firmware **fw_p)
+static int speedtch_find_firmware(struct usbatm_data *usbatm, struct usb_interface *intf,
+				  int phase, const struct firmware **fw_p)
 {
 	struct device *dev = &intf->dev;
 	const u16 bcdDevice = le16_to_cpu(interface_to_usbdev(intf)->descriptor.bcdDevice);
@@ -295,24 +295,24 @@
 	char buf[24];
 
 	sprintf(buf, "speedtch-%d.bin.%x.%02x", phase, major_revision, minor_revision);
-	dev_dbg(dev, "%s: looking for %s\n", __func__, buf);
+	usb_dbg(usbatm, "%s: looking for %s\n", __func__, buf);
 
 	if (request_firmware(fw_p, buf, dev)) {
 		sprintf(buf, "speedtch-%d.bin.%x", phase, major_revision);
-		dev_dbg(dev, "%s: looking for %s\n", __func__, buf);
+		usb_dbg(usbatm, "%s: looking for %s\n", __func__, buf);
 
 		if (request_firmware(fw_p, buf, dev)) {
 			sprintf(buf, "speedtch-%d.bin", phase);
-			dev_dbg(dev, "%s: looking for %s\n", __func__, buf);
+			usb_dbg(usbatm, "%s: looking for %s\n", __func__, buf);
 
 			if (request_firmware(fw_p, buf, dev)) {
-				dev_warn(dev, "no stage %d firmware found!\n", phase);
+				usb_err(usbatm, "%s: no stage %d firmware found!\n", __func__, phase);
 				return -ENOENT;
 			}
 		}
 	}
 
-	dev_info(dev, "found stage %d firmware %s\n", phase, buf);
+	usb_info(usbatm, "found stage %d firmware %s\n", phase, buf);
 
 	return 0;
 }
@@ -323,15 +323,16 @@
 	struct speedtch_instance_data *instance = usbatm->driver_data;
 	int ret;
 
-	if ((ret = speedtch_find_firmware(intf, 1, &fw1)) < 0)
-			return ret;
+	if ((ret = speedtch_find_firmware(usbatm, intf, 1, &fw1)) < 0)
+		return ret;
 
-	if ((ret = speedtch_find_firmware(intf, 2, &fw2)) < 0) {
+	if ((ret = speedtch_find_firmware(usbatm, intf, 2, &fw2)) < 0) {
 		release_firmware(fw1);
 		return ret;
 	}
 
-	ret = speedtch_upload_firmware(instance, fw1, fw2);
+	if ((ret = speedtch_upload_firmware(instance, fw1, fw2)) < 0)
+		usb_err(usbatm, "%s: firmware upload failed (%d)!\n", __func__, ret);
 
 	release_firmware(fw2);
 	release_firmware(fw1);
@@ -428,7 +429,9 @@
 	int down_speed, up_speed, ret;
 	unsigned char status;
 
+#ifdef VERBOSE_DEBUG
 	atm_dbg(usbatm, "%s entered\n", __func__);
+#endif
 
 	ret = speedtch_read_status(instance);
 	if (ret < 0) {
@@ -441,9 +444,9 @@
 
 	status = buf[OFFSET_7];
 
-	atm_dbg(usbatm, "%s: line state %02x\n", __func__, status);
-
 	if ((status != instance->last_status) || !status) {
+		atm_dbg(usbatm, "%s: line state 0x%02x\n", __func__, status);
+
 		switch (status) {
 		case 0:
 			atm_dev->signal = ATM_PHY_SIG_LOST;
@@ -484,7 +487,7 @@
 
 		default:
 			atm_dev->signal = ATM_PHY_SIG_UNKNOWN;
-			atm_info(usbatm, "Unknown line state %02x\n", status);
+			atm_info(usbatm, "unknown line state %02x\n", status);
 			break;
 		}
 
@@ -690,8 +693,10 @@
 
 	usb_dbg(usbatm, "%s entered\n", __func__);
 
+	/* sanity checks */
+
 	if (usb_dev->descriptor.bDeviceClass != USB_CLASS_VENDOR_SPEC) {
-		usb_dbg(usbatm, "%s: wrong device class %d\n", __func__, usb_dev->descriptor.bDeviceClass);
+		usb_err(usbatm, "%s: wrong device class %d\n", __func__, usb_dev->descriptor.bDeviceClass);
 		return -ENODEV;
 	}
 
@@ -704,7 +709,7 @@
 			ret = usb_driver_claim_interface(&speedtch_usb_driver, cur_intf, usbatm);
 
 			if (ret < 0) {
-				usb_dbg(usbatm, "%s: failed to claim interface %d (%d)\n", __func__, i, ret);
+				usb_err(usbatm, "%s: failed to claim interface %2d (%d)!\n", __func__, i, ret);
 				speedtch_release_interfaces(usb_dev, i);
 				return ret;
 			}
@@ -714,7 +719,7 @@
 	instance = kmalloc(sizeof(*instance), GFP_KERNEL);
 
 	if (!instance) {
-		usb_dbg(usbatm, "%s: no memory for instance data!\n", __func__);
+		usb_err(usbatm, "%s: no memory for instance data!\n", __func__);
 		ret = -ENOMEM;
 		goto fail_release;
 	}
@@ -754,8 +759,10 @@
 	usb_dbg(usbatm, "%s: firmware %s loaded\n", __func__, need_heavy_init ? "not" : "already");
 
 	if (*need_heavy_init)
-		if ((ret = usb_reset_device(usb_dev)) < 0)
+		if ((ret = usb_reset_device(usb_dev)) < 0) {
+			usb_err(usbatm, "%s: device reset failed (%d)!\n", __func__, ret);
 			goto fail_free;
+		}
 
         usbatm->driver_data = instance;
 
Index: kernel/usbatm.c
===================================================================
--- kernel.orig/usbatm.c	2006-01-10 08:42:02.000000000 +0100
+++ kernel/usbatm.c	2006-01-12 16:08:35.000000000 +0100
@@ -166,10 +166,10 @@
 
 /* ATM */
 
-static void usbatm_atm_dev_close(struct atm_dev *dev);
+static void usbatm_atm_dev_close(struct atm_dev *atm_dev);
 static int usbatm_atm_open(struct atm_vcc *vcc);
 static void usbatm_atm_close(struct atm_vcc *vcc);
-static int usbatm_atm_ioctl(struct atm_dev *dev, unsigned int cmd, void __user * arg);
+static int usbatm_atm_ioctl(struct atm_dev *atm_dev, unsigned int cmd, void __user * arg);
 static int usbatm_atm_send(struct atm_vcc *vcc, struct sk_buff *skb);
 static int usbatm_atm_proc_read(struct atm_dev *atm_dev, loff_t * pos, char *page);
 
@@ -234,8 +234,9 @@
 
 	ret = usb_submit_urb(urb, GFP_ATOMIC);
 	if (ret) {
-		atm_dbg(channel->usbatm, "%s: urb 0x%p submission failed (%d)!\n",
-			__func__, urb, ret);
+		if (printk_ratelimit())
+			atm_warn(channel->usbatm, "%s: urb 0x%p submission failed (%d)!\n",
+				__func__, urb, ret);
 
 		/* consider all errors transient and return the buffer back to the queue */
 		urb->status = -EAGAIN;
@@ -269,10 +270,13 @@
 
 	spin_unlock_irqrestore(&channel->lock, flags);
 
-	if (unlikely(urb->status))
+	if (unlikely(urb->status)) {
+		if (printk_ratelimit())
+			atm_warn(channel->usbatm, "%s: urb 0x%p failed (%d)!\n",
+				__func__, urb, urb->status);
 		/* throttle processing in case of an error */
 		mod_timer(&channel->delay, jiffies + msecs_to_jiffies(THROTTLE_MSECS));
-	else
+	} else
 		tasklet_schedule(&channel->tasklet);
 }
 
@@ -284,11 +288,11 @@
 static inline struct usbatm_vcc_data *usbatm_find_vcc(struct usbatm_data *instance,
 						  short vpi, int vci)
 {
-	struct usbatm_vcc_data *vcc;
+	struct usbatm_vcc_data *vcc_data;
 
-	list_for_each_entry(vcc, &instance->vcc_list, list)
-		if ((vcc->vci == vci) && (vcc->vpi == vpi))
-			return vcc;
+	list_for_each_entry(vcc_data, &instance->vcc_list, list)
+		if ((vcc_data->vci == vci) && (vcc_data->vpi == vpi))
+			return vcc_data;
 	return NULL;
 }
 
@@ -317,7 +321,7 @@
 			cached_vcc = usbatm_find_vcc(instance, vpi, vci);
 
 			if (!cached_vcc)
-				atm_dbg(instance, "%s: unknown vpi/vci (%hd/%d)!\n", __func__, vpi, vci);
+				atm_rldbg(instance, "%s: unknown vpi/vci (%hd/%d)!\n", __func__, vpi, vci);
 		}
 
 		if (!cached_vcc)
@@ -327,7 +331,9 @@
 
 		/* OAM F5 end-to-end */
 		if (pti == ATM_PTI_E2EF5) {
-			atm_warn(instance, "%s: OAM not supported (vpi %d, vci %d)!\n", __func__, vpi, vci);
+			if (printk_ratelimit())
+				atm_warn(instance, "%s: OAM not supported (vpi %d, vci %d)!\n",
+					__func__, vpi, vci);
 			atomic_inc(&vcc->stats->rx_err);
 			continue;
 		}
@@ -335,7 +341,7 @@
 		sarb = cached_vcc->sarb;
 
 		if (sarb->tail + ATM_CELL_PAYLOAD > sarb->end) {
-			atm_dbg(instance, "%s: buffer overrun (sarb->len %u, vcc: 0x%p)!\n",
+			atm_rldbg(instance, "%s: buffer overrun (sarb->len %u, vcc: 0x%p)!\n",
 					__func__, sarb->len, vcc);
 			/* discard cells already received */
 			skb_trim(sarb, 0);
@@ -354,7 +360,7 @@
 
 			/* guard against overflow */
 			if (length > ATM_MAX_AAL5_PDU) {
-				atm_dbg(instance, "%s: bogus length %u (vcc: 0x%p)!\n",
+				atm_rldbg(instance, "%s: bogus length %u (vcc: 0x%p)!\n",
 						__func__, length, vcc);
 				atomic_inc(&vcc->stats->rx_err);
 				goto out;
@@ -363,14 +369,14 @@
 			pdu_length = usbatm_pdu_length(length);
 
 			if (sarb->len < pdu_length) {
-				atm_dbg(instance, "%s: bogus pdu_length %u (sarb->len: %u, vcc: 0x%p)!\n",
+				atm_rldbg(instance, "%s: bogus pdu_length %u (sarb->len: %u, vcc: 0x%p)!\n",
 						__func__, pdu_length, sarb->len, vcc);
 				atomic_inc(&vcc->stats->rx_err);
 				goto out;
 			}
 
 			if (crc32_be(~0, sarb->tail - pdu_length, pdu_length) != 0xc704dd7b) {
-				atm_dbg(instance, "%s: packet failed crc check (vcc: 0x%p)!\n",
+				atm_rldbg(instance, "%s: packet failed crc check (vcc: 0x%p)!\n",
 						__func__, vcc);
 				atomic_inc(&vcc->stats->rx_err);
 				goto out;
@@ -379,7 +385,9 @@
 			vdbg("%s: got packet (length: %u, pdu_length: %u, vcc: 0x%p)", __func__, length, pdu_length, vcc);
 
 			if (!(skb = dev_alloc_skb(length))) {
-				atm_dbg(instance, "%s: no memory for skb (length: %u)!\n", __func__, length);
+				if (printk_ratelimit())
+					atm_err(instance, "%s: no memory for skb (length: %u)!\n",
+							__func__, length);
 				atomic_inc(&vcc->stats->rx_drop);
 				goto out;
 			}
@@ -387,7 +395,8 @@
 			vdbg("%s: allocated new sk_buff (skb: 0x%p, skb->truesize: %u)", __func__, skb, skb->truesize);
 
 			if (!atm_charge(vcc, skb->truesize)) {
-				atm_dbg(instance, "%s: failed atm_charge (skb->truesize: %u)!\n", __func__, skb->truesize);
+				atm_rldbg(instance, "%s: failed atm_charge (skb->truesize: %u)!\n",
+						__func__, skb->truesize);
 				dev_kfree_skb(skb);
 				goto out;	/* atm_charge increments rx_drop */
 			}
@@ -600,13 +609,13 @@
 	}
 
 	if (vcc->qos.aal != ATM_AAL5) {
-		atm_dbg(instance, "%s: unsupported ATM type %d!\n", __func__, vcc->qos.aal);
+		atm_rldbg(instance, "%s: unsupported ATM type %d!\n", __func__, vcc->qos.aal);
 		err = -EINVAL;
 		goto fail;
 	}
 
 	if (skb->len > ATM_MAX_AAL5_PDU) {
-		atm_dbg(instance, "%s: packet too long (%d vs %d)!\n",
+		atm_rldbg(instance, "%s: packet too long (%d vs %d)!\n",
 				__func__, skb->len, ATM_MAX_AAL5_PDU);
 		err = -EINVAL;
 		goto fail;
@@ -665,16 +674,16 @@
 **  ATM  **
 **********/
 
-static void usbatm_atm_dev_close(struct atm_dev *dev)
+static void usbatm_atm_dev_close(struct atm_dev *atm_dev)
 {
-	struct usbatm_data *instance = dev->dev_data;
+	struct usbatm_data *instance = atm_dev->dev_data;
 
 	dbg("%s", __func__);
 
 	if (!instance)
 		return;
 
-	dev->dev_data = NULL;
+	atm_dev->dev_data = NULL; /* catch bugs */
 	usbatm_put_instance(instance);	/* taken in usbatm_atm_init */
 }
 
@@ -735,13 +744,18 @@
 	atm_dbg(instance, "%s: vpi %hd, vci %d\n", __func__, vpi, vci);
 
 	/* only support AAL5 */
-	if ((vcc->qos.aal != ATM_AAL5) || (vcc->qos.rxtp.max_sdu < 0)
-	    || (vcc->qos.rxtp.max_sdu > ATM_MAX_AAL5_PDU)) {
-		atm_dbg(instance, "%s: unsupported ATM type %d!\n", __func__, vcc->qos.aal);
+	if ((vcc->qos.aal != ATM_AAL5)) {
+		atm_warn(instance, "%s: unsupported ATM type %d!\n", __func__, vcc->qos.aal);
+		return -EINVAL;
+	}
+
+	/* sanity checks */
+	if ((vcc->qos.rxtp.max_sdu < 0) || (vcc->qos.rxtp.max_sdu > ATM_MAX_AAL5_PDU)) {
+		atm_dbg(instance, "%s: max_sdu %d out of range!\n", __func__, vcc->qos.rxtp.max_sdu);
 		return -EINVAL;
 	}
 
-	down(&instance->serialize);	/* vs self, usbatm_atm_close */
+	down(&instance->serialize);	/* vs self, usbatm_atm_close, usbatm_usb_disconnect */
 
 	if (usbatm_find_vcc(instance, vpi, vci)) {
 		atm_dbg(instance, "%s: %hd/%d already in use!\n", __func__, vpi, vci);
@@ -750,7 +764,7 @@
 	}
 
 	if (!(new = kmalloc(sizeof(struct usbatm_vcc_data), GFP_KERNEL))) {
-		atm_dbg(instance, "%s: no memory for vcc_data!\n", __func__);
+		atm_err(instance, "%s: no memory for vcc_data!\n", __func__);
 		ret = -ENOMEM;
 		goto fail;
 	}
@@ -762,7 +776,7 @@
 
 	new->sarb = alloc_skb(usbatm_pdu_length(vcc->qos.rxtp.max_sdu), GFP_KERNEL);
 	if (!new->sarb) {
-		atm_dbg(instance, "%s: no memory for SAR buffer!\n", __func__);
+		atm_err(instance, "%s: no memory for SAR buffer!\n", __func__);
 		ret = -ENOMEM;
 		goto fail;
 	}
@@ -806,7 +820,7 @@
 
 	usbatm_cancel_send(instance, vcc);
 
-	down(&instance->serialize);	/* vs self, usbatm_atm_open */
+	down(&instance->serialize);	/* vs self, usbatm_atm_open, usbatm_usb_disconnect */
 
 	tasklet_disable(&instance->rx_channel.tasklet);
 	list_del(&vcc_data->list);
@@ -829,7 +843,7 @@
 	atm_dbg(instance, "%s successful\n", __func__);
 }
 
-static int usbatm_atm_ioctl(struct atm_dev *dev, unsigned int cmd,
+static int usbatm_atm_ioctl(struct atm_dev *atm_dev, unsigned int cmd,
 			  void __user * arg)
 {
 	switch (cmd) {
@@ -845,10 +859,13 @@
 	struct atm_dev *atm_dev;
 	int ret, i;
 
-	/* ATM init */
+	/* ATM init.  The ATM initialization scheme suffers from an intrinsic race
+	 * condition: callbacks we register can be executed at once, before we have
+	 * initialized the struct atm_dev.  To protect against this, all callbacks
+	 * abort if atm_dev->dev_data is NULL. */
 	atm_dev = atm_dev_register(instance->driver_name, &usbatm_atm_devops, -1, NULL);
 	if (!atm_dev) {
-		usb_dbg(instance, "%s: failed to register ATM device!\n", __func__);
+		usb_err(instance, "%s: failed to register ATM device!\n", __func__);
 		return -1;
 	}
 
@@ -862,12 +879,13 @@
 	atm_dev->link_rate = 128 * 1000 / 424;
 
 	if (instance->driver->atm_start && ((ret = instance->driver->atm_start(instance, atm_dev)) < 0)) {
-		atm_dbg(instance, "%s: atm_start failed: %d!\n", __func__, ret);
+		atm_err(instance, "%s: atm_start failed: %d!\n", __func__, ret);
 		goto fail;
 	}
 
-	/* ready for ATM callbacks */
 	usbatm_get_instance(instance);	/* dropped in usbatm_atm_dev_close */
+
+	/* ready for ATM callbacks */
 	mb();
 	atm_dev->dev_data = instance;
 
@@ -915,7 +933,7 @@
 	int ret = kernel_thread(usbatm_do_heavy_init, instance, CLONE_KERNEL);
 
 	if (ret < 0) {
-		usb_dbg(instance, "%s: failed to create kernel_thread (%d)!\n", __func__, ret);
+		usb_err(instance, "%s: failed to create kernel_thread (%d)!\n", __func__, ret);
 		return ret;
 	}
 
@@ -953,7 +971,7 @@
 	int i, length;
 	int need_heavy;
 
-	dev_dbg(dev, "%s: trying driver %s with vendor=0x%x, product=0x%x, ifnum %d\n",
+	dev_dbg(dev, "%s: trying driver %s with vendor=%04x, product=%04x, ifnum %2d\n",
 			__func__, driver->driver_name,
 			le16_to_cpu(usb_dev->descriptor.idVendor),
 			le16_to_cpu(usb_dev->descriptor.idProduct),
@@ -962,7 +980,7 @@
 	/* instance init */
 	instance = kzalloc(sizeof(*instance) + sizeof(struct urb *) * (num_rcv_urbs + num_snd_urbs), GFP_KERNEL);
 	if (!instance) {
-		dev_dbg(dev, "%s: no memory for instance data!\n", __func__);
+		dev_err(dev, "%s: no memory for instance data!\n", __func__);
 		return -ENOMEM;
 	}
 
@@ -998,7 +1016,7 @@
  bind:
 	need_heavy = 1;
 	if (driver->bind && (error = driver->bind(instance, intf, id, &need_heavy)) < 0) {
-			dev_dbg(dev, "%s: bind failed: %d!\n", __func__, error);
+			dev_err(dev, "%s: bind failed: %d!\n", __func__, error);
 			goto fail_free;
 	}
 
@@ -1044,7 +1062,7 @@
 
 		urb = usb_alloc_urb(iso_packets, GFP_KERNEL);
 		if (!urb) {
-			dev_dbg(dev, "%s: no memory for urb %d!\n", __func__, i);
+			dev_err(dev, "%s: no memory for urb %d!\n", __func__, i);
 			goto fail_unbind;
 		}
 
@@ -1052,9 +1070,10 @@
 
 		buffer = kmalloc(channel->buf_size, GFP_KERNEL);
 		if (!buffer) {
-			dev_dbg(dev, "%s: no memory for buffer %d!\n", __func__, i);
+			dev_err(dev, "%s: no memory for buffer %d!\n", __func__, i);
 			goto fail_unbind;
 		}
+		/* zero the tx padding to avoid leaking information */
 		memset(buffer, 0, channel->buf_size);
 
 		usb_fill_bulk_urb(urb, instance->usb_dev, channel->endpoint,
Index: kernel/usbatm.h
===================================================================
--- kernel.orig/usbatm.h	2006-01-04 21:43:13.000000000 +0100
+++ kernel/usbatm.h	2006-01-12 16:08:47.000000000 +0100
@@ -24,22 +24,21 @@
 #ifndef	_USBATM_H_
 #define	_USBATM_H_
 
-#include <linux/config.h>
-
-/*
-#define VERBOSE_DEBUG
-*/
-
 #include <asm/semaphore.h>
 #include <linux/atm.h>
 #include <linux/atmdev.h>
 #include <linux/completion.h>
 #include <linux/device.h>
+#include <linux/kernel.h>
 #include <linux/kref.h>
 #include <linux/list.h>
 #include <linux/stringify.h>
 #include <linux/usb.h>
 
+/*
+#define VERBOSE_DEBUG
+*/
+
 #ifdef DEBUG
 #define UDSL_ASSERT(x)	BUG_ON(!(x))
 #else
@@ -52,8 +51,13 @@
 	dev_info(&(instance)->usb_intf->dev , format , ## arg)
 #define usb_warn(instance, format, arg...)	\
 	dev_warn(&(instance)->usb_intf->dev , format , ## arg)
+#ifdef DEBUG
+#define usb_dbg(instance, format, arg...)	\
+        dev_printk(KERN_DEBUG , &(instance)->usb_intf->dev , format , ## arg)
+#else
 #define usb_dbg(instance, format, arg...)	\
-	dev_dbg(&(instance)->usb_intf->dev , format , ## arg)
+	do {} while (0)
+#endif
 
 /* FIXME: move to dev_* once ATM is driver model aware */
 #define atm_printk(level, instance, format, arg...)	\
@@ -69,9 +73,14 @@
 #ifdef DEBUG
 #define atm_dbg(instance, format, arg...)	\
 	atm_printk(KERN_DEBUG, instance , format , ## arg)
+#define atm_rldbg(instance, format, arg...)	\
+	if (printk_ratelimit())				\
+		atm_printk(KERN_DEBUG, instance , format , ## arg)
 #else
 #define atm_dbg(instance, format, arg...)	\
 	do {} while (0)
+#define atm_rldbg(instance, format, arg...)	\
+	do {} while (0)
 #endif
 
 
@@ -171,7 +180,7 @@
 	struct usbatm_channel tx_channel;
 
 	struct sk_buff_head sndqueue;
-	struct sk_buff *current_skb;			/* being emptied */
+	struct sk_buff *current_skb;	/* being emptied */
 
 	struct urb *urbs[0];
 };
Index: kernel/xusbatm.c
===================================================================
--- kernel.orig/xusbatm.c	2006-01-10 08:42:02.000000000 +0100
+++ kernel/xusbatm.c	2006-01-12 16:08:35.000000000 +0100
@@ -61,7 +61,7 @@
 	return 0;
 }
 
-static int xusbatm_bind(struct usbatm_data *usbatm_instance,
+static int xusbatm_bind(struct usbatm_data *usbatm,
 			struct usb_interface *intf, const struct usb_device_id *id,
 			int *need_heavy_init)
 {
@@ -72,14 +72,14 @@
 	u8 searched_ep = rx_ep_present ? tx_endpoint[drv_ix] : rx_endpoint[drv_ix];
 	int i, ret;
 
-	usb_dbg(usbatm_instance, "%s: binding driver %d: vendor %#x product %#x"
-		" rx: ep %#x padd %d tx: ep %#x padd %d\n",
+	usb_dbg(usbatm, "%s: binding driver %d: vendor %04x product %04x"
+		" rx: ep %02x padd %d tx: ep %02x padd %d\n",
 		__func__, drv_ix, vendor[drv_ix], product[drv_ix],
 		rx_endpoint[drv_ix], rx_padding[drv_ix],
 		tx_endpoint[drv_ix], tx_padding[drv_ix]);
 
 	if (!rx_ep_present && !tx_ep_present) {
-		usb_dbg(usbatm_instance, "%s: intf #%d has neither rx (%#x) nor tx (%#x) endpoint\n",
+		usb_dbg(usbatm, "%s: intf #%d has neither rx (%#x) nor tx (%#x) endpoint\n",
 			__func__, intf->altsetting->desc.bInterfaceNumber,
 			rx_endpoint[drv_ix], tx_endpoint[drv_ix]);
 		return -ENODEV;
@@ -93,25 +93,26 @@
 
 		if (cur_if != intf && usb_intf_has_ep(cur_if, searched_ep)) {
 			ret = usb_driver_claim_interface(&xusbatm_usb_driver,
-							 cur_if, usbatm_instance);
+							 cur_if, usbatm);
 			if (!ret)
-				usb_err(usbatm_instance, "%s: failed to claim interface #%d (%d)\n",
+				usb_err(usbatm, "%s: failed to claim interface #%d (%d)\n",
 					__func__, cur_if->altsetting->desc.bInterfaceNumber, ret);
 			return ret;
 		}
 	}
 
-	usb_err(usbatm_instance, "%s: no interface has endpoint %#x\n",
+	usb_err(usbatm, "%s: no interface has endpoint %#x\n",
 		__func__, searched_ep);
 	return -ENODEV;
 }
 
-static void xusbatm_unbind(struct usbatm_data *usbatm_instance,
+static void xusbatm_unbind(struct usbatm_data *usbatm,
 			   struct usb_interface *intf)
 {
 	struct usb_device *usb_dev = interface_to_usbdev(intf);
 	int i;
-	usb_dbg(usbatm_instance, "%s entered\n", __func__);
+
+	usb_dbg(usbatm, "%s entered\n", __func__);
 
 	for(i = 0; i < usb_dev->actconfig->desc.bNumInterfaces; i++) {
 		struct usb_interface *cur_if = usb_dev->actconfig->interface[i];
@@ -120,10 +121,10 @@
 	}
 }
 
-static int xusbatm_atm_start(struct usbatm_data *usbatm_instance,
+static int xusbatm_atm_start(struct usbatm_data *usbatm,
 			     struct atm_dev *atm_dev)
 {
-	atm_dbg(usbatm_instance, "%s entered\n", __func__);
+	atm_dbg(usbatm, "%s entered\n", __func__);
 
 	/* use random MAC as we've no way to get it from the device */
 	random_ether_addr(atm_dev->esi);

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 01/13] USBATM: trivial modifications
  2006-01-12 16:43 ` [PATCH 01/13] USBATM: trivial modifications Duncan Sands
@ 2006-01-12 17:40   ` Duncan Sands
  0 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-12 17:40 UTC (permalink / raw)
  To: usbatm; +Cc: Greg KH, linux-kernel, linux-usb-devel

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

Sorry, it wasn't a -p1 patch (I should really automate this).

Signed-off-by:	Duncan Sands <baldrick@free.fr>

[-- Attachment #2: trivial --]
[-- Type: text/x-diff, Size: 30382 bytes --]

diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/cxacru.c Linux/drivers/usb/atm/cxacru.c
--- Linux/drivers/usb/atm.orig/cxacru.c	2006-01-12 18:27:56.000000000 +0100
+++ Linux/drivers/usb/atm/cxacru.c	2006-01-12 18:25:54.000000000 +0100
@@ -352,7 +352,6 @@
 		struct atm_dev *atm_dev)
 {
 	struct cxacru_data *instance = usbatm_instance->driver_data;
-	struct device *dev = &usbatm_instance->usb_intf->dev;
 	/*
 	struct atm_dev *atm_dev = usbatm_instance->atm_dev;
 	*/
@@ -364,14 +363,14 @@
 	ret = cxacru_cm(instance, CM_REQUEST_CARD_GET_MAC_ADDRESS, NULL, 0,
 			atm_dev->esi, sizeof(atm_dev->esi));
 	if (ret < 0) {
-		dev_err(dev, "cxacru_atm_start: CARD_GET_MAC_ADDRESS returned %d\n", ret);
+		atm_err(usbatm_instance, "cxacru_atm_start: CARD_GET_MAC_ADDRESS returned %d\n", ret);
 		return ret;
 	}
 
 	/* start ADSL */
 	ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0);
 	if (ret < 0) {
-		dev_err(dev, "cxacru_atm_start: CHIP_ADSL_LINE_START returned %d\n", ret);
+		atm_err(usbatm_instance, "cxacru_atm_start: CHIP_ADSL_LINE_START returned %d\n", ret);
 		return ret;
 	}
 
@@ -383,13 +382,13 @@
 static void cxacru_poll_status(struct cxacru_data *instance)
 {
 	u32 buf[CXINF_MAX] = {};
-	struct device *dev = &instance->usbatm->usb_intf->dev;
-	struct atm_dev *atm_dev = instance->usbatm->atm_dev;
+	struct usbatm_data *usbatm = instance->usbatm;
+	struct atm_dev *atm_dev = usbatm->atm_dev;
 	int ret;
 
 	ret = cxacru_cm_get_array(instance, CM_REQUEST_CARD_INFO_GET, buf, CXINF_MAX);
 	if (ret < 0) {
-		dev_warn(dev, "poll status: error %d\n", ret);
+		atm_warn(usbatm, "poll status: error %d\n", ret);
 		goto reschedule;
 	}
 
@@ -400,50 +399,50 @@
 	switch (instance->line_status) {
 	case 0:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: down\n");
+		atm_info(usbatm, "ADSL line: down\n");
 		break;
 
 	case 1:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: attemtping to activate\n");
+		atm_info(usbatm, "ADSL line: attempting to activate\n");
 		break;
 
 	case 2:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: training\n");
+		atm_info(usbatm, "ADSL line: training\n");
 		break;
 
 	case 3:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: channel analysis\n");
+		atm_info(usbatm, "ADSL line: channel analysis\n");
 		break;
 
 	case 4:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: exchange\n");
+		atm_info(usbatm, "ADSL line: exchange\n");
 		break;
 
 	case 5:
 		atm_dev->link_rate = buf[CXINF_DOWNSTREAM_RATE] * 1000 / 424;
 		atm_dev->signal = ATM_PHY_SIG_FOUND;
 
-		dev_info(dev, "ADSL line: up (%d kb/s down | %d kb/s up)\n",
+		atm_info(usbatm, "ADSL line: up (%d kb/s down | %d kb/s up)\n",
 		     buf[CXINF_DOWNSTREAM_RATE], buf[CXINF_UPSTREAM_RATE]);
 		break;
 
 	case 6:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: waiting\n");
+		atm_info(usbatm, "ADSL line: waiting\n");
 		break;
 
 	case 7:
 		atm_dev->signal = ATM_PHY_SIG_LOST;
-		dev_info(dev, "ADSL line: initializing\n");
+		atm_info(usbatm, "ADSL line: initializing\n");
 		break;
 
 	default:
 		atm_dev->signal = ATM_PHY_SIG_UNKNOWN;
-		dev_info(dev, "Unknown line state %02x\n", instance->line_status);
+		atm_info(usbatm, "Unknown line state %02x\n", instance->line_status);
 		break;
 	}
 reschedule:
@@ -504,8 +503,8 @@
 {
 	int ret;
 	int off;
-	struct usb_device *usb_dev = instance->usbatm->usb_dev;
-	struct device *dev = &instance->usbatm->usb_intf->dev;
+	struct usbatm_data *usbatm = instance->usbatm;
+	struct usb_device *usb_dev = usbatm->usb_dev;
 	u16 signature[] = { usb_dev->descriptor.idVendor, usb_dev->descriptor.idProduct };
 	u32 val;
 
@@ -515,7 +514,7 @@
 	val = cpu_to_le32(instance->modem_type->pll_f_clk);
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, PLLFCLK_ADDR, (u8 *) &val, 4);
 	if (ret) {
-		dev_err(dev, "FirmwarePllFClkValue failed: %d\n", ret);
+		usb_err(usbatm, "FirmwarePllFClkValue failed: %d\n", ret);
 		return;
 	}
 
@@ -523,7 +522,7 @@
 	val = cpu_to_le32(instance->modem_type->pll_b_clk);
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, PLLBCLK_ADDR, (u8 *) &val, 4);
 	if (ret) {
-		dev_err(dev, "FirmwarePllBClkValue failed: %d\n", ret);
+		usb_err(usbatm, "FirmwarePllBClkValue failed: %d\n", ret);
 		return;
 	}
 
@@ -531,14 +530,14 @@
 	val = cpu_to_le32(SDRAM_ENA);
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, SDRAMEN_ADDR, (u8 *) &val, 4);
 	if (ret) {
-		dev_err(dev, "Enable SDRAM failed: %d\n", ret);
+		usb_err(usbatm, "Enable SDRAM failed: %d\n", ret);
 		return;
 	}
 
 	/* Firmware */
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, FW_ADDR, fw->data, fw->size);
 	if (ret) {
-		dev_err(dev, "Firmware upload failed: %d\n", ret);
+		usb_err(usbatm, "Firmware upload failed: %d\n", ret);
 		return;
 	}
 
@@ -546,7 +545,7 @@
 	if (instance->modem_type->boot_rom_patch) {
 		ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, BR_ADDR, bp->data, bp->size);
 		if (ret) {
-			dev_err(dev, "Boot ROM patching failed: %d\n", ret);
+			usb_err(usbatm, "Boot ROM patching failed: %d\n", ret);
 			return;
 		}
 	}
@@ -554,7 +553,7 @@
 	/* Signature */
 	ret = cxacru_fw(usb_dev, FW_WRITE_MEM, 0x2, 0x0, SIG_ADDR, (u8 *) signature, 4);
 	if (ret) {
-		dev_err(dev, "Signature storing failed: %d\n", ret);
+		usb_err(usbatm, "Signature storing failed: %d\n", ret);
 		return;
 	}
 
@@ -566,7 +565,7 @@
 		ret = cxacru_fw(usb_dev, FW_GOTO_MEM, 0x0, 0x0, FW_ADDR, NULL, 0);
 	}
 	if (ret) {
-		dev_err(dev, "Passing control to firmware failed: %d\n", ret);
+		usb_err(usbatm, "Passing control to firmware failed: %d\n", ret);
 		return;
 	}
 
@@ -580,7 +579,7 @@
 
 	ret = cxacru_cm(instance, CM_REQUEST_CARD_GET_STATUS, NULL, 0, NULL, 0);
 	if (ret < 0) {
-		dev_err(dev, "modem failed to initialize: %d\n", ret);
+		usb_err(usbatm, "modem failed to initialize: %d\n", ret);
 		return;
 	}
 
@@ -597,7 +596,7 @@
 			ret = cxacru_cm(instance, CM_REQUEST_CARD_DATA_SET,
 					(u8 *) buf, len, NULL, 0);
 			if (ret < 0) {
-				dev_err(dev, "load config data failed: %d\n", ret);
+				usb_err(usbatm, "load config data failed: %d\n", ret);
 				return;
 			}
 		}
@@ -608,18 +607,19 @@
 static int cxacru_find_firmware(struct cxacru_data *instance,
 				char* phase, const struct firmware **fw_p)
 {
-	struct device *dev = &instance->usbatm->usb_intf->dev;
+	struct usbatm_data *usbatm = instance->usbatm;
+	struct device *dev = &usbatm->usb_intf->dev;
 	char buf[16];
 
 	sprintf(buf, "cxacru-%s.bin", phase);
 	dbg("cxacru_find_firmware: looking for %s", buf);
 
 	if (request_firmware(fw_p, buf, dev)) {
-		dev_dbg(dev, "no stage %s firmware found\n", phase);
+		usb_dbg(usbatm, "no stage %s firmware found\n", phase);
 		return -ENOENT;
 	}
 
-	dev_info(dev, "found firmware %s\n", buf);
+	usb_info(usbatm, "found firmware %s\n", buf);
 
 	return 0;
 }
@@ -627,20 +627,19 @@
 static int cxacru_heavy_init(struct usbatm_data *usbatm_instance,
 			     struct usb_interface *usb_intf)
 {
-	struct device *dev = &usbatm_instance->usb_intf->dev;
 	const struct firmware *fw, *bp, *cf;
 	struct cxacru_data *instance = usbatm_instance->driver_data;
 
 	int ret = cxacru_find_firmware(instance, "fw", &fw);
 	if (ret) {
-		dev_warn(dev, "firmware (cxacru-fw.bin) unavailable (hotplug misconfiguration?)\n");
+		usb_warn(usbatm_instance, "firmware (cxacru-fw.bin) unavailable (system misconfigured?)\n");
 		return ret;
 	}
 
 	if (instance->modem_type->boot_rom_patch) {
 		ret = cxacru_find_firmware(instance, "bp", &bp);
 		if (ret) {
-			dev_warn(dev, "boot ROM patch (cxacru-bp.bin) unavailable (hotplug misconfiguration?)\n");
+			usb_warn(usbatm_instance, "boot ROM patch (cxacru-bp.bin) unavailable (system misconfigured?)\n");
 			release_firmware(fw);
 			return ret;
 		}
@@ -787,12 +786,12 @@
 	{ /* V = Conexant			P = ADSL modem (Hasbani project)	*/
 		USB_DEVICE(0x0572, 0xcb00),	.driver_info = (unsigned long) &cxacru_cb00
 	},
-	{ /* V = Conexant             P = ADSL modem (Well PTI-800 */
-		USB_DEVICE(0x0572, 0xcb02),	.driver_info = (unsigned long) &cxacru_cb00
-	},
 	{ /* V = Conexant			P = ADSL modem				*/
 		USB_DEVICE(0x0572, 0xcb01),	.driver_info = (unsigned long) &cxacru_cb00
 	},
+	{ /* V = Conexant			P = ADSL modem (Well PTI-800) */
+		USB_DEVICE(0x0572, 0xcb02),	.driver_info = (unsigned long) &cxacru_cb00
+	},
 	{ /* V = Conexant			P = ADSL modem				*/
 		USB_DEVICE(0x0572, 0xcb06),	.driver_info = (unsigned long) &cxacru_cb00
 	},
diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/speedtch.c Linux/drivers/usb/atm/speedtch.c
--- Linux/drivers/usb/atm.orig/speedtch.c	2006-01-12 18:27:56.000000000 +0100
+++ Linux/drivers/usb/atm/speedtch.c	2006-01-12 18:25:54.000000000 +0100
@@ -205,7 +205,7 @@
 				   buffer, 0x200, &actual_length, 2000);
 
 		if (ret < 0 && ret != -ETIMEDOUT)
-			usb_dbg(usbatm, "%s: read BLOCK0 from modem failed (%d)!\n", __func__, ret);
+			usb_warn(usbatm, "%s: read BLOCK0 from modem failed (%d)!\n", __func__, ret);
 		else
 			usb_dbg(usbatm, "%s: BLOCK0 downloaded (%d bytes)\n", __func__, ret);
 	}
@@ -219,7 +219,7 @@
 				   buffer, thislen, &actual_length, DATA_TIMEOUT);
 
 		if (ret < 0) {
-			usb_dbg(usbatm, "%s: write BLOCK1 to modem failed (%d)!\n", __func__, ret);
+			usb_err(usbatm, "%s: write BLOCK1 to modem failed (%d)!\n", __func__, ret);
 			goto out_free;
 		}
 		usb_dbg(usbatm, "%s: BLOCK1 uploaded (%zu bytes)\n", __func__, fw1->size);
@@ -232,7 +232,7 @@
 			   buffer, 0x200, &actual_length, DATA_TIMEOUT);
 
 	if (ret < 0) {
-		usb_dbg(usbatm, "%s: read BLOCK2 from modem failed (%d)!\n", __func__, ret);
+		usb_err(usbatm, "%s: read BLOCK2 from modem failed (%d)!\n", __func__, ret);
 		goto out_free;
 	}
 	usb_dbg(usbatm, "%s: BLOCK2 downloaded (%d bytes)\n", __func__, actual_length);
@@ -246,7 +246,7 @@
 				   buffer, thislen, &actual_length, DATA_TIMEOUT);
 
 		if (ret < 0) {
-			usb_dbg(usbatm, "%s: write BLOCK3 to modem failed (%d)!\n", __func__, ret);
+			usb_err(usbatm, "%s: write BLOCK3 to modem failed (%d)!\n", __func__, ret);
 			goto out_free;
 		}
 	}
@@ -259,7 +259,7 @@
 			   buffer, 0x200, &actual_length, DATA_TIMEOUT);
 
 	if (ret < 0) {
-		usb_dbg(usbatm, "%s: read BLOCK4 from modem failed (%d)!\n", __func__, ret);
+		usb_err(usbatm, "%s: read BLOCK4 from modem failed (%d)!\n", __func__, ret);
 		goto out_free;
 	}
 
@@ -285,8 +285,8 @@
 	return ret;
 }
 
-static int speedtch_find_firmware(struct usb_interface *intf, int phase,
-				  const struct firmware **fw_p)
+static int speedtch_find_firmware(struct usbatm_data *usbatm, struct usb_interface *intf,
+				  int phase, const struct firmware **fw_p)
 {
 	struct device *dev = &intf->dev;
 	const u16 bcdDevice = le16_to_cpu(interface_to_usbdev(intf)->descriptor.bcdDevice);
@@ -295,24 +295,24 @@
 	char buf[24];
 
 	sprintf(buf, "speedtch-%d.bin.%x.%02x", phase, major_revision, minor_revision);
-	dev_dbg(dev, "%s: looking for %s\n", __func__, buf);
+	usb_dbg(usbatm, "%s: looking for %s\n", __func__, buf);
 
 	if (request_firmware(fw_p, buf, dev)) {
 		sprintf(buf, "speedtch-%d.bin.%x", phase, major_revision);
-		dev_dbg(dev, "%s: looking for %s\n", __func__, buf);
+		usb_dbg(usbatm, "%s: looking for %s\n", __func__, buf);
 
 		if (request_firmware(fw_p, buf, dev)) {
 			sprintf(buf, "speedtch-%d.bin", phase);
-			dev_dbg(dev, "%s: looking for %s\n", __func__, buf);
+			usb_dbg(usbatm, "%s: looking for %s\n", __func__, buf);
 
 			if (request_firmware(fw_p, buf, dev)) {
-				dev_warn(dev, "no stage %d firmware found!\n", phase);
+				usb_err(usbatm, "%s: no stage %d firmware found!\n", __func__, phase);
 				return -ENOENT;
 			}
 		}
 	}
 
-	dev_info(dev, "found stage %d firmware %s\n", phase, buf);
+	usb_info(usbatm, "found stage %d firmware %s\n", phase, buf);
 
 	return 0;
 }
@@ -323,15 +323,16 @@
 	struct speedtch_instance_data *instance = usbatm->driver_data;
 	int ret;
 
-	if ((ret = speedtch_find_firmware(intf, 1, &fw1)) < 0)
-			return ret;
+	if ((ret = speedtch_find_firmware(usbatm, intf, 1, &fw1)) < 0)
+		return ret;
 
-	if ((ret = speedtch_find_firmware(intf, 2, &fw2)) < 0) {
+	if ((ret = speedtch_find_firmware(usbatm, intf, 2, &fw2)) < 0) {
 		release_firmware(fw1);
 		return ret;
 	}
 
-	ret = speedtch_upload_firmware(instance, fw1, fw2);
+	if ((ret = speedtch_upload_firmware(instance, fw1, fw2)) < 0)
+		usb_err(usbatm, "%s: firmware upload failed (%d)!\n", __func__, ret);
 
 	release_firmware(fw2);
 	release_firmware(fw1);
@@ -428,7 +429,9 @@
 	int down_speed, up_speed, ret;
 	unsigned char status;
 
+#ifdef VERBOSE_DEBUG
 	atm_dbg(usbatm, "%s entered\n", __func__);
+#endif
 
 	ret = speedtch_read_status(instance);
 	if (ret < 0) {
@@ -441,9 +444,9 @@
 
 	status = buf[OFFSET_7];
 
-	atm_dbg(usbatm, "%s: line state %02x\n", __func__, status);
-
 	if ((status != instance->last_status) || !status) {
+		atm_dbg(usbatm, "%s: line state 0x%02x\n", __func__, status);
+
 		switch (status) {
 		case 0:
 			atm_dev->signal = ATM_PHY_SIG_LOST;
@@ -484,7 +487,7 @@
 
 		default:
 			atm_dev->signal = ATM_PHY_SIG_UNKNOWN;
-			atm_info(usbatm, "Unknown line state %02x\n", status);
+			atm_info(usbatm, "unknown line state %02x\n", status);
 			break;
 		}
 
@@ -690,8 +693,10 @@
 
 	usb_dbg(usbatm, "%s entered\n", __func__);
 
+	/* sanity checks */
+
 	if (usb_dev->descriptor.bDeviceClass != USB_CLASS_VENDOR_SPEC) {
-		usb_dbg(usbatm, "%s: wrong device class %d\n", __func__, usb_dev->descriptor.bDeviceClass);
+		usb_err(usbatm, "%s: wrong device class %d\n", __func__, usb_dev->descriptor.bDeviceClass);
 		return -ENODEV;
 	}
 
@@ -704,7 +709,7 @@
 			ret = usb_driver_claim_interface(&speedtch_usb_driver, cur_intf, usbatm);
 
 			if (ret < 0) {
-				usb_dbg(usbatm, "%s: failed to claim interface %d (%d)\n", __func__, i, ret);
+				usb_err(usbatm, "%s: failed to claim interface %2d (%d)!\n", __func__, i, ret);
 				speedtch_release_interfaces(usb_dev, i);
 				return ret;
 			}
@@ -714,7 +719,7 @@
 	instance = kmalloc(sizeof(*instance), GFP_KERNEL);
 
 	if (!instance) {
-		usb_dbg(usbatm, "%s: no memory for instance data!\n", __func__);
+		usb_err(usbatm, "%s: no memory for instance data!\n", __func__);
 		ret = -ENOMEM;
 		goto fail_release;
 	}
@@ -754,8 +759,10 @@
 	usb_dbg(usbatm, "%s: firmware %s loaded\n", __func__, need_heavy_init ? "not" : "already");
 
 	if (*need_heavy_init)
-		if ((ret = usb_reset_device(usb_dev)) < 0)
+		if ((ret = usb_reset_device(usb_dev)) < 0) {
+			usb_err(usbatm, "%s: device reset failed (%d)!\n", __func__, ret);
 			goto fail_free;
+		}
 
         usbatm->driver_data = instance;
 
diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/usbatm.c Linux/drivers/usb/atm/usbatm.c
--- Linux/drivers/usb/atm.orig/usbatm.c	2006-01-12 18:27:56.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-12 18:25:54.000000000 +0100
@@ -166,10 +166,10 @@
 
 /* ATM */
 
-static void usbatm_atm_dev_close(struct atm_dev *dev);
+static void usbatm_atm_dev_close(struct atm_dev *atm_dev);
 static int usbatm_atm_open(struct atm_vcc *vcc);
 static void usbatm_atm_close(struct atm_vcc *vcc);
-static int usbatm_atm_ioctl(struct atm_dev *dev, unsigned int cmd, void __user * arg);
+static int usbatm_atm_ioctl(struct atm_dev *atm_dev, unsigned int cmd, void __user * arg);
 static int usbatm_atm_send(struct atm_vcc *vcc, struct sk_buff *skb);
 static int usbatm_atm_proc_read(struct atm_dev *atm_dev, loff_t * pos, char *page);
 
@@ -234,8 +234,9 @@
 
 	ret = usb_submit_urb(urb, GFP_ATOMIC);
 	if (ret) {
-		atm_dbg(channel->usbatm, "%s: urb 0x%p submission failed (%d)!\n",
-			__func__, urb, ret);
+		if (printk_ratelimit())
+			atm_warn(channel->usbatm, "%s: urb 0x%p submission failed (%d)!\n",
+				__func__, urb, ret);
 
 		/* consider all errors transient and return the buffer back to the queue */
 		urb->status = -EAGAIN;
@@ -269,10 +270,13 @@
 
 	spin_unlock_irqrestore(&channel->lock, flags);
 
-	if (unlikely(urb->status))
+	if (unlikely(urb->status)) {
+		if (printk_ratelimit())
+			atm_warn(channel->usbatm, "%s: urb 0x%p failed (%d)!\n",
+				__func__, urb, urb->status);
 		/* throttle processing in case of an error */
 		mod_timer(&channel->delay, jiffies + msecs_to_jiffies(THROTTLE_MSECS));
-	else
+	} else
 		tasklet_schedule(&channel->tasklet);
 }
 
@@ -284,11 +288,11 @@
 static inline struct usbatm_vcc_data *usbatm_find_vcc(struct usbatm_data *instance,
 						  short vpi, int vci)
 {
-	struct usbatm_vcc_data *vcc;
+	struct usbatm_vcc_data *vcc_data;
 
-	list_for_each_entry(vcc, &instance->vcc_list, list)
-		if ((vcc->vci == vci) && (vcc->vpi == vpi))
-			return vcc;
+	list_for_each_entry(vcc_data, &instance->vcc_list, list)
+		if ((vcc_data->vci == vci) && (vcc_data->vpi == vpi))
+			return vcc_data;
 	return NULL;
 }
 
@@ -317,7 +321,7 @@
 			cached_vcc = usbatm_find_vcc(instance, vpi, vci);
 
 			if (!cached_vcc)
-				atm_dbg(instance, "%s: unknown vpi/vci (%hd/%d)!\n", __func__, vpi, vci);
+				atm_rldbg(instance, "%s: unknown vpi/vci (%hd/%d)!\n", __func__, vpi, vci);
 		}
 
 		if (!cached_vcc)
@@ -327,7 +331,9 @@
 
 		/* OAM F5 end-to-end */
 		if (pti == ATM_PTI_E2EF5) {
-			atm_warn(instance, "%s: OAM not supported (vpi %d, vci %d)!\n", __func__, vpi, vci);
+			if (printk_ratelimit())
+				atm_warn(instance, "%s: OAM not supported (vpi %d, vci %d)!\n",
+					__func__, vpi, vci);
 			atomic_inc(&vcc->stats->rx_err);
 			continue;
 		}
@@ -335,7 +341,7 @@
 		sarb = cached_vcc->sarb;
 
 		if (sarb->tail + ATM_CELL_PAYLOAD > sarb->end) {
-			atm_dbg(instance, "%s: buffer overrun (sarb->len %u, vcc: 0x%p)!\n",
+			atm_rldbg(instance, "%s: buffer overrun (sarb->len %u, vcc: 0x%p)!\n",
 					__func__, sarb->len, vcc);
 			/* discard cells already received */
 			skb_trim(sarb, 0);
@@ -354,7 +360,7 @@
 
 			/* guard against overflow */
 			if (length > ATM_MAX_AAL5_PDU) {
-				atm_dbg(instance, "%s: bogus length %u (vcc: 0x%p)!\n",
+				atm_rldbg(instance, "%s: bogus length %u (vcc: 0x%p)!\n",
 						__func__, length, vcc);
 				atomic_inc(&vcc->stats->rx_err);
 				goto out;
@@ -363,14 +369,14 @@
 			pdu_length = usbatm_pdu_length(length);
 
 			if (sarb->len < pdu_length) {
-				atm_dbg(instance, "%s: bogus pdu_length %u (sarb->len: %u, vcc: 0x%p)!\n",
+				atm_rldbg(instance, "%s: bogus pdu_length %u (sarb->len: %u, vcc: 0x%p)!\n",
 						__func__, pdu_length, sarb->len, vcc);
 				atomic_inc(&vcc->stats->rx_err);
 				goto out;
 			}
 
 			if (crc32_be(~0, sarb->tail - pdu_length, pdu_length) != 0xc704dd7b) {
-				atm_dbg(instance, "%s: packet failed crc check (vcc: 0x%p)!\n",
+				atm_rldbg(instance, "%s: packet failed crc check (vcc: 0x%p)!\n",
 						__func__, vcc);
 				atomic_inc(&vcc->stats->rx_err);
 				goto out;
@@ -379,7 +385,9 @@
 			vdbg("%s: got packet (length: %u, pdu_length: %u, vcc: 0x%p)", __func__, length, pdu_length, vcc);
 
 			if (!(skb = dev_alloc_skb(length))) {
-				atm_dbg(instance, "%s: no memory for skb (length: %u)!\n", __func__, length);
+				if (printk_ratelimit())
+					atm_err(instance, "%s: no memory for skb (length: %u)!\n",
+							__func__, length);
 				atomic_inc(&vcc->stats->rx_drop);
 				goto out;
 			}
@@ -387,7 +395,8 @@
 			vdbg("%s: allocated new sk_buff (skb: 0x%p, skb->truesize: %u)", __func__, skb, skb->truesize);
 
 			if (!atm_charge(vcc, skb->truesize)) {
-				atm_dbg(instance, "%s: failed atm_charge (skb->truesize: %u)!\n", __func__, skb->truesize);
+				atm_rldbg(instance, "%s: failed atm_charge (skb->truesize: %u)!\n",
+						__func__, skb->truesize);
 				dev_kfree_skb(skb);
 				goto out;	/* atm_charge increments rx_drop */
 			}
@@ -600,13 +609,13 @@
 	}
 
 	if (vcc->qos.aal != ATM_AAL5) {
-		atm_dbg(instance, "%s: unsupported ATM type %d!\n", __func__, vcc->qos.aal);
+		atm_rldbg(instance, "%s: unsupported ATM type %d!\n", __func__, vcc->qos.aal);
 		err = -EINVAL;
 		goto fail;
 	}
 
 	if (skb->len > ATM_MAX_AAL5_PDU) {
-		atm_dbg(instance, "%s: packet too long (%d vs %d)!\n",
+		atm_rldbg(instance, "%s: packet too long (%d vs %d)!\n",
 				__func__, skb->len, ATM_MAX_AAL5_PDU);
 		err = -EINVAL;
 		goto fail;
@@ -665,16 +674,16 @@
 **  ATM  **
 **********/
 
-static void usbatm_atm_dev_close(struct atm_dev *dev)
+static void usbatm_atm_dev_close(struct atm_dev *atm_dev)
 {
-	struct usbatm_data *instance = dev->dev_data;
+	struct usbatm_data *instance = atm_dev->dev_data;
 
 	dbg("%s", __func__);
 
 	if (!instance)
 		return;
 
-	dev->dev_data = NULL;
+	atm_dev->dev_data = NULL; /* catch bugs */
 	usbatm_put_instance(instance);	/* taken in usbatm_atm_init */
 }
 
@@ -735,13 +744,18 @@
 	atm_dbg(instance, "%s: vpi %hd, vci %d\n", __func__, vpi, vci);
 
 	/* only support AAL5 */
-	if ((vcc->qos.aal != ATM_AAL5) || (vcc->qos.rxtp.max_sdu < 0)
-	    || (vcc->qos.rxtp.max_sdu > ATM_MAX_AAL5_PDU)) {
-		atm_dbg(instance, "%s: unsupported ATM type %d!\n", __func__, vcc->qos.aal);
+	if ((vcc->qos.aal != ATM_AAL5)) {
+		atm_warn(instance, "%s: unsupported ATM type %d!\n", __func__, vcc->qos.aal);
+		return -EINVAL;
+	}
+
+	/* sanity checks */
+	if ((vcc->qos.rxtp.max_sdu < 0) || (vcc->qos.rxtp.max_sdu > ATM_MAX_AAL5_PDU)) {
+		atm_dbg(instance, "%s: max_sdu %d out of range!\n", __func__, vcc->qos.rxtp.max_sdu);
 		return -EINVAL;
 	}
 
-	down(&instance->serialize);	/* vs self, usbatm_atm_close */
+	down(&instance->serialize);	/* vs self, usbatm_atm_close, usbatm_usb_disconnect */
 
 	if (usbatm_find_vcc(instance, vpi, vci)) {
 		atm_dbg(instance, "%s: %hd/%d already in use!\n", __func__, vpi, vci);
@@ -750,7 +764,7 @@
 	}
 
 	if (!(new = kmalloc(sizeof(struct usbatm_vcc_data), GFP_KERNEL))) {
-		atm_dbg(instance, "%s: no memory for vcc_data!\n", __func__);
+		atm_err(instance, "%s: no memory for vcc_data!\n", __func__);
 		ret = -ENOMEM;
 		goto fail;
 	}
@@ -762,7 +776,7 @@
 
 	new->sarb = alloc_skb(usbatm_pdu_length(vcc->qos.rxtp.max_sdu), GFP_KERNEL);
 	if (!new->sarb) {
-		atm_dbg(instance, "%s: no memory for SAR buffer!\n", __func__);
+		atm_err(instance, "%s: no memory for SAR buffer!\n", __func__);
 		ret = -ENOMEM;
 		goto fail;
 	}
@@ -806,7 +820,7 @@
 
 	usbatm_cancel_send(instance, vcc);
 
-	down(&instance->serialize);	/* vs self, usbatm_atm_open */
+	down(&instance->serialize);	/* vs self, usbatm_atm_open, usbatm_usb_disconnect */
 
 	tasklet_disable(&instance->rx_channel.tasklet);
 	list_del(&vcc_data->list);
@@ -829,7 +843,7 @@
 	atm_dbg(instance, "%s successful\n", __func__);
 }
 
-static int usbatm_atm_ioctl(struct atm_dev *dev, unsigned int cmd,
+static int usbatm_atm_ioctl(struct atm_dev *atm_dev, unsigned int cmd,
 			  void __user * arg)
 {
 	switch (cmd) {
@@ -845,10 +859,13 @@
 	struct atm_dev *atm_dev;
 	int ret, i;
 
-	/* ATM init */
+	/* ATM init.  The ATM initialization scheme suffers from an intrinsic race
+	 * condition: callbacks we register can be executed at once, before we have
+	 * initialized the struct atm_dev.  To protect against this, all callbacks
+	 * abort if atm_dev->dev_data is NULL. */
 	atm_dev = atm_dev_register(instance->driver_name, &usbatm_atm_devops, -1, NULL);
 	if (!atm_dev) {
-		usb_dbg(instance, "%s: failed to register ATM device!\n", __func__);
+		usb_err(instance, "%s: failed to register ATM device!\n", __func__);
 		return -1;
 	}
 
@@ -862,12 +879,13 @@
 	atm_dev->link_rate = 128 * 1000 / 424;
 
 	if (instance->driver->atm_start && ((ret = instance->driver->atm_start(instance, atm_dev)) < 0)) {
-		atm_dbg(instance, "%s: atm_start failed: %d!\n", __func__, ret);
+		atm_err(instance, "%s: atm_start failed: %d!\n", __func__, ret);
 		goto fail;
 	}
 
-	/* ready for ATM callbacks */
 	usbatm_get_instance(instance);	/* dropped in usbatm_atm_dev_close */
+
+	/* ready for ATM callbacks */
 	mb();
 	atm_dev->dev_data = instance;
 
@@ -915,7 +933,7 @@
 	int ret = kernel_thread(usbatm_do_heavy_init, instance, CLONE_KERNEL);
 
 	if (ret < 0) {
-		usb_dbg(instance, "%s: failed to create kernel_thread (%d)!\n", __func__, ret);
+		usb_err(instance, "%s: failed to create kernel_thread (%d)!\n", __func__, ret);
 		return ret;
 	}
 
@@ -953,7 +971,7 @@
 	int i, length;
 	int need_heavy;
 
-	dev_dbg(dev, "%s: trying driver %s with vendor=0x%x, product=0x%x, ifnum %d\n",
+	dev_dbg(dev, "%s: trying driver %s with vendor=%04x, product=%04x, ifnum %2d\n",
 			__func__, driver->driver_name,
 			le16_to_cpu(usb_dev->descriptor.idVendor),
 			le16_to_cpu(usb_dev->descriptor.idProduct),
@@ -962,7 +980,7 @@
 	/* instance init */
 	instance = kzalloc(sizeof(*instance) + sizeof(struct urb *) * (num_rcv_urbs + num_snd_urbs), GFP_KERNEL);
 	if (!instance) {
-		dev_dbg(dev, "%s: no memory for instance data!\n", __func__);
+		dev_err(dev, "%s: no memory for instance data!\n", __func__);
 		return -ENOMEM;
 	}
 
@@ -998,7 +1016,7 @@
  bind:
 	need_heavy = 1;
 	if (driver->bind && (error = driver->bind(instance, intf, id, &need_heavy)) < 0) {
-			dev_dbg(dev, "%s: bind failed: %d!\n", __func__, error);
+			dev_err(dev, "%s: bind failed: %d!\n", __func__, error);
 			goto fail_free;
 	}
 
@@ -1044,7 +1062,7 @@
 
 		urb = usb_alloc_urb(iso_packets, GFP_KERNEL);
 		if (!urb) {
-			dev_dbg(dev, "%s: no memory for urb %d!\n", __func__, i);
+			dev_err(dev, "%s: no memory for urb %d!\n", __func__, i);
 			goto fail_unbind;
 		}
 
@@ -1052,9 +1070,10 @@
 
 		buffer = kmalloc(channel->buf_size, GFP_KERNEL);
 		if (!buffer) {
-			dev_dbg(dev, "%s: no memory for buffer %d!\n", __func__, i);
+			dev_err(dev, "%s: no memory for buffer %d!\n", __func__, i);
 			goto fail_unbind;
 		}
+		/* zero the tx padding to avoid leaking information */
 		memset(buffer, 0, channel->buf_size);
 
 		usb_fill_bulk_urb(urb, instance->usb_dev, channel->endpoint,
diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/usbatm.h Linux/drivers/usb/atm/usbatm.h
--- Linux/drivers/usb/atm.orig/usbatm.h	2006-01-12 18:27:56.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.h	2006-01-12 18:25:54.000000000 +0100
@@ -24,22 +24,21 @@
 #ifndef	_USBATM_H_
 #define	_USBATM_H_
 
-#include <linux/config.h>
-
-/*
-#define VERBOSE_DEBUG
-*/
-
 #include <asm/semaphore.h>
 #include <linux/atm.h>
 #include <linux/atmdev.h>
 #include <linux/completion.h>
 #include <linux/device.h>
+#include <linux/kernel.h>
 #include <linux/kref.h>
 #include <linux/list.h>
 #include <linux/stringify.h>
 #include <linux/usb.h>
 
+/*
+#define VERBOSE_DEBUG
+*/
+
 #ifdef DEBUG
 #define UDSL_ASSERT(x)	BUG_ON(!(x))
 #else
@@ -52,8 +51,13 @@
 	dev_info(&(instance)->usb_intf->dev , format , ## arg)
 #define usb_warn(instance, format, arg...)	\
 	dev_warn(&(instance)->usb_intf->dev , format , ## arg)
+#ifdef DEBUG
+#define usb_dbg(instance, format, arg...)	\
+        dev_printk(KERN_DEBUG , &(instance)->usb_intf->dev , format , ## arg)
+#else
 #define usb_dbg(instance, format, arg...)	\
-	dev_dbg(&(instance)->usb_intf->dev , format , ## arg)
+	do {} while (0)
+#endif
 
 /* FIXME: move to dev_* once ATM is driver model aware */
 #define atm_printk(level, instance, format, arg...)	\
@@ -69,9 +73,14 @@
 #ifdef DEBUG
 #define atm_dbg(instance, format, arg...)	\
 	atm_printk(KERN_DEBUG, instance , format , ## arg)
+#define atm_rldbg(instance, format, arg...)	\
+	if (printk_ratelimit())				\
+		atm_printk(KERN_DEBUG, instance , format , ## arg)
 #else
 #define atm_dbg(instance, format, arg...)	\
 	do {} while (0)
+#define atm_rldbg(instance, format, arg...)	\
+	do {} while (0)
 #endif
 
 
@@ -171,7 +180,7 @@
 	struct usbatm_channel tx_channel;
 
 	struct sk_buff_head sndqueue;
-	struct sk_buff *current_skb;			/* being emptied */
+	struct sk_buff *current_skb;	/* being emptied */
 
 	struct urb *urbs[0];
 };
diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/xusbatm.c Linux/drivers/usb/atm/xusbatm.c
--- Linux/drivers/usb/atm.orig/xusbatm.c	2006-01-12 18:27:56.000000000 +0100
+++ Linux/drivers/usb/atm/xusbatm.c	2006-01-12 18:25:54.000000000 +0100
@@ -61,7 +61,7 @@
 	return 0;
 }
 
-static int xusbatm_bind(struct usbatm_data *usbatm_instance,
+static int xusbatm_bind(struct usbatm_data *usbatm,
 			struct usb_interface *intf, const struct usb_device_id *id,
 			int *need_heavy_init)
 {
@@ -72,14 +72,14 @@
 	u8 searched_ep = rx_ep_present ? tx_endpoint[drv_ix] : rx_endpoint[drv_ix];
 	int i, ret;
 
-	usb_dbg(usbatm_instance, "%s: binding driver %d: vendor %#x product %#x"
-		" rx: ep %#x padd %d tx: ep %#x padd %d\n",
+	usb_dbg(usbatm, "%s: binding driver %d: vendor %04x product %04x"
+		" rx: ep %02x padd %d tx: ep %02x padd %d\n",
 		__func__, drv_ix, vendor[drv_ix], product[drv_ix],
 		rx_endpoint[drv_ix], rx_padding[drv_ix],
 		tx_endpoint[drv_ix], tx_padding[drv_ix]);
 
 	if (!rx_ep_present && !tx_ep_present) {
-		usb_dbg(usbatm_instance, "%s: intf #%d has neither rx (%#x) nor tx (%#x) endpoint\n",
+		usb_dbg(usbatm, "%s: intf #%d has neither rx (%#x) nor tx (%#x) endpoint\n",
 			__func__, intf->altsetting->desc.bInterfaceNumber,
 			rx_endpoint[drv_ix], tx_endpoint[drv_ix]);
 		return -ENODEV;
@@ -93,25 +93,26 @@
 
 		if (cur_if != intf && usb_intf_has_ep(cur_if, searched_ep)) {
 			ret = usb_driver_claim_interface(&xusbatm_usb_driver,
-							 cur_if, usbatm_instance);
+							 cur_if, usbatm);
 			if (!ret)
-				usb_err(usbatm_instance, "%s: failed to claim interface #%d (%d)\n",
+				usb_err(usbatm, "%s: failed to claim interface #%d (%d)\n",
 					__func__, cur_if->altsetting->desc.bInterfaceNumber, ret);
 			return ret;
 		}
 	}
 
-	usb_err(usbatm_instance, "%s: no interface has endpoint %#x\n",
+	usb_err(usbatm, "%s: no interface has endpoint %#x\n",
 		__func__, searched_ep);
 	return -ENODEV;
 }
 
-static void xusbatm_unbind(struct usbatm_data *usbatm_instance,
+static void xusbatm_unbind(struct usbatm_data *usbatm,
 			   struct usb_interface *intf)
 {
 	struct usb_device *usb_dev = interface_to_usbdev(intf);
 	int i;
-	usb_dbg(usbatm_instance, "%s entered\n", __func__);
+
+	usb_dbg(usbatm, "%s entered\n", __func__);
 
 	for(i = 0; i < usb_dev->actconfig->desc.bNumInterfaces; i++) {
 		struct usb_interface *cur_if = usb_dev->actconfig->interface[i];
@@ -120,10 +121,10 @@
 	}
 }
 
-static int xusbatm_atm_start(struct usbatm_data *usbatm_instance,
+static int xusbatm_atm_start(struct usbatm_data *usbatm,
 			     struct atm_dev *atm_dev)
 {
-	atm_dbg(usbatm_instance, "%s entered\n", __func__);
+	atm_dbg(usbatm, "%s entered\n", __func__);
 
 	/* use random MAC as we've no way to get it from the device */
 	random_ether_addr(atm_dev->esi);

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 02/13] USBATM: add flags field
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
  2006-01-12 16:43 ` [PATCH 01/13] USBATM: trivial modifications Duncan Sands
@ 2006-01-12 17:48 ` Duncan Sands
  2006-01-12 18:30 ` [PATCH 00/13] USBATM: summary Greg KH
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-12 17:48 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

Have minidrivers and the core signal special requirements
using a flags field in struct usbatm_data.  For the moment
this is only used to replace the need_heavy_init bind
parameter, but there'll be new flags in later patches.

Signed-off-by:	Duncan Sands <baldrick@free.fr>

[-- Attachment #2: flags --]
[-- Type: text/x-diff, Size: 5876 bytes --]

diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/cxacru.c Linux/drivers/usb/atm/cxacru.c
--- Linux/drivers/usb/atm.orig/cxacru.c	2006-01-12 18:34:35.000000000 +0100
+++ Linux/drivers/usb/atm/cxacru.c	2006-01-12 18:34:40.000000000 +0100
@@ -666,8 +666,7 @@
 }
 
 static int cxacru_bind(struct usbatm_data *usbatm_instance,
-		       struct usb_interface *intf, const struct usb_device_id *id,
-		       int *need_heavy_init)
+		       struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct cxacru_data *instance;
 	struct usb_device *usb_dev = interface_to_usbdev(intf);
@@ -726,7 +725,7 @@
 
 	usbatm_instance->driver_data = instance;
 
-	*need_heavy_init = cxacru_card_status(instance);
+	usbatm_instance->flags = (cxacru_card_status(instance) ? 0 : UDSL_SKIP_HEAVY_INIT);
 
 	return 0;
 
Only in Linux/drivers/usb/atm: done
Only in Linux/drivers/usb/atm.orig/patches: messages
diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/speedtch.c Linux/drivers/usb/atm/speedtch.c
--- Linux/drivers/usb/atm.orig/speedtch.c	2006-01-12 18:34:35.000000000 +0100
+++ Linux/drivers/usb/atm/speedtch.c	2006-01-12 18:34:40.000000000 +0100
@@ -681,8 +681,7 @@
 
 static int speedtch_bind(struct usbatm_data *usbatm,
 			 struct usb_interface *intf,
-			 const struct usb_device_id *id,
-			 int *need_heavy_init)
+			 const struct usb_device_id *id)
 {
 	struct usb_device *usb_dev = interface_to_usbdev(intf);
 	struct usb_interface *cur_intf;
@@ -754,11 +753,11 @@
 			      0x12, 0xc0, 0x07, 0x00,
 			      instance->scratch_buffer + OFFSET_7, SIZE_7, 500);
 
-	*need_heavy_init = (ret != SIZE_7);
+	usbatm->flags = (ret == SIZE_7 ? UDSL_SKIP_HEAVY_INIT : 0);
 
-	usb_dbg(usbatm, "%s: firmware %s loaded\n", __func__, need_heavy_init ? "not" : "already");
+	usb_dbg(usbatm, "%s: firmware %s loaded\n", __func__, usbatm->flags & UDSL_SKIP_HEAVY_INIT ? "already" : "not");
 
-	if (*need_heavy_init)
+	if (!(usbatm->flags & UDSL_SKIP_HEAVY_INIT))
 		if ((ret = usb_reset_device(usb_dev)) < 0) {
 			usb_err(usbatm, "%s: device reset failed (%d)!\n", __func__, ret);
 			goto fail_free;
diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/ueagle-atm.c Linux/drivers/usb/atm/ueagle-atm.c
--- Linux/drivers/usb/atm.orig/ueagle-atm.c	2006-01-12 18:34:35.000000000 +0100
+++ Linux/drivers/usb/atm/ueagle-atm.c	2006-01-12 18:34:40.000000000 +0100
@@ -1617,7 +1617,7 @@
 }
 
 static int uea_bind(struct usbatm_data *usbatm, struct usb_interface *intf,
-		   const struct usb_device_id *id, int *heavy)
+		   const struct usb_device_id *id)
 {
 	struct usb_device *usb = interface_to_usbdev(intf);
 	struct uea_softc *sc;
@@ -1629,7 +1629,7 @@
 	if (ifnum != UEA_INTR_IFACE_NO)
 		return -ENODEV;
 
-	*heavy = sync_wait[modem_index];
+	usbatm_instance->flags = (sync_wait[modem_index] ? 0 : UDSL_SKIP_HEAVY_INIT);
 
 	/* interface 1 is for outbound traffic */
 	ret = claim_interface(usb, usbatm, UEA_US_IFACE_NO);
diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/usbatm.c Linux/drivers/usb/atm/usbatm.c
--- Linux/drivers/usb/atm.orig/usbatm.c	2006-01-12 18:34:35.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-12 18:34:40.000000000 +0100
@@ -969,7 +969,6 @@
 	char *buf;
 	int error = -ENOMEM;
 	int i, length;
-	int need_heavy;
 
 	dev_dbg(dev, "%s: trying driver %s with vendor=%04x, product=%04x, ifnum %2d\n",
 			__func__, driver->driver_name,
@@ -1014,8 +1013,7 @@
 	snprintf(buf, length, ")");
 
  bind:
-	need_heavy = 1;
-	if (driver->bind && (error = driver->bind(instance, intf, id, &need_heavy)) < 0) {
+	if (driver->bind && (error = driver->bind(instance, intf, id)) < 0) {
 			dev_err(dev, "%s: bind failed: %d!\n", __func__, error);
 			goto fail_free;
 	}
@@ -1098,7 +1096,7 @@
 		     __func__, urb->transfer_buffer, urb->transfer_buffer_length, urb);
 	}
 
-	if (need_heavy && driver->heavy_init) {
+	if (!(instance->flags & UDSL_SKIP_HEAVY_INIT) && driver->heavy_init) {
 		error = usbatm_heavy_init(instance);
 	} else {
 		complete(&instance->thread_exited);	/* pretend that heavy_init was run */
diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/usbatm.h Linux/drivers/usb/atm/usbatm.h
--- Linux/drivers/usb/atm.orig/usbatm.h	2006-01-12 18:34:35.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.h	2006-01-12 18:34:40.000000000 +0100
@@ -84,6 +84,11 @@
 #endif
 
 
+/* flags, set by mini-driver in bind() */
+
+#define UDSL_SKIP_HEAVY_INIT	(1<<0)
+
+
 /* mini driver */
 
 struct usbatm_data;
@@ -99,12 +104,9 @@
 
 	const char *driver_name;
 
-	/*
-	*  init device ... can sleep, or cause probe() failure.  Drivers with a heavy_init
-	*  method can avoid having it called by setting need_heavy_init to zero.
-	*/
+	/* init device ... can sleep, or cause probe() failure */
         int (*bind) (struct usbatm_data *, struct usb_interface *,
-		     const struct usb_device_id *id, int *need_heavy_init);
+		     const struct usb_device_id *id);
 
 	/* additional device initialization that is too slow to be done in probe() */
         int (*heavy_init) (struct usbatm_data *, struct usb_interface *);
@@ -152,6 +154,7 @@
 	struct usbatm_driver *driver;
 	void *driver_data;
 	char driver_name[16];
+	unsigned int flags; /* set by mini-driver in bind() */
 
 	/* USB device */
 	struct usb_device *usb_dev;
diff -x '*.orig' -x '*.base' -u -r Linux/drivers/usb/atm.orig/xusbatm.c Linux/drivers/usb/atm/xusbatm.c
--- Linux/drivers/usb/atm.orig/xusbatm.c	2006-01-12 18:34:35.000000000 +0100
+++ Linux/drivers/usb/atm/xusbatm.c	2006-01-12 18:34:40.000000000 +0100
@@ -62,8 +62,7 @@
 }
 
 static int xusbatm_bind(struct usbatm_data *usbatm,
-			struct usb_interface *intf, const struct usb_device_id *id,
-			int *need_heavy_init)
+			struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_device *usb_dev = interface_to_usbdev(intf);
 	int drv_ix = id - xusbatm_usb_ids;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 00/13] USBATM: summary
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
  2006-01-12 16:43 ` [PATCH 01/13] USBATM: trivial modifications Duncan Sands
  2006-01-12 17:48 ` [PATCH 02/13] USBATM: add flags field Duncan Sands
@ 2006-01-12 18:30 ` Greg KH
  2006-01-12 19:15   ` Duncan Sands
  2006-01-13 10:30   ` [linux-usb-devel] " Duncan Sands
  2006-01-13  8:36 ` [PATCH 03/13] USBATM: remove .owner Duncan Sands
                   ` (10 subsequent siblings)
  13 siblings, 2 replies; 19+ messages in thread
From: Greg KH @ 2006-01-12 18:30 UTC (permalink / raw)
  To: Duncan Sands; +Cc: linux-usb-devel, linux-kernel, usbatm

On Thu, Jan 12, 2006 at 05:29:51PM +0100, Duncan Sands wrote:
> Hi Greg, here are some fixes and improvements to the USB ATM
> modem drivers, in thirteen patches:

I only got 2 of these, is my mail just being slow (which it does at odd
times), or did you stop sending them based on some problems on your end?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 00/13] USBATM: summary
  2006-01-12 18:30 ` [PATCH 00/13] USBATM: summary Greg KH
@ 2006-01-12 19:15   ` Duncan Sands
  2006-01-13 10:30   ` [linux-usb-devel] " Duncan Sands
  1 sibling, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-12 19:15 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-usb-devel, linux-kernel, usbatm

> I only got 2 of these, is my mail just being slow (which it does at odd
> times), or did you stop sending them based on some problems on your end?

I stopped sending them based on problems on my end.  I'll send the rest
tomorrow.

Ciao,

Duncan.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 03/13] USBATM: remove .owner
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (2 preceding siblings ...)
  2006-01-12 18:30 ` [PATCH 00/13] USBATM: summary Greg KH
@ 2006-01-13  8:36 ` Duncan Sands
  2006-01-13  8:38 ` [PATCH 04/13] USBATM: kzalloc conversion Duncan Sands
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13  8:36 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

Remove the unused .owner field in struct usbatm_driver.

Signed-off-by:	Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 03-owner --]
[-- Type: text/x-diff, Size: 2696 bytes --]

Index: Linux/drivers/usb/atm/cxacru.c
===================================================================
--- Linux.orig/drivers/usb/atm/cxacru.c	2006-01-13 08:46:17.000000000 +0100
+++ Linux/drivers/usb/atm/cxacru.c	2006-01-13 08:48:09.000000000 +0100
@@ -833,7 +833,6 @@
 MODULE_DEVICE_TABLE(usb, cxacru_usb_ids);
 
 static struct usbatm_driver cxacru_driver = {
-	.owner		= THIS_MODULE,
 	.driver_name	= cxacru_driver_name,
 	.bind		= cxacru_bind,
 	.heavy_init	= cxacru_heavy_init,
Index: Linux/drivers/usb/atm/speedtch.c
===================================================================
--- Linux.orig/drivers/usb/atm/speedtch.c	2006-01-13 08:46:17.000000000 +0100
+++ Linux/drivers/usb/atm/speedtch.c	2006-01-13 08:48:09.000000000 +0100
@@ -793,7 +793,6 @@
 ***********/
 
 static struct usbatm_driver speedtch_usbatm_driver = {
-	.owner		= THIS_MODULE,
 	.driver_name	= speedtch_driver_name,
 	.bind		= speedtch_bind,
 	.heavy_init	= speedtch_heavy_init,
Index: Linux/drivers/usb/atm/ueagle-atm.c
===================================================================
--- Linux.orig/drivers/usb/atm/ueagle-atm.c	2006-01-13 08:46:17.000000000 +0100
+++ Linux/drivers/usb/atm/ueagle-atm.c	2006-01-13 08:48:09.000000000 +0100
@@ -1629,7 +1629,7 @@
 	if (ifnum != UEA_INTR_IFACE_NO)
 		return -ENODEV;
 
-	usbatm_instance->flags = (sync_wait[modem_index] ? 0 : UDSL_SKIP_HEAVY_INIT);
+	usbatm->flags = (sync_wait[modem_index] ? 0 : UDSL_SKIP_HEAVY_INIT);
 
 	/* interface 1 is for outbound traffic */
 	ret = claim_interface(usb, usbatm, UEA_US_IFACE_NO);
@@ -1701,7 +1701,6 @@
 
 static struct usbatm_driver uea_usbatm_driver = {
 	.driver_name = "ueagle-atm",
-	.owner = THIS_MODULE,
 	.bind = uea_bind,
 	.atm_start = uea_atm_open,
 	.unbind = uea_unbind,
Index: Linux/drivers/usb/atm/usbatm.h
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.h	2006-01-13 08:46:17.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.h	2006-01-13 08:48:09.000000000 +0100
@@ -100,8 +100,6 @@
 */
 
 struct usbatm_driver {
-	struct module *owner;
-
 	const char *driver_name;
 
 	/* init device ... can sleep, or cause probe() failure */
Index: Linux/drivers/usb/atm/xusbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/xusbatm.c	2006-01-13 08:46:17.000000000 +0100
+++ Linux/drivers/usb/atm/xusbatm.c	2006-01-13 08:48:09.000000000 +0100
@@ -166,7 +166,6 @@
 		xusbatm_usb_ids[i].idProduct	= product[i];
 
 
-		xusbatm_drivers[i].owner	= THIS_MODULE;
 		xusbatm_drivers[i].driver_name	= xusbatm_driver_name;
 		xusbatm_drivers[i].bind		= xusbatm_bind;
 		xusbatm_drivers[i].unbind	= xusbatm_unbind;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 04/13] USBATM: kzalloc conversion
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (3 preceding siblings ...)
  2006-01-13  8:36 ` [PATCH 03/13] USBATM: remove .owner Duncan Sands
@ 2006-01-13  8:38 ` Duncan Sands
  2006-01-13  8:48 ` [PATCH 05/13] USBATM: xusbatm rewrite Duncan Sands
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13  8:38 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

Convert kmalloc + memset to kzalloc.

Signed-off-by:	Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 04-kzalloc --]
[-- Type: text/x-diff, Size: 2572 bytes --]

Index: Linux/drivers/usb/atm/cxacru.c
===================================================================
--- Linux.orig/drivers/usb/atm/cxacru.c	2006-01-13 08:48:09.000000000 +0100
+++ Linux/drivers/usb/atm/cxacru.c	2006-01-13 08:51:00.000000000 +0100
@@ -673,14 +673,12 @@
 	int ret;
 
 	/* instance init */
-	instance = kmalloc(sizeof(*instance), GFP_KERNEL);
+	instance = kzalloc(sizeof(*instance), GFP_KERNEL);
 	if (!instance) {
 		dbg("cxacru_bind: no memory for instance data");
 		return -ENOMEM;
 	}
 
-	memset(instance, 0, sizeof(*instance));
-
 	instance->usbatm = usbatm_instance;
 	instance->modem_type = (struct cxacru_modem_type *) id->driver_info;
 
Index: Linux/drivers/usb/atm/speedtch.c
===================================================================
--- Linux.orig/drivers/usb/atm/speedtch.c	2006-01-13 08:48:09.000000000 +0100
+++ Linux/drivers/usb/atm/speedtch.c	2006-01-13 08:51:00.000000000 +0100
@@ -715,7 +715,7 @@
 		}
 	}
 
-	instance = kmalloc(sizeof(*instance), GFP_KERNEL);
+	instance = kzalloc(sizeof(*instance), GFP_KERNEL);
 
 	if (!instance) {
 		usb_err(usbatm, "%s: no memory for instance data!\n", __func__);
@@ -723,8 +723,6 @@
 		goto fail_release;
 	}
 
-	memset(instance, 0, sizeof(struct speedtch_instance_data));
-
 	instance->usbatm = usbatm;
 
 	INIT_WORK(&instance->status_checker, (void *)speedtch_check_status, instance);
Index: Linux/drivers/usb/atm/usbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.c	2006-01-13 08:46:17.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-13 08:51:00.000000000 +0100
@@ -763,13 +763,12 @@
 		goto fail;
 	}
 
-	if (!(new = kmalloc(sizeof(struct usbatm_vcc_data), GFP_KERNEL))) {
+	if (!(new = kzalloc(sizeof(struct usbatm_vcc_data), GFP_KERNEL))) {
 		atm_err(instance, "%s: no memory for vcc_data!\n", __func__);
 		ret = -ENOMEM;
 		goto fail;
 	}
 
-	memset(new, 0, sizeof(struct usbatm_vcc_data));
 	new->vcc = vcc;
 	new->vpi = vpi;
 	new->vci = vci;
@@ -1066,13 +1065,12 @@
 
 		instance->urbs[i] = urb;
 
-		buffer = kmalloc(channel->buf_size, GFP_KERNEL);
+		/* zero the tx padding to avoid leaking information */
+		buffer = kzalloc(channel->buf_size, GFP_KERNEL);
 		if (!buffer) {
 			dev_err(dev, "%s: no memory for buffer %d!\n", __func__, i);
 			goto fail_unbind;
 		}
-		/* zero the tx padding to avoid leaking information */
-		memset(buffer, 0, channel->buf_size);
 
 		usb_fill_bulk_urb(urb, instance->usb_dev, channel->endpoint,
 				  buffer, channel->buf_size, usbatm_complete, channel);

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 05/13] USBATM: xusbatm rewrite
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (4 preceding siblings ...)
  2006-01-13  8:38 ` [PATCH 04/13] USBATM: kzalloc conversion Duncan Sands
@ 2006-01-13  8:48 ` Duncan Sands
  2006-01-13  9:05 ` [PATCH 06/13] USBATM: shutdown open connections when disconnected Duncan Sands
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13  8:48 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

The xusbatm driver is for otherwise unsupported modems.
All it does is grab hold of a user-specified set of
interfaces - the generic usbatm core methods (hopefully)
do the rest.  As Aurelio Arroyo discovered when he tried
to use xusbatm (big mistake!), the interface grabbing logic
was completely borked.  Here is a rewrite that works.

Signed-off-by:	Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 05-xusbatm --]
[-- Type: text/x-csrc, Size: 6645 bytes --]

Index: Linux/drivers/usb/atm/xusbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/xusbatm.c	2006-01-13 08:48:09.000000000 +0100
+++ Linux/drivers/usb/atm/xusbatm.c	2006-01-13 08:57:07.000000000 +0100
@@ -41,6 +41,8 @@
 XUSBATM_PARM(tx_endpoint, unsigned char, byte, "tx endpoint number");
 XUSBATM_PARM(rx_padding, unsigned char, byte, "rx padding (default 0)");
 XUSBATM_PARM(tx_padding, unsigned char, byte, "tx padding (default 0)");
+XUSBATM_PARM(rx_altsetting, unsigned char, byte, "rx altsetting (default 0)");
+XUSBATM_PARM(tx_altsetting, unsigned char, byte, "rx altsetting (default 0)");
 
 static const char xusbatm_driver_name[] = "xusbatm";
 
@@ -48,61 +50,94 @@
 static struct usb_device_id xusbatm_usb_ids[XUSBATM_DRIVERS_MAX + 1];
 static struct usb_driver xusbatm_usb_driver;
 
-static int usb_intf_has_ep(const struct usb_interface *intf, u8 ep)
+static struct usb_interface *xusbatm_find_intf (struct usb_device *usb_dev, int altsetting, u8 ep)
 {
+	struct usb_host_interface *alt;
+	struct usb_interface *intf;
 	int i, j;
 
-	for (i = 0; i < intf->num_altsetting; i++) {
-		struct usb_host_interface *alt = intf->altsetting;
-		for (j = 0; j < alt->desc.bNumEndpoints; j++)
-			if ((alt->endpoint[i].desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) == ep)
-				return 1;
+	for(i = 0; i < usb_dev->actconfig->desc.bNumInterfaces; i++)
+		if ((intf = usb_dev->actconfig->interface[i]) && (alt = usb_altnum_to_altsetting(intf, altsetting)))
+			for (j = 0; j < alt->desc.bNumEndpoints; j++)
+				if (alt->endpoint[j].desc.bEndpointAddress == ep)
+					return intf;
+	return NULL;
+}
+
+static int xusbatm_capture_intf (struct usbatm_data *usbatm, struct usb_device *usb_dev,
+		struct usb_interface *intf, int altsetting, int claim)
+{
+	int ifnum = intf->altsetting->desc.bInterfaceNumber;
+	int ret;
+
+	if (claim && (ret = usb_driver_claim_interface(&xusbatm_usb_driver, intf, usbatm))) {
+		usb_err(usbatm, "%s: failed to claim interface %2d (%d)!\n", __func__, ifnum, ret);
+		return ret;
+	}
+	if ((ret = usb_set_interface(usb_dev, ifnum, altsetting))) {
+		usb_err(usbatm, "%s: altsetting %2d for interface %2d failed (%d)!\n", __func__, altsetting, ifnum, ret);
+		return ret;
 	}
 	return 0;
 }
 
+static void xusbatm_release_intf (struct usb_device *usb_dev, struct usb_interface *intf, int claimed)
+{
+	if (claimed) {
+		usb_set_intfdata(intf, NULL);
+		usb_driver_release_interface(&xusbatm_usb_driver, intf);
+	}
+}
+
 static int xusbatm_bind(struct usbatm_data *usbatm,
 			struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_device *usb_dev = interface_to_usbdev(intf);
 	int drv_ix = id - xusbatm_usb_ids;
-	int rx_ep_present = usb_intf_has_ep(intf, rx_endpoint[drv_ix]);
-	int tx_ep_present = usb_intf_has_ep(intf, tx_endpoint[drv_ix]);
-	u8 searched_ep = rx_ep_present ? tx_endpoint[drv_ix] : rx_endpoint[drv_ix];
-	int i, ret;
+	int rx_alt = rx_altsetting[drv_ix];
+	int tx_alt = tx_altsetting[drv_ix];
+	struct usb_interface *rx_intf = xusbatm_find_intf(usb_dev, rx_alt, rx_endpoint[drv_ix]);
+	struct usb_interface *tx_intf = xusbatm_find_intf(usb_dev, tx_alt, tx_endpoint[drv_ix]);
+	int ret;
 
 	usb_dbg(usbatm, "%s: binding driver %d: vendor %04x product %04x"
-		" rx: ep %02x padd %d tx: ep %02x padd %d\n",
+		" rx: ep %02x padd %d alt %2d tx: ep %02x padd %d alt %2d\n",
 		__func__, drv_ix, vendor[drv_ix], product[drv_ix],
-		rx_endpoint[drv_ix], rx_padding[drv_ix],
-		tx_endpoint[drv_ix], tx_padding[drv_ix]);
+		rx_endpoint[drv_ix], rx_padding[drv_ix], rx_alt,
+		tx_endpoint[drv_ix], tx_padding[drv_ix], tx_alt);
 
-	if (!rx_ep_present && !tx_ep_present) {
-		usb_dbg(usbatm, "%s: intf #%d has neither rx (%#x) nor tx (%#x) endpoint\n",
-			__func__, intf->altsetting->desc.bInterfaceNumber,
-			rx_endpoint[drv_ix], tx_endpoint[drv_ix]);
+	if (!rx_intf || !tx_intf) {
+		if (!rx_intf)
+			usb_dbg(usbatm, "%s: no interface contains endpoint %02x in altsetting %2d\n",
+				__func__, rx_endpoint[drv_ix], rx_alt);
+		if (!tx_intf)
+			usb_dbg(usbatm, "%s: no interface contains endpoint %02x in altsetting %2d\n",
+				__func__, tx_endpoint[drv_ix], tx_alt);
 		return -ENODEV;
 	}
 
-	if (rx_ep_present && tx_ep_present)
-		return 0;
+	if ((rx_intf != intf) && (tx_intf != intf))
+		return -ENODEV;
 
-	for(i = 0; i < usb_dev->actconfig->desc.bNumInterfaces; i++) {
-		struct usb_interface *cur_if = usb_dev->actconfig->interface[i];
+	if ((rx_intf == tx_intf) && (rx_alt != tx_alt)) {
+		usb_err(usbatm, "%s: altsettings clash on interface %2d (%2d vs %2d)!\n", __func__,
+				rx_intf->altsetting->desc.bInterfaceNumber, rx_alt, tx_alt);
+		return -EINVAL;
+	}
 
-		if (cur_if != intf && usb_intf_has_ep(cur_if, searched_ep)) {
-			ret = usb_driver_claim_interface(&xusbatm_usb_driver,
-							 cur_if, usbatm);
-			if (!ret)
-				usb_err(usbatm, "%s: failed to claim interface #%d (%d)\n",
-					__func__, cur_if->altsetting->desc.bInterfaceNumber, ret);
-			return ret;
-		}
+	usb_dbg(usbatm, "%s: rx If#=%2d; tx If#=%2d\n", __func__,
+			rx_intf->altsetting->desc.bInterfaceNumber,
+			tx_intf->altsetting->desc.bInterfaceNumber);
+
+	if ((ret = xusbatm_capture_intf(usbatm, usb_dev, rx_intf, rx_alt, rx_intf != intf)))
+		return ret;
+
+	if ((tx_intf != rx_intf) && (ret = xusbatm_capture_intf(usbatm, usb_dev, tx_intf, tx_alt, tx_intf != intf))) {
+		xusbatm_release_intf(usb_dev, rx_intf, rx_intf != intf);
+		return ret;
 	}
 
-	usb_err(usbatm, "%s: no interface has endpoint %#x\n",
-		__func__, searched_ep);
-	return -ENODEV;
+	return 0;
 }
 
 static void xusbatm_unbind(struct usbatm_data *usbatm,
@@ -114,9 +149,12 @@
 	usb_dbg(usbatm, "%s entered\n", __func__);
 
 	for(i = 0; i < usb_dev->actconfig->desc.bNumInterfaces; i++) {
-		struct usb_interface *cur_if = usb_dev->actconfig->interface[i];
-		usb_set_intfdata(cur_if, NULL);
-		usb_driver_release_interface(&xusbatm_usb_driver, cur_if);
+		struct usb_interface *cur_intf = usb_dev->actconfig->interface[i];
+
+		if (cur_intf && (usb_get_intfdata(cur_intf) == usbatm)) {
+			usb_set_intfdata(cur_intf, NULL);
+			usb_driver_release_interface(&xusbatm_usb_driver, cur_intf);
+		}
 	}
 }
 
@@ -161,11 +199,13 @@
 	}
 
 	for (i = 0; i < num_vendor; i++) {
+		rx_endpoint[i] |= USB_DIR_IN;
+		tx_endpoint[i] &= USB_ENDPOINT_NUMBER_MASK;
+
 		xusbatm_usb_ids[i].match_flags	= USB_DEVICE_ID_MATCH_DEVICE;
 		xusbatm_usb_ids[i].idVendor	= vendor[i];
 		xusbatm_usb_ids[i].idProduct	= product[i];
 
-
 		xusbatm_drivers[i].driver_name	= xusbatm_driver_name;
 		xusbatm_drivers[i].bind		= xusbatm_bind;
 		xusbatm_drivers[i].unbind	= xusbatm_unbind;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 06/13] USBATM: shutdown open connections when disconnected
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (5 preceding siblings ...)
  2006-01-13  8:48 ` [PATCH 05/13] USBATM: xusbatm rewrite Duncan Sands
@ 2006-01-13  9:05 ` Duncan Sands
  2006-08-18 19:40   ` R: " Giampaolo Tomassoni
  2006-01-13  9:07 ` [PATCH 07/13] USBATM: return correct error code when out of memory Duncan Sands
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Duncan Sands @ 2006-01-13  9:05 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel, linux-atm-general

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

This patch causes vcc_release_async to be applied to any open
vcc's when the modem is disconnected.  This signals a socket
shutdown, letting the socket user know that the game is up.
I wrote this patch because of reports that pppd would keep
connections open forever when the modem is disconnected.
This patch does not fix that problem, but it's a step in the
right direction.  It doesn't help because the pppoatm module
doesn't yet monitor state changes on the ATM socket, so simply
never realises that the ATM connection has gone down (meaning
it doesn't tell the ppp layer).  But at least there is a socket
state change now.  Unfortunately this patch may create problems
for those rare users like me who use routed IP or some other
non-ppp connection method that goes via the ATM ARP daemon: the
daemon is buggy, and with this patch will crash when the modem
is disconnected.  Users with a buggy atmarpd can simply restart
it after disconnecting the modem.

Signed-off-by: Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 06-disconnect --]
[-- Type: text/x-diff, Size: 4338 bytes --]

Index: Linux/drivers/usb/atm/usbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.c	2006-01-13 08:51:00.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-13 08:57:48.000000000 +0100
@@ -602,8 +602,12 @@
 
 	vdbg("%s called (skb 0x%p, len %u)", __func__, skb, skb->len);
 
-	if (!instance) {
-		dbg("%s: NULL data!", __func__);
+	/* racy disconnection check - fine */
+	if (!instance || instance->disconnected) {
+#ifdef DEBUG
+		if (printk_ratelimit())
+			printk(KERN_DEBUG "%s: %s!\n", __func__, instance ? "disconnected" : "NULL instance");
+#endif
 		err = -ENODEV;
 		goto fail;
 	}
@@ -715,15 +719,19 @@
 			       atomic_read(&atm_dev->stats.aal5.rx_err),
 			       atomic_read(&atm_dev->stats.aal5.rx_drop));
 
-	if (!left--)
-		switch (atm_dev->signal) {
-		case ATM_PHY_SIG_FOUND:
-			return sprintf(page, "Line up\n");
-		case ATM_PHY_SIG_LOST:
-			return sprintf(page, "Line down\n");
-		default:
-			return sprintf(page, "Line state unknown\n");
-		}
+	if (!left--) {
+		if (instance->disconnected)
+			return sprintf(page, "Disconnected\n");
+		else
+			switch (atm_dev->signal) {
+			case ATM_PHY_SIG_FOUND:
+				return sprintf(page, "Line up\n");
+			case ATM_PHY_SIG_LOST:
+				return sprintf(page, "Line down\n");
+			default:
+				return sprintf(page, "Line state unknown\n");
+			}
+	}
 
 	return 0;
 }
@@ -757,6 +765,12 @@
 
 	down(&instance->serialize);	/* vs self, usbatm_atm_close, usbatm_usb_disconnect */
 
+	if (instance->disconnected) {
+		atm_dbg(instance, "%s: disconnected!\n", __func__);
+		ret = -ENODEV;
+		goto fail;
+	}
+
 	if (usbatm_find_vcc(instance, vpi, vci)) {
 		atm_dbg(instance, "%s: %hd/%d already in use!\n", __func__, vpi, vci);
 		ret = -EADDRINUSE;
@@ -845,6 +859,13 @@
 static int usbatm_atm_ioctl(struct atm_dev *atm_dev, unsigned int cmd,
 			  void __user * arg)
 {
+	struct usbatm_data *instance = atm_dev->dev_data;
+
+	if (!instance || instance->disconnected) {
+		dbg("%s: %s!", __func__, instance ? "disconnected" : "NULL instance");
+		return -ENODEV;
+	}
+
 	switch (cmd) {
 	case ATM_QUERYLOOP:
 		return put_user(ATM_LM_NONE, (int __user *)arg) ? -EFAULT : 0;
@@ -1129,6 +1150,7 @@
 {
 	struct device *dev = &intf->dev;
 	struct usbatm_data *instance = usb_get_intfdata(intf);
+	struct usbatm_vcc_data *vcc_data;
 	int i;
 
 	dev_dbg(dev, "%s entered\n", __func__);
@@ -1141,12 +1163,18 @@
 	usb_set_intfdata(intf, NULL);
 
 	down(&instance->serialize);
+	instance->disconnected = 1;
 	if (instance->thread_pid >= 0)
 		kill_proc(instance->thread_pid, SIGTERM, 1);
 	up(&instance->serialize);
 
 	wait_for_completion(&instance->thread_exited);
 
+	down(&instance->serialize);
+	list_for_each_entry(vcc_data, &instance->vcc_list, list)
+		vcc_release_async(vcc_data->vcc, -EPIPE);
+	up(&instance->serialize);
+
 	tasklet_disable(&instance->rx_channel.tasklet);
 	tasklet_disable(&instance->tx_channel.tasklet);
 
@@ -1156,6 +1184,14 @@
 	del_timer_sync(&instance->rx_channel.delay);
 	del_timer_sync(&instance->tx_channel.delay);
 
+	/* turn usbatm_[rt]x_process into something close to a no-op */
+	/* no need to take the spinlock */
+	INIT_LIST_HEAD(&instance->rx_channel.list);
+	INIT_LIST_HEAD(&instance->tx_channel.list);
+
+	tasklet_enable(&instance->rx_channel.tasklet);
+	tasklet_enable(&instance->tx_channel.tasklet);
+
 	if (instance->atm_dev && instance->driver->atm_stop)
 		instance->driver->atm_stop(instance, instance->atm_dev);
 
@@ -1164,14 +1200,6 @@
 
 	instance->driver_data = NULL;
 
-	/* turn usbatm_[rt]x_process into noop */
-	/* no need to take the spinlock */
-	INIT_LIST_HEAD(&instance->rx_channel.list);
-	INIT_LIST_HEAD(&instance->tx_channel.list);
-
-	tasklet_enable(&instance->rx_channel.tasklet);
-	tasklet_enable(&instance->tx_channel.tasklet);
-
 	for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
 		kfree(instance->urbs[i]->transfer_buffer);
 		usb_free_urb(instance->urbs[i]);
Index: Linux/drivers/usb/atm/usbatm.h
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.h	2006-01-13 08:48:09.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.h	2006-01-13 08:57:48.000000000 +0100
@@ -168,6 +168,7 @@
 
 	struct kref refcount;
 	struct semaphore serialize;
+	int disconnected;
 
 	/* heavy init */
 	int thread_pid;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 07/13] USBATM: return correct error code when out of memory
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (6 preceding siblings ...)
  2006-01-13  9:05 ` [PATCH 06/13] USBATM: shutdown open connections when disconnected Duncan Sands
@ 2006-01-13  9:07 ` Duncan Sands
  2006-01-13  9:13 ` [PATCH 08/13] USBATM: use dev_kfree_skb_any rather than dev_kfree_skb Duncan Sands
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13  9:07 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

We weren't always returning -ENOMEM.

Signed-off-by:	Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 07-memory --]
[-- Type: text/x-diff, Size: 658 bytes --]

Index: Linux/drivers/usb/atm/usbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.c	2006-01-13 08:57:48.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-13 08:59:16.000000000 +0100
@@ -1081,6 +1081,7 @@
 		urb = usb_alloc_urb(iso_packets, GFP_KERNEL);
 		if (!urb) {
 			dev_err(dev, "%s: no memory for urb %d!\n", __func__, i);
+			error = -ENOMEM;
 			goto fail_unbind;
 		}
 
@@ -1090,6 +1091,7 @@
 		buffer = kzalloc(channel->buf_size, GFP_KERNEL);
 		if (!buffer) {
 			dev_err(dev, "%s: no memory for buffer %d!\n", __func__, i);
+			error = -ENOMEM;
 			goto fail_unbind;
 		}
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 08/13] USBATM: use dev_kfree_skb_any rather than dev_kfree_skb
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (7 preceding siblings ...)
  2006-01-13  9:07 ` [PATCH 07/13] USBATM: return correct error code when out of memory Duncan Sands
@ 2006-01-13  9:13 ` Duncan Sands
  2006-01-13  9:52 ` [PATCH 09/13] USBATM: measure buffer size in bytes; force valid sizes Duncan Sands
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13  9:13 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

In one spot (usbatm_cancel_send) we were calling dev_kfree_skb with irqs
disabled.  This mistake is just too easy to make, so systematically use
dev_kfree_skb_any rather than dev_kfree_skb.

Signed-off-by:	Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 08-dev_kfree_skb_any --]
[-- Type: text/x-diff, Size: 873 bytes --]

Index: Linux/drivers/usb/atm/usbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.c	2006-01-13 08:59:16.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-13 09:00:06.000000000 +0100
@@ -72,6 +72,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/netdevice.h>
 #include <linux/proc_fs.h>
 #include <linux/sched.h>
 #include <linux/signal.h>
@@ -199,7 +200,7 @@
 	if (vcc->pop)
 		vcc->pop(vcc, skb);
 	else
-		dev_kfree_skb(skb);
+		dev_kfree_skb_any(skb);
 }
 
 
@@ -397,7 +398,7 @@
 			if (!atm_charge(vcc, skb->truesize)) {
 				atm_rldbg(instance, "%s: failed atm_charge (skb->truesize: %u)!\n",
 						__func__, skb->truesize);
-				dev_kfree_skb(skb);
+				dev_kfree_skb_any(skb);
 				goto out;	/* atm_charge increments rx_drop */
 			}
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 09/13] USBATM: measure buffer size in bytes; force valid sizes
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (8 preceding siblings ...)
  2006-01-13  9:13 ` [PATCH 08/13] USBATM: use dev_kfree_skb_any rather than dev_kfree_skb Duncan Sands
@ 2006-01-13  9:52 ` Duncan Sands
  2006-01-13  9:59 ` [PATCH 10/13] USBATM: allow isochronous transfer Duncan Sands
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13  9:52 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

Change the module parameters rcv_buf_size and snd_buf_size to
specify buffer sizes in bytes rather than ATM cells.  Since
there is some danger that users may not notice this change,
the parameters are renamed to rcv_buf_bytes etc.  The transmit
buffer needs to be a multiple of the ATM cell size in length,
while the receive buffer should be a multiple of the endpoint
maxpacket size (this wasn't enforced before, which causes trouble
with isochronous transfers), so enforce these restrictions.  Now
that the usbatm probe method inspects the endpoint maxpacket size,
minidriver bind routines need to set the correct alternate setting
for the interface in their bind routine.  This is the reason for
the speedtch changes.

Signed-off-by:	Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 09-bytes --]
[-- Type: text/x-diff, Size: 9193 bytes --]

Index: Linux/drivers/usb/atm/usbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.c	2006-01-13 09:00:06.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-13 09:05:06.000000000 +0100
@@ -99,12 +99,11 @@
 
 #define UDSL_MAX_RCV_URBS		16
 #define UDSL_MAX_SND_URBS		16
-#define UDSL_MAX_RCV_BUF_SIZE		1024	/* ATM cells */
-#define UDSL_MAX_SND_BUF_SIZE		1024	/* ATM cells */
+#define UDSL_MAX_BUF_SIZE		64 * 1024	/* bytes */
 #define UDSL_DEFAULT_RCV_URBS		4
 #define UDSL_DEFAULT_SND_URBS		4
-#define UDSL_DEFAULT_RCV_BUF_SIZE	64	/* ATM cells */
-#define UDSL_DEFAULT_SND_BUF_SIZE	64	/* ATM cells */
+#define UDSL_DEFAULT_RCV_BUF_SIZE	64 * ATM_CELL_SIZE	/* bytes */
+#define UDSL_DEFAULT_SND_BUF_SIZE	64 * ATM_CELL_SIZE	/* bytes */
 
 #define ATM_CELL_HEADER			(ATM_CELL_SIZE - ATM_CELL_PAYLOAD)
 
@@ -112,8 +111,8 @@
 
 static unsigned int num_rcv_urbs = UDSL_DEFAULT_RCV_URBS;
 static unsigned int num_snd_urbs = UDSL_DEFAULT_SND_URBS;
-static unsigned int rcv_buf_size = UDSL_DEFAULT_RCV_BUF_SIZE;
-static unsigned int snd_buf_size = UDSL_DEFAULT_SND_BUF_SIZE;
+static unsigned int rcv_buf_bytes = UDSL_DEFAULT_RCV_BUF_SIZE;
+static unsigned int snd_buf_bytes = UDSL_DEFAULT_SND_BUF_SIZE;
 
 module_param(num_rcv_urbs, uint, S_IRUGO);
 MODULE_PARM_DESC(num_rcv_urbs,
@@ -127,15 +126,15 @@
 		 __MODULE_STRING(UDSL_MAX_SND_URBS) ", default: "
 		 __MODULE_STRING(UDSL_DEFAULT_SND_URBS) ")");
 
-module_param(rcv_buf_size, uint, S_IRUGO);
-MODULE_PARM_DESC(rcv_buf_size,
-		 "Size of the buffers used for reception in ATM cells (range: 1-"
-		 __MODULE_STRING(UDSL_MAX_RCV_BUF_SIZE) ", default: "
+module_param(rcv_buf_bytes, uint, S_IRUGO);
+MODULE_PARM_DESC(rcv_buf_bytes,
+		 "Size of the buffers used for reception, in bytes (range: 1-"
+		 __MODULE_STRING(UDSL_MAX_BUF_SIZE) ", default: "
 		 __MODULE_STRING(UDSL_DEFAULT_RCV_BUF_SIZE) ")");
 
-module_param(snd_buf_size, uint, S_IRUGO);
-MODULE_PARM_DESC(snd_buf_size,
-		 "Size of the buffers used for transmission in ATM cells (range: 1-"
+module_param(snd_buf_bytes, uint, S_IRUGO);
+MODULE_PARM_DESC(snd_buf_bytes,
+		 "Size of the buffers used for transmission, in bytes (range: 1-"
 		 __MODULE_STRING(UDSL_MAX_SND_BUF_SIZE) ", default: "
 		 __MODULE_STRING(UDSL_DEFAULT_SND_BUF_SIZE) ")");
 
@@ -430,14 +429,14 @@
 {
 	struct usbatm_control *ctrl = UDSL_SKB(skb);
 	struct atm_vcc *vcc = ctrl->atm.vcc;
-	unsigned int num_written;
+	unsigned int bytes_written;
 	unsigned int stride = instance->tx_channel.stride;
 
 	vdbg("%s: skb->len=%d, avail_space=%u", __func__, skb->len, avail_space);
 	UDSL_ASSERT(!(avail_space % stride));
 
-	for (num_written = 0; num_written < avail_space && ctrl->len;
-	     num_written += stride, target += stride) {
+	for (bytes_written = 0; bytes_written < avail_space && ctrl->len;
+	     bytes_written += stride, target += stride) {
 		unsigned int data_len = min_t(unsigned int, skb->len, ATM_CELL_PAYLOAD);
 		unsigned int left = ATM_CELL_PAYLOAD - data_len;
 		u8 *ptr = target;
@@ -480,7 +479,7 @@
 			ctrl->crc = crc32_be(ctrl->crc, ptr, left);
 	}
 
-	return num_written;
+	return bytes_written;
 }
 
 
@@ -524,7 +523,7 @@
 	struct sk_buff *skb = instance->current_skb;
 	struct urb *urb = NULL;
 	const unsigned int buf_size = instance->tx_channel.buf_size;
-	unsigned int num_written = 0;
+	unsigned int bytes_written = 0;
 	u8 *buffer = NULL;
 
 	if (!skb)
@@ -536,16 +535,16 @@
 			if (!urb)
 				break;		/* no more senders */
 			buffer = urb->transfer_buffer;
-			num_written = (urb->status == -EAGAIN) ?
+			bytes_written = (urb->status == -EAGAIN) ?
 				urb->transfer_buffer_length : 0;
 		}
 
-		num_written += usbatm_write_cells(instance, skb,
-						  buffer + num_written,
-						  buf_size - num_written);
+		bytes_written += usbatm_write_cells(instance, skb,
+						  buffer + bytes_written,
+						  buf_size - bytes_written);
 
 		vdbg("%s: wrote %u bytes from skb 0x%p to urb 0x%p",
-		     __func__, num_written, skb, urb);
+		     __func__, bytes_written, skb, urb);
 
 		if (!UDSL_SKB(skb)->len) {
 			struct atm_vcc *vcc = UDSL_SKB(skb)->atm.vcc;
@@ -556,8 +555,8 @@
 			skb = skb_dequeue(&instance->sndqueue);
 		}
 
-		if (num_written == buf_size || (!skb && num_written)) {
-			urb->transfer_buffer_length = num_written;
+		if (bytes_written == buf_size || (!skb && bytes_written)) {
+			urb->transfer_buffer_length = bytes_written;
 
 			if (usbatm_submit_urb(urb))
 				break;
@@ -990,6 +989,7 @@
 	char *buf;
 	int error = -ENOMEM;
 	int i, length;
+	unsigned int maxpacket, num_packets;
 
 	dev_dbg(dev, "%s: trying driver %s with vendor=%04x, product=%04x, ifnum %2d\n",
 			__func__, driver->driver_name,
@@ -1058,10 +1058,38 @@
 	instance->tx_channel.endpoint = usb_sndbulkpipe(usb_dev, driver->out);
 	instance->rx_channel.stride = ATM_CELL_SIZE + driver->rx_padding;
 	instance->tx_channel.stride = ATM_CELL_SIZE + driver->tx_padding;
-	instance->rx_channel.buf_size = rcv_buf_size * instance->rx_channel.stride;
-	instance->tx_channel.buf_size = snd_buf_size * instance->tx_channel.stride;
 	instance->rx_channel.usbatm = instance->tx_channel.usbatm = instance;
 
+	/* tx buffer size must be a positive multiple of the stride */
+	instance->tx_channel.buf_size = max (instance->tx_channel.stride,
+			snd_buf_bytes - (snd_buf_bytes % instance->tx_channel.stride));
+
+	/* rx buffer size must be a positive multiple of the endpoint maxpacket */
+	maxpacket = usb_maxpacket(usb_dev, instance->rx_channel.endpoint, 0);
+
+	if ((maxpacket < 1) || (maxpacket > UDSL_MAX_BUF_SIZE)) {
+		dev_err(dev, "%s: invalid endpoint %02x!\n", __func__,
+				usb_pipeendpoint(instance->rx_channel.endpoint));
+		error = -EINVAL;
+		goto fail_unbind;
+	}
+
+	num_packets = max (1U, (rcv_buf_bytes + maxpacket / 2) / maxpacket); /* round */
+
+	if (num_packets * maxpacket > UDSL_MAX_BUF_SIZE)
+		num_packets--;
+
+	instance->rx_channel.buf_size = num_packets * maxpacket;
+
+#ifdef DEBUG
+	for (i = 0; i < 2; i++) {
+		struct usbatm_channel *channel = i ?
+			&instance->tx_channel : &instance->rx_channel;
+
+		dev_dbg(dev, "%s: using %d byte buffer for %s channel 0x%p\n", __func__, channel->buf_size, i ? "tx" : "rx", channel);
+	}
+#endif
+
 	skb_queue_head_init(&instance->sndqueue);
 
 	for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
@@ -1232,10 +1260,10 @@
 
 	if ((num_rcv_urbs > UDSL_MAX_RCV_URBS)
 	    || (num_snd_urbs > UDSL_MAX_SND_URBS)
-	    || (rcv_buf_size < 1)
-	    || (rcv_buf_size > UDSL_MAX_RCV_BUF_SIZE)
-	    || (snd_buf_size < 1)
-	    || (snd_buf_size > UDSL_MAX_SND_BUF_SIZE))
+	    || (rcv_buf_bytes < 1)
+	    || (rcv_buf_bytes > UDSL_MAX_BUF_SIZE)
+	    || (snd_buf_bytes < 1)
+	    || (snd_buf_bytes > UDSL_MAX_BUF_SIZE))
 		return -EINVAL;
 
 	return 0;
Index: Linux/drivers/usb/atm/speedtch.c
===================================================================
--- Linux.orig/drivers/usb/atm/speedtch.c	2006-01-13 09:08:36.000000000 +0100
+++ Linux/drivers/usb/atm/speedtch.c	2006-01-13 09:08:53.000000000 +0100
@@ -89,6 +89,7 @@
 		 "Enable software buffering (default: "
 		 __MODULE_STRING(DEFAULT_SW_BUFFERING) ")");
 
+#define INTERFACE_DATA		1
 #define ENDPOINT_INT		0x81
 #define ENDPOINT_DATA		0x07
 #define ENDPOINT_FIRMWARE	0x05
@@ -98,6 +99,8 @@
 struct speedtch_instance_data {
 	struct usbatm_data *usbatm;
 
+	unsigned int altsetting;
+
 	struct work_struct status_checker;
 
 	unsigned char last_status;
@@ -270,6 +273,11 @@
 	   because we're in our own kernel thread anyway. */
 	msleep_interruptible(1000);
 
+	if ((ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->altsetting)) < 0) {
+		usb_err(usbatm, "%s: setting interface to %d failed (%d)!\n", __func__, instance->altsetting, ret);
+		goto out_free;
+	}
+
 	/* Enable software buffering, if requested */
 	if (sw_buffering)
 		speedtch_set_swbuff(instance, 1);
@@ -586,11 +594,6 @@
 
 	atm_dbg(usbatm, "%s entered\n", __func__);
 
-	if ((ret = usb_set_interface(usb_dev, 1, altsetting)) < 0) {
-		atm_dbg(usbatm, "%s: usb_set_interface returned %d!\n", __func__, ret);
-		return ret;
-	}
-
 	/* Set MAC address, it is stored in the serial number */
 	memset(atm_dev->esi, 0, sizeof(atm_dev->esi));
 	if (usb_string(usb_dev, usb_dev->descriptor.iSerialNumber, mac_str, sizeof(mac_str)) == 12) {
@@ -725,6 +728,23 @@
 
 	instance->usbatm = usbatm;
 
+	/* altsetting may change at any moment, so take a snapshot */
+	instance->altsetting = altsetting;
+
+	if (instance->altsetting)
+		if ((ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->altsetting)) < 0) {
+			usb_err(usbatm, "%s: setting interface to %2d failed (%d)!\n", __func__, instance->altsetting, ret);
+			instance->altsetting = 0; /* fall back to default */
+		}
+
+	if (!instance->altsetting) {
+		if ((ret = usb_set_interface(usb_dev, INTERFACE_DATA, DEFAULT_ALTSETTING)) < 0) {
+			usb_err(usbatm, "%s: setting interface to %2d failed (%d)!\n", __func__, DEFAULT_ALTSETTING, ret);
+			goto fail_free;
+		}
+		instance->altsetting = DEFAULT_ALTSETTING;
+	}
+
 	INIT_WORK(&instance->status_checker, (void *)speedtch_check_status, instance);
 
 	instance->status_checker.timer.function = speedtch_status_poll;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 10/13] USBATM: allow isochronous transfer
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (9 preceding siblings ...)
  2006-01-13  9:52 ` [PATCH 09/13] USBATM: measure buffer size in bytes; force valid sizes Duncan Sands
@ 2006-01-13  9:59 ` Duncan Sands
  2006-01-13 10:06 ` [PATCH 11/13] USBATM: handle urbs containing partial cells Duncan Sands
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13  9:59 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

While the usbatm core has had some support for using isoc urbs
for some time, there was no way for users to turn it on.  While
use of isoc transfer should still be considered experimental, it
now works well enough to let users turn it on.  Minidrivers signal
to the core that they want to use isoc transfer by setting the new
UDSL_USE_ISOC flag.  The speedtch minidriver gets a new module
parameter enable_isoc (defaults to false), plus some logic that
checks for the existence of an isoc receive endpoint (not all
speedtouch modems have one).

Signed-off-by: Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 10-isoc --]
[-- Type: text/x-diff, Size: 11945 bytes --]

Index: Linux/drivers/usb/atm/cxacru.c
===================================================================
--- Linux.orig/drivers/usb/atm/cxacru.c	2006-01-13 09:05:06.000000000 +0100
+++ Linux/drivers/usb/atm/cxacru.c	2006-01-13 09:12:53.000000000 +0100
@@ -836,8 +836,8 @@
 	.heavy_init	= cxacru_heavy_init,
 	.unbind		= cxacru_unbind,
 	.atm_start	= cxacru_atm_start,
-	.in		= CXACRU_EP_DATA,
-	.out		= CXACRU_EP_DATA,
+	.bulk_in	= CXACRU_EP_DATA,
+	.bulk_out	= CXACRU_EP_DATA,
 	.rx_padding	= 3,
 	.tx_padding	= 11,
 };
Index: Linux/drivers/usb/atm/speedtch.c
===================================================================
--- Linux.orig/drivers/usb/atm/speedtch.c	2006-01-13 09:08:53.000000000 +0100
+++ Linux/drivers/usb/atm/speedtch.c	2006-01-13 09:23:12.000000000 +0100
@@ -35,6 +35,8 @@
 #include <linux/slab.h>
 #include <linux/stat.h>
 #include <linux/timer.h>
+#include <linux/types.h>
+#include <linux/usb_ch9.h>
 #include <linux/workqueue.h>
 
 #include "usbatm.h"
@@ -66,24 +68,33 @@
 
 #define RESUBMIT_DELAY		1000	/* milliseconds */
 
-#define DEFAULT_ALTSETTING	1
+#define DEFAULT_BULK_ALTSETTING	1
+#define DEFAULT_ISOC_ALTSETTING	2
 #define DEFAULT_DL_512_FIRST	0
+#define DEFAULT_ENABLE_ISOC	0
 #define DEFAULT_SW_BUFFERING	0
 
-static int altsetting = DEFAULT_ALTSETTING;
+static unsigned int altsetting = 0; /* zero means: use the default */
 static int dl_512_first = DEFAULT_DL_512_FIRST;
+static int enable_isoc = DEFAULT_ENABLE_ISOC;
 static int sw_buffering = DEFAULT_SW_BUFFERING;
 
-module_param(altsetting, int, S_IRUGO | S_IWUSR);
+module_param(altsetting, uint, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(altsetting,
-		 "Alternative setting for data interface (default: "
-		 __MODULE_STRING(DEFAULT_ALTSETTING) ")");
+		"Alternative setting for data interface (bulk_default: "
+		__MODULE_STRING(DEFAULT_BULK_ALTSETTING) "; isoc_default: "
+		__MODULE_STRING(DEFAULT_ISOC_ALTSETTING) ")");
 
 module_param(dl_512_first, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(dl_512_first,
 		 "Read 512 bytes before sending firmware (default: "
 		 __MODULE_STRING(DEFAULT_DL_512_FIRST) ")");
 
+module_param(enable_isoc, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(enable_isoc,
+		"Use isochronous transfers if available (default: "
+		__MODULE_STRING(DEFAULT_ENABLE_ISOC) ")");
+
 module_param(sw_buffering, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(sw_buffering,
 		 "Enable software buffering (default: "
@@ -91,7 +102,8 @@
 
 #define INTERFACE_DATA		1
 #define ENDPOINT_INT		0x81
-#define ENDPOINT_DATA		0x07
+#define ENDPOINT_BULK_DATA	0x07
+#define ENDPOINT_ISOC_DATA	0x07
 #define ENDPOINT_FIRMWARE	0x05
 
 #define hex2int(c) ( (c >= '0') && (c <= '9') ? (c - '0') : ((c & 0xf) + 9) )
@@ -687,11 +699,12 @@
 			 const struct usb_device_id *id)
 {
 	struct usb_device *usb_dev = interface_to_usbdev(intf);
-	struct usb_interface *cur_intf;
+	struct usb_interface *cur_intf, *data_intf;
 	struct speedtch_instance_data *instance;
 	int ifnum = intf->altsetting->desc.bInterfaceNumber;
 	int num_interfaces = usb_dev->actconfig->desc.bNumInterfaces;
 	int i, ret;
+	int use_isoc;
 
 	usb_dbg(usbatm, "%s entered\n", __func__);
 
@@ -702,6 +715,11 @@
 		return -ENODEV;
 	}
 
+	if (!(data_intf = usb_ifnum_to_if(usb_dev, INTERFACE_DATA))) {
+		usb_err(usbatm, "%s: data interface not found!\n", __func__);
+		return -ENODEV;
+	}
+
 	/* claim all interfaces */
 
 	for (i=0; i < num_interfaces; i++) {
@@ -728,8 +746,9 @@
 
 	instance->usbatm = usbatm;
 
-	/* altsetting may change at any moment, so take a snapshot */
+	/* altsetting and enable_isoc may change at any moment, so take a snapshot */
 	instance->altsetting = altsetting;
+	use_isoc = enable_isoc;
 
 	if (instance->altsetting)
 		if ((ret = usb_set_interface(usb_dev, INTERFACE_DATA, instance->altsetting)) < 0) {
@@ -737,14 +756,44 @@
 			instance->altsetting = 0; /* fall back to default */
 		}
 
-	if (!instance->altsetting) {
-		if ((ret = usb_set_interface(usb_dev, INTERFACE_DATA, DEFAULT_ALTSETTING)) < 0) {
-			usb_err(usbatm, "%s: setting interface to %2d failed (%d)!\n", __func__, DEFAULT_ALTSETTING, ret);
-			goto fail_free;
+	if (!instance->altsetting && use_isoc)
+		if ((ret = usb_set_interface(usb_dev, INTERFACE_DATA, DEFAULT_ISOC_ALTSETTING)) < 0) {
+			usb_dbg(usbatm, "%s: setting interface to %2d failed (%d)!\n", __func__, DEFAULT_ISOC_ALTSETTING, ret);
+			use_isoc = 0; /* fall back to bulk */
 		}
-		instance->altsetting = DEFAULT_ALTSETTING;
+
+	if (use_isoc) {
+		const struct usb_host_interface *desc = data_intf->cur_altsetting;
+		const __u8 target_address = USB_DIR_IN | usbatm->driver->isoc_in;
+		int i;
+
+		use_isoc = 0; /* fall back to bulk if endpoint not found */
+
+		for (i=0; i<desc->desc.bNumEndpoints; i++) {
+			const struct usb_endpoint_descriptor *endpoint_desc = &desc->endpoint[i].desc;
+
+			if ((endpoint_desc->bEndpointAddress == target_address)) {
+				use_isoc = (endpoint_desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) ==
+					USB_ENDPOINT_XFER_ISOC;
+				break;
+			}
+		}
+
+		if (!use_isoc)
+			usb_info(usbatm, "isochronous transfer not supported - using bulk\n");
 	}
 
+	if (!use_isoc && !instance->altsetting)
+		if ((ret = usb_set_interface(usb_dev, INTERFACE_DATA, DEFAULT_BULK_ALTSETTING)) < 0) {
+			usb_err(usbatm, "%s: setting interface to %2d failed (%d)!\n", __func__, DEFAULT_BULK_ALTSETTING, ret);
+			goto fail_free;
+		}
+
+	if (!instance->altsetting)
+		instance->altsetting = use_isoc ? DEFAULT_ISOC_ALTSETTING : DEFAULT_BULK_ALTSETTING;
+
+	usbatm->flags |= (use_isoc ? UDSL_USE_ISOC : 0);
+
 	INIT_WORK(&instance->status_checker, (void *)speedtch_check_status, instance);
 
 	instance->status_checker.timer.function = speedtch_status_poll;
@@ -771,7 +820,7 @@
 			      0x12, 0xc0, 0x07, 0x00,
 			      instance->scratch_buffer + OFFSET_7, SIZE_7, 500);
 
-	usbatm->flags = (ret == SIZE_7 ? UDSL_SKIP_HEAVY_INIT : 0);
+	usbatm->flags |= (ret == SIZE_7 ? UDSL_SKIP_HEAVY_INIT : 0);
 
 	usb_dbg(usbatm, "%s: firmware %s loaded\n", __func__, usbatm->flags & UDSL_SKIP_HEAVY_INIT ? "already" : "not");
 
@@ -817,8 +866,9 @@
 	.unbind		= speedtch_unbind,
 	.atm_start	= speedtch_atm_start,
 	.atm_stop	= speedtch_atm_stop,
-	.in		= ENDPOINT_DATA,
-	.out		= ENDPOINT_DATA
+	.bulk_in	= ENDPOINT_BULK_DATA,
+	.bulk_out	= ENDPOINT_BULK_DATA,
+	.isoc_in	= ENDPOINT_ISOC_DATA
 };
 
 static int speedtch_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
Index: Linux/drivers/usb/atm/ueagle-atm.c
===================================================================
--- Linux.orig/drivers/usb/atm/ueagle-atm.c	2006-01-13 09:05:06.000000000 +0100
+++ Linux/drivers/usb/atm/ueagle-atm.c	2006-01-13 09:12:53.000000000 +0100
@@ -1705,8 +1705,8 @@
 	.atm_start = uea_atm_open,
 	.unbind = uea_unbind,
 	.heavy_init = uea_heavy,
-	.in = UEA_BULK_DATA_PIPE,
-	.out = UEA_BULK_DATA_PIPE,
+	.bulk_in = UEA_BULK_DATA_PIPE,
+	.bulk_out = UEA_BULK_DATA_PIPE,
 };
 
 static int uea_probe(struct usb_interface *intf, const struct usb_device_id *id)
Index: Linux/drivers/usb/atm/usbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.c	2006-01-13 09:05:06.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-13 09:12:53.000000000 +0100
@@ -1049,17 +1049,23 @@
 	init_completion(&instance->thread_exited);
 
 	INIT_LIST_HEAD(&instance->vcc_list);
+	skb_queue_head_init(&instance->sndqueue);
 
 	usbatm_init_channel(&instance->rx_channel);
 	usbatm_init_channel(&instance->tx_channel);
 	tasklet_init(&instance->rx_channel.tasklet, usbatm_rx_process, (unsigned long)instance);
 	tasklet_init(&instance->tx_channel.tasklet, usbatm_tx_process, (unsigned long)instance);
-	instance->rx_channel.endpoint = usb_rcvbulkpipe(usb_dev, driver->in);
-	instance->tx_channel.endpoint = usb_sndbulkpipe(usb_dev, driver->out);
 	instance->rx_channel.stride = ATM_CELL_SIZE + driver->rx_padding;
 	instance->tx_channel.stride = ATM_CELL_SIZE + driver->tx_padding;
 	instance->rx_channel.usbatm = instance->tx_channel.usbatm = instance;
 
+	if ((instance->flags & UDSL_USE_ISOC) && driver->isoc_in)
+		instance->rx_channel.endpoint = usb_rcvisocpipe(usb_dev, driver->isoc_in);
+	else
+		instance->rx_channel.endpoint = usb_rcvbulkpipe(usb_dev, driver->bulk_in);
+
+	instance->tx_channel.endpoint = usb_sndbulkpipe(usb_dev, driver->bulk_out);
+
 	/* tx buffer size must be a positive multiple of the stride */
 	instance->tx_channel.buf_size = max (instance->tx_channel.stride,
 			snd_buf_bytes - (snd_buf_bytes % instance->tx_channel.stride));
@@ -1080,6 +1086,7 @@
 		num_packets--;
 
 	instance->rx_channel.buf_size = num_packets * maxpacket;
+	instance->rx_channel.packet_size = maxpacket;
 
 #ifdef DEBUG
 	for (i = 0; i < 2; i++) {
@@ -1090,22 +1097,16 @@
 	}
 #endif
 
-	skb_queue_head_init(&instance->sndqueue);
+	/* initialize urbs */
 
 	for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
-		struct urb *urb;
 		u8 *buffer;
-		unsigned int iso_packets = 0, iso_size = 0;
 		struct usbatm_channel *channel = i < num_rcv_urbs ?
 			&instance->rx_channel : &instance->tx_channel;
+		struct urb *urb;
+		unsigned int iso_packets = usb_pipeisoc(channel->endpoint) ? channel->buf_size / channel->packet_size : 0;
 
-		if (usb_pipeisoc(channel->endpoint)) {
-			/* don't expect iso out endpoints */
-			iso_size = usb_maxpacket(instance->usb_dev, channel->endpoint, 0);
-			iso_size -= iso_size % channel->stride;	/* alignment */
-			BUG_ON(!iso_size);
-			iso_packets = (channel->buf_size - 1) / iso_size + 1;
-		}
+		UDSL_ASSERT(!usb_pipeisoc(channel->endpoint) || usb_pipein(channel->endpoint));
 
 		urb = usb_alloc_urb(iso_packets, GFP_KERNEL);
 		if (!urb) {
@@ -1132,9 +1133,8 @@
 			urb->transfer_flags = URB_ISO_ASAP;
 			urb->number_of_packets = iso_packets;
 			for (j = 0; j < iso_packets; j++) {
-				urb->iso_frame_desc[j].offset = iso_size * j;
-				urb->iso_frame_desc[j].length = min_t(int, iso_size,
-								      channel->buf_size - urb->iso_frame_desc[j].offset);
+				urb->iso_frame_desc[j].offset = channel->packet_size * j;
+				urb->iso_frame_desc[j].length = channel->packet_size;
 			}
 		}
 
Index: Linux/drivers/usb/atm/usbatm.h
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.h	2006-01-13 09:05:06.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.h	2006-01-13 09:12:53.000000000 +0100
@@ -87,6 +87,7 @@
 /* flags, set by mini-driver in bind() */
 
 #define UDSL_SKIP_HEAVY_INIT	(1<<0)
+#define UDSL_USE_ISOC		(1<<1)
 
 
 /* mini driver */
@@ -118,8 +119,9 @@
 	/* cleanup ATM device ... can sleep, but can't fail */
 	void (*atm_stop) (struct usbatm_data *, struct atm_dev *);
 
-        int in;		/* rx endpoint */
-        int out;	/* tx endpoint */
+        int bulk_in;	/* bulk rx endpoint */
+        int isoc_in;	/* isochronous rx endpoint */
+        int bulk_out;	/* bulk tx endpoint */
 
 	unsigned rx_padding;
 	unsigned tx_padding;
@@ -134,6 +136,7 @@
 	int endpoint;			/* usb pipe */
 	unsigned int stride;		/* ATM cell size + padding */
 	unsigned int buf_size;		/* urb buffer size */
+	unsigned int packet_size;	/* endpoint maxpacket */
 	spinlock_t lock;
 	struct list_head list;
 	struct tasklet_struct tasklet;
Index: Linux/drivers/usb/atm/xusbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/xusbatm.c	2006-01-13 09:05:06.000000000 +0100
+++ Linux/drivers/usb/atm/xusbatm.c	2006-01-13 09:12:53.000000000 +0100
@@ -210,8 +210,8 @@
 		xusbatm_drivers[i].bind		= xusbatm_bind;
 		xusbatm_drivers[i].unbind	= xusbatm_unbind;
 		xusbatm_drivers[i].atm_start	= xusbatm_atm_start;
-		xusbatm_drivers[i].in		= rx_endpoint[i];
-		xusbatm_drivers[i].out		= tx_endpoint[i];
+		xusbatm_drivers[i].bulk_in	= rx_endpoint[i];
+		xusbatm_drivers[i].bulk_out	= tx_endpoint[i];
 		xusbatm_drivers[i].rx_padding	= rx_padding[i];
 		xusbatm_drivers[i].tx_padding	= tx_padding[i];
 	}

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 11/13] USBATM: handle urbs containing partial cells
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (10 preceding siblings ...)
  2006-01-13  9:59 ` [PATCH 10/13] USBATM: allow isochronous transfer Duncan Sands
@ 2006-01-13 10:06 ` Duncan Sands
  2006-01-13 10:08 ` [PATCH 12/13] USBATM: bump version numbers Duncan Sands
  2006-01-13 10:12 ` [PATCH 13/13] USBATM: -EILSEQ workaround Duncan Sands
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13 10:06 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

The receive logic has always assumed that urbs contain an integral
number of ATM cells, which is a bit naughty, though it never caused
any problems with bulk transfers.  Isochronous urbs spank us soundly
for this.  Fixed thanks to this patch, mostly by Stanislaw Gruszka.

Signed-off-by: Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 11-merge --]
[-- Type: text/x-diff, Size: 12156 bytes --]

Index: Linux/drivers/usb/atm/usbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.c	2006-01-13 09:12:53.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-13 09:25:43.000000000 +0100
@@ -296,126 +296,159 @@
 	return NULL;
 }
 
-static void usbatm_extract_cells(struct usbatm_data *instance,
-			       unsigned char *source, unsigned int avail_data)
+static void usbatm_extract_one_cell(struct usbatm_data *instance, unsigned char *source)
 {
-	struct usbatm_vcc_data *cached_vcc = NULL;
 	struct atm_vcc *vcc;
 	struct sk_buff *sarb;
-	unsigned int stride = instance->rx_channel.stride;
-	int vci, cached_vci = 0;
-	short vpi, cached_vpi = 0;
-	u8 pti;
+	short vpi = ((source[0] & 0x0f) << 4)  | (source[1] >> 4);
+	int vci = ((source[1] & 0x0f) << 12) | (source[2] << 4) | (source[3] >> 4);
+	u8 pti = ((source[3] & 0xe) >> 1);
 
-	for (; avail_data >= stride; avail_data -= stride, source += stride) {
-		vpi = ((source[0] & 0x0f) << 4)  | (source[1] >> 4);
-		vci = ((source[1] & 0x0f) << 12) | (source[2] << 4) | (source[3] >> 4);
-		pti = ((source[3] & 0xe) >> 1);
+	vdbg("%s: vpi %hd, vci %d, pti %d", __func__, vpi, vci, pti);
 
-		vdbg("%s: vpi %hd, vci %d, pti %d", __func__, vpi, vci, pti);
+	if ((vci != instance->cached_vci) || (vpi != instance->cached_vpi)) {
+		instance->cached_vpi = vpi;
+		instance->cached_vci = vci;
 
-		if ((vci != cached_vci) || (vpi != cached_vpi)) {
-			cached_vpi = vpi;
-			cached_vci = vci;
+		instance->cached_vcc = usbatm_find_vcc(instance, vpi, vci);
 
-			cached_vcc = usbatm_find_vcc(instance, vpi, vci);
+		if (!instance->cached_vcc)
+			atm_rldbg(instance, "%s: unknown vpi/vci (%hd/%d)!\n", __func__, vpi, vci);
+	}
 
-			if (!cached_vcc)
-				atm_rldbg(instance, "%s: unknown vpi/vci (%hd/%d)!\n", __func__, vpi, vci);
-		}
+	if (!instance->cached_vcc)
+		return;
 
-		if (!cached_vcc)
-			continue;
+	vcc = instance->cached_vcc->vcc;
 
-		vcc = cached_vcc->vcc;
+	/* OAM F5 end-to-end */
+	if (pti == ATM_PTI_E2EF5) {
+		if (printk_ratelimit())
+			atm_warn(instance, "%s: OAM not supported (vpi %d, vci %d)!\n",
+				__func__, vpi, vci);
+		atomic_inc(&vcc->stats->rx_err);
+		return;
+	}
 
-		/* OAM F5 end-to-end */
-		if (pti == ATM_PTI_E2EF5) {
-			if (printk_ratelimit())
-				atm_warn(instance, "%s: OAM not supported (vpi %d, vci %d)!\n",
-					__func__, vpi, vci);
+	sarb = instance->cached_vcc->sarb;
+
+	if (sarb->tail + ATM_CELL_PAYLOAD > sarb->end) {
+		atm_rldbg(instance, "%s: buffer overrun (sarb->len %u, vcc: 0x%p)!\n",
+				__func__, sarb->len, vcc);
+		/* discard cells already received */
+		skb_trim(sarb, 0);
+		UDSL_ASSERT(sarb->tail + ATM_CELL_PAYLOAD <= sarb->end);
+	}
+
+	memcpy(sarb->tail, source + ATM_CELL_HEADER, ATM_CELL_PAYLOAD);
+	__skb_put(sarb, ATM_CELL_PAYLOAD);
+
+	if (pti & 1) {
+		struct sk_buff *skb;
+		unsigned int length;
+		unsigned int pdu_length;
+
+		length = (source[ATM_CELL_SIZE - 6] << 8) + source[ATM_CELL_SIZE - 5];
+
+		/* guard against overflow */
+		if (length > ATM_MAX_AAL5_PDU) {
+			atm_rldbg(instance, "%s: bogus length %u (vcc: 0x%p)!\n",
+				  __func__, length, vcc);
 			atomic_inc(&vcc->stats->rx_err);
-			continue;
+			goto out;
 		}
 
-		sarb = cached_vcc->sarb;
+		pdu_length = usbatm_pdu_length(length);
 
-		if (sarb->tail + ATM_CELL_PAYLOAD > sarb->end) {
-			atm_rldbg(instance, "%s: buffer overrun (sarb->len %u, vcc: 0x%p)!\n",
-					__func__, sarb->len, vcc);
-			/* discard cells already received */
-			skb_trim(sarb, 0);
-			UDSL_ASSERT(sarb->tail + ATM_CELL_PAYLOAD <= sarb->end);
-		}
-
-		memcpy(sarb->tail, source + ATM_CELL_HEADER, ATM_CELL_PAYLOAD);
-		__skb_put(sarb, ATM_CELL_PAYLOAD);
-
-		if (pti & 1) {
-			struct sk_buff *skb;
-			unsigned int length;
-			unsigned int pdu_length;
-
-			length = (source[ATM_CELL_SIZE - 6] << 8) + source[ATM_CELL_SIZE - 5];
-
-			/* guard against overflow */
-			if (length > ATM_MAX_AAL5_PDU) {
-				atm_rldbg(instance, "%s: bogus length %u (vcc: 0x%p)!\n",
-						__func__, length, vcc);
-				atomic_inc(&vcc->stats->rx_err);
-				goto out;
-			}
+		if (sarb->len < pdu_length) {
+			atm_rldbg(instance, "%s: bogus pdu_length %u (sarb->len: %u, vcc: 0x%p)!\n",
+				  __func__, pdu_length, sarb->len, vcc);
+			atomic_inc(&vcc->stats->rx_err);
+			goto out;
+		}
 
-			pdu_length = usbatm_pdu_length(length);
+		if (crc32_be(~0, sarb->tail - pdu_length, pdu_length) != 0xc704dd7b) {
+			atm_rldbg(instance, "%s: packet failed crc check (vcc: 0x%p)!\n",
+				  __func__, vcc);
+			atomic_inc(&vcc->stats->rx_err);
+			goto out;
+		}
 
-			if (sarb->len < pdu_length) {
-				atm_rldbg(instance, "%s: bogus pdu_length %u (sarb->len: %u, vcc: 0x%p)!\n",
-						__func__, pdu_length, sarb->len, vcc);
-				atomic_inc(&vcc->stats->rx_err);
-				goto out;
-			}
+		vdbg("%s: got packet (length: %u, pdu_length: %u, vcc: 0x%p)", __func__, length, pdu_length, vcc);
 
-			if (crc32_be(~0, sarb->tail - pdu_length, pdu_length) != 0xc704dd7b) {
-				atm_rldbg(instance, "%s: packet failed crc check (vcc: 0x%p)!\n",
-						__func__, vcc);
-				atomic_inc(&vcc->stats->rx_err);
-				goto out;
-			}
+		if (!(skb = dev_alloc_skb(length))) {
+			if (printk_ratelimit())
+				atm_err(instance, "%s: no memory for skb (length: %u)!\n",
+					__func__, length);
+			atomic_inc(&vcc->stats->rx_drop);
+			goto out;
+		}
 
-			vdbg("%s: got packet (length: %u, pdu_length: %u, vcc: 0x%p)", __func__, length, pdu_length, vcc);
+		vdbg("%s: allocated new sk_buff (skb: 0x%p, skb->truesize: %u)", __func__, skb, skb->truesize);
 
-			if (!(skb = dev_alloc_skb(length))) {
-				if (printk_ratelimit())
-					atm_err(instance, "%s: no memory for skb (length: %u)!\n",
-							__func__, length);
-				atomic_inc(&vcc->stats->rx_drop);
-				goto out;
-			}
+		if (!atm_charge(vcc, skb->truesize)) {
+			atm_rldbg(instance, "%s: failed atm_charge (skb->truesize: %u)!\n",
+				  __func__, skb->truesize);
+			dev_kfree_skb_any(skb);
+			goto out;	/* atm_charge increments rx_drop */
+		}
 
-			vdbg("%s: allocated new sk_buff (skb: 0x%p, skb->truesize: %u)", __func__, skb, skb->truesize);
+		memcpy(skb->data, sarb->tail - pdu_length, length);
+		__skb_put(skb, length);
 
-			if (!atm_charge(vcc, skb->truesize)) {
-				atm_rldbg(instance, "%s: failed atm_charge (skb->truesize: %u)!\n",
-						__func__, skb->truesize);
-				dev_kfree_skb_any(skb);
-				goto out;	/* atm_charge increments rx_drop */
-			}
+		vdbg("%s: sending skb 0x%p, skb->len %u, skb->truesize %u",
+		     __func__, skb, skb->len, skb->truesize);
 
-			memcpy(skb->data, sarb->tail - pdu_length, length);
-			__skb_put(skb, length);
+		PACKETDEBUG(skb->data, skb->len);
 
-			vdbg("%s: sending skb 0x%p, skb->len %u, skb->truesize %u",
-			     __func__, skb, skb->len, skb->truesize);
+		vcc->push(vcc, skb);
 
-			PACKETDEBUG(skb->data, skb->len);
+		atomic_inc(&vcc->stats->rx);
+	out:
+		skb_trim(sarb, 0);
+	}
+}
 
-			vcc->push(vcc, skb);
+static void usbatm_extract_cells(struct usbatm_data *instance,
+		unsigned char *source, unsigned int avail_data)
+{
+	unsigned int stride = instance->rx_channel.stride;
+	unsigned int buf_usage = instance->buf_usage;
 
-			atomic_inc(&vcc->stats->rx);
-		out:
-			skb_trim(sarb, 0);
+	/* extract cells from incoming data, taking into account that
+	 * the length of avail data may not be a multiple of stride */
+
+	if (buf_usage > 0) {
+		/* we have a partially received atm cell */
+		unsigned char *cell_buf = instance->cell_buf;
+		unsigned int space_left = stride - buf_usage;
+
+		UDSL_ASSERT(buf_usage <= stride);
+
+		if (avail_data >= space_left) {
+			/* add new data and process cell */
+			memcpy(cell_buf + buf_usage, source, space_left);
+			source += space_left;
+			avail_data -= space_left;
+			usbatm_extract_one_cell(instance, cell_buf);
+			instance->buf_usage = 0;
+		} else {
+			/* not enough data to fill the cell */
+			memcpy(cell_buf + buf_usage, source, avail_data);
+			instance->buf_usage = buf_usage + avail_data;
+			return;
 		}
 	}
+
+	for (; avail_data >= stride; avail_data -= stride, source += stride)
+		usbatm_extract_one_cell(instance, source);
+
+	if (avail_data > 0) {
+		/* length was not a multiple of stride -
+		 * save remaining data for next call */
+		memcpy(instance->cell_buf, source, avail_data);
+		instance->buf_usage = avail_data;
+	}
 }
 
 
@@ -496,16 +529,40 @@
 		vdbg("%s: processing urb 0x%p", __func__, urb);
 
 		if (usb_pipeisoc(urb->pipe)) {
+			unsigned char *merge_start = NULL;
+			unsigned int merge_length = 0;
+			const unsigned int packet_size = instance->rx_channel.packet_size;
 			int i;
-			for (i = 0; i < urb->number_of_packets; i++)
-				if (!urb->iso_frame_desc[i].status)
-					usbatm_extract_cells(instance,
-							     (u8 *)urb->transfer_buffer + urb->iso_frame_desc[i].offset,
-							     urb->iso_frame_desc[i].actual_length);
-		}
-		else
+
+			for (i = 0; i < urb->number_of_packets; i++) {
+				if (!urb->iso_frame_desc[i].status) {
+					unsigned int actual_length = urb->iso_frame_desc[i].actual_length;
+
+					UDSL_ASSERT(actual_length <= packet_size);
+
+					if (!merge_length)
+						merge_start = (unsigned char *)urb->transfer_buffer + urb->iso_frame_desc[i].offset;
+					merge_length += actual_length;
+					if (merge_length && (actual_length < packet_size)) {
+						usbatm_extract_cells(instance, merge_start, merge_length);
+						merge_length = 0;
+					}
+				} else {
+					atm_rldbg(instance, "%s: status %d in frame %d!\n", __func__, urb->status, i);
+					if (merge_length)
+						usbatm_extract_cells(instance, merge_start, merge_length);
+					merge_length = 0;
+					instance->buf_usage = 0;
+				}
+			}
+
+			if (merge_length)
+				usbatm_extract_cells(instance, merge_start, merge_length);
+		} else
 			if (!urb->status)
 				usbatm_extract_cells(instance, urb->transfer_buffer, urb->actual_length);
+			else
+				instance->buf_usage = 0;
 
 		if (usbatm_submit_urb(urb))
 			return;
@@ -797,6 +854,9 @@
 	vcc->dev_data = new;
 
 	tasklet_disable(&instance->rx_channel.tasklet);
+	instance->cached_vcc = new;
+	instance->cached_vpi = vpi;
+	instance->cached_vci = vci;
 	list_add(&new->list, &instance->vcc_list);
 	tasklet_enable(&instance->rx_channel.tasklet);
 
@@ -836,6 +896,11 @@
 	down(&instance->serialize);	/* vs self, usbatm_atm_open, usbatm_usb_disconnect */
 
 	tasklet_disable(&instance->rx_channel.tasklet);
+	if (instance->cached_vcc == vcc_data) {
+		instance->cached_vcc = NULL;
+		instance->cached_vpi = ATM_VPI_UNSPEC;
+		instance->cached_vci = ATM_VCI_UNSPEC;
+	}
 	list_del(&vcc_data->list);
 	tasklet_enable(&instance->rx_channel.tasklet);
 
@@ -1146,6 +1211,16 @@
 		     __func__, urb->transfer_buffer, urb->transfer_buffer_length, urb);
 	}
 
+	instance->cached_vpi = ATM_VPI_UNSPEC;
+	instance->cached_vci = ATM_VCI_UNSPEC;
+	instance->cell_buf = kmalloc(instance->rx_channel.stride, GFP_KERNEL);
+
+	if (!instance->cell_buf) {
+		dev_err(dev, "%s: no memory for cell buffer!\n", __func__);
+		error = -ENOMEM;
+		goto fail_unbind;
+	}
+
 	if (!(instance->flags & UDSL_SKIP_HEAVY_INIT) && driver->heavy_init) {
 		error = usbatm_heavy_init(instance);
 	} else {
@@ -1165,6 +1240,8 @@
 	if (instance->driver->unbind)
 		instance->driver->unbind(instance, intf);
  fail_free:
+	kfree(instance->cell_buf);
+
 	for (i = 0; i < num_rcv_urbs + num_snd_urbs; i++) {
 		if (instance->urbs[i])
 			kfree(instance->urbs[i]->transfer_buffer);
@@ -1236,6 +1313,8 @@
 		usb_free_urb(instance->urbs[i]);
 	}
 
+	kfree(instance->cell_buf);
+
 	/* ATM finalize */
 	if (instance->atm_dev)
 		atm_dev_deregister(instance->atm_dev);
Index: Linux/drivers/usb/atm/usbatm.h
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.h	2006-01-13 09:12:53.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.h	2006-01-13 09:25:43.000000000 +0100
@@ -187,6 +187,13 @@
 	struct sk_buff_head sndqueue;
 	struct sk_buff *current_skb;	/* being emptied */
 
+	struct usbatm_vcc_data *cached_vcc;
+	int cached_vci;
+	short cached_vpi;
+
+	unsigned char *cell_buf;	/* holds partial rx cell */
+	unsigned int buf_usage;
+
 	struct urb *urbs[0];
 };
 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 12/13] USBATM: bump version numbers
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (11 preceding siblings ...)
  2006-01-13 10:06 ` [PATCH 11/13] USBATM: handle urbs containing partial cells Duncan Sands
@ 2006-01-13 10:08 ` Duncan Sands
  2006-01-13 10:12 ` [PATCH 13/13] USBATM: -EILSEQ workaround Duncan Sands
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13 10:08 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

Signed-off-by: Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 12-version --]
[-- Type: text/x-diff, Size: 1116 bytes --]

Index: Linux/drivers/usb/atm/speedtch.c
===================================================================
--- Linux.orig/drivers/usb/atm/speedtch.c	2006-01-13 09:23:12.000000000 +0100
+++ Linux/drivers/usb/atm/speedtch.c	2006-01-13 09:27:55.000000000 +0100
@@ -42,7 +42,7 @@
 #include "usbatm.h"
 
 #define DRIVER_AUTHOR	"Johan Verrept, Duncan Sands <duncan.sands@free.fr>"
-#define DRIVER_VERSION	"1.9"
+#define DRIVER_VERSION	"1.10"
 #define DRIVER_DESC	"Alcatel SpeedTouch USB driver version " DRIVER_VERSION
 
 static const char speedtch_driver_name[] = "speedtch";
Index: Linux/drivers/usb/atm/usbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.c	2006-01-13 09:25:43.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-13 09:27:55.000000000 +0100
@@ -92,7 +92,7 @@
 #endif
 
 #define DRIVER_AUTHOR	"Johan Verrept, Duncan Sands <duncan.sands@free.fr>"
-#define DRIVER_VERSION	"1.9"
+#define DRIVER_VERSION	"1.10"
 #define DRIVER_DESC	"Generic USB ATM/DSL I/O, version " DRIVER_VERSION
 
 static const char usbatm_driver_name[] = "usbatm";

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 13/13] USBATM: -EILSEQ workaround
  2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
                   ` (12 preceding siblings ...)
  2006-01-13 10:08 ` [PATCH 12/13] USBATM: bump version numbers Duncan Sands
@ 2006-01-13 10:12 ` Duncan Sands
  13 siblings, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13 10:12 UTC (permalink / raw)
  To: Greg KH; +Cc: usbatm, linux-usb-devel, linux-kernel

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

Don't throttle on -EILSEQ urb status if requested by a minidriver.
It seems the ueagle modems are buggy, giving -EILSEQ when they
have no data to send.  The ueagle change will be sent separately
by the ueagle guys.  Patch by Matthieu Castet.

Signed-off-by: Duncan Sands <baldrick@free.fr>

[-- Attachment #2: 13-EILSEQ --]
[-- Type: text/x-diff, Size: 1000 bytes --]

Index: Linux/drivers/usb/atm/usbatm.c
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.c	2006-01-13 09:27:55.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.c	2006-01-13 09:28:29.000000000 +0100
@@ -270,7 +270,10 @@
 
 	spin_unlock_irqrestore(&channel->lock, flags);
 
-	if (unlikely(urb->status)) {
+	if (unlikely(urb->status) &&
+			(!(channel->usbatm->flags & UDSL_IGNORE_EILSEQ) ||
+			 urb->status != -EILSEQ ))
+	{
 		if (printk_ratelimit())
 			atm_warn(channel->usbatm, "%s: urb 0x%p failed (%d)!\n",
 				__func__, urb, urb->status);
Index: Linux/drivers/usb/atm/usbatm.h
===================================================================
--- Linux.orig/drivers/usb/atm/usbatm.h	2006-01-13 09:25:43.000000000 +0100
+++ Linux/drivers/usb/atm/usbatm.h	2006-01-13 09:28:29.000000000 +0100
@@ -88,6 +88,7 @@
 
 #define UDSL_SKIP_HEAVY_INIT	(1<<0)
 #define UDSL_USE_ISOC		(1<<1)
+#define UDSL_IGNORE_EILSEQ	(1<<2)
 
 
 /* mini driver */

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [linux-usb-devel] Re: [PATCH 00/13] USBATM: summary
  2006-01-12 18:30 ` [PATCH 00/13] USBATM: summary Greg KH
  2006-01-12 19:15   ` Duncan Sands
@ 2006-01-13 10:30   ` Duncan Sands
  1 sibling, 0 replies; 19+ messages in thread
From: Duncan Sands @ 2006-01-13 10:30 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-usb-devel, linux-kernel, usbatm

> I only got 2 of these, is my mail just being slow (which it does at odd
> times), or did you stop sending them based on some problems on your end?

Hi Greg, I stopped because I noticed that part of patch 2 had slipped
into patch 3 (the bit that slipped was exactly the tweak I made to patch 2
to have it compile by itself... probably ended up in patch 3 due to a
forgotten "quilt refresh"), and I wanted to check all the other patches.
They are fine, so I have now sent them.  However, patch 2 won't compile
unless you apply patch 3 as well. Since they are both trivial, I hope you
are ok with that, otherwise I can rediff them

All the best,

Duncan.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* R: [PATCH 06/13] USBATM: shutdown open connections when disconnected
  2006-01-13  9:05 ` [PATCH 06/13] USBATM: shutdown open connections when disconnected Duncan Sands
@ 2006-08-18 19:40   ` Giampaolo Tomassoni
  0 siblings, 0 replies; 19+ messages in thread
From: Giampaolo Tomassoni @ 2006-08-18 19:40 UTC (permalink / raw)
  To: Duncan Sands, Greg KH
  Cc: linux-kernel, linux-atm-general, usbatm, linux-usb-devel

> -----Messaggio originale-----
> Da: usbatm-bounces@lists.infradead.org
> [mailto:usbatm-bounces@lists.infradead.org]Per conto di Duncan Sands
> Inviato: venerdì 13 gennaio 2006 10.05
> A: Greg KH
> Cc: linux-kernel@vger.kernel.org;
> linux-atm-general@lists.sourceforge.net; usbatm@lists.infradead.org;
> linux-usb-devel@lists.sourceforge.net
> Oggetto: [PATCH 06/13] USBATM: shutdown open connections when
> disconnected
> 
> 
> This patch causes vcc_release_async to be applied to any open
> vcc's when the modem is disconnected.  This signals a socket
> shutdown, letting the socket user know that the game is up.
> I wrote this patch because of reports that pppd would keep
> connections open forever when the modem is disconnected.
> This patch does not fix that problem, but it's a step in the
> right direction.  It doesn't help because the pppoatm module
> doesn't yet monitor state changes on the ATM socket, so simply
> never realises that the ATM connection has gone down (meaning
> it doesn't tell the ppp layer).  But at least there is a socket
> state change now.

> Unfortunately this patch may create problems
> for those rare users like me who use routed IP or some other
> non-ppp connection method that goes via the ATM ARP daemon: the
> daemon is buggy, and with this patch will crash when the modem
> is disconnected.  Users with a buggy atmarpd can simply restart
> it after disconnecting the modem.

The problem is not only atmarpd, but also atmarp: it fails when establishing a PVC on a 'disconnected' VC, ie: a VC on and ADSL modem which didn't yet reach showtime.

CLIP and other protocols-over-ATM are designed to be connectionless. Thereby, you setup a CLIP connection, probably at systems startup, and forget about it: if a carrier is available, cells are exchanged, otherwise both parties discard outgoing cells. The connectionless design is one of the pros of CLIP with respect to, in example, PPPoA. It is not a con.

With this patch, the CLIP protocol becomes susceptible to connection status: it fails during setup if the ADSL modem is not yet at showtime (and who knows how long it will take to get it) and may discard the VC when the carrier happens to get lost after CLIP is started.

To my knowledge, no other ATM interface driver under Linux does reject a vcc open or a send due to the state of the physical carrier. Also note that there is not a per-interface instance of atmarpd and friends: they are scheduled systemwide and they may work at once on more interfaces. Making them die causes troubles not only to the outstanding network activity of the disconnected ADSL, but also to the one on other ATM interfaces.

pppd is designed to work both with connectionless (a cross-wire cable or an ATM VC) and connection-oriented carriers (a voice or ISDN modem): enabling its lcp echo feature usually is enough in order to detect a lost carrier. Out-of-sync peers state should be also detected. If it doesn't, it's pppd failure.

In summary, I'm not shure that the new USBATM behaviour is compatible with the (at least de-facto) actual standard of the Linux ATM stack. I'm not even shure that's compatible with ATM blueprints.

I'm however pretty shure that, when there is a patch that may impair and/or be incompatible with current software, the Linux end-user had always been provided with an option to disable the new feature and continue working the way it was used to.

I need that option.

Regards,

	giampaolo

> 
> Signed-off-by: Duncan Sands <baldrick@free.fr>
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2006-08-18 19:44 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-12 16:29 [PATCH 00/13] USBATM: summary Duncan Sands
2006-01-12 16:43 ` [PATCH 01/13] USBATM: trivial modifications Duncan Sands
2006-01-12 17:40   ` Duncan Sands
2006-01-12 17:48 ` [PATCH 02/13] USBATM: add flags field Duncan Sands
2006-01-12 18:30 ` [PATCH 00/13] USBATM: summary Greg KH
2006-01-12 19:15   ` Duncan Sands
2006-01-13 10:30   ` [linux-usb-devel] " Duncan Sands
2006-01-13  8:36 ` [PATCH 03/13] USBATM: remove .owner Duncan Sands
2006-01-13  8:38 ` [PATCH 04/13] USBATM: kzalloc conversion Duncan Sands
2006-01-13  8:48 ` [PATCH 05/13] USBATM: xusbatm rewrite Duncan Sands
2006-01-13  9:05 ` [PATCH 06/13] USBATM: shutdown open connections when disconnected Duncan Sands
2006-08-18 19:40   ` R: " Giampaolo Tomassoni
2006-01-13  9:07 ` [PATCH 07/13] USBATM: return correct error code when out of memory Duncan Sands
2006-01-13  9:13 ` [PATCH 08/13] USBATM: use dev_kfree_skb_any rather than dev_kfree_skb Duncan Sands
2006-01-13  9:52 ` [PATCH 09/13] USBATM: measure buffer size in bytes; force valid sizes Duncan Sands
2006-01-13  9:59 ` [PATCH 10/13] USBATM: allow isochronous transfer Duncan Sands
2006-01-13 10:06 ` [PATCH 11/13] USBATM: handle urbs containing partial cells Duncan Sands
2006-01-13 10:08 ` [PATCH 12/13] USBATM: bump version numbers Duncan Sands
2006-01-13 10:12 ` [PATCH 13/13] USBATM: -EILSEQ workaround Duncan Sands

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).