devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/3] Add timer driver for StarFive JH7110 RISC-V SoC
@ 2022-12-23  9:47 Xingyu Wu
  2022-12-23  9:47 ` [PATCH v1 1/3] dt-bindings: timer: Add timer for StarFive JH7110 SoC Xingyu Wu
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Xingyu Wu @ 2022-12-23  9:47 UTC (permalink / raw)
  To: linux-riscv, devicetree, Daniel Lezcano, Thomas Gleixner,
	Krzysztof Kozlowski
  Cc: Rob Herring, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Philipp Zabel, Xingyu Wu, Samin Guo, linux-kernel

This patch serises are to add timer driver for the StarFive JH7110
RISC-V SoC. The first patch adds documentation to describe device
tree bindings. The subsequent patch adds timer driver and support
JH7110 SoC. The last patch adds device node about timer to JH7110
dts.

This timer has eight free-running 32 bit counters and uses four in
StarFive JH7110 SoC. They support single-run mode and continuous-run
mode.

The last patch should be applied after the patchset [1]:
[1] https://lore.kernel.org/all/20221118011714.70877-1-hal.feng@starfivetech.com/


Xingyu Wu (3):
  dt-bindings: timer: Add timer for StarFive JH7110 SoC
  clocksource: Add StarFive timer driver
  riscv: dts: jh7110: starfive: Add timer node

 .../timer/starfive,jh7110-timers.yaml         | 105 ++++
 MAINTAINERS                                   |   7 +
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  22 +
 drivers/clocksource/Kconfig                   |  12 +
 drivers/clocksource/Makefile                  |   1 +
 drivers/clocksource/timer-starfive.c          | 465 ++++++++++++++++++
 drivers/clocksource/timer-starfive.h          | 104 ++++
 7 files changed, 716 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
 create mode 100644 drivers/clocksource/timer-starfive.c
 create mode 100644 drivers/clocksource/timer-starfive.h


base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa
prerequisite-patch-id: 6b1b43a55b9773bec61ab6c1bbaa54dccbac0837
prerequisite-patch-id: 09c98554df52d17ba5fd604125f8cdd62cbe80d1
prerequisite-patch-id: 29fe0b0c19b6f0cd31114ee9fe17fe9732047f33
prerequisite-patch-id: c59d9908de90e09ba2b9a81aadbf9fb9f00c8f04
prerequisite-patch-id: 94ac03d518993921bcfc9cc9f58d7da0c3528b51
prerequisite-patch-id: 694f7400375f5b85581fc1821e427334507826f2
prerequisite-patch-id: 699d49c4439dadb4b7cf900857f027d050cd6093
prerequisite-patch-id: 40d773f5a19912f731ee5fd4739ed2e3c2157b07
prerequisite-patch-id: 2bc3fd6df5dda116efe882045863d6c88aa81b3a
prerequisite-patch-id: 735e62255c75801bdc4c0b4107850bce821ff7f5
prerequisite-patch-id: b2a923b922e661fa6085185f33c1f1e733db9110
prerequisite-patch-id: b2bbc28354075432f059344eba5a127a653475cf
prerequisite-patch-id: 70eab7b7eee728afcd90e40f6743d1356f6d81ab
prerequisite-patch-id: 6276b2a23818c65ff2ad3d65b562615690cffee9
prerequisite-patch-id: d834ece14ffb525b8c3e661e78736692f33fca9b
prerequisite-patch-id: 4c17a3ce4dae9b788795d915bf775630f5c43c53
prerequisite-patch-id: dabb913fd478e97593e45c23fee4be9fd807f851
prerequisite-patch-id: 22fa141f7f0f80a5d619e9f3f4cf161ad06f108e
prerequisite-patch-id: f306819c257ea73aff8e06b17b5731053cdddfc8
prerequisite-patch-id: 536114f0732646095ef5302a165672b3290d4c75
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: e09e995700a814a763aa304ad3881a7222acf556
prerequisite-patch-id: 05803238293fcc90c8e83018a1103c41133a816c
prerequisite-patch-id: d074c7ffa2917a9f754d5801e3f67bc980f9de4c
prerequisite-patch-id: ef23fdf3466b3c713b3826e8545c8dd2bc6cc9d7
prerequisite-patch-id: d5da3475c6a3588e11a1678feb565bdd459b548e
-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 1/3] dt-bindings: timer: Add timer for StarFive JH7110 SoC
  2022-12-23  9:47 [PATCH v1 0/3] Add timer driver for StarFive JH7110 RISC-V SoC Xingyu Wu
@ 2022-12-23  9:47 ` Xingyu Wu
  2022-12-23 10:25   ` Krzysztof Kozlowski
  2022-12-23  9:48 ` [PATCH v1 2/3] clocksource: Add StarFive timer driver Xingyu Wu
  2022-12-23  9:48 ` [PATCH v1 3/3] riscv: dts: jh7110: starfive: Add timer node Xingyu Wu
  2 siblings, 1 reply; 10+ messages in thread
From: Xingyu Wu @ 2022-12-23  9:47 UTC (permalink / raw)
  To: linux-riscv, devicetree, Daniel Lezcano, Thomas Gleixner,
	Krzysztof Kozlowski
  Cc: Rob Herring, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Philipp Zabel, Xingyu Wu, Samin Guo, linux-kernel

Add bindings for the timer on the JH7110
RISC-V SoC by StarFive Technology Ltd.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 .../timer/starfive,jh7110-timers.yaml         | 105 ++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml

diff --git a/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml b/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
new file mode 100644
index 000000000000..fe58dc056313
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/starfive,jh7110-timers.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive Timers
+
+maintainers:
+  - Samin Guo <samin.guo@starfivetech.com>
+  - Xingyu Wu <xingyu.wu@starfivetech.com>
+
+properties:
+  compatible:
+    const: starfive,jh7110-timers
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: timer channel 0 interrupt
+      - description: timer channel 1 interrupt
+      - description: timer channel 2 interrupt
+      - description: timer channel 3 interrupt
+
+  interrupt-names:
+    items:
+      - const: timer0
+      - const: timer1
+      - const: timer2
+      - const: timer3
+
+  clocks:
+    items:
+      - description: timer channel 0 clock
+      - description: timer channel 1 clock
+      - description: timer channel 2 clock
+      - description: timer channel 3 clock
+      - description: APB clock
+
+  clock-names:
+    items:
+      - const: timer0
+      - const: timer1
+      - const: timer2
+      - const: timer3
+      - const: apb
+
+  resets:
+    items:
+      - description: timer channel 0 reset
+      - description: timer channel 1 reset
+      - description: timer channel 2 reset
+      - description: timer channel 3 reset
+      - description: APB reset
+
+  reset-names:
+    items:
+      - const: timer0
+      - const: timer1
+      - const: timer2
+      - const: timer3
+      - const: apb
+
+  clock-frequency:
+    description: The frequency of the clock that drives the counter, in Hz.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - clock-frequency
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    timer@13050000 {
+        compatible = "starfive,jh7110-timers";
+        reg = <0x13050000 0x10000>;
+        interrupts = <69>, <70>, <71> ,<72>;
+        interrupt-names = "timer0", "timer1", "timer2", "timer3";
+        clocks = <&clk 125>,
+                 <&clk 126>,
+                 <&clk 127>,
+                 <&clk 128>,
+                 <&clk 124>;
+        clock-names = "timer0", "timer1",
+                      "timer2", "timer3", "apb";
+        resets = <&rst 118>,
+                 <&rst 119>,
+                 <&rst 120>,
+                 <&rst 121>,
+                 <&rst 117>;
+        reset-names = "timer0", "timer1",
+                      "timer2", "timer3", "apb";
+        clock-frequency = <24000000>;
+    };
+
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 2/3] clocksource: Add StarFive timer driver
  2022-12-23  9:47 [PATCH v1 0/3] Add timer driver for StarFive JH7110 RISC-V SoC Xingyu Wu
  2022-12-23  9:47 ` [PATCH v1 1/3] dt-bindings: timer: Add timer for StarFive JH7110 SoC Xingyu Wu
@ 2022-12-23  9:48 ` Xingyu Wu
  2023-01-17  6:39   ` Xingyu Wu
  2023-03-02  9:39   ` Andreas Schwab
  2022-12-23  9:48 ` [PATCH v1 3/3] riscv: dts: jh7110: starfive: Add timer node Xingyu Wu
  2 siblings, 2 replies; 10+ messages in thread
From: Xingyu Wu @ 2022-12-23  9:48 UTC (permalink / raw)
  To: linux-riscv, devicetree, Daniel Lezcano, Thomas Gleixner,
	Krzysztof Kozlowski
  Cc: Rob Herring, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Philipp Zabel, Xingyu Wu, Samin Guo, linux-kernel

Add timer driver for the StarFive JH7110 SoC.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 MAINTAINERS                          |   7 +
 drivers/clocksource/Kconfig          |  12 +
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/timer-starfive.c | 465 +++++++++++++++++++++++++++
 drivers/clocksource/timer-starfive.h | 104 ++++++
 5 files changed, 589 insertions(+)
 create mode 100644 drivers/clocksource/timer-starfive.c
 create mode 100644 drivers/clocksource/timer-starfive.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a70c1d0f303e..340ad9e0a31f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19623,6 +19623,13 @@ F:	Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
 F:	drivers/reset/starfive/
 F:	include/dt-bindings/reset/starfive*
 
