[PATCH v5 0/7] kernel: Add support for restart handler call chain

[PATCH v5 0/7] kernel: Add support for restart handler 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. Several other restart drivers for arm, all
directly calling arm_pm_restart, are in the process of being integrated
into the kernel. All those drivers would benefit from the new API.

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 multiple 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.

Introduce a system restart handler call chain to solve the described problems.
This call chain is expected to be executed 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 call chain. By using the priority field in the notifier block,
callers can control restart handler execution sequence and thus ensure that
the restart handler with the optimal restart capabilities for a given system
is called first.

Since the first revision of this patchset, a number of separate patch
submissions have been made which either depend on it or could make use of it.

http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
	registers three notifiers.
https://lkml.org/lkml/2014/7/8/962
	would benefit from it.

Patch 1 of this series implements the restart handler function. Patches 2 and 3
implement calling the restart handler chain from arm and arm64 restart code.

Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
directly but machine_restart. This is done to avoid calling arm_pm_restart
from more than one place. The change makes the driver architecture independent,
so it would be possible to drop the arm dependency from its Kconfig entry.

Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
that no one gets the idea to implement a restart handler as module.

---
v5: Rebased series to v3.16-rc5
    Function renames:
    register_restart_notifier -> register_restart_handler
    unregister_restart_notifier -> unregister_restart_handler
    kernel_restart_notify -> do_kernel_restart
v4: Document restart notifier priorities
    Select 128 as default priority for newly introduced notifiers
    Fix checkpatch warning (line too long) in moxart patch
v3: Drop RFC.
    Add kernel_restart_notify wrapper function to execute notifier
    Improve documentation.
    Move restart_notifier_list into kernel/reboot.c and make it static.
v2: Add patch 4.
    Only call blocking notifier call chain if arm_pm_restart was not set.

[PATCH v5 0/7] kernel: Add support for restart handler 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. Several other restart drivers for arm, all
directly calling arm_pm_restart, are in the process of being integrated
into the kernel. All those drivers would benefit from the new API.

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 multiple 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.

Introduce a system restart handler call chain to solve the described problems.
This call chain is expected to be executed 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 call chain. By using the priority field in the notifier block,
callers can control restart handler execution sequence and thus ensure that
the restart handler with the optimal restart capabilities for a given system
is called first.

Since the first revision of this patchset, a number of separate patch
submissions have been made which either depend on it or could make use of it.

http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
	registers three notifiers.
https://lkml.org/lkml/2014/7/8/962
	would benefit from it.

Patch 1 of this series implements the restart handler function. Patches 2 and 3
implement calling the restart handler chain from arm and arm64 restart code.

Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
directly but machine_restart. This is done to avoid calling arm_pm_restart
from more than one place. The change makes the driver architecture independent,
so it would be possible to drop the arm dependency from its Kconfig entry.

Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
that no one gets the idea to implement a restart handler as module.

---
v5: Rebased series to v3.16-rc5
    Function renames:
    register_restart_notifier -> register_restart_handler
    unregister_restart_notifier -> unregister_restart_handler
    kernel_restart_notify -> do_kernel_restart
v4: Document restart notifier priorities
    Select 128 as default priority for newly introduced notifiers
    Fix checkpatch warning (line too long) in moxart patch
v3: Drop RFC.
    Add kernel_restart_notify wrapper function to execute notifier
    Improve documentation.
    Move restart_notifier_list into kernel/reboot.c and make it static.
v2: Add patch 4.
    Only call blocking notifier call chain if arm_pm_restart was not set.

[PATCH v5 1/7] kernel: Add support for kernel restart handler 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 multiple 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.

Introduce a system restart handler call chain to solve the described problems.
This call chain is expected to be executed 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 call chain. By using the priority field in the notifier block,
callers can control restart handler execution sequence and thus ensure that
the restart handler with the optimal restart capabilities for a given system
is called first.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Function renames:
    register_restart_notifier -> register_restart_handler
    unregister_restart_notifier -> unregister_restart_handler
    kernel_restart_notify -> do_kernel_restart
v4: Document and suggest values for notifier priorities
v3: Add kernel_restart_notify wrapper function to execute notifier.
    Improve documentation.
    Move restart_notifier_list into kernel/reboot.c and make it static.
v2: No change.

 include/linux/reboot.h |  3 ++
 kernel/reboot.c        | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 48bf152..67fc8fc 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -38,6 +38,9 @@ extern int reboot_force;
 extern int register_reboot_notifier(struct notifier_block *);
 extern int unregister_reboot_notifier(struct notifier_block *);
 
+extern int register_restart_handler(struct notifier_block *);
+extern int unregister_restart_handler(struct notifier_block *);
+extern void do_kernel_restart(char *cmd);
 
 /*
  * Architecture-specific implementations of sys_reboot commands.
diff --git a/kernel/reboot.c b/kernel/reboot.c
index a3a9e24..33e8170 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -104,6 +104,87 @@ int unregister_reboot_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL(unregister_reboot_notifier);
 
+/*
+ *	Notifier list for kernel code which wants to be called
+ *	to restart the system.
+ */
+static BLOCKING_NOTIFIER_HEAD(restart_handler_list);
+
+/**
+ *	register_restart_handler - Register function to be called to reset
+ *				   the system
+ *	@nb: Info about handler function to be called
+ *	@nb->priority:	Handler priority. Handlers should follow the
+ *			following guidelines for setting priorities.
+ *			0:	Restart handler of last resort,
+ *				with limited restart capabilities
+ *			128:	Default restart handler; use if no other
+ *				restart handler is expected to be available,
+ *				and/or if restart functionality is
+ *				sufficient to restart the entire system
+ *			255:	Highest priority restart handler, will
+ *				preempt all other restart handlers
+ *
+ *	Registers a function with the list of functions
+ *	to be called to restart the system.
+ *
+ *	Registered functions will be called from machine_restart as last
+ *	step of the restart sequence (if the architecture specific
+ *	machine_restart function calls do_kernel_restart - see below
+ *	for details).
+ *	Registered functions are expected to restart the system immediately.
+ *	If more than one function is registered, the restart handler priority
+ *	selects which function will be called first.
+ *
+ *	Restart handlers are expected to be registered from non-architecture
+ *	code, typically from drivers. A typical use case would be a system
+ *	where restart functionality is provided through a watchdog. Multiple
+ *	restart handlers may exist; for example, one restart handler might
+ *	restart the entire system, while another only restarts the CPU.
+ *	In such cases, the restart handler which only restarts part of the
+ *	hardware is expected to register with low priority to ensure that
+ *	it only runs if no other means to restart the system is available.
+ *
+ *	Currently always returns zero, as blocking_notifier_chain_register()
+ *	always returns zero.
+ */
+int register_restart_handler(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&restart_handler_list, nb);
+}
+EXPORT_SYMBOL(register_restart_handler);
+
+/**
+ *	unregister_restart_handler - Unregister previously registered
+ *				     restart handler
+ *	@nb: Hook to be unregistered
+ *
+ *	Unregisters a previously registered restart handler function.
+ *
+ *	Returns zero on success, or %-ENOENT on failure.
+ */
+int unregister_restart_handler(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&restart_handler_list, nb);
+}
+EXPORT_SYMBOL(unregister_restart_handler);
+
+/**
+ *	do_kernel_restart - Execute kernel restart handler call chain
+ *
+ *	Calls functions registered with register_restart_handler.
+ *
+ *	Expected to be called from machine_restart as last step of the restart
+ *	sequence.
+ *
+ *	Restarts the system immediately if a restart handler function has been
+ *	registered. Otherwise does nothing.
+ */
+void do_kernel_restart(char *cmd)
+{
+	blocking_notifier_call_chain(&restart_handler_list, reboot_mode, cmd);
+}
+
 void migrate_to_reboot_cpu(void)
 {
 	/* The boot cpu is always logical cpu 0 */
-- 
1.9.1

[PATCH v5 1/7] kernel: Add support for kernel restart handler 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 multiple 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.

Introduce a system restart handler call chain to solve the described problems.
This call chain is expected to be executed 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 call chain. By using the priority field in the notifier block,
callers can control restart handler execution sequence and thus ensure that
the restart handler with the optimal restart capabilities for a given system
is called first.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Function renames:
    register_restart_notifier -> register_restart_handler
    unregister_restart_notifier -> unregister_restart_handler
    kernel_restart_notify -> do_kernel_restart
v4: Document and suggest values for notifier priorities
v3: Add kernel_restart_notify wrapper function to execute notifier.
    Improve documentation.
    Move restart_notifier_list into kernel/reboot.c and make it static.
v2: No change.

 include/linux/reboot.h |  3 ++
 kernel/reboot.c        | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 48bf152..67fc8fc 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -38,6 +38,9 @@ extern int reboot_force;
 extern int register_reboot_notifier(struct notifier_block *);
 extern int unregister_reboot_notifier(struct notifier_block *);
 
+extern int register_restart_handler(struct notifier_block *);
+extern int unregister_restart_handler(struct notifier_block *);
+extern void do_kernel_restart(char *cmd);
 
 /*
  * Architecture-specific implementations of sys_reboot commands.
diff --git a/kernel/reboot.c b/kernel/reboot.c
index a3a9e24..33e8170 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -104,6 +104,87 @@ int unregister_reboot_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL(unregister_reboot_notifier);
 
+/*
+ *	Notifier list for kernel code which wants to be called
+ *	to restart the system.
+ */
+static BLOCKING_NOTIFIER_HEAD(restart_handler_list);
+
+/**
+ *	register_restart_handler - Register function to be called to reset
+ *				   the system
+ *	@nb: Info about handler function to be called
+ *	@nb->priority:	Handler priority. Handlers should follow the
+ *			following guidelines for setting priorities.
+ *			0:	Restart handler of last resort,
+ *				with limited restart capabilities
+ *			128:	Default restart handler; use if no other
+ *				restart handler is expected to be available,
+ *				and/or if restart functionality is
+ *				sufficient to restart the entire system
+ *			255:	Highest priority restart handler, will
+ *				preempt all other restart handlers
+ *
+ *	Registers a function with the list of functions
+ *	to be called to restart the system.
+ *
+ *	Registered functions will be called from machine_restart as last
+ *	step of the restart sequence (if the architecture specific
+ *	machine_restart function calls do_kernel_restart - see below
+ *	for details).
+ *	Registered functions are expected to restart the system immediately.
+ *	If more than one function is registered, the restart handler priority
+ *	selects which function will be called first.
+ *
+ *	Restart handlers are expected to be registered from non-architecture
+ *	code, typically from drivers. A typical use case would be a system
+ *	where restart functionality is provided through a watchdog. Multiple
+ *	restart handlers may exist; for example, one restart handler might
+ *	restart the entire system, while another only restarts the CPU.
+ *	In such cases, the restart handler which only restarts part of the
+ *	hardware is expected to register with low priority to ensure that
+ *	it only runs if no other means to restart the system is available.
+ *
+ *	Currently always returns zero, as blocking_notifier_chain_register()
+ *	always returns zero.
+ */
+int register_restart_handler(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&restart_handler_list, nb);
+}
+EXPORT_SYMBOL(register_restart_handler);
+
+/**
+ *	unregister_restart_handler - Unregister previously registered
+ *				     restart handler
+ *	@nb: Hook to be unregistered
+ *
+ *	Unregisters a previously registered restart handler function.
+ *
+ *	Returns zero on success, or %-ENOENT on failure.
+ */
+int unregister_restart_handler(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&restart_handler_list, nb);
+}
+EXPORT_SYMBOL(unregister_restart_handler);
+
+/**
+ *	do_kernel_restart - Execute kernel restart handler call chain
+ *
+ *	Calls functions registered with register_restart_handler.
+ *
+ *	Expected to be called from machine_restart as last step of the restart
+ *	sequence.
+ *
+ *	Restarts the system immediately if a restart handler function has been
+ *	registered. Otherwise does nothing.
+ */
+void do_kernel_restart(char *cmd)
+{
+	blocking_notifier_call_chain(&restart_handler_list, reboot_mode, cmd);
+}
+
 void migrate_to_reboot_cpu(void)
 {
 	/* The boot cpu is always logical cpu 0 */
-- 
1.9.1

[PATCH v5 2/7] arm64: Support restart through restart handler call chain

[Guenter Roeck]

The kernel core now supports a restart handler call chain to restart
the system. Call it if arm_pm_restart is not set.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Renamed restart function to do_kernel_restart
v4: No change.
v3: Use wrapper function to execute notifier call chain.
v2: Only call notifier call chain if arm_pm_restart is not set.
    Do not include linux/watchdog.h.

 arch/arm64/kernel/process.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 43b7c34..9591e60 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -174,6 +174,8 @@ void machine_restart(char *cmd)
 	/* Now call the architecture specific reboot code. */
 	if (arm_pm_restart)
 		arm_pm_restart(reboot_mode, cmd);
+	else
+		do_kernel_restart(cmd);
 
 	/*
 	 * Whoops - the architecture was unable to reboot.
-- 
1.9.1

[PATCH v5 2/7] arm64: Support restart through restart handler call chain

[Guenter Roeck]

The kernel core now supports a restart handler call chain to restart
the system. Call it if arm_pm_restart is not set.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Renamed restart function to do_kernel_restart
v4: No change.
v3: Use wrapper function to execute notifier call chain.
v2: Only call notifier call chain if arm_pm_restart is not set.
    Do not include linux/watchdog.h.

 arch/arm64/kernel/process.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 43b7c34..9591e60 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -174,6 +174,8 @@ void machine_restart(char *cmd)
 	/* Now call the architecture specific reboot code. */
 	if (arm_pm_restart)
 		arm_pm_restart(reboot_mode, cmd);
+	else
+		do_kernel_restart(cmd);
 
 	/*
 	 * Whoops - the architecture was unable to reboot.
-- 
1.9.1

[PATCH v5 3/7] arm: Support restart through restart handler call chain

[Guenter Roeck]

The kernel core now supports a restart handler 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. Only call the kernel restart handler
if arm_pm_restart is not set.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Renamed restart function to do_kernel_restart
v4: No change.
v3: Use wrapper function to execute notifier call chain.
v2: Only call notifier call chain if arm_pm_restart is not set.
    Do not include linux/watchdog.h.

 arch/arm/kernel/process.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 81ef686..84ca0d5 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -230,7 +230,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);
+	else
+		do_kernel_restart(cmd);
 
 	/* Give a grace period for failure to restart of 1s */
 	mdelay(1000);
-- 
1.9.1

[PATCH v5 3/7] arm: Support restart through restart handler call chain

[Guenter Roeck]

The kernel core now supports a restart handler 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. Only call the kernel restart handler
if arm_pm_restart is not set.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: Renamed restart function to do_kernel_restart
v4: No change.
v3: Use wrapper function to execute notifier call chain.
v2: Only call notifier call chain if arm_pm_restart is not set.
    Do not include linux/watchdog.h.

 arch/arm/kernel/process.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 81ef686..84ca0d5 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -230,7 +230,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);
