[PATCH 0/5] Dedicated CLINT timer driver

[PATCH 0/5] Dedicated CLINT timer driver

[Anup Patel]

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.7-rc6 and can be found at riscv_clint_v1
branch of: https://github.com/avpatel/linux.git

These patches require "New RISC-V Local Interrupt Controller Driver"
which can be found at at riscv_intc_v5 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)

Anup Patel (5):
  RISC-V: Add mechanism to provide custom IPI operations
  RISC-V: Remove CLINT related code
  clocksource/drivers/timer-riscv: Remove MMIO related stuff
  clocksource/drivers: Add CLINT timer driver
  dt-bindings: timer: Add CLINT bindings

 .../bindings/timer/sifive,clint.txt           |  33 +++
 arch/riscv/Kconfig                            |   2 +-
 arch/riscv/include/asm/clint.h                |  39 ---
 arch/riscv/include/asm/smp.h                  |  11 +
 arch/riscv/include/asm/timex.h                |  28 +--
 arch/riscv/kernel/Makefile                    |   2 +-
 arch/riscv/kernel/clint.c                     |  44 ----
 arch/riscv/kernel/setup.c                     |   2 -
 arch/riscv/kernel/smp.c                       |  53 ++--
 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 +
 15 files changed, 330 insertions(+), 145 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.txt
 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

[PATCH 1/5] RISC-V: Add mechanism to provide custom IPI operations

[Anup Patel]

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 <anup.patel@wdc.com>
---
 arch/riscv/include/asm/smp.h | 11 ++++++++
 arch/riscv/kernel/smp.c      | 52 ++++++++++++++++++++++++------------
 arch/riscv/kernel/smpboot.c  |  3 +--
 3 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 40bb1c15a731..ad0601260cb1 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -40,6 +40,17 @@ 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);
 
+struct riscv_ipi_ops {
+	void (*ipi_inject)(const unsigned long *hart_mask);
+	void (*ipi_clear)(void);
+};
+
+/* 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.
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index b1d4f452f843..8375cc5970f6 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -84,6 +84,35 @@ static void ipi_stop(void)
 		wait_for_interrupt();
 }
 
+#if IS_ENABLED(CONFIG_RISCV_SBI)
+static void clear_ipi(void)
+{
+	csr_clear(CSR_IP, IE_SIE);
+}
+
+static struct riscv_ipi_ops sbi_ipi_ops = {
+	.ipi_inject = sbi_send_ipi,
+	.ipi_clear = clear_ipi,
+};
+
+static struct riscv_ipi_ops *ipi_ops = &sbi_ipi_ops;
+#else
+static struct riscv_ipi_ops *ipi_ops;
+#endif
+
+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();
+}
+EXPORT_SYMBOL_GPL(riscv_clear_ipi);
+
 static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
 {
 	struct cpumask hartid_mask;
@@ -95,10 +124,9 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
 	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));
-	else
-		clint_send_ipi_mask(mask);
+
+	if (ipi_ops)
+		ipi_ops->ipi_inject(cpumask_bits(&hartid_mask));
 }
 
 static void send_ipi_single(int cpu, enum ipi_message_type op)
@@ -109,18 +137,8 @@ static void send_ipi_single(int cpu, enum ipi_message_type op)
 	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);
-	else
-		clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
+	if (ipi_ops)
+		ipi_ops->ipi_inject(cpumask_bits(cpumask_of(hartid)));
 }
 
 void handle_IPI(struct pt_regs *regs)
@@ -131,7 +149,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

[PATCH 2/5] RISC-V: Remove CLINT related code

[Anup Patel]

We will be having separate CLINT timer driver which will also
provide CLINT based IPI operations so let's remove CLINT related
code from arch/riscv directory.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 arch/riscv/include/asm/clint.h | 39 ------------------------------
 arch/riscv/kernel/Makefile     |  2 +-
 arch/riscv/kernel/clint.c      | 44 ----------------------------------
 arch/riscv/kernel/setup.c      |  2 --
 arch/riscv/kernel/smp.c        |  1 -
 arch/riscv/kernel/smpboot.c    |  1 -
 6 files changed, 1 insertion(+), 88 deletions(-)
 delete mode 100644 arch/riscv/include/asm/clint.h
 delete mode 100644 arch/riscv/kernel/clint.c

diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
deleted file mode 100644
index a279b17a6aad..000000000000
--- a/arch/riscv/include/asm/clint.h
+++ /dev/null
@@ -1,39 +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
-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/kernel/Makefile b/arch/riscv/kernel/Makefile
index d8bbd3207100..529cda705cfe 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 3647980d14c3..000000000000
--- a/arch/riscv/kernel/clint.c
+++ /dev/null
@@ -1,44 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (c) 2019 Christoph Hellwig.
- */
-
-#include <linux/io.h>
-#include <linux/of_address.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
- * 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;
-
-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(boot_cpu_hartid);
-}
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 145128a7e560..b07a583bf53b 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>
@@ -76,7 +75,6 @@ void __init setup_arch(char **cmdline_p)
 	setup_bootmem();
 	paging_init();
 	unflatten_device_tree();
-	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 8375cc5970f6..8a23f1eb5400 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>
-- 
2.25.1

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

[Anup Patel]

Right now the RISC-V timer is convoluted to support:
1. Linux RISC-V S-mode (with MMU) where it will use TIME CSR
   for clocksource and SBI timer calls for clockevent device.
2. Linux RISC-V M-mode (without MMU) where it will use CLINT
   MMIO counter register for clocksource and CLINT MMIO compare
   register for clockevent device.

This patch removes MMIO related stuff from RISC-V timer driver
so that we can have a separate CLINT timer driver.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 arch/riscv/Kconfig                |  2 +-
 arch/riscv/include/asm/timex.h    | 28 +++++++---------------------
 drivers/clocksource/Kconfig       |  2 +-
 drivers/clocksource/timer-riscv.c | 17 ++---------------
 4 files changed, 11 insertions(+), 38 deletions(-)

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

[PATCH 4/5] clocksource/drivers: Add CLINT timer driver

[Anup Patel]

The TIME CSR and SBI calls are not available in RISC-V M-mode so we
add CLINT driver for Linux RISC-V M-mode (i.e. RISC-V NoMMU kernel).

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 drivers/clocksource/Kconfig       |  10 ++
 drivers/clocksource/Makefile      |   1 +
 drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++++++++++++++
 include/linux/cpuhotplug.h        |   1 +
 4 files changed, 238 insertions(+)
 create mode 100644 drivers/clocksource/timer-clint.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 21950d9e3e9d..ea97bf0eb09f 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -659,6 +659,16 @@ 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 "Timer for the RISC-V platform"
