2018-12-04 14:12:08

by Anup Patel

[permalink] [raw]
Subject: [PATCH 0/3] RISC-V SBI earlycon

This patchset adds RISC-V SBI earlycon and removes RISC-V EARLY_PRINTK.

We should use earlycon over existing EARLY_PRINTK for SBI console because:
1. It's a more generic way of implementing early console for debugging
2. Current RISC-V EARLY_PRINTK is a compile-time option whereas earlycon
is enabled at run-time via kernel parameters.
3. To use earlycon with SBI, we have to pass "earlycon=sbi" in kernel
parameters. If earlycon kernel parameter is not provided then kernel
boots much faster which is very useful in real-world RISC-V deployments.

The patchset is tested on QEMU virt machine. It is based on Linux-4.20-rc5
and can be found at riscv_earlycon_v1 branch of:
https://github.com/avpatel/linux.git

Anup Patel (3):
tty/serial: Add RISC-V SBI earlycon support
RISC-V: defconfig: Enable RISC-V SBI earlycon support
RISC-V: Remove EARLY_PRINTK support

arch/riscv/Kconfig.debug | 2 --
arch/riscv/configs/defconfig | 1 +
arch/riscv/kernel/setup.c | 28 -------------------------
drivers/tty/serial/Kconfig | 12 +++++++++++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/earlycon-riscv-sbi.c | 28 +++++++++++++++++++++++++
6 files changed, 42 insertions(+), 30 deletions(-)
create mode 100644 drivers/tty/serial/earlycon-riscv-sbi.c

--
2.17.1



2018-12-04 14:12:28

by Anup Patel

[permalink] [raw]
Subject: [PATCH 1/3] tty/serial: Add RISC-V SBI earlycon support

In RISC-V, the M-mode runtime firmware provide SBI calls for
debug prints. This patch adds earlycon support using RISC-V
SBI console calls. To enable it, just pass "earlycon=sbi" in
kernel parameters.

Signed-off-by: Anup Patel <[email protected]>
---
drivers/tty/serial/Kconfig | 12 +++++++++++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/earlycon-riscv-sbi.c | 28 +++++++++++++++++++++++++
3 files changed, 41 insertions(+)
create mode 100644 drivers/tty/serial/earlycon-riscv-sbi.c

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 32886c304641..287bb41ac814 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -85,6 +85,18 @@ config SERIAL_EARLYCON_ARM_SEMIHOST
with "earlycon=smh" on the kernel command line. The console is
enabled when early_param is processed.

+config SERIAL_EARLYCON_RISCV_SBI
+ bool "Early console using RISC-V SBI"
+ depends on RISCV
+ select SERIAL_CORE
+ select SERIAL_CORE_CONSOLE
+ select SERIAL_EARLYCON
+ help
+ Support for early debug console using RISC-V SBI. This enables
+ the console before standard serial driver is probed. This is enabled
+ with "earlycon=sbi" on the kernel command line. The console is
+ enabled when early_param is processed.
+
config SERIAL_SB1250_DUART
tristate "BCM1xxx on-chip DUART serial support"
depends on SIBYTE_SB1xxx_SOC=y
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index daac675612df..3ce26ce08616 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_SERIAL_CORE) += serial_core.o

obj-$(CONFIG_SERIAL_EARLYCON) += earlycon.o
obj-$(CONFIG_SERIAL_EARLYCON_ARM_SEMIHOST) += earlycon-arm-semihost.o
+obj-$(CONFIG_SERIAL_EARLYCON_RISCV_SBI) += earlycon-riscv-sbi.o

