On Sat, 23 May 2009, Josua Dietze wrote: > Hi, all, > > I have a little patch ready (against 2.6.30-rc6) but am unsure if I > should post it here "officially". Frankly, this is my first, and I > don't know about the exact DOs and DONTs. > > So I rather include just a link here: > http://www.draisberghof.de/files/option_ms.diff In the future don't worry about such things. It's pointless to introduce barriers like this for people who want to read your code. Just include the patch inline in your email message, as described in Documentation/SubmittingPatches. > What it does: > - fixes the bug Alan Stern pointed at (reading the response after > switching) > - introduces an additional test for the vendor information included > in the INQUIRY data (ASCII); if it's not "Option", leave the > device alone. If it is, proceed according to the "option_zero_cd" > parameter It looks basically okay, but there are a couple of things worth noting: This whole business about searching for the mass-storage interface and endpoints is unnecessary, because that work has already been done. The interface is us->pusb_intf and the pipes are us->send_bulk_pipe and us->recv_bulk_pipe. All that stuff should be removed. You shouldn't return USB_STOR_TRANSPORT_GOOD from option_ms_init; return 0 instead. I have already submitted a patch changing the existing places where this mistake occurs. > Tested with Option GlobeSurfer Icon and my cell phone (Samsung > SGH_U900) which has the same ID when in storage mode. > > Bulk send/receive errors during the inquiry process are not exposed, > instead the result is the same as with any "non-Option" device: do > nothing. Is that OK? Yes. Especially since some people would argue that this code should never do anything in the first place! :-) > One thing I noticed: the switching function does not read the CSW > after the transfers. This might be a reason for quirks with > "non-Option" devices if the procedure is applied to them. You might want to go ahead and add it, then. What happens with the Option, if the mode switch succeeds and you try to read the CSW? 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