+STARFIVE TIMER DRIVER
+M:	Samin Guo <samin.guo@starfivetech.com>
+M:	Xingyu Wu <xingyu.wu@starfivetech.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/timer/starfive*
+F:	drivers/clocksource/timer-starfive*
+
 STATIC BRANCH/CALL
 M:	Peter Zijlstra <peterz@infradead.org>
 M:	Josh Poimboeuf <jpoimboe@kernel.org>
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4469e7f555e9..11400b3a8f5c 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -630,6 +630,18 @@ config RISCV_TIMER
 	  is accessed via both the SBI and the rdcycle instruction.  This is
 	  required for all RISC-V systems.
 
+config STARFIVE_TIMER
+	bool "Timer for the STARFIVE SoCs"
+	depends on RISCV && OF && (SOC_STARFIVE || COMPILE_TEST)
+	select TIMER_OF
+	select CLKSRC_MMIO
+	default SOC_STARFIVE
+	help
+	  This enables the timers for StarFive SoCs. On RISC-V platform,
+	  the system has started RISCV_TIMER. But you can also use these timers
+	  to do a lot more on StarFive SoCs. These timers can provide much
+	  higher precision than RISCV_TIMER.
+
 config CLINT_TIMER
 	bool "CLINT Timer for the RISC-V platform" if COMPILE_TEST
 	depends on GENERIC_SCHED_CLOCK && RISCV
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 64ab547de97b..276695d95cdc 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_INGENIC_TIMER)		+= ingenic-timer.o
 obj-$(CONFIG_CLKSRC_ST_LPC)		+= clksrc_st_lpc.o
 obj-$(CONFIG_X86_NUMACHIP)		+= numachip.o
 obj-$(CONFIG_RISCV_TIMER)		+= timer-riscv.o
+obj-$(CONFIG_STARFIVE_TIMER)		+= timer-starfive.o
 obj-$(CONFIG_CLINT_TIMER)		+= timer-clint.o
 obj-$(CONFIG_CSKY_MP_TIMER)		+= timer-mp-csky.o
 obj-$(CONFIG_GX6605S_TIMER)		+= timer-gx6605s.o
diff --git a/drivers/clocksource/timer-starfive.c b/drivers/clocksource/timer-starfive.c
new file mode 100644
index 000000000000..8db413a22fdc
--- /dev/null
+++ b/drivers/clocksource/timer-starfive.c
@@ -0,0 +1,465 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Starfive Timer driver
+ *
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ *
+ * Author:
+ * Xingyu Wu <xingyu.wu@starfivetech.com>
+ * Samin Guo <samin.guo@starfivetech.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_clk.h>
+#include <linux/of_irq.h>
+#include <linux/reset.h>
+#include <linux/sched_clock.h>
+
+#include "timer-starfive.h"
+
+struct starfive_timer __initdata starfive_timer_jh7110 = {
+	.ctrl		= STARFIVE_TIMER_JH7110_CTL,
+	.load		= STARFIVE_TIMER_JH7110_LOAD,
+	.enable		= STARFIVE_TIMER_JH7110_ENABLE,
+	.reload		= STARFIVE_TIMER_JH7110_RELOAD,
+	.value		= STARFIVE_TIMER_JH7110_VALUE,
+	.intclr		= STARFIVE_TIMER_JH7110_INT_CLR,
+	.intmask	= STARFIVE_TIMER_JH7110_INT_MASK,
+	.timer_base	= {STARFIVE_TIMER_BASE(0), STARFIVE_TIMER_BASE(1),
+			   STARFIVE_TIMER_BASE(2), STARFIVE_TIMER_BASE(3)},
+};
+
+static inline struct starfive_clkevt *to_starfive_clkevt(struct clock_event_device *evt)
+{
+	return container_of(evt, struct starfive_clkevt, evt);
+}
+
+/* 0:continuous-run mode, 1:single-run mode */
+static inline void starfive_timer_set_mod(struct starfive_clkevt *clkevt, int mod)
+{
+	writel(mod, clkevt->ctrl);
+}
+
+/* Interrupt Mask Register, 0:Unmask, 1:Mask */
+static inline void starfive_timer_int_enable(struct starfive_clkevt *clkevt)
+{
+	writel(INTMASK_ENABLE_DIS, clkevt->intmask);
+}
+
+static inline void starfive_timer_int_disable(struct starfive_clkevt *clkevt)
+{
+	writel(INTMASK_ENABLE, clkevt->intmask);
+}
+
+/*
+ * BIT(0): Read value represent channel intr status.
+ * Write 1 to this bit to clear interrupt. Write 0 has no effects.
+ * BIT(1): "1" means that it is clearing interrupt. BIT(0) can not be written.
+ */
+static inline void starfive_timer_int_clear(struct starfive_clkevt *clkevt)
+{
+	/* waiting interrupt can be to clearing */
+	u32 value;
+	int ret = 0;
+
+	value = readl(clkevt->intclr);
+	ret = readl_poll_timeout_atomic(clkevt->intclr, value,
+					!(value & STARFIVE_TIMER_JH7110_INT_STATUS_CLR_AVA),
+					STARFIVE_DELAY_US, STARFIVE_TIMEOUT_US);
+	if (!ret)
+		writel(1, clkevt->intclr);
+}
+
+/*
+ * The initial value to be loaded into the
+ * counter and is also used as the reload value.
+ */
+static inline void starfive_timer_set_load(struct starfive_clkevt *clkevt, u32 val)
+{
+	writel(val, clkevt->load);
+}
+
+static inline u32 starfive_timer_get_val(struct starfive_clkevt *clkevt)
+{
+	return readl(clkevt->value);
+}
+
+/*
+ * Write RELOAD register to reload preset value to counter.
+ * (Write 0 and write 1 are both ok)
+ */
+static inline void starfive_timer_set_reload(struct starfive_clkevt *clkevt)
+{
+	writel(1, clkevt->reload);
+}
+
+static inline void starfive_timer_enable(struct starfive_clkevt *clkevt)
+{
+	writel(TIMER_ENA, clkevt->enable);
+}
+
+static inline void starfive_timer_disable(struct starfive_clkevt *clkevt)
+{
+	writel(TIMER_ENA_DIS, clkevt->enable);
+}
+
+static void timer_shutdown(struct starfive_clkevt *clkevt)
+{
+	starfive_timer_int_disable(clkevt);
+	starfive_timer_disable(clkevt);
+	starfive_timer_int_clear(clkevt);
+}
+
+static void starfive_timer_suspend(struct clock_event_device *evt)
+{
+	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
+
+	clkevt->reload_val = starfive_timer_get_val(clkevt);
+
+	starfive_timer_disable(clkevt);
+	starfive_timer_int_disable(clkevt);
+	starfive_timer_int_clear(clkevt);
+}
+
+static void starfive_timer_resume(struct clock_event_device *evt)
+{
+	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
+
+	starfive_timer_set_load(clkevt, clkevt->reload_val);
+	starfive_timer_set_reload(clkevt);
+	starfive_timer_int_enable(clkevt);
+	starfive_timer_enable(clkevt);
+}
+
+static int starfive_timer_tick_resume(struct clock_event_device *evt)
+{
+	starfive_timer_resume(evt);
+
+	return 0;
+}
+
+static int starfive_timer_shutdown(struct clock_event_device *evt)
+{
+	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
+
+	timer_shutdown(clkevt);
+
+	return 0;
+}
+
+static int starfive_get_clock_rate(struct starfive_clkevt *clkevt, struct device_node *np)
+{
+	int ret;
+	u32 rate;
+
+	if (clkevt->clk) {
+		clkevt->rate = clk_get_rate(clkevt->clk);
+		if (clkevt->rate > 0) {
+			pr_debug("clk_get_rate clkevt->rate: %d\n", clkevt->rate);
+			return 0;
+		}
+	}
+
+	/* Next we try to get clock-frequency from dts.*/
+	ret = of_property_read_u32(np, "clock-frequency", &rate);
+	if (!ret) {
+		pr_debug("Timer: try get clock-frequency:%d Hz\n", rate);
+		clkevt->rate = rate;
+		return 0;
+	}
+	pr_err("Timer: get rate failed, need clock-frequency define in dts.\n");
+
+	return -ENOENT;
+}
+
+static int starfive_clocksource_init(struct starfive_clkevt *clkevt,
+				     const char *name, struct device_node *np)
+{
+	starfive_timer_set_mod(clkevt, MOD_CONTIN);
+	starfive_timer_set_load(clkevt, STARFIVE_MAX_TICKS);  /* val = rate --> 1s */
+	starfive_timer_int_disable(clkevt);
+	starfive_timer_int_clear(clkevt);
+	starfive_timer_int_enable(clkevt);
+	starfive_timer_enable(clkevt);
+
+	return clocksource_mmio_init(clkevt->value, name, clkevt->rate,
+				     STARFIVE_CLOCK_SOURCE_RATING, STARFIVE_VALID_BITS,
+				     clocksource_mmio_readl_down);
+}
+
+/*
+ * IRQ handler for the timer
+ */
+static irqreturn_t starfive_timer_interrupt(int irq, void *priv)
+{
+	struct clock_event_device *evt = (struct clock_event_device *)priv;
+	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
+
+	starfive_timer_int_clear(clkevt);
+
+	if (evt->event_handler)
+		evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static int starfive_timer_set_periodic(struct clock_event_device *evt)
+{
+	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
+
+	starfive_timer_disable(clkevt);
+	starfive_timer_set_mod(clkevt, MOD_CONTIN);
+	starfive_timer_set_load(clkevt, clkevt->periodic);
+	starfive_timer_int_disable(clkevt);
+	starfive_timer_int_clear(clkevt);
+	starfive_timer_int_enable(clkevt);
+	starfive_timer_enable(clkevt);
+
+	return 0;
+}
+
+static int starfive_timer_set_oneshot(struct clock_event_device *evt)
+{
+	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
+
+	starfive_timer_disable(clkevt);
+	starfive_timer_set_mod(clkevt, MOD_SINGLE);
+	starfive_timer_set_load(clkevt, STARFIVE_MAX_TICKS);
+	starfive_timer_int_disable(clkevt);
+	starfive_timer_int_clear(clkevt);
+	starfive_timer_int_enable(clkevt);
+	starfive_timer_enable(clkevt);
+
+	return 0;
+}
+
+static int starfive_timer_set_next_event(unsigned long next,
+					 struct clock_event_device *evt)
+{
+	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
+
+	starfive_timer_disable(clkevt);
+	starfive_timer_set_mod(clkevt, MOD_SINGLE);
+	starfive_timer_set_load(clkevt, next);
+	starfive_timer_enable(clkevt);
+
+	return 0;
+}
+
+static void starfive_set_clockevent(struct clock_event_device *evt)
+{
+	evt->features	= CLOCK_EVT_FEAT_PERIODIC |
+			  CLOCK_EVT_FEAT_ONESHOT |
+			  CLOCK_EVT_FEAT_DYNIRQ;
+	evt->set_state_shutdown	= starfive_timer_shutdown;
+	evt->set_state_periodic	= starfive_timer_set_periodic;
+	evt->set_state_oneshot	= starfive_timer_set_oneshot;
+	evt->set_state_oneshot_stopped = starfive_timer_shutdown;
+	evt->tick_resume	= starfive_timer_tick_resume;
+	evt->set_next_event	= starfive_timer_set_next_event;
+	evt->suspend		= starfive_timer_suspend;
+	evt->resume		= starfive_timer_resume;
+	evt->rating		= STARFIVE_CLOCKEVENT_RATING;
+}
+
+static int starfive_clockevents_register(struct starfive_clkevt *clkevt, unsigned int irq,
+					 struct device_node *np, const char *name)
+{
+	int ret = 0;
+
+	ret = starfive_get_clock_rate(clkevt, np);
+	if (ret)
+		return -EINVAL;
+
+	clkevt->periodic = DIV_ROUND_CLOSEST(clkevt->rate, HZ);
+
+	starfive_set_clockevent(&clkevt->evt);
+	clkevt->evt.name = name;
+	clkevt->evt.irq = irq;
+	clkevt->evt.cpumask = cpu_possible_mask;
+
+	ret = request_irq(irq, starfive_timer_interrupt,
+			  IRQF_TIMER | IRQF_IRQPOLL, name, &clkevt->evt);
+	if (ret)
+		pr_err("%s: request_irq failed\n", name);
+
+	clockevents_config_and_register(&clkevt->evt, clkevt->rate,
+					STARFIVE_MIN_TICKS, STARFIVE_MAX_TICKS);
+
+	return ret;
+}
+
+static void __init starfive_clkevt_base_init(struct starfive_timer *timer,
+					     struct starfive_clkevt *clkevt,
+					     void __iomem *base, int index)
+{
+	void __iomem *timer_base;
+
+	timer_base	= base + timer->timer_base[index];
+	clkevt->base	= timer_base;
+	clkevt->ctrl	= timer_base + timer->ctrl;
+	clkevt->load	= timer_base + timer->load;
+	clkevt->enable	= timer_base + timer->enable;
+	clkevt->reload	= timer_base + timer->reload;
+	clkevt->value	= timer_base + timer->value;
+	clkevt->intclr	= timer_base + timer->intclr;
+	clkevt->intmask	= timer_base + timer->intmask;
+}
+
+static int __init starfive_timer_jh7110_of_init(struct device_node *np)
+{
+	int index, count, irq, ret;
+	const char *name = NULL;
+	struct clk *clk;
+	struct clk *pclk;
+	struct reset_control *prst;
+	struct reset_control *rst;
+	struct starfive_clkevt *clkevt[STARFIVE_NR_TIMERS];
+	void __iomem *base;
+	struct starfive_timer *timer = &starfive_timer_jh7110;
+
+	base = of_iomap(np, 0);
+	if (!base)
+		return -ENXIO;
+
+	if (!of_device_is_available(np)) {
+		ret = -EINVAL;
+		goto err;
+	}
+
+	pclk = of_clk_get_by_name(np, "apb");
+	if (!IS_ERR(pclk)) {
+		if (clk_prepare_enable(pclk))
+			pr_warn("pclk for %pOFn is present, but could not be activated\n", np);
+	/*
+	 * Clock framework support is late, continue on
+	 * anyways if we don't find a matching clock.
+	 */
+	} else if (PTR_ERR(pclk) != -EPROBE_DEFER) {
+		ret = PTR_ERR(pclk);
+		goto err;
+	}
+
+	prst = of_reset_control_get(np, "apb");
+	if (!IS_ERR(prst)) {
+		ret = reset_control_deassert(prst);
+		if (ret)
+			goto prst_err;
+	/*
+	 * Reset framework support is late, continue on
+	 * anyways if we don't find a matching reset.
+	 */
+	} else if (PTR_ERR(prst) != -EPROBE_DEFER) {
+		ret = PTR_ERR(prst);
+		goto prst_err;
+	}
+
+	/* The number of timers used is determined according to the device tree. */
+	count = of_irq_count(np);
+	if (count > STARFIVE_NR_TIMERS || count <= 0) {
+		ret = -EINVAL;
+		goto count_err;
+	}
+
+	for (index = 0; index < count; index++) {
+		of_property_read_string_index(np, "clock-names", index, &name);
+		if (strncmp(name, "timer", strlen("timer")))
+			continue;
+
+		clkevt[index] = kzalloc(sizeof(*clkevt[index]), GFP_KERNEL);
+		if (!clkevt[index]) {
+			ret = -ENOMEM;
+			goto clkevt_err;
+		}
+
+		starfive_clkevt_base_init(timer, clkevt[index], base, index);
+
+		/* Ensure timers are disabled */
+		starfive_timer_disable(clkevt[index]);
+
+		clk = of_clk_get_by_name(np, name);
+		if (!IS_ERR(clk)) {
+			clkevt[index]->clk = clk;
+			if (clk_prepare_enable(clkevt[index]->clk))
+				pr_warn("clk for %pOFn is present, but could not be activated\n",
+					np);
+		} else if (PTR_ERR(clk) != -EPROBE_DEFER) {
+			ret = PTR_ERR(clk);
+			goto clk_err;
+		}
+
+		rst = of_reset_control_get(np, name);
+		if (!IS_ERR(rst)) {
+			clkevt[index]->rst = rst;
+			ret = reset_control_deassert(clkevt[index]->rst);
+			if (ret)
+				goto rst_err;
+		}
+
+		irq = irq_of_parse_and_map(np, index);
+		if (irq < 0) {
+			ret = -EINVAL;
+			goto irq_err;
+		}
+
+		snprintf(clkevt[index]->name, sizeof(clkevt[index]->name), "%s.ch%d",
+			 np->full_name, index);
+
+		ret = starfive_clockevents_register(clkevt[index], irq, np, clkevt[index]->name);
+		if (ret) {
+			pr_err("%s: init clockevents failed.\n", clkevt[index]->name);
+			goto register_err;
+		}
+		clkevt[index]->irq = irq;
+
+		ret = starfive_clocksource_init(clkevt[index], clkevt[index]->name, np);
+		if (ret)
+			goto init_err;
+	}
+	if (!IS_ERR(pclk))
+		clk_put(pclk);
+
+	return 0;
+
+init_err:
+register_err:
+	free_irq(clkevt[index]->irq, &clkevt[index]->evt);
+irq_err:
+rst_err:
+clk_err:
+	/* Only unregister the failed timer and the rest timers continue to work. */
+	if (!clkevt[index]->rst) {
+		reset_control_assert(clkevt[index]->rst);
+		reset_control_put(clkevt[index]->rst);
+	}
+	if (!clkevt[index]->clk) {
+		clk_disable_unprepare(clkevt[index]->clk);
+		clk_put(clkevt[index]->clk);
+	}
+	kfree(clkevt[index]);
+clkevt_err:
+count_err:
+prst_err:
+	if (!IS_ERR(pclk)) {
+		/* If no other timer successfully registers, pclk is disabled. */
+		if (!index)
+			clk_disable_unprepare(pclk);
+		clk_put(pclk);
+	}
+err:
+	iounmap(base);
+	return ret;
+}
+
+TIMER_OF_DECLARE(starfive_timer_jh7110, "starfive,jh7110-timers", starfive_timer_jh7110_of_init);
diff --git a/drivers/clocksource/timer-starfive.h b/drivers/clocksource/timer-starfive.h
new file mode 100644
index 000000000000..9fd3303b31d1
--- /dev/null
+++ b/drivers/clocksource/timer-starfive.h
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ */
+
+#ifndef __STARFIVE_TIMER_H__
+#define __STARFIVE_TIMER_H__
+
+#define STARFIVE_NR_TIMERS		TIMERS_MAX
+/* Bias: Timer0-0x0, Timer1-0x40, Timer2-0x80, and so on. */
+#define STARFIVE_PER_TIMER_LEN		0x40
+#define STARFIVE_TIMER_BASE(x)		((TIMER_##x) * STARFIVE_PER_TIMER_LEN)
+
+#define STARFIVE_CLOCK_SOURCE_RATING	200
+#define STARFIVE_VALID_BITS		32
+#define STARFIVE_DELAY_US		0
+#define STARFIVE_TIMEOUT_US		10000
+#define STARFIVE_CLOCKEVENT_RATING	300
+#define STARFIVE_MAX_TICKS		0xffffffff
+#define STARFIVE_MIN_TICKS		0xf
+
+/*
+ * JH7110 timer TIMER_INT_STATUS:
+ * ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ * |     Bits     | 08~31 | 7 | 6 | 5 |  4  | 3 | 2 | 1 | 0 |
+ * ----------------------------------------------------------
+ * | timer(n)_int |  res  | 6 | 5 | 4 | Wdt | 3 | 2 | 1 | 0 |
+ * ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ *
+ * Software can read this register to know which interrupt is occurred.
+ */
+#define STARFIVE_TIMER_JH7110_INT_STATUS	0x00
+#define STARFIVE_TIMER_JH7110_CTL		0x04
+#define STARFIVE_TIMER_JH7110_LOAD		0x08
+#define STARFIVE_TIMER_JH7110_ENABLE		0x10
+#define STARFIVE_TIMER_JH7110_RELOAD		0x14
+#define STARFIVE_TIMER_JH7110_VALUE		0x18
+#define STARFIVE_TIMER_JH7110_INT_CLR		0x20
+#define STARFIVE_TIMER_JH7110_INT_MASK		0x24
+#define STARFIVE_TIMER_JH7110_INT_STATUS_CLR_AVA	BIT(1)
+
+enum STARFIVE_TIMERS {
+	TIMER_0 = 0,
+	TIMER_1,
+	TIMER_2,
+	TIMER_3,
+	TIMER_4,  /*WDT*/
+	TIMER_5,
+	TIMER_6,
+	TIMER_7,
+	TIMERS_MAX
+};
+
+enum TIMERI_INTMASK {
+	INTMASK_ENABLE_DIS = 0,
+	INTMASK_ENABLE = 1
+};
+
+enum TIMER_MOD {
+	MOD_CONTIN = 0,
+	MOD_SINGLE = 1
+};
+
+enum TIMER_CTL_EN {
+	TIMER_ENA_DIS	= 0,
+	TIMER_ENA	= 1
+};
+
+enum {
+	INT_CLR_AVAILABLE = 0,
+	INT_CLR_NOT_AVAILABLE = 1
+};
+
+struct starfive_timer {
+	u32 ctrl;
+	u32 load;
+	u32 enable;
+	u32 reload;
+	u32 value;
+	u32 intclr;
+	u32 intmask;
+	u32 wdt_lock;   /* 0x3c+i*0x40 watchdog use ONLY */
+	u32 timer_base[STARFIVE_NR_TIMERS];
+};
+
+struct starfive_clkevt {
+	struct clock_event_device evt;
+	struct clk *clk;
+	struct reset_control *rst;
+	char name[20];
+	int irq;
+	u32 periodic;
+	u32 rate;
+	u32 reload_val;
+	void __iomem *base;
+	void __iomem *ctrl;
+	void __iomem *load;
+	void __iomem *enable;
+	void __iomem *reload;
+	void __iomem *value;
+	void __iomem *intclr;
+	void __iomem *intmask;
+};
+#endif /* __STARFIVE_TIMER_H__ */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 3/3] riscv: dts: jh7110: starfive: Add timer node
  2022-12-23  9:47 [PATCH v1 0/3] Add timer driver for StarFive JH7110 RISC-V SoC Xingyu Wu
  2022-12-23  9:47 ` [PATCH v1 1/3] dt-bindings: timer: Add timer for StarFive JH7110 SoC Xingyu Wu
  2022-12-23  9:48 ` [PATCH v1 2/3] clocksource: Add StarFive timer driver Xingyu Wu
@ 2022-12-23  9:48 ` Xingyu Wu
  2 siblings, 0 replies; 10+ messages in thread
From: Xingyu Wu @ 2022-12-23  9:48 UTC (permalink / raw)
  To: linux-riscv, devicetree, Daniel Lezcano, Thomas Gleixner,
	Krzysztof Kozlowski
  Cc: Rob Herring, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Philipp Zabel, Xingyu Wu, Samin Guo, linux-kernel

Add the timer node for the Starfive JH7110 SoC.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index c22e8f1d2640..3936c7c0c962 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -356,6 +356,28 @@ gpioa: gpio@17020000 {
 			#gpio-cells = <2>;
 		};
 
+		timer: timer@13050000 {
+			compatible = "starfive,jh7110-timers";
+			reg = <0x0 0x13050000 0x0 0x10000>;
+			interrupts = <69>, <70>, <71> ,<72>;
+			interrupt-names = "timer0", "timer1", "timer2", "timer3";
+			clocks = <&syscrg JH7110_SYSCLK_TIMER0>,
+				 <&syscrg JH7110_SYSCLK_TIMER1>,
+				 <&syscrg JH7110_SYSCLK_TIMER2>,
+				 <&syscrg JH7110_SYSCLK_TIMER3>,
+				 <&syscrg JH7110_SYSCLK_TIMER_APB>;
+			clock-names = "timer0", "timer1",
+				      "timer2", "timer3", "apb";
+			resets = <&syscrg JH7110_SYSRST_TIMER0>,
+				 <&syscrg JH7110_SYSRST_TIMER1>,
+				 <&syscrg JH7110_SYSRST_TIMER2>,
+				 <&syscrg JH7110_SYSRST_TIMER3>,
+				 <&syscrg JH7110_SYSRST_TIMER_APB>;
+			reset-names = "timer0", "timer1",
+				      "timer2", "timer3", "apb";
+			clock-frequency = <24000000>;
+		};
+
 		uart0: serial@10000000 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0x0 0x10000000 0x0 0x10000>;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] dt-bindings: timer: Add timer for StarFive JH7110 SoC
  2022-12-23  9:47 ` [PATCH v1 1/3] dt-bindings: timer: Add timer for StarFive JH7110 SoC Xingyu Wu
@ 2022-12-23 10:25   ` Krzysztof Kozlowski
  2023-01-10  2:14     ` Xingyu Wu
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-23 10:25 UTC (permalink / raw)
  To: Xingyu Wu, linux-riscv, devicetree, Daniel Lezcano,
	Thomas Gleixner, Krzysztof Kozlowski
  Cc: Rob Herring, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Philipp Zabel, Samin Guo, linux-kernel

On 23/12/2022 10:47, Xingyu Wu wrote:
> Add bindings for the timer on the JH7110
> RISC-V SoC by StarFive Technology Ltd.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586


> 
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> ---
>  .../timer/starfive,jh7110-timers.yaml         | 105 ++++++++++++++++++
>  1 file changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
> 
> diff --git a/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml b/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
> new file mode 100644
> index 000000000000..fe58dc056313
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/starfive,jh7110-timers.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive Timers


Not enough, really not enough. Describe the hardware.

> +
> +maintainers:
> +  - Samin Guo <samin.guo@starfivetech.com>
> +  - Xingyu Wu <xingyu.wu@starfivetech.com>
> +
> +properties:
> +  compatible:
> +    const: starfive,jh7110-timers

Why plural "timers", not "timer"? The module is usually called timer -
see other hardware that type.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: timer channel 0 interrupt
> +      - description: timer channel 1 interrupt
> +      - description: timer channel 2 interrupt
> +      - description: timer channel 3 interrupt
> +
> +  interrupt-names:
> +    items:
> +      - const: timer0
> +      - const: timer1
> +      - const: timer2
> +      - const: timer3

I would just drop the names, not really useful. Unless you plan to add
here some generic interrupt (like you did for clock-names)?

> +
> +  clocks:
> +    items:
> +      - description: timer channel 0 clock
> +      - description: timer channel 1 clock
> +      - description: timer channel 2 clock
> +      - description: timer channel 3 clock
> +      - description: APB clock
> +
> +  clock-names:
> +    items:
> +      - const: timer0
> +      - const: timer1
> +      - const: timer2
> +      - const: timer3
> +      - const: apb
> +
> +  resets:
> +    items:
> +      - description: timer channel 0 reset
> +      - description: timer channel 1 reset
> +      - description: timer channel 2 reset
> +      - description: timer channel 3 reset
> +      - description: APB reset
> +
> +  reset-names:
> +    items:
> +      - const: timer0
> +      - const: timer1
> +      - const: timer2
> +      - const: timer3
> +      - const: apb
> +
> +  clock-frequency:
> +    description: The frequency of the clock that drives the counter, in Hz.

Why do you need it? Use common clk framework to get that frequency.

Also, sort the nodes somehow, e.g.
compatible/reg/clocks/clock-frequency/interrupts/resets.


> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - clock-frequency
> +
> +unevaluatedProperties: false

Did you test the binding?


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] dt-bindings: timer: Add timer for StarFive JH7110 SoC
  2022-12-23 10:25   ` Krzysztof Kozlowski
@ 2023-01-10  2:14     ` Xingyu Wu
  2023-01-10  8:24       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 10+ messages in thread
From: Xingyu Wu @ 2023-01-10  2:14 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, linux-riscv, devicetree, Daniel Lezcano,
	Thomas Gleixner, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Philipp Zabel, Samin Guo, linux-kernel

On 2022/12/23 18:25, Krzysztof Kozlowski wrote:
> On 23/12/2022 10:47, Xingyu Wu wrote:
>> Add bindings for the timer on the JH7110
>> RISC-V SoC by StarFive Technology Ltd.
> 
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
> 
> 
>> 
>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>> ---
>>  .../timer/starfive,jh7110-timers.yaml         | 105 ++++++++++++++++++
>>  1 file changed, 105 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml b/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
>> new file mode 100644
>> index 000000000000..fe58dc056313
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
>> @@ -0,0 +1,105 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/timer/starfive,jh7110-timers.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive Timers
> 
> 
> Not enough, really not enough. Describe the hardware.

Will add. Thanks.

> 
>> +
>> +maintainers:
>> +  - Samin Guo <samin.guo@starfivetech.com>
>> +  - Xingyu Wu <xingyu.wu@starfivetech.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: starfive,jh7110-timers
> 
> Why plural "timers", not "timer"? The module is usually called timer -
> see other hardware that type.
> 

Will fix. Thanks.

>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    items:
>> +      - description: timer channel 0 interrupt
>> +      - description: timer channel 1 interrupt
>> +      - description: timer channel 2 interrupt
>> +      - description: timer channel 3 interrupt
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: timer0
>> +      - const: timer1
>> +      - const: timer2
>> +      - const: timer3
> 
> I would just drop the names, not really useful. Unless you plan to add
> here some generic interrupt (like you did for clock-names)?

