[PATCH v3 0/2] Support SSTC while PM operations

[PATCH v3 0/2] Support SSTC while PM operations

[Nick Hu]

When the cpu is going to be hotplug, stop the stimecmp to prevent pending
interrupt.
When the cpu is going to be suspended, save the stimecmp before entering
the suspend state and restore it in the resume path.

changes in v3:
1. Update the commit description
2. Remove csr_read/write_hi_lo from the previous patch
Link: https://lore.kernel.org/lkml/20240926065422.226518-1-nick.hu@sifive.com/T/

changes in v2:
1. Add csr_read/write_hi_lo operations
2. Apply the suggestion from Anup.
Link: https://lore.kernel.org/lkml/20240829033904.477200-3-nick.hu@sifive.com/T/#u

Nick Hu (2):
  riscv: Add stimecmp save and restore
  clocksource/drivers/timer-riscv: Stop stimecmp when cpu hotplug

 arch/riscv/include/asm/suspend.h  |  4 ++++
 arch/riscv/kernel/suspend.c       | 14 ++++++++++++++
 drivers/clocksource/timer-riscv.c |  6 ++++++
 3 files changed, 24 insertions(+)

-- 
2.34.1

[PATCH v3 1/2] riscv: Add stimecmp save and restore

[Nick Hu]

If the HW support the SSTC extension, we should save and restore the
stimecmp register while cpu non retention suspend.

Signed-off-by: Nick Hu <nick.hu@sifive.com>
---
 arch/riscv/include/asm/suspend.h |  4 ++++
 arch/riscv/kernel/suspend.c      | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
index 4ffb022b097f..dc5782b5fbad 100644
--- a/arch/riscv/include/asm/suspend.h
+++ b/arch/riscv/include/asm/suspend.h
@@ -18,6 +18,10 @@ struct suspend_context {
 	unsigned long ie;
 #ifdef CONFIG_MMU
 	unsigned long satp;
+	unsigned long stimecmp;
+#if __riscv_xlen < 64
+	unsigned long stimecmph;
+#endif
 #endif
 };
 
diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
index 9a8a0dc035b2..24b3f57d467f 100644
--- a/arch/riscv/kernel/suspend.c
+++ b/arch/riscv/kernel/suspend.c
@@ -30,6 +30,13 @@ void suspend_save_csrs(struct suspend_context *context)
 	 */
 
 #ifdef CONFIG_MMU
+	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) {
+		context->stimecmp = csr_read(CSR_STIMECMP);
+#if __riscv_xlen < 64
+		context->stimecmph = csr_read(CSR_STIMECMPH);
+#endif
+	}
+
 	context->satp = csr_read(CSR_SATP);
 #endif
 }
@@ -43,6 +50,13 @@ void suspend_restore_csrs(struct suspend_context *context)
 	csr_write(CSR_IE, context->ie);
 
 #ifdef CONFIG_MMU
+	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) {
+		csr_write(CSR_STIMECMP, context->stimecmp);
+#if __riscv_xlen < 64
+		csr_write(CSR_STIMECMPH, context->stimecmph);
+#endif
+	}
+
 	csr_write(CSR_SATP, context->satp);
 #endif
 }
-- 
2.34.1

[PATCH v3 2/2] clocksource/drivers/timer-riscv: Stop stimecmp when cpu hotplug

[Nick Hu]

Stop the timer when the cpu is going to be offline otherwise the
timer interrupt may be pending while performing power-down.

Suggested-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/lkml/20240829033904.477200-3-nick.hu@sifive.com/T/#u
Signed-off-by: Nick Hu <nick.hu@sifive.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 drivers/clocksource/timer-riscv.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
index 48ce50c5f5e6..166dee14e46b 100644
--- a/drivers/clocksource/timer-riscv.c
+++ b/drivers/clocksource/timer-riscv.c
@@ -127,6 +127,12 @@ static int riscv_timer_starting_cpu(unsigned int cpu)
 static int riscv_timer_dying_cpu(unsigned int cpu)
 {
 	disable_percpu_irq(riscv_clock_event_irq);
+	/*
+	 * Stop the timer when the cpu is going to be offline otherwise
+	 * the timer interrupt may be pending while performing power-down.
+	 */
+	riscv_clock_event_stop();
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH v3 1/2] riscv: Add stimecmp save and restore

[Anup Patel]

