2017-09-17 19:40:25

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 00/11] ath10k high latency

This is the third version of the high latency patches (stuff common for
usb and sdio).

One major difference between this version and the previous is that the
num_pending_tx counter has been disabled for high latency devices (last patch).

This fixes the previous issue with the halted USB RX.
I have tested these patches with a Linksys WUSB6100M and it looks
much better than the previous version.

High latency devices does not seem to send HTT_T2H_MSG_TYPE_TX_COMPL_IND's
to the host for outgoing packets, resulting in a transmit stop when the
upper limit of num_pending_tx is reached.

The qcacld driver mentions that the TX_COMPL_IND can be disabled for HL
devices in order to achieve better throughput:

/*
* HTT option TLV for specifying whether HL systems should indicate
* over-the-air tx completion for individual frames, or should instead
* send a bulk TX_CREDIT_UPDATE_IND except when the host explicitly
* requests an OTA tx completion for a particular tx frame.
* This option does not apply to LL systems, where the TX_COMPL_IND
* is mandatory.
* This option is primarily intended for HL systems in which the tx frame
* downloads over the host --> target bus are as slow as or slower than
* the transmissions over the WLAN PHY. For cases where the bus is faster
* than the WLAN PHY, the target will transmit relatively large A-MPDUs,
* and consquently will send one TX_COMPL_IND message that covers several
* tx frames. For cases where the WLAN PHY is faster than the bus,
* the target will end up transmitting very short A-MPDUs, and consequently
* sending many TX_COMPL_IND messages, which each cover a very small number
* of tx frames.
* The HL_SUPPRESS_TX_COMPL_IND TLV can be sent by the host to the target as
* a suffix to the VERSION_REQ message to request whether the host desires to
* use TX_CREDIT_UPDATE_IND rather than TX_COMPL_IND. The target can then
* send a HTT_SUPPRESS_TX_COMPL_IND TLV to the host as a suffix to the
* VERSION_CONF message to confirm whether TX_CREDIT_UPDATE_IND will be used
* rather than TX_COMPL_IND. TX_CREDIT_UPDATE_IND shall only be used if the
* host sends a HL_SUPPRESS_TX_COMPL_IND TLV requesting use of
* TX_CREDIT_UPDATE_IND, and the target sends a HL_SUPPRESS_TX_COMPLE_IND TLV
* back to the host confirming use of TX_CREDIT_UPDATE_IND.
* Lack of a HL_SUPPRESS_TX_COMPL_IND TLV from either host --> target or
* target --> host is equivalent to a HL_SUPPRESS_TX_COMPL_IND that
* explicitly specifies HL_ALLOW_TX_COMPL_IND in the value payload of the
* TLV.
*/

I am suspecting this is the default behavior in the firmware.

I have tried a simple wget test where I download a Linux kernel tar ball
from kernel.org. I got the below results:

wget --no-check-certificate \
https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.11.4.tar.xz

2017-09-17 16:46:20 (13.3 MB/s) - 'linux-4.11.4.tar.xz' saved
[95549228/95549228]

13.3 MB/s is a fairly OK result since I have a 100 Mbit/s internet
connection.

Changes since v2:

- Disabled htt num_pending_tx counter for HL.
- Fixed transmit flags for HL HTT TX.
- Proper marking of aggregated frames in the HL HTT RX handler.
- A few other minor fixes

Erik Stromdahl (11):
ath10k: high_latency detection
ath10k: htt: RX ring config HL support
ath10k: per target configurablity of various items
ath10k: add start_once support
ath10k: htt: High latency TX support
ath10k: htt: High latency RX support
ath10k: various fixes for high latency devices
ath10k: add QCA9377 usb hw_param item
ath10k: add QCA9377 sdio hw_param item
ath10k: wmi: disable softirq's while calling ieee80211_rx
ath10k: remove htt pending TX count for high latency

drivers/net/wireless/ath/ath10k/core.c | 128 +++++++++++++++++++-----
drivers/net/wireless/ath/ath10k/core.h | 16 +--
drivers/net/wireless/ath/ath10k/htc.c | 19 ++--
drivers/net/wireless/ath/ath10k/htt.c | 5 +-
drivers/net/wireless/ath/ath10k/htt.h | 57 ++++++++++-
drivers/net/wireless/ath/ath10k/htt_rx.c | 122 ++++++++++++++++++++++-
drivers/net/wireless/ath/ath10k/htt_tx.c | 157 +++++++++++++++++++++++++++++-
drivers/net/wireless/ath/ath10k/hw.h | 30 ++++++
drivers/net/wireless/ath/ath10k/mac.c | 5 +-
drivers/net/wireless/ath/ath10k/rx_desc.h | 15 +++
drivers/net/wireless/ath/ath10k/txrx.c | 5 +-
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 4 +-
drivers/net/wireless/ath/ath10k/wmi.c | 3 +
13 files changed, 513 insertions(+), 53 deletions(-)

--
2.14.1


2017-09-17 19:40:30

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 06/11] ath10k: htt: High latency RX support

Special HTT RX handling for high latency interfaces.

Since no DMA physical addresses are used in the RX ring
config message (this is not supported by the high latency
devices), no RX ring is allocated.
All RX skb's are allocated by the driver and passed directly
to mac80211 in the HTT RX indication handler.

A nice side effect of this is that no huge buffer will be
allocated with dma_alloc_coherent. On embedded systems with
limited memory resources, the allocation of the RX ring is
prone to fail.

Some tweaks made to "make it work":

Removal of protected bit in 802.11 header frame control field.
The chipset seems to do hw decryption but the frame_control
protected bit is still set.

This is necessary for mac80211 not to drop the frame.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 27 ++++---
drivers/net/wireless/ath/ath10k/htt.h | 47 ++++++++++++
drivers/net/wireless/ath/ath10k/htt_rx.c | 119 +++++++++++++++++++++++++++++-
drivers/net/wireless/ath/ath10k/rx_desc.h | 15 ++++
4 files changed, 195 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index c21227a74996..1880570989ae 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2083,10 +2083,12 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
ar->htt.rx_ring.in_ord_rx = !!(test_bit(WMI_SERVICE_RX_FULL_REORDER,
ar->wmi.svc_map));

- status = ath10k_htt_rx_alloc(&ar->htt);
- if (status) {
- ath10k_err(ar, "failed to alloc htt rx: %d\n", status);
- goto err_htt_tx_detach;
+ if (!ar->is_high_latency) {
+ status = ath10k_htt_rx_alloc(&ar->htt);
+ if (status) {
+ ath10k_err(ar, "failed to alloc htt rx: %d\n", status);
+ goto err_htt_tx_detach;
+ }
}

status = ath10k_hif_start(ar);
@@ -2203,10 +2205,13 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
}
}

- status = ath10k_htt_rx_ring_refill(ar);
- if (status) {
- ath10k_err(ar, "failed to refill htt rx ring: %d\n", status);
- goto err_hif_stop;
+ if (!ar->is_high_latency) {
+ status = ath10k_htt_rx_ring_refill(ar);
+ if (status) {
+ ath10k_err(ar, "failed to refill htt rx ring: %d\n",
+ status);
+ goto err_hif_stop;
+ }
}

if (ar->max_num_vdevs >= 64)
@@ -2235,7 +2240,8 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
err_hif_stop:
ath10k_hif_stop(ar);
err_htt_rx_detach:
- ath10k_htt_rx_free(&ar->htt);
+ if (!ar->is_high_latency)
+ ath10k_htt_rx_free(&ar->htt);
err_htt_tx_detach:
ath10k_htt_tx_free(&ar->htt);
err_wmi_detach:
@@ -2280,7 +2286,8 @@ void ath10k_core_stop(struct ath10k *ar)

ath10k_hif_stop(ar);
ath10k_htt_tx_stop(&ar->htt);
- ath10k_htt_rx_free(&ar->htt);
+ if (!ar->is_high_latency)
+ ath10k_htt_rx_free(&ar->htt);
ath10k_wmi_detach(ar);
ar->is_started = false;
}
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index bac453f5753e..ac5603ef4ba5 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -646,6 +646,15 @@ struct htt_rx_indication {
struct htt_rx_indication_mpdu_range mpdu_ranges[0];
} __packed;

+/* High latency version of the RX indication */
+struct htt_rx_indication_hl {
+ struct htt_rx_indication_hdr hdr;
+ struct htt_rx_indication_ppdu ppdu;
+ struct htt_rx_indication_prefix prefix;
+ struct fw_rx_desc_hl fw_desc;
+ struct htt_rx_indication_mpdu_range mpdu_ranges[0];
+} __packed;
+
static inline struct htt_rx_indication_mpdu_range *
htt_rx_ind_get_mpdu_ranges(struct htt_rx_indication *rx_ind)
{
@@ -658,6 +667,18 @@ static inline struct htt_rx_indication_mpdu_range *
return ptr;
}