Will drop. Thanks.

> 
>> +
>> +  clocks:
>> +    items:
>> +      - description: timer channel 0 clock
>> +      - description: timer channel 1 clock
>> +      - description: timer channel 2 clock
>> +      - description: timer channel 3 clock
>> +      - description: APB clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: timer0
>> +      - const: timer1
>> +      - const: timer2
>> +      - const: timer3
>> +      - const: apb
>> +
>> +  resets:
>> +    items:
>> +      - description: timer channel 0 reset
>> +      - description: timer channel 1 reset
>> +      - description: timer channel 2 reset
>> +      - description: timer channel 3 reset
>> +      - description: APB reset
>> +
>> +  reset-names:
>> +    items:
>> +      - const: timer0
>> +      - const: timer1
>> +      - const: timer2
>> +      - const: timer3
>> +      - const: apb
>> +
>> +  clock-frequency:
>> +    description: The frequency of the clock that drives the counter, in Hz.
> 
> Why do you need it? Use common clk framework to get that frequency.

Because normally this timer driver is loaded earlier than the clock tree driver, it won't get
that frequency by clk framework and this 'clock-frequency' node is used instead.

> 
> Also, sort the nodes somehow, e.g.
> compatible/reg/clocks/clock-frequency/interrupts/resets.

Will reorder. Thanks.

> 
> 
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - interrupt-names
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - reset-names
>> +  - clock-frequency
>> +
>> +unevaluatedProperties: false
> 
> Did you test the binding?

Yes, I had tested by 'dt_binding_check'. Do you mean the 'unevaluatedProperties' is wrong
and use 'additionalProperties'?

Best regards,
Xingyu Wu


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] dt-bindings: timer: Add timer for StarFive JH7110 SoC
  2023-01-10  2:14     ` Xingyu Wu
@ 2023-01-10  8:24       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2023-01-10  8:24 UTC (permalink / raw)
  To: Xingyu Wu
  Cc: Rob Herring, linux-riscv, devicetree, Daniel Lezcano,
	Thomas Gleixner, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Philipp Zabel, Samin Guo, linux-kernel

On 10/01/2023 03:14, Xingyu Wu wrote:
> On 2022/12/23 18:25, Krzysztof Kozlowski wrote:
>> On 23/12/2022 10:47, Xingyu Wu wrote:
>>> Add bindings for the timer on the JH7110
>>> RISC-V SoC by StarFive Technology Ltd.
>>
>> Please wrap commit message according to Linux coding style / submission
>> process (neither too early nor over the limit):
>> https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
>>
>>
>>>
>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>> ---
>>>  .../timer/starfive,jh7110-timers.yaml         | 105 ++++++++++++++++++
>>>  1 file changed, 105 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml b/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
>>> new file mode 100644
>>> index 000000000000..fe58dc056313
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/timer/starfive,jh7110-timers.yaml
>>> @@ -0,0 +1,105 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/timer/starfive,jh7110-timers.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: StarFive Timers
>>
>>
>> Not enough, really not enough. Describe the hardware.
> 
> Will add. Thanks.
> 
>>
>>> +
>>> +maintainers:
>>> +  - Samin Guo <samin.guo@starfivetech.com>
>>> +  - Xingyu Wu <xingyu.wu@starfivetech.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: starfive,jh7110-timers
>>
>> Why plural "timers", not "timer"? The module is usually called timer -
>> see other hardware that type.
>>
> 
> Will fix. Thanks.
> 
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    items:
>>> +      - description: timer channel 0 interrupt
>>> +      - description: timer channel 1 interrupt
>>> +      - description: timer channel 2 interrupt
>>> +      - description: timer channel 3 interrupt
>>> +
>>> +  interrupt-names:
>>> +    items:
>>> +      - const: timer0
>>> +      - const: timer1
>>> +      - const: timer2
>>> +      - const: timer3
>>
>> I would just drop the names, not really useful. Unless you plan to add
>> here some generic interrupt (like you did for clock-names)?
> 
> Will drop. Thanks.
> 
>>
>>> +
>>> +  clocks:
>>> +    items:
>>> +      - description: timer channel 0 clock
>>> +      - description: timer channel 1 clock
>>> +      - description: timer channel 2 clock
>>> +      - description: timer channel 3 clock
>>> +      - description: APB clock
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: timer0
>>> +      - const: timer1
>>> +      - const: timer2
>>> +      - const: timer3
>>> +      - const: apb
>>> +
>>> +  resets:
>>> +    items:
>>> +      - description: timer channel 0 reset
>>> +      - description: timer channel 1 reset
>>> +      - description: timer channel 2 reset
>>> +      - description: timer channel 3 reset
>>> +      - description: APB reset
>>> +
>>> +  reset-names:
>>> +    items:
>>> +      - const: timer0
>>> +      - const: timer1
>>> +      - const: timer2
>>> +      - const: timer3
>>> +      - const: apb
>>> +
>>> +  clock-frequency:
>>> +    description: The frequency of the clock that drives the counter, in Hz.
>>
>> Why do you need it? Use common clk framework to get that frequency.
> 
> Because normally this timer driver is loaded earlier than the clock tree driver, it won't get
> that frequency by clk framework and this 'clock-frequency' node is used instead.

I don't think that clk framework or fixed clocks are not available at
this time... of_clk_init is before timer.

> 
>>
>> Also, sort the nodes somehow, e.g.
>> compatible/reg/clocks/clock-frequency/interrupts/resets.
> 
> Will reorder. Thanks.
> 
>>
>>
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +  - interrupt-names
>>> +  - clocks
>>> +  - clock-names
>>> +  - resets
>>> +  - reset-names
>>> +  - clock-frequency
>>> +
>>> +unevaluatedProperties: false
>>
>> Did you test the binding?
> 
> Yes, I had tested by 'dt_binding_check'. Do you mean the 'unevaluatedProperties' is wrong
> and use 'additionalProperties'?

Yes, previously it was generating a warning but I do not see Rob's bot
answer so maybe something changed.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] clocksource: Add StarFive timer driver
  2022-12-23  9:48 ` [PATCH v1 2/3] clocksource: Add StarFive timer driver Xingyu Wu
@ 2023-01-17  6:39   ` Xingyu Wu
  2023-03-02  9:39   ` Andreas Schwab
  1 sibling, 0 replies; 10+ messages in thread
From: Xingyu Wu @ 2023-01-17  6:39 UTC (permalink / raw)
  To: linux-riscv, devicetree, Daniel Lezcano, Thomas Gleixner,
	Krzysztof Kozlowski
  Cc: Rob Herring, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Philipp Zabel, Samin Guo, linux-kernel

