[PATCH 0/2] usb/gadget: independent registration of gadgets and gadget

[PATCH 0/2] usb/gadget: independent registration of gadgets and gadget

[Ruslan Bilovol]

This patchset adds independent registration of gadgets
and gadget drivers to udc-core. This is very useful for
built-in modules into kernel case since it's possible
situation that gadget driver is probing at a time
when no gadgets are registered in udc-core.
In this case instead of silently failing without
of any attempt to recover, with independent registration
of gadgets and gadget drivers there is no matter
in which order gadgets and gadget drivers are
probed/registered.
Also it's possible to add/remove gadgets or gadget
drivers many times since it's now correctly handled
by the udc-core

This patch has side-effect on gadget drivers that had
__init/__exit attributes on some paths like bind/unbind
and (since bind/unbind may happen at any time) should
not use them now. This is covered by second patch
(please let me know if I need to break it into separate
patches for each gadget driver)

Ruslan Bilovol (2):
  usb: gadget: udc-core: independent registration of gadgets and gadget
    drivers
  usb: gadget: legacy: don't use __init/__exit attributes for
    bind/unbind path

 drivers/usb/gadget/legacy/acm_ms.c       |   6 +-
 drivers/usb/gadget/legacy/audio.c        |   6 +-
 drivers/usb/gadget/legacy/cdc2.c         |   6 +-
 drivers/usb/gadget/legacy/dbgp.c         |   2 +-
 drivers/usb/gadget/legacy/ether.c        |   8 +--
 drivers/usb/gadget/legacy/gmidi.c        |   6 +-
 drivers/usb/gadget/legacy/hid.c          |   6 +-
 drivers/usb/gadget/legacy/mass_storage.c |   4 +-
 drivers/usb/gadget/legacy/multi.c        |  16 ++---
 drivers/usb/gadget/legacy/ncm.c          |   6 +-
 drivers/usb/gadget/legacy/nokia.c        |   6 +-
 drivers/usb/gadget/legacy/printer.c      |   6 +-
 drivers/usb/gadget/legacy/serial.c       |   2 +-
 drivers/usb/gadget/legacy/webcam.c       |   4 +-
 drivers/usb/gadget/legacy/zero.c         |   2 +-
 drivers/usb/gadget/udc/udc-core.c        | 112 ++++++++++++++++++++++++++++---
 16 files changed, 147 insertions(+), 51 deletions(-)

-- 
1.9.1

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

[Ruslan Bilovol]

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.

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;
 };
 
 static struct class *udc_class;
 static LIST_HEAD(udc_list);
+static LIST_HEAD(gadget_driver_pending_list);
 static DEFINE_MUTEX(udc_lock);
 
+static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *driver,
+								bool bind_by_name);
 /* ------------------------------------------------------------------------- */
 
 #ifdef	CONFIG_HAS_DMA
@@ -244,6 +254,7 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
 {
 	struct usb_udc		*udc;
 	int			ret = -ENOMEM;
+	struct pending_gadget_driver *pending;
 
 	udc = kzalloc(sizeof(*udc), GFP_KERNEL);
 	if (!udc)
@@ -288,6 +299,24 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
 
 	usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
 
+	if (!list_empty(&gadget_driver_pending_list)) {
+		pending = list_first_entry(&gadget_driver_pending_list,
+					struct pending_gadget_driver, list);
+
+		if (pending->udc_name) {
+			if (!strcmp(pending->udc_name, dev_name(&udc->dev))) {
+				udc_bind_to_driver(udc, pending->driver, true);
+				list_del(&pending->list);
+				kfree(pending->udc_name);
+				kfree(pending);
+			}
+		} else {
+			udc_bind_to_driver(udc, pending->driver, false);
+			list_del(&pending->list);
+			kfree(pending);
+		}
+	}
+
 	mutex_unlock(&udc_lock);
 
 	return 0;
@@ -364,10 +393,32 @@ found:
 	dev_vdbg(gadget->dev.parent, "unregistering gadget\n");
 
 	list_del(&udc->list);
-	mutex_unlock(&udc_lock);
 
-	if (udc->driver)
+	if (udc->driver) {
+		struct pending_gadget_driver *pending;
+
+		pending = kzalloc(sizeof(*pending), GFP_KERNEL);
+		if (!pending)
+			goto err;
+
+		if (udc->bind_by_name) {
+			pending->udc_name = kstrdup(dev_name(&udc->dev),
+								GFP_KERNEL);
+			if (!pending->udc_name) {
+				kfree(pending);
+				goto err;
+			}
+		}
+
+		pending->driver = udc->driver;
+		list_add_tail(&pending->list, &gadget_driver_pending_list);
+
+		pr_info("udc-core: added [%s] to list of pending drivers\n",
+						pending->driver->function);
+err:
 		usb_gadget_remove_driver(udc);
+	}
+	mutex_unlock(&udc_lock);
 
 	kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
 	flush_work(&gadget->work);
@@ -378,7 +429,8 @@ EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
 
 /* ------------------------------------------------------------------------- */
 
-static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *driver)
+static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *driver,
+								bool bind_by_name)
 {
 	int ret;
 
@@ -386,6 +438,7 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri
 			driver->function);
 
 	udc->driver = driver;
+	udc->bind_by_name = bind_by_name;
 	udc->dev.driver = &driver->driver;
 	udc->gadget->dev.driver = &driver->driver;
 
@@ -423,14 +476,34 @@ int usb_udc_attach_driver(const char *name, struct usb_gadget_driver *driver)
 			break;
 	}
 	if (ret) {
-		ret = -ENODEV;
+		struct pending_gadget_driver *pending;
+
+		pending = kzalloc(sizeof(*pending), GFP_KERNEL);
+		if (!pending) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		pending->driver = driver;
+		pending->udc_name = kstrdup(name, GFP_KERNEL);
+		if (!pending->udc_name) {
+			kfree(pending);
+			ret = -ENOMEM;
+			goto out;
+		}
+
+		list_add_tail(&pending->list, &gadget_driver_pending_list);
+
+		pr_info("udc-core: couldn't find the [%s] UDC "
+				"- add [%s] to list of pending drivers\n",
+				name, driver->function);
+		ret = 0;
 		goto out;
 	}
 	if (udc->driver) {
 		ret = -EBUSY;
 		goto out;
 	}
-	ret = udc_bind_to_driver(udc, driver);
+	ret = udc_bind_to_driver(udc, driver, true);
 out:
 	mutex_unlock(&udc_lock);
 	return ret;
@@ -440,6 +513,7 @@ EXPORT_SYMBOL_GPL(usb_udc_attach_driver);
 int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
 {
 	struct usb_udc		*udc = NULL;
+	struct pending_gadget_driver *pending;
 	int			ret;
 
 	if (!driver || !driver->bind || !driver->setup)
@@ -452,11 +526,22 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
 			goto found;
 	}
 