# These Sparc drivers have to appear before others such as 8250
# which share ttySx minor node space. Otherwise console device
diff --git a/drivers/tty/serial/earlycon-riscv-sbi.c b/drivers/tty/serial/earlycon-riscv-sbi.c
new file mode 100644
index 000000000000..e1a551aae336
--- /dev/null
+++ b/drivers/tty/serial/earlycon-riscv-sbi.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RISC-V SBI based earlycon
+ *
+ * Copyright (C) 2018 Anup Patel <[email protected]>
+ */
+#include <linux/kernel.h>
+#include <linux/console.h>
+#include <linux/init.h>
+#include <linux/serial_core.h>
+#include <asm/sbi.h>
+
+static void sbi_console_write(struct console *con,
+ const char *s, unsigned int n)
+{
+ int i;
+
+ for (i = 0; i < n; ++i)
+ sbi_console_putchar(s[i]);
+}
+
+static int __init early_sbi_setup(struct earlycon_device *device,
+ const char *opt)
+{
+ device->con->write = sbi_console_write;
+ return 0;
+}
+EARLYCON_DECLARE(sbi, early_sbi_setup);
--
2.17.1


2018-12-04 14:13:14

by Anup Patel

[permalink] [raw]
Subject: [PATCH 2/3] RISC-V: defconfig: Enable RISC-V SBI earlycon support

This patch enables RISC-V SBI earlycon support in default defconfig
so that we can use "earlycon=sbi" in kernel parameters for early
debug prints.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/configs/defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index ef4f15df9adf..f399659d3b8d 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -46,6 +46,7 @@ CONFIG_INPUT_MOUSEDEV=y
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_SERIAL_EARLYCON_RISCV_SBI=y
CONFIG_HVC_RISCV_SBI=y
# CONFIG_PTP_1588_CLOCK is not set
CONFIG_DRM=y
--
2.17.1


2018-12-04 14:15:17

by Anup Patel

[permalink] [raw]
Subject: [PATCH 3/3] RISC-V: Remove EARLY_PRINTK support

The EARLY_PRINTK using SBI console calls is not required
any more because we now have RISC-V SBI support in generic
earlycon framework.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/Kconfig.debug | 2 --
arch/riscv/kernel/setup.c | 28 ----------------------------
2 files changed, 30 deletions(-)

diff --git a/arch/riscv/Kconfig.debug b/arch/riscv/Kconfig.debug
index c5a72f17c469..e69de29bb2d1 100644
--- a/arch/riscv/Kconfig.debug
+++ b/arch/riscv/Kconfig.debug
@@ -1,2 +0,0 @@
-config EARLY_PRINTK
- def_bool y
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 2c290e6aaa6e..fc8006a042eb 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -35,31 +35,9 @@
#include <asm/sections.h>
#include <asm/pgtable.h>
#include <asm/smp.h>
-#include <asm/sbi.h>
#include <asm/tlbflush.h>
#include <asm/thread_info.h>

-#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]);
- }
-}
-
-struct console riscv_sbi_early_console_dev __initdata = {
- .name = "early",
- .write = sbi_console_write,
- .flags = CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME,
- .index = -1
-};
-#endif
-
#ifdef CONFIG_DUMMY_CONSOLE
struct screen_info screen_info = {
.orig_video_lines = 30,
@@ -219,12 +197,6 @@ static void __init setup_bootmem(void)

void __init setup_arch(char **cmdline_p)
{
-#if defined(CONFIG_EARLY_PRINTK)
- if (likely(early_console == NULL)) {
- early_console = &riscv_sbi_early_console_dev;
- register_console(early_console);
- }
-#endif
*cmdline_p = boot_command_line;

parse_early_param();
--
2.17.1


2018-12-05 10:01:24

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/3] tty/serial: Add RISC-V SBI earlycon support

On Tue, Dec 04, 2018 at 07:25:05PM +0530, Anup Patel wrote:
> In RISC-V, the M-mode runtime firmware provide SBI calls for
> debug prints. This patch adds earlycon support using RISC-V
> SBI console calls. To enable it, just pass "earlycon=sbi" in
> kernel parameters.
>
> Signed-off-by: Anup Patel <[email protected]>

This makes more sense to take through the riscv tree, so feel free to
add:

Acked-by: Greg Kroah-Hartman <[email protected]>

to it and take it that way.

thanks,

greg k-h

2018-12-07 18:31:30

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 1/3] tty/serial: Add RISC-V SBI earlycon support

