dynamic debug v2 - convert cpufreq

PATCH In review Jason Baron 3 participants 7 messages Jul 15, 2008 – Jul 17, 2008

Subsystems (3)

x86 Library code CPU frequency scaling framework

lore

Share & tools

lore

Jason Baron

author

2008-07-15 21:37 UTC #0 lore raw

PATCH 6/7 dynamic debug v2 - convert cpufreq

-convert cpufreq

Signed-off-by Jason Baron <[email protected]>

---

 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c       |    7 ++-
 arch/x86/kernel/cpu/cpufreq/cpufreq-nforce2.c    |    6 ++-
 arch/x86/kernel/cpu/cpufreq/gx-suspmod.c         |    7 ++-
 arch/x86/kernel/cpu/cpufreq/longhaul.c           |    7 ++-
 arch/x86/kernel/cpu/cpufreq/longrun.c            |    6 ++-
 arch/x86/kernel/cpu/cpufreq/p4-clockmod.c        |    7 +++
 arch/x86/kernel/cpu/cpufreq/powernow-k7.c        |    6 ++-
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c        |    2 -
 arch/x86/kernel/cpu/cpufreq/powernow-k8.h        |    5 ++
 arch/x86/kernel/cpu/cpufreq/sc520_freq.c         |    6 ++-
 arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c |    6 ++-
 arch/x86/kernel/cpu/cpufreq/speedstep-ich.c      |    8 ++--
 arch/x86/kernel/cpu/cpufreq/speedstep-smi.c      |    6 ++-
 drivers/cpufreq/cpufreq.c                        |   45 +++++++++++-----------
 drivers/cpufreq/cpufreq_performance.c            |    8 ++--
 drivers/cpufreq/cpufreq_powersave.c              |    7 ++-
 drivers/cpufreq/cpufreq_userspace.c              |    6 ++-
 drivers/cpufreq/freq_table.c                     |    7 ++-
 include/linux/cpufreq.h                          |   11 ++++-
 lib/dynamic_printk.c                             |    5 +-
 20 files changed, 116 insertions(+), 52 deletions(-)

arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c +5 -2

index e2d870d..b879ddd 100644
@@ -33,6 +33,7 @@
 #include <linux/cpufreq.h>
 #include <linux/compiler.h>
 #include <linux/dmi.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include <linux/acpi.h>
 #include <acpi/processor.h>

@@ -44,7 +45,10 @@
 #include <asm/delay.h>
 #include <asm/uaccess.h>
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "acpi-cpufreq", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "acpi-cpufreq", msg); \
+	} while (0)
 
 MODULE_AUTHOR("Paul Diefenbaugh, Dominik Brodowski");
 MODULE_DESCRIPTION("ACPI Processor P-States Driver");

@@ -780,7 +784,6 @@ static int __init acpi_cpufreq_init(void)
 static void __exit acpi_cpufreq_exit(void)
 {
 	dprintk("acpi_cpufreq_exit\n");
-
 	cpufreq_unregister_driver(&acpi_cpufreq_driver);
 
 	free_percpu(acpi_perf_data);

arch/x86/kernel/cpu/cpufreq/cpufreq-nforce2.c +5 -1

index f03e915..3e3a39c 100644
@@ -14,6 +14,7 @@
 #include <linux/cpufreq.h>
 #include <linux/pci.h>
 #include <linux/delay.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #define NFORCE2_XTAL 25
 #define NFORCE2_BOOTFSB 0x48

@@ -55,7 +56,10 @@ MODULE_PARM_DESC(fid, "CPU multiplier to use (11.5 = 115)");
 MODULE_PARM_DESC(min_fsb,
                  "Minimum FSB to use, if not defined: current FSB - 50");
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "cpufreq-nforce2", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "cpufreq-nforce2", msg); \
+	} while (0)
 
 /**
  * nforce2_calc_fsb - calculate FSB

arch/x86/kernel/cpu/cpufreq/gx-suspmod.c +5 -2

index 9d9eae8..e144095 100644
@@ -79,6 +79,7 @@
 #include <linux/smp.h>
 #include <linux/cpufreq.h>
 #include <linux/pci.h>
+#include <linux/dynamic_debug_cpufreq.h>
 #include <asm/processor-cyrix.h>
 #include <asm/errno.h>

@@ -139,8 +140,10 @@ module_param (max_duration, int, 0444);
  */
 #define POLICY_MIN_DIV 20
 
