[PATCH] ath10k: pci: use mutex for diagnostic window CE polling
The DIAG copy engine is only used via polling, but it holds a spinlock
with softirqs disabled. Each iteration of our read/write loops can
theoretically take 20ms (two 10ms timeout loops), and this loop can be
run an unbounded number of times while holding the spinlock -- dependent
on the request size given by the caller.
As of commit 39501ea64116 ("ath10k: download firmware via diag Copy
Engine for QCA6174 and QCA9377."), we transfer large chunks of firmware
memory using this mechanism. With large enough firmware segments, this
becomes an exceedingly long period for disabling soft IRQs. For example,
with a 500KiB firmware segment, in testing QCA6174A, I see 200 loop
iterations of about 50-100us each, which can total about 10-20ms.
In reality, we don't really need to block softirqs for this duration.
The DIAG CE is only used in polling mode, and we only need to hold
ce_lock to make sure any CE bookkeeping is done without screwing up
another CE. Otherwise, we only need to ensure exclusion between
ath10k_pci_diag_{read,write}_mem() contexts.
This patch moves to use fine-grained locking for the shared ce_lock,
while adding a new mutex just to ensure mutual exclusion of diag
read/write operations.
Tested on QCA6174A, firmware version WLAN.RM.4.4.1-00132-QCARMSWPZ-1.
Fixes: 39501ea64116 ("ath10k: download firmware via diag Copy Engine for
QCA6174 and QCA9377.")
Signed-off-by: Brian Norris
---
I'm not quite sure if this is -stable material. It's technically an
existing bug, but it looks like previously, the DIAG interface was only
exposed via debugfs -- you could block softirqs by playing with
/sys/kernel/debug/ieee80211/phyX/ath10k/mem_value. The new usage (for
loading firmware, on QCA6174 and QCA9377) might constitute a significant
regression.
Also, I'd appreciate some review from Qualcomm folks as always.
---
drivers/net/wireless/ath/ath10k/pci.c | 41 ++-
drivers/net/wireless/ath/ath10k/pci.h | 3 ++
2 files changed, 18 insertions(+), 26 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c
b/drivers/net/wireless/ath/ath10k/pci.c
index 39e0b1cc2a12..f8356b3bf150 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -913,7 +913,6 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32
address, void *data,
int nbytes)
{
struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
- struct ath10k_ce *ce = ath10k_ce_priv(ar);
int ret = 0;
u32 *buf;
unsigned int completed_nbytes, alloc_nbytes, remaining_bytes;
@@ -924,8 +923,7 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32
address, void *data,
void *data_buf = NULL;
int i;
- spin_lock_bh(>ce_lock);
-
+ mutex_lock(_pci->ce_diag_mutex);
ce_diag = ar_pci->ce_diag;
/*
@@ -960,19 +958,17 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar,
u32 address, void *data,
nbytes = min_t(unsigned int, remaining_bytes,
DIAG_TRANSFER_LIMIT);
- ret = ce_diag->ops->ce_rx_post_buf(ce_diag, _data, ce_data);
+ ret = ath10k_ce_rx_post_buf(ce_diag, _data, ce_data);
if (ret != 0)
goto done;
/* Request CE to send from Target(!) address to Host buffer */
- ret = ath10k_ce_send_nolock(ce_diag, NULL, (u32)address,
nbytes, 0,
- 0);
+ ret = ath10k_ce_send(ce_diag, NULL, (u32)address, nbytes, 0, 0);
if (ret)
goto done;
i = 0;
- while (ath10k_ce_completed_send_next_nolock(ce_diag,
- NULL) != 0) {
+ while (ath10k_ce_completed_send_next(ce_diag, NULL) != 0) {
udelay(DIAG_ACCESS_CE_WAIT_US);
i += DIAG_ACCESS_CE_WAIT_US;
@@ -983,10 +979,8 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32
address, void *data,
}
i = 0;
- while (ath10k_ce_completed_recv_next_nolock(ce_diag,
- (void **),
- _nbytes)
- != 0) {
+ while (ath10k_ce_completed_recv_next(ce_diag, (void **),
+_nbytes) != 0) {
udelay(DIAG_ACCESS_CE_WAIT_US);
i += DIAG_ACCESS_CE_WAIT_US;
@@ -1019,7 +1013,7 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar,
u32 address, void *data,
dma_free_coherent(ar->dev, alloc_nbytes, data_buf,
ce_data_base);
- spin_unlock_bh(>ce_lock);
+ mutex_unlock(_pci->ce_diag_mutex);
[PATCH v3 3/3] ath10k: Request credit report if flow control enabled on ep
FW credit flow control is enabled for only WMI ctrl service(CE3) but credit update is requested unconditionally on all HTC services as part of HTC tx in CE3/CE0/CE4. This is causing WOW failure as FW is not expecting credit report request on other end-points(CE0/CE4). Request credit report only on those endpoints where credit flow control is enabled. Testing: Tested on WCN3990 HW. Tested FW: WLAN.HL.2.0-01192-QCAHLSWMTPLZ-1. Signed-off-by: Govind Singh --- drivers/net/wireless/ath/ath10k/htc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c index 7654a21323ce..5aa57a01c033 100644 --- a/drivers/net/wireless/ath/ath10k/htc.c +++ b/drivers/net/wireless/ath/ath10k/htc.c @@ -88,7 +88,8 @@ static void ath10k_htc_prepare_tx_skb(struct ath10k_htc_ep *ep, hdr->eid = ep->eid; hdr->len = __cpu_to_le16(skb->len - sizeof(*hdr)); hdr->flags = 0; - hdr->flags |= ATH10K_HTC_FLAG_NEED_CREDIT_UPDATE; + if (ep->tx_credit_flow_enabled) + hdr->flags |= ATH10K_HTC_FLAG_NEED_CREDIT_UPDATE; spin_lock_bh(>htc->tx_lock); hdr->seq_no = ep->seq_no++; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
[PATCH v3 0/3] Add support for suspend/resume and WOW for WCN3990 chipset
This series adds support for driver suspend/resume and wake over wlan support for WCN3990 chipset. CE2 is configured as wakeup source before driver suspend and FW can wake up application processor on Magic packet/Deauth/AP lost. Changes since v2: * Removed BUS TYPE check and introduced bus param at struct ath10k level for scalability. Changes since v1: * dropped ath10k: configure the vdev listen interval before wow suspend patch from the series. This can be send later after refactoring/tuning. * updated commit text in ath10k: Disable interface pause wow config for integrated chipset patch. Govind Singh (3): ath10k: Enable bus layer suspend/resume for WCN3990 ath10k: Disable interface pause wow config for integrated chipset ath10k: Request credit report if flow control enabled on ep drivers/net/wireless/ath/ath10k/core.c | 5 ++- drivers/net/wireless/ath/ath10k/core.h | 2 + drivers/net/wireless/ath/ath10k/coredump.c | 2 +- drivers/net/wireless/ath/ath10k/debug.c| 4 +- drivers/net/wireless/ath/ath10k/htc.c | 9 +++-- drivers/net/wireless/ath/ath10k/htt_rx.c | 10 ++--- drivers/net/wireless/ath/ath10k/htt_tx.c | 6 +-- drivers/net/wireless/ath/ath10k/pci.c | 3 +- drivers/net/wireless/ath/ath10k/snoc.c | 45 ++ drivers/net/wireless/ath/ath10k/txrx.c | 2 +- drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 + drivers/net/wireless/ath/ath10k/wmi-tlv.h | 7 12 files changed, 78 insertions(+), 19 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
[PATCH v3 2/3] ath10k: Disable interface pause wow config for integrated chipset
wow pause iface config controls the PCI D0/D3-WOW cases for pcie
bus state. Firmware does not expects WOW_IFACE_PAUSE_ENABLED config
for bus/link that cannot be suspended ex:snoc and does not trigger
common subsystem shutdown.
Disable interface pause wow config for integrated chipset(WCN3990)
for correct WOW configuration in the firmware.
Testing:
Tested on WCN3990 HW.
Tested FW: WLAN.HL.2.0-01192-QCAHLSWMTPLZ-1.
Signed-off-by: Govind Singh
---
drivers/net/wireless/ath/ath10k/core.c | 5 +++--
drivers/net/wireless/ath/ath10k/core.h | 2 ++
drivers/net/wireless/ath/ath10k/coredump.c | 2 +-
drivers/net/wireless/ath/ath10k/debug.c| 4 ++--
drivers/net/wireless/ath/ath10k/htc.c | 6 +++---
drivers/net/wireless/ath/ath10k/htt_rx.c | 10 +-
drivers/net/wireless/ath/ath10k/htt_tx.c | 6 +++---
drivers/net/wireless/ath/ath10k/pci.c | 3 ++-
drivers/net/wireless/ath/ath10k/txrx.c | 2 +-
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 ++
drivers/net/wireless/ath/ath10k/wmi-tlv.h | 7 +++
11 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 2bdb632b7b1a..01ad53f3c569 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2955,8 +2955,9 @@ static void ath10k_core_register_work(struct work_struct
*work)
int ath10k_core_register(struct ath10k *ar,
const struct ath10k_bus_params *bus_params)
{
- ar->chip_id = bus_params->chip_id;
- ar->dev_type = bus_params->dev_type;
+ ar->bus_param.chip_id = bus_params->chip_id;
+ ar->bus_param.dev_type = bus_params->dev_type;
+ ar->bus_param.link_can_suspend = bus_params->link_can_suspend;
queue_work(ar->workqueue, >register_work);
return 0;
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index d14a4f928218..2f43326decf6 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -921,6 +921,7 @@ enum ath10k_dev_type {
struct ath10k_bus_params {
u32 chip_id;
enum ath10k_dev_type dev_type;
+ bool link_can_suspend;
};
struct ath10k {
@@ -1189,6 +1190,7 @@ struct ath10k {
enum ath10k_radar_confirmation_state radar_conf_state;
struct ath10k_radar_found_info last_radar_info;
struct work_struct radar_confirmation_work;
+ struct ath10k_bus_params bus_param;
/* must be last */
u8 drv_priv[0] __aligned(sizeof(void *));
diff --git a/drivers/net/wireless/ath/ath10k/coredump.c
b/drivers/net/wireless/ath/ath10k/coredump.c
index eadae2f9206b..bf84e999466f 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.c
+++ b/drivers/net/wireless/ath/ath10k/coredump.c
@@ -1167,7 +1167,7 @@ static struct ath10k_dump_file_data
*ath10k_coredump_build(struct ath10k *ar)
dump_data->version = cpu_to_le32(ATH10K_FW_CRASH_DUMP_VERSION);
guid_copy(_data->guid, _data->guid);
- dump_data->chip_id = cpu_to_le32(ar->chip_id);
+ dump_data->chip_id = cpu_to_le32(ar->bus_param.chip_id);
dump_data->bus_type = cpu_to_le32(0);
dump_data->target_version = cpu_to_le32(ar->target_version);
dump_data->fw_version_major = cpu_to_le32(ar->fw_version_major);
diff --git a/drivers/net/wireless/ath/ath10k/debug.c
b/drivers/net/wireless/ath/ath10k/debug.c
index 02988fc378a1..9f2534d6b56f 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -58,7 +58,7 @@ void ath10k_debug_print_hwfw_info(struct ath10k *ar)
ath10k_info(ar, "%s target 0x%08x chip_id 0x%08x sub %04x:%04x",
ar->hw_params.name,
ar->target_version,
- ar->chip_id,
+ ar->bus_param.chip_id,
ar->id.subsystem_vendor, ar->id.subsystem_device);
ath10k_info(ar, "kconfig debug %d debugfs %d tracing %d dfs %d testmode
%d\n",
@@ -625,7 +625,7 @@ static ssize_t ath10k_read_chip_id(struct file *file, char
__user *user_buf,
size_t len;
char buf[50];
- len = scnprintf(buf, sizeof(buf), "0x%08x\n", ar->chip_id);
+ len = scnprintf(buf, sizeof(buf), "0x%08x\n", ar->bus_param.chip_id);
return simple_read_from_buffer(user_buf, count, ppos, buf, len);
}
diff --git a/drivers/net/wireless/ath/ath10k/htc.c
b/drivers/net/wireless/ath/ath10k/htc.c
index 28daed5981a1..7654a21323ce 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -53,7 +53,7 @@ static inline void ath10k_htc_restore_tx_skb(struct
ath10k_htc *htc,
{
struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
- if (htc->ar->dev_type != ATH10K_DEV_TYPE_HL)
+ if (htc->ar->bus_param.dev_type != ATH10K_DEV_TYPE_HL)
dma_unmap_single(htc->ar->dev, skb_cb->paddr,
[PATCH v3 1/3] ath10k: Enable bus layer suspend/resume for WCN3990
Register snoc bus layer suspend/resume PM ops and configure
the wakeup source(CE2) for the device.
Testing:
Tested on WCN3990 HW.
Tested FW: WLAN.HL.2.0-01192-QCAHLSWMTPLZ-1.
Signed-off-by: Govind Singh
---
drivers/net/wireless/ath/ath10k/snoc.c | 45 ++
1 file changed, 45 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index dad7e1ce79c0..60e0f46cfdd9 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -30,6 +30,7 @@
#define ATH10K_SNOC_RX_POST_RETRY_MS 50
#define CE_POLL_PIPE 4
+#define ATH10K_SNOC_WAKE_IRQ 2
static char *const ce_name[] = {
"WLAN_CE_0",
@@ -1041,6 +1042,46 @@ static int ath10k_snoc_hif_power_up(struct ath10k *ar)
return ret;
}
+#ifdef CONFIG_PM
+static int ath10k_snoc_hif_suspend(struct ath10k *ar)
+{
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ int ret;
+
+ if (!device_may_wakeup(ar->dev))
+ return -EPERM;
+
+ ret = enable_irq_wake(ar_snoc->ce_irqs[ATH10K_SNOC_WAKE_IRQ].irq_line);
+ if (ret) {
+ ath10k_err(ar, "failed to enable wakeup irq :%d\n", ret);
+ return ret;
+ }
+
+ ath10k_dbg(ar, ATH10K_DBG_SNOC, "snoc device suspended\n");
+
+ return ret;
+}
+
+static int ath10k_snoc_hif_resume(struct ath10k *ar)
+{
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ int ret;
+
+ if (!device_may_wakeup(ar->dev))
+ return -EPERM;
+
+ ret = disable_irq_wake(ar_snoc->ce_irqs[ATH10K_SNOC_WAKE_IRQ].irq_line);
+ if (ret) {
+ ath10k_err(ar, "failed to disable wakeup irq: %d\n", ret);
+ return ret;
+ }
+
+ ath10k_dbg(ar, ATH10K_DBG_SNOC, "snoc device resumed\n");
+
+ return ret;
+}
+#endif
+
static const struct ath10k_hif_ops ath10k_snoc_hif_ops = {
.read32 = ath10k_snoc_read32,
.write32= ath10k_snoc_write32,
@@ -1054,6 +1095,10 @@ static const struct ath10k_hif_ops ath10k_snoc_hif_ops =
{
.send_complete_check= ath10k_snoc_hif_send_complete_check,
.get_free_queue_number = ath10k_snoc_hif_get_free_queue_number,
.get_target_info= ath10k_snoc_hif_get_target_info,
+#ifdef CONFIG_PM
+ .suspend= ath10k_snoc_hif_suspend,
+ .resume = ath10k_snoc_hif_resume,
+#endif
};
static const struct ath10k_bus_ops ath10k_snoc_bus_ops = {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
