2020-07-24 07:19:24

by Anup Patel

[permalink] [raw]
Subject: [PATCH v6 0/4] Dedicated CLINT timer driver

The current RISC-V timer driver is convoluted and implements two
distinct timers:
1. S-mode timer: This is for Linux RISC-V S-mode with MMU. The
clocksource is implemented using TIME CSR and clockevent device
is implemented using SBI Timer calls.
2. M-mode timer: This is for Linux RISC-V M-mode without MMU. The
clocksource is implemented using CLINT MMIO time register and
clockevent device is implemented using CLINT MMIO timecmp registers.

This patchset removes clint related code from RISC-V timer driver and
arch/riscv directory. Instead, the series adds a dedicated MMIO based
CLINT driver under drivers/clocksource directory which can be used by
Linux RISC-V M-mode (i.e NoMMU Linux RISC-V).

The patchset is based up Linux-5.8-rc6 and can be found at riscv_clint_v6
branch of: https://github.com/avpatel/linux.git

This series is tested on:
1. QEMU RV64 virt machine using Linux RISC-V S-mode
2. QEMU RV32 virt machine using Linux RISC-V S-mode
3. QEMU RV64 virt machine using Linux RISC-V M-mode (i.e. NoMMU)

Changes since v5:
- Fixed order of compatible strings in PATCH4
- Added "additionalProperties: false" in PATCH4
- Fixed register space size for example DT node in PATCH4

Changes since v4:
- Rebased series on Linux-5.8-rc6
- Updated Kconfig option as suggested by Daniel in PATCH2
- Removed per-CPU registered flag in PATCH2
- Addressed nit comments from Atish in PATCH2

Changes since v3:
- Updated commit description of PATCH2
- Use clint_get_cycles64() in clint_rdtime() of PATCH2
- Call clockevents_config_and_register() only once for each CPU in
clint_timer_starting_cpu of PATCH2
- Select CLINT timer driver from platform Kconfig in PATCH3
- Fixed 'make dt_binding_check' for PATCH4

Changes since v2:
- Rebased series on Linux-5.8-rc5
- Squashed PATCH3 onto PATCH2 to preserve GIT bisectability
- Moved PATCH4 before PATCH2 to preserve GIT bisectability
- Replaced CLINT dt-bindings text document with YAML schema
- Use SiFive CLINT compatible string as per SiFive IP block versioning

Changes since v1:
- Rebased series on Linux-5.8-rc2
- Added pr_warn() for case where ipi_ops not available in PATCH1
- Updated ipi_inject() prototype to use "struct cpumask *" in PATCH1
- Updated CLINT_TIMER kconfig option to depend on RISCV_M_MODE in PATCH4
- Added riscv,clint0 compatible string in DT bindings document

Anup Patel (4):
RISC-V: Add mechanism to provide custom IPI operations
clocksource/drivers: Add CLINT timer driver
RISC-V: Remove CLINT related code from timer and arch
dt-bindings: timer: Add CLINT bindings

.../bindings/timer/sifive,clint.yaml | 60 +++++
arch/riscv/Kconfig | 2 +-
arch/riscv/Kconfig.socs | 2 +
arch/riscv/configs/nommu_virt_defconfig | 7 +-
arch/riscv/include/asm/clint.h | 39 ---
arch/riscv/include/asm/smp.h | 19 ++
arch/riscv/include/asm/timex.h | 28 +--
arch/riscv/kernel/Makefile | 2 +-
arch/riscv/kernel/clint.c | 44 ----
arch/riscv/kernel/sbi.c | 14 ++
arch/riscv/kernel/setup.c | 2 -
arch/riscv/kernel/smp.c | 44 ++--
arch/riscv/kernel/smpboot.c | 4 +-
drivers/clocksource/Kconfig | 12 +-
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++
drivers/clocksource/timer-riscv.c | 17 +-
include/linux/cpuhotplug.h | 1 +
18 files changed, 371 insertions(+), 153 deletions(-)
create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.yaml
delete mode 100644 arch/riscv/include/asm/clint.h
delete mode 100644 arch/riscv/kernel/clint.c
create mode 100644 drivers/clocksource/timer-clint.c

--
2.25.1


2020-07-24 07:19:37

by Anup Patel

[permalink] [raw]
Subject: [PATCH v6 1/4] RISC-V: Add mechanism to provide custom IPI operations

We add mechanism to set custom IPI operations so that CLINT driver
from drivers directory can provide custom IPI operations.

Signed-off-by: Anup Patel <[email protected]>
Tested-by: Emil Renner Berhing <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
---
arch/riscv/include/asm/clint.h | 25 --------------------
arch/riscv/include/asm/smp.h | 19 +++++++++++++++
arch/riscv/kernel/clint.c | 23 ++++++++++++++++--
arch/riscv/kernel/sbi.c | 14 +++++++++++
arch/riscv/kernel/smp.c | 43 +++++++++++++++++++---------------
arch/riscv/kernel/smpboot.c | 3 +--
6 files changed, 79 insertions(+), 48 deletions(-)

diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
index a279b17a6aad..adaba98a7d6c 100644
--- a/arch/riscv/include/asm/clint.h
+++ b/arch/riscv/include/asm/clint.h
@@ -6,34 +6,9 @@
#include <linux/smp.h>

#ifdef CONFIG_RISCV_M_MODE
-extern u32 __iomem *clint_ipi_base;
-
void clint_init_boot_cpu(void);
-
-static inline void clint_send_ipi_single(unsigned long hartid)
-{
- writel(1, clint_ipi_base + hartid);
-}
-
-static inline void clint_send_ipi_mask(const struct cpumask *mask)
-{
- int cpu;
-
- for_each_cpu(cpu, mask)
- clint_send_ipi_single(cpuid_to_hartid_map(cpu));
-}
-
-static inline void clint_clear_ipi(unsigned long hartid)
-{
- writel(0, clint_ipi_base + hartid);
-}
#else /* CONFIG_RISCV_M_MODE */
#define clint_init_boot_cpu() do { } while (0)
-
-/* stubs to for code is only reachable under IS_ENABLED(CONFIG_RISCV_M_MODE): */
-void clint_send_ipi_single(unsigned long hartid);
-void clint_send_ipi_mask(const struct cpumask *hartid_mask);
-void clint_clear_ipi(unsigned long hartid);
#endif /* CONFIG_RISCV_M_MODE */

#endif /* _ASM_RISCV_CLINT_H */
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 40bb1c15a731..68de78a8eba6 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -15,6 +15,11 @@
struct seq_file;
extern unsigned long boot_cpu_hartid;

+struct riscv_ipi_ops {
+ void (*ipi_inject)(const struct cpumask *target);
+ void (*ipi_clear)(void);
+};
+
#ifdef CONFIG_SMP
/*
* Mapping between linux logical cpu index and hartid.
@@ -40,6 +45,12 @@ void arch_send_call_function_single_ipi(int cpu);
int riscv_hartid_to_cpuid(int hartid);
void riscv_cpuid_to_hartid_mask(const struct cpumask *in, struct cpumask *out);

+/* Set custom IPI operations */
+void riscv_set_ipi_ops(struct riscv_ipi_ops *ops);
+
+/* Clear IPI for current CPU */
+void riscv_clear_ipi(void);
+
/*
* Obtains the hart ID of the currently executing task. This relies on
* THREAD_INFO_IN_TASK, but we define that unconditionally.
@@ -78,6 +89,14 @@ static inline void riscv_cpuid_to_hartid_mask(const struct cpumask *in,
cpumask_set_cpu(boot_cpu_hartid, out);
}

+static inline void riscv_set_ipi_ops(struct riscv_ipi_ops *ops)
+{
+}
+
+static inline void riscv_clear_ipi(void)
+{
+}
+
#endif /* CONFIG_SMP */

#if defined(CONFIG_HOTPLUG_CPU) && (CONFIG_SMP)
diff --git a/arch/riscv/kernel/clint.c b/arch/riscv/kernel/clint.c
index 3647980d14c3..a9845ee023e2 100644
--- a/arch/riscv/kernel/clint.c
+++ b/arch/riscv/kernel/clint.c
@@ -5,11 +5,11 @@

#include <linux/io.h>
#include <linux/of_address.h>
+#include <linux/smp.h>
#include <linux/types.h>
#include <asm/clint.h>
#include <asm/csr.h>
#include <asm/timex.h>
-#include <asm/smp.h>

/*
* This is the layout used by the SiFive clint, which is also shared by the qemu
@@ -21,6 +21,24 @@

u32 __iomem *clint_ipi_base;

+static void clint_send_ipi(const struct cpumask *target)
+{
+ unsigned int cpu;
+
+ for_each_cpu(cpu, target)
+ writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
+}
+
+static void clint_clear_ipi(void)
+{
+ writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
+}
+
+static struct riscv_ipi_ops clint_ipi_ops = {
+ .ipi_inject = clint_send_ipi,
+ .ipi_clear = clint_clear_ipi,
+};
+
void clint_init_boot_cpu(void)
{
struct device_node *np;
@@ -40,5 +58,6 @@ void clint_init_boot_cpu(void)
riscv_time_cmp = base + CLINT_TIME_CMP_OFF;
riscv_time_val = base + CLINT_TIME_VAL_OFF;

- clint_clear_ipi(boot_cpu_hartid);
+ clint_clear_ipi();
+ riscv_set_ipi_ops(&clint_ipi_ops);
}
diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
index f383ef5672b2..226ccce0f9e0 100644
--- a/arch/riscv/kernel/sbi.c
+++ b/arch/riscv/kernel/sbi.c
@@ -547,6 +547,18 @@ static inline long sbi_get_firmware_version(void)
return __sbi_base_ecall(SBI_EXT_BASE_GET_IMP_VERSION);
}

+static void sbi_send_cpumask_ipi(const struct cpumask *target)
+{
+ struct cpumask hartid_mask;
+
+ riscv_cpuid_to_hartid_mask(target, &hartid_mask);
+
+ sbi_send_ipi(cpumask_bits(&hartid_mask));
+}
+
+static struct riscv_ipi_ops sbi_ipi_ops = {
+ .ipi_inject = sbi_send_cpumask_ipi
+};

int __init sbi_init(void)
{
@@ -587,5 +599,7 @@ int __init sbi_init(void)
__sbi_rfence = __sbi_rfence_v01;
}

+ riscv_set_ipi_ops(&sbi_ipi_ops);
+
return 0;
}
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index b1d4f452f843..8b85683ce203 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -84,9 +84,25 @@ static void ipi_stop(void)
wait_for_interrupt();
}

+static struct riscv_ipi_ops *ipi_ops;
+
+void riscv_set_ipi_ops(struct riscv_ipi_ops *ops)
+{
+ ipi_ops = ops;
+}
+EXPORT_SYMBOL_GPL(riscv_set_ipi_ops);
+
+void riscv_clear_ipi(void)
+{
+ if (ipi_ops && ipi_ops->ipi_clear)
+ ipi_ops->ipi_clear();
+
+ csr_clear(CSR_IP, IE_SIE);
+}
+EXPORT_SYMBOL_GPL(riscv_clear_ipi);
+
static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
{
- struct cpumask hartid_mask;
int cpu;

smp_mb__before_atomic();
@@ -94,33 +110,22 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
set_bit(op, &ipi_data[cpu].bits);
smp_mb__after_atomic();

- riscv_cpuid_to_hartid_mask(mask, &hartid_mask);
- if (IS_ENABLED(CONFIG_RISCV_SBI))
- sbi_send_ipi(cpumask_bits(&hartid_mask));
+ if (ipi_ops && ipi_ops->ipi_inject)
+ ipi_ops->ipi_inject(mask);
else
- clint_send_ipi_mask(mask);
+ pr_warn("SMP: IPI inject method not available\n");
}

static void send_ipi_single(int cpu, enum ipi_message_type op)
{
- int hartid = cpuid_to_hartid_map(cpu);
-
smp_mb__before_atomic();
set_bit(op, &ipi_data[cpu].bits);
smp_mb__after_atomic();

- if (IS_ENABLED(CONFIG_RISCV_SBI))
- sbi_send_ipi(cpumask_bits(cpumask_of(hartid)));
- else
- clint_send_ipi_single(hartid);
-}
-
-static inline void clear_ipi(void)
-{
- if (IS_ENABLED(CONFIG_RISCV_SBI))
- csr_clear(CSR_IP, IE_SIE);
+ if (ipi_ops && ipi_ops->ipi_inject)
+ ipi_ops->ipi_inject(cpumask_of(cpu));
else
- clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
+ pr_warn("SMP: IPI inject method not available\n");
}

void handle_IPI(struct pt_regs *regs)
@@ -131,7 +136,7 @@ void handle_IPI(struct pt_regs *regs)

irq_enter();

- clear_ipi();
+ riscv_clear_ipi();

while (true) {
unsigned long ops;
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 4e9922790f6e..5fe849791bf0 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -147,8 +147,7 @@ asmlinkage __visible void smp_callin(void)
{
struct mm_struct *mm = &init_mm;

- if (!IS_ENABLED(CONFIG_RISCV_SBI))
- clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
+ riscv_clear_ipi();

/* All kernel threads share the same mm context. */
mmgrab(mm);
--
2.25.1

2020-07-24 07:19:52

by Anup Patel

[permalink] [raw]
Subject: [PATCH v6 2/4] clocksource/drivers: Add CLINT timer driver

We add a separate CLINT timer driver for Linux RISC-V M-mode (i.e.
RISC-V NoMMU kernel).

The CLINT MMIO device provides three things:
1. 64bit free running counter register
2. 64bit per-CPU time compare registers
3. 32bit per-CPU inter-processor interrupt registers

Unlike other timer devices, CLINT provides IPI registers along with
timer registers. To use CLINT IPI registers, the CLINT timer driver
provides IPI related callbacks to arch/riscv.

Signed-off-by: Anup Patel <[email protected]>
Tested-by: Emil Renner Berhing <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/Kconfig | 9 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
4 files changed, 237 insertions(+)
create mode 100644 drivers/clocksource/timer-clint.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 91418381fcd4..41f1c147c178 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -658,6 +658,15 @@ config RISCV_TIMER
is accessed via both the SBI and the rdcycle instruction. This is
required for all RISC-V systems.

