Re: [PATCH] usb: gadget: udc: skip pullup() if already connected

[Alan Stern]

On Tue, Apr 21, 2026 at 05:27:32PM +0200, Bjørn Mork wrote:
> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes:
> 
> > This patch is wrong.  To see why, read the comments just below the end 
> > of the patch and see also usb_gadget_activate().
> 
> 
> Made me look...
> 
> Must say. This strikes me as a nice way to filter out humans from the
> rest of the submitters:
> 
>                 gadget->connected = true;
>                 goto out;
>         }
>  
>         ret = gadget->ops->pullup(gadget, 1);
>         if (!ret)
>                 gadget->connected = 1;
> 
> 
> The indecisiveness looks strange. There's a nice symmetry with
> usb_gadget_disconnect_locked() though:
> 
>                 gadget->connected = false;
>                 goto out;
>         }
>  
>         ret = gadget->ops->pullup(gadget, 0);
>         if (!ret)
>                 gadget->connected = 0;

Granted, it does look a little strange.  ->connected is actually a 1-bit 
bitfield, though, so treating it as a boolean instead of an integer is 
not completely beyond the bounds of reason.

> What surprised me most was that the different variants were added by the
> same commit
> ccdf138fe3e2 ("usb: gadget: add usb_gadget_activate/deactivate functions").

But this isn't what my rejection was complaining about.  The patch's 
problem was functional, not aesthetic.

Alan Stern