Re: [PATCH 0/6] Composite Gadget Support

Hi dave,

first of all, thanks for your comments...

We'll work on them and send a newer version of the composite gadget support.

See some comments below.

On 7/29/07, David Brownell <[email protected]> wrote:
> 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.

We'll we were thinking about converting {serial, ether.
file_storage}.c into usb_functions for good, and use gadget.c built
with one of those function drivers to create a gadget driver.

Do you have a better solution here?

Alan, what do you think??

>
>
> > [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.

Sorry about that... git mis-using here... fixing right away

>
>
> > [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.

Yeah... we still didn't figure how to do this one...
Could any of you comment here? Dave? Alan?

>
> 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.

not really, it'll always be a function module/driver, when you load it
with gadget.c it'll become a gadget module, when you load it with
composite.c it'll become part of a composite gadget.

>
> 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 ...

yeah... we still need to work on a lot of code...

>
>
> > 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...

this was our first shot while thinking a way to build all the stuff
together without messing the driver up.

>
>
> > 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/
> _______________________________________________
> [email protected]
> To unsubscribe, use the last form field at:
> https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
>


-- 
Best Regards,

Felipe Balbi
[email protected]

-------------------------------------------------------------------------
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/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel