| Message ID | mvmbm4oehd5.fsf_-_@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tty/serial: emit CR before NL in RISC-V SBL console | expand |
On Thu, Jan 10, 2019 at 7:37 PM Andreas Schwab <schwab@suse.de> wrote: > > Signed-off-by: Andreas Schwab <schwab@suse.de> > --- > drivers/tty/serial/earlycon-riscv-sbi.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/tty/serial/earlycon-riscv-sbi.c b/drivers/tty/serial/earlycon-riscv-sbi.c > index e1a551aae3..9c51b945b5 100644 > --- a/drivers/tty/serial/earlycon-riscv-sbi.c > +++ b/drivers/tty/serial/earlycon-riscv-sbi.c > @@ -15,8 +15,11 @@ static void sbi_console_write(struct console *con, > { > int i; > > - for (i = 0; i < n; ++i) > + for (i = 0; i < n; ++i) { > + if (s[i] == '\n') > + sbi_console_putchar('\r'); Instead of doing '\n' handling here, we should do it in BBL or OpenSBI (i.e. SBI runtime firmware) otherwise all users of SBI_CONSOLE_PUTCHAR (namely, Linux, FreeBSD, FreeRTOS, U-Boot S-mode, etc) will endup having '\n' handling. Currently, the BBL does not handle it but we have already handled this in upcoming OpenSBI. The SBI_CONSOLE_PUTCHAR is comparable to ARM semihosting early prints (refer, tty/serial/earlycon-arm-semihost.c). Even in ARM semihosting early prints the '\n' is not explicitly handled. Regards, Anup
On Jan 10 2019, Anup Patel <anup@brainfault.org> wrote: > Instead of doing '\n' handling here, we should do it in BBL or > OpenSBI (i.e. SBI runtime firmware) otherwise all users of > SBI_CONSOLE_PUTCHAR (namely, Linux, FreeBSD, > FreeRTOS, U-Boot S-mode, etc) will endup having '\n' > handling. I don't think the serial driver should do NLCR handling on its own, without being instructed by the tty layer. Since the earlycon does not have a tty layer, it needs to handle it explicitly. Andreas.
On Thu, Jan 10, 2019 at 8:56 PM Andreas Schwab <schwab@suse.de> wrote: > > On Jan 10 2019, Anup Patel <anup@brainfault.org> wrote: > > > Instead of doing '\n' handling here, we should do it in BBL or > > OpenSBI (i.e. SBI runtime firmware) otherwise all users of > > SBI_CONSOLE_PUTCHAR (namely, Linux, FreeBSD, > > FreeRTOS, U-Boot S-mode, etc) will endup having '\n' > > handling. > > I don't think the serial driver should do NLCR handling on its own, > without being instructed by the tty layer. Since the earlycon does not > have a tty layer, it needs to handle it explicitly. I tried to investigate more. What you suggest is correct but we should use uart_console_write() API. Instead of explicitly doing NLCR here, we should do something as follows: static void sbi_putc(struct uart_port *port, int c) { sbi_console_putchar(c); } static void sbi_console_write(struct console *con, const char *s, unsigned n) { struct earlycon_device *dev = con->data; uart_console_write(&dev->port, s, n, sbi_putc); } The uart_console_write() does the NLCR handling. Regards, Anup
On Thu, 10 Jan 2019 07:26:46 PST (-0800), schwab@suse.de wrote: > On Jan 10 2019, Anup Patel <anup@brainfault.org> wrote: > >> Instead of doing '\n' handling here, we should do it in BBL or >> OpenSBI (i.e. SBI runtime firmware) otherwise all users of >> SBI_CONSOLE_PUTCHAR (namely, Linux, FreeBSD, >> FreeRTOS, U-Boot S-mode, etc) will endup having '\n' >> handling. > > I don't think the serial driver should do NLCR handling on its own, > without being instructed by the tty layer. Since the earlycon does not > have a tty layer, it needs to handle it explicitly. I think it's best if we have the SBI interfaces be defined to be bit-exact. It does mean that everyone who uses the interfaces needs to do this handling, but I think that's actually a feature because it'll be subtly different everywhere. This also has the advantage of being compatible with what we currently do in BBL and what we current expect in Linux everywhere but this earlycon driver (and will do assuming we merge this patch). On Thu, 10 Jan 2019 08:17:07 PST (-0800), anup@brainfault.org wrote: > On Thu, Jan 10, 2019 at 8:56 PM Andreas Schwab <schwab@suse.de> wrote: >> >> On Jan 10 2019, Anup Patel <anup@brainfault.org> wrote: >> >> > Instead of doing '\n' handling here, we should do it in BBL or >> > OpenSBI (i.e. SBI runtime firmware) otherwise all users of >> > SBI_CONSOLE_PUTCHAR (namely, Linux, FreeBSD, >> > FreeRTOS, U-Boot S-mode, etc) will endup having '\n' >> > handling. >> >> I don't think the serial driver should do NLCR handling on its own, >> without being instructed by the tty layer. Since the earlycon does not >> have a tty layer, it needs to handle it explicitly. > > I tried to investigate more. What you suggest is correct > but we should use uart_console_write() API. > > Instead of explicitly doing NLCR here, we should do > something as follows: > > static void sbi_putc(struct uart_port *port, int c) > { > sbi_console_putchar(c); > } > > static void sbi_console_write(struct console *con, > const char *s, unsigned n) > { > struct earlycon_device *dev = con->data; > uart_console_write(&dev->port, s, n, sbi_putc); > } > > The uart_console_write() does the NLCR handling. That does look cleaner. I'll expect a v2 :)
diff --git a/drivers/tty/serial/earlycon-riscv-sbi.c b/drivers/tty/serial/earlycon-riscv-sbi.c index e1a551aae3..9c51b945b5 100644 --- a/drivers/tty/serial/earlycon-riscv-sbi.c +++ b/drivers/tty/serial/earlycon-riscv-sbi.c @@ -15,8 +15,11 @@ static void sbi_console_write(struct console *con, { int i; - for (i = 0; i < n; ++i) + for (i = 0; i < n; ++i) { + if (s[i] == '\n') + sbi_console_putchar('\r'); sbi_console_putchar(s[i]); + } } static int __init early_sbi_setup(struct earlycon_device *device,
Signed-off-by: Andreas Schwab <schwab@suse.de> --- drivers/tty/serial/earlycon-riscv-sbi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)