On Tue, Sep 27, 2011 at 01:25:08PM +0800, Andiry Xu wrote: > On Mon, 2011-09-26 at 15:51 -0700, Sarah Sharp wrote: > > Hi Andiry, > > > > I dug the original branch out of my tree from when I started the dynamic > > ring expansion code. I've stuck it up on github, since kernel.org is > > still down: > > > > https://github.com/sarahsharp/xhci/tree/xhci-ring-expansion-simplified > > > > You might find something helpful in there. I'm not too attached to that > > code in particular over anything new you create, but I was trying to > > remember the pitfalls I ran into when I started coding the dynamic ring > > expansion. Looking at the patches I started out with (`git diff > > 7d6d62f^..e109a04), I notice a couple things: > > > > Oh, I don't realize that you've been working on it over a year ago. I'll > check that code first. No worries. I don't expect you to pay attention to every single branch I have on kernel.org. I think I was in the middle of the work right about the time the AMD team started pushing a large number of patchsets, and I put it aside to focus on code review and getting those patches queued, and simply didn't pick it up again. > > 1. My patchset is pretty careful about what it does with the toggle > > cycle bit. Does your patchset handle that properly when you > > concatenate the two rings, or does it leave the toggle cycle bit set > > in the last link TRB of the original ring, so you have the toggle > > cycle bit set twice in a ring? > > > > Yes, the cycle bit must be treated very carefully. I think the TC bit > should be only set in the last segment of a ring, so I made patch2 which > is used to allocate new segments with the same cycle bit setting as the > old ring. Ok, good. > > 2. You need to let drivers know how many scatter-gather list entries > > they can post in one URB. The USB storage driver in particular > > uses this to queue larger data phases of SCSI commands. You can do > > that by setting hcd->self.sg_tablesize to 0 (meaning "unlimited" > > entries). > > > > 3. Apparently I also needed to increase the .max_sectors > > variable in the USB storage driver to get bigger scatter-gather > > transfers. I think that would have to be a separate patch to the > > usb-storage list, but it might help you test your patchset with a > > non-isochronous device (and one that might use different sized > > transfers and exercise ring expansion at different times). > > > > Let me know if you have any other questions. Thanks for looking into > > writing the code for dynamic ring expansion. :) > > > > Apparently the ring expansion is much more complicated than I thought. > Thanks for the elaborate explanation! It's really appreciated and > helpful. I will re-consider the whole logic. I did this just for fun and > thought it may be helpful to you. It will be helpful for a couple different devices, so it would be appreciated if you could continue this work. Sarah Sharp -- 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
