[PATCH v5 RESEND 0/5] ACPI: hotplug messages improvement

[PATCH v5 RESEND 0/5] ACPI: hotplug messages improvement

[Toshi Kani]

This patchset improves logging messages for ACPI CPU, Memory, 
Container and Dock hotplug notify handlers.  The patchset 
introduces a set of new macro interfaces, acpi_pr_<level>(),
and uses them to update the notify handlers.  acpi_pr_<level>()
appends "ACPI" prefix and ACPI object path to the messages,
which has similar usage model as dev_<level>().  This improves
diagnosis of hotplug operations since an error message in 
a log file now identifies an object that caused an issue.

The patchset is based on the linux-next branch of linux-pm.git
tree.

v5:
 - Added update for ACPI Dock hotplug error messages.
 - Added error status / ID info to the messages where needed.

v4:
 - Changed to use dev_<level>() where it is appropriate.

v3:
 - Changed acpi_pr_debug() to NOP when !DEBUG and !DYNAMIC_DEBUG.
   DYNAMIC_DEBUG will be supported later when necessary.
 - Added const to a path variable in acpi_printk().
 - Added more descriptions to the change log of patch 1/4.

v2:
 - Set buffer.pointer to NULL in acpi_printk().
 - Added acpi_pr_debug().

---
Toshi Kani (5):
 ACPI: Add acpi_pr_<level>() interfaces
 ACPI: Update CPU hotplug error messages
 ACPI: Update Memory hotplug error messages
 ACPI: Update Container hotplug error messages
 ACPI: Update Dock hotplug error messages

---
 drivers/acpi/acpi_memhotplug.c  | 25 +++++++++++++------------
 drivers/acpi/container.c        | 10 ++--------
 drivers/acpi/dock.c             | 29 +++++++++++++----------------
 drivers/acpi/processor_driver.c | 38 +++++++++++++++++++++++---------------
 drivers/acpi/utils.c            | 34 ++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h         | 31 +++++++++++++++++++++++++++++++
 6 files changed, 116 insertions(+), 51 deletions(-)

[PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Toshi Kani]

This patch introduces acpi_pr_<level>(), where <level> is a kernel
message level such as err/warn/info, to support improved logging
messages for ACPI, esp. for hotplug operations.  acpi_pr_<level>()
appends "ACPI" prefix and ACPI object path to the messages.  This
improves diagnosis of hotplug operations since an error message in
a log file identifies an object that caused an issue.

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

For example:
  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

ACPI drivers can use acpi_pr_<level>() when they need to identify
a target ACPI object path in their messages, such as error cases.
The usage model is similar to dev_<level>().  acpi_pr_<level>() can
be used when device is not created/valid, which may be the case in
ACPI hotplug handlers.  ACPI object path is also consistent on the
platform, unlike device name that changes over hotplug operations.

ACPI drivers should use dev_<level>() when device is valid and
acpi_pr_<level>() is already used by the caller in its error path.
Device name provides more user friendly information.

ACPI drivers also continue to use pr_<level>() when messages do not
need to specify device information, such as boot-up messages.

Note: ACPI_[WARNING|INFO|ERROR]() are intended for the ACPICA and
are not associated with the kernel message level.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
Tested-by: Vijay Mohan Pandarathil <vijaymohan.pandarathil@hp.com>
---
 drivers/acpi/utils.c    | 34 ++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 462f7e3..4eade7d 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -457,3 +457,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
+	};
+	const 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 2242c10..93d5852 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -56,6 +56,37 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
 
 acpi_status
 acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **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__)
+
+/* REVISIT: Support CONFIG_DYNAMIC_DEBUG when necessary */
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
+#define acpi_pr_debug(handle, fmt, ...)					\
+	acpi_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__)
+#else
+#define acpi_pr_debug(handle, fmt, ...)					\
+({									\
+	if (0)								\
+		acpi_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__);	\
+	0;								\
+})
+#endif
+
 #ifdef CONFIG_ACPI
 
 #include <linux/proc_fs.h>
-- 
1.7.11.7

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Rafael J. Wysocki]

On Tuesday, November 06, 2012 08:02:06 AM Toshi Kani wrote:
> This patch introduces acpi_pr_<level>(), where <level> is a kernel
> message level such as err/warn/info, to support improved logging
> messages for ACPI, esp. for hotplug operations.  acpi_pr_<level>()
> appends "ACPI" prefix and ACPI object path to the messages.  This
> improves diagnosis of hotplug operations since an error message in
> a log file identifies an object that caused an issue.
> 
> acpi_pr_<level>() takes acpi_handle as an argument, which is passed
> to ACPI hotplug notify handlers from the ACPICA.  Therefore, it is
> always available unlike other kernel objects, such as device.
> 
> For example:
>   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
>
> ACPI drivers can use acpi_pr_<level>() when they need to identify
> a target ACPI object path in their messages, such as error cases.
> The usage model is similar to dev_<level>().  acpi_pr_<level>() can
> be used when device is not created/valid, which may be the case in
> ACPI hotplug handlers.  ACPI object path is also consistent on the
> platform, unlike device name that changes over hotplug operations.
> 
> ACPI drivers should use dev_<level>() when device is valid and
> acpi_pr_<level>() is already used by the caller in its error path.
> Device name provides more user friendly information.
> 
> ACPI drivers also continue to use pr_<level>() when messages do not
> need to specify device information, such as boot-up messages.
> 
> Note: ACPI_[WARNING|INFO|ERROR]() are intended for the ACPICA and
> are not associated with the kernel message level.

