[PATCH/RFC v4 0/5] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[PATCH/RFC v4 0/5] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Geert Uytterhoeven]

	Hi Mike, Stephen, et al.,

As discussed at ELCE last week, here's my RFC introducing a new unified
binding and driver for the Renesas CPG (Clock Pulse Generator) and MSSR
(Module Standby and Software Reset) blocks.  This is supposed to be more
in-line with current CCF best practices, and allows expansion to cover
the module reset functionality in the future.

The new bindings are intended to be used initially for the new R-Car
Gen3 SoCs (r8a7795), but we may decide to migrate older SoCs somewhen in
the future.

  - Patch 1 introduces the new bindings,
  - Patch 2 introduces clock definitions for r8a7795, which are
    technically also part of the binding,
  - Patch 3 prepares the existing div6 driver for reuse,
  - Patch 4 introduces the new CPG/MSSR driver core, to be used by
    several SoC-specific drivers,
  - Patch 5 introduces the new r8a7795-specific driver.

The idea is to get the bindings (for r8a7795, i.e. patches 1 and 2)
accepted in v4.4, so we can continue developing and prepare final
support for r8a7795 in v4.5.

Compared to v3, the major change is the addition of a preliminary
version of the actual driver.

Implementation notes:
  - The driver contains lots of pr_debug() and WARN_ON() calls. Many of
    these will be removed in future versions,
  - Conversion between (sparse) clock IDs and (packed) clock indices
    will be reworked. Probably most of it can be handled in a macro when
    populating the r8a779*_mod_clks[] arrays,
  - Probably r8a779*_clk_types can be removed in favor of just using
    CLK_TYPE_CUSTOM, and differentiating based on cpg_core_clk.id. But
    the clk_types may make it easier to share code between different
    SoCs of the same family.
  - Most clock data is __initconst, to save memory in multi-platform
    kernels,
  - Setting CLK_ENABLE_HAND_OFF for critical clocks (e.g. the GIC clock)
    is implemented (if CLK_ENABLE_HAND_OFF is available),
  - Support for using special core clocks for power management (e.g. the
    ZB clock on r8a73a4/sh7a0) is not yet fully implemented,
  - Module clocks in DTS are no longer defines, but literal numbers
    matching the numbers in the datasheet (just like IRQs),
  - include/dt-bindings/clock/*-cpg-mssr.h now contains all core clock
    outputs, as listed in the datasheet,
  - Should include/dt-bindings/clock/*-cpg-mssr.h use contiguous
    numbering, or should the same clock use the same number for
    different SoCs of the same family?

Memory considerations on r8a7791/koelsch:
  - Static size impact:
      - DTB: -7.5 KiB
      - initdata: +8 KiB (due to PAGE_SIZE rounding, actual + ca. 5 KiB)
      - reserved: -24 KiB (includes unflattened DTB)
  - However, the big saving seems to be in the runtime memory usage.
    MemAvailable has increased by 248 KiB, presumably due to less
    memory being used by the in-memory device tree representation.

For your convenience, I pushed these patches, the r8a7795 DTS
conversion, and a Proof-of-Concept for R-Car M2-W (r8a7791) to the
topic/cpg-mssr-v4 branch (based on topic/gen3-latest) of my
renesas-drivers repository at
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/cpg-mssr-v4

Thanks for your comments!

References:
  - v3 = "[PATCH/RFC v3 0/3] clk: shmobile: Add new Renesas CPG/MSSR DT
    bindings" (http://www.spinics.net/lists/linux-sh/msg45870.html)
  - v2+ ≈ "[PATCH v8 00/05] Renesas R-Car Gen3 CPG support V8"
    (http://www.spinics.net/lists/linux-clk/msg03288.html)
  - v2 = "[PATCH/RFC v2 0/4] Renesas CPG/MSTP DT Binding Proposal"
    (http://www.spinics.net/lists/linux-clk/msg03132.html)
  - v1 = "Renesas CPG/MSTP DT Binding Proposal"
    (http://www.spinics.net/lists/linux-clk/msg01189.html)

Geert Uytterhoeven (5):
  [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings
  [RFC] clk: shmobile: Add r8a7795 CPG Core Clock Definitions
  [RFC] clk: shmobile: div6: Extract cpg_div6_register()
  [RFC] clk: shmobile: cpg-mssr: Add new CPG/MSSR driver core
  [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

 .../devicetree/bindings/clock/renesas,cpg-mssr.txt |  71 +++
 drivers/clk/shmobile/Makefile                      |   3 +-
 drivers/clk/shmobile/clk-cpg-mssr.c                | 578 +++++++++++++++++++++
 drivers/clk/shmobile/clk-cpg-mssr.h                | 118 +++++
 drivers/clk/shmobile/clk-div6.c                    | 119 +++--
 drivers/clk/shmobile/clk-div6.h                    |   3 +
 drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c        | 373 +++++++++++++
 include/dt-bindings/clock/r8a7795-cpg-mssr.h       |  63 +++
 include/dt-bindings/clock/renesas-cpg-mssr.h       |  15 +
 9 files changed, 1297 insertions(+), 46 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
 create mode 100644 drivers/clk/shmobile/clk-cpg-mssr.c
 create mode 100644 drivers/clk/shmobile/clk-cpg-mssr.h
 create mode 100644 drivers/clk/shmobile/clk-div6.h
 create mode 100644 drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c
 create mode 100644 include/dt-bindings/clock/r8a7795-cpg-mssr.h
 create mode 100644 include/dt-bindings/clock/renesas-cpg-mssr.h

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

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

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

[PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Geert Uytterhoeven]

On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
Generator) and MSSR (Module Standby and Software Reset) blocks are
intimately connected, and share the same register block.

Hence it makes sense to describe these two blocks using a
single device node in DT, instead of using a hierarchical structure with
multiple nodes, using a mix of generic and SoC-specific bindings.

These new DT bindings are intended to replace the existing DT bindings
for CPG core clocks ("renesas,*-cpg-clocks", "renesas,cpg-div6-clock")
and module clocks ("renesas,*-mstp-clocks"), at least for new SoCs.

This will make it easier to add module reset support later, which is
currently not implemented, and difficult to achieve using the existing
bindings due to the intertwined register layout.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v4:
  - No changes,

v3:
  - Integrate CPG and MSSR,

v2:
  - Switch from MSTP to MSSR.
---
 .../devicetree/bindings/clock/renesas,cpg-mssr.txt | 71 ++++++++++++++++++++++
 include/dt-bindings/clock/renesas-cpg-mssr.h       | 15 +++++
 2 files changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
 create mode 100644 include/dt-bindings/clock/renesas-cpg-mssr.h

diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
new file mode 100644
index 0000000000000000..a56836aa2131a8db
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
@@ -0,0 +1,71 @@
+* Renesas Clock Pulse Generator / Module Standby and Software Reset
+
+On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator)
+and MSSR (Module Standby and Software Reset) blocks are intimately connected,
+and share the same register block.
+
+They provide the following functionalities:
+  - The CPG block generates various core clocks,
+  - The MSSR block provides two functions:
+      1. Module Standby, providing a Clock Domain to control the clock supply
+	 to individual SoC devices,
+      2. Reset Control, to perform a software reset of individual SoC devices.
+
+Required Properties:
+  - compatible: Must be one of:
+      - "renesas,r8a7791-cpg-mssr" for the r8a7791 SoC
+      - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC
+
+  - reg: Base address and length of the memory resource used by the CPG/MSSR
+    block
+
+  - clocks: References to external parent clocks, one entry for each entry in
+    clock-names
+  - clock-names: List of external parent clock names. Valid names are:
+      - "extal" (r8a7791, r8a7795)
+      - "extalr" (r8a7795)
+      - "usb_extal" (r8a7791)
+
+  - #clock-cells: Must be 2
+      - For CPG core clocks, the two clock specifier cells must be "CPG_CORE"
+	and a core clock reference, as defined in
+	<dt-bindings/clock/*-cpg-mssr.h>.
+      - For module clocks, the two clock specifier cells must be "CPG_MOD" and
+	a module number, as defined in the datasheet.
+
+  - #power-domain-cells: Must be 0
+      - SoC devices that are part of the CPG/MSSR Clock Domain and can be
+	power-managed through Module Standby should refer to the CPG device
+	node in their "power-domains" property, as documented by the generic PM
+	Domain bindings in
+	Documentation/devicetree/bindings/power/power_domain.txt.
+
+
+Examples
+--------
+
+  - CPG device node:
+
+	cpg: clock-controller@e6150000 {
+		compatible = "renesas,r8a7795-cpg-mssr";
+		reg = <0 0xe6150000 0 0x1000>;
+		clocks = <&extal_clk>, <&extalr_clk>;
+		clock-names = "extal", "extalr";
+		#clock-cells = <2>;
+		#power-domain-cells = <0>;
+	};
+
+
+  - CPG/MSSR Clock Domain member device node:
+
+	scif2: serial@e6e88000 {
+		compatible = "renesas,scif-r8a7795", "renesas,scif";
+		reg = <0 0xe6e88000 0 64>;
+		interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_MOD 310>;
+		clock-names = "sci_ick";
+		dmas = <&dmac1 0x13>, <&dmac1 0x12>;
+		dma-names = "tx", "rx";
+		power-domains = <&cpg>;
+		status = "disabled";
+	};
diff --git a/include/dt-bindings/clock/renesas-cpg-mssr.h b/include/dt-bindings/clock/renesas-cpg-mssr.h
new file mode 100644
index 0000000000000000..569a3cc33ffb5bc7
--- /dev/null
+++ b/include/dt-bindings/clock/renesas-cpg-mssr.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright (C) 2015 Renesas Electronics Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#ifndef __DT_BINDINGS_CLOCK_RENESAS_CPG_MSSR_H__
+#define __DT_BINDINGS_CLOCK_RENESAS_CPG_MSSR_H__
+
+#define CPG_CORE			0	/* Core Clock */
+#define CPG_MOD				1	/* Module Clock */
+
+#endif /* __DT_BINDINGS_CLOCK_RENESAS_CPG_MSSR_H__ */
-- 
1.9.1

[PATCH v4 2/5] [RFC] clk: shmobile: Add r8a7795 CPG Core Clock Definitions

[Geert Uytterhoeven]

Add all R-Car H3 CPG Core Clock Outputs defined in the datasheet.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v4:
  - Add all clocks instead of just the ones used by the current DTS.

v3:
  - New.
---
 include/dt-bindings/clock/r8a7795-cpg-mssr.h | 63 ++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 include/dt-bindings/clock/r8a7795-cpg-mssr.h

diff --git a/include/dt-bindings/clock/r8a7795-cpg-mssr.h b/include/dt-bindings/clock/r8a7795-cpg-mssr.h
new file mode 100644
index 0000000000000000..e864aae0a2561c4b
--- /dev/null
+++ b/include/dt-bindings/clock/r8a7795-cpg-mssr.h
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2015 Renesas Electronics Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#ifndef __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__
+#define __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/* r8a7795 CPG Core Clocks */
+#define R8A7795_CLK_Z			0
+#define R8A7795_CLK_Z2			1
+#define R8A7795_CLK_ZR			2
+#define R8A7795_CLK_ZG			3
+#define R8A7795_CLK_ZTR			4
+#define R8A7795_CLK_ZTRD2		5
+#define R8A7795_CLK_ZT			6
+#define R8A7795_CLK_ZX			7
+#define R8A7795_CLK_S0D1		8
+#define R8A7795_CLK_S0D4		9
+#define R8A7795_CLK_S1D1		10
+#define R8A7795_CLK_S1D2		11
+#define R8A7795_CLK_S1D4		12
+#define R8A7795_CLK_S2D1		13
+#define R8A7795_CLK_S2D2		14
+#define R8A7795_CLK_S2D4		15
+#define R8A7795_CLK_S3D1		16
+#define R8A7795_CLK_S3D2		17
+#define R8A7795_CLK_S3D4		18
+#define R8A7795_CLK_LB			19
+#define R8A7795_CLK_CL			20
+#define R8A7795_CLK_ZB3			21
+#define R8A7795_CLK_ZB3D2		22
+#define R8A7795_CLK_CR			23
+#define R8A7795_CLK_CRD2		24
+#define R8A7795_CLK_SD0H		25
+#define R8A7795_CLK_SD0			26
+#define R8A7795_CLK_SD1H		27
+#define R8A7795_CLK_SD1			28
+#define R8A7795_CLK_SD2H		29
+#define R8A7795_CLK_SD2			30
+#define R8A7795_CLK_SD3H		31
+#define R8A7795_CLK_SD3			32
+#define R8A7795_CLK_SSP2		33
+#define R8A7795_CLK_SSP1		34
+#define R8A7795_CLK_SSPRS		35
+#define R8A7795_CLK_RPC			36
+#define R8A7795_CLK_RPCD2		37
+#define R8A7795_CLK_MSO			38
+#define R8A7795_CLK_CANFD		39
+#define R8A7795_CLK_HDMI		40
+#define R8A7795_CLK_CSI0		41
+#define R8A7795_CLK_CSIREF		42
+#define R8A7795_CLK_CP			43
+#define R8A7795_CLK_CPEX		44
+#define R8A7795_CLK_R			45
+#define R8A7795_CLK_OSC			46
+
+#endif /* __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__ */
-- 
1.9.1

[PATCH v4 3/5] [RFC] clk: shmobile: div6: Extract cpg_div6_register()

[Geert Uytterhoeven]

Extract cpg_div6_register(), to allow registering div6 clocks from
another clock driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v4:
  - New.
---
 drivers/clk/shmobile/clk-div6.c | 119 +++++++++++++++++++++++++---------------
 drivers/clk/shmobile/clk-div6.h |   3 +
 2 files changed, 77 insertions(+), 45 deletions(-)
 create mode 100644 drivers/clk/shmobile/clk-div6.h

diff --git a/drivers/clk/shmobile/clk-div6.c b/drivers/clk/shmobile/clk-div6.c
index 57016ff9c585fc6e..5e8525fc60427229 100644
--- a/drivers/clk/shmobile/clk-div6.c
+++ b/drivers/clk/shmobile/clk-div6.c
@@ -18,6 +18,8 @@
 #include <linux/of_address.h>
 #include <linux/slab.h>
 
+#include "clk-div6.h"
+
 #define CPG_DIV6_CKSTP		BIT(8)
 #define CPG_DIV6_DIV(d)		((d) & 0x3f)
 #define CPG_DIV6_DIV_MASK	0x3f
@@ -172,60 +174,34 @@ static const struct clk_ops cpg_div6_clock_ops = {
 	.set_rate = cpg_div6_clock_set_rate,
 };
 
-static void __init cpg_div6_clock_init(struct device_node *np)
+struct clk * __init cpg_div6_register(const char *name,
+				      unsigned int num_parents,
+				      const char **parent_names,
+				      void __iomem *reg)
 {
-	unsigned int num_parents, valid_parents;
-	const char **parent_names;
+	unsigned int valid_parents;
 	struct clk_init_data init;
 	struct div6_clock *clock;
-	const char *clk_name = np->name;
 	struct clk *clk;
 	unsigned int i;
 
 	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
 	if (!clock)
-		return;
-
-	num_parents = of_clk_get_parent_count(np);
-	if (num_parents < 1) {
-		pr_err("%s: no parent found for %s DIV6 clock\n",
-		       __func__, np->name);
-		return;
-	}
+		return ERR_PTR(-ENOMEM);
 
 	clock->parents = kmalloc_array(num_parents, sizeof(*clock->parents),
-		GFP_KERNEL);
-	parent_names = kmalloc_array(num_parents, sizeof(*parent_names),
-				GFP_KERNEL);
-	if (!parent_names)
-		return;
+				       GFP_KERNEL);
+	if (!clock->parents)
+		return ERR_PTR(-ENOMEM);
 
-	/* Remap the clock register and read the divisor. Disabling the
-	 * clock overwrites the divisor, so we need to cache its value for the
-	 * enable operation.
-	 */
-	clock->reg = of_iomap(np, 0);
-	if (clock->reg == NULL) {
-		pr_err("%s: failed to map %s DIV6 clock register\n",
-		       __func__, np->name);
-		goto error;
-	}
+	clock->reg = reg;
 
+	/*
+	 * Read the divisor. Disabling the clock overwrites the divisor, so we
+	 * need to cache its value for the enable operation.
+	 */
 	clock->div = (clk_readl(clock->reg) & CPG_DIV6_DIV_MASK) + 1;
 
-	/* Parse the DT properties. */
-	of_property_read_string(np, "clock-output-names", &clk_name);
-
-	for (i = 0, valid_parents = 0; i < num_parents; i++) {
-		const char *name = of_clk_get_parent_name(np, i);
-
-		if (name) {
-			parent_names[valid_parents] = name;
-			clock->parents[valid_parents] = i;
-			valid_parents++;
-		}
-	}
-
 	switch (num_parents) {
 	case 1:
 		/* fixed parent clock */
@@ -243,12 +219,22 @@ static void __init cpg_div6_clock_init(struct device_node *np)
 		break;
 	default:
 		pr_err("%s: invalid number of parents for DIV6 clock %s\n",
-		       __func__, np->name);
+		       __func__, name);
+		clk = ERR_PTR(-EINVAL);
 		goto error;
 	}
 
+	/* Filter out invalid parents */
+	for (i = 0, valid_parents = 0; i < num_parents; i++) {
+		if (parent_names[i]) {
+			parent_names[valid_parents] = parent_names[i];
+			clock->parents[valid_parents] = i;
+			valid_parents++;
+		}
+	}
+
 	/* Register the clock. */
-	init.name = clk_name;
+	init.name = name;
 	init.ops = &cpg_div6_clock_ops;
 	init.flags = CLK_IS_BASIC;
 	init.parent_names = parent_names;
@@ -257,6 +243,50 @@ static void __init cpg_div6_clock_init(struct device_node *np)
 	clock->hw.init = &init;
 
 	clk = clk_register(NULL, &clock->hw);
