2007-07-31 07:38:10

by Michael Wu

[permalink] [raw]
Subject: [PATCH] orinoco, wl3501, zd1201: use linux/ieee80211.h instead of net/ieee80211.h

From: Michael Wu <[email protected]>

These drivers only need 802.11 definitions.

Signed-off-by: Michael Wu <[email protected]>
---

drivers/net/wireless/orinoco.c | 13 +++++++------
drivers/net/wireless/wl3501.h | 4 ++--
drivers/net/wireless/zd1201.c | 4 ++--
3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/orinoco.c b/drivers/net/wireless/orinoco.c
index 062286d..5e61a10 100644
--- a/drivers/net/wireless/orinoco.c
+++ b/drivers/net/wireless/orinoco.c
@@ -84,8 +84,8 @@
#include <linux/ethtool.h>
#include <linux/if_arp.h>
#include <linux/wireless.h>
+#include <linux/ieee80211.h>
#include <net/iw_handler.h>
-#include <net/ieee80211.h>

#include "hermes_rid.h"
#include "orinoco.h"
@@ -137,7 +137,7 @@ static const u8 encaps_hdr[] = {0xaa, 0xaa, 0x03, 0x00, 0x00, 0x00};
#define ENCAPS_OVERHEAD (sizeof(encaps_hdr) + 2)

#define ORINOCO_MIN_MTU 256
-#define ORINOCO_MAX_MTU (IEEE80211_DATA_LEN - ENCAPS_OVERHEAD)
+#define ORINOCO_MAX_MTU (IEEE80211_MAX_DATA_LEN - ENCAPS_OVERHEAD)

#define SYMBOL_MAX_VER_LEN (14)
#define USER_BAP 0
@@ -401,7 +401,7 @@ static int orinoco_change_mtu(struct net_device *dev, int new_mtu)
if ( (new_mtu < ORINOCO_MIN_MTU) || (new_mtu > ORINOCO_MAX_MTU) )
return -EINVAL;

- if ( (new_mtu + ENCAPS_OVERHEAD + IEEE80211_HLEN) >
+ if ( (new_mtu + ENCAPS_OVERHEAD + sizeof(struct ieee80211_hdr)) >
(priv->nicbuf_size - ETH_HLEN) )
return -EINVAL;

@@ -754,7 +754,7 @@ static void orinoco_rx_monitor(struct net_device *dev, u16 rxfid,
}

/* sanity check the length */
- if (datalen > IEEE80211_DATA_LEN + 12) {
+ if (datalen > IEEE80211_MAX_DATA_LEN + 12) {
printk(KERN_DEBUG "%s: oversized monitor frame, "
"data length = %d\n", dev->name, datalen);
stats->rx_length_errors++;
@@ -857,7 +857,7 @@ static void __orinoco_ev_rx(struct net_device *dev, hermes_t *hw)
data. */
return;
}
- if (length > IEEE80211_DATA_LEN) {
+ if (length > IEEE80211_MAX_DATA_LEN) {
printk(KERN_WARNING "%s: Oversized frame received (%d bytes)\n",
dev->name, length);
stats->rx_length_errors++;
@@ -2235,7 +2235,8 @@ static int orinoco_init(struct net_device *dev)

/* No need to lock, the hw_unavailable flag is already set in
* alloc_orinocodev() */
- priv->nicbuf_size = IEEE80211_FRAME_LEN + ETH_HLEN;
+ priv->nicbuf_size = IEEE80211_MAX_DATA_LEN +
+ sizeof(struct ieee80211_hdr) + ETH_HLEN;

/* Initialize the firmware */
err = hermes_init(hw);
diff --git a/drivers/net/wireless/wl3501.h b/drivers/net/wireless/wl3501.h
index 65ceb08..59bb3a5 100644
--- a/drivers/net/wireless/wl3501.h
+++ b/drivers/net/wireless/wl3501.h
@@ -2,7 +2,7 @@
#define __WL3501_H__

#include <linux/spinlock.h>
-#include <net/ieee80211.h>
+#include <linux/ieee80211.h>

/* define for WLA 2.0 */
#define WL3501_BLKSZ 256
@@ -548,7 +548,7 @@ struct wl3501_80211_tx_plcp_hdr {

struct wl3501_80211_tx_hdr {
struct wl3501_80211_tx_plcp_hdr pclp_hdr;
- struct ieee80211_hdr_4addr mac_hdr;
+ struct ieee80211_hdr mac_hdr;
} __attribute__ ((packed));

/*
diff --git a/drivers/net/wireless/zd1201.c b/drivers/net/wireless/zd1201.c
index 935b144..57b03bb 100644
--- a/drivers/net/wireless/zd1201.c
+++ b/drivers/net/wireless/zd1201.c
@@ -21,7 +21,7 @@
#include <linux/string.h>
#include <linux/if_arp.h>
#include <linux/firmware.h>
-#include <net/ieee80211.h>
+#include <linux/ieee80211.h>
#include "zd1201.h"

static struct usb_device_id zd1201_table[] = {
@@ -346,7 +346,7 @@ static void zd1201_usbrx(struct urb *urb)
frag = kmalloc(sizeof(*frag), GFP_ATOMIC);
if (!frag)
goto resubmit;
- skb = dev_alloc_skb(IEEE80211_DATA_LEN +14+2);
+ skb = dev_alloc_skb(IEEE80211_MAX_DATA_LEN+14+2);
if (!skb) {
kfree(frag);
goto resubmit;



2007-07-31 13:36:24

by Pavel Roskin

[permalink] [raw]
Subject: Re: [PATCH] orinoco, wl3501, zd1201: use linux/ieee80211.h instead of net/ieee80211.h

On Tue, 2007-07-31 at 00:36 -0700, Michael Wu wrote:
> From: Michael Wu <[email protected]>
>
> These drivers only need 802.11 definitions.

I have nothing against the change if I understand the motivation.
What's the point?

> -#define ORINOCO_MAX_MTU (IEEE80211_DATA_LEN - ENCAPS_OVERHEAD)
> +#define ORINOCO_MAX_MTU (IEEE80211_MAX_DATA_LEN - ENCAPS_OVERHEAD)

We probably want to standardize on one name and use it consistently in
all drivers.

> - if ( (new_mtu + ENCAPS_OVERHEAD + IEEE80211_HLEN) >
> + if ( (new_mtu + ENCAPS_OVERHEAD + sizeof(struct ieee80211_hdr)) >

I prefer the original code. "Size of 802.11 header" is easier to
understand than "size of the structure representing 802.11 header".

Can we please make IEEE80211_HLEN available to all wireless drivers?

And by the way, the definition of struct ieee80211_hdr includes payload
(which is a good thing in my opinion). The definition uses a
gcc-specific 0-size array, which we may want to convert to a c99 style
flexible array (i.e. payload[] as opposed to payload[0]).

Taking sizeof of a structure with a flexible array is dubious and is
likely to be discouraged in the future (I would support having a warning
in sparse). What we really want is the offset of the payload in the
frame, not the size allocated by the compiler for the frame with 0-byte
payload.

I would prefer all the necessary changes to occur without disturbing the
individual drivers.

--
Regards,
Pavel Roskin