* Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. [not found] <200304290317.h3T3HOdA027579@hera.kernel.org> @ 2003-04-29 4:15 ` Greg KH 2003-04-29 20:29 ` Max Krasnyansky 0 siblings, 1 reply; 30+ messages in thread From: Greg KH @ 2003-04-29 4:15 UTC (permalink / raw) To: maxk; +Cc: Linux Kernel Mailing List, linux-usb-devel 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 4:15 ` [Bluetooth] HCI USB driver update. Support for SCO over HCI USB Greg KH @ 2003-04-29 20:29 ` Max Krasnyansky 2003-04-29 21:15 ` Greg KH 2003-04-29 21:39 ` [linux-usb-devel] " David Brownell 0 siblings, 2 replies; 30+ messages in thread From: Max Krasnyansky @ 2003-04-29 20:29 UTC (permalink / raw) To: Greg KH; +Cc: Linux Kernel Mailing List, linux-usb-devel 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 20:29 ` Max Krasnyansky @ 2003-04-29 21:15 ` Greg KH 2003-04-29 21:34 ` Oliver Neukum 2003-04-29 21:55 ` Max Krasnyansky 2003-04-29 21:39 ` [linux-usb-devel] " David Brownell 1 sibling, 2 replies; 30+ messages in thread From: Greg KH @ 2003-04-29 21:15 UTC (permalink / raw) To: Max Krasnyansky; +Cc: Linux Kernel Mailing List, linux-usb-devel On Tue, Apr 29, 2003 at 01:29:41PM -0700, Max Krasnyansky wrote: > >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). I agree, and understand what you are doing, it makes sense for your case. Just wanted to make sure you were aware of the future danger :) > >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 <snip> Ok, that works for me. Does the patch below work out for 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. Yes, generalizing that stuff would be nice to have. Lots of drivers have to manage their urbs on their own today, and making that easier to do would be a good thing. thanks, greg k-h # usb: create usb_init_urb() for those people who like to live dangerously (like the bluetooth stack.) diff -Nru a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c --- a/drivers/usb/core/urb.c Tue Apr 29 14:07:13 2003 +++ b/drivers/usb/core/urb.c Tue Apr 29 14:07:13 2003 @@ -14,6 +14,29 @@ #include "hcd.h" /** + * usb_init_urb - initializes a urb so that it can be used by a USB driver + * @urb: pointer to the urb to initialize + * + * Initializes a urb so that the USB subsystem can use it properly. + * + * If a urb is created with a call to usb_alloc_urb() it is not + * necessary to call this function. Only use this if you allocate the + * space for a struct urb on your own. If you call this function, be + * careful when freeing the memory for your urb that it is no longer in + * use by the USB core. + */ +int usb_init_urb(struct urb *urb) +{ + if (!urb) + return -EINVAL; + memset(urb, 0, sizeof(*urb)); + urb->count = (atomic_t)ATOMIC_INIT(1); + spin_lock_init(&urb->lock); + + return 0; +} + +/** * usb_alloc_urb - creates a new urb for a USB driver to use * @iso_packets: number of iso packets for this urb * @mem_flags: the type of memory to allocate, see kmalloc() for a list of @@ -38,13 +61,14 @@ mem_flags); if (!urb) { err("alloc_urb: kmalloc failed"); - return NULL; + goto exit; + } + if (usb_init_urb(urb)) { + kfree(urb); + urb = NULL; } - memset(urb, 0, sizeof(*urb)); - urb->count = (atomic_t)ATOMIC_INIT(1); - spin_lock_init(&urb->lock); - +exit: return urb; } @@ -387,7 +411,7 @@ return -ENODEV; } -// asynchronous request completion model +EXPORT_SYMBOL(usb_init_urb); EXPORT_SYMBOL(usb_alloc_urb); EXPORT_SYMBOL(usb_free_urb); EXPORT_SYMBOL(usb_get_urb); diff -Nru a/include/linux/usb.h b/include/linux/usb.h --- a/include/linux/usb.h Tue Apr 29 14:07:13 2003 +++ b/include/linux/usb.h Tue Apr 29 14:07:13 2003 @@ -766,6 +766,7 @@ } extern struct urb *usb_alloc_urb(int iso_packets, int mem_flags); +extern int usb_init_urb(struct urb *urb); extern void usb_free_urb(struct urb *urb); #define usb_put_urb usb_free_urb extern struct urb *usb_get_urb(struct urb *urb); ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 21:15 ` Greg KH @ 2003-04-29 21:34 ` Oliver Neukum 2003-04-29 21:40 ` Greg KH 2003-04-29 21:55 ` Max Krasnyansky 1 sibling, 1 reply; 30+ messages in thread From: Oliver Neukum @ 2003-04-29 21:34 UTC (permalink / raw) To: Greg KH, Max Krasnyansky; +Cc: Linux Kernel Mailing List, linux-usb-devel > +int usb_init_urb(struct urb *urb) > +{ > + if (!urb) > + return -EINVAL; > + memset(urb, 0, sizeof(*urb)); > + urb->count = (atomic_t)ATOMIC_INIT(1); > + spin_lock_init(&urb->lock); > + > + return 0; > +} Greg, please don't do it this way. Somebody will try to free this urb. If the urb is part of a structure this must not lead to a kfree. Please init it to some insanely high dummy value in this case. Regards Oliver ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 21:34 ` Oliver Neukum @ 2003-04-29 21:40 ` Greg KH 2003-04-29 22:24 ` [linux-usb-devel] " Oliver Neukum 2003-04-30 0:44 ` Max Krasnyansky 0 siblings, 2 replies; 30+ messages in thread From: Greg KH @ 2003-04-29 21:40 UTC (permalink / raw) To: oliver; +Cc: Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel On Tue, Apr 29, 2003 at 11:34:19PM +0200, Oliver Neukum wrote: > > > +int usb_init_urb(struct urb *urb) > > +{ > > + if (!urb) > > + return -EINVAL; > > + memset(urb, 0, sizeof(*urb)); > > + urb->count = (atomic_t)ATOMIC_INIT(1); > > + spin_lock_init(&urb->lock); > > + > > + return 0; > > +} > > Greg, please don't do it this way. Somebody will > try to free this urb. If the urb is part of a structure > this must not lead to a kfree. Please init it to some > insanely high dummy value in this case. We can't init it to a high value, if we want to use it ourself in usb_alloc_urb(). And yes, I agree this is a very dangerous function to use on your own, I thought I conveyed that in the documentation for the function. But if we don't have such a function, then people like Max will just roll their own, like he just did :) Might as well make it easy for him to shoot himself in the foot if he really wants to... thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 21:40 ` Greg KH @ 2003-04-29 22:24 ` Oliver Neukum 2003-04-30 0:44 ` Max Krasnyansky 1 sibling, 0 replies; 30+ messages in thread From: Oliver Neukum @ 2003-04-29 22:24 UTC (permalink / raw) To: Greg KH; +Cc: Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel Am Dienstag, 29. April 2003 23:40 schrieb Greg KH: > On Tue, Apr 29, 2003 at 11:34:19PM +0200, Oliver Neukum wrote: > > > +int usb_init_urb(struct urb *urb) > > > +{ > > > + if (!urb) > > > + return -EINVAL; > > > + memset(urb, 0, sizeof(*urb)); > > > + urb->count = (atomic_t)ATOMIC_INIT(1); > > > + spin_lock_init(&urb->lock); > > > + > > > + return 0; > > > +} > > > > Greg, please don't do it this way. Somebody will > > try to free this urb. If the urb is part of a structure > > this must not lead to a kfree. Please init it to some > > insanely high dummy value in this case. > > We can't init it to a high value, if we want to use it ourself in > usb_alloc_urb(). So don't or make that value a parameter. > And yes, I agree this is a very dangerous function to use on your own, > I thought I conveyed that in the documentation for the function. It's not any more dangerous than what worked quite well for 2.4. > But if we don't have such a function, then people like Max will just > roll their own, like he just did :) > > Might as well make it easy for him to shoot himself in the foot if he > really wants to... Sure, why not. Regards Oliver ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 21:40 ` Greg KH 2003-04-29 22:24 ` [linux-usb-devel] " Oliver Neukum @ 2003-04-30 0:44 ` Max Krasnyansky 2003-04-30 7:06 ` Oliver Neukum 1 sibling, 1 reply; 30+ messages in thread From: Max Krasnyansky @ 2003-04-30 0:44 UTC (permalink / raw) To: Greg KH, oliver; +Cc: Linux Kernel Mailing List, linux-usb-devel At 02:40 PM 4/29/2003, Greg KH wrote: >On Tue, Apr 29, 2003 at 11:34:19PM +0200, Oliver Neukum wrote: >> >> > +int usb_init_urb(struct urb *urb) >> > +{ >> > + if (!urb) >> > + return -EINVAL; >> > + memset(urb, 0, sizeof(*urb)); >> > + urb->count = (atomic_t)ATOMIC_INIT(1); >> > + spin_lock_init(&urb->lock); >> > + >> > + return 0; >> > +} >> >> Greg, please don't do it this way. Somebody will >> try to free this urb. If the urb is part of a structure >> this must not lead to a kfree. Please init it to some >> insanely high dummy value in this case. Uh, I didn't think about that one. This stuff was first implemented for 2.4 which didn't have refcount in urb and then forward ported to 2.5. >We can't init it to a high value, if we want to use it ourself in >usb_alloc_urb(). > >And yes, I agree this is a very dangerous function to use on your own, >I thought I conveyed that in the documentation for the function. > >But if we don't have such a function, then people like Max will just >roll their own, like he just did :) > >Might as well make it easy for him to shoot himself in the foot if he >really wants to... Thanks. I'll wear bullet proof shoes, just in case :-). Max ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-30 0:44 ` Max Krasnyansky @ 2003-04-30 7:06 ` Oliver Neukum 0 siblings, 0 replies; 30+ messages in thread From: Oliver Neukum @ 2003-04-30 7:06 UTC (permalink / raw) To: Max Krasnyansky, Greg KH; +Cc: Linux Kernel Mailing List, linux-usb-devel Am Mittwoch, 30. April 2003 02:44 schrieb Max Krasnyansky: > At 02:40 PM 4/29/2003, Greg KH wrote: > >On Tue, Apr 29, 2003 at 11:34:19PM +0200, Oliver Neukum wrote: > >> > +int usb_init_urb(struct urb *urb) > >> > +{ > >> > + if (!urb) > >> > + return -EINVAL; > >> > + memset(urb, 0, sizeof(*urb)); > >> > + urb->count = (atomic_t)ATOMIC_INIT(1); > >> > + spin_lock_init(&urb->lock); > >> > + > >> > + return 0; > >> > +} > >> > >> Greg, please don't do it this way. Somebody will > >> try to free this urb. If the urb is part of a structure > >> this must not lead to a kfree. Please init it to some > >> insanely high dummy value in this case. > > Uh, I didn't think about that one. This stuff was first implemented > for 2.4 which didn't have refcount in urb and then forward ported to 2.5. It should work. However if you have a refcount bug, the failure case will be spectacular. Regards Oliver ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 21:15 ` Greg KH 2003-04-29 21:34 ` Oliver Neukum @ 2003-04-29 21:55 ` Max Krasnyansky 2003-04-29 22:04 ` Greg KH 1 sibling, 1 reply; 30+ messages in thread From: Max Krasnyansky @ 2003-04-29 21:55 UTC (permalink / raw) To: Greg KH; +Cc: Linux Kernel Mailing List, linux-usb-devel At 02:15 PM 4/29/2003, Greg KH wrote: >On Tue, Apr 29, 2003 at 01:29:41PM -0700, Max Krasnyansky wrote: >> >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). > >I agree, and understand what you are doing, it makes sense for your case. >Just wanted to make sure you were aware of the future danger :) :) >> >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 > ><snip> > >Ok, that works for me. Does the patch below work out for you? Yep. I'd make it return void though. I'll fix hci_usb to use usb_init_urb() when your patch get's in. >> 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. > >Yes, generalizing that stuff would be nice to have. Lots of drivers >have to manage their urbs on their own today, and making that easier to >do would be a good thing. Good. I'll reply do Dave's email were he asked for more details. Thanks. Max ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 21:55 ` Max Krasnyansky @ 2003-04-29 22:04 ` Greg KH 0 siblings, 0 replies; 30+ messages in thread From: Greg KH @ 2003-04-29 22:04 UTC (permalink / raw) To: Max Krasnyansky; +Cc: Linux Kernel Mailing List, linux-usb-devel On Tue, Apr 29, 2003 at 02:55:17PM -0700, Max Krasnyansky wrote: > >Ok, that works for me. Does the patch below work out for you? > Yep. I'd make it return void though. Good point, I'll change that and send it off. thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 20:29 ` Max Krasnyansky 2003-04-29 21:15 ` Greg KH @ 2003-04-29 21:39 ` David Brownell 2003-04-29 21:37 ` Greg KH 2003-04-29 22:04 ` Max Krasnyansky 1 sibling, 2 replies; 30+ messages in thread From: David Brownell @ 2003-04-29 21:39 UTC (permalink / raw) To: Max Krasnyansky; +Cc: Greg KH, Linux Kernel Mailing List, linux-usb-devel > 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. I only saw fragments of the original patch -- could you be just a bit more specific? If you're suggesting adding some "struct list_head" into "struct urb" for exclusive use of the interface's driver (instead of urb_list, which is for usbcore/hcd) ... I'd agree that'd be a good thing. In fact I recently got around to adding that to the "gadget side" analogue of an URB. For much the same kind of reasons as you mentioned. - Dave ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 21:39 ` [linux-usb-devel] " David Brownell @ 2003-04-29 21:37 ` Greg KH 2003-04-29 22:04 ` Max Krasnyansky 1 sibling, 0 replies; 30+ messages in thread From: Greg KH @ 2003-04-29 21:37 UTC (permalink / raw) To: David Brownell Cc: Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel On Tue, Apr 29, 2003 at 02:39:39PM -0700, David Brownell wrote: > > > 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. > > I only saw fragments of the original patch -- could you be just > a bit more specific? The whole patch can be seen at: http://marc.theaimsgroup.com/?l=bk-commits-head&m=105158631404987 thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 21:39 ` [linux-usb-devel] " David Brownell 2003-04-29 21:37 ` Greg KH @ 2003-04-29 22:04 ` Max Krasnyansky 2003-04-30 4:55 ` David Brownell 1 sibling, 1 reply; 30+ messages in thread From: Max Krasnyansky @ 2003-04-29 22:04 UTC (permalink / raw) To: David Brownell; +Cc: Greg KH, Linux Kernel Mailing List, linux-usb-devel At 02:39 PM 4/29/2003, David Brownell wrote: >> 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. > >I only saw fragments of the original patch -- could you be just >a bit more specific? > >If you're suggesting adding some "struct list_head" into >"struct urb" for exclusive use of the interface's driver >(instead of urb_list, which is for usbcore/hcd) ... I'd >agree that'd be a good thing. > >In fact I recently got around to adding that to the >"gadget side" analogue of an URB. For much the same >kind of reasons as you mentioned. Basically I'd like to have same kind of API that we have for SKB without an overhead of SKBs. Here is what I've done for Bluetooth HCI USB driver. struct _urb_queue { struct list_head head; spinlock_t lock; }; struct _urb { struct list_head list; struct _urb_queue *queue; int type; void *priv; struct urb urb; }; struct _urb *_urb_alloc(int isoc, int gfp); static inline void _urb_free(struct _urb *_urb); static inline void _urb_queue_init(struct _urb_queue *q); static inline void _urb_queue_head(struct _urb_queue *q, struct _urb *_urb); static inline void _urb_queue_tail(struct _urb_queue *q, struct _urb *_urb); static inline void _urb_unlink(struct _urb *_urb); struct _urb *_urb_dequeue(struct _urb_queue *q); _urb is an ugly name I know but I couldn't come up with the better one :) It's now easy to do things like _urb_queue_tail(&pending_q, _urb); usb_urb_submit(&_urb->urb); and while (_urb = _urb_dequeue(&pending_q)) usb_unlink_urb(&_urb->urb); etc Max ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-29 22:04 ` Max Krasnyansky @ 2003-04-30 4:55 ` David Brownell 2003-05-08 22:55 ` Max Krasnyansky 0 siblings, 1 reply; 30+ messages in thread From: David Brownell @ 2003-04-30 4:55 UTC (permalink / raw) To: Max Krasnyansky; +Cc: Greg KH, Linux Kernel Mailing List, linux-usb-devel Max Krasnyansky wrote: > > Basically I'd like to have same kind of API that we have for SKB without > an overhead of SKBs. I'd also like to see most of the per-request invocation costs shrink. You seem to be focussing on the queueing parts of SKB-ness, yes? The lifecycles aren't that close; and other parts of the SKB and URB models are different too. Inside USB, "usbcore" and the HCDs are already working with queues of URBs associated with each endpoint. The USB device drivers can't do that as easily, since core/HCD owns the urb->urb_list field after usb_submit_urb() and before the completion callback is issued. Much like the next layer owns the SKB after you hand it off... > Here is what I've done for Bluetooth HCI USB driver. > > struct _urb_queue { > struct list_head head; > spinlock_t lock; > }; > > struct _urb { > struct list_head list; > struct _urb_queue *queue; > struct urb urb; Those fields (reordered) are like sk_buff_head and sk_buff. How much of that is "needed here" vs. "SKBs work like that"? Today those spinlocks are driver-specific, and urb->context (or urb->hcpriv) seems to have been enough of a queue head backpointer for most drivers. The notable omission is the lack of a list_head for device drivers to use even after an URB has been submitted -- a lifecycle state that SKBs don't have. > int type; > void *priv; > }; How would "priv" differ from the current per-request state, urb->context (for device driver) or urb->hcpriv (for HCD)? I've also been interested in seeing something like skb->cb[]. HCDs could use that as pre-allocated per-request memory, avoiding per-request heap allocations. > It's now easy to do things like > _urb_queue_tail(&pending_q, _urb); > usb_urb_submit(&_urb->urb); > and > while (_urb = _urb_dequeue(&pending_q)) > usb_unlink_urb(&_urb->urb); Those resemble the kinds of primitives I might want to see in a Linux 2.7 USB API ... focussed on the endpoint (queue), rather than making the core/HCD layers do extra work to figure out what queue is involved. And ideally general enough that device drivers would work the same way. That first pair is pretty much what any HCD does today, except that the endpoint's queue is hidden/internal and the second step is "feed it to the hardware!". The second is pretty much what usbcore (now) does when it shuts down an endpoint's queue. But again, that queue is hidden/internal. Caller needs to ensure the hardware is not working on the queue during that loop, and there are some other synchronization issues too. - Dave > etc > > Max > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-04-30 4:55 ` David Brownell @ 2003-05-08 22:55 ` Max Krasnyansky 2003-05-09 19:06 ` David Brownell 0 siblings, 1 reply; 30+ messages in thread From: Max Krasnyansky @ 2003-05-08 22:55 UTC (permalink / raw) To: David Brownell; +Cc: Greg KH, Linux Kernel Mailing List, linux-usb-devel Hi David, (sorry for not replying right away) At 09:55 PM 4/29/2003, David Brownell wrote: >Max Krasnyansky wrote: >>Basically I'd like to have same kind of API that we have for SKB without >>an overhead of SKBs. > >I'd also like to see most of the per-request invocation costs >shrink. You seem to be focussing on the queueing parts of >SKB-ness, yes? Yes. >The lifecycles aren't that close; and other >parts of the SKB and URB models are different too. Totally. >Inside USB, "usbcore" and the HCDs are already working with >queues of URBs associated with each endpoint. > >The USB device drivers can't do that as easily, since core/HCD >owns the urb->urb_list field after usb_submit_urb() and before >the completion callback is issued. Much like the next layer >owns the SKB after you hand it off... Yep. That's exactly what I'd like to address. >>Here is what I've done for Bluetooth HCI USB driver. >>struct _urb_queue { >> struct list_head head; >> spinlock_t lock; >>}; >>struct _urb { >> struct list_head list; >> struct _urb_queue *queue; > >Those fields (reordered) are like sk_buff_head and sk_buff. >How much of that is "needed here" vs. "SKBs work like that"? If we want to provide urb_queue(),urb_dequeue(),etc then we'd need all of the above. >Today those spinlocks are driver-specific, and urb->context >(or urb->hcpriv) seems to have been enough of a queue head >backpointer for most drivers. I'd say it's enough for the drivers that submit only 1 or 2 request types and one URB per request. Drivers that use bulk queening and/or have to deal with multiple requests have to implement some infrastructure to keep track of it's own URBs. >The notable omission is the lack of a list_head for device drivers to use even >after an URB has been submitted -- a lifecycle state that >SKBs don't have. I guess just adding another 'struct list_head', usable by the drivers, to the 'struct urb' is probably enough. However like I mentioned above if we want to provide urb_queue() and friends we'd need 'struct urb_queue' or something similar. >> int type; >> void *priv; >>}; > > >How would "priv" differ from the current per-request state, >urb->context (for device driver) or urb->hcpriv (for HCD)? Oh, it's something that was useful in my driver. We don't have to add another priv. >I've also been interested in seeing something like skb->cb[]. >HCDs could use that as pre-allocated per-request memory, >avoiding per-request heap allocations. Yes yes yes :). And for drivers too. That were my _urb->type and stuff would go. >>It's now easy to do things like >> _urb_queue_tail(&pending_q, _urb); >> usb_urb_submit(&_urb->urb); >>and >> while (_urb = _urb_dequeue(&pending_q)) >> usb_unlink_urb(&_urb->urb); > >Those resemble the kinds of primitives I might want to see >in a Linux 2.7 USB API ... focussed on the endpoint (queue), >rather than making the core/HCD layers do extra work to >figure out what queue is involved. And ideally general >enough that device drivers would work the same way. > >That first pair is pretty much what any HCD does today, >except that the endpoint's queue is hidden/internal and >the second step is "feed it to the hardware!". > >The second is pretty much what usbcore (now) does when it >shuts down an endpoint's queue. But again, that queue >is hidden/internal. Caller needs to ensure the hardware is >not working on the queue during that loop, and there are >some other synchronization issues too. Do you think we can add this struct urb { ... struct list_head drv_list; char drv_cb[X]; char hcd_cb[X]; ... }; now though ? Max ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-08 22:55 ` Max Krasnyansky @ 2003-05-09 19:06 ` David Brownell 2003-05-09 19:29 ` Greg KH 2003-05-09 22:35 ` Max Krasnyansky 0 siblings, 2 replies; 30+ messages in thread From: David Brownell @ 2003-05-09 19:06 UTC (permalink / raw) To: Max Krasnyansky; +Cc: Greg KH, Linux Kernel Mailing List, linux-usb-devel [-- Attachment #1: Type: text/plain, Size: 1352 bytes --] Hi Max, > Do you think we can add this > struct urb { > ... > struct list_head drv_list; > char drv_cb[X]; > char hcd_cb[X]; > ... > }; Only with kerneldoc ... :) I'd certainly like the list_head. Patch attached, in case Greg agrees enough. On x86, this makes sizeof(struct urb) == 120, so it's using space that was previously wasted. As for the skb->cb analogue(s), details need working. - What should "X" be? skb->cb is 48 bytes. - Should the cb[] arrays be "long"? They tend to be used for pointers... - The HCDs want different amounts of per-urb data. Sizes on x86: * UHCI wants a lot -- 60 bytes! * OHCI typically uses 16 bytes, but more for multi-TD urbs (control 24 bytes, ISO often 36). * EHCI doesn't allocate such extra data. - The HCDs would need conversion to use hcd_cb[]. I once had a patch that did that, but it's not current. It made urb->cb replace urb->hcpriv. I suppose X=60 for hcd_cb[] will be enough, at least on 32 bit CPUs. But you start to see why in the new "USB Gadget" API, the analogue of "urb" gets allocated by the USB (device) controller rather than by generic code: so that in typical cases, no additional per-request allocations are needed. - Dave [-- Attachment #2: urb.patch --] [-- Type: text/plain, Size: 1079 bytes --] --- 1.139/include/linux/usb.h Wed May 7 00:02:43 2003 +++ edited/include/linux/usb.h Thu May 8 20:06:31 2003 @@ -511,6 +511,8 @@ /** * struct urb - USB Request Block * @urb_list: For use by current owner of the URB. + * @dev_list: For use by device driver, even while the URB is owned by + * usbcore and the HCD. Th device driver must initialize this list. * @pipe: Holds endpoint number, direction, type, and more. * Create these values with the eight macros available; * usb_{snd,rcv}TYPEpipe(dev,endpoint), where the type is "ctrl" @@ -669,7 +671,8 @@ spinlock_t lock; /* lock for the URB */ atomic_t count; /* reference count of the URB */ void *hcpriv; /* private data for host controller */ - struct list_head urb_list; /* list pointer to all active urbs */ + struct list_head urb_list; /* private data for usbcore */ + struct list_head dev_list; /* private data for device driver */ struct usb_device *dev; /* (in) pointer to associated device */ unsigned int pipe; /* (in) pipe information */ int status; /* (return) non-ISO status */ ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-09 19:06 ` David Brownell @ 2003-05-09 19:29 ` Greg KH 2003-05-09 22:35 ` Max Krasnyansky 2003-05-09 22:35 ` Max Krasnyansky 1 sibling, 1 reply; 30+ messages in thread From: Greg KH @ 2003-05-09 19:29 UTC (permalink / raw) To: David Brownell Cc: Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel On Fri, May 09, 2003 at 12:06:27PM -0700, David Brownell wrote: > > I'd certainly like the list_head. Patch attached, > in case Greg agrees enough. I agree, but will only take the patch if a driver is modified to actually use this. I'll take both patches at once :) thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-09 19:29 ` Greg KH @ 2003-05-09 22:35 ` Max Krasnyansky 0 siblings, 0 replies; 30+ messages in thread From: Max Krasnyansky @ 2003-05-09 22:35 UTC (permalink / raw) To: Greg KH; +Cc: David Brownell, Linux Kernel Mailing List, linux-usb-devel On Fri, 2003-05-09 at 12:29, Greg KH wrote: > On Fri, May 09, 2003 at 12:06:27PM -0700, David Brownell wrote: > > > > I'd certainly like the list_head. Patch attached, > > in case Greg agrees enough. > > I agree, but will only take the patch if a driver is modified to > actually use this. I'll take both patches at once :) Ok. Sounds good to me. But I also need ->drv_cb[]. Max ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-09 19:06 ` David Brownell 2003-05-09 19:29 ` Greg KH @ 2003-05-09 22:35 ` Max Krasnyansky 2003-05-09 23:05 ` Greg KH 1 sibling, 1 reply; 30+ messages in thread From: Max Krasnyansky @ 2003-05-09 22:35 UTC (permalink / raw) To: David Brownell; +Cc: Greg KH, Linux Kernel Mailing List, linux-usb-devel On Fri, 2003-05-09 at 12:06, David Brownell wrote: > Hi Max, > > > Do you think we can add this > > struct urb { > > ... > > struct list_head drv_list; > > char drv_cb[X]; > > char hcd_cb[X]; > > ... > > }; > > Only with kerneldoc ... :) > :) > I'd certainly like the list_head. Patch attached, > in case Greg agrees enough. On x86, this makes > sizeof(struct urb) == 120, so it's using space > that was previously wasted. > > > As for the skb->cb analogue(s), details need working. > > - What should "X" be? skb->cb is 48 bytes. I'd say 16 will be enough for drivers. > - Should the cb[] arrays be "long"? They tend to > be used for pointers... Probably doesn't matter as long as ->cb is aligned. > - The HCDs want different amounts of per-urb data. > Sizes on x86: > * UHCI wants a lot -- 60 bytes! > * OHCI typically uses 16 bytes, but more for > multi-TD urbs (control 24 bytes, ISO often 36). > * EHCI doesn't allocate such extra data. > > - The HCDs would need conversion to use hcd_cb[]. > I once had a patch that did that, but it's not > current. It made urb->cb replace urb->hcpriv. > > I suppose X=60 for hcd_cb[] will be enough, at least > on 32 bit CPUs. But you start to see why in the > new "USB Gadget" API, the analogue of "urb" gets > allocated by the USB (device) controller rather > than by generic code: so that in typical cases, > no additional per-request allocations are needed. Ok. Sounds like it should be uint32_t hcd_cb[16]; // 64 bytes for internal use by HCD uint32_t drv_cb[2]; // 8 bytes for internal use by USB driver ? Max ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-09 22:35 ` Max Krasnyansky @ 2003-05-09 23:05 ` Greg KH 2003-05-10 0:48 ` David Brownell 0 siblings, 1 reply; 30+ messages in thread From: Greg KH @ 2003-05-09 23:05 UTC (permalink / raw) To: Max Krasnyansky Cc: David Brownell, Linux Kernel Mailing List, linux-usb-devel On Fri, May 09, 2003 at 03:35:36PM -0700, Max Krasnyansky wrote: > Ok. Sounds like it should be > uint32_t hcd_cb[16]; // 64 bytes for internal use by HCD > uint32_t drv_cb[2]; // 8 bytes for internal use by USB driver s/uint32_t/u32/ please. And if this is going to be used for pointers, why not just say they are pointers? Otherwise people are going to have to be careful with 32 vs. 64 bit kernels to not overrun their space. struct sk_buff uses a char, any reason not to use that here too? Has being a char made things more difficult for that structure over time? thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-09 23:05 ` Greg KH @ 2003-05-10 0:48 ` David Brownell 2003-05-10 5:06 ` Brad Hards ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: David Brownell @ 2003-05-10 0:48 UTC (permalink / raw) To: Greg KH; +Cc: Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel Greg KH wrote: > On Fri, May 09, 2003 at 03:35:36PM -0700, Max Krasnyansky wrote: > >>Ok. Sounds like it should be >> uint32_t hcd_cb[16]; // 64 bytes for internal use by HCD >> uint32_t drv_cb[2]; // 8 bytes for internal use by USB driver > > > s/uint32_t/u32/ please. "u32" is prettier, but is there actually a policy against using the more standard type names? (POSIX, someone had said.) > And if this is going to be used for pointers, why not just say they are > pointers? Otherwise people are going to have to be careful with 32 vs. > 64 bit kernels to not overrun their space. > > struct sk_buff uses a char, any reason not to use that here too? Has > being a char made things more difficult for that structure over time? No, it's just that in some similar cases having the value be "long" (not "32 bit unsigned") has been simpler. I'm not religious. - Dave ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-10 0:48 ` David Brownell @ 2003-05-10 5:06 ` Brad Hards 2003-05-10 5:40 ` Greg KH 2003-05-12 17:53 ` Max Krasnyansky 2 siblings, 0 replies; 30+ messages in thread From: Brad Hards @ 2003-05-10 5:06 UTC (permalink / raw) To: David Brownell, Greg KH; +Cc: Linux Kernel Mailing List, linux-usb-devel -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Sat, 10 May 2003 10:48 am, David Brownell wrote: > > s/uint32_t/u32/ please. > > "u32" is prettier, but is there actually a policy against using > the more standard type names? (POSIX, someone had said.) Not Invented Here? Brad -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.0.7 (GNU/Linux) iD8DBQE+vIjiW6pHgIdAuOMRAm6+AKCg3P6rUZOvUEcv4emlpwXiGwAA8ACfUlLG MuHfr7Pmc8yk017a1aWGLYw= =8zSh -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-10 0:48 ` David Brownell 2003-05-10 5:06 ` Brad Hards @ 2003-05-10 5:40 ` Greg KH 2003-05-10 5:55 ` William Lee Irwin III 2003-05-12 17:53 ` Max Krasnyansky 2 siblings, 1 reply; 30+ messages in thread From: Greg KH @ 2003-05-10 5:40 UTC (permalink / raw) To: David Brownell Cc: Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel On Fri, May 09, 2003 at 05:48:16PM -0700, David Brownell wrote: > Greg KH wrote: > >On Fri, May 09, 2003 at 03:35:36PM -0700, Max Krasnyansky wrote: > > > >>Ok. Sounds like it should be > >> uint32_t hcd_cb[16]; // 64 bytes for internal use by HCD > >> uint32_t drv_cb[2]; // 8 bytes for internal use by USB driver > > > > > >s/uint32_t/u32/ please. > > "u32" is prettier, but is there actually a policy against using > the more standard type names? (POSIX, someone had said.) Yes there is. Linus has stated this a few times on lkml in the past. I have an old linux journal article that talks about this that I need to turn into docbook and add to the kernel tree to set it in stone. thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-10 5:40 ` Greg KH @ 2003-05-10 5:55 ` William Lee Irwin III 2003-05-10 6:11 ` Greg KH 0 siblings, 1 reply; 30+ messages in thread From: William Lee Irwin III @ 2003-05-10 5:55 UTC (permalink / raw) To: Greg KH Cc: David Brownell, Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel On Fri, May 09, 2003 at 05:48:16PM -0700, David Brownell wrote: >> "u32" is prettier, but is there actually a policy against using >> the more standard type names? (POSIX, someone had said.) On Fri, May 09, 2003 at 10:40:15PM -0700, Greg KH wrote: > Yes there is. Linus has stated this a few times on lkml in the past. I > have an old linux journal article that talks about this that I need to > turn into docbook and add to the kernel tree to set it in stone. If someone could clarify the motive I'd be much obliged. Thanks. -- wli ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-10 5:55 ` William Lee Irwin III @ 2003-05-10 6:11 ` Greg KH 2003-05-10 6:14 ` William Lee Irwin III 0 siblings, 1 reply; 30+ messages in thread From: Greg KH @ 2003-05-10 6:11 UTC (permalink / raw) To: William Lee Irwin III, David Brownell, Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel On Fri, May 09, 2003 at 10:55:16PM -0700, William Lee Irwin III wrote: > On Fri, May 09, 2003 at 05:48:16PM -0700, David Brownell wrote: > >> "u32" is prettier, but is there actually a policy against using > >> the more standard type names? (POSIX, someone had said.) > > On Fri, May 09, 2003 at 10:40:15PM -0700, Greg KH wrote: > > Yes there is. Linus has stated this a few times on lkml in the past. I > > have an old linux journal article that talks about this that I need to > > turn into docbook and add to the kernel tree to set it in stone. > > If someone could clarify the motive I'd be much obliged. Read Linus's comments in this thread for more insight: http://marc.theaimsgroup.com/?t=102806382900001 Hope this helps, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-10 6:11 ` Greg KH @ 2003-05-10 6:14 ` William Lee Irwin III 0 siblings, 0 replies; 30+ messages in thread From: William Lee Irwin III @ 2003-05-10 6:14 UTC (permalink / raw) To: Greg KH Cc: David Brownell, Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel On Fri, May 09, 2003 at 10:55:16PM -0700, William Lee Irwin III wrote: >> If someone could clarify the motive I'd be much obliged. On Fri, May 09, 2003 at 11:11:14PM -0700, Greg KH wrote: > Read Linus's comments in this thread for more insight: > http://marc.theaimsgroup.com/?t=102806382900001 That looks relatively arbitrary but I honestly can't be arsed to have an opinion, only to be informed. Thanks. -- wli ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-10 0:48 ` David Brownell 2003-05-10 5:06 ` Brad Hards 2003-05-10 5:40 ` Greg KH @ 2003-05-12 17:53 ` Max Krasnyansky 2003-05-12 18:01 ` Greg KH 2 siblings, 1 reply; 30+ messages in thread From: Max Krasnyansky @ 2003-05-12 17:53 UTC (permalink / raw) To: David Brownell, Greg KH; +Cc: Linux Kernel Mailing List, linux-usb-devel At 05:48 PM 5/9/2003, David Brownell wrote: >Greg KH wrote: >>On Fri, May 09, 2003 at 03:35:36PM -0700, Max Krasnyansky wrote: >> >>>Ok. Sounds like it should be >>> uint32_t hcd_cb[16]; // 64 bytes for internal use by HCD >>> uint32_t drv_cb[2]; // 8 bytes for internal use by USB driver >> >>s/uint32_t/u32/ please. > >"u32" is prettier, but is there actually a policy against using >the more standard type names? (POSIX, someone had said.) > > >>And if this is going to be used for pointers, why not just say they are >>pointers? Otherwise people are going to have to be careful with 32 vs. >>64 bit kernels to not overrun their space. >>struct sk_buff uses a char, any reason not to use that here too? Has >>being a char made things more difficult for that structure over time? > >No, it's just that in some similar cases having the value be "long" >(not "32 bit unsigned") has been simpler. I'm not religious. I don't care either. 'char' is ok for me. So, I guess in general you're ok with adding ->drv_cb and ->hcd_cb to 'struct urb' ? Max ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-12 17:53 ` Max Krasnyansky @ 2003-05-12 18:01 ` Greg KH 2003-05-12 18:55 ` Max Krasnyansky 2003-05-12 20:23 ` David Brownell 0 siblings, 2 replies; 30+ messages in thread From: Greg KH @ 2003-05-12 18:01 UTC (permalink / raw) To: Max Krasnyansky Cc: David Brownell, Linux Kernel Mailing List, linux-usb-devel On Mon, May 12, 2003 at 10:53:48AM -0700, Max Krasnyansky wrote: > > So, I guess in general you're ok with adding ->drv_cb and ->hcd_cb to 'struct urb' ? I am, as long as someone uses it :) thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-12 18:01 ` Greg KH @ 2003-05-12 18:55 ` Max Krasnyansky 2003-05-12 20:23 ` David Brownell 1 sibling, 0 replies; 30+ messages in thread From: Max Krasnyansky @ 2003-05-12 18:55 UTC (permalink / raw) To: Greg KH; +Cc: David Brownell, Linux Kernel Mailing List, linux-usb-devel At 11:01 AM 5/12/2003, Greg KH wrote: >On Mon, May 12, 2003 at 10:53:48AM -0700, Max Krasnyansky wrote: >> >> So, I guess in general you're ok with adding ->drv_cb and ->hcd_cb to 'struct urb' ? > >I am, as long as someone uses it :) Ok then. I'll go ahead and change my driver to use ->drv_list and ->drv_cb and send the patch to you. Thanks Max ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [linux-usb-devel] Re: [Bluetooth] HCI USB driver update. Support for SCO over HCI USB. 2003-05-12 18:01 ` Greg KH 2003-05-12 18:55 ` Max Krasnyansky @ 2003-05-12 20:23 ` David Brownell 1 sibling, 0 replies; 30+ messages in thread From: David Brownell @ 2003-05-12 20:23 UTC (permalink / raw) To: Greg KH; +Cc: Max Krasnyansky, Linux Kernel Mailing List, linux-usb-devel Greg KH wrote: > On Mon, May 12, 2003 at 10:53:48AM -0700, Max Krasnyansky wrote: > >>So, I guess in general you're ok with adding ->drv_cb and ->hcd_cb >> to 'struct urb' ? > > > I am, as long as someone uses it :) I'm unlikely to dig up that patch making ohci-hcd use a hcd_cb, which would mean that the submit path (a) gets rid of one fault mode, (b) slightly speeds up the submit path, and (c) all but stops using the heap. Even though they'd be fine updates ... I'd be glad to see someone else contribute such cleanups though! The uhci-hcd version would be trickier, and on 64bit CPUs it would not fit in the 64 bytes Max suggested -- without some work to shrink that driver. - Dave ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2003-05-12 19:56 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <200304290317.h3T3HOdA027579@hera.kernel.org> 2003-04-29 4:15 ` [Bluetooth] HCI USB driver update. Support for SCO over HCI USB Greg KH 2003-04-29 20:29 ` Max Krasnyansky 2003-04-29 21:15 ` Greg KH 2003-04-29 21:34 ` Oliver Neukum 2003-04-29 21:40 ` Greg KH 2003-04-29 22:24 ` [linux-usb-devel] " Oliver Neukum 2003-04-30 0:44 ` Max Krasnyansky 2003-04-30 7:06 ` Oliver Neukum 2003-04-29 21:55 ` Max Krasnyansky 2003-04-29 22:04 ` Greg KH 2003-04-29 21:39 ` [linux-usb-devel] " David Brownell 2003-04-29 21:37 ` Greg KH 2003-04-29 22:04 ` Max Krasnyansky 2003-04-30 4:55 ` David Brownell 2003-05-08 22:55 ` Max Krasnyansky 2003-05-09 19:06 ` David Brownell 2003-05-09 19:29 ` Greg KH 2003-05-09 22:35 ` Max Krasnyansky 2003-05-09 22:35 ` Max Krasnyansky 2003-05-09 23:05 ` Greg KH 2003-05-10 0:48 ` David Brownell 2003-05-10 5:06 ` Brad Hards 2003-05-10 5:40 ` Greg KH 2003-05-10 5:55 ` William Lee Irwin III 2003-05-10 6:11 ` Greg KH 2003-05-10 6:14 ` William Lee Irwin III 2003-05-12 17:53 ` Max Krasnyansky 2003-05-12 18:01 ` Greg KH 2003-05-12 18:55 ` Max Krasnyansky 2003-05-12 20:23 ` David Brownell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).