Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH 5/5] ath9k: Make EEPROM endianness swapping configurable via devicetree
From: Martin Blumenstingl @ 2016-08-28 21:10 UTC (permalink / raw)
  To: Arnd Bergmann, nbd
  Cc: kvalo, ath9k-devel, linux-wireless, ath9k-devel, devicetree,
	mark.rutland, chunkeey, robh+dt
In-Reply-To: <2303906.Xe1WvCfP0V@wuerfel>

On Mon, Aug 22, 2016 at 1:52 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday, August 21, 2016 4:49:06 PM CEST Martin Blumenstingl wrote:
>> +- qca,check-eeprom-endianness: When enabled, the driver checks if the
>> +                               endianness of the EEPROM (using two checks,
>> +                               one is based on the two magic bytes at the
>> +                               start of the EEPROM and a second one which
>> +                               relies on a flag within the EEPROM data)
>> +                               matches the host system's native endianness.
>> +                               The data will be swapped accordingly if there
>> +                               is a mismatch.
>> +                               Leaving this disabled means that the EEPROM
>> +                               data will always be interpreted in the
>> +                               system's native endianness.
>> +                               Enable this option only when the EEPROMs
>> +                               endianness identifiers are known to be
>> +                               correct, because otherwise the EEPROM data
>> +                               may be swapped and thus interpreted
>> +                               incorrectly.
>
> The property should not describe the driver behavior, but instead
> describe what the hardware is like.
>
> A default of "system's native endianess" should not be specified
> in a binding, as the same DTB can be used with both big-endian or
> little-endian kernels on some architectures (ARM and PowerPC among
> others), so disabling the property means that it is guaranteed to
> be broken on one of the two.
OK, I went back to have a separate look at the whole issue again.
Let's recap, there are two types of swapping:
1. swab16 for the whole EEPROM data
2. EEPROM format specific swapping (for all u16 and u32 values)

