[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240115-b3536efde6699e67fa15ac65@orel>
Date: Mon, 15 Jan 2024 16:23:07 +0100
From: Andrew Jones <ajones@...tanamicro.com>
To: Sunil V L <sunilvl@...tanamicro.com>
Cc: linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, linux-riscv@...ts.infradead.org,
"Rafael J . Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>, Anup Patel <anup@...infault.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>, Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Conor Dooley <conor@...nel.org>, Atish Kumar Patra <atishp@...osinc.com>,
Pavel Machek <pavel@....cz>
Subject: Re: [PATCH v2 -next 1/3] cpuidle: RISC-V: Move few functions to
arch/riscv
On Mon, Jan 15, 2024 at 03:40:54PM +0530, Sunil V L wrote:
> To support ACPI Low Power Idle (LPI), few functions are required which
> are currently static functions in the DT based cpuidle driver. Hence,
> move them under arch/riscv so that ACPI driver also can use them.
>
> Signed-off-by: Sunil V L <sunilvl@...tanamicro.com>
> ---
> arch/riscv/include/asm/suspend.h | 3 ++
> arch/riscv/kernel/suspend.c | 47 +++++++++++++++++++++++++++++
> drivers/cpuidle/cpuidle-riscv-sbi.c | 41 +------------------------
> 3 files changed, 51 insertions(+), 40 deletions(-)
>
> diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
> index 02f87867389a..5c7df5ab7a16 100644
> --- a/arch/riscv/include/asm/suspend.h
> +++ b/arch/riscv/include/asm/suspend.h
> @@ -55,4 +55,7 @@ int hibernate_resume_nonboot_cpu_disable(void);
> asmlinkage void hibernate_restore_image(unsigned long resume_satp, unsigned long satp_temp,
> unsigned long cpu_resume);
> asmlinkage int hibernate_core_restore_code(void);
> +bool is_sbi_hsm_supported(void);
> +bool sbi_suspend_state_is_valid(u32 state);
> +int sbi_suspend(u32 state);
> #endif
> diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
> index 239509367e42..a3b2e7e16a98 100644
> --- a/arch/riscv/kernel/suspend.c
> +++ b/arch/riscv/kernel/suspend.c
> @@ -128,4 +128,51 @@ static int __init sbi_system_suspend_init(void)
> }
>
> arch_initcall(sbi_system_suspend_init);
> +
> +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;
> +}
> +
> +int sbi_suspend(u32 state)
Now that this is exported, I'd name it riscv_sbi_suspend.
> +{
> + if (state & SBI_HSM_SUSP_NON_RET_BIT)
> + return cpu_suspend(state, sbi_suspend_finisher);
> + else
> + return sbi_suspend_finisher(state, 0, 0);
> +}
> +
> +bool sbi_suspend_state_is_valid(u32 state)
Also riscv_ prefix here.
> +{
> + 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;
> +}
> +
> +bool is_sbi_hsm_supported(void)
This I would rename to riscv_sbi_hsm_is_supported
> +{
> + /*
> + * The SBI HSM suspend function is only available when:
> + * 1) SBI version is 0.3 or higher
> + * 2) SBI HSM extension is available
> + */
> + if (sbi_spec_version < sbi_mk_version(0, 3) ||
> + !sbi_probe_extension(SBI_EXT_HSM)) {
> + pr_info("HSM suspend not available\n");
> + return false;
> + }
> +
> + return true;
> +}
> #endif /* CONFIG_RISCV_SBI */
> diff --git a/drivers/cpuidle/cpuidle-riscv-sbi.c b/drivers/cpuidle/cpuidle-riscv-sbi.c
> index e8094fc92491..a7f06242f67b 100644
> --- a/drivers/cpuidle/cpuidle-riscv-sbi.c
> +++ b/drivers/cpuidle/cpuidle-riscv-sbi.c
> @@ -73,26 +73,6 @@ static inline bool sbi_is_domain_state_available(void)
> 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 __cpuidle int sbi_cpuidle_enter_state(struct cpuidle_device *dev,
> struct cpuidle_driver *drv, int idx)
> {
> @@ -206,17 +186,6 @@ static const struct of_device_id sbi_cpuidle_state_match[] = {
> { },
> };
>
> -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);
> @@ -607,16 +576,8 @@ static int __init sbi_cpuidle_init(void)
> int ret;
> struct platform_device *pdev;
>
> - /*
> - * The SBI HSM suspend function is only available when:
> - * 1) SBI version is 0.3 or higher
> - * 2) SBI HSM extension is available
> - */
> - if ((sbi_spec_version < sbi_mk_version(0, 3)) ||
> - !sbi_probe_extension(SBI_EXT_HSM)) {
> - pr_info("HSM suspend not available\n");
> + if (!is_sbi_hsm_supported())
> return 0;
> - }
>
> ret = platform_driver_register(&sbi_cpuidle_driver);
> if (ret)
> --
> 2.34.1
>
Otherwise,
Reviewed-by: Andrew Jones <ajones@...tanamicro.com>
Powered by blists - more mailing lists