[Asterisk-Dev] zaptel/wcusb fixes

Wichert Akkerman wichert at wiggy.net
Mon Jun 7 04:43:56 MST 2004


I still haven't managed to get wcusb working but at least I'm making
some progress in improving things with the help of #kernelnewbiews.
Below is my current patch. Changes:

* drop support for < 2.4.20 USB stack. If anyone is running a kernel
  that old they deserve problems and keeping all the legacy cruft around
  makes the driver harder to read

* use usb_fill_control_urb in prepare_transfer_urbs. This makes the
  code simpler and also makes sure all relevant fields are initalized
  properly (the spinklock wasn't initalized for example)

* Add a #define for the read and write interfaces instead of keeping the
  magic numbers 6 and 7 in the source

Some comments:

* the code still submits bogus URBs; at some point an URB without a
  completion handler is submitted

* the completion handlers blindly resubmit URBs without checking for
  failures, which is a good way to enter a tight loop

Wichert.

diff -wur org/zaptel-0.9.1/wcfxsusb.c zaptel-0.9.1/wcfxsusb.c
--- org/zaptel-0.9.1/wcfxsusb.c	2004-04-11 00:11:33.000000000 +0200
+++ zaptel-0.9.1/wcfxsusb.c	2004-06-07 13:13:12.000000000 +0200
@@ -40,9 +40,6 @@
 #include <linux/errno.h>
 
 #include <linux/version.h>
-#if LINUX_VERSION_CODE > KERNEL_VERSION(2,4,19)
-#define USB2420
-#endif
 
 #ifdef STANDALONE_ZAPATA
 
@@ -138,14 +135,8 @@
 static int debug = 0;
 
 struct stinky_urb {
-#ifdef USB2420
 	struct urb urb;
 	struct iso_packet_descriptor isoframe[1];
-#else
-	urb_t urb;
-	iso_packet_descriptor_t isoframe[1];
-#endif
-
 };
 
 typedef enum {
@@ -175,11 +166,7 @@
 struct wc_keypad_data {
 	/* Keypad state monitoring variables */
 	keypad_state_t state;
-#ifdef USB2420
 	struct urb urb;
-#else
-	urb_t urb;
-#endif
 	int running;
 	char data;
 	char data12;
@@ -211,16 +198,9 @@
 	struct stinky_urb dataread[2];
 	struct stinky_urb datawrite[2];
 	int iostate;	/* Whether reads/writes are complete */
-#ifdef USB2420
 	struct urb *pendingurb;	/* Pending URB for transmission */
 	struct urb 		control;
 	struct usb_ctrlrequest	dr;
-#else
-
-	urb_t *pendingurb;	/* Pending URB for transmission */
-	urb_t 		control;
-	devrequest	dr;
-#endif
 	control_state_t controlstate;
 	int urbcount;
 	int flags;
@@ -304,15 +284,9 @@
 	return 0;
 }					  
 
-#ifdef USB2420
 static int wcusb_async_read(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb));
 static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb));
 static void wcusb_async_control(struct urb *urb);
-#else
-static int wcusb_async_read(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(urb_t *urb));
-static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(urb_t *urb));
-static void wcusb_async_control(urb_t *urb);
-#endif
 
 static void proslic_read_direct_async(struct wc_usb_pvt *p, unsigned char address)
 {
@@ -334,11 +308,7 @@
 	wcusb_async_write(p, WCUSB_SPORT0, p->wcregbuf, 4, STATE_WCWRITE_WRITERES, wcusb_async_control);
 }
 
-#ifdef USB2420
 static void wcusb_async_control(struct urb *urb)
-#else
-static void wcusb_async_control(urb_t *urb)
-#endif
 {
 	struct wc_usb_pvt *p = urb->context;
 	p->urbcount--;
@@ -421,11 +391,7 @@
 	}
 }
 
-#ifdef USB2420
 static void keypad_check_done(struct urb *urb)
-#else
-static void keypad_check_done(urb_t *urb)
-#endif
 {
 	struct wc_usb_pvt *p = urb->context;
 	struct wc_keypad_data *d = p->pvt_data;
@@ -534,15 +500,10 @@
 	return;
 }
 
-#ifdef USB2420
 static int wcusb_async_read(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complyyete)(struct urb *urb))
