[PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Biju Das]

This patch series aims to add support for Compare-Match Timer (TIM)
module found on RZ/V2M SoC.

it is composed of 32 channels and channels 0-7 and 24-32 are
reserved for ISP usage.

Channel 22 is modelled as clock source and Channel 23 is modelled as clock
event driver and the rest of the channels are modelled as counter driver
as it provides

1) counter for counting
2) configurable counter value for generating timer interrupt
3) userspace event for each interrupt.

logs:-
Counter driver:
Counter driver is tested by reading counts and interrupts tested by
counter-example in tools/counter/counter_example.c

Count snapshot value:
3114
Output from counter_example when it triggers interrupts:
Timestamp 0: 24142152969        Count 0: 5
Error Message 0: Success

Clock source:
Clock source driver is tested by clock-source-switch app.
[ 1275.703567] clocksource: Switched to clocksource arch_sys_counter
[ 1275.710189] clocksource: Switched to clocksource a4000b00.timer
[OK]
# Totals: pass[ 1275.718112] clocksource: Switched to clocksource arch_sys_counter
:0 fail:0 xfail:0 xpass:0 skip:0 error:0
# Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0

Clock event:
clock event driver is tested by changing the rating as 500.

Biju Das (6):
  clk: renesas: r9a09g011: Add TIM clock and reset entries
  dt-bindings: timer: Add RZ/V2M TIM binding
  clocksource/drivers/rzv2m-tim: Add Renesas RZ/V2M compare match
    timer(TIM) driver
  dt-bindings: counter: Add RZ/V2M TIM counter binding
  counter: Add Renesas RZ/V2M TIM counter driver
  arm64: dts: renesas: r9a09g011: Add tim nodes

 .../counter/renesas,rzv2m-tim-cnt.yaml        |  83 +++++
 .../bindings/timer/renesas,rzv2m-tim.yaml     |  83 +++++
 arch/arm64/boot/dts/renesas/r9a09g011.dtsi    | 192 ++++++++++
 drivers/clk/renesas/r9a09g011-cpg.c           |  22 ++
 drivers/clocksource/Kconfig                   |   7 +
 drivers/clocksource/Makefile                  |   1 +
 drivers/clocksource/rzv2m-tim.c               | 330 ++++++++++++++++++
 drivers/counter/Kconfig                       |  10 +
 drivers/counter/Makefile                      |   1 +
 drivers/counter/rzv2m-tim-cnt.c               | 312 +++++++++++++++++
 10 files changed, 1041 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/counter/renesas,rzv2m-tim-cnt.yaml
 create mode 100644 Documentation/devicetree/bindings/timer/renesas,rzv2m-tim.yaml
 create mode 100644 drivers/clocksource/rzv2m-tim.c
 create mode 100644 drivers/counter/rzv2m-tim-cnt.c

-- 
2.25.1

[PATCH 2/6] dt-bindings: timer: Add RZ/V2M TIM binding

[Biju Das]

Add device tree bindings for the RZ/V2{M, MA} Compare Match Timer
(TIM).

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../bindings/timer/renesas,rzv2m-tim.yaml     | 83 +++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/renesas,rzv2m-tim.yaml

diff --git a/Documentation/devicetree/bindings/timer/renesas,rzv2m-tim.yaml b/Documentation/devicetree/bindings/timer/renesas,rzv2m-tim.yaml
new file mode 100644
index 000000000000..9d692725bcbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/renesas,rzv2m-tim.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/renesas,rzv2m-tim.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/V2M Compare Match Timer (TIM)
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  The Compare Match Timer(TIM) on RZ/V2M like SoCs has an internal 32-bit
+  counter that can be used as an interval timer. This LSI has a total of 32
+  channels of TIM from ch. 0 to ch. 31. It supports the following features
+  * Configured with a 32-bit counter operating at INCLOCK (2 MHz)
+  * The clock input from the count clock input pin can be divided by 2, 4,
+    8, 16, 32, 64, 128, or 256, and one of these divided clocks can be
+    used as the count clock.
+  * The counter period can be set in the range of 1 to 4294967296
+    (32-bit timer) using the selected divider clock as the count clock.
+  * Generates an interrupt request signal every cycle set in the TIM
+    counter.
+  * The counter operation and the bus interface are asynchronous and
+    can operate independently regardless of the size of the respective
+    clock cycles.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a09g011-tim  # RZ/V2M
+          - renesas,r9a09g055-tim  # RZ/V2MA
+      - const: renesas,rzv2m-tim
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: APB clock
+      - description: TIM clock
+
+  clock-names:
+    items:
+      - const: apb
+      - const: tim
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a09g011-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    tim22: tim@a4000b00 {
+        compatible = "renesas,r9a09g011-tim", "renesas,rzv2m-tim";
+        reg = <0xa4000b00 0x80>;
+        interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPC_PCLK>,
+                 <&cpg CPG_MOD R9A09G011_TIM22_CLK>;
+        clock-names = "apb", "tim";
+        power-domains = <&cpg>;
+        resets = <&cpg R9A09G011_TIM_GPC_PRESETN>;
+    };
-- 
2.25.1

