diff mbox series

[1/1] acpi_driver: add CONFIG_ACPI_KERN_DEBUG config to enable KERN_DEBUG

Message ID 20241115025351.85283-1-xiongyining1480@phytium.com.cn
State New
Headers show
Series [1/1] acpi_driver: add CONFIG_ACPI_KERN_DEBUG config to enable KERN_DEBUG | expand

Commit Message

Xiong Yining Nov. 15, 2024, 2:53 a.m. UTC
In the API module, there is no unified configuration switch to control debug 
output, and the current approach is to enable debugging by adding "define DEBUG"
in the file, which is both cumbersome and difficult to manage. a global debug config 
to control the debug output of the ACPI module will be more easily and clearly.

Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>

---
 drivers/acpi/Kconfig  | 6 ++++++
 drivers/acpi/Makefile | 1 +
 2 files changed, 7 insertions(+)

Comments

Rafael J. Wysocki Dec. 10, 2024, 5:53 p.m. UTC | #1
On Fri, Nov 15, 2024 at 3:54 AM Xiong Yining
<xiongyining1480@phytium.com.cn> wrote:
>
> In the API module, there is no unified configuration switch to control debug

You mean ACPI I suppose?

> output, and the current approach is to enable debugging by adding "define DEBUG"
> in the file, which is both cumbersome and difficult to manage. a global debug config
> to control the debug output of the ACPI module will be more easily and clearly.

So there is only one KERN_DEBUG printk() statement in the entire
drivers/acpi/ directory, the rest is pr_debug() or dev_dbg() that
shouldn't need this change.

> Signed-off-by: Xiong Yining <xiongyining1480@phytium.com.cn>
>
> ---
>  drivers/acpi/Kconfig  | 6 ++++++
>  drivers/acpi/Makefile | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index cee82b473dc5..ea198ead57d7 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -479,6 +479,12 @@ config ACPI_REDUCED_HARDWARE_ONLY
>
>           If you are unsure what to do, do not enable this option.
>
> +config ACPI_KERN_DEBUG
> +       bool "Acpi kernel debugging"

Please always spell ACPI in capitals.

> +       help
> +         This is an option for use by developers, most people should
> +         say N here. This enables ACPI driver KERN_DEBUG.
> +
>  source "drivers/acpi/nfit/Kconfig"
>  source "drivers/acpi/numa/Kconfig"
>  source "drivers/acpi/apei/Kconfig"
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index eaa09bf52f17..d0a417e73071 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -4,6 +4,7 @@
>  #
>
>  ccflags-$(CONFIG_ACPI_DEBUG)   += -DACPI_DEBUG_OUTPUT
> +ccflags-$(CONFIG_ACPI_KERN_DEBUG)      := -DDEBUG

Isn't this kind of confusing?

>
>  #
>  # ACPI Boot-Time Table Parsing
> --
Xiong Yining Dec. 12, 2024, 11:54 a.m. UTC | #2
> > In the API module, there is no unified configuration switch to control de=
	bug

> You mean ACPI I suppose?

Oh~, Sorry for the typo.

> > output, and the current approach is to enable debugging by adding "define=
	DEBUG"
> > in the file, which is both cumbersome and difficult to manage. a global d=
	ebug config
> > to control the debug output of the ACPI module will be more easily and cl=
	early.

> So there is only one KERN_DEBUG printk() statement in the entire
  drivers/acpi/ directory, the rest is pr_debug() or dev_dbg() that
  shouldn't need this change.

The original intention of this patch is to manage the printing function 
of all KERN_DEBUG levels in drivers/acpi, including acpi_handle_debug(), 
pr_debug(), and dev_debg().

Why don't pr_debug() need this change?

> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index cee82b473dc5..ea198ead57d7 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -479,6 +479,12 @@ config ACPI_REDUCED_HARDWARE_ONLY
> >
> >           If you are unsure what to do, do not enable this option.
> >
> > +config ACPI_KERN_DEBUG
> > +       bool "Acpi kernel debugging"

> Please always spell ACPI in capitals.

Got it, thanks.

> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> > index eaa09bf52f17..d0a417e73071 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> >  ccflags-$(CONFIG_ACPI_DEBUG)   += -DACPI_DEBUG_OUTPUT
> > +ccflags-$(CONFIG_ACPI_KERN_DEBUG)      := -DDEBUG

> Isn't this kind of confusing?

Indeed, i will change. Thanks.
Rafael J. Wysocki Dec. 12, 2024, 12:07 p.m. UTC | #3
On Thu, Dec 12, 2024 at 12:55 PM Xiong Yining
<xiongyining1480@phytium.com.cn> wrote:
>
> > > In the API module, there is no unified configuration switch to control de=
>         bug
>
> > You mean ACPI I suppose?
>
> Oh~, Sorry for the typo.
>
> > > output, and the current approach is to enable debugging by adding "define=
>         DEBUG"
> > > in the file, which is both cumbersome and difficult to manage. a global d=
>         ebug config
> > > to control the debug output of the ACPI module will be more easily and cl=
>         early.
>
> > So there is only one KERN_DEBUG printk() statement in the entire
>   drivers/acpi/ directory, the rest is pr_debug() or dev_dbg() that
>   shouldn't need this change.
>
> The original intention of this patch is to manage the printing function
> of all KERN_DEBUG levels in drivers/acpi, including acpi_handle_debug(),
> pr_debug(), and dev_debg().
>
> Why don't pr_debug() need this change?

Because they can be enabled through dynamic debug which IMV is
superior to a compile-time switch.
diff mbox series

Patch

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index cee82b473dc5..ea198ead57d7 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -479,6 +479,12 @@  config ACPI_REDUCED_HARDWARE_ONLY
 
 	  If you are unsure what to do, do not enable this option.
 
+config ACPI_KERN_DEBUG
+	bool "Acpi kernel debugging"
+	help
+	  This is an option for use by developers, most people should
+	  say N here. This enables ACPI driver KERN_DEBUG.
+
 source "drivers/acpi/nfit/Kconfig"
 source "drivers/acpi/numa/Kconfig"
 source "drivers/acpi/apei/Kconfig"
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index eaa09bf52f17..d0a417e73071 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -4,6 +4,7 @@ 
 #
 
 ccflags-$(CONFIG_ACPI_DEBUG)	+= -DACPI_DEBUG_OUTPUT
+ccflags-$(CONFIG_ACPI_KERN_DEBUG)	:= -DDEBUG
 
 #
 # ACPI Boot-Time Table Parsing