Messages in this thread | | | From | Michael Ellerman <> | Subject | Re: [PATCH 08/17] tty: New RISC-V SBI console driver | Date | Fri, 14 Jul 2017 15:07:23 +1000 |
| |
Palmer Dabbelt <palmer@dabbelt.com> writes: > On Thu, 13 Jul 2017 05:32:26 PDT (-0700), james.hogan@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
| |