+	if (!IS_ERR(clk))
+		return clk;
+
+error:
+	kfree(clock->parents);
+	kfree(clock);
+	return clk;
+}
+
+static void __init cpg_div6_clock_init(struct device_node *np)
+{
+	unsigned int num_parents;
+	const char **parent_names;
+	const char *clk_name = np->name;
+	void __iomem *reg;
+	struct clk *clk;
+	unsigned int i;
+
+	num_parents = of_clk_get_parent_count(np);
+	if (num_parents < 1) {
+		pr_err("%s: no parent found for %s DIV6 clock\n",
+		       __func__, np->name);
+		return;
+	}
+
+	parent_names = kmalloc_array(num_parents, sizeof(*parent_names),
+				GFP_KERNEL);
+	if (!parent_names)
+		return;
+
+	reg = of_iomap(np, 0);
+	if (reg == NULL) {
+		pr_err("%s: failed to map %s DIV6 clock register\n",
+		       __func__, np->name);
+		goto error;
+	}
+
+	/* Parse the DT properties. */
+	of_property_read_string(np, "clock-output-names", &clk_name);
+
+	for (i = 0; i < num_parents; i++)
+		parent_names[i] = of_clk_get_parent_name(np, i);
+
+	clk = cpg_div6_register(clk_name, num_parents, parent_names, reg);
 	if (IS_ERR(clk)) {
 		pr_err("%s: failed to register %s DIV6 clock (%ld)\n",
 		       __func__, np->name, PTR_ERR(clk));
@@ -269,9 +299,8 @@ static void __init cpg_div6_clock_init(struct device_node *np)
 	return;
 
 error:
-	if (clock->reg)
-		iounmap(clock->reg);
+	if (reg)
+		iounmap(reg);
 	kfree(parent_names);
-	kfree(clock);
 }
 CLK_OF_DECLARE(cpg_div6_clk, "renesas,cpg-div6-clock", cpg_div6_clock_init);
diff --git a/drivers/clk/shmobile/clk-div6.h b/drivers/clk/shmobile/clk-div6.h
new file mode 100644
index 0000000000000000..d19531f42953c83f
--- /dev/null
+++ b/drivers/clk/shmobile/clk-div6.h
@@ -0,0 +1,3 @@
+
+struct clk *cpg_div6_register(const char *name, unsigned int num_parents,
+			      const char **parent_names, void __iomem *reg);
-- 
1.9.1

[PATCH v4 4/5] [RFC] clk: shmobile: cpg-mssr: Add new CPG/MSSR driver core

[Geert Uytterhoeven]

Add the common core for the new Renesas Clock Pulse Generator / Module
Standby and Software Reset driver.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v4:
  - New.
---
 drivers/clk/shmobile/clk-cpg-mssr.c | 578 ++++++++++++++++++++++++++++++++++++
 drivers/clk/shmobile/clk-cpg-mssr.h | 118 ++++++++
 2 files changed, 696 insertions(+)
 create mode 100644 drivers/clk/shmobile/clk-cpg-mssr.c
 create mode 100644 drivers/clk/shmobile/clk-cpg-mssr.h

diff --git a/drivers/clk/shmobile/clk-cpg-mssr.c b/drivers/clk/shmobile/clk-cpg-mssr.c
new file mode 100644
index 0000000000000000..304af3b7dd09b6f7
--- /dev/null
+++ b/drivers/clk/shmobile/clk-cpg-mssr.c
@@ -0,0 +1,578 @@
+/*
+ * Renesas Clock Pulse Generator / Module Standby and Software Reset
+ *
+ * Copyright (C) 2015 Glider bvba
+ *
+ * Based on clk-mstp.c, clk-rcar-gen2.c, and clk-rcar-gen3.c
+ *
+ * Copyright (C) 2013 Ideas On Board SPRL
+ * Copyright (C) 2015 Renesas Electronics Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/init.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_domain.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+#include "clk-cpg-mssr.h"
+#include "clk-div6.h"
+
+
+/*
+ * Module Standby and Software Reset register offets.
+ *
+ * If the registers exist, these are valid for SH-Mobile, R-Mobile,
+ * R-Car Gen 2, and R-Car Gen 3.
+ * These are NOT valid for R-Car Gen1 and RZ/A1!
+ */
+
+/*
+ * Module Stop Status Register offsets
+ */
+
+static const u16 mstpsr[] = {
+	0x030, 0x038, 0x040, 0x048, 0x04C, 0x03C, 0x1C0, 0x1C4,
+	0x9A0, 0x9A4, 0x9A8, 0x9AC,
+};
+
+#define	MSTPSR(i)	mstpsr[i]
+
+
+/*
+ * System Module Stop Control Register offsets
+ */
+
+static const u16 smstpcr[] = {
+	0x130, 0x134, 0x138, 0x13C, 0x140, 0x144, 0x148, 0x14C,
+	0x990, 0x994, 0x998, 0x99C,
+};
+
+#define	SMSTPCR(i)	smstpcr[i]
+
+
+/*
+ * Software Reset Register offsets
+ */
+
+static const u16 srcr[] = {
+	0x0A0, 0x0A8, 0x0B0, 0x0B8, 0x0BC, 0x0C4, 0x1C8, 0x1CC,
+	0x920, 0x924, 0x928, 0x92C,
+};
+
+#define	SRCR(i)		srcr[i]
+
+
+/* Realtime Module Stop Control Register offsets */
+#define RMSTPCR(i)	(smstpcr[i] - 0x20)
+
+/* Modem Module Stop Control Register offsets (r8a73a4) */
+#define MMSTPCR(i)	(smstpcr[i] + 0x20)
+
+/* Software Reset Clearing Register offsets */
+#define	SRSTCLR(i)	(0x940 + (i) * 4)
+
+
+#define MSTP_MAX_REGS		ARRAY_SIZE(smstpcr)
+#define MSTP_MAX_CLOCKS		(MSTP_MAX_REGS * 32)
+
+
+/**
+ * Clock Pulse Generator / Module Standby and Software Reset Private Data
+ *
+ * @base: CPG/MSSR register block base address
+ * @mstp_lock: protects writes to SMSTPCR
+ */
+struct cpg_mssr_priv {
+	void __iomem *base;
+	spinlock_t mstp_lock;
+
+	/* Core and Module Clocks */
+	struct clk **clks;
+
+	/* Core Clocks */
+	unsigned int num_core_clks;
+	unsigned int last_dt_core_clk;
+
+	/* Module Clocks */
+	unsigned int num_mod_clks;
+
+	/* Core Clocks suitable for PM, in addition to the module clocks */
+	const unsigned int *core_pm_clks;
+	unsigned int num_core_pm_clks;
+};
+
+
+/**
+ * struct mstp_clock - MSTP gating clock
+ * @hw: handle between common and hardware-specific interfaces
+ * @index: MSTP clock number
+ * @priv: CPG/MSSR private data
+ */
+struct mstp_clock {
+	struct clk_hw hw;
+	u32 index;
+	struct cpg_mssr_priv *priv;
+};
+
+#define to_mstp_clock(_hw) container_of(_hw, struct mstp_clock, hw)
+
+static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable)
+{
+	struct mstp_clock *clock = to_mstp_clock(hw);
+	struct cpg_mssr_priv *priv = clock->priv;
+	unsigned int reg = clock->index / 32;
+	unsigned int bit = clock->index % 32;
+	u32 bitmask = BIT(bit);
+	unsigned long flags;
+	unsigned int i;
+	u32 value;
+
+	pr_debug("MSTP %u%02u/%pC %s\n", reg, bit, hw->clk,
+		 enable ? "ON" : "OFF");
+	spin_lock_irqsave(&priv->mstp_lock, flags);
+
+	value = clk_readl(priv->base + SMSTPCR(reg));
+	if (enable)
+		value &= ~bitmask;
+	else
+		value |= bitmask;
+	clk_writel(value, priv->base + SMSTPCR(reg));
+
+	spin_unlock_irqrestore(&priv->mstp_lock, flags);
+
+	if (!enable)
+		return 0;
+
+	for (i = 1000; i > 0; --i) {
+		if (!(clk_readl(priv->base + MSTPSR(reg)) &
+		      bitmask))
+			break;
+		cpu_relax();
+	}
+
+	if (!i) {
+		pr_err("%s: Failed to enable %p[%d]\n", __func__,
+		       priv->base + SMSTPCR(reg), bit);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int cpg_mstp_clock_enable(struct clk_hw *hw)
+{
+	return cpg_mstp_clock_endisable(hw, true);
+}
+
+static void cpg_mstp_clock_disable(struct clk_hw *hw)
+{
+	cpg_mstp_clock_endisable(hw, false);
+}
+
+static int cpg_mstp_clock_is_enabled(struct clk_hw *hw)
+{
+	struct mstp_clock *clock = to_mstp_clock(hw);
+	struct cpg_mssr_priv *priv = clock->priv;
+	u32 value;
+
+	value = clk_readl(priv->base + MSTPSR(clock->index / 32));
+
+	return !(value & BIT(clock->index % 32));
+}
+
+static const struct clk_ops cpg_mstp_clock_ops = {
+	.enable = cpg_mstp_clock_enable,
+	.disable = cpg_mstp_clock_disable,
+	.is_enabled = cpg_mstp_clock_is_enabled,
+};
+
+static
+struct clk *cpg_mssr_clk_src_twocell_get(struct of_phandle_args *clkspec,
+					 void *data)
+{
+	unsigned int clkidx = clkspec->args[1];
+	struct cpg_mssr_priv *priv = data;
+	unsigned int idx;
+	struct clk *clk;
+
+	switch (clkspec->args[0]) {
+	case CPG_CORE:
+		if (clkidx > priv->last_dt_core_clk) {
+			pr_err("%s: Invalid %s clock index %u\n", __func__,
+			       "core", clkidx);
+			return ERR_PTR(-EINVAL);
+		}
+		clk = priv->clks[clkidx];
+		break;
+
+	case CPG_MOD:
+		/* Translate from sparse base-100 to packed index space */
+		idx = clkidx - (clkidx / 100) * (100 - 32);
+		if (clkidx % 100 > 31 || idx >= priv->num_mod_clks) {
+			pr_err("%s: Invalid %s clock index %u\n", __func__,
+			       "module", clkidx);
+			return ERR_PTR(-EINVAL);
+		}
+		clk = priv->clks[priv->num_core_clks + idx];
+		break;
+
+	default:
+		pr_err("%s: Invalid CPG clock type %u\n", __func__,
+		       clkspec->args[0]);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (IS_ERR(clk))
+		pr_err("%s: Cannot get %s clock %u: %ld", __func__,
+		       clkspec->args[0] == CPG_CORE ? "core" : "module",
+		       clkidx, PTR_ERR(clk));
+	else
+		pr_debug("%s: clock (%u, %u) is %pC\n", __func__,
+			 clkspec->args[0], clkspec->args[1], clk);
+	return clk;
+}
+
+
+// FIXME Handle packing in a macro in cpg_mssr_info.mod_clks[]???
+static unsigned int __init mod_pack(unsigned int idx,
+				    struct cpg_mssr_priv *priv)
+{
+	WARN_ON(idx % 100 > 31);
+	idx -= (idx / 100) * (100 - 32);
+	WARN_ON(idx >= priv->num_mod_clks);
+	return idx;
+}
+
+static unsigned int __init id_to_idx(unsigned int id,
+				     struct cpg_mssr_priv *priv)
+{
+	unsigned int idx;
+
+	if (id < priv->num_core_clks) {
+		/* Core Clock */
+		pr_debug("%s: %u is a core clock\n", __func__, id);
+		return id;
+	}
+
+	/* Module Clock */
+	idx = priv->num_core_clks +
+	      mod_pack(id - priv->num_core_clks, priv);
+	pr_debug("%s: %u is module clock %u at index %u\n", __func__,
+		 id, id - priv->num_core_clks, idx);
+	return idx;
+}
+
+static void __init cpg_mssr_register_core_clk(struct device_node *np,
+					      const struct cpg_core_clk *core,
+					      const struct cpg_mssr_info *info,
+					      struct cpg_mssr_priv *priv)
+{
+	struct clk *clk = NULL, *parent;
+	unsigned int idx = core->id;
+	const char *parent_name;
+
+	pr_debug("Registering core clock %s id %u type %u\n", core->name, idx,
+		 core->type);
+	WARN_ON(idx >= priv->num_core_clks);
+	WARN_ON(PTR_ERR(priv->clks[idx]) != -ENOENT);
+
+	switch (core->type) {
+	/* Generic */
+	case CLK_TYPE_IN:
+		/* External Clock Input */
+		clk = of_clk_get_by_name(np, core->name);
+		break;
+
+	case CLK_TYPE_FF:
+		/* Fixed Factor Clock */
+		WARN_ON(core->parent >= priv->num_core_clks);
+		parent = priv->clks[core->parent];
+		if (IS_ERR(parent)) {
+			clk = parent;
+			goto fail;
+		}
+
+		parent_name = __clk_get_name(parent);
+		clk = clk_register_fixed_factor(NULL, core->name, parent_name,
+						0, core->mult, core->div);
+		break;
+
+	case CLK_TYPE_DIV6P1:
+		/* DIV6 Clock with 1 parent clock */
+		WARN_ON(core->parent >= priv->num_core_clks);
+		parent = priv->clks[core->parent];
+		if (IS_ERR(parent)) {
+			clk = parent;
+			goto fail;
+		}
+
+		parent_name = __clk_get_name(parent);
+		clk = cpg_div6_register(core->name, 1, &parent_name,
+					priv->base + core->offset);
+		break;
+
+	default:
+		if (info->cpg_clk_register)
+			clk = info->cpg_clk_register(core, info, priv->clks,
+						     priv->base);
+		else
+			pr_err("%s: Unsupported clock type %u\n", __func__,
+			       core->type);
+		break;
+	}
+
+	if (IS_ERR_OR_NULL(clk))
+		goto fail;
+
+	pr_debug("%s: Registered core clock %pC\n", __func__, clk);
+	priv->clks[idx] = clk;
+	return;
+
+fail:
+	pr_err("%s: Failed to register core clock %u: %ld\n", __func__, idx,
+	       PTR_ERR(clk));
+}
+
+static void __init cpg_mssr_register_mod_clk(const struct mssr_mod_clk *mod,
+					     const struct cpg_mssr_info *info,
+					     struct cpg_mssr_priv *priv)
+{
+	unsigned int mod_idx, idx, parent_idx;
+	struct mstp_clock *clock = NULL;
+	struct clk_init_data init;
+	struct clk *parent, *clk;
+	const char *parent_name;
+	unsigned int i;
+
+	pr_debug("Registering module clock %s id %u parent %u\n", mod->name,
+		 mod->id, mod->parent);
+
+	// FIXME Incorporate offset/packing in cpg_mssr_info.mod_clks[]?
+	mod_idx = mod_pack(mod->id, priv);
+	WARN_ON(mod_idx >= priv->num_mod_clks);
+	idx = priv->num_core_clks + mod_idx;
+	parent_idx = id_to_idx(mod->parent, priv);
+	WARN_ON(parent_idx >= priv->num_core_clks + priv->num_mod_clks);
+	WARN_ON(PTR_ERR(priv->clks[idx]) != -ENOENT);
+
+	parent = priv->clks[parent_idx];
+	if (IS_ERR(parent)) {
+		clk = parent;
+		goto fail;
+	}
+
+	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
+	if (!clock) {
+		clk = ERR_PTR(-ENOMEM);
+		goto fail;
+	}
+
+	init.name = mod->name;
+	init.ops = &cpg_mstp_clock_ops;
+	init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
+	for (i = 0; i < info->num_crit_mod_clks; i++)
+		if (mod->id == info->crit_mod_clks[i]) {
+#ifdef CLK_ENABLE_HAND_OFF // FIXME Not yet in -next
+			pr_debug("%s: MSTP %s setting CLK_ENABLE_HAND_OFF\n",
+				 __func__, mod->name);
+			init.flags |= CLK_ENABLE_HAND_OFF;
+			break;
+#else
+			pr_debug("%s: Ignoring MSTP %s to prevent disabling\n",
+				 __func__, mod->name);
+			return;
+#endif
+		}
+
+	parent_name = __clk_get_name(parent);
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	clock->index = mod_idx;
+	clock->priv = priv;
+	clock->hw.init = &init;
+
+	clk = clk_register(NULL, &clock->hw);
+	if (IS_ERR(clk))
+		goto fail;
+
+	pr_debug("%s: Created module clock %pC\n", __func__, clk);
+	priv->clks[idx] = clk;
+	return;
+
+fail:
+	pr_err("%s: Failed to create module clock %u: %ld\n", __func__,
+	       mod->id, PTR_ERR(clk));
+	kfree(clock);
+}
+
+
+#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
+static const struct of_device_id cpg_mssr_match[] = {
+	// FIXME Reuse the string as matched by the driver
+	{ .compatible = "renesas,r8a7791-cpg-mssr", },
+	{ .compatible = "renesas,r8a7795-cpg-mssr", },
+	{ /* sentinel */ }
+};
+
+static bool cpg_mssr_is_pm_clk(const struct of_phandle_args *clkspec)
+{
+	if (!of_match_node(cpg_mssr_match, clkspec->np))
+		return false;
+
+	if (clkspec->args_count != 2)
+		return false;
+
+	switch (clkspec->args[0]) {
+	case CPG_CORE:
+		// FIXME Match against cpg_mssr_info.core_pm_clks[]
+		return false;
+
+	case CPG_MOD:
+		return true;
+
+	default:
+		return false;
+	}
+}
+
+static int cpg_mssr_attach_dev(struct generic_pm_domain *domain,
+			       struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct of_phandle_args clkspec;
+	struct clk *clk;
+	int i = 0;
+	int error;
+
+	while (!of_parse_phandle_with_args(np, "clocks", "#clock-cells", i,
+					   &clkspec)) {
+		if (cpg_mssr_is_pm_clk(&clkspec))
+			goto found;
+
+		of_node_put(clkspec.np);
+		i++;
+	}
+
+	return 0;
+
+found:
+	clk = of_clk_get_from_provider(&clkspec);
+	of_node_put(clkspec.np);
+
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	error = pm_clk_create(dev);
+	if (error) {
+		dev_err(dev, "pm_clk_create failed %d\n", error);
+		goto fail_put;
+	}
+
+	error = pm_clk_add_clk(dev, clk);
+	if (error) {
+		dev_err(dev, "pm_clk_add_clk %pC failed %d\n", clk, error);
+		goto fail_destroy;
+	}
+
+	return 0;
+
+fail_destroy:
+	pm_clk_destroy(dev);
+fail_put:
+	clk_put(clk);
+	return error;
+}
+
+static void cpg_mssr_detach_dev(struct generic_pm_domain *domain,
+				struct device *dev)
+{
+	if (!list_empty(&dev->power.subsys_data->clock_list))
+		pm_clk_destroy(dev);
+}
+
+static void __init cpg_mssr_add_clk_domain(struct device_node *np)
+{
+	struct generic_pm_domain *pd;
+	u32 ncells;
+
+	if (of_property_read_u32(np, "#power-domain-cells", &ncells)) {
+		pr_warn("%s lacks #power-domain-cells\n", np->full_name);
+		return;
+	}
+
+	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return;
+
+	pd->name = np->name;
+
+	pd->flags = GENPD_FLAG_PM_CLK;
+	pm_genpd_init(pd, &simple_qos_governor, false);
+	pd->attach_dev = cpg_mssr_attach_dev;
+	pd->detach_dev = cpg_mssr_detach_dev;
+
+	of_genpd_add_provider_simple(np, pd);
+}
+#else
+static inline void cpg_mssr_add_clk_domain(struct device_node *np) {}
+#endif /* !CONFIG_PM_GENERIC_DOMAINS_OF */
+
+
+void __init cpg_mssr_probe(struct device_node *np,
+			   const struct cpg_mssr_info *info)
+{
+	struct cpg_mssr_priv *priv;
+	unsigned int nclks, i;
+	struct clk **clks;
+
+	pr_debug("%s: Using %ps\n", __func__, info);
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return;
+
+	priv->base = of_iomap(np, 0);
+	if (!priv->base)
+		return;
+
+	nclks = info->num_total_core_clks + info->num_hw_mod_clks;
+	clks = kmalloc_array(nclks, sizeof(*clks), GFP_KERNEL);
+	if (!clks)
+		return;
+
+	spin_lock_init(&priv->mstp_lock);
+	priv->clks = clks;
+	priv->num_core_clks = info->num_total_core_clks;
+	priv->last_dt_core_clk = info->last_dt_core_clk;
+	priv->num_mod_clks = info->num_hw_mod_clks;
+
+	for (i = 0; i < nclks; i++)
+		clks[i] = ERR_PTR(-ENOENT);
+
+	for (i = 0; i < info->num_core_clks; i++)
+		cpg_mssr_register_core_clk(np, &info->core_clks[i], info,
+					   priv);
+
+	for (i = 0; i < info->num_mod_clks; i++)
+		cpg_mssr_register_mod_clk(&info->mod_clks[i], info, priv);
+
+	of_clk_add_provider(np, cpg_mssr_clk_src_twocell_get, priv);
+
+	cpg_mssr_add_clk_domain(np);
+}
+
+MODULE_DESCRIPTION("Renesas CPG/MSSR Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/shmobile/clk-cpg-mssr.h b/drivers/clk/shmobile/clk-cpg-mssr.h
new file mode 100644
index 0000000000000000..55100f0998265f45
--- /dev/null
+++ b/drivers/clk/shmobile/clk-cpg-mssr.h
@@ -0,0 +1,118 @@
+/*
+ * Renesas Clock Pulse Generator / Module Standby and Software Reset
+ *
+ * Copyright (C) 2015 Glider bvba
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+
+    /*
+     * Definitions of CPG Core Clocks
+     *
+     * These include:
+     *   - Clock outputs exported to DT
+     *   - External input clocks
+     *   - Internal CPG clocks
+     */
+
+struct cpg_core_clk {
+	/* Common */
+	const char *name;
+	unsigned int id;
+	unsigned int type;
+	/* Depending on type */
+	unsigned int parent;	/* Core Clocks only */
+	unsigned int div;
+	unsigned int mult;
+	unsigned int offset;
+};
+
+enum clk_types {
+	/* Generic */
+	CLK_TYPE_IN,		/* External Clock Input */
+	CLK_TYPE_FF,		/* Fixed Factor Clock */
+	CLK_TYPE_DIV6P1,	/* DIV6 Clock with 1 parent clock */
+
+	/* Custom definitions start here */
+	CLK_TYPE_CUSTOM,
+};
+
+#define DEF_TYPE(_name, _id, _type...)	\
+	{ .name = _name, .id = _id, .type = _type }
+#define DEF_BASE(_name, _id, _type, _parent...)	\
+	DEF_TYPE(_name, _id, _type, .parent = _parent)
+
+#define DEF_INPUT(_name, _id) \
+	DEF_TYPE(_name, _id, CLK_TYPE_IN)
+#define DEF_FIXED(_name, _id, _parent, _div, _mult)	\
+	DEF_BASE(_name, _id, CLK_TYPE_FF, _parent, .div = _div, .mult = _mult)
+#define DEF_DIV6P1(_name, _id, _parent, _offset)	\
+	DEF_BASE(_name, _id, CLK_TYPE_DIV6P1, _parent, .offset = _offset)
+
+
+    /*
+     * Definitions of Module Clocks
+     */
+
+struct mssr_mod_clk {
+	const char *name;
+	unsigned int id;
+	unsigned int parent;	/* Add MOD_CLK_BASE for Module Clocks */
+};
+
+
+struct device_node;
+
+    /*
+     * SoC-specific CPG/MSSR Description
+     *
+     * @core_clks: Array of Core Clock definitions
+     * @num_core_clks: Number of entries in core_clks[]
+     * @last_dt_core_clk: ID of the last Core Clock exported to DT
+     * @num_total_core_clks: Total number of Core Clocks (exported + internal)
+     *
+     * @mod_clks: Array of Module Clock definitions
+     * @num_mod_clks: Number of entries in mod_clks[]
+     * @num_hw_mod_clks: Number of module clocks supported by the hardware
+     *
+     * @crit_mod_clks: Array with Module Clock IDs of critical clocks that
+     *                 should not be disabled without a knowledgeable driver
+     * @num_crit_mod_clks: Number of entries in crit_mod_clks[]
+     *
+     * @core_pm_clks: Array with IDs of Core Clocks that are suitable for Power
+     *                Management, in addition to Module Clocks
+     * @num_core_pm_clks: Number of entries in core_pm_clks[]
+     *
+     * @cpg_clk_register: Optional callback to handle special Core Clock types
+     */
+
+struct cpg_mssr_info {
+	/* Core Clocks */
+	const struct cpg_core_clk *core_clks;
+	unsigned int num_core_clks;
+	unsigned int last_dt_core_clk;
+	unsigned int num_total_core_clks;
+
+	/* Module Clocks */
+	const struct mssr_mod_clk *mod_clks;
+	unsigned int num_mod_clks;
+	unsigned int num_hw_mod_clks;
+
+	/* Critical Module Clocks that should not be disabled */
+	const unsigned int *crit_mod_clks;
+	unsigned int num_crit_mod_clks;
+
+	/* Core Clocks suitable for PM, in addition to the module clocks */
+	const unsigned int *core_pm_clks;
+	unsigned int num_core_pm_clks;
+
+	/* Callbacks */
+	struct clk *(*cpg_clk_register)(const struct cpg_core_clk *core,
+					const struct cpg_mssr_info *info,
+					struct clk **clks, void __iomem *base);
+};
+
+void cpg_mssr_probe(struct device_node *np, const struct cpg_mssr_info *info);
-- 
1.9.1

[PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Geert Uytterhoeven]

Add a new R-Car H3 Clock Pulse Generator / Module Standby and Software
Reset driver, using the new CPG/MSSR driver core.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v4:
  - New.
---
 drivers/clk/shmobile/Makefile               |   3 +-
 drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c | 373 ++++++++++++++++++++++++++++
 2 files changed, 375 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c

diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
index 77b70b070e396a65..bbeb4d4ff715ff74 100644
--- a/drivers/clk/shmobile/Makefile
+++ b/drivers/clk/shmobile/Makefile
@@ -8,5 +8,6 @@ obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o clk-mstp.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7791)		+= clk-rcar-gen2.o clk-mstp.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7793)		+= clk-rcar-gen2.o clk-mstp.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7794)		+= clk-rcar-gen2.o clk-mstp.o clk-div6.o
-obj-$(CONFIG_ARCH_R8A7795)		+= clk-rcar-gen3.o clk-mssr.o clk-div6.o
+obj-$(CONFIG_ARCH_R8A7795)		+= clk-cpg-mssr.o \
+					   clk-r8a7795-cpg-mssr.o clk-div6.o
 obj-$(CONFIG_ARCH_SH73A0)		+= clk-sh73a0.o clk-mstp.o clk-div6.o
diff --git a/drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c b/drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c
new file mode 100644
index 0000000000000000..18444a13e5722e2b
--- /dev/null
+++ b/drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c
@@ -0,0 +1,373 @@
+/*
+ * r8a7795 Clock Pulse Generator / Module Standby and Software Reset
+ *
+ * Copyright (C) 2015 Glider bvba
+ *
+ * Based on clk-rcar-gen3.c
+ *
+ * Copyright (C) 2015 Renesas Electronics Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include <linux/bug.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/r8a7795-cpg-mssr.h>
+
+#include "clk-cpg-mssr.h"
+
+
+enum clk_ids {
+	/* Core Clock Outputs exported to DT */
+	LAST_DT_CORE_CLOCK = R8A7795_CLK_OSC,
+
+	/* External Input Clocks */
+	CLK_EXTAL,
+	CLK_EXTALR,
+
+	/* Internal Core Clocks */
+	CLK_MAIN,
+	CLK_PLL0,
+	CLK_PLL1,
+	CLK_PLL2,
+	CLK_PLL3,
+	CLK_PLL4,
+	CLK_PLL1_DIV2,
+	CLK_PLL1_DIV4,
+	CLK_S0,
+	CLK_S1,
+	CLK_S2,
+	CLK_S3,
+	CLK_SDSRC,
+	CLK_SSPSRC,
+
+	/* Module Clocks */
+	MOD_CLK_BASE
+};
+
+enum r8a7795_clk_types {
+	CLK_TYPE_GEN3_MAIN = CLK_TYPE_CUSTOM,
+	CLK_TYPE_GEN3_PLL0,
+	CLK_TYPE_GEN3_PLL1,
+	CLK_TYPE_GEN3_PLL2,
+	CLK_TYPE_GEN3_PLL3,
+	CLK_TYPE_GEN3_PLL4,
+};
+
+static const struct cpg_core_clk r8a7795_core_clks[] __initconst = {
+	/* External Clock Inputs */
+	DEF_INPUT("extal", CLK_EXTAL),
+	DEF_INPUT("extalr", CLK_EXTALR),
+
+	/* Internal Core Clocks */
+	DEF_BASE(".main",       CLK_MAIN, CLK_TYPE_GEN3_MAIN, CLK_EXTAL),
+	DEF_BASE(".pll0",       CLK_PLL0, CLK_TYPE_GEN3_PLL0, CLK_MAIN),
+	DEF_BASE(".pll1",       CLK_PLL1, CLK_TYPE_GEN3_PLL1, CLK_MAIN),
+	DEF_BASE(".pll2",       CLK_PLL2, CLK_TYPE_GEN3_PLL2, CLK_MAIN),
+	DEF_BASE(".pll3",       CLK_PLL3, CLK_TYPE_GEN3_PLL3, CLK_MAIN),
+	DEF_BASE(".pll4",       CLK_PLL4, CLK_TYPE_GEN3_PLL4, CLK_MAIN),
+
+	DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2,     CLK_PLL1,       2, 1),
+	DEF_FIXED(".pll1_div4", CLK_PLL1_DIV4,     CLK_PLL1_DIV2,  2, 1),
+	DEF_FIXED(".s0",        CLK_S0,            CLK_PLL1_DIV2,  2, 1),
+	DEF_FIXED(".s1",        CLK_S1,            CLK_PLL1_DIV2,  3, 1),
+	DEF_FIXED(".s2",        CLK_S2,            CLK_PLL1_DIV2,  4, 1),
+	DEF_FIXED(".s3",        CLK_S3,            CLK_PLL1_DIV2,  6, 1),
+
+	/* Core Clock Outputs */
+	DEF_FIXED("ztr",        R8A7795_CLK_ZTR,   CLK_PLL1_DIV2,  6, 1),
+	DEF_FIXED("ztrd2",      R8A7795_CLK_ZTRD2, CLK_PLL1_DIV2, 12, 1),
+	DEF_FIXED("zt",         R8A7795_CLK_ZT,    CLK_PLL1_DIV2,  4, 1),
+	DEF_FIXED("zx",         R8A7795_CLK_ZX,    CLK_PLL1_DIV2,  2, 1),
+	DEF_FIXED("s0d1",       R8A7795_CLK_S0D1,  CLK_S0,         1, 1),
+	DEF_FIXED("s0d4",       R8A7795_CLK_S0D4,  CLK_S0,         4, 1),
+	DEF_FIXED("s1d1",       R8A7795_CLK_S1D1,  CLK_S1,         1, 1),
+	DEF_FIXED("s1d2",       R8A7795_CLK_S1D2,  CLK_S1,         2, 1),
+	DEF_FIXED("s1d4",       R8A7795_CLK_S1D4,  CLK_S1,         4, 1),
+	DEF_FIXED("s2d1",       R8A7795_CLK_S2D1,  CLK_S2,         1, 1),
+	DEF_FIXED("s2d2",       R8A7795_CLK_S2D2,  CLK_S2,         2, 1),
+	DEF_FIXED("s2d4",       R8A7795_CLK_S2D4,  CLK_S2,         4, 1),
+	DEF_FIXED("s3d1",       R8A7795_CLK_S3D1,  CLK_S3,         1, 1),
+	DEF_FIXED("s3d2",       R8A7795_CLK_S3D2,  CLK_S3,         2, 1),
+	DEF_FIXED("s3d4",       R8A7795_CLK_S3D4,  CLK_S3,         4, 1),
+	DEF_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48, 1),
+	DEF_FIXED("cp",         R8A7795_CLK_CP,    CLK_EXTAL,      2, 1),
+	DEF_DIV6P1("mso",       R8A7795_CLK_MSO,   CLK_PLL1_DIV4, 0x014),
+	DEF_DIV6P1("hdmi",      R8A7795_CLK_HDMI,  CLK_PLL1_DIV2, 0x250),
+};
+
+static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
+	{ "scif5",		202,	R8A7795_CLK_S3D4	},
+	{ "scif4",		203,	R8A7795_CLK_S3D4	},
+	{ "scif3",		204,	R8A7795_CLK_S3D4	},
+	{ "scif1",		206,	R8A7795_CLK_S3D4	},
+	{ "scif0",		207,	R8A7795_CLK_S3D4	},
+	{ "msiof3",		208,	R8A7795_CLK_MSO		},
+	{ "msiof2",		209,	R8A7795_CLK_MSO		},
+	{ "msiof1",		210,	R8A7795_CLK_MSO		},
+	{ "msiof0",		211,	R8A7795_CLK_MSO		},
+	{ "sys-dmac2",		217,	R8A7795_CLK_S3D1	},
+	{ "sys-dmac1",		218,	R8A7795_CLK_S3D1	},
+	{ "sys-dmac0",		219,	R8A7795_CLK_S3D1	},
+	{ "scif2",		310,	R8A7795_CLK_S3D4	},
+	{ "intc-ap",		408,	R8A7795_CLK_S3D1	},
+	{ "audmac0",		502,	R8A7795_CLK_S3D4	},
+	{ "audmac1",		501,	R8A7795_CLK_S3D4	},
+	{ "hscif4",		516,	R8A7795_CLK_S3D1	},
+	{ "hscif3",		517,	R8A7795_CLK_S3D1	},
+	{ "hscif2",		518,	R8A7795_CLK_S3D1	},
+	{ "hscif1",		519,	R8A7795_CLK_S3D1	},
+	{ "hscif0",		520,	R8A7795_CLK_S3D1	},
+	{ "vspd3",		620,	R8A7795_CLK_S2D1	},
+	{ "vspd2",		621,	R8A7795_CLK_S2D1	},
+	{ "vspd1",		622,	R8A7795_CLK_S2D1	},
+	{ "vspd0",		623,	R8A7795_CLK_S2D1	},
+	{ "vspbc",		624,	R8A7795_CLK_S2D1	},
+	{ "vspbd",		626,	R8A7795_CLK_S2D1	},
+	{ "vspi2",		629,	R8A7795_CLK_S2D1	},
+	{ "vspi1",		630,	R8A7795_CLK_S2D1	},
+	{ "vspi0",		631,	R8A7795_CLK_S2D1	},
+	{ "ehci2",		701,	R8A7795_CLK_S3D4	},
+	{ "ehci1",		702,	R8A7795_CLK_S3D4	},
+	{ "ehci0",		703,	R8A7795_CLK_S3D4	},
+	{ "hsusb",		704,	R8A7795_CLK_S3D4	},
+	{ "du3",		721,	R8A7795_CLK_S2D1	},
+	{ "du2",		722,	R8A7795_CLK_S2D1	},
+	{ "du1",		723,	R8A7795_CLK_S2D1	},
+	{ "du0",		724,	R8A7795_CLK_S2D1	},
+	{ "hdmi1",		728,	R8A7795_CLK_HDMI	},
+	{ "hdmi0",		729,	R8A7795_CLK_HDMI	},
+	{ "etheravb",		812,	R8A7795_CLK_S3D2	},
+	{ "gpio7",		905,	R8A7795_CLK_CP		},
+	{ "gpio6",		906,	R8A7795_CLK_CP		},
+	{ "gpio5",		907,	R8A7795_CLK_CP		},
+	{ "gpio4",		908,	R8A7795_CLK_CP		},
+	{ "gpio3",		909,	R8A7795_CLK_CP		},
+	{ "gpio2",		910,	R8A7795_CLK_CP		},
+	{ "gpio1",		911,	R8A7795_CLK_CP		},
+	{ "gpio0",		912,	R8A7795_CLK_CP		},
+	{ "i2c6",		918,	R8A7795_CLK_S3D2	},
+	{ "i2c5",		919,	R8A7795_CLK_S3D2	},
+	{ "i2c4",		927,	R8A7795_CLK_S3D2	},
+	{ "i2c3",		928,	R8A7795_CLK_S3D2	},
+	{ "i2c2",		929,	R8A7795_CLK_S3D2	},
+	{ "i2c1",		930,	R8A7795_CLK_S3D2	},
+	{ "i2c0",		931,	R8A7795_CLK_S3D2	},
+	{ "ssi-all",		1005,	R8A7795_CLK_S3D4	},
+	{ "ssi9",		1006,	MOD_CLK_BASE + 1005	},
+	{ "ssi8",		1007,	MOD_CLK_BASE + 1005	},
+	{ "ssi7",		1008,	MOD_CLK_BASE + 1005	},
+	{ "ssi6",		1009,	MOD_CLK_BASE + 1005	},
+	{ "ssi5",		1010,	MOD_CLK_BASE + 1005	},
+	{ "ssi4",		1011,	MOD_CLK_BASE + 1005	},
+	{ "ssi3",		1012,	MOD_CLK_BASE + 1005	},
+	{ "ssi2",		1013,	MOD_CLK_BASE + 1005	},
+	{ "ssi1",		1014,	MOD_CLK_BASE + 1005	},
+	{ "ssi0",		1015,	MOD_CLK_BASE + 1005	},
+	{ "scu-all",		1017,	R8A7795_CLK_S3D4	},
+	{ "scu-dvc1",		1018,	MOD_CLK_BASE + 1017	},
+	{ "scu-dvc0",		1019,	MOD_CLK_BASE + 1017	},
+	{ "scu-ctu1-mix1",	1020,	MOD_CLK_BASE + 1017	},
+	{ "scu-ctu0-mix0",	1021,	MOD_CLK_BASE + 1017	},
+	{ "scu-src9",		1022,	MOD_CLK_BASE + 1017	},
+	{ "scu-src8",		1023,	MOD_CLK_BASE + 1017	},
+	{ "scu-src7",		1024,	MOD_CLK_BASE + 1017	},
+	{ "scu-src6",		1025,	MOD_CLK_BASE + 1017	},
+	{ "scu-src5",		1026,	MOD_CLK_BASE + 1017	},
+	{ "scu-src4",		1027,	MOD_CLK_BASE + 1017	},
+	{ "scu-src3",		1028,	MOD_CLK_BASE + 1017	},
+	{ "scu-src2",		1029,	MOD_CLK_BASE + 1017	},
+	{ "scu-src1",		1030,	MOD_CLK_BASE + 1017	},
+	{ "scu-src0",		1031,	MOD_CLK_BASE + 1017	},
+};
+
+static const unsigned int r8a7795_crit_mod_clks[] __initconst = {
+	408,	/* INTC-AP (GIC) */
+};
+
+
+#define CPG_PLL0CR	0x00d8
+#define CPG_PLL2CR	0x002c
+
+/*
+ * CPG Clock Data
+ */
+
+/*
+ *   MD		EXTAL		PLL0	PLL1	PLL2	PLL3	PLL4
+ * 14 13 19 17	(MHz)
+ *-------------------------------------------------------------------
+ * 0  0  0  0	16.66 x 1	x180	x192	x144	x192	x144
+ * 0  0  0  1	16.66 x 1	x180	x192	x144	x128	x144
+ * 0  0  1  0	Prohibited setting
+ * 0  0  1  1	16.66 x 1	x180	x192	x144	x192	x144
+ * 0  1  0  0	20    x 1	x150	x156	x120	x156	x120
+ * 0  1  0  1	20    x 1	x150	x156	x120	x106	x120
+ * 0  1  1  0	Prohibited setting
+ * 0  1  1  1	20    x 1	x150	x156	x120	x156	x120
+ * 1  0  0  0	25    x 1	x120	x128	x96	x128	x96
+ * 1  0  0  1	25    x 1	x120	x128	x96	x84	x96
+ * 1  0  1  0	Prohibited setting
+ * 1  0  1  1	25    x 1	x120	x128	x96	x128	x96
+ * 1  1  0  0	33.33 / 2	x180	x192	x144	x192	x144
+ * 1  1  0  1	33.33 / 2	x180	x192	x144	x128	x144
+ * 1  1  1  0	Prohibited setting
+ * 1  1  1  1	33.33 / 2	x180	x192	x144	x192	x144
+ */
+#define CPG_PLL_CONFIG_INDEX(md)	((((md) & BIT(14)) >> 11) | \
+					 (((md) & BIT(13)) >> 11) | \
+					 (((md) & BIT(19)) >> 18) | \
+					 (((md) & BIT(17)) >> 17))
+
+struct cpg_pll_config {
+	unsigned int extal_div;
+	unsigned int pll1_mult;
+	unsigned int pll3_mult;
+	unsigned int pll4_mult;
+};
+
+static const struct cpg_pll_config cpg_pll_configs[16] __initconst = {
+/* EXTAL div	PLL1	PLL3	PLL4 */
+	{ 1,	192,	192,	144, },
+	{ 1,	192,	128,	144, },
+	{ 0,	0,	0,	0,   }, /* Prohibited setting */
+	{ 1,	192,	192,	144, },
+	{ 1,	156,	156,	120, },
+	{ 1,	156,	106,	120, },
+	{ 0,	0,	0,	0,   }, /* Prohibited setting */
+	{ 1,	156,	156,	120, },
+	{ 1,	128,	128,	96,  },
+	{ 1,	128,	84,	96,  },
+	{ 0,	0,	0,	0,   }, /* Prohibited setting */
+	{ 1,	128,	128,	96,  },
+	{ 2,	192,	192,	144, },
+	{ 2,	192,	128,	144, },
+	{ 0,	0,	0,	0,   }, /* Prohibited setting */
+	{ 2,	192,	192,	144, },
+};
+
+static const struct cpg_pll_config *cpg_pll_config __initdata;
+
+static
+struct clk * __init r8a7795_cpg_clk_register(const struct cpg_core_clk *core,
+					     const struct cpg_mssr_info *info,
+					     struct clk **clks,
+					     void __iomem *base)
+{
+	unsigned int idx = core->id;
+	const struct clk *parent;
+	unsigned int mult = 1;
+	unsigned int div = 1;
+	u32 value;
+
+	pr_debug("Registering r8a7795 core clock %s id %u type %u\n",
+		 core->name, idx, core->type);
+	WARN_ON(idx >= info->num_total_core_clks);
+	WARN_ON(PTR_ERR(clks[idx]) != -ENOENT);
+
+	parent = clks[core->parent];
+	if (IS_ERR(parent))
+		return ERR_CAST(parent);
+
+	switch (core->type) {
+	case CLK_TYPE_GEN3_MAIN:
+		div = cpg_pll_config->extal_div;
+		break;
+
+	case CLK_TYPE_GEN3_PLL0:
+		/*
+		 * PLL0 is a configurable multiplier clock. Register it as a
+		 * fixed factor clock for now as there's no generic multiplier
+		 * clock implementation and we currently have no need to change
+		 * the multiplier value.
+		 */
+		value = readl(base + CPG_PLL0CR);
+		mult = ((value >> 24) & 0x3f) + 1;
+		break;
+
+	case CLK_TYPE_GEN3_PLL1:
+		mult = cpg_pll_config->pll1_mult;
+		break;
+
+	case CLK_TYPE_GEN3_PLL2:
+		/*
+		 * PLL2 is a configurable multiplier clock. Register it as a
+		 * fixed factor clock for now as there's no generic multiplier
+		 * clock implementation and we currently have no need to change
+		 * the multiplier value.
+		 */
+		value = readl(base + CPG_PLL2CR);
+		mult = ((value >> 24) & 0x3f) + 1;
+		break;
+
+	case CLK_TYPE_GEN3_PLL3:
+		mult = cpg_pll_config->pll3_mult;
+		break;
+
+	case CLK_TYPE_GEN3_PLL4:
+		mult = cpg_pll_config->pll4_mult;
+		break;
+
+	default:
+		pr_err("%s: Unsupported clock type %u\n", __func__, core->type);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return clk_register_fixed_factor(NULL, core->name,
+					 __clk_get_name(parent), 0, mult, div);
+}
+
+static const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
+	/* Core Clocks */
+	.core_clks = r8a7795_core_clks,
+	.num_core_clks = ARRAY_SIZE(r8a7795_core_clks),
+	.last_dt_core_clk = LAST_DT_CORE_CLOCK,
+	.num_total_core_clks = MOD_CLK_BASE,
+
+	/* Module Clocks */
+	.mod_clks = r8a7795_mod_clks,
+	.num_mod_clks = ARRAY_SIZE(r8a7795_mod_clks),
+	.num_hw_mod_clks = 12 * 32,
+
+	/* Critical Module Clocks */
+	.crit_mod_clks = r8a7795_crit_mod_clks,
+	.num_crit_mod_clks = ARRAY_SIZE(r8a7795_crit_mod_clks),
+
+	/* Callbacks */
+	.cpg_clk_register = r8a7795_cpg_clk_register,
+};
+
+static void __init r8a7795_cpg_mssr_init(struct device_node *np)
+{
+	struct regmap *regmap;
+	u32 reg, cpg_mode;
+
+	regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr");
+	if (IS_ERR(regmap) ||
+	    of_property_read_u32_index(np, "renesas,modemr", 1, &reg) ||
+	    regmap_read(regmap, reg, &cpg_mode)) {
+		pr_err("%s: failed to parse renesas,modemr\n", np->full_name);
+		return;
+	}
+
+	cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
+	if (!cpg_pll_config->extal_div) {
+		pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n",
+		       __func__, cpg_mode);
+		return;
+	}
+
+	cpg_mssr_probe(np, &r8a7795_cpg_mssr_info);
+}
+CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr",
+	       r8a7795_cpg_mssr_init);
-- 
1.9.1

