Palmer Dabbelt <pal...@dabbelt.com> writes:
> On Thu, 13 Jul 2017 05:32:26 PDT (-0700), james.ho...@imgtec.com wrote:
>> On Thu, Jul 13, 2017 at 09:59:53PM +1000, Michael Ellerman wrote:
>>>
>>> I think it's fairly uncontroversial to have the early console in arch
>>> code, especially in a case like this where there's no code shared
>>> between the console and the TTY driver. But maybe someone will prove me
>>> wrong.
>>>
>>> Doing it the other way is not really hacky IMO, you can just have an
>>> extern for the early console in one of your asm headers.
>>
>> For reference both metag and mips do something like this for JTAG based
>> consoles (with drivers both residing in drivers/tty/):
...
>>
>> Its not all that pretty but it gets you console output that much
>> earlier and is a fairly special case, so I think its worth it.
>
> If someone else is doing it, then it's good enough for me :). How does this
> look?
>
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 319fad96f537..148fd0dc414b 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -59,6 +59,14 @@ unsigned long pfn_base;
> /* The lucky hart to first increment this variable will boot the other cores
> */
> atomic_t hart_lottery;
>
> +#if defined(CONFIG_HVC_RISCV_SBI) && defined(CONFIG_EARLY_PRINTK)
^
This is always true because you
said so in your Kconfig.
> +/*
> + * The SBI's early console lives in hvc_riscv_sbi.c, but we want very early
> + * access
> + */
> +extern struct console riscv_sbi_early_console_dev;
> +#endif
I would have put it in one of your arch headers, so that the hvc driver
can include it too.
Personally I tend not to bother #ifdef'ing every extern declaration, but
there are arguments both ways.
> #ifdef CONFIG_BLK_DEV_INITRD
> static void __init setup_initrd(void)
> {
> @@ -203,6 +211,13 @@ static void __init setup_bootmem(void)
>
> void __init setup_arch(char **cmdline_p)
> {
> +#if defined(CONFIG_TTY_RISCV_SBI) && defined(CONFIG_EARLY_PRINTK)
^
HVC
> + if (likely(early_console == NULL)) {
I don't think you need to check. You're the boss of early_console at
this point in boot.
> + early_console = &riscv_sbi_early_console;
> + register_console(early_console);
> + }
> +#endif
> +
> #ifdef CONFIG_CMDLINE_BOOL
> #ifdef CONFIG_CMDLINE_OVERRIDE
> strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
cheers
