Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH 1/3] cfg80211: Add nl80211 antenna configuration
From: Bruno Randolf @ 2010-05-12  8:23 UTC (permalink / raw)
  To: johannes, linville; +Cc: linux-wireless, holgerschurig
In-Reply-To: <20100512082231.26565.16730.stgit@tt-desk>

Allow setting TX and RX antenna configuration via nl80211/cfg80211.

The antenna configuration is defined as a bitmap of allowed antennas. This
bitmap is 8 bit at the moment, each bit representing one antenna. If multiple
antennas are selected, the driver may use diversity for receive and transmit.
This allows for a simple, yet flexible configuration interface for antennas,
while drivers may reject configurations they cannot support.

Signed-off-by: Bruno Randolf <br1@einfach.org>
---
 include/linux/nl80211.h |   12 +++++
 include/net/cfg80211.h  |    3 +
 net/wireless/nl80211.c  |  116 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index b7c77f9..46a2c76 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -341,6 +341,9 @@
  *	of any other interfaces, and other interfaces will again take
  *	precedence when they are used.
  *
+ * @NL80211_CMD_SET_ANTENNA: Set a bitmap of antennas to use.
+ * @NL80211_CMD_GET_ANTENNA: Get antenna configuration from driver.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -441,6 +444,9 @@ enum nl80211_commands {
 
 	NL80211_CMD_SET_CHANNEL,
 
+	NL80211_CMD_SET_ANTENNA,
+	NL80211_CMD_GET_ANTENNA,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -725,6 +731,9 @@ enum nl80211_commands {
  * @NL80211_ATTR_AP_ISOLATE: (AP mode) Do not forward traffic between stations
  *	connected to this BSS.
  *
+ * @NL80211_ATTR_ANTENNA_TX: Bitmap of antennas to use for transmitting.
+ * @NL80211_ATTR_ANTENNA_RX: Bitmap of antennas to use for receiving.
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -882,6 +891,9 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_AP_ISOLATE,
 
+	NL80211_ATTR_ANTENNA_TX,
+	NL80211_ATTR_ANTENNA_RX,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b44a2e5..8861f40 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1176,6 +1176,9 @@ struct cfg80211_ops {
 	int	(*set_cqm_rssi_config)(struct wiphy *wiphy,
 				       struct net_device *dev,
 				       s32 rssi_thold, u32 rssi_hyst);
+
+	int	(*set_antenna)(struct wiphy *wiphy, u8 tx_ant, u8 rx_ant);
+	int	(*get_antenna)(struct wiphy *wiphy, u8 *tx_ant, u8 *rx_ant);
 };
 
 /*
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index aaa1aad..29998e0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -153,6 +153,8 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
 	[NL80211_ATTR_CQM] = { .type = NLA_NESTED, },
 	[NL80211_ATTR_LOCAL_STATE_CHANGE] = { .type = NLA_FLAG },
 	[NL80211_ATTR_AP_ISOLATE] = { .type = NLA_U8 },
+	[NL80211_ATTR_ANTENNA_TX] = { .type = NLA_U8 },
+	[NL80211_ATTR_ANTENNA_RX] = { .type = NLA_U8 },
 };
 
 /* policy for the attributes */
@@ -590,6 +592,8 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
 		NLA_PUT_U32(msg, i, NL80211_CMD_SET_WIPHY_NETNS);
 	}
 	CMD(set_channel, SET_CHANNEL);
+	CMD(set_antenna, SET_ANTENNA);
+	CMD(get_antenna, GET_ANTENNA);
 
 #undef CMD
 
@@ -4963,6 +4967,106 @@ out:
 	return err;
 }
 
+static int nl80211_set_antenna(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg80211_registered_device *rdev;
+	int res;
+	u8 rx_ant = 0, tx_ant = 0;
+
+	if (!info->attrs[NL80211_ATTR_WIPHY] ||
+	    !info->attrs[NL80211_ATTR_ANTENNA_TX] ||
+	    !info->attrs[NL80211_ATTR_ANTENNA_RX]) {
+		return -EINVAL;
+	}
+
+	tx_ant = nla_get_u8(info->attrs[NL80211_ATTR_ANTENNA_TX]);
+	rx_ant = nla_get_u8(info->attrs[NL80211_ATTR_ANTENNA_RX]);
+
+	rtnl_lock();
+
+	rdev = cfg80211_get_dev_from_info(info);
+	if (IS_ERR(rdev)) {
+		res = -ENODEV;
+		goto unlock_rtnl;
+	}
+
+	if (!rdev->ops->set_antenna) {
+		res = -EOPNOTSUPP;
+		goto unlock_rdev;
+	}
+
+	res = rdev->ops->set_antenna(&rdev->wiphy, tx_ant, rx_ant);
+
+ unlock_rdev:
+	cfg80211_unlock_rdev(rdev);
+
+ unlock_rtnl:
+	rtnl_unlock();
+	return res;
+}
+
+static int nl80211_get_antenna(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg80211_registered_device *rdev;
+	struct sk_buff *msg;
+	void *hdr;
+	int res;
+	u8 tx_ant, rx_ant;
+
+	if (!info->attrs[NL80211_ATTR_WIPHY])
+		return -EINVAL;
+
+	rtnl_lock();
+
+	rdev = cfg80211_get_dev_from_info(info);
+	if (IS_ERR(rdev)) {
+		res = -ENODEV;
+		goto unlock_rtnl;
+	}
+
+	if (!rdev->ops->get_antenna) {
+		res = -EOPNOTSUPP;
+		goto unlock_rdev;
+	}
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg) {
+		res = -ENOMEM;
+		goto unlock_rdev;
+	}
+
+	hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
+			     NL80211_CMD_GET_ANTENNA);
+	if (!hdr) {
+		res = -ENOMEM;
+		goto free_msg;
+	}
+
+	res = rdev->ops->get_antenna(&rdev->wiphy, &tx_ant, &rx_ant);
+	if (res)
+		goto free_msg;
+
+	NLA_PUT_U8(msg, NL80211_ATTR_ANTENNA_TX, tx_ant);
+	NLA_PUT_U8(msg, NL80211_ATTR_ANTENNA_RX, rx_ant);
+
+	genlmsg_end(msg, hdr);
+	res = genlmsg_reply(msg, info);
+	goto unlock_rdev;
+
+ nla_put_failure:
+	res = -ENOBUFS;
+
+ free_msg:
+	nlmsg_free(msg);
+
+ unlock_rdev:
+	cfg80211_unlock_rdev(rdev);
+
+ unlock_rtnl:
+	rtnl_unlock();
+	return res;
+}
+
 static struct genl_ops nl80211_ops[] = {
 	{
 		.cmd = NL80211_CMD_GET_WIPHY,
@@ -5279,6 +5383,18 @@ static struct genl_ops nl80211_ops[] = {
 		.policy = nl80211_policy,
 		.flags = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd = NL80211_CMD_SET_ANTENNA,
+		.doit = nl80211_set_antenna,
+		.policy = nl80211_policy,
+		.flags = GENL_ADMIN_PERM,
+	},
+	{
+		.cmd = NL80211_CMD_GET_ANTENNA,
+		.doit = nl80211_get_antenna,
+		.policy = nl80211_policy,
+		/* can be retrieved by unprivileged users */
+	},
 };
 
 static struct genl_multicast_group nl80211_mlme_mcgrp = {


^ permalink raw reply related

* [PATCH 0/3] antenna configuration
From: Bruno Randolf @ 2010-05-12  8:23 UTC (permalink / raw)
  To: johannes, linville; +Cc: linux-wireless, holgerschurig

resending my antenna patches addressing all previous comments.

bruno

---

Bruno Randolf (3):
      cfg80211: Add nl80211 antenna configuration
      mac80211: Add antenna configuration
      ath5k: Add support for antenna configuration


 drivers/net/wireless/ath/ath5k/base.c |   34 ++++++++++
 include/linux/nl80211.h               |   12 +++
 include/net/cfg80211.h                |    3 +
 include/net/mac80211.h                |    2 +
 net/mac80211/cfg.c                    |   16 +++++
 net/mac80211/driver-ops.h             |   23 +++++++
 net/mac80211/driver-trace.h           |   50 ++++++++++++++
 net/wireless/nl80211.c                |  116 +++++++++++++++++++++++++++++++++
 8 files changed, 256 insertions(+), 0 deletions(-)

-- 
Signature

^ permalink raw reply

* Re: [PATCH 2/2] compat-wireless: make patches apply again
From: Bruno Randolf @ 2010-05-12  8:20 UTC (permalink / raw)
  To: Hauke Mehrtens; +Cc: lrodriguez, linux-wireless, mcgrof
In-Reply-To: <1273526650-26282-2-git-send-email-hauke@hauke-m.de>

On Tuesday 11 May 2010 06:24:10 Hauke Mehrtens wrote:
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  patches/07-change-default-rate-alg.patch |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/patches/07-change-default-rate-alg.patch
> b/patches/07-change-default-rate-alg.patch index af65207..f0ccbce 100644
> --- a/patches/07-change-default-rate-alg.patch
> +++ b/patches/07-change-default-rate-alg.patch
> @@ -21,7 +21,7 @@ at compilation time.
>   module_param(ieee80211_default_rc_algo, charp, 0644);
>   MODULE_PARM_DESC(ieee80211_default_rc_algo,
>   		 "Default rate control algorithm for mac80211 to use");
> -@@ -118,8 +118,8 @@ ieee80211_rate_control_ops_get(const cha
> +@@ -119,8 +119,8 @@ ieee80211_rate_control_ops_get(const cha
>   		ops = 
ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo);
> 
>   	/* try built-in one if specific alg requested but not found */
> @@ -29,6 +29,6 @@ at compilation time.
>  -		ops = 
ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT);
>  +	if (!ops && strlen(CONFIG_COMPAT_MAC80211_RC_DEFAULT))
>  +		ops =
> ieee80211_try_rate_control_ops_get(CONFIG_COMPAT_MAC80211_RC_DEFAULT); +
> 	kparam_unblock_sysfs_write(ieee80211_default_rc_algo);
> 
>   	return ops;
> - }

this is somehow messed up and doesn't apply. here is a fixed version of the 
patch (not a patch for a patch)...

---

Your current kernels configuration (.config and linux/autoconf.h)
are always respected when compiling external modules. Because
of this if you are using an old kernel which preferred the
PID rate control algorithm we cannot force it to use minstrel
instead. Minstrel is now the default rate control algorithm
and we want you to use it. To let you use it we redefine here
the CONFIG_MAC80211_RC_DEFAULT to CONFIG_COMPAT_MAC80211_RC_DEFAULT
and define CONFIG_COMPAT_MAC80211_RC_DEFAULT on config.mk.
Through the compat autoconf we then get it also defined there
at compilation time.

--- a/net/mac80211/rate.c	2010-05-12 16:27:02.245747971 +0900
+++ b/net/mac80211/rate.c	2010-05-12 17:00:45.855748158 +0900
@@ -23,7 +23,7 @@
 static LIST_HEAD(rate_ctrl_algs);
 static DEFINE_MUTEX(rate_ctrl_mutex);
 
-static char *ieee80211_default_rc_algo = CONFIG_MAC80211_RC_DEFAULT;
+static char *ieee80211_default_rc_algo = CONFIG_COMPAT_MAC80211_RC_DEFAULT;
 module_param(ieee80211_default_rc_algo, charp, 0644);
 MODULE_PARM_DESC(ieee80211_default_rc_algo,
 		 "Default rate control algorithm for mac80211 to use");
@@ -118,8 +118,8 @@
 		ops = ieee80211_try_rate_control_ops_get(ieee80211_default_rc_algo);
 
 	/* try built-in one if specific alg requested but not found */
-	if (!ops && strlen(CONFIG_MAC80211_RC_DEFAULT))
-		ops = ieee80211_try_rate_control_ops_get(CONFIG_MAC80211_RC_DEFAULT);
+	if (!ops && strlen(CONFIG_COMPAT_MAC80211_RC_DEFAULT))
+		ops = 
ieee80211_try_rate_control_ops_get(CONFIG_COMPAT_MAC80211_RC_DEFAULT);
 
 	return ops;
 }

^ permalink raw reply

* Re: [PATCH 7/9] rt2x00: In debugfs frame dumping allow the TX descriptor to be part of the skb.
From: Ivo Van Doorn @ 2010-05-12  7:12 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-8-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> Preparation for futher cleanups in the area of properly maintaining the skb
> data without fiddling with the skb->data pointer.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

^ permalink raw reply

* Re: [PATCH 4/9] rt2x00: Simplify TXD handling of beacons.
From: Ivo Van Doorn @ 2010-05-12  7:11 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-5-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> The handling of tx descriptors for beacons can be simplified by updating
> write_tx_desc implementations of each driver to write directly to the
> queue entry descriptor instead of to a provided memory area.
> This is also a preparation for further clean ups where descriptors are
> properly reserved in the skb instead of fiddling with the skb data
> pointer.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

^ permalink raw reply

* Re: [PATCH 9/9] rt2x00: Properly reserve room for descriptors in skbs.
From: Ivo Van Doorn @ 2010-05-12  7:08 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-10-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> Instead of fiddling with the skb->data pointer and thereby risking
> out of bounds accesses, properly reserve the space needed in an
> skb for descriptors.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

^ permalink raw reply

