On 10/27/2014 05:26 PM, Rafael J. Wysocki wrote:
On Monday, October 27, 2014 08:55:41 AM Guenter Roeck wrote:
Register with kernel power-off handler instead of setting pm_power_off
directly. Register with high priority to reflect that the driver explicitly
overrides existing power-off handlers.
Well, I'm still rather unconvinced that notifiers are particularly suitable for
this purpose.
Specifically ->
Fine.
Cc: Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>
Cc: Len Brown <lenb@xxxxxxxxxx>
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
v3:
- Replace poweroff in all newly introduced variables and in text
with power_off or power-off as appropriate
- Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx
- Replace acpi: with ACPI: in log message
v2:
- Use define to specify poweroff handler priority
- Use pr_warn instead of pr_err
drivers/acpi/sleep.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 05a31b5..7875b92 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -16,6 +16,8 @@
#include <linux/device.h>
#include <linux/interrupt.h>
#include <linux/suspend.h>
+#include <linux/notifier.h>
+#include <linux/pm.h>
#include <linux/reboot.h>
#include <linux/acpi.h>
#include <linux/module.h>
@@ -827,14 +829,22 @@ static void acpi_power_off_prepare(void)
acpi_disable_all_gpes();
}
-static void acpi_power_off(void)
+static int acpi_power_off(struct notifier_block *this,
+ unsigned long unused1, void *unused2)
{
-> Is there any reason why any notifier in the new chain would use the
second argument for anything meaningful? And the third argument for
that matter?
/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */
printk(KERN_DEBUG "%s called\n", __func__);
local_irq_disable();
acpi_enter_sleep_state(ACPI_STATE_S5);
+
+ return NOTIFY_DONE;
Also is there any reason for any notifier in the new chain to return anything
different from NOTIFY_DONE and if so, then what happens when anything else
is returned?
As mentioned earlier, notifiers just come handy. That is all.
Having said that, I don't have a strong opinion either way. If you want me
to implement a priority based callback handler with a single argument,
just let me know and I'll be happy to implement it. It is not worth arguing
about.
Would something like
struct power_off_block {
void (*power_off_call)(struct power_off_block *);
struct power_off_block __rcu *next;
int priority;
};
for the data structure be acceptable ? The power-off handler code would then
be something like
static void acpi_power_off(struct power_off_block *this)
{
}
ie quite similar to the current power-off handler code, with an added argument.
The API would, except for the structure argument, pretty much stay the same.
Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
