Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB.

[Greg KH <greg@kroah.com>]

On Mon, Mar 24, 2003 at 09:03:28PM +0000, Linux Kernel Mailing List wrote:
> ChangeSet 1.971.22.2, 2003/03/24 13:03:28-08:00, maxk@qualcomm.com
> 
> 	[Bluetooth] HCI USB driver update. Support for SCO over HCI USB.
> 	URB and buffer managment rewrite.

Max, you need to be very careful with this:

> -static void hci_usb_interrupt(struct urb *urb, struct pt_regs *regs);
> +struct _urb *_urb_alloc(int isoc, int gfp)
> +{
> +	struct _urb *_urb = kmalloc(sizeof(struct _urb) +
> +				sizeof(struct usb_iso_packet_descriptor) * isoc, gfp);
> +	if (_urb) {
> +		memset(_urb, 0, sizeof(*_urb));
> +		_urb->urb.count = (atomic_t)ATOMIC_INIT(1);
> +		spin_lock_init(&_urb->urb.lock);
> +	}
> +	return _urb;
> +}

You aren't calling usb_alloc_urb() and:

> +struct _urb *_urb_dequeue(struct _urb_queue *q)
> +{
> +	struct _urb *_urb = NULL;
> +        unsigned long flags;
> +        spin_lock_irqsave(&q->lock, flags);
> +	{
> +		struct list_head *head = &q->head;
> +		struct list_head *next = head->next;
> +		if (next != head) {
> +			_urb = list_entry(next, struct _urb, list);
> +			list_del(next); _urb->queue = NULL;
> +		}
> +	}
> +	spin_unlock_irqrestore(&q->lock, flags);
> +	return _urb;
> +}

You aren't calling usb_free_urb() as you are embedding a struct urb
within your struct _urb structure.  Any reason you can't use a struct
urb * instead and call the usb core's functions to create and return a
urb?

Otherwise any changes to the internal urb structures, and the
usb_alloc_urb() and usb_free_urb() functions will have to be mirrored
here in your functions, and I know I will forget to do that :)

Other than that, it's nice to see Bluetooth SCO support for Linux, very
nice job.

thaks,

greg k-h