diff for duplicates of <1504720637.30546.7.camel@synopsys.com> diff --git a/a/1.txt b/N1/1.txt index 8eaac41..8132f8d 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,20 +1,20 @@ 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? @@ -37,18 +37,18 @@ Example: 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 */ This FAIL happens because we take the first registered console if we didn't select @@ -56,7 +56,7 @@ a console via "console=" option in bootargs. After my patch-v2: # cat /proc/consoles -ttyS1????????????????-W- (EC p a)????4:67 +ttyS1 -W- (EC p a) 4:67 ----------------------------------------------------------------------------------- @@ -64,7 +64,7 @@ Problem 2: printing early boot messages twice and pause in boot messages printin 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 @@ -75,17 +75,17 @@ mush earlier than 'real' serial console is enabled. 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-------- @@ -93,25 +93,25 @@ YYY - rest of boot messages, printed by serial console 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 */ -bootconsole [uart0] disabled????????/* As we have real (tty0) console we disable -?????????????????????????????????????* all bootconsoles */ -console [ttyS3] enabled?????????????/* We take ttyS3 but don't reset its? -?????????????????????????????????????* CON_PRINTBUFFER flag (as there is NO enabled - ?????* bootconsoles) */ +console [tty0] enabled /* As no console is select 'tty0' was taken */ +bootconsole [uart0] disabled /* As we have real (tty0) console we disable + * all bootconsoles */ +console [ttyS3] enabled /* We take ttyS3 but don't reset its + * CON_PRINTBUFFER flag (as there is NO enabled + * bootconsoles) */ -------------->8-------- # 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. 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 These are the problems I have faced but these are NOT THE ONLY POSSIBLE problems @@ -122,28 +122,28 @@ 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 @@ -153,13 +153,13 @@ So all comments and suggestions are more than welcome. > 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. @@ -168,8 +168,8 @@ stdout-path behavior at all - because every change will break something somewher > This is why I would like to know more info about your problem. > We need to decide if it is more important than a regression. > Or if it can be fixed another way. ->? +> > Best Regards, > Petr ---? -?Eugeniy Paltsev +-- + Eugeniy Paltsev diff --git a/a/content_digest b/N1/content_digest index 3bed1d5..5c0b24a 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,29 +1,35 @@ "ref\020170828165807.8408-1-Eugeniy.Paltsev@synopsys.com\0" "ref\020170828165807.8408-3-Eugeniy.Paltsev@synopsys.com\0" "ref\020170905145405.GH8741@pathway.suse.cz\0" - "From\0Eugeniy.Paltsev@synopsys.com (Eugeniy Paltsev)\0" - "Subject\0[PATCH v2 2/2] console: don't select first registered console if stdout-path used\0" + "From\0Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>\0" + "Subject\0Re: [PATCH v2 2/2] console: don't select first registered console if stdout-path used\0" "Date\0Wed, 6 Sep 2017 17:57:18 +0000\0" - "To\0linux-snps-arc@lists.infradead.org\0" + "To\0pmladek@suse.com <pmladek@suse.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 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" @@ -46,18 +52,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" "\n" "This FAIL happens because we take the first registered console if we didn't select\n" @@ -65,7 +71,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" @@ -73,7 +79,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" @@ -84,17 +90,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" @@ -102,25 +108,25 @@ "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" - "bootconsole [uart0] disabled????????/* As we have real (tty0) console we disable\n" - "?????????????????????????????????????* all bootconsoles */\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 [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" + "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" + "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" "-------------->8--------\n" "\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" "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" "\n" "These are the problems I have faced but these are NOT THE ONLY POSSIBLE problems\n" @@ -131,28 +137,28 @@ "\n" "So all comments and suggestions are more than welcome.\n" "\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" @@ -162,13 +168,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" @@ -177,10 +183,10 @@ "> This is why I would like to know more info about your problem.\n" "> We need to decide if it is more important than a regression.\n" "> Or if it can be fixed another way.\n" - ">?\n" + ">\302\240\n" "> Best Regards,\n" "> Petr\n" - "--?\n" - ?Eugeniy Paltsev + "--\302\240\n" + "\302\240Eugeniy Paltsev" -c7ef2c3322f8df6c53a93abd105fd4eee6e69be79c430317b0c46d5eba177c4f +c2dde89ec93b17a2dae4a0e9fa68d68ec2a3cb712e80c46107154e7e83bae160
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.