2021-02-21 09:41:53

by Anup Patel

Subject: [RFC PATCH 0/8] RISC-V CPU Idle Support

This series adds RISC-V CPU Idle support using SBI HSM suspend function.
The RISC-V SBI CPU idle driver added by this series is highly inspired
from the ARM PSCI CPU idle driver.

At high-level, this series includes the following changes:
1) Preparatory arch/riscv patches (Patches 1 to 3)
2) Defines for RISC-V SBI HSM suspend (Patch 4)
3) Preparatory patch to shared code between RISC-V SBI CPU idle driver
and ARM PSCI CPU idle driver (Patch 5)
4) RISC-V SBI CPU idle driver and related DT bindings (Patches 6 to 7)

These patches can be found in riscv_sbi_hsm_suspend_v1 branch at
https://github.com/avpatel/linux

The proposed SBI HSM suspend definition can be found in hsm_suspend_v3
branch at https://github.com/avpatel/riscv-sbi-doc

The OpenSBI implementation of SBI HSM suspend function can be found
in hsm_suspend_v1 branch at https://github.com/avpatel/opensbi

Special thanks Sandeep Tripathy for providing early feeback on SBI HSM
support in all above projects (RISC-V SBI specification, OpenSBI, and
Linux RISC-V).

Anup Patel (8):
RISC-V: Enable CPU_IDLE drivers
RISC-V: Rename relocate() and make it global
RISC-V: Add arch functions for non-retentive suspend entry/exit
RISC-V: Add SBI HSM suspend related defines
cpuidle: Factor-out power domain related code from PSCI domain driver
cpuidle: Add RISC-V SBI CPU idle driver
dt-bindings: Add bindings documentation for RISC-V idle states
RISC-V: Enable RISC-V SBI CPU Idle driver for QEMU virt machine

.../bindings/riscv/idle-states.yaml | 250 +++++++++
MAINTAINERS | 8 +
arch/riscv/Kconfig | 7 +
arch/riscv/Kconfig.socs | 3 +
arch/riscv/configs/defconfig | 8 +-
arch/riscv/configs/rv32_defconfig | 5 +-
arch/riscv/include/asm/cpuidle.h | 24 +
arch/riscv/include/asm/sbi.h | 27 +-
arch/riscv/include/asm/suspend.h | 35 ++
arch/riscv/kernel/Makefile | 2 +
arch/riscv/kernel/asm-offsets.c | 3 +
arch/riscv/kernel/cpu_ops_sbi.c | 2 +-
arch/riscv/kernel/head.S | 7 +-
arch/riscv/kernel/process.c | 3 +-
arch/riscv/kernel/suspend.c | 86 +++
arch/riscv/kernel/suspend_entry.S | 116 ++++
drivers/cpuidle/Kconfig | 9 +
drivers/cpuidle/Kconfig.arm | 1 +
drivers/cpuidle/Kconfig.riscv | 15 +
drivers/cpuidle/Makefile | 5 +
drivers/cpuidle/cpuidle-psci-domain.c | 244 +--------
drivers/cpuidle/cpuidle-psci.h | 15 +-
drivers/cpuidle/cpuidle-sbi.c | 503 ++++++++++++++++++
...{cpuidle-psci-domain.c => dt_idle_genpd.c} | 165 ++----
drivers/cpuidle/dt_idle_genpd.h | 42 ++
25 files changed, 1218 insertions(+), 367 deletions(-)
create mode 100644 Documentation/devicetree/bindings/riscv/idle-states.yaml
create mode 100644 arch/riscv/include/asm/cpuidle.h
create mode 100644 arch/riscv/include/asm/suspend.h
create mode 100644 arch/riscv/kernel/suspend.c
create mode 100644 arch/riscv/kernel/suspend_entry.S
create mode 100644 drivers/cpuidle/Kconfig.riscv
create mode 100644 drivers/cpuidle/cpuidle-sbi.c
copy drivers/cpuidle/{cpuidle-psci-domain.c => dt_idle_genpd.c} (52%)
create mode 100644 drivers/cpuidle/dt_idle_genpd.h

--
2.25.1

2021-02-21 09:42:17

by Anup Patel

Subject: [RFC PATCH 1/8] RISC-V: Enable CPU_IDLE drivers

We force select CPU_PM and provide asm/cpuidle.h so that we can
use CPU IDLE drivers for Linux RISC-V kernel.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/Kconfig | 7 +++++++
arch/riscv/configs/defconfig | 7 +++----
arch/riscv/configs/rv32_defconfig | 4 ++--
arch/riscv/include/asm/cpuidle.h | 24 ++++++++++++++++++++++++
arch/riscv/kernel/process.c | 3 ++-
5 files changed, 38 insertions(+), 7 deletions(-)
create mode 100644 arch/riscv/include/asm/cpuidle.h

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index fe6862b06ead..4901200b6b6c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -37,6 +37,7 @@ config RISCV
select CLONE_BACKWARDS
select CLINT_TIMER if !MMU
select COMMON_CLK
+ select CPU_PM if CPU_IDLE
select EDAC_SUPPORT
select GENERIC_ARCH_TOPOLOGY if SMP
select GENERIC_ATOMIC64 if !64BIT
@@ -430,4 +431,10 @@ source "kernel/power/Kconfig"

endmenu

+menu "CPU Power Management"
+
+source "drivers/cpuidle/Kconfig"
+
+endmenu
+
source "drivers/firmware/Kconfig"
diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 6c0625aa96c7..dc4927c0e44b 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -13,11 +13,13 @@ CONFIG_USER_NS=y
CONFIG_CHECKPOINT_RESTORE=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_EXPERT=y
+# CONFIG_SYSFS_SYSCALL is not set
CONFIG_BPF_SYSCALL=y
CONFIG_SOC_SIFIVE=y
CONFIG_SOC_VIRT=y
CONFIG_SMP=y
CONFIG_HOTPLUG_CPU=y
+CONFIG_CPU_IDLE=y
CONFIG_JUMP_LABEL=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
@@ -65,10 +67,9 @@ CONFIG_HW_RANDOM=y
CONFIG_HW_RANDOM_VIRTIO=y
CONFIG_SPI=y
CONFIG_SPI_SIFIVE=y
+# CONFIG_PTP_1588_CLOCK is not set
CONFIG_GPIOLIB=y
CONFIG_GPIO_SIFIVE=y
-# CONFIG_PTP_1588_CLOCK is not set
-CONFIG_POWER_RESET=y
CONFIG_DRM=y
CONFIG_DRM_RADEON=y
CONFIG_DRM_VIRTIO_GPU=y
@@ -132,5 +133,3 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y
# CONFIG_FTRACE is not set
# CONFIG_RUNTIME_TESTING_MENU is not set
CONFIG_MEMTEST=y
-# CONFIG_SYSFS_SYSCALL is not set
-CONFIG_EFI=y
diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
index 8dd02b842fef..332e43a4a2c3 100644
--- a/arch/riscv/configs/rv32_defconfig
+++ b/arch/riscv/configs/rv32_defconfig
@@ -13,12 +13,14 @@ CONFIG_USER_NS=y
CONFIG_CHECKPOINT_RESTORE=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_EXPERT=y
+# CONFIG_SYSFS_SYSCALL is not set
CONFIG_BPF_SYSCALL=y
CONFIG_SOC_SIFIVE=y
CONFIG_SOC_VIRT=y
CONFIG_ARCH_RV32I=y
CONFIG_SMP=y
CONFIG_HOTPLUG_CPU=y
+CONFIG_CPU_IDLE=y
CONFIG_JUMP_LABEL=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
@@ -67,7 +69,6 @@ CONFIG_HW_RANDOM_VIRTIO=y
CONFIG_SPI=y
CONFIG_SPI_SIFIVE=y
# CONFIG_PTP_1588_CLOCK is not set
-CONFIG_POWER_RESET=y
CONFIG_DRM=y
CONFIG_DRM_RADEON=y
CONFIG_DRM_VIRTIO_GPU=y
@@ -131,4 +132,3 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y
# CONFIG_FTRACE is not set
# CONFIG_RUNTIME_TESTING_MENU is not set
CONFIG_MEMTEST=y
-# CONFIG_SYSFS_SYSCALL is not set
diff --git a/arch/riscv/include/asm/cpuidle.h b/arch/riscv/include/asm/cpuidle.h
new file mode 100644
index 000000000000..1042d790e446
--- /dev/null
+++ b/arch/riscv/include/asm/cpuidle.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2021 Allwinner Ltd
+ * Copyright (C) 2021 Western Digital Corporation or its affiliates.
+ */
+
+#ifndef _ASM_RISCV_CPUIDLE_H
+#define _ASM_RISCV_CPUIDLE_H
+
+#include <asm/barrier.h>
+#include <asm/processor.h>
+
+static inline void cpu_do_idle(void)
+{
+ /*
+ * Add mb() here to ensure that all
+ * IO/MEM access are completed prior
+ * to enter WFI.
+ */
+ mb();
+ wait_for_interrupt();
+}
+
+#endif
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index dd5f985b1f40..b5b51fd26624 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -21,6 +21,7 @@
#include <asm/string.h>
#include <asm/switch_to.h>
#include <asm/thread_info.h>
+#include <asm/cpuidle.h>

register unsigned long gp_in_global __asm__("gp");

@@ -35,7 +36,7 @@ extern asmlinkage void ret_from_kernel_thread(void);

void arch_cpu_idle(void)
{
- wait_for_interrupt();
+ cpu_do_idle();
raw_local_irq_enable();
}

--
2.25.1

2021-02-21 09:42:38

by Anup Patel

Subject: [RFC PATCH 2/8] RISC-V: Rename relocate() and make it global

The low-level relocate() function enables mmu and relocates
execution to link-time addresses. We rename relocate() function
to relocate_enable_mmu() function which is more informative.

Also, the relocate_enable_mmu() function will be used in the
resume path when a CPU wakes-up from a non-retentive suspend
so we make it global symbol.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/kernel/head.S | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 16e9941900c4..a8aca43929d8 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -67,7 +67,8 @@ pe_head_start:

.align 2
#ifdef CONFIG_MMU
-relocate:
+ .global relocate_enable_mmu
+relocate_enable_mmu:
/* Relocate return address */
li a1, PAGE_OFFSET
la a2, _start
@@ -156,7 +157,7 @@ secondary_start_common:
#ifdef CONFIG_MMU
/* Enable virtual memory and relocate to virtual address */
la a0, swapper_pg_dir
- call relocate
+ call relocate_enable_mmu
#endif
call setup_trap_vector
tail smp_callin
@@ -264,7 +265,7 @@ clear_bss_done:
call setup_vm
#ifdef CONFIG_MMU
la a0, early_pg_dir
- call relocate
+ call relocate_enable_mmu
#endif /* CONFIG_MMU */

call setup_trap_vector
--
2.25.1

2021-02-21 09:42:40

by Anup Patel

Subject: [RFC PATCH 3/8] RISC-V: Add arch functions for non-retentive suspend entry/exit

The hart registers and CSRs are not preserved in non-retentative
suspend state so we provide arch specific helper functions which
will save/restore hart context upon entry/exit to non-retentive
suspend state. These helper functions can be used by cpuidle
drivers for non-retentive suspend entry/exit.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/suspend.h | 35 +++++++++
arch/riscv/kernel/Makefile | 2 +
arch/riscv/kernel/asm-offsets.c | 3 +
arch/riscv/kernel/suspend.c | 86 ++++++++++++++++++++++
arch/riscv/kernel/suspend_entry.S | 116 ++++++++++++++++++++++++++++++
5 files changed, 242 insertions(+)
create mode 100644 arch/riscv/include/asm/suspend.h
create mode 100644 arch/riscv/kernel/suspend.c
create mode 100644 arch/riscv/kernel/suspend_entry.S

diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
new file mode 100644
index 000000000000..63e9f434fb89
--- /dev/null
+++ b/arch/riscv/include/asm/suspend.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
+ */
+
+#ifndef _ASM_RISCV_SUSPEND_H
+#define _ASM_RISCV_SUSPEND_H
+
+#include <asm/ptrace.h>
+
+struct suspend_context {
+ /* Saved and restored by low-level functions */
+ struct pt_regs regs;
+ /* Saved and restored by high-level functions */
+ unsigned long scratch;
+ unsigned long tvec;
+ unsigned long ie;
+#ifdef CONFIG_MMU
+ unsigned long satp;
+#endif
+};
+
+/* Low-level CPU suspend entry function */
+int __cpu_suspend_enter(struct suspend_context *context);
+
+/* High-level CPU suspend which will save context and call finish() */
+int cpu_suspend(unsigned long arg,
+ int (*finish)(unsigned long arg,
+ unsigned long entry,
+ unsigned long context));
+
+/* Low-level CPU resume entry function */
+int __cpu_resume_enter(unsigned long hartid, unsigned long context);
+
+#endif
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index f6caf4d9ca15..d83038bf93b2 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -40,6 +40,8 @@ obj-$(CONFIG_SMP) += cpu_ops_spinwait.o
obj-$(CONFIG_MODULES) += module.o
obj-$(CONFIG_MODULE_SECTIONS) += module-sections.o

+obj-$(CONFIG_CPU_PM) += suspend_entry.o suspend.o
+
obj-$(CONFIG_FUNCTION_TRACER) += mcount.o ftrace.o
obj-$(CONFIG_DYNAMIC_FTRACE) += mcount-dyn.o

diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index b79ffa3561fd..8259f5f16da8 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -10,6 +10,7 @@
#include <linux/sched.h>
#include <asm/thread_info.h>
#include <asm/ptrace.h>
+#include <asm/suspend.h>

void asm_offsets(void);

@@ -108,6 +109,8 @@ void asm_offsets(void)
OFFSET(PT_BADADDR, pt_regs, badaddr);
OFFSET(PT_CAUSE, pt_regs, cause);