+config CLINT_TIMER
+ bool "CLINT Timer for the RISC-V platform" if COMPILE_TEST
+ depends on GENERIC_SCHED_CLOCK && RISCV
+ select TIMER_PROBE
+ select TIMER_OF
+ help
+ This option enables the CLINT timer for RISC-V systems. The CLINT
+ driver is usually used for NoMMU RISC-V systems.
+
config CSKY_MP_TIMER
bool "SMP Timer for the C-SKY platform" if COMPILE_TEST
depends on CSKY
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index bdda1a2e4097..18e700e703a0 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -87,6 +87,7 @@ obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o
obj-$(CONFIG_X86_NUMACHIP) += numachip.o
obj-$(CONFIG_ATCPIT100_TIMER) += timer-atcpit100.o
obj-$(CONFIG_RISCV_TIMER) += timer-riscv.o
+obj-$(CONFIG_CLINT_TIMER) += timer-clint.o
obj-$(CONFIG_CSKY_MP_TIMER) += timer-mp-csky.o
obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o
obj-$(CONFIG_HYPERV_TIMER) += hyperv_timer.o
diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
new file mode 100644
index 000000000000..8eeafa82c03d
--- /dev/null
+++ b/drivers/clocksource/timer-clint.c
@@ -0,0 +1,226 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Western Digital Corporation or its affiliates.
+ *
+ * Most of the M-mode (i.e. NoMMU) RISC-V systems usually have a
+ * CLINT MMIO timer device.
+ */
+
+#define pr_fmt(fmt) "clint: " fmt
+#include <linux/bitops.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/cpu.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/sched_clock.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+#include <linux/smp.h>
+
+#define CLINT_IPI_OFF 0
+#define CLINT_TIMER_CMP_OFF 0x4000
+#define CLINT_TIMER_VAL_OFF 0xbff8
+
+/* CLINT manages IPI and Timer for RISC-V M-mode */
+static u32 __iomem *clint_ipi_base;
+static u64 __iomem *clint_timer_cmp;
+static u64 __iomem *clint_timer_val;
+static unsigned long clint_timer_freq;
+static unsigned int clint_timer_irq;
+
+static void clint_send_ipi(const struct cpumask *target)
+{
+ unsigned int cpu;
+
+ for_each_cpu(cpu, target)
+ writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
+}
+
+static void clint_clear_ipi(void)
+{
+ writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
+}
+
+static struct riscv_ipi_ops clint_ipi_ops = {
+ .ipi_inject = clint_send_ipi,
+ .ipi_clear = clint_clear_ipi,
+};
+
+#ifdef CONFIG_64BIT
+#define clint_get_cycles() readq_relaxed(clint_timer_val)
+#else
+#define clint_get_cycles() readl_relaxed(clint_timer_val)
+#define clint_get_cycles_hi() readl_relaxed(((u32 *)clint_timer_val) + 1)
+#endif
+
+#ifdef CONFIG_64BIT
+static u64 notrace clint_get_cycles64(void)
+{
+ return clint_get_cycles();
+}
+#else /* CONFIG_64BIT */
+static u64 notrace clint_get_cycles64(void)
+{
+ u32 hi, lo;
+
+ do {
+ hi = clint_get_cycles_hi();
+ lo = clint_get_cycles();
+ } while (hi != clint_get_cycles_hi());
+
+ return ((u64)hi << 32) | lo;
+}
+#endif /* CONFIG_64BIT */
+
+static u64 clint_rdtime(struct clocksource *cs)
+{
+ return clint_get_cycles64();
+}
+
+static struct clocksource clint_clocksource = {
+ .name = "clint_clocksource",
+ .rating = 300,
+ .mask = CLOCKSOURCE_MASK(64),
+ .flags = CLOCK_SOURCE_IS_CONTINUOUS,
+ .read = clint_rdtime,
+};
+
+static int clint_clock_next_event(unsigned long delta,
+ struct clock_event_device *ce)
+{
+ void __iomem *r = clint_timer_cmp +
+ cpuid_to_hartid_map(smp_processor_id());
+
+ csr_set(CSR_IE, IE_TIE);
+ writeq_relaxed(clint_get_cycles64() + delta, r);
+ return 0;
+}
+
+static DEFINE_PER_CPU(struct clock_event_device, clint_clock_event) = {
+ .name = "clint_clockevent",
+ .features = CLOCK_EVT_FEAT_ONESHOT,
+ .rating = 100,
+ .set_next_event = clint_clock_next_event,
+};
+
+static int clint_timer_starting_cpu(unsigned int cpu)
+{
+ struct clock_event_device *ce = per_cpu_ptr(&clint_clock_event, cpu);
+
+ ce->cpumask = cpumask_of(cpu);
+ clockevents_config_and_register(ce, clint_timer_freq, 100, 0x7fffffff);
+
+ enable_percpu_irq(clint_timer_irq,
+ irq_get_trigger_type(clint_timer_irq));
+ return 0;
+}
+
+static int clint_timer_dying_cpu(unsigned int cpu)
+{
+ disable_percpu_irq(clint_timer_irq);
+ return 0;
+}
+
+static irqreturn_t clint_timer_interrupt(int irq, void *dev_id)
+{
+ struct clock_event_device *evdev = this_cpu_ptr(&clint_clock_event);
+
+ csr_clear(CSR_IE, IE_TIE);
+ evdev->event_handler(evdev);
+
+ return IRQ_HANDLED;
+}
+
+static int __init clint_timer_init_dt(struct device_node *np)
+{
+ int rc;
+ u32 i, nr_irqs;
+ void __iomem *base;
+ struct of_phandle_args oirq;
+
+ /*
+ * Ensure that CLINT device interrupts are either RV_IRQ_TIMER or
+ * RV_IRQ_SOFT. If it's anything else then we ignore the device.
+ */
+ nr_irqs = of_irq_count(np);
+ for (i = 0; i < nr_irqs; i++) {
+ if (of_irq_parse_one(np, i, &oirq)) {
+ pr_err("%pOFP: failed to parse irq %d.\n", np, i);
+ continue;
+ }
+
+ if ((oirq.args_count != 1) ||
+ (oirq.args[0] != RV_IRQ_TIMER &&
+ oirq.args[0] != RV_IRQ_SOFT)) {
+ pr_err("%pOFP: invalid irq %d (hwirq %d)\n",
+ np, i, oirq.args[0]);
+ return -ENODEV;
+ }
+
+ /* Find parent irq domain and map timer irq */
+ if (!clint_timer_irq &&
+ oirq.args[0] == RV_IRQ_TIMER &&
+ irq_find_host(oirq.np))
+ clint_timer_irq = irq_of_parse_and_map(np, i);
+ }
+
+ /* If CLINT timer irq not found then fail */
+ if (!clint_timer_irq) {
+ pr_err("%pOFP: timer irq not found\n", np);
+ return -ENODEV;
+ }
+
+ base = of_iomap(np, 0);
+ if (!base) {
+ pr_err("%pOFP: could not map registers\n", np);
+ return -ENODEV;
+ }
+
+ clint_ipi_base = base + CLINT_IPI_OFF;
+ clint_timer_cmp = base + CLINT_TIMER_CMP_OFF;
+ clint_timer_val = base + CLINT_TIMER_VAL_OFF;
+ clint_timer_freq = riscv_timebase;
+
+ pr_info("%pOFP: timer running at %ld Hz\n", np, clint_timer_freq);
+
+ rc = clocksource_register_hz(&clint_clocksource, clint_timer_freq);
+ if (rc) {
+ pr_err("%pOFP: clocksource register failed [%d]\n", np, rc);
+ goto fail_iounmap;
+ }
+
+ sched_clock_register(clint_get_cycles64, 64, clint_timer_freq);
+
+ rc = request_percpu_irq(clint_timer_irq, clint_timer_interrupt,
+ "clint-timer", &clint_clock_event);
+ if (rc) {
+ pr_err("registering percpu irq failed [%d]\n", rc);
+ goto fail_iounmap;
+ }
+
+ rc = cpuhp_setup_state(CPUHP_AP_CLINT_TIMER_STARTING,
+ "clockevents/clint/timer:starting",
+ clint_timer_starting_cpu,
+ clint_timer_dying_cpu);
+ if (rc) {
+ pr_err("%pOFP: cpuhp setup state failed [%d]\n", np, rc);
+ goto fail_free_irq;
+ }
+
+ riscv_set_ipi_ops(&clint_ipi_ops);
+ clint_clear_ipi();
+
+ return 0;
+
+fail_free_irq:
+ free_irq(clint_timer_irq, &clint_clock_event);
+fail_iounmap:
+ iounmap(base);
+ return rc;
+}
+
+TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
+TIMER_OF_DECLARE(clint_timer1, "sifive,clint0", clint_timer_init_dt);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 191772d4a4d7..1451f4625833 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -132,6 +132,7 @@ enum cpuhp_state {
CPUHP_AP_MIPS_GIC_TIMER_STARTING,
CPUHP_AP_ARC_TIMER_STARTING,
CPUHP_AP_RISCV_TIMER_STARTING,
+ CPUHP_AP_CLINT_TIMER_STARTING,
CPUHP_AP_CSKY_TIMER_STARTING,
CPUHP_AP_HYPERV_TIMER_STARTING,
CPUHP_AP_KVM_STARTING,
--
2.25.1

2020-07-24 07:20:07

by Anup Patel

[permalink] [raw]
Subject: [PATCH v6 3/4] RISC-V: Remove CLINT related code from timer and arch

Right now the RISC-V timer driver 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.

We now have a separate CLINT timer driver which also provide CLINT
based IPI operations so let's remove CLINT MMIO related code from
arch/riscv directory and RISC-V timer driver.

Signed-off-by: Anup Patel <[email protected]>
Tested-by: Emil Renner Berhing <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
arch/riscv/Kconfig | 2 +-
arch/riscv/Kconfig.socs | 2 +
arch/riscv/configs/nommu_virt_defconfig | 7 +--
arch/riscv/include/asm/clint.h | 14 ------
arch/riscv/include/asm/timex.h | 28 +++--------
arch/riscv/kernel/Makefile | 2 +-
arch/riscv/kernel/clint.c | 63 -------------------------
arch/riscv/kernel/setup.c | 2 -
arch/riscv/kernel/smp.c | 1 -
arch/riscv/kernel/smpboot.c | 1 -
drivers/clocksource/Kconfig | 3 +-
drivers/clocksource/timer-riscv.c | 17 +------
12 files changed, 16 insertions(+), 126 deletions(-)
delete mode 100644 arch/riscv/include/asm/clint.h
delete mode 100644 arch/riscv/kernel/clint.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index fedb4a72b29a..57a72ae23d10 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -74,7 +74,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 SPARSEMEM_STATIC if 32BIT
select SPARSE_IRQ
select SYSCTL_EXCEPTION_TRACE
diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index 6c88148f1b9b..8a55f6156661 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -12,6 +12,7 @@ config SOC_SIFIVE

config SOC_VIRT
bool "QEMU Virt Machine"
+ select CLINT_TIMER if RISCV_M_MODE
select POWER_RESET
select POWER_RESET_SYSCON
select POWER_RESET_SYSCON_POWEROFF
@@ -24,6 +25,7 @@ config SOC_VIRT
config SOC_KENDRYTE
bool "Kendryte K210 SoC"
depends on !MMU
+ select CLINT_TIMER if RISCV_M_MODE
select SERIAL_SIFIVE if TTY
select SERIAL_SIFIVE_CONSOLE if TTY
select SIFIVE_PLIC
diff --git a/arch/riscv/configs/nommu_virt_defconfig b/arch/riscv/configs/nommu_virt_defconfig
index cf74e179bf90..cf9388184aa3 100644
--- a/arch/riscv/configs/nommu_virt_defconfig
+++ b/arch/riscv/configs/nommu_virt_defconfig
@@ -26,6 +26,7 @@ CONFIG_EXPERT=y
CONFIG_SLOB=y
# CONFIG_SLAB_MERGE_DEFAULT is not set
# CONFIG_MMU is not set
+CONFIG_SOC_VIRT=y
CONFIG_MAXPHYSMEM_2GB=y
CONFIG_SMP=y
CONFIG_CMDLINE="root=/dev/vda rw earlycon=uart8250,mmio,0x10000000,115200n8 console=ttyS0"
@@ -48,7 +49,6 @@ CONFIG_VIRTIO_BLK=y
# CONFIG_SERIO is not set
# CONFIG_LEGACY_PTYS is not set
# CONFIG_LDISC_AUTOLOAD is not set
-# CONFIG_DEVMEM is not set
CONFIG_SERIAL_8250=y
# CONFIG_SERIAL_8250_DEPRECATED_OPTIONS is not set
CONFIG_SERIAL_8250_CONSOLE=y
@@ -56,16 +56,13 @@ CONFIG_SERIAL_8250_NR_UARTS=1
CONFIG_SERIAL_8250_RUNTIME_UARTS=1
CONFIG_SERIAL_OF_PLATFORM=y
# CONFIG_HW_RANDOM is not set
+# CONFIG_DEVMEM is not set
# CONFIG_HWMON is not set
-# CONFIG_LCD_CLASS_DEVICE is not set
-# CONFIG_BACKLIGHT_CLASS_DEVICE is not set
# CONFIG_VGA_CONSOLE is not set
# CONFIG_HID is not set
# CONFIG_USB_SUPPORT is not set
CONFIG_VIRTIO_MMIO=y
CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
-CONFIG_SIFIVE_PLIC=y
-# CONFIG_VALIDATE_FS_PARSER is not set
CONFIG_EXT2_FS=y
# CONFIG_DNOTIFY is not set
# CONFIG_INOTIFY_USER is not set
diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
deleted file mode 100644
index adaba98a7d6c..000000000000
--- a/arch/riscv/include/asm/clint.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_RISCV_CLINT_H
-#define _ASM_RISCV_CLINT_H 1
-
-#include <linux/io.h>
-#include <linux/smp.h>
-
-#ifdef CONFIG_RISCV_M_MODE
-void clint_init_boot_cpu(void);
-#else /* CONFIG_RISCV_M_MODE */
-#define clint_init_boot_cpu() do { } while (0)
-#endif /* CONFIG_RISCV_M_MODE */
-
-#endif /* _ASM_RISCV_CLINT_H */
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/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index b355cf485671..7edf15643146 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -31,7 +31,7 @@ obj-y += cacheinfo.o
obj-y += patch.o
obj-$(CONFIG_MMU) += vdso.o vdso/

-obj-$(CONFIG_RISCV_M_MODE) += clint.o traps_misaligned.o
+obj-$(CONFIG_RISCV_M_MODE) += traps_misaligned.o
obj-$(CONFIG_FPU) += fpu.o
obj-$(CONFIG_SMP) += smpboot.o
obj-$(CONFIG_SMP) += smp.o
diff --git a/arch/riscv/kernel/clint.c b/arch/riscv/kernel/clint.c
deleted file mode 100644
index a9845ee023e2..000000000000
--- a/arch/riscv/kernel/clint.c
+++ /dev/null
@@ -1,63 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (c) 2019 Christoph Hellwig.
- */
-
-#include <linux/io.h>
-#include <linux/of_address.h>
-#include <linux/smp.h>
-#include <linux/types.h>
-#include <asm/clint.h>
-#include <asm/csr.h>
-#include <asm/timex.h>
-
-/*
- * This is the layout used by the SiFive clint, which is also shared by the qemu
- * virt platform, and the Kendryte KD210 at least.
- */
-#define CLINT_IPI_OFF 0
-#define CLINT_TIME_CMP_OFF 0x4000
-#define CLINT_TIME_VAL_OFF 0xbff8
-
-u32 __iomem *clint_ipi_base;
-
-static void clint_send_ipi(const struct cpumask *target)
-{
- unsigned int cpu;
-
- for_each_cpu(cpu, target)
- writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
-}
-
-static void clint_clear_ipi(void)
-{
- writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
-}
-
-static struct riscv_ipi_ops clint_ipi_ops = {
- .ipi_inject = clint_send_ipi,
- .ipi_clear = clint_clear_ipi,
-};
-
-void clint_init_boot_cpu(void)
-{
- struct device_node *np;
- void __iomem *base;
-
- np = of_find_compatible_node(NULL, NULL, "riscv,clint0");
- if (!np) {
- panic("clint not found");
- return;
- }
-
- base = of_iomap(np, 0);
- if (!base)
- panic("could not map CLINT");
-
- clint_ipi_base = base + CLINT_IPI_OFF;
- riscv_time_cmp = base + CLINT_TIME_CMP_OFF;
- riscv_time_val = base + CLINT_TIME_VAL_OFF;
-
- clint_clear_ipi();
- riscv_set_ipi_ops(&clint_ipi_ops);
-}
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index f04373be54a6..2c6dd329312b 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -18,7 +18,6 @@
#include <linux/swiotlb.h>
#include <linux/smp.h>

-#include <asm/clint.h>
#include <asm/cpu_ops.h>
#include <asm/setup.h>
#include <asm/sections.h>
@@ -79,7 +78,6 @@ void __init setup_arch(char **cmdline_p)
#else
unflatten_device_tree();
#endif
- clint_init_boot_cpu();

#ifdef CONFIG_SWIOTLB
swiotlb_init(1);
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index 8b85683ce203..07626be78c23 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -17,7 +17,6 @@
#include <linux/seq_file.h>
#include <linux/delay.h>

-#include <asm/clint.h>
#include <asm/sbi.h>
#include <asm/tlbflush.h>
#include <asm/cacheflush.h>
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 5fe849791bf0..a6cfa9842d4b 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -24,7 +24,6 @@
#include <linux/of.h>
#include <linux/sched/task_stack.h>
#include <linux/sched/mm.h>
-#include <asm/clint.h>
#include <asm/cpu_ops.h>
#include <asm/irq.h>
#include <asm/mmu_context.h>
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 41f1c147c178..b24449da3022 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -648,9 +648,8 @@ config ATCPIT100_TIMER
This option enables support for the Andestech ATCPIT100 timers.

config RISCV_TIMER
- bool "Timer for the RISC-V platform"
+ bool "Timer for the RISC-V platform" if COMPILE_TEST
depends on GENERIC_SCHED_CLOCK && RISCV
- default y
select TIMER_PROBE
select TIMER_OF
help
diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
index 9de1dabfb126..c51c5ed15aa7 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;
}

--
2.25.1

2020-07-24 07:20:46

by Anup Patel

[permalink] [raw]
Subject: [PATCH v6 4/4] dt-bindings: timer: Add CLINT bindings

We add DT bindings documentation for CLINT device.

Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Palmer Dabbelt <[email protected]>
Tested-by: Emil Renner Berhing <[email protected]>
---
.../bindings/timer/sifive,clint.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.yaml

diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
new file mode 100644
index 000000000000..2a0e9cd9fbcf
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/sifive,clint.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SiFive Core Local Interruptor
+
+maintainers:
+ - Palmer Dabbelt <[email protected]>
+ - Anup Patel <[email protected]>
+
+description:
+ SiFive (and other RISC-V) SOCs include an implementation of the SiFive
+ Core Local Interruptor (CLINT) for M-mode timer and M-mode inter-processor
+ interrupts. It directly connects to the timer and inter-processor interrupt
+ lines of various HARTs (or CPUs) so RISC-V per-HART (or per-CPU) local
+ interrupt controller is the parent interrupt controller for CLINT device.
+ The clock frequency of CLINT is specified via "timebase-frequency" DT
+ property of "/cpus" DT node. The "timebase-frequency" DT property is
+ described in Documentation/devicetree/bindings/riscv/cpus.yaml
+
+properties:
+ compatible:
+ items:
+ - const: sifive,fu540-c000-clint
+ - const: sifive,clint0
+
+ description:
+ Should be "sifive,<chip>-clint" and "sifive,clint<version>".
+ Supported compatible strings are -
+ "sifive,fu540-c000-clint" for the SiFive CLINT v0 as integrated
+ onto the SiFive FU540 chip, and "sifive,clint0" for the SiFive
+ CLINT v0 IP block with no chip integration tweaks.
+ Please refer to sifive-blocks-ip-versioning.txt for details
+
+ reg:
+ maxItems: 1
+
+ interrupts-extended:
+ minItems: 1
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts-extended
+
+examples:
+ - |
+ timer@2000000 {
+ compatible = "sifive,fu540-c000-clint", "sifive,clint0";
+ interrupts-extended = <&cpu1intc 3 &cpu1intc 7
+ &cpu2intc 3 &cpu2intc 7
+ &cpu3intc 3 &cpu3intc 7
+ &cpu4intc 3 &cpu4intc 7>;
+ reg = <0x2000000 0x10000>;
+ };
+...
--
2.25.1

2020-07-25 05:04:21

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] clocksource/drivers: Add CLINT timer driver

On Fri, Jul 24, 2020 at 12:19 AM Anup Patel <[email protected]> wrote:
>
> We add a separate CLINT timer driver for Linux RISC-V M-mode (i.e.
> RISC-V NoMMU kernel).
>
> The CLINT MMIO device provides three things:
> 1. 64bit free running counter register
> 2. 64bit per-CPU time compare registers
> 3. 32bit per-CPU inter-processor interrupt registers
>
> Unlike other timer devices, CLINT provides IPI registers along with
> timer registers. To use CLINT IPI registers, the CLINT timer driver
> provides IPI related callbacks to arch/riscv.
>
> Signed-off-by: Anup Patel <[email protected]>
> Tested-by: Emil Renner Berhing <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---
> drivers/clocksource/Kconfig | 9 ++
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++++++++++++++
> include/linux/cpuhotplug.h | 1 +
> 4 files changed, 237 insertions(+)
> create mode 100644 drivers/clocksource/timer-clint.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 91418381fcd4..41f1c147c178 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -658,6 +658,15 @@ config RISCV_TIMER
> is accessed via both the SBI and the rdcycle instruction. This is
> required for all RISC-V systems.
>
> +config CLINT_TIMER
> + bool "CLINT Timer for the RISC-V platform" if COMPILE_TEST
> + depends on GENERIC_SCHED_CLOCK && RISCV
> + select TIMER_PROBE
> + select TIMER_OF
> + help
> + This option enables the CLINT timer for RISC-V systems. The CLINT
> + driver is usually used for NoMMU RISC-V systems.
> +
> config CSKY_MP_TIMER
> bool "SMP Timer for the C-SKY platform" if COMPILE_TEST
> depends on CSKY
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index bdda1a2e4097..18e700e703a0 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -87,6 +87,7 @@ obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o
> obj-$(CONFIG_X86_NUMACHIP) += numachip.o
> obj-$(CONFIG_ATCPIT100_TIMER) += timer-atcpit100.o
> obj-$(CONFIG_RISCV_TIMER) += timer-riscv.o
> +obj-$(CONFIG_CLINT_TIMER) += timer-clint.o
> obj-$(CONFIG_CSKY_MP_TIMER) += timer-mp-csky.o
> obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o
> obj-$(CONFIG_HYPERV_TIMER) += hyperv_timer.o
> diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
> new file mode 100644
> index 000000000000..8eeafa82c03d
> --- /dev/null
> +++ b/drivers/clocksource/timer-clint.c
> @@ -0,0 +1,226 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> + *
> + * Most of the M-mode (i.e. NoMMU) RISC-V systems usually have a
> + * CLINT MMIO timer device.
> + */
> +
> +#define pr_fmt(fmt) "clint: " fmt
> +#include <linux/bitops.h>
> +#include <linux/clocksource.h>
> +#include <linux/clockchips.h>
> +#include <linux/cpu.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/sched_clock.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_irq.h>
> +#include <linux/smp.h>
> +
> +#define CLINT_IPI_OFF 0
> +#define CLINT_TIMER_CMP_OFF 0x4000
> +#define CLINT_TIMER_VAL_OFF 0xbff8
> +
> +/* CLINT manages IPI and Timer for RISC-V M-mode */
> +static u32 __iomem *clint_ipi_base;
> +static u64 __iomem *clint_timer_cmp;
> +static u64 __iomem *clint_timer_val;
> +static unsigned long clint_timer_freq;
> +static unsigned int clint_timer_irq;
> +
> +static void clint_send_ipi(const struct cpumask *target)
> +{
> + unsigned int cpu;
> +
> + for_each_cpu(cpu, target)
> + writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
> +}
> +
> +static void clint_clear_ipi(void)
> +{
> + writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
> +}
> +
> +static struct riscv_ipi_ops clint_ipi_ops = {
> + .ipi_inject = clint_send_ipi,
> + .ipi_clear = clint_clear_ipi,
> +};
> +
> +#ifdef CONFIG_64BIT
> +#define clint_get_cycles() readq_relaxed(clint_timer_val)
> +#else
> +#define clint_get_cycles() readl_relaxed(clint_timer_val)
> +#define clint_get_cycles_hi() readl_relaxed(((u32 *)clint_timer_val) + 1)
> +#endif
> +
> +#ifdef CONFIG_64BIT
> +static u64 notrace clint_get_cycles64(void)
> +{
> + return clint_get_cycles();
> +}
> +#else /* CONFIG_64BIT */
> +static u64 notrace clint_get_cycles64(void)
> +{
> + u32 hi, lo;
> +
> + do {
> + hi = clint_get_cycles_hi();
> + lo = clint_get_cycles();
> + } while (hi != clint_get_cycles_hi());
> +
> + return ((u64)hi << 32) | lo;
> +}
> +#endif /* CONFIG_64BIT */
> +
> +static u64 clint_rdtime(struct clocksource *cs)
> +{
> + return clint_get_cycles64();
> +}
> +
> +static struct clocksource clint_clocksource = {
> + .name = "clint_clocksource",
> + .rating = 300,
> + .mask = CLOCKSOURCE_MASK(64),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> + .read = clint_rdtime,
> +};
> +
> +static int clint_clock_next_event(unsigned long delta,
> + struct clock_event_device *ce)
> +{
> + void __iomem *r = clint_timer_cmp +
> + cpuid_to_hartid_map(smp_processor_id());
> +
> + csr_set(CSR_IE, IE_TIE);
> + writeq_relaxed(clint_get_cycles64() + delta, r);
> + return 0;
> +}
> +
> +static DEFINE_PER_CPU(struct clock_event_device, clint_clock_event) = {
> + .name = "clint_clockevent",
> + .features = CLOCK_EVT_FEAT_ONESHOT,
> + .rating = 100,
> + .set_next_event = clint_clock_next_event,
> +};
> +
> +static int clint_timer_starting_cpu(unsigned int cpu)
> +{
> + struct clock_event_device *ce = per_cpu_ptr(&clint_clock_event, cpu);
> +
> + ce->cpumask = cpumask_of(cpu);
> + clockevents_config_and_register(ce, clint_timer_freq, 100, 0x7fffffff);
> +
> + enable_percpu_irq(clint_timer_irq,
> + irq_get_trigger_type(clint_timer_irq));
> + return 0;
> +}
> +
> +static int clint_timer_dying_cpu(unsigned int cpu)
> +{
> + disable_percpu_irq(clint_timer_irq);
> + return 0;
> +}
> +
> +static irqreturn_t clint_timer_interrupt(int irq, void *dev_id)
> +{
> + struct clock_event_device *evdev = this_cpu_ptr(&clint_clock_event);
> +
> + csr_clear(CSR_IE, IE_TIE);
> + evdev->event_handler(evdev);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int __init clint_timer_init_dt(struct device_node *np)
> +{
> + int rc;
> + u32 i, nr_irqs;
> + void __iomem *base;
> + struct of_phandle_args oirq;
> +
> + /*
> + * Ensure that CLINT device interrupts are either RV_IRQ_TIMER or
> + * RV_IRQ_SOFT. If it's anything else then we ignore the device.
> + */
> + nr_irqs = of_irq_count(np);
> + for (i = 0; i < nr_irqs; i++) {
> + if (of_irq_parse_one(np, i, &oirq)) {
> + pr_err("%pOFP: failed to parse irq %d.\n", np, i);
> + continue;
> + }
> +
> + if ((oirq.args_count != 1) ||
> + (oirq.args[0] != RV_IRQ_TIMER &&
> + oirq.args[0] != RV_IRQ_SOFT)) {
> + pr_err("%pOFP: invalid irq %d (hwirq %d)\n",
> + np, i, oirq.args[0]);
> + return -ENODEV;
> + }
> +
> + /* Find parent irq domain and map timer irq */
> + if (!clint_timer_irq &&
> + oirq.args[0] == RV_IRQ_TIMER &&
> + irq_find_host(oirq.np))
> + clint_timer_irq = irq_of_parse_and_map(np, i);
> + }
> +
> + /* If CLINT timer irq not found then fail */
> + if (!clint_timer_irq) {
> + pr_err("%pOFP: timer irq not found\n", np);
> + return -ENODEV;
> + }
> +
> + base = of_iomap(np, 0);
> + if (!base) {
> + pr_err("%pOFP: could not map registers\n", np);
> + return -ENODEV;
> + }
> +
> + clint_ipi_base = base + CLINT_IPI_OFF;
> + clint_timer_cmp = base + CLINT_TIMER_CMP_OFF;
> + clint_timer_val = base + CLINT_TIMER_VAL_OFF;
> + clint_timer_freq = riscv_timebase;
> +
> + pr_info("%pOFP: timer running at %ld Hz\n", np, clint_timer_freq);
> +
> + rc = clocksource_register_hz(&clint_clocksource, clint_timer_freq);
> + if (rc) {
> + pr_err("%pOFP: clocksource register failed [%d]\n", np, rc);
> + goto fail_iounmap;
> + }
> +
> + sched_clock_register(clint_get_cycles64, 64, clint_timer_freq);
> +
> + rc = request_percpu_irq(clint_timer_irq, clint_timer_interrupt,
> + "clint-timer", &clint_clock_event);
> + if (rc) {
> + pr_err("registering percpu irq failed [%d]\n", rc);
> + goto fail_iounmap;
> + }
> +
> + rc = cpuhp_setup_state(CPUHP_AP_CLINT_TIMER_STARTING,
> + "clockevents/clint/timer:starting",
> + clint_timer_starting_cpu,
> + clint_timer_dying_cpu);
> + if (rc) {
> + pr_err("%pOFP: cpuhp setup state failed [%d]\n", np, rc);
> + goto fail_free_irq;
> + }
> +
> + riscv_set_ipi_ops(&clint_ipi_ops);
> + clint_clear_ipi();
> +
> + return 0;
> +
> +fail_free_irq:
> + free_irq(clint_timer_irq, &clint_clock_event);
> +fail_iounmap:
> + iounmap(base);
> + return rc;
> +}
> +
> +TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
> +TIMER_OF_DECLARE(clint_timer1, "sifive,clint0", clint_timer_init_dt);
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 191772d4a4d7..1451f4625833 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -132,6 +132,7 @@ enum cpuhp_state {
> CPUHP_AP_MIPS_GIC_TIMER_STARTING,
> CPUHP_AP_ARC_TIMER_STARTING,
> CPUHP_AP_RISCV_TIMER_STARTING,
> + CPUHP_AP_CLINT_TIMER_STARTING,
> CPUHP_AP_CSKY_TIMER_STARTING,
> CPUHP_AP_HYPERV_TIMER_STARTING,
> CPUHP_AP_KVM_STARTING,
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv


LGTM.

Reviewed-by: Atish Patra <[email protected]>
--
Regards,
Atish

2020-07-25 05:05:33

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v6 4/4] dt-bindings: timer: Add CLINT bindings

On Fri, Jul 24, 2020 at 12:19 AM Anup Patel <[email protected]> wrote:
>
> We add DT bindings documentation for CLINT device.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Palmer Dabbelt <[email protected]>
> Tested-by: Emil Renner Berhing <[email protected]>
> ---
> .../bindings/timer/sifive,clint.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.yaml
>
> diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
> new file mode 100644
> index 000000000000..2a0e9cd9fbcf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/sifive,clint.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SiFive Core Local Interruptor
> +
> +maintainers:
> + - Palmer Dabbelt <[email protected]>
> + - Anup Patel <[email protected]>
> +
> +description:
> + SiFive (and other RISC-V) SOCs include an implementation of the SiFive
> + Core Local Interruptor (CLINT) for M-mode timer and M-mode inter-processor
> + interrupts. It directly connects to the timer and inter-processor interrupt
> + lines of various HARTs (or CPUs) so RISC-V per-HART (or per-CPU) local
> + interrupt controller is the parent interrupt controller for CLINT device.
> + The clock frequency of CLINT is specified via "timebase-frequency" DT
> + property of "/cpus" DT node. The "timebase-frequency" DT property is
> + described in Documentation/devicetree/bindings/riscv/cpus.yaml
> +
> +properties:
> + compatible:
> + items:
> + - const: sifive,fu540-c000-clint
> + - const: sifive,clint0
> +
> + description:
> + Should be "sifive,<chip>-clint" and "sifive,clint<version>".
> + Supported compatible strings are -
> + "sifive,fu540-c000-clint" for the SiFive CLINT v0 as integrated
> + onto the SiFive FU540 chip, and "sifive,clint0" for the SiFive
> + CLINT v0 IP block with no chip integration tweaks.
> + Please refer to sifive-blocks-ip-versioning.txt for details
> +
> + reg:
> + maxItems: 1
> +
> + interrupts-extended:
> + minItems: 1
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts-extended
> +
> +examples:
> + - |
> + timer@2000000 {
> + compatible = "sifive,fu540-c000-clint", "sifive,clint0";
> + interrupts-extended = <&cpu1intc 3 &cpu1intc 7
> + &cpu2intc 3 &cpu2intc 7
> + &cpu3intc 3 &cpu3intc 7
> + &cpu4intc 3 &cpu4intc 7>;
> + reg = <0x2000000 0x10000>;
> + };
> +...
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv


Reviewed-by: Atish Patra <[email protected]>

--
Regards,
Atish

2020-07-25 05:18:38

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH v6 3/4] RISC-V: Remove CLINT related code from timer and arch

