Re: Change channel bandwidth without iw command
Hi, Thank you for your answer. The panic printout would be most helpful. My best guess is this crashes in ath10k_dbg() at the very beginning of ath10k_config_chan because ar-chandef.chan is NULL. This is probably the case since you use monitor_chandef which is probably empty at the point you invoke your code. First, we confirm that ar-chandef.chan is not NULL. We find out the the error occurs when wait_for_completion_timeout is called. Ath10k is trying to stop vdev (monitor_dev) when change the channel setting. We have no idea about the details of function, but it make waiting time duration for previous event. We also test by replacing wait_for_completion_timeout to wait_for_completion (i.e., it use maximum timeout value) But the kernel panic is still there. However, it is not easy to printout the messages when the panic is occurred, because the system stop (similar to bluescreen in widnows?). After reboot the system, the log messagess from panic is not recored in kernel log messages (/var/log/syslog, dmesg, kernel_llog) . The iw set channel is for monitor interfaces only so trying to make it work with, e.g. a station interface will bring you pain. We already know about this. So, we test this in monitor mode. Thank you for your answer. Sincerely yours, Okhwan 2014-10-22 19:40 GMT+09:00 Michal Kazior michal.kaz...@tieto.com: On 22 October 2014 12:14, Okhwan Lee oh...@mwnl.snu.ac.kr wrote: Hi, We are trying to implement a protocol to evaluate the performance our algorithm using QCA9880. To implement our protocol, a receiver have to change the bandwidth when a Action frame (what we define) is successfully received. We know that the QCA9880 can change bandwidth by using iw command in monitor mode. So, we use similar function path used by iw dev set freq ... When the receiver detects the reception of the Action frame we call ieee80211_set_monitor_channel at the end of ieee80211_rx as follows: /*** net/mac80211/rx.c **/ // receive action frame, change bandwidth 80 - 20 rtnl_lock(); //lock rtnetlink used in pre_doit of nl80211 chandef = local-monitor_chandef; // copy current chandef chandef.width = NL80211_CHAN_WIDTH_20; // set bandwidth chandef.center_freq1 = 5180; // set center freq... ieee80211_set_monitor_channel(wiphy, chandef); rtnl_unlock(); // If my understanding is correct you're doing it wrong. You probably want to modify chanctx of a vif the frame is associated with and notify the driver via appropriate mac80211 helpers instead of the hack above. Remember to get your locking right. However, the receiver invokes kernel panic when receives the Action frame. As fas as we know, the panic is occurred in ath10k_config_chan of ath10k device driver. The panic printout would be most helpful. My best guess is this crashes in ath10k_dbg() at the very beginning of ath10k_config_chan because ar-chandef.chan is NULL. This is probably the case since you use monitor_chandef which is probably empty at the point you invoke your code. To the best our knowledge, our implementation exploits same function path of iw command in nl80211, cfg80211, mac80211 and ath10k. Moreover, we confirm that the input parameters (wiphy, chandef) in our implementation are identical to the parameters used by iw command. Is there any reason why we cannot change bandwidth? Of course, iw command work correctly. The iw set channel is for monitor interfaces only so trying to make it work with, e.g. a station interface will bring you pain. Michał ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2 4/6] ath10k: make warm reset a bit safer and faster
On 20 October 2014 14:14, Michal Kazior michal.kaz...@tieto.com wrote: One of the problems with warm reset I've found is that it must be guaranteed that copy engine registers are not being accessed while being reset. Otherwise in worst case scenario the host may lock up. Instead of using sleeps and hoping the device is operational in some arbitrary timeframes use firmware indication register. As a side effect this makes driver boot/stop/recovery faster. Signed-off-by: Michal Kazior michal.kaz...@tieto.com --- Notes: v2: * fix kernel panic on early fw crash due to CE not being fully initialized [Janusz] Marek reported he sees a memory leak and git bisect blames this patch. Please don't apply this patch yet. Michał ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH] ath10k: add SURVEY_INFO_IN_USE for current channel on survey
On 2014-10-23 09:13, Kalle Valo wrote: Felix Fietkau n...@openwrt.org writes: Signed-off-by: Felix Fietkau n...@openwrt.org --- drivers/net/wireless/ath/ath10k/mac.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 4670930..bc440dc 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -3975,6 +3975,9 @@ static int ath10k_get_survey(struct ieee80211_hw *hw, int idx, survey-channel = sband-channels[idx]; +if (ar-rx_channel == survey-channel) +survey-filled |= SURVEY_INFO_IN_USE; + Why? Does this fix a visible bug? What changes from user space point of view? I'm asking because I want to add something to the commit log. When user space requests survey info, it is useful to know which of the survey data refers to the channel that is currently actively being used. One of the use cases is getting the current channel noise for status output. Without this flag you would have to look up the channel separately and then compare it against the frequency in the survey output in user space. - Felix ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2 4/6] ath10k: make warm reset a bit safer and faster
Michal Kazior michal.kaz...@tieto.com writes: On 20 October 2014 14:14, Michal Kazior michal.kaz...@tieto.com wrote: One of the problems with warm reset I've found is that it must be guaranteed that copy engine registers are not being accessed while being reset. Otherwise in worst case scenario the host may lock up. Instead of using sleeps and hoping the device is operational in some arbitrary timeframes use firmware indication register. As a side effect this makes driver boot/stop/recovery faster. Signed-off-by: Michal Kazior michal.kaz...@tieto.com --- Notes: v2: * fix kernel panic on early fw crash due to CE not being fully initialized [Janusz] Marek reported he sees a memory leak and git bisect blames this patch. Please don't apply this patch yet. Ok, thanks for letting me know. What about rest of the patches in the patchset? I assume it's safe to apply them. -- Kalle Valo ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2 4/6] ath10k: make warm reset a bit safer and faster
On 23 October 2014 15:14, Kalle Valo kv...@qca.qualcomm.com wrote: Michal Kazior michal.kaz...@tieto.com writes: On 20 October 2014 14:14, Michal Kazior michal.kaz...@tieto.com wrote: One of the problems with warm reset I've found is that it must be guaranteed that copy engine registers are not being accessed while being reset. Otherwise in worst case scenario the host may lock up. Instead of using sleeps and hoping the device is operational in some arbitrary timeframes use firmware indication register. As a side effect this makes driver boot/stop/recovery faster. Signed-off-by: Michal Kazior michal.kaz...@tieto.com --- Notes: v2: * fix kernel panic on early fw crash due to CE not being fully initialized [Janusz] Marek reported he sees a memory leak and git bisect blames this patch. Please don't apply this patch yet. Ok, thanks for letting me know. What about rest of the patches in the patchset? I assume it's safe to apply them. It's safe to apply: ath10k: re-disable interrupts after target init ath10k: mask/unmask msi fw irq ath10k: split ce pipe init/alloc further Please, do not apply the following yet: ath10k: make warm reset a bit safer and faster (this patch) ath10k: split reset logic from power up ath10k: don't reset chip on power_down Michał ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2 0/3] ath10k: speed up recovery
On 20 October 2014 14:22, Michal Kazior michal.kaz...@tieto.com wrote: Patch #1 fixes a bug I've found while testing patches #2 and #3. It's safe to cherry-pick it in case patches #2 and/or #3 aren't accepted or need a re-spin. I've mainly used patch #2 to test reset and recovery. It's pretty handy for (stress-)testing. Patch #3 should improve recovery speed in some cases. Notably when there's a long chain of WMI commands involved which take a painfully long time to timeout/complete if firmware crashes mid-way. Note: Patches #2 and #3 depend on `ath10k: pci related fixes 2014-10-09`. Without the patchset patches #2 and #3 must not be applied. Patch #1 is fine though. Do not apply this patchset yet, please. I need to do a minor fix in the 'ath10k: fix possible bmi crash' regarding warm reset patch from the other patchset. Michał ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH v2 0/6] ath10k: pci related fixes 2014-10-09
Michal Kazior michal.kaz...@tieto.com writes: First 2 patches address some irq issues. Other 4 fix and cleanup device resetting. Hopefully this will make ath10k a little more robust when it comes to warm reset. As a side effect of the changes ath10k now boots a little faster (slightly over a second instead of two). v2: * don't remove power_up/down [Kalle] * fix CE crash [Janusz] * remove irq workaround in patch #2 Michal Kazior (6): ath10k: re-disable interrupts after target init ath10k: mask/unmask msi fw irq ath10k: split ce pipe init/alloc further I have applied these patches. ath10k: make warm reset a bit safer and faster ath10k: split reset logic from power up ath10k: don't reset chip on power_down And dropped these patches per your request. -- Kalle Valo ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
[PATCH] ath10k: fix pm resume after suspend
Firmware was crashing when we were trying to warm reset it
after suspend. This was due to the fact that target registeres
can be accessed only if the hardware is awaken.
This patch makes sure to awake the device also on the hif up,
not only in case of probe call path.
Signed-off-by: Bartosz Markowski bartosz.markow...@tieto.com
---
drivers/net/wireless/ath/ath10k/pci.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c
b/drivers/net/wireless/ath/ath10k/pci.c
index 86f734e..4dee319 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1858,10 +1858,16 @@ static int ath10k_pci_hif_power_up_warm(struct ath10k
*ar)
static int ath10k_pci_hif_power_up(struct ath10k *ar)
{
- int ret;
+ int ret = 0;
ath10k_dbg(ar, ATH10K_DBG_BOOT, boot hif power up\n);
+ ret = ath10k_pci_wake(ar);
+ if (ret) {
+ ath10k_err(ar, failed to wake up target: %d\n, ret);
+ return ret;
+ }
+
/*
* Hardware CUS232 version 2 has some issues with cold reset and the
* preferred (and safer) way to perform a device reset is through a
@@ -1876,7 +1882,7 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
ret);
if (ath10k_pci_reset_mode == ATH10K_PCI_RESET_WARM_ONLY)
- return ret;
+ goto err_sleep;
ath10k_warn(ar, trying cold reset\n);
@@ -1884,11 +1890,15 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
if (ret) {
ath10k_err(ar, failed to power up target using cold
reset too (%d)\n,
ret);
- return ret;
+ goto err_sleep;
}
}
- return 0;
+ return ret;
+
+err_sleep:
+ ath10k_pci_sleep(ar);
+ return ret;
}
static void ath10k_pci_hif_power_down(struct ath10k *ar)
@@ -1896,6 +1906,7 @@ static void ath10k_pci_hif_power_down(struct ath10k *ar)
ath10k_dbg(ar, ATH10K_DBG_BOOT, boot hif power down\n);
ath10k_pci_warm_reset(ar);
+ ath10k_pci_sleep(ar);
}
#ifdef CONFIG_PM
@@ -2513,6 +2524,8 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
/* This shouldn't race as the device has been reset above. */
ath10k_pci_irq_disable(ar);
+ ath10k_pci_sleep(ar);
+
ret = ath10k_core_register(ar, chip_id);
if (ret) {
ath10k_err(ar, failed to register driver core: %d\n, ret);
@@ -2564,7 +2577,6 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
ath10k_pci_deinit_irq(ar);
ath10k_pci_ce_deinit(ar);
ath10k_pci_free_ce(ar);
- ath10k_pci_sleep(ar);
ath10k_pci_release(ar);
ath10k_core_destroy(ar);
}
--
1.8.2
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 1/2] ath10k: remove tsf argument from rx_desc tracing
Michal Kazior michal.kaz...@tieto.com writes: On 21 October 2014 15:54, Michal Kazior michal.kaz...@tieto.com wrote: Fundamtenally this was wrong. Tsf is only valid in first MPDU of a PPDU. This means tsf value was s/first MPDU/last MPDU/ I must've had a brain fart when writing the commit log yesterday. @Kalle: Should I re-spin or will you fix it before applying? No need to re-spin, I edited the commit log in ath-next-test. I also fixed a typo in fundamentally. commit d75153c577ddfffa9e7438e8225dfaf564f34552 Author: Michal Kazior michal.kaz...@tieto.com Date: Thu Oct 23 17:04:27 2014 +0300 ath10k: remove tsf argument from rx_desc tracing Fundamentally this was wrong. Tsf is only valid in last MPDU of a PPDU. This means tsf value was wrong most of the time during heavy traffic. Also I don't see much point in exposing a redundant (and broken) tsf value. Userspace can already read it from the dumped rx descriptor buffer. Cc: Rajkumar Manoharan rmano...@qti.qualcomm.com Signed-off-by: Michal Kazior michal.kaz...@tieto.com Signed-off-by: Kalle Valo kv...@qca.qualcomm.com -- Kalle Valo ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH 7/9] ath10k: add extra sanity check when popping amsdu
Michal Kazior michal.kaz...@tieto.com writes:
The netbuf pop can return NULL. Make sure to check
for that. It shouldn't happen but better safe than
sorry.
Signed-off-by: Michal Kazior michal.kaz...@tieto.com
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 7fa4d87..ddb9fe9 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -428,6 +428,15 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt
*htt,
while (msdu_chained--) {
struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
+ if (!next) {
+ ath10k_err(ar, failed to pop chained msdu\n);
ath10k_warn() is better here, right? I have folded this diff to the patch:
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -429,7 +429,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
if (!next) {
- ath10k_err(ar, failed to pop chained msdu\n);
+ ath10k_warn(ar, failed to pop chained msdu\n);
ath10k_htt_rx_free_msdu_chain(*head_msdu);
*head_msdu = NULL;
msdu = NULL;
Full commit:
https://github.com/kvalo/ath/commit/03f316720d664a79e874c13df1e86a64c5f2bf5f
--
Kalle Valo
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
