On x86_64:
drivers/usb/gadget/rndis.c: In function `gen_ndis_set_resp': drivers/usb/gadget/rndis.c:758: warning: cast from pointer to integer of different size drivers/usb/gadget/rndis.c:761: warning: cast from pointer to integer of different size
It's not clear what this driver is trying to do there. It is taking some kernel address, adding 28 to it and is then examining particular bits within the resulting address!
I think what it's actually trying to do is to dereference the pointer rather than index off it...
I think so too. Plus, looking at the code a bit more (I wish I'd just run Lindent on it!) I noticed a few more problems.
Greg, please merge this updated version of Andrew's patch.
- Dave
Fixes some problems in the new RNDIS gadget code:
- Fix compile problem on x86_64, it wasn't dereferencing the
packet data correctly (from Andrew Morton)
- Warn when compiled big-endian ... RNDIS packet data is
always little-endian, lots of byteswapping is omitted.
(CONFIG_BROKEN_ON_BIG_ENDIAN might be better...)
- Don't mutilate local netdev->flags. It's not clear to
me how to handle hosts' packet filters (here or in the
real CDC code); but that mutilation is just wrong.
- Add omitted spinlock around code that processes RNDIS
commands; they're basically like deferred setup() calls.
- Handle state transitions better:
+ DATA_INITIALIZED transitions are like activating the
CDC Ethernet altsetting: carrier on/off, queues, etc
+ GET_INTERFACE returns the right answer with RNDIS
- Add comments about some corners that were cut (they could
make trouble with some MSFT patchlevels, and some would
allegedly prevent passing WHQL testing)
- Fix speling error, "INIZIALIZE"; and similar minor
code cleanups.
--- 1.34/drivers/usb/gadget/ether.c Fri Apr 2 12:11:13 2004
+++ edited/drivers/usb/gadget/ether.c Wed Apr 14 11:00:04 2004
@@ -1334,8 +1334,10 @@
struct eth_dev *dev = ep->driver_data;
/* received RNDIS command from CDC_SEND_ENCAPSULATED_COMMAND */
+ spin_lock(&dev->lock);
if (rndis_msg_parser (dev->rndis_config, (u8 *) req->buf))
ERROR(dev, "%s: rndis parse error\n", __FUNCTION__ );
+ spin_unlock(&dev->lock);
}
#endif /* RNDIS */
@@ -1486,14 +1488,14 @@
|| !dev->config
|| ctrl->wIndex > 1)
break;
- if (!dev->cdc && ctrl->wIndex != 0)
+ if (!(dev->cdc || dev->rndis) && ctrl->wIndex != 0)
break;
- /* if carrier is on, data interface is active. */
- *(u8 *)req->buf =
- ((ctrl->wIndex == 1) && netif_carrier_ok (dev->net))
- ? 1
- : 0,
+ /* for CDC, iff carrier is on, data interface is active. */
+ if (dev->rndis || ctrl->wIndex != 1)
+ *(u8 *)req->buf = 0;
+ else
+ *(u8 *)req->buf = netif_carrier_ok (dev->net) ? 1 : 0;
value = min (ctrl->wLength, (u16) 1);
break;
@@ -1552,6 +1554,7 @@
memcpy (req->buf, buf, value);
req->complete = rndis_response_complete;
}
+ /* else stalls ... spec says to avoid that */
}
break;
#endif /* RNDIS */
@@ -1589,6 +1592,8 @@
netif_carrier_off (dev->net);
eth_reset_config (dev);
spin_unlock_irqrestore (&dev->lock, flags);
+
+ /* FIXME RNDIS should enter RNDIS_UNINITIALIZED */
/* next we may get setup() calls to enumerate new connections;
* or an unbind() during shutdown (including removing module).
--- 1.3/drivers/usb/gadget/rndis.c Fri Apr 2 12:11:13 2004
+++ edited/drivers/usb/gadget/rndis.c Wed Apr 14 10:56:41 2004
@@ -36,6 +36,9 @@
#include "rndis.h"
+#ifndef __LITTLE_ENDIAN
+#warning this code is missing all cpu_to_leXX() calls ...
+#endif
#if 0
#define DEBUG if (rndis_debug) printk
@@ -89,8 +92,12 @@
static void currentFilter2devFlags (u32 currentFilter, struct net_device *dev)
{
+ /* FIXME the filter is supposed to control what gets
+ * forwarded from gadget to host; but dev->flags controls
+ * reporting from host to gadget ...
+ */
+#if 0
if (!dev) return;
-
if (currentFilter & NDIS_PACKET_TYPE_MULTICAST)
dev->flags |= IFF_MULTICAST;
if (currentFilter & NDIS_PACKET_TYPE_BROADCAST)
@@ -99,8 +106,13 @@
dev->flags |= IFF_ALLMULTI;
if (currentFilter & NDIS_PACKET_TYPE_PROMISCUOUS)
dev->flags |= IFF_PROMISC;
+#endif
}
+/* FIXME OMITTED OIDs, that RNDIS-on-USB "must" support, include
+ * - power management (OID_PNP_CAPABILITIES, ...)
+ * - network wakeup (OID_PNP_ENABLE_WAKE_UP, ...)
+ */
/* NDIS Functions */
static int gen_ndis_query_resp (int configNr, u32 OID, rndis_resp_t *r)
@@ -114,8 +126,6 @@
if (!resp) return -ENOMEM;
- if (!resp) return -ENOMEM;
-
switch (OID) {
/* mandatory */
case OID_GEN_SUPPORTED_LIST:
@@ -178,7 +188,8 @@
case OID_GEN_LINK_SPEED:
DEBUG("%s: OID_GEN_LINK_SPEED\n", __FUNCTION__);
length = 4;
- if (rndis_per_dev_params [configNr].media_state)
+ if (rndis_per_dev_params [configNr].media_state
+ == NDIS_MEDIA_STATE_DISCONNECTED)
*((u32 *) resp + 6) = 0;
else
*((u32 *) resp + 6) = rndis_per_dev_params [configNr].speed;
@@ -746,22 +757,38 @@
rndis_set_cmplt_type *resp;
int i, retval = -ENOTSUPP;
struct rndis_config_parameter *param;
-
- if (!r) return -ENOMEM;
+ struct rndis_params *params;
+ u8 *cp;
+
+ if (!r)
+ return -ENOMEM;
resp = (rndis_set_cmplt_type *) r->buf;
-
- if (!resp) return -ENOMEM;
-
+ if (!resp)
+ return -ENOMEM;
+
+ cp = (u8 *)resp;
+
switch (OID) {
case OID_GEN_CURRENT_PACKET_FILTER:
DEBUG("%s: OID_GEN_CURRENT_PACKET_FILTER\n", __FUNCTION__);
- currentFilter2devFlags ((u32) ((u8 *) resp + 28),
- rndis_per_dev_params [configNr].dev);
+ params = &rndis_per_dev_params [configNr];
+ currentFilter2devFlags(cp[28], params->dev);
retval = 0;
- if ((u32) ((u8 *) resp + 28))
- rndis_per_dev_params [configNr].state = RNDIS_INITIALIZED;
- else
- rndis_per_dev_params [configNr].state = RNDIS_UNINITIALIZED;
+
+ /* this call has a significant side effect: it's
+ * what makes the packet flow start and stop, like
+ * activating the CDC Ethernet altsetting.
+ */
+ if (cp[28]) {
+ params->state = RNDIS_DATA_INITIALIZED;
+ netif_carrier_on(params->dev);
+ if (netif_running(params->dev))
+ netif_wake_queue (params->dev);
+ } else {
+ params->state = RNDIS_INITIALIZED;
+ netif_carrier_off (params->dev);
+ netif_stop_queue (params->dev);
+ }
break;
case OID_802_3_MULTICAST_LIST:
@@ -937,10 +964,9 @@
{
rndis_keepalive_cmplt_type *resp;
rndis_resp_t *r;
-
- /* respond only in RNDIS_INITIALIZED state */
- if (rndis_per_dev_params [configNr].state != RNDIS_INITIALIZED)
- return 0;
+
+ /* host "should" check only in RNDIS_DATA_INITIALIZED state */
+
r = rndis_add_response (configNr, sizeof (rndis_keepalive_cmplt_type));
resp = (rndis_keepalive_cmplt_type *) r->buf;
if (!resp) return -ENOMEM;
@@ -1010,29 +1036,37 @@
int rndis_msg_parser (u8 configNr, u8 *buf)
{
u32 MsgType, MsgLength, *tmp;
+ struct rndis_params *params;
- if (!buf) return -ENOMEM;
+ if (!buf)
+ return -ENOMEM;
tmp = (u32 *) buf;
MsgType = *tmp;
MsgLength = *(tmp + 1);
- if (configNr >= RNDIS_MAX_CONFIGS) return -ENOTSUPP;
+ if (configNr >= RNDIS_MAX_CONFIGS)
+ return -ENOTSUPP;
+ params = &rndis_per_dev_params [configNr];
+ /* For USB: responses may take up to 10 seconds */
switch (MsgType)
{
- case REMOTE_NDIS_INIZIALIZE_MSG:
- DEBUG(KERN_INFO "%s: REMOTE_NDIS_INIZIALIZE_MSG\n",
+ case REMOTE_NDIS_INITIALIZE_MSG:
+ DEBUG(KERN_INFO "%s: REMOTE_NDIS_INITIALIZE_MSG\n",
__FUNCTION__ );
- rndis_per_dev_params [configNr].state = RNDIS_INITIALIZED;
+ params->state = RNDIS_INITIALIZED;
return rndis_init_response (configNr,
(rndis_init_msg_type *) buf);
- break;
case REMOTE_NDIS_HALT_MSG:
DEBUG(KERN_INFO "%s: REMOTE_NDIS_HALT_MSG\n",
__FUNCTION__ );
- rndis_per_dev_params [configNr].state = RNDIS_UNINITIALIZED;
+ params->state = RNDIS_UNINITIALIZED;
+ if (params->dev) {
+ netif_carrier_off (params->dev);
+ netif_stop_queue (params->dev);
+ }
return 0;
case REMOTE_NDIS_QUERY_MSG:
@@ -1040,29 +1074,26 @@
__FUNCTION__ );
return rndis_query_response (configNr,
(rndis_query_msg_type *) buf);
- break;
case REMOTE_NDIS_SET_MSG:
DEBUG(KERN_INFO "%s: REMOTE_NDIS_SET_MSG\n",
__FUNCTION__ );
return rndis_set_response (configNr,
(rndis_set_msg_type *) buf);
- break;
case REMOTE_NDIS_RESET_MSG:
DEBUG(KERN_INFO "%s: REMOTE_NDIS_RESET_MSG\n",
__FUNCTION__ );
return rndis_reset_response (configNr,
(rndis_reset_msg_type *) buf);
- break;
case REMOTE_NDIS_KEEPALIVE_MSG:
+ /* For USB: host does this every 5 seconds */
DEBUG(KERN_INFO "%s: REMOTE_NDIS_KEEPALIVE_MSG\n",
__FUNCTION__ );
return rndis_keepalive_response (configNr,
(rndis_keepalive_msg_type *)
buf);
- break;
default:
printk (KERN_ERR "%s: unknown RNDIS Message Type 0x%08X\n",
@@ -1240,9 +1271,15 @@
"vendor ID : 0x%08X\n"
"vendor : %s\n",
param->confignr, (param->used) ? "y" : "n",
- (param->state)
- ? "RNDIS_INITIALIZED"
- : "RNDIS_UNINITIALIZED",
+ ({ char *s = "?";
+ switch (param->state) {
+ case RNDIS_UNINITIALIZED:
+ s = "RNDIS_UNINITIALIZED"; break;
+ case RNDIS_INITIALIZED:
+ s = "RNDIS_INITIALIZED"; break;
+ case RNDIS_DATA_INITIALIZED:
+ s = "RNDIS_DATA_INITIALIZED"; break;
+ }; s; }),
param->medium,
(param->media_state) ? 0 : param->speed*100,
(param->media_state) ? "disconnected" : "connected",
--- 1.3/drivers/usb/gadget/rndis.h Tue Mar 30 09:16:09 2004
+++ edited/drivers/usb/gadget/rndis.h Wed Apr 14 10:56:27 2004
@@ -38,7 +38,7 @@
*/
/* Message Set for Connectionless (802.3) Devices */
-#define REMOTE_NDIS_INIZIALIZE_MSG 0x00000002U /* Initialize device */
+#define REMOTE_NDIS_INITIALIZE_MSG 0x00000002U /* Initialize device */
#define REMOTE_NDIS_HALT_MSG 0x00000003U
#define REMOTE_NDIS_QUERY_MSG 0x00000004U
#define REMOTE_NDIS_SET_MSG 0x00000005U
