[RFC PATCH 1/1] riscv: sbi: Introduce system suspend support

[Andrew Jones]

On Tue, Jan 10, 2023 at 10:23:00PM +0000, Conor Dooley wrote:
> Hey Drew!
> 
> On Fri, Jan 06, 2023 at 12:32:16PM +0100, Andrew Jones wrote:
> > When the SUSP SBI extension is present it implies that the standard
> > "suspend to RAM" type is available. Wire it up to the generic
> > platform suspend support, also applying the already present support
> > for non-retentive CPU suspend. When the kernel is built with
> > CONFIG_SUSPEND, one can do 'echo mem > /sys/power/state' to suspend.
> > Resumption will occur when a platform-specific wake-up event arrives.
> > 
> > Signed-off-by: Andrew Jones <ajones at ventanamicro.com>
> 
> First things first, anything SBI depends on !m-mode, so you've gotta add
> some sort of gating unfortunately around those ECALLs. But I figure you
> may have already seen the build failures on patchwork for nommu?

Actually, no, and I think I missed other emails like that in the past. I
assume I'm on the To: of these messages, so it's odd they're not showing
up. Can you forward me this message? I'll inspect the headers and try to
figure what's going on.

> 
> Also, when there's an actual spec would you mind doing a Link: spec.pdf?

Will do

> 
> > ---
> >  arch/riscv/Kconfig           |  5 ++++-
> >  arch/riscv/include/asm/sbi.h |  9 ++++++++
> >  arch/riscv/kernel/suspend.c  | 41 ++++++++++++++++++++++++++++++++++++
> >  3 files changed, 54 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index e2b656043abf..a53d94c1953e 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -52,7 +52,7 @@ config RISCV
> >  	select CLONE_BACKWARDS
> >  	select CLINT_TIMER if !MMU
> >  	select COMMON_CLK
> > -	select CPU_PM if CPU_IDLE
> > +	select CPU_PM if (SUSPEND || CPU_IDLE)
> >  	select EDAC_SUPPORT
> >  	select GENERIC_ARCH_TOPOLOGY
> >  	select GENERIC_ATOMIC64 if !64BIT
> > @@ -686,6 +686,9 @@ config PORTABLE
> >  	select OF
> >  	select MMU
> >  
> > +config ARCH_SUSPEND_POSSIBLE
> > +	def_bool y
> 
> Since the content you're adding depends on having an SBI extention,
> does this need to be s/y/RISCV_SBI/?

Indeed. Will do.

> 
> >  menu "Power management options"
> >  
> >  source "kernel/power/Kconfig"
> > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> > index 4ca7fbacff42..9834ba4ce3e4 100644
> > --- a/arch/riscv/include/asm/sbi.h
> > +++ b/arch/riscv/include/asm/sbi.h
> > @@ -29,6 +29,7 @@ enum sbi_ext_id {
> >  	SBI_EXT_RFENCE = 0x52464E43,
> >  	SBI_EXT_HSM = 0x48534D,
> >  	SBI_EXT_SRST = 0x53525354,
> > +	SBI_EXT_SUSP = 0x53555350,
> >  	SBI_EXT_PMU = 0x504D55,
> >  
> >  	/* Experimentals extensions must lie within this range */
> > @@ -113,6 +114,14 @@ enum sbi_srst_reset_reason {
> >  	SBI_SRST_RESET_REASON_SYS_FAILURE,
> >  };
> >  
> > +enum sbi_ext_susp_fid {
> > +	SBI_EXT_SUSP_SUSPEND = 0,
> > +};
> > +
> > +enum sbi_ext_susp_sleep_type {
> > +	SBI_SUSP_SLEEP_TYPE_SUSPEND = 0,
> > +};
> > +
> >  enum sbi_ext_pmu_fid {
> >  	SBI_EXT_PMU_NUM_COUNTERS = 0,
> >  	SBI_EXT_PMU_COUNTER_GET_INFO,
> > diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
> > index 9ba24fb8cc93..bc26e9ae4782 100644
> > --- a/arch/riscv/kernel/suspend.c
> > +++ b/arch/riscv/kernel/suspend.c
> > @@ -4,8 +4,12 @@
> >   * Copyright (c) 2022 Ventana Micro Systems Inc.
> >   */
> >  
> > +#define pr_fmt(fmt) "suspend: " fmt
> > +
> >  #include <linux/ftrace.h>
> > +#include <linux/suspend.h>
> >  #include <asm/csr.h>
> > +#include <asm/sbi.h>
> >  #include <asm/suspend.h>
> >  
> >  static void suspend_save_csrs(struct suspend_context *context)
> > @@ -85,3 +89,40 @@ int cpu_suspend(unsigned long arg,
> >  
> >  	return rc;
> >  }
> 
> And then from here down needs to be #ifdef RISCV_SBI?
> 
> Anyways, probably stating the obvious on an RFC, but ¯\_(ツ)_/¯

Thanks for pointing this out. I made a last minute change to move
the content from arch/riscv/kernel/sbi.c to arch/riscv/kernel/suspend.c
and forgot I needed to worry about RISCV_SBI.

Thanks,
drew