[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>
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c
index a766c29..3830a0a 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 @@
* 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 @@
#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 @@
}
#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 @@
.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 @@
#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 @@
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 @@
* 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;
- }
-#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
+ 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;
}
- /* stop on error */
- ERROR (dev, "can't enable %s, result %d\n", ep->name, result);
- break;
+ 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;
+ }
}
- 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 @@
/* 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 @@
spin_lock (&dev->lock);
}
}
-#endif
if (result == 0)
DEBUG (dev, "qlen %d\n", qlen (gadget));
@@ -1153,6 +1070,8 @@
if (dev->status) {
usb_ep_disable (dev->status_ep);
}
+ dev->rndis = 0;
+ dev->cdc_filter = 0;
dev->config = 0;
}
@@ -1165,9 +1084,6 @@
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_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 @@
#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 @@
} 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 @@
* 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 @@
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 @@
/* 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 @@
*/
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 @@
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 @@
{
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 @@
#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 @@
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 @@
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 @@
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 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 @@
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 @@
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 @@
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 @@
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 @@
/* ... 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 @@
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 @@
/*-------------------------------------------------------------------------*/
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,