Re: [PATCH 3/5] clocksource/drivers/timer-riscv: Remove MMIO related stuff

[Anup Patel]

On Fri, Jun 5, 2020 at 2:10 AM Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote:
>
> On Thu, 21 May 2020 06:45:42 PDT (-0700), Anup Patel wrote:
> > Right now the RISC-V timer is convoluted to support:
> > 1. Linux RISC-V S-mode (with MMU) where it will use TIME CSR
> >    for clocksource and SBI timer calls for clockevent device.
> > 2. Linux RISC-V M-mode (without MMU) where it will use CLINT
> >    MMIO counter register for clocksource and CLINT MMIO compare
> >    register for clockevent device.
> >
> > This patch removes MMIO related stuff from RISC-V timer driver
> > so that we can have a separate CLINT timer driver.
>
> This one will also break bisecting for the K210.

Same comments as PATCH2. This only affects the NoMMU kernel
which is still not 100 % complete due to on-going userspace work.

Regards,
Anup

>
> >
> > Signed-off-by: Anup Patel <anup.patel@xxxxxxx>
> > ---
> >  arch/riscv/Kconfig                |  2 +-
> >  arch/riscv/include/asm/timex.h    | 28 +++++++---------------------
> >  drivers/clocksource/Kconfig       |  2 +-
> >  drivers/clocksource/timer-riscv.c | 17 ++---------------
> >  4 files changed, 11 insertions(+), 38 deletions(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 2cf0c83c1a47..bbdc37a78f7b 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -52,7 +52,7 @@ config RISCV
> >       select PCI_DOMAINS_GENERIC if PCI
> >       select PCI_MSI if PCI
> >       select RISCV_INTC
> > -     select RISCV_TIMER
> > +     select RISCV_TIMER if RISCV_SBI
> >       select GENERIC_IRQ_MULTI_HANDLER
> >       select GENERIC_ARCH_TOPOLOGY if SMP
> >       select ARCH_HAS_PTE_SPECIAL
> > diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> > index bad2a7c2cda5..a3fb85d505d4 100644
> > --- a/arch/riscv/include/asm/timex.h
> > +++ b/arch/riscv/include/asm/timex.h
> > @@ -7,41 +7,27 @@
> >  #define _ASM_RISCV_TIMEX_H
> >
> >  #include <asm/csr.h>
> > -#include <asm/mmio.h>
> >
> >  typedef unsigned long cycles_t;
> >
> > -extern u64 __iomem *riscv_time_val;
> > -extern u64 __iomem *riscv_time_cmp;
> > -
> > -#ifdef CONFIG_64BIT
> > -#define mmio_get_cycles()    readq_relaxed(riscv_time_val)
> > -#else
> > -#define mmio_get_cycles()    readl_relaxed(riscv_time_val)
> > -#define mmio_get_cycles_hi() readl_relaxed(((u32 *)riscv_time_val) + 1)
> > -#endif
> > -
> >  static inline cycles_t get_cycles(void)
> >  {
> > -     if (IS_ENABLED(CONFIG_RISCV_SBI))
> > -             return csr_read(CSR_TIME);
> > -     return mmio_get_cycles();
> > +     return csr_read(CSR_TIME);
> >  }
> >  #define get_cycles get_cycles
> >
> > +static inline u32 get_cycles_hi(void)
> > +{
> > +     return csr_read(CSR_TIMEH);
> > +}
> > +#define get_cycles_hi get_cycles_hi
> > +
> >  #ifdef CONFIG_64BIT
> >  static inline u64 get_cycles64(void)
> >  {
> >       return get_cycles();
> >  }
> >  #else /* CONFIG_64BIT */
> > -static inline u32 get_cycles_hi(void)
> > -{
> > -     if (IS_ENABLED(CONFIG_RISCV_SBI))
> > -             return csr_read(CSR_TIMEH);
> > -     return mmio_get_cycles_hi();
> > -}
> > -
> >  static inline u64 get_cycles64(void)
> >  {
> >       u32 hi, lo;
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index f2142e6bbea3..21950d9e3e9d 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -650,7 +650,7 @@ config ATCPIT100_TIMER
> >
> >  config RISCV_TIMER
> >       bool "Timer for the RISC-V platform"
> > -     depends on GENERIC_SCHED_CLOCK && RISCV
> > +     depends on GENERIC_SCHED_CLOCK && RISCV_SBI
> >       default y
> >       select TIMER_PROBE
> >       select TIMER_OF
> > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > index 5fb7c5ba5c91..3e7e0cf5b899 100644
> > --- a/drivers/clocksource/timer-riscv.c
> > +++ b/drivers/clocksource/timer-riscv.c
> > @@ -19,26 +19,13 @@
> >  #include <linux/of_irq.h>
> >  #include <asm/smp.h>
> >  #include <asm/sbi.h>
> > -
> > -u64 __iomem *riscv_time_cmp;
> > -u64 __iomem *riscv_time_val;
> > -
> > -static inline void mmio_set_timer(u64 val)
> > -{
> > -     void __iomem *r;
> > -
> > -     r = riscv_time_cmp + cpuid_to_hartid_map(smp_processor_id());
> > -     writeq_relaxed(val, r);
> > -}
> > +#include <asm/timex.h>
> >
> >  static int riscv_clock_next_event(unsigned long delta,
> >               struct clock_event_device *ce)
> >  {
> >       csr_set(CSR_IE, IE_TIE);
> > -     if (IS_ENABLED(CONFIG_RISCV_SBI))
> > -             sbi_set_timer(get_cycles64() + delta);
> > -     else
> > -             mmio_set_timer(get_cycles64() + delta);
> > +     sbi_set_timer(get_cycles64() + delta);
> >       return 0;
> >  }