Miscellaneous improvements for ACPI

RFC PATCH In review Jiang Liu 2 participants 15 messages Feb 17, 2014 – Feb 19, 2014

Subsystems (3)

ACPI Xen hypervisor interface ACPI component architecture (acpica)

lore

Share & tools

lore

Jiang Liu

author

2014-02-17 05:00 UTC #0 lore raw

cover Miscellaneous improvements for ACPI

Hi,
This is a miscellaneous patch set to improve ACPI code. due to
resource limitation, I can't find hardware to test all patches. So appreciate any help to these patches.

Thanks!
Gerry

Jiang Liu (6):
acpi_pad: simplify notify handler by using acpi_drv_ops->notify
ACPI: rename acpi_evaluate_hotplug_ost to acpi_evaluate_ost for reuse
acpi_pad: use acpi_evaluate_pad() to replace open-coded version xen, acpi_pad: use acpi_evaluate_pad() to replace open-coded version
acpi_processor: use acpi_evaluate_pad() to replace open-coded version
ACPI: use device name LNXSYBUS.xx for ACPI \_SB and \_TZ objects

 drivers/acpi/acpi_pad.c          |   49 ++++++++------------------------------
 drivers/acpi/processor_perflib.c |   14 +++--------
 drivers/acpi/scan.c              |   20 ++++++++++++++--
 drivers/acpi/utils.c             |   14 ++++-------
 drivers/xen/xen-acpi-pad.c       |   26 ++++++--------------
 include/acpi/acpi_bus.h          |   10 +++++++-
 6 files changed, 51 insertions(+), 82 deletions(-)

Signature · 1 line

1.7.10.4

Jiang Liu

author

2014-02-17 05:02 UTC #1 lore raw

RFC PATCH 1/6 acpi_pad: simplify notify handler by using acpi_drv_ops->notify

Use callback acpi_drv_ops->notify to simplify acpi_pad driver's notification handling code.

Signed-off-by Jiang Liu <[email protected]>

---
 drivers/acpi/acpi_pad.c |   25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

drivers/acpi/acpi_pad.c +5 ~~-20~~

index df96a0f..1e3ce76 100644
@@ -443,14 +443,11 @@ static void acpi_pad_handle_notify(acpi_handle handle)
 	mutex_unlock(&isolated_cpus_lock);
 }
 
-static void acpi_pad_notify(acpi_handle handle, u32 event,
-	void *data)
+static void acpi_pad_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_device *device = data;
-
 	switch (event) {
 	case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
-		acpi_pad_handle_notify(handle);
+		acpi_pad_handle_notify(device->handle);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
 			dev_name(&device->dev), event, 0);
 		break;

@@ -462,22 +459,10 @@ static void acpi_pad_notify(acpi_handle handle, u32 event,
 
 static int acpi_pad_add(struct acpi_device *device)
 {
-	acpi_status status;
-
 	strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
 
-	if (acpi_pad_add_sysfs(device))
-		return -ENODEV;
-
-	status = acpi_install_notify_handler(device->handle,
-		ACPI_DEVICE_NOTIFY, acpi_pad_notify, device);
-	if (ACPI_FAILURE(status)) {
-		acpi_pad_remove_sysfs(device);
-		return -ENODEV;
-	}
-
-	return 0;
+	return acpi_pad_add_sysfs(device);
 }
 
 static int acpi_pad_remove(struct acpi_device *device)

@@ -486,9 +471,8 @@ static int acpi_pad_remove(struct acpi_device *device)
 	acpi_pad_idle_cpus(0);
 	mutex_unlock(&isolated_cpus_lock);
 
-	acpi_remove_notify_handler(device->handle,
-		ACPI_DEVICE_NOTIFY, acpi_pad_notify);
 	acpi_pad_remove_sysfs(device);
+
 	return 0;
 }

@@ -505,6 +489,7 @@ static struct acpi_driver acpi_pad_driver = {
 	.ops = {
 		.add = acpi_pad_add,
 		.remove = acpi_pad_remove,
+		.notify = acpi_pad_notify,
 	},
 };

Signature · 1 line

1.7.10.4

Jiang Liu

author

2014-02-17 05:02 UTC #2 lore raw

RFC PATCH 2/6 ACPI: rename acpi_evaluate_hotplug_ost to acpi_evaluate_ost for reuse

Rename acpi_evaluate_hotplug_ost() to acpi_evaluate_ost() for later resue.

Signed-off-by Jiang Liu <[email protected]>

---
 drivers/acpi/utils.c    |   14 ++++----------
 include/acpi/acpi_bus.h |   10 +++++++++-
 2 files changed, 13 insertions(+), 11 deletions(-)

drivers/acpi/utils.c +4 ~~-10~~