On Fri, Jul 24, 2020 at 12:19 AM Anup Patel <[email protected]> wrote:
>
> Right now the RISC-V timer driver 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.
>
> We now have a separate CLINT timer driver which also provide CLINT
> based IPI operations so let's remove CLINT MMIO related code from
> arch/riscv directory and RISC-V timer driver.
>
> Signed-off-by: Anup Patel <[email protected]>
> Tested-by: Emil Renner Berhing <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---
> arch/riscv/Kconfig | 2 +-
> arch/riscv/Kconfig.socs | 2 +
> arch/riscv/configs/nommu_virt_defconfig | 7 +--
> arch/riscv/include/asm/clint.h | 14 ------
> arch/riscv/include/asm/timex.h | 28 +++--------
> arch/riscv/kernel/Makefile | 2 +-
> arch/riscv/kernel/clint.c | 63 -------------------------
> arch/riscv/kernel/setup.c | 2 -
> arch/riscv/kernel/smp.c | 1 -
> arch/riscv/kernel/smpboot.c | 1 -
> drivers/clocksource/Kconfig | 3 +-
> drivers/clocksource/timer-riscv.c | 17 +------
> 12 files changed, 16 insertions(+), 126 deletions(-)
> delete mode 100644 arch/riscv/include/asm/clint.h
> delete mode 100644 arch/riscv/kernel/clint.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index fedb4a72b29a..57a72ae23d10 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -74,7 +74,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 SPARSEMEM_STATIC if 32BIT
> select SPARSE_IRQ
> select SYSCTL_EXCEPTION_TRACE
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 6c88148f1b9b..8a55f6156661 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -12,6 +12,7 @@ config SOC_SIFIVE
>
> config SOC_VIRT
> bool "QEMU Virt Machine"
> + select CLINT_TIMER if RISCV_M_MODE
> select POWER_RESET
> select POWER_RESET_SYSCON
> select POWER_RESET_SYSCON_POWEROFF
> @@ -24,6 +25,7 @@ config SOC_VIRT
> config SOC_KENDRYTE
> bool "Kendryte K210 SoC"
> depends on !MMU
> + select CLINT_TIMER if RISCV_M_MODE
> select SERIAL_SIFIVE if TTY
> select SERIAL_SIFIVE_CONSOLE if TTY
> select SIFIVE_PLIC
> diff --git a/arch/riscv/configs/nommu_virt_defconfig b/arch/riscv/configs/nommu_virt_defconfig
> index cf74e179bf90..cf9388184aa3 100644
> --- a/arch/riscv/configs/nommu_virt_defconfig
> +++ b/arch/riscv/configs/nommu_virt_defconfig
> @@ -26,6 +26,7 @@ CONFIG_EXPERT=y
> CONFIG_SLOB=y
> # CONFIG_SLAB_MERGE_DEFAULT is not set
> # CONFIG_MMU is not set
> +CONFIG_SOC_VIRT=y
> CONFIG_MAXPHYSMEM_2GB=y
> CONFIG_SMP=y
> CONFIG_CMDLINE="root=/dev/vda rw earlycon=uart8250,mmio,0x10000000,115200n8 console=ttyS0"
> @@ -48,7 +49,6 @@ CONFIG_VIRTIO_BLK=y
> # CONFIG_SERIO is not set
> # CONFIG_LEGACY_PTYS is not set
> # CONFIG_LDISC_AUTOLOAD is not set
> -# CONFIG_DEVMEM is not set
> CONFIG_SERIAL_8250=y
> # CONFIG_SERIAL_8250_DEPRECATED_OPTIONS is not set
> CONFIG_SERIAL_8250_CONSOLE=y
> @@ -56,16 +56,13 @@ CONFIG_SERIAL_8250_NR_UARTS=1
> CONFIG_SERIAL_8250_RUNTIME_UARTS=1
> CONFIG_SERIAL_OF_PLATFORM=y
> # CONFIG_HW_RANDOM is not set
> +# CONFIG_DEVMEM is not set
> # CONFIG_HWMON is not set
> -# CONFIG_LCD_CLASS_DEVICE is not set
> -# CONFIG_BACKLIGHT_CLASS_DEVICE is not set

Why these changes are in the diff ?

> # CONFIG_VGA_CONSOLE is not set
> # CONFIG_HID is not set
> # CONFIG_USB_SUPPORT is not set
> CONFIG_VIRTIO_MMIO=y
> CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
> -CONFIG_SIFIVE_PLIC=y
> -# CONFIG_VALIDATE_FS_PARSER is not set
> CONFIG_EXT2_FS=y
> # CONFIG_DNOTIFY is not set
> # CONFIG_INOTIFY_USER is not set
> diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
> deleted file mode 100644
> index adaba98a7d6c..000000000000
> --- a/arch/riscv/include/asm/clint.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef _ASM_RISCV_CLINT_H
> -#define _ASM_RISCV_CLINT_H 1
> -
> -#include <linux/io.h>
> -#include <linux/smp.h>
> -
> -#ifdef CONFIG_RISCV_M_MODE
> -void clint_init_boot_cpu(void);
> -#else /* CONFIG_RISCV_M_MODE */
> -#define clint_init_boot_cpu() do { } while (0)
> -#endif /* CONFIG_RISCV_M_MODE */
> -
> -#endif /* _ASM_RISCV_CLINT_H */
> 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/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index b355cf485671..7edf15643146 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -31,7 +31,7 @@ obj-y += cacheinfo.o
> obj-y += patch.o
> obj-$(CONFIG_MMU) += vdso.o vdso/
>
> -obj-$(CONFIG_RISCV_M_MODE) += clint.o traps_misaligned.o
> +obj-$(CONFIG_RISCV_M_MODE) += traps_misaligned.o
> obj-$(CONFIG_FPU) += fpu.o
> obj-$(CONFIG_SMP) += smpboot.o
> obj-$(CONFIG_SMP) += smp.o
> diff --git a/arch/riscv/kernel/clint.c b/arch/riscv/kernel/clint.c
> deleted file mode 100644
> index a9845ee023e2..000000000000
> --- a/arch/riscv/kernel/clint.c
> +++ /dev/null
> @@ -1,63 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Copyright (c) 2019 Christoph Hellwig.
> - */
> -
> -#include <linux/io.h>
> -#include <linux/of_address.h>
> -#include <linux/smp.h>
> -#include <linux/types.h>
> -#include <asm/clint.h>
> -#include <asm/csr.h>
> -#include <asm/timex.h>
> -
> -/*
> - * This is the layout used by the SiFive clint, which is also shared by the qemu
> - * virt platform, and the Kendryte KD210 at least.
> - */
> -#define CLINT_IPI_OFF 0
> -#define CLINT_TIME_CMP_OFF 0x4000
> -#define CLINT_TIME_VAL_OFF 0xbff8
> -
> -u32 __iomem *clint_ipi_base;
> -
> -static void clint_send_ipi(const struct cpumask *target)
> -{
> - unsigned int cpu;
> -
> - for_each_cpu(cpu, target)
> - writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
> -}
> -
> -static void clint_clear_ipi(void)
> -{
> - writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
> -}
> -
> -static struct riscv_ipi_ops clint_ipi_ops = {
> - .ipi_inject = clint_send_ipi,
> - .ipi_clear = clint_clear_ipi,
> -};
> -
> -void clint_init_boot_cpu(void)
> -{
> - struct device_node *np;
> - void __iomem *base;
> -
> - np = of_find_compatible_node(NULL, NULL, "riscv,clint0");
> - if (!np) {
> - panic("clint not found");
> - return;
> - }
> -
> - base = of_iomap(np, 0);
> - if (!base)
> - panic("could not map CLINT");
> -
> - clint_ipi_base = base + CLINT_IPI_OFF;
> - riscv_time_cmp = base + CLINT_TIME_CMP_OFF;
> - riscv_time_val = base + CLINT_TIME_VAL_OFF;
> -
> - clint_clear_ipi();
> - riscv_set_ipi_ops(&clint_ipi_ops);
> -}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index f04373be54a6..2c6dd329312b 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -18,7 +18,6 @@
> #include <linux/swiotlb.h>
> #include <linux/smp.h>
>
> -#include <asm/clint.h>
> #include <asm/cpu_ops.h>
> #include <asm/setup.h>
> #include <asm/sections.h>
> @@ -79,7 +78,6 @@ void __init setup_arch(char **cmdline_p)
> #else
> unflatten_device_tree();
> #endif
> - clint_init_boot_cpu();
>
> #ifdef CONFIG_SWIOTLB
> swiotlb_init(1);
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index 8b85683ce203..07626be78c23 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -17,7 +17,6 @@
> #include <linux/seq_file.h>
> #include <linux/delay.h>
>
> -#include <asm/clint.h>
> #include <asm/sbi.h>
> #include <asm/tlbflush.h>
> #include <asm/cacheflush.h>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 5fe849791bf0..a6cfa9842d4b 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -24,7 +24,6 @@
> #include <linux/of.h>
> #include <linux/sched/task_stack.h>
> #include <linux/sched/mm.h>
> -#include <asm/clint.h>
> #include <asm/cpu_ops.h>
> #include <asm/irq.h>
> #include <asm/mmu_context.h>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 41f1c147c178..b24449da3022 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -648,9 +648,8 @@ config ATCPIT100_TIMER
> This option enables support for the Andestech ATCPIT100 timers.
>
> config RISCV_TIMER
> - bool "Timer for the RISC-V platform"
> + bool "Timer for the RISC-V platform" if COMPILE_TEST
> depends on GENERIC_SCHED_CLOCK && RISCV
> - default y
> select TIMER_PROBE
> select TIMER_OF
> help
> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> index 9de1dabfb126..c51c5ed15aa7 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;
> }
>
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Otherwise, looks good.

