USB: atmel_usba_udc fixes, mostly disconnect()

Various fixes to Atmel's high speed UDC driver.

* Issue some missing disconnect() calls. Currently they are only made
when VBUS power goes away (on boards where the driver can sense such
changes), but that's not enough for gadget drivers to clean out all
the state that's needed. Missing calls were:

- After USB reset, before starting enumeration.
- When unregistering a gadget driver, before unbind().

* Don't assume gadget drivers provide disconnect callbacks; make sure
to not call through a null pointer!

* When the driver doesn't provide an unbind() callback, refuse to
unregister it.

Also remove two bogus "error" messages:

* Related to mis-handling of disconnect() ... don't emit error messages
for disconnect() handlers that disable endpoints. All of them should
be doing that; the problem is (unfixed) oddness in atmel_usba_udc.

* Don't emit a diagnostic for a curious and transient nonfatal error
that shows up sometimes with EP0.

Those messages spammed syslog, for no good reason.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Acked-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

authored by David Brownell and committed by Greg Kroah-Hartman 40517707 5a59bc54

+38 -10
+38 -10
drivers/usb/gadget/atmel_usba_udc.c
··· 649 650 if (!ep->desc) { 651 spin_unlock_irqrestore(&udc->lock, flags); 652 - DBG(DBG_ERR, "ep_disable: %s not enabled\n", ep->ep.name); 653 return -EINVAL; 654 } 655 ep->desc = NULL; ··· 1038 .release = nop_release, 1039 }, 1040 }, 1041 - 1042 - .lock = SPIN_LOCK_UNLOCKED, 1043 }; 1044 1045 /* ··· 1056 request_complete(ep, req, -ECONNRESET); 1057 } 1058 1059 list_for_each_entry(ep, &udc->gadget.ep_list, ep.ep_list) { 1060 if (ep->desc) { 1061 spin_unlock(&udc->lock); ··· 1229 static int handle_ep0_setup(struct usba_udc *udc, struct usba_ep *ep, 1230 struct usb_ctrlrequest *crq) 1231 { 1232 - int retval = 0;; 1233 1234 switch (crq->bRequest) { 1235 case USB_REQ_GET_STATUS: { ··· 1703 usba_writel(udc, INT_CLR, USBA_END_OF_RESET); 1704 reset_all_endpoints(udc); 1705 1706 if (status & USBA_HIGH_SPEED) { 1707 DBG(DBG_BUS, "High-speed bus reset detected\n"); 1708 udc->gadget.speed = USB_SPEED_HIGH; ··· 1734 | USBA_DET_SUSPEND 1735 | USBA_END_OF_RESUME)); 1736 1737 if (!(usba_ep_readl(ep0, CFG) & USBA_EPT_MAPPED)) 1738 - dev_warn(&udc->pdev->dev, 1739 - "WARNING: EP0 configuration is invalid!\n"); 1740 } 1741 1742 spin_unlock(&udc->lock); ··· 1773 reset_all_endpoints(udc); 1774 toggle_bias(0); 1775 usba_writel(udc, CTRL, USBA_DISABLE_MASK); 1776 - spin_unlock(&udc->lock); 1777 - udc->driver->disconnect(&udc->gadget); 1778 - spin_lock(&udc->lock); 1779 } 1780 udc->vbus_prev = vbus; 1781 } ··· 1849 1850 if (!udc->pdev) 1851 return -ENODEV; 1852 - if (driver != udc->driver) 1853 return -EINVAL; 1854 1855 if (udc->vbus_pin != -1) ··· 1863 /* This will also disable the DP pullup */ 1864 toggle_bias(0); 1865 usba_writel(udc, CTRL, USBA_DISABLE_MASK); 1866 1867 driver->unbind(&udc->gadget); 1868 udc->gadget.dev.driver = NULL; ··· 1906 goto err_get_hclk; 1907 } 1908 1909 udc->pdev = pdev; 1910 udc->pclk = pclk; 1911 udc->hclk = hclk;
··· 649 650 if (!ep->desc) { 651 spin_unlock_irqrestore(&udc->lock, flags); 652 + /* REVISIT because this driver disables endpoints in 653 + * reset_all_endpoints() before calling disconnect(), 654 + * most gadget drivers would trigger this non-error ... 655 + */ 656 + if (udc->gadget.speed != USB_SPEED_UNKNOWN) 657 + DBG(DBG_ERR, "ep_disable: %s not enabled\n", 658 + ep->ep.name); 659 return -EINVAL; 660 } 661 ep->desc = NULL; ··· 1032 .release = nop_release, 1033 }, 1034 }, 1035 }; 1036 1037 /* ··· 1052 request_complete(ep, req, -ECONNRESET); 1053 } 1054 1055 + /* NOTE: normally, the next call to the gadget driver is in 1056 + * charge of disabling endpoints... usually disconnect(). 1057 + * The exception would be entering a high speed test mode. 1058 + * 1059 + * FIXME remove this code ... and retest thoroughly. 1060 + */ 1061 list_for_each_entry(ep, &udc->gadget.ep_list, ep.ep_list) { 1062 if (ep->desc) { 1063 spin_unlock(&udc->lock); ··· 1219 static int handle_ep0_setup(struct usba_udc *udc, struct usba_ep *ep, 1220 struct usb_ctrlrequest *crq) 1221 { 1222 + int retval = 0; 1223 1224 switch (crq->bRequest) { 1225 case USB_REQ_GET_STATUS: { ··· 1693 usba_writel(udc, INT_CLR, USBA_END_OF_RESET); 1694 reset_all_endpoints(udc); 1695 1696 + if (udc->gadget.speed != USB_SPEED_UNKNOWN 1697 + && udc->driver->disconnect) { 1698 + udc->gadget.speed = USB_SPEED_UNKNOWN; 1699 + spin_unlock(&udc->lock); 1700 + udc->driver->disconnect(&udc->gadget); 1701 + spin_lock(&udc->lock); 1702 + } 1703 + 1704 if (status & USBA_HIGH_SPEED) { 1705 DBG(DBG_BUS, "High-speed bus reset detected\n"); 1706 udc->gadget.speed = USB_SPEED_HIGH; ··· 1716 | USBA_DET_SUSPEND 1717 | USBA_END_OF_RESUME)); 1718 1719 + /* 1720 + * Unclear why we hit this irregularly, e.g. in usbtest, 1721 + * but it's clearly harmless... 1722 + */ 1723 if (!(usba_ep_readl(ep0, CFG) & USBA_EPT_MAPPED)) 1724 + dev_dbg(&udc->pdev->dev, 1725 + "ODD: EP0 configuration is invalid!\n"); 1726 } 1727 1728 spin_unlock(&udc->lock); ··· 1751 reset_all_endpoints(udc); 1752 toggle_bias(0); 1753 usba_writel(udc, CTRL, USBA_DISABLE_MASK); 1754 + if (udc->driver->disconnect) { 1755 + spin_unlock(&udc->lock); 1756 + udc->driver->disconnect(&udc->gadget); 1757 + spin_lock(&udc->lock); 1758 + } 1759 } 1760 udc->vbus_prev = vbus; 1761 } ··· 1825 1826 if (!udc->pdev) 1827 return -ENODEV; 1828 + if (driver != udc->driver || !driver->unbind) 1829 return -EINVAL; 1830 1831 if (udc->vbus_pin != -1) ··· 1839 /* This will also disable the DP pullup */ 1840 toggle_bias(0); 1841 usba_writel(udc, CTRL, USBA_DISABLE_MASK); 1842 + 1843 + if (udc->driver->disconnect) 1844 + udc->driver->disconnect(&udc->gadget); 1845 1846 driver->unbind(&udc->gadget); 1847 udc->gadget.dev.driver = NULL; ··· 1879 goto err_get_hclk; 1880 } 1881 1882 + spin_lock_init(&udc->lock); 1883 udc->pdev = pdev; 1884 udc->pclk = pclk; 1885 udc->hclk = hclk;