[PATCH v6 11/11] platform: sifive: Add SiFive SMC0 driver
Nick Hu
nick.hu at sifive.com
Wed Sep 10 19:13:56 PDT 2025
On Tue, Sep 9, 2025 at 5:08 PM Anup Patel <anup at brainfault.org> wrote:
>
> On Tue, Sep 9, 2025 at 2:11 PM Anup Patel <anup at brainfault.org> wrote:
> >
> > On Tue, Sep 9, 2025 at 8:07 AM Nick Hu <nick.hu at sifive.com> wrote:
> > >
> > > On Mon, Sep 8, 2025 at 7:31 PM Anup Patel <anup at brainfault.org> wrote:
> > > >
> > > > On Fri, Sep 5, 2025 at 7:46 AM Nick Hu <nick.hu at sifive.com> wrote:
> > > > >
> > > > > The SiFive SMC0 controls the clock and power domain of the core complex
> > > > > on the SiFive platform. The core complex enters the low power state
> > > > > after the secondary cores enter the tile power gating and last core
> > > > > execute the `CEASE` instruction with the corresponding SMC0
> > > > > configurations. The devices that inside both tile power domain and core
> > > > > complex power domain will be off, including caches and timer. Therefore
> > > > > we need to flush the last level cache before entering the core complex
> > > > > power gating and update the timer after waking up.
> > > > >
> > > > > Reviewed-by: Cyan Yang <cyan.yang at sifive.com>
> > > > > Reviewed-by: Anup Patel <anup at brainfault.org>
> > > > > Signed-off-by: Nick Hu <nick.hu at sifive.com>
> > > > > ---
> > > > > include/sbi_utils/hsm/fdt_hsm_sifive_tmc0.h | 14 +
> > > > > .../sbi_utils/suspend/fdt_suspend_sifive_smc0.h | 18 ++
> > > > > lib/utils/hsm/fdt_hsm_sifive_tmc0.c | 89 ++++++
> > > > > lib/utils/suspend/Kconfig | 4 +
> > > > > lib/utils/suspend/fdt_suspend_sifive_smc0.c | 323 +++++++++++++++++++++
> > > > > lib/utils/suspend/objects.mk | 3 +
> > > > > platform/generic/configs/defconfig | 1 +
> > > > > 7 files changed, 452 insertions(+)
> > > > >
> > > > > diff --git a/include/sbi_utils/hsm/fdt_hsm_sifive_tmc0.h b/include/sbi_utils/hsm/fdt_hsm_sifive_tmc0.h
> > > > > new file mode 100644
> > > > > index 0000000000000000000000000000000000000000..06cfb390f6af32f11856d869caf52b212aceb9b0
> > > > > --- /dev/null
> > > > > +++ b/include/sbi_utils/hsm/fdt_hsm_sifive_tmc0.h
> > > > > @@ -0,0 +1,14 @@
> > > > > +/*
> > > > > + * SPDX-License-Identifier: BSD-2-Clause
> > > > > + *
> > > > > + * Copyright (c) 2025 SiFive Inc.
> > > > > + */
> > > > > +
> > > > > +#ifndef __FDT_HSM_SIFIVE_TMC0_H__
> > > > > +#define __FDT_HSM_SIFIVE_TMC0_H__
> > > > > +
> > > > > +int sifive_tmc0_set_wakemask_enareq(u32 hartid);
> > > > > +void sifive_tmc0_set_wakemask_disreq(u32 hartid);
> > > > > +bool sifive_tmc0_is_pg(u32 hartid);
> > > > > +
> > > > > +#endif
> > > > > diff --git a/include/sbi_utils/suspend/fdt_suspend_sifive_smc0.h b/include/sbi_utils/suspend/fdt_suspend_sifive_smc0.h
> > > > > new file mode 100644
> > > > > index 0000000000000000000000000000000000000000..f092be529ccd5e316a97051235ea400ae9447ea7
> > > > > --- /dev/null
> > > > > +++ b/include/sbi_utils/suspend/fdt_suspend_sifive_smc0.h
> > > > > @@ -0,0 +1,18 @@
> > > > > +/*
> > > > > + * SPDX-License-Identifier: BSD-2-Clause
> > > > > + *
> > > > > + * Copyright (c) 2025 SiFive Inc.
> > > > > + */
> > > > > +
> > > > > +#ifndef __FDT_SUSPEND_SIFIVE_SMC0_H__
> > > > > +#define __FDT_SUSPEND_SIFIVE_SMC0_H__
> > > > > +
> > > > > +#ifdef CONFIG_FDT_SUSPEND_SIFIVE_SMC0
> > > > > +bool sifive_smc0_check_warm_reset(void);
> > > > > +void sifive_smc0_mtime_update(void);
> > > > > +#else
> > > > > +static inline bool sifive_smc0_check_warm_reset(void) { return false; }
> > > > > +static inline void sifive_smc0_mtime_update(void) { return; }
> > > > > +#endif
> > > > > +
> > > > > +#endif
> > > > > diff --git a/lib/utils/hsm/fdt_hsm_sifive_tmc0.c b/lib/utils/hsm/fdt_hsm_sifive_tmc0.c
> > > > > index 768b221d085d15d1b6a4684d01f6addb2598739a..059bf44be125fbea1667f2890b72023bf1ecb1a0 100644
> > > > > --- a/lib/utils/hsm/fdt_hsm_sifive_tmc0.c
> > > > > +++ b/lib/utils/hsm/fdt_hsm_sifive_tmc0.c
> > > > > @@ -18,7 +18,10 @@
> > > > > #include <sbi_utils/fdt/fdt_driver.h>
> > > > > #include <sbi_utils/fdt/fdt_helper.h>
> > > > > #include <sbi_utils/hsm/fdt_hsm_sifive_inst.h>
> > > > > +#include <sbi_utils/hsm/fdt_hsm_sifive_tmc0.h>
> > > > > #include <sbi_utils/ipi/aclint_mswi.h>
> > > > > +#include <sbi_utils/irqchip/aplic.h>
> > > > > +#include <sbi_utils/suspend/fdt_suspend_sifive_smc0.h>
> > > > >
> > > > > struct sifive_tmc0 {
> > > > > unsigned long reg;
> > > > > @@ -78,6 +81,73 @@ static unsigned long tmc0_offset;
> > > > > #define SIFIVE_TMC_WAKE_MASK_WREQ BIT(31)
> > > > > #define SIFIVE_TMC_WAKE_MASK_ACK BIT(30)
> > > > >
> > > > > +int sifive_tmc0_set_wakemask_enareq(u32 hartid)
> > > > > +{
> > > > > + struct sbi_scratch *scratch = sbi_hartid_to_scratch(hartid);
> > > > > + struct sifive_tmc0 *tmc0 = tmc0_ptr_get(scratch);
> > > > > + unsigned long addr;
> > > > > + u32 v;
> > > > > +
> > > > > + if (!tmc0)
> > > > > + return SBI_ENODEV;
> > > > > +
> > > > > + addr = tmc0->reg + SIFIVE_TMC_WAKE_MASK_OFF;
> > > > > + v = readl((void *)addr);
> > > > > + writel(v | SIFIVE_TMC_WAKE_MASK_WREQ, (void *)addr);
> > > > > +
> > > > > + while (!(readl((void *)addr) & SIFIVE_TMC_WAKE_MASK_ACK));
> > > > > +
> > > > > + return SBI_OK;
> > > > > +}
> > > > > +
> > > > > +void sifive_tmc0_set_wakemask_disreq(u32 hartid)
> > > > > +{
> > > > > + struct sbi_scratch *scratch = sbi_hartid_to_scratch(hartid);
> > > > > + struct sifive_tmc0 *tmc0 = tmc0_ptr_get(scratch);
> > > > > + unsigned long addr;
> > > > > + u32 v;
> > > > > +
> > > > > + if (!tmc0)
> > > > > + return;
> > > > > +
> > > > > + addr = tmc0->reg + SIFIVE_TMC_WAKE_MASK_OFF;
> > > > > + v = readl((void *)addr);
> > > > > + writel(v & ~SIFIVE_TMC_WAKE_MASK_WREQ, (void *)addr);
> > > > > +
> > > > > + while (readl((void *)addr) & SIFIVE_TMC_WAKE_MASK_ACK);
> > > > > +}
> > > > > +
> > > > > +bool sifive_tmc0_is_pg(u32 hartid)
> > > > > +{
> > > > > + struct sbi_scratch *scratch = sbi_hartid_to_scratch(hartid);
> > > > > + struct sifive_tmc0 *tmc0 = tmc0_ptr_get(scratch);
> > > > > + unsigned long addr;
> > > > > + u32 v;
> > > > > +
> > > > > + if (!tmc0)
> > > > > + return false;
> > > > > +
> > > > > + addr = tmc0->reg + SIFIVE_TMC_PG_OFF;
> > > > > + v = readl((void *)addr);
> > > > > + if (!(v & SIFIVE_TMC_PG_ENA_ACK) ||
> > > > > + (v & SIFIVE_TMC_PG_ENARSP) ||
> > > > > + (v & SIFIVE_TMC_PG_DIS_REQ))
> > > > > + return false;
> > > > > +
> > > > > + return true;
> > > > > +}
> > > > > +
> > > > > +static bool sifive_tmc0_check_warm_reset(void)
> > > > > +{
> > > > > + struct sifive_tmc0 *tmc0 = tmc0_ptr_get(sbi_scratch_thishart_ptr());
> > > > > + unsigned long addr = tmc0->reg + SIFIVE_TMC_PG_OFF;
> > > > > +
> > > > > + if (!tmc0)
> > > > > + return false;
> > > > > +
> > > > > + return readl((void *)addr) & SIFIVE_TMC_PG_WARM_RESET ? true : false;
> > > > > +}
> > > > > +
> > > > > static void sifive_tmc0_set_resumepc(physical_addr_t addr)
> > > > > {
> > > > > struct sifive_tmc0 *tmc0 = tmc0_ptr_get(sbi_scratch_thishart_ptr());
> > > > > @@ -229,10 +299,29 @@ static int sifive_tmc0_stop(void)
> > > > > return SBI_OK;
> > > > > }
> > > > >
> > > > > +static void sifive_pg_resume(void)
> > > > > +{
> > > > > + /*
> > > > > + * To simplify the implementation, the SW won't clear the warm
> > > > > + * reset bit. If the system woken up from the Core Complex power
> > > > > + * gating, the warm reset bit of the TMC0 will be clear. Therefore
> > > > > + * we need to check the warm reset bit of the TMC0 first as the
> > > > > + * Tile power gating won't clear the warm reset bit of the SMC0.
> > > > > + */
> > > > > + if (sifive_tmc0_check_warm_reset())
> > > > > + return;
> > > > > +
> > > > > + if (sifive_smc0_check_warm_reset()) {
> > > > > + aplic_reinit_all();
> > > > > + sifive_smc0_mtime_update();
> > > >
> > > > Since you're directly calling aplic_reinit_all() over here, the
> > > > TMC0 Kconfig option should depend upon the APLIC Kconfig
> > > > option.
> > > >
> > > > The only other question I have is why are we re-initializing
> > > > all APLICs in hart_resume() callback which is a per-HART
> > > > function.
> > > >
> > > Currently, we only expect the APLICs to be reset in the system suspend
> > > path, where one hart executes hart_resume() while the others follow
> > > the init_warm_startup() flow.
> > > However, I believe your concern is that if the user specifies
> > > suspend_type == SBI_HSM_SUSPEND_NON_RET_DEFAULT, it would cause every
> > > hart to execute hart_resume().
> > > I’ll address this in the next version. Thanks!
> >
> > I see the issue now. You are relying on the hart_resume()
> > callback for doing APLIC re-init in the system resume path
> > because there is no system_resume() callback in
> > "struct sbi_system_suspend_device".
> >
> > I think it is better to define system_resume() callback in
> > "struct sbi_system_suspend_device" and the function
> > sbi_hsm_hart_resume_start() should call this new
> > system_resume() callback when resuming from system
> > suspend. This way the system suspend/resume path
> > remains separate from hsm suspend/resume path and
> > you can happily call APLIC re-init from system resume
> > path.
> >
>
> You can do something like below as a separate patch in this
> series ....
>
> diff --git a/include/sbi/sbi_hsm.h b/include/sbi/sbi_hsm.h
> index 38f90516..233407bf 100644
> --- a/include/sbi/sbi_hsm.h
> +++ b/include/sbi/sbi_hsm.h
> @@ -75,6 +75,7 @@ void __noreturn sbi_hsm_hart_resume_finish(struct
> sbi_scratch *scratch,
> u32 hartid);
> int sbi_hsm_hart_suspend(struct sbi_scratch *scratch, u32 suspend_type,
> ulong raddr, ulong rmode, ulong arg1);
> +void sbi_hsm_hart_set_system_suspend(struct sbi_scratch *scratch, bool enable);
> bool sbi_hsm_hart_change_state(struct sbi_scratch *scratch, long oldstate,
> long newstate);
> int __sbi_hsm_hart_get_state(u32 hartindex);
> diff --git a/include/sbi/sbi_system.h b/include/sbi/sbi_system.h
> index 0fdcc98c..eb78655f 100644
> --- a/include/sbi/sbi_system.h
> +++ b/include/sbi/sbi_system.h
> @@ -69,12 +69,18 @@ struct sbi_system_suspend_device {
> * return from system_suspend() may ignore this parameter.
> */
> int (*system_suspend)(u32 sleep_type, unsigned long mmode_resume_addr);
> +
> + /** Resume from system suspend */
> + void (*system_resume)(void);
> };
>
> +struct sbi_scratch;
> +
> const struct sbi_system_suspend_device *sbi_system_suspend_get_device(void);
> void sbi_system_suspend_set_device(struct sbi_system_suspend_device *dev);
> void sbi_system_suspend_test_enable(void);
> bool sbi_system_suspend_supported(u32 sleep_type);
> +void sbi_system_resume(struct sbi_scratch *scratch);
> int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque);
>
> #endif
> diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
> index 557ab131..62d7110a 100644
> --- a/lib/sbi/sbi_hsm.c
> +++ b/lib/sbi/sbi_hsm.c
> @@ -43,6 +43,7 @@ static int hsm_device_hart_stop(void);
> /** Per hart specific data to manage state transition **/
> struct sbi_hsm_data {
> atomic_t state;
> + bool system_suspend;
Would it make sense to store this information in
sbi_system_suspend_device, so it can be shared across all CPUs using
the same sbi_system_suspend_device?
> unsigned long suspend_type;
> unsigned long saved_mie;
> unsigned long saved_mip;
> @@ -54,6 +55,13 @@ struct sbi_hsm_data {
> atomic_t start_ticket;
> };
>
> +void sbi_hsm_hart_set_system_suspend(struct sbi_scratch *scratch, bool enable)
> +{
> + struct sbi_hsm_data *hdata = sbi_scratch_offset_ptr(scratch,
> + hart_data_offset);
> + hdata->system_suspend = enable;
> +}
> +
> bool sbi_hsm_hart_change_state(struct sbi_scratch *scratch, long oldstate,
> long newstate)
> {
> @@ -463,7 +471,10 @@ void sbi_hsm_hart_resume_start(struct sbi_scratch *scratch)
> SBI_HSM_STATE_RESUME_PENDING))
> sbi_hart_hang();
>
> - hsm_device_hart_resume();
> + if (hdata->system_suspend)
> + sbi_system_resume(scratch);
> + else
> + hsm_device_hart_resume();
> }
>
> void __noreturn sbi_hsm_hart_resume_finish(struct sbi_scratch *scratch,
> diff --git a/lib/sbi/sbi_system.c b/lib/sbi/sbi_system.c
> index cd0f4ba4..f3eedbbf 100644
> --- a/lib/sbi/sbi_system.c
> +++ b/lib/sbi/sbi_system.c
> @@ -137,6 +137,13 @@ bool sbi_system_suspend_supported(u32 sleep_type)
> suspend_dev->system_suspend_check(sleep_type) == 0;
> }
>
> +void sbi_system_resume(struct sbi_scratch *scratch)
> +{
> + sbi_hsm_hart_set_system_suspend(scratch, false);
> + if (suspend_dev && suspend_dev->system_resume)
> + suspend_dev->system_resume();
> +}
> +
> int sbi_system_suspend(u32 sleep_type, ulong resume_addr, ulong opaque)
> {
> struct sbi_domain *dom = sbi_domain_thishart_ptr();
> @@ -177,9 +184,13 @@ int sbi_system_suspend(u32 sleep_type, ulong
> resume_addr, ulong opaque)
> SBI_DOMAIN_EXECUTE))
> return SBI_EINVALID_ADDR;
>
> + sbi_hsm_hart_set_system_suspend(scratch, true);
> +
> if (!sbi_hsm_hart_change_state(scratch, SBI_HSM_STATE_STARTED,
> - SBI_HSM_STATE_SUSPENDED))
> + SBI_HSM_STATE_SUSPENDED)) {
> + sbi_hsm_hart_set_system_suspend(scratch, false);
> return SBI_EFAIL;
> + }
>
> /* Prepare for resume */
> scratch->next_mode = prev_mode;
> @@ -194,6 +205,7 @@ int sbi_system_suspend(u32 sleep_type, ulong
> resume_addr, ulong opaque)
> if (!sbi_hsm_hart_change_state(scratch, SBI_HSM_STATE_SUSPENDED,
> SBI_HSM_STATE_STARTED))
> sbi_hart_hang();
> + sbi_hsm_hart_set_system_suspend(scratch, false);
> return ret;
> }
>
>
> Regards,
> Anup
More information about the opensbi
mailing list