index 85e3b61..19d0169 100644
@@ -433,17 +433,15 @@ EXPORT_SYMBOL(acpi_get_physical_device_location);
  * When the platform does not support _OST, this function has no effect.
  */
 acpi_status
-acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
-		u32 status_code, struct acpi_buffer *status_buf)
+acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
+		  struct acpi_buffer *status_buf)
 {
-#ifdef ACPI_HOTPLUG_OST
 	union acpi_object params[3] = {
 		{.type = ACPI_TYPE_INTEGER,},
 		{.type = ACPI_TYPE_INTEGER,},
 		{.type = ACPI_TYPE_BUFFER,}
 	};
 	struct acpi_object_list arg_list = {3, params};
-	acpi_status status;
 
 	params[0].integer.value = source_event;
 	params[1].integer.value = status_code;

@@ -455,13 +453,9 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
 		params[2].buffer.length = 0;
 	}
 
-	status = acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-	return status;
-#else
-	return AE_OK;
-#endif
+	return acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
 }
-EXPORT_SYMBOL(acpi_evaluate_hotplug_ost);
+EXPORT_SYMBOL(acpi_evaluate_ost);
 
 /**
  * acpi_handle_printk: Print message with ACPI prefix and object path

include/acpi/acpi_bus.h +9 -1

index 8256eb4..adef502 100644
@@ -49,8 +49,16 @@ acpi_evaluate_reference(acpi_handle handle,
 			struct acpi_object_list *arguments,
 			struct acpi_handle_list *list);
 acpi_status
+acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
+		  struct acpi_buffer *status_buf);
+#ifdef ACPI_HOTPLUG_OST
+#define	acpi_evaluate_hotplug_ost	acpi_evaluate_ost
+#else
+static inline acpi_status
 acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
-			u32 status_code, struct acpi_buffer *status_buf);
+			u32 status_code, struct acpi_buffer *status_buf)
+{ return AE_OK; }
+#endif
 
 acpi_status
 acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld);

Signature · 1 line

1.7.10.4

Rafael J. Wysocki

2014-02-18 00:52 UTC #3 lore raw

Re: [RFC Patch v1 2/6] ACPI: rename acpi_evaluate_hotplug_ost to acpi_evaluate_ost for reuse

Quoted 13 lines · Jiang Liu

Rename acpi_evaluate_hotplug_ost() to acpi_evaluate_ost() for later resue.
Signed-off-by Jiang Liu <[email protected]>
---
 drivers/acpi/utils.c    |   14 ++++----------
 include/acpi/acpi_bus.h |   10 +++++++++-
 2 files changed, 13 insertions(+), 11 deletions(-)
drivers/acpi/utils.c +0 -0
index 85e3b61..19d0169 100644

drivers/acpi/utils.c EXPORT_SYMBOL(acpi_get_physical_device_location); · line 437

Quoted 5 lines

drivers/acpi/utils.c +0 -2

@@ -433,17 +433,15 @@ EXPORT_SYMBOL(acpi_get_physical_device_location);
  * When the platform does not support _OST, this function has no effect.
  */
 acpi_status
-acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
-		u32 status_code, struct acpi_buffer *status_buf)

There are users of this, what about updating them?

Quoted 53 lines

drivers/acpi/utils.c +4 -8

+acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
+		  struct acpi_buffer *status_buf)
 {
-#ifdef ACPI_HOTPLUG_OST
 	union acpi_object params[3] = {
 		{.type = ACPI_TYPE_INTEGER,},
 		{.type = ACPI_TYPE_INTEGER,},
 		{.type = ACPI_TYPE_BUFFER,}
 	};
 	struct acpi_object_list arg_list = {3, params};
-	acpi_status status;
 
 	params[0].integer.value = source_event;
 	params[1].integer.value = status_code;

@@ -455,13 +453,9 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
 		params[2].buffer.length = 0;
 	}
 
-	status = acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-	return status;
-#else
-	return AE_OK;
-#endif
+	return acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
 }
