[PATCH v7 00/11] kernel: Add support for restart handler call chain

[PATCH v7 00/11] 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.

The entire patch series, including additional patches depending on it,
is available from
https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/
in branch 'restart-staging'.

---
v7: Rebased to v3.17-rc1
    Dropped null_restart from arm code to make arm_pm_restart truly optional
    Reordered series. power/restart must be patched to not use arm_pm_restart
    prior to making arm_pm_restart optional.
    Added all pending patches to series
v6: Use atomic notifier call chain 
    Rebased series to v3.16
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 v7 01/11] 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>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
v7: Rebased to v3.17-rc1
v6: Use atomic notifier call chain
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..5925f5a 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 ATOMIC_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 code 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 atomic_notifier_chain_register()
+ *	always returns zero.
+ */
+int register_restart_handler(struct notifier_block *nb)
+{
+	return atomic_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 atomic_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)
+{
+	atomic_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 v7 01/11] kernel: Add support for kernel restart handler call chain

[Doug Anderson]

Guenter,

On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> Various drivers implement architecture and/or device specific means
> to restart (reset) the system. Various mechanisms have been implemented
> to support those schemes. The best known mechanism is arm_pm_restart,
> which is a function pointer to be set either from platform specific code
> or from drivers. Another mechanism is to use hardware watchdogs to issue
> a reset; this mechanism is used if there is no other method available
> to reset a board or system. Two examples are alim7101_wdt, which currently
> uses the reboot notifier to trigger a reset, and moxart_wdt, which registers
> the arm_pm_restart function.
>
> The existing mechanisms have a number of drawbacks. Typically only one scheme
> to restart the system is supported (at least if arm_pm_restart is used).
> At least in theory there can be 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>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> v7: Rebased to v3.17-rc1
> v6: Use atomic notifier call chain
> 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(+)

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

[PATCH v7 02/11] 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>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
v7: No change.
v6: No change.
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 3e51f8d..edd707e 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 v7 02/11] power/restart: Call machine_restart instead of arm_pm_restart

[Doug Anderson]

Guenter,

On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> 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>

Do you need to submit this to his patch tracker to get him to pick it
up?  How are you envisioning that this series land?  It crosses a lot
of boundaries so I guess will need a reasonable amount of coordination
between maintainers...


> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> v7: No change.
> v6: No change.
> 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(-)

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

[PATCH v7 02/11] power/restart: Call machine_restart instead of arm_pm_restart

[Guenter Roeck]

On Wed, Aug 20, 2014 at 09:10:31PM -0700, Doug Anderson wrote:
> Guenter,
> 
> On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > 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>
> 
> Do you need to submit this to his patch tracker to get him to pick it
> up?  How are you envisioning that this series land?  It crosses a lot
> of boundaries so I guess will need a reasonable amount of coordination
> between maintainers...
> 
> 
If I get an Acked-by: from all maintainers, I could send a pull request
to Linus directly. How do I send a patch to Russell's patch tracker ?
I thought I copied all mailing lists suggested by get_maintainer.pl,
but maybe I missed one.

Thanks,
Guenter

[PATCH v7 02/11] power/restart: Call machine_restart instead of arm_pm_restart

[Doug Anderson]

Guenter,

On Wed, Aug 20, 2014 at 9:42 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Wed, Aug 20, 2014 at 09:10:31PM -0700, Doug Anderson wrote:
>> Guenter,
>>
>> On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> > 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>
>>
>> Do you need to submit this to his patch tracker to get him to pick it
>> up?  How are you envisioning that this series land?  It crosses a lot
>> of boundaries so I guess will need a reasonable amount of coordination
>> between maintainers...
>>
>>
> If I get an Acked-by: from all maintainers, I could send a pull request
> to Linus directly. How do I send a patch to Russell's patch tracker ?
> I thought I copied all mailing lists suggested by get_maintainer.pl,
> but maybe I missed one.

Ah, OK.  Perhaps it's best to ignore me, then.  ;)  FYI: Russell's
Patch System is at <http://www.arm.linux.org.uk/developer/patches/>
and is used for getting individual patches into branches maintained by
Russel.  If you're just going to send a pull request for the series
then I don't think you need it.

[PATCH v7 02/11] power/restart: Call machine_restart instead of arm_pm_restart

[Guenter Roeck]

On Thu, Aug 21, 2014 at 12:30:44PM -0700, Doug Anderson wrote:
> Guenter,
> 
> On Wed, Aug 20, 2014 at 9:42 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On Wed, Aug 20, 2014 at 09:10:31PM -0700, Doug Anderson wrote:
> >> Guenter,
> >>
> >> On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> >> > 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>
> >>
> >> Do you need to submit this to his patch tracker to get him to pick it
> >> up?  How are you envisioning that this series land?  It crosses a lot
> >> of boundaries so I guess will need a reasonable amount of coordination
> >> between maintainers...
> >>
> >>
> > If I get an Acked-by: from all maintainers, I could send a pull request
> > to Linus directly. How do I send a patch to Russell's patch tracker ?
> > I thought I copied all mailing lists suggested by get_maintainer.pl,
> > but maybe I missed one.
> 
> Ah, OK.  Perhaps it's best to ignore me, then.  ;)  FYI: Russell's

No, I appreciate the information.

> Patch System is at <http://www.arm.linux.org.uk/developer/patches/>
> and is used for getting individual patches into branches maintained by
> Russel.  If you're just going to send a pull request for the series
> then I don't think you need it.
> 
I copied both linux-arm-kernel and Russell himself, so hopefully he is aware
of the series and can let me know if he wants me to make any further changes
to it, if it is ready to go, or if I need to do anything else.

Thanks,
Guenter

