Revert "usb: gadget: udc: core: Invoke usb_gadget_connect only when started"

commit f22e9b67f19ccc73de1ae04375d4b30684e261f8 upstream.

This reverts commit 0db213ea8eed5534a5169e807f28103cbc9d23df.

It introduces an issues with configuring the USB gadget hangs forever
on multiple Qualcomm and NXP i.MX SoC at least.

Cc: stable@vger.kernel.org
Fixes: 0db213ea8eed ("usb: gadget: udc: core: Invoke usb_gadget_connect only when started")
Reported-by: Stephan Gerhold <stephan@gerhold.net>
Reported-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Link: https://lore.kernel.org/all/ZF4BvgsOyoKxdPFF@francesco-nb.int.toradex.com/
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Link: https://lore.kernel.org/r/20230512131435.205464-3-francesco@dolcini.it
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index ebb2b839b470b64c20a21f817c943f5e4750bc88..23b0629a877431325c3f2f35f900388d9494d2cb 100644 (file)
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -37,10 +37,6 @@ static struct bus_type gadget_bus_type;
  * @vbus: for udcs who care about vbus status, this value is real vbus status;
  * for udcs who do not care about vbus status, this value is always true
  * @started: the UDC's started state. True if the UDC had started.
- * @connect_lock: protects udc->vbus, udc->started, gadget->connect, gadget->deactivate related
- * functions. usb_gadget_connect_locked, usb_gadget_disconnect_locked,
- * usb_udc_connect_control_locked, usb_gadget_udc_start_locked, usb_gadget_udc_stop_locked are
- * called with this lock held.
  *
  * This represents the internal data structure which is used by the UDC-class
  * to hold information about udc driver and gadget together.
@@ -52,7 +48,6 @@ struct usb_udc {
        struct list_head                list;
        bool                            vbus;
        bool                            started;
-       struct mutex                    connect_lock;
 };
 
 static struct class *udc_class;
@@ -665,9 +660,17 @@ int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
 }
 EXPORT_SYMBOL_GPL(usb_gadget_vbus_disconnect);
 
-/* Internal version of usb_gadget_connect needs to be called with connect_lock held. */
-static int usb_gadget_connect_locked(struct usb_gadget *gadget)
-       __must_hold(&gadget->udc->connect_lock)
+/**
+ * usb_gadget_connect - software-controlled connect to USB host
+ * @gadget:the peripheral being connected
+ *
+ * Enables the D+ (or potentially D-) pullup.  The host will start
+ * enumerating this gadget when the pullup is active and a VBUS session
+ * is active (the link is powered).
+ *
+ * Returns zero on success, else negative errno.
+ */
+int usb_gadget_connect(struct usb_gadget *gadget)
 {
        int ret = 0;
 
@@ -676,12 +679,10 @@ static int usb_gadget_connect_locked(struct usb_gadget *gadget)
                goto out;
        }
 