-
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "gx-suspmod", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "gx-suspmod", msg); \
+	} while (0)
 
 /**
  * we can detect a core multipiler from dir0_lsb

arch/x86/kernel/cpu/cpufreq/longhaul.c +5 -2

index 06fcce5..1a28f1f 100644
@@ -30,6 +30,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/delay.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include <asm/msr.h>
 #include <asm/timex.h>

@@ -78,8 +79,10 @@ static int scale_voltage;
 static int disable_acpi_c3;
 static int revid_errata;
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "longhaul", msg)
-
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "longhaul", msg); \
+	{ while (0)
 
 /* Clock ratios multiplied by 10 */
 static int clock_ratio[32];

arch/x86/kernel/cpu/cpufreq/longrun.c +5 -1

index af4a867..0f22a89 100644
@@ -11,12 +11,16 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/cpufreq.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include <asm/msr.h>
 #include <asm/processor.h>
 #include <asm/timex.h>
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "longrun", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "longrun", msg); \
+	} while (0)
 
 static struct cpufreq_driver	longrun_driver;

arch/x86/kernel/cpu/cpufreq/p4-clockmod.c +6 -1

index 199e4e0..107e1b6 100644
@@ -27,6 +27,7 @@
 #include <linux/cpufreq.h>
 #include <linux/slab.h>
 #include <linux/cpumask.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>

@@ -35,7 +36,11 @@
 #include "speedstep-lib.h"
 
 #define PFX	"p4-clockmod: "
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "p4-clockmod", msg)
+
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "p4-clockmod", msg); \
+	} while (0)
 
 /*
  * Duty Cycle (3bits), note DC_DISABLE is not specified in

arch/x86/kernel/cpu/cpufreq/powernow-k7.c +5 -1

index 0a61159..d9cdf4a 100644
@@ -20,6 +20,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/dmi.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include <asm/msr.h>
 #include <asm/timer.h>

@@ -101,7 +102,10 @@ static unsigned int fsb;
 static unsigned int latency;
 static char have_a0;
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "powernow-k7", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "powernow-k7", msg); \
+	} while (0)
 
 static int check_fsb(unsigned int fsbspeed)
 {

arch/x86/kernel/cpu/cpufreq/powernow-k8.c +1 -1

index 46d4034..f0bb7b0 100644
@@ -33,6 +33,7 @@
 #include <linux/string.h>
 #include <linux/cpumask.h>
 #include <linux/sched.h>	/* for current / set_cpus_allowed() */
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include <asm/msr.h>
 #include <asm/io.h>

@@ -1306,7 +1307,6 @@ static int __cpuinit powernowk8_init(void)
 static void __exit powernowk8_exit(void)
 {
 	dprintk("exit\n");
-
 	cpufreq_unregister_driver(&cpufreq_amd64_driver);
 }

arch/x86/kernel/cpu/cpufreq/powernow-k8.h +4 -1

index ab48cfe..0683a2d 100644
@@ -199,7 +199,10 @@ struct pst_s {
 	u8 vid;
 };
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "powernow-k8", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "powernow-k8", msg); \
+	} while (0)
 
 static int core_voltage_pre_transition(struct powernow_k8_data *data, u32 reqvid);
 static int core_voltage_post_transition(struct powernow_k8_data *data, u32 reqvid);

arch/x86/kernel/cpu/cpufreq/sc520_freq.c +5 -1

index 42da9bd..b5bc7fb 100644
@@ -19,6 +19,7 @@
 
 #include <linux/delay.h>
 #include <linux/cpufreq.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include <asm/msr.h>
 #include <asm/timex.h>

@@ -29,7 +30,10 @@
 
 static __u8 __iomem *cpuctl;
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "sc520_freq", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "sc520_freq", msg); \
+	} while (0)
 
 static struct cpufreq_frequency_table sc520_freq_table[] = {
 	{0x01,	100000},

arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c +5 -1

index 908dd34..cb50f37 100644
@@ -20,6 +20,7 @@
 #include <linux/sched.h>	/* current */
 #include <linux/delay.h>
 #include <linux/compiler.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include <asm/msr.h>
 #include <asm/processor.h>

@@ -28,7 +29,10 @@
 #define PFX		"speedstep-centrino: "
 #define MAINTAINER	"[email protected]"
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "speedstep-centrino", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "speedstep-centrino", msg); \
+	} while (0)
 
 #define INTEL_MSR_RANGE	(0xffff)

