[RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Guenter Roeck]

Various drivers implement architecture and/or device specific means
to restart (reset) the system. Various mechanisms have been implemented
to support those schemes. The best known mechanism is arm_pm_restart,
which is a function pointer to be set either from platform specific code
or from drivers. Another mechanism is to use hardware watchdogs to issue
a reset; this mechanism is used if there is no other method available
to reset a board or system. Two examples are alim7101_wdt, which currently
uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
the arm_pm_restart function.

The existing mechanisms have a number of drawbacks. Typically only one scheme
to restart the system is supported (at least if arm_pm_restart is used).
At least in theory there can be mutliple means to restart the system, some of
which may be less desirable (for example one mechanism may only reset the CPU,
while another may reset the entire system). Using arm_pm_restart can also be
racy if the function pointer is set from a driver, as the driver may be in
the process of being unloaded when arm_pm_restart is called.
Using the reboot notifier is always racy, as it is unknown if and when
other functions using the reboot notifier have completed execution
by the time the watchdog fires.

To solve the problem, introduce a system restart notifier. This notifier
is expected to be called from the architecture specific machine_restart()
function. Drivers providing system restart functionality (such as the watchdog
drivers mentioned above) are expected to register with this notifier.

Patch 1 of this series implements the notifier function. Patches 2 and 3
implement calling the notifier chain from arm and arm64 restart code.
Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
that no one gets the idea to implement a restart handler as module.

[RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Guenter Roeck]

Various drivers implement architecture and/or device specific means
to restart (reset) the system. Various mechanisms have been implemented
to support those schemes. The best known mechanism is arm_pm_restart,
which is a function pointer to be set either from platform specific code
or from drivers. Another mechanism is to use hardware watchdogs to issue
a reset; this mechanism is used if there is no other method available
to reset a board or system. Two examples are alim7101_wdt, which currently
uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
the arm_pm_restart function.

The existing mechanisms have a number of drawbacks. Typically only one scheme
to restart the system is supported (at least if arm_pm_restart is used).
At least in theory there can be mutliple means to restart the system, some of
which may be less desirable (for example one mechanism may only reset the CPU,
while another may reset the entire system). Using arm_pm_restart can also be
racy if the function pointer is set from a driver, as the driver may be in
the process of being unloaded when arm_pm_restart is called.
Using the reboot notifier is always racy, as it is unknown if and when
other functions using the reboot notifier have completed execution
by the time the watchdog fires.

To solve the problem, introduce a system restart notifier. This notifier
is expected to be called from the architecture specific machine_restart()
function. Drivers providing system restart functionality (such as the watchdog
drivers mentioned above) are expected to register with this notifier.

Patch 1 of this series implements the notifier function. Patches 2 and 3
implement calling the notifier chain from arm and arm64 restart code.
Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
that no one gets the idea to implement a restart handler as module.

[RFC PATCH 1/6] kernel: Add support for restart notifier call chain

[Guenter Roeck]

Various drivers implement architecture and/or device specific means
to restart (reset) the system. Various mechanisms have been implemented
to support those schemes. The best known mechanism is arm_pm_restart,
which is a function pointer to be set either from platform specific code
or from drivers. Another mechanism is to use hardware watchdogs to issue
a reset; this mechanism is used if there is no other method available
to reset a board or system. Two examples are alim7101_wdt, which currently
uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
the arm_pm_restart function.

The existing mechanisms have a number of drawbacks. Typically only one scheme
to restart the system is supported (at least if arm_pm_restart is used).
At least in theory there can be mutliple means to restart the system, some of
which may be less desirable (for example one mechanism may only reset the CPU,
while another may reset the entire system). Using arm_pm_restart can also be
racy if the function pointer is set from a driver, as the driver may be in
the process of being unloaded when arm_pm_restart is called.
Using the reboot notifier is always racy, as it is unknown if and when
other functions using the reboot notifier have completed execution
by the time the watchdog fires.

To solve the problem, introduce a system restart notifier. This notifier
is expected to be called from the architecture specific machine_restart()
function. Drivers providing system restart functionality (such as the watchdog
drivers mentioned above) are expected to register with this notifier.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 include/linux/notifier.h |  1 +
 include/linux/reboot.h   |  2 ++
 kernel/notifier.c        |  6 ++++++
 kernel/reboot.c          | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 41 insertions(+)

diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index d14a4c3..3d7d2fc 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -210,6 +210,7 @@ static inline int notifier_to_errno(int ret)
 #define KBD_POST_KEYSYM		0x0005 /* Called after keyboard keysym interpretation */
 
 extern struct blocking_notifier_head reboot_notifier_list;
+extern struct blocking_notifier_head restart_notifier_list;
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_NOTIFIER_H */
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 48bf152..f8e7e0a 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -38,6 +38,8 @@ extern int reboot_force;
 extern int register_reboot_notifier(struct notifier_block *);
 extern int unregister_reboot_notifier(struct notifier_block *);
 
+extern int register_restart_notifier(struct notifier_block *);
+extern int unregister_restart_notifier(struct notifier_block *);
 
 /*
  * Architecture-specific implementations of sys_reboot commands.
diff --git a/kernel/notifier.c b/kernel/notifier.c
index 4803da6..2f2b071 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -14,6 +14,12 @@
 BLOCKING_NOTIFIER_HEAD(reboot_notifier_list);
 
 /*
+ *	Notifier list for kernel code which wants to be called
+ *	to restart the system.
+ */
+BLOCKING_NOTIFIER_HEAD(restart_notifier_list);
+
+/*
  *	Notifier chain core routines.  The exported routines below
  *	are layered on top of these, with appropriate locking added.
  */
diff --git a/kernel/reboot.c b/kernel/reboot.c
index a3a9e24..374896f 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -104,6 +104,38 @@ int unregister_reboot_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL(unregister_reboot_notifier);
 