+ OFFSET(SUSPEND_CONTEXT_REGS, suspend_context, regs);
+
/*
* THREAD_{F,X}* might be larger than a S-type offset can handle, but
* these are used in performance-sensitive assembly so we can't resort
diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
new file mode 100644
index 000000000000..49dddec30e99
--- /dev/null
+++ b/arch/riscv/kernel/suspend.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
+ */
+
+#include <linux/ftrace.h>
+#include <asm/csr.h>
+#include <asm/suspend.h>
+
+static void suspend_save_csrs(struct suspend_context *context)
+{
+ context->scratch = csr_read(CSR_SCRATCH);
+ context->tvec = csr_read(CSR_TVEC);
+ context->ie = csr_read(CSR_IE);
+
+ /*
+ * No need to save/restore IP CSR (i.e. MIP or SIP) because:
+ *
+ * 1. For no-MMU (M-mode) kernel, the bits in MIP are set by
+ * external devices (such as interrupt controller, timer, etc).
+ * 2. For MMU (S-mode) kernel, the bits in SIP are set by
+ * M-mode firmware and external devices (such as interrupt
+ * controller, etc).
+ */
+
+#ifdef CONFIG_MMU
+ context->satp = csr_read(CSR_SATP);
+#endif
+}
+
+static void suspend_restore_csrs(struct suspend_context *context)
+{
+ csr_write(CSR_SCRATCH, context->scratch);
+ csr_write(CSR_TVEC, context->tvec);
+ csr_write(CSR_IE, context->ie);
+
+#ifdef CONFIG_MMU
+ csr_write(CSR_SATP, context->satp);
+#endif
+}
+
+int cpu_suspend(unsigned long arg,
+ int (*finish)(unsigned long arg,
+ unsigned long entry,
+ unsigned long context))
+{
+ int rc = 0;
+ struct suspend_context context = { 0 };
+
+ /* Finisher should be non-NULL */
+ if (!finish)
+ return -EINVAL;
+
+ /* Save additional CSRs*/
+ suspend_save_csrs(&context);
+
+ /*
+ * Function graph tracer state gets incosistent when the kernel
+ * calls functions that never return (aka finishers) hence disable
+ * graph tracing during their execution.
+ */
+ pause_graph_tracing();
+
+ /* Save context on stack */
+ if (__cpu_suspend_enter(&context)) {
+ /* Call the finisher */
+ rc = finish(arg, __pa_symbol(__cpu_resume_enter),
+ (ulong)&context);
+
+ /*
+ * Should never reach here, unless the suspend finisher
+ * fails. Successful cpu_suspend() should return from
+ * __cpu_resume_entry()
+ */
+ if (!rc)
+ rc = -EOPNOTSUPP;
+ }
+
+ /* Enable function graph tracer */
+ unpause_graph_tracing();
+
+ /* Restore additional CSRs */
+ suspend_restore_csrs(&context);
+
+ return rc;
+}
diff --git a/arch/riscv/kernel/suspend_entry.S b/arch/riscv/kernel/suspend_entry.S
new file mode 100644
index 000000000000..dee85c86e177
--- /dev/null
+++ b/arch/riscv/kernel/suspend_entry.S
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
+ */
+
+#include <linux/linkage.h>
+#include <asm/asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/csr.h>
+
+ .text
+ .altmacro
+ .option norelax
+
+ENTRY(__cpu_suspend_enter)
+ /* Save registers (except A0 and T0-T6) */
+ REG_S ra, (SUSPEND_CONTEXT_REGS + PT_RA)(a0)
+ REG_S sp, (SUSPEND_CONTEXT_REGS + PT_SP)(a0)
+ REG_S gp, (SUSPEND_CONTEXT_REGS + PT_GP)(a0)
+ REG_S tp, (SUSPEND_CONTEXT_REGS + PT_TP)(a0)
+ REG_S s0, (SUSPEND_CONTEXT_REGS + PT_S0)(a0)
+ REG_S s1, (SUSPEND_CONTEXT_REGS + PT_S1)(a0)
+ REG_S a1, (SUSPEND_CONTEXT_REGS + PT_A1)(a0)
+ REG_S a2, (SUSPEND_CONTEXT_REGS + PT_A2)(a0)
+ REG_S a3, (SUSPEND_CONTEXT_REGS + PT_A3)(a0)
+ REG_S a4, (SUSPEND_CONTEXT_REGS + PT_A4)(a0)
+ REG_S a5, (SUSPEND_CONTEXT_REGS + PT_A5)(a0)
+ REG_S a6, (SUSPEND_CONTEXT_REGS + PT_A6)(a0)
+ REG_S a7, (SUSPEND_CONTEXT_REGS + PT_A7)(a0)
+ REG_S s2, (SUSPEND_CONTEXT_REGS + PT_S2)(a0)
+ REG_S s3, (SUSPEND_CONTEXT_REGS + PT_S3)(a0)
+ REG_S s4, (SUSPEND_CONTEXT_REGS + PT_S4)(a0)
+ REG_S s5, (SUSPEND_CONTEXT_REGS + PT_S5)(a0)
+ REG_S s6, (SUSPEND_CONTEXT_REGS + PT_S6)(a0)
+ REG_S s7, (SUSPEND_CONTEXT_REGS + PT_S7)(a0)
+ REG_S s8, (SUSPEND_CONTEXT_REGS + PT_S8)(a0)
+ REG_S s9, (SUSPEND_CONTEXT_REGS + PT_S9)(a0)
+ REG_S s10, (SUSPEND_CONTEXT_REGS + PT_S10)(a0)
+ REG_S s11, (SUSPEND_CONTEXT_REGS + PT_S11)(a0)
+
+ /* Save CSRs */
+ csrr t0, CSR_EPC
+ REG_S t0, (SUSPEND_CONTEXT_REGS + PT_EPC)(a0)
+ csrr t0, CSR_STATUS
+ REG_S t0, (SUSPEND_CONTEXT_REGS + PT_STATUS)(a0)
+ csrr t0, CSR_TVAL
+ REG_S t0, (SUSPEND_CONTEXT_REGS + PT_BADADDR)(a0)
+ csrr t0, CSR_CAUSE
+ REG_S t0, (SUSPEND_CONTEXT_REGS + PT_CAUSE)(a0)
+
+ /* Return non-zero value */
+ li a0, 1
+
+ /* Return to C code */
+ ret
+END(__cpu_suspend_enter)
+
+ENTRY(__cpu_resume_enter)
+#ifdef CONFIG_MMU
+ /* Save A0 and A1 */
+ add t0, a0, zero
+ add t1, a1, zero
+
+ /* Enable MMU */
+ la a0, swapper_pg_dir
+ call relocate_enable_mmu
+
+ /* Restore A0 and A1 */
+ add a0, t0, zero
+ add a1, t1, zero
+#endif
+
+ /* Make A0 point to suspend context */
+ add a0, a1, zero
+
+ /* Restore CSRs */
+ REG_L t0, (SUSPEND_CONTEXT_REGS + PT_EPC)(a0)
+ csrw CSR_EPC, t0
+ REG_L t0, (SUSPEND_CONTEXT_REGS + PT_STATUS)(a0)
+ csrw CSR_STATUS, t0
+ REG_L t0, (SUSPEND_CONTEXT_REGS + PT_BADADDR)(a0)
+ csrw CSR_TVAL, t0
+ REG_L t0, (SUSPEND_CONTEXT_REGS + PT_CAUSE)(a0)
+ csrw CSR_CAUSE, t0
+
+ /* Restore registers (except A0 and T0-T6) */
+ REG_L ra, (SUSPEND_CONTEXT_REGS + PT_RA)(a0)
+ REG_L sp, (SUSPEND_CONTEXT_REGS + PT_SP)(a0)
+ REG_L gp, (SUSPEND_CONTEXT_REGS + PT_GP)(a0)
+ REG_L tp, (SUSPEND_CONTEXT_REGS + PT_TP)(a0)
+ REG_L s0, (SUSPEND_CONTEXT_REGS + PT_S0)(a0)
+ REG_L s1, (SUSPEND_CONTEXT_REGS + PT_S1)(a0)
+ REG_L a1, (SUSPEND_CONTEXT_REGS + PT_A1)(a0)
+ REG_L a2, (SUSPEND_CONTEXT_REGS + PT_A2)(a0)
+ REG_L a3, (SUSPEND_CONTEXT_REGS + PT_A3)(a0)
+ REG_L a4, (SUSPEND_CONTEXT_REGS + PT_A4)(a0)
+ REG_L a5, (SUSPEND_CONTEXT_REGS + PT_A5)(a0)
+ REG_L a6, (SUSPEND_CONTEXT_REGS + PT_A6)(a0)
+ REG_L a7, (SUSPEND_CONTEXT_REGS + PT_A7)(a0)
+ REG_L s2, (SUSPEND_CONTEXT_REGS + PT_S2)(a0)
+ REG_L s3, (SUSPEND_CONTEXT_REGS + PT_S3)(a0)
+ REG_L s4, (SUSPEND_CONTEXT_REGS + PT_S4)(a0)
+ REG_L s5, (SUSPEND_CONTEXT_REGS + PT_S5)(a0)
+ REG_L s6, (SUSPEND_CONTEXT_REGS + PT_S6)(a0)
+ REG_L s7, (SUSPEND_CONTEXT_REGS + PT_S7)(a0)
+ REG_L s8, (SUSPEND_CONTEXT_REGS + PT_S8)(a0)
+ REG_L s9, (SUSPEND_CONTEXT_REGS + PT_S9)(a0)
+ REG_L s10, (SUSPEND_CONTEXT_REGS + PT_S10)(a0)
+ REG_L s11, (SUSPEND_CONTEXT_REGS + PT_S11)(a0)
+
+ /* Return zero value */
+ add a0, zero, zero
+
+ /* Return to C code */
+ ret
+END(__cpu_resume_enter)
--
2.25.1

2021-02-21 09:43:09

by Anup Patel

Subject: [RFC PATCH 6/8] cpuidle: Add RISC-V SBI CPU idle driver

The RISC-V SBI HSM extension provides HSM suspend call which can
be used by Linux RISC-V to enter platform specific low-power state.

This patch adds a CPU idle driver based on RISC-V SBI calls which
will populate idle states from device tree and use SBI calls to
entry these idle states.

Signed-off-by: Anup Patel <[email protected]>
---
MAINTAINERS | 8 +
drivers/cpuidle/Kconfig | 5 +
drivers/cpuidle/Kconfig.riscv | 15 +
drivers/cpuidle/Makefile | 4 +
drivers/cpuidle/cpuidle-sbi.c | 503 ++++++++++++++++++++++++++++++++++
5 files changed, 535 insertions(+)
create mode 100644 drivers/cpuidle/Kconfig.riscv
create mode 100644 drivers/cpuidle/cpuidle-sbi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 667d03852191..eeeab188a8ac 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4641,6 +4641,14 @@ S: Supported
F: drivers/cpuidle/cpuidle-psci.h
F: drivers/cpuidle/cpuidle-psci-domain.c

+CPUIDLE DRIVER - RISC-V SBI
+M: Anup Patel <[email protected]>
+R: Sandeep Tripathy <[email protected]>
+L: [email protected]
+L: [email protected]
+S: Supported
+F: drivers/cpuidle/cpuidle-sbi.c
+
CRAMFS FILESYSTEM
M: Nicolas Pitre <[email protected]>
S: Maintained
diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index f1afe7ab6b54..ff71dd662880 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -66,6 +66,11 @@ depends on PPC
source "drivers/cpuidle/Kconfig.powerpc"
endmenu

