Re: [PATCH 1/2] usb: gadget: udc-core: independent registration of gadgets and gadget drivers

[Peter Chen <peter.chen@freescale.com>]

On Sun, Feb 08, 2015 at 09:04:32PM +0200, Ruslan Bilovol wrote:
> Hi Alan,
> 
> On Thu, Jan 29, 2015 at 5:56 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Thu, 29 Jan 2015, Ruslan Bilovol wrote:
> >
> >> Change behavior during registration of gadgets and
> >> gadget drivers in udc-core. Instead of previous
> >> approach when for successful probe of usb gadget driver
> >> at least one usb gadget should be already registered
> >> use another one where gadget drivers and gadgets
> >> can be registered in udc-core independently.
> >>
> >> Independent registration of gadgets and gadget drivers
> >> is useful for built-in into kernel gadget and gadget
> >> driver case - because it's possible that gadget is
> >> really probed only on late_init stage (due to deferred
> >> probe) whereas gadget driver's probe is silently failed
> >> on module_init stage due to no any UDC added.
> >>
> >> Also it is useful for modules case - now there is no
> >> difference what module to insert first: gadget module
> >> or gadget driver one.
> >
> > It's possible to do all this much more simply.  In fact, I posted a
> > patch some time ago to do exactly this (but I can't find a copy of it
> > anywhere).
> 
> Unfortunately I didn't find your patch.
> 
> >
> >> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> >> ---
> >>  drivers/usb/gadget/udc/udc-core.c | 113 +++++++++++++++++++++++++++++++++++---
> >>  1 file changed, 105 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> >> index e31d574..4c9412b 100644
> >> --- a/drivers/usb/gadget/udc/udc-core.c
> >> +++ b/drivers/usb/gadget/udc/udc-core.c
> >> @@ -43,13 +43,23 @@ struct usb_udc {
> >>       struct usb_gadget_driver        *driver;
> >>       struct usb_gadget               *gadget;
> >>       struct device                   dev;
> >> +     bool                            bind_by_name;
> >> +     struct list_head                list;
> >> +};
> >> +
> >> +struct pending_gadget_driver {
> >> +     struct usb_gadget_driver        *driver;
> >> +     char                            *udc_name;
> >>       struct list_head                list;
> >>  };
> >
> > You don't need all this stuff.  What's the point of keeping track of
> > names?  If there are any unbound gadget drivers pending, a newly
> > registered UDC should bind to the first one available.
> 
> It's because gadget driver may be bound to usb_gadget in two ways:
>  - standard way - in this case any available udc will be picked up
>  - by name of udc, in this case only matching udc will be picked up
> 
> Main feature of my path is not only deferred binding of gadget driver,
> but also possibility to register/unregister udc at any time.
> This is useful for user who can load, for example, udc module
> if needed and unload it safely, not touching gadget driver.
> Another example is USB device controllers that consist of pair of
> HS+SS controllers, each one having its own udc driver. In this case
> it's possible to switch SS/HS by registering/unregistering corresponding
> udc and not touching gadget driver.
> 
> I did all of this inside of udc-core because it looks like to be best place for
> udc <-> gadget driver housekeeping. Also it is verified on lot of combinations
> of udc and gadget drivers that can be built-in or build as modules
> 

In fact, both I and Robert Baldyga posted patches to try fix this
problem.

http://marc.info/?l=linux-usb&m=139287784610046&w=2
http://lwn.net/Articles/601839/

I tried to use Robert's solution (fix some bugs) in internal tree, but
the mass storage gadget still has problems to work if unload udc first.
The possible reason should be: it has two places to call usb_composite_unregister.

-- 

Best Regards,
Peter Chen