+/**
+ *	register_restart_notifier - Register function to be called to reset
+ *				    the system
+ *	@nb: Info about notifier function to be called
+ *
+ *	Registers a function with the list of functions
+ *	to be called to restart the system.
+ *
+ *	Currently always returns zero, as blocking_notifier_chain_register()
+ *	always returns zero.
+ */
+int register_restart_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&restart_notifier_list, nb);
+}
+EXPORT_SYMBOL(register_restart_notifier);
+
+/**
+ *	unregister_restart_notifier - Unregister previously registered
+ *				      restart notifier
+ *	@nb: Hook to be unregistered
+ *
+ *	Unregisters a previously registered restart notifier function.
+ *
+ *	Returns zero on success, or %-ENOENT on failure.
+ */
+int unregister_restart_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&restart_notifier_list, nb);
+}
+EXPORT_SYMBOL(unregister_restart_notifier);
+
 void migrate_to_reboot_cpu(void)
 {
 	/* The boot cpu is always logical cpu 0 */
-- 
1.9.1

[RFC PATCH 1/6] kernel: Add support for restart notifier call chain

[Guenter Roeck]

Various drivers implement architecture and/or device specific means
to restart (reset) the system. Various mechanisms have been implemented
to support those schemes. The best known mechanism is arm_pm_restart,
which is a function pointer to be set either from platform specific code
or from drivers. Another mechanism is to use hardware watchdogs to issue
a reset; this mechanism is used if there is no other method available
to reset a board or system. Two examples are alim7101_wdt, which currently
uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
the arm_pm_restart function.

The existing mechanisms have a number of drawbacks. Typically only one scheme
to restart the system is supported (at least if arm_pm_restart is used).
At least in theory there can be mutliple means to restart the system, some of
which may be less desirable (for example one mechanism may only reset the CPU,
while another may reset the entire system). Using arm_pm_restart can also be
racy if the function pointer is set from a driver, as the driver may be in
the process of being unloaded when arm_pm_restart is called.
Using the reboot notifier is always racy, as it is unknown if and when
other functions using the reboot notifier have completed execution
by the time the watchdog fires.

To solve the problem, introduce a system restart notifier. This notifier
is expected to be called from the architecture specific machine_restart()
function. Drivers providing system restart functionality (such as the watchdog
drivers mentioned above) are expected to register with this notifier.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 include/linux/notifier.h |  1 +
 include/linux/reboot.h   |  2 ++
 kernel/notifier.c        |  6 ++++++
 kernel/reboot.c          | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 41 insertions(+)

diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index d14a4c3..3d7d2fc 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -210,6 +210,7 @@ static inline int notifier_to_errno(int ret)
 #define KBD_POST_KEYSYM		0x0005 /* Called after keyboard keysym interpretation */
 
 extern struct blocking_notifier_head reboot_notifier_list;
+extern struct blocking_notifier_head restart_notifier_list;
 
 #endif /* __KERNEL__ */
 #endif /* _LINUX_NOTIFIER_H */
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 48bf152..f8e7e0a 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -38,6 +38,8 @@ extern int reboot_force;
 extern int register_reboot_notifier(struct notifier_block *);
 extern int unregister_reboot_notifier(struct notifier_block *);
 
+extern int register_restart_notifier(struct notifier_block *);
+extern int unregister_restart_notifier(struct notifier_block *);
 
 /*
  * Architecture-specific implementations of sys_reboot commands.
diff --git a/kernel/notifier.c b/kernel/notifier.c
index 4803da6..2f2b071 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -14,6 +14,12 @@
 BLOCKING_NOTIFIER_HEAD(reboot_notifier_list);
 
 /*
+ *	Notifier list for kernel code which wants to be called
+ *	to restart the system.
+ */
+BLOCKING_NOTIFIER_HEAD(restart_notifier_list);
+
+/*
  *	Notifier chain core routines.  The exported routines below
  *	are layered on top of these, with appropriate locking added.
  */
diff --git a/kernel/reboot.c b/kernel/reboot.c
index a3a9e24..374896f 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -104,6 +104,38 @@ int unregister_reboot_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL(unregister_reboot_notifier);
 
+/**
+ *	register_restart_notifier - Register function to be called to reset
+ *				    the system
+ *	@nb: Info about notifier function to be called
+ *
+ *	Registers a function with the list of functions
+ *	to be called to restart the system.
+ *
+ *	Currently always returns zero, as blocking_notifier_chain_register()
+ *	always returns zero.
+ */
+int register_restart_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&restart_notifier_list, nb);
+}
+EXPORT_SYMBOL(register_restart_notifier);
+
+/**
+ *	unregister_restart_notifier - Unregister previously registered
+ *				      restart notifier
+ *	@nb: Hook to be unregistered
+ *
+ *	Unregisters a previously registered restart notifier function.
+ *
+ *	Returns zero on success, or %-ENOENT on failure.
+ */
+int unregister_restart_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&restart_notifier_list, nb);
+}
+EXPORT_SYMBOL(unregister_restart_notifier);
+
 void migrate_to_reboot_cpu(void)
 {
 	/* The boot cpu is always logical cpu 0 */
-- 
1.9.1

[RFC PATCH 2/6] arm64: Support restart through restart notifier call chain

[Guenter Roeck]

The kernel core now supports a notifier call chain to restart the system.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/arm64/kernel/process.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 43b7c34..9dd2abd 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -43,6 +43,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/personality.h>
 #include <linux/notifier.h>
+#include <linux/watchdog.h>
 
 #include <asm/compat.h>
 #include <asm/cacheflush.h>
@@ -175,6 +176,8 @@ void machine_restart(char *cmd)
 	if (arm_pm_restart)
 		arm_pm_restart(reboot_mode, cmd);
 
+	blocking_notifier_call_chain(&restart_notifier_list, reboot_mode, cmd);
+
 	/*
 	 * Whoops - the architecture was unable to reboot.
 	 */
-- 
1.9.1

[RFC PATCH 2/6] arm64: Support restart through restart notifier call chain

[Guenter Roeck]

The kernel core now supports a notifier call chain to restart the system.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/arm64/kernel/process.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 43b7c34..9dd2abd 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -43,6 +43,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/personality.h>
 #include <linux/notifier.h>
+#include <linux/watchdog.h>
 
 #include <asm/compat.h>
 #include <asm/cacheflush.h>
@@ -175,6 +176,8 @@ void machine_restart(char *cmd)
 	if (arm_pm_restart)
 		arm_pm_restart(reboot_mode, cmd);
 
+	blocking_notifier_call_chain(&restart_notifier_list, reboot_mode, cmd);
+
 	/*
 	 * Whoops - the architecture was unable to reboot.
 	 */
-- 
1.9.1

Re: [RFC PATCH 2/6] arm64: Support restart through restart notifier call chain

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 824 bytes --]

On Mon, Jun 30, 2014 at 12:11:35PM -0700, Guenter Roeck wrote:
> The kernel core now supports a notifier call chain to restart the system.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  arch/arm64/kernel/process.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 43b7c34..9dd2abd 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -43,6 +43,7 @@
>  #include <linux/hw_breakpoint.h>
>  #include <linux/personality.h>
>  #include <linux/notifier.h>
> +#include <linux/watchdog.h>

I don't think you need this include, or shouldn't it be reboot.h
instead?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[RFC PATCH 2/6] arm64: Support restart through restart notifier call chain

[Maxime Ripard]

On Mon, Jun 30, 2014 at 12:11:35PM -0700, Guenter Roeck wrote:
> The kernel core now supports a notifier call chain to restart the system.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  arch/arm64/kernel/process.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 43b7c34..9dd2abd 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -43,6 +43,7 @@
>  #include <linux/hw_breakpoint.h>
>  #include <linux/personality.h>
>  #include <linux/notifier.h>
> +#include <linux/watchdog.h>

I don't think you need this include, or shouldn't it be reboot.h
instead?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140701/c23e431a/attachment.sig>

Re: [RFC PATCH 2/6] arm64: Support restart through restart notifier call chain

[Guenter Roeck]

On 07/01/2014 12:26 AM, Maxime Ripard wrote:
> On Mon, Jun 30, 2014 at 12:11:35PM -0700, Guenter Roeck wrote:
>> The kernel core now supports a notifier call chain to restart the system.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   arch/arm64/kernel/process.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>> index 43b7c34..9dd2abd 100644
>> --- a/arch/arm64/kernel/process.c
>> +++ b/arch/arm64/kernel/process.c
>> @@ -43,6 +43,7 @@
>>   #include <linux/hw_breakpoint.h>
>>   #include <linux/personality.h>
>>   #include <linux/notifier.h>
>> +#include <linux/watchdog.h>
>
> I don't think you need this include, or shouldn't it be reboot.h
> instead?
>

Yes, that will be removed in the next version of the patch.

Thanks,
Guenter

[RFC PATCH 2/6] arm64: Support restart through restart notifier call chain

[Guenter Roeck]

On 07/01/2014 12:26 AM, Maxime Ripard wrote:
> On Mon, Jun 30, 2014 at 12:11:35PM -0700, Guenter Roeck wrote:
>> The kernel core now supports a notifier call chain to restart the system.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   arch/arm64/kernel/process.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
>> index 43b7c34..9dd2abd 100644
>> --- a/arch/arm64/kernel/process.c
>> +++ b/arch/arm64/kernel/process.c
>> @@ -43,6 +43,7 @@
>>   #include <linux/hw_breakpoint.h>
>>   #include <linux/personality.h>
>>   #include <linux/notifier.h>
>> +#include <linux/watchdog.h>
>
> I don't think you need this include, or shouldn't it be reboot.h
> instead?
>

