| Message ID | 20220504174823.156709-1-linux@fw-web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | [RFC,v1] opp: add config option for debug | expand |
On Wed, May 04, 2022 at 07:48:23PM +0200, Frank Wunderlich wrote: > From: Frank Wunderlich <frank-w@public-files.de> > > 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 <frank-w@public-files.de> > --- > 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 04-05-22, 19:48, Frank Wunderlich wrote: > From: Frank Wunderlich <frank-w@public-files.de> > > 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 <frank-w@public-files.de> > --- > 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 ?
Hi, > Gesendet: Donnerstag, 05. Mai 2022 um 07:58 Uhr > Von: "Viresh Kumar" <viresh.kumar@linaro.org> > An: "Frank Wunderlich" <linux@fw-web.de> > Cc: linux-pm@vger.kernel.org, "Frank Wunderlich" <frank-w@public-files.de>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Rafael J. Wysocki" <rafael@kernel.org>, "Viresh Kumar" <vireshk@kernel.org>, "Nishanth Menon" <nm@ti.com>, "Stephen Boyd" <sboyd@kernel.org>, linux-kernel@vger.kernel.org > Betreff: Re: [RFC v1] opp: add config option for debug > > On 04-05-22, 19:48, Frank Wunderlich wrote: > > From: Frank Wunderlich <frank-w@public-files.de> > > > > 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 <frank-w@public-files.de> > > --- > > 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" <viresh.kumar@linaro.org> > > An: "Frank Wunderlich" <linux@fw-web.de> > > Cc: linux-pm@vger.kernel.org, "Frank Wunderlich" <frank-w@public-files.de>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Rafael J. Wysocki" <rafael@kernel.org>, "Viresh Kumar" <vireshk@kernel.org>, "Nishanth Menon" <nm@ti.com>, "Stephen Boyd" <sboyd@kernel.org>, linux-kernel@vger.kernel.org > > Betreff: Re: [RFC v1] opp: add config option for debug > > > > On 04-05-22, 19:48, Frank Wunderlich wrote: > > > From: Frank Wunderlich <frank-w@public-files.de> > > > > > > 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 <frank-w@public-files.de> > > > --- > > > 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
Hi, Am 4. Mai 2022 20:24:00 MESZ schrieb Greg Kroah-Hartman <gregkh@linuxfoundation.org>: >On Wed, May 04, 2022 at 07:48:23PM +0200, Frank Wunderlich wrote: >> From: Frank Wunderlich <frank-w@public-files.de> >> >> 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 <frank-w@public-files.de> >> --- >> 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 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 <gregkh@linuxfoundation.org>: > >On Wed, May 04, 2022 at 07:48:23PM +0200, Frank Wunderlich wrote: > >> From: Frank Wunderlich <frank-w@public-files.de> > >> > >> 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 <frank-w@public-files.de> > >> --- > >> 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
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.
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