usb: gadget breakage on N900: bind UDC by name passed via usb_gadget_driver structure

Hi!

USB gadget stops working for me on n900, if I merge

usb: gadget: bind UDC by name passed via usb_gadget_driver structure

patch.

commit 2284b29d3d9dd16490909962574d7f3fef83fd97

Author: Ruslan Bilovol <[email protected]>
Date:   Mon Nov 23 09:56:35 2015 +0100

usb: gadget: bind UDC by name passed via usb_gadget_driver structure

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

Signed-off-by Felipe Balbi <[email protected]>

Patch is obvious enough... if there's someone who sets udc_name to NULL. Who is supposed to do that?

(I have patch in my tree to make USB gadget work when it is not modular).

Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Quoted 3 lines

Hi!

USB gadget stops working for me on n900, if I merge

Could you please give us more details?
Which gadget driver do you use (g_nokia?)

Quoted 2 lines

(I have patch in my tree to make USB gadget work when it is not modular).

So this can be reproduced only on builtin gadget, right?

BTW, I have N900 and can try to play with it this weekend if you'll give us more details.

Quoted 1 line

USB gadget stops working for me on n900, if I merge

Could you please give us more details?
Which gadget driver do you use (g_nokia?)

Hi!

Quoted 3 lines

Quoted 1 line

USB gadget stops working for me on n900, if I merge

Could you please give us more details?
Which gadget driver do you use (g_nokia?)

Ok, so I could get it to work with v4.5, and this patch. I'm including my config, too. No, I don't think I'm using g_nokia.

Best regards,
Pavel

.config new file +127 -0

new file mode 100644
index 0000000..a504a94
...
+# CONFIG_USB_SWITCH_FSA9480 is not set
+# CONFIG_LATTICE_ECP3_CONFIG is not set
+# CONFIG_SRAM is not set
+# CONFIG_C2PORT is not set
+
+CONFIG_NET_VENDOR_WIZNET=y
+# CONFIG_WIZNET_W5100 is not set
+# CONFIG_WIZNET_W5300 is not set
+# CONFIG_PHYLIB is not set
+# CONFIG_MICREL_KS8995MA is not set
+# CONFIG_PPP is not set
+# CONFIG_SLIP is not set
+CONFIG_USB_NET_DRIVERS=y
+# CONFIG_USB_CATC is not set
+# CONFIG_USB_KAWETH is not set
+# CONFIG_USB_PEGASUS is not set
+# CONFIG_USB_RTL8150 is not set
+# CONFIG_USB_RTL8152 is not set
+# CONFIG_USB_LAN78XX is not set
+# CONFIG_USB_USBNET is not set
+# CONFIG_USB_IPHETH is not set
+CONFIG_WLAN=y
+# CONFIG_WLAN_VENDOR_ADMTEK is not set
+# CONFIG_WLAN_VENDOR_ATH is not set
+# CONFIG_WLAN_VENDOR_ATMEL is not set
+# CONFIG_WLAN_VENDOR_BROADCOM is not set
+# CONFIG_WLAN_VENDOR_CISCO is not set
+# CONFIG_WLAN_VENDOR_INTEL is not set
+# CONFIG_WLAN_VENDOR_INTERSIL is not set
+# CONFIG_WLAN_VENDOR_MARVELL is not set
+
+#
+# USB HID support
+#
+CONFIG_USB_HID=y
+# CONFIG_HID_PID is not set
+# CONFIG_USB_HIDDEV is not set
+
+#
+# I2C HID support
+#
+# CONFIG_I2C_HID is not set
+CONFIG_USB_OHCI_LITTLE_ENDIAN=y
+CONFIG_USB_SUPPORT=y
+CONFIG_USB_COMMON=y
+CONFIG_USB_ARCH_HAS_HCD=y
+CONFIG_USB=y
+CONFIG_USB_ANNOUNCE_NEW_DEVICES=y
+
+#
+# Miscellaneous USB options
+#
+CONFIG_USB_DEFAULT_PERSIST=y
+# CONFIG_USB_DYNAMIC_MINORS is not set
+# CONFIG_USB_OTG is not set
+# CONFIG_USB_OTG_WHITELIST is not set
+# CONFIG_USB_OTG_BLACKLIST_HUB is not set
+# CONFIG_USB_ULPI_BUS is not set
+# CONFIG_USB_MON is not set
+# CONFIG_USB_WUSB_CBAF is not set
+
+#
+# USB Host Controller Drivers
+#
+# CONFIG_USB_C67X00_HCD is not set
+# CONFIG_USB_XHCI_HCD is not set
+# CONFIG_USB_EHCI_HCD is not set
+# CONFIG_USB_OXU210HP_HCD is not set
+# CONFIG_USB_ISP116X_HCD is not set
+# CONFIG_USB_ISP1362_HCD is not set
+# CONFIG_USB_FOTG210_HCD is not set
+# CONFIG_USB_MAX3421_HCD is not set
+# CONFIG_USB_OHCI_HCD is not set
+# CONFIG_USB_SL811_HCD is not set
+# CONFIG_USB_R8A66597_HCD is not set
+# CONFIG_USB_HCD_TEST_MODE is not set
+
+#
+# USB Device Class drivers
+#
+# CONFIG_USB_ACM is not set
+# CONFIG_USB_PRINTER is not set
+# CONFIG_USB_WDM is not set
+# CONFIG_USB_TMC is not set
+
+#
+# NOTE: USB_STORAGE depends on SCSI but BLK_DEV_SD may
+#
+
+#
+# also be needed; see USB_STORAGE Help for more info
+#
+# CONFIG_USB_STORAGE is not set
+
+#
+# USB Imaging devices
+#
+# CONFIG_USB_MDC800 is not set
+# CONFIG_USB_MICROTEK is not set
+# CONFIG_USBIP_CORE is not set
+CONFIG_USB_MUSB_HDRC=y
+# CONFIG_USB_MUSB_HOST is not set
+CONFIG_USB_MUSB_GADGET=y
+# CONFIG_USB_MUSB_DUAL_ROLE is not set
+
+#
+# Platform Glue Layer
+#
+# CONFIG_USB_MUSB_TUSB6010 is not set
+CONFIG_USB_MUSB_OMAP2PLUS=y
+# CONFIG_USB_MUSB_AM35X is not set
+# CONFIG_USB_MUSB_DSPS is not set
+
+#
+# MUSB DMA mode
+#
+CONFIG_MUSB_PIO_ONLY=y
+# CONFIG_USB_DWC3 is not set
+# CONFIG_USB_DWC2 is not set
+# CONFIG_USB_CHIPIDEA is not set
+# CONFIG_USB_ISP1760 is not set
+
+#
+# USB port drivers
+#
+# CONFIG_USB_SERIAL is not set
+

