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

[Andrew Jones]

On Wed, Dec 06, 2023 at 12:23:16PM +0000, Conor Dooley wrote:
> On Wed, Dec 06, 2023 at 12:08:09PM +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>
> > Tested-by: Samuel Holland <samuel.holland at sifive.com>
> 
> > +#ifdef CONFIG_RISCV_SBI
> > +static int sbi_system_suspend(unsigned long sleep_type,
> > +			      unsigned long resume_addr,
> > +			      unsigned long opaque)
> > +{
> > +	struct sbiret ret;
> > +
> > +	ret = sbi_ecall(SBI_EXT_SUSP, SBI_EXT_SUSP_SYSTEM_SUSPEND,
> > +			sleep_type, resume_addr, opaque, 0, 0, 0);
> > +	if (ret.error)
> > +		return sbi_err_map_linux_errno(ret.error);
> > +
> > +	return ret.value;
> > +}
> > +
> > +static int sbi_system_suspend_enter(suspend_state_t state)
> > +{
> > +	return cpu_suspend(SBI_SUSP_SLEEP_TYPE_SUSPEND_TO_RAM, sbi_system_suspend);
> > +}
> > +
> > +static const struct platform_suspend_ops sbi_system_suspend_ops = {
> > +	.valid = suspend_valid_only_mem,
> > +	.enter = sbi_system_suspend_enter,
> > +};
> > +
> > +static int __init sbi_system_suspend_init(void)
> > +{
> > +	if (sbi_spec_version >= sbi_mk_version(2, 0) &&
> > +	    sbi_probe_extension(SBI_EXT_SUSP) > 0) {
> > +		pr_info("SBI SUSP extension detected\n");
> 
> Is there any value in probing and printing this stuff out if
> the kernel is not actually built with support for suspend?

My thinking is that the info message is like those in sbi_init(), which
just state what it sees, whether the kernel wants to use it or not (and
maybe somebody will read the message and choose to recompile with SUSPEND
to enable it...)

> I can see either that or moving the IS_ENABLED() check so that
> it controls whether we even probe for SUSP being valid so
> Reviewed-by: Conor Dooley <conor.dooley at microchip.com>

Thanks,
drew