* Re: [PATCH 6/9] rt2x00: Push beacon TX descriptor writing to drivers.
From: Ivo Van Doorn @ 2010-05-12  7:06 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-7-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> Not all the devices require a TX descriptor to be written (i.e. rt2800
> device don't require them). Push down the creation of the TX descriptor
> to the device drivers so that they can decide for themselves whether
> a TX descriptor is to be created.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
> ---
>  drivers/net/wireless/rt2x00/rt2400pci.c   |   16 ++++++++++------
>  drivers/net/wireless/rt2x00/rt2500pci.c   |   16 ++++++++++------
>  drivers/net/wireless/rt2x00/rt2500usb.c   |   11 +++++++++++
>  drivers/net/wireless/rt2x00/rt2800pci.c   |   17 +++++++++++++++++
>  drivers/net/wireless/rt2x00/rt2800usb.c   |   17 +++++++++++++++++
>  drivers/net/wireless/rt2x00/rt2x00debug.c |    1 +
>  drivers/net/wireless/rt2x00/rt2x00queue.c |   10 +---------
>  drivers/net/wireless/rt2x00/rt61pci.c     |   11 +++++++++++
>  drivers/net/wireless/rt2x00/rt73usb.c     |   11 +++++++++++
>  9 files changed, 89 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
> index def3fa4..741c531 100644
> --- a/drivers/net/wireless/rt2x00/rt2400pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2400pci.c
> @@ -33,6 +33,7 @@
>  #include <linux/eeprom_93cx6.h>
>
>  #include "rt2x00.h"
> +#include "rt2x00lib.h"
>  #include "rt2x00pci.h"
>  #include "rt2400pci.h"

rt2x00lib.h must not be used in the drivers. It is for the rt2x00lib
internal files only.

> @@ -1074,9 +1075,6 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>                                   struct txentry_desc *txdesc)
>  {
>        struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
> -       struct queue_entry_priv_pci *entry_priv = entry->priv_data;
> -       struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
> -       u32 word;
>        u32 reg;
>
>        /*
> @@ -1089,9 +1087,15 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
>
>        rt2x00queue_map_txskb(rt2x00dev, entry->skb);
>
> -       rt2x00_desc_read(entry_priv->desc, 1, &word);
> -       rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
> -       rt2x00_desc_write(entry_priv->desc, 1, word);
> +       /*
> +        * Write the TX descriptor for the beacon.
> +        */
> +       rt2400pci_write_tx_desc(rt2x00dev, entry->skb, txdesc);
> +
> +       /*
> +        * Dump beacon to userspace through debugfs.
> +        */
> +       rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);

The goal for rt2x00debug was that the logic must be inside rt2x00lib
as much as possible.
This can/should be moved into rt2x00lib where write_beacon() is being called.

Ivo

^ permalink raw reply

