On Thu, Apr 01, 2010 at 05:10:22PM -0400, Alan Stern wrote: > On Thu, 1 Apr 2010, Matthew Dharm wrote: > > > One overarching comment -- I think you're overreaching with this single > > patch. Implementing UASP support should be done first. Then, we can look > > at adding multiple commands in-flight separately. > > I disagree with part of this. The main advantage of UASP over the > older protocols is that allows concurrent execution of multiple > commands. If you're only going to do one command at a time, you almost > might as well not bother. > > The thing is, concurrent execution affects the whole design of the > driver. It can't work like usb-storage does, where we send off an URB, > wait for the response, send off a buffer of data, wait for the > response, etc. It has to be event-driven, because the device will send > back replies in whatever order it wants, not in the same order as the > original URBs were sent. > > So even if you don't want to support concurrent execution initially, > you have to design for it in advance. Otherwise, adding it would > require the entire driver to be rewritten anyhow. And once you have a > good design, adding the support is a pretty easy change. I think I actually just typo-ed that line. What I meant to say was that we should change the structure to support the event-driven model, and then implement UASP. Still two separate patches, but just a different order. You're definately right -- usb-storage would have to switch to an event-driven model in order to process whatever commands completed in whatever order the device chose. > > If the existing infrastructure of the usb-storage driver, with a single > > protocol and transport pointer isn't sufficient, then let's enhance the > > framework. Otherwise, this driver might as well as be completely separate > > from usb-storage, as just about the only parts it shares with the other > > protocol/transport drivers is the top-level interface into the SCSI core. > > Yes, that's the way it should end up going -- separate from > usb-storage. The only concern I have with this approach would be end-user confusion. But, since modules auto-load anyway, I guess this really isn't such a big deal. > > This either needs to exist in a unified framework with all the other > > sub-drivers, or should just be a separate driver completely. I would > > prefer the former. > > It could be done as a sub-driver, but my guess is that it wouldn't end > up using much of the existing core usb-storage code. Which means that > it might just as well be a whole new driver. Well, if we implemented the event-driven model, we could probably share a lot of code. But, much of those code paths would only be exercised for UASP. Matt -- Matthew Dharm Home: mdharm-usb@xxxxxxxxxxxxxxxxxx Maintainer, Linux USB Mass Storage Driver We can customize our colonels. -- Tux User Friendly, 12/1/1998
Attachment:
pgppV3FIbsEtc.pgp
Description: PGP signature