Early console buffer for OpenSBI

This series tries to improve OpenSBI debugging by:
1) Registering console device via platform early_init()
2) Introducing early console buffer

These patches can also be found in the console_early_probe_v1 branch at: https://github.com/avpatel/opensbi.git

Anup Patel (6):
? platform: Setup serial console device in early_init()
? lib: sbi: Remove sbi_console_init() and console_init() platform ??? callback
? lib: sbi: Optimize fifo enqueue/dequeue for basic data types ? lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue() ? include: sbi: Add macros to create FIFO as local or global variable ? lib: sbi: Introduce an early console buffer for caching early prints

?include/sbi/sbi_console.h????????? |? 2 -
?include/sbi/sbi_fifo.h???????????? | 14 +++++-
?include/sbi/sbi_platform.h???????? | 17 -------
?lib/sbi/Kconfig??????????????????? |? 6 ++-
?lib/sbi/sbi_console.c????????????? | 36 ++++++++++-----
?lib/sbi/sbi_fifo.c???????????????? | 72 +++++++++++++++++++++++++-----
?lib/sbi/sbi_init.c???????????????? |? 4 --
?lib/sbi/sbi_sse.c????????????????? |? 2 +-
?lib/sbi/sbi_tlb.c????????????????? |? 3 +-
?platform/fpga/ariane/platform.c??? | 26 ++++-------
?platform/fpga/openpiton/platform.c | 20 ++-------
?platform/generic/platform.c??????? | 21 ++++-----
?platform/kendryte/k210/platform.c? | 18 +++-----
?platform/nuclei/ux600/platform.c?? | 18 +++-----
?platform/template/platform.c?????? | 18 +++-----

?15 files changed, 151 insertions(+), 126 deletions(-)

Signature · 1 line

2.34.1

? 2024-07-05???? 12:44 +0530?Anup Patel???

Quoted 7 lines

This series tries to improve OpenSBI debugging by:
1) Registering console device via platform early_init()
2) Introducing early console buffer

These patches can also be found in the console_early_probe_v1 branch at: https://github.com/avpatel/opensbi.git

Quoted 9 lines

Anup Patel (6):
platform: Setup serial console device in early_init()
lib: sbi: Remove sbi_console_init() and console_init() platform
callback
lib: sbi: Optimize fifo enqueue/dequeue for basic data types
lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue()
include: sbi: Add macros to create FIFO as local or global variable
lib: sbi: Introduce an early console buffer for caching early prints

Hi, Anup

Before the console is initialized, sbi_printf is only called if an error occurs, and an error usually causes the program to enter sbi_hart_hang, resulting in no output. This makes adding an early console buffer less useful. The program will crash anyway, and we don't need to care about the order of function calls, we should just output some useful information as much as possible.

Regards,
Xiang W

Quoted 21 lines

 include/sbi/sbi_console.h          |  2 -
 include/sbi/sbi_fifo.h             | 14 +++++-
 include/sbi/sbi_platform.h         | 17 -------
 lib/sbi/Kconfig                    |  6 ++-
 lib/sbi/sbi_console.c              | 36 ++++++++++-----
 lib/sbi/sbi_fifo.c                 | 72 +++++++++++++++++++++++++-----
 lib/sbi/sbi_init.c                 |  4 --
 lib/sbi/sbi_sse.c                  |  2 +-
 lib/sbi/sbi_tlb.c                  |  3 +-
 platform/fpga/ariane/platform.c    | 26 ++++-------
 platform/fpga/openpiton/platform.c | 20 ++-------
 platform/generic/platform.c        | 21 ++++-----
 platform/kendryte/k210/platform.c  | 18 +++-----
 platform/nuclei/ux600/platform.c   | 18 +++-----
 platform/template/platform.c       | 18 +++-----
 15 files changed, 151 insertions(+), 126 deletions(-)

--
2.34.1

Don't use sbi_memcpy() for basic data types in fifo enqueue/dequeue instead use direct type-cast and assignment.

Signed-off-by Anup Patel <[email protected]>

---
 lib/sbi/sbi_fifo.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

lib/sbi/sbi_fifo.c +40 -4