Re: [PATCH v4 2/5] [RFC] clk: shmobile: Add r8a7795 CPG Core Clock Definitions

[Geert Uytterhoeven]

On Fri, Oct 16, 2015 at 2:49 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> Add all R-Car H3 CPG Core Clock Outputs defined in the datasheet.

Cfr. Table 8.2a ("List of Clocks [R-Car H3]").

> --- /dev/null
> +++ b/include/dt-bindings/clock/r8a7795-cpg-mssr.h
> @@ -0,0 +1,63 @@
> +/*
> + * Copyright (C) 2015 Renesas Electronics Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#ifndef __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__
> +#define __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__
> +
> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
> +
> +/* r8a7795 CPG Core Clocks */
> +#define R8A7795_CLK_Z                  0
> +#define R8A7795_CLK_Z2                 1
> +#define R8A7795_CLK_ZR                 2
> +#define R8A7795_CLK_ZG                 3
> +#define R8A7795_CLK_ZTR                        4
> +#define R8A7795_CLK_ZTRD2              5
> +#define R8A7795_CLK_ZT                 6
> +#define R8A7795_CLK_ZX                 7
> +#define R8A7795_CLK_S0D1               8
> +#define R8A7795_CLK_S0D4               9
> +#define R8A7795_CLK_S1D1               10
> +#define R8A7795_CLK_S1D2               11
> +#define R8A7795_CLK_S1D4               12
> +#define R8A7795_CLK_S2D1               13
> +#define R8A7795_CLK_S2D2               14
> +#define R8A7795_CLK_S2D4               15
> +#define R8A7795_CLK_S3D1               16
> +#define R8A7795_CLK_S3D2               17
> +#define R8A7795_CLK_S3D4               18

