2012-11-20 20:53:23

by Toshi Kani

[permalink] [raw]
Subject: [PATCH v6 0/5] ACPI: hotplug messages improvement

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_handle_<level>(),
and uses them to update the notify handlers.

acpi_handle_<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 identifies an object that caused an issue.

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

v6:
- Changed the interface name to acpi_handle_<level>(), and
declared them in include/linux/acpi.h.
- Added __printf(3,4) to acpi_handle_printk() prototype.
- Documented the interfaces acquire the global namespace mutex
and may not be called from interrupt context.

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_handle_<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 | 24 ++++++++++++-----------
drivers/acpi/container.c | 10 ++--------
drivers/acpi/dock.c | 30 ++++++++++++++--------------
drivers/acpi/processor_driver.c | 39 +++++++++++++++++++++++--------------
drivers/acpi/utils.c | 37 +++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 43 +++++++++++++++++++++++++++++++++++++++++
6 files changed, 133 insertions(+), 50 deletions(-)


2012-11-20 20:53:33

by Toshi Kani

[permalink] [raw]
Subject: [PATCH v6 1/5] ACPI: Add acpi_handle_<level>() interfaces

This patch introduces acpi_handle_<level>(), where <level> is
a kernel message level such as err/warn/info, to support improved
logging messages for ACPI, esp. hot-plug operations.
acpi_handle_<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. This interface acquires the global namespace
mutex and may not be called from interrupt context.

acpi_handle_<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_handle_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 hot-plug drivers can use acpi_handle_<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_handle_<level>() can be used when a device is not created or
is invalid during hot-plug operations. ACPI object path is also
consistent on the platform, unlike device name that gets incremented
over hotplug operations.

ACPI drivers should use dev_<level>() when a device object is valid.
Device name provides more user friendly information, and avoids
acquiring the global ACPI namespace mutex. ACPI drivers also
continue to use pr_<level>() when they 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 <[email protected]>
Tested-by: Vijay Mohan Pandarathil <[email protected]>
---
drivers/acpi/utils.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 462f7e3..c1f1072 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -28,6 +28,7 @@
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/types.h>
+#include <linux/acpi.h>
#include <acpi/acpi_bus.h>
#include <acpi/acpi_drivers.h>

@@ -457,3 +458,39 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
#endif
}
EXPORT_SYMBOL(acpi_evaluate_hotplug_ost);
+
+/**
+ * acpi_handle_printk: Print message with ACPI prefix and object path
+ *
+ * This function is called through acpi_handle_<level> macros and prints
+ * a message with ACPI prefix and object path. This function acquires
+ * the global namespace mutex and may not be called from interrupt context.
+ */
+void
+acpi_handle_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_handle_printk);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0bb2070..e96753e 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -517,4 +517,47 @@ static inline int acpi_dev_pm_attach(struct device *dev) { return -ENODEV; }
static inline void acpi_dev_pm_detach(struct device *dev) {}
#endif

