Re: [PATCH] usb: gadget: file_storage: don't assume wMaxPacketSize to be 512

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 4 Apr 2011, Felipe Balbi wrote:

> On Mon, Apr 04, 2011 at 11:39:49AM -0400, Alan Stern wrote:
> > On Mon, 4 Apr 2011, Felipe Balbi wrote:
> > 
> > > Like I said before: HW requests me to give lengths which are divisible
> > > by wMaxPacketSize. I can't quote parts of the HW docs here, but I *must*
> > > give the HW sizes which are divisible by wMaxPacketSize in case of OUT
> > > transfers.
> > 
> > Ah, I didn't realize this.  It's ironic then that Mian's patch reducing
> > the size of the CBW request length was just submitted.  Well, we can
> 
> heh.
> 
> > retract that patch easily enough if necessary...
> 
> ok. I think it's better to revert that and apply the patch using
> set_bulk_out_req_length() which I sent earlier, see below...

All right, we can do something like that.  I'm not sure I kept a copy 
of your earlier patch, and anyway it would need to be redone on top of 
the patch I sent yesterday.  I'll post an updated version shortly.

> > This hardware restriction doesn't affect only g_file_storage; you will
> > have to take it into account with _every_ gadget driver.  The only
> > general solution is to use a bounce buffer.  Collect all but the last
> > partial packet in the gdaget's transfer buffer as usual, then use a
> > private maxpacket-sized buffer for the remainder, which you then copy
> > to the end of the transfer buffer.
> 
> A bounce buffer isn't good enough. Remember I'm talking USB3, with the
> current 16K buffer size in g_file_storage, we will get one IRQ every:
> 
> (16 << 10) / (5 << 30) = ~3us

Hmm.  Here's what I get: SuperSpeed is 5 Gb/s = 500 MB/s = 500 KB/ms =
61 KB/uf.  Therefore a 16-KB buffer will be transferred in about 1/4
uf, or 32 us.  Probably slightly more than that because of protocol 
overhead.

Of course, that's not long enough.  With SuperSpeed, the buffer size
should be increased to at least 64 KB for optimal throughput.

> If every 3us I need to do a memcpy() of the remaining 1..1023 bytes that
> won't fly. The internal DMA, of course, can handle many requests and
> interrupt only at the end, which would decrease the amount of
> interrupts, but that would mean I would have to:
> 
> list_for_each_entry(req, &dev->req_list, list) {
> 	memcpy(req->request.buf + req->request.actual,
> 		req->bounce_buffer, req->remaining);
> 
> 	req->request.complete(&req->ep, &req->request);
> }
> 
> depending on the amount of requests which I have, that'll mean a lot of
> time spent handling one IRQ. IMHO, it's much easier - and much better
> for most of the other UDC controllers, to have req->length for OUT
> transfers to be aligned on ep->maxpacket.
> 
> That way, I don't need bounce buffers, I don't need to do any memcpy()
> and I only need to iterate over the list of requests and call the
> complete() method like it should be.
> 
> What I actually need from gadget drivers, for USB3 at least, is for them
> to queue as many requests as possible, so the storm of interrupts can
> be decreased and we have many buffers to play with.

Is it really worthwhile to worry about this?  With USB-3 you probably
want to run a UAS gadget instead of g_file_storage anyway.

> If we have, e.g. 8 requests for a particular gadget driver, we could
> interrupt after every 4 requests so that while HW is handling 4, gadget
> driver is processing and recycling the other 4.

g_file_storage does allows the number of buffers to be increased.  
With some effort, the driver could be changed to set the no_interrupt
flag on many of the requests, keeping the 16-KB buffer size -- but
again, why not use a UAS gadget instead?

> BTW, from what I could see, Storage gadget is the only one doesn't
> ensure OUT transfers to be aligned on ep->maxpacket, see gadget ether
> for instance:

...

> so IMHO, it's better to keep request->length for OUT transfers aligned
> on ep->maxpacket. What do you say ?

I have not objection.

Alan Stern

--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux