[PATCH] USB: Gadget library: centralize gadget controller numbers

This patch centralizes the assignment of bcdDevice numbers for different
gadget controllers.  This won't improve the object code at all, but it
does save a lot of repetitive and error-prone source code ... and will
simplify the work of supporting a new controller driver, since most new
gadget drivers will no longer need patches (unless some hardware quirks
limit USB protocol messaging).

Added minor cleanups and identifer hooks for the UDC in the Freescale
iMX series processors.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

drivers/usb/gadget/ether.c

@@ -2181,6 +2181,7 @@ eth_bind (struct usb_gadget *gadget)
 	u8			cdc = 1, zlp = 1, rndis = 1;
 	struct usb_ep		*in_ep, *out_ep, *status_ep = NULL;
 	int			status = -ENOMEM;
+	int			gcnum;
 
 	/* these flags are only ever cleared; compiler take note */
 #ifndef	DEV_CONFIG_CDC
@@ -2194,44 +2195,26 @@ eth_bind (struct usb_gadget *gadget)
 	 * standard protocol is _strongly_ preferred for interop purposes.
 	 * (By everyone except Microsoft.)
 	 */
-	if (gadget_is_net2280 (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0201);
-	} else if (gadget_is_dummy (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0202);
-	} else if (gadget_is_pxa (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0203);
+	if (gadget_is_pxa (gadget)) {
 		/* pxa doesn't support altsettings */
 		cdc = 0;
 	} else if (gadget_is_sh(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0204);
 		/* sh doesn't support multiple interfaces or configs */
 		cdc = 0;
 		rndis = 0;
 	} else if (gadget_is_sa1100 (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0205);
 		/* hardware can't write zlps */
 		zlp = 0;
 		/* sa1100 CAN do CDC, without status endpoint ... we use
 		 * non-CDC to be compatible with ARM Linux-2.4 "usb-eth".
 		 */
 		cdc = 0;
-	} else if (gadget_is_goku (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0206);
-	} else if (gadget_is_mq11xx (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0207);
-	} else if (gadget_is_omap (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0208);
-	} else if (gadget_is_lh7a40x(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0209);
-	} else if (gadget_is_n9604(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0210);
-	} else if (gadget_is_pxa27x(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0211);
-	} else if (gadget_is_s3c2410(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0212);
-	} else if (gadget_is_at91(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0213);
-	} else {
+	}
+
+	gcnum = usb_gadget_controller_number (gadget);
+	if (gcnum >= 0)
+		device_desc.bcdDevice = cpu_to_le16 (0x0200 + gcnum);
+	else {
 		/* can't assume CDC works.  don't want to default to
 		 * anything less functional on CDC-capable hardware,
 		 * so we fail in this case.

drivers/usb/gadget/file_storage.c

@@ -3713,6 +3713,7 @@ static void fsg_unbind(struct usb_gadget *gadget)
 static int __init check_parameters(struct fsg_dev *fsg)
 {
 	int	prot;
+	int	gcnum;
 
 	/* Store the default values */
 	mod_data.transport_type = USB_PR_BULK;
@@ -3724,33 +3725,13 @@ static int __init check_parameters(struct fsg_dev *fsg)
 		mod_data.can_stall = 0;
 
 	if (mod_data.release == 0xffff) {	// Parameter wasn't set
-		if (gadget_is_net2280(fsg->gadget))
-			mod_data.release = 0x0301;
-		else if (gadget_is_dummy(fsg->gadget))
-			mod_data.release = 0x0302;
-		else if (gadget_is_pxa(fsg->gadget))
-			mod_data.release = 0x0303;
-		else if (gadget_is_sh(fsg->gadget))
-			mod_data.release = 0x0304;
-
 		/* The sa1100 controller is not supported */
-
-		else if (gadget_is_goku(fsg->gadget))
-			mod_data.release = 0x0306;
-		else if (gadget_is_mq11xx(fsg->gadget))
-			mod_data.release = 0x0307;
-		else if (gadget_is_omap(fsg->gadget))
-			mod_data.release = 0x0308;
-		else if (gadget_is_lh7a40x(fsg->gadget))
-			mod_data.release = 0x0309;
-		else if (gadget_is_n9604(fsg->gadget))
-			mod_data.release = 0x0310;
-		else if (gadget_is_pxa27x(fsg->gadget))
-			mod_data.release = 0x0311;
-		else if (gadget_is_s3c2410(gadget))
-			mod_data.release = 0x0312;
-		else if (gadget_is_at91(fsg->gadget))
-			mod_data.release = 0x0313;
+		if (gadget_is_sa1100(fsg->gadget))
+			gcnum = -1;
+		else
+			gcnum = usb_gadget_controller_number(fsg->gadget);
+		if (gcnum >= 0)
+			mod_data.release = 0x0300 + gcnum;
 		else {
 			WARN(fsg, "controller '%s' not recognized\n",
 				fsg->gadget->name);

drivers/usb/gadget/gadget_chips.h

@@ -5,6 +5,7 @@
  *
  * This could eventually work like the ARM mach_is_*() stuff, driven by
  * some config file that gets updated as new hardware is supported.
+ * (And avoiding the runtime comparisons in typical one-choice cases.)
  *
  * NOTE:  some of these controller drivers may not be available yet.
  */
@@ -86,7 +87,61 @@
 #define gadget_is_at91(g)	0
 #endif
 
+#ifdef CONFIG_USB_GADGET_IMX
+#define gadget_is_imx(g)	!strcmp("imx_udc", (g)->name)
+#else
+#define gadget_is_imx(g)	0
+#endif
+
 // CONFIG_USB_GADGET_SX2
 // CONFIG_USB_GADGET_AU1X00
 // ...
 
+
+/**
+ * usb_gadget_controller_number - support bcdDevice id convention
+ * @gadget: the controller being driven
+ *
+ * Return a 2-digit BCD value associated with the peripheral controller,
+ * suitable for use as part of a bcdDevice value, or a negative error code.
+ *
+ * NOTE:  this convention is purely optional, and has no meaning in terms of
+ * any USB specification.  If you want to use a different convention in your
+ * gadget driver firmware -- maybe a more formal revision ID -- feel free.
+ *
+ * Hosts see these bcdDevice numbers, and are allowed (but not encouraged!)
+ * to change their behavior accordingly.  For example it might help avoiding
+ * some chip bug.
+ */
+static inline int usb_gadget_controller_number(struct usb_gadget *gadget)
+{
+	if (gadget_is_net2280(gadget))
+		return 0x01;
+	else if (gadget_is_dummy(gadget))
+		return 0x02;
+	else if (gadget_is_pxa(gadget))
+		return 0x03;
+	else if (gadget_is_sh(gadget))
+		return 0x04;
+	else if (gadget_is_sa1100(gadget))
+		return 0x05;
+	else if (gadget_is_goku(gadget))
+		return 0x06;
+	else if (gadget_is_mq11xx(gadget))
+		return 0x07;
+	else if (gadget_is_omap(gadget))
+		return 0x08;
+	else if (gadget_is_lh7a40x(gadget))
+		return 0x09;
+	else if (gadget_is_n9604(gadget))
+		return 0x10;
+	else if (gadget_is_pxa27x(gadget))
+		return 0x11;
+	else if (gadget_is_s3c2410(gadget))
+		return 0x12;
+	else if (gadget_is_at91(gadget))
+		return 0x13;
+	else if (gadget_is_imx(gadget))
+		return 0x14;
+	return -ENOENT;
+}

drivers/usb/gadget/serial.c

@@ -1422,49 +1422,20 @@ static int gs_bind(struct usb_gadget *gadget)
 	int ret;
 	struct usb_ep *ep;
 	struct gs_dev *dev;
+	int gcnum;
 
-	/* device specific */
-	if (gadget_is_net2280(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0001);
-	} else if (gadget_is_pxa(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0002);
-	} else if (gadget_is_sh(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0003);
-		/* sh doesn't support multiple interfaces or configs */
+	/* Some controllers can't support CDC ACM:
+	 * - sh doesn't support multiple interfaces or configs;
+	 * - sa1100 doesn't have a third interrupt endpoint
+	 */
+	if (gadget_is_sh(gadget) || gadget_is_sa1100(gadget))
 		use_acm = 0;
-	} else if (gadget_is_sa1100(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0004);
-		/* sa1100 doesn't support necessary endpoints */
-		use_acm = 0;
-	} else if (gadget_is_goku(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0005);
-	} else if (gadget_is_mq11xx(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0006);
-	} else if (gadget_is_omap(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0007);
-	} else if (gadget_is_lh7a40x(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0008);
-	} else if (gadget_is_n9604(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0009);
-	} else if (gadget_is_pxa27x(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0011);
-	} else if (gadget_is_s3c2410(gadget)) {
-		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0012);
-	} else if (gadget_is_at91(gadget)) {
+
+	gcnum = usb_gadget_controller_number(gadget);
+	if (gcnum >= 0)
 		gs_device_desc.bcdDevice =
-			__constant_cpu_to_le16(GS_VERSION_NUM|0x0013);
-	} else {
+				cpu_to_le16(GS_VERSION_NUM | gcnum);
+	else {
 		printk(KERN_WARNING "gs_bind: controller '%s' not recognized\n",
 			gadget->name);
 		/* unrecognized, but safe unless bulk is REALLY quirky */

drivers/usb/gadget/zero.c

@@ -1139,6 +1139,13 @@ zero_bind (struct usb_gadget *gadget)
 {
 	struct zero_dev		*dev;
 	struct usb_ep		*ep;
+	int			gcnum;
+
+	/* FIXME this can't yet work right with SH ... it has only
+	 * one configuration, numbered one.
+	 */
+	if (gadget_is_sh(gadget))
+		return -ENODEV;
 
 	/* Bulk-only drivers like this one SHOULD be able to
 	 * autoconfigure on any sane usb controller driver,
@@ -1161,43 +1168,10 @@ autoconf_fail:
 	EP_OUT_NAME = ep->name;
 	ep->driver_data = ep;	/* claim */
 
-
-	/*
-	 * DRIVER POLICY CHOICE:  you may want to do this differently.
-	 * One thing to avoid is reusing a bcdDevice revision code
-	 * with different host-visible configurations or behavior
-	 * restrictions -- using ep1in/ep2out vs ep1out/ep3in, etc
-	 */
-	if (gadget_is_net2280 (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0201);
-	} else if (gadget_is_pxa (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0203);
-#if 0
-	} else if (gadget_is_sh(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0204);
-		/* SH has only one configuration; see "loopdefault" */
-		device_desc.bNumConfigurations = 1;
-		/* FIXME make 1 == default.bConfigurationValue */
-#endif
-	} else if (gadget_is_sa1100 (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0205);
-	} else if (gadget_is_goku (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0206);
-	} else if (gadget_is_mq11xx (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0207);
-	} else if (gadget_is_omap (gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0208);
-	} else if (gadget_is_lh7a40x(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0209);
-	} else if (gadget_is_n9604(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0210);
-	} else if (gadget_is_pxa27x(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0211);
-	} else if (gadget_is_s3c2410(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0212);
-	} else if (gadget_is_at91(gadget)) {
-		device_desc.bcdDevice = __constant_cpu_to_le16 (0x0213);
-	} else {
+	gcnum = usb_gadget_controller_number (gadget);
+	if (gcnum >= 0)
+		device_desc.bcdDevice = cpu_to_le16 (0x0200 + gcnum);
+	else {
 		/* gadget zero is so simple (for now, no altsettings) that
 		 * it SHOULD NOT have problems with bulk-capable hardware.
 		 * so warn about unrcognized controllers, don't panic.