-	pr_debug("couldn't find an available UDC\n");
+	pending = kzalloc(sizeof(*pending), GFP_KERNEL);
+	if (!pending) {
+		mutex_unlock(&udc_lock);
+		return -ENOMEM;
+	}
+	pending->driver = driver;
+	list_add_tail(&pending->list, &gadget_driver_pending_list);
+
+	pr_info("udc-core: couldn't find an available UDC "
+			"- add [%s] to list of pending drivers\n",
+			driver->function);
+
 	mutex_unlock(&udc_lock);
-	return -ENODEV;
+	return 0;
 found:
-	ret = udc_bind_to_driver(udc, driver);
+	ret = udc_bind_to_driver(udc, driver, false);
 	mutex_unlock(&udc_lock);
 	return ret;
 }
@@ -465,6 +550,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_probe_driver);
 int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
 {
 	struct usb_udc		*udc = NULL;
+	struct pending_gadget_driver *pending = NULL;
 	int			ret = -ENODEV;
 
 	if (!driver || !driver->unbind)
@@ -480,6 +566,17 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
 			break;
 		}
 
+	if (ret) {
+		list_for_each_entry(pending, &gadget_driver_pending_list, list)
+			if (pending->driver == driver) {
+				list_del(&pending->list);
+				kfree(pending->udc_name);
+				kfree(pending);
+				ret = 0;
+				break;
+			}
+	}
+
 	mutex_unlock(&udc_lock);
 	return ret;
 }
-- 
1.9.1

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

[Alan Stern]

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

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

Just add a "pending" list_head into the usb_gadget_driver structure and 
forget about all the rest.  (Or try to find my patch in the mailing 
list archives somehow see if you think it needs to be changed.)

Alan Stern

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

[Ruslan Bilovol]

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

Best regards,
Ruslan

>
> Just add a "pending" list_head into the usb_gadget_driver structure and
> forget about all the rest.  (Or try to find my patch in the mailing
> list archives somehow see if you think it needs to be changed.)
>
> Alan Stern
>

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

[Peter Chen]

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

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

[Alan Stern]

On Sun, 8 Feb 2015, 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.

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

Where did this "by name" feature come from?  You did not mention it in 
the patch description.

Why bother matching by name?  Why not simply take the first available 
UDC?

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

We can already do that with the existing code.  There's no need for a 
patch.

Also, it's not clear that the existing gadget drivers will work 
properly if they are unbound from one UDC and then bound again to 
another one.  They were not written with that sort of thing in mind.


On Mon, 9 Feb 2015, Peter Chen wrote:

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

That's right.  The patch I wrote was a lot like Robert's patch (the 
marc.info URL above).  His approach can be simplified a little; the 
"attached" field isn't needed if the driver_list holds only unbound 
gadget drivers.

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

The mass-storage gadget driver can be fixed, if necessary.

Alan Stern

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

[Krzysztof Opasiak]

Hi,

(... snip ...)

> 
> > > 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
> 
> Where did this "by name" feature come from?  You did not mention it
> in
> the patch description.
> 
> Why bother matching by name?  Why not simply take the first
> available
> UDC?

Because you may have more than one udc. This would allow to pick one by
name just like using configfs interface.

> 
> > 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.
> 
> We can already do that with the existing code.  There's no need for
> a
> patch.
> 
> Also, it's not clear that the existing gadget drivers will work
> properly if they are unbound from one UDC and then bound again to
> another one.  They were not written with that sort of thing in
> mind.
> 

What you have described is one of basics configfs features.
You should be able to bind and unbind your gadget whenever you want
and it should work properly after doing:

## create gadget
$ echo "udc.0" > UDC
$ echo "" > UDC
$ echo "udc.1" > UDC

Function shouldn't care which udc it has been bound previously.
Only current one is important and on each unbind each function
should cleanup its state and prepare to be bound to another udc.
Configfs interface doesn't prohibit this and I haven't seen any
info about such restriction. If some function is not working in
such situation there is a bug in that function and it should be fixed.

I have tried to test this on my odroid with dwc2 and dummy_hcd.
Most of functions seems to be working but for example ecm isn't.
After some digging with Robert we found that it's always reusing
endpoints received in first bind(). Once again in my opinion it's
a bug which should be fixed and not treated as general assumption.

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opasiak@samsung.com

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

[Krzysztof Opasiak]

> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Krzysztof Opasiak
> Sent: Monday, February 09, 2015 7:06 PM
> To: 'Alan Stern'; 'Ruslan Bilovol'; 'Peter Chen'
> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org;
> 'Balbi, Felipe'; gregkh@linuxfoundation.org; Andrzej Pietrasiewicz
> Subject: RE: [PATCH 1/2] usb: gadget: udc-core: independent
> registration of gadgets and gadget drivers
> 
> Hi,
> 
> (... snip ...)
> 
> >
> > > > 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
> >
> > Where did this "by name" feature come from?  You did not mention
> it
> > in
> > the patch description.
> >
> > Why bother matching by name?  Why not simply take the first
> > available
> > UDC?
> 
> Because you may have more than one udc. This would allow to pick
> one by
> name just like using configfs interface.
> 
> >
> > > 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.
> >
> > We can already do that with the existing code.  There's no need
> for
> > a
> > patch.
> >
> > Also, it's not clear that the existing gadget drivers will work
> > properly if they are unbound from one UDC and then bound again to
> > another one.  They were not written with that sort of thing in
> > mind.
> >
> 
> What you have described is one of basics configfs features.
> You should be able to bind and unbind your gadget whenever you want
> and it should work properly after doing:
> 
> ## create gadget
> $ echo "udc.0" > UDC
> $ echo "" > UDC
> $ echo "udc.1" > UDC
> 
> Function shouldn't care which udc it has been bound previously.
> Only current one is important and on each unbind each function
> should cleanup its state and prepare to be bound to another udc.
> Configfs interface doesn't prohibit this and I haven't seen any
> info about such restriction. If some function is not working in
> such situation there is a bug in that function and it should be
> fixed.
> 
> I have tried to test this on my odroid with dwc2 and dummy_hcd.
> Most of functions seems to be working but for example ecm isn't.

^ above is ok

> After some digging with Robert we found that it's always reusing
> endpoints received in first bind().

Fixup:

That's bullshit ignore it please. ecm_opts->bound is not used to take
endpoints but only to register net device. Went too far after short
reading.
All in all, ecm is not working when binding form one udc to another.
Don't know exact reason, but in my opinion it's more a bug than common
assumption.


-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opasiak@samsung.com

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

[Alan Stern]

On Mon, 9 Feb 2015, Krzysztof Opasiak wrote:

> > Why bother matching by name?  Why not simply take the first
> > available
> > UDC?
> 
> Because you may have more than one udc. This would allow to pick one by
> name just like using configfs interface.

Clearly it would be more flexible to allow the user to do the matching, 
the way configfs does (sysfs too).

> > > 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.
> > 
> > We can already do that with the existing code.  There's no need for
> > a
> > patch.
> > 
> > Also, it's not clear that the existing gadget drivers will work
> > properly if they are unbound from one UDC and then bound again to
> > another one.  They were not written with that sort of thing in
> > mind.
> > 
> 
> What you have described is one of basics configfs features.
> You should be able to bind and unbind your gadget whenever you want
> and it should work properly after doing:
> 
> ## create gadget
> $ echo "udc.0" > UDC
> $ echo "" > UDC
> $ echo "udc.1" > UDC
> 
> Function shouldn't care which udc it has been bound previously.
> Only current one is important and on each unbind each function
> should cleanup its state and prepare to be bound to another udc.
> Configfs interface doesn't prohibit this and I haven't seen any
> info about such restriction.