Actually I am not 100% sure what #1 exists. In OpenWrt and LEDE it's
only used by the brcm63xx and lantiq targets (I personally have only
lantiq based devices, so that's what I can test with).
Without patch 4 from this series the EEPROM data is loaded like this:
1. out of tree code extracts the EEPROM data from NOR/SPI/NAND flash
2. board specific entry (usually device-tree) tells the code to apply
swab16 to the data
3. board specific entry (usually device-tree again) sets the
"endian_check" property in the ath9k_platform_data to true
4.a ath9k sees that the magic bytes are not matching and applies swab16
4.b while doing that it notifies the EEPROM format specific swapping

However, with patch 4 from this series steps 4.a and 4.b are replaced with:
4. ath9k checks the eepMisc field of the EEPROM and applies the EEPROM
format specific swapping

Currently this code is still guarded by a check whether swapping is
enabled or not.
However, FreeBSD uses the same check but has no guarding if-clause for it.

TL;DR: if we remove if (ah->ah_flags & AH_NO_EEP_SWAP) from patch 4 we
don't need an extra device-tree property.

The question is how we can test this properly:
I can test this on the boards I own (3 in total) - but I don't think
that this is enough.
Maybe we can test this together with some LEDE people - this should
get us test-coverage for embedded devices.
I'm open to more/better suggestions


Regards,
Martin

^ permalink raw reply

* Re: [PATCH v5 1/3] Documentation: dt: net: add ath9k wireless device binding
From: Martin Blumenstingl @ 2016-08-28 20:51 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: ath9k-devel, devicetree, linux-wireless, ath9k-devel, mcgrof,
	mark.rutland, robh+dt, kvalo, chunkeey, arend.vanspriel,
	julian.calaby, bjorn, linux
In-Reply-To: <2913835.hr5kg63Vcv@wuerfel>

On Mon, Aug 22, 2016 at 11:08 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Sunday, August 21, 2016 4:31:03 PM CEST Martin Blumenstingl wrote:
>> +pci {
>> +       pcie@0 {
>> +               reg = <0 0 0 0 0>;
>
> It's not clear what these two nodes refer to in the example. Is the top-level
> node the PCI host bridge, and the second node the first PCIe port?
>
> Maybe add the properties from a real system here to make this a little
> clearer.
>
> The unit address for the slot should be "00,0", not "0" here.
good point - I'll skip the part where I am describing the bridge as
well and keep it simple.

>> +               ath9k@0,0 {
>
> According to the PCI binding, the name should be the same as the
> compatible string here, or match the class code in the table.
The original example was from an actual system (where an ath9k is
connected to the PCIe bug). Unfortunately the PCIe driver contains
some hacks, so I'm not sure if these values serve as a good example.
Thus I took an example from a device where the ath9k chip is connected
via PCI (no "express" - found in sysfs at:
/sys/bus/pci/devices/0000:00:0e.0):
&pci0 {
    ath9k@168c,002d {
        compatible = "pci168c,002d";
        reg = <0x7000 0 0 0 0>;
        qca,disable-5ghz;
    };
};

>> +                       compatible = "pci168c,0030";
>> +                       reg = <0 0 0 0 0>;
>
> Are the device/fn numbers all zero on your system? This is a bit
> confusing, as it's not immediately clear what the reg properties
> refers to. Also, I think the length should reflect the actual length
> of the config space, either 0x100 or 0x1000.
The first issue is solved with the updated example (see above).
Where would the size go (is it the second-last or last value)?


Thanks for your patience!
Regards,
Martin

^ permalink raw reply

* [PATCH] staging: wilc1000: fix spelling mistake: "retyring" -> "retrying"
From: Colin King @ 2016-08-28 11:28 UTC (permalink / raw)
  To: Aditya Shankar, Ganesh Krishna, Greg Kroah-Hartman,
	linux-wireless, devel
  Cc: linux-kernel

From: Colin Ian King <colin.king@canonical.com>

trivial fix to spelling mistake in dev_err message

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/staging/wilc1000/wilc_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
index 22cf4b7..0f8d625 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -871,7 +871,7 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 		/* Read failed. Try with CRC off. This might happen when module
 		 * is removed but chip isn't reset*/
 		g_spi.crc_off = 1;
-		dev_err(&spi->dev, "Failed internal read protocol with CRC on, retyring with CRC off...\n");
+		dev_err(&spi->dev, "Failed internal read protocol with CRC on, retrying with CRC off...\n");
 		if (!spi_internal_read(wilc, WILC_SPI_PROTOCOL_OFFSET, &reg)) {
 			/* Reaad failed with both CRC on and off, something went bad */
 			dev_err(&spi->dev,
-- 
2.9.3

^ permalink raw reply related

* [PATCH] mac80211: fix tim recalculation after PS response
From: Felix Fietkau @ 2016-08-28 11:10 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes

Handle the case where the mac80211 intermediate queues are empty and the
driver has buffered frames

Fixes: ba8c3d6f16a1 ("mac80211: add an intermediate software queue implementation")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/mac80211/sta_info.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 19f14c9..251e14e 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1616,7 +1616,6 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
 
 		sta_info_recalc_tim(sta);
 	} else {
-		unsigned long tids = sta->txq_buffered_tids & driver_release_tids;
 		int tid;
 
 		/*
@@ -1648,7 +1647,8 @@ ieee80211_sta_ps_deliver_response(struct sta_info *sta,
 		for (tid = 0; tid < ARRAY_SIZE(sta->sta.txq); tid++) {
 			struct txq_info *txqi = to_txq_info(sta->sta.txq[tid]);
 
-			if (!(tids & BIT(tid)) || txqi->tin.backlog_packets)
+			if (!(driver_release_tids & BIT(tid)) ||
+			    txqi->tin.backlog_packets)
 				continue;
 
 			sta_info_recalc_tim(sta);
-- 
2.8.4

^ permalink raw reply related

* RE: [PATCH] wlcore: mesh: add zone time sync support
From: Mishol, Guy @ 2016-08-28  7:34 UTC (permalink / raw)
  To: Pedersen, Thomas; +Cc: Kalle Valo, linux-wireless@vger.kernel.org
In-Reply-To: <1472151013.3432.33.camel@qca.qualcomm.com>

T24gVGh1LCBBdWcgMjUsIDIwMTYgYXQgMjE6NTA6NTksIFBlZGVyc2VuLCBUaG9tYXMgd3JvdGU6
DQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0hdIHdsY29yZTogbWVzaDogYWRkIHpvbmUgdGltZSBzeW5j
IHN1cHBvcnQNCj4gDQo+IE9uIFN1biwgMjAxNi0wOC0yMSBhdCAxMDoxMyArMDAwMCwgTWlzaG9s
LCBHdXkgd3JvdGU6DQo+ID4gS2FsbGUvVGhvbWFzLA0KPiA+IFRoYW5rcyBmb3IgeW91ciBmZWVk
YmFjay4NCj4gPg0KPiA+IFRoaXMgdGltZSBzeW5jIHN1cHBvcnQgaXMgZGlmZmVyZW50IGZyb20g
dGhlIG9uZSB0aGF0IG1hYzgwMjExIA0KPiA+IG1haW50YWlucw0KPiB3aXRoIG1lc2ggcGVlcnMu
DQo+ID4gVGhpcyB0aW1lIHN5bmMgaXMgbW9zdGx5IHVzZWQgYnkgdXBwZXIgbGF5ZXJzIGZvciBz
ZXZlcmFsIA0KPiA+IGFwcGxpY2F0aW9ucyAobGlrZQ0KPiBhdWRpbykuDQo+ID4gSW4gdGhpcyBj
YXNlIHdlIGFsbG93IHRoZSB1c2VyIHRvIGNvbmZpZ3VyZSBmb3IgZWFjaCB6b25lIHdobyB3aWxs
IA0KPiA+IGJlIHRoZQ0KPiBzeW5jaHJvbml6ZXIuDQo+ID4gWW91IGNhbiByZWZlciB0byB0aGUg
Zm9sbG93aW5nIEFwcGxpY2F0aW9uIFJlcG9ydCAoY2hhcHRlciA2KSBmb3IgDQo+ID4gbW9yZQ0K
PiA+IGluZm9ybWF0aW9uOiBodHRwOi8vd3d3LnRpLmNvbS9saXQvYW4vc3dhYTE2Ni9zd2FhMTY2
LnBkZg0KPiANCj4gU28gaXQgYnJlYWtzIHJlZ3VsYXIgbWVzaCBzeW5jaHJvbml6YXRpb24/IFBv
d2VyIFNhdmUsIGFuZCBpbiB0aGUgDQo+IGZ1dHVyZSBwb3NzaWJseSBERlMgYW5kIE1DQ0EgcmVs
eSBvbiB0aGlzIHRvIHdvcmsuDQo+IA0KPiBJdCBzb3VuZHMgbGlrZSB5b3Ugd2FudCBzb21lIGFi
c29sdXRlIFRTRiB0aW1lIHN5bmNocm9uaXplZCB3aXRoaW4gMjB1cy4NCj4gTWVzaCBUU0Ygc3lu
Y2hyb25pemF0aW9uIGNhbiBwcm92aWRlIHRoaXMsIGl0J3MganVzdCBhIGxpdHRsZSBtb3JlIA0K
PiBjb21wbGljYXRlZCBhcyB5b3UnbGwgbmVlZCB0byBjb21wdXRlIGF0IHRoZSBhcHBsaWNhdGlv
biBsZXZlbCB1c2luZyB0aGUgcGVyLXN0YSBvZmZzZXRzLg0KPiBNZXNoIHN5bmNocm9uaXphdGlv
biBhY2N1cmFjeSB3YXMgcHJldHR5IGdvb2QgSUlSQywgdW5mb3J0dW5hdGVseSBJIA0KPiBkb24n
dCBoYXZlIGFueSB0ZXN0IGRhdGEgdG8gc2hhcmUgOigNCg0KWWVzLiBUaGlzIGZlYXR1cmUgY2Fu
IGhpdCB0aW1lIGFjY3VyYWN5IG9mIGxlc3MgdGhhbiAyMHVzIHdoZW4gdXNpbmcgd2wxOHh4IGRl
dmljZXMuDQpJdCBpcyBub3QgYnVpbHQgZm9yIG1lc2ggcm9sZSBzcGVjaWZpY2FsbHkgYnV0IGZv
ciBTVEEgYW5kIEFQIGFzIHdlbGwuDQpVbmxpa2UgU1RBIGFuZCBBUCBtb2RlcyB3aGVyZSB0aGUg
dG9wb2xvZ3kgaXMgd2VsbCBkZWZpbmVkIHdoZW4gcnVubmluZyBtZXNoDQp5b3UgaGF2ZSBzZXZl
cmFsIHpvbmVzIHRoYXQgY2FuIGJlIHRyZWF0ZWQgYXMgU1RBLUFQIHRvcG9sb2d5Lg0KVGhpcyBw
YXRjaCBqdXN0IGFkZCB0aGUgQVBJIGZvciBjb25maWd1cmluZyB3aGljaCBtZXNoIHBlZXIgd2ls
bCBhY3QgYXMgQVAgb24gaXRzIHpvbmUNCkZvciB0aGUgdGltZSBzeW5jaHJvbml6YXRpb24gbWF0
dGVyLg0KSXQgaGFzIG5vdGhpbmcgdG8gZG8gd2l0aCBNZXNoIFRTRiBzeW5jaHJvbml6YXRpb24g
c3BlY2lmaWNhbGx5IHNvIGl0IHNob3VsZG7igJl0IGJyZWFrIGFueXRoaW5nLg0KDQpHdXkNCg==

^ permalink raw reply

* Re: [PATCH] rtlwifi/rtl8192de: Fix print format string
From: Larry Finger @ 2016-08-28  2:30 UTC (permalink / raw)
  To: Oleg Drokin, Chaoming Li, Kalle Valo; +Cc: linux-wireless, linux-kernel
In-Reply-To: <1472267543-810602-1-git-send-email-green@linuxhacker.ru>

On 08/26/2016 10:12 PM, Oleg Drokin wrote:
> %ul was likely meant as %lu to print an unsigned long,
> not an unsigned with a letter l at the end.
> But in fact the value printed is u32 anyway, so just drop
> the l completely.

This patch looks OK, thus

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks,

Larry

>
> Signed-off-by: Oleg Drokin <green@linuxhacker.ru>
> ---
>  drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> index d334d2a..2a4810d 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> @@ -588,7 +588,7 @@ static bool _rtl92d_phy_config_bb_with_headerfile(struct ieee80211_hw *hw,
>  				 * setting. */
>  				udelay(1);
>  				RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> -					 "The Rtl819XAGCTAB_Array_Table[0] is %ul Rtl819XPHY_REGArray[1] is %ul\n",
> +					 "The Rtl819XAGCTAB_Array_Table[0] is %u Rtl819XPHY_REGArray[1] is %u\n",
>  					 agctab_array_table[i],
>  					 agctab_array_table[i + 1]);
>  			}
> @@ -604,7 +604,7 @@ static bool _rtl92d_phy_config_bb_with_headerfile(struct ieee80211_hw *hw,
>  					 * setting. */
>  					udelay(1);
>  					RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> -						 "The Rtl819XAGCTAB_Array_Table[0] is %ul Rtl819XPHY_REGArray[1] is %ul\n",
> +						 "The Rtl819XAGCTAB_Array_Table[0] is %u Rtl819XPHY_REGArray[1] is %u\n",
>  						 agctab_array_table[i],
>  						 agctab_array_table[i + 1]);
>  				}
> @@ -620,7 +620,7 @@ static bool _rtl92d_phy_config_bb_with_headerfile(struct ieee80211_hw *hw,
>  					 * setting. */
>  					udelay(1);
>  					RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
> -						 "The Rtl819XAGCTAB_5GArray_Table[0] is %ul Rtl819XPHY_REGArray[1] is %ul\n",
> +						 "The Rtl819XAGCTAB_5GArray_Table[0] is %u Rtl819XPHY_REGArray[1] is %u\n",
>  						 agctab_5garray_table[i],
>  						 agctab_5garray_table[i + 1]);
>  				}
>

^ permalink raw reply

* [PATCH] fix:gpio: mark symbols static where possible
From: Baoyou Xie @ 2016-08-27 16:27 UTC (permalink / raw)
  To: ath9k-devel, kvalo
  Cc: linux-wireless, ath9k-devel, netdev, linux-kernel, arnd,
	baoyou.xie, xie.baoyou

We get 1 warning about global functions without a declaration
in the ath9k gpio driver when building with W=1:
drivers/net/wireless/ath/ath9k/gpio.c:25:6: warning: no previous prototype for 'ath_fill_led_pin' [-Wmissing-prototypes]

In fact, this function is only used in the file in which it is declared
and don't need a declaration, but can be made static.
so this patch marks it 'static'.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 drivers/net/wireless/ath/ath9k/gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/gpio.c b/drivers/net/wireless/ath/ath9k/gpio.c
index 490f74d..ddb2886 100644
--- a/drivers/net/wireless/ath/ath9k/gpio.c
+++ b/drivers/net/wireless/ath/ath9k/gpio.c
@@ -22,7 +22,7 @@
 
 #ifdef CONFIG_MAC80211_LEDS
 
-void ath_fill_led_pin(struct ath_softc *sc)
+static void ath_fill_led_pin(struct ath_softc *sc)
 {
 	struct ath_hw *ah = sc->sc_ah;
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH] fix:rtl8xxxu_core: mark symbols static where possible
From: Baoyou Xie @ 2016-08-27 16:14 UTC (permalink / raw)
  To: Jes.Sorensen, kvalo
  Cc: linux-wireless, netdev, linux-kernel, arnd, baoyou.xie,
	xie.baoyou

We get 1 warning about global functions without a declaration
in the rtl8xxxu rtl8xxxu_core.c when building with W=1:
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c:898:1: warning: no previous prototype for 'rtl8xxxu_gen1_h2c_cmd' [-Wmissing-prototypes]

In fact, this function is only used in the file in which it is declared
and don't need a declaration, but can be made static.
so this patch marks it 'static'.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 77048db..95b54b8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -894,7 +894,7 @@ int rtl8xxxu_write_rfreg(struct rtl8xxxu_priv *priv,
 	return retval;
 }
 
-int
+static int
 rtl8xxxu_gen1_h2c_cmd(struct rtl8xxxu_priv *priv, struct h2c_cmd *h2c, int len)
 {
 	struct device *dev = &priv->udev->dev;
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] ath10k: fix spelling mistake "montior" -> "monitor"
From: Julian Calaby @ 2016-08-27 13:16 UTC (permalink / raw)
  To: Colin King
  Cc: Kalle Valo, ath10k, linux-wireless, netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <20160826180852.32248-1-colin.king@canonical.com>

Hi All,

On Sat, Aug 27, 2016 at 4:08 AM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in ath10k_warn message.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

^ permalink raw reply

* [PATCH 1/6] rtl8723au: remove declaration of unimplemented functions
From: Luca Ceresoli @ 2016-08-27 12:40 UTC (permalink / raw)
  To: devel
  Cc: Luca Ceresoli, Larry Finger, Jes Sorensen, Greg Kroah-Hartman,
	linux-wireless, linux-kernel

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Jes Sorensen <Jes.Sorensen@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-wireless@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/staging/rtl8723au/include/recv_osdep.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/rtl8723au/include/recv_osdep.h b/drivers/staging/rtl8723au/include/recv_osdep.h
index c2d3f1bd5948..c159dbc5c752 100644
--- a/drivers/staging/rtl8723au/include/recv_osdep.h
+++ b/drivers/staging/rtl8723au/include/recv_osdep.h
@@ -26,9 +26,6 @@ int rtw_recv_indicatepkt23a(struct rtw_adapter *adapter, struct recv_frame *prec
 
 void rtw_handle_tkip_mic_err23a(struct rtw_adapter *padapter, u8 bgroup);
 
-int	rtw_init_recv_priv(struct recv_priv *precvpriv, struct rtw_adapter *padapter);
-void rtw_free_recv_priv (struct recv_priv *precvpriv);
-
 int rtw_os_recv_resource_init(struct recv_priv *precvpriv, struct rtw_adapter *padapter);
 
 void rtw_init_recv_timer23a(struct recv_reorder_ctrl *preorder_ctrl);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2] fix:brcmfmac: add missing header dependencies
From: Baoyou Xie @ 2016-08-27  6:08 UTC (permalink / raw)
  To: franky.lin, hante.meuleman, kvalo
  Cc: baoyou.xie, linux-wireless, brcm80211-dev-list.pdl, netdev,
	linux-kernel, arnd, xie.baoyou

We get 1 warning when biuld kernel with W=1:
drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c:23:6: warning: no previous prototype for '__brcmf_err' [-Wmissing-
prototypes]

In fact, this function is declared in brcmfmac/debug.h, so this patch
add missing header dependencies

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
index a10f35c..fe67559 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
@@ -19,6 +19,7 @@
 #ifndef __CHECKER__
 #define CREATE_TRACE_POINTS
 #include "tracepoint.h"
+#include "debug.h"
 
 void __brcmf_err(const char *func, const char *fmt, ...)
 {
-- 
2.7.4

^ permalink raw reply related

* [PATCH] fix:brcmfmac: add missing header dependencies
From: Baoyou Xie @ 2016-08-27  5:54 UTC (permalink / raw)
  To: franky.lin, hante.meuleman, kvalo
  Cc: baoyou.xie, linux-wireless, brcm80211-dev-list.pdl, netdev,
	linux-kernel, arnd, xie.baoyou

We get 1 warning when biuld kernel with W=1:
drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c:23:6: warning: no previous prototype for '__brcmf_err' [-Wmissing-
prototypes]

In fact, this function is declared in brcmfmac/debuge.h, so this patch
add missing header dependencies

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
index a10f35c..fe67559 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
@@ -19,6 +19,7 @@
 #ifndef __CHECKER__
 #define CREATE_TRACE_POINTS
 #include "tracepoint.h"
+#include "debug.h"
 
 void __brcmf_err(const char *func, const char *fmt, ...)
 {
-- 
2.7.4

^ permalink raw reply related

* [PATCH] rtlwifi/rtl8192de: Fix print format string
From: Oleg Drokin @ 2016-08-27  3:12 UTC (permalink / raw)
  To: Larry Finger, Chaoming Li, Kalle Valo
  Cc: linux-wireless, linux-kernel, Oleg Drokin

%ul was likely meant as %lu to print an unsigned long,
not an unsigned with a letter l at the end.
But in fact the value printed is u32 anyway, so just drop
the l completely.

Signed-off-by: Oleg Drokin <green@linuxhacker.ru>
---
 drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
index d334d2a..2a4810d 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
@@ -588,7 +588,7 @@ static bool _rtl92d_phy_config_bb_with_headerfile(struct ieee80211_hw *hw,
 				 * setting. */
 				udelay(1);
 				RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
-					 "The Rtl819XAGCTAB_Array_Table[0] is %ul Rtl819XPHY_REGArray[1] is %ul\n",
+					 "The Rtl819XAGCTAB_Array_Table[0] is %u Rtl819XPHY_REGArray[1] is %u\n",
 					 agctab_array_table[i],
 					 agctab_array_table[i + 1]);
 			}
@@ -604,7 +604,7 @@ static bool _rtl92d_phy_config_bb_with_headerfile(struct ieee80211_hw *hw,
 					 * setting. */
 					udelay(1);
 					RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
-						 "The Rtl819XAGCTAB_Array_Table[0] is %ul Rtl819XPHY_REGArray[1] is %ul\n",
+						 "The Rtl819XAGCTAB_Array_Table[0] is %u Rtl819XPHY_REGArray[1] is %u\n",
 						 agctab_array_table[i],
 						 agctab_array_table[i + 1]);
 				}
@@ -620,7 +620,7 @@ static bool _rtl92d_phy_config_bb_with_headerfile(struct ieee80211_hw *hw,
 					 * setting. */
 					udelay(1);
 					RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE,
-						 "The Rtl819XAGCTAB_5GArray_Table[0] is %ul Rtl819XPHY_REGArray[1] is %ul\n",
+						 "The Rtl819XAGCTAB_5GArray_Table[0] is %u Rtl819XPHY_REGArray[1] is %u\n",
 						 agctab_5garray_table[i],
 						 agctab_5garray_table[i + 1]);
 				}
-- 
2.7.4

^ permalink raw reply related

* mac80211_hwsim: handling RSSI
From: Ramon Fontes @ 2016-08-26 21:53 UTC (permalink / raw)
  To: linux-wireless

Hi,

I am developing a wireless network emulator
(https://github.com/intrig-unicamp/mininet-wifi) and I was wondering
what is the best way to change the RSSI information provided by
mac80211_hwsim. I am able to calculate the RSSI with my emulator, but
the RSSI is not updated by mac80211_hwsim, of course. Do I have to
make changes in nl80211?

Thank you in advance,
Ramon Fontes

^ permalink raw reply

* [PATCH] ath10k: fix spelling mistake "montior" -> "monitor"
From: Colin King @ 2016-08-26 18:08 UTC (permalink / raw)
  To: Kalle Valo, ath10k, linux-wireless, netdev; +Cc: linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Trivial fix to spelling mistake in ath10k_warn message.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 0bbd0a0..841f288 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -5186,7 +5186,7 @@ static void ath10k_configure_filter(struct ieee80211_hw *hw,
 
 	ret = ath10k_monitor_recalc(ar);
 	if (ret)
-		ath10k_warn(ar, "failed to recalc montior: %d\n", ret);
+		ath10k_warn(ar, "failed to recalc monitor: %d\n", ret);
 
 	mutex_unlock(&ar->conf_mutex);
 }
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH v3] ath10k: implement NAPI support
From: Rajkumar Manoharan @ 2016-08-26 16:07 UTC (permalink / raw)
  To: Dave Taht; +Cc: Johannes Berg, Rajkumar Manoharan, ath10k, linux-wireless
In-Reply-To: <CAA93jw5mEMbUVs-i-Bpjkv6tX-umsO=9ZKPEnFcEj=22yjWX1g@mail.gmail.com>

On 2016-08-26 17:19, Dave Taht wrote:
> On Fri, Aug 26, 2016 at 4:12 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> On Fri, 2016-08-26 at 03:48 -0700, Dave Taht wrote:
>>> I'm always rather big on people testing latency under load, and napi
>>> tends to add some.
>> 
>> That's a completely useless comment.
>> 
>> Obviously, everybody uses NAPI; it's necessary for system load and 
>> thus
>> performance, and lets drivers take advantage of TCP merging to reduce
>> ACKs, which is tremendously helpful (over wifi in particular.)
>> 
>> Please stop making such drive-by comments that focus only on the 
>> single
>> thing you find important above all; not all people can care only about
>> that single thing, and unconstructively reiterating it over and over
>> doesn't help.
> 
> Well, I apologize for being testy. It is  I spent a lot of time
> testing michal's patchset for the ath10k back in may, and I *will* go
> and retest ath10k, when these patches land. My principal concern with
> using napi is at lower rates than the maxes typically reported in a
> patchset.
> 

You are always welcome to validate this change and share your feedback.

>  But it would be nice if people always did test for latency under load
> when making improvements, before getting to me, and despite having
> helped make a very comprehensive test suite available (flent) that
> tests all sorts of things for wifi, getting people to actually use it
> to see real problems, (in addition to latency under load!) while their
> fingers are still hot in the codebase, and track/plot their results,
> remains an ongoing issue across the entire industry.
> 
> http://blog.cerowrt.org/post/fq_codel_on_ath10k/
> 
As you know, NAPI is designed to improve performance of high speed n/w 
devices. From LWN: "NAPI is a proven 
(www.cyberus.ca/~hadi/usenix-paper.tgz) technique to improve network 
performance on Linux." Even most of Gig-speed network drivers were 
already migrated to NAPI. Tasklets are heavy CPU consumers and it will 
impact performance of other low priority tasks. The article[1] explains 
the problems of tasklet.

 From my observations, average CPU usage got reduced by 10% under heavy 
data traffic against same peak throughput. I validated this change on 
both IPQ4019 platform (Quad-Core ARM Cortex A7 processor) and AP135 
platform (uni-core MIPS 720 MHz processor). I did not observe any 
regression.

> There are many other problems in wifi, of course, that could use
> engineering mental internalization, like airtime fairness, and the
> mis-behavior of the hardware queues,
> 
> http://blog.cerowrt.org/post/cs5_lockout/
> 
> wifi channel scans
> 
> http://blog.cerowrt.org/post/disabling_channel_scans/
> 
> and so on.
> 
> I have a ton more datasets and blog entries left to write up from the
> ath9k work thus far which point to some other issues (minstrel,
> aggregation, retries)
> 
your data are really impressive. Once again, feel free to validate this 
change and share your inputs.

[1] http://lwn.net/Articles/239633/

-Rajkumar

^ permalink raw reply

* Re: [PATCH v3] ath10k: implement NAPI support
From: Dave Taht @ 2016-08-26 11:49 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Rajkumar Manoharan, ath10k, linux-wireless, Rajkumar Manoharan
In-Reply-To: <1472209920.390.34.camel@sipsolutions.net>

On Fri, Aug 26, 2016 at 4:12 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Fri, 2016-08-26 at 03:48 -0700, Dave Taht wrote:
>> I'm always rather big on people testing latency under load, and napi
>> tends to add some.
>
> That's a completely useless comment.
>
> Obviously, everybody uses NAPI; it's necessary for system load and thus
> performance, and lets drivers take advantage of TCP merging to reduce
> ACKs, which is tremendously helpful (over wifi in particular.)
>
> Please stop making such drive-by comments that focus only on the single
> thing you find important above all; not all people can care only about
> that single thing, and unconstructively reiterating it over and over
> doesn't help.

Well, I apologize for being testy. It is  I spent a lot of time
testing michal's patchset for the ath10k back in may, and I *will* go
and retest ath10k, when these patches land. My principal concern with
using napi is at lower rates than the maxes typically reported in a
patchset.

 But it would be nice if people always did test for latency under load
when making improvements, before getting to me, and despite having
helped make a very comprehensive test suite available (flent) that
tests all sorts of things for wifi, getting people to actually use it
to see real problems, (in addition to latency under load!) while their
fingers are still hot in the codebase, and track/plot their results,
remains an ongoing issue across the entire industry.

http://blog.cerowrt.org/post/fq_codel_on_ath10k/

There are many other problems in wifi, of course, that could use
engineering mental internalization, like airtime fairness, and the
mis-behavior of the hardware queues,

http://blog.cerowrt.org/post/cs5_lockout/

wifi channel scans

http://blog.cerowrt.org/post/disabling_channel_scans/

and so on.

I have a ton more datasets and blog entries left to write up from the
ath9k work thus far which point to some other issues (minstrel,
aggregation, retries)

> Thanks,
> johannes



--=20
Dave T=C3=A4ht
Let's go make home routers and wifi faster! With better software!
http://blog.cerowrt.org

^ permalink raw reply

* Re: [PATCH v3] ath10k: implement NAPI support
From: Johannes Berg @ 2016-08-26 11:12 UTC (permalink / raw)
  To: Dave Taht, Rajkumar Manoharan; +Cc: ath10k, linux-wireless, Rajkumar Manoharan
In-Reply-To: <CAA93jw7vHWROH9Ymq1UoOKA5kw+fDbCM=vJehs8J=QP2PVs_RA@mail.gmail.com>

On Fri, 2016-08-26 at 03:48 -0700, Dave Taht wrote:
> I'm always rather big on people testing latency under load, and napi
> tends to add some.

That's a completely useless comment.

Obviously, everybody uses NAPI; it's necessary for system load and thus
performance, and lets drivers take advantage of TCP merging to reduce
ACKs, which is tremendously helpful (over wifi in particular.)

Please stop making such drive-by comments that focus only on the single
thing you find important above all; not all people can care only about
that single thing, and unconstructively reiterating it over and over
doesn't help.

Thanks,
johannes

^ permalink raw reply

* Re: [PATCH v3] ath10k: implement NAPI support
From: Dave Taht @ 2016-08-26 10:48 UTC (permalink / raw)
  To: Rajkumar Manoharan; +Cc: ath10k, linux-wireless, Rajkumar Manoharan
In-Reply-To: <20160826102250.24174-1-rmanohar@qti.qualcomm.com>

I'm always rather big on people testing latency under load, and napi
tends to add some.

^ permalink raw reply

* [PATCH v3] ath10k: implement NAPI support
From: Rajkumar Manoharan @ 2016-08-26 10:22 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, rmanohar, Rajkumar Manoharan

Add NAPI support for rx and tx completion. NAPI poll is scheduled
from interrupt handler. The design is as below

 - on interrupt
     - schedule napi and mask interrupts
 - on poll
   - process all pipes (no actual Tx/Rx)
   - process Rx within budget
   - if quota exceeds budget reschedule napi poll by returning budget
   - process Tx completions and update budget if necessary
   - process Tx fetch indications (pull-push)
   - push any other pending Tx (if possible)
   - before resched or napi completion replenish htt rx ring buffer
   - if work done < budget, complete napi poll and unmask interrupts

This change also get rid of two tasklets (intr_tq and txrx_compl_task).

Measured peak throughput with NAPI on IPQ4019 platform in controlled
environment. No noticeable reduction in throughput is seen and also
observed improvements in CPU usage. Approx. 15% CPU usage got reduced
in UDP uplink case.

DL: AP DUT Tx
UL: AP DUT Rx

IPQ4019 (avg. cpu usage %)
========
                TOT              +NAPI
              ===========      =============
TCP DL       644 Mbps (42%)    645 Mbps (36%)
TCP UL       673 Mbps (30%)    675 Mbps (26%)
UDP DL       682 Mbps (49%)    680 Mbps (49%)
UDP UL       720 Mbps (28%)    717 Mbps (11%)

Signed-off-by: Rajkumar Manoharan <rmanohar@qti.qualcomm.com>
---
v2: rebased change
v3: napi_synchronize and napi_disable should be called on deinit. Otherwise
    it is causing invalid memory access. Thanks to Kalle for reporting this issue.

 drivers/net/wireless/ath/ath10k/ahb.c    |  10 +-
 drivers/net/wireless/ath/ath10k/core.c   |   2 +
 drivers/net/wireless/ath/ath10k/core.h   |   8 ++
 drivers/net/wireless/ath/ath10k/htt.h    |   2 +-
 drivers/net/wireless/ath/ath10k/htt_rx.c | 154 +++++++++++++++++++------------
 drivers/net/wireless/ath/ath10k/htt_tx.c |   2 -
 drivers/net/wireless/ath/ath10k/pci.c    |  71 ++++++++------
 drivers/net/wireless/ath/ath10k/pci.h    |   6 +-
 8 files changed, 159 insertions(+), 96 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ahb.c b/drivers/net/wireless/ath/ath10k/ahb.c
index acec16b9cf49..83ecfb6d1a36 100644
--- a/drivers/net/wireless/ath/ath10k/ahb.c
+++ b/drivers/net/wireless/ath/ath10k/ahb.c
@@ -462,13 +462,13 @@ static void ath10k_ahb_halt_chip(struct ath10k *ar)
 static irqreturn_t ath10k_ahb_interrupt_handler(int irq, void *arg)
 {
 	struct ath10k *ar = arg;
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
 	if (!ath10k_pci_irq_pending(ar))
 		return IRQ_NONE;
 
 	ath10k_pci_disable_and_clear_legacy_irq(ar);
-	tasklet_schedule(&ar_pci->intr_tq);
+	ath10k_pci_irq_msi_fw_mask(ar);
+	napi_schedule(&ar->napi);
 
 	return IRQ_HANDLED;
 }
@@ -717,6 +717,9 @@ static void ath10k_ahb_hif_stop(struct ath10k *ar)
 	synchronize_irq(ar_ahb->irq);
 
 	ath10k_pci_flush(ar);
+
+	napi_synchronize(&ar->napi);
+	napi_disable(&ar->napi);
 }
 
 static int ath10k_ahb_hif_power_up(struct ath10k *ar)
