linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] console: Don't prefer first registered if DT specifies stdout-path
@ 2016-08-09 12:50 Paul Burton
  2016-08-09 14:12 ` kbuild test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Paul Burton @ 2016-08-09 12:50 UTC (permalink / raw)
  To: linux-mips
  Cc: Ralf Baechle, Paul Burton, Tejun Heo, Sergey Senozhatsky,
	Tony Luck, Jiri Slaby, Daniel Vetter, Russell King, Emil Velikov,
	Ivan Delalande, Thierry Reding, Petr Mladek, linux-kernel,
	Geliang Tang, Dave Young, Greg Kroah-Hartman, devicetree,
	Rob Herring, Frank Rowand, Mathias Krause, Andrew Morton

If a device tree specifies a preferred device for kernel console output
via the stdout-path or linux,stdout-path chosen node properties or the
stdout alias then the kernel ought to honor it & output the kernel
console to that device. As it stands, this isn't the case. Whilst we
parse the stdout-path properties & set an of_stdout variable from
of_alias_scan(), and use that from of_console_check() to determine
whether to add a console device as a preferred console whilst
registering it, we also prefer the first registered console if no other
has been selected at the time of its registration.

This means that if a console other than the one the device tree selects
via stdout-path is registered first, we will switch to using it & when
the stdout-path console is later registered the call to
add_preferred_console() via of_console_check() is too late to do
anything useful. In practice this seems to mean that we switch to the
dummy console device fairly early & see no further console output:

    Console: colour dummy device 80x25
    console [tty0] enabled
    bootconsole [ns16550a0] disabled

Fix this by not automatically preferring the first registered console if
one is specified by the device tree. This allows consoles to be
registered but not enabled, and once the driver for the console selected
by stdout-path calls of_console_check() the driver will be added to the
list of preferred consoles before any other console has been enabled.
When that console is then registered via register_console() it will be
enabled as expected.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>

---

 drivers/of/base.c       |  2 ++
 include/linux/console.h |  6 ++++++
 kernel/printk/printk.c  | 13 ++++++++++++-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index ebf84e3..2ea6877 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1974,6 +1974,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
 			name = of_get_property(of_aliases, "stdout", NULL);
 		if (name)
 			of_stdout = of_find_node_opts_by_path(name, &of_stdout_options);
+		if (of_stdout)
+			console_set_by_of();
 	}
 
 	if (!of_aliases)
diff --git a/include/linux/console.h b/include/linux/console.h
index 98c8615..71cc04f 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -166,6 +166,12 @@ static inline void console_sysfs_notify(void)
 #endif
 extern bool console_suspend_enabled;
 
+#ifdef CONFIG_OF
+extern void console_set_by_of(void);
+#else
+static inline void console_set_by_of(void);
+#endif
+
 /* Suspend and resume console messages over PM events */
 extern void suspend_console(void);
 extern void resume_console(void);
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 60cdf63..f1f03ca 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -149,6 +149,17 @@ static int preferred_console = -1;
 int console_set_on_cmdline;
 EXPORT_SYMBOL(console_set_on_cmdline);
 
+#ifdef CONFIG_OF
+static bool of_specified_console;
+
+void console_set_by_of(void)
+{
+	of_specified_console = true;
+}
+#else
+# define of_specified_console false
+#endif
+
 /* Flag: console code may call schedule() */
 static int console_may_schedule;
 
@@ -2509,7 +2520,7 @@ void register_console(struct console *newcon)
 	 *	didn't select a console we take the first one
 	 *	that registers here.
 	 */
-	if (preferred_console < 0) {
+	if (preferred_console < 0 && !of_specified_console) {
 		if (newcon->index < 0)
 			newcon->index = 0;
 		if (newcon->setup == NULL ||
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH] console: Don't prefer first registered if DT specifies stdout-path
  2016-08-09 12:50 [PATCH] console: Don't prefer first registered if DT specifies stdout-path Paul Burton
@ 2016-08-09 14:12 ` kbuild test robot
  2016-08-09 15:10 ` kbuild test robot
  2016-08-09 15:19 ` [PATCH v2] " Paul Burton
  2 siblings, 0 replies; 32+ messages in thread
From: kbuild test robot @ 2016-08-09 14:12 UTC (permalink / raw)
  To: Paul Burton
  Cc: kbuild-all, linux-mips, Ralf Baechle, Paul Burton, Tejun Heo,
	Sergey Senozhatsky, Tony Luck, Jiri Slaby, Daniel Vetter,
	Russell King, Emil Velikov, Ivan Delalande, Thierry Reding,
	Petr Mladek, linux-kernel, Geliang Tang, Dave Young,
	Greg Kroah-Hartman, devicetree, Rob Herring, Frank Rowand,
	Mathias Krause, Andrew Morton

[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]

Hi Paul,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc1 next-20160809]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Burton/console-Don-t-prefer-first-registered-if-DT-specifies-stdout-path/20160809-205816
config: sh-ecovec24_defconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sh 

All errors (new ones prefixed by >>):

   In file included from arch/sh/kernel/setup.c:14:0:
>> include/linux/console.h:179:20: error: 'console_set_by_of' declared 'static' but never defined [-Werror=unused-function]
    static inline void console_set_by_of(void);
                       ^
   cc1: all warnings being treated as errors

vim +179 include/linux/console.h

   173	#endif
   174	extern bool console_suspend_enabled;
   175	
   176	#ifdef CONFIG_OF
   177	extern void console_set_by_of(void);
   178	#else
 > 179	static inline void console_set_by_of(void);
   180	#endif
   181	
   182	/* Suspend and resume console messages over PM events */

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 15889 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] console: Don't prefer first registered if DT specifies stdout-path
  2016-08-09 12:50 [PATCH] console: Don't prefer first registered if DT specifies stdout-path Paul Burton
  2016-08-09 14:12 ` kbuild test robot
@ 2016-08-09 15:10 ` kbuild test robot
  2016-08-09 15:19 ` [PATCH v2] " Paul Burton
  2 siblings, 0 replies; 32+ messages in thread
From: kbuild test robot @ 2016-08-09 15:10 UTC (permalink / raw)
  To: Paul Burton
  Cc: kbuild-all, linux-mips, Ralf Baechle, Paul Burton, Tejun Heo,
	Sergey Senozhatsky, Tony Luck, Jiri Slaby, Daniel Vetter,
	Russell King, Emil Velikov, Ivan Delalande, Thierry Reding,
	Petr Mladek, linux-kernel, Geliang Tang, Dave Young,
	Greg Kroah-Hartman, devicetree, Rob Herring, Frank Rowand,
	Mathias Krause, Andrew Morton

[-- Attachment #1: Type: text/plain, Size: 6766 bytes --]

Hi Paul,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.8-rc1 next-20160809]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Burton/console-Don-t-prefer-first-registered-if-DT-specifies-stdout-path/20160809-205816
config: x86_64-randconfig-s2-08092115 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

>> include/linux/console.h:179: warning: 'console_set_by_of' declared 'static' but never defined
--
>> include/linux/console.h:179: warning: 'console_set_by_of' declared 'static' but never defined
   kernel/printk/printk.c: In function 'devkmsg_sysctl_set_loglvl':
   kernel/printk/printk.c:158: warning: 'old' may be used uninitialized in this function
--
>> include/linux/console.h:179: warning: 'console_set_by_of' declared 'static' but never defined
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: In function 'amdgpu_debugfs_regs_read':
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2182: warning: 'instance_bank' may be used uninitialized in this function
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2182: warning: 'sh_bank' may be used uninitialized in this function
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2182: warning: 'se_bank' may be used uninitialized in this function
--
>> include/linux/console.h:179: warning: 'console_set_by_of' declared 'static' but never defined
   drivers/gpu/drm/bochs/bochs_mm.c: In function 'bochs_gem_create':
   drivers/gpu/drm/bochs/bochs_mm.c:384: warning: 'bochsbo' may be used uninitialized in this function
--
>> include/linux/console.h:179: warning: 'console_set_by_of' declared 'static' but never defined
   drivers/tty/serial/serial_core.c: In function 'uart_start':
   drivers/tty/serial/serial_core.c:142: warning: 'flags' may be used uninitialized in this function
   drivers/tty/serial/serial_core.c: In function 'uart_stop':
   drivers/tty/serial/serial_core.c:121: warning: 'flags' may be used uninitialized in this function
   drivers/tty/serial/serial_core.c: In function 'uart_chars_in_buffer':
   drivers/tty/serial/serial_core.c:611: warning: 'flags' may be used uninitialized in this function
   drivers/tty/serial/serial_core.c: In function 'uart_write_room':
   drivers/tty/serial/serial_core.c:598: warning: 'flags' may be used uninitialized in this function
   drivers/tty/serial/serial_core.c: In function 'uart_put_char':
   drivers/tty/serial/serial_core.c:531: warning: 'flags' may be used uninitialized in this function
   drivers/tty/serial/serial_core.c: In function 'uart_flush_buffer':
   drivers/tty/serial/serial_core.c:624: warning: 'flags' may be used uninitialized in this function
   drivers/tty/serial/serial_core.c: In function 'uart_write':
   drivers/tty/serial/serial_core.c:559: warning: 'flags' may be used uninitialized in this function
--
>> include/linux/console.h:179: warning: 'console_set_by_of' declared 'static' but never defined
   drivers/tty/vt/vt_ioctl.c: In function 'vt_ioctl':
   drivers/tty/vt/vt_ioctl.c:825: warning: 'cc' may be used uninitialized in this function
--
>> include/linux/console.h:179: warning: 'console_set_by_of' declared 'static' but never defined
   drivers/tty/vt/consolemap.c: In function 'conv_uni_to_pc':
   drivers/tty/vt/consolemap.c:801: warning: 'h' may be used uninitialized in this function
--
>> include/linux/console.h:179: warning: 'console_set_by_of' declared 'static' but never defined
   drivers/tty/serial/8250/8250_port.c: In function 'serial8250_console_write':
   drivers/tty/serial/8250/8250_port.c:3110: warning: 'flags' may be used uninitialized in this function
--
>> include/linux/console.h:179: warning: 'console_set_by_of' declared 'static' but never defined
   drivers/video/fbdev/aty/aty128fb.c: In function 'aty128_decode_var':
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.bpp' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.depth' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.yoffset' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.xoffset' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.vyres' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.vxres' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.offset_cntl' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.pitch' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.v_sync_strt_wid' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.h_sync_strt_wid' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.v_total' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.h_total' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1567: warning: 'crtc.gen_cntl' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1569: warning: 'fifo_reg.dda_on_off' may be used uninitialized in this function
   drivers/video/fbdev/aty/aty128fb.c:1569: warning: 'fifo_reg.dda_config' may be used uninitialized in this function

vim +179 include/linux/console.h

   163	extern void console_start(struct console *);
   164	extern int is_console_locked(void);
   165	extern int braille_register_console(struct console *, int index,
   166			char *console_options, char *braille_options);
   167	extern int braille_unregister_console(struct console *);
   168	#ifdef CONFIG_TTY
   169	extern void console_sysfs_notify(void);
   170	#else
   171	static inline void console_sysfs_notify(void)
   172	{ }
   173	#endif
   174	extern bool console_suspend_enabled;
   175	
   176	#ifdef CONFIG_OF
   177	extern void console_set_by_of(void);
   178	#else
 > 179	static inline void console_set_by_of(void);
   180	#endif
   181	
   182	/* Suspend and resume console messages over PM events */
   183	extern void suspend_console(void);
   184	extern void resume_console(void);
   185	
   186	int mda_console_init(void);
   187	void prom_con_init(void);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23471 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path
  2016-08-09 12:50 [PATCH] console: Don't prefer first registered if DT specifies stdout-path Paul Burton
  2016-08-09 14:12 ` kbuild test robot
  2016-08-09 15:10 ` kbuild test robot
@ 2016-08-09 15:19 ` Paul Burton
  2016-08-09 21:57   ` Andrew Morton
  2016-10-16 18:07   ` Andreas Schwab
  2 siblings, 2 replies; 32+ messages in thread
From: Paul Burton @ 2016-08-09 15:19 UTC (permalink / raw)
  To: linux-mips
  Cc: Ralf Baechle, Paul Burton, Tejun Heo, Sergey Senozhatsky,
	Jiri Slaby, Daniel Vetter, Ivan Delalande, Thierry Reding,
	Borislav Petkov, Jan Kara, Petr Mladek, linux-kernel,
	Joe Perches, Greg Kroah-Hartman, devicetree, Rob Herring,
	Andrew Morton, Frank Rowand

If a device tree specifies a preferred device for kernel console output
via the stdout-path or linux,stdout-path chosen node properties or the
stdout alias then the kernel ought to honor it & output the kernel
console to that device. As it stands, this isn't the case. Whilst we
parse the stdout-path properties & set an of_stdout variable from
of_alias_scan(), and use that from of_console_check() to determine
whether to add a console device as a preferred console whilst
registering it, we also prefer the first registered console if no other
has been selected at the time of its registration.

This means that if a console other than the one the device tree selects
via stdout-path is registered first, we will switch to using it & when
the stdout-path console is later registered the call to
add_preferred_console() via of_console_check() is too late to do
anything useful. In practice this seems to mean that we switch to the
dummy console device fairly early & see no further console output:

    Console: colour dummy device 80x25
    console [tty0] enabled
    bootconsole [ns16550a0] disabled

Fix this by not automatically preferring the first registered console if
one is specified by the device tree. This allows consoles to be
registered but not enabled, and once the driver for the console selected
by stdout-path calls of_console_check() the driver will be added to the
list of preferred consoles before any other console has been enabled.
When that console is then registered via register_console() it will be
enabled as expected.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>

---

Changes in v2:
- Declare, not just define, static dummy function.

 drivers/of/base.c       |  2 ++
 include/linux/console.h |  6 ++++++
 kernel/printk/printk.c  | 13 ++++++++++++-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 7792266..a930ec9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2011,6 +2011,8 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
 			name = of_get_property(of_aliases, "stdout", NULL);
 		if (name)
 			of_stdout = of_find_node_opts_by_path(name, &of_stdout_options);
+		if (of_stdout)
+			console_set_by_of();
 	}
 
 	if (!of_aliases)
diff --git a/include/linux/console.h b/include/linux/console.h
index d530c46..3672809 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -173,6 +173,12 @@ static inline void console_sysfs_notify(void)
 #endif
 extern bool console_suspend_enabled;
 
+#ifdef CONFIG_OF
+extern void console_set_by_of(void);
+#else
+static inline void console_set_by_of(void) {}
+#endif
+
 /* Suspend and resume console messages over PM events */
 extern void suspend_console(void);
 extern void resume_console(void);
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a5ef95c..2b4947c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -253,6 +253,17 @@ static int preferred_console = -1;
 int console_set_on_cmdline;
 EXPORT_SYMBOL(console_set_on_cmdline);
 
+#ifdef CONFIG_OF
+static bool of_specified_console;
+
+void console_set_by_of(void)
+{
+	of_specified_console = true;
+}
+#else
+# define of_specified_console false
+#endif
+
 /* Flag: console code may call schedule() */
 static int console_may_schedule;
 
@@ -2668,7 +2679,7 @@ void register_console(struct console *newcon)
 	 *	didn't select a console we take the first one
 	 *	that registers here.
 	 */