+	else
+		do_kernel_restart(cmd);
 
 	/* Give a grace period for failure to restart of 1s */
 	mdelay(1000);
-- 
1.9.1

[PATCH v5 4/7] power/restart: Call machine_restart instead of arm_pm_restart

[Guenter Roeck]

machine_restart is supported on non-ARM platforms, and and ultimately calls
arm_pm_restart, so dont call arm_pm_restart directly but use the more
generic function.

Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: No change.
v4: No change.
v3: No change.
v2: Added patch.

 drivers/power/reset/restart-poweroff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/power/reset/restart-poweroff.c b/drivers/power/reset/restart-poweroff.c
index 5758033..47f10eb 100644
--- a/drivers/power/reset/restart-poweroff.c
+++ b/drivers/power/reset/restart-poweroff.c
@@ -20,7 +20,8 @@
 
 static void restart_poweroff_do_poweroff(void)
 {
-	arm_pm_restart(REBOOT_HARD, NULL);
+	reboot_mode = REBOOT_HARD;
+	machine_restart(NULL);
 }
 
 static int restart_poweroff_probe(struct platform_device *pdev)
-- 
1.9.1

[PATCH v5 4/7] power/restart: Call machine_restart instead of arm_pm_restart

[Guenter Roeck]

machine_restart is supported on non-ARM platforms, and and ultimately calls
arm_pm_restart, so dont call arm_pm_restart directly but use the more
generic function.

Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v5: No change.
v4: No change.
v3: No change.
v2: Added patch.

 drivers/power/reset/restart-poweroff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/power/reset/restart-poweroff.c b/drivers/power/reset/restart-poweroff.c
index 5758033..47f10eb 100644
--- a/drivers/power/reset/restart-poweroff.c
+++ b/drivers/power/reset/restart-poweroff.c
@@ -20,7 +20,8 @@
 
 static void restart_poweroff_do_poweroff(void)
 {
-	arm_pm_restart(REBOOT_HARD, NULL);
+	reboot_mode = REBOOT_HARD;
+	machine_restart(NULL);
 }
 
 static int restart_poweroff_probe(struct platform_device *pdev)
-- 
1.9.1

[PATCH v5 5/7] watchdog: moxart: Register restart handler with kernel restart handler

[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>
---
v5: Functions and variables renamed: *notifier -> *handler
v4: Set notifier priority to 128.
v3: Move struct notifier_block into struct moxart_wdt_dev.
    Drop static variable previously needed to access struct moxart_wdt_dev
    from notifier function; use container_of instead.
v2: No change.

 drivers/watchdog/moxart_wdt.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
index 4aa3a8a..a64405b 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
@@ -29,17 +29,22 @@ struct moxart_wdt_dev {
 	struct watchdog_device dev;
 	void __iomem *base;
 	unsigned int clock_frequency;
+	struct notifier_block restart_handler;
 };
 
-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_handle(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);
+	struct moxart_wdt_dev *moxart_wdt = container_of(this,
+							 struct moxart_wdt_dev,
+							 restart_handler);
+	writel(1, moxart_wdt->base + REG_COUNT);
+	writel(0x5ab9, moxart_wdt->base + REG_MODE);
+	writel(0x03, moxart_wdt->base + REG_ENABLE);
+
+	return NOTIFY_DONE;
 }
 
 static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
@@ -136,8 +141,12 @@ static int moxart_wdt_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	moxart_restart_ctx = moxart_wdt;
-	arm_pm_restart = moxart_wdt_restart;
+	moxart_wdt->restart_handler.notifier_call = moxart_restart_handle;
+	moxart_wdt->restart_handler.priority = 128;
+	err = register_restart_handler(&moxart_wdt->restart_handler);
+	if (err)
+		dev_err(dev, "cannot register restart 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_handler(&moxart_wdt->restart_handler);
 	moxart_wdt_stop(&moxart_wdt->dev);
-	watchdog_unregister_device(&moxart_wdt->dev);
 
 	return 0;
 }
-- 
1.9.1

[PATCH v5 5/7] watchdog: moxart: Register restart handler with kernel restart handler

[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>
---
v5: Functions and variables renamed: *notifier -> *handler
v4: Set notifier priority to 128.
v3: Move struct notifier_block into struct moxart_wdt_dev.
    Drop static variable previously needed to access struct moxart_wdt_dev
    from notifier function; use container_of instead.
v2: No change.

 drivers/watchdog/moxart_wdt.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
index 4aa3a8a..a64405b 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
@@ -29,17 +29,22 @@ struct moxart_wdt_dev {
 	struct watchdog_device dev;
 	void __iomem *base;
 	unsigned int clock_frequency;
+	struct notifier_block restart_handler;
 };
 
-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_handle(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);
+	struct moxart_wdt_dev *moxart_wdt = container_of(this,
+							 struct moxart_wdt_dev,
+							 restart_handler);
+	writel(1, moxart_wdt->base + REG_COUNT);
+	writel(0x5ab9, moxart_wdt->base + REG_MODE);
+	writel(0x03, moxart_wdt->base + REG_ENABLE);
+
+	return NOTIFY_DONE;
 }
 
 static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
@@ -136,8 +141,12 @@ static int moxart_wdt_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	moxart_restart_ctx = moxart_wdt;
-	arm_pm_restart = moxart_wdt_restart;
+	moxart_wdt->restart_handler.notifier_call = moxart_restart_handle;
+	moxart_wdt->restart_handler.priority = 128;
+	err = register_restart_handler(&moxart_wdt->restart_handler);
+	if (err)
+		dev_err(dev, "cannot register restart 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_handler(&moxart_wdt->restart_handler);
 	moxart_wdt_stop(&moxart_wdt->dev);
-	watchdog_unregister_device(&moxart_wdt->dev);
 
 	return 0;
 }
-- 
1.9.1

[PATCH v5 6/7] watchdog: alim7101: Register restart handler with kernel restart handler

[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>
---
v5: Function and variable renames: *notifier -> *handler
v4: Set restart notifier priority to 128.
v3: No change
v2: No change

 drivers/watchdog/alim7101_wdt.c | 42 +++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/alim7101_wdt.c b/drivers/watchdog/alim7101_wdt.c
index 996b2f7..665e0e7 100644
--- a/drivers/watchdog/alim7101_wdt.c
+++ b/drivers/watchdog/alim7101_wdt.c
@@ -301,6 +301,28 @@ static struct miscdevice wdt_miscdev = {
 	.fops	=	&wdt_fops,
 };
 
+static int wdt_restart_handle(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_handler = {
+	.notifier_call = wdt_restart_handle,
+	.priority = 128,
+};
+
 /*
  *	Notifier for system down
  */
@@ -311,15 +333,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 +351,7 @@ static void __exit alim7101_wdt_unload(void)
 	/* Deregister */
 	misc_deregister(&wdt_miscdev);
 	unregister_reboot_notifier(&wdt_notifier);
+	unregister_restart_handler(&wdt_restart_handler);
 	pci_dev_put(alim7101_pmu);
 }
 
@@ -390,11 +404,17 @@ static int __init alim7101_wdt_init(void)
 		goto err_out;
 	}
 