drivers/usb/gadget/udc/udc-core.c +63 -7

index b86a6f0..bee109f 100644
@@ -542,14 +542,37 @@ err1:
  	return ret;
  }
 
-int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
+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;
  	int			ret = -ENODEV;
 
-	if (!driver || !driver->bind || !driver->setup)
-		return -EINVAL;
-
  	mutex_lock(&udc_lock);
  	if (driver->udc_name) {
  		list_for_each_entry(udc, &udc_list, list) {

@@ -567,16 +590,47 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
  		}
  	}
 
-	list_add_tail(&driver->pending, &gadget_driver_pending_list);
+//	list_add_tail(&driver->pending, &gadget_driver_pending_list); FIXME
  	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 0;
+	return ret;
  found:
  	ret = udc_bind_to_driver(udc, driver);
  	mutex_unlock(&udc_lock);
  	return ret;
  }
+
+#define USB_GADGET_BIND_RETRIES		5
+#define USB_GADGET_BIND_TIMEOUT		(3 * HZ)
+static void usb_gadget_work(struct work_struct *work)
+{
+	struct usb_gadget_driver *driver = container_of(work,
+						struct usb_gadget_driver,
+						work.work);
+	int res;
+	
+	if (driver->retries++ > USB_GADGET_BIND_RETRIES) {
+		pr_err("couldn't find an available UDC\n");
+		return;
+	}
+
+	res = __usb_gadget_probe_driver(driver);
+	if (res == -ENODEV)
+		schedule_delayed_work(&driver->work, USB_GADGET_BIND_TIMEOUT);
+}
+
+int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
+{
+	if (!driver || !driver->bind || !driver->setup)
+		return -EINVAL;
+
+	INIT_DELAYED_WORK(&driver->work, usb_gadget_work);
+	schedule_delayed_work(&driver->work, 0);
+
+	return 0;
+}
  EXPORT_SYMBOL_GPL(usb_gadget_probe_driver);
 
  int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)

@@ -587,6 +641,8 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
  	if (!driver || !driver->unbind)
  		return -EINVAL;
 