[PATCH 4/6] dt-bindings: counter: Add RZ/V2M TIM counter binding

[Biju Das]

Add device tree binding for the Renesas RZ/V2M Counter Match Timer
(a.k.a TIM).

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../counter/renesas,rzv2m-tim-cnt.yaml        | 83 +++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/counter/renesas,rzv2m-tim-cnt.yaml

diff --git a/Documentation/devicetree/bindings/counter/renesas,rzv2m-tim-cnt.yaml b/Documentation/devicetree/bindings/counter/renesas,rzv2m-tim-cnt.yaml
new file mode 100644
index 000000000000..963dffe1c957
--- /dev/null
+++ b/Documentation/devicetree/bindings/counter/renesas,rzv2m-tim-cnt.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/counter/renesas,rzv2m-tim-cnt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/V2M Compare Match Timer (TIM)
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  The Compare Match Timer(TIM) on RZ/V2M like SoCs has an internal 32-bit
+  counter that can be used as an interval timer. This LSI has a total of 32
+  channels of TIM from ch. 0 to ch. 31. It supports the following features
+  * Configured with a 32-bit counter operating at INCLOCK (2 MHz)
+  * The clock input from the count clock input pin can be divided by 2, 4,
+    8, 16, 32, 64, 128, or 256, and one of these divided clocks can be
+    used as the count clock.
+  * The counter period can be set in the range of 1 to 4294967296
+    (32-bit timer) using the selected divider clock as the count clock.
+  * Generates an interrupt request signal every cycle set in the TIM
+    counter.
+  * The counter operation and the bus interface are asynchronous and
+    can operate independently regardless of the size of the respective
+    clock cycles.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a09g011-tim-cnt  # RZ/V2M
+          - renesas,r9a09g055-tim-cnt  # RZ/V2MA
+      - const: renesas,rzv2m-tim-cnt
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: APB clock
+      - description: TIM clock
+
+  clock-names:
+    items:
+      - const: apb
+      - const: tim
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a09g011-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    tim8: tim@a4000400 {
+        compatible = "renesas,r9a09g011-tim-cnt", "renesas,rzv2m-tim-cnt";
+        reg = <0xa4000400 0x80>;
+        interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPB_PCLK>,
+                 <&cpg CPG_MOD R9A09G011_TIM8_CLK>;
+        clock-names = "apb", "tim";
+        power-domains = <&cpg>;
+        resets = <&cpg R9A09G011_TIM_GPB_PRESETN>;
+    };
-- 
2.25.1

[PATCH 6/6] arm64: dts: renesas: r9a09g011: Add tim nodes

[Biju Das]

Add device nodes for the compare match timer(TIM) channels that are
not assigned to the ISP.

The channels 22 is assigned for clock source and channel 23 for
clock event and rest of the channels are assigned for counter
operation.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a09g011.dtsi | 192 +++++++++++++++++++++
 1 file changed, 192 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g011.dtsi b/arch/arm64/boot/dts/renesas/r9a09g011.dtsi
index dcd3a05e54fe..69c1ebc5e0dd 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g011.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g011.dtsi
@@ -135,6 +135,198 @@ sys: system-controller@a3f03000 {
 			reg = <0 0xa3f03000 0 0x400>;
 		};
 
