vt_ioctl.c: fix compile error with pm_set_vt_switch()

PATCH In review H Hartley Sweeten 3 participants 6 messages Sep 2, 2011 – Sep 6, 2011

Subsystems (3)

TTY layer and serial drivers Suspend to RAM Hibernation

lore

Share & tools

lore

H Hartley Sweeten

author

2011-09-02 21:03 UTC #0 lore raw

PATCH 1/1 vt_ioctl.c: fix compile error with pm_set_vt_switch()

When CONFIG_PM_SLEEP, CONFIG_VT, and CONFIG_VT_CONSOLE are not enabled the function pm_set_vt_switch() is defined as an inline in linux/suspend.h. This causes a compile error if CONFIG_VT is enabled.

drivers/tty/vt/vt_ioctl.c:1794: error: redefinition of 'pm_set_vt_switch' include/linux/suspend.h:17: error: previous definition of 'pm_set_vt_switch' was here

Fix this with an #if defined guard around the function in vt_ioctl.c.

Signed-off-by H Hartley Sweeten <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Arthur Taylor <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Jiri Slaby <[email protected]>

---

drivers/tty/vt/vt_ioctl.c +4 -0

index 5e096f4..7bb1b4d 100644
@@ -1784,6 +1784,8 @@ int vt_move_to_console(unsigned int vt, int alloc)
 	return prev;
 }
 
