On Tue, 2015-05-12 at 14:10 +0600, Alexander Kuleshov wrote:
> The early_printk function is usable only after the setup_early_printk will
> be executed. We pass 'earlyprintk' through the kernel command line, so it
> will be usable only after the 'parse_early_param' will be executed. This means
> that we have usable earlyprintk only during early boot, kernel decompression
> and after call of the 'parse_early_param'. This patchset makes earlyprintk
> usable before the call of the 'parse_early_param'.
>
> This patch makes setup_early_printk visible for head{32,64}.c. So the
> 'early_printk' function will be usabable after decompression of the
> kernel and before parse_early_param will be called. It also must be
> safe if CONFIG_CMDLINE_BOOL and CONFIG_CMDLINE_OVERRIDE are set, because
> setup_cmdline function will be called before setup_early_printk.
>
Few comments below.
> Tested it with qemu, so early_printk() is usable and prints to serial
> console right after setup_early_printk function called.
>
> Signed-off-by: Alexander Kuleshov <kuleshovm...@gmail.com>
> ---
> arch/x86/include/asm/serial.h | 3 +++
> arch/x86/kernel/early_printk.c | 25 +++++++++++++++++++++++++
> arch/x86/kernel/head32.c | 5 +++++
> arch/x86/kernel/head64.c | 5 +++++
> 4 files changed, 38 insertions(+)
>
> diff --git a/arch/x86/include/asm/serial.h b/arch/x86/include/asm/serial.h
> index 8378b8c..198f041 100644
> --- a/arch/x86/include/asm/serial.h
> +++ b/arch/x86/include/asm/serial.h
> @@ -26,4 +26,7 @@
> { .uart = 0, BASE_BAUD, 0x3E8, 4, STD_COMX_FLAGS }, /*
> ttyS2 */ \
> { .uart = 0, BASE_BAUD, 0x2E8, 3, STD_COM4_FLAGS }, /*
> ttyS3 */
>
> +/* used by arch/x86/kernel/head{32,64}.c */
> +int __init setup_early_serial_console(void);
> +
Actually, have you investigated how this works on the other supported
architectures? It might be better to align this with them.
> #endif /* _ASM_X86_SERIAL_H */
> diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
> index 89427d8..96425c3 100644
> --- a/arch/x86/kernel/early_printk.c
> +++ b/arch/x86/kernel/early_printk.c
> @@ -385,4 +385,29 @@ static int __init setup_early_printk(char *buf)
> return 0;
> }
>
> +int __init setup_early_serial_console(void)
> +{
> +#ifdef CONFIG_EARLY_PRINTK
> + char *arg;
> +
> + /*
> + * make sure that we have:
> + * "serial,0x3f8,115200"
> + * "serial,ttyS0,115200"
> + * "ttyS0,115200"
> + */
Indentation and text styling problems.
> + arg = strstr(boot_command_line, "earlyprintk=serial");
> + if (!arg)
> + arg = strstr(boot_command_line, "earlyprintk=ttyS");
> + if (!arg)
> + return -1;
What about other cases like that described in setup_early_printk()?
> +
> + arg = strstr(boot_command_line, "earlyprintk=");
> + /* += strlen("earlyprintk"); */
> + arg += 12;
> +
> + return setup_early_printk(arg);
> +#endif
#ifdef seems redundant here, you already build this module iff
EARLY_PRINTK is set.
> +}
So, the logic of this function seems broken. I don't get why you have to
check the contents of earlyprintk parameter.
> +
> early_param("earlyprintk", setup_early_printk);
> diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
> index 92e8b5f..d325d0c 100644
> --- a/arch/x86/kernel/head32.c
> +++ b/arch/x86/kernel/head32.c
> @@ -32,6 +32,11 @@ static void __init i386_default_early_setup(void)
> asmlinkage __visible void __init i386_start_kernel(void)
> {
> setup_builtin_cmdline();
> + setup_early_serial_console();
> +
> + if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> + early_printk("Kernel alive\n");
This should be unified (like printed "Early printk is initialized" in
setup_early_printk() ), moreover what about other architectures?
> +
> cr4_init_shadow();
> sanitize_boot_params(&boot_params);
>
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 38da21c..06fcc1b 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -173,6 +173,11 @@ asmlinkage __visible void __init
> x86_64_start_kernel(char * real_mode_data)
> copy_bootdata(__va(real_mode_data));
> setup_builtin_cmdline();
>
> + setup_early_serial_console();
Those two can be grouped in the same way like in previous change (see
above).
> +
> + if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> + early_printk("Kernel alive\n");
Same comment as in above chunk.
> +
> /*
> * Load microcode early on BSP.
> */
> --
> 2.4.0
--
Andy Shevchenko <andriy.shevche...@intel.com>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
