On Mon, 19 Mar 2007, Felipe Balbi wrote: > This patch adds some ifdefs to make the file-storage gadget work > as a usb_function module.
This is really ugly. You have added _lots_ of changes like this: > @@ -1127,3 +1150,8 @@ static void fsg_disconnect(struct usb_ga > { > +#ifdef USB_COMPOSITE_DEVICE > + struct usb_composite_dev *cdev = get_gadget_data(gadget); > + struct fsg_dev *fsg = get_composite_data(cdev); > +#else > struct fsg_dev *fsg = get_gadget_data(gadget); > +#endif Why not instead set things up in a header file so that when USB_COMPOSITE_DEVICE isn't defined, we have #define get_composite_data(cdev) cdev Then just do struct fsg_dev *fsg = get_composite_data(get_gadget_data(gadget)); You also have lots of changes like this: > @@ -1303,2 +1339,6 @@ static int class_setup_req(struct fsg_de > > +#ifdef USB_COMPOSITE_DEVICE > + w_index -= cdev->current_func->interface_shift; > +#endif /* USB_COMPOSITE_DEVICE */ If you would simply add an "interface_shift" field to struct fsg_dev (normally 0), then this could simply be: w_index -= fsg->interface_shift; Better yet, define a macro like this: #ifdef USB_COMPOSITE_DEVICE #define ADJUST_INTF(i) do {i -= fsg->interface_shift;} while (0) #else #define ADJUST_INTF(i) do {} while (0) #endif Together these two changes would remove over half of your giant patch. Alan Stern ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel