2006-02-05 12:56:13

by Olaf Hering

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

On Fri, Feb 03, Linux Kernel Mailing List wrote:

> tree 8f70444139c8564c0f1e88e1f33adda036ae6a96
> parent 278ff9537030bbb292b33504f5e1f6e0126793eb
> author Dave Jones <[email protected]> Fri, 03 Feb 2006 19:03:44 -0800
> committer Linus Torvalds <[email protected]> Sat, 04 Feb 2006 00:32:00 -0800
>
> [PATCH] Fix build failure in recent pm_prepare_* changes.
>
> kernel/power/power.h:49: error: static declaration of 'pm_prepare_console' follows non-static declaration
> include/linux/suspend.h:46: error: previous declaration of 'pm_prepare_console' was here
> kernel/power/power.h:50: error: static declaration of 'pm_restore_console' follows non-static declaration
> include/linux/suspend.h:47: error: previous declaration of 'pm_restore_console' was here
>
> Signed-off-by: Dave Jones <[email protected]>

this one is not correct, please have a closer look at
f7b8988ff50d99c99746f65f420364e91362c065

CC drivers/macintosh/via-pmu.o
drivers/macintosh/via-pmu.c: In function 'pmac_suspend_devices':
drivers/macintosh/via-pmu.c:2078: error: implicit declaration of function 'pm_prepare_console'
drivers/macintosh/via-pmu.c: In function 'pmac_wakeup_devices':
drivers/macintosh/via-pmu.c:2194: error: implicit declaration of function 'pm_restore_console'
make[2]: *** [drivers/macintosh/via-pmu.o] Error 1



--
short story of a lazy sysadmin:
alias appserv=wotan


2006-02-05 19:02:54

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

On Sun, Feb 05, 2006 at 01:56:10PM +0100, Olaf Hering wrote:
> On Fri, Feb 03, Linux Kernel Mailing List wrote:
>
> > tree 8f70444139c8564c0f1e88e1f33adda036ae6a96
> > parent 278ff9537030bbb292b33504f5e1f6e0126793eb
> > author Dave Jones <[email protected]> Fri, 03 Feb 2006 19:03:44 -0800
> > committer Linus Torvalds <[email protected]> Sat, 04 Feb 2006 00:32:00 -0800
> >
> > [PATCH] Fix build failure in recent pm_prepare_* changes.
> >
> > kernel/power/power.h:49: error: static declaration of 'pm_prepare_console' follows non-static declaration
> > include/linux/suspend.h:46: error: previous declaration of 'pm_prepare_console' was here
> > kernel/power/power.h:50: error: static declaration of 'pm_restore_console' follows non-static declaration
> > include/linux/suspend.h:47: error: previous declaration of 'pm_restore_console' was here
> >
> > Signed-off-by: Dave Jones <[email protected]>
>
> this one is not correct, please have a closer look at
> f7b8988ff50d99c99746f65f420364e91362c065
>
> CC drivers/macintosh/via-pmu.o
> drivers/macintosh/via-pmu.c: In function 'pmac_suspend_devices':
> drivers/macintosh/via-pmu.c:2078: error: implicit declaration of function 'pm_prepare_console'
> drivers/macintosh/via-pmu.c: In function 'pmac_wakeup_devices':
> drivers/macintosh/via-pmu.c:2194: error: implicit declaration of function 'pm_restore_console'
> make[2]: *** [drivers/macintosh/via-pmu.o] Error 1

Strange, my ppc[64] builds compiled and linked without failure. I think perhaps
it's time I added -Werror-implicit-function-declaration to the Makefile.

I'll not get a chance to look into fixing this until tomorrow, so if Rafael/Pavel
have time before then, maybe they'll beat me to it.


Dave

2006-02-05 19:50:48

by Adrian Bunk

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

On Sun, Feb 05, 2006 at 02:02:20PM -0500, Dave Jones wrote:
> On Sun, Feb 05, 2006 at 01:56:10PM +0100, Olaf Hering wrote:
> > On Fri, Feb 03, Linux Kernel Mailing List wrote:
> >
> > > tree 8f70444139c8564c0f1e88e1f33adda036ae6a96
> > > parent 278ff9537030bbb292b33504f5e1f6e0126793eb
> > > author Dave Jones <[email protected]> Fri, 03 Feb 2006 19:03:44 -0800
> > > committer Linus Torvalds <[email protected]> Sat, 04 Feb 2006 00:32:00 -0800
> > >
> > > [PATCH] Fix build failure in recent pm_prepare_* changes.
> > >
> > > kernel/power/power.h:49: error: static declaration of 'pm_prepare_console' follows non-static declaration
> > > include/linux/suspend.h:46: error: previous declaration of 'pm_prepare_console' was here
> > > kernel/power/power.h:50: error: static declaration of 'pm_restore_console' follows non-static declaration
> > > include/linux/suspend.h:47: error: previous declaration of 'pm_restore_console' was here
> > >
> > > Signed-off-by: Dave Jones <[email protected]>
> >
> > this one is not correct, please have a closer look at
> > f7b8988ff50d99c99746f65f420364e91362c065
> >
> > CC drivers/macintosh/via-pmu.o
> > drivers/macintosh/via-pmu.c: In function 'pmac_suspend_devices':
> > drivers/macintosh/via-pmu.c:2078: error: implicit declaration of function 'pm_prepare_console'
> > drivers/macintosh/via-pmu.c: In function 'pmac_wakeup_devices':
> > drivers/macintosh/via-pmu.c:2194: error: implicit declaration of function 'pm_restore_console'
> > make[2]: *** [drivers/macintosh/via-pmu.o] Error 1
>
> Strange, my ppc[64] builds compiled and linked without failure. I think perhaps
> it's time I added -Werror-implicit-function-declaration to the Makefile.
>...

If it compiles and links and gcc guessed the prototype correctly
everything is fine. If it compiles and links and gcc guessed the
prototype wrongly, you have a hard to find runtime error...

I'd love to add this flag to the global Makefile, but Andrew always
rejected it because it turns link errors into compile errors (sic)
breaking his powerpc all*config builds due to virt_to_bus/bus_to_virt.
But then he rejected to mark virt_to_bus/bus_to_virt as __deprecated on
i386 because it currently generates some warnings... :-(

> Dave

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2006-02-06 07:28:54

by Olaf Hering

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

On Sun, Feb 05, Dave Jones wrote:

> On Sun, Feb 05, 2006 at 01:56:10PM +0100, Olaf Hering wrote:
> > On Fri, Feb 03, Linux Kernel Mailing List wrote:
> >
> > > tree 8f70444139c8564c0f1e88e1f33adda036ae6a96
> > > parent 278ff9537030bbb292b33504f5e1f6e0126793eb
> > > author Dave Jones <[email protected]> Fri, 03 Feb 2006 19:03:44 -0800
> > > committer Linus Torvalds <[email protected]> Sat, 04 Feb 2006 00:32:00 -0800
> > >
> > > [PATCH] Fix build failure in recent pm_prepare_* changes.
> > >
> > > kernel/power/power.h:49: error: static declaration of 'pm_prepare_console' follows non-static declaration
> > > include/linux/suspend.h:46: error: previous declaration of 'pm_prepare_console' was here
> > > kernel/power/power.h:50: error: static declaration of 'pm_restore_console' follows non-static declaration
> > > include/linux/suspend.h:47: error: previous declaration of 'pm_restore_console' was here
> > >
> > > Signed-off-by: Dave Jones <[email protected]>
> >
> > this one is not correct, please have a closer look at
> > f7b8988ff50d99c99746f65f420364e91362c065
> >
> > CC drivers/macintosh/via-pmu.o
> > drivers/macintosh/via-pmu.c: In function 'pmac_suspend_devices':
> > drivers/macintosh/via-pmu.c:2078: error: implicit declaration of function 'pm_prepare_console'
> > drivers/macintosh/via-pmu.c: In function 'pmac_wakeup_devices':
> > drivers/macintosh/via-pmu.c:2194: error: implicit declaration of function 'pm_restore_console'
> > make[2]: *** [drivers/macintosh/via-pmu.o] Error 1

Ben, does via-pmu still need the pm_prepare_console call? If yes, the
declaration has to be moved from kernel/power/power.h to
include/linux/suspend.h

--
short story of a lazy sysadmin:
alias appserv=wotan

2006-02-06 14:23:18

by Olaf Hering

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

On Mon, Feb 06, Olaf Hering wrote:

> On Sun, Feb 05, Dave Jones wrote:
>
> > On Sun, Feb 05, 2006 at 01:56:10PM +0100, Olaf Hering wrote:
> > > On Fri, Feb 03, Linux Kernel Mailing List wrote:
> > >
> > > > tree 8f70444139c8564c0f1e88e1f33adda036ae6a96
> > > > parent 278ff9537030bbb292b33504f5e1f6e0126793eb
> > > > author Dave Jones <[email protected]> Fri, 03 Feb 2006 19:03:44 -0800
> > > > committer Linus Torvalds <[email protected]> Sat, 04 Feb 2006 00:32:00 -0800
> > > >
> > > > [PATCH] Fix build failure in recent pm_prepare_* changes.
> > > >
> > > > kernel/power/power.h:49: error: static declaration of 'pm_prepare_console' follows non-static declaration
> > > > include/linux/suspend.h:46: error: previous declaration of 'pm_prepare_console' was here
> > > > kernel/power/power.h:50: error: static declaration of 'pm_restore_console' follows non-static declaration
> > > > include/linux/suspend.h:47: error: previous declaration of 'pm_restore_console' was here
> > > >
> > > > Signed-off-by: Dave Jones <[email protected]>
> > >
> > > this one is not correct, please have a closer look at
> > > f7b8988ff50d99c99746f65f420364e91362c065
> > >
> > > CC drivers/macintosh/via-pmu.o
> > > drivers/macintosh/via-pmu.c: In function 'pmac_suspend_devices':
> > > drivers/macintosh/via-pmu.c:2078: error: implicit declaration of function 'pm_prepare_console'
> > > drivers/macintosh/via-pmu.c: In function 'pmac_wakeup_devices':
> > > drivers/macintosh/via-pmu.c:2194: error: implicit declaration of function 'pm_restore_console'
> > > make[2]: *** [drivers/macintosh/via-pmu.o] Error 1


via-pmu needs to call console functions directly from the pmu_ioctl.

drivers/macintosh/via-pmu.c | 4 ++++
1 files changed, 4 insertions(+)

Index: linux-2.6.16-rc2-olh/drivers/macintosh/via-pmu.c
===================================================================
--- linux-2.6.16-rc2-olh.orig/drivers/macintosh/via-pmu.c
+++ linux-2.6.16-rc2-olh/drivers/macintosh/via-pmu.c
@@ -2070,6 +2070,10 @@ restore_via_state(void)
out_8(&via[IER], IER_SET | SR_INT | CB1_INT);
}