@@ -748,6 +751,7 @@ static int ath10k_ahb_hif_power_up(struct ath10k *ar)
 		ath10k_err(ar, "could not wake up target CPU: %d\n", ret);
 		goto err_ce_deinit;
 	}
+	napi_enable(&ar->napi);
 
 	return 0;
 
@@ -831,7 +835,7 @@ static int ath10k_ahb_probe(struct platform_device *pdev)
 		goto err_resource_deinit;
 	}
 
-	ath10k_pci_init_irq_tasklets(ar);
+	ath10k_pci_init_napi(ar);
 
 	ret = ath10k_ahb_request_irq_legacy(ar);
 	if (ret)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index e88982921aa3..0ca58cf0ffea 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2249,6 +2249,8 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
 	INIT_WORK(&ar->register_work, ath10k_core_register_work);
 	INIT_WORK(&ar->restart_work, ath10k_core_restart);
 
+	init_dummy_netdev(&ar->napi_dev);
+
 	ret = ath10k_debug_create(ar);
 	if (ret)
 		goto err_free_aux_wq;
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 30ae5bf81611..f36c2b274ee5 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -65,6 +65,10 @@
 #define ATH10K_KEEPALIVE_MAX_IDLE 3895
 #define ATH10K_KEEPALIVE_MAX_UNRESPONSIVE 3900
 