-#else
-static int wcusb_async_read(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(urb_t *urb))
-#endif
 {
 	__u16 size = len;
 	__u16 ind = index;
-#ifdef USB2420
 	struct urb *urb = &p->control;
 	memset(urb, 0, sizeof(struct urb));
 
@@ -551,16 +512,6 @@
 	p->dr.wValue = 0;
 	p->dr.wIndex = cpu_to_le16(ind);
 	p->dr.wLength = cpu_to_le16(size);
-#else
-	urb_t *urb = &p->control;
-	memset(urb, 0, sizeof(urb_t));
-
-	p->dr.requesttype = USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-	p->dr.request = REQUEST_NORMAL;
-	p->dr.value = 0;
-	p->dr.index = cpu_to_le16(ind);
-	p->dr.length = cpu_to_le16(size);
-#endif
 
 	FILL_CONTROL_URB(urb, p->dev, usb_rcvctrlpipe(p->dev, 0), (unsigned char *)&p->dr, data, len, complete, p);
 	if (usb_submit_urb(urb)) {
@@ -572,15 +523,10 @@
 	return 0;
 }
 
-#ifdef USB2420
 static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb))
-#else
-static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(urb_t *urb))
-#endif
 {
 	__u16 size = len;
 	__u16 ind = index;
-#ifdef USB2420
 	struct urb *urb = &p->control;
 	memset(urb, 0, sizeof(struct urb));
 
@@ -589,16 +535,6 @@
 	p->dr.wValue = 0;
 	p->dr.wIndex = cpu_to_le16(ind);
 	p->dr.wLength = cpu_to_le16(size);
-#else
-	urb_t *urb = &p->control;
-	memset(urb, 0, sizeof(urb_t));
-
-	p->dr.requesttype = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-	p->dr.request = REQUEST_NORMAL;
-	p->dr.value = 0;
-	p->dr.index = cpu_to_le16(ind);
-	p->dr.length = cpu_to_le16(size);
-#endif
 
 	FILL_CONTROL_URB(urb, p->dev, usb_sndctrlpipe(p->dev, 0), (unsigned char *)&p->dr, data, len, complete, p);
 	if (usb_submit_urb(urb)) {
diff -wur org/zaptel-0.9.1/wcusb.c zaptel-0.9.1/wcusb.c
--- org/zaptel-0.9.1/wcusb.c	2004-01-06 14:26:44.000000000 +0100
+++ zaptel-0.9.1/wcusb.c	2004-06-07 13:35:52.000000000 +0200
@@ -40,10 +40,6 @@
 #include <linux/errno.h>
 
 #include <linux/version.h>
-#if LINUX_VERSION_CODE > KERNEL_VERSION(2,4,19)
-#define USB2420
-#endif
-
 #ifdef STANDALONE_ZAPATA
 
 #include "zaptel.h"
@@ -54,10 +50,6 @@
 #include "wcusb.h"
 #include "proslic.h"
 
-#ifndef FILL_CONTROL_URB
-#define FILL_CONTROL_URB usb_fill_control_urb
-#endif
-
 #ifdef DEBUG_WILDCARD
 #define DPRINTK(x) printk x
 #else
@@ -121,6 +113,11 @@
 {42,"DCDC_XTRA",0x1000},
 };
 
+/* The USB interface number of the wave-in device*/
+#define USB_IF_READ	0x06
+/* The USB interface number of the wave-out device*/
+#define USB_IF_WRITE	0x07
+
 static int debug = 0;
 
 #define FLAG_FLIP_RELAYS	(1 << 0)
@@ -181,7 +178,6 @@
 	return 0;
 }					  
 
-#ifdef USB2420
 #ifdef LINUX26
 static int wcusb_async_read(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb, struct pt_regs *regs));
 static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb, struct pt_regs *regs));
@@ -191,11 +187,6 @@
 static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb));
 static void wcusb_async_control(struct urb *urb);
 #endif /* LINUX26 */
-#else
-static int wcusb_async_read(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(urb_t *urb));
-static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(urb_t *urb));
-static void wcusb_async_control(urb_t *urb);
-#endif
 
 static void proslic_read_direct_async(struct wc_usb_pvt *p, unsigned char address)
 {
@@ -217,15 +208,11 @@
 	wcusb_async_write(p, WCUSB_SPORT0, p->wcregbuf, 4, STATE_WCWRITE_WRITERES, wcusb_async_control);
 }
 