Please note that I deliberately left out S[0-3], as they're used as
internal clock
sources only.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Michael Turquette]

Hi Geert,

Quoting Geert Uytterhoeven (2015-10-16 05:49:16)
> On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
> Generator) and MSSR (Module Standby and Software Reset) blocks are
> intimately connected, and share the same register block.
> 
> Hence it makes sense to describe these two blocks using a
> single device node in DT, instead of using a hierarchical structure with
> multiple nodes, using a mix of generic and SoC-specific bindings.
> 
> These new DT bindings are intended to replace the existing DT bindings
> for CPG core clocks ("renesas,*-cpg-clocks", "renesas,cpg-div6-clock")
> and module clocks ("renesas,*-mstp-clocks"), at least for new SoCs.
> 
> This will make it easier to add module reset support later, which is
> currently not implemented, and difficult to achieve using the existing
> bindings due to the intertwined register layout.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks for re-working the binding per our discussion at ELC-E. Please
feel free to add my Ack to patches #1 and #2.

Regards,
Mike

> ---
> v4:
>   - No changes,
> 
> v3:
>   - Integrate CPG and MSSR,
> 
> v2:
>   - Switch from MSTP to MSSR.
> ---
>  .../devicetree/bindings/clock/renesas,cpg-mssr.txt | 71 ++++++++++++++++++++++
>  include/dt-bindings/clock/renesas-cpg-mssr.h       | 15 +++++
>  2 files changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
>  create mode 100644 include/dt-bindings/clock/renesas-cpg-mssr.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
> new file mode 100644
> index 0000000000000000..a56836aa2131a8db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
> @@ -0,0 +1,71 @@
> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
> +
> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator)
> +and MSSR (Module Standby and Software Reset) blocks are intimately connected,
> +and share the same register block.
> +
> +They provide the following functionalities:
> +  - The CPG block generates various core clocks,
> +  - The MSSR block provides two functions:
> +      1. Module Standby, providing a Clock Domain to control the clock supply
> +        to individual SoC devices,
> +      2. Reset Control, to perform a software reset of individual SoC devices.
> +
> +Required Properties:
> +  - compatible: Must be one of:
> +      - "renesas,r8a7791-cpg-mssr" for the r8a7791 SoC
> +      - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC
> +
> +  - reg: Base address and length of the memory resource used by the CPG/MSSR
> +    block
> +
> +  - clocks: References to external parent clocks, one entry for each entry in
> +    clock-names
> +  - clock-names: List of external parent clock names. Valid names are:
> +      - "extal" (r8a7791, r8a7795)
> +      - "extalr" (r8a7795)
> +      - "usb_extal" (r8a7791)
> +
> +  - #clock-cells: Must be 2
> +      - For CPG core clocks, the two clock specifier cells must be "CPG_CORE"
> +       and a core clock reference, as defined in
> +       <dt-bindings/clock/*-cpg-mssr.h>.
> +      - For module clocks, the two clock specifier cells must be "CPG_MOD" and
> +       a module number, as defined in the datasheet.
> +
> +  - #power-domain-cells: Must be 0
> +      - SoC devices that are part of the CPG/MSSR Clock Domain and can be
> +       power-managed through Module Standby should refer to the CPG device
> +       node in their "power-domains" property, as documented by the generic PM
> +       Domain bindings in
> +       Documentation/devicetree/bindings/power/power_domain.txt.
> +
> +
> +Examples
> +--------
> +
> +  - CPG device node:
> +
> +       cpg: clock-controller@e6150000 {
> +               compatible = "renesas,r8a7795-cpg-mssr";
> +               reg = <0 0xe6150000 0 0x1000>;
> +               clocks = <&extal_clk>, <&extalr_clk>;
> +               clock-names = "extal", "extalr";
> +               #clock-cells = <2>;
> +               #power-domain-cells = <0>;
> +       };
> +
> +
> +  - CPG/MSSR Clock Domain member device node:
> +
> +       scif2: serial@e6e88000 {
> +               compatible = "renesas,scif-r8a7795", "renesas,scif";
> +               reg = <0 0xe6e88000 0 64>;
> +               interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
> +               clocks = <&cpg CPG_MOD 310>;
> +               clock-names = "sci_ick";
> +               dmas = <&dmac1 0x13>, <&dmac1 0x12>;
> +               dma-names = "tx", "rx";
> +               power-domains = <&cpg>;
> +               status = "disabled";
> +       };
> diff --git a/include/dt-bindings/clock/renesas-cpg-mssr.h b/include/dt-bindings/clock/renesas-cpg-mssr.h
> new file mode 100644
> index 0000000000000000..569a3cc33ffb5bc7
> --- /dev/null
> +++ b/include/dt-bindings/clock/renesas-cpg-mssr.h
> @@ -0,0 +1,15 @@
> +/*
> + * Copyright (C) 2015 Renesas Electronics Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#ifndef __DT_BINDINGS_CLOCK_RENESAS_CPG_MSSR_H__
> +#define __DT_BINDINGS_CLOCK_RENESAS_CPG_MSSR_H__
> +
> +#define CPG_CORE                       0       /* Core Clock */
> +#define CPG_MOD                                1       /* Module Clock */
> +
> +#endif /* __DT_BINDINGS_CLOCK_RENESAS_CPG_MSSR_H__ */
> -- 
> 1.9.1
> 

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Geert Uytterhoeven]

On Fri, Oct 16, 2015 at 2:49 PM, Geert Uytterhoeven
<geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org> wrote:
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
> @@ -0,0 +1,71 @@
> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
> +
> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator)
> +and MSSR (Module Standby and Software Reset) blocks are intimately connected,
> +and share the same register block.
> +
> +They provide the following functionalities:
> +  - The CPG block generates various core clocks,
> +  - The MSSR block provides two functions:
> +      1. Module Standby, providing a Clock Domain to control the clock supply
> +        to individual SoC devices,
> +      2. Reset Control, to perform a software reset of individual SoC devices.
> +
> +Required Properties:
> +  - compatible: Must be one of:
> +      - "renesas,r8a7791-cpg-mssr" for the r8a7791 SoC

I'll drop the reference to "r8a7791", as we won't convert r8a7791 (yet).