-	if (preferred_console < 0) {
+	if (preferred_console < 0 && !of_specified_console) {
 		if (newcon->index < 0)
 			newcon->index = 0;
 		if (newcon->setup == NULL ||
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path
  2016-08-09 15:19 ` [PATCH v2] " Paul Burton
@ 2016-08-09 21:57   ` Andrew Morton
  2016-10-16 18:07   ` Andreas Schwab
  1 sibling, 0 replies; 32+ messages in thread
From: Andrew Morton @ 2016-08-09 21:57 UTC (permalink / raw)
  To: Paul Burton
  Cc: linux-mips, Ralf Baechle, Tejun Heo, Sergey Senozhatsky,
	Jiri Slaby, Daniel Vetter, Ivan Delalande, Thierry Reding,
	Borislav Petkov, Jan Kara, Petr Mladek, linux-kernel,
	Joe Perches, Greg Kroah-Hartman, devicetree, Rob Herring,
	Frank Rowand

On Tue, 9 Aug 2016 16:19:37 +0100 Paul Burton <paul.burton@imgtec.com> wrote:

> If a device tree specifies a preferred device for kernel console output
> via the stdout-path or linux,stdout-path chosen node properties or the
> stdout alias then the kernel ought to honor it & output the kernel
> console to that device. As it stands, this isn't the case. Whilst we
> parse the stdout-path properties & set an of_stdout variable from
> of_alias_scan(), and use that from of_console_check() to determine
> whether to add a console device as a preferred console whilst
> registering it, we also prefer the first registered console if no other
> has been selected at the time of its registration.
> 
> This means that if a console other than the one the device tree selects
> via stdout-path is registered first, we will switch to using it & when
> the stdout-path console is later registered the call to
> add_preferred_console() via of_console_check() is too late to do
> anything useful. In practice this seems to mean that we switch to the
> dummy console device fairly early & see no further console output:
> 
>     Console: colour dummy device 80x25
>     console [tty0] enabled
>     bootconsole [ns16550a0] disabled
> 
> Fix this by not automatically preferring the first registered console if
> one is specified by the device tree. This allows consoles to be
> registered but not enabled, and once the driver for the console selected
> by stdout-path calls of_console_check() the driver will be added to the
> list of preferred consoles before any other console has been enabled.
> When that console is then registered via register_console() it will be
> enabled as expected.

Looks reasonable.  Could you please do `grep -r stdout-path
Documentation' and check that everything therein is complete, accurate
and necessary?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path
  2016-08-09 15:19 ` [PATCH v2] " Paul Burton
  2016-08-09 21:57   ` Andrew Morton
@ 2016-10-16 18:07   ` Andreas Schwab
  2016-10-17 10:33     ` Paul Burton
  1 sibling, 1 reply; 32+ messages in thread
From: Andreas Schwab @ 2016-10-16 18:07 UTC (permalink / raw)
  To: Paul Burton
  Cc: linux-mips, Ralf Baechle, Tejun Heo, Sergey Senozhatsky,
	Jiri Slaby, Daniel Vetter, Ivan Delalande, Thierry Reding,
	Borislav Petkov, Jan Kara, Petr Mladek, linux-kernel,
	Joe Perches, Greg Kroah-Hartman, devicetree, Rob Herring,
	Andrew Morton, Frank Rowand, linuxppc-dev

On Aug 09 2016, Paul Burton <paul.burton@imgtec.com> wrote:

> Fix this by not automatically preferring the first registered console if
> one is specified by the device tree. This allows consoles to be
> registered but not enabled, and once the driver for the console selected
> by stdout-path calls of_console_check() the driver will be added to the
> list of preferred consoles before any other console has been enabled.
> When that console is then registered via register_console() it will be
> enabled as expected.

This breaks the console on PowerMac.  There is no output and it panics
eventually.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path
  2016-10-16 18:07   ` Andreas Schwab
@ 2016-10-17 10:33     ` Paul Burton
  2016-10-17 17:39       ` Andreas Schwab
  0 siblings, 1 reply; 32+ messages in thread
From: Paul Burton @ 2016-10-17 10:33 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: linux-mips, Ralf Baechle, Tejun Heo, Sergey Senozhatsky,
	Jiri Slaby, Daniel Vetter, Ivan Delalande, Thierry Reding,
	Borislav Petkov, Jan Kara, Petr Mladek, linux-kernel,
	Joe Perches, Greg Kroah-Hartman, devicetree, Rob Herring,
	Andrew Morton, Frank Rowand, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]

On Sunday, 16 October 2016 20:07:18 BST Andreas Schwab wrote:
> On Aug 09 2016, Paul Burton <paul.burton@imgtec.com> wrote:
> > Fix this by not automatically preferring the first registered console if
> > one is specified by the device tree. This allows consoles to be
> > registered but not enabled, and once the driver for the console selected
> > by stdout-path calls of_console_check() the driver will be added to the
> > list of preferred consoles before any other console has been enabled.
> > When that console is then registered via register_console() it will be
> > enabled as expected.
> 
> This breaks the console on PowerMac.  There is no output and it panics
> eventually.
> 
> Andreas.

Hi Andreas,

Could you share the device tree from your system?

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path
  2016-10-17 10:33     ` Paul Burton
@ 2016-10-17 17:39       ` Andreas Schwab
  2016-10-18  9:18         ` [PATCH] console: use first console if stdout-path device doesn't appear Paul Burton
  2016-10-18  9:21         ` [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path Paul Burton
  0 siblings, 2 replies; 32+ messages in thread
From: Andreas Schwab @ 2016-10-17 17:39 UTC (permalink / raw)
  To: Paul Burton
  Cc: linux-mips, Ralf Baechle, Tejun Heo, Sergey Senozhatsky,
	Jiri Slaby, Daniel Vetter, Ivan Delalande, Thierry Reding,
	Borislav Petkov, Jan Kara, Petr Mladek, linux-kernel,
	Joe Perches, Greg Kroah-Hartman, devicetree, Rob Herring,
	Andrew Morton, Frank Rowand, linuxppc-dev

On Okt 17 2016, Paul Burton <paul.burton@imgtec.com> wrote:

> Could you share the device tree from your system?

This is the contents of chosen/linux,stdout-path on the systems I have:

chosen/linux,stdout-path
                 "/pci@f0000000/ATY,SnowyParent@10/ATY,Snowy_A@0"

chosen/linux,stdout-path                                                        
                 "/pci@0,f0000000/NVDA,Parent@10/NVDA,Display-B@1"

Is that what you need?  There is also chosen/stdout, but no
aliases/stdout.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH] console: use first console if stdout-path device doesn't appear
  2016-10-17 17:39       ` Andreas Schwab
@ 2016-10-18  9:18         ` Paul Burton
  2016-10-18 18:58           ` Andreas Schwab
  2016-10-18  9:21         ` [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path Paul Burton
  1 sibling, 1 reply; 32+ messages in thread
From: Paul Burton @ 2016-10-18  9:18 UTC (permalink / raw)
  To: linux-kernel, Andreas Schwab
  Cc: Paul Burton, Andrew Morton, Petr Mladek, Sergey Senozhatsky,
	Borislav Petkov, Tejun Heo, linuxppc-dev

If a device tree specified a preferred device for kernel console output
via the stdout-path or linux,stdout-path chosen node properties there's
no guarantee that it will have specified a device for which we have a
driver. It may also be the case that we do have a driver but it doesn't
call of_console_check() to register as a preferred console (eg. offb
driver as used on powermac systems). In these cases try to ensure that
we provide some console output by enabling the first console in the
console_drivers list.

As I don't have access to an affected system this has only been build
tested - testing would be most appreciated.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: 05fd007e4629 ("console: don't prefer first registered if DT specifies stdout-path")
Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
---
A potential alternative to this might be to have the affected offb
driver call of_check_console(), and perhaps that should happen anyway,
but doing so seems non-trivial since the offb driver doesn't know the
index of the framebuffer console device it may be about to register &
the fbdev core doesn't know the associated device tree node. This also
wouldn't catch the case of us not having a driver for the device
specified by stdout-path, so this fallback seems worthwhile anyway.
---
 kernel/printk/printk.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index d5e3973..7091e2f 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2835,10 +2835,45 @@ EXPORT_SYMBOL(unregister_console);
  * intersects with the init section. Note that code exists elsewhere to get
  * rid of the boot console as soon as the proper console shows up, so there
  * won't be side-effects from postponing the removal.
+ *
+ * Additionally we may be using a device tree which specifies valid
+ * stdout-path referencing a device for which we don't have a driver, or for
+ * which we have a driver that doesn't register itself as preferred console
+ * using of_console_check(). In these cases we attempt here to enable the
+ * first registered console.
  */
 static int __init printk_late_init(void)
 {
-	struct console *con;
+	struct console *con, *enabled;
+
+	if (of_specified_console) {
+		console_lock();
+
+		/* Find the enabled console, if there is one */
+		enabled = NULL;
+		for_each_console(con) {
+			if (!(con->flags & CON_ENABLED))
+				continue;
+
+			enabled = con;
+			break;
+		}
+
+		/* Enable the first console if none were already enabled */
+		con = console_drivers;
+		if (!enabled && con) {
+			if (con->index < 0)
+				con->index = 0;
+			if (con->setup == NULL ||
+			    con->setup(con, NULL) == 0) {
+				con->flags |= CON_ENABLED;
+				if (con->device)
+					con->flags |= CON_CONSDEV;
+			}
+		}
+
+		console_unlock();
+	}
 
 	for_each_console(con) {
 		if (!keep_bootcon && con->flags & CON_BOOT) {
-- 
2.10.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path
  2016-10-17 17:39       ` Andreas Schwab
  2016-10-18  9:18         ` [PATCH] console: use first console if stdout-path device doesn't appear Paul Burton
@ 2016-10-18  9:21         ` Paul Burton
  1 sibling, 0 replies; 32+ messages in thread
From: Paul Burton @ 2016-10-18  9:21 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: linux-mips, Ralf Baechle, Tejun Heo, Sergey Senozhatsky,
	Jiri Slaby, Daniel Vetter, Ivan Delalande, Thierry Reding,
	Borislav Petkov, Jan Kara, Petr Mladek, linux-kernel,
	Joe Perches, Greg Kroah-Hartman, devicetree, Rob Herring,
	Andrew Morton, Frank Rowand, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

On Monday, 17 October 2016 19:39:57 BST Andreas Schwab wrote:
> On Okt 17 2016, Paul Burton <paul.burton@imgtec.com> wrote:
> > Could you share the device tree from your system?
> 
> This is the contents of chosen/linux,stdout-path on the systems I have:
> 
> chosen/linux,stdout-path
>                  "/pci@f0000000/ATY,SnowyParent@10/ATY,Snowy_A@0"
> 
> chosen/linux,stdout-path
>                  "/pci@0,f0000000/NVDA,Parent@10/NVDA,Display-B@1"
> 
> Is that what you need?  There is also chosen/stdout, but no
> aliases/stdout.
> 
> Andreas.

Hi Andreas,

I think I see the problem & I'm hoping this patch will fix it:

https://lkml.org/lkml/2016/10/18/142

Could you give it a try & let me know?

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] console: use first console if stdout-path device doesn't appear
  2016-10-18  9:18         ` [PATCH] console: use first console if stdout-path device doesn't appear Paul Burton
@ 2016-10-18 18:58           ` Andreas Schwab
  2016-10-30  9:46             ` Andreas Schwab
  0 siblings, 1 reply; 32+ messages in thread
From: Andreas Schwab @ 2016-10-18 18:58 UTC (permalink / raw)
  To: Paul Burton
  Cc: linux-kernel, Andrew Morton, Petr Mladek, Sergey Senozhatsky,
	Borislav Petkov, Tejun Heo, linuxppc-dev

On Okt 18 2016, Paul Burton <paul.burton@imgtec.com> wrote:

> If a device tree specified a preferred device for kernel console output
> via the stdout-path or linux,stdout-path chosen node properties there's
> no guarantee that it will have specified a device for which we have a
> driver. It may also be the case that we do have a driver but it doesn't
> call of_console_check() to register as a preferred console (eg. offb
> driver as used on powermac systems). In these cases try to ensure that
> we provide some console output by enabling the first console in the
> console_drivers list.
>
> As I don't have access to an affected system this has only been build
> tested - testing would be most appreciated.

Unfortunately that doesn't work.  The initial console still cannot be
opened.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] console: use first console if stdout-path device doesn't appear
  2016-10-18 18:58           ` Andreas Schwab
@ 2016-10-30  9:46             ` Andreas Schwab
  2016-10-31  5:28               ` Michael Ellerman
  0 siblings, 1 reply; 32+ messages in thread
From: Andreas Schwab @ 2016-10-30  9:46 UTC (permalink / raw)
  To: Paul Burton
  Cc: linux-kernel, Andrew Morton, Petr Mladek, Sergey Senozhatsky,
	Borislav Petkov, Tejun Heo, linuxppc-dev

Any news?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] console: use first console if stdout-path device doesn't appear
  2016-10-30  9:46             ` Andreas Schwab
@ 2016-10-31  5:28               ` Michael Ellerman
  2016-10-31 12:14                 ` [PATCH v2] " Paul Burton
  2016-10-31 12:23                 ` [PATCH] " Paul Burton
  0 siblings, 2 replies; 32+ messages in thread
From: Michael Ellerman @ 2016-10-31  5:28 UTC (permalink / raw)
  To: Andreas Schwab, Paul Burton
  Cc: Petr Mladek, linux-kernel, Sergey Senozhatsky, Tejun Heo,
	Andrew Morton, Borislav Petkov, linuxppc-dev, Thorsten Leemhuis

Andreas Schwab <schwab@linux-m68k.org> writes:

> Any news?

We discovered it also breaks VGA on qemu, which presumably is not the
type of news you were hoping for.


To reproduce you just need to build a ppc64le kernel:

$ apt-get install gcc-powerpc64le-linux-gnu
$ make ARCH=powerpc CROSS_COMPILE=powerpc64le-linux-gnu- pseries_le_defconfig
$ make ARCH=powerpc CROSS_COMPILE=powerpc64le-linux-gnu-

And then run qemu:

$ qemu-system-ppc64 -M pseries -m 1G  -kernel vmlinux


Which will display the Tux logo but then nothing else and then reboot
after 10 seconds or so.

If you revert 05fd007e4629 on top of rc3 it boots fine.


Paul I tried your "console: use first console if stdout-path device
doesn't appear" patch, but it didn't help. I'll try and work out why,
but it's a bit hard with no output :)


If we can't come up with a fix soon I'm inclined to ask for a revert.

cheers

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v2] console: use first console if stdout-path device doesn't appear
  2016-10-31  5:28               ` Michael Ellerman
@ 2016-10-31 12:14                 ` Paul Burton
  2016-10-31 15:50                   ` Paul Burton
  2016-10-31 15:58                   ` Larry Finger
  2016-10-31 12:23                 ` [PATCH] " Paul Burton
  1 sibling, 2 replies; 32+ messages in thread
From: Paul Burton @ 2016-10-31 12:14 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Paul Burton, Andreas Schwab, Andrew Morton, Borislav Petkov,
	Larry Finger, Petr Mladek, Sergey Senozhatsky, Tejun Heo,
	linux-kernel, linuxppc-dev

If a device tree specified a preferred device for kernel console output
via the stdout-path or linux,stdout-path chosen node properties there's
no guarantee that it will have specified a device for which we have a
driver. It may also be the case that we do have a driver but it doesn't
call of_console_check() to register as a preferred console (eg. offb
driver as used on powermac systems). In these cases try to ensure that
we provide some console output by enabling the first usable registered
console, which we keep track of with the of_fallback_console variable.

Tested in QEMU with a PowerPC pseries_defconfig kernel.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: 05fd007e4629 ("console: don't prefer first registered if DT specifies stdout-path")
Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org

---

Changes in v2:
- Split enable_console() out of register_console() & call in the fallback case.
- Track the console we would have enabled as of_fallback_console.

 kernel/printk/printk.c | 60 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 55 insertions(+), 5 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index de08fc9..b02c00a 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -264,6 +264,13 @@ void console_set_by_of(void)
 # define of_specified_console false
 #endif
 
+/*
+ * The first usable console, which we'll fall back to using if the system
+ * uses a device tree which indicates a stdout-path device for which we
+ * have no driver, or for which our driver doesn't call of_console_check().
+ */
+static struct console *of_fallback_console;
+
 /* Flag: console code may call schedule() */
 static int console_may_schedule;
 
@@ -2598,6 +2605,8 @@ static int __init keep_bootcon_setup(char *str)
 
 early_param("keep_bootcon", keep_bootcon_setup);
 
+static void enable_console(struct console *newcon);
+
 /*
  * The console driver calls this routine during kernel initialization
  * to register the console printing procedure with printk() and to
@@ -2620,7 +2629,6 @@ early_param("keep_bootcon", keep_bootcon_setup);
 void register_console(struct console *newcon)
 {
 	int i;
-	unsigned long flags;
 	struct console *bcon = NULL;
 	struct console_cmdline *c;
 
@@ -2657,7 +2665,9 @@ void register_console(struct console *newcon)
 	 *	didn't select a console we take the first one
 	 *	that registers here.
 	 */
-	if (preferred_console < 0 && !of_specified_console) {
+	if (preferred_console < 0 && of_specified_console) {
+		of_fallback_console = of_fallback_console ?: newcon;
+	} else if (preferred_console < 0) {
 		if (newcon->index < 0)
 			newcon->index = 0;
 		if (newcon->setup == NULL ||
@@ -2705,8 +2715,18 @@ void register_console(struct console *newcon)
 		break;
 	}
 
-	if (!(newcon->flags & CON_ENABLED))
-		return;
+	if (newcon->flags & CON_ENABLED)
+		enable_console(newcon);
+}
+EXPORT_SYMBOL(register_console);
+
+static void enable_console(struct console *newcon)
+{
+	struct console *bcon = NULL;
+	unsigned long flags;
+
+	if (console_drivers && console_drivers->flags & CON_BOOT)
+		bcon = console_drivers;
 
 	/*
 	 * If we have a bootconsole, and are switching to a real console,
@@ -2777,7 +2797,6 @@ void register_console(struct console *newcon)
 				unregister_console(bcon);
 	}
 }
-EXPORT_SYMBOL(register_console);
 
 int unregister_console(struct console *console)
 {
@@ -2839,10 +2858,41 @@ EXPORT_SYMBOL(unregister_console);
  * intersects with the init section. Note that code exists elsewhere to get
  * rid of the boot console as soon as the proper console shows up, so there
  * won't be side-effects from postponing the removal.
+ *
+ * Additionally we may be using a device tree which specifies valid
+ * stdout-path referencing a device for which we don't have a driver, or for
+ * which we have a driver that doesn't register itself as preferred console
+ * using of_console_check(). In these cases we attempt here to enable the
+ * first usable registered console, which we assigned to of_fallback_console.
  */
 static int __init printk_late_init(void)
 {
 	struct console *con;
+	bool any_enabled = false;
+
+	for_each_console(con) {
+		if (!(con->flags & CON_ENABLED))
+			continue;
+
+		any_enabled = true;
+		break;
+	}
+
+	if (!any_enabled && of_fallback_console) {
+		if (of_fallback_console->index < 0)
+			of_fallback_console->index = 0;
+
+		if (!of_fallback_console->setup ||
+		    !of_fallback_console->setup(of_fallback_console, NULL)) {
+			of_fallback_console->flags |= CON_ENABLED;
+			if (of_fallback_console->device) {
+				of_fallback_console->flags |= CON_CONSDEV;
+				preferred_console = 0;
+			}
+		}
+
+		enable_console(of_fallback_console);
+	}
 
 	for_each_console(con) {
 		if (!keep_bootcon && con->flags & CON_BOOT) {
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH] console: use first console if stdout-path device doesn't appear
  2016-10-31  5:28               ` Michael Ellerman
  2016-10-31 12:14                 ` [PATCH v2] " Paul Burton
@ 2016-10-31 12:23                 ` Paul Burton
  1 sibling, 0 replies; 32+ messages in thread
