On Monday, March 18, 2013 12:52 PM Felipe Balbi wrote: > Hi, > > On Mon, Mar 18, 2013 at 10:23:32AM +0100, Andrzej Pietrasiewicz wrote: > > > The prerequisite for providing the configfs interface for USB > > > gadgets and all their users is converting them to the new function > > > interface from Sebastian. > > > > > > This patch series serves the purpose stated above for mass storage, > > > usb Ethernet, FunctionFS and serial. > > > > <snip> > > > > @Felipe, @Sebastian: Converting the above mentioned functions to the > > new interface has already been a considerable effort, so could you > > please share your opinion? > > > > I understand that Sebastian might not have enough time to do this step > > of conversion to configfs himself, but with converting the functions > > to the new function interface I followed his example from e.g. acm. > > > > Additional questions: > > > > 1. Is converting the functions to the new function interface the right > > way to go? (given the discussions so far I believe it is, but kindly > > ask for confirmation or pointing at what to improve) > > > > 2. What is the preferred way of finally arriving at a configfs-based > > gadget: breadth-first approach (convert all functions to the new > > function interface, then move to configfs) or depth-first approach > > (convert a function and then move it to configfs, then do the same > > with some next function and so on) > > I think it's best to convert them one at a time, so that we can focus on a > single function and make sure there are no regressions before working on > the following one. > > What concerns me the most, however, is how do we keep the traditional > modprobe g_mass_storage functionality once we move to configfs based > approach ? > > We need to keep that around at least for a few years. Any suggestions are > highly appreciated. > (@Sebastian: Do you think it is possible to simply keep the g_* modules for some time along with the new configfs-based approach?) If it is not possible to keep old g_* modules along new configfs-based stuff, then I think it _should_ be possible to create an adapter module, which to the user looks like the old gadget module (e.g. g_mass_storage), but internally operates on configfs to create the gadget, write PID/VID etc. A proof-of-concept (not intended for merging or discussing, just pointing it for reference) can be found here: http://www.spinics.net/lists/linux-usb/msg74871.html As far as I remember it involves two things: 1) The configfs subsystem must be available to the adapter (the UFG_SUBSYSTEM in the above patch) 2) Programmatic operation on configfs entries tends to be lengthy, like: ci->ci_type->ct_item_ops->store_attribute(ci, attr, s, n); which suggests that it is not the usual way to use configfs. If the adapters are to be temporary, we could live with that, though. Perhaps the adapters could be organized in a more generic way. Below is a bunch of ideas to shout at :O Suppose there is just one generic adapter module, which accepts some textual descriptions from userspace, and based on that it creates appropriate entries in configfs. The textual descriptions could be loaded as "firmware". To provide the old interface the specialized g_* adapter modules would be very simple (like 30 lines of code or so) and just point to appropriate "firmware" to be used by the generic adapter, e.g.: g_mass_storage.c: static struct usb_configfs_adapter msg_adapter = { .name = "g_mass_storage", /* ... other stuff ... */ }; int msg_adapter_init(void) { int ret; ret = generic_adapter_provide(&msg_adapter); if (ret) return ret; /* ... additional stuff required for mass storage ... e.g. sysfs */ return generic_adapter_enable(&msg_adapter); } module_init(msg_adapter_init); void msg_adapter_cleanup(void) { generic_adapter_disable(&msg_adapter); /* ... additional stuff required for mass storage ... e.g. sysfs */ generic_adapter_remove(&msg_adapter); } module_exit(msg_adapter_cleanup); The generic adapter would know to load "firmware" file named e.g. "g_mass_storage.def", and process it accordingly. The "firmware" would specify what configfs directories to create and where, and what names to store in configfs files, e.g.: create:item store:item/attribute:some_val would mean to create "item" and then store "some_val" into "item/attribute". Alternatively, there could be some generic data structure to be filled by specialized adapters instead of loading anything from userspace: enum adapter_data_type { USB_CONFIGFS_ATTR, USB_CONFIGFS_ITEM, }; struct usb_configfs_adapter_data { enum adapter_data_type type; const char *path; const char *value; }; g_mass_storage.c: static struct usb_configfs_adapter_data[] = { { .type = USB_CONFIGFS_ITEM, .path = "item", /* .value not required for ITEM */ }, { .type = USB_CONFIGFS_ATTR, .path = "item/attribute", .value = "some_val", }, }; but this tends to be lengthy. On the other hand no worries about accepting (potentially suspicious) data from userspace. Another benefit of the generic data structure is that it can be created by the specialized adapter at runtime, while the "firmware" file is static by its nature (what if the user does modprobe g_mass_storage nluns=5?). Anyway, this is not ideal, because we exchange a problem of repeated configfs mkdirs/writes for a problem of repeated creations of the above structure with configfs operations being localized to the generic adapter. Of course, some gadgets would still need some more code in their specialized adapters, e.g. for mass storage there is a need to set up sysfs entries. Andrzej -- 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