+/* NAPI poll budget */
+#define ATH10K_NAPI_BUDGET      64
+#define ATH10K_NAPI_QUOTA_LIMIT 60
+
 struct ath10k;
 
 enum ath10k_bus {
@@ -936,6 +940,10 @@ struct ath10k {
 	struct ath10k_thermal thermal;
 	struct ath10k_wow wow;
 
+	/* NAPI */
+	struct net_device napi_dev;
+	struct napi_struct napi;
+
 	/* must be last */
 	u8 drv_priv[0] __aligned(sizeof(void *));
 };
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 430a83e142aa..98c14247021b 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1665,7 +1665,6 @@ struct ath10k_htt {
 
 	/* This is used to group tx/rx completions separately and process them
 	 * in batches to reduce cache stalls */
-	struct tasklet_struct txrx_compl_task;
 	struct sk_buff_head rx_compl_q;
 	struct sk_buff_head rx_in_ord_compl_q;
 	struct sk_buff_head tx_fetch_ind_q;
@@ -1798,5 +1797,6 @@ int ath10k_htt_tx(struct ath10k_htt *htt,
 		  struct sk_buff *msdu);
 void ath10k_htt_rx_pktlog_completion_handler(struct ath10k *ar,
 					     struct sk_buff *skb);
+int ath10k_htt_txrx_compl_task(struct ath10k *ar, int budget);
 
 #endif
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 78db5d679f19..d3f8baf532d4 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -34,7 +34,6 @@
 #define HTT_RX_RING_REFILL_RESCHED_MS 5
 
 static int ath10k_htt_rx_get_csum_state(struct sk_buff *skb);
-static void ath10k_htt_txrx_compl_task(unsigned long ptr);
 
 static struct sk_buff *
 ath10k_htt_rx_find_skb_paddr(struct ath10k *ar, u32 paddr)
@@ -226,7 +225,6 @@ int ath10k_htt_rx_ring_refill(struct ath10k *ar)
 void ath10k_htt_rx_free(struct ath10k_htt *htt)
 {
 	del_timer_sync(&htt->rx_ring.refill_retry_timer);
-	tasklet_kill(&htt->txrx_compl_task);
 
 	skb_queue_purge(&htt->rx_compl_q);
 	skb_queue_purge(&htt->rx_in_ord_compl_q);
@@ -520,9 +518,6 @@ int ath10k_htt_rx_alloc(struct ath10k_htt *htt)
 	skb_queue_head_init(&htt->tx_fetch_ind_q);
 	atomic_set(&htt->num_mpdus_ready, 0);
 
-	tasklet_init(&htt->txrx_compl_task, ath10k_htt_txrx_compl_task,
-		     (unsigned long)htt);
-
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "htt rx ring size %d fill_level %d\n",
 		   htt->rx_ring.size, htt->rx_ring.fill_level);
 	return 0;
@@ -958,7 +953,7 @@ static void ath10k_process_rx(struct ath10k *ar,
 	trace_ath10k_rx_hdr(ar, skb->data, skb->len);
 	trace_ath10k_rx_payload(ar, skb->data, skb->len);
 
-	ieee80211_rx(ar->hw, skb);
+	ieee80211_rx_napi(ar->hw, NULL, skb, &ar->napi);
 }
 
 static int ath10k_htt_rx_nwifi_hdrlen(struct ath10k *ar,
@@ -1527,7 +1522,7 @@ static int ath10k_htt_rx_handle_amsdu(struct ath10k_htt *htt)
 	struct ath10k *ar = htt->ar;
 	static struct ieee80211_rx_status rx_status;
 	struct sk_buff_head amsdu;
-	int ret;
+	int ret, num_msdus;
 
 	__skb_queue_head_init(&amsdu);
 
@@ -1549,13 +1544,14 @@ static int ath10k_htt_rx_handle_amsdu(struct ath10k_htt *htt)
 		return ret;
 	}
 
+	num_msdus = skb_queue_len(&amsdu);
 	ath10k_htt_rx_h_ppdu(ar, &amsdu, &rx_status, 0xffff);
 	ath10k_htt_rx_h_unchain(ar, &amsdu, ret > 0);
 	ath10k_htt_rx_h_filter(ar, &amsdu, &rx_status);
 	ath10k_htt_rx_h_mpdu(ar, &amsdu, &rx_status);
 	ath10k_htt_rx_h_deliver(ar, &amsdu, &rx_status);
 
-	return 0;
+	return num_msdus;
 }
 
 static void ath10k_htt_rx_proc_rx_ind(struct ath10k_htt *htt,
@@ -1579,15 +1575,6 @@ static void ath10k_htt_rx_proc_rx_ind(struct ath10k_htt *htt,
 		mpdu_count += mpdu_ranges[i].mpdu_count;
 
 	atomic_add(mpdu_count, &htt->num_mpdus_ready);
-
-	tasklet_schedule(&htt->txrx_compl_task);
-}
-
-static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt)
-{
-	atomic_inc(&htt->num_mpdus_ready);
-
-	tasklet_schedule(&htt->txrx_compl_task);
 }
 
 static void ath10k_htt_rx_tx_compl_ind(struct ath10k *ar,
@@ -1772,14 +1759,15 @@ static void ath10k_htt_rx_h_rx_offload_prot(struct ieee80211_rx_status *status,
 			RX_FLAG_MMIC_STRIPPED;
 }
 
-static void ath10k_htt_rx_h_rx_offload(struct ath10k *ar,
-				       struct sk_buff_head *list)
+static int ath10k_htt_rx_h_rx_offload(struct ath10k *ar,
+				      struct sk_buff_head *list)
 {
 	struct ath10k_htt *htt = &ar->htt;
 	struct ieee80211_rx_status *status = &htt->rx_status;
 	struct htt_rx_offload_msdu *rx;
 	struct sk_buff *msdu;
 	size_t offset;
+	int num_msdu = 0;
 
 	while ((msdu = __skb_dequeue(list))) {
 		/* Offloaded frames don't have Rx descriptor. Instead they have
@@ -1819,10 +1807,12 @@ static void ath10k_htt_rx_h_rx_offload(struct ath10k *ar,
 		ath10k_htt_rx_h_rx_offload_prot(status, msdu);
 		ath10k_htt_rx_h_channel(ar, status, NULL, rx->vdev_id);
 		ath10k_process_rx(ar, status, msdu);
+		num_msdu++;
 	}
+	return num_msdu;
 }
 
-static void ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
+static int ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
 {
 	struct ath10k_htt *htt = &ar->htt;
 	struct htt_resp *resp = (void *)skb->data;
@@ -1835,12 +1825,12 @@ static void ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
 	u8 tid;
 	bool offload;
 	bool frag;
-	int ret;
+	int ret, num_msdus = 0;
 
 	lockdep_assert_held(&htt->rx_ring.lock);
 
 	if (htt->rx_confused)
-		return;
+		return -EIO;
 
 	skb_pull(skb, sizeof(resp->hdr));
 	skb_pull(skb, sizeof(resp->rx_in_ord_ind));
@@ -1859,7 +1849,7 @@ static void ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
 
 	if (skb->len < msdu_count * sizeof(*resp->rx_in_ord_ind.msdu_descs)) {
 		ath10k_warn(ar, "dropping invalid in order rx indication\n");
-		return;
+		return -EINVAL;
 	}
 
 	/* The event can deliver more than 1 A-MSDU. Each A-MSDU is later
@@ -1870,14 +1860,14 @@ static void ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
 	if (ret < 0) {
 		ath10k_warn(ar, "failed to pop paddr list: %d\n", ret);
 		htt->rx_confused = true;
-		return;
+		return -EIO;
 	}
 
 	/* Offloaded frames are very different and need to be handled
 	 * separately.
 	 */
 	if (offload)
-		ath10k_htt_rx_h_rx_offload(ar, &list);
+		num_msdus = ath10k_htt_rx_h_rx_offload(ar, &list);
 
 	while (!skb_queue_empty(&list)) {
 		__skb_queue_head_init(&amsdu);
@@ -1890,6 +1880,7 @@ static void ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
 			 * better to report something than nothing though. This
 			 * should still give an idea about rx rate to the user.
 			 */
+			num_msdus += skb_queue_len(&amsdu);
 			ath10k_htt_rx_h_ppdu(ar, &amsdu, status, vdev_id);
 			ath10k_htt_rx_h_filter(ar, &amsdu, status);
 			ath10k_htt_rx_h_mpdu(ar, &amsdu, status);
@@ -1902,9 +1893,10 @@ static void ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
 			ath10k_warn(ar, "failed to extract amsdu: %d\n", ret);
 			htt->rx_confused = true;
 			__skb_queue_purge(&list);
-			return;
+			return -EIO;
 		}
 	}
+	return num_msdus;
 }
 
 static void ath10k_htt_rx_tx_fetch_resp_id_confirm(struct ath10k *ar,
@@ -2267,7 +2259,6 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 	}
 	case HTT_T2H_MSG_TYPE_TX_COMPL_IND:
 		ath10k_htt_rx_tx_compl_ind(htt->ar, skb);
-		tasklet_schedule(&htt->txrx_compl_task);
 		break;
 	case HTT_T2H_MSG_TYPE_SEC_IND: {
 		struct ath10k *ar = htt->ar;
@@ -2284,7 +2275,7 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 	case HTT_T2H_MSG_TYPE_RX_FRAG_IND: {
 		ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt event: ",
 				skb->data, skb->len);
-		ath10k_htt_rx_frag_handler(htt);
+		atomic_inc(&htt->num_mpdus_ready);
 		break;
 	}
 	case HTT_T2H_MSG_TYPE_TEST:
@@ -2320,8 +2311,7 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 		break;
 	}
 	case HTT_T2H_MSG_TYPE_RX_IN_ORD_PADDR_IND: {
-		skb_queue_tail(&htt->rx_in_ord_compl_q, skb);
-		tasklet_schedule(&htt->txrx_compl_task);
+		__skb_queue_tail(&htt->rx_in_ord_compl_q, skb);
 		return false;
 	}
 	case HTT_T2H_MSG_TYPE_TX_CREDIT_UPDATE_IND:
@@ -2347,7 +2337,6 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
 			break;
 		}
 		skb_queue_tail(&htt->tx_fetch_ind_q, tx_fetch_ind);
