On Sat, Jul 31, 2010 at 10:52:54PM +0200, Christian Lamparter wrote:
> > > The driver uses a sophisticated 3 anchor layout:
> > > 1. rx_pool => a pool of prepared urbs, which can be
> > > submitted to the device anytime.
> >
> > Why a pool? Is allocating new ones somehow too slow? If so, why?
>
> Because the device uses some sort of streaming technology.
That doesn't matter.
> Basically, it aggregates multiple frames/command responses and traps
> into a single, large transfer (which can be as large as 32K)
> (Note: the _current_ limit is at 8K, but the HW supports up to 32K)
>
> So, the usual replacement buffers on-demand stuff is not an option.
Why? At some point you need to fill up an urb and send it down. You
can always just allocate it at that point in time.
Yes, urb pools are cool and fun and seem to work quicker, but I've yet
to see any platform that really needs them, as the USB transfer rates
dwarf any processing time to allocate the urbs dynamically as needed.
But hey, it's your code, and your complexity to manage them :)
> > > 2. rx_work => storage for received urbs, which will be
> > > processed in the rx tasklet.
> > >
> > > 3. rx_anchor => all active rx urbs (i.e.: those which have been
> > > submitted to the device)
> > >
> > > ---
> > > diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
> > > index 7c05555..93f8f4c 100644
> > > --- a/drivers/usb/core/urb.c
> > > +++ b/drivers/usb/core/urb.c
> > > @@ -137,6 +137,16 @@ void usb_anchor_urb(struct urb *urb, struct usb_anchor *anchor)
> > > }
> > > EXPORT_SYMBOL_GPL(usb_anchor_urb);
> > >
> > > +static void __usb_unanchor_urb(struct urb *urb, struct usb_anchor *anchor)
> > > +{
> > > + urb->anchor = NULL;
> > > +
> > > + list_del(&urb->anchor_list);
> > > + usb_put_urb(urb);
> > > + if (list_empty(&anchor->urb_list))
> > > + wake_up(&anchor->wait);
> >
> > You are calling wake_up with a spinlock held, is that ok?
> >
> uhh, I know that you can't sleep while holding a spinlock.
> But I haven't read anything about why you can't call
> wake_up (or complete for that matter?) while holding a lock.
Hm, I don't really know, for some reason I always thought you couldn't
do this, but I might be totally wrong.
> I see you point, though. Do you have an idea for an alternative
> way without making a real mess of it?
Off the top of my head, no, I don't.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
