2009-11-23 21:44:56

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH v2 0/8] Assorted fixes and cleanups for rt2x00 and mac80211.

The following patch series contains a list of fixes and cleanups for rt2x00.
Also included is a fix for mac80211 that fixes the insufficient headroom
bug reported by David Ellingsworth.

v2:
Handle review comments from Julian, Johannes and Ivo.
Split up patch 5 of first version in 3 different parts.

[PATCH v2 1/8] rt2x00: Only initialize HT on rt2800 devices that support it.
[PATCH v2 2/8] rt2x00: Remove unused variable frame_control from rt2x00mac_tx.
[PATCH v2 3/8] rt2x00: Clean up use of rt2x00_intf_is_pci.
[PATCH v2 4/8] rt2x00: Fix typo (lengt --> length) in rt2x00queue.c
[PATCH v2 5/8] rt2x00: Whitespace cleanup.
[PATCH v2 6/8] rt2x00: Centralize setting of extra TX headroom requested by rt2x00.
[PATCH v2 7/8] mac80211: Add define for TX headroom reserved by mac80211 itself.
[PATCH v2 8/8] rt2x00: Properly request tx headroom for alignment operations.

---
Gertjan.


2009-11-23 21:44:59

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH v2 1/8] rt2x00: Only initialize HT on rt2800 devices that support it.

Some RT28xx/RT30xx devices don't support 802.11n, when they are combined with
the RF2020 chipset. Ensure that HT is disabled for these devices.

Signed-off-by: Gertjan van Wingerde <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800lib.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index e94f1e1..fcd0c88 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -2072,7 +2072,11 @@ int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
/*
* Initialize HT information.
*/
- spec->ht.ht_supported = true;
+ if (!rt2x00_rf(chip, RF2020))
+ spec->ht.ht_supported = true;
+ else
+ spec->ht.ht_supported = false;
+
spec->ht.cap =
IEEE80211_HT_CAP_SUP_WIDTH_20_40 |
IEEE80211_HT_CAP_GRN_FLD |
--
1.6.5.3


2009-11-25 20:00:37

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] rt2x00: Properly request tx headroom for alignment operations.

On Mon, Nov 23, 2009 at 10:44:54PM +0100, Gertjan van Wingerde wrote:
> Current rt2x00 drivers may result in a "ieee80211_tx_status: headroom too
> small" error message when a frame needs to be properly aligned before
> transmitting it.
> This is because the space needed to ensure proper alignment isn't
> requested from mac80211.
> Fix this by adding sufficient amount of alignment space to the amount
> of headroom requested for TX frames.
>
> Reported-by: David Ellingsworth <[email protected]>
> Signed-off-by: Gertjan van Wingerde <[email protected]>

CC [M] drivers/net/wireless/rt2x00/rt2x00dev.o
drivers/net/wireless/rt2x00/rt2x00dev.c: In function ‘rt2x00lib_probe_hw’:
drivers/net/wireless/rt2x00/rt2x00dev.c:690: error: ‘IEEE80211_TX_STATUS_HEADROOM’ undeclared (first use in this function)
drivers/net/wireless/rt2x00/rt2x00dev.c:690: error: (Each undeclared identifier is reported only once
drivers/net/wireless/rt2x00/rt2x00dev.c:690: error: for each function it appears in.)

Did I miss a patch?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2009-11-23 21:45:16

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH v2 7/8] mac80211: Add define for TX headroom reserved by mac80211 itself.

Add a definition of the amount of TX headroom reserved by mac80211 itself
for its own purposes. Also add BUILD_BUG_ON to validate the value.
This define can then be used by drivers to request additional TX headroom
in the most efficient manner.

Signed-off-by: Gertjan van Wingerde <[email protected]>
Cc: Johannes Berg <[email protected]>
---
include/net/mac80211.h | 6 ++++++
net/mac80211/main.c | 2 ++
2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 3754ea4..a113458 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1727,6 +1727,12 @@ static inline void ieee80211_rx_ni(struct ieee80211_hw *hw,
local_bh_enable();
}

+/*
+ * The TX headroom reserved by mac80211 for its own tx_status functions.
+ * This is enough for the radiotap header.
+ */
+#define IEEE80211_TX_STATUS_HEADROOM 13
+
/**
* ieee80211_tx_status - transmit status callback
*
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index dd8ec8d..f35d4aa 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -515,6 +515,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
* and we need some headroom for passing the frame to monitor
* interfaces, but never both at the same time.
*/
+ BUILD_BUG_ON(IEEE80211_TX_STATUS_HEADROOM !=
+ sizeof(struct ieee80211_tx_status_rtap_hdr));
local->tx_headroom = max_t(unsigned int , local->hw.extra_tx_headroom,
sizeof(struct ieee80211_tx_status_rtap_hdr));

--
1.6.5.3


2009-11-24 19:12:55

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] rt2x00: Properly request tx headroom for alignment operations.

On 11/24/09 18:19, Ivo van Doorn wrote:
> On Monday 23 November 2009, Gertjan van Wingerde wrote:
>> Current rt2x00 drivers may result in a "ieee80211_tx_status: headroom too
>> small" error message when a frame needs to be properly aligned before
>> transmitting it.
>> This is because the space needed to ensure proper alignment isn't
>> requested from mac80211.
>> Fix this by adding sufficient amount of alignment space to the amount
>> of headroom requested for TX frames.
>>
>> Reported-by: David Ellingsworth <[email protected]>
>> Signed-off-by: Gertjan van Wingerde <[email protected]>
>> ---
>> drivers/net/wireless/rt2x00/rt2x00.h | 6 ++++++
>> drivers/net/wireless/rt2x00/rt2x00dev.c | 12 +++++++++++-
>> 2 files changed, 17 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
>> index 4d841c0..dcfc8c2 100644
>> --- a/drivers/net/wireless/rt2x00/rt2x00.h
>> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
>> @@ -113,6 +113,12 @@
>> ( ((unsigned long)((__skb)->data + (__header))) & 3 )
>>
>> /*
>> + * Constants for extra TX headroom for alignment purposes.
>> + */
>> +#define RT2X00_ALIGN_SIZE 4 /* Only whole frame needs alignment */
>> +#define RT2X00_L2PAD_SIZE 8 /* Both header & payload need alignment */
>
> Now that I think of it, why do we need 8 byte for L2 padding?
> Shouldn't we need the same size for regular alignment, L2 padding only indicates
> the padding occurs between header and payload rather then before the header only.
> So unless I am mistaken about how I coded the header & payload moving in rt2x00lib,
> you always need at most 4 bytes.
>

