On Wed, 29 Jun 2011, Tanya Brokhman wrote: > Hi Jassi, > > > > > > > I've started fixing the gadget drivers and removing the above ifdef > > as we > > > discussed but it seems to me that the change is a bit more > > complicated. All > > > gadget drivers provide hs discriptors if gadget_is_dual_speed() > > returns > > > true, in other words - if CONFIG_USB_GADGET_DUALSPEED is defined. So More precisely, all gadget drivers that support high-speed operation do. At present this is all of them, but it doesn't have to be. > > if we > > > remove the #ifdef above we also need to update the gadget drivers to > > provide > > > HS descriptors without stipulating their addition on the > > > gadget_is_dual_speed(). No we don't. At the time a gadget driver is compiled, if it is known that the available UDCs don't support high-speed operation, there's no reason for the gadget driver to include high-speed descriptors. > > > The above seems to me like the complete solution. CONFIG_USB_GADGET_DUALSPEED is present so that gadget drivers can conserve space by omitting descriptors if it is known they will never be used. This has very little to do with max_speed. > > Perhaps gadget drivers should rely on the 'is_dualspeed' flag provided > > by the UDC ? > > You're right. Actually there is a comment in the implementation of gadget_is_dualspeed() that sais that "run time test should check g->is_dualspeed" although right now the return value from that function is according to #ifdef CONFIG_USB_GADGET_DUALSPEED. So what needs to be modified is the gadget_is_dualspeed() function and not the gadget drivers. No, gadget drivers shouldn't need to look at is_dualspeed, except perhaps if they want to avoid sending device-qualifier and other-speed descriptors to the host in situations where the current UDC hardware doesn't support high-speed operation. > At the moment the CONFIG_USB_GADGET_DUALSPEED is defined for each UDC that supports HS, so the code will work. I'm not sure though that each of the existing UDCs update the is_dualspeed flag as they should. So I prefer not to change the implementation of gadget_is_dualspeed() since it requires to verify all the UDCs correct handling of it and it seems to me like a separate commit from what I'm doing now. > > > > > IMHO that is the right thing to do - gadget driver declaring DUALSPEED > > support to host > > without the underlying UDC supporting it, is pointless. It is not pointless. The same gadget driver module can be used with many different UDCs, some of which do support high speed and some of which don't. The gadget driver needs to set max_speed appropriately so that it can work with the UDCs that do support high speed. Alan Stern -- 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