Reviewed-by: Atish Patra <[email protected]>

--
Regards,
Atish

2020-07-26 05:20:08

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v6 3/4] RISC-V: Remove CLINT related code from timer and arch

On Sat, Jul 25, 2020 at 10:46 AM Atish Patra <[email protected]> wrote:
>
> On Fri, Jul 24, 2020 at 12:19 AM Anup Patel <[email protected]> wrote:
> >
> > Right now the RISC-V timer driver 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.
> >
> > We now have a separate CLINT timer driver which also provide CLINT
> > based IPI operations so let's remove CLINT MMIO related code from
> > arch/riscv directory and RISC-V timer driver.
> >
> > Signed-off-by: Anup Patel <[email protected]>
> > Tested-by: Emil Renner Berhing <[email protected]>
> > Acked-by: Daniel Lezcano <[email protected]>
> > ---
> > arch/riscv/Kconfig | 2 +-
> > arch/riscv/Kconfig.socs | 2 +
> > arch/riscv/configs/nommu_virt_defconfig | 7 +--
> > arch/riscv/include/asm/clint.h | 14 ------
> > arch/riscv/include/asm/timex.h | 28 +++--------
> > arch/riscv/kernel/Makefile | 2 +-
> > arch/riscv/kernel/clint.c | 63 -------------------------
> > arch/riscv/kernel/setup.c | 2 -
> > arch/riscv/kernel/smp.c | 1 -
> > arch/riscv/kernel/smpboot.c | 1 -
> > drivers/clocksource/Kconfig | 3 +-
> > drivers/clocksource/timer-riscv.c | 17 +------
> > 12 files changed, 16 insertions(+), 126 deletions(-)
> > delete mode 100644 arch/riscv/include/asm/clint.h
> > delete mode 100644 arch/riscv/kernel/clint.c
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index fedb4a72b29a..57a72ae23d10 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -74,7 +74,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 SPARSEMEM_STATIC if 32BIT
> > select SPARSE_IRQ
> > select SYSCTL_EXCEPTION_TRACE
> > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > index 6c88148f1b9b..8a55f6156661 100644
> > --- a/arch/riscv/Kconfig.socs
> > +++ b/arch/riscv/Kconfig.socs
> > @@ -12,6 +12,7 @@ config SOC_SIFIVE
> >
> > config SOC_VIRT
> > bool "QEMU Virt Machine"
> > + select CLINT_TIMER if RISCV_M_MODE
> > select POWER_RESET
> > select POWER_RESET_SYSCON
> > select POWER_RESET_SYSCON_POWEROFF
> > @@ -24,6 +25,7 @@ config SOC_VIRT
> > config SOC_KENDRYTE
> > bool "Kendryte K210 SoC"
> > depends on !MMU
> > + select CLINT_TIMER if RISCV_M_MODE
> > select SERIAL_SIFIVE if TTY
> > select SERIAL_SIFIVE_CONSOLE if TTY
> > select SIFIVE_PLIC
> > diff --git a/arch/riscv/configs/nommu_virt_defconfig b/arch/riscv/configs/nommu_virt_defconfig
> > index cf74e179bf90..cf9388184aa3 100644
> > --- a/arch/riscv/configs/nommu_virt_defconfig
> > +++ b/arch/riscv/configs/nommu_virt_defconfig
> > @@ -26,6 +26,7 @@ CONFIG_EXPERT=y
> > CONFIG_SLOB=y
> > # CONFIG_SLAB_MERGE_DEFAULT is not set
> > # CONFIG_MMU is not set
> > +CONFIG_SOC_VIRT=y
> > CONFIG_MAXPHYSMEM_2GB=y
> > CONFIG_SMP=y
> > CONFIG_CMDLINE="root=/dev/vda rw earlycon=uart8250,mmio,0x10000000,115200n8 console=ttyS0"
> > @@ -48,7 +49,6 @@ CONFIG_VIRTIO_BLK=y
> > # CONFIG_SERIO is not set
> > # CONFIG_LEGACY_PTYS is not set
> > # CONFIG_LDISC_AUTOLOAD is not set
> > -# CONFIG_DEVMEM is not set
> > CONFIG_SERIAL_8250=y
> > # CONFIG_SERIAL_8250_DEPRECATED_OPTIONS is not set
> > CONFIG_SERIAL_8250_CONSOLE=y
> > @@ -56,16 +56,13 @@ CONFIG_SERIAL_8250_NR_UARTS=1
> > CONFIG_SERIAL_8250_RUNTIME_UARTS=1
> > CONFIG_SERIAL_OF_PLATFORM=y
> > # CONFIG_HW_RANDOM is not set
> > +# CONFIG_DEVMEM is not set
> > # CONFIG_HWMON is not set
> > -# CONFIG_LCD_CLASS_DEVICE is not set
> > -# CONFIG_BACKLIGHT_CLASS_DEVICE is not set
>
> Why these changes are in the diff ?

These are generated by "make savedefconfig".

To preserve bisectability, I have enabled CONFIG_SOC_VIRT
in nommu_virt_defconfig and I have used "make savedefconfig"
to update nommu_virt_defconfig.

>
> > # CONFIG_VGA_CONSOLE is not set
> > # CONFIG_HID is not set
> > # CONFIG_USB_SUPPORT is not set
> > CONFIG_VIRTIO_MMIO=y
> > CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
> > -CONFIG_SIFIVE_PLIC=y
> > -# CONFIG_VALIDATE_FS_PARSER is not set
> > CONFIG_EXT2_FS=y
> > # CONFIG_DNOTIFY is not set
> > # CONFIG_INOTIFY_USER is not set
> > diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
> > deleted file mode 100644
> > index adaba98a7d6c..000000000000
> > --- a/arch/riscv/include/asm/clint.h
> > +++ /dev/null
> > @@ -1,14 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0 */
> > -#ifndef _ASM_RISCV_CLINT_H
> > -#define _ASM_RISCV_CLINT_H 1
> > -
> > -#include <linux/io.h>
> > -#include <linux/smp.h>
> > -
> > -#ifdef CONFIG_RISCV_M_MODE
> > -void clint_init_boot_cpu(void);
> > -#else /* CONFIG_RISCV_M_MODE */
> > -#define clint_init_boot_cpu() do { } while (0)
> > -#endif /* CONFIG_RISCV_M_MODE */
> > -
> > -#endif /* _ASM_RISCV_CLINT_H */
> > 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/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> > index b355cf485671..7edf15643146 100644
> > --- a/arch/riscv/kernel/Makefile
> > +++ b/arch/riscv/kernel/Makefile
> > @@ -31,7 +31,7 @@ obj-y += cacheinfo.o
> > obj-y += patch.o
> > obj-$(CONFIG_MMU) += vdso.o vdso/
> >
> > -obj-$(CONFIG_RISCV_M_MODE) += clint.o traps_misaligned.o
> > +obj-$(CONFIG_RISCV_M_MODE) += traps_misaligned.o
> > obj-$(CONFIG_FPU) += fpu.o
> > obj-$(CONFIG_SMP) += smpboot.o
> > obj-$(CONFIG_SMP) += smp.o
> > diff --git a/arch/riscv/kernel/clint.c b/arch/riscv/kernel/clint.c
> > deleted file mode 100644
> > index a9845ee023e2..000000000000
> > --- a/arch/riscv/kernel/clint.c
> > +++ /dev/null
> > @@ -1,63 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -/*
> > - * Copyright (c) 2019 Christoph Hellwig.
> > - */
> > -
> > -#include <linux/io.h>
> > -#include <linux/of_address.h>
> > -#include <linux/smp.h>
> > -#include <linux/types.h>
> > -#include <asm/clint.h>
> > -#include <asm/csr.h>
> > -#include <asm/timex.h>
> > -
> > -/*
> > - * This is the layout used by the SiFive clint, which is also shared by the qemu
> > - * virt platform, and the Kendryte KD210 at least.
> > - */
> > -#define CLINT_IPI_OFF 0
> > -#define CLINT_TIME_CMP_OFF 0x4000
> > -#define CLINT_TIME_VAL_OFF 0xbff8
> > -
> > -u32 __iomem *clint_ipi_base;
> > -
> > -static void clint_send_ipi(const struct cpumask *target)
> > -{
> > - unsigned int cpu;
> > -
> > - for_each_cpu(cpu, target)
> > - writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
> > -}
> > -
> > -static void clint_clear_ipi(void)
> > -{
> > - writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
> > -}
> > -
> > -static struct riscv_ipi_ops clint_ipi_ops = {
> > - .ipi_inject = clint_send_ipi,
> > - .ipi_clear = clint_clear_ipi,
> > -};
> > -
> > -void clint_init_boot_cpu(void)
> > -{
> > - struct device_node *np;
> > - void __iomem *base;
> > -
> > - np = of_find_compatible_node(NULL, NULL, "riscv,clint0");
> > - if (!np) {
> > - panic("clint not found");
> > - return;
> > - }
> > -
> > - base = of_iomap(np, 0);
> > - if (!base)
> > - panic("could not map CLINT");
> > -
> > - clint_ipi_base = base + CLINT_IPI_OFF;
> > - riscv_time_cmp = base + CLINT_TIME_CMP_OFF;
> > - riscv_time_val = base + CLINT_TIME_VAL_OFF;
> > -
> > - clint_clear_ipi();
> > - riscv_set_ipi_ops(&clint_ipi_ops);
> > -}
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index f04373be54a6..2c6dd329312b 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -18,7 +18,6 @@
> > #include <linux/swiotlb.h>
> > #include <linux/smp.h>
> >
> > -#include <asm/clint.h>
> > #include <asm/cpu_ops.h>
> > #include <asm/setup.h>
> > #include <asm/sections.h>
> > @@ -79,7 +78,6 @@ void __init setup_arch(char **cmdline_p)
> > #else
> > unflatten_device_tree();
> > #endif
> > - clint_init_boot_cpu();
> >
> > #ifdef CONFIG_SWIOTLB
> > swiotlb_init(1);
> > diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> > index 8b85683ce203..07626be78c23 100644
> > --- a/arch/riscv/kernel/smp.c
> > +++ b/arch/riscv/kernel/smp.c
> > @@ -17,7 +17,6 @@
> > #include <linux/seq_file.h>
> > #include <linux/delay.h>
> >
> > -#include <asm/clint.h>
> > #include <asm/sbi.h>
> > #include <asm/tlbflush.h>
> > #include <asm/cacheflush.h>
> > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > index 5fe849791bf0..a6cfa9842d4b 100644
> > --- a/arch/riscv/kernel/smpboot.c
> > +++ b/arch/riscv/kernel/smpboot.c
> > @@ -24,7 +24,6 @@
> > #include <linux/of.h>
> > #include <linux/sched/task_stack.h>
> > #include <linux/sched/mm.h>
> > -#include <asm/clint.h>
> > #include <asm/cpu_ops.h>
> > #include <asm/irq.h>
> > #include <asm/mmu_context.h>
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 41f1c147c178..b24449da3022 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -648,9 +648,8 @@ config ATCPIT100_TIMER
> > This option enables support for the Andestech ATCPIT100 timers.
> >
> > config RISCV_TIMER
> > - bool "Timer for the RISC-V platform"
> > + bool "Timer for the RISC-V platform" if COMPILE_TEST
> > depends on GENERIC_SCHED_CLOCK && RISCV
> > - default y
> > select TIMER_PROBE
> > select TIMER_OF
> > help
> > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> > index 9de1dabfb126..c51c5ed15aa7 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;
> > }
> >
> > --
> > 2.25.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> Otherwise, looks good.
>
> Reviewed-by: Atish Patra <[email protected]>
>
> --
> Regards,
> Atish

Regards,
Anup

2020-07-27 19:56:57

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v6 4/4] dt-bindings: timer: Add CLINT bindings

On Fri, 24 Jul 2020 12:48:22 +0530, Anup Patel wrote:
> We add DT bindings documentation for CLINT device.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Palmer Dabbelt <[email protected]>
> Tested-by: Emil Renner Berhing <[email protected]>
> ---
> .../bindings/timer/sifive,clint.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.yaml
>

Reviewed-by: Rob Herring <[email protected]>

2020-08-05 01:48:30

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] clocksource/drivers: Add CLINT timer driver