-EXPORT_SYMBOL(acpi_evaluate_hotplug_ost);
+EXPORT_SYMBOL(acpi_evaluate_ost);
 
 /**
  * acpi_handle_printk: Print message with ACPI prefix and object path

include/acpi/acpi_bus.h +9 -1

index 8256eb4..adef502 100644
@@ -49,8 +49,16 @@ acpi_evaluate_reference(acpi_handle handle,
 			struct acpi_object_list *arguments,
 			struct acpi_handle_list *list);
 acpi_status
+acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
+		  struct acpi_buffer *status_buf);
+#ifdef ACPI_HOTPLUG_OST
+#define	acpi_evaluate_hotplug_ost	acpi_evaluate_ost
+#else
+static inline acpi_status
 acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
-			u32 status_code, struct acpi_buffer *status_buf);
+			u32 status_code, struct acpi_buffer *status_buf)
+{ return AE_OK; }
+#endif
 
 acpi_status
 acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld);

Signature · 2 lines

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

Jiang Liu

author

2014-02-19 05:47 UTC #4 lore raw

Re: [RFC Patch v1 2/6] ACPI: rename acpi_evaluate_hotplug_ost to acpi_evaluate_ost for reuse

Hi Rafael,
We have following code to deal with existing users.
+#ifdef ACPI_HOTPLUG_OST
+#define acpi_evaluate_hotplug_ost acpi_evaluate_ost
+#else
+static inline acpi_status
acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
- u32 status_code, struct acpi_buffer *status_buf);
+ u32 status_code, struct acpi_buffer *status_buf)
+{ return AE_OK; }
+#endif

Thanks!

Quoted 76 lines · Rafael J. Wysocki

Quoted 13 lines · Jiang Liu

Rename acpi_evaluate_hotplug_ost() to acpi_evaluate_ost() for later resue.
Signed-off-by Jiang Liu <[email protected]>
---
 drivers/acpi/utils.c    |   14 ++++----------
 include/acpi/acpi_bus.h |   10 +++++++++-
 2 files changed, 13 insertions(+), 11 deletions(-)
drivers/acpi/utils.c +0 -0
index 85e3b61..19d0169 100644

drivers/acpi/utils.c EXPORT_SYMBOL(acpi_get_physical_device_location); · line 437

Quoted 5 lines

drivers/acpi/utils.c +0 -2

@@ -433,17 +433,15 @@ EXPORT_SYMBOL(acpi_get_physical_device_location);
  * When the platform does not support _OST, this function has no effect.
  */
 acpi_status
-acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
-		u32 status_code, struct acpi_buffer *status_buf)

There are users of this, what about updating them?

Quoted 53 lines

drivers/acpi/utils.c +4 -8

+acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
+		  struct acpi_buffer *status_buf)
 {
-#ifdef ACPI_HOTPLUG_OST
 	union acpi_object params[3] = {
 		{.type = ACPI_TYPE_INTEGER,},
 		{.type = ACPI_TYPE_INTEGER,},
 		{.type = ACPI_TYPE_BUFFER,}
 	};
 	struct acpi_object_list arg_list = {3, params};
-	acpi_status status;
 
 	params[0].integer.value = source_event;
 	params[1].integer.value = status_code;

@@ -455,13 +453,9 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
 		params[2].buffer.length = 0;
 	}
 
-	status = acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-	return status;
-#else
-	return AE_OK;
-#endif
+	return acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
 }
-EXPORT_SYMBOL(acpi_evaluate_hotplug_ost);
+EXPORT_SYMBOL(acpi_evaluate_ost);
 
 /**
  * acpi_handle_printk: Print message with ACPI prefix and object path

include/acpi/acpi_bus.h +9 -1

index 8256eb4..adef502 100644
@@ -49,8 +49,16 @@ acpi_evaluate_reference(acpi_handle handle,
 			struct acpi_object_list *arguments,
 			struct acpi_handle_list *list);
 acpi_status
+acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
+		  struct acpi_buffer *status_buf);
+#ifdef ACPI_HOTPLUG_OST
+#define	acpi_evaluate_hotplug_ost	acpi_evaluate_ost
+#else
+static inline acpi_status
 acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
-			u32 status_code, struct acpi_buffer *status_buf);
+			u32 status_code, struct acpi_buffer *status_buf)
+{ return AE_OK; }
+#endif
 
 acpi_status
 acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld);

Rafael J. Wysocki

2014-02-19 15:38 UTC #5 lore raw

Re: [RFC Patch v1 2/6] ACPI: rename acpi_evaluate_hotplug_ost to acpi_evaluate_ost for reuse

include/acpi/acpi_bus.h acpi_evaluate_reference(acpi_handle handle, · line 55

Quoted 4 lines · Jiang Liu

Hi Rafael,
We have following code to deal with existing users.
include/acpi/acpi_bus.h +2 -0
+#ifdef ACPI_HOTPLUG_OST
+#define	acpi_evaluate_hotplug_ost	acpi_evaluate_ost

Well, I'd very much prefer the users to be updated.

Quoted 92 lines

include/acpi/acpi_bus.h +5 -1

+#else
+static inline acpi_status
 acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
-			u32 status_code, struct acpi_buffer *status_buf);
+			u32 status_code, struct acpi_buffer *status_buf)
+{ return AE_OK; }
+#endif

Thanks!

Quoted 72 lines · Rafael J. Wysocki

Quoted 13 lines · Jiang Liu

Rename acpi_evaluate_hotplug_ost() to acpi_evaluate_ost() for later resue.
Signed-off-by Jiang Liu <[email protected]>
---
 drivers/acpi/utils.c    |   14 ++++----------
 include/acpi/acpi_bus.h |   10 +++++++++-
 2 files changed, 13 insertions(+), 11 deletions(-)
drivers/acpi/utils.c +0 -0
index 85e3b61..19d0169 100644

drivers/acpi/utils.c EXPORT_SYMBOL(acpi_get_physical_device_location); · line 437

Quoted 5 lines

drivers/acpi/utils.c +0 -2

@@ -433,17 +433,15 @@ EXPORT_SYMBOL(acpi_get_physical_device_location);
  * When the platform does not support _OST, this function has no effect.
  */
 acpi_status
-acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
-		u32 status_code, struct acpi_buffer *status_buf)

There are users of this, what about updating them?

Quoted 50 lines

drivers/acpi/utils.c +4 -8

+acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
+		  struct acpi_buffer *status_buf)
 {
-#ifdef ACPI_HOTPLUG_OST
 	union acpi_object params[3] = {
 		{.type = ACPI_TYPE_INTEGER,},
 		{.type = ACPI_TYPE_INTEGER,},
 		{.type = ACPI_TYPE_BUFFER,}
 	};
 	struct acpi_object_list arg_list = {3, params};
-	acpi_status status;
 
 	params[0].integer.value = source_event;
 	params[1].integer.value = status_code;

@@ -455,13 +453,9 @@ acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
 		params[2].buffer.length = 0;
 	}
 
-	status = acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-	return status;
-#else
-	return AE_OK;
-#endif
+	return acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
 }
-EXPORT_SYMBOL(acpi_evaluate_hotplug_ost);
+EXPORT_SYMBOL(acpi_evaluate_ost);
 
 /**
  * acpi_handle_printk: Print message with ACPI prefix and object path

include/acpi/acpi_bus.h +9 -1

index 8256eb4..adef502 100644
@@ -49,8 +49,16 @@ acpi_evaluate_reference(acpi_handle handle,
 			struct acpi_object_list *arguments,
 			struct acpi_handle_list *list);
 acpi_status
+acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code,
+		  struct acpi_buffer *status_buf);
+#ifdef ACPI_HOTPLUG_OST
+#define	acpi_evaluate_hotplug_ost	acpi_evaluate_ost
+#else
+static inline acpi_status
 acpi_evaluate_hotplug_ost(acpi_handle handle, u32 source_event,
-			u32 status_code, struct acpi_buffer *status_buf);
+			u32 status_code, struct acpi_buffer *status_buf)
+{ return AE_OK; }
+#endif

Quoted 4 lines

include/acpi/acpi_bus.h acpi_evaluate_reference(acpi_handle handle, · line 64
Quoted 3 lines
include/acpi/acpi_bus.h +0 -0
 acpi_status
 acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines

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

Jiang Liu

author

2014-02-17 05:02 UTC #6 lore raw

RFC PATCH 3/6 acpi_pad: use acpi_evaluate_pad() to replace open-coded version

In reply to Jiang Liu

Use public function acpi_evaluate_pad() to replace open-coded version of evaluating ACPI _OST method.

Signed-off-by Jiang Liu <[email protected]>

---
 drivers/acpi/acpi_pad.c |   24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

drivers/acpi/acpi_pad.c +5 ~~-19~~

index 1e3ce76..86b4e11 100644
@@ -408,28 +408,14 @@ static int acpi_pad_pur(acpi_handle handle)
 	return num;
 }
 
-/* Notify firmware how many CPUs are idle */
-static void acpi_pad_ost(acpi_handle handle, int stat,
-	uint32_t idle_cpus)
-{
-	union acpi_object params[3] = {
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_BUFFER,},
-	};
-	struct acpi_object_list arg_list = {3, params};
-
-	params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
-	params[1].integer.value =  stat;
-	params[2].buffer.length = 4;
-	params[2].buffer.pointer = (void *)&idle_cpus;
-	acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-}
-
 static void acpi_pad_handle_notify(acpi_handle handle)
 {
 	int num_cpus;
 	uint32_t idle_cpus;
+	struct acpi_buffer param = {
+		.length = 4,
+		.pointer = (void *)&idle_cpus,
+	};
 
 	mutex_lock(&isolated_cpus_lock);
 	num_cpus = acpi_pad_pur(handle);

@@ -439,7 +425,7 @@ static void acpi_pad_handle_notify(acpi_handle handle)
 	}
 	acpi_pad_idle_cpus(num_cpus);
 	idle_cpus = acpi_pad_idle_cpus_num();
