[PATCH] USB: ethernet gadget updates (mostly cleanup)

Some cleanup for the the Ethernet part of the Ethernet/RNDIS gadget driver:

  - Remove remnants of ancient endpoint init logic; this is simpler, clearer

  - Save a smidgeon of space in the object file

  - Get rid of some #ifdeffery, mostly by using some newish inlines

  - Reset more driver state as part of USB reset

  - Remove a needless wrapper around an RNDIS call

  - Improve and comment the status interrupt handling:
      * RNDIS sometimes needs to queue these transfers (rarely in normal
        cases, but reproducibly while Windows was deadlocking its USB stack)
      * Mark requests as busy/not

  - Enable the SET_NETDEV_DEV() call; sysfs seems to behave sanely now

This is a net shrink of source code.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

1 files changed, 107 insertions, 180 deletions

diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index a766c29c3ec9..3830a0a0fd50 100644
--- a/drivers/usb/gadget/ether.c
+++ b/drivers/usb/gadget/ether.c
@@ -84,7 +84,7 @@
  */
 
 #define DRIVER_DESC		"Ethernet Gadget"
-#define DRIVER_VERSION		"Equinox 2004"
+#define DRIVER_VERSION		"May Day 2005"
 
 static const char shortname [] = "ether";
 static const char driver_desc [] = DRIVER_DESC;
@@ -141,9 +141,6 @@ struct eth_dev {
  * It also ASSUMES a self-powered device, without remote wakeup,
  * although remote wakeup support would make sense.
  */
-static const char *EP_IN_NAME;
-static const char *EP_OUT_NAME;
-static const char *EP_STATUS_NAME;
 
 /*-------------------------------------------------------------------------*/
 
@@ -313,6 +310,7 @@ static inline int rndis_active(struct eth_dev *dev)
 #define	FS_BPS		(19 *  64 * 1 * 1000 * 8)
 
 #ifdef CONFIG_USB_GADGET_DUALSPEED
+#define	DEVSPEED	USB_SPEED_HIGH
 
 static unsigned qmult = 5;
 module_param (qmult, uint, S_IRUGO|S_IWUSR);
@@ -331,6 +329,8 @@ static inline int BITRATE(struct usb_gadget *g)
 }
 
 #else	/* full speed (low speed doesn't do bulk) */
+#define	DEVSPEED	USB_SPEED_FULL
+
 #define qlen(gadget) DEFAULT_QLEN
 
 static inline int BITRATE(struct usb_gadget *g)
@@ -540,7 +540,7 @@ static const struct usb_cdc_call_mgmt_descriptor call_mgmt_descriptor = {
 	.bDataInterface = 	0x01,
 };
 