+	cancel_delayed_work(&driver->work);
+
  	mutex_lock(&udc_lock);
  	list_for_each_entry(udc, &udc_list, list)
  		if (udc->driver == driver) {

@@ -747,7 +803,7 @@ static int __init usb_udc_init(void)
  	udc_class->dev_uevent = usb_udc_uevent;
  	return 0;
  }
-subsys_initcall(usb_udc_init);
+late_initcall_sync(usb_udc_init);
 
  static void __exit usb_udc_exit(void)
  {

include/linux/usb/gadget.h +2 -0

index d82d006..adb1e68 100644
@@ -1014,6 +1014,8 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget)
   * @resume: Invoked on USB resume.  May be called in_interrupt.
   * @reset: Invoked on USB bus reset. It is mandatory for all gadget drivers
   *	and should be called in_interrupt.
+ * @work: Gadget work used to bind to the USB controller.
+ * @retries: Gadget retries to bind to the USB controller.
   * @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.

Hi

Quoted 7 lines · Pavel Machek

Hi!

Quoted 3 lines

Quoted 1 line

USB gadget stops working for me on n900, if I merge

Could you please give us more details?
Which gadget driver do you use (g_nokia?)

Ok, so I could get it to work with v4.5, and this patch. I'm including my config, too. No, I don't think I'm using g_nokia.

This shows that there are some serious problems, probably with your udc driver, which doesn't handle deferred probe properly. Your patch workaround it by waiting some predefined time before registering gadget driver, however this is not the proper way. Please try to isolate what exactly is needed to get the gadget driver registered properly in your system or at least please provide some logs from the failed case.

Quoted 122 lines

drivers/usb/gadget/udc/udc-core.c +63 -7

index b86a6f0..bee109f 100644
@@ -542,14 +542,37 @@ err1:
 	return ret;
 }
-int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
+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;
 	int			ret = -ENODEV;
-	if (!driver || !driver->bind || !driver->setup)
-		return -EINVAL;
-
 	mutex_lock(&udc_lock);
 	if (driver->udc_name) {
 		list_for_each_entry(udc, &udc_list, list) {

@@ -567,16 +590,47 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
 		}
 	}
-	list_add_tail(&driver->pending, &gadget_driver_pending_list);
+//	list_add_tail(&driver->pending, &gadget_driver_pending_list); FIXME
 	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 0;
+	return ret;
 found:
 	ret = udc_bind_to_driver(udc, driver);
 	mutex_unlock(&udc_lock);
 	return ret;
 }
+
+#define USB_GADGET_BIND_RETRIES		5
+#define USB_GADGET_BIND_TIMEOUT		(3 * HZ)
+static void usb_gadget_work(struct work_struct *work)
+{
+	struct usb_gadget_driver *driver = container_of(work,
+						struct usb_gadget_driver,
+						work.work);
+	int res;
+	
+	if (driver->retries++ > USB_GADGET_BIND_RETRIES) {
+		pr_err("couldn't find an available UDC\n");
+		return;
+	}
+
+	res = __usb_gadget_probe_driver(driver);
+	if (res == -ENODEV)
+		schedule_delayed_work(&driver->work, USB_GADGET_BIND_TIMEOUT);
+}
+
+int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
+{
+	if (!driver || !driver->bind || !driver->setup)
+		return -EINVAL;
+
+	INIT_DELAYED_WORK(&driver->work, usb_gadget_work);
+	schedule_delayed_work(&driver->work, 0);
+
+	return 0;
+}
 EXPORT_SYMBOL_GPL(usb_gadget_probe_driver);
 int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)

@@ -587,6 +641,8 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
 	if (!driver || !driver->unbind)
 		return -EINVAL;
+	cancel_delayed_work(&driver->work);
+
 	mutex_lock(&udc_lock);
 	list_for_each_entry(udc, &udc_list, list)
 		if (udc->driver == driver) {

@@ -747,7 +803,7 @@ static int __init usb_udc_init(void)
 	udc_class->dev_uevent = usb_udc_uevent;
 	return 0;
 }
-subsys_initcall(usb_udc_init);
+late_initcall_sync(usb_udc_init);
 static void __exit usb_udc_exit(void)
 {

include/linux/usb/gadget.h +2 -0

index d82d006..adb1e68 100644
@@ -1014,6 +1014,8 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget)
  * @resume: Invoked on USB resume.  May be called in_interrupt.
  * @reset: Invoked on USB bus reset. It is mandatory for all gadget drivers
  *	and should be called in_interrupt.
