diff for duplicates of <20171107130454.GD2652@pathway.suse.cz> diff --git a/a/1.txt b/N1/1.txt index a1ae00e..3891966 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -7,21 +7,21 @@ trivial. I am getting snowed under many other tasks as well. On Wed 2017-09-06 17:57:18, Eugeniy Paltsev wrote: > Hi Petr, > -> On Tue, 2017-09-05@16:54 +0200, Petr Mladek wrote: +> On Tue, 2017-09-05 at 16:54 +0200, Petr Mladek wrote: > > On Mon 2017-08-28 19:58:07, Eugeniy Paltsev wrote: > > > In the current implementation we take the first console that > > > registers if we didn't select one. -> > >? +> > > > > > But if we specify console via "stdout-path" property in device tree > > > we don't want first console that registers here to be selected. > > > Otherwise we may choose wrong console - for example if some console > > > is registered earlier than console is pointed in "stdout-path" > > > property because console pointed in "stdout-path" property can be add as > > > preferred quite late - when it's driver is probed. -> >? +> > > > register_console() is really twisted function. I would like to better > > understand your problems before we add yet another twist there. -> >? +> > > > Could you please be more specific about your problems? > > What was the output of "cat /proc/consoles" before and after the fix? > > What exactly started and stopped working? @@ -44,18 +44,18 @@ On Wed 2017-09-06 17:57:18, Eugeniy Paltsev wrote: > Device tree: > -------------->8-------- > chosen { -> ????bootargs = "" -> ????stdout-path = &serial_1; +> bootargs = "" +> stdout-path = &serial_1; > }; > -> serial_0: uart-0 at ... {} /* FAIL: serial_0 is used as console (ttyS0) as it is -> ?????????????????????????* probed earlier */ -> serial_1: uart-1 at ... {} +> serial_0: uart-0@... {} /* FAIL: serial_0 is used as console (ttyS0) as it is +> * probed earlier */ +> serial_1: uart-1@... {} > -------------->8-------- > > # cat /proc/consoles -> ttyS0????????????????-W- (EC???a)????4:64????/* FAIL: ttyS0 is used instead of? -> ??????????????????????????????????????????????* ttyS1 */ +> ttyS0 -W- (EC a) 4:64 /* FAIL: ttyS0 is used instead of +> * ttyS1 */ I guess that you know this. But let's be sure that we understand the problem the same way. @@ -119,7 +119,7 @@ might have been cleaner. > > After my patch-v2: > # cat /proc/consoles -> ttyS1????????????????-W- (EC p a)????4:67 +> ttyS1 -W- (EC p a) 4:67 > > ----------------------------------------------------------------------------------- @@ -127,7 +127,7 @@ might have been cleaner. > > Context: > We use early console. Serial console device (and early console device) specified -> via "stdout-path" property in device tree.? +> via "stdout-path" property in device tree. > Support for console on virtual terminal is enabled (CONFIG_VT_CONSOLE=y) > > In this case early boot messages will be printed twice - firstly by @@ -138,17 +138,17 @@ might have been cleaner. > Example: > -------------->8-------- > chosen { -> ????bootargs = "earlycon" -> ????stdout-path = &serial_3; +> bootargs = "earlycon" +> stdout-path = &serial_3; > }; > -> serial_3: uart-3 at ... {}? +> serial_3: uart-3@... {} > -------------->8-------- > > So output of serial console will be be like that: > -------------->8-------- > XXX - early boot messages, printed by bootconsole -> ????- FAIL: pause in boot messages printing +> - FAIL: pause in boot messages printing > XXX - FAIL: again early boot messages, printed by serial console > YYY - rest of boot messages, printed by serial console > -------------->8-------- @@ -156,7 +156,7 @@ might have been cleaner. > So the order of enabling/disabling consoles will be like that: > -------------->8-------- > bootconsole [uart0] enabled -> console [tty0] enabled??????????????/* As no console is select 'tty0' was taken */ +> console [tty0] enabled /* As no console is select 'tty0' was taken */ There is a special hack in param_setup_earlycon(). It allows to mention just "earlycon" in the device tree. The particular @@ -171,8 +171,8 @@ earlycon_init_is_deferred was set, and the deferred handling caused fallback to ttyS0. -> bootconsole [uart0] disabled????????/* As we have real (tty0) console we disable -> ?????????????????????????????????????* all bootconsoles */ +> bootconsole [uart0] disabled /* As we have real (tty0) console we disable +> * all bootconsoles */ This is one big and old problem of console registration code. @@ -190,9 +190,9 @@ A proper solution is to rework the console matching mechanism and allow to match early and real consoles. It is a lot of work. -> console [ttyS3] enabled?????????????/* We take ttyS3 but don't reset its? -> ?????????????????????????????????????* CON_PRINTBUFFER flag (as there is NO enabled -> ?????* bootconsoles) */ +> console [ttyS3] enabled /* We take ttyS3 but don't reset its +> * CON_PRINTBUFFER flag (as there is NO enabled +> * bootconsoles) */ The "sad" thing is that the race with early console helped to register the configured ttyS3 instead of ttyS0. @@ -201,8 +201,8 @@ to register the configured ttyS3 instead of ttyS0. > > > # cat /proc/consoles -> ttyS3????????????????-W- (EC p a)????4:67 -> tty0?????????????????-WU (E?????)????4:1 +> ttyS3 -W- (EC p a) 4:67 +> tty0 -WU (E ) 4:1 > > As you can see CON_PRINTBUFFER flag (p) set for ttyS3 - that is wrong. @@ -213,8 +213,8 @@ should not be duplicated. > After my patch-v2: > # cat /proc/consoles -> ttyS3????????????????-W- (EC???a)????4:67 -> tty0?????????????????-WU (E??p??)????4:1 +> ttyS3 -W- (EC a) 4:67 +> tty0 -WU (E p ) 4:1 I think that this is actually worse. You will miss many messages on the ttyS3 console. @@ -236,28 +236,28 @@ a shape. But it seems to be a long term task. > but in this case we will face with someone complaining about "tty0". > > So all comments and suggestions are more than welcome. -? + > > > We retain previous behavior for tty0 console (if "stdout-path" used) > > > as a special case: > > > tty0 will be registered even if it was specified neither > > > in "bootargs" nor in "stdout-path". > > > We had to retain this behavior because a lot of ARM boards (and some > > > powerpc) rely on it. -> >? +> > > > My main concern is the exception for "tty". Yes, it was regiression > > reported in the commit c6c7d83b9c9e6a8b3e ("Revert "console: don't > > prefer first registered if DT specifies stdout-path""). But is this > > the only possible regression? -> >? -> >? +> > +> > > > All this is about the fallback code that tries to enable all > > consoles until a real one with tty binding (newcon->device) > > is enabled. -> >? +> > > > v1 version of you patch disabled this fallback code when a console > > was defined by stdout-path in the device tree. This emulates > > defining the console by console= parameter on the command line. -> >? +> > > > It might make sense until some complains that a console is not > > longer automatically enabled while it was before. But wait. > > Someone already complained about "tty0". We can solve this @@ -267,13 +267,13 @@ a shape. But it seems to be a long term task. > > We might endup with so many exceptions that the fallback code > > will be always used. But then we are back in the square > > and have the original behavior before your patch. -> >? +> > > > Yes, I understand your concerns. > > But I also have another concern: If we decide to left current behavior untouched > (like after reverting patch 05fd007e4629) -> more and more boards and devices will use current broken stdout-path behavior in? +> more and more boards and devices will use current broken stdout-path behavior in > any form and in the results we will get the situation when we can't fix > stdout-path behavior at all - because every change will break something somewhere. diff --git a/a/content_digest b/N1/content_digest index 5abc321..0b0ca15 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -2,10 +2,16 @@ "ref\020170828165807.8408-3-Eugeniy.Paltsev@synopsys.com\0" "ref\020170905145405.GH8741@pathway.suse.cz\0" "ref\01504720637.30546.7.camel@synopsys.com\0" - "From\0pmladek@suse.com (Petr Mladek)\0" - "Subject\0[PATCH v2 2/2] console: don't select first registered console if stdout-path used\0" + "From\0Petr Mladek <pmladek@suse.com>\0" + "Subject\0Re: [PATCH v2 2/2] console: don't select first registered console if stdout-path used\0" "Date\0Tue, 7 Nov 2017 14:04:54 +0100\0" - "To\0linux-snps-arc@lists.infradead.org\0" + "To\0Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>\0" + "Cc\0hdegoede@redhat.com <hdegoede@redhat.com>" + paul.burton@imgtec.com <paul.burton@imgtec.com> + linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> + sergey.senozhatsky@gmail.com <sergey.senozhatsky@gmail.com> + rostedt@goodmis.org <rostedt@goodmis.org> + " linux-snps-arc@lists.infradead.org <linux-snps-arc@lists.infradead.org>\0" "\00:1\0" "b\0" "Hi Eigeniy,\n" @@ -17,21 +23,21 @@ "On Wed 2017-09-06 17:57:18, Eugeniy Paltsev wrote:\n" "> Hi Petr,\n" "> \n" - "> On Tue, 2017-09-05@16:54 +0200, Petr Mladek wrote:\n" + "> On Tue, 2017-09-05 at 16:54 +0200, Petr Mladek wrote:\n" "> > On Mon 2017-08-28 19:58:07, Eugeniy Paltsev wrote:\n" "> > > In the current implementation we take the first console that\n" "> > > registers if we didn't select one.\n" - "> > >?\n" + "> > >\302\240\n" "> > > But if we specify console via \"stdout-path\" property in device tree\n" "> > > we don't want first console that registers here to be selected.\n" "> > > Otherwise we may choose wrong console - for example if some console\n" "> > > is registered earlier than console is pointed in \"stdout-path\"\n" "> > > property because console pointed in \"stdout-path\" property can be add as\n" "> > > preferred quite late - when it's driver is probed.\n" - "> >?\n" + "> >\302\240\n" "> > register_console() is really twisted function. I would like to better\n" "> > understand your problems before we add yet another twist there.\n" - "> >?\n" + "> >\302\240\n" "> > Could you please be more specific about your problems?\n" "> > What was the output of \"cat /proc/consoles\" before and after the fix?\n" "> > What exactly started and stopped working?\n" @@ -54,18 +60,18 @@ "> Device tree:\n" "> -------------->8--------\n" "> chosen {\n" - "> ????bootargs = \"\"\n" - "> ????stdout-path = &serial_1;\n" + "> \302\240\302\240\302\240\302\240bootargs = \"\"\n" + "> \302\240\302\240\302\240\302\240stdout-path = &serial_1;\n" "> };\n" "> \n" - "> serial_0: uart-0 at ... {} /* FAIL: serial_0 is used as console (ttyS0) as it is\n" - "> ?????????????????????????* probed earlier */\n" - "> serial_1: uart-1 at ... {}\n" + "> serial_0: uart-0@... {} /* FAIL: serial_0 is used as console (ttyS0) as it is\n" + "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240* probed earlier */\n" + "> serial_1: uart-1@... {}\n" "> -------------->8--------\n" ">\n" "> # cat /proc/consoles\n" - "> ttyS0????????????????-W- (EC???a)????4:64????/* FAIL: ttyS0 is used instead of?\n" - "> ??????????????????????????????????????????????* ttyS1 */\n" + "> ttyS0\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240-W- (EC\302\240\302\240\302\240a)\302\240\302\240\302\240\302\2404:64\302\240\302\240\302\240\302\240/* FAIL: ttyS0 is used instead of\302\240\n" + "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240* ttyS1 */\n" "\n" "I guess that you know this. But let's be sure that we\n" "understand the problem the same way.\n" @@ -129,7 +135,7 @@ "> \n" "> After my patch-v2:\n" "> # cat /proc/consoles\n" - "> ttyS1????????????????-W- (EC p a)????4:67\n" + "> ttyS1\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240-W- (EC p a)\302\240\302\240\302\240\302\2404:67\n" "\n" "> \n" "> -----------------------------------------------------------------------------------\n" @@ -137,7 +143,7 @@ "> \n" "> Context:\n" "> We use early console. Serial console device (and early console device) specified\n" - "> via \"stdout-path\" property in device tree.?\n" + "> via \"stdout-path\" property in device tree.\302\240\n" "> Support for console on virtual terminal is enabled (CONFIG_VT_CONSOLE=y)\n" "> \n" "> In this case early boot messages will be printed twice - firstly by\n" @@ -148,17 +154,17 @@ "> Example:\n" "> -------------->8--------\n" "> chosen {\n" - "> ????bootargs = \"earlycon\"\n" - "> ????stdout-path = &serial_3;\n" + "> \302\240\302\240\302\240\302\240bootargs = \"earlycon\"\n" + "> \302\240\302\240\302\240\302\240stdout-path = &serial_3;\n" "> };\n" "> \n" - "> serial_3: uart-3 at ... {}?\n" + "> serial_3: uart-3@... {}\302\240\n" "> -------------->8--------\n" "> \n" "> So output of serial console will be be like that:\n" "> -------------->8--------\n" "> XXX - early boot messages, printed by bootconsole\n" - "> ????- FAIL: pause in boot messages printing\n" + "> \302\240\302\240\302\240\302\240- FAIL: pause in boot messages printing\n" "> XXX - FAIL: again early boot messages, printed by serial console\n" "> YYY - rest of boot messages, printed by serial console\n" "> -------------->8--------\n" @@ -166,7 +172,7 @@ "> So the order of enabling/disabling consoles will be like that:\n" "> -------------->8--------\n" "> bootconsole [uart0] enabled\n" - "> console [tty0] enabled??????????????/* As no console is select 'tty0' was taken */\n" + "> console [tty0] enabled\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240/* As no console is select 'tty0' was taken */\n" "\n" "There is a special hack in param_setup_earlycon(). It allows to\n" "mention just \"earlycon\" in the device tree. The particular\n" @@ -181,8 +187,8 @@ "caused fallback to ttyS0.\n" "\n" "\n" - "> bootconsole [uart0] disabled????????/* As we have real (tty0) console we disable\n" - "> ?????????????????????????????????????* all bootconsoles */\n" + "> bootconsole [uart0] disabled\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240/* As we have real (tty0) console we disable\n" + "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240* all bootconsoles */\n" "\n" "\n" "This is one big and old problem of console registration code.\n" @@ -200,9 +206,9 @@ "and allow to match early and real consoles. It is a lot of work.\n" "\n" "\n" - "> console [ttyS3] enabled?????????????/* We take ttyS3 but don't reset its?\n" - "> ?????????????????????????????????????* CON_PRINTBUFFER flag (as there is NO enabled\n" - "> \t\t\t\t?????* bootconsoles) */\n" + "> console [ttyS3] enabled\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240/* We take ttyS3 but don't reset its\302\240\n" + "> \302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240* CON_PRINTBUFFER flag (as there is NO enabled\n" + "> \t\t\t\t\302\240\302\240\302\240\302\240\302\240* bootconsoles) */\n" "\n" "The \"sad\" thing is that the race with early console helped\n" "to register the configured ttyS3 instead of ttyS0.\n" @@ -211,8 +217,8 @@ "> \n" "> \n" "> # cat /proc/consoles\n" - "> ttyS3????????????????-W- (EC p a)????4:67\n" - "> tty0?????????????????-WU (E?????)????4:1\n" + "> ttyS3\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240-W- (EC p a)\302\240\302\240\302\240\302\2404:67\n" + "> tty0\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240-WU (E\302\240\302\240\302\240\302\240\302\240)\302\240\302\240\302\240\302\2404:1\n" "> \n" "> As you can see CON_PRINTBUFFER flag (p) set for ttyS3 - that is wrong.\n" "\n" @@ -223,8 +229,8 @@ "\n" "> After my patch-v2:\n" "> # cat /proc/consoles\n" - "> ttyS3????????????????-W- (EC???a)????4:67\n" - "> tty0?????????????????-WU (E??p??)????4:1\n" + "> ttyS3\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240-W- (EC\302\240\302\240\302\240a)\302\240\302\240\302\240\302\2404:67\n" + "> tty0\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240\302\240-WU (E\302\240\302\240p\302\240\302\240)\302\240\302\240\302\240\302\2404:1\n" "\n" "I think that this is actually worse. You will miss many messages\n" "on the ttyS3 console.\n" @@ -246,28 +252,28 @@ "> but in this case we will face with someone complaining about \"tty0\".\n" "> \n" "> So all comments and suggestions are more than welcome.\n" - "?\n" + "\302\240\n" "> > > We retain previous behavior for tty0 console (if \"stdout-path\" used)\n" "> > > as a special case:\n" "> > > tty0 will be registered even if it was specified neither\n" "> > > in \"bootargs\" nor in \"stdout-path\".\n" "> > > We had to retain this behavior because a lot of ARM boards (and some\n" "> > > powerpc) rely on it.\n" - "> >?\n" + "> >\302\240\n" "> > My main concern is the exception for \"tty\". Yes, it was regiression\n" "> > reported in the commit c6c7d83b9c9e6a8b3e (\"Revert \"console: don't\n" "> > prefer first registered if DT specifies stdout-path\"\"). But is this\n" "> > the only possible regression?\n" - "> >?\n" - "> >?\n" + "> >\302\240\n" + "> >\302\240\n" "> > All this is about the fallback code that tries to enable all\n" "> > consoles until a real one with tty binding (newcon->device)\n" "> > is enabled.\n" - "> >?\n" + "> >\302\240\n" "> > v1 version of you patch disabled this fallback code when a console\n" "> > was defined by stdout-path in the device tree. This emulates\n" "> > defining the console by console= parameter on the command line.\n" - "> >?\n" + "> >\302\240\n" "> > It might make sense until some complains that a console is not\n" "> > longer automatically enabled while it was before. But wait.\n" "> > Someone already complained about \"tty0\". We can solve this\n" @@ -277,13 +283,13 @@ "> > We might endup with so many exceptions that the fallback code\n" "> > will be always used. But then we are back in the square\n" "> > and have the original behavior before your patch.\n" - "> >?\n" + "> >\302\240\n" "> \n" "> Yes, I understand your concerns.\n" "> \n" "> But I also have another concern: If we decide to left current behavior untouched\n" "> (like after reverting patch 05fd007e4629)\n" - "> more and more boards and devices will use current broken stdout-path behavior in?\n" + "> more and more boards and devices will use current broken stdout-path behavior in\302\240\n" "> any form and in the results we will get the situation when we can't fix\n" "> stdout-path behavior at all - because every change will break something somewhere.\n" "\n" @@ -325,4 +331,4 @@ "Best Regards,\n" Petr -8af43e6cafe6a472b4e327f4ca0d18a4a583b4f59d477e5bccca3d92d904fa29 +ec3a0047d0510e45c81b7d682906439567dabf6f6547a388f7fe6c04de73aa8a
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.