[PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[Biju Das]

The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
channels and one 32-bit timer channel. It supports the following
functions
 - Counter
 - Timer
 - PWM

This patch series aim to add MFD and counter driver for MTU3a.
Subsequent patch seies will add TImer and PWM driver support
also enhancements to counter driver.

The 8/16/32 bit registers are mixed in each channel. The HW
specifications of the IP is described in patch#2.

Current patch set is tested for 16-bit phase counting mode on
MTU1 channel.

Please share your valuable comments on this patch series.

Biju Das (8):
  clk: renesas: r9a07g044: Add MTU3a clock and reset entry
  dt-bindings: mfd: Document RZ/G2L MTU3a bindings
  mfd: Add RZ/G2L MTU3 driver
  dt-bindings: mfd: rzg2l-mtu3: Document RZ/G2UL MTU3 counter
  counter: Add RZ/G2L MTU3 counter driver
  arm64: dts: renesas: r9a07g044: Add MTU3a node
  arm64: dts: renesas: r9a07g054: Add MTU3a node
  arm64: dts: renesas: rzg2l-smarc: [HACK] Enable MTU for 16-bit phase
    count testing

 .../bindings/mfd/renesas,rzg2l-mtu3.yaml      | 310 ++++++++++++++
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi    |  83 ++++
 .../boot/dts/renesas/r9a07g044l2-smarc.dts    |   2 -
 arch/arm64/boot/dts/renesas/r9a07g054.dtsi    |  83 ++++
 .../dts/renesas/rzg2l-smarc-pinfunction.dtsi  |  11 +
 arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi  |  25 +-
 drivers/clk/renesas/r9a07g044-cpg.c           |   5 +-
 drivers/counter/Kconfig                       |   9 +
 drivers/counter/Makefile                      |   1 +
 drivers/counter/rzg2l-mtu3-cnt.c              | 367 +++++++++++++++++
 drivers/mfd/Kconfig                           |   9 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/rzg2l-mtu3.c                      | 377 ++++++++++++++++++
 include/linux/mfd/rzg2l-mtu3.h                | 124 ++++++
 14 files changed, 1403 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
 create mode 100644 drivers/counter/rzg2l-mtu3-cnt.c
 create mode 100644 drivers/mfd/rzg2l-mtu3.c
 create mode 100644 include/linux/mfd/rzg2l-mtu3.h

-- 
2.25.1

[PATCH RFC 1/8] clk: renesas: r9a07g044: Add MTU3a clock and reset entry

[Biju Das]

Add MTU3a clock and reset entry to CPG driver.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g044-cpg.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index fd7c4eecd398..0c300ce7ebf8 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -182,7 +182,7 @@ static const struct {
 };
 
 static const struct {
-	struct rzg2l_mod_clk common[76];
+	struct rzg2l_mod_clk common[77];
 #ifdef CONFIG_CLK_R9A07G054
 	struct rzg2l_mod_clk drp[0];
 #endif
@@ -204,6 +204,8 @@ static const struct {
 					0x534, 1),
 		DEF_MOD("ostm2_pclk",	R9A07G044_OSTM2_PCLK, R9A07G044_CLK_P0,
 					0x534, 2),
+		DEF_MOD("mtu_x_mck",	R9A07G044_MTU_X_MCK_MTU3, R9A07G044_CLK_P0,
+					0x538, 0),
 		DEF_MOD("gpt_pclk",	R9A07G044_GPT_PCLK, R9A07G044_CLK_P0,
 					0x540, 0),
 		DEF_MOD("poeg_a_clkp",	R9A07G044_POEG_A_CLKP, R9A07G044_CLK_P0,
@@ -356,6 +358,7 @@ static struct rzg2l_reset r9a07g044_resets[] = {
 	DEF_RST(R9A07G044_OSTM0_PRESETZ, 0x834, 0),
 	DEF_RST(R9A07G044_OSTM1_PRESETZ, 0x834, 1),
 	DEF_RST(R9A07G044_OSTM2_PRESETZ, 0x834, 2),
+	DEF_RST(R9A07G044_MTU_X_PRESET_MTU3, 0x838, 0),
 	DEF_RST(R9A07G044_GPT_RST_C, 0x840, 0),
 	DEF_RST(R9A07G044_POEG_A_RST, 0x844, 0),
 	DEF_RST(R9A07G044_POEG_B_RST, 0x844, 1),
-- 
2.25.1

[PATCH RFC 2/8] dt-bindings: mfd: Document RZ/G2L MTU3a bindings

[Biju Das]

The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
channels and one 32-bit timer channel. It supports the following
functions
 - Counter
 - Timer
 - PWM

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

diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
new file mode 100644
index 000000000000..c1fae8e8d9f9
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
@@ -0,0 +1,275 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/renesas,rzg2l-mtu3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 (MTU3a) bindings
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  This hardware block pconsisting of eight 16-bit timer channels and one
+  32- bit timer channel. It supports the following specifications:
+    - Pulse input/output: 28 lines max.
+    - Pulse input 3 lines
+    - Count clock 11 clocks for each channel (14 clocks for MTU0, 12 clocks
+      for MTU2, and 10 clocks for MTU5, four clocks for MTU1-MTU2 combination
+      (when LWA = 1))
+    - Operating frequency Up to 100 MHz
+    - Available operations [MTU0 to MTU4, MTU6, MTU7, and MTU8]
+        - Waveform output on compare match
+        - Input capture function (noise filter setting available)
+        - Counter-clearing operation
+        - Simultaneous writing to multiple timer counters (TCNT)
+          (excluding MTU8).
+        - Simultaneous clearing on compare match or input capture
+          (excluding MTU8).
+        - Simultaneous input and output to registers in synchronization with
+          counter operations           (excluding MTU8).
+        - Up to 12-phase PWM output in combination with synchronous operation
+          (excluding MTU8)
+    - [MTU0 MTU3, MTU4, MTU6, MTU7, and MTU8]
+        - Buffer operation specifiable
+    - [MTU1, MTU2]
+        - Phase counting mode can be specified independently
+        - 32-bit phase counting mode can be specified for interlocked operation
+          of MTU1 and MTU2 (when TMDR3.LWA = 1)
+        - Cascade connection operation available
+    - [MTU3, MTU4, MTU6, and MTU7]
+        - Through interlocked operation of MTU3/4 and MTU6/7, the positive and
+          negative signals in six phases (12 phases in total) can be output in
+          complementary PWM and reset-synchronized PWM operation.
+        - In complementary PWM mode, values can be transferred from buffer
+          registers to temporary registers at crests and troughs of the timer-
+          counter values or when the buffer registers (TGRD registers in MTU4
+          and MTU7) are written to.
+        - Double-buffering selectable in complementary PWM mode.
+    - [MTU3 and MTU4]
+        - Through interlocking with MTU0, a mode for driving AC synchronous
+          motors (brushless DC motors) by using complementary PWM output and
+          reset-synchronized PWM output is settable and allows the selection
+          of two types of waveform output (chopping or level).
+    - [MTU5]
+        - Capable of operation as a dead-time compensation counter.
+    - [MTU0/MTU5, MTU1, MTU2, and MTU8]
+        - 32-bit phase counting mode specifiable by combining MTU1 and MTU2 and
+          through interlocked operation with MTU0/MTU5 and MTU8.
+    - Interrupt-skipping function
+        - In complementary PWM mode, interrupts on crests and troughs of counter
+          values and triggers to start conversion by the A/D converter can be
+          skipped.
+    - Interrupt sources: 43 sources.
+    - Buffer operation:
+        - Automatic transfer of register data (transfer from the buffer
+          register to the timer register).
+    - Trigger generation
+        - A/D converter start triggers can be generated
+        - A/D converter start request delaying function enables A/D converter
+          to be started with any desired timing and to be synchronized with
+          PWM output.
+    - Low power consumption function
+        - The MTU3a can be placed in the module-stop state.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a07g044-mtu3  # RZ/G2{L,LC}
+          - renesas,r9a07g054-mtu3  # RZ/V2L
+      - const: renesas,rzg2l-mtu3
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: MTU0.TGRA input capture/compare match
+      - description: MTU0.TGRB input capture/compare match
+      - description: MTU0.TGRC input capture/compare match
+      - description: MTU0.TGRD input capture/compare match
+      - description: MTU0.TCNT overflow
+      - description: MTU0.TGRE compare match
+      - description: MTU0.TGRF compare match
+      - description: MTU1.TGRA input capture/compare match
+      - description: MTU1.TGRB input capture/compare match
+      - description: MTU1.TCNT overflow
+      - description: MTU1.TCNT underflow
+      - description: MTU2.TGRA input capture/compare match
+      - description: MTU2.TGRB input capture/compare match
+      - description: MTU2.TCNT overflow
+      - description: MTU2.TCNT underflow
+      - description: MTU3.TGRA input capture/compare match
+      - description: MTU3.TGRB input capture/compare match
+      - description: MTU3.TGRC input capture/compare match
+      - description: MTU3.TGRD input capture/compare match
+      - description: MTU3.TCNT overflow
+      - description: MTU4.TGRA input capture/compare match
+      - description: MTU4.TGRB input capture/compare match
+      - description: MTU4.TGRC input capture/compare match
+      - description: MTU4.TGRD input capture/compare match
+      - description: MTU4.TCNT overflow/underflow
+      - description: MTU5.TGRU input capture/compare match
+      - description: MTU5.TGRV input capture/compare match
+      - description: MTU5.TGRW input capture/compare match
+      - description: MTU6.TGRA input capture/compare match
+      - description: MTU6.TGRB input capture/compare match
+      - description: MTU6.TGRC input capture/compare match
+      - description: MTU6.TGRD input capture/compare match
+      - description: MTU6.TCNT overflow
+      - description: MTU7.TGRA input capture/compare match
+      - description: MTU7.TGRB input capture/compare match
+      - description: MTU7.TGRC input capture/compare match
+      - description: MTU7.TGRD input capture/compare match
+      - description: MTU7.TCNT overflow/underflow
+      - description: MTU8.TGRA input capture/compare match
+      - description: MTU8.TGRB input capture/compare match
+      - description: MTU8.TGRC input capture/compare match
+      - description: MTU8.TGRD input capture/compare match
+      - description: MTU8.TCNT overflow
+      - description: MTU8.TCNT underflow
+
+  interrupt-names:
+    items:
+      - const: tgia0
+      - const: tgib0
+      - const: tgic0
+      - const: tgid0
+      - const: tgiv0
+      - const: tgie0
+      - const: tgif0
+      - const: tgia1
+      - const: tgib1
+      - const: tgiv1
+      - const: tgiu1
+      - const: tgia2
+      - const: tgib2
+      - const: tgiv2
+      - const: tgiu2
+      - const: tgia3
+      - const: tgib3
+      - const: tgic3
+      - const: tgid3
+      - const: tgiv3
+      - const: tgia4
+      - const: tgib4
+      - const: tgic4
+      - const: tgid4
+      - const: tgiv4
+      - const: tgiu5
+      - const: tgiv5
+      - const: tgiw5
+      - const: tgia6
+      - const: tgib6
+      - const: tgic6
+      - const: tgid6
+      - const: tgiv6
+      - const: tgia7
+      - const: tgib7
+      - const: tgic7
+      - const: tgid7
+      - const: tgiv7
+      - const: tgia8
+      - const: tgib8
+      - const: tgic8
+      - const: tgid8
+      - const: tgiv8
+      - const: tgiu8
+
+  clocks:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a07g044-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    mtu3: timer@10001200 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "renesas,r9a07g044-mtu3", "renesas,rzg2l-mtu3";
+      reg = <0x10001200 0xb00>;
+      interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 172 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 173 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 174 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 175 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 176 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 178 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 179 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 180 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 181 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 182 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 183 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 184 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 185 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 186 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 190 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 191 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 192 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 194 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 195 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 198 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 199 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 200 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 201 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 202 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 203 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 204 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 205 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 206 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 207 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 208 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 209 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 210 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 211 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 212 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 213 IRQ_TYPE_EDGE_RISING>;
+      interrupt-names = "tgia0", "tgib0", "tgic0", "tgid0", "tgiv0", "tgie0",
+                        "tgif0",
+                        "tgia1", "tgib1", "tgiv1", "tgiu1",
+                        "tgia2", "tgib2", "tgiv2", "tgiu2",
+                        "tgia3", "tgib3", "tgic3", "tgid3", "tgiv3",
+                        "tgia4", "tgib4", "tgic4", "tgid4", "tgiv4",
+                        "tgiu5", "tgiv5", "tgiw5",
+                        "tgia6", "tgib6", "tgic6", "tgid6", "tgiv6",
+                        "tgia7", "tgib7", "tgic7", "tgid7", "tgiv7",
+                        "tgia8", "tgib8", "tgic8", "tgid8", "tgiv8", "tgiu8";
+      clocks = <&cpg CPG_MOD R9A07G044_MTU_X_MCK_MTU3>;
+      power-domains = <&cpg>;
+      resets = <&cpg R9A07G044_MTU_X_PRESET_MTU3>;
+    };
+
+...
-- 
2.25.1

[PATCH RFC 3/8] mfd: Add RZ/G2L MTU3 driver

[Biju Das]

Add RZ/G2L MTU3 MFD driver. It can support counter, timer and
pwm functionality.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/mfd/Kconfig            |   9 +
 drivers/mfd/Makefile           |   1 +
 drivers/mfd/rzg2l-mtu3.c       | 377 +++++++++++++++++++++++++++++++++
 include/linux/mfd/rzg2l-mtu3.h | 124 +++++++++++
 4 files changed, 511 insertions(+)
 create mode 100644 drivers/mfd/rzg2l-mtu3.c
 create mode 100644 include/linux/mfd/rzg2l-mtu3.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index abb58ab1a1a4..a435ade4426b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1974,6 +1974,15 @@ config MFD_ROHM_BD957XMUF
 	  BD9573MUF Power Management ICs. BD9576 and BD9573 are primarily
 	  designed to be used to power R-Car series processors.
 
+config MFD_RZG2L_MTU3
+	tristate "Support for RZ/G2L MTU3 timers"
+	depends on (ARCH_RZG2L && OF) || COMPILE_TEST
+	select MFD_CORE
+	help
+	  Select this option to enable RZ/G2L MTU3 timers driver used
+	  for PWM, Clock Source, Clock event and Counter. This driver allow to
+	  share the registers between the others drivers.
+
 config MFD_STM32_LPTIMER
 	tristate "Support for STM32 Low-Power Timer"
 	depends on (ARCH_STM32 && OF) || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 858cacf659d6..b52575556e93 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -251,6 +251,7 @@ obj-$(CONFIG_MFD_ALTERA_SYSMGR) += altera-sysmgr.o
 obj-$(CONFIG_MFD_STPMIC1)	+= stpmic1.o
 obj-$(CONFIG_MFD_SUN4I_GPADC)	+= sun4i-gpadc.o
 
+obj-$(CONFIG_MFD_RZG2L_MTU3) 	+= rzg2l-mtu3.o
 obj-$(CONFIG_MFD_STM32_LPTIMER)	+= stm32-lptimer.o
 obj-$(CONFIG_MFD_STM32_TIMERS) 	+= stm32-timers.o
 obj-$(CONFIG_MFD_MXS_LRADC)     += mxs-lradc.o
diff --git a/drivers/mfd/rzg2l-mtu3.c b/drivers/mfd/rzg2l-mtu3.c
new file mode 100644
index 000000000000..16bf05218d91
--- /dev/null
+++ b/drivers/mfd/rzg2l-mtu3.c
@@ -0,0 +1,377 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 - MTU3a
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+
+#include <linux/bitfield.h>
+#include <linux/mfd/rzg2l-mtu3.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/irq.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+static const unsigned long rzg2l_mtu3_8bit_ch_reg_offs[][13] = {
+	{
+		[RZG2L_MTU3_TIER] = 0x4, [RZG2L_MTU3_NFCR] = 0x70,
+		[RZG2L_MTU3_TCR] = 0x0, [RZG2L_MTU3_TCR2] = 0x28,
+		[RZG2L_MTU3_TMDR1] = 0x1, [RZG2L_MTU3_TIORH] = 0x2,
+		[RZG2L_MTU3_TIORL] = 0x3
+	},
+	{
+		[RZG2L_MTU3_TIER] = 0x4, [RZG2L_MTU3_NFCR] = 0xef,
+		[RZG2L_MTU3_TSR] = 0x5, [RZG2L_MTU3_TCR] = 0x0,
+		[RZG2L_MTU3_TCR2] = 0x14, [RZG2L_MTU3_TMDR1] = 0x1,
+		[RZG2L_MTU3_TIOR] = 0x2
+	},
+	{
+		[RZG2L_MTU3_TIER] = 0x4, [RZG2L_MTU3_NFCR] = 0x16e,
+		[RZG2L_MTU3_TSR] = 0x5, [RZG2L_MTU3_TCR] = 0x0,
+		[RZG2L_MTU3_TCR2] = 0xc, [RZG2L_MTU3_TMDR1] = 0x1,
+		[RZG2L_MTU3_TIOR] = 0x2
+	},
+	{
+		[RZG2L_MTU3_TIER] = 0x8, [RZG2L_MTU3_NFCR] = 0x93,
+		[RZG2L_MTU3_TSR] = 0x2c, [RZG2L_MTU3_TCR] = 0x0,
+		[RZG2L_MTU3_TCR2] = 0x4c, [RZG2L_MTU3_TMDR1] = 0x2,
+		[RZG2L_MTU3_TIORH] = 0x4, [RZG2L_MTU3_TIORL] = 0x5,
+		[RZG2L_MTU3_TBTM] = 0x38
+	},
+	{
+		[RZG2L_MTU3_TIER] = 0x8, [RZG2L_MTU3_NFCR] = 0x93,
+		[RZG2L_MTU3_TSR] = 0x2c, [RZG2L_MTU3_TCR] = 0x0,
+		[RZG2L_MTU3_TCR2] = 0x4c, [RZG2L_MTU3_TMDR1] = 0x2,
+		[RZG2L_MTU3_TIORH] = 0x5, [RZG2L_MTU3_TIORL] = 0x6,
+		[RZG2L_MTU3_TBTM] = 0x38
+	},
+	{
+		[RZG2L_MTU3_TIER] = 0x32, [RZG2L_MTU3_NFCR] = 0x1eb,
+		[RZG2L_MTU3_TSTR] = 0x34, [RZG2L_MTU3_TCNTCMPCLR] = 0x36,
+		[RZG2L_MTU3_TCRU] = 0x4, [RZG2L_MTU3_TCR2U] = 0x5,
+		[RZG2L_MTU3_TIORU] = 0x6, [RZG2L_MTU3_TCRV] = 0x14,
+		[RZG2L_MTU3_TCR2V] = 0x15, [RZG2L_MTU3_TIORV] = 0x16,
+		[RZG2L_MTU3_TCRW] = 0x24, [RZG2L_MTU3_TCR2W] = 0x25,
+		[RZG2L_MTU3_TIORW] = 0x26
+	},
+	{
+		[RZG2L_MTU3_TIER] = 0x8, [RZG2L_MTU3_NFCR] = 0x93,
+		[RZG2L_MTU3_TSR] = 0x2c, [RZG2L_MTU3_TCR] = 0x0,
+		[RZG2L_MTU3_TCR2] = 0x4c, [RZG2L_MTU3_TMDR1] = 0x2,
+		[RZG2L_MTU3_TIORH] = 0x4, [RZG2L_MTU3_TIORL] = 0x5,
+		[RZG2L_MTU3_TBTM] = 0x38
+	},
+	{
+		[RZG2L_MTU3_TIER] = 0x8, [RZG2L_MTU3_NFCR] = 0x93,
+		[RZG2L_MTU3_TSR] = 0x2c, [RZG2L_MTU3_TCR] = 0x0,
+		[RZG2L_MTU3_TCR2] = 0x4c, [RZG2L_MTU3_TMDR1] = 0x2,
+		[RZG2L_MTU3_TIORH] = 0x5, [RZG2L_MTU3_TIORL] = 0x6,
+		[RZG2L_MTU3_TBTM] = 0x38
+	},
+	{
+		[RZG2L_MTU3_TIER] = 0x4, [RZG2L_MTU3_NFCR] = 0x368,
+		[RZG2L_MTU3_TCR] = 0x0, [RZG2L_MTU3_TCR2] = 0x6,
+		[RZG2L_MTU3_TMDR1] = 0x1, [RZG2L_MTU3_TIORH] = 0x2,
+		[RZG2L_MTU3_TIORL] = 0x3
+	}
+};
+
+static const unsigned long rzg2l_mtu3_16bit_ch_reg_offs[][12] = {
+	{
+		[RZG2L_MTU3_TCNT] = 0x6, [RZG2L_MTU3_TGRA] = 0x8,
+		[RZG2L_MTU3_TGRB] = 0xa, [RZG2L_MTU3_TGRC] = 0xc,
+		[RZG2L_MTU3_TGRD] = 0xe, [RZG2L_MTU3_TGRE] = 0x20,
+		[RZG2L_MTU3_TGRF] = 0x22
+	},
+	{
+		[RZG2L_MTU3_TCNT] = 0x6, [RZG2L_MTU3_TGRA] = 0x8,
+		[RZG2L_MTU3_TGRB] = 0xa
+	},
+	{
+		[RZG2L_MTU3_TCNT] = 0x6, [RZG2L_MTU3_TGRA] = 0x8,
+		[RZG2L_MTU3_TGRB] = 0xa
+	},
+	{
+		[RZG2L_MTU3_TCNT] = 0x10, [RZG2L_MTU3_TGRA] = 0x18,
+		[RZG2L_MTU3_TGRB] = 0x1a, [RZG2L_MTU3_TGRC] = 0x24,
+		[RZG2L_MTU3_TGRD] = 0x26, [RZG2L_MTU3_TGRE] = 0x72
+	},
+	{
+		[RZG2L_MTU3_TCNT] = 0x11, [RZG2L_MTU3_TGRA] = 0x1b,
+		[RZG2L_MTU3_TGRB] = 0x1d, [RZG2L_MTU3_TGRC] = 0x27,
+		[RZG2L_MTU3_TGRD] = 0x29, [RZG2L_MTU3_TGRE] = 0x73,
+		[RZG2L_MTU3_TGRF] = 0x75, [RZG2L_MTU3_TADCR] = 0x3f,
+		[RZG2L_MTU3_TADCORA] = 0x43, [RZG2L_MTU3_TADCORB] = 0x45,
+		[RZG2L_MTU3_TADCOBRA] = 0x47,
+		[RZG2L_MTU3_TADCOBRB] = 0x49
+	},
+	{
+		[RZG2L_MTU3_TCNTU] = 0x0, [RZG2L_MTU3_TGRU] = 0x2,
+		[RZG2L_MTU3_TCNTV] = 0x10, [RZG2L_MTU3_TGRV] = 0x12,
+		[RZG2L_MTU3_TCNTW] = 0x20, [RZG2L_MTU3_TGRW] = 0x22
+	},
+	{
+		[RZG2L_MTU3_TCNT] = 0x10, [RZG2L_MTU3_TGRA] = 0x18,
+		[RZG2L_MTU3_TGRB] = 0x1a, [RZG2L_MTU3_TGRC] = 0x24,
+		[RZG2L_MTU3_TGRD] = 0x26, [RZG2L_MTU3_TGRE] = 0x72
+	},
+	{
+		[RZG2L_MTU3_TCNT] = 0x11, [RZG2L_MTU3_TGRA] = 0x1b,
+		[RZG2L_MTU3_TGRB] = 0x1d, [RZG2L_MTU3_TGRC] = 0x27,
+		[RZG2L_MTU3_TGRD] = 0x29, [RZG2L_MTU3_TGRE] = 0x73,
+		[RZG2L_MTU3_TGRF] = 0x75, [RZG2L_MTU3_TADCR] = 0x3f,
+		[RZG2L_MTU3_TADCORA] = 0x43, [RZG2L_MTU3_TADCORB] = 0x45,
+		[RZG2L_MTU3_TADCOBRA] = 0x47,
+		[RZG2L_MTU3_TADCOBRB] = 0x49
+	},
+};
+
+static bool rzg2l_mtu3_is_16bit_shared_reg(u16 off)
+{
+	return (off == RZG2L_MTU3_TDDRA || off == RZG2L_MTU3_TDDRB ||
+		off == RZG2L_MTU3_TCDRA || off == RZG2L_MTU3_TCDRB ||
+		off == RZG2L_MTU3_TCBRA || off == RZG2L_MTU3_TCBRB ||
+		off == RZG2L_MTU3_TCNTSA || off == RZG2L_MTU3_TCNTSB);
+}
+
+u16 rzg2l_mtu3_shared_reg_read(struct rzg2l_mtu3_channel *ch, u16 off)
+{
+	struct rzg2l_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
+
+	if (rzg2l_mtu3_is_16bit_shared_reg(off))
+		return ioread16(mtu->mmio + off);
+	else
+		return ioread8(mtu->mmio + off);
+}
+EXPORT_SYMBOL_GPL(rzg2l_mtu3_shared_reg_read);
+
+u8 rzg2l_mtu3_8bit_ch_read(struct rzg2l_mtu3_channel *ch, u16 off)
+{
+	u16 ch_offs;
+
+	ch_offs = rzg2l_mtu3_8bit_ch_reg_offs[ch->index][off];
+	if (off != RZG2L_MTU3_TCR && ch_offs == 0)
+		return -EINVAL;
+
+	/*
+	 * NFCR register addresses on MTU{0,1,2,5,8} channels are smaller than
+	 * channel's base address.
+	 */
+	if (off == RZG2L_MTU3_NFCR && (ch->index <= RZG2L_MTU2 ||
+				       ch->index == RZG2L_MTU5 ||
+				       ch->index == RZG2L_MTU8))
+		return ioread8(ch->base - ch_offs);
+	else
+		return ioread8(ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rzg2l_mtu3_8bit_ch_read);
+
+u16 rzg2l_mtu3_16bit_ch_read(struct rzg2l_mtu3_channel *ch, u16 off)
+{
+	u16 ch_offs;
+
+	/* MTU8 doesn't have 16-bit registers */
+	if (ch->index == RZG2L_MTU8)
+		return 0;
+
+	ch_offs = rzg2l_mtu3_16bit_ch_reg_offs[ch->index][off];
+	if (ch->index != RZG2L_MTU5 && off != RZG2L_MTU3_TCNTU && ch_offs == 0)
+		return 0;
+
+	return ioread16(ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rzg2l_mtu3_16bit_ch_read);
+
+void rzg2l_mtu3_8bit_ch_write(struct rzg2l_mtu3_channel *ch, u16 off, u8 val)
+{
+	u16 ch_offs;
+
+	ch_offs = rzg2l_mtu3_8bit_ch_reg_offs[ch->index][off];
+	if (ch->index != RZG2L_MTU5 && off != RZG2L_MTU3_TCR && ch_offs == 0)
+		return;
+
+	/*
+	 * NFCR register addresses on MTU{0,1,2,5,8} channels are smaller than
+	 * channel's base address.
+	 */
+	if (off == RZG2L_MTU3_NFCR && (ch->index <= RZG2L_MTU2 ||
+				       ch->index == RZG2L_MTU5 ||
+				       ch->index == RZG2L_MTU8))
+		iowrite8(val, ch->base - ch_offs);
+	else
+		iowrite8(val, ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rzg2l_mtu3_8bit_ch_write);
+
+void rzg2l_mtu3_16bit_ch_write(struct rzg2l_mtu3_channel *ch, u16 off, u16 val)
+{
+	u16 ch_offs;
+
+	/* MTU8 doesn't have 16-bit registers */
+	if (ch->index == RZG2L_MTU8)
+		return;
+
+	ch_offs = rzg2l_mtu3_16bit_ch_reg_offs[ch->index][off];
+	if (ch->index != RZG2L_MTU5 && off != RZG2L_MTU3_TCNTU && ch_offs == 0)
+		return;
+
+	iowrite16(val, ch->base + ch_offs);
+}
+EXPORT_SYMBOL_GPL(rzg2l_mtu3_16bit_ch_write);
+
+static inline void rzg2l_mtu3_shared_reg_write(struct rzg2l_mtu3_channel *ch,
+					       u16 off, u16 value)
+{
+	struct rzg2l_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
+
+	if (rzg2l_mtu3_is_16bit_shared_reg(off))
+		iowrite16(value, mtu->mmio + off);
+	else
+		iowrite8((u8)value, mtu->mmio + off);
+}
+
+static void rzg2l_mtu3_start_stop_ch(struct rzg2l_mtu3_channel *ch, bool start)
+{
+	struct rzg2l_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
+	unsigned long flags, value;
+	u8 offs;
+
+	/* start stop register shared by multiple timer channels */
+	raw_spin_lock_irqsave(&mtu->lock, flags);
+
+	if (ch->index == RZG2L_MTU6 || ch->index == RZG2L_MTU7) {
+		value = rzg2l_mtu3_shared_reg_read(ch, RZG2L_MTU3_TSTRB);
+		if (start)
+			value |= 1 << ch->index;
+		else
+			value &= ~(1 << ch->index);
+		rzg2l_mtu3_shared_reg_write(ch, RZG2L_MTU3_TSTRB, value);
+	} else if (ch->index != RZG2L_MTU5) {
+		value = rzg2l_mtu3_shared_reg_read(ch, RZG2L_MTU3_TSTRA);
+		if (ch->index == RZG2L_MTU8)
+			offs = 0x08;
+		else if (ch->index < RZG2L_MTU3)
+			offs = 1 << ch->index;
+		else
+			offs = 1 << (ch->index + 3);
+		if (start)
+			value |= offs;
+		else
+			value &= ~offs;
+		rzg2l_mtu3_shared_reg_write(ch, RZG2L_MTU3_TSTRA, value);
+	}
+
+	raw_spin_unlock_irqrestore(&mtu->lock, flags);
+}
+
+int rzg2l_mtu3_enable(struct rzg2l_mtu3_channel *ch)
+{
+	struct rzg2l_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
+	int ret;
+
+	ret = clk_enable(mtu->clk);
+	if (ret) {
+		dev_err(ch->dev, "ch%u: cannot enable clock\n",
+			ch->index);
+		return ret;
+	}
+
+	/* enable channel */
+	rzg2l_mtu3_start_stop_ch(ch, true);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rzg2l_mtu3_enable);
+
+void rzg2l_mtu3_disable(struct rzg2l_mtu3_channel *ch)
+{
+	struct rzg2l_mtu3 *mtu = dev_get_drvdata(ch->dev->parent);
+
+	/* disable channel */
+	rzg2l_mtu3_start_stop_ch(ch, false);
+	clk_disable(mtu->clk);
+}
+EXPORT_SYMBOL_GPL(rzg2l_mtu3_disable);
+
+static const unsigned int ch_reg_offsets[] = {
+	0x100, 0x180, 0x200, 0x000, 0x001, 0xa80, 0x800, 0x801, 0x400
+};
+
+static void rzg2l_mtu3_reset_assert(void *data)
+{
+	struct reset_control *rstc = data;
+
+	reset_control_assert(rstc);
+}
+
+static int rzg2l_mtu3_probe(struct platform_device *pdev)
+{
+	struct reset_control *rstc;
+	struct rzg2l_mtu3 *ddata;
+	unsigned int i;
+	int ret;
+
+	ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
+	if (!ddata)
+		return -ENOMEM;
+
+	ddata->mmio = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ddata->mmio))
+		return PTR_ERR(ddata->mmio);
+
+	rstc = devm_reset_control_get(&pdev->dev, NULL);
+	if (IS_ERR(rstc))
+		return PTR_ERR(rstc);
+
+	ret = devm_add_action_or_reset(&pdev->dev, rzg2l_mtu3_reset_assert,
+				       rstc);
+	if (ret < 0)
+		return ret;
+
+	ddata->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(ddata->clk))
+		return PTR_ERR(ddata->clk);
+
+	raw_spin_lock_init(&ddata->lock);
+	reset_control_deassert(rstc);
+
+	for (i = 0; i < RZG2L_MTU_NUM_CHANNELS; i++) {
+		ddata->channels[i].index = i;
+		ddata->channels[i].base = ddata->mmio + ch_reg_offsets[i];
+	}
+
+	platform_set_drvdata(pdev, ddata);
+
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+static int rzg2l_mtu3_remove(struct platform_device *pdev)
+{
+	of_platform_depopulate(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id rzg2l_mtu3_of_match[] = {
+	{ .compatible = "renesas,rzg2l-mtu3", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rzg2l_mtu3_of_match);
+
+static struct platform_driver rzg2l_mtu3_driver = {
+	.probe = rzg2l_mtu3_probe,
+	.remove = rzg2l_mtu3_remove,
+	.driver	= {
+		.name = "rzg2l-mtu3",
+		.of_match_table = rzg2l_mtu3_of_match,
+	},
+};
+module_platform_driver(rzg2l_mtu3_driver);
+
+MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/G2L MTU3 Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/rzg2l-mtu3.h b/include/linux/mfd/rzg2l-mtu3.h
new file mode 100644
index 000000000000..69d4323d1126
--- /dev/null
+++ b/include/linux/mfd/rzg2l-mtu3.h
@@ -0,0 +1,124 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+
+#ifndef __LINUX_RZG2L_MTU3_H__
+#define __LINUX_RZG2L_MTU3_H__
+
+#include <linux/clk.h>
+
+/* 8-bit shared register offsets macros */
+#define RZG2L_MTU3_TSTRA	0x080 /* Timer start register A */
+#define RZG2L_MTU3_TSTRB	0x880 /* Timer start register B */
+
+/* 16-bit shared register offset macros */
+#define RZG2L_MTU3_TDDRA	0x016 /* Timer dead time data register A */
+#define RZG2L_MTU3_TDDRB	0x816 /* Timer dead time data register B */
+#define RZG2L_MTU3_TCDRA	0x014 /* Timer cycle data register A */
+#define RZG2L_MTU3_TCDRB	0x814 /* Timer cycle data register B */
+#define RZG2L_MTU3_TCBRA	0x022 /* Timer cycle buffer register A */
+#define RZG2L_MTU3_TCBRB	0x822 /* Timer cycle buffer register B */
+#define RZG2L_MTU3_TCNTSA	0x020 /* Timer subcounter A */
+#define RZG2L_MTU3_TCNTSB	0x820 /* Timer subcounter B */
+
+/*
+ * MTU5 contains 3 timer counter registers and is totaly different
+ * from other channels, so we must separate its offset
+ */
+
+/* 8-bit register offset macros of MTU3 channels except MTU5 */
+#define RZG2L_MTU3_TIER		0 /* Timer interrupt register */
+#define RZG2L_MTU3_NFCR		1 /* Noise filter control register */
+#define RZG2L_MTU3_TSR		2 /* Timer status register */
+#define RZG2L_MTU3_TCR		3 /* Timer control register */
+#define RZG2L_MTU3_TCR2		4 /* Timer control register 2 */
+#define RZG2L_MTU3_TMDR1	5 /* Timer mode register 1 */
+#define RZG2L_MTU3_TIOR		6 /* Timer I/O control register */
+#define RZG2L_MTU3_TIORH	6 /* Timer I/O control register H */
+#define RZG2L_MTU3_TIORL	7 /* Timer I/O control register L */
+/* Only MTU3/4/6/7 have TBTM registers */
+#define RZG2L_MTU3_TBTM		8 /* Timer buffer operation transfer mode register */
+
+/* 8-bit MTU5 register offset macros */
+#define RZG2L_MTU3_TSTR		2 /* MTU5 Timer start register */
+#define RZG2L_MTU3_TCNTCMPCLR	3 /* MTU5 Timer compare match clear register */
+#define RZG2L_MTU3_TCRU		4 /* Timer control register U */
+#define RZG2L_MTU3_TCR2U	5 /* Timer control register 2U */
+#define RZG2L_MTU3_TIORU	6 /* Timer I/O control register U */
+#define RZG2L_MTU3_TCRV		7 /* Timer control register V */
+#define RZG2L_MTU3_TCR2V	8 /* Timer control register 2V */
+#define RZG2L_MTU3_TIORV	9 /* Timer I/O control register V */
+#define RZG2L_MTU3_TCRW		10 /* Timer control register W */
+#define RZG2L_MTU3_TCR2W	11 /* Timer control register 2W */
+#define RZG2L_MTU3_TIORW	12 /* Timer I/O control register W */
+
+/* 16-bit register offset macros of MTU3 channels except MTU5 */
+#define RZG2L_MTU3_TCNT		0 /* Timer counter */
+#define RZG2L_MTU3_TGRA		1 /* Timer general register A */
+#define RZG2L_MTU3_TGRB		2 /* Timer general register B */
+#define RZG2L_MTU3_TGRC		3 /* Timer general register C */
+#define RZG2L_MTU3_TGRD		4 /* Timer general register D */
+#define RZG2L_MTU3_TGRE		5 /* Timer general register E */
+#define RZG2L_MTU3_TGRF		6 /* Timer general register F */
+/* Timer A/D converter start request registers */
+#define RZG2L_MTU3_TADCR	7 /* control register */
+#define RZG2L_MTU3_TADCORA	8 /* cycle set register A */
+#define RZG2L_MTU3_TADCORB	9 /* cycle set register B */
+#define RZG2L_MTU3_TADCOBRA	10 /* cycle set buffer register A */
+#define RZG2L_MTU3_TADCOBRB	11 /* cycle set buffer register B */
+
+/* 16-bit MTU5 register offset macros */
+#define RZG2L_MTU3_TCNTU	0 /* MTU5 Timer counter U */
+#define RZG2L_MTU3_TGRU		1 /* MTU5 Timer general register U */
+#define RZG2L_MTU3_TCNTV	2 /* MTU5 Timer counter V */
+#define RZG2L_MTU3_TGRV		3 /* MTU5 Timer general register V */
+#define RZG2L_MTU3_TCNTW	4 /* MTU5 Timer counter W */
+#define RZG2L_MTU3_TGRW		5 /* MTU5 Timer general register W */
+
+/* Macros for setting registers */
+#define RZG2L_MTU3_TCR_CCLR_TGRA	BIT(5)
+
+enum rzg2l_mtu3_channels {
+	RZG2L_MTU0,
+	RZG2L_MTU1,
+	RZG2L_MTU2,
+	RZG2L_MTU3,
+	RZG2L_MTU4,
+	RZG2L_MTU5,
+	RZG2L_MTU6,
+	RZG2L_MTU7,
+	RZG2L_MTU8,
+	RZG2L_MTU_NUM_CHANNELS
+};
+
+enum rzg2l_mtu3_functions {
+	RZG2L_MTU3_NORMAL,
+	RZG2L_MTU3_16BIT_PHASE_COUNTING,
+};
+
+struct rzg2l_mtu3_channel {
+	struct device *dev;
+	unsigned int index;
+	void __iomem *base;
+	enum rzg2l_mtu3_functions function;
+};
+
+struct rzg2l_mtu3 {
+	struct clk *clk;
+	void __iomem *mmio;
+	raw_spinlock_t lock; /* Protect the shared registers */
+	struct rzg2l_mtu3_channel channels[RZG2L_MTU_NUM_CHANNELS];
+};
+
+void rzg2l_mtu3_disable(struct rzg2l_mtu3_channel *ch);
+int rzg2l_mtu3_enable(struct rzg2l_mtu3_channel *ch);
+
+u16 rzg2l_mtu3_shared_reg_read(struct rzg2l_mtu3_channel *ch, u16 off);
+u8 rzg2l_mtu3_8bit_ch_read(struct rzg2l_mtu3_channel *ch, u16 off);
+u16 rzg2l_mtu3_16bit_ch_read(struct rzg2l_mtu3_channel *ch, u16 off);
+
+void rzg2l_mtu3_8bit_ch_write(struct rzg2l_mtu3_channel *ch, u16 off, u8 val);
+void rzg2l_mtu3_16bit_ch_write(struct rzg2l_mtu3_channel *ch, u16 off, u16 val);
+
+#endif /* __LINUX_RZG2L_MTU3_H__ */
-- 
2.25.1

[PATCH RFC 4/8] dt-bindings: mfd: rzg2l-mtu3: Document RZ/G2UL MTU3 counter

[Biju Das]

Document 16-bit and 32-bit phase counting mode support on
RZ/G2L MTU3 IP.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../bindings/mfd/renesas,rzg2l-mtu3.yaml      | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
index c1fae8e8d9f9..c4bcf28623d6 100644
--- a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
+++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
@@ -192,6 +192,37 @@ properties:
   "#size-cells":
     const: 0
 
+patternProperties:
+  "^counter@[1-2]+$":
+    type: object
+
+    properties:
+      compatible:
+        const: renesas,rzg2l-mtu3-counter
+
+      reg:
+        description: Identify counter channels.
+        items:
+          enum: [ 1, 2 ]
+
+      renesas,32bit-phase-counting:
+        type: boolean
+        description: Enable 32-bit phase counting mode.
+
+      renesas,ext-input-phase-clock-select:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        enum: [ 0, 1 ]
+        default: 1
+        description: |
+          Selects the external clock pin for phase counting mode.
+            <0> : MTCLKA and MTCLKB are selected for the external phase clock.
+            <1> : MTCLKC and MTCLKD are selected for the external phase clock
+                  (default)
+
+    required:
+      - compatible
+      - reg
+
 required:
   - compatible
   - reg
@@ -270,6 +301,10 @@ examples:
       clocks = <&cpg CPG_MOD R9A07G044_MTU_X_MCK_MTU3>;
       power-domains = <&cpg>;
       resets = <&cpg R9A07G044_MTU_X_PRESET_MTU3>;
+      counter@1 {
+        compatible = "renesas,rzg2l-mtu3-counter";
+        reg = <1>;
+      };
     };
 
 ...
-- 
2.25.1

[PATCH RFC 5/8] counter: Add RZ/G2L MTU3 counter driver

[Biju Das]

Add RZ/G2L MTU3 counter driver. Currently it supports 16-bit phase
counting mode on MTU{1,2} channels.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/counter/Kconfig          |   9 +
 drivers/counter/Makefile         |   1 +
 drivers/counter/rzg2l-mtu3-cnt.c | 367 +++++++++++++++++++++++++++++++
 3 files changed, 377 insertions(+)
 create mode 100644 drivers/counter/rzg2l-mtu3-cnt.c

diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
index 5edd155f1911..6bdc0756f9c4 100644
--- a/drivers/counter/Kconfig
+++ b/drivers/counter/Kconfig
@@ -39,6 +39,15 @@ config INTERRUPT_CNT
 	  To compile this driver as a module, choose M here: the
 	  module will be called interrupt-cnt.
 
+config RZG2L_MTU3_CNT
+	tristate "RZ/G2L MTU3 counter driver"
+	depends on MFD_RZG2L_MTU3 || COMPILE_TEST
+	help
+	  Select this option to enable RZ/G2L MTU3 counter driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rzg2l-mtu3-cnt.
+
 config STM32_TIMER_CNT
 	tristate "STM32 Timer encoder counter driver"
 	depends on MFD_STM32_TIMERS || COMPILE_TEST
diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
index 8fde6c100ebc..f9138f3e14f7 100644
--- a/drivers/counter/Makefile
+++ b/drivers/counter/Makefile
@@ -8,6 +8,7 @@ counter-y := counter-core.o counter-sysfs.o counter-chrdev.o
 
 obj-$(CONFIG_104_QUAD_8)	+= 104-quad-8.o
 obj-$(CONFIG_INTERRUPT_CNT)		+= interrupt-cnt.o
+obj-$(CONFIG_RZG2L_MTU3_CNT)	+= rzg2l-mtu3-cnt.o
 obj-$(CONFIG_STM32_TIMER_CNT)	+= stm32-timer-cnt.o
 obj-$(CONFIG_STM32_LPTIMER_CNT)	+= stm32-lptimer-cnt.o
 obj-$(CONFIG_TI_EQEP)		+= ti-eqep.o
diff --git a/drivers/counter/rzg2l-mtu3-cnt.c b/drivers/counter/rzg2l-mtu3-cnt.c
new file mode 100644
index 000000000000..c324cd831f1d
--- /dev/null
+++ b/drivers/counter/rzg2l-mtu3-cnt.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G2L MTU3a Counter driver
+ *
+ * Copyright (C) 2022 Renesas Electronics Corporation
+ */
+#include <linux/counter.h>
+#include <linux/mfd/rzg2l-mtu3.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#define RZG2L_MTU3_TSR_TCFD	BIT(7)
+
+#define RZG2L_MTU3_TMDR1_PH_CNT_MODE_1	(4)
+#define RZG2L_MTU3_TMDR1_PH_CNT_MODE_2	(5)
+#define RZG2L_MTU3_TMDR1_PH_CNT_MODE_3	(6)
+#define RZG2L_MTU3_TMDR1_PH_CNT_MODE_4	(7)
+#define RZG2L_MTU3_TMDR1_PH_CNT_MODE_5	(9)
+#define RZG2L_MTU3_TMDR1_PH_CNT_MODE_MASK	(0xf)
+
+struct rzg2l_mtu3_cnt {
+	struct clk *clk;
+	void __iomem *mmio;
+	struct rzg2l_mtu3_channel *ch;
+};
+
+static const enum counter_function rzg2l_mtu3_count_functions[] = {
+	COUNTER_FUNCTION_QUADRATURE_X4,
+	COUNTER_FUNCTION_PULSE_DIRECTION,
+	COUNTER_FUNCTION_QUADRATURE_X2_B,
+};
+
+static int rzg2l_mtu3_count_read(struct counter_device *counter,
+				 struct counter_count *count, u64 *val)
+{
+	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
+	u32 cnt;
+
+	cnt = rzg2l_mtu3_16bit_ch_read(priv->ch, RZG2L_MTU3_TCNT);
+	*val = cnt;
+
+	return 0;
+}
+
+static int rzg2l_mtu3_count_write(struct counter_device *counter,
+				  struct counter_count *count, const u64 val)
+{
+	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
+	u16 ceiling;
+
+	ceiling = rzg2l_mtu3_16bit_ch_read(priv->ch, RZG2L_MTU3_TGRA);
+
+	if (val > ceiling)
+		return -EINVAL;
+
+	rzg2l_mtu3_16bit_ch_write(priv->ch, RZG2L_MTU3_TCNT, (u16)val);
+
+	return 0;
+}
+
+static int rzg2l_mtu3_count_function_read(struct counter_device *counter,
+					  struct counter_count *count,
+					  enum counter_function *function)
+{
+	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
+	u8 val;
+
+	val = rzg2l_mtu3_8bit_ch_read(priv->ch, RZG2L_MTU3_TMDR1);
+
+	switch (val & RZG2L_MTU3_TMDR1_PH_CNT_MODE_MASK) {
+	case RZG2L_MTU3_TMDR1_PH_CNT_MODE_1:
+		*function = COUNTER_FUNCTION_QUADRATURE_X4;
+		break;
+	case RZG2L_MTU3_TMDR1_PH_CNT_MODE_2:
+		*function = COUNTER_FUNCTION_PULSE_DIRECTION;
+		break;
+	case RZG2L_MTU3_TMDR1_PH_CNT_MODE_4:
+		*function = COUNTER_FUNCTION_QUADRATURE_X2_B;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int rzg2l_mtu3_count_function_write(struct counter_device *counter,
+					   struct counter_count *count,
+					   enum counter_function function)
+{
+	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
+	u8 mode;
+
+	switch (function) {
+	case COUNTER_FUNCTION_QUADRATURE_X4:
+		mode = RZG2L_MTU3_TMDR1_PH_CNT_MODE_1;
+		break;
+	case COUNTER_FUNCTION_PULSE_DIRECTION:
+		mode = RZG2L_MTU3_TMDR1_PH_CNT_MODE_2;
+		break;
+	case COUNTER_FUNCTION_QUADRATURE_X2_B:
+		mode = RZG2L_MTU3_TMDR1_PH_CNT_MODE_4;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TMDR1, mode);
+
+	return 0;
+}
+
+static int rzg2l_mtu3_count_direction_read(struct counter_device *counter,
+					   struct counter_count *count,
+					   enum counter_count_direction *direction)
+{
+	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
+	u8 cnt;
+
+	cnt = rzg2l_mtu3_8bit_ch_read(priv->ch, RZG2L_MTU3_TSR);
+
+	if (cnt & RZG2L_MTU3_TSR_TCFD)
+		*direction = COUNTER_COUNT_DIRECTION_FORWARD;
+	else
+		*direction = COUNTER_COUNT_DIRECTION_BACKWARD;
+
+	return 0;
+}
+
+static int rzg2l_mtu3_count_ceiling_read(struct counter_device *counter,
+					 struct counter_count *count,
+					 u64 *ceiling)
+{
+	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
+	u32 val;
+
+	val = rzg2l_mtu3_16bit_ch_read(priv->ch, RZG2L_MTU3_TGRA);
+	*ceiling = val;
+
+	return 0;
+}
+
+static int rzg2l_mtu3_count_ceiling_write(struct counter_device *counter,
+					  struct counter_count *count,
+					  u64 ceiling)
+{
+	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
+
+	if (ceiling > U16_MAX)
+		return -ERANGE;
+
+	rzg2l_mtu3_16bit_ch_write(priv->ch, RZG2L_MTU3_TGRA, (u16)ceiling);
+	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TCR,
+				 RZG2L_MTU3_TCR_CCLR_TGRA);
+
+	return 0;
+}
+
+static int rzg2l_mtu3_count_enable_read(struct counter_device *counter,
+					struct counter_count *count, u8 *enable)
+{
+	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
+	int ch = priv->ch->index;
+
+	*enable = (rzg2l_mtu3_shared_reg_read(priv->ch, RZG2L_MTU3_TSTRA) &
+		(0x1 << ch)) >> ch;
+
+	return 0;
+}
+
+static int rzg2l_mtu3_count_enable_write(struct counter_device *counter,
+					 struct counter_count *count, u8 enable)
+{
+	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
+
+	if (enable)
+		rzg2l_mtu3_enable(priv->ch);
+	else
+		rzg2l_mtu3_disable(priv->ch);
+
+	return 0;
+}
+
+static struct counter_comp rzg2l_mtu3_count_ext[] = {
+	COUNTER_COMP_DIRECTION(rzg2l_mtu3_count_direction_read),
+	COUNTER_COMP_ENABLE(rzg2l_mtu3_count_enable_read,
+			    rzg2l_mtu3_count_enable_write),
+	COUNTER_COMP_CEILING(rzg2l_mtu3_count_ceiling_read,
+			     rzg2l_mtu3_count_ceiling_write),
+};
+
+static const enum counter_synapse_action rzg2l_mtu3_synapse_actions[] = {
+	COUNTER_SYNAPSE_ACTION_NONE,
+	COUNTER_SYNAPSE_ACTION_BOTH_EDGES
+};
+
+static int rzg2l_mtu3_action_read(struct counter_device *counter,
+				  struct counter_count *count,
+				  struct counter_synapse *synapse,
+				  enum counter_synapse_action *action)
+{
+	enum counter_function function;
+	int err;
+
+	err = rzg2l_mtu3_count_function_read(counter, count, &function);
+	if (err)
+		return err;
+
+	switch (function) {
+	case COUNTER_FUNCTION_PULSE_DIRECTION:
+		/*
+		 * Rising edges on signal A updates the respective count.
+		 * The input level of signal B determines direction.
+		 */
+		*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
+		break;
+	case COUNTER_FUNCTION_QUADRATURE_X2_B:
+		/*
+		 * Any state transition on quadrature pair signal B updates
+		 * the respective count.
+		 */
+		*action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;
+		break;
+	case COUNTER_FUNCTION_QUADRATURE_X4:
+		/* counts up/down on both edges of A and B signal*/
+		*action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct counter_ops rzg2l_mtu3_cnt_ops = {
+	.count_read = rzg2l_mtu3_count_read,
+	.count_write = rzg2l_mtu3_count_write,
+	.function_read = rzg2l_mtu3_count_function_read,
+	.function_write = rzg2l_mtu3_count_function_write,
+	.action_read = rzg2l_mtu3_action_read,
+};
+
+static struct counter_signal rzg2l_mtu3_signals[] = {
+	{
+		.id = 0,
+		.name = "Channel 1 Quadrature A"
+	},
+	{
+		.id = 1,
+		.name = "Channel 1 Quadrature B"
+	}
+};
+
+static struct counter_synapse rzg2l_mtu3_count_synapses[] = {
+	{
+		.actions_list = rzg2l_mtu3_synapse_actions,
+		.num_actions = ARRAY_SIZE(rzg2l_mtu3_synapse_actions),
+		.signal = &rzg2l_mtu3_signals[0]
+	},
+	{
+		.actions_list = rzg2l_mtu3_synapse_actions,
+		.num_actions = ARRAY_SIZE(rzg2l_mtu3_synapse_actions),
+		.signal = &rzg2l_mtu3_signals[1]
+	}
+};
+
+static struct counter_count rzg2l_mtu3_counts = {
+	.id = 0,
+	.name = "Channel 1 Count",
+	.functions_list = rzg2l_mtu3_count_functions,
+	.num_functions = ARRAY_SIZE(rzg2l_mtu3_count_functions),
+	.synapses = rzg2l_mtu3_count_synapses,
+	.num_synapses = ARRAY_SIZE(rzg2l_mtu3_count_synapses),
+	.ext = rzg2l_mtu3_count_ext,
+	.num_ext = ARRAY_SIZE(rzg2l_mtu3_count_ext)
+};
+
+static int rzg2l_mtu3_cnt_probe(struct platform_device *pdev)
+{
+	struct rzg2l_mtu3 *ddata = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct counter_device *counter;
+	struct rzg2l_mtu3_cnt *priv;
+	int ret;
+	u32 ch;
+
+	if (IS_ERR_OR_NULL(ddata))
+		return -EINVAL;
+
+	counter = devm_counter_alloc(dev, sizeof(*priv));
+	if (!counter)
+		return -ENOMEM;
+
+	priv = counter_priv(counter);
+
+	ret = of_property_read_u32(dev->of_node, "reg", &ch);
+	if (ret) {
+		dev_err(dev, "%pOF: No reg property found\n", dev->of_node);
+		return -EINVAL;
+	}
+
+	if (ch != RZG2L_MTU1 && ch != RZG2L_MTU2) {
+		dev_err(dev, "%pOF: Invalid channel '%u'\n", dev->of_node, ch);
+		return -EINVAL;
+	}
+
+	priv->clk = ddata->clk;
+	priv->ch = &ddata->channels[ch];
+	priv->ch->dev = dev;
+
+	counter->name = dev_name(dev);
+	counter->parent = dev;
+	counter->ops = &rzg2l_mtu3_cnt_ops;
+	counter->counts = &rzg2l_mtu3_counts;
+	counter->num_counts = 1;
+	counter->signals = rzg2l_mtu3_signals;
+	counter->num_signals = ARRAY_SIZE(rzg2l_mtu3_signals);
+	platform_set_drvdata(pdev, priv);
+
+	/* Register Counter device */
+	ret = devm_counter_add(dev, counter);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to add counter\n");
+
+	priv->ch->function = RZG2L_MTU3_16BIT_PHASE_COUNTING;
+	ret = clk_prepare_enable(ddata->clk);
+	if (ret)
+		return ret;
+
+	/*
+	 * Phase counting mode 1 will be used as default
+	 * when initializing counters.
+	 */
+	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TMDR1,
+				 RZG2L_MTU3_TMDR1_PH_CNT_MODE_1);
+
+	/* Initialize 16-bit counter max value */
+	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TCR,
+				 RZG2L_MTU3_TCR_CCLR_TGRA);
+	rzg2l_mtu3_16bit_ch_write(priv->ch, RZG2L_MTU3_TGRA, U16_MAX);
+
+	clk_disable(ddata->clk);
+
+	return 0;
+}
+
+static const struct of_device_id rzg2l_mtu3_cnt_of_match[] = {
+	{ .compatible = "renesas,rzg2l-mtu3-counter", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rzg2l_mtu3_cnt_of_match);
+
+static struct platform_driver rzg2l_mtu3_cnt_driver = {
+	.probe = rzg2l_mtu3_cnt_probe,
+	.driver = {
+		.name = "rzg2l-mtu3-counter",
+		.of_match_table = rzg2l_mtu3_cnt_of_match,
+	},
+};
+module_platform_driver(rzg2l_mtu3_cnt_driver);
+
+MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
+MODULE_ALIAS("platform:rzg2l-mtu3-counter");
+MODULE_DESCRIPTION("Renesas RZ/G2L MTU3a counter driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH RFC 6/8] arm64: dts: renesas: r9a07g044: Add MTU3a node

[Biju Das]

Add MTU3a node to R9A07G044 (RZ/G2L) SoC DTSI.

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

diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index 2283d4fb8736..a8c31a27314c 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -168,6 +168,89 @@ soc: soc {
 		#size-cells = <2>;
 		ranges;
 
+		mtu3: timer@10001200 {
+			compatible = "renesas,r9a07g044-mtu3",
+				     "renesas,rzg2l-mtu3";
+			reg = <0 0x10001200 0 0xb00>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 172 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 173 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 174 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 175 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 176 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 178 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 179 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 180 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 181 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 182 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 183 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 184 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 185 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 186 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 190 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 191 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 192 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 194 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 195 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 198 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 199 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 200 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 201 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 202 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 203 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 204 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 205 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 206 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 207 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 208 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 209 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 210 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 211 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 212 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 213 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "tgia0", "tgib0", "tgic0", "tgid0",
+					  "tgiv0", "tgie0", "tgif0",
+					  "tgia1", "tgib1", "tgiv1", "tgiu1",
+					  "tgia2", "tgib2", "tgiv2", "tgiu2",
+					  "tgia3", "tgib3", "tgic3", "tgid3",
+					  "tgiv3",
+					  "tgia4", "tgib4", "tgic4", "tgid4",
+					  "tgiv4",
+					  "tgiu5", "tgiv5", "tgiw5",
+					  "tgia6", "tgib6", "tgic6", "tgid6",
+					  "tgiv6",
+					  "tgia7", "tgib7", "tgic7", "tgid7",
+					  "tgiv7",
+					  "tgia8", "tgib8", "tgic8", "tgid8",
+					  "tgiv8", "tgiu8";
+			clocks = <&cpg CPG_MOD R9A07G044_MTU_X_MCK_MTU3>;
+			power-domains = <&cpg>;
+			resets = <&cpg R9A07G044_MTU_X_PRESET_MTU3>;
+			status = "disabled";
+
+			counter@1 {
+				compatible = "renesas,rzg2l-mtu3-counter";
+				reg = <1>;
+				status = "disabled";
+			};
+
+			counter@2 {
+				compatible = "renesas,rzg2l-mtu3-counter";
+				reg = <2>;
+				status = "disabled";
+			};
+		};
+
 		ssi0: ssi@10049c00 {
 			compatible = "renesas,r9a07g044-ssi",
 				     "renesas,rz-ssi";
-- 
2.25.1

[PATCH RFC 7/8] arm64: dts: renesas: r9a07g054: Add MTU3a node

[Biju Das]

Add MTU3a node to R9A07G054 (RZ/V2L) SoC DTSI.

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

diff --git a/arch/arm64/boot/dts/renesas/r9a07g054.dtsi b/arch/arm64/boot/dts/renesas/r9a07g054.dtsi
index 358d4c34465f..da78a75bc4d6 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g054.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g054.dtsi
@@ -168,6 +168,89 @@ soc: soc {
 		#size-cells = <2>;
 		ranges;
 
+		mtu3: timer@10001200 {
+			compatible = "renesas,r9a07g054-mtu3",
+				     "renesas,rzg2l-mtu3";
+			reg = <0 0x10001200 0 0xb00>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 172 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 173 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 174 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 175 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 176 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 178 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 179 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 180 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 181 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 182 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 183 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 184 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 185 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 186 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 190 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 191 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 192 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 194 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 195 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 198 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 199 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 200 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 201 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 202 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 203 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 204 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 205 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 206 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 207 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 208 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 209 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 210 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 211 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 212 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 213 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "tgia0", "tgib0", "tgic0", "tgid0",
+					  "tgiv0", "tgie0", "tgif0",
+					  "tgia1", "tgib1", "tgiv1", "tgiu1",
+					  "tgia2", "tgib2", "tgiv2", "tgiu2",
+					  "tgia3", "tgib3", "tgic3", "tgid3",
+					  "tgiv3",
+					  "tgia4", "tgib4", "tgic4", "tgid4",
+					  "tgiv4",
+					  "tgiu5", "tgiv5", "tgiw5",
+					  "tgia6", "tgib6", "tgic6", "tgid6",
+					  "tgiv6",
+					  "tgia7", "tgib7", "tgic7", "tgid7",
+					  "tgiv7",
+					  "tgia8", "tgib8", "tgic8", "tgid8",
+					  "tgiv8", "tgiu8";
+			clocks = <&cpg CPG_MOD R9A07G054_MTU_X_MCK_MTU3>;
+			power-domains = <&cpg>;
+			resets = <&cpg R9A07G054_MTU_X_PRESET_MTU3>;
+			status = "disabled";
+
+			counter@1 {
+				compatible = "renesas,rzg2l-mtu3-counter";
+				reg = <1>;
+				status = "disabled";
+			};
+
+			counter@2 {
+				compatible = "renesas,rzg2l-mtu3-counter";
+				reg = <2>;
+				status = "disabled";
+			};
+		};
+
 		ssi0: ssi@10049c00 {
 			compatible = "renesas,r9a07g054-ssi",
 				     "renesas,rz-ssi";
-- 
2.25.1

[PATCH RFC 8/8] arm64: dts: renesas: rzg2l-smarc: [HACK] Enable MTU for 16-bit phase count testing

[Biju Das]

Enable MTU{1,2} for 16-bit phase count testing.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../boot/dts/renesas/r9a07g044l2-smarc.dts    |  2 --
 .../dts/renesas/rzg2l-smarc-pinfunction.dtsi  | 11 ++++++++
 arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi  | 25 ++++++++++++++++++-
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts
index bc2af6c92ccd..247b0b3f1b58 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts
+++ b/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dts
@@ -8,8 +8,6 @@
 /dts-v1/;
 #include "r9a07g044l2.dtsi"
 #include "rzg2l-smarc-som.dtsi"
-#include "rzg2l-smarc-pinfunction.dtsi"
-#include "rz-smarc-common.dtsi"
 #include "rzg2l-smarc.dtsi"
 
 / {
diff --git a/arch/arm64/boot/dts/renesas/rzg2l-smarc-pinfunction.dtsi b/arch/arm64/boot/dts/renesas/rzg2l-smarc-pinfunction.dtsi
index 9085d8c76ce1..8c25c9f31ec0 100644
--- a/arch/arm64/boot/dts/renesas/rzg2l-smarc-pinfunction.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc-pinfunction.dtsi
@@ -53,17 +53,28 @@ i2c3_pins: i2c3 {
 			 <RZG2L_PORT_PINMUX(18, 1, 3)>; /* SCL */
 	};
 
+#if (MTU3_PHASE_COUNTING_SUPPORT)
+	mtu3_pins: mtu3 {
+		mtu3_clk {
+			pinmux = <RZG2L_PORT_PINMUX(48, 0, 4)>, /* MTCLKA */
+				 <RZG2L_PORT_PINMUX(48, 1, 4)>; /* MTLCKB */
+		};
+	};
+#endif
+
 	scif0_pins: scif0 {
 		pinmux = <RZG2L_PORT_PINMUX(38, 0, 1)>,	/* TxD */
 			 <RZG2L_PORT_PINMUX(38, 1, 1)>;	/* RxD */
 	};
 
+#if (!MTU3_PHASE_COUNTING_SUPPORT)
 	scif2_pins: scif2 {
 		pinmux = <RZG2L_PORT_PINMUX(48, 0, 1)>, /* TxD */
 			 <RZG2L_PORT_PINMUX(48, 1, 1)>, /* RxD */
 			 <RZG2L_PORT_PINMUX(48, 3, 1)>, /* CTS# */
 			 <RZG2L_PORT_PINMUX(48, 4, 1)>; /* RTS# */
 	};
+#endif
 
 	sd1-pwr-en-hog {
 		gpio-hog;
diff --git a/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
index e180a955b6ac..79b3088d2eda 100644
--- a/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg2l-smarc.dtsi
@@ -9,7 +9,14 @@
 #include <dt-bindings/pinctrl/rzg2l-pinctrl.h>
 
 /* comment the #define statement to disable SCIF2 (SER0) on PMOD1 (CN7) */
-#define PMOD1_SER0	1
+#define PMOD1_SER0	0
+
+#if (!PMOD1_SER0)
+#define MTU3_PHASE_COUNTING_SUPPORT	1
+#endif
+
+#include "rzg2l-smarc-pinfunction.dtsi"
+#include "rz-smarc-common.dtsi"
 
 / {
 	aliases {
@@ -36,6 +43,22 @@ wm8978: codec@1a {
 	};
 };
 
+#if (MTU3_PHASE_COUNTING_SUPPORT)
+&mtu3 {
+	pinctrl-0 = <&mtu3_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+	counter@1 {
+		status = "okay";
+	};
+
+	counter@2 {
+		status = "okay";
+	};
+};
+#endif
+
 /*
  * To enable SCIF2 (SER0) on PMOD1 (CN7)
  * SW1 should be at position 2->3 so that SER0_CTS# line is activated
-- 
2.25.1

Re: [PATCH RFC 3/8] mfd: Add RZ/G2L MTU3 driver

[Philipp Zabel]

Hi Biju,

On Mo, 2022-09-26 at 14:21 +0100, Biju Das wrote:
[...]
> +static int rzg2l_mtu3_probe(struct platform_device *pdev)
> +{
> +	struct reset_control *rstc;
> +	struct rzg2l_mtu3 *ddata;
> +	unsigned int i;
> +	int ret;
> +
> +	ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
> +	if (!ddata)
> +		return -ENOMEM;
> +
> +	ddata->mmio = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(ddata->mmio))
> +		return PTR_ERR(ddata->mmio);
> +
> +	rstc = devm_reset_control_get(&pdev->dev, NULL);

Please use

+	rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);

instead.

regards
Philipp

RE: [PATCH RFC 3/8] mfd: Add RZ/G2L MTU3 driver

[Biju Das]

Hi Philipp,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 3/8] mfd: Add RZ/G2L MTU3 driver
> 
> Hi Biju,
> 
> On Mo, 2022-09-26 at 14:21 +0100, Biju Das wrote:
> [...]
> > +static int rzg2l_mtu3_probe(struct platform_device *pdev) {
> > +	struct reset_control *rstc;
> > +	struct rzg2l_mtu3 *ddata;
> > +	unsigned int i;
> > +	int ret;
> > +
> > +	ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
> > +	if (!ddata)
> > +		return -ENOMEM;
> > +
> > +	ddata->mmio = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(ddata->mmio))
> > +		return PTR_ERR(ddata->mmio);
> > +
> > +	rstc = devm_reset_control_get(&pdev->dev, NULL);
> 
> Please use
> 
> +	rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> 
> instead.

Agreed.

Cheers,
Biju

Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[William Breathitt Gray]

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

On Mon, Sep 26, 2022 at 02:21:06PM +0100, Biju Das wrote:
> The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
> the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
> channels and one 32-bit timer channel. It supports the following
> functions
>  - Counter
>  - Timer
>  - PWM
> 
> This patch series aim to add MFD and counter driver for MTU3a.
> Subsequent patch seies will add TImer and PWM driver support
> also enhancements to counter driver.

Hello Biju,

I see this device consists of several channels, but only one Count is
defined in the counter patch ("Channel 1 Count"). Do all channels
support counting, or is it limited to just one channel?

Thanks,

William Breathitt Gray

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

RE: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[Biju Das]

Hi William Breathitt Gray,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Mon, Sep 26, 2022 at 02:21:06PM +0100, Biju Das wrote:
> > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
> > the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
> > channels and one 32-bit timer channel. It supports the following
> > functions
> >  - Counter
> >  - Timer
> >  - PWM
> >
> > This patch series aim to add MFD and counter driver for MTU3a.
> > Subsequent patch seies will add TImer and PWM driver support also
> > enhancements to counter driver.
> 
> Hello Biju,
> 
> I see this device consists of several channels, but only one Count is
> defined in the counter patch ("Channel 1 Count"). Do all channels
> support counting, or is it limited to just one channel?

It is like this
MTU1 channel :- 1 16-bit phase counter
MTU2-Channel :- 1 16-bit phase counter
MTU1 + MTU2 channel combined:- 1 32-bit phase counter
Other channels are not supporting phase counting.

Each counter device will have 1 channel. Currently it supports
16-bit phase counting.

Please see my test program. Am I missing something here?

My test program:-

echo 1 > /sys/bus/counter/devices/counter0/count0/enable
echo 50 > /sys/bus/counter/devices/counter0/count0/ceiling
devmem2 0x10001391 b 0x00 # Enable phase clock selection A for MTU2.
echo 1 > /sys/bus/counter/devices/counter1/count0/enable
echo 50 > /sys/bus/counter/devices/counter1/count0/ceiling

for i in {1..5};
do cat /sys/bus/counter/devices/counter0/count0/count ;
cat /sys/bus/counter/devices/counter0/count0/direction;
cat /sys/bus/counter/devices/counter1/count0/count;
cat /sys/bus/counter/devices/counter1/count0/direction;
done

Cheers,
Biju

Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[William Breathitt Gray]

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

On Wed, Sep 28, 2022 at 06:14:57AM +0000, Biju Das wrote:
> Hi William Breathitt Gray,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> > 
> > On Mon, Sep 26, 2022 at 02:21:06PM +0100, Biju Das wrote:
> > > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
> > > the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
> > > channels and one 32-bit timer channel. It supports the following
> > > functions
> > >  - Counter
> > >  - Timer
> > >  - PWM
> > >
> > > This patch series aim to add MFD and counter driver for MTU3a.
> > > Subsequent patch seies will add TImer and PWM driver support also
> > > enhancements to counter driver.
> > 
> > Hello Biju,
> > 
> > I see this device consists of several channels, but only one Count is
> > defined in the counter patch ("Channel 1 Count"). Do all channels
> > support counting, or is it limited to just one channel?
> 
> It is like this
> MTU1 channel :- 1 16-bit phase counter
> MTU2-Channel :- 1 16-bit phase counter
> MTU1 + MTU2 channel combined:- 1 32-bit phase counter
> Other channels are not supporting phase counting.
> 
> Each counter device will have 1 channel. Currently it supports
> 16-bit phase counting.
> 
> Please see my test program. Am I missing something here?
> 
> My test program:-
> 
> echo 1 > /sys/bus/counter/devices/counter0/count0/enable
> echo 50 > /sys/bus/counter/devices/counter0/count0/ceiling
> devmem2 0x10001391 b 0x00 # Enable phase clock selection A for MTU2.
> echo 1 > /sys/bus/counter/devices/counter1/count0/enable
> echo 50 > /sys/bus/counter/devices/counter1/count0/ceiling
> 
> for i in {1..5};
> do cat /sys/bus/counter/devices/counter0/count0/count ;
> cat /sys/bus/counter/devices/counter0/count0/direction;
> cat /sys/bus/counter/devices/counter1/count0/count;
> cat /sys/bus/counter/devices/counter1/count0/direction;
> done
> 
> Cheers,
> Biju

I'm not familiar with this hardware, but it looks like MTU1 and MTU2 are
on the same device. I think a more natural way to expose this
functionality in the Counter subsystem would be to define a Count for
each count value you can support; so something like this (all under
/sys/bus/counter/devices/counter0):

* count0 :- MTU1
* count1 :- MTU2
* count3 :- MTU1 + MTU2

You can then control the phase selection using a top-level Counter
device extension (e.g. /sys/bus/counter/devices/counter0/phase) that
configures whether you're in 16-bit phase or 32-phase counting mode.

William Breathitt Gray

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

Re: [PATCH RFC 5/8] counter: Add RZ/G2L MTU3 counter driver

[William Breathitt Gray]

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

On Mon, Sep 26, 2022 at 02:21:11PM +0100, Biju Das wrote:
> Add RZ/G2L MTU3 counter driver. Currently it supports 16-bit phase
> counting mode on MTU{1,2} channels.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Hi Biju,

This driver will likely change in your next revision, but I want to give
some feedback anyway on a few things I noticed. See the comments below.

> +struct rzg2l_mtu3_cnt {
> +	struct clk *clk;
> +	void __iomem *mmio;
> +	struct rzg2l_mtu3_channel *ch;
> +};

Add kernel-doc comments to document this structure. It seems that
neither clk nor mmio is access in the code from this structure; what's
the purpose of having them here?

> +static int rzg2l_mtu3_count_read(struct counter_device *counter,
> +				 struct counter_count *count, u64 *val)
> +{
> +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> +	u32 cnt;
> +
> +	cnt = rzg2l_mtu3_16bit_ch_read(priv->ch, RZG2L_MTU3_TCNT);
> +	*val = cnt;

The rzg2l_mtu3_16bit_ch_read() function returns a u16, so there's no
need for the cnt variable; just return the count via val directly.

> +static int rzg2l_mtu3_count_write(struct counter_device *counter,
> +				  struct counter_count *count, const u64 val)
> +{
> +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> +	u16 ceiling;
> +
> +	ceiling = rzg2l_mtu3_16bit_ch_read(priv->ch, RZG2L_MTU3_TGRA);
> +
> +	if (val > ceiling)
> +		return -EINVAL;

Return -ERANGE instead to indicate the request is outside the boundary.

> +
> +	rzg2l_mtu3_16bit_ch_write(priv->ch, RZG2L_MTU3_TCNT, (u16)val);

Remove the explicit cast to u16, it's already implicit in the call. You
probably also need some sort of lock in this function to ensure that
your ceiling value does not change before you write to the register.

> +static int rzg2l_mtu3_count_ceiling_read(struct counter_device *counter,
> +					 struct counter_count *count,
> +					 u64 *ceiling)
> +{
> +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> +	u32 val;
> +
> +	val = rzg2l_mtu3_16bit_ch_read(priv->ch, RZG2L_MTU3_TGRA);
> +	*ceiling = val;

Same comment as in rzg2l_mtu3_count_read().

> +static int rzg2l_mtu3_count_ceiling_write(struct counter_device *counter,
> +					  struct counter_count *count,
> +					  u64 ceiling)
> +{
> +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> +
> +	if (ceiling > U16_MAX)
> +		return -ERANGE;
> +
> +	rzg2l_mtu3_16bit_ch_write(priv->ch, RZG2L_MTU3_TGRA, (u16)ceiling);
> +	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TCR,
> +				 RZG2L_MTU3_TCR_CCLR_TGRA);

Same comments about cast and lock as in rzg2l_mtu3_count_write().

> +static int rzg2l_mtu3_count_enable_read(struct counter_device *counter,
> +					struct counter_count *count, u8 *enable)
> +{
> +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> +	int ch = priv->ch->index;
> +
> +	*enable = (rzg2l_mtu3_shared_reg_read(priv->ch, RZG2L_MTU3_TSTRA) &
> +		(0x1 << ch)) >> ch;

A lot of operations happening in a single line; can this be broken down
to clearer distinct steps?

> +static int rzg2l_mtu3_action_read(struct counter_device *counter,
> +				  struct counter_count *count,
> +				  struct counter_synapse *synapse,
> +				  enum counter_synapse_action *action)
> +{
> +	enum counter_function function;
> +	int err;
> +
> +	err = rzg2l_mtu3_count_function_read(counter, count, &function);
> +	if (err)
> +		return err;
> +
> +	switch (function) {
> +	case COUNTER_FUNCTION_PULSE_DIRECTION:
> +		/*
> +		 * Rising edges on signal A updates the respective count.
> +		 * The input level of signal B determines direction.
> +		 */
> +		*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;

You need to differentiate between signal A and B here: the Synapse for
signal A will have an action mode of COUNTER_SYNAPSE_ACTION_RING_EDGE,
but the Synapse for Signal B will have an action mode of
COUNTER_SYNAPSE_ACTION_NONE because its not the trigger point for the
respective Count value update.

> +		break;
> +	case COUNTER_FUNCTION_QUADRATURE_X2_B:
> +		/*
> +		 * Any state transition on quadrature pair signal B updates
> +		 * the respective count.
> +		 */
> +		*action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;

Similar to above, you need to differentiate between signal A and B here
as well.

> +static struct counter_count rzg2l_mtu3_counts = {
> +	.id = 0,

The id member is an optional way for driver authors to identify their
own Counts; it can be set to anything your like, and if you don't use
it in your code then you don't need to set it at all.

> +static int rzg2l_mtu3_cnt_probe(struct platform_device *pdev)
> +{
> +	struct rzg2l_mtu3 *ddata = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct counter_device *counter;
> +	struct rzg2l_mtu3_cnt *priv;
> +	int ret;
> +	u32 ch;
> +
> +	if (IS_ERR_OR_NULL(ddata))
> +		return -EINVAL;

Is this actually possible? What situation would cause this, and why is
it not handled before we reach probe()?

> +
> +	counter = devm_counter_alloc(dev, sizeof(*priv));
> +	if (!counter)
> +		return -ENOMEM;
> +
> +	priv = counter_priv(counter);
> +
> +	ret = of_property_read_u32(dev->of_node, "reg", &ch);
> +	if (ret) {
> +		dev_err(dev, "%pOF: No reg property found\n", dev->of_node);
> +		return -EINVAL;
> +	}
> +
> +	if (ch != RZG2L_MTU1 && ch != RZG2L_MTU2) {
> +		dev_err(dev, "%pOF: Invalid channel '%u'\n", dev->of_node, ch);
> +		return -EINVAL;
> +	}
> +
> +	priv->clk = ddata->clk;
> +	priv->ch = &ddata->channels[ch];
> +	priv->ch->dev = dev;
> +
> +	counter->name = dev_name(dev);
> +	counter->parent = dev;
> +	counter->ops = &rzg2l_mtu3_cnt_ops;
> +	counter->counts = &rzg2l_mtu3_counts;
> +	counter->num_counts = 1;

Even though you only have one Count defined, use ARRAY_SIZE here for
consistency with the other Counter drivers as well as making the
intention of the code clear.

> +	counter->signals = rzg2l_mtu3_signals;
> +	counter->num_signals = ARRAY_SIZE(rzg2l_mtu3_signals);
> +	platform_set_drvdata(pdev, priv);
> +
> +	/* Register Counter device */
> +	ret = devm_counter_add(dev, counter);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to add counter\n");

The Counter driver goes live with the call to devm_counter_add() so move
it to the end after your device initialization code below.

> +
> +	priv->ch->function = RZG2L_MTU3_16BIT_PHASE_COUNTING;
> +	ret = clk_prepare_enable(ddata->clk);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Phase counting mode 1 will be used as default
> +	 * when initializing counters.
> +	 */
> +	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TMDR1,
> +				 RZG2L_MTU3_TMDR1_PH_CNT_MODE_1);
> +
> +	/* Initialize 16-bit counter max value */
> +	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TCR,
> +				 RZG2L_MTU3_TCR_CCLR_TGRA);
> +	rzg2l_mtu3_16bit_ch_write(priv->ch, RZG2L_MTU3_TGRA, U16_MAX);
> +
> +	clk_disable(ddata->clk);

Should this be moved up near the clk_prepare_enable() call above?

> +MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
> +MODULE_ALIAS("platform:rzg2l-mtu3-counter");
> +MODULE_DESCRIPTION("Renesas RZ/G2L MTU3a counter driver");
> +MODULE_LICENSE("GPL");

Add MODULE_IMPORT_NS(COUNTER) to import the COUNTER namespace.

Make sure you're based on top of the counter-next branch. You can find
the Counter tree here: https://git.kernel.org/pub/scm/linux/kernel/git/wbg/counter.git

William Breathitt Gray

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

RE: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[Biju Das]

Hi William Breathitt Gray,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Wed, Sep 28, 2022 at 06:14:57AM +0000, Biju Das wrote:
> > Hi William Breathitt Gray,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter
> driver
> > >
> > > On Mon, Sep 26, 2022 at 02:21:06PM +0100, Biju Das wrote:
> > > > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded
> > > > in the Renesas RZ/G2L family SoC's. It consists of eight 16-bit
> > > > timer channels and one 32-bit timer channel. It supports the
> > > > following functions
> > > >  - Counter
> > > >  - Timer
> > > >  - PWM
> > > >
> > > > This patch series aim to add MFD and counter driver for MTU3a.
> > > > Subsequent patch seies will add TImer and PWM driver support
> also
> > > > enhancements to counter driver.
> > >
> > > Hello Biju,
> > >
> > > I see this device consists of several channels, but only one Count
> > > is defined in the counter patch ("Channel 1 Count"). Do all
> channels
> > > support counting, or is it limited to just one channel?
> >
> > It is like this
> > MTU1 channel :- 1 16-bit phase counter MTU2-Channel :- 1 16-bit
> phase
> > counter
> > MTU1 + MTU2 channel combined:- 1 32-bit phase counter Other channels
> > are not supporting phase counting.
> >
> > Each counter device will have 1 channel. Currently it supports 16-
> bit
> > phase counting.
> >
> > Please see my test program. Am I missing something here?
> >
> > My test program:-
> >
> > echo 1 > /sys/bus/counter/devices/counter0/count0/enable
> > echo 50 > /sys/bus/counter/devices/counter0/count0/ceiling
> > devmem2 0x10001391 b 0x00 # Enable phase clock selection A for MTU2.
> > echo 1 > /sys/bus/counter/devices/counter1/count0/enable
> > echo 50 > /sys/bus/counter/devices/counter1/count0/ceiling
> >
> > for i in {1..5};
> > do cat /sys/bus/counter/devices/counter0/count0/count ; cat
> > /sys/bus/counter/devices/counter0/count0/direction;
> > cat /sys/bus/counter/devices/counter1/count0/count;
> > cat /sys/bus/counter/devices/counter1/count0/direction;
> > done
> >
> > Cheers,
> > Biju
> 
> I'm not familiar with this hardware, but it looks like MTU1 and MTU2
> are on the same device. I think a more natural way to expose this
> functionality in the Counter subsystem would be to define a Count for
> each count value you can support; so something like this (all under
> /sys/bus/counter/devices/counter0):
> 
> * count0 :- MTU1
> * count1 :- MTU2
> * count3 :- MTU1 + MTU2

OK, sounds good. so count3 :- 32 bit phase counting
count 0 or count1 or both then 16 bit phase counting

> 
> You can then control the phase selection using a top-level Counter
> device extension (e.g. /sys/bus/counter/devices/counter0/phase) that
> configures whether you're in 16-bit phase or 32-phase counting mode.

So I need to introduce a new sysfs called phase. Use that one for
Selecting the external clock pin for phase counting mode.
Please correct me if I am wrong??

Hardware supports 4 pins for phase counting mode,

MTCLKA Input External clock A input pin (MTU1/MTU2 phase counting mode A phase input)
MTCLKB Input External clock B input pin (MTU1/MTU2 phase counting mode B phase input)
MTCLKC Input External clock C input pin (MTU2 phase counting mode A phase input)
MTCLKD Input External clock D input pin (MTU2 phase counting mode B phase input)

For MTU1, it is fixed MTCLKA and MTCLKB.
But for MTU2, it can be either 0-{ MTCLKA, MTCLKB} or 1 - { MTCLKC , MTCLKD}
On reset it is set to { MTCLKC , MTCLKD}.

Cheers,
Biju

Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[William Breathitt Gray]

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

On Sat, Oct 01, 2022 at 04:45:55PM +0000, Biju Das wrote:
> Hi William Breathitt Gray,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> > 
> > On Wed, Sep 28, 2022 at 06:14:57AM +0000, Biju Das wrote:
> > > Hi William Breathitt Gray,
> > >
> > > Thanks for the feedback.
> > >
> > > > Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter
> > driver
> > > >
> > > > On Mon, Sep 26, 2022 at 02:21:06PM +0100, Biju Das wrote:
> > > > > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded
> > > > > in the Renesas RZ/G2L family SoC's. It consists of eight 16-bit
> > > > > timer channels and one 32-bit timer channel. It supports the
> > > > > following functions
> > > > >  - Counter
> > > > >  - Timer
> > > > >  - PWM
> > > > >
> > > > > This patch series aim to add MFD and counter driver for MTU3a.
> > > > > Subsequent patch seies will add TImer and PWM driver support
> > also
> > > > > enhancements to counter driver.
> > > >
> > > > Hello Biju,
> > > >
> > > > I see this device consists of several channels, but only one Count
> > > > is defined in the counter patch ("Channel 1 Count"). Do all
> > channels
> > > > support counting, or is it limited to just one channel?
> > >
> > > It is like this
> > > MTU1 channel :- 1 16-bit phase counter MTU2-Channel :- 1 16-bit
> > phase
> > > counter
> > > MTU1 + MTU2 channel combined:- 1 32-bit phase counter Other channels
> > > are not supporting phase counting.
> > >
> > > Each counter device will have 1 channel. Currently it supports 16-
> > bit
> > > phase counting.
> > >
> > > Please see my test program. Am I missing something here?
> > >
> > > My test program:-
> > >
> > > echo 1 > /sys/bus/counter/devices/counter0/count0/enable
> > > echo 50 > /sys/bus/counter/devices/counter0/count0/ceiling
> > > devmem2 0x10001391 b 0x00 # Enable phase clock selection A for MTU2.
> > > echo 1 > /sys/bus/counter/devices/counter1/count0/enable
> > > echo 50 > /sys/bus/counter/devices/counter1/count0/ceiling
> > >
> > > for i in {1..5};
> > > do cat /sys/bus/counter/devices/counter0/count0/count ; cat
> > > /sys/bus/counter/devices/counter0/count0/direction;
> > > cat /sys/bus/counter/devices/counter1/count0/count;
> > > cat /sys/bus/counter/devices/counter1/count0/direction;
> > > done
> > >
> > > Cheers,
> > > Biju
> > 
> > I'm not familiar with this hardware, but it looks like MTU1 and MTU2
> > are on the same device. I think a more natural way to expose this
> > functionality in the Counter subsystem would be to define a Count for
> > each count value you can support; so something like this (all under
> > /sys/bus/counter/devices/counter0):
> > 
> > * count0 :- MTU1
> > * count1 :- MTU2
> > * count3 :- MTU1 + MTU2
> 
> OK, sounds good. so count3 :- 32 bit phase counting
> count 0 or count1 or both then 16 bit phase counting

That "count3" should be "count2" (sorry for the typo), but yes all three
Counts should be defined; if a particular Count can't be read/written
due to the current phase counting mode selected, you can return -EBUSY
or -EINVAL as appropriate.

To clarify one more time, do you have two 16-bit registers holding count
values (one for MTU1 and one for MTU2), and when configured for 32-bit
phase counting mode you combine both registers to give you a 32-bit
count value?

> > 
> > You can then control the phase selection using a top-level Counter
> > device extension (e.g. /sys/bus/counter/devices/counter0/phase) that
> > configures whether you're in 16-bit phase or 32-phase counting mode.
> 
> So I need to introduce a new sysfs called phase. Use that one for
> Selecting the external clock pin for phase counting mode.
> Please correct me if I am wrong??
> 
> Hardware supports 4 pins for phase counting mode,
> 
> MTCLKA Input External clock A input pin (MTU1/MTU2 phase counting mode A phase input)
> MTCLKB Input External clock B input pin (MTU1/MTU2 phase counting mode B phase input)
> MTCLKC Input External clock C input pin (MTU2 phase counting mode A phase input)
> MTCLKD Input External clock D input pin (MTU2 phase counting mode B phase input)
> 
> For MTU1, it is fixed MTCLKA and MTCLKB.
> But for MTU2, it can be either 0-{ MTCLKA, MTCLKB} or 1 - { MTCLKC , MTCLKD}
> On reset it is set to { MTCLKC , MTCLKD}.
> 
> Cheers,
> Biju

It doesn't need to be named "phase" specifically, but it seems like a
new sysfs file will be necessary in order to select the proper phase
counting mode.

Are these MTCLK signals the quadrature A and B Signals you defined in
the counter driver?

William Breathitt Gray

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

RE: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[Biju Das]

> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Sat, Oct 01, 2022 at 04:45:55PM +0000, Biju Das wrote:
> > Hi William Breathitt Gray,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter
> driver
> > >
> > > On Wed, Sep 28, 2022 at 06:14:57AM +0000, Biju Das wrote:
> > > > Hi William Breathitt Gray,
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > > Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter
> > > driver
> > > > >
> > > > > On Mon, Sep 26, 2022 at 02:21:06PM +0100, Biju Das wrote:
> > > > > > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is
> > > > > > embedded in the Renesas RZ/G2L family SoC's. It consists of
> > > > > > eight 16-bit timer channels and one 32-bit timer channel. It
> > > > > > supports the following functions
> > > > > >  - Counter
> > > > > >  - Timer
> > > > > >  - PWM
> > > > > >
> > > > > > This patch series aim to add MFD and counter driver for
> MTU3a.
> > > > > > Subsequent patch seies will add TImer and PWM driver support
> > > also
> > > > > > enhancements to counter driver.
> > > > >
> > > > > Hello Biju,
> > > > >
> > > > > I see this device consists of several channels, but only one
> > > > > Count is defined in the counter patch ("Channel 1 Count"). Do
> > > > > all
> > > channels
> > > > > support counting, or is it limited to just one channel?
> > > >
> > > > It is like this
> > > > MTU1 channel :- 1 16-bit phase counter MTU2-Channel :- 1 16-bit
> > > phase
> > > > counter
> > > > MTU1 + MTU2 channel combined:- 1 32-bit phase counter Other
> > > > channels are not supporting phase counting.
> > > >
> > > > Each counter device will have 1 channel. Currently it supports
> 16-
> > > bit
> > > > phase counting.
> > > >
> > > > Please see my test program. Am I missing something here?
> > > >
> > > > My test program:-
> > > >
> > > > echo 1 > /sys/bus/counter/devices/counter0/count0/enable
> > > > echo 50 > /sys/bus/counter/devices/counter0/count0/ceiling
> > > > devmem2 0x10001391 b 0x00 # Enable phase clock selection A for
> MTU2.
> > > > echo 1 > /sys/bus/counter/devices/counter1/count0/enable
> > > > echo 50 > /sys/bus/counter/devices/counter1/count0/ceiling
> > > >
> > > > for i in {1..5};
> > > > do cat /sys/bus/counter/devices/counter0/count0/count ; cat
> > > > /sys/bus/counter/devices/counter0/count0/direction;
> > > > cat /sys/bus/counter/devices/counter1/count0/count;
> > > > cat /sys/bus/counter/devices/counter1/count0/direction;
> > > > done
> > > >
> > > > Cheers,
> > > > Biju
> > >
> > > I'm not familiar with this hardware, but it looks like MTU1 and
> MTU2
> > > are on the same device. I think a more natural way to expose this
> > > functionality in the Counter subsystem would be to define a Count
> > > for each count value you can support; so something like this (all
> > > under
> > > /sys/bus/counter/devices/counter0):
> > >
> > > * count0 :- MTU1
> > > * count1 :- MTU2
> > > * count3 :- MTU1 + MTU2
> >
> > OK, sounds good. so count3 :- 32 bit phase counting count 0 or
> count1
> > or both then 16 bit phase counting
> 
> That "count3" should be "count2" (sorry for the typo), but yes all
> three Counts should be defined; if a particular Count can't be
> read/written due to the current phase counting mode selected, you can
> return -EBUSY or -EINVAL as appropriate.
> 

OK.

> To clarify one more time, do you have two 16-bit registers holding
> count values (one for MTU1 and one for MTU2), and when configured for
> 32-bit phase counting mode you combine both registers to give you a
> 32-bit count value?

Yes, that is correct.

> 
> > >
> > > You can then control the phase selection using a top-level Counter
> > > device extension (e.g. /sys/bus/counter/devices/counter0/phase)
> that
> > > configures whether you're in 16-bit phase or 32-phase counting
> mode.
> >
> > So I need to introduce a new sysfs called phase. Use that one for
> > Selecting the external clock pin for phase counting mode.
> > Please correct me if I am wrong??
> >
> > Hardware supports 4 pins for phase counting mode,
> >
> > MTCLKA Input External clock A input pin (MTU1/MTU2 phase counting
> mode
> > A phase input) MTCLKB Input External clock B input pin (MTU1/MTU2
> > phase counting mode B phase input) MTCLKC Input External clock C
> input
> > pin (MTU2 phase counting mode A phase input) MTCLKD Input External
> > clock D input pin (MTU2 phase counting mode B phase input)
> >
> > For MTU1, it is fixed MTCLKA and MTCLKB.
> > But for MTU2, it can be either 0-{ MTCLKA, MTCLKB} or 1 - { MTCLKC ,
> > MTCLKD} On reset it is set to { MTCLKC , MTCLKD}.
> >
> > Cheers,
> > Biju
> 
> It doesn't need to be named "phase" specifically, but it seems like a
> new sysfs file will be necessary in order to select the proper phase
> counting mode.
> 
> Are these MTCLK signals the quadrature A and B Signals you defined in
> the counter driver?

Yes, that is correct.

Cheers,
Biju

Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[William Breathitt Gray]

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

On Sat, Oct 01, 2022 at 05:12:56PM +0000, Biju Das wrote:
> > > > You can then control the phase selection using a top-level Counter
> > > > device extension (e.g. /sys/bus/counter/devices/counter0/phase)
> > that
> > > > configures whether you're in 16-bit phase or 32-phase counting
> > mode.
> > >
> > > So I need to introduce a new sysfs called phase. Use that one for
> > > Selecting the external clock pin for phase counting mode.
> > > Please correct me if I am wrong??
> > >
> > > Hardware supports 4 pins for phase counting mode,
> > >
> > > MTCLKA Input External clock A input pin (MTU1/MTU2 phase counting
> > mode
> > > A phase input) MTCLKB Input External clock B input pin (MTU1/MTU2
> > > phase counting mode B phase input) MTCLKC Input External clock C
> > input
> > > pin (MTU2 phase counting mode A phase input) MTCLKD Input External
> > > clock D input pin (MTU2 phase counting mode B phase input)
> > >
> > > For MTU1, it is fixed MTCLKA and MTCLKB.
> > > But for MTU2, it can be either 0-{ MTCLKA, MTCLKB} or 1 - { MTCLKC ,
> > > MTCLKD} On reset it is set to { MTCLKC , MTCLKD}.
> > >
> > > Cheers,
> > > Biju
> > 
> > It doesn't need to be named "phase" specifically, but it seems like a
> > new sysfs file will be necessary in order to select the proper phase
> > counting mode.
> > 
> > Are these MTCLK signals the quadrature A and B Signals you defined in
> > the counter driver?
> 
> Yes, that is correct.
> 
> Cheers,
> Biju

You should define a Signal then for each of the four MTCLK inputs.
Create synapse arrays for each Count respectively; e.g. Count 0 will
have Synapses for MTCLKA and MTCLKB, but Count 1 will probably need
Synapses for all four Signals (the action mode for two of them will be
COUNTER_SYNAPSE_ACTION_NONE depending on the configuration set).

What is the configuration when 32-bit phase counting mode is selected?
Does MTCLKA and MTCLKB serve as the counting signals in this case, with
overflows on the MTU1 register incrementing the MTU2 register?

William Breathitt Gray

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

RE: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[Biju Das]

> -----Original Message-----
> From: William Breathitt Gray <william.gray@linaro.org>
> Sent: 01 October 2022 18:44
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: William Breathitt Gray <wbg@kernel.org>; Rob Herring
> <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; Philipp Zabel
> <p.zabel@pengutronix.de>; Michael Turquette <mturquette@baylibre.com>;
> Stephen Boyd <sboyd@kernel.org>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Lee Jones <lee@kernel.org>; Uwe Kleine-
> König <u.kleine-koenig@pengutronix.de>; linux-pwm@vger.kernel.org;
> linux-iio@vger.kernel.org; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; Chris Paterson
> <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>;
> Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
> linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Sat, Oct 01, 2022 at 05:12:56PM +0000, Biju Das wrote:
> > > > > You can then control the phase selection using a top-level
> > > > > Counter device extension (e.g.
> > > > > /sys/bus/counter/devices/counter0/phase)
> > > that
> > > > > configures whether you're in 16-bit phase or 32-phase counting
> > > mode.
> > > >
> > > > So I need to introduce a new sysfs called phase. Use that one
> for
> > > > Selecting the external clock pin for phase counting mode.
> > > > Please correct me if I am wrong??
> > > >
> > > > Hardware supports 4 pins for phase counting mode,
> > > >
> > > > MTCLKA Input External clock A input pin (MTU1/MTU2 phase
> counting
> > > mode
> > > > A phase input) MTCLKB Input External clock B input pin
> (MTU1/MTU2
> > > > phase counting mode B phase input) MTCLKC Input External clock C
> > > input
> > > > pin (MTU2 phase counting mode A phase input) MTCLKD Input
> External
> > > > clock D input pin (MTU2 phase counting mode B phase input)
> > > >
> > > > For MTU1, it is fixed MTCLKA and MTCLKB.
> > > > But for MTU2, it can be either 0-{ MTCLKA, MTCLKB} or 1 - {
> MTCLKC
> > > > , MTCLKD} On reset it is set to { MTCLKC , MTCLKD}.
> > > >
> > > > Cheers,
> > > > Biju
> > >
> > > It doesn't need to be named "phase" specifically, but it seems
> like
> > > a new sysfs file will be necessary in order to select the proper
> > > phase counting mode.
> > >
> > > Are these MTCLK signals the quadrature A and B Signals you defined
> > > in the counter driver?
> >
> > Yes, that is correct.
> >
> > Cheers,
> > Biju
> 
> You should define a Signal then for each of the four MTCLK inputs.
> Create synapse arrays for each Count respectively; e.g. Count 0 will
> have Synapses for MTCLKA and MTCLKB, but Count 1 will probably need
> Synapses for all four Signals (the action mode for two of them will be
> COUNTER_SYNAPSE_ACTION_NONE depending on the configuration set).

OK.

> 
> What is the configuration when 32-bit phase counting mode is selected?

LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to set for 32-bit phase counting mode.

b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
0: 16-bit access is enabled.
1: 32-bit access is enabled.

> Does MTCLKA and MTCLKB serve as the counting signals in this case,

For 16-bit and 32-bit counting signals same. We can set 

1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB

Or 

2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC and MTCLKD


b1 PHCKSEL 1 R/W External Input Phase Clock Select
Selects the external clock pin for phase counting mode.
0: MTCLKA and MTCLKB are selected for the external phase clock.
1: MTCLKC and MTCLKD are selected for the external phase clock

> with overflows on the MTU1 register incrementing the MTU2 register?

No. that won't happen as we need to use different register for Long word access

These are the regiters used
16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW

Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
Counter in MTU2   MTU2.TCNT Word

General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW Longword
General register A in MTU2 MTU2.TGRA Word

General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW Longword
General register B in MTU2 MTU2.TGRB Word

Cheers,
Biju

Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[William Breathitt Gray]

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

On Sat, Oct 01, 2022 at 06:03:37PM +0000, Biju Das wrote:
> > What is the configuration when 32-bit phase counting mode is selected?
> 
> LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to set for 32-bit phase counting mode.
> 
> b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
> 0: 16-bit access is enabled.
> 1: 32-bit access is enabled.
> 
> > Does MTCLKA and MTCLKB serve as the counting signals in this case,
> 
> For 16-bit and 32-bit counting signals same. We can set 
> 
> 1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB
> 
> Or 
> 
> 2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC and MTCLKD

I'm having trouble understanding this case. If 32-bit access is enabled
by setting the LWA bit, and the MTU1 signals are configured as MTCLKA
and MTCLKB while at the same time the MTU2 signals are configured as
MTCLKC and MTCLKD, how is the 32-bit count value determined -- wouldn't
MTU1 and MTU2 be counting independently if they each had separate input
clocks fed to them?

William Breathitt Gray

> 
> 
> b1 PHCKSEL 1 R/W External Input Phase Clock Select
> Selects the external clock pin for phase counting mode.
> 0: MTCLKA and MTCLKB are selected for the external phase clock.
> 1: MTCLKC and MTCLKD are selected for the external phase clock
> 
> > with overflows on the MTU1 register incrementing the MTU2 register?
> 
> No. that won't happen as we need to use different register for Long word access
> 
> These are the regiters used
> 16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
> 32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW
> 
> Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
> Counter in MTU2   MTU2.TCNT Word
> 
> General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW Longword
> General register A in MTU2 MTU2.TGRA Word
> 
> General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW Longword
> General register B in MTU2 MTU2.TGRB Word
> 
> Cheers,
> Biju

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

RE: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[Biju Das]

> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Sat, Oct 01, 2022 at 06:03:37PM +0000, Biju Das wrote:
> > > What is the configuration when 32-bit phase counting mode is
> selected?
> >
> > LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to set
> for 32-bit phase counting mode.
> >
> > b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
> > 0: 16-bit access is enabled.
> > 1: 32-bit access is enabled.
> >
> > > Does MTCLKA and MTCLKB serve as the counting signals in this case,
> >
> > For 16-bit and 32-bit counting signals same. We can set
> >
> > 1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB
> >
> > Or
> >
> > 2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC and
> > MTCLKD
> 
> I'm having trouble understanding this case. If 32-bit access is
> enabled by setting the LWA bit, and the MTU1 signals are configured as
> MTCLKA and MTCLKB while at the same time the MTU2 signals are
> configured as MTCLKC and MTCLKD, how is the 32-bit count value
> determined -- wouldn't
> MTU1 and MTU2 be counting independently if they each had separate
> input clocks fed to them?

It is taken care by the HW. We just configure the register as mentioned below
and hardware provide counter values once feeding the signals to 
either
{MTCLKA and MTCLKB} for both MTU1 and  MTU2 

or 

MTU1{MTCLKA and MTCLKB} and MTU2{MTCLKC and MTCLKD}

The signal feeding is same for 16-bit and 32-bit phase modes.

Note:- I haven't tested 32-bit mode yet. 

Cheers,
Biju

> 
> >
> >
> > b1 PHCKSEL 1 R/W External Input Phase Clock Select Selects the
> > external clock pin for phase counting mode.
> > 0: MTCLKA and MTCLKB are selected for the external phase clock.
> > 1: MTCLKC and MTCLKD are selected for the external phase clock
> >
> > > with overflows on the MTU1 register incrementing the MTU2
> register?
> >
> > No. that won't happen as we need to use different register for Long
> > word access
> >
> > These are the regiters used
> > 16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
> > 32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW
> >
> > Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
> > Counter in MTU2   MTU2.TCNT Word
> >
> > General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW Longword
> > General register A in MTU2 MTU2.TGRA Word
> >
> > General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW Longword
> > General register B in MTU2 MTU2.TGRB Word
> >
> > Cheers,
> > Biju

Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[William Breathitt Gray]

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

On Sat, Oct 01, 2022 at 06:51:48PM +0000, Biju Das wrote:
> > On Sat, Oct 01, 2022 at 06:03:37PM +0000, Biju Das wrote:
> > > > What is the configuration when 32-bit phase counting mode is
> > selected?
> > >
> > > LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to set
> > for 32-bit phase counting mode.
> > >
> > > b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
> > > 0: 16-bit access is enabled.
> > > 1: 32-bit access is enabled.
> > >
> > > > Does MTCLKA and MTCLKB serve as the counting signals in this case,
> > >
> > > For 16-bit and 32-bit counting signals same. We can set
> > >
> > > 1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB
> > >
> > > Or
> > >
> > > 2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC and
> > > MTCLKD
> > 
> > I'm having trouble understanding this case. If 32-bit access is
> > enabled by setting the LWA bit, and the MTU1 signals are configured as
> > MTCLKA and MTCLKB while at the same time the MTU2 signals are
> > configured as MTCLKC and MTCLKD, how is the 32-bit count value
> > determined -- wouldn't
> > MTU1 and MTU2 be counting independently if they each had separate
> > input clocks fed to them?
> 
> It is taken care by the HW. We just configure the register as mentioned below
> and hardware provide counter values once feeding the signals to 
> either
> {MTCLKA and MTCLKB} for both MTU1 and  MTU2 
> 
> or 
> 
> MTU1{MTCLKA and MTCLKB} and MTU2{MTCLKC and MTCLKD}
> 
> The signal feeding is same for 16-bit and 32-bit phase modes.
> 
> Note:- I haven't tested 32-bit mode yet. 
> 
> Cheers,
> Biju

I'm not quite grokking it yet, but I'll trust that you're right for now.
I suspect it'll make more sense to me once your next revision is
submitted and I've had time to evaluate the code more closely.

Thanks,

William Breathitt Gray

> 
> > 
> > >
> > >
> > > b1 PHCKSEL 1 R/W External Input Phase Clock Select Selects the
> > > external clock pin for phase counting mode.
> > > 0: MTCLKA and MTCLKB are selected for the external phase clock.
> > > 1: MTCLKC and MTCLKD are selected for the external phase clock
> > >
> > > > with overflows on the MTU1 register incrementing the MTU2
> > register?
> > >
> > > No. that won't happen as we need to use different register for Long
> > > word access
> > >
> > > These are the regiters used
> > > 16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
> > > 32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW
> > >
> > > Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
> > > Counter in MTU2   MTU2.TCNT Word
> > >
> > > General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW Longword
> > > General register A in MTU2 MTU2.TGRA Word
> > >
> > > General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW Longword
> > > General register B in MTU2 MTU2.TGRB Word
> > >
> > > Cheers,
> > > Biju

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

RE: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver

[Biju Das]

> Subject: Re: [PATCH RFC 0/8] Add RZ/G2L MTU3a MFD and Counter driver
> 
> On Sat, Oct 01, 2022 at 06:51:48PM +0000, Biju Das wrote:
> > > On Sat, Oct 01, 2022 at 06:03:37PM +0000, Biju Das wrote:
> > > > > What is the configuration when 32-bit phase counting mode is
> > > selected?
> > > >
> > > > LWA Bit (MTU1/MTU2 Combination Longword Access Control) needs to
> > > > set
> > > for 32-bit phase counting mode.
> > > >
> > > > b0 LWA 0 R/W MTU1/MTU2 Combination Longword Access Control
> > > > 0: 16-bit access is enabled.
> > > > 1: 32-bit access is enabled.
> > > >
> > > > > Does MTCLKA and MTCLKB serve as the counting signals in this
> > > > > case,
> > > >
> > > > For 16-bit and 32-bit counting signals same. We can set
> > > >
> > > > 1) MTU 1 and MTU2 signals as MTCLKA and MTCLKB
> > > >
> > > > Or
> > > >
> > > > 2) MTU 1 signal as MTCLKA and MTCLKB and MTU2 signals as MTCLKC
> > > > and MTCLKD
> > >
> > > I'm having trouble understanding this case. If 32-bit access is
> > > enabled by setting the LWA bit, and the MTU1 signals are
> configured
> > > as MTCLKA and MTCLKB while at the same time the MTU2 signals are
> > > configured as MTCLKC and MTCLKD, how is the 32-bit count value
> > > determined -- wouldn't
> > > MTU1 and MTU2 be counting independently if they each had separate
> > > input clocks fed to them?
> >
> > It is taken care by the HW. We just configure the register as
> > mentioned below and hardware provide counter values once feeding the
> > signals to either {MTCLKA and MTCLKB} for both MTU1 and  MTU2
> >
> > or
> >
> > MTU1{MTCLKA and MTCLKB} and MTU2{MTCLKC and MTCLKD}
> >
> > The signal feeding is same for 16-bit and 32-bit phase modes.
> >
> > Note:- I haven't tested 32-bit mode yet.
> >
> > Cheers,
> > Biju
> 
> I'm not quite grokking it yet, but I'll trust that you're right for
> now.
> I suspect it'll make more sense to me once your next revision is
> submitted and I've had time to evaluate the code more closely.

OK, softwarewise we don't need to anything for 16-bit and 32-bit 
as HW provide separate registers for accessing 32-bit counter values
eventhoug internally, it is formed by combining 2 16-bit counters on
MTU1 and MTU2.

OK, we will have more idea when I submit patches for 32-bit mode.

Cheers,
Biju


> 
> Thanks,
> 
> William Breathitt Gray
> 
> >
> > >
> > > >
> > > >
> > > > b1 PHCKSEL 1 R/W External Input Phase Clock Select Selects the
> > > > external clock pin for phase counting mode.
> > > > 0: MTCLKA and MTCLKB are selected for the external phase clock.
> > > > 1: MTCLKC and MTCLKD are selected for the external phase clock
> > > >
> > > > > with overflows on the MTU1 register incrementing the MTU2
> > > register?
> > > >
> > > > No. that won't happen as we need to use different register for
> > > > Long word access
> > > >
> > > > These are the regiters used
> > > > 16-bit:- TCNT{MTU1,MTU2}, TGRA{MTU1,MTU2},  and TGRB{MTU1,MTU2},
> > > > 32-bit:- MTU1.TCNT_1_LW, MTU1.TGRA_1_LW and MTU1.TGRB_1_LW
> > > >
> > > > Counter in MTU1   MTU1.TCNT Word MTU1.TCNT_1_LW Longword
> > > > Counter in MTU2   MTU2.TCNT Word
> > > >
> > > > General register A in MTU1 MTU1.TGRA Word MTU1.TGRA_1_LW
> Longword
> > > > General register A in MTU2 MTU2.TGRA Word
> > > >
> > > > General register B in MTU1 MTU1.TGRB Word MTU1.TGRB_1_LW
> Longword
> > > > General register B in MTU2 MTU2.TGRB Word
> > > >
> > > > Cheers,
> > > > Biju

Re: [PATCH RFC 2/8] dt-bindings: mfd: Document RZ/G2L MTU3a bindings

[Krzysztof Kozlowski]

On 26/09/2022 15:21, Biju Das wrote:
> The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
> the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
> channels and one 32-bit timer channel. It supports the following
> functions
>  - Counter
>  - Timer
>  - PWM
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../bindings/mfd/renesas,rzg2l-mtu3.yaml      | 275 ++++++++++++++++++
>  1 file changed, 275 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> new file mode 100644
> index 000000000000..c1fae8e8d9f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> @@ -0,0 +1,275 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/renesas,rzg2l-mtu3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 (MTU3a) bindings

Drop "bindings"

Since this is RFC, not a ready patch, the review is fast and not
thorough. Please send final patch for review (when ready).

> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +
> +description: |
> +  This hardware block pconsisting of eight 16-bit timer channels and one
> +  32- bit timer channel. It supports the following specifications:
> +    - Pulse input/output: 28 lines max.
> +    - Pulse input 3 lines
> +    - Count clock 11 clocks for each channel (14 clocks for MTU0, 12 clocks
> +      for MTU2, and 10 clocks for MTU5, four clocks for MTU1-MTU2 combination
> +      (when LWA = 1))
> +    - Operating frequency Up to 100 MHz
> +    - Available operations [MTU0 to MTU4, MTU6, MTU7, and MTU8]
> +        - Waveform output on compare match
> +        - Input capture function (noise filter setting available)
> +        - Counter-clearing operation
> +        - Simultaneous writing to multiple timer counters (TCNT)
> +          (excluding MTU8).
> +        - Simultaneous clearing on compare match or input capture
> +          (excluding MTU8).
> +        - Simultaneous input and output to registers in synchronization with
> +          counter operations           (excluding MTU8).
> +        - Up to 12-phase PWM output in combination with synchronous operation
> +          (excluding MTU8)
> +    - [MTU0 MTU3, MTU4, MTU6, MTU7, and MTU8]
> +        - Buffer operation specifiable
> +    - [MTU1, MTU2]
> +        - Phase counting mode can be specified independently
> +        - 32-bit phase counting mode can be specified for interlocked operation
> +          of MTU1 and MTU2 (when TMDR3.LWA = 1)
> +        - Cascade connection operation available
> +    - [MTU3, MTU4, MTU6, and MTU7]
> +        - Through interlocked operation of MTU3/4 and MTU6/7, the positive and
> +          negative signals in six phases (12 phases in total) can be output in
> +          complementary PWM and reset-synchronized PWM operation.
> +        - In complementary PWM mode, values can be transferred from buffer
> +          registers to temporary registers at crests and troughs of the timer-
> +          counter values or when the buffer registers (TGRD registers in MTU4
> +          and MTU7) are written to.
> +        - Double-buffering selectable in complementary PWM mode.
> +    - [MTU3 and MTU4]
> +        - Through interlocking with MTU0, a mode for driving AC synchronous
> +          motors (brushless DC motors) by using complementary PWM output and
> +          reset-synchronized PWM output is settable and allows the selection
> +          of two types of waveform output (chopping or level).
> +    - [MTU5]
> +        - Capable of operation as a dead-time compensation counter.
> +    - [MTU0/MTU5, MTU1, MTU2, and MTU8]
> +        - 32-bit phase counting mode specifiable by combining MTU1 and MTU2 and
> +          through interlocked operation with MTU0/MTU5 and MTU8.
> +    - Interrupt-skipping function
> +        - In complementary PWM mode, interrupts on crests and troughs of counter
> +          values and triggers to start conversion by the A/D converter can be
> +          skipped.
> +    - Interrupt sources: 43 sources.
> +    - Buffer operation:
> +        - Automatic transfer of register data (transfer from the buffer
> +          register to the timer register).
> +    - Trigger generation
> +        - A/D converter start triggers can be generated
> +        - A/D converter start request delaying function enables A/D converter
> +          to be started with any desired timing and to be synchronized with
> +          PWM output.
> +    - Low power consumption function
> +        - The MTU3a can be placed in the module-stop state.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - renesas,r9a07g044-mtu3  # RZ/G2{L,LC}
> +          - renesas,r9a07g054-mtu3  # RZ/V2L
> +      - const: renesas,rzg2l-mtu3
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: MTU0.TGRA input capture/compare match
> +      - description: MTU0.TGRB input capture/compare match
> +      - description: MTU0.TGRC input capture/compare match
> +      - description: MTU0.TGRD input capture/compare match
> +      - description: MTU0.TCNT overflow
> +      - description: MTU0.TGRE compare match
> +      - description: MTU0.TGRF compare match
> +      - description: MTU1.TGRA input capture/compare match
> +      - description: MTU1.TGRB input capture/compare match
> +      - description: MTU1.TCNT overflow
> +      - description: MTU1.TCNT underflow
> +      - description: MTU2.TGRA input capture/compare match
> +      - description: MTU2.TGRB input capture/compare match
> +      - description: MTU2.TCNT overflow
> +      - description: MTU2.TCNT underflow
> +      - description: MTU3.TGRA input capture/compare match
> +      - description: MTU3.TGRB input capture/compare match
> +      - description: MTU3.TGRC input capture/compare match
> +      - description: MTU3.TGRD input capture/compare match
> +      - description: MTU3.TCNT overflow
> +      - description: MTU4.TGRA input capture/compare match
> +      - description: MTU4.TGRB input capture/compare match
> +      - description: MTU4.TGRC input capture/compare match
> +      - description: MTU4.TGRD input capture/compare match
> +      - description: MTU4.TCNT overflow/underflow
> +      - description: MTU5.TGRU input capture/compare match
> +      - description: MTU5.TGRV input capture/compare match
> +      - description: MTU5.TGRW input capture/compare match
> +      - description: MTU6.TGRA input capture/compare match
> +      - description: MTU6.TGRB input capture/compare match
> +      - description: MTU6.TGRC input capture/compare match
> +      - description: MTU6.TGRD input capture/compare match
> +      - description: MTU6.TCNT overflow
> +      - description: MTU7.TGRA input capture/compare match
> +      - description: MTU7.TGRB input capture/compare match
> +      - description: MTU7.TGRC input capture/compare match
> +      - description: MTU7.TGRD input capture/compare match
> +      - description: MTU7.TCNT overflow/underflow
> +      - description: MTU8.TGRA input capture/compare match
> +      - description: MTU8.TGRB input capture/compare match
> +      - description: MTU8.TGRC input capture/compare match
> +      - description: MTU8.TGRD input capture/compare match
> +      - description: MTU8.TCNT overflow
> +      - description: MTU8.TCNT underflow
> +
> +  interrupt-names:
> +    items:
> +      - const: tgia0
> +      - const: tgib0
> +      - const: tgic0
> +      - const: tgid0
> +      - const: tgiv0
> +      - const: tgie0
> +      - const: tgif0
> +      - const: tgia1
> +      - const: tgib1
> +      - const: tgiv1
> +      - const: tgiu1
> +      - const: tgia2
> +      - const: tgib2
> +      - const: tgiv2
> +      - const: tgiu2
> +      - const: tgia3
> +      - const: tgib3
> +      - const: tgic3
> +      - const: tgid3
> +      - const: tgiv3
> +      - const: tgia4
> +      - const: tgib4
> +      - const: tgic4
> +      - const: tgid4
> +      - const: tgiv4
> +      - const: tgiu5
> +      - const: tgiv5
> +      - const: tgiw5
> +      - const: tgia6
> +      - const: tgib6
> +      - const: tgic6
> +      - const: tgid6
> +      - const: tgiv6
> +      - const: tgia7
> +      - const: tgib7
> +      - const: tgic7
> +      - const: tgid7
> +      - const: tgiv7
> +      - const: tgia8
> +      - const: tgib8
> +      - const: tgic8
> +      - const: tgid8
> +      - const: tgiv8
> +      - const: tgiu8
> +
> +  clocks:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0

Why do you need these two? There are no children here.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - clocks
> +  - power-domains
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    mtu3: timer@10001200 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      compatible = "renesas,r9a07g044-mtu3", "renesas,rzg2l-mtu3";
> +      reg = <0x10001200 0xb00>;

Order of properties: compatible, reg, then the rest.

> +      interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>,

Best regards,
Krzysztof

Re: [PATCH RFC 4/8] dt-bindings: mfd: rzg2l-mtu3: Document RZ/G2UL MTU3 counter

[Krzysztof Kozlowski]

On 26/09/2022 15:21, Biju Das wrote:
> Document 16-bit and 32-bit phase counting mode support on
> RZ/G2L MTU3 IP.
> 

Squash with previous. New devices are added complete, not artificially
split into multiple non-working components.

> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../bindings/mfd/renesas,rzg2l-mtu3.yaml      | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> index c1fae8e8d9f9..c4bcf28623d6 100644
> --- a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> @@ -192,6 +192,37 @@ properties:
>    "#size-cells":
>      const: 0
>  
> +patternProperties:
> +  "^counter@[1-2]+$":
> +    type: object

    additionalProperties: false

> +
> +    properties:
> +      compatible:
> +        const: renesas,rzg2l-mtu3-counter
> +
> +      reg:
> +        description: Identify counter channels.
> +        items:
> +          enum: [ 1, 2 ]
> +
> +      renesas,32bit-phase-counting:
> +        type: boolean
> +        description: Enable 32-bit phase counting mode.
> +
> +      renesas,ext-input-phase-clock-select:

I propose to drop "input". I understand you just select pins with clock?
If it is external clock, then why not using generic clock bindings?

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [ 0, 1 ]
> +        default: 1
> +        description: |
> +          Selects the external clock pin for phase counting mode.
> +            <0> : MTCLKA and MTCLKB are selected for the external phase clock.
> +            <1> : MTCLKC and MTCLKD are selected for the external phase clock
> +                  (default)
> +
> +    required:
> +      - compatible
> +      - reg
> +
>  required:
>    - compatible
>    - reg
> @@ -270,6 +301,10 @@ examples:
>        clocks = <&cpg CPG_MOD R9A07G044_MTU_X_MCK_MTU3>;
>        power-domains = <&cpg>;
>        resets = <&cpg R9A07G044_MTU_X_PRESET_MTU3>;

Blank line

> +      counter@1 {
> +        compatible = "renesas,rzg2l-mtu3-counter";
> +        reg = <1>;
> +      };
>      };
>  
>  ...

Best regards,
Krzysztof

RE: [PATCH RFC 2/8] dt-bindings: mfd: Document RZ/G2L MTU3a bindings

[Biju Das]

Hi Krzysztof Kozlowski,

> Subject: Re: [PATCH RFC 2/8] dt-bindings: mfd: Document RZ/G2L MTU3a
> bindings
> 
> On 26/09/2022 15:21, Biju Das wrote:
> > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in
> > the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer
> > channels and one 32-bit timer channel. It supports the following
> > functions
> >  - Counter
> >  - Timer
> >  - PWM
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  .../bindings/mfd/renesas,rzg2l-mtu3.yaml      | 275
> ++++++++++++++++++
> >  1 file changed, 275 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> > b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> > new file mode 100644
> > index 000000000000..c1fae8e8d9f9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> > @@ -0,0 +1,275 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> >
> 
> Drop "bindings"
> 
> Since this is RFC, not a ready patch, the review is fast and not
> thorough. Please send final patch for review (when ready).
> 
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +description: |
> > +  This hardware block pconsisting of eight 16-bit timer channels
> and
> > +one
> > +  32- bit timer channel. It supports the following specifications:
> > +    - Pulse input/output: 28 lines max.
> > +    - Pulse input 3 lines
> > +    - Count clock 11 clocks for each channel (14 clocks for MTU0,
> 12 clocks
> > +      for MTU2, and 10 clocks for MTU5, four clocks for MTU1-MTU2
> combination
> > +      (when LWA = 1))
> > +    - Operating frequency Up to 100 MHz
> > +    - Available operations [MTU0 to MTU4, MTU6, MTU7, and MTU8]
> > +        - Waveform output on compare match
> > +        - Input capture function (noise filter setting available)
> > +        - Counter-clearing operation
> > +        - Simultaneous writing to multiple timer counters (TCNT)
> > +          (excluding MTU8).
> > +        - Simultaneous clearing on compare match or input capture
> > +          (excluding MTU8).
> > +        - Simultaneous input and output to registers in
> synchronization with
> > +          counter operations           (excluding MTU8).
> > +        - Up to 12-phase PWM output in combination with synchronous
> operation
> > +          (excluding MTU8)
> > +    - [MTU0 MTU3, MTU4, MTU6, MTU7, and MTU8]
> > +        - Buffer operation specifiable
> > +    - [MTU1, MTU2]
> > +        - Phase counting mode can be specified independently
> > +        - 32-bit phase counting mode can be specified for
> interlocked operation
> > +          of MTU1 and MTU2 (when TMDR3.LWA = 1)
> > +        - Cascade connection operation available
> > +    - [MTU3, MTU4, MTU6, and MTU7]
> > +        - Through interlocked operation of MTU3/4 and MTU6/7, the
> positive and
> > +          negative signals in six phases (12 phases in total) can
> be output in
> > +          complementary PWM and reset-synchronized PWM operation.
> > +        - In complementary PWM mode, values can be transferred from
> buffer
> > +          registers to temporary registers at crests and troughs of
> the timer-
> > +          counter values or when the buffer registers (TGRD
> registers in MTU4
> > +          and MTU7) are written to.
> > +        - Double-buffering selectable in complementary PWM mode.
> > +    - [MTU3 and MTU4]
> > +        - Through interlocking with MTU0, a mode for driving AC
> synchronous
> > +          motors (brushless DC motors) by using complementary PWM
> output and
> > +          reset-synchronized PWM output is settable and allows the
> selection
> > +          of two types of waveform output (chopping or level).
> > +    - [MTU5]
> > +        - Capable of operation as a dead-time compensation counter.
> > +    - [MTU0/MTU5, MTU1, MTU2, and MTU8]
> > +        - 32-bit phase counting mode specifiable by combining MTU1
> and MTU2 and
> > +          through interlocked operation with MTU0/MTU5 and MTU8.
> > +    - Interrupt-skipping function
> > +        - In complementary PWM mode, interrupts on crests and
> troughs of counter
> > +          values and triggers to start conversion by the A/D
> converter can be
> > +          skipped.
> > +    - Interrupt sources: 43 sources.
> > +    - Buffer operation:
> > +        - Automatic transfer of register data (transfer from the
> buffer
> > +          register to the timer register).
> > +    - Trigger generation
> > +        - A/D converter start triggers can be generated
> > +        - A/D converter start request delaying function enables A/D
> converter
> > +          to be started with any desired timing and to be
> synchronized with
> > +          PWM output.
> > +    - Low power consumption function
> > +        - The MTU3a can be placed in the module-stop state.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - renesas,r9a07g044-mtu3  # RZ/G2{L,LC}
> > +          - renesas,r9a07g054-mtu3  # RZ/V2L
> > +      - const: renesas,rzg2l-mtu3
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    items:
> > +      - description: MTU0.TGRA input capture/compare match
> > +      - description: MTU0.TGRB input capture/compare match
> > +      - description: MTU0.TGRC input capture/compare match
> > +      - description: MTU0.TGRD input capture/compare match
> > +      - description: MTU0.TCNT overflow
> > +      - description: MTU0.TGRE compare match
> > +      - description: MTU0.TGRF compare match
> > +      - description: MTU1.TGRA input capture/compare match
> > +      - description: MTU1.TGRB input capture/compare match
> > +      - description: MTU1.TCNT overflow
> > +      - description: MTU1.TCNT underflow
> > +      - description: MTU2.TGRA input capture/compare match
> > +      - description: MTU2.TGRB input capture/compare match
> > +      - description: MTU2.TCNT overflow
> > +      - description: MTU2.TCNT underflow
> > +      - description: MTU3.TGRA input capture/compare match
> > +      - description: MTU3.TGRB input capture/compare match
> > +      - description: MTU3.TGRC input capture/compare match
> > +      - description: MTU3.TGRD input capture/compare match
> > +      - description: MTU3.TCNT overflow
> > +      - description: MTU4.TGRA input capture/compare match
> > +      - description: MTU4.TGRB input capture/compare match
> > +      - description: MTU4.TGRC input capture/compare match
> > +      - description: MTU4.TGRD input capture/compare match
> > +      - description: MTU4.TCNT overflow/underflow
> > +      - description: MTU5.TGRU input capture/compare match
> > +      - description: MTU5.TGRV input capture/compare match
> > +      - description: MTU5.TGRW input capture/compare match
> > +      - description: MTU6.TGRA input capture/compare match
> > +      - description: MTU6.TGRB input capture/compare match
> > +      - description: MTU6.TGRC input capture/compare match
> > +      - description: MTU6.TGRD input capture/compare match
> > +      - description: MTU6.TCNT overflow
> > +      - description: MTU7.TGRA input capture/compare match
> > +      - description: MTU7.TGRB input capture/compare match
> > +      - description: MTU7.TGRC input capture/compare match
> > +      - description: MTU7.TGRD input capture/compare match
> > +      - description: MTU7.TCNT overflow/underflow
> > +      - description: MTU8.TGRA input capture/compare match
> > +      - description: MTU8.TGRB input capture/compare match
> > +      - description: MTU8.TGRC input capture/compare match
> > +      - description: MTU8.TGRD input capture/compare match
> > +      - description: MTU8.TCNT overflow
> > +      - description: MTU8.TCNT underflow
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: tgia0
> > +      - const: tgib0
> > +      - const: tgic0
> > +      - const: tgid0
> > +      - const: tgiv0
> > +      - const: tgie0
> > +      - const: tgif0
> > +      - const: tgia1
> > +      - const: tgib1
> > +      - const: tgiv1
> > +      - const: tgiu1
> > +      - const: tgia2
> > +      - const: tgib2
> > +      - const: tgiv2
> > +      - const: tgiu2
> > +      - const: tgia3
> > +      - const: tgib3
> > +      - const: tgic3
> > +      - const: tgid3
> > +      - const: tgiv3
> > +      - const: tgia4
> > +      - const: tgib4
> > +      - const: tgic4
> > +      - const: tgid4
> > +      - const: tgiv4
> > +      - const: tgiu5
> > +      - const: tgiv5
> > +      - const: tgiw5
> > +      - const: tgia6
> > +      - const: tgib6
> > +      - const: tgic6
> > +      - const: tgid6
> > +      - const: tgiv6
> > +      - const: tgia7
> > +      - const: tgib7
> > +      - const: tgic7
> > +      - const: tgid7
> > +      - const: tgiv7
> > +      - const: tgia8
> > +      - const: tgib8
> > +      - const: tgic8
> > +      - const: tgid8
> > +      - const: tgiv8
> > +      - const: tgiu8
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> 
> Why do you need these two? There are no children here.

On the next 2 patches children added. Good point will move this to next patch.


> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-names
> > +  - clocks
> > +  - power-domains
> > +  - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r9a07g044-cpg.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    mtu3: timer@10001200 {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +      compatible = "renesas,r9a07g044-mtu3", "renesas,rzg2l-mtu3";
> > +      reg = <0x10001200 0xb00>;
> 
> Order of properties: compatible, reg, then the rest.

OK, will fix this in next version.

Cheers,
Biju

RE: [PATCH RFC 4/8] dt-bindings: mfd: rzg2l-mtu3: Document RZ/G2UL MTU3 counter

[Biju Das]

Hi Krzysztof Kozlowski,

> Subject: Re: [PATCH RFC 4/8] dt-bindings: mfd: rzg2l-mtu3: Document
> RZ/G2UL MTU3 counter
> 
> On 26/09/2022 15:21, Biju Das wrote:
> > Document 16-bit and 32-bit phase counting mode support on RZ/G2L
> MTU3
> > IP.
> >
> 
> Squash with previous. New devices are added complete, not artificially
> split into multiple non-working components.

Ok will squash into previous patch.

> 
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  .../bindings/mfd/renesas,rzg2l-mtu3.yaml      | 35
> +++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> > b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> > index c1fae8e8d9f9..c4bcf28623d6 100644
> > --- a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml
> > @@ -192,6 +192,37 @@ properties:
> >    "#size-cells":
> >      const: 0
> >
> > +patternProperties:
> > +  "^counter@[1-2]+$":
> > +    type: object
> 
>     additionalProperties: false

OK.

> 
> > +
> > +    properties:
> > +      compatible:
> > +        const: renesas,rzg2l-mtu3-counter
> > +
> > +      reg:
> > +        description: Identify counter channels.
> > +        items:
> > +          enum: [ 1, 2 ]
> > +
> > +      renesas,32bit-phase-counting:
> > +        type: boolean
> > +        description: Enable 32-bit phase counting mode.
> > +
> > +      renesas,ext-input-phase-clock-select:
> 
> I propose to drop "input". I understand you just select pins with
> clock?
> If it is external clock, then why not using generic clock bindings?


It is basically clock pins as discussed in [1],

There will be a new sysfs in counter subsytem for handling this.

Hardware supports 4 pins for phase counting mode,

MTCLKA Input External clock A input pin (MTU1/MTU2 phase counting mode A phase input)
MTCLKB Input External clock B input pin (MTU1/MTU2 phase counting mode B phase input)
MTCLKC Input External clock C input pin (MTU2 phase counting mode A phase input)
MTCLKD Input External clock D input pin (MTU2 phase counting mode B phase input)

For MTU1, it is fixed MTCLKA and MTCLKB.
But for MTU2, it can be either 0-{ MTCLKA, MTCLKB} or 1 - { MTCLKC , MTCLKD}
On reset it is set to { MTCLKC , MTCLKD}.

[1] https://lore.kernel.org/linux-renesas-soc/OS0PR01MB59223F69EA3215528519F49086599@OS0PR01MB5922.jpnprd01.prod.outlook.com/T/#mb7db950c9eb61ac52f405cdc654a409b8cb198f9


As per [1],
it is going to be modelled as , if everyone ok with it.

  counter:
    type: object

    properties:
      compatible:
        const: renesas,rzg2l-mtu3-counter

> 
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        enum: [ 0, 1 ]
> > +        default: 1
> > +        description: |
> > +          Selects the external clock pin for phase counting mode.
> > +            <0> : MTCLKA and MTCLKB are selected for the external
> phase clock.
> > +            <1> : MTCLKC and MTCLKD are selected for the external
> phase clock
> > +                  (default)
> > +
> > +    required:
> > +      - compatible
> > +      - reg
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -270,6 +301,10 @@ examples:
> >        clocks = <&cpg CPG_MOD R9A07G044_MTU_X_MCK_MTU3>;
> >        power-domains = <&cpg>;
> >        resets = <&cpg R9A07G044_MTU_X_PRESET_MTU3>;
> 
> Blank line

OK will fix it.

Cheers,
Biju

> 
> > +      counter@1 {
> > +        compatible = "renesas,rzg2l-mtu3-counter";
> > +        reg = <1>;
> > +      };
> >      };
> >
> >  ...
> 
> Best regards,
> Krzysztof

RE: [PATCH RFC 5/8] counter: Add RZ/G2L MTU3 counter driver

[Biju Das]

Hi William Breathitt Gray,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 5/8] counter: Add RZ/G2L MTU3 counter driver
> 
> On Mon, Sep 26, 2022 at 02:21:11PM +0100, Biju Das wrote:
> > Add RZ/G2L MTU3 counter driver. Currently it supports 16-bit phase
> > counting mode on MTU{1,2} channels.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Hi Biju,
> 
> This driver will likely change in your next revision, but I want to
> give some feedback anyway on a few things I noticed. See the comments
> below.
> 
> > +struct rzg2l_mtu3_cnt {
> > +	struct clk *clk;
> > +	void __iomem *mmio;
> > +	struct rzg2l_mtu3_channel *ch;
> > +};
> 
> Add kernel-doc comments to document this structure. It seems that
> neither clk nor mmio is access in the code from this structure; what's
> the purpose of having them here?

OK, will do. mmio is not required. But clk is needed.

> 
> > +static int rzg2l_mtu3_count_read(struct counter_device *counter,
> > +				 struct counter_count *count, u64 *val) {
> > +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> > +	u32 cnt;
> > +
> > +	cnt = rzg2l_mtu3_16bit_ch_read(priv->ch, RZG2L_MTU3_TCNT);
> > +	*val = cnt;
> 
> The rzg2l_mtu3_16bit_ch_read() function returns a u16, so there's no
> need for the cnt variable; just return the count via val directly.

OK. Agreed.

> 
> > +static int rzg2l_mtu3_count_write(struct counter_device *counter,
> > +				  struct counter_count *count, const u64 val) {
> > +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> > +	u16 ceiling;
> > +
> > +	ceiling = rzg2l_mtu3_16bit_ch_read(priv->ch, RZG2L_MTU3_TGRA);
> > +
> > +	if (val > ceiling)
> > +		return -EINVAL;
> 
> Return -ERANGE instead to indicate the request is outside the
> boundary.

Ok. Agreed.

> 
> > +
> > +	rzg2l_mtu3_16bit_ch_write(priv->ch, RZG2L_MTU3_TCNT, (u16)val);
> 
> Remove the explicit cast to u16, it's already implicit in the call.
> You probably also need some sort of lock in this function to ensure
> that your ceiling value does not change before you write to the
> register.

OK agreed.

> 
> > +static int rzg2l_mtu3_count_ceiling_read(struct counter_device
> *counter,
> > +					 struct counter_count *count,
> > +					 u64 *ceiling)
> > +{
> > +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> > +	u32 val;
> > +
> > +	val = rzg2l_mtu3_16bit_ch_read(priv->ch, RZG2L_MTU3_TGRA);
> > +	*ceiling = val;
> 
> Same comment as in rzg2l_mtu3_count_read().

OK agreed.
> 
> > +static int rzg2l_mtu3_count_ceiling_write(struct counter_device
> *counter,
> > +					  struct counter_count *count,
> > +					  u64 ceiling)
> > +{
> > +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> > +
> > +	if (ceiling > U16_MAX)
> > +		return -ERANGE;
> > +
> > +	rzg2l_mtu3_16bit_ch_write(priv->ch, RZG2L_MTU3_TGRA,
> (u16)ceiling);
> > +	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TCR,
> > +				 RZG2L_MTU3_TCR_CCLR_TGRA);
> 
> Same comments about cast and lock as in rzg2l_mtu3_count_write().

OK agreed.

> 
> > +static int rzg2l_mtu3_count_enable_read(struct counter_device
> *counter,
> > +					struct counter_count *count, u8 *enable)
> {
> > +	struct rzg2l_mtu3_cnt *const priv = counter_priv(counter);
> > +	int ch = priv->ch->index;
> > +
> > +	*enable = (rzg2l_mtu3_shared_reg_read(priv->ch, RZG2L_MTU3_TSTRA)
> &
> > +		(0x1 << ch)) >> ch;
> 
> A lot of operations happening in a single line; can this be broken
> down to clearer distinct steps?

OK agreed.

> 
> > +static int rzg2l_mtu3_action_read(struct counter_device *counter,
> > +				  struct counter_count *count,
> > +				  struct counter_synapse *synapse,
> > +				  enum counter_synapse_action *action) {
> > +	enum counter_function function;
> > +	int err;
> > +
> > +	err = rzg2l_mtu3_count_function_read(counter, count, &function);
> > +	if (err)
> > +		return err;
> > +
> > +	switch (function) {
> > +	case COUNTER_FUNCTION_PULSE_DIRECTION:
> > +		/*
> > +		 * Rising edges on signal A updates the respective count.
> > +		 * The input level of signal B determines direction.
> > +		 */
> > +		*action = COUNTER_SYNAPSE_ACTION_RISING_EDGE;
> 
> You need to differentiate between signal A and B here: the Synapse for
> signal A will have an action mode of COUNTER_SYNAPSE_ACTION_RING_EDGE,
> but the Synapse for Signal B will have an action mode of
> COUNTER_SYNAPSE_ACTION_NONE because its not the trigger point for the
> respective Count value update.

OK, Will do.

> 
> > +		break;
> > +	case COUNTER_FUNCTION_QUADRATURE_X2_B:
> > +		/*
> > +		 * Any state transition on quadrature pair signal B updates
> > +		 * the respective count.
> > +		 */
> > +		*action = COUNTER_SYNAPSE_ACTION_BOTH_EDGES;
> 
> Similar to above, you need to differentiate between signal A and B
> here as well.

OK, will do.

> 
> > +static struct counter_count rzg2l_mtu3_counts = {
> > +	.id = 0,
> 
> The id member is an optional way for driver authors to identify their
> own Counts; it can be set to anything your like, and if you don't use
> it in your code then you don't need to set it at all.

It is being used in the next version.

> 
> > +static int rzg2l_mtu3_cnt_probe(struct platform_device *pdev) {
> > +	struct rzg2l_mtu3 *ddata = dev_get_drvdata(pdev->dev.parent);
> > +	struct device *dev = &pdev->dev;
> > +	struct counter_device *counter;
> > +	struct rzg2l_mtu3_cnt *priv;
> > +	int ret;
> > +	u32 ch;
> > +
> > +	if (IS_ERR_OR_NULL(ddata))
> > +		return -EINVAL;
> 
> Is this actually possible? What situation would cause this, and why is
> it not handled before we reach probe()?

Theoretically not required as parent device populates child devices.
I will remove it from here.

> 
> > +
> > +	counter = devm_counter_alloc(dev, sizeof(*priv));
> > +	if (!counter)
> > +		return -ENOMEM;
> > +
> > +	priv = counter_priv(counter);
> > +
> > +	ret = of_property_read_u32(dev->of_node, "reg", &ch);
> > +	if (ret) {
> > +		dev_err(dev, "%pOF: No reg property found\n", dev-
> >of_node);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (ch != RZG2L_MTU1 && ch != RZG2L_MTU2) {
> > +		dev_err(dev, "%pOF: Invalid channel '%u'\n", dev->of_node,
> ch);
> > +		return -EINVAL;
> > +	}
> > +
> > +	priv->clk = ddata->clk;
> > +	priv->ch = &ddata->channels[ch];
> > +	priv->ch->dev = dev;
> > +
> > +	counter->name = dev_name(dev);
> > +	counter->parent = dev;
> > +	counter->ops = &rzg2l_mtu3_cnt_ops;
> > +	counter->counts = &rzg2l_mtu3_counts;
> > +	counter->num_counts = 1;
> 
> Even though you only have one Count defined, use ARRAY_SIZE here for
> consistency with the other Counter drivers as well as making the
> intention of the code clear.

OK will use array.

> 
> > +	counter->signals = rzg2l_mtu3_signals;
> > +	counter->num_signals = ARRAY_SIZE(rzg2l_mtu3_signals);
> > +	platform_set_drvdata(pdev, priv);
> > +
> > +	/* Register Counter device */
> > +	ret = devm_counter_add(dev, counter);
> > +	if (ret < 0)
> > +		return dev_err_probe(dev, ret, "Failed to add counter\n");
> 
> The Counter driver goes live with the call to devm_counter_add() so
> move it to the end after your device initialization code below.

OK. Agreed.

> 
> > +
> > +	priv->ch->function = RZG2L_MTU3_16BIT_PHASE_COUNTING;
> > +	ret = clk_prepare_enable(ddata->clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/*
> > +	 * Phase counting mode 1 will be used as default
> > +	 * when initializing counters.
> > +	 */
> > +	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TMDR1,
> > +				 RZG2L_MTU3_TMDR1_PH_CNT_MODE_1);
> > +
> > +	/* Initialize 16-bit counter max value */
> > +	rzg2l_mtu3_8bit_ch_write(priv->ch, RZG2L_MTU3_TCR,
> > +				 RZG2L_MTU3_TCR_CCLR_TGRA);
> > +	rzg2l_mtu3_16bit_ch_write(priv->ch, RZG2L_MTU3_TGRA, U16_MAX);
> > +
> > +	clk_disable(ddata->clk);
> 
> Should this be moved up near the clk_prepare_enable() call above?

OK.

> 
> > +MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>");
> > +MODULE_ALIAS("platform:rzg2l-mtu3-counter");
> > +MODULE_DESCRIPTION("Renesas RZ/G2L MTU3a counter driver");
> > +MODULE_LICENSE("GPL");
> 
> Add MODULE_IMPORT_NS(COUNTER) to import the COUNTER namespace.

OK.

> 
> Make sure you're based on top of the counter-next branch. You can find
> the Counter tree here:
> https://git.kernel.org/pub/scm/linux/kernel/git/wbg/counter.git

Agreed.


Cheers,
Biju