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
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