* Re: [PATCH 8/9] rt2x00: Reverse calling order of bus write_tx_desc and driver write_tx_desc.
From: Ivo Van Doorn @ 2010-05-12  7:02 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-9-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> For rt2800 reverse the calling order of rt2x00pci_write_data and
> rt2800pci_write_data. Currently rt2800pci_write_data calls rt2x00pci_write_data
> as there can be only 1 driver callback function specified by the driver.
> Reverse this calling order by introducing a new driver callback function,
> called add_tx_datadesc, which is called from the bus-specific write_tx_data
> functions.
> Preparation for futher cleanups in the skb data handling of rt2x00.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
> ---
>  drivers/net/wireless/rt2x00/rt2800pci.c |   15 ++++-----------
>  drivers/net/wireless/rt2x00/rt2x00.h    |    2 ++
>  drivers/net/wireless/rt2x00/rt2x00pci.c |    6 ++++++
>  drivers/net/wireless/rt2x00/rt2x00usb.c |    6 ++++++
>  4 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index fcca30c..bbd6481 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -614,18 +614,10 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
>  /*
>  * TX descriptor initialization
>  */
> -static int rt2800pci_write_tx_data(struct queue_entry* entry,
> -                                  struct txentry_desc *txdesc)
> +static void rt2800pci_add_tx_datadesc(struct queue_entry* entry,
> +                                     struct txentry_desc *txdesc)
>  {
> -       int ret;
> -
> -       ret = rt2x00pci_write_tx_data(entry, txdesc);
> -       if (ret)
> -               return ret;
> -
>        rt2800_write_txwi(entry->skb, txdesc);
> -
> -       return 0;
>  }
>
>
> @@ -1080,7 +1072,8 @@ static const struct rt2x00lib_ops rt2800pci_rt2x00_ops = {
>        .reset_tuner            = rt2800_reset_tuner,
>        .link_tuner             = rt2800_link_tuner,
>        .write_tx_desc          = rt2800pci_write_tx_desc,
> -       .write_tx_data          = rt2800pci_write_tx_data,
> +       .write_tx_data          = rt2x00pci_write_tx_data,
> +       .add_tx_datadesc        = rt2800pci_add_tx_datadesc,
>        .write_beacon           = rt2800pci_write_beacon,
>        .kick_tx_queue          = rt2800pci_kick_tx_queue,
>        .kill_tx_queue          = rt2800pci_kill_tx_queue,

Doesn't the name write_tx_datadesc make more sense?

Ivo

^ permalink raw reply

* Re: [PATCH 3/9] rt2x00: Re-order tx descriptor writing code in drivers.
From: Ivo Van Doorn @ 2010-05-12  7:00 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-4-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> Where possible, write the tx descriptor words from start to end, to
> follow a logical ordering of words.
> Where this is not possible (in rt2400pci, rt2500pci and rt61pci) add
> a comment as to why word 0 needs to be written last.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

^ permalink raw reply

* Re: [PATCH 5/9] rt2x00: Dump beacons under a different identifier than TX frames.
From: Ivo Van Doorn @ 2010-05-12  7:00 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-6-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> This allows for specific identification of beacons in the debugfs
> frame stream.
> Preparation for later differences between dumped TX frames and dumped
> beacons.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

^ permalink raw reply

* Re: [PATCH 1/9] rt2x00: Consistently name skb frame descriptor skbdesc.
From: Ivo Van Doorn @ 2010-05-12  6:59 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-2-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> The skb frame descriptor is called everywhere skbdesc, except in one
> place in rt2x00debug_dump_frame. Change that occurence to have
> consistent naming.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

> ---
>  drivers/net/wireless/rt2x00/rt2x00debug.c |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00debug.c b/drivers/net/wireless/rt2x00/rt2x00debug.c
> index 70c04c2..9e2eed5 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00debug.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00debug.c
> @@ -155,7 +155,7 @@ void rt2x00debug_dump_frame(struct rt2x00_dev *rt2x00dev,
>                            enum rt2x00_dump_type type, struct sk_buff *skb)
>  {
>        struct rt2x00debug_intf *intf = rt2x00dev->debugfs_intf;
> -       struct skb_frame_desc *desc = get_skb_frame_desc(skb);
> +       struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
>        struct sk_buff *skbcopy;
>        struct rt2x00dump_hdr *dump_hdr;
>        struct timeval timestamp;
> @@ -170,7 +170,7 @@ void rt2x00debug_dump_frame(struct rt2x00_dev *rt2x00dev,
>                return;
>        }
>
> -       skbcopy = alloc_skb(sizeof(*dump_hdr) + desc->desc_len + skb->len,
> +       skbcopy = alloc_skb(sizeof(*dump_hdr) + skbdesc->desc_len + skb->len,
>                            GFP_ATOMIC);
>        if (!skbcopy) {
>                DEBUG(rt2x00dev, "Failed to copy skb for dump.\n");
> @@ -180,18 +180,19 @@ void rt2x00debug_dump_frame(struct rt2x00_dev *rt2x00dev,
>        dump_hdr = (struct rt2x00dump_hdr *)skb_put(skbcopy, sizeof(*dump_hdr));
>        dump_hdr->version = cpu_to_le32(DUMP_HEADER_VERSION);
>        dump_hdr->header_length = cpu_to_le32(sizeof(*dump_hdr));
> -       dump_hdr->desc_length = cpu_to_le32(desc->desc_len);
> +       dump_hdr->desc_length = cpu_to_le32(skbdesc->desc_len);
>        dump_hdr->data_length = cpu_to_le32(skb->len);
>        dump_hdr->chip_rt = cpu_to_le16(rt2x00dev->chip.rt);
>        dump_hdr->chip_rf = cpu_to_le16(rt2x00dev->chip.rf);
>        dump_hdr->chip_rev = cpu_to_le16(rt2x00dev->chip.rev);
>        dump_hdr->type = cpu_to_le16(type);
> -       dump_hdr->queue_index = desc->entry->queue->qid;
> -       dump_hdr->entry_index = desc->entry->entry_idx;
> +       dump_hdr->queue_index = skbdesc->entry->queue->qid;
> +       dump_hdr->entry_index = skbdesc->entry->entry_idx;
>        dump_hdr->timestamp_sec = cpu_to_le32(timestamp.tv_sec);
>        dump_hdr->timestamp_usec = cpu_to_le32(timestamp.tv_usec);
>
> -       memcpy(skb_put(skbcopy, desc->desc_len), desc->desc, desc->desc_len);
> +       memcpy(skb_put(skbcopy, skbdesc->desc_len), skbdesc->desc,
> +              skbdesc->desc_len);
>        memcpy(skb_put(skbcopy, skb->len), skb->data, skb->len);
>
>        skb_queue_tail(&intf->frame_dump_skbqueue, skbcopy);
> --
> 1.7.1
>
>

^ permalink raw reply

* Re: [PATCH 2/9] rt2x00: Fix beacon descriptor writing for rt61pci.
From: Ivo Van Doorn @ 2010-05-12  6:59 UTC (permalink / raw)
  To: Gertjan van Wingerde; +Cc: John W. Linville, linux-wireless, users
In-Reply-To: <1273614705-25118-3-git-send-email-gwingerde@gmail.com>

On Tue, May 11, 2010 at 11:51 PM, Gertjan van Wingerde
<gwingerde@gmail.com> wrote:
> The buffer address descriptor word is not part of the TXINFO structure
> needed for beacons. The current writing of that word for beacons is
> therefore an out-of-bounds write.
> Fix this by only writing the buffer address descriptor word for TX
> queues.
>
> Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
> ---
>  drivers/net/wireless/rt2x00/rt61pci.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
> index 2436363..99c2981 100644
> --- a/drivers/net/wireless/rt2x00/rt61pci.c
> +++ b/drivers/net/wireless/rt2x00/rt61pci.c
> @@ -1801,12 +1801,12 @@ static void rt61pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
>        rt2x00_set_field32(&word, TXD_W5_WAITING_DMA_DONE_INT, 1);
>        rt2x00_desc_write(txd, 5, word);
>
> -       rt2x00_desc_read(txd, 6, &word);
> -       rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
> -                          skbdesc->skb_dma);
> -       rt2x00_desc_write(txd, 6, word);
> +       if (txdesc->queue != QID_BEACON) {
> +               rt2x00_desc_read(txd, 6, &word);
> +               rt2x00_set_field32(&word, TXD_W6_BUFFER_PHYSICAL_ADDRESS,
> +                                  skbdesc->skb_dma);
> +               rt2x00_desc_write(txd, 6, word);
>
> -       if (skbdesc->desc_len > TXINFO_SIZE) {
>                rt2x00_desc_read(txd, 11, &word);
>                rt2x00_set_field32(&word, TXD_W11_BUFFER_LENGTH0,
>                                   txdesc->length);

Shouldn't the check for TXINFO_SIZE be used rather than explicitly
checking for the QID?

Ivo

^ permalink raw reply

* Re: [PATCH 3/4] ath9k: add debugfs files for reading/writing registers
From: Benoit Papillault @ 2010-05-12  6:03 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, lrodriguez, linville
In-Reply-To: <1273591383-76696-3-git-send-email-nbd@openwrt.org>

Le 11/05/2010 17:23, Felix Fietkau a écrit :
> Signed-off-by: Felix Fietkau<nbd@openwrt.org>

This patch has been around in my tree for quite a long time and it's 
really helpful to play with registers.

Acked-by: Benoit Papillault <benoit.papillault@free.fr>

> ---
>   drivers/net/wireless/ath/ath9k/debug.c |   89 ++++++++++++++++++++++++++++++++
>   drivers/net/wireless/ath/ath9k/debug.h |    1 +
>   2 files changed, 90 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
> index 8d7c046..29898f8 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.c
> +++ b/drivers/net/wireless/ath/ath9k/debug.c
> @@ -795,6 +795,86 @@ static const struct file_operations fops_recv = {
>   	.owner = THIS_MODULE
>   };
>
> +static ssize_t read_file_regidx(struct file *file, char __user *user_buf,
> +                                size_t count, loff_t *ppos)
> +{
> +	struct ath_softc *sc = file->private_data;
> +	char buf[32];
> +	unsigned int len;
> +
> +	len = snprintf(buf, sizeof(buf), "0x%08x\n", sc->debug.regidx);
> +	return simple_read_from_buffer(user_buf, count, ppos, buf, len);
> +}
> +
> +static ssize_t write_file_regidx(struct file *file, const char __user *user_buf,
> +			     size_t count, loff_t *ppos)
> +{
> +	struct ath_softc *sc = file->private_data;
> +	unsigned long regidx;
> +	char buf[32];
> +	ssize_t len;
> +
> +	len = min(count, sizeof(buf) - 1);
> +	if (copy_from_user(buf, user_buf, len))
> +		return -EINVAL;
> +
> +	buf[len] = '\0';
> +	if (strict_strtoul(buf, 0,&regidx))
> +		return -EINVAL;
> +
> +	sc->debug.regidx = regidx;
> +	return count;
> +}
> +
> +static const struct file_operations fops_regidx = {
> +	.read = read_file_regidx,
> +	.write = write_file_regidx,
> +	.open = ath9k_debugfs_open,
> +	.owner = THIS_MODULE
> +};
> +
> +static ssize_t read_file_regval(struct file *file, char __user *user_buf,
> +			     size_t count, loff_t *ppos)
> +{
> +	struct ath_softc *sc = file->private_data;
> +	struct ath_hw *ah = sc->sc_ah;
> +	char buf[32];
> +	unsigned int len;
> +	u32 regval;
> +
> +	regval = REG_READ_D(ah, sc->debug.regidx);
> +	len = snprintf(buf, sizeof(buf), "0x%08x\n", regval);
> +	return simple_read_from_buffer(user_buf, count, ppos, buf, len);
> +}
> +
> +static ssize_t write_file_regval(struct file *file, const char __user *user_buf,
> +			     size_t count, loff_t *ppos)
> +{
> +	struct ath_softc *sc = file->private_data;
> +	struct ath_hw *ah = sc->sc_ah;
> +	unsigned long regval;
> +	char buf[32];
> +	ssize_t len;
> +
> +	len = min(count, sizeof(buf) - 1);
> +	if (copy_from_user(buf, user_buf, len))
> +		return -EINVAL;
> +
> +	buf[len] = '\0';
> +	if (strict_strtoul(buf, 0,&regval))
> +		return -EINVAL;
> +
> +	REG_WRITE_D(ah, sc->debug.regidx, regval);
> +	return count;
> +}
> +
> +static const struct file_operations fops_regval = {
> +	.read = read_file_regval,
> +	.write = write_file_regval,
> +	.open = ath9k_debugfs_open,
> +	.owner = THIS_MODULE
> +};
> +
>   int ath9k_init_debug(struct ath_hw *ah)
>   {
>   	struct ath_common *common = ath9k_hw_common(ah);
> @@ -846,6 +926,15 @@ int ath9k_init_debug(struct ath_hw *ah)
>   			sc->debug.debugfs_phy, sc,&fops_tx_chainmask))
>   		goto err;
>
> +	if (!debugfs_create_file("regidx", S_IRUSR | S_IWUSR,
> +			sc->debug.debugfs_phy, sc,&fops_regidx))
> +		goto err;
> +
> +	if (!debugfs_create_file("regval", S_IRUSR | S_IWUSR,
> +			sc->debug.debugfs_phy, sc,&fops_regval))
> +		goto err;
> +
> +	sc->debug.regidx = 0;
>   	return 0;
>   err:
>   	ath9k_exit_debug(ah);
> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
> index 7314360..5147b87 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.h
> +++ b/drivers/net/wireless/ath/ath9k/debug.h
> @@ -153,6 +153,7 @@ struct ath_stats {
>
>   struct ath9k_debug {
>   	struct dentry *debugfs_phy;
> +	u32 regidx;
>   	struct ath_stats stats;
>   };
>


^ permalink raw reply

* Re: [RFC PATCH 1/2] mac80211: Add nl80211 antenna configuration
From: Luis R. Rodriguez @ 2010-05-12  1:51 UTC (permalink / raw)
  To: Bruno Randolf; +Cc: johannes, linville, linux-wireless, holgerschurig
In-Reply-To: <201005121039.32882.br1@einfach.org>

On Tue, May 11, 2010 at 6:39 PM, Bruno Randolf <br1@einfach.org> wrote:
> On Wednesday 12 May 2010 03:14:30 Luis R. Rodriguez wrote:
>> Subject should be for cfg80211, not mac80211. In fact can you submit
>> the mac80211 stuff in a separate secondary patch? Some more comments
>> below.
>
> ok.
>
>> I think we should call this TX / RX chainmask given that with 802.11n
>> hardware this is what this is called.
>
> from the following discussion, i'll stick with antenna...
>
>> > The antenna configuration is defined as a bitmap of allowed antennas.
>> > This bitmap is 8 bit at the moment, each bit representing one antenna.
>>
>> If you use chainmask for this instead of 'antenna configuration' the
>> wording would be something like:
>>
>> The chainmask is defined as a bitmap of chain configurations used for
>> TX/RX. The bitmap allows for configuring up to up to 4 chains for both TX
>> and RX, 4 bits for each TX chain, 4 bits for each RX chain.
>
> actually we have 8 bit for TX and 8 bit for RX.
>
>> > If multiple
>> > antennas are selected, the driver may use diversity for receive and
>> > transmit.
>>
>> For 802.11n this is called "selection diversity" but typically just
>> referred to as "diversity", for legacy this is called "antenna
>> diversity". It may be good to elaborate how selection diversity or
>> antenna diversity might be enabled, ie, will this be another command,
>> or what. I think for legacy another command makes sense, and it may be
>> possible for us to use the same command for enabling selection
>> diversity, I am not sure if we can fine tune the diversity algorithm
>> at this time, I will have to review this and get back to you.
>
> my idea was that if multiple antennas are selected in the bitmap this means
> that "antenna diversity" will be enabled. for RX this is clear. for TX this is
> a bit ambigous: it could mean "send on both antennas", which i believe is
> impossible on legacy hardware, so it means "let HW diversity choose TX
> antenna". for 802.11n - i think the TX part is different, because multiple TX
> chains will mean "send on multiple antennas". but otherwise can't we think of
> it as some form of advanced antenna diversity?

802.11n has chains but is also allows two antennas to be assigned to
one chain and the hardware then would do something equivalent to
legacy as with antenna diversity for one chain but it is called
"selection diversity" for 802.11n.

> of course there are more possibillities of antenna configurations which cannot
> be configured with this api - like the "multiple sector antennas + RTS/CTS on
> omni" configuration of ath5k, but i don't think these are very common.

Yeah I was reminded of these setups today as well, but as you note it
is rare and the defaults should typically work on those.

> the most important use case, imho, is to limit the antennas to one fixed
> antenna, if we know only one antenna is connected. this might well apply for
> 802.11n too, no?

802.11n does allow for 1 stream devices with a 1x1 TX/RX chain setup
but the hardware *can* have two antennas like your typical legacy
device with diversity enabled. This is the case for AR9285, for
example.

 Luis

^ permalink raw reply

* Re: [PATCH v2 1/2] mac80211: add offload channel switch support
From: Guy, Wey-Yi @ 2010-05-12  2:48 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Johannes Berg, David Quan, Michael Green, Stephen Chen, Dan Tian,
	Kevin Hayes, Cliff Holden, linux-wireless@vger.kernel.org
In-Reply-To: <AANLkTikKTOOV0NA53oVcr4ltb2zon1Tu5j1iR1p9lg5a@mail.gmail.com>

Hi Luis,

On Tue, 2010-05-11 at 13:55 -0700, Luis R. Rodriguez wrote:
> On Tue, May 11, 2010 at 7:00 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Thu, 2010-05-06 at 14:58 -0700, Luis R. Rodriguez wrote:
> >> On Thu, May 6, 2010 at 8:25 AM,  <wey-yi.w.guy@intel.com> wrote:
> >> > From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> 
> >> > with this offload approach, driver has more control on how to handle the
> >> > channel switch request from AP, also can provide more accurate timing
> >> > calculation
> >>
> >> Is the current timing insufficient, and if so can you provide more
> >> details. If the real reason for this callback is not timing
> >> considerations is the real reason a firmware API thing? If so it
> >> doesn't hurt to just say that to avoid confusing developer in deciding
> >> which approach to take.
> >
> > The current mac80211 approach is flawed, for various reasons which I
> > won't get into here, most of which we can fix. However, due to
> > regulatory concerns our firmware also wants to have more control, like
> > checking that the AP is beaconing again after a channel switch before
> > letting us transmit frames.
> 
> OK this makes perfect sense if the channel you are switching to is
> also a DFS channel, but it sounds like something we can also implement
> on mac80211, unless of course firmware can make that guarantee for us
> quicker. This is the sort of explanation that I think might be very
> useful to the driver developer when choosing which mac80211 CSA
> operation to implement -- either the standard mac80211 implementation
> or a driver specific one.
> 
> I take it this tries to resolve some sort of race condition where the
> AP decided to switch to another DFS channel, told the STAs, went to
> the other channel, and as it does the swtich gets radar signals and
> needs to quite down.
> 
Just like Johannes mention, the current mac80211 approach is flawed and
it is possible to cause multiple problems(at least in iwlwifi driver),
this patch provide the option for driver to offload and take control the
operation if the device can handle the "channel switch" operation. BUt
it is still driver developer's option to choice which approach to use.

> > Timing is obviously also a consideration,
> > since the firmware can re-enable transmission quickly after the channel
> > switch, regardless of the delay in processing the frame or the timer on
> > the host.
> 
> Reason for me asking for details about this was because I was
> wondering whether the reasons for you guys implementing a separate CSA
> callback could be addressed by adjusting the internal existing
> mac80211 CSA implementation further. Timing constraints to re-enable
> TX seem to be the biggest concern here since checking whether or not
> the AP is beaconing *can* certainly be done on mac80211. Is there a
> measurable TX drop due to the extra latency introduced by using a host
> based implementation?
> 
We do not have any measurement being done at this time, Timing is one of
the reason to implement this patch, but it is not the main reason. 
On the other hand, if firmware can handle the timing, for sure it can be
less latency, so why not give the option for driver/firmware to handle
it?

> If a driver developer reads the documentation it would be nice for
> them to easily get enough information to decide which approach to take
> and to do that the more details we can provide the better. If we
> haven't tested a mac80211 enhanced approach why not try that first?
> 
Yes, I can provide better document, from my point of view, both options
are valid option; it is really driver developer's choice which one to
choice based on the firmware capabilities

> >> > The drivers like to support the channel switch offloading function
> >>
> >> Maybe: "The drivers that require a dedicated channel switch callback"...
> >>
> >> > will have
> >> > to provide the mactime information (at least for mgmt and action frames)
> >>
> >> Might be good to specify why, or at least in the documentation code below.
> >
> > Actually, it's up to them. But if you implement the callback, you'll
> > want to know precisely when to expect the channel switch
> 
> Right, that was my point, it was not clear that this was the reason
> for having it so it might help the developer if the documentation
> stated that.
> 
Sure

> > so you'll want
> > to know when the frame that contained the CSA was received, which you
> > have to provide to mac80211 in the "mactime" rx status field.
> 
> Right, thanks for the details, I was just hoping we could clarify that
> to the driver developers a little more on the patch.
> 
> >> > +/**
> >> > + * ieee80211_chswitch_done - Complete channel switch process
> >> > + * @vif: &struct ieee80211_vif pointer from the add_interface callback.
> >> > + * @is_seccess: make the channel switch successful or not
> >>
> >> Typo, is_seccess, not success. Also, I don't get what this is for, can
> >> you elaborate?
> >
> > Channel switching could fail, for instance if the AP doesn't show up on
> > the new channel. We don't have a way to handle that yet in mac80211, but
> > why not let it know.
> 
> Oh definitely I agree, I was just hoping this can be explained ont he
> kdoc above, it was not clear from reading the code.
> 
Agree, more detail document is better

> >> I'd appreciate more feedback on the why this is being done. Its not
> >> clear to me how we are limited by the current implementation.
> >
> > Ok like I said -- timing is a big thing. Regulatory enforcement in our
> > firmware is another.
> 
> Regulatory enforcement is already handled by the mac80211 CSA, the
> check for beaconing on the channel we move to *can* be done by
> mac80211 as well, so that would only leave timing constraints.
> 
 
The current mac80211 implement is not clean enough, 
it mix "channel switch" with all the other possibilities to change channels; 
it might cause protential confusion and issues (at least in iwlwifi driver); 
having separated callback is much cleaner approach. 

Thanks
Wey


^ permalink raw reply

* Re: [RFC PATCH 1/2] mac80211: Add nl80211 antenna configuration
From: Bruno Randolf @ 2010-05-12  1:39 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: johannes, linville, linux-wireless, holgerschurig
In-Reply-To: <AANLkTilowwAyqLrIJ0_mlZxnqY-TqSHM3xdXkbO8aH6-@mail.gmail.com>

On Wednesday 12 May 2010 03:14:30 Luis R. Rodriguez wrote:
> Subject should be for cfg80211, not mac80211. In fact can you submit
> the mac80211 stuff in a separate secondary patch? Some more comments
> below.

ok.

> I think we should call this TX / RX chainmask given that with 802.11n
> hardware this is what this is called.

from the following discussion, i'll stick with antenna...

> > The antenna configuration is defined as a bitmap of allowed antennas.
> > This bitmap is 8 bit at the moment, each bit representing one antenna.
> 
> If you use chainmask for this instead of 'antenna configuration' the
> wording would be something like:
> 
> The chainmask is defined as a bitmap of chain configurations used for
> TX/RX. The bitmap allows for configuring up to up to 4 chains for both TX
> and RX, 4 bits for each TX chain, 4 bits for each RX chain.

actually we have 8 bit for TX and 8 bit for RX.
 
> > If multiple
> > antennas are selected, the driver may use diversity for receive and
> > transmit.
> 
> For 802.11n this is called "selection diversity" but typically just
> referred to as "diversity", for legacy this is called "antenna
> diversity". It may be good to elaborate how selection diversity or
> antenna diversity might be enabled, ie, will this be another command,
> or what. I think for legacy another command makes sense, and it may be
> possible for us to use the same command for enabling selection
> diversity, I am not sure if we can fine tune the diversity algorithm
> at this time, I will have to review this and get back to you.

my idea was that if multiple antennas are selected in the bitmap this means 
that "antenna diversity" will be enabled. for RX this is clear. for TX this is 
a bit ambigous: it could mean "send on both antennas", which i believe is 
impossible on legacy hardware, so it means "let HW diversity choose TX 
antenna". for 802.11n - i think the TX part is different, because multiple TX 
chains will mean "send on multiple antennas". but otherwise can't we think of 
it as some form of advanced antenna diversity?

of course there are more possibillities of antenna configurations which cannot 
be configured with this api - like the "multiple sector antennas + RTS/CTS on 
omni" configuration of ath5k, but i don't think these are very common. 

the most important use case, imho, is to limit the antennas to one fixed 
antenna, if we know only one antenna is connected. this might well apply for 
802.11n too, no?

bruno

^ permalink raw reply

* Re: ar9170-fw II
From: Christian Lamparter @ 2010-05-11 23:27 UTC (permalink / raw)
  To: David H. Lynch Jr.; +Cc: linux-wireless
In-Reply-To: <4BE9D3DB.7040005@dlasys.net>

On Wednesday 12 May 2010 00:02:03 David H. Lynch Jr. wrote:
> On 05/02/2010 08:52 AM, Christian Lamparter wrote:
> > Well, then you have two more good reasons why to use carl9170:
> >   * carl9170 has the ability to store additional per-frame data.
> >     In fact, if you don't need to have a different retry rates
> >     you could realloc the 3 * 32 bit "rr" (as in retry rate)
> >     array in the carl9170_tx_superdesc and _carl9170_tx_superdesc
> >     struct (see wlan.h) for your purposes (storage for your time values).
> >
> >     And if you fetched all the data, everything will be sent
> >     with an ordinary tx status feedback report to the application
> >     (add the timer fields into carl9170_tx_status and _carl9170_tx_status
> >      struct - see fwcmd.h)
>      cmd.h __check() tests for sizeof(carl9170_tx_status) != 2
>      is there an assumption somewhere that carl9170_tx_status == 2 ?
good catch!

I have now replace some of those meaningless numerical values with
precalculated #defines *_SIZE.

This way, firmware and drivers can verify the structs sizes
and check whenever the compiler really did what it was supposed
to do.

Regards,
	Chr

^ permalink raw reply

* Re: ar9170-fw II
From: David H. Lynch Jr. @ 2010-05-11 22:02 UTC (permalink / raw)
  To: Christian Lamparter, linux-wireless
In-Reply-To: <201005021452.01101.chunkeey@googlemail.com>

On 05/02/2010 08:52 AM, Christian Lamparter wrote:
> Well, then you have two more good reasons why to use carl9170:
>   * carl9170 has the ability to store additional per-frame data.
>     In fact, if you don't need to have a different retry rates
>     you could realloc the 3 * 32 bit "rr" (as in retry rate)
>     array in the carl9170_tx_superdesc and _carl9170_tx_superdesc
>     struct (see wlan.h) for your purposes (storage for your time values).
>
>     And if you fetched all the data, everything will be sent
>     with an ordinary tx status feedback report to the application
>     (add the timer fields into carl9170_tx_status and _carl9170_tx_status
>      struct - see fwcmd.h)
     cmd.h __check() tests for sizeof(carl9170_tx_status) != 2
     is there an assumption somewhere that carl9170_tx_status == 2 ?


-- 
Dave Lynch 					  	    DLA Systems
Software Development:  				         Embedded Linux
717.587.7774 	       dhlii@dlasys.net 	  http://www.dlasys.net
Over 25 years' experience in platforms, languages, and technologies too numerous to list.

"Any intelligent fool can make things bigger and more complex... It takes a touch of genius - and a lot of courage to move in the opposite direction."
Albert Einstein


^ permalink raw reply

* [PATCH 5/9] rt2x00: Dump beacons under a different identifier than TX frames.
From: Gertjan van Wingerde @ 2010-05-11 21:51 UTC (permalink / raw)
  To: John W. Linville
  Cc: Ivo van Doorn, linux-wireless, users, Gertjan van Wingerde
In-Reply-To: <1273614705-25118-1-git-send-email-gwingerde@gmail.com>

This allows for specific identification of beacons in the debugfs
frame stream.
Preparation for later differences between dumped TX frames and dumped
beacons.

Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00dump.h  |    3 +++
 drivers/net/wireless/rt2x00/rt2x00queue.c |    5 ++++-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00dump.h b/drivers/net/wireless/rt2x00/rt2x00dump.h
index 727019a..ed303b4 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dump.h
+++ b/drivers/net/wireless/rt2x00/rt2x00dump.h
@@ -62,11 +62,14 @@
  *	the tx event which has either succeeded or failed. A frame
  *	with this type should also have been reported with as a
  *	%DUMP_FRAME_TX frame.
+ * @DUMP_FRAME_BEACON: This beacon frame is queued for transmission to the
+ *	hardware.
  */
 enum rt2x00_dump_type {
 	DUMP_FRAME_RXDONE = 1,
 	DUMP_FRAME_TX = 2,
 	DUMP_FRAME_TXDONE = 3,
+	DUMP_FRAME_BEACON = 4,
 };
 
 /**
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index 891d5f7..e5969a5 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -420,6 +420,7 @@ static void rt2x00queue_write_tx_descriptor(struct queue_entry *entry,
 {
 	struct data_queue *queue = entry->queue;
 	struct rt2x00_dev *rt2x00dev = queue->rt2x00dev;
+	enum rt2x00_dump_type;
 
 	rt2x00dev->ops->lib->write_tx_desc(rt2x00dev, entry->skb, txdesc);
 
@@ -427,7 +428,9 @@ static void rt2x00queue_write_tx_descriptor(struct queue_entry *entry,
 	 * All processing on the frame has been completed, this means
 	 * it is now ready to be dumped to userspace through debugfs.
 	 */
-	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_TX, entry->skb);
+	dump_type = (txdesc->queue == QID_BEACON) ?
+					DUMP_FRAME_BEACON : DUMP_FRAME_TX;
+	rt2x00debug_dump_frame(rt2x00dev, dump_type, entry->skb);
 }
 
 static void rt2x00queue_kick_tx_queue(struct queue_entry *entry,
-- 
1.7.1


^ permalink raw reply related

* [PATCH 8/9] rt2x00: Reverse calling order of bus write_tx_desc and driver write_tx_desc.
From: Gertjan van Wingerde @ 2010-05-11 21:51 UTC (permalink / raw)
  To: John W. Linville
  Cc: Ivo van Doorn, linux-wireless, users, Gertjan van Wingerde
In-Reply-To: <1273614705-25118-1-git-send-email-gwingerde@gmail.com>

For rt2800 reverse the calling order of rt2x00pci_write_data and
rt2800pci_write_data. Currently rt2800pci_write_data calls rt2x00pci_write_data
as there can be only 1 driver callback function specified by the driver.
Reverse this calling order by introducing a new driver callback function,
called add_tx_datadesc, which is called from the bus-specific write_tx_data
functions.
Preparation for futher cleanups in the skb data handling of rt2x00.

Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2800pci.c |   15 ++++-----------
 drivers/net/wireless/rt2x00/rt2x00.h    |    2 ++
 drivers/net/wireless/rt2x00/rt2x00pci.c |    6 ++++++
 drivers/net/wireless/rt2x00/rt2x00usb.c |    6 ++++++
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index fcca30c..bbd6481 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -614,18 +614,10 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
 /*
  * TX descriptor initialization
  */
-static int rt2800pci_write_tx_data(struct queue_entry* entry,
-				   struct txentry_desc *txdesc)
+static void rt2800pci_add_tx_datadesc(struct queue_entry* entry,
+				      struct txentry_desc *txdesc)
 {
-	int ret;
-
-	ret = rt2x00pci_write_tx_data(entry, txdesc);
-	if (ret)
-		return ret;
-
 	rt2800_write_txwi(entry->skb, txdesc);
-
-	return 0;
 }
 
 
@@ -1080,7 +1072,8 @@ static const struct rt2x00lib_ops rt2800pci_rt2x00_ops = {
 	.reset_tuner		= rt2800_reset_tuner,
 	.link_tuner		= rt2800_link_tuner,
 	.write_tx_desc		= rt2800pci_write_tx_desc,
-	.write_tx_data		= rt2800pci_write_tx_data,
+	.write_tx_data		= rt2x00pci_write_tx_data,
+	.add_tx_datadesc	= rt2800pci_add_tx_datadesc,
 	.write_beacon		= rt2800pci_write_beacon,
 	.kick_tx_queue		= rt2800pci_kick_tx_queue,
 	.kill_tx_queue		= rt2800pci_kill_tx_queue,
diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 6c1ff4c..5f2531f 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -551,6 +551,8 @@ struct rt2x00lib_ops {
 			       struct txentry_desc *txdesc);
 	int (*write_tx_data) (struct queue_entry *entry,
 			      struct txentry_desc *txdesc);
+	void (*add_tx_datadesc) (struct queue_entry *entry,
+				 struct txentry_desc *txdesc);
 	void (*write_beacon) (struct queue_entry *entry,
 			      struct txentry_desc *txdesc);
 	int (*get_tx_data_len) (struct queue_entry *entry);
diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c
index ff80ef7..cd61d6f 100644
--- a/drivers/net/wireless/rt2x00/rt2x00pci.c
+++ b/drivers/net/wireless/rt2x00/rt2x00pci.c
@@ -80,6 +80,12 @@ int rt2x00pci_write_tx_data(struct queue_entry *entry,
 		return -EINVAL;
 	}
 
+	/*
+	 * Call the driver's add_tx_datadesc function, if it exists.
+	 */
+	if (rt2x00dev->ops->lib->add_tx_datadesc)
+		rt2x00dev->ops->lib->add_tx_datadesc(entry, txdesc);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rt2x00pci_write_tx_data);
diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index a4f0551..e731389 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -247,6 +247,12 @@ int rt2x00usb_write_tx_data(struct queue_entry *entry,
 	 */
 	skb_pull(entry->skb, entry->queue->desc_size);
 
+	/*
+	 * Call the driver's add_tx_datadesc function, if it exists.
+	 */
+	if (rt2x00dev->ops->lib->add_tx_datadesc)
+		rt2x00dev->ops->lib->add_tx_datadesc(entry, txdesc);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rt2x00usb_write_tx_data);
-- 
1.7.1


^ permalink raw reply related

* [PATCH 6/9] rt2x00: Push beacon TX descriptor writing to drivers.
From: Gertjan van Wingerde @ 2010-05-11 21:51 UTC (permalink / raw)
  To: John W. Linville
  Cc: Ivo van Doorn, linux-wireless, users, Gertjan van Wingerde
In-Reply-To: <1273614705-25118-1-git-send-email-gwingerde@gmail.com>

Not all the devices require a TX descriptor to be written (i.e. rt2800
device don't require them). Push down the creation of the TX descriptor
to the device drivers so that they can decide for themselves whether
a TX descriptor is to be created.

Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2400pci.c   |   16 ++++++++++------
 drivers/net/wireless/rt2x00/rt2500pci.c   |   16 ++++++++++------
 drivers/net/wireless/rt2x00/rt2500usb.c   |   11 +++++++++++
 drivers/net/wireless/rt2x00/rt2800pci.c   |   17 +++++++++++++++++
 drivers/net/wireless/rt2x00/rt2800usb.c   |   17 +++++++++++++++++
 drivers/net/wireless/rt2x00/rt2x00debug.c |    1 +
 drivers/net/wireless/rt2x00/rt2x00queue.c |   10 +---------
 drivers/net/wireless/rt2x00/rt61pci.c     |   11 +++++++++++
 drivers/net/wireless/rt2x00/rt73usb.c     |   11 +++++++++++
 9 files changed, 89 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index def3fa4..741c531 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -33,6 +33,7 @@
 #include <linux/eeprom_93cx6.h>
 
 #include "rt2x00.h"
+#include "rt2x00lib.h"
 #include "rt2x00pci.h"
 #include "rt2400pci.h"
 
@@ -1074,9 +1075,6 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
 				   struct txentry_desc *txdesc)
 {
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
-	struct queue_entry_priv_pci *entry_priv = entry->priv_data;
-	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
-	u32 word;
 	u32 reg;
 
 	/*
@@ -1089,9 +1087,15 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
 
 	rt2x00queue_map_txskb(rt2x00dev, entry->skb);
 
-	rt2x00_desc_read(entry_priv->desc, 1, &word);
-	rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
-	rt2x00_desc_write(entry_priv->desc, 1, word);
+	/*
+	 * Write the TX descriptor for the beacon.
+	 */
+	rt2400pci_write_tx_desc(rt2x00dev, entry->skb, txdesc);
+
+	/*
+	 * Dump beacon to userspace through debugfs.
+	 */
+	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
 
 	/*
 	 * Enable beaconing again.
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 070c23e..4dc101e 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -33,6 +33,7 @@
 #include <linux/eeprom_93cx6.h>
 
 #include "rt2x00.h"
+#include "rt2x00lib.h"
 #include "rt2x00pci.h"
 #include "rt2500pci.h"
 
@@ -1231,9 +1232,6 @@ static void rt2500pci_write_beacon(struct queue_entry *entry,
 				   struct txentry_desc *txdesc)
 {
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
-	struct queue_entry_priv_pci *entry_priv = entry->priv_data;
-	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
-	u32 word;
 	u32 reg;
 
 	/*
@@ -1246,9 +1244,15 @@ static void rt2500pci_write_beacon(struct queue_entry *entry,
 
 	rt2x00queue_map_txskb(rt2x00dev, entry->skb);
 
-	rt2x00_desc_read(entry_priv->desc, 1, &word);
-	rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
-	rt2x00_desc_write(entry_priv->desc, 1, word);
+	/*
+	 * Write the TX descriptor for the beacon.
+	 */
+	rt2500pci_write_tx_desc(rt2x00dev, entry->skb, txdesc);
+
+	/*
+	 * Dump beacon to userspace through debugfs.
+	 */
+	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
 
 	/*
 	 * Enable beaconing again.
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index b985d8f..4911d1a 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -32,6 +32,7 @@
 #include <linux/usb.h>
 
 #include "rt2x00.h"
+#include "rt2x00lib.h"
 #include "rt2x00usb.h"
 #include "rt2500usb.h"
 
@@ -1107,6 +1108,16 @@ static void rt2500usb_write_beacon(struct queue_entry *entry,
 	rt2500usb_register_write(rt2x00dev, TXRX_CSR19, reg);
 
 	/*
+	 * Write the TX descriptor for the beacon.
+	 */
+	rt2500usb_write_tx_desc(rt2x00dev, entry->skb, txdesc);
+
+	/*
+	 * Dump beacon to userspace through debugfs.
+	 */
+	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
+
+	/*
 	 * Take the descriptor in front of the skb into account.
 	 */
 	skb_push(entry->skb, TXD_DESC_SIZE);
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index b2f2327..fcca30c 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -42,6 +42,7 @@
 #include <linux/eeprom_93cx6.h>
 
 #include "rt2x00.h"
+#include "rt2x00lib.h"
 #include "rt2x00pci.h"
 #include "rt2x00soc.h"
 #include "rt2800lib.h"
@@ -688,6 +689,7 @@ static void rt2800pci_write_beacon(struct queue_entry *entry,
 				   struct txentry_desc *txdesc)
 {
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
+	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 	unsigned int beacon_base;
 	u32 reg;
 
@@ -700,9 +702,24 @@ static void rt2800pci_write_beacon(struct queue_entry *entry,
 	rt2800_register_write(rt2x00dev, BCN_TIME_CFG, reg);
 
 	/*
+	 * Register descriptor details in skb frame descriptor.
+	 */
+	skbdesc->desc = entry->skb->data - TXWI_DESC_SIZE;
+	skbdesc->desc_len = TXWI_DESC_SIZE;
+
+	/*
 	 * Add the TXWI for the beacon to the skb.
 	 */
 	rt2800_write_txwi(entry->skb, txdesc);
+
+	/*
+	 * Dump beacon to userspace through debugfs.
+	 */
+	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
+
+	/*
+	 * Adjust skb to take TXWI into account.
+	 */
 	skb_push(entry->skb, TXWI_DESC_SIZE);
 
 	/*
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 1b87daa..9a29f73 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -37,6 +37,7 @@
 #include <linux/usb.h>
 
 #include "rt2x00.h"
+#include "rt2x00lib.h"
 #include "rt2x00usb.h"
 #include "rt2800lib.h"
 #include "rt2800.h"
@@ -437,6 +438,7 @@ static void rt2800usb_write_beacon(struct queue_entry *entry,
 				   struct txentry_desc *txdesc)
 {
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
+	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 	unsigned int beacon_base;
 	u32 reg;
 
@@ -449,9 +451,24 @@ static void rt2800usb_write_beacon(struct queue_entry *entry,
 	rt2800_register_write(rt2x00dev, BCN_TIME_CFG, reg);
 
 	/*
+	 * Register descriptor details in skb frame descriptor.
+	 */
+	skbdesc->desc = entry->skb->data - TXWI_DESC_SIZE;
+	skbdesc->desc_len = TXWI_DESC_SIZE;
+
+	/*
 	 * Add the TXWI for the beacon to the skb.
 	 */
 	rt2800_write_txwi(entry->skb, txdesc);
+
+	/*
+	 * Dump beacon to userspace through debugfs.
+	 */
+	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
+
+	/*
+	 * Adjust skb to take TXWI into account.
+	 */
 	skb_push(entry->skb, TXWI_DESC_SIZE);
 
 	/*
diff --git a/drivers/net/wireless/rt2x00/rt2x00debug.c b/drivers/net/wireless/rt2x00/rt2x00debug.c
index 9e2eed5..85e9990 100644
--- a/drivers/net/wireless/rt2x00/rt2x00debug.c
+++ b/drivers/net/wireless/rt2x00/rt2x00debug.c
@@ -204,6 +204,7 @@ void rt2x00debug_dump_frame(struct rt2x00_dev *rt2x00dev,
 	if (!test_bit(FRAME_DUMP_FILE_OPEN, &intf->frame_dump_flags))
 		skb_queue_purge(&intf->frame_dump_skbqueue);
 }
+EXPORT_SYMBOL_GPL(rt2x00debug_dump_frame);
 
 static int rt2x00debug_file_open(struct inode *inode, struct file *file)
 {
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index e5969a5..9e48bbc 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -420,7 +420,6 @@ static void rt2x00queue_write_tx_descriptor(struct queue_entry *entry,
 {
 	struct data_queue *queue = entry->queue;
 	struct rt2x00_dev *rt2x00dev = queue->rt2x00dev;
-	enum rt2x00_dump_type;
 
 	rt2x00dev->ops->lib->write_tx_desc(rt2x00dev, entry->skb, txdesc);
 
@@ -428,9 +427,7 @@ static void rt2x00queue_write_tx_descriptor(struct queue_entry *entry,
 	 * All processing on the frame has been completed, this means
 	 * it is now ready to be dumped to userspace through debugfs.
 	 */
-	dump_type = (txdesc->queue == QID_BEACON) ?
-					DUMP_FRAME_BEACON : DUMP_FRAME_TX;
-	rt2x00debug_dump_frame(rt2x00dev, dump_type, entry->skb);
+	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_TX, entry->skb);
 }
 
 static void rt2x00queue_kick_tx_queue(struct queue_entry *entry,
@@ -594,11 +591,6 @@ int rt2x00queue_update_beacon(struct rt2x00_dev *rt2x00dev,
 	skbdesc->entry = intf->beacon;
 
 	/*
-	 * Write TX descriptor into reserved room in front of the beacon.
-	 */
-	rt2x00queue_write_tx_descriptor(intf->beacon, &txdesc);
-
-	/*
 	 * Send beacon to hardware and enable beacon genaration..
 	 */
 	rt2x00dev->ops->lib->write_beacon(intf->beacon, &txdesc);
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 1be1d7d..cf9d507 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -34,6 +34,7 @@
 #include <linux/eeprom_93cx6.h>
 
 #include "rt2x00.h"
+#include "rt2x00lib.h"
 #include "rt2x00pci.h"
 #include "rt61pci.h"
 
@@ -1872,6 +1873,16 @@ static void rt61pci_write_beacon(struct queue_entry *entry,
 	rt2x00pci_register_write(rt2x00dev, TXRX_CSR9, reg);
 
 	/*
+	 * Write the TX descriptor for the beacon.
+	 */
+	rt61pci_write_tx_desc(rt2x00dev, entry->skb, txdesc);
+
+	/*
+	 * Dump beacon to userspace through debugfs.
+	 */
+	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
+
+	/*
 	 * Write entire beacon with descriptor to register.
 	 */
 	beacon_base = HW_BEACON_OFFSET(entry->entry_idx);
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index fca661c..085f76e 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -33,6 +33,7 @@
 #include <linux/usb.h>
 
 #include "rt2x00.h"
+#include "rt2x00lib.h"
 #include "rt2x00usb.h"
 #include "rt73usb.h"
 
@@ -1526,6 +1527,16 @@ static void rt73usb_write_beacon(struct queue_entry *entry,
 	rt2x00usb_register_write(rt2x00dev, TXRX_CSR9, reg);
 
 	/*
+	 * Write the TX descriptor for the beacon.
+	 */
+	rt73usb_write_tx_desc(rt2x00dev, entry->skb, txdesc);
+
+	/*
+	 * Dump beacon to userspace through debugfs.
+	 */
+	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
+
+	/*
 	 * Take the descriptor in front of the skb into account.
 	 */
 	skb_push(entry->skb, TXD_DESC_SIZE);
-- 
1.7.1


^ permalink raw reply related

* [PATCH 4/9] rt2x00: Simplify TXD handling of beacons.
From: Gertjan van Wingerde @ 2010-05-11 21:51 UTC (permalink / raw)
  To: John W. Linville
  Cc: Ivo van Doorn, linux-wireless, users, Gertjan van Wingerde
In-Reply-To: <1273614705-25118-1-git-send-email-gwingerde@gmail.com>

The handling of tx descriptors for beacons can be simplified by updating
write_tx_desc implementations of each driver to write directly to the
queue entry descriptor instead of to a provided memory area.
This is also a preparation for further clean ups where descriptors are
properly reserved in the skb instead of fiddling with the skb data
pointer.

Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2400pci.c   |   21 +++++++++------------
 drivers/net/wireless/rt2x00/rt2500pci.c   |   21 +++++++++------------
 drivers/net/wireless/rt2x00/rt2500usb.c   |   21 ++++++++++++---------
 drivers/net/wireless/rt2x00/rt2800pci.c   |   14 ++++++++++----
 drivers/net/wireless/rt2x00/rt2800usb.c   |    8 +++++++-
 drivers/net/wireless/rt2x00/rt2x00pci.c   |    9 ---------
 drivers/net/wireless/rt2x00/rt2x00queue.c |   10 ----------
 drivers/net/wireless/rt2x00/rt2x00usb.c   |    8 --------
 drivers/net/wireless/rt2x00/rt61pci.c     |   20 +++++++++++++-------
 drivers/net/wireless/rt2x00/rt73usb.c     |   21 ++++++++++++---------
 10 files changed, 72 insertions(+), 81 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 1eff6ec..def3fa4 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -1006,15 +1006,15 @@ static void rt2400pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
 	struct queue_entry_priv_pci *entry_priv = skbdesc->entry->priv_data;
-	__le32 *txd = skbdesc->desc;
+	__le32 *txd = entry_priv->desc;
 	u32 word;
 
 	/*
 	 * Start writing the descriptor words.
 	 */
-	rt2x00_desc_read(entry_priv->desc, 1, &word);
+	rt2x00_desc_read(txd, 1, &word);
 	rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
-	rt2x00_desc_write(entry_priv->desc, 1, word);
+	rt2x00_desc_write(txd, 1, word);
 
 	rt2x00_desc_read(txd, 2, &word);
 	rt2x00_set_field32(&word, TXD_W2_BUFFER_LENGTH, txdesc->length);
@@ -1059,6 +1059,12 @@ static void rt2400pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 	rt2x00_set_field32(&word, TXD_W0_RETRY_MODE,
 			   test_bit(ENTRY_TXD_RETRY_MODE, &txdesc->flags));
 	rt2x00_desc_write(txd, 0, word);
+
+	/*
+	 * Register descriptor details in skb frame descriptor.
+	 */
+	skbdesc->desc = txd;
+	skbdesc->desc_len = TXD_DESC_SIZE;
 }
 
 /*
@@ -1081,15 +1087,6 @@ static void rt2400pci_write_beacon(struct queue_entry *entry,
 	rt2x00_set_field32(&reg, CSR14_BEACON_GEN, 0);
 	rt2x00pci_register_write(rt2x00dev, CSR14, reg);
 
-	/*
-	 * Replace rt2x00lib allocated descriptor with the
-	 * pointer to the _real_ hardware descriptor.
-	 * After that, map the beacon to DMA and update the
-	 * descriptor.
-	 */
-	memcpy(entry_priv->desc, skbdesc->desc, skbdesc->desc_len);
-	skbdesc->desc = entry_priv->desc;
-
 	rt2x00queue_map_txskb(rt2x00dev, entry->skb);
 
 	rt2x00_desc_read(entry_priv->desc, 1, &word);
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 8d3e95e..070c23e 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -1164,15 +1164,15 @@ static void rt2500pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
 	struct queue_entry_priv_pci *entry_priv = skbdesc->entry->priv_data;
-	__le32 *txd = skbdesc->desc;
+	__le32 *txd = entry_priv->desc;
 	u32 word;
 
 	/*
 	 * Start writing the descriptor words.
 	 */
-	rt2x00_desc_read(entry_priv->desc, 1, &word);
+	rt2x00_desc_read(txd, 1, &word);
 	rt2x00_set_field32(&word, TXD_W1_BUFFER_ADDRESS, skbdesc->skb_dma);
-	rt2x00_desc_write(entry_priv->desc, 1, word);
+	rt2x00_desc_write(txd, 1, word);
 
 	rt2x00_desc_read(txd, 2, &word);
 	rt2x00_set_field32(&word, TXD_W2_IV_OFFSET, IEEE80211_HEADER);
@@ -1216,6 +1216,12 @@ static void rt2500pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 	rt2x00_set_field32(&word, TXD_W0_DATABYTE_COUNT, txdesc->length);
 	rt2x00_set_field32(&word, TXD_W0_CIPHER_ALG, CIPHER_NONE);
 	rt2x00_desc_write(txd, 0, word);
+
+	/*
+	 * Register descriptor details in skb frame descriptor.
+	 */
+	skbdesc->desc = txd;
+	skbdesc->desc_len = TXD_DESC_SIZE;
 }
 
 /*
@@ -1238,15 +1244,6 @@ static void rt2500pci_write_beacon(struct queue_entry *entry,
 	rt2x00_set_field32(&reg, CSR14_BEACON_GEN, 0);
 	rt2x00pci_register_write(rt2x00dev, CSR14, reg);
 
-	/*
-	 * Replace rt2x00lib allocated descriptor with the
-	 * pointer to the _real_ hardware descriptor.
-	 * After that, map the beacon to DMA and update the
-	 * descriptor.
-	 */
-	memcpy(entry_priv->desc, skbdesc->desc, skbdesc->desc_len);
-	skbdesc->desc = entry_priv->desc;
-
 	rt2x00queue_map_txskb(rt2x00dev, entry->skb);
 
 	rt2x00_desc_read(entry_priv->desc, 1, &word);
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index 9a915be..b985d8f 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -1033,7 +1033,7 @@ static void rt2500usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 				    struct txentry_desc *txdesc)
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
-	__le32 *txd = skbdesc->desc;
+	__le32 *txd = (__le32 *)(skb->data - TXD_DESC_SIZE);
 	u32 word;
 
 	/*
@@ -1075,6 +1075,12 @@ static void rt2500usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 		_rt2x00_desc_write(txd, 3, skbdesc->iv[0]);
 		_rt2x00_desc_write(txd, 4, skbdesc->iv[1]);
 	}
+
+	/*
+	 * Register descriptor details in skb frame descriptor.
+	 */
+	skbdesc->desc = txd;
+	skbdesc->desc_len = TXD_DESC_SIZE;
 }
 
 /*
@@ -1088,19 +1094,11 @@ static void rt2500usb_write_beacon(struct queue_entry *entry,
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
 	struct usb_device *usb_dev = to_usb_device_intf(rt2x00dev->dev);
 	struct queue_entry_priv_usb_bcn *bcn_priv = entry->priv_data;
-	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 	int pipe = usb_sndbulkpipe(usb_dev, entry->queue->usb_endpoint);
 	int length;
 	u16 reg, reg0;
 
 	/*
-	 * Add the descriptor in front of the skb.
-	 */
-	skb_push(entry->skb, entry->queue->desc_size);
-	memcpy(entry->skb->data, skbdesc->desc, skbdesc->desc_len);
-	skbdesc->desc = entry->skb->data;
-
-	/*
 	 * Disable beaconing while we are reloading the beacon data,
 	 * otherwise we might be sending out invalid data.
 	 */
@@ -1109,6 +1107,11 @@ static void rt2500usb_write_beacon(struct queue_entry *entry,
 	rt2500usb_register_write(rt2x00dev, TXRX_CSR19, reg);
 
 	/*
+	 * Take the descriptor in front of the skb into account.
+	 */
+	skb_push(entry->skb, TXD_DESC_SIZE);
+
+	/*
 	 * USB devices cannot blindly pass the skb->len as the
 	 * length of the data to usb_fill_bulk_urb. Pass the skb
 	 * to the driver to determine what the length should be.
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 7d4778d..b2f2327 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -633,7 +633,8 @@ static void rt2800pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 				    struct txentry_desc *txdesc)
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
-	__le32 *txd = skbdesc->desc;
+	struct queue_entry_priv_pci *entry_priv = skbdesc->entry->priv_data;
+	__le32 *txd = entry_priv->desc;
 	u32 word;
 
 	/*
@@ -657,15 +658,14 @@ static void rt2800pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 			   !test_bit(ENTRY_TXD_MORE_FRAG, &txdesc->flags));
 	rt2x00_set_field32(&word, TXD_W1_BURST,
 			   test_bit(ENTRY_TXD_BURST, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W1_SD_LEN0,
-			   rt2x00dev->ops->extra_tx_headroom);
+	rt2x00_set_field32(&word, TXD_W1_SD_LEN0, TXWI_DESC_SIZE);
 	rt2x00_set_field32(&word, TXD_W1_LAST_SEC0, 0);
 	rt2x00_set_field32(&word, TXD_W1_DMA_DONE, 0);
 	rt2x00_desc_write(txd, 1, word);
 
 	rt2x00_desc_read(txd, 2, &word);
 	rt2x00_set_field32(&word, TXD_W2_SD_PTR1,
-			   skbdesc->skb_dma + rt2x00dev->ops->extra_tx_headroom);
+			   skbdesc->skb_dma + TXWI_DESC_SIZE);
 	rt2x00_desc_write(txd, 2, word);
 
 	rt2x00_desc_read(txd, 3, &word);
@@ -673,6 +673,12 @@ static void rt2800pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 			   !test_bit(ENTRY_TXD_ENCRYPT_IV, &txdesc->flags));
 	rt2x00_set_field32(&word, TXD_W3_QSEL, 2);
 	rt2x00_desc_write(txd, 3, word);
+
+	/*
+	 * Register descriptor details in skb frame descriptor.
+	 */
+	skbdesc->desc = txd;
+	skbdesc->desc_len = TXD_DESC_SIZE;
 }
 
 /*
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index d48d705..1b87daa 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -400,7 +400,7 @@ static void rt2800usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 				    struct txentry_desc *txdesc)
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
-	__le32 *txi = skbdesc->desc;
+	__le32 *txi = (__le32 *)(skb->data - TXWI_DESC_SIZE - TXINFO_DESC_SIZE);
 	u32 word;
 
 	/*
@@ -422,6 +422,12 @@ static void rt2800usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 	rt2x00_set_field32(&word, TXINFO_W0_USB_DMA_TX_BURST,
 			   test_bit(ENTRY_TXD_BURST, &txdesc->flags));
 	rt2x00_desc_write(txi, 0, word);
+
+	/*
+	 * Register descriptor details in skb frame descriptor.
+	 */
+	skbdesc->desc = txi;
+	skbdesc->desc_len = TXINFO_DESC_SIZE + TXWI_DESC_SIZE;
 }
 
 /*
diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c
index 2fe9f29..ff80ef7 100644
--- a/drivers/net/wireless/rt2x00/rt2x00pci.c
+++ b/drivers/net/wireless/rt2x00/rt2x00pci.c
@@ -66,8 +66,6 @@ int rt2x00pci_write_tx_data(struct queue_entry *entry,
 			    struct txentry_desc *txdesc)
 {
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
-	struct queue_entry_priv_pci *entry_priv = entry->priv_data;
-	struct skb_frame_desc *skbdesc;
 
 	/*
 	 * This should not happen, we already checked the entry
@@ -82,13 +80,6 @@ int rt2x00pci_write_tx_data(struct queue_entry *entry,
 		return -EINVAL;
 	}
 
-	/*
-	 * Fill in skb descriptor
-	 */
-	skbdesc = get_skb_frame_desc(entry->skb);
-	skbdesc->desc = entry_priv->desc;
-	skbdesc->desc_len = entry->queue->desc_size;
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rt2x00pci_write_tx_data);
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index 97b2c76..891d5f7 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -552,7 +552,6 @@ int rt2x00queue_update_beacon(struct rt2x00_dev *rt2x00dev,
 	struct rt2x00_intf *intf = vif_to_intf(vif);
 	struct skb_frame_desc *skbdesc;
 	struct txentry_desc txdesc;
-	__le32 desc[16];
 
 	if (unlikely(!intf->beacon))
 		return -ENOBUFS;
@@ -585,19 +584,10 @@ int rt2x00queue_update_beacon(struct rt2x00_dev *rt2x00dev,
 	rt2x00queue_create_tx_descriptor(intf->beacon, &txdesc);
 
 	/*
-	 * For the descriptor we use a local array from where the
-	 * driver can move it to the correct location required for
-	 * the hardware.
-	 */
-	memset(desc, 0, sizeof(desc));
-
-	/*
 	 * Fill in skb descriptor
 	 */
 	skbdesc = get_skb_frame_desc(intf->beacon->skb);
 	memset(skbdesc, 0, sizeof(*skbdesc));
-	skbdesc->desc = desc;
-	skbdesc->desc_len = intf->beacon->queue->desc_size;
 	skbdesc->entry = intf->beacon;
 
 	/*
diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index acf3282..a4f0551 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -221,7 +221,6 @@ int rt2x00usb_write_tx_data(struct queue_entry *entry,
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
 	struct usb_device *usb_dev = to_usb_device_intf(rt2x00dev->dev);
 	struct queue_entry_priv_usb *entry_priv = entry->priv_data;
-	struct skb_frame_desc *skbdesc;
 	u32 length;
 
 	/*
@@ -231,13 +230,6 @@ int rt2x00usb_write_tx_data(struct queue_entry *entry,
 	memset(entry->skb->data, 0, entry->queue->desc_size);
 
 	/*
-	 * Fill in skb descriptor
-	 */
-	skbdesc = get_skb_frame_desc(entry->skb);
-	skbdesc->desc = entry->skb->data;
-	skbdesc->desc_len = entry->queue->desc_size;
-
-	/*
 	 * USB devices cannot blindly pass the skb->len as the
 	 * length of the data to usb_fill_bulk_urb. Pass the skb
 	 * to the driver to determine what the length should be.
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 55aa010..1be1d7d 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -1763,7 +1763,8 @@ static void rt61pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 				  struct txentry_desc *txdesc)
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
-	__le32 *txd = skbdesc->desc;
+	struct queue_entry_priv_pci *entry_priv = skbdesc->entry->priv_data;
+	__le32 *txd = entry_priv->desc;
 	u32 word;
 
 	/*
@@ -1842,6 +1843,13 @@ static void rt61pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 			   test_bit(ENTRY_TXD_BURST, &txdesc->flags));
 	rt2x00_set_field32(&word, TXD_W0_CIPHER_ALG, txdesc->cipher);
 	rt2x00_desc_write(txd, 0, word);
+
+	/*
+	 * Register descriptor details in skb frame descriptor.
+	 */
+	skbdesc->desc = txd;
+	skbdesc->desc_len =
+		(txdesc->queue == QID_BEACON) ?  TXINFO_SIZE : TXD_DESC_SIZE;
 }
 
 /*
@@ -1851,7 +1859,7 @@ static void rt61pci_write_beacon(struct queue_entry *entry,
 				 struct txentry_desc *txdesc)
 {
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
-	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
+	struct queue_entry_priv_pci *entry_priv = entry->priv_data;
 	unsigned int beacon_base;
 	u32 reg;
 
@@ -1867,11 +1875,9 @@ static void rt61pci_write_beacon(struct queue_entry *entry,
 	 * Write entire beacon with descriptor to register.
 	 */
 	beacon_base = HW_BEACON_OFFSET(entry->entry_idx);
-	rt2x00pci_register_multiwrite(rt2x00dev,
-				      beacon_base,
-				      skbdesc->desc, skbdesc->desc_len);
-	rt2x00pci_register_multiwrite(rt2x00dev,
-				      beacon_base + skbdesc->desc_len,
+	rt2x00pci_register_multiwrite(rt2x00dev, beacon_base,
+				      entry_priv->desc, TXINFO_SIZE);
+	rt2x00pci_register_multiwrite(rt2x00dev, beacon_base + TXINFO_SIZE,
 				      entry->skb->data, entry->skb->len);
 
 	/*
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index fa40d43..fca661c 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -1440,7 +1440,7 @@ static void rt73usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 				  struct txentry_desc *txdesc)
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
-	__le32 *txd = skbdesc->desc;
+	__le32 *txd = (__le32 *)(skb->data - TXD_DESC_SIZE);
 	u32 word;
 
 	/*
@@ -1499,6 +1499,12 @@ static void rt73usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 			   TXPOWER_TO_DEV(rt2x00dev->tx_power));
 	rt2x00_set_field32(&word, TXD_W5_WAITING_DMA_DONE_INT, 1);
 	rt2x00_desc_write(txd, 5, word);
+
+	/*
+	 * Register descriptor details in skb frame descriptor.
+	 */
+	skbdesc->desc = txd;
+	skbdesc->desc_len = TXD_DESC_SIZE;
 }
 
 /*
@@ -1508,18 +1514,10 @@ static void rt73usb_write_beacon(struct queue_entry *entry,
 				 struct txentry_desc *txdesc)
 {
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
-	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
 	unsigned int beacon_base;
 	u32 reg;
 
 	/*
-	 * Add the descriptor in front of the skb.
-	 */
-	skb_push(entry->skb, entry->queue->desc_size);
-	memcpy(entry->skb->data, skbdesc->desc, skbdesc->desc_len);
-	skbdesc->desc = entry->skb->data;
-
-	/*
 	 * Disable beaconing while we are reloading the beacon data,
 	 * otherwise we might be sending out invalid data.
 	 */
@@ -1528,6 +1526,11 @@ static void rt73usb_write_beacon(struct queue_entry *entry,
 	rt2x00usb_register_write(rt2x00dev, TXRX_CSR9, reg);
 
 	/*
+	 * Take the descriptor in front of the skb into account.
+	 */
+	skb_push(entry->skb, TXD_DESC_SIZE);
+
+	/*
 	 * Write entire beacon with descriptor to register.
 	 */
 	beacon_base = HW_BEACON_OFFSET(entry->entry_idx);
-- 
1.7.1


^ permalink raw reply related

* [PATCH 9/9] rt2x00: Properly reserve room for descriptors in skbs.
From: Gertjan van Wingerde @ 2010-05-11 21:51 UTC (permalink / raw)
  To: John W. Linville
  Cc: Ivo van Doorn, linux-wireless, users, Gertjan van Wingerde
In-Reply-To: <1273614705-25118-1-git-send-email-gwingerde@gmail.com>

Instead of fiddling with the skb->data pointer and thereby risking
out of bounds accesses, properly reserve the space needed in an
skb for descriptors.

Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2400pci.c   |    2 +-
 drivers/net/wireless/rt2x00/rt2500pci.c   |    2 +-
 drivers/net/wireless/rt2x00/rt2500usb.c   |   14 ++++++----
 drivers/net/wireless/rt2x00/rt2800lib.c   |    3 +-
 drivers/net/wireless/rt2x00/rt2800lib.h   |    2 +-
 drivers/net/wireless/rt2x00/rt2800pci.c   |   23 ++++++++--------
 drivers/net/wireless/rt2x00/rt2800usb.c   |   22 +++++++++------
 drivers/net/wireless/rt2x00/rt2x00.h      |    7 +++++
 drivers/net/wireless/rt2x00/rt2x00dev.c   |    5 ---
 drivers/net/wireless/rt2x00/rt2x00lib.h   |    7 -----
 drivers/net/wireless/rt2x00/rt2x00pci.c   |   40 +++++++++++++++++++++++++++++
 drivers/net/wireless/rt2x00/rt2x00pci.h   |    8 ++++++
 drivers/net/wireless/rt2x00/rt2x00queue.c |   24 +----------------
 drivers/net/wireless/rt2x00/rt2x00usb.c   |   11 +++----
 drivers/net/wireless/rt2x00/rt61pci.c     |    4 +-
 drivers/net/wireless/rt2x00/rt73usb.c     |   14 ++++++----
 16 files changed, 109 insertions(+), 79 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 741c531..38ee78b 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -1228,7 +1228,7 @@ static void rt2400pci_txdone(struct rt2x00_dev *rt2x00dev,
 		}
 		txdesc.retry = rt2x00_get_field32(word, TXD_W0_RETRY_COUNT);
 
-		rt2x00lib_txdone(entry, &txdesc);
+		rt2x00pci_txdone(entry, &txdesc);
 	}
 }
 
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 4dc101e..cc7091c 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -1364,7 +1364,7 @@ static void rt2500pci_txdone(struct rt2x00_dev *rt2x00dev,
 		}
 		txdesc.retry = rt2x00_get_field32(word, TXD_W0_RETRY_COUNT);
 
-		rt2x00lib_txdone(entry, &txdesc);
+		rt2x00pci_txdone(entry, &txdesc);
 	}
 }
 
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index 4911d1a..b6ccb57 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -1034,7 +1034,7 @@ static void rt2500usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 				    struct txentry_desc *txdesc)
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
-	__le32 *txd = (__le32 *)(skb->data - TXD_DESC_SIZE);
+	__le32 *txd = (__le32 *) skb->data;
 	u32 word;
 
 	/*
@@ -1080,6 +1080,7 @@ static void rt2500usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 	/*
 	 * Register descriptor details in skb frame descriptor.
 	 */
+	skbdesc->flags |= SKBDESC_DESC_IN_SKB;
 	skbdesc->desc = txd;
 	skbdesc->desc_len = TXD_DESC_SIZE;
 }
