Re: PATCH: (as265) Fix bluetooth driver's wait_for_urb()

[Greg KH <greg@kroah.com>]

On Wed, Apr 28, 2004 at 01:25:08PM -0400, Alan Stern wrote:
> Greg:
> 
> Your recent change to struct urb broke this function in the bluetooth 
> driver.

I know.  I really hate what the Bluetooth driver does, and it's up to
them to keep up with the changes in urbs due to them statically
including a urb in their structures.  That is what they agreed to when
they did this a while ago.  They are on their own here...

> You know, I think usb_wait_for_urb() would make an excellent 
> addition to usbcore.

No, no one should do that.  Just use the proper usb_alloc_urb() and
usb_free_urb() and you will be fine.

> At some future time we could consider replacing 
> synchronous unlink_urb with asynchronous unlink plus wait_for_urb.

Why?  Does it cause undue hardship in the host controllers to have a
synchronous unlink_urb?

> ===== drivers/bluetooth/hci_usb.c 1.43 vs edited =====
> --- 1.43/drivers/bluetooth/hci_usb.c	Wed Apr 21 01:11:06 2004
> +++ edited/drivers/bluetooth/hci_usb.c	Wed Apr 28 13:20:22 2004
> @@ -342,7 +342,7 @@
>  
>  static inline void hci_usb_wait_for_urb(struct urb *urb)
>  {
> -	while (atomic_read(&urb->count) > 1) {
> +	while (atomic_read(&urb->kref.refcount) > 1) {
>  		current->state = TASK_UNINTERRUPTIBLE;
>  		schedule_timeout((5 * HZ + 999) / 1000);
>  	}

I really just hate that whole function, it's such a hack.

So, Bluetooth developers, have you reconsidered your "need" to put a
static urb in your structure?  Are you convinced yet of the wrongness of
your ways?  Do you want a patch to change your subsystem to follow the
rest of the kernel with regards to USB?

thanks,

greg k-h