+ * @work: Gadget work used to bind to the USB controller.
+ * @retries: Gadget retries to bind to the USB controller.
  * @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.

include/linux/usb/gadget.h struct usb_gadget_driver { · line 1084

Quoted 8 lines

include/linux/usb/gadget.h +2 -0

@@ -1075,6 +1077,8 @@ struct usb_gadget_driver {
 	void			(*suspend)(struct usb_gadget *);
 	void			(*resume)(struct usb_gadget *);
 	void			(*reset)(struct usb_gadget *);
+	struct delayed_work	work;
+	int			retries;
 	/* FIXME support safe rmmod */
 	struct device_driver	driver;
 

Best regards

Quoted 11 lines · Marek Szyprowski

Hi

Quoted 7 lines · Pavel Machek

Hi!

Quoted 3 lines

Quoted 1 line

USB gadget stops working for me on n900, if I merge

Could you please give us more details?
Which gadget driver do you use (g_nokia?)

Ok, so I could get it to work with v4.5, and this patch. I'm including my config, too. No, I don't think I'm using g_nokia.

Quoted 6 lines

This shows that there are some serious problems, probably with your udc driver, which doesn't handle deferred probe properly. Your patch workaround it by waiting some predefined time before registering gadget driver, however this is not the proper way. Please try to isolate what exactly is needed to get the gadget driver registered properly in your system or at least please provide some logs from the failed case.

Well, Ruslan said he has n900 available, so I provided requested information.

This is the dmesg with the hack:

[    0.623138] of_get_named_gpiod_flags: parsed 'ti,power-gpio'
property of node '/ocp/spi at 480ba00
0/wl1251 at 0[0]' - status (0)
[    0.623565] wl1251: ERROR vio regulator missing: -517
[    0.624359] musb probe HACK cf9a9e00
[    0.624420] musb probe HACK/2 cf9a9e00
[    0.624511] musb probe HACK/3 cf9a9e00
[    0.624755] HS USB OTG: no transceiver configured
[    0.624847] musb-hdrc musb-hdrc.0.auto: musb_init_controller failed
with status -517
[    0.626525] mousedev: PS/2 mouse device common for all mice
...
[    2.877441] ieee80211 phy1: Selected rate control algorithm
'minstrel'
[    2.879882] wl1251: loaded
[    2.889617] wl1251: initialized
[    2.897521] Two musb controllers?  HACK
[    2.904968] musb probe HACK cf9a9e00
[    2.912048] musb probe HACK/2 cf9a9e00
[    2.918823] musb probe HACK/3 cf9a9e00
[    2.928985] musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk
combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn)
[    2.929016] musb-hdrc: MHDRC RTL version 1.400
[    2.929046] musb-hdrc: setup fifo_mode 4
[    2.929077] musb-hdrc: 28/31 max ep, 16384/16384 memory
[    2.930511] tsc2005 spi1.0: GPIO lookup for consumer reset
[    2.930541] tsc2005 spi1.0: using device tree for GPIO lookup

In the kernel without the hack, there'll be only the first part of the dmesg, AFAICT.

If there's an interest, I can repeat the test without the hack.

Thanks,
Pavel

Quoted 124 lines

Quoted 124 lines

drivers/usb/gadget/udc/udc-core.c +63 -7

index b86a6f0..bee109f 100644
@@ -542,14 +542,37 @@ err1:
    return ret;
 }
-int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
+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;
    int                     ret = -ENODEV;
-    if (!driver || !driver->bind || !driver->setup)
-            return -EINVAL;
-
    mutex_lock(&udc_lock);
    if (driver->udc_name) {
            list_for_each_entry(udc, &udc_list, list) {

@@ -567,16 +590,47 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
            }
    }
-    list_add_tail(&driver->pending, &gadget_driver_pending_list);
+//  list_add_tail(&driver->pending, &gadget_driver_pending_list); FIXME
    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 0;
+    return ret;
 found:
    ret = udc_bind_to_driver(udc, driver);
    mutex_unlock(&udc_lock);
    return ret;
 }