+menu "RISC-V CPU Idle Drivers"
+depends on RISCV
+source "drivers/cpuidle/Kconfig.riscv"
+endmenu
+
config HALTPOLL_CPUIDLE
tristate "Halt poll cpuidle driver"
depends on X86 && KVM_GUEST
diff --git a/drivers/cpuidle/Kconfig.riscv b/drivers/cpuidle/Kconfig.riscv
new file mode 100644
index 000000000000..78518c26af74
--- /dev/null
+++ b/drivers/cpuidle/Kconfig.riscv
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# RISC-V CPU Idle drivers
+#
+
+config RISCV_SBI_CPUIDLE
+ bool "RISC-V SBI CPU idle Driver"
+ depends on RISCV_SBI
+ select DT_IDLE_STATES
+ select CPU_IDLE_MULTIPLE_DRIVERS
+ select DT_IDLE_GENPD if PM_GENERIC_DOMAINS_OF
+ help
+ Select this option to enable RISC-V SBI firmware based CPU idle
+ driver for RISC-V systems. This drivers also supports hierarchical
+ DT based layout of the idle state.
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 11a26cef279f..a36922c18510 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -35,3 +35,7 @@ obj-$(CONFIG_MIPS_CPS_CPUIDLE) += cpuidle-cps.o
# POWERPC drivers
obj-$(CONFIG_PSERIES_CPUIDLE) += cpuidle-pseries.o
obj-$(CONFIG_POWERNV_CPUIDLE) += cpuidle-powernv.o
+
+###############################################################################
+# RISC-V drivers
+obj-$(CONFIG_RISCV_SBI_CPUIDLE) += cpuidle-sbi.o
diff --git a/drivers/cpuidle/cpuidle-sbi.c b/drivers/cpuidle/cpuidle-sbi.c
new file mode 100644
index 000000000000..abbcabca0e91
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-sbi.c
@@ -0,0 +1,503 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * RISC-V SBI CPU idle driver.
+ *
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
+ */
+
+#define pr_fmt(fmt) "cpuidle-sbi: " fmt
+
+#include <linux/cpuidle.h>
+#include <linux/cpumask.h>
+#include <linux/cpu_pm.h>
+#include <linux/cpu_cooling.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <asm/cpuidle.h>
+#include <asm/sbi.h>
+#include <asm/suspend.h>
+
+#include "dt_idle_states.h"
+#include "dt_idle_genpd.h"
+
+struct sbi_cpuidle_data {
+ u32 *states;
+ struct device *dev;
+};
+
+struct sbi_domain_state {
+ bool available;
+ u32 state;
+};
+
+static DEFINE_PER_CPU_READ_MOSTLY(struct sbi_cpuidle_data, sbi_cpuidle_data);
+static DEFINE_PER_CPU(struct sbi_domain_state, domain_state);
+static bool sbi_cpuidle_use_osi;
+static bool sbi_cpuidle_use_cpuhp;
+static bool sbi_cpuidle_pd_allow_domain_state;
+
+static inline void sbi_set_domain_state(u32 state)
+{
+ struct sbi_domain_state *data = this_cpu_ptr(&domain_state);
+
+ data->available = true;
+ data->state = state;
+}
+
+static inline u32 sbi_get_domain_state(void)
+{
+ struct sbi_domain_state *data = this_cpu_ptr(&domain_state);
+
+ return data->state;
+}
+
+static inline void sbi_clear_domain_state(void)
+{
+ struct sbi_domain_state *data = this_cpu_ptr(&domain_state);
+
+ data->available = false;
+}
+
+static inline bool sbi_is_domain_state_available(void)
+{
+ struct sbi_domain_state *data = this_cpu_ptr(&domain_state);
+
+ return data->available;
+}
+
+static int sbi_suspend_finisher(unsigned long suspend_type,
+ unsigned long resume_addr,
+ unsigned long opaque)
+{
+ struct sbiret ret;
+
+ ret = sbi_ecall(SBI_EXT_HSM, SBI_EXT_HSM_HART_SUSPEND,
+ suspend_type, resume_addr, opaque, 0, 0, 0);
+
+ return (ret.error) ? sbi_err_map_linux_errno(ret.error) : 0;
+}
+
+static int sbi_suspend(u32 state)
+{
+ if (state & SBI_HSM_SUSP_NON_RET_BIT)
+ return cpu_suspend(state, sbi_suspend_finisher);
+ else
+ return sbi_suspend_finisher(state, 0, 0);
+}
+
+static int sbi_cpuidle_enter_state(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int idx)
+{
+ u32 *states = __this_cpu_read(sbi_cpuidle_data.states);
+
+ return CPU_PM_CPU_IDLE_ENTER_PARAM(sbi_suspend, idx, states[idx]);
+}
+
+static int __sbi_enter_domain_idle_state(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int idx,
+ bool s2idle)
+{
+ struct sbi_cpuidle_data *data = this_cpu_ptr(&sbi_cpuidle_data);
+ u32 *states = data->states;
+ struct device *pd_dev = data->dev;
+ u32 state;
+ int ret;
+
+ ret = cpu_pm_enter();
+ if (ret)
+ return -1;
+
+ /* Do runtime PM to manage a hierarchical CPU toplogy. */
+ rcu_irq_enter_irqson();
+ if (s2idle)
+ dev_pm_genpd_suspend(pd_dev);
+ else
+ pm_runtime_put_sync_suspend(pd_dev);
+ rcu_irq_exit_irqson();
+
+ if (sbi_is_domain_state_available())
+ state = sbi_get_domain_state();
+ else
+ state = states[idx];
+
+ ret = sbi_suspend(state) ? -1 : idx;
+
+ rcu_irq_enter_irqson();
+ if (s2idle)
+ dev_pm_genpd_resume(pd_dev);
+ else
+ pm_runtime_get_sync(pd_dev);
+ rcu_irq_exit_irqson();
+
+ cpu_pm_exit();
+
+ /* Clear the domain state to start fresh when back from idle. */
+ sbi_clear_domain_state();
+ return ret;
+}
+
+static int sbi_enter_domain_idle_state(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int idx)
+{
+ return __sbi_enter_domain_idle_state(dev, drv, idx, false);
+}
+
+static int sbi_enter_s2idle_domain_idle_state(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int idx)
+{
+ return __sbi_enter_domain_idle_state(dev, drv, idx, true);
+}
+
+static int sbi_cpuidle_cpuhp_up(unsigned int cpu)
+{
+ struct device *pd_dev = __this_cpu_read(sbi_cpuidle_data.dev);
+
+ if (pd_dev)
+ pm_runtime_get_sync(pd_dev);
+
+ return 0;
+}
+
+static int sbi_cpuidle_cpuhp_down(unsigned int cpu)
+{
+ struct device *pd_dev = __this_cpu_read(sbi_cpuidle_data.dev);
+
+ if (pd_dev) {
+ pm_runtime_put_sync(pd_dev);
+ /* Clear domain state to start fresh at next online. */
+ sbi_clear_domain_state();
+ }
+
+ return 0;
+}
+
+static void sbi_idle_init_cpuhp(void)
+{
+ int err;
+
+ if (!sbi_cpuidle_use_cpuhp)
+ return;
+
+ err = cpuhp_setup_state_nocalls(CPUHP_AP_CPU_PM_STARTING,
+ "cpuidle/sbi:online",
+ sbi_cpuidle_cpuhp_up,
+ sbi_cpuidle_cpuhp_down);
+ if (err)
+ pr_warn("Failed %d while setup cpuhp state\n", err);
+}
+
+static const struct of_device_id sbi_cpuidle_state_match[] = {
+ { .compatible = "riscv,idle-state",
+ .data = sbi_cpuidle_enter_state },
+ { },
+};
+
+static bool sbi_suspend_state_is_valid(u32 state)
+{
+ if (state > SBI_HSM_SUSPEND_RET_DEFAULT &&
+ state < SBI_HSM_SUSPEND_RET_PLATFORM)
+ return false;
+ if (state > SBI_HSM_SUSPEND_NON_RET_DEFAULT &&
+ state < SBI_HSM_SUSPEND_NON_RET_PLATFORM)
+ return false;
+ return true;
+}
+
+static int sbi_dt_parse_state_node(struct device_node *np, u32 *state)
+{
+ int err = of_property_read_u32(np, "riscv,sbi-suspend-param", state);
+
+ if (err) {
+ pr_warn("%pOF missing riscv,sbi-suspend-param property\n", np);
+ return err;
+ }
+
+ if (!sbi_suspend_state_is_valid(*state)) {
+ pr_warn("Invalid SBI suspend state %#x\n", *state);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int sbi_dt_cpu_init_topology(struct cpuidle_driver *drv,
+ struct sbi_cpuidle_data *data,
+ unsigned int state_count, int cpu)
+{
+ /* Currently limit the hierarchical topology to be used in OSI mode. */
+ if (!sbi_cpuidle_use_osi)
+ return 0;
+
+ data->dev = dt_idle_genpd_attach_cpu(cpu, "sbi");
+ if (IS_ERR_OR_NULL(data->dev))
+ return PTR_ERR_OR_ZERO(data->dev);
+
+ /*
+ * Using the deepest state for the CPU to trigger a potential selection
+ * of a shared state for the domain, assumes the domain states are all
+ * deeper states.
+ */
+ drv->states[state_count - 1].enter = sbi_enter_domain_idle_state;
+ drv->states[state_count - 1].enter_s2idle = sbi_enter_s2idle_domain_idle_state;
+ sbi_cpuidle_use_cpuhp = true;
+
+ return 0;
+}
+
+static int sbi_cpuidle_dt_init_states(struct device *dev,
+ struct cpuidle_driver *drv,
+ unsigned int cpu,
+ unsigned int state_count)
+{
+ struct sbi_cpuidle_data *data = per_cpu_ptr(&sbi_cpuidle_data, cpu);
+ struct device_node *state_node;
+ struct device_node *cpu_node;
+ u32 *states;
+ int i, ret;
+
+ cpu_node = of_cpu_device_node_get(cpu);
+ if (!cpu_node)
+ return -ENODEV;
+
+ states = devm_kcalloc(dev, state_count, sizeof(*states), GFP_KERNEL);
+ if (!states) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+ /* Parse SBI specific details from state DT nodes */
+ for (i = 1; i < state_count; i++) {
+ state_node = of_get_cpu_state_node(cpu_node, i - 1);
+ if (!state_node)
+ break;
+
+ ret = sbi_dt_parse_state_node(state_node, &states[i]);
+ of_node_put(state_node);
+
+ if (ret)
+ return ret;
+
+ pr_debug("sbi-state %#x index %d\n", states[i], i);
+ }
+ if (i != state_count) {
+ ret = -ENODEV;
+ goto fail;
+ }
+
+ /* Initialize optional data, used for the hierarchical topology. */
+ ret = sbi_dt_cpu_init_topology(drv, data, state_count, cpu);
+ if (ret < 0)
+ return ret;
+
+ /* Store states in the per-cpu struct. */
+ data->states = states;
+
+fail:
+ of_node_put(cpu_node);
+
+ return ret;
+}
+
+static void sbi_cpuidle_deinit_cpu(int cpu)
+{
+ struct sbi_cpuidle_data *data = per_cpu_ptr(&sbi_cpuidle_data, cpu);
+
+ dt_idle_genpd_detach_cpu(data->dev);
+ sbi_cpuidle_use_cpuhp = false;
+}
+
+static int sbi_cpuidle_init_cpu(struct device *dev, int cpu)
+{
+ struct cpuidle_driver *drv;
+ unsigned int state_count = 0;
+ int ret = 0;
+
+ drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
+ if (!drv)
+ return -ENOMEM;
+
+ drv->name = "sbi_cpuidle";
+ drv->owner = THIS_MODULE;
+ drv->cpumask = (struct cpumask *)cpumask_of(cpu);
+
+ /* RISC-V architectural WFI to be represented as state index 0. */
+ drv->states[0].enter = sbi_cpuidle_enter_state;
+ drv->states[0].exit_latency = 1;
+ drv->states[0].target_residency = 1;
+ drv->states[0].power_usage = UINT_MAX;
+ strcpy(drv->states[0].name, "WFI");
+ strcpy(drv->states[0].desc, "RISC-V WFI");
+
+ /*
+ * If no DT idle states are detected (ret == 0) let the driver
+ * initialization fail accordingly since there is no reason to
+ * initialize the idle driver if only wfi is supported, the
+ * default archictectural back-end already executes wfi
+ * on idle entry.
+ */
+ ret = dt_init_idle_driver(drv, sbi_cpuidle_state_match, 1);
+ if (ret <= 0) {
+ pr_debug("HART%ld: failed to parse DT idle states\n",
+ cpuid_to_hartid_map(cpu));
+ return ret ? : -ENODEV;
+ }
+ state_count = ret + 1; /* Include WFI state as well */
+
+ /* Initialize idle states from DT. */
+ ret = sbi_cpuidle_dt_init_states(dev, drv, cpu, state_count);
+ if (ret) {
+ pr_err("HART%ld: failed to init idle states\n",
+ cpuid_to_hartid_map(cpu));
+ return ret;
+ }
+
+ ret = cpuidle_register(drv, NULL);
+ if (ret)
+ goto deinit;
+
+ cpuidle_cooling_register(drv);
+
+ return 0;
+deinit:
+ sbi_cpuidle_deinit_cpu(cpu);
+ return ret;
+}
+
+static int sbi_cpuidle_pd_power_off(struct generic_pm_domain *pd)
+{
+ struct genpd_power_state *state = &pd->states[pd->state_idx];
+ u32 *pd_state;
+
+ if (!state->data)
+ return 0;
+
+ if (!sbi_cpuidle_pd_allow_domain_state)
+ return -EBUSY;
+
+ /* OSI mode is enabled, set the corresponding domain state. */
+ pd_state = state->data;
+ sbi_set_domain_state(*pd_state);
+
+ return 0;
+}
+
+static void sbi_cpuidle_domain_sync_state(struct device *dev)
+{
+ /*
+ * All devices have now been attached/probed to the PM domain
+ * topology, hence it's fine to allow domain states to be picked.
+ */
+ sbi_cpuidle_pd_allow_domain_state = true;
+}
+
+static struct dt_idle_genpd_ops sbi_genpd_ops = {
+ .parse_state_node = sbi_dt_parse_state_node,
+};
+
+static int sbi_cpuidle_probe(struct platform_device *pdev)
+{
+ int cpu, ret;
+ struct cpuidle_driver *drv;
+ struct cpuidle_device *dev;
+ struct device_node *np, *pds_node;
+
+ /* Detect OSI support based on CPU DT nodes */
+ sbi_cpuidle_use_osi = true;
+ for_each_possible_cpu(cpu) {
+ np = of_cpu_device_node_get(cpu);
+ if (np &&
+ of_find_property(np, "power-domains", NULL) &&
+ of_find_property(np, "power-domain-names", NULL)) {
+ continue;
+ } else {
+ sbi_cpuidle_use_osi = false;
+ break;
+ }
+ }
+
+ if (sbi_cpuidle_use_osi)
+ sbi_genpd_ops.power_off = sbi_cpuidle_pd_power_off;
+
+ /* Populate generic power domains from DT nodes */
+ pds_node = of_find_node_by_path("/cpus/sbi-power-domains");
+ if (pds_node) {
+ ret = dt_idle_genpd_probe(&sbi_genpd_ops, pds_node);
+ of_node_put(pds_node);
+ if (ret)
+ return ret;
+ }
+
+ /* Initialize CPU idle driver for each CPU */
+ for_each_possible_cpu(cpu) {
+ ret = sbi_cpuidle_init_cpu(&pdev->dev, cpu);
+ if (ret) {
+ pr_debug("HART%ld: idle driver init failed\n",
+ cpuid_to_hartid_map(cpu));
+ goto out_fail;
+ }
+ }
+
+ /* Setup CPU hotplut notifiers */
+ sbi_idle_init_cpuhp();
+
+ pr_info("idle driver registered for all CPUs\n");
+
+ return 0;
+
+out_fail:
+ while (--cpu >= 0) {
+ dev = per_cpu(cpuidle_devices, cpu);
+ drv = cpuidle_get_cpu_driver(dev);
+ cpuidle_unregister(drv);
+ sbi_cpuidle_deinit_cpu(cpu);
+ }
+
+ return ret;
+}
+
+static struct platform_driver sbi_cpuidle_driver = {
+ .probe = sbi_cpuidle_probe,
+ .driver = {
+ .name = "sbi-cpuidle",
+ .sync_state = sbi_cpuidle_domain_sync_state,
+ },
+};
+
+static int __init sbi_cpuidle_init(void)
+{
+ int ret;
+ struct platform_device *pdev;
+
+ /*
+ * The SBI HSM suspend function is only available when:
+ * 1) SBI HSM extension is available
+ * 2) SBI version is 0.3 or higher
+ */
+ if (sbi_major_version() < 0 ||
+ sbi_minor_version() < 3 ||
+ sbi_probe_extension(SBI_EXT_HSM) <= 0) {
+ pr_info("HSM suspend not available\n");
+ return 0;
+ }
+
+ ret = platform_driver_register(&sbi_cpuidle_driver);
+ if (ret)
+ return ret;
+
+ pdev = platform_device_register_simple("sbi-cpuidle",
+ -1, NULL, 0);
+ if (IS_ERR(pdev)) {
+ platform_driver_unregister(&sbi_cpuidle_driver);
+ return PTR_ERR(pdev);
+ }
+
+ return 0;
+}
+device_initcall(sbi_cpuidle_init);
--
2.25.1

2021-02-21 09:43:46

by Anup Patel

Subject: [RFC PATCH 7/8] dt-bindings: Add bindings documentation for RISC-V idle states

The RISC-V CPU idle states will be described in DT under the
/cpus/riscv-idle-states DT node. This patch adds the bindings
documentation for riscv-idle-states DT nodes and idle state DT
nodes under it.

Signed-off-by: Anup Patel <[email protected]>
---
.../bindings/riscv/idle-states.yaml | 250 ++++++++++++++++++
1 file changed, 250 insertions(+)
create mode 100644 Documentation/devicetree/bindings/riscv/idle-states.yaml

diff --git a/Documentation/devicetree/bindings/riscv/idle-states.yaml b/Documentation/devicetree/bindings/riscv/idle-states.yaml
new file mode 100644
index 000000000000..3eff763fed23
--- /dev/null
+++ b/Documentation/devicetree/bindings/riscv/idle-states.yaml
@@ -0,0 +1,250 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/riscv/idle-states.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V idle states binding description
+
+maintainers:
+ - Anup Patel <[email protected]>
+
+description: |+
+ RISC-V systems can manage power consumption dynamically, where HARTs
+ (or CPUs) [1] can be put in different platform specific suspend (or
+ idle) states (ranging from simple WFI, power gating, etc). The RISC-V
+ SBI [2] hart state management extension provides a standard mechanism
+ for OSes to request HART state transitions.
+
+ The platform specific suspend (or idle) states of a hart can be either
+ retentive or non-rententive in nature. A retentive suspend state will
+ preserve hart register and CSR values for all privilege modes whereas
+ a non-retentive suspend state will not preserve hart register and CSR
+ values. The suspend (or idle) state entered by executing the WFI
+ instruction is considered standard on all RISC-V systems and therefore
+ must not be listed in device tree.
+
+ The device tree binding definition for RISC-V idle states described
+ in this document is quite similar to the ARM idle states [3].
+
+ References
+
+ [1] RISC-V Linux Kernel documentation - CPUs bindings
+ Documentation/devicetree/bindings/riscv/cpus.yaml
+
+ [2] RISC-V Supervisor Binary Interface (SBI)
+ http://github.com/riscv/riscv-sbi-doc/riscv-sbi.adoc
+
+ [3] ARM idle states binding description - Idle states bindings
+ Documentation/devicetree/bindings/arm/idle-states.yaml
+
+properties:
+ $nodename:
+ const: riscv-idle-states
+
+patternProperties:
+ "^(cpu|cluster)-":
+ type: object
+ description: |
+ Each state node represents an idle state description and must be
+ defined as follows.
+
+ properties:
+ compatible:
+ const: riscv,idle-state
+
+ local-timer-stop:
+ description:
+ If present the CPU local timer control logic is lost on state
+ entry, otherwise it is retained.
+ type: boolean
+
+ entry-latency-us:
+ description:
+ Worst case latency in microseconds required to enter the idle state.
+
+ exit-latency-us:
+ description:
+ Worst case latency in microseconds required to exit the idle state.
+ The exit-latency-us duration may be guaranteed only after
+ entry-latency-us has passed.
+
+ min-residency-us:
+ description:
+ Minimum residency duration in microseconds, inclusive of preparation
+ and entry, for this idle state to be considered worthwhile energy
+ wise (refer to section 2 of this document for a complete description).
+
+ wakeup-latency-us:
+ description: |
+ Maximum delay between the signaling of a wake-up event and the CPU
+ being able to execute normal code again. If omitted, this is assumed
+ to be equal to:
+
+ entry-latency-us + exit-latency-us
+
+ It is important to supply this value on systems where the duration
+ of PREP phase (see diagram 1, section 2) is non-neglibigle. In such
+ systems entry-latency-us + exit-latency-us will exceed
+ wakeup-latency-us by this duration.
+
+ idle-state-name:
+ $ref: /schemas/types.yaml#/definitions/string
+ description:
+ A string used as a descriptive name for the idle state.
+
+ required:
+ - compatible
+ - entry-latency-us
+ - exit-latency-us
+ - min-residency-us
+
+additionalProperties: false
+
+examples:
+ - |
+
+ cpus {
+ #size-cells = <0>;
+ #address-cells = <1>;
+
+ cpu@0 {
+ device_type = "cpu";
+ compatible = "riscv";
+ reg = <0x0>;
+ riscv,isa = "rv64imafdc";
+ mmu-type = "riscv,sv48";
+ cpu-idle-states = <&CPU_RET_0_0 &CPU_NONRET_0_0
+ &CLUSTER_RET_0 &CLUSTER_NONRET_0>;
+
+ cpu_intc0: interrupt-controller {
+ #interrupt-cells = <1>;
+ compatible = "riscv,cpu-intc";
+ interrupt-controller;
+ };
+ };
+
+ cpu@1 {
+ device_type = "cpu";
+ compatible = "riscv";
+ reg = <0x1>;
+ riscv,isa = "rv64imafdc";
+ mmu-type = "riscv,sv48";
+ cpu-idle-states = <&CPU_RET_0_0 &CPU_NONRET_0_0
+ &CLUSTER_RET_0 &CLUSTER_NONRET_0>;
+
+ cpu_intc1: interrupt-controller {
+ #interrupt-cells = <1>;
+ compatible = "riscv,cpu-intc";
+ interrupt-controller;
+ };
+ };
+
+ cpu@10 {
+ device_type = "cpu";
+ compatible = "riscv";
+ reg = <0x10>;
+ riscv,isa = "rv64imafdc";
+ mmu-type = "riscv,sv48";
+ cpu-idle-states = <&CPU_RET_1_0 &CPU_NONRET_1_0
+ &CLUSTER_RET_1 &CLUSTER_NONRET_1>;
+
+ cpu_intc10: interrupt-controller {
+ #interrupt-cells = <1>;
+ compatible = "riscv,cpu-intc";
+ interrupt-controller;
+ };
+ };
+
+ cpu@11 {
+ device_type = "cpu";
+ compatible = "riscv";
+ reg = <0x11>;
+ riscv,isa = "rv64imafdc";
+ mmu-type = "riscv,sv48";
+ cpu-idle-states = <&CPU_RET_1_0 &CPU_NONRET_1_0
+ &CLUSTER_RET_1 &CLUSTER_NONRET_1>;
+
+ cpu_intc11: interrupt-controller {
+ #interrupt-cells = <1>;
+ compatible = "riscv,cpu-intc";
+ interrupt-controller;
+ };
+ };
+
+ riscv-idle-states {
+ CPU_RET_0_0: cpu-retentive-0-0 {
+ compatible = "riscv,idle-state";
+ riscv,sbi-suspend-param = <0x10000000>;
+ entry-latency-us = <20>;
+ exit-latency-us = <40>;
+ min-residency-us = <80>;
+ };
+
+ CPU_NONRET_0_0: cpu-nonretentive-0-0 {
+ compatible = "riscv,idle-state";
+ riscv,sbi-suspend-param = <0x90000000>;
+ entry-latency-us = <250>;
+ exit-latency-us = <500>;
+ min-residency-us = <950>;
+ };
+
+ CLUSTER_RET_0: cluster-retentive-0 {
+ compatible = "riscv,idle-state";
+ riscv,sbi-suspend-param = <0x11000000>;
+ local-timer-stop;
+ entry-latency-us = <50>;
+ exit-latency-us = <100>;
+ min-residency-us = <250>;
+ wakeup-latency-us = <130>;
+ };
+
+ CLUSTER_NONRET_0: cluster-nonretentive-0 {
+ compatible = "riscv,idle-state";
+ riscv,sbi-suspend-param = <0x91000000>;
+ local-timer-stop;
+ entry-latency-us = <600>;
+ exit-latency-us = <1100>;
+ min-residency-us = <2700>;
+ wakeup-latency-us = <1500>;
+ };
+
+ CPU_RET_1_0: cpu-retentive-1-0 {
+ compatible = "riscv,idle-state";
+ riscv,sbi-suspend-param = <0x10000010>;
+ entry-latency-us = <20>;
+ exit-latency-us = <40>;
+ min-residency-us = <80>;
+ };
+
+ CPU_NONRET_1_0: cpu-nonretentive-1-0 {
+ compatible = "riscv,idle-state";
+ riscv,sbi-suspend-param = <0x90000010>;
+ entry-latency-us = <250>;
+ exit-latency-us = <500>;
+ min-residency-us = <950>;
+ };
+
+ CLUSTER_RET_1: cluster-retentive-1 {
+ compatible = "riscv,idle-state";
+ riscv,sbi-suspend-param = <0x11000010>;
+ local-timer-stop;
+ entry-latency-us = <50>;
+ exit-latency-us = <100>;
+ min-residency-us = <250>;
+ wakeup-latency-us = <130>;
+ };
+
+ CLUSTER_NONRET_1: cluster-nonretentive-1 {
+ compatible = "riscv,idle-state";
+ riscv,sbi-suspend-param = <0x91000010>;
+ local-timer-stop;
+ entry-latency-us = <600>;
+ exit-latency-us = <1100>;
+ min-residency-us = <2700>;
+ wakeup-latency-us = <1500>;
+ };
+ };
+ };
+
+...
--
2.25.1

2021-02-21 09:43:47

by Anup Patel

Subject: [RFC PATCH 4/8] RISC-V: Add SBI HSM suspend related defines

We add defines related to SBI HSM suspend call and also
update HSM states naming as-per latest SBI specification.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/include/asm/sbi.h | 27 ++++++++++++++++++++++-----
arch/riscv/kernel/cpu_ops_sbi.c | 2 +-
2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 5b2d6d614c20..4f101f1aa4ea 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -62,15 +62,32 @@ enum sbi_ext_hsm_fid {
SBI_EXT_HSM_HART_START = 0,
SBI_EXT_HSM_HART_STOP,
SBI_EXT_HSM_HART_STATUS,
+ SBI_EXT_HSM_HART_SUSPEND,
};

-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_hsm_hart_state {
+ SBI_HSM_STATE_STARTED = 0,
+ SBI_HSM_STATE_STOPPED,
+ SBI_HSM_STATE_START_PENDING,
+ SBI_HSM_STATE_STOP_PENDING,
+ SBI_HSM_STATE_SUSPENDED,
+ SBI_HSM_STATE_SUSPEND_PENDING,
+ SBI_HSM_STATE_RESUME_PENDING,
};

+#define SBI_HSM_SUSP_BASE_MASK 0x7fffffff
+#define SBI_HSM_SUSP_NON_RET_BIT 0x80000000
+#define SBI_HSM_SUSP_PLAT_BASE 0x10000000
+
+#define SBI_HSM_SUSPEND_RET_DEFAULT 0x00000000
+#define SBI_HSM_SUSPEND_RET_PLATFORM SBI_HSM_SUSP_PLAT_BASE
+#define SBI_HSM_SUSPEND_RET_LAST SBI_HSM_SUSP_BASE_MASK
+#define SBI_HSM_SUSPEND_NON_RET_DEFAULT SBI_HSM_SUSP_NON_RET_BIT
+#define SBI_HSM_SUSPEND_NON_RET_PLATFORM (SBI_HSM_SUSP_NON_RET_BIT | \
+ SBI_HSM_SUSP_PLAT_BASE)
+#define SBI_HSM_SUSPEND_NON_RET_LAST (SBI_HSM_SUSP_NON_RET_BIT | \
+ SBI_HSM_SUSP_BASE_MASK)
+
enum sbi_ext_srst_fid {
SBI_EXT_SRST_RESET = 0,
};
diff --git a/arch/riscv/kernel/cpu_ops_sbi.c b/arch/riscv/kernel/cpu_ops_sbi.c
index 685fae72b7f5..5fd90f03a3e9 100644
--- a/arch/riscv/kernel/cpu_ops_sbi.c
+++ b/arch/riscv/kernel/cpu_ops_sbi.c
@@ -97,7 +97,7 @@ static int sbi_cpu_is_stopped(unsigned int cpuid)

