On Fri, Jun 19, 2020 at 06:27:04PM +0800, Woody Lin wrote:
> Hi Guenter,
>
> > The code should include an explanation for the need for the double
> > indirection (ie why schedule another worker instead of calling
> > kthread_run directly). Also, there should be a comment explaining
> > how it works, especially if kthread_run() fails to run a thread,
> > and why kthread_run() is needed in the first place.
>
> No problem, I am adding this to the next version.
>
> > This isn't really a "reboot target". It is just a string passed
> > to kernel_restart(). For the most part that string doesn't really
> > do anything useful. What is it supposed to accomplish in your case,
> > other than causing a reboot with the configured reboot_mode
> > (which is a global variable) ?
> >
> > If the underlying idea is to cause an orderly restart following
> > reboot_mode, a boolean would accomplish pretty much the same.
> >
> > Am I missing something ?
>
> This argument is next sent to registered reboot notifiers and machine_restart,
> according to implementation of 'kernel_restart'. So for platforms which
> implemented different handlers for each 'cmd', the timeout can be handled with
> desired behavior. For example, some platforms might want to reboot to rescue
> mode when timeout.
>
> void kernel_restart(char *cmd)
> kernel_restart_prepare(cmd);
> -> blocking_notifier_call_chain(&reboot_notifier_list, SYS_RESTART, cmd);
> ...
> machine_restart(cmd);
>
> Btw, according to the signature of kernel_restart, I should name this argument
> 'soft_reboot_cmd', instead of '*_target'. Sorry for causing the confusion.
>
Yes, that would make more sense.
Thanks,
Guenter
> > > + if (soft_active_on_boot) {
> > > + set_bit(WDOG_HW_RUNNING, &softdog_dev.status);
> > > + set_bit(WDOG_ACTIVE, &softdog_dev.status);
> > > + }
> >
> > Strictly speaking you should call the ping function here to start the timer.
> > Otherwise the watchdog won't really start if watchdog_register_device()
> > fails.
>
> Thanks for catching this, I will revise this in the next version.
>
> Woody
