Skip to contentSkip to conversation

timers: Add support for RZ/N1 SoCs timers

PATCH Superseded Herve Codina (Schneider Electric) 2 participants 6 messages Sep 29

Subsystems (4)

v6 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v6 cover timers: Add support for RZ/N1 SoCs timers

The Renesas RZ/N1 SoCs family has two timers block controller available.

Each controller is charge of 8 timers (six 16-bit timers + two 32-bit timers).

Each timer has its own interrupt, its own prescaler that can be used to divide the clock by 25 (timers block are fed with a 25 MHz clock) and all timers can work in either one-shot or periodic mode.

This series adds support for those timers blocks.

Best regards,
Hervé

Changes v5 -> v6
v5: https://lore.kernel.org/all/[email protected]/

Patch 2:
- Fix a comment
- Add 'Tested-by: Wolfram Sang'

Patch 1, 3 and 4:
No changes

Changes v4 -> v5
v4: https://lore.kernel.org/all/[email protected]/

Rebase on top of v7.3-rc1

Wolfram sent his 'Tested-by' tag for patch 2 in the v4 iteration but because of timers re-assignment done this v5, I haven't add his tag in patch 2.

Patch 1:
- Reduce the reg area size
- Add 'Reviewed-by: Wolfram Sang'

Patch 2:
- Take into account Shashiko's feedback and avoid paths which could lead to a NULL pointer dereference in the interrupt handle. Even if these paths cannot be reached with our hardware timers, remove them from the driver.
- Reassign timers usage for the first timers instance probed:
* First and second 16-bit timers: clock events per CPU
* Last (second one) 32-bit timer: clocksource and sched_clock
* Other timers: clock events for all possible CPUs

Patch 3:
- Reduce the reg area size
- Remove some empty lines in commit log
- Add 'Reviewed-by: Wolfram Sang'
- Add 'Tested-by: Wolfram Sang'

Patch 4:
- Add 'Reviewed-by: Wolfram Sang'.

Changes v3 -> v4
v3: https://lore.kernel.org/all/[email protected]/

Patches 1, 3 and 4:
No changes.

Patch 2:
Took into account Shashiko's feedback
- Remove CLOCK_SOURCE_SUSPEND_NONSTOP flag
- Fix CPU ids checks against number of 32-bit timers (avoid out-of-bound accesses)
- Enable IRQ in CPU hotpluig startup function
- Add CPU hotplug teardown function and disable IRQ in this function.
- Adapt probe() to avoid IRQ handled by CPU hotplug startup/teardown been enabled by default.

Changes v2 -> v3
v2: https://lore.kernel.org/all/[email protected]/

Rebase on top of v7.2-rc1.

Patch 1, 3 and 4:
- Add 'Reviewed-by: Geert Uytterhoeven'

Patch 2:
- Fix typos in commit log.
- Split Kconfig 'depends on' in 2 lines.
- Fix indentation, space and alignment in code.
- Fix dev_err_probe() conversion format.
- Introduce rzn1_timer_setup_timers() and rzn1_timer_register_clkevt() to avoid code duplication in rzn1_timer_probe_{first,other}()
- Add '.suppress_bind_attrs = true'.

Changes v1 -> v2
v1: https://lore.kernel.org/all/[email protected]/

Rebase on top of v7.1-rc1.

Patch 1:
- Drop minItems related to interrupts property
- Add 'Reviewed-by: Krzysztof Kozlowski'

Patches 2, 3 and 4:
No changes

Herve Codina (Schneider Electric) (4):
dt-bindings: timer: Add the Renesas RZ/N1 timer clocksource/drivers: Add support for the Renesas RZ/N1 timers
ARM: dts: r9a06g032: Add support for timers
MAINTAINERS: Add the Renesas RZ/N1 timers driver entry

 .../bindings/timer/renesas,rzn1-timer.yaml    |  74 +++
 MAINTAINERS                                   |   7 +
 arch/arm/boot/dts/renesas/r9a06g032.dtsi      |  34 ++
 drivers/clocksource/Kconfig                   |  11 +
 drivers/clocksource/Makefile                  |   1 +
 drivers/clocksource/timer-rzn1.c              | 481 ++++++++++++++++++
 6 files changed, 608 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/renesas,rzn1-timer.yaml
 create mode 100644 drivers/clocksource/timer-rzn1.c
Signature · 1 line
2.55.0
#1 lore raw

PATCH v6 1/4 dt-bindings: timer: Add the Renesas RZ/N1 timer

The Renesas RZ/N1 timer block controller is the controller in charge of timers available in the Renesas RZ/N1 SoCs family.

This controller handles 8 timers:
- 6 16-bit timers
- 2 32-bit timers

