diff mbox

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

Message ID 20160318202041.GA20196@amd (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Machek March 18, 2016, 8:20 p.m. UTC
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?)

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

diff --git a/.config b/.config
new file mode 100644
index 0000000..a504a94
--- /dev/null
+++ b/.config
...
+# 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
+

Comments

Marek Szyprowski March 21, 2016, 11:51 a.m. UTC | #1
Hi

On 2016-03-18 21:20, Pavel Machek wrote:
> 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?)
> 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.


>
> Best regards,
> 								Pavel
>
> diff --git a/.config b/.config
> new file mode 100644
> index 0000000..a504a94
> --- /dev/null
> +++ b/.config
> ...
> +# 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
> +
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index b86a6f0..bee109f 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -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)
>   {
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index d82d006..adb1e68 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -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 *);
>   	void			(*reset)(struct usb_gadget *);
> +	struct delayed_work	work;
> +	int			retries;
>   
>   	/* FIXME support safe rmmod */
>   	struct device_driver	driver;
>

Best regards
Pavel Machek March 23, 2016, 12:21 p.m. UTC | #2
On Mon 2016-03-21 12:51:51, Marek Szyprowski wrote:
> Hi
> 
> On 2016-03-18 21:20, Pavel Machek wrote:
> >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?)
> >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.

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@480ba00
0/wl1251@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


> >diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> >index b86a6f0..bee109f 100644
> >--- a/drivers/usb/gadget/udc/udc-core.c
> >+++ b/drivers/usb/gadget/udc/udc-core.c
> >@@ -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)
> >  {
> >diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> >index d82d006..adb1e68 100644
> >--- a/include/linux/usb/gadget.h
> >+++ b/include/linux/usb/gadget.h
> >@@ -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 *);
> >  	void			(*reset)(struct usb_gadget *);
> >+	struct delayed_work	work;
> >+	int			retries;
> >  	/* FIXME support safe rmmod */
> >  	struct device_driver	driver;
> >
> 
> Best regards
Ruslan Bilovol March 24, 2016, 12:45 a.m. UTC | #3
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

On Wed, Mar 23, 2016 at 2:21 PM, Pavel Machek <pavel@ucw.cz> wrote:
> On Mon 2016-03-21 12:51:51, Marek Szyprowski wrote:
>> Hi
>>
>> On 2016-03-18 21:20, Pavel Machek wrote:
>> >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?)
>> >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.
>
> 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@480ba00
> 0/wl1251@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
>
>
>> >diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
>> >index b86a6f0..bee109f 100644
>> >--- a/drivers/usb/gadget/udc/udc-core.c
>> >+++ b/drivers/usb/gadget/udc/udc-core.c
>> >@@ -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)
>> >  {
>> >diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>> >index d82d006..adb1e68 100644
>> >--- a/include/linux/usb/gadget.h
>> >+++ b/include/linux/usb/gadget.h
>> >@@ -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 *);
>> >     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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ruslan Bilovol March 25, 2016, 3:21 a.m. UTC | #4
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

Regards,
Ruslan

On Thu, Mar 24, 2016 at 2:45 AM, Ruslan Bilovol
<ruslan.bilovol@gmail.com> wrote:
> 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
>
> On Wed, Mar 23, 2016 at 2:21 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> On Mon 2016-03-21 12:51:51, Marek Szyprowski wrote:
>>> Hi
>>>
>>> On 2016-03-18 21:20, Pavel Machek wrote:
>>> >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?)
>>> >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.
>>
>> 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@480ba00
>> 0/wl1251@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
>>
>>
>>> >diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
>>> >index b86a6f0..bee109f 100644
>>> >--- a/drivers/usb/gadget/udc/udc-core.c
>>> >+++ b/drivers/usb/gadget/udc/udc-core.c
>>> >@@ -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)
>>> >  {
>>> >diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>>> >index d82d006..adb1e68 100644
>>> >--- a/include/linux/usb/gadget.h
>>> >+++ b/include/linux/usb/gadget.h
>>> >@@ -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 *);
>>> >     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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek March 25, 2016, 9:09 p.m. UTC | #5
Hi!

> 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

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
Pavel Machek March 25, 2016, 9:15 p.m. UTC | #6
Hi!

> 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
Ruslan Bilovol March 26, 2016, 9:32 p.m. UTC | #7
Hi

On Fri, Mar 25, 2016 at 11:09 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> 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
>
> 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?

>
> gzipped config is attached.

Thanks, I'll try that

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

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

Best regards
Ruslan

>
> Best regards,
>
>                                                                         Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ruslan Bilovol March 26, 2016, 9:41 p.m. UTC | #8
Hi

On Fri, Mar 25, 2016 at 11:15 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> 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?

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

Please note that I'm building simple rootfs cpio binary into the kernel
so you may need to change CONFIG_INITRAMFS_SOURCE value