> +      - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC
> +
> +  - reg: Base address and length of the memory resource used by the CPG/MSSR
> +    block
> +
> +  - clocks: References to external parent clocks, one entry for each entry in
> +    clock-names
> +  - clock-names: List of external parent clock names. Valid names are:
> +      - "extal" (r8a7791, r8a7795)
> +      - "extalr" (r8a7795)
> +      - "usb_extal" (r8a7791)

Likewise.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Michael Turquette]

Hi Geert,

Quoting Geert Uytterhoeven (2015-10-16 05:49:20)
> +static void __init r8a7795_cpg_mssr_init(struct device_node *np)
> +{
> +       struct regmap *regmap;
> +       u32 reg, cpg_mode;
> +
> +       regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr");
> +       if (IS_ERR(regmap) ||
> +           of_property_read_u32_index(np, "renesas,modemr", 1, &reg) ||
> +           regmap_read(regmap, reg, &cpg_mode)) {
> +               pr_err("%s: failed to parse renesas,modemr\n", np->full_name);
> +               return;
> +       }
> +
> +       cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
> +       if (!cpg_pll_config->extal_div) {
> +               pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n",
> +                      __func__, cpg_mode);
> +               return;
> +       }
> +
> +       cpg_mssr_probe(np, &r8a7795_cpg_mssr_info);
> +}
> +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr",
> +              r8a7795_cpg_mssr_init);

Is CLK_OF_DECLARE needed? Is it possible to make this a real
platform_driver à la drivers/clk/qcom/gcc-apq8084.c?

Sorry if I already asked this in a previous version, but a quick search
of my email didn't reveal anything.

Regards,
Mike

> -- 
> 1.9.1
> 

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Geert Uytterhoeven]

Hi Mike,

On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette
<mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> Quoting Geert Uytterhoeven (2015-10-16 05:49:20)
>> +static void __init r8a7795_cpg_mssr_init(struct device_node *np)
>> +{
>> +       struct regmap *regmap;
>> +       u32 reg, cpg_mode;
>> +
>> +       regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr");
>> +       if (IS_ERR(regmap) ||
>> +           of_property_read_u32_index(np, "renesas,modemr", 1, &reg) ||
>> +           regmap_read(regmap, reg, &cpg_mode)) {
>> +               pr_err("%s: failed to parse renesas,modemr\n", np->full_name);
>> +               return;
>> +       }
>> +
>> +       cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
>> +       if (!cpg_pll_config->extal_div) {
>> +               pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n",
>> +                      __func__, cpg_mode);
>> +               return;
>> +       }
>> +
>> +       cpg_mssr_probe(np, &r8a7795_cpg_mssr_info);
>> +}
>> +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr",
>> +              r8a7795_cpg_mssr_init);
>
> Is CLK_OF_DECLARE needed? Is it possible to make this a real
> platform_driver à la drivers/clk/qcom/gcc-apq8084.c?

I tried making it a real platform driver, but it failed: devices that are
part of the Clock Domain failed to get their clock (error -2, IIRC, which is
-ENOENT), and thus couldn't be instantiated.
I didn't look deeper at that time.

[... reading code ...]

Aha, this may be caused by __of_clk_get_from_provider() returning
hardcoded -ENOENT instead of propagating the error returned by
__clk_create_clk()?

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Michael Turquette]

Quoting Geert Uytterhoeven (2015-10-20 05:31:12)
> Hi Mike,
> 
> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette
> <mturquette@baylibre.com> wrote:
> > Quoting Geert Uytterhoeven (2015-10-16 05:49:20)
> >> +static void __init r8a7795_cpg_mssr_init(struct device_node *np)
> >> +{
> >> +       struct regmap *regmap;
> >> +       u32 reg, cpg_mode;
> >> +
> >> +       regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr");
> >> +       if (IS_ERR(regmap) ||
> >> +           of_property_read_u32_index(np, "renesas,modemr", 1, &reg) ||
> >> +           regmap_read(regmap, reg, &cpg_mode)) {
> >> +               pr_err("%s: failed to parse renesas,modemr\n", np->full_name);
> >> +               return;
> >> +       }
> >> +
> >> +       cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
> >> +       if (!cpg_pll_config->extal_div) {
> >> +               pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n",
> >> +                      __func__, cpg_mode);
> >> +               return;
> >> +       }
> >> +
> >> +       cpg_mssr_probe(np, &r8a7795_cpg_mssr_info);
> >> +}
> >> +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr",
> >> +              r8a7795_cpg_mssr_init);
> >
> > Is CLK_OF_DECLARE needed? Is it possible to make this a real
> > platform_driver à la drivers/clk/qcom/gcc-apq8084.c?
> 
> I tried making it a real platform driver, but it failed: devices that are
> part of the Clock Domain failed to get their clock (error -2, IIRC, which is
> -ENOENT), and thus couldn't be instantiated.
> I didn't look deeper at that time.
> 
> [... reading code ...]
> 
> Aha, this may be caused by __of_clk_get_from_provider() returning
> hardcoded -ENOENT instead of propagating the error returned by
> __clk_create_clk()?

Well the only other error thrown by __clk_create_clk is -ENOMEM, so I'm
not sure how that would help things.

The bindings should go in for 4.4, but if the driver is slated for 4.5
then can you investigate this some more? Stephen and I are on a mission
to have _real_ clk drivers.

Regards,
Mike

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Geert Uytterhoeven]

Hi Mike,

On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette
<mturquette@baylibre.com> wrote:
> Quoting Geert Uytterhoeven (2015-10-20 05:31:12)
>> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette
>> <mturquette@baylibre.com> wrote:
>> > Quoting Geert Uytterhoeven (2015-10-16 05:49:20)
>> >> +static void __init r8a7795_cpg_mssr_init(struct device_node *np)
>> >> +{
>> >> +       struct regmap *regmap;
>> >> +       u32 reg, cpg_mode;
>> >> +
>> >> +       regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr");
>> >> +       if (IS_ERR(regmap) ||
>> >> +           of_property_read_u32_index(np, "renesas,modemr", 1, &reg) ||
>> >> +           regmap_read(regmap, reg, &cpg_mode)) {
>> >> +               pr_err("%s: failed to parse renesas,modemr\n", np->full_name);
>> >> +               return;
>> >> +       }
>> >> +
>> >> +       cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
>> >> +       if (!cpg_pll_config->extal_div) {
>> >> +               pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n",
>> >> +                      __func__, cpg_mode);
>> >> +               return;
>> >> +       }
>> >> +
>> >> +       cpg_mssr_probe(np, &r8a7795_cpg_mssr_info);
>> >> +}
>> >> +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr",
>> >> +              r8a7795_cpg_mssr_init);
>> >
>> > Is CLK_OF_DECLARE needed? Is it possible to make this a real
>> > platform_driver à la drivers/clk/qcom/gcc-apq8084.c?
>>
>> I tried making it a real platform driver, but it failed: devices that are
>> part of the Clock Domain failed to get their clock (error -2, IIRC, which is
>> -ENOENT), and thus couldn't be instantiated.
>> I didn't look deeper at that time.
>>
>> [... reading code ...]
>>
>> Aha, this may be caused by __of_clk_get_from_provider() returning
>> hardcoded -ENOENT instead of propagating the error returned by
>> __clk_create_clk()?
>
> Well the only other error thrown by __clk_create_clk is -ENOMEM, so I'm
> not sure how that would help things.

Hmm, you're right.

> The bindings should go in for 4.4, but if the driver is slated for 4.5
> then can you investigate this some more? Stephen and I are on a mission
> to have _real_ clk drivers.

Sure, I'll have a deeper look.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Magnus Damm]

Hi Geert,

On Tue, Oct 20, 2015 at 9:16 PM, Geert Uytterhoeven
<geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> wrote:
> On Fri, Oct 16, 2015 at 2:49 PM, Geert Uytterhoeven
> <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org> wrote:
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
>> @@ -0,0 +1,71 @@
>> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
>> +
>> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator)
>> +and MSSR (Module Standby and Software Reset) blocks are intimately connected,
>> +and share the same register block.
>> +
>> +They provide the following functionalities:
>> +  - The CPG block generates various core clocks,
>> +  - The MSSR block provides two functions:
>> +      1. Module Standby, providing a Clock Domain to control the clock supply
>> +        to individual SoC devices,
>> +      2. Reset Control, to perform a software reset of individual SoC devices.
>> +
>> +Required Properties:
>> +  - compatible: Must be one of:
>> +      - "renesas,r8a7791-cpg-mssr" for the r8a7791 SoC
>
> I'll drop the reference to "r8a7791", as we won't convert r8a7791 (yet).
>
>> +      - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC
>> +
>> +  - reg: Base address and length of the memory resource used by the CPG/MSSR
>> +    block
>> +
>> +  - clocks: References to external parent clocks, one entry for each entry in
>> +    clock-names
>> +  - clock-names: List of external parent clock names. Valid names are:
>> +      - "extal" (r8a7791, r8a7795)
>> +      - "extalr" (r8a7795)
>> +      - "usb_extal" (r8a7791)
>
> Likewise.

Yeah, dropping r8a7791 from the initial version of this DT binding
seems like a good idea to me as well.

All looks good to me. Thanks for preparing this nicely written DT
binding document!

Reviewed-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/5] [RFC] clk: shmobile: Add r8a7795 CPG Core Clock Definitions

[Magnus Damm]

On Tue, Oct 20, 2015 at 7:09 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Fri, Oct 16, 2015 at 2:49 PM, Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
>> Add all R-Car H3 CPG Core Clock Outputs defined in the datasheet.
>
> Cfr. Table 8.2a ("List of Clocks [R-Car H3]").

Thanks for this additional information. Would it be possible to add
that table pointer together with data sheet revision to the commit
message or a comment in the code so we know which version of the
documentation this file is based on?

>> --- /dev/null
>> +++ b/include/dt-bindings/clock/r8a7795-cpg-mssr.h
>> @@ -0,0 +1,63 @@
>> +/*
>> + * Copyright (C) 2015 Renesas Electronics Corp.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +#ifndef __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__
>> +#define __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__
>> +
>> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
>> +
>> +/* r8a7795 CPG Core Clocks */
>> +#define R8A7795_CLK_Z                  0
>> +#define R8A7795_CLK_Z2                 1
>> +#define R8A7795_CLK_ZR                 2
>> +#define R8A7795_CLK_ZG                 3
>> +#define R8A7795_CLK_ZTR                        4
>> +#define R8A7795_CLK_ZTRD2              5
>> +#define R8A7795_CLK_ZT                 6
>> +#define R8A7795_CLK_ZX                 7
>> +#define R8A7795_CLK_S0D1               8
>> +#define R8A7795_CLK_S0D4               9
>> +#define R8A7795_CLK_S1D1               10
>> +#define R8A7795_CLK_S1D2               11
>> +#define R8A7795_CLK_S1D4               12
>> +#define R8A7795_CLK_S2D1               13
>> +#define R8A7795_CLK_S2D2               14
>> +#define R8A7795_CLK_S2D4               15
>> +#define R8A7795_CLK_S3D1               16
>> +#define R8A7795_CLK_S3D2               17
>> +#define R8A7795_CLK_S3D4               18
>
> Please note that I deliberately left out S[0-3], as they're used as
> internal clock
> sources only.

Omitting internal clock sources sounds sane to me. If you're going to
update the commit message they may I suggest that you list the
intentionally omitted clocks there?

Reviewed-by: Magnus Damm <damm+renesas@opensource.se>

Thanks for your help!

Best,

/ magnus

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Geert Uytterhoeven]

Hi Mike,

On Tue, Oct 20, 2015 at 3:07 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette
> <mturquette@baylibre.com> wrote:
>> Quoting Geert Uytterhoeven (2015-10-20 05:31:12)
>>> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette
>>> <mturquette@baylibre.com> wrote:
>>> > Quoting Geert Uytterhoeven (2015-10-16 05:49:20)
>>> >> +static void __init r8a7795_cpg_mssr_init(struct device_node *np)
>>> >> +{
>>> >> +       struct regmap *regmap;
>>> >> +       u32 reg, cpg_mode;
>>> >> +
>>> >> +       regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr");
>>> >> +       if (IS_ERR(regmap) ||
>>> >> +           of_property_read_u32_index(np, "renesas,modemr", 1, &reg) ||
>>> >> +           regmap_read(regmap, reg, &cpg_mode)) {
>>> >> +               pr_err("%s: failed to parse renesas,modemr\n", np->full_name);
>>> >> +               return;
>>> >> +       }
>>> >> +
>>> >> +       cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
>>> >> +       if (!cpg_pll_config->extal_div) {
>>> >> +               pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n",
>>> >> +                      __func__, cpg_mode);
>>> >> +               return;
>>> >> +       }
>>> >> +
>>> >> +       cpg_mssr_probe(np, &r8a7795_cpg_mssr_info);
>>> >> +}
>>> >> +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr",
>>> >> +              r8a7795_cpg_mssr_init);
>>> >
>>> > Is CLK_OF_DECLARE needed? Is it possible to make this a real
>>> > platform_driver à la drivers/clk/qcom/gcc-apq8084.c?
>>>
>>> I tried making it a real platform driver, but it failed: devices that are
>>> part of the Clock Domain failed to get their clock (error -2, IIRC, which is
>>> -ENOENT), and thus couldn't be instantiated.
>>> I didn't look deeper at that time.
>>>
>>> [... reading code ...]
>>>
>>> Aha, this may be caused by __of_clk_get_from_provider() returning
>>> hardcoded -ENOENT instead of propagating the error returned by
>>> __clk_create_clk()?
>>
>> Well the only other error thrown by __clk_create_clk is -ENOMEM, so I'm
>> not sure how that would help things.
>
> Hmm, you're right.
>
>> The bindings should go in for 4.4, but if the driver is slated for 4.5
>> then can you investigate this some more? Stephen and I are on a mission
>> to have _real_ clk drivers.
>
> Sure, I'll have a deeper look.

And so I did (on r8a7791/koelsch).

As I want to have as much clock data/code __init as possible (think
multi-platform kernels --- pinmux data is a disaster here), I have to use
platform_driver_probe().

  - Calling platform_driver_probe() from core_initcall() or postcore_initcall()
    is too early, as the platform device for the CPG hasn't been created yet.
    Hence the CPG Clock Domain isn't registered, and all devices fail to probe
    as they can't be attached to their Clock Domain.
      -> This is where the -ENOENT came from (I incorrectly assumed it came
         from the clock code; sorry for that), and it's converted into
         -EPROBE_DEFER by genpd_dev_pm_attach().

  - Calling platform_driver_probe() from arch_initcall() is too late, as the
    IRQC is initialized first (it's located before the CPG in .dtsi).
    Hence the IRQC can't find it's Clock Domain, and its probe is deferred.
    IRQC will be reprobed later, but in the mean time the Ethernet PHY can't
    find its IRQ, as the of_mdio code uses irq_of_parse_and_map(), which
    plainly ignores EPROBE_DEFER :-(
    Nevertheless, Ethernet works...

  - Using subsys_initcall() and later causes even more probe deferral.

So that's why I went with CLK_OF_DECLARE() again...

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Laurent Pinchart]

Hello,

On Wednesday 21 October 2015 01:01:03 Magnus Damm wrote:
> On Tue, Oct 20, 2015 at 9:16 PM, Geert Uytterhoeven wrote:
> > On Fri, Oct 16, 2015 at 2:49 PM, Geert Uytterhoeven wrote:
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
> >> @@ -0,0 +1,71 @@
> >> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
> >> +
> >> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
> >> Generator)
> >> +and MSSR (Module Standby and Software Reset) blocks are intimately
> >> connected,
> >> +and share the same register block.
> >> +
> >> +They provide the following functionalities:
> >> +  - The CPG block generates various core clocks,
> >> +  - The MSSR block provides two functions:
> >> +      1. Module Standby, providing a Clock Domain to control the clock
> >> supply
> >> +        to individual SoC devices,
> >> +      2. Reset Control, to perform a software reset of individual SoC
> >> devices.
> >> +
> >> +Required Properties:
> >> +  - compatible: Must be one of:
> >> +      - "renesas,r8a7791-cpg-mssr" for the r8a7791 SoC
> > 
> > I'll drop the reference to "r8a7791", as we won't convert r8a7791 (yet).

I'm fine with supporting r8a7795 only in the initial patch set, but when do we 
plan to convert r8a7791 ? I think it would be a good idea to validate these 
bindings on r8a7791 before considering them as stable.

> >> +      - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC
> >> +
> >> +  - reg: Base address and length of the memory resource used by the
> >> CPG/MSSR
> >> +    block
> >> +
> >> +  - clocks: References to external parent clocks, one entry for each
> >> entry in
> >> +    clock-names
> >> +  - clock-names: List of external parent clock names. Valid names are:
> >> +      - "extal" (r8a7791, r8a7795)
> >> +      - "extalr" (r8a7795)
> >> +      - "usb_extal" (r8a7791)
> > 
> > Likewise.
> 
> Yeah, dropping r8a7791 from the initial version of this DT binding
> seems like a good idea to me as well.
> 
> All looks good to me. Thanks for preparing this nicely written DT
> binding document!
> 
> Reviewed-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Geert Uytterhoeven]

Hi Laurent,

On Fri, Oct 23, 2015 at 1:05 PM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> On Wednesday 21 October 2015 01:01:03 Magnus Damm wrote:
>> On Tue, Oct 20, 2015 at 9:16 PM, Geert Uytterhoeven wrote:
>> > On Fri, Oct 16, 2015 at 2:49 PM, Geert Uytterhoeven wrote:
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
>> >> @@ -0,0 +1,71 @@
>> >> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
>> >> +
>> >> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
>> >> Generator)
>> >> +and MSSR (Module Standby and Software Reset) blocks are intimately
>> >> connected,
>> >> +and share the same register block.
>> >> +
>> >> +They provide the following functionalities:
>> >> +  - The CPG block generates various core clocks,
>> >> +  - The MSSR block provides two functions:
>> >> +      1. Module Standby, providing a Clock Domain to control the clock
>> >> supply
>> >> +        to individual SoC devices,
>> >> +      2. Reset Control, to perform a software reset of individual SoC
>> >> devices.
>> >> +
>> >> +Required Properties:
>> >> +  - compatible: Must be one of:
>> >> +      - "renesas,r8a7791-cpg-mssr" for the r8a7791 SoC
>> >
>> > I'll drop the reference to "r8a7791", as we won't convert r8a7791 (yet).
>
> I'm fine with supporting r8a7795 only in the initial patch set, but when do we
> plan to convert r8a7791 ? I think it would be a good idea to validate these