+	rc = register_restart_handler(&wdt_restart_handler);
+	if (rc) {
+		pr_err("cannot register restart handler (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 +424,8 @@ static int __init alim7101_wdt_init(void)
 		timeout, nowayout);
 	return 0;
 
+err_out_restart:
+	unregister_restart_handler(&wdt_restart_handler);
 err_out_reboot:
 	unregister_reboot_notifier(&wdt_notifier);
 err_out:
-- 
1.9.1

[PATCH v5 6/7] watchdog: alim7101: Register restart handler with kernel restart handler

[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>
---
v5: Function and variable renames: *notifier -> *handler
v4: Set restart notifier priority to 128.
v3: No change
v2: No change

 drivers/watchdog/alim7101_wdt.c | 42 +++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/alim7101_wdt.c b/drivers/watchdog/alim7101_wdt.c
index 996b2f7..665e0e7 100644
--- a/drivers/watchdog/alim7101_wdt.c
+++ b/drivers/watchdog/alim7101_wdt.c
@@ -301,6 +301,28 @@ static struct miscdevice wdt_miscdev = {
 	.fops	=	&wdt_fops,
 };
 
+static int wdt_restart_handle(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_handler = {
+	.notifier_call = wdt_restart_handle,
+	.priority = 128,
+};
+
 /*
  *	Notifier for system down
  */
@@ -311,15 +333,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 +351,7 @@ static void __exit alim7101_wdt_unload(void)
 	/* Deregister */
 	misc_deregister(&wdt_miscdev);
 	unregister_reboot_notifier(&wdt_notifier);
+	unregister_restart_handler(&wdt_restart_handler);
 	pci_dev_put(alim7101_pmu);
 }
 
@@ -390,11 +404,17 @@ static int __init alim7101_wdt_init(void)
 		goto err_out;
 	}
 
+	rc = register_restart_handler(&wdt_restart_handler);
+	if (rc) {
+		pr_err("cannot register restart handler (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 +424,8 @@ static int __init alim7101_wdt_init(void)
 		timeout, nowayout);
 	return 0;
 
+err_out_restart:
+	unregister_restart_handler(&wdt_restart_handler);
 err_out_reboot:
 	unregister_reboot_notifier(&wdt_notifier);
 err_out:
-- 
1.9.1

[PATCH v5 7/7] 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>
---
v5: No change
v4: No change
v3: No change
v2: No change

 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 84ca0d5..27bdf11 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -125,7 +125,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 9591e60..f5fddff 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -92,7 +92,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

[PATCH v5 7/7] 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>
---
v5: No change
v4: No change
v3: No change
v2: No change

 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 84ca0d5..27bdf11 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -125,7 +125,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 9591e60..f5fddff 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -92,7 +92,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: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Catalin Marinas]

On Fri, Jul 18, 2014 at 08:34:09AM +0100, Guenter Roeck wrote:
> Patch 1 of this series implements the restart handler function. Patches 2 and 3
> implement calling the restart handler chain from arm and arm64 restart code.
> 
> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
> directly but machine_restart. This is done to avoid calling arm_pm_restart
> from more than one place. The change makes the driver architecture independent,
> so it would be possible to drop the arm dependency from its Kconfig entry.
> 
> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
> that no one gets the idea to implement a restart handler as module.

For the current patches:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Do you plan to convert more of the power/drivers/reset/ code?

BTW, is it worth doing something similar for pm_power_off (there is
generic code calling it directly, so slightly more complicated than
pm_restart)?

Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Catalin Marinas]

On Fri, Jul 18, 2014 at 08:34:09AM +0100, Guenter Roeck wrote:
> Patch 1 of this series implements the restart handler function. Patches 2 and 3
> implement calling the restart handler chain from arm and arm64 restart code.
> 
> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
> directly but machine_restart. This is done to avoid calling arm_pm_restart
> from more than one place. The change makes the driver architecture independent,
> so it would be possible to drop the arm dependency from its Kconfig entry.
> 
> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
> that no one gets the idea to implement a restart handler as module.

For the current patches:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Do you plan to convert more of the power/drivers/reset/ code?

BTW, is it worth doing something similar for pm_power_off (there is
generic code calling it directly, so slightly more complicated than
pm_restart)?

[PATCH v5 0/7] kernel: Add support for restart handler call chain

[Catalin Marinas]

On Fri, Jul 18, 2014 at 08:34:09AM +0100, Guenter Roeck wrote:
> Patch 1 of this series implements the restart handler function. Patches 2 and 3
> implement calling the restart handler chain from arm and arm64 restart code.
> 
> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
> directly but machine_restart. This is done to avoid calling arm_pm_restart
> from more than one place. The change makes the driver architecture independent,
> so it would be possible to drop the arm dependency from its Kconfig entry.
> 
> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
> that no one gets the idea to implement a restart handler as module.

For the current patches:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Do you plan to convert more of the power/drivers/reset/ code?

BTW, is it worth doing something similar for pm_power_off (there is
generic code calling it directly, so slightly more complicated than
pm_restart)?

Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

Hi Catalin,

On 07/21/2014 05:30 AM, Catalin Marinas wrote:
> On Fri, Jul 18, 2014 at 08:34:09AM +0100, Guenter Roeck wrote:
>> Patch 1 of this series implements the restart handler function. Patches 2 and 3
>> implement calling the restart handler chain from arm and arm64 restart code.
>>
>> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
>> directly but machine_restart. This is done to avoid calling arm_pm_restart
>> from more than one place. The change makes the driver architecture independent,
>> so it would be possible to drop the arm dependency from its Kconfig entry.
>>
>> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
>> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
>> that no one gets the idea to implement a restart handler as module.
>
> For the current patches:
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
> Do you plan to convert more of the power/drivers/reset/ code?
>

Once this series has been accepted, I at least wanted to give it a try.

> BTW, is it worth doing something similar for pm_power_off (there is
> generic code calling it directly, so slightly more complicated than
> pm_restart)?

I have not thought about it yet.

A quick glance suggests it is much more widely used than the reset handler.
It looks like the original idea, like with machine_restart, was to have
one function per architecture, but it seems that is widely abused.

Looks pretty messy to me. Maybe after this one is done, if there is interest
and, more importantly, the need to do something about it.

Thanks,
Guenter

Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

Hi Catalin,

On 07/21/2014 05:30 AM, Catalin Marinas wrote:
> On Fri, Jul 18, 2014 at 08:34:09AM +0100, Guenter Roeck wrote:
>> Patch 1 of this series implements the restart handler function. Patches 2 and 3
>> implement calling the restart handler chain from arm and arm64 restart code.
>>
>> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
>> directly but machine_restart. This is done to avoid calling arm_pm_restart
>> from more than one place. The change makes the driver architecture independent,
>> so it would be possible to drop the arm dependency from its Kconfig entry.
>>
>> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
>> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
>> that no one gets the idea to implement a restart handler as module.
>
> For the current patches:
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
> Do you plan to convert more of the power/drivers/reset/ code?
>

Once this series has been accepted, I at least wanted to give it a try.

> BTW, is it worth doing something similar for pm_power_off (there is
> generic code calling it directly, so slightly more complicated than
> pm_restart)?

I have not thought about it yet.

A quick glance suggests it is much more widely used than the reset handler.
It looks like the original idea, like with machine_restart, was to have
one function per architecture, but it seems that is widely abused.

Looks pretty messy to me. Maybe after this one is done, if there is interest
and, more importantly, the need to do something about it.

Thanks,
Guenter

[PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

Hi Catalin,

On 07/21/2014 05:30 AM, Catalin Marinas wrote:
> On Fri, Jul 18, 2014 at 08:34:09AM +0100, Guenter Roeck wrote:
>> Patch 1 of this series implements the restart handler function. Patches 2 and 3
>> implement calling the restart handler chain from arm and arm64 restart code.
>>
>> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
>> directly but machine_restart. This is done to avoid calling arm_pm_restart
>> from more than one place. The change makes the driver architecture independent,
>> so it would be possible to drop the arm dependency from its Kconfig entry.
>>
>> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
>> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
>> that no one gets the idea to implement a restart handler as module.
>
> For the current patches:
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>
> Do you plan to convert more of the power/drivers/reset/ code?
>

Once this series has been accepted, I at least wanted to give it a try.

> BTW, is it worth doing something similar for pm_power_off (there is
> generic code calling it directly, so slightly more complicated than
> pm_restart)?

I have not thought about it yet.

A quick glance suggests it is much more widely used than the reset handler.
It looks like the original idea, like with machine_restart, was to have
one function per architecture, but it seems that is widely abused.

Looks pretty messy to me. Maybe after this one is done, if there is interest
and, more importantly, the need to do something about it.

Thanks,
Guenter

Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

Any idea, anyone, what might be necessary to get this series accepted ?

Guenter

On Fri, Jul 18, 2014 at 12:34:09AM -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. Several other restart drivers for arm, all
> directly calling arm_pm_restart, are in the process of being integrated
> into the kernel. All those drivers would benefit from the new API.
> 
> 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 multiple 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.
> 
> Introduce a system restart handler call chain to solve the described problems.
> This call chain is expected to be executed 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 call chain. By using the priority field in the notifier block,
> callers can control restart handler execution sequence and thus ensure that
> the restart handler with the optimal restart capabilities for a given system
> is called first.
> 
> Since the first revision of this patchset, a number of separate patch
> submissions have been made which either depend on it or could make use of it.
> 
> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
> 	registers three notifiers.
> https://lkml.org/lkml/2014/7/8/962
> 	would benefit from it.
> 
> Patch 1 of this series implements the restart handler function. Patches 2 and 3
> implement calling the restart handler chain from arm and arm64 restart code.
> 
> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
> directly but machine_restart. This is done to avoid calling arm_pm_restart
> from more than one place. The change makes the driver architecture independent,
> so it would be possible to drop the arm dependency from its Kconfig entry.
> 
> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
> that no one gets the idea to implement a restart handler as module.
> 
> ---
> v5: Rebased series to v3.16-rc5
>     Function renames:
>     register_restart_notifier -> register_restart_handler
>     unregister_restart_notifier -> unregister_restart_handler
>     kernel_restart_notify -> do_kernel_restart
> v4: Document restart notifier priorities
>     Select 128 as default priority for newly introduced notifiers
>     Fix checkpatch warning (line too long) in moxart patch
> v3: Drop RFC.
>     Add kernel_restart_notify wrapper function to execute notifier
>     Improve documentation.
>     Move restart_notifier_list into kernel/reboot.c and make it static.
> v2: Add patch 4.
>     Only call blocking notifier call chain if arm_pm_restart was not set.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

Any idea, anyone, what might be necessary to get this series accepted ?

Guenter

On Fri, Jul 18, 2014 at 12:34:09AM -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. Several other restart drivers for arm, all
> directly calling arm_pm_restart, are in the process of being integrated
> into the kernel. All those drivers would benefit from the new API.
> 
> 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 multiple 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.
> 
> Introduce a system restart handler call chain to solve the described problems.
> This call chain is expected to be executed 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 call chain. By using the priority field in the notifier block,
> callers can control restart handler execution sequence and thus ensure that
> the restart handler with the optimal restart capabilities for a given system
> is called first.
> 
> Since the first revision of this patchset, a number of separate patch
> submissions have been made which either depend on it or could make use of it.
> 
> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
> 	registers three notifiers.
> https://lkml.org/lkml/2014/7/8/962
> 	would benefit from it.
> 
> Patch 1 of this series implements the restart handler function. Patches 2 and 3
> implement calling the restart handler chain from arm and arm64 restart code.
> 
> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
> directly but machine_restart. This is done to avoid calling arm_pm_restart
> from more than one place. The change makes the driver architecture independent,
> so it would be possible to drop the arm dependency from its Kconfig entry.
> 
> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
> that no one gets the idea to implement a restart handler as module.
> 
> ---
> v5: Rebased series to v3.16-rc5
>     Function renames:
>     register_restart_notifier -> register_restart_handler
>     unregister_restart_notifier -> unregister_restart_handler
>     kernel_restart_notify -> do_kernel_restart
> v4: Document restart notifier priorities
>     Select 128 as default priority for newly introduced notifiers
>     Fix checkpatch warning (line too long) in moxart patch
> v3: Drop RFC.
>     Add kernel_restart_notify wrapper function to execute notifier
>     Improve documentation.
>     Move restart_notifier_list into kernel/reboot.c and make it static.
> v2: Add patch 4.
>     Only call blocking notifier call chain if arm_pm_restart was not set.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

On 07/18/2014 12:34 AM, 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. Several other restart drivers for arm, all
> directly calling arm_pm_restart, are in the process of being integrated
> into the kernel. All those drivers would benefit from the new API.
>
> 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 multiple 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.
>
> Introduce a system restart handler call chain to solve the described problems.
> This call chain is expected to be executed 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 call chain. By using the priority field in the notifier block,
> callers can control restart handler execution sequence and thus ensure that
> the restart handler with the optimal restart capabilities for a given system
> is called first.
>
> Since the first revision of this patchset, a number of separate patch
> submissions have been made which either depend on it or could make use of it.
>
> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
> 	registers three notifiers.
> https://lkml.org/lkml/2014/7/8/962
> 	would benefit from it.
>
> Patch 1 of this series implements the restart handler function. Patches 2 and 3
> implement calling the restart handler chain from arm and arm64 restart code.
>
> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
> directly but machine_restart. This is done to avoid calling arm_pm_restart
> from more than one place. The change makes the driver architecture independent,
> so it would be possible to drop the arm dependency from its Kconfig entry.
>
> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
> that no one gets the idea to implement a restart handler as module.
>
> ---
> v5: Rebased series to v3.16-rc5
>      Function renames:
>      register_restart_notifier -> register_restart_handler
>      unregister_restart_notifier -> unregister_restart_handler
>      kernel_restart_notify -> do_kernel_restart
> v4: Document restart notifier priorities
>      Select 128 as default priority for newly introduced notifiers
>      Fix checkpatch warning (line too long) in moxart patch
> v3: Drop RFC.
>      Add kernel_restart_notify wrapper function to execute notifier
>      Improve documentation.
>      Move restart_notifier_list into kernel/reboot.c and make it static.
> v2: Add patch 4.
>      Only call blocking notifier call chain if arm_pm_restart was not set.
> --

To get more test coverage, this series plus a few add-on patches which
depend on it are now available in the restart-staging branch of my
repository at kernel.org [1]. The branch is currently based on 3.16-rc7.
Extensive build test results are available at [2]; look for the column
marked 'restart-staging' on the far right of the tables.

I would encourage everyone interested in this series to send me Reviewed-by:
or at least Acked-by: tags. Note that I removed all of the earlier tags since
I feel that the changes made subsequently warrant updated tags. An Acked-by:
from affected maintainers would also be very helpful.

Thanks,
Guenter

---
[1] https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/
[2] http://server.roeck-us.net:8010/builders

[PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

On 07/18/2014 12:34 AM, 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. Several other restart drivers for arm, all
> directly calling arm_pm_restart, are in the process of being integrated
> into the kernel. All those drivers would benefit from the new API.
>
> 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 multiple 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.
>
> Introduce a system restart handler call chain to solve the described problems.
> This call chain is expected to be executed 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 call chain. By using the priority field in the notifier block,
> callers can control restart handler execution sequence and thus ensure that
> the restart handler with the optimal restart capabilities for a given system
> is called first.
>
> Since the first revision of this patchset, a number of separate patch
> submissions have been made which either depend on it or could make use of it.
>
> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
> 	registers three notifiers.
> https://lkml.org/lkml/2014/7/8/962
> 	would benefit from it.
>
> Patch 1 of this series implements the restart handler function. Patches 2 and 3
> implement calling the restart handler chain from arm and arm64 restart code.
>
> Patch 4 modifies the restart-poweroff driver to no longer call arm_pm_restart
> directly but machine_restart. This is done to avoid calling arm_pm_restart
> from more than one place. The change makes the driver architecture independent,
> so it would be possible to drop the arm dependency from its Kconfig entry.
>
> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
> that no one gets the idea to implement a restart handler as module.
>
> ---
> v5: Rebased series to v3.16-rc5
>      Function renames:
>      register_restart_notifier -> register_restart_handler
>      unregister_restart_notifier -> unregister_restart_handler
>      kernel_restart_notify -> do_kernel_restart
> v4: Document restart notifier priorities
>      Select 128 as default priority for newly introduced notifiers
>      Fix checkpatch warning (line too long) in moxart patch
> v3: Drop RFC.
>      Add kernel_restart_notify wrapper function to execute notifier
>      Improve documentation.
>      Move restart_notifier_list into kernel/reboot.c and make it static.
> v2: Add patch 4.
>      Only call blocking notifier call chain if arm_pm_restart was not set.
> --

To get more test coverage, this series plus a few add-on patches which
depend on it are now available in the restart-staging branch of my
repository at kernel.org [1]. The branch is currently based on 3.16-rc7.
Extensive build test results are available at [2]; look for the column
marked 'restart-staging' on the far right of the tables.

I would encourage everyone interested in this series to send me Reviewed-by:
or at least Acked-by: tags. Note that I removed all of the earlier tags since
I feel that the changes made subsequently warrant updated tags. An Acked-by:
from affected maintainers would also be very helpful.

Thanks,
Guenter

---
[1] https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/
[2] http://server.roeck-us.net:8010/builders

Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Heiko Stübner]

Hi Guenter,

Am Dienstag, 29. Juli 2014, 18:50:47 schrieb Guenter Roeck:
> On 07/18/2014 12:34 AM, 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. Several other restart drivers for
> > arm, all directly calling arm_pm_restart, are in the process of being
> > integrated into the kernel. All those drivers would benefit from the new
> > API.
> > 
> > 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 multiple 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.
> > 
> > Introduce a system restart handler call chain to solve the described
> > problems. This call chain is expected to be executed 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 call chain. By using the
> > priority field in the notifier block, callers can control restart handler
> > execution sequence and thus ensure that the restart handler with the
> > optimal restart capabilities for a given system is called first.
> > 
> > Since the first revision of this patchset, a number of separate patch
> > submissions have been made which either depend on it or could make use of
> > it.
> > 
> > http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
> > 
> > 	registers three notifiers.
> > 
> > https://lkml.org/lkml/2014/7/8/962
> > 
> > 	would benefit from it.
> > 
> > Patch 1 of this series implements the restart handler function. Patches 2
> > and 3 implement calling the restart handler chain from arm and arm64
> > restart code.
> > 
> > Patch 4 modifies the restart-poweroff driver to no longer call
> > arm_pm_restart directly but machine_restart. This is done to avoid
> > calling arm_pm_restart from more than one place. The change makes the
> > driver architecture independent, so it would be possible to drop the arm
> > dependency from its Kconfig entry.
> > 
> > Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
> > to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
> > that no one gets the idea to implement a restart handler as module.
> > 
> > ---
> > v5: Rebased series to v3.16-rc5
> > 
> >      Function renames:
> >      register_restart_notifier -> register_restart_handler
> >      unregister_restart_notifier -> unregister_restart_handler
> >      kernel_restart_notify -> do_kernel_restart
> > 
> > v4: Document restart notifier priorities
> > 
> >      Select 128 as default priority for newly introduced notifiers
> >      Fix checkpatch warning (line too long) in moxart patch
> > 
> > v3: Drop RFC.
> > 
> >      Add kernel_restart_notify wrapper function to execute notifier
> >      Improve documentation.
> >      Move restart_notifier_list into kernel/reboot.c and make it static.
> > 
> > v2: Add patch 4.
> > 
> >      Only call blocking notifier call chain if arm_pm_restart was not set.
> > 
> > --
> 
> To get more test coverage, this series plus a few add-on patches which
> depend on it are now available in the restart-staging branch of my
> repository at kernel.org [1]. The branch is currently based on 3.16-rc7.
> Extensive build test results are available at [2]; look for the column
> marked 'restart-staging' on the far right of the tables.
> 
> I would encourage everyone interested in this series to send me Reviewed-by:
> or at least Acked-by: tags. Note that I removed all of the earlier tags
> since I feel that the changes made subsequently warrant updated tags. An
> Acked-by: from affected maintainers would also be very helpful.

Thanks for adapting my Samsung restart-patches to the API changes.

The one thing I found is, in
	"clk: samsung: register restart handlers for s3c2412 and s3c2443"
you seem to have forgotten the priority in the clk-s3c2443.c part while in 
clk-s3c2412.c it is present.

And I'm not sure if there shouldn't be some sort of delay, to give the 
watchdog some time to work, as Tomasz suggested in my initial submission?


I'll update the Rockchip specific restart-patches after the merge window, when 
the core clock support has landed.

And I still like the whole concept very much, so
Acked-by: Heiko Stuebner <heiko@sntech.de>


Heiko

[PATCH v5 0/7] kernel: Add support for restart handler call chain

[Heiko Stübner]

Hi Guenter,

Am Dienstag, 29. Juli 2014, 18:50:47 schrieb Guenter Roeck:
> On 07/18/2014 12:34 AM, 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. Several other restart drivers for
> > arm, all directly calling arm_pm_restart, are in the process of being
> > integrated into the kernel. All those drivers would benefit from the new
> > API.
> > 
> > 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 multiple 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.
> > 
> > Introduce a system restart handler call chain to solve the described
> > problems. This call chain is expected to be executed 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 call chain. By using the
> > priority field in the notifier block, callers can control restart handler
> > execution sequence and thus ensure that the restart handler with the
> > optimal restart capabilities for a given system is called first.
> > 
> > Since the first revision of this patchset, a number of separate patch
> > submissions have been made which either depend on it or could make use of
> > it.
> > 
> > http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
> > 
> > 	registers three notifiers.
> > 
> > https://lkml.org/lkml/2014/7/8/962
> > 
> > 	would benefit from it.
> > 
> > Patch 1 of this series implements the restart handler function. Patches 2
> > and 3 implement calling the restart handler chain from arm and arm64
> > restart code.
> > 
> > Patch 4 modifies the restart-poweroff driver to no longer call
> > arm_pm_restart directly but machine_restart. This is done to avoid
> > calling arm_pm_restart from more than one place. The change makes the
> > driver architecture independent, so it would be possible to drop the arm
> > dependency from its Kconfig entry.
> > 
> > Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
> > to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
> > that no one gets the idea to implement a restart handler as module.
> > 
> > ---
> > v5: Rebased series to v3.16-rc5
> > 
> >      Function renames:
> >      register_restart_notifier -> register_restart_handler
> >      unregister_restart_notifier -> unregister_restart_handler
> >      kernel_restart_notify -> do_kernel_restart
> > 
> > v4: Document restart notifier priorities
> > 
> >      Select 128 as default priority for newly introduced notifiers
> >      Fix checkpatch warning (line too long) in moxart patch
> > 
> > v3: Drop RFC.
> > 
> >      Add kernel_restart_notify wrapper function to execute notifier
> >      Improve documentation.
> >      Move restart_notifier_list into kernel/reboot.c and make it static.
> > 
> > v2: Add patch 4.
> > 
> >      Only call blocking notifier call chain if arm_pm_restart was not set.
> > 
> > --
> 
> To get more test coverage, this series plus a few add-on patches which
> depend on it are now available in the restart-staging branch of my
> repository at kernel.org [1]. The branch is currently based on 3.16-rc7.
> Extensive build test results are available at [2]; look for the column
> marked 'restart-staging' on the far right of the tables.
> 
> I would encourage everyone interested in this series to send me Reviewed-by:
> or at least Acked-by: tags. Note that I removed all of the earlier tags
> since I feel that the changes made subsequently warrant updated tags. An
> Acked-by: from affected maintainers would also be very helpful.

Thanks for adapting my Samsung restart-patches to the API changes.

The one thing I found is, in
	"clk: samsung: register restart handlers for s3c2412 and s3c2443"
you seem to have forgotten the priority in the clk-s3c2443.c part while in 
clk-s3c2412.c it is present.

And I'm not sure if there shouldn't be some sort of delay, to give the 
watchdog some time to work, as Tomasz suggested in my initial submission?


I'll update the Rockchip specific restart-patches after the merge window, when 
the core clock support has landed.

And I still like the whole concept very much, so
Acked-by: Heiko Stuebner <heiko@sntech.de>


Heiko

Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

On 07/30/2014 05:16 AM, Heiko Stübner wrote:
> Hi Guenter,
>
> Am Dienstag, 29. Juli 2014, 18:50:47 schrieb Guenter Roeck:
>> On 07/18/2014 12:34 AM, 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. Several other restart drivers for
>>> arm, all directly calling arm_pm_restart, are in the process of being
>>> integrated into the kernel. All those drivers would benefit from the new
>>> API.
>>>
>>> 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 multiple 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.
>>>
>>> Introduce a system restart handler call chain to solve the described
>>> problems. This call chain is expected to be executed 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 call chain. By using the
>>> priority field in the notifier block, callers can control restart handler
>>> execution sequence and thus ensure that the restart handler with the
>>> optimal restart capabilities for a given system is called first.
>>>
>>> Since the first revision of this patchset, a number of separate patch
>>> submissions have been made which either depend on it or could make use of
>>> it.
>>>
>>> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
>>>
>>> 	registers three notifiers.
>>>
>>> https://lkml.org/lkml/2014/7/8/962
>>>
>>> 	would benefit from it.
>>>
>>> Patch 1 of this series implements the restart handler function. Patches 2
>>> and 3 implement calling the restart handler chain from arm and arm64
>>> restart code.
>>>
>>> Patch 4 modifies the restart-poweroff driver to no longer call
>>> arm_pm_restart directly but machine_restart. This is done to avoid
>>> calling arm_pm_restart from more than one place. The change makes the
>>> driver architecture independent, so it would be possible to drop the arm
>>> dependency from its Kconfig entry.
>>>
>>> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
>>> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
>>> that no one gets the idea to implement a restart handler as module.
>>>
>>> ---
>>> v5: Rebased series to v3.16-rc5
>>>
>>>       Function renames:
>>>       register_restart_notifier -> register_restart_handler
>>>       unregister_restart_notifier -> unregister_restart_handler
>>>       kernel_restart_notify -> do_kernel_restart
>>>
>>> v4: Document restart notifier priorities
>>>
>>>       Select 128 as default priority for newly introduced notifiers
>>>       Fix checkpatch warning (line too long) in moxart patch
>>>
>>> v3: Drop RFC.
>>>
>>>       Add kernel_restart_notify wrapper function to execute notifier
>>>       Improve documentation.
>>>       Move restart_notifier_list into kernel/reboot.c and make it static.
>>>
>>> v2: Add patch 4.
>>>
>>>       Only call blocking notifier call chain if arm_pm_restart was not set.
>>>
>>> --
>>
>> To get more test coverage, this series plus a few add-on patches which
>> depend on it are now available in the restart-staging branch of my
>> repository at kernel.org [1]. The branch is currently based on 3.16-rc7.
>> Extensive build test results are available at [2]; look for the column
>> marked 'restart-staging' on the far right of the tables.
>>
>> I would encourage everyone interested in this series to send me Reviewed-by:
>> or at least Acked-by: tags. Note that I removed all of the earlier tags
>> since I feel that the changes made subsequently warrant updated tags. An
>> Acked-by: from affected maintainers would also be very helpful.
>
> Thanks for adapting my Samsung restart-patches to the API changes.
>
> The one thing I found is, in
> 	"clk: samsung: register restart handlers for s3c2412 and s3c2443"
> you seem to have forgotten the priority in the clk-s3c2443.c part while in
> clk-s3c2412.c it is present.
>
Yes, you are correct. I fixed that after I got the message from Fengguang's
robot. Which means that my auto-builder misses to build that file. Too bad
that arm:allmodconfig fails with other errors and is unusable. Do you know
if there is a defconfig which builds clk-s3c2443.c ?

> And I'm not sure if there shouldn't be some sort of delay, to give the
> watchdog some time to work, as Tomasz suggested in my initial submission?
>
In your case the reset handler is in the clock code, itn's it ? Question
would be if the write to SWRST results in an immediate reset or if it needs
a delay. From the context, it looks to me as if reaction would be immediate,
but obviously I don't have the specification so that is just a wild guess.

>
> I'll update the Rockchip specific restart-patches after the merge window, when
> the core clock support has landed.
>
> And I still like the whole concept very much, so
> Acked-by: Heiko Stuebner <heiko@sntech.de>

Thanks!

Guenter

Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

On 07/30/2014 05:16 AM, Heiko Stübner wrote:
> Hi Guenter,
>
> Am Dienstag, 29. Juli 2014, 18:50:47 schrieb Guenter Roeck:
>> On 07/18/2014 12:34 AM, 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. Several other restart drivers for
>>> arm, all directly calling arm_pm_restart, are in the process of being
>>> integrated into the kernel. All those drivers would benefit from the new
>>> API.
>>>
>>> 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 multiple 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.
>>>
>>> Introduce a system restart handler call chain to solve the described
>>> problems. This call chain is expected to be executed 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 call chain. By using the
>>> priority field in the notifier block, callers can control restart handler
>>> execution sequence and thus ensure that the restart handler with the
>>> optimal restart capabilities for a given system is called first.
>>>
>>> Since the first revision of this patchset, a number of separate patch
>>> submissions have been made which either depend on it or could make use of
>>> it.
>>>
>>> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
>>>
>>> 	registers three notifiers.
>>>
>>> https://lkml.org/lkml/2014/7/8/962
>>>
>>> 	would benefit from it.
>>>
>>> Patch 1 of this series implements the restart handler function. Patches 2
>>> and 3 implement calling the restart handler chain from arm and arm64
>>> restart code.
>>>
>>> Patch 4 modifies the restart-poweroff driver to no longer call
>>> arm_pm_restart directly but machine_restart. This is done to avoid
>>> calling arm_pm_restart from more than one place. The change makes the
>>> driver architecture independent, so it would be possible to drop the arm
>>> dependency from its Kconfig entry.
>>>
>>> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
>>> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
>>> that no one gets the idea to implement a restart handler as module.
>>>
>>> ---
>>> v5: Rebased series to v3.16-rc5
>>>
>>>       Function renames:
>>>       register_restart_notifier -> register_restart_handler
>>>       unregister_restart_notifier -> unregister_restart_handler
>>>       kernel_restart_notify -> do_kernel_restart
>>>
>>> v4: Document restart notifier priorities
>>>
>>>       Select 128 as default priority for newly introduced notifiers
>>>       Fix checkpatch warning (line too long) in moxart patch
>>>
>>> v3: Drop RFC.
>>>
>>>       Add kernel_restart_notify wrapper function to execute notifier
>>>       Improve documentation.
>>>       Move restart_notifier_list into kernel/reboot.c and make it static.
>>>
>>> v2: Add patch 4.
>>>
>>>       Only call blocking notifier call chain if arm_pm_restart was not set.
>>>
>>> --
>>
>> To get more test coverage, this series plus a few add-on patches which
>> depend on it are now available in the restart-staging branch of my
>> repository at kernel.org [1]. The branch is currently based on 3.16-rc7.
>> Extensive build test results are available at [2]; look for the column
>> marked 'restart-staging' on the far right of the tables.
>>
>> I would encourage everyone interested in this series to send me Reviewed-by:
>> or at least Acked-by: tags. Note that I removed all of the earlier tags
>> since I feel that the changes made subsequently warrant updated tags. An
>> Acked-by: from affected maintainers would also be very helpful.
>
> Thanks for adapting my Samsung restart-patches to the API changes.
>
> The one thing I found is, in
> 	"clk: samsung: register restart handlers for s3c2412 and s3c2443"
> you seem to have forgotten the priority in the clk-s3c2443.c part while in
> clk-s3c2412.c it is present.
>
Yes, you are correct. I fixed that after I got the message from Fengguang's
robot. Which means that my auto-builder misses to build that file. Too bad
that arm:allmodconfig fails with other errors and is unusable. Do you know
if there is a defconfig which builds clk-s3c2443.c ?

> And I'm not sure if there shouldn't be some sort of delay, to give the
> watchdog some time to work, as Tomasz suggested in my initial submission?
>
In your case the reset handler is in the clock code, itn's it ? Question
would be if the write to SWRST results in an immediate reset or if it needs
a delay. From the context, it looks to me as if reaction would be immediate,
but obviously I don't have the specification so that is just a wild guess.

>
> I'll update the Rockchip specific restart-patches after the merge window, when
> the core clock support has landed.
>
> And I still like the whole concept very much, so
> Acked-by: Heiko Stuebner <heiko@sntech.de>

Thanks!

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

On 07/30/2014 05:16 AM, Heiko St?bner wrote:
> Hi Guenter,
>
> Am Dienstag, 29. Juli 2014, 18:50:47 schrieb Guenter Roeck:
>> On 07/18/2014 12:34 AM, 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. Several other restart drivers for
>>> arm, all directly calling arm_pm_restart, are in the process of being
>>> integrated into the kernel. All those drivers would benefit from the new
>>> API.
>>>
>>> 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 multiple 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.
>>>
>>> Introduce a system restart handler call chain to solve the described
>>> problems. This call chain is expected to be executed 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 call chain. By using the
>>> priority field in the notifier block, callers can control restart handler
>>> execution sequence and thus ensure that the restart handler with the
>>> optimal restart capabilities for a given system is called first.
>>>
>>> Since the first revision of this patchset, a number of separate patch
>>> submissions have been made which either depend on it or could make use of
>>> it.
>>>
>>> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
>>>
>>> 	registers three notifiers.
>>>
>>> https://lkml.org/lkml/2014/7/8/962
>>>
>>> 	would benefit from it.
>>>
>>> Patch 1 of this series implements the restart handler function. Patches 2
>>> and 3 implement calling the restart handler chain from arm and arm64
>>> restart code.
>>>
>>> Patch 4 modifies the restart-poweroff driver to no longer call
>>> arm_pm_restart directly but machine_restart. This is done to avoid
>>> calling arm_pm_restart from more than one place. The change makes the
>>> driver architecture independent, so it would be possible to drop the arm
>>> dependency from its Kconfig entry.
>>>
>>> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
>>> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
>>> that no one gets the idea to implement a restart handler as module.
>>>
>>> ---
>>> v5: Rebased series to v3.16-rc5
>>>
>>>       Function renames:
>>>       register_restart_notifier -> register_restart_handler
>>>       unregister_restart_notifier -> unregister_restart_handler
>>>       kernel_restart_notify -> do_kernel_restart
>>>
>>> v4: Document restart notifier priorities
>>>
>>>       Select 128 as default priority for newly introduced notifiers
>>>       Fix checkpatch warning (line too long) in moxart patch
>>>
>>> v3: Drop RFC.
>>>
>>>       Add kernel_restart_notify wrapper function to execute notifier
>>>       Improve documentation.
>>>       Move restart_notifier_list into kernel/reboot.c and make it static.
>>>
>>> v2: Add patch 4.
>>>
>>>       Only call blocking notifier call chain if arm_pm_restart was not set.
>>>
>>> --
>>
>> To get more test coverage, this series plus a few add-on patches which
>> depend on it are now available in the restart-staging branch of my
>> repository at kernel.org [1]. The branch is currently based on 3.16-rc7.
>> Extensive build test results are available at [2]; look for the column
>> marked 'restart-staging' on the far right of the tables.
>>
>> I would encourage everyone interested in this series to send me Reviewed-by:
>> or at least Acked-by: tags. Note that I removed all of the earlier tags
>> since I feel that the changes made subsequently warrant updated tags. An
>> Acked-by: from affected maintainers would also be very helpful.
>
> Thanks for adapting my Samsung restart-patches to the API changes.
>
> The one thing I found is, in
> 	"clk: samsung: register restart handlers for s3c2412 and s3c2443"
> you seem to have forgotten the priority in the clk-s3c2443.c part while in
> clk-s3c2412.c it is present.
>
Yes, you are correct. I fixed that after I got the message from Fengguang's
robot. Which means that my auto-builder misses to build that file. Too bad
that arm:allmodconfig fails with other errors and is unusable. Do you know
if there is a defconfig which builds clk-s3c2443.c ?

> And I'm not sure if there shouldn't be some sort of delay, to give the
> watchdog some time to work, as Tomasz suggested in my initial submission?
>
In your case the reset handler is in the clock code, itn's it ? Question
would be if the write to SWRST results in an immediate reset or if it needs
a delay. From the context, it looks to me as if reaction would be immediate,
but obviously I don't have the specification so that is just a wild guess.

>
> I'll update the Rockchip specific restart-patches after the merge window, when
> the core clock support has landed.
>
> And I still like the whole concept very much, so
> Acked-by: Heiko Stuebner <heiko@sntech.de>

Thanks!

Guenter

Re: [PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

On 07/30/2014 07:10 AM, Guenter Roeck wrote:
> On 07/30/2014 05:16 AM, Heiko Stübner wrote:
>> Hi Guenter,
>>
>> Am Dienstag, 29. Juli 2014, 18:50:47 schrieb Guenter Roeck:
>>> On 07/18/2014 12:34 AM, 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. Several other restart drivers for
>>>> arm, all directly calling arm_pm_restart, are in the process of being
>>>> integrated into the kernel. All those drivers would benefit from the new
>>>> API.
>>>>
>>>> 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 multiple 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.
>>>>
>>>> Introduce a system restart handler call chain to solve the described
>>>> problems. This call chain is expected to be executed 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 call chain. By using the
>>>> priority field in the notifier block, callers can control restart handler
>>>> execution sequence and thus ensure that the restart handler with the
>>>> optimal restart capabilities for a given system is called first.
>>>>
>>>> Since the first revision of this patchset, a number of separate patch
>>>> submissions have been made which either depend on it or could make use of
>>>> it.
>>>>
>>>> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
>>>>
>>>>     registers three notifiers.
>>>>
>>>> https://lkml.org/lkml/2014/7/8/962
>>>>
>>>>     would benefit from it.
>>>>
>>>> Patch 1 of this series implements the restart handler function. Patches 2
>>>> and 3 implement calling the restart handler chain from arm and arm64
>>>> restart code.
>>>>
>>>> Patch 4 modifies the restart-poweroff driver to no longer call
>>>> arm_pm_restart directly but machine_restart. This is done to avoid
>>>> calling arm_pm_restart from more than one place. The change makes the
>>>> driver architecture independent, so it would be possible to drop the arm
>>>> dependency from its Kconfig entry.
>>>>
>>>> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
>>>> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
>>>> that no one gets the idea to implement a restart handler as module.
>>>>
>>>> ---
>>>> v5: Rebased series to v3.16-rc5
>>>>
>>>>       Function renames:
>>>>       register_restart_notifier -> register_restart_handler
>>>>       unregister_restart_notifier -> unregister_restart_handler
>>>>       kernel_restart_notify -> do_kernel_restart
>>>>
>>>> v4: Document restart notifier priorities
>>>>
>>>>       Select 128 as default priority for newly introduced notifiers
>>>>       Fix checkpatch warning (line too long) in moxart patch
>>>>
>>>> v3: Drop RFC.
>>>>
>>>>       Add kernel_restart_notify wrapper function to execute notifier
>>>>       Improve documentation.
>>>>       Move restart_notifier_list into kernel/reboot.c and make it static.
>>>>
>>>> v2: Add patch 4.
>>>>
>>>>       Only call blocking notifier call chain if arm_pm_restart was not set.
>>>>
>>>> --
>>>
>>> To get more test coverage, this series plus a few add-on patches which
>>> depend on it are now available in the restart-staging branch of my
>>> repository at kernel.org [1]. The branch is currently based on 3.16-rc7.
>>> Extensive build test results are available at [2]; look for the column
>>> marked 'restart-staging' on the far right of the tables.
>>>
>>> I would encourage everyone interested in this series to send me Reviewed-by:
>>> or at least Acked-by: tags. Note that I removed all of the earlier tags
>>> since I feel that the changes made subsequently warrant updated tags. An
>>> Acked-by: from affected maintainers would also be very helpful.
>>
>> Thanks for adapting my Samsung restart-patches to the API changes.
>>
>> The one thing I found is, in
>>     "clk: samsung: register restart handlers for s3c2412 and s3c2443"
>> you seem to have forgotten the priority in the clk-s3c2443.c part while in
>> clk-s3c2412.c it is present.
>>
> Yes, you are correct. I fixed that after I got the message from Fengguang's
> robot. Which means that my auto-builder misses to build that file. Too bad
> that arm:allmodconfig fails with other errors and is unusable. Do you know
> if there is a defconfig which builds clk-s3c2443.c ?
>
>> And I'm not sure if there shouldn't be some sort of delay, to give the
>> watchdog some time to work, as Tomasz suggested in my initial submission?
>>
> In your case the reset handler is in the clock code, itn's it ? Question
> would be if the write to SWRST results in an immediate reset or if it needs
> a delay. From the context, it looks to me as if reaction would be immediate,
> but obviously I don't have the specification so that is just a wild guess.
>

After re-reading your patches, I guess you refer to the s3c2410 changes.
Yes, that could use a delay. Care to send an updated patch ?

Thanks,
Guenter

[PATCH v5 0/7] kernel: Add support for restart handler call chain

[Guenter Roeck]

On 07/30/2014 07:10 AM, Guenter Roeck wrote:
> On 07/30/2014 05:16 AM, Heiko St?bner wrote:
>> Hi Guenter,
>>
>> Am Dienstag, 29. Juli 2014, 18:50:47 schrieb Guenter Roeck:
>>> On 07/18/2014 12:34 AM, 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. Several other restart drivers for
>>>> arm, all directly calling arm_pm_restart, are in the process of being
>>>> integrated into the kernel. All those drivers would benefit from the new
>>>> API.
>>>>
>>>> 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 multiple 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.
>>>>
>>>> Introduce a system restart handler call chain to solve the described
>>>> problems. This call chain is expected to be executed 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 call chain. By using the
>>>> priority field in the notifier block, callers can control restart handler
>>>> execution sequence and thus ensure that the restart handler with the
>>>> optimal restart capabilities for a given system is called first.
>>>>
>>>> Since the first revision of this patchset, a number of separate patch
>>>> submissions have been made which either depend on it or could make use of
>>>> it.
>>>>
>>>> http://www.spinics.net/linux/lists/arm-kernel/msg344796.html
>>>>
>>>>     registers three notifiers.
>>>>
>>>> https://lkml.org/lkml/2014/7/8/962
>>>>
>>>>     would benefit from it.
>>>>
>>>> Patch 1 of this series implements the restart handler function. Patches 2
>>>> and 3 implement calling the restart handler chain from arm and arm64
>>>> restart code.
>>>>
>>>> Patch 4 modifies the restart-poweroff driver to no longer call
>>>> arm_pm_restart directly but machine_restart. This is done to avoid
>>>> calling arm_pm_restart from more than one place. The change makes the
>>>> driver architecture independent, so it would be possible to drop the arm
>>>> dependency from its Kconfig entry.
>>>>
>>>> Patch 5 and 6 convert existing restart handlers in the watchdog subsystem
>>>> to use the restart handler. Patch 7 unexports arm_pm_restart to ensure
>>>> that no one gets the idea to implement a restart handler as module.
>>>>
>>>> ---
>>>> v5: Rebased series to v3.16-rc5
>>>>
>>>>       Function renames:
>>>>       register_restart_notifier -> register_restart_handler
>>>>       unregister_restart_notifier -> unregister_restart_handler
>>>>       kernel_restart_notify -> do_kernel_restart
>>>>
>>>> v4: Document restart notifier priorities
>>>>
>>>>       Select 128 as default priority for newly introduced notifiers
>>>>       Fix checkpatch warning (line too long) in moxart patch
>>>>
>>>> v3: Drop RFC.
>>>>
>>>>       Add kernel_restart_notify wrapper function to execute notifier
>>>>       Improve documentation.
>>>>       Move restart_notifier_list into kernel/reboot.c and make it static.
>>>>
>>>> v2: Add patch 4.
>>>>
>>>>       Only call blocking notifier call chain if arm_pm_restart was not set.
>>>>
>>>> --
>>>
>>> To get more test coverage, this series plus a few add-on patches which
>>> depend on it are now available in the restart-staging branch of my
>>> repository at kernel.org [1]. The branch is currently based on 3.16-rc7.
>>> Extensive build test results are available at [2]; look for the column
>>> marked 'restart-staging' on the far right of the tables.
>>>
>>> I would encourage everyone interested in this series to send me Reviewed-by:
>>> or at least Acked-by: tags. Note that I removed all of the earlier tags
>>> since I feel that the changes made subsequently warrant updated tags. An
>>> Acked-by: from affected maintainers would also be very helpful.
>>
>> Thanks for adapting my Samsung restart-patches to the API changes.
>>
>> The one thing I found is, in
>>     "clk: samsung: register restart handlers for s3c2412 and s3c2443"
>> you seem to have forgotten the priority in the clk-s3c2443.c part while in
>> clk-s3c2412.c it is present.
>>
> Yes, you are correct. I fixed that after I got the message from Fengguang's
> robot. Which means that my auto-builder misses to build that file. Too bad
> that arm:allmodconfig fails with other errors and is unusable. Do you know
> if there is a defconfig which builds clk-s3c2443.c ?
>
>> And I'm not sure if there shouldn't be some sort of delay, to give the
>> watchdog some time to work, as Tomasz suggested in my initial submission?
>>
> In your case the reset handler is in the clock code, itn's it ? Question
> would be if the write to SWRST results in an immediate reset or if it needs
> a delay. From the context, it looks to me as if reaction would be immediate,
> but obviously I don't have the specification so that is just a wild guess.
>

After re-reading your patches, I guess you refer to the s3c2410 changes.
Yes, that could use a delay. Care to send an updated patch ?

Thanks,
Guenter

Re: [PATCH v5 1/7] kernel: Add support for kernel restart handler call chain

[Stephen Boyd]

On 07/18/14 00:34, Guenter Roeck wrote:
> +/**
> + *	do_kernel_restart - Execute kernel restart handler call chain
> + *
> + *	Calls functions registered with register_restart_handler.
> + *
> + *	Expected to be called from machine_restart as last step of the restart
> + *	sequence.
> + *
> + *	Restarts the system immediately if a restart handler function has been
> + *	registered. Otherwise does nothing.
> + */
> +void do_kernel_restart(char *cmd)
> +{
> +	blocking_notifier_call_chain(&restart_handler_list, reboot_mode, cmd);
> +}

Has this been tested with scheduling while atomic checking
(CONFIG_DEBUG_ATOMIC_SLEEP) or lockdep (CONFIG_PROVE_LOCKING)? ARM would
call this with irqs disabled and blocking_notifier_call_chain() would
spit out a warning when it tries to down_read() on it's semaphore. This
should probably be an atomic notifier chain instead.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v5 1/7] kernel: Add support for kernel restart handler call chain

[Stephen Boyd]

On 07/18/14 00:34, Guenter Roeck wrote:
> +/**
> + *	do_kernel_restart - Execute kernel restart handler call chain
> + *
> + *	Calls functions registered with register_restart_handler.
> + *
> + *	Expected to be called from machine_restart as last step of the restart
> + *	sequence.
> + *
> + *	Restarts the system immediately if a restart handler function has been
> + *	registered. Otherwise does nothing.
> + */
> +void do_kernel_restart(char *cmd)
> +{
> +	blocking_notifier_call_chain(&restart_handler_list, reboot_mode, cmd);
> +}

Has this been tested with scheduling while atomic checking
(CONFIG_DEBUG_ATOMIC_SLEEP) or lockdep (CONFIG_PROVE_LOCKING)? ARM would
call this with irqs disabled and blocking_notifier_call_chain() would
spit out a warning when it tries to down_read() on it's semaphore. This
should probably be an atomic notifier chain instead.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v5 1/7] kernel: Add support for kernel restart handler call chain

