Hello,
This is a resurrection of the patches initially submitted by Ruslan
Bilovol in the following thread: https://lkml.org/lkml/2015/6/22/554
The changes since the original submission (v5) includes rebase onto
latest linux-next branch, simplification of the code requested by Alan
Stern and Felipe Balbi, removal of a patch, which deleted __init/__exit
attributes (this change has been already merged) and fixes of the
checkpatch issues.
This feature is urgently needed, because it is not longer possible to
use workaround to avoid deferred probe in UDC drivers due to
not-yet-probed i2c regulator drivers (for more information see
https://lkml.org/lkml/2015/10/30/374 ).
This patchset has been successfully tested on Odroid XU3 boards with
DWC3 UDC driver being deferred by missing regulator drivers.
Best regards
Marek Szyprowski
Samsung R&D Institute Poland
Patch summary:
Ruslan Bilovol (4):
usb: gadget: bind UDC by name passed via usb_gadget_driver structure
usb: gadget: configfs: pass UDC name via usb_gadget_driver struct
usb: gadget: udc-core: remove unused usb_udc_attach_driver()
usb: gadget: udc-core: independent registration of gadgets and gadget
drivers
drivers/usb/gadget/configfs.c | 29 +++++++-------
drivers/usb/gadget/udc/udc-core.c | 79 ++++++++++++++++++++++-----------------
include/linux/usb/gadget.h | 8 +++-
3 files changed, 67 insertions(+), 49 deletions(-)
--
1.9.2
From: Ruslan Bilovol <[email protected]>
Introduce new 'udc_name' member to usb_gadget_driver structure.
The 'udc_name' is a name of UDC that usb_gadget_driver should
be bound to. If udc_name is NULL, it will be bound to any
available UDC.
Tested-by: Maxime Ripard <[email protected]>
Signed-off-by: Ruslan Bilovol <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
Tested-by: Peter Chen <[email protected]>
---
drivers/usb/gadget/udc/udc-core.c | 24 +++++++++++++++++++-----
include/linux/usb/gadget.h | 4 ++++
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index f660afb..429d64e 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -549,21 +549,35 @@ EXPORT_SYMBOL_GPL(usb_udc_attach_driver);
int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
{
struct usb_udc *udc = NULL;
- int ret;
+ int ret = -ENODEV;
if (!driver || !driver->bind || !driver->setup)
return -EINVAL;
mutex_lock(&udc_lock);
- list_for_each_entry(udc, &udc_list, list) {
- /* For now we take the first one */
- if (!udc->driver)
+ if (driver->udc_name) {
+ list_for_each_entry(udc, &udc_list, list) {
+ ret = strcmp(driver->udc_name, dev_name(&udc->dev));
+ if (!ret)
+ break;
+ }
+ if (ret)
+ ret = -ENODEV;
+ else if (udc->driver)
+ ret = -EBUSY;
+ else
goto found;
+ } else {
+ list_for_each_entry(udc, &udc_list, list) {
+ /* For now we take the first one */
+ if (!udc->driver)
+ goto found;
+ }
}
pr_debug("couldn't find an available UDC\n");
mutex_unlock(&udc_lock);
- return -ENODEV;
+ return ret;
found:
ret = udc_bind_to_driver(udc, driver);
mutex_unlock(&udc_lock);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 3d583a1..63963c2 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1012,6 +1012,8 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget)
* @reset: Invoked on USB bus reset. It is mandatory for all gadget drivers
* and should be called in_interrupt.
* @driver: Driver model state for this driver.
+ * @udc_name: A name of UDC this driver should be bound to. If udc_name is NULL,
+ * this driver will be bound to any available UDC.
*
* Devices are disabled till a gadget driver successfully bind()s, which
* means the driver will handle setup() requests needed to enumerate (and
@@ -1072,6 +1074,8 @@ struct usb_gadget_driver {
/* FIXME support safe rmmod */
struct device_driver driver;
+
+ char *udc_name;
};
--
1.9.2
From: Ruslan Bilovol <[email protected]>
Now when udc-core supports binding to specific UDC by passing
its name via 'udc_name' member of usb_gadget_driver struct,
switch to this generic approach.
Tested-by: Maxime Ripard <[email protected]>
Signed-off-by: Ruslan Bilovol <[email protected]>
[rebased and fixed checkpatch issues]
Signed-off-by: Marek Szyprowski <[email protected]>
Tested-by: Peter Chen <[email protected]>
---
drivers/usb/gadget/configfs.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 163d305..590c449 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -56,7 +56,6 @@ struct gadget_info {
struct list_head string_list;
struct list_head available_func;
- const char *udc_name;
struct usb_composite_driver composite;
struct usb_composite_dev cdev;
bool use_os_desc;
@@ -233,21 +232,23 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct config_item *item,
static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char *page)
{
- return sprintf(page, "%s\n", to_gadget_info(item)->udc_name ?: "");
+ char *udc_name = to_gadget_info(item)->composite.gadget_driver.udc_name;
+
+ return sprintf(page, "%s\n", udc_name ?: "");
}
static int unregister_gadget(struct gadget_info *gi)
{
int ret;
- if (!gi->udc_name)
+ if (!gi->composite.gadget_driver.udc_name)
return -ENODEV;
ret = usb_gadget_unregister_driver(&gi->composite.gadget_driver);
if (ret)
return ret;
- kfree(gi->udc_name);
- gi->udc_name = NULL;
+ kfree(gi->composite.gadget_driver.udc_name);
+ gi->composite.gadget_driver.udc_name = NULL;
return 0;
}
@@ -271,14 +272,16 @@ static ssize_t gadget_dev_desc_UDC_store(struct config_item *item,
if (ret)
goto err;
} else {
- if (gi->udc_name) {
+ if (gi->composite.gadget_driver.udc_name) {
ret = -EBUSY;
goto err;
}
- ret = usb_udc_attach_driver(name, &gi->composite.gadget_driver);
- if (ret)
+ gi->composite.gadget_driver.udc_name = name;
+ ret = usb_gadget_probe_driver(&gi->composite.gadget_driver);
+ if (ret) {
+ gi->composite.gadget_driver.udc_name = NULL;
goto err;
- gi->udc_name = name;
+ }
}
mutex_unlock(&gi->lock);
return len;
@@ -427,9 +430,9 @@ static int config_usb_cfg_unlink(
* remove the function.
*/
mutex_lock(&gi->lock);
- if (gi->udc_name)
+ if (gi->composite.gadget_driver.udc_name)
unregister_gadget(gi);
- WARN_ON(gi->udc_name);
+ WARN_ON(gi->composite.gadget_driver.udc_name);
list_for_each_entry(f, &cfg->func_list, list) {
if (f->fi == fi) {
@@ -873,10 +876,10 @@ static int os_desc_unlink(struct config_item *os_desc_ci,
struct usb_composite_dev *cdev = &gi->cdev;
mutex_lock(&gi->lock);
- if (gi->udc_name)
+ if (gi->composite.gadget_driver.udc_name)
unregister_gadget(gi);
cdev->os_desc_config = NULL;
- WARN_ON(gi->udc_name);
+ WARN_ON(gi->composite.gadget_driver.udc_name);
mutex_unlock(&gi->lock);
return 0;
}
--
1.9.2
From: Ruslan Bilovol <[email protected]>
Now when last user of usb_udc_attach_driver() is switched
to passing UDC name via usb_gadget_driver struct, it's safe
to remove this function
Tested-by: Maxime Ripard <[email protected]>
Signed-off-by: Ruslan Bilovol <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
Tested-by: Peter Chen <[email protected]>
---
drivers/usb/gadget/udc/udc-core.c | 26 --------------------------
include/linux/usb/gadget.h | 2 --
2 files changed, 28 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index 429d64e..f76ebc8 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -520,32 +520,6 @@ err1:
return ret;
}
-int usb_udc_attach_driver(const char *name, struct usb_gadget_driver *driver)
-{
- struct usb_udc *udc = NULL;
- int ret = -ENODEV;
-
- mutex_lock(&udc_lock);
- list_for_each_entry(udc, &udc_list, list) {
- ret = strcmp(name, dev_name(&udc->dev));
- if (!ret)
- break;
- }
- if (ret) {
- ret = -ENODEV;
- goto out;
- }
- if (udc->driver) {
- ret = -EBUSY;
- goto out;
- }
- ret = udc_bind_to_driver(udc, driver);
-out:
- mutex_unlock(&udc_lock);
- return ret;
-}
-EXPORT_SYMBOL_GPL(usb_udc_attach_driver);
-
int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
{
struct usb_udc *udc = NULL;
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 63963c2..ce2188d 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1121,8 +1121,6 @@ extern int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget, void (*release)(struct device *dev));
extern int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget);
extern void usb_del_gadget_udc(struct usb_gadget *gadget);
-extern int usb_udc_attach_driver(const char *name,
- struct usb_gadget_driver *driver);
/*-------------------------------------------------------------------------*/
--
1.9.2
From: Ruslan Bilovol <[email protected]>
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.
Tested-by: Maxime Ripard <[email protected]>
Signed-off-by: Ruslan Bilovol <[email protected]>
[simplified code as requested by Alan Stern and Felipe Balbi,
fixed checkpatch issues]
Signed-off-by: Marek Szyprowski <[email protected]>
Tested-by: Peter Chen <[email protected]>
---
drivers/usb/gadget/udc/udc-core.c | 41 ++++++++++++++++++++++++++++++---------
include/linux/usb/gadget.h | 2 ++
2 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index f76ebc8..fd73a3e 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -51,8 +51,12 @@ struct usb_udc {
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);
+
/* ------------------------------------------------------------------------- */
#ifdef CONFIG_HAS_DMA
@@ -356,6 +360,7 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
void (*release)(struct device *dev))
{
struct usb_udc *udc;
+ struct usb_gadget_driver *driver;
int ret = -ENOMEM;
udc = kzalloc(sizeof(*udc), GFP_KERNEL);
@@ -403,6 +408,18 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
udc->vbus = true;
+ /* pick up one of pending gadget drivers */
+ list_for_each_entry(driver, &gadget_driver_pending_list, pending) {
+ if (!driver->udc_name || strcmp(driver->udc_name,
+ dev_name(&udc->dev)) == 0) {
+ ret = udc_bind_to_driver(udc, driver);
+ if (ret)
+ goto err4;
+ list_del(&driver->pending);
+ break;
+ }
+ }
+
mutex_unlock(&udc_lock);
return 0;
@@ -473,10 +490,14 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
mutex_lock(&udc_lock);
list_del(&udc->list);
- mutex_unlock(&udc_lock);
- if (udc->driver)
+ if (udc->driver) {
+ struct usb_gadget_driver *driver = udc->driver;
+
usb_gadget_remove_driver(udc);
+ list_add(&driver->pending, &gadget_driver_pending_list);
+ }
+ mutex_unlock(&udc_lock);
kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
flush_work(&gadget->work);
@@ -535,11 +556,7 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
if (!ret)
break;
}
- if (ret)
- ret = -ENODEV;
- else if (udc->driver)
- ret = -EBUSY;
- else
+ if (!ret && !udc->driver)
goto found;
} else {
list_for_each_entry(udc, &udc_list, list) {
@@ -549,9 +566,11 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
}
}
- pr_debug("couldn't find an available UDC\n");
+ list_add_tail(&driver->pending, &gadget_driver_pending_list);
+ pr_info("udc-core: couldn't find an available UDC - added [%s] to list of pending drivers\n",
+ driver->function);
mutex_unlock(&udc_lock);
- return ret;
+ return 0;
found:
ret = udc_bind_to_driver(udc, driver);
mutex_unlock(&udc_lock);
@@ -577,6 +596,10 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
break;
}
+ if (ret) {
+ list_del(&driver->pending);
+ ret = 0;
+ }
mutex_unlock(&udc_lock);
return ret;
}
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index ce2188d..92467ee 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -1014,6 +1014,7 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget)
* @driver: Driver model state for this driver.
* @udc_name: A name of UDC this driver should be bound to. If udc_name is NULL,
* this driver will be bound to any available UDC.
+ * @pending: UDC core private data used for deferred probe of this driver.
*
* Devices are disabled till a gadget driver successfully bind()s, which
* means the driver will handle setup() requests needed to enumerate (and
@@ -1076,6 +1077,7 @@ struct usb_gadget_driver {
struct device_driver driver;
char *udc_name;
+ struct list_head pending;
};
--
1.9.2
On Mon, 23 Nov 2015, Marek Szyprowski wrote:
> From: Ruslan Bilovol <[email protected]>
>
> 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.
>
> Tested-by: Maxime Ripard <[email protected]>
> Signed-off-by: Ruslan Bilovol <[email protected]>
> [simplified code as requested by Alan Stern and Felipe Balbi,
> fixed checkpatch issues]
> Signed-off-by: Marek Szyprowski <[email protected]>
> Tested-by: Peter Chen <[email protected]>
I missed this the last time through...
> @@ -403,6 +408,18 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
> usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
> udc->vbus = true;
>
> + /* pick up one of pending gadget drivers */
> + list_for_each_entry(driver, &gadget_driver_pending_list, pending) {
> + if (!driver->udc_name || strcmp(driver->udc_name,
> + dev_name(&udc->dev)) == 0) {
> + ret = udc_bind_to_driver(udc, driver);
> + if (ret)
> + goto err4;
> + list_del(&driver->pending);
This has to be list_del_init(). And somewhere in
usb_gadget_probe_driver() you need to do
INIT_LIST_HEAD(&driver->pending).
> @@ -577,6 +596,10 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
> break;
> }
>
> + if (ret) {
> + list_del(&driver->pending);
Otherwise this will cause a crash or corrupt some random area of
memory.
Alan Stern
On Mon, Nov 23, 2015 at 10:32:53AM -0500, Alan Stern wrote:
> On Mon, 23 Nov 2015, Marek Szyprowski wrote:
>
> > From: Ruslan Bilovol <[email protected]>
> >
> > 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.
> >
> > Tested-by: Maxime Ripard <[email protected]>
> > Signed-off-by: Ruslan Bilovol <[email protected]>
> > [simplified code as requested by Alan Stern and Felipe Balbi,
> > fixed checkpatch issues]
> > Signed-off-by: Marek Szyprowski <[email protected]>
> > Tested-by: Peter Chen <[email protected]>
>
> I missed this the last time through...
>
> > @@ -403,6 +408,18 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
> > usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
> > udc->vbus = true;
> >
> > + /* pick up one of pending gadget drivers */
> > + list_for_each_entry(driver, &gadget_driver_pending_list, pending) {
> > + if (!driver->udc_name || strcmp(driver->udc_name,
> > + dev_name(&udc->dev)) == 0) {
> > + ret = udc_bind_to_driver(udc, driver);
> > + if (ret)
> > + goto err4;
> > + list_del(&driver->pending);
>
> This has to be list_del_init(). And somewhere in
> usb_gadget_probe_driver() you need to do
> INIT_LIST_HEAD(&driver->pending).
>
> > @@ -577,6 +596,10 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
> > break;
> > }
> >
> > + if (ret) {
> > + list_del(&driver->pending);
>
> Otherwise this will cause a crash or corrupt some random area of
> memory.
>
Alan, would you please explain more what use case will cause memory
corruption for above code?
--
Best Regards,
Peter Chen
Hello,
On 2015-11-23 16:32, Alan Stern wrote:
> On Mon, 23 Nov 2015, Marek Szyprowski wrote:
>
>> From: Ruslan Bilovol <[email protected]>
>>
>> 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.
>>
>> Tested-by: Maxime Ripard <[email protected]>
>> Signed-off-by: Ruslan Bilovol <[email protected]>
>> [simplified code as requested by Alan Stern and Felipe Balbi,
>> fixed checkpatch issues]
>> Signed-off-by: Marek Szyprowski <[email protected]>
>> Tested-by: Peter Chen <[email protected]>
> I missed this the last time through...
>
>> @@ -403,6 +408,18 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
>> usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
>> udc->vbus = true;
>>
>> + /* pick up one of pending gadget drivers */
>> + list_for_each_entry(driver, &gadget_driver_pending_list, pending) {
>> + if (!driver->udc_name || strcmp(driver->udc_name,
>> + dev_name(&udc->dev)) == 0) {
>> + ret = udc_bind_to_driver(udc, driver);
>> + if (ret)
>> + goto err4;
>> + list_del(&driver->pending);
> This has to be list_del_init(). And somewhere in
> usb_gadget_probe_driver() you need to do
> INIT_LIST_HEAD(&driver->pending).
>
>> @@ -577,6 +596,10 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
>> break;
>> }
>>
>> + if (ret) {
>> + list_del(&driver->pending);
> Otherwise this will cause a crash or corrupt some random area of
> memory.
'pending' handle is added to gadget_driver_pending_list in
usb_gadget_probe_driver(),
then when udc is available it is removed from this list and bound to the
given driver.
In usb_gadget_unregister_driver() the gadget is either unbound from the
udc or if is
was not bound to any udc, removed from pending list. I see no need for
adding
INIT_LIST_HEAD.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Tue, 24 Nov 2015, Peter Chen wrote:
> > > @@ -403,6 +408,18 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
> > > usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED);
> > > udc->vbus = true;
> > >
> > > + /* pick up one of pending gadget drivers */
> > > + list_for_each_entry(driver, &gadget_driver_pending_list, pending) {
> > > + if (!driver->udc_name || strcmp(driver->udc_name,
> > > + dev_name(&udc->dev)) == 0) {
> > > + ret = udc_bind_to_driver(udc, driver);
> > > + if (ret)
> > > + goto err4;
> > > + list_del(&driver->pending);
> >
> > This has to be list_del_init(). And somewhere in
> > usb_gadget_probe_driver() you need to do
> > INIT_LIST_HEAD(&driver->pending).
> >
> > > @@ -577,6 +596,10 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
> > > break;
> > > }
> > >
> > > + if (ret) {
> > > + list_del(&driver->pending);
> >
> > Otherwise this will cause a crash or corrupt some random area of
> > memory.
> >
>
> Alan, would you please explain more what use case will cause memory
> corruption for above code?
On Tue, 24 Nov 2015, Marek Szyprowski wrote:
> 'pending' handle is added to gadget_driver_pending_list in
> usb_gadget_probe_driver(),
> then when udc is available it is removed from this list and bound to the
> given driver.
> In usb_gadget_unregister_driver() the gadget is either unbound from the
> udc or if is
> was not bound to any udc, removed from pending list. I see no need for
> adding
> INIT_LIST_HEAD.
I'm sorry, you guys are right. I didn't notice the "if (ret) {" line
added to usb_gadget_unregister_driver(). That line insures that we
don't try to remove the driver from the pending list unless it really
is on the list.
So never mind...
Alan Stern
Hi Marek,
On Mon, Nov 23, 2015 at 10:56 AM, Marek Szyprowski
<[email protected]> wrote:
> Hello,
>
> This is a resurrection of the patches initially submitted by Ruslan
> Bilovol in the following thread: https://lkml.org/lkml/2015/6/22/554
>
> The changes since the original submission (v5) includes rebase onto
> latest linux-next branch, simplification of the code requested by Alan
> Stern and Felipe Balbi, removal of a patch, which deleted __init/__exit
> attributes (this change has been already merged) and fixes of the
> checkpatch issues.
Thank you for handling it as I'm still to busy for continuing on this feature.
Best regards,
Ruslan
>
> This feature is urgently needed, because it is not longer possible to
> use workaround to avoid deferred probe in UDC drivers due to
> not-yet-probed i2c regulator drivers (for more information see
> https://lkml.org/lkml/2015/10/30/374 ).
>
> This patchset has been successfully tested on Odroid XU3 boards with
> DWC3 UDC driver being deferred by missing regulator drivers.
>
> Best regards
> Marek Szyprowski
> Samsung R&D Institute Poland
>
>
> Patch summary:
>
> Ruslan Bilovol (4):
> usb: gadget: bind UDC by name passed via usb_gadget_driver structure
> usb: gadget: configfs: pass UDC name via usb_gadget_driver struct
> usb: gadget: udc-core: remove unused usb_udc_attach_driver()
> usb: gadget: udc-core: independent registration of gadgets and gadget
> drivers
>
> drivers/usb/gadget/configfs.c | 29 +++++++-------
> drivers/usb/gadget/udc/udc-core.c | 79 ++++++++++++++++++++++-----------------
> include/linux/usb/gadget.h | 8 +++-
> 3 files changed, 67 insertions(+), 49 deletions(-)
>
> --
> 1.9.2
>
Hi,
Marek Szyprowski <[email protected]> writes:
> Hello,
>
> This is a resurrection of the patches initially submitted by Ruslan
> Bilovol in the following thread: https://lkml.org/lkml/2015/6/22/554
>
> The changes since the original submission (v5) includes rebase onto
> latest linux-next branch, simplification of the code requested by Alan
> Stern and Felipe Balbi, removal of a patch, which deleted __init/__exit
> attributes (this change has been already merged) and fixes of the
> checkpatch issues.
>
> This feature is urgently needed, because it is not longer possible to
> use workaround to avoid deferred probe in UDC drivers due to
> not-yet-probed i2c regulator drivers (for more information see
> https://lkml.org/lkml/2015/10/30/374 ).
>
> This patchset has been successfully tested on Odroid XU3 boards with
> DWC3 UDC driver being deferred by missing regulator drivers.
there is one problem with this patchset. If I try to statically link
gadget drivers, only one can be chosen, even though I can enable both
dwc3 and dummy_hcd just fine. And, actually, this brings another
problem. How do we handle systems which have 2 USB peripheral
controllers (say, 2 instances of dwc3) and choose which gadget driver
will bind to which controller ?
--
balbi
Hi,
Felipe Balbi <[email protected]> writes:
> Marek Szyprowski <[email protected]> writes:
>> Hello,
>>
>> This is a resurrection of the patches initially submitted by Ruslan
>> Bilovol in the following thread: https://lkml.org/lkml/2015/6/22/554
>>
>> The changes since the original submission (v5) includes rebase onto
>> latest linux-next branch, simplification of the code requested by Alan
>> Stern and Felipe Balbi, removal of a patch, which deleted __init/__exit
>> attributes (this change has been already merged) and fixes of the
>> checkpatch issues.
>>
>> This feature is urgently needed, because it is not longer possible to
>> use workaround to avoid deferred probe in UDC drivers due to
>> not-yet-probed i2c regulator drivers (for more information see
>> https://lkml.org/lkml/2015/10/30/374 ).
>>
>> This patchset has been successfully tested on Odroid XU3 boards with
>> DWC3 UDC driver being deferred by missing regulator drivers.
>
> there is one problem with this patchset. If I try to statically link
> gadget drivers, only one can be chosen, even though I can enable both
> dwc3 and dummy_hcd just fine. And, actually, this brings another
> problem. How do we handle systems which have 2 USB peripheral
> controllers (say, 2 instances of dwc3) and choose which gadget driver
> will bind to which controller ?
We also seem to have issues with Kconfig. If I try to make gadget driver
built-in, when compiling I'll get asked again if I want gadget drivers
built-in.
Another one: I just tried dummy_hcd built-in, g_zero built-in, dwc3 as a
module. I can never load anything to dwc3 ;-)
--
balbi
Hi,
Felipe Balbi <[email protected]> writes:
> Felipe Balbi <[email protected]> writes:
>> Marek Szyprowski <[email protected]> writes:
>>> Hello,
>>>
>>> This is a resurrection of the patches initially submitted by Ruslan
>>> Bilovol in the following thread: https://lkml.org/lkml/2015/6/22/554
>>>
>>> The changes since the original submission (v5) includes rebase onto
>>> latest linux-next branch, simplification of the code requested by Alan
>>> Stern and Felipe Balbi, removal of a patch, which deleted __init/__exit
>>> attributes (this change has been already merged) and fixes of the
>>> checkpatch issues.
>>>
>>> This feature is urgently needed, because it is not longer possible to
>>> use workaround to avoid deferred probe in UDC drivers due to
>>> not-yet-probed i2c regulator drivers (for more information see
>>> https://lkml.org/lkml/2015/10/30/374 ).
>>>
>>> This patchset has been successfully tested on Odroid XU3 boards with
>>> DWC3 UDC driver being deferred by missing regulator drivers.
>>
>> there is one problem with this patchset. If I try to statically link
>> gadget drivers, only one can be chosen, even though I can enable both
>> dwc3 and dummy_hcd just fine. And, actually, this brings another
>> problem. How do we handle systems which have 2 USB peripheral
>> controllers (say, 2 instances of dwc3) and choose which gadget driver
>> will bind to which controller ?
>
> We also seem to have issues with Kconfig. If I try to make gadget driver
> built-in, when compiling I'll get asked again if I want gadget drivers
> built-in.
>
> Another one: I just tried dummy_hcd built-in, g_zero built-in, dwc3 as a
> module. I can never load anything to dwc3 ;-)
In all fairness, none of these are regressions. Can we agree to look at
these during v4.5-rc so maybe v4.6 has a final solution ?
cheers
--
balbi
Hello,
On 2015-12-10 18:13, Felipe Balbi wrote:
> Felipe Balbi <[email protected]> writes:
>> Felipe Balbi <[email protected]> writes:
>>> Marek Szyprowski <[email protected]> writes:
>>>> This is a resurrection of the patches initially submitted by Ruslan
>>>> Bilovol in the following thread: https://lkml.org/lkml/2015/6/22/554
>>>>
>>>> The changes since the original submission (v5) includes rebase onto
>>>> latest linux-next branch, simplification of the code requested by Alan
>>>> Stern and Felipe Balbi, removal of a patch, which deleted __init/__exit
>>>> attributes (this change has been already merged) and fixes of the
>>>> checkpatch issues.
>>>>
>>>> This feature is urgently needed, because it is not longer possible to
>>>> use workaround to avoid deferred probe in UDC drivers due to
>>>> not-yet-probed i2c regulator drivers (for more information see
>>>> https://lkml.org/lkml/2015/10/30/374 ).
>>>>
>>>> This patchset has been successfully tested on Odroid XU3 boards with
>>>> DWC3 UDC driver being deferred by missing regulator drivers.
>>> there is one problem with this patchset. If I try to statically link
>>> gadget drivers, only one can be chosen, even though I can enable both
>>> dwc3 and dummy_hcd just fine. And, actually, this brings another
>>> problem. How do we handle systems which have 2 USB peripheral
>>> controllers (say, 2 instances of dwc3) and choose which gadget driver
>>> will bind to which controller ?
>> We also seem to have issues with Kconfig. If I try to make gadget driver
>> built-in, when compiling I'll get asked again if I want gadget drivers
>> built-in.
>>
>> Another one: I just tried dummy_hcd built-in, g_zero built-in, dwc3 as a
>> module. I can never load anything to dwc3 ;-)
> In all fairness, none of these are regressions. Can we agree to look at
> these during v4.5-rc so maybe v4.6 has a final solution ?
IMHO solving all the above issues requires reviving one of the previously
abandoned gadget-bus patch series, i.e.
http://thread.gmane.org/gmane.linux.usb.general/109745
Is this an approach You are interested in?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Thu, Dec 10, 2015 at 11:13:24AM -0600, Felipe Balbi wrote:
>
> Hi,
>
> Felipe Balbi <[email protected]> writes:
> > Felipe Balbi <[email protected]> writes:
> >> Marek Szyprowski <[email protected]> writes:
> >>> Hello,
> >>>
> >>> This is a resurrection of the patches initially submitted by Ruslan
> >>> Bilovol in the following thread: https://lkml.org/lkml/2015/6/22/554
> >>>
> >>> The changes since the original submission (v5) includes rebase onto
> >>> latest linux-next branch, simplification of the code requested by Alan
> >>> Stern and Felipe Balbi, removal of a patch, which deleted __init/__exit
> >>> attributes (this change has been already merged) and fixes of the
> >>> checkpatch issues.
> >>>
> >>> This feature is urgently needed, because it is not longer possible to
> >>> use workaround to avoid deferred probe in UDC drivers due to
> >>> not-yet-probed i2c regulator drivers (for more information see
> >>> https://lkml.org/lkml/2015/10/30/374 ).
> >>>
> >>> This patchset has been successfully tested on Odroid XU3 boards with
> >>> DWC3 UDC driver being deferred by missing regulator drivers.
> >>
> >> there is one problem with this patchset. If I try to statically link
> >> gadget drivers, only one can be chosen, even though I can enable both
> >> dwc3 and dummy_hcd just fine. And, actually, this brings another
> >> problem. How do we handle systems which have 2 USB peripheral
> >> controllers (say, 2 instances of dwc3) and choose which gadget driver
> >> will bind to which controller ?
> >
> > We also seem to have issues with Kconfig. If I try to make gadget driver
> > built-in, when compiling I'll get asked again if I want gadget drivers
> > built-in.
> >
> > Another one: I just tried dummy_hcd built-in, g_zero built-in, dwc3 as a
> > module. I can never load anything to dwc3 ;-)
>
> In all fairness, none of these are regressions. Can we agree to look at
> these during v4.5-rc so maybe v4.6 has a final solution ?
>
> cheers
>
Hi Felipe,
To support all of these, it will let legacy gadget driver usage as same
as configfs. If the user wants these relative new features, why not let
they use configfs. For legacy gadget drivers, it is better to keep the
main structure unchanging, then the user will not be surprise.
Here, we just want a solution for known issue, but not new features.
--
Best Regards,
Peter Chen