Re: [RFC v1] opp: add config option for debug

[Greg Kroah-Hartman]

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@xxxxxxxxxxxxxxxxxxx>:
> >On Wed, May 04, 2022 at 07:48:23PM +0200, Frank Wunderlich wrote:
> >> From: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
> >> 
> >> 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@xxxxxxxxxxxxxxx>
> >> ---
> >>  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