-	acpi_pad_ost(handle, 0, idle_cpus);
+	acpi_evaluate_ost(handle, ACPI_PROCESSOR_AGGREGATOR_NOTIFY, 0, &param);
 	mutex_unlock(&isolated_cpus_lock);
 }

Signature · 1 line

1.7.10.4

Rafael J. Wysocki

2014-02-18 00:52 UTC #7 lore raw

Re: [RFC Patch v1 3/6] acpi_pad: use acpi_evaluate_pad() to replace open-coded version

Quoted 1 line · Jiang Liu

Use public function acpi_evaluate_pad() to replace open-coded

I suppose you mean acpi_evaluate_ost()? And in the other patches in this series too?

Quoted 54 lines

version of evaluating ACPI _OST method.

Signed-off-by Jiang Liu <[email protected]>

---
 drivers/acpi/acpi_pad.c |   24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

drivers/acpi/acpi_pad.c +5 ~~-19~~

index 1e3ce76..86b4e11 100644
@@ -408,28 +408,14 @@ static int acpi_pad_pur(acpi_handle handle)
 	return num;
 }
 
-/* Notify firmware how many CPUs are idle */
-static void acpi_pad_ost(acpi_handle handle, int stat,
-	uint32_t idle_cpus)
-{
-	union acpi_object params[3] = {
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_BUFFER,},
-	};
-	struct acpi_object_list arg_list = {3, params};
-
-	params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
-	params[1].integer.value =  stat;
-	params[2].buffer.length = 4;
-	params[2].buffer.pointer = (void *)&idle_cpus;
-	acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-}
-
 static void acpi_pad_handle_notify(acpi_handle handle)
 {
 	int num_cpus;
 	uint32_t idle_cpus;
+	struct acpi_buffer param = {
+		.length = 4,
+		.pointer = (void *)&idle_cpus,
+	};
 
 	mutex_lock(&isolated_cpus_lock);
 	num_cpus = acpi_pad_pur(handle);

@@ -439,7 +425,7 @@ static void acpi_pad_handle_notify(acpi_handle handle)
 	}
 	acpi_pad_idle_cpus(num_cpus);
 	idle_cpus = acpi_pad_idle_cpus_num();
-	acpi_pad_ost(handle, 0, idle_cpus);
+	acpi_evaluate_ost(handle, ACPI_PROCESSOR_AGGREGATOR_NOTIFY, 0, &param);
 	mutex_unlock(&isolated_cpus_lock);
 }

Signature · 2 lines

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

Jiang Liu

author

2014-02-19 05:58 UTC #8 lore raw

Re: [RFC Patch v1 3/6] acpi_pad: use acpi_evaluate_pad() to replace open-coded version

Hi Rafael,
Thanks for reminder, will fix it in next version.

Cheers!
Gerry

Quoted 61 lines · Rafael J. Wysocki

Quoted 1 line · Jiang Liu

Use public function acpi_evaluate_pad() to replace open-coded

I suppose you mean acpi_evaluate_ost()? And in the other patches in this series too?

Quoted 54 lines

version of evaluating ACPI _OST method.

Signed-off-by Jiang Liu <[email protected]>

---
 drivers/acpi/acpi_pad.c |   24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

drivers/acpi/acpi_pad.c +5 ~~-19~~

index 1e3ce76..86b4e11 100644
@@ -408,28 +408,14 @@ static int acpi_pad_pur(acpi_handle handle)
 	return num;
 }
 
-/* Notify firmware how many CPUs are idle */
-static void acpi_pad_ost(acpi_handle handle, int stat,
-	uint32_t idle_cpus)
-{
-	union acpi_object params[3] = {
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_BUFFER,},
-	};
-	struct acpi_object_list arg_list = {3, params};
-
-	params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
-	params[1].integer.value =  stat;
-	params[2].buffer.length = 4;
-	params[2].buffer.pointer = (void *)&idle_cpus;
-	acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-}
-
 static void acpi_pad_handle_notify(acpi_handle handle)
 {
 	int num_cpus;
 	uint32_t idle_cpus;
+	struct acpi_buffer param = {
+		.length = 4,
+		.pointer = (void *)&idle_cpus,
+	};
 
 	mutex_lock(&isolated_cpus_lock);
 	num_cpus = acpi_pad_pur(handle);

@@ -439,7 +425,7 @@ static void acpi_pad_handle_notify(acpi_handle handle)
 	}
 	acpi_pad_idle_cpus(num_cpus);
 	idle_cpus = acpi_pad_idle_cpus_num();
-	acpi_pad_ost(handle, 0, idle_cpus);
+	acpi_evaluate_ost(handle, ACPI_PROCESSOR_AGGREGATOR_NOTIFY, 0, &param);
 	mutex_unlock(&isolated_cpus_lock);
 }