That's a political question...

> bindings on r8a7791 before considering them as stable.

I did the validation on both r8a7795/salvator-x and r8a7791/koelsch.
Please try branch topic/cpg-mssr-v4 of renesas-drivers.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Friday 16 October 2015 14:49:16 Geert Uytterhoeven wrote:
> On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
> Generator) and MSSR (Module Standby and Software Reset) blocks are
> intimately connected, and share the same register block.
> 
> Hence it makes sense to describe these two blocks using a
> single device node in DT, instead of using a hierarchical structure with
> multiple nodes, using a mix of generic and SoC-specific bindings.
> 
> These new DT bindings are intended to replace the existing DT bindings
> for CPG core clocks ("renesas,*-cpg-clocks", "renesas,cpg-div6-clock")
> and module clocks ("renesas,*-mstp-clocks"), at least for new SoCs.
> 
> This will make it easier to add module reset support later, which is
> currently not implemented, and difficult to achieve using the existing
> bindings due to the intertwined register layout.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v4:
>   - No changes,
> 
> v3:
>   - Integrate CPG and MSSR,
> 
> v2:
>   - Switch from MSTP to MSSR.
> ---
>  .../devicetree/bindings/clock/renesas,cpg-mssr.txt | 71 +++++++++++++++++++
>  include/dt-bindings/clock/renesas-cpg-mssr.h       | 15 +++++
>  2 files changed, 86 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt create mode
> 100644 include/dt-bindings/clock/renesas-cpg-mssr.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
> b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt new file
> mode 100644
> index 0000000000000000..a56836aa2131a8db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
> @@ -0,0 +1,71 @@
> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
> +
> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
> Generator)
> +and MSSR (Module Standby and Software Reset) blocks are intimately
> connected,
> +and share the same register block.
> +
> +They provide the following functionalities:
> +  - The CPG block generates various core clocks,
> +  - The MSSR block provides two functions:
> +      1. Module Standby, providing a Clock Domain to control the clock
> supply
> +	 to individual SoC devices,
> +      2. Reset Control, to perform a software reset of individual SoC
> devices.
> +
> +Required Properties:
> +  - compatible: Must be one of:
> +      - "renesas,r8a7791-cpg-mssr" for the r8a7791 SoC
> +      - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC
> +
> +  - reg: Base address and length of the memory resource used by the
> CPG/MSSR
> +    block
> +
> +  - clocks: References to external parent clocks, one entry for each entry
> in
> +    clock-names
> +  - clock-names: List of external parent clock names. Valid names are:
> +      - "extal" (r8a7791, r8a7795)
> +      - "extalr" (r8a7795)
> +      - "usb_extal" (r8a7791)
> +
> +  - #clock-cells: Must be 2
> +      - For CPG core clocks, the two clock specifier cells must be
> "CPG_CORE"
> +	and a core clock reference, as defined in
> +	<dt-bindings/clock/*-cpg-mssr.h>.
> +      - For module clocks, the two clock specifier cells must be "CPG_MOD"
> and
> +	a module number, as defined in the datasheet.
> +
> +  - #power-domain-cells: Must be 0
> +      - SoC devices that are part of the CPG/MSSR Clock Domain and can be
> +	power-managed through Module Standby should refer to the CPG device
> +	node in their "power-domains" property, as documented by the generic PM
> +	Domain bindings in
> +	Documentation/devicetree/bindings/power/power_domain.txt.
> +
> +
> +Examples
> +--------
> +
> +  - CPG device node:
> +
> +	cpg: clock-controller@e6150000 {
> +		compatible = "renesas,r8a7795-cpg-mssr";
> +		reg = <0 0xe6150000 0 0x1000>;
> +		clocks = <&extal_clk>, <&extalr_clk>;
> +		clock-names = "extal", "extalr";
> +		#clock-cells = <2>;
> +		#power-domain-cells = <0>;
> +	};
> +
> +
> +  - CPG/MSSR Clock Domain member device node:

Would it make sense to show two examples, one for a device whose driver 
manages the MSTP clock manually, and another one for a device whose driver 
delegates that to the power domain ?

I hate using the word driver in DT bindings, but unfortunately that's 
essentially what it boils down to here as the decision to specify the power 
domain is really based on the driver, not on the hardware.

Apart from that the rest looks good to me.

> +	scif2: serial@e6e88000 {
> +		compatible = "renesas,scif-r8a7795", "renesas,scif";
> +		reg = <0 0xe6e88000 0 64>;
> +		interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg CPG_MOD 310>;
> +		clock-names = "sci_ick";
> +		dmas = <&dmac1 0x13>, <&dmac1 0x12>;
> +		dma-names = "tx", "rx";
> +		power-domains = <&cpg>;
> +		status = "disabled";
> +	};
> diff --git a/include/dt-bindings/clock/renesas-cpg-mssr.h
> b/include/dt-bindings/clock/renesas-cpg-mssr.h new file mode 100644
> index 0000000000000000..569a3cc33ffb5bc7
> --- /dev/null
> +++ b/include/dt-bindings/clock/renesas-cpg-mssr.h
> @@ -0,0 +1,15 @@
> +/*
> + * Copyright (C) 2015 Renesas Electronics Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#ifndef __DT_BINDINGS_CLOCK_RENESAS_CPG_MSSR_H__
> +#define __DT_BINDINGS_CLOCK_RENESAS_CPG_MSSR_H__
> +
> +#define CPG_CORE			0	/* Core Clock */
> +#define CPG_MOD				1	/* Module Clock */
> +
> +#endif /* __DT_BINDINGS_CLOCK_RENESAS_CPG_MSSR_H__ */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Laurent Pinchart]

Hi Geert,

On Friday 23 October 2015 13:09:22 Geert Uytterhoeven wrote:
> On Fri, Oct 23, 2015 at 1:05 PM, Laurent Pinchart wrote:
> > On Wednesday 21 October 2015 01:01:03 Magnus Damm wrote:
> >> On Tue, Oct 20, 2015 at 9:16 PM, Geert Uytterhoeven wrote:
> >>> On Fri, Oct 16, 2015 at 2:49 PM, Geert Uytterhoeven wrote:
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
> >>>> @@ -0,0 +1,71 @@
> >>>> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
> >>>> +
> >>>> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
> >>>> Generator)
> >>>> +and MSSR (Module Standby and Software Reset) blocks are intimately
> >>>> connected,
> >>>> +and share the same register block.
> >>>> +
> >>>> +They provide the following functionalities:
> >>>> +  - The CPG block generates various core clocks,
> >>>> +  - The MSSR block provides two functions:
> >>>> +      1. Module Standby, providing a Clock Domain to control the
> >>>> clock supply
> >>>> +        to individual SoC devices,
> >>>> +      2. Reset Control, to perform a software reset of individual SoC
> >>>> devices.
> >>>> +
> >>>> +Required Properties:
> >>>> +  - compatible: Must be one of:
> >>>> +      - "renesas,r8a7791-cpg-mssr" for the r8a7791 SoC
> >>> 
> >>> I'll drop the reference to "r8a7791", as we won't convert r8a7791
> >>> (yet).
> > 
> > I'm fine with supporting r8a7795 only in the initial patch set, but when
> > do we plan to convert r8a7791 ? I think it would be a good idea to
> > validate these
>
> That's a political question...
> 
> > bindings on r8a7791 before considering them as stable.
> 
> I did the validation on both r8a7795/salvator-x and r8a7791/koelsch.
> Please try branch topic/cpg-mssr-v4 of renesas-drivers.

If it works what's blocking r8a7791 support from being upstreamed ? :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/5] [RFC] clk: shmobile: Add r8a7795 CPG Core Clock Definitions

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Friday 16 October 2015 14:49:17 Geert Uytterhoeven wrote:
> Add all R-Car H3 CPG Core Clock Outputs defined in the datasheet.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v4:
>   - Add all clocks instead of just the ones used by the current DTS.
> 
> v3:
>   - New.
> ---
>  include/dt-bindings/clock/r8a7795-cpg-mssr.h | 63 +++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 include/dt-bindings/clock/r8a7795-cpg-mssr.h
> 
> diff --git a/include/dt-bindings/clock/r8a7795-cpg-mssr.h
> b/include/dt-bindings/clock/r8a7795-cpg-mssr.h new file mode 100644
> index 0000000000000000..e864aae0a2561c4b
> --- /dev/null
> +++ b/include/dt-bindings/clock/r8a7795-cpg-mssr.h
> @@ -0,0 +1,63 @@
> +/*
> + * Copyright (C) 2015 Renesas Electronics Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#ifndef __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__
> +#define __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__
> +
> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
> +
> +/* r8a7795 CPG Core Clocks */
> +#define R8A7795_CLK_Z			0
> +#define R8A7795_CLK_Z2			1
> +#define R8A7795_CLK_ZR			2
> +#define R8A7795_CLK_ZG			3
> +#define R8A7795_CLK_ZTR			4
> +#define R8A7795_CLK_ZTRD2		5
> +#define R8A7795_CLK_ZT			6
> +#define R8A7795_CLK_ZX			7
> +#define R8A7795_CLK_S0D1		8
> +#define R8A7795_CLK_S0D4		9
> +#define R8A7795_CLK_S1D1		10
> +#define R8A7795_CLK_S1D2		11
> +#define R8A7795_CLK_S1D4		12
> +#define R8A7795_CLK_S2D1		13
> +#define R8A7795_CLK_S2D2		14
> +#define R8A7795_CLK_S2D4		15
> +#define R8A7795_CLK_S3D1		16
> +#define R8A7795_CLK_S3D2		17
> +#define R8A7795_CLK_S3D4		18
> +#define R8A7795_CLK_LB			19
> +#define R8A7795_CLK_CL			20
> +#define R8A7795_CLK_ZB3			21
> +#define R8A7795_CLK_ZB3D2		22
> +#define R8A7795_CLK_CR			23
> +#define R8A7795_CLK_CRD2		24
> +#define R8A7795_CLK_SD0H		25
> +#define R8A7795_CLK_SD0			26
> +#define R8A7795_CLK_SD1H		27
> +#define R8A7795_CLK_SD1			28
> +#define R8A7795_CLK_SD2H		29
> +#define R8A7795_CLK_SD2			30
> +#define R8A7795_CLK_SD3H		31
> +#define R8A7795_CLK_SD3			32
> +#define R8A7795_CLK_SSP2		33
> +#define R8A7795_CLK_SSP1		34
> +#define R8A7795_CLK_SSPRS		35
> +#define R8A7795_CLK_RPC			36
> +#define R8A7795_CLK_RPCD2		37
> +#define R8A7795_CLK_MSO			38
> +#define R8A7795_CLK_CANFD		39
> +#define R8A7795_CLK_HDMI		40
> +#define R8A7795_CLK_CSI0		41
> +#define R8A7795_CLK_CSIREF		42
> +#define R8A7795_CLK_CP			43
> +#define R8A7795_CLK_CPEX		44
> +#define R8A7795_CLK_R			45
> +#define R8A7795_CLK_OSC			46

Those two clocks are called RCLK and OSCCLK in the datasheet, shouldn't we use 
those names ?

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +#endif /* __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__ */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/5] [RFC] clk: shmobile: Add r8a7795 CPG Core Clock Definitions

[Geert Uytterhoeven]

Hi Laurent,

On Fri, Oct 23, 2015 at 1:21 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> +#define R8A7795_CLK_R                        45
>> +#define R8A7795_CLK_OSC                      46
>
> Those two clocks are called RCLK and OSCCLK in the datasheet, shouldn't we use
> those names ?

..._CLK_RCLK and ..._CLK_OSCCLK sound a bit heavy to me.

Besides, all other clocks in the datasheet have a "φ" suffix, which also
means "CLK", and which we don't retain.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v4 3/5] [RFC] clk: shmobile: div6: Extract cpg_div6_register()

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Friday 16 October 2015 14:49:18 Geert Uytterhoeven wrote:
> Extract cpg_div6_register(), to allow registering div6 clocks from
> another clock driver.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v4:
>   - New.
> ---
>  drivers/clk/shmobile/clk-div6.c | 119 +++++++++++++++++++++++-------------
>  drivers/clk/shmobile/clk-div6.h |   3 +
>  2 files changed, 77 insertions(+), 45 deletions(-)
>  create mode 100644 drivers/clk/shmobile/clk-div6.h
> 
> diff --git a/drivers/clk/shmobile/clk-div6.c
> b/drivers/clk/shmobile/clk-div6.c index 57016ff9c585fc6e..5e8525fc60427229
> 100644
> --- a/drivers/clk/shmobile/clk-div6.c
> +++ b/drivers/clk/shmobile/clk-div6.c
> @@ -18,6 +18,8 @@
>  #include <linux/of_address.h>
>  #include <linux/slab.h>
> 
> +#include "clk-div6.h"
> +
>  #define CPG_DIV6_CKSTP		BIT(8)
>  #define CPG_DIV6_DIV(d)		((d) & 0x3f)
>  #define CPG_DIV6_DIV_MASK	0x3f
> @@ -172,60 +174,34 @@ static const struct clk_ops cpg_div6_clock_ops = {
>  	.set_rate = cpg_div6_clock_set_rate,
>  };

Non-static functions deserve a bit of documentation :-)

> -static void __init cpg_div6_clock_init(struct device_node *np)
> +struct clk * __init cpg_div6_register(const char *name,
> +				      unsigned int num_parents,
> +				      const char **parent_names,
> +				      void __iomem *reg)
>  {
> -	unsigned int num_parents, valid_parents;
> -	const char **parent_names;
> +	unsigned int valid_parents;
>  	struct clk_init_data init;
>  	struct div6_clock *clock;
> -	const char *clk_name = np->name;
>  	struct clk *clk;
>  	unsigned int i;
> 
>  	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
>  	if (!clock)
> -		return;
> -
> -	num_parents = of_clk_get_parent_count(np);
> -	if (num_parents < 1) {
> -		pr_err("%s: no parent found for %s DIV6 clock\n",
> -		       __func__, np->name);
> -		return;
> -	}
> +		return ERR_PTR(-ENOMEM);
> 
>  	clock->parents = kmalloc_array(num_parents, sizeof(*clock->parents),
> -		GFP_KERNEL);
> -	parent_names = kmalloc_array(num_parents, sizeof(*parent_names),
> -				GFP_KERNEL);
> -	if (!parent_names)
> -		return;
> +				       GFP_KERNEL);
> +	if (!clock->parents)
> +		return ERR_PTR(-ENOMEM);
> 
> -	/* Remap the clock register and read the divisor. Disabling the
> -	 * clock overwrites the divisor, so we need to cache its value for the
> -	 * enable operation.
> -	 */
> -	clock->reg = of_iomap(np, 0);
> -	if (clock->reg == NULL) {
> -		pr_err("%s: failed to map %s DIV6 clock register\n",
> -		       __func__, np->name);
> -		goto error;
> -	}
> +	clock->reg = reg;
> 
> +	/*
> +	 * Read the divisor. Disabling the clock overwrites the divisor, so we
> +	 * need to cache its value for the enable operation.
> +	 */
>  	clock->div = (clk_readl(clock->reg) & CPG_DIV6_DIV_MASK) + 1;
> 
> -	/* Parse the DT properties. */
> -	of_property_read_string(np, "clock-output-names", &clk_name);
> -
> -	for (i = 0, valid_parents = 0; i < num_parents; i++) {
> -		const char *name = of_clk_get_parent_name(np, i);
> -
> -		if (name) {
> -			parent_names[valid_parents] = name;
> -			clock->parents[valid_parents] = i;
> -			valid_parents++;
> -		}
> -	}
> -
>  	switch (num_parents) {
>  	case 1:
>  		/* fixed parent clock */
> @@ -243,12 +219,22 @@ static void __init cpg_div6_clock_init(struct
> device_node *np) break;
>  	default:
>  		pr_err("%s: invalid number of parents for DIV6 clock %s\n",
> -		       __func__, np->name);
> +		       __func__, name);
> +		clk = ERR_PTR(-EINVAL);
>  		goto error;
>  	}
> 
> +	/* Filter out invalid parents */
> +	for (i = 0, valid_parents = 0; i < num_parents; i++) {
> +		if (parent_names[i]) {
> +			parent_names[valid_parents] = parent_names[i];
> +			clock->parents[valid_parents] = i;
> +			valid_parents++;
> +		}
> +	}
> +
>  	/* Register the clock. */
> -	init.name = clk_name;
> +	init.name = name;
>  	init.ops = &cpg_div6_clock_ops;
>  	init.flags = CLK_IS_BASIC;
>  	init.parent_names = parent_names;
> @@ -257,6 +243,50 @@ static void __init cpg_div6_clock_init(struct
> device_node *np) clock->hw.init = &init;
> 
>  	clk = clk_register(NULL, &clock->hw);
> +	if (!IS_ERR(clk))
> +		return clk;
> +
> +error:
> +	kfree(clock->parents);
> +	kfree(clock);

If you think that errors when registering the DIV6 clock are not fatal enough 
to not have to care about memory leakage, you should kfree(clock) when the 
kmalloc_array() above fails.