+/* to avoid include mess */
+extern int pm_prepare_console(void);
+extern void pm_restore_console(void);
+
static int
pmac_suspend_devices(void)
{

--
short story of a lazy sysadmin:
alias appserv=wotan

2006-02-06 15:02:14

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

Hi,

On Monday 06 February 2006 15:23, Olaf Hering wrote:
> On Mon, Feb 06, Olaf Hering wrote:
>
> > On Sun, Feb 05, Dave Jones wrote:
> >
> > > On Sun, Feb 05, 2006 at 01:56:10PM +0100, Olaf Hering wrote:
> > > > On Fri, Feb 03, Linux Kernel Mailing List wrote:
> > > >
> > > > > tree 8f70444139c8564c0f1e88e1f33adda036ae6a96
> > > > > parent 278ff9537030bbb292b33504f5e1f6e0126793eb
> > > > > author Dave Jones <[email protected]> Fri, 03 Feb 2006 19:03:44 -0800
> > > > > committer Linus Torvalds <[email protected]> Sat, 04 Feb 2006 00:32:00 -0800
> > > > >
> > > > > [PATCH] Fix build failure in recent pm_prepare_* changes.
> > > > >
> > > > > kernel/power/power.h:49: error: static declaration of 'pm_prepare_console' follows non-static declaration
> > > > > include/linux/suspend.h:46: error: previous declaration of 'pm_prepare_console' was here
> > > > > kernel/power/power.h:50: error: static declaration of 'pm_restore_console' follows non-static declaration
> > > > > include/linux/suspend.h:47: error: previous declaration of 'pm_restore_console' was here

Sorry, my recent change has broken it, but pm_prepare_console() and
pm_restore_console() are only static if CONFIG_VT or CONFIG_VT_CONSOLE
is not set which Ben told me should not happen on Macs.

> > > > >
> > > > > Signed-off-by: Dave Jones <[email protected]>
> > > >
> > > > this one is not correct, please have a closer look at
> > > > f7b8988ff50d99c99746f65f420364e91362c065
> > > >
> > > > CC drivers/macintosh/via-pmu.o
> > > > drivers/macintosh/via-pmu.c: In function 'pmac_suspend_devices':
> > > > drivers/macintosh/via-pmu.c:2078: error: implicit declaration of function 'pm_prepare_console'
> > > > drivers/macintosh/via-pmu.c: In function 'pmac_wakeup_devices':
> > > > drivers/macintosh/via-pmu.c:2194: error: implicit declaration of function 'pm_restore_console'
> > > > make[2]: *** [drivers/macintosh/via-pmu.o] Error 1

>
>
> via-pmu needs to call console functions directly from the pmu_ioctl.
>
> drivers/macintosh/via-pmu.c | 4 ++++
> 1 files changed, 4 insertions(+)
>
> Index: linux-2.6.16-rc2-olh/drivers/macintosh/via-pmu.c
> ===================================================================
> --- linux-2.6.16-rc2-olh.orig/drivers/macintosh/via-pmu.c
> +++ linux-2.6.16-rc2-olh/drivers/macintosh/via-pmu.c
> @@ -2070,6 +2070,10 @@ restore_via_state(void)
> out_8(&via[IER], IER_SET | SR_INT | CB1_INT);
> }
>
> +/* to avoid include mess */
> +extern int pm_prepare_console(void);
> +extern void pm_restore_console(void);

if CONFIG_VT or CONFIG_VT_CONSOLE is not set, these functions are not defined.

> +
> static int
> pmac_suspend_devices(void)
> {
>

I think the complete fix should look like that:

--- linux-2.6.16-rc1-mm5.orig/drivers/macintosh/via-pmu.c
+++ linux-2.6.16-rc1-mm5/drivers/macintosh/via-pmu.c
@@ -2070,6 +2070,14 @@ restore_via_state(void)
out_8(&via[IER], IER_SET | SR_INT | CB1_INT);
}