Jiang Liu

author

2014-02-17 05:02 UTC #9 lore raw

RFC PATCH 4/6 xen, acpi_pad: use acpi_evaluate_pad() to replace open-coded version

In reply to Jiang Liu

Use public function acpi_evaluate_pad() to replace open-coded version of evaluating ACPI _OST method.

Signed-off-by Jiang Liu <[email protected]>

---
 drivers/xen/xen-acpi-pad.c |   26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

drivers/xen/xen-acpi-pad.c +7 ~~-19~~

index 40c4bc0..f83b754 100644
@@ -77,27 +77,14 @@ static int acpi_pad_pur(acpi_handle handle)
 	return num;
 }
 
-/* Notify firmware how many CPUs are idle */
-static void acpi_pad_ost(acpi_handle handle, int stat,
-	uint32_t idle_nums)
-{
-	union acpi_object params[3] = {
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_BUFFER,},
-	};
-	struct acpi_object_list arg_list = {3, params};
-
-	params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
-	params[1].integer.value =  stat;
-	params[2].buffer.length = 4;
-	params[2].buffer.pointer = (void *)&idle_nums;
-	acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-}
-
 static void acpi_pad_handle_notify(acpi_handle handle)
 {
 	int idle_nums;
+	struct acpi_buffer param = {
+		.length = 4,
+		.pointer = (void *)&idle_nums,
+	};
+
 
 	mutex_lock(&xen_cpu_lock);
 	idle_nums = acpi_pad_pur(handle);

@@ -109,7 +96,8 @@ static void acpi_pad_handle_notify(acpi_handle handle)
 	idle_nums = xen_acpi_pad_idle_cpus(idle_nums)
 		    ?: xen_acpi_pad_idle_cpus_num();
 	if (idle_nums >= 0)
-		acpi_pad_ost(handle, 0, idle_nums);
+		acpi_evaluate_ost(handle, ACPI_PROCESSOR_AGGREGATOR_NOTIFY,
+				  0, &param);
 	mutex_unlock(&xen_cpu_lock);
 }

Signature · 1 line

1.7.10.4

Jiang Liu

author

2014-02-17 05:02 UTC #10 lore raw

RFC PATCH 4/6 xen, acpi_pad: use acpi_evaluate_pad() to replace open-coded version

In reply to Jiang Liu

Use public function acpi_evaluate_pad() to replace open-coded version of evaluating ACPI _OST method.

Signed-off-by Jiang Liu <[email protected]>

---
 drivers/xen/xen-acpi-pad.c |   26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

drivers/xen/xen-acpi-pad.c +7 ~~-19~~

index 40c4bc0..f83b754 100644
@@ -77,27 +77,14 @@ static int acpi_pad_pur(acpi_handle handle)
 	return num;
 }
 
-/* Notify firmware how many CPUs are idle */
-static void acpi_pad_ost(acpi_handle handle, int stat,
-	uint32_t idle_nums)
-{
-	union acpi_object params[3] = {
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_BUFFER,},
-	};
-	struct acpi_object_list arg_list = {3, params};
-
-	params[0].integer.value = ACPI_PROCESSOR_AGGREGATOR_NOTIFY;
-	params[1].integer.value =  stat;
-	params[2].buffer.length = 4;
-	params[2].buffer.pointer = (void *)&idle_nums;
-	acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-}
-
 static void acpi_pad_handle_notify(acpi_handle handle)
 {
 	int idle_nums;
+	struct acpi_buffer param = {
+		.length = 4,
+		.pointer = (void *)&idle_nums,
+	};
+
 
 	mutex_lock(&xen_cpu_lock);
 	idle_nums = acpi_pad_pur(handle);

@@ -109,7 +96,8 @@ static void acpi_pad_handle_notify(acpi_handle handle)
 	idle_nums = xen_acpi_pad_idle_cpus(idle_nums)
 		    ?: xen_acpi_pad_idle_cpus_num();
 	if (idle_nums >= 0)
-		acpi_pad_ost(handle, 0, idle_nums);
+		acpi_evaluate_ost(handle, ACPI_PROCESSOR_AGGREGATOR_NOTIFY,
+				  0, &param);
 	mutex_unlock(&xen_cpu_lock);
 }

Signature · 1 line

1.7.10.4

Jiang Liu

author

2014-02-17 05:02 UTC #11 lore raw

RFC PATCH 5/6 acpi_processor: use acpi_evaluate_pad() to replace open-coded version

In reply to Jiang Liu

Use public function acpi_evaluate_pad() to replace open-coded version of evaluating ACPI _OST method.