+static inline struct htt_rx_indication_mpdu_range *
+ htt_rx_ind_get_mpdu_ranges_hl(struct htt_rx_indication_hl *rx_ind)
+{
+ void *ptr = rx_ind;
+
+ ptr += sizeof(rx_ind->hdr)
+ + sizeof(rx_ind->ppdu)
+ + sizeof(rx_ind->prefix)
+ + sizeof(rx_ind->fw_desc);
+ return ptr;
+}
+
enum htt_rx_flush_mpdu_status {
HTT_RX_FLUSH_MPDU_DISCARD = 0,
HTT_RX_FLUSH_MPDU_REORDER = 1,
@@ -1530,6 +1551,7 @@ struct htt_resp {
struct htt_mgmt_tx_completion mgmt_tx_completion;
struct htt_data_tx_completion data_tx_completion;
struct htt_rx_indication rx_ind;
+ struct htt_rx_indication_hl rx_ind_hl;
struct htt_rx_fragment_indication rx_frag_ind;
struct htt_rx_peer_map peer_map;
struct htt_rx_peer_unmap peer_unmap;
@@ -1754,6 +1776,31 @@ struct htt_rx_desc {
u8 msdu_payload[0];
};

+#define HTT_RX_DESC_HL_INFO_SEQ_NUM_MASK 0x00000fff
+#define HTT_RX_DESC_HL_INFO_SEQ_NUM_LSB 0
+#define HTT_RX_DESC_HL_INFO_ENCRYPTED_MASK 0x00001000
+#define HTT_RX_DESC_HL_INFO_ENCRYPTED_LSB 12
+#define HTT_RX_DESC_HL_INFO_CHAN_INFO_PRESENT_MASK 0x00002000
+#define HTT_RX_DESC_HL_INFO_CHAN_INFO_PRESENT_LSB 13
+#define HTT_RX_DESC_HL_INFO_MCAST_BCAST_MASK 0x00008000
+#define HTT_RX_DESC_HL_INFO_MCAST_BCAST_LSB 15
+#define HTT_RX_DESC_HL_INFO_FRAGMENT_MASK 0x00010000
+#define HTT_RX_DESC_HL_INFO_FRAGMENT_LSB 16
+#define HTT_RX_DESC_HL_INFO_KEY_ID_OCT_MASK 0x01fe0000
+#define HTT_RX_DESC_HL_INFO_KEY_ID_OCT_LSB 17
+
+struct htt_rx_desc_base_hl {
+ __le32 info; /* HTT_RX_DESC_HL_INFO_ */
+};
+
+struct htt_rx_chan_info {
+ __le16 primary_chan_center_freq_mhz;
+ __le16 contig_chan1_center_freq_mhz;
+ __le16 contig_chan2_center_freq_mhz;
+ u8 phy_mode;
+ u8 reserved;
+} __packed;
+
#define HTT_RX_DESC_ALIGN 8

#define HTT_MAC_ADDR_LEN 6
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index a3f5dc78353f..7461555ccad5 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1590,8 +1590,116 @@ static int ath10k_htt_rx_handle_amsdu(struct ath10k_htt *htt)
return num_msdus;
}

-static void ath10k_htt_rx_proc_rx_ind(struct ath10k_htt *htt,
- struct htt_rx_indication *rx)
+static bool ath10k_htt_rx_proc_rx_ind_hl(struct ath10k_htt *htt,
+ struct htt_rx_indication_hl *rx,
+ struct sk_buff *skb)
+{
+ struct ath10k *ar = htt->ar;
+ struct ath10k_peer *peer;
+ struct htt_rx_indication_mpdu_range *mpdu_ranges;
+ struct fw_rx_desc_hl *fw_desc;
+ struct ieee80211_hdr *hdr;
+ struct ieee80211_rx_status *rx_status;
+ u16 peer_id;
+ u8 rx_desc_len;
+ int num_mpdu_ranges;
+ size_t tot_hdr_len;
+ struct ieee80211_channel *ch;
+
+ peer_id = __le16_to_cpu(rx->hdr.peer_id);
+
+ peer = ath10k_peer_find_by_id(ar, peer_id);
+ if (!peer)
+ ath10k_warn(ar, "Got RX ind from invalid peer: %u\n", peer_id);
+
+ num_mpdu_ranges = MS(__le32_to_cpu(rx->hdr.info1),
+ HTT_RX_INDICATION_INFO1_NUM_MPDU_RANGES);
+ mpdu_ranges = htt_rx_ind_get_mpdu_ranges_hl(rx);
+ fw_desc = &rx->fw_desc;
+ rx_desc_len = fw_desc->len;
+
+ /* I have not yet seen any case where num_mpdu_ranges > 1.
+ * qcacld does not seem handle that case either, so we introduce the
+ * same limitiation here as well.
+ */
+ if (num_mpdu_ranges > 1)
+ ath10k_warn(ar,
+ "Unsupported number of MPDU ranges: %d, ignoring all but the first\n",
+ num_mpdu_ranges);
+
+ if (mpdu_ranges->mpdu_range_status !=
+ HTT_RX_IND_MPDU_STATUS_OK) {
+ ath10k_warn(ar, "MPDU range status: %d\n",
+ mpdu_ranges->mpdu_range_status);
+ goto err;
+ }
+
+ /* Strip off all headers before the MAC header before delivery to
+ * mac80211
+ */
+ tot_hdr_len = sizeof(struct htt_resp_hdr) + sizeof(rx->hdr) +
+ sizeof(rx->ppdu) + sizeof(rx->prefix) +
+ sizeof(rx->fw_desc) +
+ sizeof(*mpdu_ranges) * num_mpdu_ranges + rx_desc_len;
+ skb_pull(skb, tot_hdr_len);
+
+ hdr = (struct ieee80211_hdr *)skb->data;
+ rx_status = IEEE80211_SKB_RXCB(skb);
+ rx_status->chains |= BIT(0);
+ rx_status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
+ rx->ppdu.combined_rssi;
+ rx_status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
+
+ spin_lock_bh(&ar->data_lock);
+ ch = ar->scan_channel;
+ if (!ch)
+ ch = ar->rx_channel;
+ if (!ch)
+ ch = ath10k_htt_rx_h_any_channel(ar);
+ if (!ch)
+ ch = ar->tgt_oper_chan;
+ spin_unlock_bh(&ar->data_lock);
+
+ if (ch) {
+ rx_status->band = ch->band;
+ rx_status->freq = ch->center_freq;
+ }
+ if (rx->fw_desc.flags & FW_RX_DESC_FLAGS_LAST_MSDU)
+ rx_status->flag &= ~RX_FLAG_AMSDU_MORE;
+ else
+ rx_status->flag |= RX_FLAG_AMSDU_MORE;
+
+ /* Not entirely sure about this, but all frames from the chipset has
+ * the protected flag set even though they have already been decrypted.
+ * Unmasking this flag is necessary in order for mac80211 not to drop
+ * the frame.
+ * TODO: Verify this is always the case or find out a way to check
+ * if there has been hw decryption.
+ */
+ if (ieee80211_has_protected(hdr->frame_control)) {
+ hdr->frame_control &= ~__cpu_to_le16(IEEE80211_FCTL_PROTECTED);
+ rx_status->flag |= RX_FLAG_DECRYPTED |
+ RX_FLAG_IV_STRIPPED |
+ RX_FLAG_MMIC_STRIPPED;
+ }
+
+ local_bh_disable();
+ ieee80211_rx(ar->hw, skb);
+ local_bh_enable();
+
+ /* We have delivered the skb to the upper layers (mac80211) so we
+ * must not free it.
+ */
+ return false;
+err:
+ /* Tell the caller that it must free the skb since we have not
+ * consumed it
+ */
+ return true;
+}
+
+static void ath10k_htt_rx_proc_rx_ind_ll(struct ath10k_htt *htt,
+ struct htt_rx_indication *rx)
{
struct ath10k *ar = htt->ar;
struct htt_rx_indication_mpdu_range *mpdu_ranges;
@@ -2383,7 +2491,12 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
break;
}
case HTT_T2H_MSG_TYPE_RX_IND:
- ath10k_htt_rx_proc_rx_ind(htt, &resp->rx_ind);
+ if (ar->is_high_latency)
+ return ath10k_htt_rx_proc_rx_ind_hl(htt,
+ &resp->rx_ind_hl,
+ skb);
+ else
+ ath10k_htt_rx_proc_rx_ind_ll(htt, &resp->rx_ind);
break;
case HTT_T2H_MSG_TYPE_PEER_MAP: {
struct htt_peer_map_event ev = {
diff --git a/drivers/net/wireless/ath/ath10k/rx_desc.h b/drivers/net/wireless/ath/ath10k/rx_desc.h
index c1022a1cf855..76b2fe51ddac 100644
--- a/drivers/net/wireless/ath/ath10k/rx_desc.h
+++ b/drivers/net/wireless/ath/ath10k/rx_desc.h
@@ -1222,4 +1222,19 @@ struct fw_rx_desc_base {
u8 info0;
} __packed;

+#define FW_RX_DESC_FLAGS_FIRST_MSDU (1 << 0)
+#define FW_RX_DESC_FLAGS_LAST_MSDU (1 << 1)
+#define FW_RX_DESC_C3_FAILED (1 << 2)
+#define FW_RX_DESC_C4_FAILED (1 << 3)
+#define FW_RX_DESC_IPV6 (1 << 4)
+#define FW_RX_DESC_TCP (1 << 5)
+#define FW_RX_DESC_UDP (1 << 6)
+
+struct fw_rx_desc_hl {
+ u8 info0;
+ u8 version;
+ u8 len;
+ u8 flags;
+} __packed;
+
#endif /* _RX_DESC_H_ */
--
2.14.1

2017-09-17 19:40:32

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 08/11] ath10k: add QCA9377 usb hw_param item

Hardware parameters for QCA9377 usb devices.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 23 +++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/hw.h | 1 +
2 files changed, 24 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 1880570989ae..b6893254ef53 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -328,6 +328,29 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
.vht160_mcs_rx_highest = 0,
.vht160_mcs_tx_highest = 0,
},
+ {
+ .id = QCA9377_HW_1_1_DEV_VERSION,
+ .dev_id = QCA9377_1_0_DEVICE_ID,
+ .name = "qca9377 hw1.1 usb",
+ .patch_load_addr = QCA9377_HW_1_0_PATCH_LOAD_ADDR,
+ .uart_pin = 6,
+ .otp_exe_param = 0,
+ .channel_counters_freq_hz = 88000,
+ .max_probe_resp_desc_thres = 0,
+ .cal_data_len = 8124,
+ .fw = {
+ .dir = QCA9377_HW_1_0_FW_DIR,
+ .board = QCA9377_HW_1_0_BOARD_DATA_FILE_USB,
+ .board_size = QCA9377_BOARD_DATA_SZ,
+ .board_ext_size = QCA9377_BOARD_EXT_DATA_SZ,
+ },
+ .hw_ops = &qca988x_ops,
+ .decap_align_bytes = 4,
+ .max_num_peers = TARGET_QCA9377_HL_NUM_PEERS,
+ .is_high_latency = true,
+ .bus = ATH10K_BUS_USB,
+ .start_once = true,
+ },
{
.id = QCA4019_HW_1_0_DEV_VERSION,
.dev_id = 0,
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index fd0536077404..420851e26a09 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -127,6 +127,7 @@ enum qca9377_chip_id_rev {
/* QCA9377 1.0 definitions */
#define QCA9377_HW_1_0_FW_DIR ATH10K_FW_DIR "/QCA9377/hw1.0"
#define QCA9377_HW_1_0_BOARD_DATA_FILE "board.bin"
+#define QCA9377_HW_1_0_BOARD_DATA_FILE_USB "board-usb.bin"
#define QCA9377_HW_1_0_PATCH_LOAD_ADDR 0x1234

/* QCA4019 1.0 definitions */
--
2.14.1

2017-09-17 19:40:25

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 01/11] ath10k: high_latency detection

The setup of high latency chips (USB and SDIO) is
sometimes different than for chips using low latency
interfaces.

The bus type is used to determine if the interface is
a high latency interface.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 1 +
drivers/net/wireless/ath/ath10k/core.h | 7 +++++++
2 files changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index a4f635820f35..f1924c974a12 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2496,6 +2496,7 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
ar->hw_rev = hw_rev;
ar->hif.ops = hif_ops;
ar->hif.bus = bus;
+ ar->is_high_latency = ath10k_is_high_latency(bus);

switch (hw_rev) {
case ATH10K_HW_QCA988X:
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 949ebb3e967b..dc9ecf773d51 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -789,6 +789,8 @@ struct ath10k {

bool p2p;

+ bool is_high_latency;
+
struct {
enum ath10k_bus bus;
const struct ath10k_hif_ops *ops;
@@ -1013,6 +1015,11 @@ static inline bool ath10k_peer_stats_enabled(struct ath10k *ar)
return false;
}

+static inline bool ath10k_is_high_latency(enum ath10k_bus bus)
+{
+ return ((bus == ATH10K_BUS_SDIO) || (bus == ATH10K_BUS_USB));
+}
+
struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
enum ath10k_bus bus,
enum ath10k_hw_rev hw_rev,
--
2.14.1

2017-09-17 19:40:34

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 10/11] ath10k: wmi: disable softirq's while calling ieee80211_rx

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/wmi.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 38a97086708b..10bb5be6ab00 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2408,7 +2408,10 @@ int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
status->freq, status->band, status->signal,
status->rate_idx);

+ local_bh_disable();
ieee80211_rx(ar->hw, skb);
+ local_bh_enable();
+
return 0;
}

--
2.14.1

2017-09-17 19:40:33

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 09/11] ath10k: add QCA9377 sdio hw_param item

