Hi, There are a couple instances where your code is not formatted in the correct style [1]. You can use tools/checkpatch.pl to help you fix these.
[1] https://www.denx.de/wiki/U-Boot/CodingStyle On 5/20/20 1:33 AM, Kongou Hikari wrote: > - This patch supports debug serial and console from SBI syscall. > > Signed-off-by: Kongou Hikari <hik...@nucleisys.com> > --- > drivers/serial/Kconfig | 17 +++++ > drivers/serial/Makefile | 1 + > drivers/serial/serial_riscv_sbi.c | 104 ++++++++++++++++++++++++++++++ > 3 files changed, 122 insertions(+) > create mode 100644 drivers/serial/serial_riscv_sbi.c > > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig > index 90e3983170..60dcf9bc9a 100644 > --- a/drivers/serial/Kconfig > +++ b/drivers/serial/Kconfig > @@ -388,12 +388,20 @@ config DEBUG_UART_MTK > driver will be available until the real driver model serial is > running. > > + > +config DEBUG_UART_RISCV_SBI > + bool "RISC-V SBI CONSOLE" > + depends on RISCV_SBI_CONSOLE > + help > + Select this to enable a debug UART using RISC-V SBI console driver. > + > endchoice > > config DEBUG_UART_BASE > hex "Base address of UART" > depends on DEBUG_UART > default 0 if DEBUG_UART_SANDBOX > + default 0 if DEBUG_UART_RISCV_SBI > help > This is the base address of your UART for memory-mapped UARTs. > > @@ -404,6 +412,7 @@ config DEBUG_UART_CLOCK > int "UART input clock" > depends on DEBUG_UART > default 0 if DEBUG_UART_SANDBOX > + default 0 if DEBUG_UART_RISCV_SBI > help > The UART input clock determines the speed of the internal UART > circuitry. The baud rate is derived from this by dividing the input > @@ -481,6 +490,14 @@ config ALTERA_JTAG_UART_BYPASS > output will wait forever until a JTAG terminal is connected. If you > not are sure, say Y. > > +config RISCV_SBI_CONSOLE > + bool "RISC-V SBI console support" > + depends on RISCV > + help > + This enables support for console via RISC-V SBI calls. > + > + If you don't know what do to here, say Y. > + > config ALTERA_UART > bool "Altera UART support" > depends on DM_SERIAL > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile > index e4a92bbbb7..15b2a3ea6f 100644 > --- a/drivers/serial/Makefile > +++ b/drivers/serial/Makefile > @@ -46,6 +46,7 @@ obj-$(CONFIG_MXC_UART) += serial_mxc.o > obj-$(CONFIG_PXA_SERIAL) += serial_pxa.o > obj-$(CONFIG_MESON_SERIAL) += serial_meson.o > obj-$(CONFIG_INTEL_MID_SERIAL) += serial_intel_mid.o > +obj-$(CONFIG_RISCV_SBI_CONSOLE) += serial_riscv_sbi.o > ifdef CONFIG_SPL_BUILD > obj-$(CONFIG_ROCKCHIP_SERIAL) += serial_rockchip.o > endif > diff --git a/drivers/serial/serial_riscv_sbi.c > b/drivers/serial/serial_riscv_sbi.c > new file mode 100644 > index 0000000000..add11be04e > --- /dev/null > +++ b/drivers/serial/serial_riscv_sbi.c > @@ -0,0 +1,104 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2008 David Gibson, IBM Corporation > + * Copyright (C) 2012 Regents of the University of California > + * Copyright (C) 2020 Nuclei System Technologies > + * Copyright (C) 2020 Ruigang Wan <rg...@nucleisys.com> > + */ > + > +#include <common.h> The following includes should be sorted alphabetially > +#include <serial.h> > +#include <errno.h> > +#include <dm.h> > +#include <fdtdec.h> > + > +#include <asm/sbi.h> > + > + I believe it's conventional to put the debug uart at the end of a file. > +#ifdef CONFIG_DEBUG_UART_RISCV_SBI > + This include should probably be at the top and outside of this ifdef. > +#include <debug_uart.h> > + > + > +static inline void _debug_uart_init(void) > +{ Don't use C++-style comments, except in SPDX identifiers. > + //Nothing > +} > + > +static inline void _debug_uart_putc(int ch) > +{ > + sbi_console_putchar(ch); > +} > + > +DEBUG_UART_FUNCS > + > +#endif > + > +static int sbi_tty_pending_char = -1; > + > +static int sbi_tty_put(struct udevice *dev, const char ch) > +{ > + > + sbi_console_putchar(ch); > + > + return 0; > +} > + > +static int sbi_tty_get(struct udevice *dev) > +{ > + int c; There should be a blank line here. > + if (sbi_tty_pending_char != -1) Opening braces go on the same line as statement; the only exception are functions. For example, if (foo) { } else { } > + { > + c = sbi_tty_pending_char; > + sbi_tty_pending_char = -1; > + } > + else > + { > + c = sbi_console_getchar(); > + if (c < 0) > + return -EAGAIN; > + } > + > + return c; > +} > + > +static int sbi_tty_setbrg(struct udevice *dev, int baudrate) > +{ > + return 0; > +} > + > +static int sbi_tty_pending(struct udevice *dev, bool input) > +{ > + int c; > + if (input) > + { > + if (sbi_tty_pending_char != -1) > + return 1; > + > + c = sbi_console_getchar(); > + if(c < 0) > + return 0; > + sbi_tty_pending_char = c; > + return 1; > + } > + return 0; > +} > + > +static const struct udevice_id serial_riscv_sbi_ids[] = { > + { .compatible = "sbi,console" }, > + { } > +}; > + This should also be static. > +const struct dm_serial_ops serial_riscv_sbi_ops = { > + .putc = sbi_tty_put, > + .pending = sbi_tty_pending, > + .getc = sbi_tty_get, > + .setbrg = sbi_tty_setbrg, > +}; > + > +U_BOOT_DRIVER(serial_riscv_sbi) = { > + .name = "serial_riscv_sbi", > + .id = UCLASS_SERIAL, > + .of_match = serial_riscv_sbi_ids, > + .ops = &serial_riscv_sbi_ops, > +}; > --Sean