+#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
+extern int pm_prepare_console(void);
+extern void pm_restore_console(void);
+#else
+static int pm_prepare_console(void) { return 0; }
+static void pm_restore_console(void) {}
+#endif
+
static int
pmac_suspend_devices(void)
{


Greetings,
Rafael

2006-02-06 19:39:15

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

"Rafael J. Wysocki" <[email protected]> wrote:
>
> Sorry, my recent change has broken it, but pm_prepare_console() and
> pm_restore_console() are only static if CONFIG_VT or CONFIG_VT_CONSOLE
> is not set which Ben told me should not happen on Macs.

But kernel/power/power.h has

#ifdef SUSPEND_CONSOLE
extern int pm_prepare_console(void);
extern void pm_restore_console(void);
#else
static int pm_prepare_console(void) { return 0; }
static void pm_restore_console(void) {}
#endif

> --- linux-2.6.16-rc1-mm5.orig/drivers/macintosh/via-pmu.c
> +++ linux-2.6.16-rc1-mm5/drivers/macintosh/via-pmu.c
> @@ -2070,6 +2070,14 @@ restore_via_state(void)
> out_8(&via[IER], IER_SET | SR_INT | CB1_INT);
> }
>
> +#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
> +extern int pm_prepare_console(void);
> +extern void pm_restore_console(void);
> +#else
> +static int pm_prepare_console(void) { return 0; }
> +static void pm_restore_console(void) {}
> +#endif
> +

These should be in a header file. Presumably one which
kernel/power/power.h includes, too.

2006-02-06 23:55:57

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

On Monday 06 February 2006 20:38, Andrew Morton wrote:
> "Rafael J. Wysocki" <[email protected]> wrote:
> >
> > Sorry, my recent change has broken it, but pm_prepare_console() and
> > pm_restore_console() are only static if CONFIG_VT or CONFIG_VT_CONSOLE
> > is not set which Ben told me should not happen on Macs.
>
> But kernel/power/power.h has
>
> #ifdef SUSPEND_CONSOLE

There is

#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
#define SUSPEND_CONSOLE (MAX_NR_CONSOLES-1)
#endif

earlier in there.

> extern int pm_prepare_console(void);
> extern void pm_restore_console(void);
> #else
> static int pm_prepare_console(void) { return 0; }
> static void pm_restore_console(void) {}
> #endif
>
> > --- linux-2.6.16-rc1-mm5.orig/drivers/macintosh/via-pmu.c
> > +++ linux-2.6.16-rc1-mm5/drivers/macintosh/via-pmu.c
> > @@ -2070,6 +2070,14 @@ restore_via_state(void)
> > out_8(&via[IER], IER_SET | SR_INT | CB1_INT);
> > }
> >
> > +#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
> > +extern int pm_prepare_console(void);
> > +extern void pm_restore_console(void);
> > +#else
> > +static int pm_prepare_console(void) { return 0; }
> > +static void pm_restore_console(void) {}
> > +#endif
> > +
>
> These should be in a header file. Presumably one which
> kernel/power/power.h includes, too.

Then I think I should move all that to include/linux/suspend.h.

Greetings,
Rafael

2006-02-07 00:42:31

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

"Rafael J. Wysocki" <[email protected]> wrote:
>
> > > --- linux-2.6.16-rc1-mm5.orig/drivers/macintosh/via-pmu.c
> > > +++ linux-2.6.16-rc1-mm5/drivers/macintosh/via-pmu.c
> > > @@ -2070,6 +2070,14 @@ restore_via_state(void)
> > > out_8(&via[IER], IER_SET | SR_INT | CB1_INT);
> > > }
> > >
> > > +#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
> > > +extern int pm_prepare_console(void);
> > > +extern void pm_restore_console(void);
> > > +#else
> > > +static int pm_prepare_console(void) { return 0; }
> > > +static void pm_restore_console(void) {}
> > > +#endif
> > > +
> >
> > These should be in a header file. Presumably one which
> > kernel/power/power.h includes, too.
>
> Then I think I should move all that to include/linux/suspend.h.

Sounds sane. Or <linux/console.h>.

2006-02-07 14:50:16

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] Fix build failure in recent pm_prepare_* changes.

