speedtch speedtch.c,1.53,1.54

Duncan Sands duncan at infradead.org
Mon Apr 25 17:47:34 EDT 2005


Update of /home/cvs/speedtch
In directory phoenix.infradead.org:/tmp/cvs-serv23121

Modified Files:
	speedtch.c 
Log Message:
Resubmit the int_urb after a one-second delay if something goes wrong with
it.  This is to avoid tight loops without giving up on line monitoring.
There could be some small races with line status changes depending on how
the int urb stuff is implemented in the modem: if the line status changes
but the int urb has not been submitted yet, do you get to see the change?
I don't know (yet), so I've tried to make this even less likely by scheduling
a status check after the int urb is submitted, so if the int urb missed a
line change then we'll hopefully see it with the status check.


Index: speedtch.c
===================================================================
RCS file: /home/cvs/speedtch/speedtch.c,v
retrieving revision 1.53
retrieving revision 1.54
diff -u -r1.53 -r1.54
--- speedtch.c	25 Apr 2005 15:53:02 -0000	1.53
+++ speedtch.c	25 Apr 2005 21:47:30 -0000	1.54
@@ -61,35 +61,37 @@
 #define SIZE_e		1
 #define SIZE_f		1
 
-#define SPEEDTCH_MIN_POLL_DELAY		5000	/* milliseconds */
-#define SPEEDTCH_MAX_POLL_DELAY		60000	/* milliseconds */
+#define MIN_POLL_DELAY		5000	/* milliseconds */
+#define MAX_POLL_DELAY		60000	/* milliseconds */
 
-#define SPEEDTCH_DEFAULT_ALTSETTING	1
-#define SPEEDTCH_DEFAULT_DL_512_FIRST	0
-#define SPEEDTCH_DEFAULT_SW_BUFFERING	0
-
-static int altsetting = SPEEDTCH_DEFAULT_ALTSETTING;
-static int dl_512_first = SPEEDTCH_DEFAULT_DL_512_FIRST;
-static int sw_buffering = SPEEDTCH_DEFAULT_SW_BUFFERING;
+#define RESUBMIT_DELAY		1000	/* milliseconds */
+
+#define DEFAULT_ALTSETTING	1
+#define DEFAULT_DL_512_FIRST	0
+#define DEFAULT_SW_BUFFERING	0
+
+static int altsetting = DEFAULT_ALTSETTING;
+static int dl_512_first = DEFAULT_DL_512_FIRST;
+static int sw_buffering = DEFAULT_SW_BUFFERING;
 
 module_param(altsetting, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(altsetting,
 		 "Alternative setting for data interface (default: "
-		 __MODULE_STRING(SPEEDTCH_DEFAULT_ALTSETTING) ")");
+		 __MODULE_STRING(DEFAULT_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(SPEEDTCH_DEFAULT_DL_512_FIRST) ")");
+		 __MODULE_STRING(DEFAULT_DL_512_FIRST) ")");
 
 module_param(sw_buffering, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(sw_buffering,
 		 "Enable software buffering (default: "
-		 __MODULE_STRING(SPEEDTCH_DEFAULT_SW_BUFFERING) ")");
+		 __MODULE_STRING(DEFAULT_SW_BUFFERING) ")");
 
-#define SPEEDTCH_ENDPOINT_INT		0x81
-#define SPEEDTCH_ENDPOINT_DATA		0x07
-#define SPEEDTCH_ENDPOINT_FIRMWARE	0x05
+#define ENDPOINT_INT		0x81
+#define ENDPOINT_DATA		0x07
+#define ENDPOINT_FIRMWARE	0x05
 
 #define hex2int(c) ( (c >= '0') && (c <= '9') ? (c - '0') : ((c & 0xf) + 9) )
 
@@ -100,6 +102,7 @@
 
 	int poll_delay; /* milliseconds */
 
+	struct timer_list resubmit_timer;
 	struct urb *int_urb;
 	unsigned char int_data[16];
 
@@ -196,7 +199,7 @@
 
 	/* URB 7 */
 	if (dl_512_first) {	/* some modems need a read before writing the firmware */
-		ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, SPEEDTCH_ENDPOINT_FIRMWARE),
+		ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
 				   buffer, 0x200, &actual_length, 2000);
 
 		if (ret < 0 && ret != -ETIMEDOUT)
@@ -210,7 +213,7 @@
 		int thislen = min_t(int, PAGE_SIZE, fw1->size - offset);
 		memcpy(buffer, fw1->data + offset, thislen);
 