[PATCH v7 02/11] power/restart: Call machine_restart instead of arm_pm_restart

[Sebastian Reichel]

Hi,

On Tue, Aug 19, 2014 at 05:45:29PM -0700, Guenter Roeck wrote:
> 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>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Heiko Stuebner <heiko@sntech.de>

Acked-by: Sebastian Reichel <sre@kernel.org>

Do you want me to take the patch via battery-2.6.git?

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140821/9a65832c/attachment.sig>

[PATCH v7 02/11] power/restart: Call machine_restart instead of arm_pm_restart

[Guenter Roeck]

On 08/21/2014 01:39 PM, Sebastian Reichel wrote:
> Hi,
>
> On Tue, Aug 19, 2014 at 05:45:29PM -0700, Guenter Roeck wrote:
>> 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>
>> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>> Acked-by: Heiko Stuebner <heiko@sntech.de>
>
> Acked-by: Sebastian Reichel <sre@kernel.org>
>
> Do you want me to take the patch via battery-2.6.git?
>

Hi Sebastian,

thanks a lot for the Ack. I am not sure if it is better to submit all
patches together or through individual maintainer trees. Let's wait for
Andrew's suggestion how we should proceed.

Thanks,
Guenter

[PATCH v7 03/11] 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>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
v7: No change.
v6: No change.
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 1309d64..0d3fb9f 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -180,6 +180,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 v7 03/11] arm64: Support restart through restart handler call chain

[Doug Anderson]

Guenter,

On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> 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>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> v7: No change.
> v6: No change.
> 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(+)

Reviewed-by: Doug Anderson <dianders@chromium.org>

[PATCH v7 04/11] 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
drop its initialization and 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>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
v7: Dropped null_restart and made arm_pm_restart truly optional.
v6: No change.
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 | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 81ef686..ea279f7 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -114,17 +114,13 @@ void soft_restart(unsigned long addr)
 	BUG();
 }
 
-static void null_restart(enum reboot_mode reboot_mode, const char *cmd)
-{
-}
-
 /*
  * Function pointers to optional machine specific functions
  */
 void (*pm_power_off)(void);
 EXPORT_SYMBOL(pm_power_off);
 
-void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart;
+void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
 EXPORT_SYMBOL_GPL(arm_pm_restart);
 
 /*
@@ -230,7 +226,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 v7 04/11] arm: Support restart through restart handler call chain

[Doug Anderson]

Guenter,

On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> 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
> drop its initialization and 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>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> v7: Dropped null_restart and made arm_pm_restart truly optional.
> v6: No change.
> 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 | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

[PATCH v7 04/11] arm: Support restart through restart handler call chain

[Andreas Färber]

Hi,

Am 20.08.2014 02:45, schrieb 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
> drop its initialization and check if it is set before calling it.
> Only call the kernel restart handler if arm_pm_restart is not set.
[...]
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 81ef686..ea279f7 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -114,17 +114,13 @@ void soft_restart(unsigned long addr)
>  	BUG();
>  }
>  
> -static void null_restart(enum reboot_mode reboot_mode, const char *cmd)
> -{
> -}
> -
>  /*
>   * Function pointers to optional machine specific functions
>   */
>  void (*pm_power_off)(void);
>  EXPORT_SYMBOL(pm_power_off);
>  
> -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart;
> +void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);

Stupid newbie question maybe, but isn't this variable uninitialized now,
like any non-static variable in C99? Or does the kernel assure that all
such "fields" are zero-initialized?

>  EXPORT_SYMBOL_GPL(arm_pm_restart);

(This doesn't seem to be affecting the value of arm_pm_restart, just
redeclaring it extern and adding further derived symbols.)

>  
>  /*
> @@ -230,7 +226,10 @@ void machine_restart(char *cmd)
>  	local_irq_disable();
>  	smp_send_stop();
>  
> -	arm_pm_restart(reboot_mode, cmd);
> +	if (arm_pm_restart)

Here we seem to be relying on arm_pm_restart to be NULL when not
callable. I.e., wondering whether it's ruled out that the following line
is triggered due to non-zero garbage in arm_pm_restart?

Thanks,
Andreas

> +		arm_pm_restart(reboot_mode, cmd);
> +	else
> +		do_kernel_restart(cmd);
>  
>  	/* Give a grace period for failure to restart of 1s */
>  	mdelay(1000);

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg

[PATCH v7 04/11] arm: Support restart through restart handler call chain

[Guenter Roeck]

On Fri, Aug 22, 2014 at 03:32:42AM +0200, Andreas F?rber wrote:
> Hi,
> 
> Am 20.08.2014 02:45, schrieb 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
> > drop its initialization and check if it is set before calling it.
> > Only call the kernel restart handler if arm_pm_restart is not set.
> [...]
> > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> > index 81ef686..ea279f7 100644
> > --- a/arch/arm/kernel/process.c
> > +++ b/arch/arm/kernel/process.c
> > @@ -114,17 +114,13 @@ void soft_restart(unsigned long addr)
> >  	BUG();
> >  }
> >  
> > -static void null_restart(enum reboot_mode reboot_mode, const char *cmd)
> > -{
> > -}
> > -
> >  /*
> >   * Function pointers to optional machine specific functions
> >   */
> >  void (*pm_power_off)(void);
> >  EXPORT_SYMBOL(pm_power_off);
> >  
> > -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart;
> > +void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
> 
> Stupid newbie question maybe, but isn't this variable uninitialized now,
> like any non-static variable in C99? Or does the kernel assure that all
> such "fields" are zero-initialized?
> 
It is initialized with NULL, like all other global and static variables in the
kernel (and like pm_power_off a few lines above).

Guenter

[PATCH v7 04/11] arm: Support restart through restart handler call chain

