* [PATCH v2 0/3]
@ 2020-07-31 18:27 ` Rakesh Pillai
0 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-07-31 18:27 UTC (permalink / raw)
To: ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev,
Rakesh Pillai
The history recording will be compiled only if
ATH10K_DEBUG is enabled, and also enabled via
the module parameter. Once the history recording
is enabled via module parameter, it can be enabled
or disabled runtime via debugfs.
---
Changes from v1:
- Add module param and debugfs to enable/disable history recording.
Rakesh Pillai (3):
ath10k: Add history for tracking certain events
ath10k: Add module param to enable history
ath10k: Add debugfs support to enable event history
drivers/net/wireless/ath/ath10k/ce.c | 1 +
drivers/net/wireless/ath/ath10k/core.c | 3 +
drivers/net/wireless/ath/ath10k/core.h | 82 ++++++++++++
drivers/net/wireless/ath/ath10k/debug.c | 207 ++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/debug.h | 75 +++++++++++
drivers/net/wireless/ath/ath10k/snoc.c | 15 ++-
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
drivers/net/wireless/ath/ath10k/wmi.c | 10 ++
8 files changed, 393 insertions(+), 1 deletion(-)
--
2.7.4
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 1/3] ath10k: Add history for tracking certain events
2020-07-31 18:27 ` Rakesh Pillai
@ 2020-07-31 18:27 ` Rakesh Pillai
-1 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-07-31 18:27 UTC (permalink / raw)
To: ath10k
Cc: netdev, linux-wireless, linux-kernel, Rakesh Pillai, kuba, davem,
kvalo
Add history for tracking the below events
- register read
- register write
- IRQ trigger
- NAPI poll
- CE service
- WMI cmd
- WMI event
- WMI tx completion
This will help in debugging any crash or any
improper behaviour.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/ce.c | 1 +
drivers/net/wireless/ath/ath10k/core.h | 74 +++++++++++++++++
drivers/net/wireless/ath/ath10k/debug.c | 133 ++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/debug.h | 74 +++++++++++++++++
drivers/net/wireless/ath/ath10k/snoc.c | 15 +++-
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
7 files changed, 307 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 84ec80c..0f541de 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -1299,6 +1299,7 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
u32 ctrl_addr = ce_state->ctrl_addr;
+ ath10k_record_ce_event(ar, ATH10K_CE_SERVICE, ce_id);
/*
* Clear before handling
*
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 5c18f6c..46bd5aa 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -970,6 +970,75 @@ struct ath10k_bus_params {
bool hl_msdu_ids;
};
+#define ATH10K_REG_ACCESS_HISTORY_MAX 512
+#define ATH10K_CE_EVENT_HISTORY_MAX 1024
+#define ATH10K_WMI_EVENT_HISTORY_MAX 512
+#define ATH10K_WMI_CMD_HISTORY_MAX 256
+
+#define ATH10K_WMI_DATA_LEN 16
+
+enum ath10k_ce_event {
+ ATH10K_IRQ_TRIGGER,
+ ATH10K_NAPI_POLL,
+ ATH10K_CE_SERVICE,
+ ATH10K_NAPI_COMPLETE,
+ ATH10K_NAPI_RESCHED,
+ ATH10K_IRQ_SUMMARY,
+};
+
+enum ath10k_wmi_type {
+ ATH10K_WMI_EVENT,
+ ATH10K_WMI_CMD,
+ ATH10K_WMI_TX_COMPL,
+};
+
+struct ath10k_reg_access_entry {
+ u32 cpu_id;
+ bool write;
+ u32 offset;
+ u32 val;
+ u64 timestamp;
+};
+
+struct ath10k_wmi_event_entry {
+ u32 cpu_id;
+ enum ath10k_wmi_type type;
+ u32 id;
+ u64 timestamp;
+ unsigned char data[ATH10K_WMI_DATA_LEN];
+};
+
+struct ath10k_ce_event_entry {
+ u32 cpu_id;
+ enum ath10k_ce_event event_type;
+ u32 ce_id;
+ u64 timestamp;
+};
+
+struct ath10k_wmi_event_history {
+ struct ath10k_wmi_event_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing wmi event history */
+ spinlock_t hist_lock;
+};
+
+struct ath10k_ce_event_history {
+ struct ath10k_ce_event_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing ce event history */
+ spinlock_t hist_lock;
+};
+
+struct ath10k_reg_access_history {
+ struct ath10k_reg_access_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing register access history */
+ spinlock_t hist_lock;
+};
+
struct ath10k {
struct ath_common ath_common;
struct ieee80211_hw *hw;
@@ -1261,6 +1330,11 @@ struct ath10k {
bool coex_support;
int coex_gpio_pin;
+ struct ath10k_reg_access_history reg_access_history;
+ struct ath10k_ce_event_history ce_event_history;
+ struct ath10k_wmi_event_history wmi_event_history;
+ struct ath10k_wmi_event_history wmi_cmd_history;
+
/* must be last */
u8 drv_priv[] __aligned(sizeof(void *));
};
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index e8250a6..9105b0b 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2722,4 +2722,137 @@ void ath10k_dbg_dump(struct ath10k *ar,
}
EXPORT_SYMBOL(ath10k_dbg_dump);
+int ath10k_core_reg_access_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->reg_access_history.record = vzalloc(max_entries *
+ sizeof(struct ath10k_reg_access_entry));
+ if (!ar->reg_access_history.record)
+ return -ENOMEM;
+
+ ar->reg_access_history.max_entries = max_entries;
+ atomic_set(&ar->reg_access_history.index, 0);
+ spin_lock_init(&ar->reg_access_history.hist_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_reg_access_history_init);
+
+int ath10k_core_wmi_cmd_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->wmi_cmd_history.record = vzalloc(max_entries *
+ sizeof(struct ath10k_wmi_event_entry));
+ if (!ar->wmi_cmd_history.record)
+ return -ENOMEM;
+
+ ar->wmi_cmd_history.max_entries = max_entries;
+ atomic_set(&ar->wmi_cmd_history.index, 0);
+ spin_lock_init(&ar->wmi_cmd_history.hist_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_wmi_cmd_history_init);
+
+int ath10k_core_wmi_event_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->wmi_event_history.record = vzalloc(max_entries *
+ sizeof(struct ath10k_wmi_event_entry));
+ if (!ar->wmi_event_history.record)
+ return -ENOMEM;
+
+ ar->wmi_event_history.max_entries = max_entries;
+ atomic_set(&ar->wmi_event_history.index, 0);
+ spin_lock_init(&ar->wmi_event_history.hist_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_wmi_event_history_init);
+
+int ath10k_core_ce_event_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->ce_event_history.record = vzalloc(max_entries *
+ sizeof(struct ath10k_ce_event_entry));
+ if (!ar->ce_event_history.record)
+ return -ENOMEM;
+
+ ar->ce_event_history.max_entries = max_entries;
+ atomic_set(&ar->ce_event_history.index, 0);
+ spin_lock_init(&ar->ce_event_history.hist_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_ce_event_history_init);
+
+void ath10k_record_reg_access(struct ath10k *ar, u32 offset, u32 val, bool write)
+{
+ struct ath10k_reg_access_entry *entry;
+ u32 idx;
+
+ if (!ar->reg_access_history.record)
+ return;
+
+ idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
+ ar->reg_access_history.max_entries);
+ entry = &ar->reg_access_history.record[idx];
+
+ entry->timestamp = ath10k_core_get_timestamp();
+ entry->write = write;
+ entry->offset = offset;
+ entry->val = val;
+}
+EXPORT_SYMBOL(ath10k_record_reg_access);
+
+void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
+ u32 id, unsigned char *data)
+{
+ struct ath10k_wmi_event_entry *entry;
+ u32 idx;
+
+ if (type == ATH10K_WMI_EVENT) {
+ if (!ar->wmi_event_history.record)
+ return;
+
+ spin_lock_bh(&ar->wmi_event_history.hist_lock);
+ idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
+ ar->wmi_event_history.max_entries);
+ spin_unlock_bh(&ar->wmi_event_history.hist_lock);
+ entry = &ar->wmi_event_history.record[idx];
+ } else {
+ if (!ar->wmi_cmd_history.record)
+ return;
+
+ spin_lock_bh(&ar->wmi_cmd_history.hist_lock);
+ idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
+ ar->wmi_cmd_history.max_entries);
+ spin_unlock_bh(&ar->wmi_cmd_history.hist_lock);
+ entry = &ar->wmi_cmd_history.record[idx];
+ }
+
+ entry->timestamp = ath10k_core_get_timestamp();
+ entry->cpu_id = smp_processor_id();
+ entry->type = type;
+ entry->id = id;
+ memcpy(&entry->data, data + 4, ATH10K_WMI_DATA_LEN);
+}
+EXPORT_SYMBOL(ath10k_record_wmi_event);
+
+void ath10k_record_ce_event(struct ath10k *ar, enum ath10k_ce_event event_type,
+ int ce_id)
+{
+ struct ath10k_ce_event_entry *entry;
+ u32 idx;
+
+ if (!ar->ce_event_history.record)
+ return;
+
+ idx = ath10k_core_get_next_idx(&ar->ce_event_history.index,
+ ar->ce_event_history.max_entries);
+ entry = &ar->ce_event_history.record[idx];
+
+ entry->timestamp = ath10k_core_get_timestamp();
+ entry->cpu_id = smp_processor_id();
+ entry->event_type = event_type;
+ entry->ce_id = ce_id;
+}
+EXPORT_SYMBOL(ath10k_record_ce_event);
+
#endif /* CONFIG_ATH10K_DEBUG */
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index 997c1c8..c28aeb1 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -258,6 +258,38 @@ void ath10k_dbg_dump(struct ath10k *ar,
enum ath10k_debug_mask mask,
const char *msg, const char *prefix,
const void *buf, size_t len);
+
+/* ========== History init APIs =========== */
+int ath10k_core_reg_access_history_init(struct ath10k *ar, u32 max_entries);
+int ath10k_core_wmi_cmd_history_init(struct ath10k *ar, u32 max_entries);
+int ath10k_core_wmi_event_history_init(struct ath10k *ar, u32 max_entries);
+int ath10k_core_ce_event_history_init(struct ath10k *ar, u32 max_entries);
+
+/* ========== History record APIs =========== */
+void ath10k_record_reg_access(struct ath10k *ar, u32 offset, u32 val,
+ bool write);
+void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
+ u32 id, unsigned char *data);
+void ath10k_record_ce_event(struct ath10k *ar,
+ enum ath10k_ce_event event_type,
+ int ce_id);
+
+static inline u64 ath10k_core_get_timestamp(void)
+{
+ struct timespec64 ts;
+
+ ktime_get_real_ts64(&ts);
+ return ((u64)ts.tv_sec * 1000000) + (ts.tv_nsec / 1000);
+}
+
+static inline int ath10k_core_get_next_idx(atomic_t *index, u32 max_entries)
+{
+ u32 curr_idx;
+
+ curr_idx = atomic_fetch_inc(index);
+ return (curr_idx & (max_entries - 1));
+}
+
#else /* CONFIG_ATH10K_DEBUG */
static inline int __ath10k_dbg(struct ath10k *ar,
@@ -273,6 +305,48 @@ static inline void ath10k_dbg_dump(struct ath10k *ar,
const void *buf, size_t len)
{
}
+
+static inline int ath10k_core_reg_access_history_init(struct ath10k *ar,
+ u32 max_entries)
+{
+ return 0;
+}
+
+static inline int ath10k_core_wmi_cmd_history_init(struct ath10k *ar,
+ u32 max_entries)
+{
+ return 0;
+}
+
+static inline int ath10k_core_wmi_event_history_init(struct ath10k *ar,
+ u32 max_entries)
+{
+ return 0;
+}
+
+static inline int ath10k_core_ce_event_history_init(struct ath10k *ar,
+ u32 max_entries)
+{
+ return 0;
+}
+
+static inline void ath10k_record_reg_access(struct ath10k *ar, u32 offset,
+ u32 val, bool write)
+{
+}
+
+static inline void ath10k_record_wmi_event(struct ath10k *ar,
+ enum ath10k_wmi_type type,
+ u32 id, unsigned char *data)
+{
+}
+
+static inline void ath10k_record_ce_event(struct ath10k *ar,
+ enum ath10k_ce_event event_type,
+ int ce_id)
+{
+}
+
#endif /* CONFIG_ATH10K_DEBUG */
/* Avoid calling __ath10k_dbg() if debug_mask is not set and tracing
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 1ef5fdb..aa7ee32 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -473,6 +473,7 @@ static void ath10k_snoc_write32(struct ath10k *ar, u32 offset, u32 value)
{
struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ ath10k_record_reg_access(ar, offset, value, true);
iowrite32(value, ar_snoc->mem + offset);
}
@@ -482,6 +483,7 @@ static u32 ath10k_snoc_read32(struct ath10k *ar, u32 offset)
u32 val;
val = ioread32(ar_snoc->mem + offset);
+ ath10k_record_reg_access(ar, offset, val, false);
return val;
}
@@ -1159,6 +1161,7 @@ static irqreturn_t ath10k_snoc_per_engine_handler(int irq, void *arg)
ce_id);
return IRQ_HANDLED;
}
+ ath10k_record_ce_event(ar, ATH10K_IRQ_TRIGGER, ce_id);
ath10k_ce_disable_interrupt(ar, ce_id);
set_bit(ce_id, ar_snoc->pending_ce_irqs);
@@ -1175,6 +1178,7 @@ static int ath10k_snoc_napi_poll(struct napi_struct *ctx, int budget)
int done = 0;
int ce_id;
+ ath10k_record_ce_event(ar, ATH10K_NAPI_POLL, 0);
if (test_bit(ATH10K_FLAG_CRASH_FLUSH, &ar->dev_flags)) {
napi_complete(ctx);
return done;
@@ -1188,8 +1192,12 @@ static int ath10k_snoc_napi_poll(struct napi_struct *ctx, int budget)
done = ath10k_htt_txrx_compl_task(ar, budget);
- if (done < budget)
+ if (done < budget) {
napi_complete(ctx);
+ ath10k_record_ce_event(ar, ATH10K_NAPI_COMPLETE, 0);
+ } else {
+ ath10k_record_ce_event(ar, ATH10K_NAPI_RESCHED, 0);
+ }
return done;
}
@@ -1660,6 +1668,11 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
ar->ce_priv = &ar_snoc->ce;
msa_size = drv_data->msa_size;
+ ath10k_core_reg_access_history_init(ar, ATH10K_REG_ACCESS_HISTORY_MAX);
+ ath10k_core_wmi_event_history_init(ar, ATH10K_WMI_EVENT_HISTORY_MAX);
+ ath10k_core_wmi_cmd_history_init(ar, ATH10K_WMI_CMD_HISTORY_MAX);
+ ath10k_core_ce_event_history_init(ar, ATH10K_CE_EVENT_HISTORY_MAX);
+
ath10k_snoc_quirks_init(ar);
ret = ath10k_snoc_resource_init(ar);
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 932266d..9df5748 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -627,6 +627,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar, struct sk_buff *skb)
if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
goto out;
+ ath10k_record_wmi_event(ar, ATH10K_WMI_EVENT, id, skb->data);
trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
consumed = ath10k_tm_event_wmi(ar, id, skb);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index a81a1ab..8ebd05c 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1802,6 +1802,15 @@ struct sk_buff *ath10k_wmi_alloc_skb(struct ath10k *ar, u32 len)
static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
{
+ struct wmi_cmd_hdr *cmd_hdr;
+ enum wmi_tlv_event_id id;
+
+ cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
+ id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
+
+ ath10k_record_wmi_event(ar, ATH10K_WMI_TX_COMPL, id,
+ skb->data + sizeof(struct wmi_cmd_hdr));
+
dev_kfree_skb(skb);
}
@@ -1912,6 +1921,7 @@ int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id)
might_sleep();
+ ath10k_record_wmi_event(ar, ATH10K_WMI_CMD, cmd_id, skb->data);
if (cmd_id == WMI_CMD_UNSUPPORTED) {
ath10k_warn(ar, "wmi command %d is not supported by firmware\n",
cmd_id);
--
2.7.4
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 1/3] ath10k: Add history for tracking certain events
@ 2020-07-31 18:27 ` Rakesh Pillai
0 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-07-31 18:27 UTC (permalink / raw)
To: ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev,
Rakesh Pillai
Add history for tracking the below events
- register read
- register write
- IRQ trigger
- NAPI poll
- CE service
- WMI cmd
- WMI event
- WMI tx completion
This will help in debugging any crash or any
improper behaviour.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/ce.c | 1 +
drivers/net/wireless/ath/ath10k/core.h | 74 +++++++++++++++++
drivers/net/wireless/ath/ath10k/debug.c | 133 ++++++++++++++++++++++++++++++
drivers/net/wireless/ath/ath10k/debug.h | 74 +++++++++++++++++
drivers/net/wireless/ath/ath10k/snoc.c | 15 +++-
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
7 files changed, 307 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 84ec80c..0f541de 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -1299,6 +1299,7 @@ void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
u32 ctrl_addr = ce_state->ctrl_addr;
+ ath10k_record_ce_event(ar, ATH10K_CE_SERVICE, ce_id);
/*
* Clear before handling
*
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 5c18f6c..46bd5aa 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -970,6 +970,75 @@ struct ath10k_bus_params {
bool hl_msdu_ids;
};
+#define ATH10K_REG_ACCESS_HISTORY_MAX 512
+#define ATH10K_CE_EVENT_HISTORY_MAX 1024
+#define ATH10K_WMI_EVENT_HISTORY_MAX 512
+#define ATH10K_WMI_CMD_HISTORY_MAX 256
+
+#define ATH10K_WMI_DATA_LEN 16
+
+enum ath10k_ce_event {
+ ATH10K_IRQ_TRIGGER,
+ ATH10K_NAPI_POLL,
+ ATH10K_CE_SERVICE,
+ ATH10K_NAPI_COMPLETE,
+ ATH10K_NAPI_RESCHED,
+ ATH10K_IRQ_SUMMARY,
+};
+
+enum ath10k_wmi_type {
+ ATH10K_WMI_EVENT,
+ ATH10K_WMI_CMD,
+ ATH10K_WMI_TX_COMPL,
+};
+
+struct ath10k_reg_access_entry {
+ u32 cpu_id;
+ bool write;
+ u32 offset;
+ u32 val;
+ u64 timestamp;
+};
+
+struct ath10k_wmi_event_entry {
+ u32 cpu_id;
+ enum ath10k_wmi_type type;
+ u32 id;
+ u64 timestamp;
+ unsigned char data[ATH10K_WMI_DATA_LEN];
+};
+
+struct ath10k_ce_event_entry {
+ u32 cpu_id;
+ enum ath10k_ce_event event_type;
+ u32 ce_id;
+ u64 timestamp;
+};
+
+struct ath10k_wmi_event_history {
+ struct ath10k_wmi_event_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing wmi event history */
+ spinlock_t hist_lock;
+};
+
+struct ath10k_ce_event_history {
+ struct ath10k_ce_event_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing ce event history */
+ spinlock_t hist_lock;
+};
+
+struct ath10k_reg_access_history {
+ struct ath10k_reg_access_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing register access history */
+ spinlock_t hist_lock;
+};
+
struct ath10k {
struct ath_common ath_common;
struct ieee80211_hw *hw;
@@ -1261,6 +1330,11 @@ struct ath10k {
bool coex_support;
int coex_gpio_pin;
+ struct ath10k_reg_access_history reg_access_history;
+ struct ath10k_ce_event_history ce_event_history;
+ struct ath10k_wmi_event_history wmi_event_history;
+ struct ath10k_wmi_event_history wmi_cmd_history;
+
/* must be last */
u8 drv_priv[] __aligned(sizeof(void *));
};
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index e8250a6..9105b0b 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2722,4 +2722,137 @@ void ath10k_dbg_dump(struct ath10k *ar,
}
EXPORT_SYMBOL(ath10k_dbg_dump);
+int ath10k_core_reg_access_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->reg_access_history.record = vzalloc(max_entries *
+ sizeof(struct ath10k_reg_access_entry));
+ if (!ar->reg_access_history.record)
+ return -ENOMEM;
+
+ ar->reg_access_history.max_entries = max_entries;
+ atomic_set(&ar->reg_access_history.index, 0);
+ spin_lock_init(&ar->reg_access_history.hist_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_reg_access_history_init);
+
+int ath10k_core_wmi_cmd_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->wmi_cmd_history.record = vzalloc(max_entries *
+ sizeof(struct ath10k_wmi_event_entry));
+ if (!ar->wmi_cmd_history.record)
+ return -ENOMEM;
+
+ ar->wmi_cmd_history.max_entries = max_entries;
+ atomic_set(&ar->wmi_cmd_history.index, 0);
+ spin_lock_init(&ar->wmi_cmd_history.hist_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_wmi_cmd_history_init);
+
+int ath10k_core_wmi_event_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->wmi_event_history.record = vzalloc(max_entries *
+ sizeof(struct ath10k_wmi_event_entry));
+ if (!ar->wmi_event_history.record)
+ return -ENOMEM;
+
+ ar->wmi_event_history.max_entries = max_entries;
+ atomic_set(&ar->wmi_event_history.index, 0);
+ spin_lock_init(&ar->wmi_event_history.hist_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_wmi_event_history_init);
+
+int ath10k_core_ce_event_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->ce_event_history.record = vzalloc(max_entries *
+ sizeof(struct ath10k_ce_event_entry));
+ if (!ar->ce_event_history.record)
+ return -ENOMEM;
+
+ ar->ce_event_history.max_entries = max_entries;
+ atomic_set(&ar->ce_event_history.index, 0);
+ spin_lock_init(&ar->ce_event_history.hist_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_ce_event_history_init);
+
+void ath10k_record_reg_access(struct ath10k *ar, u32 offset, u32 val, bool write)
+{
+ struct ath10k_reg_access_entry *entry;
+ u32 idx;
+
+ if (!ar->reg_access_history.record)
+ return;
+
+ idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
+ ar->reg_access_history.max_entries);
+ entry = &ar->reg_access_history.record[idx];
+
+ entry->timestamp = ath10k_core_get_timestamp();
+ entry->write = write;
+ entry->offset = offset;
+ entry->val = val;
+}
+EXPORT_SYMBOL(ath10k_record_reg_access);
+
+void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
+ u32 id, unsigned char *data)
+{
+ struct ath10k_wmi_event_entry *entry;
+ u32 idx;
+
+ if (type == ATH10K_WMI_EVENT) {
+ if (!ar->wmi_event_history.record)
+ return;
+
+ spin_lock_bh(&ar->wmi_event_history.hist_lock);
+ idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
+ ar->wmi_event_history.max_entries);
+ spin_unlock_bh(&ar->wmi_event_history.hist_lock);
+ entry = &ar->wmi_event_history.record[idx];
+ } else {
+ if (!ar->wmi_cmd_history.record)
+ return;
+
+ spin_lock_bh(&ar->wmi_cmd_history.hist_lock);
+ idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
+ ar->wmi_cmd_history.max_entries);
+ spin_unlock_bh(&ar->wmi_cmd_history.hist_lock);
+ entry = &ar->wmi_cmd_history.record[idx];
+ }
+
+ entry->timestamp = ath10k_core_get_timestamp();
+ entry->cpu_id = smp_processor_id();
+ entry->type = type;
+ entry->id = id;
+ memcpy(&entry->data, data + 4, ATH10K_WMI_DATA_LEN);
+}
+EXPORT_SYMBOL(ath10k_record_wmi_event);
+
+void ath10k_record_ce_event(struct ath10k *ar, enum ath10k_ce_event event_type,
+ int ce_id)
+{
+ struct ath10k_ce_event_entry *entry;
+ u32 idx;
+
+ if (!ar->ce_event_history.record)
+ return;
+
+ idx = ath10k_core_get_next_idx(&ar->ce_event_history.index,
+ ar->ce_event_history.max_entries);
+ entry = &ar->ce_event_history.record[idx];
+
+ entry->timestamp = ath10k_core_get_timestamp();
+ entry->cpu_id = smp_processor_id();
+ entry->event_type = event_type;
+ entry->ce_id = ce_id;
+}
+EXPORT_SYMBOL(ath10k_record_ce_event);
+
#endif /* CONFIG_ATH10K_DEBUG */
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index 997c1c8..c28aeb1 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -258,6 +258,38 @@ void ath10k_dbg_dump(struct ath10k *ar,
enum ath10k_debug_mask mask,
const char *msg, const char *prefix,
const void *buf, size_t len);
+
+/* ========== History init APIs =========== */
+int ath10k_core_reg_access_history_init(struct ath10k *ar, u32 max_entries);
+int ath10k_core_wmi_cmd_history_init(struct ath10k *ar, u32 max_entries);
+int ath10k_core_wmi_event_history_init(struct ath10k *ar, u32 max_entries);
+int ath10k_core_ce_event_history_init(struct ath10k *ar, u32 max_entries);
+
+/* ========== History record APIs =========== */
+void ath10k_record_reg_access(struct ath10k *ar, u32 offset, u32 val,
+ bool write);
+void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
+ u32 id, unsigned char *data);
+void ath10k_record_ce_event(struct ath10k *ar,
+ enum ath10k_ce_event event_type,
+ int ce_id);
+
+static inline u64 ath10k_core_get_timestamp(void)
+{
+ struct timespec64 ts;
+
+ ktime_get_real_ts64(&ts);
+ return ((u64)ts.tv_sec * 1000000) + (ts.tv_nsec / 1000);
+}
+
+static inline int ath10k_core_get_next_idx(atomic_t *index, u32 max_entries)
+{
+ u32 curr_idx;
+
+ curr_idx = atomic_fetch_inc(index);
+ return (curr_idx & (max_entries - 1));
+}
+
#else /* CONFIG_ATH10K_DEBUG */
static inline int __ath10k_dbg(struct ath10k *ar,
@@ -273,6 +305,48 @@ static inline void ath10k_dbg_dump(struct ath10k *ar,
const void *buf, size_t len)
{
}
+
+static inline int ath10k_core_reg_access_history_init(struct ath10k *ar,
+ u32 max_entries)
+{
+ return 0;
+}
+
+static inline int ath10k_core_wmi_cmd_history_init(struct ath10k *ar,
+ u32 max_entries)
+{
+ return 0;
+}
+
+static inline int ath10k_core_wmi_event_history_init(struct ath10k *ar,
+ u32 max_entries)
+{
+ return 0;
+}
+
+static inline int ath10k_core_ce_event_history_init(struct ath10k *ar,
+ u32 max_entries)
+{
+ return 0;
+}
+
+static inline void ath10k_record_reg_access(struct ath10k *ar, u32 offset,
+ u32 val, bool write)
+{
+}
+
+static inline void ath10k_record_wmi_event(struct ath10k *ar,
+ enum ath10k_wmi_type type,
+ u32 id, unsigned char *data)
+{
+}
+
+static inline void ath10k_record_ce_event(struct ath10k *ar,
+ enum ath10k_ce_event event_type,
+ int ce_id)
+{
+}
+
#endif /* CONFIG_ATH10K_DEBUG */
/* Avoid calling __ath10k_dbg() if debug_mask is not set and tracing
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 1ef5fdb..aa7ee32 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -473,6 +473,7 @@ static void ath10k_snoc_write32(struct ath10k *ar, u32 offset, u32 value)
{
struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ ath10k_record_reg_access(ar, offset, value, true);
iowrite32(value, ar_snoc->mem + offset);
}
@@ -482,6 +483,7 @@ static u32 ath10k_snoc_read32(struct ath10k *ar, u32 offset)
u32 val;
val = ioread32(ar_snoc->mem + offset);
+ ath10k_record_reg_access(ar, offset, val, false);
return val;
}
@@ -1159,6 +1161,7 @@ static irqreturn_t ath10k_snoc_per_engine_handler(int irq, void *arg)
ce_id);
return IRQ_HANDLED;
}
+ ath10k_record_ce_event(ar, ATH10K_IRQ_TRIGGER, ce_id);
ath10k_ce_disable_interrupt(ar, ce_id);
set_bit(ce_id, ar_snoc->pending_ce_irqs);
@@ -1175,6 +1178,7 @@ static int ath10k_snoc_napi_poll(struct napi_struct *ctx, int budget)
int done = 0;
int ce_id;
+ ath10k_record_ce_event(ar, ATH10K_NAPI_POLL, 0);
if (test_bit(ATH10K_FLAG_CRASH_FLUSH, &ar->dev_flags)) {
napi_complete(ctx);
return done;
@@ -1188,8 +1192,12 @@ static int ath10k_snoc_napi_poll(struct napi_struct *ctx, int budget)
done = ath10k_htt_txrx_compl_task(ar, budget);
- if (done < budget)
+ if (done < budget) {
napi_complete(ctx);
+ ath10k_record_ce_event(ar, ATH10K_NAPI_COMPLETE, 0);
+ } else {
+ ath10k_record_ce_event(ar, ATH10K_NAPI_RESCHED, 0);
+ }
return done;
}
@@ -1660,6 +1668,11 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
ar->ce_priv = &ar_snoc->ce;
msa_size = drv_data->msa_size;
+ ath10k_core_reg_access_history_init(ar, ATH10K_REG_ACCESS_HISTORY_MAX);
+ ath10k_core_wmi_event_history_init(ar, ATH10K_WMI_EVENT_HISTORY_MAX);
+ ath10k_core_wmi_cmd_history_init(ar, ATH10K_WMI_CMD_HISTORY_MAX);
+ ath10k_core_ce_event_history_init(ar, ATH10K_CE_EVENT_HISTORY_MAX);
+
ath10k_snoc_quirks_init(ar);
ret = ath10k_snoc_resource_init(ar);
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 932266d..9df5748 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -627,6 +627,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar, struct sk_buff *skb)
if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
goto out;
+ ath10k_record_wmi_event(ar, ATH10K_WMI_EVENT, id, skb->data);
trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
consumed = ath10k_tm_event_wmi(ar, id, skb);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index a81a1ab..8ebd05c 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1802,6 +1802,15 @@ struct sk_buff *ath10k_wmi_alloc_skb(struct ath10k *ar, u32 len)
static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
{
+ struct wmi_cmd_hdr *cmd_hdr;
+ enum wmi_tlv_event_id id;
+
+ cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
+ id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
+
+ ath10k_record_wmi_event(ar, ATH10K_WMI_TX_COMPL, id,
+ skb->data + sizeof(struct wmi_cmd_hdr));
+
dev_kfree_skb(skb);
}
@@ -1912,6 +1921,7 @@ int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id)
might_sleep();
+ ath10k_record_wmi_event(ar, ATH10K_WMI_CMD, cmd_id, skb->data);
if (cmd_id == WMI_CMD_UNSUPPORTED) {
ath10k_warn(ar, "wmi command %d is not supported by firmware\n",
cmd_id);
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/3] ath10k: Add history for tracking certain events
2020-07-31 18:27 ` Rakesh Pillai
@ 2020-07-31 18:38 ` Ben Greear
-1 siblings, 0 replies; 28+ messages in thread
From: Ben Greear @ 2020-07-31 18:38 UTC (permalink / raw)
To: Rakesh Pillai, ath10k
Cc: netdev, linux-wireless, linux-kernel, kuba, davem, kvalo
On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> Add history for tracking the below events
> - register read
> - register write
> - IRQ trigger
> - NAPI poll
> - CE service
> - WMI cmd
> - WMI event
> - WMI tx completion
>
> This will help in debugging any crash or any
> improper behaviour.
>
> Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> ---
> drivers/net/wireless/ath/ath10k/ce.c | 1 +
> drivers/net/wireless/ath/ath10k/core.h | 74 +++++++++++++++++
> drivers/net/wireless/ath/ath10k/debug.c | 133 ++++++++++++++++++++++++++++++
> drivers/net/wireless/ath/ath10k/debug.h | 74 +++++++++++++++++
> drivers/net/wireless/ath/ath10k/snoc.c | 15 +++-
> drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
> 7 files changed, 307 insertions(+), 1 deletion(-)
>
> +void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
> + u32 id, unsigned char *data)
> +{
> + struct ath10k_wmi_event_entry *entry;
> + u32 idx;
> +
> + if (type == ATH10K_WMI_EVENT) {
> + if (!ar->wmi_event_history.record)
> + return;
This check above is duplicated below, add it once at top of the method
instead.
> +
> + spin_lock_bh(&ar->wmi_event_history.hist_lock);
> + idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
> + ar->wmi_event_history.max_entries);
> + spin_unlock_bh(&ar->wmi_event_history.hist_lock);
> + entry = &ar->wmi_event_history.record[idx];
> + } else {
> + if (!ar->wmi_cmd_history.record)
> + return;
> +
> + spin_lock_bh(&ar->wmi_cmd_history.hist_lock);
> + idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
> + ar->wmi_cmd_history.max_entries);
> + spin_unlock_bh(&ar->wmi_cmd_history.hist_lock);
> + entry = &ar->wmi_cmd_history.record[idx];
> + }
> +
> + entry->timestamp = ath10k_core_get_timestamp();
> + entry->cpu_id = smp_processor_id();
> + entry->type = type;
> + entry->id = id;
> + memcpy(&entry->data, data + 4, ATH10K_WMI_DATA_LEN);
> +}
> +EXPORT_SYMBOL(ath10k_record_wmi_event);
> @@ -1660,6 +1668,11 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
> ar->ce_priv = &ar_snoc->ce;
> msa_size = drv_data->msa_size;
>
> + ath10k_core_reg_access_history_init(ar, ATH10K_REG_ACCESS_HISTORY_MAX);
> + ath10k_core_wmi_event_history_init(ar, ATH10K_WMI_EVENT_HISTORY_MAX);
> + ath10k_core_wmi_cmd_history_init(ar, ATH10K_WMI_CMD_HISTORY_MAX);
> + ath10k_core_ce_event_history_init(ar, ATH10K_CE_EVENT_HISTORY_MAX);
Maybe only enable this once user turns it on? It sucks up a bit of memory?
> +
> ath10k_snoc_quirks_init(ar);
>
> ret = ath10k_snoc_resource_init(ar);
> diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> index 932266d..9df5748 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> @@ -627,6 +627,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar, struct sk_buff *skb)
> if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
> goto out;
>
> + ath10k_record_wmi_event(ar, ATH10K_WMI_EVENT, id, skb->data);
> trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
>
> consumed = ath10k_tm_event_wmi(ar, id, skb);
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index a81a1ab..8ebd05c 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -1802,6 +1802,15 @@ struct sk_buff *ath10k_wmi_alloc_skb(struct ath10k *ar, u32 len)
>
> static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
> {
> + struct wmi_cmd_hdr *cmd_hdr;
> + enum wmi_tlv_event_id id;
> +
> + cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
> + id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
> +
> + ath10k_record_wmi_event(ar, ATH10K_WMI_TX_COMPL, id,
> + skb->data + sizeof(struct wmi_cmd_hdr));
> +
> dev_kfree_skb(skb);
> }
I think guard the above new code with if (unlikely(ar->ce_event_history.record)) { ... }
All in all, I think I'd want to compile this out (while leaving other debug compiled
in) since it seems this stuff would be rarely used and it adds method calls to hot
paths.
That is a decision for Kalle though, so see what he says...
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/3] ath10k: Add history for tracking certain events
@ 2020-07-31 18:38 ` Ben Greear
0 siblings, 0 replies; 28+ messages in thread
From: Ben Greear @ 2020-07-31 18:38 UTC (permalink / raw)
To: Rakesh Pillai, ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev
On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> Add history for tracking the below events
> - register read
> - register write
> - IRQ trigger
> - NAPI poll
> - CE service
> - WMI cmd
> - WMI event
> - WMI tx completion
>
> This will help in debugging any crash or any
> improper behaviour.
>
> Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> ---
> drivers/net/wireless/ath/ath10k/ce.c | 1 +
> drivers/net/wireless/ath/ath10k/core.h | 74 +++++++++++++++++
> drivers/net/wireless/ath/ath10k/debug.c | 133 ++++++++++++++++++++++++++++++
> drivers/net/wireless/ath/ath10k/debug.h | 74 +++++++++++++++++
> drivers/net/wireless/ath/ath10k/snoc.c | 15 +++-
> drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
> 7 files changed, 307 insertions(+), 1 deletion(-)
>
> +void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
> + u32 id, unsigned char *data)
> +{
> + struct ath10k_wmi_event_entry *entry;
> + u32 idx;
> +
> + if (type == ATH10K_WMI_EVENT) {
> + if (!ar->wmi_event_history.record)
> + return;
This check above is duplicated below, add it once at top of the method
instead.
> +
> + spin_lock_bh(&ar->wmi_event_history.hist_lock);
> + idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
> + ar->wmi_event_history.max_entries);
> + spin_unlock_bh(&ar->wmi_event_history.hist_lock);
> + entry = &ar->wmi_event_history.record[idx];
> + } else {
> + if (!ar->wmi_cmd_history.record)
> + return;
> +
> + spin_lock_bh(&ar->wmi_cmd_history.hist_lock);
> + idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
> + ar->wmi_cmd_history.max_entries);
> + spin_unlock_bh(&ar->wmi_cmd_history.hist_lock);
> + entry = &ar->wmi_cmd_history.record[idx];
> + }
> +
> + entry->timestamp = ath10k_core_get_timestamp();
> + entry->cpu_id = smp_processor_id();
> + entry->type = type;
> + entry->id = id;
> + memcpy(&entry->data, data + 4, ATH10K_WMI_DATA_LEN);
> +}
> +EXPORT_SYMBOL(ath10k_record_wmi_event);
> @@ -1660,6 +1668,11 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
> ar->ce_priv = &ar_snoc->ce;
> msa_size = drv_data->msa_size;
>
> + ath10k_core_reg_access_history_init(ar, ATH10K_REG_ACCESS_HISTORY_MAX);
> + ath10k_core_wmi_event_history_init(ar, ATH10K_WMI_EVENT_HISTORY_MAX);
> + ath10k_core_wmi_cmd_history_init(ar, ATH10K_WMI_CMD_HISTORY_MAX);
> + ath10k_core_ce_event_history_init(ar, ATH10K_CE_EVENT_HISTORY_MAX);
Maybe only enable this once user turns it on? It sucks up a bit of memory?
> +
> ath10k_snoc_quirks_init(ar);
>
> ret = ath10k_snoc_resource_init(ar);
> diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> index 932266d..9df5748 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> @@ -627,6 +627,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar, struct sk_buff *skb)
> if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
> goto out;
>
> + ath10k_record_wmi_event(ar, ATH10K_WMI_EVENT, id, skb->data);
> trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
>
> consumed = ath10k_tm_event_wmi(ar, id, skb);
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index a81a1ab..8ebd05c 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -1802,6 +1802,15 @@ struct sk_buff *ath10k_wmi_alloc_skb(struct ath10k *ar, u32 len)
>
> static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff *skb)
> {
> + struct wmi_cmd_hdr *cmd_hdr;
> + enum wmi_tlv_event_id id;
> +
> + cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
> + id = MS(__le32_to_cpu(cmd_hdr->cmd_id), WMI_CMD_HDR_CMD_ID);
> +
> + ath10k_record_wmi_event(ar, ATH10K_WMI_TX_COMPL, id,
> + skb->data + sizeof(struct wmi_cmd_hdr));
> +
> dev_kfree_skb(skb);
> }
I think guard the above new code with if (unlikely(ar->ce_event_history.record)) { ... }
All in all, I think I'd want to compile this out (while leaving other debug compiled
in) since it seems this stuff would be rarely used and it adds method calls to hot
paths.
That is a decision for Kalle though, so see what he says...
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 28+ messages in thread* RE: [PATCH v2 1/3] ath10k: Add history for tracking certain events
2020-07-31 18:38 ` Ben Greear
@ 2020-08-01 5:13 ` Rakesh Pillai
-1 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-08-01 5:13 UTC (permalink / raw)
To: 'Ben Greear', ath10k
Cc: netdev, linux-wireless, linux-kernel, kuba, davem, kvalo
> -----Original Message-----
> From: Ben Greear <greearb@candelatech.com>
> Sent: Saturday, August 1, 2020 12:08 AM
> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] ath10k: Add history for tracking certain events
>
> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> > Add history for tracking the below events
> > - register read
> > - register write
> > - IRQ trigger
> > - NAPI poll
> > - CE service
> > - WMI cmd
> > - WMI event
> > - WMI tx completion
> >
> > This will help in debugging any crash or any
> > improper behaviour.
> >
> > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
> >
> > Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> > ---
> > drivers/net/wireless/ath/ath10k/ce.c | 1 +
> > drivers/net/wireless/ath/ath10k/core.h | 74 +++++++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.c | 133
> ++++++++++++++++++++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.h | 74 +++++++++++++++++
> > drivers/net/wireless/ath/ath10k/snoc.c | 15 +++-
> > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> > drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
> > 7 files changed, 307 insertions(+), 1 deletion(-)
> >
>
> > +void ath10k_record_wmi_event(struct ath10k *ar, enum
> ath10k_wmi_type type,
> > + u32 id, unsigned char *data)
> > +{
> > + struct ath10k_wmi_event_entry *entry;
> > + u32 idx;
> > +
> > + if (type == ATH10K_WMI_EVENT) {
> > + if (!ar->wmi_event_history.record)
> > + return;
>
> This check above is duplicated below, add it once at top of the method
> instead.
The same function is used to record WMI events and CMD, which are stored in different memory locations.
Hence the check " if (type == ATH10K_WMI_EVENT) {" is necessary.
>
> > +
> > + spin_lock_bh(&ar->wmi_event_history.hist_lock);
> > + idx = ath10k_core_get_next_idx(&ar-
> >reg_access_history.index,
> > + ar-
> >wmi_event_history.max_entries);
> > + spin_unlock_bh(&ar->wmi_event_history.hist_lock);
> > + entry = &ar->wmi_event_history.record[idx];
> > + } else {
> > + if (!ar->wmi_cmd_history.record)
> > + return;
> > +
> > + spin_lock_bh(&ar->wmi_cmd_history.hist_lock);
> > + idx = ath10k_core_get_next_idx(&ar-
> >reg_access_history.index,
> > + ar-
> >wmi_cmd_history.max_entries);
> > + spin_unlock_bh(&ar->wmi_cmd_history.hist_lock);
> > + entry = &ar->wmi_cmd_history.record[idx];
> > + }
> > +
> > + entry->timestamp = ath10k_core_get_timestamp();
> > + entry->cpu_id = smp_processor_id();
> > + entry->type = type;
> > + entry->id = id;
> > + memcpy(&entry->data, data + 4, ATH10K_WMI_DATA_LEN);
> > +}
> > +EXPORT_SYMBOL(ath10k_record_wmi_event);
>
> > @@ -1660,6 +1668,11 @@ static int ath10k_snoc_probe(struct
> platform_device *pdev)
> > ar->ce_priv = &ar_snoc->ce;
> > msa_size = drv_data->msa_size;
> >
> > + ath10k_core_reg_access_history_init(ar,
> ATH10K_REG_ACCESS_HISTORY_MAX);
> > + ath10k_core_wmi_event_history_init(ar,
> ATH10K_WMI_EVENT_HISTORY_MAX);
> > + ath10k_core_wmi_cmd_history_init(ar,
> ATH10K_WMI_CMD_HISTORY_MAX);
> > + ath10k_core_ce_event_history_init(ar,
> ATH10K_CE_EVENT_HISTORY_MAX);
>
> Maybe only enable this once user turns it on? It sucks up a bit of memory?
This memory will be allocated only if the history is enabled via module param, else the function just returns 0.
>
> > +
> > ath10k_snoc_quirks_init(ar);
> >
> > ret = ath10k_snoc_resource_init(ar);
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > index 932266d..9df5748 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > @@ -627,6 +627,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar,
> struct sk_buff *skb)
> > if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
> > goto out;
> >
> > + ath10k_record_wmi_event(ar, ATH10K_WMI_EVENT, id, skb->data);
> > trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
> >
> > consumed = ath10k_tm_event_wmi(ar, id, skb);
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
> b/drivers/net/wireless/ath/ath10k/wmi.c
> > index a81a1ab..8ebd05c 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> > @@ -1802,6 +1802,15 @@ struct sk_buff *ath10k_wmi_alloc_skb(struct
> ath10k *ar, u32 len)
> >
> > static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff
> *skb)
> > {
> > + struct wmi_cmd_hdr *cmd_hdr;
> > + enum wmi_tlv_event_id id;
> > +
> > + cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
> > + id = MS(__le32_to_cpu(cmd_hdr->cmd_id),
> WMI_CMD_HDR_CMD_ID);
> > +
> > + ath10k_record_wmi_event(ar, ATH10K_WMI_TX_COMPL, id,
> > + skb->data + sizeof(struct wmi_cmd_hdr));
> > +
> > dev_kfree_skb(skb);
> > }
>
> I think guard the above new code with if (unlikely(ar-
> >ce_event_history.record)) { ... }
>
> All in all, I think I'd want to compile this out (while leaving other debug
> compiled
> in) since it seems this stuff would be rarely used and it adds method calls to
> hot
> paths.
>
> That is a decision for Kalle though, so see what he says...
Sure let me add this check.
>
> Thanks,
> Ben
>
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc http://www.candelatech.com
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 28+ messages in thread* RE: [PATCH v2 1/3] ath10k: Add history for tracking certain events
@ 2020-08-01 5:13 ` Rakesh Pillai
0 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-08-01 5:13 UTC (permalink / raw)
To: 'Ben Greear', ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev
> -----Original Message-----
> From: Ben Greear <greearb@candelatech.com>
> Sent: Saturday, August 1, 2020 12:08 AM
> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] ath10k: Add history for tracking certain events
>
> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> > Add history for tracking the below events
> > - register read
> > - register write
> > - IRQ trigger
> > - NAPI poll
> > - CE service
> > - WMI cmd
> > - WMI event
> > - WMI tx completion
> >
> > This will help in debugging any crash or any
> > improper behaviour.
> >
> > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
> >
> > Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> > ---
> > drivers/net/wireless/ath/ath10k/ce.c | 1 +
> > drivers/net/wireless/ath/ath10k/core.h | 74 +++++++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.c | 133
> ++++++++++++++++++++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.h | 74 +++++++++++++++++
> > drivers/net/wireless/ath/ath10k/snoc.c | 15 +++-
> > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> > drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
> > 7 files changed, 307 insertions(+), 1 deletion(-)
> >
>
> > +void ath10k_record_wmi_event(struct ath10k *ar, enum
> ath10k_wmi_type type,
> > + u32 id, unsigned char *data)
> > +{
> > + struct ath10k_wmi_event_entry *entry;
> > + u32 idx;
> > +
> > + if (type == ATH10K_WMI_EVENT) {
> > + if (!ar->wmi_event_history.record)
> > + return;
>
> This check above is duplicated below, add it once at top of the method
> instead.
The same function is used to record WMI events and CMD, which are stored in different memory locations.
Hence the check " if (type == ATH10K_WMI_EVENT) {" is necessary.
>
> > +
> > + spin_lock_bh(&ar->wmi_event_history.hist_lock);
> > + idx = ath10k_core_get_next_idx(&ar-
> >reg_access_history.index,
> > + ar-
> >wmi_event_history.max_entries);
> > + spin_unlock_bh(&ar->wmi_event_history.hist_lock);
> > + entry = &ar->wmi_event_history.record[idx];
> > + } else {
> > + if (!ar->wmi_cmd_history.record)
> > + return;
> > +
> > + spin_lock_bh(&ar->wmi_cmd_history.hist_lock);
> > + idx = ath10k_core_get_next_idx(&ar-
> >reg_access_history.index,
> > + ar-
> >wmi_cmd_history.max_entries);
> > + spin_unlock_bh(&ar->wmi_cmd_history.hist_lock);
> > + entry = &ar->wmi_cmd_history.record[idx];
> > + }
> > +
> > + entry->timestamp = ath10k_core_get_timestamp();
> > + entry->cpu_id = smp_processor_id();
> > + entry->type = type;
> > + entry->id = id;
> > + memcpy(&entry->data, data + 4, ATH10K_WMI_DATA_LEN);
> > +}
> > +EXPORT_SYMBOL(ath10k_record_wmi_event);
>
> > @@ -1660,6 +1668,11 @@ static int ath10k_snoc_probe(struct
> platform_device *pdev)
> > ar->ce_priv = &ar_snoc->ce;
> > msa_size = drv_data->msa_size;
> >
> > + ath10k_core_reg_access_history_init(ar,
> ATH10K_REG_ACCESS_HISTORY_MAX);
> > + ath10k_core_wmi_event_history_init(ar,
> ATH10K_WMI_EVENT_HISTORY_MAX);
> > + ath10k_core_wmi_cmd_history_init(ar,
> ATH10K_WMI_CMD_HISTORY_MAX);
> > + ath10k_core_ce_event_history_init(ar,
> ATH10K_CE_EVENT_HISTORY_MAX);
>
> Maybe only enable this once user turns it on? It sucks up a bit of memory?
This memory will be allocated only if the history is enabled via module param, else the function just returns 0.
>
> > +
> > ath10k_snoc_quirks_init(ar);
> >
> > ret = ath10k_snoc_resource_init(ar);
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > index 932266d..9df5748 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > @@ -627,6 +627,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar,
> struct sk_buff *skb)
> > if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
> > goto out;
> >
> > + ath10k_record_wmi_event(ar, ATH10K_WMI_EVENT, id, skb->data);
> > trace_ath10k_wmi_event(ar, id, skb->data, skb->len);
> >
> > consumed = ath10k_tm_event_wmi(ar, id, skb);
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
> b/drivers/net/wireless/ath/ath10k/wmi.c
> > index a81a1ab..8ebd05c 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> > @@ -1802,6 +1802,15 @@ struct sk_buff *ath10k_wmi_alloc_skb(struct
> ath10k *ar, u32 len)
> >
> > static void ath10k_wmi_htc_tx_complete(struct ath10k *ar, struct sk_buff
> *skb)
> > {
> > + struct wmi_cmd_hdr *cmd_hdr;
> > + enum wmi_tlv_event_id id;
> > +
> > + cmd_hdr = (struct wmi_cmd_hdr *)skb->data;
> > + id = MS(__le32_to_cpu(cmd_hdr->cmd_id),
> WMI_CMD_HDR_CMD_ID);
> > +
> > + ath10k_record_wmi_event(ar, ATH10K_WMI_TX_COMPL, id,
> > + skb->data + sizeof(struct wmi_cmd_hdr));
> > +
> > dev_kfree_skb(skb);
> > }
>
> I think guard the above new code with if (unlikely(ar-
> >ce_event_history.record)) { ... }
>
> All in all, I think I'd want to compile this out (while leaving other debug
> compiled
> in) since it seems this stuff would be rarely used and it adds method calls to
> hot
> paths.
>
> That is a decision for Kalle though, so see what he says...
Sure let me add this check.
>
> Thanks,
> Ben
>
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 2/3] ath10k: Add module param to enable history
2020-07-31 18:27 ` Rakesh Pillai
@ 2020-07-31 18:27 ` Rakesh Pillai
-1 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-07-31 18:27 UTC (permalink / raw)
To: ath10k
Cc: netdev, linux-wireless, linux-kernel, Rakesh Pillai, kuba, davem,
kvalo
Add a module param to enable recording history
of certain debug events. This module parameter
is a mask of the different history recording to
be enabled.
The memory for recording the history will not be
allocated if its not enabled via module parameter.
This is to avoid unnecessary memory allocation when
recording the history is not needed.
To enable all the history recording, the driver
should be loaded as below
"insmod ath10k_core.ko history_enable_mask=0xf"
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/core.c | 3 +++
drivers/net/wireless/ath/ath10k/core.h | 8 ++++++++
drivers/net/wireless/ath/ath10k/debug.c | 26 ++++++++++++++++++++++----
drivers/net/wireless/ath/ath10k/debug.h | 1 +
4 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 9104496..f91a9d0 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -29,6 +29,7 @@
unsigned int ath10k_debug_mask;
EXPORT_SYMBOL(ath10k_debug_mask);
+unsigned long ath10k_history_enable_mask;
static unsigned int ath10k_cryptmode_param;
static bool uart_print;
static bool skip_otp;
@@ -46,6 +47,7 @@ module_param(skip_otp, bool, 0644);
module_param(rawmode, bool, 0644);
module_param(fw_diag_log, bool, 0644);
module_param_named(coredump_mask, ath10k_coredump_mask, ulong, 0444);
+module_param_named(history_enable_mask, ath10k_history_enable_mask, ulong, 0444);
MODULE_PARM_DESC(debug_mask, "Debugging mask");
MODULE_PARM_DESC(uart_print, "Uart target debugging");
@@ -54,6 +56,7 @@ MODULE_PARM_DESC(cryptmode, "Crypto mode: 0-hardware, 1-software");
MODULE_PARM_DESC(rawmode, "Use raw 802.11 frame datapath");
MODULE_PARM_DESC(coredump_mask, "Bitfield of what to include in firmware crash file");
MODULE_PARM_DESC(fw_diag_log, "Diag based fw log debugging");
+MODULE_PARM_DESC(history_enable_mask, "Enable events history recording");
static const struct ath10k_hw_params ath10k_hw_params_list[] = {
{
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 46bd5aa..ce429df 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -977,6 +977,14 @@ struct ath10k_bus_params {
#define ATH10K_WMI_DATA_LEN 16
+enum ath10k_history {
+ ATH10K_REG_ACCESS_HISTORY,
+ ATH10K_CE_EVENTS_HISTORY,
+ ATH10K_WMI_CMD_HISTORY,
+ ATH10K_WMI_EVENTS_HISTORY,
+ ATH10K_HISTORY_MAX,
+};
+
enum ath10k_ce_event {
ATH10K_IRQ_TRIGGER,
ATH10K_NAPI_POLL,
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 9105b0b..5d08652 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2724,6 +2724,9 @@ EXPORT_SYMBOL(ath10k_dbg_dump);
int ath10k_core_reg_access_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_REG_ACCESS_HISTORY, &ath10k_history_enable_mask))
+ return 0;
+
ar->reg_access_history.record = vzalloc(max_entries *
sizeof(struct ath10k_reg_access_entry));
if (!ar->reg_access_history.record)
@@ -2739,6 +2742,9 @@ EXPORT_SYMBOL(ath10k_core_reg_access_history_init);
int ath10k_core_wmi_cmd_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_WMI_CMD_HISTORY, &ath10k_history_enable_mask))
+ return 0;
+
ar->wmi_cmd_history.record = vzalloc(max_entries *
sizeof(struct ath10k_wmi_event_entry));
if (!ar->wmi_cmd_history.record)
@@ -2754,6 +2760,9 @@ EXPORT_SYMBOL(ath10k_core_wmi_cmd_history_init);
int ath10k_core_wmi_event_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_WMI_EVENTS_HISTORY, &ath10k_history_enable_mask))
+ return 0;
+
ar->wmi_event_history.record = vzalloc(max_entries *
sizeof(struct ath10k_wmi_event_entry));
if (!ar->wmi_event_history.record)
@@ -2769,6 +2778,9 @@ EXPORT_SYMBOL(ath10k_core_wmi_event_history_init);
int ath10k_core_ce_event_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_CE_EVENTS_HISTORY, &ath10k_history_enable_mask))
+ return 0;
+
ar->ce_event_history.record = vzalloc(max_entries *
sizeof(struct ath10k_ce_event_entry));
if (!ar->ce_event_history.record)
@@ -2787,7 +2799,8 @@ void ath10k_record_reg_access(struct ath10k *ar, u32 offset, u32 val, bool write
struct ath10k_reg_access_entry *entry;
u32 idx;
- if (!ar->reg_access_history.record)
+ if (!test_bit(ATH10K_REG_ACCESS_HISTORY, &ath10k_history_enable_mask) ||
+ !ar->reg_access_history.record)
return;
idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
@@ -2808,7 +2821,9 @@ void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
u32 idx;
if (type == ATH10K_WMI_EVENT) {
- if (!ar->wmi_event_history.record)
+ if (!test_bit(ATH10K_WMI_EVENTS_HISTORY,
+ &ath10k_history_enable_mask) ||
+ !ar->wmi_event_history.record)
return;
spin_lock_bh(&ar->wmi_event_history.hist_lock);
@@ -2817,7 +2832,9 @@ void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
spin_unlock_bh(&ar->wmi_event_history.hist_lock);
entry = &ar->wmi_event_history.record[idx];
} else {
- if (!ar->wmi_cmd_history.record)
+ if (!test_bit(ATH10K_WMI_CMD_HISTORY,
+ &ath10k_history_enable_mask) ||
+ !ar->wmi_event_history.record)
return;
spin_lock_bh(&ar->wmi_cmd_history.hist_lock);
@@ -2841,7 +2858,8 @@ void ath10k_record_ce_event(struct ath10k *ar, enum ath10k_ce_event event_type,
struct ath10k_ce_event_entry *entry;
u32 idx;
- if (!ar->ce_event_history.record)
+ if (!test_bit(ATH10K_CE_EVENTS_HISTORY, &ath10k_history_enable_mask) ||
+ !ar->ce_event_history.record)
return;
idx = ath10k_core_get_next_idx(&ar->ce_event_history.index,
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index c28aeb1..7799b89 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -75,6 +75,7 @@ struct ath10k_pktlog_hdr {
#define ATH10K_TX_POWER_MIN_VAL 0
extern unsigned int ath10k_debug_mask;
+extern unsigned long ath10k_history_enable_mask;
__printf(2, 3) void ath10k_info(struct ath10k *ar, const char *fmt, ...);
__printf(2, 3) void ath10k_err(struct ath10k *ar, const char *fmt, ...);
--
2.7.4
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 2/3] ath10k: Add module param to enable history
@ 2020-07-31 18:27 ` Rakesh Pillai
0 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-07-31 18:27 UTC (permalink / raw)
To: ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev,
Rakesh Pillai
Add a module param to enable recording history
of certain debug events. This module parameter
is a mask of the different history recording to
be enabled.
The memory for recording the history will not be
allocated if its not enabled via module parameter.
This is to avoid unnecessary memory allocation when
recording the history is not needed.
To enable all the history recording, the driver
should be loaded as below
"insmod ath10k_core.ko history_enable_mask=0xf"
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/core.c | 3 +++
drivers/net/wireless/ath/ath10k/core.h | 8 ++++++++
drivers/net/wireless/ath/ath10k/debug.c | 26 ++++++++++++++++++++++----
drivers/net/wireless/ath/ath10k/debug.h | 1 +
4 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 9104496..f91a9d0 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -29,6 +29,7 @@
unsigned int ath10k_debug_mask;
EXPORT_SYMBOL(ath10k_debug_mask);
+unsigned long ath10k_history_enable_mask;
static unsigned int ath10k_cryptmode_param;
static bool uart_print;
static bool skip_otp;
@@ -46,6 +47,7 @@ module_param(skip_otp, bool, 0644);
module_param(rawmode, bool, 0644);
module_param(fw_diag_log, bool, 0644);
module_param_named(coredump_mask, ath10k_coredump_mask, ulong, 0444);
+module_param_named(history_enable_mask, ath10k_history_enable_mask, ulong, 0444);
MODULE_PARM_DESC(debug_mask, "Debugging mask");
MODULE_PARM_DESC(uart_print, "Uart target debugging");
@@ -54,6 +56,7 @@ MODULE_PARM_DESC(cryptmode, "Crypto mode: 0-hardware, 1-software");
MODULE_PARM_DESC(rawmode, "Use raw 802.11 frame datapath");
MODULE_PARM_DESC(coredump_mask, "Bitfield of what to include in firmware crash file");
MODULE_PARM_DESC(fw_diag_log, "Diag based fw log debugging");
+MODULE_PARM_DESC(history_enable_mask, "Enable events history recording");
static const struct ath10k_hw_params ath10k_hw_params_list[] = {
{
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 46bd5aa..ce429df 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -977,6 +977,14 @@ struct ath10k_bus_params {
#define ATH10K_WMI_DATA_LEN 16
+enum ath10k_history {
+ ATH10K_REG_ACCESS_HISTORY,
+ ATH10K_CE_EVENTS_HISTORY,
+ ATH10K_WMI_CMD_HISTORY,
+ ATH10K_WMI_EVENTS_HISTORY,
+ ATH10K_HISTORY_MAX,
+};
+
enum ath10k_ce_event {
ATH10K_IRQ_TRIGGER,
ATH10K_NAPI_POLL,
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 9105b0b..5d08652 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2724,6 +2724,9 @@ EXPORT_SYMBOL(ath10k_dbg_dump);
int ath10k_core_reg_access_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_REG_ACCESS_HISTORY, &ath10k_history_enable_mask))
+ return 0;
+
ar->reg_access_history.record = vzalloc(max_entries *
sizeof(struct ath10k_reg_access_entry));
if (!ar->reg_access_history.record)
@@ -2739,6 +2742,9 @@ EXPORT_SYMBOL(ath10k_core_reg_access_history_init);
int ath10k_core_wmi_cmd_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_WMI_CMD_HISTORY, &ath10k_history_enable_mask))
+ return 0;
+
ar->wmi_cmd_history.record = vzalloc(max_entries *
sizeof(struct ath10k_wmi_event_entry));
if (!ar->wmi_cmd_history.record)
@@ -2754,6 +2760,9 @@ EXPORT_SYMBOL(ath10k_core_wmi_cmd_history_init);
int ath10k_core_wmi_event_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_WMI_EVENTS_HISTORY, &ath10k_history_enable_mask))
+ return 0;
+
ar->wmi_event_history.record = vzalloc(max_entries *
sizeof(struct ath10k_wmi_event_entry));
if (!ar->wmi_event_history.record)
@@ -2769,6 +2778,9 @@ EXPORT_SYMBOL(ath10k_core_wmi_event_history_init);
int ath10k_core_ce_event_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_CE_EVENTS_HISTORY, &ath10k_history_enable_mask))
+ return 0;
+
ar->ce_event_history.record = vzalloc(max_entries *
sizeof(struct ath10k_ce_event_entry));
if (!ar->ce_event_history.record)
@@ -2787,7 +2799,8 @@ void ath10k_record_reg_access(struct ath10k *ar, u32 offset, u32 val, bool write
struct ath10k_reg_access_entry *entry;
u32 idx;
- if (!ar->reg_access_history.record)
+ if (!test_bit(ATH10K_REG_ACCESS_HISTORY, &ath10k_history_enable_mask) ||
+ !ar->reg_access_history.record)
return;
idx = ath10k_core_get_next_idx(&ar->reg_access_history.index,
@@ -2808,7 +2821,9 @@ void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
u32 idx;
if (type == ATH10K_WMI_EVENT) {
- if (!ar->wmi_event_history.record)
+ if (!test_bit(ATH10K_WMI_EVENTS_HISTORY,
+ &ath10k_history_enable_mask) ||
+ !ar->wmi_event_history.record)
return;
spin_lock_bh(&ar->wmi_event_history.hist_lock);
@@ -2817,7 +2832,9 @@ void ath10k_record_wmi_event(struct ath10k *ar, enum ath10k_wmi_type type,
spin_unlock_bh(&ar->wmi_event_history.hist_lock);
entry = &ar->wmi_event_history.record[idx];
} else {
- if (!ar->wmi_cmd_history.record)
+ if (!test_bit(ATH10K_WMI_CMD_HISTORY,
+ &ath10k_history_enable_mask) ||
+ !ar->wmi_event_history.record)
return;
spin_lock_bh(&ar->wmi_cmd_history.hist_lock);
@@ -2841,7 +2858,8 @@ void ath10k_record_ce_event(struct ath10k *ar, enum ath10k_ce_event event_type,
struct ath10k_ce_event_entry *entry;
u32 idx;
- if (!ar->ce_event_history.record)
+ if (!test_bit(ATH10K_CE_EVENTS_HISTORY, &ath10k_history_enable_mask) ||
+ !ar->ce_event_history.record)
return;
idx = ath10k_core_get_next_idx(&ar->ce_event_history.index,
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index c28aeb1..7799b89 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -75,6 +75,7 @@ struct ath10k_pktlog_hdr {
#define ATH10K_TX_POWER_MIN_VAL 0
extern unsigned int ath10k_debug_mask;
+extern unsigned long ath10k_history_enable_mask;
__printf(2, 3) void ath10k_info(struct ath10k *ar, const char *fmt, ...);
__printf(2, 3) void ath10k_err(struct ath10k *ar, const char *fmt, ...);
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 3/3] ath10k: Add debugfs support to enable event history
2020-07-31 18:27 ` Rakesh Pillai
@ 2020-07-31 18:27 ` Rakesh Pillai
-1 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-07-31 18:27 UTC (permalink / raw)
To: ath10k
Cc: netdev, linux-wireless, linux-kernel, Rakesh Pillai, kuba, davem,
kvalo
Add the support to enable/disable the recording of
debug events history.
The enable/disable of the history from debugfs will
not make any affect if its not enabled via module
parameter.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/debug.c | 56 +++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 5d08652..6785fae 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -610,6 +610,59 @@ static const struct file_operations fops_simulate_fw_crash = {
.llseek = default_llseek,
};
+static ssize_t ath10k_read_history_enable(struct file *file,
+ char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ const char buf[] =
+ "To enable recording of certain event history, write to this file with the enable mask\n"
+ "BIT(0): Enable Reg Access history\n"
+ " - Register write events\n"
+ " - Register read events\n"
+ "BIT(1): Enable CE events history\n"
+ " - ATH10K_IRQ_TRIGGER event\n"
+ " - ATH10K_NAPI_POLL event\n"
+ " - ATH10K_CE_SERVICE event\n"
+ " - ATH10K_NAPI_COMPLETE event\n"
+ " - ATH10K_NAPI_RESCHED event\n"
+ " - ATH10K_IRQ_SUMMARY event\n"
+ "BIT(2): Enable WMI CMD history\n"
+ " - WMI CMD event\n"
+ " - WMI CMD TX completion event\n"
+ "BIT(3): Enable WMI events history\n"
+ " - WMI Events event\n";
+
+ return simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
+}
+
+static ssize_t ath10k_write_history_enable(struct file *file,
+ const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ u32 history_enable_mask;
+ int i, ret;
+
+ ret = kstrtou32_from_user(user_buf, count, 0, &history_enable_mask);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < ATH10K_HISTORY_MAX; i++)
+ if (history_enable_mask & BIT(i))
+ set_bit(i, &ath10k_history_enable_mask);
+ else
+ clear_bit(i, &ath10k_history_enable_mask);
+
+ return count;
+}
+
+static const struct file_operations fops_history_enable = {
+ .read = ath10k_read_history_enable,
+ .write = ath10k_write_history_enable,
+ .open = simple_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
static ssize_t ath10k_read_chip_id(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
@@ -2658,6 +2711,9 @@ int ath10k_debug_register(struct ath10k *ar)
debugfs_create_file("reset_htt_stats", 0200, ar->debug.debugfs_phy, ar,
&fops_reset_htt_stats);
+ debugfs_create_file("history_enable", 0644, ar->debug.debugfs_phy, ar,
+ &fops_history_enable);
+
return 0;
}
--
2.7.4
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 3/3] ath10k: Add debugfs support to enable event history
@ 2020-07-31 18:27 ` Rakesh Pillai
0 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-07-31 18:27 UTC (permalink / raw)
To: ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev,
Rakesh Pillai
Add the support to enable/disable the recording of
debug events history.
The enable/disable of the history from debugfs will
not make any affect if its not enabled via module
parameter.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/debug.c | 56 +++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 5d08652..6785fae 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -610,6 +610,59 @@ static const struct file_operations fops_simulate_fw_crash = {
.llseek = default_llseek,
};
+static ssize_t ath10k_read_history_enable(struct file *file,
+ char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ const char buf[] =
+ "To enable recording of certain event history, write to this file with the enable mask\n"
+ "BIT(0): Enable Reg Access history\n"
+ " - Register write events\n"
+ " - Register read events\n"
+ "BIT(1): Enable CE events history\n"
+ " - ATH10K_IRQ_TRIGGER event\n"
+ " - ATH10K_NAPI_POLL event\n"
+ " - ATH10K_CE_SERVICE event\n"
+ " - ATH10K_NAPI_COMPLETE event\n"
+ " - ATH10K_NAPI_RESCHED event\n"
+ " - ATH10K_IRQ_SUMMARY event\n"
+ "BIT(2): Enable WMI CMD history\n"
+ " - WMI CMD event\n"
+ " - WMI CMD TX completion event\n"
+ "BIT(3): Enable WMI events history\n"
+ " - WMI Events event\n";
+
+ return simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
+}
+
+static ssize_t ath10k_write_history_enable(struct file *file,
+ const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ u32 history_enable_mask;
+ int i, ret;
+
+ ret = kstrtou32_from_user(user_buf, count, 0, &history_enable_mask);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < ATH10K_HISTORY_MAX; i++)
+ if (history_enable_mask & BIT(i))
+ set_bit(i, &ath10k_history_enable_mask);
+ else
+ clear_bit(i, &ath10k_history_enable_mask);
+
+ return count;
+}
+
+static const struct file_operations fops_history_enable = {
+ .read = ath10k_read_history_enable,
+ .write = ath10k_write_history_enable,
+ .open = simple_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
static ssize_t ath10k_read_chip_id(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
@@ -2658,6 +2711,9 @@ int ath10k_debug_register(struct ath10k *ar)
debugfs_create_file("reset_htt_stats", 0200, ar->debug.debugfs_phy, ar,
&fops_reset_htt_stats);
+ debugfs_create_file("history_enable", 0644, ar->debug.debugfs_phy, ar,
+ &fops_history_enable);
+
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/3]
2020-07-31 18:27 ` Rakesh Pillai
@ 2020-07-31 18:46 ` Florian Fainelli
-1 siblings, 0 replies; 28+ messages in thread
From: Florian Fainelli @ 2020-07-31 18:46 UTC (permalink / raw)
To: Rakesh Pillai, ath10k
Cc: netdev, linux-wireless, linux-kernel, kuba, davem, kvalo
On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> The history recording will be compiled only if
> ATH10K_DEBUG is enabled, and also enabled via
> the module parameter. Once the history recording
> is enabled via module parameter, it can be enabled
> or disabled runtime via debugfs.
Why not use trace prints and retrieving them via the function tracer?
This seems very ad-hoc.
>
> ---
> Changes from v1:
> - Add module param and debugfs to enable/disable history recording.
>
> Rakesh Pillai (3):
> ath10k: Add history for tracking certain events
> ath10k: Add module param to enable history
> ath10k: Add debugfs support to enable event history
>
> drivers/net/wireless/ath/ath10k/ce.c | 1 +
> drivers/net/wireless/ath/ath10k/core.c | 3 +
> drivers/net/wireless/ath/ath10k/core.h | 82 ++++++++++++
> drivers/net/wireless/ath/ath10k/debug.c | 207 ++++++++++++++++++++++++++++++
> drivers/net/wireless/ath/ath10k/debug.h | 75 +++++++++++
> drivers/net/wireless/ath/ath10k/snoc.c | 15 ++-
> drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> drivers/net/wireless/ath/ath10k/wmi.c | 10 ++
> 8 files changed, 393 insertions(+), 1 deletion(-)
>
--
Florian
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/3]
@ 2020-07-31 18:46 ` Florian Fainelli
0 siblings, 0 replies; 28+ messages in thread
From: Florian Fainelli @ 2020-07-31 18:46 UTC (permalink / raw)
To: Rakesh Pillai, ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev
On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> The history recording will be compiled only if
> ATH10K_DEBUG is enabled, and also enabled via
> the module parameter. Once the history recording
> is enabled via module parameter, it can be enabled
> or disabled runtime via debugfs.
Why not use trace prints and retrieving them via the function tracer?
This seems very ad-hoc.
>
> ---
> Changes from v1:
> - Add module param and debugfs to enable/disable history recording.
>
> Rakesh Pillai (3):
> ath10k: Add history for tracking certain events
> ath10k: Add module param to enable history
> ath10k: Add debugfs support to enable event history
>
> drivers/net/wireless/ath/ath10k/ce.c | 1 +
> drivers/net/wireless/ath/ath10k/core.c | 3 +
> drivers/net/wireless/ath/ath10k/core.h | 82 ++++++++++++
> drivers/net/wireless/ath/ath10k/debug.c | 207 ++++++++++++++++++++++++++++++
> drivers/net/wireless/ath/ath10k/debug.h | 75 +++++++++++
> drivers/net/wireless/ath/ath10k/snoc.c | 15 ++-
> drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> drivers/net/wireless/ath/ath10k/wmi.c | 10 ++
> 8 files changed, 393 insertions(+), 1 deletion(-)
>
--
Florian
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH v2 0/3]
2020-07-31 18:46 ` Florian Fainelli
@ 2020-08-01 5:10 ` Rakesh Pillai
-1 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-08-01 5:10 UTC (permalink / raw)
To: 'Florian Fainelli', ath10k
Cc: netdev, linux-wireless, linux-kernel, kuba, davem, kvalo
> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Saturday, August 1, 2020 12:17 AM
> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2 0/3]
>
> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> > The history recording will be compiled only if
> > ATH10K_DEBUG is enabled, and also enabled via
> > the module parameter. Once the history recording
> > is enabled via module parameter, it can be enabled
> > or disabled runtime via debugfs.
>
> Why not use trace prints and retrieving them via the function tracer?
> This seems very ad-hoc.
Tracing needs to be enabled to capture the events.
But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
It wont even allocate memory if not enabled via module parameter.
>
> >
> > ---
> > Changes from v1:
> > - Add module param and debugfs to enable/disable history recording.
> >
> > Rakesh Pillai (3):
> > ath10k: Add history for tracking certain events
> > ath10k: Add module param to enable history
> > ath10k: Add debugfs support to enable event history
> >
> > drivers/net/wireless/ath/ath10k/ce.c | 1 +
> > drivers/net/wireless/ath/ath10k/core.c | 3 +
> > drivers/net/wireless/ath/ath10k/core.h | 82 ++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.c | 207
> ++++++++++++++++++++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.h | 75 +++++++++++
> > drivers/net/wireless/ath/ath10k/snoc.c | 15 ++-
> > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> > drivers/net/wireless/ath/ath10k/wmi.c | 10 ++
> > 8 files changed, 393 insertions(+), 1 deletion(-)
> >
>
>
> --
> Florian
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH v2 0/3]
@ 2020-08-01 5:10 ` Rakesh Pillai
0 siblings, 0 replies; 28+ messages in thread
From: Rakesh Pillai @ 2020-08-01 5:10 UTC (permalink / raw)
To: 'Florian Fainelli', ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev
> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Saturday, August 1, 2020 12:17 AM
> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2 0/3]
>
> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> > The history recording will be compiled only if
> > ATH10K_DEBUG is enabled, and also enabled via
> > the module parameter. Once the history recording
> > is enabled via module parameter, it can be enabled
> > or disabled runtime via debugfs.
>
> Why not use trace prints and retrieving them via the function tracer?
> This seems very ad-hoc.
Tracing needs to be enabled to capture the events.
But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
It wont even allocate memory if not enabled via module parameter.
>
> >
> > ---
> > Changes from v1:
> > - Add module param and debugfs to enable/disable history recording.
> >
> > Rakesh Pillai (3):
> > ath10k: Add history for tracking certain events
> > ath10k: Add module param to enable history
> > ath10k: Add debugfs support to enable event history
> >
> > drivers/net/wireless/ath/ath10k/ce.c | 1 +
> > drivers/net/wireless/ath/ath10k/core.c | 3 +
> > drivers/net/wireless/ath/ath10k/core.h | 82 ++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.c | 207
> ++++++++++++++++++++++++++++++
> > drivers/net/wireless/ath/ath10k/debug.h | 75 +++++++++++
> > drivers/net/wireless/ath/ath10k/snoc.c | 15 ++-
> > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> > drivers/net/wireless/ath/ath10k/wmi.c | 10 ++
> > 8 files changed, 393 insertions(+), 1 deletion(-)
> >
>
>
> --
> Florian
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/3]
2020-08-01 5:10 ` Rakesh Pillai
@ 2020-08-01 5:24 ` Florian Fainelli
-1 siblings, 0 replies; 28+ messages in thread
From: Florian Fainelli @ 2020-08-01 5:24 UTC (permalink / raw)
To: Rakesh Pillai, ath10k
Cc: netdev, linux-wireless, linux-kernel, kuba, davem, kvalo
On 7/31/2020 10:10 PM, Rakesh Pillai wrote:
>
>
>> -----Original Message-----
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Sent: Saturday, August 1, 2020 12:17 AM
>> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
>> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
>> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
>> netdev@vger.kernel.org
>> Subject: Re: [PATCH v2 0/3]
>>
>> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
>>> The history recording will be compiled only if
>>> ATH10K_DEBUG is enabled, and also enabled via
>>> the module parameter. Once the history recording
>>> is enabled via module parameter, it can be enabled
>>> or disabled runtime via debugfs.
>>
>> Why not use trace prints and retrieving them via the function tracer?
>> This seems very ad-hoc.
>
> Tracing needs to be enabled to capture the events.
> But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
> It wont even allocate memory if not enabled via module parameter.
I would suggest researching what other drivers do and also considering
the benefits, for someone doing system analysis of plugging into the
kernel's general tracing mechanism to have all information in the same
place and just do filtering on the record/report side.
--
Florian
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/3]
@ 2020-08-01 5:24 ` Florian Fainelli
0 siblings, 0 replies; 28+ messages in thread
From: Florian Fainelli @ 2020-08-01 5:24 UTC (permalink / raw)
To: Rakesh Pillai, ath10k
Cc: linux-wireless, linux-kernel, kvalo, davem, kuba, netdev
On 7/31/2020 10:10 PM, Rakesh Pillai wrote:
>
>
>> -----Original Message-----
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Sent: Saturday, August 1, 2020 12:17 AM
>> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
>> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
>> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
>> netdev@vger.kernel.org
>> Subject: Re: [PATCH v2 0/3]
>>
>> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
>>> The history recording will be compiled only if
>>> ATH10K_DEBUG is enabled, and also enabled via
>>> the module parameter. Once the history recording
>>> is enabled via module parameter, it can be enabled
>>> or disabled runtime via debugfs.
>>
>> Why not use trace prints and retrieving them via the function tracer?
>> This seems very ad-hoc.
>
> Tracing needs to be enabled to capture the events.
> But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
> It wont even allocate memory if not enabled via module parameter.
I would suggest researching what other drivers do and also considering
the benefits, for someone doing system analysis of plugging into the
kernel's general tracing mechanism to have all information in the same
place and just do filtering on the record/report side.
--
Florian
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/3]
2020-07-31 18:27 ` Rakesh Pillai
@ 2020-07-31 20:31 ` Jakub Kicinski
-1 siblings, 0 replies; 28+ messages in thread
From: Jakub Kicinski @ 2020-07-31 20:31 UTC (permalink / raw)
To: Rakesh Pillai; +Cc: netdev, linux-wireless, linux-kernel, ath10k, davem, kvalo
On Fri, 31 Jul 2020 23:57:19 +0530 Rakesh Pillai wrote:
> The history recording will be compiled only if
> ATH10K_DEBUG is enabled, and also enabled via
> the module parameter. Once the history recording
> is enabled via module parameter, it can be enabled
> or disabled runtime via debugfs.
Have you seen the trace_devlink_hwmsg() interface?
Could it be used here?
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 28+ messages in thread