Re: [PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-13 Thread Michael Ellerman
Palmer Dabbelt  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


Re: [PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-13 Thread Palmer Dabbelt
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:
>> Palmer Dabbelt  writes:
>>
>> > On Wed, 12 Jul 2017 04:04:00 PDT (-0700), m...@ellerman.id.au wrote:
>> >> Palmer Dabbelt  writes:
>> >>
>> >>> On Mon, 10 Jul 2017 23:21:07 PDT (-0700), m...@ellerman.id.au wrote:
>>  Palmer Dabbelt  writes:
>> >
>>  ...
>> > +#ifdef CONFIG_EARLY_PRINTK
>> > +static void sbi_console_write(struct console *co, const char *buf,
>> > +unsigned int n)
>> > +{
>> > +  int i;
>> > +
>> > +  for (i = 0; i < n; ++i) {
>> > +  if (buf[i] == '\n')
>> > +  sbi_console_putchar('\r');
>> > +  sbi_console_putchar(buf[i]);
>> > +  }
>> > +}
>> > +
>> > +static struct console early_console_dev __initdata = {
>> > +  .name   = "early",
>> > +  .write  = sbi_console_write,
>> > +  .flags  = CON_PRINTBUFFER | CON_BOOT,
>> 
>>  AFAICS you could add CON_ANYTIME here, which would mean this console
>>  would print output before the CPU is online.
>> 
>>  I think it doesn't currently matter because you call parse_early_param()
>>  from setup_arch(), at which point the boot CPU has been marked online.
>> 
>>  But if this console can actually work earlier then you might be better
>>  off just registering it unconditionally very early.
>> >>>
>> >>> That seems like a good idea.  I'm not familiar with how all this works, 
>> >>> but
>> >>> from my understanding of this early_initcall() should be sufficient to 
>> >>> make
>> >>> this work?  The only other driver that sets CON_ANYTIME and supports
>> >>> EARLY_PRINTK is hvc_xen, but that installs a header to let init code 
>> >>> register
>> >>> the console directly.  The early_initcall mechanism seems cleaner if it 
>> >>> does
>> >>> the right thing.
>> >>
>> >> Unfortunately early_initcall is not very "early" :)  It's earlier than
>> >> all the other initcalls, but it's late compared to most of the arch boot
>> >> code.
>> >>
>> >> The early_param() will work better, ie. register the console earlier
>> >> and increase the chance of you getting output from an early crash, than
>> >> early_initcall. But it requires you to put earlyprintk on the command 
>> >> line.
>> >>
>> >> The best option is to just register the console as early as you can, ie.
>> >> as soon as it can give you output. So somewhere in your setup_arch(), or
>> >> even earlier (I haven't read your boot code).
>> >
>> > Doing it that way would require either moving the TTY driver into arch 
>> > code (it
>> > was specifically suggested we move it out) or adding a header file to allow
>> > setup_arch() to call into the driver (XEN does this, and we're doing it 
>> > for our
>> > timer, but it seems hacky).
>>
>> 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/):
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/metag/kernel/setup.c#n107
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/metag/kernel/setup.c#n234
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/include/asm/cdmm.h#n98
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/kernel/setup.c#n958
>
> 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)
+/*
+ * 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
+
 #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)
+   if (likely(early_console == NULL)) {
+   early_console = &riscv_sbi_early_console;
+   register_console(ear

Re: [PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-13 Thread James Hogan
On Thu, Jul 13, 2017 at 09:59:53PM +1000, Michael Ellerman wrote:
> Palmer Dabbelt  writes:
> 
> > On Wed, 12 Jul 2017 04:04:00 PDT (-0700), m...@ellerman.id.au wrote:
> >> Palmer Dabbelt  writes:
> >>
> >>> On Mon, 10 Jul 2017 23:21:07 PDT (-0700), m...@ellerman.id.au wrote:
>  Palmer Dabbelt  writes:
> >
>  ...
> > +#ifdef CONFIG_EARLY_PRINTK
> > +static void sbi_console_write(struct console *co, const char *buf,
> > + unsigned int n)
> > +{
> > +   int i;
> > +
> > +   for (i = 0; i < n; ++i) {
> > +   if (buf[i] == '\n')
> > +   sbi_console_putchar('\r');
> > +   sbi_console_putchar(buf[i]);
> > +   }
> > +}
> > +
> > +static struct console early_console_dev __initdata = {
> > +   .name   = "early",
> > +   .write  = sbi_console_write,
> > +   .flags  = CON_PRINTBUFFER | CON_BOOT,
> 
>  AFAICS you could add CON_ANYTIME here, which would mean this console
>  would print output before the CPU is online.
> 
>  I think it doesn't currently matter because you call parse_early_param()
>  from setup_arch(), at which point the boot CPU has been marked online.
> 
>  But if this console can actually work earlier then you might be better
>  off just registering it unconditionally very early.
> >>>
> >>> That seems like a good idea.  I'm not familiar with how all this works, 
> >>> but
> >>> from my understanding of this early_initcall() should be sufficient to 
> >>> make
> >>> this work?  The only other driver that sets CON_ANYTIME and supports
> >>> EARLY_PRINTK is hvc_xen, but that installs a header to let init code 
> >>> register
> >>> the console directly.  The early_initcall mechanism seems cleaner if it 
> >>> does
> >>> the right thing.
> >>
> >> Unfortunately early_initcall is not very "early" :)  It's earlier than
> >> all the other initcalls, but it's late compared to most of the arch boot
> >> code.
> >>
> >> The early_param() will work better, ie. register the console earlier
> >> and increase the chance of you getting output from an early crash, than
> >> early_initcall. But it requires you to put earlyprintk on the command line.
> >>
> >> The best option is to just register the console as early as you can, ie.
> >> as soon as it can give you output. So somewhere in your setup_arch(), or
> >> even earlier (I haven't read your boot code).
> >
> > Doing it that way would require either moving the TTY driver into arch code 
> > (it
> > was specifically suggested we move it out) or adding a header file to allow
> > setup_arch() to call into the driver (XEN does this, and we're doing it for 
> > our
> > timer, but it seems hacky).
> 
> 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/):

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/metag/kernel/setup.c#n107
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/metag/kernel/setup.c#n234

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/include/asm/cdmm.h#n98
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/mips/kernel/setup.c#n958

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.

Cheers
James


signature.asc
Description: Digital signature


Re: [PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-13 Thread Michael Ellerman
Palmer Dabbelt  writes:

> On Wed, 12 Jul 2017 04:04:00 PDT (-0700), m...@ellerman.id.au wrote:
>> Palmer Dabbelt  writes:
>>
>>> On Mon, 10 Jul 2017 23:21:07 PDT (-0700), m...@ellerman.id.au wrote:
 Palmer Dabbelt  writes:
>
 ...
> +#ifdef CONFIG_EARLY_PRINTK
> +static void sbi_console_write(struct console *co, const char *buf,
> +   unsigned int n)
> +{
> + int i;
> +
> + for (i = 0; i < n; ++i) {
> + if (buf[i] == '\n')
> + sbi_console_putchar('\r');
> + sbi_console_putchar(buf[i]);
> + }
> +}
> +
> +static struct console early_console_dev __initdata = {
> + .name   = "early",
> + .write  = sbi_console_write,
> + .flags  = CON_PRINTBUFFER | CON_BOOT,

 AFAICS you could add CON_ANYTIME here, which would mean this console
 would print output before the CPU is online.

 I think it doesn't currently matter because you call parse_early_param()
 from setup_arch(), at which point the boot CPU has been marked online.

 But if this console can actually work earlier then you might be better
 off just registering it unconditionally very early.
>>>
>>> That seems like a good idea.  I'm not familiar with how all this works, but
>>> from my understanding of this early_initcall() should be sufficient to make
>>> this work?  The only other driver that sets CON_ANYTIME and supports
>>> EARLY_PRINTK is hvc_xen, but that installs a header to let init code 
>>> register
>>> the console directly.  The early_initcall mechanism seems cleaner if it does
>>> the right thing.
>>
>> Unfortunately early_initcall is not very "early" :)  It's earlier than
>> all the other initcalls, but it's late compared to most of the arch boot
>> code.
>>
>> The early_param() will work better, ie. register the console earlier
>> and increase the chance of you getting output from an early crash, than
>> early_initcall. But it requires you to put earlyprintk on the command line.
>>
>> The best option is to just register the console as early as you can, ie.
>> as soon as it can give you output. So somewhere in your setup_arch(), or
>> even earlier (I haven't read your boot code).
>
> Doing it that way would require either moving the TTY driver into arch code 
> (it
> was specifically suggested we move it out) or adding a header file to allow
> setup_arch() to call into the driver (XEN does this, and we're doing it for 
> our
> timer, but it seems hacky).

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.

But anyway none of that is really that important so I'll shuddup :)

cheers


Re: [PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-12 Thread Palmer Dabbelt
On Wed, 12 Jul 2017 04:04:00 PDT (-0700), m...@ellerman.id.au wrote:
> Palmer Dabbelt  writes:
>
>> On Mon, 10 Jul 2017 23:21:07 PDT (-0700), m...@ellerman.id.au wrote:
>>> Palmer Dabbelt  writes:

>>> ...
 +#ifdef CONFIG_EARLY_PRINTK
 +static void sbi_console_write(struct console *co, const char *buf,
 +unsigned int n)
 +{
 +  int i;
 +
 +  for (i = 0; i < n; ++i) {
 +  if (buf[i] == '\n')
 +  sbi_console_putchar('\r');
 +  sbi_console_putchar(buf[i]);
 +  }
 +}
 +
 +static struct console early_console_dev __initdata = {
 +  .name   = "early",
 +  .write  = sbi_console_write,
 +  .flags  = CON_PRINTBUFFER | CON_BOOT,
>>>
>>> AFAICS you could add CON_ANYTIME here, which would mean this console
>>> would print output before the CPU is online.
>>>
>>> I think it doesn't currently matter because you call parse_early_param()
>>> from setup_arch(), at which point the boot CPU has been marked online.
>>>
>>> But if this console can actually work earlier then you might be better
>>> off just registering it unconditionally very early.
>>
>> That seems like a good idea.  I'm not familiar with how all this works, but
>> from my understanding of this early_initcall() should be sufficient to make
>> this work?  The only other driver that sets CON_ANYTIME and supports
>> EARLY_PRINTK is hvc_xen, but that installs a header to let init code register
>> the console directly.  The early_initcall mechanism seems cleaner if it does
>> the right thing.
>
> Unfortunately early_initcall is not very "early" :)  It's earlier than
> all the other initcalls, but it's late compared to most of the arch boot
> code.
>
> The early_param() will work better, ie. register the console earlier
> and increase the chance of you getting output from an early crash, than
> early_initcall. But it requires you to put earlyprintk on the command line.
>
> The best option is to just register the console as early as you can, ie.
> as soon as it can give you output. So somewhere in your setup_arch(), or
> even earlier (I haven't read your boot code).

Doing it that way would require either moving the TTY driver into arch code (it
was specifically suggested we move it out) or adding a header file to allow
setup_arch() to call into the driver (XEN does this, and we're doing it for our
timer, but it seems hacky).


Re: [PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-12 Thread Michael Ellerman
Palmer Dabbelt  writes:

> On Mon, 10 Jul 2017 23:21:07 PDT (-0700), m...@ellerman.id.au wrote:
>> Palmer Dabbelt  writes:
>>>
>> ...
>>> +#ifdef CONFIG_EARLY_PRINTK
>>> +static void sbi_console_write(struct console *co, const char *buf,
>>> + unsigned int n)
>>> +{
>>> +   int i;
>>> +
>>> +   for (i = 0; i < n; ++i) {
>>> +   if (buf[i] == '\n')
>>> +   sbi_console_putchar('\r');
>>> +   sbi_console_putchar(buf[i]);
>>> +   }
>>> +}
>>> +
>>> +static struct console early_console_dev __initdata = {
>>> +   .name   = "early",
>>> +   .write  = sbi_console_write,
>>> +   .flags  = CON_PRINTBUFFER | CON_BOOT,
>>
>> AFAICS you could add CON_ANYTIME here, which would mean this console
>> would print output before the CPU is online.
>>
>> I think it doesn't currently matter because you call parse_early_param()
>> from setup_arch(), at which point the boot CPU has been marked online.
>>
>> But if this console can actually work earlier then you might be better
>> off just registering it unconditionally very early.
>
> That seems like a good idea.  I'm not familiar with how all this works, but
> from my understanding of this early_initcall() should be sufficient to make
> this work?  The only other driver that sets CON_ANYTIME and supports
> EARLY_PRINTK is hvc_xen, but that installs a header to let init code register
> the console directly.  The early_initcall mechanism seems cleaner if it does
> the right thing.

Unfortunately early_initcall is not very "early" :)  It's earlier than
all the other initcalls, but it's late compared to most of the arch boot
code.

The early_param() will work better, ie. register the console earlier
and increase the chance of you getting output from an early crash, than
early_initcall. But it requires you to put earlyprintk on the command line.

The best option is to just register the console as early as you can, ie.
as soon as it can give you output. So somewhere in your setup_arch(), or
even earlier (I haven't read your boot code).

cheers


[PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-11 Thread Palmer Dabbelt
The RISC-V ISA defines a simple console that is availiable via SBI calls
on all systems.  This patch adds a driver for this console interface
that can act as both a target for early printk and as the system
console.

Signed-off-by: Palmer Dabbelt 
---
 drivers/tty/hvc/Kconfig   |  11 +
 drivers/tty/hvc/Makefile  |   1 +
 drivers/tty/hvc/hvc_sbi.c | 103 ++
 3 files changed, 115 insertions(+)
 create mode 100644 drivers/tty/hvc/hvc_sbi.c

diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
index b8d5ea0ae26b..24cbaaf47d8d 100644
--- a/drivers/tty/hvc/Kconfig
+++ b/drivers/tty/hvc/Kconfig
@@ -97,6 +97,17 @@ config HVC_BFIN_JTAG
 the HVC driver.  If you don't have JTAG, then you probably don't
 want this option.
 
+config HVC_SBI
+   bool "SBI console support"
+   depends on RISCV
+   select HVC_DRIVER
+   default y
+   help
+ This enables support for console output via RISC-V SBI calls, which
+ is normally used only during boot to output printk.
+
+ If you don't know what do to here, say Y.
+
 config HVCS
tristate "IBM Hypervisor Virtual Console Server support"
depends on PPC_PSERIES && HVC_CONSOLE
diff --git a/drivers/tty/hvc/Makefile b/drivers/tty/hvc/Makefile
index 6a2702be76d1..2d63bfe4a96b 100644
--- a/drivers/tty/hvc/Makefile
+++ b/drivers/tty/hvc/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_HVC_IUCV)+= hvc_iucv.o
 obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o
 obj-$(CONFIG_HVC_BFIN_JTAG)+= hvc_bfin_jtag.o
 obj-$(CONFIG_HVCS) += hvcs.o
+obj-$(CONFIG_HVC_SBI)  += hvc_sbi.o
diff --git a/drivers/tty/hvc/hvc_sbi.c b/drivers/tty/hvc/hvc_sbi.c
new file mode 100644
index ..534d6b75a2c6
--- /dev/null
+++ b/drivers/tty/hvc/hvc_sbi.c
@@ -0,0 +1,103 @@
+/*
+ * RISC-V SBI interface to hvc_console.c
+ *  based on drivers-tty/hvc/hvc_udbg.c
+ *
+ * Copyright (C) 2008 David Gibson, IBM Corporation
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2017 SiFive
+ *
+ *   This program is free software; you can redistribute it and/or
+ *   modify it under the terms of the GNU General Public License
+ *   as published by the Free Software Foundation, version 2.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "hvc_console.h"
+
+static int hvc_sbi_tty_put(uint32_t vtermno, const char *buf, int count)
+{
+   int i;
+
+   for (i = 0; i < count; i++)
+   sbi_console_putchar(buf[i]);
+
+   return i;
+}
+
+static int hvc_sbi_tty_get(uint32_t vtermno, char *buf, int count)
+{
+   int i, c;
+
+   for (i = 0; i < count; i++) {
+   if ((c = sbi_console_getchar()) < 0)
+   break;
+   buf[i] = c;
+   }
+
+   return i;
+}
+
+static const struct hv_ops hvc_sbi_ops = {
+   .get_chars = hvc_sbi_tty_get,
+   .put_chars = hvc_sbi_tty_put,
+};
+
+static int __init hvc_sbi_init(void)
+{
+   return PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_ops, 16));
+}
+device_initcall(hvc_sbi_init);
+
+static int __init hvc_sbi_console_init(void)
+{
+   hvc_instantiate(0, 0, &hvc_sbi_ops);
+   add_preferred_console("hvc", 0, NULL);
+
+   return 0;
+}
+console_initcall(hvc_sbi_console_init);
+
+#ifdef CONFIG_EARLY_PRINTK
+static void sbi_console_write(struct console *co, const char *buf,
+ unsigned int n)
+{
+   int i;
+
+   for (i = 0; i < n; ++i) {
+   if (buf[i] == '\n')
+   sbi_console_putchar('\r');
+   sbi_console_putchar(buf[i]);
+   }
+}
+
+static struct console early_console_dev __initdata = {
+   .name   = "early",
+   .write  = sbi_console_write,
+   .flags  = CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME,
+   .index  = -1
+};
+
+static int __init setup_early_printk(void)
+{
+   if (early_console == NULL) {
+   early_console = &early_console_dev;
+   register_console(early_console);
+   }
+   return 0;
+}
+early_initcall(setup_early_printk);
+#endif
-- 
2.13.0



Re: [PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-11 Thread Palmer Dabbelt
On Mon, 10 Jul 2017 23:21:07 PDT (-0700), m...@ellerman.id.au wrote:
> Palmer Dabbelt  writes:
>>
> ...
>> +#ifdef CONFIG_EARLY_PRINTK
>> +static void sbi_console_write(struct console *co, const char *buf,
>> +  unsigned int n)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < n; ++i) {
>> +if (buf[i] == '\n')
>> +sbi_console_putchar('\r');
>> +sbi_console_putchar(buf[i]);
>> +}
>> +}
>> +
>> +static struct console early_console_dev __initdata = {
>> +.name   = "early",
>> +.write  = sbi_console_write,
>> +.flags  = CON_PRINTBUFFER | CON_BOOT,
>
> AFAICS you could add CON_ANYTIME here, which would mean this console
> would print output before the CPU is online.
>
> I think it doesn't currently matter because you call parse_early_param()
> from setup_arch(), at which point the boot CPU has been marked online.
>
> But if this console can actually work earlier then you might be better
> off just registering it unconditionally very early.

That seems like a good idea.  I'm not familiar with how all this works, but
from my understanding of this early_initcall() should be sufficient to make
this work?  The only other driver that sets CON_ANYTIME and supports
EARLY_PRINTK is hvc_xen, but that installs a header to let init code register
the console directly.  The early_initcall mechanism seems cleaner if it does
the right thing.

How does this look?

  diff --git a/drivers/tty/hvc/hvc_sbi.c b/drivers/tty/hvc/hvc_sbi.c
  index 98114cbd85f1..534d6b75a2c6 100644
  --- a/drivers/tty/hvc/hvc_sbi.c
  +++ b/drivers/tty/hvc/hvc_sbi.c
  @@ -87,11 +87,11 @@ static void sbi_console_write(struct console *co, const 
char *buf,
   static struct console early_console_dev __initdata = {
  .name   = "early",
  .write  = sbi_console_write,
  -   .flags  = CON_PRINTBUFFER | CON_BOOT,
  +   .flags  = CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME,
  .index  = -1
   };

  -static int __init setup_early_printk(char *str)
  +static int __init setup_early_printk(void)
   {
  if (early_console == NULL) {
  early_console = &early_console_dev;
  @@ -99,5 +99,5 @@ static int __init setup_early_printk(char *str)
  }
  return 0;
   }
  -early_param("earlyprintk", setup_early_printk);
  +early_initcall(setup_early_printk);
   #endif


Re: [PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-10 Thread Michael Ellerman
Palmer Dabbelt  writes:
>
...
> +#ifdef CONFIG_EARLY_PRINTK
> +static void sbi_console_write(struct console *co, const char *buf,
> +   unsigned int n)
> +{
> + int i;
> +
> + for (i = 0; i < n; ++i) {
> + if (buf[i] == '\n')
> + sbi_console_putchar('\r');
> + sbi_console_putchar(buf[i]);
> + }
> +}
> +
> +static struct console early_console_dev __initdata = {
> + .name   = "early",
> + .write  = sbi_console_write,
> + .flags  = CON_PRINTBUFFER | CON_BOOT,

AFAICS you could add CON_ANYTIME here, which would mean this console
would print output before the CPU is online.

I think it doesn't currently matter because you call parse_early_param()
from setup_arch(), at which point the boot CPU has been marked online.

But if this console can actually work earlier then you might be better
off just registering it unconditionally very early.

cheers

> +static int __init setup_early_printk(char *str)
> +{
> + if (early_console == NULL) {
> + early_console = &early_console_dev;
> + register_console(early_console);
> + }
> + return 0;
> +}
> +early_param("earlyprintk", setup_early_printk);
> +#endif
> -- 
> 2.13.0


[PATCH 08/17] tty: New RISC-V SBI console driver

2017-07-10 Thread Palmer Dabbelt
The RISC-V ISA defines a simple console that is availiable via SBI calls
on all systems.  This patch adds a driver for this console interface
that can act as both a target for early printk and as the system
console.

Signed-off-by: Palmer Dabbelt 
---
 drivers/tty/hvc/Kconfig   |  11 +
 drivers/tty/hvc/Makefile  |   1 +
 drivers/tty/hvc/hvc_sbi.c | 103 ++
 3 files changed, 115 insertions(+)
 create mode 100644 drivers/tty/hvc/hvc_sbi.c

diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
index b8d5ea0ae26b..24cbaaf47d8d 100644
--- a/drivers/tty/hvc/Kconfig
+++ b/drivers/tty/hvc/Kconfig
@@ -97,6 +97,17 @@ config HVC_BFIN_JTAG
 the HVC driver.  If you don't have JTAG, then you probably don't
 want this option.
 
+config HVC_SBI
+   bool "SBI console support"
+   depends on RISCV
+   select HVC_DRIVER
+   default y
+   help
+ This enables support for console output via RISC-V SBI calls, which
+ is normally used only during boot to output printk.
+
+ If you don't know what do to here, say Y.
+
 config HVCS
tristate "IBM Hypervisor Virtual Console Server support"
depends on PPC_PSERIES && HVC_CONSOLE
diff --git a/drivers/tty/hvc/Makefile b/drivers/tty/hvc/Makefile
index 6a2702be76d1..2d63bfe4a96b 100644
--- a/drivers/tty/hvc/Makefile
+++ b/drivers/tty/hvc/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_HVC_IUCV)+= hvc_iucv.o
 obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o
 obj-$(CONFIG_HVC_BFIN_JTAG)+= hvc_bfin_jtag.o
 obj-$(CONFIG_HVCS) += hvcs.o
+obj-$(CONFIG_HVC_SBI)  += hvc_sbi.o
diff --git a/drivers/tty/hvc/hvc_sbi.c b/drivers/tty/hvc/hvc_sbi.c
new file mode 100644
index ..98114cbd85f1
--- /dev/null
+++ b/drivers/tty/hvc/hvc_sbi.c
@@ -0,0 +1,103 @@
+/*
+ * RISC-V SBI interface to hvc_console.c
+ *  based on drivers-tty/hvc/hvc_udbg.c
+ *
+ * Copyright (C) 2008 David Gibson, IBM Corporation
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2017 SiFive
+ *
+ *   This program is free software; you can redistribute it and/or
+ *   modify it under the terms of the GNU General Public License
+ *   as published by the Free Software Foundation, version 2.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "hvc_console.h"
+
+static int hvc_sbi_tty_put(uint32_t vtermno, const char *buf, int count)
+{
+   int i;
+
+   for (i = 0; i < count; i++)
+   sbi_console_putchar(buf[i]);
+
+   return i;
+}
+
+static int hvc_sbi_tty_get(uint32_t vtermno, char *buf, int count)
+{
+   int i, c;
+
+   for (i = 0; i < count; i++) {
+   if ((c = sbi_console_getchar()) < 0)
+   break;
+   buf[i] = c;
+   }
+
+   return i;
+}
+
+static const struct hv_ops hvc_sbi_ops = {
+   .get_chars = hvc_sbi_tty_get,
+   .put_chars = hvc_sbi_tty_put,
+};
+
+static int __init hvc_sbi_init(void)
+{
+   return PTR_ERR_OR_ZERO(hvc_alloc(0, 0, &hvc_sbi_ops, 16));
+}
+device_initcall(hvc_sbi_init);
+
+static int __init hvc_sbi_console_init(void)
+{
+   hvc_instantiate(0, 0, &hvc_sbi_ops);
+   add_preferred_console("hvc", 0, NULL);
+
+   return 0;
+}
+console_initcall(hvc_sbi_console_init);
+
+#ifdef CONFIG_EARLY_PRINTK
+static void sbi_console_write(struct console *co, const char *buf,
+ unsigned int n)
+{
+   int i;
+
+   for (i = 0; i < n; ++i) {
+   if (buf[i] == '\n')
+   sbi_console_putchar('\r');
+   sbi_console_putchar(buf[i]);
+   }
+}
+
+static struct console early_console_dev __initdata = {
+   .name   = "early",
+   .write  = sbi_console_write,
+   .flags  = CON_PRINTBUFFER | CON_BOOT,
+   .index  = -1
+};
+
+static int __init setup_early_printk(char *str)
+{
+   if (early_console == NULL) {
+   early_console = &early_console_dev;
+   register_console(early_console);
+   }
+   return 0;
+}
+early_param("earlyprintk", setup_early_printk);
+#endif
-- 
2.13.0