Well, the idea is generally good, but unfortunately acpi_get_name() is
not a cheap operation.  Namely, it takes the global namespace mutex,
so your acpi_printk() may be a source of serious contention on that
lock if used excessively from concurrent threads.

Do you think you can address this problem?

Moreover, this also means that acpi_printk() cannot be used from interrupt
context, so it is not a printk() replacement, which at least should be
documented.

Thanks,
Rafael

 
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> Tested-by: Vijay Mohan Pandarathil <vijaymohan.pandarathil@hp.com>
> ---
>  drivers/acpi/utils.c    | 34 ++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> index 462f7e3..4eade7d 100644
> --- a/drivers/acpi/utils.c
> +++ b/drivers/acpi/utils.c
> @@ -457,3 +457,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
> +	};
> +	const 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 2242c10..93d5852 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -56,6 +56,37 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
>  
>  acpi_status
>  acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **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__)
> +
> +/* REVISIT: Support CONFIG_DYNAMIC_DEBUG when necessary */
> +#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
> +#define acpi_pr_debug(handle, fmt, ...)					\
> +	acpi_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__)
> +#else
> +#define acpi_pr_debug(handle, fmt, ...)					\
> +({									\
> +	if (0)								\
> +		acpi_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__);	\
> +	0;								\
> +})
> +#endif
> +
>  #ifdef CONFIG_ACPI
>  
>  #include <linux/proc_fs.h>
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Toshi Kani]

On Mon, 2012-11-19 at 09:10 +0100, Rafael J. Wysocki wrote:
> On Tuesday, November 06, 2012 08:02:06 AM Toshi Kani wrote:
> > This patch introduces acpi_pr_<level>(), where <level> is a kernel
> > message level such as err/warn/info, to support improved logging
> > messages for ACPI, esp. for hotplug operations.  acpi_pr_<level>()
> > appends "ACPI" prefix and ACPI object path to the messages.  This
> > improves diagnosis of hotplug operations since an error message in
> > a log file identifies an object that caused an issue.
> > 
> > acpi_pr_<level>() takes acpi_handle as an argument, which is passed
> > to ACPI hotplug notify handlers from the ACPICA.  Therefore, it is
> > always available unlike other kernel objects, such as device.
> > 
> > For example:
> >   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
> >
> > ACPI drivers can use acpi_pr_<level>() when they need to identify
> > a target ACPI object path in their messages, such as error cases.
> > The usage model is similar to dev_<level>().  acpi_pr_<level>() can
> > be used when device is not created/valid, which may be the case in
> > ACPI hotplug handlers.  ACPI object path is also consistent on the
> > platform, unlike device name that changes over hotplug operations.
> > 
> > ACPI drivers should use dev_<level>() when device is valid and
> > acpi_pr_<level>() is already used by the caller in its error path.
> > Device name provides more user friendly information.
> > 
> > ACPI drivers also continue to use pr_<level>() when messages do not
> > need to specify device information, such as boot-up messages.
> > 
> > Note: ACPI_[WARNING|INFO|ERROR]() are intended for the ACPICA and
> > are not associated with the kernel message level.
> 
> Well, the idea is generally good, but unfortunately acpi_get_name() is
> not a cheap operation.  Namely, it takes the global namespace mutex,
> so your acpi_printk() may be a source of serious contention on that
> lock if used excessively from concurrent threads.
> 
> Do you think you can address this problem?

Hi Rafael,

I agree with you that the interface name may sound too generic as if it
can be used for any way.  How about changing the interface name to
acpi_hp_<level>() to clarify that it is intended for hot-plug operations
only?  The key goal is to be able to identify a failed device in
hot-plug error messages, so that we can diagnose an issue.  When used in
hot-plug operations, especially in error paths, the lock contention is
not an issue.  In regular code path (i.e. non-hot-plug operations),
dev_<level>() should be used since device object is available.

Here is a measurement result for the interface.  When there is no
locking contention, acpi_get_name() is reasonably fast as it does not
execute AML.

Avg. acpi_get_name()             587 ns
Avg. printk()                   3420 ns
Avg. kfree()                     238 ns
Avg. acpi_get_time()+kfree()     825 ns


> Moreover, this also means that acpi_printk() cannot be used from interrupt
> context, so it is not a printk() replacement, which at least should be
> documented.

Right.  I will document it in the comment and change log.


Thanks,
-Toshi

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Rafael J. Wysocki]

