> > > --- On Sun, 10/3/10, tlinder <tlinder@xxxxxxxxxxxxxx> wrote: > >> In order not to force all the FDs to supply > > What do File Descriptors have to do with this? > > If you don't mean FD == File Descriptor, then > please spell out what you do mean, instead of > trying to repurpose a widely used abbreviation. By FD we meant Function Drivers. We'll update the patch description. > > > SuperSpeed >> descriptors when >> operating in SuperSpeed mode the following approach was >> taken: >> If we're operating in SuperSpeed mode and the FD didn't >> supply SuperSpeed >> descriptors, the composite layer will automatically create >> SuperSpeed >> descriptors with default values. > > That bothers me in two ways. First, I want to > see a solution that maintains today's policy where > the composite framework is optional for all gadget > drivers. This policy is maintained. Gadget drivers are not required to use the composite framework in order to work in SuperSpeed mode. If a certain driver wishes to do so, without using the composite framework, it should add definitions of SuperSpeed descriptors and support of new SuperSpeed features. If a driver wishes to work in High/Low speeds then no changes are needed. Please note that this change is backward compatible. In order to test a non composite gadget driver we used the File storage. We managed to load the file_storage gadget driver (while the SuperSpeed gadget flag is enabled) and the File Storage successfully registered as a high speed driver and enumerated with a super speed host. We also tested a composite gadget driver (mass storage) that defines the descriptors and doesn't use the automatic and it worked successfully. All the other gadget drivers were also tested using the automatic composite framework and managed to enumerate. > > Second, that kind of automagic bothers me. > > What could be wrong with expecting gadget > drivers to provide all the descriptors they need, > instead of introducing automagic? In the current implementation a gadget driver can still provide his own SuperSpeed descriptors as it provides HighSpeed descriptors and not use the automation. Implementing the automatic creation of the SuperSpeed descriptors approach has several benefits: 1. All existing gadget drivers (that use the composite framework) are operational in SuperSpeed mode without any modifications to their code 2. Code duplication, of the additional SuperSpeed descriptors in each existing gadget driver, is spared: A gadget driver that doesn't wish to exploit the SuperSpeed capabilities (such as streaming for example) but wishes to operate in a SuperSpeed mode is not required to provide additional SuperSpeed descriptors with default values (meaning, that none of the SuperSpeed capabilities are supported by it). Such will be created for it automatically. 2. Single snippet of code tested for all existing gadget drivers > > >> Support for new SuperSpeed BOS descriptor was > > Wireless USB BOS descriptors exist too, yes? Does > this approach cover them, or just SuperSpeed? (We > may someday want to support Wireless USB on the > peripheral/gadget side too... In the current implementation the composite framework supports GetDescriptor(BOS) request only if the gadget driver supports SuperSpeed. In order to extend this functionality for wireless USB one should add another condition (is_wireless_USB()) to the existing if(). > > > >> +++++++++++++++++++++++++++++++++++++--- >> include/linux/usb/ch9.h > > I like to see patches related to USB-IF formats > and protocols be separate from functional changes > in the USB stack or its drivers. which may rely > on those formats/protocols; less entanglement. We will divide this patch into two patches. > >> + > >> +config USB_GADGET_SUPERSPEED >> + boolean "Gadget opperating in Super >> Speed" >> + depends on USB_GADGET >> + depends on USB_GADGET_DUALSPEED >> + default n >> + help >> + Enabling this feature enables >> Super Speed support in the Gadget >> + driver. It means that gadget >> drivers should include extra (SuperSpeed) >> + descriptors. > > That is: the automagic isn't needed. The > concepts in this patch seem to be a bit on > the self-contradictory side... You are correct. We should change the comment as follows; "...It means that gadget drivers should provide extra (SuperSpeed) descriptors to the host." > > ep_comp_desc = { >> + .bDescriptorType = >> USB_DT_SS_ENDPOINT_COMP, >> + .bLength = 0x06, >> + .bMaxBurst = 0, >> /*the default is we don't support bursting*/ > > I've not followed the SuperSpeed stuff as closely > as I might, but ... doesn't bursting require some > hardware support? So that not all UDC + driver > stacks can support it? (That'd be a case, if so, > for more sanity checks ... and the gadget driver to > explicitly say if it handles bursting. You're right. Bursting and streaming functionality support is defined by the UDC and not the gadget framework. Due to the above, the create_ss_descriptors() is used for providing default SuperSpeed descriptors, meaning that streaming and bursting is not supported. > > > > -- 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