[1/1] lib: utils/serial: Add support for Gaisler APBUART

Message ID	20210430063556.4153719-1-cederman@gaisler.com
State	Accepted
Headers	show Return-Path: <opensbi-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org> From: Daniel Cederman <cederman@gaisler.com> To: opensbi@lists.infradead.org Cc: daniel@gaisler.com Subject: [PATCH 1/1] lib: utils/serial: Add support for Gaisler APBUART Date: Fri, 30 Apr 2021 08:35:56 +0200 Message-Id: <20210430063556.4153719-1-cederman@gaisler.com> MIME-Version: 1.0 preview: This patch adds support for the UART used by the NOEL-V processor. Cobham Gaisler's NOEL-V RISC-V processor IP is available under GPL and commercial license and is described in more detail at https://www.gaisler.com/noelv. Content analysis details: (0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches SPF record Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "opensbi" <opensbi-bounces@lists.infradead.org> Errors-To: opensbi-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org
Series	[1/1] lib: utils/serial: Add support for Gaisler APBUART \| expand [1/1] lib: utils/serial: Add support for Gaisler APBUART

Daniel Cederman April 30, 2021, 6:35 a.m. UTC

This patch adds support for the UART used by the NOEL-V processor.

Cobham Gaisler's NOEL-V RISC-V processor IP is available under GPL
and commercial license and is described in more detail at
https://www.gaisler.com/noelv.

Signed-off-by: Daniel Cederman <cederman@gaisler.com>
---
 include/sbi_utils/fdt/fdt_helper.h      |  3 +
 include/sbi_utils/serial/gaisler-uart.h | 17 +++++
 lib/utils/fdt/fdt_helper.c              | 40 ++++++++++++
 lib/utils/serial/fdt_serial.c           |  2 +
 lib/utils/serial/fdt_serial_gaisler.c   | 35 ++++++++++
 lib/utils/serial/gaisler-uart.c         | 86 +++++++++++++++++++++++++
 lib/utils/serial/objects.mk             |  2 +
 7 files changed, 185 insertions(+)
 create mode 100644 include/sbi_utils/serial/gaisler-uart.h
 create mode 100644 lib/utils/serial/fdt_serial_gaisler.c
 create mode 100644 lib/utils/serial/gaisler-uart.c

Bin Meng April 30, 2021, 9:16 a.m. UTC | #1