On Monday, November 19, 2012 09:10:58 AM Rafael J. Wysocki wrote:
> On Tuesday, November 06, 2012 08:02:06 AM Toshi Kani wrote:
> > This patch introduces acpi_pr_<level>(), where <level> is a kernel
> > message level such as err/warn/info, to support improved logging
> > messages for ACPI, esp. for hotplug operations.  acpi_pr_<level>()
> > appends "ACPI" prefix and ACPI object path to the messages.  This
> > improves diagnosis of hotplug operations since an error message in
> > a log file identifies an object that caused an issue.
> > 
> > acpi_pr_<level>() takes acpi_handle as an argument, which is passed
> > to ACPI hotplug notify handlers from the ACPICA.  Therefore, it is
> > always available unlike other kernel objects, such as device.
> > 
> > For example:
> >   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
> >
> > ACPI drivers can use acpi_pr_<level>() when they need to identify
> > a target ACPI object path in their messages, such as error cases.
> > The usage model is similar to dev_<level>().  acpi_pr_<level>() can
> > be used when device is not created/valid, which may be the case in
> > ACPI hotplug handlers.  ACPI object path is also consistent on the
> > platform, unlike device name that changes over hotplug operations.
> > 
> > ACPI drivers should use dev_<level>() when device is valid and
> > acpi_pr_<level>() is already used by the caller in its error path.
> > Device name provides more user friendly information.
> > 
> > ACPI drivers also continue to use pr_<level>() when messages do not
> > need to specify device information, such as boot-up messages.
> > 
> > Note: ACPI_[WARNING|INFO|ERROR]() are intended for the ACPICA and
> > are not associated with the kernel message level.
> 
> Well, the idea is generally good, but unfortunately acpi_get_name() is
> not a cheap operation.  Namely, it takes the global namespace mutex,
> so your acpi_printk() may be a source of serious contention on that
> lock if used excessively from concurrent threads.
> 
> Do you think you can address this problem?
> 
> Moreover, this also means that acpi_printk() cannot be used from interrupt
> context, so it is not a printk() replacement, which at least should be
> documented.

Unfortunately, I lost your reply to my previous message in this thread
due to my e-mail client malfunction.  Sorry about that.

What about calling them acpi_handle_printk() and acpi_handle_<level>,
respectively?  Then, if it is clearly documented that those things
acquire the global namespace mutex and are not suitable for interrupt
context, it should be OK.

And please take Joe's feedback into account. :-)

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Toshi Kani]

On Tue, 2012-11-20 at 02:17 +0100, Rafael J. Wysocki wrote:
> On Monday, November 19, 2012 09:10:58 AM Rafael J. Wysocki wrote:
> > On Tuesday, November 06, 2012 08:02:06 AM Toshi Kani wrote:
> > > This patch introduces acpi_pr_<level>(), where <level> is a kernel
> > > message level such as err/warn/info, to support improved logging
> > > messages for ACPI, esp. for hotplug operations.  acpi_pr_<level>()
> > > appends "ACPI" prefix and ACPI object path to the messages.  This
> > > improves diagnosis of hotplug operations since an error message in
> > > a log file identifies an object that caused an issue.
> > > 
> > > acpi_pr_<level>() takes acpi_handle as an argument, which is passed
> > > to ACPI hotplug notify handlers from the ACPICA.  Therefore, it is
> > > always available unlike other kernel objects, such as device.
> > > 
> > > For example:
> > >   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
> > >
> > > ACPI drivers can use acpi_pr_<level>() when they need to identify
> > > a target ACPI object path in their messages, such as error cases.
> > > The usage model is similar to dev_<level>().  acpi_pr_<level>() can
> > > be used when device is not created/valid, which may be the case in
> > > ACPI hotplug handlers.  ACPI object path is also consistent on the
> > > platform, unlike device name that changes over hotplug operations.
> > > 
> > > ACPI drivers should use dev_<level>() when device is valid and
> > > acpi_pr_<level>() is already used by the caller in its error path.
> > > Device name provides more user friendly information.
> > > 
> > > ACPI drivers also continue to use pr_<level>() when messages do not
> > > need to specify device information, such as boot-up messages.
> > > 
> > > Note: ACPI_[WARNING|INFO|ERROR]() are intended for the ACPICA and
> > > are not associated with the kernel message level.
> > 
> > Well, the idea is generally good, but unfortunately acpi_get_name() is
> > not a cheap operation.  Namely, it takes the global namespace mutex,
> > so your acpi_printk() may be a source of serious contention on that
> > lock if used excessively from concurrent threads.
> > 
> > Do you think you can address this problem?
> > 
> > Moreover, this also means that acpi_printk() cannot be used from interrupt
> > context, so it is not a printk() replacement, which at least should be
> > documented.
> 
> Unfortunately, I lost your reply to my previous message in this thread
> due to my e-mail client malfunction.  Sorry about that.
> 
> What about calling them acpi_handle_printk() and acpi_handle_<level>,
> respectively?  Then, if it is clearly documented that those things
> acquire the global namespace mutex and are not suitable for interrupt
> context, it should be OK.

