Hi,
On Tue, Apr 05, 2011 at 10:08:04AM -0400, Alan Stern wrote:
> 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.
Sure, I can rebase on top of what you send, no biggy :-)
> > > 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.
I think 64kb is still too small, we will get Interrupt on every 164 uS.
I think it's better to either start support sglists and allocate more
space, or have many different 16kb buffers and set no_interrupt
whenever necessary
> > 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.
I can't be sure what customers will do :-) Of course I can always
suggest to use UAS as that's definitely better.
> > 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?
See above :-)
> > 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.
cool, thanks for the fruitful discussion Alan. I'll wait until you send
your patch and will rebase mine on top of that :-) Thanks
--
balbi
--
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