On Fri, Apr 30, 2021 at 2:37 PM Daniel Cederman <cederman@gaisler.com> wrote:
>
> This patch adds support for the UART used by the NOEL-V processor.
>
> Cobham Gaisler's NOEL-V RISC-V processor IP is available under GPL
> and commercial license and is described in more detail at
> https://www.gaisler.com/noelv.
>
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
> ---
>  include/sbi_utils/fdt/fdt_helper.h      |  3 +
>  include/sbi_utils/serial/gaisler-uart.h | 17 +++++
>  lib/utils/fdt/fdt_helper.c              | 40 ++++++++++++
>  lib/utils/serial/fdt_serial.c           |  2 +
>  lib/utils/serial/fdt_serial_gaisler.c   | 35 ++++++++++
>  lib/utils/serial/gaisler-uart.c         | 86 +++++++++++++++++++++++++
>  lib/utils/serial/objects.mk             |  2 +
>  7 files changed, 185 insertions(+)
>  create mode 100644 include/sbi_utils/serial/gaisler-uart.h
>  create mode 100644 lib/utils/serial/fdt_serial_gaisler.c
>  create mode 100644 lib/utils/serial/gaisler-uart.c
>
> diff --git a/include/sbi_utils/fdt/fdt_helper.h b/include/sbi_utils/fdt/fdt_helper.h
> index f5222de..c89f2e6 100644
> --- a/include/sbi_utils/fdt/fdt_helper.h
> +++ b/include/sbi_utils/fdt/fdt_helper.h
> @@ -39,6 +39,9 @@ int fdt_parse_hart_id(void *fdt, int cpu_offset, u32 *hartid);
>
>  int fdt_parse_max_hart_id(void *fdt, u32 *max_hartid);
>
> +int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
> +                               struct platform_uart_data *uart);
> +
>  int fdt_parse_shakti_uart_node(void *fdt, int nodeoffset,
>                                struct platform_uart_data *uart);
>
> diff --git a/include/sbi_utils/serial/gaisler-uart.h b/include/sbi_utils/serial/gaisler-uart.h
> new file mode 100644
> index 0000000..11d1f20
> --- /dev/null
> +++ b/include/sbi_utils/serial/gaisler-uart.h
> @@ -0,0 +1,17 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2021 Cobham Gaisler AB.
> + *
> + * Authors:
> + *   Daniel Cederman <cederman@gaisler.com>
> + */
> +
> +#ifndef __SERIAL_GAISLER_APBUART_H__
> +#define __SERIAL_GAISLER_APBUART_H__
> +
> +#include <sbi/sbi_types.h>
> +
> +int gaisler_uart_init(unsigned long base, u32 in_freq, u32 baudrate);
> +
> +#endif
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> index 909de01..9143e44 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -26,6 +26,8 @@
>  #define DEFAULT_SIFIVE_UART_REG_SHIFT          0
>  #define DEFAULT_SIFIVE_UART_REG_IO_WIDTH       4
>
> +#define DEFAULT_GAISLER_UART_REG_IO_WIDTH      4
> +
>  #define DEFAULT_SHAKTI_UART_FREQ               50000000
>  #define DEFAULT_SHAKTI_UART_BAUD               115200
>
> @@ -213,6 +215,44 @@ int fdt_parse_max_hart_id(void *fdt, u32 *max_hartid)
>         return 0;
>  }
>
> +int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
> +                               struct platform_uart_data *uart)
> +{
> +       int len, rc;
> +       const fdt32_t *val;
> +       unsigned long reg_addr, reg_size;
> +
> +       if (nodeoffset < 0 || !uart || !fdt)
> +               return SBI_ENODEV;
> +
> +       rc = fdt_get_node_addr_size(fdt, nodeoffset, &reg_addr, &reg_size);
> +       if (rc < 0 || !reg_addr || !reg_size)
> +               return SBI_ENODEV;
> +       uart->addr = reg_addr;
> +
> +       /**
> +        * UART address is mandatory. clock-frequency and current-speed
> +        * may not be present. Don't return error.
> +        */
> +       val = (fdt32_t *)fdt_getprop(fdt, nodeoffset, "clock-frequency", &len);
> +       if (len > 0 && val)
> +               uart->freq = fdt32_to_cpu(*val);
> +       else
> +               uart->freq = DEFAULT_UART_FREQ;
> +
> +       val = (fdt32_t *)fdt_getprop(fdt, nodeoffset, "current-speed", &len);
> +       if (len > 0 && val)
> +               uart->baud = fdt32_to_cpu(*val);
> +       else
> +               uart->baud = DEFAULT_UART_BAUD;
> +
> +       /* For Gaisler APBUART, the reg-shift and reg-io-width are fixed .*/
> +       uart->reg_shift    = DEFAULT_UART_REG_SHIFT;
> +       uart->reg_io_width = DEFAULT_GAISLER_UART_REG_IO_WIDTH;
> +
> +       return 0;
> +}
> +
>  int fdt_parse_shakti_uart_node(void *fdt, int nodeoffset,
>                                struct platform_uart_data *uart)
>  {
> diff --git a/lib/utils/serial/fdt_serial.c b/lib/utils/serial/fdt_serial.c
> index 43c55e8..bad3387 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_gaisler;
>
>  static struct fdt_serial *serial_drivers[] = {
>         &fdt_serial_uart8250,
>         &fdt_serial_sifive,
>         &fdt_serial_htif,
>         &fdt_serial_shakti,
> +       &fdt_serial_gaisler
>  };
>
>  static struct fdt_serial dummy = {
> diff --git a/lib/utils/serial/fdt_serial_gaisler.c b/lib/utils/serial/fdt_serial_gaisler.c
> new file mode 100644
> index 0000000..9a9f9a8
> --- /dev/null
> +++ b/lib/utils/serial/fdt_serial_gaisler.c
> @@ -0,0 +1,35 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2021 Cobham Gaisler AB.
> + *
> + * Authors:
> + *   Daniel Cederman <cederman@gaisler.com>
> + */
> +
> +#include <sbi_utils/fdt/fdt_helper.h>
> +#include <sbi_utils/serial/fdt_serial.h>
> +#include <sbi_utils/serial/gaisler-uart.h>
> +
> +static int serial_gaisler_init(void *fdt, int nodeoff,
> +                              const struct fdt_match *match)
> +{
> +       int rc;
> +       struct platform_uart_data uart;
> +
> +       rc = fdt_parse_gaisler_uart_node(fdt, nodeoff, &uart);
> +       if (rc)
> +               return rc;
> +
> +       return gaisler_uart_init(uart.addr, uart.freq, uart.baud);
> +}
> +
> +static const struct fdt_match serial_gaisler_match[] = {
> +       { .compatible = "gaisler,apbuart" },

Is this an approved compatible string? I did not find such in the
upstream Linux kernel.

> +       {},
> +};
> +
> +struct fdt_serial fdt_serial_gaisler = {
> +       .match_table = serial_gaisler_match,
> +       .init = serial_gaisler_init
> +};
> diff --git a/lib/utils/serial/gaisler-uart.c b/lib/utils/serial/gaisler-uart.c
> new file mode 100644
> index 0000000..49298e9
> --- /dev/null

[snip]

Regards,
Bin

Daniel Cederman April 30, 2021, 11:52 a.m. UTC | #2

Hello Bin,

2021-04-30 11:16 Bin Meng wrote:
> On Fri, Apr 30, 2021 at 2:37 PM Daniel Cederman <cederman@gaisler.com> 
> wrote:
>> 
>> +
>> +static const struct fdt_match serial_gaisler_match[] = {
>> +       { .compatible = "gaisler,apbuart" },
> 
> Is this an approved compatible string? I did not find such in the
> upstream Linux kernel.
> 

This is the corresponding Linux driver: drivers/tty/serial/apbuart.c. It 
was originally written for SPARC and does not have a compatible string. 
We have a patch to make the driver usable with RISC-V, but we have not 
upstreamed it yet. I agree with you that it is probably best not to 
commit this patch until the Linux patch has been accepted to avoid name 
conflicts.

Other than the compatible string, does the patch look ok?

Regards,
Daniel

Bin Meng April 30, 2021, 12:32 p.m. UTC | #3

Hi Daniel,

On Fri, Apr 30, 2021 at 7:54 PM Daniel Cederman <cederman@gaisler.com> wrote:
>
> Hello Bin,
>
> 2021-04-30 11:16 Bin Meng wrote:
> > On Fri, Apr 30, 2021 at 2:37 PM Daniel Cederman <cederman@gaisler.com>
> > wrote:
> >>
> >> +
> >> +static const struct fdt_match serial_gaisler_match[] = {
> >> +       { .compatible = "gaisler,apbuart" },
> >
> > Is this an approved compatible string? I did not find such in the
> > upstream Linux kernel.
> >
>
> This is the corresponding Linux driver: drivers/tty/serial/apbuart.c. It
> was originally written for SPARC and does not have a compatible string.
> We have a patch to make the driver usable with RISC-V, but we have not
> upstreamed it yet. I agree with you that it is probably best not to
> commit this patch until the Linux patch has been accepted to avoid name
> conflicts.

Thanks for the clarification.

> Other than the compatible string, does the patch look ok?

The patch looks good to me.

Regards,
Bin

Anup Patel May 4, 2021, 3:45 a.m. UTC | #4

> -----Original Message-----
> From: opensbi <opensbi-bounces@lists.infradead.org> On Behalf Of Daniel
> Cederman
> Sent: 30 April 2021 12:06
> To: opensbi@lists.infradead.org
> Cc: daniel@gaisler.com
> Subject: [PATCH 1/1] lib: utils/serial: Add support for Gaisler APBUART
> 
> This patch adds support for the UART used by the NOEL-V processor.
> 
> Cobham Gaisler's NOEL-V RISC-V processor IP is available under GPL and
> commercial license and is described in more detail at
> https://www.gaisler.com/noelv.
> 
> Signed-off-by: Daniel Cederman <cederman@gaisler.com>

Looks good to me.

Reviewed-by: Anup Patel <anup.patel@wdc.com>

> ---
>  include/sbi_utils/fdt/fdt_helper.h      |  3 +
>  include/sbi_utils/serial/gaisler-uart.h | 17 +++++
>  lib/utils/fdt/fdt_helper.c              | 40 ++++++++++++
>  lib/utils/serial/fdt_serial.c           |  2 +
>  lib/utils/serial/fdt_serial_gaisler.c   | 35 ++++++++++
>  lib/utils/serial/gaisler-uart.c         | 86 +++++++++++++++++++++++++
>  lib/utils/serial/objects.mk             |  2 +
>  7 files changed, 185 insertions(+)
>  create mode 100644 include/sbi_utils/serial/gaisler-uart.h
>  create mode 100644 lib/utils/serial/fdt_serial_gaisler.c
>  create mode 100644 lib/utils/serial/gaisler-uart.c
> 
> diff --git a/include/sbi_utils/fdt/fdt_helper.h
> b/include/sbi_utils/fdt/fdt_helper.h
> index f5222de..c89f2e6 100644
> --- a/include/sbi_utils/fdt/fdt_helper.h
> +++ b/include/sbi_utils/fdt/fdt_helper.h
> @@ -39,6 +39,9 @@ int fdt_parse_hart_id(void *fdt, int cpu_offset, u32
> *hartid);
> 
>  int fdt_parse_max_hart_id(void *fdt, u32 *max_hartid);
> 
> +int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
> +				struct platform_uart_data *uart);
> +
>  int fdt_parse_shakti_uart_node(void *fdt, int nodeoffset,
>  			       struct platform_uart_data *uart);
> 
> diff --git a/include/sbi_utils/serial/gaisler-uart.h
> b/include/sbi_utils/serial/gaisler-uart.h
> new file mode 100644
> index 0000000..11d1f20
> --- /dev/null
> +++ b/include/sbi_utils/serial/gaisler-uart.h
> @@ -0,0 +1,17 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2021 Cobham Gaisler AB.
> + *
> + * Authors:
> + *   Daniel Cederman <cederman@gaisler.com>
> + */
> +
> +#ifndef __SERIAL_GAISLER_APBUART_H__
> +#define __SERIAL_GAISLER_APBUART_H__
> +
> +#include <sbi/sbi_types.h>
> +
> +int gaisler_uart_init(unsigned long base, u32 in_freq, u32 baudrate);
> +
> +#endif
> diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c index
> 909de01..9143e44 100644
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -26,6 +26,8 @@
>  #define DEFAULT_SIFIVE_UART_REG_SHIFT		0
>  #define DEFAULT_SIFIVE_UART_REG_IO_WIDTH	4
> 
> +#define DEFAULT_GAISLER_UART_REG_IO_WIDTH	4
> +
>  #define DEFAULT_SHAKTI_UART_FREQ		50000000
>  #define DEFAULT_SHAKTI_UART_BAUD		115200
> 
> @@ -213,6 +215,44 @@ int fdt_parse_max_hart_id(void *fdt, u32
> *max_hartid)
>  	return 0;
>  }
> 
> +int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
> +				struct platform_uart_data *uart)
> +{
> +	int len, rc;
> +	const fdt32_t *val;
> +	unsigned long reg_addr, reg_size;
> +
> +	if (nodeoffset < 0 || !uart || !fdt)
> +		return SBI_ENODEV;
> +
> +	rc = fdt_get_node_addr_size(fdt, nodeoffset, &reg_addr,
> &reg_size);
> +	if (rc < 0 || !reg_addr || !reg_size)
> +		return SBI_ENODEV;
> +	uart->addr = reg_addr;
> +
> +	/**
> +	 * UART address is mandatory. clock-frequency and current-speed
> +	 * may not be present. Don't return error.
> +	 */
> +	val = (fdt32_t *)fdt_getprop(fdt, nodeoffset, "clock-frequency",
> &len);
> +	if (len > 0 && val)
> +		uart->freq = fdt32_to_cpu(*val);
> +	else
> +		uart->freq = DEFAULT_UART_FREQ;
> +
> +	val = (fdt32_t *)fdt_getprop(fdt, nodeoffset, "current-speed",
> &len);
> +	if (len > 0 && val)
> +		uart->baud = fdt32_to_cpu(*val);
> +	else
> +		uart->baud = DEFAULT_UART_BAUD;
> +
> +	/* For Gaisler APBUART, the reg-shift and reg-io-width are fixed .*/
> +	uart->reg_shift	   = DEFAULT_UART_REG_SHIFT;
> +	uart->reg_io_width = DEFAULT_GAISLER_UART_REG_IO_WIDTH;
> +
> +	return 0;
> +}
> +
>  int fdt_parse_shakti_uart_node(void *fdt, int nodeoffset,
>  			       struct platform_uart_data *uart)  { diff --git
> a/lib/utils/serial/fdt_serial.c b/lib/utils/serial/fdt_serial.c index
> 43c55e8..bad3387 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_gaisler;
> 
>  static struct fdt_serial *serial_drivers[] = {
>  	&fdt_serial_uart8250,
>  	&fdt_serial_sifive,
>  	&fdt_serial_htif,
>  	&fdt_serial_shakti,
> +	&fdt_serial_gaisler
>  };
> 
>  static struct fdt_serial dummy = {
> diff --git a/lib/utils/serial/fdt_serial_gaisler.c
> b/lib/utils/serial/fdt_serial_gaisler.c
> new file mode 100644
> index 0000000..9a9f9a8
> --- /dev/null
> +++ b/lib/utils/serial/fdt_serial_gaisler.c
> @@ -0,0 +1,35 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2021 Cobham Gaisler AB.
> + *
> + * Authors:
> + *   Daniel Cederman <cederman@gaisler.com>
> + */
> +
> +#include <sbi_utils/fdt/fdt_helper.h>
> +#include <sbi_utils/serial/fdt_serial.h> #include
> +<sbi_utils/serial/gaisler-uart.h>
> +
> +static int serial_gaisler_init(void *fdt, int nodeoff,
> +			       const struct fdt_match *match) {
> +	int rc;
> +	struct platform_uart_data uart;
> +
> +	rc = fdt_parse_gaisler_uart_node(fdt, nodeoff, &uart);
> +	if (rc)
> +		return rc;
> +
> +	return gaisler_uart_init(uart.addr, uart.freq, uart.baud); }
> +
> +static const struct fdt_match serial_gaisler_match[] = {
> +	{ .compatible = "gaisler,apbuart" },

We do have compatible strings in OpenSBI which are not standardized by
Linux DT bindings so this compatible string is okay for now.

It is very important that whenever Linux DT bindings are accepted, we
add the accepted compatible string here as well.

Regards,
Anup

> +	{},
> +};
> +
> +struct fdt_serial fdt_serial_gaisler = {
> +	.match_table = serial_gaisler_match,
> +	.init = serial_gaisler_init
> +};
> diff --git a/lib/utils/serial/gaisler-uart.c b/lib/utils/serial/gaisler-uart.c new file
> mode 100644 index 0000000..49298e9
> --- /dev/null
> +++ b/lib/utils/serial/gaisler-uart.c
> @@ -0,0 +1,86 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2021 Cobham Gaisler AB.
> + *
> + * Authors:
> + *   Daniel Cederman <cederman@gaisler.com>
> + */
> +
> +#include <sbi/riscv_io.h>
> +#include <sbi/sbi_console.h>
> +#include <sbi_utils/serial/gaisler-uart.h>
> +
> +/* clang-format off */
> +
> +#define UART_REG_DATA		0
> +#define UART_REG_STATUS		1
> +#define UART_REG_CTRL		2
> +#define UART_REG_SCALER		3
> +
> +#define UART_DATA_DATA		0x000000ff
> +#define UART_STATUS_FIFOFULL	0x00000200
> +#define UART_STATUS_DATAREADY	0x00000001
> +
> +#define UART_CTRL_DB		(1<<11)
> +#define UART_CTRL_FL		(1<<6)
> +#define UART_CTRL_TE		(1<<1)
> +#define UART_CTRL_RE		(1<<0)
> +
> +/* clang-format on */
> +
> +static volatile void *uart_base;
> +
> +static u32 get_reg(u32 num)
> +{
> +	return readl(uart_base + (num * 0x4)); }
> +
> +static void set_reg(u32 num, u32 val)
> +{
> +	writel(val, uart_base + (num * 0x4));
> +}
> +
> +static void gaisler_uart_putc(char ch)
> +{
> +	while (get_reg(UART_REG_STATUS) & UART_STATUS_FIFOFULL)
> +		;
> +
> +	set_reg(UART_REG_DATA, ch);
> +}
> +
> +static int gaisler_uart_getc(void)
> +{
> +	u32 ret = get_reg(UART_REG_STATUS);
> +	if (!(ret & UART_STATUS_DATAREADY))
> +		return -1;
> +	return get_reg(UART_REG_DATA) & UART_DATA_DATA; }
> +
> +static struct sbi_console_device gaisler_console = {
> +	.name	      = "gaisler_uart",
> +	.console_putc = gaisler_uart_putc,
> +	.console_getc = gaisler_uart_getc
> +};
> +
> +int gaisler_uart_init(unsigned long base, u32 in_freq, u32 baudrate) {
> +	u32 ctrl;
> +
> +	uart_base = (volatile void *)base;
> +
> +	/* Configure baudrate */
> +	if (in_freq)
> +		set_reg(UART_REG_SCALER, in_freq / (baudrate * 8 + 7));
> +
> +	ctrl = get_reg(UART_REG_CTRL);
> +	/* Preserve debug mode and flow control */
> +	ctrl &= (UART_CTRL_DB | UART_CTRL_FL);
> +	/* Enable TX and RX */
> +	ctrl |= UART_CTRL_TE | UART_CTRL_RE;
> +	set_reg(UART_REG_CTRL, ctrl);
> +
> +	sbi_console_set_device(&gaisler_console);
> +
> +	return 0;
> +}
> diff --git a/lib/utils/serial/objects.mk b/lib/utils/serial/objects.mk index
> c0746f0..9fb0902 100644
> --- a/lib/utils/serial/objects.mk
> +++ b/lib/utils/serial/objects.mk
> @@ -8,10 +8,12 @@
>  #
> 
>  libsbiutils-objs-y += serial/fdt_serial.o
> +libsbiutils-objs-y += serial/fdt_serial_gaisler.o
>  libsbiutils-objs-y += serial/fdt_serial_htif.o  libsbiutils-objs-y +=
> serial/fdt_serial_shakti.o  libsbiutils-objs-y += serial/fdt_serial_sifive.o
> libsbiutils-objs-y += serial/fdt_serial_uart8250.o
> +libsbiutils-objs-y += serial/gaisler-uart.o
>  libsbiutils-objs-y += serial/shakti-uart.o  libsbiutils-objs-y += serial/sifive-
> uart.o  libsbiutils-objs-y += serial/uart8250.o
> --
> 2.25.1
> 
> 
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Bin Meng May 4, 2021, 9:24 a.m. UTC | #5

On Tue, May 4, 2021 at 11:46 AM Anup Patel <Anup.Patel@wdc.com> wrote:
>
>
>
> > -----Original Message-----
> > From: opensbi <opensbi-bounces@lists.infradead.org> On Behalf Of Daniel
> > Cederman
> > Sent: 30 April 2021 12:06
> > To: opensbi@lists.infradead.org
> > Cc: daniel@gaisler.com
> > Subject: [PATCH 1/1] lib: utils/serial: Add support for Gaisler APBUART
> >
> > This patch adds support for the UART used by the NOEL-V processor.
> >
> > Cobham Gaisler's NOEL-V RISC-V processor IP is available under GPL and
> > commercial license and is described in more detail at
> > https://www.gaisler.com/noelv.
> >
> > Signed-off-by: Daniel Cederman <cederman@gaisler.com>
>
> Looks good to me.
>
> Reviewed-by: Anup Patel <anup.patel@wdc.com>
>
> > ---
> >  include/sbi_utils/fdt/fdt_helper.h      |  3 +
> >  include/sbi_utils/serial/gaisler-uart.h | 17 +++++
> >  lib/utils/fdt/fdt_helper.c              | 40 ++++++++++++
> >  lib/utils/serial/fdt_serial.c           |  2 +
> >  lib/utils/serial/fdt_serial_gaisler.c   | 35 ++++++++++
> >  lib/utils/serial/gaisler-uart.c         | 86 +++++++++++++++++++++++++
> >  lib/utils/serial/objects.mk             |  2 +
> >  7 files changed, 185 insertions(+)
> >  create mode 100644 include/sbi_utils/serial/gaisler-uart.h
> >  create mode 100644 lib/utils/serial/fdt_serial_gaisler.c
> >  create mode 100644 lib/utils/serial/gaisler-uart.c
> >
> > diff --git a/include/sbi_utils/fdt/fdt_helper.h
> > b/include/sbi_utils/fdt/fdt_helper.h
> > index f5222de..c89f2e6 100644
> > --- a/include/sbi_utils/fdt/fdt_helper.h
> > +++ b/include/sbi_utils/fdt/fdt_helper.h
> > @@ -39,6 +39,9 @@ int fdt_parse_hart_id(void *fdt, int cpu_offset, u32
> > *hartid);
> >
> >  int fdt_parse_max_hart_id(void *fdt, u32 *max_hartid);
> >
> > +int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
> > +                             struct platform_uart_data *uart);
> > +
> >  int fdt_parse_shakti_uart_node(void *fdt, int nodeoffset,
> >                              struct platform_uart_data *uart);
> >
> > diff --git a/include/sbi_utils/serial/gaisler-uart.h
> > b/include/sbi_utils/serial/gaisler-uart.h
> > new file mode 100644
> > index 0000000..11d1f20
> > --- /dev/null
> > +++ b/include/sbi_utils/serial/gaisler-uart.h
> > @@ -0,0 +1,17 @@
> > +/*
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2021 Cobham Gaisler AB.
> > + *
> > + * Authors:
> > + *   Daniel Cederman <cederman@gaisler.com>
> > + */
> > +
> > +#ifndef __SERIAL_GAISLER_APBUART_H__
> > +#define __SERIAL_GAISLER_APBUART_H__
> > +
> > +#include <sbi/sbi_types.h>
> > +
> > +int gaisler_uart_init(unsigned long base, u32 in_freq, u32 baudrate);
> > +
> > +#endif
> > diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c index
> > 909de01..9143e44 100644
> > --- a/lib/utils/fdt/fdt_helper.c
> > +++ b/lib/utils/fdt/fdt_helper.c
> > @@ -26,6 +26,8 @@
> >  #define DEFAULT_SIFIVE_UART_REG_SHIFT                0
> >  #define DEFAULT_SIFIVE_UART_REG_IO_WIDTH     4
> >
> > +#define DEFAULT_GAISLER_UART_REG_IO_WIDTH    4
> > +
> >  #define DEFAULT_SHAKTI_UART_FREQ             50000000
> >  #define DEFAULT_SHAKTI_UART_BAUD             115200
> >
> > @@ -213,6 +215,44 @@ int fdt_parse_max_hart_id(void *fdt, u32
> > *max_hartid)
> >       return 0;
> >  }
> >
> > +int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
> > +                             struct platform_uart_data *uart)
> > +{
> > +     int len, rc;
> > +     const fdt32_t *val;
> > +     unsigned long reg_addr, reg_size;
> > +
> > +     if (nodeoffset < 0 || !uart || !fdt)
> > +             return SBI_ENODEV;
> > +
> > +     rc = fdt_get_node_addr_size(fdt, nodeoffset, &reg_addr,
> > &reg_size);
> > +     if (rc < 0 || !reg_addr || !reg_size)
> > +             return SBI_ENODEV;
> > +     uart->addr = reg_addr;
> > +
> > +     /**
> > +      * UART address is mandatory. clock-frequency and current-speed
> > +      * may not be present. Don't return error.
> > +      */
> > +     val = (fdt32_t *)fdt_getprop(fdt, nodeoffset, "clock-frequency",
> > &len);
> > +     if (len > 0 && val)
> > +             uart->freq = fdt32_to_cpu(*val);
> > +     else
> > +             uart->freq = DEFAULT_UART_FREQ;
> > +
> > +     val = (fdt32_t *)fdt_getprop(fdt, nodeoffset, "current-speed",
> > &len);
> > +     if (len > 0 && val)
> > +             uart->baud = fdt32_to_cpu(*val);
> > +     else
> > +             uart->baud = DEFAULT_UART_BAUD;
> > +
> > +     /* For Gaisler APBUART, the reg-shift and reg-io-width are fixed .*/
> > +     uart->reg_shift    = DEFAULT_UART_REG_SHIFT;
> > +     uart->reg_io_width = DEFAULT_GAISLER_UART_REG_IO_WIDTH;
> > +
> > +     return 0;
> > +}
> > +
> >  int fdt_parse_shakti_uart_node(void *fdt, int nodeoffset,
> >                              struct platform_uart_data *uart)  { diff --git
> > a/lib/utils/serial/fdt_serial.c b/lib/utils/serial/fdt_serial.c index
> > 43c55e8..bad3387 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_gaisler;
> >
> >  static struct fdt_serial *serial_drivers[] = {
> >       &fdt_serial_uart8250,
> >       &fdt_serial_sifive,
> >       &fdt_serial_htif,
> >       &fdt_serial_shakti,
> > +     &fdt_serial_gaisler
> >  };
> >
> >  static struct fdt_serial dummy = {
> > diff --git a/lib/utils/serial/fdt_serial_gaisler.c
> > b/lib/utils/serial/fdt_serial_gaisler.c
> > new file mode 100644
> > index 0000000..9a9f9a8
> > --- /dev/null
> > +++ b/lib/utils/serial/fdt_serial_gaisler.c
> > @@ -0,0 +1,35 @@
> > +/*
> > + * SPDX-License-Identifier: BSD-2-Clause
> > + *
> > + * Copyright (c) 2021 Cobham Gaisler AB.
> > + *
> > + * Authors:
> > + *   Daniel Cederman <cederman@gaisler.com>
> > + */
> > +
> > +#include <sbi_utils/fdt/fdt_helper.h>
> > +#include <sbi_utils/serial/fdt_serial.h> #include
> > +<sbi_utils/serial/gaisler-uart.h>
> > +
> > +static int serial_gaisler_init(void *fdt, int nodeoff,
> > +                            const struct fdt_match *match) {
> > +     int rc;
> > +     struct platform_uart_data uart;
> > +
> > +     rc = fdt_parse_gaisler_uart_node(fdt, nodeoff, &uart);
> > +     if (rc)
> > +             return rc;
> > +
> > +     return gaisler_uart_init(uart.addr, uart.freq, uart.baud); }
> > +
> > +static const struct fdt_match serial_gaisler_match[] = {
> > +     { .compatible = "gaisler,apbuart" },
>
> We do have compatible strings in OpenSBI which are not standardized by
> Linux DT bindings so this compatible string is okay for now.
>
> It is very important that whenever Linux DT bindings are accepted, we
> add the accepted compatible string here as well.

Good to know :)

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin

Anup Patel May 6, 2021, 9:08 a.m. UTC | #6

> -----Original Message-----
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: 04 May 2021 14:55
> To: Anup Patel <Anup.Patel@wdc.com>
> Cc: Daniel Cederman <cederman@gaisler.com>;
> opensbi@lists.infradead.org; daniel@gaisler.com
> Subject: Re: [PATCH 1/1] lib: utils/serial: Add support for Gaisler APBUART
> 
> On Tue, May 4, 2021 at 11:46 AM Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: opensbi <opensbi-bounces@lists.infradead.org> On Behalf Of
> > > Daniel Cederman
> > > Sent: 30 April 2021 12:06
> > > To: opensbi@lists.infradead.org
> > > Cc: daniel@gaisler.com
> > > Subject: [PATCH 1/1] lib: utils/serial: Add support for Gaisler
> > > APBUART
> > >
> > > This patch adds support for the UART used by the NOEL-V processor.
> > >
> > > Cobham Gaisler's NOEL-V RISC-V processor IP is available under GPL
> > > and commercial license and is described in more detail at
> > > https://www.gaisler.com/noelv.
> > >
> > > Signed-off-by: Daniel Cederman <cederman@gaisler.com>
> >
> > Looks good to me.
> >
> > Reviewed-by: Anup Patel <anup.patel@wdc.com>
> >
> > > ---
> > >  include/sbi_utils/fdt/fdt_helper.h      |  3 +
> > >  include/sbi_utils/serial/gaisler-uart.h | 17 +++++
> > >  lib/utils/fdt/fdt_helper.c              | 40 ++++++++++++
> > >  lib/utils/serial/fdt_serial.c           |  2 +
> > >  lib/utils/serial/fdt_serial_gaisler.c   | 35 ++++++++++
> > >  lib/utils/serial/gaisler-uart.c         | 86 +++++++++++++++++++++++++
> > >  lib/utils/serial/objects.mk             |  2 +
> > >  7 files changed, 185 insertions(+)
> > >  create mode 100644 include/sbi_utils/serial/gaisler-uart.h
> > >  create mode 100644 lib/utils/serial/fdt_serial_gaisler.c
> > >  create mode 100644 lib/utils/serial/gaisler-uart.c
> > >
> > > diff --git a/include/sbi_utils/fdt/fdt_helper.h
> > > b/include/sbi_utils/fdt/fdt_helper.h
> > > index f5222de..c89f2e6 100644
> > > --- a/include/sbi_utils/fdt/fdt_helper.h
> > > +++ b/include/sbi_utils/fdt/fdt_helper.h
> > > @@ -39,6 +39,9 @@ int fdt_parse_hart_id(void *fdt, int cpu_offset,
> > > u32 *hartid);
> > >
> > >  int fdt_parse_max_hart_id(void *fdt, u32 *max_hartid);
> > >
> > > +int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
> > > +                             struct platform_uart_data *uart);
> > > +
> > >  int fdt_parse_shakti_uart_node(void *fdt, int nodeoffset,
> > >                              struct platform_uart_data *uart);
> > >
> > > diff --git a/include/sbi_utils/serial/gaisler-uart.h
> > > b/include/sbi_utils/serial/gaisler-uart.h
> > > new file mode 100644
> > > index 0000000..11d1f20
> > > --- /dev/null
> > > +++ b/include/sbi_utils/serial/gaisler-uart.h
> > > @@ -0,0 +1,17 @@
> > > +/*
> > > + * SPDX-License-Identifier: BSD-2-Clause
> > > + *
> > > + * Copyright (c) 2021 Cobham Gaisler AB.
> > > + *
> > > + * Authors:
> > > + *   Daniel Cederman <cederman@gaisler.com>
> > > + */
> > > +
> > > +#ifndef __SERIAL_GAISLER_APBUART_H__ #define
> > > +__SERIAL_GAISLER_APBUART_H__
> > > +
> > > +#include <sbi/sbi_types.h>
> > > +
> > > +int gaisler_uart_init(unsigned long base, u32 in_freq, u32
> > > +baudrate);
> > > +
> > > +#endif
> > > diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> > > index
> > > 909de01..9143e44 100644
> > > --- a/lib/utils/fdt/fdt_helper.c
> > > +++ b/lib/utils/fdt/fdt_helper.c
> > > @@ -26,6 +26,8 @@
> > >  #define DEFAULT_SIFIVE_UART_REG_SHIFT                0
> > >  #define DEFAULT_SIFIVE_UART_REG_IO_WIDTH     4
> > >
> > > +#define DEFAULT_GAISLER_UART_REG_IO_WIDTH    4
> > > +
> > >  #define DEFAULT_SHAKTI_UART_FREQ             50000000
> > >  #define DEFAULT_SHAKTI_UART_BAUD             115200
> > >
> > > @@ -213,6 +215,44 @@ int fdt_parse_max_hart_id(void *fdt, u32
> > > *max_hartid)
> > >       return 0;
> > >  }
> > >
> > > +int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
> > > +                             struct platform_uart_data *uart) {
> > > +     int len, rc;
> > > +     const fdt32_t *val;
> > > +     unsigned long reg_addr, reg_size;
> > > +
> > > +     if (nodeoffset < 0 || !uart || !fdt)
> > > +             return SBI_ENODEV;
> > > +
> > > +     rc = fdt_get_node_addr_size(fdt, nodeoffset, &reg_addr,
> > > &reg_size);
> > > +     if (rc < 0 || !reg_addr || !reg_size)
> > > +             return SBI_ENODEV;
> > > +     uart->addr = reg_addr;
> > > +
> > > +     /**
> > > +      * UART address is mandatory. clock-frequency and current-speed
> > > +      * may not be present. Don't return error.
> > > +      */
> > > +     val = (fdt32_t *)fdt_getprop(fdt, nodeoffset,
> > > + "clock-frequency",
> > > &len);
> > > +     if (len > 0 && val)
> > > +             uart->freq = fdt32_to_cpu(*val);
> > > +     else
> > > +             uart->freq = DEFAULT_UART_FREQ;
> > > +
> > > +     val = (fdt32_t *)fdt_getprop(fdt, nodeoffset, "current-speed",
> > > &len);
> > > +     if (len > 0 && val)
> > > +             uart->baud = fdt32_to_cpu(*val);
> > > +     else
> > > +             uart->baud = DEFAULT_UART_BAUD;
> > > +
> > > +     /* For Gaisler APBUART, the reg-shift and reg-io-width are fixed .*/
> > > +     uart->reg_shift    = DEFAULT_UART_REG_SHIFT;
> > > +     uart->reg_io_width = DEFAULT_GAISLER_UART_REG_IO_WIDTH;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >  int fdt_parse_shakti_uart_node(void *fdt, int nodeoffset,
> > >                              struct platform_uart_data *uart)  {
> > > diff --git a/lib/utils/serial/fdt_serial.c
> > > b/lib/utils/serial/fdt_serial.c index
> > > 43c55e8..bad3387 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_gaisler;
> > >
> > >  static struct fdt_serial *serial_drivers[] = {
> > >       &fdt_serial_uart8250,
> > >       &fdt_serial_sifive,
> > >       &fdt_serial_htif,
> > >       &fdt_serial_shakti,
> > > +     &fdt_serial_gaisler
> > >  };
> > >
> > >  static struct fdt_serial dummy = {
> > > diff --git a/lib/utils/serial/fdt_serial_gaisler.c
> > > b/lib/utils/serial/fdt_serial_gaisler.c
> > > new file mode 100644
> > > index 0000000..9a9f9a8
> > > --- /dev/null
> > > +++ b/lib/utils/serial/fdt_serial_gaisler.c
> > > @@ -0,0 +1,35 @@
> > > +/*
> > > + * SPDX-License-Identifier: BSD-2-Clause
> > > + *
> > > + * Copyright (c) 2021 Cobham Gaisler AB.
> > > + *
> > > + * Authors:
> > > + *   Daniel Cederman <cederman@gaisler.com>
> > > + */
> > > +
> > > +#include <sbi_utils/fdt/fdt_helper.h> #include
> > > +<sbi_utils/serial/fdt_serial.h> #include
> > > +<sbi_utils/serial/gaisler-uart.h>
> > > +
> > > +static int serial_gaisler_init(void *fdt, int nodeoff,
> > > +                            const struct fdt_match *match) {
> > > +     int rc;
> > > +     struct platform_uart_data uart;
> > > +
> > > +     rc = fdt_parse_gaisler_uart_node(fdt, nodeoff, &uart);
> > > +     if (rc)
> > > +             return rc;
> > > +
> > > +     return gaisler_uart_init(uart.addr, uart.freq, uart.baud); }
> > > +
> > > +static const struct fdt_match serial_gaisler_match[] = {
> > > +     { .compatible = "gaisler,apbuart" },
> >
> > We do have compatible strings in OpenSBI which are not standardized by
> > Linux DT bindings so this compatible string is okay for now.
> >
> > It is very important that whenever Linux DT bindings are accepted, we
> > add the accepted compatible string here as well.
> 
> Good to know :)
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Applied this patch to the riscv/opensbi repo

Regards,
Anup

[1/1] lib: utils/serial: Add support for Gaisler APBUART

Commit Message

Comments

Patch