* [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver
2022-11-11 18:20 [PATCH v2 0/7] Add support for Renesas RZ/Five SoC Prabhakar
@ 2022-11-11 18:20 ` Prabhakar
2022-11-15 7:30 ` Bin Meng
2022-11-11 18:21 ` [PATCH v2 2/7] lib: utils: serial: Add FDT driver for Renesas SCIF Prabhakar
` (5 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Prabhakar @ 2022-11-11 18:20 UTC (permalink / raw)
To: opensbi
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add Renesas SCIF driver.
Based on a patch in the BSP by Takeki Hamada
<takeki.hamada.ak@bp.renesas.com>
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
* Fixed comments pointed by Xiang W
---
include/sbi_utils/serial/renesas-scif.h | 11 ++
lib/utils/serial/Kconfig | 4 +
lib/utils/serial/objects.mk | 1 +
lib/utils/serial/renesas_scif.c | 139 ++++++++++++++++++++++++
4 files changed, 155 insertions(+)
create mode 100644 include/sbi_utils/serial/renesas-scif.h
create mode 100644 lib/utils/serial/renesas_scif.c
diff --git a/include/sbi_utils/serial/renesas-scif.h b/include/sbi_utils/serial/renesas-scif.h
new file mode 100644
index 0000000..0002a1a
--- /dev/null
+++ b/include/sbi_utils/serial/renesas-scif.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+
+#ifndef __SERIAL_RENESAS_SCIF_H__
+#define __SERIAL_RENESAS_SCIF_H__
+
+int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate);
+
+#endif /* __SERIAL_RENESAS_SCIF_H__ */
diff --git a/lib/utils/serial/Kconfig b/lib/utils/serial/Kconfig
index da549a7..f6ed803 100644
--- a/lib/utils/serial/Kconfig
+++ b/lib/utils/serial/Kconfig
@@ -59,6 +59,10 @@ config SERIAL_GAISLER
bool "Gaisler UART support"
default n
+config SERIAL_RENESAS_SCIF
+ bool "Renesas SCIF support"
+ default n
+
config SERIAL_SHAKTI
bool "Shakti UART support"
default n
diff --git a/lib/utils/serial/objects.mk b/lib/utils/serial/objects.mk
index 98f3f9a..6520424 100644
--- a/lib/utils/serial/objects.mk
+++ b/lib/utils/serial/objects.mk
@@ -36,6 +36,7 @@ libsbiutils-objs-$(CONFIG_FDT_SERIAL_XILINX_UARTLITE) += serial/fdt_serial_xlnx_
libsbiutils-objs-$(CONFIG_SERIAL_CADENCE) += serial/cadence-uart.o
libsbiutils-objs-$(CONFIG_SERIAL_GAISLER) += serial/gaisler-uart.o
+libsbiutils-objs-$(CONFIG_SERIAL_RENESAS_SCIF) += serial/renesas_scif.o
libsbiutils-objs-$(CONFIG_SERIAL_SHAKTI) += serial/shakti-uart.o
libsbiutils-objs-$(CONFIG_SERIAL_SIFIVE) += serial/sifive-uart.o
libsbiutils-objs-$(CONFIG_SERIAL_LITEX) += serial/litex-uart.o
diff --git a/lib/utils/serial/renesas_scif.c b/lib/utils/serial/renesas_scif.c
new file mode 100644
index 0000000..0586e1a
--- /dev/null
+++ b/lib/utils/serial/renesas_scif.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+
+#include <sbi/riscv_io.h>
+#include <sbi/sbi_console.h>
+#include <sbi/sbi_timer.h>
+#include <sbi_utils/serial/renesas-scif.h>
+
+/* clang-format off */
+
+#define SCIF_REG_SMR 0x0
+#define SCIF_REG_BRR 0x2
+#define SCIF_REG_SCR 0x4
+#define SCIF_REG_FTDR 0x6
+#define SCIF_REG_FSR 0x8
+#define SCIF_REG_FRDR 0xa
+#define SCIF_REG_FCR 0xc
+#define SCIF_REG_LSR 0x12
+#define SCIF_REG_SEMR 0x14
+
+#define SCIF_RFRST 0x2 /* Reset assert receive-FIFO (bit[1]) */
+#define SCIF_TFRST 0x4 /* Reset assert transmit-FIFO(bit[2]) */
+
+#define SCIF_FCR_RST_ASSRT_TFRF (SCIF_RFRST | SCIF_TFRST) /* Reset assert tx-FIFO & rx-FIFO */
+#define SCIF_FCR_RST_NGATE_TFRF 0x0 /* Reset negate tx-FIFO & rx-FIFO*/
+
+#define SCIF_RE 0x10 /* Enable receive (bit[4]) */
+#define SCIF_TE 0x20 /* Enable transmit(bit[5]) */
+#define SCIF_SCR_RCV_TRN_EN (SCIF_RE | SCIF_TE) /* Enable receive & transmit */
+#define SCIF_SCR_RCV_TRN_DIS 0x0 /* Disable receive & transmit */
+
+#define SCIF_FSR_ER 0x80 /* Receive error flag */
+#define SCIF_FSR_TEND 0x40 /* Detect break flag */
+#define SCIF_FSR_TDFE 0x20 /* Detect break flag */
+#define SCIF_FSR_BRK 0x10 /* Detect break flag */
+#define SCIF_FSR_RDF 0x2 /* Receive FIFO data full flag */
+#define SCIF_FSR_DR 0x1 /* Receive data ready flag */
+
+#define SCIF_FSR_RXD_CHK (SCIF_FSR_ER | SCIF_FSR_BRK | SCIF_FSR_DR)
+#define SCIF_FSR_TXD_CHK (SCIF_FSR_TEND | SCIF_FSR_TDFE)
+
+#define SCIF_LSR_ORER 0x1 /* Overrun error flag */
+
+#define SCIF_SPTR_SPB2DT 0x1 /* if SCR.TE setting, don't care */
+#define SCIF_SPTR_SPB2IO 0x2 /* if SCR.TE setting, don't care */
+
+#define SCIF_SEMR_BRME 0x20 /* bit-rate modulation enable */
+#define SCIF_SEMR_MDDRS 0x10 /* MDDR access enable */
+
+#define SCIF_SIZE(reg) ((reg == SCIF_REG_BRR) || \
+ (reg == SCIF_REG_FTDR) || \
+ (reg == SCIF_REG_FRDR) || \
+ (reg == SCIF_REG_SEMR))
+
+#define SCBRR_VALUE(clk, baudrate) ((clk) / (32 * (baudrate)) - 1)
+
+/* clang-format on */
+
+static volatile char *scif_base;
+
+static u32 get_reg(u32 offset)
+{
+ if (SCIF_SIZE(offset))
+ return readb(scif_base + offset);
+
+ return readw(scif_base + offset);
+}
+
+static void set_reg(u32 offset, u32 val)
+{
+ if (SCIF_SIZE(offset))
+ return writeb(val, scif_base + offset);
+
+ return writew(val, scif_base + offset);
+}
+
+static void scif_wait(unsigned long baudrate)
+{
+ unsigned long utime;
+
+ utime = 1000000 / baudrate + 1;
+
+ sbi_timer_udelay(utime);
+}
+
+static void renesas_scif_putc(char ch)
+{
+ uint16_t reg;
+
+ while (!(SCIF_FSR_TXD_CHK & get_reg(SCIF_REG_FSR)))
+ ;
+
+ set_reg(SCIF_REG_FTDR, ch);
+ reg = get_reg(SCIF_REG_FSR);
+ reg &= ~SCIF_FSR_TXD_CHK;
+ set_reg(SCIF_REG_FSR, reg);
+}
+
+static struct sbi_console_device renesas_scif_console = {
+ .name = "renesas_scif",
+ .console_putc = renesas_scif_putc,
+};
+
+int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate)
+{
+ volatile uint16_t data16;
+
+ scif_base = (volatile char *)base;
+
+ set_reg(SCIF_REG_SCR, SCIF_SCR_RCV_TRN_DIS); /* Disable receive & transmit */
+ set_reg(SCIF_REG_FCR, SCIF_FCR_RST_ASSRT_TFRF); /* Reset assert tx-FIFO & rx-FIFO */
+
+ data16 = get_reg(SCIF_REG_FSR); /* Dummy read */
+ set_reg(SCIF_REG_FSR, 0x0); /* Clear all error bit */
+
+ data16 = get_reg(SCIF_REG_LSR); /* Dummy read */
+ set_reg(SCIF_REG_LSR, 0x0); /* Clear ORER bit */
+
+ set_reg(SCIF_REG_SCR, 0x0); /* Select internal clock, SC_CLK pin unused for output pin */
+
+ set_reg(SCIF_REG_SMR, 0x0); /* Set asynchronous, 8bit data, no-parity, 1 stop and Po/1 */
+
+ data16 = get_reg(SCIF_REG_SEMR);
+ set_reg(SCIF_REG_SEMR, data16 & (~SCIF_SEMR_MDDRS)); /* Select to access BRR */
+ set_reg(SCIF_REG_BRR, SCBRR_VALUE(in_freq, baudrate));
+
+ scif_wait(baudrate);
+
+ /* FTCR is left at initial value, because this interrupt isn't used. */
+ set_reg(SCIF_REG_FCR, SCIF_FCR_RST_NGATE_TFRF); /* Reset negate tx-FIFO, rx-FIFO. */
+
+ set_reg(SCIF_REG_SCR, SCIF_SCR_RCV_TRN_EN); /* Enable receive & transmit w/SC_CLK=no output */
+
+ sbi_console_set_device(&renesas_scif_console);
+
+ return 0;
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver
2022-11-11 18:20 ` [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver Prabhakar
@ 2022-11-15 7:30 ` Bin Meng
2022-11-15 17:33 ` Lad, Prabhakar
0 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2022-11-15 7:30 UTC (permalink / raw)
To: opensbi
On Sat, Nov 12, 2022 at 2:22 AM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add Renesas SCIF driver.
>
> Based on a patch in the BSP by Takeki Hamada
> <takeki.hamada.ak@bp.renesas.com>
If there is a public URL for the BSP you mentioned, better put that in
the commit here for future reference.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> * Fixed comments pointed by Xiang W
> ---
> include/sbi_utils/serial/renesas-scif.h | 11 ++
> lib/utils/serial/Kconfig | 4 +
> lib/utils/serial/objects.mk | 1 +
> lib/utils/serial/renesas_scif.c | 139 ++++++++++++++++++++++++
> 4 files changed, 155 insertions(+)
> create mode 100644 include/sbi_utils/serial/renesas-scif.h
> create mode 100644 lib/utils/serial/renesas_scif.c
>
> diff --git a/include/sbi_utils/serial/renesas-scif.h b/include/sbi_utils/serial/renesas-scif.h
> new file mode 100644
> index 0000000..0002a1a
> --- /dev/null
> +++ b/include/sbi_utils/serial/renesas-scif.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2022 Renesas Electronics Corporation
> + */
> +
> +#ifndef __SERIAL_RENESAS_SCIF_H__
> +#define __SERIAL_RENESAS_SCIF_H__
> +
> +int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate);
> +
> +#endif /* __SERIAL_RENESAS_SCIF_H__ */
> diff --git a/lib/utils/serial/Kconfig b/lib/utils/serial/Kconfig
> index da549a7..f6ed803 100644
> --- a/lib/utils/serial/Kconfig
> +++ b/lib/utils/serial/Kconfig
> @@ -59,6 +59,10 @@ config SERIAL_GAISLER
> bool "Gaisler UART support"
> default n
>
> +config SERIAL_RENESAS_SCIF
> + bool "Renesas SCIF support"
> + default n
> +
> config SERIAL_SHAKTI
> bool "Shakti UART support"
> default n
> diff --git a/lib/utils/serial/objects.mk b/lib/utils/serial/objects.mk
> index 98f3f9a..6520424 100644
> --- a/lib/utils/serial/objects.mk
> +++ b/lib/utils/serial/objects.mk
> @@ -36,6 +36,7 @@ libsbiutils-objs-$(CONFIG_FDT_SERIAL_XILINX_UARTLITE) += serial/fdt_serial_xlnx_
>
> libsbiutils-objs-$(CONFIG_SERIAL_CADENCE) += serial/cadence-uart.o
> libsbiutils-objs-$(CONFIG_SERIAL_GAISLER) += serial/gaisler-uart.o
> +libsbiutils-objs-$(CONFIG_SERIAL_RENESAS_SCIF) += serial/renesas_scif.o
> libsbiutils-objs-$(CONFIG_SERIAL_SHAKTI) += serial/shakti-uart.o
> libsbiutils-objs-$(CONFIG_SERIAL_SIFIVE) += serial/sifive-uart.o
> libsbiutils-objs-$(CONFIG_SERIAL_LITEX) += serial/litex-uart.o
> diff --git a/lib/utils/serial/renesas_scif.c b/lib/utils/serial/renesas_scif.c
> new file mode 100644
> index 0000000..0586e1a
> --- /dev/null
> +++ b/lib/utils/serial/renesas_scif.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: BSD-2-Clause
> +/*
> + * Copyright (C) 2022 Renesas Electronics Corporation
> + */
> +
> +#include <sbi/riscv_io.h>
> +#include <sbi/sbi_console.h>
> +#include <sbi/sbi_timer.h>
> +#include <sbi_utils/serial/renesas-scif.h>
> +
> +/* clang-format off */
> +
> +#define SCIF_REG_SMR 0x0
> +#define SCIF_REG_BRR 0x2
> +#define SCIF_REG_SCR 0x4
> +#define SCIF_REG_FTDR 0x6
> +#define SCIF_REG_FSR 0x8
> +#define SCIF_REG_FRDR 0xa
This register is not used by the driver. Drop it.
> +#define SCIF_REG_FCR 0xc
> +#define SCIF_REG_LSR 0x12
> +#define SCIF_REG_SEMR 0x14
> +
> +#define SCIF_RFRST 0x2 /* Reset assert receive-FIFO (bit[1]) */
> +#define SCIF_TFRST 0x4 /* Reset assert transmit-FIFO(bit[2]) */
The above 2 macros should probably be named as SCIF_FCR_* as they only
apply to the FCR register.
> +
> +#define SCIF_FCR_RST_ASSRT_TFRF (SCIF_RFRST | SCIF_TFRST) /* Reset assert tx-FIFO & rx-FIFO */
The macro name should probably be named to have RF come before TF,
like SCIF_FCR_RST_ASSRT_RFTF, to keep in consistency with the rest,
and the codes.
#define SCIF_FCR_RST_ASSRT_RFTF (SCIF_RFRST | SCIF_TFRST) /*
Reset assert rx-FIFO & tx-FIFO */
> +#define SCIF_FCR_RST_NGATE_TFRF 0x0 /* Reset negate tx-FIFO & rx-FIFO*/
ditto
need a space before */
> +
> +#define SCIF_RE 0x10 /* Enable receive (bit[4]) */
> +#define SCIF_TE 0x20 /* Enable transmit(bit[5]) */
The above 2 macros should probably be named as SCIF_SCR_* as they only
apply to the SCR register.
nits: only need one space fore */
> +#define SCIF_SCR_RCV_TRN_EN (SCIF_RE | SCIF_TE) /* Enable receive & transmit */
> +#define SCIF_SCR_RCV_TRN_DIS 0x0 /* Disable receive & transmit */
> +
> +#define SCIF_FSR_ER 0x80 /* Receive error flag */
> +#define SCIF_FSR_TEND 0x40 /* Detect break flag */
> +#define SCIF_FSR_TDFE 0x20 /* Detect break flag */
The comments of the above 2 bit fields are wrong
> +#define SCIF_FSR_BRK 0x10 /* Detect break flag */
> +#define SCIF_FSR_RDF 0x2 /* Receive FIFO data full flag */
This bit field seems not to be used anywhere. Drop it?
> +#define SCIF_FSR_DR 0x1 /* Receive data ready flag */
> +
> +#define SCIF_FSR_RXD_CHK (SCIF_FSR_ER | SCIF_FSR_BRK | SCIF_FSR_DR)
> +#define SCIF_FSR_TXD_CHK (SCIF_FSR_TEND | SCIF_FSR_TDFE)
> +
> +#define SCIF_LSR_ORER 0x1 /* Overrun error flag */
This macro is not used anywhere. Drop it.
> +
> +#define SCIF_SPTR_SPB2DT 0x1 /* if SCR.TE setting, don't care */
> +#define SCIF_SPTR_SPB2IO 0x2 /* if SCR.TE setting, don't care */
The above 2 macros are not used anywhere. Drop them.
> +
> +#define SCIF_SEMR_BRME 0x20 /* bit-rate modulation enable */
This macro is not used anywhere. Drop it.
> +#define SCIF_SEMR_MDDRS 0x10 /* MDDR access enable */
> +
> +#define SCIF_SIZE(reg) ((reg == SCIF_REG_BRR) || \
> + (reg == SCIF_REG_FTDR) || \
> + (reg == SCIF_REG_FRDR) || \
I don't see the driver programs SCIF_REG_FRDR register. We can just
remove it to save some cycles.
> + (reg == SCIF_REG_SEMR))
The name of this macro is not meaningful. It looks like it's the size
of a register at the first glance but it's actually returning logical
true or false.
Better rename it to something like SCIF_REG_8BIT(reg).
> +
> +#define SCBRR_VALUE(clk, baudrate) ((clk) / (32 * (baudrate)) - 1)
> +
> +/* clang-format on */
> +
> +static volatile char *scif_base;
> +
> +static u32 get_reg(u32 offset)
> +{
> + if (SCIF_SIZE(offset))
> + return readb(scif_base + offset);
> +
> + return readw(scif_base + offset);
> +}
> +
> +static void set_reg(u32 offset, u32 val)
> +{
> + if (SCIF_SIZE(offset))
> + return writeb(val, scif_base + offset);
> +
> + return writew(val, scif_base + offset);
> +}
> +
> +static void scif_wait(unsigned long baudrate)
> +{
> + unsigned long utime;
> +
> + utime = 1000000 / baudrate + 1;
What is this?
> +
> + sbi_timer_udelay(utime);
> +}
> +
> +static void renesas_scif_putc(char ch)
> +{
> + uint16_t reg;
> +
> + while (!(SCIF_FSR_TXD_CHK & get_reg(SCIF_REG_FSR)))
> + ;
> +
> + set_reg(SCIF_REG_FTDR, ch);
> + reg = get_reg(SCIF_REG_FSR);
> + reg &= ~SCIF_FSR_TXD_CHK;
> + set_reg(SCIF_REG_FSR, reg);
The FSR Tx status bits clear should happen before writing a new char
to the Tx fifo, no?
> +}
> +
> +static struct sbi_console_device renesas_scif_console = {
> + .name = "renesas_scif",
> + .console_putc = renesas_scif_putc,
> +};
> +
> +int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate)
> +{
> + volatile uint16_t data16;
> +
> + scif_base = (volatile char *)base;
> +
> + set_reg(SCIF_REG_SCR, SCIF_SCR_RCV_TRN_DIS); /* Disable receive & transmit */
> + set_reg(SCIF_REG_FCR, SCIF_FCR_RST_ASSRT_TFRF); /* Reset assert tx-FIFO & rx-FIFO */
> +
> + data16 = get_reg(SCIF_REG_FSR); /* Dummy read */
> + set_reg(SCIF_REG_FSR, 0x0); /* Clear all error bit */
> +
> + data16 = get_reg(SCIF_REG_LSR); /* Dummy read */
> + set_reg(SCIF_REG_LSR, 0x0); /* Clear ORER bit */
> +
> + set_reg(SCIF_REG_SCR, 0x0); /* Select internal clock, SC_CLK pin unused for output pin */
> +
> + set_reg(SCIF_REG_SMR, 0x0); /* Set asynchronous, 8bit data, no-parity, 1 stop and Po/1 */
> +
> + data16 = get_reg(SCIF_REG_SEMR);
> + set_reg(SCIF_REG_SEMR, data16 & (~SCIF_SEMR_MDDRS)); /* Select to access BRR */
> + set_reg(SCIF_REG_BRR, SCBRR_VALUE(in_freq, baudrate));
> +
> + scif_wait(baudrate);
What is this?
> +
> + /* FTCR is left at initial value, because this interrupt isn't used. */
What is FTCR? There is no such register in your defines.
> + set_reg(SCIF_REG_FCR, SCIF_FCR_RST_NGATE_TFRF); /* Reset negate tx-FIFO, rx-FIFO. */
> +
> + set_reg(SCIF_REG_SCR, SCIF_SCR_RCV_TRN_EN); /* Enable receive & transmit w/SC_CLK=no output */
> +
> + sbi_console_set_device(&renesas_scif_console);
> +
> + return 0;
> +}
Regards,
Bin
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver
2022-11-15 7:30 ` Bin Meng
@ 2022-11-15 17:33 ` Lad, Prabhakar
2022-11-17 7:52 ` Bin Meng
0 siblings, 1 reply; 20+ messages in thread
From: Lad, Prabhakar @ 2022-11-15 17:33 UTC (permalink / raw)
To: opensbi
Hi Bin,
Thank you for the review.
On Tue, Nov 15, 2022 at 7:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Nov 12, 2022 at 2:22 AM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> >
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add Renesas SCIF driver.
> >
> > Based on a patch in the BSP by Takeki Hamada
> > <takeki.hamada.ak@bp.renesas.com>
>
> If there is a public URL for the BSP you mentioned, better put that in
> the commit here for future reference.
>
you mean link for the BSP/File/Commit. Ive followed the same approach
that we follow on Linux.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > * Fixed comments pointed by Xiang W
> > ---
> > include/sbi_utils/serial/renesas-scif.h | 11 ++
> > lib/utils/serial/Kconfig | 4 +
> > lib/utils/serial/objects.mk | 1 +
> > lib/utils/serial/renesas_scif.c | 139 ++++++++++++++++++++++++
> > 4 files changed, 155 insertions(+)
> > create mode 100644 include/sbi_utils/serial/renesas-scif.h
> > create mode 100644 lib/utils/serial/renesas_scif.c
> >
> > diff --git a/include/sbi_utils/serial/renesas-scif.h b/include/sbi_utils/serial/renesas-scif.h
> > new file mode 100644
> > index 0000000..0002a1a
> > --- /dev/null
> > +++ b/include/sbi_utils/serial/renesas-scif.h
> > @@ -0,0 +1,11 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2022 Renesas Electronics Corporation
> > + */
> > +
> > +#ifndef __SERIAL_RENESAS_SCIF_H__
> > +#define __SERIAL_RENESAS_SCIF_H__
> > +
> > +int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate);
> > +
> > +#endif /* __SERIAL_RENESAS_SCIF_H__ */
> > diff --git a/lib/utils/serial/Kconfig b/lib/utils/serial/Kconfig
> > index da549a7..f6ed803 100644
> > --- a/lib/utils/serial/Kconfig
> > +++ b/lib/utils/serial/Kconfig
> > @@ -59,6 +59,10 @@ config SERIAL_GAISLER
> > bool "Gaisler UART support"
> > default n
> >
> > +config SERIAL_RENESAS_SCIF
> > + bool "Renesas SCIF support"
> > + default n
> > +
> > config SERIAL_SHAKTI
> > bool "Shakti UART support"
> > default n
> > diff --git a/lib/utils/serial/objects.mk b/lib/utils/serial/objects.mk
> > index 98f3f9a..6520424 100644
> > --- a/lib/utils/serial/objects.mk
> > +++ b/lib/utils/serial/objects.mk
> > @@ -36,6 +36,7 @@ libsbiutils-objs-$(CONFIG_FDT_SERIAL_XILINX_UARTLITE) += serial/fdt_serial_xlnx_
> >
> > libsbiutils-objs-$(CONFIG_SERIAL_CADENCE) += serial/cadence-uart.o
> > libsbiutils-objs-$(CONFIG_SERIAL_GAISLER) += serial/gaisler-uart.o
> > +libsbiutils-objs-$(CONFIG_SERIAL_RENESAS_SCIF) += serial/renesas_scif.o
> > libsbiutils-objs-$(CONFIG_SERIAL_SHAKTI) += serial/shakti-uart.o
> > libsbiutils-objs-$(CONFIG_SERIAL_SIFIVE) += serial/sifive-uart.o
> > libsbiutils-objs-$(CONFIG_SERIAL_LITEX) += serial/litex-uart.o
> > diff --git a/lib/utils/serial/renesas_scif.c b/lib/utils/serial/renesas_scif.c
> > new file mode 100644
> > index 0000000..0586e1a
> > --- /dev/null
> > +++ b/lib/utils/serial/renesas_scif.c
> > @@ -0,0 +1,139 @@
> > +// SPDX-License-Identifier: BSD-2-Clause
> > +/*
> > + * Copyright (C) 2022 Renesas Electronics Corporation
> > + */
> > +
> > +#include <sbi/riscv_io.h>
> > +#include <sbi/sbi_console.h>
> > +#include <sbi/sbi_timer.h>
> > +#include <sbi_utils/serial/renesas-scif.h>
> > +
> > +/* clang-format off */
> > +
> > +#define SCIF_REG_SMR 0x0
> > +#define SCIF_REG_BRR 0x2
> > +#define SCIF_REG_SCR 0x4
> > +#define SCIF_REG_FTDR 0x6
> > +#define SCIF_REG_FSR 0x8
> > +#define SCIF_REG_FRDR 0xa
>
> This register is not used by the driver. Drop it.
>
OK, I will drop it.
> > +#define SCIF_REG_FCR 0xc
> > +#define SCIF_REG_LSR 0x12
> > +#define SCIF_REG_SEMR 0x14
> > +
> > +#define SCIF_RFRST 0x2 /* Reset assert receive-FIFO (bit[1]) */
> > +#define SCIF_TFRST 0x4 /* Reset assert transmit-FIFO(bit[2]) */
>
> The above 2 macros should probably be named as SCIF_FCR_* as they only
> apply to the FCR register.
>
Agreed.
> > +
> > +#define SCIF_FCR_RST_ASSRT_TFRF (SCIF_RFRST | SCIF_TFRST) /* Reset assert tx-FIFO & rx-FIFO */
>
> The macro name should probably be named to have RF come before TF,
> like SCIF_FCR_RST_ASSRT_RFTF, to keep in consistency with the rest,
> and the codes.
>
Agreed.
> #define SCIF_FCR_RST_ASSRT_RFTF (SCIF_RFRST | SCIF_TFRST) /*
> Reset assert rx-FIFO & tx-FIFO */
>
> > +#define SCIF_FCR_RST_NGATE_TFRF 0x0 /* Reset negate tx-FIFO & rx-FIFO*/
>
> ditto
>
> need a space before */
>
OK, I will fix it.
> > +
> > +#define SCIF_RE 0x10 /* Enable receive (bit[4]) */
> > +#define SCIF_TE 0x20 /* Enable transmit(bit[5]) */
>
> The above 2 macros should probably be named as SCIF_SCR_* as they only
> apply to the SCR register.
>
Agreed.
> nits: only need one space fore */
>
> > +#define SCIF_SCR_RCV_TRN_EN (SCIF_RE | SCIF_TE) /* Enable receive & transmit */
> > +#define SCIF_SCR_RCV_TRN_DIS 0x0 /* Disable receive & transmit */
> > +
> > +#define SCIF_FSR_ER 0x80 /* Receive error flag */
> > +#define SCIF_FSR_TEND 0x40 /* Detect break flag */
> > +#define SCIF_FSR_TDFE 0x20 /* Detect break flag */
>
> The comments of the above 2 bit fields are wrong
>
Ouch will fix that.
> > +#define SCIF_FSR_BRK 0x10 /* Detect break flag */
> > +#define SCIF_FSR_RDF 0x2 /* Receive FIFO data full flag */
>
> This bit field seems not to be used anywhere. Drop it?
>
Sure, I will drop it.
> > +#define SCIF_FSR_DR 0x1 /* Receive data ready flag */
> > +
> > +#define SCIF_FSR_RXD_CHK (SCIF_FSR_ER | SCIF_FSR_BRK | SCIF_FSR_DR)
> > +#define SCIF_FSR_TXD_CHK (SCIF_FSR_TEND | SCIF_FSR_TDFE)
> > +
> > +#define SCIF_LSR_ORER 0x1 /* Overrun error flag */
>
> This macro is not used anywhere. Drop it.
>
ditto.
> > +
> > +#define SCIF_SPTR_SPB2DT 0x1 /* if SCR.TE setting, don't care */
> > +#define SCIF_SPTR_SPB2IO 0x2 /* if SCR.TE setting, don't care */
>
> The above 2 macros are not used anywhere. Drop them.
>
ditto.
> > +
> > +#define SCIF_SEMR_BRME 0x20 /* bit-rate modulation enable */
>
> This macro is not used anywhere. Drop it.
>
ditto.
> > +#define SCIF_SEMR_MDDRS 0x10 /* MDDR access enable */
> > +
> > +#define SCIF_SIZE(reg) ((reg == SCIF_REG_BRR) || \
> > + (reg == SCIF_REG_FTDR) || \
> > + (reg == SCIF_REG_FRDR) || \
>
> I don't see the driver programs SCIF_REG_FRDR register. We can just
> remove it to save some cycles.
>
> > + (reg == SCIF_REG_SEMR))
>
> The name of this macro is not meaningful. It looks like it's the size
> of a register at the first glance but it's actually returning logical
> true or false.
> Better rename it to something like SCIF_REG_8BIT(reg).
>
Good point, I'll rename it.
> > +
> > +#define SCBRR_VALUE(clk, baudrate) ((clk) / (32 * (baudrate)) - 1)
> > +
> > +/* clang-format on */
> > +
> > +static volatile char *scif_base;
> > +
> > +static u32 get_reg(u32 offset)
> > +{
> > + if (SCIF_SIZE(offset))
> > + return readb(scif_base + offset);
> > +
> > + return readw(scif_base + offset);
> > +}
> > +
> > +static void set_reg(u32 offset, u32 val)
> > +{
> > + if (SCIF_SIZE(offset))
> > + return writeb(val, scif_base + offset);
> > +
> > + return writew(val, scif_base + offset);
> > +}
> > +
> > +static void scif_wait(unsigned long baudrate)
> > +{
> > + unsigned long utime;
> > +
> > + utime = 1000000 / baudrate + 1;
>
> What is this?
>
It's the delay required when using the external clock. But as we arent
using it on RZ/Five SMARC we can get rid of it for now.
> > +
> > + sbi_timer_udelay(utime);
> > +}
> > +
> > +static void renesas_scif_putc(char ch)
> > +{
> > + uint16_t reg;
> > +
> > + while (!(SCIF_FSR_TXD_CHK & get_reg(SCIF_REG_FSR)))
> > + ;
> > +
> > + set_reg(SCIF_REG_FTDR, ch);
> > + reg = get_reg(SCIF_REG_FSR);
> > + reg &= ~SCIF_FSR_TXD_CHK;
> > + set_reg(SCIF_REG_FSR, reg);
>
> The FSR Tx status bits clear should happen before writing a new char
> to the Tx fifo, no?
>
No, this has to be cleared only after char write.
> > +}
> > +
> > +static struct sbi_console_device renesas_scif_console = {
> > + .name = "renesas_scif",
> > + .console_putc = renesas_scif_putc,
> > +};
> > +
> > +int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate)
> > +{
> > + volatile uint16_t data16;
> > +
> > + scif_base = (volatile char *)base;
> > +
> > + set_reg(SCIF_REG_SCR, SCIF_SCR_RCV_TRN_DIS); /* Disable receive & transmit */
> > + set_reg(SCIF_REG_FCR, SCIF_FCR_RST_ASSRT_TFRF); /* Reset assert tx-FIFO & rx-FIFO */
> > +
> > + data16 = get_reg(SCIF_REG_FSR); /* Dummy read */
> > + set_reg(SCIF_REG_FSR, 0x0); /* Clear all error bit */
> > +
> > + data16 = get_reg(SCIF_REG_LSR); /* Dummy read */
> > + set_reg(SCIF_REG_LSR, 0x0); /* Clear ORER bit */
> > +
> > + set_reg(SCIF_REG_SCR, 0x0); /* Select internal clock, SC_CLK pin unused for output pin */
> > +
> > + set_reg(SCIF_REG_SMR, 0x0); /* Set asynchronous, 8bit data, no-parity, 1 stop and Po/1 */
> > +
> > + data16 = get_reg(SCIF_REG_SEMR);
> > + set_reg(SCIF_REG_SEMR, data16 & (~SCIF_SEMR_MDDRS)); /* Select to access BRR */
> > + set_reg(SCIF_REG_BRR, SCBRR_VALUE(in_freq, baudrate));
> > +
> > + scif_wait(baudrate);
>
> What is this?
>
As mentioned above I'll get rid of it.
> > +
> > + /* FTCR is left at initial value, because this interrupt isn't used. */
>
> What is FTCR? There is no such register in your defines.
>
It's the "FIFO Trigger Control Register". I'll drop the comment.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver
2022-11-15 17:33 ` Lad, Prabhakar
@ 2022-11-17 7:52 ` Bin Meng
2022-11-18 13:07 ` Lad, Prabhakar
0 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2022-11-17 7:52 UTC (permalink / raw)
To: opensbi
Hi Prabhakar,
On Wed, Nov 16, 2022 at 1:34 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Bin,
>
> Thank you for the review.
>
> On Tue, Nov 15, 2022 at 7:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Sat, Nov 12, 2022 at 2:22 AM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > >
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Add Renesas SCIF driver.
> > >
> > > Based on a patch in the BSP by Takeki Hamada
> > > <takeki.hamada.ak@bp.renesas.com>
> >
> > If there is a public URL for the BSP you mentioned, better put that in
> > the commit here for future reference.
> >
> you mean link for the BSP/File/Commit. Ive followed the same approach
> that we follow on Linux.
Do you have any link for the BSP files, ie: on Renesas website?
>
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > > RFC->v2
> > > * Fixed comments pointed by Xiang W
> > > ---
> > > include/sbi_utils/serial/renesas-scif.h | 11 ++
> > > lib/utils/serial/Kconfig | 4 +
> > > lib/utils/serial/objects.mk | 1 +
> > > lib/utils/serial/renesas_scif.c | 139 ++++++++++++++++++++++++
> > > 4 files changed, 155 insertions(+)
> > > create mode 100644 include/sbi_utils/serial/renesas-scif.h
> > > create mode 100644 lib/utils/serial/renesas_scif.c
> > >
> > > diff --git a/include/sbi_utils/serial/renesas-scif.h b/include/sbi_utils/serial/renesas-scif.h
> > > new file mode 100644
> > > index 0000000..0002a1a
> > > --- /dev/null
> > > +++ b/include/sbi_utils/serial/renesas-scif.h
> > > @@ -0,0 +1,11 @@
> > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > +/*
> > > + * Copyright (C) 2022 Renesas Electronics Corporation
> > > + */
> > > +
> > > +#ifndef __SERIAL_RENESAS_SCIF_H__
> > > +#define __SERIAL_RENESAS_SCIF_H__
> > > +
> > > +int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate);
> > > +
> > > +#endif /* __SERIAL_RENESAS_SCIF_H__ */
> > > diff --git a/lib/utils/serial/Kconfig b/lib/utils/serial/Kconfig
> > > index da549a7..f6ed803 100644
> > > --- a/lib/utils/serial/Kconfig
> > > +++ b/lib/utils/serial/Kconfig
> > > @@ -59,6 +59,10 @@ config SERIAL_GAISLER
> > > bool "Gaisler UART support"
> > > default n
> > >
> > > +config SERIAL_RENESAS_SCIF
> > > + bool "Renesas SCIF support"
> > > + default n
> > > +
> > > config SERIAL_SHAKTI
> > > bool "Shakti UART support"
> > > default n
> > > diff --git a/lib/utils/serial/objects.mk b/lib/utils/serial/objects.mk
> > > index 98f3f9a..6520424 100644
> > > --- a/lib/utils/serial/objects.mk
> > > +++ b/lib/utils/serial/objects.mk
> > > @@ -36,6 +36,7 @@ libsbiutils-objs-$(CONFIG_FDT_SERIAL_XILINX_UARTLITE) += serial/fdt_serial_xlnx_
> > >
> > > libsbiutils-objs-$(CONFIG_SERIAL_CADENCE) += serial/cadence-uart.o
> > > libsbiutils-objs-$(CONFIG_SERIAL_GAISLER) += serial/gaisler-uart.o
> > > +libsbiutils-objs-$(CONFIG_SERIAL_RENESAS_SCIF) += serial/renesas_scif.o
> > > libsbiutils-objs-$(CONFIG_SERIAL_SHAKTI) += serial/shakti-uart.o
> > > libsbiutils-objs-$(CONFIG_SERIAL_SIFIVE) += serial/sifive-uart.o
> > > libsbiutils-objs-$(CONFIG_SERIAL_LITEX) += serial/litex-uart.o
> > > diff --git a/lib/utils/serial/renesas_scif.c b/lib/utils/serial/renesas_scif.c
> > > new file mode 100644
> > > index 0000000..0586e1a
> > > --- /dev/null
> > > +++ b/lib/utils/serial/renesas_scif.c
> > > @@ -0,0 +1,139 @@
> > > +// SPDX-License-Identifier: BSD-2-Clause
> > > +/*
> > > + * Copyright (C) 2022 Renesas Electronics Corporation
> > > + */
> > > +
> > > +#include <sbi/riscv_io.h>
> > > +#include <sbi/sbi_console.h>
> > > +#include <sbi/sbi_timer.h>
> > > +#include <sbi_utils/serial/renesas-scif.h>
> > > +
> > > +/* clang-format off */
> > > +
> > > +#define SCIF_REG_SMR 0x0
> > > +#define SCIF_REG_BRR 0x2
> > > +#define SCIF_REG_SCR 0x4
> > > +#define SCIF_REG_FTDR 0x6
> > > +#define SCIF_REG_FSR 0x8
> > > +#define SCIF_REG_FRDR 0xa
> >
> > This register is not used by the driver. Drop it.
> >
> OK, I will drop it.
>
> > > +#define SCIF_REG_FCR 0xc
> > > +#define SCIF_REG_LSR 0x12
> > > +#define SCIF_REG_SEMR 0x14
> > > +
> > > +#define SCIF_RFRST 0x2 /* Reset assert receive-FIFO (bit[1]) */
> > > +#define SCIF_TFRST 0x4 /* Reset assert transmit-FIFO(bit[2]) */
> >
> > The above 2 macros should probably be named as SCIF_FCR_* as they only
> > apply to the FCR register.
> >
> Agreed.
>
> > > +
> > > +#define SCIF_FCR_RST_ASSRT_TFRF (SCIF_RFRST | SCIF_TFRST) /* Reset assert tx-FIFO & rx-FIFO */
> >
> > The macro name should probably be named to have RF come before TF,
> > like SCIF_FCR_RST_ASSRT_RFTF, to keep in consistency with the rest,
> > and the codes.
> >
> Agreed.
>
> > #define SCIF_FCR_RST_ASSRT_RFTF (SCIF_RFRST | SCIF_TFRST) /*
> > Reset assert rx-FIFO & tx-FIFO */
> >
> > > +#define SCIF_FCR_RST_NGATE_TFRF 0x0 /* Reset negate tx-FIFO & rx-FIFO*/
> >
> > ditto
> >
> > need a space before */
> >
> OK, I will fix it.
>
> > > +
> > > +#define SCIF_RE 0x10 /* Enable receive (bit[4]) */
> > > +#define SCIF_TE 0x20 /* Enable transmit(bit[5]) */
> >
> > The above 2 macros should probably be named as SCIF_SCR_* as they only
> > apply to the SCR register.
> >
> Agreed.
>
> > nits: only need one space fore */
> >
> > > +#define SCIF_SCR_RCV_TRN_EN (SCIF_RE | SCIF_TE) /* Enable receive & transmit */
> > > +#define SCIF_SCR_RCV_TRN_DIS 0x0 /* Disable receive & transmit */
> > > +
> > > +#define SCIF_FSR_ER 0x80 /* Receive error flag */
> > > +#define SCIF_FSR_TEND 0x40 /* Detect break flag */
> > > +#define SCIF_FSR_TDFE 0x20 /* Detect break flag */
> >
> > The comments of the above 2 bit fields are wrong
> >
> Ouch will fix that.
>
> > > +#define SCIF_FSR_BRK 0x10 /* Detect break flag */
> > > +#define SCIF_FSR_RDF 0x2 /* Receive FIFO data full flag */
> >
> > This bit field seems not to be used anywhere. Drop it?
> >
> Sure, I will drop it.
>
> > > +#define SCIF_FSR_DR 0x1 /* Receive data ready flag */
> > > +
> > > +#define SCIF_FSR_RXD_CHK (SCIF_FSR_ER | SCIF_FSR_BRK | SCIF_FSR_DR)
> > > +#define SCIF_FSR_TXD_CHK (SCIF_FSR_TEND | SCIF_FSR_TDFE)
> > > +
> > > +#define SCIF_LSR_ORER 0x1 /* Overrun error flag */
> >
> > This macro is not used anywhere. Drop it.
> >
> ditto.
>
> > > +
> > > +#define SCIF_SPTR_SPB2DT 0x1 /* if SCR.TE setting, don't care */
> > > +#define SCIF_SPTR_SPB2IO 0x2 /* if SCR.TE setting, don't care */
> >
> > The above 2 macros are not used anywhere. Drop them.
> >
> ditto.
>
> > > +
> > > +#define SCIF_SEMR_BRME 0x20 /* bit-rate modulation enable */
> >
> > This macro is not used anywhere. Drop it.
> >
> ditto.
>
> > > +#define SCIF_SEMR_MDDRS 0x10 /* MDDR access enable */
> > > +
> > > +#define SCIF_SIZE(reg) ((reg == SCIF_REG_BRR) || \
> > > + (reg == SCIF_REG_FTDR) || \
> > > + (reg == SCIF_REG_FRDR) || \
> >
> > I don't see the driver programs SCIF_REG_FRDR register. We can just
> > remove it to save some cycles.
> >
> > > + (reg == SCIF_REG_SEMR))
> >
> > The name of this macro is not meaningful. It looks like it's the size
> > of a register at the first glance but it's actually returning logical
> > true or false.
> > Better rename it to something like SCIF_REG_8BIT(reg).
> >
> Good point, I'll rename it.
>
> > > +
> > > +#define SCBRR_VALUE(clk, baudrate) ((clk) / (32 * (baudrate)) - 1)
> > > +
> > > +/* clang-format on */
> > > +
> > > +static volatile char *scif_base;
> > > +
> > > +static u32 get_reg(u32 offset)
> > > +{
> > > + if (SCIF_SIZE(offset))
> > > + return readb(scif_base + offset);
> > > +
> > > + return readw(scif_base + offset);
> > > +}
> > > +
> > > +static void set_reg(u32 offset, u32 val)
> > > +{
> > > + if (SCIF_SIZE(offset))
> > > + return writeb(val, scif_base + offset);
> > > +
> > > + return writew(val, scif_base + offset);
> > > +}
> > > +
> > > +static void scif_wait(unsigned long baudrate)
> > > +{
> > > + unsigned long utime;
> > > +
> > > + utime = 1000000 / baudrate + 1;
> >
> > What is this?
> >
> It's the delay required when using the external clock. But as we arent
> using it on RZ/Five SMARC we can get rid of it for now.
Could we make such logic be based on DTS settings, instead of hardcoding?
This driver should be able to support both internal and external clocks, right?
>
> > > +
> > > + sbi_timer_udelay(utime);
> > > +}
> > > +
> > > +static void renesas_scif_putc(char ch)
> > > +{
> > > + uint16_t reg;
> > > +
> > > + while (!(SCIF_FSR_TXD_CHK & get_reg(SCIF_REG_FSR)))
> > > + ;
> > > +
> > > + set_reg(SCIF_REG_FTDR, ch);
> > > + reg = get_reg(SCIF_REG_FSR);
> > > + reg &= ~SCIF_FSR_TXD_CHK;
> > > + set_reg(SCIF_REG_FSR, reg);
> >
> > The FSR Tx status bits clear should happen before writing a new char
> > to the Tx fifo, no?
> >
> No, this has to be cleared only after char write.
Based on my read of the user manual, after the while () check, we can
clear the status bits. After you write a new char to the FIFO, the
status bits might be triggered by the new char write, instead of the
previous one. Hence clearing status bits after writing char will cause
the status bits check next time fail.
>
> > > +}
> > > +
> > > +static struct sbi_console_device renesas_scif_console = {
> > > + .name = "renesas_scif",
> > > + .console_putc = renesas_scif_putc,
> > > +};
> > > +
> > > +int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate)
> > > +{
> > > + volatile uint16_t data16;
> > > +
> > > + scif_base = (volatile char *)base;
> > > +
> > > + set_reg(SCIF_REG_SCR, SCIF_SCR_RCV_TRN_DIS); /* Disable receive & transmit */
> > > + set_reg(SCIF_REG_FCR, SCIF_FCR_RST_ASSRT_TFRF); /* Reset assert tx-FIFO & rx-FIFO */
> > > +
> > > + data16 = get_reg(SCIF_REG_FSR); /* Dummy read */
> > > + set_reg(SCIF_REG_FSR, 0x0); /* Clear all error bit */
> > > +
> > > + data16 = get_reg(SCIF_REG_LSR); /* Dummy read */
> > > + set_reg(SCIF_REG_LSR, 0x0); /* Clear ORER bit */
> > > +
> > > + set_reg(SCIF_REG_SCR, 0x0); /* Select internal clock, SC_CLK pin unused for output pin */
> > > +
> > > + set_reg(SCIF_REG_SMR, 0x0); /* Set asynchronous, 8bit data, no-parity, 1 stop and Po/1 */
> > > +
> > > + data16 = get_reg(SCIF_REG_SEMR);
> > > + set_reg(SCIF_REG_SEMR, data16 & (~SCIF_SEMR_MDDRS)); /* Select to access BRR */
> > > + set_reg(SCIF_REG_BRR, SCBRR_VALUE(in_freq, baudrate));
> > > +
> > > + scif_wait(baudrate);
> >
> > What is this?
> >
> As mentioned above I'll get rid of it.
>
> > > +
> > > + /* FTCR is left at initial value, because this interrupt isn't used. */
> >
> > What is FTCR? There is no such register in your defines.
> >
> It's the "FIFO Trigger Control Register". I'll drop the comment.
>
Regards,
Bin
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver
2022-11-17 7:52 ` Bin Meng
@ 2022-11-18 13:07 ` Lad, Prabhakar
2022-11-20 11:59 ` Bin Meng
0 siblings, 1 reply; 20+ messages in thread
From: Lad, Prabhakar @ 2022-11-18 13:07 UTC (permalink / raw)
To: opensbi
Hi Bin,
On Thu, Nov 17, 2022 at 7:52 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Prabhakar,
>
> On Wed, Nov 16, 2022 at 1:34 AM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> >
> > Hi Bin,
> >
> > Thank you for the review.
> >
> > On Tue, Nov 15, 2022 at 7:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > On Sat, Nov 12, 2022 at 2:22 AM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > >
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Add Renesas SCIF driver.
> > > >
> > > > Based on a patch in the BSP by Takeki Hamada
> > > > <takeki.hamada.ak@bp.renesas.com>
> > >
> > > If there is a public URL for the BSP you mentioned, better put that in
> > > the commit here for future reference.
> > >
> > you mean link for the BSP/File/Commit. Ive followed the same approach
> > that we follow on Linux.
>
> Do you have any link for the BSP files, ie: on Renesas website?
>
We have a meta layer link [1] on the website but we got github links
to the files/commit-id too if this is needed.
https://www.renesas.com/us/en/software-tool/rzfive-board-support-package-v10-update1-510-cip#overview
> >
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > ---
> > > > RFC->v2
> > > > * Fixed comments pointed by Xiang W
> > > > ---
> > > > include/sbi_utils/serial/renesas-scif.h | 11 ++
> > > > lib/utils/serial/Kconfig | 4 +
> > > > lib/utils/serial/objects.mk | 1 +
> > > > lib/utils/serial/renesas_scif.c | 139 ++++++++++++++++++++++++
> > > > 4 files changed, 155 insertions(+)
> > > > create mode 100644 include/sbi_utils/serial/renesas-scif.h
> > > > create mode 100644 lib/utils/serial/renesas_scif.c
> > > >
> > > > diff --git a/include/sbi_utils/serial/renesas-scif.h b/include/sbi_utils/serial/renesas-scif.h
> > > > new file mode 100644
> > > > index 0000000..0002a1a
> > > > --- /dev/null
> > > > +++ b/include/sbi_utils/serial/renesas-scif.h
> > > > @@ -0,0 +1,11 @@
> > > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > > +/*
> > > > + * Copyright (C) 2022 Renesas Electronics Corporation
> > > > + */
> > > > +
> > > > +#ifndef __SERIAL_RENESAS_SCIF_H__
> > > > +#define __SERIAL_RENESAS_SCIF_H__
> > > > +
> > > > +int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate);
> > > > +
> > > > +#endif /* __SERIAL_RENESAS_SCIF_H__ */
> > > > diff --git a/lib/utils/serial/Kconfig b/lib/utils/serial/Kconfig
> > > > index da549a7..f6ed803 100644
> > > > --- a/lib/utils/serial/Kconfig
> > > > +++ b/lib/utils/serial/Kconfig
> > > > @@ -59,6 +59,10 @@ config SERIAL_GAISLER
> > > > bool "Gaisler UART support"
> > > > default n
> > > >
> > > > +config SERIAL_RENESAS_SCIF
> > > > + bool "Renesas SCIF support"
> > > > + default n
> > > > +
> > > > config SERIAL_SHAKTI
> > > > bool "Shakti UART support"
> > > > default n
> > > > diff --git a/lib/utils/serial/objects.mk b/lib/utils/serial/objects.mk
> > > > index 98f3f9a..6520424 100644
> > > > --- a/lib/utils/serial/objects.mk
> > > > +++ b/lib/utils/serial/objects.mk
> > > > @@ -36,6 +36,7 @@ libsbiutils-objs-$(CONFIG_FDT_SERIAL_XILINX_UARTLITE) += serial/fdt_serial_xlnx_
> > > >
> > > > libsbiutils-objs-$(CONFIG_SERIAL_CADENCE) += serial/cadence-uart.o
> > > > libsbiutils-objs-$(CONFIG_SERIAL_GAISLER) += serial/gaisler-uart.o
> > > > +libsbiutils-objs-$(CONFIG_SERIAL_RENESAS_SCIF) += serial/renesas_scif.o
> > > > libsbiutils-objs-$(CONFIG_SERIAL_SHAKTI) += serial/shakti-uart.o
> > > > libsbiutils-objs-$(CONFIG_SERIAL_SIFIVE) += serial/sifive-uart.o
> > > > libsbiutils-objs-$(CONFIG_SERIAL_LITEX) += serial/litex-uart.o
> > > > diff --git a/lib/utils/serial/renesas_scif.c b/lib/utils/serial/renesas_scif.c
> > > > new file mode 100644
> > > > index 0000000..0586e1a
> > > > --- /dev/null
> > > > +++ b/lib/utils/serial/renesas_scif.c
> > > > @@ -0,0 +1,139 @@
> > > > +// SPDX-License-Identifier: BSD-2-Clause
> > > > +/*
> > > > + * Copyright (C) 2022 Renesas Electronics Corporation
> > > > + */
> > > > +
> > > > +#include <sbi/riscv_io.h>
> > > > +#include <sbi/sbi_console.h>
> > > > +#include <sbi/sbi_timer.h>
> > > > +#include <sbi_utils/serial/renesas-scif.h>
> > > > +
> > > > +/* clang-format off */
> > > > +
> > > > +#define SCIF_REG_SMR 0x0
> > > > +#define SCIF_REG_BRR 0x2
> > > > +#define SCIF_REG_SCR 0x4
> > > > +#define SCIF_REG_FTDR 0x6
> > > > +#define SCIF_REG_FSR 0x8
> > > > +#define SCIF_REG_FRDR 0xa
> > >
> > > This register is not used by the driver. Drop it.
> > >
> > OK, I will drop it.
> >
> > > > +#define SCIF_REG_FCR 0xc
> > > > +#define SCIF_REG_LSR 0x12
> > > > +#define SCIF_REG_SEMR 0x14
> > > > +
> > > > +#define SCIF_RFRST 0x2 /* Reset assert receive-FIFO (bit[1]) */
> > > > +#define SCIF_TFRST 0x4 /* Reset assert transmit-FIFO(bit[2]) */
> > >
> > > The above 2 macros should probably be named as SCIF_FCR_* as they only
> > > apply to the FCR register.
> > >
> > Agreed.
> >
> > > > +
> > > > +#define SCIF_FCR_RST_ASSRT_TFRF (SCIF_RFRST | SCIF_TFRST) /* Reset assert tx-FIFO & rx-FIFO */
> > >
> > > The macro name should probably be named to have RF come before TF,
> > > like SCIF_FCR_RST_ASSRT_RFTF, to keep in consistency with the rest,
> > > and the codes.
> > >
> > Agreed.
> >
> > > #define SCIF_FCR_RST_ASSRT_RFTF (SCIF_RFRST | SCIF_TFRST) /*
> > > Reset assert rx-FIFO & tx-FIFO */
> > >
> > > > +#define SCIF_FCR_RST_NGATE_TFRF 0x0 /* Reset negate tx-FIFO & rx-FIFO*/
> > >
> > > ditto
> > >
> > > need a space before */
> > >
> > OK, I will fix it.
> >
> > > > +
> > > > +#define SCIF_RE 0x10 /* Enable receive (bit[4]) */
> > > > +#define SCIF_TE 0x20 /* Enable transmit(bit[5]) */
> > >
> > > The above 2 macros should probably be named as SCIF_SCR_* as they only
> > > apply to the SCR register.
> > >
> > Agreed.
> >
> > > nits: only need one space fore */
> > >
> > > > +#define SCIF_SCR_RCV_TRN_EN (SCIF_RE | SCIF_TE) /* Enable receive & transmit */
> > > > +#define SCIF_SCR_RCV_TRN_DIS 0x0 /* Disable receive & transmit */
> > > > +
> > > > +#define SCIF_FSR_ER 0x80 /* Receive error flag */
> > > > +#define SCIF_FSR_TEND 0x40 /* Detect break flag */
> > > > +#define SCIF_FSR_TDFE 0x20 /* Detect break flag */
> > >
> > > The comments of the above 2 bit fields are wrong
> > >
> > Ouch will fix that.
> >
> > > > +#define SCIF_FSR_BRK 0x10 /* Detect break flag */
> > > > +#define SCIF_FSR_RDF 0x2 /* Receive FIFO data full flag */
> > >
> > > This bit field seems not to be used anywhere. Drop it?
> > >
> > Sure, I will drop it.
> >
> > > > +#define SCIF_FSR_DR 0x1 /* Receive data ready flag */
> > > > +
> > > > +#define SCIF_FSR_RXD_CHK (SCIF_FSR_ER | SCIF_FSR_BRK | SCIF_FSR_DR)
> > > > +#define SCIF_FSR_TXD_CHK (SCIF_FSR_TEND | SCIF_FSR_TDFE)
> > > > +
> > > > +#define SCIF_LSR_ORER 0x1 /* Overrun error flag */
> > >
> > > This macro is not used anywhere. Drop it.
> > >
> > ditto.
> >
> > > > +
> > > > +#define SCIF_SPTR_SPB2DT 0x1 /* if SCR.TE setting, don't care */
> > > > +#define SCIF_SPTR_SPB2IO 0x2 /* if SCR.TE setting, don't care */
> > >
> > > The above 2 macros are not used anywhere. Drop them.
> > >
> > ditto.
> >
> > > > +
> > > > +#define SCIF_SEMR_BRME 0x20 /* bit-rate modulation enable */
> > >
> > > This macro is not used anywhere. Drop it.
> > >
> > ditto.
> >
> > > > +#define SCIF_SEMR_MDDRS 0x10 /* MDDR access enable */
> > > > +
> > > > +#define SCIF_SIZE(reg) ((reg == SCIF_REG_BRR) || \
> > > > + (reg == SCIF_REG_FTDR) || \
> > > > + (reg == SCIF_REG_FRDR) || \
> > >
> > > I don't see the driver programs SCIF_REG_FRDR register. We can just
> > > remove it to save some cycles.
> > >
> > > > + (reg == SCIF_REG_SEMR))
> > >
> > > The name of this macro is not meaningful. It looks like it's the size
> > > of a register at the first glance but it's actually returning logical
> > > true or false.
> > > Better rename it to something like SCIF_REG_8BIT(reg).
> > >
> > Good point, I'll rename it.
> >
> > > > +
> > > > +#define SCBRR_VALUE(clk, baudrate) ((clk) / (32 * (baudrate)) - 1)
> > > > +
> > > > +/* clang-format on */
> > > > +
> > > > +static volatile char *scif_base;
> > > > +
> > > > +static u32 get_reg(u32 offset)
> > > > +{
> > > > + if (SCIF_SIZE(offset))
> > > > + return readb(scif_base + offset);
> > > > +
> > > > + return readw(scif_base + offset);
> > > > +}
> > > > +
> > > > +static void set_reg(u32 offset, u32 val)
> > > > +{
> > > > + if (SCIF_SIZE(offset))
> > > > + return writeb(val, scif_base + offset);
> > > > +
> > > > + return writew(val, scif_base + offset);
> > > > +}
> > > > +
> > > > +static void scif_wait(unsigned long baudrate)
> > > > +{
> > > > + unsigned long utime;
> > > > +
> > > > + utime = 1000000 / baudrate + 1;
> > >
> > > What is this?
> > >
> > It's the delay required when using the external clock. But as we arent
> > using it on RZ/Five SMARC we can get rid of it for now.
>
> Could we make such logic be based on DTS settings, instead of hardcoding?
>
> This driver should be able to support both internal and external clocks, right?
>
Agreed it can be checked by DTS. For now I will drop this until I have
the HW to test this with EXT clock.
> >
> > > > +
> > > > + sbi_timer_udelay(utime);
> > > > +}
> > > > +
> > > > +static void renesas_scif_putc(char ch)
> > > > +{
> > > > + uint16_t reg;
> > > > +
> > > > + while (!(SCIF_FSR_TXD_CHK & get_reg(SCIF_REG_FSR)))
> > > > + ;
> > > > +
> > > > + set_reg(SCIF_REG_FTDR, ch);
> > > > + reg = get_reg(SCIF_REG_FSR);
> > > > + reg &= ~SCIF_FSR_TXD_CHK;
> > > > + set_reg(SCIF_REG_FSR, reg);
> > >
> > > The FSR Tx status bits clear should happen before writing a new char
> > > to the Tx fifo, no?
> > >
> > No, this has to be cleared only after char write.
>
> Based on my read of the user manual, after the while () check, we can
> clear the status bits. After you write a new char to the FIFO, the
> status bits might be triggered by the new char write, instead of the
> previous one. Hence clearing status bits after writing char will cause
> the status bits check next time fail.
>
From the HW manual..
[Clearing conditions]
? TDFE is cleared to 0 when 0 is written in the TDFE bit after reading TDFE = 1.
? When transmit data is written to the FTDR register
^^ hence its cleared after the write here.
In Linux and u-boot too we follow the same approach with the existing
SCI/F driver.
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver
2022-11-18 13:07 ` Lad, Prabhakar
@ 2022-11-20 11:59 ` Bin Meng
2022-11-29 12:13 ` Lad, Prabhakar
0 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2022-11-20 11:59 UTC (permalink / raw)
To: opensbi
Hi Prabhakar,
On Fri, Nov 18, 2022 at 9:08 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Bin,
>
> On Thu, Nov 17, 2022 at 7:52 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Prabhakar,
> >
> > On Wed, Nov 16, 2022 at 1:34 AM Lad, Prabhakar
> > <prabhakar.csengg@gmail.com> wrote:
> > >
> > > Hi Bin,
> > >
> > > Thank you for the review.
> > >
> > > On Tue, Nov 15, 2022 at 7:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > On Sat, Nov 12, 2022 at 2:22 AM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > > >
> > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > >
> > > > > Add Renesas SCIF driver.
> > > > >
> > > > > Based on a patch in the BSP by Takeki Hamada
> > > > > <takeki.hamada.ak@bp.renesas.com>
> > > >
> > > > If there is a public URL for the BSP you mentioned, better put that in
> > > > the commit here for future reference.
> > > >
> > > you mean link for the BSP/File/Commit. Ive followed the same approach
> > > that we follow on Linux.
> >
> > Do you have any link for the BSP files, ie: on Renesas website?
> >
> We have a meta layer link [1] on the website but we got github links
> to the files/commit-id too if this is needed.
>
> https://www.renesas.com/us/en/software-tool/rzfive-board-support-package-v10-update1-510-cip#overview
>
> > >
> > > > >
> > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > ---
> > > > > RFC->v2
> > > > > * Fixed comments pointed by Xiang W
> > > > > ---
> > > > > include/sbi_utils/serial/renesas-scif.h | 11 ++
> > > > > lib/utils/serial/Kconfig | 4 +
> > > > > lib/utils/serial/objects.mk | 1 +
> > > > > lib/utils/serial/renesas_scif.c | 139 ++++++++++++++++++++++++
> > > > > 4 files changed, 155 insertions(+)
> > > > > create mode 100644 include/sbi_utils/serial/renesas-scif.h
> > > > > create mode 100644 lib/utils/serial/renesas_scif.c
> > > > >
> > > > > diff --git a/include/sbi_utils/serial/renesas-scif.h b/include/sbi_utils/serial/renesas-scif.h
> > > > > new file mode 100644
> > > > > index 0000000..0002a1a
> > > > > --- /dev/null
> > > > > +++ b/include/sbi_utils/serial/renesas-scif.h
> > > > > @@ -0,0 +1,11 @@
> > > > > +/* SPDX-License-Identifier: BSD-2-Clause */
> > > > > +/*
> > > > > + * Copyright (C) 2022 Renesas Electronics Corporation
> > > > > + */
> > > > > +
> > > > > +#ifndef __SERIAL_RENESAS_SCIF_H__
> > > > > +#define __SERIAL_RENESAS_SCIF_H__
> > > > > +
> > > > > +int renesas_scif_init(unsigned long base, u32 in_freq, u32 baudrate);
> > > > > +
> > > > > +#endif /* __SERIAL_RENESAS_SCIF_H__ */
> > > > > diff --git a/lib/utils/serial/Kconfig b/lib/utils/serial/Kconfig
> > > > > index da549a7..f6ed803 100644
> > > > > --- a/lib/utils/serial/Kconfig
> > > > > +++ b/lib/utils/serial/Kconfig
> > > > > @@ -59,6 +59,10 @@ config SERIAL_GAISLER
> > > > > bool "Gaisler UART support"
> > > > > default n
> > > > >
> > > > > +config SERIAL_RENESAS_SCIF
> > > > > + bool "Renesas SCIF support"
> > > > > + default n
> > > > > +
> > > > > config SERIAL_SHAKTI
> > > > > bool "Shakti UART support"
> > > > > default n
> > > > > diff --git a/lib/utils/serial/objects.mk b/lib/utils/serial/objects.mk
> > > > > index 98f3f9a..6520424 100644
> > > > > --- a/lib/utils/serial/objects.mk
> > > > > +++ b/lib/utils/serial/objects.mk
> > > > > @@ -36,6 +36,7 @@ libsbiutils-objs-$(CONFIG_FDT_SERIAL_XILINX_UARTLITE) += serial/fdt_serial_xlnx_
> > > > >
> > > > > libsbiutils-objs-$(CONFIG_SERIAL_CADENCE) += serial/cadence-uart.o
> > > > > libsbiutils-objs-$(CONFIG_SERIAL_GAISLER) += serial/gaisler-uart.o
> > > > > +libsbiutils-objs-$(CONFIG_SERIAL_RENESAS_SCIF) += serial/renesas_scif.o
> > > > > libsbiutils-objs-$(CONFIG_SERIAL_SHAKTI) += serial/shakti-uart.o
> > > > > libsbiutils-objs-$(CONFIG_SERIAL_SIFIVE) += serial/sifive-uart.o
> > > > > libsbiutils-objs-$(CONFIG_SERIAL_LITEX) += serial/litex-uart.o
> > > > > diff --git a/lib/utils/serial/renesas_scif.c b/lib/utils/serial/renesas_scif.c
> > > > > new file mode 100644
> > > > > index 0000000..0586e1a
> > > > > --- /dev/null
> > > > > +++ b/lib/utils/serial/renesas_scif.c
> > > > > @@ -0,0 +1,139 @@
> > > > > +// SPDX-License-Identifier: BSD-2-Clause
> > > > > +/*
> > > > > + * Copyright (C) 2022 Renesas Electronics Corporation
> > > > > + */
> > > > > +
> > > > > +#include <sbi/riscv_io.h>
> > > > > +#include <sbi/sbi_console.h>
> > > > > +#include <sbi/sbi_timer.h>
> > > > > +#include <sbi_utils/serial/renesas-scif.h>
> > > > > +
> > > > > +/* clang-format off */
> > > > > +
> > > > > +#define SCIF_REG_SMR 0x0
> > > > > +#define SCIF_REG_BRR 0x2
> > > > > +#define SCIF_REG_SCR 0x4
> > > > > +#define SCIF_REG_FTDR 0x6
> > > > > +#define SCIF_REG_FSR 0x8
> > > > > +#define SCIF_REG_FRDR 0xa
> > > >
> > > > This register is not used by the driver. Drop it.
> > > >
> > > OK, I will drop it.
> > >
> > > > > +#define SCIF_REG_FCR 0xc
> > > > > +#define SCIF_REG_LSR 0x12
> > > > > +#define SCIF_REG_SEMR 0x14
> > > > > +
> > > > > +#define SCIF_RFRST 0x2 /* Reset assert receive-FIFO (bit[1]) */
> > > > > +#define SCIF_TFRST 0x4 /* Reset assert transmit-FIFO(bit[2]) */
> > > >
> > > > The above 2 macros should probably be named as SCIF_FCR_* as they only
> > > > apply to the FCR register.
> > > >
> > > Agreed.
> > >
> > > > > +
> > > > > +#define SCIF_FCR_RST_ASSRT_TFRF (SCIF_RFRST | SCIF_TFRST) /* Reset assert tx-FIFO & rx-FIFO */
> > > >
> > > > The macro name should probably be named to have RF come before TF,
> > > > like SCIF_FCR_RST_ASSRT_RFTF, to keep in consistency with the rest,
> > > > and the codes.
> > > >
> > > Agreed.
> > >
> > > > #define SCIF_FCR_RST_ASSRT_RFTF (SCIF_RFRST | SCIF_TFRST) /*
> > > > Reset assert rx-FIFO & tx-FIFO */
> > > >
> > > > > +#define SCIF_FCR_RST_NGATE_TFRF 0x0 /* Reset negate tx-FIFO & rx-FIFO*/
> > > >
> > > > ditto
> > > >
> > > > need a space before */
> > > >
> > > OK, I will fix it.
> > >
> > > > > +
> > > > > +#define SCIF_RE 0x10 /* Enable receive (bit[4]) */
> > > > > +#define SCIF_TE 0x20 /* Enable transmit(bit[5]) */
> > > >
> > > > The above 2 macros should probably be named as SCIF_SCR_* as they only
> > > > apply to the SCR register.
> > > >
> > > Agreed.
> > >
> > > > nits: only need one space fore */
> > > >
> > > > > +#define SCIF_SCR_RCV_TRN_EN (SCIF_RE | SCIF_TE) /* Enable receive & transmit */
> > > > > +#define SCIF_SCR_RCV_TRN_DIS 0x0 /* Disable receive & transmit */
> > > > > +
> > > > > +#define SCIF_FSR_ER 0x80 /* Receive error flag */
> > > > > +#define SCIF_FSR_TEND 0x40 /* Detect break flag */
> > > > > +#define SCIF_FSR_TDFE 0x20 /* Detect break flag */
> > > >
> > > > The comments of the above 2 bit fields are wrong
> > > >
> > > Ouch will fix that.
> > >
> > > > > +#define SCIF_FSR_BRK 0x10 /* Detect break flag */
> > > > > +#define SCIF_FSR_RDF 0x2 /* Receive FIFO data full flag */
> > > >
> > > > This bit field seems not to be used anywhere. Drop it?
> > > >
> > > Sure, I will drop it.
> > >
> > > > > +#define SCIF_FSR_DR 0x1 /* Receive data ready flag */
> > > > > +
> > > > > +#define SCIF_FSR_RXD_CHK (SCIF_FSR_ER | SCIF_FSR_BRK | SCIF_FSR_DR)
> > > > > +#define SCIF_FSR_TXD_CHK (SCIF_FSR_TEND | SCIF_FSR_TDFE)
> > > > > +
> > > > > +#define SCIF_LSR_ORER 0x1 /* Overrun error flag */
> > > >
> > > > This macro is not used anywhere. Drop it.
> > > >
> > > ditto.
> > >
> > > > > +
> > > > > +#define SCIF_SPTR_SPB2DT 0x1 /* if SCR.TE setting, don't care */
> > > > > +#define SCIF_SPTR_SPB2IO 0x2 /* if SCR.TE setting, don't care */
> > > >
> > > > The above 2 macros are not used anywhere. Drop them.
> > > >
> > > ditto.
> > >
> > > > > +
> > > > > +#define SCIF_SEMR_BRME 0x20 /* bit-rate modulation enable */
> > > >
> > > > This macro is not used anywhere. Drop it.
> > > >
> > > ditto.
> > >
> > > > > +#define SCIF_SEMR_MDDRS 0x10 /* MDDR access enable */
> > > > > +
> > > > > +#define SCIF_SIZE(reg) ((reg == SCIF_REG_BRR) || \
> > > > > + (reg == SCIF_REG_FTDR) || \
> > > > > + (reg == SCIF_REG_FRDR) || \
> > > >
> > > > I don't see the driver programs SCIF_REG_FRDR register. We can just
> > > > remove it to save some cycles.
> > > >
> > > > > + (reg == SCIF_REG_SEMR))
> > > >
> > > > The name of this macro is not meaningful. It looks like it's the size
> > > > of a register at the first glance but it's actually returning logical
> > > > true or false.
> > > > Better rename it to something like SCIF_REG_8BIT(reg).
> > > >
> > > Good point, I'll rename it.
> > >
> > > > > +
> > > > > +#define SCBRR_VALUE(clk, baudrate) ((clk) / (32 * (baudrate)) - 1)
> > > > > +
> > > > > +/* clang-format on */
> > > > > +
> > > > > +static volatile char *scif_base;
> > > > > +
> > > > > +static u32 get_reg(u32 offset)
> > > > > +{
> > > > > + if (SCIF_SIZE(offset))
> > > > > + return readb(scif_base + offset);
> > > > > +
> > > > > + return readw(scif_base + offset);
> > > > > +}
> > > > > +
> > > > > +static void set_reg(u32 offset, u32 val)
> > > > > +{
> > > > > + if (SCIF_SIZE(offset))
> > > > > + return writeb(val, scif_base + offset);
> > > > > +
> > > > > + return writew(val, scif_base + offset);
> > > > > +}
> > > > > +
> > > > > +static void scif_wait(unsigned long baudrate)
> > > > > +{
> > > > > + unsigned long utime;
> > > > > +
> > > > > + utime = 1000000 / baudrate + 1;
> > > >
> > > > What is this?
> > > >
> > > It's the delay required when using the external clock. But as we arent
> > > using it on RZ/Five SMARC we can get rid of it for now.
> >
> > Could we make such logic be based on DTS settings, instead of hardcoding?
> >
> > This driver should be able to support both internal and external clocks, right?
> >
> Agreed it can be checked by DTS. For now I will drop this until I have
> the HW to test this with EXT clock.
>
> > >
> > > > > +
> > > > > + sbi_timer_udelay(utime);
> > > > > +}
> > > > > +
> > > > > +static void renesas_scif_putc(char ch)
> > > > > +{
> > > > > + uint16_t reg;
> > > > > +
> > > > > + while (!(SCIF_FSR_TXD_CHK & get_reg(SCIF_REG_FSR)))
> > > > > + ;
> > > > > +
> > > > > + set_reg(SCIF_REG_FTDR, ch);
> > > > > + reg = get_reg(SCIF_REG_FSR);
> > > > > + reg &= ~SCIF_FSR_TXD_CHK;
> > > > > + set_reg(SCIF_REG_FSR, reg);
> > > >
> > > > The FSR Tx status bits clear should happen before writing a new char
> > > > to the Tx fifo, no?
> > > >
> > > No, this has to be cleared only after char write.
> >
> > Based on my read of the user manual, after the while () check, we can
> > clear the status bits. After you write a new char to the FIFO, the
> > status bits might be triggered by the new char write, instead of the
> > previous one. Hence clearing status bits after writing char will cause
> > the status bits check next time fail.
> >
> From the HW manual..
> [Clearing conditions]
> ? TDFE is cleared to 0 when 0 is written in the TDFE bit after reading TDFE = 1.
> ? When transmit data is written to the FTDR register
> ^^ hence its cleared after the write here.
These 2 conditions are OR, not AND, right?
So when the while loop breaks, it satisfies the first condition, and
we can clear the TDFE bit by writing zero to FSR.
But if you write the tx data to the FTDR register, after you do
set_reg(SCIF_REG_FTDR, ch);
There is a possibility that the TDFE bit is generated before before you do
reg = get_reg(SCIF_REG_FSR);
reg &= ~SCIF_FSR_TXD_CHK;
set_reg(SCIF_REG_FSR, reg);
then the TDFE bit gets cleared unexpectedly.
Next time when you want to write another char to the serial port the
while loop will never break.
>
> In Linux and u-boot too we follow the same approach with the existing
> SCI/F driver.
Regards,
Bin
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver
2022-11-20 11:59 ` Bin Meng
@ 2022-11-29 12:13 ` Lad, Prabhakar
0 siblings, 0 replies; 20+ messages in thread
From: Lad, Prabhakar @ 2022-11-29 12:13 UTC (permalink / raw)
To: opensbi
Hi Bin,
On Sun, Nov 20, 2022 at 11:59 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Prabhakar,
>
> On Fri, Nov 18, 2022 at 9:08 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> >
> > Hi Bin,
> >
> > On Thu, Nov 17, 2022 at 7:52 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Prabhakar,
> > >
> > > On Wed, Nov 16, 2022 at 1:34 AM Lad, Prabhakar
> > > <prabhakar.csengg@gmail.com> wrote:
> > > >
> > > > Hi Bin,
<snip>
> > > > > > +static void renesas_scif_putc(char ch)
> > > > > > +{
> > > > > > + uint16_t reg;
> > > > > > +
> > > > > > + while (!(SCIF_FSR_TXD_CHK & get_reg(SCIF_REG_FSR)))
> > > > > > + ;
> > > > > > +
> > > > > > + set_reg(SCIF_REG_FTDR, ch);
> > > > > > + reg = get_reg(SCIF_REG_FSR);
> > > > > > + reg &= ~SCIF_FSR_TXD_CHK;
> > > > > > + set_reg(SCIF_REG_FSR, reg);
> > > > >
> > > > > The FSR Tx status bits clear should happen before writing a new char
> > > > > to the Tx fifo, no?
> > > > >
> > > > No, this has to be cleared only after char write.
> > >
> > > Based on my read of the user manual, after the while () check, we can
> > > clear the status bits. After you write a new char to the FIFO, the
> > > status bits might be triggered by the new char write, instead of the
> > > previous one. Hence clearing status bits after writing char will cause
> > > the status bits check next time fail.
> > >
> > From the HW manual..
> > [Clearing conditions]
> > ? TDFE is cleared to 0 when 0 is written in the TDFE bit after reading TDFE = 1.
> > ? When transmit data is written to the FTDR register
> > ^^ hence its cleared after the write here.
>
> These 2 conditions are OR, not AND, right?
>
> So when the while loop breaks, it satisfies the first condition, and
> we can clear the TDFE bit by writing zero to FSR.
>
> But if you write the tx data to the FTDR register, after you do
>
> set_reg(SCIF_REG_FTDR, ch);
>
> There is a possibility that the TDFE bit is generated before before you do
>
> reg = get_reg(SCIF_REG_FSR);
> reg &= ~SCIF_FSR_TXD_CHK;
> set_reg(SCIF_REG_FSR, reg);
>
> then the TDFE bit gets cleared unexpectedly.
>
> Next time when you want to write another char to the serial port the
> while loop will never break.
>
I altered the code sequence as per your suggestion and haven't seen
any issue with some limited testing.
Here are references [0]/[1] for why the sequence should be as in the
current patch. Looking at [1] I'm not seeing the issue mentioned there
if I re-order the code sequence.
(I am awaiting a response from the HW team yet.)
As the current code sequence has been there in Linux/u-boot ([2]/[3]
and our flashwriter code too) for many years and haven't seen an issue
of infinite looping I would like to stick with the current code
sequence. And if in future if we see any issue we can provide a patch
to fix this.
I hope this is OK with you.
[0] https://pasteboard.co/fXHV51yQD8zU.png
[1] https://patchwork.kernel.org/project/linux-sh/patch/4.2.0.58.J.20090601123439.00b52740 at router.itonet.co.jp/#63581
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/sh-sci.c?h=v6.1-rc7#n698
[3] https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/serial/serial_sh.c#L83
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/7] lib: utils: serial: Add FDT driver for Renesas SCIF
2022-11-11 18:20 [PATCH v2 0/7] Add support for Renesas RZ/Five SoC Prabhakar
2022-11-11 18:20 ` [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver Prabhakar
@ 2022-11-11 18:21 ` Prabhakar
2022-11-11 18:21 ` [PATCH v2 3/7] platform: andes/ae350: Split platform.h header file Prabhakar
` (4 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Prabhakar @ 2022-11-11 18:21 UTC (permalink / raw)
To: opensbi
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add FDT driver for Renesas SCIF.
dts example:
soc: soc {
....
scif0: serial at 1004b800 {
compatible = "renesas,scif-r9a07g043",
"renesas,scif-r9a07g044";
reg = <0 0x1004b800 0 0x400>;
interrupts = <412 IRQ_TYPE_LEVEL_HIGH>,
<414 IRQ_TYPE_LEVEL_HIGH>,
<415 IRQ_TYPE_LEVEL_HIGH>,
<413 IRQ_TYPE_LEVEL_HIGH>,
<416 IRQ_TYPE_LEVEL_HIGH>,
<416 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "eri", "rxi", "txi",
"bri", "dri", "tei";
clocks = <&cpg CPG_MOD R9A07G043_SCIF0_CLK_PCK>;
clock-names = "fck";
power-domains = <&cpg>;
resets = <&cpg R9A07G043_SCIF0_RST_SYSTEM_N>;
status = "disabled";
};
....
};
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
* Dropped DEFAULT_RENESAS_SCIF_REG_SHIFT and
DEFAULT_RENESAS_SCIF_REG_IO_WIDTH macros.
DT binding [0].
[0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/serial/renesas,scif.yaml?h=next-20221111#n80
---
include/sbi_utils/fdt/fdt_helper.h | 3 +++
lib/utils/fdt/fdt_helper.c | 11 ++++++++
lib/utils/serial/Kconfig | 5 ++++
lib/utils/serial/fdt_serial_renesas_scif.c | 31 ++++++++++++++++++++++
lib/utils/serial/objects.mk | 3 +++
5 files changed, 53 insertions(+)
create mode 100644 lib/utils/serial/fdt_serial_renesas_scif.c
diff --git a/include/sbi_utils/fdt/fdt_helper.h b/include/sbi_utils/fdt/fdt_helper.h
index c39f77a..09f3095 100644
--- a/include/sbi_utils/fdt/fdt_helper.h
+++ b/include/sbi_utils/fdt/fdt_helper.h
@@ -59,6 +59,9 @@ int fdt_parse_timebase_frequency(void *fdt, unsigned long *freq);
int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
struct platform_uart_data *uart);
+int fdt_parse_renesas_scif_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/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
index d390436..8f2d619 100644
--- a/lib/utils/fdt/fdt_helper.c
+++ b/lib/utils/fdt/fdt_helper.c
@@ -23,6 +23,9 @@
#define DEFAULT_UART_REG_IO_WIDTH 1
#define DEFAULT_UART_REG_OFFSET 0
+#define DEFAULT_RENESAS_SCIF_FREQ 100000000
+#define DEFAULT_RENESAS_SCIF_BAUD 115200
+
#define DEFAULT_SIFIVE_UART_FREQ 0
#define DEFAULT_SIFIVE_UART_BAUD 115200
#define DEFAULT_SIFIVE_UART_REG_SHIFT 0
@@ -370,6 +373,14 @@ int fdt_parse_gaisler_uart_node(void *fdt, int nodeoffset,
return 0;
}
+int fdt_parse_renesas_scif_node(void *fdt, int nodeoffset,
+ struct platform_uart_data *uart)
+{
+ return fdt_parse_uart_node_common(fdt, nodeoffset, uart,
+ DEFAULT_RENESAS_SCIF_FREQ,
+ DEFAULT_RENESAS_SCIF_BAUD);
+}
+
int fdt_parse_shakti_uart_node(void *fdt, int nodeoffset,
struct platform_uart_data *uart)
{
diff --git a/lib/utils/serial/Kconfig b/lib/utils/serial/Kconfig
index f6ed803..e3589ca 100644
--- a/lib/utils/serial/Kconfig
+++ b/lib/utils/serial/Kconfig
@@ -24,6 +24,11 @@ config FDT_SERIAL_HTIF
select SYS_HTIF
default n
+config FDT_SERIAL_RENESAS_SCIF
+ bool "Renesas SCIF FDT driver"
+ select SERIAL_RENESAS_SCIF
+ default n
+
config FDT_SERIAL_SHAKTI
bool "Shakti UART FDT driver"
select SERIAL_SHAKTI
diff --git a/lib/utils/serial/fdt_serial_renesas_scif.c b/lib/utils/serial/fdt_serial_renesas_scif.c
new file mode 100644
index 0000000..c331ca1
--- /dev/null
+++ b/lib/utils/serial/fdt_serial_renesas_scif.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+
+#include <sbi_utils/fdt/fdt_helper.h>
+#include <sbi_utils/serial/fdt_serial.h>
+#include <sbi_utils/serial/renesas-scif.h>
+
+static int serial_renesas_scif_init(void *fdt, int nodeoff,
+ const struct fdt_match *match)
+{
+ struct platform_uart_data uart = { 0 };
+ int ret;
+
+ ret = fdt_parse_renesas_scif_node(fdt, nodeoff, &uart);
+ if (ret)
+ return ret;
+
+ return renesas_scif_init(uart.addr, uart.freq, uart.baud);
+}
+
+static const struct fdt_match serial_renesas_scif_match[] = {
+ { .compatible = "renesas,scif-r9a07g043" },
+ { /* sentinel */ }
+};
+
+struct fdt_serial fdt_serial_renesas_scif = {
+ .match_table = serial_renesas_scif_match,
+ .init = serial_renesas_scif_init
+};
diff --git a/lib/utils/serial/objects.mk b/lib/utils/serial/objects.mk
index 6520424..1e6bd2e 100644
--- a/lib/utils/serial/objects.mk
+++ b/lib/utils/serial/objects.mk
@@ -19,6 +19,9 @@ libsbiutils-objs-$(CONFIG_FDT_SERIAL_GAISLER) += serial/fdt_serial_gaisler.o
carray-fdt_serial_drivers-$(CONFIG_FDT_SERIAL_HTIF) += fdt_serial_htif
libsbiutils-objs-$(CONFIG_FDT_SERIAL_HTIF) += serial/fdt_serial_htif.o
+carray-fdt_serial_drivers-$(CONFIG_FDT_SERIAL_RENESAS_SCIF) += fdt_serial_renesas_scif
+libsbiutils-objs-$(CONFIG_FDT_SERIAL_RENESAS_SCIF) += serial/fdt_serial_renesas_scif.o
+
carray-fdt_serial_drivers-$(CONFIG_FDT_SERIAL_SHAKTI) += fdt_serial_shakti
libsbiutils-objs-$(CONFIG_FDT_SERIAL_SHAKTI) += serial/fdt_serial_shakti.o
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 3/7] platform: andes/ae350: Split platform.h header file
2022-11-11 18:20 [PATCH v2 0/7] Add support for Renesas RZ/Five SoC Prabhakar
2022-11-11 18:20 ` [PATCH v2 1/7] lib: utils: serial: Add Renesas SCIF driver Prabhakar
2022-11-11 18:21 ` [PATCH v2 2/7] lib: utils: serial: Add FDT driver for Renesas SCIF Prabhakar
@ 2022-11-11 18:21 ` Prabhakar
2022-11-11 18:21 ` [PATCH v2 4/7] lib: utils/irqchip: Add compatible string for Andestech NCEPLIC100 Prabhakar
` (3 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Prabhakar @ 2022-11-11 18:21 UTC (permalink / raw)
To: opensbi
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Split up the platform.h header file into
- platform.h #Contains platform specific macros
- common-platform.h #contains macros common across Andes platforms
This is in preparation for adding the Renesas RZ/Five SoC which is based
on Andes AX45MP core.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Yu Chien Peter Lin <peterlin@andestech.com>
---
RFC->v2
* Updated patch subject
* Included RB tag from Yu Chien Peter Lin
---
platform/andes/ae350/common-platform.h | 94 ++++++++++++++++++++++++++
platform/andes/ae350/platform.h | 82 +---------------------
2 files changed, 96 insertions(+), 80 deletions(-)
create mode 100644 platform/andes/ae350/common-platform.h
diff --git a/platform/andes/ae350/common-platform.h b/platform/andes/ae350/common-platform.h
new file mode 100644
index 0000000..6334a2e
--- /dev/null
+++ b/platform/andes/ae350/common-platform.h
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (c) 2019 Andes Technology Corporation
+ *
+ * Authors:
+ * Zong Li <zong@andestech.com>
+ * Nylon Chen <nylon7@andestech.com>
+ */
+
+#ifndef _AE350_COMMON_PLATFORM_H_
+#define _AE350_COMMON_PLATFORM_H_
+
+/* Memory and Miscellaneous Registers */
+#define CSR_MILMB 0x7c0
+#define CSR_MDLMB 0x7c1
+#define CSR_MECC_CDOE 0x7c2
+#define CSR_MNVEC 0x7c3
+#define CSR_MPFTCTL 0x7c5
+#define CSR_MCACHECTL 0x7ca
+#define CSR_MCCTLBEGINADDR 0x7cb
+#define CSR_MCCTLCOMMAND 0x7cc
+#define CSR_MCCTLDATA 0x7cc
+#define CSR_SCCTLDATA 0x9cd
+#define CSR_UCCTLBEGINADDR 0x80c
+#define CSR_MMISCCTL 0x7d0
+
+/* nds v5 mmisc_ctl register*/
+#define V5_MMISC_CTL_VEC_PLIC_OFFSET 1
+#define V5_MMISC_CTL_RVCOMPM_OFFSET 2
+#define V5_MMISC_CTL_BRPE_OFFSET 3
+#define V5_MMISC_CTL_MSA_OR_UNA_OFFSET 6
+#define V5_MMISC_CTL_NON_BLOCKING_OFFSET 8
+#define V5_MCACHE_CTL_L1I_PREFETCH_OFFSET 9
+#define V5_MCACHE_CTL_L1D_PREFETCH_OFFSET 10
+#define V5_MCACHE_CTL_DC_WAROUND_OFFSET_1 13
+#define V5_MCACHE_CTL_DC_WAROUND_OFFSET_2 14
+
+#define V5_MMISC_CTL_VEC_PLIC_EN (1UL << V5_MMISC_CTL_VEC_PLIC_OFFSET)
+#define V5_MMISC_CTL_RVCOMPM_EN (1UL << V5_MMISC_CTL_RVCOMPM_OFFSET)
+#define V5_MMISC_CTL_BRPE_EN (1UL << V5_MMISC_CTL_BRPE_OFFSET)
+#define V5_MMISC_CTL_MSA_OR_UNA_EN (1UL << V5_MMISC_CTL_MSA_OR_UNA_OFFSET)
+#define V5_MMISC_CTL_NON_BLOCKING_EN (1UL << V5_MMISC_CTL_NON_BLOCKING_OFFSET)
+#define V5_MCACHE_CTL_L1I_PREFETCH_EN (1UL << V5_MCACHE_CTL_L1I_PREFETCH_OFFSET)
+#define V5_MCACHE_CTL_L1D_PREFETCH_EN (1UL << V5_MCACHE_CTL_L1D_PREFETCH_OFFSET)
+#define V5_MCACHE_CTL_DC_WAROUND_1_EN (1UL << V5_MCACHE_CTL_DC_WAROUND_OFFSET_1)
+#define V5_MCACHE_CTL_DC_WAROUND_2_EN (1UL << V5_MCACHE_CTL_DC_WAROUND_OFFSET_2)
+
+#define V5_MMISC_CTL_MASK (V5_MMISC_CTL_VEC_PLIC_EN | V5_MMISC_CTL_RVCOMPM_EN | \
+ V5_MMISC_CTL_BRPE_EN | V5_MMISC_CTL_MSA_OR_UNA_EN | \
+ V5_MMISC_CTL_NON_BLOCKING_EN)
+
+/* nds mcache_ctl register */
+#define V5_MCACHE_CTL_IC_EN_OFFSET 0
+#define V5_MCACHE_CTL_DC_EN_OFFSET 1
+#define V5_MCACHE_CTL_IC_ECCEN_OFFSET 2
+#define V5_MCACHE_CTL_DC_ECCEN_OFFSET 4
+#define V5_MCACHE_CTL_IC_RWECC_OFFSET 6
+#define V5_MCACHE_CTL_DC_RWECC_OFFSET 7
+#define V5_MCACHE_CTL_CCTL_SUEN_OFFSET 8
+
+/* nds cctl command */
+#define V5_UCCTL_L1D_WBINVAL_ALL 6
+#define V5_UCCTL_L1D_WB_ALL 7
+
+#define V5_MCACHE_CTL_IC_EN (1UL << V5_MCACHE_CTL_IC_EN_OFFSET)
+#define V5_MCACHE_CTL_DC_EN (1UL << V5_MCACHE_CTL_DC_EN_OFFSET)
+#define V5_MCACHE_CTL_IC_RWECC (1UL << V5_MCACHE_CTL_IC_RWECC_OFFSET)
+#define V5_MCACHE_CTL_DC_RWECC (1UL << V5_MCACHE_CTL_DC_RWECC_OFFSET)
+#define V5_MCACHE_CTL_CCTL_SUEN (1UL << V5_MCACHE_CTL_CCTL_SUEN_OFFSET)
+
+#define V5_MCACHE_CTL_MASK (V5_MCACHE_CTL_IC_EN | V5_MCACHE_CTL_DC_EN | \
+ V5_MCACHE_CTL_IC_RWECC | V5_MCACHE_CTL_DC_RWECC | \
+ V5_MCACHE_CTL_CCTL_SUEN | V5_MCACHE_CTL_L1I_PREFETCH_EN | \
+ V5_MCACHE_CTL_L1D_PREFETCH_EN | V5_MCACHE_CTL_DC_WAROUND_1_EN | \
+ V5_MCACHE_CTL_DC_WAROUND_2_EN)
+
+#define V5_L2C_CTL_OFFSET 0x8
+#define V5_L2C_CTL_ENABLE_OFFSET 0
+#define V5_L2C_CTL_IPFDPT_OFFSET 3
+#define V5_L2C_CTL_DPFDPT_OFFSET 5
+#define V5_L2C_CTL_TRAMOCTL_OFFSET 8
+#define V5_L2C_CTL_TRAMICTL_OFFSET 10
+#define V5_L2C_CTL_DRAMOCTL_OFFSET 11
+#define V5_L2C_CTL_DRAMICTL_OFFSET 13
+
+#define V5_L2C_CTL_ENABLE_MASK (1UL << V5_L2C_CTL_ENABLE_OFFSET)
+#define V5_L2C_CTL_IPFDPT_MASK (3UL << V5_L2C_CTL_IPFDPT_OFFSET)
+#define V5_L2C_CTL_DPFDPT_MASK (3UL << V5_L2C_CTL_DPFDPT_OFFSET)
+#define V5_L2C_CTL_TRAMOCTL_MASK (3UL << V5_L2C_CTL_TRAMOCTL_OFFSET)
+#define V5_L2C_CTL_TRAMICTL_MASK (1UL << V5_L2C_CTL_TRAMICTL_OFFSET)
+#define V5_L2C_CTL_DRAMOCTL_MASK (3UL << V5_L2C_CTL_DRAMOCTL_OFFSET)
+#define V5_L2C_CTL_DRAMICTL_MASK (1UL << V5_L2C_CTL_DRAMICTL_OFFSET)
+
+#endif /* _AE350_COMMON_PLATFORM_H_ */
diff --git a/platform/andes/ae350/platform.h b/platform/andes/ae350/platform.h
index 903bef0..45de792 100644
--- a/platform/andes/ae350/platform.h
+++ b/platform/andes/ae350/platform.h
@@ -11,21 +11,9 @@
#ifndef _AE350_PLATFORM_H_
#define _AE350_PLATFORM_H_
-#define AE350_L2C_ADDR 0xe0500000
+#include "common-platform.h"
-/*Memory and Miscellaneous Registers*/
-#define CSR_MILMB 0x7c0
-#define CSR_MDLMB 0x7c1
-#define CSR_MECC_CDOE 0x7c2
-#define CSR_MNVEC 0x7c3
-#define CSR_MPFTCTL 0x7c5
-#define CSR_MCACHECTL 0x7ca
-#define CSR_MCCTLBEGINADDR 0x7cb
-#define CSR_MCCTLCOMMAND 0x7cc
-#define CSR_MCCTLDATA 0x7cc
-#define CSR_SCCTLDATA 0x9cd
-#define CSR_UCCTLBEGINADDR 0x80c
-#define CSR_MMISCCTL 0x7d0
+#define AE350_L2C_ADDR 0xe0500000
enum sbi_ext_andes_fid {
SBI_EXT_ANDES_GET_MCACHE_CTL_STATUS = 0,
@@ -40,70 +28,4 @@ enum sbi_ext_andes_fid {
SBI_EXT_ANDES_WRITE_AROUND,
};
-/* nds v5 mmisc_ctl register*/
-#define V5_MMISC_CTL_VEC_PLIC_OFFSET 1
-#define V5_MMISC_CTL_RVCOMPM_OFFSET 2
-#define V5_MMISC_CTL_BRPE_OFFSET 3
-#define V5_MMISC_CTL_MSA_OR_UNA_OFFSET 6
-#define V5_MMISC_CTL_NON_BLOCKING_OFFSET 8
-#define V5_MCACHE_CTL_L1I_PREFETCH_OFFSET 9
-#define V5_MCACHE_CTL_L1D_PREFETCH_OFFSET 10
-#define V5_MCACHE_CTL_DC_WAROUND_OFFSET_1 13
-#define V5_MCACHE_CTL_DC_WAROUND_OFFSET_2 14
-
-#define V5_MMISC_CTL_VEC_PLIC_EN (1UL << V5_MMISC_CTL_VEC_PLIC_OFFSET)
-#define V5_MMISC_CTL_RVCOMPM_EN (1UL << V5_MMISC_CTL_RVCOMPM_OFFSET)
-#define V5_MMISC_CTL_BRPE_EN (1UL << V5_MMISC_CTL_BRPE_OFFSET)
-#define V5_MMISC_CTL_MSA_OR_UNA_EN (1UL << V5_MMISC_CTL_MSA_OR_UNA_OFFSET)
-#define V5_MMISC_CTL_NON_BLOCKING_EN (1UL << V5_MMISC_CTL_NON_BLOCKING_OFFSET)
-#define V5_MCACHE_CTL_L1I_PREFETCH_EN (1UL << V5_MCACHE_CTL_L1I_PREFETCH_OFFSET)
-#define V5_MCACHE_CTL_L1D_PREFETCH_EN (1UL << V5_MCACHE_CTL_L1D_PREFETCH_OFFSET)
-#define V5_MCACHE_CTL_DC_WAROUND_1_EN (1UL << V5_MCACHE_CTL_DC_WAROUND_OFFSET_1)
-#define V5_MCACHE_CTL_DC_WAROUND_2_EN (1UL << V5_MCACHE_CTL_DC_WAROUND_OFFSET_2)
-
-#define V5_MMISC_CTL_MASK (V5_MMISC_CTL_VEC_PLIC_EN | V5_MMISC_CTL_RVCOMPM_EN \
- | V5_MMISC_CTL_BRPE_EN | V5_MMISC_CTL_MSA_OR_UNA_EN | V5_MMISC_CTL_NON_BLOCKING_EN)
-
-/* nds mcache_ctl register*/
-#define V5_MCACHE_CTL_IC_EN_OFFSET 0
-#define V5_MCACHE_CTL_DC_EN_OFFSET 1
-#define V5_MCACHE_CTL_IC_ECCEN_OFFSET 2
-#define V5_MCACHE_CTL_DC_ECCEN_OFFSET 4
-#define V5_MCACHE_CTL_IC_RWECC_OFFSET 6
-#define V5_MCACHE_CTL_DC_RWECC_OFFSET 7
-#define V5_MCACHE_CTL_CCTL_SUEN_OFFSET 8
-
-/*nds cctl command*/
-#define V5_UCCTL_L1D_WBINVAL_ALL 6
-#define V5_UCCTL_L1D_WB_ALL 7
-
-#define V5_MCACHE_CTL_IC_EN (1UL << V5_MCACHE_CTL_IC_EN_OFFSET)
-#define V5_MCACHE_CTL_DC_EN (1UL << V5_MCACHE_CTL_DC_EN_OFFSET)
-#define V5_MCACHE_CTL_IC_RWECC (1UL << V5_MCACHE_CTL_IC_RWECC_OFFSET)
-#define V5_MCACHE_CTL_DC_RWECC (1UL << V5_MCACHE_CTL_DC_RWECC_OFFSET)
-#define V5_MCACHE_CTL_CCTL_SUEN (1UL << V5_MCACHE_CTL_CCTL_SUEN_OFFSET)
-
-#define V5_MCACHE_CTL_MASK (V5_MCACHE_CTL_IC_EN | V5_MCACHE_CTL_DC_EN \
- | V5_MCACHE_CTL_IC_RWECC | V5_MCACHE_CTL_DC_RWECC \
- | V5_MCACHE_CTL_CCTL_SUEN | V5_MCACHE_CTL_L1I_PREFETCH_EN \
- | V5_MCACHE_CTL_L1D_PREFETCH_EN | V5_MCACHE_CTL_DC_WAROUND_1_EN \
- | V5_MCACHE_CTL_DC_WAROUND_2_EN)
-
-#define V5_L2C_CTL_OFFSET 0x8
-#define V5_L2C_CTL_ENABLE_OFFSET 0
-#define V5_L2C_CTL_IPFDPT_OFFSET 3
-#define V5_L2C_CTL_DPFDPT_OFFSET 5
-#define V5_L2C_CTL_TRAMOCTL_OFFSET 8
-#define V5_L2C_CTL_TRAMICTL_OFFSET 10
-#define V5_L2C_CTL_DRAMOCTL_OFFSET 11
-#define V5_L2C_CTL_DRAMICTL_OFFSET 13
-
-#define V5_L2C_CTL_ENABLE_MASK (1UL << V5_L2C_CTL_ENABLE_OFFSET)
-#define V5_L2C_CTL_IPFDPT_MASK (3UL << V5_L2C_CTL_IPFDPT_OFFSET)
-#define V5_L2C_CTL_DPFDPT_MASK (3UL << V5_L2C_CTL_DPFDPT_OFFSET)
-#define V5_L2C_CTL_TRAMOCTL_MASK (3UL << V5_L2C_CTL_TRAMOCTL_OFFSET)
-#define V5_L2C_CTL_TRAMICTL_MASK (1UL << V5_L2C_CTL_TRAMICTL_OFFSET)
-#define V5_L2C_CTL_DRAMOCTL_MASK (3UL << V5_L2C_CTL_DRAMOCTL_OFFSET)
-#define V5_L2C_CTL_DRAMICTL_MASK (1UL << V5_L2C_CTL_DRAMICTL_OFFSET)
-
#endif /* _AE350_PLATFORM_H_ */
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 4/7] lib: utils/irqchip: Add compatible string for Andestech NCEPLIC100
2022-11-11 18:20 [PATCH v2 0/7] Add support for Renesas RZ/Five SoC Prabhakar
` (2 preceding siblings ...)
2022-11-11 18:21 ` [PATCH v2 3/7] platform: andes/ae350: Split platform.h header file Prabhakar
@ 2022-11-11 18:21 ` Prabhakar
2022-11-15 11:30 ` Anup Patel
2022-11-15 14:06 ` Bin Meng
2022-11-11 18:21 ` [PATCH v2 5/7] platform: Add Renesas RZ/Five initial support Prabhakar
` (2 subsequent siblings)
6 siblings, 2 replies; 20+ messages in thread
From: Prabhakar @ 2022-11-11 18:21 UTC (permalink / raw)
To: opensbi
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Add compatible string for Andestech NCEPLIC100 found on Renesas RZ/Five SoC
which is equipped with AX45MP AndesCore.
While at it drop the comma after the sentinel as it does not make sense to
have a comma after a sentinel, as any new elements must be added before the
sentinel.
dts example (Single-core AX45MP):
soc: soc {
....
plic: interrupt-controller at 12c00000 {
compatible = "renesas,r9a07g043-plic", "andestech,nceplic100";
#interrupt-cells = <2>;
#address-cells = <0>;
riscv,ndev = <511>;
interrupt-controller;
reg = <0x0 0x12c00000 0 0x400000>;
clocks = <&cpg CPG_MOD R9A07G043_NCEPLIC_ACLK>;
power-domains = <&cpg>;
resets = <&cpg R9A07G043_NCEPLIC_ARESETN>;
interrupts-extended = <&cpu0_intc 11 &cpu0_intc 9>;
};
....
};
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
* No change
DT binding [0]
[0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml?h=next-20221111#n56
---
lib/utils/irqchip/fdt_irqchip_plic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
index a6e185c..83d091a 100644
--- a/lib/utils/irqchip/fdt_irqchip_plic.c
+++ b/lib/utils/irqchip/fdt_irqchip_plic.c
@@ -163,7 +163,8 @@ static const struct fdt_match irqchip_plic_match[] = {
{ .compatible = "sifive,plic-1.0.0" },
{ .compatible = "thead,c900-plic",
.data = thead_plic_plat_init },
- { },
+ { .compatible = "andestech,nceplic100" },
+ { /* sentinel */ }
};
struct fdt_irqchip fdt_irqchip_plic = {
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 4/7] lib: utils/irqchip: Add compatible string for Andestech NCEPLIC100
2022-11-11 18:21 ` [PATCH v2 4/7] lib: utils/irqchip: Add compatible string for Andestech NCEPLIC100 Prabhakar
@ 2022-11-15 11:30 ` Anup Patel
2022-11-15 14:06 ` Bin Meng
1 sibling, 0 replies; 20+ messages in thread
From: Anup Patel @ 2022-11-15 11:30 UTC (permalink / raw)
To: opensbi
On Fri, Nov 11, 2022 at 11:51 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add compatible string for Andestech NCEPLIC100 found on Renesas RZ/Five SoC
> which is equipped with AX45MP AndesCore.
>
> While at it drop the comma after the sentinel as it does not make sense to
> have a comma after a sentinel, as any new elements must be added before the
> sentinel.
>
> dts example (Single-core AX45MP):
>
> soc: soc {
> ....
> plic: interrupt-controller at 12c00000 {
> compatible = "renesas,r9a07g043-plic", "andestech,nceplic100";
> #interrupt-cells = <2>;
> #address-cells = <0>;
> riscv,ndev = <511>;
> interrupt-controller;
> reg = <0x0 0x12c00000 0 0x400000>;
> clocks = <&cpg CPG_MOD R9A07G043_NCEPLIC_ACLK>;
> power-domains = <&cpg>;
> resets = <&cpg R9A07G043_NCEPLIC_ARESETN>;
> interrupts-extended = <&cpu0_intc 11 &cpu0_intc 9>;
> };
> ....
> };
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> RFC->v2
> * No change
>
> DT binding [0]
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml?h=next-20221111#n56
> ---
> lib/utils/irqchip/fdt_irqchip_plic.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/utils/irqchip/fdt_irqchip_plic.c b/lib/utils/irqchip/fdt_irqchip_plic.c
> index a6e185c..83d091a 100644
> --- a/lib/utils/irqchip/fdt_irqchip_plic.c
> +++ b/lib/utils/irqchip/fdt_irqchip_plic.c
> @@ -163,7 +163,8 @@ static const struct fdt_match irqchip_plic_match[] = {
> { .compatible = "sifive,plic-1.0.0" },
> { .compatible = "thead,c900-plic",
> .data = thead_plic_plat_init },
> - { },
> + { .compatible = "andestech,nceplic100" },
> + { /* sentinel */ }
> };
>
> struct fdt_irqchip fdt_irqchip_plic = {
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 4/7] lib: utils/irqchip: Add compatible string for Andestech NCEPLIC100
2022-11-11 18:21 ` [PATCH v2 4/7] lib: utils/irqchip: Add compatible string for Andestech NCEPLIC100 Prabhakar
2022-11-15 11:30 ` Anup Patel
@ 2022-11-15 14:06 ` Bin Meng
1 sibling, 0 replies; 20+ messages in thread
From: Bin Meng @ 2022-11-15 14:06 UTC (permalink / raw)
To: opensbi
On Sat, Nov 12, 2022 at 2:22 AM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add compatible string for Andestech NCEPLIC100 found on Renesas RZ/Five SoC
> which is equipped with AX45MP AndesCore.
>
> While at it drop the comma after the sentinel as it does not make sense to
> have a comma after a sentinel, as any new elements must be added before the
> sentinel.
>
> dts example (Single-core AX45MP):
>
> soc: soc {
> ....
> plic: interrupt-controller at 12c00000 {
> compatible = "renesas,r9a07g043-plic", "andestech,nceplic100";
> #interrupt-cells = <2>;
> #address-cells = <0>;
> riscv,ndev = <511>;
> interrupt-controller;
> reg = <0x0 0x12c00000 0 0x400000>;
> clocks = <&cpg CPG_MOD R9A07G043_NCEPLIC_ACLK>;
> power-domains = <&cpg>;
> resets = <&cpg R9A07G043_NCEPLIC_ARESETN>;
> interrupts-extended = <&cpu0_intc 11 &cpu0_intc 9>;
> };
> ....
> };
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> * No change
>
> DT binding [0]
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml?h=next-20221111#n56
> ---
> lib/utils/irqchip/fdt_irqchip_plic.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Reviewed-by: Bin Meng <bmeng@tinylab.org>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 5/7] platform: Add Renesas RZ/Five initial support
2022-11-11 18:20 [PATCH v2 0/7] Add support for Renesas RZ/Five SoC Prabhakar
` (3 preceding siblings ...)
2022-11-11 18:21 ` [PATCH v2 4/7] lib: utils/irqchip: Add compatible string for Andestech NCEPLIC100 Prabhakar
@ 2022-11-11 18:21 ` Prabhakar
2022-11-15 11:44 ` Anup Patel
2022-11-28 9:23 ` Yu-Chien Peter Lin
2022-11-11 18:21 ` [PATCH v2 6/7] scripts: Add Renesas RZ/Five to platform list in the binary archive script Prabhakar
2022-11-11 18:21 ` [PATCH v2 7/7] docs: platform: Add documentation for Renesas RZ/Five SoC Prabhakar
6 siblings, 2 replies; 20+ messages in thread
From: Prabhakar @ 2022-11-11 18:21 UTC (permalink / raw)
To: opensbi
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
This commit provides basic support for the Renesas RZ/Five
(R9A07G043F) SoC.
The RZ/Five microprocessor includes a single RISC-V CPU Core (Andes AX45MP)
1.0 GHz, 16-bit DDR3L/DDR4 interface. Supported interfaces include:
- Gigabit Ethernet 2ch
- CAN interface (CAN-FD) 2ch
- USB 2.0 interface 2ch
- SD interface 2ch
- AD converter 2ch
Useful links:
-------------
[0] https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-mpus/rzfive-risc-v-general-purpose-microprocessors-risc-v-cpu-core-andes-ax45mp-single-10-ghz-2ch-gigabit-ethernet
[1] http://www.andestech.com/en/products-solutions/andescore-processors/riscv-ax45mp/
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
* Updated commit description
---
platform/renesas/rzfive/Kconfig | 32 +++++
platform/renesas/rzfive/configs/defconfig | 0
platform/renesas/rzfive/objects.mk | 18 +++
platform/renesas/rzfive/platform.c | 161 ++++++++++++++++++++++
platform/renesas/rzfive/platform.h | 24 ++++
5 files changed, 235 insertions(+)
create mode 100644 platform/renesas/rzfive/Kconfig
create mode 100644 platform/renesas/rzfive/configs/defconfig
create mode 100644 platform/renesas/rzfive/objects.mk
create mode 100644 platform/renesas/rzfive/platform.c
create mode 100644 platform/renesas/rzfive/platform.h
diff --git a/platform/renesas/rzfive/Kconfig b/platform/renesas/rzfive/Kconfig
new file mode 100644
index 0000000..c5e8a19
--- /dev/null
+++ b/platform/renesas/rzfive/Kconfig
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: BSD-2-Clause
+
+config PLATFORM_RENESAS_RZFIVE
+ bool
+ select FDT
+ select FDT_SERIAL
+ select FDT_SERIAL_RENESAS_SCIF
+ select FDT_TIMER
+ select FDT_TIMER_PLMT
+ select FDT_IRQCHIP
+ select FDT_IRQCHIP_PLIC
+ select FDT_IPI
+ select FDT_IPI_PLICSW
+ default y
+
+if PLATFORM_RENESAS_RZFIVE
+
+config PLATFORM_RENESAS_RZFIVE_NAME
+ string "Platform default name"
+ default "Renesas RZ/Five"
+
+config PLATFORM_RENESAS_RZFIVE_MAJOR_VER
+ int "Platform major version"
+ range 0 65535
+ default 0
+
+config PLATFORM_RENESAS_RZFIVE_MINOR_VER
+ int "Platform minor version"
+ range 0 65535
+ default 1
+
+endif
diff --git a/platform/renesas/rzfive/configs/defconfig b/platform/renesas/rzfive/configs/defconfig
new file mode 100644
index 0000000..e69de29
diff --git a/platform/renesas/rzfive/objects.mk b/platform/renesas/rzfive/objects.mk
new file mode 100644
index 0000000..302b3d6
--- /dev/null
+++ b/platform/renesas/rzfive/objects.mk
@@ -0,0 +1,18 @@
+#
+# SPDX-License-Identifier: BSD-2-Clause
+#
+# Copyright (C) 2022 Renesas Electronics Corp.
+#
+
+# Compiler flags
+platform-cppflags-y =
+platform-cflags-y =
+platform-asflags-y =
+platform-ldflags-y =
+
+# Objects to build
+platform-objs-y += ../../andes/ae350/cache.o platform.o
+
+# Blobs to build
+FW_TEXT_START=0x00000000
+FW_DYNAMIC=y
diff --git a/platform/renesas/rzfive/platform.c b/platform/renesas/rzfive/platform.c
new file mode 100644
index 0000000..cfc557c
--- /dev/null
+++ b/platform/renesas/rzfive/platform.c
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ *
+ * Based on platform/andes/ae350/platform.c
+ * Copyright (c) 2019 Andes Technology Corporation
+ */
+
+#include <libfdt.h>
+#include <sbi/riscv_asm.h>
+#include <sbi/riscv_encoding.h>
+#include <sbi/sbi_console.h>
+#include <sbi/sbi_const.h>
+#include <sbi/sbi_hartmask.h>
+#include <sbi/sbi_ipi.h>
+#include <sbi/sbi_platform.h>
+#include <sbi/sbi_trap.h>
+#include <sbi_utils/fdt/fdt_domain.h>
+#include <sbi_utils/fdt/fdt_fixup.h>
+#include <sbi_utils/fdt/fdt_helper.h>
+#include <sbi_utils/ipi/fdt_ipi.h>
+#include <sbi_utils/irqchip/fdt_irqchip.h>
+#include <sbi_utils/serial/fdt_serial.h>
+#include <sbi_utils/timer/fdt_timer.h>
+
+#include "../../andes/ae350/cache.h"
+
+#include "platform.h"
+
+struct sbi_platform platform;
+unsigned long fw_platform_init(unsigned long arg0, unsigned long arg1,
+ unsigned long arg2, unsigned long arg3,
+ unsigned long arg4)
+{
+ const char *model;
+ void *fdt = (void *)arg1;
+ u32 hartid, hart_count = 0;
+ int rc, root_offset, cpus_offset, cpu_offset, len;
+
+ root_offset = fdt_path_offset(fdt, "/");
+ if (root_offset < 0)
+ goto fail;
+
+ model = fdt_getprop(fdt, root_offset, "model", &len);
+ if (model)
+ sbi_strncpy(platform.name, model, sizeof(platform.name) - 1);
+
+ cpus_offset = fdt_path_offset(fdt, "/cpus");
+ if (cpus_offset < 0)
+ goto fail;
+
+ fdt_for_each_subnode(cpu_offset, fdt, cpus_offset) {
+ rc = fdt_parse_hart_id(fdt, cpu_offset, &hartid);
+ if (rc)
+ continue;
+
+ if (SBI_HARTMASK_MAX_BITS <= hartid)
+ continue;
+
+ hart_count++;
+ }
+
+ platform.hart_count = hart_count;
+
+ /* Return original FDT pointer */
+ return arg1;
+
+fail:
+ while (1)
+ wfi();
+}
+
+/* Platform final initialization. */
+static int rzfive_final_init(bool cold_boot)
+{
+ void *fdt;
+
+ if (!cold_boot)
+ return 0;
+
+ fdt = fdt_get_address();
+ fdt_fixups(fdt);
+ fdt_domain_fixup(fdt);
+
+ return 0;
+}
+
+/* Vendor-Specific SBI handler */
+static int rzfive_vendor_ext_provider(long extid, long funcid,
+ const struct sbi_trap_regs *regs,
+ unsigned long *out_value,
+ struct sbi_trap_info *out_trap)
+{
+ int ret = 0;
+
+ switch (funcid) {
+ case SBI_EXT_RENESAS_RZFIVE_GET_MCACHE_CTL_STATUS:
+ *out_value = csr_read(CSR_MCACHECTL);
+ break;
+ case SBI_EXT_RENESAS_RZFIVE_GET_MMISC_CTL_STATUS:
+ *out_value = csr_read(CSR_MMISCCTL);
+ break;
+ case SBI_EXT_RENESAS_RZFIVE_SET_MCACHE_CTL:
+ ret = mcall_set_mcache_ctl(regs->a0);
+ break;
+ case SBI_EXT_RENESAS_RZFIVE_SET_MMISC_CTL:
+ ret = mcall_set_mmisc_ctl(regs->a0);
+ break;
+ case SBI_EXT_RENESAS_RZFIVE_ICACHE_OP:
+ ret = mcall_icache_op(regs->a0);
+ break;
+ case SBI_EXT_RENESAS_RZFIVE_DCACHE_OP:
+ ret = mcall_dcache_op(regs->a0);
+ break;
+ case SBI_EXT_RENESAS_RZFIVE_L1CACHE_I_PREFETCH:
+ ret = mcall_l1_cache_i_prefetch_op(regs->a0);
+ break;
+ case SBI_EXT_RENESAS_RZFIVE_L1CACHE_D_PREFETCH:
+ ret = mcall_l1_cache_d_prefetch_op(regs->a0);
+ break;
+ case SBI_EXT_RENESAS_RZFIVE_NON_BLOCKING_LOAD_STORE:
+ ret = mcall_non_blocking_load_store(regs->a0);
+ break;
+ case SBI_EXT_RENESAS_RZFIVE_WRITE_AROUND:
+ ret = mcall_write_around(regs->a0);
+ break;
+ default:
+ sbi_printf("Unsupported vendor sbi call : %ld\n", funcid);
+ asm volatile("ebreak");
+ }
+
+ return ret;
+}
+
+static int rzfive_domains_init(void)
+{
+ return fdt_domains_populate(fdt_get_address());
+}
+
+/* Platform descriptor. */
+static const struct sbi_platform_operations platform_ops = {
+ .final_init = rzfive_final_init,
+ .domains_init = rzfive_domains_init,
+ .console_init = fdt_serial_init,
+ .irqchip_init = fdt_irqchip_init,
+ .ipi_init = fdt_ipi_init,
+ .timer_init = fdt_timer_init,
+ .vendor_ext_provider = rzfive_vendor_ext_provider
+};
+
+struct sbi_platform platform = {
+ .opensbi_version = OPENSBI_VERSION,
+ .platform_version =
+ SBI_PLATFORM_VERSION(CONFIG_PLATFORM_RENESAS_RZFIVE_MAJOR_VER,
+ CONFIG_PLATFORM_RENESAS_RZFIVE_MINOR_VER),
+ .name = CONFIG_PLATFORM_RENESAS_RZFIVE_NAME,
+ .features = SBI_PLATFORM_DEFAULT_FEATURES,
+ .hart_count = SBI_HARTMASK_MAX_BITS,
+ .hart_stack_size = SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
+ .platform_ops_addr = (unsigned long)&platform_ops
+};
diff --git a/platform/renesas/rzfive/platform.h b/platform/renesas/rzfive/platform.h
new file mode 100644
index 0000000..e46ca91
--- /dev/null
+++ b/platform/renesas/rzfive/platform.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ */
+
+#ifndef _RZFIVE_PLATFORM_H_
+#define _RZFIVE_PLATFORM_H_
+
+#include "../../andes/ae350/common-platform.h"
+
+enum sbi_ext_andes_fid {
+ SBI_EXT_RENESAS_RZFIVE_GET_MCACHE_CTL_STATUS = 0,
+ SBI_EXT_RENESAS_RZFIVE_GET_MMISC_CTL_STATUS,
+ SBI_EXT_RENESAS_RZFIVE_SET_MCACHE_CTL,
+ SBI_EXT_RENESAS_RZFIVE_SET_MMISC_CTL,
+ SBI_EXT_RENESAS_RZFIVE_ICACHE_OP,
+ SBI_EXT_RENESAS_RZFIVE_DCACHE_OP,
+ SBI_EXT_RENESAS_RZFIVE_L1CACHE_I_PREFETCH,
+ SBI_EXT_RENESAS_RZFIVE_L1CACHE_D_PREFETCH,
+ SBI_EXT_RENESAS_RZFIVE_NON_BLOCKING_LOAD_STORE,
+ SBI_EXT_RENESAS_RZFIVE_WRITE_AROUND,
+};
+
+#endif /* _RZFIVE_PLATFORM_H_ */
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 5/7] platform: Add Renesas RZ/Five initial support
2022-11-11 18:21 ` [PATCH v2 5/7] platform: Add Renesas RZ/Five initial support Prabhakar
@ 2022-11-15 11:44 ` Anup Patel
2022-11-16 15:53 ` Lad, Prabhakar
2022-11-28 9:23 ` Yu-Chien Peter Lin
1 sibling, 1 reply; 20+ messages in thread
From: Anup Patel @ 2022-11-15 11:44 UTC (permalink / raw)
To: opensbi
On Fri, Nov 11, 2022 at 11:51 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> This commit provides basic support for the Renesas RZ/Five
> (R9A07G043F) SoC.
>
> The RZ/Five microprocessor includes a single RISC-V CPU Core (Andes AX45MP)
> 1.0 GHz, 16-bit DDR3L/DDR4 interface. Supported interfaces include:
> - Gigabit Ethernet 2ch
> - CAN interface (CAN-FD) 2ch
> - USB 2.0 interface 2ch
> - SD interface 2ch
> - AD converter 2ch
>
> Useful links:
> -------------
> [0] https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-mpus/rzfive-risc-v-general-purpose-microprocessors-risc-v-cpu-core-andes-ax45mp-single-10-ghz-2ch-gigabit-ethernet
> [1] http://www.andestech.com/en/products-solutions/andescore-processors/riscv-ax45mp/
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> RFC->v2
> * Updated commit description
> ---
> platform/renesas/rzfive/Kconfig | 32 +++++
> platform/renesas/rzfive/configs/defconfig | 0
> platform/renesas/rzfive/objects.mk | 18 +++
> platform/renesas/rzfive/platform.c | 161 ++++++++++++++++++++++
> platform/renesas/rzfive/platform.h | 24 ++++
> 5 files changed, 235 insertions(+)
> create mode 100644 platform/renesas/rzfive/Kconfig
> create mode 100644 platform/renesas/rzfive/configs/defconfig
> create mode 100644 platform/renesas/rzfive/objects.mk
> create mode 100644 platform/renesas/rzfive/platform.c
> create mode 100644 platform/renesas/rzfive/platform.h
>
> diff --git a/platform/renesas/rzfive/Kconfig b/platform/renesas/rzfive/Kconfig
> new file mode 100644
> index 0000000..c5e8a19
> --- /dev/null
> +++ b/platform/renesas/rzfive/Kconfig
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: BSD-2-Clause
> +
> +config PLATFORM_RENESAS_RZFIVE
> + bool
> + select FDT
> + select FDT_SERIAL
> + select FDT_SERIAL_RENESAS_SCIF
> + select FDT_TIMER
> + select FDT_TIMER_PLMT
> + select FDT_IRQCHIP
> + select FDT_IRQCHIP_PLIC
> + select FDT_IPI
> + select FDT_IPI_PLICSW
> + default y
> +
> +if PLATFORM_RENESAS_RZFIVE
> +
> +config PLATFORM_RENESAS_RZFIVE_NAME
> + string "Platform default name"
> + default "Renesas RZ/Five"
> +
> +config PLATFORM_RENESAS_RZFIVE_MAJOR_VER
> + int "Platform major version"
> + range 0 65535
> + default 0
> +
> +config PLATFORM_RENESAS_RZFIVE_MINOR_VER
> + int "Platform minor version"
> + range 0 65535
> + default 1
> +
> +endif
> diff --git a/platform/renesas/rzfive/configs/defconfig b/platform/renesas/rzfive/configs/defconfig
> new file mode 100644
> index 0000000..e69de29
> diff --git a/platform/renesas/rzfive/objects.mk b/platform/renesas/rzfive/objects.mk
> new file mode 100644
> index 0000000..302b3d6
> --- /dev/null
> +++ b/platform/renesas/rzfive/objects.mk
> @@ -0,0 +1,18 @@
> +#
> +# SPDX-License-Identifier: BSD-2-Clause
> +#
> +# Copyright (C) 2022 Renesas Electronics Corp.
> +#
> +
> +# Compiler flags
> +platform-cppflags-y =
> +platform-cflags-y =
> +platform-asflags-y =
> +platform-ldflags-y =
> +
> +# Objects to build
> +platform-objs-y += ../../andes/ae350/cache.o platform.o
The fact that you have to compile ae350/cache.c as part of this platform
shows we need to improve code re-use.
Further, the platform.c added by this patch is very similar to platform.c
of andes/ae350. Both platform.c files are subset of what is already there
in generic/platform.c.
I suggest you do the following:
1) Move andes/ae350 as a platform override under generic/platform/andes/ae350
2) Add rzfive as platform override under generic/platform/renesas/rzfive
3) The common Andes cache operations will be a library under
generic/platform/andes/ae350 which can be selected by renesas/rzfive
4) The common Andes header can be under generic/include/andes/ae350
5) Enable drivers required by andes/ae350 and renesas/rzfive platform
overrides in generic/configs/defconfig
Regards,
Anup
> +
> +# Blobs to build
> +FW_TEXT_START=0x00000000
> +FW_DYNAMIC=y
> diff --git a/platform/renesas/rzfive/platform.c b/platform/renesas/rzfive/platform.c
> new file mode 100644
> index 0000000..cfc557c
> --- /dev/null
> +++ b/platform/renesas/rzfive/platform.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + *
> + * Based on platform/andes/ae350/platform.c
> + * Copyright (c) 2019 Andes Technology Corporation
> + */
> +
> +#include <libfdt.h>
> +#include <sbi/riscv_asm.h>
> +#include <sbi/riscv_encoding.h>
> +#include <sbi/sbi_console.h>
> +#include <sbi/sbi_const.h>
> +#include <sbi/sbi_hartmask.h>
> +#include <sbi/sbi_ipi.h>
> +#include <sbi/sbi_platform.h>
> +#include <sbi/sbi_trap.h>
> +#include <sbi_utils/fdt/fdt_domain.h>
> +#include <sbi_utils/fdt/fdt_fixup.h>
> +#include <sbi_utils/fdt/fdt_helper.h>
> +#include <sbi_utils/ipi/fdt_ipi.h>
> +#include <sbi_utils/irqchip/fdt_irqchip.h>
> +#include <sbi_utils/serial/fdt_serial.h>
> +#include <sbi_utils/timer/fdt_timer.h>
> +
> +#include "../../andes/ae350/cache.h"
> +
> +#include "platform.h"
> +
> +struct sbi_platform platform;
> +unsigned long fw_platform_init(unsigned long arg0, unsigned long arg1,
> + unsigned long arg2, unsigned long arg3,
> + unsigned long arg4)
> +{
> + const char *model;
> + void *fdt = (void *)arg1;
> + u32 hartid, hart_count = 0;
> + int rc, root_offset, cpus_offset, cpu_offset, len;
> +
> + root_offset = fdt_path_offset(fdt, "/");
> + if (root_offset < 0)
> + goto fail;
> +
> + model = fdt_getprop(fdt, root_offset, "model", &len);
> + if (model)
> + sbi_strncpy(platform.name, model, sizeof(platform.name) - 1);
> +
> + cpus_offset = fdt_path_offset(fdt, "/cpus");
> + if (cpus_offset < 0)
> + goto fail;
> +
> + fdt_for_each_subnode(cpu_offset, fdt, cpus_offset) {
> + rc = fdt_parse_hart_id(fdt, cpu_offset, &hartid);
> + if (rc)
> + continue;
> +
> + if (SBI_HARTMASK_MAX_BITS <= hartid)
> + continue;
> +
> + hart_count++;
> + }
> +
> + platform.hart_count = hart_count;
> +
> + /* Return original FDT pointer */
> + return arg1;
> +
> +fail:
> + while (1)
> + wfi();
> +}
> +
> +/* Platform final initialization. */
> +static int rzfive_final_init(bool cold_boot)
> +{
> + void *fdt;
> +
> + if (!cold_boot)
> + return 0;
> +
> + fdt = fdt_get_address();
> + fdt_fixups(fdt);
> + fdt_domain_fixup(fdt);
> +
> + return 0;
> +}
> +
> +/* Vendor-Specific SBI handler */
> +static int rzfive_vendor_ext_provider(long extid, long funcid,
> + const struct sbi_trap_regs *regs,
> + unsigned long *out_value,
> + struct sbi_trap_info *out_trap)
> +{
> + int ret = 0;
> +
> + switch (funcid) {
> + case SBI_EXT_RENESAS_RZFIVE_GET_MCACHE_CTL_STATUS:
> + *out_value = csr_read(CSR_MCACHECTL);
> + break;
> + case SBI_EXT_RENESAS_RZFIVE_GET_MMISC_CTL_STATUS:
> + *out_value = csr_read(CSR_MMISCCTL);
> + break;
> + case SBI_EXT_RENESAS_RZFIVE_SET_MCACHE_CTL:
> + ret = mcall_set_mcache_ctl(regs->a0);
> + break;
> + case SBI_EXT_RENESAS_RZFIVE_SET_MMISC_CTL:
> + ret = mcall_set_mmisc_ctl(regs->a0);
> + break;
> + case SBI_EXT_RENESAS_RZFIVE_ICACHE_OP:
> + ret = mcall_icache_op(regs->a0);
> + break;
> + case SBI_EXT_RENESAS_RZFIVE_DCACHE_OP:
> + ret = mcall_dcache_op(regs->a0);
> + break;
> + case SBI_EXT_RENESAS_RZFIVE_L1CACHE_I_PREFETCH:
> + ret = mcall_l1_cache_i_prefetch_op(regs->a0);
> + break;
> + case SBI_EXT_RENESAS_RZFIVE_L1CACHE_D_PREFETCH:
> + ret = mcall_l1_cache_d_prefetch_op(regs->a0);
> + break;
> + case SBI_EXT_RENESAS_RZFIVE_NON_BLOCKING_LOAD_STORE:
> + ret = mcall_non_blocking_load_store(regs->a0);
> + break;
> + case SBI_EXT_RENESAS_RZFIVE_WRITE_AROUND:
> + ret = mcall_write_around(regs->a0);
> + break;
> + default:
> + sbi_printf("Unsupported vendor sbi call : %ld\n", funcid);
> + asm volatile("ebreak");
> + }
> +
> + return ret;
> +}
> +
> +static int rzfive_domains_init(void)
> +{
> + return fdt_domains_populate(fdt_get_address());
> +}
> +
> +/* Platform descriptor. */
> +static const struct sbi_platform_operations platform_ops = {
> + .final_init = rzfive_final_init,
> + .domains_init = rzfive_domains_init,
> + .console_init = fdt_serial_init,
> + .irqchip_init = fdt_irqchip_init,
> + .ipi_init = fdt_ipi_init,
> + .timer_init = fdt_timer_init,
> + .vendor_ext_provider = rzfive_vendor_ext_provider
> +};
> +
> +struct sbi_platform platform = {
> + .opensbi_version = OPENSBI_VERSION,
> + .platform_version =
> + SBI_PLATFORM_VERSION(CONFIG_PLATFORM_RENESAS_RZFIVE_MAJOR_VER,
> + CONFIG_PLATFORM_RENESAS_RZFIVE_MINOR_VER),
> + .name = CONFIG_PLATFORM_RENESAS_RZFIVE_NAME,
> + .features = SBI_PLATFORM_DEFAULT_FEATURES,
> + .hart_count = SBI_HARTMASK_MAX_BITS,
> + .hart_stack_size = SBI_PLATFORM_DEFAULT_HART_STACK_SIZE,
> + .platform_ops_addr = (unsigned long)&platform_ops
> +};
> diff --git a/platform/renesas/rzfive/platform.h b/platform/renesas/rzfive/platform.h
> new file mode 100644
> index 0000000..e46ca91
> --- /dev/null
> +++ b/platform/renesas/rzfive/platform.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (C) 2022 Renesas Electronics Corp.
> + */
> +
> +#ifndef _RZFIVE_PLATFORM_H_
> +#define _RZFIVE_PLATFORM_H_
> +
> +#include "../../andes/ae350/common-platform.h"
> +
> +enum sbi_ext_andes_fid {
> + SBI_EXT_RENESAS_RZFIVE_GET_MCACHE_CTL_STATUS = 0,
> + SBI_EXT_RENESAS_RZFIVE_GET_MMISC_CTL_STATUS,
> + SBI_EXT_RENESAS_RZFIVE_SET_MCACHE_CTL,
> + SBI_EXT_RENESAS_RZFIVE_SET_MMISC_CTL,
> + SBI_EXT_RENESAS_RZFIVE_ICACHE_OP,
> + SBI_EXT_RENESAS_RZFIVE_DCACHE_OP,
> + SBI_EXT_RENESAS_RZFIVE_L1CACHE_I_PREFETCH,
> + SBI_EXT_RENESAS_RZFIVE_L1CACHE_D_PREFETCH,
> + SBI_EXT_RENESAS_RZFIVE_NON_BLOCKING_LOAD_STORE,
> + SBI_EXT_RENESAS_RZFIVE_WRITE_AROUND,
> +};
> +
> +#endif /* _RZFIVE_PLATFORM_H_ */
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 5/7] platform: Add Renesas RZ/Five initial support
2022-11-15 11:44 ` Anup Patel
@ 2022-11-16 15:53 ` Lad, Prabhakar
0 siblings, 0 replies; 20+ messages in thread
From: Lad, Prabhakar @ 2022-11-16 15:53 UTC (permalink / raw)
To: opensbi
Hi Anup,
Thank you for the review.
On Tue, Nov 15, 2022 at 11:44 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Fri, Nov 11, 2022 at 11:51 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> >
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > This commit provides basic support for the Renesas RZ/Five
> > (R9A07G043F) SoC.
> >
> > The RZ/Five microprocessor includes a single RISC-V CPU Core (Andes AX45MP)
> > 1.0 GHz, 16-bit DDR3L/DDR4 interface. Supported interfaces include:
> > - Gigabit Ethernet 2ch
> > - CAN interface (CAN-FD) 2ch
> > - USB 2.0 interface 2ch
> > - SD interface 2ch
> > - AD converter 2ch
> >
> > Useful links:
> > -------------
> > [0] https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz-mpus/rzfive-risc-v-general-purpose-microprocessors-risc-v-cpu-core-andes-ax45mp-single-10-ghz-2ch-gigabit-ethernet
> > [1] http://www.andestech.com/en/products-solutions/andescore-processors/riscv-ax45mp/
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > RFC->v2
> > * Updated commit description
> > ---
> > platform/renesas/rzfive/Kconfig | 32 +++++
> > platform/renesas/rzfive/configs/defconfig | 0
> > platform/renesas/rzfive/objects.mk | 18 +++
> > platform/renesas/rzfive/platform.c | 161 ++++++++++++++++++++++
> > platform/renesas/rzfive/platform.h | 24 ++++
> > 5 files changed, 235 insertions(+)
> > create mode 100644 platform/renesas/rzfive/Kconfig
> > create mode 100644 platform/renesas/rzfive/configs/defconfig
> > create mode 100644 platform/renesas/rzfive/objects.mk
> > create mode 100644 platform/renesas/rzfive/platform.c
> > create mode 100644 platform/renesas/rzfive/platform.h
> >
> > diff --git a/platform/renesas/rzfive/Kconfig b/platform/renesas/rzfive/Kconfig
> > new file mode 100644
> > index 0000000..c5e8a19
> > --- /dev/null
> > +++ b/platform/renesas/rzfive/Kconfig
> > @@ -0,0 +1,32 @@
> > +# SPDX-License-Identifier: BSD-2-Clause
> > +
> > +config PLATFORM_RENESAS_RZFIVE
> > + bool
> > + select FDT
> > + select FDT_SERIAL
> > + select FDT_SERIAL_RENESAS_SCIF
> > + select FDT_TIMER
> > + select FDT_TIMER_PLMT
> > + select FDT_IRQCHIP
> > + select FDT_IRQCHIP_PLIC
> > + select FDT_IPI
> > + select FDT_IPI_PLICSW
> > + default y
> > +
> > +if PLATFORM_RENESAS_RZFIVE
> > +
> > +config PLATFORM_RENESAS_RZFIVE_NAME
> > + string "Platform default name"
> > + default "Renesas RZ/Five"
> > +
> > +config PLATFORM_RENESAS_RZFIVE_MAJOR_VER
> > + int "Platform major version"
> > + range 0 65535
> > + default 0
> > +
> > +config PLATFORM_RENESAS_RZFIVE_MINOR_VER
> > + int "Platform minor version"
> > + range 0 65535
> > + default 1
> > +
> > +endif
> > diff --git a/platform/renesas/rzfive/configs/defconfig b/platform/renesas/rzfive/configs/defconfig
> > new file mode 100644
> > index 0000000..e69de29
> > diff --git a/platform/renesas/rzfive/objects.mk b/platform/renesas/rzfive/objects.mk
> > new file mode 100644
> > index 0000000..302b3d6
> > --- /dev/null
> > +++ b/platform/renesas/rzfive/objects.mk
> > @@ -0,0 +1,18 @@
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause
> > +#
> > +# Copyright (C) 2022 Renesas Electronics Corp.
> > +#
> > +
> > +# Compiler flags
> > +platform-cppflags-y =
> > +platform-cflags-y =
> > +platform-asflags-y =
> > +platform-ldflags-y =
> > +
> > +# Objects to build
> > +platform-objs-y += ../../andes/ae350/cache.o platform.o
>
> The fact that you have to compile ae350/cache.c as part of this platform
> shows we need to improve code re-use.
>
> Further, the platform.c added by this patch is very similar to platform.c
> of andes/ae350. Both platform.c files are subset of what is already there
> in generic/platform.c.
>
> I suggest you do the following:
> 1) Move andes/ae350 as a platform override under generic/platform/andes/ae350
> 2) Add rzfive as platform override under generic/platform/renesas/rzfive
> 3) The common Andes cache operations will be a library under
> generic/platform/andes/ae350 which can be selected by renesas/rzfive
> 4) The common Andes header can be under generic/include/andes/ae350
> 5) Enable drivers required by andes/ae350 and renesas/rzfive platform
> overrides in generic/configs/defconfig
>
- The reason for not using generic is because the FW_TEXT_START is different.
- I came across the thread where because the generic FW uses FW_PIC
it doesn matter about the FW_TEXT_START, is this correct?
- Moving forward I built rz/five as part of generic override successfully
- But when I try and build the u-boot with fw_dynamic.bin I get the
below errors:
- Image 'main-section' is missing external blobs and is
non-functional: opensbi
- Image 'main-section' has faked external blobs and is
non-functional: fw_dynamic.bin
- Ignoring the warning and still flashing the image I dont get any
output from opensbi.
Am I missing something here?
Cheers,
Prabhakar
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 5/7] platform: Add Renesas RZ/Five initial support
2022-11-11 18:21 ` [PATCH v2 5/7] platform: Add Renesas RZ/Five initial support Prabhakar
2022-11-15 11:44 ` Anup Patel
@ 2022-11-28 9:23 ` Yu-Chien Peter Lin
2022-11-28 3:37 ` Anup Patel
1 sibling, 1 reply; 20+ messages in thread
From: Yu-Chien Peter Lin @ 2022-11-28 9:23 UTC (permalink / raw)
To: opensbi
Hi Prabhakar,
> > The fact that you have to compile ae350/cache.c as part of this platform
> > shows we need to improve code re-use.
> >
> > Further, the platform.c added by this patch is very similar to platform.c
> > of andes/ae350. Both platform.c files are subset of what is already there
> > in generic/platform.c.
> >
> > I suggest you do the following:
> > 1) Move andes/ae350 as a platform override under generic/platform/andes/ae350
> > 2) Add rzfive as platform override under generic/platform/renesas/rzfive
> > 3) The common Andes cache operations will be a library under
> > generic/platform/andes/ae350 which can be selected by renesas/rzfive
> > 4) The common Andes header can be under generic/include/andes/ae350
> > 5) Enable drivers required by andes/ae350 and renesas/rzfive platform
> > overrides in generic/configs/defconfig
> >
> - The reason for not using generic is because the FW_TEXT_START is different.
> - I came across the thread where because the generic FW uses FW_PIC
> it doesn matter about the FW_TEXT_START, is this correct?
We will use generic platform as Anup suggested. I'll send our patch soon.
When FW_PIC=y (default), set CONFIG_SPL_OPENSBI_LOAD_ADDR to 0x44000000
in U-boot should be able to run OpenSBI.
> - Moving forward I built rz/five as part of generic override successfully
> - But when I try and build the u-boot with fw_dynamic.bin I get the
> below errors:
> - Image 'main-section' is missing external blobs and is
> non-functional: opensbi
> - Image 'main-section' has faked external blobs and is
> non-functional: fw_dynamic.bin
> - Ignoring the warning and still flashing the image I dont get any
> output from opensbi.
The error message looks like the fw_dynamic.bin is not copied to top
directory of U-boot, so a dummy one is created.
The load/entry addresses and binary of OpenSBI can be observed by using
`dtc u-boot.itb`.
> Am I missing something here?
>
> Cheers,
> Prabhakar
>
Best regards,
Peter Lin
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 5/7] platform: Add Renesas RZ/Five initial support
2022-11-28 9:23 ` Yu-Chien Peter Lin
@ 2022-11-28 3:37 ` Anup Patel
0 siblings, 0 replies; 20+ messages in thread
From: Anup Patel @ 2022-11-28 3:37 UTC (permalink / raw)
To: opensbi
On Mon, Nov 28, 2022 at 6:56 AM Yu-Chien Peter Lin
<peterlin@andestech.com> wrote:
>
> Hi Prabhakar,
>
> > > The fact that you have to compile ae350/cache.c as part of this platform
> > > shows we need to improve code re-use.
> > >
> > > Further, the platform.c added by this patch is very similar to platform.c
> > > of andes/ae350. Both platform.c files are subset of what is already there
> > > in generic/platform.c.
> > >
> > > I suggest you do the following:
> > > 1) Move andes/ae350 as a platform override under generic/platform/andes/ae350
> > > 2) Add rzfive as platform override under generic/platform/renesas/rzfive
> > > 3) The common Andes cache operations will be a library under
> > > generic/platform/andes/ae350 which can be selected by renesas/rzfive
> > > 4) The common Andes header can be under generic/include/andes/ae350
> > > 5) Enable drivers required by andes/ae350 and renesas/rzfive platform
> > > overrides in generic/configs/defconfig
> > >
> > - The reason for not using generic is because the FW_TEXT_START is different.
> > - I came across the thread where because the generic FW uses FW_PIC
> > it doesn matter about the FW_TEXT_START, is this correct?
>
> We will use generic platform as Anup suggested. I'll send our patch soon.
> When FW_PIC=y (default), set CONFIG_SPL_OPENSBI_LOAD_ADDR to 0x44000000
> in U-boot should be able to run OpenSBI.
Thanks for taking this up.
Please keep in mind that OpenSBI v1.2 release will be towards the end
of Dec 2022. It will be nice to support both andes/ae350 and RZ/Five
in this upcoming release.
>
> > - Moving forward I built rz/five as part of generic override successfully
> > - But when I try and build the u-boot with fw_dynamic.bin I get the
> > below errors:
> > - Image 'main-section' is missing external blobs and is
> > non-functional: opensbi
> > - Image 'main-section' has faked external blobs and is
> > non-functional: fw_dynamic.bin
> > - Ignoring the warning and still flashing the image I dont get any
> > output from opensbi.
>
> The error message looks like the fw_dynamic.bin is not copied to top
> directory of U-boot, so a dummy one is created.
> The load/entry addresses and binary of OpenSBI can be observed by using
> `dtc u-boot.itb`.
>
> > Am I missing something here?
> >
> > Cheers,
> > Prabhakar
> >
>
> Best regards,
> Peter Lin
Regards,
Anup
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 6/7] scripts: Add Renesas RZ/Five to platform list in the binary archive script
2022-11-11 18:20 [PATCH v2 0/7] Add support for Renesas RZ/Five SoC Prabhakar
` (4 preceding siblings ...)
2022-11-11 18:21 ` [PATCH v2 5/7] platform: Add Renesas RZ/Five initial support Prabhakar
@ 2022-11-11 18:21 ` Prabhakar
2022-11-11 18:21 ` [PATCH v2 7/7] docs: platform: Add documentation for Renesas RZ/Five SoC Prabhakar
6 siblings, 0 replies; 20+ messages in thread
From: Prabhakar @ 2022-11-11 18:21 UTC (permalink / raw)
To: opensbi
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
This patch adds Renesas RZ/Five (R9A07G043F) SoC to RV64 platform list in
the binary archive script.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
* No change
---
scripts/create-binary-archive.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/create-binary-archive.sh b/scripts/create-binary-archive.sh
index 261a45a..28a5b2f 100755
--- a/scripts/create-binary-archive.sh
+++ b/scripts/create-binary-archive.sh
@@ -104,6 +104,7 @@ build_opensbi() {
BUILD_PLATFORM_SUBDIR+=("fpga/ariane")
BUILD_PLATFORM_SUBDIR+=("fpga/openpiton")
BUILD_PLATFORM_SUBDIR+=("andes/ae350")
+ BUILD_PLATFORM_SUBDIR+=("renesas/rzfive")
BUILD_PLATFORM_SUBDIR+=("generic")
;;
*)
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v2 7/7] docs: platform: Add documentation for Renesas RZ/Five SoC
2022-11-11 18:20 [PATCH v2 0/7] Add support for Renesas RZ/Five SoC Prabhakar
` (5 preceding siblings ...)
2022-11-11 18:21 ` [PATCH v2 6/7] scripts: Add Renesas RZ/Five to platform list in the binary archive script Prabhakar
@ 2022-11-11 18:21 ` Prabhakar
6 siblings, 0 replies; 20+ messages in thread
From: Prabhakar @ 2022-11-11 18:21 UTC (permalink / raw)
To: opensbi
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
This patch adds documentation to build Renesas RZ/Five (R9A07G043F) SoC.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
RFC->v2
* Updated doc
---
docs/platform/platform.md | 5 ++
docs/platform/renesas-rzfive.md | 143 ++++++++++++++++++++++++++++++++
2 files changed, 148 insertions(+)
create mode 100644 docs/platform/renesas-rzfive.md
diff --git a/docs/platform/platform.md b/docs/platform/platform.md
index f291931..4504d87 100644
--- a/docs/platform/platform.md
+++ b/docs/platform/platform.md
@@ -39,6 +39,10 @@ OpenSBI currently supports the following virtual and hardware platforms:
processor based SOCs. More details on this platform can be found in the
file *[shakti_cclass.md]*.
+* **Renesas RZ/Five SoC**: Platform support for Renesas RZ/Five (R9A07G043F) SoC
+ used on the Renesas RZ/Five SMARC EVK board. More details on this platform can
+ be found in the file *[renesas-rzfive.md]*.
+
The code for these supported platforms can be used as example to implement
support for other platforms. The *platform/template* directory also provides
template files for implementing support for a new platform. The *objects.mk*,
@@ -54,3 +58,4 @@ comments to facilitate the implementation.
[spike.md]: spike.md
[fpga-openpiton.md]: fpga-openpiton.md
[shakti_cclass.md]: shakti_cclass.md
+[renesas-rzfive.md]: renesas-rzfive.md
diff --git a/docs/platform/renesas-rzfive.md b/docs/platform/renesas-rzfive.md
new file mode 100644
index 0000000..94c7654
--- /dev/null
+++ b/docs/platform/renesas-rzfive.md
@@ -0,0 +1,143 @@
+Renesas RZ/Five SoC (R9A07G043F) Platform
+=========================================
+The RZ/Five microprocessor includes a single RISC-V CPU Core (Andes AX45MP)
+1.0 GHz, 16-bit DDR3L/DDR4 interface. Supported interfaces include:
+- Gigabit Ethernet 2ch
+- CAN interface (CAN-FD) 2ch
+- USB 2.0 interface 2ch
+- SD interface 2ch
+- AD converter 2ch
+making it ideal for applications such as entry-class social infrastructure
+gateway control and industrial gateway control.
+
+To build platform specific library and firmwares, provide the
+*PLATFORM=renesas/rzfive* parameter to the top level make command.
+
+Platform Options
+----------------
+
+The Renesas RZ/Five platform does not have any platform-specific options.
+
+Building Renesas RZ/Five Platform
+---------------------------------
+
+```
+make PLATFORM=renesas/rzfive
+```
+
+DTS Example: (RZ/Five AX45MP)
+-----------------------------
+
+```
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ timebase-frequency = <12000000>;
+
+ cpu0: cpu at 0 {
+ compatible = "andestech,ax45mp", "riscv";
+ device_type = "cpu";
+ reg = <0x0>;
+ status = "okay";
+ riscv,isa = "rv64imafdc";
+ mmu-type = "riscv,sv39";
+ i-cache-size = <0x8000>;
+ i-cache-line-size = <0x40>;
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <0x40>;
+ clocks = <&cpg CPG_CORE R9A07G043_AX45MP_CORE0_CLK>,
+ <&cpg CPG_CORE R9A07G043_AX45MP_ACLK>;
+
+ cpu0_intc: interrupt-controller {
+ #interrupt-cells = <1>;
+ compatible = "riscv,cpu-intc";
+ interrupt-controller;
+ };
+ };
+ };
+
+ soc {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ranges;
+
+ scif0: serial at 1004b800 {
+ compatible = "renesas,scif-r9a07g043",
+ "renesas,scif-r9a07g044";
+ reg = <0 0x1004b800 0 0x400>;
+ interrupts = <412 IRQ_TYPE_LEVEL_HIGH>,
+ <414 IRQ_TYPE_LEVEL_HIGH>,
+ <415 IRQ_TYPE_LEVEL_HIGH>,
+ <413 IRQ_TYPE_LEVEL_HIGH>,
+ <416 IRQ_TYPE_LEVEL_HIGH>,
+ <416 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "eri", "rxi", "txi",
+ "bri", "dri", "tei";
+ clocks = <&cpg CPG_MOD R9A07G043_SCIF0_CLK_PCK>;
+ clock-names = "fck";
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G043_SCIF0_RST_SYSTEM_N>;
+ status = "disabled";
+ };
+
+ cpg: clock-controller at 11010000 {
+ compatible = "renesas,r9a07g043-cpg";
+ reg = <0 0x11010000 0 0x10000>;
+ clocks = <&extal_clk>;
+ clock-names = "extal";
+ #clock-cells = <2>;
+ #reset-cells = <1>;
+ #power-domain-cells = <0>;
+ };
+
+ sysc: system-controller at 11020000 {
+ compatible = "renesas,r9a07g043-sysc";
+ reg = <0 0x11020000 0 0x10000>;
+ status = "disabled";
+ };
+
+ pinctrl: pinctrl at 11030000 {
+ compatible = "renesas,r9a07g043-pinctrl";
+ reg = <0 0x11030000 0 0x10000>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #interrupt-cells = <2>;
+ interrupt-controller;
+ gpio-ranges = <&pinctrl 0 0 152>;
+ clocks = <&cpg CPG_MOD R9A07G043_GPIO_HCLK>;
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G043_GPIO_RSTN>,
+ <&cpg R9A07G043_GPIO_PORT_RESETN>,
+ <&cpg R9A07G043_GPIO_SPARE_RESETN>;
+ };
+
+ plmt0: plmt0 at 110c0000 {
+ compatible = "andestech,plmt0", "riscv,plmt0";
+ reg = <0x00000000 0x110c0000 0x00000000 0x00100000>;
+ interrupts-extended = <&cpu0_intc 7>;
+ };
+
+ plic: interrupt-controller at 12c00000 {
+ compatible = "renesas,r9a07g043-plic", "andestech,nceplic100";
+ #interrupt-cells = <2>;
+ #address-cells = <0>;
+ riscv,ndev = <511>;
+ interrupt-controller;
+ reg = <0x0 0x12c00000 0 0x400000>;
+ clocks = <&cpg CPG_MOD R9A07G043_NCEPLIC_ACLK>;
+ power-domains = <&cpg>;
+ resets = <&cpg R9A07G043_NCEPLIC_ARESETN>;
+ interrupts-extended = <&cpu0_intc 11 &cpu0_intc 9>;
+ };
+
+ plicsw: interrupt-controller at 13000000 {
+ compatible = "andestech,plicsw";
+ reg = <0x00000000 0x13000000 0x00000000 0x00400000>;
+ interrupts-extended = <&cpu0_intc 3>;
+ interrupt-controller;
+ #address-cells = <2>;
+ #interrupt-cells = <2>;
+ };
+ };
+```
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
