Re: [PATCH v2] ath: Add support for tracing
Kalle Valo wrote: The current debug printing code in ath10k is something like 100 lines, I don't see the point of trying to make that common with all ath* drivers. In my opinion ath.ko should only have code which used at least two different drivers (and I consider ath9k.ko and ath9k_htc.ko as one driver). So the right thing here would be to actually move all debugging code from ath.ko to ath9k, as it's the only user anyway. I don't think anyone is interested in moving debug code from ath.ko to ath9k.ko at this point. There is no benefit and it is just code churn. Sujith -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ath: Add support for tracing
Sujith Manoharan suj...@msujith.org writes: From: Sujith Manoharan c_man...@qca.qualcomm.com Signed-off-by: Sujith Manoharan c_man...@qca.qualcomm.com --- drivers/net/wireless/ath/Kconfig | 8 + drivers/net/wireless/ath/Makefile | 4 +++ drivers/net/wireless/ath/ath.h| 1 + drivers/net/wireless/ath/main.c | 3 ++ drivers/net/wireless/ath/trace.c | 20 +++ drivers/net/wireless/ath/trace.h | 71 +++ Why add it to ath.ko module? What drivers are going to use this? Because all ath* drivers are different, I suspect there is not that much to share between drivers and that's why it would be better to have the trace points per driver. And I just checked that currently we have trace points for ath10k, ath5k, ath6kl and wil6210. IMHO adding more trace points to ath.ko just makes this confusing. -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ath: Add support for tracing
Kalle Valo wrote: Why add it to ath.ko module? What drivers are going to use this? The debug code for ath9k is in ath.ko. Sujith -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ath: Add support for tracing
Sujith Manoharan suj...@msujith.org writes: Kalle Valo wrote: Why add it to ath.ko module? What drivers are going to use this? The debug code for ath9k is in ath.ko. You mean ath_printk() friends? But that doesn't require tracing code to be in ath.ko as well, right? If I understood correctly, trace.c could be under ath9k directory and the kconfig option could be ATH9K_TRACEPOINTS. I think it's just misleading and confusing for the user to call it Atheros wireless tracing when it only affects ath9k. It's easier to understand if each driver has it's own tracing kconfig option. -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ath: Add support for tracing
Kalle Valo wrote: You mean ath_printk() friends? But that doesn't require tracing code to be in ath.ko as well, right? If I understood correctly, trace.c could be under ath9k directory and the kconfig option could be ATH9K_TRACEPOINTS. I think it's just misleading and confusing for the user to call it Atheros wireless tracing when it only affects ath9k. It's easier to understand if each driver has it's own tracing kconfig option. We have CONFIG_ATH_DEBUG, which is used by ath9k and ath9k_htc. I think it is okay to have CONFIG_ATH_TRACEPOINTS, which can be used by ath9k/ath9k_htc too. The original motive of ath.ko was to have a common module with debugging facilities that can be shared by Atheros drivers. But, each driver has ended up reinventing things. Since it is mentioned in the help text that ath9k is the only driver making use of ATH_DEBUG/ATH_TRACEPOINTS, I don't think it is confusing. Sujith -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ath: Add support for tracing
Sujith Manoharan suj...@msujith.org writes: Kalle Valo wrote: You mean ath_printk() friends? But that doesn't require tracing code to be in ath.ko as well, right? If I understood correctly, trace.c could be under ath9k directory and the kconfig option could be ATH9K_TRACEPOINTS. I think it's just misleading and confusing for the user to call it Atheros wireless tracing when it only affects ath9k. It's easier to understand if each driver has it's own tracing kconfig option. We have CONFIG_ATH_DEBUG, which is used by ath9k and ath9k_htc. I think it is okay to have CONFIG_ATH_TRACEPOINTS, which can be used by ath9k/ath9k_htc too. In my opinion that just creates even a bigger mess. The original motive of ath.ko was to have a common module with debugging facilities that can be shared by Atheros drivers. But, each driver has ended up reinventing things. The current debug printing code in ath10k is something like 100 lines, I don't see the point of trying to make that common with all ath* drivers. In my opinion ath.ko should only have code which used at least two different drivers (and I consider ath9k.ko and ath9k_htc.ko as one driver). So the right thing here would be to actually move all debugging code from ath.ko to ath9k, as it's the only user anyway. Since it is mentioned in the help text that ath9k is the only driver making use of ATH_DEBUG/ATH_TRACEPOINTS, I don't think it is confusing. To me it is. But it's a mess already so I guess I'm worrying nothing. -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] ath: Add support for tracing
From: Sujith Manoharan c_man...@qca.qualcomm.com
Signed-off-by: Sujith Manoharan c_man...@qca.qualcomm.com
---
drivers/net/wireless/ath/Kconfig | 8 +
drivers/net/wireless/ath/Makefile | 4 +++
drivers/net/wireless/ath/ath.h| 1 +
drivers/net/wireless/ath/main.c | 3 ++
drivers/net/wireless/ath/trace.c | 20 +++
drivers/net/wireless/ath/trace.h | 71 +++
6 files changed, 107 insertions(+)
create mode 100644 drivers/net/wireless/ath/trace.c
create mode 100644 drivers/net/wireless/ath/trace.h
diff --git a/drivers/net/wireless/ath/Kconfig b/drivers/net/wireless/ath/Kconfig
index c63d115..ce78260 100644
--- a/drivers/net/wireless/ath/Kconfig
+++ b/drivers/net/wireless/ath/Kconfig
@@ -25,6 +25,14 @@ config ATH_DEBUG
Say Y, if you want to debug atheros wireless drivers.
Right now only ath9k makes use of this.
+config ATH_TRACEPOINTS
+ bool Atheros wireless tracing
+ depends on ATH_DEBUG
+ depends on EVENT_TRACING
+ ---help---
+ This option enables tracepoints for atheros wireless drivers.
+Currently, ath9k makes use of this facility.
+
config ATH_REG_DYNAMIC_USER_REG_HINTS
bool Atheros dynamic user regulatory hints
depends on CFG80211_CERTIFICATION_ONUS
diff --git a/drivers/net/wireless/ath/Makefile
b/drivers/net/wireless/ath/Makefile
index 7d023b0..89f8d59 100644
--- a/drivers/net/wireless/ath/Makefile
+++ b/drivers/net/wireless/ath/Makefile
@@ -17,4 +17,8 @@ ath-objs := main.o \
dfs_pri_detector.o
ath-$(CONFIG_ATH_DEBUG) += debug.o
+ath-$(CONFIG_ATH_TRACEPOINTS) += trace.o
+
ccflags-y += -D__CHECK_ENDIAN__
+
+CFLAGS_trace.o := -I$(src)
diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index a3b6e27..e5ba6fa 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -268,6 +268,7 @@ enum ATH_DEBUG {
};
#define ATH_DBG_DEFAULT (ATH_DBG_FATAL)
+#define ATH_DBG_MAX_LEN 512
#ifdef CONFIG_ATH_DEBUG
diff --git a/drivers/net/wireless/ath/main.c b/drivers/net/wireless/ath/main.c
index 8b0ac14..83f47af 100644
--- a/drivers/net/wireless/ath/main.c
+++ b/drivers/net/wireless/ath/main.c
@@ -20,6 +20,7 @@
#include linux/module.h
#include ath.h
+#include trace.h
MODULE_AUTHOR(Atheros Communications);
MODULE_DESCRIPTION(Shared library for Atheros wireless LAN cards.);
@@ -84,6 +85,8 @@ void ath_printk(const char *level, const struct ath_common*
common,
else
printk(%sath: %pV, level, vaf);
+ trace_ath_log(common-hw-wiphy, vaf);
+
va_end(args);
}
EXPORT_SYMBOL(ath_printk);
diff --git a/drivers/net/wireless/ath/trace.c b/drivers/net/wireless/ath/trace.c
new file mode 100644
index 000..18fb3a0
--- /dev/null
+++ b/drivers/net/wireless/ath/trace.c
@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2014 Qualcomm Atheros, Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include linux/module.h
+
+#define CREATE_TRACE_POINTS
+#include trace.h
diff --git a/drivers/net/wireless/ath/trace.h b/drivers/net/wireless/ath/trace.h
new file mode 100644
index 000..ba71164
--- /dev/null
+++ b/drivers/net/wireless/ath/trace.h
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2014 Qualcomm Atheros, Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#if !defined(_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_H
+
+#include linux/tracepoint.h
+#include ath.h
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM ath
+
+#if !defined(CONFIG_ATH_TRACEPOINTS)
+
+#undef TRACE_EVENT
+#define TRACE_EVENT(name,