Hardware parameters for QCA9377 sdio devices.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 25 +++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/hw.h | 1 +
2 files changed, 26 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index b6893254ef53..146a9f61b5f0 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -351,6 +351,31 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
.bus = ATH10K_BUS_USB,
.start_once = true,
},
+ {
+ .id = QCA9377_HW_1_1_DEV_VERSION,
+ .dev_id = QCA9377_1_0_DEVICE_ID,
+ .name = "qca9377 hw1.1 sdio",
+ .patch_load_addr = QCA9377_HW_1_0_PATCH_LOAD_ADDR,
+ .uart_pin = 19,
+ .otp_exe_param = 0,
+ .channel_counters_freq_hz = 88000,
+ .max_probe_resp_desc_thres = 0,
+ .cal_data_len = 8124,
+ .fw = {
+ .dir = QCA9377_HW_1_0_FW_DIR,
+ .board = QCA9377_HW_1_0_BOARD_DATA_FILE_SDIO,
+ .board_size = QCA9377_BOARD_DATA_SZ,
+ .board_ext_size = QCA9377_BOARD_EXT_DATA_SZ,
+ },
+ .hw_ops = &qca6174_ops,
+ .hw_clk = qca6174_clk,
+ .target_cpu_freq = 176000000,
+ .decap_align_bytes = 4,
+ .max_num_peers = TARGET_QCA9377_HL_NUM_PEERS,
+ .is_high_latency = true,
+ .bus = ATH10K_BUS_SDIO,
+ .start_once = true,
+ },
{
.id = QCA4019_HW_1_0_DEV_VERSION,
.dev_id = 0,
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 420851e26a09..77011a6cafa1 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -128,6 +128,7 @@ enum qca9377_chip_id_rev {
#define QCA9377_HW_1_0_FW_DIR ATH10K_FW_DIR "/QCA9377/hw1.0"
#define QCA9377_HW_1_0_BOARD_DATA_FILE "board.bin"
#define QCA9377_HW_1_0_BOARD_DATA_FILE_USB "board-usb.bin"
+#define QCA9377_HW_1_0_BOARD_DATA_FILE_SDIO "board-sdio.bin"
#define QCA9377_HW_1_0_PATCH_LOAD_ADDR 0x1234

/* QCA4019 1.0 definitions */
--
2.14.1

2017-09-17 19:40:29

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 05/11] ath10k: htt: High latency TX support

Add HTT TX function for HL interfaces.
Intended for SDIO and USB.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt.h | 9 ++--
drivers/net/wireless/ath/ath10k/htt_tx.c | 91 +++++++++++++++++++++++++++++++-
drivers/net/wireless/ath/ath10k/mac.c | 5 +-
3 files changed, 99 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 7ffa1d41f478..bac453f5753e 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1830,9 +1830,12 @@ int ath10k_htt_tx_mgmt_inc_pending(struct ath10k_htt *htt, bool is_mgmt,
int ath10k_htt_tx_alloc_msdu_id(struct ath10k_htt *htt, struct sk_buff *skb);
void ath10k_htt_tx_free_msdu_id(struct ath10k_htt *htt, u16 msdu_id);
int ath10k_htt_mgmt_tx(struct ath10k_htt *htt, struct sk_buff *msdu);
-int ath10k_htt_tx(struct ath10k_htt *htt,
- enum ath10k_hw_txrx_mode txmode,
- struct sk_buff *msdu);
+int ath10k_htt_tx_ll(struct ath10k_htt *htt,
+ enum ath10k_hw_txrx_mode txmode,
+ struct sk_buff *msdu);
+int ath10k_htt_tx_hl(struct ath10k_htt *htt,
+ enum ath10k_hw_txrx_mode txmode,
+ struct sk_buff *msdu);
void ath10k_htt_rx_pktlog_completion_handler(struct ath10k *ar,
struct sk_buff *skb);
int ath10k_htt_txrx_compl_task(struct ath10k *ar, int budget);
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 8d85f82ad8f8..a3d69f852e38 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -946,8 +946,95 @@ int ath10k_htt_mgmt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
return res;
}

-int ath10k_htt_tx(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txmode,
- struct sk_buff *msdu)
+#define HTT_TX_HL_NEEDED_HEADROOM \
+ (unsigned int)(sizeof(struct htt_cmd_hdr) + \
+ sizeof(struct htt_data_tx_desc) + \
+ sizeof(struct ath10k_htc_hdr))
+
+int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txmode,
+ struct sk_buff *msdu)
+{
+ struct ath10k *ar = htt->ar;
+ int res, data_len;
+ struct htt_cmd_hdr *cmd_hdr;
+ struct htt_data_tx_desc *tx_desc;
+ struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(msdu);
+ bool is_eth = (txmode == ATH10K_HW_TXRX_ETHERNET);
+ u8 vdev_id = ath10k_htt_tx_get_vdev_id(ar, msdu);
+ u8 tid = ath10k_htt_tx_get_tid(msdu, is_eth);
+ u8 flags0 = 0;
+ u16 flags1 = 0;
+
+ data_len = msdu->len;
+
+ switch (txmode) {
+ case ATH10K_HW_TXRX_RAW:
+ case ATH10K_HW_TXRX_NATIVE_WIFI:
+ flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT;
+ /* fall through */
+ case ATH10K_HW_TXRX_ETHERNET:
+ flags0 |= SM(txmode, HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE);
+ break;
+ case ATH10K_HW_TXRX_MGMT:
+ flags0 |= SM(ATH10K_HW_TXRX_MGMT,
+ HTT_DATA_TX_DESC_FLAGS0_PKT_TYPE);
+ flags0 |= HTT_DATA_TX_DESC_FLAGS0_MAC_HDR_PRESENT;
+ break;
+ }
+
+ if (skb_cb->flags & ATH10K_SKB_F_NO_HWCRYPT)
+ flags0 |= HTT_DATA_TX_DESC_FLAGS0_NO_ENCRYPT;
+
+ flags1 |= SM((u16)vdev_id, HTT_DATA_TX_DESC_FLAGS1_VDEV_ID);
+ flags1 |= SM((u16)tid, HTT_DATA_TX_DESC_FLAGS1_EXT_TID);
+ if (msdu->ip_summed == CHECKSUM_PARTIAL &&
+ !test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags)) {
+ flags1 |= HTT_DATA_TX_DESC_FLAGS1_CKSUM_L3_OFFLOAD;
+ flags1 |= HTT_DATA_TX_DESC_FLAGS1_CKSUM_L4_OFFLOAD;
+ }
+
+ /* Prepend the HTT header and TX desc struct to the data message
+ * and realloc the skb if it does not have enough headroom.
+ */
+ if (skb_headroom(msdu) < HTT_TX_HL_NEEDED_HEADROOM) {
+ struct sk_buff *tmp_skb = msdu;
+
+ ath10k_dbg(htt->ar, ATH10K_DBG_HTT,
+ "Not enough headroom in skb. Current headroom: %u, needed: %u. Reallocating...\n",
+ skb_headroom(msdu), HTT_TX_HL_NEEDED_HEADROOM);
+ msdu = skb_realloc_headroom(msdu, HTT_TX_HL_NEEDED_HEADROOM);
+ kfree_skb(tmp_skb);
+ if (!msdu) {
+ ath10k_warn(htt->ar, "htt hl tx: Unable to realloc skb!\n");
+ res = -ENOMEM;
+ goto out;
+ }
+ }
+
+ skb_push(msdu, sizeof(*cmd_hdr));
+ skb_push(msdu, sizeof(*tx_desc));
+ cmd_hdr = (struct htt_cmd_hdr *)msdu->data;
+ tx_desc = (struct htt_data_tx_desc *)(msdu->data + sizeof(*cmd_hdr));
+
+ cmd_hdr->msg_type = HTT_H2T_MSG_TYPE_TX_FRM;
+ tx_desc->flags0 = flags0;
+ tx_desc->flags1 = __cpu_to_le16(flags1);
+ tx_desc->len = __cpu_to_le16(data_len);
+ tx_desc->id = 0;
+ tx_desc->frags_paddr = 0; /* always zero */
+ /* Initialize peer_id to INVALID_PEER because this is NOT
+ * Reinjection path
+ */
+ tx_desc->peerid = __cpu_to_le32(HTT_INVALID_PEERID);
+
+ res = ath10k_htc_send(&htt->ar->htc, htt->eid, msdu);
+
+out:
+ return res;
+}
+
+int ath10k_htt_tx_ll(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txmode,
+ struct sk_buff *msdu)
{
struct ath10k *ar = htt->ar;
struct device *dev = ar->dev;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 5683f1a5330e..aa817b8fcae4 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3585,7 +3585,10 @@ static int ath10k_mac_tx_submit(struct ath10k *ar,

switch (txpath) {
case ATH10K_MAC_TX_HTT:
- ret = ath10k_htt_tx(htt, txmode, skb);
+ if (ar->is_high_latency)
+ ret = ath10k_htt_tx_hl(htt, txmode, skb);
+ else
+ ret = ath10k_htt_tx_ll(htt, txmode, skb);
break;
case ATH10K_MAC_TX_HTT_MGMT:
ret = ath10k_htt_mgmt_tx(htt, skb);
--
2.14.1

2017-09-17 19:40:26

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 02/11] ath10k: htt: RX ring config HL support

Special HTT RX ring config message used by high latency
devices.

The main difference between HL and LL is that HL devices
do not use shared memory between device and host and thus,
no host paddr's are added to the RX config message.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt.c | 5 +++-
drivers/net/wireless/ath/ath10k/htt.h | 1 +
drivers/net/wireless/ath/ath10k/htt_tx.c | 51 ++++++++++++++++++++++++++++++++
3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index cd160b16db1e..29ed4afe52a4 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -258,7 +258,10 @@ int ath10k_htt_setup(struct ath10k_htt *htt)
if (status)
return status;

- status = ath10k_htt_send_rx_ring_cfg_ll(htt);
+ if (ar->is_high_latency)
+ status = ath10k_htt_send_rx_ring_cfg_hl(htt);
+ else
+ status = ath10k_htt_send_rx_ring_cfg_ll(htt);
if (status) {
ath10k_warn(ar, "failed to setup rx ring: %d\n",
status);
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index 6305308422c4..7ffa1d41f478 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1805,6 +1805,7 @@ int ath10k_htt_h2t_ver_req_msg(struct ath10k_htt *htt);
int ath10k_htt_h2t_stats_req(struct ath10k_htt *htt, u8 mask, u64 cookie);
int ath10k_htt_send_frag_desc_bank_cfg(struct ath10k_htt *htt);
int ath10k_htt_send_rx_ring_cfg_ll(struct ath10k_htt *htt);
+int ath10k_htt_send_rx_ring_cfg_hl(struct ath10k_htt *htt);
int ath10k_htt_h2t_aggr_cfg_msg(struct ath10k_htt *htt,
u8 max_subfrms_ampdu,
u8 max_subfrms_amsdu);
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 685faac1368f..8d85f82ad8f8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -693,6 +693,57 @@ int ath10k_htt_send_rx_ring_cfg_ll(struct ath10k_htt *htt)
return 0;
}

+int ath10k_htt_send_rx_ring_cfg_hl(struct ath10k_htt *htt)
+{
+ struct ath10k *ar = htt->ar;
+ struct sk_buff *skb;
+ struct htt_cmd *cmd;
+ struct htt_rx_ring_setup_ring *ring;
+ const int num_rx_ring = 1;
+ u16 flags;
+ int len;
+ int ret;
+
+ /*
+ * the HW expects the buffer to be an integral number of 4-byte
+ * "words"
+ */
+ BUILD_BUG_ON(!IS_ALIGNED(HTT_RX_BUF_SIZE, 4));
+ BUILD_BUG_ON((HTT_RX_BUF_SIZE & HTT_MAX_CACHE_LINE_SIZE_MASK) != 0);
+
+ len = sizeof(cmd->hdr) + sizeof(cmd->rx_setup.hdr)
+ + (sizeof(*ring) * num_rx_ring);
+ skb = ath10k_htc_alloc_skb(ar, len);
+ if (!skb)
+ return -ENOMEM;
+
+ skb_put(skb, len);
+
+ cmd = (struct htt_cmd *)skb->data;
+ ring = &cmd->rx_setup.rings[0];
+
+ cmd->hdr.msg_type = HTT_H2T_MSG_TYPE_RX_RING_CFG;
+ cmd->rx_setup.hdr.num_rings = 1;
+
+ flags = 0;
+ flags |= HTT_RX_RING_FLAGS_MSDU_PAYLOAD;
+ flags |= HTT_RX_RING_FLAGS_UNICAST_RX;
+ flags |= HTT_RX_RING_FLAGS_MULTICAST_RX;
+
+ memset(ring, 0, sizeof(*ring));
+ ring->rx_ring_len = __cpu_to_le16(HTT_RX_RING_SIZE_MIN);
+ ring->rx_ring_bufsize = __cpu_to_le16(HTT_RX_BUF_SIZE);
+ ring->flags = __cpu_to_le16(flags);
+
+ ret = ath10k_htc_send(&htt->ar->htc, htt->eid, skb);
+ if (ret) {
+ dev_kfree_skb_any(skb);
+ return ret;
+ }
+
+ return 0;
+}
+
int ath10k_htt_h2t_aggr_cfg_msg(struct ath10k_htt *htt,
u8 max_subfrms_ampdu,
u8 max_subfrms_amsdu)
--
2.14.1

2017-09-17 19:40:27

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 03/11] ath10k: per target configurablity of various items

Added ability to set bus type and configure the max number of
peers in the ath10k_hw_params struct.

With this functionality it is possible to have a different
hw configuration depending on bus type for the same radio
chipset.

E.g. SDIO and USB devices using the same chipset as PCIe
devices will potentially use different board files and perhaps
other configuration parameters.

One such parameter is the max number of peers.
Instead of using a default value (suitable for PCIe devices)
derived from the WMI op version, a per target value can be
used instead.

This is needed by the QCA9377 USB device in order to prevent
the target fw to crash after HTT RX ring cfg is issued.

Apparently, the QCA9377 HL device does not seem to handle the
same amount of peers as the LL devices.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 33 +++++++++++++++++++++++--------
drivers/net/wireless/ath/ath10k/core.h | 7 -------
drivers/net/wireless/ath/ath10k/hw.h | 22 +++++++++++++++++++++
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 4 ++--
4 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index f1924c974a12..a4a326c89e0d 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1663,9 +1663,19 @@ static int ath10k_init_hw_params(struct ath10k *ar)
for (i = 0; i < ARRAY_SIZE(ath10k_hw_params_list); i++) {
hw_params = &ath10k_hw_params_list[i];

- if (hw_params->id == ar->target_version &&
- hw_params->dev_id == ar->dev_id)
- break;
+ if (ar->is_high_latency) {
+ /* High latency devices will use different fw depending
+ * on if it is a USB or SDIO device.
+ */
+ if (hw_params->bus == ar->hif.bus &&
+ hw_params->id == ar->target_version &&
+ hw_params->dev_id == ar->dev_id)
+ break;
+ } else {
+ if (hw_params->id == ar->target_version &&
+ hw_params->dev_id == ar->dev_id)
+ break;
+ }
}

if (i == ARRAY_SIZE(ath10k_hw_params_list)) {
@@ -1764,6 +1774,7 @@ static void ath10k_core_set_coverage_class_work(struct work_struct *work)
static int ath10k_core_init_firmware_features(struct ath10k *ar)
{
struct ath10k_fw_file *fw_file = &ar->normal_mode_fw.fw_file;
+ int max_num_peers;

if (test_bit(ATH10K_FW_FEATURE_WMI_10_2, fw_file->fw_features) &&
!test_bit(ATH10K_FW_FEATURE_WMI_10X, fw_file->fw_features)) {
@@ -1843,7 +1854,7 @@ static int ath10k_core_init_firmware_features(struct ath10k *ar)

switch (fw_file->wmi_op_version) {
case ATH10K_FW_WMI_OP_VERSION_MAIN:
- ar->max_num_peers = TARGET_NUM_PEERS;
+ max_num_peers = TARGET_NUM_PEERS;
ar->max_num_stations = TARGET_NUM_STATIONS;
ar->max_num_vdevs = TARGET_NUM_VDEVS;
ar->htt.max_num_pending_tx = TARGET_NUM_MSDU_DESC;
@@ -1855,10 +1866,10 @@ static int ath10k_core_init_firmware_features(struct ath10k *ar)
case ATH10K_FW_WMI_OP_VERSION_10_2:
case ATH10K_FW_WMI_OP_VERSION_10_2_4:
if (ath10k_peer_stats_enabled(ar)) {
- ar->max_num_peers = TARGET_10X_TX_STATS_NUM_PEERS;
+ max_num_peers = TARGET_10X_TX_STATS_NUM_PEERS;
ar->max_num_stations = TARGET_10X_TX_STATS_NUM_STATIONS;
} else {
- ar->max_num_peers = TARGET_10X_NUM_PEERS;
+ max_num_peers = TARGET_10X_NUM_PEERS;
ar->max_num_stations = TARGET_10X_NUM_STATIONS;
}
ar->max_num_vdevs = TARGET_10X_NUM_VDEVS;
@@ -1867,7 +1878,7 @@ static int ath10k_core_init_firmware_features(struct ath10k *ar)
ar->max_spatial_stream = WMI_MAX_SPATIAL_STREAM;
break;
case ATH10K_FW_WMI_OP_VERSION_TLV:
- ar->max_num_peers = TARGET_TLV_NUM_PEERS;
+ max_num_peers = TARGET_TLV_NUM_PEERS;
ar->max_num_stations = TARGET_TLV_NUM_STATIONS;
ar->max_num_vdevs = TARGET_TLV_NUM_VDEVS;
ar->max_num_tdls_vdevs = TARGET_TLV_NUM_TDLS_VDEVS;
@@ -1878,7 +1889,7 @@ static int ath10k_core_init_firmware_features(struct ath10k *ar)
ar->max_spatial_stream = WMI_MAX_SPATIAL_STREAM;
break;
case ATH10K_FW_WMI_OP_VERSION_10_4:
- ar->max_num_peers = TARGET_10_4_NUM_PEERS;
+ max_num_peers = TARGET_10_4_NUM_PEERS;
ar->max_num_stations = TARGET_10_4_NUM_STATIONS;
ar->num_active_peers = TARGET_10_4_ACTIVE_PEERS;
ar->max_num_vdevs = TARGET_10_4_NUM_VDEVS;
@@ -1896,10 +1907,16 @@ static int ath10k_core_init_firmware_features(struct ath10k *ar)
break;
case ATH10K_FW_WMI_OP_VERSION_UNSET:
case ATH10K_FW_WMI_OP_VERSION_MAX:
+ default:
WARN_ON(1);
return -EINVAL;
}

+ if (ar->hw_params.max_num_peers)
+ ar->max_num_peers = ar->hw_params.max_num_peers;
+ else
+ ar->max_num_peers = max_num_peers;
+
/* Backwards compatibility for firmwares without
* ATH10K_FW_IE_HTT_OP_VERSION.
*/
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index dc9ecf773d51..64dadcd6e531 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -88,13 +88,6 @@

struct ath10k;

-enum ath10k_bus {
- ATH10K_BUS_PCI,
- ATH10K_BUS_AHB,
- ATH10K_BUS_SDIO,
- ATH10K_BUS_USB,
-};
-
static inline const char *ath10k_bus_str(enum ath10k_bus bus)
{
switch (bus) {
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 0c089f6dd3d9..8cf7b963f3d4 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -20,6 +20,13 @@

#include "targaddrs.h"

+enum ath10k_bus {
+ ATH10K_BUS_PCI,
+ ATH10K_BUS_AHB,
+ ATH10K_BUS_SDIO,
+ ATH10K_BUS_USB,
+};
+
#define ATH10K_FW_DIR "ath10k"

#define QCA988X_2_0_DEVICE_ID (0x003c)
@@ -550,6 +557,18 @@ struct ath10k_hw_params {
*/
int vht160_mcs_rx_highest;
int vht160_mcs_tx_highest;
+
+ /* max_num_peers can be used to override the setting derived from
+ * the WMI op version. If this value is non-zero, it will always
+ * be used instead of the default value derived from the WMI op
+ * version.
+ */
+ int max_num_peers;
+
+ /* Specifies whether or not the device is a high latency device */
+ bool is_high_latency;
+
+ enum ath10k_bus bus;
};

struct htt_rx_desc;
@@ -660,6 +679,9 @@ ath10k_rx_desc_get_l3_pad_bytes(struct ath10k_hw_params *hw,
#define TARGET_TLV_NUM_MSDU_DESC (1024 + 32)
#define TARGET_TLV_NUM_WOW_PATTERNS 22

+/* Target specific defines for QCA9377 high latency firmware */
+#define TARGET_QCA9377_HL_NUM_PEERS 15
+
/* Diagnostic Window */
#define CE_DIAG_PIPE 7

diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 7616c1c4bbd3..34e977049f00 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1406,7 +1406,7 @@ static struct sk_buff *ath10k_wmi_tlv_op_gen_init(struct ath10k *ar)
cmd->num_host_mem_chunks = __cpu_to_le32(ar->wmi.num_mem_chunks);

cfg->num_vdevs = __cpu_to_le32(TARGET_TLV_NUM_VDEVS);
- cfg->num_peers = __cpu_to_le32(TARGET_TLV_NUM_PEERS);
+ cfg->num_peers = __cpu_to_le32(ar->max_num_peers);

if (test_bit(WMI_SERVICE_RX_FULL_REORDER, ar->wmi.svc_map)) {
cfg->num_offload_peers = __cpu_to_le32(TARGET_TLV_NUM_VDEVS);
@@ -1417,7 +1417,7 @@ static struct sk_buff *ath10k_wmi_tlv_op_gen_init(struct ath10k *ar)
}

cfg->num_peer_keys = __cpu_to_le32(2);
- cfg->num_tids = __cpu_to_le32(TARGET_TLV_NUM_TIDS);
+ cfg->num_tids = __cpu_to_le32(ar->max_num_peers * 2);
cfg->ast_skid_limit = __cpu_to_le32(0x10);
cfg->tx_chain_mask = __cpu_to_le32(0x7);
cfg->rx_chain_mask = __cpu_to_le32(0x7);
--
2.14.1

2017-09-17 19:40:35

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 11/11] ath10k: remove htt pending TX count for high latency

High latency chipsest does not seem to send any
HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing frames.

This means that htt->num_pending_tx will never be
decremented and we will eventually hit the maximum
limit. All outgoing packets will then be discarded.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_tx.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 82d01139ff92..c74fc137ac67 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -153,6 +153,9 @@ void ath10k_htt_tx_txq_update(struct ieee80211_hw *hw,

void ath10k_htt_tx_dec_pending(struct ath10k_htt *htt)
{
+ if (htt->ar->is_high_latency)
+ return;
+
lockdep_assert_held(&htt->tx_lock);

htt->num_pending_tx--;
@@ -162,6 +165,9 @@ void ath10k_htt_tx_dec_pending(struct ath10k_htt *htt)

int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt)
{
+ if (htt->ar->is_high_latency)
+ return 0;
+
lockdep_assert_held(&htt->tx_lock);

if (htt->num_pending_tx >= htt->max_num_pending_tx)
--
2.14.1

2017-09-17 19:40:28

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 04/11] ath10k: add start_once support

Add possibility to configure the driver to only start target once.
This can reduce startup time of SDIO devices significantly since
loading the firmware can take a substantial amount of time.

The patch is also necessary for high latency devices in general
since it does not seem to be possible to rerun the BMI phase
(fw upload) without power-cycling the device.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.c | 19 +++++++++++++++----
drivers/net/wireless/ath/ath10k/core.h | 2 ++
drivers/net/wireless/ath/ath10k/hw.h | 6 ++++++
3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index a4a326c89e0d..c21227a74996 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1999,6 +1999,9 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
int status;
u32 val;

+ if (ar->is_started && ar->hw_params.start_once)
+ return 0;
+
lockdep_assert_held(&ar->conf_mutex);

clear_bit(ATH10K_FLAG_CRASH_FLUSH, &ar->dev_flags);
@@ -2226,6 +2229,7 @@ int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
if (status)
goto err_hif_stop;

+ ar->is_started = true;
return 0;

err_hif_stop:
@@ -2278,6 +2282,7 @@ void ath10k_core_stop(struct ath10k *ar)
ath10k_htt_tx_stop(&ar->htt);
ath10k_htt_rx_free(&ar->htt);
ath10k_wmi_detach(ar);
+ ar->is_started = false;
}
EXPORT_SYMBOL(ath10k_core_stop);

@@ -2380,12 +2385,18 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
goto err_unlock;
}

- ath10k_debug_print_boot_info(ar);
- ath10k_core_stop(ar);
+ /* Leave target running if hw_params.start_once is set */
+ if (ar->hw_params.start_once) {
+ mutex_unlock(&ar->conf_mutex);
+ } else {
+ ath10k_debug_print_boot_info(ar);
+ ath10k_core_stop(ar);

- mutex_unlock(&ar->conf_mutex);
+ mutex_unlock(&ar->conf_mutex);
+
+ ath10k_hif_power_down(ar);
+ }

- ath10k_hif_power_down(ar);
return 0;

err_unlock:
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 64dadcd6e531..0b5b1dd00e16 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -784,6 +784,8 @@ struct ath10k {

bool is_high_latency;

+ bool is_started;
+
struct {
enum ath10k_bus bus;
const struct ath10k_hif_ops *ops;
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 8cf7b963f3d4..fd0536077404 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -569,6 +569,12 @@ struct ath10k_hw_params {
bool is_high_latency;

enum ath10k_bus bus;
+
+ /* Specifies whether or not the device should be started once.
+ * If set, the device will be started once by the early fw probe
+ * and it will not be terminated afterwards.
+ */
+ bool start_once;
};

struct htt_rx_desc;
--
2.14.1

2017-09-17 19:40:31

by Erik Stromdahl

[permalink] [raw]
Subject: [RFC v3 07/11] ath10k: various fixes for high latency devices

Several DMA related functions (such as the dma_map_xxx functions)
are not used with high latency devices and don't need to be invoked
in this case.

A few other execution paths are not applicable for high latency
devices and can be skipped.

Signed-off-by: Erik Stromdahl <[email protected]>
---
drivers/net/wireless/ath/ath10k/htc.c | 19 ++++++++++++-------
drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
drivers/net/wireless/ath/ath10k/htt_tx.c | 9 +++++++--
drivers/net/wireless/ath/ath10k/txrx.c | 5 +++--
4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index e5c80f582ff5..75c2a3ea7ec9 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -53,7 +53,8 @@ static inline void ath10k_htc_restore_tx_skb(struct ath10k_htc *htc,
{
struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);

- dma_unmap_single(htc->ar->dev, skb_cb->paddr, skb->len, DMA_TO_DEVICE);
+ if (!htc->ar->is_high_latency)
+ dma_unmap_single(htc->ar->dev, skb_cb->paddr, skb->len, DMA_TO_DEVICE);
skb_pull(skb, sizeof(struct ath10k_htc_hdr));
}

@@ -137,11 +138,14 @@ int ath10k_htc_send(struct ath10k_htc *htc,
ath10k_htc_prepare_tx_skb(ep, skb);

skb_cb->eid = eid;
- skb_cb->paddr = dma_map_single(dev, skb->data, skb->len, DMA_TO_DEVICE);
- ret = dma_mapping_error(dev, skb_cb->paddr);
- if (ret) {
- ret = -EIO;
- goto err_credits;
+ if (!ar->is_high_latency) {
+ skb_cb->paddr = dma_map_single(dev, skb->data, skb->len,
+ DMA_TO_DEVICE);
+ ret = dma_mapping_error(dev, skb_cb->paddr);
+ if (ret) {
+ ret = -EIO;
+ goto err_credits;
+ }
}

sg_item.transfer_id = ep->eid;
@@ -157,7 +161,8 @@ int ath10k_htc_send(struct ath10k_htc *htc,
return 0;

err_unmap:
- dma_unmap_single(dev, skb_cb->paddr, skb->len, DMA_TO_DEVICE);
+ if (!ar->is_high_latency)
+ dma_unmap_single(dev, skb_cb->paddr, skb->len, DMA_TO_DEVICE);
err_credits:
if (ep->tx_credit_flow_enabled) {
spin_lock_bh(&htc->tx_lock);
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 7461555ccad5..569edd0720c6 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2541,7 +2541,8 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb)
break;
}
case HTT_T2H_MSG_TYPE_TX_COMPL_IND:
- ath10k_htt_rx_tx_compl_ind(htt->ar, skb);
+ if (!ar->is_high_latency)
+ ath10k_htt_rx_tx_compl_ind(htt->ar, skb);
break;
case HTT_T2H_MSG_TYPE_SEC_IND: {
struct ath10k *ar = htt->ar;
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index a3d69f852e38..82d01139ff92 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -409,6 +409,9 @@ int ath10k_htt_tx_start(struct ath10k_htt *htt)
if (htt->tx_mem_allocated)
return 0;

+ if (ar->is_high_latency)
+ return 0;
+
ret = ath10k_htt_tx_alloc_buf(htt);
if (ret)
goto free_idr_pending_tx;
@@ -445,7 +448,8 @@ void ath10k_htt_tx_destroy(struct ath10k_htt *htt)
return;

ath10k_htt_tx_free_cont_txbuf(htt);
- ath10k_htt_tx_free_txq(htt);
+ if (!htt->ar->is_high_latency)
+ ath10k_htt_tx_free_txq(htt);
ath10k_htt_tx_free_cont_frag_desc(htt);
ath10k_htt_tx_free_txdone_fifo(htt);
htt->tx_mem_allocated = false;
@@ -935,7 +939,8 @@ int ath10k_htt_mgmt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
return 0;

err_unmap_msdu:
- dma_unmap_single(dev, skb_cb->paddr, msdu->len, DMA_TO_DEVICE);
+ if (!ar->is_high_latency)
+ dma_unmap_single(dev, skb_cb->paddr, msdu->len, DMA_TO_DEVICE);
err_free_txdesc:
dev_kfree_skb_any(txdesc);
err_free_msdu_id:
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index d4986f626c35..fae143e4dcfa 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -90,11 +90,12 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,

ath10k_htt_tx_free_msdu_id(htt, tx_done->msdu_id);
ath10k_htt_tx_dec_pending(htt);
- if (htt->num_pending_tx == 0)
+ if (!ar->is_high_latency && (htt->num_pending_tx == 0))
wake_up(&htt->empty_tx_wq);
spin_unlock_bh(&htt->tx_lock);

- dma_unmap_single(dev, skb_cb->paddr, msdu->len, DMA_TO_DEVICE);
+ if (!ar->is_high_latency)
+ dma_unmap_single(dev, skb_cb->paddr, msdu->len, DMA_TO_DEVICE);

ath10k_report_offchan_tx(htt->ar, msdu);

--
2.14.1

2017-12-22 15:32:14

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 06/11] ath10k: htt: High latency RX support

Erik Stromdahl <[email protected]> writes:

> Special HTT RX handling for high latency interfaces.
>
> Since no DMA physical addresses are used in the RX ring
> config message (this is not supported by the high latency
> devices), no RX ring is allocated.
> All RX skb's are allocated by the driver and passed directly
> to mac80211 in the HTT RX indication handler.
>
> A nice side effect of this is that no huge buffer will be
> allocated with dma_alloc_coherent. On embedded systems with
> limited memory resources, the allocation of the RX ring is
> prone to fail.
>
> Some tweaks made to "make it work":
>
> Removal of protected bit in 802.11 header frame control field.
> The chipset seems to do hw decryption but the frame_control
> protected bit is still set.
>
> This is necessary for mac80211 not to drop the frame.
>
> Signed-off-by: Erik Stromdahl <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/core.c | 27 ++++---
> drivers/net/wireless/ath/ath10k/htt.h | 47 ++++++++++++
> drivers/net/wireless/ath/ath10k/htt_rx.c | 119 ++++++++++++++++++++++++=
+++++-
> drivers/net/wireless/ath/ath10k/rx_desc.h | 15 ++++
> 4 files changed, 195 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireles=
s/ath/ath10k/core.c
> index c21227a74996..1880570989ae 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -2083,10 +2083,12 @@ int ath10k_core_start(struct ath10k *ar, enum ath=
10k_firmware_mode mode,
> ar->htt.rx_ring.in_ord_rx =3D !!(test_bit(WMI_SERVICE_RX_FULL_REORDER,
> ar->wmi.svc_map));
> =20
> - status =3D ath10k_htt_rx_alloc(&ar->htt);
> - if (status) {
> - ath10k_err(ar, "failed to alloc htt rx: %d\n", status);
> - goto err_htt_tx_detach;
> + if (!ar->is_high_latency) {
> + status =3D ath10k_htt_rx_alloc(&ar->htt);
> + if (status) {
> + ath10k_err(ar, "failed to alloc htt rx: %d\n", status);
> + goto err_htt_tx_detach;
> + }
> }

Wouldn't it be cleaner code to have the is_high_latency check within
ath10k_htt_rx_alloc() call?

> @@ -2203,10 +2205,13 @@ int ath10k_core_start(struct ath10k *ar, enum ath=
10k_firmware_mode mode,
> }
> }
> =20
> - status =3D ath10k_htt_rx_ring_refill(ar);
> - if (status) {
> - ath10k_err(ar, "failed to refill htt rx ring: %d\n", status);
> - goto err_hif_stop;
> + if (!ar->is_high_latency) {
> + status =3D ath10k_htt_rx_ring_refill(ar);
> + if (status) {
> + ath10k_err(ar, "failed to refill htt rx ring: %d\n",
> + status);
> + goto err_hif_stop;
> + }
> }

Ditto.

> @@ -2235,7 +2240,8 @@ int ath10k_core_start(struct ath10k *ar, enum ath10=
k_firmware_mode mode,
> err_hif_stop:
> ath10k_hif_stop(ar);
> err_htt_rx_detach:
> - ath10k_htt_rx_free(&ar->htt);
> + if (!ar->is_high_latency)
> + ath10k_htt_rx_free(&ar->htt);
> err_htt_tx_detach:
> ath10k_htt_tx_free(&ar->htt);
> err_wmi_detach:

Ditto.

> @@ -2280,7 +2286,8 @@ void ath10k_core_stop(struct ath10k *ar)
> =20
> ath10k_hif_stop(ar);
> ath10k_htt_tx_stop(&ar->htt);
> - ath10k_htt_rx_free(&ar->htt);
> + if (!ar->is_high_latency)
> + ath10k_htt_rx_free(&ar->htt);
> ath10k_wmi_detach(ar);
> ar->is_started =3D false;
> }

Ditto.

> + /* Not entirely sure about this, but all frames from the chipset has
> + * the protected flag set even though they have already been decrypted.
> + * Unmasking this flag is necessary in order for mac80211 not to drop
> + * the frame.
> + * TODO: Verify this is always the case or find out a way to check
> + * if there has been hw decryption.
> + */
> + if (ieee80211_has_protected(hdr->frame_control)) {
> + hdr->frame_control &=3D ~__cpu_to_le16(IEEE80211_FCTL_PROTECTED);
> + rx_status->flag |=3D RX_FLAG_DECRYPTED |
> + RX_FLAG_IV_STRIPPED |
> + RX_FLAG_MMIC_STRIPPED;
> + }
> +
> + local_bh_disable();
> + ieee80211_rx(ar->hw, skb);
> + local_bh_enable();

ieee80211_rx_ni()?

--=20
Kalle Valo=

2017-12-22 15:06:08

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 01/11] ath10k: high_latency detection

Erik Stromdahl <[email protected]> writes:

> The setup of high latency chips (USB and SDIO) is
> sometimes different than for chips using low latency
> interfaces.
>
> The bus type is used to determine if the interface is
> a high latency interface.
>
> Signed-off-by: Erik Stromdahl <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/core.c | 1 +
> drivers/net/wireless/ath/ath10k/core.h | 7 +++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireles=
s/ath/ath10k/core.c
> index a4f635820f35..f1924c974a12 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -2496,6 +2496,7 @@ struct ath10k *ath10k_core_create(size_t priv_size,=
struct device *dev,
> ar->hw_rev =3D hw_rev;
> ar->hif.ops =3D hif_ops;
> ar->hif.bus =3D bus;
> + ar->is_high_latency =3D ath10k_is_high_latency(bus);

I would prefer the bus driver to provide this via a parameter in
ath10k_core_register() call.

> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -789,6 +789,8 @@ struct ath10k {
> =20
> bool p2p;
> =20
> + bool is_high_latency;
> +
> struct {
> enum ath10k_bus bus;
> const struct ath10k_hif_ops *ops;
> @@ -1013,6 +1015,11 @@ static inline bool ath10k_peer_stats_enabled(struc=
t ath10k *ar)
> return false;
> }
> =20
> +static inline bool ath10k_is_high_latency(enum ath10k_bus bus)
> +{
> + return ((bus =3D=3D ATH10K_BUS_SDIO) || (bus =3D=3D ATH10K_BUS_USB));
> +}

That way this function is not needed.

Also I'm wondering should the parameter be actually 'struct
ath10k_bus_params' (or something like that) to make it easier to extend.

--=20
Kalle Valo=

2017-12-22 15:26:27

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 05/11] ath10k: htt: High latency TX support

Erik Stromdahl <[email protected]> writes:

> Add HTT TX function for HL interfaces.
> Intended for SDIO and USB.
>
> Signed-off-by: Erik Stromdahl <[email protected]>

[...]

> + /* Prepend the HTT header and TX desc struct to the data message
> + * and realloc the skb if it does not have enough headroom.
> + */
> + if (skb_headroom(msdu) < HTT_TX_HL_NEEDED_HEADROOM) {
> + struct sk_buff *tmp_skb =3D msdu;

Style: please have the variable declaration in the beginning of the
function.

--=20
Kalle Valo=

2017-12-22 15:46:22

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 08/11] ath10k: add QCA9377 usb hw_param item

Erik Stromdahl <[email protected]> writes:

> Hardware parameters for QCA9377 usb devices.
>
> Signed-off-by: Erik Stromdahl <[email protected]>

[...]

> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -127,6 +127,7 @@ enum qca9377_chip_id_rev {
> /* QCA9377 1.0 definitions */
> #define QCA9377_HW_1_0_FW_DIR ATH10K_FW_DIR "/QCA9377/hw1.0"
> #define QCA9377_HW_1_0_BOARD_DATA_FILE "board.bin"
> +#define QCA9377_HW_1_0_BOARD_DATA_FILE_USB "board-usb.bin"
> #define QCA9377_HW_1_0_PATCH_LOAD_ADDR 0x1234

Not sure if we need board-usb.bin. board-2.bin already has a bus
attribute and board.bin is supposed to be used only by developers etc.

--=20
Kalle Valo=

2017-12-28 12:43:14

by Erik Stromdahl

[permalink] [raw]
Subject: Re: [RFC v3 03/11] ath10k: per target configurablity of various items



On 2017-12-22 16:19, Kalle Valo wrote:
> Erik Stromdahl <[email protected]> writes:
>
>> Added ability to set bus type and configure the max number of
>> peers in the ath10k_hw_params struct.
>>
>> With this functionality it is possible to have a different
>> hw configuration depending on bus type for the same radio
>> chipset.
>>
>> E.g. SDIO and USB devices using the same chipset as PCIe
>> devices will potentially use different board files and perhaps
>> other configuration parameters.
>>
>> One such parameter is the max number of peers.
>> Instead of using a default value (suitable for PCIe devices)
>> derived from the WMI op version, a per target value can be
>> used instead.
>>
>> This is needed by the QCA9377 USB device in order to prevent
>> the target fw to crash after HTT RX ring cfg is issued.
>>
>> Apparently, the QCA9377 HL device does not seem to handle the
>> same amount of peers as the LL devices.
>>
>> Signed-off-by: Erik Stromdahl <[email protected]>
>
> I was a bit torn about this, I definitely see the need for this but on
> the other hand it creates duplicate data (for example two entries for
> QCA9377 chip). I guess this is the right approach, at least I cannot
> come up anything better.
>
> But this patch should be split into two:
>
> 1) add bus field to struct ath10k_hw_params
>
> 2) add max_num_peers field to struct ath10k_hw_params
>
> And it seems 2) is already implemented in commit 9f2992fea580 ("ath10k:
> wmi: get wmi init parameter values from hw params"), so hopefully we
> only need 1) anymore.
>

Before commit 9f2992fea580a48135591873e5e3ac7e01444207,
TARGET_TLV_NUM_PEERS was used both in the WMI TLV init command
and as the value of *max_num_peers* in *struct ath10k* (ar->max_num_peers).

commit 9f2992fea580a48135591873e5e3ac7e01444207 does not set
*ar->max_num_peers* to the value of *ar->hw_param->num_peers*.

Is this correct?

As I see it, there is a possible mismatch between what is written
to the device in the WMI init message and the value of *ar->max_num_peers*.

Do we still need *max_num_peers* in *struct ath10k* now that we have the
*num_peers* member in *struct ath10k_hw_params*?

I am currently rewriting my HL patches and I was thinking about adding
a separate patch related to this.

>> --- a/drivers/net/wireless/ath/ath10k/core.c
>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>> @@ -1663,9 +1663,19 @@ static int ath10k_init_hw_params(struct ath10k *ar)
>> for (i = 0; i < ARRAY_SIZE(ath10k_hw_params_list); i++) {
>> hw_params = &ath10k_hw_params_list[i];
>>
>> - if (hw_params->id == ar->target_version &&
>> - hw_params->dev_id == ar->dev_id)
>> - break;
>> + if (ar->is_high_latency) {
>> + /* High latency devices will use different fw depending
>> + * on if it is a USB or SDIO device.
>> + */
>> + if (hw_params->bus == ar->hif.bus &&
>> + hw_params->id == ar->target_version &&
>> + hw_params->dev_id == ar->dev_id)
>> + break;
>> + } else {
>> + if (hw_params->id == ar->target_version &&
>> + hw_params->dev_id == ar->dev_id)
>> + break;
>> + }
>
> I don't like the is_high_latency test here at all. The bus field should
> be checked with all entries, not just high latency ones. And because of
> this even most of the hw_param bus field entries were not initialised.
>
> So only thing to do is to initialise the bus field for all the entries
> and the ugly test here can be removed. Just remember that QCA4019 uses
> AHB, I think all the rest is PCI. Or do we have AHB devices supported?

I noticed that there has been introduced a new bus type (SNOC).
Do you know which devices are SNOC devices?
Btw, what the heck is SNOC anyway?

>
>> @@ -550,6 +557,18 @@ struct ath10k_hw_params {
>> */
>> int vht160_mcs_rx_highest;
>> int vht160_mcs_tx_highest;
>> +
>> + /* max_num_peers can be used to override the setting derived from
>> + * the WMI op version. If this value is non-zero, it will always
>> + * be used instead of the default value derived from the WMI op
>> + * version.
>> + */
>> + int max_num_peers;
>> +
>> + /* Specifies whether or not the device is a high latency device */
>> + bool is_high_latency;
>> +
>> + enum ath10k_bus bus;
>> };
>
> Please move the bus field between dev_id and name fields. It's so
> important that it should not be in the end.
>

2017-12-22 15:55:45

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 00/11] ath10k high latency

Erik Stromdahl <[email protected]> writes:

> This is the third version of the high latency patches (stuff common for
> usb and sdio).

Sorry for the late review, this fall I have had difficulties to keep up
with all the patches. This set looks pretty good, I had few smaller
comments but nothing major really.

> One major difference between this version and the previous is that the
> num_pending_tx counter has been disabled for high latency devices (last p=
atch).
>
> This fixes the previous issue with the halted USB RX.
> I have tested these patches with a Linksys WUSB6100M and it looks
> much better than the previous version.

I quickly tested this set few weeks ago on my test laptop and the kernel
was crashing a lot, I don't know was it because I usually try to run all
kernel debug features enabled or what. I don't have the logs at hand
right now. I think we should try to investigate that before adding the
hw_params entry for the usb device, but rest of the patches in this set
can go in even before. Most important is that we don't create any
regressions (including kernel crashes).

--=20
Kalle Valo=

2017-12-22 15:49:40

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 08/11] ath10k: add QCA9377 usb hw_param item

Erik Stromdahl <[email protected]> writes:

> Hardware parameters for QCA9377 usb devices.
>
> Signed-off-by: Erik Stromdahl <[email protected]>

[...]

> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -328,6 +328,29 @@ static const struct ath10k_hw_params ath10k_hw_param=
s_list[] =3D {
> .vht160_mcs_rx_highest =3D 0,
> .vht160_mcs_tx_highest =3D 0,
> },
> + {
> + .id =3D QCA9377_HW_1_1_DEV_VERSION,
> + .dev_id =3D QCA9377_1_0_DEVICE_ID,
> + .name =3D "qca9377 hw1.1 usb",
> + .patch_load_addr =3D QCA9377_HW_1_0_PATCH_LOAD_ADDR,
> + .uart_pin =3D 6,
> + .otp_exe_param =3D 0,
> + .channel_counters_freq_hz =3D 88000,
> + .max_probe_resp_desc_thres =3D 0,
> + .cal_data_len =3D 8124,
> + .fw =3D {
> + .dir =3D QCA9377_HW_1_0_FW_DIR,
> + .board =3D QCA9377_HW_1_0_BOARD_DATA_FILE_USB,
> + .board_size =3D QCA9377_BOARD_DATA_SZ,
> + .board_ext_size =3D QCA9377_BOARD_EXT_DATA_SZ,
> + },
> + .hw_ops =3D &qca988x_ops,
> + .decap_align_bytes =3D 4,
> + .max_num_peers =3D TARGET_QCA9377_HL_NUM_PEERS,
> + .is_high_latency =3D true,
> + .bus =3D ATH10K_BUS_USB,
> + .start_once =3D true,
> + },

Oh, forgot to mention that adding hw_params entry should be last in the
patchset and only added once all fixes etc are already in place (eg
patch 10 and 11 should be before this).

--=20
Kalle Valo=

2017-12-22 15:25:13

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 04/11] ath10k: add start_once support

Erik Stromdahl <[email protected]> writes:

> Add possibility to configure the driver to only start target once.
> This can reduce startup time of SDIO devices significantly since
> loading the firmware can take a substantial amount of time.

But it also makes it impossible to restart the firmware if it crashes,
right? Good to mention that in the commit log.

> The patch is also necessary for high latency devices in general since
> it does not seem to be possible to rerun the BMI phase (fw upload)
> without power-cycling the device.
>
> Signed-off-by: Erik Stromdahl <[email protected]>

[...]

> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -784,6 +784,8 @@ struct ath10k {
> =20
> bool is_high_latency;
> =20
> + bool is_started;

Is a separate boolean really needed? State management becomes really
difficult if an enum ath10k_state and this boolean to define the state
of the device. Can't you use ar->state?

> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -569,6 +569,12 @@ struct ath10k_hw_params {
> bool is_high_latency;
> =20
> enum ath10k_bus bus;
> +
> + /* Specifies whether or not the device should be started once.
> + * If set, the device will be started once by the early fw probe
> + * and it will not be terminated afterwards.
> + */
> + bool start_once;
> };

I would actually prefer that the bus driver (eg. usb.c) decides this and
provides it though ath10k_core_register(). It might be that some SDIO
devices have a GPIO line to reset the device etc.

The struct ath10k_bus_params I mentioned earlier might be handy also
here.

--=20
Kalle Valo=

2017-12-22 15:44:01

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 07/11] ath10k: various fixes for high latency devices

Erik Stromdahl <[email protected]> writes:

> Several DMA related functions (such as the dma_map_xxx functions)
> are not used with high latency devices and don't need to be invoked
> in this case.
>
> A few other execution paths are not applicable for high latency
> devices and can be skipped.
>
> Signed-off-by: Erik Stromdahl <[email protected]>

This and patch 6 are somehow connected but the division is not clear. I
think at least dma map/unmap changes should be split into their own
patch.=20

I recommend revisiting both patches 6 and 7. The htt layer can be quite
tricky so better to have smaller logical changes to make it easier
review them.

> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -409,6 +409,9 @@ int ath10k_htt_tx_start(struct ath10k_htt *htt)
> if (htt->tx_mem_allocated)
> return 0;
> =20
> + if (ar->is_high_latency)
> + return 0;
> +
> ret =3D ath10k_htt_tx_alloc_buf(htt);
> if (ret)
> goto free_idr_pending_tx;
> @@ -445,7 +448,8 @@ void ath10k_htt_tx_destroy(struct ath10k_htt *htt)
> return;
> =20
> ath10k_htt_tx_free_cont_txbuf(htt);
> - ath10k_htt_tx_free_txq(htt);
> + if (!htt->ar->is_high_latency)
> + ath10k_htt_tx_free_txq(htt);
> ath10k_htt_tx_free_cont_frag_desc(htt);
> ath10k_htt_tx_free_txdone_fifo(htt);
> htt->tx_mem_allocated =3D false;

These two don't look symmetric. You prevent calling these functions:

ath10k_htt_tx_alloc_cont_txbuf(htt);
ath10k_htt_tx_alloc_cont_frag_desc(htt);
ath10k_htt_tx_alloc_txq(htt);
ath10k_htt_tx_alloc_txdone_fifo(htt);

But during destroy you only prevent calling ath10k_htt_tx_free_txq(htt)
and allow these functions:

ath10k_htt_tx_free_cont_txbuf(htt);
ath10k_htt_tx_free_cont_frag_desc(htt);
ath10k_htt_tx_free_txdone_fifo(htt);

--=20
Kalle Valo=

2017-12-22 15:47:47

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 09/11] ath10k: add QCA9377 sdio hw_param item

Erik Stromdahl <[email protected]> writes:

> Hardware parameters for QCA9377 sdio devices.
>
> Signed-off-by: Erik Stromdahl <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/core.c | 25 +++++++++++++++++++++++++
> drivers/net/wireless/ath/ath10k/hw.h | 1 +
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireles=
s/ath/ath10k/core.c
> index b6893254ef53..146a9f61b5f0 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -351,6 +351,31 @@ static const struct ath10k_hw_params ath10k_hw_param=
s_list[] =3D {
> .bus =3D ATH10K_BUS_USB,
> .start_once =3D true,
> },
> + {
> + .id =3D QCA9377_HW_1_1_DEV_VERSION,
> + .dev_id =3D QCA9377_1_0_DEVICE_ID,
> + .name =3D "qca9377 hw1.1 sdio",
> + .patch_load_addr =3D QCA9377_HW_1_0_PATCH_LOAD_ADDR,
> + .uart_pin =3D 19,
> + .otp_exe_param =3D 0,
> + .channel_counters_freq_hz =3D 88000,
> + .max_probe_resp_desc_thres =3D 0,
> + .cal_data_len =3D 8124,
> + .fw =3D {
> + .dir =3D QCA9377_HW_1_0_FW_DIR,
> + .board =3D QCA9377_HW_1_0_BOARD_DATA_FILE_SDIO,
> + .board_size =3D QCA9377_BOARD_DATA_SZ,
> + .board_ext_size =3D QCA9377_BOARD_EXT_DATA_SZ,
> + },
> + .hw_ops =3D &qca6174_ops,
> + .hw_clk =3D qca6174_clk,
> + .target_cpu_freq =3D 176000000,
> + .decap_align_bytes =3D 4,
> + .max_num_peers =3D TARGET_QCA9377_HL_NUM_PEERS,
> + .is_high_latency =3D true,
> + .bus =3D ATH10K_BUS_SDIO,
> + .start_once =3D true,
> + },
> {
> .id =3D QCA4019_HW_1_0_DEV_VERSION,
> .dev_id =3D 0,
> diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/=
ath/ath10k/hw.h
> index 420851e26a09..77011a6cafa1 100644
> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -128,6 +128,7 @@ enum qca9377_chip_id_rev {
> #define QCA9377_HW_1_0_FW_DIR ATH10K_FW_DIR "/QCA9377/hw1.0"
> #define QCA9377_HW_1_0_BOARD_DATA_FILE "board.bin"
> #define QCA9377_HW_1_0_BOARD_DATA_FILE_USB "board-usb.bin"
> +#define QCA9377_HW_1_0_BOARD_DATA_FILE_SDIO "board-sdio.bin"

Similar comment about not sure about having separate board.bin for sdio.

--=20
Kalle Valo=

2017-12-22 15:47:44

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 10/11] ath10k: wmi: disable softirq's while calling ieee80211_rx

Erik Stromdahl <[email protected]> writes:

> Signed-off-by: Erik Stromdahl <[email protected]>

Please explain in the commit log why this is needed.

> ---
> drivers/net/wireless/ath/ath10k/wmi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless=
/ath/ath10k/wmi.c
> index 38a97086708b..10bb5be6ab00 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -2408,7 +2408,10 @@ int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, st=
ruct sk_buff *skb)
> status->freq, status->band, status->signal,
> status->rate_idx);
> =20
> + local_bh_disable();
> ieee80211_rx(ar->hw, skb);
> + local_bh_enable();

ieee80211_rx_ni()?

--=20
Kalle Valo=

2017-12-22 15:19:28

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 03/11] ath10k: per target configurablity of various items

Erik Stromdahl <[email protected]> writes:

> Added ability to set bus type and configure the max number of
> peers in the ath10k_hw_params struct.
>
> With this functionality it is possible to have a different
> hw configuration depending on bus type for the same radio
> chipset.
>
> E.g. SDIO and USB devices using the same chipset as PCIe
> devices will potentially use different board files and perhaps
> other configuration parameters.
>
> One such parameter is the max number of peers.
> Instead of using a default value (suitable for PCIe devices)
> derived from the WMI op version, a per target value can be
> used instead.
>
> This is needed by the QCA9377 USB device in order to prevent
> the target fw to crash after HTT RX ring cfg is issued.
>
> Apparently, the QCA9377 HL device does not seem to handle the
> same amount of peers as the LL devices.
>
> Signed-off-by: Erik Stromdahl <[email protected]>

I was a bit torn about this, I definitely see the need for this but on
the other hand it creates duplicate data (for example two entries for
QCA9377 chip). I guess this is the right approach, at least I cannot
come up anything better.

But this patch should be split into two:

1) add bus field to struct ath10k_hw_params

2) add max_num_peers field to struct ath10k_hw_params

And it seems 2) is already implemented in commit 9f2992fea580 ("ath10k:
wmi: get wmi init parameter values from hw params"), so hopefully we
only need 1) anymore.

> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -1663,9 +1663,19 @@ static int ath10k_init_hw_params(struct ath10k *ar=
)
> for (i =3D 0; i < ARRAY_SIZE(ath10k_hw_params_list); i++) {
> hw_params =3D &ath10k_hw_params_list[i];
> =20
> - if (hw_params->id =3D=3D ar->target_version &&
> - hw_params->dev_id =3D=3D ar->dev_id)
> - break;
> + if (ar->is_high_latency) {
> + /* High latency devices will use different fw depending
> + * on if it is a USB or SDIO device.
> + */
> + if (hw_params->bus =3D=3D ar->hif.bus &&
> + hw_params->id =3D=3D ar->target_version &&
> + hw_params->dev_id =3D=3D ar->dev_id)
> + break;
> + } else {
> + if (hw_params->id =3D=3D ar->target_version &&
> + hw_params->dev_id =3D=3D ar->dev_id)
> + break;
> + }

I don't like the is_high_latency test here at all. The bus field should
be checked with all entries, not just high latency ones. And because of
this even most of the hw_param bus field entries were not initialised.

So only thing to do is to initialise the bus field for all the entries
and the ugly test here can be removed. Just remember that QCA4019 uses
AHB, I think all the rest is PCI. Or do we have AHB devices supported?

> @@ -550,6 +557,18 @@ struct ath10k_hw_params {
> */
> int vht160_mcs_rx_highest;
> int vht160_mcs_tx_highest;
> +
> + /* max_num_peers can be used to override the setting derived from
> + * the WMI op version. If this value is non-zero, it will always
> + * be used instead of the default value derived from the WMI op
> + * version.
> + */
> + int max_num_peers;
> +
> + /* Specifies whether or not the device is a high latency device */
> + bool is_high_latency;
> +
> + enum ath10k_bus bus;
> };

Please move the bus field between dev_id and name fields. It's so
important that it should not be in the end.

--=20
Kalle Valo=

2018-01-08 13:41:56

by Kalle Valo

[permalink] [raw]
Subject: Re: [RFC v3 03/11] ath10k: per target configurablity of various items

Erik Stromdahl <[email protected]> writes:

> On 2017-12-22 16:19, Kalle Valo wrote:
>
>> I was a bit torn about this, I definitely see the need for this but on
>> the other hand it creates duplicate data (for example two entries for
>> QCA9377 chip). I guess this is the right approach, at least I cannot
>> come up anything better.
>>
>> But this patch should be split into two:
>>
>> 1) add bus field to struct ath10k_hw_params
>>
>> 2) add max_num_peers field to struct ath10k_hw_params
>>
>> And it seems 2) is already implemented in commit 9f2992fea580 ("ath10k:
>> wmi: get wmi init parameter values from hw params"), so hopefully we
>> only need 1) anymore.
>>
>
> Before commit 9f2992fea580a48135591873e5e3ac7e01444207,
> TARGET_TLV_NUM_PEERS was used both in the WMI TLV init command
> and as the value of *max_num_peers* in *struct ath10k* (ar->max_num_peers).
>
> commit 9f2992fea580a48135591873e5e3ac7e01444207 does not set
> *ar->max_num_peers* to the value of *ar->hw_param->num_peers*.
>
> Is this correct?
>
> As I see it, there is a possible mismatch between what is written
> to the device in the WMI init message and the value of *ar->max_num_peers*.
>
> Do we still need *max_num_peers* in *struct ath10k* now that we have the
> *num_peers* member in *struct ath10k_hw_params*?

