[U-Boot] [PATCH v7 0/4] RISC-V S-mode support
This patchset allows us runing u-boot in S-mode which is useful on platforms where M-mode runtime firmware is an independent firmware and u-boot is used as last stage OS bootloader.
The patchset based upon git://git.denx.de/u-boot-riscv.git and is tested on QEMU in both M-mode and S-mode.
For S-mode testing, we have used u-boot.bin as payload of latest BBL (at commit 6ebd0f2a46255d0c76dad3c05b16c1d154795d26) applied with following changes:
diff --git a/machine/emulation.c b/machine/emulation.c index 132e977..def75e1 100644 --- a/machine/emulation.c +++ b/machine/emulation.c @@ -162,6 +162,12 @@ static inline int emulate_read_csr(int num, uintptr_t mstatus, uintptr_t* result
switch (num) { + case CSR_MISA: + *result = read_csr(misa); + return 0; + case CSR_MHARTID: + *result = read_csr(mhartid); + return 0; case CSR_CYCLE: if (!((counteren >> (CSR_CYCLE - CSR_CYCLE)) & 1)) return -1;
Changes since v6: - Added patch for S-mode timer implementation
Changes since v5: - Dropped PATCH4 to remove redundant a2 store on DRAM base in start.S because it will taken care by Rick as separate patch - Added MODE_PREFIX() macro to generate mode specific CSR names
Changes since v4: - Rebased series based on commit 52923c6db7f00e0197ec894c8c1bb8a7681974bb of git://git.denx.de/u-boot-riscv.git - Added a patch to remove redundant a2 store on DRAM base. This store was creating problem booting U-Boot in S-mode using BBL.
Changes since v3: - Replaced 'u-boot' with 'U-Boot' in commit message - Dropped 'an' in RISCV_SMODE kconfig option help message - Added appropriate #ifdef in arch/riscv/lib/interrupts.c
Changes since v2: - Dropped 'default n" from RISCV_SMODE kconfig option - Replaced '-smode_' in defconfig names with '_smode_'
Changes since v1: - Rebased upon latest git://git.denx.de/u-boot-riscv.git - Add details in cover letter for running u-boot in S-mode using BBL
Anup Patel (4): riscv: Add kconfig option to run U-Boot in S-mode riscv: qemu: Use different SYS_TEXT_BASE for S-mode riscv: Add S-mode defconfigs for QEMU virt machine RISC-V: Add S-mode timer implementation
arch/Kconfig | 1 - arch/riscv/Kconfig | 17 ++++++++ arch/riscv/cpu/start.S | 23 ++++++---- arch/riscv/include/asm/encoding.h | 6 +++ arch/riscv/lib/Makefile | 1 + arch/riscv/lib/interrupts.c | 31 +++++++++---- arch/riscv/lib/time.c | 60 ++++++++++++++++++++++++++ board/emulation/qemu-riscv/Kconfig | 3 +- board/emulation/qemu-riscv/MAINTAINERS | 2 + configs/qemu-riscv32_smode_defconfig | 10 +++++ configs/qemu-riscv64_smode_defconfig | 11 +++++ 11 files changed, 146 insertions(+), 19 deletions(-) create mode 100644 arch/riscv/lib/time.c create mode 100644 configs/qemu-riscv32_smode_defconfig create mode 100644 configs/qemu-riscv64_smode_defconfig
This patch adds kconfig option RISCV_SMODE to run U-Boot in S-mode. When this opition is enabled we use s<xyz> CSRs instead of m<xyz> CSRs.
It is important to note that there is no equivalent S-mode CSR for misa and mhartid CSRs so we expect M-mode runtime firmware (BBL or equivalent) to emulate misa and mhartid CSR read.
In-future, we will have more patches to avoid accessing misa and mhartid CSRs from S-mode.
Signed-off-by: Anup Patel anup@brainfault.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de --- arch/riscv/Kconfig | 5 +++++ arch/riscv/cpu/start.S | 23 +++++++++++++++-------- arch/riscv/include/asm/encoding.h | 6 ++++++ arch/riscv/lib/interrupts.c | 31 ++++++++++++++++++++++--------- 4 files changed, 48 insertions(+), 17 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3e0af55e71..732a357a99 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -55,6 +55,11 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
+config RISCV_SMODE + bool "Run in S-Mode" + help + Enable this option to build U-Boot for RISC-V S-Mode + config 32BIT bool
diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 15e1b8199a..3f055bdb7e 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -41,10 +41,10 @@ _start: li t0, CONFIG_SYS_SDRAM_BASE SREG a2, 0(t0) la t0, trap_entry - csrw mtvec, t0 + csrw MODE_PREFIX(tvec), t0
/* mask all interrupts */ - csrw mie, zero + csrw MODE_PREFIX(ie), zero
/* Enable cache */ jal icache_enable @@ -166,7 +166,7 @@ fix_rela_dyn: */ la t0, trap_entry add t0, t0, t6 - csrw mtvec, t0 + csrw MODE_PREFIX(tvec), t0
clear_bss: la t0, __bss_start /* t0 <- rel __bss_start in FLASH */ @@ -238,17 +238,24 @@ trap_entry: SREG x29, 29*REGBYTES(sp) SREG x30, 30*REGBYTES(sp) SREG x31, 31*REGBYTES(sp) - csrr a0, mcause - csrr a1, mepc + csrr a0, MODE_PREFIX(cause) + csrr a1, MODE_PREFIX(epc) mv a2, sp jal handle_trap - csrw mepc, a0 + csrw MODE_PREFIX(epc), a0
+#ifdef CONFIG_RISCV_SMODE +/* + * Remain in S-mode after sret + */ + li t0, SSTATUS_SPP +#else /* * Remain in M-mode after mret */ li t0, MSTATUS_MPP - csrs mstatus, t0 +#endif + csrs MODE_PREFIX(status), t0 LREG x1, 1*REGBYTES(sp) LREG x2, 2*REGBYTES(sp) LREG x3, 3*REGBYTES(sp) @@ -281,4 +288,4 @@ trap_entry: LREG x30, 30*REGBYTES(sp) LREG x31, 31*REGBYTES(sp) addi sp, sp, 32*REGBYTES - mret + MODE_PREFIX(ret) diff --git a/arch/riscv/include/asm/encoding.h b/arch/riscv/include/asm/encoding.h index 9ea50ce640..97cf906aa6 100644 --- a/arch/riscv/include/asm/encoding.h +++ b/arch/riscv/include/asm/encoding.h @@ -7,6 +7,12 @@ #ifndef RISCV_CSR_ENCODING_H #define RISCV_CSR_ENCODING_H
+#ifdef CONFIG_RISCV_SMODE +#define MODE_PREFIX(__suffix) s##__suffix +#else +#define MODE_PREFIX(__suffix) m##__suffix +#endif + #define MSTATUS_UIE 0x00000001 #define MSTATUS_SIE 0x00000002 #define MSTATUS_HIE 0x00000004 diff --git a/arch/riscv/lib/interrupts.c b/arch/riscv/lib/interrupts.c index 903a1c4cd5..3aff006977 100644 --- a/arch/riscv/lib/interrupts.c +++ b/arch/riscv/lib/interrupts.c @@ -34,17 +34,30 @@ int disable_interrupts(void) return 0; }
-ulong handle_trap(ulong mcause, ulong epc, struct pt_regs *regs) +ulong handle_trap(ulong cause, ulong epc, struct pt_regs *regs) { - ulong is_int; + ulong is_irq, irq;
- is_int = (mcause & MCAUSE_INT); - if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_EXT)) - external_interrupt(0); /* handle_m_ext_interrupt */ - else if ((is_int) && ((mcause & MCAUSE_CAUSE) == IRQ_M_TIMER)) - timer_interrupt(0); /* handle_m_timer_interrupt */ - else - _exit_trap(mcause, epc, regs); + is_irq = (cause & MCAUSE_INT); + irq = (cause & ~MCAUSE_INT); + + if (is_irq) { + switch (irq) { + case IRQ_M_EXT: + case IRQ_S_EXT: + external_interrupt(0); /* handle external interrupt */ + break; + case IRQ_M_TIMER: + case IRQ_S_TIMER: + timer_interrupt(0); /* handle timer interrupt */ + break; + default: + _exit_trap(cause, epc, regs); + break; + }; + } else { + _exit_trap(cause, epc, regs); + }
return epc; }
When u-boot runs in S-mode, the M-mode runtime firmware (BBL or equivalent) uses memory range in 0x80000000 to 0x80200000. Due to this, we cannot use 0x80000000 as SYS_TEXT_BASE when running in S-mode. Instead for S-mode, we use 0x80200000 as SYS_TEXT_BASE.
Even Linux RISC-V kernel ignores/reserves memory range 0x80000000 to 0x80200000 because it runs in S-mode.
Signed-off-by: Anup Patel anup@brainfault.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de --- board/emulation/qemu-riscv/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/board/emulation/qemu-riscv/Kconfig b/board/emulation/qemu-riscv/Kconfig index 33ca253432..56bb5337d4 100644 --- a/board/emulation/qemu-riscv/Kconfig +++ b/board/emulation/qemu-riscv/Kconfig @@ -13,7 +13,8 @@ config SYS_CONFIG_NAME default "qemu-riscv"
config SYS_TEXT_BASE - default 0x80000000 + default 0x80000000 if !RISCV_SMODE + default 0x80200000 if RISCV_SMODE
config BOARD_SPECIFIC_OPTIONS # dummy def_bool y
This patch adds S-mode defconfigs for QEMU virt machine so that we can run u-boot in S-mode on QEMU using M-mode runtime firmware (BBL or equivalent).
Signed-off-by: Anup Patel anup@brainfault.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de --- board/emulation/qemu-riscv/MAINTAINERS | 2 ++ configs/qemu-riscv32_smode_defconfig | 10 ++++++++++ configs/qemu-riscv64_smode_defconfig | 11 +++++++++++ 3 files changed, 23 insertions(+) create mode 100644 configs/qemu-riscv32_smode_defconfig create mode 100644 configs/qemu-riscv64_smode_defconfig
diff --git a/board/emulation/qemu-riscv/MAINTAINERS b/board/emulation/qemu-riscv/MAINTAINERS index 3c6eb4f844..c701c83d77 100644 --- a/board/emulation/qemu-riscv/MAINTAINERS +++ b/board/emulation/qemu-riscv/MAINTAINERS @@ -4,4 +4,6 @@ S: Maintained F: board/emulation/qemu-riscv/ F: include/configs/qemu-riscv.h F: configs/qemu-riscv32_defconfig +F: configs/qemu-riscv32_smode_defconfig F: configs/qemu-riscv64_defconfig +F: configs/qemu-riscv64_smode_defconfig diff --git a/configs/qemu-riscv32_smode_defconfig b/configs/qemu-riscv32_smode_defconfig new file mode 100644 index 0000000000..0a84ec1874 --- /dev/null +++ b/configs/qemu-riscv32_smode_defconfig @@ -0,0 +1,10 @@ +CONFIG_RISCV=y +CONFIG_TARGET_QEMU_VIRT=y +CONFIG_RISCV_SMODE=y +CONFIG_DISTRO_DEFAULTS=y +CONFIG_NR_DRAM_BANKS=1 +CONFIG_FIT=y +CONFIG_DISPLAY_CPUINFO=y +CONFIG_DISPLAY_BOARDINFO=y +# CONFIG_CMD_MII is not set +CONFIG_OF_PRIOR_STAGE=y diff --git a/configs/qemu-riscv64_smode_defconfig b/configs/qemu-riscv64_smode_defconfig new file mode 100644 index 0000000000..b012443370 --- /dev/null +++ b/configs/qemu-riscv64_smode_defconfig @@ -0,0 +1,11 @@ +CONFIG_RISCV=y +CONFIG_TARGET_QEMU_VIRT=y +CONFIG_ARCH_RV64I=y +CONFIG_RISCV_SMODE=y +CONFIG_DISTRO_DEFAULTS=y +CONFIG_NR_DRAM_BANKS=1 +CONFIG_FIT=y +CONFIG_DISPLAY_CPUINFO=y +CONFIG_DISPLAY_BOARDINFO=y +# CONFIG_CMD_MII is not set +CONFIG_OF_PRIOR_STAGE=y
When running in S-mode, we can use rdtime and rdtimeh instructions for reading timer ticks (just like Linux). The frequency of timer ticks is passed by prior booting stages in "timebase-frequency" DT property of the "/cpus" DT node.
This patch provides a generic timer implementation for U-Boot running in S-mode. For U-Boot running in M-mode, specific timer drivers will have to be provided.
Signed-off-by: Anup Patel anup@brainfault.org --- arch/Kconfig | 1 - arch/riscv/Kconfig | 22 +++++++++++---- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/time.c
diff --git a/arch/Kconfig b/arch/Kconfig index 9fdd2f7e66..a4fcb3522d 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -72,7 +72,6 @@ config RISCV imply BLK imply CLK imply MTD - imply TIMER imply CMD_DM
config SANDBOX diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 732a357a99..20a060454b 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -44,6 +44,23 @@ config ARCH_RV64I
endchoice
+choice + prompt "Run Mode" + default RISCV_MMODE + +config RISCV_MMODE + bool "Machine" + select TIMER + help + Choose this option to build U-Boot for RISC-V M-Mode. + +config RISCV_SMODE + bool "Supervisor" + help + Choose this option to build U-Boot for RISC-V S-Mode. + +endchoice + config RISCV_ISA_C bool "Emit compressed instructions" default y @@ -55,11 +72,6 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
-config RISCV_SMODE - bool "Run in S-Mode" - help - Enable this option to build U-Boot for RISC-V S-Mode - config 32BIT bool
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index b58db89752..98aa6d40e7 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += interrupts.o obj-y += reset.o obj-y += setjmp.o +obj-$(CONFIG_RISCV_SMODE) += time.o
# For building EFI apps CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c new file mode 100644 index 0000000000..077e568ca6 --- /dev/null +++ b/arch/riscv/lib/time.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2018, Anup Patel anup@brainfault.org + */ + +#include <common.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR; + +static unsigned int tbclk; + +static void setup_tbclk(void) +{ + int cpus; + + if (!gd->fdt_blob || tbclk) + return; + + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); + if (cpus < 0) { + debug("%s: Missing /cpus node\n", __func__); + return; + } + + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, + "timebase-frequency", 1000000); +} + +ulong notrace get_tbclk(void) +{ + setup_tbclk(); + + return tbclk; +} + +#ifdef CONFIG_64BIT +uint64_t notrace get_ticks(void) +{ + unsigned long n; + + __asm__ __volatile__ ( + "rdtime %0" + : "=r" (n)); + return n; +} +#else +uint64_t notrace get_ticks(void) +{ + uint32_t lo, hi, tmp; + __asm__ __volatile__ ( + "1:\n" + "rdtimeh %0\n" + "rdtime %1\n" + "rdtimeh %2\n" + "bne %0, %2, 1b" + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); + return ((uint64_t)hi << 32) | lo; +} +#endif
Hi Anup,
On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote:
When running in S-mode, we can use rdtime and rdtimeh instructions for reading timer ticks (just like Linux). The frequency of timer ticks is passed by prior booting stages in "timebase-frequency" DT property of the "/cpus" DT node.
This patch provides a generic timer implementation for U-Boot running in S-mode. For U-Boot running in M-mode, specific timer drivers will have to be provided.
Signed-off-by: Anup Patel anup@brainfault.org
arch/Kconfig | 1 - arch/riscv/Kconfig | 22 +++++++++++---- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/time.c
diff --git a/arch/Kconfig b/arch/Kconfig index 9fdd2f7e66..a4fcb3522d 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -72,7 +72,6 @@ config RISCV imply BLK imply CLK imply MTD
imply TIMER
No, please don't do this.
imply CMD_DMconfig SANDBOX diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 732a357a99..20a060454b 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -44,6 +44,23 @@ config ARCH_RV64I
endchoice
+choice
prompt "Run Mode"default RISCV_MMODE+config RISCV_MMODE
bool "Machine"select TIMERhelpChoose this option to build U-Boot for RISC-V M-Mode.+config RISCV_SMODE
bool "Supervisor"helpChoose this option to build U-Boot for RISC-V S-Mode.+endchoice
The above changes deserve separate patch, or merge that in the 1st patch in the series.
config RISCV_ISA_C bool "Emit compressed instructions" default y @@ -55,11 +72,6 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
-config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index b58db89752..98aa6d40e7 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += interrupts.o obj-y += reset.o obj-y += setjmp.o +obj-$(CONFIG_RISCV_SMODE) += time.o
# For building EFI apps CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c new file mode 100644 index 0000000000..077e568ca6 --- /dev/null +++ b/arch/riscv/lib/time.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2018, Anup Patel anup@brainfault.org
- */
+#include <common.h> +#include <fdtdec.h>
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned int tbclk;
+static void setup_tbclk(void) +{
int cpus;if (!gd->fdt_blob || tbclk)return;cpus = fdt_path_offset(gd->fdt_blob, "/cpus");if (cpus < 0) {debug("%s: Missing /cpus node\n", __func__);return;}tbclk = fdtdec_get_int(gd->fdt_blob, cpus,"timebase-frequency", 1000000);+}
+ulong notrace get_tbclk(void) +{
setup_tbclk();return tbclk;+}
+#ifdef CONFIG_64BIT +uint64_t notrace get_ticks(void) +{
unsigned long n;__asm__ __volatile__ ("rdtime %0": "=r" (n));return n;+} +#else +uint64_t notrace get_ticks(void) +{
uint32_t lo, hi, tmp;__asm__ __volatile__ ("1:\n""rdtimeh %0\n""rdtime %1\n""rdtimeh %2\n""bne %0, %2, 1b": "=&r" (hi), "=&r" (lo), "=&r" (tmp));return ((uint64_t)hi << 32) | lo;+}
+#endif
The proper way to implement timer driver is via DM. Could you please implement a DM timer driver for S-mode?
The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI.
Regards, Bin
On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote:
When running in S-mode, we can use rdtime and rdtimeh instructions for reading timer ticks (just like Linux). The frequency of timer ticks is passed by prior booting stages in "timebase-frequency" DT property of the "/cpus" DT node.
This patch provides a generic timer implementation for U-Boot running in S-mode. For U-Boot running in M-mode, specific timer drivers will have to be provided.
Signed-off-by: Anup Patel anup@brainfault.org
arch/Kconfig | 1 - arch/riscv/Kconfig | 22 +++++++++++---- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/time.c
diff --git a/arch/Kconfig b/arch/Kconfig index 9fdd2f7e66..a4fcb3522d 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -72,7 +72,6 @@ config RISCV imply BLK imply CLK imply MTD
imply TIMERNo, please don't do this.
I am removing here but selecting it only for M-mode U-Boot.
imply CMD_DMconfig SANDBOX diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 732a357a99..20a060454b 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -44,6 +44,23 @@ config ARCH_RV64I
endchoice
+choice
prompt "Run Mode"default RISCV_MMODE+config RISCV_MMODE
bool "Machine"select TIMERhelpChoose this option to build U-Boot for RISC-V M-Mode.+config RISCV_SMODE
bool "Supervisor"helpChoose this option to build U-Boot for RISC-V S-Mode.+endchoice
The above changes deserve separate patch, or merge that in the 1st patch in the series.
Sure, I will put Kconfig changes as separate patch.
config RISCV_ISA_C bool "Emit compressed instructions" default y @@ -55,11 +72,6 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
-config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index b58db89752..98aa6d40e7 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += interrupts.o obj-y += reset.o obj-y += setjmp.o +obj-$(CONFIG_RISCV_SMODE) += time.o
# For building EFI apps CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c new file mode 100644 index 0000000000..077e568ca6 --- /dev/null +++ b/arch/riscv/lib/time.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2018, Anup Patel anup@brainfault.org
- */
+#include <common.h> +#include <fdtdec.h>
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned int tbclk;
+static void setup_tbclk(void) +{
int cpus;if (!gd->fdt_blob || tbclk)return;cpus = fdt_path_offset(gd->fdt_blob, "/cpus");if (cpus < 0) {debug("%s: Missing /cpus node\n", __func__);return;}tbclk = fdtdec_get_int(gd->fdt_blob, cpus,"timebase-frequency", 1000000);+}
+ulong notrace get_tbclk(void) +{
setup_tbclk();return tbclk;+}
+#ifdef CONFIG_64BIT +uint64_t notrace get_ticks(void) +{
unsigned long n;__asm__ __volatile__ ("rdtime %0": "=r" (n));return n;+} +#else +uint64_t notrace get_ticks(void) +{
uint32_t lo, hi, tmp;__asm__ __volatile__ ("1:\n""rdtimeh %0\n""rdtime %1\n""rdtimeh %2\n""bne %0, %2, 1b": "=&r" (hi), "=&r" (lo), "=&r" (tmp));return ((uint64_t)hi << 32) | lo;+}
+#endif
The proper way to implement timer driver is via DM. Could you please implement a DM timer driver for S-mode?
The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI.
"rdtime" is an instruction. It's not an device for which we can implement DM driver.
This is similar to how U-Boot ARM64 uses CNTPCT_EL0 register.
We cannot use DM driver here.
Regards, Anup
Hi Anup,
On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote:
When running in S-mode, we can use rdtime and rdtimeh instructions for reading timer ticks (just like Linux). The frequency of timer ticks is passed by prior booting stages in "timebase-frequency" DT property of the "/cpus" DT node.
This patch provides a generic timer implementation for U-Boot running in S-mode. For U-Boot running in M-mode, specific timer drivers will have to be provided.
Signed-off-by: Anup Patel anup@brainfault.org
arch/Kconfig | 1 - arch/riscv/Kconfig | 22 +++++++++++---- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/time.c
diff --git a/arch/Kconfig b/arch/Kconfig index 9fdd2f7e66..a4fcb3522d 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -72,7 +72,6 @@ config RISCV imply BLK imply CLK imply MTD
imply TIMERNo, please don't do this.
I am removing here but selecting it only for M-mode U-Boot.
imply CMD_DMconfig SANDBOX diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 732a357a99..20a060454b 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -44,6 +44,23 @@ config ARCH_RV64I
endchoice
+choice
prompt "Run Mode"default RISCV_MMODE+config RISCV_MMODE
bool "Machine"select TIMERhelpChoose this option to build U-Boot for RISC-V M-Mode.+config RISCV_SMODE
bool "Supervisor"helpChoose this option to build U-Boot for RISC-V S-Mode.+endchoice
The above changes deserve separate patch, or merge that in the 1st patch in the series.
Sure, I will put Kconfig changes as separate patch.
config RISCV_ISA_C bool "Emit compressed instructions" default y @@ -55,11 +72,6 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
-config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index b58db89752..98aa6d40e7 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += interrupts.o obj-y += reset.o obj-y += setjmp.o +obj-$(CONFIG_RISCV_SMODE) += time.o
# For building EFI apps CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c new file mode 100644 index 0000000000..077e568ca6 --- /dev/null +++ b/arch/riscv/lib/time.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2018, Anup Patel anup@brainfault.org
- */
+#include <common.h> +#include <fdtdec.h>
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned int tbclk;
+static void setup_tbclk(void) +{
int cpus;if (!gd->fdt_blob || tbclk)return;cpus = fdt_path_offset(gd->fdt_blob, "/cpus");if (cpus < 0) {debug("%s: Missing /cpus node\n", __func__);return;}tbclk = fdtdec_get_int(gd->fdt_blob, cpus,"timebase-frequency", 1000000);+}
+ulong notrace get_tbclk(void) +{
setup_tbclk();return tbclk;+}
+#ifdef CONFIG_64BIT +uint64_t notrace get_ticks(void) +{
unsigned long n;__asm__ __volatile__ ("rdtime %0": "=r" (n));return n;+} +#else +uint64_t notrace get_ticks(void) +{
uint32_t lo, hi, tmp;__asm__ __volatile__ ("1:\n""rdtimeh %0\n""rdtime %1\n""rdtimeh %2\n""bne %0, %2, 1b": "=&r" (hi), "=&r" (lo), "=&r" (tmp));return ((uint64_t)hi << 32) | lo;+}
+#endif
The proper way to implement timer driver is via DM. Could you please implement a DM timer driver for S-mode?
The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI.
"rdtime" is an instruction. It's not an device for which we can implement DM driver.
Yes, I know. But that does not mean we cannot write a driver to do the paper work.
This is similar to how U-Boot ARM64 uses CNTPCT_EL0 register.
We cannot use DM driver here.
The issue is that eventually we will migrate all U-Boot device drivers to DM, which means get_tbclk() and get_ticks() you implemented will be broken someday in the future.
Regards, Bin
On Mon, Dec 3, 2018 at 12:32 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote:
When running in S-mode, we can use rdtime and rdtimeh instructions for reading timer ticks (just like Linux). The frequency of timer ticks is passed by prior booting stages in "timebase-frequency" DT property of the "/cpus" DT node.
This patch provides a generic timer implementation for U-Boot running in S-mode. For U-Boot running in M-mode, specific timer drivers will have to be provided.
Signed-off-by: Anup Patel anup@brainfault.org
arch/Kconfig | 1 - arch/riscv/Kconfig | 22 +++++++++++---- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/time.c
diff --git a/arch/Kconfig b/arch/Kconfig index 9fdd2f7e66..a4fcb3522d 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -72,7 +72,6 @@ config RISCV imply BLK imply CLK imply MTD
imply TIMERNo, please don't do this.
I am removing here but selecting it only for M-mode U-Boot.
imply CMD_DMconfig SANDBOX diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 732a357a99..20a060454b 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -44,6 +44,23 @@ config ARCH_RV64I
endchoice
+choice
prompt "Run Mode"default RISCV_MMODE+config RISCV_MMODE
bool "Machine"select TIMERhelpChoose this option to build U-Boot for RISC-V M-Mode.+config RISCV_SMODE
bool "Supervisor"helpChoose this option to build U-Boot for RISC-V S-Mode.+endchoice
The above changes deserve separate patch, or merge that in the 1st patch in the series.
Sure, I will put Kconfig changes as separate patch.
config RISCV_ISA_C bool "Emit compressed instructions" default y @@ -55,11 +72,6 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
-config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index b58db89752..98aa6d40e7 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += interrupts.o obj-y += reset.o obj-y += setjmp.o +obj-$(CONFIG_RISCV_SMODE) += time.o
# For building EFI apps CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c new file mode 100644 index 0000000000..077e568ca6 --- /dev/null +++ b/arch/riscv/lib/time.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2018, Anup Patel anup@brainfault.org
- */
+#include <common.h> +#include <fdtdec.h>
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned int tbclk;
+static void setup_tbclk(void) +{
int cpus;if (!gd->fdt_blob || tbclk)return;cpus = fdt_path_offset(gd->fdt_blob, "/cpus");if (cpus < 0) {debug("%s: Missing /cpus node\n", __func__);return;}tbclk = fdtdec_get_int(gd->fdt_blob, cpus,"timebase-frequency", 1000000);+}
+ulong notrace get_tbclk(void) +{
setup_tbclk();return tbclk;+}
+#ifdef CONFIG_64BIT +uint64_t notrace get_ticks(void) +{
unsigned long n;__asm__ __volatile__ ("rdtime %0": "=r" (n));return n;+} +#else +uint64_t notrace get_ticks(void) +{
uint32_t lo, hi, tmp;__asm__ __volatile__ ("1:\n""rdtimeh %0\n""rdtime %1\n""rdtimeh %2\n""bne %0, %2, 1b": "=&r" (hi), "=&r" (lo), "=&r" (tmp));return ((uint64_t)hi << 32) | lo;+}
+#endif
The proper way to implement timer driver is via DM. Could you please implement a DM timer driver for S-mode?
The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI.
"rdtime" is an instruction. It's not an device for which we can implement DM driver.
Yes, I know. But that does not mean we cannot write a driver to do the paper work.
There is no DT node for "rdtime" so how should we probe the DM driver.
This is similar to how U-Boot ARM64 uses CNTPCT_EL0 register.
We cannot use DM driver here.
The issue is that eventually we will migrate all U-Boot device drivers to DM, which means get_tbclk() and get_ticks() you implemented will be broken someday in the future.
Yes, I agree we should have DM driver whenever possible but sometime there is no DT bindings for things like timer because it's part of CPU instruction or CSRs
Regards, Anup
Hi Anup,
On Mon, Dec 3, 2018 at 3:19 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:32 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote:
When running in S-mode, we can use rdtime and rdtimeh instructions for reading timer ticks (just like Linux). The frequency of timer ticks is passed by prior booting stages in "timebase-frequency" DT property of the "/cpus" DT node.
This patch provides a generic timer implementation for U-Boot running in S-mode. For U-Boot running in M-mode, specific timer drivers will have to be provided.
Signed-off-by: Anup Patel anup@brainfault.org
arch/Kconfig | 1 - arch/riscv/Kconfig | 22 +++++++++++---- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/time.c
diff --git a/arch/Kconfig b/arch/Kconfig index 9fdd2f7e66..a4fcb3522d 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -72,7 +72,6 @@ config RISCV imply BLK imply CLK imply MTD
imply TIMERNo, please don't do this.
I am removing here but selecting it only for M-mode U-Boot.
imply CMD_DMconfig SANDBOX diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 732a357a99..20a060454b 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -44,6 +44,23 @@ config ARCH_RV64I
endchoice
+choice
prompt "Run Mode"default RISCV_MMODE+config RISCV_MMODE
bool "Machine"select TIMERhelpChoose this option to build U-Boot for RISC-V M-Mode.+config RISCV_SMODE
bool "Supervisor"helpChoose this option to build U-Boot for RISC-V S-Mode.+endchoice
The above changes deserve separate patch, or merge that in the 1st patch in the series.
Sure, I will put Kconfig changes as separate patch.
config RISCV_ISA_C bool "Emit compressed instructions" default y @@ -55,11 +72,6 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
-config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index b58db89752..98aa6d40e7 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += interrupts.o obj-y += reset.o obj-y += setjmp.o +obj-$(CONFIG_RISCV_SMODE) += time.o
# For building EFI apps CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c new file mode 100644 index 0000000000..077e568ca6 --- /dev/null +++ b/arch/riscv/lib/time.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2018, Anup Patel anup@brainfault.org
- */
+#include <common.h> +#include <fdtdec.h>
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned int tbclk;
+static void setup_tbclk(void) +{
int cpus;if (!gd->fdt_blob || tbclk)return;cpus = fdt_path_offset(gd->fdt_blob, "/cpus");if (cpus < 0) {debug("%s: Missing /cpus node\n", __func__);return;}tbclk = fdtdec_get_int(gd->fdt_blob, cpus,"timebase-frequency", 1000000);+}
+ulong notrace get_tbclk(void) +{
setup_tbclk();return tbclk;+}
+#ifdef CONFIG_64BIT +uint64_t notrace get_ticks(void) +{
unsigned long n;__asm__ __volatile__ ("rdtime %0": "=r" (n));return n;+} +#else +uint64_t notrace get_ticks(void) +{
uint32_t lo, hi, tmp;__asm__ __volatile__ ("1:\n""rdtimeh %0\n""rdtime %1\n""rdtimeh %2\n""bne %0, %2, 1b": "=&r" (hi), "=&r" (lo), "=&r" (tmp));return ((uint64_t)hi << 32) | lo;+}
+#endif
The proper way to implement timer driver is via DM. Could you please implement a DM timer driver for S-mode?
The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI.
"rdtime" is an instruction. It's not an device for which we can implement DM driver.
Yes, I know. But that does not mean we cannot write a driver to do the paper work.
There is no DT node for "rdtime" so how should we probe the DM driver.
I think you can do some modification to create the S-mode timer driver based on the M-mode driver patch I provided.
In the riscv_timer_probe(), do not require syscon driver to be loaded, ie: not calling syscon_get_by_driver_data(). In the riscv_timer_get_count(), implement that with rdtime for S-mode.
This is similar to how U-Boot ARM64 uses CNTPCT_EL0 register.
We cannot use DM driver here.
The issue is that eventually we will migrate all U-Boot device drivers to DM, which means get_tbclk() and get_ticks() you implemented will be broken someday in the future.
Yes, I agree we should have DM driver whenever possible but sometime there is no DT bindings for things like timer because it's part of CPU instruction or CSRs
Regards, Bin
On Mon, Dec 3, 2018 at 12:56 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:19 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:32 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote:
When running in S-mode, we can use rdtime and rdtimeh instructions for reading timer ticks (just like Linux). The frequency of timer ticks is passed by prior booting stages in "timebase-frequency" DT property of the "/cpus" DT node.
This patch provides a generic timer implementation for U-Boot running in S-mode. For U-Boot running in M-mode, specific timer drivers will have to be provided.
Signed-off-by: Anup Patel anup@brainfault.org
arch/Kconfig | 1 - arch/riscv/Kconfig | 22 +++++++++++---- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/time.c
diff --git a/arch/Kconfig b/arch/Kconfig index 9fdd2f7e66..a4fcb3522d 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -72,7 +72,6 @@ config RISCV imply BLK imply CLK imply MTD
imply TIMERNo, please don't do this.
I am removing here but selecting it only for M-mode U-Boot.
imply CMD_DMconfig SANDBOX diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 732a357a99..20a060454b 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -44,6 +44,23 @@ config ARCH_RV64I
endchoice
+choice
prompt "Run Mode"default RISCV_MMODE+config RISCV_MMODE
bool "Machine"select TIMERhelpChoose this option to build U-Boot for RISC-V M-Mode.+config RISCV_SMODE
bool "Supervisor"helpChoose this option to build U-Boot for RISC-V S-Mode.+endchoice
The above changes deserve separate patch, or merge that in the 1st patch in the series.
Sure, I will put Kconfig changes as separate patch.
config RISCV_ISA_C bool "Emit compressed instructions" default y @@ -55,11 +72,6 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y
-config RISCV_SMODE
bool "Run in S-Mode"helpEnable this option to build U-Boot for RISC-V S-Modeconfig 32BIT bool
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index b58db89752..98aa6d40e7 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += interrupts.o obj-y += reset.o obj-y += setjmp.o +obj-$(CONFIG_RISCV_SMODE) += time.o
# For building EFI apps CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c new file mode 100644 index 0000000000..077e568ca6 --- /dev/null +++ b/arch/riscv/lib/time.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2018, Anup Patel anup@brainfault.org
- */
+#include <common.h> +#include <fdtdec.h>
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned int tbclk;
+static void setup_tbclk(void) +{
int cpus;if (!gd->fdt_blob || tbclk)return;cpus = fdt_path_offset(gd->fdt_blob, "/cpus");if (cpus < 0) {debug("%s: Missing /cpus node\n", __func__);return;}tbclk = fdtdec_get_int(gd->fdt_blob, cpus,"timebase-frequency", 1000000);+}
+ulong notrace get_tbclk(void) +{
setup_tbclk();return tbclk;+}
+#ifdef CONFIG_64BIT +uint64_t notrace get_ticks(void) +{
unsigned long n;__asm__ __volatile__ ("rdtime %0": "=r" (n));return n;+} +#else +uint64_t notrace get_ticks(void) +{
uint32_t lo, hi, tmp;__asm__ __volatile__ ("1:\n""rdtimeh %0\n""rdtime %1\n""rdtimeh %2\n""bne %0, %2, 1b": "=&r" (hi), "=&r" (lo), "=&r" (tmp));return ((uint64_t)hi << 32) | lo;+}
+#endif
The proper way to implement timer driver is via DM. Could you please implement a DM timer driver for S-mode?
The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI.
"rdtime" is an instruction. It's not an device for which we can implement DM driver.
Yes, I know. But that does not mean we cannot write a driver to do the paper work.
There is no DT node for "rdtime" so how should we probe the DM driver.
I think you can do some modification to create the S-mode timer driver based on the M-mode driver patch I provided.
I think you missed my point.
CLINT is SiFive specific. You have to rename your driver as clint_timer. The riscv_timer name is in-appropriate.
We cannot use CLINT udevice for "rdtime" because: 1. SOC can implement "rdtime" in HW 2. SOC can implement some MMIO device (like CLINT) and emulate "rdtime" in runtime firmware.
In the riscv_timer_probe(), do not require syscon driver to be loaded, ie: not calling syscon_get_by_driver_data(). In the riscv_timer_get_count(), implement that with rdtime for S-mode.
There is no udevice for "rdtime" or time CSRs.
CLINT is a SiFive specific device. Please rename your driver.
Regards, Anup
Hi Anup,
On Mon, Dec 3, 2018 at 3:31 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:56 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:19 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:32 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote: > > When running in S-mode, we can use rdtime and rdtimeh instructions > for reading timer ticks (just like Linux). The frequency of timer > ticks is passed by prior booting stages in "timebase-frequency" DT > property of the "/cpus" DT node. > > This patch provides a generic timer implementation for U-Boot > running in S-mode. For U-Boot running in M-mode, specific timer > drivers will have to be provided. > > Signed-off-by: Anup Patel anup@brainfault.org > --- > arch/Kconfig | 1 - > arch/riscv/Kconfig | 22 +++++++++++---- > arch/riscv/lib/Makefile | 1 + > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 78 insertions(+), 6 deletions(-) > create mode 100644 arch/riscv/lib/time.c > > diff --git a/arch/Kconfig b/arch/Kconfig > index 9fdd2f7e66..a4fcb3522d 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -72,7 +72,6 @@ config RISCV > imply BLK > imply CLK > imply MTD > - imply TIMER
No, please don't do this.
I am removing here but selecting it only for M-mode U-Boot.
> imply CMD_DM > > config SANDBOX > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 732a357a99..20a060454b 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -44,6 +44,23 @@ config ARCH_RV64I > > endchoice > > +choice > + prompt "Run Mode" > + default RISCV_MMODE > + > +config RISCV_MMODE > + bool "Machine" > + select TIMER > + help > + Choose this option to build U-Boot for RISC-V M-Mode. > + > +config RISCV_SMODE > + bool "Supervisor" > + help > + Choose this option to build U-Boot for RISC-V S-Mode. > + > +endchoice > +
The above changes deserve separate patch, or merge that in the 1st patch in the series.
Sure, I will put Kconfig changes as separate patch.
> config RISCV_ISA_C > bool "Emit compressed instructions" > default y > @@ -55,11 +72,6 @@ config RISCV_ISA_C > config RISCV_ISA_A > def_bool y > > -config RISCV_SMODE > - bool "Run in S-Mode" > - help > - Enable this option to build U-Boot for RISC-V S-Mode > - > config 32BIT > bool > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > index b58db89752..98aa6d40e7 100644 > --- a/arch/riscv/lib/Makefile > +++ b/arch/riscv/lib/Makefile > @@ -12,6 +12,7 @@ obj-y += cache.o > obj-y += interrupts.o > obj-y += reset.o > obj-y += setjmp.o > +obj-$(CONFIG_RISCV_SMODE) += time.o > > # For building EFI apps > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > new file mode 100644 > index 0000000000..077e568ca6 > --- /dev/null > +++ b/arch/riscv/lib/time.c > @@ -0,0 +1,60 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2018, Anup Patel anup@brainfault.org > + */ > + > +#include <common.h> > +#include <fdtdec.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +static unsigned int tbclk; > + > +static void setup_tbclk(void) > +{ > + int cpus; > + > + if (!gd->fdt_blob || tbclk) > + return; > + > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > + if (cpus < 0) { > + debug("%s: Missing /cpus node\n", __func__); > + return; > + } > + > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > + "timebase-frequency", 1000000); > +} > + > +ulong notrace get_tbclk(void) > +{ > + setup_tbclk(); > + > + return tbclk; > +} > + > +#ifdef CONFIG_64BIT > +uint64_t notrace get_ticks(void) > +{ > + unsigned long n; > + > + __asm__ __volatile__ ( > + "rdtime %0" > + : "=r" (n)); > + return n; > +} > +#else > +uint64_t notrace get_ticks(void) > +{ > + uint32_t lo, hi, tmp; > + __asm__ __volatile__ ( > + "1:\n" > + "rdtimeh %0\n" > + "rdtime %1\n" > + "rdtimeh %2\n" > + "bne %0, %2, 1b" > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > + return ((uint64_t)hi << 32) | lo; > +} > +#endif > --
The proper way to implement timer driver is via DM. Could you please implement a DM timer driver for S-mode?
The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI.
"rdtime" is an instruction. It's not an device for which we can implement DM driver.
Yes, I know. But that does not mean we cannot write a driver to do the paper work.
There is no DT node for "rdtime" so how should we probe the DM driver.
I think you can do some modification to create the S-mode timer driver based on the M-mode driver patch I provided.
I think you missed my point.
CLINT is SiFive specific. You have to rename your driver as clint_timer. The riscv_timer name is in-appropriate.
Yes, I am reconsidering this. Lukas had similar comments, and Rick confirmed that Andes's chip implemented different mtimer/IPI block from SiFive's.
We cannot use CLINT udevice for "rdtime" because:
- SOC can implement "rdtime" in HW
- SOC can implement some MMIO device (like CLINT)
and emulate "rdtime" in runtime firmware.
It looks you misunderstood things. I was not suggesting you use CLINT device for S-mode timer driver at all. I am not sure why you misunderstood it. Please read my comments carefully.
In the riscv_timer_probe(), do not require syscon driver to be loaded, ie: not calling syscon_get_by_driver_data(). In the riscv_timer_get_count(), implement that with rdtime for S-mode.
There is no udevice for "rdtime" or time CSRs.
CLINT is a SiFive specific device. Please rename your driver.
Regards, Bin
On Mon, Dec 3, 2018 at 1:08 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:31 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:56 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:19 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:32 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote: > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > for reading timer ticks (just like Linux). The frequency of timer > > ticks is passed by prior booting stages in "timebase-frequency" DT > > property of the "/cpus" DT node. > > > > This patch provides a generic timer implementation for U-Boot > > running in S-mode. For U-Boot running in M-mode, specific timer > > drivers will have to be provided. > > > > Signed-off-by: Anup Patel anup@brainfault.org > > --- > > arch/Kconfig | 1 - > > arch/riscv/Kconfig | 22 +++++++++++---- > > arch/riscv/lib/Makefile | 1 + > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 78 insertions(+), 6 deletions(-) > > create mode 100644 arch/riscv/lib/time.c > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > index 9fdd2f7e66..a4fcb3522d 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -72,7 +72,6 @@ config RISCV > > imply BLK > > imply CLK > > imply MTD > > - imply TIMER > > No, please don't do this.
I am removing here but selecting it only for M-mode U-Boot.
> > > imply CMD_DM > > > > config SANDBOX > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index 732a357a99..20a060454b 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > endchoice > > > > +choice > > + prompt "Run Mode" > > + default RISCV_MMODE > > + > > +config RISCV_MMODE > > + bool "Machine" > > + select TIMER > > + help > > + Choose this option to build U-Boot for RISC-V M-Mode. > > + > > +config RISCV_SMODE > > + bool "Supervisor" > > + help > > + Choose this option to build U-Boot for RISC-V S-Mode. > > + > > +endchoice > > + > > The above changes deserve separate patch, or merge that in the 1st > patch in the series.
Sure, I will put Kconfig changes as separate patch.
> > > config RISCV_ISA_C > > bool "Emit compressed instructions" > > default y > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > config RISCV_ISA_A > > def_bool y > > > > -config RISCV_SMODE > > - bool "Run in S-Mode" > > - help > > - Enable this option to build U-Boot for RISC-V S-Mode > > - > > config 32BIT > > bool > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > index b58db89752..98aa6d40e7 100644 > > --- a/arch/riscv/lib/Makefile > > +++ b/arch/riscv/lib/Makefile > > @@ -12,6 +12,7 @@ obj-y += cache.o > > obj-y += interrupts.o > > obj-y += reset.o > > obj-y += setjmp.o > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > # For building EFI apps > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > new file mode 100644 > > index 0000000000..077e568ca6 > > --- /dev/null > > +++ b/arch/riscv/lib/time.c > > @@ -0,0 +1,60 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (C) 2018, Anup Patel anup@brainfault.org > > + */ > > + > > +#include <common.h> > > +#include <fdtdec.h> > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +static unsigned int tbclk; > > + > > +static void setup_tbclk(void) > > +{ > > + int cpus; > > + > > + if (!gd->fdt_blob || tbclk) > > + return; > > + > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > + if (cpus < 0) { > > + debug("%s: Missing /cpus node\n", __func__); > > + return; > > + } > > + > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > + "timebase-frequency", 1000000); > > +} > > + > > +ulong notrace get_tbclk(void) > > +{ > > + setup_tbclk(); > > + > > + return tbclk; > > +} > > + > > +#ifdef CONFIG_64BIT > > +uint64_t notrace get_ticks(void) > > +{ > > + unsigned long n; > > + > > + __asm__ __volatile__ ( > > + "rdtime %0" > > + : "=r" (n)); > > + return n; > > +} > > +#else > > +uint64_t notrace get_ticks(void) > > +{ > > + uint32_t lo, hi, tmp; > > + __asm__ __volatile__ ( > > + "1:\n" > > + "rdtimeh %0\n" > > + "rdtime %1\n" > > + "rdtimeh %2\n" > > + "bne %0, %2, 1b" > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > + return ((uint64_t)hi << 32) | lo; > > +} > > +#endif > > -- > > The proper way to implement timer driver is via DM. Could you please > implement a DM timer driver for S-mode? > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI.
"rdtime" is an instruction. It's not an device for which we can implement DM driver.
Yes, I know. But that does not mean we cannot write a driver to do the paper work.
There is no DT node for "rdtime" so how should we probe the DM driver.
I think you can do some modification to create the S-mode timer driver based on the M-mode driver patch I provided.
I think you missed my point.
CLINT is SiFive specific. You have to rename your driver as clint_timer. The riscv_timer name is in-appropriate.
Yes, I am reconsidering this. Lukas had similar comments, and Rick confirmed that Andes's chip implemented different mtimer/IPI block from SiFive's.
We cannot use CLINT udevice for "rdtime" because:
- SOC can implement "rdtime" in HW
- SOC can implement some MMIO device (like CLINT)
and emulate "rdtime" in runtime firmware.
It looks you misunderstood things. I was not suggesting you use CLINT device for S-mode timer driver at all. I am not sure why you misunderstood it. Please read my comments carefully.
Can you explain how will the timer probed called if there is no matching device in DT?
Regards, Anup
Hi Anup,
On Mon, Dec 3, 2018 at 3:44 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 1:08 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:31 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:56 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:19 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:32 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote: > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote: > > > > Hi Anup, > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote: > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > for reading timer ticks (just like Linux). The frequency of timer > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > property of the "/cpus" DT node. > > > > > > This patch provides a generic timer implementation for U-Boot > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > drivers will have to be provided. > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > --- > > > arch/Kconfig | 1 - > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > arch/riscv/lib/Makefile | 1 + > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > create mode 100644 arch/riscv/lib/time.c > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > --- a/arch/Kconfig > > > +++ b/arch/Kconfig > > > @@ -72,7 +72,6 @@ config RISCV > > > imply BLK > > > imply CLK > > > imply MTD > > > - imply TIMER > > > > No, please don't do this. > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > imply CMD_DM > > > > > > config SANDBOX > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > index 732a357a99..20a060454b 100644 > > > --- a/arch/riscv/Kconfig > > > +++ b/arch/riscv/Kconfig > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > endchoice > > > > > > +choice > > > + prompt "Run Mode" > > > + default RISCV_MMODE > > > + > > > +config RISCV_MMODE > > > + bool "Machine" > > > + select TIMER > > > + help > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > + > > > +config RISCV_SMODE > > > + bool "Supervisor" > > > + help > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > + > > > +endchoice > > > + > > > > The above changes deserve separate patch, or merge that in the 1st > > patch in the series. > > Sure, I will put Kconfig changes as separate patch. > > > > > > config RISCV_ISA_C > > > bool "Emit compressed instructions" > > > default y > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > config RISCV_ISA_A > > > def_bool y > > > > > > -config RISCV_SMODE > > > - bool "Run in S-Mode" > > > - help > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > - > > > config 32BIT > > > bool > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > index b58db89752..98aa6d40e7 100644 > > > --- a/arch/riscv/lib/Makefile > > > +++ b/arch/riscv/lib/Makefile > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > obj-y += interrupts.o > > > obj-y += reset.o > > > obj-y += setjmp.o > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > # For building EFI apps > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > new file mode 100644 > > > index 0000000000..077e568ca6 > > > --- /dev/null > > > +++ b/arch/riscv/lib/time.c > > > @@ -0,0 +1,60 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Copyright (C) 2018, Anup Patel anup@brainfault.org > > > + */ > > > + > > > +#include <common.h> > > > +#include <fdtdec.h> > > > + > > > +DECLARE_GLOBAL_DATA_PTR; > > > + > > > +static unsigned int tbclk; > > > + > > > +static void setup_tbclk(void) > > > +{ > > > + int cpus; > > > + > > > + if (!gd->fdt_blob || tbclk) > > > + return; > > > + > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > + if (cpus < 0) { > > > + debug("%s: Missing /cpus node\n", __func__); > > > + return; > > > + } > > > + > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > + "timebase-frequency", 1000000); > > > +} > > > + > > > +ulong notrace get_tbclk(void) > > > +{ > > > + setup_tbclk(); > > > + > > > + return tbclk; > > > +} > > > + > > > +#ifdef CONFIG_64BIT > > > +uint64_t notrace get_ticks(void) > > > +{ > > > + unsigned long n; > > > + > > > + __asm__ __volatile__ ( > > > + "rdtime %0" > > > + : "=r" (n)); > > > + return n; > > > +} > > > +#else > > > +uint64_t notrace get_ticks(void) > > > +{ > > > + uint32_t lo, hi, tmp; > > > + __asm__ __volatile__ ( > > > + "1:\n" > > > + "rdtimeh %0\n" > > > + "rdtime %1\n" > > > + "rdtimeh %2\n" > > > + "bne %0, %2, 1b" > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > + return ((uint64_t)hi << 32) | lo; > > > +} > > > +#endif > > > -- > > > > The proper way to implement timer driver is via DM. Could you please > > implement a DM timer driver for S-mode? > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > "rdtime" is an instruction. It's not an device for which we can > implement DM driver. >
Yes, I know. But that does not mean we cannot write a driver to do the paper work.
There is no DT node for "rdtime" so how should we probe the DM driver.
I think you can do some modification to create the S-mode timer driver based on the M-mode driver patch I provided.
I think you missed my point.
CLINT is SiFive specific. You have to rename your driver as clint_timer. The riscv_timer name is in-appropriate.
Yes, I am reconsidering this. Lukas had similar comments, and Rick confirmed that Andes's chip implemented different mtimer/IPI block from SiFive's.
We cannot use CLINT udevice for "rdtime" because:
- SOC can implement "rdtime" in HW
- SOC can implement some MMIO device (like CLINT)
and emulate "rdtime" in runtime firmware.
It looks you misunderstood things. I was not suggesting you use CLINT device for S-mode timer driver at all. I am not sure why you misunderstood it. Please read my comments carefully.
Can you explain how will the timer probed called if there is no matching device in DT?
The timer driver is bound by the CPU driver. See http://patchwork.ozlabs.org/patch/996902/.
Regards, Bin
On Mon, Dec 3, 2018 at 1:27 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:44 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 1:08 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:31 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:56 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:19 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:32 PM Bin Meng bmeng.cn@gmail.com wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote: > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote: > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > property of the "/cpus" DT node. > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > drivers will have to be provided. > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > --- > > > > arch/Kconfig | 1 - > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > arch/riscv/lib/Makefile | 1 + > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > --- a/arch/Kconfig > > > > +++ b/arch/Kconfig > > > > @@ -72,7 +72,6 @@ config RISCV > > > > imply BLK > > > > imply CLK > > > > imply MTD > > > > - imply TIMER > > > > > > No, please don't do this. > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > imply CMD_DM > > > > > > > > config SANDBOX > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > index 732a357a99..20a060454b 100644 > > > > --- a/arch/riscv/Kconfig > > > > +++ b/arch/riscv/Kconfig > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > endchoice > > > > > > > > +choice > > > > + prompt "Run Mode" > > > > + default RISCV_MMODE > > > > + > > > > +config RISCV_MMODE > > > > + bool "Machine" > > > > + select TIMER > > > > + help > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > + > > > > +config RISCV_SMODE > > > > + bool "Supervisor" > > > > + help > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > + > > > > +endchoice > > > > + > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > patch in the series. > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > config RISCV_ISA_C > > > > bool "Emit compressed instructions" > > > > default y > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > config RISCV_ISA_A > > > > def_bool y > > > > > > > > -config RISCV_SMODE > > > > - bool "Run in S-Mode" > > > > - help > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > - > > > > config 32BIT > > > > bool > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > index b58db89752..98aa6d40e7 100644 > > > > --- a/arch/riscv/lib/Makefile > > > > +++ b/arch/riscv/lib/Makefile > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > obj-y += interrupts.o > > > > obj-y += reset.o > > > > obj-y += setjmp.o > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > # For building EFI apps > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > new file mode 100644 > > > > index 0000000000..077e568ca6 > > > > --- /dev/null > > > > +++ b/arch/riscv/lib/time.c > > > > @@ -0,0 +1,60 @@ > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > +/* > > > > + * Copyright (C) 2018, Anup Patel anup@brainfault.org > > > > + */ > > > > + > > > > +#include <common.h> > > > > +#include <fdtdec.h> > > > > + > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > + > > > > +static unsigned int tbclk; > > > > + > > > > +static void setup_tbclk(void) > > > > +{ > > > > + int cpus; > > > > + > > > > + if (!gd->fdt_blob || tbclk) > > > > + return; > > > > + > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > + if (cpus < 0) { > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > + return; > > > > + } > > > > + > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > + "timebase-frequency", 1000000); > > > > +} > > > > + > > > > +ulong notrace get_tbclk(void) > > > > +{ > > > > + setup_tbclk(); > > > > + > > > > + return tbclk; > > > > +} > > > > + > > > > +#ifdef CONFIG_64BIT > > > > +uint64_t notrace get_ticks(void) > > > > +{ > > > > + unsigned long n; > > > > + > > > > + __asm__ __volatile__ ( > > > > + "rdtime %0" > > > > + : "=r" (n)); > > > > + return n; > > > > +} > > > > +#else > > > > +uint64_t notrace get_ticks(void) > > > > +{ > > > > + uint32_t lo, hi, tmp; > > > > + __asm__ __volatile__ ( > > > > + "1:\n" > > > > + "rdtimeh %0\n" > > > > + "rdtime %1\n" > > > > + "rdtimeh %2\n" > > > > + "bne %0, %2, 1b" > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > + return ((uint64_t)hi << 32) | lo; > > > > +} > > > > +#endif > > > > -- > > > > > > The proper way to implement timer driver is via DM. Could you please > > > implement a DM timer driver for S-mode? > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > "rdtime" is an instruction. It's not an device for which we can > > implement DM driver. > > > > Yes, I know. But that does not mean we cannot write a driver to do the > paper work.
There is no DT node for "rdtime" so how should we probe the DM driver.
I think you can do some modification to create the S-mode timer driver based on the M-mode driver patch I provided.
I think you missed my point.
CLINT is SiFive specific. You have to rename your driver as clint_timer. The riscv_timer name is in-appropriate.
Yes, I am reconsidering this. Lukas had similar comments, and Rick confirmed that Andes's chip implemented different mtimer/IPI block from SiFive's.
We cannot use CLINT udevice for "rdtime" because:
- SOC can implement "rdtime" in HW
- SOC can implement some MMIO device (like CLINT)
and emulate "rdtime" in runtime firmware.
It looks you misunderstood things. I was not suggesting you use CLINT device for S-mode timer driver at all. I am not sure why you misunderstood it. Please read my comments carefully.
Can you explain how will the timer probed called if there is no matching device in DT?
The timer driver is bound by the CPU driver. See http://patchwork.ozlabs.org/patch/996902/.
Makes sense now. There is no DT based probing. Instead, you are explicitly bind the timer driver.
M-mode U-Boot will never use the generic riscv_timer based on "rdtime" instruction. We will mostly have SoC specific timer drivers for M-mode U-Boot.
I think your CPU driver should only do the explicit device binding if CONFIG_RISCV_SMODE=y.
There is inter-dependency here.
My suggestion is, I can include you CPU driver patch and add DM driver for "rdtime" based upon that. Only If you are fine ??
Regards, Anup
Hi Anup,
On Mon, Dec 3, 2018 at 4:12 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 1:27 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:44 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 1:08 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:31 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:56 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:19 PM Anup Patel anup@brainfault.org wrote: > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng bmeng.cn@gmail.com wrote: > > > > Hi Anup, > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote: > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote: > > > > > > > > Hi Anup, > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote: > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > property of the "/cpus" DT node. > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > drivers will have to be provided. > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > --- > > > > > arch/Kconfig | 1 - > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > arch/riscv/lib/Makefile | 1 + > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > --- a/arch/Kconfig > > > > > +++ b/arch/Kconfig > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > imply BLK > > > > > imply CLK > > > > > imply MTD > > > > > - imply TIMER > > > > > > > > No, please don't do this. > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > config SANDBOX > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > index 732a357a99..20a060454b 100644 > > > > > --- a/arch/riscv/Kconfig > > > > > +++ b/arch/riscv/Kconfig > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > endchoice > > > > > > > > > > +choice > > > > > + prompt "Run Mode" > > > > > + default RISCV_MMODE > > > > > + > > > > > +config RISCV_MMODE > > > > > + bool "Machine" > > > > > + select TIMER > > > > > + help > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > + > > > > > +config RISCV_SMODE > > > > > + bool "Supervisor" > > > > > + help > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > + > > > > > +endchoice > > > > > + > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > patch in the series. > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > config RISCV_ISA_C > > > > > bool "Emit compressed instructions" > > > > > default y > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > config RISCV_ISA_A > > > > > def_bool y > > > > > > > > > > -config RISCV_SMODE > > > > > - bool "Run in S-Mode" > > > > > - help > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > - > > > > > config 32BIT > > > > > bool > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > index b58db89752..98aa6d40e7 100644 > > > > > --- a/arch/riscv/lib/Makefile > > > > > +++ b/arch/riscv/lib/Makefile > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > obj-y += interrupts.o > > > > > obj-y += reset.o > > > > > obj-y += setjmp.o > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > # For building EFI apps > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > new file mode 100644 > > > > > index 0000000000..077e568ca6 > > > > > --- /dev/null > > > > > +++ b/arch/riscv/lib/time.c > > > > > @@ -0,0 +1,60 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > +/* > > > > > + * Copyright (C) 2018, Anup Patel anup@brainfault.org > > > > > + */ > > > > > + > > > > > +#include <common.h> > > > > > +#include <fdtdec.h> > > > > > + > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > + > > > > > +static unsigned int tbclk; > > > > > + > > > > > +static void setup_tbclk(void) > > > > > +{ > > > > > + int cpus; > > > > > + > > > > > + if (!gd->fdt_blob || tbclk) > > > > > + return; > > > > > + > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > + if (cpus < 0) { > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > + return; > > > > > + } > > > > > + > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > + "timebase-frequency", 1000000); > > > > > +} > > > > > + > > > > > +ulong notrace get_tbclk(void) > > > > > +{ > > > > > + setup_tbclk(); > > > > > + > > > > > + return tbclk; > > > > > +} > > > > > + > > > > > +#ifdef CONFIG_64BIT > > > > > +uint64_t notrace get_ticks(void) > > > > > +{ > > > > > + unsigned long n; > > > > > + > > > > > + __asm__ __volatile__ ( > > > > > + "rdtime %0" > > > > > + : "=r" (n)); > > > > > + return n; > > > > > +} > > > > > +#else > > > > > +uint64_t notrace get_ticks(void) > > > > > +{ > > > > > + uint32_t lo, hi, tmp; > > > > > + __asm__ __volatile__ ( > > > > > + "1:\n" > > > > > + "rdtimeh %0\n" > > > > > + "rdtime %1\n" > > > > > + "rdtimeh %2\n" > > > > > + "bne %0, %2, 1b" > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > +} > > > > > +#endif > > > > > -- > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > implement a DM timer driver for S-mode? > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > implement DM driver. > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > paper work. > > There is no DT node for "rdtime" so how should we probe the DM driver. >
I think you can do some modification to create the S-mode timer driver based on the M-mode driver patch I provided.
I think you missed my point.
CLINT is SiFive specific. You have to rename your driver as clint_timer. The riscv_timer name is in-appropriate.
Yes, I am reconsidering this. Lukas had similar comments, and Rick confirmed that Andes's chip implemented different mtimer/IPI block from SiFive's.
We cannot use CLINT udevice for "rdtime" because:
- SOC can implement "rdtime" in HW
- SOC can implement some MMIO device (like CLINT)
and emulate "rdtime" in runtime firmware.
It looks you misunderstood things. I was not suggesting you use CLINT device for S-mode timer driver at all. I am not sure why you misunderstood it. Please read my comments carefully.
Can you explain how will the timer probed called if there is no matching device in DT?
The timer driver is bound by the CPU driver. See http://patchwork.ozlabs.org/patch/996902/.
Makes sense now. There is no DT based probing. Instead, you are explicitly bind the timer driver.
M-mode U-Boot will never use the generic riscv_timer based on "rdtime" instruction. We will mostly have SoC specific timer drivers for M-mode U-Boot.
Unfortunately yes, M-mode timer driver cannot be generic and that's what bothered me. It should have been architected clearly in the very beginning since the timer is supposed to deliver interrupts directly into [M|S]IP on the hart and allowing platform implementation to me is creating fragmented solutions. If there are other timers available on SoC device, I will definitely use them instead.
I think your CPU driver should only do the explicit device binding if CONFIG_RISCV_SMODE=y.
Yep I haven't figured out a clean way to do the driver binding if there are different mtimer implementations, as the mtimer driver name has to be made known to the CPU driver. But I think the S-mode driver can be generic.
There is inter-dependency here.
My suggestion is, I can include you CPU driver patch and add DM driver for "rdtime" based upon that. Only If you are fine ??
I am fine with that. However there are some review comments that need be addressed in my v1 CPU/timer driver series so this should be fixed at first. Maybe we can just delay the S-mode timer driver support until later, after my series goes in.
Regards, Bin
On Mon, Dec 3, 2018 at 2:16 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 4:12 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 1:27 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:44 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 1:08 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:31 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 12:56 PM Bin Meng bmeng.cn@gmail.com wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel anup@brainfault.org wrote: > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng bmeng.cn@gmail.com wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel anup@brainfault.org wrote: > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng bmeng.cn@gmail.com wrote: > > > > > > > > > > Hi Anup, > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel anup@brainfault.org wrote: > > > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > > drivers will have to be provided. > > > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > --- > > > > > > arch/Kconfig | 1 - > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > --- a/arch/Kconfig > > > > > > +++ b/arch/Kconfig > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > imply BLK > > > > > > imply CLK > > > > > > imply MTD > > > > > > - imply TIMER > > > > > > > > > > No, please don't do this. > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > config SANDBOX > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > index 732a357a99..20a060454b 100644 > > > > > > --- a/arch/riscv/Kconfig > > > > > > +++ b/arch/riscv/Kconfig > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > endchoice > > > > > > > > > > > > +choice > > > > > > + prompt "Run Mode" > > > > > > + default RISCV_MMODE > > > > > > + > > > > > > +config RISCV_MMODE > > > > > > + bool "Machine" > > > > > > + select TIMER > > > > > > + help > > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > > + > > > > > > +config RISCV_SMODE > > > > > > + bool "Supervisor" > > > > > > + help > > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > > + > > > > > > +endchoice > > > > > > + > > > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > > patch in the series. > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > bool "Emit compressed instructions" > > > > > > default y > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > config RISCV_ISA_A > > > > > > def_bool y > > > > > > > > > > > > -config RISCV_SMODE > > > > > > - bool "Run in S-Mode" > > > > > > - help > > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > > - > > > > > > config 32BIT > > > > > > bool > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > obj-y += interrupts.o > > > > > > obj-y += reset.o > > > > > > obj-y += setjmp.o > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > # For building EFI apps > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > new file mode 100644 > > > > > > index 0000000000..077e568ca6 > > > > > > --- /dev/null > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > @@ -0,0 +1,60 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > +/* > > > > > > + * Copyright (C) 2018, Anup Patel anup@brainfault.org > > > > > > + */ > > > > > > + > > > > > > +#include <common.h> > > > > > > +#include <fdtdec.h> > > > > > > + > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > + > > > > > > +static unsigned int tbclk; > > > > > > + > > > > > > +static void setup_tbclk(void) > > > > > > +{ > > > > > > + int cpus; > > > > > > + > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > + return; > > > > > > + > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > + if (cpus < 0) { > > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > > + return; > > > > > > + } > > > > > > + > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > + "timebase-frequency", 1000000); > > > > > > +} > > > > > > + > > > > > > +ulong notrace get_tbclk(void) > > > > > > +{ > > > > > > + setup_tbclk(); > > > > > > + > > > > > > + return tbclk; > > > > > > +} > > > > > > + > > > > > > +#ifdef CONFIG_64BIT > > > > > > +uint64_t notrace get_ticks(void) > > > > > > +{ > > > > > > + unsigned long n; > > > > > > + > > > > > > + __asm__ __volatile__ ( > > > > > > + "rdtime %0" > > > > > > + : "=r" (n)); > > > > > > + return n; > > > > > > +} > > > > > > +#else > > > > > > +uint64_t notrace get_ticks(void) > > > > > > +{ > > > > > > + uint32_t lo, hi, tmp; > > > > > > + __asm__ __volatile__ ( > > > > > > + "1:\n" > > > > > > + "rdtimeh %0\n" > > > > > > + "rdtime %1\n" > > > > > > + "rdtimeh %2\n" > > > > > > + "bne %0, %2, 1b" > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > +} > > > > > > +#endif > > > > > > -- > > > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > > implement DM driver. > > > > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > > paper work. > > > > There is no DT node for "rdtime" so how should we probe the DM driver. > > > > I think you can do some modification to create the S-mode timer driver > based on the M-mode driver patch I provided.
I think you missed my point.
CLINT is SiFive specific. You have to rename your driver as clint_timer. The riscv_timer name is in-appropriate.
Yes, I am reconsidering this. Lukas had similar comments, and Rick confirmed that Andes's chip implemented different mtimer/IPI block from SiFive's.
We cannot use CLINT udevice for "rdtime" because:
- SOC can implement "rdtime" in HW
- SOC can implement some MMIO device (like CLINT)
and emulate "rdtime" in runtime firmware.
It looks you misunderstood things. I was not suggesting you use CLINT device for S-mode timer driver at all. I am not sure why you misunderstood it. Please read my comments carefully.
Can you explain how will the timer probed called if there is no matching device in DT?
The timer driver is bound by the CPU driver. See http://patchwork.ozlabs.org/patch/996902/.
Makes sense now. There is no DT based probing. Instead, you are explicitly bind the timer driver.
M-mode U-Boot will never use the generic riscv_timer based on "rdtime" instruction. We will mostly have SoC specific timer drivers for M-mode U-Boot.
Unfortunately yes, M-mode timer driver cannot be generic and that's what bothered me. It should have been architected clearly in the very beginning since the timer is supposed to deliver interrupts directly into [M|S]IP on the hart and allowing platform implementation to me is creating fragmented solutions. If there are other timers available on SoC device, I will definitely use them instead.
Even, I think timer CSRs should be clearly defined and accessible from both M and S modes.
I think your CPU driver should only do the explicit device binding if CONFIG_RISCV_SMODE=y.
Yep I haven't figured out a clean way to do the driver binding if there are different mtimer implementations, as the mtimer driver name has to be made known to the CPU driver. But I think the S-mode driver can be generic.
There is inter-dependency here.
My suggestion is, I can include you CPU driver patch and add DM driver for "rdtime" based upon that. Only If you are fine ??
I am fine with that. However there are some review comments that need be addressed in my v1 CPU/timer driver series so this should be fixed at first. Maybe we can just delay the S-mode timer driver support until later, after my series goes in.
I think first three patches of this series can go in without S-mode timer patch.
Regards, Anup
Hi Bin,
On Mon, 2018-12-03 at 16:45 +0800, Bin Meng wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 4:12 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 1:27 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:44 PM Anup Patel anup@brainfault.org wrote:
On Mon, Dec 3, 2018 at 1:08 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Mon, Dec 3, 2018 at 3:31 PM Anup Patel < anup@brainfault.org> wrote:
On Mon, Dec 3, 2018 at 12:56 PM Bin Meng < bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel < > anup@brainfault.org> wrote: > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng < > > bmeng.cn@gmail.com> wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel < > > > anup@brainfault.org> wrote: > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng < > > > > bmeng.cn@gmail.com> wrote: > > > > > > > > > > Hi Anup, > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel < > > > > > anup@brainfault.org> wrote: > > > > > > > > > > > > When running in S-mode, we can use rdtime and > > > > > > rdtimeh instructions > > > > > > for reading timer ticks (just like Linux). The > > > > > > frequency of timer > > > > > > ticks is passed by prior booting stages in > > > > > > "timebase-frequency" DT > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > This patch provides a generic timer > > > > > > implementation for U-Boot > > > > > > running in S-mode. For U-Boot running in M- > > > > > > mode, specific timer > > > > > > drivers will have to be provided. > > > > > > > > > > > > Signed-off-by: Anup Patel anup@brainfault.org > > > > > > --- > > > > > > arch/Kconfig | 1 - > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > arch/riscv/lib/time.c | 60 > > > > > > +++++++++++++++++++++++++++++++++++++++++ > > > > > > 4 files changed, 78 insertions(+), 6 > > > > > > deletions(-) > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > --- a/arch/Kconfig > > > > > > +++ b/arch/Kconfig > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > imply BLK > > > > > > imply CLK > > > > > > imply MTD > > > > > > - imply TIMER > > > > > > > > > > No, please don't do this. > > > > > > > > I am removing here but selecting it only for M-mode > > > > U-Boot. > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > config SANDBOX > > > > > > diff --git a/arch/riscv/Kconfig > > > > > > b/arch/riscv/Kconfig > > > > > > index 732a357a99..20a060454b 100644 > > > > > > --- a/arch/riscv/Kconfig > > > > > > +++ b/arch/riscv/Kconfig > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > endchoice > > > > > > > > > > > > +choice > > > > > > + prompt "Run Mode" > > > > > > + default RISCV_MMODE > > > > > > + > > > > > > +config RISCV_MMODE > > > > > > + bool "Machine" > > > > > > + select TIMER > > > > > > + help > > > > > > + Choose this option to build U-Boot > > > > > > for RISC-V M-Mode. > > > > > > + > > > > > > +config RISCV_SMODE > > > > > > + bool "Supervisor" > > > > > > + help > > > > > > + Choose this option to build U-Boot > > > > > > for RISC-V S-Mode. > > > > > > + > > > > > > +endchoice > > > > > > + > > > > > > > > > > The above changes deserve separate patch, or > > > > > merge that in the 1st > > > > > patch in the series. > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > bool "Emit compressed instructions" > > > > > > default y > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > config RISCV_ISA_A > > > > > > def_bool y > > > > > > > > > > > > -config RISCV_SMODE > > > > > > - bool "Run in S-Mode" > > > > > > - help > > > > > > - Enable this option to build U-Boot > > > > > > for RISC-V S-Mode > > > > > > - > > > > > > config 32BIT > > > > > > bool > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile > > > > > > b/arch/riscv/lib/Makefile > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > obj-y += interrupts.o > > > > > > obj-y += reset.o > > > > > > obj-y += setjmp.o > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > # For building EFI apps > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > diff --git a/arch/riscv/lib/time.c > > > > > > b/arch/riscv/lib/time.c > > > > > > new file mode 100644 > > > > > > index 0000000000..077e568ca6 > > > > > > --- /dev/null > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > @@ -0,0 +1,60 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > +/* > > > > > > + * Copyright (C) 2018, Anup Patel < > > > > > > anup@brainfault.org> > > > > > > + */ > > > > > > + > > > > > > +#include <common.h> > > > > > > +#include <fdtdec.h> > > > > > > + > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > + > > > > > > +static unsigned int tbclk; > > > > > > + > > > > > > +static void setup_tbclk(void) > > > > > > +{ > > > > > > + int cpus; > > > > > > + > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > + return; > > > > > > + > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, > > > > > > "/cpus"); > > > > > > + if (cpus < 0) { > > > > > > + debug("%s: Missing /cpus > > > > > > node\n", __func__); > > > > > > + return; > > > > > > + } > > > > > > + > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, > > > > > > cpus, > > > > > > + "timebase- > > > > > > frequency", 1000000); > > > > > > +} > > > > > > + > > > > > > +ulong notrace get_tbclk(void) > > > > > > +{ > > > > > > + setup_tbclk(); > > > > > > + > > > > > > + return tbclk; > > > > > > +} > > > > > > + > > > > > > +#ifdef CONFIG_64BIT > > > > > > +uint64_t notrace get_ticks(void) > > > > > > +{ > > > > > > + unsigned long n; > > > > > > + > > > > > > + __asm__ __volatile__ ( > > > > > > + "rdtime %0" > > > > > > + : "=r" (n)); > > > > > > + return n; > > > > > > +} > > > > > > +#else > > > > > > +uint64_t notrace get_ticks(void) > > > > > > +{ > > > > > > + uint32_t lo, hi, tmp; > > > > > > + __asm__ __volatile__ ( > > > > > > + "1:\n" > > > > > > + "rdtimeh %0\n" > > > > > > + "rdtime %1\n" > > > > > > + "rdtimeh %2\n" > > > > > > + "bne %0, %2, 1b" > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" > > > > > > (tmp)); > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > +} > > > > > > +#endif > > > > > > -- > > > > > > > > > > The proper way to implement timer driver is via > > > > > DM. Could you please > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > The M-mode timer driver is @ > > > > > http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > "rdtime" is an instruction. It's not an device for > > > > which we can > > > > implement DM driver. > > > > > > > > > > Yes, I know. But that does not mean we cannot write a > > > driver to do the > > > paper work. > > > > There is no DT node for "rdtime" so how should we probe > > the DM driver. > > > > I think you can do some modification to create the S-mode > timer driver > based on the M-mode driver patch I provided.
I think you missed my point.
CLINT is SiFive specific. You have to rename your driver as clint_timer. The riscv_timer name is in-appropriate.
Yes, I am reconsidering this. Lukas had similar comments, and Rick confirmed that Andes's chip implemented different mtimer/IPI block from SiFive's.
We cannot use CLINT udevice for "rdtime" because:
- SOC can implement "rdtime" in HW
- SOC can implement some MMIO device (like CLINT)
and emulate "rdtime" in runtime firmware.
It looks you misunderstood things. I was not suggesting you use CLINT device for S-mode timer driver at all. I am not sure why you misunderstood it. Please read my comments carefully.
Can you explain how will the timer probed called if there is no matching device in DT?
The timer driver is bound by the CPU driver. See http://patchwork.ozlabs.org/patch/996902/.
Makes sense now. There is no DT based probing. Instead, you are explicitly bind the timer driver.
M-mode U-Boot will never use the generic riscv_timer based on "rdtime" instruction. We will mostly have SoC specific timer drivers for M-mode U-Boot.
Unfortunately yes, M-mode timer driver cannot be generic and that's what bothered me. It should have been architected clearly in the very beginning since the timer is supposed to deliver interrupts directly into [M|S]IP on the hart and allowing platform implementation to me is creating fragmented solutions. If there are other timers available on SoC device, I will definitely use them instead.
I think your CPU driver should only do the explicit device binding if CONFIG_RISCV_SMODE=y.
Yep I haven't figured out a clean way to do the driver binding if there are different mtimer implementations, as the mtimer driver name has to be made known to the CPU driver. But I think the S-mode driver can be generic.
How about something like the driver misc/tegra_car.c? So one misc driver for riscv,clint0, which explicitly binds the M-mode timer and the syscon driver.
Thanks, Lukas
There is inter-dependency here.
My suggestion is, I can include you CPU driver patch and add DM driver for "rdtime" based upon that. Only If you are fine ??
I am fine with that. However there are some review comments that need be addressed in my v1 CPU/timer driver series so this should be fixed at first. Maybe we can just delay the S-mode timer driver support until later, after my series goes in.
Regards, Bin
participants (3)
-
Anup Patel -
Auer, Lukas -
Bin Meng