* [PATCH] ACPI: Add CPU hotplug support for processor device objects
@ 2012-03-19 19:08 Toshi Kani
2012-03-20 16:55 ` Bjorn Helgaas
0 siblings, 1 reply; 3+ messages in thread
From: Toshi Kani @ 2012-03-19 19:08 UTC (permalink / raw)
To: lenb; +Cc: linux-acpi, Toshi Kani
acpi_processor_install_hotplug_notify() registers processor objects to
receive ACPI CPU hotplug event notifications. This patch additionally
registers processor device objects (ACPI0007) to receive the notifications
as well.
Signed-off-by: Toshi Kani <toshi.kani@hp.com>
---
drivers/acpi/processor_driver.c | 48 ++++++++++++++++++++++++++++++--------
1 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 8ae05ce..50be277 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -68,6 +68,7 @@
#define ACPI_PROCESSOR_NOTIFY_PERFORMANCE 0x80
#define ACPI_PROCESSOR_NOTIFY_POWER 0x81
#define ACPI_PROCESSOR_NOTIFY_THROTTLING 0x82
+#define ACPI_PROCESSOR_DEVICE_HID "ACPI0007"
#define ACPI_PROCESSOR_LIMIT_USER 0
#define ACPI_PROCESSOR_LIMIT_THERMAL 1
@@ -88,7 +89,7 @@ static int acpi_processor_start(struct acpi_processor *pr);
static const struct acpi_device_id processor_device_ids[] = {
{ACPI_PROCESSOR_OBJECT_HID, 0},
- {"ACPI0007", 0},
+ {ACPI_PROCESSOR_DEVICE_HID, 0},
{"", 0},
};
MODULE_DEVICE_TABLE(acpi, processor_device_ids);
@@ -741,20 +742,46 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
return;
}
+static acpi_status is_processor_device(acpi_handle handle)
+{
+ struct acpi_device_info *info;
+ char *hid;
+ acpi_status status;
+
+ status = acpi_get_object_info(handle, &info);
+ if (ACPI_FAILURE(status))
+ return status;
+
+ if (info->type == ACPI_TYPE_PROCESSOR) {
+ kfree(info);
+ return AE_OK; /* found a processor object */
+ }
+
+ if (!(info->valid & ACPI_VALID_HID)) {
+ kfree(info);
+ return AE_ERROR;
+ }
+
+ hid = info->hardware_id.string;
+ if ((hid == NULL) || strcmp(hid, ACPI_PROCESSOR_DEVICE_HID)) {
+ kfree(info);
+ return AE_ERROR;
+ }
+
+ kfree(info);
+ return AE_OK; /* found a processor device object */
+}
+
static acpi_status
processor_walk_namespace_cb(acpi_handle handle,
u32 lvl, void *context, void **rv)
{
acpi_status status;
int *action = context;
- acpi_object_type type = 0;
- status = acpi_get_type(handle, &type);
+ status = is_processor_device(handle);
if (ACPI_FAILURE(status))
- return (AE_OK);
-
- if (type != ACPI_TYPE_PROCESSOR)
- return (AE_OK);
+ return AE_OK; /* not a processor; continue to walk */
switch (*action) {
case INSTALL_NOTIFY_HANDLER:
@@ -772,7 +799,8 @@ processor_walk_namespace_cb(acpi_handle handle,
break;
}
- return (AE_OK);
+ /* found a processor; skip walking underneath */
+ return AE_CTRL_DEPTH;
}
static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
@@ -830,7 +858,7 @@ void acpi_processor_install_hotplug_notify(void)
{
#ifdef CONFIG_ACPI_HOTPLUG_CPU
int action = INSTALL_NOTIFY_HANDLER;
- acpi_walk_namespace(ACPI_TYPE_PROCESSOR,
+ acpi_walk_namespace(ACPI_TYPE_ANY,
ACPI_ROOT_OBJECT,
ACPI_UINT32_MAX,
processor_walk_namespace_cb, NULL, &action, NULL);
@@ -843,7 +871,7 @@ void acpi_processor_uninstall_hotplug_notify(void)
{
#ifdef CONFIG_ACPI_HOTPLUG_CPU
int action = UNINSTALL_NOTIFY_HANDLER;
- acpi_walk_namespace(ACPI_TYPE_PROCESSOR,
+ acpi_walk_namespace(ACPI_TYPE_ANY,
ACPI_ROOT_OBJECT,
ACPI_UINT32_MAX,
processor_walk_namespace_cb, NULL, &action, NULL);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ACPI: Add CPU hotplug support for processor device objects
2012-03-19 19:08 [PATCH] ACPI: Add CPU hotplug support for processor device objects Toshi Kani
@ 2012-03-20 16:55 ` Bjorn Helgaas
2012-03-20 18:10 ` Toshi Kani
0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2012-03-20 16:55 UTC (permalink / raw)
To: Toshi Kani; +Cc: lenb, linux-acpi
On Mon, Mar 19, 2012 at 1:08 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> acpi_processor_install_hotplug_notify() registers processor objects to
> receive ACPI CPU hotplug event notifications. This patch additionally
> registers processor device objects (ACPI0007) to receive the notifications
> as well.
>
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> ---
> drivers/acpi/processor_driver.c | 48 ++++++++++++++++++++++++++++++--------
> 1 files changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 8ae05ce..50be277 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -68,6 +68,7 @@
> #define ACPI_PROCESSOR_NOTIFY_PERFORMANCE 0x80
> #define ACPI_PROCESSOR_NOTIFY_POWER 0x81
> #define ACPI_PROCESSOR_NOTIFY_THROTTLING 0x82
> +#define ACPI_PROCESSOR_DEVICE_HID "ACPI0007"
>
> #define ACPI_PROCESSOR_LIMIT_USER 0
> #define ACPI_PROCESSOR_LIMIT_THERMAL 1
> @@ -88,7 +89,7 @@ static int acpi_processor_start(struct acpi_processor *pr);
>
> static const struct acpi_device_id processor_device_ids[] = {
> {ACPI_PROCESSOR_OBJECT_HID, 0},
> - {"ACPI0007", 0},
> + {ACPI_PROCESSOR_DEVICE_HID, 0},
> {"", 0},
> };
> MODULE_DEVICE_TABLE(acpi, processor_device_ids);
> @@ -741,20 +742,46 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
> return;
> }
>
> +static acpi_status is_processor_device(acpi_handle handle)
> +{
> + struct acpi_device_info *info;
> + char *hid;
> + acpi_status status;
> +
> + status = acpi_get_object_info(handle, &info);
> + if (ACPI_FAILURE(status))
> + return status;
> +
> + if (info->type == ACPI_TYPE_PROCESSOR) {
> + kfree(info);
> + return AE_OK; /* found a processor object */
> + }
> +
> + if (!(info->valid & ACPI_VALID_HID)) {
> + kfree(info);
> + return AE_ERROR;
> + }
> +
> + hid = info->hardware_id.string;
> + if ((hid == NULL) || strcmp(hid, ACPI_PROCESSOR_DEVICE_HID)) {
> + kfree(info);
> + return AE_ERROR;
It's common to combine cleanup paths using goto and labels so things
like the kfree() don't have to be repeated. In this case, it's so
simple that I'm not sure there's much to be gained, so I think it's
fine either way.
> + }
> +
> + kfree(info);
> + return AE_OK; /* found a processor device object */
> +}
> +
> static acpi_status
> processor_walk_namespace_cb(acpi_handle handle,
> u32 lvl, void *context, void **rv)
> {
> acpi_status status;
> int *action = context;
> - acpi_object_type type = 0;
>
> - status = acpi_get_type(handle, &type);
> + status = is_processor_device(handle);
> if (ACPI_FAILURE(status))
This patch looks great to me. The only trivial thing I'd change is to
make "is_processor_device()" a bool, since that's what the name
suggests, and it would make this code slightly simpler.
Other than that:
Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
> - return (AE_OK);
> -
> - if (type != ACPI_TYPE_PROCESSOR)
> - return (AE_OK);
> + return AE_OK; /* not a processor; continue to walk */
>
> switch (*action) {
> case INSTALL_NOTIFY_HANDLER:
> @@ -772,7 +799,8 @@ processor_walk_namespace_cb(acpi_handle handle,
> break;
> }
>
> - return (AE_OK);
> + /* found a processor; skip walking underneath */
> + return AE_CTRL_DEPTH;
> }
>
> static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
> @@ -830,7 +858,7 @@ void acpi_processor_install_hotplug_notify(void)
> {
> #ifdef CONFIG_ACPI_HOTPLUG_CPU
> int action = INSTALL_NOTIFY_HANDLER;
> - acpi_walk_namespace(ACPI_TYPE_PROCESSOR,
> + acpi_walk_namespace(ACPI_TYPE_ANY,
> ACPI_ROOT_OBJECT,
> ACPI_UINT32_MAX,
> processor_walk_namespace_cb, NULL, &action, NULL);
> @@ -843,7 +871,7 @@ void acpi_processor_uninstall_hotplug_notify(void)
> {
> #ifdef CONFIG_ACPI_HOTPLUG_CPU
> int action = UNINSTALL_NOTIFY_HANDLER;
> - acpi_walk_namespace(ACPI_TYPE_PROCESSOR,
> + acpi_walk_namespace(ACPI_TYPE_ANY,
> ACPI_ROOT_OBJECT,
> ACPI_UINT32_MAX,
> processor_walk_namespace_cb, NULL, &action, NULL);
> --
> 1.7.7.6
>
> --
> 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
--
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ACPI: Add CPU hotplug support for processor device objects
2012-03-20 16:55 ` Bjorn Helgaas
@ 2012-03-20 18:10 ` Toshi Kani
0 siblings, 0 replies; 3+ messages in thread
From: Toshi Kani @ 2012-03-20 18:10 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: lenb, linux-acpi
Hi Bjorn,
On Tue, 2012-03-20 at 10:55 -0600, Bjorn Helgaas wrote:
> On Mon, Mar 19, 2012 at 1:08 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > acpi_processor_install_hotplug_notify() registers processor objects to
> > receive ACPI CPU hotplug event notifications. This patch additionally
> > registers processor device objects (ACPI0007) to receive the notifications
> > as well.
> >
> > Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> > ---
> > drivers/acpi/processor_driver.c | 48 ++++++++++++++++++++++++++++++--------
> > 1 files changed, 38 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> > index 8ae05ce..50be277 100644
> > --- a/drivers/acpi/processor_driver.c
> > +++ b/drivers/acpi/processor_driver.c
> > @@ -68,6 +68,7 @@
> > #define ACPI_PROCESSOR_NOTIFY_PERFORMANCE 0x80
> > #define ACPI_PROCESSOR_NOTIFY_POWER 0x81
> > #define ACPI_PROCESSOR_NOTIFY_THROTTLING 0x82
> > +#define ACPI_PROCESSOR_DEVICE_HID "ACPI0007"
> >
> > #define ACPI_PROCESSOR_LIMIT_USER 0
> > #define ACPI_PROCESSOR_LIMIT_THERMAL 1
> > @@ -88,7 +89,7 @@ static int acpi_processor_start(struct acpi_processor *pr);
> >
> > static const struct acpi_device_id processor_device_ids[] = {
> > {ACPI_PROCESSOR_OBJECT_HID, 0},
> > - {"ACPI0007", 0},
> > + {ACPI_PROCESSOR_DEVICE_HID, 0},
> > {"", 0},
> > };
> > MODULE_DEVICE_TABLE(acpi, processor_device_ids);
> > @@ -741,20 +742,46 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
> > return;
> > }
> >
> > +static acpi_status is_processor_device(acpi_handle handle)
> > +{
> > + struct acpi_device_info *info;
> > + char *hid;
> > + acpi_status status;
> > +
> > + status = acpi_get_object_info(handle, &info);
> > + if (ACPI_FAILURE(status))
> > + return status;
> > +
> > + if (info->type == ACPI_TYPE_PROCESSOR) {
> > + kfree(info);
> > + return AE_OK; /* found a processor object */
> > + }
> > +
> > + if (!(info->valid & ACPI_VALID_HID)) {
> > + kfree(info);
> > + return AE_ERROR;
> > + }
> > +
> > + hid = info->hardware_id.string;
> > + if ((hid == NULL) || strcmp(hid, ACPI_PROCESSOR_DEVICE_HID)) {
> > + kfree(info);
> > + return AE_ERROR;
>
> It's common to combine cleanup paths using goto and labels so things
> like the kfree() don't have to be repeated. In this case, it's so
> simple that I'm not sure there's much to be gained, so I think it's
> fine either way.
I will change it to combine the cleanup paths.
> > + }
> > +
> > + kfree(info);
> > + return AE_OK; /* found a processor device object */
> > +}
> > +
> > static acpi_status
> > processor_walk_namespace_cb(acpi_handle handle,
> > u32 lvl, void *context, void **rv)
> > {
> > acpi_status status;
> > int *action = context;
> > - acpi_object_type type = 0;
> >
> > - status = acpi_get_type(handle, &type);
> > + status = is_processor_device(handle);
> > if (ACPI_FAILURE(status))
>
> This patch looks great to me. The only trivial thing I'd change is to
> make "is_processor_device()" a bool, since that's what the name
> suggests, and it would make this code slightly simpler.
Good point. I will make that change as well.
> Other than that:
>
> Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
Great. Thanks a lot!
-Toshi
> > - return (AE_OK);
> > -
> > - if (type != ACPI_TYPE_PROCESSOR)
> > - return (AE_OK);
> > + return AE_OK; /* not a processor; continue to walk */
> >
> > switch (*action) {
> > case INSTALL_NOTIFY_HANDLER:
> > @@ -772,7 +799,8 @@ processor_walk_namespace_cb(acpi_handle handle,
> > break;
> > }
> >
> > - return (AE_OK);
> > + /* found a processor; skip walking underneath */
> > + return AE_CTRL_DEPTH;
> > }
> >
> > static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
> > @@ -830,7 +858,7 @@ void acpi_processor_install_hotplug_notify(void)
> > {
> > #ifdef CONFIG_ACPI_HOTPLUG_CPU
> > int action = INSTALL_NOTIFY_HANDLER;
> > - acpi_walk_namespace(ACPI_TYPE_PROCESSOR,
> > + acpi_walk_namespace(ACPI_TYPE_ANY,
> > ACPI_ROOT_OBJECT,
> > ACPI_UINT32_MAX,
> > processor_walk_namespace_cb, NULL, &action, NULL);
> > @@ -843,7 +871,7 @@ void acpi_processor_uninstall_hotplug_notify(void)
> > {
> > #ifdef CONFIG_ACPI_HOTPLUG_CPU
> > int action = UNINSTALL_NOTIFY_HANDLER;
> > - acpi_walk_namespace(ACPI_TYPE_PROCESSOR,
> > + acpi_walk_namespace(ACPI_TYPE_ANY,
> > ACPI_ROOT_OBJECT,
> > ACPI_UINT32_MAX,
> > processor_walk_namespace_cb, NULL, &action, NULL);
> > --
> > 1.7.7.6
> >
> > --
> > 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
> --
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-03-20 18:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-19 19:08 [PATCH] ACPI: Add CPU hotplug support for processor device objects Toshi Kani
2012-03-20 16:55 ` Bjorn Helgaas
2012-03-20 18:10 ` Toshi Kani
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox