diff mbox

[v2,1/4] ACPI: Add acpi_pr_<level>() interfaces

Message ID 1342803256-17514-2-git-send-email-toshi.kani@hp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Toshi Kani July 20, 2012, 4:54 p.m. UTC
This patch introduces acpi_pr_<level>(), where <level> is a message
level such as err/warn/info, to support improved logging messages
for ACPI, esp. in hotplug operations.  acpi_pr_<level>() appends
"ACPI" prefix and ACPI object path to the messages.  This improves
diagnostics in hotplug operations since it identifies an object that
caused an issue in a log file.

acpi_pr_<level>() takes acpi_handle as an argument, which is passed
to ACPI hotplug notify handlers from the ACPI CA.  Therefore, it is
always available unlike other kernel objects, such as device.

For example, the statement below
  acpi_pr_err(handle, "Device don't exist, dropping EJECT\n");
logs an error message like this at KERN_ERR.
  ACPI: \_SB_.SCK4.CPU4: Device don't exist, dropping EJECT

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/utils.c    |   34 ++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |   20 ++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

Comments

Joe Perches July 25, 2012, 7:06 a.m. UTC | #1
On Fri, 2012-07-20 at 10:54 -0600, Toshi Kani wrote:
> This patch introduces acpi_pr_<level>(), where <level> is a message
> level such as err/warn/info, to support improved logging messages
> for ACPI, esp. in hotplug operations.  acpi_pr_<level>() appends
> "ACPI" prefix and ACPI object path to the messages.  This improves
> diagnostics in hotplug operations since it identifies an object that
> caused an issue in a log file.

trivia:

> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
[]
> @@ -454,3 +454,37 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
[]
> +	char *path;

const ?

> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
[]
> @@ -85,6 +85,26 @@ struct acpi_pld {
[]
> +#define acpi_pr_debug(handle, fmt, ...)				\
> +	acpi_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__)

Might be nicer if this somehow had a dynamic debug
mechanism too.

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Toshi Kani July 25, 2012, 5:53 p.m. UTC | #2
On Wed, 2012-07-25 at 00:06 -0700, Joe Perches wrote:
> On Fri, 2012-07-20 at 10:54 -0600, Toshi Kani wrote:
> > This patch introduces acpi_pr_<level>(), where <level> is a message
> > level such as err/warn/info, to support improved logging messages
> > for ACPI, esp. in hotplug operations.  acpi_pr_<level>() appends
> > "ACPI" prefix and ACPI object path to the messages.  This improves
> > diagnostics in hotplug operations since it identifies an object that
> > caused an issue in a log file.

Hi Joe,

Thanks for reviewing!

> trivia:

Did you mean to say something more on this?  Just checking...

> > diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> []
> > @@ -454,3 +454,37 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
> []
> > +	char *path;
> 
> const ?

Agreed.  I will update with the change.

> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> []
> > @@ -85,6 +85,26 @@ struct acpi_pld {
> []
> > +#define acpi_pr_debug(handle, fmt, ...)				\
> > +	acpi_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__)
> 
> Might be nicer if this somehow had a dynamic debug
> mechanism too.

Good point!  I will try to add a dynamic debug mechanism on this.

Thanks,
-Toshi


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches July 25, 2012, 6:11 p.m. UTC | #3
On Wed, 2012-07-25 at 11:53 -0600, Toshi Kani wrote:
> On Wed, 2012-07-25 at 00:06 -0700, Joe Perches wrote:
> > On Fri, 2012-07-20 at 10:54 -0600, Toshi Kani wrote:
> > > This patch introduces acpi_pr_<level>(), where <level> is a message
> > > level such as err/warn/info, to support improved logging messages
> > > for ACPI, esp. in hotplug operations.  acpi_pr_<level>() appends
> > > "ACPI" prefix and ACPI object path to the messages.  This improves
> > > diagnostics in hotplug operations since it identifies an object that
> > > caused an issue in a log file.
> 
> Hi Joe,
> 
> Thanks for reviewing!
> 
> > trivia:
> 
> Did you mean to say something more on this?  Just checking...

