Hi, On Mon, Nov 11, 2013 at 12:46:02PM +0100, Michael Grzeschik wrote: > We currently enable the udc on bind_to_driver. This breaks the > de/activate mechanism for drivers that depent on userspace components. > This patch fixes this by moving the pullup call to each bind. > > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx> Yeah, we _do_ want to do this, but not like this. We want to move the connect/disconnect capability to configfs too, eventually, so that a usbgadgetd can handle that for every gadget. I had sent a patch a while back which would add a flag and if that flag was set, we wouldn't call usb_gadget_connect from udc-core.c, patches attached but they probably need a good rebase. -- balbi
From a23800e2463ae1f4eafa7c0a15bb44afee75994f Mon Sep 17 00:00:00 2001
From: Felipe Balbi <balbi@xxxxxx>
Date: Thu, 26 Jul 2012 14:23:44 +0300
Subject: [PATCH 1/2] usb: gadget: let gadgets control pullup on their own
This is useful on gadgets that depend on userland
daemons to function properly. We can delay connection
to the host until userland is ready.
Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
drivers/usb/gadget/composite.c | 3 ++-
drivers/usb/gadget/udc-core.c | 26 ++++++++++++++++++++++++--
include/linux/usb/composite.h | 4 ++++
include/linux/usb/gadget.h | 3 +++
4 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 7c821de..790ccf2 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1784,8 +1784,9 @@ int usb_composite_probe(struct usb_composite_driver *driver)
driver->name = "composite";
driver->gadget_driver = composite_driver_template;
- gadget_driver = &driver->gadget_driver;
+ gadget_driver = &driver->gadget_driver;
+ gadget_driver->controls_pullups = driver->controls_pullups;
gadget_driver->function = (char *) driver->name;
gadget_driver->driver.name = driver->name;
gadget_driver->max_speed = driver->max_speed;
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index 8a1eeb2..c0f4fca 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -235,7 +235,18 @@ static void usb_gadget_remove_driver(struct usb_udc *udc)
kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
- usb_gadget_disconnect(udc->gadget);
+ /*
+ * NOTICE: if gadget driver wants to control
+ * pullup, it needs to make sure that when
+ * user tries to rmmod the gadget driver, it
+ * will disconnect the pullups before returning
+ * from its ->unbind() method.
+ *
+ * We are truly trusting the gadget driver here.
+ */
+ if (!udc->driver->controls_pullups)
+ usb_gadget_disconnect(udc->gadget);
+
udc->driver->disconnect(udc->gadget);
udc->driver->unbind(udc->gadget);
usb_gadget_udc_stop(udc->gadget, udc->driver);
@@ -300,7 +311,18 @@ static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *dri
driver->unbind(udc->gadget);
goto err1;
}
- usb_gadget_connect(udc->gadget);
+
+ /*
+ * NOTICE: if gadget driver wants to control
+ * pullups, it needs to make sure its calls
+ * to usb_function_activate() and
+ * usb_function_deactivate() are balanced,
+ * otherwise gadget_driver will never enumerate.
+ *
+ * We are truly trusting the gadget driver here.
+ */
+ if (!driver->controls_pullups)
+ usb_gadget_connect(udc->gadget);
kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
return 0;
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 3c671c1..7ae797c 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -157,6 +157,7 @@ struct usb_function {
int (*get_status)(struct usb_function *);
int (*func_suspend)(struct usb_function *,
u8 suspend_opt);
+
/* private: */
/* internals */
struct list_head list;
@@ -279,6 +280,8 @@ enum {
* @max_speed: Highest speed the driver supports.
* @needs_serial: set to 1 if the gadget needs userspace to provide
* a serial number. If one is not provided, warning will be printed.
+ * @controls_pullups: this driver will control pullup and udc-core shouldn't
+ * enable it by default
* @bind: (REQUIRED) Used to allocate resources that are shared across the
* whole device, such as string IDs, and add its configurations using
* @usb_add_config(). This may fail by returning a negative errno
@@ -308,6 +311,7 @@ struct usb_composite_driver {
struct usb_gadget_strings **strings;
enum usb_device_speed max_speed;
unsigned needs_serial:1;
+ unsigned controls_pullups:1;
int (*bind)(struct usb_composite_dev *cdev);
int (*unbind)(struct usb_composite_dev *);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 32b734d..87971fa 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -774,6 +774,7 @@ static inline int usb_gadget_disconnect(struct usb_gadget *gadget)
* @suspend: Invoked on USB suspend. May be called in_interrupt.
* @resume: Invoked on USB resume. May be called in_interrupt.
* @driver: Driver model state for this driver.
+ * @controls_pullups: tells udc-core to not enable pullups by default
*
* Devices are disabled till a gadget driver successfully bind()s, which
* means the driver will handle setup() requests needed to enumerate (and
@@ -833,6 +834,8 @@ struct usb_gadget_driver {
/* FIXME support safe rmmod */
struct device_driver driver;
+
+ unsigned controls_pullups:1;
};
--
1.8.4.1.559.gdb9bdfb
From 0b733885e276a38cc9fa415c0977f063f9ce4d9d Mon Sep 17 00:00:00 2001
From: Felipe Balbi <balbi@xxxxxx>
Date: Wed, 6 Feb 2013 12:34:33 +0200
Subject: [PATCH 2/2] usb: gadget: webcam: let it control pullups
this gadget driver needs to make sure that
we will only enumerate after its userland
counterpart is up and running.
Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
drivers/usb/gadget/webcam.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/webcam.c b/drivers/usb/gadget/webcam.c
index 8cef1e6..41a4d03 100644
--- a/drivers/usb/gadget/webcam.c
+++ b/drivers/usb/gadget/webcam.c
@@ -388,6 +388,7 @@ static __refdata struct usb_composite_driver webcam_driver = {
.max_speed = USB_SPEED_SUPER,
.bind = webcam_bind,
.unbind = webcam_unbind,
+ .controls_pullups = true,
};
static int __init
--
1.8.4.1.559.gdb9bdfb
Attachment:
signature.asc
Description: Digital signature