arch/x86/kernel/cpu/cpufreq/speedstep-ich.c +5 -3

index 1b50244..6aee9a5 100644
@@ -25,6 +25,7 @@
 #include <linux/pci.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include "speedstep-lib.h"

@@ -53,9 +54,10 @@ static struct cpufreq_frequency_table speedstep_freqs[] = {
 	{0,			CPUFREQ_TABLE_END},
 };
 
-
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "speedstep-ich", msg)
-
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "speedstep-ich", msg); \
+	} while (0)
 
 /**
  * speedstep_find_register - read the PMBASE address

arch/x86/kernel/cpu/cpufreq/speedstep-smi.c +5 -1

index 8a85c93..4819908 100644
@@ -19,6 +19,7 @@
 #include <linux/cpufreq.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
+#include <linux/dynamic_debug_cpufreq.h>
 #include <asm/ist.h>
 #include <asm/io.h>

@@ -56,7 +57,10 @@ static struct cpufreq_frequency_table speedstep_freqs[] = {
  * of DMA activity going on? */
 #define SMI_TRIES 5
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "speedstep-smi", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "speedstep-smi", msg); \
+	} while (0)
 
 /**
  * speedstep_smi_ownership

drivers/cpufreq/cpufreq.c +23 ~~-22~~

76 lines not on this page · raw message

drivers/cpufreq/cpufreq_performance.c +5 -3

index e8e1451..985ba2b 100644
@@ -14,10 +14,12 @@
 #include <linux/module.h>
 #include <linux/cpufreq.h>
 #include <linux/init.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
-#define dprintk(msg...) \
-	cpufreq_debug_printk(CPUFREQ_DEBUG_GOVERNOR, "performance", msg)
-
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_GOVERNOR, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_GOVERNOR, "performance", msg); \
+	} while (0)
 
 static int cpufreq_governor_performance(struct cpufreq_policy *policy,
 					unsigned int event)

drivers/cpufreq/cpufreq_powersave.c +5 -2

index 13fe06b..727f8c0 100644
@@ -14,9 +14,12 @@
 #include <linux/module.h>
 #include <linux/cpufreq.h>
 #include <linux/init.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
-#define dprintk(msg...) \
-	cpufreq_debug_printk(CPUFREQ_DEBUG_GOVERNOR, "powersave", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_GOVERNOR, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_GOVERNOR, "powersave", msg); \
+	} while (0)
 
 static int cpufreq_governor_powersave(struct cpufreq_policy *policy,
 					unsigned int event)

drivers/cpufreq/cpufreq_userspace.c +5 -1

index cb2ac01..2b77964 100644
@@ -23,6 +23,7 @@
 #include <linux/fs.h>
 #include <linux/sysfs.h>
 #include <linux/mutex.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
 #include <asm/uaccess.h>

@@ -39,7 +40,10 @@ static unsigned int	cpu_is_managed[NR_CPUS];
 static DEFINE_MUTEX	(userspace_mutex);
 static int cpus_using_userspace_governor;
 
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_GOVERNOR, "userspace", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_GOVERNOR, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_GOVERNOR, "userspace", msg); \
+	} while (0)
 
 /* keep track of frequency transitions */
 static int

drivers/cpufreq/freq_table.c +5 -2

index ae6cd60..2f241ce 100644
@@ -8,9 +8,12 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/cpufreq.h>
+#include <linux/dynamic_debug_cpufreq.h>
 
