[PATCH] lib: utils: Add Xilinx uartlite implementation
chenguokai17
chenguokai17 at mails.ucas.ac.cn
Wed Mar 31 16:33:28 BST 2021
在 2021-03-31 10:33:20， "Bin Meng" <bmeng.cn at gmail.com> 写道：
>On Tue, Mar 30, 2021 at 11:01 AM Guokai Chen <chenguokai123 at gmail.com> wrote:
>>
>> Uartlite is a device commonly used on Xilinx platforms. We add OpenSBI support for uartlite so that OpenSBI boots on SoCs that utilize uartlite for output.
>
>Please wrap the commit message at ~70 characters a line
>
>>
>> Signed-off-by: Guokai Chen <chenguokai17 at mails.ucas.ac.cn>
>> ---
>> include/sbi_utils/serial/xilinx-uart.h | 20 ++++++++++++
>> lib/utils/serial/fdt_serial.c | 2 ++
>> lib/utils/serial/fdt_serial_xilinx.c | 36 ++++++++++++++++++++++
>> lib/utils/serial/objects.mk | 2 ++
>> lib/utils/serial/xilinx-uart.c | 42 ++++++++++++++++++++++++++
>> 5 files changed, 102 insertions(+)
>> create mode 100644 include/sbi_utils/serial/xilinx-uart.h
>> create mode 100644 lib/utils/serial/fdt_serial_xilinx.c
>> create mode 100644 lib/utils/serial/xilinx-uart.c
>>
>> diff --git a/include/sbi_utils/serial/xilinx-uart.h b/include/sbi_utils/serial/xilinx-uart.h
>> new file mode 100644
>> index 0000000..e3d667e
>> --- /dev/null
>> +++ b/include/sbi_utils/serial/xilinx-uart.h
>
>The name suggests you are adding Xilinx UART driver, instead of UART
>lite driver. Are the same the IP?
>
>> @@ -0,0 +1,20 @@
>> +/*
>> + * SPDX-License-Identifier: BSD-2-Clause
>> + *
>> + * Copyright (c) 2021 Guokai Chen <chenguokai17 at mails.ucas.ac.cn>
>> + *
>> + * This code is based on include/sbi_utils/serial/shakti-uart.h
>> + */
>> +
>> +#ifndef __SERIAL_XILINX_UART_H__
>> +#define __SERIAL_XILINX_UART_H__
>> +
>> +#include <sbi/sbi_types.h>
>> +
>> +void xilinx_uart_putc(char ch);
>> +
>> +int xilinx_uart_getc(void);
>> +
>> +int xilinx_uart_init(unsigned long base, u32 in_freq, u32 baudrate);
>> +
>> +#endif
>> diff --git a/lib/utils/serial/fdt_serial.c b/lib/utils/serial/fdt_serial.c
>> index b9ce67e..8671ee3 100644
>> --- a/lib/utils/serial/fdt_serial.c
>> +++ b/lib/utils/serial/fdt_serial.c
>> @@ -16,12 +16,14 @@ extern struct fdt_serial fdt_serial_uart8250;
>> extern struct fdt_serial fdt_serial_sifive;
>> extern struct fdt_serial fdt_serial_htif;
>> extern struct fdt_serial fdt_serial_shakti;
>> +extern struct fdt_serial fdt_serial_xilinx;
>>
>> static struct fdt_serial *serial_drivers[] = {
>> &fdt_serial_uart8250,
>> &fdt_serial_sifive,
>> &fdt_serial_htif,
>> &fdt_serial_shakti,
>> + &fdt_serial_xilinx,
>> };
>>
>> static void dummy_putc(char ch)
>> diff --git a/lib/utils/serial/fdt_serial_xilinx.c b/lib/utils/serial/fdt_serial_xilinx.c
>> new file mode 100644
>> index 0000000..8b52a7d
>> --- /dev/null
>> +++ b/lib/utils/serial/fdt_serial_xilinx.c
>> @@ -0,0 +1,36 @@
>> +/*
>> + * SPDX-License-Identifier: BSD-2-Clause
>> + *
>> + * Copyright (c) 2021 Guokai Chen <chenguokai17 at mails.ucas.ac.cn>
>> + *
>> + * This code is based on lib/utils/serial/fdt_serial_shakti.c
>> + */
>> +
>> +#include <sbi_utils/fdt/fdt_helper.h>
>> +#include <sbi_utils/serial/fdt_serial.h>
>> +#include <sbi_utils/serial/xilinx-uart.h>
>> +
>> +static int serial_xilinx_init(void *fdt, int nodeoff,
>> + const struct fdt_match *match)
>> +{
>> +
>> + int rc;
>> + struct platform_uart_data uart;
>> + // no need to reinvent a wheel
>
>Use /* */
>
>> + rc = fdt_parse_shakti_uart_node(fdt, nodeoff, &uart);
>
>Can we refactor the existing _uart parse codes a little bit? Calling
>into another hardware's parse routine is odd.
>
>> + if (rc)
>> + return rc;
>> + return xilinx_uart_init(uart.addr, uart.freq, uart.baud);
>> +}
>> +
>> +static const struct fdt_match serial_xilinx_match[] = {
>> + { .compatible = "xilinx,uartlite" },
>
>I didn't find such a compatible string in the upstream Linux kernel.
>We should use the official DT bindings.
>
>> + { },
>> +};
>> +
>> +struct fdt_serial fdt_serial_xilinx = {
>> + .match_table = serial_xilinx_match,
>> + .init = serial_xilinx_init,
>> + .getc = xilinx_uart_getc,
>> + .putc = xilinx_uart_putc
>> +};
>> diff --git a/lib/utils/serial/objects.mk b/lib/utils/serial/objects.mk
>> index c0746f0..6089944 100644
>> --- a/lib/utils/serial/objects.mk
>> +++ b/lib/utils/serial/objects.mk
>> @@ -10,8 +10,10 @@
>> libsbiutils-objs-y += serial/fdt_serial.o
>> libsbiutils-objs-y += serial/fdt_serial_htif.o
>> libsbiutils-objs-y += serial/fdt_serial_shakti.o
>> +libsbiutils-objs-y += serial/fdt_serial_xilinx.o
>> libsbiutils-objs-y += serial/fdt_serial_sifive.o
>> libsbiutils-objs-y += serial/fdt_serial_uart8250.o
>> libsbiutils-objs-y += serial/shakti-uart.o
>> +libsbiutils-objs-y += serial/xilinx-uart.o
>> libsbiutils-objs-y += serial/sifive-uart.o
>> libsbiutils-objs-y += serial/uart8250.o
>> diff --git a/lib/utils/serial/xilinx-uart.c b/lib/utils/serial/xilinx-uart.c
>> new file mode 100644
>> index 0000000..ea32d47
>> --- /dev/null
>> +++ b/lib/utils/serial/xilinx-uart.c
>> @@ -0,0 +1,42 @@
>> +/*
>> + * SPDX-License-Identifier: BSD-2-Clause
>> + *
>> + * Copyright (c) 2021 Guokai Chen <chenguokai17 at mails.ucas.ac.cn>
>> + *
>> + * This code is based on lib/utils/serial/xilinx-uart.c
>> + */
>> +
>> +#include <sbi/riscv_io.h>
>> +#include <sbi/sbi_console.h>
>> +#include <sbi_utils/serial/xilinx-uart.h>
>> +
>> +#define REG_TX 0x04
>> +#define REG_RX 0x00
>
>Please put 0x00 before 0x04
>
>> +#define REG_STATUS 0x08
>> +#define REG_CONTROL 0x0C
>> +
>> +#define UART_TX_FULL (1<<0x3)
>
>nits: use _BITUL(3)
>
>> +#define UART_RX_VALID (1<<0x0)
>> +
>> +static volatile void *uart_base;
>> +
>> +void xilinx_uart_putc(char ch)
>> +{
>> + while((readb(uart_base + REG_STATUS) & UART_TX_FULL))
>> + ;
>> + writeb(ch, uart_base + REG_TX);
>> +}
>> +
>> +int xilinx_uart_getc(void)
>> +{
>> + u16 status = readb(uart_base + REG_STATUS);
>> + if (status & UART_RX_VALID)
>> + return readb(uart_base + REG_RX);
>> + return -1;
>> +}
>> +
>> +int xilinx_uart_init(unsigned long base, u32 in_freq, u32 baudrate)
>> +{
>> + uart_base = (volatile void *)base;
>> + return 0;
>> +}
>> --
>
>Regards,
>Bin
A new patch will be sent within a few days, with an improved commit message and refactored code.
> nits: use _BITUL(3)
Nits lie on undocumented coding rules.
Regards,
Guokai Chen
More information about the opensbi
mailing list