Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200521134544.816918-4-anup.patel@wdc.com> <mhng-c95851de-d021-4272-aa99-6e9b492091ea@palmerdabbelt-glaptop1>
In-Reply-To: <mhng-c95851de-d021-4272-aa99-6e9b492091ea@palmerdabbelt-glaptop1>
From:   Anup Patel <anup@brainfault.org>
Date:   Sun, 7 Jun 2020 09:45:21 +0530
Message-ID: <CAAhSdy3Y08HEJBeib22GwyL0eFE7QeQ5Tbd3EX20M7yriErdsg@mail.gmail.com>
Subject: Re: [PATCH 3/5] clocksource/drivers/timer-riscv: Remove MMIO related stuff
To:     Palmer Dabbelt <palmer@dabbelt.com>
Cc:     Anup Patel <Anup.Patel@wdc.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Rob Herring <robh+dt@kernel.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Damien Le Moal <Damien.LeMoal@wdc.com>,
        Atish Patra <Atish.Patra@wdc.com>,
        Alistair Francis <Alistair.Francis@wdc.com>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>,
        devicetree@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 5, 2020 at 2:10 AM Palmer Dabbelt <palmer@dabbelt.com> 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@wdc.com>
> > ---
> >  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;
> >  }