-		ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, SPEEDTCH_ENDPOINT_FIRMWARE),
+		ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
 				   buffer, thislen, &actual_length, DATA_TIMEOUT);
 
 		if (ret < 0) {
@@ -223,7 +226,7 @@
 	/* USB led blinking green, ADSL led off */
 
 	/* URB 11 */
-	ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, SPEEDTCH_ENDPOINT_FIRMWARE),
+	ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
 			   buffer, 0x200, &actual_length, DATA_TIMEOUT);
 
 	if (ret < 0) {
@@ -237,7 +240,7 @@
 		int thislen = min_t(int, PAGE_SIZE, fw2->size - offset);
 		memcpy(buffer, fw2->data + offset, thislen);
 
-		ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, SPEEDTCH_ENDPOINT_FIRMWARE),
+		ret = usb_bulk_msg(usb_dev, usb_sndbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
 				   buffer, thislen, &actual_length, DATA_TIMEOUT);
 
 		if (ret < 0) {
@@ -250,7 +253,7 @@
 	/* USB led static green, ADSL led static red */
 
 	/* URB 142 */
-	ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, SPEEDTCH_ENDPOINT_FIRMWARE),
+	ret = usb_bulk_msg(usb_dev, usb_rcvbulkpipe(usb_dev, ENDPOINT_FIRMWARE),
 			   buffer, 0x200, &actual_length, DATA_TIMEOUT);
 
 	if (ret < 0) {
@@ -427,12 +430,12 @@
 	ret = speedtch_read_status(instance);
 	if (ret < 0) {
 		atm_warn(usbatm, "error %d fetching device status\n", ret);
-		if (instance->poll_delay < SPEEDTCH_MAX_POLL_DELAY)
+		if (instance->poll_delay < MAX_POLL_DELAY)
 			instance->poll_delay *= 2;
 		return;
 	}
 
-	if (instance->poll_delay > SPEEDTCH_MIN_POLL_DELAY)
+	if (instance->poll_delay > MIN_POLL_DELAY)
 		instance->poll_delay /= 2;
 
 	atm_dbg(usbatm, "%s: line state %02x\n", __func__, buf[OFFSET_7]);
@@ -498,27 +501,49 @@
 	schedule_work(&instance->status_checker);
 
 	/* The following check is racy, but the race is harmless */
-	if (instance->poll_delay < SPEEDTCH_MAX_POLL_DELAY)
+	if (instance->poll_delay < MAX_POLL_DELAY)
 		mod_timer(&instance->status_checker.timer, jiffies + msecs_to_jiffies(instance->poll_delay));
 	else
 		atm_warn(instance->usbatm, "Too many failures - disabling line status polling\n");
 }
 
-static void speedtch_handle_int(struct urb *urb, struct pt_regs *regs)
+static void speedtch_resubmit_int(unsigned long data)
 {
-	struct speedtch_instance_data *instance = urb->context;
+	struct speedtch_instance_data *instance = (void *)data;
 	struct usbatm_data *usbatm = instance->usbatm;
-	unsigned int count = urb->actual_length;
+	struct urb *int_urb = instance->int_urb;
 	int ret;
 
+	atm_dbg(usbatm, "%s entered\n", __func__);
+
+	if (int_urb) {
+		ret = usb_submit_urb(int_urb, GFP_ATOMIC);
+		if (!ret)
+			schedule_work(&instance->status_checker);
+		else {
+			atm_dbg(usbatm, "%s: usb_submit_urb failed with result %d\n", __func__, ret);
+			mod_timer(&instance->resubmit_timer, jiffies + msecs_to_jiffies(RESUBMIT_DELAY));
+		}
+	}
+}
+
+static void speedtch_handle_int(struct urb *int_urb, struct pt_regs *regs)
+{
+	struct speedtch_instance_data *instance = int_urb->context;
+	struct usbatm_data *usbatm = instance->usbatm;
+	unsigned int count = int_urb->actual_length;
+	int ret = int_urb->status;
+
 	/* The magic interrupt for "up state" */
 	const static unsigned char up_int[6]   = { 0xa1, 0x00, 0x01, 0x00, 0x00, 0x00 };
 	/* The magic interrupt for "down state" */
 	const static unsigned char down_int[6] = { 0xa1, 0x00, 0x00, 0x00, 0x00, 0x00 };
 
-	if (urb->status) {
-		atm_dbg(usbatm, "%s: nonzero urb status %d!\n", __func__, urb->status);
-		return;
+	atm_dbg(usbatm, "%s entered\n", __func__);
+
+	if (ret < 0) {
+		atm_dbg(usbatm, "%s: nonzero urb status %d!\n", __func__, ret);
+		goto fail;
 	}
 
 	if ((count == 6) && !memcmp(up_int, instance->int_data, 6)) {
@@ -533,13 +558,23 @@
 		for (i = 0; i < count; i++)
 			printk(" %02x", instance->int_data[i]);
 		printk("\n");
+		goto fail;
 	}
 
-	schedule_work(&instance->status_checker);
+	if ((int_urb = instance->int_urb)) {
+		ret = usb_submit_urb(int_urb, GFP_ATOMIC);
+		schedule_work(&instance->status_checker);
+		if (ret < 0) {
+			atm_dbg(usbatm, "%s: usb_submit_urb failed with result %d\n", __func__, ret);
+			goto fail;
+		}
+	}
 
-	ret = usb_submit_urb(urb, GFP_ATOMIC);
-	if (ret < 0)
-		atm_dbg(usbatm, "%s: usb_submit_urb failed with result %d\n", __func__, ret);
+	return;
+
+fail:
+	if ((int_urb = instance->int_urb))
+		mod_timer(&instance->resubmit_timer, jiffies + msecs_to_jiffies(RESUBMIT_DELAY));
 }
 
 static int speedtch_atm_start(struct usbatm_data *usbatm, struct atm_dev *atm_dev)
@@ -556,7 +591,7 @@
 		return ret;
 	}
 
