Lars-Peter Clausen <lars@xxxxxxxxxx> writes: > On 01/11/2015 01:30 PM, Måns Rullgård wrote: >> Lars-Peter Clausen <lars@xxxxxxxxxx> writes: >> >>> On 01/11/2015 01:15 PM, Måns Rullgård wrote: >>>> Guenter Roeck <linux@xxxxxxxxxxxx> writes: >>>> >>>>> On 01/10/2015 12:08 PM, Måns Rullgård wrote: >>>>>> Lars-Peter Clausen <lars@xxxxxxxxxx> 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 <lars@xxxxxxxxxx> >>>>>>> --- >>>>>>> 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. >> >> ARM does it its arch machine_restart() function. MIPS should probably >> follow suit. >> > > That's just cargo culting... True. > Either it is required, then it should probably be moved into the core > kernel, or it is not required in which case it can be removed from the > ARM machine_restart() implementation. The quickest way to find out is probably to send a patch adding such code to the end of kernel_restart(). -- Måns Rullgård mans@xxxxxxxxx