-static struct usb_cdc_acm_descriptor acm_descriptor = {
+static const struct usb_cdc_acm_descriptor acm_descriptor = {
 	.bLength =  		sizeof acm_descriptor,
 	.bDescriptorType = 	USB_DT_CS_INTERFACE,
 	.bDescriptorSubType = 	USB_CDC_ACM_TYPE,
@@ -848,7 +848,7 @@ static const struct usb_descriptor_header *hs_rndis_function [] = {
 #else
 
 /* if there's no high speed support, maxpacket doesn't change. */
-#define ep_desc(g,hs,fs) fs
+#define ep_desc(g,hs,fs) (((void)(g)), (fs))
 
 static inline void __init hs_subset_descriptors(void)
 {
@@ -948,10 +948,31 @@ config_buf (enum usb_device_speed speed,
 static void eth_start (struct eth_dev *dev, int gfp_flags);
 static int alloc_requests (struct eth_dev *dev, unsigned n, int gfp_flags);
 
-#ifdef	DEV_CONFIG_CDC
-static inline int ether_alt_ep_setup (struct eth_dev *dev, struct usb_ep *ep)
+static int
+set_ether_config (struct eth_dev *dev, int gfp_flags)
 {
-	const struct usb_endpoint_descriptor	*d;
+	int					result = 0;
+	struct usb_gadget			*gadget = dev->gadget;
+
+	/* status endpoint used for RNDIS and (optionally) CDC */
+	if (!subset_active(dev) && dev->status_ep) {
+		dev->status = ep_desc (gadget, &hs_status_desc,
+						&fs_status_desc);
+		dev->status_ep->driver_data = dev;
+
+		result = usb_ep_enable (dev->status_ep, dev->status);
+		if (result != 0) {
+			DEBUG (dev, "enable %s --> %d\n", 
+				dev->status_ep->name, result);
+			goto done;
+		}
+	}
+
+	dev->in = ep_desc (dev->gadget, &hs_source_desc, &fs_source_desc);
+	dev->in_ep->driver_data = dev;
+
+	dev->out = ep_desc (dev->gadget, &hs_sink_desc, &fs_sink_desc);
+	dev->out_ep->driver_data = dev;
 
 	/* With CDC,  the host isn't allowed to use these two data
 	 * endpoints in the default altsetting for the interface.
@@ -961,135 +982,33 @@ static inline int ether_alt_ep_setup (struct eth_dev *dev, struct usb_ep *ep)
 	 * a side effect of setting a packet filter.  Deactivation is
 	 * from REMOTE_NDIS_HALT_MSG, reset from REMOTE_NDIS_RESET_MSG.
 	 */
-
-	/* one endpoint writes data back IN to the host */
-	if (strcmp (ep->name, EP_IN_NAME) == 0) {
-		d = ep_desc (dev->gadget, &hs_source_desc, &fs_source_desc);
-		ep->driver_data = dev;
-		dev->in = d;
-
-	/* one endpoint just reads OUT packets */
-	} else if (strcmp (ep->name, EP_OUT_NAME) == 0) {
-		d = ep_desc (dev->gadget, &hs_sink_desc, &fs_sink_desc);
-		ep->driver_data = dev;
-		dev->out = d;
-
-	/* optional status/notification endpoint */
-	} else if (EP_STATUS_NAME &&
-			strcmp (ep->name, EP_STATUS_NAME) == 0) {
-		int			result;
-
-		d = ep_desc (dev->gadget, &hs_status_desc, &fs_status_desc);
-		result = usb_ep_enable (ep, d);
-		if (result < 0)
-			return result;
-
-		ep->driver_data = dev;
-		dev->status = d;
-	}
-	return 0;
-}
-#endif
-
-#if	defined(DEV_CONFIG_SUBSET) || defined(CONFIG_USB_ETH_RNDIS)
-static inline int ether_ep_setup (struct eth_dev *dev, struct usb_ep *ep)
-{
-	int					result;
-	const struct usb_endpoint_descriptor	*d;
-
-	/* CDC subset is simpler:  if the device is there,
-	 * it's live with rx and tx endpoints.
-	 *
-	 * Do this as a shortcut for RNDIS too.
-	 */
-
-	/* one endpoint writes data back IN to the host */
-	if (strcmp (ep->name, EP_IN_NAME) == 0) {
-		d = ep_desc (dev->gadget, &hs_source_desc, &fs_source_desc);
-		result = usb_ep_enable (ep, d);
-		if (result < 0)
-			return result;
-
-		ep->driver_data = dev;
-		dev->in = d;
-
-	/* one endpoint just reads OUT packets */
-	} else if (strcmp (ep->name, EP_OUT_NAME) == 0) {
-		d = ep_desc (dev->gadget, &hs_sink_desc, &fs_sink_desc);
-		result = usb_ep_enable (ep, d);
-		if (result < 0)
-			return result;
-
-		ep->driver_data = dev;
-		dev->out = d;
-	}
-
-	return 0;
-}
-#endif
-
-static int
-set_ether_config (struct eth_dev *dev, int gfp_flags)
-{
-	int			result = 0;
-	struct usb_ep		*ep;
-	struct usb_gadget	*gadget = dev->gadget;
-
-	gadget_for_each_ep (ep, gadget) {
-#ifdef	DEV_CONFIG_CDC
-		if (!dev->rndis && dev->cdc) {
-			result = ether_alt_ep_setup (dev, ep);
-			if (result == 0)
-				continue;
+	if (!cdc_active(dev)) {
+		result = usb_ep_enable (dev->in_ep, dev->in);
+		if (result != 0) {
+			DEBUG(dev, "enable %s --> %d\n", 
+				dev->in_ep->name, result);
+			goto done;
 		}
-#endif
 
-#ifdef	CONFIG_USB_ETH_RNDIS
-		if (dev->rndis && strcmp (ep->name, EP_STATUS_NAME) == 0) {
-			const struct usb_endpoint_descriptor	*d;
-			d = ep_desc (gadget, &hs_status_desc, &fs_status_desc);
-			result = usb_ep_enable (ep, d);
-			if (result == 0) {
-				ep->driver_data = dev;
-				dev->status = d;
-				continue;
-			}
-		} else
-#endif
-
-		{
-#if	defined(DEV_CONFIG_SUBSET) || defined(CONFIG_USB_ETH_RNDIS)
-			result = ether_ep_setup (dev, ep);
-			if (result == 0)
-				continue;
-#endif
+		result = usb_ep_enable (dev->out_ep, dev->out);
+		if (result != 0) {
+			DEBUG (dev, "enable %s --> %d\n", 
+				dev->in_ep->name, result);
+			goto done;
 		}
-
-		/* stop on error */
-		ERROR (dev, "can't enable %s, result %d\n", ep->name, result);
-		break;
 	}
-	if (!result && (!dev->in_ep || !dev->out_ep))
-		result = -ENODEV;
 
+done:
 	if (result == 0)
 		result = alloc_requests (dev, qlen (gadget), gfp_flags);
 
 	/* on error, disable any endpoints  */
 	if (result < 0) {
-#if defined(DEV_CONFIG_CDC) || defined(CONFIG_USB_ETH_RNDIS)
-		if (dev->status)
+		if (!subset_active(dev))
 			(void) usb_ep_disable (dev->status_ep);
-#endif
 		dev->status = NULL;
-#if defined(DEV_CONFIG_SUBSET) || defined(CONFIG_USB_ETH_RNDIS)
-		if (dev->rndis || !dev->cdc) {
-			if (dev->in)
-				(void) usb_ep_disable (dev->in_ep);
-			if (dev->out)
-				(void) usb_ep_disable (dev->out_ep);
-		}
-#endif
+		(void) usb_ep_disable (dev->in_ep);
+		(void) usb_ep_disable (dev->out_ep);
 		dev->in = NULL;
 		dev->out = NULL;
 	} else
@@ -1097,8 +1016,7 @@ set_ether_config (struct eth_dev *dev, int gfp_flags)
 	/* activate non-CDC configs right away
 	 * this isn't strictly according to the RNDIS spec
 	 */
-#if defined(DEV_CONFIG_SUBSET) || defined(CONFIG_USB_ETH_RNDIS)
-	if (dev->rndis || !dev->cdc) {
+	if (!cdc_active (dev)) {
 		netif_carrier_on (dev->net);
 		if (netif_running (dev->net)) {
 			spin_unlock (&dev->lock);
@@ -1106,7 +1024,6 @@ set_ether_config (struct eth_dev *dev, int gfp_flags)
 			spin_lock (&dev->lock);
 		}
 	}
-#endif
 
 	if (result == 0)
 		DEBUG (dev, "qlen %d\n", qlen (gadget));
@@ -1153,6 +1070,8 @@ static void eth_reset_config (struct eth_dev *dev)
 	if (dev->status) {
 		usb_ep_disable (dev->status_ep);
 	}
+	dev->rndis = 0;
+	dev->cdc_filter = 0;
 	dev->config = 0;
 }
 
@@ -1165,9 +1084,6 @@ eth_set_config (struct eth_dev *dev, unsigned number, int gfp_flags)
 	int			result = 0;
 	struct usb_gadget	*gadget = dev->gadget;
 
-	if (number == dev->config)
-		return 0;
-
 	if (gadget_is_sa1100 (gadget)
 			&& dev->config
 			&& atomic_read (&dev->tx_qlen) != 0) {
@@ -1177,12 +1093,8 @@ eth_set_config (struct eth_dev *dev, unsigned number, int gfp_flags)
 	}
 	eth_reset_config (dev);
 
-	/* default:  pass all packets, no multicast filtering */
-	dev->cdc_filter = DEFAULT_FILTER;
-
 	switch (number) {
 	case DEV_CONFIG_VALUE:
-		dev->rndis = 0;
 		result = set_ether_config (dev, gfp_flags);
 		break;
 #ifdef	CONFIG_USB_ETH_RNDIS
@@ -1234,6 +1146,13 @@ eth_set_config (struct eth_dev *dev, unsigned number, int gfp_flags)
 
 #ifdef	DEV_CONFIG_CDC
 
+/* The interrupt endpoint is used in CDC networking models (Ethernet, ATM)
+ * only to notify the host about link status changes (which we support) or
+ * report completion of some encapsulated command (as used in RNDIS).  Since
+ * we want this CDC Ethernet code to be vendor-neutral, we don't use that
+ * command mechanism; and only one status request is ever queued.
+ */
+
 static void eth_status_complete (struct usb_ep *ep, struct usb_request *req)
 {
 	struct usb_cdc_notification	*event = req->buf;
@@ -1262,7 +1181,7 @@ static void eth_status_complete (struct usb_ep *ep, struct usb_request *req)
 	} else if (value != -ECONNRESET)
 		DEBUG (dev, "event %02x --> %d\n",
 			event->bNotificationType, value);
-	event->bmRequestType = 0xff;
+	req->context = NULL;
 }
 
 static void issue_start_status (struct eth_dev *dev)
@@ -1279,6 +1198,8 @@ static void issue_start_status (struct eth_dev *dev)
 	 * a "cancel the whole queue" primitive since any
 	 * unlink-one primitive has way too many error modes.
 	 * here, we "know" toggle is already clear...
+	 *
+	 * FIXME iff req->context != null just dequeue it
 	 */
 	usb_ep_disable (dev->status_ep);
 	usb_ep_enable (dev->status_ep, dev->status);
@@ -1295,6 +1216,8 @@ static void issue_start_status (struct eth_dev *dev)
 
 	req->length = sizeof *event;
 	req->complete = eth_status_complete;
+	req->context = dev;
+
 	value = usb_ep_queue (dev->status_ep, req, GFP_ATOMIC);
 	if (value < 0)
 		DEBUG (dev, "status buf queue --> %d\n", value);
@@ -1459,9 +1382,11 @@ eth_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 
 			/* CDC requires the data transfers not be done from
 			 * the default interface setting ... also, setting
-			 * the non-default interface clears filters etc.
+			 * the non-default interface resets filters etc.
 			 */
 			if (wValue == 1) {
+				if (!cdc_active (dev))
+					break;
 				usb_ep_enable (dev->in_ep, dev->in);
 				usb_ep_enable (dev->out_ep, dev->out);
 				dev->cdc_filter = DEFAULT_FILTER;
@@ -1691,10 +1616,8 @@ rx_submit (struct eth_dev *dev, struct usb_request *req, int gfp_flags)
 	 */
 	size = (sizeof (struct ethhdr) + dev->net->mtu + RX_EXTRA);
 	size += dev->out_ep->maxpacket - 1;
-#ifdef CONFIG_USB_ETH_RNDIS
-	if (dev->rndis)
+	if (rndis_active(dev))
 		size += sizeof (struct rndis_packet_msg_type);
-#endif	
 	size -= size % dev->out_ep->maxpacket;
 
 	if ((skb = alloc_skb (size + NET_IP_ALIGN, gfp_flags)) == 0) {
@@ -1862,8 +1785,6 @@ static void rx_fill (struct eth_dev *dev, int gfp_flags)
 	struct usb_request	*req;
 	unsigned long		flags;
 
-	clear_bit (WORK_RX_MEMORY, &dev->todo);
-
 	/* fill unused rxq slots with some skb */
 	spin_lock_irqsave (&dev->lock, flags);
 	while (!list_empty (&dev->rx_reqs)) {
@@ -1886,11 +1807,9 @@ static void eth_work (void *_dev)
 {
 	struct eth_dev		*dev = _dev;
 
-	if (test_bit (WORK_RX_MEMORY, &dev->todo)) {
+	if (test_and_clear_bit (WORK_RX_MEMORY, &dev->todo)) {
 		if (netif_running (dev->net))
 			rx_fill (dev, GFP_KERNEL);
-		else
-			clear_bit (WORK_RX_MEMORY, &dev->todo);
 	}
 
 	if (dev->todo)
@@ -2039,27 +1958,31 @@ drop:
 
 #ifdef CONFIG_USB_ETH_RNDIS
 
-static void rndis_send_media_state (struct eth_dev *dev, int connect)
-{
-	if (!dev)
-		return;
-	
-	if (connect) {
-		if (rndis_signal_connect (dev->rndis_config))
-			return;
-	} else {
-		if (rndis_signal_disconnect (dev->rndis_config))
-			return;
-	}
-}
+/* The interrupt endpoint is used in RNDIS to notify the host when messages
+ * other than data packets are available ... notably the REMOTE_NDIS_*_CMPLT
+ * messages, but also REMOTE_NDIS_INDICATE_STATUS_MSG and potentially even
+ * REMOTE_NDIS_KEEPALIVE_MSG.
+ *
+ * The RNDIS control queue is processed by GET_ENCAPSULATED_RESPONSE, and
+ * normally just one notification will be queued.
+ */
+
+static struct usb_request *eth_req_alloc (struct usb_ep *, unsigned, unsigned);
+static void eth_req_free (struct usb_ep *ep, struct usb_request *req);
 
 static void
 rndis_control_ack_complete (struct usb_ep *ep, struct usb_request *req)
 {
+	struct eth_dev          *dev = ep->driver_data;
+
 	if (req->status || req->actual != req->length)
-		DEBUG ((struct eth_dev *) ep->driver_data,
+		DEBUG (dev,
 			"rndis control ack complete --> %d, %d/%d\n",
 			req->status, req->actual, req->length);
+	req->context = NULL;
+
+	if (req != dev->stat_req)
+		eth_req_free(ep, req);
 }
 
 static int rndis_control_ack (struct net_device *net)
@@ -2074,11 +1997,19 @@ static int rndis_control_ack (struct net_device *net)
 		return -ENODEV;
 	}
 
+	/* in case queue length > 1 */
+	if (resp->context) {
+		resp = eth_req_alloc (dev->status_ep, 8, GFP_ATOMIC);
+		if (!resp)
+			return -ENOMEM;
+	}
+
 	/* Send RNDIS RESPONSE_AVAILABLE notification;
 	 * USB_CDC_NOTIFY_RESPONSE_AVAILABLE should work too
 	 */
 	resp->length = 8;
 	resp->complete = rndis_control_ack_complete;
+	resp->context = dev;
 	
 	*((__le32 *) resp->buf) = __constant_cpu_to_le32 (1);
 	*((__le32 *) resp->buf + 1) = __constant_cpu_to_le32 (0);
@@ -2109,7 +2040,7 @@ static void eth_start (struct eth_dev *dev, int gfp_flags)
 		rndis_set_param_medium (dev->rndis_config,
 					NDIS_MEDIUM_802_3,
 					BITRATE(dev->gadget)/100);
-		rndis_send_media_state (dev, 1);
+		(void) rndis_signal_connect (dev->rndis_config);
 	}
 #endif	
 }
@@ -2156,7 +2087,7 @@ static int eth_stop (struct net_device *net)
 	if (dev->rndis) {
 		rndis_set_param_medium (dev->rndis_config,
 					NDIS_MEDIUM_802_3, 0);
-		rndis_send_media_state (dev, 0);
+		(void) rndis_signal_disconnect (dev->rndis_config);
 	}
 #endif
 
@@ -2165,15 +2096,16 @@ static int eth_stop (struct net_device *net)
 
 /*-------------------------------------------------------------------------*/
 
-static struct usb_request *eth_req_alloc (struct usb_ep *ep, unsigned size)
+static struct usb_request *
+eth_req_alloc (struct usb_ep *ep, unsigned size, unsigned gfp_flags)
 {
 	struct usb_request	*req;
 
-	req = usb_ep_alloc_request (ep, GFP_KERNEL);
+	req = usb_ep_alloc_request (ep, gfp_flags);
 	if (!req)
 		return NULL;
 
-	req->buf = kmalloc (size, GFP_KERNEL);
+	req->buf = kmalloc (size, gfp_flags);
 	if (!req->buf) {
 		usb_ep_free_request (ep, req);
 		req = NULL;
@@ -2371,13 +2303,11 @@ autoconf_fail:
 			gadget->name);
 		return -ENODEV;
 	}
-	EP_IN_NAME = in_ep->name;
 	in_ep->driver_data = in_ep;	/* claim */
 	
 	out_ep = usb_ep_autoconfig (gadget, &fs_sink_desc);
 	if (!out_ep)
 		goto autoconf_fail;
-	EP_OUT_NAME = out_ep->name;
 	out_ep->driver_data = out_ep;	/* claim */
 
 #if defined(DEV_CONFIG_CDC) || defined(CONFIG_USB_ETH_RNDIS)
@@ -2387,7 +2317,6 @@ autoconf_fail:
 	if (cdc || rndis) {
 		status_ep = usb_ep_autoconfig (gadget, &fs_status_desc);
 		if (status_ep) {
-			EP_STATUS_NAME = status_ep->name;
 			status_ep->driver_data = status_ep;	/* claim */
 		} else if (rndis) {
 			dev_err (&gadget->dev,
@@ -2429,7 +2358,7 @@ autoconf_fail:
 	hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
 	hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
 #if defined(DEV_CONFIG_CDC) || defined(CONFIG_USB_ETH_RNDIS)
-	if (EP_STATUS_NAME)
+	if (status_ep)
 		hs_status_desc.bEndpointAddress =
 				fs_status_desc.bEndpointAddress;
 #endif
@@ -2502,7 +2431,7 @@ autoconf_fail:
 	SET_ETHTOOL_OPS(net, &ops);
 
 	/* preallocate control message data and buffer */
-	dev->req = eth_req_alloc (gadget->ep0, USB_BUFSIZ);
+	dev->req = eth_req_alloc (gadget->ep0, USB_BUFSIZ, GFP_KERNEL);
 	if (!dev->req)
 		goto fail;
 	dev->req->complete = eth_setup_complete;
@@ -2510,11 +2439,12 @@ autoconf_fail:
 	/* ... and maybe likewise for status transfer */
 	if (dev->status_ep) {
 		dev->stat_req = eth_req_alloc (dev->status_ep,
-					STATUS_BYTECOUNT);
+					STATUS_BYTECOUNT, GFP_KERNEL);
 		if (!dev->stat_req) {
 			eth_req_free (gadget->ep0, dev->req);
 			goto fail;
 		}
+		dev->stat_req->context = NULL;
 	}
 
 	/* finish hookup to lower layer ... */
@@ -2529,16 +2459,16 @@ autoconf_fail:
 	netif_stop_queue (dev->net);
 	netif_carrier_off (dev->net);
 
- 	// SET_NETDEV_DEV (dev->net, &gadget->dev);
+ 	SET_NETDEV_DEV (dev->net, &gadget->dev);
  	status = register_netdev (dev->net);
 	if (status < 0)
 		goto fail1;
 
 	INFO (dev, "%s, version: " DRIVER_VERSION "\n", driver_desc);
 	INFO (dev, "using %s, OUT %s IN %s%s%s\n", gadget->name,
-		EP_OUT_NAME, EP_IN_NAME,
-		EP_STATUS_NAME ? " STATUS " : "",
-		EP_STATUS_NAME ? EP_STATUS_NAME : ""
+		out_ep->name, in_ep->name,
+		status_ep ? " STATUS " : "",
+		status_ep ? status_ep->name : ""
 		);
 	INFO (dev, "MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
 		net->dev_addr [0], net->dev_addr [1],
@@ -2613,11 +2543,8 @@ eth_resume (struct usb_gadget *gadget)
 /*-------------------------------------------------------------------------*/
 
 static struct usb_gadget_driver eth_driver = {
-#ifdef CONFIG_USB_GADGET_DUALSPEED
-	.speed		= USB_SPEED_HIGH,
-#else
-	.speed		= USB_SPEED_FULL,
-#endif
+	.speed		= DEVSPEED,
+
 	.function	= (char *) driver_desc,
 	.bind		= eth_bind,
 	.unbind		= eth_unbind,