+	depends on GENERIC_SCHED_CLOCK && RISCV
+	default y
+	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 641ba5383ab5..dca308b5ff98 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -86,6 +86,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..7fc4f145da65
--- /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/irqchip/irq-riscv-intc.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+#include <linux/smp.h>
+
+#define CLINT_IPI_OFF		0
+#define CLINT_TIME_CMP_OFF	0x4000
+#define CLINT_TIME_VAL_OFF	0xbff8
+
+/* CLINT manages IPI and Timer for RISC-V M-mode  */
+static u32 __iomem *clint_ipi_base;
+static u64 __iomem *clint_time_cmp;
+static u64 __iomem *clint_time_val;
+static unsigned long clint_freq;
+static unsigned int clint_irq;
+
+static void clint_send_ipi(const unsigned long *hart_mask)
+{
+	u32 hartid;
+
+	for_each_set_bit(hartid, hart_mask, NR_CPUS)
+		writel(1, clint_ipi_base + hartid);
+}
+
+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_time_val)
+#else
+#define clint_get_cycles()	readl_relaxed(clint_time_val)
+#define clint_get_cycles_hi()	readl_relaxed(((u32 *)clint_time_val) + 1)
+#endif
+
+#ifdef CONFIG_64BIT
+static u64 clint_get_cycles64(void)
+{
+	return clint_get_cycles();
+}
+#else /* CONFIG_64BIT */
+static u64 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 int clint_clock_next_event(unsigned long delta,
+				   struct clock_event_device *ce)
+{
+	void __iomem *r = clint_time_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 u64 clint_rdtime(struct clocksource *cs)
+{
+	return readq_relaxed(clint_time_val);
+}
+
+static u64 notrace clint_sched_clock(void)
+{
+	return readq_relaxed(clint_time_val);
+}
+
+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_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_freq, 200, ULONG_MAX);
+
+	enable_percpu_irq(clint_irq, irq_get_trigger_type(clint_irq));
+	return 0;
+}
+
+static int clint_timer_dying_cpu(unsigned int cpu)
+{
+	disable_percpu_irq(clint_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_info("%pOFP: invalid irq %d (hwirq %d)\n",
+				np, i, oirq.args[0]);
+			return 0;
+		}
+	}
+
+	oirq.np = riscv_of_intc_domain_node();
+	oirq.args_count = 1;
+	oirq.args[0] = RV_IRQ_TIMER;
+	clint_irq = irq_create_of_mapping(&oirq);
+	if (!clint_irq) {
+		pr_err("%pOFP: could not map hwirq %d\n", np, RV_IRQ_TIMER);
+		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_time_cmp = base + CLINT_TIME_CMP_OFF;
+	clint_time_val = base + CLINT_TIME_VAL_OFF;
+	clint_freq = riscv_timebase;
+
+	pr_info("%pOFP: timer running at %ld Hz\n", np, clint_freq);
+
+	rc = clocksource_register_hz(&clint_clocksource, clint_freq);
+	if (rc) {
+		iounmap(base);
+		pr_err("%pOFP: clocksource register failed [%d]\n", np, rc);
+		return rc;
+	}
+
+	sched_clock_register(clint_sched_clock, 64, clint_freq);
+
+	rc = request_percpu_irq(clint_irq, clint_timer_interrupt,
+				 "clint-timer", &clint_clock_event);
+	if (rc) {
+		iounmap(base);
+		pr_err("registering percpu irq failed [%d]\n", rc);
+		return rc;
+	}
+
+	rc = cpuhp_setup_state(CPUHP_AP_CLINT_TIMER_STARTING,
+				"clockevents/clint/timer:starting",
+				clint_timer_starting_cpu,
+				clint_timer_dying_cpu);
+	if (rc) {
+		free_irq(clint_irq, &clint_clock_event);
+		iounmap(base);
+		pr_err("%pOFP: cpuhp setup state failed [%d]\n", np, rc);
+		return rc;
+	}
+
+	riscv_set_ipi_ops(&clint_ipi_ops);
+	clint_clear_ipi();
+
+	return 0;
+}
+
+TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
+TIMER_OF_DECLARE(clint_timer1, "sifive,clint-1.0.0", clint_timer_init_dt);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 57b1f8f777d9..52552492c2f2 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

[PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Anup Patel]

We add DT bindings documentation for CLINT device.

Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 .../bindings/timer/sifive,clint.txt           | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.txt

diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.txt b/Documentation/devicetree/bindings/timer/sifive,clint.txt
new file mode 100644
index 000000000000..cae2dad1223a
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/sifive,clint.txt
@@ -0,0 +1,33 @@
+SiFive Core Local Interruptor (CLINT)
+-------------------------------------
+
+SiFive (and other RISC-V) SOCs include an implementation of the SiFive Core
+Local Interruptor (CLINT) for M-mode timer and 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
+
+Required properties:
+- compatible : "sifive,clint-1.0.0" and a string identifying the actual
+  detailed implementation in case that specific bugs need to be worked around.
+- reg : Should contain 1 register range (address and length).
+- interrupts-extended : Specifies which HARTs (or CPUs) are connected to
+  the CLINT.  Each node pointed to should be a riscv,cpu-intc node, which
+  has a riscv node as parent.
+
+Example:
+
+	clint@2000000 {
+		compatible = "sifive,clint-1.0.0", "sifive,fu540-c000-clint";
+		interrupts-extended = <
+			&cpu1-intc 3 &cpu1-intc 7
+			&cpu2-intc 3 &cpu2-intc 7
+			&cpu3-intc 3 &cpu3-intc 7
+			&cpu4-intc 3 &cpu4-intc 7>;
+		reg = <0x2000000 0x4000000>;
+	};
-- 
2.25.1

Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Sean Anderson]

On 5/21/20 9:45 AM, Anup Patel wrote:
> We add DT bindings documentation for CLINT device.
> 
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  .../bindings/timer/sifive,clint.txt           | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.txt
> 
> diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.txt b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> new file mode 100644
> index 000000000000..cae2dad1223a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> @@ -0,0 +1,33 @@
> +SiFive Core Local Interruptor (CLINT)
> +-------------------------------------
> +
> +SiFive (and other RISC-V) SOCs include an implementation of the SiFive Core
> +Local Interruptor (CLINT) for M-mode timer and 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
> +
> +Required properties:
> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
> +  detailed implementation in case that specific bugs need to be worked around.

Should the "riscv,clint0" compatible string be documented here? This
peripheral is not really specific to sifive, as it is present in most
rocket-chip cores.

