| Message ID | 20240129072625.2837771-1-leyfoon.tan@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] clocksource: timer-riscv: Clear timer interrupt on timer initialization | expand |
On Mon, Jan 29, 2024 at 1:12 PM Ley Foon Tan <leyfoon.tan@starfivetech.com> wrote: > > In the RISC-V specification, the stimecmp register doesn't have a default > value. To prevent the timer interrupt from being triggered during timer > initialization, clear the timer interrupt by writing stimecmp with a > maximum value. > > Fixes: 9f7a8ff6391f ("RISC-V: Prefer sstc extension if available") > Cc: <stable@vger.kernel.org> > Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> LGTM. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup > > --- > v2: > Resolved comments from Anup. > - Moved riscv_clock_event_stop() to riscv_timer_starting_cpu(). > - Added Fixes tag > --- > drivers/clocksource/timer-riscv.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index e66dcbd66566..672669eb7281 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -116,6 +116,9 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > ce->rating = 450; > clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff); > > + /* Clear timer interrupt */ > + riscv_clock_event_stop(); > + > enable_percpu_irq(riscv_clock_event_irq, > irq_get_trigger_type(riscv_clock_event_irq)); > return 0; > -- > 2.43.0 > >
On 2024-01-29 1:26 AM, Ley Foon Tan wrote: > In the RISC-V specification, the stimecmp register doesn't have a default > value. To prevent the timer interrupt from being triggered during timer > initialization, clear the timer interrupt by writing stimecmp with a > maximum value. > > Fixes: 9f7a8ff6391f ("RISC-V: Prefer sstc extension if available") > Cc: <stable@vger.kernel.org> > Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> > > --- > v2: > Resolved comments from Anup. > - Moved riscv_clock_event_stop() to riscv_timer_starting_cpu(). > - Added Fixes tag > --- > drivers/clocksource/timer-riscv.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index e66dcbd66566..672669eb7281 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -116,6 +116,9 @@ static int riscv_timer_starting_cpu(unsigned int cpu) > ce->rating = 450; > clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff); > > + /* Clear timer interrupt */ > + riscv_clock_event_stop(); This change breaks boot on Unmatched. The bug is that the above call to clockevents_config_and_register() sets the timer (see below), but the timer interrupt never fires due to the added call to riscv_clock_event_stop(). You need to move this line earlier in the function. Here's the stack trace from the initial call to riscv_clock_next_event(): riscv_clock_next_event+0x12/0x3c clockevents_program_event+0x9c/0x1c6 tick_setup_periodic+0x82/0x9e tick_setup_device+0xa0/0xbe tick_check_new_device+0x96/0xc6 clockevents_register_device+0xbe/0x128 clockevents_config_and_register+0x20/0x30 riscv_timer_starting_cpu+0xa2/0xec cpuhp_invoke_callback+0x160/0x61e cpuhp_issue_call+0x140/0x16e __cpuhp_setup_state_cpuslocked+0x186/0x2b0 __cpuhp_setup_state+0x3a/0x60 riscv_timer_init_common+0xea/0x184 riscv_timer_init_dt+0xbe/0xc2 timer_probe+0x70/0xdc time_init+0x74/0xa0 start_kernel+0x194/0x35e Regards, Samuel > + > enable_percpu_irq(riscv_clock_event_irq, > irq_get_trigger_type(riscv_clock_event_irq)); > return 0;
diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c index e66dcbd66566..672669eb7281 100644 --- a/drivers/clocksource/timer-riscv.c +++ b/drivers/clocksource/timer-riscv.c @@ -116,6 +116,9 @@ static int riscv_timer_starting_cpu(unsigned int cpu) ce->rating = 450; clockevents_config_and_register(ce, riscv_timebase, 100, 0x7fffffff); + /* Clear timer interrupt */ + riscv_clock_event_stop(); + enable_percpu_irq(riscv_clock_event_irq, irq_get_trigger_type(riscv_clock_event_irq)); return 0;
In the RISC-V specification, the stimecmp register doesn't have a default value. To prevent the timer interrupt from being triggered during timer initialization, clear the timer interrupt by writing stimecmp with a maximum value. Fixes: 9f7a8ff6391f ("RISC-V: Prefer sstc extension if available") Cc: <stable@vger.kernel.org> Signed-off-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> --- v2: Resolved comments from Anup. - Moved riscv_clock_event_stop() to riscv_timer_starting_cpu(). - Added Fixes tag --- drivers/clocksource/timer-riscv.c | 3 +++ 1 file changed, 3 insertions(+)