Signed-off-by Jiang Liu <[email protected]>

---
 drivers/acpi/processor_perflib.c |   14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

drivers/acpi/processor_perflib.c +3 ~~-11~~

index ff90054..cfc8aba 100644
@@ -156,17 +156,9 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
  */
 static void acpi_processor_ppc_ost(acpi_handle handle, int status)
 {
-	union acpi_object params[2] = {
-		{.type = ACPI_TYPE_INTEGER,},
-		{.type = ACPI_TYPE_INTEGER,},
-	};
-	struct acpi_object_list arg_list = {2, params};
-
-	if (acpi_has_method(handle, "_OST")) {
-		params[0].integer.value = ACPI_PROCESSOR_NOTIFY_PERFORMANCE;
-		params[1].integer.value =  status;
-		acpi_evaluate_object(handle, "_OST", &arg_list, NULL);
-	}
+	if (acpi_has_method(handle, "_OST"))
+		acpi_evaluate_ost(handle, ACPI_PROCESSOR_NOTIFY_PERFORMANCE,
+				  status, NULL);
 }
 
 int acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)

Signature · 1 line

1.7.10.4

Jiang Liu

author

2014-02-17 05:02 UTC #12 lore raw

RFC PATCH 6/6 ACPI: use device name LNXSYBUS.xx for ACPI \_SB and \_TZ objects

In reply to Jiang Liu

Use device name "LNXSYBUS:xx" instead of "device.xx" for ACPI objects \_SB and \_TZ.

BTW, the original check of "handle == ACPI_ROOT_OBJECT" in statment "else if (list_empty(&pnp->ids) && handle == ACPI_ROOT_OBJECT)" is always false because of the code at the begin of that block.

                if (handle == ACPI_ROOT_OBJECT) {
                        acpi_add_id(pnp, ACPI_SYSTEM_HID);
                        break;
                }

Signed-off-by Jiang Liu <[email protected]>

---
 drivers/acpi/scan.c |   20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

drivers/acpi/scan.c +18 -2

index 57b053f..62ec4ba 100644
@@ -1706,6 +1706,20 @@ static bool acpi_ibm_smbus_match(acpi_handle handle)
 	return false;
 }
 