On Mon, Oct 28, 2024 at 9:09 AM Nick Hu <nick.hu@sifive.com> wrote:
>
> If the HW support the SSTC extension, we should save and restore the
> stimecmp register while cpu non retention suspend.
>
> Signed-off-by: Nick Hu <nick.hu@sifive.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/include/asm/suspend.h |  4 ++++
>  arch/riscv/kernel/suspend.c      | 14 ++++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/arch/riscv/include/asm/suspend.h b/arch/riscv/include/asm/suspend.h
> index 4ffb022b097f..dc5782b5fbad 100644
> --- a/arch/riscv/include/asm/suspend.h
> +++ b/arch/riscv/include/asm/suspend.h
> @@ -18,6 +18,10 @@ struct suspend_context {
>         unsigned long ie;
>  #ifdef CONFIG_MMU
>         unsigned long satp;
> +       unsigned long stimecmp;
> +#if __riscv_xlen < 64
> +       unsigned long stimecmph;
> +#endif
>  #endif
>  };
>
> diff --git a/arch/riscv/kernel/suspend.c b/arch/riscv/kernel/suspend.c
> index 9a8a0dc035b2..24b3f57d467f 100644
> --- a/arch/riscv/kernel/suspend.c
> +++ b/arch/riscv/kernel/suspend.c
> @@ -30,6 +30,13 @@ void suspend_save_csrs(struct suspend_context *context)
>          */
>
>  #ifdef CONFIG_MMU
> +       if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) {
> +               context->stimecmp = csr_read(CSR_STIMECMP);
> +#if __riscv_xlen < 64
> +               context->stimecmph = csr_read(CSR_STIMECMPH);
> +#endif
> +       }
> +
>         context->satp = csr_read(CSR_SATP);
>  #endif
>  }
> @@ -43,6 +50,13 @@ void suspend_restore_csrs(struct suspend_context *context)
>         csr_write(CSR_IE, context->ie);
>
>  #ifdef CONFIG_MMU
> +       if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SSTC)) {
> +               csr_write(CSR_STIMECMP, context->stimecmp);
> +#if __riscv_xlen < 64
> +               csr_write(CSR_STIMECMPH, context->stimecmph);
> +#endif
> +       }
> +
>         csr_write(CSR_SATP, context->satp);
>  #endif
>  }
> --
> 2.34.1
>
>

Re: [PATCH v3 2/2] clocksource/drivers/timer-riscv: Stop stimecmp when cpu hotplug

[Daniel Lezcano]

On 28/10/2024 04:39, Nick Hu wrote:
> Stop the timer when the cpu is going to be offline otherwise the
> timer interrupt may be pending while performing power-down.
> 
> Suggested-by: Anup Patel <anup@brainfault.org>
> Link: https://lore.kernel.org/lkml/20240829033904.477200-3-nick.hu@sifive.com/T/#u
> Signed-off-by: Nick Hu <nick.hu@sifive.com>
> Reviewed-by: Anup Patel <anup@brainfault.org>
> ---
>   drivers/clocksource/timer-riscv.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> index 48ce50c5f5e6..166dee14e46b 100644
> --- a/drivers/clocksource/timer-riscv.c
> +++ b/drivers/clocksource/timer-riscv.c
> @@ -127,6 +127,12 @@ static int riscv_timer_starting_cpu(unsigned int cpu)
>   static int riscv_timer_dying_cpu(unsigned int cpu)
>   {
>   	disable_percpu_irq(riscv_clock_event_irq);
> +	/*
> +	 * Stop the timer when the cpu is going to be offline otherwise
> +	 * the timer interrupt may be pending while performing power-down.
> +	 */
> +	riscv_clock_event_stop();
> +
>   	return 0;
>   }

Should it not be the opposite?

First stop the clock which clears the interrupt and then disable the irq?





-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 2/2] clocksource/drivers/timer-riscv: Stop stimecmp when cpu hotplug

[Nick Hu]

Hi Daniel,

On Mon, Oct 28, 2024 at 5:44 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 28/10/2024 04:39, Nick Hu wrote:
> > Stop the timer when the cpu is going to be offline otherwise the
> > timer interrupt may be pending while performing power-down.
> >
> > Suggested-by: Anup Patel <anup@brainfault.org>
> > Link: https://lore.kernel.org/lkml/20240829033904.477200-3-nick.hu@sifive.com/T/#u
> > Signed-off-by: Nick Hu <nick.hu@sifive.com>
> > Reviewed-by: Anup Patel <anup@brainfault.org>
> > ---
> >   drivers/clocksource/timer-riscv.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > index 48ce50c5f5e6..166dee14e46b 100644
> > --- a/drivers/clocksource/timer-riscv.c
> > +++ b/drivers/clocksource/timer-riscv.c
> > @@ -127,6 +127,12 @@ static int riscv_timer_starting_cpu(unsigned int cpu)
> >   static int riscv_timer_dying_cpu(unsigned int cpu)
> >   {
> >       disable_percpu_irq(riscv_clock_event_irq);
> > +     /*
> > +      * Stop the timer when the cpu is going to be offline otherwise
> > +      * the timer interrupt may be pending while performing power-down.
> > +      */
> > +     riscv_clock_event_stop();
> > +
> >       return 0;
> >   }
>
> Should it not be the opposite?
>
> First stop the clock which clears the interrupt and then disable the irq?
>
SIE.STIE = 0 ->
Mtimer interrupt comes -> trap to m-mode -> raise STIP ->
stop the clock
Is the above case you are concerned about?

>
>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 0/2] Support SSTC while PM operations

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (for-next)
by Alexandre Ghiti <alexghiti@rivosinc.com>:

On Mon, 28 Oct 2024 11:39:24 +0800 you wrote:
> When the cpu is going to be hotplug, stop the stimecmp to prevent pending
> interrupt.
> When the cpu is going to be suspended, save the stimecmp before entering
> the suspend state and restore it in the resume path.
> 
> changes in v3:
> 1. Update the commit description
> 2. Remove csr_read/write_hi_lo from the previous patch
> Link: https://lore.kernel.org/lkml/20240926065422.226518-1-nick.hu@sifive.com/T/
> 
> [...]

Here is the summary with links:
  - [v3,1/2] riscv: Add stimecmp save and restore
    https://git.kernel.org/riscv/c/ffef54ad4110
  - [v3,2/2] clocksource/drivers/timer-riscv: Stop stimecmp when cpu hotplug
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html