On 2022/12/23 17:48, Xingyu Wu wrote:
> Add timer driver for the StarFive JH7110 SoC.
> 
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> ---
>  MAINTAINERS                          |   7 +
>  drivers/clocksource/Kconfig          |  12 +
>  drivers/clocksource/Makefile         |   1 +
>  drivers/clocksource/timer-starfive.c | 465 +++++++++++++++++++++++++++
>  drivers/clocksource/timer-starfive.h | 104 ++++++
>  5 files changed, 589 insertions(+)
>  create mode 100644 drivers/clocksource/timer-starfive.c
>  create mode 100644 drivers/clocksource/timer-starfive.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a70c1d0f303e..340ad9e0a31f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19623,6 +19623,13 @@ F:	Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
>  F:	drivers/reset/starfive/
>  F:	include/dt-bindings/reset/starfive*
>  
> +STARFIVE TIMER DRIVER
> +M:	Samin Guo <samin.guo@starfivetech.com>
> +M:	Xingyu Wu <xingyu.wu@starfivetech.com>
> +S:	Supported
> +F:	Documentation/devicetree/bindings/timer/starfive*
> +F:	drivers/clocksource/timer-starfive*
> +
>  STATIC BRANCH/CALL
>  M:	Peter Zijlstra <peterz@infradead.org>
>  M:	Josh Poimboeuf <jpoimboe@kernel.org>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 4469e7f555e9..11400b3a8f5c 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -630,6 +630,18 @@ config RISCV_TIMER
>  	  is accessed via both the SBI and the rdcycle instruction.  This is
>  	  required for all RISC-V systems.
>  
> +config STARFIVE_TIMER
> +	bool "Timer for the STARFIVE SoCs"
> +	depends on RISCV && OF && (SOC_STARFIVE || COMPILE_TEST)
> +	select TIMER_OF
> +	select CLKSRC_MMIO
> +	default SOC_STARFIVE
> +	help
> +	  This enables the timers for StarFive SoCs. On RISC-V platform,
> +	  the system has started RISCV_TIMER. But you can also use these timers
> +	  to do a lot more on StarFive SoCs. These timers can provide much
> +	  higher precision than RISCV_TIMER.
> +
>  config CLINT_TIMER
>  	bool "CLINT Timer for the RISC-V platform" if COMPILE_TEST
>  	depends on GENERIC_SCHED_CLOCK && RISCV
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 64ab547de97b..276695d95cdc 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -80,6 +80,7 @@ obj-$(CONFIG_INGENIC_TIMER)		+= ingenic-timer.o
>  obj-$(CONFIG_CLKSRC_ST_LPC)		+= clksrc_st_lpc.o
>  obj-$(CONFIG_X86_NUMACHIP)		+= numachip.o
>  obj-$(CONFIG_RISCV_TIMER)		+= timer-riscv.o
> +obj-$(CONFIG_STARFIVE_TIMER)		+= timer-starfive.o
>  obj-$(CONFIG_CLINT_TIMER)		+= timer-clint.o
>  obj-$(CONFIG_CSKY_MP_TIMER)		+= timer-mp-csky.o
>  obj-$(CONFIG_GX6605S_TIMER)		+= timer-gx6605s.o
> diff --git a/drivers/clocksource/timer-starfive.c b/drivers/clocksource/timer-starfive.c
> new file mode 100644
> index 000000000000..8db413a22fdc
> --- /dev/null
> +++ b/drivers/clocksource/timer-starfive.c
> @@ -0,0 +1,465 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Starfive Timer driver
> + *
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + *
> + * Author:
> + * Xingyu Wu <xingyu.wu@starfivetech.com>
> + * Samin Guo <samin.guo@starfivetech.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/clocksource.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_clk.h>
> +#include <linux/of_irq.h>
> +#include <linux/reset.h>
> +#include <linux/sched_clock.h>
> +
> +#include "timer-starfive.h"
> +
> +struct starfive_timer __initdata starfive_timer_jh7110 = {
> +	.ctrl		= STARFIVE_TIMER_JH7110_CTL,
> +	.load		= STARFIVE_TIMER_JH7110_LOAD,
> +	.enable		= STARFIVE_TIMER_JH7110_ENABLE,
> +	.reload		= STARFIVE_TIMER_JH7110_RELOAD,
> +	.value		= STARFIVE_TIMER_JH7110_VALUE,
> +	.intclr		= STARFIVE_TIMER_JH7110_INT_CLR,
> +	.intmask	= STARFIVE_TIMER_JH7110_INT_MASK,
> +	.timer_base	= {STARFIVE_TIMER_BASE(0), STARFIVE_TIMER_BASE(1),
> +			   STARFIVE_TIMER_BASE(2), STARFIVE_TIMER_BASE(3)},
> +};
> +
> +static inline struct starfive_clkevt *to_starfive_clkevt(struct clock_event_device *evt)
> +{
> +	return container_of(evt, struct starfive_clkevt, evt);
> +}
> +
> +/* 0:continuous-run mode, 1:single-run mode */
> +static inline void starfive_timer_set_mod(struct starfive_clkevt *clkevt, int mod)
> +{
> +	writel(mod, clkevt->ctrl);
> +}
> +
> +/* Interrupt Mask Register, 0:Unmask, 1:Mask */
> +static inline void starfive_timer_int_enable(struct starfive_clkevt *clkevt)
> +{
> +	writel(INTMASK_ENABLE_DIS, clkevt->intmask);
> +}
> +
> +static inline void starfive_timer_int_disable(struct starfive_clkevt *clkevt)
> +{
> +	writel(INTMASK_ENABLE, clkevt->intmask);
> +}
> +
> +/*
> + * BIT(0): Read value represent channel intr status.
> + * Write 1 to this bit to clear interrupt. Write 0 has no effects.
> + * BIT(1): "1" means that it is clearing interrupt. BIT(0) can not be written.
> + */
> +static inline void starfive_timer_int_clear(struct starfive_clkevt *clkevt)
> +{
> +	/* waiting interrupt can be to clearing */
> +	u32 value;
> +	int ret = 0;
> +
> +	value = readl(clkevt->intclr);
> +	ret = readl_poll_timeout_atomic(clkevt->intclr, value,
> +					!(value & STARFIVE_TIMER_JH7110_INT_STATUS_CLR_AVA),
> +					STARFIVE_DELAY_US, STARFIVE_TIMEOUT_US);
> +	if (!ret)
> +		writel(1, clkevt->intclr);
> +}
> +
> +/*
> + * The initial value to be loaded into the
> + * counter and is also used as the reload value.
> + */
> +static inline void starfive_timer_set_load(struct starfive_clkevt *clkevt, u32 val)
> +{
> +	writel(val, clkevt->load);
> +}
> +
> +static inline u32 starfive_timer_get_val(struct starfive_clkevt *clkevt)
> +{
> +	return readl(clkevt->value);
> +}
> +
> +/*
> + * Write RELOAD register to reload preset value to counter.
> + * (Write 0 and write 1 are both ok)
> + */
> +static inline void starfive_timer_set_reload(struct starfive_clkevt *clkevt)
> +{
> +	writel(1, clkevt->reload);
> +}
> +
> +static inline void starfive_timer_enable(struct starfive_clkevt *clkevt)
> +{
> +	writel(TIMER_ENA, clkevt->enable);
> +}
> +
> +static inline void starfive_timer_disable(struct starfive_clkevt *clkevt)
> +{
> +	writel(TIMER_ENA_DIS, clkevt->enable);
> +}
> +
> +static void timer_shutdown(struct starfive_clkevt *clkevt)
> +{
> +	starfive_timer_int_disable(clkevt);
> +	starfive_timer_disable(clkevt);
> +	starfive_timer_int_clear(clkevt);
> +}
> +
> +static void starfive_timer_suspend(struct clock_event_device *evt)
> +{
> +	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
> +
> +	clkevt->reload_val = starfive_timer_get_val(clkevt);
> +
> +	starfive_timer_disable(clkevt);
> +	starfive_timer_int_disable(clkevt);
> +	starfive_timer_int_clear(clkevt);
> +}
> +
> +static void starfive_timer_resume(struct clock_event_device *evt)
> +{
> +	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
> +
> +	starfive_timer_set_load(clkevt, clkevt->reload_val);
> +	starfive_timer_set_reload(clkevt);
> +	starfive_timer_int_enable(clkevt);
> +	starfive_timer_enable(clkevt);
> +}
> +
> +static int starfive_timer_tick_resume(struct clock_event_device *evt)
> +{
> +	starfive_timer_resume(evt);
> +
> +	return 0;
> +}
> +
> +static int starfive_timer_shutdown(struct clock_event_device *evt)
> +{
> +	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
> +
> +	timer_shutdown(clkevt);
> +
> +	return 0;
> +}
> +
> +static int starfive_get_clock_rate(struct starfive_clkevt *clkevt, struct device_node *np)
> +{
> +	int ret;
> +	u32 rate;
> +
> +	if (clkevt->clk) {
> +		clkevt->rate = clk_get_rate(clkevt->clk);
> +		if (clkevt->rate > 0) {
> +			pr_debug("clk_get_rate clkevt->rate: %d\n", clkevt->rate);
> +			return 0;
> +		}
> +	}
> +
> +	/* Next we try to get clock-frequency from dts.*/
> +	ret = of_property_read_u32(np, "clock-frequency", &rate);
> +	if (!ret) {
> +		pr_debug("Timer: try get clock-frequency:%d Hz\n", rate);
> +		clkevt->rate = rate;
> +		return 0;
> +	}
> +	pr_err("Timer: get rate failed, need clock-frequency define in dts.\n");
> +
> +	return -ENOENT;
> +}
> +
> +static int starfive_clocksource_init(struct starfive_clkevt *clkevt,
> +				     const char *name, struct device_node *np)
> +{
> +	starfive_timer_set_mod(clkevt, MOD_CONTIN);
> +	starfive_timer_set_load(clkevt, STARFIVE_MAX_TICKS);  /* val = rate --> 1s */
> +	starfive_timer_int_disable(clkevt);
> +	starfive_timer_int_clear(clkevt);
> +	starfive_timer_int_enable(clkevt);
> +	starfive_timer_enable(clkevt);
> +
> +	return clocksource_mmio_init(clkevt->value, name, clkevt->rate,
> +				     STARFIVE_CLOCK_SOURCE_RATING, STARFIVE_VALID_BITS,
> +				     clocksource_mmio_readl_down);
> +}
> +
> +/*
> + * IRQ handler for the timer
> + */
> +static irqreturn_t starfive_timer_interrupt(int irq, void *priv)
> +{
> +	struct clock_event_device *evt = (struct clock_event_device *)priv;
> +	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
> +
> +	starfive_timer_int_clear(clkevt);
> +
> +	if (evt->event_handler)
> +		evt->event_handler(evt);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int starfive_timer_set_periodic(struct clock_event_device *evt)
> +{
> +	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
> +
> +	starfive_timer_disable(clkevt);
> +	starfive_timer_set_mod(clkevt, MOD_CONTIN);
> +	starfive_timer_set_load(clkevt, clkevt->periodic);
> +	starfive_timer_int_disable(clkevt);
> +	starfive_timer_int_clear(clkevt);
> +	starfive_timer_int_enable(clkevt);
> +	starfive_timer_enable(clkevt);
> +
> +	return 0;
> +}
> +
> +static int starfive_timer_set_oneshot(struct clock_event_device *evt)
> +{
> +	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
> +
> +	starfive_timer_disable(clkevt);
> +	starfive_timer_set_mod(clkevt, MOD_SINGLE);
> +	starfive_timer_set_load(clkevt, STARFIVE_MAX_TICKS);
> +	starfive_timer_int_disable(clkevt);
> +	starfive_timer_int_clear(clkevt);
> +	starfive_timer_int_enable(clkevt);
> +	starfive_timer_enable(clkevt);
> +
> +	return 0;
> +}
> +
> +static int starfive_timer_set_next_event(unsigned long next,
> +					 struct clock_event_device *evt)
> +{
> +	struct starfive_clkevt *clkevt = to_starfive_clkevt(evt);
> +
> +	starfive_timer_disable(clkevt);
> +	starfive_timer_set_mod(clkevt, MOD_SINGLE);
> +	starfive_timer_set_load(clkevt, next);
> +	starfive_timer_enable(clkevt);
> +
> +	return 0;
> +}
> +
> +static void starfive_set_clockevent(struct clock_event_device *evt)
> +{
> +	evt->features	= CLOCK_EVT_FEAT_PERIODIC |
> +			  CLOCK_EVT_FEAT_ONESHOT |
> +			  CLOCK_EVT_FEAT_DYNIRQ;
> +	evt->set_state_shutdown	= starfive_timer_shutdown;
> +	evt->set_state_periodic	= starfive_timer_set_periodic;
> +	evt->set_state_oneshot	= starfive_timer_set_oneshot;
> +	evt->set_state_oneshot_stopped = starfive_timer_shutdown;
> +	evt->tick_resume	= starfive_timer_tick_resume;
> +	evt->set_next_event	= starfive_timer_set_next_event;
> +	evt->suspend		= starfive_timer_suspend;
> +	evt->resume		= starfive_timer_resume;
> +	evt->rating		= STARFIVE_CLOCKEVENT_RATING;
> +}
> +
> +static int starfive_clockevents_register(struct starfive_clkevt *clkevt, unsigned int irq,
> +					 struct device_node *np, const char *name)
> +{
> +	int ret = 0;
> +
> +	ret = starfive_get_clock_rate(clkevt, np);
> +	if (ret)
> +		return -EINVAL;
> +
> +	clkevt->periodic = DIV_ROUND_CLOSEST(clkevt->rate, HZ);
> +
> +	starfive_set_clockevent(&clkevt->evt);
> +	clkevt->evt.name = name;
> +	clkevt->evt.irq = irq;
> +	clkevt->evt.cpumask = cpu_possible_mask;
> +
> +	ret = request_irq(irq, starfive_timer_interrupt,
> +			  IRQF_TIMER | IRQF_IRQPOLL, name, &clkevt->evt);
> +	if (ret)
> +		pr_err("%s: request_irq failed\n", name);
> +
> +	clockevents_config_and_register(&clkevt->evt, clkevt->rate,
> +					STARFIVE_MIN_TICKS, STARFIVE_MAX_TICKS);
> +
> +	return ret;
> +}
> +
> +static void __init starfive_clkevt_base_init(struct starfive_timer *timer,
> +					     struct starfive_clkevt *clkevt,
> +					     void __iomem *base, int index)
> +{
> +	void __iomem *timer_base;
> +
> +	timer_base	= base + timer->timer_base[index];
> +	clkevt->base	= timer_base;
> +	clkevt->ctrl	= timer_base + timer->ctrl;
> +	clkevt->load	= timer_base + timer->load;
> +	clkevt->enable	= timer_base + timer->enable;
> +	clkevt->reload	= timer_base + timer->reload;
> +	clkevt->value	= timer_base + timer->value;
> +	clkevt->intclr	= timer_base + timer->intclr;
> +	clkevt->intmask	= timer_base + timer->intmask;
> +}
> +
> +static int __init starfive_timer_jh7110_of_init(struct device_node *np)
> +{
> +	int index, count, irq, ret;
> +	const char *name = NULL;
> +	struct clk *clk;
> +	struct clk *pclk;
> +	struct reset_control *prst;
> +	struct reset_control *rst;
> +	struct starfive_clkevt *clkevt[STARFIVE_NR_TIMERS];
> +	void __iomem *base;
> +	struct starfive_timer *timer = &starfive_timer_jh7110;
> +
> +	base = of_iomap(np, 0);
> +	if (!base)
> +		return -ENXIO;
> +
> +	if (!of_device_is_available(np)) {
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	pclk = of_clk_get_by_name(np, "apb");
> +	if (!IS_ERR(pclk)) {
> +		if (clk_prepare_enable(pclk))
> +			pr_warn("pclk for %pOFn is present, but could not be activated\n", np);
> +	/*
> +	 * Clock framework support is late, continue on
> +	 * anyways if we don't find a matching clock.
> +	 */
> +	} else if (PTR_ERR(pclk) != -EPROBE_DEFER) {
> +		ret = PTR_ERR(pclk);
> +		goto err;
> +	}
> +
> +	prst = of_reset_control_get(np, "apb");
> +	if (!IS_ERR(prst)) {
> +		ret = reset_control_deassert(prst);
> +		if (ret)
> +			goto prst_err;
> +	/*
> +	 * Reset framework support is late, continue on
> +	 * anyways if we don't find a matching reset.
> +	 */
> +	} else if (PTR_ERR(prst) != -EPROBE_DEFER) {
> +		ret = PTR_ERR(prst);
> +		goto prst_err;
> +	}
> +
> +	/* The number of timers used is determined according to the device tree. */
> +	count = of_irq_count(np);
> +	if (count > STARFIVE_NR_TIMERS || count <= 0) {
> +		ret = -EINVAL;
> +		goto count_err;
> +	}
> +
> +	for (index = 0; index < count; index++) {
> +		of_property_read_string_index(np, "clock-names", index, &name);
> +		if (strncmp(name, "timer", strlen("timer")))
> +			continue;
> +
> +		clkevt[index] = kzalloc(sizeof(*clkevt[index]), GFP_KERNEL);
> +		if (!clkevt[index]) {
> +			ret = -ENOMEM;
> +			goto clkevt_err;
> +		}
> +
> +		starfive_clkevt_base_init(timer, clkevt[index], base, index);
> +
> +		/* Ensure timers are disabled */
> +		starfive_timer_disable(clkevt[index]);
> +
> +		clk = of_clk_get_by_name(np, name);
> +		if (!IS_ERR(clk)) {
> +			clkevt[index]->clk = clk;
> +			if (clk_prepare_enable(clkevt[index]->clk))
> +				pr_warn("clk for %pOFn is present, but could not be activated\n",
> +					np);
> +		} else if (PTR_ERR(clk) != -EPROBE_DEFER) {
> +			ret = PTR_ERR(clk);
> +			goto clk_err;
> +		}
> +
> +		rst = of_reset_control_get(np, name);
> +		if (!IS_ERR(rst)) {
> +			clkevt[index]->rst = rst;
> +			ret = reset_control_deassert(clkevt[index]->rst);
> +			if (ret)
> +				goto rst_err;
> +		}
> +
> +		irq = irq_of_parse_and_map(np, index);
> +		if (irq < 0) {
> +			ret = -EINVAL;
> +			goto irq_err;
> +		}
> +
> +		snprintf(clkevt[index]->name, sizeof(clkevt[index]->name), "%s.ch%d",
> +			 np->full_name, index);
> +
> +		ret = starfive_clockevents_register(clkevt[index], irq, np, clkevt[index]->name);
> +		if (ret) {
> +			pr_err("%s: init clockevents failed.\n", clkevt[index]->name);
> +			goto register_err;
> +		}
> +		clkevt[index]->irq = irq;
> +
> +		ret = starfive_clocksource_init(clkevt[index], clkevt[index]->name, np);
> +		if (ret)
> +			goto init_err;
> +	}
> +	if (!IS_ERR(pclk))
> +		clk_put(pclk);
> +
> +	return 0;
> +
> +init_err:
> +register_err:
> +	free_irq(clkevt[index]->irq, &clkevt[index]->evt);
> +irq_err:
> +rst_err:
> +clk_err:
> +	/* Only unregister the failed timer and the rest timers continue to work. */
> +	if (!clkevt[index]->rst) {
> +		reset_control_assert(clkevt[index]->rst);
> +		reset_control_put(clkevt[index]->rst);
> +	}
> +	if (!clkevt[index]->clk) {
> +		clk_disable_unprepare(clkevt[index]->clk);
> +		clk_put(clkevt[index]->clk);
> +	}
> +	kfree(clkevt[index]);
> +clkevt_err:
> +count_err:
> +prst_err:
> +	if (!IS_ERR(pclk)) {
> +		/* If no other timer successfully registers, pclk is disabled. */
> +		if (!index)
> +			clk_disable_unprepare(pclk);
> +		clk_put(pclk);
> +	}
> +err:
> +	iounmap(base);
> +	return ret;
> +}
> +
> +TIMER_OF_DECLARE(starfive_timer_jh7110, "starfive,jh7110-timers", starfive_timer_jh7110_of_init);
> diff --git a/drivers/clocksource/timer-starfive.h b/drivers/clocksource/timer-starfive.h
> new file mode 100644
> index 000000000000..9fd3303b31d1
> --- /dev/null
> +++ b/drivers/clocksource/timer-starfive.h
> @@ -0,0 +1,104 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + */
> +
> +#ifndef __STARFIVE_TIMER_H__
> +#define __STARFIVE_TIMER_H__
> +
> +#define STARFIVE_NR_TIMERS		TIMERS_MAX
> +/* Bias: Timer0-0x0, Timer1-0x40, Timer2-0x80, and so on. */
> +#define STARFIVE_PER_TIMER_LEN		0x40
> +#define STARFIVE_TIMER_BASE(x)		((TIMER_##x) * STARFIVE_PER_TIMER_LEN)
> +
> +#define STARFIVE_CLOCK_SOURCE_RATING	200
> +#define STARFIVE_VALID_BITS		32
> +#define STARFIVE_DELAY_US		0
> +#define STARFIVE_TIMEOUT_US		10000
> +#define STARFIVE_CLOCKEVENT_RATING	300
> +#define STARFIVE_MAX_TICKS		0xffffffff
> +#define STARFIVE_MIN_TICKS		0xf
> +
> +/*
> + * JH7110 timer TIMER_INT_STATUS:
> + * ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> + * |     Bits     | 08~31 | 7 | 6 | 5 |  4  | 3 | 2 | 1 | 0 |
> + * ----------------------------------------------------------
> + * | timer(n)_int |  res  | 6 | 5 | 4 | Wdt | 3 | 2 | 1 | 0 |
> + * ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> + *
> + * Software can read this register to know which interrupt is occurred.
> + */
> +#define STARFIVE_TIMER_JH7110_INT_STATUS	0x00
> +#define STARFIVE_TIMER_JH7110_CTL		0x04
> +#define STARFIVE_TIMER_JH7110_LOAD		0x08
> +#define STARFIVE_TIMER_JH7110_ENABLE		0x10
> +#define STARFIVE_TIMER_JH7110_RELOAD		0x14
> +#define STARFIVE_TIMER_JH7110_VALUE		0x18
> +#define STARFIVE_TIMER_JH7110_INT_CLR		0x20
> +#define STARFIVE_TIMER_JH7110_INT_MASK		0x24
> +#define STARFIVE_TIMER_JH7110_INT_STATUS_CLR_AVA	BIT(1)
> +
> +enum STARFIVE_TIMERS {
> +	TIMER_0 = 0,
> +	TIMER_1,
> +	TIMER_2,
> +	TIMER_3,
> +	TIMER_4,  /*WDT*/
> +	TIMER_5,
> +	TIMER_6,
> +	TIMER_7,
> +	TIMERS_MAX
> +};
> +
> +enum TIMERI_INTMASK {
> +	INTMASK_ENABLE_DIS = 0,
> +	INTMASK_ENABLE = 1
> +};
> +
> +enum TIMER_MOD {
> +	MOD_CONTIN = 0,
> +	MOD_SINGLE = 1
> +};
> +
> +enum TIMER_CTL_EN {
> +	TIMER_ENA_DIS	= 0,
> +	TIMER_ENA	= 1
> +};
> +
> +enum {
> +	INT_CLR_AVAILABLE = 0,
> +	INT_CLR_NOT_AVAILABLE = 1
> +};
> +
> +struct starfive_timer {
> +	u32 ctrl;
> +	u32 load;
> +	u32 enable;
> +	u32 reload;
> +	u32 value;
> +	u32 intclr;
> +	u32 intmask;
> +	u32 wdt_lock;   /* 0x3c+i*0x40 watchdog use ONLY */
> +	u32 timer_base[STARFIVE_NR_TIMERS];
> +};
> +
> +struct starfive_clkevt {
> +	struct clock_event_device evt;
> +	struct clk *clk;
> +	struct reset_control *rst;
> +	char name[20];
> +	int irq;
> +	u32 periodic;
> +	u32 rate;
> +	u32 reload_val;
> +	void __iomem *base;
> +	void __iomem *ctrl;
> +	void __iomem *load;
> +	void __iomem *enable;
> +	void __iomem *reload;
> +	void __iomem *value;
> +	void __iomem *intclr;
> +	void __iomem *intmask;
> +};
> +#endif /* __STARFIVE_TIMER_H__ */

Hi Daniel/Thomas,

Could you please help to review and provide any comments on this patch?
Thanks.

Best regards,
Xingyu Wu


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] clocksource: Add StarFive timer driver
  2022-12-23  9:48 ` [PATCH v1 2/3] clocksource: Add StarFive timer driver Xingyu Wu
  2023-01-17  6:39   ` Xingyu Wu
@ 2023-03-02  9:39   ` Andreas Schwab
  2023-03-07  3:47     ` Xingyu Wu
  1 sibling, 1 reply; 10+ messages in thread
From: Andreas Schwab @ 2023-03-02  9:39 UTC (permalink / raw)
  To: Xingyu Wu
  Cc: linux-riscv, devicetree, Daniel Lezcano, Thomas Gleixner,
	Krzysztof Kozlowski, Rob Herring, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Philipp Zabel, Samin Guo, linux-kernel

drivers/clocksource/timer-starfive.c:117:13: error: conflicting types for 'timer_shutdown'; have 'void(struct starfive_clkevt *)'
  117 | static void timer_shutdown(struct starfive_clkevt *clkevt)
      |             ^~~~~~~~~~~~~~
In file included from ./include/linux/workqueue.h:9,
                 from ./include/linux/srcu.h:21,
                 from ./include/linux/notifier.h:16,
                 from ./include/linux/clk.h:14,
                 from drivers/clocksource/timer-starfive.c:12:
./include/linux/timer.h:188:12: note: previous declaration of 'timer_shutdown' with type 'int(struct timer_list *)'
  188 | extern int timer_shutdown(struct timer_list *timer);
      |            ^~~~~~~~~~~~~~

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] clocksource: Add StarFive timer driver
  2023-03-02  9:39   ` Andreas Schwab
@ 2023-03-07  3:47     ` Xingyu Wu
  0 siblings, 0 replies; 10+ messages in thread