-#define dprintk(msg...) \
-	cpufreq_debug_printk(CPUFREQ_DEBUG_CORE, "freq-table", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_CORE, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_CORE, "freq-table", msg); \
+	} while (0)
 
 /*********************************************************************
  *                     FREQUENCY TABLE HELPERS                       *

include/linux/cpufreq.h +9 -2

index ddd8652..b056dfd 100644
@@ -366,11 +366,18 @@ void cpufreq_frequency_table_put_attr(unsigned int cpu);
 #define CPUFREQ_DEBUG_DRIVER	2
 #define CPUFREQ_DEBUG_GOVERNOR	4
 
-#ifdef CONFIG_CPU_FREQ_DEBUG
+#define CPUFREQ_DEBUG_FLAGS_NUM 3
+#define CPUFREQ_DEBUG_FLAGS "CPUFREQ_DEBUG_CORE,CPUFREQ_DEBUG_DRIVER,CPUFREQ_DEBUG_GOVERNOR"
+
+extern unsigned int cpufreq_debug;
+#if defined(CONFIG_CPU_FREQ_DEBUG) && !defined(CONFIG_DYNAMIC_PRINTK_DEBUG)
+#define DEBUG 1
+#endif
+
+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)
 
 extern void cpufreq_debug_printk(unsigned int type, const char *prefix, 
 				 const char *fmt, ...);
-
 #else
 
 #define cpufreq_debug_printk(msg...) do { } while(0)

lib/dynamic_printk.c +3 -2

index 326355d..1bc070f 100644
@@ -209,7 +209,8 @@ int register_debug_module(char *mod_name, int type, char *share_name,
 			num_enabled++;
 		}
 	}
-	if (strcmp(mod_name, share_name)) {
+	if (strcmp(mod_name, share_name) && (!elem->parent)) {
+		printk("register_debug_module: %s :%s\n", mod_name, share_name);
 		parent = find_debug_module(share_name);
 		if (!parent) {
 			__add_debug_module(share_name);

@@ -227,7 +228,7 @@ int register_debug_module(char *mod_name, int type, char *share_name,
 		list_add_rcu(&elem->shared_list, &parent->shared_list);
 	} else
 		elem->type = type;
-	if (num_flags > 0) {
+	if (num_flags > 0 && (!elem->flags)) {
 		if ((elem->type_sharing == SHARING_MEMBER) && elem->parent)
 			elem = elem->parent;
 		elem->flags = kzalloc(sizeof(struct flags_descriptor),

Dominik Brodowski

2008-07-15 23:07 UTC #1 lore raw

Hi,

Quoted 1 line · Jason Baron

+#include <linux/dynamic_debug_cpufreq.h>

what's contained in this file (couldn't find it in this or one of the other diffs, but may have missed it).

arch/x86/kernel/cpu/cpufreq/cpufreq-nforce2.c MODULE_PARM_DESC(fid, "CPU multiplier to use (11.5 = 115)"); · line 62

Quoted 5 lines

arch/x86/kernel/cpu/cpufreq/cpufreq-nforce2.c +4 -1

-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "cpufreq-nforce2", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "cpufreq-nforce2", msg); \
+	} while (0)

Hm.... What about leaving this as it is, renaming the drivers/cpufreq/cpufreq.c function to __cpufreq_debug_printk(), and then adding to include/linux/cpufreq.h

#define cpufreq_debug_printk(type, prefix, msg...) do { \
	if (dynamic_dbg_enabled(TYPE_FLAG, type, cpufreq_debug))
		cpufreq_debug_printk(type, prefix, msg); \
	} while (0)

Quoted 1 line

+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)

...

Quoted 1 line

+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)

can't we just depend on thing on another?

drivers/cpufreq/cpufreq.c void cpufreq_debug_printk(unsigned int type, const char *prefix, · line 253

Quoted 5 lines

drivers/cpufreq/cpufreq.c +2 -2

-module_param(debug, uint, 0644);
-MODULE_PARM_DESC(debug, "CPUfreq debugging: add 1 to debug core,"
+module_param(cpufreq_debug, uint, 0644);
+MODULE_PARM_DESC(cpufreq_debug, "CPUfreq debugging: add 1 to debug core,"
 			" 2 to debug drivers, and 4 to debug governors.");

cpufreq.cpufreq_debug is ugly and not backwards compatible... what about

module_param_named(debug, cpufreq_debug, uint, 0644) [or the other way around, I always forget...])

Best,
Dominik

Jason Baron

author

2008-07-17 21:06 UTC #2 lore raw

Quoted 9 lines · Dominik Brodowski

Hi,
Quoted 1 line · Jason Baron
+#include <linux/dynamic_debug_cpufreq.h>
what's contained in this file (couldn't find it in this or one of the other diffs, but may have missed it).

i forgot to include it. A full patch is found below.

Quoted 10 lines

Review
Quoted 5 lines
(quoted diff) +4 -1
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "cpufreq-nforce2", msg)
+#define dprintk(msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, CPUFREQ_DEBUG_DRIVER, cpufreq_debug)) \
+		cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "cpufreq-nforce2", msg); \
+	} while (0)
Hm.... What about leaving this as it is, renaming the drivers/cpufreq/cpufreq.c function to __cpufreq_debug_printk(), and then adding to include/linux/cpufreq.h

Quoted 5 lines

#define cpufreq_debug_printk(type, prefix, msg...) do { \
	if (dynamic_dbg_enabled(TYPE_FLAG, type, cpufreq_debug))
		cpufreq_debug_printk(type, prefix, msg); \
	} while (0)

i agree that would be a bit cleaner. the new patch below includes this suggestion.

Quoted 6 lines

Quoted 1 line
+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)
...
Quoted 1 line
+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)
can't we just depend on thing on another?

We could make CONFIG_CPU_FREQ_DEBUG force CONFIG_DYNAMIC_PRINTK_DEBUG to be on. However, i'm trying to allow CONFIG_CPU_FREQ_DEBUG to be turned on without enabling CONFIG_DYNAMIC_PRINTK_DEBUG. That's consistent with how i'm trying to do this patch series. That is, individual subsystems can turn their respective debugging on without forcing on CONFIG_DYNAMIC_PRINTK_DEBUG.

Quoted 11 lines

Review
Quoted 5 lines
(quoted diff) +2 -2
-module_param(debug, uint, 0644);
-MODULE_PARM_DESC(debug, "CPUfreq debugging: add 1 to debug core,"
+module_param(cpufreq_debug, uint, 0644);
+MODULE_PARM_DESC(cpufreq_debug, "CPUfreq debugging: add 1 to debug core,"
 			" 2 to debug drivers, and 4 to debug governors.");
cpufreq.cpufreq_debug is ugly and not backwards compatible... what about
module_param_named(debug, cpufreq_debug, uint, 0644) [or the other way around, I always forget...])

makes sense.

thanks,

-Jason

arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c +1 -0

index e2d870d..549edbd 100644
@@ -25,6 +25,7 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>

arch/x86/kernel/cpu/cpufreq/cpufreq-nforce2.c +1 -0

index f03e915..3eb343d 100644
@@ -7,6 +7,7 @@
  *  BIG FAT DISCLAIMER: Work in progress code. Possibly *dangerous*
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>

arch/x86/kernel/cpu/cpufreq/gx-suspmod.c +1 -0

index 9d9eae8..212204f 100644
@@ -73,6 +73,7 @@
  *			Suspend Modulation - Definitions		*
  ************************************************************************/
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>