On Tuesday 07 February 2006 01:44, Andrew Morton wrote:
> "Rafael J. Wysocki" <[email protected]> wrote:
> >
> > > > --- linux-2.6.16-rc1-mm5.orig/drivers/macintosh/via-pmu.c
> > > > +++ linux-2.6.16-rc1-mm5/drivers/macintosh/via-pmu.c
> > > > @@ -2070,6 +2070,14 @@ restore_via_state(void)
> > > > out_8(&via[IER], IER_SET | SR_INT | CB1_INT);
> > > > }
> > > >
> > > > +#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
> > > > +extern int pm_prepare_console(void);
> > > > +extern void pm_restore_console(void);
> > > > +#else
> > > > +static int pm_prepare_console(void) { return 0; }
> > > > +static void pm_restore_console(void) {}
> > > > +#endif
> > > > +
> > >
> > > These should be in a header file. Presumably one which
> > > kernel/power/power.h includes, too.
> >
> > Then I think I should move all that to include/linux/suspend.h.
>
> Sounds sane. Or <linux/console.h>.

I chose include/linux/suspend.h (it was there before and the functions are
defined in kernel/power/console.c).

The appended patch (against -mm5) has been compile-tested on x86-64 and i386
with and withoud CONFIG_VT, CONFIG_VT_CONSOLE. [I have no access to a Mac,
though.]

Greetings,
Rafael


Fix compilation problem in PM headers.

Signed-off-by: Rafael J. Wysocki <[email protected]>

include/linux/suspend.h | 10 +++++++++-
kernel/power/console.c | 4 +++-
kernel/power/power.h | 16 ----------------
3 files changed, 12 insertions(+), 18 deletions(-)

Index: linux-2.6.16-rc1-mm5/include/linux/suspend.h
===================================================================
--- linux-2.6.16-rc1-mm5.orig/include/linux/suspend.h
+++ linux-2.6.16-rc1-mm5/include/linux/suspend.h
@@ -42,13 +42,21 @@ extern void mark_free_pages(struct zone
#ifdef CONFIG_PM
/* kernel/power/swsusp.c */
extern int software_suspend(void);
+
+#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
+extern int pm_prepare_console(void);
+extern void pm_restore_console(void);
+#else
+static inline int pm_prepare_console(void) { return 0; }
+static inline void pm_restore_console(void) {}
+#endif /* defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE) */
#else
static inline int software_suspend(void)
{
printk("Warning: fake suspend called\n");
return -EPERM;
}
-#endif
+#endif /* CONFIG_PM */

#ifdef CONFIG_SUSPEND_SMP
extern void disable_nonboot_cpus(void);
Index: linux-2.6.16-rc1-mm5/kernel/power/power.h
===================================================================
--- linux-2.6.16-rc1-mm5.orig/kernel/power/power.h
+++ linux-2.6.16-rc1-mm5/kernel/power/power.h
@@ -1,14 +1,6 @@
#include <linux/suspend.h>
#include <linux/utsname.h>

-/* With SUSPEND_CONSOLE defined suspend looks *really* cool, but
- we probably do not take enough locks for switching consoles, etc,
- so bad things might happen.
-*/
-#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
-#define SUSPEND_CONSOLE (MAX_NR_CONSOLES-1)
-#endif
-
struct swsusp_info {
struct new_utsname uts;
u32 version_code;
@@ -43,14 +35,6 @@ static struct subsys_attribute _name##_a

extern struct subsystem power_subsys;

-#ifdef SUSPEND_CONSOLE
-extern int pm_prepare_console(void);
-extern void pm_restore_console(void);
-#else
-static int pm_prepare_console(void) { return 0; }
-static void pm_restore_console(void) {}
-#endif
-
/* References to section boundaries */
extern const void __nosave_begin, __nosave_end;

Index: linux-2.6.16-rc1-mm5/kernel/power/console.c
===================================================================
--- linux-2.6.16-rc1-mm5.orig/kernel/power/console.c
+++ linux-2.6.16-rc1-mm5/kernel/power/console.c
@@ -9,7 +9,9 @@
#include <linux/console.h>
#include "power.h"

-#ifdef SUSPEND_CONSOLE
+#if defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
+#define SUSPEND_CONSOLE (MAX_NR_CONSOLES-1)
+
static int orig_fgconsole, orig_kmsg;

int pm_prepare_console(void)