@@ -1108,6 +1109,12 @@ static void rt2500usb_write_beacon(struct queue_entry *entry,
 	rt2500usb_register_write(rt2x00dev, TXRX_CSR19, reg);
 
 	/*
+	 * Add space for the descriptor in front of the skb.
+	 */
+	skb_push(entry->skb, TXD_DESC_SIZE);
+	memset(entry->skb->data, 0, TXD_DESC_SIZE);
+
+	/*
 	 * Write the TX descriptor for the beacon.
 	 */
 	rt2500usb_write_tx_desc(rt2x00dev, entry->skb, txdesc);
@@ -1118,11 +1125,6 @@ static void rt2500usb_write_beacon(struct queue_entry *entry,
 	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
 
 	/*
-	 * Take the descriptor in front of the skb into account.
-	 */
-	skb_push(entry->skb, TXD_DESC_SIZE);
-
-	/*
 	 * USB devices cannot blindly pass the skb->len as the
 	 * length of the data to usb_fill_bulk_urb. Pass the skb
 	 * to the driver to determine what the length should be.
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 7410ac1..53a3257 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -281,9 +281,8 @@ int rt2800_wait_wpdma_ready(struct rt2x00_dev *rt2x00dev)
 }
 EXPORT_SYMBOL_GPL(rt2800_wait_wpdma_ready);
 
-void rt2800_write_txwi(struct sk_buff *skb, struct txentry_desc *txdesc)
+void rt2800_write_txwi(__le32 *txwi, struct txentry_desc *txdesc)
 {
-	__le32 *txwi = (__le32 *)(skb->data - TXWI_DESC_SIZE);
 	u32 word;
 
 	/*
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.h b/drivers/net/wireless/rt2x00/rt2800lib.h
index 94de999..0f0a13c 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.h
+++ b/drivers/net/wireless/rt2x00/rt2800lib.h
@@ -111,7 +111,7 @@ void rt2800_mcu_request(struct rt2x00_dev *rt2x00dev,
 			const u8 command, const u8 token,
 			const u8 arg0, const u8 arg1);
 
-void rt2800_write_txwi(struct sk_buff *skb, struct txentry_desc *txdesc);
+void rt2800_write_txwi(__le32 *txwi, struct txentry_desc *txdesc);
 void rt2800_process_rxwi(struct sk_buff *skb, struct rxdone_entry_desc *txdesc);
 
 extern const struct rt2x00debug rt2800_rt2x00debug;
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index bbd6481..86f7042 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -617,7 +617,7 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
 static void rt2800pci_add_tx_datadesc(struct queue_entry* entry,
 				      struct txentry_desc *txdesc)
 {
-	rt2800_write_txwi(entry->skb, txdesc);
+	rt2800_write_txwi((__le32 *) entry->skb->data, txdesc);
 }
 
 
@@ -694,15 +694,22 @@ static void rt2800pci_write_beacon(struct queue_entry *entry,
 	rt2800_register_write(rt2x00dev, BCN_TIME_CFG, reg);
 
 	/*
+	 * Add space for the TXWI in front of the skb.
+	 */
+	skb_push(entry->skb, TXWI_DESC_SIZE);
+	memset(entry->skb, 0, TXWI_DESC_SIZE);
+
+	/*
 	 * Register descriptor details in skb frame descriptor.
 	 */
-	skbdesc->desc = entry->skb->data - TXWI_DESC_SIZE;
+	skbdesc->flags |= SKBDESC_DESC_IN_SKB;
+	skbdesc->desc = entry->skb->data;
 	skbdesc->desc_len = TXWI_DESC_SIZE;
 
 	/*
 	 * Add the TXWI for the beacon to the skb.
 	 */
-	rt2800_write_txwi(entry->skb, txdesc);
+	rt2800_write_txwi((__le32 *)entry->skb->data, txdesc);
 
 	/*
 	 * Dump beacon to userspace through debugfs.
@@ -710,11 +717,6 @@ static void rt2800pci_write_beacon(struct queue_entry *entry,
 	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
 
 	/*
-	 * Adjust skb to take TXWI into account.
-	 */
-	skb_push(entry->skb, TXWI_DESC_SIZE);
-
-	/*
 	 * Write entire beacon with TXWI to register.
 	 */
 	beacon_base = HW_BEACON_OFFSET(entry->entry_idx);
@@ -889,8 +891,7 @@ static void rt2800pci_txdone(struct rt2x00_dev *rt2x00dev)
 
 		/* Check if we got a match by looking at WCID/ACK/PID
 		 * fields */
-		txwi = (__le32 *)(entry->skb->data -
-				  rt2x00dev->ops->extra_tx_headroom);
+		txwi = (__le32 *) entry->skb->data;
 
 		rt2x00_desc_read(txwi, 1, &word);
 		tx_wcid = rt2x00_get_field32(word, TXWI_W1_WIRELESS_CLI_ID);
@@ -935,7 +936,7 @@ static void rt2800pci_txdone(struct rt2x00_dev *rt2x00dev)
 		__set_bit(TXDONE_FALLBACK, &txdesc.flags);
 
 
-		rt2x00lib_txdone(entry, &txdesc);
+		rt2x00pci_txdone(entry, &txdesc);
 	}
 }
 
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 9a29f73..a8e6e71 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -401,13 +401,14 @@ static void rt2800usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 				    struct txentry_desc *txdesc)
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
-	__le32 *txi = (__le32 *)(skb->data - TXWI_DESC_SIZE - TXINFO_DESC_SIZE);
+	__le32 *txi = (__le32 *) skb->data;
+	__le32 *txwi = (__le32 *) (skb->data + TXINFO_DESC_SIZE);
 	u32 word;
 
 	/*
 	 * Initialize TXWI descriptor
 	 */