Nope. If I read the code right then the header can be moved by more than 4 bytes, namely
when the payload needs to be shifted more than the header. In that case the header is
shifted an extra 4 bytes to make room for the payload. However, 8 is the obvious maximum.

---
Gertjan.

2009-11-23 21:45:05

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH v2 3/8] rt2x00: Clean up use of rt2x00_intf_is_pci.

RT chipsets are unique across both PCI and USB busses, and don't overlap.
Therefore there is no need to test for bus type when only checking for
chipset type. Remove the redundant checks.

Signed-off-by: Gertjan van Wingerde <[email protected]>
Acked-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800lib.c | 17 +++++++----------
1 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index fcd0c88..02ffcf5 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -217,14 +217,12 @@ void rt2800_mcu_request(struct rt2x00_dev *rt2x00dev,
{
u32 reg;

- if (rt2x00_intf_is_pci(rt2x00dev)) {
- /*
- * RT2880 and RT3052 don't support MCU requests.
- */
- if (rt2x00_rt(&rt2x00dev->chip, RT2880) ||
- rt2x00_rt(&rt2x00dev->chip, RT3052))
- return;
- }
+ /*
+ * RT2880 and RT3052 don't support MCU requests.
+ */
+ if (rt2x00_rt(&rt2x00dev->chip, RT2880) ||
+ rt2x00_rt(&rt2x00dev->chip, RT3052))
+ return;

mutex_lock(&rt2x00dev->csr_mutex);

@@ -1482,8 +1480,7 @@ int rt2800_init_bbp(struct rt2x00_dev *rt2x00dev)
rt2800_bbp_write(rt2x00dev, 105, 0x05);
}

- if (rt2x00_intf_is_pci(rt2x00dev) &&
- rt2x00_rt(&rt2x00dev->chip, RT3052)) {
+ if (rt2x00_rt(&rt2x00dev->chip, RT3052)) {
rt2800_bbp_write(rt2x00dev, 31, 0x08);
rt2800_bbp_write(rt2x00dev, 78, 0x0e);
rt2800_bbp_write(rt2x00dev, 80, 0x08);
--
1.6.5.3


2009-11-23 23:23:24

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2 7/8] mac80211: Add define for TX headroom reserved by mac80211 itself.

On Mon, 2009-11-23 at 22:44 +0100, Gertjan van Wingerde wrote:
> Add a definition of the amount of TX headroom reserved by mac80211 itself
> for its own purposes. Also add BUILD_BUG_ON to validate the value.
> This define can then be used by drivers to request additional TX headroom
> in the most efficient manner.
>
> Signed-off-by: Gertjan van Wingerde <[email protected]>
> Cc: Johannes Berg <[email protected]>

That makes more sense to me, thanks.

johannes

> ---
> include/net/mac80211.h | 6 ++++++
> net/mac80211/main.c | 2 ++
> 2 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 3754ea4..a113458 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1727,6 +1727,12 @@ static inline void ieee80211_rx_ni(struct ieee80211_hw *hw,
> local_bh_enable();
> }
>
> +/*
> + * The TX headroom reserved by mac80211 for its own tx_status functions.
> + * This is enough for the radiotap header.
> + */
> +#define IEEE80211_TX_STATUS_HEADROOM 13
> +
> /**
> * ieee80211_tx_status - transmit status callback
> *
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index dd8ec8d..f35d4aa 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -515,6 +515,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
> * and we need some headroom for passing the frame to monitor
> * interfaces, but never both at the same time.
> */
> + BUILD_BUG_ON(IEEE80211_TX_STATUS_HEADROOM !=
> + sizeof(struct ieee80211_tx_status_rtap_hdr));
> local->tx_headroom = max_t(unsigned int , local->hw.extra_tx_headroom,
> sizeof(struct ieee80211_tx_status_rtap_hdr));
>


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-11-23 21:45:06

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH v2 4/8] rt2x00: Fix typo (lengt --> length) in rt2x00queue.c

Signed-off-by: Gertjan van Wingerde <[email protected]>
Acked-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00queue.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index eaedee8..32d4aea 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -162,10 +162,10 @@ void rt2x00queue_align_frame(struct sk_buff *skb)
skb_trim(skb, frame_length);
}

-void rt2x00queue_align_payload(struct sk_buff *skb, unsigned int header_lengt)
+void rt2x00queue_align_payload(struct sk_buff *skb, unsigned int header_length)
{
unsigned int frame_length = skb->len;
- unsigned int align = ALIGN_SIZE(skb, header_lengt);
+ unsigned int align = ALIGN_SIZE(skb, header_length);

if (!align)
return;
--
1.6.5.3


2009-11-23 21:45:18

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH v2 8/8] rt2x00: Properly request tx headroom for alignment operations.

Current rt2x00 drivers may result in a "ieee80211_tx_status: headroom too
small" error message when a frame needs to be properly aligned before
transmitting it.
This is because the space needed to ensure proper alignment isn't
requested from mac80211.
Fix this by adding sufficient amount of alignment space to the amount
of headroom requested for TX frames.

Reported-by: David Ellingsworth <[email protected]>
Signed-off-by: Gertjan van Wingerde <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00.h | 6 ++++++
drivers/net/wireless/rt2x00/rt2x00dev.c | 12 +++++++++++-
2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 4d841c0..dcfc8c2 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -113,6 +113,12 @@
( ((unsigned long)((__skb)->data + (__header))) & 3 )

