| Message ID | 20241028033928.223218-3-nick.hu@sifive.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Support SSTC while PM operations | expand |
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?
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
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; }