From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp-out-127.synserver.de ([212.40.185.127]:1043 "EHLO smtp-out-127.synserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750922AbbAKMZ3 (ORCPT ); Sun, 11 Jan 2015 07:25:29 -0500 Message-ID: <54B26BB4.50707@metafoo.de> Date: Sun, 11 Jan 2015 13:25:24 +0100 From: Lars-Peter Clausen MIME-Version: 1.0 To: =?windows-1252?Q?M=E5ns_Rullg=E5rd?= , Guenter Roeck CC: Ralf Baechle , Wim Van Sebroeck , Paul Burton , Paul Cercueil , Maarten ter Huurne , linux-mips@linux-mips.org, linux-watchdog@vger.kernel.org Subject: Re: [PATCH 1/3] MIPS: Use do_kernel_restart() as the default restart handler References: <1420914550-18335-1-git-send-email-lars@metafoo.de> <54B1CF9B.3060606@roeck-us.net> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-watchdog-owner@vger.kernel.org List-Id: linux-watchdog@vger.kernel.org On 01/11/2015 01:15 PM, Måns Rullgård wrote: > Guenter Roeck writes: > >> On 01/10/2015 12:08 PM, Måns Rullgård wrote: >>> Lars-Peter Clausen writes: >>> >>>> Use the recently introduced do_kernel_restart() function as the default restart >>>> handler if the platform did not explicitly provide a restart handler. This >>>> allows use restart handler that have been registered by device drivers to >>>> restart the machine. >>>> >>>> Signed-off-by: Lars-Peter Clausen >>>> --- >>>> arch/mips/kernel/reset.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/mips/kernel/reset.c b/arch/mips/kernel/reset.c >>>> index 07fc524..36cd80c 100644 >>>> --- a/arch/mips/kernel/reset.c >>>> +++ b/arch/mips/kernel/reset.c >>>> @@ -19,7 +19,7 @@ >>>> * So handle all using function pointers to machine specific >>>> * functions. >>>> */ >>>> -void (*_machine_restart)(char *command); >>>> +void (*_machine_restart)(char *command) = do_kernel_restart; >>>> void (*_machine_halt)(void); >>>> void (*pm_power_off)(void); >>> >>> There is already a similar patch posted by Kevin Cernekee: >>> http://www.linux-mips.org/archives/linux-mips/2014-12/msg00410.html >>> >> Personally I prefer the earlier patch, though I guess that is personal >> preference. > > They both achieve the same thing, though Kevin's is more in line with > what ARM does. Missing from both is a fallback while(1) loop in case no > restart handlers are registered. With the restart moved to the watchdog > driver, there's a possibility that this might happen. > In my opinion if such a fallback is needed it should be put into the kernel core reboot implementation and not into individual restart handler implementations. My first version of this patch was do_kernel_restart() followed by a machine_halt() (so it goes to sleep instead of busy looping) as a fallback. But I couldn't find a good reason why that should be done at the individual restart handler level, so I dropped it. - Lars