+		tim8: timer@a4000400 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000400 0 0x80>;
+			interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPB_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM8_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPB_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim9: timer@a4000480 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000480 0 0x80>;
+			interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPB_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM9_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPB_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim10: timer@a4000500 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000500 0 0x80>;
+			interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPB_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM10_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPB_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim11: timer@a4000580 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000580 0 0x80>;
+			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPB_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM11_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPB_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim12: timer@a4000600 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000600 0 0x80>;
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPB_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM12_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPB_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim13: timer@a4000680 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000680 0 0x80>;
+			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPB_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM13_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPB_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim14: timer@a4000700 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000700 0 0x80>;
+			interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPB_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM14_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPB_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim15: timer@a4000780 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000780 0 0x80>;
+			interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPB_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM15_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPB_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim16: timer@a4000800 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000800 0 0x80>;
+			interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPC_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM16_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPC_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim17: timer@a4000880 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000880 0 0x80>;
+			interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPC_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM17_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPC_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim18: timer@a4000900 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000900 0 0x80>;
+			interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPC_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM18_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPC_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim19: timer@a4000980 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000980 0 0x80>;
+			interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPC_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM19_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPC_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim20: timer@a4000a00 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000a00 0 0x80>;
+			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPC_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM20_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPC_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim21: timer@a4000a80 {
+			compatible = "renesas,r9a09g011-tim-cnt",
+				     "renesas,rzv2m-tim-cnt";
+			reg = <0 0xa4000a80 0 0x80>;
+			interrupts = <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPC_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM21_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPC_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim22: timer@a4000b00 {
+			compatible = "renesas,r9a09g011-tim",
+				     "renesas,rzv2m-tim";
+			reg = <0 0xa4000b00 0 0x80>;
+			interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPC_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM22_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPC_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
+		tim23: timer@a4000b80 {
+			compatible = "renesas,r9a09g011-tim",
+				     "renesas,rzv2m-tim";
+			reg = <0 0xa4000b80 0 0x80>;
+			interrupts = <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A09G011_CPERI_GRPC_PCLK>,
+				 <&cpg CPG_MOD R9A09G011_TIM23_CLK>;
+			clock-names = "apb", "tim";
+			resets = <&cpg R9A09G011_TIM_GPC_PRESETN>;
+			power-domains = <&cpg>;
+		};
+
 		pwm8: pwm@a4010400 {
 			compatible = "renesas,r9a09g011-pwm",
 				     "renesas,rzv2m-pwm";
-- 
2.25.1

Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Rob Herring]

On Mon, Dec 05, 2022 at 02:59:49PM +0000, Biju Das wrote:
> This patch series aims to add support for Compare-Match Timer (TIM)
> module found on RZ/V2M SoC.
> 
> it is composed of 32 channels and channels 0-7 and 24-32 are
> reserved for ISP usage.
> 
> Channel 22 is modelled as clock source and Channel 23 is modelled as clock
> event driver and the rest of the channels are modelled as counter driver
> as it provides

Why did you pick those 2 counters for those functions?

Unless the h/w blocks are different, this is an abuse of compatible 
strings. What's the h/w difference that makes you care which counter the 
OS picks? That's what the DT should describe. If any timer will do, just 
let the OS pick.

> 
> 1) counter for counting
> 2) configurable counter value for generating timer interrupt
> 3) userspace event for each interrupt.
> 
> logs:-
> Counter driver:
> Counter driver is tested by reading counts and interrupts tested by
> counter-example in tools/counter/counter_example.c
> 
> Count snapshot value:
> 3114
> Output from counter_example when it triggers interrupts:
> Timestamp 0: 24142152969        Count 0: 5
> Error Message 0: Success
> 
> Clock source:
> Clock source driver is tested by clock-source-switch app.
> [ 1275.703567] clocksource: Switched to clocksource arch_sys_counter
> [ 1275.710189] clocksource: Switched to clocksource a4000b00.timer

Do you have any use case to really switch. Doing so disables the vDSO 
access to the clocksource.

Rob

RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Biju Das]

Hi Rob,

Thanks for the feedback.

> Subject: Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support
> 
> On Mon, Dec 05, 2022 at 02:59:49PM +0000, Biju Das wrote:
> > This patch series aims to add support for Compare-Match Timer (TIM)
> > module found on RZ/V2M SoC.
> >
> > it is composed of 32 channels and channels 0-7 and 24-32 are reserved
> > for ISP usage.
> >
> > Channel 22 is modelled as clock source and Channel 23 is modelled as
> > clock event driver and the rest of the channels are modelled as
> > counter driver as it provides
> 
> Why did you pick those 2 counters for those functions?

