[PATCH] usb: gadget: udc: Clear a pending connection when starting the UDC

[PATCH] usb: gadget: udc: Clear a pending connection when starting the UDC

[Alexey Charkov]

Since commit 62911bc82b03 ("usb: gadget: udc: skip pullup() if already
connected"), usb_gadget_connect_locked() returns early when
gadget->connected is set. A connection requested while the UDC is
stopped sets that flag too, without enabling the pullup. A later
gadget_bind_driver() or soft_connect "connect" then finds it set and
never enables the pullup, so the host doesn't see the gadget until it is
unbound and bound again.

This happens whenever a UDC driver reports VBUS through
usb_udc_vbus_handler() while no gadget driver is bound, e.g. when a
Type-C port controller switches the controller to the device role at
boot, seconds before userspace binds a configfs gadget.

The pullup is never enabled while the UDC is stopped, so clear
gadget->connected once it starts. The bind connects according to the
VBUS state that the core tracks anyway, and a function that deactivates
the gadget during bind still has the connection recorded for
usb_gadget_activate().

Fixes: 62911bc82b03 ("usb: gadget: udc: skip pullup() if already connected")
Cc: stable@vger.kernel.org
Signed-off-by: Alexey Charkov <alchark@flipper.net>
---
 drivers/usb/gadget/udc/core.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index d5844384c368..577bc15f61a1 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -1236,8 +1236,16 @@ static inline int usb_gadget_udc_start_locked(struct usb_udc *udc)
 	}
 
 	ret = udc->gadget->ops->udc_start(udc->gadget, udc->driver);
-	if (!ret)
+	if (!ret) {
 		udc->started = true;
+		/*
+		 * The pullup is never enabled while the UDC is stopped, so any
+		 * connection recorded meanwhile was only a request. Forget it,
+		 * so that the next usb_gadget_connect_locked() enables the
+		 * pullup rather than returning early.
+		 */
+		udc->gadget->connected = false;
+	}
 
 	return ret;
 }

---
base-commit: aaa2f954c21b205a0bcd0961e5b2be4e82e58455
change-id: 20261112-udc-connect-61e0f11cf93b

Best regards,
--  
Alexey Charkov <alchark@flipper.net>

Re: [PATCH] usb: gadget: udc: Clear a pending connection when starting the UDC

[sashiko-bot]

> Since commit 62911bc82b03 ("usb: gadget: udc: skip pullup() if already
> connected"), usb_gadget_connect_locked() returns early when
> gadget->connected is set. A connection requested while the UDC is
> stopped sets that flag too, without enabling the pullup. A later
> gadget_bind_driver() or soft_connect "connect" then finds it set and
> never enables the pullup, so the host doesn't see the gadget until it is
> unbound and bound again.
> 
> This happens whenever a UDC driver reports VBUS through
> usb_udc_vbus_handler() while no gadget driver is bound, e.g. when a
> Type-C port controller switches the controller to the device role at
> boot, seconds before userspace binds a configfs gadget.
> 
> The pullup is never enabled while the UDC is stopped, so clear
> gadget->connected once it starts. The bind connects according to the
> VBUS state that the core tracks anyway, and a function that deactivates
> the gadget during bind still has the connection recorded for
> usb_gadget_activate().
> 
> Fixes: 62911bc82b03 ("usb: gadget: udc: skip pullup() if already connected")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexey Charkov <alchark@flipper.net>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-udc-connect-v1-1-03c73259642b@flipper.net?part=1

Re: [PATCH] usb: gadget: udc: Clear a pending connection when starting the UDC

[Alan Stern]

On Thu, Nov 12, 2026 at 06:31:35PM +0400, Alexey Charkov wrote:
> Since commit 62911bc82b03 ("usb: gadget: udc: skip pullup() if already
> connected"), usb_gadget_connect_locked() returns early when
> gadget->connected is set. A connection requested while the UDC is
> stopped sets that flag too, without enabling the pullup. A later
> gadget_bind_driver() or soft_connect "connect" then finds it set and
> never enables the pullup, so the host doesn't see the gadget until it is
> unbound and bound again.
> 
> This happens whenever a UDC driver reports VBUS through
> usb_udc_vbus_handler() while no gadget driver is bound, e.g. when a
> Type-C port controller switches the controller to the device role at
> boot, seconds before userspace binds a configfs gadget.
> 
> The pullup is never enabled while the UDC is stopped, so clear
> gadget->connected once it starts. The bind connects according to the
> VBUS state that the core tracks anyway, and a function that deactivates
> the gadget during bind still has the connection recorded for
> usb_gadget_activate().

This is good but it doesn't go far enough.

The UDC core has gone a little overboard in the ways it allows gadgets 
to connect to or disconnect from the host.  Along with the automatic 
actions associated with bind and unbind, there's vbus_event_work(), 
usb_gadget_{de}activate(), and soft_connect_store().  In some respects 
these interfaces act at cross purposes.

(There's also usb_gadget_{dis}connect().  Those two routines seem to be 
EXPORTed solely for the benefit of 
drivers/power/supply/isp1704_charger.c -- is this really a good idea?)

In addition, there's a good case to be made that soft_connect_store() 
should rely entirely on udc->connect_lock, not on 
device_lock(&udc->gadget->dev), and it should check 
udc->allow_connect instead of udc->driver, and it should not 
call usb_gadget_udc_start_locked() or usb_gadget_udc_stop_locked().

And of course, as you implied but didn't state explicitly, 
gadget->connected is being used for two different purposes: to indicate 
the current connection state, and to indicate whether a connection 
should be started when it becomes possible to do so.  There should be a 
separate variable -- or none at all -- to handle this secondary purpose.

The whole situation needs to be straightened out.  I think the following 
approach makes sense:

    1.	Connecting is never allowed if no driver is bound, because 
	in that condition the gadget can't do anything.

    2.  Connecting is not allowed if Vbus power isn't present, because 
	it would violate the USB spec.

    3.	Connecting is not allowed if the gadget driver has
	deactivated the gadget.  Note that the activation setting has 
	to be forgotten when the gadget driver unbinds, since it is
	driver-specific.

    4.	soft_connect_store() should be able to force a disconnect, 
	but it shouldn't be able to override 1 - 3 by forcing a
	connect.

    5.	Whenever the state changes, the gadget should try to connect 
	if not prevented from doing so by 1 - 4.

In addition, the gadget should be started when it binds to a driver and 
stopped when it unbinds, with no changes at any other times.  (Read the 
kerneldoc for usb_gadget_udc_stop_locked() to see why.)

And the proper way to test whether none of conditions 1 - 3 apply should 
be to check udc->allow_connect while holding udc->connect_lock.  All the 
relevant routines should maintain this invariant, perhaps by calling a 
subroutine to set udc->allow_connect whenever the state changes.

Have I left anything out?

We do a lot of this already, but not all of it.  How would you like to 
implement this scheme fully?

Alan Stern