[RFC 0/7] powerpc: Rework pm_power_off & machine_[restart|power_off|halt]

[RFC 0/7] powerpc: Rework pm_power_off & machine_[restart|power_off|halt]

[Mark A. Greer]

We seem to have the pm_power_off hook wrong in arch/powerpc.  From the
other arches and from how its used by the rest of the kernel (e.g., ipmi),
it should point to the lowest-level power off function not to
machine_power_off().  Actually, machine_power_off() should call pm_power_off
since AFAICT it is the one and only interface used by the rest of the kernel
to power a machine off (with one exception which I believe to be a bug and
have a patch in this series to fix).

While looking at this, I found several bits of code that needed minor
rework and/or cleaning up.  These bits include: refactoring some
common code used by the machine_xxx routines, having machine_power_off
call ppc_md.halt if there ppc_md.power_off is NULL or returns, having
machine_halt call ppc_md.power_off it ppc_md.halt is NULL or returns,
and removing some useless xxx_halt and xxx_power_off routines in
platform code.

With the new usage of pm_power_off, the ppc_md.power_off hook is
no longer needed and pm_power_off will be assigned in the platform
probe routine.

The end result of all of these patches should make the check of
pm_power_off being NULL in kernel/sys.c:sys_reboot useful for powerpc,
eliminate the need for platform halt routines to call the power off routine
(and vice versa), allow things like IPMI to take over pm_power_off
when they need to, and make the power off/halt related code a bit cleaner
& consistent.

I still have to make sure I didn't miss anything and I haven't compiled
all the files I've touched so these aren't submission candidates yet.

I'd appreciate any feedback.

Thanks,

Mark

[PATCH 1/7] powerpc: Drivers should call machine_power_off not pm_power_off

[Mark A. Greer]

From: Mark A. Greer <mgreer@mvista.com>

Drivers should call machine_power_off() not pm_power_off to power off
a machine.

Signed-off-by: Mark A. Greer <mgreer@mvista.com>
---
 drivers/parisc/power.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/parisc/power.c b/drivers/parisc/power.c
index 90cca5e..188a1ac 100644
--- a/drivers/parisc/power.c
+++ b/drivers/parisc/power.c
@@ -93,11 +93,9 @@ static void process_shutdown(void)
 		lcd_print(msg);
 
 		/* send kill signal */
-		if (kill_cad_pid(SIGINT, 1)) {
+		if (kill_cad_pid(SIGINT, 1))
 			/* just in case killing init process failed */
-			if (pm_power_off)
-				pm_power_off();
-		}
+			machine_power_off();
 	}
 }
 

[PATCH 2/7] powerpc: xmon should call machine_xxx not ppc_md.xxx directly

[Mark A. Greer]

From: Mark A. Greer <mgreer@mvista.com>

xmon should call machine_[restart|halt|power_off] instead of
ppc_md.[restart|halt|power_off].

Signed-off-by: Mark A. Greer <mgreer@mvista.com>
---
I'm not sure about this one.  Does anyone see a problem with this?

 arch/powerpc/xmon/xmon.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 121b04d..56267e3 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -908,11 +908,11 @@ static void bootcmds(void)
 
 	cmd = inchar();
 	if (cmd == 'r')
-		ppc_md.restart(NULL);
+		machine_restart();
 	else if (cmd == 'h')
-		ppc_md.halt();
+		machine_halt();
 	else if (cmd == 'p')
-		ppc_md.power_off();
+		machine_power_off();
 }
 
 static int cpu_cmd(void)

[PATCH 3/7] powerpc: ras.c should call machine_power_off()

[Mark A. Greer]

From: Mark A. Greer <mgreer@mvista.com>

machine_power_off() is the proper interface to use for powering
off a machine.

Signed-off-by: Mark A. Greer <mgreer@mvista.com>
---
 arch/powerpc/platforms/pseries/ras.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index a1ab25c..64564b2 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -242,7 +242,7 @@ static irqreturn_t ras_error_interrupt(int irq, void *dev_id)
 		 * without actually failing while injecting errors.
 		 * Error data will not be logged to syslog.
 		 */
-		ppc_md.power_off();
+		machine_power_off();
 #endif
 	} else {
 		udbg_printf("Recoverable HW Error <0x%lx 0x%x>\n",

[PATCH 4/7] powerpc: Rework the machine_[restart|power_off|halt] routines

[Mark A. Greer]

From: Mark A. Greer <mgreer@mvista.com>

Factor out common code from the machine_xxx routines and make them better
handle a ppc_md hook that doesn't exist or fails better.  In particular,
have machine_power_off() try ppc_md.halt if ppc_md.power_off is NULL or fails,
and have machine_halt() try to power off when ppc_md.halt is NULL or fails.

Signed-off-by: Mark A. Greer <mgreer@mvista.com>
---
 arch/powerpc/kernel/setup-common.c |   40 ++++++++++++++-------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 6adb5a1..1f8f9aa 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -105,17 +105,23 @@ void machine_shutdown(void)
 		ppc_md.machine_shutdown();
 }
 
-void machine_restart(char *cmd)
+static void default_halt(const char *s)
 {
-	machine_shutdown();
-	if (ppc_md.restart)
-		ppc_md.restart(cmd);
 #ifdef CONFIG_SMP
 	smp_send_stop();
 #endif
-	printk(KERN_EMERG "System Halted, OK to turn off power\n");
+	printk(KERN_EMERG "%s", s);
 	local_irq_disable();
-	while (1) ;
+	while (1);
+}
+
+void machine_restart(char *cmd)
+{
+	machine_shutdown();
+	if (ppc_md.restart)
+		ppc_md.restart(cmd);
+	default_halt("System not restarted; halting instead.\n"
+			"OK to turn off power\n");
 }
 
 void machine_power_off(void)
@@ -123,12 +129,10 @@ void machine_power_off(void)
 	machine_shutdown();
 	if (ppc_md.power_off)
 		ppc_md.power_off();
-#ifdef CONFIG_SMP
-	smp_send_stop();
-#endif
-	printk(KERN_EMERG "System Halted, OK to turn off power\n");
-	local_irq_disable();
-	while (1) ;
+	printk(KERN_EMERG "System not powered off; halting instead.\n");
+	if (ppc_md.halt)
+		ppc_md.halt();
+	default_halt("OK to turn off power\n");
 }
 /* Used by the G5 thermal driver */
 EXPORT_SYMBOL_GPL(machine_power_off);
@@ -141,12 +145,12 @@ void machine_halt(void)
 	machine_shutdown();
 	if (ppc_md.halt)
 		ppc_md.halt();
-#ifdef CONFIG_SMP
-	smp_send_stop();
-#endif
-	printk(KERN_EMERG "System Halted, OK to turn off power\n");
-	local_irq_disable();
-	while (1) ;
+	if (ppc_md.power_off) {
+		printk(KERN_EMERG "System not halted; powering off instead.\n");
+		ppc_md.power_off();
+		printk(KERN_EMERG "Poweroff failed.\n");
+	}
+	default_halt("OK to turn off power\n");
 }
 
 

Re: [PATCH 4/7] powerpc: Rework the machine_[restart|power_off|halt] routines

[Benjamin Herrenschmidt]

On Mon, 2007-12-03 at 22:47 -0700, Mark A. Greer wrote:
> From: Mark A. Greer <mgreer@mvista.com>
> 
> Factor out common code from the machine_xxx routines and make them better
> handle a ppc_md hook that doesn't exist or fails better.  In particular,
> have machine_power_off() try ppc_md.halt if ppc_md.power_off is NULL or fails,
> and have machine_halt() try to power off when ppc_md.halt is NULL or fails.
> 
> Signed-off-by: Mark A. Greer <mgreer@mvista.com>

Quick glance... looks fine.

> ---
>  arch/powerpc/kernel/setup-common.c |   40 ++++++++++++++-------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 6adb5a1..1f8f9aa 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -105,17 +105,23 @@ void machine_shutdown(void)
>  		ppc_md.machine_shutdown();
>  }
>  
> -void machine_restart(char *cmd)
> +static void default_halt(const char *s)
>  {
> -	machine_shutdown();
> -	if (ppc_md.restart)
> -		ppc_md.restart(cmd);
>  #ifdef CONFIG_SMP
>  	smp_send_stop();
>  #endif
> -	printk(KERN_EMERG "System Halted, OK to turn off power\n");
> +	printk(KERN_EMERG "%s", s);
>  	local_irq_disable();
> -	while (1) ;
> +	while (1);
> +}
> +
> +void machine_restart(char *cmd)
> +{
> +	machine_shutdown();
> +	if (ppc_md.restart)
> +		ppc_md.restart(cmd);
> +	default_halt("System not restarted; halting instead.\n"
> +			"OK to turn off power\n");
>  }
>  
>  void machine_power_off(void)
> @@ -123,12 +129,10 @@ void machine_power_off(void)
>  	machine_shutdown();
>  	if (ppc_md.power_off)
>  		ppc_md.power_off();
> -#ifdef CONFIG_SMP
> -	smp_send_stop();
> -#endif
> -	printk(KERN_EMERG "System Halted, OK to turn off power\n");
> -	local_irq_disable();
> -	while (1) ;
> +	printk(KERN_EMERG "System not powered off; halting instead.\n");
> +	if (ppc_md.halt)
> +		ppc_md.halt();
> +	default_halt("OK to turn off power\n");
>  }
>  /* Used by the G5 thermal driver */
>  EXPORT_SYMBOL_GPL(machine_power_off);
> @@ -141,12 +145,12 @@ void machine_halt(void)
>  	machine_shutdown();
>  	if (ppc_md.halt)
>  		ppc_md.halt();
> -#ifdef CONFIG_SMP
> -	smp_send_stop();
> -#endif
> -	printk(KERN_EMERG "System Halted, OK to turn off power\n");
> -	local_irq_disable();
> -	while (1) ;
> +	if (ppc_md.power_off) {
> +		printk(KERN_EMERG "System not halted; powering off instead.\n");
> +		ppc_md.power_off();
> +		printk(KERN_EMERG "Poweroff failed.\n");
> +	}
> +	default_halt("OK to turn off power\n");
>  }
>  
> 

[PATCH 5/7] powerpc: Replace ppc_md.power_off with pm_power_off

[Mark A. Greer]

From: Mark A. Greer <mgreer@mvista.com>

The ppc_md.power_off hook performs the same function that the
pm_power_off hook is supposed to.  However, it is powerpc-specific
and prevents kernel drivers (e.g., IPMI) from changing how a platform
is powered off.  So, get rid of ppc_md.power_off and replace it with
pm_power_off.

Signed-off-by: Mark A. Greer <mgreer@mvista.com>
---
 arch/powerpc/kernel/setup-common.c               |   14 +--
 arch/powerpc/platforms/52xx/efika.c              |    2 
 arch/powerpc/platforms/cell/setup.c              |    2 
 arch/powerpc/platforms/celleb/setup.c            |    2 
 arch/powerpc/platforms/chrp/setup.c              |    2 
 arch/powerpc/platforms/embedded6xx/linkstation.c |    3 
 arch/powerpc/platforms/iseries/setup.c           |    2 
 arch/powerpc/platforms/maple/setup.c             |    4 
 arch/powerpc/platforms/powermac/setup.c          |    2 
 arch/powerpc/platforms/ps3/setup.c               |    2 
 arch/powerpc/platforms/pseries/setup.c           |   59 ++++++-------
 include/asm-powerpc/machdep.h                    |    1 
 12 files changed, 48 insertions(+), 47 deletions(-)


diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 1f8f9aa..d9f2e07 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -76,6 +76,9 @@ EXPORT_SYMBOL(ppc_md);
 struct machdep_calls *machine_id;
 EXPORT_SYMBOL(machine_id);
 
+void (*pm_power_off)(void);
+EXPORT_SYMBOL_GPL(pm_power_off);
+
 unsigned long klimit = (unsigned long) _end;
 
 char cmd_line[COMMAND_LINE_SIZE];
@@ -127,8 +130,8 @@ void machine_restart(char *cmd)
 void machine_power_off(void)
 {
 	machine_shutdown();
-	if (ppc_md.power_off)
-		ppc_md.power_off();
+	if (pm_power_off)
+		pm_power_off();
 	printk(KERN_EMERG "System not powered off; halting instead.\n");
 	if (ppc_md.halt)
 		ppc_md.halt();
@@ -137,17 +140,14 @@ void machine_power_off(void)
 /* Used by the G5 thermal driver */
 EXPORT_SYMBOL_GPL(machine_power_off);
 
-void (*pm_power_off)(void) = machine_power_off;
-EXPORT_SYMBOL_GPL(pm_power_off);
-
 void machine_halt(void)
 {
 	machine_shutdown();
 	if (ppc_md.halt)
 		ppc_md.halt();
-	if (ppc_md.power_off) {
+	if (pm_power_off) {
 		printk(KERN_EMERG "System not halted; powering off instead.\n");
-		ppc_md.power_off();
+		pm_power_off();
 		printk(KERN_EMERG "Poweroff failed.\n");
 	}
 	default_halt("OK to turn off power\n");
diff --git a/arch/powerpc/platforms/52xx/efika.c b/arch/powerpc/platforms/52xx/efika.c
index a0da70c..c2d5f06 100644
--- a/arch/powerpc/platforms/52xx/efika.c
+++ b/arch/powerpc/platforms/52xx/efika.c
@@ -205,6 +205,7 @@ static int __init efika_probe(void)
 	DMA_MODE_READ = 0x44;
 	DMA_MODE_WRITE = 0x48;
 
+	pm_power_off = rtas_power_off;
 	return 1;
 }
 
@@ -218,7 +219,6 @@ define_machine(efika)
 	.init_IRQ		= mpc52xx_init_irq,
 	.get_irq		= mpc52xx_get_irq,
 	.restart		= rtas_restart,
-	.power_off		= rtas_power_off,
 	.halt			= rtas_halt,
 	.set_rtc_time		= rtas_set_rtc_time,
 	.get_rtc_time		= rtas_get_rtc_time,
diff --git a/arch/powerpc/platforms/cell/setup.c b/arch/powerpc/platforms/cell/setup.c
index 98e7ef8..06f44f5 100644
--- a/arch/powerpc/platforms/cell/setup.c
+++ b/arch/powerpc/platforms/cell/setup.c
@@ -191,6 +191,7 @@ static int __init cell_probe(void)
 		return 0;
 
 	hpte_init_native();
+	pm_power_off = rtas_power_off;
 
 	return 1;
 }
@@ -201,7 +202,6 @@ define_machine(cell) {
 	.setup_arch		= cell_setup_arch,
 	.show_cpuinfo		= cell_show_cpuinfo,
 	.restart		= rtas_restart,
-	.power_off		= rtas_power_off,
 	.halt			= rtas_halt,
 	.get_boot_time		= rtas_get_boot_time,
 	.get_rtc_time		= rtas_get_rtc_time,
diff --git a/arch/powerpc/platforms/celleb/setup.c b/arch/powerpc/platforms/celleb/setup.c
index ddfb35a..450841a 100644
--- a/arch/powerpc/platforms/celleb/setup.c
+++ b/arch/powerpc/platforms/celleb/setup.c
@@ -116,6 +116,7 @@ static int __init celleb_probe(void)
 
 	powerpc_firmware_features |= FW_FEATURE_CELLEB_POSSIBLE;
 	hpte_init_beat_v3();
+	pm_power_off = beat_power_off;
 	return 1;
 }
 
@@ -145,7 +146,6 @@ define_machine(celleb) {
 	.setup_arch		= celleb_setup_arch,
 	.show_cpuinfo		= celleb_show_cpuinfo,
 	.restart		= beat_restart,
-	.power_off		= beat_power_off,
 	.halt			= beat_halt,
 	.get_rtc_time		= beat_get_rtc_time,
 	.set_rtc_time		= beat_set_rtc_time,
diff --git a/arch/powerpc/platforms/chrp/setup.c b/arch/powerpc/platforms/chrp/setup.c
index 5930626..99edb04 100644
--- a/arch/powerpc/platforms/chrp/setup.c
+++ b/arch/powerpc/platforms/chrp/setup.c
@@ -586,6 +586,7 @@ static int __init chrp_probe(void)
 	DMA_MODE_READ = 0x44;
 	DMA_MODE_WRITE = 0x48;
 
+	pm_power_off = rtas_power_off;
 	return 1;
 }
 
@@ -597,7 +598,6 @@ define_machine(chrp) {
 	.show_cpuinfo		= chrp_show_cpuinfo,
 	.init_IRQ		= chrp_init_IRQ,
 	.restart		= rtas_restart,
-	.power_off		= rtas_power_off,
 	.halt			= rtas_halt,
 	.time_init		= chrp_time_init,
 	.set_rtc_time		= chrp_set_rtc_time,
diff --git a/arch/powerpc/platforms/embedded6xx/linkstation.c b/arch/powerpc/platforms/embedded6xx/linkstation.c
index eb5d74e..8792840 100644
--- a/arch/powerpc/platforms/embedded6xx/linkstation.c
+++ b/arch/powerpc/platforms/embedded6xx/linkstation.c
@@ -182,6 +182,8 @@ static int __init linkstation_probe(void)
 
 	if (!of_flat_dt_is_compatible(root, "linkstation"))
 		return 0;
+
+	pm_power_off = linkstation_power_off;
 	return 1;
 }
 
@@ -193,7 +195,6 @@ define_machine(linkstation){
 	.show_cpuinfo 		= linkstation_show_cpuinfo,
 	.get_irq 		= mpic_get_irq,
 	.restart 		= linkstation_restart,
-	.power_off 		= linkstation_power_off,
 	.halt	 		= linkstation_halt,
 	.calibrate_decr 	= generic_calibrate_decr,
 };
diff --git a/arch/powerpc/platforms/iseries/setup.c b/arch/powerpc/platforms/iseries/setup.c
index 2175a71..6051204 100644
--- a/arch/powerpc/platforms/iseries/setup.c
+++ b/arch/powerpc/platforms/iseries/setup.c
@@ -633,6 +633,7 @@ static int __init iseries_probe(void)
 	/* iSeries does not support 16M pages */
 	cur_cpu_spec->cpu_features &= ~CPU_FTR_16M_PAGE;
 
+	pm_power_off = mf_power_off;
 	return 1;
 }
 
@@ -645,7 +646,6 @@ define_machine(iseries) {
 	.init_early	= iSeries_init_early,
 	.pcibios_fixup	= iSeries_pci_final_fixup,
 	.restart	= mf_reboot,
-	.power_off	= mf_power_off,
 	.halt		= mf_power_off,
 	.get_boot_time	= iSeries_get_boot_time,
 	.set_rtc_time	= iSeries_set_rtc_time,
diff --git a/arch/powerpc/platforms/maple/setup.c b/arch/powerpc/platforms/maple/setup.c
index 144177d..d0eb901 100644
--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -172,7 +172,7 @@ static void __init maple_use_rtas_reboot_and_halt_if_present(void)
 	if (rtas_service_present("system-reboot") &&
 	    rtas_service_present("power-off")) {
 		ppc_md.restart = rtas_restart;
-		ppc_md.power_off = rtas_power_off;
+		pm_power_off = rtas_power_off;
 		ppc_md.halt = rtas_halt;
 	}
 }
@@ -315,6 +315,7 @@ static int __init maple_probe(void)
 	alloc_dart_table();
 
 	hpte_init_native();
+	pm_power_off = maple_power_off;
 
 	return 1;
 }