+static bool acpi_object_is_system_bus(acpi_handle handle)
+{
+	acpi_handle tmp;
+
+	if (ACPI_SUCCESS(acpi_get_handle(NULL, "\\_SB", &tmp)) &&
+	    tmp == handle)
+		return true;
+	if (ACPI_SUCCESS(acpi_get_handle(NULL, "\\_TZ", &tmp)) &&
+	    tmp == handle)
+		return true;
+
+	return false;
+}
+
 static void acpi_set_pnp_ids(acpi_handle handle, struct acpi_device_pnp *pnp,
 				int device_type)
 {

@@ -1757,8 +1771,10 @@ static void acpi_set_pnp_ids(acpi_handle handle, struct acpi_device_pnp *pnp,
 			acpi_add_id(pnp, ACPI_DOCK_HID);
 		else if (acpi_ibm_smbus_match(handle))
 			acpi_add_id(pnp, ACPI_SMBUS_IBM_HID);
-		else if (list_empty(&pnp->ids) && handle == ACPI_ROOT_OBJECT) {
-			acpi_add_id(pnp, ACPI_BUS_HID); /* \_SB, LNXSYBUS */
+		else if (list_empty(&pnp->ids) &&
+			 acpi_object_is_system_bus(handle)) {
+			/* \_SB, \_TZ, LNXSYBUS */
+			acpi_add_id(pnp, ACPI_BUS_HID);
 			strcpy(pnp->device_name, ACPI_BUS_DEVICE_NAME);
 			strcpy(pnp->device_class, ACPI_BUS_CLASS);
 		}

Signature · 1 line

1.7.10.4

Rafael J. Wysocki

2014-02-18 00:51 UTC #13 lore raw

Re: [RFC Patch v1 1/6] acpi_pad: simplify notify handler by using acpi_drv_ops->notify

In reply to Jiang Liu

Quoted 4 lines · Jiang Liu

Use callback acpi_drv_ops->notify to simplify acpi_pad driver's notification handling code.
Signed-off-by Jiang Liu <[email protected]>

There is a plan to move away from using struct acpi_driver objects and this change will need to be reverted then. I'm not sure if that's really useful?

Quoted 69 lines

---
 drivers/acpi/acpi_pad.c |   25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

drivers/acpi/acpi_pad.c +5 ~~-20~~

index df96a0f..1e3ce76 100644
@@ -443,14 +443,11 @@ static void acpi_pad_handle_notify(acpi_handle handle)
 	mutex_unlock(&isolated_cpus_lock);
 }
 
-static void acpi_pad_notify(acpi_handle handle, u32 event,
-	void *data)
+static void acpi_pad_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_device *device = data;
-
 	switch (event) {
 	case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
-		acpi_pad_handle_notify(handle);
+		acpi_pad_handle_notify(device->handle);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
 			dev_name(&device->dev), event, 0);
 		break;

@@ -462,22 +459,10 @@ static void acpi_pad_notify(acpi_handle handle, u32 event,
 
 static int acpi_pad_add(struct acpi_device *device)
 {
-	acpi_status status;
-
 	strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
 
-	if (acpi_pad_add_sysfs(device))
-		return -ENODEV;
-
-	status = acpi_install_notify_handler(device->handle,
-		ACPI_DEVICE_NOTIFY, acpi_pad_notify, device);
-	if (ACPI_FAILURE(status)) {
-		acpi_pad_remove_sysfs(device);
-		return -ENODEV;
-	}
-
-	return 0;
+	return acpi_pad_add_sysfs(device);
 }
 
 static int acpi_pad_remove(struct acpi_device *device)

@@ -486,9 +471,8 @@ static int acpi_pad_remove(struct acpi_device *device)
 	acpi_pad_idle_cpus(0);
 	mutex_unlock(&isolated_cpus_lock);
 
-	acpi_remove_notify_handler(device->handle,
-		ACPI_DEVICE_NOTIFY, acpi_pad_notify);
 	acpi_pad_remove_sysfs(device);
+
 	return 0;
 }

@@ -505,6 +489,7 @@ static struct acpi_driver acpi_pad_driver = {
 	.ops = {
 		.add = acpi_pad_add,
 		.remove = acpi_pad_remove,
+		.notify = acpi_pad_notify,
 	},
 };

Signature · 2 lines

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

Jiang Liu

author

2014-02-19 05:45 UTC #14 lore raw

Re: [RFC Patch v1 1/6] acpi_pad: simplify notify handler by using acpi_drv_ops->notify

Thanks, Rafael. I will drop this patch then.

Quoted 80 lines · Rafael J. Wysocki

Quoted 4 lines · Jiang Liu

Use callback acpi_drv_ops->notify to simplify acpi_pad driver's notification handling code.
Signed-off-by Jiang Liu <[email protected]>

There is a plan to move away from using struct acpi_driver objects and this change will need to be reverted then. I'm not sure if that's really useful?

Quoted 69 lines

---
 drivers/acpi/acpi_pad.c |   25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

drivers/acpi/acpi_pad.c +5 ~~-20~~

index df96a0f..1e3ce76 100644
@@ -443,14 +443,11 @@ static void acpi_pad_handle_notify(acpi_handle handle)
 	mutex_unlock(&isolated_cpus_lock);
 }
 
-static void acpi_pad_notify(acpi_handle handle, u32 event,
-	void *data)
+static void acpi_pad_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_device *device = data;
-
 	switch (event) {
 	case ACPI_PROCESSOR_AGGREGATOR_NOTIFY:
-		acpi_pad_handle_notify(handle);
+		acpi_pad_handle_notify(device->handle);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
 			dev_name(&device->dev), event, 0);
 		break;

@@ -462,22 +459,10 @@ static void acpi_pad_notify(acpi_handle handle, u32 event,
 
 static int acpi_pad_add(struct acpi_device *device)
 {
-	acpi_status status;
-
 	strcpy(acpi_device_name(device), ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME);
 	strcpy(acpi_device_class(device), ACPI_PROCESSOR_AGGREGATOR_CLASS);
 
-	if (acpi_pad_add_sysfs(device))
-		return -ENODEV;
-
-	status = acpi_install_notify_handler(device->handle,
-		ACPI_DEVICE_NOTIFY, acpi_pad_notify, device);
-	if (ACPI_FAILURE(status)) {
-		acpi_pad_remove_sysfs(device);
-		return -ENODEV;
-	}
-
-	return 0;
+	return acpi_pad_add_sysfs(device);
 }
 
 static int acpi_pad_remove(struct acpi_device *device)

@@ -486,9 +471,8 @@ static int acpi_pad_remove(struct acpi_device *device)
 	acpi_pad_idle_cpus(0);
 	mutex_unlock(&isolated_cpus_lock);
 
-	acpi_remove_notify_handler(device->handle,
-		ACPI_DEVICE_NOTIFY, acpi_pad_notify);
 	acpi_pad_remove_sysfs(device);
+
 	return 0;
 }

@@ -505,6 +489,7 @@ static struct acpi_driver acpi_pad_driver = {
 	.ops = {
 		.add = acpi_pad_add,
 		.remove = acpi_pad_remove,
+		.notify = acpi_pad_notify,
 	},
 };