From: Xingyu Wu @ 2023-03-07  3:47 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: linux-riscv, devicetree, Daniel Lezcano, Thomas Gleixner,
	Krzysztof Kozlowski, Rob Herring, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Philipp Zabel, Samin Guo, linux-kernel

On 2023/3/2 17:39, Andreas Schwab wrote:
> drivers/clocksource/timer-starfive.c:117:13: error: conflicting types for 'timer_shutdown'; have 'void(struct starfive_clkevt *)'
>   117 | static void timer_shutdown(struct starfive_clkevt *clkevt)
>       |             ^~~~~~~~~~~~~~
> In file included from ./include/linux/workqueue.h:9,
>                  from ./include/linux/srcu.h:21,
>                  from ./include/linux/notifier.h:16,
>                  from ./include/linux/clk.h:14,
>                  from drivers/clocksource/timer-starfive.c:12:
> ./include/linux/timer.h:188:12: note: previous declaration of 'timer_shutdown' with type 'int(struct timer_list *)'
>   188 | extern int timer_shutdown(struct timer_list *timer);
>       |            ^~~~~~~~~~~~~~
> 

Oh, this timer_shutdown(struct timer_list *timer) function was not available
in 6.1-rc5 which this patchset was base on. And in the new version, it's the
same name as my own function. I will change the name of this function.

Best regards,
Xingyu Wu

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-03-07  3:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-23  9:47 [PATCH v1 0/3] Add timer driver for StarFive JH7110 RISC-V SoC Xingyu Wu
2022-12-23  9:47 ` [PATCH v1 1/3] dt-bindings: timer: Add timer for StarFive JH7110 SoC Xingyu Wu
2022-12-23 10:25   ` Krzysztof Kozlowski
2023-01-10  2:14     ` Xingyu Wu
2023-01-10  8:24       ` Krzysztof Kozlowski
2022-12-23  9:48 ` [PATCH v1 2/3] clocksource: Add StarFive timer driver Xingyu Wu
2023-01-17  6:39   ` Xingyu Wu
2023-03-02  9:39   ` Andreas Schwab
2023-03-07  3:47     ` Xingyu Wu
2022-12-23  9:48 ` [PATCH v1 3/3] riscv: dts: jh7110: starfive: Add timer node Xingyu Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).