From: Paul Burton @ 2016-10-31 12:23 UTC (permalink / raw)
  To: Michael Ellerman, Andreas Schwab, larry.finger
  Cc: Petr Mladek, linux-kernel, Sergey Senozhatsky, Tejun Heo,
	Andrew Morton, Borislav Petkov, linuxppc-dev, Thorsten Leemhuis

[-- Attachment #1: Type: text/plain, Size: 648 bytes --]

Hi Michael et al,

On Monday, 31 October 2016 16:28:59 GMT Michael Ellerman wrote:
> Andreas Schwab <schwab@linux-m68k.org> writes:
> > Any news?
> 
> We discovered it also breaks VGA on qemu, which presumably is not the
> type of news you were hoping for.

On the contrary, that's wonderful news - I can test that! Huzzah for 
brokenness!

Thanks for your steps to reproduce the issue. Could you try my v2 patch? It 
works for me in QEMU, at least as far as seeing the kernel console output for 
a panic due to not having a rootfs. Hopefully it works for you (& Andreas & 
Larry) too:

https://patchwork.kernel.org/patch/9405415/

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: use first console if stdout-path device doesn't appear
  2016-10-31 12:14                 ` [PATCH v2] " Paul Burton
@ 2016-10-31 15:50                   ` Paul Burton
  2016-10-31 19:21                     ` Larry Finger
  2016-10-31 23:09                     ` Sergey Senozhatsky
  2016-10-31 15:58                   ` Larry Finger
  1 sibling, 2 replies; 32+ messages in thread
From: Paul Burton @ 2016-10-31 15:50 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Andreas Schwab, Andrew Morton, Borislav Petkov, Larry Finger,
	Petr Mladek, Sergey Senozhatsky, Tejun Heo, linux-kernel,
	linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 5759 bytes --]

On Monday, 31 October 2016 12:14:55 GMT Paul Burton wrote:
> If a device tree specified a preferred device for kernel console output
> via the stdout-path or linux,stdout-path chosen node properties there's
> no guarantee that it will have specified a device for which we have a
> driver. It may also be the case that we do have a driver but it doesn't
> call of_console_check() to register as a preferred console (eg. offb
> driver as used on powermac systems). In these cases try to ensure that
> we provide some console output by enabling the first usable registered
> console, which we keep track of with the of_fallback_console variable.
> 
> Tested in QEMU with a PowerPC pseries_defconfig kernel.

Actually whilst this fixes the output in QEMU it has other problems. I'm still 
digging...

Thanks,
    Paul

> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Fixes: 05fd007e4629 ("console: don't prefer first registered if DT specifies
> stdout-path") Reported-by: Andreas Schwab <schwab@linux-m68k.org>
> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> 
> ---
> 
> Changes in v2:
> - Split enable_console() out of register_console() & call in the fallback
> case. - Track the console we would have enabled as of_fallback_console.
> 
>  kernel/printk/printk.c | 60
> +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 55
> insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index de08fc9..b02c00a 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -264,6 +264,13 @@ void console_set_by_of(void)
>  # define of_specified_console false
>  #endif
> 
> +/*
> + * The first usable console, which we'll fall back to using if the system
> + * uses a device tree which indicates a stdout-path device for which we
> + * have no driver, or for which our driver doesn't call of_console_check().
> + */
> +static struct console *of_fallback_console;
> +
>  /* Flag: console code may call schedule() */
>  static int console_may_schedule;
> 
> @@ -2598,6 +2605,8 @@ static int __init keep_bootcon_setup(char *str)
> 
>  early_param("keep_bootcon", keep_bootcon_setup);
> 
> +static void enable_console(struct console *newcon);
> +
>  /*
>   * The console driver calls this routine during kernel initialization
>   * to register the console printing procedure with printk() and to
> @@ -2620,7 +2629,6 @@ early_param("keep_bootcon", keep_bootcon_setup);
>  void register_console(struct console *newcon)
>  {
>  	int i;
> -	unsigned long flags;
>  	struct console *bcon = NULL;
>  	struct console_cmdline *c;
> 
> @@ -2657,7 +2665,9 @@ void register_console(struct console *newcon)
>  	 *	didn't select a console we take the first one
>  	 *	that registers here.
>  	 */
> -	if (preferred_console < 0 && !of_specified_console) {
> +	if (preferred_console < 0 && of_specified_console) {
> +		of_fallback_console = of_fallback_console ?: newcon;
> +	} else if (preferred_console < 0) {
>  		if (newcon->index < 0)
>  			newcon->index = 0;
>  		if (newcon->setup == NULL ||
> @@ -2705,8 +2715,18 @@ void register_console(struct console *newcon)
>  		break;
>  	}
> 
> -	if (!(newcon->flags & CON_ENABLED))
> -		return;
> +	if (newcon->flags & CON_ENABLED)
> +		enable_console(newcon);
> +}
> +EXPORT_SYMBOL(register_console);
> +
> +static void enable_console(struct console *newcon)
> +{
> +	struct console *bcon = NULL;
> +	unsigned long flags;
> +
> +	if (console_drivers && console_drivers->flags & CON_BOOT)
> +		bcon = console_drivers;
> 
>  	/*
>  	 * If we have a bootconsole, and are switching to a real console,
> @@ -2777,7 +2797,6 @@ void register_console(struct console *newcon)
>  				unregister_console(bcon);
>  	}
>  }
> -EXPORT_SYMBOL(register_console);
> 
>  int unregister_console(struct console *console)
>  {
> @@ -2839,10 +2858,41 @@ EXPORT_SYMBOL(unregister_console);
>   * intersects with the init section. Note that code exists elsewhere to get
> * rid of the boot console as soon as the proper console shows up, so there
> * won't be side-effects from postponing the removal.
> + *
> + * Additionally we may be using a device tree which specifies valid
> + * stdout-path referencing a device for which we don't have a driver, or
> for + * which we have a driver that doesn't register itself as preferred
> console + * using of_console_check(). In these cases we attempt here to
> enable the + * first usable registered console, which we assigned to
> of_fallback_console. */
>  static int __init printk_late_init(void)
>  {
>  	struct console *con;
> +	bool any_enabled = false;
> +
> +	for_each_console(con) {
> +		if (!(con->flags & CON_ENABLED))
> +			continue;
> +
> +		any_enabled = true;
> +		break;
> +	}
> +
> +	if (!any_enabled && of_fallback_console) {
> +		if (of_fallback_console->index < 0)
> +			of_fallback_console->index = 0;
> +
> +		if (!of_fallback_console->setup ||
> +		    !of_fallback_console->setup(of_fallback_console, NULL)) {
> +			of_fallback_console->flags |= CON_ENABLED;
> +			if (of_fallback_console->device) {
> +				of_fallback_console->flags |= CON_CONSDEV;
> +				preferred_console = 0;
> +			}
> +		}
> +
> +		enable_console(of_fallback_console);
> +	}
> 
>  	for_each_console(con) {
>  		if (!keep_bootcon && con->flags & CON_BOOT) {


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: use first console if stdout-path device doesn't appear
  2016-10-31 12:14                 ` [PATCH v2] " Paul Burton
  2016-10-31 15:50                   ` Paul Burton
@ 2016-10-31 15:58                   ` Larry Finger
  1 sibling, 0 replies; 32+ messages in thread
From: Larry Finger @ 2016-10-31 15:58 UTC (permalink / raw)
  To: Paul Burton, Michael Ellerman
  Cc: Andreas Schwab, Andrew Morton, Borislav Petkov, Petr Mladek,
	Sergey Senozhatsky, Tejun Heo, linux-kernel, linuxppc-dev

On 10/31/2016 07:14 AM, Paul Burton wrote:
> If a device tree specified a preferred device for kernel console output
> via the stdout-path or linux,stdout-path chosen node properties there's
> no guarantee that it will have specified a device for which we have a
> driver. It may also be the case that we do have a driver but it doesn't
> call of_console_check() to register as a preferred console (eg. offb
> driver as used on powermac systems). In these cases try to ensure that
> we provide some console output by enabling the first usable registered
> console, which we keep track of with the of_fallback_console variable.
>
> Tested in QEMU with a PowerPC pseries_defconfig kernel.

The patch fails to fix my real PowerPC. I still get a kernel panic for an 
attempt to kill init. This first test was done with 4.9-rc2. I am in the process 
of updating to -rc3 to see if that changes anything. Later today when that build 
finishes, I will report those results.

Larry

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: use first console if stdout-path device doesn't appear
  2016-10-31 15:50                   ` Paul Burton
@ 2016-10-31 19:21                     ` Larry Finger
  2016-10-31 23:09                     ` Sergey Senozhatsky
  1 sibling, 0 replies; 32+ messages in thread
From: Larry Finger @ 2016-10-31 19:21 UTC (permalink / raw)
  To: Paul Burton, Michael Ellerman
  Cc: Andreas Schwab, Andrew Morton, Borislav Petkov, Petr Mladek,
	Sergey Senozhatsky, Tejun Heo, linux-kernel, linuxppc-dev

On 10/31/2016 10:50 AM, Paul Burton wrote:
> On Monday, 31 October 2016 12:14:55 GMT Paul Burton wrote:
>> If a device tree specified a preferred device for kernel console output
>> via the stdout-path or linux,stdout-path chosen node properties there's
>> no guarantee that it will have specified a device for which we have a
>> driver. It may also be the case that we do have a driver but it doesn't
>> call of_console_check() to register as a preferred console (eg. offb
>> driver as used on powermac systems). In these cases try to ensure that
>> we provide some console output by enabling the first usable registered
>> console, which we keep track of with the of_fallback_console variable.
>>
>> Tested in QEMU with a PowerPC pseries_defconfig kernel.
>
> Actually whilst this fixes the output in QEMU it has other problems. I'm still
> digging...

As expected, it does n ot work with 4.9-rc3 on the real PowerPC.

Larry

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: use first console if stdout-path device doesn't appear
  2016-10-31 15:50                   ` Paul Burton
  2016-10-31 19:21                     ` Larry Finger
@ 2016-10-31 23:09                     ` Sergey Senozhatsky
  2016-10-31 23:31                       ` Larry Finger
  2016-11-01  4:39                       ` Michael Ellerman
  1 sibling, 2 replies; 32+ messages in thread
From: Sergey Senozhatsky @ 2016-10-31 23:09 UTC (permalink / raw)
  To: Paul Burton
  Cc: Michael Ellerman, Andreas Schwab, Andrew Morton, Borislav Petkov,
	Larry Finger, Petr Mladek, Sergey Senozhatsky, Tejun Heo,
	linux-kernel, linuxppc-dev

Hi,

On (10/31/16 15:50), Paul Burton wrote:
[..]
> Actually whilst this fixes the output in QEMU it has other problems. I'm still 
> digging...

I propose a revert of '05fd007e46296', so you guys can find the
problem and fix it, not being under 'rc3' pressure.

	-ss

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: use first console if stdout-path device doesn't appear
  2016-10-31 23:09                     ` Sergey Senozhatsky
@ 2016-10-31 23:31                       ` Larry Finger
  2016-11-03 12:57                         ` [PATCH v3] " Paul Burton
  2016-11-03 13:04                         ` [PATCH v2] console: use first console if stdout-path device doesn't appear Paul Burton
  2016-11-01  4:39                       ` Michael Ellerman
  1 sibling, 2 replies; 32+ messages in thread
From: Larry Finger @ 2016-10-31 23:31 UTC (permalink / raw)
  To: Sergey Senozhatsky, Paul Burton
  Cc: Michael Ellerman, Andreas Schwab, Andrew Morton, Borislav Petkov,
	Petr Mladek, Tejun Heo, linux-kernel, linuxppc-dev

On 10/31/2016 06:09 PM, Sergey Senozhatsky wrote:
> Hi,
>
> On (10/31/16 15:50), Paul Burton wrote:
> [..]
>> Actually whilst this fixes the output in QEMU it has other problems. I'm still
>> digging...
>
> I propose a revert of '05fd007e46296', so you guys can find the
> problem and fix it, not being under 'rc3' pressure.

If we were at -rc4 or -rc5, then I would agree; however, I think there is still 
time to fix the problem.

Larry

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: use first console if stdout-path device doesn't appear
  2016-10-31 23:09                     ` Sergey Senozhatsky
  2016-10-31 23:31                       ` Larry Finger
@ 2016-11-01  4:39                       ` Michael Ellerman
  1 sibling, 0 replies; 32+ messages in thread
From: Michael Ellerman @ 2016-11-01  4:39 UTC (permalink / raw)
  To: Sergey Senozhatsky, Paul Burton
  Cc: Andreas Schwab, Andrew Morton, Borislav Petkov, Larry Finger,
	Petr Mladek, Sergey Senozhatsky, Tejun Heo, linux-kernel,
	linuxppc-dev

Sergey Senozhatsky <sergey.senozhatsky@gmail.com> writes:
> On (10/31/16 15:50), Paul Burton wrote:
> [..]
>> Actually whilst this fixes the output in QEMU it has other problems. I'm still 
>> digging...
>
> I propose a revert of '05fd007e46296', so you guys can find the
> problem and fix it, not being under 'rc3' pressure.

Yeah I agree. The v2 fix doesn't work for me either.

Paul are you OK with that? And if so are you happy to send a revert to
Linus?

cheers

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v3] console: use first console if stdout-path device doesn't appear
  2016-10-31 23:31                       ` Larry Finger
@ 2016-11-03 12:57                         ` Paul Burton
  2016-11-03 17:40                           ` Sergey Senozhatsky
                                             ` (2 more replies)
  2016-11-03 13:04                         ` [PATCH v2] console: use first console if stdout-path device doesn't appear Paul Burton
  1 sibling, 3 replies; 32+ messages in thread
From: Paul Burton @ 2016-11-03 12:57 UTC (permalink / raw)
  To: Larry Finger, Michael Ellerman, Sergey Senozhatsky
  Cc: Paul Burton, Andreas Schwab, Andrew Morton, Borislav Petkov,
	Petr Mladek, Tejun Heo, linux-kernel, linuxppc-dev

If a device tree specified a preferred device for kernel console output
via the stdout-path or linux,stdout-path chosen node properties there's
no guarantee that it will have specified a device for which we have a
driver. It may also be the case that we do have a driver but it doesn't
call of_console_check() to register as a preferred console (eg. offb
driver as used on powermac systems).

In these cases try to ensure that we provide some console output by
enabling the first usable registered console, which we keep track of
with the of_fallback_console variable. Affected systems will enable
their console later than they did prior to commit 05fd007e4629
("console: don't prefer first registered if DT specifies stdout-path")
but should otherwise produce the same output.

Tested in QEMU with a PowerPC pseries_defconfig kernel.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Fixes: 05fd007e4629 ("console: don't prefer first registered if DT specifies stdout-path")
Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org

---

Changes in v3:
- Be less invasive by simply calling register_console() again rather than splitting it.
- Check for CON_CONSDEV on the first console driver rather than for CON_ENABLED on any.
- Clean up the tracking of the fallback console.

Changes in v2:
- Split enable_console() out of register_console() & call in the fallback case.
- Track the console we would have enabled as of_fallback_console.

 kernel/printk/printk.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index de08fc9..c86e6d0 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -260,10 +260,18 @@ void console_set_by_of(void)
 {
 	of_specified_console = true;
 }
+
+static void clear_of_specified_console(void)
+{
+	of_specified_console = false;
+}
 #else
 # define of_specified_console false
+static void clear_of_specified_console(void) { }
 #endif
 
+struct console *of_fallback_console;
+
 /* Flag: console code may call schedule() */
 static int console_may_schedule;
 
@@ -2657,10 +2665,26 @@ void register_console(struct console *newcon)
 	 *	didn't select a console we take the first one
 	 *	that registers here.
 	 */
-	if (preferred_console < 0 && !of_specified_console) {
+	if (preferred_console < 0) {
 		if (newcon->index < 0)
 			newcon->index = 0;
-		if (newcon->setup == NULL ||
+		if (of_specified_console) {
+			/*
+			 * The device tree stdout-path chosen node property was
+			 * specified so we don't want to enable the first
+			 * registered console just now in order to give the
+			 * device indicated by stdout-path a chance to be
+			 * registered first. Do however keep track of the
+			 * first console we see so that we can fall back to
+			 * using it if we don't see the desired device, either
+			 * because stdout-path isn't valid, or because we have
+			 * no driver for the device or our driver doesn't call
+			 * of_console_check(). See printk_late_init() for this
+			 * fallback.
+			 */
+			if (!of_fallback_console)
+				of_fallback_console = newcon;
+		} else if (newcon->setup == NULL ||
 		    newcon->setup(newcon, NULL) == 0) {
 			newcon->flags |= CON_ENABLED;
 			if (newcon->device) {
@@ -2844,6 +2868,22 @@ static int __init printk_late_init(void)
 {
 	struct console *con;
 
+	if (of_specified_console && of_fallback_console &&
+	    (!console_drivers || !(console_drivers->flags & CON_CONSDEV))) {
+		/*
+		 * The system has a device tree which specified stdout-path,
+		 * but we haven't seen a console associated with the device
+		 * specified by the stdout-path chosen node property.
+		 *
+		 * We do however know which console would have been used
+		 * if stdout-path weren't specified at all, so in an attempt
+		 * to provide some output we'll re-register that console
+		 * pretending that we never saw stdout-path.
+		 */
+		clear_of_specified_console();
+		register_console(of_fallback_console);
+	}
+
 	for_each_console(con) {
 		if (!keep_bootcon && con->flags & CON_BOOT) {
 			/*
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v2] console: use first console if stdout-path device doesn't appear
  2016-10-31 23:31                       ` Larry Finger
  2016-11-03 12:57                         ` [PATCH v3] " Paul Burton
@ 2016-11-03 13:04                         ` Paul Burton
  1 sibling, 0 replies; 32+ messages in thread