[Guenter Roeck]

On 07/30/2014 01:18 PM, Stephen Boyd wrote:
> On 07/18/14 00:34, Guenter Roeck wrote:
>> +/**
>> + *	do_kernel_restart - Execute kernel restart handler call chain
>> + *
>> + *	Calls functions registered with register_restart_handler.
>> + *
>> + *	Expected to be called from machine_restart as last step of the restart
>> + *	sequence.
>> + *
>> + *	Restarts the system immediately if a restart handler function has been
>> + *	registered. Otherwise does nothing.
>> + */
>> +void do_kernel_restart(char *cmd)
>> +{
>> +	blocking_notifier_call_chain(&restart_handler_list, reboot_mode, cmd);
>> +}
>
> Has this been tested with scheduling while atomic checking
> (CONFIG_DEBUG_ATOMIC_SLEEP) or lockdep (CONFIG_PROVE_LOCKING)? ARM would
> call this with irqs disabled and blocking_notifier_call_chain() would
> spit out a warning when it tries to down_read() on it's semaphore. This
> should probably be an atomic notifier chain instead.
>

Good point. I'll update the code accordingly.

I'll wait for additional comments until early next week to avoid unnecessary
churn.

Thanks,
Guenter

[PATCH v5 1/7] kernel: Add support for kernel restart handler call chain