On Tue, 04 Dec 2018 05:55:05 PST (-0800), [email protected] wrote:
> In RISC-V, the M-mode runtime firmware provide SBI calls for
> debug prints. This patch adds earlycon support using RISC-V
> SBI console calls. To enable it, just pass "earlycon=sbi" in
> kernel parameters.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> drivers/tty/serial/Kconfig | 12 +++++++++++
> drivers/tty/serial/Makefile | 1 +
> drivers/tty/serial/earlycon-riscv-sbi.c | 28 +++++++++++++++++++++++++
> 3 files changed, 41 insertions(+)
> create mode 100644 drivers/tty/serial/earlycon-riscv-sbi.c
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 32886c304641..287bb41ac814 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -85,6 +85,18 @@ config SERIAL_EARLYCON_ARM_SEMIHOST
> with "earlycon=smh" on the kernel command line. The console is
> enabled when early_param is processed.
>
> +config SERIAL_EARLYCON_RISCV_SBI
> + bool "Early console using RISC-V SBI"
> + depends on RISCV
> + select SERIAL_CORE
> + select SERIAL_CORE_CONSOLE
> + select SERIAL_EARLYCON
> + help
> + Support for early debug console using RISC-V SBI. This enables
> + the console before standard serial driver is probed. This is enabled
> + with "earlycon=sbi" on the kernel command line. The console is
> + enabled when early_param is processed.
> +
> config SERIAL_SB1250_DUART
> tristate "BCM1xxx on-chip DUART serial support"
> depends on SIBYTE_SB1xxx_SOC=y
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index daac675612df..3ce26ce08616 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_SERIAL_CORE) += serial_core.o
>
> obj-$(CONFIG_SERIAL_EARLYCON) += earlycon.o
> obj-$(CONFIG_SERIAL_EARLYCON_ARM_SEMIHOST) += earlycon-arm-semihost.o
> +obj-$(CONFIG_SERIAL_EARLYCON_RISCV_SBI) += earlycon-riscv-sbi.o
>
> # These Sparc drivers have to appear before others such as 8250
> # which share ttySx minor node space. Otherwise console device
> diff --git a/drivers/tty/serial/earlycon-riscv-sbi.c b/drivers/tty/serial/earlycon-riscv-sbi.c
> new file mode 100644
> index 000000000000..e1a551aae336
> --- /dev/null
> +++ b/drivers/tty/serial/earlycon-riscv-sbi.c
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RISC-V SBI based earlycon
> + *
> + * Copyright (C) 2018 Anup Patel <[email protected]>
> + */
> +#include <linux/kernel.h>
> +#include <linux/console.h>
> +#include <linux/init.h>
> +#include <linux/serial_core.h>
> +#include <asm/sbi.h>
> +
> +static void sbi_console_write(struct console *con,
> + const char *s, unsigned int n)
> +{
> + int i;
> +
> + for (i = 0; i < n; ++i)
> + sbi_console_putchar(s[i]);
> +}
> +
> +static int __init early_sbi_setup(struct earlycon_device *device,
> + const char *opt)
> +{
> + device->con->write = sbi_console_write;
> + return 0;
> +}
> +EARLYCON_DECLARE(sbi, early_sbi_setup);

Reviewed-by: Palmer Dabbelt <[email protected]>

2018-12-07 18:31:43

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 2/3] RISC-V: defconfig: Enable RISC-V SBI earlycon support

On Tue, 04 Dec 2018 05:55:06 PST (-0800), [email protected] wrote:
> This patch enables RISC-V SBI earlycon support in default defconfig
> so that we can use "earlycon=sbi" in kernel parameters for early
> debug prints.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/configs/defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> index ef4f15df9adf..f399659d3b8d 100644
> --- a/arch/riscv/configs/defconfig
> +++ b/arch/riscv/configs/defconfig
> @@ -46,6 +46,7 @@ CONFIG_INPUT_MOUSEDEV=y
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_SERIAL_OF_PLATFORM=y
> +CONFIG_SERIAL_EARLYCON_RISCV_SBI=y
> CONFIG_HVC_RISCV_SBI=y
> # CONFIG_PTP_1588_CLOCK is not set
> CONFIG_DRM=y