> +	return clk;
> +}
> +
> +static void __init cpg_div6_clock_init(struct device_node *np)
> +{
> +	unsigned int num_parents;
> +	const char **parent_names;
> +	const char *clk_name = np->name;
> +	void __iomem *reg;
> +	struct clk *clk;
> +	unsigned int i;
> +
> +	num_parents = of_clk_get_parent_count(np);
> +	if (num_parents < 1) {
> +		pr_err("%s: no parent found for %s DIV6 clock\n",
> +		       __func__, np->name);
> +		return;
> +	}
> +
> +	parent_names = kmalloc_array(num_parents, sizeof(*parent_names),
> +				GFP_KERNEL);
> +	if (!parent_names)
> +		return;
> +
> +	reg = of_iomap(np, 0);
> +	if (reg == NULL) {
> +		pr_err("%s: failed to map %s DIV6 clock register\n",
> +		       __func__, np->name);
> +		goto error;
> +	}
> +
> +	/* Parse the DT properties. */
> +	of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +	for (i = 0; i < num_parents; i++)
> +		parent_names[i] = of_clk_get_parent_name(np, i);
> +
> +	clk = cpg_div6_register(clk_name, num_parents, parent_names, reg);
>  	if (IS_ERR(clk)) {
>  		pr_err("%s: failed to register %s DIV6 clock (%ld)\n",
>  		       __func__, np->name, PTR_ERR(clk));
> @@ -269,9 +299,8 @@ static void __init cpg_div6_clock_init(struct
> device_node *np) return;
> 
>  error:
> -	if (clock->reg)
> -		iounmap(clock->reg);
> +	if (reg)
> +		iounmap(reg);
>  	kfree(parent_names);
> -	kfree(clock);
>  }
>  CLK_OF_DECLARE(cpg_div6_clk, "renesas,cpg-div6-clock",
> cpg_div6_clock_init); diff --git a/drivers/clk/shmobile/clk-div6.h
> b/drivers/clk/shmobile/clk-div6.h new file mode 100644
> index 0000000000000000..d19531f42953c83f
> --- /dev/null
> +++ b/drivers/clk/shmobile/clk-div6.h
> @@ -0,0 +1,3 @@
> +
> +struct clk *cpg_div6_register(const char *name, unsigned int num_parents,
> +			      const char **parent_names, void __iomem *reg);

Could you please add #ifdef include guard macros ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Stephen Boyd]

On 10/22, Geert Uytterhoeven wrote:
> Hi Mike,
> 
> On Tue, Oct 20, 2015 at 3:07 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette
> > <mturquette@baylibre.com> wrote:
> >> Quoting Geert Uytterhoeven (2015-10-20 05:31:12)
> >>> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette
> >>> <mturquette@baylibre.com> wrote:
> >
> >> The bindings should go in for 4.4, but if the driver is slated for 4.5
> >> then can you investigate this some more? Stephen and I are on a mission
> >> to have _real_ clk drivers.
> >
> > Sure, I'll have a deeper look.
> 
> And so I did (on r8a7791/koelsch).
> 
> As I want to have as much clock data/code __init as possible (think
> multi-platform kernels --- pinmux data is a disaster here), I have to use
> platform_driver_probe().
> 
>   - Calling platform_driver_probe() from core_initcall() or postcore_initcall()
>     is too early, as the platform device for the CPG hasn't been created yet.
>     Hence the CPG Clock Domain isn't registered, and all devices fail to probe
>     as they can't be attached to their Clock Domain.
>       -> This is where the -ENOENT came from (I incorrectly assumed it came
>          from the clock code; sorry for that), and it's converted into
>          -EPROBE_DEFER by genpd_dev_pm_attach().
> 
>   - Calling platform_driver_probe() from arch_initcall() is too late, as the
>     IRQC is initialized first (it's located before the CPG in .dtsi).
>     Hence the IRQC can't find it's Clock Domain, and its probe is deferred.
>     IRQC will be reprobed later, but in the mean time the Ethernet PHY can't
>     find its IRQ, as the of_mdio code uses irq_of_parse_and_map(), which
>     plainly ignores EPROBE_DEFER :-(
>     Nevertheless, Ethernet works...
> 
>   - Using subsys_initcall() and later causes even more probe deferral.
> 
> So that's why I went with CLK_OF_DECLARE() again...
> 

Understandable for the few clocks that are used by the interrupt
controller, but for the other clocks, could those be registered
from a real platform device driver probe path? I've been
considering making some API that lets devices associate with the
clocks that the file had to register with CLK_OF_DECLARE(). The
driver would have to be builtin then (no modules) but otherwise
we would be able to benefit from the device driver model for most
other clocks.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Geert Uytterhoeven]

Hi Stephen,

On Sat, Oct 24, 2015 at 3:10 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/22, Geert Uytterhoeven wrote:
>> On Tue, Oct 20, 2015 at 3:07 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>> > On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette
>> > <mturquette@baylibre.com> wrote:
>> >> Quoting Geert Uytterhoeven (2015-10-20 05:31:12)
>> >>> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette
>> >>> <mturquette@baylibre.com> wrote:
>> >
>> >> The bindings should go in for 4.4, but if the driver is slated for 4.5
>> >> then can you investigate this some more? Stephen and I are on a mission
>> >> to have _real_ clk drivers.
>> >
>> > Sure, I'll have a deeper look.
>>
>> And so I did (on r8a7791/koelsch).
>>
>> As I want to have as much clock data/code __init as possible (think
>> multi-platform kernels --- pinmux data is a disaster here), I have to use
>> platform_driver_probe().
>>
>>   - Calling platform_driver_probe() from core_initcall() or postcore_initcall()
>>     is too early, as the platform device for the CPG hasn't been created yet.
>>     Hence the CPG Clock Domain isn't registered, and all devices fail to probe
>>     as they can't be attached to their Clock Domain.
>>       -> This is where the -ENOENT came from (I incorrectly assumed it came
>>          from the clock code; sorry for that), and it's converted into
>>          -EPROBE_DEFER by genpd_dev_pm_attach().
>>
>>   - Calling platform_driver_probe() from arch_initcall() is too late, as the
>>     IRQC is initialized first (it's located before the CPG in .dtsi).
>>     Hence the IRQC can't find it's Clock Domain, and its probe is deferred.
>>     IRQC will be reprobed later, but in the mean time the Ethernet PHY can't
>>     find its IRQ, as the of_mdio code uses irq_of_parse_and_map(), which
>>     plainly ignores EPROBE_DEFER :-(
>>     Nevertheless, Ethernet works...
>>
>>   - Using subsys_initcall() and later causes even more probe deferral.
>>
>> So that's why I went with CLK_OF_DECLARE() again...
>
> Understandable for the few clocks that are used by the interrupt
> controller, but for the other clocks, could those be registered
> from a real platform device driver probe path? I've been

In this particular case, that would be the IRQC module clocks and its
parent, the CP clock, which is derived directly from the external crystal.

If we ever have to do this for the GIC, that's gonna be several more
clocks (INTC-SYS / ZS / PLL1 / MAIN / external crystal).

> considering making some API that lets devices associate with the
> clocks that the file had to register with CLK_OF_DECLARE(). The
> driver would have to be builtin then (no modules) but otherwise
> we would be able to benefit from the device driver model for most
> other clocks.

To be honest, that still feels like a hack to me.

I hope dependencies and probe order, and obscure drivers not handling
deferred probe correctly, will be solved later sooner or later,
so hacks like that are no longer needed.

For new SoCs like r8a7795 we can probably just make it a real platform driver,
and make sure the irqc node is located after the cpg node in the .dtsi.

Upon second thought, that may even be feasible for existing SoCs, as
they would need a DTS update to make use of the new bindings/driver anyway,
so the irqc node can be moved at the same time. Backwards compatibility
through the old bindings/drivers would keep on using CLK_OF_DECLARE().

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Laurent Pinchart]

Hi Geert,

On Saturday 24 October 2015 19:34:03 Geert Uytterhoeven wrote:
> On Sat, Oct 24, 2015 at 3:10 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/22, Geert Uytterhoeven wrote:
> >> On Tue, Oct 20, 2015 at 3:07 PM, Geert Uytterhoeven wrote:
> >>> On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette wrote:
> >>>> Quoting Geert Uytterhoeven (2015-10-20 05:31:12)
> >>>>> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette wrote:
> >>>> The bindings should go in for 4.4, but if the driver is slated for 4.5
> >>>> then can you investigate this some more? Stephen and I are on a
> >>>> mission to have _real_ clk drivers.
> >>> 
> >>> Sure, I'll have a deeper look.
> >> 
> >> And so I did (on r8a7791/koelsch).
> >> 
> >> As I want to have as much clock data/code __init as possible (think
> >> multi-platform kernels --- pinmux data is a disaster here), I have to use
> >> platform_driver_probe().

That sounds like an __init issue, doesn't it ? The CPG driver will always be 
builtin and probed during the init process, what's preventing us from using 
normal driver probing ?

> >>   - Calling platform_driver_probe() from core_initcall() or
> >>     postcore_initcall() is too early, as the platform device for the CPG
> >>     hasn't been created yet. Hence the CPG Clock Domain isn't registered,
> >>     and all devices fail to probe as they can't be attached to their
> >>     Clock Domain.
> >>     
> >>       -> This is where the -ENOENT came from (I incorrectly assumed it
> >>          came from the clock code; sorry for that), and it's converted
> >>          into -EPROBE_DEFER by genpd_dev_pm_attach().
> >>   
> >>   - Calling platform_driver_probe() from arch_initcall() is too late, as
> >>     the IRQC is initialized first (it's located before the CPG in .dtsi).
> >>     Hence the IRQC can't find it's Clock Domain, and its probe is
> >>     deferred. IRQC will be reprobed later, but in the mean time the
> >>     Ethernet PHY can't find its IRQ, as the of_mdio code uses
> >>     irq_of_parse_and_map(), which plainly ignores EPROBE_DEFER :-(
> >>     Nevertheless, Ethernet works...
> >>   
> >>   - Using subsys_initcall() and later causes even more probe deferral.
> >> 
> >> So that's why I went with CLK_OF_DECLARE() again...
> > 
> > Understandable for the few clocks that are used by the interrupt
> > controller, but for the other clocks, could those be registered
> > from a real platform device driver probe path? I've been
> 
> In this particular case, that would be the IRQC module clocks and its
> parent, the CP clock, which is derived directly from the external crystal.
> 
> If we ever have to do this for the GIC, that's gonna be several more
> clocks (INTC-SYS / ZS / PLL1 / MAIN / external crystal).
> 
> > considering making some API that lets devices associate with the
> > clocks that the file had to register with CLK_OF_DECLARE(). The
> > driver would have to be builtin then (no modules) but otherwise
> > we would be able to benefit from the device driver model for most
> > other clocks.
> 
> To be honest, that still feels like a hack to me.
> 
> I hope dependencies and probe order, and obscure drivers not handling
> deferred probe correctly, will be solved later sooner or later,
> so hacks like that are no longer needed.
> 
> For new SoCs like r8a7795 we can probably just make it a real platform
> driver, and make sure the irqc node is located after the cpg node in the
> .dtsi.

That's another hack :-) We really shouldn't depend on DT nodes order.

I'm all for getting rid of CLK_OF_DECLARE

> Upon second thought, that may even be feasible for existing SoCs, as
> they would need a DTS update to make use of the new bindings/driver anyway,
> so the irqc node can be moved at the same time. Backwards compatibility
> through the old bindings/drivers would keep on using CLK_OF_DECLARE().

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Oct 26, 2015 at 3:25 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Saturday 24 October 2015 19:34:03 Geert Uytterhoeven wrote:
>> On Sat, Oct 24, 2015 at 3:10 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> > On 10/22, Geert Uytterhoeven wrote:
>> >> As I want to have as much clock data/code __init as possible (think
>> >> multi-platform kernels --- pinmux data is a disaster here), I have to use
>> >> platform_driver_probe().
>
> That sounds like an __init issue, doesn't it ? The CPG driver will always be
> builtin and probed during the init process, what's preventing us from using
> normal driver probing ?

When using platform_driver_register(), the tables cannot be __init, as that
would cause a section type mismatch. Remember, the driver core handles
platform devices appearing later, so .probe() should continue to be available.

Note: in theory it should be possible to compile the CPG/MSSR driver as a
module, and have the module in your initramfs. But I don't think anyone
really wants to do that?

>> For new SoCs like r8a7795 we can probably just make it a real platform
>> driver, and make sure the irqc node is located after the cpg node in the
>> .dtsi.
>
> That's another hack :-) We really shouldn't depend on DT nodes order.

I agree. But if there's an unfixed bug somewhere else, we cannot introduce
regressions (for already supported SoCs).

> I'm all for getting rid of CLK_OF_DECLARE

Me too.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Geert Uytterhoeven]

Hi Laurent,

On Fri, Oct 23, 2015 at 1:10 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Friday 16 October 2015 14:49:16 Geert Uytterhoeven wrote:
>> On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
>> Generator) and MSSR (Module Standby and Software Reset) blocks are
>> intimately connected, and share the same register block.
>>
>> Hence it makes sense to describe these two blocks using a
>> single device node in DT, instead of using a hierarchical structure with
>> multiple nodes, using a mix of generic and SoC-specific bindings.
>>
>> These new DT bindings are intended to replace the existing DT bindings
>> for CPG core clocks ("renesas,*-cpg-clocks", "renesas,cpg-div6-clock")
>> and module clocks ("renesas,*-mstp-clocks"), at least for new SoCs.
>>
>> This will make it easier to add module reset support later, which is
>> currently not implemented, and difficult to achieve using the existing
>> bindings due to the intertwined register layout.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
>> @@ -0,0 +1,71 @@
>> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
>> +
>> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
>> Generator)
>> +and MSSR (Module Standby and Software Reset) blocks are intimately
>> connected,
>> +and share the same register block.
>> +
>> +They provide the following functionalities:
>> +  - The CPG block generates various core clocks,
>> +  - The MSSR block provides two functions:
>> +      1. Module Standby, providing a Clock Domain to control the clock
>> supply
>> +      to individual SoC devices,
>> +      2. Reset Control, to perform a software reset of individual SoC
>> devices.

[...]

>> +  - #power-domain-cells: Must be 0
>> +      - SoC devices that are part of the CPG/MSSR Clock Domain and can be
>> +     power-managed through Module Standby should refer to the CPG device
>> +     node in their "power-domains" property, as documented by the generic PM
>> +     Domain bindings in
>> +     Documentation/devicetree/bindings/power/power_domain.txt.
>> +
>> +
>> +Examples
>> +--------
>> +
>> +  - CPG device node:
>> +
>> +     cpg: clock-controller@e6150000 {
>> +             compatible = "renesas,r8a7795-cpg-mssr";
>> +             reg = <0 0xe6150000 0 0x1000>;
>> +             clocks = <&extal_clk>, <&extalr_clk>;
>> +             clock-names = "extal", "extalr";
>> +             #clock-cells = <2>;
>> +             #power-domain-cells = <0>;
>> +     };
>> +
>> +
>> +  - CPG/MSSR Clock Domain member device node:
>
> Would it make sense to show two examples, one for a device whose driver
> manages the MSTP clock manually, and another one for a device whose driver
> delegates that to the power domain ?
>
> I hate using the word driver in DT bindings, but unfortunately that's
> essentially what it boils down to here as the decision to specify the power
> domain is really based on the driver, not on the hardware.

IMHO it's not the driver, but the on-SoC module and its representation in DT.
Cfr. commit 797a0626e08ca4af ("ARM: shmobile: r8a7791 dtsi: Add CPG/MSTP Clock
Domain", which states:

|   Add "power-domains" properties to all device nodes for devices that are
|   part of the CPG/MSTP Clock Domain and can be power-managed through an
|   MSTP clock.  This applies to most on-SoC devices, which have a
|   one-to-one mapping from SoC device to DT device node.  Notable
|   exceptions are the "display" and "sound" nodes, which represent multiple
|   SoC devices, each having their own MSTP clocks.

If a single device block (sharing the same register space), represented in
DT as a single device node, actually represents multiple modules, there's no
one-to-one mapping from SoC device to DT device node.
Hence the device node will have multiple module clocks, and the driver will
have to take care of managing them explicitly.

The register space sharing is an SoC-specific issue.
The single device node is a DT binding issue.

Perhaps such devices should use subnodes, so each of them can represent a
module, each with its own module clock.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Laurent Pinchart]

Hi Geert,

On Monday 26 October 2015 20:02:45 Geert Uytterhoeven wrote:
> On Fri, Oct 23, 2015 at 1:10 PM, Laurent Pinchart wrote:
> > On Friday 16 October 2015 14:49:16 Geert Uytterhoeven wrote:
> >> On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
> >> Generator) and MSSR (Module Standby and Software Reset) blocks are
> >> intimately connected, and share the same register block.
> >> 
> >> Hence it makes sense to describe these two blocks using a
> >> single device node in DT, instead of using a hierarchical structure with
> >> multiple nodes, using a mix of generic and SoC-specific bindings.
> >> 
> >> These new DT bindings are intended to replace the existing DT bindings
> >> for CPG core clocks ("renesas,*-cpg-clocks", "renesas,cpg-div6-clock")
> >> and module clocks ("renesas,*-mstp-clocks"), at least for new SoCs.
> >> 
> >> This will make it easier to add module reset support later, which is
> >> currently not implemented, and difficult to achieve using the existing
> >> bindings due to the intertwined register layout.
> >> 
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> 
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
> >> @@ -0,0 +1,71 @@
> >> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
> >> +
> >> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
> >> Generator)
> >> +and MSSR (Module Standby and Software Reset) blocks are intimately
> >> connected,
> >> +and share the same register block.
> >> +
> >> +They provide the following functionalities:
> >> +  - The CPG block generates various core clocks,
> >> +  - The MSSR block provides two functions:
> >> +      1. Module Standby, providing a Clock Domain to control the clock
> >> supply
> >> +      to individual SoC devices,
> >> +      2. Reset Control, to perform a software reset of individual SoC
> >> devices.
> 
> [...]
> 
> >> +  - #power-domain-cells: Must be 0
> >> +      - SoC devices that are part of the CPG/MSSR Clock Domain and can
> >> be
> >> +     power-managed through Module Standby should refer to the CPG device
> >> +     node in their "power-domains" property, as documented by the
> >> generic PM +     Domain bindings in
> >> +     Documentation/devicetree/bindings/power/power_domain.txt.
> >> +
> >> +
> >> +Examples
> >> +--------
> >> +
> >> +  - CPG device node:
> >> +
> >> +     cpg: clock-controller@e6150000 {
> >> +             compatible = "renesas,r8a7795-cpg-mssr";
> >> +             reg = <0 0xe6150000 0 0x1000>;
> >> +             clocks = <&extal_clk>, <&extalr_clk>;
> >> +             clock-names = "extal", "extalr";
> >> +             #clock-cells = <2>;
> >> +             #power-domain-cells = <0>;
> >> +     };
> >> +
> >> +
> > 
> >> +  - CPG/MSSR Clock Domain member device node:
> > Would it make sense to show two examples, one for a device whose driver
> > manages the MSTP clock manually, and another one for a device whose driver
> > delegates that to the power domain ?
> > 
> > I hate using the word driver in DT bindings, but unfortunately that's
> > essentially what it boils down to here as the decision to specify the
> > power domain is really based on the driver, not on the hardware.
> 
> IMHO it's not the driver, but the on-SoC module and its representation in
> DT. Cfr. commit 797a0626e08ca4af ("ARM: shmobile: r8a7791 dtsi: Add
> CPG/MSTP Clock Domain", which states:
>
> |   Add "power-domains" properties to all device nodes for devices that are
> |   part of the CPG/MSTP Clock Domain and can be power-managed through an
> |   MSTP clock.  This applies to most on-SoC devices, which have a
> |   one-to-one mapping from SoC device to DT device node.  Notable
> |   exceptions are the "display" and "sound" nodes, which represent multiple
> |   SoC devices, each having their own MSTP clocks.

You're quoting your own documentation to support your point, that's not fair 
:-)