+#ifdef CONFIG_ACPI
+__printf(3, 4)
+void acpi_handle_printk(const char *level, acpi_handle handle,
+ const char *fmt, ...);
+#else /* !CONFIG_ACPI */
+static inline __printf(3, 4) void
+acpi_handle_printk(const char *level, void *handle, const char *fmt, ...) {}
+#endif /* !CONFIG_ACPI */
+
+/*
+ * acpi_handle_<level>: Print message with ACPI prefix and object path
+ *
+ * These interfaces acquire the global namespace mutex and may not be
+ * called from interrupt context.
+ */
+#define acpi_handle_emerg(handle, fmt, ...) \
+ acpi_handle_printk(KERN_EMERG, handle, fmt, ##__VA_ARGS__)
+#define acpi_handle_alert(handle, fmt, ...) \
+ acpi_handle_printk(KERN_ALERT, handle, fmt, ##__VA_ARGS__)
+#define acpi_handle_crit(handle, fmt, ...) \
+ acpi_handle_printk(KERN_CRIT, handle, fmt, ##__VA_ARGS__)
+#define acpi_handle_err(handle, fmt, ...) \
+ acpi_handle_printk(KERN_ERR, handle, fmt, ##__VA_ARGS__)
+#define acpi_handle_warn(handle, fmt, ...) \
+ acpi_handle_printk(KERN_WARNING, handle, fmt, ##__VA_ARGS__)
+#define acpi_handle_notice(handle, fmt, ...) \
+ acpi_handle_printk(KERN_NOTICE, handle, fmt, ##__VA_ARGS__)
+#define acpi_handle_info(handle, fmt, ...) \
+ acpi_handle_printk(KERN_INFO, handle, fmt, ##__VA_ARGS__)
+
+/* REVISIT: Support CONFIG_DYNAMIC_DEBUG when necessary */
+#if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
+#define acpi_handle_debug(handle, fmt, ...) \
+ acpi_handle_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__)
+#else
+#define acpi_handle_debug(handle, fmt, ...) \
+({ \
+ if (0) \
+ acpi_handle_printk(KERN_DEBUG, handle, fmt, ##__VA_ARGS__); \
+ 0; \
+})
+#endif
+
#endif /*_LINUX_ACPI_H*/
--
1.7.11.7

2012-11-20 20:53:40

by Toshi Kani

[permalink] [raw]
Subject: [PATCH v6 3/5] ACPI: Update Memory hotplug error messages

Updated Memory hotplug error messages with acpi_handle_<level>(),
dev_<level>() and pr_<level>(). Added missing "\n".

Signed-off-by: Toshi Kani <[email protected]>
---
drivers/acpi/acpi_memhotplug.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index e0f7425..eb30e5a 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -31,6 +31,7 @@
#include <linux/types.h>
#include <linux/memory_hotplug.h>
#include <linux/slab.h>
+#include <linux/acpi.h>
#include <acpi/acpi_drivers.h>

#define ACPI_MEMORY_DEVICE_CLASS "memory"
@@ -175,7 +176,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_handle_warn(phandle, "Cannot get acpi bus device\n");
return -EINVAL;
}

@@ -185,14 +186,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_handle_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;
}

@@ -229,7 +230,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;
}
@@ -276,7 +278,7 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
num_enabled++;
}
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;
}
@@ -336,7 +338,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_handle_err(handle, "Cannot find driver data\n");
break;
}

@@ -344,7 +346,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_handle_err(handle,"Cannot enable memory device\n");
break;
}

@@ -356,12 +358,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_handle_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_handle_err(handle, "Driver Data is NULL\n");
break;
}

@@ -429,13 +431,13 @@ 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));

if (!acpi_memory_check_device(mem_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");
acpi_memory_device_free(mem_device);
}
--
1.7.11.7

2012-11-20 20:53:59

by Toshi Kani

[permalink] [raw]
Subject: [PATCH v6 5/5] ACPI: Update Dock hotplug error messages

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

Signed-off-by: Toshi Kani <[email protected]>
---
drivers/acpi/dock.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index ae4ebf2..f32bd47 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -31,6 +31,7 @@
#include <linux/platform_device.h>
#include <linux/jiffies.h>
#include <linux/stddef.h>
+#include <linux/acpi.h>
#include <acpi/acpi_bus.h>
#include <acpi/acpi_drivers.h>

@@ -460,12 +461,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_handle_info(ds->handle, "%s\n", dock ? "docking" : "undocking");

/* _DCK method has one argument */
arg_list.count = 1;
@@ -474,11 +471,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_handle_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 +521,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_handle_warn(ds->handle,
+ "Locking device failed (0x%x)\n", status);
else
- printk(KERN_WARNING PREFIX "Unlocking device failed\n");
+ acpi_handle_warn(ds->handle,
+ "Unlocking device failed (0x%x)\n", status);
}
}

