[PATCH 3/7] Celleb: Support for Power/Reset buttons

[Guennadi Liakhovetski]

On Wed, 26 Sep 2007, Arnd Bergmann wrote:

> On Wednesday 26 September 2007, Ishizaki Kou wrote:
> 
> > +static irqreturn_t beat_power_event(int virq, void *arg)
> > +{
> > +	printk(KERN_DEBUG "Beat: power button pressed\n");
> > +	beat_pm_poweroff_flag = 1;
> > +	if (kill_cad_pid(SIGINT, 1)) {
> > +		/* Just in case killing init process failed */
> > +		beat_power_off();
> > +	}
> > +	return IRQ_HANDLED;
> > +}
> 
> I think this should call ctrl_alt_del() instead of doing 
> kill_cad_pid() directly.

Agree.

> Also, I think you should better not call the low-level
> beat_power_off() function, but rather a high-level function
> that goes through the reboot notifiers first.
> 
> kernel_restart() seems appropriate for that.

Now a comment to this one says:

 *	This is not safe to call in interrupt context.

and AFAIU, this is called from an interrupt. Am I missing anything or 
would this be wrong to call kernel_restart() here?

> > +static irqreturn_t beat_reset_event(int virq, void *arg)
> > +{
> > +	printk(KERN_DEBUG "Beat: reset button pressed\n");
> > +	beat_pm_poweroff_flag = 0;
> > +	if (kill_cad_pid(SIGINT, 1)) {
> > +		/* Just in case killing init process failed */
> > +		beat_restart(NULL);
> > +	}
> > +	return IRQ_HANDLED;
> > +}
> 
> same here, except calling kernel_halt() in the end.

kernel_halt() doesn't have a similar comment, but it does call the 
kernel_shutdown_prepare() too, so, seems like it should not be called from 
an interrupt either?

Now, another question, is it generally good to hard-wire "reset" and 
"power-off" buttons in the kernel? Isn't it better to just register them 
as input event sources and let userspace (power-management daemon) decide 
what to do with them? Like poweroff / reboot / suspend / hibernate / ...

Thanks
Guennadi
---
Guennadi Liakhovetski