Thanks for the responses everyone > IMHO, considering the amount of Android users, we can merge this into > composite.c itself. Just make the code depend on CONFIG_ANDROID. Or > something along the lines of I suspect that once you see what this entails you won't be so willing :) The code for handling accessory control requests is heavily tied into f_accessory itself. See acc_ctrlrequest in https://android.googlesource.com/kernel/common/+/android-4.14/drivers/usb/gadget/function/f_accessory.c We have configfs handle the requests in android_setup https://android.googlesource.com/kernel/common/+/android-4.14/drivers/usb/gadget/configfs.c The three requests we care about are ACCESSORY_START, which sends a uevent signal, ACCESSORY_SEND_STRING, which stores strings that an ioctl later retrieves, and ACCESSORY_GET_PROTOCOL, where the device must respond with its protocol version. We'd probably want to change these communication methods into configfs attrs under accessory, which means taking all the other pieces of f_accessory as well. This is kind of weird since the function itself minus control requests is quite easy to change to functionfs. It may take more effort to tidy this up than to implement it generically. > If I understand correctly the purpose of creating a dedicated configuration > is to allow linking the functions to it and this way marking these functions > "special" (not assigned to a configuration proper). yep > While I understand that engineers should discuss facts rather than feelings, > the idea of creating a special configuration doesn't feel right to me, > because this way you are creating a configuration which is not meant to be > ever selected by the host. I can be convinced with arguments, though. The configuration is not just never meant to be selected, the host doesn't even see it in descriptors since it is not in cdev. Using a configuration is convenient because the config group already exists and bind() already takes in a configuration. Otherwise I might store a list of the functions directly in gadget_info instead. > I would like to draw your attention to one fact: a functionfs instance > directory in configfs is intentionally empty, since the functionfs delegates > actual implementation, including descriptors and strings, to userspace. > I would advise to consider (or at least prove wrong) the idea of adding some > attribute (file) to functionfs's configfs directory and using it for marking > this functionfs instance as special. This would require implementing some > exclusion mechanism (if this ffs instance is linked to a configuration, > it is impossible to mark it as special and vice versa). I think the reason functionfs config attrs are empty is because we already have several ways to pass in arguments to functionfs (mount args, descriptor flags) that are already being used. I'm not sure that it's feasible (without changing an interface somewhere) to enable this functionality directly within functionfs. The issue is in order to handle setups a function has to be alloced and bound to a configuration with an underlying cdev. It's theoretically possible to just bind to a cdev, but then you'd need to create new methods in struct usb_function. Also, I think having a list of functions could be useful -- a vendor could have a kernel function handling some control requests, and a functionfs handling all the rest. In that case, they'd just link the functions to control config in order. > Let me just mention that if we had a generic interface we could cover > not only android requirements but also do a great job for people who are > fuzzing USB hosts. Such an interface would allow to easily craft some > random requests without all this noise related to using GadgetFS... My main intention is to handle vendor specific requests (compliant but not standard). This is kind of interesting though and it would not be that hard to allow -- have a bool flag attr control_config_priority and if that is set, give setups to functionfs *before* composite. I can provide patches in either case. I've already started experimenting Thanks, Jerry -- 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