Currently it uses architecture timer for broadcast timer, so I thought
Since TIM has 24 channels, use 1 channel for broadcast timer and 1
Channel for clock source. But having said that SoC has an aarch64 architecture
clock source strictly speaking we don't need this.

> 
> Unless the h/w blocks are different, this is an abuse of compatible
> strings. What's the h/w difference that makes you care which counter the
> OS picks? That's what the DT should describe. If any timer will do, just
> let the OS pick.

There is no HW difference. Same HW block can be used for mutually exclusive
functionality.

One is for Linux Clock source/event functionality((scheduler tick/broadcast tick etc) and 

the other purpose is to expose count and event ticks from this module to user space,
so that wide range of applications can make use of it.

If it is an abuse of compatible strings for mutually exclusive functionality
, then I would like to drop clock source and use all the channels as 
Either clock events(for broadcast ticks and real time usage??) or as counters.

If this is not OK, then I need to pick one. I will go with counters.

Please share your thoughts.

> 
> >
> > 1) counter for counting
> > 2) configurable counter value for generating timer interrupt
> > 3) userspace event for each interrupt.
> >
> > logs:-
> > Counter driver:
> > Counter driver is tested by reading counts and interrupts tested by
> > counter-example in tools/counter/counter_example.c
> >
> > Count snapshot value:
> > 3114
> > Output from counter_example when it triggers interrupts:
> > Timestamp 0: 24142152969        Count 0: 5
> > Error Message 0: Success
> >
> > Clock source:
> > Clock source driver is tested by clock-source-switch app.
> > [ 1275.703567] clocksource: Switched to clocksource arch_sys_counter
> > [ 1275.710189] clocksource: Switched to clocksource a4000b00.timer
> 
> Do you have any use case to really switch. Doing so disables the vDSO
> access to the clocksource.

Not really. Architecture timer should be sufficient for clocksource.

Cheers,
Biju

Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Geert Uytterhoeven]

Hi Biju,

On Tue, Dec 6, 2022 at 9:13 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support
> > On Mon, Dec 05, 2022 at 02:59:49PM +0000, Biju Das wrote:
> > > This patch series aims to add support for Compare-Match Timer (TIM)
> > > module found on RZ/V2M SoC.
> > >
> > > it is composed of 32 channels and channels 0-7 and 24-32 are reserved
> > > for ISP usage.
> > >
> > > Channel 22 is modelled as clock source and Channel 23 is modelled as
> > > clock event driver and the rest of the channels are modelled as
> > > counter driver as it provides
> >
> > Why did you pick those 2 counters for those functions?
>
> Currently it uses architecture timer for broadcast timer, so I thought
> Since TIM has 24 channels, use 1 channel for broadcast timer and 1
> Channel for clock source. But having said that SoC has an aarch64 architecture
> clock source strictly speaking we don't need this.
>
> > Unless the h/w blocks are different, this is an abuse of compatible
> > strings. What's the h/w difference that makes you care which counter the
> > OS picks? That's what the DT should describe. If any timer will do, just
> > let the OS pick.
>
> There is no HW difference. Same HW block can be used for mutually exclusive
> functionality.
>
> One is for Linux Clock source/event functionality((scheduler tick/broadcast tick etc) and
>
> the other purpose is to expose count and event ticks from this module to user space,
> so that wide range of applications can make use of it.
>
> If it is an abuse of compatible strings for mutually exclusive functionality
> , then I would like to drop clock source and use all the channels as
> Either clock events(for broadcast ticks and real time usage??) or as counters.
>
> If this is not OK, then I need to pick one. I will go with counters.
>
> Please share your thoughts.

Can't you handle this like sh_cmt.c does:

        /*
         * Use the first channel as a clock event device and the second channel
         * as a clock source. If only one channel is available use it for both.
         */

> > > 1) counter for counting
> > > 2) configurable counter value for generating timer interrupt
> > > 3) userspace event for each interrupt.
> > >
> > > logs:-
> > > Counter driver:
> > > Counter driver is tested by reading counts and interrupts tested by
> > > counter-example in tools/counter/counter_example.c
> > >
> > > Count snapshot value:
> > > 3114
> > > Output from counter_example when it triggers interrupts:
> > > Timestamp 0: 24142152969        Count 0: 5
> > > Error Message 0: Success
> > >
> > > Clock source:
> > > Clock source driver is tested by clock-source-switch app.
> > > [ 1275.703567] clocksource: Switched to clocksource arch_sys_counter
> > > [ 1275.710189] clocksource: Switched to clocksource a4000b00.timer
> >
> > Do you have any use case to really switch. Doing so disables the vDSO
> > access to the clocksource.
>
> Not really. Architecture timer should be sufficient for clocksource.