Yes, that will be removed in the next version of the patch.

Thanks,
Guenter

[RFC PATCH 3/6] arm: Support restart through restart notifier call chain

[Guenter Roeck]

The kernel core now supports a notifier call chain for system
restart functions.

With this change, the arm_pm_restart callback is now optional,
so check if it is set before calling it.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/arm/kernel/process.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 81ef686..93765f2 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -32,6 +32,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/leds.h>
 #include <linux/reboot.h>
+#include <linux/watchdog.h>
 
 #include <asm/cacheflush.h>
 #include <asm/idmap.h>
@@ -230,7 +231,10 @@ void machine_restart(char *cmd)
 	local_irq_disable();
 	smp_send_stop();
 
-	arm_pm_restart(reboot_mode, cmd);
+	if (arm_pm_restart)
+		arm_pm_restart(reboot_mode, cmd);
+
+	blocking_notifier_call_chain(&restart_notifier_list, reboot_mode, cmd);
 
 	/* Give a grace period for failure to restart of 1s */
 	mdelay(1000);
-- 
1.9.1

[RFC PATCH 3/6] arm: Support restart through restart notifier call chain

[Guenter Roeck]

The kernel core now supports a notifier call chain for system
restart functions.

With this change, the arm_pm_restart callback is now optional,
so check if it is set before calling it.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/arm/kernel/process.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 81ef686..93765f2 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -32,6 +32,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/leds.h>
 #include <linux/reboot.h>
+#include <linux/watchdog.h>
 
 #include <asm/cacheflush.h>
 #include <asm/idmap.h>
@@ -230,7 +231,10 @@ void machine_restart(char *cmd)
 	local_irq_disable();
 	smp_send_stop();
 
-	arm_pm_restart(reboot_mode, cmd);
+	if (arm_pm_restart)
+		arm_pm_restart(reboot_mode, cmd);
+
+	blocking_notifier_call_chain(&restart_notifier_list, reboot_mode, cmd);
 
 	/* Give a grace period for failure to restart of 1s */
 	mdelay(1000);
-- 
1.9.1

Re: [RFC PATCH 3/6] arm: Support restart through restart notifier call chain

[Russell King - ARM Linux]

On Mon, Jun 30, 2014 at 12:11:36PM -0700, Guenter Roeck wrote:
> The kernel core now supports a notifier call chain for system
> restart functions.
> 
> With this change, the arm_pm_restart callback is now optional,
> so check if it is set before calling it.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  arch/arm/kernel/process.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 81ef686..93765f2 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -32,6 +32,7 @@
>  #include <linux/hw_breakpoint.h>
>  #include <linux/leds.h>
>  #include <linux/reboot.h>
> +#include <linux/watchdog.h>
>  
>  #include <asm/cacheflush.h>
>  #include <asm/idmap.h>
> @@ -230,7 +231,10 @@ void machine_restart(char *cmd)
>  	local_irq_disable();
>  	smp_send_stop();
>  
> -	arm_pm_restart(reboot_mode, cmd);
> +	if (arm_pm_restart)
> +		arm_pm_restart(reboot_mode, cmd);
> +
> +	blocking_notifier_call_chain(&restart_notifier_list, reboot_mode, cmd);

One or the other please, because there could (and I think there are)
some implementations which tell stuff to power down, and then continue
into the mdelay() below.  In such situations, we don't want to call
the notifier.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[RFC PATCH 3/6] arm: Support restart through restart notifier call chain

[Russell King - ARM Linux]

On Mon, Jun 30, 2014 at 12:11:36PM -0700, Guenter Roeck wrote:
> The kernel core now supports a notifier call chain for system
> restart functions.
> 
> With this change, the arm_pm_restart callback is now optional,
> so check if it is set before calling it.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  arch/arm/kernel/process.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 81ef686..93765f2 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -32,6 +32,7 @@
>  #include <linux/hw_breakpoint.h>
>  #include <linux/leds.h>
>  #include <linux/reboot.h>
> +#include <linux/watchdog.h>
>  
>  #include <asm/cacheflush.h>
>  #include <asm/idmap.h>
> @@ -230,7 +231,10 @@ void machine_restart(char *cmd)
>  	local_irq_disable();
>  	smp_send_stop();
>  
> -	arm_pm_restart(reboot_mode, cmd);
> +	if (arm_pm_restart)
> +		arm_pm_restart(reboot_mode, cmd);
> +
> +	blocking_notifier_call_chain(&restart_notifier_list, reboot_mode, cmd);

One or the other please, because there could (and I think there are)
some implementations which tell stuff to power down, and then continue
into the mdelay() below.  In such situations, we don't want to call
the notifier.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[RFC PATCH 4/6] watchdog: moxart: Register restart handler with restart notifier

[Guenter Roeck]

The kernel now provides an API to trigger a system restart.
Register with it instead of setting arm_pm_restart.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/moxart_wdt.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
index 4aa3a8a..9a862bf 100644
--- a/drivers/watchdog/moxart_wdt.c
+++ b/drivers/watchdog/moxart_wdt.c
@@ -15,12 +15,12 @@
 #include <linux/module.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
+#include <linux/notifier.h>
 #include <linux/platform_device.h>
+#include <linux/reboot.h>
 #include <linux/watchdog.h>
 #include <linux/moduleparam.h>
 
-#include <asm/system_misc.h>
-
 #define REG_COUNT			0x4
 #define REG_MODE			0x8
 #define REG_ENABLE			0xC
@@ -35,13 +35,20 @@ static struct moxart_wdt_dev *moxart_restart_ctx;
 
 static int heartbeat;
 
-static void moxart_wdt_restart(enum reboot_mode reboot_mode, const char *cmd)
+static int moxart_restart_notify(struct notifier_block *this, unsigned long mode,
+				 void *cmd)
 {
 	writel(1, moxart_restart_ctx->base + REG_COUNT);
 	writel(0x5ab9, moxart_restart_ctx->base + REG_MODE);
 	writel(0x03, moxart_restart_ctx->base + REG_ENABLE);
+
+	return NOTIFY_DONE;
 }
 
+static struct notifier_block moxart_restart_notifier = {
+	.notifier_call = moxart_restart_notify,
+};
+
 static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
 {
 	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
@@ -137,7 +144,9 @@ static int moxart_wdt_probe(struct platform_device *pdev)
 		return err;
 
 	moxart_restart_ctx = moxart_wdt;
-	arm_pm_restart = moxart_wdt_restart;
+	err = register_restart_notifier(&moxart_restart_notifier);
+	if (err)
+		dev_err(dev, "cannot register reset notifier (err=%d)\n", err);
 
 	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
 		moxart_wdt->dev.timeout, nowayout);
@@ -149,9 +158,8 @@ static int moxart_wdt_remove(struct platform_device *pdev)
 {
 	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
 
-	arm_pm_restart = NULL;
+	unregister_restart_notifier(&moxart_restart_notifier);
 	moxart_wdt_stop(&moxart_wdt->dev);
-	watchdog_unregister_device(&moxart_wdt->dev);
 
 	return 0;
 }
-- 
1.9.1

[RFC PATCH 4/6] watchdog: moxart: Register restart handler with restart notifier

[Guenter Roeck]

The kernel now provides an API to trigger a system restart.
Register with it instead of setting arm_pm_restart.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/moxart_wdt.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
index 4aa3a8a..9a862bf 100644
--- a/drivers/watchdog/moxart_wdt.c
+++ b/drivers/watchdog/moxart_wdt.c
@@ -15,12 +15,12 @@
 #include <linux/module.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
+#include <linux/notifier.h>
 #include <linux/platform_device.h>
+#include <linux/reboot.h>
 #include <linux/watchdog.h>
 #include <linux/moduleparam.h>
 