arch/x86/kernel/cpu/cpufreq/longhaul.c +1 -0

index 06fcce5..b910935 100644
@@ -21,6 +21,7 @@
  *  BIG FAT DISCLAIMER: Work in progress code. Possibly *dangerous*
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>

arch/x86/kernel/cpu/cpufreq/longrun.c +1 -0

index af4a867..11ce4fa 100644
@@ -6,6 +6,7 @@
  *  BIG FAT DISCLAIMER: Work in progress code. Possibly *dangerous*
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>

arch/x86/kernel/cpu/cpufreq/p4-clockmod.c +1 -0

index 199e4e0..868cda4 100644
@@ -20,6 +20,7 @@
  *
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>

arch/x86/kernel/cpu/cpufreq/powernow-k7.c +1 -0

index 0a61159..1305afb 100644
@@ -12,6 +12,7 @@
  * - We disable half multipliers if ACPI is used on A0 stepping CPUs.
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>

arch/x86/kernel/cpu/cpufreq/powernow-k8.c +1 -0

index 46d4034..d18f9bd 100644
@@ -24,6 +24,7 @@
  *     http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/30430.pdf
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/smp.h>
 #include <linux/module.h>

arch/x86/kernel/cpu/cpufreq/sc520_freq.c +1 -0

index 42da9bd..7da87dd 100644
@@ -13,6 +13,7 @@
  *	2005-03-30: - initial revision
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>

arch/x86/kernel/cpu/cpufreq/speedstep-centrino.c +1 -0

index 908dd34..6fcd23f 100644
@@ -13,6 +13,7 @@
  * Copyright (C) 2003 Jeremy Fitzhardinge <[email protected]>
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>

arch/x86/kernel/cpu/cpufreq/speedstep-ich.c +1 -0

index 1b50244..43dd80e 100644
@@ -18,6 +18,7 @@
  *                        SPEEDSTEP - DEFINITIONS                    *
  *********************************************************************/
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>

