Re: [RFC][PATCH] misc: Introduce reboot_reason driver

[Bjorn Andersson <bjorn.andersson@sonymobile.com>]

On Tue 08 Dec 13:52 PST 2015, Arnd Bergmann wrote:

> On Tuesday 08 December 2015 13:29:22 John Stultz wrote:
> 
[..]
> > +static int reboot_reason(struct notifier_block *nb, unsigned long action,
> > +								void *data)
> > +{
> > +	char *cmd = (char *)data;
> > +	long reason = reasons[NONE];
> > +
> > +	if (!reboot_reason_addr)
> > +		return NOTIFY_DONE;
> > +
> > +	if (cmd != NULL) {
> > +		if (!strncmp(cmd, "bootloader", 10))
> > +			reason = reasons[BOOTLOADER];
> > +		else if (!strncmp(cmd, "recovery", 8))
> > +			reason = reasons[RECOVERY];
> > +		else if (!strncmp(cmd, "oem-", 4)) {
> > +			unsigned long code;
> > +
> > +			if (!kstrtoul(cmd+4, 0, &code))
> > +				reason = reasons[OEM] | (code & 0xff);
> > +		}
> > +	}
> > +
> > +	if (reason != -1)
> > +		writel(reason, reboot_reason_addr);
> > +	return NOTIFY_DONE;
> > +}
> 
> Will this reboot the machine?
> 

It will store the magic in IMEM, that is then read by the boot loader
after the reboot to make it trigger alternative boot flows - e.g. the
recovery/update path or enter fastboot mode.

> > +	/* Install the notifier */
> > +	restart_nb.notifier_call = reboot_reason;
> > +	restart_nb.priority = 256;
> > +	if (register_restart_handler(&restart_nb)) {
> 
> If not, you should use register_reboot_notifier() rather than
> register_restart_handler(). The former gets called to do something
> just before rebooting, while the latter attempts to actually reboot
> the machine.
> 

It should be a reboot_notifier, sorry for missing this in my answer.

> > +static int __init reboot_reason_init(void)
> > +{
> > +	return platform_driver_register(&reboot_reason_driver);
> > +}
> > +arch_initcall(reboot_reason_init);
> 
> Why this early? If it can be a normal device_initcall, you can use
> module_platform_driver().
> 

Not represented in this patch is that several Android vendors uses this
mechanism to communicate a panic() to the boot loader; to let the system
enter some sort of memory dump state.

Adding support for that would be convenient and mandate the early,
otherwise it's just userspace that will trigger this so module would be
fine.

Regards,
Bjorn