Reviewed-by: Palmer Dabbelt <[email protected]>

2018-12-07 18:32:06

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 0/3] RISC-V SBI earlycon

On Tue, 04 Dec 2018 05:55:04 PST (-0800), [email protected] wrote:
> This patchset adds RISC-V SBI earlycon and removes RISC-V EARLY_PRINTK.
>
> We should use earlycon over existing EARLY_PRINTK for SBI console because:
> 1. It's a more generic way of implementing early console for debugging
> 2. Current RISC-V EARLY_PRINTK is a compile-time option whereas earlycon
> is enabled at run-time via kernel parameters.
> 3. To use earlycon with SBI, we have to pass "earlycon=sbi" in kernel
> parameters. If earlycon kernel parameter is not provided then kernel
> boots much faster which is very useful in real-world RISC-V deployments.
>
> The patchset is tested on QEMU virt machine. It is based on Linux-4.20-rc5
> and can be found at riscv_earlycon_v1 branch of:
> https://github.com/avpatel/linux.git
>
> Anup Patel (3):
> tty/serial: Add RISC-V SBI earlycon support
> RISC-V: defconfig: Enable RISC-V SBI earlycon support
> RISC-V: Remove EARLY_PRINTK support
>
> arch/riscv/Kconfig.debug | 2 --
> arch/riscv/configs/defconfig | 1 +
> arch/riscv/kernel/setup.c | 28 -------------------------
> drivers/tty/serial/Kconfig | 12 +++++++++++
> drivers/tty/serial/Makefile | 1 +
> drivers/tty/serial/earlycon-riscv-sbi.c | 28 +++++++++++++++++++++++++
> 6 files changed, 42 insertions(+), 30 deletions(-)
> create mode 100644 drivers/tty/serial/earlycon-riscv-sbi.c

I'm adding these to my for-next.

Thanks!

2018-12-07 18:33:45

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 3/3] RISC-V: Remove EARLY_PRINTK support

On Tue, 04 Dec 2018 05:55:07 PST (-0800), [email protected] wrote:
> The EARLY_PRINTK using SBI console calls is not required
> any more because we now have RISC-V SBI support in generic
> earlycon framework.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/Kconfig.debug | 2 --
> arch/riscv/kernel/setup.c | 28 ----------------------------
> 2 files changed, 30 deletions(-)
>
> diff --git a/arch/riscv/Kconfig.debug b/arch/riscv/Kconfig.debug
> index c5a72f17c469..e69de29bb2d1 100644
> --- a/arch/riscv/Kconfig.debug
> +++ b/arch/riscv/Kconfig.debug
> @@ -1,2 +0,0 @@
> -config EARLY_PRINTK
> - def_bool y
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 2c290e6aaa6e..fc8006a042eb 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -35,31 +35,9 @@
> #include <asm/sections.h>
> #include <asm/pgtable.h>
> #include <asm/smp.h>
> -#include <asm/sbi.h>
> #include <asm/tlbflush.h>
> #include <asm/thread_info.h>
>
> -#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]);
> - }
> -}
> -
> -struct console riscv_sbi_early_console_dev __initdata = {
> - .name = "early",
> - .write = sbi_console_write,
> - .flags = CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME,
> - .index = -1
> -};
> -#endif
> -
> #ifdef CONFIG_DUMMY_CONSOLE
> struct screen_info screen_info = {
> .orig_video_lines = 30,
> @@ -219,12 +197,6 @@ static void __init setup_bootmem(void)
>
> void __init setup_arch(char **cmdline_p)
> {
> -#if defined(CONFIG_EARLY_PRINTK)
> - if (likely(early_console == NULL)) {
> - early_console = &riscv_sbi_early_console_dev;
> - register_console(early_console);
> - }
> -#endif
> *cmdline_p = boot_command_line;
>
> parse_early_param();

Reviewed-by: Palmer Dabbelt <[email protected]>

2018-12-07 18:47:45

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 1/3] tty/serial: Add RISC-V SBI earlycon support

On Wed, 05 Dec 2018 01:58:46 PST (-0800), Greg KH wrote:
> On Tue, Dec 04, 2018 at 07:25:05PM +0530, Anup Patel wrote:
>> In RISC-V, the M-mode runtime firmware provide SBI calls for
>> debug prints. This patch adds earlycon support using RISC-V
>> SBI console calls. To enable it, just pass "earlycon=sbi" in
>> kernel parameters.
>>
>> Signed-off-by: Anup Patel <[email protected]>
>
> This makes more sense to take through the riscv tree, so feel free to
> add:
>
> Acked-by: Greg Kroah-Hartman <[email protected]>
>
> to it and take it that way.

It should be in my for-next now.

Thanks!

2019-01-10 14:09:02

by Andreas Schwab

[permalink] [raw]
Subject: [PATCH] tty/serial: emit CR before NL in RISC-V SBL console

Signed-off-by: Andreas Schwab <[email protected]>
---
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');
sbi_console_putchar(s[i]);
+ }
}