arch/x86/kernel/cpu/cpufreq/speedstep-smi.c +1 -0

index 8a85c93..8aef4bf 100644
@@ -12,6 +12,7 @@
  *                        SPEEDSTEP - DEFINITIONS                    *
  *********************************************************************/
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>

drivers/cpufreq/cpufreq.c +20 ~~-22~~

75 lines not on this page · raw message

drivers/cpufreq/cpufreq_performance.c +1 -0

index e8e1451..cee0e66 100644
@@ -10,6 +10,7 @@
  *
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/cpufreq.h>

drivers/cpufreq/cpufreq_powersave.c +1 -0

index 13fe06b..4680ccc 100644
@@ -10,6 +10,7 @@
  *
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/cpufreq.h>

drivers/cpufreq/cpufreq_userspace.c +1 -0

index cb2ac01..8a33636 100644
@@ -11,6 +11,7 @@
  *
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/smp.h>

drivers/cpufreq/freq_table.c +1 -0

index ae6cd60..506cc33 100644
@@ -4,6 +4,7 @@
  * Copyright (C) 2002 - 2003 Dominik Brodowski
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>

include/linux/cpufreq.h +9 -4

index ddd8652..74ce70f 100644
@@ -366,14 +366,19 @@ void cpufreq_frequency_table_put_attr(unsigned int cpu);
 #define CPUFREQ_DEBUG_DRIVER	2
 #define CPUFREQ_DEBUG_GOVERNOR	4
 
-#ifdef CONFIG_CPU_FREQ_DEBUG
+#define cpufreq_debug_printk(flag, name, msg...) do { \
+	if (dynamic_dbg_enabled(TYPE_FLAG, flag, cpufreq_debug)) \
+		__cpufreq_debug_printk(flag, name, msg); \
+	} while (0)
 
-extern void cpufreq_debug_printk(unsigned int type, const char *prefix, 
-				 const char *fmt, ...);
+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)
 
+extern unsigned int cpufreq_debug;
+extern void __cpufreq_debug_printk(unsigned int type, const char *prefix, 
+				 const char *fmt, ...);
 #else
 
-#define cpufreq_debug_printk(msg...) do { } while(0)
+#define __cpufreq_debug_printk(msg...) do { } while(0)
 
 #endif /* CONFIG_CPU_FREQ_DEBUG */

include/linux/dynamic_debug_cpufreq.h new file +8 -0

new file mode 100644
index 0000000..b0cff6d
@@ -0,0 +1,8 @@
+#define DYNAMIC_DEBUG_NUM_FLAGS "3"
+#define DYNAMIC_DEBUG_FLAG_NAMES "CPUFREQ_DEBUG_CORE,CPUFREQ_DEBUG_DRIVER,CPUFREQ_DEBUG_GOVERNOR"
+#define DYNAMIC_DEBUG_TYPE "2"
+#define DYNAMIC_DEBUG_MODNAME "cpufreq_shared"
+
+#ifdef CONFIG_CPU_FREQ_DEBUG
+#define DEBUG 1
+#endif

Greg KH

2008-07-17 21:18 UTC #3 lore raw

arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c line 31

Quoted 10 lines · Jason Baron

arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c +1 -0

@@ -25,6 +25,7 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>

kernel.h includes dynamic_debug.h right? Then why not just put the _cpufreq.h stuff in the current cpufreq.h file(s)?

thanks,

greg k-h

Dominik Brodowski

2008-07-17 21:27 UTC #4 lore raw

Quoted 7 lines · Greg KH

Quoted 6 lines · Jason Baron
arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c +0 -0
@@ -25,6 +25,7 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 

Quoted 7 lines

arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c line 31
Quoted 4 lines
arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c +1 -0
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
kernel.h includes dynamic_debug.h right? Then why not just put the _cpufreq.h stuff in the current cpufreq.h file(s)?

Seconded. Makes the cpufreq-related patch even smaller, for we won't have to touch all drivers.

Quoted 4 lines · Jason Baron

Quoted 4 lines
Quoted 1 line
+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)
...
Quoted 1 line
+#if defined(CONFIG_CPU_FREQ_DEBUG) || defined (CONFIG_DYNAMIC_PRINTK_DEBUG)

Quoted 8 lines

