lkml.org 
[lkml]   [2003]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB.
At 09:15 PM 4/28/2003, Greg KH wrote:
>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:
I know ;-)

>> +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 ?
I didn't want to do two allocations (one for struct _urb and one for struct 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 :)
How about

static inline urb_init(urb)
{
urb->xx = YY;
...
}

urb_alloc()
{
urb = kmalloc()
urb_init(urb);
}

?

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

I was actually going to ask you guys if you'd be interested in generalizing this _urb_queue()
stuff that I have for other drivers. Current URB api does not provide any interface for
queueing/linking/etc of URBs in the _driver_ itself. Things like next, prev, etc are used in
the HCD. So if driver submits bunch of different URBs (and potentially multiple URBs of the
same type like hci_usb does) it has to implement its own lists, arrays and stuff. I used to
use SKBs for URB queues but struct sk_buff is to big for that simple task.

Max

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:35    [W:1.514 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site