/*
+ * Constants for extra TX headroom for alignment purposes.
+ */
+#define RT2X00_ALIGN_SIZE 4 /* Only whole frame needs alignment */
+#define RT2X00_L2PAD_SIZE 8 /* Both header & payload need alignment */
+
+/*
* Standard timing and size defines.
* These values should follow the ieee80211 specifications.
*/
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 06c43ca..265e66d 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -686,7 +686,17 @@ static int rt2x00lib_probe_hw(struct rt2x00_dev *rt2x00dev)
/*
* Initialize extra TX headroom required.
*/
- rt2x00dev->hw->extra_tx_headroom = rt2x00dev->ops->extra_tx_headroom;
+ rt2x00dev->hw->extra_tx_headroom =
+ max_t(unsigned int, IEEE80211_TX_STATUS_HEADROOM,
+ rt2x00dev->ops->extra_tx_headroom);
+
+ /*
+ * Take TX headroom required for alignment into account.
+ */
+ if (test_bit(DRIVER_REQUIRE_L2PAD, &rt2x00dev->flags))
+ rt2x00dev->hw->extra_tx_headroom += RT2X00_L2PAD_SIZE;
+ else if (test_bit(DRIVER_REQUIRE_DMA, &rt2x00dev->flags))
+ rt2x00dev->hw->extra_tx_headroom += RT2X00_ALIGN_SIZE;

/*
* Register HW.
--
1.6.5.3


2009-11-23 21:45:09

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH v2 5/8] rt2x00: Whitespace cleanup.

Clean up the use of whitespace in the initialization of the rt2x00_ops
structures. This is preparatory for a later patch that adds members
to that structure, which require different whitespace alignment.

Signed-off-by: Gertjan van Wingerde <[email protected]>
---
drivers/net/wireless/rt2x00/rt2400pci.c | 26 +++++++++++++-------------
drivers/net/wireless/rt2x00/rt2500pci.c | 26 +++++++++++++-------------
drivers/net/wireless/rt2x00/rt2500usb.c | 26 +++++++++++++-------------
drivers/net/wireless/rt2x00/rt2800pci.c | 24 ++++++++++++------------
drivers/net/wireless/rt2x00/rt2800usb.c | 24 ++++++++++++------------
drivers/net/wireless/rt2x00/rt61pci.c | 24 ++++++++++++------------
drivers/net/wireless/rt2x00/rt73usb.c | 24 ++++++++++++------------
7 files changed, 87 insertions(+), 87 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 6e68bc7..7f900be 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -1623,20 +1623,20 @@ static const struct data_queue_desc rt2400pci_queue_atim = {
};

static const struct rt2x00_ops rt2400pci_ops = {
- .name = KBUILD_MODNAME,
- .max_sta_intf = 1,
- .max_ap_intf = 1,
- .eeprom_size = EEPROM_SIZE,
- .rf_size = RF_SIZE,
- .tx_queues = NUM_TX_QUEUES,
- .rx = &rt2400pci_queue_rx,
- .tx = &rt2400pci_queue_tx,
- .bcn = &rt2400pci_queue_bcn,
- .atim = &rt2400pci_queue_atim,
- .lib = &rt2400pci_rt2x00_ops,
- .hw = &rt2400pci_mac80211_ops,
+ .name = KBUILD_MODNAME,
+ .max_sta_intf = 1,
+ .max_ap_intf = 1,
+ .eeprom_size = EEPROM_SIZE,
+ .rf_size = RF_SIZE,
+ .tx_queues = NUM_TX_QUEUES,
+ .rx = &rt2400pci_queue_rx,
+ .tx = &rt2400pci_queue_tx,
+ .bcn = &rt2400pci_queue_bcn,
+ .atim = &rt2400pci_queue_atim,
+ .lib = &rt2400pci_rt2x00_ops,
+ .hw = &rt2400pci_mac80211_ops,
#ifdef CONFIG_RT2X00_LIB_DEBUGFS
- .debugfs = &rt2400pci_rt2x00debug,
+ .debugfs = &rt2400pci_rt2x00debug,
#endif /* CONFIG_RT2X00_LIB_DEBUGFS */
};

diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 9a31e5e..30960fd 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -1922,20 +1922,20 @@ static const struct data_queue_desc rt2500pci_queue_atim = {
};

static const struct rt2x00_ops rt2500pci_ops = {
- .name = KBUILD_MODNAME,
- .max_sta_intf = 1,
- .max_ap_intf = 1,
- .eeprom_size = EEPROM_SIZE,
- .rf_size = RF_SIZE,
- .tx_queues = NUM_TX_QUEUES,
- .rx = &rt2500pci_queue_rx,
- .tx = &rt2500pci_queue_tx,
- .bcn = &rt2500pci_queue_bcn,
- .atim = &rt2500pci_queue_atim,
- .lib = &rt2500pci_rt2x00_ops,
- .hw = &rt2500pci_mac80211_ops,
+ .name = KBUILD_MODNAME,
+ .max_sta_intf = 1,
+ .max_ap_intf = 1,
+ .eeprom_size = EEPROM_SIZE,
+ .rf_size = RF_SIZE,
+ .tx_queues = NUM_TX_QUEUES,
+ .rx = &rt2500pci_queue_rx,
+ .tx = &rt2500pci_queue_tx,
+ .bcn = &rt2500pci_queue_bcn,
+ .atim = &rt2500pci_queue_atim,
+ .lib = &rt2500pci_rt2x00_ops,
+ .hw = &rt2500pci_mac80211_ops,
#ifdef CONFIG_RT2X00_LIB_DEBUGFS
- .debugfs = &rt2500pci_rt2x00debug,
+ .debugfs = &rt2500pci_rt2x00debug,
#endif /* CONFIG_RT2X00_LIB_DEBUGFS */
};

diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index b2de43e..02290f6 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -1823,20 +1823,20 @@ static const struct data_queue_desc rt2500usb_queue_atim = {
};