-		tasklet_schedule(&htt->txrx_compl_task);
 		break;
 	}
 	case HTT_T2H_MSG_TYPE_TX_FETCH_CONFIRM:
@@ -2376,27 +2365,77 @@ void ath10k_htt_rx_pktlog_completion_handler(struct ath10k *ar,
 }
 EXPORT_SYMBOL(ath10k_htt_rx_pktlog_completion_handler);
 
-static void ath10k_htt_txrx_compl_task(unsigned long ptr)
+int ath10k_htt_txrx_compl_task(struct ath10k *ar, int budget)
 {
-	struct ath10k_htt *htt = (struct ath10k_htt *)ptr;
-	struct ath10k *ar = htt->ar;
+	struct ath10k_htt *htt = &ar->htt;
 	struct htt_tx_done tx_done = {};
-	struct sk_buff_head rx_ind_q;
 	struct sk_buff_head tx_ind_q;
 	struct sk_buff *skb;
 	unsigned long flags;
-	int num_mpdus;
+	int quota = 0, done, num_rx_msdus;
+	bool resched_napi = false;
 
-	__skb_queue_head_init(&rx_ind_q);
 	__skb_queue_head_init(&tx_ind_q);
 
-	spin_lock_irqsave(&htt->rx_in_ord_compl_q.lock, flags);
-	skb_queue_splice_init(&htt->rx_in_ord_compl_q, &rx_ind_q);
-	spin_unlock_irqrestore(&htt->rx_in_ord_compl_q.lock, flags);
+	/* Since in-ord-ind can deliver more than 1 A-MSDU in single event,
+	 * process it first to utilize full available quota.
+	 */
+	while (quota < budget) {
+		if (skb_queue_empty(&htt->rx_in_ord_compl_q))
+			break;
 
-	spin_lock_irqsave(&htt->tx_fetch_ind_q.lock, flags);
-	skb_queue_splice_init(&htt->tx_fetch_ind_q, &tx_ind_q);
-	spin_unlock_irqrestore(&htt->tx_fetch_ind_q.lock, flags);
+		skb = __skb_dequeue(&htt->rx_in_ord_compl_q);
+		if (!skb) {
+			resched_napi = true;
+			goto exit;
+		}
+
+		spin_lock_bh(&htt->rx_ring.lock);
+		num_rx_msdus = ath10k_htt_rx_in_ord_ind(ar, skb);
+		spin_unlock_bh(&htt->rx_ring.lock);
+		if (num_rx_msdus < 0) {
+			resched_napi = true;
+			goto exit;
+		}
+
+		dev_kfree_skb_any(skb);
+		if (num_rx_msdus > 0)
+			quota += num_rx_msdus;
+
+		if ((quota > ATH10K_NAPI_QUOTA_LIMIT) &&
+		    !skb_queue_empty(&htt->rx_in_ord_compl_q)) {
+			resched_napi = true;
+			goto exit;
+		}
+	}
+
+	while (quota < budget) {
+		/* no more data to receive */
+		if (!atomic_read(&htt->num_mpdus_ready))
+			break;
+
+		num_rx_msdus = ath10k_htt_rx_handle_amsdu(htt);
+		if (num_rx_msdus < 0) {
+			resched_napi = true;
+			goto exit;
+		}
+
+		quota += num_rx_msdus;
+		atomic_dec(&htt->num_mpdus_ready);
+		if ((quota > ATH10K_NAPI_QUOTA_LIMIT) &&
+		    atomic_read(&htt->num_mpdus_ready)) {
+			resched_napi = true;
+			goto exit;
+		}
+	}
+
+	/* From NAPI documentation:
+	 *  The napi poll() function may also process TX completions, in which
+	 *  case if it processes the entire TX ring then it should count that
+	 *  work as the rest of the budget.
+	 */
+	if ((quota < budget) && !kfifo_is_empty(&htt->txdone_fifo))
+		quota = budget;
 
 	/* kfifo_get: called only within txrx_tasklet so it's neatly serialized.
 	 * From kfifo_get() documentation:
@@ -2406,27 +2445,22 @@ static void ath10k_htt_txrx_compl_task(unsigned long ptr)
 	while (kfifo_get(&htt->txdone_fifo, &tx_done))
 		ath10k_txrx_tx_unref(htt, &tx_done);
 
+	spin_lock_irqsave(&htt->tx_fetch_ind_q.lock, flags);
+	skb_queue_splice_init(&htt->tx_fetch_ind_q, &tx_ind_q);
+	spin_unlock_irqrestore(&htt->tx_fetch_ind_q.lock, flags);
+
 	while ((skb = __skb_dequeue(&tx_ind_q))) {
 		ath10k_htt_rx_tx_fetch_ind(ar, skb);
 		dev_kfree_skb_any(skb);
 	}
 
-	num_mpdus = atomic_read(&htt->num_mpdus_ready);
-
-	while (num_mpdus) {
-		if (ath10k_htt_rx_handle_amsdu(htt))
-			break;
-
-		num_mpdus--;
-		atomic_dec(&htt->num_mpdus_ready);
-	}
-
-	while ((skb = __skb_dequeue(&rx_ind_q))) {
-		spin_lock_bh(&htt->rx_ring.lock);
-		ath10k_htt_rx_in_ord_ind(ar, skb);
-		spin_unlock_bh(&htt->rx_ring.lock);
-		dev_kfree_skb_any(skb);
-	}
-
+exit:
 	ath10k_htt_rx_msdu_buff_replenish(htt);
+	/* In case of rx failure or more data to read, report budget
+	 * to reschedule NAPI poll
+	 */
+	done = resched_napi ? budget : quota;
+
+	return done;
 }