It's not prohibited, but it also was never required.  Therefore it may 
not be implemented in all gadget drivers.

>  If some function is not working in
> such situation there is a bug in that function and it should be fixed.

That's fine.  I wasn't pointing out a fundamental limitation, just a 
fact that will have to be taken into account.

Anyway, instead of going through all this, why not do what I suggested 
earlier (see http://marc.info/?l=linux-usb&m=139888691230119&w=2) and 
create a "gadget" bus type?  That would give userspace explicit control 
over which gadget driver was bound to which UDC.

Or maybe that's not a very good fit with the existing code, since most 
gadget drivers assume they will be bound to only one UDC at a time.  So 
instead, why not create a sysfs interface that allows userspace to 
control which gadget drivers are bound to which UDCs?

As I recall, the original problem people were complaining about was
deferred probing.  They didn't need fancy matching; all they wanted was
the ability to bind a gadget driver to a UDC some time after the gadget
driver was loaded and initialized.  Something simple like Robert
Baldyga's patch is enough to do that.

Alan Stern

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

[Ruslan Bilovol]

Hi guys,

On Mon, Feb 9, 2015 at 10:00 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Mon, 9 Feb 2015, Krzysztof Opasiak wrote:
>
>> > Why bother matching by name?  Why not simply take the first
>> > available
>> > UDC?
>>
>> Because you may have more than one udc. This would allow to pick one by
>> name just like using configfs interface.
>
> Clearly it would be more flexible to allow the user to do the matching,
> the way configfs does (sysfs too).
>
>> > > 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.
>> >
>> > We can already do that with the existing code.  There's no need for
>> > a
>> > patch.
>> >
>> > Also, it's not clear that the existing gadget drivers will work
>> > properly if they are unbound from one UDC and then bound again to
>> > another one.  They were not written with that sort of thing in
>> > mind.
>> >
>>
>> What you have described is one of basics configfs features.
>> You should be able to bind and unbind your gadget whenever you want
>> and it should work properly after doing:
>>
>> ## create gadget
>> $ echo "udc.0" > UDC
>> $ echo "" > UDC
>> $ echo "udc.1" > UDC
>>
>> Function shouldn't care which udc it has been bound previously.
>> Only current one is important and on each unbind each function
>> should cleanup its state and prepare to be bound to another udc.
>> Configfs interface doesn't prohibit this and I haven't seen any
>> info about such restriction.

Thank you Krzysztof for this explanation that makes things more clear
for reviewers.

>
> It's not prohibited, but it also was never required.  Therefore it may
> not be implemented in all gadget drivers.
>
>>  If some function is not working in
>> such situation there is a bug in that function and it should be fixed.
>
> That's fine.  I wasn't pointing out a fundamental limitation, just a
> fact that will have to be taken into account.

I also don't see any restrictions to bind/unbind gadget driver few times
and in case of such bug we just can fix it. I didn't see any issue with
gadget drivers that I used for verification, however, to be honest, I didn't
test it with all possible ones.
Anyway, it should work in legacy way (one gadget driver is bound to one uds)
so current behavior at least is not broken.

>
> Anyway, instead of going through all this, why not do what I suggested
> earlier (see http://marc.info/?l=linux-usb&m=139888691230119&w=2) and
> create a "gadget" bus type?  That would give userspace explicit control
> over which gadget driver was bound to which UDC.

Exactly, my patch transforms udc-core to something like tiny bus with very basic
functionality. But in mentioned mailthread I see good ideas that is possible to
implement with small overhead.

>
> Or maybe that's not a very good fit with the existing code, since most
> gadget drivers assume they will be bound to only one UDC at a time.  So
> instead, why not create a sysfs interface that allows userspace to
> control which gadget drivers are bound to which UDCs?
>
> As I recall, the original problem people were complaining about was
> deferred probing.  They didn't need fancy matching; all they wanted was
> the ability to bind a gadget driver to a UDC some time after the gadget
> driver was loaded and initialized.  Something simple like Robert
> Baldyga's patch is enough to do that.

This simplicity is also a limitation. As I mentioned before, sometimes it is
needed to be able to bind/unbind gadget driver multiple times to different UDCs.
A real case I faced recently is SoC with HighSpeed-only and SuperSpeed-only
UDCs. SuperSpeed-only UDC can't work on USB 2.0 speeds and vice versa.
The system has USB3.0 USB connector with soldered HS lines from
mentioned HS-only and SS lines from SS-only UDCs. Each UDC has it's own
device driver, so if we want to use both of them with one gadget driver, we
must be able to bind/unbind it multiple times to different UDCs.
Another one usecase is users who can unload udc drivers without carrying
about gadget drivers.
Third usecase is, for example, developers who can switch between dummy_hcd
and real UDC hardware without unloading gadget driver.

I'll work on improved version and will send new patch soon...

Best regards,
Ruslan

>
> Alan Stern
>

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

[Krzysztof Opasiak]

> -----Original Message-----
> From: Ruslan Bilovol [mailto:ruslan.bilovol@gmail.com]
> Sent: Tuesday, February 10, 2015 12:46 AM
> To: Alan Stern
> Cc: Krzysztof Opasiak; Peter Chen; linux-usb@vger.kernel.org;
> linux-kernel@vger.kernel.org; Balbi, Felipe;
> gregkh@linuxfoundation.org; Andrzej Pietrasiewicz
> Subject: Re: [PATCH 1/2] usb: gadget: udc-core: independent
> registration of gadgets and gadget drivers
> 
> Hi guys,
> 
> On Mon, Feb 9, 2015 at 10:00 PM, Alan Stern
> <stern@rowland.harvard.edu> wrote:
> > On Mon, 9 Feb 2015, Krzysztof Opasiak wrote:
> >
> >> > Why bother matching by name?  Why not simply take the first
> >> > available
> >> > UDC?
> >>
> >> Because you may have more than one udc. This would allow to pick
> one by
> >> name just like using configfs interface.
> >
> > Clearly it would be more flexible to allow the user to do the
> matching,
> > the way configfs does (sysfs too).
> >
> >> > > 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.
> >> >
> >> > We can already do that with the existing code.  There's no
> need for
> >> > a
> >> > patch.
> >> >
> >> > Also, it's not clear that the existing gadget drivers will
> work
> >> > properly if they are unbound from one UDC and then bound again
> to
> >> > another one.  They were not written with that sort of thing in
> >> > mind.
> >> >
> >>
> >> What you have described is one of basics configfs features.
> >> You should be able to bind and unbind your gadget whenever you
> want
> >> and it should work properly after doing:
> >>
> >> ## create gadget
> >> $ echo "udc.0" > UDC
> >> $ echo "" > UDC
> >> $ echo "udc.1" > UDC
> >>
> >> Function shouldn't care which udc it has been bound previously.
> >> Only current one is important and on each unbind each function
> >> should cleanup its state and prepare to be bound to another udc.
> >> Configfs interface doesn't prohibit this and I haven't seen any
> >> info about such restriction.
> 
> Thank you Krzysztof for this explanation that makes things more
> clear
> for reviewers.
> 
> >
> > It's not prohibited, but it also was never required.  Therefore
> it may
> > not be implemented in all gadget drivers.
> >
> >>  If some function is not working in
> >> such situation there is a bug in that function and it should be
> fixed.
> >
> > That's fine.  I wasn't pointing out a fundamental limitation,
> just a
> > fact that will have to be taken into account.
> 
> I also don't see any restrictions to bind/unbind gadget driver few
> times
> and in case of such bug we just can fix it. I didn't see any issue
> with
> gadget drivers that I used for verification, however, to be honest,
> I didn't
> test it with all possible ones.
> Anyway, it should work in legacy way (one gadget driver is bound to
> one uds)
> so current behavior at least is not broken.
> 
> >
> > Anyway, instead of going through all this, why not do what I
> suggested
> > earlier (see http://marc.info/?l=linux-usb&m=139888691230119&w=2)
> and
> > create a "gadget" bus type?  That would give userspace explicit
> control
> > over which gadget driver was bound to which UDC.
> 
> Exactly, my patch transforms udc-core to something like tiny bus
> with very basic
> functionality. But in mentioned mailthread I see good ideas that is
> possible to
> implement with small overhead.
> 
> >
> > Or maybe that's not a very good fit with the existing code, since
> most
> > gadget drivers assume they will be bound to only one UDC at a
> time.  So
> > instead, why not create a sysfs interface that allows userspace
> to
> > control which gadget drivers are bound to which UDCs?
> >
> > As I recall, the original problem people were complaining about
> was
> > deferred probing.  They didn't need fancy matching; all they
> wanted was
> > the ability to bind a gadget driver to a UDC some time after the
> gadget
> > driver was loaded and initialized.  Something simple like Robert
> > Baldyga's patch is enough to do that.
> 
> This simplicity is also a limitation. As I mentioned before,
> sometimes it is
> needed to be able to bind/unbind gadget driver multiple times to
> different UDCs.
> A real case I faced recently is SoC with HighSpeed-only and
> SuperSpeed-only
> UDCs. SuperSpeed-only UDC can't work on USB 2.0 speeds and vice
> versa.
> The system has USB3.0 USB connector with soldered HS lines from
> mentioned HS-only and SS lines from SS-only UDCs. Each UDC has it's
> own
> device driver, so if we want to use both of them with one gadget
> driver, we
> must be able to bind/unbind it multiple times to different UDCs.
> Another one usecase is users who can unload udc drivers without
> carrying
> about gadget drivers.
> Third usecase is, for example, developers who can switch between
> dummy_hcd
> and real UDC hardware without unloading gadget driver.
> 

Just a stupid question. Why don't you use configfs composite gadget
instead of legacy gadgets?

In my opinion all things which you have described are working out-of-box
when you use configfs interface. It's mostly ready so you may create
equivalent of most legacy gadgets (apart from printer and tcm) and
just bind from one udc to another whenever you want.

Cheers,

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opasiak@samsung.com

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

[Ruslan Bilovol]

Hi Krzysztof,

On Tue, Feb 10, 2015 at 10:47 AM, Krzysztof Opasiak
<k.opasiak@samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Ruslan Bilovol [mailto:ruslan.bilovol@gmail.com]
>> Sent: Tuesday, February 10, 2015 12:46 AM
>> To: Alan Stern
>> Cc: Krzysztof Opasiak; Peter Chen; linux-usb@vger.kernel.org;
>> linux-kernel@vger.kernel.org; Balbi, Felipe;
>> gregkh@linuxfoundation.org; Andrzej Pietrasiewicz
>> Subject: Re: [PATCH 1/2] usb: gadget: udc-core: independent
>> registration of gadgets and gadget drivers
>>
>> Hi guys,
>>
>> On Mon, Feb 9, 2015 at 10:00 PM, Alan Stern
>> <stern@rowland.harvard.edu> wrote:
>> > On Mon, 9 Feb 2015, Krzysztof Opasiak wrote:
>> >
>> >> > Why bother matching by name?  Why not simply take the first
>> >> > available
>> >> > UDC?
>> >>
>> >> Because you may have more than one udc. This would allow to pick
>> one by
>> >> name just like using configfs interface.
>> >
>> > Clearly it would be more flexible to allow the user to do the
>> matching,
>> > the way configfs does (sysfs too).
>> >
>> >> > > 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.
>> >> >
>> >> > We can already do that with the existing code.  There's no
>> need for
>> >> > a
>> >> > patch.
>> >> >
>> >> > Also, it's not clear that the existing gadget drivers will
>> work
>> >> > properly if they are unbound from one UDC and then bound again
>> to
>> >> > another one.  They were not written with that sort of thing in
>> >> > mind.
>> >> >
>> >>
>> >> What you have described is one of basics configfs features.
>> >> You should be able to bind and unbind your gadget whenever you
>> want
>> >> and it should work properly after doing:
>> >>
>> >> ## create gadget
>> >> $ echo "udc.0" > UDC
>> >> $ echo "" > UDC
>> >> $ echo "udc.1" > UDC
>> >>
>> >> Function shouldn't care which udc it has been bound previously.
>> >> Only current one is important and on each unbind each function
>> >> should cleanup its state and prepare to be bound to another udc.
>> >> Configfs interface doesn't prohibit this and I haven't seen any
>> >> info about such restriction.
>>
>> Thank you Krzysztof for this explanation that makes things more
>> clear
>> for reviewers.
>>
>> >
>> > It's not prohibited, but it also was never required.  Therefore
>> it may
>> > not be implemented in all gadget drivers.
>> >
>> >>  If some function is not working in
>> >> such situation there is a bug in that function and it should be
>> fixed.
>> >
>> > That's fine.  I wasn't pointing out a fundamental limitation,
>> just a
>> > fact that will have to be taken into account.
>>
>> I also don't see any restrictions to bind/unbind gadget driver few
>> times
>> and in case of such bug we just can fix it. I didn't see any issue
>> with
>> gadget drivers that I used for verification, however, to be honest,
>> I didn't
>> test it with all possible ones.
>> Anyway, it should work in legacy way (one gadget driver is bound to
>> one uds)
>> so current behavior at least is not broken.
>>
>> >
>> > Anyway, instead of going through all this, why not do what I
>> suggested
>> > earlier (see http://marc.info/?l=linux-usb&m=139888691230119&w=2)
>> and
>> > create a "gadget" bus type?  That would give userspace explicit
>> control
>> > over which gadget driver was bound to which UDC.
>>
>> Exactly, my patch transforms udc-core to something like tiny bus
>> with very basic
>> functionality. But in mentioned mailthread I see good ideas that is
>> possible to
>> implement with small overhead.
>>
>> >
>> > Or maybe that's not a very good fit with the existing code, since
>> most
>> > gadget drivers assume they will be bound to only one UDC at a
>> time.  So
>> > instead, why not create a sysfs interface that allows userspace
>> to
>> > control which gadget drivers are bound to which UDCs?
>> >
>> > As I recall, the original problem people were complaining about
>> was
>> > deferred probing.  They didn't need fancy matching; all they
>> wanted was
>> > the ability to bind a gadget driver to a UDC some time after the
>> gadget
>> > driver was loaded and initialized.  Something simple like Robert
>> > Baldyga's patch is enough to do that.
>>
>> This simplicity is also a limitation. As I mentioned before,
>> sometimes it is
>> needed to be able to bind/unbind gadget driver multiple times to
>> different UDCs.
>> A real case I faced recently is SoC with HighSpeed-only and
>> SuperSpeed-only
>> UDCs. SuperSpeed-only UDC can't work on USB 2.0 speeds and vice
>> versa.
>> The system has USB3.0 USB connector with soldered HS lines from
>> mentioned HS-only and SS lines from SS-only UDCs. Each UDC has it's
>> own
>> device driver, so if we want to use both of them with one gadget
>> driver, we
>> must be able to bind/unbind it multiple times to different UDCs.
>> Another one usecase is users who can unload udc drivers without
>> carrying
>> about gadget drivers.
>> Third usecase is, for example, developers who can switch between
>> dummy_hcd
>> and real UDC hardware without unloading gadget driver.
>>
>
> Just a stupid question. Why don't you use configfs composite gadget
> instead of legacy gadgets?
>
> In my opinion all things which you have described are working out-of-box
> when you use configfs interface. It's mostly ready so you may create
> equivalent of most legacy gadgets (apart from printer and tcm) and
> just bind from one udc to another whenever you want.

It's because legacy gadgets are easy to use and usually don't need any
userspace-side configuration. Are there any plans to remove legacy
drivers in the future?

Best regards,
Ruslan

>
> Cheers,
>
> --
> Krzysztof Opasiak
> Samsung R&D Institute Poland
> Samsung Electronics
> k.opasiak@samsung.com
>
>
>
>



-- 
Best regards,
Ruslan Bilvol

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

[Andrzej Pietrasiewicz]

W dniu 15.02.2015 o 23:43, Ruslan Bilovol pisze:

<snip>

>>
>> In my opinion all things which you have described are working out-of-box
>> when you use configfs interface. It's mostly ready so you may create
>> equivalent of most legacy gadgets (apart from printer and tcm) and
>> just bind from one udc to another whenever you want.
>
> It's because legacy gadgets are easy to use and usually don't need any
> userspace-side configuration. Are there any plans to remove legacy
> drivers in the future?
>

I'm not going to express strong opinions here, but their name implies
that this can happen, some time in the future.

And I also think it will not happen before the userspace part
(libusbg, gt, gadgetd etc) is mature enough. My personal opinion
in that matter is that it will take at least a couple of years
to remove legacy gadgets entirely.

AP

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

[Ruslan Bilovol]

Hi Andrzej,

On Mon, Feb 16, 2015 at 10:07 AM, Andrzej Pietrasiewicz
<andrzej.p@samsung.com> wrote:
> W dniu 15.02.2015 o 23:43, Ruslan Bilovol pisze:
>
> <snip>
>
>>>
>>> In my opinion all things which you have described are working out-of-box
>>> when you use configfs interface. It's mostly ready so you may create
>>> equivalent of most legacy gadgets (apart from printer and tcm) and
>>> just bind from one udc to another whenever you want.
>>
>>
>> It's because legacy gadgets are easy to use and usually don't need any
>> userspace-side configuration. Are there any plans to remove legacy
>> drivers in the future?
>>
>
> I'm not going to express strong opinions here, but their name implies
> that this can happen, some time in the future.
>
> And I also think it will not happen before the userspace part
> (libusbg, gt, gadgetd etc) is mature enough. My personal opinion
> in that matter is that it will take at least a couple of years
> to remove legacy gadgets entirely.

OK, so it looks like there is a sense even to add new gadget/functions
with legacy support

Thanks,
Ruslan

>
> AP
>

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

[Andrzej Pietrasiewicz]

W dniu 17.02.2015 o 22:02, Ruslan Bilovol pisze:
> Hi Andrzej,
>
> On Mon, Feb 16, 2015 at 10:07 AM, Andrzej Pietrasiewicz
> <andrzej.p@samsung.com> wrote:
>> W dniu 15.02.2015 o 23:43, Ruslan Bilovol pisze:
>>
>> <snip>
>>
>>>>
>>>> In my opinion all things which you have described are working out-of-box
>>>> when you use configfs interface. It's mostly ready so you may create
>>>> equivalent of most legacy gadgets (apart from printer and tcm) and
>>>> just bind from one udc to another whenever you want.
>>>
>>>
>>> It's because legacy gadgets are easy to use and usually don't need any
>>> userspace-side configuration. Are there any plans to remove legacy
>>> drivers in the future?
>>>
>>
>> I'm not going to express strong opinions here, but their name implies
>> that this can happen, some time in the future.
>>
>> And I also think it will not happen before the userspace part
>> (libusbg, gt, gadgetd etc) is mature enough. My personal opinion
>> in that matter is that it will take at least a couple of years
>> to remove legacy gadgets entirely.
>
> OK, so it looks like there is a sense even to add new gadget/functions
> with legacy support
>

I'm not sure what you mean exactly.

For sure legacy gadgets are supported as long as they are
a part of the mainline kernel. So any changes you make
to the kernel must not affect the legacy gadgets, or you
need to modify the legacy gadgets too and have them working.

But adding new legacy-style gadgets is a completely different
story. IMHO you need a _very_ good reason to succeed,
but I remember Felipe expressing an opinion that chances
or merging another legacy gadget were zero.

AP

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

[Ruslan Bilovol]

Hi Alan,

On Mon, Feb 9, 2015 at 10:00 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Mon, 9 Feb 2015, Krzysztof Opasiak wrote:
>
>> > Why bother matching by name?  Why not simply take the first
>> > available
>> > UDC?
>>
>> Because you may have more than one udc. This would allow to pick one by
>> name just like using configfs interface.
>
> Clearly it would be more flexible to allow the user to do the matching,
> the way configfs does (sysfs too).
>
>> > > 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.
>> >
>> > We can already do that with the existing code.  There's no need for
>> > a
>> > patch.
>> >
>> > Also, it's not clear that the existing gadget drivers will work
>> > properly if they are unbound from one UDC and then bound again to
>> > another one.  They were not written with that sort of thing in
>> > mind.
>> >
>>
>> What you have described is one of basics configfs features.
>> You should be able to bind and unbind your gadget whenever you want
>> and it should work properly after doing:
>>
>> ## create gadget
>> $ echo "udc.0" > UDC
>> $ echo "" > UDC
>> $ echo "udc.1" > UDC
>>
>> Function shouldn't care which udc it has been bound previously.
>> Only current one is important and on each unbind each function
>> should cleanup its state and prepare to be bound to another udc.
>> Configfs interface doesn't prohibit this and I haven't seen any
>> info about such restriction.
>
> It's not prohibited, but it also was never required.  Therefore it may
> not be implemented in all gadget drivers.
>
>>  If some function is not working in
>> such situation there is a bug in that function and it should be fixed.
>
> That's fine.  I wasn't pointing out a fundamental limitation, just a
> fact that will have to be taken into account.
>
> Anyway, instead of going through all this, why not do what I suggested
> earlier (see http://marc.info/?l=linux-usb&m=139888691230119&w=2) and
> create a "gadget" bus type?  That would give userspace explicit control
> over which gadget driver was bound to which UDC.
>
> Or maybe that's not a very good fit with the existing code, since most
> gadget drivers assume they will be bound to only one UDC at a time.  So
> instead, why not create a sysfs interface that allows userspace to
> control which gadget drivers are bound to which UDCs?
>
> As I recall, the original problem people were complaining about was
> deferred probing.  They didn't need fancy matching; all they wanted was
> the ability to bind a gadget driver to a UDC some time after the gadget
> driver was loaded and initialized.  Something simple like Robert
> Baldyga's patch is enough to do that.

I looked over my patch and see that it doesn't automatically rebind
gadget driver to another available UDC after previous UDC is unbound.
The Gadget Bus mentioned previously is good thing but so far it seems there
is no users of such approach. So I left only deferred registration
from my patch.
I still keep it inside of udc-core since we also need to keep tracking UDC name
if somebody wanted to bind gadget driver to specific UDC and it looks
like good place to maintain this. I'll send second version of patches soon

-- 
Best regards,
Ruslan Bilvol

[PATCH 2/2] usb: gadget: legacy: don't use __init/__exit attributes for bind/unbind path

[Ruslan Bilovol]

Since it's possible now to do independent gadget and
gadget driver registration in udc-core, some of the
functions can't have __init/__exit attributes (almost
bind/unbind callbacks are affected)

Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
---
 drivers/usb/gadget/legacy/acm_ms.c       |  6 +++---
 drivers/usb/gadget/legacy/audio.c        |  6 +++---
 drivers/usb/gadget/legacy/cdc2.c         |  6 +++---
 drivers/usb/gadget/legacy/dbgp.c         |  2 +-
 drivers/usb/gadget/legacy/ether.c        |  8 ++++----
 drivers/usb/gadget/legacy/gmidi.c        |  6 +++---
 drivers/usb/gadget/legacy/hid.c          |  6 +++---
 drivers/usb/gadget/legacy/mass_storage.c |  4 ++--
 drivers/usb/gadget/legacy/multi.c        | 16 ++++++++--------
 drivers/usb/gadget/legacy/ncm.c          |  6 +++---
 drivers/usb/gadget/legacy/nokia.c        |  6 +++---
 drivers/usb/gadget/legacy/printer.c      |  6 +++---
 drivers/usb/gadget/legacy/serial.c       |  2 +-
 drivers/usb/gadget/legacy/webcam.c       |  4 ++--
 drivers/usb/gadget/legacy/zero.c         |  2 +-
 15 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/gadget/legacy/acm_ms.c b/drivers/usb/gadget/legacy/acm_ms.c
index c30b7b5..3a48aab 100644
--- a/drivers/usb/gadget/legacy/acm_ms.c
+++ b/drivers/usb/gadget/legacy/acm_ms.c
@@ -121,7 +121,7 @@ static struct usb_function *f_msg;
 /*
  * We _always_ have both ACM and mass storage functions.
  */
-static int __init acm_ms_do_config(struct usb_configuration *c)
+static int acm_ms_do_config(struct usb_configuration *c)
 {
 	struct fsg_opts *opts;
 	int	status;
@@ -174,7 +174,7 @@ static struct usb_configuration acm_ms_config_driver = {
 
 /*-------------------------------------------------------------------------*/
 
-static int __init acm_ms_bind(struct usb_composite_dev *cdev)
+static int acm_ms_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget	*gadget = cdev->gadget;
 	struct fsg_opts		*opts;
@@ -249,7 +249,7 @@ fail_get_msg:
 	return status;
 }
 
-static int __exit acm_ms_unbind(struct usb_composite_dev *cdev)
+static int acm_ms_unbind(struct usb_composite_dev *cdev)
 {
 	usb_put_function(f_msg);
 	usb_put_function_instance(fi_msg);
diff --git a/drivers/usb/gadget/legacy/audio.c b/drivers/usb/gadget/legacy/audio.c
index f46a395..ba95518 100644
--- a/drivers/usb/gadget/legacy/audio.c
+++ b/drivers/usb/gadget/legacy/audio.c
@@ -167,7 +167,7 @@ static const struct usb_descriptor_header *otg_desc[] = {
 
 /*-------------------------------------------------------------------------*/
 
-static int __init audio_do_config(struct usb_configuration *c)
+static int audio_do_config(struct usb_configuration *c)
 {
 	int status;
 
@@ -216,7 +216,7 @@ static struct usb_configuration audio_config_driver = {
 
 /*-------------------------------------------------------------------------*/
 
-static int __init audio_bind(struct usb_composite_dev *cdev)
+static int audio_bind(struct usb_composite_dev *cdev)
 {
 #ifndef CONFIG_GADGET_UAC1
 	struct f_uac2_opts	*uac2_opts;
@@ -276,7 +276,7 @@ fail:
 	return status;
 }
 
-static int __exit audio_unbind(struct usb_composite_dev *cdev)
+static int audio_unbind(struct usb_composite_dev *cdev)
 {
 #ifdef CONFIG_GADGET_UAC1
 	if (!IS_ERR_OR_NULL(f_uac1))
diff --git a/drivers/usb/gadget/legacy/cdc2.c b/drivers/usb/gadget/legacy/cdc2.c
index 2e85d94..8d1985c 100644
--- a/drivers/usb/gadget/legacy/cdc2.c
+++ b/drivers/usb/gadget/legacy/cdc2.c
@@ -104,7 +104,7 @@ static struct usb_function_instance *fi_ecm;
 /*
  * We _always_ have both CDC ECM and CDC ACM functions.
  */
-static int __init cdc_do_config(struct usb_configuration *c)
+static int cdc_do_config(struct usb_configuration *c)
 {
 	int	status;
 
@@ -153,7 +153,7 @@ static struct usb_configuration cdc_config_driver = {
 
 /*-------------------------------------------------------------------------*/
 
-static int __init cdc_bind(struct usb_composite_dev *cdev)
+static int cdc_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget	*gadget = cdev->gadget;
 	struct f_ecm_opts	*ecm_opts;
@@ -211,7 +211,7 @@ fail:
 	return status;
 }
 
-static int __exit cdc_unbind(struct usb_composite_dev *cdev)
+static int cdc_unbind(struct usb_composite_dev *cdev)
 {
 	usb_put_function(f_acm);
 	usb_put_function_instance(fi_serial);
diff --git a/drivers/usb/gadget/legacy/dbgp.c b/drivers/usb/gadget/legacy/dbgp.c
index 633683a..7c42b01 100644
--- a/drivers/usb/gadget/legacy/dbgp.c
+++ b/drivers/usb/gadget/legacy/dbgp.c
@@ -284,7 +284,7 @@ fail_1:
 	return -ENODEV;
 }
 
-static int __init dbgp_bind(struct usb_gadget *gadget,
+static int dbgp_bind(struct usb_gadget *gadget,
 		struct usb_gadget_driver *driver)
 {
 	int err, stp;
diff --git a/drivers/usb/gadget/legacy/ether.c b/drivers/usb/gadget/legacy/ether.c
index c5fdc61..4283969 100644
--- a/drivers/usb/gadget/legacy/ether.c
+++ b/drivers/usb/gadget/legacy/ether.c
@@ -222,7 +222,7 @@ static struct usb_function *f_rndis;
  * the first one present.  That's to make Microsoft's drivers happy,
  * and to follow DOCSIS 1.0 (cable modem standard).
  */
-static int __init rndis_do_config(struct usb_configuration *c)
+static int rndis_do_config(struct usb_configuration *c)
 {
 	int status;
 
@@ -264,7 +264,7 @@ MODULE_PARM_DESC(use_eem, "use CDC EEM mode");
 /*
  * We _always_ have an ECM, CDC Subset, or EEM configuration.
  */
-static int __init eth_do_config(struct usb_configuration *c)
+static int eth_do_config(struct usb_configuration *c)
 {
 	int status = 0;
 
@@ -318,7 +318,7 @@ static struct usb_configuration eth_config_driver = {
 
 /*-------------------------------------------------------------------------*/
 
-static int __init eth_bind(struct usb_composite_dev *cdev)
+static int eth_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget	*gadget = cdev->gadget;
 	struct f_eem_opts	*eem_opts = NULL;
@@ -447,7 +447,7 @@ fail:
 	return status;
 }
 
-static int __exit eth_unbind(struct usb_composite_dev *cdev)
+static int eth_unbind(struct usb_composite_dev *cdev)
 {
 	if (has_rndis()) {
 		usb_put_function(f_rndis);
diff --git a/drivers/usb/gadget/legacy/gmidi.c b/drivers/usb/gadget/legacy/gmidi.c
index e02a095..c098281 100644
--- a/drivers/usb/gadget/legacy/gmidi.c
+++ b/drivers/usb/gadget/legacy/gmidi.c
@@ -118,7 +118,7 @@ static struct usb_gadget_strings *dev_strings[] = {
 static struct usb_function_instance *fi_midi;
 static struct usb_function *f_midi;
 
-static int __exit midi_unbind(struct usb_composite_dev *dev)
+static int midi_unbind(struct usb_composite_dev *dev)
 {
 	usb_put_function(f_midi);
 	usb_put_function_instance(fi_midi);
@@ -133,7 +133,7 @@ static struct usb_configuration midi_config = {
 	.MaxPower	= CONFIG_USB_GADGET_VBUS_DRAW,
 };
 
-static int __init midi_bind_config(struct usb_configuration *c)
+static int midi_bind_config(struct usb_configuration *c)
 {
 	int status;
 
@@ -150,7 +150,7 @@ static int __init midi_bind_config(struct usb_configuration *c)
 	return 0;
 }
 
-static int __init midi_bind(struct usb_composite_dev *cdev)
+static int midi_bind(struct usb_composite_dev *cdev)
 {
 	struct f_midi_opts *midi_opts;
 	int status;
diff --git a/drivers/usb/gadget/legacy/hid.c b/drivers/usb/gadget/legacy/hid.c
index 614b06d..11de01b 100644
--- a/drivers/usb/gadget/legacy/hid.c
+++ b/drivers/usb/gadget/legacy/hid.c
@@ -106,7 +106,7 @@ static struct usb_gadget_strings *dev_strings[] = {
 
 /****************************** Configurations ******************************/
 
-static int __init do_config(struct usb_configuration *c)
+static int do_config(struct usb_configuration *c)
 {
 	struct hidg_func_node *e, *n;
 	int status = 0;
@@ -147,7 +147,7 @@ static struct usb_configuration config_driver = {
 
 /****************************** Gadget Bind ******************************/
 
-static int __init hid_bind(struct usb_composite_dev *cdev)
+static int hid_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget *gadget = cdev->gadget;
 	struct list_head *tmp;
@@ -205,7 +205,7 @@ put:
 	return status;
 }
 
-static int __exit hid_unbind(struct usb_composite_dev *cdev)
+static int hid_unbind(struct usb_composite_dev *cdev)
 {
 	struct hidg_func_node *n;
 
diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c
index 8e27a8c..d169d43 100644
--- a/drivers/usb/gadget/legacy/mass_storage.c
+++ b/drivers/usb/gadget/legacy/mass_storage.c
@@ -130,7 +130,7 @@ static int msg_thread_exits(struct fsg_common *common)
 	return 0;
 }
 
-static int __init msg_do_config(struct usb_configuration *c)
+static int msg_do_config(struct usb_configuration *c)
 {
 	struct fsg_opts *opts;
 	int ret;
@@ -170,7 +170,7 @@ static struct usb_configuration msg_config_driver = {
 
 /****************************** Gadget Bind ******************************/
 
-static int __init msg_bind(struct usb_composite_dev *cdev)
+static int msg_bind(struct usb_composite_dev *cdev)
 {
 	static const struct fsg_operations ops = {
 		.thread_exits = msg_thread_exits,
diff --git a/drivers/usb/gadget/legacy/multi.c b/drivers/usb/gadget/legacy/multi.c
index 39d27bb..4a4a1a1 100644
--- a/drivers/usb/gadget/legacy/multi.c
+++ b/drivers/usb/gadget/legacy/multi.c
@@ -149,7 +149,7 @@ static struct usb_function *f_acm_rndis;
 static struct usb_function *f_rndis;
 static struct usb_function *f_msg_rndis;
 
-static __init int rndis_do_config(struct usb_configuration *c)
+static int rndis_do_config(struct usb_configuration *c)
 {
 	struct fsg_opts *fsg_opts;
 	int ret;
@@ -206,7 +206,7 @@ err_func_rndis:
 	return ret;
 }
 
-static __ref int rndis_config_register(struct usb_composite_dev *cdev)
+static int rndis_config_register(struct usb_composite_dev *cdev)
 {
 	static struct usb_configuration config = {
 		.bConfigurationValue	= MULTI_RNDIS_CONFIG_NUM,
@@ -221,7 +221,7 @@ static __ref int rndis_config_register(struct usb_composite_dev *cdev)
 
 #else
 
-static __ref int rndis_config_register(struct usb_composite_dev *cdev)
+static int rndis_config_register(struct usb_composite_dev *cdev)
 {
 	return 0;
 }
@@ -237,7 +237,7 @@ static struct usb_function *f_acm_multi;
 static struct usb_function *f_ecm;
 static struct usb_function *f_msg_multi;
 
-static __init int cdc_do_config(struct usb_configuration *c)
+static int cdc_do_config(struct usb_configuration *c)
 {
 	struct fsg_opts *fsg_opts;
 	int ret;
@@ -295,7 +295,7 @@ err_func_ecm:
 	return ret;
 }
 
-static __ref int cdc_config_register(struct usb_composite_dev *cdev)
+static int cdc_config_register(struct usb_composite_dev *cdev)
 {
 	static struct usb_configuration config = {
 		.bConfigurationValue	= MULTI_CDC_CONFIG_NUM,
@@ -310,7 +310,7 @@ static __ref int cdc_config_register(struct usb_composite_dev *cdev)
 
 #else
 
-static __ref int cdc_config_register(struct usb_composite_dev *cdev)
+static int cdc_config_register(struct usb_composite_dev *cdev)
 {
 	return 0;
 }
@@ -321,7 +321,7 @@ static __ref int cdc_config_register(struct usb_composite_dev *cdev)
 
 /****************************** Gadget Bind ******************************/
 
-static int __ref multi_bind(struct usb_composite_dev *cdev)
+static int multi_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget *gadget = cdev->gadget;
 #ifdef CONFIG_USB_G_MULTI_CDC
@@ -466,7 +466,7 @@ fail:
 	return status;
 }
 
-static int __exit multi_unbind(struct usb_composite_dev *cdev)
+static int multi_unbind(struct usb_composite_dev *cdev)
 {
 #ifdef CONFIG_USB_G_MULTI_CDC
 	usb_put_function(f_msg_multi);
diff --git a/drivers/usb/gadget/legacy/ncm.c b/drivers/usb/gadget/legacy/ncm.c
index e90e23d..fb14e0c 100644
--- a/drivers/usb/gadget/legacy/ncm.c
+++ b/drivers/usb/gadget/legacy/ncm.c
@@ -107,7 +107,7 @@ static struct usb_function *f_ncm;
 
 /*-------------------------------------------------------------------------*/
 
-static int __init ncm_do_config(struct usb_configuration *c)
+static int ncm_do_config(struct usb_configuration *c)
 {
 	int status;
 
@@ -143,7 +143,7 @@ static struct usb_configuration ncm_config_driver = {
 
 /*-------------------------------------------------------------------------*/
 
-static int __init gncm_bind(struct usb_composite_dev *cdev)
+static int gncm_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget	*gadget = cdev->gadget;
 	struct f_ncm_opts	*ncm_opts;
@@ -186,7 +186,7 @@ fail:
 	return status;
 }
 
-static int __exit gncm_unbind(struct usb_composite_dev *cdev)
+static int gncm_unbind(struct usb_composite_dev *cdev)
 {
 	if (!IS_ERR_OR_NULL(f_ncm))
 		usb_put_function(f_ncm);
diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
index 9b8fd70..7ad8398 100644
--- a/drivers/usb/gadget/legacy/nokia.c
+++ b/drivers/usb/gadget/legacy/nokia.c
@@ -118,7 +118,7 @@ static struct usb_function_instance *fi_obex1;
 static struct usb_function_instance *fi_obex2;
 static struct usb_function_instance *fi_phonet;
 
-static int __init nokia_bind_config(struct usb_configuration *c)
+static int nokia_bind_config(struct usb_configuration *c)
 {
 	struct usb_function *f_acm;
 	struct usb_function *f_phonet = NULL;
@@ -224,7 +224,7 @@ err_get_acm:
 	return status;
 }
 
-static int __init nokia_bind(struct usb_composite_dev *cdev)
+static int nokia_bind(struct usb_composite_dev *cdev)
 {
 	struct usb_gadget	*gadget = cdev->gadget;
 	int			status;
@@ -307,7 +307,7 @@ err_usb:
 	return status;
 }
 
-static int __exit nokia_unbind(struct usb_composite_dev *cdev)
+static int nokia_unbind(struct usb_composite_dev *cdev)
 {
 	if (!IS_ERR_OR_NULL(f_obex1_cfg2))
 		usb_put_function(f_obex1_cfg2);
diff --git a/drivers/usb/gadget/legacy/printer.c b/drivers/usb/gadget/legacy/printer.c
index 9054598..da2a89b 100644
--- a/drivers/usb/gadget/legacy/printer.c
+++ b/drivers/usb/gadget/legacy/printer.c
@@ -1034,7 +1034,7 @@ unknown:
 	return value;
 }
 
-static int __init printer_func_bind(struct usb_configuration *c,
+static int printer_func_bind(struct usb_configuration *c,
 		struct usb_function *f)
 {
 	struct printer_dev *dev = container_of(f, struct printer_dev, function);
@@ -1162,7 +1162,7 @@ static struct usb_configuration printer_cfg_driver = {
 	.bmAttributes		= USB_CONFIG_ATT_ONE | USB_CONFIG_ATT_SELFPOWER,
 };
 
-static int __init printer_bind_config(struct usb_configuration *c)
+static int printer_bind_config(struct usb_configuration *c)
 {
 	struct usb_gadget	*gadget = c->cdev->gadget;
 	struct printer_dev	*dev;
@@ -1284,7 +1284,7 @@ static int printer_unbind(struct usb_composite_dev *cdev)
 	return 0;
 }
 
-static int __init printer_bind(struct usb_composite_dev *cdev)
+static int printer_bind(struct usb_composite_dev *cdev)
 {
 	int ret;
 
diff --git a/drivers/usb/gadget/legacy/serial.c b/drivers/usb/gadget/legacy/serial.c
index 1f5f978..812780f 100644
--- a/drivers/usb/gadget/legacy/serial.c
+++ b/drivers/usb/gadget/legacy/serial.c
@@ -174,7 +174,7 @@ out:
 	return ret;
 }
 
-static int __init gs_bind(struct usb_composite_dev *cdev)
+static int gs_bind(struct usb_composite_dev *cdev)
 {
 	int			status;
 
diff --git a/drivers/usb/gadget/legacy/webcam.c b/drivers/usb/gadget/legacy/webcam.c
index 04a3da2..d200ab4 100644
--- a/drivers/usb/gadget/legacy/webcam.c
+++ b/drivers/usb/gadget/legacy/webcam.c
@@ -334,7 +334,7 @@ static const struct uvc_descriptor_header * const uvc_ss_streaming_cls[] = {
  * USB configuration
  */
 
-static int __init
+static int
 webcam_config_bind(struct usb_configuration *c)
 {
 	int status = 0;
@@ -368,7 +368,7 @@ webcam_unbind(struct usb_composite_dev *cdev)
 	return 0;
 }
 
-static int __init
+static int
 webcam_bind(struct usb_composite_dev *cdev)
 {
 	struct f_uvc_opts *uvc_opts;
diff --git a/drivers/usb/gadget/legacy/zero.c b/drivers/usb/gadget/legacy/zero.c
index ff97ac9..f805bc2 100644
--- a/drivers/usb/gadget/legacy/zero.c
+++ b/drivers/usb/gadget/legacy/zero.c
@@ -289,7 +289,7 @@ static struct usb_function_instance *func_inst_lb;
 module_param_named(qlen, gzero_options.qlen, uint, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(qlen, "depth of loopback queue");
 
-static int __init zero_bind(struct usb_composite_dev *cdev)
+static int zero_bind(struct usb_composite_dev *cdev)
 {
 	struct f_ss_opts	*ss_opts;
 	struct f_lb_opts	*lb_opts;
-- 
1.9.1