> +- reg : Should contain 1 register range (address and length).
> +- interrupts-extended : Specifies which HARTs (or CPUs) are connected to
> +  the CLINT.  Each node pointed to should be a riscv,cpu-intc node, which
> +  has a riscv node as parent.
> +
> +Example:
> +
> +	clint@2000000 {
> +		compatible = "sifive,clint-1.0.0", "sifive,fu540-c000-clint";
> +		interrupts-extended = <
> +			&cpu1-intc 3 &cpu1-intc 7
> +			&cpu2-intc 3 &cpu2-intc 7
> +			&cpu3-intc 3 &cpu3-intc 7
> +			&cpu4-intc 3 &cpu4-intc 7>;
> +		reg = <0x2000000 0x4000000>;
> +	};
> 

--Sean

Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Anup Patel]

On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> On 5/21/20 9:45 AM, Anup Patel wrote:
> > We add DT bindings documentation for CLINT device.
> >
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >  .../bindings/timer/sifive,clint.txt           | 33 +++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.txt
> >
> > diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.txt b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> > new file mode 100644
> > index 000000000000..cae2dad1223a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> > @@ -0,0 +1,33 @@
> > +SiFive Core Local Interruptor (CLINT)
> > +-------------------------------------
> > +
> > +SiFive (and other RISC-V) SOCs include an implementation of the SiFive Core
> > +Local Interruptor (CLINT) for M-mode timer and 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
> > +
> > +Required properties:
> > +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
> > +  detailed implementation in case that specific bugs need to be worked around.
>
> Should the "riscv,clint0" compatible string be documented here? This

Yes, I forgot to add this compatible string. I will add in v2.

> peripheral is not really specific to sifive, as it is present in most
> rocket-chip cores.

I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
FPGAs but this IP is only documented as part of SiFive FU540 SOC.
(Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)

The RISC-V foundation should host the CLINT spec independently
under https://github.com/riscv and make CLINT spec totally open.

For now, I have documented it just like PLIC DT bindings found at:
Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt

If RISC-V maintainers agree then I will document it as "RISC-V CLINT".

@Palmer ?? @Paul ??

>
> > +- reg : Should contain 1 register range (address and length).
> > +- interrupts-extended : Specifies which HARTs (or CPUs) are connected to
> > +  the CLINT.  Each node pointed to should be a riscv,cpu-intc node, which
> > +  has a riscv node as parent.
> > +
> > +Example:
> > +
> > +     clint@2000000 {
> > +             compatible = "sifive,clint-1.0.0", "sifive,fu540-c000-clint";
> > +             interrupts-extended = <
> > +                     &cpu1-intc 3 &cpu1-intc 7
> > +                     &cpu2-intc 3 &cpu2-intc 7
> > +                     &cpu3-intc 3 &cpu3-intc 7
> > +                     &cpu4-intc 3 &cpu4-intc 7>;
> > +             reg = <0x2000000 0x4000000>;
> > +     };
> >
>
> --Sean

Regards,
Anup

Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Sean Anderson]

On 5/22/20 1:54 AM, Anup Patel wrote:
> On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> On 5/21/20 9:45 AM, Anup Patel wrote:
>>> +Required properties:
>>> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
>>> +  detailed implementation in case that specific bugs need to be worked around.
>>
>> Should the "riscv,clint0" compatible string be documented here? This
> 
> Yes, I forgot to add this compatible string. I will add in v2.
> 
>> peripheral is not really specific to sifive, as it is present in most
>> rocket-chip cores.
> 
> I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
> FPGAs but this IP is only documented as part of SiFive FU540 SOC.
> (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
> 
> The RISC-V foundation should host the CLINT spec independently
> under https://github.com/riscv and make CLINT spec totally open.
> 
> For now, I have documented it just like PLIC DT bindings found at:
> Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt

The PLIC seems to have its own RISC-V-sponsored documentation [1] which
was split off from the older privileged specs. By your logic above,
should it be renamed to riscv,plic0.txt (with a corresponding change in
the documented compatible strings)?

[1] https://github.com/riscv/riscv-plic-spec

> 
> If RISC-V maintainers agree then I will document it as "RISC-V CLINT".
> 
> @Palmer ?? @Paul ??
> 
> Regards,
> Anup
> 

--Sean

Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Anup Patel]

On Fri, May 22, 2020 at 11:59 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> On 5/22/20 1:54 AM, Anup Patel wrote:
> > On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
> >>
> >> On 5/21/20 9:45 AM, Anup Patel wrote:
> >>> +Required properties:
> >>> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
> >>> +  detailed implementation in case that specific bugs need to be worked around.
> >>
> >> Should the "riscv,clint0" compatible string be documented here? This
> >
> > Yes, I forgot to add this compatible string. I will add in v2.
> >
> >> peripheral is not really specific to sifive, as it is present in most
> >> rocket-chip cores.
> >
> > I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
> > FPGAs but this IP is only documented as part of SiFive FU540 SOC.
> > (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
> >
> > The RISC-V foundation should host the CLINT spec independently
> > under https://github.com/riscv and make CLINT spec totally open.
> >
> > For now, I have documented it just like PLIC DT bindings found at:
> > Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
>
> The PLIC seems to have its own RISC-V-sponsored documentation [1] which
> was split off from the older privileged specs. By your logic above,
> should it be renamed to riscv,plic0.txt (with a corresponding change in
> the documented compatible strings)?
>
> [1] https://github.com/riscv/riscv-plic-spec

For PLIC bindings, we can certainly do the renaming because now
we have PLIC v1 specification hosted on RISC-V Foundation Github.

Regards,
Anup

Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Palmer Dabbelt]

On Thu, 21 May 2020 23:29:36 PDT (-0700), seanga2@gmail.com wrote:
> On 5/22/20 1:54 AM, Anup Patel wrote:
>> On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>>>
>>> On 5/21/20 9:45 AM, Anup Patel wrote:
>>>> +Required properties:
>>>> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
>>>> +  detailed implementation in case that specific bugs need to be worked around.
>>>
>>> Should the "riscv,clint0" compatible string be documented here? This
>> 
>> Yes, I forgot to add this compatible string. I will add in v2.
>> 
>>> peripheral is not really specific to sifive, as it is present in most
>>> rocket-chip cores.
>> 
>> I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
>> FPGAs but this IP is only documented as part of SiFive FU540 SOC.
>> (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
>> 
>> The RISC-V foundation should host the CLINT spec independently
>> under https://github.com/riscv and make CLINT spec totally open.
>> 
>> For now, I have documented it just like PLIC DT bindings found at:
>> Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
>
> The PLIC seems to have its own RISC-V-sponsored documentation [1] which
> was split off from the older privileged specs. By your logic above,
> should it be renamed to riscv,plic0.txt (with a corresponding change in
> the documented compatible strings)?
>
> [1] https://github.com/riscv/riscv-plic-spec

Let's propose tagging that PLIC spec as v1.0.0 in the platform spec group, but
I don't see a reason why that wouldn't be viable.  Assuming that's all OK, we
can start calling this a RISC-V PLIC (in addition to a SiFive PLIC, as they'll
be compatible).

>> 
>> If RISC-V maintainers agree then I will document it as "RISC-V CLINT".
>> 
>> @Palmer ?? @Paul ??

The CLINT is a SiFive spec.  It has open source RTL so it's been implemented in
other designs, but it's not a RISC-V spec.  The CLIC, which is a superset of
the CLINT, is a RISC-V spec.  IIRC it's not finished yet (it's the fast
interrupts task group), but presumably we should have a "riscv,clic-2.0.0" (or
whatever it ends up being called) compat string to go along with the
specification.