-#ifdef USB2420
 #ifdef LINUX26
 static void wcusb_async_control(struct urb *urb, struct pt_regs *regs)
 #else
 static void wcusb_async_control(struct urb *urb)
 #endif
-#else
-static void wcusb_async_control(urb_t *urb)
-#endif
 {
 	struct wc_usb_pvt *p = urb->context;
 	p->urbcount--;
@@ -309,15 +296,11 @@
 	}
 }
 
-#ifdef USB2420
 #ifdef LINUX26
 static void keypad_check_done(struct urb *urb, struct pt_regs *regs)
 #else
 static void keypad_check_done(struct urb *urb)
 #endif
-#else
-static void keypad_check_done(urb_t *urb)
-#endif
 {
 	struct wc_usb_pvt *p = urb->context;
 	struct wc_keypad_data *d = p->pvt_data;
@@ -426,19 +409,14 @@
 	return;
 }
 
-#ifdef USB2420
 #ifdef LINUX26
 static int wcusb_async_read(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb, struct pt_regs *regs))
 #else
 static int wcusb_async_read(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb))
 #endif /* LINUX26 */
-#else
-static int wcusb_async_read(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(urb_t *urb))
-#endif
 {
 	__u16 size = len;
 	__u16 ind = index;
-#ifdef USB2420
 	struct urb *urb = &p->control;
 	memset(urb, 0, sizeof(struct urb));
 
@@ -447,18 +425,9 @@
 	p->dr.wValue = 0;
 	p->dr.wIndex = cpu_to_le16(ind);
 	p->dr.wLength = cpu_to_le16(size);
-#else
-	urb_t *urb = &p->control;
-	memset(urb, 0, sizeof(urb_t));
 
-	p->dr.requesttype = USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-	p->dr.request = REQUEST_NORMAL;
-	p->dr.value = 0;
-	p->dr.index = cpu_to_le16(ind);
-	p->dr.length = cpu_to_le16(size);
-#endif
-
-	FILL_CONTROL_URB(urb, p->dev, usb_rcvctrlpipe(p->dev, 0), (unsigned char *)&p->dr, data, len, complete, p);
+	usb_fill_control_urb(urb, p->dev, usb_rcvctrlpipe(p->dev, 0),
+			(unsigned char *)&p->dr, data, len, complete, p);
 #ifdef LINUX26
 	if (usb_submit_urb(urb, GFP_KERNEL)) 
 #else	
@@ -474,19 +443,15 @@
 	return 0;
 }
 
-#ifdef USB2420
 #ifdef LINUX26
 static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb, struct pt_regs *regs))
 #else
 static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(struct urb *urb))
 #endif /* LINUX26 */