+
+#define USB_GADGET_BIND_RETRIES             5
+#define USB_GADGET_BIND_TIMEOUT             (3 * HZ)
+static void usb_gadget_work(struct work_struct *work)
+{
+    struct usb_gadget_driver *driver = container_of(work,
+                                            struct usb_gadget_driver,
+                                            work.work);
+    int res;
+
+    if (driver->retries++ > USB_GADGET_BIND_RETRIES) {
+            pr_err("couldn't find an available UDC\n");
+            return;
+    }
+
+    res = __usb_gadget_probe_driver(driver);
+    if (res == -ENODEV)
+            schedule_delayed_work(&driver->work, USB_GADGET_BIND_TIMEOUT);
+}
+
+int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
+{
+    if (!driver || !driver->bind || !driver->setup)
+            return -EINVAL;
+
+    INIT_DELAYED_WORK(&driver->work, usb_gadget_work);
+    schedule_delayed_work(&driver->work, 0);
+
+    return 0;
+}
 EXPORT_SYMBOL_GPL(usb_gadget_probe_driver);
 int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)

@@ -587,6 +641,8 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
    if (!driver || !driver->unbind)
            return -EINVAL;
+    cancel_delayed_work(&driver->work);
+
    mutex_lock(&udc_lock);
    list_for_each_entry(udc, &udc_list, list)
            if (udc->driver == driver) {

@@ -747,7 +803,7 @@ static int __init usb_udc_init(void)
    udc_class->dev_uevent = usb_udc_uevent;
    return 0;
 }
-subsys_initcall(usb_udc_init);
+late_initcall_sync(usb_udc_init);
 static void __exit usb_udc_exit(void)
 {

include/linux/usb/gadget.h +2 -0

index d82d006..adb1e68 100644
@@ -1014,6 +1014,8 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget)
  * @resume: Invoked on USB resume.  May be called in_interrupt.
  * @reset: Invoked on USB bus reset. It is mandatory for all gadget drivers
  * and should be called in_interrupt.
+ * @work: Gadget work used to bind to the USB controller.
+ * @retries: Gadget retries to bind to the USB controller.
  * @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.

@@ -1075,6 +1077,8 @@ struct usb_gadget_driver {
    void                    (*suspend)(struct usb_gadget *);
    void                    (*resume)(struct usb_gadget *);

Quoted 8 lines

include/linux/usb/gadget.h struct usb_gadget_driver {

Quoted 6 lines

include/linux/usb/gadget.h +2 -0

    void                    (*reset)(struct usb_gadget *);
+    struct delayed_work     work;
+    int                     retries;
    /* FIXME support safe rmmod */
    struct device_driver    driver;
 

Best regards

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Hi Pavel,

I didn't use my N900 for years, it is deeply discharged and can't boot, so I left it connected overnight to the wall adapter to let it get some juice.

Meanwhile I looked into sources and still can't understand how it happens.
Could you please attach your full .config and also dmesg without hacks?

Regards,
Ruslan

Quoted 197 lines · Pavel Machek

Quoted 11 lines · Marek Szyprowski

Hi

Quoted 7 lines · Pavel Machek

Hi!

Quoted 3 lines

Quoted 1 line

USB gadget stops working for me on n900, if I merge

Could you please give us more details?
Which gadget driver do you use (g_nokia?)

Ok, so I could get it to work with v4.5, and this patch. I'm including my config, too. No, I don't think I'm using g_nokia.

Quoted 6 lines

This shows that there are some serious problems, probably with your udc driver, which doesn't handle deferred probe properly. Your patch workaround it by waiting some predefined time before registering gadget driver, however this is not the proper way. Please try to isolate what exactly is needed to get the gadget driver registered properly in your system or at least please provide some logs from the failed case.

Well, Ruslan said he has n900 available, so I provided requested information.

This is the dmesg with the hack:

[    0.623138] of_get_named_gpiod_flags: parsed 'ti,power-gpio'
property of node '/ocp/spi at 480ba00
0/wl1251 at 0[0]' - status (0)
[    0.623565] wl1251: ERROR vio regulator missing: -517
[    0.624359] musb probe HACK cf9a9e00
[    0.624420] musb probe HACK/2 cf9a9e00
[    0.624511] musb probe HACK/3 cf9a9e00
[    0.624755] HS USB OTG: no transceiver configured
[    0.624847] musb-hdrc musb-hdrc.0.auto: musb_init_controller failed
with status -517
[    0.626525] mousedev: PS/2 mouse device common for all mice
...
[    2.877441] ieee80211 phy1: Selected rate control algorithm
'minstrel'
[    2.879882] wl1251: loaded
[    2.889617] wl1251: initialized
[    2.897521] Two musb controllers?  HACK
[    2.904968] musb probe HACK cf9a9e00
[    2.912048] musb probe HACK/2 cf9a9e00
[    2.918823] musb probe HACK/3 cf9a9e00
[    2.928985] musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk
combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn)
[    2.929016] musb-hdrc: MHDRC RTL version 1.400
[    2.929046] musb-hdrc: setup fifo_mode 4
[    2.929077] musb-hdrc: 28/31 max ep, 16384/16384 memory
[    2.930511] tsc2005 spi1.0: GPIO lookup for consumer reset
[    2.930541] tsc2005 spi1.0: using device tree for GPIO lookup

In the kernel without the hack, there'll be only the first part of the dmesg, AFAICT.

If there's an interest, I can repeat the test without the hack.

Thanks,
Pavel

Quoted 124 lines

Quoted 124 lines

drivers/usb/gadget/udc/udc-core.c +63 -7

index b86a6f0..bee109f 100644
@@ -542,14 +542,37 @@ err1:
    return ret;
 }