>> Regards,
>> Anup
>> 
>
> --Sean

Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Sean Anderson]

On 5/26/20 8:32 PM, Palmer Dabbelt wrote:
> On Thu, 21 May 2020 23:29:36 PDT (-0700), seanga2@gmail.com wrote:
>> On 5/22/20 1:54 AM, Anup Patel wrote:
>>> On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>>>>
>>>> On 5/21/20 9:45 AM, Anup Patel wrote:
>>>>> +Required properties:
>>>>> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
>>>>> +  detailed implementation in case that specific bugs need to be worked around.
>>>>
>>>> Should the "riscv,clint0" compatible string be documented here? This
>>>
>>> Yes, I forgot to add this compatible string. I will add in v2.
>>>
>>>> peripheral is not really specific to sifive, as it is present in most
>>>> rocket-chip cores.
>>>
>>> I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
>>> FPGAs but this IP is only documented as part of SiFive FU540 SOC.
>>> (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
>>>
>>> The RISC-V foundation should host the CLINT spec independently
>>> under https://github.com/riscv and make CLINT spec totally open.
>>>
>>> For now, I have documented it just like PLIC DT bindings found at:
>>> Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
>>
>> The PLIC seems to have its own RISC-V-sponsored documentation [1] which
>> was split off from the older privileged specs. By your logic above,
>> should it be renamed to riscv,plic0.txt (with a corresponding change in
>> the documented compatible strings)?
>>
>> [1] https://github.com/riscv/riscv-plic-spec
> 
> Let's propose tagging that PLIC spec as v1.0.0 in the platform spec group, but
> I don't see a reason why that wouldn't be viable.  Assuming that's all OK, we
> can start calling this a RISC-V PLIC (in addition to a SiFive PLIC, as they'll
> be compatible).

Is there a version anyewhere in that spec? I looked around a bit and
couldn't find one.

>>>
>>> If RISC-V maintainers agree then I will document it as "RISC-V CLINT".
>>>
>>> @Palmer ?? @Paul ??
> 
> The CLINT is a SiFive spec.  It has open source RTL so it's been implemented in
> other designs, but it's not a RISC-V spec.  The CLIC, which is a superset of
> the CLINT, is a RISC-V spec.  IIRC it's not finished yet (it's the fast
> interrupts task group), but presumably we should have a "riscv,clic-2.0.0" (or
> whatever it ends up being called) compat string to go along with the
> specification.

The rocket chip is a Chips Alliance project on github; presumably the
"proper" compatibility string would be something like
"chips-alliance,clint"? Alternatively, it is already referred to as
"riscv,clint0" in U-Boot, following the pattern of the plic.

--Sean

Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Rob Herring]

On Tue, May 26, 2020 at 05:32:30PM -0700, Palmer Dabbelt wrote:
> On Thu, 21 May 2020 23:29:36 PDT (-0700), seanga2@gmail.com wrote:
> > On 5/22/20 1:54 AM, Anup Patel wrote:
> > > On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
> > > > 
> > > > On 5/21/20 9:45 AM, Anup Patel wrote:
> > > > > +Required properties:
> > > > > +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
> > > > > +  detailed implementation in case that specific bugs need to be worked around.
> > > > 
> > > > Should the "riscv,clint0" compatible string be documented here? This
> > > 
> > > Yes, I forgot to add this compatible string. I will add in v2.
> > > 
> > > > peripheral is not really specific to sifive, as it is present in most
> > > > rocket-chip cores.
> > > 
> > > I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
> > > FPGAs but this IP is only documented as part of SiFive FU540 SOC.
> > > (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
> > > 
> > > The RISC-V foundation should host the CLINT spec independently
> > > under https://github.com/riscv and make CLINT spec totally open.
> > > 
> > > For now, I have documented it just like PLIC DT bindings found at:
> > > Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
> > 
> > The PLIC seems to have its own RISC-V-sponsored documentation [1] which
> > was split off from the older privileged specs. By your logic above,
> > should it be renamed to riscv,plic0.txt (with a corresponding change in
> > the documented compatible strings)?
> > 
> > [1] https://github.com/riscv/riscv-plic-spec
> 
> Let's propose tagging that PLIC spec as v1.0.0 in the platform spec group, but
> I don't see a reason why that wouldn't be viable.  Assuming that's all OK, we
> can start calling this a RISC-V PLIC (in addition to a SiFive PLIC, as they'll
> be compatible).
> 
> > > 
> > > If RISC-V maintainers agree then I will document it as "RISC-V CLINT".
> > > 
> > > @Palmer ?? @Paul ??
> 
> The CLINT is a SiFive spec.  It has open source RTL so it's been implemented in
> other designs, but it's not a RISC-V spec.  The CLIC, which is a superset of
> the CLINT, is a RISC-V spec.  IIRC it's not finished yet (it's the fast
> interrupts task group), but presumably we should have a "riscv,clic-2.0.0" (or
> whatever it ends up being called) compat string to go along with the
> specification.

Whatever you all decide on, note that "sifive,<block><num>" is a SiFive 
thing (as it is documented) and <num> corresponds to tag of the IP 
implmentation (at least it is supposed to). So you can't just copy that 
with 'riscv,<block><num>' unless you have the same IP versioning 
and update the documentation.

Using a spec version is fine, but not standalone. You need 
implementation specific compatible too because no one perfectly 
implements any spec and/or there details a spec may not cover.

Rob

Re: [PATCH 1/5] RISC-V: Add mechanism to provide custom IPI operations

[Palmer Dabbelt]

