On Saturday 19 May 2007, Ragner Magalhaes wrote: > The following series implements USB Composite Gadget Support.
And I'm finally looking at this again. :) These patches seem to be "step one", just starting to make conversions. I'm assuming that you're circulating them to get feedback (which, sorry, has been a bit hard to come by) before later steps... > [PATCH 1/6] USB gadget driver. This is badly misnamed -- "gadget.c" implies that it's the one and only gadget driver. But it isn't; a "gadget" is whatever talks to the abstracted hardware. And gadgets aren't required to use this utility code. This is instead a composite gadget driver optimized to work with a *single* function driver. It needs to be renamed. I'd suggest "single.c", "singlefunc.c", or somesuch. > [PATCH 2/6] Composite gadget driver. This should actually be patch #1. Among other things, it defines the <linux/usb/composite.h> header used in your patch #1! Now, this patch is basically a version of something I sent some time back. Explicitly *without* a signed-off-by. So it's wrong for you to both (a) list it as "From:" you, and also to (b) add my signed-off-by line. > [PATCH 3/6] Composite gadget driver upgrade. I'm reviewing this with the expectation that the good bits will merge directly into your #2. In fact, since you're including an older version of my patch, some of that has already been done... > [PATCH 4/6] Kconfig modifications for USB Composite gadget support. Not really ... there's (a) stuff to switch two gadget drivers over so they're "function" drivers using the "single.c" glue code, and (b) stuff to build all the composite gadget utility code into one module, but it's missing (c) an actual composite gadget. I know you had an actual composite gadget in earlier patches ... you should include one, so that we can see that it all works. You seem to be assuming that for example that a g_ether module would be an Ethernet gadget driver module if there's no composite gadget defined, else it would be a function module. That's the wrong way to do things. The point of the "g_*" naming convention, and also the Kconfig, is that when you select "Ethernet Gadget" you get an Ethernet gadget module. The way to assemble a given composite gadget, let's call it "Fred" and named "g_fred", should be to link all the functions together into that "g_fred.ko" module. It must be possible to modprobe "g_ether" *OR* "g_fred"... > [PATCH 5/6] Composite File Storage gadget support. > [PATCH 6/6] Composite Ether gadget support. #5 and #6 just convert those gadget drivers to "function" drivers. But I see that they still have most of the code which should have been eliminated by such a conversion ... > The Composite Gadget can handle one or two configurations. > When the first usb_function modprobe'ed Don't assume functions can be modprobed by themselves to mix'n'match. There needs to be "glue" code which understands that for example a specific combination has particular product and vendor IDs, and in general uses particular device descriptors. The device class info can matter a lot, for example... > has two configurations > the Composite Gadget will have two configurations, for the > other functions modprobe'ed after will be used the selected or > standard configuration only, their interfaces will be part > of the Composite's Configurations. > Then exchanging configs in the Composite will only affect the > first function. > In case the first modprobe'ed function has only one configure, > the Composite Gadget WILL have only one configuration. > This behavior is useful when modprobe'ing g_ether as the first > usb_function due to the RNDIS and CDC Configurations. That seems like a fair approach to take, at least in terms of managing configurations. Although I don't see why only the first function should be special. On the other hand, folk are finding that RNDIS is significantly less than Microsoft has promised, so maybe we shouldn't care so much about that. Wouldn't it simplify things if we didn't have to handle that case? ... and then there's the whole "other speed config" thing. There need to be both full speed and high speed configs, if the hardware supports high speed operation... > When the ether is modprobe'ed first: > > (Device Decriptor) > / \ > (Config 0) (Config 1) > (eth config 0 + fsg config) (eth config 1 + fsg config) Did you test that with MS-Windows? The point about multiple configs that Microsoft demands that the first listed config be RNDIS. (That's not necessarily config 0!!) I don't recall any expectation that "RNDIS + mass storage" could work... ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