On Fri, 24 Jul 2020 00:18:20 PDT (-0700), Anup Patel wrote:
> We add a separate CLINT timer driver for Linux RISC-V M-mode (i.e.
> RISC-V NoMMU kernel).
>
> The CLINT MMIO device provides three things:
> 1. 64bit free running counter register
> 2. 64bit per-CPU time compare registers
> 3. 32bit per-CPU inter-processor interrupt registers
>
> Unlike other timer devices, CLINT provides IPI registers along with
> timer registers. To use CLINT IPI registers, the CLINT timer driver
> provides IPI related callbacks to arch/riscv.
>
> Signed-off-by: Anup Patel <[email protected]>
> Tested-by: Emil Renner Berhing <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---
> drivers/clocksource/Kconfig | 9 ++
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++++++++++++++
> include/linux/cpuhotplug.h | 1 +
> 4 files changed, 237 insertions(+)
> create mode 100644 drivers/clocksource/timer-clint.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 91418381fcd4..41f1c147c178 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -658,6 +658,15 @@ config RISCV_TIMER
> is accessed via both the SBI and the rdcycle instruction. This is
> required for all RISC-V systems.
>
> +config CLINT_TIMER
> + bool "CLINT Timer for the RISC-V platform" if COMPILE_TEST
> + depends on GENERIC_SCHED_CLOCK && RISCV
> + select TIMER_PROBE
> + select TIMER_OF
> + help
> + This option enables the CLINT timer for RISC-V systems. The CLINT
> + driver is usually used for NoMMU RISC-V systems.
> +
> config CSKY_MP_TIMER
> bool "SMP Timer for the C-SKY platform" if COMPILE_TEST
> depends on CSKY
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index bdda1a2e4097..18e700e703a0 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -87,6 +87,7 @@ obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o
> obj-$(CONFIG_X86_NUMACHIP) += numachip.o
> obj-$(CONFIG_ATCPIT100_TIMER) += timer-atcpit100.o
> obj-$(CONFIG_RISCV_TIMER) += timer-riscv.o
> +obj-$(CONFIG_CLINT_TIMER) += timer-clint.o
> obj-$(CONFIG_CSKY_MP_TIMER) += timer-mp-csky.o
> obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o
> obj-$(CONFIG_HYPERV_TIMER) += hyperv_timer.o
> diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
> new file mode 100644
> index 000000000000..8eeafa82c03d
> --- /dev/null
> +++ b/drivers/clocksource/timer-clint.c
> @@ -0,0 +1,226 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> + *
> + * Most of the M-mode (i.e. NoMMU) RISC-V systems usually have a
> + * CLINT MMIO timer device.
> + */
> +
> +#define pr_fmt(fmt) "clint: " fmt
> +#include <linux/bitops.h>
> +#include <linux/clocksource.h>
> +#include <linux/clockchips.h>
> +#include <linux/cpu.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/sched_clock.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_irq.h>
> +#include <linux/smp.h>
> +
> +#define CLINT_IPI_OFF 0
> +#define CLINT_TIMER_CMP_OFF 0x4000
> +#define CLINT_TIMER_VAL_OFF 0xbff8
> +
> +/* CLINT manages IPI and Timer for RISC-V M-mode */
> +static u32 __iomem *clint_ipi_base;
> +static u64 __iomem *clint_timer_cmp;
> +static u64 __iomem *clint_timer_val;
> +static unsigned long clint_timer_freq;
> +static unsigned int clint_timer_irq;
> +
> +static void clint_send_ipi(const struct cpumask *target)
> +{
> + unsigned int cpu;
> +
> + for_each_cpu(cpu, target)
> + writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
> +}
> +
> +static void clint_clear_ipi(void)
> +{
> + writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
> +}
> +
> +static struct riscv_ipi_ops clint_ipi_ops = {
> + .ipi_inject = clint_send_ipi,
> + .ipi_clear = clint_clear_ipi,
> +};
> +
> +#ifdef CONFIG_64BIT
> +#define clint_get_cycles() readq_relaxed(clint_timer_val)
> +#else
> +#define clint_get_cycles() readl_relaxed(clint_timer_val)
> +#define clint_get_cycles_hi() readl_relaxed(((u32 *)clint_timer_val) + 1)
> +#endif
> +
> +#ifdef CONFIG_64BIT
> +static u64 notrace clint_get_cycles64(void)
> +{
> + return clint_get_cycles();
> +}
> +#else /* CONFIG_64BIT */
> +static u64 notrace clint_get_cycles64(void)
> +{
> + u32 hi, lo;
> +
> + do {
> + hi = clint_get_cycles_hi();
> + lo = clint_get_cycles();
> + } while (hi != clint_get_cycles_hi());
> +
> + return ((u64)hi << 32) | lo;
> +}
> +#endif /* CONFIG_64BIT */
> +
> +static u64 clint_rdtime(struct clocksource *cs)
> +{
> + return clint_get_cycles64();
> +}
> +
> +static struct clocksource clint_clocksource = {
> + .name = "clint_clocksource",
> + .rating = 300,
> + .mask = CLOCKSOURCE_MASK(64),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> + .read = clint_rdtime,
> +};
> +
> +static int clint_clock_next_event(unsigned long delta,
> + struct clock_event_device *ce)
> +{
> + void __iomem *r = clint_timer_cmp +
> + cpuid_to_hartid_map(smp_processor_id());
> +
> + csr_set(CSR_IE, IE_TIE);
> + writeq_relaxed(clint_get_cycles64() + delta, r);
> + return 0;
> +}
> +
> +static DEFINE_PER_CPU(struct clock_event_device, clint_clock_event) = {
> + .name = "clint_clockevent",
> + .features = CLOCK_EVT_FEAT_ONESHOT,
> + .rating = 100,
> + .set_next_event = clint_clock_next_event,
> +};
> +
> +static int clint_timer_starting_cpu(unsigned int cpu)
> +{
> + struct clock_event_device *ce = per_cpu_ptr(&clint_clock_event, cpu);
> +
> + ce->cpumask = cpumask_of(cpu);
> + clockevents_config_and_register(ce, clint_timer_freq, 100, 0x7fffffff);
> +
> + enable_percpu_irq(clint_timer_irq,
> + irq_get_trigger_type(clint_timer_irq));
> + return 0;
> +}
> +
> +static int clint_timer_dying_cpu(unsigned int cpu)
> +{
> + disable_percpu_irq(clint_timer_irq);
> + return 0;
> +}
> +
> +static irqreturn_t clint_timer_interrupt(int irq, void *dev_id)
> +{
> + struct clock_event_device *evdev = this_cpu_ptr(&clint_clock_event);
> +
> + csr_clear(CSR_IE, IE_TIE);
> + evdev->event_handler(evdev);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int __init clint_timer_init_dt(struct device_node *np)
> +{
> + int rc;
> + u32 i, nr_irqs;
> + void __iomem *base;
> + struct of_phandle_args oirq;
> +
> + /*
> + * Ensure that CLINT device interrupts are either RV_IRQ_TIMER or
> + * RV_IRQ_SOFT. If it's anything else then we ignore the device.
> + */
> + nr_irqs = of_irq_count(np);
> + for (i = 0; i < nr_irqs; i++) {
> + if (of_irq_parse_one(np, i, &oirq)) {
> + pr_err("%pOFP: failed to parse irq %d.\n", np, i);
> + continue;
> + }
> +
> + if ((oirq.args_count != 1) ||
> + (oirq.args[0] != RV_IRQ_TIMER &&
> + oirq.args[0] != RV_IRQ_SOFT)) {
> + pr_err("%pOFP: invalid irq %d (hwirq %d)\n",
> + np, i, oirq.args[0]);
> + return -ENODEV;
> + }
> +
> + /* Find parent irq domain and map timer irq */
> + if (!clint_timer_irq &&
> + oirq.args[0] == RV_IRQ_TIMER &&
> + irq_find_host(oirq.np))
> + clint_timer_irq = irq_of_parse_and_map(np, i);
> + }
> +
> + /* If CLINT timer irq not found then fail */
> + if (!clint_timer_irq) {
> + pr_err("%pOFP: timer irq not found\n", np);
> + return -ENODEV;
> + }
> +
> + base = of_iomap(np, 0);
> + if (!base) {
> + pr_err("%pOFP: could not map registers\n", np);
> + return -ENODEV;
> + }
> +
> + clint_ipi_base = base + CLINT_IPI_OFF;
> + clint_timer_cmp = base + CLINT_TIMER_CMP_OFF;
> + clint_timer_val = base + CLINT_TIMER_VAL_OFF;
> + clint_timer_freq = riscv_timebase;
> +
> + pr_info("%pOFP: timer running at %ld Hz\n", np, clint_timer_freq);
> +
> + rc = clocksource_register_hz(&clint_clocksource, clint_timer_freq);
> + if (rc) {
> + pr_err("%pOFP: clocksource register failed [%d]\n", np, rc);
> + goto fail_iounmap;
> + }
> +
> + sched_clock_register(clint_get_cycles64, 64, clint_timer_freq);
> +
> + rc = request_percpu_irq(clint_timer_irq, clint_timer_interrupt,
> + "clint-timer", &clint_clock_event);
> + if (rc) {
> + pr_err("registering percpu irq failed [%d]\n", rc);
> + goto fail_iounmap;
> + }
> +
> + rc = cpuhp_setup_state(CPUHP_AP_CLINT_TIMER_STARTING,
> + "clockevents/clint/timer:starting",
> + clint_timer_starting_cpu,
> + clint_timer_dying_cpu);
> + if (rc) {
> + pr_err("%pOFP: cpuhp setup state failed [%d]\n", np, rc);
> + goto fail_free_irq;
> + }
> +
> + riscv_set_ipi_ops(&clint_ipi_ops);
> + clint_clear_ipi();
> +
> + return 0;
> +
> +fail_free_irq:
> + free_irq(clint_timer_irq, &clint_clock_event);
> +fail_iounmap:
> + iounmap(base);
> + return rc;
> +}
> +
> +TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
> +TIMER_OF_DECLARE(clint_timer1, "sifive,clint0", clint_timer_init_dt);
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 191772d4a4d7..1451f4625833 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -132,6 +132,7 @@ enum cpuhp_state {
> CPUHP_AP_MIPS_GIC_TIMER_STARTING,
> CPUHP_AP_ARC_TIMER_STARTING,
> CPUHP_AP_RISCV_TIMER_STARTING,
> + CPUHP_AP_CLINT_TIMER_STARTING,
> CPUHP_AP_CSKY_TIMER_STARTING,
> CPUHP_AP_HYPERV_TIMER_STARTING,
> CPUHP_AP_KVM_STARTING,

Reviewed-by: Palmer Dabbelt <[email protected]>

2020-08-05 01:48:34

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v6 3/4] RISC-V: Remove CLINT related code from timer and arch

On Fri, 24 Jul 2020 00:18:21 PDT (-0700), Anup Patel wrote:
> Right now the RISC-V timer driver 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.
>
> We now have a separate CLINT timer driver which also provide CLINT
> based IPI operations so let's remove CLINT MMIO related code from
> arch/riscv directory and RISC-V timer driver.
>
> Signed-off-by: Anup Patel <[email protected]>
> Tested-by: Emil Renner Berhing <[email protected]>
> Acked-by: Daniel Lezcano <[email protected]>
> ---
> arch/riscv/Kconfig | 2 +-
> arch/riscv/Kconfig.socs | 2 +
> arch/riscv/configs/nommu_virt_defconfig | 7 +--
> arch/riscv/include/asm/clint.h | 14 ------
> arch/riscv/include/asm/timex.h | 28 +++--------
> arch/riscv/kernel/Makefile | 2 +-
> arch/riscv/kernel/clint.c | 63 -------------------------
> arch/riscv/kernel/setup.c | 2 -
> arch/riscv/kernel/smp.c | 1 -
> arch/riscv/kernel/smpboot.c | 1 -
> drivers/clocksource/Kconfig | 3 +-
> drivers/clocksource/timer-riscv.c | 17 +------
> 12 files changed, 16 insertions(+), 126 deletions(-)
> delete mode 100644 arch/riscv/include/asm/clint.h
> delete mode 100644 arch/riscv/kernel/clint.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index fedb4a72b29a..57a72ae23d10 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -74,7 +74,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 SPARSEMEM_STATIC if 32BIT
> select SPARSE_IRQ
> select SYSCTL_EXCEPTION_TRACE
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 6c88148f1b9b..8a55f6156661 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -12,6 +12,7 @@ config SOC_SIFIVE
>
> config SOC_VIRT
> bool "QEMU Virt Machine"
> + select CLINT_TIMER if RISCV_M_MODE
> select POWER_RESET
> select POWER_RESET_SYSCON
> select POWER_RESET_SYSCON_POWEROFF
> @@ -24,6 +25,7 @@ config SOC_VIRT
> config SOC_KENDRYTE
> bool "Kendryte K210 SoC"
> depends on !MMU
> + select CLINT_TIMER if RISCV_M_MODE
> select SERIAL_SIFIVE if TTY
> select SERIAL_SIFIVE_CONSOLE if TTY
> select SIFIVE_PLIC
> diff --git a/arch/riscv/configs/nommu_virt_defconfig b/arch/riscv/configs/nommu_virt_defconfig
> index cf74e179bf90..cf9388184aa3 100644
> --- a/arch/riscv/configs/nommu_virt_defconfig
> +++ b/arch/riscv/configs/nommu_virt_defconfig
> @@ -26,6 +26,7 @@ CONFIG_EXPERT=y
> CONFIG_SLOB=y
> # CONFIG_SLAB_MERGE_DEFAULT is not set
> # CONFIG_MMU is not set
> +CONFIG_SOC_VIRT=y
> CONFIG_MAXPHYSMEM_2GB=y
> CONFIG_SMP=y
> CONFIG_CMDLINE="root=/dev/vda rw earlycon=uart8250,mmio,0x10000000,115200n8 console=ttyS0"
> @@ -48,7 +49,6 @@ CONFIG_VIRTIO_BLK=y
> # CONFIG_SERIO is not set
> # CONFIG_LEGACY_PTYS is not set
> # CONFIG_LDISC_AUTOLOAD is not set
> -# CONFIG_DEVMEM is not set
> CONFIG_SERIAL_8250=y
> # CONFIG_SERIAL_8250_DEPRECATED_OPTIONS is not set
> CONFIG_SERIAL_8250_CONSOLE=y
> @@ -56,16 +56,13 @@ CONFIG_SERIAL_8250_NR_UARTS=1
> CONFIG_SERIAL_8250_RUNTIME_UARTS=1
> CONFIG_SERIAL_OF_PLATFORM=y
> # CONFIG_HW_RANDOM is not set
> +# CONFIG_DEVMEM is not set
> # CONFIG_HWMON is not set
> -# CONFIG_LCD_CLASS_DEVICE is not set
> -# CONFIG_BACKLIGHT_CLASS_DEVICE is not set
> # CONFIG_VGA_CONSOLE is not set
> # CONFIG_HID is not set
> # CONFIG_USB_SUPPORT is not set
> CONFIG_VIRTIO_MMIO=y
> CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
> -CONFIG_SIFIVE_PLIC=y
> -# CONFIG_VALIDATE_FS_PARSER is not set
> CONFIG_EXT2_FS=y
> # CONFIG_DNOTIFY is not set
> # CONFIG_INOTIFY_USER is not set
> diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
> deleted file mode 100644
> index adaba98a7d6c..000000000000
> --- a/arch/riscv/include/asm/clint.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef _ASM_RISCV_CLINT_H
> -#define _ASM_RISCV_CLINT_H 1
> -
> -#include <linux/io.h>
> -#include <linux/smp.h>
> -
> -#ifdef CONFIG_RISCV_M_MODE
> -void clint_init_boot_cpu(void);
> -#else /* CONFIG_RISCV_M_MODE */
> -#define clint_init_boot_cpu() do { } while (0)
> -#endif /* CONFIG_RISCV_M_MODE */
> -
> -#endif /* _ASM_RISCV_CLINT_H */
> 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/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index b355cf485671..7edf15643146 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -31,7 +31,7 @@ obj-y += cacheinfo.o
> obj-y += patch.o
> obj-$(CONFIG_MMU) += vdso.o vdso/
>
> -obj-$(CONFIG_RISCV_M_MODE) += clint.o traps_misaligned.o
> +obj-$(CONFIG_RISCV_M_MODE) += traps_misaligned.o
> obj-$(CONFIG_FPU) += fpu.o
> obj-$(CONFIG_SMP) += smpboot.o
> obj-$(CONFIG_SMP) += smp.o
> diff --git a/arch/riscv/kernel/clint.c b/arch/riscv/kernel/clint.c
> deleted file mode 100644
> index a9845ee023e2..000000000000
> --- a/arch/riscv/kernel/clint.c
> +++ /dev/null
> @@ -1,63 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Copyright (c) 2019 Christoph Hellwig.
> - */
> -
> -#include <linux/io.h>
> -#include <linux/of_address.h>
> -#include <linux/smp.h>
> -#include <linux/types.h>
> -#include <asm/clint.h>
> -#include <asm/csr.h>
> -#include <asm/timex.h>
> -
> -/*
> - * This is the layout used by the SiFive clint, which is also shared by the qemu
> - * virt platform, and the Kendryte KD210 at least.
> - */
> -#define CLINT_IPI_OFF 0
> -#define CLINT_TIME_CMP_OFF 0x4000
> -#define CLINT_TIME_VAL_OFF 0xbff8
> -
> -u32 __iomem *clint_ipi_base;
> -
> -static void clint_send_ipi(const struct cpumask *target)
> -{
> - unsigned int cpu;
> -
> - for_each_cpu(cpu, target)
> - writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
> -}
> -
> -static void clint_clear_ipi(void)
> -{
> - writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
> -}
> -
> -static struct riscv_ipi_ops clint_ipi_ops = {
> - .ipi_inject = clint_send_ipi,
> - .ipi_clear = clint_clear_ipi,
> -};
> -
> -void clint_init_boot_cpu(void)
> -{
> - struct device_node *np;
> - void __iomem *base;
> -
> - np = of_find_compatible_node(NULL, NULL, "riscv,clint0");
> - if (!np) {
> - panic("clint not found");
> - return;
> - }
> -
> - base = of_iomap(np, 0);
> - if (!base)
> - panic("could not map CLINT");
> -
> - clint_ipi_base = base + CLINT_IPI_OFF;
> - riscv_time_cmp = base + CLINT_TIME_CMP_OFF;
> - riscv_time_val = base + CLINT_TIME_VAL_OFF;
> -
> - clint_clear_ipi();
> - riscv_set_ipi_ops(&clint_ipi_ops);
> -}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index f04373be54a6..2c6dd329312b 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -18,7 +18,6 @@
> #include <linux/swiotlb.h>
> #include <linux/smp.h>
>
> -#include <asm/clint.h>
> #include <asm/cpu_ops.h>
> #include <asm/setup.h>
> #include <asm/sections.h>
> @@ -79,7 +78,6 @@ void __init setup_arch(char **cmdline_p)
> #else
> unflatten_device_tree();
> #endif
> - clint_init_boot_cpu();
>
> #ifdef CONFIG_SWIOTLB
> swiotlb_init(1);
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index 8b85683ce203..07626be78c23 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -17,7 +17,6 @@
> #include <linux/seq_file.h>
> #include <linux/delay.h>
>
> -#include <asm/clint.h>
> #include <asm/sbi.h>
> #include <asm/tlbflush.h>
> #include <asm/cacheflush.h>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 5fe849791bf0..a6cfa9842d4b 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -24,7 +24,6 @@
> #include <linux/of.h>
> #include <linux/sched/task_stack.h>
> #include <linux/sched/mm.h>
> -#include <asm/clint.h>
> #include <asm/cpu_ops.h>
> #include <asm/irq.h>
> #include <asm/mmu_context.h>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 41f1c147c178..b24449da3022 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -648,9 +648,8 @@ config ATCPIT100_TIMER
> This option enables support for the Andestech ATCPIT100 timers.
>
> config RISCV_TIMER
> - bool "Timer for the RISC-V platform"
> + bool "Timer for the RISC-V platform" if COMPILE_TEST
> depends on GENERIC_SCHED_CLOCK && RISCV
> - default y
> select TIMER_PROBE
> select TIMER_OF
> help
> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> index 9de1dabfb126..c51c5ed15aa7 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;
> }

