[PATCH] USB: ethernet gadget updates (mostly cleanup)
authorDavid Brownell <david-b@pacbell.net>
Thu, 28 Apr 2005 20:48:09 +0000 (13:48 -0700)
committerGreg Kroah-Hartman <gregkh@suse.de>
Mon, 27 Jun 2005 21:43:50 +0000 (14:43 -0700)
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>
drivers/usb/gadget/ether.c

index a766c29c3ec9a3681419537e01563c68327c6ce3..3830a0a0fd5012b1a2d46d08b02e7a79f0f89ac4 100644 (file)
@@ -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,