+EXPORT_SYMBOL(ath10k_htt_txrx_compl_task);
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 7c072b605bc7..ae5b33fe5ba8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -390,8 +390,6 @@ void ath10k_htt_tx_free(struct ath10k_htt *htt)
 {
 	int size;
 
-	tasklet_kill(&htt->txrx_compl_task);
-
 	idr_for_each(&htt->pending_tx, ath10k_htt_tx_clean_up_pending, htt->ar);
 	idr_destroy(&htt->pending_tx);
 
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 9a22c478dd1b..4bc663979856 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1506,12 +1506,10 @@ void ath10k_pci_hif_send_complete_check(struct ath10k *ar, u8 pipe,
 	ath10k_ce_per_engine_service(ar, pipe);
 }
 
-void ath10k_pci_kill_tasklet(struct ath10k *ar)
+static void ath10k_pci_rx_retry_sync(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	tasklet_kill(&ar_pci->intr_tq);
-
 	del_timer_sync(&ar_pci->rx_post_retry);
 }
 
@@ -1570,7 +1568,7 @@ void ath10k_pci_hif_get_default_pipe(struct ath10k *ar,
 						 ul_pipe, dl_pipe);
 }
 
-static void ath10k_pci_irq_msi_fw_mask(struct ath10k *ar)
+void ath10k_pci_irq_msi_fw_mask(struct ath10k *ar)
 {
 	u32 val;
 
@@ -1753,7 +1751,7 @@ void ath10k_pci_ce_deinit(struct ath10k *ar)
 
 void ath10k_pci_flush(struct ath10k *ar)
 {
-	ath10k_pci_kill_tasklet(ar);
+	ath10k_pci_rx_retry_sync(ar);
 	ath10k_pci_buffer_cleanup(ar);
 }
 
@@ -1780,6 +1778,8 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
 	ath10k_pci_irq_disable(ar);
 	ath10k_pci_irq_sync(ar);
 	ath10k_pci_flush(ar);
+	napi_synchronize(&ar->napi);
+	napi_disable(&ar->napi);
 
 	spin_lock_irqsave(&ar_pci->ps_lock, flags);
 	WARN_ON(ar_pci->ps_wake_refcount > 0);
@@ -2533,6 +2533,7 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
 		ath10k_err(ar, "could not wake up target CPU: %d\n", ret);
 		goto err_ce;
 	}