[Andreas Färber]

Am 22.08.2014 04:19, schrieb Guenter Roeck:
> On Fri, Aug 22, 2014 at 03:32:42AM +0200, Andreas F?rber wrote:
>> Am 20.08.2014 02:45, schrieb 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
>>> drop its initialization and check if it is set before calling it.
>>> Only call the kernel restart handler if arm_pm_restart is not set.
>> [...]
>>> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
>>> index 81ef686..ea279f7 100644
>>> --- a/arch/arm/kernel/process.c
>>> +++ b/arch/arm/kernel/process.c
>>> @@ -114,17 +114,13 @@ void soft_restart(unsigned long addr)
>>>  	BUG();
>>>  }
>>>  
>>> -static void null_restart(enum reboot_mode reboot_mode, const char *cmd)
>>> -{
>>> -}
>>> -
>>>  /*
>>>   * Function pointers to optional machine specific functions
>>>   */
>>>  void (*pm_power_off)(void);
>>>  EXPORT_SYMBOL(pm_power_off);
>>>  
>>> -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart;
>>> +void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
>>
>> Stupid newbie question maybe, but isn't this variable uninitialized now,
>> like any non-static variable in C99? Or does the kernel assure that all
>> such "fields" are zero-initialized?
>>
> It is initialized with NULL, like all other global and static variables in the
> kernel (and like pm_power_off a few lines above).

Thanks for the explanation. In that case FWIW

Reviewed-by: Andreas F?rber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg

[PATCH v7 05/11] 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>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
v7: No change.
v6: No change.
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 v7 06/11] 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>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
v7: No change.
v6: No change.
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 v7 07/11] watchdog: sunxi: Register restart handler with kernel restart handler

[Guenter Roeck]

The kernel core 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>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
v7: Added patch to series. Necessary since the restart handler in the driver
    is now available upstream.

 drivers/watchdog/sunxi_wdt.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
index 60deb9d..480bb55 100644
--- a/drivers/watchdog/sunxi_wdt.c
+++ b/drivers/watchdog/sunxi_wdt.c
@@ -21,14 +21,13 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/notifier.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/reboot.h>
 #include <linux/types.h>
 #include <linux/watchdog.h>
 
-#include <asm/system_misc.h>
-
 #define WDT_MAX_TIMEOUT         16
 #define WDT_MIN_TIMEOUT         1
 #define WDT_MODE_TIMEOUT(n)     ((n) << 3)
@@ -50,6 +49,7 @@ static unsigned int timeout = WDT_MAX_TIMEOUT;
 struct sunxi_wdt_dev {
 	struct watchdog_device wdt_dev;
 	void __iomem *wdt_base;
+	struct notifier_block restart_handler;
 };
 
 /*
@@ -74,24 +74,29 @@ static const int wdt_timeout_map[] = {
 	[16] = 0xB, /* 16s */
 };
 
-static void __iomem *reboot_wdt_base;
 
-static void sun4i_wdt_restart(enum reboot_mode mode, const char *cmd)
+static int sunxi_restart_handle(struct notifier_block *this, unsigned long mode,
+				void *cmd)
 {
+	struct sunxi_wdt_dev *sunxi_wdt = container_of(this,
+						       struct sunxi_wdt_dev,
+						       restart_handler);
+	void __iomem *wdt_base = sunxi_wdt->wdt_base;
+
 	/* Enable timer and set reset bit in the watchdog */
-	writel(WDT_MODE_EN | WDT_MODE_RST_EN, reboot_wdt_base + WDT_MODE);
+	writel(WDT_MODE_EN | WDT_MODE_RST_EN, wdt_base + WDT_MODE);
 
 	/*
 	 * Restart the watchdog. The default (and lowest) interval
 	 * value for the watchdog is 0.5s.
 	 */
-	writel(WDT_CTRL_RELOAD, reboot_wdt_base + WDT_CTRL);
+	writel(WDT_CTRL_RELOAD, wdt_base + WDT_CTRL);
 
 	while (1) {
 		mdelay(5);
-		writel(WDT_MODE_EN | WDT_MODE_RST_EN,
-		       reboot_wdt_base + WDT_MODE);
+		writel(WDT_MODE_EN | WDT_MODE_RST_EN, wdt_base + WDT_MODE);
 	}
+	return NOTIFY_DONE;
 }
 
 static int sunxi_wdt_ping(struct watchdog_device *wdt_dev)
@@ -205,8 +210,12 @@ static int sunxi_wdt_probe(struct platform_device *pdev)
 	if (unlikely(err))
 		return err;
 
-	reboot_wdt_base = sunxi_wdt->wdt_base;
-	arm_pm_restart = sun4i_wdt_restart;
+	sunxi_wdt->restart_handler.notifier_call = sunxi_restart_handle;
+	sunxi_wdt->restart_handler.priority = 128;
+	err = register_restart_handler(&sunxi_wdt->restart_handler);
+	if (err)
+		dev_err(&pdev->dev,
+			"cannot register restart handler (err=%d)\n", err);
 
 	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
 			sunxi_wdt->wdt_dev.timeout, nowayout);
