* [PATCH 0/5] riscv: enable SBI system reset
@ 2021-03-04 17:00 Heinrich Schuchardt
2021-03-04 17:00 ` [PATCH 1/5] risv: add missing SBI extension definitions Heinrich Schuchardt
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Heinrich Schuchardt @ 2021-03-04 17:00 UTC (permalink / raw)
To: u-boot
The purpose of this series is to provide the UEFI ResetSystem() service at
runtime on RISC-V systems.
With SBI v0.3 a system reset extension is available. This allows to
implement reboot and poweroff in U-Boot in a system independent way.
OpenSBI already provides an implementation of the extension for the
Kendryte K210 and other platforms.
* Provide a system reset driver using the system reset extension.
* Enable the driver on the MAIX board.
* Change the sbi command to use contants instead of raw numbers where
applicable.
Heinrich Schuchardt (5):
risv: add missing SBI extension definitions
cmd/sbi: use constants instead of numerical values
sysreset: provide SBI based sysreset driver
pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON
maix: enable SBI system reset for MAIX
MAINTAINERS | 1 +
arch/riscv/include/asm/sbi.h | 37 +++++++++-
arch/riscv/lib/sbi.c | 21 ++++--
board/sipeed/maix/maix.c | 4 ++
cmd/riscv/sbi.c | 30 ++++----
configs/sipeed_maix_smode_defconfig | 2 +
drivers/pinctrl/Kconfig | 2 +
drivers/sysreset/Kconfig | 7 ++
drivers/sysreset/Makefile | 1 +
drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++
lib/efi_loader/Kconfig | 2 +-
11 files changed, 187 insertions(+), 22 deletions(-)
create mode 100644 drivers/sysreset/sysreset_sbi.c
--
2.30.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/5] risv: add missing SBI extension definitions
2021-03-04 17:00 [PATCH 0/5] riscv: enable SBI system reset Heinrich Schuchardt
@ 2021-03-04 17:00 ` Heinrich Schuchardt
2021-03-04 23:28 ` Sean Anderson
2021-03-06 5:59 ` Bin Meng
2021-03-04 17:00 ` [PATCH 2/5] cmd/sbi: use constants instead of numerical values Heinrich Schuchardt
` (3 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Heinrich Schuchardt @ 2021-03-04 17:00 UTC (permalink / raw)
To: u-boot
Add the System Reset Extension and the Hart State Management Extension
definitions.
Add missing RFENCE Extension enum values.
The SBI 0.1 extension constants are needed for for the sbi command. Remove
an #ifdef.
Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 53ca316180..c598bb11ce 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -12,7 +12,6 @@
#include <linux/types.h>
enum sbi_ext_id {
-#ifdef CONFIG_SBI_V01
SBI_EXT_0_1_SET_TIMER = 0x0,
SBI_EXT_0_1_CONSOLE_PUTCHAR = 0x1,
SBI_EXT_0_1_CONSOLE_GETCHAR = 0x2,
@@ -22,11 +21,12 @@ enum sbi_ext_id {
SBI_EXT_0_1_REMOTE_SFENCE_VMA = 0x6,
SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID = 0x7,
SBI_EXT_0_1_SHUTDOWN = 0x8,
-#endif
SBI_EXT_BASE = 0x10,
SBI_EXT_TIME = 0x54494D45,
SBI_EXT_IPI = 0x735049,
SBI_EXT_RFENCE = 0x52464E43,
+ SBI_EXT_HSM = 0x48534D,
+ SBI_EXT_SRST = 0x53525354,
};
enum sbi_ext_base_fid {
@@ -51,6 +51,38 @@ enum sbi_ext_rfence_fid {
SBI_EXT_RFENCE_REMOTE_FENCE_I = 0,
SBI_EXT_RFENCE_REMOTE_SFENCE_VMA,
SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID,
+ SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA,
+ SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID,
+ SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA,
+ SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID,
+};
+
+enum sbi_ext_hsm_fid {
+ SBI_EXT_HSM_HART_START = 0,
+ SBI_EXT_HSM_HART_STOP,
+ SBI_EXT_HSM_HART_STATUS,
+};
+
+enum sbi_hsm_hart_status {
+ SBI_HSM_HART_STATUS_STARTED = 0,
+ SBI_HSM_HART_STATUS_STOPPED,
+ SBI_HSM_HART_STATUS_START_PENDING,
+ SBI_HSM_HART_STATUS_STOP_PENDING,
+};
+
+enum sbi_ext_srst_fid {
+ SBI_EXT_SRST_RESET = 0,
+};
+
+enum sbi_srst_reset_type {
+ SBI_SRST_RESET_TYPE_SHUTDOWN = 0,
+ SBI_SRST_RESET_TYPE_COLD_REBOOT,
+ SBI_SRST_RESET_TYPE_WARM_REBOOT,
+};
+
+enum sbi_srst_reset_reason {
+ SBI_SRST_RESET_REASON_NONE = 0,
+ SBI_SRST_RESET_REASON_SYS_FAILURE,
};
#ifdef CONFIG_SBI_V01
--
2.30.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/5] cmd/sbi: use constants instead of numerical values
2021-03-04 17:00 [PATCH 0/5] riscv: enable SBI system reset Heinrich Schuchardt
2021-03-04 17:00 ` [PATCH 1/5] risv: add missing SBI extension definitions Heinrich Schuchardt
@ 2021-03-04 17:00 ` Heinrich Schuchardt
2021-03-04 23:31 ` Sean Anderson
2021-03-08 6:25 ` Leo Liang
2021-03-04 17:00 ` [PATCH 3/5] sysreset: provide SBI based sysreset driver Heinrich Schuchardt
` (2 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Heinrich Schuchardt @ 2021-03-04 17:00 UTC (permalink / raw)
To: u-boot
Use constants for extension IDs.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
cmd/riscv/sbi.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
index 90c0811e14..65a2c93290 100644
--- a/cmd/riscv/sbi.c
+++ b/cmd/riscv/sbi.c
@@ -29,21 +29,21 @@ static struct sbi_imp implementations[] = {
};
static struct sbi_ext extensions[] = {
- { 0x00000000, "sbi_set_timer" },
- { 0x00000001, "sbi_console_putchar" },
- { 0x00000002, "sbi_console_getchar" },
- { 0x00000003, "sbi_clear_ipi" },
- { 0x00000004, "sbi_send_ipi" },
- { 0x00000005, "sbi_remote_fence_i" },
- { 0x00000006, "sbi_remote_sfence_vma" },
- { 0x00000007, "sbi_remote_sfence_vma_asid" },
- { 0x00000008, "sbi_shutdown" },
- { 0x00000010, "SBI Base Functionality" },
- { 0x54494D45, "Timer Extension" },
- { 0x00735049, "IPI Extension" },
- { 0x52464E43, "RFENCE Extension" },
- { 0x0048534D, "Hart State Management Extension" },
- { 0x53525354, "System Reset Extension" },
+ { SBI_EXT_0_1_SET_TIMER, "sbi_set_timer" },
+ { SBI_EXT_0_1_CONSOLE_PUTCHAR, "sbi_console_putchar" },
+ { SBI_EXT_0_1_CONSOLE_GETCHAR, "sbi_console_getchar" },
+ { SBI_EXT_0_1_CLEAR_IPI, "sbi_clear_ipi" },
+ { SBI_EXT_0_1_SEND_IPI, "sbi_send_ipi" },
+ { SBI_EXT_0_1_REMOTE_FENCE_I, "sbi_remote_fence_i" },
+ { SBI_EXT_0_1_REMOTE_SFENCE_VMA, "sbi_remote_sfence_vma" },
+ { SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID, "sbi_remote_sfence_vma_asid" },
+ { SBI_EXT_0_1_SHUTDOWN, "sbi_shutdown" },
+ { SBI_EXT_BASE, "SBI Base Functionality" },
+ { SBI_EXT_TIME, "Timer Extension" },
+ { SBI_EXT_IPI, "IPI Extension" },
+ { SBI_EXT_RFENCE, "RFENCE Extension" },
+ { SBI_EXT_HSM, "Hart State Management Extension" },
+ { SBI_EXT_SRST, "System Reset Extension" },
};
static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc,
--
2.30.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/5] sysreset: provide SBI based sysreset driver
2021-03-04 17:00 [PATCH 0/5] riscv: enable SBI system reset Heinrich Schuchardt
2021-03-04 17:00 ` [PATCH 1/5] risv: add missing SBI extension definitions Heinrich Schuchardt
2021-03-04 17:00 ` [PATCH 2/5] cmd/sbi: use constants instead of numerical values Heinrich Schuchardt
@ 2021-03-04 17:00 ` Heinrich Schuchardt
2021-03-04 23:50 ` Sean Anderson
2021-03-04 17:00 ` [PATCH 4/5] pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON Heinrich Schuchardt
2021-03-04 17:00 ` [PATCH 5/5] maix: enable SBI system reset for MAIX Heinrich Schuchardt
4 siblings, 1 reply; 19+ messages in thread
From: Heinrich Schuchardt @ 2021-03-04 17:00 UTC (permalink / raw)
To: u-boot
Provide sysreset driver using the SBI system reset extension.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
MAINTAINERS | 1 +
arch/riscv/include/asm/sbi.h | 1 +
arch/riscv/lib/sbi.c | 21 +++++--
drivers/sysreset/Kconfig | 11 ++++
drivers/sysreset/Makefile | 1 +
drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++++++
lib/efi_loader/Kconfig | 2 +-
7 files changed, 134 insertions(+), 5 deletions(-)
create mode 100644 drivers/sysreset/sysreset_sbi.c
diff --git a/MAINTAINERS b/MAINTAINERS
index de499940e5..192db06ff9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -985,6 +985,7 @@ T: git https://source.denx.de/u-boot/custodians/u-boot-riscv.git
F: arch/riscv/
F: cmd/riscv/
F: doc/usage/sbi.rst
+F: drivers/sysreset/sysreset_sbi.c
F: drivers/timer/andes_plmt_timer.c
F: drivers/timer/sifive_clint_timer.c
F: tools/prelink-riscv.c
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index c598bb11ce..058e2e23a8 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -150,5 +150,6 @@ void sbi_set_timer(uint64_t stime_value);
long sbi_get_spec_version(void);
int sbi_get_impl_id(void);
int sbi_probe_extension(int ext);
+void sbi_srst_reset(unsigned long type, unsigned long reason);
#endif
diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c
index 77845a73ca..8508041f2a 100644
--- a/arch/riscv/lib/sbi.c
+++ b/arch/riscv/lib/sbi.c
@@ -8,13 +8,14 @@
*/
#include <common.h>
+#include <efi_loader.h>
#include <asm/encoding.h>
#include <asm/sbi.h>
-struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
- unsigned long arg1, unsigned long arg2,
- unsigned long arg3, unsigned long arg4,
- unsigned long arg5)
+struct sbiret __efi_runtime sbi_ecall(int ext, int fid, unsigned long arg0,
+ unsigned long arg1, unsigned long arg2,
+ unsigned long arg3, unsigned long arg4,
+ unsigned long arg5)
{
struct sbiret ret;
@@ -108,6 +109,18 @@ int sbi_probe_extension(int extid)
return -ENOTSUPP;
}
+/**
+ * sbi_srst_reset() - invoke system reset extension
+ *
+ * @type: type of reset
+ * @reason: reason for reset
+ */
+void __efi_runtime sbi_srst_reset(unsigned long type, unsigned long reason)
+{
+ sbi_ecall(SBI_EXT_SRST, SBI_EXT_SRST_RESET, type, reason,
+ 0, 0, 0, 0);
+}
+
#ifdef CONFIG_SBI_V01
/**
diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
index 0e5c7c9971..05a7e26300 100644
--- a/drivers/sysreset/Kconfig
+++ b/drivers/sysreset/Kconfig
@@ -79,6 +79,17 @@ config SYSRESET_PSCI
Enable PSCI SYSTEM_RESET function call. To use this, PSCI firmware
must be running on your system.
+config SYSRESET_SBI
+ bool "Enable support for SBI System Reset"
+ depends on RISCV_SMODE && SBI_V02
+ select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
+ help
+ Enable system reset and poweroff via the SBI system reset extension.
+ If the SBI implemention provides the extension, is board specific.
+ The extension was introduced in version 0.3 of the SBI specification.
+ The SBI system reset driver supports the UEFI ResetSystem() service
+ at runtime.
+
config SYSRESET_SOCFPGA
bool "Enable support for Intel SOCFPGA family"
depends on ARCH_SOCFPGA && (TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10)
diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
index de81c399d7..8e00be0779 100644
--- a/drivers/sysreset/Makefile
+++ b/drivers/sysreset/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_SYSRESET_MPC83XX) += sysreset_mpc83xx.o
obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o
obj-$(CONFIG_SYSRESET_OCTEON) += sysreset_octeon.o
obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o
+obj-$(CONFIG_SYSRESET_SBI) += sysreset_sbi.o
obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o
obj-$(CONFIG_SYSRESET_SOCFPGA_SOC64) += sysreset_socfpga_soc64.o
obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o
diff --git a/drivers/sysreset/sysreset_sbi.c b/drivers/sysreset/sysreset_sbi.c
new file mode 100644
index 0000000000..87fbc3ea3e
--- /dev/null
+++ b/drivers/sysreset/sysreset_sbi.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2021, Heinrich Schuchardt <xypron.glpk@gmx.de>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <efi_loader.h>
+#include <log.h>
+#include <sysreset.h>
+#include <asm/sbi.h>
+
+static long __efi_runtime_data have_reset;
+
+static int sbi_sysreset_request(struct udevice *dev, enum sysreset_t type)
+{
+ enum sbi_srst_reset_type reset_type;
+
+ if (!have_reset)
+ return -ENOENT;
+
+ switch (type) {
+ case SYSRESET_WARM:
+ reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;
+ break;
+ case SYSRESET_COLD:
+ reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;
+ break;
+ case SYSRESET_POWER_OFF:
+ reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;
+ break;
+ default:
+ log_err("SBI has no system reset extension\n");
+ return -ENOSYS;
+ }
+
+ sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
+
+ return -EINPROGRESS;
+}
+
+efi_status_t efi_reset_system_init(void)
+{
+ return EFI_SUCCESS;
+}
+
+void __efi_runtime EFIAPI efi_reset_system(enum efi_reset_type type,
+ efi_status_t reset_status,
+ unsigned long data_size,
+ void *reset_data)
+{
+ enum sbi_srst_reset_type reset_type;
+
+ if (have_reset)
+ switch (type) {
+ case SYSRESET_WARM:
+ reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;
+ break;
+ case SYSRESET_COLD:
+ reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;
+ break;
+ case SYSRESET_POWER_OFF:
+ reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;
+ break;
+ default:
+ goto hang;
+ }
+
+ sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
+
+hang:
+ while (1)
+ ;
+}
+
+static int sbi_sysreset_probe(struct udevice *dev)
+{
+ have_reset = sbi_probe_extension(SBI_EXT_SRST);
+ if (have_reset)
+ return 0;
+
+ log_err("SBI has no system reset extension\n");
+ return -ENOENT;
+}
+
+static const struct udevice_id sbi_sysreset_ids[] = {
+ { .compatible = "riscv" },
+ { }
+};
+
+static struct sysreset_ops sbi_sysreset_ops = {
+ .request = sbi_sysreset_request,
+};
+
+U_BOOT_DRIVER(sbi_sysreset) = {
+ .name = "sbi-sysreset",
+ .id = UCLASS_SYSRESET,
+ .of_match = sbi_sysreset_ids,
+ .ops = &sbi_sysreset_ops,
+ .probe = sbi_sysreset_probe,
+};
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index e729f727df..7e76435339 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -277,7 +277,7 @@ config EFI_HAVE_RUNTIME_RESET
bool
default y
depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \
- SANDBOX || SYSRESET_X86
+ SANDBOX || SYSRESET_SBI || SYSRESET_X86
config EFI_GRUB_ARM32_WORKAROUND
bool "Workaround for GRUB on 32bit ARM"
--
2.30.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/5] pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON
2021-03-04 17:00 [PATCH 0/5] riscv: enable SBI system reset Heinrich Schuchardt
` (2 preceding siblings ...)
2021-03-04 17:00 ` [PATCH 3/5] sysreset: provide SBI based sysreset driver Heinrich Schuchardt
@ 2021-03-04 17:00 ` Heinrich Schuchardt
2021-03-04 23:15 ` Sean Anderson
2021-03-08 6:29 ` Leo Liang
2021-03-04 17:00 ` [PATCH 5/5] maix: enable SBI system reset for MAIX Heinrich Schuchardt
4 siblings, 2 replies; 19+ messages in thread
From: Heinrich Schuchardt @ 2021-03-04 17:00 UTC (permalink / raw)
To: u-boot
Select missing Kconfig dependencies.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
drivers/pinctrl/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 77fb851114..2a859ab5d0 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -294,6 +294,8 @@ config ASPEED_AST2500_PINCTRL
config PINCTRL_K210
bool "Kendryte K210 Fully-Programmable Input/Output Array driver"
depends on DM && PINCTRL_GENERIC
+ select REGMAP
+ select SYSCON
help
Support pin multiplexing on the K210. The "FPIOA" can remap any
supported function to any multifunctional IO pin. It can also perform
--
2.30.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/5] maix: enable SBI system reset for MAIX
2021-03-04 17:00 [PATCH 0/5] riscv: enable SBI system reset Heinrich Schuchardt
` (3 preceding siblings ...)
2021-03-04 17:00 ` [PATCH 4/5] pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON Heinrich Schuchardt
@ 2021-03-04 17:00 ` Heinrich Schuchardt
2021-03-04 23:24 ` Sean Anderson
4 siblings, 1 reply; 19+ messages in thread
From: Heinrich Schuchardt @ 2021-03-04 17:00 UTC (permalink / raw)
To: u-boot
When running in S-mode we can use the SBI system reset extension to provide
the system reset.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
board/sipeed/maix/maix.c | 5 +++++
configs/sipeed_maix_smode_defconfig | 2 ++
2 files changed, 7 insertions(+)
diff --git a/board/sipeed/maix/maix.c b/board/sipeed/maix/maix.c
index cbcb23cf5c..388eddee6e 100644
--- a/board/sipeed/maix/maix.c
+++ b/board/sipeed/maix/maix.c
@@ -6,6 +6,7 @@
#include <common.h>
#include <clk.h>
#include <dm.h>
+#include <dm/lists.h>
#include <fdt_support.h>
#include <asm/io.h>
@@ -37,5 +38,9 @@ int board_init(void)
return ret;
}
+ if (IS_ENABLED(CONFIG_SYSRESET_SBI))
+ ret = device_bind_driver(gd->dm_root, "sbi-sysreset",
+ "sbi-sysreset", NULL);
+
return 0;
}
diff --git a/configs/sipeed_maix_smode_defconfig b/configs/sipeed_maix_smode_defconfig
index 2516bb7258..aa95893feb 100644
--- a/configs/sipeed_maix_smode_defconfig
+++ b/configs/sipeed_maix_smode_defconfig
@@ -7,4 +7,6 @@ CONFIG_STACK_SIZE=0x100000
# CONFIG_NET is not set
# CONFIG_INPUT is not set
# CONFIG_DM_ETH is not set
+CONFIG_SYSRESET_SBI=y
+# CONFIG_SYSRESET_SYSCON is not set
# CONFIG_EFI_UNICODE_CAPITALIZATION is not set
--
2.30.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/5] pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON
2021-03-04 17:00 ` [PATCH 4/5] pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON Heinrich Schuchardt
@ 2021-03-04 23:15 ` Sean Anderson
2021-03-08 6:29 ` Leo Liang
1 sibling, 0 replies; 19+ messages in thread
From: Sean Anderson @ 2021-03-04 23:15 UTC (permalink / raw)
To: u-boot
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
> Select missing Kconfig dependencies.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> drivers/pinctrl/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 77fb851114..2a859ab5d0 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -294,6 +294,8 @@ config ASPEED_AST2500_PINCTRL
> config PINCTRL_K210
> bool "Kendryte K210 Fully-Programmable Input/Output Array driver"
> depends on DM && PINCTRL_GENERIC
> + select REGMAP
> + select SYSCON
> help
> Support pin multiplexing on the K210. The "FPIOA" can remap any
> supported function to any multifunctional IO pin. It can also perform
> --
> 2.30.1
>
Reviewed-by: Sean Anderson <seanga2@gmail.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/5] maix: enable SBI system reset for MAIX
2021-03-04 17:00 ` [PATCH 5/5] maix: enable SBI system reset for MAIX Heinrich Schuchardt
@ 2021-03-04 23:24 ` Sean Anderson
2021-03-06 7:31 ` Heinrich Schuchardt
0 siblings, 1 reply; 19+ messages in thread
From: Sean Anderson @ 2021-03-04 23:24 UTC (permalink / raw)
To: u-boot
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
> When running in S-mode we can use the SBI system reset extension to provide
> the system reset.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> board/sipeed/maix/maix.c | 5 +++++
> configs/sipeed_maix_smode_defconfig | 2 ++
> 2 files changed, 7 insertions(+)
>
> diff --git a/board/sipeed/maix/maix.c b/board/sipeed/maix/maix.c
> index cbcb23cf5c..388eddee6e 100644
> --- a/board/sipeed/maix/maix.c
> +++ b/board/sipeed/maix/maix.c
> @@ -6,6 +6,7 @@
> #include <common.h>
> #include <clk.h>
> #include <dm.h>
> +#include <dm/lists.h>
> #include <fdt_support.h>
> #include <asm/io.h>
>
> @@ -37,5 +38,9 @@ int board_init(void)
> return ret;
> }
>
> + if (IS_ENABLED(CONFIG_SYSRESET_SBI))
> + ret = device_bind_driver(gd->dm_root, "sbi-sysreset",
> + "sbi-sysreset", NULL);
> +
Besides that this isn't a device-tree driver, shouldn't this live in
arch_early_init_r or similar? This isn't really board-specific.
--Sean
> return 0;
> }
> diff --git a/configs/sipeed_maix_smode_defconfig b/configs/sipeed_maix_smode_defconfig
> index 2516bb7258..aa95893feb 100644
> --- a/configs/sipeed_maix_smode_defconfig
> +++ b/configs/sipeed_maix_smode_defconfig
> @@ -7,4 +7,6 @@ CONFIG_STACK_SIZE=0x100000
> # CONFIG_NET is not set
> # CONFIG_INPUT is not set
> # CONFIG_DM_ETH is not set
> +CONFIG_SYSRESET_SBI=y
> +# CONFIG_SYSRESET_SYSCON is not set
> # CONFIG_EFI_UNICODE_CAPITALIZATION is not set
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/5] risv: add missing SBI extension definitions
2021-03-04 17:00 ` [PATCH 1/5] risv: add missing SBI extension definitions Heinrich Schuchardt
@ 2021-03-04 23:28 ` Sean Anderson
2021-03-06 5:33 ` Heinrich Schuchardt
2021-03-06 5:59 ` Bin Meng
1 sibling, 1 reply; 19+ messages in thread
From: Sean Anderson @ 2021-03-04 23:28 UTC (permalink / raw)
To: u-boot
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
> Add the System Reset Extension and the Hart State Management Extension
> definitions.
>
> Add missing RFENCE Extension enum values.
>
> The SBI 0.1 extension constants are needed for for the sbi command. Remove
> an #ifdef.
>
> Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 53ca316180..c598bb11ce 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -12,7 +12,6 @@
> #include <linux/types.h>
>
> enum sbi_ext_id {
> -#ifdef CONFIG_SBI_V01
> SBI_EXT_0_1_SET_TIMER = 0x0,
> SBI_EXT_0_1_CONSOLE_PUTCHAR = 0x1,
> SBI_EXT_0_1_CONSOLE_GETCHAR = 0x2,
> @@ -22,11 +21,12 @@ enum sbi_ext_id {
> SBI_EXT_0_1_REMOTE_SFENCE_VMA = 0x6,
> SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID = 0x7,
> SBI_EXT_0_1_SHUTDOWN = 0x8,
> -#endif
> SBI_EXT_BASE = 0x10,
> SBI_EXT_TIME = 0x54494D45,
> SBI_EXT_IPI = 0x735049,
> SBI_EXT_RFENCE = 0x52464E43,
> + SBI_EXT_HSM = 0x48534D,
> + SBI_EXT_SRST = 0x53525354,
> };
>
> enum sbi_ext_base_fid {
> @@ -51,6 +51,38 @@ enum sbi_ext_rfence_fid {
> SBI_EXT_RFENCE_REMOTE_FENCE_I = 0,
> SBI_EXT_RFENCE_REMOTE_SFENCE_VMA,
> SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID,
> + SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA,
> + SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID,
> + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA,
> + SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID,
Isn't this backwards? That is, we should have
+ SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID,
+ SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA,
+ SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID,
+ SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA,
so the FIDs match.
--Sean
> +};
> +
> +enum sbi_ext_hsm_fid {
> + SBI_EXT_HSM_HART_START = 0,
> + SBI_EXT_HSM_HART_STOP,
> + SBI_EXT_HSM_HART_STATUS,
> +};
> +
> +enum sbi_hsm_hart_status {
> + SBI_HSM_HART_STATUS_STARTED = 0,
> + SBI_HSM_HART_STATUS_STOPPED,
> + SBI_HSM_HART_STATUS_START_PENDING,
> + SBI_HSM_HART_STATUS_STOP_PENDING,
> +};
> +
> +enum sbi_ext_srst_fid {
> + SBI_EXT_SRST_RESET = 0,
> +};
> +
> +enum sbi_srst_reset_type {
> + SBI_SRST_RESET_TYPE_SHUTDOWN = 0,
> + SBI_SRST_RESET_TYPE_COLD_REBOOT,
> + SBI_SRST_RESET_TYPE_WARM_REBOOT,
> +};
> +
> +enum sbi_srst_reset_reason {
> + SBI_SRST_RESET_REASON_NONE = 0,
> + SBI_SRST_RESET_REASON_SYS_FAILURE,
> };
>
> #ifdef CONFIG_SBI_V01
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/5] cmd/sbi: use constants instead of numerical values
2021-03-04 17:00 ` [PATCH 2/5] cmd/sbi: use constants instead of numerical values Heinrich Schuchardt
@ 2021-03-04 23:31 ` Sean Anderson
2021-03-08 6:25 ` Leo Liang
1 sibling, 0 replies; 19+ messages in thread
From: Sean Anderson @ 2021-03-04 23:31 UTC (permalink / raw)
To: u-boot
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
> Use constants for extension IDs.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> cmd/riscv/sbi.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
> index 90c0811e14..65a2c93290 100644
> --- a/cmd/riscv/sbi.c
> +++ b/cmd/riscv/sbi.c
> @@ -29,21 +29,21 @@ static struct sbi_imp implementations[] = {
> };
>
> static struct sbi_ext extensions[] = {
> - { 0x00000000, "sbi_set_timer" },
> - { 0x00000001, "sbi_console_putchar" },
> - { 0x00000002, "sbi_console_getchar" },
> - { 0x00000003, "sbi_clear_ipi" },
> - { 0x00000004, "sbi_send_ipi" },
> - { 0x00000005, "sbi_remote_fence_i" },
> - { 0x00000006, "sbi_remote_sfence_vma" },
> - { 0x00000007, "sbi_remote_sfence_vma_asid" },
> - { 0x00000008, "sbi_shutdown" },
> - { 0x00000010, "SBI Base Functionality" },
> - { 0x54494D45, "Timer Extension" },
> - { 0x00735049, "IPI Extension" },
> - { 0x52464E43, "RFENCE Extension" },
> - { 0x0048534D, "Hart State Management Extension" },
> - { 0x53525354, "System Reset Extension" },
> + { SBI_EXT_0_1_SET_TIMER, "sbi_set_timer" },
> + { SBI_EXT_0_1_CONSOLE_PUTCHAR, "sbi_console_putchar" },
> + { SBI_EXT_0_1_CONSOLE_GETCHAR, "sbi_console_getchar" },
> + { SBI_EXT_0_1_CLEAR_IPI, "sbi_clear_ipi" },
> + { SBI_EXT_0_1_SEND_IPI, "sbi_send_ipi" },
> + { SBI_EXT_0_1_REMOTE_FENCE_I, "sbi_remote_fence_i" },
> + { SBI_EXT_0_1_REMOTE_SFENCE_VMA, "sbi_remote_sfence_vma" },
> + { SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID, "sbi_remote_sfence_vma_asid" },
> + { SBI_EXT_0_1_SHUTDOWN, "sbi_shutdown" },
> + { SBI_EXT_BASE, "SBI Base Functionality" },
> + { SBI_EXT_TIME, "Timer Extension" },
> + { SBI_EXT_IPI, "IPI Extension" },
> + { SBI_EXT_RFENCE, "RFENCE Extension" },
> + { SBI_EXT_HSM, "Hart State Management Extension" },
> + { SBI_EXT_SRST, "System Reset Extension" },
> };
>
> static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc,
> --
> 2.30.1
>
Reviewed-by: Sean Anderson <seanga2@gmail.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/5] sysreset: provide SBI based sysreset driver
2021-03-04 17:00 ` [PATCH 3/5] sysreset: provide SBI based sysreset driver Heinrich Schuchardt
@ 2021-03-04 23:50 ` Sean Anderson
2021-03-06 7:18 ` Heinrich Schuchardt
0 siblings, 1 reply; 19+ messages in thread
From: Sean Anderson @ 2021-03-04 23:50 UTC (permalink / raw)
To: u-boot
On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
> Provide sysreset driver using the SBI system reset extension.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> MAINTAINERS | 1 +
> arch/riscv/include/asm/sbi.h | 1 +
> arch/riscv/lib/sbi.c | 21 +++++--
> drivers/sysreset/Kconfig | 11 ++++
> drivers/sysreset/Makefile | 1 +
> drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++++++
> lib/efi_loader/Kconfig | 2 +-
> 7 files changed, 134 insertions(+), 5 deletions(-)
> create mode 100644 drivers/sysreset/sysreset_sbi.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index de499940e5..192db06ff9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -985,6 +985,7 @@ T: git https://source.denx.de/u-boot/custodians/u-boot-riscv.git
> F: arch/riscv/
> F: cmd/riscv/
> F: doc/usage/sbi.rst
> +F: drivers/sysreset/sysreset_sbi.c
> F: drivers/timer/andes_plmt_timer.c
> F: drivers/timer/sifive_clint_timer.c
> F: tools/prelink-riscv.c
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index c598bb11ce..058e2e23a8 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -150,5 +150,6 @@ void sbi_set_timer(uint64_t stime_value);
> long sbi_get_spec_version(void);
> int sbi_get_impl_id(void);
> int sbi_probe_extension(int ext);
> +void sbi_srst_reset(unsigned long type, unsigned long reason);
>
> #endif
> diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c
> index 77845a73ca..8508041f2a 100644
> --- a/arch/riscv/lib/sbi.c
> +++ b/arch/riscv/lib/sbi.c
> @@ -8,13 +8,14 @@
> */
>
> #include <common.h>
> +#include <efi_loader.h>
> #include <asm/encoding.h>
> #include <asm/sbi.h>
>
> -struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
> - unsigned long arg1, unsigned long arg2,
> - unsigned long arg3, unsigned long arg4,
> - unsigned long arg5)
> +struct sbiret __efi_runtime sbi_ecall(int ext, int fid, unsigned long arg0,
> + unsigned long arg1, unsigned long arg2,
> + unsigned long arg3, unsigned long arg4,
> + unsigned long arg5)
> {
> struct sbiret ret;
>
> @@ -108,6 +109,18 @@ int sbi_probe_extension(int extid)
> return -ENOTSUPP;
> }
>
> +/**
> + * sbi_srst_reset() - invoke system reset extension
> + *
> + * @type: type of reset
> + * @reason: reason for reset
> + */
> +void __efi_runtime sbi_srst_reset(unsigned long type, unsigned long reason)
> +{
> + sbi_ecall(SBI_EXT_SRST, SBI_EXT_SRST_RESET, type, reason,
> + 0, 0, 0, 0);
> +}
> +
> #ifdef CONFIG_SBI_V01
>
> /**
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index 0e5c7c9971..05a7e26300 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -79,6 +79,17 @@ config SYSRESET_PSCI
> Enable PSCI SYSTEM_RESET function call. To use this, PSCI firmware
> must be running on your system.
>
> +config SYSRESET_SBI
> + bool "Enable support for SBI System Reset"
> + depends on RISCV_SMODE && SBI_V02
> + select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
> + help
> + Enable system reset and poweroff via the SBI system reset extension.
> + If the SBI implemention provides the extension, is board specific.
> + The extension was introduced in version 0.3 of the SBI specification.
> + The SBI system reset driver supports the UEFI ResetSystem() service
> + at runtime.
> +
> config SYSRESET_SOCFPGA
> bool "Enable support for Intel SOCFPGA family"
> depends on ARCH_SOCFPGA && (TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10)
> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
> index de81c399d7..8e00be0779 100644
> --- a/drivers/sysreset/Makefile
> +++ b/drivers/sysreset/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_SYSRESET_MPC83XX) += sysreset_mpc83xx.o
> obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o
> obj-$(CONFIG_SYSRESET_OCTEON) += sysreset_octeon.o
> obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o
> +obj-$(CONFIG_SYSRESET_SBI) += sysreset_sbi.o
> obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o
> obj-$(CONFIG_SYSRESET_SOCFPGA_SOC64) += sysreset_socfpga_soc64.o
> obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o
> diff --git a/drivers/sysreset/sysreset_sbi.c b/drivers/sysreset/sysreset_sbi.c
> new file mode 100644
> index 0000000000..87fbc3ea3e
> --- /dev/null
> +++ b/drivers/sysreset/sysreset_sbi.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2021, Heinrich Schuchardt <xypron.glpk@gmx.de>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <efi_loader.h>
> +#include <log.h>
> +#include <sysreset.h>
> +#include <asm/sbi.h>
> +
> +static long __efi_runtime_data have_reset;
> +
> +static int sbi_sysreset_request(struct udevice *dev, enum sysreset_t type)
> +{
> + enum sbi_srst_reset_type reset_type;
> +
> + if (!have_reset)
> + return -ENOENT;
Is this necessary? This should never be called if there is no reset,
since we just fail to probe.
> +
> + switch (type) {
> + case SYSRESET_WARM:
> + reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;
> + break;
> + case SYSRESET_COLD:
> + reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;
> + break;
> + case SYSRESET_POWER_OFF:
> + reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;
> + break;
> + default:
> + log_err("SBI has no system reset extension\n");
> + return -ENOSYS;
> + }
> +
> + sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
> +
> + return -EINPROGRESS;
> +}
> +
> +efi_status_t efi_reset_system_init(void)
> +{
> + return EFI_SUCCESS;
> +}
> +
> +void __efi_runtime EFIAPI efi_reset_system(enum efi_reset_type type,
> + efi_status_t reset_status,
> + unsigned long data_size,
> + void *reset_data)
As an aside, is there a reason why the generic (weak) efi_reset_system
does not just call sysreset_walk_halt(type)?
> +{
> + enum sbi_srst_reset_type reset_type;
> +
> + if (have_reset)
> + switch (type) {
> + case SYSRESET_WARM:
> + reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;
> + break;
> + case SYSRESET_COLD:
> + reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;
> + break;
> + case SYSRESET_POWER_OFF:
> + reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;
> + break;
> + default:
> + goto hang;
Why do we hang by default? For comparison, sysreset_x86 has
if (reset_type == EFI_RESET_COLD ||
reset_type == EFI_RESET_PLATFORM_SPECIFIC)
value = SYS_RST | RST_CPU | FULL_RST;
else /* assume EFI_RESET_WARM since we cannot return an error */
value = SYS_RST | RST_CPU;
> + }
> +
> + sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
Should we instead do
enum sbi_srst_reset_reason reset_reason;
if (reset_status == EFI_SUCCESS)
reset_reason = SBI_SRST_RESET_REASON_NONE;
else
reset_reason = SBI_SRST_RESET_REASON_SYS_FAILURE;
sbi_srst_reset(reset_type, reset_status == EFI_SUCCESS ? SBI_SRST_RESET_REASON_NONE : SBI_SRST_RESET_REASON_SYS_FAILURE);
since we have reset status available?
> +
> +hang:
> + while (1)
> + ;
> +}
> +
> +static int sbi_sysreset_probe(struct udevice *dev)
> +{
> + have_reset = sbi_probe_extension(SBI_EXT_SRST);
> + if (have_reset)
> + return 0;
> +
> + log_err("SBI has no system reset extension\n");
Is this an error? It's perfectly normal for SBI implementations to lack
support for some extensions. Perhaps a warning/info would be better.
> + return -ENOENT;
> +}
> +
> +static const struct udevice_id sbi_sysreset_ids[] = {
> + { .compatible = "riscv" },
This compatible string is already in-use for RISC-V CPUs. And if this
isn't supposed to have a device tree binding, do we even need of_match?
> + { }
> +};
> +
> +static struct sysreset_ops sbi_sysreset_ops = {
> + .request = sbi_sysreset_request,
> +};
> +
> +U_BOOT_DRIVER(sbi_sysreset) = {
> + .name = "sbi-sysreset",
> + .id = UCLASS_SYSRESET,
> + .of_match = sbi_sysreset_ids,
> + .ops = &sbi_sysreset_ops,
> + .probe = sbi_sysreset_probe,
> +};
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e729f727df..7e76435339 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -277,7 +277,7 @@ config EFI_HAVE_RUNTIME_RESET
> bool
> default y
> depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \
> - SANDBOX || SYSRESET_X86
> + SANDBOX || SYSRESET_SBI || SYSRESET_X86
As a general note, perhaps it is better to set this from other Kconfigs?
How long will this list get?
--Sean
>
> config EFI_GRUB_ARM32_WORKAROUND
> bool "Workaround for GRUB on 32bit ARM"
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/5] risv: add missing SBI extension definitions
2021-03-04 23:28 ` Sean Anderson
@ 2021-03-06 5:33 ` Heinrich Schuchardt
0 siblings, 0 replies; 19+ messages in thread
From: Heinrich Schuchardt @ 2021-03-06 5:33 UTC (permalink / raw)
To: u-boot
On 3/5/21 12:28 AM, Sean Anderson wrote:
> On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
>> Add the System Reset Extension and the Hart State Management Extension
>> definitions.
>>
>> Add missing RFENCE Extension enum values.
>>
>> The SBI 0.1 extension constants are needed for for the sbi command.
>> Remove
>> an #ifdef.
>>
>> Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> ? arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++--
>> ? 1 file changed, 34 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
>> index 53ca316180..c598bb11ce 100644
>> --- a/arch/riscv/include/asm/sbi.h
>> +++ b/arch/riscv/include/asm/sbi.h
>> @@ -12,7 +12,6 @@
>> ? #include <linux/types.h>
>>
>> ? enum sbi_ext_id {
>> -#ifdef CONFIG_SBI_V01
>> ????? SBI_EXT_0_1_SET_TIMER = 0x0,
>> ????? SBI_EXT_0_1_CONSOLE_PUTCHAR = 0x1,
>> ????? SBI_EXT_0_1_CONSOLE_GETCHAR = 0x2,
>> @@ -22,11 +21,12 @@ enum sbi_ext_id {
>> ????? SBI_EXT_0_1_REMOTE_SFENCE_VMA = 0x6,
>> ????? SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID = 0x7,
>> ????? SBI_EXT_0_1_SHUTDOWN = 0x8,
>> -#endif
>> ????? SBI_EXT_BASE = 0x10,
>> ????? SBI_EXT_TIME = 0x54494D45,
>> ????? SBI_EXT_IPI = 0x735049,
>> ????? SBI_EXT_RFENCE = 0x52464E43,
>> +??? SBI_EXT_HSM = 0x48534D,
>> +??? SBI_EXT_SRST = 0x53525354,
>> ? };
>>
>> ? enum sbi_ext_base_fid {
>> @@ -51,6 +51,38 @@ enum sbi_ext_rfence_fid {
>> ????? SBI_EXT_RFENCE_REMOTE_FENCE_I = 0,
>> ????? SBI_EXT_RFENCE_REMOTE_SFENCE_VMA,
>> ????? SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID,
>> +??? SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA,
>> +??? SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID,
>> +??? SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA,
>> +??? SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID,
>
> Isn't this backwards? That is, we should have
>
> +??? SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID,
> +??? SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA,
> +??? SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID,
> +??? SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA,
>
> so the FIDs match.
You are right. I should not have copied from Linux without checking.
Cf.
https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc#78-function-listing
Best regards
Heinrich
>
> --Sean
>
>> +};
>> +
>> +enum sbi_ext_hsm_fid {
>> +??? SBI_EXT_HSM_HART_START = 0,
>> +??? SBI_EXT_HSM_HART_STOP,
>> +??? SBI_EXT_HSM_HART_STATUS,
>> +};
>> +
>> +enum sbi_hsm_hart_status {
>> +??? SBI_HSM_HART_STATUS_STARTED = 0,
>> +??? SBI_HSM_HART_STATUS_STOPPED,
>> +??? SBI_HSM_HART_STATUS_START_PENDING,
>> +??? SBI_HSM_HART_STATUS_STOP_PENDING,
>> +};
>> +
>> +enum sbi_ext_srst_fid {
>> +??? SBI_EXT_SRST_RESET = 0,
>> +};
>> +
>> +enum sbi_srst_reset_type {
>> +??? SBI_SRST_RESET_TYPE_SHUTDOWN = 0,
>> +??? SBI_SRST_RESET_TYPE_COLD_REBOOT,
>> +??? SBI_SRST_RESET_TYPE_WARM_REBOOT,
>> +};
>> +
>> +enum sbi_srst_reset_reason {
>> +??? SBI_SRST_RESET_REASON_NONE = 0,
>> +??? SBI_SRST_RESET_REASON_SYS_FAILURE,
>> ? };
>>
>> ? #ifdef CONFIG_SBI_V01
>> --
>> 2.30.1
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/5] risv: add missing SBI extension definitions
2021-03-04 17:00 ` [PATCH 1/5] risv: add missing SBI extension definitions Heinrich Schuchardt
2021-03-04 23:28 ` Sean Anderson
@ 2021-03-06 5:59 ` Bin Meng
2021-03-06 21:48 ` Sean Anderson
1 sibling, 1 reply; 19+ messages in thread
From: Bin Meng @ 2021-03-06 5:59 UTC (permalink / raw)
To: u-boot
On Fri, Mar 5, 2021 at 1:02 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Add the System Reset Extension and the Hart State Management Extension
> definitions.
>
> Add missing RFENCE Extension enum values.
>
> The SBI 0.1 extension constants are needed for for the sbi command. Remove
> an #ifdef.
>
> Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 34 insertions(+), 2 deletions(-)
>
U-Boot does not need HSM.
Regards,
Bin
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/5] sysreset: provide SBI based sysreset driver
2021-03-04 23:50 ` Sean Anderson
@ 2021-03-06 7:18 ` Heinrich Schuchardt
2021-03-06 16:40 ` Sean Anderson
0 siblings, 1 reply; 19+ messages in thread
From: Heinrich Schuchardt @ 2021-03-06 7:18 UTC (permalink / raw)
To: u-boot
On 3/5/21 12:50 AM, Sean Anderson wrote:
> On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
>> Provide sysreset driver using the SBI system reset extension.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> ? MAINTAINERS???????????????????? |?? 1 +
>> ? arch/riscv/include/asm/sbi.h??? |?? 1 +
>> ? arch/riscv/lib/sbi.c??????????? |? 21 +++++--
>> ? drivers/sysreset/Kconfig??????? |? 11 ++++
>> ? drivers/sysreset/Makefile?????? |?? 1 +
>> ? drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++++++
>> ? lib/efi_loader/Kconfig????????? |?? 2 +-
>> ? 7 files changed, 134 insertions(+), 5 deletions(-)
>> ? create mode 100644 drivers/sysreset/sysreset_sbi.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index de499940e5..192db06ff9 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -985,6 +985,7 @@ T:??? git
>> https://source.denx.de/u-boot/custodians/u-boot-riscv.git
>> ? F:??? arch/riscv/
>> ? F:??? cmd/riscv/
>> ? F:??? doc/usage/sbi.rst
>> +F:??? drivers/sysreset/sysreset_sbi.c
>> ? F:??? drivers/timer/andes_plmt_timer.c
>> ? F:??? drivers/timer/sifive_clint_timer.c
>> ? F:??? tools/prelink-riscv.c
>> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
>> index c598bb11ce..058e2e23a8 100644
>> --- a/arch/riscv/include/asm/sbi.h
>> +++ b/arch/riscv/include/asm/sbi.h
>> @@ -150,5 +150,6 @@ void sbi_set_timer(uint64_t stime_value);
>> ? long sbi_get_spec_version(void);
>> ? int sbi_get_impl_id(void);
>> ? int sbi_probe_extension(int ext);
>> +void sbi_srst_reset(unsigned long type, unsigned long reason);
>>
>> ? #endif
>> diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c
>> index 77845a73ca..8508041f2a 100644
>> --- a/arch/riscv/lib/sbi.c
>> +++ b/arch/riscv/lib/sbi.c
>> @@ -8,13 +8,14 @@
>> ?? */
>>
>> ? #include <common.h>
>> +#include <efi_loader.h>
>> ? #include <asm/encoding.h>
>> ? #include <asm/sbi.h>
>>
>> -struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
>> -??????????? unsigned long arg1, unsigned long arg2,
>> -??????????? unsigned long arg3, unsigned long arg4,
>> -??????????? unsigned long arg5)
>> +struct sbiret __efi_runtime sbi_ecall(int ext, int fid, unsigned long
>> arg0,
>> +????????????????????? unsigned long arg1, unsigned long arg2,
>> +????????????????????? unsigned long arg3, unsigned long arg4,
>> +????????????????????? unsigned long arg5)
>> ? {
>> ????? struct sbiret ret;
>>
>> @@ -108,6 +109,18 @@ int sbi_probe_extension(int extid)
>> ????? return -ENOTSUPP;
>> ? }
>>
>> +/**
>> + * sbi_srst_reset() - invoke system reset extension
>> + *
>> + * @type:??? type of reset
>> + * @reason:??? reason for reset
>> + */
>> +void __efi_runtime sbi_srst_reset(unsigned long type, unsigned long
>> reason)
>> +{
>> +??? sbi_ecall(SBI_EXT_SRST, SBI_EXT_SRST_RESET, type, reason,
>> +????????? 0, 0, 0, 0);
>> +}
>> +
>> ? #ifdef CONFIG_SBI_V01
>>
>> ? /**
>> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
>> index 0e5c7c9971..05a7e26300 100644
>> --- a/drivers/sysreset/Kconfig
>> +++ b/drivers/sysreset/Kconfig
>> @@ -79,6 +79,17 @@ config SYSRESET_PSCI
>> ??????? Enable PSCI SYSTEM_RESET function call.? To use this, PSCI
>> firmware
>> ??????? must be running on your system.
>>
>> +config SYSRESET_SBI
>> +??? bool "Enable support for SBI System Reset"
>> +??? depends on RISCV_SMODE && SBI_V02
>> +??? select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
>> +??? help
>> +????? Enable system reset and poweroff via the SBI system reset
>> extension.
>> +????? If the SBI implemention provides the extension, is board specific.
>> +????? The extension was introduced in version 0.3 of the SBI
>> specification.
>> +????? The SBI system reset driver supports the UEFI ResetSystem()
>> service
>> +????? at runtime.
>> +
>> ? config SYSRESET_SOCFPGA
>> ????? bool "Enable support for Intel SOCFPGA family"
>> ????? depends on ARCH_SOCFPGA && (TARGET_SOCFPGA_GEN5 ||
>> TARGET_SOCFPGA_ARRIA10)
>> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
>> index de81c399d7..8e00be0779 100644
>> --- a/drivers/sysreset/Makefile
>> +++ b/drivers/sysreset/Makefile
>> @@ -13,6 +13,7 @@ obj-$(CONFIG_SYSRESET_MPC83XX) += sysreset_mpc83xx.o
>> ? obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o
>> ? obj-$(CONFIG_SYSRESET_OCTEON) += sysreset_octeon.o
>> ? obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o
>> +obj-$(CONFIG_SYSRESET_SBI) += sysreset_sbi.o
>> ? obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o
>> ? obj-$(CONFIG_SYSRESET_SOCFPGA_SOC64) += sysreset_socfpga_soc64.o
>> ? obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o
>> diff --git a/drivers/sysreset/sysreset_sbi.c
>> b/drivers/sysreset/sysreset_sbi.c
>> new file mode 100644
>> index 0000000000..87fbc3ea3e
>> --- /dev/null
>> +++ b/drivers/sysreset/sysreset_sbi.c
>> @@ -0,0 +1,102 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2021, Heinrich Schuchardt <xypron.glpk@gmx.de>
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <efi_loader.h>
>> +#include <log.h>
>> +#include <sysreset.h>
>> +#include <asm/sbi.h>
>> +
>> +static long __efi_runtime_data have_reset;
>> +
>> +static int sbi_sysreset_request(struct udevice *dev, enum sysreset_t
>> type)
>> +{
>> +??? enum sbi_srst_reset_type reset_type;
>> +
>> +??? if (!have_reset)
>> +??????? return -ENOENT;
>
> Is this necessary? This should never be called if there is no reset,
> since we just fail to probe.
Thank you for reviewing.
Yes, we can skip it.
>
>> +
>> +??? switch (type) {
>> +??? case SYSRESET_WARM:
>> +??????? reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;
>> +??????? break;
>> +??? case SYSRESET_COLD:
>> +??????? reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;
>> +??????? break;
>> +??? case SYSRESET_POWER_OFF:
>> +??????? reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;
>> +??????? break;
>> +??? default:
>> +??????? log_err("SBI has no system reset extension\n");
>> +??????? return -ENOSYS;
>> +??? }
>> +
>> +??? sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
>> +
>> +??? return -EINPROGRESS;
>> +}
>> +
>> +efi_status_t efi_reset_system_init(void)
>> +{
>> +??? return EFI_SUCCESS;
>> +}
>> +
>> +void __efi_runtime EFIAPI efi_reset_system(enum efi_reset_type type,
>> +?????????????????????? efi_status_t reset_status,
>> +?????????????????????? unsigned long data_size,
>> +?????????????????????? void *reset_data)
>
> As an aside, is there a reason why the generic (weak) efi_reset_system
> does not just call sysreset_walk_halt(type)?
efi_reset_system is invoked at UEFI runtime. Most functions of U-Boot
are no longer in memory after ExitBootServices(). Only functions in the
__efi_runtime section may be called.
>
>> +{
>> +??? enum sbi_srst_reset_type reset_type;
>> +
>> +??? if (have_reset)
>> +??????? switch (type) {
>> +??????? case SYSRESET_WARM:
>> +??????????? reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;
>> +??????????? break;
>> +??????? case SYSRESET_COLD:
>> +??????????? reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;
>> +??????????? break;
>> +??????? case SYSRESET_POWER_OFF:
>> +??????????? reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;
>> +??????????? break;
>> +??????? default:
>> +??????????? goto hang;
>
> Why do we hang by default? For comparison, sysreset_x86 has
>
> ????if (reset_type == EFI_RESET_COLD ||
> ???????? reset_type == EFI_RESET_PLATFORM_SPECIFIC)
> ??????? value = SYS_RST | RST_CPU | FULL_RST;
> ????else /* assume EFI_RESET_WARM since we cannot return an error */
> ??????? value = SYS_RST | RST_CPU;
ok
>
>> +??? }
>> +
>> +??? sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
>
> Should we instead do
>
> ????enum sbi_srst_reset_reason reset_reason;
> ????if (reset_status == EFI_SUCCESS)
> ??????? reset_reason = SBI_SRST_RESET_REASON_NONE;
> ????else
> ??????? reset_reason = SBI_SRST_RESET_REASON_SYS_FAILURE;
> ????sbi_srst_reset(reset_type, reset_status == EFI_SUCCESS ?
> SBI_SRST_RESET_REASON_NONE : SBI_SRST_RESET_REASON_SYS_FAILURE);
>
> since we have reset status available?
Makes sense.
>
>> +
>> +hang:
>> +??? while (1)
>> +??????? ;
>> +}
>> +
>> +static int sbi_sysreset_probe(struct udevice *dev)
>> +{
>> +??? have_reset = sbi_probe_extension(SBI_EXT_SRST);
>> +??? if (have_reset)
>> +??????? return 0;
>> +
>> +??? log_err("SBI has no system reset extension\n");
>
> Is this an error? It's perfectly normal for SBI implementations to lack
> support for some extensions. Perhaps a warning/info would be better.
We shouldn't have chosen this driver in the configuration if the SBI
does not support it.
I will change this to a warning.
>
>> +??? return -ENOENT;
>> +}
>> +
>> +static const struct udevice_id sbi_sysreset_ids[] = {
>> +??? { .compatible = "riscv" },
>
> This compatible string is already in-use for RISC-V CPUs. And if this
> isn't supposed to have a device tree binding, do we even need of_match?
I discussed this with Simon in
https://lists.denx.de/pipermail/u-boot/2021-March/443270.html
Instead of adding code to the board files we should have a device-tree
node. A reasonable compatible string would be "riscv,sbi-sysreset".
>
>> +??? { }
>> +};
>> +
>> +static struct sysreset_ops sbi_sysreset_ops = {
>> +??? .request = sbi_sysreset_request,
>> +};
>> +
>> +U_BOOT_DRIVER(sbi_sysreset) = {
>> +??? .name = "sbi-sysreset",
>> +??? .id = UCLASS_SYSRESET,
>> +??? .of_match = sbi_sysreset_ids,
>> +??? .ops = &sbi_sysreset_ops,
>> +??? .probe = sbi_sysreset_probe,
>> +};
>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>> index e729f727df..7e76435339 100644
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -277,7 +277,7 @@ config EFI_HAVE_RUNTIME_RESET
>> ????? bool
>> ????? default y
>> ????? depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \
>> -?????????? SANDBOX || SYSRESET_X86
>> +?????????? SANDBOX || SYSRESET_SBI || SYSRESET_X86
>
> As a general note, perhaps it is better to set this from other Kconfigs?
> How long will this list get?
This seems to be a matter of taste.
Best regards
Heinrich
>
> --Sean
>
>>
>> ? config EFI_GRUB_ARM32_WORKAROUND
>> ????? bool "Workaround for GRUB on 32bit ARM"
>> --
>> 2.30.1
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/5] maix: enable SBI system reset for MAIX
2021-03-04 23:24 ` Sean Anderson
@ 2021-03-06 7:31 ` Heinrich Schuchardt
0 siblings, 0 replies; 19+ messages in thread
From: Heinrich Schuchardt @ 2021-03-06 7:31 UTC (permalink / raw)
To: u-boot
On 3/5/21 12:24 AM, Sean Anderson wrote:
> On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
>> When running in S-mode we can use the SBI system reset extension to
>> provide
>> the system reset.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> ? board/sipeed/maix/maix.c??????????? | 5 +++++
>> ? configs/sipeed_maix_smode_defconfig | 2 ++
>> ? 2 files changed, 7 insertions(+)
>>
>> diff --git a/board/sipeed/maix/maix.c b/board/sipeed/maix/maix.c
>> index cbcb23cf5c..388eddee6e 100644
>> --- a/board/sipeed/maix/maix.c
>> +++ b/board/sipeed/maix/maix.c
>> @@ -6,6 +6,7 @@
>> ? #include <common.h>
>> ? #include <clk.h>
>> ? #include <dm.h>
>> +#include <dm/lists.h>
>> ? #include <fdt_support.h>
>> ? #include <asm/io.h>
>>
>> @@ -37,5 +38,9 @@ int board_init(void)
>> ????????????? return ret;
>> ????? }
>>
>> +??? if (IS_ENABLED(CONFIG_SYSRESET_SBI))
>> +??????? ret = device_bind_driver(gd->dm_root, "sbi-sysreset",
>> +???????????????????? "sbi-sysreset", NULL);
>> +
>
> Besides that this isn't a device-tree driver, shouldn't this live in
> arch_early_init_r or similar? This isn't really board-specific.
Using arch_early_init_r() seems to be a viable alternative to putting
some dummy node into each device-tree.
Best regards
Heinrich
>
> --Sean
>
>> ????? return 0;
>> ? }
>> diff --git a/configs/sipeed_maix_smode_defconfig
>> b/configs/sipeed_maix_smode_defconfig
>> index 2516bb7258..aa95893feb 100644
>> --- a/configs/sipeed_maix_smode_defconfig
>> +++ b/configs/sipeed_maix_smode_defconfig
>> @@ -7,4 +7,6 @@ CONFIG_STACK_SIZE=0x100000
>> ? # CONFIG_NET is not set
>> ? # CONFIG_INPUT is not set
>> ? # CONFIG_DM_ETH is not set
>> +CONFIG_SYSRESET_SBI=y
>> +# CONFIG_SYSRESET_SYSCON is not set
>> ? # CONFIG_EFI_UNICODE_CAPITALIZATION is not set
>> --
>> 2.30.1
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/5] sysreset: provide SBI based sysreset driver
2021-03-06 7:18 ` Heinrich Schuchardt
@ 2021-03-06 16:40 ` Sean Anderson
0 siblings, 0 replies; 19+ messages in thread
From: Sean Anderson @ 2021-03-06 16:40 UTC (permalink / raw)
To: u-boot
On 3/6/21 2:18 AM, Heinrich Schuchardt wrote:
> On 3/5/21 12:50 AM, Sean Anderson wrote:
>> On 3/4/21 12:00 PM, Heinrich Schuchardt wrote:
>>> Provide sysreset driver using the SBI system reset extension.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>> MAINTAINERS | 1 +
>>> arch/riscv/include/asm/sbi.h | 1 +
>>> arch/riscv/lib/sbi.c | 21 +++++--
>>> drivers/sysreset/Kconfig | 11 ++++
>>> drivers/sysreset/Makefile | 1 +
>>> drivers/sysreset/sysreset_sbi.c | 102 ++++++++++++++++++++++++++++++++
>>> lib/efi_loader/Kconfig | 2 +-
>>> 7 files changed, 134 insertions(+), 5 deletions(-)
>>> create mode 100644 drivers/sysreset/sysreset_sbi.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index de499940e5..192db06ff9 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -985,6 +985,7 @@ T: git
>>> https://source.denx.de/u-boot/custodians/u-boot-riscv.git
>>> F: arch/riscv/
>>> F: cmd/riscv/
>>> F: doc/usage/sbi.rst
>>> +F: drivers/sysreset/sysreset_sbi.c
>>> F: drivers/timer/andes_plmt_timer.c
>>> F: drivers/timer/sifive_clint_timer.c
>>> F: tools/prelink-riscv.c
>>> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
>>> index c598bb11ce..058e2e23a8 100644
>>> --- a/arch/riscv/include/asm/sbi.h
>>> +++ b/arch/riscv/include/asm/sbi.h
>>> @@ -150,5 +150,6 @@ void sbi_set_timer(uint64_t stime_value);
>>> long sbi_get_spec_version(void);
>>> int sbi_get_impl_id(void);
>>> int sbi_probe_extension(int ext);
>>> +void sbi_srst_reset(unsigned long type, unsigned long reason);
>>>
>>> #endif
>>> diff --git a/arch/riscv/lib/sbi.c b/arch/riscv/lib/sbi.c
>>> index 77845a73ca..8508041f2a 100644
>>> --- a/arch/riscv/lib/sbi.c
>>> +++ b/arch/riscv/lib/sbi.c
>>> @@ -8,13 +8,14 @@
>>> */
>>>
>>> #include <common.h>
>>> +#include <efi_loader.h>
>>> #include <asm/encoding.h>
>>> #include <asm/sbi.h>
>>>
>>> -struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
>>> - unsigned long arg1, unsigned long arg2,
>>> - unsigned long arg3, unsigned long arg4,
>>> - unsigned long arg5)
>>> +struct sbiret __efi_runtime sbi_ecall(int ext, int fid, unsigned long
>>> arg0,
>>> + unsigned long arg1, unsigned long arg2,
>>> + unsigned long arg3, unsigned long arg4,
>>> + unsigned long arg5)
>>> {
>>> struct sbiret ret;
>>>
>>> @@ -108,6 +109,18 @@ int sbi_probe_extension(int extid)
>>> return -ENOTSUPP;
>>> }
>>>
>>> +/**
>>> + * sbi_srst_reset() - invoke system reset extension
>>> + *
>>> + * @type: type of reset
>>> + * @reason: reason for reset
>>> + */
>>> +void __efi_runtime sbi_srst_reset(unsigned long type, unsigned long
>>> reason)
>>> +{
>>> + sbi_ecall(SBI_EXT_SRST, SBI_EXT_SRST_RESET, type, reason,
>>> + 0, 0, 0, 0);
>>> +}
>>> +
>>> #ifdef CONFIG_SBI_V01
>>>
>>> /**
>>> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
>>> index 0e5c7c9971..05a7e26300 100644
>>> --- a/drivers/sysreset/Kconfig
>>> +++ b/drivers/sysreset/Kconfig
>>> @@ -79,6 +79,17 @@ config SYSRESET_PSCI
>>> Enable PSCI SYSTEM_RESET function call. To use this, PSCI
>>> firmware
>>> must be running on your system.
>>>
>>> +config SYSRESET_SBI
>>> + bool "Enable support for SBI System Reset"
>>> + depends on RISCV_SMODE && SBI_V02
>>> + select SYSRESET_CMD_POWEROFF if CMD_POWEROFF
>>> + help
>>> + Enable system reset and poweroff via the SBI system reset
>>> extension.
>>> + If the SBI implemention provides the extension, is board specific.
>>> + The extension was introduced in version 0.3 of the SBI
>>> specification.
>>> + The SBI system reset driver supports the UEFI ResetSystem()
>>> service
>>> + at runtime.
>>> +
>>> config SYSRESET_SOCFPGA
>>> bool "Enable support for Intel SOCFPGA family"
>>> depends on ARCH_SOCFPGA && (TARGET_SOCFPGA_GEN5 ||
>>> TARGET_SOCFPGA_ARRIA10)
>>> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
>>> index de81c399d7..8e00be0779 100644
>>> --- a/drivers/sysreset/Makefile
>>> +++ b/drivers/sysreset/Makefile
>>> @@ -13,6 +13,7 @@ obj-$(CONFIG_SYSRESET_MPC83XX) += sysreset_mpc83xx.o
>>> obj-$(CONFIG_SYSRESET_MICROBLAZE) += sysreset_microblaze.o
>>> obj-$(CONFIG_SYSRESET_OCTEON) += sysreset_octeon.o
>>> obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o
>>> +obj-$(CONFIG_SYSRESET_SBI) += sysreset_sbi.o
>>> obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o
>>> obj-$(CONFIG_SYSRESET_SOCFPGA_SOC64) += sysreset_socfpga_soc64.o
>>> obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o
>>> diff --git a/drivers/sysreset/sysreset_sbi.c
>>> b/drivers/sysreset/sysreset_sbi.c
>>> new file mode 100644
>>> index 0000000000..87fbc3ea3e
>>> --- /dev/null
>>> +++ b/drivers/sysreset/sysreset_sbi.c
>>> @@ -0,0 +1,102 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright 2021, Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <dm.h>
>>> +#include <errno.h>
>>> +#include <efi_loader.h>
>>> +#include <log.h>
>>> +#include <sysreset.h>
>>> +#include <asm/sbi.h>
>>> +
>>> +static long __efi_runtime_data have_reset;
>>> +
>>> +static int sbi_sysreset_request(struct udevice *dev, enum sysreset_t
>>> type)
>>> +{
>>> + enum sbi_srst_reset_type reset_type;
>>> +
>>> + if (!have_reset)
>>> + return -ENOENT;
>>
>> Is this necessary? This should never be called if there is no reset,
>> since we just fail to probe.
>
> Thank you for reviewing.
>
> Yes, we can skip it.
>
>>
>>> +
>>> + switch (type) {
>>> + case SYSRESET_WARM:
>>> + reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;
>>> + break;
>>> + case SYSRESET_COLD:
>>> + reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;
>>> + break;
>>> + case SYSRESET_POWER_OFF:
>>> + reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;
>>> + break;
>>> + default:
>>> + log_err("SBI has no system reset extension\n");
>>> + return -ENOSYS;
>>> + }
>>> +
>>> + sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
>>> +
>>> + return -EINPROGRESS;
>>> +}
>>> +
>>> +efi_status_t efi_reset_system_init(void)
>>> +{
>>> + return EFI_SUCCESS;
>>> +}
>>> +
>>> +void __efi_runtime EFIAPI efi_reset_system(enum efi_reset_type type,
>>> + efi_status_t reset_status,
>>> + unsigned long data_size,
>>> + void *reset_data)
>>
>> As an aside, is there a reason why the generic (weak) efi_reset_system
>> does not just call sysreset_walk_halt(type)?
>
> efi_reset_system is invoked at UEFI runtime. Most functions of U-Boot
> are no longer in memory after ExitBootServices(). Only functions in the
> __efi_runtime section may be called.
>
>>
>>> +{
>>> + enum sbi_srst_reset_type reset_type;
>>> +
>>> + if (have_reset)
>>> + switch (type) {
>>> + case SYSRESET_WARM:
>>> + reset_type = SBI_SRST_RESET_TYPE_WARM_REBOOT;
>>> + break;
>>> + case SYSRESET_COLD:
>>> + reset_type = SBI_SRST_RESET_TYPE_COLD_REBOOT;
>>> + break;
>>> + case SYSRESET_POWER_OFF:
>>> + reset_type = SBI_SRST_RESET_TYPE_SHUTDOWN;
>>> + break;
>>> + default:
>>> + goto hang;
>>
>> Why do we hang by default? For comparison, sysreset_x86 has
>>
>> if (reset_type == EFI_RESET_COLD ||
>> reset_type == EFI_RESET_PLATFORM_SPECIFIC)
>> value = SYS_RST | RST_CPU | FULL_RST;
>> else /* assume EFI_RESET_WARM since we cannot return an error */
>> value = SYS_RST | RST_CPU;
>
> ok
>
>>
>>> + }
>>> +
>>> + sbi_srst_reset(reset_type, SBI_SRST_RESET_REASON_NONE);
>>
>> Should we instead do
>>
>> enum sbi_srst_reset_reason reset_reason;
>> if (reset_status == EFI_SUCCESS)
>> reset_reason = SBI_SRST_RESET_REASON_NONE;
>> else
>> reset_reason = SBI_SRST_RESET_REASON_SYS_FAILURE;
>> sbi_srst_reset(reset_type, reset_status == EFI_SUCCESS ?
>> SBI_SRST_RESET_REASON_NONE : SBI_SRST_RESET_REASON_SYS_FAILURE);
>>
>> since we have reset status available?
>
> Makes sense.
>
>>
>>> +
>>> +hang:
>>> + while (1)
>>> + ;
>>> +}
>>> +
>>> +static int sbi_sysreset_probe(struct udevice *dev)
>>> +{
>>> + have_reset = sbi_probe_extension(SBI_EXT_SRST);
>>> + if (have_reset)
>>> + return 0;
>>> +
>>> + log_err("SBI has no system reset extension\n");
>>
>> Is this an error? It's perfectly normal for SBI implementations to lack
>> support for some extensions. Perhaps a warning/info would be better.
>
> We shouldn't have chosen this driver in the configuration if the SBI
> does not support it.
Yes, but we would like to use the same config for differing SBI
implementations (e.g. say a vendor's SBI implementation and SBI).
> I will change this to a warning.
Great.
>
>>
>>> + return -ENOENT;
>>> +}
>>> +
>>> +static const struct udevice_id sbi_sysreset_ids[] = {
>>> + { .compatible = "riscv" },
>>
>> This compatible string is already in-use for RISC-V CPUs. And if this
>> isn't supposed to have a device tree binding, do we even need of_match?
>
> I discussed this with Simon in
>
> https://lists.denx.de/pipermail/u-boot/2021-March/443270.html
>
> Instead of adding code to the board files we should have a device-tree
> node. A reasonable compatible string would be "riscv,sbi-sysreset".
Ok, so will patch 5 have the board parts dropped?
--Sean
>>
>>> + { }
>>> +};
>>> +
>>> +static struct sysreset_ops sbi_sysreset_ops = {
>>> + .request = sbi_sysreset_request,
>>> +};
>>> +
>>> +U_BOOT_DRIVER(sbi_sysreset) = {
>>> + .name = "sbi-sysreset",
>>> + .id = UCLASS_SYSRESET,
>>> + .of_match = sbi_sysreset_ids,
>>> + .ops = &sbi_sysreset_ops,
>>> + .probe = sbi_sysreset_probe,
>>> +};
>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>> index e729f727df..7e76435339 100644
>>> --- a/lib/efi_loader/Kconfig
>>> +++ b/lib/efi_loader/Kconfig
>>> @@ -277,7 +277,7 @@ config EFI_HAVE_RUNTIME_RESET
>>> bool
>>> default y
>>> depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || \
>>> - SANDBOX || SYSRESET_X86
>>> + SANDBOX || SYSRESET_SBI || SYSRESET_X86
>>
>> As a general note, perhaps it is better to set this from other Kconfigs?
>> How long will this list get?
>
> This seems to be a matter of taste.
>
> Best regards
>
> Heinrich
>
>>
>> --Sean
>>
>>>
>>> config EFI_GRUB_ARM32_WORKAROUND
>>> bool "Workaround for GRUB on 32bit ARM"
>>> --
>>> 2.30.1
>>>
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/5] risv: add missing SBI extension definitions
2021-03-06 5:59 ` Bin Meng
@ 2021-03-06 21:48 ` Sean Anderson
0 siblings, 0 replies; 19+ messages in thread
From: Sean Anderson @ 2021-03-06 21:48 UTC (permalink / raw)
To: u-boot
On 3/6/21 12:59 AM, Bin Meng wrote:
> On Fri, Mar 5, 2021 at 1:02 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> Add the System Reset Extension and the Hart State Management Extension
>> definitions.
>>
>> Add missing RFENCE Extension enum values.
>>
>> The SBI 0.1 extension constants are needed for for the sbi command. Remove
>> an #ifdef.
>>
>> Cf. https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> arch/riscv/include/asm/sbi.h | 36 ++++++++++++++++++++++++++++++++++--
>> 1 file changed, 34 insertions(+), 2 deletions(-)
>>
>
> U-Boot does not need HSM.
(yet)
As far as I can tell, it's completely conforming for all harts but one
to be put into the STOPPED state by the SBI (with the expectation that
U-Boot or something else start them). Also, while we currently figure
out which harts to start via device tree passed from SBI, that behavior
is not standardized. Going forward, I we may see some SBI
implementations which communicate this sort of info through HSM. Though
this has not happened yet...
--Sean
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/5] cmd/sbi: use constants instead of numerical values
2021-03-04 17:00 ` [PATCH 2/5] cmd/sbi: use constants instead of numerical values Heinrich Schuchardt
2021-03-04 23:31 ` Sean Anderson
@ 2021-03-08 6:25 ` Leo Liang
1 sibling, 0 replies; 19+ messages in thread
From: Leo Liang @ 2021-03-08 6:25 UTC (permalink / raw)
To: u-boot
On Thu, Mar 04, 2021 at 05:00:48PM +0000, Heinrich Schuchardt wrote:
> Use constants for extension IDs.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> cmd/riscv/sbi.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/cmd/riscv/sbi.c b/cmd/riscv/sbi.c
> index 90c0811e14..65a2c93290 100644
> --- a/cmd/riscv/sbi.c
> +++ b/cmd/riscv/sbi.c
> @@ -29,21 +29,21 @@ static struct sbi_imp implementations[] = {
> };
>
> static struct sbi_ext extensions[] = {
> - { 0x00000000, "sbi_set_timer" },
> - { 0x00000001, "sbi_console_putchar" },
> - { 0x00000002, "sbi_console_getchar" },
> - { 0x00000003, "sbi_clear_ipi" },
> - { 0x00000004, "sbi_send_ipi" },
> - { 0x00000005, "sbi_remote_fence_i" },
> - { 0x00000006, "sbi_remote_sfence_vma" },
> - { 0x00000007, "sbi_remote_sfence_vma_asid" },
> - { 0x00000008, "sbi_shutdown" },
> - { 0x00000010, "SBI Base Functionality" },
> - { 0x54494D45, "Timer Extension" },
> - { 0x00735049, "IPI Extension" },
> - { 0x52464E43, "RFENCE Extension" },
> - { 0x0048534D, "Hart State Management Extension" },
> - { 0x53525354, "System Reset Extension" },
> + { SBI_EXT_0_1_SET_TIMER, "sbi_set_timer" },
> + { SBI_EXT_0_1_CONSOLE_PUTCHAR, "sbi_console_putchar" },
> + { SBI_EXT_0_1_CONSOLE_GETCHAR, "sbi_console_getchar" },
> + { SBI_EXT_0_1_CLEAR_IPI, "sbi_clear_ipi" },
> + { SBI_EXT_0_1_SEND_IPI, "sbi_send_ipi" },
> + { SBI_EXT_0_1_REMOTE_FENCE_I, "sbi_remote_fence_i" },
> + { SBI_EXT_0_1_REMOTE_SFENCE_VMA, "sbi_remote_sfence_vma" },
> + { SBI_EXT_0_1_REMOTE_SFENCE_VMA_ASID, "sbi_remote_sfence_vma_asid" },
> + { SBI_EXT_0_1_SHUTDOWN, "sbi_shutdown" },
> + { SBI_EXT_BASE, "SBI Base Functionality" },
> + { SBI_EXT_TIME, "Timer Extension" },
> + { SBI_EXT_IPI, "IPI Extension" },
> + { SBI_EXT_RFENCE, "RFENCE Extension" },
> + { SBI_EXT_HSM, "Hart State Management Extension" },
> + { SBI_EXT_SRST, "System Reset Extension" },
> };
>
> static int do_sbi(struct cmd_tbl *cmdtp, int flag, int argc,
> --
> 2.30.1
>
Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/5] pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON
2021-03-04 17:00 ` [PATCH 4/5] pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON Heinrich Schuchardt
2021-03-04 23:15 ` Sean Anderson
@ 2021-03-08 6:29 ` Leo Liang
1 sibling, 0 replies; 19+ messages in thread
From: Leo Liang @ 2021-03-08 6:29 UTC (permalink / raw)
To: u-boot
On Thu, Mar 04, 2021 at 05:00:50PM +0000, Heinrich Schuchardt wrote:
> Select missing Kconfig dependencies.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> drivers/pinctrl/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 77fb851114..2a859ab5d0 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -294,6 +294,8 @@ config ASPEED_AST2500_PINCTRL
> config PINCTRL_K210
> bool "Kendryte K210 Fully-Programmable Input/Output Array driver"
> depends on DM && PINCTRL_GENERIC
> + select REGMAP
> + select SYSCON
> help
> Support pin multiplexing on the K210. The "FPIOA" can remap any
> supported function to any multifunctional IO pin. It can also perform
> --
> 2.30.1
>
Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-03-08 6:29 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-04 17:00 [PATCH 0/5] riscv: enable SBI system reset Heinrich Schuchardt
2021-03-04 17:00 ` [PATCH 1/5] risv: add missing SBI extension definitions Heinrich Schuchardt
2021-03-04 23:28 ` Sean Anderson
2021-03-06 5:33 ` Heinrich Schuchardt
2021-03-06 5:59 ` Bin Meng
2021-03-06 21:48 ` Sean Anderson
2021-03-04 17:00 ` [PATCH 2/5] cmd/sbi: use constants instead of numerical values Heinrich Schuchardt
2021-03-04 23:31 ` Sean Anderson
2021-03-08 6:25 ` Leo Liang
2021-03-04 17:00 ` [PATCH 3/5] sysreset: provide SBI based sysreset driver Heinrich Schuchardt
2021-03-04 23:50 ` Sean Anderson
2021-03-06 7:18 ` Heinrich Schuchardt
2021-03-06 16:40 ` Sean Anderson
2021-03-04 17:00 ` [PATCH 4/5] pinctrl: K210_PINCTRL depends on REGMAP and on SYSCON Heinrich Schuchardt
2021-03-04 23:15 ` Sean Anderson
2021-03-08 6:29 ` Leo Liang
2021-03-04 17:00 ` [PATCH 5/5] maix: enable SBI system reset for MAIX Heinrich Schuchardt
2021-03-04 23:24 ` Sean Anderson
2021-03-06 7:31 ` Heinrich Schuchardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.