We're using power domains to gate clocks. The fact that it's not related to 
power supplies can already be borderline, but I can buy the argument that 
clocks relate to power consumption here. However, where the power domain 
abstraction is really abused is that we're adding all kind of devices to a 
single power domain while they're controlled by one clock gate each. That's a 
software hack, and we're using DT to tell whether our core code should control 
clock gating or not. That's not a hardware description, sorry.

> If a single device block (sharing the same register space), represented in
> DT as a single device node, actually represents multiple modules, there's no
> one-to-one mapping from SoC device to DT device node.
>
> Hence the device node will have multiple module clocks, and the driver will
> have to take care of managing them explicitly.

There can be other reasons why the clocks need to be controlled explicitly by 
the driver. At the end of the end it's a per-driver decision whether it wants 
to delegate clock management to runtime PM (which will be the default cause) 
or handle it itself.

> The register space sharing is an SoC-specific issue.
> The single device node is a DT binding issue.
> 
> Perhaps such devices should use subnodes, so each of them can represent a
> module, each with its own module clock.

I'm not sure to see how that would help, as we'd have a single struct device 
in that case (associated with the top-level DT node), so runtime PM couldn't 
be used to manage clocks independently.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Geert Uytterhoeven]

Hi Laurent,

CC linux-pm

On Tue, Oct 27, 2015 at 2:34 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 26 October 2015 20:02:45 Geert Uytterhoeven wrote:
>> On Fri, Oct 23, 2015 at 1:10 PM, Laurent Pinchart wrote:
>> > On Friday 16 October 2015 14:49:16 Geert Uytterhoeven wrote:
>> >> On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
>> >> Generator) and MSSR (Module Standby and Software Reset) blocks are
>> >> intimately connected, and share the same register block.
>> >>
>> >> Hence it makes sense to describe these two blocks using a
>> >> single device node in DT, instead of using a hierarchical structure with
>> >> multiple nodes, using a mix of generic and SoC-specific bindings.
>> >>
>> >> These new DT bindings are intended to replace the existing DT bindings
>> >> for CPG core clocks ("renesas,*-cpg-clocks", "renesas,cpg-div6-clock")
>> >> and module clocks ("renesas,*-mstp-clocks"), at least for new SoCs.
>> >>
>> >> This will make it easier to add module reset support later, which is
>> >> currently not implemented, and difficult to achieve using the existing
>> >> bindings due to the intertwined register layout.
>> >>
>> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> >>
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
>> >> @@ -0,0 +1,71 @@
>> >> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
>> >> +
>> >> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
>> >> Generator)
>> >> +and MSSR (Module Standby and Software Reset) blocks are intimately
>> >> connected,
>> >> +and share the same register block.
>> >> +
>> >> +They provide the following functionalities:
>> >> +  - The CPG block generates various core clocks,
>> >> +  - The MSSR block provides two functions:
>> >> +      1. Module Standby, providing a Clock Domain to control the clock
>> >> supply
>> >> +      to individual SoC devices,
>> >> +      2. Reset Control, to perform a software reset of individual SoC
>> >> devices.
>>
>> [...]
>>
>> >> +  - #power-domain-cells: Must be 0
>> >> +      - SoC devices that are part of the CPG/MSSR Clock Domain and can
>> >> be
>> >> +     power-managed through Module Standby should refer to the CPG device
>> >> +     node in their "power-domains" property, as documented by the
>> >> generic PM +     Domain bindings in
>> >> +     Documentation/devicetree/bindings/power/power_domain.txt.
>> >> +
>> >> +
>> >> +Examples
>> >> +--------
>> >> +
>> >> +  - CPG device node:
>> >> +
>> >> +     cpg: clock-controller@e6150000 {
>> >> +             compatible = "renesas,r8a7795-cpg-mssr";
>> >> +             reg = <0 0xe6150000 0 0x1000>;
>> >> +             clocks = <&extal_clk>, <&extalr_clk>;
>> >> +             clock-names = "extal", "extalr";
>> >> +             #clock-cells = <2>;
>> >> +             #power-domain-cells = <0>;
>> >> +     };
>> >> +
>> >> +
>> >
>> >> +  - CPG/MSSR Clock Domain member device node:
>> > Would it make sense to show two examples, one for a device whose driver
>> > manages the MSTP clock manually, and another one for a device whose driver
>> > delegates that to the power domain ?
>> >
>> > I hate using the word driver in DT bindings, but unfortunately that's
>> > essentially what it boils down to here as the decision to specify the
>> > power domain is really based on the driver, not on the hardware.
>>
>> IMHO it's not the driver, but the on-SoC module and its representation in
>> DT. Cfr. commit 797a0626e08ca4af ("ARM: shmobile: r8a7791 dtsi: Add
>> CPG/MSTP Clock Domain", which states:
>>
>> |   Add "power-domains" properties to all device nodes for devices that are
>> |   part of the CPG/MSTP Clock Domain and can be power-managed through an
>> |   MSTP clock.  This applies to most on-SoC devices, which have a
>> |   one-to-one mapping from SoC device to DT device node.  Notable
>> |   exceptions are the "display" and "sound" nodes, which represent multiple
>> |   SoC devices, each having their own MSTP clocks.
>
> You're quoting your own documentation to support your point, that's not fair
> :-)

I quoted it because it explains what was done.

> We're using power domains to gate clocks. The fact that it's not related to
> power supplies can already be borderline, but I can buy the argument that
> clocks relate to power consumption here. However, where the power domain
> abstraction is really abused is that we're adding all kind of devices to a
> single power domain while they're controlled by one clock gate each. That's a
> software hack, and we're using DT to tell whether our core code should control
> clock gating or not. That's not a hardware description, sorry.

IMHO the "power-domains" property naming is wrong: it should have been
"pm-domains", as it's following the spirit of the Linux PM Domain abstraction.

PM Domain = Collection of devices treated similarly w.r.t. power management

"similarly" here means "using module clocks". And yes the datasheet states
that's what the module clocks do: control supply of the clock signal to the
module.

>> If a single device block (sharing the same register space), represented in
>> DT as a single device node, actually represents multiple modules, there's no
>> one-to-one mapping from SoC device to DT device node.
>>
>> Hence the device node will have multiple module clocks, and the driver will
>> have to take care of managing them explicitly.
>
> There can be other reasons why the clocks need to be controlled explicitly by
> the driver. At the end of the end it's a per-driver decision whether it wants
> to delegate clock management to runtime PM (which will be the default cause)
> or handle it itself.

The driver can still override if it feels the need.

>> The register space sharing is an SoC-specific issue.
>> The single device node is a DT binding issue.
>>
>> Perhaps such devices should use subnodes, so each of them can represent a
>> module, each with its own module clock.
>
> I'm not sure to see how that would help, as we'd have a single struct device
> in that case (associated with the top-level DT node), so runtime PM couldn't
> be used to manage clocks independently.

The driver for the subnode device can still create child devices, can't it?

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Geert Uytterhoeven]

On Thu, Oct 22, 2015 at 2:58 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>   - Calling platform_driver_probe() from arch_initcall() is too late, as the
>     IRQC is initialized first (it's located before the CPG in .dtsi).
>     Hence the IRQC can't find it's Clock Domain, and its probe is deferred.
>     IRQC will be reprobed later, but in the mean time the Ethernet PHY can't
>     find its IRQ, as the of_mdio code uses irq_of_parse_and_map(), which
>     plainly ignores EPROBE_DEFER :-(
>     Nevertheless, Ethernet works...

To correct myself: renesas-irqc is initialized first because it uses
postcore_initcall().

The of_mdio issue on R-Car Gen2 boards can be worked around by changing that
to device_initcall(). That would cause a few more probe deferrals on R-Mobile
APE6 (r8a73a4), where IRQC is not only the external interrupt controller,
but also the parent interrupt controller of the PFC/GPIO combo.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

[Laurent Pinchart]

Hi Geert,

On Monday 26 October 2015 09:03:44 Geert Uytterhoeven wrote:
> On Mon, Oct 26, 2015 at 3:25 AM, Laurent Pinchart wrote:
> > On Saturday 24 October 2015 19:34:03 Geert Uytterhoeven wrote:
> >> On Sat, Oct 24, 2015 at 3:10 AM, Stephen Boyd wrote:
> >> > On 10/22, Geert Uytterhoeven wrote:
> >> >> As I want to have as much clock data/code __init as possible (think
> >> >> multi-platform kernels --- pinmux data is a disaster here), I have to
> >> >> use platform_driver_probe().
> > 
> > That sounds like an __init issue, doesn't it ? The CPG driver will always
> > be builtin and probed during the init process, what's preventing us from
> > using normal driver probing ?
> 
> When using platform_driver_register(), the tables cannot be __init, as that
> would cause a section type mismatch. Remember, the driver core handles
> platform devices appearing later, so .probe() should continue to be
> available.

Of course, my bad.

> Note: in theory it should be possible to compile the CPG/MSSR driver as a
> module, and have the module in your initramfs. But I don't think anyone
> really wants to do that?

I don't think we should allow that, no.

> >> For new SoCs like r8a7795 we can probably just make it a real platform
> >> driver, and make sure the irqc node is located after the cpg node in the
> >> .dtsi.
> > 
> > That's another hack :-) We really shouldn't depend on DT nodes order.
> 
> I agree. But if there's an unfixed bug somewhere else, we cannot introduce
> regressions (for already supported SoCs).

Sure, and I'm fine relying on DT nodes order as a short term hack, but we need 
to design a proper solution for the longer term.

> > I'm all for getting rid of CLK_OF_DECLARE
> 
> Me too.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/5] [RFC] clk: shmobile: Add new Renesas CPG/MSSR DT bindings

[Laurent Pinchart]

Hi Geert,

On Tuesday 27 October 2015 09:14:15 Geert Uytterhoeven wrote:
> On Tue, Oct 27, 2015 at 2:34 AM, Laurent Pinchart wrote:
> > On Monday 26 October 2015 20:02:45 Geert Uytterhoeven wrote:
> >> On Fri, Oct 23, 2015 at 1:10 PM, Laurent Pinchart wrote:
> >>> On Friday 16 October 2015 14:49:16 Geert Uytterhoeven wrote:
> >>>> On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
> >>>> Generator) and MSSR (Module Standby and Software Reset) blocks are
> >>>> intimately connected, and share the same register block.
> >>>> 
> >>>> Hence it makes sense to describe these two blocks using a
> >>>> single device node in DT, instead of using a hierarchical structure
> >>>> with multiple nodes, using a mix of generic and SoC-specific bindings.
> >>>> 
> >>>> These new DT bindings are intended to replace the existing DT bindings
> >>>> for CPG core clocks ("renesas,*-cpg-clocks", "renesas,cpg-div6-clock")
> >>>> and module clocks ("renesas,*-mstp-clocks"), at least for new SoCs.
> >>>> 
> >>>> This will make it easier to add module reset support later, which is
> >>>> currently not implemented, and difficult to achieve using the existing
> >>>> bindings due to the intertwined register layout.
> >>>> 
> >>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>> 
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
> >>>> @@ -0,0 +1,71 @@
> >>>> +* Renesas Clock Pulse Generator / Module Standby and Software Reset
> >>>> +
> >>>> +On Renesas ARM SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse
> >>>> Generator)
> >>>> +and MSSR (Module Standby and Software Reset) blocks are intimately
> >>>> connected,
> >>>> +and share the same register block.
> >>>> +
> >>>> +They provide the following functionalities:
> >>>> +  - The CPG block generates various core clocks,
> >>>> +  - The MSSR block provides two functions:
> >>>> +      1. Module Standby, providing a Clock Domain to control the
> >>>> clock supply
> >>>> +      to individual SoC devices,
> >>>> +      2. Reset Control, to perform a software reset of individual SoC
> >>>> devices.
> >> 
> >> [...]
> >> 
> >>>> +  - #power-domain-cells: Must be 0
> >>>> +      - SoC devices that are part of the CPG/MSSR Clock Domain and
> >>>> can be
> >>>> +     power-managed through Module Standby should refer to the CPG
> >>>> device
> >>>> +     node in their "power-domains" property, as documented by the
> >>>> generic PM
> >>>> +     Domain bindings in
> >>>> +     Documentation/devicetree/bindings/power/power_domain.txt.
> >>>> +
> >>>> +
> >>>> +Examples
> >>>> +--------
> >>>> +
> >>>> +  - CPG device node:
> >>>> +
> >>>> +     cpg: clock-controller@e6150000 {
> >>>> +             compatible = "renesas,r8a7795-cpg-mssr";
> >>>> +             reg = <0 0xe6150000 0 0x1000>;
> >>>> +             clocks = <&extal_clk>, <&extalr_clk>;
> >>>> +             clock-names = "extal", "extalr";
> >>>> +             #clock-cells = <2>;
> >>>> +             #power-domain-cells = <0>;
> >>>> +     };
> >> >> +
> >> >> +
> >> > 
> >> >> +  - CPG/MSSR Clock Domain member device node:
> >>>
> >>> Would it make sense to show two examples, one for a device whose driver
> >>> manages the MSTP clock manually, and another one for a device whose
> >>> driver delegates that to the power domain ?
> >>> 
> >>> I hate using the word driver in DT bindings, but unfortunately that's
> >>> essentially what it boils down to here as the decision to specify the
> >>> power domain is really based on the driver, not on the hardware.
> >> 
> >> IMHO it's not the driver, but the on-SoC module and its representation in
> >> DT. Cfr. commit 797a0626e08ca4af ("ARM: shmobile: r8a7791 dtsi: Add
> >> 
> >> CPG/MSTP Clock Domain", which states:
> >> |   Add "power-domains" properties to all device nodes for devices that
> >> |   are part of the CPG/MSTP Clock Domain and can be power-managed
> >> |   through an MSTP clock.  This applies to most on-SoC devices, which
> >> |   have a one-to-one mapping from SoC device to DT device node.  Notable
> >> |   exceptions are the "display" and "sound" nodes, which represent
> >> |   multiple SoC devices, each having their own MSTP clocks.
> > 
> > You're quoting your own documentation to support your point, that's not
> > fair :-)
> 
> I quoted it because it explains what was done.
> 
> > We're using power domains to gate clocks. The fact that it's not related
> > to power supplies can already be borderline, but I can buy the argument
> > that clocks relate to power consumption here. However, where the power
> > domain abstraction is really abused is that we're adding all kind of
> > devices to a single power domain while they're controlled by one clock
> > gate each. That's a software hack, and we're using DT to tell whether our
> > core code should control clock gating or not. That's not a hardware
> > description, sorry.
> 
> IMHO the "power-domains" property naming is wrong: it should have been
> "pm-domains", as it's following the spirit of the Linux PM Domain
> abstraction.
> 
> PM Domain = Collection of devices treated similarly w.r.t. power management

Doesn't treated refer to how the Linux kernel software implementation treats 
the devices ?

> "similarly" here means "using module clocks". And yes the datasheet states
> that's what the module clocks do: control supply of the clock signal to the
> module.
> 
> >> If a single device block (sharing the same register space), represented
> >> in DT as a single device node, actually represents multiple modules,
> >> there's no one-to-one mapping from SoC device to DT device node.
> >> 
> >> Hence the device node will have multiple module clocks, and the driver
> >> will have to take care of managing them explicitly.
> > 
> > There can be other reasons why the clocks need to be controlled explicitly
> > by the driver. At the end of the end it's a per-driver decision whether
> > it wants to delegate clock management to runtime PM (which will be the
> > default cause) or handle it itself.
> 
> The driver can still override if it feels the need.

How should it do so by the way ?

> >> The register space sharing is an SoC-specific issue.
> >> The single device node is a DT binding issue.
> >> 
> >> Perhaps such devices should use subnodes, so each of them can represent a
> >> module, each with its own module clock.
> > 
> > I'm not sure to see how that would help, as we'd have a single struct
> > device in that case (associated with the top-level DT node), so runtime
> > PM couldn't be used to manage clocks independently.
> 
> The driver for the subnode device can still create child devices, can't it?

I think it could, yes, but I'm not sure if that would really simplify much 
compared to having three independent DT nodes. It could be experimented 
though. The LVDS transmitter in particular should probably be a separate DT 
node, linked to the DU using phandles.

-- 
Regards,

Laurent Pinchart