index 72c6d39..9199a30 100644
@@ -66,7 +66,26 @@ static inline void  __sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
        if (head >= fifo->num_entries)
                head = head - fifo->num_entries;
 
-       sbi_memcpy((char *)fifo->queue + head * fifo->entry_size, data, fifo->entry_size);
+       switch (fifo->entry_size) {
+       case 1:
+               *(char *)(fifo->queue + head * fifo->entry_size) = *(char *)data;
+               break;
+       case 2:
+               *(u16 *)(fifo->queue + head * fifo->entry_size) = *(u16 *)data;
+               break;
+       case 4:
+               *(u32 *)(fifo->queue + head * fifo->entry_size) = *(u32 *)data;
+               break;
+#if __riscv_xlen > 32
+       case 8:
+               *(u64 *)(fifo->queue + head * fifo->entry_size) = *(u64 *)data;
+               break;
+#endif
+       default:
+               sbi_memcpy(fifo->queue + head * fifo->entry_size,
+                          data, fifo->entry_size);
+               break;
+       }
 
        fifo->avail++;
 }

@@ -184,8 +203,26 @@ int sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data)
                return SBI_ENOENT;
        }
 
-       sbi_memcpy(data, (char *)fifo->queue + (u32)fifo->tail * fifo->entry_size,
-              fifo->entry_size);
+       switch (fifo->entry_size) {
+       case 1:
+               *(char *)data = *(char *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+               break;
+       case 2:
+               *(u16 *)data = *(u16 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+               break;
+       case 4:
+               *(u32 *)data = *(u32 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+               break;
+#if __riscv_xlen > 32
+       case 8:
+               *(u64 *)data = *(u64 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+               break;
+#endif
+       default:
+               sbi_memcpy(data, fifo->queue + (u32)fifo->tail * fifo->entry_size,
+                          fifo->entry_size);
+               break;
+       }
 
        fifo->avail--;
        fifo->tail++;
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Extend sbi_fifo_enqueue() to allow forceful queueing by droping data from the tail.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi/sbi_fifo.h |  2 +-
 lib/sbi/sbi_fifo.c     | 69 +++++++++++++++++++++++++-----------------
 lib/sbi/sbi_sse.c      |  2 +-
 lib/sbi/sbi_tlb.c      |  3 +-
 4 files changed, 45 insertions(+), 31 deletions(-)

include/sbi/sbi_fifo.h +1 -1

index 1a85f07..af1632a 100644
@@ -30,7 +30,7 @@ enum sbi_fifo_inplace_update_types {
 };
 
 int sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data);
-int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data);
+int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data, bool force);
 void sbi_fifo_init(struct sbi_fifo *fifo, void *queue_mem, u16 entries,
                   u16 entry_size);
 int sbi_fifo_is_empty(struct sbi_fifo *fifo);

lib/sbi/sbi_fifo.c +41 -28

index 9199a30..d07ebff 100644
@@ -90,6 +90,39 @@ static inline void  __sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
        fifo->avail++;
 }
 