-#else
-static int wcusb_async_write(struct wc_usb_pvt *p, unsigned char index, unsigned char *data, int len, int state, void (*complete)(urb_t *urb))
-#endif
 {
 	__u16 size = len;
 	__u16 ind = index;
-#ifdef USB2420
+
 	struct urb *urb = &p->control;
 	memset(urb, 0, sizeof(struct urb));
 
@@ -495,18 +460,9 @@
 	p->dr.wValue = 0;
 	p->dr.wIndex = cpu_to_le16(ind);
 	p->dr.wLength = cpu_to_le16(size);
-#else
-	urb_t *urb = &p->control;
-	memset(urb, 0, sizeof(urb_t));
 	
-	p->dr.requesttype = USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE;
-	p->dr.request = REQUEST_NORMAL;
-	p->dr.value = 0;
-	p->dr.index = cpu_to_le16(ind);
-	p->dr.length = cpu_to_le16(size);
-#endif
-
-	FILL_CONTROL_URB(urb, p->dev, usb_sndctrlpipe(p->dev, 0), (unsigned char *)&p->dr, data, len, complete, p);
+	usb_fill_control_urb(urb, p->dev, usb_sndctrlpipe(p->dev, 0),
+			(unsigned char *)&p->dr, data, len, complete, p);
 #ifdef LINUX26
 	if (usb_submit_urb(urb, GFP_KERNEL)) 
 #else	
@@ -1024,40 +980,37 @@
 {
 	int x;
 	/* Endpoint 6 is the wave-in device */
-	unsigned int readpipe = usb_rcvisocpipe(p->dev, 0x06);
+	unsigned int readpipe = usb_rcvisocpipe(p->dev, USB_IF_READ);
 	/* Endpoint 7 is the wave-out device */
-	unsigned int writepipe = usb_sndisocpipe(p->dev, 0x07);
+	unsigned int writepipe = usb_sndisocpipe(p->dev, USB_IF_WRITE);
 
 	for (x = 0; x < 2; x++) {
-		p->dataread[x].urb.dev = p->dev;
-		p->dataread[x].urb.pipe = readpipe;
+		usb_fill_bulk_urb(&p->dataread[x].urb, p->dev, readpipe,
+					p->readchunk + ZT_CHUNKSIZE * x,
+					ZT_CHUNKSIZE*2, wcusb_read_complete, p);
+		p->dataread[x].urb.count.counter=100; /* WTA: evil test **/
 #ifdef LINUX26
 		p->dataread[x].urb.transfer_flags = URB_ISO_ASAP;
 #else
 		p->dataread[x].urb.transfer_flags = USB_ISO_ASAP;
 #endif		
 		p->dataread[x].urb.number_of_packets = 1;
-		p->dataread[x].urb.context = p;
-		p->dataread[x].urb.complete = wcusb_read_complete;
 		p->dataread[x].urb.iso_frame_desc[0].length = ZT_CHUNKSIZE * 2;
 		p->dataread[x].urb.iso_frame_desc[0].offset = 0;
-		p->dataread[x].urb.transfer_buffer = p->readchunk + ZT_CHUNKSIZE * x;
-		p->dataread[x].urb.transfer_buffer_length = ZT_CHUNKSIZE * 2;
 
-		p->datawrite[x].urb.dev = p->dev;
-		p->datawrite[x].urb.pipe = writepipe;
+		usb_fill_bulk_urb(&p->dataread[x].urb, p->dev, writepipe,
+					p->writechunk + ZT_CHUNKSIZE * x,
+					ZT_CHUNKSIZE * 2, wcusb_write_complete, p);
+					
+		p->datawrite[x].urb.count.counter=100; /* WTA: evil test **/
 #ifdef LINUX26
 		p->datawrite[x].urb.transfer_flags = URB_ISO_ASAP;
 #else
 		p->datawrite[x].urb.transfer_flags = USB_ISO_ASAP;
 #endif
 		p->datawrite[x].urb.number_of_packets = 1;
-		p->datawrite[x].urb.context = p;
-		p->datawrite[x].urb.complete = wcusb_write_complete;
 		p->datawrite[x].urb.iso_frame_desc[0].length = ZT_CHUNKSIZE * 2;
 		p->datawrite[x].urb.iso_frame_desc[0].offset = 0;
-		p->datawrite[x].urb.transfer_buffer = p->writechunk + ZT_CHUNKSIZE * x;
-		p->datawrite[x].urb.transfer_buffer_length = ZT_CHUNKSIZE * 2;
 	
 	}
 	return 0;
@@ -1066,29 +1019,33 @@
 static int begin_transfer(struct wc_usb_pvt *p)
 {
 
-	int x;
+	int x,res;
 	p->urbcount = 4;
 	p->flags |= FLAG_RUNNING;
 
 	for (x = 0; x < 2; x++) {
+		int abort=0;
 #ifdef LINUX26
-		if (usb_submit_urb(&p->dataread[x].urb, GFP_KERNEL)) 
+		if ((res=usb_submit_urb(&p->dataread[x].urb, GFP_KERNEL)))
 #else
-		if (usb_submit_urb(&p->dataread[x].urb)) 
+		if ((res=usb_submit_urb(&p->dataread[x].urb)))
 #endif
 		{
-			printk(KERN_ERR "wcusb: Read submit failed\n");
-			return -1;
+			printk(KERN_ERR "wcusb: [frop] Read submit failed with %d\n", res);
+			abort=1;
+			/*return -1; */
 		}
 #ifdef LINUX26
-		if (usb_submit_urb(&p->datawrite[x].urb, GFP_KERNEL))
+		if ((res=usb_submit_urb(&p->datawrite[x].urb, GFP_KERNEL)))
 #else
-		if (usb_submit_urb(&p->datawrite[x].urb)) 
+		if ((res=usb_submit_urb(&p->datawrite[x].urb)))
 #endif		
 		{
-			printk(KERN_ERR "wcusb: Write submit failed\n");
-			return -1;
+			printk(KERN_ERR "wcusb: Write submit failed with %d\n", res);
+			abort=1;
+			/*return -1;*/
 		}
+		if (abort) return -1;
 	}
 	/* Start checking for interrupts */
 	wcusb_check_interrupt(p);
@@ -1269,6 +1226,8 @@
 	/* Set the stream to just pass the data from the device uninhibited */
 	p->sample = STREAM_NORMAL;
 
+	printk(KERN_INFO "About to register span %s\n", p->span.name);
+
 	if (zt_register(&p->span, 0)) {
 		printk("wcusb: Unable to register span %s\n", p->span.name);
 		return -1;
@@ -1289,6 +1248,7 @@
 	struct usb_device *dev = interface_to_usbdev(intf);
 #endif	
 
+	printk(KERN_INFO "DEBUG: Entering wc_usb_probe\n");
 	int x;
 	for (x=0;x<WC_MAX_IFACES;x++) {
 		/* Find first dead or empty space */
diff -wur org/zaptel-0.9.1/wcusb.h zaptel-0.9.1/wcusb.h
--- org/zaptel-0.9.1/wcusb.h	2004-01-06 14:26:44.000000000 +0100
+++ zaptel-0.9.1/wcusb.h	2004-06-07 13:11:54.000000000 +0200
@@ -8,9 +8,6 @@
 #include <linux/usb.h>
 
 #include <linux/version.h>
-#if LINUX_VERSION_CODE > KERNEL_VERSION(2,4,19)
-#define USB2420
-#endif
 
 #include "zaptel.h"
 
@@ -69,11 +66,7 @@
 
 struct wc_keypad_data {
         keypad_state_t state; /* Current state in the keypad detect routine */
-#ifdef USB2420
         struct urb urb;  /* urb used for the keypad data transport ... can't remember whether it is used or not */
-#else
-        urb_t urb;  /* urb used for the keypad data transport ... can't remember whether it is used or not */
-#endif
 	int running;
         char data;
         char data12;
@@ -87,15 +80,8 @@
 };
 
 struct stinky_urb {
-#ifdef USB2420
         struct urb urb;
-#ifndef LINUX26
-        struct iso_packet_descriptor isoframe[1];
-#endif		
-#else
-        urb_t urb;
-        iso_packet_descriptor_t isoframe[1];
-#endif
+        struct usb_iso_packet_descriptor isoframe[1];
 };
 
 struct wc_usb_pvt {
@@ -107,13 +93,8 @@
         struct zt_chan chan;
         struct stinky_urb dataread[2];
         struct stinky_urb datawrite[2];
-#ifdef USB2420
         struct urb          control;
         struct usb_ctrlrequest      dr;
-#else
-        urb_t           control;
-        devrequest      dr;
-#endif
         proslic_state_t controlstate;
         int urbcount;
         int flags;
diff -wur org/zaptel-0.9.1/ztdummy.h zaptel-0.9.1/ztdummy.h
--- org/zaptel-0.9.1/ztdummy.h	2003-02-24 07:00:31.000000000 +0100
+++ zaptel-0.9.1/ztdummy.h	2004-06-07 13:15:44.000000000 +0200
@@ -25,9 +25,6 @@
 */
 
 #include <linux/version.h>
-#if LINUX_VERSION_CODE > KERNEL_VERSION(2,4,19)
-#define USB2420
-#endif
 
 struct ztdummy {
 	struct zt_span span;
@@ -74,13 +71,8 @@
 	dma_addr_t setup_packet_dma;
 	dma_addr_t transfer_buffer_dma;
 	unsigned long started;
-#ifdef USB2420
         struct urb *next_queued_urb;    // next queued urb for this EP
         struct urb *prev_queued_urb;
-#else
-        urb_t *next_queued_urb;         
-        urb_t *prev_queued_urb;
-#endif
 	uhci_desc_t *bottom_qh;
 	uhci_desc_t *next_qh;       	// next helper QH
 	char use_loop;

-- 
Wichert Akkerman <wichert at wiggy.net>    It is simple to make things.
http://www.wiggy.net/                   It is hard to make things simple.



More information about the asterisk-dev mailing list