---
 .../bindings/timer/renesas,rzn1-timer.yaml    | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/renesas,rzn1-timer.yaml
Documentation/devicetree/bindings/timer/renesas,rzn1-timer.yaml new file +74 -0
new file mode 100644
index 000000000000..7218c0c2671d
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/renesas,rzn1-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/N1 timers
+
+maintainers:
+  - Herve Codina <[email protected]>
+
+description: |
+  The Renesas RZ/N1 SoCs timers block controller is composed of 8 independent
+  timers.
+    - 6 are 16-bit timers
+    - 2 are 32-bit timers
+
+  Each timer has its own interrupt line and can work in either one-shot or
+  periodic mode.
+
+properties:
+  compatible:
+    items:
+      - const: renesas,r9a06g032-timer   # RZ/N1D
+      - const: renesas,rzn1-timer
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: APB internal bus clock
+
+  clock-names:
+    items:
+      - const: pclk
+
+  power-domains:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 8
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - power-domains
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/r9a06g032-sysctrl.h>
+
+    timer@51001000 {
+        compatible = "renesas,r9a06g032-timer", "renesas,rzn1-timer";
+        reg = <0x51001000 0x200>;
+        clocks = <&sysctrl R9A06G032_HCLK_TIMER0>;
+        clock-names = "pclk";
+        power-domains = <&sysctrl>;
+        interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+    };
+...
Signature · 1 line
2.55.0
#2 lore raw

PATCH v6 2/4 clocksource/drivers: Add support for the Renesas RZ/N1 timers

In reply to Herve Codina (Schneider Electric)

The Renesas RZ/N1 timer block controller is the controller in charge of timers available in the Renesas RZ/N1 SoCs family.

This controller handles 8 timers:
- 6 16-bit timers
- 2 32-bit timers

Each timer has its own interrupt, its own prescaler that can be used to divide the clock by 25 and all of them can work in either one-shot or periodic mode.

---
 drivers/clocksource/Kconfig      |  11 +
 drivers/clocksource/Makefile     |   1 +
 drivers/clocksource/timer-rzn1.c | 481 +++++++++++++++++++++++++++++++
 3 files changed, 493 insertions(+)
 create mode 100644 drivers/clocksource/timer-rzn1.c
drivers/clocksource/Kconfig +11 -0
index d9c76dd443f8..da83483296d5 100644
@@ -152,6 +152,17 @@ config REALTEK_OTTO_TIMER
 	  RT8391, RTL8392, RTL8393 and RTL8396 and chips of the RTL930x series
 	  such as RTL9301, RTL9302 or RTL9303.
 
+config RZN1_TIMER
+	bool "Renesas RZ/N1 Timer"
+	depends on HAS_IOMEM && COMMON_CLK
+	depends on ARCH_RZN1 || COMPILE_TEST
+	help
+	  Enables support for RZ/N1 SoC timers.
+	  A timers block in RZ/N1 SoCs is composed of 8 timers
+	    - 6 16-bit timers
+	    - 2 32-bit timers
+	  Two timers blocks are available in RZ/N1 SoCs.
+
 config SUN4I_TIMER
 	bool "Sun4i timer driver" if COMPILE_TEST
 	depends on HAS_IOMEM
drivers/clocksource/Makefile +1 -0
index b46376af6b49..464ec6690ab6 100644
@@ -96,3 +96,4 @@ obj-$(CONFIG_EP93XX_TIMER)		+= timer-ep93xx.o
 obj-$(CONFIG_RALINK_TIMER)		+= timer-ralink.o
 obj-$(CONFIG_NXP_STM_TIMER)		+= timer-nxp-stm.o
 obj-$(CONFIG_RTK_SYSTIMER)		+= timer-realtek.o
+obj-$(CONFIG_RZN1_TIMER)		+= timer-rzn1.o
drivers/clocksource/timer-rzn1.c new file +481 -0

481 lines not on this page · raw message

Signature · 1 line
2.55.0
#3 lore raw

Re: [PATCH v6 2/4] clocksource/drivers: Add support for the Renesas RZ/N1 timers

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] timer: rzn1: NULL pointer deref in irq handler for detached timers --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] timer: rzn1: NULL pointer deref in irq handler for detached timers --

commit c504b8f1cef80ec82616cd19b137210a244f7ec4
Author: Herve Codina (Schneider Electric) <[email protected]>

clocksource/drivers: Add support for the Renesas RZ/N1 timers

The Renesas RZ/N1 timer block controller is the controller in charge of timers available in the Renesas RZ/N1 SoCs family.

Quoted 5 lines
drivers/clocksource/timer-rzn1.c new file +0 -0
new file mode 100644
index 0000000000000..3b173b4086e92

[ ... ]

drivers/clocksource/timer-rzn1.c line 118

