Skip to contentSkip to conversation

opp: add config option for debug

RFC In review Frank Wunderlich 4 participants 8 messages May 4, 2022 – May 6, 2022

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

RFC 1/1 opp: add config option for debug

From: Frank Wunderlich <[email protected]>

Currently OPP debug is enabled by DEBUG_DRIVER option. This is generic driver debug and opp floods serial console. This is annoying if opp is not needed so give it an additional config-key.

---
 drivers/base/Kconfig | 1 +
 drivers/opp/Kconfig  | 7 +++++++
 drivers/opp/Makefile | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)
drivers/base/Kconfig +1 -0
index 6f04b831a5c0..8ae826c95d5f 100644
@@ -130,6 +130,7 @@ config DEV_COREDUMP
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"
 	depends on DEBUG_KERNEL
+	imply DEBUG_OPP
 	help
 	  Say Y here if you want the Driver core to produce a bunch of
 	  debug messages to the system log. Select this if you are having a
drivers/opp/Kconfig +7 -0
index e8ce47b32735..6a2d2c6c1143 100644
@@ -12,3 +12,10 @@ config PM_OPP
 	  representing individual voltage domains and provides SOC
 	  implementations a ready to use framework to manage OPPs.
 	  For more information, read <file:Documentation/power/opp.rst>
+
+menu "Operating Performance Points (OPP)"
+config DEBUG_OPP
+	bool "Debug Operating Performance Points"
+	help
+	  enable opp debugging
+endmenu
drivers/opp/Makefile +1 -1
index f65ed5985bb4..2589915eef95 100644
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
-ccflags-$(CONFIG_DEBUG_DRIVER)	:= -DDEBUG
+ccflags-$(CONFIG_DEBUG_OPP)	:= -DDEBUG
 obj-y				+= core.o cpu.o
 obj-$(CONFIG_OF)		+= of.o
 obj-$(CONFIG_DEBUG_FS)		+= debugfs.o
Signature · 1 line
2.25.1
#1 lore raw
Quoted 6 lines · Frank Wunderlich

From: Frank Wunderlich <[email protected]>

Currently OPP debug is enabled by DEBUG_DRIVER option. This is generic driver debug and opp floods serial console. This is annoying if opp is not needed so give it an additional config-key.

Quoted 6 lines
---
 drivers/base/Kconfig | 1 +
 drivers/opp/Kconfig  | 7 +++++++
 drivers/opp/Makefile | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

Isn't something like Dynamic Debug helpful here ?

Signature · 1 line
viresh
#2 lore raw

Hi,

Quoted 13 lines

Gesendet: Donnerstag, 05. Mai 2022 um 07:58 Uhr
Von: "Viresh Kumar" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: [email protected], "Frank Wunderlich" <[email protected]>, "Greg Kroah-Hartman" <[email protected]>, "Rafael J. Wysocki" <[email protected]>, "Viresh Kumar" <[email protected]>, "Nishanth Menon" <[email protected]>, "Stephen Boyd" <[email protected]>, [email protected]
Betreff: Re: [RFC v1] opp: add config option for debug

Quoted 6 lines · Frank Wunderlich

From: Frank Wunderlich <[email protected]>

Currently OPP debug is enabled by DEBUG_DRIVER option. This is generic driver debug and opp floods serial console. This is annoying if opp is not needed so give it an additional config-key.

Quoted 8 lines
Quoted 6 lines
---
 drivers/base/Kconfig | 1 +
 drivers/opp/Kconfig  | 7 +++++++
 drivers/opp/Makefile | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

Isn't something like Dynamic Debug helpful here ?

you mean something like this:

https://www.kernel.org/doc/html/v5.17/admin-guide/dynamic-debug-howto.html#debug-messages-during-boot-process

so enabling debug only with cmdline-param...

have you a simple example how to implement it? have not done anything with dynamic-debug yet...seems mighty but not trivial to implement.

currently dev_dbg() is used for the messages that i try to disable...but show others from driver_debug at debug level.

What needs to be changed to filter it via DYNAMIC_DEBUG?

found this, but i'm not sure if i interpret it the right way...

https://elixir.bootlin.com/linux/v5.18-rc5/source/drivers/acpi/utils.c#L495
defines __acpi_handle_debug
called via acpi_handle_debug macro
https://elixir.bootlin.com/linux/v5.18-rc5/source/include/linux/acpi.h#L1136