-	rt2800_write_txwi(skb, txdesc);
+	rt2800_write_txwi(txwi, txdesc);
 
 	/*
 	 * Initialize TXINFO descriptor
@@ -427,6 +428,7 @@ static void rt2800usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 	/*
 	 * Register descriptor details in skb frame descriptor.
 	 */
+	skbdesc->flags |= SKBDESC_DESC_IN_SKB;
 	skbdesc->desc = txi;
 	skbdesc->desc_len = TXINFO_DESC_SIZE + TXWI_DESC_SIZE;
 }
@@ -451,15 +453,22 @@ static void rt2800usb_write_beacon(struct queue_entry *entry,
 	rt2800_register_write(rt2x00dev, BCN_TIME_CFG, reg);
 
 	/*
+	 * Add space for the TXWI in front of the skb.
+	 */
+	skb_push(entry->skb, TXWI_DESC_SIZE);
+	memset(entry->skb, 0, TXWI_DESC_SIZE);
+
+	/*
 	 * Register descriptor details in skb frame descriptor.
 	 */
-	skbdesc->desc = entry->skb->data - TXWI_DESC_SIZE;
+	skbdesc->flags |= SKBDESC_DESC_IN_SKB;
+	skbdesc->desc = entry->skb->data;
 	skbdesc->desc_len = TXWI_DESC_SIZE;
 
 	/*
 	 * Add the TXWI for the beacon to the skb.
 	 */
-	rt2800_write_txwi(entry->skb, txdesc);
+	rt2800_write_txwi((__le32 *) entry->skb->data, txdesc);
 
 	/*
 	 * Dump beacon to userspace through debugfs.
@@ -467,11 +476,6 @@ static void rt2800usb_write_beacon(struct queue_entry *entry,
 	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
 
 	/*
-	 * Adjust skb to take TXWI into account.
-	 */
-	skb_push(entry->skb, TXWI_DESC_SIZE);
-
-	/*
 	 * Write entire beacon with descriptor to register.
 	 */
 	beacon_base = HW_BEACON_OFFSET(entry->entry_idx);
diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 5f2531f..4cb9f87 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -1001,6 +1001,13 @@ static inline bool rt2x00_is_soc(struct rt2x00_dev *rt2x00dev)
 void rt2x00queue_map_txskb(struct rt2x00_dev *rt2x00dev, struct sk_buff *skb);
 
 /**
+ * rt2x00queue_unmap_skb - Unmap a skb from DMA.
+ * @rt2x00dev: Pointer to &struct rt2x00_dev.
+ * @skb: The skb to unmap.
+ */
+void rt2x00queue_unmap_skb(struct rt2x00_dev *rt2x00dev, struct sk_buff *skb);
+
+/**
  * rt2x00queue_get_queue - Convert queue index to queue pointer
  * @rt2x00dev: Pointer to &struct rt2x00_dev.
  * @queue: rt2x00 queue index (see &enum data_queue_qid).
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 33c2f5f..6790441 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -210,11 +210,6 @@ void rt2x00lib_txdone(struct queue_entry *entry,
 	bool success;
 
 	/*
-	 * Unmap the skb.
-	 */
-	rt2x00queue_unmap_skb(rt2x00dev, entry->skb);
-
-	/*
 	 * Remove L2 padding which was added during
 	 */
 	if (test_bit(DRIVER_REQUIRE_L2PAD, &rt2x00dev->flags))
diff --git a/drivers/net/wireless/rt2x00/rt2x00lib.h b/drivers/net/wireless/rt2x00/rt2x00lib.h
index be2e37f..2bffc51 100644
--- a/drivers/net/wireless/rt2x00/rt2x00lib.h
+++ b/drivers/net/wireless/rt2x00/rt2x00lib.h
@@ -107,13 +107,6 @@ struct sk_buff *rt2x00queue_alloc_rxskb(struct rt2x00_dev *rt2x00dev,
 					struct queue_entry *entry);
 
 /**
- * rt2x00queue_unmap_skb - Unmap a skb from DMA.
- * @rt2x00dev: Pointer to &struct rt2x00_dev.
- * @skb: The skb to unmap.
- */
-void rt2x00queue_unmap_skb(struct rt2x00_dev *rt2x00dev, struct sk_buff *skb);
-
-/**
  * rt2x00queue_free_skb - free a skb
  * @rt2x00dev: Pointer to &struct rt2x00_dev.
  * @skb: The skb to free.
diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c
index cd61d6f..e474629 100644
--- a/drivers/net/wireless/rt2x00/rt2x00pci.c
+++ b/drivers/net/wireless/rt2x00/rt2x00pci.c
@@ -81,11 +81,23 @@ int rt2x00pci_write_tx_data(struct queue_entry *entry,
 	}
 
 	/*
+	 * Add the requested extra tx headroom in front of the skb.
+	 */
+	skb_push(entry->skb, rt2x00dev->ops->extra_tx_headroom);
+	memset(entry->skb->data, 0, rt2x00dev->ops->extra_tx_headroom);
+
+	/*
 	 * Call the driver's add_tx_datadesc function, if it exists.
 	 */
 	if (rt2x00dev->ops->lib->add_tx_datadesc)
 		rt2x00dev->ops->lib->add_tx_datadesc(entry, txdesc);
 
+	/*
+	 * Map the skb to DMA.
+	 */
+	if (test_bit(DRIVER_REQUIRE_DMA, &rt2x00dev->flags))
+		rt2x00queue_map_txskb(rt2x00dev, entry->skb);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rt2x00pci_write_tx_data);
@@ -93,6 +105,34 @@ EXPORT_SYMBOL_GPL(rt2x00pci_write_tx_data);
 /*
  * TX/RX data handlers.
  */
