> On Fri, 1 Apr 2016, Michal Nazarewicz wrote: >> @@ -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) { > On Fri, Apr 01 2016, Alan Stern wrote: > Do you mean: if (!common->thread_task)? Correct. >> + 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. > > I'm not so sure about this. Earlier I was undecided about what to do > if there are multiple mass-storage interfaces in the same config, but > now I realize that one thread won't work in that situation. The > difficulty is exception handling. If an error occurs in one of the > interfaces, it shouldn't affect the others -- but with only one thread > you can't make that work. > > I don't know the details of how the composite core works (I don't even > remember the difference between a usb_function and > a usb_function_instance). configfs made things quite confusing, yes. IIRC, usb_function roughly corresponds to an interface while usb_function_instance corresponds to a module/function driver, except you can easily create multiple instances. > Suppose there are several interfaces in one config, each bound to the > mass-storage driver. Then won't the driver's bind routine get called > several times whenever the config is installed? > > If so, then each of those calls should create a new thread. There's no > need to check if the thread has already been started. > > If not, then there's no way to use f_mass_storage on multiple > interfaces in a single config. The driver should refuse to run on > more than one interface at a time. Right, but that’s not what is happening. We have a situation where single mass storage instance is bound to two interfaces on two different configs: mkdir functions/mass_storage.0 echo $file > functions/mass_storage.0/lun.0/file ln -s functions/mass_storage.0 configs/c.1 ln -s functions/mass_storage.0 configs/c.2 configfs doesn’t even allows to express situation where a single instance of a function is bound to multiple interfaces in a single configuration (IIRC). At the same time, mass storage should work fine if it’s bound to multiple configurations. Only one configuration can be active at any given time so interfaces on different configurations cannot interfere with each other. The problem we are having is that when mass storage is added to a configuration, fsg_bind is called and it starts the thread. If someone wanted to have two mass storage interfaces on a single configuration they would have to create two mass storage instances and each instance has it’s own fsg_common struct (see fsg_alloc_inst function). PS. It seems that legacy/multi is broken because it calls fsg_common_run_thread twice. -- 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