On Thu, 21 May 2020 06:45:40 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 <anup.patel@wdc.com>
> ---
>  arch/riscv/include/asm/smp.h | 11 ++++++++
>  arch/riscv/kernel/smp.c      | 52 ++++++++++++++++++++++++------------
>  arch/riscv/kernel/smpboot.c  |  3 +--
>  3 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
> index 40bb1c15a731..ad0601260cb1 100644
> --- a/arch/riscv/include/asm/smp.h
> +++ b/arch/riscv/include/asm/smp.h
> @@ -40,6 +40,17 @@ 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);
>
> +struct riscv_ipi_ops {
> +	void (*ipi_inject)(const unsigned long *hart_mask);
> +	void (*ipi_clear)(void);
> +};
> +
> +/* 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.
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index b1d4f452f843..8375cc5970f6 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -84,6 +84,35 @@ static void ipi_stop(void)
>  		wait_for_interrupt();
>  }
>
> +#if IS_ENABLED(CONFIG_RISCV_SBI)
> +static void clear_ipi(void)
> +{
> +	csr_clear(CSR_IP, IE_SIE);
> +}
> +
> +static struct riscv_ipi_ops sbi_ipi_ops = {
> +	.ipi_inject = sbi_send_ipi,
> +	.ipi_clear = clear_ipi,
> +};
> +
> +static struct riscv_ipi_ops *ipi_ops = &sbi_ipi_ops;
> +#else
> +static struct riscv_ipi_ops *ipi_ops;
> +#endif
> +
> +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();
> +}
> +EXPORT_SYMBOL_GPL(riscv_clear_ipi);

There should at least be a warning on SMP systems when an ipi_ops hasn't been
set, as otherwise the system will just hang.

> +
>  static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
>  {
>  	struct cpumask hartid_mask;
> @@ -95,10 +124,9 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
>  	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));
> -	else
> -		clint_send_ipi_mask(mask);
> +
> +	if (ipi_ops)
> +		ipi_ops->ipi_inject(cpumask_bits(&hartid_mask));
>  }
>
>  static void send_ipi_single(int cpu, enum ipi_message_type op)
> @@ -109,18 +137,8 @@ static void send_ipi_single(int cpu, enum ipi_message_type op)
>  	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);
> -	else
> -		clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
> +	if (ipi_ops)
> +		ipi_ops->ipi_inject(cpumask_bits(cpumask_of(hartid)));
>  }
>
>  void handle_IPI(struct pt_regs *regs)
> @@ -131,7 +149,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);

Re: [PATCH 2/5] RISC-V: Remove CLINT related code

[Palmer Dabbelt]

On Thu, 21 May 2020 06:45:41 PDT (-0700), Anup Patel wrote:
> We will be having separate CLINT timer driver which will also
> provide CLINT based IPI operations so let's remove CLINT related
> code from arch/riscv directory.

This will leave the system unbootable, which breaks bisecting.

>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  arch/riscv/include/asm/clint.h | 39 ------------------------------
>  arch/riscv/kernel/Makefile     |  2 +-
>  arch/riscv/kernel/clint.c      | 44 ----------------------------------
>  arch/riscv/kernel/setup.c      |  2 --
>  arch/riscv/kernel/smp.c        |  1 -
>  arch/riscv/kernel/smpboot.c    |  1 -
>  6 files changed, 1 insertion(+), 88 deletions(-)
>  delete mode 100644 arch/riscv/include/asm/clint.h
>  delete mode 100644 arch/riscv/kernel/clint.c
>
> diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
> deleted file mode 100644
> index a279b17a6aad..000000000000
> --- a/arch/riscv/include/asm/clint.h
> +++ /dev/null
> @@ -1,39 +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
> -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/kernel/Makefile b/arch/riscv/kernel/Makefile
> index d8bbd3207100..529cda705cfe 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 3647980d14c3..000000000000
> --- a/arch/riscv/kernel/clint.c
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Copyright (c) 2019 Christoph Hellwig.
> - */
> -
> -#include <linux/io.h>
> -#include <linux/of_address.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
> - * 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;
> -
> -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(boot_cpu_hartid);
> -}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 145128a7e560..b07a583bf53b 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>
> @@ -76,7 +75,6 @@ void __init setup_arch(char **cmdline_p)
>  	setup_bootmem();
>  	paging_init();
>  	unflatten_device_tree();
> -	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 8375cc5970f6..8a23f1eb5400 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>

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

[Palmer Dabbelt]

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

This one will also break bisecting for the K210.

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

Re: [PATCH 4/5] clocksource/drivers: Add CLINT timer driver

[Palmer Dabbelt]

On Thu, 21 May 2020 06:45:43 PDT (-0700), Anup Patel wrote:
> The TIME CSR and SBI calls are not available in RISC-V M-mode so we
> add CLINT driver for Linux RISC-V M-mode (i.e. RISC-V NoMMU kernel).
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  drivers/clocksource/Kconfig       |  10 ++
>  drivers/clocksource/Makefile      |   1 +
>  drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++++++++++++++
>  include/linux/cpuhotplug.h        |   1 +
>  4 files changed, 238 insertions(+)
>  create mode 100644 drivers/clocksource/timer-clint.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 21950d9e3e9d..ea97bf0eb09f 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -659,6 +659,16 @@ 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 "Timer for the RISC-V platform"
> +	depends on GENERIC_SCHED_CLOCK && RISCV

Presumably this also depends on RISCV_M_MODE?

> +	default y
> +	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 641ba5383ab5..dca308b5ff98 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -86,6 +86,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..7fc4f145da65
> --- /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/irqchip/irq-riscv-intc.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_irq.h>
> +#include <linux/smp.h>
> +
> +#define CLINT_IPI_OFF		0
> +#define CLINT_TIME_CMP_OFF	0x4000
> +#define CLINT_TIME_VAL_OFF	0xbff8
> +
> +/* CLINT manages IPI and Timer for RISC-V M-mode  */
> +static u32 __iomem *clint_ipi_base;

It seems odd to have IPIs in the timer driver.  I know the CLINT handles both
of them, but these really feel like two separate drivers.

