Messages in this thread | | | Date | Tue, 16 Apr 2024 18:49:51 -0700 (PDT) | Subject | Re: [PATCH] clocksource/drivers/timer-riscv: Drop extra CSR write | From | Palmer Dabbelt <> |
| |
On Wed, 13 Mar 2024 09:56:34 PDT (-0700), apatel@ventanamicro.com wrote: > On Wed, Mar 13, 2024 at 1:03 AM Samuel Holland > <samuel.holland@sifive.com> wrote: >> >> On riscv32, the time comparator value is split across two CSRs. We write >> both when stopping the timer, but realistically the time is just as >> unlikely to reach 0xffffffff00000000 as 0xffffffffffffffff, so there is >> no need to write the low CSR. > > Even though unlikely, there is still a theoretical possibility of > counter reaching value 0xffffffff00000000.
I guess that depends on the timebase frequency, but if my math is right then we've got some timers that will overflow a 32-bit counter in 10 minutes -- take that with a grain of salt as they're all 64-bit systems (we don't have any 32-bit DTs upstream?), but it still seems plausible for 32-bit overflows to happen here on real systems.
> The good thing about value 0xffffffffffffffff is that the counter will > immediately wrap around after reaching it.
I'm not sure how that's good? Luckily we've got ~100,000 years to figure out, even on these systems with pretty fast timers ;)
> Regards, > Anup > > >> >> Signed-off-by: Samuel Holland <samuel.holland@sifive.com> >> --- >> >> drivers/clocksource/timer-riscv.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c >> index e66dcbd66566..eaaf01f3c34b 100644 >> --- a/drivers/clocksource/timer-riscv.c >> +++ b/drivers/clocksource/timer-riscv.c >> @@ -35,9 +35,10 @@ static bool riscv_timer_cannot_wake_cpu; >> static void riscv_clock_event_stop(void) >> { >> if (static_branch_likely(&riscv_sstc_available)) { >> - csr_write(CSR_STIMECMP, ULONG_MAX); >> if (IS_ENABLED(CONFIG_32BIT)) >> csr_write(CSR_STIMECMPH, ULONG_MAX); >> + else >> + csr_write(CSR_STIMECMP, ULONG_MAX); >> } else { >> sbi_set_timer(U64_MAX); >> } >> -- >> 2.43.1 >> >>
| |