@@ -667,7 +665,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_handle_err(ds->handle, "Unable to undock!\n");
return -EBUSY;
}
complete_undock(ds);
@@ -715,7 +713,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_handle_err(handle, "Unable to dock!\n");
complete_dock(ds);
break;
}
@@ -743,7 +741,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_handle_err(handle, "Unknown dock event %d\n", event);
}
}

@@ -987,7 +985,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_handle_err(handle, "%s encountered error %d\n", __func__, ret);
return ret;
}

@@ -1043,12 +1041,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

2012-11-20 20:53:45

by Toshi Kani

[permalink] [raw]
Subject: [PATCH v6 4/5] ACPI: Update Container hotplug error messages

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

Signed-off-by: Toshi Kani <[email protected]>
---
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..811910b 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_handle_warn(handle, "Failed to add container\n");
break;
}

--
1.7.11.7

2012-11-20 20:53:38

by Toshi Kani

[permalink] [raw]
Subject: [PATCH v6 2/5] ACPI: Update CPU hotplug error messages

Updated CPU hotplug error messages with acpi_handle_<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 <[email protected]>
Tested-by: Vijay Mohan Pandarathil <[email protected]>
---
drivers/acpi/processor_driver.c | 39 ++++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 8cc33d0..e83311b 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -44,6 +44,7 @@
#include <linux/moduleparam.h>
#include <linux/cpuidle.h>
#include <linux/slab.h>
+#include <linux/acpi.h>

#include <asm/io.h>
#include <asm/cpu.h>
@@ -282,7 +283,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 +304,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 +349,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 +434,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 +496,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 +567,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 +723,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_handle_err(handle, "Unable to add the device\n");
break;
}

@@ -728,17 +735,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_handle_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_handle_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_handle_err(handle, "No memory, dropping EJECT\n");
break;
}

@@ -848,7 +857,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

2012-11-20 21:10:40

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v6 1/5] ACPI: Add acpi_handle_<level>() interfaces

On Tue, 2012-11-20 at 13:44 -0700, Toshi Kani wrote:
> This interface acquires the global namespace
> mutex and may not be called from interrupt context.
[]
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
[]
> +/**
> + * acpi_handle_printk: Print message with ACPI prefix and object path
> + *
> + * This function is called through acpi_handle_<level> macros and prints
> + * a message with ACPI prefix and object path. This function acquires
> + * the global namespace mutex and may not be called from interrupt context.
> + */
> +void
> +acpi_handle_printk(const char *level, acpi_handle handle, const char *fmt, ...)
> +{
[]
> + ret = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> + if (ret == AE_OK)
> + path = buffer.pointer;
> + else
> + path = "<n/a>";

Perhaps:
if (in_interrupt() ||
acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer) != AE_OK)
path = "<n/a>";
else
path = buffer.pointer;

so that any interrupt context still works, just
not printing the correct acpi_get_name;

2012-11-20 21:20:31

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH v6 1/5] ACPI: Add acpi_handle_<level>() interfaces

On Tue, 2012-11-20 at 13:10 -0800, Joe Perches wrote:
> On Tue, 2012-11-20 at 13:44 -0700, Toshi Kani wrote:
> > This interface acquires the global namespace
> > mutex and may not be called from interrupt context.
> []
> > diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> []
> > +/**
> > + * acpi_handle_printk: Print message with ACPI prefix and object path
> > + *
> > + * This function is called through acpi_handle_<level> macros and prints
> > + * a message with ACPI prefix and object path. This function acquires
> > + * the global namespace mutex and may not be called from interrupt context.
> > + */
> > +void
> > +acpi_handle_printk(const char *level, acpi_handle handle, const char *fmt, ...)
> > +{
> []
> > + ret = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> > + if (ret == AE_OK)
> > + path = buffer.pointer;
> > + else
> > + path = "<n/a>";
>
> Perhaps:
> if (in_interrupt() ||
> acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer) != AE_OK)
> path = "<n/a>";
> else
> path = buffer.pointer;
>
> so that any interrupt context still works, just
> not printing the correct acpi_get_name;

Hi Joe,

Good idea. I will update the patch as you suggested.

Thanks,
-Toshi