Hi John,
Please ignore previous submission titled "[PATCH 0/3] wil6210 updates"
I found better solution for one of patches there. To avoid mis-interpretation,
resending all relevant patches. It is:
- Sync with latest firmware updates
- simple bug fixes and some optimization, all for data path
- prepare for various offloads - WIP by Erez
Vladimir Kondratiev (5):
wil6210: Align WMI header with latest FW
wil6210: fix wrong index in wil_vring_free
wil6210: Optimize Tx completion
wil6210: Introduce struct for sw context
wil6210: fix subtle race in wil_tx_vring
drivers/net/wireless/ath/wil6210/debugfs.c | 4 +-
drivers/net/wireless/ath/wil6210/trace.h | 22 +++++----
drivers/net/wireless/ath/wil6210/txrx.c | 79 +++++++++++++++++-------------
drivers/net/wireless/ath/wil6210/wil6210.h | 27 ++++++++--
drivers/net/wireless/ath/wil6210/wmi.c | 14 ++++--
5 files changed, 92 insertions(+), 54 deletions(-)
--
1.8.1.2
Finish all SW context modifications prior to notifying hardware
It used to be race condition: if HW finish Tx and issue Tx completion IRQ very fast,
prior to SW context update in wil_tx_vring, Tx completion will mis-handle descriptor, as
SW part will have no skb pointer stored.
Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/txrx.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index 44cdd2a..2a9d56a 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -712,6 +712,12 @@ static int wil_tx_vring(struct wil6210_priv *wil, struct vring *vring,
d->dma.d0 |= BIT(DMA_CFG_DESC_TX_0_CMD_DMA_IT_POS);
*_d = *d;
+ /* hold reference to skb
+ * to prevent skb release before accounting
+ * in case of immediate "tx done"
+ */
+ vring->ctx[i].skb = skb_get(skb);
+
wil_hex_dump_txrx("Tx ", DUMP_PREFIX_NONE, 32, 4,
(const void *)d, sizeof(*d), false);
@@ -720,11 +726,6 @@ static int wil_tx_vring(struct wil6210_priv *wil, struct vring *vring,
wil_dbg_txrx(wil, "Tx swhead %d -> %d\n", swhead, vring->swhead);
trace_wil6210_tx(vring_index, swhead, skb->len, nr_frags);
iowrite32(vring->swhead, wil->csr + HOSTADDR(vring->hwtail));
- /* hold reference to skb
- * to prevent skb release before accounting
- * in case of immediate "tx done"
- */
- vring->ctx[i].skb = skb_get(skb);
return 0;
dma_error:
--
1.8.1.2
Enable adding more data to the SW context.
For now, add flag "mapped_as_page", to separate decisions on free-ing skb
and type of DMA mapping.
This allows linking skb itself to any descriptor of fragmented skb.
Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 4 +--
drivers/net/wireless/ath/wil6210/txrx.c | 58 +++++++++++++++++-------------
drivers/net/wireless/ath/wil6210/wil6210.h | 10 +++++-
3 files changed, 44 insertions(+), 28 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index e8308ec..971ce46 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -51,7 +51,7 @@ static void wil_print_vring(struct seq_file *s, struct wil6210_priv *wil,
if ((i % 64) == 0 && (i != 0))
seq_printf(s, "\n");
seq_printf(s, "%s", (d->dma.status & BIT(0)) ?
- "S" : (vring->ctx[i] ? "H" : "h"));
+ "S" : (vring->ctx[i].skb ? "H" : "h"));
}
seq_printf(s, "\n");
}
@@ -406,7 +406,7 @@ static int wil_txdesc_debugfs_show(struct seq_file *s, void *data)
volatile struct vring_tx_desc *d =
&(vring->va[dbg_txdesc_index].tx);
volatile u32 *u = (volatile u32 *)d;
- struct sk_buff *skb = vring->ctx[dbg_txdesc_index];
+ struct sk_buff *skb = vring->ctx[dbg_txdesc_index].skb;
seq_printf(s, "Tx[%3d] = {\n", dbg_txdesc_index);
seq_printf(s, " MAC = 0x%08x 0x%08x 0x%08x 0x%08x\n",
diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index dd5f35e..44cdd2a 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -70,7 +70,7 @@ static int wil_vring_alloc(struct wil6210_priv *wil, struct vring *vring)
vring->swhead = 0;
vring->swtail = 0;
- vring->ctx = kzalloc(vring->size * sizeof(vring->ctx[0]), GFP_KERNEL);
+ vring->ctx = kcalloc(vring->size, sizeof(vring->ctx[0]), GFP_KERNEL);
if (!vring->ctx) {
vring->va = NULL;
return -ENOMEM;
@@ -108,39 +108,39 @@ static void wil_vring_free(struct wil6210_priv *wil, struct vring *vring,
while (!wil_vring_is_empty(vring)) {
dma_addr_t pa;
- struct sk_buff *skb;
u16 dmalen;
+ struct wil_ctx *ctx;
if (tx) {
struct vring_tx_desc dd, *d = ⅆ
volatile struct vring_tx_desc *_d =
&vring->va[vring->swtail].tx;
+ ctx = &vring->ctx[vring->swtail];
*d = *_d;
pa = wil_desc_addr(&d->dma.addr);
dmalen = le16_to_cpu(d->dma.length);
- skb = vring->ctx[vring->swtail];
- if (skb) {
- dma_unmap_single(dev, pa, dmalen,
- DMA_TO_DEVICE);
- dev_kfree_skb_any(skb);
- vring->ctx[vring->swtail] = NULL;
- } else {
+ if (vring->ctx[vring->swtail].mapped_as_page) {
dma_unmap_page(dev, pa, dmalen,
DMA_TO_DEVICE);
+ } else {
+ dma_unmap_single(dev, pa, dmalen,
+ DMA_TO_DEVICE);
}
+ if (ctx->skb)
+ dev_kfree_skb_any(ctx->skb);
vring->swtail = wil_vring_next_tail(vring);
} else { /* rx */
struct vring_rx_desc dd, *d = ⅆ
volatile struct vring_rx_desc *_d =
&vring->va[vring->swhead].rx;
+ ctx = &vring->ctx[vring->swhead];
*d = *_d;
pa = wil_desc_addr(&d->dma.addr);
dmalen = le16_to_cpu(d->dma.length);
- skb = vring->ctx[vring->swhead];
dma_unmap_single(dev, pa, dmalen, DMA_FROM_DEVICE);
- kfree_skb(skb);
+ kfree_skb(ctx->skb);
wil_vring_advance_head(vring, 1);
}
}
@@ -187,7 +187,7 @@ static int wil_vring_alloc_skb(struct wil6210_priv *wil, struct vring *vring,
d->dma.status = 0; /* BIT(0) should be 0 for HW_OWNED */
d->dma.length = cpu_to_le16(sz);
*_d = *d;
- vring->ctx[i] = skb;
+ vring->ctx[i].skb = skb;
return 0;
}
@@ -352,11 +352,11 @@ static struct sk_buff *wil_vring_reap_rx(struct wil6210_priv *wil,
return NULL;
}
- skb = vring->ctx[vring->swhead];
+ skb = vring->ctx[vring->swhead].skb;
d = wil_skb_rxdesc(skb);
*d = *_d;
pa = wil_desc_addr(&d->dma.addr);
- vring->ctx[vring->swhead] = NULL;
+ vring->ctx[vring->swhead].skb = NULL;
wil_vring_advance_head(vring, 1);
dma_unmap_single(dev, pa, sz, DMA_FROM_DEVICE);
@@ -703,7 +703,7 @@ static int wil_tx_vring(struct wil6210_priv *wil, struct vring *vring,
if (unlikely(dma_mapping_error(dev, pa)))
goto dma_error;
wil_tx_desc_map(d, pa, len, vring_index);
- vring->ctx[i] = NULL;
+ vring->ctx[i].mapped_as_page = 1;
*_d = *d;
}
/* for the last seg only */
@@ -724,7 +724,7 @@ static int wil_tx_vring(struct wil6210_priv *wil, struct vring *vring,
* to prevent skb release before accounting
* in case of immediate "tx done"
*/
- vring->ctx[i] = skb_get(skb);
+ vring->ctx[i].skb = skb_get(skb);
return 0;
dma_error:
@@ -732,6 +732,7 @@ static int wil_tx_vring(struct wil6210_priv *wil, struct vring *vring,
/* Note: increment @f to operate with positive index */
for (f++; f > 0; f--) {
u16 dmalen;
+ struct wil_ctx *ctx = &vring->ctx[i];
i = (swhead + f) % vring->size;
_d = &(vring->va[i].tx);
@@ -739,10 +740,15 @@ static int wil_tx_vring(struct wil6210_priv *wil, struct vring *vring,
_d->dma.status = TX_DMA_STATUS_DU;
pa = wil_desc_addr(&d->dma.addr);
dmalen = le16_to_cpu(d->dma.length);
- if (vring->ctx[i])
- dma_unmap_single(dev, pa, dmalen, DMA_TO_DEVICE);
- else
+ if (ctx->mapped_as_page)
dma_unmap_page(dev, pa, dmalen, DMA_TO_DEVICE);
+ else
+ dma_unmap_single(dev, pa, dmalen, DMA_TO_DEVICE);
+
+ if (ctx->skb)
+ dev_kfree_skb_any(ctx->skb);
+
+ memset(ctx, 0, sizeof(*ctx));
}
return -EINVAL;
@@ -821,8 +827,9 @@ int wil_tx_complete(struct wil6210_priv *wil, int ringid)
&vring->va[vring->swtail].tx;
struct vring_tx_desc dd, *d = ⅆ
dma_addr_t pa;
- struct sk_buff *skb;
u16 dmalen;
+ struct wil_ctx *ctx = &vring->ctx[vring->swtail];
+ struct sk_buff *skb = ctx->skb;
*d = *_d;
@@ -840,7 +847,11 @@ int wil_tx_complete(struct wil6210_priv *wil, int ringid)
(const void *)d, sizeof(*d), false);
pa = wil_desc_addr(&d->dma.addr);
- skb = vring->ctx[vring->swtail];
+ if (ctx->mapped_as_page)
+ dma_unmap_page(dev, pa, dmalen, DMA_TO_DEVICE);
+ else
+ dma_unmap_single(dev, pa, dmalen, DMA_TO_DEVICE);
+
if (skb) {
if (d->dma.error == 0) {
ndev->stats.tx_packets++;
@@ -849,12 +860,9 @@ int wil_tx_complete(struct wil6210_priv *wil, int ringid)
ndev->stats.tx_errors++;
}
- dma_unmap_single(dev, pa, dmalen, DMA_TO_DEVICE);
dev_kfree_skb_any(skb);
- vring->ctx[vring->swtail] = NULL;
- } else {
- dma_unmap_page(dev, pa, dmalen, DMA_TO_DEVICE);
}
+ memset(ctx, 0, sizeof(*ctx));
/*
* There is no need to touch HW descriptor:
* - ststus bit TX_DMA_STATUS_DU is set by design,
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 129c480..c4a5163 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -183,6 +183,14 @@ struct pending_wmi_event {
} __packed event;
};
+/**
+ * struct wil_ctx - software context for Vring descriptor
+ */
+struct wil_ctx {
+ struct sk_buff *skb;
+ u8 mapped_as_page:1;
+};
+
union vring_desc;
struct vring {
@@ -192,7 +200,7 @@ struct vring {
u32 swtail;
u32 swhead;
u32 hwtail; /* write here to inform hw */
- void **ctx; /* void *ctx[size] - software context */
+ struct wil_ctx *ctx; /* ctx[size] - software context */
};
enum { /* for wil6210_priv.status */
--
1.8.1.2
When destroying Rx vring, branch for Rx used wrong Tx descriptor:
while SW context was taken for "head", HW descriptor was, by mistake,
taken from "tail"
Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/txrx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index d240b24..8fde73a 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -133,7 +133,7 @@ static void wil_vring_free(struct wil6210_priv *wil, struct vring *vring,
} else { /* rx */
struct vring_rx_desc dd, *d = ⅆ
volatile struct vring_rx_desc *_d =
- &vring->va[vring->swtail].rx;
+ &vring->va[vring->swhead].rx;
*d = *_d;
pa = wil_desc_addr(&d->dma.addr);
--
1.8.1.2
FW guys changed header structure; align driver code
Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/trace.h | 22 +++++++++++++---------
drivers/net/wireless/ath/wil6210/wil6210.h | 17 ++++++++++++++---
drivers/net/wireless/ath/wil6210/wmi.c | 14 +++++++++-----
3 files changed, 36 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/trace.h b/drivers/net/wireless/ath/wil6210/trace.h
index eff1239..e59239d 100644
--- a/drivers/net/wireless/ath/wil6210/trace.h
+++ b/drivers/net/wireless/ath/wil6210/trace.h
@@ -37,36 +37,40 @@ static inline void trace_ ## name(proto) {}
#endif /* !CONFIG_WIL6210_TRACING || defined(__CHECKER__) */
DECLARE_EVENT_CLASS(wil6210_wmi,
- TP_PROTO(u16 id, void *buf, u16 buf_len),
+ TP_PROTO(struct wil6210_mbox_hdr_wmi *wmi, void *buf, u16 buf_len),
- TP_ARGS(id, buf, buf_len),
+ TP_ARGS(wmi, buf, buf_len),
TP_STRUCT__entry(
+ __field(u8, mid)
__field(u16, id)
+ __field(u32, timestamp)
__field(u16, buf_len)
__dynamic_array(u8, buf, buf_len)
),
TP_fast_assign(
- __entry->id = id;
+ __entry->mid = wmi->mid;
+ __entry->id = le16_to_cpu(wmi->id);
+ __entry->timestamp = le32_to_cpu(wmi->timestamp);
__entry->buf_len = buf_len;
memcpy(__get_dynamic_array(buf), buf, buf_len);
),
TP_printk(
- "id 0x%04x len %d",
- __entry->id, __entry->buf_len
+ "MID %d id 0x%04x len %d timestamp %d",
+ __entry->mid, __entry->id, __entry->buf_len, __entry->timestamp
)
);
DEFINE_EVENT(wil6210_wmi, wil6210_wmi_cmd,
- TP_PROTO(u16 id, void *buf, u16 buf_len),
- TP_ARGS(id, buf, buf_len)
+ TP_PROTO(struct wil6210_mbox_hdr_wmi *wmi, void *buf, u16 buf_len),
+ TP_ARGS(wmi, buf, buf_len)
);
DEFINE_EVENT(wil6210_wmi, wil6210_wmi_event,
- TP_PROTO(u16 id, void *buf, u16 buf_len),
- TP_ARGS(id, buf, buf_len)
+ TP_PROTO(struct wil6210_mbox_hdr_wmi *wmi, void *buf, u16 buf_len),
+ TP_ARGS(wmi, buf, buf_len)
);
#define WIL6210_MSG_MAX (200)
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 44fdab5..129c480 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -156,11 +156,22 @@ struct wil6210_mbox_hdr {
/* max. value for wil6210_mbox_hdr.len */
#define MAX_MBOXITEM_SIZE (240)
+/**
+ * struct wil6210_mbox_hdr_wmi - WMI header
+ *
+ * @mid: MAC ID
+ * 00 - default, created by FW
+ * 01..0f - WiFi ports, driver to create
+ * 10..fe - debug
+ * ff - broadcast
+ * @id: command/event ID
+ * @timestamp: FW fills for events, free-running msec timer
+ */
struct wil6210_mbox_hdr_wmi {
- u8 reserved0[2];
+ u8 mid;
+ u8 reserved;
__le16 id;
- __le16 info1; /* bits [0..3] - device_id, rest - unused */
- u8 reserved1[2];
+ __le32 timestamp;
} __packed;
struct pending_wmi_event {
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index dc8059a..a62511a 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -172,8 +172,8 @@ static int __wmi_send(struct wil6210_priv *wil, u16 cmdid, void *buf, u16 len)
.len = cpu_to_le16(sizeof(cmd.wmi) + len),
},
.wmi = {
+ .mid = 0,
.id = cpu_to_le16(cmdid),
- .info1 = 0,
},
};
struct wil6210_mbox_ring *r = &wil->mbox_ctl.tx;
@@ -248,7 +248,7 @@ static int __wmi_send(struct wil6210_priv *wil, u16 cmdid, void *buf, u16 len)
iowrite32(r->head = next_head, wil->csr + HOST_MBOX +
offsetof(struct wil6210_mbox_ctl, tx.head));
- trace_wil6210_wmi_cmd(cmdid, buf, len);
+ trace_wil6210_wmi_cmd(&cmd.wmi, buf, len);
/* interrupt to FW */
iowrite32(SW_INT_MBOX, wil->csr + HOST_SW_INT);
@@ -640,9 +640,13 @@ void wmi_recv_cmd(struct wil6210_priv *wil)
hdr.flags);
if ((hdr.type == WIL_MBOX_HDR_TYPE_WMI) &&
(len >= sizeof(struct wil6210_mbox_hdr_wmi))) {
- u16 id = le16_to_cpu(evt->event.wmi.id);
- wil_dbg_wmi(wil, "WMI event 0x%04x\n", id);
- trace_wil6210_wmi_event(id, &evt->event.wmi, len);
+ struct wil6210_mbox_hdr_wmi *wmi = &evt->event.wmi;
+ u16 id = le16_to_cpu(wmi->id);
+ u32 tstamp = le32_to_cpu(wmi->timestamp);
+ wil_dbg_wmi(wil, "WMI event 0x%04x MID %d @%d msec\n",
+ id, wmi->mid, tstamp);
+ trace_wil6210_wmi_event(wmi, &wmi[1],
+ len - sizeof(*wmi));
}
wil_hex_dump_wmi("evt ", DUMP_PREFIX_OFFSET, 16, 1,
&evt->event.hdr, sizeof(hdr) + len, true);
--
1.8.1.2
No need to modify HW descriptor, as it will be re-initialized on Tx.
Signed-off-by: Vladimir Kondratiev <[email protected]>
---
drivers/net/wireless/ath/wil6210/txrx.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index 8fde73a..dd5f35e 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -855,10 +855,12 @@ int wil_tx_complete(struct wil6210_priv *wil, int ringid)
} else {
dma_unmap_page(dev, pa, dmalen, DMA_TO_DEVICE);
}
- d->dma.addr.addr_low = 0;
- d->dma.addr.addr_high = 0;
- d->dma.length = 0;
- d->dma.status = TX_DMA_STATUS_DU;
+ /*
+ * There is no need to touch HW descriptor:
+ * - ststus bit TX_DMA_STATUS_DU is set by design,
+ * so hardware will not try to process this desc.,
+ * - rest of descriptor will be initialized on Tx.
+ */
vring->swtail = wil_vring_next_tail(vring);
done++;
}
--
1.8.1.2