Best regards,
Ruslan

>
> Thanks and best regards,
>                                                                 Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Ruslan Bilovol March 26, 2016, 9:59 p.m. UTC | #9
On Sat, Mar 26, 2016 at 11:32 PM, Ruslan Bilovol
<ruslan.bilovol@gmail.com> wrote:
> Hi
>
> On Fri, Mar 25, 2016 at 11:09 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> Hi!
>>
>>> 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
>>
>> 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?
>
>>
>> gzipped config is attached.
>
> Thanks, I'll try that
>
>>
>> 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.
>
> 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?

Best regards,
Ruslan Bilovol
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ruslan Bilovol March 27, 2016, 9:44 p.m. UTC | #10
Hi,

On Sun, Mar 27, 2016 at 11:26 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> >> 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
>> >
>> > 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 .
>
>> > gzipped config is attached.
>>
>> Thanks, I'll try that
>
> Thanks.
>
>> > 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.

Best regards,
Ruslan
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek March 28, 2016, 9:33 p.m. UTC | #11
Hi!

> >> >> 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
> >> >
> >> > 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 .
> >
> >> > gzipped config is attached.
> >>
> >> Thanks, I'll try that
> >
> > Thanks.
> >
> >> > 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.

I did a quick test here, and unplug/replug does not help, I still get
error message on my PC.

[53907.784294] usb usb4-port1: unable to enumerate USB device
[53990.480064] usb 1-5: new high-speed USB device number 27 using
ehci-pci
[53990.592036] usb 1-5: device descriptor read/64, error -32
[53990.808092] usb 1-5: device descriptor read/64, error -32
[53991.024042] usb 1-5: new high-speed USB device number 28 using
ehci-pci
[53991.136043] usb 1-5: device descriptor read/64, error -32
[53991.352041] usb 1-5: device descriptor read/64, error -32
[53991.568076] usb 1-5: new high-speed USB device number 29 using
ehci-pci
[53991.589955] usb 1-5: device descriptor read/8, error -32
[53991.709703] usb 1-5: device descriptor read/8, error -32
[53991.924577] usb 1-5: new high-speed USB device number 30 using
ehci-pci
[53991.944944] usb 1-5: device descriptor read/8, error -32
[53992.065822] usb 1-5: device descriptor read/8, error -32

Best regards,
									Pavel
Pavel Machek April 1, 2016, 2:38 p.m. UTC | #12
Hi!

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

Hmm, strange. Unplug/replug of the USB cable results in:

<6>usb 1-5: new high-speed USB device number 100 using ehci-pci
<3>usb 1-5: device descriptor read/64, error -32
tail: /proc/kmsg: file truncated
<3>usb 1-5: device descriptor read/64, error -32
<3>usb 1-5: device descriptor read/64, error -32
<3>usb 1-5: device descriptor read/8, error -32
<3>usb 1-5: device descriptor read/8, error -32
<6>usb 1-5: new high-speed USB device number 103 using ehci-pci
<3>usb 1-5: device descriptor read/8, error -32
<3>usb 1-5: device descriptor read/8, error -32
<3>usb usb1-port5: unable to enumerate USB device
<6>usb 4-1: new full-speed USB device number 80 using uhci_hcd
<3>usb 4-1: device descriptor read/64, error -32
<3>usb 4-1: device descriptor read/64, error -32
<6>usb 4-1: new full-speed USB device number 81 using uhci_hcd
<3>usb 4-1: device descriptor read/64, error -32
<3>usb 4-1: device descriptor read/64, error -32
<3>usb usb4-port1: unable to enumerate USB device

echo connect does not seem to do the trick, either:

root@n900:~# echo connect >
/sys/devices/platform/68000000.ocp/480ab000.usb_otg_hs/musb-hdrc.0.auto/udc/musb-hdrc.0.auto/soft_connect
-bash: echo: write error: Operation not supported
root@n900:~# uname -a
Linux n900 4.4.0-omap3-149558-g5cf5ee5-dirty #168 PREEMPT Tue Mar 29
09:49:40 CEST 2016 armv7l GNU/Linux

Hmm, documentation is not too helpful, either...

What:           /sys/class/udc/<udc>/soft_connect
Date:           June 2011
KernelVersion:  3.1
Contact:        Felipe Balbi <balbi@kernel.org>
Description:
		Allows users to disconnect data pullup resistors thus
		causing a
 		logical disconnection from the USB
 		Host.

Dmesg says:

[  619.471374] udc musb-hdrc.0.auto: soft-connect without a gadget
driver

Any ideas?
									Pavel
diff mbox

Patch

diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index b86a6f0..bee109f 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -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)
 {
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index d82d006..adb1e68 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -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 *);
 	void			(*reset)(struct usb_gadget *);
+	struct delayed_work	work;
+	int			retries;
 
 	/* FIXME support safe rmmod */
 	struct device_driver	driver;