so basicly convert dev_dbg to __dynamic_pr_debug

at least much more changed code because all dev_*/pr_* needs to be changed to own handler which does the switch based on CONFIG_DYNAMIC_DEBUG set or not.

regards Frank

#3 lore raw
Quoted 36 lines · Frank Wunderlich

Hi,

Quoted 13 lines

Gesendet: Donnerstag, 05. Mai 2022 um 07:58 Uhr
Von: "Viresh Kumar" <[email protected]>
An: "Frank Wunderlich" <[email protected]>
Cc: [email protected], "Frank Wunderlich" <[email protected]>, "Greg Kroah-Hartman" <[email protected]>, "Rafael J. Wysocki" <[email protected]>, "Viresh Kumar" <[email protected]>, "Nishanth Menon" <[email protected]>, "Stephen Boyd" <[email protected]>, [email protected]
Betreff: Re: [RFC v1] opp: add config option for debug

Quoted 6 lines · Frank Wunderlich

From: Frank Wunderlich <[email protected]>

Currently OPP debug is enabled by DEBUG_DRIVER option. This is generic driver debug and opp floods serial console. This is annoying if opp is not needed so give it an additional config-key.

Quoted 8 lines
Quoted 6 lines
---
 drivers/base/Kconfig | 1 +
 drivers/opp/Kconfig  | 7 +++++++
 drivers/opp/Makefile | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

Isn't something like Dynamic Debug helpful here ?

you mean something like this:

https://www.kernel.org/doc/html/v5.17/admin-guide/dynamic-debug-howto.html#debug-messages-during-boot-process

so enabling debug only with cmdline-param...

have you a simple example how to implement it? have not done anything with dynamic-debug yet...seems mighty but not trivial to implement.

currently dev_dbg() is used for the messages that i try to disable...but show others from driver_debug at debug level.

What needs to be changed to filter it via DYNAMIC_DEBUG?

Quoted 8 lines

found this, but i'm not sure if i interpret it the right way...

https://elixir.bootlin.com/linux/v5.18-rc5/source/drivers/acpi/utils.c#L495
defines __acpi_handle_debug
called via acpi_handle_debug macro
https://elixir.bootlin.com/linux/v5.18-rc5/source/include/linux/acpi.h#L1136

so basicly convert dev_dbg to __dynamic_pr_debug

Ick, no, just stick with all dev_dbg() calls and do not add any Makefile changes and all should be fine. And drop the Kconfig option, should not be needed for a subsystem/driver at all.

thanks,

greg k-h

#4 lore raw
In reply to Frank Wunderlich
Quoted 3 lines · Frank Wunderlich

you mean something like this:

https://www.kernel.org/doc/html/v5.17/admin-guide/dynamic-debug-howto.html#debug-messages-during-boot-process

Yes, though I haven't used it in a long time myself :)

Quoted 1 line

so enabling debug only with cmdline-param...

Yes and via debugfs file. You can basically control debug messages based on subsystems, files, functions, etc.

Quoted 5 lines

have you a simple example how to implement it? have not done anything with dynamic-debug yet...seems mighty but not trivial to implement.

currently dev_dbg() is used for the messages that i try to disable...but show others from driver_debug at debug level.

What needs to be changed to filter it via DYNAMIC_DEBUG?

Nothing, just enable the config for dynamic debug.

Quoted 10 lines

found this, but i'm not sure if i interpret it the right way...

https://elixir.bootlin.com/linux/v5.18-rc5/source/drivers/acpi/utils.c#L495
defines __acpi_handle_debug
called via acpi_handle_debug macro
https://elixir.bootlin.com/linux/v5.18-rc5/source/include/linux/acpi.h#L1136

so basicly convert dev_dbg to __dynamic_pr_debug

at least much more changed code because all dev_*/pr_* needs to be changed to own handler which does the switch based on CONFIG_DYNAMIC_DEBUG set or not.

You aren't required to change anything there.

Signature · 1 line
viresh
#5 lore raw
In reply to Frank Wunderlich
Quoted 18 lines · Frank Wunderlich

From: Frank Wunderlich <[email protected]>

Currently OPP debug is enabled by DEBUG_DRIVER option. This is generic driver debug and opp floods serial console. This is annoying if opp is not needed so give it an additional config-key.

---
 drivers/base/Kconfig | 1 +
 drivers/opp/Kconfig  | 7 +++++++
 drivers/opp/Makefile | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)
drivers/base/Kconfig +0 -0
index 6f04b831a5c0..8ae826c95d5f 100644

drivers/base/Kconfig config DEV_COREDUMP · line 133

Quoted 4 lines
drivers/base/Kconfig +1 -0
@@ -130,6 +130,7 @@ config DEV_COREDUMP
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"
 	depends on DEBUG_KERNEL
+	imply DEBUG_OPP

This should not be needed, otherwise we would have to do that for all random driver subsystem in the kernel.

Quoted 23 lines
drivers/base/Kconfig +0 -0
 	help
 	  Say Y here if you want the Driver core to produce a bunch of
 	  debug messages to the system log. Select this if you are having a
drivers/opp/Kconfig +7 -0
index e8ce47b32735..6a2d2c6c1143 100644
@@ -12,3 +12,10 @@ config PM_OPP
 	  representing individual voltage domains and provides SOC
 	  implementations a ready to use framework to manage OPPs.
 	  For more information, read <file:Documentation/power/opp.rst>
+
+menu "Operating Performance Points (OPP)"
+config DEBUG_OPP
+	bool "Debug Operating Performance Points"
+	help
+	  enable opp debugging
+endmenu
drivers/opp/Makefile +0 -0
index f65ed5985bb4..2589915eef95 100644

drivers/opp/Makefile line 2

Quoted 3 lines
drivers/opp/Makefile +1 -1
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
-ccflags-$(CONFIG_DEBUG_DRIVER)	:= -DDEBUG
+ccflags-$(CONFIG_DEBUG_OPP)	:= -DDEBUG

This feels wrong, you shouldn't need a -DDEBUG for anything if all is going correctly. Why is opp so odd this way? Just use the normal dev_dbg() macros and all will be fine, nothing special should be needed at all.

And don't use a config option for it either, no one will turn it on, it needs to "just work" for all systems.

thanks,

greg k-h

#6 lore raw

Hi,

Am 4. Mai 2022 20:24:00 MESZ schrieb Greg Kroah-Hartman <[email protected]>:

Quoted 16 lines · Frank Wunderlich
Quoted 3 lines

From: Frank Wunderlich <[email protected]>

Currently OPP debug is enabled by DEBUG_DRIVER option. This is

generic

Quoted 1 line

driver debug and opp floods serial console. This is annoying if opp

is

Quoted 9 lines

not needed so give it an additional config-key.

---
 drivers/base/Kconfig | 1 +
 drivers/opp/Kconfig  | 7 +++++++
 drivers/opp/Makefile | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)
Quoted 12 lines

drivers/base/Kconfig config DEV_COREDUMP · line 133

Quoted 9 lines
drivers/base/Kconfig +1 -0
index 6f04b831a5c0..8ae826c95d5f 100644
@@ -130,6 +130,7 @@ config DEV_COREDUMP
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"
 	depends on DEBUG_KERNEL
+	imply DEBUG_OPP

This should not be needed, otherwise we would have to do that for all random driver subsystem in the kernel.

Have added this to have same behaviour if anyone sets DEBUG_DRIVER via defconfig. Else this is disabled by default.

Quoted 23 lines
Quoted 23 lines
drivers/base/Kconfig +0 -0
 	help
 	  Say Y here if you want the Driver core to produce a bunch of
 	  debug messages to the system log. Select this if you are having a
drivers/opp/Kconfig +7 -0
index e8ce47b32735..6a2d2c6c1143 100644
@@ -12,3 +12,10 @@ config PM_OPP
 	  representing individual voltage domains and provides SOC
 	  implementations a ready to use framework to manage OPPs.
 	  For more information, read <file:Documentation/power/opp.rst>
+
+menu "Operating Performance Points (OPP)"
+config DEBUG_OPP
+	bool "Debug Operating Performance Points"
+	help
+	  enable opp debugging
+endmenu
drivers/opp/Makefile +0 -0
index f65ed5985bb4..2589915eef95 100644
@@ -1,5 +1,5 @@
Quoted 8 lines

drivers/opp/Makefile line 2

Quoted 3 lines
drivers/opp/Makefile +1 -1
 # SPDX-License-Identifier: GPL-2.0-only
-ccflags-$(CONFIG_DEBUG_DRIVER)	:= -DDEBUG
+ccflags-$(CONFIG_DEBUG_OPP)	:= -DDEBUG

This feels wrong, you shouldn't need a -DDEBUG for anything if all is going correctly. Why is opp so odd this way? Just use the normal dev_dbg() macros and all will be fine, nothing special should be needed at all.

I have looked more into it,just wanted to get driver debug (probing/binding) and dev_dbg messages without the opp spam (floods serial console).

Quoted 2 lines

And don't use a config option for it either, no one will turn it on, it needs to "just work" for all systems.

Config option is to enable if needed and not via driver-debug.

Quoted 3 lines

thanks,

greg k-h

regards Frank

#7 lore raw
Quoted 66 lines · Frank Wunderlich

Hi,

Am 4. Mai 2022 20:24:00 MESZ schrieb Greg Kroah-Hartman <[email protected]>:

Quoted 16 lines · Frank Wunderlich
Quoted 3 lines

From: Frank Wunderlich <[email protected]>

Currently OPP debug is enabled by DEBUG_DRIVER option. This is

generic

Quoted 1 line

driver debug and opp floods serial console. This is annoying if opp

is

Quoted 9 lines

not needed so give it an additional config-key.

---
 drivers/base/Kconfig | 1 +
 drivers/opp/Kconfig  | 7 +++++++
 drivers/opp/Makefile | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)
Quoted 12 lines

drivers/base/Kconfig config DEV_COREDUMP · line 133

Quoted 9 lines
drivers/base/Kconfig +1 -0
index 6f04b831a5c0..8ae826c95d5f 100644
@@ -130,6 +130,7 @@ config DEV_COREDUMP
 config DEBUG_DRIVER
 	bool "Driver Core verbose debug messages"
 	depends on DEBUG_KERNEL
+	imply DEBUG_OPP

This should not be needed, otherwise we would have to do that for all random driver subsystem in the kernel.

Have added this to have same behaviour if anyone sets DEBUG_DRIVER via defconfig. Else this is disabled by default.

Quoted 23 lines
Quoted 23 lines
drivers/base/Kconfig +0 -0
 	help
 	  Say Y here if you want the Driver core to produce a bunch of
 	  debug messages to the system log. Select this if you are having a
drivers/opp/Kconfig +7 -0
index e8ce47b32735..6a2d2c6c1143 100644
@@ -12,3 +12,10 @@ config PM_OPP
 	  representing individual voltage domains and provides SOC
 	  implementations a ready to use framework to manage OPPs.
 	  For more information, read <file:Documentation/power/opp.rst>
+
+menu "Operating Performance Points (OPP)"
+config DEBUG_OPP
+	bool "Debug Operating Performance Points"
+	help
+	  enable opp debugging
+endmenu
drivers/opp/Makefile +0 -0
index f65ed5985bb4..2589915eef95 100644
@@ -1,5 +1,5 @@
Quoted 8 lines

drivers/opp/Makefile line 2

Quoted 3 lines
drivers/opp/Makefile +1 -1
 # SPDX-License-Identifier: GPL-2.0-only
-ccflags-$(CONFIG_DEBUG_DRIVER)	:= -DDEBUG
+ccflags-$(CONFIG_DEBUG_OPP)	:= -DDEBUG

This feels wrong, you shouldn't need a -DDEBUG for anything if all is going correctly. Why is opp so odd this way? Just use the normal dev_dbg() macros and all will be fine, nothing special should be needed at all.

Quoted 6 lines

I have looked more into it,just wanted to get driver debug (probing/binding) and dev_dbg messages without the opp spam (floods serial console).

Quoted 2 lines

And don't use a config option for it either, no one will turn it on, it needs to "just work" for all systems.

Config option is to enable if needed and not via driver-debug.

Please do not do that, you should never need subsystem/driver Kconfig options like this. Distros will never enable them and you can't ask a user to rebuild their kernel easily. Just rely on the same infrastructure like all other subsystems do please.

thanks,

greg k-h