static const struct rt2x00_ops rt2500usb_ops = {
- .name = KBUILD_MODNAME,
- .max_sta_intf = 1,
- .max_ap_intf = 1,
- .eeprom_size = EEPROM_SIZE,
- .rf_size = RF_SIZE,
- .tx_queues = NUM_TX_QUEUES,
- .rx = &rt2500usb_queue_rx,
- .tx = &rt2500usb_queue_tx,
- .bcn = &rt2500usb_queue_bcn,
- .atim = &rt2500usb_queue_atim,
- .lib = &rt2500usb_rt2x00_ops,
- .hw = &rt2500usb_mac80211_ops,
+ .name = KBUILD_MODNAME,
+ .max_sta_intf = 1,
+ .max_ap_intf = 1,
+ .eeprom_size = EEPROM_SIZE,
+ .rf_size = RF_SIZE,
+ .tx_queues = NUM_TX_QUEUES,
+ .rx = &rt2500usb_queue_rx,
+ .tx = &rt2500usb_queue_tx,
+ .bcn = &rt2500usb_queue_bcn,
+ .atim = &rt2500usb_queue_atim,
+ .lib = &rt2500usb_rt2x00_ops,
+ .hw = &rt2500usb_mac80211_ops,
#ifdef CONFIG_RT2X00_LIB_DEBUGFS
- .debugfs = &rt2500usb_rt2x00debug,
+ .debugfs = &rt2500usb_rt2x00debug,
#endif /* CONFIG_RT2X00_LIB_DEBUGFS */
};

diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 87a5094..029a45f 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -1201,19 +1201,19 @@ static const struct data_queue_desc rt2800pci_queue_bcn = {
};

static const struct rt2x00_ops rt2800pci_ops = {
- .name = KBUILD_MODNAME,
- .max_sta_intf = 1,
- .max_ap_intf = 8,
- .eeprom_size = EEPROM_SIZE,
- .rf_size = RF_SIZE,
- .tx_queues = NUM_TX_QUEUES,
- .rx = &rt2800pci_queue_rx,
- .tx = &rt2800pci_queue_tx,
- .bcn = &rt2800pci_queue_bcn,
- .lib = &rt2800pci_rt2x00_ops,
- .hw = &rt2800_mac80211_ops,
+ .name = KBUILD_MODNAME,
+ .max_sta_intf = 1,
+ .max_ap_intf = 8,
+ .eeprom_size = EEPROM_SIZE,
+ .rf_size = RF_SIZE,
+ .tx_queues = NUM_TX_QUEUES,
+ .rx = &rt2800pci_queue_rx,
+ .tx = &rt2800pci_queue_tx,
+ .bcn = &rt2800pci_queue_bcn,
+ .lib = &rt2800pci_rt2x00_ops,
+ .hw = &rt2800_mac80211_ops,
#ifdef CONFIG_RT2X00_LIB_DEBUGFS
- .debugfs = &rt2800_rt2x00debug,
+ .debugfs = &rt2800_rt2x00debug,
#endif /* CONFIG_RT2X00_LIB_DEBUGFS */
};

diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 9ab15c4..208316a 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -790,19 +790,19 @@ static const struct data_queue_desc rt2800usb_queue_bcn = {
};

static const struct rt2x00_ops rt2800usb_ops = {
- .name = KBUILD_MODNAME,
- .max_sta_intf = 1,
- .max_ap_intf = 8,
- .eeprom_size = EEPROM_SIZE,
- .rf_size = RF_SIZE,
- .tx_queues = NUM_TX_QUEUES,
- .rx = &rt2800usb_queue_rx,
- .tx = &rt2800usb_queue_tx,
- .bcn = &rt2800usb_queue_bcn,
- .lib = &rt2800usb_rt2x00_ops,
- .hw = &rt2800_mac80211_ops,
+ .name = KBUILD_MODNAME,
+ .max_sta_intf = 1,
+ .max_ap_intf = 8,
+ .eeprom_size = EEPROM_SIZE,
+ .rf_size = RF_SIZE,
+ .tx_queues = NUM_TX_QUEUES,
+ .rx = &rt2800usb_queue_rx,
+ .tx = &rt2800usb_queue_tx,
+ .bcn = &rt2800usb_queue_bcn,
+ .lib = &rt2800usb_rt2x00_ops,
+ .hw = &rt2800_mac80211_ops,
#ifdef CONFIG_RT2X00_LIB_DEBUGFS
- .debugfs = &rt2800_rt2x00debug,
+ .debugfs = &rt2800_rt2x00debug,
#endif /* CONFIG_RT2X00_LIB_DEBUGFS */
};

diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index bf04605..4cb9afe 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -2788,19 +2788,19 @@ static const struct data_queue_desc rt61pci_queue_bcn = {
};

static const struct rt2x00_ops rt61pci_ops = {
- .name = KBUILD_MODNAME,
- .max_sta_intf = 1,
- .max_ap_intf = 4,
- .eeprom_size = EEPROM_SIZE,
- .rf_size = RF_SIZE,
- .tx_queues = NUM_TX_QUEUES,
- .rx = &rt61pci_queue_rx,
- .tx = &rt61pci_queue_tx,
- .bcn = &rt61pci_queue_bcn,
- .lib = &rt61pci_rt2x00_ops,
- .hw = &rt61pci_mac80211_ops,
+ .name = KBUILD_MODNAME,
+ .max_sta_intf = 1,
+ .max_ap_intf = 4,
+ .eeprom_size = EEPROM_SIZE,
+ .rf_size = RF_SIZE,
+ .tx_queues = NUM_TX_QUEUES,
+ .rx = &rt61pci_queue_rx,
+ .tx = &rt61pci_queue_tx,
+ .bcn = &rt61pci_queue_bcn,
+ .lib = &rt61pci_rt2x00_ops,
+ .hw = &rt61pci_mac80211_ops,
#ifdef CONFIG_RT2X00_LIB_DEBUGFS
- .debugfs = &rt61pci_rt2x00debug,
+ .debugfs = &rt61pci_rt2x00debug,
#endif /* CONFIG_RT2X00_LIB_DEBUGFS */
};

diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index 5bbcf66..d13a051 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -2306,19 +2306,19 @@ static const struct data_queue_desc rt73usb_queue_bcn = {
};