Quoted 2 lines
can't we just depend on thing on another?
We could make CONFIG_CPU_FREQ_DEBUG force CONFIG_DYNAMIC_PRINTK_DEBUG to be on. However, i'm trying to allow CONFIG_CPU_FREQ_DEBUG to be turned on without enabling CONFIG_DYNAMIC_PRINTK_DEBUG. That's consistent with how i'm trying to do this patch series. That is, individual subsystems can turn their respective debugging on without forcing on CONFIG_DYNAMIC_PRINTK_DEBUG.

Makes sense, and we can discuss depending one thing on another later on.

Looking forward to seeing this infrastructure merged. I guess it will help a lot.

Best,
Dominik

Jason Baron

author

2008-07-17 21:47 UTC #5 lore raw

In reply to Greg KH

Quoted 7 lines · Greg KH

Quoted 6 lines · Jason Baron
arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c +0 -0
@@ -25,6 +25,7 @@
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  */
 

Quoted 6 lines

arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c line 31
Quoted 4 lines
arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c +1 -0
+#include <linux/dynamic_debug_cpufreq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
kernel.h includes dynamic_debug.h right?

yes

Then why not just put the

Quoted 2 lines

_cpufreq.h stuff in the current cpufreq.h file(s)?

1) cpufreq.h is included by other subsystems, and thus those other subsystems potentially have the wrong debugging information.

2) For the case where CONFIG_CPU_FREQ_DEBUG is set and CONFIG_DYNAMIC_PRINTK_DEBUG is not set i need to be able get the proper definition of 'dynamic_dbg_enabled()' out of dynamic_print.h (included by kernel.h). This proper definition is obtained by setting 'DEBUG', and thus i need to define this before the kernel.h include.

We can solve #2 by defining an explicit definition for dynamic_dbg_enabled() when only CONFIG_CPU_FREQ_DEBUG within cpufreq.h. roughly:

#ifdef CONFIG_CPU_FREQ_DEBUG
#define cpufreq_debug_printk(flag)
if (cpufreq_debug & flag)
__cpufreq_debug_printk()

However, the way i have it now I don't need two definitions for cpufreq_debug_printk....

If we could pull the subsystem specific header in dynamic_printk.h for each subsytem that would do the trick and be really clean, but i'm not sure of an easy way to do that...

-Jason

Dominik Brodowski

2008-07-17 22:20 UTC #6 lore raw

Quoted 6 lines · Jason Baron

Then why not just put the
Quoted 2 lines
_cpufreq.h stuff in the current cpufreq.h file(s)?
1) cpufreq.h is included by other subsystems, and thus those other subsystems potentially have the wrong debugging information.

...

Quoted 3 lines

If we could pull the subsystem specific header in dynamic_printk.h for each subsytem that would do the trick and be really clean, but i'm not sure of an easy way to do that...

<within all cpufreq drivers>
#define DYNAMIC_DEBUG_CPU_FREQ 1

<dynamic_printk.h>
#ifdef DYNAMIC_DEBUG_CPU_FREQ
#define DYNAMIC_DEBUG_NUM_FLAGS "3"
...
#else
...
...
#endif

or -- even better -- could we determine the definition of DYNAMIC_DEBUG_CPU_FREQ on the fly? It needs to be set to true for exactly all "modules" which contain a call to cpufreq_debug_printk()...

Quoted 9 lines

2) For the case where CONFIG_CPU_FREQ_DEBUG is set and CONFIG_DYNAMIC_PRINTK_DEBUG is not set i need to be able get the proper definition of 'dynamic_dbg_enabled()' out of dynamic_print.h (included by kernel.h). This proper definition is obtained by setting 'DEBUG', and thus i need to define this before the kernel.h include.
We can solve #2 by defining an explicit definition for dynamic_dbg_enabled() when only CONFIG_CPU_FREQ_DEBUG within cpufreq.h. roughly:

Quoted 7 lines

#ifdef CONFIG_CPU_FREQ_DEBUG
#define cpufreq_debug_printk(flag)
if (cpufreq_debug & flag)
__cpufreq_debug_printk()
However, the way i have it now I don't need two definitions for cpufreq_debug_printk....

Not that much of a nuiseance, IMO. Or SELECT CONFIG_DYNAMIC_PRINTK_DEBUG when CONFIG_CPU_FREQ_DEBUG is enabled (if davej agrees ;) )

Best,
Dominik