-       if (gadget->deactivated || !gadget->udc->started) {
+       if (gadget->deactivated) {
                /*
                 * If gadget is deactivated we only save new state.
                 * Gadget will be connected automatically after activation.
-                *
-                * udc first needs to be started before gadget can be pulled up.
                 */
                gadget->connected = true;
                goto out;
@@ -696,32 +697,22 @@ static int usb_gadget_connect_locked(struct usb_gadget *gadget)
 
        return ret;
 }
+EXPORT_SYMBOL_GPL(usb_gadget_connect);
 
 /**
- * usb_gadget_connect - software-controlled connect to USB host
- * @gadget:the peripheral being connected
+ * usb_gadget_disconnect - software-controlled disconnect from USB host
+ * @gadget:the peripheral being disconnected
  *
- * Enables the D+ (or potentially D-) pullup.  The host will start
- * enumerating this gadget when the pullup is active and a VBUS session
- * is active (the link is powered).
+ * Disables the D+ (or potentially D-) pullup, which the host may see
+ * as a disconnect (when a VBUS session is active).  Not all systems
+ * support software pullup controls.
+ *
+ * Following a successful disconnect, invoke the ->disconnect() callback
+ * for the current gadget driver so that UDC drivers don't need to.
  *
  * Returns zero on success, else negative errno.
  */
-int usb_gadget_connect(struct usb_gadget *gadget)
-{
-       int ret;
-
-       mutex_lock(&gadget->udc->connect_lock);
-       ret = usb_gadget_connect_locked(gadget);
-       mutex_unlock(&gadget->udc->connect_lock);
-
-       return ret;
-}
-EXPORT_SYMBOL_GPL(usb_gadget_connect);
-
-/* Internal version of usb_gadget_disconnect needs to be called with connect_lock held. */
-static int usb_gadget_disconnect_locked(struct usb_gadget *gadget)
-       __must_hold(&gadget->udc->connect_lock)
+int usb_gadget_disconnect(struct usb_gadget *gadget)
 {
        int ret = 0;
 
@@ -733,12 +724,10 @@ static int usb_gadget_disconnect_locked(struct usb_gadget *gadget)
        if (!gadget->connected)
                goto out;
 
-       if (gadget->deactivated || !gadget->udc->started) {
+       if (gadget->deactivated) {
                /*
                 * If gadget is deactivated we only save new state.
                 * Gadget will stay disconnected after activation.
-                *
-                * udc should have been started before gadget being pulled down.
                 */
                gadget->connected = false;
                goto out;
@@ -758,30 +747,6 @@ static int usb_gadget_disconnect_locked(struct usb_gadget *gadget)
 
        return ret;
 }
-
-/**
- * usb_gadget_disconnect - software-controlled disconnect from USB host
- * @gadget:the peripheral being disconnected
- *
- * Disables the D+ (or potentially D-) pullup, which the host may see
- * as a disconnect (when a VBUS session is active).  Not all systems
- * support software pullup controls.
- *
- * Following a successful disconnect, invoke the ->disconnect() callback
- * for the current gadget driver so that UDC drivers don't need to.
- *
- * Returns zero on success, else negative errno.
- */
-int usb_gadget_disconnect(struct usb_gadget *gadget)
-{
-       int ret;
-
-       mutex_lock(&gadget->udc->connect_lock);
-       ret = usb_gadget_disconnect_locked(gadget);
-       mutex_unlock(&gadget->udc->connect_lock);
-
-       return ret;
-}
 EXPORT_SYMBOL_GPL(usb_gadget_disconnect);
 
 /**
@@ -802,11 +767,10 @@ int usb_gadget_deactivate(struct usb_gadget *gadget)
        if (gadget->deactivated)
                goto out;
 
-       mutex_lock(&gadget->udc->connect_lock);
        if (gadget->connected) {
-               ret = usb_gadget_disconnect_locked(gadget);
+               ret = usb_gadget_disconnect(gadget);
                if (ret)
-                       goto unlock;
+                       goto out;
 
                /*
                 * If gadget was being connected before deactivation, we want
@@ -816,8 +780,6 @@ int usb_gadget_deactivate(struct usb_gadget *gadget)
        }
        gadget->deactivated = true;
 
-unlock:
-       mutex_unlock(&gadget->udc->connect_lock);
 out:
        trace_usb_gadget_deactivate(gadget, ret);
 
@@ -841,7 +803,6 @@ int usb_gadget_activate(struct usb_gadget *gadget)
        if (!gadget->deactivated)
                goto out;
 
-       mutex_lock(&gadget->udc->connect_lock);
        gadget->deactivated = false;
 
        /*
@@ -849,8 +810,7 @@ int usb_gadget_activate(struct usb_gadget *gadget)
         * while it was being deactivated, we call usb_gadget_connect().
         */
        if (gadget->connected)
-               ret = usb_gadget_connect_locked(gadget);
-       mutex_unlock(&gadget->udc->connect_lock);
+               ret = usb_gadget_connect(gadget);
 
 out:
        trace_usb_gadget_activate(gadget, ret);
@@ -1091,13 +1051,12 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
 
 /* ------------------------------------------------------------------------- */
 
-/* Acquire connect_lock before calling this function. */
-static void usb_udc_connect_control_locked(struct usb_udc *udc) __must_hold(&udc->connect_lock)
+static void usb_udc_connect_control(struct usb_udc *udc)
 {
-       if (udc->vbus && udc->started)
-               usb_gadget_connect_locked(udc->gadget);
+       if (udc->vbus)
+               usb_gadget_connect(udc->gadget);
        else
-               usb_gadget_disconnect_locked(udc->gadget);
+               usb_gadget_disconnect(udc->gadget);
 }
 
 /**
@@ -1113,12 +1072,10 @@ void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
 {
        struct usb_udc *udc = gadget->udc;
 
-       mutex_lock(&udc->connect_lock);
        if (udc) {
                udc->vbus = status;
-               usb_udc_connect_control_locked(udc);
+               usb_udc_connect_control(udc);
        }
-       mutex_unlock(&udc->connect_lock);
 }
 EXPORT_SYMBOL_GPL(usb_udc_vbus_handler);
 
@@ -1140,7 +1097,7 @@ void usb_gadget_udc_reset(struct usb_gadget *gadget,
 EXPORT_SYMBOL_GPL(usb_gadget_udc_reset);
 
 /**
- * usb_gadget_udc_start_locked - tells usb device controller to start up
+ * usb_gadget_udc_start - tells usb device controller to start up
  * @udc: The UDC to be started
  *
  * This call is issued by the UDC Class driver when it's about
@@ -1151,11 +1108,8 @@ EXPORT_SYMBOL_GPL(usb_gadget_udc_reset);
  * necessary to have it powered on.
  *
  * Returns zero on success, else negative errno.
- *
- * Caller should acquire connect_lock before invoking this function.
  */
-static inline int usb_gadget_udc_start_locked(struct usb_udc *udc)
-       __must_hold(&udc->connect_lock)
+static inline int usb_gadget_udc_start(struct usb_udc *udc)
 {
        int ret;
 
@@ -1172,7 +1126,7 @@ static inline int usb_gadget_udc_start_locked(struct usb_udc *udc)
 }
 
 /**
- * usb_gadget_udc_stop_locked - tells usb device controller we don't need it anymore
+ * usb_gadget_udc_stop - tells usb device controller we don't need it anymore
  * @udc: The UDC to be stopped
  *
  * This call is issued by the UDC Class driver after calling
@@ -1181,11 +1135,8 @@ static inline int usb_gadget_udc_start_locked(struct usb_udc *udc)
  * The details are implementation specific, but it can go as
  * far as powering off UDC completely and disable its data
  * line pullups.
- *
- * Caller should acquire connect lock before invoking this function.
  */
-static inline void usb_gadget_udc_stop_locked(struct usb_udc *udc)
-       __must_hold(&udc->connect_lock)
+static inline void usb_gadget_udc_stop(struct usb_udc *udc)
 {
        if (!udc->started) {
                dev_err(&udc->dev, "UDC had already stopped\n");
@@ -1344,7 +1295,6 @@ int usb_add_gadget(struct usb_gadget *gadget)
 
        udc->gadget = gadget;
        gadget->udc = udc;
-       mutex_init(&udc->connect_lock);
 
        udc->started = false;
 
@@ -1546,15 +1496,11 @@ static int gadget_bind_driver(struct device *dev)
        if (ret)
                goto err_bind;
 
-       mutex_lock(&udc->connect_lock);
-       ret = usb_gadget_udc_start_locked(udc);
-       if (ret) {
-               mutex_unlock(&udc->connect_lock);
+       ret = usb_gadget_udc_start(udc);
+       if (ret)
                goto err_start;
-       }
        usb_gadget_enable_async_callbacks(udc);
-       usb_udc_connect_control_locked(udc);
-       mutex_unlock(&udc->connect_lock);
+       usb_udc_connect_control(udc);
 
        kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
        return 0;
@@ -1585,14 +1531,12 @@ static void gadget_unbind_driver(struct device *dev)
 
        kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
 
-       mutex_lock(&udc->connect_lock);
-       usb_gadget_disconnect_locked(gadget);
+       usb_gadget_disconnect(gadget);
        usb_gadget_disable_async_callbacks(udc);
        if (gadget->irq)
                synchronize_irq(gadget->irq);
        udc->driver->unbind(gadget);
-       usb_gadget_udc_stop_locked(udc);
-       mutex_unlock(&udc->connect_lock);
+       usb_gadget_udc_stop(udc);
 
        mutex_lock(&udc_lock);
        driver->is_bound = false;
@@ -1678,15 +1622,11 @@ static ssize_t soft_connect_store(struct device *dev,
        }
 
        if (sysfs_streq(buf, "connect")) {
-               mutex_lock(&udc->connect_lock);
-               usb_gadget_udc_start_locked(udc);
-               usb_gadget_connect_locked(udc->gadget);
-               mutex_unlock(&udc->connect_lock);
+               usb_gadget_udc_start(udc);
+               usb_gadget_connect(udc->gadget);
        } else if (sysfs_streq(buf, "disconnect")) {
-               mutex_lock(&udc->connect_lock);
-               usb_gadget_disconnect_locked(udc->gadget);
-               usb_gadget_udc_stop_locked(udc);
-               mutex_unlock(&udc->connect_lock);
+               usb_gadget_disconnect(udc->gadget);
+               usb_gadget_udc_stop(udc);
        } else {
                dev_err(dev, "unsupported command '%s'\n", buf);
                ret = -EINVAL;