Renesas CMT, MTU2 and TMU timers DT support

Document DT bindings and parse them in the TMU driver.

Signed-off-by Laurent Pinchart <[email protected]>

drivers/clocksource/sh_tmu.c +23 -6

index ef8fb0b..2ece4ab 100644
@@ -24,6 +24,7 @@
 #include <linux/ioport.h>
 #include <linux/irq.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>

@@ -509,23 +510,46 @@ static int sh_tmu_map_memory(struct sh_tmu_device *tmu)
 	return 0;
 }
 
+static int sh_tmu_parse_dt(struct sh_tmu_device *tmu)
+{
+	struct device_node *np = tmu->pdev->dev.of_node;
+
+	tmu->num_channels = 3;
+	of_property_read_u32(np, "#renesas,channels", &tmu->num_channels);
+
+	if (tmu->num_channels != 2 && tmu->num_channels != 3) {
+		dev_err(&tmu->pdev->dev, "invalid number of channels %u\n",
+			tmu->num_channels);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int sh_tmu_setup(struct sh_tmu_device *tmu, struct platform_device *pdev)
 {
-	struct sh_timer_config *cfg = pdev->dev.platform_data;
 	const struct platform_device_id *id = pdev->id_entry;
 	unsigned int i;
 	int ret;
 
-	if (!cfg) {
-		dev_err(&tmu->pdev->dev, "missing platform data\n");
-		return -ENXIO;
-	}
-
 	tmu->pdev = pdev;
 	tmu->model = id->driver_data;
 
 	raw_spin_lock_init(&tmu->lock);
 
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		ret = sh_tmu_parse_dt(tmu);
+		if (ret < 0)
+			return ret;
+	} else if (pdev->dev.platform_data) {
+		struct sh_timer_config *cfg = pdev->dev.platform_data;

Quoted 4 lines · Laurent Pinchart

Document DT bindings and parse them in the TMU driver.

Signed-off-by: Laurent Pinchart
<[email protected]>

[snip]

Quoted 4 lines

drivers/clocksource/sh_tmu.c +0 -0

index ef8fb0b..2ece4ab 100644

Quoted 55 lines

drivers/clocksource/sh_tmu.c +29 -6

@@ -509,23 +510,46 @@ static int sh_tmu_map_memory(struct sh_tmu_device
 *tmu)
 	return 0;
 }
 
+static int sh_tmu_parse_dt(struct sh_tmu_device *tmu)
+{
+	struct device_node *np = tmu->pdev->dev.of_node;
+
+	tmu->num_channels = 3;
+	of_property_read_u32(np, "#renesas,channels", &tmu->num_channels);
+
+	if (tmu->num_channels != 2 && tmu->num_channels != 3) {
+		dev_err(&tmu->pdev->dev, "invalid number of channels %u\n",
+			tmu->num_channels);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int sh_tmu_setup(struct sh_tmu_device *tmu, struct platform_device
 *pdev) {
-	struct sh_timer_config *cfg = pdev->dev.platform_data;
 	const struct platform_device_id *id = pdev->id_entry;
 	unsigned int i;
 	int ret;
 
-	if (!cfg) {
-		dev_err(&tmu->pdev->dev, "missing platform data\n");
-		return -ENXIO;
-	}
-
 	tmu->pdev = pdev;
 	tmu->model = id->driver_data;
 
 	raw_spin_lock_init(&tmu->lock);
 
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		ret = sh_tmu_parse_dt(tmu);
+		if (ret < 0)
+			return ret;
+	} else if (pdev->dev.platform_data) {
+		struct sh_timer_config *cfg = pdev->dev.platform_data;
+
+		tmu->num_channels = hweight8(cfg->channels_mask);
+	} else {
+		dev_err(&tmu->pdev->dev, "missing platform data\n");
+		return -ENXIO;
+	}
+
 
 	/* Get hold of clock. */

drivers/clocksource/sh_tmu.c +0 -0

 	tmu->clk = clk_get(&tmu->pdev->dev, "fck");
 	if (IS_ERR(tmu->clk)) {

[snip]

I had a bit of trouble running this code and after some investigation it seems to me that in the case of using DT id will be NULL and thus things do not go well when id->driver_data is accessed.

The following incremental patch resolved the problem for me.

drivers/clocksource/sh_tmu.c +2 -1

index 2ece4ab..3f90927 100644
@@ -523,6 +523,8 @@ static int sh_tmu_parse_dt(struct sh_tmu_device *tmu)
 		return -EINVAL;
 	}
 
+	tmu->model = SH_TMU;
+
 	return 0;
 }
 

@@ -533,7 +535,6 @@ static int sh_tmu_setup(struct sh_tmu_device *tmu,
 struct platform_device *pdev) int ret;
 
 	tmu->pdev = pdev;
-	tmu->model = id->driver_data;
 
 	raw_spin_lock_init(&tmu->lock);
 

@@ -545,6 +546,7 @@ static int sh_tmu_setup(struct sh_tmu_device *tmu,

struct platform_device *pdev) struct sh_timer_config *cfg = pdev->dev.platform_data;

drivers/clocksource/sh_tmu.c +1 -0

 		tmu->num_channels = hweight8(cfg->channels_mask);
+		tmu->model = id->driver_data;
 	} else {
 		dev_err(&tmu->pdev->dev, "missing platform data\n");
 		return -ENXIO;

Document DT bindings and parse them in the TMU driver.

Signed-off-by Laurent Pinchart <[email protected]>

---
 .../devicetree/bindings/timer/renesas,tmu.txt      | 40 +++++++++++++++++++
 drivers/clocksource/sh_tmu.c                       | 45 ++++++++++++++++++----
 2 files changed, 77 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/timer/renesas,tmu.txt

Documentation/devicetree/bindings/timer/renesas,tmu.txt new file +30 -0

new file mode 100644
index 0000000..2392e89
@@ -0,0 +1,40 @@
+* Renesas R-Car Timer Unit (TMU)
+
+The TMU is a 32-bit timer/counter with configurable clock inputs and
+programmable compare match.
+
+Channels share hardware resources but their counter and compare match value
+are independent. The TMU hardware supports up to three channels.
+
+Required Properties:
+
+  - compatible: must contain "renesas,tmu"
+
+  - reg: base address and length of the registers block for the timer module.
+
+  - interrupts: interrupt-specifier for the timer, one per channel.
+
+  - clocks: a list of phandle + clock-specifier pairs, one for each entry
+    in clock-names.
+  - clock-names: must contain "fck" for the functional clock.
+
+Optional Properties:
+
+  - #renesas,channels: number of channels implemented by the timer, must be 2
+    or 3 (if not specified the value defaults to 3).
+
+
+Example: R8A7779 (R-Car H1) TMU0 node
+
+	tmu0: timer at ffd80000 {
+		compatible = "renesas,tmu";

Quoted 59 lines · Laurent Pinchart

Document DT bindings and parse them in the TMU driver.

Signed-off-by: Laurent Pinchart
<[email protected]>
---

 .../devicetree/bindings/timer/renesas,tmu.txt      | 40 +++++++++++++++++
 drivers/clocksource/sh_tmu.c                       | 45 +++++++++++++----
 2 files changed, 77 insertions(+), 8 deletions(-)
 create mode 100644

Documentation/devicetree/bindings/timer/renesas,tmu.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/timer/renesas,tmu.txt new file mode 100644
index 0000000..2392e89

Documentation/devicetree/bindings/timer/renesas,tmu.txt new file +6 -0

@@ -0,0 +1,40 @@
+* Renesas R-Car Timer Unit (TMU)
+
+The TMU is a 32-bit timer/counter with configurable clock inputs and
+programmable compare match.
+
+Channels share hardware resources but their counter and compare match

value

Documentation/devicetree/bindings/timer/renesas,tmu.txt new file +7 -0

+are independent. The TMU hardware supports up to three channels.
+
+Required Properties:
+
+  - compatible: must contain "renesas,tmu"
+
+  - reg: base address and length of the registers block for the timer

module.

Documentation/devicetree/bindings/timer/renesas,tmu.txt new file +10 -0

+
+  - interrupts: interrupt-specifier for the timer, one per channel.
+
+  - clocks: a list of phandle + clock-specifier pairs, one for each entry
+    in clock-names.
+  - clock-names: must contain "fck" for the functional clock.
+
+Optional Properties:
+
+  - #renesas,channels: number of channels implemented by the timer, must

be 2

Documentation/devicetree/bindings/timer/renesas,tmu.txt new file +14 -0

+    or 3 (if not specified the value defaults to 3).
+
+
+Example: R8A7779 (R-Car H1) TMU0 node
+
+	tmu0: timer at ffd80000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd80000 0x30>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 41 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 42 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
+		clock-names = "fck";

Documentation/devicetree/bindings/timer/renesas,tmu.txt line 40

Quoted 3 lines

Documentation/devicetree/bindings/timer/renesas,tmu.txt new file +3 -0

+
+		#renesas,channels = <3>;
+	};

As per my feedback for "[PATCH v3 14/19] ARM: shmobile: r8a7779: Add TMU devices to DT" I believe the IRQ values should be 32, 33 and 34, not 40, 41, 42. I realise this is just an example but I feel that I should mention it.

Document DT bindings and parse them in the MTU2 driver.

Signed-off-by Laurent Pinchart <[email protected]>

Tested-by Wolfram Sang <[email protected]>

---
  .../devicetree/bindings/timer/renesas,mtu2.txt     | 40 ++++++++++++++++++++++
  drivers/clocksource/sh_mtu2.c                      |  8 +++++
  2 files changed, 48 insertions(+)

create mode 100644 Documentation/devicetree/bindings/timer/renesas,mtu2.txt

Documentation/devicetree/bindings/timer/renesas,mtu2.txt new file +6 -0

new file mode 100644
index 0000000..c5dd032
@@ -0,0 +1,40 @@
+* Renesas R-Car Multi-Function Timer Pulse Unit 2 (MTU2)
+
+The MTU2 is a multi-purpose, multi-channel timer/counter with configurable
+clock inputs and programmable compare match.
+
+Channels share hardware resources but their counter and compare match value

drivers/clocksource/sh_mtu2.c +0 -0

index ebdf9d1..79387af 100644

drivers/clocksource/sh_mtu2.c +2 -0

@@ -502,11 +503,18 @@ static const struct platform_device_id sh_mtu2_id_table[] = {
  };
  MODULE_DEVICE_TABLE(platform, sh_mtu2_id_table);
 
+static const struct of_device_id sh_mtu2_of_table[] = {
+	{ .compatible = "renesas,mtu2" },

Quoted 36 lines · Laurent Pinchart

Document DT bindings and parse them in the MTU2 driver.

Signed-off-by: Laurent Pinchart
<[email protected]>

Tested-by Wolfram Sang <[email protected]>

---

 .../devicetree/bindings/timer/renesas,mtu2.txt     | 40 +++++++++++++++++
 drivers/clocksource/sh_mtu2.c                      |  8 +++++
  2 files changed, 48 insertions(+)

create mode 100644 Documentation/devicetree/bindings/timer/renesas,mtu2.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/timer/renesas,mtu2.txt new file mode 100644
index 0000000..c5dd032

Documentation/devicetree/bindings/timer/renesas,mtu2.txt new file +3 -0

@@ -0,0 +1,40 @@
+* Renesas R-Car Multi-Function Timer Pulse Unit 2 (MTU2)
+
+The MTU2 is a multi-purpose, multi-channel timer/counter with

configurable

Documentation/devicetree/bindings/timer/renesas,mtu2.txt new file +3 -0

+clock inputs and programmable compare match.
+
+Channels share hardware resources but their counter and compare match

value
+are independent. The MTU2 hardware supports five channels indexed from 0 to 4.

Documentation/devicetree/bindings/timer/renesas,mtu2.txt new file +6 -0

+
+Required Properties:
+
+  - compatible: must contain "renesas,mtu2"
+
+  - reg: base address and length of the registers block for the timer

Quoted 5 lines

module.

Documentation/devicetree/bindings/timer/renesas,mtu2.txt new file +2 -0

+
+  - interrupt-parent, interrupts: interrupt specifiers for the timer, one

for
+ each entry in interrupt-names.

The "interrupt-parent" has never been a required property, it can and should be inherited from an upper level node.

[...]

Quoted 4 lines

drivers/clocksource/sh_mtu2.c +0 -0

index ebdf9d1..79387af 100644

[...]

Quoted 5 lines

drivers/clocksource/sh_mtu2.c +0 -0

@@ -502,11 +503,18 @@ static const struct platform_device_id

sh_mtu2_id_table[] = {>
};
MODULE_DEVICE_TABLE(platform, sh_mtu2_id_table);

drivers/clocksource/sh_mtu2.c static const struct platform_device_id sh_mtu2_id_table[] = { · line 517

Quoted 12 lines

drivers/clocksource/sh_mtu2.c +7 -0

+static const struct of_device_id sh_mtu2_of_table[] = {
+	{ .compatible = "renesas,mtu2" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sh_mtu2_of_table);
+
  static struct platform_driver sh_mtu2_device_driver = {
  	.probe		= sh_mtu2_probe,
  	.remove		= sh_mtu2_remove,
  	.driver		= {
  		.name	= "sh_mtu2",
+		.of_match_table = of_match_ptr(sh_mtu2_of_table),

No need for of_match_ptr() since the table us not protected by #ifdef CONFIG_OF...

Quoted 14 lines · On Saturday 14 June 2014 21:00:20 Sergei Shtylyov wrote:

Quoted 13 lines · Laurent Pinchart

Document DT bindings and parse them in the MTU2 driver.

Signed-off-by: Laurent Pinchart
<[email protected]>

Tested-by Wolfram Sang <[email protected]>

---

 .../devicetree/bindings/timer/renesas,mtu2.txt     | 40 +++++++++++++++
 drivers/clocksource/sh_mtu2.c                      |  8 +++++

  2 files changed, 48 insertions(+)

create mode 100644 Documentation/devicetree/bindings/timer/renesas,mtu2.txt

Quoted 13 lines

Quoted 5 lines

drivers/clocksource/sh_mtu2.c +0 -0

index ebdf9d1..79387af 100644

[...]

Quoted 5 lines

drivers/clocksource/sh_mtu2.c +0 -0

@@ -502,11 +503,18 @@ static const struct platform_device_id
 sh_mtu2_id_table[] = {
  };
  MODULE_DEVICE_TABLE(platform, sh_mtu2_id_table);
 

Quoted 15 lines

drivers/clocksource/sh_mtu2.c static const struct platform_device_id sh_mtu2_id_table[] = { · line 517

Quoted 12 lines

drivers/clocksource/sh_mtu2.c +7 -0

+static const struct of_device_id sh_mtu2_of_table[] = {
+	{ .compatible = "renesas,mtu2" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sh_mtu2_of_table);
+
  static struct platform_driver sh_mtu2_device_driver = {
  	.probe		= sh_mtu2_probe,
  	.remove		= sh_mtu2_remove,
  	.driver		= {
  		.name	= "sh_mtu2",
+		.of_match_table = of_match_ptr(sh_mtu2_of_table),

No need for of_match_ptr() since the table us not protected by #ifdef CONFIG_OF...

I'll fix that.

Add the CMT0 and CMT1 counters to the r8a7790 device tree and make them disabled by default.

Signed-off-by Laurent Pinchart <[email protected]>

---
  arch/arm/boot/dts/r8a7790.dtsi | 34 ++++++++++++++++++++++++++++++++++
  1 file changed, 34 insertions(+)

arch/arm/boot/dts/r8a7790.dtsi +0 -0

index 29907c9..cc32ac5 100644

arch/arm/boot/dts/r8a7790.dtsi +4 -0

+		interrupts = <0 142 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 143 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp1_clks R8A7790_CLK_CMT0>;
+		clock-names = "fck";

Quoted 18 lines · Laurent Pinchart

Add the CMT0 and CMT1 counters to the r8a7790 device tree and make them disabled by default.

Signed-off-by: Laurent Pinchart
<[email protected]>
---

  arch/arm/boot/dts/r8a7790.dtsi | 34 ++++++++++++++++++++++++++++++++++
  1 file changed, 34 insertions(+)

(quoted diff) +0 -0

b/arch/arm/boot/dts/r8a7790.dtsi index 29907c9..cc32ac5 100644

arch/arm/boot/dts/r8a7790.dtsi +0 -0

@@ -206,6 +206,40 @@
  			     <1 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
  	};
 

arch/arm/boot/dts/r8a7790.dtsi line 212

Quoted 4 lines

arch/arm/boot/dts/r8a7790.dtsi +4 -0

+	cmt0: timer at ffca0000 {
+		compatible = "renesas,cmt-48-gen2";
+		reg = <0 0xffca0000 0 0x1004>;
+		interrupt-parent = <&gic>;

This prop is inherited from the root node, so unneeded here.

Quoted 4 lines

arch/arm/boot/dts/r8a7790.dtsi +4 -0

+		interrupts = <0 142 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 143 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp1_clks R8A7790_CLK_CMT0>;
+		clock-names = "fck";

arch/arm/boot/dts/r8a7790.dtsi line 226

Quoted 10 lines

arch/arm/boot/dts/r8a7790.dtsi +10 -0

+
+		renesas,channels-mask = <0x60>;
+
+		status = "disabled";
+	};
+
+	cmt1: timer at e6130000 {
+		compatible = "renesas,cmt-48-gen2";
+		reg = <0 0xe6130000 0 0x1004>;
+		interrupt-parent = <&gic>;

Likewise.

Add the CMT0 and CMT1 counters to the r8a7791 device tree and make them disabled by default.

Signed-off-by Laurent Pinchart <[email protected]>

---
  arch/arm/boot/dts/r8a7791.dtsi | 34 ++++++++++++++++++++++++++++++++++
  1 file changed, 34 insertions(+)

arch/arm/boot/dts/r8a7791.dtsi +0 -0

index 380d058..b56a995 100644

arch/arm/boot/dts/r8a7791.dtsi +4 -0

+		interrupts = <0 142 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 143 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp1_clks R8A7791_CLK_CMT0>;
+		clock-names = "fck";

Add the TMU0, TMU1 and TMU2 counters to the r8a7779 device tree and make them disabled by default.

Signed-off-by Laurent Pinchart <[email protected]>

---
 arch/arm/boot/dts/r8a7779.dtsi | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

arch/arm/boot/dts/r8a7779.dtsi +42 -0

index 94e2fc8..bf716ce 100644
@@ -266,6 +266,48 @@
 		reg = <0xffc48000 0x38>;
 	};
 
+	tmu0: timer at ffd80000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd80000 0x30>;
+		interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 41 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 42 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
+		clock-names = "fck";
+
+		#renesas,channels = <3>;
+
+		status = "disabled";
+	};
+
+	tmu1: timer at ffd81000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd81000 0x30>;
+		interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 45 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 46 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU1>;
+		clock-names = "fck";
+
+		#renesas,channels = <3>;
+
+		status = "disabled";
+	};
+
+	tmu2: timer at ffd82000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd82000 0x30>;
+		interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 49 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 50 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU2>;
+		clock-names = "fck";
+
+		#renesas,channels = <3>;
+
+		status = "disabled";
+	};
+
 	sata: sata at fc600000 {
 		compatible = "renesas,rcar-sata";
 		reg = <0xfc600000 0x2000>;

Signature · 1 line

1.8.5.5

Quoted 59 lines · Laurent Pinchart

Add the TMU0, TMU1 and TMU2 counters to the r8a7779 device tree and make them disabled by default.

Signed-off-by: Laurent Pinchart
<[email protected]>
---

 arch/arm/boot/dts/r8a7779.dtsi | 42 +++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

(quoted diff) +0 -0

b/arch/arm/boot/dts/r8a7779.dtsi index 94e2fc8..bf716ce 100644

arch/arm/boot/dts/r8a7779.dtsi +41 -0

@@ -266,6 +266,48 @@
 		reg = <0xffc48000 0x38>;
 	};
 
+	tmu0: timer at ffd80000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd80000 0x30>;
+		interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 41 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 42 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
+		clock-names = "fck";
+
+		#renesas,channels = <3>;
+
+		status = "disabled";
+	};
+
+	tmu1: timer at ffd81000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd81000 0x30>;
+		interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 45 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 46 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU1>;
+		clock-names = "fck";
+
+		#renesas,channels = <3>;
+
+		status = "disabled";
+	};
+
+	tmu2: timer at ffd82000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd82000 0x30>;
+		interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 49 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 50 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU2>;
+		clock-names = "fck";
+
+		#renesas,channels = <3>;
+
+		status = "disabled";
+	};

arch/arm/boot/dts/r8a7779.dtsi line 313

Quoted 4 lines

arch/arm/boot/dts/r8a7779.dtsi +1 -0

+
 	sata: sata at fc600000 {
 		compatible = "renesas,rcar-sata";
 		reg = <0xfc600000 0x2000>;

There are no interrupt-parents in in the nodes above nodes although the documentation of the binding has one in the example. Is it just an oversight in this patch?

I believe that the IRQ numbers are not correct.

In setup-r8a7779.c I see gic_iid(0x40). But above I see 40. I think the correct value is 0x40 - 32 = 32. Likewise for the other IRQs of tmu0. And looking at the documentaiton this seems true for tmu1 and tmu2 too.

I have successfully booted a marzen board with the following incremental patch to the DT nodes.

arch/arm/boot/dts/r8a7779.dtsi +4 -3

index bf716ce..81714ce 100644
@@ -269,9 +269,10 @@
 	tmu0: timer at ffd80000 {
 		compatible = "renesas,tmu";
 		reg = <0xffd80000 0x30>;
-		interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>,
-			     <0 41 IRQ_TYPE_LEVEL_HIGH>,
-			     <0 42 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 33 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 34 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
 		clock-names = "fck";
 

arch/arm/boot/dts/r8a7779.dtsi +0 -0

 		clocks = <&mstp0_clks R8A7779_CLK_TMU1>;
 		clock-names = "fck";
 

@@ -296,10 +298,11 @@
 

arch/arm/boot/dts/r8a7779.dtsi +0 -0

 		clocks = <&mstp0_clks R8A7779_CLK_TMU2>;
 		clock-names = "fck";

Quoted 18 lines

arch/arm/boot/dts/r8a7779.dtsi +4 -3

index bf716ce..81714ce 100644
@@ -269,9 +269,10 @@
      tmu0: timer at ffd80000 {
              compatible = "renesas,tmu";
              reg = <0xffd80000 0x30>;
-             interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 41 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 42 IRQ_TYPE_LEVEL_HIGH>;
+             interrupt-parent = <&gic>;
+             interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 33 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 34 IRQ_TYPE_LEVEL_HIGH>;
              clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
              clock-names = "fck";
 

arch/arm/boot/dts/r8a7779.dtsi line 290

Quoted 11 lines

arch/arm/boot/dts/r8a7779.dtsi +4 -3

@@ -283,9 +284,10 @@
      tmu1: timer at ffd81000 {
              compatible = "renesas,tmu";
              reg = <0xffd81000 0x30>;
-             interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 45 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 46 IRQ_TYPE_LEVEL_HIGH>;
+             interrupt-parent = <&gic>;
+             interrupts = <0 35 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 36 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 37 IRQ_TYPE_LEVEL_HIGH>;

If I'm not mistaken the interrupts should be 36, 37 and 38.

Quoted 5 lines

arch/arm/boot/dts/r8a7779.dtsi +0 -0

              clocks = <&mstp0_clks R8A7779_CLK_TMU1>;
              clock-names = "fck";
 

@@ -296,10 +298,11 @@
 

arch/arm/boot/dts/r8a7779.dtsi line 305

Quoted 10 lines

arch/arm/boot/dts/r8a7779.dtsi +4 -3

      tmu2: timer at ffd82000 {
              compatible = "renesas,tmu";
+             interrupt-parent = <&gic>;
              reg = <0xffd82000 0x30>;
-             interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 49 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 50 IRQ_TYPE_LEVEL_HIGH>;
+             interrupts = <0 38 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 39 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 40 IRQ_TYPE_LEVEL_HIGH>;

And 40, 41 and 42 here.

arch/arm/boot/dts/r8a7779.dtsi line 307

Quoted 2 lines

arch/arm/boot/dts/r8a7779.dtsi +0 -0

              clocks = <&mstp0_clks R8A7779_CLK_TMU2>;
              clock-names = "fck";

Quoted 19 lines · Laurent Pinchart

Quoted 19 lines

(quoted diff) +0 -0

b/arch/arm/boot/dts/r8a7779.dtsi index bf716ce..81714ce 100644

arch/arm/boot/dts/r8a7779.dtsi +4 -3

@@ -269,9 +269,10 @@
      tmu0: timer at ffd80000 {
              compatible = "renesas,tmu";
              reg = <0xffd80000 0x30>;
-             interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 41 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 42 IRQ_TYPE_LEVEL_HIGH>;
+             interrupt-parent = <&gic>;
+             interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 33 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 34 IRQ_TYPE_LEVEL_HIGH>;
 
              clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
              clock-names = "fck";

Quoted 13 lines

arch/arm/boot/dts/r8a7779.dtsi line 290

Quoted 11 lines

arch/arm/boot/dts/r8a7779.dtsi +4 -3

@@ -283,9 +284,10 @@
      tmu1: timer at ffd81000 {
              compatible = "renesas,tmu";
              reg = <0xffd81000 0x30>;
-             interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 45 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 46 IRQ_TYPE_LEVEL_HIGH>;
+             interrupt-parent = <&gic>;
+             interrupts = <0 35 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 36 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 37 IRQ_TYPE_LEVEL_HIGH>;

If I'm not mistaken the interrupts should be 36, 37 and 38.

(diving into the documentation)

Yes, probably 36, 37, and 38. I say "probably", as I find it non-trivial to convert from docs to DT.

What happens with 35, which is "TMU21 (input capture)"? As DT describes the hardware, shouldn't it be mentioned here?

Quoted 5 lines

Quoted 5 lines

arch/arm/boot/dts/r8a7779.dtsi +0 -0

              clocks = <&mstp0_clks R8A7779_CLK_TMU1>;
              clock-names = "fck";
 

@@ -296,10 +298,11 @@
 

Quoted 12 lines

arch/arm/boot/dts/r8a7779.dtsi line 305

Quoted 10 lines

arch/arm/boot/dts/r8a7779.dtsi +4 -3

      tmu2: timer at ffd82000 {
              compatible = "renesas,tmu";
+             interrupt-parent = <&gic>;
              reg = <0xffd82000 0x30>;
-             interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 49 IRQ_TYPE_LEVEL_HIGH>,
-                          <0 50 IRQ_TYPE_LEVEL_HIGH>;
+             interrupts = <0 38 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 39 IRQ_TYPE_LEVEL_HIGH>,
+                          <0 40 IRQ_TYPE_LEVEL_HIGH>;

And 40, 41 and 42 here.

Likewise for "TMU51 (input capture)"?

Quoted 2 lines

Quoted 2 lines

arch/arm/boot/dts/r8a7779.dtsi +0 -0

              clocks = <&mstp0_clks R8A7779_CLK_TMU2>;
              clock-names = "fck";

Hi Simon,

Quoted 60 lines · On Monday 16 June 2014 17:47:07 Simon Horman wrote:

Quoted 59 lines · Laurent Pinchart

Add the TMU0, TMU1 and TMU2 counters to the r8a7779 device tree and make them disabled by default.

Signed-off-by: Laurent Pinchart
<[email protected]>
---

 arch/arm/boot/dts/r8a7779.dtsi | 42 +++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

(quoted diff) +0 -0

b/arch/arm/boot/dts/r8a7779.dtsi index 94e2fc8..bf716ce 100644

arch/arm/boot/dts/r8a7779.dtsi +41 -0

@@ -266,6 +266,48 @@
 		reg = <0xffc48000 0x38>;
 	};
 
+	tmu0: timer at ffd80000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd80000 0x30>;
+		interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 41 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 42 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
+		clock-names = "fck";
+
+		#renesas,channels = <3>;
+
+		status = "disabled";
+	};
+
+	tmu1: timer at ffd81000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd81000 0x30>;
+		interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 45 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 46 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU1>;
+		clock-names = "fck";
+
+		#renesas,channels = <3>;
+
+		status = "disabled";
+	};
+
+	tmu2: timer at ffd82000 {
+		compatible = "renesas,tmu";
+		reg = <0xffd82000 0x30>;
+		interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 49 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 50 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp0_clks R8A7779_CLK_TMU2>;
+		clock-names = "fck";
+
+		#renesas,channels = <3>;
+
+		status = "disabled";
+	};

Quoted 8 lines

arch/arm/boot/dts/r8a7779.dtsi line 313

Quoted 4 lines

arch/arm/boot/dts/r8a7779.dtsi +1 -0

+
 	sata: sata at fc600000 {
 		compatible = "renesas,rcar-sata";
 		reg = <0xfc600000 0x2000>;

There are no interrupt-parents in in the nodes above nodes although the documentation of the binding has one in the example. Is it just an oversight in this patch?

Not really.

The interrupt-parent property is required but can be inherited. Furthermore, the interrupts-extended property can be used to replace the interrupts and interrupt-parent properties. This should all be documented in the DT bindings, but not by duplicating the full explanation in dozens of different flavours in all bindings. We need standard wordings for common properties such as interrupts, clocks or reg.

Quoted 9 lines

I believe that the IRQ numbers are not correct.

In setup-r8a7779.c I see gic_iid(0x40). But above I see 40. I think the correct value is 0x40 - 32 = 32. Likewise for the other IRQs of tmu0. And looking at the documentaiton this seems true for tmu1 and tmu2 too.

I have successfully booted a marzen board with the following incremental patch to the DT nodes.

You're right, sorry about that. I'll fix the interrupt numbers and resubmit. The interrupt-parent is specified at the top level so there's no need to add it to the TMU nodes.

Quoted 18 lines

arch/arm/boot/dts/r8a7779.dtsi +4 -3

index bf716ce..81714ce 100644
@@ -269,9 +269,10 @@
 	tmu0: timer at ffd80000 {
 		compatible = "renesas,tmu";
 		reg = <0xffd80000 0x30>;
-		interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>,
-			     <0 41 IRQ_TYPE_LEVEL_HIGH>,
-			     <0 42 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 33 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 34 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp0_clks R8A7779_CLK_TMU0>;
 		clock-names = "fck";
 

arch/arm/boot/dts/r8a7779.dtsi line 290

Quoted 11 lines

arch/arm/boot/dts/r8a7779.dtsi +4 -3

@@ -283,9 +284,10 @@
 	tmu1: timer at ffd81000 {
 		compatible = "renesas,tmu";
 		reg = <0xffd81000 0x30>;
-		interrupts = <0 44 IRQ_TYPE_LEVEL_HIGH>,
-			     <0 45 IRQ_TYPE_LEVEL_HIGH>,
-			     <0 46 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 35 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 36 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 37 IRQ_TYPE_LEVEL_HIGH>;

If I'm not mistaken the interrupts should be 36, 37 and 38.

Quoted 5 lines

arch/arm/boot/dts/r8a7779.dtsi +0 -0

 		clocks = <&mstp0_clks R8A7779_CLK_TMU1>;
 		clock-names = "fck";
 

@@ -296,10 +298,11 @@
 

arch/arm/boot/dts/r8a7779.dtsi line 305

Quoted 10 lines

arch/arm/boot/dts/r8a7779.dtsi +4 -3

 	tmu2: timer at ffd82000 {
 		compatible = "renesas,tmu";
+		interrupt-parent = <&gic>;
 		reg = <0xffd82000 0x30>;
-		interrupts = <0 48 IRQ_TYPE_LEVEL_HIGH>,
-			     <0 49 IRQ_TYPE_LEVEL_HIGH>,
-			     <0 50 IRQ_TYPE_LEVEL_HIGH>;
+		interrupts = <0 38 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 39 IRQ_TYPE_LEVEL_HIGH>,
+			     <0 40 IRQ_TYPE_LEVEL_HIGH>;

And 40, 41 and 42 here.

Quoted 2 lines

arch/arm/boot/dts/r8a7779.dtsi +0 -0

 		clocks = <&mstp0_clks R8A7779_CLK_TMU2>;
 		clock-names = "fck";

Hello,

This patch set adds DT bindings to the Renesas CMT, MTU2 and TMU timers. Patches 02/19 to 13/19, 16/19 and 17/19 have already been posted in the previous version of this series. Patches 14/19, 15/19, 18/19 and 19/19 are new.

Compared to v2 I've rebased the patches on top of Simon's latest devel branch.

Compared to v1 I've dropped the channel subnodes from the CMT DT bindings and just use a channels mask, as I don't need to specify per-channel properties. I'm slightly uncertain about this change though, as I can't easily predict whether per-channel properties would be needed later for new hardware. It won't be difficult to reintroduce channel subnodes then, so I'd rather not clutter the DT bindings with channel subnodes now. Please feel free to disagree.

The first 11 patches should go through the timers tree while the last 8 patches should go through the Renesas SoC tree. Patches 12/19 to 15/19 have no build time or run time dependency on the drivers changes. Patches 16/19 to 19/19 may have a run time dependency on the drivers changes depending on the kernel configuration. Patches 16/19 and 17/19, for instance, require the driver changes only when architected timer support isn't enabled in the kernel.

Laurent Pinchart (19):
o clocksource: sh_tmu: Fix channel IRQ retrieval in legacy case
o clocksource: sh_cmt: Drop support for legacy platform data
o clocksource: sh_cmt: Replace global spinlock with a per-device spinlock
o clocksource: sh_tmu: Drop support for legacy platform data
o clocksource: sh_tmu: Replace global spinlock with a per-device spinlock
clocksource: sh_mtu2: Drop support for legacy platform data
clocksource: sh_mtu2: Replace global spinlock with a per-device
spinlock
o clocksource: shmobile: Remove unused sh_timer_config members
o clocksource: sh_cmt: Add DT support
clocksource: sh_tmu: Add DT support
clocksource: sh_mtu2: Add DT support

o ARM: shmobile: r8a7790: Add CMT devices to DT
o ARM: shmobile: r8a7791: Add CMT devices to DT
ARM: shmobile: r8a7779: Add TMU devices to DT
ARM: shmobile: r7s72100: Add MTU2 device to DT
o ARM: shmobile: lager-reference: Enable CMT0 in device tree
o ARM: shmobile: koelsch-reference: Enable CMT0 in device tree
ARM: shmobile: marzen-reference: Enable TMU0 in device tree
ARM: shmobile: genmai-reference: Enable MTU2 in device tree

 .../devicetree/bindings/timer/renesas,cmt.txt      |  49 +++++
 .../devicetree/bindings/timer/renesas,mtu2.txt     |  40 ++++
 .../devicetree/bindings/timer/renesas,tmu.txt      |  40 ++++
 arch/arm/boot/dts/r7s72100-genmai.dts              |   4 +
 arch/arm/boot/dts/r7s72100.dtsi                    |  10 +
 arch/arm/boot/dts/r8a7779-marzen.dts               |   4 +
 arch/arm/boot/dts/r8a7779.dtsi                     |  42 ++++
 arch/arm/boot/dts/r8a7790-lager.dts                |   4 +
 arch/arm/boot/dts/r8a7790.dtsi                     |  34 +++
 arch/arm/boot/dts/r8a7791-koelsch.dts              |   4 +
 arch/arm/boot/dts/r8a7791.dtsi                     |  34 +++
 arch/arm/mach-shmobile/board-genmai-reference.c    |  16 --
 arch/arm/mach-shmobile/board-genmai.c              |  14 +-
 arch/arm/mach-shmobile/board-koelsch-reference.c   |   2 -
 arch/arm/mach-shmobile/board-lager-reference.c     |   2 -
 arch/arm/mach-shmobile/board-marzen-reference.c    |  10 -
 arch/arm/mach-shmobile/include/mach/r7s72100.h     |   1 -
 arch/arm/mach-shmobile/include/mach/r8a7779.h      |   1 -
 arch/arm/mach-shmobile/include/mach/r8a7790.h      |   1 -
 arch/arm/mach-shmobile/include/mach/r8a7791.h      |   1 -
 arch/arm/mach-shmobile/setup-r7s72100.c            |  21 --
 arch/arm/mach-shmobile/setup-r8a7779.c             |  17 +-
 arch/arm/mach-shmobile/setup-r8a7790.c             |   7 +-
 arch/arm/mach-shmobile/setup-r8a7791.c             |   7 +-
 drivers/clocksource/sh_cmt.c                       | 233 ++++++++-------------
 drivers/clocksource/sh_mtu2.c                      | 146 ++++---------
 drivers/clocksource/sh_tmu.c                       | 125 +++++------
 include/linux/sh_timer.h                           |   5 -
 28 files changed, 467 insertions(+), 407 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/timer/renesas,cmt.txt
 create mode 100644 Documentation/devicetree/bindings/timer/renesas,mtu2.txt
 create mode 100644 Documentation/devicetree/bindings/timer/renesas,tmu.txt

Signature · 8 lines

Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html