+void rt2x00pci_txdone(struct queue_entry *entry,
+		      struct txdone_entry_desc *txdesc)
+{
+	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
+	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
+
+	/*
+	 * Unmap the skb.
+	 */
+	rt2x00queue_unmap_skb(rt2x00dev, entry->skb);
+
+	/*
+	 * Remove the extra tx headroom from the skb.
+	 */
+	skb_pull(entry->skb, rt2x00dev->ops->extra_tx_headroom);
+
+	/*
+	 * Signal that the TX descriptor is no longer in the skb.
+	 */
+	skbdesc->flags &= ~SKBDESC_DESC_IN_SKB;
+
+	/*
+	 * Pass on to rt2x00lib.
+	 */
+	rt2x00lib_txdone(entry, txdesc);
+}
+EXPORT_SYMBOL_GPL(rt2x00pci_txdone);
+
 void rt2x00pci_rxdone(struct rt2x00_dev *rt2x00dev)
 {
 	struct data_queue *queue = rt2x00dev->rx;
diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.h b/drivers/net/wireless/rt2x00/rt2x00pci.h
index 51bcef3..00528b8 100644
--- a/drivers/net/wireless/rt2x00/rt2x00pci.h
+++ b/drivers/net/wireless/rt2x00/rt2x00pci.h
@@ -109,6 +109,14 @@ struct queue_entry_priv_pci {
 };
 
 /**
+ * rt2x00pci_txdone - Handle TX done events.
+ * @entry: The queue entry for which a TX done event was received.
+ * @txdesc: The TX done descriptor for the entry.
+ */
+void rt2x00pci_txdone(struct queue_entry *entry,
+		      struct txdone_entry_desc *txdesc);
+
+/**
  * rt2x00pci_rxdone - Handle RX done events
  * @rt2x00dev: Device pointer, see &struct rt2x00_dev.
  */
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index 9e48bbc..bcdbf95 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -99,21 +99,8 @@ void rt2x00queue_map_txskb(struct rt2x00_dev *rt2x00dev, struct sk_buff *skb)
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
 
-	/*
-	 * If device has requested headroom, we should make sure that
-	 * is also mapped to the DMA so it can be used for transfering
-	 * additional descriptor information to the hardware.
-	 */
-	skb_push(skb, rt2x00dev->ops->extra_tx_headroom);
-
 	skbdesc->skb_dma =
 	    dma_map_single(rt2x00dev->dev, skb->data, skb->len, DMA_TO_DEVICE);
-
-	/*
-	 * Restore data pointer to original location again.
-	 */
-	skb_pull(skb, rt2x00dev->ops->extra_tx_headroom);
-
 	skbdesc->flags |= SKBDESC_DMA_MAPPED_TX;
 }
 EXPORT_SYMBOL_GPL(rt2x00queue_map_txskb);