A good point, I didn't thought of that during review. No time to
investigate this right now, but maybe Rakesh and Govind (CCed) can
comment?

> I am currently rewriting my HL patches and I was thinking about adding
> a separate patch related to this.

Yeah, a separate patch to sort that out is a good idea.

>>> --- a/drivers/net/wireless/ath/ath10k/core.c
>>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>>> @@ -1663,9 +1663,19 @@ static int ath10k_init_hw_params(struct ath10k *ar)
>>> for (i = 0; i < ARRAY_SIZE(ath10k_hw_params_list); i++) {
>>> hw_params = &ath10k_hw_params_list[i];
>>> - if (hw_params->id == ar->target_version &&
>>> - hw_params->dev_id == ar->dev_id)
>>> - break;
>>> + if (ar->is_high_latency) {
>>> + /* High latency devices will use different fw depending
>>> + * on if it is a USB or SDIO device.
>>> + */
>>> + if (hw_params->bus == ar->hif.bus &&
>>> + hw_params->id == ar->target_version &&
>>> + hw_params->dev_id == ar->dev_id)
>>> + break;
>>> + } else {
>>> + if (hw_params->id == ar->target_version &&
>>> + hw_params->dev_id == ar->dev_id)
>>> + break;
>>> + }
>>
>> I don't like the is_high_latency test here at all. The bus field should
>> be checked with all entries, not just high latency ones. And because of
>> this even most of the hw_param bus field entries were not initialised.
>>
>> So only thing to do is to initialise the bus field for all the entries
>> and the ugly test here can be removed. Just remember that QCA4019 uses
>> AHB, I think all the rest is PCI. Or do we have AHB devices supported?
>
> I noticed that there has been introduced a new bus type (SNOC).
> Do you know which devices are SNOC devices?

SNOC is for wcn3990.

> Btw, what the heck is SNOC anyway?

I have forgetten already what the acronym meant but it's basically some
sort of shared memory communication method with the firmware.

--
Kalle Valo

2018-01-08 14:03:49

by Govind Singh

[permalink] [raw]
Subject: RE: [RFC v3 03/11] ath10k: per target configurablity of various items

>> A good point, I didn't thought of that during review. No time to investi=
gate this right now, but maybe Rakesh and Govind (CCed) can comment?
Yes, ar->max_num_peers needs to be assigned with ar->hw_param->num_peers. T=
his can create mismatch for wcn3990 target if we=20
create multiple peers( TARGET_HL_10_TLV_NUM_PEERS vs TARGET_TLV_NUM_PEERS).=
We will fix this and raise this as separate change.

>>> Btw, what the heck is SNOC anyway?
SNOC is system NOC(network on chip). WCN3990 is integrated chipset connecte=
d over SNOC and only RF part is discrete to the SoC.

BR,
Govind

-----Original Message-----
From: Kalle Valo [mailto:[email protected]]=20
Sent: Monday, January 8, 2018 7:12 PM
To: Erik Stromdahl <[email protected]>
Cc: [email protected]; [email protected]; Rakesh Pill=
ai <[email protected]>; Govind Singh <[email protected]>
Subject: Re: [RFC v3 03/11] ath10k: per target configurablity of various it=
ems

Erik Stromdahl <[email protected]> writes:

> On 2017-12-22 16:19, Kalle Valo wrote:
>
>> I was a bit torn about this, I definitely see the need for this but=20
>> on the other hand it creates duplicate data (for example two entries=20
>> for
>> QCA9377 chip). I guess this is the right approach, at least I cannot=20
>> come up anything better.
>>
>> But this patch should be split into two:
>>
>> 1) add bus field to struct ath10k_hw_params
>>
>> 2) add max_num_peers field to struct ath10k_hw_params
>>
>> And it seems 2) is already implemented in commit 9f2992fea580 ("ath10k:
>> wmi: get wmi init parameter values from hw params"), so hopefully we=20
>> only need 1) anymore.
>>
>
> Before commit 9f2992fea580a48135591873e5e3ac7e01444207,
> TARGET_TLV_NUM_PEERS was used both in the WMI TLV init command and as=20
> the value of *max_num_peers* in *struct ath10k* (ar->max_num_peers).
>
> commit 9f2992fea580a48135591873e5e3ac7e01444207 does not set
> *ar->max_num_peers* to the value of *ar->hw_param->num_peers*.
>
> Is this correct?
>
> As I see it, there is a possible mismatch between what is written to=20
> the device in the WMI init message and the value of *ar->max_num_peers*.
>
> Do we still need *max_num_peers* in *struct ath10k* now that we have=20
> the
> *num_peers* member in *struct ath10k_hw_params*?