From: Paul Burton @ 2016-11-03 13:04 UTC (permalink / raw)
  To: Larry Finger, Sergey Senozhatsky, Michael Ellerman
  Cc: Andreas Schwab, Andrew Morton, Borislav Petkov, Petr Mladek,
	Tejun Heo, linux-kernel, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

On Monday, 31 October 2016 18:31:43 GMT Larry Finger wrote:
> On 10/31/2016 06:09 PM, Sergey Senozhatsky wrote:
> > Hi,
> > 
> > On (10/31/16 15:50), Paul Burton wrote:
> > [..]
> > 
> >> Actually whilst this fixes the output in QEMU it has other problems. I'm
> >> still digging...
> > 
> > I propose a revert of '05fd007e46296', so you guys can find the
> > problem and fix it, not being under 'rc3' pressure.
> 
> If we were at -rc4 or -rc5, then I would agree; however, I think there is
> still time to fix the problem.
> 
> Larry

Hi Larry et al,

If you could give the v3 I just submitted a try that would be great:

    https://patchwork.kernel.org/patch/9410849/

If it still doesn't work for you then we're back to a place where I can't test 
an affected system, since QEMU pseries now works fine with my patch. Would you 
perhaps be able to get some console output by specifying console=<whatever> on 
the kernel command line? Doing that ought to be unaffected by 05fd007e4629 
("console: don't prefer first registered if DT specifies stdout-path") and 
could be a way to get us some debug output.

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] console: use first console if stdout-path device doesn't appear
  2016-11-03 12:57                         ` [PATCH v3] " Paul Burton
@ 2016-11-03 17:40                           ` Sergey Senozhatsky
  2016-11-03 21:17                             ` Paul Burton
  2016-11-04  8:05                           ` Andreas Schwab
       [not found]                           ` <8737j3n18r.fsf@concordia.ellerman.id.au>
  2 siblings, 1 reply; 32+ messages in thread
From: Sergey Senozhatsky @ 2016-11-03 17:40 UTC (permalink / raw)
  To: Paul Burton
  Cc: Larry Finger, Michael Ellerman, Sergey Senozhatsky,
	Andreas Schwab, Andrew Morton, Borislav Petkov, Petr Mladek,
	Tejun Heo, linux-kernel, linuxppc-dev

On (11/03/16 12:57), Paul Burton wrote:
> If a device tree specified a preferred device for kernel console output
> via the stdout-path or linux,stdout-path chosen node properties there's
> no guarantee that it will have specified a device for which we have a
> driver. It may also be the case that we do have a driver but it doesn't
> call of_console_check() to register as a preferred console (eg. offb
> driver as used on powermac systems).

so why that driver doesn't call of_console_check() then? if there is a
misconfiguration then why do we want to fix it/fallback in printk code?

[..]
> @@ -260,10 +260,18 @@ void console_set_by_of(void)
>  {
>  	of_specified_console = true;
>  }
> +
> +static void clear_of_specified_console(void)
> +{
> +	of_specified_console = false;
> +}
>  #else
>  # define of_specified_console false
> +static void clear_of_specified_console(void) { }
>  #endif
>  
> +struct console *of_fallback_console;
> +
>  /* Flag: console code may call schedule() */
>  static int console_may_schedule;
>  
> @@ -2657,10 +2665,26 @@ void register_console(struct console *newcon)
>  	 *	didn't select a console we take the first one
>  	 *	that registers here.
>  	 */
> -	if (preferred_console < 0 && !of_specified_console) {
> +	if (preferred_console < 0) {
>  		if (newcon->index < 0)
>  			newcon->index = 0;
> -		if (newcon->setup == NULL ||
> +		if (of_specified_console) {
> +			/*
> +			 * The device tree stdout-path chosen node property was
> +			 * specified so we don't want to enable the first
> +			 * registered console just now in order to give the
> +			 * device indicated by stdout-path a chance to be
> +			 * registered first. Do however keep track of the
> +			 * first console we see so that we can fall back to
> +			 * using it if we don't see the desired device, either
> +			 * because stdout-path isn't valid, or because we have
> +			 * no driver for the device or our driver doesn't call
> +			 * of_console_check(). See printk_late_init() for this
> +			 * fallback.

if the path is not valid then correct the path. no?

> +			 */
> +			if (!of_fallback_console)
> +				of_fallback_console = newcon;
> +		} else if (newcon->setup == NULL ||
>  		    newcon->setup(newcon, NULL) == 0) {
>  			newcon->flags |= CON_ENABLED;
>  			if (newcon->device) {
> @@ -2844,6 +2868,22 @@ static int __init printk_late_init(void)
>  {
>  	struct console *con;
>  
> +	if (of_specified_console && of_fallback_console &&
> +	    (!console_drivers || !(console_drivers->flags & CON_CONSDEV))) {
> +		/*
> +		 * The system has a device tree which specified stdout-path,
> +		 * but we haven't seen a console associated with the device
> +		 * specified by the stdout-path chosen node property.
> +		 *
> +		 * We do however know which console would have been used
> +		 * if stdout-path weren't specified at all, so in an attempt
> +		 * to provide some output we'll re-register that console
> +		 * pretending that we never saw stdout-path.
> +		 */

DT screwed up, so why would printk() care? does any other
sub-system/driver fixes up a DT misconfiguration?

	-ss

> +		clear_of_specified_console();
> +		register_console(of_fallback_console);
> +	}
> +
>  	for_each_console(con) {
>  		if (!keep_bootcon && con->flags & CON_BOOT) {
>  			/*
> -- 
> 2.10.2
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] console: use first console if stdout-path device doesn't appear
  2016-11-03 17:40                           ` Sergey Senozhatsky