Quoted 8 lines
drivers/clocksource/timer-rzn1.c new file +8 -0
+static irqreturn_t rzn1_timer_interrupt(int irq, void *dev_id)
+{
+	struct rzn1_timer *timer = dev_id;
+
+	/* Ack the interrupt */
+	readl(timer->base + RZN1_TIMER_CLEAR_INT_REG);
+
+	timer->ced.event_handler(&timer->ced);

[Severity: High]
Does this code risk a NULL pointer dereference if the timer is detached?

The driver unconditionally registers multiple clockevent devices and their IRQs. The clockevents core leaves unused timers in CLOCK_EVT_STATE_DETACHED, which means their event_handler remains NULL.

Quoted 2 lines
drivers/clocksource/timer-rzn1.c new file +2 -0
+	return IRQ_HANDLED;
+}

[ ... ]

Quoted 7 lines
drivers/clocksource/timer-rzn1.c new file +7 -0
+static void rzn1_timer_register_clkevt(struct rzn1_timer *tab_timers,
+				       unsigned int first_timer_index,
+				       unsigned int last_timer_index)
+{
+	struct rzn1_timer *timer;
+	unsigned int i;
+

drivers/clocksource/timer-rzn1.c line 324

Quoted 10 lines
drivers/clocksource/timer-rzn1.c new file +10 -0
+	for (i = first_timer_index; i <= last_timer_index; i++) {
+		timer = &tab_timers[i];
+		clockevents_config_and_register(&timer->ced, timer->rate,
+						1, GENMASK(timer->width - 1, 0));
+		/*
+		 * Time to enable the interrupt even if no interrupt can be
+		 * fired yet. Indeed, the timer is still globally disabled.
+		 * It will be enabled when rzn1_timer_config() is called
+		 */
+		enable_irq(timer->ced.irq);

[Severity: High]
Can enabling the IRQ unconditionally here trigger the NULL pointer dereference above?

If the clockevents core leaves a timer unused and we enable its IRQ anyway, a spurious interrupt on this hardware line will cause the interrupt handler to blindly call timer->ced.event_handler and crash.

Quoted 2 lines
drivers/clocksource/timer-rzn1.c new file +2 -0
+	}
+}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#4 lore raw

PATCH v6 3/4 ARM: dts: r9a06g032: Add support for timers

In reply to Herve Codina (Schneider Electric)

In the Renesas RZ/N1 SoCs family, two timers block are available. Each block contains 8 timers composed of 6 16-bits timers and 2 32-bits timers. Each timer has its own interrupt line.

Describe those timers blocks.

---
 arch/arm/boot/dts/renesas/r9a06g032.dtsi | 34 ++++++++++++++++++++++++
 1 file changed, 34 insertions(+)
arch/arm/boot/dts/renesas/r9a06g032.dtsi +34 -0
index 19c9bce0a26d..6fae6db2850e 100644
@@ -806,6 +806,40 @@ gpioirqmux: interrupt-controller@51000480 {
 			status = "disabled";
 		};
 
+		timer0: timer@51001000 {
+			compatible = "renesas,r9a06g032-timer", "renesas,rzn1-timer";
+			reg = <0x51001000 0x200>;
+			clocks = <&sysctrl R9A06G032_HCLK_TIMER0>;
+			clock-names = "pclk";
+			power-domains = <&sysctrl>;
+			interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		timer1: timer@51002000 {
+			compatible = "renesas,r9a06g032-timer", "renesas,rzn1-timer";
+			reg = <0x51002000 0x200>;
+			clocks = <&sysctrl R9A06G032_HCLK_TIMER1>;
+			clock-names = "pclk";
+			power-domains = <&sysctrl>;
+			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
 		can0: can@52104000 {
 			compatible = "renesas,r9a06g032-sja1000", "renesas,rzn1-sja1000";
 			reg = <0x52104000 0x800>;
Signature · 1 line
2.55.0
#5 lore raw

PATCH v6 4/4 MAINTAINERS: Add the Renesas RZ/N1 timers driver entry

In reply to Herve Codina (Schneider Electric)

After contributing the driver, add myself as the maintainer for the Renesas RZ/N1 timers driver.

---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)
MAINTAINERS +7 -0
index 3a19da74d00c..91b014d27577 100644
@@ -23296,6 +23296,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
 F:	drivers/rtc/rtc-rzn1.c
 
+RENESAS RZ/N1 TIMERS DRIVER
+M:	Herve Codina <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/timer/renesas,rzn1-timer.yaml
+F:	drivers/clocksource/timer-rzn1.c
+
 RENESAS RZ/N1 USBF CONTROLLER DRIVER
 M:	Herve Codina <[email protected]>
 L:	[email protected]
Signature · 1 line
2.55.0