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