When multiple clocksources are registered, the clocksource
subsystems picks the best one anyway, right?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Thomas Gleixner]

On Tue, Dec 06 2022 at 09:40, Geert Uytterhoeven wrote:
> On Tue, Dec 6, 2022 at 9:13 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>> > Do you have any use case to really switch. Doing so disables the vDSO
>> > access to the clocksource.
>>
>> Not really. Architecture timer should be sufficient for clocksource.
>
> When multiple clocksources are registered, the clocksource
> subsystems picks the best one anyway, right?

As it does for the clock event devices. If there is an architected timer
then that should be always preferred.

No idea why there is a need for the extra hardware and the drivers which
are both never utilized.

Thanks,

        tglx

RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support
> 
> Hi Biju,
> 
> On Tue, Dec 6, 2022 at 9:13 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM)
> > > support On Mon, Dec 05, 2022 at 02:59:49PM +0000, Biju Das wrote:
> > > > This patch series aims to add support for Compare-Match Timer
> > > > (TIM) module found on RZ/V2M SoC.
> > > >
> > > > it is composed of 32 channels and channels 0-7 and 24-32 are
> > > > reserved for ISP usage.
> > > >
> > > > Channel 22 is modelled as clock source and Channel 23 is modelled
> > > > as clock event driver and the rest of the channels are modelled as
> > > > counter driver as it provides
> > >
> > > Why did you pick those 2 counters for those functions?
> >
> > Currently it uses architecture timer for broadcast timer, so I thought
> > Since TIM has 24 channels, use 1 channel for broadcast timer and 1
> > Channel for clock source. But having said that SoC has an aarch64
> > architecture clock source strictly speaking we don't need this.
> >
> > > Unless the h/w blocks are different, this is an abuse of compatible
> > > strings. What's the h/w difference that makes you care which counter
> > > the OS picks? That's what the DT should describe. If any timer will
> > > do, just let the OS pick.
> >
> > There is no HW difference. Same HW block can be used for mutually
> > exclusive functionality.
> >
> > One is for Linux Clock source/event functionality((scheduler
> > tick/broadcast tick etc) and
> >
> > the other purpose is to expose count and event ticks from this module
> > to user space, so that wide range of applications can make use of it.
> >
> > If it is an abuse of compatible strings for mutually exclusive
> > functionality , then I would like to drop clock source and use all the
> > channels as Either clock events(for broadcast ticks and real time
> usage??) or as counters.
> >
> > If this is not OK, then I need to pick one. I will go with counters.
> >
> > Please share your thoughts.
> 
> Can't you handle this like sh_cmt.c does:
> 
>         /*
>          * Use the first channel as a clock event device and the second
> channel
>          * as a clock source. If only one channel is available use it for
> both.
>          */

Currently it is handled like above except for the case " If only one channel is available use it for
Both", see patch#3 [1] probe function.
The only difference is here we have separate address space, clocks, and interrupts for each channel.
[1]
https://patchwork.kernel.org/project/linux-renesas-soc/patch/20221205145955.391526-4-biju.das.jz@bp.renesas.com/

Our customer BSP, uses this hw module for exposing timer interrupt event to user space
by using custom driver. The same functionality can be achieved through counter driver.
That is the reason, I have added counter driver to expose this functionality as well.

> 
> > > > 1) counter for counting
> > > > 2) configurable counter value for generating timer interrupt
> > > > 3) userspace event for each interrupt.
> > > >
> > > > logs:-
> > > > Counter driver:
> > > > Counter driver is tested by reading counts and interrupts tested
> > > > by counter-example in tools/counter/counter_example.c
> > > >
> > > > Count snapshot value:
> > > > 3114
> > > > Output from counter_example when it triggers interrupts:
> > > > Timestamp 0: 24142152969        Count 0: 5
> > > > Error Message 0: Success
> > > >
> > > > Clock source:
> > > > Clock source driver is tested by clock-source-switch app.
> > > > [ 1275.703567] clocksource: Switched to clocksource
> > > > arch_sys_counter [ 1275.710189] clocksource: Switched to
> > > > clocksource a4000b00.timer
> > >
> > > Do you have any use case to really switch. Doing so disables the
> > > vDSO access to the clocksource.
> >
> > Not really. Architecture timer should be sufficient for clocksource.
> 
> When multiple clocksources are registered, the clocksource subsystems
> picks the best one anyway, right?

