* [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock
From: Geoff Lansberry @ 2016-12-20 16:10 UTC (permalink / raw)
To: linux-wireless
Cc: lauro.venancio, aloisio.almeida, sameo, robh+dt, mark.rutland,
netdev, devicetree, linux-kernel, mgreer, justin, Geoff Lansberry
From: Geoff Lansberry <geoff@kuvee.com>
The TRF7970A has configuration options to support hardware designs
which use a 27.12MHz clock. This commit adds a device tree option
'clock-frequency' to support configuring the this chip for default
13.56MHz clock or the optional 27.12MHz clock.
---
.../devicetree/bindings/net/nfc/trf7970a.txt | 4 ++
drivers/nfc/trf7970a.c | 50 +++++++++++++++++-----
2 files changed, 43 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index 32b35a0..e262ac1 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -21,6 +21,8 @@ Optional SoC Specific Properties:
- t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
where an extra byte is returned by Read Multiple Block commands issued
to Type 5 tags.
+- clock-frequency: Set to specify that the input frequency to the trf7970a is 13560000Hz or 27120000Hz
+
Example (for ARM-based BeagleBone with TRF7970A on SPI1):
@@ -43,6 +45,8 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
irq-status-read-quirk;
en2-rf-quirk;
t5t-rmb-extra-byte-quirk;
+ vdd_io_1v8;
+ clock-frequency = <27120000>;
status = "okay";
};
};
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index 26c9dbb..4e051e9 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -124,6 +124,9 @@
NFC_PROTO_ISO15693_MASK | NFC_PROTO_NFC_DEP_MASK)
#define TRF7970A_AUTOSUSPEND_DELAY 30000 /* 30 seconds */
+#define TRF7970A_13MHZ_CLOCK_FREQUENCY 13560000
+#define TRF7970A_27MHZ_CLOCK_FREQUENCY 27120000
+
#define TRF7970A_RX_SKB_ALLOC_SIZE 256
@@ -1056,12 +1059,11 @@ static int trf7970a_init(struct trf7970a *trf)
trf->chip_status_ctrl &= ~TRF7970A_CHIP_STATUS_RF_ON;
- ret = trf7970a_write(trf, TRF7970A_MODULATOR_SYS_CLK_CTRL, 0);
+ ret = trf7970a_write(trf, TRF7970A_MODULATOR_SYS_CLK_CTRL,
+ trf->modulator_sys_clk_ctrl);
if (ret)
goto err_out;
- trf->modulator_sys_clk_ctrl = 0;
-
ret = trf7970a_write(trf, TRF7970A_ADJUTABLE_FIFO_IRQ_LEVELS,
TRF7970A_ADJUTABLE_FIFO_IRQ_LEVELS_WLH_96 |
TRF7970A_ADJUTABLE_FIFO_IRQ_LEVELS_WLL_32);
@@ -1181,27 +1183,37 @@ static int trf7970a_in_config_rf_tech(struct trf7970a *trf, int tech)
switch (tech) {
case NFC_DIGITAL_RF_TECH_106A:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_14443A_106;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_OOK;
trf->guard_time = TRF7970A_GUARD_TIME_NFCA;
break;
case NFC_DIGITAL_RF_TECH_106B:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_14443B_106;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
trf->guard_time = TRF7970A_GUARD_TIME_NFCB;
break;
case NFC_DIGITAL_RF_TECH_212F:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_FELICA_212;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
trf->guard_time = TRF7970A_GUARD_TIME_NFCF;
break;
case NFC_DIGITAL_RF_TECH_424F:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_FELICA_424;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
trf->guard_time = TRF7970A_GUARD_TIME_NFCF;
break;
case NFC_DIGITAL_RF_TECH_ISO15693:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_15693_SGL_1OF4_2648;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_OOK;
trf->guard_time = TRF7970A_GUARD_TIME_15693;
break;
default:
@@ -1571,17 +1583,23 @@ static int trf7970a_tg_config_rf_tech(struct trf7970a *trf, int tech)
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_NFC_NFC_CE_MODE |
TRF7970A_ISO_CTRL_NFC_CE |
TRF7970A_ISO_CTRL_NFC_CE_14443A;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_OOK;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_OOK;
break;
case NFC_DIGITAL_RF_TECH_212F:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_NFC_NFC_CE_MODE |
TRF7970A_ISO_CTRL_NFC_NFCF_212;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
break;
case NFC_DIGITAL_RF_TECH_424F:
trf->iso_ctrl_tech = TRF7970A_ISO_CTRL_NFC_NFC_CE_MODE |
TRF7970A_ISO_CTRL_NFC_NFCF_424;
- trf->modulator_sys_clk_ctrl = TRF7970A_MODULATOR_DEPTH_ASK10;
+ trf->modulator_sys_clk_ctrl =
+ (trf->modulator_sys_clk_ctrl & 0xF8) |
+ TRF7970A_MODULATOR_DEPTH_ASK10;
break;
default:
dev_dbg(trf->dev, "Unsupported rf technology: %d\n", tech);
@@ -1987,6 +2005,7 @@ static int trf7970a_probe(struct spi_device *spi)
struct device_node *np = spi->dev.of_node;
struct trf7970a *trf;
int uvolts, autosuspend_delay, ret;
+ u32 clk_freq = 13560000;
if (!np) {
dev_err(&spi->dev, "No Device Tree entry\n");
@@ -2043,6 +2062,15 @@ static int trf7970a_probe(struct spi_device *spi)
return ret;
}
+ of_property_read_u32(np, "clock-frequency", &clk_freq);
+ if ((clk_freq != TRF7970A_27MHZ_CLOCK_FREQUENCY) ||
+ (clk_freq != TRF7970A_27MHZ_CLOCK_FREQUENCY)) {
+ dev_err(trf->dev,
+ "clock-frequency (%u Hz) unsupported\n",
+ clk_freq);
+ return -EINVAL;
+ }
+
if (of_property_read_bool(np, "en2-rf-quirk"))
trf->quirks |= TRF7970A_QUIRK_EN2_MUST_STAY_LOW;
--
Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
^ permalink raw reply related
* Re: [PATCH] rtlwifi: Fix kernel oops introduced with commit e49656147359
From: Larry Finger @ 2016-12-20 15:25 UTC (permalink / raw)
To: Kalle Valo
Cc: Linus Torvalds, devel, linux-wireless, linux-kernel, driver-devel,
Stable, Wei Yongjun
In-Reply-To: <87wpeuu9qm.fsf@kamboji.qca.qualcomm.com>
On 12/20/2016 05:21 AM, Kalle Valo wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
>
>> With commit e49656147359 {"rtlwifi: Use dev_kfree_skb_irq instead of
>> kfree_skb"), the method used to free an skb was changed because the
>> kfree_skb() was inside a spinlock. What was forgotten is that kfree_skb()
>> guards against a NULL value for the argument. Routine dev_kfree_skb_irq()
>> does not, and a test is needed to prevent kernel panics.
>>
>> Fixes: commit e49656147359 {"rtlwifi: Use dev_kfree_skb_irq instead of kfree_skb")
>
> This should be:
>
> Fixes: e49656147359 ("rtlwifi: Use dev_kfree_skb_irq instead of kfree_skb")
>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Cc: Stable <stable@vger.kernel.org> (4.9+)
>
> And this:
>
> Cc: Stable <stable@vger.kernel.org> # 4.9+
>
> I can fix both of them.
>
>> Cc: Wei Yongjun <weiyongjun1@huawei.com>
>> ---
>> Kalle,
>>
>> This change should be sent to mainline during the 4.10 merge period,
>> or as soon as possible.
>
> Ok, I'll queue this to 4.10. Most likely I'll send a pull request to
> Dave later this week or so.
Thanks for the suggested changes, and for the quick action.
Larry
^ permalink raw reply
* Re: ath10k: free host-mem with DMA_BIRECTIONAL flag.
From: Kalle Valo @ 2016-12-20 15:06 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless, Ben Greear, ath10k
In-Reply-To: <1480962519-13027-1-git-send-email-greearb@candelatech.com>
Ben Greear <greearb@candelatech.com> wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> Hopefully this fixes the problem reported by Kalle:
>
> Noticed this in my log, but I don't have time to investigate this in
> detail right now:
>
> [ 413.795346] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
> [ 414.158755] IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
> [ 477.439659] ath10k_pci 0000:02:00.0: could not get mac80211 beacon
> [ 481.666630] ------------[ cut here ]------------
> [ 481.666669] WARNING: CPU: 0 PID: 1978 at lib/dma-debug.c:1155 check_unmap+0x320/0x8e0
> [ 481.666688] ath10k_pci 0000:02:00.0: DMA-API: device driver frees DMA memory with different direction [device address=0x000000002d130000] [size=63800 bytes] [mapped with DMA_BIDIRECTIONAL] [unmapped with DMA_TO_DEVICE]
> [ 481.666703] Modules linked in: ctr ccm ath10k_pci(E-) ath10k_core(E) ath(E) mac80211(E) cfg80211(E) snd_hda_codec_hdmi snd_hda_codec_idt snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_pcm snd_seq_midi arc4 snd_rawmidi snd_seq_midi_event snd_seq btusb btintel snd_seq_device joydev coret
> [ 481.671468] CPU: 0 PID: 1978 Comm: rmmod Tainted: G E 4.9.0-rc7-wt+ #54
> [ 481.671478] Hardware name: Hewlett-Packard HP ProBook 6540b/1722, BIOS 68CDD Ver. F.04 01/27/2010
> [ 481.671489] ef49dcec c842ee92 c8b5830e ef49dd34 ef49dd20 c80850f5 c8b5a13c ef49dd50
> [ 481.671560] 000007ba c8b5830e 00000483 c8461830 c8461830 00000483 ef49ddcc f34e64b8
> [ 481.671641] c8b58360 ef49dd3c c80851bb 00000009 00000000 ef49dd34 c8b5a13c ef49dd50
> [ 481.671716] Call Trace:
> [ 481.671731] [<c842ee92>] dump_stack+0x76/0xb4
> [ 481.671745] [<c80850f5>] __warn+0xe5/0x100
> [ 481.671757] [<c8461830>] ? check_unmap+0x320/0x8e0
> [ 481.671769] [<c8461830>] ? check_unmap+0x320/0x8e0
> [ 481.671780] [<c80851bb>] warn_slowpath_fmt+0x3b/0x40
> [ 481.671791] [<c8461830>] check_unmap+0x320/0x8e0
> [ 481.671804] [<c8462054>] debug_dma_unmap_page+0x84/0xa0
> [ 481.671835] [<f937cd7a>] ath10k_wmi_free_host_mem+0x9a/0xe0 [ath10k_core]
> [ 481.671861] [<f9363400>] ath10k_core_destroy+0x50/0x60 [ath10k_core]
> [ 481.671875] [<f8e13969>] ath10k_pci_remove+0x79/0xa0 [ath10k_pci]
> [ 481.671889] [<c848d8d8>] pci_device_remove+0x38/0xb0
> [ 481.671901] [<c859fe4b>] __device_release_driver+0x7b/0x110
> [ 481.671913] [<c85a00e7>] driver_detach+0x97/0xa0
> [ 481.671923] [<c859ef8b>] bus_remove_driver+0x4b/0xb0
> [ 481.671934] [<c85a0cda>] driver_unregister+0x2a/0x60
> [ 481.671949] [<c848c888>] pci_unregister_driver+0x18/0x70
> [ 481.671965] [<f8e14dae>] ath10k_pci_exit+0xd/0x25f [ath10k_pci]
> [ 481.671979] [<c812bb84>] SyS_delete_module+0xf4/0x180
> [ 481.671995] [<c81f801b>] ? __might_fault+0x8b/0xa0
> [ 481.672009] [<c80037d0>] do_fast_syscall_32+0xa0/0x1e0
> [ 481.672025] [<c88d4c88>] sysenter_past_esp+0x45/0x74
> [ 481.672037] ---[ end trace 3fd23759e17e1622 ]---
> [ 481.672049] Mapped at:
> [ 481.672060] [ 481.672072] [<c846062c>] debug_dma_map_page.part.25+0x1c/0xf0
> [ 481.672083] [ 481.672095] [<c8460799>] debug_dma_map_page+0x99/0xc0
> [ 481.672106] [ 481.672132] [<f93745ec>] ath10k_wmi_alloc_chunk+0x12c/0x1f0 [ath10k_core]
> [ 481.672142] [ 481.672168] [<f937d0c4>] ath10k_wmi_event_service_ready_work+0x304/0x540 [ath10k_core]
> [ 481.672178] [ 481.672190] [<c80a3643>] process_one_work+0x1c3/0x670
> [ 482.137134] ath10k_pci 0000:02:00.0: pci irq msi oper_irq_mode 2 irq_mode 0 reset_mode 0
> [ 482.313144] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/pre-cal-pci-0000:02:00.0.bin failed with error -2
> [ 482.313274] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/cal-pci-0000:02:00.0.bin failed with error -2
> [ 482.313768] ath10k_pci 0000:02:00.0: qca988x hw2.0 target 0x4100016c chip_id 0x043202ff sub 0000:0000
> [ 482.313777] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 0 testmode 1
> [ 482.313974] ath10k_pci 0000:02:00.0: firmware ver 10.2.4.70.59-2 api 5 features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
> [ 482.369858] ath10k_pci 0000:02:00.0: Direct firmware load for ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
> [ 482.370011] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id N/A crc32 bebc7c08
> [ 483.596770] ath10k_pci 0000:02:00.0: htt-ver 2.1 wmi-op 5 htt-op 2 cal otp max-sta 128 raw 0 hwcrypto 1
> [ 483.701686] ath: EEPROM regdomain: 0x0
> [ 483.701706] ath: EEPROM indicates default country code should be used
> [ 483.701713] ath: doing EEPROM country->regdmn map search
> [ 483.701721] ath: country maps to regdmn code: 0x3a
> [ 483.701730] ath: Country alpha2 being used: US
> [ 483.701737] ath: Regpair used: 0x3a
>
> Reported-by: Kalle Valo <kvalo@qca.qualcomm.com>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
Patch applied to ath-current branch of ath.git, thanks.
d4166b8b3365 ath10k: free host-mem with DMA_BIRECTIONAL flag
--
https://patchwork.kernel.org/patch/9461307/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: regression: ath_tx_edma_tasklet() Illegal idle entry in RCU read-side critical section
From: Valo, Kalle @ 2016-12-20 14:20 UTC (permalink / raw)
To: Gabriel C
Cc: paulmck@linux.vnet.ibm.com, Tobias Klausmann, lkml, ath9k-devel,
linux-wireless@vger.kernel.org, ath9k-devel@lists.ath9k.org,
netdev@vger.kernel.org, nbd@nbd.name
In-Reply-To: <20b64afc-8270-27d5-6d45-592ce6dd24c7@gmail.com>
Gabriel C <nix.or.die@gmail.com> writes:
> On 18.12.2016 21:14, Paul E. McKenney wrote:
>> On Sun, Dec 18, 2016 at 07:57:42PM +0000, Valo, Kalle wrote:
>>> Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de> writes:
>>>
>>>> A patch for this is already floating on the ML for a while now latest:
>>>> (ath9k: do not return early to fix rcu unlocking)
>>>
>>> It's here:
>>>
>>> https://patchwork.kernel.org/patch/9472709/
>>
>> Feel free to add:
>>
>> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>>
>
> This patch fixes the bug for me.
>
> You can add my Tested-by if you wish.
>
> Tested-by: Gabriel Craciunescu <nix.or.die@gmail.com>
I added both of these to the commit log, thanks.
--=20
Kalle Valo=
^ permalink raw reply
* Re: [for-4.10, 1/2] brcmfmac: fix memory leak in brcmf_cfg80211_attach()
From: Kalle Valo @ 2016-12-20 14:10 UTC (permalink / raw)
To: Arend Van Spriel; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1481283254-17883-2-git-send-email-arend.vanspriel@broadcom.com>
Arend Van Spriel <arend.vanspriel@broadcom.com> wrote:
> In brcmf_cfg80211_attach() there was one error path not properly
> handled as it leaked memory allocated in brcmf_btcoex_attach().
>
> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
2 patches applied to wireless-drivers.git, thanks.
cb853da3a368 brcmfmac: fix memory leak in brcmf_cfg80211_attach()
2b66325d5ea7 brcmfmac: fix uninitialized field in scheduled scan ssid configuration
--
https://patchwork.kernel.org/patch/9467971/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Kalle Valo @ 2016-12-20 11:47 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Pali Rohár, Daniel Wagner, Luis R. Rodriguez, Tom Gundersen,
Johannes Berg, Ming Lei, Mimi Zohar, Bjorn Andersson,
Rafał Miłecki, Sebastian Reichel, Pavel Machek,
Michal Kazior, Ivaylo Dimitrov, Aaro Koskinen, Tony Lindgren,
linux-wireless, Network Development, linux-kernel@vger.kernel.org,
David Woodhouse, Takashi Iwai, Josh Boyer, Dmitry Torokhov
In-Reply-To: <fd180271-1cd4-9891-e3d5-ae9cd0fb088b@broadcom.com>
Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
> On 18-12-2016 13:09, Pali Roh=C3=A1r wrote:
>
>> File wl1251-nvs.bin is provided by linux-firmware package and contains=20
>> default data which should be overriden by model specific calibrated=20
>> data.
>
> Ah. Someone thought it was a good idea to provide the "one ring to rule
> them all". Nice.
Yes, that was a bad idea. wl1251-nvs.bin in linux-firmware.git should be
renamed to wl1251-nvs.bin.example, or something like that, as it should
be only installed to a real system only if there's no real calibration
data available (only for developers to use, not real users).
>> But overwriting that one file is not possible as it next update of=20
>> linux-firmware package will overwrite it back. It break any normal usage=
=20
>> of package management.
>>=20
>> Also it is ridiculously broken by design if some "boot" files needs to=20
>> be overwritten to initialize hardware properly. To not break booting you=
=20
>> need to overwrite that file before first boot. But without booting=20
>> device you cannot read calibration data. So some hack with autoreboot=20
>> after boot is needed.
Providing the calibration data via Device Tree is the proper way to
solve this. Yes yes, I know N900 doesn't support it but that's a
deficiency in N900, not Linux.
>> And how to detect that we have real overwritten calibration data and
>> not default one from linux-firmware? Any heuristic or checks will be
>> broken here. And no, nothing like you need to reboot your device now
>> (and similar concept) from windows world is not accepted.
>
> Well. After reading and creating calibration data you could just rebind
> the driver to the device to have it probed again.
Or load wl1251 as a module and make sure calibration data is installed
before the module is loaded. LEDE does that with ath10k:
https://git.lede-project.org/?p=3Dsource.git;a=3Dblob;f=3Dtarget/linux/ar71=
xx/base-files/etc/hotplug.d/firmware/11-ath10k-caldata;h=3D97875bd79a579a00=
10da3f60324b6ec966fe9c6a;hb=3DHEAD
> But yeah, the default one from linux-firmware should never have been
> there in the first place.
Agreed.
--=20
Kalle Valo
^ permalink raw reply
* Re: [PATCH] rtlwifi: Fix kernel oops introduced with commit e49656147359
From: Kalle Valo @ 2016-12-20 11:21 UTC (permalink / raw)
To: Larry Finger
Cc: Linus Torvalds, devel, linux-wireless, linux-kernel, driver-devel,
Stable, Wei Yongjun
In-Reply-To: <20161220023812.5999-1-Larry.Finger@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> writes:
> With commit e49656147359 {"rtlwifi: Use dev_kfree_skb_irq instead of
> kfree_skb"), the method used to free an skb was changed because the
> kfree_skb() was inside a spinlock. What was forgotten is that kfree_skb()
> guards against a NULL value for the argument. Routine dev_kfree_skb_irq()
> does not, and a test is needed to prevent kernel panics.
>
> Fixes: commit e49656147359 {"rtlwifi: Use dev_kfree_skb_irq instead of kfree_skb")
This should be:
Fixes: e49656147359 ("rtlwifi: Use dev_kfree_skb_irq instead of kfree_skb")
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@vger.kernel.org> (4.9+)
And this:
Cc: Stable <stable@vger.kernel.org> # 4.9+
I can fix both of them.
> Cc: Wei Yongjun <weiyongjun1@huawei.com>
> ---
> Kalle,
>
> This change should be sent to mainline during the 4.10 merge period,
> or as soon as possible.
Ok, I'll queue this to 4.10. Most likely I'll send a pull request to
Dave later this week or so.
--
Kalle Valo
^ permalink raw reply
* [PATCH v3] ath10k: Fix crash during rmmod when probe firmware fails
From: Mohammed Shafi Shajakhan @ 2016-12-20 8:09 UTC (permalink / raw)
To: ath10k; +Cc: mohammed, linux-wireless, Mohammed Shafi Shajakhan
From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
This fixes the below crash when ath10k probe firmware fails,
NAPI polling tries to access a rx ring resource which was never
allocated, fix this by disabling NAPI right away once the probe
firmware fails by calling 'ath10k_hif_stop'. Its good to note
that the error is never propogated to 'ath10k_pci_probe' when
ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup
PCI related things seems to be ok
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
__ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
Call Trace:
[<ffffffffa113ec62>] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90
[ath10k_core]
[<ffffffffa113f393>] ath10k_htt_txrx_compl_task+0x433/0x17d0
[ath10k_core]
[<ffffffff8114406d>] ? __wake_up_common+0x4d/0x80
[<ffffffff811349ec>] ? cpu_load_update+0xdc/0x150
[<ffffffffa119301d>] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]
[<ffffffffa1195b17>] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]
[<ffffffff817863af>] net_rx_action+0x20f/0x370
Reported-by: Ben Greear <greearb@candelatech.com>
Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support")
Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
---
drivers/net/wireless/ath/ath10k/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index f7ea4de..15bccc9 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
ath10k_core_free_firmware_files(ar);
err_power_down:
+ ath10k_hif_stop(ar);
ath10k_hif_power_down(ar);
return ret;
--
1.9.1
^ permalink raw reply related
* [PATCH v2] ath10k: Fix crash during rmmod when probe firmware fails
From: Mohammed Shafi Shajakhan @ 2016-12-20 8:05 UTC (permalink / raw)
To: ath10k; +Cc: mohammed, linux-wireless, Mohammed Shafi Shajakhan
From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
This fixes the below crash when ath10k probe firmware fails,
NAPI polling tries to access a rx ring resource which was never
allocated, fix this by disabling NAPI right away once the probe
firmware fails by calling 'ath10k_hif_stop'. Its good to note
that the error is never propogated to 'ath10k_pci_probe' when
ath10k_core_register fails, so calling 'ath10k_hif_stop' to cleanup
PCI related things seems to be ok
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: __ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
__ath10k_htt_rx_ring_fill_n+0x19/0x230 [ath10k_core]
Call Trace:
[<ffffffffa113ec62>] ath10k_htt_rx_msdu_buff_replenish+0x42/0x90
[ath10k_core]
[<ffffffffa113f393>] ath10k_htt_txrx_compl_task+0x433/0x17d0
[ath10k_core]
[<ffffffff8114406d>] ? __wake_up_common+0x4d/0x80
[<ffffffff811349ec>] ? cpu_load_update+0xdc/0x150
[<ffffffffa119301d>] ? ath10k_pci_read32+0xd/0x10 [ath10k_pci]
[<ffffffffa1195b17>] ath10k_pci_napi_poll+0x47/0x110 [ath10k_pci]
[<ffffffff817863af>] net_rx_action+0x20f/0x370
Reported-by: Ben Greear <greearb@candelatech.com>
Fixes: 3c97f5de1f28 ("ath10k: implement NAPI support")
Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
---
drivers/net/wireless/ath/ath10k/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index f7ea4de..15bccc9 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2164,6 +2164,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
ath10k_core_free_firmware_files(ar);
err_power_down:
+ ath10k_hif_stop(ar);
ath10k_hif_power_down(ar);
return ret;
--
1.9.1
^ permalink raw reply related
* Re: "rfkill: Add rfkill-any LED trigger" causes deadlock
From: Johannes Berg @ 2016-12-20 7:47 UTC (permalink / raw)
To: Michał Kępień
Cc: Михаил Кринкин,
linux-wireless, davem, netdev
In-Reply-To: <20161219140115.GA18194@kmp-mobile.hq.kempniu.pl>
> Just to ensure things get cleaned up properly, as of now it looks
> like you only reverted patch 2/2 of my v2 and Arnd's fix to patch
> 1/2, not patch 1/2 itself.
Oops. I've fixed that up to only revert your patch - I wanted the
revert in the tree to document the issue, rather than just dropping the
patch entirely. Thanks for noticing that mistake.
johannes
^ permalink raw reply
* [PATCH] rtlwifi: Fix kernel oops introduced with commit e49656147359
From: Larry Finger @ 2016-12-20 2:38 UTC (permalink / raw)
To: kvalo, Linus Torvalds
Cc: devel, linux-wireless, Larry Finger, linux-kernel, driver-devel,
Stable, Wei Yongjun
With commit e49656147359 {"rtlwifi: Use dev_kfree_skb_irq instead of
kfree_skb"), the method used to free an skb was changed because the
kfree_skb() was inside a spinlock. What was forgotten is that kfree_skb()
guards against a NULL value for the argument. Routine dev_kfree_skb_irq()
does not, and a test is needed to prevent kernel panics.
Fixes: commit e49656147359 {"rtlwifi: Use dev_kfree_skb_irq instead of kfree_skb")
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org> (4.9+)
Cc: Wei Yongjun <weiyongjun1@huawei.com>
---
Kalle,
This change should be sent to mainline during the 4.10 merge period,
or as soon as possible.
Thanks,
Larry
---
drivers/net/wireless/realtek/rtlwifi/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/core.c b/drivers/net/wireless/realtek/rtlwifi/core.c
index 955055b..df8b977 100644
--- a/drivers/net/wireless/realtek/rtlwifi/core.c
+++ b/drivers/net/wireless/realtek/rtlwifi/core.c
@@ -1823,7 +1823,8 @@ bool rtl_cmd_send_packet(struct ieee80211_hw *hw, struct sk_buff *skb)
spin_lock_irqsave(&rtlpriv->locks.irq_th_lock, flags);
pskb = __skb_dequeue(&ring->queue);
- dev_kfree_skb_irq(pskb);
+ if (pskb)
+ dev_kfree_skb_irq(pskb);
/*this is wrong, fill_tx_cmddesc needs update*/
pdesc = &ring->desc[0];
--
2.10.2
^ permalink raw reply related
* Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock
From: Geoff Lansberry @ 2016-12-19 23:23 UTC (permalink / raw)
To: Rob Herring
Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
Samuel Ortiz, mark.rutland, netdev, devicetree, linux-kernel,
Mark Greer, Justin Bronder
In-Reply-To: <20161219223111.itu77g7wsqtj6cvs@rob-hp-laptop>
I can make that change, however, I worry that it may be a bit
misleading, since there are only two supported clock frequencies, but
a number like that to me implies that it could be set to any number
you want. I'm new at this, and so I'll go ahead and change it as you
request, but I'd like to hear your thoughts on my concern.
Thanks
Geoff
Geoff Lansberry
Engineering Guy
Kuv=C3=A9e, Inc
125 Kingston St., 3rd Floor
Boston, MA 02111
1-617-290-1118 (m)
geoff.lansberry (skype)
http://www.kuvee.com
On Mon, Dec 19, 2016 at 5:31 PM, Rob Herring <robh@kernel.org> wrote:
> On Thu, Dec 15, 2016 at 05:30:42PM -0500, Geoff Lansberry wrote:
>> From: Geoff Lansberry <geoff@kuvee.com>
>>
>> ---
>> .../devicetree/bindings/net/nfc/trf7970a.txt | 3 ++
>> drivers/nfc/trf7970a.c | 42 +++++++++++++++=
+------
>> 2 files changed, 34 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Do=
cumentation/devicetree/bindings/net/nfc/trf7970a.txt
>> index 32b35a0..9dda879 100644
>> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> @@ -21,6 +21,8 @@ Optional SoC Specific Properties:
>> - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
>> where an extra byte is returned by Read Multiple Block commands issue=
d
>> to Type 5 tags.
>> +- crystal_27mhz: Set to specify that the input frequency to the trf7970=
a is 27.12MHz
>> +
>
> Can't you use 'clock-frequency =3D "27000000";'?
>
>>
>> Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>>
>> @@ -43,6 +45,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI=
1):
>> irq-status-read-quirk;
>> en2-rf-quirk;
>> t5t-rmb-extra-byte-quirk;
>> + crystal_27mhz;
>> status =3D "okay";
>> };
>> };
^ permalink raw reply
* Re: [PATCH 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
From: Rob Herring @ 2016-12-19 22:35 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo,
mark.rutland, netdev, devicetree, linux-kernel, mgreer, justin
In-Reply-To: <1481841044-4314-2-git-send-email-glansberry@gmail.com>
On Thu, Dec 15, 2016 at 05:30:43PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <geoff@kuvee.com>
>
> ---
> Documentation/devicetree/bindings/net/nfc/trf7970a.txt | 2 ++
> drivers/nfc/trf7970a.c | 13 ++++++++++++-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 9dda879..208f045 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -21,6 +21,7 @@ Optional SoC Specific Properties:
> - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
> where an extra byte is returned by Read Multiple Block commands issued
> to Type 5 tags.
> +- vdd_io_1v8: Set to specify that the trf7970a io voltage should be set to 1.8V
Use the regulator binding and provide a fixed 1.8V supply.
> - crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
>
>
> @@ -45,6 +46,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
> irq-status-read-quirk;
> en2-rf-quirk;
> t5t-rmb-extra-byte-quirk;
> + vdd_io_1v8;
> crystal_27mhz;
> status = "okay";
> };
^ permalink raw reply
* Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock
From: Rob Herring @ 2016-12-19 22:31 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo,
mark.rutland, netdev, devicetree, linux-kernel, mgreer, justin
In-Reply-To: <1481841044-4314-1-git-send-email-glansberry@gmail.com>
On Thu, Dec 15, 2016 at 05:30:42PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <geoff@kuvee.com>
>
> ---
> .../devicetree/bindings/net/nfc/trf7970a.txt | 3 ++
> drivers/nfc/trf7970a.c | 42 ++++++++++++++++------
> 2 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 32b35a0..9dda879 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -21,6 +21,8 @@ Optional SoC Specific Properties:
> - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
> where an extra byte is returned by Read Multiple Block commands issued
> to Type 5 tags.
> +- crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
> +
Can't you use 'clock-frequency = "27000000";'?
>
> Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>
> @@ -43,6 +45,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
> irq-status-read-quirk;
> en2-rf-quirk;
> t5t-rmb-extra-byte-quirk;
> + crystal_27mhz;
> status = "okay";
> };
> };
^ permalink raw reply
* Re: [PATCH] ath10k: merge extended peer info data with existing peers info
From: Christian Lamparter @ 2016-12-19 18:37 UTC (permalink / raw)
To: Mohammed Shafi Shajakhan; +Cc: linux-wireless, ath10k, Kalle Valo
In-Reply-To: <20161219164957.GA13022@atheros-ThinkPad-T61>
Hello Shafi,
On Monday, December 19, 2016 10:19:57 PM CET Mohammed Shafi Shajakhan wrote:
> On Sat, Dec 17, 2016 at 06:46:34PM +0100, Christian Lamparter wrote:
> > The 10.4 firmware adds extended peer information to the
> > firmware's statistics payload. This additional info is
> > stored as a separate data field. During review of
> > "ath10k: add accounting for the extended peer statistics" [0]
> >
> > Mohammed Shafi Shajakhan commented that the extended peer statistics
> > lists are of little use:"... there is not much use in appending
> > the extended peer stats (which gets periodically updated) to the
> > linked list '&ar->debug.fw_stats.peers_extd)' and should we get
> > rid of the below (and the required cleanup as well)
> >
> > list_splice_tail_init(&stats.peers_extd,
> > &ar->debug.fw_stats.peers_extd);
> >
> > since rx_duration is getting updated periodically to the per sta
> > information."
> >
> > This patch replaces the extended peers list with a lookup and
> > puts the retrieved data (rx_duration) into the existing
> > ath10k_fw_stats_peer entry that was created earlier.
>
> [shafi] Its good to maintain the extended stats variable
> and retain the two different functions to update rx_duration.
>
> a) extended peer stats supported - mainly for 10.4
> b) extender peer stats not supported - for 10.2
Well, I have to ask why you want to retain the two different
functions to update the same arsta->rx_duration? I think a
little bit of code that helps to explain what's on your mind
(or how you would do it) would help immensely in this case.
Since I have the feeling that this is the problem here.
So please explain it in C(lang).
> > [0] <https://lkml.kernel.org/r/992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunkeey@googlemail.com>
> > Cc: Mohammed Shafi Shajakhan <mohammed@codeaurora.org>
> > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> > ---
> > drivers/net/wireless/ath/ath10k/core.h | 2 --
> > drivers/net/wireless/ath/ath10k/debug.c | 17 --------------
> > drivers/net/wireless/ath/ath10k/debugfs_sta.c | 32 ++-----------------------
> > drivers/net/wireless/ath/ath10k/wmi.c | 34 ++++++++++++++++++++-------
> > 4 files changed, 28 insertions(+), 57 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> > index 09ff8b8a6441..3fffbbb18c25 100644
> > --- a/drivers/net/wireless/ath/ath10k/core.h
> > +++ b/drivers/net/wireless/ath/ath10k/core.h
> > @@ -268,11 +268,9 @@ struct ath10k_fw_stats_pdev {
> > };
> >
> > struct ath10k_fw_stats {
> > - bool extended;
> > struct list_head pdevs;
> > struct list_head vdevs;
> > struct list_head peers;
> > - struct list_head peers_extd;
> > };
> >
> > #define ATH10K_TPC_TABLE_TYPE_FLAG 1
> > diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> > index 82a4c67f3672..89f7fde77cdf 100644
> > --- a/drivers/net/wireless/ath/ath10k/debug.c
> > +++ b/drivers/net/wireless/ath/ath10k/debug.c
> > @@ -315,25 +315,13 @@ static void ath10k_fw_stats_peers_free(struct list_head *head)
> > }
> > }
> >
> > -static void ath10k_fw_extd_stats_peers_free(struct list_head *head)
> > -{
> > - struct ath10k_fw_extd_stats_peer *i, *tmp;
> > -
> > - list_for_each_entry_safe(i, tmp, head, list) {
> > - list_del(&i->list);
> > - kfree(i);
> > - }
> > -}
> > -
> > static void ath10k_debug_fw_stats_reset(struct ath10k *ar)
> > {
> > spin_lock_bh(&ar->data_lock);
> > ar->debug.fw_stats_done = false;
> > - ar->debug.fw_stats.extended = false;
>
> [shafi] this looks fine, but not removing the 'extended' variable
> from ath10k_fw_stats structure, I see the design for 'rx_duration'
> arguably some what convoluted !
I removed extended because it is now a write-only variable.
So I figured, there's no point in keeping it around? I don't have
access to the firmware interface documentation, so I don't know
if there's a reason why it would be good to have it later.
So please tell me, what information do we gain from it?
> *We get periodic events from firmware updating 'ath10k_debug_fw_stats_process'
> *Fetch rx_duration from 'ath10k_wmi_pull_fw_stats(ar, skb, &stats)'
> {certainly 'stats' object is for this particular update only, and freed
> up later)
> *Update immediately using 'ath10k_sta_update_rx_duration'
>
> 'ath10k_wmi_pull_fw_stats' has a slightly different implementation
> for 10.2 and 10.4 (the later supporting extended peer stats)
I see that 10.2.4's ath10k_wmi_10_2_4_op_pull_fw_stats()
passes the rx_duration as part of the wmi_10_2_4_ext_peer_stats
element which is basically a carbon-copy of wmi_10_2_4_peer_stats
(but with one extra __le32 for the rx_duration at the end.)
This rx_duration is then used to set the rx_duration field in the
generated ath10k_fw_stats_peer struct.
10.4's ath10k_wmi_10_4_op_pull_fw_stats() has a "fixed" wmi_10_4_peer_stats
element and uses an separate "fixed" wmi_10_4_peer_extd_stats element for
the communicating the rx_duration to the driver.
Thing is, both function have the same signature. They produce the same
struct ath10k_fw_stats_peer for the given data in the sk_buff input. So
why does 10.4 need to have it's peer_extd infrastructure, when it can use
the existing rx_duration field in the *universal* ath10k_fw_stats_peer?
What's with the extra layers / HAL here? Because it looks like it's merged
back together in the same manner into the same arsta->rx_duration?
[ath10k_sta_update_stats_rx_duration() vs.
ath10k_sta_update_extd_stats_rx_duration() - they are almost carbon copies too]
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> > index c893314a191f..c7ec7b9e9b55 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> > @@ -3044,23 +3044,41 @@ static int ath10k_wmi_10_4_op_pull_fw_stats(struct ath10k *ar,
> > if ((stats_id & WMI_10_4_STAT_PEER_EXTD) == 0)
> > return 0;
> >
> > - stats->extended = true;
> > -
> > for (i = 0; i < num_peer_stats; i++) {
> > const struct wmi_10_4_peer_extd_stats *src;
> > - struct ath10k_fw_extd_stats_peer *dst;
> > + struct ath10k_fw_stats_peer *dst;
> >
> > src = (void *)skb->data;
> > if (!skb_pull(skb, sizeof(*src)))
> > return -EPROTO;
> >
> > - dst = kzalloc(sizeof(*dst), GFP_ATOMIC);
> > - if (!dst)
> > - continue;
> > + /* Because the stat data may exceed htc-wmi buffer
> > + * limit the firmware might split the stats data
> > + * and delivers it in multiple update events.
> > + * if we can't find the entry in the current event
> > + * payload, we have to look in main list as well.
> > + */
> > + list_for_each_entry(dst, &stats->peers, list) {
> > + if (ether_addr_equal(dst->peer_macaddr,
> > + src->peer_macaddr.addr))
> > + goto found;
> > + }
> > +
> > +#ifdef CONFIG_ATH10K_DEBUGFS
> > + list_for_each_entry(dst, &ar->debug.fw_stats.peers, list) {
> > + if (ether_addr_equal(dst->peer_macaddr,
> > + src->peer_macaddr.addr))
> > + goto found;
> > + }
> > +#endif
> > +
> > + ath10k_dbg(ar, ATH10K_DBG_WMI,
> > + "Orphaned extended stats entry for station %pM.\n",
> > + src->peer_macaddr.addr);
> > + continue;
> >
> > - ether_addr_copy(dst->peer_macaddr, src->peer_macaddr.addr);
> > +found:
> > dst->rx_duration = __le32_to_cpu(src->rx_duration);
> > - list_add_tail(&dst->list, &stats->peers_extd);
> > }
> >
> > return 0;
>
> [shafi] Yes i am bit concerned about this change making 10.4 to update
> over the existing peer_stats structure, the idea is to maintain uniformity
> between the structures shared between ath10k and its corresponding to avoid
> confusion/ bugs in the future. Kindly let me know your thoughts, feel free
> to correct me if any of my analysis is incorrect. thank you !
Isn't the point of the ath10k_wmi_10_*_op_pull_fw_stats() functions to make
a "universal" statistic (in your case a unified ath10k_fw_stats_peer structure)
that other functions can use, without caring about if the FW was 10.4
or 10.2.4?
There's no indication that the rx_duration field in wmi_10_2_4_ext_peer_stats
conveys any different information than the rx_duration in
wmi_10_4_peer_extd_stats. So, what's going on here? Can't you just make
wmi_10_4_peer_extd_stats's rx_duration use the existing field in
ath10k_fw_stats_peer? And if not: why exactly?
Regards,
Christian
^ permalink raw reply
* Re: [PATCH] ath10k: merge extended peer info data with existing peers info
From: Mohammed Shafi Shajakhan @ 2016-12-19 16:58 UTC (permalink / raw)
To: Christian Lamparter; +Cc: Kalle Valo, linux-wireless, ath10k
In-Reply-To: <20161219164957.GA13022@atheros-ThinkPad-T61>
On Mon, Dec 19, 2016 at 10:19:57PM +0530, Mohammed Shafi Shajakhan wrote:
> Hi Christian,
>
> On Sat, Dec 17, 2016 at 06:46:34PM +0100, Christian Lamparter wrote:
> > The 10.4 firmware adds extended peer information to the
> > firmware's statistics payload. This additional info is
> > stored as a separate data field. During review of
> > "ath10k: add accounting for the extended peer statistics" [0]
> >
> > Mohammed Shafi Shajakhan commented that the extended peer statistics
> > lists are of little use:"... there is not much use in appending
> > the extended peer stats (which gets periodically updated) to the
> > linked list '&ar->debug.fw_stats.peers_extd)' and should we get
> > rid of the below (and the required cleanup as well)
> >
> > list_splice_tail_init(&stats.peers_extd,
> > &ar->debug.fw_stats.peers_extd);
> >
> > since rx_duration is getting updated periodically to the per sta
> > information."
>
> [shafi] thanks !
>
> >
> > This patch replaces the extended peers list with a lookup and
> > puts the retrieved data (rx_duration) into the existing
> > ath10k_fw_stats_peer entry that was created earlier.
>
> [shafi] Its good to maintain the extended stats variable
> and retain the two different functions to update rx_duration.
>
> a) extended peer stats supported - mainly for 10.4
> b) extender peer stats not supported - for 10.2
>
>
> >
> > [0] <https://lkml.kernel.org/r/992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunkeey@googlemail.com>
> > Cc: Mohammed Shafi Shajakhan <mohammed@codeaurora.org>
> > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> > ---
> > drivers/net/wireless/ath/ath10k/core.h | 2 --
> > drivers/net/wireless/ath/ath10k/debug.c | 17 --------------
> > drivers/net/wireless/ath/ath10k/debugfs_sta.c | 32 ++-----------------------
> > drivers/net/wireless/ath/ath10k/wmi.c | 34 ++++++++++++++++++++-------
> > 4 files changed, 28 insertions(+), 57 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> > index 09ff8b8a6441..3fffbbb18c25 100644
> > --- a/drivers/net/wireless/ath/ath10k/core.h
> > +++ b/drivers/net/wireless/ath/ath10k/core.h
> > @@ -268,11 +268,9 @@ struct ath10k_fw_stats_pdev {
> > };
> >
> > struct ath10k_fw_stats {
> > - bool extended;
> > struct list_head pdevs;
> > struct list_head vdevs;
> > struct list_head peers;
> > - struct list_head peers_extd;
> > };
> >
> > #define ATH10K_TPC_TABLE_TYPE_FLAG 1
> > diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> > index 82a4c67f3672..89f7fde77cdf 100644
> > --- a/drivers/net/wireless/ath/ath10k/debug.c
> > +++ b/drivers/net/wireless/ath/ath10k/debug.c
> > @@ -315,25 +315,13 @@ static void ath10k_fw_stats_peers_free(struct list_head *head)
> > }
> > }
> >
> > -static void ath10k_fw_extd_stats_peers_free(struct list_head *head)
> > -{
> > - struct ath10k_fw_extd_stats_peer *i, *tmp;
> > -
> > - list_for_each_entry_safe(i, tmp, head, list) {
> > - list_del(&i->list);
> > - kfree(i);
> > - }
> > -}
> > -
> > static void ath10k_debug_fw_stats_reset(struct ath10k *ar)
> > {
> > spin_lock_bh(&ar->data_lock);
> > ar->debug.fw_stats_done = false;
> > - ar->debug.fw_stats.extended = false;
>
> [shafi] this looks fine, but not removing the 'extended' variable
> from ath10k_fw_stats structure, I see the design for 'rx_duration'
> arguably some what convoluted !
>
> *We get periodic events from firmware updating 'ath10k_debug_fw_stats_process'
> *Fetch rx_duration from 'ath10k_wmi_pull_fw_stats(ar, skb, &stats)'
> {certainly 'stats' object is for this particular update only, and freed
> up later)
> *Update immediately using 'ath10k_sta_update_rx_duration'
>
> 'ath10k_wmi_pull_fw_stats' has a slightly different implementation
> for 10.2 and 10.4 (the later supporting extended peer stats)
>
>
>
> > ath10k_fw_stats_pdevs_free(&ar->debug.fw_stats.pdevs);
> > ath10k_fw_stats_vdevs_free(&ar->debug.fw_stats.vdevs);
> > ath10k_fw_stats_peers_free(&ar->debug.fw_stats.peers);
> > - ath10k_fw_extd_stats_peers_free(&ar->debug.fw_stats.peers_extd);
> > spin_unlock_bh(&ar->data_lock);
> > }
> >
> > @@ -348,7 +336,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
> > INIT_LIST_HEAD(&stats.pdevs);
> > INIT_LIST_HEAD(&stats.vdevs);
> > INIT_LIST_HEAD(&stats.peers);
> > - INIT_LIST_HEAD(&stats.peers_extd);
> >
> > spin_lock_bh(&ar->data_lock);
> > ret = ath10k_wmi_pull_fw_stats(ar, skb, &stats);
> > @@ -411,8 +398,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
> >
> > list_splice_tail_init(&stats.peers, &ar->debug.fw_stats.peers);
> > list_splice_tail_init(&stats.vdevs, &ar->debug.fw_stats.vdevs);
> > - list_splice_tail_init(&stats.peers_extd,
> > - &ar->debug.fw_stats.peers_extd);
> > }
> >
> > complete(&ar->debug.fw_stats_complete);
> > @@ -424,7 +409,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
> > ath10k_fw_stats_pdevs_free(&stats.pdevs);
> > ath10k_fw_stats_vdevs_free(&stats.vdevs);
> > ath10k_fw_stats_peers_free(&stats.peers);
> > - ath10k_fw_extd_stats_peers_free(&stats.peers_extd);
> >
> > spin_unlock_bh(&ar->data_lock);
> > }
> > @@ -2347,7 +2331,6 @@ int ath10k_debug_create(struct ath10k *ar)
> > INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs);
> > INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs);
> > INIT_LIST_HEAD(&ar->debug.fw_stats.peers);
> > - INIT_LIST_HEAD(&ar->debug.fw_stats.peers_extd);
> >
> > return 0;
> > }
> > diff --git a/drivers/net/wireless/ath/ath10k/debugfs_sta.c b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
> > index fce6f8137d33..bf2d49cbb3bb 100644
> > --- a/drivers/net/wireless/ath/ath10k/debugfs_sta.c
> > +++ b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
> > @@ -18,27 +18,8 @@
> > #include "wmi-ops.h"
> > #include "debug.h"
> >
> > -static void ath10k_sta_update_extd_stats_rx_duration(struct ath10k *ar,
> > - struct ath10k_fw_stats *stats)
> > -{
> > - struct ath10k_fw_extd_stats_peer *peer;
> > - struct ieee80211_sta *sta;
> > - struct ath10k_sta *arsta;
> > -
> > - rcu_read_lock();
> > - list_for_each_entry(peer, &stats->peers_extd, list) {
> > - sta = ieee80211_find_sta_by_ifaddr(ar->hw, peer->peer_macaddr,
> > - NULL);
> > - if (!sta)
> > - continue;
> > - arsta = (struct ath10k_sta *)sta->drv_priv;
> > - arsta->rx_duration += (u64)peer->rx_duration;
> > - }
> > - rcu_read_unlock();
> > -}
> > -
> > -static void ath10k_sta_update_stats_rx_duration(struct ath10k *ar,
> > - struct ath10k_fw_stats *stats)
> > +void ath10k_sta_update_rx_duration(struct ath10k *ar,
> > + struct ath10k_fw_stats *stats)
> > {
> > struct ath10k_fw_stats_peer *peer;
> > struct ieee80211_sta *sta;
> > @@ -56,15 +37,6 @@ static void ath10k_sta_update_stats_rx_duration(struct ath10k *ar,
> > rcu_read_unlock();
> > }
> >
> > -void ath10k_sta_update_rx_duration(struct ath10k *ar,
> > - struct ath10k_fw_stats *stats)
> > -{
> > - if (stats->extended)
> > - ath10k_sta_update_extd_stats_rx_duration(ar, stats);
> > - else
> > - ath10k_sta_update_stats_rx_duration(ar, stats);
> > -}
> > -
> > void ath10k_sta_statistics(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > struct ieee80211_sta *sta,
> > struct station_info *sinfo)
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> > index c893314a191f..c7ec7b9e9b55 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> > @@ -3044,23 +3044,41 @@ static int ath10k_wmi_10_4_op_pull_fw_stats(struct ath10k *ar,
> > if ((stats_id & WMI_10_4_STAT_PEER_EXTD) == 0)
> > return 0;
> >
> > - stats->extended = true;
> > -
> > for (i = 0; i < num_peer_stats; i++) {
> > const struct wmi_10_4_peer_extd_stats *src;
> > - struct ath10k_fw_extd_stats_peer *dst;
> > + struct ath10k_fw_stats_peer *dst;
> >
> > src = (void *)skb->data;
> > if (!skb_pull(skb, sizeof(*src)))
> > return -EPROTO;
> >
> > - dst = kzalloc(sizeof(*dst), GFP_ATOMIC);
> > - if (!dst)
> > - continue;
> > + /* Because the stat data may exceed htc-wmi buffer
> > + * limit the firmware might split the stats data
> > + * and delivers it in multiple update events.
> > + * if we can't find the entry in the current event
> > + * payload, we have to look in main list as well.
> > + */
> > + list_for_each_entry(dst, &stats->peers, list) {
> > + if (ether_addr_equal(dst->peer_macaddr,
> > + src->peer_macaddr.addr))
> > + goto found;
> > + }
> > +
> > +#ifdef CONFIG_ATH10K_DEBUGFS
> > + list_for_each_entry(dst, &ar->debug.fw_stats.peers, list) {
> > + if (ether_addr_equal(dst->peer_macaddr,
> > + src->peer_macaddr.addr))
> > + goto found;
> > + }
> > +#endif
> > +
> > + ath10k_dbg(ar, ATH10K_DBG_WMI,
> > + "Orphaned extended stats entry for station %pM.\n",
> > + src->peer_macaddr.addr);
> > + continue;
> >
> > - ether_addr_copy(dst->peer_macaddr, src->peer_macaddr.addr);
> > +found:
> > dst->rx_duration = __le32_to_cpu(src->rx_duration);
> > - list_add_tail(&dst->list, &stats->peers_extd);
> > }
> >
> > return 0;
>
> [shafi] Yes i am bit concerned about this change making 10.4 to update
> over the existing peer_stats structure, the idea is to maintain uniformity
> between the structures shared between ath10k and its corresponding *firmware* to avoid
> bugs in the future. Kindly let me know your thoughts, feel free
> to correct me if any of my analysis is incorrect. thank you !
>
> regards,
> shafi
>
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply
* Re: [PATCH] ath10k: merge extended peer info data with existing peers info
From: Mohammed Shafi Shajakhan @ 2016-12-19 16:49 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, ath10k, Kalle Valo
In-Reply-To: <20161217174634.4531-1-chunkeey@googlemail.com>
Hi Christian,
On Sat, Dec 17, 2016 at 06:46:34PM +0100, Christian Lamparter wrote:
> The 10.4 firmware adds extended peer information to the
> firmware's statistics payload. This additional info is
> stored as a separate data field. During review of
> "ath10k: add accounting for the extended peer statistics" [0]
>
> Mohammed Shafi Shajakhan commented that the extended peer statistics
> lists are of little use:"... there is not much use in appending
> the extended peer stats (which gets periodically updated) to the
> linked list '&ar->debug.fw_stats.peers_extd)' and should we get
> rid of the below (and the required cleanup as well)
>
> list_splice_tail_init(&stats.peers_extd,
> &ar->debug.fw_stats.peers_extd);
>
> since rx_duration is getting updated periodically to the per sta
> information."
[shafi] thanks !
>
> This patch replaces the extended peers list with a lookup and
> puts the retrieved data (rx_duration) into the existing
> ath10k_fw_stats_peer entry that was created earlier.
[shafi] Its good to maintain the extended stats variable
and retain the two different functions to update rx_duration.
a) extended peer stats supported - mainly for 10.4
b) extender peer stats not supported - for 10.2
>
> [0] <https://lkml.kernel.org/r/992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunkeey@googlemail.com>
> Cc: Mohammed Shafi Shajakhan <mohammed@codeaurora.org>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
> drivers/net/wireless/ath/ath10k/core.h | 2 --
> drivers/net/wireless/ath/ath10k/debug.c | 17 --------------
> drivers/net/wireless/ath/ath10k/debugfs_sta.c | 32 ++-----------------------
> drivers/net/wireless/ath/ath10k/wmi.c | 34 ++++++++++++++++++++-------
> 4 files changed, 28 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> index 09ff8b8a6441..3fffbbb18c25 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -268,11 +268,9 @@ struct ath10k_fw_stats_pdev {
> };
>
> struct ath10k_fw_stats {
> - bool extended;
> struct list_head pdevs;
> struct list_head vdevs;
> struct list_head peers;
> - struct list_head peers_extd;
> };
>
> #define ATH10K_TPC_TABLE_TYPE_FLAG 1
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> index 82a4c67f3672..89f7fde77cdf 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -315,25 +315,13 @@ static void ath10k_fw_stats_peers_free(struct list_head *head)
> }
> }
>
> -static void ath10k_fw_extd_stats_peers_free(struct list_head *head)
> -{
> - struct ath10k_fw_extd_stats_peer *i, *tmp;
> -
> - list_for_each_entry_safe(i, tmp, head, list) {
> - list_del(&i->list);
> - kfree(i);
> - }
> -}
> -
> static void ath10k_debug_fw_stats_reset(struct ath10k *ar)
> {
> spin_lock_bh(&ar->data_lock);
> ar->debug.fw_stats_done = false;
> - ar->debug.fw_stats.extended = false;
[shafi] this looks fine, but not removing the 'extended' variable
from ath10k_fw_stats structure, I see the design for 'rx_duration'
arguably some what convoluted !
*We get periodic events from firmware updating 'ath10k_debug_fw_stats_process'
*Fetch rx_duration from 'ath10k_wmi_pull_fw_stats(ar, skb, &stats)'
{certainly 'stats' object is for this particular update only, and freed
up later)
*Update immediately using 'ath10k_sta_update_rx_duration'
'ath10k_wmi_pull_fw_stats' has a slightly different implementation
for 10.2 and 10.4 (the later supporting extended peer stats)
> ath10k_fw_stats_pdevs_free(&ar->debug.fw_stats.pdevs);
> ath10k_fw_stats_vdevs_free(&ar->debug.fw_stats.vdevs);
> ath10k_fw_stats_peers_free(&ar->debug.fw_stats.peers);
> - ath10k_fw_extd_stats_peers_free(&ar->debug.fw_stats.peers_extd);
> spin_unlock_bh(&ar->data_lock);
> }
>
> @@ -348,7 +336,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
> INIT_LIST_HEAD(&stats.pdevs);
> INIT_LIST_HEAD(&stats.vdevs);
> INIT_LIST_HEAD(&stats.peers);
> - INIT_LIST_HEAD(&stats.peers_extd);
>
> spin_lock_bh(&ar->data_lock);
> ret = ath10k_wmi_pull_fw_stats(ar, skb, &stats);
> @@ -411,8 +398,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
>
> list_splice_tail_init(&stats.peers, &ar->debug.fw_stats.peers);
> list_splice_tail_init(&stats.vdevs, &ar->debug.fw_stats.vdevs);
> - list_splice_tail_init(&stats.peers_extd,
> - &ar->debug.fw_stats.peers_extd);
> }
>
> complete(&ar->debug.fw_stats_complete);
> @@ -424,7 +409,6 @@ void ath10k_debug_fw_stats_process(struct ath10k *ar, struct sk_buff *skb)
> ath10k_fw_stats_pdevs_free(&stats.pdevs);
> ath10k_fw_stats_vdevs_free(&stats.vdevs);
> ath10k_fw_stats_peers_free(&stats.peers);
> - ath10k_fw_extd_stats_peers_free(&stats.peers_extd);
>
> spin_unlock_bh(&ar->data_lock);
> }
> @@ -2347,7 +2331,6 @@ int ath10k_debug_create(struct ath10k *ar)
> INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs);
> INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs);
> INIT_LIST_HEAD(&ar->debug.fw_stats.peers);
> - INIT_LIST_HEAD(&ar->debug.fw_stats.peers_extd);
>
> return 0;
> }
> diff --git a/drivers/net/wireless/ath/ath10k/debugfs_sta.c b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
> index fce6f8137d33..bf2d49cbb3bb 100644
> --- a/drivers/net/wireless/ath/ath10k/debugfs_sta.c
> +++ b/drivers/net/wireless/ath/ath10k/debugfs_sta.c
> @@ -18,27 +18,8 @@
> #include "wmi-ops.h"
> #include "debug.h"
>
> -static void ath10k_sta_update_extd_stats_rx_duration(struct ath10k *ar,
> - struct ath10k_fw_stats *stats)
> -{
> - struct ath10k_fw_extd_stats_peer *peer;
> - struct ieee80211_sta *sta;
> - struct ath10k_sta *arsta;
> -
> - rcu_read_lock();
> - list_for_each_entry(peer, &stats->peers_extd, list) {
> - sta = ieee80211_find_sta_by_ifaddr(ar->hw, peer->peer_macaddr,
> - NULL);
> - if (!sta)
> - continue;
> - arsta = (struct ath10k_sta *)sta->drv_priv;
> - arsta->rx_duration += (u64)peer->rx_duration;
> - }
> - rcu_read_unlock();
> -}
> -
> -static void ath10k_sta_update_stats_rx_duration(struct ath10k *ar,
> - struct ath10k_fw_stats *stats)
> +void ath10k_sta_update_rx_duration(struct ath10k *ar,
> + struct ath10k_fw_stats *stats)
> {
> struct ath10k_fw_stats_peer *peer;
> struct ieee80211_sta *sta;
> @@ -56,15 +37,6 @@ static void ath10k_sta_update_stats_rx_duration(struct ath10k *ar,
> rcu_read_unlock();
> }
>
> -void ath10k_sta_update_rx_duration(struct ath10k *ar,
> - struct ath10k_fw_stats *stats)
> -{
> - if (stats->extended)
> - ath10k_sta_update_extd_stats_rx_duration(ar, stats);
> - else
> - ath10k_sta_update_stats_rx_duration(ar, stats);
> -}
> -
> void ath10k_sta_statistics(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> struct ieee80211_sta *sta,
> struct station_info *sinfo)
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index c893314a191f..c7ec7b9e9b55 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -3044,23 +3044,41 @@ static int ath10k_wmi_10_4_op_pull_fw_stats(struct ath10k *ar,
> if ((stats_id & WMI_10_4_STAT_PEER_EXTD) == 0)
> return 0;
>
> - stats->extended = true;
> -
> for (i = 0; i < num_peer_stats; i++) {
> const struct wmi_10_4_peer_extd_stats *src;
> - struct ath10k_fw_extd_stats_peer *dst;
> + struct ath10k_fw_stats_peer *dst;
>
> src = (void *)skb->data;
> if (!skb_pull(skb, sizeof(*src)))
> return -EPROTO;
>
> - dst = kzalloc(sizeof(*dst), GFP_ATOMIC);
> - if (!dst)
> - continue;
> + /* Because the stat data may exceed htc-wmi buffer
> + * limit the firmware might split the stats data
> + * and delivers it in multiple update events.
> + * if we can't find the entry in the current event
> + * payload, we have to look in main list as well.
> + */
> + list_for_each_entry(dst, &stats->peers, list) {
> + if (ether_addr_equal(dst->peer_macaddr,
> + src->peer_macaddr.addr))
> + goto found;
> + }
> +
> +#ifdef CONFIG_ATH10K_DEBUGFS
> + list_for_each_entry(dst, &ar->debug.fw_stats.peers, list) {
> + if (ether_addr_equal(dst->peer_macaddr,
> + src->peer_macaddr.addr))
> + goto found;
> + }
> +#endif
> +
> + ath10k_dbg(ar, ATH10K_DBG_WMI,
> + "Orphaned extended stats entry for station %pM.\n",
> + src->peer_macaddr.addr);
> + continue;
>
> - ether_addr_copy(dst->peer_macaddr, src->peer_macaddr.addr);
> +found:
> dst->rx_duration = __le32_to_cpu(src->rx_duration);
> - list_add_tail(&dst->list, &stats->peers_extd);
> }
>
> return 0;
[shafi] Yes i am bit concerned about this change making 10.4 to update
over the existing peer_stats structure, the idea is to maintain uniformity
between the structures shared between ath10k and its corresponding to avoid
confusion/ bugs in the future. Kindly let me know your thoughts, feel free
to correct me if any of my analysis is incorrect. thank you !
regards,
shafi
^ permalink raw reply
* Re: "rfkill: Add rfkill-any LED trigger" causes deadlock
From: Michał Kępień @ 2016-12-19 14:01 UTC (permalink / raw)
To: Johannes Berg
Cc: Михаил Кринкин,
linux-wireless, davem, netdev
In-Reply-To: <1482137772.31461.1.camel@sipsolutions.net>
Mike, Johannes, sorry for the trouble caused.
> Thanks for the report. I'm sorry I missed this in review - obviously we
> can't call something that acquires the mutex from rfkill_set_sw_state()
> which clearly states, in the documentation:
>
> * This function can be called in any context, even from within rfkill
> * callbacks.
Drat, I missed that, sorry.
> I've reverted the change (and the follow-up fix) now.
Just to ensure things get cleaned up properly, as of now it looks like
you only reverted patch 2/2 of my v2 and Arnd's fix to patch 1/2, not
patch 1/2 itself.
> Michał, if you want to resubmit with this fixed, please also make sure
> you don't reintroduce the unused label warning and have the appropriate
> #ifdef that Arnd had later added for your change.
Noted, thanks. I will post v3 once I figure out how to handle locking
properly.
--
Best regards,
Michał Kępień
^ permalink raw reply
* Re: [RFC 2/3] mac80211: Implement data xmit for 802.11 encap offload
From: Thiagarajan, Vasanthakumar @ 2016-12-19 12:02 UTC (permalink / raw)
To: Kalle Valo; +Cc: Johannes Berg, nbd@nbd.name, linux-wireless@vger.kernel.org
In-Reply-To: <878trckurn.fsf@purkki.adurom.net>
On Monday 19 December 2016 05:15 PM, Kalle Valo wrote:
> "Thiagarajan, Vasanthakumar" <vthiagar@qti.qualcomm.com> writes:
>
>> On Thursday 15 December 2016 07:23 PM, Johannes Berg wrote:
>>>
>>>>> I agree. Dynamic switch part is buggy, we can start with not
>>>>> allowing interfaces resulting in dynamic switch.
>>>>
>>>> Does this mean that when bringing up multiple interfaces, users would
>>>> need to figure out the 'magic' order that works?
>>>
>>> I think we need to talk about hardware capabilities at this point.
>>
>> QCA988X does not have capability to configure vif specific decap mode. E=
ncap mode
>> is configurable per packet for all the ath10k based chips so this part s=
hould be
>> fine to support per vif configuration. Newer QCA chips like QCA9984, QCA=
4019, QCA9888
>> and QCA99X0 supports decap mode configuration per vif.
>
> When you say "all" are you also taking into account QCA6174 and QCA9377?
Good point. I see some workarounds for QCA6174 to send data frames in ether=
net mode,
so QCA6174 should be fine in this regard. I assume QCA9377 also works fine =
with
ethernet encap configuration per htt desc, need to confirm.
Vasanth=
^ permalink raw reply
* Re: [RFC 2/3] mac80211: Implement data xmit for 802.11 encap offload
From: Kalle Valo @ 2016-12-19 11:45 UTC (permalink / raw)
To: Thiagarajan, Vasanthakumar
Cc: Johannes Berg, nbd@nbd.name, linux-wireless@vger.kernel.org
In-Reply-To: <58537DA3.1090901@qti.qualcomm.com>
"Thiagarajan, Vasanthakumar" <vthiagar@qti.qualcomm.com> writes:
> On Thursday 15 December 2016 07:23 PM, Johannes Berg wrote:
>>
>>>> I agree. Dynamic switch part is buggy, we can start with not
>>>> allowing interfaces resulting in dynamic switch.
>>>
>>> Does this mean that when bringing up multiple interfaces, users would
>>> need to figure out the 'magic' order that works?
>>
>> I think we need to talk about hardware capabilities at this point.
>
> QCA988X does not have capability to configure vif specific decap mode. Encap mode
> is configurable per packet for all the ath10k based chips so this part should be
> fine to support per vif configuration. Newer QCA chips like QCA9984, QCA4019, QCA9888
> and QCA99X0 supports decap mode configuration per vif.
When you say "all" are you also taking into account QCA6174 and QCA9377?
Just checking...
--
Kalle Valo
^ permalink raw reply
* [PATCH v2] cfg80211: add set/get link loss profile
From: Lazar, Alexei Avshalom @ 2016-12-19 10:58 UTC (permalink / raw)
To: Kalle Valo; +Cc: johannes, linux-wireless, wil6210
In-Reply-To: <8737ipm4jy.fsf@kamboji.qca.qualcomm.com>
>From 7e41d45179a8762a7d2e1653db35751c19c8c747 Mon Sep 17 00:00:00 2001
From: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
Date: Sun, 18 Dec 2016 15:06:38 +0200
Subject: [PATCH] cfg80211: add set/get link loss profile
Introduce NL80211_CMD_SET_LINK_LOSS_PROFILE and
NL80211_CMD_GET_LINK_LOSS_PROFILE needed by user space to configure the
link loss profile.
The link loss profile represents the priority of maintaining link up
(connection to AP) in different link quality environments.
This is intended for full mac80211 drivers. Typically link loss detection
is offloaded to firmware, since it requires close monitoring of the RF.
The firmware will disconnect when link quality drops.
There are scenarios that require to lose connection faster once reduced
link quality is detected in order to maintain better user experience.
This can be used in AP and station mode.
Example use case is FST (Fast Session Transfer), where we have a fast
11ad connection but close-range and a backup 11ac connection which is
"always on". In such cases if we detect poor link quality in the active
11ad band we prefer to disconnect and switch to the backup 11ac connection
with the good link quality. This will be reached by setting "aggressive"
profile that will cause more rapid disconnect in such cases.
Three types of behavior for link loss are defined:
LINK_LOSS_PROFILE_RELAXED: prefer maintaining link up even in a poor
link quality environment.
LINK_LOSS_PROFILE_DEFAULT: The default behavior for maintaining link
up vs link quality.
LINK_LOSS_PROFILE_AGGRESSIVE: prefer losing link up in a poor link
quality environment.
New cfg80211 API also been added, set/get_link_loss_profile.
Signed-off-by: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/cfg80211.c | 18 ++++++++
include/net/cfg80211.h | 13 ++++++
include/uapi/linux/nl80211.h | 32 +++++++++++++
net/wireless/nl80211.c | 70 +++++++++++++++++++++++++++++
net/wireless/rdev-ops.h | 28 ++++++++++++
net/wireless/trace.h | 39 ++++++++++++++++
6 files changed, 200 insertions(+)
diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index 6aa3ff4..681b792 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -1499,6 +1499,22 @@ static int wil_cfg80211_set_power_mgmt(struct wiphy *wiphy,
return rc;
}
+static int
+wil_cfg80211_set_link_loss_profile(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ enum nl80211_link_loss_profile profile,
+ const u8 *addr)
+{
+ return -ENOTSUPP;
+}
+
+static enum nl80211_link_loss_profile
+wil_cfg80211_get_link_loss_profile(struct wiphy *wiphy,
+ struct wireless_dev *wdev, const u8 *addr)
+{
+ return -ENOTSUPP;
+}
+
static struct cfg80211_ops wil_cfg80211_ops = {
.add_virtual_intf = wil_cfg80211_add_iface,
.del_virtual_intf = wil_cfg80211_del_iface,
@@ -1528,6 +1544,8 @@ static struct cfg80211_ops wil_cfg80211_ops = {
.start_p2p_device = wil_cfg80211_start_p2p_device,
.stop_p2p_device = wil_cfg80211_stop_p2p_device,
.set_power_mgmt = wil_cfg80211_set_power_mgmt,
+ .set_link_loss_profile = wil_cfg80211_set_link_loss_profile,
+ .get_link_loss_profile = wil_cfg80211_get_link_loss_profile,
};
static void wil_wiphy_init(struct wiphy *wiphy)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ca2ac1c..7c2e420 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2764,6 +2764,10 @@ struct cfg80211_nan_func {
* All other parameters must be ignored.
*
* @set_multicast_to_unicast: configure multicast to unicast conversion for BSS
+ *
+ * @set_link_loss_profile: Set link loss profile for specific connection.
+ * @get_link_loss_profile: Get the current link loss profile of specific
+ * connection.
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3048,6 +3052,15 @@ struct cfg80211_ops {
int (*set_multicast_to_unicast)(struct wiphy *wiphy,
struct net_device *dev,
const bool enabled);
+
+ int (*set_link_loss_profile)(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ enum nl80211_link_loss_profile profile,
+ const u8 *addr);
+ enum nl80211_link_loss_profile (*get_link_loss_profile)(
+ struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ const u8 *addr);
};
/*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index d74e10b..7723294 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -894,6 +894,11 @@
* does not result in a change for the current association. Currently,
* only the %NL80211_ATTR_IE data is used and updated with this command.
*
+ * @NL80211_CMD_SET_LINK_LOSS_PROFILE: Set link loss profile (behavior) for
+ * specific connection.
+ * @NL80211_CMD_GET_LINK_LOSS_PROFILE: Get current link loss profile of specific
+ * connection.
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -1093,6 +1098,9 @@ enum nl80211_commands {
NL80211_CMD_UPDATE_CONNECT_PARAMS,
+ NL80211_CMD_SET_LINK_LOSS_PROFILE,
+ NL80211_CMD_GET_LINK_LOSS_PROFILE,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
@@ -1980,6 +1988,9 @@ enum nl80211_commands {
* @NL80211_ATTR_BSSID: The BSSID of the AP. Note that %NL80211_ATTR_MAC is also
* used in various commands/events for specifying the BSSID.
*
+ * @NL80211_ATTR_LINK_LOSS_PROFILE: attribute that indicate the link loss
+ * behavior using &enum nl80211_link_loss_profile values.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2386,6 +2397,8 @@ enum nl80211_attrs {
NL80211_ATTR_BSSID,
+ NL80211_ATTR_LINK_LOSS_PROFILE,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -5188,4 +5201,23 @@ enum nl80211_nan_match_attributes {
NL80211_NAN_MATCH_ATTR_MAX = NUM_NL80211_NAN_MATCH_ATTR - 1
};
+/**
+ * enum nl80211_link_loss_profile.
+ *
+ * Used by set_link_loss_profile() and get_link_loss_profile()
+ * to set/get link loss behavior.
+ *
+ * @NL80211_LINK_LOSS_PROFILE_RELAXED: prefer maintaining link
+ * up even in poor link quality environment
+ * @NL80211_LINK_LOSS_PROFILE_DEFAULT: The default behavior for
+ * maintaining link up vs link quality.
+ * @NL80211_LINK_LOSS_PROFILE_AGGRESSIVE: prefer losing link
+ * up in poor link quality environment
+ */
+enum nl80211_link_loss_profile {
+ NL80211_LINK_LOSS_PROFILE_RELAXED,
+ NL80211_LINK_LOSS_PROFILE_DEFAULT,
+ NL80211_LINK_LOSS_PROFILE_AGGRESSIVE
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2369265..e160630 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -405,6 +405,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
[NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
+ [NL80211_ATTR_LINK_LOSS_PROFILE] = { .type = NLA_U8 },
};
/* policy for the key attributes */
@@ -11817,6 +11818,60 @@ static int nl80211_set_multicast_to_unicast(struct sk_buff *skb,
return rdev_set_multicast_to_unicast(rdev, dev, enabled);
}
+static int nl80211_set_link_loss_profile(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+ enum nl80211_link_loss_profile profile;
+ const u8 *addr;
+ int ret;
+
+ if (!info->attrs[NL80211_ATTR_LINK_LOSS_PROFILE] ||
+ !info->attrs[NL80211_ATTR_MAC])
+ return -EINVAL;
+
+ profile = nla_get_u8(info->attrs[NL80211_ATTR_LINK_LOSS_PROFILE]);
+ addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
+
+ if (profile != NL80211_LINK_LOSS_PROFILE_RELAXED &&
+ profile != NL80211_LINK_LOSS_PROFILE_DEFAULT &&
+ profile != NL80211_LINK_LOSS_PROFILE_AGGRESSIVE)
+ return -EINVAL;
+
+ if (!rdev->ops->set_link_loss_profile)
+ return -EOPNOTSUPP;
+
+ wdev_lock(wdev);
+ ret = rdev_set_link_loss_profile(rdev, wdev, profile, addr);
+ wdev_unlock(wdev);
+
+ return ret;
+}
+
+static int nl80211_get_link_loss_profile(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+ const u8 *addr;
+ enum nl80211_link_loss_profile profile;
+
+ if (!info->attrs[NL80211_ATTR_MAC])
+ return -EINVAL;
+
+ addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
+
+ if (!rdev->ops->get_link_loss_profile)
+ return -EOPNOTSUPP;
+
+ wdev_lock(wdev);
+ profile = rdev_get_link_loss_profile(rdev, wdev, addr);
+ wdev_unlock(wdev);
+
+ return profile;
+}
+
#define NL80211_FLAG_NEED_WIPHY 0x01
#define NL80211_FLAG_NEED_NETDEV 0x02
#define NL80211_FLAG_NEED_RTNL 0x04
@@ -12692,6 +12747,21 @@ static const struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
+ {
+ .cmd = NL80211_CMD_SET_LINK_LOSS_PROFILE,
+ .doit = nl80211_set_link_loss_profile,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
+ .cmd = NL80211_CMD_GET_LINK_LOSS_PROFILE,
+ .doit = nl80211_get_link_loss_profile,
+ .policy = nl80211_policy,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
};
static struct genl_family nl80211_fam __ro_after_init = {
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 2f42507..caf0127 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1153,4 +1153,32 @@ rdev_set_coalesce(struct cfg80211_registered_device *rdev,
trace_rdev_return_int(&rdev->wiphy, ret);
return ret;
}
+
+static inline int
+rdev_set_link_loss_profile(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev,
+ enum nl80211_link_loss_profile profile,
+ const u8 *addr)
+{
+ int ret;
+
+ trace_rdev_set_link_loss_profile(&rdev->wiphy, wdev, profile, addr);
+ ret = rdev->ops->set_link_loss_profile(&rdev->wiphy, wdev, profile,
+ addr);
+ trace_rdev_return_int(&rdev->wiphy, ret);
+ return ret;
+}
+
+static inline enum nl80211_link_loss_profile
+rdev_get_link_loss_profile(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev,
+ const u8 *addr)
+{
+ enum nl80211_link_loss_profile profile;
+
+ trace_rdev_get_link_loss_profile(&rdev->wiphy, wdev, addr);
+ profile = rdev->ops->get_link_loss_profile(&rdev->wiphy, wdev, addr);
+ trace_rdev_return_int(&rdev->wiphy, profile);
+ return profile;
+}
#endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index ea1b47e..3af813b 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2230,6 +2230,45 @@ TRACE_EVENT(rdev_tdls_cancel_channel_switch,
WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(addr))
);
+TRACE_EVENT(rdev_set_link_loss_profile,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
+ enum nl80211_link_loss_profile profile, const u8 *addr),
+ TP_ARGS(wiphy, wdev, profile, addr),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ WDEV_ENTRY
+ __field(enum nl80211_link_loss_profile, profile)
+ MAC_ENTRY(addr)
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ WDEV_ASSIGN;
+ __entry->profile = profile;
+ MAC_ASSIGN(addr, addr);
+ ),
+ TP_printk(WIPHY_PR_FMT ", " WDEV_PR_FMT ", " MAC_PR_FMT ", PROFILE %d",
+ WIPHY_PR_ARG, WDEV_PR_ARG, MAC_PR_ARG(addr),
+ __entry->profile)
+);
+
+TRACE_EVENT(rdev_get_link_loss_profile,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
+ const u8 *addr),
+ TP_ARGS(wiphy, wdev, addr),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ WDEV_ENTRY
+ MAC_ENTRY(addr)
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ WDEV_ASSIGN;
+ MAC_ASSIGN(addr, addr);
+ ),
+ TP_printk(WIPHY_PR_FMT ", " WDEV_PR_FMT ", " MAC_PR_FMT, WIPHY_PR_ARG,
+ WDEV_PR_ARG, MAC_PR_ARG(addr))
+);
+
/*************************************************************
* cfg80211 exported functions traces *
*************************************************************/
--
2.5.0
^ permalink raw reply related
* Re: cfg80211: add set/get link loss profile
From: Lazar, Alexei Avshalom @ 2016-12-19 10:58 UTC (permalink / raw)
To: Kalle Valo; +Cc: johannes, linux-wireless, wil6210
In-Reply-To: <8737ipm4jy.fsf@kamboji.qca.qualcomm.com>
On 11/18/2016 1:05 PM, Kalle Valo wrote:
> Hi Alexei,
>
> "Lazar, Alexei Avshalom" <ailizaro@codeaurora.org> writes:
>
>> In this context the term "link" does mean connection to AP. There are
>> scenarios that require to lose connection faster once reduced link
>> quality detected in order maintain better user experience (from both
>> sides - AP and STA). Example case is FST (Fast Session Transfer),
>> where we have a fast 11ad connection but close-range and a backup 11ac
>> connection which is "always on". in such cases if we detect poor link
>> quality in the active band we prefer to disconnect and switch to the
>> backup connection with the good link quality. This will be reached by
>> setting "aggressive" profile that will cause more rapid disconnect in
>> such cases.
>
> The documentation should clearly document what this command is supposed,
> currently it only tells me "maintain/lose link" and does not specify in
> detail what this is supposed to do. Also there's no mention in what
> modes this can be used. (STA, P2P?)
>
Ok I will send a new patch with updated description.
> Also please refrain from top posting in public mailing lists, the
> preferred way is to edit quotes and bottom post. It's much more natural
> way to have long detailed discussions.
>
Sorry for that, I will refrain from top posting.
--
Alexei Lazar
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: cfg80211: add set/get link loss profile
From: Lazar, Alexei Avshalom @ 2016-12-19 10:58 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, wil6210
In-Reply-To: <1480344742.8107.63.camel@sipsolutions.net>
On 11/28/2016 4:52 PM, Johannes Berg wrote:
>
>> Introduce NL80211_CMD_SET_LINK_LOSS_PROFILE and
>> NL80211_CMD_GET_LINK_LOSS_PROFILE as it required by the user space
>> to configure the link loss profile.
>
> I'm not convinced.
>
>> The link loss profile represents the priority of maintaining link up
>> in different link quality environments.
>> Three types of behavior for link loss defined:
>> LINK_LOSS_PROFILE_RELAXED: prefer maintaining link up even in poor
>> link quality environment.
>> LINK_LOSS_PROFILE_DEFAULT: The default behavior for maintaining link
>> up vs link quality.
>> LINK_LOSS_PROFILE_AGGRESSIVE: prefer losing link up in poor link
>> quality environment.
>
> What you're doing here is *extremely* vague. No definitions of what
> this means, no description of how it's intended to be implemented, no
> note even on whether or not this is for full-MAC chips or not, etc.
>
The patch is intended for full-MAC chips and I will update the commit
message to include more details.
> At least for mac80211, we can define signals to userspace regarding
> "link quality" and make decisions in wpa_s then?
>
The policy is offloaded today to the FW and the decision being made by the FW.
We are only modifying the behavior and not making the decision.
>
>> --- a/drivers/net/wireless/ath/wil6210/cfg80211.c
>
> Those changes obviously don't belong into this patch anyway.
>
> johannes
>
--
Alexei Lazar
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH 11/11] rt2x00: add mutex to synchronize config and link tuner
From: Stanislaw Gruszka @ 2016-12-19 10:52 UTC (permalink / raw)
To: linux-wireless; +Cc: Helmut Schaa, Stanislaw Gruszka
In-Reply-To: <1482144777-16760-1-git-send-email-sgruszka@redhat.com>
Do not perform mac80211 config and link_tuner at the same time,
this can possibly result in wrong RF or BBP configuration.
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/ralink/rt2x00/rt2x00.h | 4 ++++
drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 1 +
drivers/net/wireless/ralink/rt2x00/rt2x00link.c | 5 +++++
drivers/net/wireless/ralink/rt2x00/rt2x00mac.c | 5 +++++
4 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00.h b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
index 01ce8a4..60dd4ac 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
@@ -834,6 +834,10 @@ struct rt2x00_dev {
struct mutex csr_mutex;
/*
+ * Mutex to synchronize config and link tuner.
+ */
+ struct mutex conf_mutex;
+ /*
* Current packet filter configuration for the device.
* This contains all currently active FIF_* flags send
* to us by mac80211 during configure_filter().
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 550eaf1..e464bdc 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -1296,6 +1296,7 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
spin_lock_init(&rt2x00dev->irqmask_lock);
mutex_init(&rt2x00dev->csr_mutex);
+ mutex_init(&rt2x00dev->conf_mutex);
INIT_LIST_HEAD(&rt2x00dev->bar_list);
spin_lock_init(&rt2x00dev->bar_list_lock);
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00link.c b/drivers/net/wireless/ralink/rt2x00/rt2x00link.c
index 73cbf23..2010a77 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00link.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00link.c
@@ -363,6 +363,9 @@ static void rt2x00link_tuner(struct work_struct *work)
test_bit(DEVICE_STATE_SCANNING, &rt2x00dev->flags))
return;
+ /* Do not race with rt2x00mac_config(). */
+ mutex_lock(&rt2x00dev->conf_mutex);
+
if (rt2x00dev->intf_sta_count)
rt2x00link_tuner_sta(rt2x00dev, link);
@@ -375,6 +378,8 @@ static void rt2x00link_tuner(struct work_struct *work)
(link->count % (VCO_SECONDS / LINK_TUNE_SECONDS)) == 0)
rt2x00dev->ops->lib->vco_calibration(rt2x00dev);
+ mutex_unlock(&rt2x00dev->conf_mutex);
+
/*
* Increase tuner counter, and reschedule the next link tuner run.
*/
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c
index d4b50fb..3cc1384 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c
@@ -320,6 +320,9 @@ int rt2x00mac_config(struct ieee80211_hw *hw, u32 changed)
*/
rt2x00queue_stop_queue(rt2x00dev->rx);
+ /* Do not race with with link tuner. */
+ mutex_lock(&rt2x00dev->conf_mutex);
+
/*
* When we've just turned on the radio, we want to reprogram
* everything to ensure a consistent state
@@ -335,6 +338,8 @@ int rt2x00mac_config(struct ieee80211_hw *hw, u32 changed)
*/
rt2x00lib_config_antenna(rt2x00dev, rt2x00dev->default_ant);
+ mutex_unlock(&rt2x00dev->conf_mutex);
+
/* Turn RX back on */
rt2x00queue_start_queue(rt2x00dev->rx);
--
1.7.1
^ permalink raw reply related
* [PATCH 10/11] rt2800: replace msleep() with usleep_range() on channel switch
From: Stanislaw Gruszka @ 2016-12-19 10:52 UTC (permalink / raw)
To: linux-wireless; +Cc: Helmut Schaa, Stanislaw Gruszka
In-Reply-To: <1482144777-16760-1-git-send-email-sgruszka@redhat.com>
msleep(1) can sleep much more time then requested 1ms, this is not good
on channel switch, which we want to be performed fast (i.e. to make scan
faster). Replace msleep() with usleep_range(), which has much smaller
maximum sleeping time boundary.
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 29d98e4..0220184 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -2110,7 +2110,9 @@ static void rt2800_config_channel_rf3xxx(struct rt2x00_dev *rt2x00dev,
rt2800_rfcsr_read(rt2x00dev, 30, &rfcsr);
rt2x00_set_field8(&rfcsr, RFCSR30_RF_CALIBRATION, 1);
rt2800_rfcsr_write(rt2x00dev, 30, rfcsr);
- msleep(1);
+
+ usleep_range(1000, 1500);
+
rt2x00_set_field8(&rfcsr, RFCSR30_RF_CALIBRATION, 0);
rt2800_rfcsr_write(rt2x00dev, 30, rfcsr);
}
@@ -3442,7 +3444,7 @@ static void rt2800_config_channel(struct rt2x00_dev *rt2x00dev,
}
}
- msleep(1);
+ usleep_range(1000, 1500);
/*
* Clear channel statistic counters
--
1.7.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox