Skip to contentSkip to conversation

alpha build pm_power_off hack

PATCH In review Paul Jackson 3 participants 5 messages Dec 11, 2005 – Dec 12, 2005 Alpha port

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 alpha build pm_power_off hack

This follows up Eric W. Biederman's patch of Dec 8, 2005:
[PATCH] Don't attempt to power off if power off is not implemented.

To avoid having problems with one arch break the crosstool builds which developers for other arch's do to ensure they haven't added an arch-specific build bug, add a NULL pm_power_off() function pointer definition to the alpha build.

Without this change, an alpha build fails in the final link stage, for the missing 'pm_power_off' symbol that is used in kernel/sys.c

If the alpha developers don't like the behaviour of '/sbin/halt' on their kernel, I will leave that to them to figure out.

Signed-off-by Paul Jackson

---

 arch/alpha/kernel/process.c |    5 +++++
 1 files changed, 5 insertions(+)
arch/alpha/kernel/process.c +5 -0
@@ -43,6 +43,11 @@
 #include "proto.h"
 #include "pci_impl.h"
 
+/*
+ * Power off function, if any
+ */
+void (*pm_power_off)(void);
+
 void
 cpu_idle(void)
 {
Signature · 3 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401
#1 lore raw
Quoted 8 lines · Paul Jackson

This follows up Eric W. Biederman's patch of Dec 8, 2005:
[PATCH] Don't attempt to power off if power off is not implemented.

To avoid having problems with one arch break the crosstool builds which developers for other arch's do to ensure they haven't added an arch-specific build bug, add a NULL pm_power_off() function pointer definition to the alpha build.

Quoted 6 lines

Without this change, an alpha build fails in the final link stage, for the missing 'pm_power_off' symbol that is used in kernel/sys.c

If the alpha developers don't like the behaviour of '/sbin/halt' on their kernel, I will leave that to them to figure out.

Taking a quick glance at alpha causes me to think we always want pm_power_off to be non null on alpha.

Eric

#2 lore raw
Quoted 2 lines · Eric

Taking a quick glance at alpha causes me to think we always want pm_power_off to be non null on alpha.

So I presume you think that some alpha person should write such a function?

I can't quite guess whether you are agreeing with my patch, or disagreeing with it.

At the very least, I don't want to leave the crosstools build of alpha with a default config broken.

Signature · 3 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.925.600.0401
#3 lore raw
Quoted 6 lines · Paul Jackson
Quoted 2 lines · Eric

Taking a quick glance at alpha causes me to think we always want pm_power_off to be non null on alpha.

So I presume you think that some alpha person should write such a function?

Sorry I had about 5 minutes to do something when this issue came up and unfortunately I knew how to solve it cleanly :( So I wrote the patch. It was my hope that by putting it into -mm I could sucker someone with some time into doing fixing the architectures that broke.

I think what we want is one of the too patches below. Basically something that just sets pm_power_off to a non NULL value is enough to continue the alphas current behaviour.

Quoted 2 lines

I can't quite guess whether you are agreeing with my patch, or disagreeing with it.

Disagreeing in saying it didn't quite go far enough pm_power_off should be initialized on alpha.

Quoted 2 lines

At the very least, I don't want to leave the crosstools build of alpha with a default config broken.

That is a good point. But I really don't want to see a fix that just blindly fixes a build issue. When it takes about 10 minutes to audit machine_power_off and see what really needs to be done.

To that extent I have hacked up your patch two different ways one of them should be usable. I'm just not certain which non-zero initializer I like better. The second setting pm_power_off to machine_power_off is what the powerpc port currently does.

Eric

---

 arch/alpha/kernel/process.c |    5 +++++
 1 files changed, 5 insertions(+)

--- 2.6.15-rc5-mm2.orig/arch/alpha/kernel/process.c 2005-12-11 15:07:52.000000000 -0800
+++ 2.6.15-rc5-mm2/arch/alpha/kernel/process.c 2005-12-11 15:09:33.000000000 -0800
@@ -43,6 +43,11 @@
#include "proto.h"
#include "pci_impl.h"

+/*
+ * Power off function, alpha doesn't use but we should
+ * always call machine_power_off.
+ */
+void (*pm_power_off)(void) = ERR_PTR(-EINVAL); +
void
cpu_idle(void)
{

---

 arch/alpha/kernel/process.c |    5 +++++
 1 files changed, 5 insertions(+)

--- 2.6.15-rc5-mm2.orig/arch/alpha/kernel/process.c 2005-12-11 15:07:52.000000000 -0800
+++ 2.6.15-rc5-mm2/arch/alpha/kernel/process.c 2005-12-11 15:09:33.000000000 -0800
@@ -43,6 +43,11 @@
#include "proto.h"
#include "pci_impl.h"

+/*
+ * Power off function, alpha doesn't use but we should
+ * always call machine_power_off.
+ */
+void (*pm_power_off)(void) = machine_power_off; +
void
cpu_idle(void)
{

#4 lore raw

arch/alpha/kernel/process.c line 50

Quoted 6 lines · Eric W. Biederman
arch/alpha/kernel/process.c +6 -0
+/*
+ * Power off function, alpha doesn't use but we should
+ * always call machine_power_off.
+ */
+void (*pm_power_off)(void) = machine_power_off;
+

This one makes sense, thanks.

Ivan.