@@ -218,7 +227,7 @@ static int sunxi_wdt_remove(struct platform_device *pdev)
 {
 	struct sunxi_wdt_dev *sunxi_wdt = platform_get_drvdata(pdev);
 
-	arm_pm_restart = NULL;
+	unregister_restart_handler(&sunxi_wdt->restart_handler);
 
 	watchdog_unregister_device(&sunxi_wdt->wdt_dev);
 	watchdog_set_drvdata(&sunxi_wdt->wdt_dev, NULL);
-- 
1.9.1

[PATCH v7 08/11] 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>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
---
v7: No change
v6: No change
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 ea279f7..250b6f6 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -121,7 +121,6 @@ void (*pm_power_off)(void);
 EXPORT_SYMBOL(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.
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 0d3fb9f..398ab05 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -98,7 +98,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 v7 08/11] arm/arm64: Unexport restart handlers

[Doug Anderson]

Guenter,

On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> 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>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> v7: No change
> v6: No change
> 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(-)

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

(FYI: all patches tested by me were tested on rk3288-evb)

[PATCH v7 08/11] arm/arm64: Unexport restart handlers

[Geert Uytterhoeven]

Hi G?nther,

On Wed, Aug 20, 2014 at 2:45 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> 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.

Why not? I was just going to do that, but I got greeted by:

ERROR: "arm_pm_restart" [drivers/power/reset/rmobile-reset.ko] undefined!

So now we have to make sure all reset drivers for a zillion different
hardware devices are builtin, and can't be modular?

> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Heiko Stuebner <heiko@sntech.de>
> ---
> v7: No change
> v6: No change
> 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 ea279f7..250b6f6 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -121,7 +121,6 @@ void (*pm_power_off)(void);
>  EXPORT_SYMBOL(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.
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 0d3fb9f..398ab05 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -98,7 +98,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.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v7 08/11] arm/arm64: Unexport restart handlers

[Guenter Roeck]

On 12/04/2014 05:36 AM, Geert Uytterhoeven wrote:
> Hi G?nther,
>
> On Wed, Aug 20, 2014 at 2:45 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> 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.
>
> Why not? I was just going to do that, but I got greeted by:
>

Because you should register a restart handler instead, like the other
drivers in the same directory now do.

> ERROR: "arm_pm_restart" [drivers/power/reset/rmobile-reset.ko] undefined!
>
> So now we have to make sure all reset drivers for a zillion different
> hardware devices are builtin, and can't be modular?
>
No. All those drivers need to do is to register a restart handler using
the API provided in the patch series.

Ultimately all restart handlers should do that and arm_pm_restart should
go away entirely. That was the point of the patch series.

Guenter

[PATCH v7 08/11] arm/arm64: Unexport restart handlers

[Geert Uytterhoeven]

Hi G?nther,

On Thu, Dec 4, 2014 at 3:26 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 12/04/2014 05:36 AM, Geert Uytterhoeven wrote:
>> On Wed, Aug 20, 2014 at 2:45 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>>> 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.
>>
>> Why not? I was just going to do that, but I got greeted by:
>
> Because you should register a restart handler instead, like the other
> drivers in the same directory now do.

That's a different thing. "there would be no guarantee that the module is
loaded when a restart is needed" is also valid for restart handlers...

>> ERROR: "arm_pm_restart" [drivers/power/reset/rmobile-reset.ko] undefined!
>>
>> So now we have to make sure all reset drivers for a zillion different
>> hardware devices are builtin, and can't be modular?
>>
> No. All those drivers need to do is to register a restart handler using
> the API provided in the patch series.
>
> Ultimately all restart handlers should do that and arm_pm_restart should
> go away entirely. That was the point of the patch series.

Good. That's what I'm doing right know ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v7 08/11] arm/arm64: Unexport restart handlers

[Guenter Roeck]

On 12/04/2014 06:44 AM, Geert Uytterhoeven wrote:
> Hi G?nther,
>
> On Thu, Dec 4, 2014 at 3:26 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 12/04/2014 05:36 AM, Geert Uytterhoeven wrote:
>>> On Wed, Aug 20, 2014 at 2:45 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>>>> 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.
>>>
>>> Why not? I was just going to do that, but I got greeted by:
>>
>> Because you should register a restart handler instead, like the other
>> drivers in the same directory now do.
>
> That's a different thing. "there would be no guarantee that the module is
> loaded when a restart is needed" is also valid for restart handlers...
>

Not really, because you are supposed to unregister the restart handler
on unload. Sure, you can instead clear arm_pm_reastart and leave the system
with no means to restart ...

Guenter

>>> ERROR: "arm_pm_restart" [drivers/power/reset/rmobile-reset.ko] undefined!
>>>
>>> So now we have to make sure all reset drivers for a zillion different
>>> hardware devices are builtin, and can't be modular?
>>>
>> No. All those drivers need to do is to register a restart handler using
>> the API provided in the patch series.
>>
>> Ultimately all restart handlers should do that and arm_pm_restart should
>> go away entirely. That was the point of the patch series.
>
> Good. That's what I'm doing right know ;-)
>
> Gr{oetje,eeting}s,
>
>                          Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
>

[PATCH v7 08/11] arm/arm64: Unexport restart handlers

[Arnd Bergmann]

On Thursday 04 December 2014 06:51:49 Guenter Roeck wrote:
> On 12/04/2014 06:44 AM, Geert Uytterhoeven wrote:
> > On Thu, Dec 4, 2014 at 3:26 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> >> On 12/04/2014 05:36 AM, Geert Uytterhoeven wrote:
> >>> On Wed, Aug 20, 2014 at 2:45 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> >>>> 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.
> >>>
> >>> Why not? I was just going to do that, but I got greeted by:
> >>
> >> Because you should register a restart handler instead, like the other
> >> drivers in the same directory now do.
> >
> > That's a different thing. "there would be no guarantee that the module is
> > loaded when a restart is needed" is also valid for restart handlers...
> >
> 
> Not really, because you are supposed to unregister the restart handler
> on unload. Sure, you can instead clear arm_pm_reastart and leave the system
> with no means to restart ...

I agree with Geert that your commit message was confusing, it sounds like
you were referring to drivers that are not yet loaded, while the problem
that you are really address is drivers that have been unloaded later.

	Arnd

[PATCH v7 08/11] arm/arm64: Unexport restart handlers