static int __init early_sbi_setup(struct earlycon_device *device,
--
2.20.1

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2019-01-10 15:28:34

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: emit CR before NL in RISC-V SBL console

On Jan 10 2019, Anup Patel <[email protected]> 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.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2019-01-10 20:17:04

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: emit CR before NL in RISC-V SBL console

On Thu, Jan 10, 2019 at 7:37 PM Andreas Schwab <[email protected]> wrote:
>
> Signed-off-by: Andreas Schwab <[email protected]>
> ---
> 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

2019-01-10 20:21:18

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: emit CR before NL in RISC-V SBL console

On Thu, Jan 10, 2019 at 8:56 PM Andreas Schwab <[email protected]> wrote:
>
> On Jan 10 2019, Anup Patel <[email protected]> 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

2019-01-10 21:35:41

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: emit CR before NL in RISC-V SBL console

On Thu, 10 Jan 2019 07:26:46 PST (-0800), [email protected] wrote:
> On Jan 10 2019, Anup Patel <[email protected]> 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), [email protected] wrote:
> On Thu, Jan 10, 2019 at 8:56 PM Andreas Schwab <[email protected]> wrote:
>>
>> On Jan 10 2019, Anup Patel <[email protected]> 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 :)

2019-01-10 22:18:19

by Andreas Schwab

[permalink] [raw]
Subject: [PATCH] tty/serial: use uart_console_write in the RISC-V SBL early console

This enables proper NLCR processing.

Suggested-by: Anup Patel <[email protected]>
Signed-off-by: Andreas Schwab <[email protected]>
---
drivers/tty/serial/earlycon-riscv-sbi.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/earlycon-riscv-sbi.c b/drivers/tty/serial/earlycon-riscv-sbi.c
index e1a551aae3..ce81523c31 100644
--- a/drivers/tty/serial/earlycon-riscv-sbi.c
+++ b/drivers/tty/serial/earlycon-riscv-sbi.c
@@ -10,13 +10,16 @@
#include <linux/serial_core.h>
#include <asm/sbi.h>

-static void sbi_console_write(struct console *con,
- const char *s, unsigned int n)
+static void sbi_putc(struct uart_port *port, int c)
{
- int i;
+ sbi_console_putchar(c);
+}

- for (i = 0; i < n; ++i)
- sbi_console_putchar(s[i]);
+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);
}