Sounds good to me.  I will update the function names and document it in
the comment and change log.

> And please take Joe's feedback into account. :-)

Yes, Joe has been great help on this patchset (Thanks Joe!).  I think
the only remaining point is the header file location, which I will reply
to your other email.

Thanks,
-Toshi

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Joe Perches]

On Tue, 2012-11-06 at 08:02 -0700, Toshi Kani wrote:
> This patch introduces acpi_pr_<level>(), where <level> is a kernel
> message level such as err/warn/info, to support improved logging
> messages for ACPI, esp. for hotplug operations.
[]
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
[]
> @@ -56,6 +56,37 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
>  
>  acpi_status
>  acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld);
> +
> +void acpi_printk(const char *level, acpi_handle handle, const char *fmt, ...);

This should be declared with __printf(3, 4)

is acpi_bus.h really the right file for these prototypes?

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Toshi Kani]

On Mon, 2012-11-19 at 00:56 -0800, Joe Perches wrote:
> On Tue, 2012-11-06 at 08:02 -0700, Toshi Kani wrote:
> > This patch introduces acpi_pr_<level>(), where <level> is a kernel
> > message level such as err/warn/info, to support improved logging
> > messages for ACPI, esp. for hotplug operations.
> []
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> []
> > @@ -56,6 +56,37 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
> >  
> >  acpi_status
> >  acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld);
> > +
> > +void acpi_printk(const char *level, acpi_handle handle, const char *fmt, ...);
> 
> This should be declared with __printf(3, 4)

Hi Joe,

Yes, I will add __printf(3,4).  Thanks for pointing this out.

> is acpi_bus.h really the right file for these prototypes?

This interface is limited for ACPI, so it should be declared in a header
file under include/acpi.  Among the files in this directory, acpi_bus.h
seems to be a good fit as it declares the interfaces provided by ACPI
core.

Thanks,
-Toshi  

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Joe Perches]

On Mon, 2012-11-19 at 08:51 -0700, Toshi Kani wrote:
> On Mon, 2012-11-19 at 00:56 -0800, Joe Perches wrote:
> > is acpi_bus.h really the right file for these prototypes?
> This interface is limited for ACPI, so it should be declared in a header
> file under include/acpi.  Among the files in this directory, acpi_bus.h
> seems to be a good fit as it declares the interfaces provided by ACPI
> core.

I'd've put them in acpi.h or maybe created acpi_utils.h 

cheers, Joe

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Toshi Kani]

On Mon, 2012-11-19 at 08:06 -0800, Joe Perches wrote:
> On Mon, 2012-11-19 at 08:51 -0700, Toshi Kani wrote:
> > On Mon, 2012-11-19 at 00:56 -0800, Joe Perches wrote:
> > > is acpi_bus.h really the right file for these prototypes?
> > This interface is limited for ACPI, so it should be declared in a header
> > file under include/acpi.  Among the files in this directory, acpi_bus.h
> > seems to be a good fit as it declares the interfaces provided by ACPI
> > core.
> 
> I'd've put them in acpi.h or maybe created acpi_utils.h 

Hi Joe,

We cannot use acpi.h since it is an ACPICA header.  Creating
acpi_utils.h sounds good idea.  However, in line 40 of acpi_bus.h, it
has the following comment and declares multiple utility interfaces.

====
/* acpi_utils.h */
acpi_status
acpi_extract_package(union acpi_object *package,
                struct acpi_buffer *format, struct acpi_buffer *buffer);
  :
====

This looks to me that there was acpi_utils.h before, but someone had
merged it into acpi_bus.h for some reason.  This change was made before
the initial git repository was created, so I cannot check the change
log.  So, as of today, acpi_bus.h seems to be the place to declare ACPI
utility interfaces for ACPI core.

Thanks,
-Toshi 

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Rafael J. Wysocki]

On Monday, November 19, 2012 09:16:52 AM Toshi Kani wrote:
> On Mon, 2012-11-19 at 08:06 -0800, Joe Perches wrote:
> > On Mon, 2012-11-19 at 08:51 -0700, Toshi Kani wrote:
> > > On Mon, 2012-11-19 at 00:56 -0800, Joe Perches wrote:
> > > > is acpi_bus.h really the right file for these prototypes?
> > > This interface is limited for ACPI, so it should be declared in a header
> > > file under include/acpi.  Among the files in this directory, acpi_bus.h
> > > seems to be a good fit as it declares the interfaces provided by ACPI
> > > core.
> > 
> > I'd've put them in acpi.h or maybe created acpi_utils.h 
> 
> Hi Joe,
> 
> We cannot use acpi.h since it is an ACPICA header.

Which acpi.h are you referring to?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Toshi Kani]

