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

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

[Ishizaki Kou]

This is a patch to support Power/Reset buttons on Beat on Celleb.

On Beat, we have an event from Beat if Power button or Reset button
is pressed. This patch catches the event and convert it to a signal
to INIT process.

/sbin/inittab have no entry to turn the machine power off so we have
to detect if power button is pressed or not internally in our driver.
This idea is taken from PS3's event handling subsystem.

Signed-off-by: Kou Ishizaki <Kou.Ishizaki@toshiba.co.jp>
---

This patch is taken after "[PATCH 2/8] Move pause() and kexec_cpu_down()
to beat.c" applied, so some lines are taken in from that patch.
No functional dependency is incorporated.


Index: linux-powerpc-git/arch/powerpc/platforms/celleb/beat.c
===================================================================
--- linux-powerpc-git.orig/arch/powerpc/platforms/celleb/beat.c
+++ linux-powerpc-git/arch/powerpc/platforms/celleb/beat.c
@@ -22,16 +22,23 @@
 #include <linux/init.h>
 #include <linux/err.h>
 #include <linux/rtc.h>
+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
 
 #include <asm/hvconsole.h>
 #include <asm/time.h>
+#include <asm/machdep.h>
+#include <asm/firmware.h>
 
 #include "beat_wrapper.h"
 #include "beat.h"
+#include "interrupt.h"
+
+static int beat_pm_poweroff_flag;
 
 void beat_restart(char *cmd)
 {
-	beat_shutdown_logical_partition(1);
+	beat_shutdown_logical_partition(!beat_pm_poweroff_flag);
 }
 
 void beat_power_off(void)
@@ -170,6 +177,96 @@ void beat_kexec_cpu_down(int crash, int 
 }
 #endif
 
+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;
+}
+
+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;
+}
+
+static struct beat_event_list {
+	const char *typecode;
+	irq_handler_t handler;
+	unsigned int virq;
+} beat_event_list[] = {
+	{ "power", beat_power_event, 0 },
+	{ "reset", beat_reset_event, 0 },
+};
+
+static int __init beat_register_event(void)
+{
+	u64 path[4], data[2];
+	int rc, i;
+	unsigned int virq;
+
+	for (i = 0; i < ARRAY_SIZE(beat_event_list); i++) {
+		struct beat_event_list *ev = &beat_event_list[i];
+
+		if (beat_construct_event_receive_port(data) != 0) {
+			printk(KERN_ERR "Beat: "
+			       "cannot construct event receive port for %s\n",
+			       ev->typecode);
+			return -EINVAL;
+		}
+
+		virq = irq_create_mapping(NULL, data[0]);
+		if (virq == NO_IRQ) {
+			printk(KERN_ERR "Beat: failed to get virtual IRQ"
+			       " for event receive port for %s\n",
+			       ev->typecode);
+			beat_destruct_event_receive_port(data[0]);
+			return -EIO;
+		}
+		ev->virq = virq;
+
+		rc = request_irq(virq, ev->handler, IRQF_DISABLED,
+				      ev->typecode, NULL);
+		if (rc != 0) {
+			printk(KERN_ERR "Beat: failed to request virtual IRQ"
+			       " for event receive port for %s\n",
+			       ev->typecode);
+			beat_destruct_event_receive_port(data[0]);
+			return rc;
+		}
+
+		path[0] = 0x1000000065780000ul;	/* 1,ex */
+		path[1] = 0x627574746f6e0000ul;	/* button */
+		path[2] = 0;
+		strncpy((char *)&path[2], ev->typecode, 8);
+		path[3] = 0;
+		data[1] = 0;
+
+		beat_create_repository_node(path, data);
+	}
+	return 0;
+}
+
+static int __init beat_event_init(void)
+{
+	if (!machine_is(celleb) || !firmware_has_feature(FW_FEATURE_BEAT))
+		return -EINVAL;
+
+	beat_pm_poweroff_flag = 0;
+	return beat_register_event();
+}
+
+device_initcall(beat_event_init);
+
 EXPORT_SYMBOL(beat_get_term_char);
 EXPORT_SYMBOL(beat_put_term_char);
 EXPORT_SYMBOL(beat_halt_code);

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

[Arnd Bergmann]

On Wednesday 26 September 2007, Ishizaki Kou wrote:
> This is a patch to support Power/Reset buttons on Beat on Celleb.
> 
> On Beat, we have an event from Beat if Power button or Reset button
> is pressed. This patch catches the event and convert it to a signal
> to INIT process.
> 
> /sbin/inittab have no entry to turn the machine power off so we have
> to detect if power button is pressed or not internally in our driver.
> This idea is taken from PS3's event handling subsystem.
> 
> Signed-off-by: Kou Ishizaki <Kou.Ishizaki@toshiba.co.jp>

Basically looks good. At some point I want to do a similar driver
for the IBM cell blade, but I fear there is not much code we
can share.

> +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.

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.

> +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.

> +static int __init beat_event_init(void)
> +{
> +	if (!machine_is(celleb) || !firmware_has_feature(FW_FEATURE_BEAT))
> +		return -EINVAL;

Shouldn't one of the two be sufficent? It seems to me that you want to
probe for either celleb or BEAT, but not both, considering that celleb
implies BEAT.

	Arnd <><

Re: [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

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

[Arnd Bergmann]

On Sunday 30 September 2007, you wrote:
>=20
> > kernel_restart() seems appropriate for that.
>=20
> Now a comment to this one says:
>=20
> =A0*=A0=A0=A0=A0=A0=A0This is not safe to call in interrupt context.
>=20
> and AFAIU, this is called from an interrupt. Am I missing anything or=20
> would this be wrong to call kernel_restart() here?

It should be called through a workqueue, afaics, do avoid calling
a sleeping notifier from an interrupt.

> > > +static irqreturn_t beat_reset_event(int virq, void *arg)
> > > +{
> > > +=A0=A0=A0printk(KERN_DEBUG "Beat: reset button pressed\n");
> > > +=A0=A0=A0beat_pm_poweroff_flag =3D 0;
> > > +=A0=A0=A0if (kill_cad_pid(SIGINT, 1)) {
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0/* Just in case killing init proces=
s failed */
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0beat_restart(NULL);
> > > +=A0=A0=A0}
> > > +=A0=A0=A0return IRQ_HANDLED;
> > > +}
> >=20
> > same here, except calling kernel_halt() in the end.
>=20
> kernel_halt() doesn't have a similar comment, but it does call the=20
> kernel_shutdown_prepare() too, so, seems like it should not be called fro=
m=20
> an interrupt either?

yes

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

Tough question. I can see arguments for either way, but I guess I'd tend
to handling it in the kernel, choosing simplicity over consistency.

	Arnd <><