@ 2016-11-03 21:17                             ` Paul Burton
  2016-11-04 15:44                               ` Sergey Senozhatsky
  0 siblings, 1 reply; 32+ messages in thread
From: Paul Burton @ 2016-11-03 21:17 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Larry Finger, Michael Ellerman, Andreas Schwab, Andrew Morton,
	Borislav Petkov, Petr Mladek, Tejun Heo, linux-kernel,
	linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 5141 bytes --]

Hi Sergey,

On Friday, 4 November 2016 02:40:40 GMT Sergey Senozhatsky wrote:
> On (11/03/16 12:57), Paul Burton wrote:
> > If a device tree specified a preferred device for kernel console output
> > via the stdout-path or linux,stdout-path chosen node properties there's
> > no guarantee that it will have specified a device for which we have a
> > driver. It may also be the case that we do have a driver but it doesn't
> > call of_console_check() to register as a preferred console (eg. offb
> > driver as used on powermac systems).
> 
> so why that driver doesn't call of_console_check() then? if there is a
> misconfiguration then why do we want to fix it/fallback in printk code?

Ideally I think the driver (or perhaps fbdev/fbcon) ought to call 
of_console_check() which would allow the console to be enabled earlier again. 
However there isn't any single place I can see that currently has all the 
information required to do so - the device tree node, the name & index of the 
console.