rc = sbi_hsm_hart_get_status(hartid);

- if (rc == SBI_HSM_HART_STATUS_STOPPED)
+ if (rc == SBI_HSM_STATE_STOPPED)
return 0;
return rc;
}
--
2.25.1

2021-02-21 09:43:59

by Anup Patel

Subject: [RFC PATCH 5/8] cpuidle: Factor-out power domain related code from PSCI domain driver

The generic power domain related code in PSCI domain driver is largely
independent of PSCI and can be shared with RISC-V SBI domain driver
hence we factor-out this code into dt_idle_genpd.c and dt_idle_genpd.h.

Signed-off-by: Anup Patel <[email protected]>
---
drivers/cpuidle/Kconfig | 4 +
drivers/cpuidle/Kconfig.arm | 1 +
drivers/cpuidle/Makefile | 1 +
drivers/cpuidle/cpuidle-psci-domain.c | 244 +-----------------
drivers/cpuidle/cpuidle-psci.h | 15 +-
...{cpuidle-psci-domain.c => dt_idle_genpd.c} | 165 ++++--------
drivers/cpuidle/dt_idle_genpd.h | 42 +++
7 files changed, 121 insertions(+), 351 deletions(-)
copy drivers/cpuidle/{cpuidle-psci-domain.c => dt_idle_genpd.c} (52%)
create mode 100644 drivers/cpuidle/dt_idle_genpd.h

diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index c0aeedd66f02..f1afe7ab6b54 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -47,6 +47,10 @@ config CPU_IDLE_GOV_HALTPOLL
config DT_IDLE_STATES
bool

+config DT_IDLE_GENPD
+ depends on PM_GENERIC_DOMAINS_OF
+ bool
+
menu "ARM CPU Idle Drivers"
depends on ARM || ARM64
source "drivers/cpuidle/Kconfig.arm"
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 0844fadc4be8..1007435ae298 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -27,6 +27,7 @@ config ARM_PSCI_CPUIDLE_DOMAIN
bool "PSCI CPU idle Domain"
depends on ARM_PSCI_CPUIDLE
depends on PM_GENERIC_DOMAINS_OF
+ select DT_IDLE_GENPD
default y
help
Select this to enable the PSCI based CPUidle driver to use PM domains,
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 26bbc5e74123..11a26cef279f 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -6,6 +6,7 @@
obj-y += cpuidle.o driver.o governor.o sysfs.o governors/
obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
obj-$(CONFIG_DT_IDLE_STATES) += dt_idle_states.o
+obj-$(CONFIG_DT_IDLE_GENPD) += dt_idle_genpd.o
obj-$(CONFIG_ARCH_HAS_CPU_RELAX) += poll_state.o
obj-$(CONFIG_HALTPOLL_CPUIDLE) += cpuidle-haltpoll.o

diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c
index ff2c3f8e4668..b0621d890ab7 100644
--- a/drivers/cpuidle/cpuidle-psci-domain.c
+++ b/drivers/cpuidle/cpuidle-psci-domain.c
@@ -16,17 +16,9 @@
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
#include <linux/psci.h>
-#include <linux/slab.h>
-#include <linux/string.h>

#include "cpuidle-psci.h"

-struct psci_pd_provider {
- struct list_head link;
- struct device_node *node;
-};
-
-static LIST_HEAD(psci_pd_providers);
static bool psci_pd_allow_domain_state;

static int psci_pd_power_off(struct generic_pm_domain *pd)
@@ -47,178 +39,6 @@ static int psci_pd_power_off(struct generic_pm_domain *pd)
return 0;
}

-static int psci_pd_parse_state_nodes(struct genpd_power_state *states,
- int state_count)
-{
- int i, ret;
- u32 psci_state, *psci_state_buf;
-
- for (i = 0; i < state_count; i++) {
- ret = psci_dt_parse_state_node(to_of_node(states[i].fwnode),
- &psci_state);
- if (ret)
- goto free_state;
-
- psci_state_buf = kmalloc(sizeof(u32), GFP_KERNEL);
- if (!psci_state_buf) {
- ret = -ENOMEM;
- goto free_state;
- }
- *psci_state_buf = psci_state;
- states[i].data = psci_state_buf;
- }
-
- return 0;
-
-free_state:
- i--;
- for (; i >= 0; i--)
- kfree(states[i].data);
- return ret;
-}
-
-static int psci_pd_parse_states(struct device_node *np,
- struct genpd_power_state **states, int *state_count)
-{
- int ret;
-
- /* Parse the domain idle states. */
- ret = of_genpd_parse_idle_states(np, states, state_count);
- if (ret)
- return ret;
-
- /* Fill out the PSCI specifics for each found state. */
- ret = psci_pd_parse_state_nodes(*states, *state_count);
- if (ret)
- kfree(*states);
-
- return ret;
-}
-
-static void psci_pd_free_states(struct genpd_power_state *states,
- unsigned int state_count)
-{
- int i;
-
- for (i = 0; i < state_count; i++)
- kfree(states[i].data);
- kfree(states);
-}
-
-static int psci_pd_init(struct device_node *np, bool use_osi)
-{
- struct generic_pm_domain *pd;
- struct psci_pd_provider *pd_provider;
- struct dev_power_governor *pd_gov;
- struct genpd_power_state *states = NULL;
- int ret = -ENOMEM, state_count = 0;
-
- pd = kzalloc(sizeof(*pd), GFP_KERNEL);
- if (!pd)
- goto out;
-
- pd_provider = kzalloc(sizeof(*pd_provider), GFP_KERNEL);
- if (!pd_provider)
- goto free_pd;
-
- pd->name = kasprintf(GFP_KERNEL, "%pOF", np);
- if (!pd->name)
- goto free_pd_prov;
-
- /*
- * Parse the domain idle states and let genpd manage the state selection
- * for those being compatible with "domain-idle-state".
- */
- ret = psci_pd_parse_states(np, &states, &state_count);
- if (ret)
- goto free_name;
-
- pd->free_states = psci_pd_free_states;
- pd->name = kbasename(pd->name);
- pd->states = states;
- pd->state_count = state_count;
- pd->flags |= GENPD_FLAG_IRQ_SAFE | GENPD_FLAG_CPU_DOMAIN;
-
- /* Allow power off when OSI has been successfully enabled. */
- if (use_osi)
- pd->power_off = psci_pd_power_off;
- else
- pd->flags |= GENPD_FLAG_ALWAYS_ON;
-
- /* Use governor for CPU PM domains if it has some states to manage. */
- pd_gov = state_count > 0 ? &pm_domain_cpu_gov : NULL;
-
- ret = pm_genpd_init(pd, pd_gov, false);
- if (ret) {
- psci_pd_free_states(states, state_count);
- goto free_name;
- }
-
- ret = of_genpd_add_provider_simple(np, pd);
- if (ret)
- goto remove_pd;
-
- pd_provider->node = of_node_get(np);
- list_add(&pd_provider->link, &psci_pd_providers);
-
- pr_debug("init PM domain %s\n", pd->name);
- return 0;
-
-remove_pd:
- pm_genpd_remove(pd);
-free_name:
- kfree(pd->name);
-free_pd_prov:
- kfree(pd_provider);
-free_pd:
- kfree(pd);
-out:
- pr_err("failed to init PM domain ret=%d %pOF\n", ret, np);
- return ret;
-}
-
-static void psci_pd_remove(void)
-{
- struct psci_pd_provider *pd_provider, *it;
- struct generic_pm_domain *genpd;
-
- list_for_each_entry_safe(pd_provider, it, &psci_pd_providers, link) {
- of_genpd_del_provider(pd_provider->node);
-
- genpd = of_genpd_remove_last(pd_provider->node);
- if (!IS_ERR(genpd))
- kfree(genpd);
-
- of_node_put(pd_provider->node);
- list_del(&pd_provider->link);
- kfree(pd_provider);
- }
-}
-
-static int psci_pd_init_topology(struct device_node *np)
-{
- struct device_node *node;
- struct of_phandle_args child, parent;
- int ret;
-
- for_each_child_of_node(np, node) {
- if (of_parse_phandle_with_args(node, "power-domains",
- "#power-domain-cells", 0, &parent))
- continue;
-
- child.np = node;
- child.args_count = 0;
- ret = of_genpd_add_subdomain(&parent, &child);
- of_node_put(parent.np);
- if (ret) {
- of_node_put(node);
- return ret;
- }
- }
-
- return 0;
-}
-
static bool psci_pd_try_set_osi_mode(void)
{
int ret;
@@ -244,6 +64,10 @@ static void psci_cpuidle_domain_sync_state(struct device *dev)
psci_pd_allow_domain_state = true;
}

+static struct dt_idle_genpd_ops psci_genpd_ops = {
+ .parse_state_node = psci_dt_parse_state_node,
+};
+
static const struct of_device_id psci_of_match[] = {
{ .compatible = "arm,psci-1.0" },
{}
@@ -252,48 +76,25 @@ static const struct of_device_id psci_of_match[] = {
static int psci_cpuidle_domain_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct device_node *node;
bool use_osi;
- int ret = 0, pd_count = 0;
+ int ret = 0;

if (!np)
return -ENODEV;

/* If OSI mode is supported, let's try to enable it. */
use_osi = psci_pd_try_set_osi_mode();
+ if (use_osi)
+ psci_genpd_ops.power_off = psci_pd_power_off;

- /*
- * Parse child nodes for the "#power-domain-cells" property and
- * initialize a genpd/genpd-of-provider pair when it's found.
- */
- for_each_child_of_node(np, node) {
- if (!of_find_property(node, "#power-domain-cells", NULL))
- continue;
-
- ret = psci_pd_init(node, use_osi);
- if (ret)
- goto put_node;
-
- pd_count++;
- }
-
- /* Bail out if not using the hierarchical CPU topology. */
- if (!pd_count)
- goto no_pd;
-
- /* Link genpd masters/subdomains to model the CPU topology. */
- ret = psci_pd_init_topology(np);
+ /* Generic power domain probing based on DT node. */
+ ret = dt_idle_genpd_probe(&psci_genpd_ops, np);
if (ret)
- goto remove_pd;
+ goto no_pd;

pr_info("Initialized CPU PM domain topology\n");
return 0;

-put_node:
- of_node_put(node);
-remove_pd:
- psci_pd_remove();
- pr_err("failed to create CPU PM domains ret=%d\n", ret);
no_pd:
if (use_osi)
psci_set_osi_mode(false);
@@ -314,28 +115,3 @@ static int __init psci_idle_init_domains(void)
return platform_driver_register(&psci_cpuidle_domain_driver);
}
subsys_initcall(psci_idle_init_domains);
-
-struct device *psci_dt_attach_cpu(int cpu)
-{
- struct device *dev;
-
- dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), "psci");
- if (IS_ERR_OR_NULL(dev))
- return dev;
-
- pm_runtime_irq_safe(dev);
- if (cpu_online(cpu))
- pm_runtime_get_sync(dev);
-
- dev_pm_syscore_device(dev, true);
-
- return dev;
-}
-
-void psci_dt_detach_cpu(struct device *dev)
-{
- if (IS_ERR_OR_NULL(dev))
- return;
-
- dev_pm_domain_detach(dev, false);
-}
diff --git a/drivers/cpuidle/cpuidle-psci.h b/drivers/cpuidle/cpuidle-psci.h
index d8e925e84c27..70de1e3c00af 100644
--- a/drivers/cpuidle/cpuidle-psci.h
+++ b/drivers/cpuidle/cpuidle-psci.h
@@ -10,8 +10,19 @@ void psci_set_domain_state(u32 state);
int psci_dt_parse_state_node(struct device_node *np, u32 *state);