static int __init early_sbi_setup(struct earlycon_device *device,
--
2.20.1

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

2019-01-11 14:17:21

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: use uart_console_write in the RISC-V SBL early console

On Thu, Jan 10, 2019 at 10:41 PM Andreas Schwab <[email protected]> wrote:
>
> This enables proper NLCR processing.
>
> Suggested-by: Anup Patel <[email protected]>
> Signed-off-by: Andreas Schwab <[email protected]>
> ---
> drivers/tty/serial/earlycon-riscv-sbi.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/tty/serial/earlycon-riscv-sbi.c b/drivers/tty/serial/earlycon-riscv-sbi.c
> index e1a551aae3..ce81523c31 100644
> --- a/drivers/tty/serial/earlycon-riscv-sbi.c
> +++ b/drivers/tty/serial/earlycon-riscv-sbi.c
> @@ -10,13 +10,16 @@
> #include <linux/serial_core.h>
> #include <asm/sbi.h>
>
> -static void sbi_console_write(struct console *con,
> - const char *s, unsigned int n)
> +static void sbi_putc(struct uart_port *port, int c)
> {
> - int i;
> + sbi_console_putchar(c);
> +}
>
> - for (i = 0; i < n; ++i)
> - sbi_console_putchar(s[i]);
> +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);
> }
>
> static int __init early_sbi_setup(struct earlycon_device *device,
> --
> 2.20.1
>
> --
> Andreas Schwab, SUSE Labs, [email protected]
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Looks good to me.

Reviewed-by: Anup Patel <[email protected]>

Regards,
Anup

2019-01-15 17:18:51

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: use uart_console_write in the RISC-V SBL early console

On Thu, Jan 10, 2019 at 06:11:39PM +0100, Andreas Schwab wrote:
> This enables proper NLCR processing.
>
> Suggested-by: Anup Patel <[email protected]>
> Signed-off-by: Andreas Schwab <[email protected]>

Looks good,

Reviewed-by: Christoph Hellwig <[email protected]>

2019-01-23 23:59:08

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] tty/serial: use uart_console_write in the RISC-V SBL early console

On Fri, 11 Jan 2019 03:13:30 PST (-0800), [email protected] wrote:
> On Thu, Jan 10, 2019 at 10:41 PM Andreas Schwab <[email protected]> wrote:
>>
>> This enables proper NLCR processing.
>>
>> Suggested-by: Anup Patel <[email protected]>
>> Signed-off-by: Andreas Schwab <[email protected]>
>> ---
>> drivers/tty/serial/earlycon-riscv-sbi.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/tty/serial/earlycon-riscv-sbi.c b/drivers/tty/serial/earlycon-riscv-sbi.c
>> index e1a551aae3..ce81523c31 100644
>> --- a/drivers/tty/serial/earlycon-riscv-sbi.c
>> +++ b/drivers/tty/serial/earlycon-riscv-sbi.c
>> @@ -10,13 +10,16 @@
>> #include <linux/serial_core.h>
>> #include <asm/sbi.h>
>>
>> -static void sbi_console_write(struct console *con,
>> - const char *s, unsigned int n)
>> +static void sbi_putc(struct uart_port *port, int c)
>> {
>> - int i;
>> + sbi_console_putchar(c);
>> +}
>>
>> - for (i = 0; i < n; ++i)
>> - sbi_console_putchar(s[i]);
>> +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);
>> }
>>
>> static int __init early_sbi_setup(struct earlycon_device *device,
>> --
>> 2.20.1
>>
>> --
>> Andreas Schwab, SUSE Labs, [email protected]
>> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
>> "And now for something completely different."
>
> Looks good to me.
>
> Reviewed-by: Anup Patel <[email protected]>
>
> Regards,
> Anup

Thanks. I'm going to include this in my next PR, as it's pretty self contained
and is necessary to actually use this sanely.

2019-03-25 16:24:11

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH 0/3] RISC-V SBI earlycon

On Dez 04 2018, Anup Patel <[email protected]> wrote:

> This patchset adds RISC-V SBI earlycon and removes RISC-V EARLY_PRINTK.
>
> We should use earlycon over existing EARLY_PRINTK for SBI console because:
> 1. It's a more generic way of implementing early console for debugging
> 2. Current RISC-V EARLY_PRINTK is a compile-time option whereas earlycon
> is enabled at run-time via kernel parameters.
> 3. To use earlycon with SBI, we have to pass "earlycon=sbi" in kernel
> parameters. If earlycon kernel parameter is not provided then kernel
> boots much faster which is very useful in real-world RISC-V deployments.

Why doesn't this earlycon disable itself when the real console kicks in?

Andreas.

--
Andreas Schwab, SUSE Labs, [email protected]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."