On Mon, 2012-11-19 at 18:35 +0100, Rafael J. Wysocki wrote:
> On Monday, November 19, 2012 09:16:52 AM Toshi Kani wrote:
> > On Mon, 2012-11-19 at 08:06 -0800, Joe Perches wrote:
> > > On Mon, 2012-11-19 at 08:51 -0700, Toshi Kani wrote:
> > > > On Mon, 2012-11-19 at 00:56 -0800, Joe Perches wrote:
> > > > > is acpi_bus.h really the right file for these prototypes?
> > > > This interface is limited for ACPI, so it should be declared in a header
> > > > file under include/acpi.  Among the files in this directory, acpi_bus.h
> > > > seems to be a good fit as it declares the interfaces provided by ACPI
> > > > core.
> > > 
> > > I'd've put them in acpi.h or maybe created acpi_utils.h 
> > 
> > Hi Joe,
> > 
> > We cannot use acpi.h since it is an ACPICA header.
> 
> Which acpi.h are you referring to?


I was referring "include/acpi/acpi.h".

Thanks,
-Toshi

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Rafael J. Wysocki]

On Monday, November 19, 2012 10:26:43 AM Toshi Kani wrote:
> On Mon, 2012-11-19 at 18:35 +0100, Rafael J. Wysocki wrote:
> > On Monday, November 19, 2012 09:16:52 AM Toshi Kani wrote:
> > > On Mon, 2012-11-19 at 08:06 -0800, Joe Perches wrote:
> > > > On Mon, 2012-11-19 at 08:51 -0700, Toshi Kani wrote:
> > > > > On Mon, 2012-11-19 at 00:56 -0800, Joe Perches wrote:
> > > > > > is acpi_bus.h really the right file for these prototypes?
> > > > > This interface is limited for ACPI, so it should be declared in a header
> > > > > file under include/acpi.  Among the files in this directory, acpi_bus.h
> > > > > seems to be a good fit as it declares the interfaces provided by ACPI
> > > > > core.
> > > > 
> > > > I'd've put them in acpi.h or maybe created acpi_utils.h 
> > > 
> > > Hi Joe,
> > > 
> > > We cannot use acpi.h since it is an ACPICA header.
> > 
> > Which acpi.h are you referring to?
> 
> 
> I was referring "include/acpi/acpi.h".

There is another acpi.h in include/linux that can be used for this purpose.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v5 RESEND 1/5] ACPI: Add acpi_pr_<level>() interfaces

[Toshi Kani]

On Tue, 2012-11-20 at 02:04 +0100, Rafael J. Wysocki wrote:
> On Monday, November 19, 2012 10:26:43 AM Toshi Kani wrote:
> > On Mon, 2012-11-19 at 18:35 +0100, Rafael J. Wysocki wrote:
> > > On Monday, November 19, 2012 09:16:52 AM Toshi Kani wrote:
> > > > On Mon, 2012-11-19 at 08:06 -0800, Joe Perches wrote:
> > > > > On Mon, 2012-11-19 at 08:51 -0700, Toshi Kani wrote:
> > > > > > On Mon, 2012-11-19 at 00:56 -0800, Joe Perches wrote:
> > > > > > > is acpi_bus.h really the right file for these prototypes?
> > > > > > This interface is limited for ACPI, so it should be declared in a header
> > > > > > file under include/acpi.  Among the files in this directory, acpi_bus.h
> > > > > > seems to be a good fit as it declares the interfaces provided by ACPI
> > > > > > core.
> > > > > 
> > > > > I'd've put them in acpi.h or maybe created acpi_utils.h 
> > > > 
> > > > Hi Joe,
> > > > 
> > > > We cannot use acpi.h since it is an ACPICA header.
> > > 
> > > Which acpi.h are you referring to?
> > 
> > 
> > I was referring "include/acpi/acpi.h".
> 
> There is another acpi.h in include/linux that can be used for this purpose.

I see.  I agree that include/linux/acpi.h can be used.  I will update
the patchset to use this header.

Thanks,
-Toshi

[PATCH v5 RESEND 2/5] ACPI: Update CPU hotplug error messages

[Toshi Kani]

Updated CPU hotplug error messages with acpi_pr_<level>(),
dev_<level>() and pr_<level>().  Modified some messages for
clarity.  Added error status / id info to the messages where
needed.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
Tested-by: Vijay Mohan Pandarathil <vijaymohan.pandarathil@hp.com>
---
 drivers/acpi/processor_driver.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 8cc33d0..df21fa0 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -282,7 +282,9 @@ static int acpi_processor_get_info(struct acpi_device *device)
 		/* Declared with "Processor" statement; match ProcessorID */
 		status = acpi_evaluate_object(pr->handle, NULL, NULL, &buffer);
 		if (ACPI_FAILURE(status)) {
-			printk(KERN_ERR PREFIX "Evaluating processor object\n");
+			dev_err(&device->dev,
+				"Failed to evaluate processor object (0x%x)\n",
+				status);
 			return -ENODEV;
 		}
 
@@ -301,8 +303,9 @@ static int acpi_processor_get_info(struct acpi_device *device)
 		status = acpi_evaluate_integer(pr->handle, METHOD_NAME__UID,
 						NULL, &value);
 		if (ACPI_FAILURE(status)) {
-			printk(KERN_ERR PREFIX
-			    "Evaluating processor _UID [%#x]\n", status);
+			dev_err(&device->dev,
+				"Failed to evaluate processor _UID (0x%x)\n",
+				status);
 			return -ENODEV;
 		}
 		device_declaration = 1;