static const struct rt2x00_ops rt73usb_ops = {
- .name = KBUILD_MODNAME,
- .max_sta_intf = 1,
- .max_ap_intf = 4,
- .eeprom_size = EEPROM_SIZE,
- .rf_size = RF_SIZE,
- .tx_queues = NUM_TX_QUEUES,
- .rx = &rt73usb_queue_rx,
- .tx = &rt73usb_queue_tx,
- .bcn = &rt73usb_queue_bcn,
- .lib = &rt73usb_rt2x00_ops,
- .hw = &rt73usb_mac80211_ops,
+ .name = KBUILD_MODNAME,
+ .max_sta_intf = 1,
+ .max_ap_intf = 4,
+ .eeprom_size = EEPROM_SIZE,
+ .rf_size = RF_SIZE,
+ .tx_queues = NUM_TX_QUEUES,
+ .rx = &rt73usb_queue_rx,
+ .tx = &rt73usb_queue_tx,
+ .bcn = &rt73usb_queue_bcn,
+ .lib = &rt73usb_rt2x00_ops,
+ .hw = &rt73usb_mac80211_ops,
#ifdef CONFIG_RT2X00_LIB_DEBUGFS
- .debugfs = &rt73usb_rt2x00debug,
+ .debugfs = &rt73usb_rt2x00debug,
#endif /* CONFIG_RT2X00_LIB_DEBUGFS */
};

--
1.6.5.3


2009-11-24 17:19:30

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] rt2x00: Properly request tx headroom for alignment operations.

On Monday 23 November 2009, Gertjan van Wingerde wrote:
> Current rt2x00 drivers may result in a "ieee80211_tx_status: headroom too
> small" error message when a frame needs to be properly aligned before
> transmitting it.
> This is because the space needed to ensure proper alignment isn't
> requested from mac80211.
> Fix this by adding sufficient amount of alignment space to the amount
> of headroom requested for TX frames.
>
> Reported-by: David Ellingsworth <[email protected]>
> Signed-off-by: Gertjan van Wingerde <[email protected]>
> ---
> drivers/net/wireless/rt2x00/rt2x00.h | 6 ++++++
> drivers/net/wireless/rt2x00/rt2x00dev.c | 12 +++++++++++-
> 2 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
> index 4d841c0..dcfc8c2 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00.h
> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
> @@ -113,6 +113,12 @@
> ( ((unsigned long)((__skb)->data + (__header))) & 3 )
>
> /*
> + * Constants for extra TX headroom for alignment purposes.
> + */
> +#define RT2X00_ALIGN_SIZE 4 /* Only whole frame needs alignment */
> +#define RT2X00_L2PAD_SIZE 8 /* Both header & payload need alignment */

Now that I think of it, why do we need 8 byte for L2 padding?
Shouldn't we need the same size for regular alignment, L2 padding only indicates
the padding occurs between header and payload rather then before the header only.
So unless I am mistaken about how I coded the header & payload moving in rt2x00lib,
you always need at most 4 bytes.

> +/*
> * Standard timing and size defines.
> * These values should follow the ieee80211 specifications.
> */
> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> index 06c43ca..265e66d 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -686,7 +686,17 @@ static int rt2x00lib_probe_hw(struct rt2x00_dev *rt2x00dev)
> /*
> * Initialize extra TX headroom required.
> */
> - rt2x00dev->hw->extra_tx_headroom = rt2x00dev->ops->extra_tx_headroom;
> + rt2x00dev->hw->extra_tx_headroom =
> + max_t(unsigned int, IEEE80211_TX_STATUS_HEADROOM,
> + rt2x00dev->ops->extra_tx_headroom);
> +
> + /*
> + * Take TX headroom required for alignment into account.
> + */
> + if (test_bit(DRIVER_REQUIRE_L2PAD, &rt2x00dev->flags))
> + rt2x00dev->hw->extra_tx_headroom += RT2X00_L2PAD_SIZE;
> + else if (test_bit(DRIVER_REQUIRE_DMA, &rt2x00dev->flags))
> + rt2x00dev->hw->extra_tx_headroom += RT2X00_ALIGN_SIZE;
>
> /*
> * Register HW.



2009-11-23 22:18:42

by David Ellingsworth

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH v2 1/8] rt2x00: Only initialize HT on rt2800 devices that support it.

On Mon, Nov 23, 2009 at 4:44 PM, Gertjan van Wingerde
<[email protected]> wrote:
> Some RT28xx/RT30xx devices don't support 802.11n, when they are combined with
> the RF2020 chipset. Ensure that HT is disabled for these devices.
>
> Signed-off-by: Gertjan van Wingerde <[email protected]>
> ---
> ?drivers/net/wireless/rt2x00/rt2800lib.c | ? ?6 +++++-
> ?1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> index e94f1e1..fcd0c88 100644
> --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> @@ -2072,7 +2072,11 @@ int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
> ? ? ? ?/*
> ? ? ? ? * Initialize HT information.
> ? ? ? ? */
> - ? ? ? spec->ht.ht_supported = true;
> + ? ? ? if (!rt2x00_rf(chip, RF2020))
> + ? ? ? ? ? ? ? spec->ht.ht_supported = true;
> + ? ? ? else
> + ? ? ? ? ? ? ? spec->ht.ht_supported = false;
> +

Maybe I'm the only one, but I hate conditional statements with no
meaning. Maybe this instead?

spec->ht.ht_supported = !rt2x00_rf(chip, RF2020);

> ? ? ? ?spec->ht.cap =
> ? ? ? ? ? ?IEEE80211_HT_CAP_SUP_WIDTH_20_40 |
> ? ? ? ? ? ?IEEE80211_HT_CAP_GRN_FLD |
> --
> 1.6.5.3
>
>
> _______________________________________________
> users mailing list
> [email protected]
> http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com
>

2009-11-24 17:13:01

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH v2 5/8] rt2x00: Whitespace cleanup.

On Monday 23 November 2009, Gertjan van Wingerde wrote:
> Clean up the use of whitespace in the initialization of the rt2x00_ops
> structures. This is preparatory for a later patch that adds members
> to that structure, which require different whitespace alignment.
>
> Signed-off-by: Gertjan van Wingerde <[email protected]>

Acked-by: Ivo van Doorn <[email protected]>

2009-11-23 21:45:02

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH v2 2/8] rt2x00: Remove unused variable frame_control from rt2x00mac_tx.

As additional fallout also remove the also unused variable ieee80211hdr.

