Hi, Binyamin Sharet <bsharet@xxxxxxxxx> writes: >>>>>>> I think this will cause existing implementation over gadgetfs to fail >>>>>>> with this >>>>>>> special kernel (as now it will delegate everything all of the time). How >>>>>>> about >>>>>>> using a ioctl to configure it, but wrapping this ioctl with Kconfig? >>>>>>> This way >>>>>>> gadgetfs will operate as always unless a user activates "delegate all" >>>>>>> in runtime. >>>>>>> >>>>>>> Sounds reasonable? >>>>>> You mean a ep0 ioctl? Makes sense to me. Then we can add more features >>>>>> later. Perhaps just add a "Get supported features" IOCTL which returns >>>>>> a struct with 256 bits (4x uint64_t). I doubt we will ever need that >>>>>> many bits, but better safe than sorry, I guess. >>>>>> >>>>> Don't we need some "set feature" IOCTL as well? >>>> Why? Features are enabled at kernel compile time. Userspace issues ioctl >>>> to ask the kernel "which features do you have enabled for >>>> GadgetFS?". Note that we have a backwards compatibility problem here. If >>>> "which features do you have enabled for GadgetFS" ioctl fails, we will >>>> assume that kernel is older than the first kernel where this ioctl was >>>> added. >>>> >>>> To help userland a little more, we can make sure that, from now on, the >>>> ioctl itself is always enabled and bit0 out of the 256 bits we have MUST >>>> always be set. This means that "Forward all requests to userspace" >>>> feature has to be, at least, bit1. >>>> >>>> This mimics what USB descriptors where bit7 of config descriptor's >>>> bmAttributes must always be set. We're just using bit0 instead. >>>> >>> What I mean is - when the gadgetfs compiled with "Forward all requests to >>> userspace" feature, it should behave as before until an ep0 ioctl is issued >>> to actually enable this feature (e.g. forward the messages) otherwise, this >>> version of gadgetfs will break any current userspace application that uses >>> it. >>> Am I missing something? >> No, you're not. I misunderstood what you meant ;-) Yeah, I agree, we >> also need an IOCTL to enable specific features after discovering they >> exist. >> > > Took me some time to get to it. > > Do we need to wrap the ioctl with a Kconfig option if it is disabled > by default and can only be enabled via an ioctl? yes. This is not the type of thing we want e.g. android systems to support. Even if disabled by default. We don't want it to EVER be enabled as it could break USB communication. Imagine a bug in adb ends up mistakenly enabling this during FW flashing :-) -- balbi
Attachment:
signature.asc
Description: PGP signature