@@ -345,7 +348,7 @@ static int acpi_processor_get_info(struct acpi_device *device)
 	if (!object.processor.pblk_address)
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n"));
 	else if (object.processor.pblk_length != 6)
-		printk(KERN_ERR PREFIX "Invalid PBLK length [%d]\n",
+		dev_err(&device->dev, "Invalid PBLK length [%d]\n",
 			    object.processor.pblk_length);
 	else {
 		pr->throttling.address = object.processor.pblk_address;
@@ -430,8 +433,8 @@ static int acpi_cpu_soft_notify(struct notifier_block *nfb,
 		 * Initialize missing things
 		 */
 		if (pr->flags.need_hotplug_init) {
-			printk(KERN_INFO "Will online and init hotplugged "
-			       "CPU: %d\n", pr->id);
+			pr_info("Will online and init hotplugged CPU: %d\n",
+				pr->id);
 			WARN(acpi_processor_start(pr), "Failed to start CPU:"
 				" %d\n", pr->id);
 			pr->flags.need_hotplug_init = 0;
@@ -492,14 +495,16 @@ static __ref int acpi_processor_start(struct acpi_processor *pr)
 				   &pr->cdev->device.kobj,
 				   "thermal_cooling");
 	if (result) {
-		printk(KERN_ERR PREFIX "Create sysfs link\n");
+		dev_err(&device->dev,
+			"Failed to create sysfs link 'thermal_cooling'\n");
 		goto err_thermal_unregister;
 	}
 	result = sysfs_create_link(&pr->cdev->device.kobj,
 				   &device->dev.kobj,
 				   "device");
 	if (result) {
-		printk(KERN_ERR PREFIX "Create sysfs link\n");
+		dev_err(&pr->cdev->device,
+			"Failed to create sysfs link 'device'\n");
 		goto err_remove_sysfs_thermal;
 	}
 
@@ -561,8 +566,9 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device)
 	 */
 	if (per_cpu(processor_device_array, pr->id) != NULL &&
 	    per_cpu(processor_device_array, pr->id) != device) {
-		printk(KERN_WARNING "BIOS reported wrong ACPI id "
-			"for the processor\n");
+		dev_warn(&device->dev,
+			"BIOS reported wrong ACPI id %d for the processor\n",
+			pr->id);
 		result = -ENODEV;
 		goto err_free_cpumask;
 	}
@@ -716,7 +722,7 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
 
 		result = acpi_processor_device_add(handle, &device);
 		if (result) {
-			printk(KERN_ERR PREFIX "Unable to add the device\n");
+			acpi_pr_err(handle, "Unable to add the device\n");
 			break;
 		}
 
@@ -728,17 +734,19 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
 				  "received ACPI_NOTIFY_EJECT_REQUEST\n"));
 
 		if (acpi_bus_get_device(handle, &device)) {
-			pr_err(PREFIX "Device don't exist, dropping EJECT\n");
+			acpi_pr_err(handle,
+				"Device don't exist, dropping EJECT\n");
 			break;
 		}
 		if (!acpi_driver_data(device)) {
-			pr_err(PREFIX "Driver data is NULL, dropping EJECT\n");
+			acpi_pr_err(handle,
+				"Driver data is NULL, dropping EJECT\n");
 			break;
 		}
 
 		ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
 		if (!ej_event) {
-			pr_err(PREFIX "No memory, dropping EJECT\n");
+			acpi_pr_err(handle, "No memory, dropping EJECT\n");
 			break;
 		}
 
@@ -848,7 +856,7 @@ static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
 	 * and do it when the CPU gets online the first time
 	 * TBD: Cleanup above functions and try to do this more elegant.
 	 */
-	printk(KERN_INFO "CPU %d got hotplugged\n", pr->id);
+	pr_info("CPU %d got hotplugged\n", pr->id);
 	pr->flags.need_hotplug_init = 1;
 
 	return AE_OK;
-- 
1.7.11.7

[PATCH v5 RESEND 3/5] ACPI: Update Memory hotplug error messages

[Toshi Kani]