A good point, I didn't thought of that during review. No time to investigat=
e this right now, but maybe Rakesh and Govind (CCed) can comment?

> I am currently rewriting my HL patches and I was thinking about adding=20
> a separate patch related to this.

Yeah, a separate patch to sort that out is a good idea.

>>> --- a/drivers/net/wireless/ath/ath10k/core.c
>>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>>> @@ -1663,9 +1663,19 @@ static int ath10k_init_hw_params(struct ath10k *=
ar)
>>> for (i =3D 0; i < ARRAY_SIZE(ath10k_hw_params_list); i++) {
>>> hw_params =3D &ath10k_hw_params_list[i];
>>> - if (hw_params->id =3D=3D ar->target_version &&
>>> - hw_params->dev_id =3D=3D ar->dev_id)
>>> - break;
>>> + if (ar->is_high_latency) {
>>> + /* High latency devices will use different fw depending
>>> + * on if it is a USB or SDIO device.
>>> + */
>>> + if (hw_params->bus =3D=3D ar->hif.bus &&
>>> + hw_params->id =3D=3D ar->target_version &&
>>> + hw_params->dev_id =3D=3D ar->dev_id)
>>> + break;
>>> + } else {
>>> + if (hw_params->id =3D=3D ar->target_version &&
>>> + hw_params->dev_id =3D=3D ar->dev_id)
>>> + break;
>>> + }
>>
>> I don't like the is_high_latency test here at all. The bus field=20
>> should be checked with all entries, not just high latency ones. And=20
>> because of this even most of the hw_param bus field entries were not ini=
tialised.
>>
>> So only thing to do is to initialise the bus field for all the=20
>> entries and the ugly test here can be removed. Just remember that=20
>> QCA4019 uses AHB, I think all the rest is PCI. Or do we have AHB devices=
supported?
>
> I noticed that there has been introduced a new bus type (SNOC).
> Do you know which devices are SNOC devices?

SNOC is for wcn3990.

> Btw, what the heck is SNOC anyway?

I have forgetten already what the acronym meant but it's basically some sor=
t of shared memory communication method with the firmware.

--
Kalle Valo