[Guenter Roeck]

On 07/30/2014 01:18 PM, Stephen Boyd wrote:
> On 07/18/14 00:34, Guenter Roeck wrote:
>> +/**
>> + *	do_kernel_restart - Execute kernel restart handler call chain
>> + *
>> + *	Calls functions registered with register_restart_handler.
>> + *
>> + *	Expected to be called from machine_restart as last step of the restart
>> + *	sequence.
>> + *
>> + *	Restarts the system immediately if a restart handler function has been
>> + *	registered. Otherwise does nothing.
>> + */
>> +void do_kernel_restart(char *cmd)
>> +{
>> +	blocking_notifier_call_chain(&restart_handler_list, reboot_mode, cmd);
>> +}
>
> Has this been tested with scheduling while atomic checking
> (CONFIG_DEBUG_ATOMIC_SLEEP) or lockdep (CONFIG_PROVE_LOCKING)? ARM would
> call this with irqs disabled and blocking_notifier_call_chain() would
> spit out a warning when it tries to down_read() on it's semaphore. This
> should probably be an atomic notifier chain instead.
>

Good point. I'll update the code accordingly.

I'll wait for additional comments until early next week to avoid unnecessary
churn.

Thanks,
Guenter

[PATCH v5.1 9/7] watchdog: s3c2410: add restart handler