Updated Memory hotplug error messages with acpi_pr_<level>(),
dev_<level>() and pr_<level>().

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/acpi_memhotplug.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 92c973a..d00de4b 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -178,7 +178,7 @@ acpi_memory_get_device(acpi_handle handle,
 	/* Get the parent device */
 	result = acpi_bus_get_device(phandle, &pdevice);
 	if (result) {
-		printk(KERN_WARNING PREFIX "Cannot get acpi bus device");
+		acpi_pr_warn(phandle, "Cannot get acpi bus device\n");
 		return -EINVAL;
 	}
 
@@ -188,14 +188,14 @@ acpi_memory_get_device(acpi_handle handle,
 	 */
 	result = acpi_bus_add(&device, pdevice, handle, ACPI_BUS_TYPE_DEVICE);
 	if (result) {
-		printk(KERN_WARNING PREFIX "Cannot add acpi bus");
+		acpi_pr_warn(handle, "Cannot add acpi bus\n");
 		return -EINVAL;
 	}
 
       end:
 	*mem_device = acpi_driver_data(device);
 	if (!(*mem_device)) {
-		printk(KERN_ERR "\n driver data not found");
+		dev_err(&device->dev, "driver data not found\n");
 		return -ENODEV;
 	}
 
@@ -232,7 +232,8 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
 	/* Get the range from the _CRS */
 	result = acpi_memory_get_device_resources(mem_device);
 	if (result) {
-		printk(KERN_ERR PREFIX "get_device_resources failed\n");
+		dev_err(&mem_device->device->dev,
+			"get_device_resources failed\n");
 		mem_device->state = MEMORY_INVALID_STATE;
 		return result;
 	}
@@ -267,7 +268,7 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
 	}
 	mutex_unlock(&mem_device->list_lock);
 	if (!num_enabled) {
-		printk(KERN_ERR PREFIX "add_memory failed\n");
+		dev_err(&mem_device->device->dev, "add_memory failed\n");
 		mem_device->state = MEMORY_INVALID_STATE;
 		return -EINVAL;
 	}
@@ -377,7 +378,7 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
 			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 					  "\nReceived DEVICE CHECK notification for device\n"));
 		if (acpi_memory_get_device(handle, &mem_device)) {
-			printk(KERN_ERR PREFIX "Cannot find driver data\n");
+			acpi_pr_err(handle, "Cannot find driver data\n");
 			break;
 		}
 
@@ -385,7 +386,7 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
 			break;
 
 		if (acpi_memory_enable_device(mem_device)) {
-			printk(KERN_ERR PREFIX "Cannot enable memory device\n");
+			acpi_pr_err(handle, "Cannot enable memory device\n");
 			break;
 		}
 
@@ -397,12 +398,12 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
 				  "\nReceived EJECT REQUEST notification for device\n"));
 
 		if (acpi_bus_get_device(handle, &device)) {
-			printk(KERN_ERR PREFIX "Device doesn't exist\n");
+			acpi_pr_err(handle, "Device doesn't exist\n");
 			break;
 		}
 		mem_device = acpi_driver_data(device);
 		if (!mem_device) {
-			printk(KERN_ERR PREFIX "Driver Data is NULL\n");
+			acpi_pr_err(handle, "Driver Data is NULL\n");
 			break;
 		}
 
