2005-12-11 23:24:39

by Paul Jackson

[permalink] [raw]
Subject: [PATCH] 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(+)

--- 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, if any
+ */
+void (*pm_power_off)(void);
+
void
cpu_idle(void)
{

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401


2005-12-12 05:25:33

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH] alpha build pm_power_off hack

Paul Jackson <[email protected]> writes:

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

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

Eric

2005-12-12 06:24:28

by Paul Jackson

[permalink] [raw]
Subject: Re: [PATCH] alpha build pm_power_off hack

Eric wrote:
> 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.

--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401

2005-12-12 12:12:26

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH] alpha build pm_power_off hack

Paul Jackson <[email protected]> writes:

> Eric wrote:
>> 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.

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

> 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)
{



2005-12-12 13:44:30

by Ivan Kokshaysky

[permalink] [raw]
Subject: Re: [PATCH] alpha build pm_power_off hack

On Mon, Dec 12, 2005 at 05:10:34AM -0700, Eric W. Biederman wrote:
> +/*
> + * 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.