Reported-by: Johannes Stezenbach <[email protected]>
Signed-off-by: Gertjan van Wingerde <[email protected]>
Acked-by: Ivo van Doorn <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00mac.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00mac.c b/drivers/net/wireless/rt2x00/rt2x00mac.c
index 9c90ceb..de549c2 100644
--- a/drivers/net/wireless/rt2x00/rt2x00mac.c
+++ b/drivers/net/wireless/rt2x00/rt2x00mac.c
@@ -103,10 +103,8 @@ int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
{
struct rt2x00_dev *rt2x00dev = hw->priv;
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
- struct ieee80211_hdr *ieee80211hdr = (struct ieee80211_hdr *)skb->data;
enum data_queue_qid qid = skb_get_queue_mapping(skb);
struct data_queue *queue;
- u16 frame_control;

/*
* Mac80211 might be calling this function while we are trying
@@ -141,7 +139,6 @@ int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
* either RTS or CTS-to-self frame and handles everything
* inside the hardware.
*/
- frame_control = le16_to_cpu(ieee80211hdr->frame_control);
if ((tx_info->control.rates[0].flags & (IEEE80211_TX_RC_USE_RTS_CTS |
IEEE80211_TX_RC_USE_CTS_PROTECT)) &&
!rt2x00dev->ops->hw->set_rts_threshold) {
--
1.6.5.3


2009-11-23 21:45:13

by Gertjan van Wingerde

[permalink] [raw]
Subject: [PATCH v2 6/8] rt2x00: Centralize setting of extra TX headroom requested by rt2x00.

Set the value of extra_tx_headroom in a central place, rather than in each
of the drivers. This is preparatory for taking alignment space into account
in the TX headroom requested by rt2x00.

Signed-off-by: Gertjan van Wingerde <[email protected]>
---
drivers/net/wireless/rt2x00/rt2400pci.c | 2 +-
drivers/net/wireless/rt2x00/rt2500pci.c | 3 +--
drivers/net/wireless/rt2x00/rt2500usb.c | 3 +--
drivers/net/wireless/rt2x00/rt2800lib.c | 6 ------
drivers/net/wireless/rt2x00/rt2800pci.c | 7 ++++---
drivers/net/wireless/rt2x00/rt2800usb.c | 1 +
drivers/net/wireless/rt2x00/rt2x00.h | 1 +
drivers/net/wireless/rt2x00/rt2x00dev.c | 5 +++++
drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
drivers/net/wireless/rt2x00/rt73usb.c | 2 +-
10 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 7f900be..e7f4640 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -1432,7 +1432,6 @@ static int rt2400pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
IEEE80211_HW_SIGNAL_DBM |
IEEE80211_HW_SUPPORTS_PS |
IEEE80211_HW_PS_NULLFUNC_STACK;
- rt2x00dev->hw->extra_tx_headroom = 0;

SET_IEEE80211_DEV(rt2x00dev->hw, rt2x00dev->dev);
SET_IEEE80211_PERM_ADDR(rt2x00dev->hw,
@@ -1629,6 +1628,7 @@ static const struct rt2x00_ops rt2400pci_ops = {
.eeprom_size = EEPROM_SIZE,
.rf_size = RF_SIZE,
.tx_queues = NUM_TX_QUEUES,
+ .extra_tx_headroom = 0,
.rx = &rt2400pci_queue_rx,
.tx = &rt2400pci_queue_tx,
.bcn = &rt2400pci_queue_bcn,
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 30960fd..408fcfc 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -1733,8 +1733,6 @@ static int rt2500pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
IEEE80211_HW_SUPPORTS_PS |
IEEE80211_HW_PS_NULLFUNC_STACK;

- rt2x00dev->hw->extra_tx_headroom = 0;
-
SET_IEEE80211_DEV(rt2x00dev->hw, rt2x00dev->dev);
SET_IEEE80211_PERM_ADDR(rt2x00dev->hw,
rt2x00_eeprom_addr(rt2x00dev,
@@ -1928,6 +1926,7 @@ static const struct rt2x00_ops rt2500pci_ops = {
.eeprom_size = EEPROM_SIZE,
.rf_size = RF_SIZE,
.tx_queues = NUM_TX_QUEUES,
+ .extra_tx_headroom = 0,
.rx = &rt2500pci_queue_rx,
.tx = &rt2500pci_queue_tx,
.bcn = &rt2500pci_queue_bcn,
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index 02290f6..83f2592 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -1656,8 +1656,6 @@ static int rt2500usb_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
IEEE80211_HW_SUPPORTS_PS |
IEEE80211_HW_PS_NULLFUNC_STACK;

- rt2x00dev->hw->extra_tx_headroom = TXD_DESC_SIZE;
-
SET_IEEE80211_DEV(rt2x00dev->hw, rt2x00dev->dev);
SET_IEEE80211_PERM_ADDR(rt2x00dev->hw,
rt2x00_eeprom_addr(rt2x00dev,
@@ -1829,6 +1827,7 @@ static const struct rt2x00_ops rt2500usb_ops = {
.eeprom_size = EEPROM_SIZE,
.rf_size = RF_SIZE,
.tx_queues = NUM_TX_QUEUES,
+ .extra_tx_headroom = TXD_DESC_SIZE,
.rx = &rt2500usb_queue_rx,
.tx = &rt2500usb_queue_tx,
.bcn = &rt2500usb_queue_bcn,
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 02ffcf5..eb1e1d0 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -2030,12 +2030,6 @@ int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
IEEE80211_HW_SUPPORTS_PS |
IEEE80211_HW_PS_NULLFUNC_STACK;

- if (rt2x00_intf_is_usb(rt2x00dev))
- rt2x00dev->hw->extra_tx_headroom =
- TXINFO_DESC_SIZE + TXWI_DESC_SIZE;
- else if (rt2x00_intf_is_pci(rt2x00dev))
- rt2x00dev->hw->extra_tx_headroom = TXWI_DESC_SIZE;
-
SET_IEEE80211_DEV(rt2x00dev->hw, rt2x00dev->dev);
SET_IEEE80211_PERM_ADDR(rt2x00dev->hw,
rt2x00_eeprom_addr(rt2x00dev,
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 029a45f..dfc886f 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -652,7 +652,7 @@ static void rt2800pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
{
struct skb_frame_desc *skbdesc = get_skb_frame_desc(skb);
__le32 *txd = skbdesc->desc;
- __le32 *txwi = (__le32 *)(skb->data - rt2x00dev->hw->extra_tx_headroom);
+ __le32 *txwi = (__le32 *)(skb->data - rt2x00dev->ops->extra_tx_headroom);
u32 word;

/*
@@ -725,14 +725,14 @@ static void rt2800pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
rt2x00_set_field32(&word, TXD_W1_BURST,
test_bit(ENTRY_TXD_BURST, &txdesc->flags));
rt2x00_set_field32(&word, TXD_W1_SD_LEN0,
- rt2x00dev->hw->extra_tx_headroom);
+ rt2x00dev->ops->extra_tx_headroom);
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->hw->extra_tx_headroom);
+ skbdesc->skb_dma + rt2x00dev->ops->extra_tx_headroom);
rt2x00_desc_write(txd, 2, word);

rt2x00_desc_read(txd, 3, &word);
@@ -1207,6 +1207,7 @@ static const struct rt2x00_ops rt2800pci_ops = {
.eeprom_size = EEPROM_SIZE,
.rf_size = RF_SIZE,
.tx_queues = NUM_TX_QUEUES,
+ .extra_tx_headroom = TXWI_DESC_SIZE,
.rx = &rt2800pci_queue_rx,
.tx = &rt2800pci_queue_tx,
.bcn = &rt2800pci_queue_bcn,
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 208316a..af85d18 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -796,6 +796,7 @@ static const struct rt2x00_ops rt2800usb_ops = {
.eeprom_size = EEPROM_SIZE,
.rf_size = RF_SIZE,
.tx_queues = NUM_TX_QUEUES,
+ .extra_tx_headroom = TXINFO_DESC_SIZE + TXWI_DESC_SIZE,
.rx = &rt2800usb_queue_rx,
.tx = &rt2800usb_queue_tx,
.bcn = &rt2800usb_queue_bcn,
diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 1cbb7ac..4d841c0 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -588,6 +588,7 @@ struct rt2x00_ops {
const unsigned int eeprom_size;
const unsigned int rf_size;
const unsigned int tx_queues;
+ const unsigned int extra_tx_headroom;
const struct data_queue_desc *rx;
const struct data_queue_desc *tx;
const struct data_queue_desc *bcn;
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 4a4b7e4..06c43ca 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -684,6 +684,11 @@ static int rt2x00lib_probe_hw(struct rt2x00_dev *rt2x00dev)
rt2x00dev->hw->queues = rt2x00dev->ops->tx_queues;

/*
+ * Initialize extra TX headroom required.
+ */
+ rt2x00dev->hw->extra_tx_headroom = rt2x00dev->ops->extra_tx_headroom;
+
+ /*
* Register HW.
*/
status = ieee80211_register_hw(rt2x00dev->hw);
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 4cb9afe..687e17d 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -2546,7 +2546,6 @@ static int rt61pci_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
IEEE80211_HW_SIGNAL_DBM |
IEEE80211_HW_SUPPORTS_PS |
IEEE80211_HW_PS_NULLFUNC_STACK;
- rt2x00dev->hw->extra_tx_headroom = 0;

SET_IEEE80211_DEV(rt2x00dev->hw, rt2x00dev->dev);
SET_IEEE80211_PERM_ADDR(rt2x00dev->hw,
@@ -2794,6 +2793,7 @@ static const struct rt2x00_ops rt61pci_ops = {
.eeprom_size = EEPROM_SIZE,
.rf_size = RF_SIZE,
.tx_queues = NUM_TX_QUEUES,
+ .extra_tx_headroom = 0,
.rx = &rt61pci_queue_rx,
.tx = &rt61pci_queue_tx,
.bcn = &rt61pci_queue_bcn,
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index d13a051..ced3b6a 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -2069,7 +2069,6 @@ static int rt73usb_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
IEEE80211_HW_SIGNAL_DBM |
IEEE80211_HW_SUPPORTS_PS |
IEEE80211_HW_PS_NULLFUNC_STACK;
- rt2x00dev->hw->extra_tx_headroom = TXD_DESC_SIZE;

SET_IEEE80211_DEV(rt2x00dev->hw, rt2x00dev->dev);
SET_IEEE80211_PERM_ADDR(rt2x00dev->hw,
@@ -2312,6 +2311,7 @@ static const struct rt2x00_ops rt73usb_ops = {
.eeprom_size = EEPROM_SIZE,
.rf_size = RF_SIZE,
.tx_queues = NUM_TX_QUEUES,
+ .extra_tx_headroom = TXD_DESC_SIZE,
.rx = &rt73usb_queue_rx,
.tx = &rt73usb_queue_tx,
.bcn = &rt73usb_queue_bcn,
--
1.6.5.3


2009-11-24 17:14:01

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH v2 6/8] rt2x00: Centralize setting of extra TX headroom requested by rt2x00.

On Monday 23 November 2009, Gertjan van Wingerde wrote:
> Set the value of extra_tx_headroom in a central place, rather than in each
> of the drivers. This is preparatory for taking alignment space into account
> in the TX headroom requested by rt2x00.
>
> Signed-off-by: Gertjan van Wingerde <[email protected]>

Acked-by: Ivo van Doorn <[email protected]>

2009-11-24 20:04:48

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] rt2x00: Properly request tx headroom for alignment operations.

On Tuesday 24 November 2009, Gertjan van Wingerde wrote:
> On 11/24/09 18:19, Ivo van Doorn wrote:
> > On Monday 23 November 2009, Gertjan van Wingerde wrote:
> >> Current rt2x00 drivers may result in a "ieee80211_tx_status: headroom too
> >> small" error message when a frame needs to be properly aligned before
> >> transmitting it.
> >> This is because the space needed to ensure proper alignment isn't
> >> requested from mac80211.
> >> Fix this by adding sufficient amount of alignment space to the amount
> >> of headroom requested for TX frames.
> >>
> >> Reported-by: David Ellingsworth <[email protected]>
> >> Signed-off-by: Gertjan van Wingerde <[email protected]>
> >> ---
> >> drivers/net/wireless/rt2x00/rt2x00.h | 6 ++++++
> >> drivers/net/wireless/rt2x00/rt2x00dev.c | 12 +++++++++++-
> >> 2 files changed, 17 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
> >> index 4d841c0..dcfc8c2 100644
> >> --- a/drivers/net/wireless/rt2x00/rt2x00.h
> >> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
> >> @@ -113,6 +113,12 @@
> >> ( ((unsigned long)((__skb)->data + (__header))) & 3 )
> >>
> >> /*
> >> + * Constants for extra TX headroom for alignment purposes.
> >> + */
> >> +#define RT2X00_ALIGN_SIZE 4 /* Only whole frame needs alignment */
> >> +#define RT2X00_L2PAD_SIZE 8 /* Both header & payload need alignment */
> >
> > Now that I think of it, why do we need 8 byte for L2 padding?
> > Shouldn't we need the same size for regular alignment, L2 padding only indicates
> > the padding occurs between header and payload rather then before the header only.
> > So unless I am mistaken about how I coded the header & payload moving in rt2x00lib,
> > you always need at most 4 bytes.
> >
>
> Nope. If I read the code right then the header can be moved by more than 4 bytes, namely
> when the payload needs to be shifted more than the header. In that case the header is
> shifted an extra 4 bytes to make room for the payload. However, 8 is the obvious maximum.

Ah right, well 8bytes is ok then. :)

Acked-by: Ivo van Doorn <[email protected]>

2009-11-25 22:00:42

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] rt2x00: Properly request tx headroom for alignment operations.

On Wed, Nov 25, 2009 at 10:29:48PM +0100, Gertjan van Wingerde wrote:
> On 11/25/09 20:47, John W. Linville wrote:
> > On Mon, Nov 23, 2009 at 10:44:54PM +0100, Gertjan van Wingerde wrote:
> >> Current rt2x00 drivers may result in a "ieee80211_tx_status: headroom too
> >> small" error message when a frame needs to be properly aligned before
> >> transmitting it.
> >> This is because the space needed to ensure proper alignment isn't
> >> requested from mac80211.
> >> Fix this by adding sufficient amount of alignment space to the amount
> >> of headroom requested for TX frames.
> >>
> >> Reported-by: David Ellingsworth <[email protected]>
> >> Signed-off-by: Gertjan van Wingerde <[email protected]>
> >
> > CC [M] drivers/net/wireless/rt2x00/rt2x00dev.o
> > drivers/net/wireless/rt2x00/rt2x00dev.c: In function ‘rt2x00lib_probe_hw’:
> > drivers/net/wireless/rt2x00/rt2x00dev.c:690: error: ‘IEEE80211_TX_STATUS_HEADROOM’ undeclared (first use in this function)
> > drivers/net/wireless/rt2x00/rt2x00dev.c:690: error: (Each undeclared identifier is reported only once
> > drivers/net/wireless/rt2x00/rt2x00dev.c:690: error: for each function it appears in.)
> >
> > Did I miss a patch?
>
> Patch 7 of the series creates that define within mac80211. It is a prerequisite for this patch.

OK, thanks -- somehow it didn't make it out of my mailbox in into
the mbox file I fed to git...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2009-11-24 17:12:26

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: [rt2x00-users] [PATCH v2 1/8] rt2x00: Only initialize HT on rt2800 devices that support it.

On Monday 23 November 2009, David Ellingsworth wrote:
> On Mon, Nov 23, 2009 at 4:44 PM, Gertjan van Wingerde
> <[email protected]> wrote:
> > Some RT28xx/RT30xx devices don't support 802.11n, when they are combined with
> > the RF2020 chipset. Ensure that HT is disabled for these devices.
> >
> > Signed-off-by: Gertjan van Wingerde <[email protected]>
> > ---
> > ?drivers/net/wireless/rt2x00/rt2800lib.c | ? ?6 +++++-
> > ?1 files changed, 5 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
> > index e94f1e1..fcd0c88 100644
> > --- a/drivers/net/wireless/rt2x00/rt2800lib.c
> > +++ b/drivers/net/wireless/rt2x00/rt2800lib.c
> > @@ -2072,7 +2072,11 @@ int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
> > ? ? ? ?/*
> > ? ? ? ? * Initialize HT information.
> > ? ? ? ? */
> > - ? ? ? spec->ht.ht_supported = true;
> > + ? ? ? if (!rt2x00_rf(chip, RF2020))
> > + ? ? ? ? ? ? ? spec->ht.ht_supported = true;
> > + ? ? ? else
> > + ? ? ? ? ? ? ? spec->ht.ht_supported = false;
> > +
>
> Maybe I'm the only one, but I hate conditional statements with no
> meaning. Maybe this instead?
>
> spec->ht.ht_supported = !rt2x00_rf(chip, RF2020);

I think this would look better as well. But either way you can add my:

Acked-by: Ivo van Doorn <[email protected]>

2009-11-25 21:29:46

by Gertjan van Wingerde

[permalink] [raw]
Subject: Re: [PATCH v2 8/8] rt2x00: Properly request tx headroom for alignment operations.

On 11/25/09 20:47, John W. Linville wrote:
> On Mon, Nov 23, 2009 at 10:44:54PM +0100, Gertjan van Wingerde wrote:
>> Current rt2x00 drivers may result in a "ieee80211_tx_status: headroom too
>> small" error message when a frame needs to be properly aligned before
>> transmitting it.
>> This is because the space needed to ensure proper alignment isn't
>> requested from mac80211.
>> Fix this by adding sufficient amount of alignment space to the amount
>> of headroom requested for TX frames.
>>
>> Reported-by: David Ellingsworth <[email protected]>
>> Signed-off-by: Gertjan van Wingerde <[email protected]>
>
> CC [M] drivers/net/wireless/rt2x00/rt2x00dev.o
> drivers/net/wireless/rt2x00/rt2x00dev.c: In function ‘rt2x00lib_probe_hw’:
> drivers/net/wireless/rt2x00/rt2x00dev.c:690: error: ‘IEEE80211_TX_STATUS_HEADROOM’ undeclared (first use in this function)
> drivers/net/wireless/rt2x00/rt2x00dev.c:690: error: (Each undeclared identifier is reported only once
> drivers/net/wireless/rt2x00/rt2x00dev.c:690: error: for each function it appears in.)
>
> Did I miss a patch?

Patch 7 of the series creates that define within mac80211. It is a prerequisite for this patch.

---
Gertjan.