On Mon, Mar 9, 2015 at 1:27 PM, Peter Hurley <[email protected]> wrote:
> setup_earlycon() will now match and register the desired earlycon
> from the param string (as if 'earlycon=...' had been set on the
> command line). Use setup_earlycon() from existing arch call sites
> which start an earlycon directly.
>
Hi,
Looks like this patcheset cause regression:
when set grub console to 115200, and later kernel only have
console=uart8250,io,0x3f8
the kernel will revert baud rate to 9600 instead of keeping 115200.
in setup_earlycon: you say:
* Registers the earlycon console matching the earlycon specified
* in the param string @buf. Acceptable param strings are of the form
* <name>,io|mmio|mmio32,<addr>,<options>
* <name>,0x<addr>,<options>
* <name>,<options>
* <name>
*
* Only for the third form does the earlycon setup() method receive the
* <options> string in the 'options' parameter; all other forms set
* the parameter to NULL.
so that change the old behavior that we defined in
Documentation/kernel-parameters.txt
uart[8250],io,<addr>[,options]
uart[8250],mmio,<addr>[,options]
uart[8250],mmio32,<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address.
MMIO inter-register address stride is either 8-bit
(mmio) or 32-bit (mmio32).
The options are the same as for ttyS, above.
The old behavior: options is optional , and will use baud rate that is
set by bootloader.
Please fix the problem and restore to old behavior.
Thanks
Yinghai
Hi Yinghai,
On 04/01/2015 10:04 PM, Yinghai Lu wrote:
> On Mon, Mar 9, 2015 at 1:27 PM, Peter Hurley <[email protected]> wrote:
>> setup_earlycon() will now match and register the desired earlycon
>> from the param string (as if 'earlycon=...' had been set on the
>> command line). Use setup_earlycon() from existing arch call sites
>> which start an earlycon directly.
>>
>
> Hi,
>
> Looks like this patcheset cause regression:
> when set grub console to 115200, and later kernel only have
>
> console=uart8250,io,0x3f8
>
> the kernel will revert baud rate to 9600 instead of keeping 115200.
>
> in setup_earlycon: you say:
>
> * Registers the earlycon console matching the earlycon specified
> * in the param string @buf. Acceptable param strings are of the form
> * <name>,io|mmio|mmio32,<addr>,<options>
> * <name>,0x<addr>,<options>
> * <name>,<options>
> * <name>
> *
> * Only for the third form does the earlycon setup() method receive the
> * <options> string in the 'options' parameter; all other forms set
> * the parameter to NULL.
>
>
> so that change the old behavior that we defined in
> Documentation/kernel-parameters.txt
>
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> uart[8250],mmio32,<addr>[,options]
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address.
> MMIO inter-register address stride is either 8-bit
> (mmio) or 32-bit (mmio32).
> The options are the same as for ttyS, above.
^^^^^^^^^^^^
The documented behavior of console=ttyS options, to which your
quote refers, clearly states:
Default is "9600n8".
> The old behavior: options is optional , and will use baud rate that is
> set by bootloader.
so the previous behavior was actually at odds with the documentation.
> Please fix the problem and restore to old behavior.
Is this really necessary (or even desirable)?
I think it's a bad idea to have one console type (ttyS) initialize its
options to default settings, but yet allow another console type (uart) to
probe the existing state.
Also, this expectation is an impediment when adding support for other
8250-like designs that don't have the same 8250 divisor registers
(ie., _every_ new design). To properly support this requirement for
just the existing 8250 hardware will require special probe_baud()
functions for: dw_8250, intel byt, intel mid, omap_8250, exar 17v35 series,
omap 1510.
Is specifying the line speed on the command line really a burden?
Regards,
Peter Hurley
On Wed, Apr 1, 2015 at 8:22 PM, Peter Hurley <[email protected]> wrote:
> The documented behavior of console=ttyS options, to which your
> quote refers, clearly states:
>
> Default is "9600n8".
drivers/tty/serial/8250/8250_early.c:early_serial8250_setup
still have calling to probe_baud, but it is not triggered.
Here is root cause.
The gap between entries in earlycon_table cause
iteration fail to find next entry, so uart8250 handler is
not called proplerly.
attached patch fix the problem.
Thanks
Yinghai
Hi Yinghai,
On 04/02/2015 05:15 AM, Yinghai Lu wrote:
> On Wed, Apr 1, 2015 at 8:22 PM, Peter Hurley <[email protected]> wrote:
>> The documented behavior of console=ttyS options, to which your
>> quote refers, clearly states:
>>
>> Default is "9600n8".
>
> drivers/tty/serial/8250/8250_early.c:early_serial8250_setup
> still have calling to probe_baud, but it is not triggered.
>
> Here is root cause.
> The gap between entries in earlycon_table cause
> iteration fail to find next entry, so uart8250 handler is
> not called proplerly.
Thanks for finding that bug; so the earlycon never started, right?
> attached patch fix the problem.
Would you please try the patch below instead?
Regards,
Peter Hurley
--- >% ---
From: Peter Hurley <[email protected]>
Subject: [PATCH] earlycon: Fix __earlycon_table stride
Signed-off-by: Peter Hurley <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 2 +-
include/linux/serial_core.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 7b0ef49..2e11f31 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -151,7 +151,7 @@
#endif
#ifdef CONFIG_SERIAL_EARLYCON
-#define EARLYCON_TABLE() . = ALIGN(8); \
+#define EARLYCON_TABLE() STRUCT_ALIGN(); \
VMLINUX_SYMBOL(__earlycon_table) = .; \
*(__earlycon_table) \
*(__earlycon_table_end)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 34de168..025dad9 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -342,7 +342,7 @@ struct earlycon_device {
struct earlycon_id {
char name[16];
int (*setup)(struct earlycon_device *, const char *options);
-};
+} __aligned(32);
extern int setup_earlycon(char *buf);
extern int of_setup_earlycon(unsigned long addr,
--
2.3.5
On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <[email protected]> wrote:
> Would you please try the patch below instead?
>
> --- >% ---
> From: Peter Hurley <[email protected]>
> Subject: [PATCH] earlycon: Fix __earlycon_table stride
>
> Signed-off-by: Peter Hurley <[email protected]>
> ---
> include/asm-generic/vmlinux.lds.h | 2 +-
> include/linux/serial_core.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 7b0ef49..2e11f31 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -151,7 +151,7 @@
> #endif
>
> #ifdef CONFIG_SERIAL_EARLYCON
> -#define EARLYCON_TABLE() . = ALIGN(8); \
> +#define EARLYCON_TABLE() STRUCT_ALIGN(); \
> VMLINUX_SYMBOL(__earlycon_table) = .; \
> *(__earlycon_table) \
> *(__earlycon_table_end)
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 34de168..025dad9 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -342,7 +342,7 @@ struct earlycon_device {
> struct earlycon_id {
> char name[16];
> int (*setup)(struct earlycon_device *, const char *options);
> -};
> +} __aligned(32);
>
> extern int setup_earlycon(char *buf);
> extern int of_setup_earlycon(unsigned long addr,
> --
Great. that works, and less lines change than my version.
ffffffff832049a0 T __earlycon_table
ffffffff832049a0 t __earlycon_uart
ffffffff832049c0 t __earlycon_uart8250
ffffffff832049e0 t __earlycon_table_sentinel
[ 0.000000] bootconsole [uart0] enabled
[ 0.000000] uart8250 probed_baud_rate: 115200
[ 0.000000] size of earlycon_id: 0x20
Thanks
Yinghai
On Thu, Apr 2, 2015 at 10:23 AM, Yinghai Lu <[email protected]> wrote:
> On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <[email protected]> wrote:
>
>> Would you please try the patch below instead?
>>
> Great. that works, and less lines change than my version.
>
still have another problem.
when using console=uart8250,io,0x3f8
it works as earlycon at first.
but after handover to normal console
it will revert back to 9600 again.
On Thu, Apr 2, 2015 at 3:12 PM, Yinghai Lu <[email protected]> wrote:
> On Thu, Apr 2, 2015 at 10:23 AM, Yinghai Lu <[email protected]> wrote:
>> On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <[email protected]> wrote:
>>
>>> Would you please try the patch below instead?
>>>
>> Great. that works, and less lines change than my version.
>>
>
> still have another problem.
> when using console=uart8250,io,0x3f8
> it works as earlycon at first.
> but after handover to normal console
> it will revert back to 9600 again.
this regression should be caused by:
commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
Author: Peter Hurley <[email protected]>
Date: Mon Mar 9 16:27:12 2015 -0400
console: Add extensible console matching
On Thu, Apr 2, 2015 at 3:36 PM, Yinghai Lu <[email protected]> wrote:
> On Thu, Apr 2, 2015 at 3:12 PM, Yinghai Lu <[email protected]> wrote:
>> On Thu, Apr 2, 2015 at 10:23 AM, Yinghai Lu <[email protected]> wrote:
>>> On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <[email protected]> wrote:
>>>
>>>> Would you please try the patch below instead?
>>>>
>>> Great. that works, and less lines change than my version.
>>>
>>
>> still have another problem.
>> when using console=uart8250,io,0x3f8
>> it works as earlycon at first.
>> but after handover to normal console
>> it will revert back to 9600 again.
>
> this regression should be caused by:
>
> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
> Author: Peter Hurley <[email protected]>
> Date: Mon Mar 9 16:27:12 2015 -0400
>
> console: Add extensible console matching
current code will:
for earlycon, will probe baud rate, and save it back to device->options.
and later update the command line
uart to ttyS, update the options with baud rate.
And your change will not try to find the exact port to have baud rate.
and just use command.
So will have have that probed baud rate passed.
Please check if you can fix this regression.
Maybe you have to put back command line update code back.
Thanks
Yinghai
On Thu, Apr 2, 2015 at 5:02 PM, Yinghai Lu <[email protected]> wrote:
> On Thu, Apr 2, 2015 at 3:36 PM, Yinghai Lu <[email protected]> wrote:
>> On Thu, Apr 2, 2015 at 3:12 PM, Yinghai Lu <[email protected]> wrote:
>>> On Thu, Apr 2, 2015 at 10:23 AM, Yinghai Lu <[email protected]> wrote:
>>>> On Thu, Apr 2, 2015 at 9:31 AM, Peter Hurley <[email protected]> wrote:
>>>>
>>>>> Would you please try the patch below instead?
>>>>>
>>>> Great. that works, and less lines change than my version.
>>>>
>>>
>>> still have another problem.
>>> when using console=uart8250,io,0x3f8
>>> it works as earlycon at first.
>>> but after handover to normal console
>>> it will revert back to 9600 again.
>>
>> this regression should be caused by:
>>
>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>> Author: Peter Hurley <[email protected]>
>> Date: Mon Mar 9 16:27:12 2015 -0400
>>
>> console: Add extensible console matching
>
So your whole patchset will need the first patch ?
or can you just drop the first one patch ?
Thanks
Yinghai
On Thu, Apr 2, 2015 at 5:22 PM, Yinghai Lu <[email protected]> wrote:
>>>> still have another problem.
>>>> when using console=uart8250,io,0x3f8
>>>> it works as earlycon at first.
>>>> but after handover to normal console
>>>> it will revert back to 9600 again.
>>>
>>> this regression should be caused by:
>>>
>>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>>> Author: Peter Hurley <[email protected]>
>>> Date: Mon Mar 9 16:27:12 2015 -0400
>>>
>>> console: Add extensible console matching
>>
> So your whole patchset will need the first patch ?
>
> or can you just drop the first one patch ?
Please check attached patch that fix regresion by. commit c7cef0a849
("console: Add extensible console matching")
or you have better way?
Thanks
Yinghai
On 04/02/2015 10:38 PM, Yinghai Lu wrote:
> On Thu, Apr 2, 2015 at 5:22 PM, Yinghai Lu <[email protected]> wrote:
>
>>>>> still have another problem.
>>>>> when using console=uart8250,io,0x3f8
>>>>> it works as earlycon at first.
>>>>> but after handover to normal console
>>>>> it will revert back to 9600 again.
>>>>
>>>> this regression should be caused by:
>>>>
>>>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>>>> Author: Peter Hurley <[email protected]>
>>>> Date: Mon Mar 9 16:27:12 2015 -0400
>>>>
>>>> console: Add extensible console matching
>>>
>> So your whole patchset will need the first patch ?
>>
>> or can you just drop the first one patch ?
>
> Please check attached patch that fix regresion by. commit c7cef0a849
> ("console: Add extensible console matching")
>
> or you have better way?
Please re-read my first response to this problem.
Regards,
Peter Hurley
On Fri, Apr 3, 2015 at 3:37 AM, Peter Hurley <[email protected]> wrote:
> On 04/02/2015 10:38 PM, Yinghai Lu wrote:
>> On Thu, Apr 2, 2015 at 5:22 PM, Yinghai Lu <[email protected]> wrote:
>>
>>>>>> still have another problem.
>>>>>> when using console=uart8250,io,0x3f8
>>>>>> it works as earlycon at first.
>>>>>> but after handover to normal console
>>>>>> it will revert back to 9600 again.
>>>>>
>>>>> this regression should be caused by:
>>>>>
>>>>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>>>>> Author: Peter Hurley <[email protected]>
>>>>> Date: Mon Mar 9 16:27:12 2015 -0400
>>>>>
>>>>> console: Add extensible console matching
>>>>
>>> So your whole patchset will need the first patch ?
>>>
>>> or can you just drop the first one patch ?
>>
>> Please check attached patch that fix regresion by. commit c7cef0a849
>> ("console: Add extensible console matching")
>>
>> or you have better way?
>
> Please re-read my first response to this problem.
>
> Also, this expectation is an impediment when adding support for other
> 8250-like designs that don't have the same 8250 divisor registers
> (ie., _every_ new design). To properly support this requirement for
> just the existing 8250 hardware will require special probe_baud()
> functions for: dw_8250, intel byt, intel mid, omap_8250, exar 17v35 series,
> omap 1510.
But you can not break old setup!
old behavior: console=uart8250,io,0x3f8
when bootloader or firmware has 115200 used already,
earlycon, and console all keep 115200.
Now with your change, earlycon will use 115200 still,
but console will switch to 9600.
>
> Is specifying the line speed on the command line really a burden?
>
Yes. We have two setups, and two BIOS versions.
product version is using 9600 and devel/debug is using 115200.
This is a regression, and must be addressed.
Greg,
Please don't send merge request for tty-next to Linus before
this regression get resolved.
Thanks
Yinghai
On 04/03/2015 12:57 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 3:37 AM, Peter Hurley <[email protected]> wrote:
>> On 04/02/2015 10:38 PM, Yinghai Lu wrote:
>>> On Thu, Apr 2, 2015 at 5:22 PM, Yinghai Lu <[email protected]> wrote:
>>>
>>>>>>> still have another problem.
>>>>>>> when using console=uart8250,io,0x3f8
>>>>>>> it works as earlycon at first.
>>>>>>> but after handover to normal console
>>>>>>> it will revert back to 9600 again.
>>>>>>
>>>>>> this regression should be caused by:
>>>>>>
>>>>>> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
>>>>>> Author: Peter Hurley <[email protected]>
>>>>>> Date: Mon Mar 9 16:27:12 2015 -0400
>>>>>>
>>>>>> console: Add extensible console matching
>>>>>
>>>> So your whole patchset will need the first patch ?
>>>>
>>>> or can you just drop the first one patch ?
>>>
>>> Please check attached patch that fix regresion by. commit c7cef0a849
>>> ("console: Add extensible console matching")
>>>
>>> or you have better way?
>>
>> Please re-read my first response to this problem.
>>
>
>> Also, this expectation is an impediment when adding support for other
>> 8250-like designs that don't have the same 8250 divisor registers
>> (ie., _every_ new design). To properly support this requirement for
>> just the existing 8250 hardware will require special probe_baud()
>> functions for: dw_8250, intel byt, intel mid, omap_8250, exar 17v35 series,
>> omap 1510.
>
> But you can not break old setup!
>
> old behavior: console=uart8250,io,0x3f8
>
> when bootloader or firmware has 115200 used already,
> earlycon, and console all keep 115200.
>
> Now with your change, earlycon will use 115200 still,
> but console will switch to 9600.
>
>>
>> Is specifying the line speed on the command line really a burden?
>>
>
> Yes. We have two setups, and two BIOS versions.
> product version is using 9600 and devel/debug is using 115200.
Wait -- you have earlycon in a product??
> This is a regression, and must be addressed.
>
> Greg,
>
> Please don't send merge request for tty-next to Linus before
> this regression get resolved.
On Fri, Apr 3, 2015 at 10:38 AM, Peter Hurley <[email protected]> wrote:
> On 04/03/2015 12:57 PM, Yinghai Lu wrote:
>
> Wait -- you have earlycon in a product??
What do you mean?
when you are using "console=uart8250,io,0x3f8"
you will have earlycon at first, and handover to console.
On 04/03/2015 01:44 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 10:38 AM, Peter Hurley <[email protected]> wrote:
>> On 04/03/2015 12:57 PM, Yinghai Lu wrote:
>>
>> Wait -- you have earlycon in a product??
>
> What do you mean?
I mean, what will happen if I put in a big debug banner like Steven
did for ftrace?
Kernel developers need earlycon for debugging arch code; often the earlycon
is just hacked together especially when it requires fixmap support.
Putting it in a product and _relying on undocumented behavior_ is a bad idea.
> when you are using "console=uart8250,io,0x3f8"
> you will have earlycon at first, and handover to console.
which means you have to have CONFIG_SERIAL_EARLYCON to get a serial console
at all.
I have no problem fixing this _if this is causing actual regressions_,
and not simply some minor inconvenience that can be fixed by editing your
grub command line.
However, I see this undocumented behavior as a cautionary tale for why not
every kernel command line hack should be accepted in mainline.
Regards,
Peter Hurley
On Fri, Apr 03, 2015 at 02:27:30PM -0400, Peter Hurley wrote:
> I have no problem fixing this _if this is causing actual regressions_,
> and not simply some minor inconvenience that can be fixed by editing your
> grub command line.
But, you can't break working command lines from previous kernel
releases. Undocumented or not, sorry. It's a working configuration.
Upgrading a kernel can not break that.
sorry,
greg k-h
Restore undocumented behavior of kernel command line parameters of
the forms:
console=uart[####],io|mmio|mmio32,<addr>[,options]
console=uart[####],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.
Document the required behavior of the original implementation.
Reported-by: Yinghai Lu <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
Documentation/kernel-parameters.txt | 15 +++++++++++----
drivers/tty/serial/8250/8250_core.c | 15 ++++++++++++++-
drivers/tty/serial/8250/8250_early.c | 19 -------------------
3 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index bfcb1a6..59c9832 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -711,12 +711,19 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
Documentation/networking/netconsole.txt for an
alternative.
- uart[8250],io,<addr>[,options]
- uart[8250],mmio,<addr>[,options]
+ uart[<n>],io,<addr>[,options]
+ uart[<n>],mmio,<addr>[,options]
+ uart[<n>],mmio32,<addr>[,options]
+ uart[<n>],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address,
- switching to the matching ttyS device later. The
- options are the same as for ttyS, above.
+ switching to the matching ttyS device later.
+ Any optional number <n> is accepted and ignored.
+ If none of [io|mmio|mmio32], <addr> is assumed to be
+ equivalent to 'mmio'. 'options' are specified in the
+ same format described for ttyS above; if unspecified,
+ the h/w is not re-initialized.
+
hvc<n> Use the hypervisor console device <n>. This is for
both Xen and PowerPC hypervisors.
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e0fb5f0..5f41a9c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3456,12 +3456,17 @@ static int univ8250_console_setup(struct console *co, char *options)
*
* Only attempts to match console command lines of the form:
* console=uart<>,io|mmio|mmio32,<addr>,<options>
- * console=uart<>,<addr>,options
+ * console=uart<>,0x<addr>,<options>
* This form is used to register an initial earlycon boot console and
* replace it with the serial8250_console at 8250 driver init.
*
* Performs console setup for a match (as required by interface)
*
+ * ** HACK ALERT **
+ * If no <options> are specified, then assume the h/w is already setup.
+ * This was the undocumented behavior of the original implementation so
+ * it is cast in stone forever.
+ *
* Returns 0 if console matches; otherwise non-zero to use default matching
*/
static int univ8250_console_match(struct console *co, char *name, int idx,
@@ -3491,6 +3496,14 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
continue;
co->index = i;
+
+ /* if no line settings, then assume h/w was setup */
+ if (!options) {
+ /* link port to console */
+ port->cons = co;
+ return 0;
+ }
+
return univ8250_console_setup(co, options);
}
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index e95ebfe..8e11968 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
serial8250_early_out(port, UART_IER, ier);
}
-static unsigned int __init probe_baud(struct uart_port *port)
-{
- unsigned char lcr, dll, dlm;
- unsigned int quot;
-
- lcr = serial8250_early_in(port, UART_LCR);
- serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
- dll = serial8250_early_in(port, UART_DLL);
- dlm = serial8250_early_in(port, UART_DLM);
- serial8250_early_out(port, UART_LCR, lcr);
-
- quot = (dlm << 8) | dll;
- return (port->uartclk / 16) / quot;
-}
-
static void __init init_port(struct earlycon_device *device)
{
struct uart_port *port = &device->port;
@@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
struct uart_port *port = &device->port;
unsigned int ier;
- device->baud = probe_baud(&device->port);
- snprintf(device->options, sizeof(device->options), "%u",
- device->baud);
-
/* assume the device was initialized, only mask interrupts */
ier = serial8250_early_in(port, UART_IER);
serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
--
2.3.5
Restore undocumented behavior of kernel command line parameters of
the forms:
console=uart[8250],io|mmio|mmio32,<addr>[,options]
console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.
Document the required behavior of the original implementation.
Reported-by: Yinghai Lu <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
v2: Fixed regression which allowed "console=uart1337,..." to start a
console (but not an earlycon)
+ fixed earlycon= documentation related required behavior fixed by
this patch
Documentation/kernel-parameters.txt | 18 +++++++++++++++---
drivers/tty/serial/8250/8250_core.c | 20 ++++++++++++++++++--
drivers/tty/serial/8250/8250_early.c | 19 -------------------
3 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index bfcb1a6..1facf0b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
uart[8250],io,<addr>[,options]
uart[8250],mmio,<addr>[,options]
+ uart[8250],mmio32,<addr>[,options]
+ uart[8250],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address,
- switching to the matching ttyS device later. The
- options are the same as for ttyS, above.
+ switching to the matching ttyS device later.
+ MMIO inter-register address stride is either 8-bit
+ (mmio) or 32-bit (mmio32).
+ If none of [io|mmio|mmio32], <addr> is assumed to be
+ equivalent to 'mmio'. 'options' are specified in the
+ same format described for ttyS above; if unspecified,
+ the h/w is not re-initialized.
+
hvc<n> Use the hypervisor console device <n>. This is for
both Xen and PowerPC hypervisors.
@@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
uart[8250],io,<addr>[,options]
uart[8250],mmio,<addr>[,options]
uart[8250],mmio32,<addr>[,options]
+ uart[8250],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address.
MMIO inter-register address stride is either 8-bit
(mmio) or 32-bit (mmio32).
- The options are the same as for ttyS, above.
+ If none of [io|mmio|mmio32], <addr> is assumed to be
+ equivalent to 'mmio'. 'options' are specified in the
+ same format described for "console=ttyS<n>"; if
+ unspecified, the h/w is not initialized.
pl011,<addr>
Start an early, polled-mode console on a pl011 serial
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e0fb5f0..bda2a57 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3455,13 +3455,18 @@ static int univ8250_console_setup(struct console *co, char *options)
* @options: ptr to option string from console command line
*
* Only attempts to match console command lines of the form:
- * console=uart<>,io|mmio|mmio32,<addr>,<options>
- * console=uart<>,<addr>,options
+ * console=uart[8250],io|mmio|mmio32,<addr>[,<options>]
+ * console=uart[8250],0x<addr>[,<options>]
* This form is used to register an initial earlycon boot console and
* replace it with the serial8250_console at 8250 driver init.
*
* Performs console setup for a match (as required by interface)
*
+ * ** HACK ALERT **
+ * If no <options> are specified, then assume the h/w is already setup.
+ * This was the undocumented behavior of the original implementation so
+ * it is cast in stone forever.
+ *
* Returns 0 if console matches; otherwise non-zero to use default matching
*/
static int univ8250_console_match(struct console *co, char *name, int idx,
@@ -3475,6 +3480,9 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
if (strncmp(name, match, 4) != 0)
return -ENODEV;
+ if (idx && idx != 8250)
+ return -ENODEV;
+
if (uart_parse_earlycon(options, &iotype, &addr, &options))
return -ENODEV;
@@ -3491,6 +3499,14 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
continue;
co->index = i;
+
+ /* if no line settings, then assume h/w was setup */
+ if (!options) {
+ /* link port to console */
+ port->cons = co;
+ return 0;
+ }
+
return univ8250_console_setup(co, options);
}
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index e95ebfe..8e11968 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
serial8250_early_out(port, UART_IER, ier);
}
-static unsigned int __init probe_baud(struct uart_port *port)
-{
- unsigned char lcr, dll, dlm;
- unsigned int quot;
-
- lcr = serial8250_early_in(port, UART_LCR);
- serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
- dll = serial8250_early_in(port, UART_DLL);
- dlm = serial8250_early_in(port, UART_DLM);
- serial8250_early_out(port, UART_LCR, lcr);
-
- quot = (dlm << 8) | dll;
- return (port->uartclk / 16) / quot;
-}
-
static void __init init_port(struct earlycon_device *device)
{
struct uart_port *port = &device->port;
@@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
struct uart_port *port = &device->port;
unsigned int ier;
- device->baud = probe_baud(&device->port);
- snprintf(device->options, sizeof(device->options), "%u",
- device->baud);
-
/* assume the device was initialized, only mask interrupts */
ier = serial8250_early_in(port, UART_IER);
serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
--
2.3.5
On Fri, Apr 3, 2015 at 11:27 AM, Peter Hurley <[email protected]> wrote:
> On 04/03/2015 01:44 PM, Yinghai Lu wrote:
>> On Fri, Apr 3, 2015 at 10:38 AM, Peter Hurley <[email protected]> wrote:
>>> On 04/03/2015 12:57 PM, Yinghai Lu wrote:
>>>
>>> Wait -- you have earlycon in a product??
>>
>> What do you mean?
>
> I mean, what will happen if I put in a big debug banner like Steven
> did for ftrace?
>
> Kernel developers need earlycon for debugging arch code; often the earlycon
> is just hacked together especially when it requires fixmap support.
>
> Putting it in a product and _relying on undocumented behavior_ is a bad idea.
let me repeat again:
when you have "console=uart8250,io,0x3f8", you will have earlycon and
then console.
That is just for kernel developer for debugging.
When you have x86 with bunch of dimms and cpus, user will have to wait couple
of minutes to output from console=ttyS0....and if it hang early, no
one would know what happen.
Also it is only boot time only....
On Fri, Apr 3, 2015 at 11:27 AM, Peter Hurley <[email protected]> wrote:
> On 04/03/2015 01:44 PM, Yinghai Lu wrote:
>
> which means you have to have CONFIG_SERIAL_EARLYCON to get a serial console
> at all.
what do you mean? Now in drivers/tty/serial/8250/Kconfig we have
config SERIAL_8250_CONSOLE
bool "Console on 8250/16550 and compatible serial port"
depends on SERIAL_8250=y
select SERIAL_CORE_CONSOLE
select SERIAL_EARLYCON
do you mean, we should not have that "select SERIAL_EARLYCON" ?
On 04/03/2015 08:52 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 11:27 AM, Peter Hurley <[email protected]> wrote:
>> On 04/03/2015 01:44 PM, Yinghai Lu wrote:
>>> On Fri, Apr 3, 2015 at 10:38 AM, Peter Hurley <[email protected]> wrote:
>>>> On 04/03/2015 12:57 PM, Yinghai Lu wrote:
>>>>
>>>> Wait -- you have earlycon in a product??
>>>
>>> What do you mean?
>>
>> I mean, what will happen if I put in a big debug banner like Steven
>> did for ftrace?
>>
>> Kernel developers need earlycon for debugging arch code; often the earlycon
>> is just hacked together especially when it requires fixmap support.
>>
>> Putting it in a product and _relying on undocumented behavior_ is a bad idea.
>
> let me repeat again:
> when you have "console=uart8250,io,0x3f8", you will have earlycon and
> then console.
>
> That is just for kernel developer for debugging.
>
> When you have x86 with bunch of dimms and cpus, user will have to wait couple
> of minutes to output from console=ttyS0....and if it hang early, no
> one would know what happen.
>
> Also it is only boot time only....
The fix is sitting in your inbox. Pointless to discuss it now.
On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
> Restore undocumented behavior of kernel command line parameters of
> the forms:
> console=uart[8250],io|mmio|mmio32,<addr>[,options]
> console=uart[8250],<addr>[,options]
> where 'options' have not been specified; in this case, the hardware
> is assumed to be initialized.
Please separated bug fix and other documentation change to different patches.
also fix the patch title to make it clear and you need to mention which commit
cause the regression.
This patch fix regression for the hand over. Thanks.
Another regression.
when user have
console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
before your patchset:
port_0x3f8 is early console, and will be console later.
and port_0x2f8 is ignored, because only ONE early console is allowed.
and old console setup, only handle ttyS0.
after your patchset:
port_0x3f8 is early console, and will be console later.
port_0x2f8 will become default console, as new console with match method
treat all uart8250 as ttyS0.
Please fix that too.
On 04/03/2015 10:19 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
>> Restore undocumented behavior of kernel command line parameters of
>> the forms:
>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>> console=uart[8250],<addr>[,options]
>> where 'options' have not been specified; in this case, the hardware
>> is assumed to be initialized.
>
>
> Please separated bug fix and other documentation change to different patches.
No. The documentation reflects exactly what the bug fix is for.
> also fix the patch title to make it clear and you need to mention which commit
> cause the regression.
Please bisect and send bisect log so I know which commit broke
your setup.
> This patch fix regression for the hand over. Thanks.
>
> Another regression.
> when user have
> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
>
> before your patchset:
> port_0x3f8 is early console, and will be console later.
> and port_0x2f8 is ignored, because only ONE early console is allowed.
> and old console setup, only handle ttyS0.
>
> after your patchset:
> port_0x3f8 is early console, and will be console later.
> port_0x2f8 will become default console, as new console with match method
> treat all uart8250 as ttyS0.
>
> Please fix that too.
That's a new feature, not a regression.
On 04/03/2015 10:29 PM, Peter Hurley wrote:
> On 04/03/2015 10:19 PM, Yinghai Lu wrote:
>> Another regression.
>> when user have
>> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
>>
>> before your patchset:
>> port_0x3f8 is early console, and will be console later.
>> and port_0x2f8 is ignored, because only ONE early console is allowed.
>> and old console setup, only handle ttyS0.
>>
>> after your patchset:
>> port_0x3f8 is early console, and will be console later.
>> port_0x2f8 will become default console, as new console with match method
>> treat all uart8250 as ttyS0.
>>
>> Please fix that too.
>
> That's a new feature, not a regression.
So, in all seriousness, you actually have this setup and want me to
fix this?
IOW, to support more than 1 earlycon in the future will require
using a totally different command line parameter. Awesome.
On Fri, Apr 3, 2015 at 7:29 PM, Peter Hurley <[email protected]> wrote:
> On 04/03/2015 10:19 PM, Yinghai Lu wrote:
>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
>>> Restore undocumented behavior of kernel command line parameters of
>>> the forms:
>>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>> console=uart[8250],<addr>[,options]
>>> where 'options' have not been specified; in this case, the hardware
>>> is assumed to be initialized.
>>
>>
>> Please separated bug fix and other documentation change to different patches.
>
> No. The documentation reflects exactly what the bug fix is for.
what is
+ uart[<n>],mmio,<addr>[,options]
+ uart[<n>],mmio32,<addr>[,options]
>
>> also fix the patch title to make it clear and you need to mention which commit
>> cause the regression.
>
> Please bisect and send bisect log so I know which commit broke
> your setup.
I already gave your the patch "title". Here it is again:
commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
Author: Peter Hurley <[email protected]>
Date: Mon Mar 9 16:27:12 2015 -0400
console: Add extensible console matching
>
>> This patch fix regression for the hand over. Thanks.
>>
>> Another regression.
>> when user have
>> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
>>
>> before your patchset:
>> port_0x3f8 is early console, and will be console later.
>> and port_0x2f8 is ignored, because only ONE early console is allowed.
>> and old console setup, only handle ttyS0.
>>
>> after your patchset:
>> port_0x3f8 is early console, and will be console later.
>> port_0x2f8 will become default console, as new console with match method
>> treat all uart8250 as ttyS0.
>>
>> Please fix that too.
>
> That's a new feature, not a regression.
ok, think that more.
with this patch:
[PATCH] earlycon: 8250: Fix command line regression
When you have console=uart8250,io,0x3f8 console=uart,io,0x2f8
you will skip the setup to 0x2f8, as the options is null.
then if the bios or bootloader only setup baud rate for 0x3f8.
the user could get nothing from 0x2f8.
Do you need to call univ8250_console_setup(co, options)
to set the default baud rate ?
On Fri, Apr 3, 2015 at 7:50 PM, Peter Hurley <[email protected]> wrote:
> On 04/03/2015 10:29 PM, Peter Hurley wrote:
>> On 04/03/2015 10:19 PM, Yinghai Lu wrote:
>>> Another regression.
>>> when user have
>>> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
>>>
>>> before your patchset:
>>> port_0x3f8 is early console, and will be console later.
>>> and port_0x2f8 is ignored, because only ONE early console is allowed.
>>> and old console setup, only handle ttyS0.
>>>
>>> after your patchset:
>>> port_0x3f8 is early console, and will be console later.
>>> port_0x2f8 will become default console, as new console with match method
>>> treat all uart8250 as ttyS0.
>>>
>>> Please fix that too.
>>
>> That's a new feature, not a regression.
>
> So, in all seriousness, you actually have this setup and want me to
> fix this?
No.
Just add one count in the console match for it.
>
> IOW, to support more than 1 earlycon in the future will require
> using a totally different command line parameter. Awesome.
>
That is future, right? Show me patch.
Thanks
Yinghai
On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <[email protected]> wrote:
> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
>> Restore undocumented behavior of kernel command line parameters of
>> the forms:
>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>> console=uart[8250],<addr>[,options]
>> where 'options' have not been specified; in this case, the hardware
>> is assumed to be initialized.
>
>
> This patch fix regression for the hand over. Thanks.
Just now noticed, this patch only fix partial problem. Kernel console is ok.
But later initrd/init scripts revert back to 9600 again.
So the patch does not fix the regression from
commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
Author: Peter Hurley <[email protected]>
Date: Mon Mar 9 16:27:12 2015 -0400
console: Add extensible console matching
On 04/03/2015 10:56 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 7:29 PM, Peter Hurley <[email protected]> wrote:
>> On 04/03/2015 10:19 PM, Yinghai Lu wrote:
>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
>>>> Restore undocumented behavior of kernel command line parameters of
>>>> the forms:
>>>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>> console=uart[8250],<addr>[,options]
>>>> where 'options' have not been specified; in this case, the hardware
>>>> is assumed to be initialized.
>>>
>>>
>>> Please separated bug fix and other documentation change to different patches.
>>
>> No. The documentation reflects exactly what the bug fix is for.
>
> what is
>
> + uart[<n>],mmio,<addr>[,options]
> + uart[<n>],mmio32,<addr>[,options]
Wrong patch version; that's v1.
>>> also fix the patch title to make it clear and you need to mention which commit
>>> cause the regression.
>>
>> Please bisect and send bisect log so I know which commit broke
>> your setup.
>
> I already gave your the patch "title". Here it is again:
>
> commit c7cef0a84912cab3c9df8949b034e4aa62982ec9
> Author: Peter Hurley <[email protected]>
> Date: Mon Mar 9 16:27:12 2015 -0400
>
> console: Add extensible console matching
Ok.
>>> This patch fix regression for the hand over. Thanks.
>>>
>>> Another regression.
>>> when user have
>>> console=uart8250,io,0x3f8 console=uart8250,io,0x2f8
>>>
>>> before your patchset:
>>> port_0x3f8 is early console, and will be console later.
>>> and port_0x2f8 is ignored, because only ONE early console is allowed.
>>> and old console setup, only handle ttyS0.
>>>
>>> after your patchset:
>>> port_0x3f8 is early console, and will be console later.
>>> port_0x2f8 will become default console, as new console with match method
>>> treat all uart8250 as ttyS0.
>>>
>>> Please fix that too.
>>
>> That's a new feature, not a regression.
>
> ok, think that more.
>
> with this patch:
>
> [PATCH] earlycon: 8250: Fix command line regression
>
> When you have console=uart8250,io,0x3f8 console=uart,io,0x2f8
>
> you will skip the setup to 0x2f8, as the options is null.
> then if the bios or bootloader only setup baud rate for 0x3f8.
> the user could get nothing from 0x2f8.
>
> Do you need to call univ8250_console_setup(co, options)
> to set the default baud rate ?
No. The user specified that the console had already been initialized
by leaving out the option string. Isn't that what this whole
email thread has been about?
All this automatic behavior and aliases for the same things just
adds unnecessary complexity and maintenance burden, without measurable
gain.
The real tragedy is that all this was totally unnecessary; what was
wrong with "earlycon=uart,io,0x3f8 console=ttyS0"?
Regards,
Peter Hurley
On 04/03/2015 11:09 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <[email protected]> wrote:
>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
>>> Restore undocumented behavior of kernel command line parameters of
>>> the forms:
>>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>> console=uart[8250],<addr>[,options]
>>> where 'options' have not been specified; in this case, the hardware
>>> is assumed to be initialized.
>>
>>
>> This patch fix regression for the hand over. Thanks.
>
> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>
> But later initrd/init scripts revert back to 9600 again.
Please share the init scripts.
On Fri, Apr 3, 2015 at 8:15 PM, Peter Hurley <[email protected]> wrote:
> On 04/03/2015 11:09 PM, Yinghai Lu wrote:
>> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <[email protected]> wrote:
>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
>>>> Restore undocumented behavior of kernel command line parameters of
>>>> the forms:
>>>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>> console=uart[8250],<addr>[,options]
>>>> where 'options' have not been specified; in this case, the hardware
>>>> is assumed to be initialized.
>>>
>>>
>>> This patch fix regression for the hand over. Thanks.
>>
>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>
>> But later initrd/init scripts revert back to 9600 again.
>
> Please share the init scripts.
I am using opensuse 13.1 rescue disk as ramdisk.
On Fri, Apr 3, 2015 at 8:09 PM, Peter Hurley <[email protected]> wrote:
>
> All this automatic behavior and aliases for the same things just
> adds unnecessary complexity and maintenance burden, without measurable
> gain.
>
> The real tragedy is that all this was totally unnecessary; what was
> wrong with "earlycon=uart,io,0x3f8 console=ttyS0"?
When I worked on the patch
| commit 18a8bd949d6adb311ea816125ff65050df1f3f6e
| Date: Sun Jul 15 23:37:59 2007 -0700
|
| serial: convert early_uart to earlycon for 8250
Andrew requested that.
Thanks
Yinghai
On Fri, Apr 3, 2015 at 8:24 PM, Yinghai Lu <[email protected]> wrote:
> On Fri, Apr 3, 2015 at 8:15 PM, Peter Hurley <[email protected]> wrote:
>> On 04/03/2015 11:09 PM, Yinghai Lu wrote:
>>> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <[email protected]> wrote:
>>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
>>>>> Restore undocumented behavior of kernel command line parameters of
>>>>> the forms:
>>>>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>>> console=uart[8250],<addr>[,options]
>>>>> where 'options' have not been specified; in this case, the hardware
>>>>> is assumed to be initialized.
>>>>
>>>>
>>>> This patch fix regression for the hand over. Thanks.
>>>
>>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>>
>>> But later initrd/init scripts revert back to 9600 again.
>>
>> Please share the init scripts.
>
> I am using opensuse 13.1 rescue disk as ramdisk.
or opensuse 12.3 rescue disk.
On 04/03/2015 11:24 PM, Yinghai Lu wrote:
> On Fri, Apr 3, 2015 at 8:15 PM, Peter Hurley <[email protected]> wrote:
>> On 04/03/2015 11:09 PM, Yinghai Lu wrote:
>>> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <[email protected]> wrote:
>>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
>>>>> Restore undocumented behavior of kernel command line parameters of
>>>>> the forms:
>>>>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>>> console=uart[8250],<addr>[,options]
>>>>> where 'options' have not been specified; in this case, the hardware
>>>>> is assumed to be initialized.
>>>>
>>>>
>>>> This patch fix regression for the hand over. Thanks.
>>>
>>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>>
>>> But later initrd/init scripts revert back to 9600 again.
>>
>> Please share the init scripts.
>
> I am using opensuse 13.1 rescue disk as ramdisk.
NVM, I know what the problem is.
On Fri, Apr 3, 2015 at 8:32 PM, Peter Hurley <[email protected]> wrote:
>>>>
>>>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>>>
>>>> But later initrd/init scripts revert back to 9600 again.
>>>
>>> Please share the init scripts.
>>
>> I am using opensuse 13.1 rescue disk as ramdisk.
>
> NVM, I know what the problem is.
>
What is NVM here?
On 04/03/2015 11:37 PM, Yinghai Lu wrote:
> What is NVM here?
nvm == nevermind
On Fri, Apr 3, 2015 at 8:32 PM, Peter Hurley <[email protected]> wrote:
> On 04/03/2015 11:24 PM, Yinghai Lu wrote:
>> On Fri, Apr 3, 2015 at 8:15 PM, Peter Hurley <[email protected]> wrote:
>>> On 04/03/2015 11:09 PM, Yinghai Lu wrote:
>>>> On Fri, Apr 3, 2015 at 7:19 PM, Yinghai Lu <[email protected]> wrote:
>>>>> On Fri, Apr 3, 2015 at 5:04 PM, Peter Hurley <[email protected]> wrote:
>>>>>> Restore undocumented behavior of kernel command line parameters of
>>>>>> the forms:
>>>>>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>>>>>> console=uart[8250],<addr>[,options]
>>>>>> where 'options' have not been specified; in this case, the hardware
>>>>>> is assumed to be initialized.
>>>>>
>>>>>
>>>>> This patch fix regression for the hand over. Thanks.
>>>>
>>>> Just now noticed, this patch only fix partial problem. Kernel console is ok.
>>>>
>>>> But later initrd/init scripts revert back to 9600 again.
>>>
I updated my fix a little bit. please check attached.
Thanks
Yinghai
Restore undocumented behavior of kernel command line parameters of
the forms:
console=uart[8250],io|mmio|mmio32,<addr>[,options]
console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.
Document the required behavior of the original implementation.
Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
Reported-by: Yinghai Lu <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
v3: Fixed automatic console to port line settings initialization;
open-coded serial8250_console_setup() so the baud can be probed;
added sha reference in commit log
v2: Fixed regression which allowed "console=uart1337,..." to start a
console (but not an earlycon)
+ fixed earlycon= documentation related required behavior fixed by
this patch
Documentation/kernel-parameters.txt | 18 ++++++++++++++---
drivers/tty/serial/8250/8250_core.c | 38 +++++++++++++++++++++++++++++++++---
drivers/tty/serial/8250/8250_early.c | 19 ------------------
3 files changed, 50 insertions(+), 25 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index bfcb1a6..1facf0b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
uart[8250],io,<addr>[,options]
uart[8250],mmio,<addr>[,options]
+ uart[8250],mmio32,<addr>[,options]
+ uart[8250],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address,
- switching to the matching ttyS device later. The
- options are the same as for ttyS, above.
+ switching to the matching ttyS device later.
+ MMIO inter-register address stride is either 8-bit
+ (mmio) or 32-bit (mmio32).
+ If none of [io|mmio|mmio32], <addr> is assumed to be
+ equivalent to 'mmio'. 'options' are specified in the
+ same format described for ttyS above; if unspecified,
+ the h/w is not re-initialized.
+
hvc<n> Use the hypervisor console device <n>. This is for
both Xen and PowerPC hypervisors.
@@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
uart[8250],io,<addr>[,options]
uart[8250],mmio,<addr>[,options]
uart[8250],mmio32,<addr>[,options]
+ uart[8250],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address.
MMIO inter-register address stride is either 8-bit
(mmio) or 32-bit (mmio32).
- The options are the same as for ttyS, above.
+ If none of [io|mmio|mmio32], <addr> is assumed to be
+ equivalent to 'mmio'. 'options' are specified in the
+ same format described for "console=ttyS<n>"; if
+ unspecified, the h/w is not initialized.
pl011,<addr>
Start an early, polled-mode console on a pl011 serial
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e0fb5f0..f59c7a0 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3447,6 +3447,22 @@ static int univ8250_console_setup(struct console *co, char *options)
return serial8250_console_setup(up, options);
}
+/* FIXME: this is broken on most other 8250 h/w */
+static unsigned int probe_baud(struct uart_port *port)
+{
+ unsigned char lcr, dll, dlm;
+ unsigned int quot;
+
+ lcr = serial_port_in(port, UART_LCR);
+ serial_port_out(port, UART_LCR, lcr | UART_LCR_DLAB);
+ dll = serial_port_in(port, UART_DLL);
+ dlm = serial_port_in(port, UART_DLM);
+ serial_port_out(port, UART_LCR, lcr);
+
+ quot = (dlm << 8) | dll;
+ return (port->uartclk / 16) / quot;
+}
+
/**
* univ8250_console_match - non-standard console matching
* @co: registering console
@@ -3455,13 +3471,18 @@ static int univ8250_console_setup(struct console *co, char *options)
* @options: ptr to option string from console command line
*
* Only attempts to match console command lines of the form:
- * console=uart<>,io|mmio|mmio32,<addr>,<options>
- * console=uart<>,<addr>,options
+ * console=uart[8250],io|mmio|mmio32,<addr>[,<options>]
+ * console=uart[8250],0x<addr>[,<options>]
* This form is used to register an initial earlycon boot console and
* replace it with the serial8250_console at 8250 driver init.
*
* Performs console setup for a match (as required by interface)
*
+ * ** HACK ALERT **
+ * If no <options> are specified, then assume the h/w is already setup.
+ * This was the undocumented behavior of the original implementation so
+ * it is cast in stone forever.
+ *
* Returns 0 if console matches; otherwise non-zero to use default matching
*/
static int univ8250_console_match(struct console *co, char *name, int idx,
@@ -3475,12 +3496,16 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
if (strncmp(name, match, 4) != 0)
return -ENODEV;
+ if (idx && idx != 8250)
+ return -ENODEV;
+
if (uart_parse_earlycon(options, &iotype, &addr, &options))
return -ENODEV;
/* try to match the port specified on the command line */
for (i = 0; i < nr_uarts; i++) {
struct uart_port *port = &serial8250_ports[i].port;
+ int baud, bits = 8, parity = 'n', flow = 'n';
if (port->iotype != iotype)
continue;
@@ -3490,8 +3515,15 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
if (iotype == UPIO_PORT && port->iobase != addr)
continue;
+ /* link port to console */
co->index = i;
- return univ8250_console_setup(co, options);
+ port->cons = co;
+
+ if (options && *options)
+ uart_parse_options(options, &baud, &parity, &bits, &flow);
+ else
+ baud = probe_baud(port);
+ return uart_set_options(port, port->cons, baud, parity, bits, flow);
}
return -ENODEV;
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index e95ebfe..8e11968 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
serial8250_early_out(port, UART_IER, ier);
}
-static unsigned int __init probe_baud(struct uart_port *port)
-{
- unsigned char lcr, dll, dlm;
- unsigned int quot;
-
- lcr = serial8250_early_in(port, UART_LCR);
- serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
- dll = serial8250_early_in(port, UART_DLL);
- dlm = serial8250_early_in(port, UART_DLM);
- serial8250_early_out(port, UART_LCR, lcr);
-
- quot = (dlm << 8) | dll;
- return (port->uartclk / 16) / quot;
-}
-
static void __init init_port(struct earlycon_device *device)
{
struct uart_port *port = &device->port;
@@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
struct uart_port *port = &device->port;
unsigned int ier;
- device->baud = probe_baud(&device->port);
- snprintf(device->options, sizeof(device->options), "%u",
- device->baud);
-
/* assume the device was initialized, only mask interrupts */
ier = serial8250_early_in(port, UART_IER);
serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
--
2.3.5
On Sat, Apr 04, 2015 at 10:27:30AM -0400, Peter Hurley wrote:
> Restore undocumented behavior of kernel command line parameters of
> the forms:
> console=uart[8250],io|mmio|mmio32,<addr>[,options]
> console=uart[8250],<addr>[,options]
> where 'options' have not been specified; in this case, the hardware
> is assumed to be initialized.
>
> Document the required behavior of the original implementation.
>
> Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
> Reported-by: Yinghai Lu <[email protected]>
> Signed-off-by: Peter Hurley <[email protected]>
> ---
>
> v3: Fixed automatic console to port line settings initialization;
> open-coded serial8250_console_setup() so the baud can be probed;
> added sha reference in commit log
>
> v2: Fixed regression which allowed "console=uart1337,..." to start a
> console (but not an earlycon)
> + fixed earlycon= documentation related required behavior fixed by
> this patch
>
> Documentation/kernel-parameters.txt | 18 ++++++++++++++---
> drivers/tty/serial/8250/8250_core.c | 38 +++++++++++++++++++++++++++++++++---
> drivers/tty/serial/8250/8250_early.c | 19 ------------------
> 3 files changed, 50 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index bfcb1a6..1facf0b 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> + uart[8250],mmio32,<addr>[,options]
> + uart[8250],0x<addr>[,options]
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address,
> - switching to the matching ttyS device later. The
> - options are the same as for ttyS, above.
> + switching to the matching ttyS device later.
> + MMIO inter-register address stride is either 8-bit
> + (mmio) or 32-bit (mmio32).
> + If none of [io|mmio|mmio32], <addr> is assumed to be
> + equivalent to 'mmio'. 'options' are specified in the
> + same format described for ttyS above; if unspecified,
> + the h/w is not re-initialized.
> +
> hvc<n> Use the hypervisor console device <n>. This is for
> both Xen and PowerPC hypervisors.
>
> @@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> uart[8250],mmio32,<addr>[,options]
> + uart[8250],0x<addr>[,options]
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address.
> MMIO inter-register address stride is either 8-bit
> (mmio) or 32-bit (mmio32).
> - The options are the same as for ttyS, above.
> + If none of [io|mmio|mmio32], <addr> is assumed to be
> + equivalent to 'mmio'. 'options' are specified in the
> + same format described for "console=ttyS<n>"; if
> + unspecified, the h/w is not initialized.
>
> pl011,<addr>
> Start an early, polled-mode console on a pl011 serial
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index e0fb5f0..f59c7a0 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -3447,6 +3447,22 @@ static int univ8250_console_setup(struct console *co, char *options)
> return serial8250_console_setup(up, options);
> }
>
> +/* FIXME: this is broken on most other 8250 h/w */
What do you mean by "most other"? What hardware does this work for?
What is it broken for? What is someone supposed to think/do with this
comment?
thanks,
greg k-h
On 04/04/2015 12:09 PM, Greg Kroah-Hartman wrote:
> On Sat, Apr 04, 2015 at 10:27:30AM -0400, Peter Hurley wrote:
>> Restore undocumented behavior of kernel command line parameters of
>> the forms:
>> console=uart[8250],io|mmio|mmio32,<addr>[,options]
>> console=uart[8250],<addr>[,options]
>> where 'options' have not been specified; in this case, the hardware
>> is assumed to be initialized.
>>
>> Document the required behavior of the original implementation.
>>
>> Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
>> Reported-by: Yinghai Lu <[email protected]>
>> Signed-off-by: Peter Hurley <[email protected]>
>> ---
>>
>> v3: Fixed automatic console to port line settings initialization;
>> open-coded serial8250_console_setup() so the baud can be probed;
>> added sha reference in commit log
>>
>> v2: Fixed regression which allowed "console=uart1337,..." to start a
>> console (but not an earlycon)
>> + fixed earlycon= documentation related required behavior fixed by
>> this patch
>>
>> Documentation/kernel-parameters.txt | 18 ++++++++++++++---
>> drivers/tty/serial/8250/8250_core.c | 38 +++++++++++++++++++++++++++++++++---
>> drivers/tty/serial/8250/8250_early.c | 19 ------------------
>> 3 files changed, 50 insertions(+), 25 deletions(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index bfcb1a6..1facf0b 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>
>> uart[8250],io,<addr>[,options]
>> uart[8250],mmio,<addr>[,options]
>> + uart[8250],mmio32,<addr>[,options]
>> + uart[8250],0x<addr>[,options]
>> Start an early, polled-mode console on the 8250/16550
>> UART at the specified I/O port or MMIO address,
>> - switching to the matching ttyS device later. The
>> - options are the same as for ttyS, above.
>> + switching to the matching ttyS device later.
>> + MMIO inter-register address stride is either 8-bit
>> + (mmio) or 32-bit (mmio32).
>> + If none of [io|mmio|mmio32], <addr> is assumed to be
>> + equivalent to 'mmio'. 'options' are specified in the
>> + same format described for ttyS above; if unspecified,
>> + the h/w is not re-initialized.
>> +
>> hvc<n> Use the hypervisor console device <n>. This is for
>> both Xen and PowerPC hypervisors.
>>
>> @@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>> uart[8250],io,<addr>[,options]
>> uart[8250],mmio,<addr>[,options]
>> uart[8250],mmio32,<addr>[,options]
>> + uart[8250],0x<addr>[,options]
>> Start an early, polled-mode console on the 8250/16550
>> UART at the specified I/O port or MMIO address.
>> MMIO inter-register address stride is either 8-bit
>> (mmio) or 32-bit (mmio32).
>> - The options are the same as for ttyS, above.
>> + If none of [io|mmio|mmio32], <addr> is assumed to be
>> + equivalent to 'mmio'. 'options' are specified in the
>> + same format described for "console=ttyS<n>"; if
>> + unspecified, the h/w is not initialized.
>>
>> pl011,<addr>
>> Start an early, polled-mode console on a pl011 serial
>> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>> index e0fb5f0..f59c7a0 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -3447,6 +3447,22 @@ static int univ8250_console_setup(struct console *co, char *options)
>> return serial8250_console_setup(up, options);
>> }
>>
>> +/* FIXME: this is broken on most other 8250 h/w */
>
> What do you mean by "most other"? What hardware does this work for?
> What is it broken for? What is someone supposed to think/do with this
> comment?
It's a direct copy of the existing behavior for 8250 earlycon, which is
broken for h/w with non-standard divisor registers. That's basically
_every_ new design, because that's what vendors need to extend to support
modern bitrates.
Affected h/w that I know of includes: 8250_dw, exar xr17v35 cards, omap8250,
omap15xx, intel byt, intel mid.
But it was already broken for these and so not a regression.
Regards,
Peter Hurley
On Sat, Apr 04, 2015 at 12:23:14PM -0400, Peter Hurley wrote:
> On 04/04/2015 12:09 PM, Greg Kroah-Hartman wrote:
> > On Sat, Apr 04, 2015 at 10:27:30AM -0400, Peter Hurley wrote:
> >> Restore undocumented behavior of kernel command line parameters of
> >> the forms:
> >> console=uart[8250],io|mmio|mmio32,<addr>[,options]
> >> console=uart[8250],<addr>[,options]
> >> where 'options' have not been specified; in this case, the hardware
> >> is assumed to be initialized.
> >>
> >> Document the required behavior of the original implementation.
> >>
> >> Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
> >> Reported-by: Yinghai Lu <[email protected]>
> >> Signed-off-by: Peter Hurley <[email protected]>
> >> ---
> >>
> >> v3: Fixed automatic console to port line settings initialization;
> >> open-coded serial8250_console_setup() so the baud can be probed;
> >> added sha reference in commit log
> >>
> >> v2: Fixed regression which allowed "console=uart1337,..." to start a
> >> console (but not an earlycon)
> >> + fixed earlycon= documentation related required behavior fixed by
> >> this patch
> >>
> >> Documentation/kernel-parameters.txt | 18 ++++++++++++++---
> >> drivers/tty/serial/8250/8250_core.c | 38 +++++++++++++++++++++++++++++++++---
> >> drivers/tty/serial/8250/8250_early.c | 19 ------------------
> >> 3 files changed, 50 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> >> index bfcb1a6..1facf0b 100644
> >> --- a/Documentation/kernel-parameters.txt
> >> +++ b/Documentation/kernel-parameters.txt
> >> @@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> >>
> >> uart[8250],io,<addr>[,options]
> >> uart[8250],mmio,<addr>[,options]
> >> + uart[8250],mmio32,<addr>[,options]
> >> + uart[8250],0x<addr>[,options]
> >> Start an early, polled-mode console on the 8250/16550
> >> UART at the specified I/O port or MMIO address,
> >> - switching to the matching ttyS device later. The
> >> - options are the same as for ttyS, above.
> >> + switching to the matching ttyS device later.
> >> + MMIO inter-register address stride is either 8-bit
> >> + (mmio) or 32-bit (mmio32).
> >> + If none of [io|mmio|mmio32], <addr> is assumed to be
> >> + equivalent to 'mmio'. 'options' are specified in the
> >> + same format described for ttyS above; if unspecified,
> >> + the h/w is not re-initialized.
> >> +
> >> hvc<n> Use the hypervisor console device <n>. This is for
> >> both Xen and PowerPC hypervisors.
> >>
> >> @@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> >> uart[8250],io,<addr>[,options]
> >> uart[8250],mmio,<addr>[,options]
> >> uart[8250],mmio32,<addr>[,options]
> >> + uart[8250],0x<addr>[,options]
> >> Start an early, polled-mode console on the 8250/16550
> >> UART at the specified I/O port or MMIO address.
> >> MMIO inter-register address stride is either 8-bit
> >> (mmio) or 32-bit (mmio32).
> >> - The options are the same as for ttyS, above.
> >> + If none of [io|mmio|mmio32], <addr> is assumed to be
> >> + equivalent to 'mmio'. 'options' are specified in the
> >> + same format described for "console=ttyS<n>"; if
> >> + unspecified, the h/w is not initialized.
> >>
> >> pl011,<addr>
> >> Start an early, polled-mode console on a pl011 serial
> >> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> >> index e0fb5f0..f59c7a0 100644
> >> --- a/drivers/tty/serial/8250/8250_core.c
> >> +++ b/drivers/tty/serial/8250/8250_core.c
> >> @@ -3447,6 +3447,22 @@ static int univ8250_console_setup(struct console *co, char *options)
> >> return serial8250_console_setup(up, options);
> >> }
> >>
> >> +/* FIXME: this is broken on most other 8250 h/w */
> >
> > What do you mean by "most other"? What hardware does this work for?
> > What is it broken for? What is someone supposed to think/do with this
> > comment?
>
> It's a direct copy of the existing behavior for 8250 earlycon, which is
> broken for h/w with non-standard divisor registers. That's basically
> _every_ new design, because that's what vendors need to extend to support
> modern bitrates.
>
> Affected h/w that I know of includes: 8250_dw, exar xr17v35 cards, omap8250,
> omap15xx, intel byt, intel mid.
>
> But it was already broken for these and so not a regression.
Sorry, I know the function is a copy, you haven't broken anything that
wasn't already broken. I see this comment as a "rant", but we might as
well turn that "rant" into something descriptive to let other people
know what is really going on here.
Sound reasonable?
thanks,
greg k-h
On 04/04/2015 12:52 PM, Greg Kroah-Hartman wrote:
> On Sat, Apr 04, 2015 at 12:23:14PM -0400, Peter Hurley wrote:
>> On 04/04/2015 12:09 PM, Greg Kroah-Hartman wrote:
>>> On Sat, Apr 04, 2015 at 10:27:30AM -0400, Peter Hurley wrote:
[...]
>>>> +/* FIXME: this is broken on most other 8250 h/w */
>>>
>>> What do you mean by "most other"? What hardware does this work for?
>>> What is it broken for? What is someone supposed to think/do with this
>>> comment?
>>
>> It's a direct copy of the existing behavior for 8250 earlycon, which is
>> broken for h/w with non-standard divisor registers. That's basically
>> _every_ new design, because that's what vendors need to extend to support
>> modern bitrates.
>>
>> Affected h/w that I know of includes: 8250_dw, exar xr17v35 cards, omap8250,
>> omap15xx, intel byt, intel mid.
>>
>> But it was already broken for these and so not a regression.
>
> Sorry, I know the function is a copy, you haven't broken anything that
> wasn't already broken. I see this comment as a "rant", but we might as
> well turn that "rant" into something descriptive to let other people
> know what is really going on here.
It's not intended as rant. My concern is that it appears as "desirable"
code because it's new, when that's not my intention.
When others go to implement earlycon-to-console handoff, my hope is
that they don't follow this exemplar. Unfortunately, since this is
the only driver that implements earlycon-to-console handoff (right now),
some might just simply copy the behavior here. My intention is to
discourage that.
> Sound reasonable?
Ok. How about "most new"?
Regards,
Peter Hurley
Restore undocumented behavior of kernel command line parameters of
the forms:
console=uart[8250],io|mmio|mmio32,<addr>[,options]
console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.
Document the required behavior of the original implementation.
Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
Reported-by: Yinghai Lu <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
v4: Removed FIXME comment
v3: Fixed automatic console to port line settings initialization;
open-coded serial8250_console_setup() so the baud can be probed;
added sha reference in commit log
v2: Fixed regression which allowed "console=uart1337,..." to start a
console (but not an earlycon)
+ fixed earlycon= documentation related required behavior fixed by
this patch
Documentation/kernel-parameters.txt | 18 +++++++++++++++---
drivers/tty/serial/8250/8250_core.c | 37 +++++++++++++++++++++++++++++++++---
drivers/tty/serial/8250/8250_early.c | 19 ------------------
3 files changed, 49 insertions(+), 25 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index bfcb1a6..1facf0b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
uart[8250],io,<addr>[,options]
uart[8250],mmio,<addr>[,options]
+ uart[8250],mmio32,<addr>[,options]
+ uart[8250],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address,
- switching to the matching ttyS device later. The
- options are the same as for ttyS, above.
+ switching to the matching ttyS device later.
+ MMIO inter-register address stride is either 8-bit
+ (mmio) or 32-bit (mmio32).
+ If none of [io|mmio|mmio32], <addr> is assumed to be
+ equivalent to 'mmio'. 'options' are specified in the
+ same format described for ttyS above; if unspecified,
+ the h/w is not re-initialized.
+
hvc<n> Use the hypervisor console device <n>. This is for
both Xen and PowerPC hypervisors.
@@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
uart[8250],io,<addr>[,options]
uart[8250],mmio,<addr>[,options]
uart[8250],mmio32,<addr>[,options]
+ uart[8250],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address.
MMIO inter-register address stride is either 8-bit
(mmio) or 32-bit (mmio32).
- The options are the same as for ttyS, above.
+ If none of [io|mmio|mmio32], <addr> is assumed to be
+ equivalent to 'mmio'. 'options' are specified in the
+ same format described for "console=ttyS<n>"; if
+ unspecified, the h/w is not initialized.
pl011,<addr>
Start an early, polled-mode console on a pl011 serial
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e0fb5f0..456606f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3447,6 +3447,21 @@ static int univ8250_console_setup(struct console *co, char *options)
return serial8250_console_setup(up, options);
}
+static unsigned int probe_baud(struct uart_port *port)
+{
+ unsigned char lcr, dll, dlm;
+ unsigned int quot;
+
+ lcr = serial_port_in(port, UART_LCR);
+ serial_port_out(port, UART_LCR, lcr | UART_LCR_DLAB);
+ dll = serial_port_in(port, UART_DLL);
+ dlm = serial_port_in(port, UART_DLM);
+ serial_port_out(port, UART_LCR, lcr);
+
+ quot = (dlm << 8) | dll;
+ return (port->uartclk / 16) / quot;
+}
+
/**
* univ8250_console_match - non-standard console matching
* @co: registering console
@@ -3455,13 +3470,18 @@ static int univ8250_console_setup(struct console *co, char *options)
* @options: ptr to option string from console command line
*
* Only attempts to match console command lines of the form:
- * console=uart<>,io|mmio|mmio32,<addr>,<options>
- * console=uart<>,<addr>,options
+ * console=uart[8250],io|mmio|mmio32,<addr>[,<options>]
+ * console=uart[8250],0x<addr>[,<options>]
* This form is used to register an initial earlycon boot console and
* replace it with the serial8250_console at 8250 driver init.
*
* Performs console setup for a match (as required by interface)
*
+ * ** HACK ALERT **
+ * If no <options> are specified, then assume the h/w is already setup.
+ * This was the undocumented behavior of the original implementation so
+ * it is cast in stone forever.
+ *
* Returns 0 if console matches; otherwise non-zero to use default matching
*/
static int univ8250_console_match(struct console *co, char *name, int idx,
@@ -3475,12 +3495,16 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
if (strncmp(name, match, 4) != 0)
return -ENODEV;
+ if (idx && idx != 8250)
+ return -ENODEV;
+
if (uart_parse_earlycon(options, &iotype, &addr, &options))
return -ENODEV;
/* try to match the port specified on the command line */
for (i = 0; i < nr_uarts; i++) {
struct uart_port *port = &serial8250_ports[i].port;
+ int baud, bits = 8, parity = 'n', flow = 'n';
if (port->iotype != iotype)
continue;
@@ -3490,8 +3514,15 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
if (iotype == UPIO_PORT && port->iobase != addr)
continue;
+ /* link port to console */
co->index = i;
- return univ8250_console_setup(co, options);
+ port->cons = co;
+
+ if (options && *options)
+ uart_parse_options(options, &baud, &parity, &bits, &flow);
+ else
+ baud = probe_baud(port);
+ return uart_set_options(port, port->cons, baud, parity, bits, flow);
}
return -ENODEV;
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index e95ebfe..8e11968 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
serial8250_early_out(port, UART_IER, ier);
}
-static unsigned int __init probe_baud(struct uart_port *port)
-{
- unsigned char lcr, dll, dlm;
- unsigned int quot;
-
- lcr = serial8250_early_in(port, UART_LCR);
- serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
- dll = serial8250_early_in(port, UART_DLL);
- dlm = serial8250_early_in(port, UART_DLM);
- serial8250_early_out(port, UART_LCR, lcr);
-
- quot = (dlm << 8) | dll;
- return (port->uartclk / 16) / quot;
-}
-
static void __init init_port(struct earlycon_device *device)
{
struct uart_port *port = &device->port;
@@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
struct uart_port *port = &device->port;
unsigned int ier;
- device->baud = probe_baud(&device->port);
- snprintf(device->options, sizeof(device->options), "%u",
- device->baud);
-
/* assume the device was initialized, only mask interrupts */
ier = serial8250_early_in(port, UART_IER);
serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
--
2.3.5
On 04/04/2015 01:19 PM, Peter Hurley wrote:
> Restore undocumented behavior of kernel command line parameters of
> the forms:
> console=uart[8250],io|mmio|mmio32,<addr>[,options]
> console=uart[8250],<addr>[,options]
> where 'options' have not been specified; in this case, the hardware
> is assumed to be initialized.
>
> Document the required behavior of the original implementation.
Greg,
Please wait for a Tested-by from Yinghai before pushing; I have no
platform to test if this fixes his regression.
Regards,
Peter Hurley
On Sat, Apr 04, 2015 at 01:24:51PM -0400, Peter Hurley wrote:
> On 04/04/2015 01:19 PM, Peter Hurley wrote:
> > Restore undocumented behavior of kernel command line parameters of
> > the forms:
> > console=uart[8250],io|mmio|mmio32,<addr>[,options]
> > console=uart[8250],<addr>[,options]
> > where 'options' have not been specified; in this case, the hardware
> > is assumed to be initialized.
> >
> > Document the required behavior of the original implementation.
>
> Greg,
>
> Please wait for a Tested-by from Yinghai before pushing; I have no
> platform to test if this fixes his regression.
Ok, will do, thanks for redoing this and spinning up a fix so quickly.
greg k-h
On Sat, Apr 4, 2015 at 10:19 AM, Peter Hurley <[email protected]> wrote:
...
> Documentation/kernel-parameters.txt | 18 +++++++++++++++---
> drivers/tty/serial/8250/8250_core.c | 37 +++++++++++++++++++++++++++++++++---
> drivers/tty/serial/8250/8250_early.c | 19 ------------------
> 3 files changed, 49 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index bfcb1a6..1facf0b 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> + uart[8250],mmio32,<addr>[,options]
> + uart[8250],0x<addr>[,options]
put this to other patch please.
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address,
> - switching to the matching ttyS device later. The
> - options are the same as for ttyS, above.
> + switching to the matching ttyS device later.
> + MMIO inter-register address stride is either 8-bit
> + (mmio) or 32-bit (mmio32).
> + If none of [io|mmio|mmio32], <addr> is assumed to be
> + equivalent to 'mmio'. 'options' are specified in the
> + same format described for ttyS above; if unspecified,
> + the h/w is not re-initialized.
> +
> hvc<n> Use the hypervisor console device <n>. This is for
> both Xen and PowerPC hypervisors.
>
> @@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> uart[8250],mmio32,<addr>[,options]
> + uart[8250],0x<addr>[,options]
and this to another patch please.
> Start an early, polled-mode console on the 8250/16550
> UART at the specified I/O port or MMIO address.
> MMIO inter-register address stride is either 8-bit
> (mmio) or 32-bit (mmio32).
> - The options are the same as for ttyS, above.
> + If none of [io|mmio|mmio32], <addr> is assumed to be
> + equivalent to 'mmio'. 'options' are specified in the
> + same format described for "console=ttyS<n>"; if
> + unspecified, the h/w is not initialized.
>
> pl011,<addr>
> Start an early, polled-mode console on a pl011 serial
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index e0fb5f0..456606f 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -3447,6 +3447,21 @@ static int univ8250_console_setup(struct console *co, char *options)
> return serial8250_console_setup(up, options);
> }
>
> +static unsigned int probe_baud(struct uart_port *port)
> +{
> + unsigned char lcr, dll, dlm;
> + unsigned int quot;
> +
> + lcr = serial_port_in(port, UART_LCR);
> + serial_port_out(port, UART_LCR, lcr | UART_LCR_DLAB);
> + dll = serial_port_in(port, UART_DLL);
> + dlm = serial_port_in(port, UART_DLM);
> + serial_port_out(port, UART_LCR, lcr);
> +
> + quot = (dlm << 8) | dll;
> + return (port->uartclk / 16) / quot;
> +}
> +
You said some "newer" chips do not support probe baud. why do you move
code to core ?
I was thinking some embedded guys could comment out 8280_early.c, now you get
extra work for them to comment out code from 8250_core.c.
> /**
> * univ8250_console_match - non-standard console matching
> * @co: registering console
> @@ -3455,13 +3470,18 @@ static int univ8250_console_setup(struct console *co, char *options)
> * @options: ptr to option string from console command line
> *
> * Only attempts to match console command lines of the form:
> - * console=uart<>,io|mmio|mmio32,<addr>,<options>
> - * console=uart<>,<addr>,options
> + * console=uart[8250],io|mmio|mmio32,<addr>[,<options>]
> + * console=uart[8250],0x<addr>[,<options>]
> * This form is used to register an initial earlycon boot console and
> * replace it with the serial8250_console at 8250 driver init.
> *
> * Performs console setup for a match (as required by interface)
> *
> + * ** HACK ALERT **
That is not HACK original.
but your fix make it to be hack.
> + * If no <options> are specified, then assume the h/w is already setup.
> + * This was the undocumented behavior of the original implementation so
> + * it is cast in stone forever.
> + *
> * Returns 0 if console matches; otherwise non-zero to use default matching
> */
> static int univ8250_console_match(struct console *co, char *name, int idx,
> @@ -3475,12 +3495,16 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
> if (strncmp(name, match, 4) != 0)
> return -ENODEV;
>
> + if (idx && idx != 8250)
> + return -ENODEV;
> +
Your fix is getting ugly now.
> if (uart_parse_earlycon(options, &iotype, &addr, &options))
> return -ENODEV;
>
> /* try to match the port specified on the command line */
> for (i = 0; i < nr_uarts; i++) {
> struct uart_port *port = &serial8250_ports[i].port;
> + int baud, bits = 8, parity = 'n', flow = 'n';
>
> if (port->iotype != iotype)
> continue;
> @@ -3490,8 +3514,15 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
> if (iotype == UPIO_PORT && port->iobase != addr)
> continue;
>
> + /* link port to console */
> co->index = i;
> - return univ8250_console_setup(co, options);
> + port->cons = co;
> +
> + if (options && *options)
> + uart_parse_options(options, &baud, &parity, &bits, &flow);
> + else
> + baud = probe_baud(port);
> + return uart_set_options(port, port->cons, baud, parity, bits, flow);
what a mess.
Now sure if it is safe to move down probe_baud, when serial port is still used.
> }
>
> return -ENODEV;
> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
> index e95ebfe..8e11968 100644
> --- a/drivers/tty/serial/8250/8250_early.c
> +++ b/drivers/tty/serial/8250/8250_early.c
> @@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
> serial8250_early_out(port, UART_IER, ier);
> }
>
> -static unsigned int __init probe_baud(struct uart_port *port)
> -{
> - unsigned char lcr, dll, dlm;
> - unsigned int quot;
> -
> - lcr = serial8250_early_in(port, UART_LCR);
> - serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
> - dll = serial8250_early_in(port, UART_DLL);
> - dlm = serial8250_early_in(port, UART_DLM);
> - serial8250_early_out(port, UART_LCR, lcr);
> -
> - quot = (dlm << 8) | dll;
> - return (port->uartclk / 16) / quot;
> -}
> -
> static void __init init_port(struct earlycon_device *device)
> {
> struct uart_port *port = &device->port;
> @@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
> struct uart_port *port = &device->port;
> unsigned int ier;
>
> - device->baud = probe_baud(&device->port);
> - snprintf(device->options, sizeof(device->options), "%u",
> - device->baud);
> -
> /* assume the device was initialized, only mask interrupts */
> ier = serial8250_early_in(port, UART_IER);
> serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
> --
Greg, Please consider apply my fix as attached, it is much cleaner.
Or just drop commit c7cef0a84912 ("console: Add extensible console matching")
from the tty-next now.
Thanks
Yinghai
On 04/05/2015 03:09 AM, Yinghai Lu wrote:
> On Sat, Apr 4, 2015 at 10:19 AM, Peter Hurley <[email protected]> wrote:
> ...
>> Documentation/kernel-parameters.txt | 18 +++++++++++++++---
>> drivers/tty/serial/8250/8250_core.c | 37 +++++++++++++++++++++++++++++++++---
>> drivers/tty/serial/8250/8250_early.c | 19 ------------------
>> 3 files changed, 49 insertions(+), 25 deletions(-)
>>
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index bfcb1a6..1facf0b 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>
>> uart[8250],io,<addr>[,options]
>> uart[8250],mmio,<addr>[,options]
>> + uart[8250],mmio32,<addr>[,options]
>> + uart[8250],0x<addr>[,options]
>
> put this to other patch please.
>
>> Start an early, polled-mode console on the 8250/16550
>> UART at the specified I/O port or MMIO address,
>> - switching to the matching ttyS device later. The
>> - options are the same as for ttyS, above.
>> + switching to the matching ttyS device later.
>> + MMIO inter-register address stride is either 8-bit
>> + (mmio) or 32-bit (mmio32).
>> + If none of [io|mmio|mmio32], <addr> is assumed to be
>> + equivalent to 'mmio'. 'options' are specified in the
>> + same format described for ttyS above; if unspecified,
>> + the h/w is not re-initialized.
>> +
>> hvc<n> Use the hypervisor console device <n>. This is for
>> both Xen and PowerPC hypervisors.
>>
>> @@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>> uart[8250],io,<addr>[,options]
>> uart[8250],mmio,<addr>[,options]
>> uart[8250],mmio32,<addr>[,options]
>> + uart[8250],0x<addr>[,options]
>
> and this to another patch please.
>
>> Start an early, polled-mode console on the 8250/16550
>> UART at the specified I/O port or MMIO address.
>> MMIO inter-register address stride is either 8-bit
>> (mmio) or 32-bit (mmio32).
>> - The options are the same as for ttyS, above.
>> + If none of [io|mmio|mmio32], <addr> is assumed to be
>> + equivalent to 'mmio'. 'options' are specified in the
>> + same format described for "console=ttyS<n>"; if
>> + unspecified, the h/w is not initialized.
>>
>> pl011,<addr>
>> Start an early, polled-mode console on a pl011 serial
>> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>> index e0fb5f0..456606f 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -3447,6 +3447,21 @@ static int univ8250_console_setup(struct console *co, char *options)
>> return serial8250_console_setup(up, options);
>> }
>>
>> +static unsigned int probe_baud(struct uart_port *port)
>> +{
>> + unsigned char lcr, dll, dlm;
>> + unsigned int quot;
>> +
>> + lcr = serial_port_in(port, UART_LCR);
>> + serial_port_out(port, UART_LCR, lcr | UART_LCR_DLAB);
>> + dll = serial_port_in(port, UART_DLL);
>> + dlm = serial_port_in(port, UART_DLM);
>> + serial_port_out(port, UART_LCR, lcr);
>> +
>> + quot = (dlm << 8) | dll;
>> + return (port->uartclk / 16) / quot;
>> +}
>> +
>
> You said some "newer" chips do not support probe baud. why do you move
> code to core ?
There's no functional difference, but here it's at least possible
to add support for exar, synopsys, ti omap, intel, etc. based on either
port type or a function vector initialized by the sub-driver.
> I was thinking some embedded guys could comment out 8280_early.c, now you get
> extra work for them to comment out code from 8250_core.c.
Huh? That's just ridiculous.
>> /**
>> * univ8250_console_match - non-standard console matching
>> * @co: registering console
>> @@ -3455,13 +3470,18 @@ static int univ8250_console_setup(struct console *co, char *options)
>> * @options: ptr to option string from console command line
>> *
>> * Only attempts to match console command lines of the form:
>> - * console=uart<>,io|mmio|mmio32,<addr>,<options>
>> - * console=uart<>,<addr>,options
>> + * console=uart[8250],io|mmio|mmio32,<addr>[,<options>]
>> + * console=uart[8250],0x<addr>[,<options>]
>> * This form is used to register an initial earlycon boot console and
>> * replace it with the serial8250_console at 8250 driver init.
>> *
>> * Performs console setup for a match (as required by interface)
>> *
>> + * ** HACK ALERT **
>
> That is not HACK original.
>
> but your fix make it to be hack.
>
>> + * If no <options> are specified, then assume the h/w is already setup.
>> + * This was the undocumented behavior of the original implementation so
>> + * it is cast in stone forever.
>> + *
>> * Returns 0 if console matches; otherwise non-zero to use default matching
>> */
>> static int univ8250_console_match(struct console *co, char *name, int idx,
>> @@ -3475,12 +3495,16 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
>> if (strncmp(name, match, 4) != 0)
>> return -ENODEV;
>>
>> + if (idx && idx != 8250)
>> + return -ENODEV;
>> +
>
> Your fix is getting ugly now.
This is required anyway. I'm not the one that decided it would be
cute to have "uart" and "uart8250" mean the same thing.
>> if (uart_parse_earlycon(options, &iotype, &addr, &options))
>> return -ENODEV;
>>
>> /* try to match the port specified on the command line */
>> for (i = 0; i < nr_uarts; i++) {
>> struct uart_port *port = &serial8250_ports[i].port;
>> + int baud, bits = 8, parity = 'n', flow = 'n';
>>
>> if (port->iotype != iotype)
>> continue;
>> @@ -3490,8 +3514,15 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
>> if (iotype == UPIO_PORT && port->iobase != addr)
>> continue;
>>
>> + /* link port to console */
>> co->index = i;
>> - return univ8250_console_setup(co, options);
>> + port->cons = co;
>> +
>> + if (options && *options)
>> + uart_parse_options(options, &baud, &parity, &bits, &flow);
>> + else
>> + baud = probe_baud(port);
>> + return uart_set_options(port, port->cons, baud, parity, bits, flow);
>
> what a mess.
This was the mess:
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
>
> -static int serial8250_console_early_setup(void)
> -{
> - return serial8250_find_port_for_earlycon();
> -}
>
> @@ -3347,7 +3392,7 @@ static struct console serial8250_console = {
> .write = serial8250_console_write,
> .device = uart_console_device,
> .setup = serial8250_console_setup,
> - .early_setup = serial8250_console_early_setup,
> .flags = CON_PRINTBUFFER | CON_ANYTIME,
> .index = -1,
> .data = &serial8250_reg,
> @@ -3361,19 +3406,6 @@ static int __init serial8250_console_init(void)
>
> -int serial8250_find_port(struct uart_port *p)
> -{
> - int line;
> - struct uart_port *port;
> -
> - for (line = 0; line < nr_uarts; line++) {
> - port = &serial8250_ports[line].port;
> - if (uart_match_port(p, port))
> - return line;
> - }
> - return -ENODEV;
> -}
> -
>
> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
>
> -
> -int serial8250_find_port_for_earlycon(void)
> -{
> - struct earlycon_device *device = early_device;
> - struct uart_port *port = device ? &device->port : NULL;
> - int line;
> - int ret;
> -
> - if (!port || (!port->membase && !port->iobase))
> - return -ENODEV;
> -
> - line = serial8250_find_port(port);
> - if (line < 0)
> - return -ENODEV;
> -
> - ret = update_console_cmdline("uart", 8250,
> - "ttyS", line, device->options);
> - if (ret < 0)
> - ret = update_console_cmdline("uart", 0,
> - "ttyS", line, device->options);
> -
> - return ret;
> -}
>
> diff --git a/include/linux/console.h b/include/linux/console.h
>
> -extern int update_console_cmdline(char *name, int idx, char *name_new, int idx_new, char *options);
>
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
>
> -extern int serial8250_find_port(struct uart_port *p);
> -extern int serial8250_find_port_for_earlycon(void);
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>
> -int update_console_cmdline(char *name, int idx, char *name_new, int idx_new, char *options)
> -{
> - struct console_cmdline *c;
> - int i;
> -
> - for (i = 0, c = console_cmdline;
> - i < MAX_CMDLINECONSOLES && c->name[0];
> - i++, c++)
> - if (strcmp(c->name, name) == 0 && c->index == idx) {
> - strlcpy(c->name, name_new, sizeof(c->name));
> - c->options = options;
> - c->index = idx_new;
> - return i;
> - }
> - /* not found */
> - return -1;
> -}
> -
>
> @@ -2436,9 +2418,6 @@ void register_console(struct console *newcon)
>
> - if (newcon->early_setup)
> - newcon->early_setup();
> -
> Now sure if it is safe to move down probe_baud, when serial port is still used.
>
>> }
>>
>> return -ENODEV;
>> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
>> index e95ebfe..8e11968 100644
>> --- a/drivers/tty/serial/8250/8250_early.c
>> +++ b/drivers/tty/serial/8250/8250_early.c
>> @@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
>> serial8250_early_out(port, UART_IER, ier);
>> }
>>
>> -static unsigned int __init probe_baud(struct uart_port *port)
>> -{
>> - unsigned char lcr, dll, dlm;
>> - unsigned int quot;
>> -
>> - lcr = serial8250_early_in(port, UART_LCR);
>> - serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
>> - dll = serial8250_early_in(port, UART_DLL);
>> - dlm = serial8250_early_in(port, UART_DLM);
>> - serial8250_early_out(port, UART_LCR, lcr);
>> -
>> - quot = (dlm << 8) | dll;
>> - return (port->uartclk / 16) / quot;
>> -}
>> -
>> static void __init init_port(struct earlycon_device *device)
>> {
>> struct uart_port *port = &device->port;
>> @@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
>> struct uart_port *port = &device->port;
>> unsigned int ier;
>>
>> - device->baud = probe_baud(&device->port);
>> - snprintf(device->options, sizeof(device->options), "%u",
>> - device->baud);
>> -
>> /* assume the device was initialized, only mask interrupts */
>> ier = serial8250_early_in(port, UART_IER);
>> serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
>> --
>
> Greg, Please consider apply my fix as attached, it is much cleaner.
On what planet is 27 loc across 4 source files cleaner than
6 loc that might be reducible to 2?
Restore undocumented behavior of kernel command line parameters of
the forms:
console=uart[8250],io|mmio|mmio32,<addr>[,options]
console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.
Document the required behavior of the original implementation.
Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
Reported-by: Yinghai Lu <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
v5: Refactored serial8250_console_setup() & univ8250_console_setup()
to eliminate open-coding; net +1 LOC (excludes documentation)
v4: Removed FIXME comment
v3: Fixed automatic console to port line settings initialization;
open-coded serial8250_console_setup() so the baud can be probed;
added sha reference in commit log
v2: Fixed regression which allowed "console=uart1337,..." to start a
console (but not an earlycon)
+ fixed earlycon= documentation related required behavior fixed by
this patch
Documentation/kernel-parameters.txt | 18 ++++++++++++---
drivers/tty/serial/8250/8250_core.c | 45 ++++++++++++++++++++++++++++--------
drivers/tty/serial/8250/8250_early.c | 19 ---------------
3 files changed, 50 insertions(+), 32 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index bfcb1a6..1facf0b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
uart[8250],io,<addr>[,options]
uart[8250],mmio,<addr>[,options]
+ uart[8250],mmio32,<addr>[,options]
+ uart[8250],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address,
- switching to the matching ttyS device later. The
- options are the same as for ttyS, above.
+ switching to the matching ttyS device later.
+ MMIO inter-register address stride is either 8-bit
+ (mmio) or 32-bit (mmio32).
+ If none of [io|mmio|mmio32], <addr> is assumed to be
+ equivalent to 'mmio'. 'options' are specified in the
+ same format described for ttyS above; if unspecified,
+ the h/w is not re-initialized.
+
hvc<n> Use the hypervisor console device <n>. This is for
both Xen and PowerPC hypervisors.
@@ -944,11 +952,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
uart[8250],io,<addr>[,options]
uart[8250],mmio,<addr>[,options]
uart[8250],mmio32,<addr>[,options]
+ uart[8250],0x<addr>[,options]
Start an early, polled-mode console on the 8250/16550
UART at the specified I/O port or MMIO address.
MMIO inter-register address stride is either 8-bit
(mmio) or 32-bit (mmio32).
- The options are the same as for ttyS, above.
+ If none of [io|mmio|mmio32], <addr> is assumed to be
+ equivalent to 'mmio'. 'options' are specified in the
+ same format described for "console=ttyS<n>"; if
+ unspecified, the h/w is not initialized.
pl011,<addr>
Start an early, polled-mode console on a pl011 serial
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e0fb5f0..f8c27de 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3412,9 +3412,23 @@ static void univ8250_console_write(struct console *co, const char *s,
serial8250_console_write(up, s, count);
}
-static int serial8250_console_setup(struct uart_8250_port *up, char *options)
+static unsigned int probe_baud(struct uart_port *port)
+{
+ unsigned char lcr, dll, dlm;
+ unsigned int quot;
+
+ lcr = serial_port_in(port, UART_LCR);
+ serial_port_out(port, UART_LCR, lcr | UART_LCR_DLAB);
+ dll = serial_port_in(port, UART_DLL);
+ dlm = serial_port_in(port, UART_DLM);
+ serial_port_out(port, UART_LCR, lcr);
+
+ quot = (dlm << 8) | dll;
+ return (port->uartclk / 16) / quot;
+}
+
+static int serial8250_console_setup(struct uart_port *port, char *options, bool probe)
{
- struct uart_port *port = &up->port;
int baud = 9600;
int bits = 8;
int parity = 'n';
@@ -3423,15 +3437,17 @@ static int serial8250_console_setup(struct uart_8250_port *up, char *options)
if (!port->iobase && !port->membase)
return -ENODEV;
- if (options)
+ if (options && (!probe || *options))
uart_parse_options(options, &baud, &parity, &bits, &flow);
+ else if (probe)
+ baud = probe_baud(port);
return uart_set_options(port, port->cons, baud, parity, bits, flow);
}
static int univ8250_console_setup(struct console *co, char *options)
{
- struct uart_8250_port *up;
+ struct uart_port *port;
/*
* Check whether an invalid uart number has been specified, and
@@ -3440,11 +3456,11 @@ static int univ8250_console_setup(struct console *co, char *options)
*/
if (co->index >= nr_uarts)
co->index = 0;
- up = &serial8250_ports[co->index];
+ port = &serial8250_ports[co->index].port;
/* link port to console */
- up->port.cons = co;
+ port->cons = co;
- return serial8250_console_setup(up, options);
+ return serial8250_console_setup(port, options, false);
}
/**
@@ -3455,13 +3471,18 @@ static int univ8250_console_setup(struct console *co, char *options)
* @options: ptr to option string from console command line
*
* Only attempts to match console command lines of the form:
- * console=uart<>,io|mmio|mmio32,<addr>,<options>
- * console=uart<>,<addr>,options
+ * console=uart[8250],io|mmio|mmio32,<addr>[,<options>]
+ * console=uart[8250],0x<addr>[,<options>]
* This form is used to register an initial earlycon boot console and
* replace it with the serial8250_console at 8250 driver init.
*
* Performs console setup for a match (as required by interface)
*
+ * ** HACK ALERT **
+ * If no <options> are specified, then assume the h/w is already setup.
+ * This was the undocumented behavior of the original implementation so
+ * it is cast in stone forever.
+ *
* Returns 0 if console matches; otherwise non-zero to use default matching
*/
static int univ8250_console_match(struct console *co, char *name, int idx,
@@ -3475,6 +3496,9 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
if (strncmp(name, match, 4) != 0)
return -ENODEV;
+ if (idx && idx != 8250)
+ return -ENODEV;
+
if (uart_parse_earlycon(options, &iotype, &addr, &options))
return -ENODEV;
@@ -3491,7 +3515,8 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
continue;
co->index = i;
- return univ8250_console_setup(co, options);
+ port->cons = co;
+ return serial8250_console_setup(port, options, true);
}
return -ENODEV;
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index e95ebfe..8e11968 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
serial8250_early_out(port, UART_IER, ier);
}
-static unsigned int __init probe_baud(struct uart_port *port)
-{
- unsigned char lcr, dll, dlm;
- unsigned int quot;
-
- lcr = serial8250_early_in(port, UART_LCR);
- serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
- dll = serial8250_early_in(port, UART_DLL);
- dlm = serial8250_early_in(port, UART_DLM);
- serial8250_early_out(port, UART_LCR, lcr);
-
- quot = (dlm << 8) | dll;
- return (port->uartclk / 16) / quot;
-}
-
static void __init init_port(struct earlycon_device *device)
{
struct uart_port *port = &device->port;
@@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
struct uart_port *port = &device->port;
unsigned int ier;
- device->baud = probe_baud(&device->port);
- snprintf(device->options, sizeof(device->options), "%u",
- device->baud);
-
/* assume the device was initialized, only mask interrupts */
ier = serial8250_early_in(port, UART_IER);
serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
--
2.3.5
On Sun, Apr 5, 2015 at 7:52 AM, Peter Hurley <[email protected]> wrote:
> Restore undocumented behavior of kernel command line parameters of
> the forms:
> console=uart[8250],io|mmio|mmio32,<addr>[,options]
> console=uart[8250],<addr>[,options]
> where 'options' have not been specified; in this case, the hardware
> is assumed to be initialized.
>
> Document the required behavior of the original implementation.
>
> Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
> Reported-by: Yinghai Lu <[email protected]>
> Signed-off-by: Peter Hurley <[email protected]>
> ---
> v5: Refactored serial8250_console_setup() & univ8250_console_setup()
> to eliminate open-coding; net +1 LOC (excludes documentation)
>
> v4: Removed FIXME comment
>
> v3: Fixed automatic console to port line settings initialization;
> open-coded serial8250_console_setup() so the baud can be probed;
> added sha reference in commit log
>
> v2: Fixed regression which allowed "console=uart1337,..." to start a
> console (but not an earlycon)
> + fixed earlycon= documentation related required behavior fixed by
> this patch
>
> Documentation/kernel-parameters.txt | 18 ++++++++++++---
> drivers/tty/serial/8250/8250_core.c | 45 ++++++++++++++++++++++++++++--------
> drivers/tty/serial/8250/8250_early.c | 19 ---------------
> 3 files changed, 50 insertions(+), 32 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index bfcb1a6..1facf0b 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -713,10 +713,18 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>
> uart[8250],io,<addr>[,options]
> uart[8250],mmio,<addr>[,options]
> + uart[8250],mmio32,<addr>[,options]
> + uart[8250],0x<addr>[,options]
You just don't listen.
I wasted too much time on this.
Greg,
Please just drop c7cef0a84912cab3c9df8 ("console: Add extensible
console matching").
Thanks
Yinghai
On Sun, Apr 5, 2015 at 6:06 AM, Peter Hurley <[email protected]> wrote:
> On 04/05/2015 03:09 AM, Yinghai Lu wrote:
> On what planet is 27 loc across 4 source files cleaner than
> 6 loc that might be reducible to 2?
loc is not only thing to decide if it is cleaner.
Let's Greg and Andrew to check which one is clean.
Restore undocumented behavior of kernel command line parameters of
the forms:
console=uart[8250],io|mmio|mmio32,<addr>[,options]
console=uart[8250],<addr>[,options]
where 'options' have not been specified; in this case, the hardware
is assumed to be initialized.
Fixes: c7cef0a84912cab3c9df8 ("console: Add extensible console matching")
Reported-by: Yinghai Lu <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
v6: Removed documentation
+ removed unnecessary checks; this fix does not attempt to replicate
the _exact_ original behavior, but only the reported breakage.
v5: Refactored serial8250_console_setup() & univ8250_console_setup()
to eliminate open-coding; net +1 LOC (excludes documentation)
v4: Removed FIXME comment
v3: Fixed automatic console to port line settings initialization;
open-coded serial8250_console_setup() so the baud can be probed;
added sha reference in commit log
v2: Fixed regression which allowed "console=uart1337,..." to start a
console (but not an earlycon)
+ fixed earlycon= documentation related required behavior fixed by
this patch
drivers/tty/serial/8250/8250_core.c | 31 ++++++++++++++++++++++++-------
drivers/tty/serial/8250/8250_early.c | 19 -------------------
2 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index e0fb5f0..18142ee 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -3412,9 +3412,23 @@ static void univ8250_console_write(struct console *co, const char *s,
serial8250_console_write(up, s, count);
}
-static int serial8250_console_setup(struct uart_8250_port *up, char *options)
+static unsigned int probe_baud(struct uart_port *port)
+{
+ unsigned char lcr, dll, dlm;
+ unsigned int quot;
+
+ lcr = serial_port_in(port, UART_LCR);
+ serial_port_out(port, UART_LCR, lcr | UART_LCR_DLAB);
+ dll = serial_port_in(port, UART_DLL);
+ dlm = serial_port_in(port, UART_DLM);
+ serial_port_out(port, UART_LCR, lcr);
+
+ quot = (dlm << 8) | dll;
+ return (port->uartclk / 16) / quot;
+}
+
+static int serial8250_console_setup(struct uart_port *port, char *options, bool probe)
{
- struct uart_port *port = &up->port;
int baud = 9600;
int bits = 8;
int parity = 'n';
@@ -3425,13 +3439,15 @@ static int serial8250_console_setup(struct uart_8250_port *up, char *options)
if (options)
uart_parse_options(options, &baud, &parity, &bits, &flow);
+ else if (probe)
+ baud = probe_baud(port);
return uart_set_options(port, port->cons, baud, parity, bits, flow);
}
static int univ8250_console_setup(struct console *co, char *options)
{
- struct uart_8250_port *up;
+ struct uart_port *port;
/*
* Check whether an invalid uart number has been specified, and
@@ -3440,11 +3456,11 @@ static int univ8250_console_setup(struct console *co, char *options)
*/
if (co->index >= nr_uarts)
co->index = 0;
- up = &serial8250_ports[co->index];
+ port = &serial8250_ports[co->index].port;
/* link port to console */
- up->port.cons = co;
+ port->cons = co;
- return serial8250_console_setup(up, options);
+ return serial8250_console_setup(port, options, false);
}
/**
@@ -3491,7 +3507,8 @@ static int univ8250_console_match(struct console *co, char *name, int idx,
continue;
co->index = i;
- return univ8250_console_setup(co, options);
+ port->cons = co;
+ return serial8250_console_setup(port, options, true);
}
return -ENODEV;
diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index e95ebfe..8e11968 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -105,21 +105,6 @@ static void __init early_serial8250_write(struct console *console,
serial8250_early_out(port, UART_IER, ier);
}
-static unsigned int __init probe_baud(struct uart_port *port)
-{
- unsigned char lcr, dll, dlm;
- unsigned int quot;
-
- lcr = serial8250_early_in(port, UART_LCR);
- serial8250_early_out(port, UART_LCR, lcr | UART_LCR_DLAB);
- dll = serial8250_early_in(port, UART_DLL);
- dlm = serial8250_early_in(port, UART_DLM);
- serial8250_early_out(port, UART_LCR, lcr);
-
- quot = (dlm << 8) | dll;
- return (port->uartclk / 16) / quot;
-}
-
static void __init init_port(struct earlycon_device *device)
{
struct uart_port *port = &device->port;
@@ -151,10 +136,6 @@ static int __init early_serial8250_setup(struct earlycon_device *device,
struct uart_port *port = &device->port;
unsigned int ier;
- device->baud = probe_baud(&device->port);
- snprintf(device->options, sizeof(device->options), "%u",
- device->baud);
-
/* assume the device was initialized, only mask interrupts */
ier = serial8250_early_in(port, UART_IER);
serial8250_early_out(port, UART_IER, ier & UART_IER_UUE);
--
2.3.5