@@ -129,16 +116,12 @@ void rt2x00queue_unmap_skb(struct rt2x00_dev *rt2x00dev, struct sk_buff *skb)
 	}
 
 	if (skbdesc->flags & SKBDESC_DMA_MAPPED_TX) {
-		/*
-		 * Add headroom to the skb length, it has been removed
-		 * by the driver, but it was actually mapped to DMA.
-		 */
-		dma_unmap_single(rt2x00dev->dev, skbdesc->skb_dma,
-				 skb->len + rt2x00dev->ops->extra_tx_headroom,
+		dma_unmap_single(rt2x00dev->dev, skbdesc->skb_dma, skb->len,
 				 DMA_TO_DEVICE);
 		skbdesc->flags &= ~SKBDESC_DMA_MAPPED_TX;
 	}
 }
+EXPORT_SYMBOL_GPL(rt2x00queue_unmap_skb);
 
 void rt2x00queue_free_skb(struct rt2x00_dev *rt2x00dev, struct sk_buff *skb)
 {
@@ -533,9 +516,6 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb,
 		return -EIO;
 	}
 
-	if (test_bit(DRIVER_REQUIRE_DMA, &queue->rt2x00dev->flags))
-		rt2x00queue_map_txskb(queue->rt2x00dev, skb);
-
 	set_bit(ENTRY_DATA_PENDING, &entry->flags);
 
 	rt2x00queue_index_inc(queue, Q_INDEX);
diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index e731389..9388e44 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -197,6 +197,11 @@ static void rt2x00usb_interrupt_txdone(struct urb *urb)
 		return;
 
 	/*
+	 * Remove the descriptor from the front of the skb.
+	 */
+	skb_pull(entry->skb, entry->queue->desc_size);
+
+	/*
 	 * Obtain the status about this packet.
 	 * Note that when the status is 0 it does not mean the
 	 * frame was send out correctly. It only means the frame
@@ -242,12 +247,6 @@ int rt2x00usb_write_tx_data(struct queue_entry *entry,
 			  rt2x00usb_interrupt_txdone, entry);
 
 	/*
-	 * Make sure the skb->data pointer points to the frame, not the
-	 * descriptor.
-	 */
-	skb_pull(entry->skb, entry->queue->desc_size);
-
-	/*
 	 * Call the driver's add_tx_datadesc function, if it exists.
 	 */
 	if (rt2x00dev->ops->lib->add_tx_datadesc)
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index cf9d507..5fd6481 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -2109,7 +2109,7 @@ static void rt61pci_txdone(struct rt2x00_dev *rt2x00dev)
 			__set_bit(TXDONE_UNKNOWN, &txdesc.flags);
 			txdesc.retry = 0;
 
-			rt2x00lib_txdone(entry_done, &txdesc);
+			rt2x00pci_txdone(entry_done, &txdesc);
 			entry_done = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
 		}
 
@@ -2129,7 +2129,7 @@ static void rt61pci_txdone(struct rt2x00_dev *rt2x00dev)
 		}
 		txdesc.retry = rt2x00_get_field32(reg, STA_CSR4_RETRY_COUNT);
 