No.  I just intended to note that the comments that followed
weren't particularly important nor should it really stop the
patch from being applied if you didn't want to update it.

cheers, Joe

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Toshi Kani July 25, 2012, 6:30 p.m. UTC | #4
On Wed, 2012-07-25 at 11:11 -0700, Joe Perches wrote:
> On Wed, 2012-07-25 at 11:53 -0600, Toshi Kani wrote:
> > On Wed, 2012-07-25 at 00:06 -0700, Joe Perches wrote:
> > > On Fri, 2012-07-20 at 10:54 -0600, Toshi Kani wrote:
> > > > This patch introduces acpi_pr_<level>(), where <level> is a message
> > > > level such as err/warn/info, to support improved logging messages
> > > > for ACPI, esp. in hotplug operations.  acpi_pr_<level>() appends
> > > > "ACPI" prefix and ACPI object path to the messages.  This improves
> > > > diagnostics in hotplug operations since it identifies an object that
> > > > caused an issue in a log file.
> > 
> > Hi Joe,
> > 
> > Thanks for reviewing!
> > 
> > > trivia:
> > 
> > Did you mean to say something more on this?  Just checking...
> 
> No.  I just intended to note that the comments that followed
> weren't particularly important nor should it really stop the
> patch from being applied if you didn't want to update it.
> 
> cheers, Joe

Thanks Joe for the clarification and suggestion.  If the dynamic debug
change becomes something non-trivial, I will postpone it.

-Toshi


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 3e87c9c..d4311de 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -454,3 +454,37 @@  acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
 #endif
 }
 EXPORT_SYMBOL(acpi_evaluate_hotplug_ost);
+
+/**
+ * acpi_printk: Print messages with ACPI prefix and object path
+ *
+ * This function is intended to be called through acpi_pr_<level> macros.
+ */
+void
+acpi_printk(const char *level, acpi_handle handle, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+	struct acpi_buffer buffer = {
+		.length = ACPI_ALLOCATE_BUFFER,
+		.pointer = NULL
+	};
+	char *path;
+	acpi_status ret;
+
+	va_start(args, fmt);
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	ret = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+	if (ret == AE_OK)
+		path = buffer.pointer;
+	else
+		path = "<n/a>";
+
+	printk("%sACPI: %s: %pV", level, path, &vaf);
+
+	va_end(args);
+	kfree(buffer.pointer);
+}
+EXPORT_SYMBOL(acpi_printk);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index b22b774..2ceeca3 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -85,6 +85,26 @@  struct acpi_pld {
 
 acpi_status
 acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld *pld);
+
+void acpi_printk(const char *level, acpi_handle handle, const char *fmt, ...);
+
+#define acpi_pr_emerg(handle, fmt, ...)				\
+	acpi_printk(KERN_EMERG, handle, fmt, ##__VA_ARGS__)
+#define acpi_pr_alert(handle, fmt, ...)				\
+	acpi_printk(KERN_ALERT, handle, fmt, ##__VA_ARGS__)
+#define acpi_pr_crit(handle, fmt, ...)				\
+	acpi_printk(KERN_CRIT, handle, fmt, ##__VA_ARGS__)
+#define acpi_pr_err(handle, fmt, ...)				\
+	acpi_printk(KERN_ERR, handle, fmt, ##__VA_ARGS__)
+#define acpi_pr_warn(handle, fmt, ...)				\
+	acpi_printk(KERN_WARNING, handle, fmt, ##__VA_ARGS__)
+#define acpi_pr_notice(handle, fmt, ...)			\
+	acpi_printk(KERN_NOTICE, handle, fmt, ##__VA_ARGS__)
+#define acpi_pr_info(handle, fmt, ...)				\
+	acpi_printk(KERN_INFO, handle, fmt, ##__VA_ARGS__)
+#define acpi_pr_debug(handle, fmt, ...)				\
+	acpi_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__)
+
 #ifdef CONFIG_ACPI
 
 #include <linux/proc_fs.h>