-int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
+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;
    int                     ret = -ENODEV;
-    if (!driver || !driver->bind || !driver->setup)
-            return -EINVAL;
-
    mutex_lock(&udc_lock);
    if (driver->udc_name) {
            list_for_each_entry(udc, &udc_list, list) {

@@ -567,16 +590,47 @@ int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
            }
    }
-    list_add_tail(&driver->pending, &gadget_driver_pending_list);
+//  list_add_tail(&driver->pending, &gadget_driver_pending_list); FIXME
    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 0;
+    return ret;
 found:
    ret = udc_bind_to_driver(udc, driver);
    mutex_unlock(&udc_lock);
    return ret;
 }
+
+#define USB_GADGET_BIND_RETRIES             5
+#define USB_GADGET_BIND_TIMEOUT             (3 * HZ)
+static void usb_gadget_work(struct work_struct *work)
+{
+    struct usb_gadget_driver *driver = container_of(work,
+                                            struct usb_gadget_driver,
+                                            work.work);
+    int res;
+
+    if (driver->retries++ > USB_GADGET_BIND_RETRIES) {
+            pr_err("couldn't find an available UDC\n");
+            return;
+    }
+
+    res = __usb_gadget_probe_driver(driver);
+    if (res == -ENODEV)
+            schedule_delayed_work(&driver->work, USB_GADGET_BIND_TIMEOUT);
+}
+
+int usb_gadget_probe_driver(struct usb_gadget_driver *driver)
+{
+    if (!driver || !driver->bind || !driver->setup)
+            return -EINVAL;
+
+    INIT_DELAYED_WORK(&driver->work, usb_gadget_work);
+    schedule_delayed_work(&driver->work, 0);
+
+    return 0;
+}
 EXPORT_SYMBOL_GPL(usb_gadget_probe_driver);
 int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)

@@ -587,6 +641,8 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver)
    if (!driver || !driver->unbind)
            return -EINVAL;
+    cancel_delayed_work(&driver->work);
+
    mutex_lock(&udc_lock);
    list_for_each_entry(udc, &udc_list, list)
            if (udc->driver == driver) {

@@ -747,7 +803,7 @@ static int __init usb_udc_init(void)
    udc_class->dev_uevent = usb_udc_uevent;
    return 0;
 }
-subsys_initcall(usb_udc_init);
+late_initcall_sync(usb_udc_init);
 static void __exit usb_udc_exit(void)
 {

include/linux/usb/gadget.h +2 -0

index d82d006..adb1e68 100644
@@ -1014,6 +1014,8 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget)
  * @resume: Invoked on USB resume.  May be called in_interrupt.
  * @reset: Invoked on USB bus reset. It is mandatory for all gadget drivers
  * and should be called in_interrupt.
+ * @work: Gadget work used to bind to the USB controller.
+ * @retries: Gadget retries to bind to the USB controller.
  * @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.