> +static u64 __iomem *clint_time_cmp;
> +static u64 __iomem *clint_time_val;
> +static unsigned long clint_freq;
> +static unsigned int clint_irq;
> +
> +static void clint_send_ipi(const unsigned long *hart_mask)
> +{
> +	u32 hartid;
> +
> +	for_each_set_bit(hartid, hart_mask, NR_CPUS)
> +		writel(1, clint_ipi_base + hartid);
> +}
> +
> +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_time_val)
> +#else
> +#define clint_get_cycles()	readl_relaxed(clint_time_val)
> +#define clint_get_cycles_hi()	readl_relaxed(((u32 *)clint_time_val) + 1)
> +#endif
> +
> +#ifdef CONFIG_64BIT
> +static u64 clint_get_cycles64(void)
> +{
> +	return clint_get_cycles();
> +}
> +#else /* CONFIG_64BIT */
> +static u64 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 int clint_clock_next_event(unsigned long delta,
> +				   struct clock_event_device *ce)
> +{
> +	void __iomem *r = clint_time_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 u64 clint_rdtime(struct clocksource *cs)
> +{
> +	return readq_relaxed(clint_time_val);
> +}
> +
> +static u64 notrace clint_sched_clock(void)
> +{
> +	return readq_relaxed(clint_time_val);
> +}
> +
> +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_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_freq, 200, ULONG_MAX);
> +
> +	enable_percpu_irq(clint_irq, irq_get_trigger_type(clint_irq));
> +	return 0;
> +}
> +
> +static int clint_timer_dying_cpu(unsigned int cpu)
> +{
> +	disable_percpu_irq(clint_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_info("%pOFP: invalid irq %d (hwirq %d)\n",
> +				np, i, oirq.args[0]);
> +			return 0;
> +		}
> +	}
> +
> +	oirq.np = riscv_of_intc_domain_node();
> +	oirq.args_count = 1;
> +	oirq.args[0] = RV_IRQ_TIMER;
> +	clint_irq = irq_create_of_mapping(&oirq);
> +	if (!clint_irq) {
> +		pr_err("%pOFP: could not map hwirq %d\n", np, RV_IRQ_TIMER);
> +		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_time_cmp = base + CLINT_TIME_CMP_OFF;
> +	clint_time_val = base + CLINT_TIME_VAL_OFF;
> +	clint_freq = riscv_timebase;
> +
> +	pr_info("%pOFP: timer running at %ld Hz\n", np, clint_freq);
> +
> +	rc = clocksource_register_hz(&clint_clocksource, clint_freq);
> +	if (rc) {
> +		iounmap(base);
> +		pr_err("%pOFP: clocksource register failed [%d]\n", np, rc);
> +		return rc;
> +	}
> +
> +	sched_clock_register(clint_sched_clock, 64, clint_freq);
> +
> +	rc = request_percpu_irq(clint_irq, clint_timer_interrupt,
> +				 "clint-timer", &clint_clock_event);
> +	if (rc) {
> +		iounmap(base);
> +		pr_err("registering percpu irq failed [%d]\n", rc);
> +		return rc;
> +	}
> +
> +	rc = cpuhp_setup_state(CPUHP_AP_CLINT_TIMER_STARTING,
> +				"clockevents/clint/timer:starting",
> +				clint_timer_starting_cpu,
> +				clint_timer_dying_cpu);
> +	if (rc) {
> +		free_irq(clint_irq, &clint_clock_event);
> +		iounmap(base);
> +		pr_err("%pOFP: cpuhp setup state failed [%d]\n", np, rc);
> +		return rc;
> +	}
> +
> +	riscv_set_ipi_ops(&clint_ipi_ops);
> +	clint_clear_ipi();
> +
> +	return 0;
> +}
> +
> +TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
> +TIMER_OF_DECLARE(clint_timer1, "sifive,clint-1.0.0", clint_timer_init_dt);
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 57b1f8f777d9..52552492c2f2 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,

Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Palmer Dabbelt]

On Thu, 21 May 2020 06:45:44 PDT (-0700), Anup Patel wrote:
> We add DT bindings documentation for CLINT device.
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  .../bindings/timer/sifive,clint.txt           | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.txt b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> new file mode 100644
> index 000000000000..cae2dad1223a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> @@ -0,0 +1,33 @@
> +SiFive Core Local Interruptor (CLINT)
> +-------------------------------------
> +
> +SiFive (and other RISC-V) SOCs include an implementation of the SiFive Core
> +Local Interruptor (CLINT) for M-mode timer and 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
> +
> +Required properties:
> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
> +  detailed implementation in case that specific bugs need to be worked around.
> +- reg : Should contain 1 register range (address and length).
> +- interrupts-extended : Specifies which HARTs (or CPUs) are connected to
> +  the CLINT.  Each node pointed to should be a riscv,cpu-intc node, which
> +  has a riscv node as parent.
> +
> +Example:
> +
> +	clint@2000000 {
> +		compatible = "sifive,clint-1.0.0", "sifive,fu540-c000-clint";
> +		interrupts-extended = <
> +			&cpu1-intc 3 &cpu1-intc 7
> +			&cpu2-intc 3 &cpu2-intc 7
> +			&cpu3-intc 3 &cpu3-intc 7
> +			&cpu4-intc 3 &cpu4-intc 7>;
> +		reg = <0x2000000 0x4000000>;
> +	};

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>

Re: [PATCH 2/5] RISC-V: Remove CLINT related code

[Anup Patel]

On Fri, Jun 5, 2020 at 2:10 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 21 May 2020 06:45:41 PDT (-0700), Anup Patel wrote:
> > We will be having separate CLINT timer driver which will also
> > provide CLINT based IPI operations so let's remove CLINT related
> > code from arch/riscv directory.
>
> This will leave the system unbootable, which breaks bisecting.

This only affects the NoMMU kernel where userspace FPDPIC work
is still in-progress so NoMMU kernel is anyway not 100% complete
without upstreamed userspace support.

Are you suggesting to squash PATCH2, PATCH3, and PATCH4 for
preserving bistability ?

Regards,
Anup


>
> >
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >  arch/riscv/include/asm/clint.h | 39 ------------------------------
> >  arch/riscv/kernel/Makefile     |  2 +-
> >  arch/riscv/kernel/clint.c      | 44 ----------------------------------
> >  arch/riscv/kernel/setup.c      |  2 --
> >  arch/riscv/kernel/smp.c        |  1 -
> >  arch/riscv/kernel/smpboot.c    |  1 -
> >  6 files changed, 1 insertion(+), 88 deletions(-)
> >  delete mode 100644 arch/riscv/include/asm/clint.h
> >  delete mode 100644 arch/riscv/kernel/clint.c
> >
> > diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
> > deleted file mode 100644
> > index a279b17a6aad..000000000000
> > --- a/arch/riscv/include/asm/clint.h
> > +++ /dev/null
> > @@ -1,39 +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
> > -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/kernel/Makefile b/arch/riscv/kernel/Makefile
> > index d8bbd3207100..529cda705cfe 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 3647980d14c3..000000000000
> > --- a/arch/riscv/kernel/clint.c
> > +++ /dev/null
> > @@ -1,44 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -/*
> > - * Copyright (c) 2019 Christoph Hellwig.
> > - */
> > -
> > -#include <linux/io.h>
> > -#include <linux/of_address.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
> > - * 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;
> > -
> > -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(boot_cpu_hartid);
> > -}
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index 145128a7e560..b07a583bf53b 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>
> > @@ -76,7 +75,6 @@ void __init setup_arch(char **cmdline_p)
> >       setup_bootmem();
> >       paging_init();
> >       unflatten_device_tree();
> > -     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 8375cc5970f6..8a23f1eb5400 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>

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