Yes, it picks based on the rating.

Cheers,
Biju

RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Biju Das]

> Subject: Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support
> 
> On Tue, Dec 06 2022 at 09:40, Geert Uytterhoeven wrote:
> > On Tue, Dec 6, 2022 at 9:13 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> >> > Do you have any use case to really switch. Doing so disables the
> >> > vDSO access to the clocksource.
> >>
> >> Not really. Architecture timer should be sufficient for clocksource.
> >
> > When multiple clocksources are registered, the clocksource subsystems
> > picks the best one anyway, right?
> 
> As it does for the clock event devices. If there is an architected timer
> then that should be always preferred.
> 
> No idea why there is a need for the extra hardware and the drivers which
> are both never utilized.

Maybe, if you use secure OS, this timer hardware can be used there.

If you use PM deep states, Maybe, it could be used as broadcast event for waking up the system??

Cheers,
Biju

RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Biju Das]

Hi Thomas Gleixner and Geert,

> Subject: Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support
> 
> On Tue, Dec 06 2022 at 09:40, Geert Uytterhoeven wrote:
> > On Tue, Dec 6, 2022 at 9:13 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> >> > Do you have any use case to really switch. Doing so disables the
> >> > vDSO access to the clocksource.
> >>
> >> Not really. Architecture timer should be sufficient for clocksource.
> >
> > When multiple clocksources are registered, the clocksource subsystems
> > picks the best one anyway, right?
> 
> As it does for the clock event devices. If there is an architected timer
> then that should be always preferred.
> 
> No idea why there is a need for the extra hardware and the drivers which
> are both never utilized.

I got feedback from BSP team for the actual usage of this timer.

Basically, this HW timer is used for measuring the processing time
of DRP-AI accurately compared to the CPU timer normally we use.

The example use cases,
Timer in FREERUN mode, Check the timer value after the restart(1usec)"
Timer in FREERUN mode, Check the timer value after the restart(10000000usec)"

What is the model to be used for this kind of HW usage? Counter or Timer?

I can think of one possible HW usage by using Counter model.
Not sure how timer model can be used for this kind of HW usage??

Eg: we can set ceiling values 1usec and 10000000usec using counter framework
  And that will trigger interrupt events corresponding to the ceiling values
  to user space and user space app can accurately measure the DRP-AI processing time.

Also counter model exposes count values to user space from the counter HW.

Cheers,
Biju

RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Thomas Gleixner]

Biju!

On Wed, Dec 07 2022 at 07:52, Biju Das wrote:
>> On Tue, Dec 06 2022 at 09:40, Geert Uytterhoeven wrote:
>> > When multiple clocksources are registered, the clocksource subsystems
>> > picks the best one anyway, right?
>> 
>> As it does for the clock event devices. If there is an architected timer
>> then that should be always preferred.
>> 
>> No idea why there is a need for the extra hardware and the drivers which
>> are both never utilized.
>
> I got feedback from BSP team for the actual usage of this timer.
>
> Basically, this HW timer is used for measuring the processing time
> of DRP-AI accurately compared to the CPU timer normally we use.

How is a slow to access timer with a lower clock frequency more
accurate?

> The example use cases,
> Timer in FREERUN mode, Check the timer value after the restart(1usec)"
> Timer in FREERUN mode, Check the timer value after the restart(10000000usec)"
>
> What is the model to be used for this kind of HW usage? Counter or Timer?
>
> I can think of one possible HW usage by using Counter model.
> Not sure how timer model can be used for this kind of HW usage??
>
> Eg: we can set ceiling values 1usec and 10000000usec using counter framework
>   And that will trigger interrupt events corresponding to the ceiling values
>   to user space and user space app can accurately measure the DRP-AI processing time.
>
> Also counter model exposes count values to user space from the counter HW.