+	napi_enable(&ar->napi);
 
 	return 0;
 
@@ -2772,35 +2773,53 @@ static irqreturn_t ath10k_pci_interrupt_handler(int irq, void *arg)
 		return IRQ_NONE;
 	}
 
-	if (ar_pci->oper_irq_mode == ATH10K_PCI_IRQ_LEGACY) {
-		if (!ath10k_pci_irq_pending(ar))
-			return IRQ_NONE;
-
-		ath10k_pci_disable_and_clear_legacy_irq(ar);
-	}
+	if ((ar_pci->oper_irq_mode == ATH10K_PCI_IRQ_LEGACY) &&
+	    !ath10k_pci_irq_pending(ar))
+		return IRQ_NONE;
 
-	tasklet_schedule(&ar_pci->intr_tq);
+	ath10k_pci_disable_and_clear_legacy_irq(ar);
+	ath10k_pci_irq_msi_fw_mask(ar);
+	napi_schedule(&ar->napi);
 
 	return IRQ_HANDLED;
 }
 
-static void ath10k_pci_tasklet(unsigned long data)
+static int ath10k_pci_napi_poll(struct napi_struct *ctx, int budget)
 {
-	struct ath10k *ar = (struct ath10k *)data;
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	struct ath10k *ar = container_of(ctx, struct ath10k, napi);
+	int done = 0;
 
 	if (ath10k_pci_has_fw_crashed(ar)) {
-		ath10k_pci_irq_disable(ar);
 		ath10k_pci_fw_crashed_clear(ar);
 		ath10k_pci_fw_crashed_dump(ar);
-		return;
+		napi_complete(ctx);
+		return done;
 	}
 
 	ath10k_ce_per_engine_service_any(ar);
 
-	/* Re-enable legacy irq that was disabled in the irq handler */
-	if (ar_pci->oper_irq_mode == ATH10K_PCI_IRQ_LEGACY)
+	done = ath10k_htt_txrx_compl_task(ar, budget);
+
+	if (done < budget) {
+		napi_complete(ctx);
+		/* In case of MSI, it is possible that interrupts are received
+		 * while NAPI poll is inprogress. So pending interrupts that are
+		 * received after processing all copy engine pipes by NAPI poll
+		 * will not be handled again. This is causing failure to
+		 * complete boot sequence in x86 platform. So before enabling
+		 * interrupts safer to check for pending interrupts for
+		 * immediate servicing.
+		 */
+		if (CE_INTERRUPT_SUMMARY(ar)) {
+			napi_reschedule(ctx);
+			goto out;
+		}
 		ath10k_pci_enable_legacy_irq(ar);
+		ath10k_pci_irq_msi_fw_unmask(ar);
+	}
+
+out:
+	return done;
 }
 
 static int ath10k_pci_request_irq_msi(struct ath10k *ar)
@@ -2858,11 +2877,10 @@ static void ath10k_pci_free_irq(struct ath10k *ar)
 	free_irq(ar_pci->pdev->irq, ar);
 }
 
-void ath10k_pci_init_irq_tasklets(struct ath10k *ar)
+void ath10k_pci_init_napi(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	tasklet_init(&ar_pci->intr_tq, ath10k_pci_tasklet, (unsigned long)ar);
+	netif_napi_add(&ar->napi_dev, &ar->napi, ath10k_pci_napi_poll,
+		       ATH10K_NAPI_BUDGET);
 }
 
 static int ath10k_pci_init_irq(struct ath10k *ar)
@@ -2870,7 +2888,7 @@ static int ath10k_pci_init_irq(struct ath10k *ar)
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	int ret;
 
-	ath10k_pci_init_irq_tasklets(ar);
+	ath10k_pci_init_napi(ar);
 
 	if (ath10k_pci_irq_mode != ATH10K_PCI_IRQ_AUTO)
 		ath10k_info(ar, "limiting irq mode to: %d\n",
@@ -3131,7 +3149,8 @@ int ath10k_pci_setup_resource(struct ath10k *ar)
 
 void ath10k_pci_release_resource(struct ath10k *ar)
 {
-	ath10k_pci_kill_tasklet(ar);
+	ath10k_pci_rx_retry_sync(ar);
+	netif_napi_del(&ar->napi);
 	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_free_pipes(ar);
 }
@@ -3298,7 +3317,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 err_free_irq:
 	ath10k_pci_free_irq(ar);
-	ath10k_pci_kill_tasklet(ar);
+	ath10k_pci_rx_retry_sync(ar);
 
 err_deinit_irq:
 	ath10k_pci_deinit_irq(ar);
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 6eca1df2ce60..9854ad56b2de 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -177,8 +177,6 @@ struct ath10k_pci {
 	/* Operating interrupt mode */
 	enum ath10k_pci_irq_mode oper_irq_mode;
 
-	struct tasklet_struct intr_tq;
-
 	struct ath10k_pci_pipe pipe_info[CE_COUNT_MAX];
 
 	/* Copy Engine used for Diagnostic Accesses */
@@ -294,8 +292,7 @@ void ath10k_pci_free_pipes(struct ath10k *ar);
 void ath10k_pci_free_pipes(struct ath10k *ar);
 void ath10k_pci_rx_replenish_retry(unsigned long ptr);
 void ath10k_pci_ce_deinit(struct ath10k *ar);
-void ath10k_pci_init_irq_tasklets(struct ath10k *ar);
-void ath10k_pci_kill_tasklet(struct ath10k *ar);
+void ath10k_pci_init_napi(struct ath10k *ar);
 int ath10k_pci_init_pipes(struct ath10k *ar);
 int ath10k_pci_init_config(struct ath10k *ar);
 void ath10k_pci_rx_post(struct ath10k *ar);
@@ -303,6 +300,7 @@ void ath10k_pci_flush(struct ath10k *ar);
 void ath10k_pci_enable_legacy_irq(struct ath10k *ar);
 bool ath10k_pci_irq_pending(struct ath10k *ar);
 void ath10k_pci_disable_and_clear_legacy_irq(struct ath10k *ar);
+void ath10k_pci_irq_msi_fw_mask(struct ath10k *ar);
 int ath10k_pci_wait_for_target_init(struct ath10k *ar);
 int ath10k_pci_setup_resource(struct ath10k *ar);
 void ath10k_pci_release_resource(struct ath10k *ar);
-- 
2.9.2

^ permalink raw reply related

* Re: [Make-wifi-fast] [PATCH v2] mac80211: Move crypto IV generation to after TXQ dequeue.
From: Toke Høiland-Jørgensen @ 2016-08-26  8:54 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Dave Taht, make-wifi-fast, linux-wireless, Felix Fietkau
In-Reply-To: <1472200706.390.29.camel@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

> On Mon, 2016-08-22 at 16:47 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>>=C2=A0
>> I suppose that could be a way to do it (i.e. have
>> ieee80211_tx_dequeue call all the TX hooks etc), but am not sure
>> whether there would be problems doing all this work in the loop
>> that's building aggregates (which is what would happen for ath9k at
>> least).
>
> I don't know, but it seems that it's worth trying.
>
>> An alternative could be to split the process up in two: An "early"
>> and "late" stage, where the early stage does everything that is not
>> sensitive to reordering and the occasional drop, and the late stage
>> is everything that is. Then the queueing step could happen in-between=20
>> the two stages, and the non-queueing path could just call both stages
>> at once. In effect, this would just make the current work-arounds be
>> more explicit in the structure, rather than being marked as
>> exceptions.
>
> I'm not sure that works the way you think it does.
>
> What you did works for fast-xmit, but *only* because that doesn't do
> software crypto. If, for some reason, the TXQ stuff combines with
> software crypto, which doesn't seem impossible (ath9k even has a module
> parameter, iirc), then you have no way for this to work.