[Anup Patel]

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

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

Regards,
Anup

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

Re: [PATCH 4/5] clocksource/drivers: Add CLINT timer driver

[Anup Patel]

On Fri, Jun 5, 2020 at 2:10 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 21 May 2020 06:45:43 PDT (-0700), Anup Patel wrote:
> > The TIME CSR and SBI calls are not available in RISC-V M-mode so we
> > add CLINT driver for Linux RISC-V M-mode (i.e. RISC-V NoMMU kernel).
> >
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > ---
> >  drivers/clocksource/Kconfig       |  10 ++
> >  drivers/clocksource/Makefile      |   1 +
> >  drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++++++++++++++
> >  include/linux/cpuhotplug.h        |   1 +
> >  4 files changed, 238 insertions(+)
> >  create mode 100644 drivers/clocksource/timer-clint.c
> >
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 21950d9e3e9d..ea97bf0eb09f 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -659,6 +659,16 @@ 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 "Timer for the RISC-V platform"
> > +     depends on GENERIC_SCHED_CLOCK && RISCV
>
> Presumably this also depends on RISCV_M_MODE?

Ahh, good catch. I will update in v2.

>
> > +     default y
> > +     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 641ba5383ab5..dca308b5ff98 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -86,6 +86,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..7fc4f145da65
> > --- /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/irqchip/irq-riscv-intc.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/smp.h>
> > +
> > +#define CLINT_IPI_OFF                0
> > +#define CLINT_TIME_CMP_OFF   0x4000
> > +#define CLINT_TIME_VAL_OFF   0xbff8
> > +
> > +/* CLINT manages IPI and Timer for RISC-V M-mode  */
> > +static u32 __iomem *clint_ipi_base;
>
> It seems odd to have IPIs in the timer driver.  I know the CLINT handles both
> of them, but these really feel like two separate drivers.

AFAIK, there are no dedicated APIs in the kernel/irq subsystem for
IPI injections. Every architecture have their own way of registering
IPI injection mechanism. In ARM/ARM64, the arch code provides
set_smp_cross_call() API which drivers use to register IPI injections
mechanism. The PATCH1 of this series adds riscv_set_ipi_ops()
for this purpose.

Generally for most architectures (e.g. ARM, ARM64, MIPS, etc),
the IPI injections mechanism is registered from the irqchip driver
but for Linux RISC-V NoMMU the  IPI injection mechanism is
part of CLINT hence part of this CLINT driver.

Regards,
Anup

>
> > +static u64 __iomem *clint_time_cmp;
> > +static u64 __iomem *clint_time_val;
> > +static unsigned long clint_freq;
> > +static unsigned int clint_irq;
> > +
> > +static void clint_send_ipi(const unsigned long *hart_mask)
> > +{
> > +     u32 hartid;
> > +
> > +     for_each_set_bit(hartid, hart_mask, NR_CPUS)
> > +             writel(1, clint_ipi_base + hartid);
> > +}
> > +
> > +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_time_val)
> > +#else
> > +#define clint_get_cycles()   readl_relaxed(clint_time_val)
> > +#define clint_get_cycles_hi()        readl_relaxed(((u32 *)clint_time_val) + 1)
> > +#endif
> > +
> > +#ifdef CONFIG_64BIT
> > +static u64 clint_get_cycles64(void)
> > +{
> > +     return clint_get_cycles();
> > +}
> > +#else /* CONFIG_64BIT */
> > +static u64 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 int clint_clock_next_event(unsigned long delta,
> > +                                struct clock_event_device *ce)
> > +{
> > +     void __iomem *r = clint_time_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 u64 clint_rdtime(struct clocksource *cs)
> > +{
> > +     return readq_relaxed(clint_time_val);
> > +}
> > +
> > +static u64 notrace clint_sched_clock(void)
> > +{
> > +     return readq_relaxed(clint_time_val);
> > +}
> > +
> > +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_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_freq, 200, ULONG_MAX);
> > +
> > +     enable_percpu_irq(clint_irq, irq_get_trigger_type(clint_irq));
> > +     return 0;
> > +}
> > +
> > +static int clint_timer_dying_cpu(unsigned int cpu)
> > +{
> > +     disable_percpu_irq(clint_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_info("%pOFP: invalid irq %d (hwirq %d)\n",
> > +                             np, i, oirq.args[0]);
> > +                     return 0;
> > +             }
> > +     }
> > +
> > +     oirq.np = riscv_of_intc_domain_node();
> > +     oirq.args_count = 1;
> > +     oirq.args[0] = RV_IRQ_TIMER;
> > +     clint_irq = irq_create_of_mapping(&oirq);
> > +     if (!clint_irq) {
> > +             pr_err("%pOFP: could not map hwirq %d\n", np, RV_IRQ_TIMER);
> > +             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_time_cmp = base + CLINT_TIME_CMP_OFF;
> > +     clint_time_val = base + CLINT_TIME_VAL_OFF;
> > +     clint_freq = riscv_timebase;
> > +
> > +     pr_info("%pOFP: timer running at %ld Hz\n", np, clint_freq);
> > +
> > +     rc = clocksource_register_hz(&clint_clocksource, clint_freq);
> > +     if (rc) {
> > +             iounmap(base);
> > +             pr_err("%pOFP: clocksource register failed [%d]\n", np, rc);
> > +             return rc;
> > +     }
> > +
> > +     sched_clock_register(clint_sched_clock, 64, clint_freq);
> > +
> > +     rc = request_percpu_irq(clint_irq, clint_timer_interrupt,
> > +                              "clint-timer", &clint_clock_event);
> > +     if (rc) {
> > +             iounmap(base);
> > +             pr_err("registering percpu irq failed [%d]\n", rc);
> > +             return rc;
> > +     }
> > +
> > +     rc = cpuhp_setup_state(CPUHP_AP_CLINT_TIMER_STARTING,
> > +                             "clockevents/clint/timer:starting",
> > +                             clint_timer_starting_cpu,
> > +                             clint_timer_dying_cpu);
> > +     if (rc) {
> > +             free_irq(clint_irq, &clint_clock_event);
> > +             iounmap(base);
> > +             pr_err("%pOFP: cpuhp setup state failed [%d]\n", np, rc);
> > +             return rc;
> > +     }
> > +
> > +     riscv_set_ipi_ops(&clint_ipi_ops);
> > +     clint_clear_ipi();
> > +
> > +     return 0;
> > +}
> > +
> > +TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
> > +TIMER_OF_DECLARE(clint_timer1, "sifive,clint-1.0.0", clint_timer_init_dt);
> > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > index 57b1f8f777d9..52552492c2f2 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,