[Guenter Roeck]

On Thu, Dec 04, 2014 at 04:06:22PM +0100, Arnd Bergmann wrote:
> On Thursday 04 December 2014 06:51:49 Guenter Roeck wrote:
> > On 12/04/2014 06:44 AM, Geert Uytterhoeven wrote:
> > > On Thu, Dec 4, 2014 at 3:26 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > >> On 12/04/2014 05:36 AM, Geert Uytterhoeven wrote:
> > >>> On Wed, Aug 20, 2014 at 2:45 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> > >>>> 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.
> > >>>
> > >>> Why not? I was just going to do that, but I got greeted by:
> > >>
> > >> Because you should register a restart handler instead, like the other
> > >> drivers in the same directory now do.
> > >
> > > That's a different thing. "there would be no guarantee that the module is
> > > loaded when a restart is needed" is also valid for restart handlers...
> > >
> > 
> > Not really, because you are supposed to unregister the restart handler
> > on unload. Sure, you can instead clear arm_pm_reastart and leave the system
> > with no means to restart ...
> 
> I agree with Geert that your commit message was confusing, it sounds like
> you were referring to drivers that are not yet loaded, while the problem
> that you are really address is drivers that have been unloaded later.
> 
Yes, I realize that now. I should probably have added the rationale
for the entire series to this commit. I'll try to do better next time.

Guenter

[PATCH v7 09/11] watchdog: s3c2410: add restart handler

[Guenter Roeck]

From: Heiko St?bner <heiko@sntech.de>

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>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v7: Added patch to series.

 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;
-- 
1.9.1

[PATCH v7 10/11] clk: samsung: register restart handlers for s3c2412 and s3c2443

[Guenter Roeck]

From: Heiko St?bner <heiko@sntech.de>

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>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v7: Added patch to series.

 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 34af09f..2ceedaf 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) {
@@ -267,6 +292,10 @@ void __init s3c2412_common_clk_init(struct device_node *np, unsigned long xti_f,
 	s3c2412_clk_sleep_init();
 
 	samsung_clk_of_add_provider(np, ctx);
+
+	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 c92f853..0c3c182 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) {
@@ -447,6 +462,10 @@ void __init s3c2443_common_clk_init(struct device_node *np, unsigned long xti_f,
 	s3c2443_clk_sleep_init();
 
 	samsung_clk_of_add_provider(np, ctx);
+
+	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)
-- 
1.9.1

[PATCH v7 10/11] clk: samsung: register restart handlers for s3c2412 and s3c2443

[Mike Turquette]

Quoting Guenter Roeck (2014-08-19 17:45:37)
> From: Heiko St?bner <heiko@sntech.de>
> 
> 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>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Acked-by: Mike Turquette <mturquette@linaro.org>

> ---
> v7: Added patch to series.
> 
>  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 34af09f..2ceedaf 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) {
> @@ -267,6 +292,10 @@ void __init s3c2412_common_clk_init(struct device_node *np, unsigned long xti_f,
>         s3c2412_clk_sleep_init();
>  
>         samsung_clk_of_add_provider(np, ctx);
> +
> +       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 c92f853..0c3c182 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) {
> @@ -447,6 +462,10 @@ void __init s3c2443_common_clk_init(struct device_node *np, unsigned long xti_f,
>         s3c2443_clk_sleep_init();
>  
>         samsung_clk_of_add_provider(np, ctx);
> +
> +       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)
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v7 11/11] clk: rockchip: add restart handler

[Guenter Roeck]

From: Heiko St?bner <heiko@sntech.de>

Add infrastructure to write the correct value to the restart register and
register the restart notifier for both rk3188 (including rk3066) and rk3288.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v7: Added patch to series.

 drivers/clk/rockchip/clk-rk3188.c |  2 ++
 drivers/clk/rockchip/clk-rk3288.c |  2 ++
 drivers/clk/rockchip/clk.c        | 25 +++++++++++++++++++++++++
 drivers/clk/rockchip/clk.h        |  1 +
 4 files changed, 30 insertions(+)

diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
index a83a6d8..71b661a 100644
--- a/drivers/clk/rockchip/clk-rk3188.c
+++ b/drivers/clk/rockchip/clk-rk3188.c
@@ -631,6 +631,8 @@ static void __init rk3188_common_clk_init(struct device_node *np)
 
 	rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
 				  ROCKCHIP_SOFTRST_HIWORD_MASK);
+
+	rockchip_register_restart_notifier(RK2928_GLB_SRST_FST);
 }
 
 static void __init rk3066a_clk_init(struct device_node *np)
diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index 0d8c6c5..b604217 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -713,5 +713,7 @@ static void __init rk3288_clk_init(struct device_node *np)
 
 	rockchip_register_softrst(np, 9, reg_base + RK3288_SOFTRST_CON(0),
 				  ROCKCHIP_SOFTRST_HIWORD_MASK);
+
+	rockchip_register_restart_notifier(RK3288_GLB_SRST_FST);
 }
 CLK_OF_DECLARE(rk3288_cru, "rockchip,rk3288-cru", rk3288_clk_init);
diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index 278cf9d..aa41433 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -25,6 +25,7 @@
 #include <linux/clk-provider.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/reboot.h>
 #include "clk.h"
 
 /**
@@ -242,3 +243,27 @@ void __init rockchip_clk_register_branches(
 		rockchip_clk_add_lookup(clk, list->id);
 	}
 }
+
+static unsigned int reg_restart;
+static int rockchip_restart_notify(struct notifier_block *this,
+				   unsigned long mode, void *cmd)
+{
+	writel(0xfdb9, reg_base + reg_restart);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block rockchip_restart_handler = {
+	.notifier_call = rockchip_restart_notify,
+	.priority = 128,
+};
+
+void __init rockchip_register_restart_notifier(unsigned int reg)
+{
+	int ret;
+
+	reg_restart = reg;
+	ret = register_restart_handler(&rockchip_restart_handler);
+	if (ret)
+		pr_err("%s: cannot register restart handler, %d\n",
+		       __func__, ret);
+}
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index 887cbde..0b5eab5 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -329,6 +329,7 @@ void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list,
 				    unsigned int nr_clk);
 void rockchip_clk_register_plls(struct rockchip_pll_clock *pll_list,
 				unsigned int nr_pll, int grf_lock_offset);
+void rockchip_register_restart_notifier(unsigned int reg);
 
 #define ROCKCHIP_SOFTRST_HIWORD_MASK	BIT(0)
 
-- 
1.9.1

[PATCH v7 11/11] clk: rockchip: add restart handler

[Doug Anderson]

Guenter / Heiko,

On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> From: Heiko St?bner <heiko@sntech.de>
>
> Add infrastructure to write the correct value to the restart register and
> register the restart notifier for both rk3188 (including rk3066) and rk3288.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v7: Added patch to series.
>
>  drivers/clk/rockchip/clk-rk3188.c |  2 ++
>  drivers/clk/rockchip/clk-rk3288.c |  2 ++
>  drivers/clk/rockchip/clk.c        | 25 +++++++++++++++++++++++++
>  drivers/clk/rockchip/clk.h        |  1 +
>  4 files changed, 30 insertions(+)

This patch doesn't apply cleanly with the in-flight (clk: rockchip:
protect critical clocks from getting disabled) patch from Heiko.  It's
trivial to resolve and unclear which will land first, so I think it's
fine...

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

(FYI: all patches tested by me were tested on rk3288-evb)

[PATCH v7 11/11] clk: rockchip: add restart handler

[Heiko Stübner]

Am Mittwoch, 20. August 2014, 21:15:10 schrieb Doug Anderson:
> Guenter / Heiko,
> 
> On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > From: Heiko St?bner <heiko@sntech.de>
> > 
> > Add infrastructure to write the correct value to the restart register and
> > register the restart notifier for both rk3188 (including rk3066) and
> > rk3288.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > v7: Added patch to series.
> > 
> >  drivers/clk/rockchip/clk-rk3188.c |  2 ++
> >  drivers/clk/rockchip/clk-rk3288.c |  2 ++
> >  drivers/clk/rockchip/clk.c        | 25 +++++++++++++++++++++++++
> >  drivers/clk/rockchip/clk.h        |  1 +
> >  4 files changed, 30 insertions(+)
> 
> This patch doesn't apply cleanly with the in-flight (clk: rockchip:
> protect critical clocks from getting disabled) patch from Heiko.  It's
> trivial to resolve and unclear which will land first, so I think it's
> fine...

This should be expected :-) .

The patch about critical clocks will (hopefully soon) go through Mike's tree, 
while this series will most likely go through somewhere else (Linus' tree 
directly or something), so I guess the merge conflic will be resolved by Linus 
when both trees meet each other during the merge window.
So both this patch as well as the other one should stay independent of each 
other.


Heiko

[PATCH v7 11/11] clk: rockchip: add restart handler

[Mike Turquette]

Quoting Guenter Roeck (2014-08-19 17:45:38)
> From: Heiko St?bner <heiko@sntech.de>
> 
> Add infrastructure to write the correct value to the restart register and
> register the restart notifier for both rk3188 (including rk3066) and rk3288.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Acked-by: Mike Turquette <mturquette@linaro.org>

> ---
> v7: Added patch to series.
> 
>  drivers/clk/rockchip/clk-rk3188.c |  2 ++
>  drivers/clk/rockchip/clk-rk3288.c |  2 ++
>  drivers/clk/rockchip/clk.c        | 25 +++++++++++++++++++++++++
>  drivers/clk/rockchip/clk.h        |  1 +
>  4 files changed, 30 insertions(+)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3188.c b/drivers/clk/rockchip/clk-rk3188.c
> index a83a6d8..71b661a 100644
> --- a/drivers/clk/rockchip/clk-rk3188.c
> +++ b/drivers/clk/rockchip/clk-rk3188.c
> @@ -631,6 +631,8 @@ static void __init rk3188_common_clk_init(struct device_node *np)
>  
>         rockchip_register_softrst(np, 9, reg_base + RK2928_SOFTRST_CON(0),
>                                   ROCKCHIP_SOFTRST_HIWORD_MASK);
> +
> +       rockchip_register_restart_notifier(RK2928_GLB_SRST_FST);
>  }
>  
>  static void __init rk3066a_clk_init(struct device_node *np)
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index 0d8c6c5..b604217 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -713,5 +713,7 @@ static void __init rk3288_clk_init(struct device_node *np)
>  
>         rockchip_register_softrst(np, 9, reg_base + RK3288_SOFTRST_CON(0),
>                                   ROCKCHIP_SOFTRST_HIWORD_MASK);
> +
> +       rockchip_register_restart_notifier(RK3288_GLB_SRST_FST);
>  }
>  CLK_OF_DECLARE(rk3288_cru, "rockchip,rk3288-cru", rk3288_clk_init);
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 278cf9d..aa41433 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -25,6 +25,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
> +#include <linux/reboot.h>
>  #include "clk.h"
>  
>  /**
> @@ -242,3 +243,27 @@ void __init rockchip_clk_register_branches(
>                 rockchip_clk_add_lookup(clk, list->id);
>         }
>  }
> +
> +static unsigned int reg_restart;
> +static int rockchip_restart_notify(struct notifier_block *this,
> +                                  unsigned long mode, void *cmd)
> +{
> +       writel(0xfdb9, reg_base + reg_restart);
> +       return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block rockchip_restart_handler = {
> +       .notifier_call = rockchip_restart_notify,
> +       .priority = 128,
> +};
> +
> +void __init rockchip_register_restart_notifier(unsigned int reg)
> +{
> +       int ret;
> +
> +       reg_restart = reg;
> +       ret = register_restart_handler(&rockchip_restart_handler);
> +       if (ret)
> +               pr_err("%s: cannot register restart handler, %d\n",
> +                      __func__, ret);
> +}
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index 887cbde..0b5eab5 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -329,6 +329,7 @@ void rockchip_clk_register_branches(struct rockchip_clk_branch *clk_list,
>                                     unsigned int nr_clk);
>  void rockchip_clk_register_plls(struct rockchip_pll_clock *pll_list,
>                                 unsigned int nr_pll, int grf_lock_offset);
> +void rockchip_register_restart_notifier(unsigned int reg);
>  
>  #define ROCKCHIP_SOFTRST_HIWORD_MASK   BIT(0)
>  
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v7 00/11] kernel: Add support for restart handler call chain

[Guenter Roeck]

On Tue, Aug 19, 2014 at 05:45:27PM -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.
> 
> The entire patch series, including additional patches depending on it,
> is available from
> https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/
> in branch 'restart-staging'.
> 
Hi Andrew,

I think this series is ready for upstream integration. Question now
is how we should proceed to get it actually integrated.

I can see a number of options:
- You take patch #1, the rest goes in through maintainer trees.
- You take all patches after we get missing maintainer Acks.
- I send a pull request directly to Linus after we get missing
  maintainer Acks.

What do you think would be the best way to proceed ?

Thanks,
Guenter

[PATCH v7 00/11] kernel: Add support for restart handler call chain

[Heiko Stübner]

Am Samstag, 23. August 2014, 09:35:05 schrieb Guenter Roeck:
> On Tue, Aug 19, 2014 at 05:45:27PM -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.
> > 
> > The entire patch series, including additional patches depending on it,
> > is available from
> > https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/
> > in branch 'restart-staging'.
> 
> Hi Andrew,
> 
> I think this series is ready for upstream integration. Question now
> is how we should proceed to get it actually integrated.
> 
> I can see a number of options:
> - You take patch #1, the rest goes in through maintainer trees.

I don't think you can split the patches like this. Patch1 introduces 
(un)register_restart_handler functions used by later patches in the series. 
You therefore cannot really split the series, as otherwise you would get build 
failures in the individual trees.


Heiko


> - You take all patches after we get missing maintainer Acks.
> - I send a pull request directly to Linus after we get missing
>   maintainer Acks.
> 
> What do you think would be the best way to proceed ?
> 
> Thanks,
> Guenter

[PATCH v7 00/11] kernel: Add support for restart handler call chain

[Guenter Roeck]

On 08/23/2014 04:00 PM, Heiko St?bner wrote:
> Am Samstag, 23. August 2014, 09:35:05 schrieb Guenter Roeck:
>> On Tue, Aug 19, 2014 at 05:45:27PM -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.
>>>
>>> The entire patch series, including additional patches depending on it,
>>> is available from
>>> https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/
>>> in branch 'restart-staging'.
>>
>> Hi Andrew,
>>
>> I think this series is ready for upstream integration. Question now
>> is how we should proceed to get it actually integrated.
>>
>> I can see a number of options:
>> - You take patch #1, the rest goes in through maintainer trees.
>
> I don't think you can split the patches like this. Patch1 introduces
> (un)register_restart_handler functions used by later patches in the series.
> You therefore cannot really split the series, as otherwise you would get build
> failures in the individual trees.
>
No, it would simply delay integration of the entire series by a release
or two. First two patches go in first, then #3 and #4, then the rest.

I don't like that option too much either, but it is better than nothing.

Guenter

[PATCH v7 00/11] kernel: Add support for restart handler call chain

[Pramod Gurav]

Hello Guenter,

Thanks for the patches.
Tested patches 1,2 & 4 from this series on qcom apq8064 based IFC6410
with Josh's patches which replace arm restart with restart notifier for
this target.

Tested-by: pramod.gurav at smartplayin.com

On Wednesday 20 August 2014 06:15 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.
> 
> The entire patch series, including additional patches depending on it,
> is available from
> https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/
> in branch 'restart-staging'.
> 
> ---
> v7: Rebased to v3.17-rc1
>     Dropped null_restart from arm code to make arm_pm_restart truly optional
>     Reordered series. power/restart must be patched to not use arm_pm_restart
>     prior to making arm_pm_restart optional.
>     Added all pending patches to series
> v6: Use atomic notifier call chain 
>     Rebased series to v3.16
> 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-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

[PATCH v7 00/11] kernel: Add support for restart handler call chain

[Andrew Morton]

On Tue, 19 Aug 2014 17:45:27 -0700 Guenter Roeck <linux@roeck-us.net> wrote:

> Introduce a system restart handler call chain to solve the described problems.

So someone has merged eight of these patches into linux-next but these
three:

watchdog-s3c2410-add-restart-handler.patch
clk-samsung-register-restart-handlers-for-s3c2412-and-s3c2443.patch
clk-rockchip-add-restart-handler.patch

were omitted.  What's up?

[PATCH v7 00/11] kernel: Add support for restart handler call chain

[Guenter Roeck]

On Tue, Sep 30, 2014 at 02:20:02PM -0700, Andrew Morton wrote:
> On Tue, 19 Aug 2014 17:45:27 -0700 Guenter Roeck <linux@roeck-us.net> wrote:
> 
> > Introduce a system restart handler call chain to solve the described problems.
> 
> So someone has merged eight of these patches into linux-next but these
> three:
> 
> watchdog-s3c2410-add-restart-handler.patch
> clk-samsung-register-restart-handlers-for-s3c2412-and-s3c2443.patch
> clk-rockchip-add-restart-handler.patch
> 
> were omitted.  What's up?

Most likely PBKC on my side; Looks like I forgot to add those when I created
the immutable branch for others to merge. Sorry for that :-(.

Having said that, I somehow thought that the clock patches would go in through
the clock tree. Heiko, did I get that wrong ? Separately, I sent a pull request
that includes the watchdog patch to Wim.

Thanks,
Guenter

[PATCH v7 00/11] kernel: Add support for restart handler call chain

[Stephen Rothwell]

Hi Guenter,

On Tue, 30 Sep 2014 15:30:00 -0700 Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Tue, Sep 30, 2014 at 02:20:02PM -0700, Andrew Morton wrote:
> > On Tue, 19 Aug 2014 17:45:27 -0700 Guenter Roeck <linux@roeck-us.net> wrote:
> > 
> > > Introduce a system restart handler call chain to solve the described problems.
> > 
> > So someone has merged eight of these patches into linux-next but these
> > three:
> > 
> > watchdog-s3c2410-add-restart-handler.patch
> > clk-samsung-register-restart-handlers-for-s3c2412-and-s3c2443.patch
> > clk-rockchip-add-restart-handler.patch
> > 
> > were omitted.  What's up?
> 
> Most likely PBKC on my side; Looks like I forgot to add those when I created
> the immutable branch for others to merge. Sorry for that :-(.
> 
> Having said that, I somehow thought that the clock patches would go in through
> the clock tree. Heiko, did I get that wrong ? Separately, I sent a pull request
> that includes the watchdog patch to Wim.

So far, that immutable branch has been merged into the battery tree
(and thus into linux-next) by Sebastian in order to add (I assume):

18a702e0de98 power: reset: use restart_notifier mechanism for msm-poweroff
371bb20d6927 power: Add simple gpio-restart driver

on top of it.

So, I guess the watchdog and clk trees also need to merge that
immutable branch and then add their respective patches from the mmotm
series to their trees.
-- 
Cheers,
Stephen Rothwell                    sfr at canb.auug.org.au
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141001/874b8c8b/attachment.sig>

[PATCH v7 00/11] kernel: Add support for restart handler call chain

[Guenter Roeck]

On 09/30/2014 04:40 PM, Stephen Rothwell wrote:
> Hi Guenter,
>
> On Tue, 30 Sep 2014 15:30:00 -0700 Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On Tue, Sep 30, 2014 at 02:20:02PM -0700, Andrew Morton wrote:
>>> On Tue, 19 Aug 2014 17:45:27 -0700 Guenter Roeck <linux@roeck-us.net> wrote:
>>>
>>>> Introduce a system restart handler call chain to solve the described problems.
>>>
>>> So someone has merged eight of these patches into linux-next but these
>>> three:
>>>
>>> watchdog-s3c2410-add-restart-handler.patch
>>> clk-samsung-register-restart-handlers-for-s3c2412-and-s3c2443.patch
>>> clk-rockchip-add-restart-handler.patch
>>>
>>> were omitted.  What's up?
>>
>> Most likely PBKC on my side; Looks like I forgot to add those when I created
>> the immutable branch for others to merge. Sorry for that :-(.
>>
>> Having said that, I somehow thought that the clock patches would go in through
>> the clock tree. Heiko, did I get that wrong ? Separately, I sent a pull request
>> that includes the watchdog patch to Wim.
>
> So far, that immutable branch has been merged into the battery tree
> (and thus into linux-next) by Sebastian in order to add (I assume):
>
> 18a702e0de98 power: reset: use restart_notifier mechanism for msm-poweroff
> 371bb20d6927 power: Add simple gpio-restart driver
>
> on top of it.
>
> So, I guess the watchdog and clk trees also need to merge that
> immutable branch and then add their respective patches from the mmotm
> series to their trees.
>
Yes, and now I remember why I did not include those patches: They are not
authored by myself. I thought it was not appropriate for me to include them
in the branch I created. Maybe flawed thinking, but that was my reasoning.

Guenter

[PATCH v7 00/11] kernel: Add support for restart handler call chain

[Heiko Stübner]

Am Dienstag, 30. September 2014, 15:30:00 schrieb Guenter Roeck:
> On Tue, Sep 30, 2014 at 02:20:02PM -0700, Andrew Morton wrote:
> > On Tue, 19 Aug 2014 17:45:27 -0700 Guenter Roeck <linux@roeck-us.net> 
wrote:
> > > Introduce a system restart handler call chain to solve the described
> > > problems.> 
> > So someone has merged eight of these patches into linux-next but these
> > three:
> > 
> > watchdog-s3c2410-add-restart-handler.patch
> > clk-samsung-register-restart-handlers-for-s3c2412-and-s3c2443.patch
> > clk-rockchip-add-restart-handler.patch
> > 
> > were omitted.  What's up?
> 
> Most likely PBKC on my side; Looks like I forgot to add those when I created
> the immutable branch for others to merge. Sorry for that :-(.
> 
> Having said that, I somehow thought that the clock patches would go in
> through the clock tree. Heiko, did I get that wrong ? Separately, I sent a
> pull request that includes the watchdog patch to Wim.

I didn't realise that the patches would get dropped from -mm until I the 
removal notices this morning. So meanwhile I've send a pull to Mike for the 
Rockchip and S3C2412/2416/2443 restart handling.

So, as your pull includes the samsung watchdog restart, we should be hopefully 
all set again :-) .


Heiko