+#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_VT_CONSOLE)
+
 /*
  * Normally during a suspend, we allocate a new console and switch to it.
  * When we resume, we switch back to the original console.  This switch

@@ -1798,3 +1800,5 @@ void pm_set_vt_switch(int do_switch)
 	console_unlock();
 }
 EXPORT_SYMBOL(pm_set_vt_switch);
+
+#endif

Alan Cox

2011-09-03 10:47 UTC #1 lore raw

Quoted 8 lines · On Fri, 2 Sep 2011 14:02:42 -0700 H Hartley Sweeten…

When CONFIG_PM_SLEEP, CONFIG_VT, and CONFIG_VT_CONSOLE are not enabled the function pm_set_vt_switch() is defined as an inline in linux/suspend.h. This causes a compile error if CONFIG_VT is enabled.
drivers/tty/vt/vt_ioctl.c:1794: error: redefinition of 'pm_set_vt_switch' include/linux/suspend.h:17: error: previous definition of 'pm_set_vt_switch' was here
Fix this with an #if defined guard around the function in vt_ioctl.c.

Can you put the #if uglies into the buggy suspend.h header instead so the code proper is kept clean of ifdefs.

Alan

Arnd Bergmann

2011-09-05 14:55 UTC #2 lore raw

Quoted 7 lines · Alan Cox

Quoted 4 lines · On Fri, 2 Sep 2011 14:02:42 -0700 H Hartley Sweeten…
When CONFIG_PM_SLEEP, CONFIG_VT, and CONFIG_VT_CONSOLE are not enabled the function pm_set_vt_switch() is defined as an inline in linux/suspend.h. This causes a compile error if CONFIG_VT is enabled.

Quoted 7 lines

Quoted 4 lines
drivers/tty/vt/vt_ioctl.c:1794: error: redefinition of 'pm_set_vt_switch' include/linux/suspend.h:17: error: previous definition of 'pm_set_vt_switch' was here
Fix this with an #if defined guard around the function in vt_ioctl.c.
Can you put the #if uglies into the buggy suspend.h header instead so the code proper is kept clean of ifdefs.

I had discovered the same problem earlier but did not get to submit a fix yet. I think the patch below should do it. Hartley, can you confirm that?

Arnd
---
vt/suspend: pm_set_vt_switch does not depend on CONFIG_PM_SLEEP

This avoid having multiple conflicting definitions of pm_set_vt_switch when CONFIG_VT is set but CONFIG_PM_SLEEP or CONFIG_VT_CONSOLE are not.

drivers/tty/vt/vt_ioctl.c:1794: error: redefinition of 'pm_set_vt_switch' include/linux/suspend.h:17: error: previous definition of 'pm_set_vt_switch' was here

Signed-off-by Arnd Bergmann <[email protected]>

---

include/linux/suspend.h +6 -3

index 6bbcef2..69f3b39 100644
@@ -8,15 +8,18 @@
 #include <linux/mm.h>
 #include <asm/errno.h>
 
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
+#if defined(CONFIG_VT)
 extern void pm_set_vt_switch(int);
-extern int pm_prepare_console(void);
-extern void pm_restore_console(void);
 #else
 static inline void pm_set_vt_switch(int do_switch)
 {
 }
+#endif
 
+#if defined(CONFIG_PM_SLEEP) && 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;

H Hartley Sweeten

author Tested-by

2011-09-06 18:26 UTC #3 lore raw

Quoted 11 lines · Arnd Bergmann

Quoted 10 lines · Alan Cox
Quoted 7 lines · On Fri, 2 Sep 2011 14:02:42 -0700 H Hartley Sweeten…
When CONFIG_PM_SLEEP, CONFIG_VT, and CONFIG_VT_CONSOLE are not enabled the function pm_set_vt_switch() is defined as an inline in linux/suspend.h. This causes a compile error if CONFIG_VT is enabled.
drivers/tty/vt/vt_ioctl.c:1794: error: redefinition of 'pm_set_vt_switch' include/linux/suspend.h:17: error: previous definition of 'pm_set_vt_switch' was here

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Fix this with an #if defined guard around the function in vt_ioctl.c.
Can you put the #if uglies into the buggy suspend.h header instead so the code proper is kept clean of ifdefs.
I had discovered the same problem earlier but did not get to submit a fix yet. I think the patch below should do it. Hartley, can you confirm that?

Arnd,

Your patch below does appear to fix the problem.

Tested-by H Hartley Sweeten <[email protected]>

There is still a bit of #if ugliness with the pm_{prepare,restore}_console.

The source file kernel/power/console.c is compiled when CONFIG_PM_SLEEP is enabled. But that file ends up empty if CONFIG_VT and CONFIG_VT_CONSOLE are not defined.

I wonder if it would be cleaner to make a new entry in kernel/power/Kconfig:

config PM_SLEEP_CONSOLE

	def_bool y
	depends on PM_SLEEP && VT && VT_CONSOLE

Then use that config option in the Makefile and in suspend.h.

Regards,
Hartley

Arnd
---
vt/suspend: pm_set_vt_switch does not depend on CONFIG_PM_SLEEP

This avoid having multiple conflicting definitions of pm_set_vt_switch when CONFIG_VT is set but CONFIG_PM_SLEEP or CONFIG_VT_CONSOLE are not.

drivers/tty/vt/vt_ioctl.c:1794: error: redefinition of 'pm_set_vt_switch' include/linux/suspend.h:17: error: previous definition of 'pm_set_vt_switch' was here

Signed-off-by Arnd Bergmann <[email protected]>

---

include/linux/suspend.h +6 -3

index 6bbcef2..69f3b39 100644
@@ -8,15 +8,18 @@
 #include <linux/mm.h>
 #include <asm/errno.h>
 
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
+#if defined(CONFIG_VT)
 extern void pm_set_vt_switch(int);
-extern int pm_prepare_console(void);
-extern void pm_restore_console(void);
 #else
 static inline void pm_set_vt_switch(int do_switch)
 {
 }
+#endif
 
+#if defined(CONFIG_PM_SLEEP) && 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;

Arnd Bergmann

2011-09-06 19:30 UTC #4 lore raw

Quoted 11 lines · On Tuesday 06 September 2011 13:26:12 H Hartley Sweeten wrote:

Your patch below does appear to fix the problem.
Tested-by H Hartley Sweeten <[email protected]>
There is still a bit of #if ugliness with the pm_{prepare,restore}_console.
The source file kernel/power/console.c is compiled when CONFIG_PM_SLEEP is enabled. But that file ends up empty if CONFIG_VT and CONFIG_VT_CONSOLE are not defined.

Quoted 7 lines

I wonder if it would be cleaner to make a new entry in kernel/power/Kconfig:
config PM_SLEEP_CONSOLE
        def_bool y
        depends on PM_SLEEP && VT && VT_CONSOLE
Then use that config option in the Makefile and in suspend.h.

Yes, that would be cleaner. I would probably move it to drivers/tty/vt in the same step and name the option VT_CONSOLE_SLEEP.

Do you want to send a patch?

Thanks for testing!

Arnd

H Hartley Sweeten

author

2011-09-06 20:25 UTC #5 lore raw

Quoted 14 lines · Arnd Bergmann

Quoted 13 lines · On Tuesday 06 September 2011 13:26:12 H Hartley Sweeten wrote:
Your patch below does appear to fix the problem.
Tested-by H Hartley Sweeten <[email protected]>
There is still a bit of #if ugliness with the pm_{prepare,restore}_console.
The source file kernel/power/console.c is compiled when CONFIG_PM_SLEEP is enabled. But that file ends up empty if CONFIG_VT and CONFIG_VT_CONSOLE are not defined.
I wonder if it would be cleaner to make a new entry in kernel/power/Kconfig:

Quoted 8 lines

Quoted 5 lines
config PM_SLEEP_CONSOLE
        def_bool y
        depends on PM_SLEEP && VT && VT_CONSOLE
Then use that config option in the Makefile and in suspend.h.
Yes, that would be cleaner. I would probably move it to drivers/tty/vt in the same step and name the option VT_CONSOLE_SLEEP.

Ok.

Quoted 1 line

Do you want to send a patch?

Will do in just a minute.

Quoted 1 line

Thanks for testing!

Not a problem!

Regards,
Hartley