-		rt2x00lib_txdone(entry, &txdesc);
+		rt2x00pci_txdone(entry, &txdesc);
 	}
 }
 
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index 085f76e..10893d5 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -1441,7 +1441,7 @@ static void rt73usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 				  struct txentry_desc *txdesc)
 {
 	struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
-	__le32 *txd = (__le32 *)(skb->data - TXD_DESC_SIZE);
+	__le32 *txd = (__le32 *) skb->data;
 	u32 word;
 
 	/*
@@ -1504,6 +1504,7 @@ static void rt73usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 	/*
 	 * Register descriptor details in skb frame descriptor.
 	 */
+	skbdesc->flags |= SKBDESC_DESC_IN_SKB;
 	skbdesc->desc = txd;
 	skbdesc->desc_len = TXD_DESC_SIZE;
 }
@@ -1527,6 +1528,12 @@ static void rt73usb_write_beacon(struct queue_entry *entry,
 	rt2x00usb_register_write(rt2x00dev, TXRX_CSR9, reg);
 
 	/*
+	 * Add space for the descriptor in front of the skb.
+	 */
+	skb_push(entry->skb, TXD_DESC_SIZE);
+	memset(entry->skb->data, 0, TXD_DESC_SIZE);
+
+	/*
 	 * Write the TX descriptor for the beacon.
 	 */
 	rt73usb_write_tx_desc(rt2x00dev, entry->skb, txdesc);
@@ -1537,11 +1544,6 @@ static void rt73usb_write_beacon(struct queue_entry *entry,
 	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_BEACON, entry->skb);
 
 	/*
-	 * Take the descriptor in front of the skb into account.
-	 */
-	skb_push(entry->skb, TXD_DESC_SIZE);
-
-	/*
 	 * Write entire beacon with descriptor to register.
 	 */
 	beacon_base = HW_BEACON_OFFSET(entry->entry_idx);
-- 
1.7.1


^ permalink raw reply related

* [PATCH 3/9] rt2x00: Re-order tx descriptor writing code in drivers.
From: Gertjan van Wingerde @ 2010-05-11 21:51 UTC (permalink / raw)
  To: John W. Linville
  Cc: Ivo van Doorn, linux-wireless, users, Gertjan van Wingerde
In-Reply-To: <1273614705-25118-1-git-send-email-gwingerde@gmail.com>

Where possible, write the tx descriptor words from start to end, to
follow a logical ordering of words.
Where this is not possible (in rt2400pci, rt2500pci and rt61pci) add
a comment as to why word 0 needs to be written last.

Signed-off-by: Gertjan van Wingerde <gwingerde@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2400pci.c |    5 +++
 drivers/net/wireless/rt2x00/rt2500pci.c |    5 +++
 drivers/net/wireless/rt2x00/rt2500usb.c |   36 +++++++++++-----------
 drivers/net/wireless/rt2x00/rt61pci.c   |    5 +++
 drivers/net/wireless/rt2x00/rt73usb.c   |   52 +++++++++++++++---------------
 5 files changed, 59 insertions(+), 44 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 4b38451..1eff6ec 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -1039,6 +1039,11 @@ static void rt2400pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 	rt2x00_set_field32(&word, TXD_W3_PLCP_LENGTH_HIGH_BUSY, 1);
 	rt2x00_desc_write(txd, 4, word);
 
+	/*
+	 * Writing TXD word 0 must the last to prevent a race condition with
+	 * the device, whereby the device may take hold of the TXD before we
+	 * finished updating it.
+	 */
 	rt2x00_desc_read(txd, 0, &word);
 	rt2x00_set_field32(&word, TXD_W0_OWNER_NIC, 1);
 	rt2x00_set_field32(&word, TXD_W0_VALID, 1);
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index d876c6d..8d3e95e 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -1193,6 +1193,11 @@ static void rt2500pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 			   test_bit(ENTRY_TXD_RTS_FRAME, &txdesc->flags));
 	rt2x00_desc_write(txd, 10, word);
 
+	/*
+	 * Writing TXD word 0 must the last to prevent a race condition with
+	 * the device, whereby the device may take hold of the TXD before we
+	 * finished updating it.
+	 */
 	rt2x00_desc_read(txd, 0, &word);
 	rt2x00_set_field32(&word, TXD_W0_OWNER_NIC, 1);
 	rt2x00_set_field32(&word, TXD_W0_VALID, 1);
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index 30c0544..9a915be 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -1039,6 +1039,24 @@ static void rt2500usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 	/*
 	 * Start writing the descriptor words.
 	 */
+	rt2x00_desc_read(txd, 0, &word);
+	rt2x00_set_field32(&word, TXD_W0_RETRY_LIMIT, txdesc->retry_limit);
+	rt2x00_set_field32(&word, TXD_W0_MORE_FRAG,
+			   test_bit(ENTRY_TXD_MORE_FRAG, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_ACK,
+			   test_bit(ENTRY_TXD_ACK, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_TIMESTAMP,
+			   test_bit(ENTRY_TXD_REQ_TIMESTAMP, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_OFDM,
+			   (txdesc->rate_mode == RATE_MODE_OFDM));
+	rt2x00_set_field32(&word, TXD_W0_NEW_SEQ,
+			   test_bit(ENTRY_TXD_FIRST_FRAGMENT, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_IFS, txdesc->ifs);
+	rt2x00_set_field32(&word, TXD_W0_DATABYTE_COUNT, txdesc->length);
+	rt2x00_set_field32(&word, TXD_W0_CIPHER, !!txdesc->cipher);
+	rt2x00_set_field32(&word, TXD_W0_KEY_ID, txdesc->key_idx);
+	rt2x00_desc_write(txd, 0, word);
+
 	rt2x00_desc_read(txd, 1, &word);
 	rt2x00_set_field32(&word, TXD_W1_IV_OFFSET, txdesc->iv_offset);
 	rt2x00_set_field32(&word, TXD_W1_AIFS, txdesc->aifs);
@@ -1057,24 +1075,6 @@ static void rt2500usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 		_rt2x00_desc_write(txd, 3, skbdesc->iv[0]);
 		_rt2x00_desc_write(txd, 4, skbdesc->iv[1]);
 	}
-
-	rt2x00_desc_read(txd, 0, &word);
-	rt2x00_set_field32(&word, TXD_W0_RETRY_LIMIT, txdesc->retry_limit);
-	rt2x00_set_field32(&word, TXD_W0_MORE_FRAG,
-			   test_bit(ENTRY_TXD_MORE_FRAG, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_ACK,
-			   test_bit(ENTRY_TXD_ACK, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_TIMESTAMP,
-			   test_bit(ENTRY_TXD_REQ_TIMESTAMP, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_OFDM,
-			   (txdesc->rate_mode == RATE_MODE_OFDM));
-	rt2x00_set_field32(&word, TXD_W0_NEW_SEQ,
-			   test_bit(ENTRY_TXD_FIRST_FRAGMENT, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_IFS, txdesc->ifs);
-	rt2x00_set_field32(&word, TXD_W0_DATABYTE_COUNT, txdesc->length);
-	rt2x00_set_field32(&word, TXD_W0_CIPHER, !!txdesc->cipher);
-	rt2x00_set_field32(&word, TXD_W0_KEY_ID, txdesc->key_idx);
-	rt2x00_desc_write(txd, 0, word);
 }
 
 /*
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 99c2981..55aa010 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -1813,6 +1813,11 @@ static void rt61pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 		rt2x00_desc_write(txd, 11, word);
 	}
 
+	/*
+	 * Writing TXD word 0 must the last to prevent a race condition with
+	 * the device, whereby the device may take hold of the TXD before we
+	 * finished updating it.
+	 */
 	rt2x00_desc_read(txd, 0, &word);
 	rt2x00_set_field32(&word, TXD_W0_OWNER_NIC, 1);
 	rt2x00_set_field32(&word, TXD_W0_VALID, 1);
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index 81f6db1..fa40d43 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -1446,6 +1446,32 @@ static void rt73usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 	/*
 	 * Start writing the descriptor words.
 	 */
+	rt2x00_desc_read(txd, 0, &word);
+	rt2x00_set_field32(&word, TXD_W0_BURST,
+			   test_bit(ENTRY_TXD_BURST, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_VALID, 1);
+	rt2x00_set_field32(&word, TXD_W0_MORE_FRAG,
+			   test_bit(ENTRY_TXD_MORE_FRAG, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_ACK,
+			   test_bit(ENTRY_TXD_ACK, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_TIMESTAMP,
+			   test_bit(ENTRY_TXD_REQ_TIMESTAMP, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_OFDM,
+			   (txdesc->rate_mode == RATE_MODE_OFDM));
+	rt2x00_set_field32(&word, TXD_W0_IFS, txdesc->ifs);
+	rt2x00_set_field32(&word, TXD_W0_RETRY_MODE,
+			   test_bit(ENTRY_TXD_RETRY_MODE, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_TKIP_MIC,
+			   test_bit(ENTRY_TXD_ENCRYPT_MMIC, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_KEY_TABLE,
+			   test_bit(ENTRY_TXD_ENCRYPT_PAIRWISE, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_KEY_INDEX, txdesc->key_idx);
+	rt2x00_set_field32(&word, TXD_W0_DATABYTE_COUNT, txdesc->length);
+	rt2x00_set_field32(&word, TXD_W0_BURST2,
+			   test_bit(ENTRY_TXD_BURST, &txdesc->flags));
+	rt2x00_set_field32(&word, TXD_W0_CIPHER_ALG, txdesc->cipher);
+	rt2x00_desc_write(txd, 0, word);
+
 	rt2x00_desc_read(txd, 1, &word);
 	rt2x00_set_field32(&word, TXD_W1_HOST_Q_ID, txdesc->queue);
 	rt2x00_set_field32(&word, TXD_W1_AIFSN, txdesc->aifs);
@@ -1473,32 +1499,6 @@ static void rt73usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
 			   TXPOWER_TO_DEV(rt2x00dev->tx_power));
 	rt2x00_set_field32(&word, TXD_W5_WAITING_DMA_DONE_INT, 1);
 	rt2x00_desc_write(txd, 5, word);
-
-	rt2x00_desc_read(txd, 0, &word);
-	rt2x00_set_field32(&word, TXD_W0_BURST,
-			   test_bit(ENTRY_TXD_BURST, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_VALID, 1);
-	rt2x00_set_field32(&word, TXD_W0_MORE_FRAG,
-			   test_bit(ENTRY_TXD_MORE_FRAG, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_ACK,
-			   test_bit(ENTRY_TXD_ACK, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_TIMESTAMP,
-			   test_bit(ENTRY_TXD_REQ_TIMESTAMP, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_OFDM,
-			   (txdesc->rate_mode == RATE_MODE_OFDM));
-	rt2x00_set_field32(&word, TXD_W0_IFS, txdesc->ifs);
-	rt2x00_set_field32(&word, TXD_W0_RETRY_MODE,
-			   test_bit(ENTRY_TXD_RETRY_MODE, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_TKIP_MIC,
-			   test_bit(ENTRY_TXD_ENCRYPT_MMIC, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_KEY_TABLE,
-			   test_bit(ENTRY_TXD_ENCRYPT_PAIRWISE, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_KEY_INDEX, txdesc->key_idx);
-	rt2x00_set_field32(&word, TXD_W0_DATABYTE_COUNT, txdesc->length);
-	rt2x00_set_field32(&word, TXD_W0_BURST2,
-			   test_bit(ENTRY_TXD_BURST, &txdesc->flags));
-	rt2x00_set_field32(&word, TXD_W0_CIPHER_ALG, txdesc->cipher);
-	rt2x00_desc_write(txd, 0, word);
 }
 
 /*
-- 
1.7.1


^ permalink raw reply related

* [PATCH 0/9] rt2x00: Further fixes and cleanups.
From: Gertjan van Wingerde @ 2010-05-11 21:51 UTC (permalink / raw)
  To: John W. Linville
  Cc: Ivo van Doorn, linux-wireless, users, Gertjan van Wingerde

Another set of rt2x00 fixes and cleanups.
Most of the patches are working towards a nice cleanup in which rt2x00 starts
properly reserving room in skbs for descriptors instead of fiddling with
pointers.
Also one fix for rt61pci that no-one ever hit, whereby, for beacons, rt61pci
wrote part of the tx descriptor out-of-bounds.

These patches can also be pulled from:
	git://git.gwingerde.nl/rt2x00-next-2.6.git

Gertjan van Wingerde (9):
  rt2x00: Consistently name skb frame descriptor skbdesc.
  rt2x00: Fix beacon descriptor writing for rt61pci.
  rt2x00: Re-order tx descriptor writing code in drivers.
  rt2x00: Simplify TXD handling of beacons.
  rt2x00: Dump beacons under a different identifier than TX frames.
  rt2x00: Push beacon TX descriptor writing to drivers.
  rt2x00: In debugfs frame dumping allow the TX descriptor to be part
    of the skb.
  rt2x00: Reverse calling order of bus write_tx_desc and driver
    write_tx_desc.
  rt2x00: Properly reserve room for descriptors in skbs.

 drivers/net/wireless/rt2x00/rt2400pci.c   |   42 ++++++++------
 drivers/net/wireless/rt2x00/rt2500pci.c   |   42 ++++++++------
 drivers/net/wireless/rt2x00/rt2500usb.c   |   68 ++++++++++++++---------
 drivers/net/wireless/rt2x00/rt2800lib.c   |    3 +-
 drivers/net/wireless/rt2x00/rt2800lib.h   |    2 +-
 drivers/net/wireless/rt2x00/rt2800pci.c   |   59 +++++++++++++-------
 drivers/net/wireless/rt2x00/rt2800usb.c   |   35 +++++++++++--
 drivers/net/wireless/rt2x00/rt2x00.h      |    9 +++
 drivers/net/wireless/rt2x00/rt2x00debug.c |   22 +++++---
 drivers/net/wireless/rt2x00/rt2x00dev.c   |    5 --
 drivers/net/wireless/rt2x00/rt2x00dump.h  |    3 +
 drivers/net/wireless/rt2x00/rt2x00lib.h   |    7 ---
 drivers/net/wireless/rt2x00/rt2x00pci.c   |   49 +++++++++++++++--
 drivers/net/wireless/rt2x00/rt2x00pci.h   |    8 +++
 drivers/net/wireless/rt2x00/rt2x00queue.c |   39 +-------------
 drivers/net/wireless/rt2x00/rt2x00queue.h |    3 +
 drivers/net/wireless/rt2x00/rt2x00usb.c   |   19 +++----
 drivers/net/wireless/rt2x00/rt61pci.c     |   50 ++++++++++++-----
 drivers/net/wireless/rt2x00/rt73usb.c     |   84 +++++++++++++++++------------
 19 files changed, 338 insertions(+), 211 deletions(-)


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox