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.
Signed-off-by: Frank Wunderlich <[email protected]>
---
drivers/base/Kconfig | 1 +
drivers/opp/Kconfig | 7 +++++++
drivers/opp/Makefile | 2 +-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 6f04b831a5c0..8ae826c95d5f 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -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
diff --git a/drivers/opp/Kconfig b/drivers/opp/Kconfig
index e8ce47b32735..6a2d2c6c1143 100644
--- a/drivers/opp/Kconfig
+++ b/drivers/opp/Kconfig
@@ -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
diff --git a/drivers/opp/Makefile b/drivers/opp/Makefile
index f65ed5985bb4..2589915eef95 100644
--- a/drivers/opp/Makefile
+++ b/drivers/opp/Makefile
@@ -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
--
2.25.1
On 04-05-22, 19:48, Frank Wunderlich wrote:
> 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.
>
> Signed-off-by: Frank Wunderlich <[email protected]>
> ---
> 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 ?
--
viresh
Hi,
> 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
>
> On 04-05-22, 19:48, Frank Wunderlich wrote:
> > 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.
> >
> > Signed-off-by: Frank Wunderlich <[email protected]>
> > ---
> > 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
On Thu, May 05, 2022 at 05:54:18PM +0200, Frank Wunderlich wrote:
> Hi,
>
> > 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
> >
> > On 04-05-22, 19:48, Frank Wunderlich wrote:
> > > 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.
> > >
> > > Signed-off-by: Frank Wunderlich <[email protected]>
> > > ---
> > > 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
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
On Wed, May 04, 2022 at 07:48:23PM +0200, Frank Wunderlich wrote:
> 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.
>
> Signed-off-by: Frank Wunderlich <[email protected]>
> ---
> drivers/base/Kconfig | 1 +
> drivers/opp/Kconfig | 7 +++++++
> drivers/opp/Makefile | 2 +-
> 3 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 6f04b831a5c0..8ae826c95d5f 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -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.
> 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
> diff --git a/drivers/opp/Kconfig b/drivers/opp/Kconfig
> index e8ce47b32735..6a2d2c6c1143 100644
> --- a/drivers/opp/Kconfig
> +++ b/drivers/opp/Kconfig
> @@ -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
> diff --git a/drivers/opp/Makefile b/drivers/opp/Makefile
> index f65ed5985bb4..2589915eef95 100644
> --- a/drivers/opp/Makefile
> +++ b/drivers/opp/Makefile
> @@ -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
On Thu, May 05, 2022 at 07:50:56PM +0200, Frank Wunderlich wrote:
> Hi,
>
> Am 4. Mai 2022 20:24:00 MESZ schrieb Greg Kroah-Hartman <[email protected]>:
> >On Wed, May 04, 2022 at 07:48:23PM +0200, Frank Wunderlich wrote:
> >> 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.
> >>
> >> Signed-off-by: Frank Wunderlich <[email protected]>
> >> ---
> >> drivers/base/Kconfig | 1 +
> >> drivers/opp/Kconfig | 7 +++++++
> >> drivers/opp/Makefile | 2 +-
> >> 3 files changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> >> index 6f04b831a5c0..8ae826c95d5f 100644
> >> --- a/drivers/base/Kconfig
> >> +++ b/drivers/base/Kconfig
> >> @@ -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.
>
> >> 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
> >> diff --git a/drivers/opp/Kconfig b/drivers/opp/Kconfig
> >> index e8ce47b32735..6a2d2c6c1143 100644
> >> --- a/drivers/opp/Kconfig
> >> +++ b/drivers/opp/Kconfig
> >> @@ -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
> >> diff --git a/drivers/opp/Makefile b/drivers/opp/Makefile
> >> index f65ed5985bb4..2589915eef95 100644
> >> --- a/drivers/opp/Makefile
> >> +++ b/drivers/opp/Makefile
> >> @@ -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.
>
> 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).
>
> >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
Hi,
Am 4. Mai 2022 20:24:00 MESZ schrieb Greg Kroah-Hartman <[email protected]>:
>On Wed, May 04, 2022 at 07:48:23PM +0200, Frank Wunderlich wrote:
>> 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.
>>
>> Signed-off-by: Frank Wunderlich <[email protected]>
>> ---
>> drivers/base/Kconfig | 1 +
>> drivers/opp/Kconfig | 7 +++++++
>> drivers/opp/Makefile | 2 +-
>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
>> index 6f04b831a5c0..8ae826c95d5f 100644
>> --- a/drivers/base/Kconfig
>> +++ b/drivers/base/Kconfig
>> @@ -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.
>> 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
>> diff --git a/drivers/opp/Kconfig b/drivers/opp/Kconfig
>> index e8ce47b32735..6a2d2c6c1143 100644
>> --- a/drivers/opp/Kconfig
>> +++ b/drivers/opp/Kconfig
>> @@ -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
>> diff --git a/drivers/opp/Makefile b/drivers/opp/Makefile
>> index f65ed5985bb4..2589915eef95 100644
>> --- a/drivers/opp/Makefile
>> +++ b/drivers/opp/Makefile
>> @@ -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.
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).
>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.
>thanks,
>
>greg k-h
regards Frank
On 05-05-22, 17:54, Frank Wunderlich wrote:
> 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 :)
> so enabling debug only with cmdline-param...
Yes and via debugfs file. You can basically control debug messages
based on subsystems, files, functions, etc.
> 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.
> 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.
--
viresh