On Mon, Jun 04, 2007 at 10:38:44AM +0200, Marcel Holtmann wrote:
> Hi,
>
> after the discussion with Oliver at the LinuxTag last week, I started to
> re-write the hci_usb driver to remove this ugly cruft around the URB
> management in the driver. The basic driver (without ISOC support) is
> working perfectly fine and thanks to the reference count of the URB and
> the new USB anchor extension it is a really small and simple driver now.
>
> However the buffer management for URBs that get re-submitted all the
> time is really ugly. It can be done inside the driver, but I think the
> USB core should provide some helpers here.
>
> The attached patch is an attempt to integrate the buffer into the URB
> and let the URB take care of freeing it when it is no longer needed.
> This might not be optimal for all drivers, but it helps to reduce a lot
> of code in many drivers. And of course the old method of allocating or
> providing an external buffer is still available.
>
> The main use cases are interrupt and bulk in endpoints where fragmented
> packets are used and the driver has to reassemble them. In this cases
> one or multiple URBs with and attached buffer can be used. When shutting
> down these URBs via an USB anchor, it will take care of freeing the
> buffers and the driver doesn't have to worry about.
>
> The patch misses setup routines for interrupt and ISOC URBs, but they
> are straight forward. Please let me know what you think.
>
> Regards
>
> Marcel
>
> diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
> index 94ea972..e683726 100644
> --- a/drivers/usb/core/urb.c
> +++ b/drivers/usb/core/urb.c
> @@ -11,6 +11,11 @@
> static void urb_destroy(struct kref *kref)
> {
> struct urb *urb = to_urb(kref);
> +
> + if (urb->transfer_flags & URB_FREE_BUFFER)
> + usb_buffer_free(urb->dev, urb->transfer_buffer_length,
> + urb->transfer_buffer, urb->transfer_dma);
> +
> kfree(urb);
> }
>
> @@ -478,6 +483,36 @@ void usb_kill_urb(struct urb *urb)
I like this portion, if we make it kfree() instead of usb_buffer_free.
> spin_unlock_irq(&urb->lock);
> }
>
> +/**
> + * usb_setup_bulk_urb - macro to help initialize a bulk urb
<snip>
It seems this function is not needed as per the discussion in this
thread.
Care to resend this with just the above functionality? It will let me
clean up a number of different drivers.
thanks,
greg k-h
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