Reviewed-by: Palmer Dabbelt <[email protected]>

2020-08-05 01:48:39

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v6 0/4] Dedicated CLINT timer driver

On Fri, 24 Jul 2020 00:18:18 PDT (-0700), Anup Patel wrote:
> The current RISC-V timer driver is convoluted and implements two
> distinct timers:
> 1. S-mode timer: This is for Linux RISC-V S-mode with MMU. The
> clocksource is implemented using TIME CSR and clockevent device
> is implemented using SBI Timer calls.
> 2. M-mode timer: This is for Linux RISC-V M-mode without MMU. The
> clocksource is implemented using CLINT MMIO time register and
> clockevent device is implemented using CLINT MMIO timecmp registers.
>
> This patchset removes clint related code from RISC-V timer driver and
> arch/riscv directory. Instead, the series adds a dedicated MMIO based
> CLINT driver under drivers/clocksource directory which can be used by
> Linux RISC-V M-mode (i.e NoMMU Linux RISC-V).
>
> The patchset is based up Linux-5.8-rc6 and can be found at riscv_clint_v6
> branch of: https://github.com/avpatel/linux.git
>
> This series is tested on:
> 1. QEMU RV64 virt machine using Linux RISC-V S-mode
> 2. QEMU RV32 virt machine using Linux RISC-V S-mode
> 3. QEMU RV64 virt machine using Linux RISC-V M-mode (i.e. NoMMU)
>
> Changes since v5:
> - Fixed order of compatible strings in PATCH4
> - Added "additionalProperties: false" in PATCH4
> - Fixed register space size for example DT node in PATCH4
>
> Changes since v4:
> - Rebased series on Linux-5.8-rc6
> - Updated Kconfig option as suggested by Daniel in PATCH2
> - Removed per-CPU registered flag in PATCH2
> - Addressed nit comments from Atish in PATCH2
>
> Changes since v3:
> - Updated commit description of PATCH2
> - Use clint_get_cycles64() in clint_rdtime() of PATCH2
> - Call clockevents_config_and_register() only once for each CPU in
> clint_timer_starting_cpu of PATCH2
> - Select CLINT timer driver from platform Kconfig in PATCH3
> - Fixed 'make dt_binding_check' for PATCH4
>
> Changes since v2:
> - Rebased series on Linux-5.8-rc5
> - Squashed PATCH3 onto PATCH2 to preserve GIT bisectability
> - Moved PATCH4 before PATCH2 to preserve GIT bisectability
> - Replaced CLINT dt-bindings text document with YAML schema
> - Use SiFive CLINT compatible string as per SiFive IP block versioning
>
> Changes since v1:
> - Rebased series on Linux-5.8-rc2
> - Added pr_warn() for case where ipi_ops not available in PATCH1
> - Updated ipi_inject() prototype to use "struct cpumask *" in PATCH1
> - Updated CLINT_TIMER kconfig option to depend on RISCV_M_MODE in PATCH4
> - Added riscv,clint0 compatible string in DT bindings document
>
> Anup Patel (4):
> RISC-V: Add mechanism to provide custom IPI operations
> clocksource/drivers: Add CLINT timer driver
> RISC-V: Remove CLINT related code from timer and arch
> dt-bindings: timer: Add CLINT bindings
>
> .../bindings/timer/sifive,clint.yaml | 60 +++++
> arch/riscv/Kconfig | 2 +-
> arch/riscv/Kconfig.socs | 2 +
> arch/riscv/configs/nommu_virt_defconfig | 7 +-
> arch/riscv/include/asm/clint.h | 39 ---
> arch/riscv/include/asm/smp.h | 19 ++
> arch/riscv/include/asm/timex.h | 28 +--
> arch/riscv/kernel/Makefile | 2 +-
> arch/riscv/kernel/clint.c | 44 ----
> arch/riscv/kernel/sbi.c | 14 ++
> arch/riscv/kernel/setup.c | 2 -
> arch/riscv/kernel/smp.c | 44 ++--
> arch/riscv/kernel/smpboot.c | 4 +-
> drivers/clocksource/Kconfig | 12 +-
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++
> drivers/clocksource/timer-riscv.c | 17 +-
> include/linux/cpuhotplug.h | 1 +
> 18 files changed, 371 insertions(+), 153 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.yaml
> delete mode 100644 arch/riscv/include/asm/clint.h
> delete mode 100644 arch/riscv/kernel/clint.c
> create mode 100644 drivers/clocksource/timer-clint.c

Thanks, this is way cleaner. Patchwork is still broken but IIRC we reached
consensus on these. I'm not going to include these in my first 5.9 PR, as I
want to get that out tomorrow to avoid more merge conflicts, but assuming
there's reviews from the other maintainers I'd like to take this for my second
5.9 merge window PR.

Assuming you've been collecting reviews and acks, do you mind posting another
version with them? If not I have some scripts to dig them out, so it's not a
big deal.

2020-08-05 01:49:13

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] RISC-V: Add mechanism to provide custom IPI operations

On Fri, 24 Jul 2020 00:18:19 PDT (-0700), Anup Patel wrote:
> We add mechanism to set custom IPI operations so that CLINT driver
> from drivers directory can provide custom IPI operations.
>
> Signed-off-by: Anup Patel <[email protected]>
> Tested-by: Emil Renner Berhing <[email protected]>
> Reviewed-by: Atish Patra <[email protected]>
> ---
> arch/riscv/include/asm/clint.h | 25 --------------------
> arch/riscv/include/asm/smp.h | 19 +++++++++++++++
> arch/riscv/kernel/clint.c | 23 ++++++++++++++++--
> arch/riscv/kernel/sbi.c | 14 +++++++++++
> arch/riscv/kernel/smp.c | 43 +++++++++++++++++++---------------
> arch/riscv/kernel/smpboot.c | 3 +--
> 6 files changed, 79 insertions(+), 48 deletions(-)
>
> diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
> index a279b17a6aad..adaba98a7d6c 100644
> --- a/arch/riscv/include/asm/clint.h
> +++ b/arch/riscv/include/asm/clint.h
> @@ -6,34 +6,9 @@
> #include <linux/smp.h>
>
> #ifdef CONFIG_RISCV_M_MODE
> -extern u32 __iomem *clint_ipi_base;
> -
> void clint_init_boot_cpu(void);
> -
> -static inline void clint_send_ipi_single(unsigned long hartid)
> -{
> - writel(1, clint_ipi_base + hartid);
> -}
> -
> -static inline void clint_send_ipi_mask(const struct cpumask *mask)
> -{
> - int cpu;
> -
> - for_each_cpu(cpu, mask)
> - clint_send_ipi_single(cpuid_to_hartid_map(cpu));
> -}
> -
> -static inline void clint_clear_ipi(unsigned long hartid)
> -{
> - writel(0, clint_ipi_base + hartid);
> -}
> #else /* CONFIG_RISCV_M_MODE */
> #define clint_init_boot_cpu() do { } while (0)
> -
> -/* stubs to for code is only reachable under IS_ENABLED(CONFIG_RISCV_M_MODE): */
> -void clint_send_ipi_single(unsigned long hartid);
> -void clint_send_ipi_mask(const struct cpumask *hartid_mask);
> -void clint_clear_ipi(unsigned long hartid);
> #endif /* CONFIG_RISCV_M_MODE */
>
> #endif /* _ASM_RISCV_CLINT_H */

So this is entirely unrelated to the actual contents of this patch, I'd just
forgotten about it before: the scheme used by clint_init_boot_cpu() only works
once per system reset (and as far as I can tell, that code only works for
two-CPU systems). Not really an issue with this patch set, but we should
probably put a sanity check in the code -- the two-CPU thing should be easy,
but the "only use the CLINT init trick once" thing need some sort of platform
interface.

> diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
> index 40bb1c15a731..68de78a8eba6 100644
> --- a/arch/riscv/include/asm/smp.h
> +++ b/arch/riscv/include/asm/smp.h
> @@ -15,6 +15,11 @@
> struct seq_file;
> extern unsigned long boot_cpu_hartid;
>
> +struct riscv_ipi_ops {
> + void (*ipi_inject)(const struct cpumask *target);
> + void (*ipi_clear)(void);
> +};
> +
> #ifdef CONFIG_SMP
> /*
> * Mapping between linux logical cpu index and hartid.
> @@ -40,6 +45,12 @@ void arch_send_call_function_single_ipi(int cpu);
> int riscv_hartid_to_cpuid(int hartid);
> void riscv_cpuid_to_hartid_mask(const struct cpumask *in, struct cpumask *out);
>
> +/* Set custom IPI operations */
> +void riscv_set_ipi_ops(struct riscv_ipi_ops *ops);
> +
> +/* Clear IPI for current CPU */
> +void riscv_clear_ipi(void);
> +
> /*
> * Obtains the hart ID of the currently executing task. This relies on
> * THREAD_INFO_IN_TASK, but we define that unconditionally.
> @@ -78,6 +89,14 @@ static inline void riscv_cpuid_to_hartid_mask(const struct cpumask *in,
> cpumask_set_cpu(boot_cpu_hartid, out);
> }
>
> +static inline void riscv_set_ipi_ops(struct riscv_ipi_ops *ops)
> +{
> +}

It's a bit pedantic, but it seems like this should do something as of this
patch -- even though there's only a single driver, it's odd to have this both
exist and do nothing.

> +
> +static inline void riscv_clear_ipi(void)
> +{
> +}
> +
> #endif /* CONFIG_SMP */
>
> #if defined(CONFIG_HOTPLUG_CPU) && (CONFIG_SMP)
> diff --git a/arch/riscv/kernel/clint.c b/arch/riscv/kernel/clint.c
> index 3647980d14c3..a9845ee023e2 100644
> --- a/arch/riscv/kernel/clint.c
> +++ b/arch/riscv/kernel/clint.c
> @@ -5,11 +5,11 @@
>
> #include <linux/io.h>
> #include <linux/of_address.h>
> +#include <linux/smp.h>
> #include <linux/types.h>
> #include <asm/clint.h>
> #include <asm/csr.h>
> #include <asm/timex.h>
> -#include <asm/smp.h>
>
> /*
> * This is the layout used by the SiFive clint, which is also shared by the qemu
> @@ -21,6 +21,24 @@
>
> u32 __iomem *clint_ipi_base;
>
> +static void clint_send_ipi(const struct cpumask *target)
> +{
> + unsigned int cpu;
> +
> + for_each_cpu(cpu, target)
> + writel(1, clint_ipi_base + cpuid_to_hartid_map(cpu));
> +}
> +
> +static void clint_clear_ipi(void)
> +{
> + writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
> +}
> +
> +static struct riscv_ipi_ops clint_ipi_ops = {
> + .ipi_inject = clint_send_ipi,
> + .ipi_clear = clint_clear_ipi,
> +};
> +
> void clint_init_boot_cpu(void)
> {
> struct device_node *np;
> @@ -40,5 +58,6 @@ void clint_init_boot_cpu(void)
> riscv_time_cmp = base + CLINT_TIME_CMP_OFF;
> riscv_time_val = base + CLINT_TIME_VAL_OFF;
>
> - clint_clear_ipi(boot_cpu_hartid);
> + clint_clear_ipi();
> + riscv_set_ipi_ops(&clint_ipi_ops);
> }
> diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
> index f383ef5672b2..226ccce0f9e0 100644
> --- a/arch/riscv/kernel/sbi.c
> +++ b/arch/riscv/kernel/sbi.c
> @@ -547,6 +547,18 @@ static inline long sbi_get_firmware_version(void)
> return __sbi_base_ecall(SBI_EXT_BASE_GET_IMP_VERSION);
> }
>
> +static void sbi_send_cpumask_ipi(const struct cpumask *target)
> +{
> + struct cpumask hartid_mask;
> +
> + riscv_cpuid_to_hartid_mask(target, &hartid_mask);
> +
> + sbi_send_ipi(cpumask_bits(&hartid_mask));
> +}
> +
> +static struct riscv_ipi_ops sbi_ipi_ops = {
> + .ipi_inject = sbi_send_cpumask_ipi
> +};
>
> int __init sbi_init(void)
> {
> @@ -587,5 +599,7 @@ int __init sbi_init(void)
> __sbi_rfence = __sbi_rfence_v01;
> }
>
> + riscv_set_ipi_ops(&sbi_ipi_ops);
> +
> return 0;
> }
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index b1d4f452f843..8b85683ce203 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -84,9 +84,25 @@ static void ipi_stop(void)
> wait_for_interrupt();
> }
>
> +static struct riscv_ipi_ops *ipi_ops;
> +
> +void riscv_set_ipi_ops(struct riscv_ipi_ops *ops)
> +{
> + ipi_ops = ops;
> +}
> +EXPORT_SYMBOL_GPL(riscv_set_ipi_ops);
> +
> +void riscv_clear_ipi(void)
> +{
> + if (ipi_ops && ipi_ops->ipi_clear)
> + ipi_ops->ipi_clear();
> +
> + csr_clear(CSR_IP, IE_SIE);
> +}
> +EXPORT_SYMBOL_GPL(riscv_clear_ipi);
> +
> static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
> {
> - struct cpumask hartid_mask;
> int cpu;
>
> smp_mb__before_atomic();
> @@ -94,33 +110,22 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
> set_bit(op, &ipi_data[cpu].bits);
> smp_mb__after_atomic();
>
> - riscv_cpuid_to_hartid_mask(mask, &hartid_mask);
> - if (IS_ENABLED(CONFIG_RISCV_SBI))
> - sbi_send_ipi(cpumask_bits(&hartid_mask));
> + if (ipi_ops && ipi_ops->ipi_inject)
> + ipi_ops->ipi_inject(mask);
> else
> - clint_send_ipi_mask(mask);
> + pr_warn("SMP: IPI inject method not available\n");
> }
>
> static void send_ipi_single(int cpu, enum ipi_message_type op)
> {
> - int hartid = cpuid_to_hartid_map(cpu);
> -
> smp_mb__before_atomic();
> set_bit(op, &ipi_data[cpu].bits);
> smp_mb__after_atomic();
>
> - if (IS_ENABLED(CONFIG_RISCV_SBI))
> - sbi_send_ipi(cpumask_bits(cpumask_of(hartid)));
> - else
> - clint_send_ipi_single(hartid);
> -}
> -
> -static inline void clear_ipi(void)
> -{
> - if (IS_ENABLED(CONFIG_RISCV_SBI))
> - csr_clear(CSR_IP, IE_SIE);
> + if (ipi_ops && ipi_ops->ipi_inject)
> + ipi_ops->ipi_inject(cpumask_of(cpu));
> else
> - clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
> + pr_warn("SMP: IPI inject method not available\n");
> }
>
> void handle_IPI(struct pt_regs *regs)
> @@ -131,7 +136,7 @@ void handle_IPI(struct pt_regs *regs)
>
> irq_enter();
>
> - clear_ipi();
> + riscv_clear_ipi();
>
> while (true) {
> unsigned long ops;
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 4e9922790f6e..5fe849791bf0 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -147,8 +147,7 @@ asmlinkage __visible void smp_callin(void)
> {
> struct mm_struct *mm = &init_mm;
>
> - if (!IS_ENABLED(CONFIG_RISCV_SBI))
> - clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
> + riscv_clear_ipi();
>
> /* All kernel threads share the same mm context. */
> mmgrab(mm);

I don't really care that much, though, so:

Reviewed-by: Palmer Dabbelt <[email protected]>

2020-08-05 01:51:26

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v6 4/4] dt-bindings: timer: Add CLINT bindings

On Fri, 24 Jul 2020 00:18:22 PDT (-0700), Anup Patel wrote:
> We add DT bindings documentation for CLINT device.
>
> Signed-off-by: Anup Patel <[email protected]>
> Reviewed-by: Palmer Dabbelt <[email protected]>
> Tested-by: Emil Renner Berhing <[email protected]>
> ---
> .../bindings/timer/sifive,clint.yaml | 60 +++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.yaml
>
> diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
> new file mode 100644
> index 000000000000..2a0e9cd9fbcf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/sifive,clint.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SiFive Core Local Interruptor
> +
> +maintainers:
> + - Palmer Dabbelt <[email protected]>
> + - Anup Patel <[email protected]>
> +
> +description:
> + SiFive (and other RISC-V) SOCs include an implementation of the SiFive
> + Core Local Interruptor (CLINT) for M-mode timer and M-mode inter-processor
> + interrupts. It directly connects to the timer and inter-processor interrupt
> + lines of various HARTs (or CPUs) so RISC-V per-HART (or per-CPU) local
> + interrupt controller is the parent interrupt controller for CLINT device.
> + The clock frequency of CLINT is specified via "timebase-frequency" DT
> + property of "/cpus" DT node. The "timebase-frequency" DT property is
> + described in Documentation/devicetree/bindings/riscv/cpus.yaml
> +
> +properties:
> + compatible:
> + items:
> + - const: sifive,fu540-c000-clint
> + - const: sifive,clint0
> +
> + description:
> + Should be "sifive,<chip>-clint" and "sifive,clint<version>".
> + Supported compatible strings are -
> + "sifive,fu540-c000-clint" for the SiFive CLINT v0 as integrated
> + onto the SiFive FU540 chip, and "sifive,clint0" for the SiFive
> + CLINT v0 IP block with no chip integration tweaks.
> + Please refer to sifive-blocks-ip-versioning.txt for details

Perfect! I was going to mention that we forgot to define the
"sifive,${name}${version}" scheme but I guess I just forgot that we did define
it ;)

> +
> + reg:
> + maxItems: 1
> +
> + interrupts-extended:
> + minItems: 1
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts-extended
> +
> +examples:
> + - |
> + timer@2000000 {
> + compatible = "sifive,fu540-c000-clint", "sifive,clint0";
> + interrupts-extended = <&cpu1intc 3 &cpu1intc 7
> + &cpu2intc 3 &cpu2intc 7
> + &cpu3intc 3 &cpu3intc 7
> + &cpu4intc 3 &cpu4intc 7>;
> + reg = <0x2000000 0x10000>;
> + };
> +...

Reviewed-by: Palmer Dabbelt <[email protected]>

2020-08-05 09:39:53

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH v6 0/4] Dedicated CLINT timer driver

On Wed, Aug 5, 2020 at 7:17 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Fri, 24 Jul 2020 00:18:18 PDT (-0700), Anup Patel wrote:
> > The current RISC-V timer driver is convoluted and implements two
> > distinct timers:
> > 1. S-mode timer: This is for Linux RISC-V S-mode with MMU. The
> > clocksource is implemented using TIME CSR and clockevent device
> > is implemented using SBI Timer calls.
> > 2. M-mode timer: This is for Linux RISC-V M-mode without MMU. The
> > clocksource is implemented using CLINT MMIO time register and
> > clockevent device is implemented using CLINT MMIO timecmp registers.
> >
> > This patchset removes clint related code from RISC-V timer driver and
> > arch/riscv directory. Instead, the series adds a dedicated MMIO based
> > CLINT driver under drivers/clocksource directory which can be used by
> > Linux RISC-V M-mode (i.e NoMMU Linux RISC-V).
> >
> > The patchset is based up Linux-5.8-rc6 and can be found at riscv_clint_v6
> > branch of: https://github.com/avpatel/linux.git
> >
> > This series is tested on:
> > 1. QEMU RV64 virt machine using Linux RISC-V S-mode
> > 2. QEMU RV32 virt machine using Linux RISC-V S-mode
> > 3. QEMU RV64 virt machine using Linux RISC-V M-mode (i.e. NoMMU)
> >
> > Changes since v5:
> > - Fixed order of compatible strings in PATCH4
> > - Added "additionalProperties: false" in PATCH4
> > - Fixed register space size for example DT node in PATCH4
> >
> > Changes since v4:
> > - Rebased series on Linux-5.8-rc6
> > - Updated Kconfig option as suggested by Daniel in PATCH2
> > - Removed per-CPU registered flag in PATCH2
> > - Addressed nit comments from Atish in PATCH2
> >
> > Changes since v3:
> > - Updated commit description of PATCH2
> > - Use clint_get_cycles64() in clint_rdtime() of PATCH2
> > - Call clockevents_config_and_register() only once for each CPU in
> > clint_timer_starting_cpu of PATCH2
> > - Select CLINT timer driver from platform Kconfig in PATCH3
> > - Fixed 'make dt_binding_check' for PATCH4
> >
> > Changes since v2:
> > - Rebased series on Linux-5.8-rc5
> > - Squashed PATCH3 onto PATCH2 to preserve GIT bisectability
> > - Moved PATCH4 before PATCH2 to preserve GIT bisectability
> > - Replaced CLINT dt-bindings text document with YAML schema
> > - Use SiFive CLINT compatible string as per SiFive IP block versioning
> >
> > Changes since v1:
> > - Rebased series on Linux-5.8-rc2
> > - Added pr_warn() for case where ipi_ops not available in PATCH1
> > - Updated ipi_inject() prototype to use "struct cpumask *" in PATCH1
> > - Updated CLINT_TIMER kconfig option to depend on RISCV_M_MODE in PATCH4
> > - Added riscv,clint0 compatible string in DT bindings document
> >
> > Anup Patel (4):
> > RISC-V: Add mechanism to provide custom IPI operations
> > clocksource/drivers: Add CLINT timer driver
> > RISC-V: Remove CLINT related code from timer and arch
> > dt-bindings: timer: Add CLINT bindings
> >
> > .../bindings/timer/sifive,clint.yaml | 60 +++++
> > arch/riscv/Kconfig | 2 +-
> > arch/riscv/Kconfig.socs | 2 +
> > arch/riscv/configs/nommu_virt_defconfig | 7 +-
> > arch/riscv/include/asm/clint.h | 39 ---
> > arch/riscv/include/asm/smp.h | 19 ++
> > arch/riscv/include/asm/timex.h | 28 +--
> > arch/riscv/kernel/Makefile | 2 +-
> > arch/riscv/kernel/clint.c | 44 ----
> > arch/riscv/kernel/sbi.c | 14 ++
> > arch/riscv/kernel/setup.c | 2 -
> > arch/riscv/kernel/smp.c | 44 ++--
> > arch/riscv/kernel/smpboot.c | 4 +-
> > drivers/clocksource/Kconfig | 12 +-
> > drivers/clocksource/Makefile | 1 +
> > drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++
> > drivers/clocksource/timer-riscv.c | 17 +-
> > include/linux/cpuhotplug.h | 1 +
> > 18 files changed, 371 insertions(+), 153 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.yaml
> > delete mode 100644 arch/riscv/include/asm/clint.h
> > delete mode 100644 arch/riscv/kernel/clint.c
> > create mode 100644 drivers/clocksource/timer-clint.c
>
> Thanks, this is way cleaner. Patchwork is still broken but IIRC we reached
> consensus on these. I'm not going to include these in my first 5.9 PR, as I
> want to get that out tomorrow to avoid more merge conflicts, but assuming
> there's reviews from the other maintainers I'd like to take this for my second
> 5.9 merge window PR.
>
> Assuming you've been collecting reviews and acks, do you mind posting another
> version with them? If not I have some scripts to dig them out, so it's not a
> big deal.

Most of the Reviewed-by and Ack-by are already there, except yours and
Rob Herring's Reviewed-by. I will post v7 based on Linux-5.9-rc1

Regards,
Anup