Re: USB gadgets with configfs hang reboot

[Michal Nazarewicz <mina86@xxxxxxxxxx>]

On Thu, Mar 31 2016, Alan Stern wrote:
> Michal, I'm not sure how you intended to handle this.

For legacy/nokia setting no_configfs should be a valid solution:

---- >8 ----------------------------------------------------------------
diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
index 0997504..e4bc607 100644
--- a/drivers/usb/gadget/legacy/nokia.c
+++ b/drivers/usb/gadget/legacy/nokia.c
@@ -223,6 +223,7 @@ static int nokia_bind_config(struct usb_configuration *c)
        }
 
        fsg_opts = fsg_opts_from_func_inst(fi_msg);
+       fsg_opts->no_configfs = true;
 
        status = fsg_common_run_thread(fsg_opts->common);
        if (status)
---- >8 ----------------------------------------------------------------

This is what other legacy gadgets do after all.

For configfs, why not simply check if the thread has already been
started:

---- >8 ----------------------------------------------------------------
diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index acf210f..62854b6 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2984,8 +2984,10 @@ int fsg_common_run_thread(struct fsg_common *common)
        common->thread_task =
                kthread_create(fsg_main_thread, common, "file-storage");
        if (IS_ERR(common->thread_task)) {
+               int ret = PTR_ERR(common->thread_task);
+               common->thread_task = NULL;
                common->state = FSG_STATE_TERMINATED;
-               return PTR_ERR(common->thread_task);
+               return ret;
        }
 
        DBG(common, "I/O thread pid: %d\n", task_pid_nr(common->thread_task));
@@ -3005,6 +3007,7 @@ static void fsg_common_release(struct kref *ref)
        if (common->state != FSG_STATE_TERMINATED) {
                raise_exception(common, FSG_STATE_EXIT);
                wait_for_completion(&common->thread_notifier);
+               common->thread_task = NULL;
        }
 
        for (i = 0; i < ARRAY_SIZE(common->luns); ++i) {
@@ -3050,9 +3053,11 @@ static int fsg_bind(struct usb_configuration *c, struct usb_function *f)
                if (ret)
                        return ret;
                fsg_common_set_inquiry_string(fsg->common, NULL, NULL);
-               ret = fsg_common_run_thread(fsg->common);
-               if (ret)
-                       return ret;
+               if (common->thread_task) {
+                       ret = fsg_common_run_thread(fsg->common);
+                       if (ret)
+                               return ret;
+               }
        }
 
        fsg->gadget = gadget;
---- >8 ----------------------------------------------------------------

With that, the whole fsg_common_run_thread call should be moved outside
of the if (!no_configfs) and legacy should no longer call it IMO.

-- 
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html