[Heiko Stübner]

On a lot of Samsung systems the watchdog is responsible for restarting the
system and until now this code was contained in plat-samsung/watchdog-reset.c .

With the introduction of the restart handlers, this code can now move into
driver itself, removing the need for arch-specific code.

Tested on a S3C2442 based GTA02
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
[Guenter Roeck: Update to latest API; do not require static context variable]
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
changes since v5:
- reintroduce the mdelay(500) from the platform code, to make sure the
  watchdog has time to assert, as suggested by Tomasz Figa

 drivers/watchdog/s3c2410_wdt.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 7c6ccd0..2955aef 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -41,6 +41,8 @@
 #include <linux/of.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/reboot.h>
+#include <linux/delay.h>
 
 #define S3C2410_WTCON		0x00
 #define S3C2410_WTDAT		0x04
@@ -128,6 +130,7 @@ struct s3c2410_wdt {
 	unsigned long		wtdat_save;
 	struct watchdog_device	wdt_device;
 	struct notifier_block	freq_transition;
+	struct notifier_block	restart_handler;
 	struct s3c2410_wdt_variant *drv_data;
 	struct regmap *pmureg;
 };
@@ -438,6 +441,31 @@ static inline void s3c2410wdt_cpufreq_deregister(struct s3c2410_wdt *wdt)
 }
 #endif
 
+static int s3c2410wdt_restart(struct notifier_block *this,
+			      unsigned long mode, void *cmd)
+{
+	struct s3c2410_wdt *wdt = container_of(this, struct s3c2410_wdt,
+					       restart_handler);
+	void __iomem *wdt_base = wdt->reg_base;
+
+	/* disable watchdog, to be safe  */
+	writel(0, wdt_base + S3C2410_WTCON);
+
+	/* put initial values into count and data */
+	writel(0x80, wdt_base + S3C2410_WTCNT);
+	writel(0x80, wdt_base + S3C2410_WTDAT);
+
+	/* set the watchdog to go and reset... */
+	writel(S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV16 |
+		S3C2410_WTCON_RSTEN | S3C2410_WTCON_PRESCALE(0x20),
+		wdt_base + S3C2410_WTCON);
+
+	/* wait for reset to assert... */
+	mdelay(500);
+
+	return NOTIFY_DONE;
+}
+
 static inline unsigned int s3c2410wdt_get_bootstatus(struct s3c2410_wdt *wdt)
 {
 	unsigned int rst_stat;
@@ -592,6 +620,12 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, wdt);
 
+	wdt->restart_handler.notifier_call = s3c2410wdt_restart;
+	wdt->restart_handler.priority = 128;
+	ret = register_restart_handler(&wdt->restart_handler);
+	if (ret)
+		pr_err("cannot register restart handler, %d\n", ret);
+
 	/* print out a statement of readiness */
 
 	wtcon = readl(wdt->reg_base + S3C2410_WTCON);
@@ -621,6 +655,8 @@ static int s3c2410wdt_remove(struct platform_device *dev)
 	int ret;
 	struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
 
+	unregister_restart_handler(&wdt->restart_handler);
+
 	ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
 	if (ret < 0)
 		return ret;
-- 
2.0.1

[PATCH v5.1 9/7] watchdog: s3c2410: add restart handler

[Heiko Stübner]

On a lot of Samsung systems the watchdog is responsible for restarting the
system and until now this code was contained in plat-samsung/watchdog-reset.c .

With the introduction of the restart handlers, this code can now move into
driver itself, removing the need for arch-specific code.

Tested on a S3C2442 based GTA02
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
[Guenter Roeck: Update to latest API; do not require static context variable]
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
changes since v5:
- reintroduce the mdelay(500) from the platform code, to make sure the
  watchdog has time to assert, as suggested by Tomasz Figa

 drivers/watchdog/s3c2410_wdt.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 7c6ccd0..2955aef 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -41,6 +41,8 @@
 #include <linux/of.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/reboot.h>
+#include <linux/delay.h>
 
 #define S3C2410_WTCON		0x00
 #define S3C2410_WTDAT		0x04
@@ -128,6 +130,7 @@ struct s3c2410_wdt {
 	unsigned long		wtdat_save;
 	struct watchdog_device	wdt_device;
 	struct notifier_block	freq_transition;
+	struct notifier_block	restart_handler;
 	struct s3c2410_wdt_variant *drv_data;
 	struct regmap *pmureg;
 };
@@ -438,6 +441,31 @@ static inline void s3c2410wdt_cpufreq_deregister(struct s3c2410_wdt *wdt)
 }
 #endif
 
+static int s3c2410wdt_restart(struct notifier_block *this,
+			      unsigned long mode, void *cmd)
+{
+	struct s3c2410_wdt *wdt = container_of(this, struct s3c2410_wdt,
+					       restart_handler);
+	void __iomem *wdt_base = wdt->reg_base;
+
+	/* disable watchdog, to be safe  */
+	writel(0, wdt_base + S3C2410_WTCON);
+
+	/* put initial values into count and data */
+	writel(0x80, wdt_base + S3C2410_WTCNT);
+	writel(0x80, wdt_base + S3C2410_WTDAT);
+
+	/* set the watchdog to go and reset... */
+	writel(S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV16 |
+		S3C2410_WTCON_RSTEN | S3C2410_WTCON_PRESCALE(0x20),
+		wdt_base + S3C2410_WTCON);
+
+	/* wait for reset to assert... */
+	mdelay(500);
+
+	return NOTIFY_DONE;
+}
+
 static inline unsigned int s3c2410wdt_get_bootstatus(struct s3c2410_wdt *wdt)
 {
 	unsigned int rst_stat;
@@ -592,6 +620,12 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, wdt);
 
