On Wed, Jun 29, 2011 at 2:27 PM, Tanya Brokhman <tlinder@xxxxxxxxxxxxxx> wrote: > Hi Alan > >> > > @@ -165,6 +165,11 @@ static struct usb_composite_driver >> audio_driver = { >> > > .name = "g_audio", >> > > .dev = &device_desc, >> > > .strings = audio_strings, >> > > +#ifdef CONFIG_USB_GADGET_DUALSPEED >> > > + .max_speed = USB_SPEED_HIGH, >> > > +#else >> > > + .max_speed = USB_SPEED_FULL, >> > > +#endif /* CONFIG_USB_GADGET_DUALSPEED */ >> > >> > Really? >> > >> > Come on, this is a mess, we don't want #ifdefs in .c files for a >> reason, >> > and this is an example of that. >> >> This was discussed in a recent email exchange. Those #ifdefs are >> completely unnecessary: max_speed is supposed to be the maximum speed >> supported by the driver, regardless of what the hardware allows. >> >> In this example, the code should simply say: >> >> .max_speed = USB_SPEED_HIGH, >> >> The other drivers should be handled similarly. >> > > 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 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(). > The above seems to me like the complete solution. Perhaps gadget drivers should rely on the 'is_dualspeed' flag provided by the UDC ? IMHO that is the right thing to do - gadget driver declaring DUALSPEED support to host without the underlying UDC supporting it, is pointless. And the gadget driver should (must ?) support both speeds if UDC does so too. Though that might mean some modifications to the gadget stack! Cheers! -jassi -- 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