From 292bff9480c8d52fc58028979c4162abd83f1aec Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Mon, 14 Dec 2020 23:24:17 +0000 Subject: [PATCH 1/7] ath11k: add missing null check on allocated skb Currently the null check on a newly allocated skb is missing and this can lead to a null pointer dereference is the allocation fails. Fix this by adding a null check and returning -ENOMEM. Addresses-Coverity: ("Dereference null return") Fixes: 43ed15e1ee01 ("ath11k: put hw to DBS using WMI_PDEV_SET_HW_MODE_CMDID") Signed-off-by: Colin Ian King Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/20201214232417.84556-1-colin.king@canonical.com --- drivers/net/wireless/ath/ath11k/wmi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c index da4b546b62cb..73869d445c5b 100644 --- a/drivers/net/wireless/ath/ath11k/wmi.c +++ b/drivers/net/wireless/ath/ath11k/wmi.c @@ -3460,6 +3460,9 @@ int ath11k_wmi_set_hw_mode(struct ath11k_base *ab, len = sizeof(*cmd); skb = ath11k_wmi_alloc_skb(wmi_ab, len); + if (!skb) + return -ENOMEM; + cmd = (struct wmi_pdev_set_hw_mode_cmd_param *)skb->data; cmd->tlv_header = FIELD_PREP(WMI_TLV_TAG, WMI_TAG_PDEV_SET_HW_MODE_CMD) | From 3597010630d0aa96f5778901e691c6068bb86318 Mon Sep 17 00:00:00 2001 From: Carl Huang Date: Fri, 11 Dec 2020 00:56:13 -0500 Subject: [PATCH 2/7] ath11k: fix crash caused by NULL rx_channel During connect and disconnect stress test, crashed happened because ar->rx_channel is NULL. Fix it by checking whether ar->rx_channel is NULL. Crash stack is as below: RIP: 0010:ath11k_dp_rx_h_ppdu+0x110/0x230 [ath11k] [ 5028.808963] ath11k_dp_rx_wbm_err+0x14a/0x360 [ath11k] [ 5028.808970] ath11k_dp_rx_process_wbm_err+0x41c/0x520 [ath11k] [ 5028.808978] ath11k_dp_service_srng+0x25e/0x2d0 [ath11k] [ 5028.808982] ath11k_pci_ext_grp_napi_poll+0x23/0x80 [ath11k_pci] [ 5028.808986] net_rx_action+0x27e/0x400 [ 5028.808990] __do_softirq+0xfd/0x2bb [ 5028.808993] irq_exit+0xa6/0xb0 [ 5028.808995] do_IRQ+0x56/0xe0 [ 5028.808997] common_interrupt+0xf/0xf Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1 Signed-off-by: Carl Huang Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/20201211055613.9310-1-cjhuang@codeaurora.org --- drivers/net/wireless/ath/ath11k/dp_rx.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c index 205c0f1a40e9..920e5026a635 100644 --- a/drivers/net/wireless/ath/ath11k/dp_rx.c +++ b/drivers/net/wireless/ath/ath11k/dp_rx.c @@ -2294,6 +2294,7 @@ static void ath11k_dp_rx_h_ppdu(struct ath11k *ar, struct hal_rx_desc *rx_desc, { u8 channel_num; u32 center_freq; + struct ieee80211_channel *channel; rx_status->freq = 0; rx_status->rate_idx = 0; @@ -2314,9 +2315,12 @@ static void ath11k_dp_rx_h_ppdu(struct ath11k *ar, struct hal_rx_desc *rx_desc, rx_status->band = NL80211_BAND_5GHZ; } else { spin_lock_bh(&ar->data_lock); - rx_status->band = ar->rx_channel->band; - channel_num = - ieee80211_frequency_to_channel(ar->rx_channel->center_freq); + channel = ar->rx_channel; + if (channel) { + rx_status->band = channel->band; + channel_num = + ieee80211_frequency_to_channel(channel->center_freq); + } spin_unlock_bh(&ar->data_lock); ath11k_dbg_dump(ar->ab, ATH11K_DBG_DATA, NULL, "rx_desc: ", rx_desc, sizeof(struct hal_rx_desc)); From aa44b2f3ecd41f90b7e477158036648a49d21a32 Mon Sep 17 00:00:00 2001 From: Carl Huang Date: Fri, 11 Dec 2020 00:13:58 -0500 Subject: [PATCH 3/7] ath11k: start vdev if a bss peer is already created For QCA6390, bss peer must be created before vdev is to start. This change is to start vdev if a bss peer is created. Otherwise, ath11k delays to start vdev. This fixes an issue in a case where HT/VHT/HE settings change between authentication and association, e.g., due to the user space request to disable HT. Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1 Signed-off-by: Carl Huang Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/20201211051358.9191-1-cjhuang@codeaurora.org --- drivers/net/wireless/ath/ath11k/mac.c | 8 ++++++-- drivers/net/wireless/ath/ath11k/peer.c | 17 +++++++++++++++++ drivers/net/wireless/ath/ath11k/peer.h | 2 ++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index 5c175e3e09b2..c1608f64ea95 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -3021,6 +3021,7 @@ static int ath11k_mac_station_add(struct ath11k *ar, } if (ab->hw_params.vdev_start_delay && + !arvif->is_started && arvif->vdev_type != WMI_VDEV_TYPE_AP) { ret = ath11k_start_vdev_delay(ar->hw, vif); if (ret) { @@ -5284,7 +5285,8 @@ ath11k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw, /* for QCA6390 bss peer must be created before vdev_start */ if (ab->hw_params.vdev_start_delay && arvif->vdev_type != WMI_VDEV_TYPE_AP && - arvif->vdev_type != WMI_VDEV_TYPE_MONITOR) { + arvif->vdev_type != WMI_VDEV_TYPE_MONITOR && + !ath11k_peer_find_by_vdev_id(ab, arvif->vdev_id)) { memcpy(&arvif->chanctx, ctx, sizeof(*ctx)); ret = 0; goto out; @@ -5295,7 +5297,9 @@ ath11k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw, goto out; } - if (ab->hw_params.vdev_start_delay) { + if (ab->hw_params.vdev_start_delay && + (arvif->vdev_type == WMI_VDEV_TYPE_AP || + arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)) { param.vdev_id = arvif->vdev_id; param.peer_type = WMI_PEER_TYPE_DEFAULT; param.peer_addr = ar->mac_addr; diff --git a/drivers/net/wireless/ath/ath11k/peer.c b/drivers/net/wireless/ath/ath11k/peer.c index 1866d82678fa..b69e7ebfa930 100644 --- a/drivers/net/wireless/ath/ath11k/peer.c +++ b/drivers/net/wireless/ath/ath11k/peer.c @@ -76,6 +76,23 @@ struct ath11k_peer *ath11k_peer_find_by_id(struct ath11k_base *ab, return NULL; } +struct ath11k_peer *ath11k_peer_find_by_vdev_id(struct ath11k_base *ab, + int vdev_id) +{ + struct ath11k_peer *peer; + + spin_lock_bh(&ab->base_lock); + + list_for_each_entry(peer, &ab->peers, list) { + if (vdev_id == peer->vdev_id) { + spin_unlock_bh(&ab->base_lock); + return peer; + } + } + spin_unlock_bh(&ab->base_lock); + return NULL; +} + void ath11k_peer_unmap_event(struct ath11k_base *ab, u16 peer_id) { struct ath11k_peer *peer; diff --git a/drivers/net/wireless/ath/ath11k/peer.h b/drivers/net/wireless/ath/ath11k/peer.h index bba2e00b6944..8553ed061aea 100644 --- a/drivers/net/wireless/ath/ath11k/peer.h +++ b/drivers/net/wireless/ath/ath11k/peer.h @@ -43,5 +43,7 @@ int ath11k_peer_create(struct ath11k *ar, struct ath11k_vif *arvif, struct ieee80211_sta *sta, struct peer_create_params *param); int ath11k_wait_for_peer_delete_done(struct ath11k *ar, u32 vdev_id, const u8 *addr); +struct ath11k_peer *ath11k_peer_find_by_vdev_id(struct ath11k_base *ab, + int vdev_id); #endif /* _PEER_H_ */ From 9b09456258ea2f35fc8a99c4ac4829dcba0ca4be Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 16 Dec 2020 11:31:19 +0300 Subject: [PATCH 4/7] ath11k: Fix error code in ath11k_core_suspend() The "if (!ret)" condition is inverted and it should be "if (ret)". It means that we return success when we had intended to return an error code. This also caused a spurious warning even when the suspend was successful: [ 297.186612] ath11k_pci 0000:06:00.0: failed to suspend hif: 0 Fixes: d1b0c33850d2 ("ath11k: implement suspend for QCA6390 PCI devices") Signed-off-by: Dan Carpenter Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/X9nF17L2/EKOSbn/@mwanda --- drivers/net/wireless/ath/ath11k/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c index b97c38b9a270..350b7913622c 100644 --- a/drivers/net/wireless/ath/ath11k/core.c +++ b/drivers/net/wireless/ath/ath11k/core.c @@ -185,7 +185,7 @@ int ath11k_core_suspend(struct ath11k_base *ab) ath11k_hif_ce_irq_disable(ab); ret = ath11k_hif_suspend(ab); - if (!ret) { + if (ret) { ath11k_warn(ab, "failed to suspend hif: %d\n", ret); return ret; } From 30d085039314fcad2c2e33a2dfc8e79765ddf408 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 16 Dec 2020 11:32:12 +0300 Subject: [PATCH 5/7] ath11k: Fix ath11k_pci_fix_l1ss() All these conditions are reversed so presumably most of the function is dead code. This caused a spurious warning: [ 95.734922] ath11k_pci 0000:06:00.0: failed to set sysclk: 0 Fixes: 0699940755e9 ("ath11k: pci: fix L1ss clock unstable problem") Signed-off-by: Dan Carpenter Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/X9nGDHiTh+Z+asDy@mwanda --- drivers/net/wireless/ath/ath11k/pci.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c index 857647aa57c8..9f9a824a4c2d 100644 --- a/drivers/net/wireless/ath/ath11k/pci.c +++ b/drivers/net/wireless/ath/ath11k/pci.c @@ -274,7 +274,7 @@ static int ath11k_pci_fix_l1ss(struct ath11k_base *ab) PCIE_QSERDES_COM_SYSCLK_EN_SEL_REG, PCIE_QSERDES_COM_SYSCLK_EN_SEL_VAL, PCIE_QSERDES_COM_SYSCLK_EN_SEL_MSK); - if (!ret) { + if (ret) { ath11k_warn(ab, "failed to set sysclk: %d\n", ret); return ret; } @@ -283,7 +283,7 @@ static int ath11k_pci_fix_l1ss(struct ath11k_base *ab) PCIE_USB3_PCS_MISC_OSC_DTCT_CONFIG1_REG, PCIE_USB3_PCS_MISC_OSC_DTCT_CONFIG1_VAL, PCIE_USB3_PCS_MISC_OSC_DTCT_CONFIG_MSK); - if (!ret) { + if (ret) { ath11k_warn(ab, "failed to set dtct config1 error: %d\n", ret); return ret; } @@ -292,7 +292,7 @@ static int ath11k_pci_fix_l1ss(struct ath11k_base *ab) PCIE_USB3_PCS_MISC_OSC_DTCT_CONFIG2_REG, PCIE_USB3_PCS_MISC_OSC_DTCT_CONFIG2_VAL, PCIE_USB3_PCS_MISC_OSC_DTCT_CONFIG_MSK); - if (!ret) { + if (ret) { ath11k_warn(ab, "failed to set dtct config2: %d\n", ret); return ret; } @@ -301,7 +301,7 @@ static int ath11k_pci_fix_l1ss(struct ath11k_base *ab) PCIE_USB3_PCS_MISC_OSC_DTCT_CONFIG4_REG, PCIE_USB3_PCS_MISC_OSC_DTCT_CONFIG4_VAL, PCIE_USB3_PCS_MISC_OSC_DTCT_CONFIG_MSK); - if (!ret) { + if (ret) { ath11k_warn(ab, "failed to set dtct config4: %d\n", ret); return ret; } From f6f92968e1e5a7a9d211faaebefc26ebe408dad7 Mon Sep 17 00:00:00 2001 From: Carl Huang Date: Thu, 17 Dec 2020 09:04:57 +0200 Subject: [PATCH 6/7] ath11k: qmi: try to allocate a big block of DMA memory first Not all firmware versions support allocating DMA memory in smaller blocks so first try to allocate big block of DMA memory for QMI. If the allocation fails, let firmware request multiple blocks of DMA memory with smaller size. This also fixes an unnecessary error message seen during ath11k probe on QCA6390: ath11k_pci 0000:06:00.0: Respond mem req failed, result: 1, err: 0 ath11k_pci 0000:06:00.0: qmi failed to respond fw mem req:-22 Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1 Signed-off-by: Carl Huang Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/1608127593-15192-1-git-send-email-kvalo@codeaurora.org --- drivers/net/wireless/ath/ath11k/qmi.c | 24 ++++++++++++++++++++++-- drivers/net/wireless/ath/ath11k/qmi.h | 1 + 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c index f0b5c50974f3..0db623ff4bb9 100644 --- a/drivers/net/wireless/ath/ath11k/qmi.c +++ b/drivers/net/wireless/ath/ath11k/qmi.c @@ -1660,6 +1660,7 @@ static int ath11k_qmi_respond_fw_mem_request(struct ath11k_base *ab) struct qmi_wlanfw_respond_mem_resp_msg_v01 resp; struct qmi_txn txn = {}; int ret = 0, i; + bool delayed; req = kzalloc(sizeof(*req), GFP_KERNEL); if (!req) @@ -1672,11 +1673,13 @@ static int ath11k_qmi_respond_fw_mem_request(struct ath11k_base *ab) * failure to FW and FW will then request mulitple blocks of small * chunk size memory. */ - if (!ab->bus_params.fixed_mem_region && ab->qmi.mem_seg_count <= 2) { + if (!ab->bus_params.fixed_mem_region && ab->qmi.target_mem_delayed) { + delayed = true; ath11k_dbg(ab, ATH11K_DBG_QMI, "qmi delays mem_request %d\n", ab->qmi.mem_seg_count); memset(req, 0, sizeof(*req)); } else { + delayed = false; req->mem_seg_len = ab->qmi.mem_seg_count; for (i = 0; i < req->mem_seg_len ; i++) { @@ -1708,6 +1711,12 @@ static int ath11k_qmi_respond_fw_mem_request(struct ath11k_base *ab) } if (resp.resp.result != QMI_RESULT_SUCCESS_V01) { + /* the error response is expected when + * target_mem_delayed is true. + */ + if (delayed && resp.resp.error == 0) + goto out; + ath11k_warn(ab, "Respond mem req failed, result: %d, err: %d\n", resp.resp.result, resp.resp.error); ret = -EINVAL; @@ -1742,6 +1751,8 @@ static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab) int i; struct target_mem_chunk *chunk; + ab->qmi.target_mem_delayed = false; + for (i = 0; i < ab->qmi.mem_seg_count; i++) { chunk = &ab->qmi.target_mem[i]; chunk->vaddr = dma_alloc_coherent(ab->dev, @@ -1749,6 +1760,15 @@ static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab) &chunk->paddr, GFP_KERNEL); if (!chunk->vaddr) { + if (ab->qmi.mem_seg_count <= 2) { + ath11k_dbg(ab, ATH11K_DBG_QMI, + "qmi dma allocation failed (%d B type %u), will try later with small size\n", + chunk->size, + chunk->type); + ath11k_qmi_free_target_mem_chunk(ab); + ab->qmi.target_mem_delayed = true; + return 0; + } ath11k_err(ab, "failed to alloc memory, size: 0x%x, type: %u\n", chunk->size, chunk->type); @@ -2517,7 +2537,7 @@ static void ath11k_qmi_msg_mem_request_cb(struct qmi_handle *qmi_hdl, ret); return; } - } else if (msg->mem_seg_len > 2) { + } else { ret = ath11k_qmi_alloc_target_mem_chunk(ab); if (ret) { ath11k_warn(ab, "qmi failed to alloc target memory: %d\n", diff --git a/drivers/net/wireless/ath/ath11k/qmi.h b/drivers/net/wireless/ath/ath11k/qmi.h index 92925c9eac67..7bad374cc23a 100644 --- a/drivers/net/wireless/ath/ath11k/qmi.h +++ b/drivers/net/wireless/ath/ath11k/qmi.h @@ -125,6 +125,7 @@ struct ath11k_qmi { struct target_mem_chunk target_mem[ATH11K_QMI_WLANFW_MAX_NUM_MEM_SEG_V01]; u32 mem_seg_count; u32 target_mem_mode; + bool target_mem_delayed; u8 cal_done; struct target_info target; struct m3_mem_region m3_mem; From e9603f4bdcc04417f1c7b3585e63654819dc11f6 Mon Sep 17 00:00:00 2001 From: Carl Huang Date: Thu, 17 Dec 2020 17:22:10 +0200 Subject: [PATCH 7/7] ath11k: pci: disable ASPM L0sLs before downloading firmware Sometimes QCA6390 doesn't switch to amss state as device enters L1ss state, so disable L0sL1s during firmware downloading. Driver recovers the ASPM to default value in start callback or powerdown callback. Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1 Signed-off-by: Carl Huang Signed-off-by: Kalle Valo Link: https://lore.kernel.org/r/1608218530-15426-1-git-send-email-kvalo@codeaurora.org --- drivers/net/wireless/ath/ath11k/pci.c | 36 +++++++++++++++++++++++++++ drivers/net/wireless/ath/ath11k/pci.h | 2 ++ 2 files changed, 38 insertions(+) diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c index 9f9a824a4c2d..20b415cd96c4 100644 --- a/drivers/net/wireless/ath/ath11k/pci.c +++ b/drivers/net/wireless/ath/ath11k/pci.c @@ -886,6 +886,32 @@ static void ath11k_pci_free_region(struct ath11k_pci *ab_pci) pci_disable_device(pci_dev); } +static void ath11k_pci_aspm_disable(struct ath11k_pci *ab_pci) +{ + struct ath11k_base *ab = ab_pci->ab; + + pcie_capability_read_word(ab_pci->pdev, PCI_EXP_LNKCTL, + &ab_pci->link_ctl); + + ath11k_dbg(ab, ATH11K_DBG_PCI, "pci link_ctl 0x%04x L0s %d L1 %d\n", + ab_pci->link_ctl, + u16_get_bits(ab_pci->link_ctl, PCI_EXP_LNKCTL_ASPM_L0S), + u16_get_bits(ab_pci->link_ctl, PCI_EXP_LNKCTL_ASPM_L1)); + + /* disable L0s and L1 */ + pcie_capability_write_word(ab_pci->pdev, PCI_EXP_LNKCTL, + ab_pci->link_ctl & ~PCI_EXP_LNKCTL_ASPMC); + + set_bit(ATH11K_PCI_ASPM_RESTORE, &ab_pci->flags); +} + +static void ath11k_pci_aspm_restore(struct ath11k_pci *ab_pci) +{ + if (test_and_clear_bit(ATH11K_PCI_ASPM_RESTORE, &ab_pci->flags)) + pcie_capability_write_word(ab_pci->pdev, PCI_EXP_LNKCTL, + ab_pci->link_ctl); +} + static int ath11k_pci_power_up(struct ath11k_base *ab) { struct ath11k_pci *ab_pci = ath11k_pci_priv(ab); @@ -895,6 +921,11 @@ static int ath11k_pci_power_up(struct ath11k_base *ab) clear_bit(ATH11K_PCI_FLAG_INIT_DONE, &ab_pci->flags); ath11k_pci_sw_reset(ab_pci->ab, true); + /* Disable ASPM during firmware download due to problems switching + * to AMSS state. + */ + ath11k_pci_aspm_disable(ab_pci); + ret = ath11k_mhi_start(ab_pci); if (ret) { ath11k_err(ab, "failed to start mhi: %d\n", ret); @@ -908,6 +939,9 @@ static void ath11k_pci_power_down(struct ath11k_base *ab) { struct ath11k_pci *ab_pci = ath11k_pci_priv(ab); + /* restore aspm in case firmware bootup fails */ + ath11k_pci_aspm_restore(ab_pci); + ath11k_pci_force_wake(ab_pci->ab); ath11k_mhi_stop(ab_pci); clear_bit(ATH11K_PCI_FLAG_INIT_DONE, &ab_pci->flags); @@ -965,6 +999,8 @@ static int ath11k_pci_start(struct ath11k_base *ab) set_bit(ATH11K_PCI_FLAG_INIT_DONE, &ab_pci->flags); + ath11k_pci_aspm_restore(ab_pci); + ath11k_pci_ce_irqs_enable(ab); ath11k_ce_rx_post_buf(ab); diff --git a/drivers/net/wireless/ath/ath11k/pci.h b/drivers/net/wireless/ath/ath11k/pci.h index 0432a702416b..fe44d0dfce19 100644 --- a/drivers/net/wireless/ath/ath11k/pci.h +++ b/drivers/net/wireless/ath/ath11k/pci.h @@ -63,6 +63,7 @@ struct ath11k_msi_config { enum ath11k_pci_flags { ATH11K_PCI_FLAG_INIT_DONE, ATH11K_PCI_FLAG_IS_MSI_64, + ATH11K_PCI_ASPM_RESTORE, }; struct ath11k_pci { @@ -80,6 +81,7 @@ struct ath11k_pci { /* enum ath11k_pci_flags */ unsigned long flags; + u16 link_ctl; }; static inline struct ath11k_pci *ath11k_pci_priv(struct ath11k_base *ab)