[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,