Counter subsystem != clocksource/event subsystem.

We are debating a clocksource/clockevent driver and not a counter
driver, right?

Thanks,

        tglx

RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Biju Das]

Hi Thomas Gleixner,

Thanks for the feedback.

> Subject: RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support
> 
> Biju!
> 
> On Wed, Dec 07 2022 at 07:52, Biju Das wrote:
> >> On Tue, Dec 06 2022 at 09:40, Geert Uytterhoeven wrote:
> >> > When multiple clocksources are registered, the clocksource
> >> > subsystems picks the best one anyway, right?
> >>
> >> As it does for the clock event devices. If there is an architected
> >> timer then that should be always preferred.
> >>
> >> No idea why there is a need for the extra hardware and the drivers
> >> which are both never utilized.
> >
> > I got feedback from BSP team for the actual usage of this timer.
> >
> > Basically, this HW timer is used for measuring the processing time of
> > DRP-AI accurately compared to the CPU timer normally we use.
> 
> How is a slow to access timer with a lower clock frequency more accurate?

But our tick frequency for arm64 defconfig is CONFIG_HZ_250=y. So we get timer interrupt
at every 4 msec. 

How do we get timer event interrupt, eg: for 1 microsec?

> 
> > The example use cases,
> > Timer in FREERUN mode, Check the timer value after the restart(1usec)"
> > Timer in FREERUN mode, Check the timer value after the
> restart(10000000usec)"
> >
> > What is the model to be used for this kind of HW usage? Counter or
> Timer?
> >
> > I can think of one possible HW usage by using Counter model.
> > Not sure how timer model can be used for this kind of HW usage??
> >
> > Eg: we can set ceiling values 1usec and 10000000usec using counter
> framework
> >   And that will trigger interrupt events corresponding to the ceiling
> values
> >   to user space and user space app can accurately measure the DRP-AI
> processing time.
> >
> > Also counter model exposes count values to user space from the counter
> HW.
> 
> Counter subsystem != clocksource/event subsystem.
> 
> We are debating a clocksource/clockevent driver and not a counter driver,
> right?

Yes, Rob pointed out we should not misuse the compatibles as I have both
Timer and counter bindings for a given HW timer.

Timer, It can be used as broadcast and highres timer for RT.

Counter, It can be used as measuring the processing time of DRP-AI.

What is the best way to use this hardware to take care of all this use cases? 

So far all the Renesas timers used timer model. But for RZ/V2M the HW usage is different.
The customer BSP mainly uses this timer for measuring the processing time of DRP-AI, 
so the expectation is we should at least have support for this use case.

Cheers,
Biju

RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Thomas Gleixner]

Biju!

On Wed, Dec 07 2022 at 11:35, Biju Das wrote:
>> Subject: RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support
>> On Wed, Dec 07 2022 at 07:52, Biju Das wrote:
>> > Basically, this HW timer is used for measuring the processing time of
>> > DRP-AI accurately compared to the CPU timer normally we use.
>> 
>> How is a slow to access timer with a lower clock frequency more accurate?
>
> But our tick frequency for arm64 defconfig is CONFIG_HZ_250=y. So we
> get timer interrupt at every 4 msec.

CONFIG_HIGH_RES_TIMERS=y

> How do we get timer event interrupt, eg: for 1 microsec?

clock_nanosleep(...);

Though 1usec is wishful thinking with either variant of timer hardware.

>> 
>> We are debating a clocksource/clockevent driver and not a counter driver,
>> right?
>
> Yes, Rob pointed out we should not misuse the compatibles as I have both
> Timer and counter bindings for a given HW timer.
>
> Timer, It can be used as broadcast and highres timer for RT.

I buy the broadcast part if you really have a ARM architected timer
which stops in deep idle states. Highres for RT, no! The arm architected
timer works perfectly fine for that.

> Counter, It can be used as measuring the processing time of DRP-AI.

Sigh. You can do that with the architected timer too, especially when
you are going to do the measurement in user space.

clock_gettime(), which uses the VDSO with the architected timer is fast
to access and accurate.

Thanks,

        tglx

Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]