Re: [PATCH 1/5] RISC-V: Add mechanism to provide custom IPI operations

[Anup Patel]

On Fri, Jun 5, 2020 at 2:10 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 21 May 2020 06:45:40 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 <anup.patel@wdc.com>
> > ---
> >  arch/riscv/include/asm/smp.h | 11 ++++++++
> >  arch/riscv/kernel/smp.c      | 52 ++++++++++++++++++++++++------------
> >  arch/riscv/kernel/smpboot.c  |  3 +--
> >  3 files changed, 47 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
> > index 40bb1c15a731..ad0601260cb1 100644
> > --- a/arch/riscv/include/asm/smp.h
> > +++ b/arch/riscv/include/asm/smp.h
> > @@ -40,6 +40,17 @@ 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);
> >
> > +struct riscv_ipi_ops {
> > +     void (*ipi_inject)(const unsigned long *hart_mask);
> > +     void (*ipi_clear)(void);
> > +};
> > +
> > +/* 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.
> > diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> > index b1d4f452f843..8375cc5970f6 100644
> > --- a/arch/riscv/kernel/smp.c
> > +++ b/arch/riscv/kernel/smp.c
> > @@ -84,6 +84,35 @@ static void ipi_stop(void)
> >               wait_for_interrupt();
> >  }
> >
> > +#if IS_ENABLED(CONFIG_RISCV_SBI)
> > +static void clear_ipi(void)
> > +{
> > +     csr_clear(CSR_IP, IE_SIE);
> > +}
> > +
> > +static struct riscv_ipi_ops sbi_ipi_ops = {
> > +     .ipi_inject = sbi_send_ipi,
> > +     .ipi_clear = clear_ipi,
> > +};
> > +
> > +static struct riscv_ipi_ops *ipi_ops = &sbi_ipi_ops;
> > +#else
> > +static struct riscv_ipi_ops *ipi_ops;
> > +#endif
> > +
> > +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();
> > +}
> > +EXPORT_SYMBOL_GPL(riscv_clear_ipi);
>
> There should at least be a warning on SMP systems when an ipi_ops hasn't been
> set, as otherwise the system will just hang.

Sure, I will add pr_warn() here.

>
> > +
> >  static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
> >  {
> >       struct cpumask hartid_mask;
> > @@ -95,10 +124,9 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
> >       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));
> > -     else
> > -             clint_send_ipi_mask(mask);
> > +
> > +     if (ipi_ops)
> > +             ipi_ops->ipi_inject(cpumask_bits(&hartid_mask));
> >  }
> >
> >  static void send_ipi_single(int cpu, enum ipi_message_type op)
> > @@ -109,18 +137,8 @@ static void send_ipi_single(int cpu, enum ipi_message_type op)
> >       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);
> > -     else
> > -             clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
> > +     if (ipi_ops)
> > +             ipi_ops->ipi_inject(cpumask_bits(cpumask_of(hartid)));
> >  }
> >
> >  void handle_IPI(struct pt_regs *regs)
> > @@ -131,7 +149,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);

Regards,
Anup

Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings

[Anup Patel]

On Fri, May 29, 2020 at 4:48 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, May 26, 2020 at 05:32:30PM -0700, Palmer Dabbelt wrote:
> > On Thu, 21 May 2020 23:29:36 PDT (-0700), seanga2@gmail.com wrote:
> > > On 5/22/20 1:54 AM, Anup Patel wrote:
> > > > On Fri, May 22, 2020 at 1:35 AM Sean Anderson <seanga2@gmail.com> wrote:
> > > > >
> > > > > On 5/21/20 9:45 AM, Anup Patel wrote:
> > > > > > +Required properties:
> > > > > > +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
> > > > > > +  detailed implementation in case that specific bugs need to be worked around.
> > > > >
> > > > > Should the "riscv,clint0" compatible string be documented here? This
> > > >
> > > > Yes, I forgot to add this compatible string. I will add in v2.
> > > >
> > > > > peripheral is not really specific to sifive, as it is present in most
> > > > > rocket-chip cores.
> > > >
> > > > I agree that CLINT is present in a lot of non-SiFive RISC-V SOCs and
> > > > FPGAs but this IP is only documented as part of SiFive FU540 SOC.
> > > > (Refer, https://static.dev.sifive.com/FU540-C000-v1.0.pdf)
> > > >
> > > > The RISC-V foundation should host the CLINT spec independently
> > > > under https://github.com/riscv and make CLINT spec totally open.
> > > >
> > > > For now, I have documented it just like PLIC DT bindings found at:
> > > > Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.txt
> > >
> > > The PLIC seems to have its own RISC-V-sponsored documentation [1] which
> > > was split off from the older privileged specs. By your logic above,
> > > should it be renamed to riscv,plic0.txt (with a corresponding change in
> > > the documented compatible strings)?
> > >
> > > [1] https://github.com/riscv/riscv-plic-spec
> >
> > Let's propose tagging that PLIC spec as v1.0.0 in the platform spec group, but
> > I don't see a reason why that wouldn't be viable.  Assuming that's all OK, we
> > can start calling this a RISC-V PLIC (in addition to a SiFive PLIC, as they'll
> > be compatible).
> >
> > > >
> > > > If RISC-V maintainers agree then I will document it as "RISC-V CLINT".
> > > >
> > > > @Palmer ?? @Paul ??
> >
> > The CLINT is a SiFive spec.  It has open source RTL so it's been implemented in
> > other designs, but it's not a RISC-V spec.  The CLIC, which is a superset of
> > the CLINT, is a RISC-V spec.  IIRC it's not finished yet (it's the fast
> > interrupts task group), but presumably we should have a "riscv,clic-2.0.0" (or
> > whatever it ends up being called) compat string to go along with the
> > specification.
>
> Whatever you all decide on, note that "sifive,<block><num>" is a SiFive
> thing (as it is documented) and <num> corresponds to tag of the IP
> implmentation (at least it is supposed to). So you can't just copy that
> with 'riscv,<block><num>' unless you have the same IP versioning
> and update the documentation.

I agree that "sifive,<block><num>" is a SiFive thingy. Unfortunately,
lot of RISC-V implementations (SiFive and non-SiFive) have DTS
generated from RTL (not part of Linux sources) where most of them
use "riscv,clint0" as compatible string for CLINT.

>
> Using a spec version is fine, but not standalone. You need
> implementation specific compatible too because no one perfectly
> implements any spec and/or there details a spec may not cover.

Sure, a generic compatible string "riscv,clint0" OR "sifive,clint-1.0.0"
along with an implementation specific compatible string sounds
good to me.

Regards,
Anup