-#include <asm/system_misc.h>
-
 #define REG_COUNT			0x4
 #define REG_MODE			0x8
 #define REG_ENABLE			0xC
@@ -35,13 +35,20 @@ static struct moxart_wdt_dev *moxart_restart_ctx;
 
 static int heartbeat;
 
-static void moxart_wdt_restart(enum reboot_mode reboot_mode, const char *cmd)
+static int moxart_restart_notify(struct notifier_block *this, unsigned long mode,
+				 void *cmd)
 {
 	writel(1, moxart_restart_ctx->base + REG_COUNT);
 	writel(0x5ab9, moxart_restart_ctx->base + REG_MODE);
 	writel(0x03, moxart_restart_ctx->base + REG_ENABLE);
+
+	return NOTIFY_DONE;
 }
 
+static struct notifier_block moxart_restart_notifier = {
+	.notifier_call = moxart_restart_notify,
+};
+
 static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
 {
 	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
@@ -137,7 +144,9 @@ static int moxart_wdt_probe(struct platform_device *pdev)
 		return err;
 
 	moxart_restart_ctx = moxart_wdt;
-	arm_pm_restart = moxart_wdt_restart;
+	err = register_restart_notifier(&moxart_restart_notifier);
+	if (err)
+		dev_err(dev, "cannot register reset notifier (err=%d)\n", err);
 
 	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
 		moxart_wdt->dev.timeout, nowayout);
@@ -149,9 +158,8 @@ static int moxart_wdt_remove(struct platform_device *pdev)
 {
 	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
 
-	arm_pm_restart = NULL;
+	unregister_restart_notifier(&moxart_restart_notifier);
 	moxart_wdt_stop(&moxart_wdt->dev);
-	watchdog_unregister_device(&moxart_wdt->dev);
 
 	return 0;
 }
-- 
1.9.1

[RFC PATCH 5/6] watchdog: alim7101: Register restart handler with restart notifier

[Guenter Roeck]

The kernel core now provides an API to trigger a system restart.
Register with it to restart the system instead of misusing the
reboot notifier.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/alim7101_wdt.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/alim7101_wdt.c b/drivers/watchdog/alim7101_wdt.c
index 996b2f7..239d6b2 100644
--- a/drivers/watchdog/alim7101_wdt.c
+++ b/drivers/watchdog/alim7101_wdt.c
@@ -301,6 +301,27 @@ static struct miscdevice wdt_miscdev = {
 	.fops	=	&wdt_fops,
 };
 
+static int wdt_restart_notify(struct notifier_block *this, unsigned long mode,
+			      void *cmd)
+{
+	/*
+	 * Cobalt devices have no way of rebooting themselves other
+	 * than getting the watchdog to pull reset, so we restart the
+	 * watchdog on reboot with no heartbeat.
+	 */
+	wdt_change(WDT_ENABLE);
+
+	/* loop until the watchdog fires */
+	while (true)
+		;
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block wdt_restart_notifier = {
+	.notifier_call = wdt_restart_notify,
+};
+
 /*
  *	Notifier for system down
  */
@@ -311,15 +332,6 @@ static int wdt_notify_sys(struct notifier_block *this,
 	if (code == SYS_DOWN || code == SYS_HALT)
 		wdt_turnoff();
 
-	if (code == SYS_RESTART) {
-		/*
-		 * Cobalt devices have no way of rebooting themselves other
-		 * than getting the watchdog to pull reset, so we restart the
-		 * watchdog on reboot with no heartbeat
-		 */
-		wdt_change(WDT_ENABLE);
-		pr_info("Watchdog timer is now enabled with no heartbeat - should reboot in ~1 second\n");
-	}
 	return NOTIFY_DONE;
 }
 
@@ -338,6 +350,7 @@ static void __exit alim7101_wdt_unload(void)
 	/* Deregister */
 	misc_deregister(&wdt_miscdev);
 	unregister_reboot_notifier(&wdt_notifier);
+	unregister_restart_notifier(&wdt_restart_notifier);
 	pci_dev_put(alim7101_pmu);
 }
 
@@ -390,11 +403,17 @@ static int __init alim7101_wdt_init(void)
 		goto err_out;
 	}
 
+	rc = register_restart_notifier(&wdt_restart_notifier);
+	if (rc) {
+		pr_err("cannot register reboot notifier (err=%d)\n", rc);
+		goto err_out_reboot;
+	}
+
 	rc = misc_register(&wdt_miscdev);
 	if (rc) {
 		pr_err("cannot register miscdev on minor=%d (err=%d)\n",
 		       wdt_miscdev.minor, rc);
-		goto err_out_reboot;
+		goto err_out_restart;
 	}
 
 	if (nowayout)
@@ -404,6 +423,8 @@ static int __init alim7101_wdt_init(void)
 		timeout, nowayout);
 	return 0;
 
+err_out_restart:
+	unregister_restart_notifier(&wdt_restart_notifier);
 err_out_reboot:
 	unregister_reboot_notifier(&wdt_notifier);
 err_out:
-- 
1.9.1

[RFC PATCH 5/6] watchdog: alim7101: Register restart handler with restart notifier

[Guenter Roeck]

The kernel core now provides an API to trigger a system restart.
Register with it to restart the system instead of misusing the
reboot notifier.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/alim7101_wdt.c | 41 +++++++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/alim7101_wdt.c b/drivers/watchdog/alim7101_wdt.c
index 996b2f7..239d6b2 100644
--- a/drivers/watchdog/alim7101_wdt.c
+++ b/drivers/watchdog/alim7101_wdt.c
@@ -301,6 +301,27 @@ static struct miscdevice wdt_miscdev = {
 	.fops	=	&wdt_fops,
 };
 
+static int wdt_restart_notify(struct notifier_block *this, unsigned long mode,
+			      void *cmd)
+{
+	/*
+	 * Cobalt devices have no way of rebooting themselves other
+	 * than getting the watchdog to pull reset, so we restart the
+	 * watchdog on reboot with no heartbeat.
+	 */
+	wdt_change(WDT_ENABLE);
+
+	/* loop until the watchdog fires */
+	while (true)
+		;
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block wdt_restart_notifier = {
+	.notifier_call = wdt_restart_notify,
+};
+
 /*
  *	Notifier for system down
  */
@@ -311,15 +332,6 @@ static int wdt_notify_sys(struct notifier_block *this,
 	if (code == SYS_DOWN || code == SYS_HALT)
 		wdt_turnoff();
 
-	if (code == SYS_RESTART) {
-		/*
-		 * Cobalt devices have no way of rebooting themselves other
-		 * than getting the watchdog to pull reset, so we restart the
-		 * watchdog on reboot with no heartbeat
-		 */
-		wdt_change(WDT_ENABLE);
-		pr_info("Watchdog timer is now enabled with no heartbeat - should reboot in ~1 second\n");
-	}
 	return NOTIFY_DONE;
 }
 
@@ -338,6 +350,7 @@ static void __exit alim7101_wdt_unload(void)
 	/* Deregister */
 	misc_deregister(&wdt_miscdev);
 	unregister_reboot_notifier(&wdt_notifier);
+	unregister_restart_notifier(&wdt_restart_notifier);
 	pci_dev_put(alim7101_pmu);
 }
 
@@ -390,11 +403,17 @@ static int __init alim7101_wdt_init(void)
 		goto err_out;
 	}
 
+	rc = register_restart_notifier(&wdt_restart_notifier);
+	if (rc) {
+		pr_err("cannot register reboot notifier (err=%d)\n", rc);
+		goto err_out_reboot;
+	}
+
 	rc = misc_register(&wdt_miscdev);
 	if (rc) {
 		pr_err("cannot register miscdev on minor=%d (err=%d)\n",
 		       wdt_miscdev.minor, rc);
-		goto err_out_reboot;
+		goto err_out_restart;
 	}
 
 	if (nowayout)