+	wdt->restart_handler.notifier_call = s3c2410wdt_restart;
+	wdt->restart_handler.priority = 128;
+	ret = register_restart_handler(&wdt->restart_handler);
+	if (ret)
+		pr_err("cannot register restart handler, %d\n", ret);
+
 	/* print out a statement of readiness */
 
 	wtcon = readl(wdt->reg_base + S3C2410_WTCON);
@@ -621,6 +655,8 @@ static int s3c2410wdt_remove(struct platform_device *dev)
 	int ret;
 	struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
 
+	unregister_restart_handler(&wdt->restart_handler);
+
 	ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
 	if (ret < 0)
 		return ret;
-- 
2.0.1

[PATCH v5.1 10/7] clk: samsung: register restart handlers for s3c2412 and s3c2443

[Heiko Stübner]

S3C2412, S3C2443 and their derivatives contain a special software-reset
register in their system-controller.

Therefore register a restart handler for those.

Tested on a s3c2416-based board, s3c2412 compile-tested.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
[ Guenter Roeck: Updated to latest API version]
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
changes since v5:
- increase priority to 129, as all Samsung platforms can use the watchdog
  reset, but it might be nice to use the more specialized reset method
  where it is available.

 drivers/clk/samsung/clk-s3c2412.c | 29 +++++++++++++++++++++++++++++
 drivers/clk/samsung/clk-s3c2443.c | 19 +++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/clk/samsung/clk-s3c2412.c b/drivers/clk/samsung/clk-s3c2412.c
index 23e4313..456439d 100644
--- a/drivers/clk/samsung/clk-s3c2412.c
+++ b/drivers/clk/samsung/clk-s3c2412.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/reboot.h>
 
 #include <dt-bindings/clock/s3c2412.h>
 
@@ -26,6 +27,7 @@
 #define CLKCON		0x0c
 #define CLKDIVN		0x14
 #define CLKSRC		0x1c
+#define SWRST		0x30
 
 /* list of PLLs to be registered */
 enum s3c2412_plls {
@@ -204,6 +206,28 @@ struct samsung_clock_alias s3c2412_aliases[] __initdata = {
 	ALIAS(MSYSCLK, NULL, "fclk"),
 };
 
+static int s3c2412_restart(struct notifier_block *this,
+			   unsigned long mode, void *cmd)
+{
+	/* errata "Watch-dog/Software Reset Problem" specifies that
+	 * this reset must be done with the SYSCLK sourced from
+	 * EXTCLK instead of FOUT to avoid a glitch in the reset
+	 * mechanism.
+	 *
+	 * See the watchdog section of the S3C2412 manual for more
+	 * information on this fix.
+	 */
+
+	__raw_writel(0x00, reg_base + CLKSRC);
+	__raw_writel(0x533C2412, reg_base + SWRST);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block s3c2412_restart_handler = {
+	.notifier_call = s3c2412_restart,
+	.priority = 129,
+};
+
 /*
  * fixed rate clocks generated outside the soc
  * Only necessary until the devicetree-move is complete
@@ -233,6 +257,7 @@ void __init s3c2412_common_clk_init(struct device_node *np, unsigned long xti_f,
 				    unsigned long ext_f, void __iomem *base)
 {
 	struct samsung_clk_provider *ctx;
+	int ret;
 	reg_base = base;
 
 	if (np) {
@@ -265,6 +290,10 @@ void __init s3c2412_common_clk_init(struct device_node *np, unsigned long xti_f,
 				   ARRAY_SIZE(s3c2412_aliases));
 
 	s3c2412_clk_sleep_init();
+
+	ret = register_restart_handler(&s3c2412_restart_handler);
+	if (ret)
+		pr_warn("cannot register restart handler, %d\n", ret);
 }
 
 static void __init s3c2412_clk_init(struct device_node *np)
diff --git a/drivers/clk/samsung/clk-s3c2443.c b/drivers/clk/samsung/clk-s3c2443.c
index c4bbdab..3c35db7 100644
--- a/drivers/clk/samsung/clk-s3c2443.c
+++ b/drivers/clk/samsung/clk-s3c2443.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/reboot.h>
 
 #include <dt-bindings/clock/s3c2443.h>
 
@@ -33,6 +34,7 @@
 #define HCLKCON		0x30
 #define PCLKCON		0x34
 #define SCLKCON		0x38
+#define SWRST		0x44
 
 /* the soc types */
 enum supported_socs {
@@ -354,6 +356,18 @@ struct samsung_clock_alias s3c2450_aliases[] __initdata = {
 	ALIAS(PCLK_I2C1, "s3c2410-i2c.1", "i2c"),
 };
 
+static int s3c2443_restart(struct notifier_block *this,
+			   unsigned long mode, void *cmd)
+{
+	__raw_writel(0x533c2443, reg_base + SWRST);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block s3c2443_restart_handler = {
+	.notifier_call = s3c2443_restart,
+	.priority = 129,
+};
+
 /*
  * fixed rate clocks generated outside the soc
  * Only necessary until the devicetree-move is complete
@@ -378,6 +392,7 @@ void __init s3c2443_common_clk_init(struct device_node *np, unsigned long xti_f,
 				    void __iomem *base)
 {
 	struct samsung_clk_provider *ctx;
+	int ret;
 	reg_base = base;
 
 	if (np) {
@@ -445,6 +460,10 @@ void __init s3c2443_common_clk_init(struct device_node *np, unsigned long xti_f,
 	}
 
 	s3c2443_clk_sleep_init();
+
+	ret = register_restart_handler(&s3c2443_restart_handler);
+	if (ret)
+		pr_warn("cannot register restart handler, %d\n", ret);
 }
 
 static void __init s3c2416_clk_init(struct device_node *np)
-- 
2.0.1

[PATCH v5.1 10/7] clk: samsung: register restart handlers for s3c2412 and s3c2443

[Heiko Stübner]

S3C2412, S3C2443 and their derivatives contain a special software-reset
register in their system-controller.

Therefore register a restart handler for those.

Tested on a s3c2416-based board, s3c2412 compile-tested.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
[ Guenter Roeck: Updated to latest API version]
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
changes since v5:
- increase priority to 129, as all Samsung platforms can use the watchdog
  reset, but it might be nice to use the more specialized reset method
  where it is available.

 drivers/clk/samsung/clk-s3c2412.c | 29 +++++++++++++++++++++++++++++
 drivers/clk/samsung/clk-s3c2443.c | 19 +++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/clk/samsung/clk-s3c2412.c b/drivers/clk/samsung/clk-s3c2412.c
index 23e4313..456439d 100644
--- a/drivers/clk/samsung/clk-s3c2412.c
+++ b/drivers/clk/samsung/clk-s3c2412.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/reboot.h>
 
 #include <dt-bindings/clock/s3c2412.h>
 
@@ -26,6 +27,7 @@
 #define CLKCON		0x0c
 #define CLKDIVN		0x14
 #define CLKSRC		0x1c
+#define SWRST		0x30
 
 /* list of PLLs to be registered */
 enum s3c2412_plls {
@@ -204,6 +206,28 @@ struct samsung_clock_alias s3c2412_aliases[] __initdata = {
 	ALIAS(MSYSCLK, NULL, "fclk"),
 };
 
+static int s3c2412_restart(struct notifier_block *this,
+			   unsigned long mode, void *cmd)
+{
+	/* errata "Watch-dog/Software Reset Problem" specifies that
+	 * this reset must be done with the SYSCLK sourced from
+	 * EXTCLK instead of FOUT to avoid a glitch in the reset
+	 * mechanism.
+	 *
+	 * See the watchdog section of the S3C2412 manual for more
+	 * information on this fix.
+	 */
+
+	__raw_writel(0x00, reg_base + CLKSRC);
+	__raw_writel(0x533C2412, reg_base + SWRST);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block s3c2412_restart_handler = {
+	.notifier_call = s3c2412_restart,
+	.priority = 129,
+};
+
 /*
  * fixed rate clocks generated outside the soc
  * Only necessary until the devicetree-move is complete
@@ -233,6 +257,7 @@ void __init s3c2412_common_clk_init(struct device_node *np, unsigned long xti_f,
 				    unsigned long ext_f, void __iomem *base)
 {
 	struct samsung_clk_provider *ctx;
+	int ret;
 	reg_base = base;
 
 	if (np) {
@@ -265,6 +290,10 @@ void __init s3c2412_common_clk_init(struct device_node *np, unsigned long xti_f,
 				   ARRAY_SIZE(s3c2412_aliases));
 
 	s3c2412_clk_sleep_init();
+
+	ret = register_restart_handler(&s3c2412_restart_handler);
+	if (ret)
+		pr_warn("cannot register restart handler, %d\n", ret);
 }
 
 static void __init s3c2412_clk_init(struct device_node *np)
diff --git a/drivers/clk/samsung/clk-s3c2443.c b/drivers/clk/samsung/clk-s3c2443.c
index c4bbdab..3c35db7 100644
--- a/drivers/clk/samsung/clk-s3c2443.c
+++ b/drivers/clk/samsung/clk-s3c2443.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/reboot.h>
 
 #include <dt-bindings/clock/s3c2443.h>
 
@@ -33,6 +34,7 @@
 #define HCLKCON		0x30
 #define PCLKCON		0x34
 #define SCLKCON		0x38
+#define SWRST		0x44
 
 /* the soc types */
 enum supported_socs {
@@ -354,6 +356,18 @@ struct samsung_clock_alias s3c2450_aliases[] __initdata = {
 	ALIAS(PCLK_I2C1, "s3c2410-i2c.1", "i2c"),
 };
 
+static int s3c2443_restart(struct notifier_block *this,
+			   unsigned long mode, void *cmd)
+{
+	__raw_writel(0x533c2443, reg_base + SWRST);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block s3c2443_restart_handler = {
+	.notifier_call = s3c2443_restart,
+	.priority = 129,
+};
+
 /*
  * fixed rate clocks generated outside the soc
  * Only necessary until the devicetree-move is complete
@@ -378,6 +392,7 @@ void __init s3c2443_common_clk_init(struct device_node *np, unsigned long xti_f,
 				    void __iomem *base)
 {
 	struct samsung_clk_provider *ctx;
+	int ret;
 	reg_base = base;
 
 	if (np) {
@@ -445,6 +460,10 @@ void __init s3c2443_common_clk_init(struct device_node *np, unsigned long xti_f,
 	}
 
 	s3c2443_clk_sleep_init();
+
+	ret = register_restart_handler(&s3c2443_restart_handler);
+	if (ret)
+		pr_warn("cannot register restart handler, %d\n", ret);
 }
 
 static void __init s3c2416_clk_init(struct device_node *np)
-- 
2.0.1