On Wed, Dec 07, 2022 at 05:49:09PM +0100, Thomas Gleixner wrote:
> On Wed, Dec 07 2022 at 11:35, Biju Das wrote:
> > Counter, It can be used as measuring the processing time of DRP-AI.
> 
> Sigh. You can do that with the architected timer too, especially when
> you are going to do the measurement in user space.
> 
> clock_gettime(), which uses the VDSO with the architected timer is fast
> to access and accurate.
> 
> Thanks,
> 
>         tglx

Hi Biju,

It's true that you could implement a Counter driver to achieve what you
want here, but I don't think that's the most apt interface for this
device. Your device is used to measure the processing time of DRP-AI, so
modeling this as a clocksource seems like the right approach to take.

Of course, if there is something missing from clocksource/clockevent
that you need, then it should be added to the subsystem. So let's try to
narrow down exactly what functionality you need.

You gave a Counter use-case example earlier where you can configure the
ceiling value of the timer (e.g. to 1usec or 10000000usec) and push
Counter events on the interrupts that trigger off that that
configuration; the Counter subsystem can logs the current system time
everytime a Counter event is pushed.

Could the same thing be achieved using clockevents framework instead?
With this approach you would register an event to fire in the future
(e.g. 1usec or 10000000usec) and then call clock_gettime() to get the
current system when you're notified of the event. Would this approach
work for your use-case, or is something else missing here?

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Biju Das]

Hi William Breathitt Gray,

Thanks for the feedback.

> Subject: Re: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support
> 
> On Wed, Dec 07, 2022 at 05:49:09PM +0100, Thomas Gleixner wrote:
> > On Wed, Dec 07 2022 at 11:35, Biju Das wrote:
> > > Counter, It can be used as measuring the processing time of DRP-AI.
> >
> > Sigh. You can do that with the architected timer too, especially when
> > you are going to do the measurement in user space.
> >
> > clock_gettime(), which uses the VDSO with the architected timer is
> > fast to access and accurate.
> >
> > Thanks,
> >
> >         tglx
> 
> Hi Biju,
> 
> It's true that you could implement a Counter driver to achieve what you
> want here, but I don't think that's the most apt interface for this
> device. Your device is used to measure the processing time of DRP-AI, so
> modeling this as a clocksource seems like the right approach to take.
> 
> Of course, if there is something missing from clocksource/clockevent that
> you need, then it should be added to the subsystem. So let's try to narrow
> down exactly what functionality you need.
> 
> You gave a Counter use-case example earlier where you can configure the
> ceiling value of the timer (e.g. to 1usec or 10000000usec) and push
> Counter events on the interrupts that trigger off that that configuration;
> the Counter subsystem can logs the current system time everytime a Counter
> event is pushed.
> 
> Could the same thing be achieved using clockevents framework instead?

If I am correct, from this thread discussion, we can make use of architecture timer
with hrtimer, which will give call back events to user space after time expires. 
In the callback, we could call clock_gettime(), which uses the VDSO which is fast
and accurate.

So there is no need to use any extra HW timer.

scheduling tick is 4millisec. so if we want callback at 1 microsec, then we need to 
use clock_nanosleep. Getting 1 microsec callback to user space is challenging as
the scheduling tick is only 4 millisec.

Cheers,
Biju

> With this approach you would register an event to fire in the future (e.g.
> 1usec or 10000000usec) and then call clock_gettime() to get the current
> system when you're notified of the event. Would this approach work for
> your use-case, or is something else missing here?

RE: [PATCH 0/6] Add RZ/V2M Compare-Match Timer (TIM) support

[Thomas Gleixner]

On Sat, Dec 10 2022 at 07:52, Biju Das wrote:
> scheduling tick is 4millisec. so if we want callback at 1 microsec,
> then we need to use clock_nanosleep. Getting 1 microsec callback to
> user space is challenging as the scheduling tick is only 4 millisec.

The tick is only relevant if high resolution timers are disabled because
then hrtimers are expired in the tick. If high resolution timers are
enabled then the hrtimer expiry happens at the exact expiry time.

What's challenging about the 1 microsecond accuracy is that the system
immanent latencies are already in that range. So while the timer fires
exactly, the actual execution of the woken up task in user space is not
exact as that is subject to the worst case sum of latencies in the
system.

Thanks,

        tglx