Even if we change that in the future & do call of_console_check(), I can't 
guarantee that offb is the only driver that would encounter this case, and it 
still wouldn't cover the case of us not having any driver for the specified 
stdout-path device. The fallback thus seems like a sensible thing to do.

> 
> [..]
> 
> > @@ -260,10 +260,18 @@ void console_set_by_of(void)
> > 
> >  {
> >  
> >  	of_specified_console = true;
> >  
> >  }
> > 
> > +
> > +static void clear_of_specified_console(void)
> > +{
> > +	of_specified_console = false;
> > +}
> > 
> >  #else
> >  # define of_specified_console false
> > 
> > +static void clear_of_specified_console(void) { }
> > 
> >  #endif
> > 
> > +struct console *of_fallback_console;
> > +
> > 
> >  /* Flag: console code may call schedule() */
> >  static int console_may_schedule;
> > 
> > @@ -2657,10 +2665,26 @@ void register_console(struct console *newcon)
> > 
> >  	 *	didn't select a console we take the first one
> >  	 *	that registers here.
> >  	 */
> > 
> > -	if (preferred_console < 0 && !of_specified_console) {
> > +	if (preferred_console < 0) {
> > 
> >  		if (newcon->index < 0)
> >  		
> >  			newcon->index = 0;
> > 
> > -		if (newcon->setup == NULL ||
> > +		if (of_specified_console) {
> > +			/*
> > +			 * The device tree stdout-path chosen node property was
> > +			 * specified so we don't want to enable the first
> > +			 * registered console just now in order to give the
> > +			 * device indicated by stdout-path a chance to be
> > +			 * registered first. Do however keep track of the
> > +			 * first console we see so that we can fall back to
> > +			 * using it if we don't see the desired device, either
> > +			 * because stdout-path isn't valid, or because we have
> > +			 * no driver for the device or our driver doesn't call
> > +			 * of_console_check(). See printk_late_init() for this
> > +			 * fallback.
> 
> if the path is not valid then correct the path. no?

...but what if the path is valid and we simply don't have a driver for the 
device it references? As I said in that comment we may not have a driver at 
all.

> 
> > +			 */
> > +			if (!of_fallback_console)
> > +				of_fallback_console = newcon;
> > +		} else if (newcon->setup == NULL ||
> > 
> >  		    newcon->setup(newcon, NULL) == 0) {
> >  			
> >  			newcon->flags |= CON_ENABLED;
> >  			if (newcon->device) {
> > 
> > @@ -2844,6 +2868,22 @@ static int __init printk_late_init(void)
> > 
> >  {
> >  
> >  	struct console *con;
> > 
> > +	if (of_specified_console && of_fallback_console &&
> > +	    (!console_drivers || !(console_drivers->flags & CON_CONSDEV))) {
> > +		/*
> > +		 * The system has a device tree which specified stdout-path,
> > +		 * but we haven't seen a console associated with the device
> > +		 * specified by the stdout-path chosen node property.
> > +		 *
> > +		 * We do however know which console would have been used
> > +		 * if stdout-path weren't specified at all, so in an attempt
> > +		 * to provide some output we'll re-register that console
> > +		 * pretending that we never saw stdout-path.
> > +		 */
> 
> DT screwed up, so why would printk() care? does any other
> sub-system/driver fixes up a DT misconfiguration?
> 
> 	-ss

...and again, it may not be a misconfiguration - that's one possibility of a 
few that I mentioned. Even if the DT is misconfigured & stdout-path is 
completely bonkers it's still arguable that falling back to the first console 
registered (ie. our previous behaviour) is the sensible thing to do. Perhaps 
we ought to warn in such cases, but even then we can't distinguish between the 
3 cases I mentioned (invalid stdout-path, driver which doesn't call 
of_console_check() or no driver at all) so we'd certainly end up warning on 
systems like these affected PowerPC systems, which makes me think that may be 
a better warning to introduce once we expect systems to not hit it.

Thanks,
    Paul

> 
> > +		clear_of_specified_console();
> > +		register_console(of_fallback_console);
> > +	}
> > +
> > 
> >  	for_each_console(con) {
> >  	
> >  		if (!keep_bootcon && con->flags & CON_BOOT) {
> >  		
> >  			/*


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] console: use first console if stdout-path device doesn't appear
  2016-11-03 12:57                         ` [PATCH v3] " Paul Burton
  2016-11-03 17:40                           ` Sergey Senozhatsky
@ 2016-11-04  8:05                           ` Andreas Schwab
       [not found]                           ` <8737j3n18r.fsf@concordia.ellerman.id.au>
  2 siblings, 0 replies; 32+ messages in thread
From: Andreas Schwab @ 2016-11-04  8:05 UTC (permalink / raw)
  To: Paul Burton
  Cc: Larry Finger, Michael Ellerman, Sergey Senozhatsky,
	Andrew Morton, Borislav Petkov, Petr Mladek, Tejun Heo,
	linux-kernel, linuxppc-dev

On Nov 03 2016, Paul Burton <paul.burton@imgtec.com> wrote:

> If a device tree specified a preferred device for kernel console output
> via the stdout-path or linux,stdout-path chosen node properties there's
> no guarantee that it will have specified a device for which we have a
> driver. It may also be the case that we do have a driver but it doesn't
> call of_console_check() to register as a preferred console (eg. offb
> driver as used on powermac systems).
>
> In these cases try to ensure that we provide some console output by
> enabling the first usable registered console, which we keep track of
> with the of_fallback_console variable. Affected systems will enable
> their console later than they did prior to commit 05fd007e4629
> ("console: don't prefer first registered if DT specifies stdout-path")
> but should otherwise produce the same output.
>
> Tested in QEMU with a PowerPC pseries_defconfig kernel.

Unfortunately, that still doesn't work on PowerMac.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] console: use first console if stdout-path device doesn't appear
  2016-11-03 21:17                             ` Paul Burton
@ 2016-11-04 15:44                               ` Sergey Senozhatsky
  0 siblings, 0 replies; 32+ messages in thread
From: Sergey Senozhatsky @ 2016-11-04 15:44 UTC (permalink / raw)
  To: Paul Burton
  Cc: Sergey Senozhatsky, Larry Finger, Michael Ellerman,
	Andreas Schwab, Andrew Morton, Borislav Petkov, Petr Mladek,
	Tejun Heo, linux-kernel, linuxppc-dev

Hi Paul,

On (11/03/16 21:17), Paul Burton wrote:
> > [..]
> > > +			 * The device tree stdout-path chosen node property was
> > > +			 * specified so we don't want to enable the first
> > > +			 * registered console just now in order to give the
> > > +			 * device indicated by stdout-path a chance to be
> > > +			 * registered first. Do however keep track of the
> > > +			 * first console we see so that we can fall back to
> > > +			 * using it if we don't see the desired device, either
> > > +			 * because stdout-path isn't valid, or because we have
> > > +			 * no driver for the device or our driver doesn't call
> > > +			 * of_console_check(). See printk_late_init() for this
> > > +			 * fallback.
> > 
> > if the path is not valid then correct the path. no?
> 
> ...but what if the path is valid and we simply don't have a driver for the 
> device it references? As I said in that comment we may not have a driver at 
> all.

well, I suppose, in this case normally one would go and enable the
missing .config option. no?

	-ss

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] console: use first console if stdout-path device doesn't appear
       [not found]                           ` <8737j3n18r.fsf@concordia.ellerman.id.au>
@ 2016-11-07  9:18                             ` Paul Burton
  2016-11-07 15:26                               ` Larry Finger
  0 siblings, 1 reply; 32+ messages in thread
From: Paul Burton @ 2016-11-07  9:18 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Larry Finger, Sergey Senozhatsky, Andreas Schwab, Andrew Morton,
	Borislav Petkov, Petr Mladek, Tejun Heo, linux-kernel,
	linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

On Monday, 7 November 2016 19:27:32 GMT Michael Ellerman wrote:
> Paul Burton <paul.burton@imgtec.com> writes:
> > If a device tree specified a preferred device for kernel console output
> > via the stdout-path or linux,stdout-path chosen node properties there's
> > no guarantee that it will have specified a device for which we have a
> > driver. It may also be the case that we do have a driver but it doesn't
> > call of_console_check() to register as a preferred console (eg. offb
> > driver as used on powermac systems).
> > 
> > In these cases try to ensure that we provide some console output by
> > enabling the first usable registered console, which we keep track of
> > with the of_fallback_console variable. Affected systems will enable
> > their console later than they did prior to commit 05fd007e4629
> > ("console: don't prefer first registered if DT specifies stdout-path")
> > but should otherwise produce the same output.
> > 
> > Tested in QEMU with a PowerPC pseries_defconfig kernel.
> 
> Hi Paul,
> 
> This does "work", as in it boots and I get a console. But the delay in
> getting output on the VGA is not workable. I get pretty much no output
> until the machine is booted entirely to userspace, meaning any crash
> prior to that will be undebuggable.
> 
> I also note Andreas reports it doesn't work at all on PowerMac.
> 
> Please send a revert and we can try again next cycle.
> 
> cheers

Hi Michael,

A revert was already submitted by Hans de Goede & is being discussed over 
here:

https://marc.info/?l=linux-kernel&m=147826151427455&w=2

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] console: use first console if stdout-path device doesn't appear
  2016-11-07  9:18                             ` Paul Burton
@ 2016-11-07 15:26                               ` Larry Finger
  2016-11-07 17:21                                 ` Paul Burton
  0 siblings, 1 reply; 32+ messages in thread
From: Larry Finger @ 2016-11-07 15:26 UTC (permalink / raw)
  To: Paul Burton, Michael Ellerman
  Cc: Sergey Senozhatsky, Andreas Schwab, Andrew Morton,
	Borislav Petkov, Petr Mladek, Tejun Heo, linux-kernel,
	linuxppc-dev

On 11/07/2016 03:18 AM, Paul Burton wrote:
> On Monday, 7 November 2016 19:27:32 GMT Michael Ellerman wrote:
>> Paul Burton <paul.burton@imgtec.com> writes:
>>> If a device tree specified a preferred device for kernel console output
>>> via the stdout-path or linux,stdout-path chosen node properties there's
>>> no guarantee that it will have specified a device for which we have a
>>> driver. It may also be the case that we do have a driver but it doesn't
>>> call of_console_check() to register as a preferred console (eg. offb
>>> driver as used on powermac systems).
>>>
>>> In these cases try to ensure that we provide some console output by
>>> enabling the first usable registered console, which we keep track of
>>> with the of_fallback_console variable. Affected systems will enable
>>> their console later than they did prior to commit 05fd007e4629
>>> ("console: don't prefer first registered if DT specifies stdout-path")
>>> but should otherwise produce the same output.
>>>
>>> Tested in QEMU with a PowerPC pseries_defconfig kernel.
>>
>> Hi Paul,
>>
>> This does "work", as in it boots and I get a console. But the delay in
>> getting output on the VGA is not workable. I get pretty much no output
>> until the machine is booted entirely to userspace, meaning any crash
>> prior to that will be undebuggable.
>>
>> I also note Andreas reports it doesn't work at all on PowerMac.
>>
>> Please send a revert and we can try again next cycle.
>>
>> cheers
>
> Hi Michael,
>
> A revert was already submitted by Hans de Goede & is being discussed over
> here:
>
> https://marc.info/?l=linux-kernel&m=147826151427455&w=2

I am a little surprised that I was not CCd on that thread. To reiterate, my 
PowerBook G4 with a PPC32 processor CRASHES on boot. That is a lot more serious 
than the console output disappearing.

As it seems unlikely that this regression will be fixed in the current cycle, I 
recommend that the reversion of commit 05fd007e4629 until a proper fix is available.

Larry

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] console: use first console if stdout-path device doesn't appear
  2016-11-07 15:26                               ` Larry Finger
@ 2016-11-07 17:21                                 ` Paul Burton
  2016-11-07 18:27                                   ` Larry Finger
  0 siblings, 1 reply; 32+ messages in thread
From: Paul Burton @ 2016-11-07 17:21 UTC (permalink / raw)
  To: Larry Finger
  Cc: Michael Ellerman, Sergey Senozhatsky, Andreas Schwab,
	Andrew Morton, Borislav Petkov, Petr Mladek, Tejun Heo,
	linux-kernel, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

Hi Larry,

On Monday, 7 November 2016 09:26:37 GMT Larry Finger wrote:
> > A revert was already submitted by Hans de Goede & is being discussed over
> > here:
> > 
> > https://marc.info/?l=linux-kernel&m=147826151427455&w=2
> 
> I am a little surprised that I was not CCd on that thread.

Hans started that thread without copying you just as you started your thread 
without copying Andreas, who reported issues first.

> To reiterate, my
> PowerBook G4 with a PPC32 processor CRASHES on boot. That is a lot more
> serious than the console output disappearing.

Crashes as in init dies due to lack of a console, right?

> As it seems unlikely that this regression will be fixed in the current
> cycle, I recommend that the reversion of commit 05fd007e4629 until a proper
> fix is available.

I agree that reverting is probably the best option for now, and have replied 
with that in the other thread too.

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3] console: use first console if stdout-path device doesn't appear
  2016-11-07 17:21                                 ` Paul Burton
@ 2016-11-07 18:27                                   ` Larry Finger
  2016-11-08 13:21                                     ` revert 05fd007e46296afb (was: [PATCH v3] console: use first console if stdout-path device doesn't appear) Sergey Senozhatsky
  0 siblings, 1 reply; 32+ messages in thread
From: Larry Finger @ 2016-11-07 18:27 UTC (permalink / raw)
  To: Paul Burton
  Cc: Michael Ellerman, Sergey Senozhatsky, Andreas Schwab,
	Andrew Morton, Borislav Petkov, Petr Mladek, Tejun Heo,
	linux-kernel, linuxppc-dev

On 11/07/2016 11:21 AM, Paul Burton wrote:
> Hi Larry,
>
> On Monday, 7 November 2016 09:26:37 GMT Larry Finger wrote:
>>> A revert was already submitted by Hans de Goede & is being discussed over
>>> here:
>>>
>>> https://marc.info/?l=linux-kernel&m=147826151427455&w=2
>>
>> I am a little surprised that I was not CCd on that thread.
>
> Hans started that thread without copying you just as you started your thread
> without copying Andreas, who reported issues first.

My searching had failed to find his report.

>> To reiterate, my
>> PowerBook G4 with a PPC32 processor CRASHES on boot. That is a lot more
>> serious than the console output disappearing.
>
> Crashes as in init dies due to lack of a console, right?

It gets a kernel panic because of an attempt to kill process 1 (init). It then 
waits 120 seconds and tries to reboot.

>> As it seems unlikely that this regression will be fixed in the current
>> cycle, I recommend that the reversion of commit 05fd007e4629 until a proper
>> fix is available.
>
> I agree that reverting is probably the best option for now, and have replied
> with that in the other thread too.

Good.

Larry

^ permalink raw reply	[flat|nested] 32+ messages in thread

* revert 05fd007e46296afb (was: [PATCH v3] console: use first console if stdout-path device doesn't appear)
  2016-11-07 18:27                                   ` Larry Finger
@ 2016-11-08 13:21                                     ` Sergey Senozhatsky
  0 siblings, 0 replies; 32+ messages in thread
From: Sergey Senozhatsky @ 2016-11-08 13:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Paul Burton, Michael Ellerman, Sergey Senozhatsky,
	Andreas Schwab, Andrew Morton, Borislav Petkov, Petr Mladek,
	Tejun Heo, linux-kernel, linuxppc-dev, Larry Finger

Hello Andrew,


(just in case)

please revert 'commit 05fd007e46296afb24 ("console: don't prefer first
registered if DT specifies stdout-path")'

	-ss

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2016-11-08 13:22 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09 12:50 [PATCH] console: Don't prefer first registered if DT specifies stdout-path Paul Burton
2016-08-09 14:12 ` kbuild test robot
2016-08-09 15:10 ` kbuild test robot
2016-08-09 15:19 ` [PATCH v2] " Paul Burton
2016-08-09 21:57   ` Andrew Morton
2016-10-16 18:07   ` Andreas Schwab
2016-10-17 10:33     ` Paul Burton
2016-10-17 17:39       ` Andreas Schwab
2016-10-18  9:18         ` [PATCH] console: use first console if stdout-path device doesn't appear Paul Burton
2016-10-18 18:58           ` Andreas Schwab
2016-10-30  9:46             ` Andreas Schwab
2016-10-31  5:28               ` Michael Ellerman
2016-10-31 12:14                 ` [PATCH v2] " Paul Burton
2016-10-31 15:50                   ` Paul Burton
2016-10-31 19:21                     ` Larry Finger
2016-10-31 23:09                     ` Sergey Senozhatsky
2016-10-31 23:31                       ` Larry Finger
2016-11-03 12:57                         ` [PATCH v3] " Paul Burton
2016-11-03 17:40                           ` Sergey Senozhatsky
2016-11-03 21:17                             ` Paul Burton
2016-11-04 15:44                               ` Sergey Senozhatsky
2016-11-04  8:05                           ` Andreas Schwab
     [not found]                           ` <8737j3n18r.fsf@concordia.ellerman.id.au>
2016-11-07  9:18                             ` Paul Burton
2016-11-07 15:26                               ` Larry Finger
2016-11-07 17:21                                 ` Paul Burton
2016-11-07 18:27                                   ` Larry Finger
2016-11-08 13:21                                     ` revert 05fd007e46296afb (was: [PATCH v3] console: use first console if stdout-path device doesn't appear) Sergey Senozhatsky
2016-11-03 13:04                         ` [PATCH v2] console: use first console if stdout-path device doesn't appear Paul Burton
2016-11-01  4:39                       ` Michael Ellerman
2016-10-31 15:58                   ` Larry Finger
2016-10-31 12:23                 ` [PATCH] " Paul Burton
2016-10-18  9:21         ` [PATCH v2] console: Don't prefer first registered if DT specifies stdout-path Paul Burton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).