Re: ieee80211 phy0: rt2x00queue_write_tx_frame: Error - Dropping frame due to full tx queue...?
26.03.2018 12:35, Stanislaw Gruszka: Hi Mathias sorry for the delayed testing. I had to create a new test setup first, fought with buggy hardware and was busy with other stuff. Thanks for doing it. The two attached patches are causing a performance regression for me again: OpenWrt head (forced HT40, 100Mbit wired interface) wireless (iperf client) to wired (iperf server) Interval Transfer Bitrate Retr 0.00-60.00 sec 584 MBytes 81.6 Mbits/sec 666 sender 0.00-60.00 sec 584 MBytes 81.6 Mbits/secreceiver wired (iperf client) to wireless (iperf server) Interval Transfer Bitrate Retr 0.00-60.00 sec 620 MBytes 86.7 Mbits/sec 33 sender 0.00-60.00 sec 617 MBytes 86.2 Mbits/secreceiver OpenWrt head (forced HT40, 100Mbit wired interface) + rt2800_change_rx_ampdu_factor.patch + rt2800_change_ba_size.patch wireless (iperf client) to wired (iperf server) Interval Transfer Bitrate Retr 0.00-60.00 sec 356 MBytes 49.8 Mbits/sec6 sender 0.00-60.00 sec 356 MBytes 49.7 Mbits/secreceiver wired (iperf client) to wireless (iperf server) Interval Transfer Bitrate Retr 0.00-60.00 sec 627 MBytes 87.7 Mbits/sec5 sender 0.00-60.00 sec 626 MBytes 87.5 Mbits/secreceiver Due to the regression I haven't tested your ampdu_density patch so far. Let me hear if you want to see more tests done. Could you test just RX AMPDU patches, i.e. rt2800_change_rx_ampdu_factor.patch rt2800_change_rx_ampdu_density.patch I have somewhat positive results on RX performance on some devices with those. Perhaps you could confirm that :-) This time I've done the test with HT20 only, to not annoy my neighbours. The test setup is the following: ath9k (STA) <=> (AP) o2 Box 6431 (RJ45) <=> desktop With the patches applied the bandwidth is somewhat around 10 MBit/sec lower. Even if I was able to reliable reproduce it, I'm not sure if it is within the measuring tolerance. OpenWrt head mkresin@desktop ~ $ iperf3 -c 192.168.1.156 Connecting to host 192.168.1.156, port 5201 Interval Transfer Bandwidth Retr 0.00-10.00 sec 89.5 MBytes 75.0 Mbits/sec1 sender 0.00-10.00 sec 88.2 MBytes 74.0 Mbits/secreceiver mkresin@desktop ~ $ iperf3 -c 192.168.1.156 -R Connecting to host 192.168.1.156, port 5201 Reverse mode, remote host 192.168.1.156 is sending Interval Transfer Bandwidth Retr 0.00-10.00 sec 70.8 MBytes 59.4 Mbits/sec0 sender 0.00-10.00 sec 70.5 MBytes 59.1 Mbits/secreceiver OpenWrt head + rt2800_change_rx_ampdu_factor.patch + rt2800_change_rx_ampdu_density.patch mkresin@desktop ~ $ iperf3 -c 192.168.1.156 Connecting to host 192.168.1.156, port 5201 Interval Transfer Bandwidth Retr 0.00-10.00 sec 78.2 MBytes 65.6 Mbits/sec0 sender 0.00-10.00 sec 77.1 MBytes 64.7 Mbits/secreceiver mkresin@desktop ~ $ iperf3 -c 192.168.1.156 -R Connecting to host 192.168.1.156, port 5201 Reverse mode, remote host 192.168.1.156 is sending Interval Transfer Bandwidth Retr 0.00-10.00 sec 51.8 MBytes 43.5 Mbits/sec1 sender 0.00-10.00 sec 51.7 MBytes 43.3 Mbits/secreceiver Mathias
Re: [PATCH] ath9k: introduce endian_check module parameter
19.03.2018 09:11, Martin Blumenstingl:
Hello everyone,
On Wed, Mar 14, 2018 at 10:34 PM, Arend van Spriel
wrote:
+ Martin
On 3/14/2018 3:34 PM, Kalle Valo wrote:
Bas Vermeulen writes:
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -67,6 +67,9 @@ static int ath9k_ps_enable;
module_param_named(ps_enable, ath9k_ps_enable, int, 0444);
MODULE_PARM_DESC(ps_enable, "Enable WLAN PowerSave");
+static int ath9k_endian_check;
+module_param_named(endian_check, ath9k_endian_check, int, 0444);
+MODULE_PARM_DESC(endian_check, "Check EEPROM for endianness
compatibility");
#ifdef CONFIG_ATH9K_CHANNEL_CONTEXT
int ath9k_use_chanctx;
@@ -587,7 +590,8 @@ static int ath9k_of_init(struct ath_softc *sc)
ether_addr_copy(common->macaddr, mac);
ah->ah_flags &= ~AH_USE_EEPROM;
- ah->ah_flags |= AH_NO_EEP_SWAP;
+ if (!ath9k_endian_check)
+ ah->ah_flags |= AH_NO_EEP_SWAP;
A bit annoying to have a module parameter, isn't there any automatic
way
to detect/try this? But on the other hand I guess this isn't a common
problem as nobody has reported this before?
There is an automatic way to detect this, but that is disabled by the
AH_NO_EEP_SWAP flag.
Ah, I didn't check the code at all.
The platform initialisation does not set this flag if the endian_check
member of pdata is set to true, but there is no way to not set this
when using a device tree. I used a module parameter instead of a
device tree variable because I don't know of a way to modify the
device tree my PowerMac boots with.
Ok, makes sense. A module parameter is not an ideal solution I guess
it's ok in this case.
Kalle: Are there any changes you want me to make in order to get this
accepted? I didn't see anything for me to resolve, but I may have
missed something.
I can submit a patch to not set the AH_NO_EEP_SWAP flag by default if
you prefer, as that would fix my problem as well. I am just not sure
that doesn't break things for some platform/device I don't have.
I'm not really sure what to do. Basically this is a choise between bad
for user experience (the module parameter) or risk of regressions
(disable AH_NO_EEP_SWAP by default). As ath9k is used in very exotic
hardware I'm starting to lean towards the module parameter approach
(your patch) but would like to know what others think, especially Felix
(CCed).
Hi Kalle,
Sorry for barging in, but I figured git history might tell us something. The
flag was originally added by Felix (commit a59dadbeeaf7 ("ath9k: add support
for endian swap of eeprom from platform data")) and the function
ath9k_of_init() was added by Martin (CCed):
commit 138b41253d9c9f9a06c8b086880cd3e839a23d69
Author: Martin Blumenstingl
Date: Sun Oct 16 22:59:07 2016 +0200
ath9k: parse the device configuration from an OF node
Maybe he recalls what device(s) needed it.
lots embedded devices (supported by OpenWrt) use ath9k chips
my primary target was the BT HomeHub 5A and some AVM Fritz Box (all
using a lantiq SoC, but there are many more). these typically ship
with:
- a generic ath9k EEPROM which is sometimes even stored on NAND (= not
directly connected to the ath9k chipset), which is why we need the
"qca,no-eeprom"
- some ship with a broken EEPROM that enables the 5GHz band on a
2.4GHz-only card, which is why we need "qca,disable-5ghz"
- some ship with an EEPROM that doesn't have a unique mac address (the
mac address is sometimes also stored on NAND), which is why we support
the "mac-address" property
- some ship an EEPROM where only the magic bytes are swapped (but the
content is not), while others have both (magic bytes and content)
byte-swapped
looking at ath9k_of_init it seems that "ah->ah_flags &=
~AH_USE_EEPROM" and "ah->ah_flags |= AH_NO_EEP_SWAP" are called
unconditionally.
maybe these should be part of the "qca,no-eeprom" if-block a few lines above
I also added Mathias to the CC list
@Mathias: I believe all our .dts files in OpenWrt which specify an
ath9k chipset also set the "qca,no-eeprom" property, right (a quick
check suggests "yes")?
Yes, they all should have the qca,no-eeprom property. If not, it is a
bug in the OpenWrt dts files.
Mathias
Re: ieee80211 phy0: rt2x00queue_write_tx_frame: Error - Dropping frame due to full tx queue...?
07.03.2018 13:29, Stanislaw Gruszka: I forgot to attach the patches, do it now. On Wed, Mar 07, 2018 at 01:27:01PM +0100, Stanislaw Gruszka wrote: On Thu, Mar 01, 2018 at 04:30:10PM +0100, Daniel Golle wrote: [forwarding to all other involved players] On Thu, Mar 01, 2018 at 05:50:51PM +0300, Jamie Stuart wrote: Hi Daniel, The driver seems much improved after this fix. it's about those two [PATCH 1/2] rt2x00: pause almost full queue early [PATCH 2/2] rt2x00: do not pause queue unconditionally on error path Under very heavy load (30 clients downloading multi-GB files from SD card on the server concurrently), wifi dies with errors: This is some testbed? Could you share how did you setup such environment and what are client devices ? [ 7794.230376] ieee80211 phy0: rt2x00lib_rxdone_read_signal: Warning - Frame received with unrecognized signal, mode=0x0001, signal=0x010c, type=4 This is indicator that HW/FW has a problem. There could be various reasons for that. One possible I can also observe in my setup,is strange mishmash of seq on frames which were not acked in BlockACK and had to be resent. This can happen when many frames are wrongly decoded (i.e. when there is bad radio condition or we have not correct low level RF/BBP setup for a Ralink device). To mitigate that problem we can limit length of agreggeted AMPDU frame. I attached two patches which do that. One for RX side second for TX side. Please check if they make a diffrent. You can also hardcode ba_size = 0 for those 30 clients setup. Note the patches can cause (possibly small) perfromance degradation on good setups. Mathias, could you check them as well and see if they do not cause performance regression on your device ? Lastly when I changed ba_size setting, it was a problem on your setup. Hey Stansilaw, sorry for the delayed testing. I had to create a new test setup first, fought with buggy hardware and was busy with other stuff. The two attached patches are causing a performance regression for me again: OpenWrt head (forced HT40, 100Mbit wired interface) wireless (iperf client) to wired (iperf server) Interval Transfer Bitrate Retr 0.00-60.00 sec 584 MBytes 81.6 Mbits/sec 666 sender 0.00-60.00 sec 584 MBytes 81.6 Mbits/secreceiver wired (iperf client) to wireless (iperf server) Interval Transfer Bitrate Retr 0.00-60.00 sec 620 MBytes 86.7 Mbits/sec 33 sender 0.00-60.00 sec 617 MBytes 86.2 Mbits/secreceiver OpenWrt head (forced HT40, 100Mbit wired interface) + rt2800_change_rx_ampdu_factor.patch + rt2800_change_ba_size.patch wireless (iperf client) to wired (iperf server) Interval Transfer Bitrate Retr 0.00-60.00 sec 356 MBytes 49.8 Mbits/sec6 sender 0.00-60.00 sec 356 MBytes 49.7 Mbits/secreceiver wired (iperf client) to wireless (iperf server) Interval Transfer Bitrate Retr 0.00-60.00 sec 627 MBytes 87.7 Mbits/sec5 sender 0.00-60.00 sec 626 MBytes 87.5 Mbits/secreceiver Due to the regression I haven't tested your ampdu_density patch so far. Let me hear if you want to see more tests done. Mathias
Re: [PATCH v12] ath10k: add LED and GPIO controlling support for various chipsets
10.03.2018 08:56, Sebastian Gottschall: taken a look at the specific code, and from my point of view the code that sets up the LED (including callback) is so trivial that it's simply not worth dealing with adding the leds-gpio driver to the mix. It adds extra complexity and an extra dependency for no reason at all. There's no extra functionality to be gained by using it. Stupid question: If the LED driver isn't using the GPIO subsystem (when enabled), what happens if the user uses the GPIO subsystem to fiddle with the pin the LED is connected to? in our case here it can coexist and will not have a negative effect since the led will still run. (except if you reconfigure the gpio to input) for sure it makes no sense. Well, it will have negative effects as I noticed in OpenWrt. If the same GPIO is controlled by the internal LED function it can't be really used via the GPIO controller as there will be "bogus" changes of the GPIO pin state. but however i can block the gpio for beeing reused if the led driver took it. this has been made in ath9k for instance. Most likely I'm not aware of the upstream code you are talking about. But OpenWrt has a custom patch which registers the ath9k pins as GPIO controller. As soon as there are pins, manufactures will use them and the assumption about a default led connected to a specific pin will fail. I've seen (home)routers routers using the "default" ath9k LED pin as button or for LEDs with a different purpose than "wireless". I never was able to find any kind of information which explains why exactly this specific pin is used for the ath9k-led. But that's another story. My solution for OpenWrt was a patch which prevents the creation of the ath9k-led if the "default" pin is used as GPIO. If mach files are used, the ath9k led isn't created (ath9k led pin is set to -1) in case the platform data have a button or led using the same pin. If the device tree is used, the ath9k led isn't created (ath9k led pin is set to -1) if the devicetree node for the ath9k device has the gpio-controller property. I'm not really proud of the code and it can be most likely done better, but it fixes the issue outlined above. I've done it this way since the use of the GPIO controller should always override any kind of default LEDs. In the end ath[0|10]k-led is only required for hardware configurations where the wireless isn't fixed like with miniPCIe, USB ... wireless cards. If the configuration is fixed - like it is for most of the (home)routers - the GPIO controller can be registered via the devicetree and gpio-leds can be used. Something similar can be most likely done via mach files (I barely touch mach files). I'm aware of the issue since the first version of your patch. But since I'm very interested in getting the ath10k pins exposed as gpio controller, I planned to add a workaround similar to what we have for ath9k to OpenWrt. Mathias
Re: [PATCH] rt2x00: call clk_get_rate only if we have a clock
08.07.2017 13:15, Jonas Gorski: Hi, On 8 July 2017 at 09:27, Mathias Kresin wrote: If clk_get returns an error, rt2x00dev->clk is set to NULL. In contrast to the common clock framework provided clk_get_rate(), at least the ramips and bcm63xx legacy implementation of the clk API access the rate member of the clk struct without a NULL check. This results into a kernel panic if we do not have a (SoC) clock. Call clk_get_rate only if we have a clock to fix the issues. This approach is similar to what is done in the kernel at various places. Usually clk_get_rate() is only called if clk_get_rate() doesn't return Did you mean clk_get() as the second one? Yes I do. ... is only called if clk_get() doesn't return an error. an error. Signed-off-by: Mathias Kresin Tbh, I'd rather have this fixed at the source than adding a workaround to consumers, to have a consistent API across implementations (with drivers/clk/clk.c as the reference). And there don't seem that many, at least searching for clk_get_rate gave me only two handful of implementations of which many already check for NULL. I do not necessarily see an error in the archs legacy clock implementation. It rather looks like it works with the common clock framework due the lucky coincident that someone has added (an extra) null check. I only had a brief look at the common clock framework, but it seams to me one should not pass the error returned by clk_get() and/or call clk_get_rate() at all if there was an error. That is what I've already seen and mentioned in the commit message. The only difference here is that we do not have the error check and the clk_get_rate() call in the same function. rt2800_clk_is_20mhz() isn't aware that clk_get() failed. rt2x00dev->clk is set back to NULL in rt2x00soc_probe() in case of an clk_get error. Mathias
[PATCH] rt2x00: call clk_get_rate only if we have a clock
If clk_get returns an error, rt2x00dev->clk is set to NULL. In
contrast to the common clock framework provided clk_get_rate(), at
least the ramips and bcm63xx legacy implementation of the clk API
access the rate member of the clk struct without a NULL check. This
results into a kernel panic if we do not have a (SoC) clock.
Call clk_get_rate only if we have a clock to fix the issues. This
approach is similar to what is done in the kernel at various places.
Usually clk_get_rate() is only called if clk_get_rate() doesn't return
an error.
Signed-off-by: Mathias Kresin
---
Resend, the first mail had the wrong list in cc.
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index d11c7b2..2a525b9 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -2059,6 +2059,9 @@ static void rt2800_config_lna_gain(struct rt2x00_dev
*rt2x00dev,
static inline bool rt2800_clk_is_20mhz(struct rt2x00_dev *rt2x00dev)
{
+ if (!rt2x00dev->clk)
+ return 0;
+
return clk_get_rate(rt2x00dev->clk) == 2000;
}
--
2.7.4
Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor
2016-11-16 13:02 GMT+01:00 Stanislaw Gruszka : > On Wed, Nov 16, 2016 at 09:07:00AM +0100, Mathias Kresin wrote: >> Here are the results of the requested tests. Please keep in mind, I'm not in >> a lab environment: >> >> LEDE head >> connect >> action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3 >> action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3 >> action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3 >> action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3 > > No problem here - buf_size corresponds to ampdu_factor. > >> ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed >> to flush >> ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed >> to flush > > I think we do not give device enough time to post AMPDU consisted > with bigger amount of frames. If we want to increase ba_size we will > need also some other changes in the driver. Anyway I already request > Kalle to drop this patch. I assume other patches do not cause > regression for you, correct? Correct. Just 05/10 caused issues.
Re: [PATCH 04/10] rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE
14.11.2016 09:42, Stanislaw Gruszka: On Sat, Nov 05, 2016 at 01:55:14PM +0100, Mathias Kresin wrote: 02.11.2016 15:10, Stanislaw Gruszka: We already initlized WPDMA_GLO_CFG_WP_DMA_BURST_SIZE to 3 on rt2800_init_registers(). Signed-off-by: Stanislaw Gruszka --- drivers/net/wireless/ralink/rt2x00/rt2800lib.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c index feceb13..9ecdc4c 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c @@ -6756,7 +6756,6 @@ int rt2800_enable_radio(struct rt2x00_dev *rt2x00dev) rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, ®); rt2x00_set_field32(®, WPDMA_GLO_CFG_ENABLE_TX_DMA, 1); rt2x00_set_field32(®, WPDMA_GLO_CFG_ENABLE_RX_DMA, 1); - rt2x00_set_field32(®, WPDMA_GLO_CFG_WP_DMA_BURST_SIZE, 2); More a notice than a potential issue since I don't have much knowledge about the driver/chip internals. But WPDMA_GLO_CFG_WP_DMA_BURST_SIZE in rt2800_init_registers() is set conditionally for rt2x00_is_usb(rt2x00dev), where this one is set unconditionally. Not sure if this change has side effects. Default HW setting of WP_DMA_BURTS_SIZE is 2, hence patch does not change behaviour on PCI devices. However I looked at RT3290 and RT5592 PCI vendor drivers and there this value is initialized to 3. So I think we can remove is_usb condition in rt2800_init_registers(). Shouldn't one of the explanations (depending on where you decide to set WP_DMA_BURTS_SIZE) go into the commit message? As said in my last mail, without further explanation it looks like introducing a bug. Mathias
Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor
14.11.2016 13:43, Stanislaw Gruszka:
On Mon, Nov 14, 2016 at 09:45:36AM +0100, Stanislaw Gruszka wrote:
Could you check below patch and see if it helps? If it does not,
could you printk sta->ht_cap.ampdu_density and ba_size values
and provide them here.
Actually please print parameters from below patch. I think ba_size
should be based on per TID buf_size instead of ampdu_factor, in case
STA has buf size different for some TIDs.
Also adding Felix to cc since my orginal patch:
http://marc.info/?l=linux-wireless&m=147809595316289&w=2
was shamelessly stolen from mt76 driver. Perhaps Felix could provide
us some expertise.
Thanks
Stanislaw
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 2515702..35bc38c 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -7950,6 +7950,8 @@ int rt2800_ampdu_action(struct ieee80211_hw *hw, struct
ieee80211_vif *vif,
struct rt2x00_sta *sta_priv = (struct rt2x00_sta *)sta->drv_priv;
int ret = 0;
+ printk("action %d sta %pM tid %u buf_size %u ampdu_factor %u\n", params->action,
sta->addr, params->tid, params->buf_size, sta->ht_cap.ampdu_factor);
+
/*
* Don't allow aggregation for stations the hardware isn't aware
* of because tx status reports for frames to an unknown station
Here are the results of the requested tests. Please keep in mind, I'm
not in a lab environment:
LEDE head
connect
action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
wireless (iperf client) to wired (iperf server)
Interval Transfer Bandwidth
0.00-60.01 sec 544 MBytes 76.1 Mbits/sec sender
0.00-60.01 sec 544 MBytes 76.1 Mbits/sec receiver
wired (iperf client) to wireless (iperf server)
Interval Transfer Bandwidth Retr
0.00-60.00 sec 609 MBytes 85.1 Mbits/sec 96 sender
0.00-60.00 sec 606 MBytes 84.8 Mbits/sec receiver
on interface down
action 4 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3
LEDE + vanilla driver (without LEDE rt2x00 patches)
connect
action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
wireless (iperf client) to wired (iperf server)
Interval Transfer Bandwidth
0.00-60.02 sec 522 MBytes 73.0 Mbits/sec sender
0.00-60.02 sec 522 MBytes 73.0 Mbits/sec receiver
wired (iperf client) to wireless (iperf server)
Interval Transfer Bandwidth Retr
0.00-60.00 sec 583 MBytes 81.5 Mbits/sec 104 sender
0.00-60.00 sec 581 MBytes 81.2 Mbits/sec receiver
on interface down
action 4 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3
LEDE + vanilla driver + patchset
connect
action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
wireless (iperf client) to wired (iperf server)
Interval Transfer Bandwidth
0.00-60.02 sec 377 MBytes 52.7 Mbits/sec sender
0.00-60.02 sec 377 MBytes 52.6 Mbits/sec receive
on interface down
action 4 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3
ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
to flush
ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue 2 failed
to flush
wired (iperf client) to wireless (iperf server)
* not reliable reproducible stalls/reconnects:
action 1 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 0 ampdu_factor 3
action 2 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 0 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 6 sta 9c:f3:87:bc:AA:BB tid 0 buf_size 64 ampdu_factor 3
action 0 sta 9c:f3:87:bc:AA:BB tid 6 buf_size 64 ampdu_factor 3
* one ti
Re: [PATCH 05/10] rt2800: make ba_size depend on ampdu_factor
02.11.2016 15:11, Stanislaw Gruszka:
We can calculate BA window size (max number of pending frames not
yet block acked) of remote station using Maximum A-MPDU length factor
for that station.
Signed-off-by: Stanislaw Gruszka
---
drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 15 ++-
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
index 68b620b..9da89e3 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
@@ -305,14 +305,19 @@ static void rt2x00queue_create_tx_descriptor_ht(struct
rt2x00_dev *rt2x00dev,
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
struct ieee80211_tx_rate *txrate = &tx_info->control.rates[0];
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
- struct rt2x00_sta *sta_priv = NULL;
+ u8 ba_size = 0;
if (sta) {
- txdesc->u.ht.mpdu_density =
- sta->ht_cap.ampdu_density;
+ struct rt2x00_sta *sta_priv = sta_to_rt2x00_sta(sta);
- sta_priv = sta_to_rt2x00_sta(sta);
+ txdesc->u.ht.mpdu_density = sta->ht_cap.ampdu_density;
txdesc->u.ht.wcid = sta_priv->wcid;
+
+ if (!(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)) {
+ ba_size = IEEE80211_MIN_AMPDU_BUF;
+ ba_size <<= sta->ht_cap.ampdu_factor;
+ ba_size = min_t(int, 63, ba_size - 1);
+ }
}
/*
@@ -345,7 +350,7 @@ static void rt2x00queue_create_tx_descriptor_ht(struct
rt2x00_dev *rt2x00dev,
return;
}
- txdesc->u.ht.ba_size = 7;/* FIXME: What value is needed? */
+ txdesc->u.ht.ba_size = ba_size;
/*
* Only one STBC stream is supported for now.
Having this patch applied, the throughput on a vgv7510kw22 (RT3062F) in
AP mode using LEDE head is decreased by somewhat around 10 Mbits/sec.
I'm using iperf3 for throughput tests and having this patch reverted the
throughout is back to 80 Mbits/sec.
When bringing down the wifi interface the following messages are logged
with the patch applied:
[ 281.738373] ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue
2 failed to flush
[ 281.906380] ieee80211 phy0: rt2x00queue_flush_queue: Warning - Queue
2 failed to flush
Mathias
Re: [PATCH 04/10] rt2800: do not overwrite WPDMA_GLO_CFG_WP_DMA_BURST_SIZE
02.11.2016 15:10, Stanislaw Gruszka: We already initlized WPDMA_GLO_CFG_WP_DMA_BURST_SIZE to 3 on rt2800_init_registers(). Signed-off-by: Stanislaw Gruszka --- drivers/net/wireless/ralink/rt2x00/rt2800lib.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c index feceb13..9ecdc4c 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c @@ -6756,7 +6756,6 @@ int rt2800_enable_radio(struct rt2x00_dev *rt2x00dev) rt2800_register_read(rt2x00dev, WPDMA_GLO_CFG, ®); rt2x00_set_field32(®, WPDMA_GLO_CFG_ENABLE_TX_DMA, 1); rt2x00_set_field32(®, WPDMA_GLO_CFG_ENABLE_RX_DMA, 1); - rt2x00_set_field32(®, WPDMA_GLO_CFG_WP_DMA_BURST_SIZE, 2); More a notice than a potential issue since I don't have much knowledge about the driver/chip internals. But WPDMA_GLO_CFG_WP_DMA_BURST_SIZE in rt2800_init_registers() is set conditionally for rt2x00_is_usb(rt2x00dev), where this one is set unconditionally. Not sure if this change has side effects. Mathias
[PATCH v2] rt2x00: add support for mac addr from device tree
On some devices the EEPROMs of Ralink Wi-Fi chips have a default Ralink
MAC address set (RT3062F: 00:0C:43:30:62:00, RT3060F:
00:0C:43:30:60:00). Using multiple of these devices in the same network
can cause nasty issues.
Allow to override the MAC in the EEPROM with (a known good) one set in
the device tree to bypass the issue.
Signed-off-by: Mathias Kresin
---
As discussed before, forcing a random MAC for known default MACs would
be a wonky approach. I wouldn't be surprise to see ODMs setting an ODM
specific default MAC in the EEPROM. This would require a permanent
update of the list of known default MACs.
Changes in v2:
- new commit message, the former one was incomprehensible
drivers/net/wireless/ralink/rt2x00/rt2400pci.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt2500pci.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt2500usb.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt2x00.h| 1 +
drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 17 +
drivers/net/wireless/ralink/rt2x00/rt61pci.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt73usb.c | 5 +
8 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
index 155f343..085c5b4 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
@@ -1459,10 +1459,7 @@ static int rt2400pci_validate_eeprom(struct rt2x00_dev
*rt2x00dev)
* Start validation of the data that has been read.
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
- if (!is_valid_ether_addr(mac)) {
- eth_random_addr(mac);
- rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
- }
+ rt2x00lib_set_mac_address(rt2x00dev, mac);
rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
if (word == 0x) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
index 2553cdd..9832fd5 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
@@ -1585,10 +1585,7 @@ static int rt2500pci_validate_eeprom(struct rt2x00_dev
*rt2x00dev)
* Start validation of the data that has been read.
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
- if (!is_valid_ether_addr(mac)) {
- eth_random_addr(mac);
- rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
- }
+ rt2x00lib_set_mac_address(rt2x00dev, mac);
rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
if (word == 0x) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
index 2d64611..cd3ab5a 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
@@ -1349,10 +1349,7 @@ static int rt2500usb_validate_eeprom(struct rt2x00_dev
*rt2x00dev)
* Start validation of the data that has been read.
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
- if (!is_valid_ether_addr(mac)) {
- eth_random_addr(mac);
- rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
- }
+ rt2x00lib_set_mac_address(rt2x00dev, mac);
rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
if (word == 0x) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index bf3f0a3..59c49af 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -6919,10 +6919,7 @@ static int rt2800_validate_eeprom(struct rt2x00_dev
*rt2x00dev)
* Start validation of the data that has been read.
*/
mac = rt2800_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
- if (!is_valid_ether_addr(mac)) {
- eth_random_addr(mac);
- rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
- }
+ rt2x00lib_set_mac_address(rt2x00dev, mac);
rt2800_eeprom_read(rt2x00dev, EEPROM_NIC_CONF0, &word);
if (word == 0x) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
index f68d492..aa3d4cee 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
@@ -1403,6 +1403,7 @@ static inline void rt2x00debug_dump_frame(struct
rt2x00_dev *rt2x00dev,
*/
u32 rt2x00lib_get_bssidx(struct rt2x00_dev *rt2x00dev,
struct ieee80211_vif *vif);
+void rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x00dev, u8
*eeprom_mac_addr);
/*
* Interrupt context handlers.
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00
Re: [PATCH] rt2x00: add support for mac addr from device tree
2016-08-25 15:19 GMT+02:00 Stanislaw Gruszka : > On Thu, Aug 25, 2016 at 01:12:22PM +0200, Mathias Kresin wrote: >> 2016-08-25 11:33 GMT+02:00 Stanislaw Gruszka : >> > On Thu, Aug 25, 2016 at 10:19:22AM +0200, Mathias Kresin wrote: >> CPE = Customer Premises Equipment or the small plastic box from your >> ISP at home. The whole point of the patch is that the MAC stored in >> the wifi EEPROM might not be unique and need to be overridden. I'm >> aware of three different "home router", where each model has the same >> generic ralink MAC address stored in the wifi EEPROM. This can cause >> nasty issues. > > I think we still want MAC from EEPROM instead of random one on systems > without OF. Otherwise we could just use random MAC every time, but this > does not seems lika a good idea. Either I got you wrong, the code does something different than I intended/tested or you misread the code. The mac address stored in the EEPROM is only overridden in case: a) a mac address is defined in the device tree. b) invalid/no mac stored in EEPROM => random one If none of the above is true, the EEPROM mac will be used. What I've added is a). Everything else is the same as before. > Can we check against that particular MAC that repeats on those CPEs and > if it match use random one ? Or use some other identification to find > out that EEPROM MAC is not good ? IMHO this would be a wonky approach. I had a look at the EEPROM MACs from two of the affected devices and spotted just now a coherence with the used wifi chip: VGV7510KW22 (RT3062F): 00:0C:43:30:62:00 ARV7506 (RT3060F): 00:0C:43:30:60:00 IMHO, it would be reckless to assume that it's the same for other affected models. You know, ODMs doing silly things. Personally I prefer to deal with the issue in device tree rather than forcing a random MAC address. This way I can set the MAC used with the stock firmware to fix the issue. Mathias
Re: [PATCH] rt2x00: add support for mac addr from device tree
2016-08-25 11:33 GMT+02:00 Stanislaw Gruszka :
>
> On Thu, Aug 25, 2016 at 10:19:22AM +0200, Mathias Kresin wrote:
> > The EEPROM used on some CPEs has for every device the same generic
> > ralink mac in EEPROM and needs to be overridden.
>
> I don't know what is CPE, but even if I would know that, I most likely
> sill will not understand that description.
Well, seams to me the commit message can be improved. If a v2 is
required or a v2 is required because of the commit message, I'll take
care of it.
CPE = Customer Premises Equipment or the small plastic box from your
ISP at home. The whole point of the patch is that the MAC stored in
the wifi EEPROM might not be unique and need to be overridden. I'm
aware of three different "home router", where each model has the same
generic ralink MAC address stored in the wifi EEPROM. This can cause
nasty issues.
> > +++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
> > @@ -1459,10 +1459,7 @@ static int rt2400pci_validate_eeprom(struct
> > rt2x00_dev *rt2x00dev)
> >* Start validation of the data that has been read.
> >*/
> > mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
> > - if (!is_valid_ether_addr(mac)) {
> > - eth_random_addr(mac);
> > - rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
> > - }
> > + rt2x00lib_set_mac_address(rt2x00dev, mac);
>
> > +#include
> > +#include
> >
> > #include "rt2x00.h"
> > #include "rt2x00lib.h"
> > @@ -931,6 +933,21 @@ static void rt2x00lib_rate(struct ieee80211_rate
> > *entry,
> > entry->flags |= IEEE80211_RATE_SHORT_PREAMBLE;
> > }
> >
> > +void rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x00dev, u8
> > *eeprom_mac_addr)
> > +{
> > + const char *mac_addr;
> > +
> > + mac_addr = of_get_mac_address(rt2x00dev->dev->of_node);
>
> Shouldn't use dev_of_node(&rt2x00dev->dev) and check against NULL ?
Not sure if dev_of_node() is meant to be used by every driver. Or at
least the function is only used by base stuff and not by any driver.
The NULL check doesn't seam to me required. The of_node is finally
passed to __of_find_property which does the NULL check before using
of_node.
Mathias
[PATCH] rt2x00: add support for mac addr from device tree
The EEPROM used on some CPEs has for every device the same generic
ralink mac in EEPROM and needs to be overridden.
Signed-off-by: Mathias Kresin
---
drivers/net/wireless/ralink/rt2x00/rt2400pci.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt2500pci.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt2500usb.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt2x00.h| 1 +
drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 17 +
drivers/net/wireless/ralink/rt2x00/rt61pci.c | 5 +
drivers/net/wireless/ralink/rt2x00/rt73usb.c | 5 +
8 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
index 155f343..085c5b4 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2400pci.c
@@ -1459,10 +1459,7 @@ static int rt2400pci_validate_eeprom(struct rt2x00_dev
*rt2x00dev)
* Start validation of the data that has been read.
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
- if (!is_valid_ether_addr(mac)) {
- eth_random_addr(mac);
- rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
- }
+ rt2x00lib_set_mac_address(rt2x00dev, mac);
rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
if (word == 0x) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
index 2553cdd..9832fd5 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2500pci.c
@@ -1585,10 +1585,7 @@ static int rt2500pci_validate_eeprom(struct rt2x00_dev
*rt2x00dev)
* Start validation of the data that has been read.
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
- if (!is_valid_ether_addr(mac)) {
- eth_random_addr(mac);
- rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
- }
+ rt2x00lib_set_mac_address(rt2x00dev, mac);
rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
if (word == 0x) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
index 2d64611..cd3ab5a 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2500usb.c
@@ -1349,10 +1349,7 @@ static int rt2500usb_validate_eeprom(struct rt2x00_dev
*rt2x00dev)
* Start validation of the data that has been read.
*/
mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
- if (!is_valid_ether_addr(mac)) {
- eth_random_addr(mac);
- rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
- }
+ rt2x00lib_set_mac_address(rt2x00dev, mac);
rt2x00_eeprom_read(rt2x00dev, EEPROM_ANTENNA, &word);
if (word == 0x) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index bf3f0a3..59c49af 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -6919,10 +6919,7 @@ static int rt2800_validate_eeprom(struct rt2x00_dev
*rt2x00dev)
* Start validation of the data that has been read.
*/
mac = rt2800_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
- if (!is_valid_ether_addr(mac)) {
- eth_random_addr(mac);
- rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", mac);
- }
+ rt2x00lib_set_mac_address(rt2x00dev, mac);
rt2800_eeprom_read(rt2x00dev, EEPROM_NIC_CONF0, &word);
if (word == 0x) {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
index f68d492..aa3d4cee 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
@@ -1403,6 +1403,7 @@ static inline void rt2x00debug_dump_frame(struct
rt2x00_dev *rt2x00dev,
*/
u32 rt2x00lib_get_bssidx(struct rt2x00_dev *rt2x00dev,
struct ieee80211_vif *vif);
+void rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x00dev, u8
*eeprom_mac_addr);
/*
* Interrupt context handlers.
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 4e0c565..d659250 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -26,6 +26,8 @@
#include
#include
#include
+#include
+#include
#include "rt2x00.h"
#include "rt2x00lib.h"
@@ -931,6 +933,21 @@ static void rt2x00lib_rate(struct ieee80211_rate *entry,
entry->flags |= IEEE80211_RATE_SHORT_PREAMBLE;
}
+void rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x0