+/* Note: must be called with fifo->qlock held */
+static inline void  __sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data)
+{
+       if (!data)
+               goto skip_data_copy;
+
+       switch (fifo->entry_size) {
+       case 1:
+               *(char *)data = *(char *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+               break;
+       case 2:
+               *(u16 *)data = *(u16 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+               break;
+       case 4:
+               *(u32 *)data = *(u32 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+               break;
+#if __riscv_xlen > 32
+       case 8:
+               *(u64 *)data = *(u64 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
+               break;
+#endif
+       default:
+               sbi_memcpy(data, fifo->queue + (u32)fifo->tail * fifo->entry_size,
+                          fifo->entry_size);
+               break;
+       }
+
+skip_data_copy:
+       fifo->avail--;
+       fifo->tail++;
+       if (fifo->tail >= fifo->num_entries)
+               fifo->tail = 0;
+}
 
 /* Note: must be called with fifo->qlock held */
 static inline bool __sbi_fifo_is_empty(struct sbi_fifo *fifo)

@@ -173,7 +206,7 @@ int sbi_fifo_inplace_update(struct sbi_fifo *fifo, void *in,
        return ret;
 }
 
-int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
+int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data, bool force)
 {
        if (!fifo || !data)
                return SBI_EINVAL;

@@ -181,9 +214,13 @@ int sbi_fifo_enqueue(struct sbi_fifo *fifo, void *data)
        spin_lock(&fifo->qlock);
 
        if (__sbi_fifo_is_full(fifo)) {
-               spin_unlock(&fifo->qlock);
-               return SBI_ENOSPC;
+               if (!force) {
+                       spin_unlock(&fifo->qlock);
+                       return SBI_ENOSPC;
+               }
+               __sbi_fifo_dequeue(fifo, NULL);
        }
+
        __sbi_fifo_enqueue(fifo, data);
 
        spin_unlock(&fifo->qlock);

@@ -203,31 +240,7 @@ int sbi_fifo_dequeue(struct sbi_fifo *fifo, void *data)
                return SBI_ENOENT;
        }
 
-       switch (fifo->entry_size) {
-       case 1:
-               *(char *)data = *(char *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
-               break;
-       case 2:
-               *(u16 *)data = *(u16 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
-               break;
-       case 4:
-               *(u32 *)data = *(u32 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
-               break;
-#if __riscv_xlen > 32
-       case 8:
-               *(u64 *)data = *(u64 *)(fifo->queue + (u32)fifo->tail * fifo->entry_size);
-               break;
-#endif
-       default:
-               sbi_memcpy(data, fifo->queue + (u32)fifo->tail * fifo->entry_size,
-                          fifo->entry_size);
-               break;
-       }
-
-       fifo->avail--;
-       fifo->tail++;
-       if (fifo->tail >= fifo->num_entries)
-               fifo->tail = 0;
+       __sbi_fifo_dequeue(fifo, data);
 
        spin_unlock(&fifo->qlock);
 

lib/sbi/sbi_sse.c +1 -1

index e39963f..fe36a64 100644
@@ -667,7 +667,7 @@ static int sse_ipi_inject_send(unsigned long hartid, uint32_t event_id)
        sse_inject_fifo_r =
                sbi_scratch_offset_ptr(remote_scratch, sse_inject_fifo_off);
 
-       ret = sbi_fifo_enqueue(sse_inject_fifo_r, &evt);
+       ret = sbi_fifo_enqueue(sse_inject_fifo_r, &evt, false);
        if (ret)
                return SBI_EFAIL;
 

lib/sbi/sbi_tlb.c +2 -2

index cca319f..01b31f4 100644
@@ -351,7 +351,8 @@ static int tlb_update(struct sbi_scratch *scratch,
 
        ret = sbi_fifo_inplace_update(tlb_fifo_r, data, tlb_update_cb);
 
-       if (ret == SBI_FIFO_UNCHANGED && sbi_fifo_enqueue(tlb_fifo_r, data) < 0) {
+       if (ret == SBI_FIFO_UNCHANGED &&
+           sbi_fifo_enqueue(tlb_fifo_r, data, false) < 0) {
                /**
                 * For now, Busy loop until there is space in the fifo.
                 * There may be case where target hart is also
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

The FIFO data structure is quite handy of variety of use-case so add SBI_FIFO_INITIALIZER() and SBI_FIFO_DEFINE() helper macros to create FIFO as local or global variable.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi/sbi_fifo.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

include/sbi/sbi_fifo.h +12 -1

index af1632a..89a2ea2 100644
@@ -23,6 +23,18 @@ struct sbi_fifo {
        u16 tail;
 };
 
+#define SBI_FIFO_INITIALIZER(__queue_mem, __entries, __entry_size)     \
+{      .queue = __queue_mem,                                           \
+       .qlock = SPIN_LOCK_INITIALIZER,                                 \
+       .num_entries = __entries,                                       \
+       .entry_size = __entry_size,                                     \
+       .avail = 0,                                                     \
+       .tail = 0,                                                      \
+}
+
+#define SBI_FIFO_DEFINE(__name, __queue_mem, __entries, __entry_size)  \
+struct sbi_fifo __name = SBI_FIFO_INITIALIZER(__queue_mem, __entries, __entry_size)
+
 enum sbi_fifo_inplace_update_types {
        SBI_FIFO_SKIP,
        SBI_FIFO_UPDATED,
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

The console device is registered by platform only in early_init() callback so any prints before this point will be lost. Introduce an early console buffer for caching prints before platform early_init().

For crashes before platform early_init(), users can simply dump the contents of the console_early_buffer[] string using a debugger. The relative address of the console_early_buffer[] string can be found using following two commands:

CONSOLE_EARLY_FIFO_ADDR=`${CROSS_COMPILE}objdump -D \
build/platform/generic/firmware/fw_dynamic.elf | \
grep "<console_early_fifo>:" | awk '{print $1}'`

${CROSS_COMPILE}objdump -R build/platform/generic/firmware/fw_dynamic.elf | \
grep $CONSOLE_EARLY_FIFO_ADDR | awk '{print $3}'

Signed-off-by Anup Patel <[email protected]>

---
 lib/sbi/Kconfig       |  6 +++++-
 lib/sbi/sbi_console.c | 27 +++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

lib/sbi/Kconfig +5 -1

index 6cf54ce..bd8ba2b 100644
@@ -1,6 +1,10 @@
 # SPDX-License-Identifier: BSD-2-Clause
 
-menu "SBI Extension Support"
+menu "Generic SBI Support"
+
+config CONSOLE_EARLY_BUFFER_SIZE
+       int "Early console buffer size (bytes)"
+       default 256
 
 config SBI_ECALL_TIME
        bool "Timer extension"

lib/sbi/sbi_console.c +27 -1

index 194529d..d760885 100644
@@ -9,6 +9,7 @@
 
 #include <sbi/riscv_locks.h>
 #include <sbi/sbi_console.h>
+#include <sbi/sbi_fifo.h>
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_platform.h>
 #include <sbi/sbi_scratch.h>

@@ -21,6 +22,15 @@ static char console_tbuf[CONSOLE_TBUF_MAX];
 static u32 console_tbuf_len;
 static spinlock_t console_out_lock            = SPIN_LOCK_INITIALIZER;
 
+#ifdef CONFIG_CONSOLE_EARLY_BUFFER_SIZE
+#define CONSOLE_EARLY_BUFFER_SIZE      CONFIG_CONSOLE_EARLY_BUFFER_SIZE
+#else
+#define CONSOLE_EARLY_BUFFER_SIZE      256
+#endif
+static char console_early_buffer[CONSOLE_EARLY_BUFFER_SIZE] = { 0 };
+static SBI_FIFO_DEFINE(console_early_fifo, console_early_buffer, \
+                      CONSOLE_EARLY_BUFFER_SIZE, sizeof(char));
+
 bool sbi_isprintable(char c)
 {
        if (((31 < c) && (c < 127)) || (c == '\f') || (c == '\r') ||

@@ -39,6 +49,7 @@ int sbi_getc(void)
 
 static unsigned long nputs(const char *str, unsigned long len)
 {
+       char ch;
        unsigned long i;
 
        if (console_dev) {

@@ -51,6 +62,11 @@ static unsigned long nputs(const char *str, unsigned long len)
                                console_dev->console_putc(str[i]);
                        }
                }
+       } else {
+               for (i = 0; i < len; i++) {
+                       ch = str[i];
+                       sbi_fifo_enqueue(&console_early_fifo, &ch, true);
+               }
        }
        return len;
 }

@@ -472,8 +488,19 @@ const struct sbi_console_device *sbi_console_get_device(void)
 
 void sbi_console_set_device(const struct sbi_console_device *dev)
 {
+       char ch;
+       bool flush_early_fifo = false;
+
        if (!dev)
                return;
 
+       if (!console_dev)
+               flush_early_fifo = true;
+
        console_dev = dev;
+
+       if (flush_early_fifo) {
+               while (!sbi_fifo_dequeue(&console_early_fifo, &ch))
+                       sbi_putc(ch);
+       }
 }
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

The sbi_console_init() does not do any special initialization so setup serial console device in early_init() so that console prints work as early as possible.

Signed-off-by Anup Patel <[email protected]>

---
 platform/fpga/ariane/platform.c    | 26 +++++++++-----------------
 platform/fpga/openpiton/platform.c | 20 ++++----------------
 platform/generic/platform.c        | 21 +++++++++++----------
 platform/kendryte/k210/platform.c  | 18 ++++++------------
 platform/nuclei/ux600/platform.c   | 18 +++++++-----------
 platform/template/platform.c       | 18 ++++++------------
 6 files changed, 43 insertions(+), 78 deletions(-)

platform/fpga/ariane/platform.c +9 -17

index 8be5e6c..8d46d88 100644
@@ -7,7 +7,6 @@
 #include <sbi/riscv_asm.h>
 #include <sbi/riscv_encoding.h>
 #include <sbi/riscv_io.h>
-#include <sbi/sbi_console.h>
 #include <sbi/sbi_const.h>
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_platform.h>

@@ -67,8 +66,15 @@ static struct aclint_mtimer_data mtimer = {
  */
 static int ariane_early_init(bool cold_boot)
 {
-       /* For now nothing to do. */
-       return 0;
+       if (!cold_boot)
+               return 0;
+
+       return uart8250_init(ARIANE_UART_ADDR,
+                            ARIANE_UART_FREQ,
+                            ARIANE_UART_BAUDRATE,
+                            ARIANE_UART_REG_SHIFT,
+                            ARIANE_UART_REG_WIDTH,
+                            ARIANE_UART_REG_OFFSET);
 }
 
 /*

@@ -87,19 +93,6 @@ static int ariane_final_init(bool cold_boot)
        return 0;
 }
 
-/*
- * Initialize the ariane console.
- */
-static int ariane_console_init(void)
-{
-       return uart8250_init(ARIANE_UART_ADDR,
-                            ARIANE_UART_FREQ,
-                            ARIANE_UART_BAUDRATE,
-                            ARIANE_UART_REG_SHIFT,
-                            ARIANE_UART_REG_WIDTH,
-                            ARIANE_UART_REG_OFFSET);
-}
-
 static int plic_ariane_warm_irqchip_init(int m_cntx_id, int s_cntx_id)
 {
        int ret;

@@ -175,7 +168,6 @@ static int ariane_timer_init(bool cold_boot)
 const struct sbi_platform_operations platform_ops = {
        .early_init = ariane_early_init,
        .final_init = ariane_final_init,
-       .console_init = ariane_console_init,
        .irqchip_init = ariane_irqchip_init,
        .ipi_init = ariane_ipi_init,
        .timer_init = ariane_timer_init,

platform/fpga/openpiton/platform.c +4 -16

index 2317a89..dd74a0c 100644
@@ -6,7 +6,6 @@
 #include <sbi/riscv_asm.h>
 #include <sbi/riscv_encoding.h>
 #include <sbi/riscv_io.h>
-#include <sbi/sbi_console.h>
 #include <sbi/sbi_const.h>
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_platform.h>

@@ -103,7 +102,10 @@ static int openpiton_early_init(bool cold_boot)
                                    ACLINT_DEFAULT_MTIMECMP_OFFSET;
        }
 
-       return 0;
+       return uart8250_init(uart.addr, uart.freq, uart.baud,
+                            OPENPITON_DEFAULT_UART_REG_SHIFT,
+                            OPENPITON_DEFAULT_UART_REG_WIDTH,
+                            OPENPITON_DEFAULT_UART_REG_OFFSET);
 }
 
 /*

@@ -122,19 +124,6 @@ static int openpiton_final_init(bool cold_boot)
        return 0;
 }
 
-/*
- * Initialize the openpiton console.
- */
-static int openpiton_console_init(void)
-{
-       return uart8250_init(uart.addr,
-                            uart.freq,
-                            uart.baud,
-                            OPENPITON_DEFAULT_UART_REG_SHIFT,
-                            OPENPITON_DEFAULT_UART_REG_WIDTH,
-                            OPENPITON_DEFAULT_UART_REG_OFFSET);
-}
-
 static int plic_openpiton_warm_irqchip_init(int m_cntx_id, int s_cntx_id)
 {
        int ret;

@@ -210,7 +199,6 @@ static int openpiton_timer_init(bool cold_boot)
 const struct sbi_platform_operations platform_ops = {
        .early_init = openpiton_early_init,
        .final_init = openpiton_final_init,
-       .console_init = openpiton_console_init,
        .irqchip_init = openpiton_irqchip_init,
        .ipi_init = openpiton_ipi_init,
        .timer_init = openpiton_timer_init,

platform/generic/platform.c +11 -10

index 1f46b76..3470474 100644
@@ -221,9 +221,19 @@ static int generic_nascent_init(void)
 
 static int generic_early_init(bool cold_boot)
 {
-       if (cold_boot)
+       int rc;
+
+       if (cold_boot) {
                fdt_reset_init();
 
+               if (semihosting_enabled())
+                       rc = semihosting_init();
+               else
+                       rc = fdt_serial_init();
+               if (rc)
+                       return rc;
+       }
+
        if (!generic_plat || !generic_plat->early_init)
                return 0;
 

@@ -378,14 +388,6 @@ static uint64_t generic_pmu_xlate_to_mhpmevent(uint32_t event_idx,
        return evt_val;
 }
 
-static int generic_console_init(void)
-{
-       if (semihosting_enabled())
-               return semihosting_init();
-       else
-               return fdt_serial_init();
-}
-
 const struct sbi_platform_operations platform_ops = {
        .cold_boot_allowed      = generic_cold_boot_allowed,
        .nascent_init           = generic_nascent_init,

@@ -395,7 +397,6 @@ const struct sbi_platform_operations platform_ops = {
        .final_exit             = generic_final_exit,
        .extensions_init        = generic_extensions_init,
        .domains_init           = generic_domains_init,
-       .console_init           = generic_console_init,
        .irqchip_init           = fdt_irqchip_init,
        .irqchip_exit           = fdt_irqchip_exit,
        .ipi_init               = fdt_ipi_init,

platform/kendryte/k210/platform.c +6 -12

index 27b23f7..531adcd 100644
@@ -9,7 +9,6 @@
 
 #include <sbi/riscv_asm.h>
 #include <sbi/riscv_encoding.h>
-#include <sbi/sbi_console.h>
 #include <sbi/sbi_const.h>
 #include <sbi/sbi_platform.h>
 #include <sbi/sbi_system.h>

@@ -109,10 +108,13 @@ static struct sbi_system_reset_device k210_reset = {
 
 static int k210_early_init(bool cold_boot)
 {
-       if (cold_boot)
-               sbi_system_reset_add_device(&k210_reset);
+       if (!cold_boot)
+               return 0;
 
-       return 0;
+       sbi_system_reset_add_device(&k210_reset);
+
+       return sifive_uart_init(K210_UART_BASE_ADDR, k210_get_clk_freq(),
+                               K210_UART_BAUDRATE);
 }
 
 static int k210_final_init(bool cold_boot)

@@ -130,12 +132,6 @@ static int k210_final_init(bool cold_boot)
        return 0;
 }
 
-static int k210_console_init(void)
-{
-       return sifive_uart_init(K210_UART_BASE_ADDR, k210_get_clk_freq(),
-                               K210_UART_BAUDRATE);
-}
-
 static int k210_irqchip_init(bool cold_boot)
 {
        int rc;

@@ -181,8 +177,6 @@ const struct sbi_platform_operations platform_ops = {
 
        .final_init     = k210_final_init,
 
-       .console_init   = k210_console_init,
-
        .irqchip_init = k210_irqchip_init,
 
        .ipi_init  = k210_ipi_init,

platform/nuclei/ux600/platform.c +7 -11

index f688b50..2289366 100644
@@ -11,7 +11,6 @@
 #include <sbi/riscv_asm.h>
 #include <sbi/riscv_io.h>
 #include <sbi/riscv_encoding.h>
-#include <sbi/sbi_console.h>
 #include <sbi/sbi_const.h>
 #include <sbi/sbi_platform.h>
 #include <sbi/sbi_system.h>

@@ -150,8 +149,10 @@ static int ux600_early_init(bool cold_boot)
 {
        u32 regval;
 
-       if (cold_boot)
-               sbi_system_reset_add_device(&ux600_reset);
+       if (!cold_boot)
+               return 0;
+
+       sbi_system_reset_add_device(&ux600_reset);
 
        /* Measure CPU Frequency using Timer */
        ux600_clk_freq = ux600_get_clk_freq();

@@ -163,7 +164,9 @@ static int ux600_early_init(bool cold_boot)
        regval = readl((void *)(UX600_GPIO_ADDR + UX600_GPIO_IOF_EN_OFS)) |
                UX600_GPIO_IOF_UART0_MASK;
        writel(regval, (void *)(UX600_GPIO_ADDR + UX600_GPIO_IOF_EN_OFS));
-       return 0;
+
+       return sifive_uart_init(UX600_DEBUG_UART, ux600_clk_freq,
+                               UX600_UART_BAUDRATE);
 }
 
 static void ux600_modify_dt(void *fdt)

@@ -184,12 +187,6 @@ static int ux600_final_init(bool cold_boot)
        return 0;
 }
 
-static int ux600_console_init(void)
-{
-       return sifive_uart_init(UX600_DEBUG_UART, ux600_clk_freq,
-                               UX600_UART_BAUDRATE);
-}
-
 static int ux600_irqchip_init(bool cold_boot)
 {
        int rc;

@@ -234,7 +231,6 @@ static int ux600_timer_init(bool cold_boot)
 const struct sbi_platform_operations platform_ops = {
        .early_init             = ux600_early_init,
        .final_init             = ux600_final_init,
-       .console_init           = ux600_console_init,
        .irqchip_init           = ux600_irqchip_init,
        .ipi_init               = ux600_ipi_init,
        .timer_init             = ux600_timer_init,

platform/template/platform.c +6 -13

index 4b3f2ac..b4d30a5 100644
@@ -64,7 +64,12 @@ static struct aclint_mtimer_data mtimer = {
  */
 static int platform_early_init(bool cold_boot)
 {
-       return 0;
+       if (!cold_boot)
+               return 0;
+
+       /* Example if the generic UART8250 driver is used */
+       return uart8250_init(PLATFORM_UART_ADDR, PLATFORM_UART_INPUT_FREQ,
+                            PLATFORM_UART_BAUDRATE, 0, 1, 0);
 }
 
 /*

@@ -75,16 +80,6 @@ static int platform_final_init(bool cold_boot)
        return 0;
 }
 
-/*
- * Initialize the platform console.
- */
-static int platform_console_init(void)
-{
-       /* Example if the generic UART8250 driver is used */
-       return uart8250_init(PLATFORM_UART_ADDR, PLATFORM_UART_INPUT_FREQ,
-                            PLATFORM_UART_BAUDRATE, 0, 1, 0);
-}
-
 /*
  * Initialize the platform interrupt controller for current HART.
  */

@@ -143,7 +138,6 @@ static int platform_timer_init(bool cold_boot)
 const struct sbi_platform_operations platform_ops = {
        .early_init             = platform_early_init,
        .final_init             = platform_final_init,
-       .console_init           = platform_console_init,
        .irqchip_init           = platform_irqchip_init,
        .ipi_init               = platform_ipi_init,
        .timer_init             = platform_timer_init
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Now that all platforms have been updated to initialize serial console device in early_init(), the sbi_console_init() and console_init() platform callback are redundant hence remove them.

Signed-off-by Anup Patel <[email protected]>

---
 include/sbi/sbi_console.h  |  2 --
 include/sbi/sbi_platform.h | 17 -----------------
 lib/sbi/sbi_console.c      | 11 -----------
 lib/sbi/sbi_init.c         |  4 ----
 4 files changed, 34 deletions(-)

include/sbi/sbi_console.h +0 -2

index 0979765..3097db1 100644
@@ -58,8 +58,6 @@ void sbi_console_set_device(const struct sbi_console_device *dev);
 
 struct sbi_scratch;
 
-int sbi_console_init(struct sbi_scratch *scratch);
-
 #define SBI_ASSERT(cond, args) do { \
        if (unlikely(!(cond))) \
                sbi_panic args; \

include/sbi/sbi_platform.h +0 -17

index 581935a..7b3ac4b 100644
@@ -111,9 +111,6 @@ struct sbi_platform_operations {
        /** Get platform specific mhpmevent value */
        uint64_t (*pmu_xlate_to_mhpmevent)(uint32_t event_idx, uint64_t data);
 
-       /** Initialize the platform console */
-       int (*console_init)(void);
-
        /** Initialize the platform interrupt controller for current HART */
        int (*irqchip_init)(bool cold_boot);
        /** Exit the platform interrupt controller for current HART */

@@ -549,20 +546,6 @@ static inline uint64_t sbi_platform_pmu_xlate_to_mhpmevent(const struct sbi_plat
        return 0;
 }
 
-/**
- * Initialize the platform console
- *
- * @param plat pointer to struct sbi_platform
- *
- * @return 0 on success and negative error code on failure
- */
-static inline int sbi_platform_console_init(const struct sbi_platform *plat)
-{
-       if (plat && sbi_platform_ops(plat)->console_init)
-               return sbi_platform_ops(plat)->console_init();
-       return 0;
-}
-
 /**
  * Initialize the platform interrupt controller for current HART
  *

lib/sbi/sbi_console.c +0 -11

index d3ec461..194529d 100644
@@ -477,14 +477,3 @@ void sbi_console_set_device(const struct sbi_console_device *dev)
 
        console_dev = dev;
 }
-
-int sbi_console_init(struct sbi_scratch *scratch)
-{
-       int rc = sbi_platform_console_init(sbi_platform_ptr(scratch));
-
-       /* console is not a necessary device */
-       if (rc == SBI_ENODEV)
-               return 0;
-
-       return rc;
-}

lib/sbi/sbi_init.c +0 -5

index 0f9e14c..d80efe9 100644
@@ -261,10 +261,6 @@ static void __noreturn init_coldboot(struct sbi_scratch *scratch, u32 hartid)
        if (rc)
                sbi_hart_hang();
 
-       rc = sbi_console_init(scratch);
-       if (rc)
-               sbi_hart_hang();
-
        rc = sbi_sse_init(scratch, true);
        if (rc) {
                sbi_printf("%s: sse init failed (error %d)\n", __func__, rc);
--

2.34.1

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

This series tries to improve OpenSBI debugging by:
1) Registering console device via platform early_init()
2) Introducing early console buffer

These patches can also be found in the console_early_probe_v1 branch at: https://github.com/avpatel/opensbi.git

Anup Patel (6):
platform: Setup serial console device in early_init()
lib: sbi: Remove sbi_console_init() and console_init() platform
callback
lib: sbi: Optimize fifo enqueue/dequeue for basic data types
lib: sbi: Allow forceful queueing of data in sbi_fifo_enqueue()
include: sbi: Add macros to create FIFO as local or global variable
lib: sbi: Introduce an early console buffer for caching early prints

 include/sbi/sbi_console.h          |  2 -
 include/sbi/sbi_fifo.h             | 14 +++++-
 include/sbi/sbi_platform.h         | 17 -------
 lib/sbi/Kconfig                    |  6 ++-
 lib/sbi/sbi_console.c              | 36 ++++++++++-----
 lib/sbi/sbi_fifo.c                 | 72 +++++++++++++++++++++++++-----
 lib/sbi/sbi_init.c                 |  4 --
 lib/sbi/sbi_sse.c                  |  2 +-
 lib/sbi/sbi_tlb.c                  |  3 +-
 platform/fpga/ariane/platform.c    | 26 ++++-------
 platform/fpga/openpiton/platform.c | 20 ++-------
 platform/generic/platform.c        | 21 ++++-----
 platform/kendryte/k210/platform.c  | 18 +++-----
 platform/nuclei/ux600/platform.c   | 18 +++-----
 platform/template/platform.c       | 18 +++-----
 15 files changed, 151 insertions(+), 126 deletions(-)

--
2.34.1