Hi Marek, Thank you for the patch.
On Thu, Sep 03, 2026 at 04:02, Marek Vasut <[email protected]> wrote: > After fsg_unbind() is called on UMS teardown, the fsg_main_thread() > which also invokes handle_exception() is unlikely to run afterward. > Therefore, any exceptions raised in fsg_unbind(), and also exceptions > raised in fsg_common_release() which is called from both fsg_unbind() > and fsg_common_init() fail path, will remain unhandled. > > This leads to a problem where FSG_STATE_TERMINATED/EXIT handling is > not performed on UMS exit and the do_set_interface(..., NULL) which > shuts down endpoints is also not performed, leaving the endpoints > enabled. > > The CI UDC driver does check for repeated enablement of endpoints and > returns -EBUSY if an endpoint is enabled repeatedly. That prevents > UMS from being started after it was exited once already. That is, > running "ums 0 mmc 0 ; ums 0 mmc 0" on U-Boot command line will lead > to one successful and one failed UMS session. The later session ends > up triggering fsg_is_set() assertion in fsg_setup() and triggers a > panic(). > > Fix this by synchronously invoking the do_set_interface() from both > fsg_unbind() and fsg_common_release(). That makes the unbind behave > as intended, as if the fsg_main_thread() ran after fsg_unbind() and > handled the outstanding raised exceptions. > > Signed-off-by: Marek Vasut <[email protected]> The change looks fine to me. I hard a hard time understanding why there was not an equivalent fix in Linux. I think it's because we have cyclic in U-Boot and real threading in the kernel. In any case: Reviewed-by: Mattijs Korpershoek <[email protected]> > --- > Cc: Mattijs Korpershoek <[email protected]> > Cc: Tom Rini <[email protected]> > Cc: [email protected] > --- > drivers/usb/gadget/f_mass_storage.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/f_mass_storage.c > b/drivers/usb/gadget/f_mass_storage.c > index 7eb667c130d..76ee08dc5e8 100644 > --- a/drivers/usb/gadget/f_mass_storage.c > +++ b/drivers/usb/gadget/f_mass_storage.c > @@ -2565,8 +2565,8 @@ static void fsg_common_release(struct fsg_common > *common) > { > /* If the thread isn't already dead, tell it to exit now */ > if (common->state != FSG_STATE_TERMINATED) { > - raise_exception(common, FSG_STATE_EXIT); > - wait_for_completion(&common->thread_notifier); > + do_set_interface(common, NULL); > + common->state = FSG_STATE_EXIT; > } > > if (likely(common->luns)) { > @@ -2647,8 +2647,8 @@ static void fsg_unbind(struct usb_configuration *c, > struct usb_function *f) > > DBG(fsg, "unbind\n"); > if (fsg->common->fsg == fsg) { > - fsg->common->new_fsg = NULL; > - raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE); > + do_set_interface(fsg->common, NULL); > + fsg->common->state = FSG_STATE_CONFIG_CHANGE; > } > > fsg_common_release(fsg->common); > -- > 2.53.0
