| Message ID | 20220527042937.1124009-4-atishp@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Sstc extension support | expand |
On Fri, May 27, 2022 at 9:59 AM Atish Patra <atishp@rivosinc.com> wrote: > > RISC-V ISA has sstc extension which allows updating the next clock event > via a CSR (stimecmp) instead of an SBI call. This should happen dynamically > if sstc extension is available. Otherwise, it will fallback to SBI call > to maintain backward compatibility. > > Signed-off-by: Atish Patra <atishp@rivosinc.com> > --- > drivers/clocksource/timer-riscv.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index 1767f8bf2013..881d9335c92d 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -7,6 +7,9 @@ > * either be read from the "time" and "timeh" CSRs, and can use the SBI to > * setup events, or directly accessed using MMIO registers. > */ > + > +#define pr_fmt(fmt) "timer: " fmt The "timer: " prefix is too generic. I suggest to use "riscv-timer: " as a prefix. > + > #include <linux/clocksource.h> > #include <linux/clockchips.h> > #include <linux/cpu.h> > @@ -23,11 +26,24 @@ > #include <asm/sbi.h> > #include <asm/timex.h> > > +static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > + > static int riscv_clock_next_event(unsigned long delta, > struct clock_event_device *ce) > { > + u64 next_tval = get_cycles64() + delta; > + > csr_set(CSR_IE, IE_TIE); > - sbi_set_timer(get_cycles64() + delta); > + if (static_branch_likely(&riscv_sstc_available)) { > +#if defined(CONFIG_32BIT) > + csr_write(CSR_STIMECMP, next_tval & 0xFFFFFFFF); > + csr_write(CSR_STIMECMPH, next_tval >> 32); > +#else > + csr_write(CSR_STIMECMP, next_tval); > +#endif > + } else > + sbi_set_timer(next_tval); > + > return 0; > } > > @@ -165,6 +181,12 @@ static int __init riscv_timer_init_dt(struct device_node *n) > if (error) > pr_err("cpu hp setup state failed for RISCV timer [%d]\n", > error); > + > + if (riscv_isa_extension_available(NULL, SSTC)) { > + pr_info("Timer interrupt in S-mode is available via sstc extension\n"); > + static_branch_enable(&riscv_sstc_available); > + } > + > return error; > } > > -- > 2.25.1 > Apart from the minor comment above, this looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup
On Thu, Jun 9, 2022 at 9:41 PM Anup Patel <anup@brainfault.org> wrote: > > On Fri, May 27, 2022 at 9:59 AM Atish Patra <atishp@rivosinc.com> wrote: > > > > RISC-V ISA has sstc extension which allows updating the next clock event > > via a CSR (stimecmp) instead of an SBI call. This should happen dynamically > > if sstc extension is available. Otherwise, it will fallback to SBI call > > to maintain backward compatibility. > > > > Signed-off-by: Atish Patra <atishp@rivosinc.com> > > --- > > drivers/clocksource/timer-riscv.c | 24 +++++++++++++++++++++++- > > 1 file changed, 23 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > > index 1767f8bf2013..881d9335c92d 100644 > > --- a/drivers/clocksource/timer-riscv.c > > +++ b/drivers/clocksource/timer-riscv.c > > @@ -7,6 +7,9 @@ > > * either be read from the "time" and "timeh" CSRs, and can use the SBI to > > * setup events, or directly accessed using MMIO registers. > > */ > > + > > +#define pr_fmt(fmt) "timer: " fmt > > The "timer: " prefix is too generic. I suggest to use "riscv-timer: " > as a prefix. > Sured. Fixed in the next version. > > + > > #include <linux/clocksource.h> > > #include <linux/clockchips.h> > > #include <linux/cpu.h> > > @@ -23,11 +26,24 @@ > > #include <asm/sbi.h> > > #include <asm/timex.h> > > > > +static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); > > + > > static int riscv_clock_next_event(unsigned long delta, > > struct clock_event_device *ce) > > { > > + u64 next_tval = get_cycles64() + delta; > > + > > csr_set(CSR_IE, IE_TIE); > > - sbi_set_timer(get_cycles64() + delta); > > + if (static_branch_likely(&riscv_sstc_available)) { > > +#if defined(CONFIG_32BIT) > > + csr_write(CSR_STIMECMP, next_tval & 0xFFFFFFFF); > > + csr_write(CSR_STIMECMPH, next_tval >> 32); > > +#else > > + csr_write(CSR_STIMECMP, next_tval); > > +#endif > > + } else > > + sbi_set_timer(next_tval); > > + > > return 0; > > } > > > > @@ -165,6 +181,12 @@ static int __init riscv_timer_init_dt(struct device_node *n) > > if (error) > > pr_err("cpu hp setup state failed for RISCV timer [%d]\n", > > error); > > + > > + if (riscv_isa_extension_available(NULL, SSTC)) { > > + pr_info("Timer interrupt in S-mode is available via sstc extension\n"); > > + static_branch_enable(&riscv_sstc_available); > > + } > > + > > return error; > > } > > > > -- > > 2.25.1 > > > > Apart from the minor comment above, this looks good to me. > > Reviewed-by: Anup Patel <anup@brainfault.org> > > Regards, > Anup
diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c index 1767f8bf2013..881d9335c92d 100644 --- a/drivers/clocksource/timer-riscv.c +++ b/drivers/clocksource/timer-riscv.c @@ -7,6 +7,9 @@ * either be read from the "time" and "timeh" CSRs, and can use the SBI to * setup events, or directly accessed using MMIO registers. */ + +#define pr_fmt(fmt) "timer: " fmt + #include <linux/clocksource.h> #include <linux/clockchips.h> #include <linux/cpu.h> @@ -23,11 +26,24 @@ #include <asm/sbi.h> #include <asm/timex.h> +static DEFINE_STATIC_KEY_FALSE(riscv_sstc_available); + static int riscv_clock_next_event(unsigned long delta, struct clock_event_device *ce) { + u64 next_tval = get_cycles64() + delta; + csr_set(CSR_IE, IE_TIE); - sbi_set_timer(get_cycles64() + delta); + if (static_branch_likely(&riscv_sstc_available)) { +#if defined(CONFIG_32BIT) + csr_write(CSR_STIMECMP, next_tval & 0xFFFFFFFF); + csr_write(CSR_STIMECMPH, next_tval >> 32); +#else + csr_write(CSR_STIMECMP, next_tval); +#endif + } else + sbi_set_timer(next_tval); + return 0; } @@ -165,6 +181,12 @@ static int __init riscv_timer_init_dt(struct device_node *n) if (error) pr_err("cpu hp setup state failed for RISCV timer [%d]\n", error); + + if (riscv_isa_extension_available(NULL, SSTC)) { + pr_info("Timer interrupt in S-mode is available via sstc extension\n"); + static_branch_enable(&riscv_sstc_available); + } + return error; }
RISC-V ISA has sstc extension which allows updating the next clock event via a CSR (stimecmp) instead of an SBI call. This should happen dynamically if sstc extension is available. Otherwise, it will fallback to SBI call to maintain backward compatibility. Signed-off-by: Atish Patra <atishp@rivosinc.com> --- drivers/clocksource/timer-riscv.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)