#ifdef CONFIG_ARM_PSCI_CPUIDLE_DOMAIN
-struct device *psci_dt_attach_cpu(int cpu);
-void psci_dt_detach_cpu(struct device *dev);
+
+#include "dt_idle_genpd.h"
+
+static inline struct device *psci_dt_attach_cpu(int cpu)
+{
+ return dt_idle_genpd_attach_cpu(cpu, "psci");
+}
+
+static inline void psci_dt_detach_cpu(struct device *dev)
+{
+ dt_idle_genpd_detach_cpu(dev);
+}
+
#else
static inline struct device *psci_dt_attach_cpu(int cpu) { return NULL; }
static inline void psci_dt_detach_cpu(struct device *dev) { }
diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/dt_idle_genpd.c
similarity index 52%
copy from drivers/cpuidle/cpuidle-psci-domain.c
copy to drivers/cpuidle/dt_idle_genpd.c
index ff2c3f8e4668..805c4c81d60f 100644
--- a/drivers/cpuidle/cpuidle-psci-domain.c
+++ b/drivers/cpuidle/dt_idle_genpd.c
@@ -1,71 +1,52 @@
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0-only
/*
- * PM domains for CPUs via genpd - managed by cpuidle-psci.
+ * PM domains for CPUs via genpd.
*
* Copyright (C) 2019 Linaro Ltd.
* Author: Ulf Hansson <[email protected]>
*
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
*/

-#define pr_fmt(fmt) "CPUidle PSCI: " fmt
+#define pr_fmt(fmt) "dt-idle-genpd: " fmt

#include <linux/cpu.h>
#include <linux/device.h>
#include <linux/kernel.h>
-#include <linux/platform_device.h>
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
-#include <linux/psci.h>
#include <linux/slab.h>
#include <linux/string.h>

-#include "cpuidle-psci.h"
+#include "dt_idle_genpd.h"