@@ -404,6 +423,8 @@ static int __init alim7101_wdt_init(void)
 		timeout, nowayout);
 	return 0;
 
+err_out_restart:
+	unregister_restart_notifier(&wdt_restart_notifier);
 err_out_reboot:
 	unregister_reboot_notifier(&wdt_notifier);
 err_out:
-- 
1.9.1

[RFC PATCH 6/6] arm/arm64: Unexport restart handlers

[Guenter Roeck]

Implementing a restart handler in a module don't make sense
as there would be no guarantee that the module is loaded when
a restart is needed. Unexport arm_pm_restart to ensure that
no one gets the idea to do it anyway.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/arm/kernel/process.c   | 1 -
 arch/arm64/kernel/process.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 93765f2..b587ef5 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -126,7 +126,6 @@ void (*pm_power_off)(void);
 EXPORT_SYMBOL(pm_power_off);
 
 void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart;
-EXPORT_SYMBOL_GPL(arm_pm_restart);
 
 /*
  * This is our default idle handler.
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 9dd2abd..074079d 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -93,7 +93,6 @@ void (*pm_power_off)(void);
 EXPORT_SYMBOL_GPL(pm_power_off);
 
 void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
-EXPORT_SYMBOL_GPL(arm_pm_restart);
 
 /*
  * This is our default idle handler.
-- 
1.9.1

[RFC PATCH 6/6] arm/arm64: Unexport restart handlers

[Guenter Roeck]

Implementing a restart handler in a module don't make sense
as there would be no guarantee that the module is loaded when
a restart is needed. Unexport arm_pm_restart to ensure that
no one gets the idea to do it anyway.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 arch/arm/kernel/process.c   | 1 -
 arch/arm64/kernel/process.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 93765f2..b587ef5 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -126,7 +126,6 @@ void (*pm_power_off)(void);
 EXPORT_SYMBOL(pm_power_off);
 
 void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart;
-EXPORT_SYMBOL_GPL(arm_pm_restart);
 
 /*
  * This is our default idle handler.
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 9dd2abd..074079d 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -93,7 +93,6 @@ void (*pm_power_off)(void);
 EXPORT_SYMBOL_GPL(pm_power_off);
 
 void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
-EXPORT_SYMBOL_GPL(arm_pm_restart);
 
 /*
  * This is our default idle handler.
-- 
1.9.1

Re: [RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Russell King - ARM Linux]

On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
> Various drivers implement architecture and/or device specific means
> to restart (reset) the system. Various mechanisms have been implemented
> to support those schemes. The best known mechanism is arm_pm_restart,
> which is a function pointer to be set either from platform specific code
> or from drivers. Another mechanism is to use hardware watchdogs to issue
> a reset; this mechanism is used if there is no other method available
> to reset a board or system. Two examples are alim7101_wdt, which currently
> uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
> the arm_pm_restart function.
> 
> The existing mechanisms have a number of drawbacks. Typically only one scheme
> to restart the system is supported (at least if arm_pm_restart is used).
> At least in theory there can be mutliple means to restart the system, some of
> which may be less desirable (for example one mechanism may only reset the CPU,
> while another may reset the entire system). Using arm_pm_restart can also be
> racy if the function pointer is set from a driver, as the driver may be in
> the process of being unloaded when arm_pm_restart is called.
> Using the reboot notifier is always racy, as it is unknown if and when
> other functions using the reboot notifier have completed execution
> by the time the watchdog fires.
> 
> To solve the problem, introduce a system restart notifier. This notifier
> is expected to be called from the architecture specific machine_restart()
> function. Drivers providing system restart functionality (such as the watchdog
> drivers mentioned above) are expected to register with this notifier.
> 
> Patch 1 of this series implements the notifier function. Patches 2 and 3
> implement calling the notifier chain from arm and arm64 restart code.
> Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
> to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
> that no one gets the idea to implement a restart handler as module.

I think you need to restructure stuff somewhat, because I think
you've missed drivers/power/reset/ entirely, or at least you've
missed drivers/power/reset/restart-poweroff.c which calls
arm_pm_restart directly.  I'm not quite sure how we ended up with
that...

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Russell King - ARM Linux]

On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
> Various drivers implement architecture and/or device specific means
> to restart (reset) the system. Various mechanisms have been implemented
> to support those schemes. The best known mechanism is arm_pm_restart,
> which is a function pointer to be set either from platform specific code
> or from drivers. Another mechanism is to use hardware watchdogs to issue
> a reset; this mechanism is used if there is no other method available
> to reset a board or system. Two examples are alim7101_wdt, which currently
> uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
> the arm_pm_restart function.
> 
> The existing mechanisms have a number of drawbacks. Typically only one scheme
> to restart the system is supported (at least if arm_pm_restart is used).
> At least in theory there can be mutliple means to restart the system, some of
> which may be less desirable (for example one mechanism may only reset the CPU,
> while another may reset the entire system). Using arm_pm_restart can also be
> racy if the function pointer is set from a driver, as the driver may be in
> the process of being unloaded when arm_pm_restart is called.
> Using the reboot notifier is always racy, as it is unknown if and when
> other functions using the reboot notifier have completed execution
> by the time the watchdog fires.
> 
> To solve the problem, introduce a system restart notifier. This notifier
> is expected to be called from the architecture specific machine_restart()
> function. Drivers providing system restart functionality (such as the watchdog
> drivers mentioned above) are expected to register with this notifier.
> 
> Patch 1 of this series implements the notifier function. Patches 2 and 3
> implement calling the notifier chain from arm and arm64 restart code.
> Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
> to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
> that no one gets the idea to implement a restart handler as module.

I think you need to restructure stuff somewhat, because I think
you've missed drivers/power/reset/ entirely, or at least you've
missed drivers/power/reset/restart-poweroff.c which calls
arm_pm_restart directly.  I'm not quite sure how we ended up with
that...

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Guenter Roeck]

On Mon, Jun 30, 2014 at 08:59:47PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
> > Various drivers implement architecture and/or device specific means
> > to restart (reset) the system. Various mechanisms have been implemented
> > to support those schemes. The best known mechanism is arm_pm_restart,
> > which is a function pointer to be set either from platform specific code
> > or from drivers. Another mechanism is to use hardware watchdogs to issue
> > a reset; this mechanism is used if there is no other method available
> > to reset a board or system. Two examples are alim7101_wdt, which currently
> > uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
> > the arm_pm_restart function.
> > 
> > The existing mechanisms have a number of drawbacks. Typically only one scheme
> > to restart the system is supported (at least if arm_pm_restart is used).
> > At least in theory there can be mutliple means to restart the system, some of
> > which may be less desirable (for example one mechanism may only reset the CPU,
> > while another may reset the entire system). Using arm_pm_restart can also be
> > racy if the function pointer is set from a driver, as the driver may be in
> > the process of being unloaded when arm_pm_restart is called.
> > Using the reboot notifier is always racy, as it is unknown if and when
> > other functions using the reboot notifier have completed execution
> > by the time the watchdog fires.
> > 
> > To solve the problem, introduce a system restart notifier. This notifier
> > is expected to be called from the architecture specific machine_restart()
> > function. Drivers providing system restart functionality (such as the watchdog
> > drivers mentioned above) are expected to register with this notifier.
> > 
> > Patch 1 of this series implements the notifier function. Patches 2 and 3
> > implement calling the notifier chain from arm and arm64 restart code.
> > Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
> > to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
> > that no one gets the idea to implement a restart handler as module.
> 
> I think you need to restructure stuff somewhat, because I think
> you've missed drivers/power/reset/ entirely, or at least you've
> missed drivers/power/reset/restart-poweroff.c which calls
> arm_pm_restart directly.  I'm not quite sure how we ended up with
> that...
> 
Yes, guess I missed (and did not really expect) that arm_pm_restart
is called from multiple places.

What is restart-poweroff supposed to do in the first place, and why
doesn't it call machine_restart() ? If it is what I think it is, ie
a fallback for pm_power_off, it could be made generic and does not
really have to depend on ARM.

Thanks,
Guenter

[RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Guenter Roeck]

On Mon, Jun 30, 2014 at 08:59:47PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
> > Various drivers implement architecture and/or device specific means
> > to restart (reset) the system. Various mechanisms have been implemented
> > to support those schemes. The best known mechanism is arm_pm_restart,
> > which is a function pointer to be set either from platform specific code
> > or from drivers. Another mechanism is to use hardware watchdogs to issue
> > a reset; this mechanism is used if there is no other method available
> > to reset a board or system. Two examples are alim7101_wdt, which currently
> > uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
> > the arm_pm_restart function.
> > 
> > The existing mechanisms have a number of drawbacks. Typically only one scheme
> > to restart the system is supported (at least if arm_pm_restart is used).
> > At least in theory there can be mutliple means to restart the system, some of
> > which may be less desirable (for example one mechanism may only reset the CPU,
> > while another may reset the entire system). Using arm_pm_restart can also be
> > racy if the function pointer is set from a driver, as the driver may be in
> > the process of being unloaded when arm_pm_restart is called.
> > Using the reboot notifier is always racy, as it is unknown if and when
> > other functions using the reboot notifier have completed execution
> > by the time the watchdog fires.
> > 
> > To solve the problem, introduce a system restart notifier. This notifier
> > is expected to be called from the architecture specific machine_restart()
> > function. Drivers providing system restart functionality (such as the watchdog
> > drivers mentioned above) are expected to register with this notifier.
> > 
> > Patch 1 of this series implements the notifier function. Patches 2 and 3
> > implement calling the notifier chain from arm and arm64 restart code.
> > Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
> > to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
> > that no one gets the idea to implement a restart handler as module.
> 
> I think you need to restructure stuff somewhat, because I think
> you've missed drivers/power/reset/ entirely, or at least you've
> missed drivers/power/reset/restart-poweroff.c which calls
> arm_pm_restart directly.  I'm not quite sure how we ended up with
> that...
> 
Yes, guess I missed (and did not really expect) that arm_pm_restart
is called from multiple places.

What is restart-poweroff supposed to do in the first place, and why
doesn't it call machine_restart() ? If it is what I think it is, ie
a fallback for pm_power_off, it could be made generic and does not
really have to depend on ARM.

Thanks,
Guenter

Re: [RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Catalin Marinas]

On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote:
> On Mon, Jun 30, 2014 at 08:59:47PM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
> > > Various drivers implement architecture and/or device specific means
> > > to restart (reset) the system. Various mechanisms have been implemented
> > > to support those schemes. The best known mechanism is arm_pm_restart,
> > > which is a function pointer to be set either from platform specific code
> > > or from drivers. Another mechanism is to use hardware watchdogs to issue
> > > a reset; this mechanism is used if there is no other method available
> > > to reset a board or system. Two examples are alim7101_wdt, which currently
> > > uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
> > > the arm_pm_restart function.
> > > 
> > > The existing mechanisms have a number of drawbacks. Typically only one scheme
> > > to restart the system is supported (at least if arm_pm_restart is used).
> > > At least in theory there can be mutliple means to restart the system, some of
> > > which may be less desirable (for example one mechanism may only reset the CPU,
> > > while another may reset the entire system). Using arm_pm_restart can also be
> > > racy if the function pointer is set from a driver, as the driver may be in
> > > the process of being unloaded when arm_pm_restart is called.
> > > Using the reboot notifier is always racy, as it is unknown if and when
> > > other functions using the reboot notifier have completed execution
> > > by the time the watchdog fires.
> > > 
> > > To solve the problem, introduce a system restart notifier. This notifier
> > > is expected to be called from the architecture specific machine_restart()
> > > function. Drivers providing system restart functionality (such as the watchdog
> > > drivers mentioned above) are expected to register with this notifier.
> > > 
> > > Patch 1 of this series implements the notifier function. Patches 2 and 3
> > > implement calling the notifier chain from arm and arm64 restart code.
> > > Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
> > > to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
> > > that no one gets the idea to implement a restart handler as module.
> > 
> > I think you need to restructure stuff somewhat, because I think
> > you've missed drivers/power/reset/ entirely, or at least you've
> > missed drivers/power/reset/restart-poweroff.c which calls
> > arm_pm_restart directly.  I'm not quite sure how we ended up with
> > that...
> 
> Yes, guess I missed (and did not really expect) that arm_pm_restart
> is called from multiple places.

Most of the ARM-specific code in drivers/power/reset/ consists of SoC
power-off/restart back-ends (e.g. vexpress-poweroff.c). Since there is
no generic pm_restart, we continued to use arm_pm_restart (also for
arm64 since we share some of the drivers). Maybe some driver model here
would help.

> What is restart-poweroff supposed to do in the first place, and why
> doesn't it call machine_restart() ? If it is what I think it is, ie
> a fallback for pm_power_off, it could be made generic and does not
> really have to depend on ARM.

I think this one pretends to do a power-off via restart. It could call
machine_restart() but this only passes the default reboot_mode to
arm_pm_restart().

-- 
Catalin

[RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Catalin Marinas]

On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote:
> On Mon, Jun 30, 2014 at 08:59:47PM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
> > > Various drivers implement architecture and/or device specific means
> > > to restart (reset) the system. Various mechanisms have been implemented
> > > to support those schemes. The best known mechanism is arm_pm_restart,
> > > which is a function pointer to be set either from platform specific code
> > > or from drivers. Another mechanism is to use hardware watchdogs to issue
> > > a reset; this mechanism is used if there is no other method available
> > > to reset a board or system. Two examples are alim7101_wdt, which currently
> > > uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
> > > the arm_pm_restart function.
> > > 
> > > The existing mechanisms have a number of drawbacks. Typically only one scheme
> > > to restart the system is supported (at least if arm_pm_restart is used).
> > > At least in theory there can be mutliple means to restart the system, some of
> > > which may be less desirable (for example one mechanism may only reset the CPU,
> > > while another may reset the entire system). Using arm_pm_restart can also be
> > > racy if the function pointer is set from a driver, as the driver may be in
> > > the process of being unloaded when arm_pm_restart is called.
> > > Using the reboot notifier is always racy, as it is unknown if and when
> > > other functions using the reboot notifier have completed execution
> > > by the time the watchdog fires.
> > > 
> > > To solve the problem, introduce a system restart notifier. This notifier
> > > is expected to be called from the architecture specific machine_restart()
> > > function. Drivers providing system restart functionality (such as the watchdog
> > > drivers mentioned above) are expected to register with this notifier.
> > > 
> > > Patch 1 of this series implements the notifier function. Patches 2 and 3
> > > implement calling the notifier chain from arm and arm64 restart code.
> > > Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
> > > to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
> > > that no one gets the idea to implement a restart handler as module.
> > 
> > I think you need to restructure stuff somewhat, because I think
> > you've missed drivers/power/reset/ entirely, or at least you've
> > missed drivers/power/reset/restart-poweroff.c which calls
> > arm_pm_restart directly.  I'm not quite sure how we ended up with
> > that...
> 
> Yes, guess I missed (and did not really expect) that arm_pm_restart
> is called from multiple places.

Most of the ARM-specific code in drivers/power/reset/ consists of SoC
power-off/restart back-ends (e.g. vexpress-poweroff.c). Since there is
no generic pm_restart, we continued to use arm_pm_restart (also for
arm64 since we share some of the drivers). Maybe some driver model here
would help.

> What is restart-poweroff supposed to do in the first place, and why
> doesn't it call machine_restart() ? If it is what I think it is, ie
> a fallback for pm_power_off, it could be made generic and does not
> really have to depend on ARM.

I think this one pretends to do a power-off via restart. It could call
machine_restart() but this only passes the default reboot_mode to
arm_pm_restart().

-- 
Catalin

Re: [RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Catalin Marinas]

On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote:
> On Mon, Jun 30, 2014 at 08:59:47PM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
> > > Various drivers implement architecture and/or device specific means
> > > to restart (reset) the system. Various mechanisms have been implemented
> > > to support those schemes. The best known mechanism is arm_pm_restart,
> > > which is a function pointer to be set either from platform specific code
> > > or from drivers. Another mechanism is to use hardware watchdogs to issue
> > > a reset; this mechanism is used if there is no other method available
> > > to reset a board or system. Two examples are alim7101_wdt, which currently
> > > uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
> > > the arm_pm_restart function.
> > > 
> > > The existing mechanisms have a number of drawbacks. Typically only one scheme
> > > to restart the system is supported (at least if arm_pm_restart is used).
> > > At least in theory there can be mutliple means to restart the system, some of
> > > which may be less desirable (for example one mechanism may only reset the CPU,
> > > while another may reset the entire system). Using arm_pm_restart can also be
> > > racy if the function pointer is set from a driver, as the driver may be in
> > > the process of being unloaded when arm_pm_restart is called.
> > > Using the reboot notifier is always racy, as it is unknown if and when
> > > other functions using the reboot notifier have completed execution
> > > by the time the watchdog fires.
> > > 
> > > To solve the problem, introduce a system restart notifier. This notifier
> > > is expected to be called from the architecture specific machine_restart()
> > > function. Drivers providing system restart functionality (such as the watchdog
> > > drivers mentioned above) are expected to register with this notifier.
> > > 
> > > Patch 1 of this series implements the notifier function. Patches 2 and 3
> > > implement calling the notifier chain from arm and arm64 restart code.
> > > Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
> > > to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
> > > that no one gets the idea to implement a restart handler as module.
> > 
> > I think you need to restructure stuff somewhat, because I think
> > you've missed drivers/power/reset/ entirely, or at least you've
> > missed drivers/power/reset/restart-poweroff.c which calls
> > arm_pm_restart directly.  I'm not quite sure how we ended up with
> > that...
> 
> Yes, guess I missed (and did not really expect) that arm_pm_restart
> is called from multiple places.

Most of the ARM-specific code in drivers/power/reset/ consists of SoC
power-off/restart back-ends (e.g. vexpress-poweroff.c). Since there is
no generic pm_restart, we continued to use arm_pm_restart (also for
arm64 since we share some of the drivers). Maybe some driver model here
would help.

> What is restart-poweroff supposed to do in the first place, and why
> doesn't it call machine_restart() ? If it is what I think it is, ie
> a fallback for pm_power_off, it could be made generic and does not
> really have to depend on ARM.

I think this one pretends to do a power-off via restart. It could call
machine_restart() but this only passes the default reboot_mode to
arm_pm_restart().

-- 
Catalin

Re: [RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Guenter Roeck]

On 07/01/2014 04:24 AM, Catalin Marinas wrote:
> On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote:
>> On Mon, Jun 30, 2014 at 08:59:47PM +0100, Russell King - ARM Linux wrote:
>>> On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
>>>> Various drivers implement architecture and/or device specific means
>>>> to restart (reset) the system. Various mechanisms have been implemented
>>>> to support those schemes. The best known mechanism is arm_pm_restart,
>>>> which is a function pointer to be set either from platform specific code
>>>> or from drivers. Another mechanism is to use hardware watchdogs to issue
>>>> a reset; this mechanism is used if there is no other method available
>>>> to reset a board or system. Two examples are alim7101_wdt, which currently
>>>> uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
>>>> the arm_pm_restart function.
>>>>
>>>> The existing mechanisms have a number of drawbacks. Typically only one scheme
>>>> to restart the system is supported (at least if arm_pm_restart is used).
>>>> At least in theory there can be mutliple means to restart the system, some of
>>>> which may be less desirable (for example one mechanism may only reset the CPU,
>>>> while another may reset the entire system). Using arm_pm_restart can also be
>>>> racy if the function pointer is set from a driver, as the driver may be in
>>>> the process of being unloaded when arm_pm_restart is called.
>>>> Using the reboot notifier is always racy, as it is unknown if and when
>>>> other functions using the reboot notifier have completed execution
>>>> by the time the watchdog fires.
>>>>
>>>> To solve the problem, introduce a system restart notifier. This notifier
>>>> is expected to be called from the architecture specific machine_restart()
>>>> function. Drivers providing system restart functionality (such as the watchdog
>>>> drivers mentioned above) are expected to register with this notifier.
>>>>
>>>> Patch 1 of this series implements the notifier function. Patches 2 and 3
>>>> implement calling the notifier chain from arm and arm64 restart code.
>>>> Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
>>>> to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
>>>> that no one gets the idea to implement a restart handler as module.
>>>
>>> I think you need to restructure stuff somewhat, because I think
>>> you've missed drivers/power/reset/ entirely, or at least you've
>>> missed drivers/power/reset/restart-poweroff.c which calls
>>> arm_pm_restart directly.  I'm not quite sure how we ended up with
>>> that...
>>
>> Yes, guess I missed (and did not really expect) that arm_pm_restart
>> is called from multiple places.
>
> Most of the ARM-specific code in drivers/power/reset/ consists of SoC
> power-off/restart back-ends (e.g. vexpress-poweroff.c). Since there is
> no generic pm_restart, we continued to use arm_pm_restart (also for
> arm64 since we share some of the drivers). Maybe some driver model here
> would help.
>
>> What is restart-poweroff supposed to do in the first place, and why
>> doesn't it call machine_restart() ? If it is what I think it is, ie
>> a fallback for pm_power_off, it could be made generic and does not
>> really have to depend on ARM.
>
> I think this one pretends to do a power-off via restart. It could call
> machine_restart() but this only passes the default reboot_mode to
> arm_pm_restart().
>

Unless one sets reboot_mode first. See my proposed patch in
https://lkml.org/lkml/2014/6/30/792.

Guenter

[RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Guenter Roeck]

On 07/01/2014 04:24 AM, Catalin Marinas wrote:
> On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote:
>> On Mon, Jun 30, 2014 at 08:59:47PM +0100, Russell King - ARM Linux wrote:
>>> On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
>>>> Various drivers implement architecture and/or device specific means
>>>> to restart (reset) the system. Various mechanisms have been implemented
>>>> to support those schemes. The best known mechanism is arm_pm_restart,
>>>> which is a function pointer to be set either from platform specific code
>>>> or from drivers. Another mechanism is to use hardware watchdogs to issue
>>>> a reset; this mechanism is used if there is no other method available
>>>> to reset a board or system. Two examples are alim7101_wdt, which currently
>>>> uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
>>>> the arm_pm_restart function.
>>>>
>>>> The existing mechanisms have a number of drawbacks. Typically only one scheme
>>>> to restart the system is supported (at least if arm_pm_restart is used).
>>>> At least in theory there can be mutliple means to restart the system, some of
>>>> which may be less desirable (for example one mechanism may only reset the CPU,
>>>> while another may reset the entire system). Using arm_pm_restart can also be
>>>> racy if the function pointer is set from a driver, as the driver may be in
>>>> the process of being unloaded when arm_pm_restart is called.
>>>> Using the reboot notifier is always racy, as it is unknown if and when
>>>> other functions using the reboot notifier have completed execution
>>>> by the time the watchdog fires.
>>>>
>>>> To solve the problem, introduce a system restart notifier. This notifier
>>>> is expected to be called from the architecture specific machine_restart()
>>>> function. Drivers providing system restart functionality (such as the watchdog
>>>> drivers mentioned above) are expected to register with this notifier.
>>>>
>>>> Patch 1 of this series implements the notifier function. Patches 2 and 3
>>>> implement calling the notifier chain from arm and arm64 restart code.
>>>> Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
>>>> to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
>>>> that no one gets the idea to implement a restart handler as module.
>>>
>>> I think you need to restructure stuff somewhat, because I think
>>> you've missed drivers/power/reset/ entirely, or at least you've
>>> missed drivers/power/reset/restart-poweroff.c which calls
>>> arm_pm_restart directly.  I'm not quite sure how we ended up with
>>> that...
>>
>> Yes, guess I missed (and did not really expect) that arm_pm_restart
>> is called from multiple places.
>
> Most of the ARM-specific code in drivers/power/reset/ consists of SoC
> power-off/restart back-ends (e.g. vexpress-poweroff.c). Since there is
> no generic pm_restart, we continued to use arm_pm_restart (also for
> arm64 since we share some of the drivers). Maybe some driver model here
> would help.
>
>> What is restart-poweroff supposed to do in the first place, and why
>> doesn't it call machine_restart() ? If it is what I think it is, ie
>> a fallback for pm_power_off, it could be made generic and does not
>> really have to depend on ARM.
>
> I think this one pretends to do a power-off via restart. It could call
> machine_restart() but this only passes the default reboot_mode to
> arm_pm_restart().
>

Unless one sets reboot_mode first. See my proposed patch in
https://lkml.org/lkml/2014/6/30/792.

Guenter

Re: [RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Guenter Roeck]

On 07/01/2014 04:24 AM, Catalin Marinas wrote:
> On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote:
>> On Mon, Jun 30, 2014 at 08:59:47PM +0100, Russell King - ARM Linux wrote:
>>> On Mon, Jun 30, 2014 at 12:11:33PM -0700, Guenter Roeck wrote:
>>>> Various drivers implement architecture and/or device specific means
>>>> to restart (reset) the system. Various mechanisms have been implemented
>>>> to support those schemes. The best known mechanism is arm_pm_restart,
>>>> which is a function pointer to be set either from platform specific code
>>>> or from drivers. Another mechanism is to use hardware watchdogs to issue
>>>> a reset; this mechanism is used if there is no other method available
>>>> to reset a board or system. Two examples are alim7101_wdt, which currently
>>>> uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
>>>> the arm_pm_restart function.
>>>>
>>>> The existing mechanisms have a number of drawbacks. Typically only one scheme
>>>> to restart the system is supported (at least if arm_pm_restart is used).
>>>> At least in theory there can be mutliple means to restart the system, some of
>>>> which may be less desirable (for example one mechanism may only reset the CPU,
>>>> while another may reset the entire system). Using arm_pm_restart can also be
>>>> racy if the function pointer is set from a driver, as the driver may be in
>>>> the process of being unloaded when arm_pm_restart is called.
>>>> Using the reboot notifier is always racy, as it is unknown if and when
>>>> other functions using the reboot notifier have completed execution
>>>> by the time the watchdog fires.
>>>>
>>>> To solve the problem, introduce a system restart notifier. This notifier
>>>> is expected to be called from the architecture specific machine_restart()
>>>> function. Drivers providing system restart functionality (such as the watchdog
>>>> drivers mentioned above) are expected to register with this notifier.
>>>>
>>>> Patch 1 of this series implements the notifier function. Patches 2 and 3
>>>> implement calling the notifier chain from arm and arm64 restart code.
>>>> Patch 4 and 5 convert existing restart handlers in the watchdog subsystem
>>>> to use the restart notifier. Patch 6 unexports arm_pm_restart to ensure
>>>> that no one gets the idea to implement a restart handler as module.
>>>
>>> I think you need to restructure stuff somewhat, because I think
>>> you've missed drivers/power/reset/ entirely, or at least you've
>>> missed drivers/power/reset/restart-poweroff.c which calls
>>> arm_pm_restart directly.  I'm not quite sure how we ended up with
>>> that...
>>
>> Yes, guess I missed (and did not really expect) that arm_pm_restart
>> is called from multiple places.
>
> Most of the ARM-specific code in drivers/power/reset/ consists of SoC
> power-off/restart back-ends (e.g. vexpress-poweroff.c). Since there is
> no generic pm_restart, we continued to use arm_pm_restart (also for
> arm64 since we share some of the drivers). Maybe some driver model here
> would help.
>
>> What is restart-poweroff supposed to do in the first place, and why
>> doesn't it call machine_restart() ? If it is what I think it is, ie
>> a fallback for pm_power_off, it could be made generic and does not
>> really have to depend on ARM.
>
> I think this one pretends to do a power-off via restart. It could call
> machine_restart() but this only passes the default reboot_mode to
> arm_pm_restart().
>

Unless one sets reboot_mode first. See my proposed patch in
https://lkml.org/lkml/2014/6/30/792.

Guenter

Re: [RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Catalin Marinas]

On Tue, Jul 01, 2014 at 02:45:24PM +0100, Guenter Roeck wrote:
> On 07/01/2014 04:24 AM, Catalin Marinas wrote:
> > On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote:
> >> What is restart-poweroff supposed to do in the first place, and why
> >> doesn't it call machine_restart() ? If it is what I think it is, ie
> >> a fallback for pm_power_off, it could be made generic and does not
> >> really have to depend on ARM.
> >
> > I think this one pretends to do a power-off via restart. It could call
> > machine_restart() but this only passes the default reboot_mode to
> > arm_pm_restart().
> 
> Unless one sets reboot_mode first. See my proposed patch in
> https://lkml.org/lkml/2014/6/30/792.

It looks fine to me.

-- 
Catalin

[RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Catalin Marinas]

On Tue, Jul 01, 2014 at 02:45:24PM +0100, Guenter Roeck wrote:
> On 07/01/2014 04:24 AM, Catalin Marinas wrote:
> > On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote:
> >> What is restart-poweroff supposed to do in the first place, and why
> >> doesn't it call machine_restart() ? If it is what I think it is, ie
> >> a fallback for pm_power_off, it could be made generic and does not
> >> really have to depend on ARM.
> >
> > I think this one pretends to do a power-off via restart. It could call
> > machine_restart() but this only passes the default reboot_mode to
> > arm_pm_restart().
> 
> Unless one sets reboot_mode first. See my proposed patch in
> https://lkml.org/lkml/2014/6/30/792.

It looks fine to me.

-- 
Catalin

Re: [RFC PATCH 0/6] kernel: Add support for restart notifier call chain

[Catalin Marinas]

On Tue, Jul 01, 2014 at 02:45:24PM +0100, Guenter Roeck wrote:
> On 07/01/2014 04:24 AM, Catalin Marinas wrote:
> > On Mon, Jun 30, 2014 at 09:28:51PM +0100, Guenter Roeck wrote:
> >> What is restart-poweroff supposed to do in the first place, and why
> >> doesn't it call machine_restart() ? If it is what I think it is, ie
> >> a fallback for pm_power_off, it could be made generic and does not
> >> really have to depend on ARM.
> >
> > I think this one pretends to do a power-off via restart. It could call
> > machine_restart() but this only passes the default reboot_mode to
> > arm_pm_restart().
> 
> Unless one sets reboot_mode first. See my proposed patch in
> https://lkml.org/lkml/2014/6/30/792.

It looks fine to me.

-- 
Catalin