@@ -1075,6 +1077,8 @@ struct usb_gadget_driver {
    void                    (*suspend)(struct usb_gadget *);
    void                    (*resume)(struct usb_gadget *);

Quoted 8 lines

include/linux/usb/gadget.h struct usb_gadget_driver {

Quoted 6 lines

include/linux/usb/gadget.h +2 -0

    void                    (*reset)(struct usb_gadget *);
+    struct delayed_work     work;
+    int                     retries;
    /* FIXME support safe rmmod */
    struct device_driver    driver;
 

Best regards

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

OK, so at last I finished charging of my N900; found 1.8V USB to UART adapter and soldered it to the phone.

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

Pavel, I still don't see how you've got your issue, please share more detials

Hi!

Quoted 8 lines

OK, so at last I finished charging of my N900; found 1.8V USB to UART adapter and soldered it to the phone.

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

Quoted 2 lines

Pavel, I still don't see how you've got your issue, please share more detials

Ok, let me try. I undid all the changes in drivers/phy drivers/usb and include/linux/usb . I have all the gadget stuff built-in, so that I could use nfsroot, but this is boot from mmcblk.

gzipped config is attached.

PC is unable to work with the gadget:

[256526.716099] usb 2-1: new full-speed USB device number 52 using
uhci_hcd
[256526.832091] usb 2-1: device descriptor read/64, error -32
[256527.052095] usb 2-1: device descriptor read/64, error -32
[256527.268160] usb 2-1: new full-speed USB device number 53 using
uhci_hcd
[256527.388121] usb 2-1: device descriptor read/64, error -32
[256527.608116] usb 2-1: device descriptor read/64, error -32
[256527.824170] usb 2-1: new full-speed USB device number 54 using
uhci_hcd
[256527.851175] usb 2-1: device descriptor read/8, error -32
[256527.975175] usb 2-1: device descriptor read/8, error -32
[256528.188133] usb 2-1: new full-speed USB device number 55 using
uhci_hcd
[256528.218184] usb 2-1: device descriptor read/8, error -32
[256528.343183] usb 2-1: device descriptor read/8, error -32
[256528.444314] usb usb2-port1: unable to enumerate USB device

Dmesg from the n900 is attached as /tmp/delme.gz. I did _not_ apply the patch from https://lkml.org/lkml/2016/3/24/670 , yet, as I'm using devicetree boot.

Best regards,

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Hi

Quoted 10 lines · Pavel Machek

Hi!

Quoted 8 lines

OK, so at last I finished charging of my N900; found 1.8V USB to UART adapter and soldered it to the phone.

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

Quoted 6 lines

Quoted 2 lines

Pavel, I still don't see how you've got your issue, please share more detials

Ok, let me try. I undid all the changes in drivers/phy drivers/usb and include/linux/usb . I have all the gadget stuff built-in, so that I could use nfsroot, but this is boot from mmcblk.

So do you mean you use original rootfs that was shipeed with N900?

Quoted 2 lines

gzipped config is attached.

Thanks, I'll try that

Quoted 21 lines

PC is unable to work with the gadget:

[256526.716099] usb 2-1: new full-speed USB device number 52 using
uhci_hcd
[256526.832091] usb 2-1: device descriptor read/64, error -32
[256527.052095] usb 2-1: device descriptor read/64, error -32
[256527.268160] usb 2-1: new full-speed USB device number 53 using
uhci_hcd
[256527.388121] usb 2-1: device descriptor read/64, error -32
[256527.608116] usb 2-1: device descriptor read/64, error -32
[256527.824170] usb 2-1: new full-speed USB device number 54 using
uhci_hcd
[256527.851175] usb 2-1: device descriptor read/8, error -32
[256527.975175] usb 2-1: device descriptor read/8, error -32
[256528.188133] usb 2-1: new full-speed USB device number 55 using
uhci_hcd
[256528.218184] usb 2-1: device descriptor read/8, error -32
[256528.343183] usb 2-1: device descriptor read/8, error -32
[256528.444314] usb usb2-port1: unable to enumerate USB device

Quoted 3 lines

Dmesg from the n900 is attached as /tmp/delme.gz. I did _not_ apply the patch from https://lkml.org/lkml/2016/3/24/670 , yet, as I'm using devicetree boot.

Hmm.. don't see anything strange in the boot log related to USB, I'll try your config

OK, so at last I finished charging of my N900; found 1.8V USB to UART adapter and soldered it to the phone.

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

Hi!

Quoted 7 lines

OK, so at last I finished charging of my N900; found 1.8V USB to UART adapter and soldered it to the phone.

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

I tried to add that one, too, and no change. Can I get your .config?

Thanks and best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Quoted 5 lines

Quoted 5 lines

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

Quoted 6 lines

Quoted 2 lines

Pavel, I still don't see how you've got your issue, please share more detials

Ok, let me try. I undid all the changes in drivers/phy drivers/usb and include/linux/usb . I have all the gadget stuff built-in, so that I could use nfsroot, but this is boot from mmcblk.

So do you mean you use original rootfs that was shipeed with N900?

Quoted 1 line

gzipped config is attached.

Thanks, I'll try that

Quoted 3 lines

Dmesg from the n900 is attached as /tmp/delme.gz. I did _not_ apply the patch from https://lkml.org/lkml/2016/3/24/670 , yet, as I'm using devicetree boot.

Hmm.. don't see anything strange in the boot log related to USB, I'll try your config

Quoted 4 lines

Quoted 3 lines

Dmesg from the n900 is attached as /tmp/delme.gz. I did _not_ apply the patch from https://lkml.org/lkml/2016/3/24/670 , yet, as I'm using devicetree boot.

Quoted 2 lines

Hmm.. don't see anything strange in the boot log related to USB, I'll try your config

After looking into your config and boot log again, I see you use kernel 4.4, whereas the patch you've told about ("usb: gadget: bind UDC by name passed via usb_gadget_driver structure") was merged only into 4.5 kernel. Could you please try vanilla v4.5 with your config and see if it helps?

Quoted 6 lines

Quoted 4 lines

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

I tried to add that one, too, and no change. Can I get your .config?

Since you use DT boot, this patch will not have any effect. See my config attached.

Hi!

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Pavel, I still don't see how you've got your issue, please share more detials

Ok, let me try. I undid all the changes in drivers/phy drivers/usb and include/linux/usb . I have all the gadget stuff built-in, so that I could use nfsroot, but this is boot from mmcblk.

So do you mean you use original rootfs that was shipeed with N900?

No. I'm using Debian ARM root. It includes useful X and even phone functions. https://gitlab.com/tui/tui .

Quoted 3 lines

Quoted 1 line

gzipped config is attached.

Thanks, I'll try that

Thanks.

Quoted 6 lines

Quoted 3 lines

Dmesg from the n900 is attached as /tmp/delme.gz. I did _not_ apply the patch from https://lkml.org/lkml/2016/3/24/670 , yet, as I'm using devicetree boot.

Hmm.. don't see anything strange in the boot log related to USB, I'll try your config

Let me know how it went.

Quoted 23 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Pavel, I still don't see how you've got your issue, please share more detials

Ok, let me try. I undid all the changes in drivers/phy drivers/usb and include/linux/usb . I have all the gadget stuff built-in, so that I could use nfsroot, but this is boot from mmcblk.

So do you mean you use original rootfs that was shipeed with N900?

No. I'm using Debian ARM root. It includes useful X and even phone functions. https://gitlab.com/tui/tui .

Quoted 3 lines

Quoted 1 line

gzipped config is attached.

Thanks, I'll try that

Thanks.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

Dmesg from the n900 is attached as /tmp/delme.gz. I did _not_ apply the patch from https://lkml.org/lkml/2016/3/24/670 , yet, as I'm using devicetree boot.

Hmm.. don't see anything strange in the boot log related to USB, I'll try your config

Let me know how it went.

I tried your config (with slight modifications) with vanilla kernel v4.5 (modifications: cmdline changed to default; used INITRAMFS_SOURCE for prebuilt cpio archive with rootfs) and Device Tree booting. Works fine for me except I need to unplug-plug usb cable or issue

# echo connect > /sys/devices/platform/68000000.ocp/480ab000
.usb_otg_hs/musb-hdrc.0.auto/udc/musb-hdrc.0.auto/soft_connect
to make connection.
The gadget driver successfully binds to the gadget, but I don't know
why the musb core doesn't issue pullup on USB lines during boot.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

Dmesg from the n900 is attached as /tmp/delme.gz. I did _not_ apply the patch from https://lkml.org/lkml/2016/3/24/670 , yet, as I'm using devicetree boot.

Hmm.. don't see anything strange in the boot log related to USB, I'll try your config

Let me know how it went.

I tried your config (with slight modifications) with vanilla kernel v4.5 (modifications: cmdline changed to default; used INITRAMFS_SOURCE for prebuilt cpio archive with rootfs) and Device Tree booting. Works fine for me except I need to unplug-plug usb cable or issue

# echo connect > /sys/devices/platform/68000000.ocp/480ab000
.usb_otg_hs/musb-hdrc.0.auto/udc/musb-hdrc.0.auto/soft_connect
to make connection.
The gadget driver successfully binds to the gadget, but I don't know
why the musb core doesn't issue pullup on USB lines during boot.

OK, so at last I finished charging of my N900; found 1.8V USB to UART adapter and soldered it to the phone.

I managed to boot N900 with working USB gadget (builtin g_ether) in boardfile mode, can ping it from PC and transfer data. I don't see any issue (except of musb name issue in twl phy driver, I've already sent a fix for that: https://lkml.org/lkml/2016/3/24/670 )

Pavel, I still don't see how you've got your issue, please share more detials