-struct psci_pd_provider {
+struct dt_pd_provider {
struct list_head link;
struct device_node *node;
};

-static LIST_HEAD(psci_pd_providers);
-static bool psci_pd_allow_domain_state;
+static LIST_HEAD(dt_pd_providers);

-static int psci_pd_power_off(struct generic_pm_domain *pd)
-{
- struct genpd_power_state *state = &pd->states[pd->state_idx];
- u32 *pd_state;
-
- if (!state->data)
- return 0;
-
- if (!psci_pd_allow_domain_state)
- return -EBUSY;
-
- /* OSI mode is enabled, set the corresponding domain state. */
- pd_state = state->data;
- psci_set_domain_state(*pd_state);
-
- return 0;
-}
-
-static int psci_pd_parse_state_nodes(struct genpd_power_state *states,
- int state_count)
+static int dt_pd_parse_state_nodes(const struct dt_idle_genpd_ops *ops,
+ struct genpd_power_state *states,
+ int state_count)
{
int i, ret;
- u32 psci_state, *psci_state_buf;
+ u32 state, *state_buf;

for (i = 0; i < state_count; i++) {
- ret = psci_dt_parse_state_node(to_of_node(states[i].fwnode),
- &psci_state);
+ ret = ops->parse_state_node(to_of_node(states[i].fwnode),
+ &state);
if (ret)
goto free_state;

- psci_state_buf = kmalloc(sizeof(u32), GFP_KERNEL);
- if (!psci_state_buf) {
+ state_buf = kmalloc(sizeof(u32), GFP_KERNEL);
+ if (!state_buf) {
ret = -ENOMEM;
goto free_state;
}
- *psci_state_buf = psci_state;
- states[i].data = psci_state_buf;
+ *state_buf = state;
+ states[i].data = state_buf;
}

return 0;
@@ -77,8 +58,10 @@ static int psci_pd_parse_state_nodes(struct genpd_power_state *states,
return ret;
}

-static int psci_pd_parse_states(struct device_node *np,
- struct genpd_power_state **states, int *state_count)
+static int dt_pd_parse_states(const struct dt_idle_genpd_ops *ops,
+ struct device_node *np,
+ struct genpd_power_state **states,
+ int *state_count)
{
int ret;

@@ -87,15 +70,15 @@ static int psci_pd_parse_states(struct device_node *np,
if (ret)
return ret;

- /* Fill out the PSCI specifics for each found state. */
- ret = psci_pd_parse_state_nodes(*states, *state_count);
+ /* Fill out the dt specifics for each found state. */
+ ret = dt_pd_parse_state_nodes(ops, *states, *state_count);
if (ret)
kfree(*states);

return ret;
}

-static void psci_pd_free_states(struct genpd_power_state *states,
+static void dt_pd_free_states(struct genpd_power_state *states,
unsigned int state_count)
{
int i;
@@ -105,10 +88,11 @@ static void psci_pd_free_states(struct genpd_power_state *states,
kfree(states);
}

-static int psci_pd_init(struct device_node *np, bool use_osi)
+static int dt_pd_init(const struct dt_idle_genpd_ops *ops,
+ struct device_node *np)
{
struct generic_pm_domain *pd;
- struct psci_pd_provider *pd_provider;
+ struct dt_pd_provider *pd_provider;
struct dev_power_governor *pd_gov;
struct genpd_power_state *states = NULL;
int ret = -ENOMEM, state_count = 0;
@@ -129,19 +113,19 @@ static int psci_pd_init(struct device_node *np, bool use_osi)
* Parse the domain idle states and let genpd manage the state selection
* for those being compatible with "domain-idle-state".
*/
- ret = psci_pd_parse_states(np, &states, &state_count);
+ ret = dt_pd_parse_states(ops, np, &states, &state_count);
if (ret)
goto free_name;

- pd->free_states = psci_pd_free_states;
+ pd->free_states = dt_pd_free_states;
pd->name = kbasename(pd->name);
pd->states = states;
pd->state_count = state_count;
pd->flags |= GENPD_FLAG_IRQ_SAFE | GENPD_FLAG_CPU_DOMAIN;

- /* Allow power off when OSI has been successfully enabled. */
- if (use_osi)
- pd->power_off = psci_pd_power_off;
+ /* Allow power off when available. */
+ if (ops->power_off)
+ pd->power_off = ops->power_off;
else
pd->flags |= GENPD_FLAG_ALWAYS_ON;

@@ -150,7 +134,7 @@ static int psci_pd_init(struct device_node *np, bool use_osi)

ret = pm_genpd_init(pd, pd_gov, false);
if (ret) {
- psci_pd_free_states(states, state_count);
+ dt_pd_free_states(states, state_count);
goto free_name;
}

@@ -159,7 +143,7 @@ static int psci_pd_init(struct device_node *np, bool use_osi)
goto remove_pd;

pd_provider->node = of_node_get(np);
- list_add(&pd_provider->link, &psci_pd_providers);
+ list_add(&pd_provider->link, &dt_pd_providers);

pr_debug("init PM domain %s\n", pd->name);
return 0;
@@ -177,12 +161,12 @@ static int psci_pd_init(struct device_node *np, bool use_osi)
return ret;
}

-static void psci_pd_remove(void)
+static void dt_pd_remove(void)
{
- struct psci_pd_provider *pd_provider, *it;
+ struct dt_pd_provider *pd_provider, *it;
struct generic_pm_domain *genpd;

- list_for_each_entry_safe(pd_provider, it, &psci_pd_providers, link) {
+ list_for_each_entry_safe(pd_provider, it, &dt_pd_providers, link) {
of_genpd_del_provider(pd_provider->node);

genpd = of_genpd_remove_last(pd_provider->node);
@@ -195,7 +179,7 @@ static void psci_pd_remove(void)
}
}

-static int psci_pd_init_topology(struct device_node *np)
+static int dt_pd_init_topology(struct device_node *np)
{
struct device_node *node;
struct of_phandle_args child, parent;
@@ -219,49 +203,15 @@ static int psci_pd_init_topology(struct device_node *np)
return 0;
}

-static bool psci_pd_try_set_osi_mode(void)
-{
- int ret;
-
- if (!psci_has_osi_support())
- return false;
-
- ret = psci_set_osi_mode(true);
- if (ret) {
- pr_warn("failed to enable OSI mode: %d\n", ret);
- return false;
- }
-
- return true;
-}
-
-static void psci_cpuidle_domain_sync_state(struct device *dev)
+int dt_idle_genpd_probe(const struct dt_idle_genpd_ops *ops,
+ struct device_node *np)
{
- /*
- * All devices have now been attached/probed to the PM domain topology,
- * hence it's fine to allow domain states to be picked.
- */
- psci_pd_allow_domain_state = true;
-}
-
-static const struct of_device_id psci_of_match[] = {
- { .compatible = "arm,psci-1.0" },
- {}
-};
-
-static int psci_cpuidle_domain_probe(struct platform_device *pdev)
-{
- struct device_node *np = pdev->dev.of_node;
struct device_node *node;
- bool use_osi;
int ret = 0, pd_count = 0;

- if (!np)
+ if (!np || !ops || !ops->parse_state_node)
return -ENODEV;

- /* If OSI mode is supported, let's try to enable it. */
- use_osi = psci_pd_try_set_osi_mode();
-
/*
* Parse child nodes for the "#power-domain-cells" property and
* initialize a genpd/genpd-of-provider pair when it's found.
@@ -270,7 +220,7 @@ static int psci_cpuidle_domain_probe(struct platform_device *pdev)
if (!of_find_property(node, "#power-domain-cells", NULL))
continue;

- ret = psci_pd_init(node, use_osi);
+ ret = dt_pd_init(ops, node);
if (ret)
goto put_node;

@@ -282,44 +232,27 @@ static int psci_cpuidle_domain_probe(struct platform_device *pdev)
goto no_pd;

/* Link genpd masters/subdomains to model the CPU topology. */
- ret = psci_pd_init_topology(np);
+ ret = dt_pd_init_topology(np);
if (ret)
goto remove_pd;

- pr_info("Initialized CPU PM domain topology\n");
return 0;

put_node:
of_node_put(node);
remove_pd:
- psci_pd_remove();
+ dt_pd_remove();
pr_err("failed to create CPU PM domains ret=%d\n", ret);
no_pd:
- if (use_osi)
- psci_set_osi_mode(false);
return ret;
}
+EXPORT_SYMBOL_GPL(dt_idle_genpd_probe);

-static struct platform_driver psci_cpuidle_domain_driver = {
- .probe = psci_cpuidle_domain_probe,
- .driver = {
- .name = "psci-cpuidle-domain",
- .of_match_table = psci_of_match,
- .sync_state = psci_cpuidle_domain_sync_state,
- },
-};
-
-static int __init psci_idle_init_domains(void)
-{
- return platform_driver_register(&psci_cpuidle_domain_driver);
-}
-subsys_initcall(psci_idle_init_domains);
-
-struct device *psci_dt_attach_cpu(int cpu)
+struct device *dt_idle_genpd_attach_cpu(int cpu, const char *name)
{
struct device *dev;

- dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), "psci");
+ dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), name);
if (IS_ERR_OR_NULL(dev))
return dev;

@@ -331,11 +264,13 @@ struct device *psci_dt_attach_cpu(int cpu)

return dev;
}
+EXPORT_SYMBOL_GPL(dt_idle_genpd_attach_cpu);

-void psci_dt_detach_cpu(struct device *dev)
+void dt_idle_genpd_detach_cpu(struct device *dev)
{
if (IS_ERR_OR_NULL(dev))
return;

dev_pm_domain_detach(dev, false);
}
+EXPORT_SYMBOL_GPL(dt_idle_genpd_detach_cpu);
diff --git a/drivers/cpuidle/dt_idle_genpd.h b/drivers/cpuidle/dt_idle_genpd.h
new file mode 100644
index 000000000000..a3d3d2e85871
--- /dev/null
+++ b/drivers/cpuidle/dt_idle_genpd.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __DT_IDLE_GENPD
+#define __DT_IDLE_GENPD
+
+struct device_node;
+struct generic_pm_domain;
+
+struct dt_idle_genpd_ops {
+ int (*parse_state_node)(struct device_node *np, u32 *state);
+ int (*power_off)(struct generic_pm_domain *pd);
+};
+
+#ifdef CONFIG_DT_IDLE_GENPD
+
+int dt_idle_genpd_probe(const struct dt_idle_genpd_ops *ops,
+ struct device_node *np);
+
+struct device *dt_idle_genpd_attach_cpu(int cpu, const char *name);
+
+void dt_idle_genpd_detach_cpu(struct device *dev);
+
+#else
+
+int dt_idle_genpd_probe(const struct dt_idle_genpd_ops *ops,
+ struct device_node *np)
+{
+ return 0;
+}
+
+static inline struct device *dt_idle_genpd_attach_cpu(int cpu,
+ const char *name)
+{
+ return NULL;
+}
+
+static inline void dt_idle_genpd_detach_cpu(struct device *dev)
+{
+}
+
+#endif
+
+#endif
--
2.25.1

2021-02-21 09:46:00

by Anup Patel

Subject: [RFC PATCH 8/8] RISC-V: Enable RISC-V SBI CPU Idle driver for QEMU virt machine

We enable RISC-V SBI CPU Idle driver for QEMU virt machine to test
SBI HSM Supend on QEMU.

Signed-off-by: Anup Patel <[email protected]>
---
arch/riscv/Kconfig.socs | 3 +++
arch/riscv/configs/defconfig | 1 +
arch/riscv/configs/rv32_defconfig | 1 +
3 files changed, 5 insertions(+)

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index 3284d5c291be..5f6f4a520772 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -19,6 +19,9 @@ config SOC_VIRT
select GOLDFISH
select RTC_DRV_GOLDFISH if RTC_CLASS
select SIFIVE_PLIC
+ select PM_GENERIC_DOMAINS if PM
+ select PM_GENERIC_DOMAINS_OF if PM && OF
+ select RISCV_SBI_CPUIDLE if CPU_IDLE
help
This enables support for QEMU Virt Machine.

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index dc4927c0e44b..aac26c20bbf5 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -19,6 +19,7 @@ CONFIG_SOC_SIFIVE=y
CONFIG_SOC_VIRT=y
CONFIG_SMP=y
CONFIG_HOTPLUG_CPU=y
+CONFIG_PM=y
CONFIG_CPU_IDLE=y
CONFIG_JUMP_LABEL=y
CONFIG_MODULES=y
diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
index 332e43a4a2c3..2285c95e34b3 100644
--- a/arch/riscv/configs/rv32_defconfig
+++ b/arch/riscv/configs/rv32_defconfig
@@ -20,6 +20,7 @@ CONFIG_SOC_VIRT=y
CONFIG_ARCH_RV32I=y
CONFIG_SMP=y
CONFIG_HOTPLUG_CPU=y
+CONFIG_PM=y
CONFIG_CPU_IDLE=y
CONFIG_JUMP_LABEL=y
CONFIG_MODULES=y
--
2.25.1

2021-02-26 13:19:49

by Alexandre Ghiti

Subject: Re: [RFC PATCH 1/8] RISC-V: Enable CPU_IDLE drivers

Hi Anup,

Le 2/21/21 ? 4:37 AM, Anup Patel a ?crit?:
> We force select CPU_PM and provide asm/cpuidle.h so that we can
> use CPU IDLE drivers for Linux RISC-V kernel.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> arch/riscv/Kconfig | 7 +++++++
> arch/riscv/configs/defconfig | 7 +++----
> arch/riscv/configs/rv32_defconfig | 4 ++--
> arch/riscv/include/asm/cpuidle.h | 24 ++++++++++++++++++++++++
> arch/riscv/kernel/process.c | 3 ++-
> 5 files changed, 38 insertions(+), 7 deletions(-)
> create mode 100644 arch/riscv/include/asm/cpuidle.h
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index fe6862b06ead..4901200b6b6c 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -37,6 +37,7 @@ config RISCV
> select CLONE_BACKWARDS
> select CLINT_TIMER if !MMU
> select COMMON_CLK
> + select CPU_PM if CPU_IDLE
> select EDAC_SUPPORT
> select GENERIC_ARCH_TOPOLOGY if SMP
> select GENERIC_ATOMIC64 if !64BIT
> @@ -430,4 +431,10 @@ source "kernel/power/Kconfig"
>
> endmenu
>
> +menu "CPU Power Management"
> +
> +source "drivers/cpuidle/Kconfig"
> +
> +endmenu
> +
> source "drivers/firmware/Kconfig"
> diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> index 6c0625aa96c7..dc4927c0e44b 100644
> --- a/arch/riscv/configs/defconfig
> +++ b/arch/riscv/configs/defconfig
> @@ -13,11 +13,13 @@ CONFIG_USER_NS=y
> CONFIG_CHECKPOINT_RESTORE=y
> CONFIG_BLK_DEV_INITRD=y
> CONFIG_EXPERT=y
> +# CONFIG_SYSFS_SYSCALL is not set
> CONFIG_BPF_SYSCALL=y
> CONFIG_SOC_SIFIVE=y
> CONFIG_SOC_VIRT=y
> CONFIG_SMP=y
> CONFIG_HOTPLUG_CPU=y
> +CONFIG_CPU_IDLE=y
> CONFIG_JUMP_LABEL=y
> CONFIG_MODULES=y
> CONFIG_MODULE_UNLOAD=y
> @@ -65,10 +67,9 @@ CONFIG_HW_RANDOM=y
> CONFIG_HW_RANDOM_VIRTIO=y
> CONFIG_SPI=y
> CONFIG_SPI_SIFIVE=y
> +# CONFIG_PTP_1588_CLOCK is not set
> CONFIG_GPIOLIB=y
> CONFIG_GPIO_SIFIVE=y
> -# CONFIG_PTP_1588_CLOCK is not set
> -CONFIG_POWER_RESET=y

Why do you remove this config ?

> CONFIG_DRM=y
> CONFIG_DRM_RADEON=y
> CONFIG_DRM_VIRTIO_GPU=y
> @@ -132,5 +133,3 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y
> # CONFIG_FTRACE is not set
> # CONFIG_RUNTIME_TESTING_MENU is not set
> CONFIG_MEMTEST=y
> -# CONFIG_SYSFS_SYSCALL is not set
> -CONFIG_EFI=y

And this is one too ? If those removals are intentional, maybe you can
add something about that in the commit description ?

> diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
> index 8dd02b842fef..332e43a4a2c3 100644
> --- a/arch/riscv/configs/rv32_defconfig
> +++ b/arch/riscv/configs/rv32_defconfig
> @@ -13,12 +13,14 @@ CONFIG_USER_NS=y
> CONFIG_CHECKPOINT_RESTORE=y
> CONFIG_BLK_DEV_INITRD=y
> CONFIG_EXPERT=y
> +# CONFIG_SYSFS_SYSCALL is not set
> CONFIG_BPF_SYSCALL=y
> CONFIG_SOC_SIFIVE=y
> CONFIG_SOC_VIRT=y
> CONFIG_ARCH_RV32I=y
> CONFIG_SMP=y
> CONFIG_HOTPLUG_CPU=y
> +CONFIG_CPU_IDLE=y
> CONFIG_JUMP_LABEL=y
> CONFIG_MODULES=y
> CONFIG_MODULE_UNLOAD=y
> @@ -67,7 +69,6 @@ CONFIG_HW_RANDOM_VIRTIO=y
> CONFIG_SPI=y
> CONFIG_SPI_SIFIVE=y
> # CONFIG_PTP_1588_CLOCK is not set
> -CONFIG_POWER_RESET=y
> CONFIG_DRM=y
> CONFIG_DRM_RADEON=y
> CONFIG_DRM_VIRTIO_GPU=y
> @@ -131,4 +132,3 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y
> # CONFIG_FTRACE is not set
> # CONFIG_RUNTIME_TESTING_MENU is not set
> CONFIG_MEMTEST=y
> -# CONFIG_SYSFS_SYSCALL is not set
> diff --git a/arch/riscv/include/asm/cpuidle.h b/arch/riscv/include/asm/cpuidle.h
> new file mode 100644
> index 000000000000..1042d790e446
> --- /dev/null
> +++ b/arch/riscv/include/asm/cpuidle.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2021 Allwinner Ltd
> + * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> + */
> +
> +#ifndef _ASM_RISCV_CPUIDLE_H
> +#define _ASM_RISCV_CPUIDLE_H
> +
> +#include <asm/barrier.h>
> +#include <asm/processor.h>
> +
> +static inline void cpu_do_idle(void)
> +{
> + /*
> + * Add mb() here to ensure that all
> + * IO/MEM access are completed prior

accessES ?

> + * to enter WFI.
> + */
> + mb();
> + wait_for_interrupt();
> +}
> +
> +#endif
> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> index dd5f985b1f40..b5b51fd26624 100644
> --- a/arch/riscv/kernel/process.c
> +++ b/arch/riscv/kernel/process.c
> @@ -21,6 +21,7 @@
> #include <asm/string.h>
> #include <asm/switch_to.h>
> #include <asm/thread_info.h>
> +#include <asm/cpuidle.h>
>
> register unsigned long gp_in_global __asm__("gp");
>
> @@ -35,7 +36,7 @@ extern asmlinkage void ret_from_kernel_thread(void);
>
> void arch_cpu_idle(void)
> {
> - wait_for_interrupt();
> + cpu_do_idle();
> raw_local_irq_enable();
> }
>
>

2021-02-27 04:03:53

by Anup Patel

Subject: Re: [RFC PATCH 1/8] RISC-V: Enable CPU_IDLE drivers

Hi Alex,

On Fri, Feb 26, 2021 at 6:46 PM Alex Ghiti <[email protected]> wrote:
>
> Hi Anup,
>
> Le 2/21/21 à 4:37 AM, Anup Patel a écrit :
> > We force select CPU_PM and provide asm/cpuidle.h so that we can
> > use CPU IDLE drivers for Linux RISC-V kernel.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/Kconfig | 7 +++++++
> > arch/riscv/configs/defconfig | 7 +++----
> > arch/riscv/configs/rv32_defconfig | 4 ++--
> > arch/riscv/include/asm/cpuidle.h | 24 ++++++++++++++++++++++++
> > arch/riscv/kernel/process.c | 3 ++-
> > 5 files changed, 38 insertions(+), 7 deletions(-)
> > create mode 100644 arch/riscv/include/asm/cpuidle.h
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index fe6862b06ead..4901200b6b6c 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -37,6 +37,7 @@ config RISCV
> > select CLONE_BACKWARDS
> > select CLINT_TIMER if !MMU
> > select COMMON_CLK
> > + select CPU_PM if CPU_IDLE
> > select EDAC_SUPPORT
> > select GENERIC_ARCH_TOPOLOGY if SMP
> > select GENERIC_ATOMIC64 if !64BIT
> > @@ -430,4 +431,10 @@ source "kernel/power/Kconfig"
> >
> > endmenu
> >
> > +menu "CPU Power Management"
> > +
> > +source "drivers/cpuidle/Kconfig"
> > +
> > +endmenu
> > +
> > source "drivers/firmware/Kconfig"
> > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> > index 6c0625aa96c7..dc4927c0e44b 100644
> > --- a/arch/riscv/configs/defconfig
> > +++ b/arch/riscv/configs/defconfig
> > @@ -13,11 +13,13 @@ CONFIG_USER_NS=y
> > CONFIG_CHECKPOINT_RESTORE=y
> > CONFIG_BLK_DEV_INITRD=y
> > CONFIG_EXPERT=y
> > +# CONFIG_SYSFS_SYSCALL is not set
> > CONFIG_BPF_SYSCALL=y
> > CONFIG_SOC_SIFIVE=y
> > CONFIG_SOC_VIRT=y
> > CONFIG_SMP=y
> > CONFIG_HOTPLUG_CPU=y
> > +CONFIG_CPU_IDLE=y
> > CONFIG_JUMP_LABEL=y
> > CONFIG_MODULES=y
> > CONFIG_MODULE_UNLOAD=y
> > @@ -65,10 +67,9 @@ CONFIG_HW_RANDOM=y
> > CONFIG_HW_RANDOM_VIRTIO=y
> > CONFIG_SPI=y
> > CONFIG_SPI_SIFIVE=y
> > +# CONFIG_PTP_1588_CLOCK is not set
> > CONFIG_GPIOLIB=y
> > CONFIG_GPIO_SIFIVE=y
> > -# CONFIG_PTP_1588_CLOCK is not set
> > -CONFIG_POWER_RESET=y
>
> Why do you remove this config ?

Argh, I don't know how this got here. I will remove
this change in the next revision. Thanks for catching.

>
> > CONFIG_DRM=y
> > CONFIG_DRM_RADEON=y
> > CONFIG_DRM_VIRTIO_GPU=y
> > @@ -132,5 +133,3 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y
> > # CONFIG_FTRACE is not set
> > # CONFIG_RUNTIME_TESTING_MENU is not set
> > CONFIG_MEMTEST=y
> > -# CONFIG_SYSFS_SYSCALL is not set
> > -CONFIG_EFI=y
>
> And this is one too ? If those removals are intentional, maybe you can
> add something about that in the commit description ?

I will remove this change as well.

>
> > diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
> > index 8dd02b842fef..332e43a4a2c3 100644
> > --- a/arch/riscv/configs/rv32_defconfig
> > +++ b/arch/riscv/configs/rv32_defconfig
> > @@ -13,12 +13,14 @@ CONFIG_USER_NS=y
> > CONFIG_CHECKPOINT_RESTORE=y
> > CONFIG_BLK_DEV_INITRD=y
> > CONFIG_EXPERT=y
> > +# CONFIG_SYSFS_SYSCALL is not set
> > CONFIG_BPF_SYSCALL=y
> > CONFIG_SOC_SIFIVE=y
> > CONFIG_SOC_VIRT=y
> > CONFIG_ARCH_RV32I=y
> > CONFIG_SMP=y
> > CONFIG_HOTPLUG_CPU=y
> > +CONFIG_CPU_IDLE=y
> > CONFIG_JUMP_LABEL=y
> > CONFIG_MODULES=y
> > CONFIG_MODULE_UNLOAD=y
> > @@ -67,7 +69,6 @@ CONFIG_HW_RANDOM_VIRTIO=y
> > CONFIG_SPI=y
> > CONFIG_SPI_SIFIVE=y
> > # CONFIG_PTP_1588_CLOCK is not set
> > -CONFIG_POWER_RESET=y
> > CONFIG_DRM=y
> > CONFIG_DRM_RADEON=y
> > CONFIG_DRM_VIRTIO_GPU=y
> > @@ -131,4 +132,3 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y
> > # CONFIG_FTRACE is not set
> > # CONFIG_RUNTIME_TESTING_MENU is not set
> > CONFIG_MEMTEST=y
> > -# CONFIG_SYSFS_SYSCALL is not set
> > diff --git a/arch/riscv/include/asm/cpuidle.h b/arch/riscv/include/asm/cpuidle.h
> > new file mode 100644
> > index 000000000000..1042d790e446
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/cpuidle.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2021 Allwinner Ltd
> > + * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> > + */
> > +
> > +#ifndef _ASM_RISCV_CPUIDLE_H
> > +#define _ASM_RISCV_CPUIDLE_H
> > +
> > +#include <asm/barrier.h>
> > +#include <asm/processor.h>
> > +
> > +static inline void cpu_do_idle(void)
> > +{
> > + /*
> > + * Add mb() here to ensure that all
> > + * IO/MEM access are completed prior
>
> accessES ?

Okay will update.

>
> > + * to enter WFI.
> > + */
> > + mb();
> > + wait_for_interrupt();
> > +}
> > +
> > +#endif
> > diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> > index dd5f985b1f40..b5b51fd26624 100644
> > --- a/arch/riscv/kernel/process.c
> > +++ b/arch/riscv/kernel/process.c
> > @@ -21,6 +21,7 @@
> > #include <asm/string.h>
> > #include <asm/switch_to.h>
> > #include <asm/thread_info.h>
> > +#include <asm/cpuidle.h>
> >
> > register unsigned long gp_in_global __asm__("gp");
> >
> > @@ -35,7 +36,7 @@ extern asmlinkage void ret_from_kernel_thread(void);
> >
> > void arch_cpu_idle(void)
> > {
> > - wait_for_interrupt();
> > + cpu_do_idle();
> > raw_local_irq_enable();
> > }
> >
> >

Regards,
Anup

2021-03-02 20:08:27

by Anup Patel

Subject: Re: [RFC PATCH 1/8] RISC-V: Enable CPU_IDLE drivers

On Fri, Feb 26, 2021 at 6:46 PM Alex Ghiti <[email protected]> wrote:
>
> Hi Anup,
>
> Le 2/21/21 à 4:37 AM, Anup Patel a écrit :
> > We force select CPU_PM and provide asm/cpuidle.h so that we can
> > use CPU IDLE drivers for Linux RISC-V kernel.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > arch/riscv/Kconfig | 7 +++++++
> > arch/riscv/configs/defconfig | 7 +++----
> > arch/riscv/configs/rv32_defconfig | 4 ++--
> > arch/riscv/include/asm/cpuidle.h | 24 ++++++++++++++++++++++++
> > arch/riscv/kernel/process.c | 3 ++-
> > 5 files changed, 38 insertions(+), 7 deletions(-)
> > create mode 100644 arch/riscv/include/asm/cpuidle.h
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index fe6862b06ead..4901200b6b6c 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -37,6 +37,7 @@ config RISCV
> > select CLONE_BACKWARDS
> > select CLINT_TIMER if !MMU
> > select COMMON_CLK
> > + select CPU_PM if CPU_IDLE
> > select EDAC_SUPPORT
> > select GENERIC_ARCH_TOPOLOGY if SMP
> > select GENERIC_ATOMIC64 if !64BIT
> > @@ -430,4 +431,10 @@ source "kernel/power/Kconfig"
> >
> > endmenu
> >
> > +menu "CPU Power Management"
> > +
> > +source "drivers/cpuidle/Kconfig"
> > +
> > +endmenu
> > +
> > source "drivers/firmware/Kconfig"
> > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
> > index 6c0625aa96c7..dc4927c0e44b 100644
> > --- a/arch/riscv/configs/defconfig
> > +++ b/arch/riscv/configs/defconfig
> > @@ -13,11 +13,13 @@ CONFIG_USER_NS=y
> > CONFIG_CHECKPOINT_RESTORE=y
> > CONFIG_BLK_DEV_INITRD=y
> > CONFIG_EXPERT=y
> > +# CONFIG_SYSFS_SYSCALL is not set
> > CONFIG_BPF_SYSCALL=y
> > CONFIG_SOC_SIFIVE=y
> > CONFIG_SOC_VIRT=y
> > CONFIG_SMP=y
> > CONFIG_HOTPLUG_CPU=y
> > +CONFIG_CPU_IDLE=y
> > CONFIG_JUMP_LABEL=y
> > CONFIG_MODULES=y
> > CONFIG_MODULE_UNLOAD=y
> > @@ -65,10 +67,9 @@ CONFIG_HW_RANDOM=y
> > CONFIG_HW_RANDOM_VIRTIO=y
> > CONFIG_SPI=y
> > CONFIG_SPI_SIFIVE=y
> > +# CONFIG_PTP_1588_CLOCK is not set
> > CONFIG_GPIOLIB=y
> > CONFIG_GPIO_SIFIVE=y
> > -# CONFIG_PTP_1588_CLOCK is not set
> > -CONFIG_POWER_RESET=y
>
> Why do you remove this config ?

This option is selected by CONFIG_SOC_VIRT so it is being
removed from defconfig by savedefconfig.

>
> > CONFIG_DRM=y
> > CONFIG_DRM_RADEON=y
> > CONFIG_DRM_VIRTIO_GPU=y
> > @@ -132,5 +133,3 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y
> > # CONFIG_FTRACE is not set
> > # CONFIG_RUNTIME_TESTING_MENU is not set
> > CONFIG_MEMTEST=y
> > -# CONFIG_SYSFS_SYSCALL is not set
> > -CONFIG_EFI=y
>
> And this is one too ? If those removals are intentional, maybe you can
> add something about that in the commit description ?

This is enabled by default so savedefconfig removes it.

>
> > diff --git a/arch/riscv/configs/rv32_defconfig b/arch/riscv/configs/rv32_defconfig
> > index 8dd02b842fef..332e43a4a2c3 100644
> > --- a/arch/riscv/configs/rv32_defconfig
> > +++ b/arch/riscv/configs/rv32_defconfig
> > @@ -13,12 +13,14 @@ CONFIG_USER_NS=y
> > CONFIG_CHECKPOINT_RESTORE=y
> > CONFIG_BLK_DEV_INITRD=y
> > CONFIG_EXPERT=y
> > +# CONFIG_SYSFS_SYSCALL is not set
> > CONFIG_BPF_SYSCALL=y
> > CONFIG_SOC_SIFIVE=y
> > CONFIG_SOC_VIRT=y
> > CONFIG_ARCH_RV32I=y
> > CONFIG_SMP=y
> > CONFIG_HOTPLUG_CPU=y
> > +CONFIG_CPU_IDLE=y
> > CONFIG_JUMP_LABEL=y
> > CONFIG_MODULES=y
> > CONFIG_MODULE_UNLOAD=y
> > @@ -67,7 +69,6 @@ CONFIG_HW_RANDOM_VIRTIO=y
> > CONFIG_SPI=y
> > CONFIG_SPI_SIFIVE=y
> > # CONFIG_PTP_1588_CLOCK is not set
> > -CONFIG_POWER_RESET=y
> > CONFIG_DRM=y
> > CONFIG_DRM_RADEON=y
> > CONFIG_DRM_VIRTIO_GPU=y
> > @@ -131,4 +132,3 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y
> > # CONFIG_FTRACE is not set
> > # CONFIG_RUNTIME_TESTING_MENU is not set
> > CONFIG_MEMTEST=y
> > -# CONFIG_SYSFS_SYSCALL is not set
> > diff --git a/arch/riscv/include/asm/cpuidle.h b/arch/riscv/include/asm/cpuidle.h
> > new file mode 100644
> > index 000000000000..1042d790e446
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/cpuidle.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2021 Allwinner Ltd
> > + * Copyright (C) 2021 Western Digital Corporation or its affiliates.
> > + */
> > +
> > +#ifndef _ASM_RISCV_CPUIDLE_H
> > +#define _ASM_RISCV_CPUIDLE_H
> > +
> > +#include <asm/barrier.h>
> > +#include <asm/processor.h>
> > +
> > +static inline void cpu_do_idle(void)
> > +{
> > + /*
> > + * Add mb() here to ensure that all
> > + * IO/MEM access are completed prior
>
> accessES ?
>
> > + * to enter WFI.
> > + */
> > + mb();
> > + wait_for_interrupt();
> > +}
> > +
> > +#endif
> > diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
> > index dd5f985b1f40..b5b51fd26624 100644
> > --- a/arch/riscv/kernel/process.c
> > +++ b/arch/riscv/kernel/process.c
> > @@ -21,6 +21,7 @@
> > #include <asm/string.h>
> > #include <asm/switch_to.h>
> > #include <asm/thread_info.h>
> > +#include <asm/cpuidle.h>
> >
> > register unsigned long gp_in_global __asm__("gp");
> >
> > @@ -35,7 +36,7 @@ extern asmlinkage void ret_from_kernel_thread(void);
> >
> > void arch_cpu_idle(void)
> > {
> > - wait_for_interrupt();
> > + cpu_do_idle();
> > raw_local_irq_enable();
> > }
> >
> >

Regards,
Anup

2021-03-05 23:26:34

by Rob Herring (Arm)

Subject: Re: [RFC PATCH 7/8] dt-bindings: Add bindings documentation for RISC-V idle states

On Sun, Feb 21, 2021 at 03:07:57PM +0530, Anup Patel wrote:
> The RISC-V CPU idle states will be described in DT under the
> /cpus/riscv-idle-states DT node. This patch adds the bindings
> documentation for riscv-idle-states DT nodes and idle state DT
> nodes under it.
>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> .../bindings/riscv/idle-states.yaml | 250 ++++++++++++++++++
> 1 file changed, 250 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/riscv/idle-states.yaml
>
> diff --git a/Documentation/devicetree/bindings/riscv/idle-states.yaml b/Documentation/devicetree/bindings/riscv/idle-states.yaml
> new file mode 100644
> index 000000000000..3eff763fed23
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/riscv/idle-states.yaml
> @@ -0,0 +1,250 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/riscv/idle-states.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RISC-V idle states binding description
> +
> +maintainers:
> + - Anup Patel <[email protected]>
> +
> +description: |+
> + RISC-V systems can manage power consumption dynamically, where HARTs
> + (or CPUs) [1] can be put in different platform specific suspend (or
> + idle) states (ranging from simple WFI, power gating, etc). The RISC-V
> + SBI [2] hart state management extension provides a standard mechanism
> + for OSes to request HART state transitions.
> +
> + The platform specific suspend (or idle) states of a hart can be either
> + retentive or non-rententive in nature. A retentive suspend state will
> + preserve hart register and CSR values for all privilege modes whereas
> + a non-retentive suspend state will not preserve hart register and CSR
> + values. The suspend (or idle) state entered by executing the WFI
> + instruction is considered standard on all RISC-V systems and therefore
> + must not be listed in device tree.
> +
> + The device tree binding definition for RISC-V idle states described
> + in this document is quite similar to the ARM idle states [3].
> +
> + References
> +
> + [1] RISC-V Linux Kernel documentation - CPUs bindings
> + Documentation/devicetree/bindings/riscv/cpus.yaml
> +
> + [2] RISC-V Supervisor Binary Interface (SBI)
> + http://github.com/riscv/riscv-sbi-doc/riscv-sbi.adoc
> +
> + [3] ARM idle states binding description - Idle states bindings
> + Documentation/devicetree/bindings/arm/idle-states.yaml

I'd assume there's common parts we can share.

> +
> +properties:
> + $nodename:
> + const: riscv-idle-states

Just 'idle-states' like Arm.

> +
> +patternProperties:
> + "^(cpu|cluster)-":
> + type: object
> + description: |
> + Each state node represents an idle state description and must be
> + defined as follows.
> +

additionalProperties: false

> + properties:
> + compatible:
> + const: riscv,idle-state
> +
> + local-timer-stop:
> + description:
> + If present the CPU local timer control logic is lost on state
> + entry, otherwise it is retained.
> + type: boolean
> +
> + entry-latency-us:
> + description:
> + Worst case latency in microseconds required to enter the idle state.
> +
> + exit-latency-us:
> + description:
> + Worst case latency in microseconds required to exit the idle state.
> + The exit-latency-us duration may be guaranteed only after
> + entry-latency-us has passed.
> +
> + min-residency-us:
> + description:
> + Minimum residency duration in microseconds, inclusive of preparation
> + and entry, for this idle state to be considered worthwhile energy
> + wise (refer to section 2 of this document for a complete description).
> +
> + wakeup-latency-us:
> + description: |
> + Maximum delay between the signaling of a wake-up event and the CPU
> + being able to execute normal code again. If omitted, this is assumed
> + to be equal to:
> +
> + entry-latency-us + exit-latency-us
> +
> + It is important to supply this value on systems where the duration
> + of PREP phase (see diagram 1, section 2) is non-neglibigle. In such
> + systems entry-latency-us + exit-latency-us will exceed
> + wakeup-latency-us by this duration.
> +
> + idle-state-name:
> + $ref: /schemas/types.yaml#/definitions/string
> + description:
> + A string used as a descriptive name for the idle state.
> +
> + required:
> + - compatible
> + - entry-latency-us
> + - exit-latency-us
> + - min-residency-us
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +
> + cpus {
> + #size-cells = <0>;
> + #address-cells = <1>;
> +
> + cpu@0 {
> + device_type = "cpu";
> + compatible = "riscv";
> + reg = <0x0>;
> + riscv,isa = "rv64imafdc";
> + mmu-type = "riscv,sv48";
> + cpu-idle-states = <&CPU_RET_0_0 &CPU_NONRET_0_0
> + &CLUSTER_RET_0 &CLUSTER_NONRET_0>;

You should need to add this property to your cpu schema.

> +
> + cpu_intc0: interrupt-controller {
> + #interrupt-cells = <1>;
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + };
> + };
> +
> + cpu@1 {
> + device_type = "cpu";
> + compatible = "riscv";
> + reg = <0x1>;
> + riscv,isa = "rv64imafdc";
> + mmu-type = "riscv,sv48";
> + cpu-idle-states = <&CPU_RET_0_0 &CPU_NONRET_0_0
> + &CLUSTER_RET_0 &CLUSTER_NONRET_0>;
> +
> + cpu_intc1: interrupt-controller {
> + #interrupt-cells = <1>;
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + };
> + };
> +
> + cpu@10 {
> + device_type = "cpu";
> + compatible = "riscv";
> + reg = <0x10>;
> + riscv,isa = "rv64imafdc";
> + mmu-type = "riscv,sv48";
> + cpu-idle-states = <&CPU_RET_1_0 &CPU_NONRET_1_0
> + &CLUSTER_RET_1 &CLUSTER_NONRET_1>;
> +
> + cpu_intc10: interrupt-controller {
> + #interrupt-cells = <1>;
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + };
> + };
> +
> + cpu@11 {
> + device_type = "cpu";
> + compatible = "riscv";
> + reg = <0x11>;
> + riscv,isa = "rv64imafdc";
> + mmu-type = "riscv,sv48";
> + cpu-idle-states = <&CPU_RET_1_0 &CPU_NONRET_1_0
> + &CLUSTER_RET_1 &CLUSTER_NONRET_1>;
> +
> + cpu_intc11: interrupt-controller {
> + #interrupt-cells = <1>;
> + compatible = "riscv,cpu-intc";
> + interrupt-controller;
> + };
> + };
> +
> + riscv-idle-states {
> + CPU_RET_0_0: cpu-retentive-0-0 {
> + compatible = "riscv,idle-state";
> + riscv,sbi-suspend-param = <0x10000000>;

Not documented.

> + entry-latency-us = <20>;
> + exit-latency-us = <40>;
> + min-residency-us = <80>;
> + };
> +
> + CPU_NONRET_0_0: cpu-nonretentive-0-0 {
> + compatible = "riscv,idle-state";
> + riscv,sbi-suspend-param = <0x90000000>;
> + entry-latency-us = <250>;
> + exit-latency-us = <500>;
> + min-residency-us = <950>;
> + };
> +
> + CLUSTER_RET_0: cluster-retentive-0 {
> + compatible = "riscv,idle-state";
> + riscv,sbi-suspend-param = <0x11000000>;
> + local-timer-stop;
> + entry-latency-us = <50>;
> + exit-latency-us = <100>;
> + min-residency-us = <250>;
> + wakeup-latency-us = <130>;
> + };
> +
> + CLUSTER_NONRET_0: cluster-nonretentive-0 {
> + compatible = "riscv,idle-state";
> + riscv,sbi-suspend-param = <0x91000000>;
> + local-timer-stop;
> + entry-latency-us = <600>;
> + exit-latency-us = <1100>;
> + min-residency-us = <2700>;
> + wakeup-latency-us = <1500>;
> + };
> +
> + CPU_RET_1_0: cpu-retentive-1-0 {
> + compatible = "riscv,idle-state";
> + riscv,sbi-suspend-param = <0x10000010>;
> + entry-latency-us = <20>;
> + exit-latency-us = <40>;
> + min-residency-us = <80>;
> + };
> +
> + CPU_NONRET_1_0: cpu-nonretentive-1-0 {
> + compatible = "riscv,idle-state";
> + riscv,sbi-suspend-param = <0x90000010>;
> + entry-latency-us = <250>;
> + exit-latency-us = <500>;
> + min-residency-us = <950>;
> + };
> +
> + CLUSTER_RET_1: cluster-retentive-1 {
> + compatible = "riscv,idle-state";
> + riscv,sbi-suspend-param = <0x11000010>;
> + local-timer-stop;
> + entry-latency-us = <50>;
> + exit-latency-us = <100>;
> + min-residency-us = <250>;
> + wakeup-latency-us = <130>;
> + };
> +
> + CLUSTER_NONRET_1: cluster-nonretentive-1 {
> + compatible = "riscv,idle-state";
> + riscv,sbi-suspend-param = <0x91000010>;
> + local-timer-stop;
> + entry-latency-us = <600>;
> + exit-latency-us = <1100>;
> + min-residency-us = <2700>;
> + wakeup-latency-us = <1500>;
> + };
> + };
> + };
> +
> +...
> --
> 2.25.1
>

2021-03-08 10:14:14

by Anup Patel

Subject: Re: [RFC PATCH 7/8] dt-bindings: Add bindings documentation for RISC-V idle states

On Sat, Mar 6, 2021 at 4:52 AM Rob Herring <[email protected]> wrote:
>
> On Sun, Feb 21, 2021 at 03:07:57PM +0530, Anup Patel wrote:
> > The RISC-V CPU idle states will be described in DT under the
> > /cpus/riscv-idle-states DT node. This patch adds the bindings
> > documentation for riscv-idle-states DT nodes and idle state DT
> > nodes under it.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > ---
> > .../bindings/riscv/idle-states.yaml | 250 ++++++++++++++++++
> > 1 file changed, 250 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/riscv/idle-states.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/idle-states.yaml b/Documentation/devicetree/bindings/riscv/idle-states.yaml
> > new file mode 100644
> > index 000000000000..3eff763fed23
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/riscv/idle-states.yaml
> > @@ -0,0 +1,250 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/riscv/idle-states.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: RISC-V idle states binding description
> > +
> > +maintainers:
> > + - Anup Patel <[email protected]>
> > +
> > +description: |+
> > + RISC-V systems can manage power consumption dynamically, where HARTs
> > + (or CPUs) [1] can be put in different platform specific suspend (or
> > + idle) states (ranging from simple WFI, power gating, etc). The RISC-V
> > + SBI [2] hart state management extension provides a standard mechanism
> > + for OSes to request HART state transitions.
> > +
> > + The platform specific suspend (or idle) states of a hart can be either
> > + retentive or non-rententive in nature. A retentive suspend state will
> > + preserve hart register and CSR values for all privilege modes whereas
> > + a non-retentive suspend state will not preserve hart register and CSR
> > + values. The suspend (or idle) state entered by executing the WFI
> > + instruction is considered standard on all RISC-V systems and therefore
> > + must not be listed in device tree.
> > +
> > + The device tree binding definition for RISC-V idle states described
> > + in this document is quite similar to the ARM idle states [3].
> > +
> > + References
> > +
> > + [1] RISC-V Linux Kernel documentation - CPUs bindings
> > + Documentation/devicetree/bindings/riscv/cpus.yaml
> > +
> > + [2] RISC-V Supervisor Binary Interface (SBI)
> > + http://github.com/riscv/riscv-sbi-doc/riscv-sbi.adoc
> > +
> > + [3] ARM idle states binding description - Idle states bindings
> > + Documentation/devicetree/bindings/arm/idle-states.yaml
>
> I'd assume there's common parts we can share.

Yes, except few properties most are the same.

We can have a shared DT bindings for both ARM and RISC-V but
both architectures will always have some architecture specific details
(or properties) which need to be documented under arch specific
DT documentation. Is it okay if this is done as a separate series ?

>
> > +
> > +properties:
> > + $nodename:
> > + const: riscv-idle-states
>
> Just 'idle-states' like Arm.

I had tried "idle-states" node name but DT bindings check complaints
conflict with ARM idle state bindings.

>
> > +
> > +patternProperties:
> > + "^(cpu|cluster)-":
> > + type: object
> > + description: |
> > + Each state node represents an idle state description and must be
> > + defined as follows.
> > +
>
> additionalProperties: false

okay, will update.

>
> > + properties:
> > + compatible:
> > + const: riscv,idle-state
> > +
> > + local-timer-stop:
> > + description:
> > + If present the CPU local timer control logic is lost on state
> > + entry, otherwise it is retained.
> > + type: boolean
> > +
> > + entry-latency-us:
> > + description:
> > + Worst case latency in microseconds required to enter the idle state.
> > +
> > + exit-latency-us:
> > + description:
> > + Worst case latency in microseconds required to exit the idle state.
> > + The exit-latency-us duration may be guaranteed only after
> > + entry-latency-us has passed.
> > +
> > + min-residency-us:
> > + description:
> > + Minimum residency duration in microseconds, inclusive of preparation
> > + and entry, for this idle state to be considered worthwhile energy
> > + wise (refer to section 2 of this document for a complete description).
> > +
> > + wakeup-latency-us:
> > + description: |
> > + Maximum delay between the signaling of a wake-up event and the CPU
> > + being able to execute normal code again. If omitted, this is assumed
> > + to be equal to:
> > +
> > + entry-latency-us + exit-latency-us
> > +
> > + It is important to supply this value on systems where the duration
> > + of PREP phase (see diagram 1, section 2) is non-neglibigle. In such
> > + systems entry-latency-us + exit-latency-us will exceed
> > + wakeup-latency-us by this duration.
> > +
> > + idle-state-name:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + description:
> > + A string used as a descriptive name for the idle state.
> > +
> > + required:
> > + - compatible
> > + - entry-latency-us
> > + - exit-latency-us
> > + - min-residency-us
> > +
> > +additionalProperties: false

I will move this up.

> > +
> > +examples:
> > + - |
> > +
> > + cpus {
> > + #size-cells = <0>;
> > + #address-cells = <1>;
> > +
> > + cpu@0 {
> > + device_type = "cpu";
> > + compatible = "riscv";
> > + reg = <0x0>;
> > + riscv,isa = "rv64imafdc";
> > + mmu-type = "riscv,sv48";
> > + cpu-idle-states = <&CPU_RET_0_0 &CPU_NONRET_0_0
> > + &CLUSTER_RET_0 &CLUSTER_NONRET_0>;
>
> You should need to add this property to your cpu schema.

Okay, will update.

>
> > +
> > + cpu_intc0: interrupt-controller {
> > + #interrupt-cells = <1>;
> > + compatible = "riscv,cpu-intc";
> > + interrupt-controller;
> > + };
> > + };
> > +
> > + cpu@1 {
> > + device_type = "cpu";
> > + compatible = "riscv";
> > + reg = <0x1>;
> > + riscv,isa = "rv64imafdc";
> > + mmu-type = "riscv,sv48";
> > + cpu-idle-states = <&CPU_RET_0_0 &CPU_NONRET_0_0
> > + &CLUSTER_RET_0 &CLUSTER_NONRET_0>;
> > +
> > + cpu_intc1: interrupt-controller {
> > + #interrupt-cells = <1>;
> > + compatible = "riscv,cpu-intc";
> > + interrupt-controller;
> > + };
> > + };
> > +
> > + cpu@10 {
> > + device_type = "cpu";
> > + compatible = "riscv";
> > + reg = <0x10>;
> > + riscv,isa = "rv64imafdc";
> > + mmu-type = "riscv,sv48";
> > + cpu-idle-states = <&CPU_RET_1_0 &CPU_NONRET_1_0
> > + &CLUSTER_RET_1 &CLUSTER_NONRET_1>;
> > +
> > + cpu_intc10: interrupt-controller {
> > + #interrupt-cells = <1>;
> > + compatible = "riscv,cpu-intc";
> > + interrupt-controller;
> > + };
> > + };
> > +
> > + cpu@11 {
> > + device_type = "cpu";
> > + compatible = "riscv";
> > + reg = <0x11>;
> > + riscv,isa = "rv64imafdc";
> > + mmu-type = "riscv,sv48";
> > + cpu-idle-states = <&CPU_RET_1_0 &CPU_NONRET_1_0
> > + &CLUSTER_RET_1 &CLUSTER_NONRET_1>;
> > +
> > + cpu_intc11: interrupt-controller {
> > + #interrupt-cells = <1>;
> > + compatible = "riscv,cpu-intc";
> > + interrupt-controller;
> > + };
> > + };
> > +
> > + riscv-idle-states {
> > + CPU_RET_0_0: cpu-retentive-0-0 {
> > + compatible = "riscv,idle-state";
> > + riscv,sbi-suspend-param = <0x10000000>;
>
> Not documented.

Ahh, I missed this one. I will add it in next patch revision.

>
> > + entry-latency-us = <20>;
> > + exit-latency-us = <40>;
> > + min-residency-us = <80>;
> > + };
> > +
> > + CPU_NONRET_0_0: cpu-nonretentive-0-0 {
> > + compatible = "riscv,idle-state";
> > + riscv,sbi-suspend-param = <0x90000000>;
> > + entry-latency-us = <250>;
> > + exit-latency-us = <500>;
> > + min-residency-us = <950>;
> > + };
> > +
> > + CLUSTER_RET_0: cluster-retentive-0 {
> > + compatible = "riscv,idle-state";
> > + riscv,sbi-suspend-param = <0x11000000>;
> > + local-timer-stop;
> > + entry-latency-us = <50>;
> > + exit-latency-us = <100>;
> > + min-residency-us = <250>;
> > + wakeup-latency-us = <130>;
> > + };
> > +
> > + CLUSTER_NONRET_0: cluster-nonretentive-0 {
> > + compatible = "riscv,idle-state";
> > + riscv,sbi-suspend-param = <0x91000000>;
> > + local-timer-stop;
> > + entry-latency-us = <600>;
> > + exit-latency-us = <1100>;
> > + min-residency-us = <2700>;
> > + wakeup-latency-us = <1500>;
> > + };
> > +
> > + CPU_RET_1_0: cpu-retentive-1-0 {
> > + compatible = "riscv,idle-state";
> > + riscv,sbi-suspend-param = <0x10000010>;
> > + entry-latency-us = <20>;
> > + exit-latency-us = <40>;
> > + min-residency-us = <80>;
> > + };
> > +
> > + CPU_NONRET_1_0: cpu-nonretentive-1-0 {
> > + compatible = "riscv,idle-state";
> > + riscv,sbi-suspend-param = <0x90000010>;
> > + entry-latency-us = <250>;
> > + exit-latency-us = <500>;
> > + min-residency-us = <950>;
> > + };
> > +
> > + CLUSTER_RET_1: cluster-retentive-1 {
> > + compatible = "riscv,idle-state";
> > + riscv,sbi-suspend-param = <0x11000010>;
> > + local-timer-stop;
> > + entry-latency-us = <50>;
> > + exit-latency-us = <100>;
> > + min-residency-us = <250>;
> > + wakeup-latency-us = <130>;
> > + };
> > +
> > + CLUSTER_NONRET_1: cluster-nonretentive-1 {
> > + compatible = "riscv,idle-state";
> > + riscv,sbi-suspend-param = <0x91000010>;
> > + local-timer-stop;
> > + entry-latency-us = <600>;
> > + exit-latency-us = <1100>;
> > + min-residency-us = <2700>;
> > + wakeup-latency-us = <1500>;
> > + };
> > + };
> > + };
> > +
> > +...
> > --
> > 2.25.1
> >

Regards,
Anup

2021-03-16 20:47:06

by Rob Herring (Arm)

Subject: Re: [RFC PATCH 7/8] dt-bindings: Add bindings documentation for RISC-V idle states

On Sun, Mar 7, 2021 at 8:18 PM Anup Patel <[email protected]> wrote:
>
> On Sat, Mar 6, 2021 at 4:52 AM Rob Herring <[email protected]> wrote:
> >
> > On Sun, Feb 21, 2021 at 03:07:57PM +0530, Anup Patel wrote:
> > > The RISC-V CPU idle states will be described in DT under the
> > > /cpus/riscv-idle-states DT node. This patch adds the bindings
> > > documentation for riscv-idle-states DT nodes and idle state DT
> > > nodes under it.
> > >
> > > Signed-off-by: Anup Patel <[email protected]>
> > > ---
> > > .../bindings/riscv/idle-states.yaml | 250 ++++++++++++++++++
> > > 1 file changed, 250 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/riscv/idle-states.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/riscv/idle-states.yaml b/Documentation/devicetree/bindings/riscv/idle-states.yaml
> > > new file mode 100644
> > > index 000000000000..3eff763fed23
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/riscv/idle-states.yaml
> > > @@ -0,0 +1,250 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/riscv/idle-states.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: RISC-V idle states binding description
> > > +
> > > +maintainers:
> > > + - Anup Patel <[email protected]>
> > > +
> > > +description: |+
> > > + RISC-V systems can manage power consumption dynamically, where HARTs
> > > + (or CPUs) [1] can be put in different platform specific suspend (or
> > > + idle) states (ranging from simple WFI, power gating, etc). The RISC-V
> > > + SBI [2] hart state management extension provides a standard mechanism
> > > + for OSes to request HART state transitions.
> > > +
> > > + The platform specific suspend (or idle) states of a hart can be either
> > > + retentive or non-rententive in nature. A retentive suspend state will
> > > + preserve hart register and CSR values for all privilege modes whereas
> > > + a non-retentive suspend state will not preserve hart register and CSR
> > > + values. The suspend (or idle) state entered by executing the WFI
> > > + instruction is considered standard on all RISC-V systems and therefore
> > > + must not be listed in device tree.
> > > +
> > > + The device tree binding definition for RISC-V idle states described
> > > + in this document is quite similar to the ARM idle states [3].
> > > +
> > > + References
> > > +
> > > + [1] RISC-V Linux Kernel documentation - CPUs bindings
> > > + Documentation/devicetree/bindings/riscv/cpus.yaml
> > > +
> > > + [2] RISC-V Supervisor Binary Interface (SBI)
> > > + http://github.com/riscv/riscv-sbi-doc/riscv-sbi.adoc
> > > +
> > > + [3] ARM idle states binding description - Idle states bindings
> > > + Documentation/devicetree/bindings/arm/idle-states.yaml
> >
> > I'd assume there's common parts we can share.
>
> Yes, except few properties most are the same.
>
> We can have a shared DT bindings for both ARM and RISC-V but
> both architectures will always have some architecture specific details
> (or properties) which need to be documented under arch specific
> DT documentation. Is it okay if this is done as a separate series ?

No...

> > > +
> > > +properties:
> > > + $nodename:
> > > + const: riscv-idle-states
> >
> > Just 'idle-states' like Arm.
>
> I had tried "idle-states" node name but DT bindings check complaints
> conflict with ARM idle state bindings.

...and this being one reason why.

Actually, I think this can all be in 1 doc if you want. It's fine with
me if a common doc has RiscV and Arm specific properties.

> > > +
> > > +patternProperties:
> > > + "^(cpu|cluster)-":
> > > + type: object
> > > + description: |
> > > + Each state node represents an idle state description and must be
> > > + defined as follows.
> > > +
> >
> > additionalProperties: false
>
> okay, will update.
>
> >
> > > + properties:
> > > + compatible:
> > > + const: riscv,idle-state
> > > +
> > > + local-timer-stop:
> > > + description:
> > > + If present the CPU local timer control logic is lost on state
> > > + entry, otherwise it is retained.
> > > + type: boolean
> > > +
> > > + entry-latency-us:
> > > + description:
> > > + Worst case latency in microseconds required to enter the idle state.
> > > +
> > > + exit-latency-us:
> > > + description:
> > > + Worst case latency in microseconds required to exit the idle state.
> > > + The exit-latency-us duration may be guaranteed only after
> > > + entry-latency-us has passed.
> > > +
> > > + min-residency-us:
> > > + description:
> > > + Minimum residency duration in microseconds, inclusive of preparation
> > > + and entry, for this idle state to be considered worthwhile energy
> > > + wise (refer to section 2 of this document for a complete description).
> > > +
> > > + wakeup-latency-us:
> > > + description: |
> > > + Maximum delay between the signaling of a wake-up event and the CPU
> > > + being able to execute normal code again. If omitted, this is assumed
> > > + to be equal to:
> > > +
> > > + entry-latency-us + exit-latency-us
> > > +
> > > + It is important to supply this value on systems where the duration
> > > + of PREP phase (see diagram 1, section 2) is non-neglibigle. In such
> > > + systems entry-latency-us + exit-latency-us will exceed
> > > + wakeup-latency-us by this duration.
> > > +
> > > + idle-state-name:
> > > + $ref: /schemas/types.yaml#/definitions/string
> > > + description:
> > > + A string used as a descriptive name for the idle state.
> > > +
> > > + required:
> > > + - compatible
> > > + - entry-latency-us
> > > + - exit-latency-us
> > > + - min-residency-us
> > > +
> > > +additionalProperties: false
>
> I will move this up.

TBC, you need this at 2 levels. Both the idle-states node and child nodes.

Rob

2021-03-18 11:04:42

by Anup Patel

Subject: Re: [RFC PATCH 7/8] dt-bindings: Add bindings documentation for RISC-V idle states

On Tue, Mar 16, 2021 at 9:24 PM Rob Herring <[email protected]> wrote:
>
> On Sun, Mar 7, 2021 at 8:18 PM Anup Patel <[email protected]> wrote:
> >
> > On Sat, Mar 6, 2021 at 4:52 AM Rob Herring <[email protected]> wrote:
> > >
> > > On Sun, Feb 21, 2021 at 03:07:57PM +0530, Anup Patel wrote:
> > > > The RISC-V CPU idle states will be described in DT under the
> > > > /cpus/riscv-idle-states DT node. This patch adds the bindings
> > > > documentation for riscv-idle-states DT nodes and idle state DT
> > > > nodes under it.
> > > >
> > > > Signed-off-by: Anup Patel <[email protected]>
> > > > ---
> > > > .../bindings/riscv/idle-states.yaml | 250 ++++++++++++++++++
> > > > 1 file changed, 250 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/riscv/idle-states.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/riscv/idle-states.yaml b/Documentation/devicetree/bindings/riscv/idle-states.yaml
> > > > new file mode 100644
> > > > index 000000000000..3eff763fed23
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/riscv/idle-states.yaml
> > > > @@ -0,0 +1,250 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/riscv/idle-states.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: RISC-V idle states binding description
> > > > +
> > > > +maintainers:
> > > > + - Anup Patel <[email protected]>
> > > > +
> > > > +description: |+
> > > > + RISC-V systems can manage power consumption dynamically, where HARTs
> > > > + (or CPUs) [1] can be put in different platform specific suspend (or
> > > > + idle) states (ranging from simple WFI, power gating, etc). The RISC-V
> > > > + SBI [2] hart state management extension provides a standard mechanism
> > > > + for OSes to request HART state transitions.
> > > > +
> > > > + The platform specific suspend (or idle) states of a hart can be either
> > > > + retentive or non-rententive in nature. A retentive suspend state will
> > > > + preserve hart register and CSR values for all privilege modes whereas
> > > > + a non-retentive suspend state will not preserve hart register and CSR
> > > > + values. The suspend (or idle) state entered by executing the WFI
> > > > + instruction is considered standard on all RISC-V systems and therefore
> > > > + must not be listed in device tree.
> > > > +
> > > > + The device tree binding definition for RISC-V idle states described
> > > > + in this document is quite similar to the ARM idle states [3].
> > > > +
> > > > + References
> > > > +
> > > > + [1] RISC-V Linux Kernel documentation - CPUs bindings
> > > > + Documentation/devicetree/bindings/riscv/cpus.yaml
> > > > +
> > > > + [2] RISC-V Supervisor Binary Interface (SBI)
> > > > + http://github.com/riscv/riscv-sbi-doc/riscv-sbi.adoc
> > > > +
> > > > + [3] ARM idle states binding description - Idle states bindings
> > > > + Documentation/devicetree/bindings/arm/idle-states.yaml
> > >
> > > I'd assume there's common parts we can share.
> >
> > Yes, except few properties most are the same.
> >
> > We can have a shared DT bindings for both ARM and RISC-V but
> > both architectures will always have some architecture specific details
> > (or properties) which need to be documented under arch specific
> > DT documentation. Is it okay if this is done as a separate series ?
>
> No...

Okay, I will create a common DT bindings for both ARM and RISC-V
in the next revision.

>
> > > > +
> > > > +properties:
> > > > + $nodename:
> > > > + const: riscv-idle-states
> > >
> > > Just 'idle-states' like Arm.
> >
> > I had tried "idle-states" node name but DT bindings check complaints
> > conflict with ARM idle state bindings.
>
> ...and this being one reason why.
>
> Actually, I think this can all be in 1 doc if you want. It's fine with
> me if a common doc has RiscV and Arm specific properties.

Sure, will add common DT bindings.

>
> > > > +
> > > > +patternProperties:
> > > > + "^(cpu|cluster)-":
> > > > + type: object
> > > > + description: |
> > > > + Each state node represents an idle state description and must be
> > > > + defined as follows.
> > > > +
> > >
> > > additionalProperties: false
> >
> > okay, will update.
> >
> > >
> > > > + properties:
> > > > + compatible:
> > > > + const: riscv,idle-state
> > > > +
> > > > + local-timer-stop:
> > > > + description:
> > > > + If present the CPU local timer control logic is lost on state
> > > > + entry, otherwise it is retained.
> > > > + type: boolean
> > > > +
> > > > + entry-latency-us:
> > > > + description:
> > > > + Worst case latency in microseconds required to enter the idle state.
> > > > +
> > > > + exit-latency-us:
> > > > + description:
> > > > + Worst case latency in microseconds required to exit the idle state.
> > > > + The exit-latency-us duration may be guaranteed only after
> > > > + entry-latency-us has passed.
> > > > +
> > > > + min-residency-us:
> > > > + description:
> > > > + Minimum residency duration in microseconds, inclusive of preparation
> > > > + and entry, for this idle state to be considered worthwhile energy
> > > > + wise (refer to section 2 of this document for a complete description).
> > > > +
> > > > + wakeup-latency-us:
> > > > + description: |
> > > > + Maximum delay between the signaling of a wake-up event and the CPU
> > > > + being able to execute normal code again. If omitted, this is assumed
> > > > + to be equal to:
> > > > +
> > > > + entry-latency-us + exit-latency-us
> > > > +
> > > > + It is important to supply this value on systems where the duration
> > > > + of PREP phase (see diagram 1, section 2) is non-neglibigle. In such
> > > > + systems entry-latency-us + exit-latency-us will exceed
> > > > + wakeup-latency-us by this duration.
> > > > +
> > > > + idle-state-name:
> > > > + $ref: /schemas/types.yaml#/definitions/string
> > > > + description:
> > > > + A string used as a descriptive name for the idle state.
> > > > +
> > > > + required:
> > > > + - compatible
> > > > + - entry-latency-us
> > > > + - exit-latency-us
> > > > + - min-residency-us
> > > > +
> > > > +additionalProperties: false
> >
> > I will move this up.
>
> TBC, you need this at 2 levels. Both the idle-states node and child nodes.

Sure, I will add at both levels.

Regards,
Anup