-	/* set MAC address, it is stored in the serial number */
+	/* 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) {
 		for (i = 0; i < 6; i++)
@@ -586,12 +621,28 @@
 static void speedtch_atm_stop(struct usbatm_data *usbatm, struct atm_dev *atm_dev)
 {
 	struct speedtch_instance_data *instance = usbatm->driver_data;
+	struct urb *int_urb = instance->int_urb;
 
 	atm_dbg(usbatm, "%s entered\n", __func__);
 
 	del_timer_sync(&instance->status_checker.timer);
-	usb_kill_urb(instance->int_urb);
-	wmb();
+
+	/*
+	 * Since resubmit_timer and int_urb can schedule themselves and
+	 * each other, shutting them down correctly takes some care
+	 */
+	instance->int_urb = NULL; /* signal shutdown */
+	mb();
+	usb_kill_urb(int_urb);
+	del_timer_sync(&instance->resubmit_timer);
+	/*
+	 * At this point, speedtch_handle_int and speedtch_resubmit_int
+	 * can run or be running, but instance->int_urb == NULL means that
+	 * they will not reschedule
+	 */
+	usb_kill_urb(int_urb);
+	del_timer_sync(&instance->resubmit_timer);
+
 	flush_scheduled_work();
 }
 
@@ -679,13 +730,17 @@
 
 	instance->status_checker.timer.function = speedtch_status_poll;
 	instance->status_checker.timer.data = (unsigned long)instance;
-	instance->poll_delay = SPEEDTCH_MIN_POLL_DELAY;
+	instance->poll_delay = MIN_POLL_DELAY;
+
+	init_timer(&instance->resubmit_timer);
+	instance->resubmit_timer.function = speedtch_resubmit_int;
+	instance->resubmit_timer.data = (unsigned long)instance;
 
 	instance->int_urb = usb_alloc_urb(0, GFP_KERNEL);
 
 	if (instance->int_urb)
 		usb_fill_int_urb(instance->int_urb, usb_dev,
-				 usb_rcvintpipe(usb_dev, SPEEDTCH_ENDPOINT_INT),
+				 usb_rcvintpipe(usb_dev, ENDPOINT_INT),
 				 instance->int_data, sizeof(instance->int_data),
 				 speedtch_handle_int, instance, 50);
 	else
@@ -741,8 +796,8 @@
 	.unbind		= speedtch_unbind,
 	.atm_start	= speedtch_atm_start,
 	.atm_stop	= speedtch_atm_stop,
-	.in		= SPEEDTCH_ENDPOINT_DATA,
-	.out		= SPEEDTCH_ENDPOINT_DATA
+	.in		= ENDPOINT_DATA,
+	.out		= ENDPOINT_DATA
 };
 
 static int speedtch_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)




More information about the Usbatm-commits mailing list