@@ -413,7 +414,7 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data)
 		 *      with generic sysfs driver
 		 */
 		if (acpi_memory_disable_device(mem_device)) {
-			printk(KERN_ERR PREFIX "Disable memory device\n");
+			acpi_pr_err(handle, "Failed to remove memory device\n");
 			/*
 			 * If _EJ0 was called but failed, _OST is not
 			 * necessary.
@@ -475,7 +476,7 @@ static int acpi_memory_device_add(struct acpi_device *device)
 	/* Set the device state */
 	mem_device->state = MEMORY_POWER_ON_STATE;
 
-	printk(KERN_DEBUG "%s \n", acpi_device_name(device));
+	pr_debug("%s\n", acpi_device_name(device));
 
 	/*
 	 * Early boot code has recognized memory area by EFI/E820.
@@ -490,7 +491,7 @@ static int acpi_memory_device_add(struct acpi_device *device)
 		/* call add_memory func */
 		result = acpi_memory_enable_device(mem_device);
 		if (result)
-			printk(KERN_ERR PREFIX
+			dev_err(&device->dev,
 				"Error in acpi_memory_enable_device\n");
 	}
 	return result;
-- 
1.7.11.7

[PATCH v5 RESEND 4/5] ACPI: Update Container hotplug error messages

[Toshi Kani]

Updated Container hotplug error messages with acpi_pr_<level>()
and pr_<level>().  Removed an unnecessary check to device pointer
in acpi_container_add().

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/container.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/container.c b/drivers/acpi/container.c
index 69e2d6b..71d77ab 100644
--- a/drivers/acpi/container.c
+++ b/drivers/acpi/container.c
@@ -110,12 +110,6 @@ static int acpi_container_add(struct acpi_device *device)
 {
 	struct acpi_container *container;
 
-
-	if (!device) {
-		printk(KERN_ERR PREFIX "device is NULL\n");
-		return -EINVAL;
-	}
-
 	container = kzalloc(sizeof(struct acpi_container), GFP_KERNEL);
 	if (!container)
 		return -ENOMEM;
@@ -177,7 +171,7 @@ static void container_notify_cb(acpi_handle handle, u32 type, void *context)
 	case ACPI_NOTIFY_BUS_CHECK:
 		/* Fall through */
 	case ACPI_NOTIFY_DEVICE_CHECK:
-		printk(KERN_WARNING "Container driver received %s event\n",
+		pr_debug("Container driver received %s event\n",
 		       (type == ACPI_NOTIFY_BUS_CHECK) ?
 		       "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK");
 
@@ -198,7 +192,7 @@ static void container_notify_cb(acpi_handle handle, u32 type, void *context)
 
 		result = container_device_add(&device, handle);
 		if (result) {
-			printk(KERN_WARNING "Failed to add container\n");
+			acpi_pr_warn(handle, "Failed to add container\n");
 			break;
 		}
 
-- 
1.7.11.7

[PATCH v5 RESEND 5/5] ACPI: Update Dock hotplug error messages

[Toshi Kani]

Updated Dock hotplug error messages with acpi_pr_<level>()
and pr_<level>().  Replaced acpi_get_name() & kfree() with
apci_pr_<level>().  Added error status to the messages where
needed.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
 drivers/acpi/dock.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index ae4ebf2..52e4bfd 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -460,12 +460,8 @@ static void handle_dock(struct dock_station *ds, int dock)
 	struct acpi_object_list arg_list;
 	union acpi_object arg;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	struct acpi_buffer name_buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 
-	acpi_get_name(ds->handle, ACPI_FULL_PATHNAME, &name_buffer);
-
-	printk(KERN_INFO PREFIX "%s - %s\n",
-		(char *)name_buffer.pointer, dock ? "docking" : "undocking");
+	acpi_pr_info(ds->handle, "%s\n", dock ? "docking" : "undocking");
 
 	/* _DCK method has one argument */
 	arg_list.count = 1;
@@ -474,11 +470,10 @@ static void handle_dock(struct dock_station *ds, int dock)
 	arg.integer.value = dock;
 	status = acpi_evaluate_object(ds->handle, "_DCK", &arg_list, &buffer);
 	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND)
-		ACPI_EXCEPTION((AE_INFO, status, "%s - failed to execute"
-			" _DCK\n", (char *)name_buffer.pointer));
+		acpi_pr_err(ds->handle, "Failed to execute _DCK (0x%x)\n",
+				status);
 
 	kfree(buffer.pointer);
-	kfree(name_buffer.pointer);
 }
 
 static inline void dock(struct dock_station *ds)
@@ -525,9 +520,11 @@ static void dock_lock(struct dock_station *ds, int lock)
 	status = acpi_evaluate_object(ds->handle, "_LCK", &arg_list, NULL);
 	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
 		if (lock)
-			printk(KERN_WARNING PREFIX "Locking device failed\n");
+			acpi_pr_warn(ds->handle,
+				"Locking device failed (0x%x)\n", status);
 		else
-			printk(KERN_WARNING PREFIX "Unlocking device failed\n");
+			acpi_pr_warn(ds->handle,
+				"Unlocking device failed (0x%x)\n", status);
 	}
 }
 
@@ -667,7 +664,7 @@ static int handle_eject_request(struct dock_station *ds, u32 event)
 	dock_lock(ds, 0);
 	eject_dock(ds);
 	if (dock_present(ds)) {
-		printk(KERN_ERR PREFIX "Unable to undock!\n");
+		acpi_pr_err(ds->handle, "Unable to undock!\n");
 		return -EBUSY;
 	}
 	complete_undock(ds);
@@ -715,7 +712,7 @@ static void dock_notify(acpi_handle handle, u32 event, void *data)
 			begin_dock(ds);
 			dock(ds);
 			if (!dock_present(ds)) {
-				printk(KERN_ERR PREFIX "Unable to dock!\n");
+				acpi_pr_err(handle, "Unable to dock!\n");
 				complete_dock(ds);
 				break;
 			}
@@ -743,7 +740,7 @@ static void dock_notify(acpi_handle handle, u32 event, void *data)
 			dock_event(ds, event, UNDOCK_EVENT);
 		break;
 	default:
-		printk(KERN_ERR PREFIX "Unknown dock event %d\n", event);
+		acpi_pr_err(handle, "Unknown dock event %d\n", event);
 	}
 }
 
@@ -987,7 +984,7 @@ err_rmgroup:
 	sysfs_remove_group(&dd->dev.kobj, &dock_attribute_group);
 err_unregister:
 	platform_device_unregister(dd);
-	printk(KERN_ERR "%s encountered error %d\n", __func__, ret);
+	acpi_pr_err(handle, "%s encountered error %d\n", __func__, ret);
 	return ret;
 }
 
@@ -1043,12 +1040,12 @@ static int __init dock_init(void)
 		ACPI_UINT32_MAX, find_dock_and_bay, NULL, NULL, NULL);
 
 	if (!dock_station_count) {
-		printk(KERN_INFO PREFIX "No dock devices found.\n");
+		pr_info(PREFIX "No dock devices found.\n");
 		return 0;
 	}
 
 	register_acpi_bus_notifier(&dock_acpi_notifier);
-	printk(KERN_INFO PREFIX "%s: %d docks/bays found\n",
+	pr_info(PREFIX "%s: %d docks/bays found\n",
 		ACPI_DOCK_DRIVER_DESCRIPTION, dock_station_count);
 	return 0;
 }
-- 
1.7.11.7