On Fri, May 19, 2006 at 01:21:52AM -0400, Paul Giblock wrote:
> Hello, sorry about the last message. I figured it out on my own. I
> was always sort of surprised that usb_bulk_msg would work fine with
> interrupt endpoints, so it was just a matter of time until it would
> not. Anyways, I solved the problem with a callback and a condition
> variable.
>
> Everything seems to be working fine, but I am concerned about possible
> residual bugs or memory leaks. If anyone can give these two
> procedures a quick look I would appreciate it. The code isn't the
> best -- it is just a rapid prototype. Thanks.
>
> /* HQCT device structure */
> struct usb_hqct {
> struct usb_device * udev;
> struct usb_interface * interface;
> struct semaphore limit_sem;
> struct completion read_comp;
> unsigned char * intr_in_buffer;
> size_t intr_in_size;
> __u8 intr_in_endpointAddr;
> __u8 intr_out_endpointAddr;
> struct kref kref;
> int endp_interval;
> };
>
> static void hqct_read_callback(struct urb *urb, struct pt_regs *regs)
> {
> struct usb_hqct *dev;
> int i;
>
> dev = (struct usb_hqct *)urb->context;
Cast not needed.
>
> printk("READ CALLBACK status=%d \n", urb->status);
Need KERN_ level.
> for(i=0; i < 32; i++) {
Space after the "for".
> printk("%.2X ", dev->intr_in_buffer[i]);
> }
No {} needed for 1 line for statements (or if statements).
> printk("\n");
>
> complete(&(dev->read_comp));
What does this completion handler do?
Why not do some work here? Handle status values properly?
> printk("READ CALLBACK completed.\n");
> }
>
>
> /* Read: will poll for the next status packet and return it in the raw */
> static ssize_t hqct_read(struct file *file, char *buffer, size_t
> count, loff_t *ppos)
> {
> struct usb_hqct *dev;
> struct urb *urb = NULL;
> int retval = 0;
> int bytes_read = 0;
>
> dev = (struct usb_hqct *)file->private_data;
No cast needed.
>
> /* create a urb, and a buffer for it, and copy the data to the urb */
> urb = usb_alloc_urb(0, GFP_KERNEL);
> if (!urb) {
> retval = -ENOMEM;
> goto error;
> }
>
> /* fill the interrupt urb in */
> usb_fill_int_urb(urb, dev->udev,
> usb_rcvintpipe(dev->udev,
> dev->intr_in_endpointAddr),
> dev->intr_in_buffer,
> min(dev->intr_in_size,count), hqct_read_callback, dev,
> dev->endp_interval);
>
> retval = usb_submit_urb(urb, GFP_KERNEL);
> if (retval) {
> err("%s - failed submitting write urb, error %d",
> __FUNCTION__, retval);
> goto error;
> }
>
> /* block until we receive the buffer */
> printk("HQCT Blocking...\n");
> wait_for_completion(&(dev->read_comp) );
What happens if the interrupt completes before you get here? Hm, maybe
that will be safe, haven't looked at that in a long time, you might want
to verify this.
> bytes_read = urb->actual_length;
>
> printk("HQCT READ %d %d\n", count, bytes_read);
>
> /* if successful read, copy the data to userspace */
> if (urb->status == 0) {
> if (copy_to_user(buffer, dev->intr_in_buffer, count))
> retval = -EFAULT;
> else
> retval = bytes_read;
> }
>
> usb_free_urb(urb);
> return retval;
Why not just use the blocking function?
Oh crap, we don't have a blocking usb_interrupt_msg() type function do
we? I've wanted that for a long time, let me go add that to the core.
That should do what you need, right?
thanks,
greg k-h
-------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