@@ -328,7 +329,6 @@ define_machine(maple_md) {
 	.pci_irq_fixup		= maple_pci_irq_fixup,
 	.pci_get_legacy_ide_irq	= maple_pci_get_legacy_ide_irq,
 	.restart		= maple_restart,
-	.power_off		= maple_power_off,
 	.halt			= maple_halt,
        	.get_boot_time		= maple_get_boot_time,
        	.set_rtc_time		= maple_set_rtc_time,
diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
index 02c5330..8429002 100644
--- a/arch/powerpc/platforms/powermac/setup.c
+++ b/arch/powerpc/platforms/powermac/setup.c
@@ -577,6 +577,7 @@ static int __init pmac_probe(void)
 
 	hpte_init_native();
 #endif
+	pm_power_off = pmac_power_off;
 
 #ifdef CONFIG_PPC32
 	/* isa_io_base gets set in pmac_pci_init */
@@ -676,7 +677,6 @@ define_machine(powermac) {
 	.get_irq		= NULL,	/* changed later */
 	.pci_irq_fixup		= pmac_pci_irq_fixup,
 	.restart		= pmac_restart,
-	.power_off		= pmac_power_off,
 	.halt			= pmac_halt,
 	.time_init		= pmac_time_init,
 	.get_boot_time		= pmac_get_boot_time,
diff --git a/arch/powerpc/platforms/ps3/setup.c b/arch/powerpc/platforms/ps3/setup.c
index 5c2cbb0..74942d8 100644
--- a/arch/powerpc/platforms/ps3/setup.c
+++ b/arch/powerpc/platforms/ps3/setup.c
@@ -233,6 +233,7 @@ static int __init ps3_probe(void)
 	ps3_mm_init();
 	ps3_mm_vas_create(&htab_size);
 	ps3_hpte_init(htab_size);
+	pm_power_off = ps3_power_off;
 
 	DBG(" <- %s:%d\n", __func__, __LINE__);
 	return 1;
@@ -265,7 +266,6 @@ define_machine(ps3) {
 	.calibrate_decr			= ps3_calibrate_decr,
 	.progress			= ps3_progress,
 	.restart			= ps3_restart,
-	.power_off			= ps3_power_off,
 #if defined(CONFIG_KEXEC)
 	.kexec_cpu_down			= ps3_kexec_cpu_down,
 	.machine_kexec			= default_machine_kexec,
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index fdb9b1c..51ef84c 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -353,6 +353,34 @@ static int __init pSeries_probe_hypertas(unsigned long node,
 	return 1;
 }
 
+/**
+ * pSeries_power_off - tell firmware about how to power off the system.
+ *
+ * This function calls either the power-off rtas token in normal cases
+ * or the ibm,power-off-ups token (if present & requested) in case of
+ * a power failure. If power-off token is used, power on will only be
+ * possible with power button press. If ibm,power-off-ups token is used
+ * it will allow auto poweron after power is restored.
+ */
+void pSeries_power_off(void)
+{
+	int rc;
+	int rtas_poweroff_ups_token = rtas_token("ibm,power-off-ups");
+
+	if (rtas_flash_term_hook)
+		rtas_flash_term_hook(SYS_POWER_OFF);
+
+	if (rtas_poweron_auto == 0 ||
+		rtas_poweroff_ups_token == RTAS_UNKNOWN_SERVICE) {
+		rc = rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1);
+		printk(KERN_INFO "RTAS power-off returned %d\n", rc);
+	} else {
+		rc = rtas_call(rtas_poweroff_ups_token, 0, 1, NULL);
+		printk(KERN_INFO "RTAS ibm,power-off-ups returned %d\n", rc);
+	}
+	for (;;);
+}
+
 static int __init pSeries_probe(void)
 {
 	unsigned long root = of_get_flat_dt_root();
@@ -380,6 +408,8 @@ static int __init pSeries_probe(void)
 	else
 		hpte_init_native();
 
+	pm_power_off = pSeries_power_off;
+
 	DBG("Machine is%s LPAR !\n",
 	    (powerpc_firmware_features & FW_FEATURE_LPAR) ? "" : " not");
 
@@ -463,34 +493,6 @@ static int pSeries_pci_probe_mode(struct pci_bus *bus)
 	return PCI_PROBE_NORMAL;
 }
 
-/**
- * pSeries_power_off - tell firmware about how to power off the system.
- *
- * This function calls either the power-off rtas token in normal cases
- * or the ibm,power-off-ups token (if present & requested) in case of
- * a power failure. If power-off token is used, power on will only be
- * possible with power button press. If ibm,power-off-ups token is used
- * it will allow auto poweron after power is restored.
- */
-void pSeries_power_off(void)
-{
-	int rc;
-	int rtas_poweroff_ups_token = rtas_token("ibm,power-off-ups");
-
-	if (rtas_flash_term_hook)
-		rtas_flash_term_hook(SYS_POWER_OFF);
-
-	if (rtas_poweron_auto == 0 ||
-		rtas_poweroff_ups_token == RTAS_UNKNOWN_SERVICE) {
-		rc = rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1);
-		printk(KERN_INFO "RTAS power-off returned %d\n", rc);
-	} else {
-		rc = rtas_call(rtas_poweroff_ups_token, 0, 1, NULL);
-		printk(KERN_INFO "RTAS ibm,power-off-ups returned %d\n", rc);
-	}
-	for (;;);
-}
-
 #ifndef CONFIG_PCI
 void pSeries_final_fixup(void) { }
 #endif
@@ -505,7 +507,6 @@ define_machine(pseries) {
 	.pcibios_fixup		= pSeries_final_fixup,
 	.pci_probe_mode		= pSeries_pci_probe_mode,
 	.restart		= rtas_restart,
-	.power_off		= pSeries_power_off,
 	.halt			= rtas_halt,
 	.panic			= rtas_os_term,
 	.get_boot_time		= rtas_get_boot_time,
diff --git a/include/asm-powerpc/machdep.h b/include/asm-powerpc/machdep.h
index 6968f43..bca57bc 100644
--- a/include/asm-powerpc/machdep.h
+++ b/include/asm-powerpc/machdep.h
@@ -129,7 +129,6 @@ struct machdep_calls {
 #endif
 
 	void		(*restart)(char *cmd);
-	void		(*power_off)(void);
 	void		(*halt)(void);
 	void		(*panic)(char *str);
 	void		(*cpu_die)(void);

Re: [PATCH 5/7] powerpc: Replace ppc_md.power_off with pm_power_off

[Benjamin Herrenschmidt]

On Mon, 2007-12-03 at 22:48 -0700, Mark A. Greer wrote:
> From: Mark A. Greer <mgreer@mvista.com>
> 
> The ppc_md.power_off hook performs the same function that the
> pm_power_off hook is supposed to.  However, it is powerpc-specific
> and prevents kernel drivers (e.g., IPMI) from changing how a platform
> is powered off.  So, get rid of ppc_md.power_off and replace it with
> pm_power_off.

I'm less happy with that one... probably aesthetics :-)

Can't we just have the generic code call pm_power_off and ppc_md and
which ever powers the machine off wins ?

> Signed-off-by: Mark A. Greer <mgreer@mvista.com>
> ---
>  arch/powerpc/kernel/setup-common.c               |   14 +--
>  arch/powerpc/platforms/52xx/efika.c              |    2 
>  arch/powerpc/platforms/cell/setup.c              |    2 
>  arch/powerpc/platforms/celleb/setup.c            |    2 
>  arch/powerpc/platforms/chrp/setup.c              |    2 
>  arch/powerpc/platforms/embedded6xx/linkstation.c |    3 
>  arch/powerpc/platforms/iseries/setup.c           |    2 
>  arch/powerpc/platforms/maple/setup.c             |    4 
>  arch/powerpc/platforms/powermac/setup.c          |    2 
>  arch/powerpc/platforms/ps3/setup.c               |    2 
>  arch/powerpc/platforms/pseries/setup.c           |   59 ++++++-------
>  include/asm-powerpc/machdep.h                    |    1 
>  12 files changed, 48 insertions(+), 47 deletions(-)
> 
> 
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 1f8f9aa..d9f2e07 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -76,6 +76,9 @@ EXPORT_SYMBOL(ppc_md);
>  struct machdep_calls *machine_id;
>  EXPORT_SYMBOL(machine_id);
>  
> +void (*pm_power_off)(void);
> +EXPORT_SYMBOL_GPL(pm_power_off);
> +
>  unsigned long klimit = (unsigned long) _end;
>  
>  char cmd_line[COMMAND_LINE_SIZE];
> @@ -127,8 +130,8 @@ void machine_restart(char *cmd)
>  void machine_power_off(void)
>  {
>  	machine_shutdown();
> -	if (ppc_md.power_off)
> -		ppc_md.power_off();
> +	if (pm_power_off)
> +		pm_power_off();
>  	printk(KERN_EMERG "System not powered off; halting instead.\n");
>  	if (ppc_md.halt)
>  		ppc_md.halt();
> @@ -137,17 +140,14 @@ void machine_power_off(void)
>  /* Used by the G5 thermal driver */
>  EXPORT_SYMBOL_GPL(machine_power_off);
>  
> -void (*pm_power_off)(void) = machine_power_off;
> -EXPORT_SYMBOL_GPL(pm_power_off);
> -
>  void machine_halt(void)
>  {
>  	machine_shutdown();
>  	if (ppc_md.halt)
>  		ppc_md.halt();
> -	if (ppc_md.power_off) {
> +	if (pm_power_off) {
>  		printk(KERN_EMERG "System not halted; powering off instead.\n");
> -		ppc_md.power_off();
> +		pm_power_off();
>  		printk(KERN_EMERG "Poweroff failed.\n");
>  	}
>  	default_halt("OK to turn off power\n");
> diff --git a/arch/powerpc/platforms/52xx/efika.c b/arch/powerpc/platforms/52xx/efika.c
> index a0da70c..c2d5f06 100644
> --- a/arch/powerpc/platforms/52xx/efika.c
> +++ b/arch/powerpc/platforms/52xx/efika.c
> @@ -205,6 +205,7 @@ static int __init efika_probe(void)
>  	DMA_MODE_READ = 0x44;
>  	DMA_MODE_WRITE = 0x48;
>  
> +	pm_power_off = rtas_power_off;
>  	return 1;
>  }
>  
> @@ -218,7 +219,6 @@ define_machine(efika)
>  	.init_IRQ		= mpc52xx_init_irq,
>  	.get_irq		= mpc52xx_get_irq,
>  	.restart		= rtas_restart,
> -	.power_off		= rtas_power_off,
>  	.halt			= rtas_halt,
>  	.set_rtc_time		= rtas_set_rtc_time,
>  	.get_rtc_time		= rtas_get_rtc_time,
> diff --git a/arch/powerpc/platforms/cell/setup.c b/arch/powerpc/platforms/cell/setup.c
> index 98e7ef8..06f44f5 100644
> --- a/arch/powerpc/platforms/cell/setup.c
> +++ b/arch/powerpc/platforms/cell/setup.c
> @@ -191,6 +191,7 @@ static int __init cell_probe(void)
>  		return 0;
>  
>  	hpte_init_native();
> +	pm_power_off = rtas_power_off;
>  
>  	return 1;
>  }
> @@ -201,7 +202,6 @@ define_machine(cell) {
>  	.setup_arch		= cell_setup_arch,
>  	.show_cpuinfo		= cell_show_cpuinfo,
>  	.restart		= rtas_restart,
> -	.power_off		= rtas_power_off,
>  	.halt			= rtas_halt,
>  	.get_boot_time		= rtas_get_boot_time,
>  	.get_rtc_time		= rtas_get_rtc_time,
> diff --git a/arch/powerpc/platforms/celleb/setup.c b/arch/powerpc/platforms/celleb/setup.c
> index ddfb35a..450841a 100644
> --- a/arch/powerpc/platforms/celleb/setup.c
> +++ b/arch/powerpc/platforms/celleb/setup.c
> @@ -116,6 +116,7 @@ static int __init celleb_probe(void)
>  
>  	powerpc_firmware_features |= FW_FEATURE_CELLEB_POSSIBLE;
>  	hpte_init_beat_v3();
> +	pm_power_off = beat_power_off;
>  	return 1;
>  }
>  
> @@ -145,7 +146,6 @@ define_machine(celleb) {
>  	.setup_arch		= celleb_setup_arch,
>  	.show_cpuinfo		= celleb_show_cpuinfo,
>  	.restart		= beat_restart,
> -	.power_off		= beat_power_off,
>  	.halt			= beat_halt,
>  	.get_rtc_time		= beat_get_rtc_time,
>  	.set_rtc_time		= beat_set_rtc_time,
> diff --git a/arch/powerpc/platforms/chrp/setup.c b/arch/powerpc/platforms/chrp/setup.c
> index 5930626..99edb04 100644
> --- a/arch/powerpc/platforms/chrp/setup.c
> +++ b/arch/powerpc/platforms/chrp/setup.c
> @@ -586,6 +586,7 @@ static int __init chrp_probe(void)
>  	DMA_MODE_READ = 0x44;
>  	DMA_MODE_WRITE = 0x48;
>  
> +	pm_power_off = rtas_power_off;
>  	return 1;
>  }
>  
> @@ -597,7 +598,6 @@ define_machine(chrp) {
>  	.show_cpuinfo		= chrp_show_cpuinfo,
>  	.init_IRQ		= chrp_init_IRQ,
>  	.restart		= rtas_restart,
> -	.power_off		= rtas_power_off,
>  	.halt			= rtas_halt,
>  	.time_init		= chrp_time_init,
>  	.set_rtc_time		= chrp_set_rtc_time,
> diff --git a/arch/powerpc/platforms/embedded6xx/linkstation.c b/arch/powerpc/platforms/embedded6xx/linkstation.c
> index eb5d74e..8792840 100644
> --- a/arch/powerpc/platforms/embedded6xx/linkstation.c
> +++ b/arch/powerpc/platforms/embedded6xx/linkstation.c
> @@ -182,6 +182,8 @@ static int __init linkstation_probe(void)
>  
>  	if (!of_flat_dt_is_compatible(root, "linkstation"))
>  		return 0;
> +
> +	pm_power_off = linkstation_power_off;
>  	return 1;
>  }
>  
> @@ -193,7 +195,6 @@ define_machine(linkstation){
>  	.show_cpuinfo 		= linkstation_show_cpuinfo,
>  	.get_irq 		= mpic_get_irq,
>  	.restart 		= linkstation_restart,
> -	.power_off 		= linkstation_power_off,
>  	.halt	 		= linkstation_halt,
>  	.calibrate_decr 	= generic_calibrate_decr,
>  };
> diff --git a/arch/powerpc/platforms/iseries/setup.c b/arch/powerpc/platforms/iseries/setup.c
> index 2175a71..6051204 100644
> --- a/arch/powerpc/platforms/iseries/setup.c
> +++ b/arch/powerpc/platforms/iseries/setup.c
> @@ -633,6 +633,7 @@ static int __init iseries_probe(void)
>  	/* iSeries does not support 16M pages */
>  	cur_cpu_spec->cpu_features &= ~CPU_FTR_16M_PAGE;
>  
> +	pm_power_off = mf_power_off;
>  	return 1;
>  }
>  
> @@ -645,7 +646,6 @@ define_machine(iseries) {
>  	.init_early	= iSeries_init_early,
>  	.pcibios_fixup	= iSeries_pci_final_fixup,
>  	.restart	= mf_reboot,
> -	.power_off	= mf_power_off,
>  	.halt		= mf_power_off,
>  	.get_boot_time	= iSeries_get_boot_time,
>  	.set_rtc_time	= iSeries_set_rtc_time,
> diff --git a/arch/powerpc/platforms/maple/setup.c b/arch/powerpc/platforms/maple/setup.c
> index 144177d..d0eb901 100644
> --- a/arch/powerpc/platforms/maple/setup.c
> +++ b/arch/powerpc/platforms/maple/setup.c
> @@ -172,7 +172,7 @@ static void __init maple_use_rtas_reboot_and_halt_if_present(void)
>  	if (rtas_service_present("system-reboot") &&
>  	    rtas_service_present("power-off")) {
>  		ppc_md.restart = rtas_restart;
> -		ppc_md.power_off = rtas_power_off;
> +		pm_power_off = rtas_power_off;
>  		ppc_md.halt = rtas_halt;
>  	}
>  }
> @@ -315,6 +315,7 @@ static int __init maple_probe(void)
>  	alloc_dart_table();
>  
>  	hpte_init_native();
> +	pm_power_off = maple_power_off;
>  
>  	return 1;
>  }
> @@ -328,7 +329,6 @@ define_machine(maple_md) {
>  	.pci_irq_fixup		= maple_pci_irq_fixup,
>  	.pci_get_legacy_ide_irq	= maple_pci_get_legacy_ide_irq,
>  	.restart		= maple_restart,
> -	.power_off		= maple_power_off,
>  	.halt			= maple_halt,
>         	.get_boot_time		= maple_get_boot_time,
>         	.set_rtc_time		= maple_set_rtc_time,
> diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
> index 02c5330..8429002 100644
> --- a/arch/powerpc/platforms/powermac/setup.c
> +++ b/arch/powerpc/platforms/powermac/setup.c
> @@ -577,6 +577,7 @@ static int __init pmac_probe(void)
>  
>  	hpte_init_native();
>  #endif
> +	pm_power_off = pmac_power_off;
>  
>  #ifdef CONFIG_PPC32
>  	/* isa_io_base gets set in pmac_pci_init */
> @@ -676,7 +677,6 @@ define_machine(powermac) {
>  	.get_irq		= NULL,	/* changed later */
>  	.pci_irq_fixup		= pmac_pci_irq_fixup,
>  	.restart		= pmac_restart,
> -	.power_off		= pmac_power_off,
>  	.halt			= pmac_halt,
>  	.time_init		= pmac_time_init,
>  	.get_boot_time		= pmac_get_boot_time,
> diff --git a/arch/powerpc/platforms/ps3/setup.c b/arch/powerpc/platforms/ps3/setup.c
> index 5c2cbb0..74942d8 100644
> --- a/arch/powerpc/platforms/ps3/setup.c
> +++ b/arch/powerpc/platforms/ps3/setup.c
> @@ -233,6 +233,7 @@ static int __init ps3_probe(void)
>  	ps3_mm_init();
>  	ps3_mm_vas_create(&htab_size);
>  	ps3_hpte_init(htab_size);
> +	pm_power_off = ps3_power_off;
>  
>  	DBG(" <- %s:%d\n", __func__, __LINE__);
>  	return 1;
> @@ -265,7 +266,6 @@ define_machine(ps3) {
>  	.calibrate_decr			= ps3_calibrate_decr,
>  	.progress			= ps3_progress,
>  	.restart			= ps3_restart,
> -	.power_off			= ps3_power_off,
>  #if defined(CONFIG_KEXEC)
>  	.kexec_cpu_down			= ps3_kexec_cpu_down,
>  	.machine_kexec			= default_machine_kexec,
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index fdb9b1c..51ef84c 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -353,6 +353,34 @@ static int __init pSeries_probe_hypertas(unsigned long node,
>  	return 1;
>  }
>  
> +/**
> + * pSeries_power_off - tell firmware about how to power off the system.
> + *
> + * This function calls either the power-off rtas token in normal cases
> + * or the ibm,power-off-ups token (if present & requested) in case of
> + * a power failure. If power-off token is used, power on will only be
> + * possible with power button press. If ibm,power-off-ups token is used
> + * it will allow auto poweron after power is restored.
> + */
> +void pSeries_power_off(void)
> +{
> +	int rc;
> +	int rtas_poweroff_ups_token = rtas_token("ibm,power-off-ups");
> +
> +	if (rtas_flash_term_hook)
> +		rtas_flash_term_hook(SYS_POWER_OFF);
> +
> +	if (rtas_poweron_auto == 0 ||
> +		rtas_poweroff_ups_token == RTAS_UNKNOWN_SERVICE) {
> +		rc = rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1);
> +		printk(KERN_INFO "RTAS power-off returned %d\n", rc);
> +	} else {
> +		rc = rtas_call(rtas_poweroff_ups_token, 0, 1, NULL);
> +		printk(KERN_INFO "RTAS ibm,power-off-ups returned %d\n", rc);
> +	}
> +	for (;;);
> +}
> +
>  static int __init pSeries_probe(void)
>  {
>  	unsigned long root = of_get_flat_dt_root();
> @@ -380,6 +408,8 @@ static int __init pSeries_probe(void)
>  	else
>  		hpte_init_native();
>  
> +	pm_power_off = pSeries_power_off;
> +
>  	DBG("Machine is%s LPAR !\n",
>  	    (powerpc_firmware_features & FW_FEATURE_LPAR) ? "" : " not");
>  
> @@ -463,34 +493,6 @@ static int pSeries_pci_probe_mode(struct pci_bus *bus)
>  	return PCI_PROBE_NORMAL;
>  }
>  
> -/**
> - * pSeries_power_off - tell firmware about how to power off the system.
> - *
> - * This function calls either the power-off rtas token in normal cases
> - * or the ibm,power-off-ups token (if present & requested) in case of
> - * a power failure. If power-off token is used, power on will only be
> - * possible with power button press. If ibm,power-off-ups token is used
> - * it will allow auto poweron after power is restored.
> - */
> -void pSeries_power_off(void)
> -{
> -	int rc;
> -	int rtas_poweroff_ups_token = rtas_token("ibm,power-off-ups");
> -
> -	if (rtas_flash_term_hook)
> -		rtas_flash_term_hook(SYS_POWER_OFF);
> -
> -	if (rtas_poweron_auto == 0 ||
> -		rtas_poweroff_ups_token == RTAS_UNKNOWN_SERVICE) {
> -		rc = rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1);
> -		printk(KERN_INFO "RTAS power-off returned %d\n", rc);
> -	} else {
> -		rc = rtas_call(rtas_poweroff_ups_token, 0, 1, NULL);
> -		printk(KERN_INFO "RTAS ibm,power-off-ups returned %d\n", rc);
> -	}
> -	for (;;);
> -}
> -
>  #ifndef CONFIG_PCI
>  void pSeries_final_fixup(void) { }
>  #endif
> @@ -505,7 +507,6 @@ define_machine(pseries) {
>  	.pcibios_fixup		= pSeries_final_fixup,
>  	.pci_probe_mode		= pSeries_pci_probe_mode,
>  	.restart		= rtas_restart,
> -	.power_off		= pSeries_power_off,
>  	.halt			= rtas_halt,
>  	.panic			= rtas_os_term,
>  	.get_boot_time		= rtas_get_boot_time,
> diff --git a/include/asm-powerpc/machdep.h b/include/asm-powerpc/machdep.h
> index 6968f43..bca57bc 100644
> --- a/include/asm-powerpc/machdep.h
> +++ b/include/asm-powerpc/machdep.h
> @@ -129,7 +129,6 @@ struct machdep_calls {
>  #endif
>  
>  	void		(*restart)(char *cmd);
> -	void		(*power_off)(void);
>  	void		(*halt)(void);
>  	void		(*panic)(char *str);
>  	void		(*cpu_die)(void);

Re: [PATCH 5/7] powerpc: Replace ppc_md.power_off with pm_power_off

[Mark A. Greer]

On Tue, Dec 04, 2007 at 06:23:09PM +1100, Benjamin Herrenschmidt wrote:
> 
> On Mon, 2007-12-03 at 22:48 -0700, Mark A. Greer wrote:
> > From: Mark A. Greer <mgreer@mvista.com>
> > 
> > The ppc_md.power_off hook performs the same function that the
> > pm_power_off hook is supposed to.  However, it is powerpc-specific
> > and prevents kernel drivers (e.g., IPMI) from changing how a platform
> > is powered off.  So, get rid of ppc_md.power_off and replace it with
> > pm_power_off.
> 
> I'm less happy with that one... probably aesthetics :-)
> 
> Can't we just have the generic code call pm_power_off and ppc_md and
> which ever powers the machine off wins ?

Yes, that would be easy to do.  Seems like duplication though.
If you are sure you're okay with the duplication, I'll do that.

Mark

Re: [PATCH 5/7] powerpc: Replace ppc_md.power_off with pm_power_off

[Benjamin Herrenschmidt]

On Tue, 2007-12-04 at 11:01 -0700, Mark A. Greer wrote:
> On Tue, Dec 04, 2007 at 06:23:09PM +1100, Benjamin Herrenschmidt wrote:
> > 
> > On Mon, 2007-12-03 at 22:48 -0700, Mark A. Greer wrote:
> > > From: Mark A. Greer <mgreer@mvista.com>
> > > 
> > > The ppc_md.power_off hook performs the same function that the
> > > pm_power_off hook is supposed to.  However, it is powerpc-specific
> > > and prevents kernel drivers (e.g., IPMI) from changing how a platform
> > > is powered off.  So, get rid of ppc_md.power_off and replace it with
> > > pm_power_off.
> > 
> > I'm less happy with that one... probably aesthetics :-)
> > 
> > Can't we just have the generic code call pm_power_off and ppc_md and
> > which ever powers the machine off wins ?
> 
> Yes, that would be easy to do.  Seems like duplication though.
> If you are sure you're okay with the duplication, I'll do that.

Let's ask Paulus what he thinks.

Ben.

Re: [PATCH 5/7] powerpc: Replace ppc_md.power_off with pm_power_off

[Grant Likely]

On 12/4/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Tue, 2007-12-04 at 11:01 -0700, Mark A. Greer wrote:
> > On Tue, Dec 04, 2007 at 06:23:09PM +1100, Benjamin Herrenschmidt wrote:
> > >
> > > On Mon, 2007-12-03 at 22:48 -0700, Mark A. Greer wrote:
> > > > From: Mark A. Greer <mgreer@mvista.com>
> > > >
> > > > The ppc_md.power_off hook performs the same function that the
> > > > pm_power_off hook is supposed to.  However, it is powerpc-specific
> > > > and prevents kernel drivers (e.g., IPMI) from changing how a platform
> > > > is powered off.  So, get rid of ppc_md.power_off and replace it with
> > > > pm_power_off.
> > >
> > > I'm less happy with that one... probably aesthetics :-)
> > >
> > > Can't we just have the generic code call pm_power_off and ppc_md and
> > > which ever powers the machine off wins ?
> >
> > Yes, that would be easy to do.  Seems like duplication though.
> > If you are sure you're okay with the duplication, I'll do that.
>
> Let's ask Paulus what he thinks.

We could simply have the setup code copy the ppc_md.power_off pointer
into pm_power_off; that we retain the nice assignment in
define_machine(), but eliminate the duplicated calls.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH 5/7] powerpc: Replace ppc_md.power_off with pm_power_off

[Benjamin Herrenschmidt]

On Tue, 2007-12-04 at 13:05 -0700, Grant Likely wrote:
> We could simply have the setup code copy the ppc_md.power_off pointer
> into pm_power_off; that we retain the nice assignment in
> define_machine(), but eliminate the duplicated calls.

Good idea.

Ben.

Re: [PATCH 5/7] powerpc: Replace ppc_md.power_off with pm_power_off

[Mark A. Greer]

On Tue, Dec 04, 2007 at 01:05:46PM -0700, Grant Likely wrote:
> On 12/4/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> >
> > On Tue, 2007-12-04 at 11:01 -0700, Mark A. Greer wrote:
> > > On Tue, Dec 04, 2007 at 06:23:09PM +1100, Benjamin Herrenschmidt wrote:
> > > >
> > > > On Mon, 2007-12-03 at 22:48 -0700, Mark A. Greer wrote:
> > > > > From: Mark A. Greer <mgreer@mvista.com>
> > > > >
> > > > > The ppc_md.power_off hook performs the same function that the
> > > > > pm_power_off hook is supposed to.  However, it is powerpc-specific
> > > > > and prevents kernel drivers (e.g., IPMI) from changing how a platform
> > > > > is powered off.  So, get rid of ppc_md.power_off and replace it with
> > > > > pm_power_off.
> > > >
> > > > I'm less happy with that one... probably aesthetics :-)
> > > >
> > > > Can't we just have the generic code call pm_power_off and ppc_md and
> > > > which ever powers the machine off wins ?
> > >
> > > Yes, that would be easy to do.  Seems like duplication though.
> > > If you are sure you're okay with the duplication, I'll do that.
> >
> > Let's ask Paulus what he thinks.
> 
> We could simply have the setup code copy the ppc_md.power_off pointer
> into pm_power_off; that we retain the nice assignment in
> define_machine(), but eliminate the duplicated calls.

Hmm, yeah, that would look nice--nicer than what I have.  The only
issue I have with it is that we still have duplication and potential
for reassigning the wrong one (e.g., reassigning ppc_md.power_off instead
of pm_power_off in maple/setup.c:maple_use_rtas_reboot_and_halt_if_present()).

We could call both in machine_power_off but that's messy too (IMHO).

Paul, do you have an opinion?

Mark

[PATCH 6/7] powerpc: Remove redundant power_off and halt routines

[Mark A. Greer]

From: Mark A. Greer <mgreer@mvista.com>

Remove platform-specific power_off and halt routines, and ppc_md
initializations that are not necessary.

Signed-off-by: Mark A. Greer <mgreer@mvista.com>
---
 arch/powerpc/platforms/embedded6xx/holly.c        |   12 ------------
 arch/powerpc/platforms/embedded6xx/linkstation.c  |    7 -------
 arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c |   11 -----------
 arch/powerpc/platforms/iseries/setup.c            |    1 -
 arch/powerpc/platforms/maple/setup.c              |    6 ------
 arch/powerpc/platforms/powermac/setup.c           |    7 -------
 6 files changed, 44 deletions(-)

diff --git a/arch/powerpc/platforms/embedded6xx/holly.c b/arch/powerpc/platforms/embedded6xx/holly.c
index b6de2b5..70cfd37 100644
--- a/arch/powerpc/platforms/embedded6xx/holly.c
+++ b/arch/powerpc/platforms/embedded6xx/holly.c
@@ -253,18 +253,6 @@ void holly_restart(char *cmd)
 	for (;;) ;
 }
 
-void holly_power_off(void)
-{
-	local_irq_disable();
-	/* No way to shut power off with software */
-	for (;;) ;
-}
-
-void holly_halt(void)
-{
-	holly_power_off();
-}
-
 /*
  * Called very early, device-tree isn't unflattened
  */
diff --git a/arch/powerpc/platforms/embedded6xx/linkstation.c b/arch/powerpc/platforms/embedded6xx/linkstation.c
index 8792840..3382eef 100644
--- a/arch/powerpc/platforms/embedded6xx/linkstation.c
+++ b/arch/powerpc/platforms/embedded6xx/linkstation.c
@@ -162,12 +162,6 @@ static void linkstation_power_off(void)
 	/* NOTREACHED */
 }
 
-static void linkstation_halt(void)
-{
-	linkstation_power_off();
-	/* NOTREACHED */
-}
-
 static void linkstation_show_cpuinfo(struct seq_file *m)
 {
 	seq_printf(m, "vendor\t\t: Buffalo Technology\n");
@@ -195,6 +189,5 @@ define_machine(linkstation){
 	.show_cpuinfo 		= linkstation_show_cpuinfo,
 	.get_irq 		= mpic_get_irq,
 	.restart 		= linkstation_restart,
-	.halt	 		= linkstation_halt,
 	.calibrate_decr 	= generic_calibrate_decr,
 };
diff --git a/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c b/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c
index a2c04b9..557a6fe 100644
--- a/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c
+++ b/arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c
@@ -179,17 +179,6 @@ void mpc7448_hpc2_restart(char *cmd)
 	for (;;) ;		/* Spin until reset happens */
 }
 
-void mpc7448_hpc2_power_off(void)
-{
-	local_irq_disable();
-	for (;;) ;		/* No way to shut power off with software */
-}
-
-void mpc7448_hpc2_halt(void)
-{
-	mpc7448_hpc2_power_off();
-}
-
 /*
  * Called very early, device-tree isn't unflattened
  */
diff --git a/arch/powerpc/platforms/iseries/setup.c b/arch/powerpc/platforms/iseries/setup.c
index 362084e..0eabbc3 100644
--- a/arch/powerpc/platforms/iseries/setup.c
+++ b/arch/powerpc/platforms/iseries/setup.c
@@ -650,7 +650,6 @@ define_machine(iseries) {
 	.init_early	= iSeries_init_early,
 	.pcibios_fixup	= iSeries_pci_final_fixup,
 	.restart	= mf_reboot,
-	.halt		= mf_power_off,
 	.get_boot_time	= iSeries_get_boot_time,
 	.set_rtc_time	= iSeries_set_rtc_time,
 	.get_rtc_time	= iSeries_get_rtc_time,
diff --git a/arch/powerpc/platforms/maple/setup.c b/arch/powerpc/platforms/maple/setup.c
index d0eb901..ac5f99a 100644
--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -151,11 +151,6 @@ static void maple_power_off(void)
 	printk(KERN_EMERG "Maple: Manual Power-Down Required\n");
 }
 
-static void maple_halt(void)
-{
-	maple_power_off();
-}
-
 #ifdef CONFIG_SMP
 struct smp_ops_t maple_smp_ops = {
 	.probe		= smp_mpic_probe,
@@ -329,7 +324,6 @@ define_machine(maple_md) {
 	.pci_irq_fixup		= maple_pci_irq_fixup,
 	.pci_get_legacy_ide_irq	= maple_pci_get_legacy_ide_irq,
 	.restart		= maple_restart,
-	.halt			= maple_halt,
        	.get_boot_time		= maple_get_boot_time,
        	.set_rtc_time		= maple_set_rtc_time,
        	.get_rtc_time		= maple_get_rtc_time,
diff --git a/arch/powerpc/platforms/powermac/setup.c b/arch/powerpc/platforms/powermac/setup.c
index 8429002..002b0b4 100644
--- a/arch/powerpc/platforms/powermac/setup.c
+++ b/arch/powerpc/platforms/powermac/setup.c
@@ -499,12 +499,6 @@ static void pmac_power_off(void)
 	}
 }
 
-static void
-pmac_halt(void)
-{
-	pmac_power_off();
-}
-
 /* 
  * Early initialization.
  */
@@ -677,7 +671,6 @@ define_machine(powermac) {
 	.get_irq		= NULL,	/* changed later */
 	.pci_irq_fixup		= pmac_pci_irq_fixup,
 	.restart		= pmac_restart,
-	.halt			= pmac_halt,
 	.time_init		= pmac_time_init,
 	.get_boot_time		= pmac_get_boot_time,
 	.set_rtc_time		= pmac_set_rtc_time,

[PATCH 7/7] powerpc: Remove incorrect panic() calls

[Mark A. Greer]

From: Mark A. Greer <mgreer@mvista.com>

Platform-specific restart routines should not call panic() when they
fail.  Instead, they should return so the caller (machine_restart())
can halt the system more gracefully.

Signed-off-by: Mark A. Greer <mgreer@mvista.com>
---
 arch/powerpc/platforms/82xx/pq2.c              |    2 --
 arch/powerpc/platforms/8xx/m8xx_setup.c        |    1 -
 arch/powerpc/platforms/embedded6xx/prpmc2800.c |    1 -
 3 files changed, 4 deletions(-)

diff --git a/arch/powerpc/platforms/82xx/pq2.c b/arch/powerpc/platforms/82xx/pq2.c
index a497cba..16cd460 100644
--- a/arch/powerpc/platforms/82xx/pq2.c
+++ b/arch/powerpc/platforms/82xx/pq2.c
@@ -31,8 +31,6 @@ void pq2_restart(char *cmd)
 	/* Clear the ME,EE,IR & DR bits in MSR to cause checkstop */
 	mtmsr(mfmsr() & ~(MSR_ME | MSR_EE | MSR_IR | MSR_DR));
 	in_8(&cpm2_immr->im_clkrst.res[0]);
-
-	panic("Restart failed\n");
 }
 
 #ifdef CONFIG_PCI
diff --git a/arch/powerpc/platforms/8xx/m8xx_setup.c b/arch/powerpc/platforms/8xx/m8xx_setup.c
index d35eda8..1014310 100644
--- a/arch/powerpc/platforms/8xx/m8xx_setup.c
+++ b/arch/powerpc/platforms/8xx/m8xx_setup.c
@@ -221,7 +221,6 @@ void mpc8xx_restart(char *cmd)
 	mtmsr(mfmsr() & ~0x1000);
 
 	in_8(&clk_r->res[0]);
-	panic("Restart failed\n");
 }
 
 static void cpm_cascade(unsigned int irq, struct irq_desc *desc)
diff --git a/arch/powerpc/platforms/embedded6xx/prpmc2800.c b/arch/powerpc/platforms/embedded6xx/prpmc2800.c
index 653a5eb..fe5920c 100644
--- a/arch/powerpc/platforms/embedded6xx/prpmc2800.c
+++ b/arch/powerpc/platforms/embedded6xx/prpmc2800.c
@@ -108,7 +108,6 @@ static void prpmc2800_restart(char *cmd)
 	prpmc2800_reset_board();
 
 	while (i-- > 0);
-	panic("restart failed\n");
 }
 
 #ifdef CONFIG_NOT_COHERENT_CACHE