Yeah, I realised that when I started reviewing the slow path (sorry for
not realising that straight away). The v3 takes the "split handlers"
approach for this reason. That saved having to deal with fragmentation
on TXQ dequeue, and it means that some of the processing can be done
before queueing (such as GSO splitting; having packets be as small as
possible before applying FQ to them is a good thing if we want to
realise the full potential).

It seems there are still some bugs to work out with that patch, but I'd
be grateful if you could glance at it and comment on whether you think
this is a viable way forward (provided we can work out all the bugs, of
course).

>> > Now, it's unlikely to be that simple - fragmentation, for example,
>> > might mess this up.
>> >=20
>> > Overall though, I'm definitely wondering if it should be this way,
>> > since all the special cases just add complexity.
>>=20
>> I agree that the work-arounds are iffy, but I do also think it's
>> important to keep in mind that we are improving latency by orders of
>> magnitude here. A few special cases is worth it to achieve that, IMO.
>> And then iterating towards a design that don't need them, of course
>> :)
>
> I don't really agree, I'm not going to treat this unlike any other
> feature, which gets merged when it's ready for that.
>
> Right now, your code here obviously isn't, since it doesn't even
> address the cases that ath9k could run into, so either ath9k shouldn't
> use this mac80211 feature, or the mac80211 feature needs to be fixed
> before ath9k can use it.

Yeah, I agree now that I've looked at it some more :)

> I have no problems with documenting that the TXQ stuff can only be
> used with full hardware crypto, but then we should add some checks and
> warnings in mac80211 to ensure that, i.e. not allow software keys when
> TXQ stuff is used, nor allow keys with mac80211 PN assignment, etc.

I'd much rather fix things so it works in all cases. My patch to ath9k
to use this stuff completely removes the old TX path, and things like
the airtime fairness scheduler needs the intermediate queues to work.

-Toke

^ permalink raw reply

* Re: [Make-wifi-fast] [PATCH v2] mac80211: Move crypto IV generation to after TXQ dequeue.
From: Johannes Berg @ 2016-08-26  8:38 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Dave Taht, make-wifi-fast, linux-wireless, Felix Fietkau
In-Reply-To: <87oa4k508r.fsf@toke.dk>

On Mon, 2016-08-22 at 16:47 +0200, Toke Høiland-Jørgensen wrote:
> 
> I suppose that could be a way to do it (i.e. have
> ieee80211_tx_dequeue call all the TX hooks etc), but am not sure
> whether there would be problems doing all this work in the loop
> that's building aggregates (which is what would happen for ath9k at
> least).

I don't know, but it seems that it's worth trying.

> An alternative could be to split the process up in two: An "early"
> and "late" stage, where the early stage does everything that is not
> sensitive to reordering and the occasional drop, and the late stage
> is everything that is. Then the queueing step could happen in-between 
> the two stages, and the non-queueing path could just call both stages
> at once. In effect, this would just make the current work-arounds be
> more explicit in the structure, rather than being marked as
> exceptions.

I'm not sure that works the way you think it does.

What you did works for fast-xmit, but *only* because that doesn't do
software crypto. If, for some reason, the TXQ stuff combines with
software crypto, which doesn't seem impossible (ath9k even has a module
parameter, iirc), then you have no way for this to work.

> > Now, it's unlikely to be that simple - fragmentation, for example,
> > might mess this up.
> > 
> > Overall though, I'm definitely wondering if it should be this way,
> > since all the special cases just add complexity.
> 
> I agree that the work-arounds are iffy, but I do also think it's
> important to keep in mind that we are improving latency by orders of
> magnitude here. A few special cases is worth it to achieve that, IMO.
> And then iterating towards a design that don't need them, of course
> :)

I don't really agree, I'm not going to treat this unlike any other
feature, which gets merged when it's ready for that.

Right now, your code here obviously isn't, since it doesn't even
address the cases that ath9k could run into, so either ath9k shouldn't
use this mac80211 feature, or the mac80211 feature needs to be fixed
before ath9k can use it.

I have no problems with documenting that the TXQ stuff can only be used
with full hardware crypto, but then we should add some checks and
warnings in mac80211 to ensure that, i.e. not allow software keys when
TXQ stuff is used, nor allow keys with mac80211 PN assignment, etc.

Even QoS-seqno assignment will be broken btw, so you do need a bunch
more offloads to make this work.

johannes

^ permalink raw reply

* Re: [PATCH v2] rt2x00: add support for mac addr from device tree
From: Stanislaw Gruszka @ 2016-08-26  8:22 UTC (permalink / raw)
  To: Mathias Kresin; +Cc: linux-wireless
In-Reply-To: <1472195813-30471-1-git-send-email-dev@kresin.me>

On Fri, Aug 26, 2016 at 09:16:53AM +0200, Mathias Kresin wrote:
> 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 <dev@kresin.me>

Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>

^ permalink raw reply

* Re: [PATCH] cfg80211: Add support for user configurable beacon data rate
From: Johannes Berg @ 2016-08-26  8:14 UTC (permalink / raw)
  To: Undekari, Sunil Dutt, Kushwaha, Purushottam
  Cc: linux-wireless@vger.kernel.org, Malinen, Jouni,
	Kondabattini, Ganesh, Kalikot Veetil, Mahesh Kumar,
	Hullur Subramanyam, Amarnath, Kumar, Deepak (QCA)
In-Reply-To: <17fc8f16b6184c2ba09146faefd2bc8c@aphydexm01f.ap.qualcomm.com>

On Wed, 2016-08-24 at 12:30 +0000, Undekari, Sunil Dutt wrote:

> > It also doesn't check that you specified exactly one rate, but it's
> > not clear how else this would work?
> I can think of the following options here . 
> 1. Consider these rates as only the preference ( in order ) to the
> host driver , but the User space would have no way of knowing what
> the driver has selected.
> 2. Limit this beacon rate to only one . cfg80211 to verify that there
> is exactly one rate to be specified. 
> 
> Is option 2 a viable option ?
> 
Yeah, I think option 1 doesn't make much sense.

johannes

^ permalink raw reply

* Re: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps
From: Johannes Berg @ 2016-08-26  8:13 UTC (permalink / raw)
  To: Denis Kenzior, linux-wireless
In-Reply-To: <1472157884-3174-1-git-send-email-denkenz@gmail.com>

On Thu, 2016-08-25 at 15:44 -0500, Denis Kenzior wrote:
> dump_wiphy_parse only assigns filter_wiphy if one of the supported
> NL80211 attributes is present.  So for unfiltered dumps, filter_wiphy
> was always initialized to 0, and only interface 0 was dumped.
> 
> This was introduced in commit
> 2d75da13fbb957e955d212555b91101cef36f0ce.
> 
> Reported-by: Arend Van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
> 
Thanks guys, I've squashed this (and moved it into the initializer we
already had.)

johannes

^ 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