[PATCH v4 00/05] Renesas R-Car Gen3 CPG support V4

[PATCH v4 00/05] Renesas R-Car Gen3 CPG support V4

[Magnus Damm]

Renesas R-Car Gen3 CPG support V4

[PATCH v4 01/05] clk: shmobile: Rework CONFIG_ARCH_SHMOBILE_MULTI
[PATCH v4 02/05] clk: shmobile: Add Renesas R-Car Gen3 CPG support
[PATCH v4 03/05] clk: shmobile: rcar-gen3: Add CPG/MSTP Clock Domain support
[PATCH v4 04/05] clk: shmobile: Add r8a7795 SoC to MSTP bindings
[PATCH v4 05/05] clk: shmobile: Make MSTP clock-output-names optional

This series is my second attempt to provide R-Car Gen3 clock support
through a single "easy to use" patch series for drivers/clk and DT
documentation bits.

There are no special runtime or build time dependencies. SoC / board
specific integration code (DTS) is provided separately from this series.

This version includes several minor updates for all the patches.
For details please see each individual patch.

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

 Written to be picked up independently from other R-Car Gen3 patches.

 Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt      |    1 
 Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt |   57 ++
 drivers/clk/Makefile                                                     |    1 
 drivers/clk/shmobile/Makefile                                            |   23 
 drivers/clk/shmobile/clk-mstp.c                                          |   28 -
 drivers/clk/shmobile/clk-rcar-gen3.c                                     |  251 ++++++++++
 6 files changed, 338 insertions(+), 23 deletions(-)
damm@little-apple ~/patches/20150829/gen3-clk-v4 $ 

[PATCH v4 01/05] clk: shmobile: Rework CONFIG_ARCH_SHMOBILE_MULTI

[Magnus Damm]

From: Magnus Damm <damm+renesas@opensource.se>

Shmobile is all multiplatform these days, so in get rid of the
reference to CONFIG_ARCH_SHMOBILE_MULTI in drivers/clk/shmobile/.

Also instead of always enabling DIV6 and MSTP adjust the Makefile
to enable DIV6 and MSTP depending on if they are included in the
SoC or not.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---

 Earlier known as "clk: shmobile: Get rid of CONFIG_ARCH_SHMOBILE_MULTI"

 Changes since V3:
 - Got rid of hunks that modified drivers/clk/Makefile
 - This to prevent build issue with non-CCF enabled SH arch
 - A nice side effect is that this patch now can be merged any time

 Changes since V2:
 - Fixed patch subject typo CONFIG_SHMOBILE_MULTI -> CONFIG_ARCH_SHMOBILE_MULTI

 drivers/clk/shmobile/Makefile |   22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

--- 0001/drivers/clk/shmobile/Makefile
+++ work/drivers/clk/shmobile/Makefile	2015-08-29 16:14:57.522366518 +0900
@@ -1,13 +1,11 @@
 obj-$(CONFIG_ARCH_EMEV2)		+= clk-emev2.o
-obj-$(CONFIG_ARCH_R7S72100)		+= clk-rz.o
-obj-$(CONFIG_ARCH_R8A73A4)		+= clk-r8a73a4.o
-obj-$(CONFIG_ARCH_R8A7740)		+= clk-r8a7740.o
-obj-$(CONFIG_ARCH_R8A7778)		+= clk-r8a7778.o
-obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
-obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o
-obj-$(CONFIG_ARCH_R8A7791)		+= clk-rcar-gen2.o
-obj-$(CONFIG_ARCH_R8A7793)		+= clk-rcar-gen2.o
-obj-$(CONFIG_ARCH_R8A7794)		+= clk-rcar-gen2.o
-obj-$(CONFIG_ARCH_SH73A0)		+= clk-sh73a0.o
-obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= clk-div6.o
-obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= clk-mstp.o
+obj-$(CONFIG_ARCH_R7S72100)		+= clk-rz.o clk-mstp.o
+obj-$(CONFIG_ARCH_R8A73A4)		+= clk-r8a73a4.o clk-mstp.o clk-div6.o
+obj-$(CONFIG_ARCH_R8A7740)		+= clk-r8a7740.o clk-mstp.o clk-div6.o
+obj-$(CONFIG_ARCH_R8A7778)		+= clk-r8a7778.o clk-mstp.o
+obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o clk-mstp.o
+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_SH73A0)		+= clk-sh73a0.o clk-mstp.o clk-div6.o

[PATCH v4 02/05] clk: shmobile: Add Renesas R-Car Gen3 CPG support

[Magnus Damm]

From: Gaku Inami <gaku.inami.xw@bp.renesas.com>

This patch adds initial CPG support for R-Car Generation 3 and in
particular the R8A7795 SoC.

The R-Car Gen3 clock hardware has a register write protection feature that
needs to be shared between the CPG function needs to be shared between
the CPG and MSTP hardware somehow. So far this feature is simply ignored.

Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Changes since V3: (Magnus Damm <damm+renesas@opensource.se>)
 - Reworked driver to incorporate most feedback from Stephen Boyd - thanks!!
 - Major things like syscon and driver model require more discussion.
 - Added hunk to build drivers/clk/shmobile if ARCH_RENESAS is set.

 Changes since V2: (Magnus Damm <damm+renesas@opensource.se>)
 - Reworked driver to rely on clock index instead of strings.
 - Dropped use of "clock-output-names".

 Earlier versions: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
 Initial version: Gaku Inami <gaku.inami.xw@bp.renesas.com>

 Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt |   31 +
 drivers/clk/Makefile                                                     |    1 
 drivers/clk/shmobile/Makefile                                            |    1 
 drivers/clk/shmobile/clk-rcar-gen3.c                                     |  249 ++++++++++
 4 files changed, 282 insertions(+)

--- /dev/null
+++ work/Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt	2015-08-29 16:23:54.982366518 +0900
@@ -0,0 +1,31 @@
+* Renesas R-Car Gen3 Clock Pulse Generator (CPG)
+
+The CPG generates core clocks for the R-Car Gen3 SoCs. It includes three PLLs
+and several fixed ratio dividers.
+
+Required Properties:
+
+  - compatible: Must be one of
+    - "renesas,r8a7795-cpg-clocks" for the r8a7795 CPG
+    - "renesas,rcar-gen3-cpg-clocks" for the generic R-Car Gen3 CPG
+
+  - reg: Base address and length of the memory resource used by the CPG
+
+  - clocks: References to the parent clocks: first to the EXTAL clock
+  - #clock-cells: Must be 1
+  - clock-output-names: The names of the clocks. Supported clocks are
+    "main", "pll0", "pll1", "pll2", "pll3", "pll4"
+
+
+Example
+-------
+
+	cpg_clocks: cpg_clocks@e6150000 {
+		compatible = "renesas,r8a7795-cpg-clocks",
+			     "renesas,rcar-gen3-cpg-clocks";
+		reg = <0 0xe6150000 0 0x1000>;
+		clocks = <&extal_clk>;
+		#clock-cells = <1>;
+		clock-output-names = "main", "pll0", "pll1","pll2",
+				     "pll3", "pll4";
+	};
--- 0001/drivers/clk/Makefile
+++ work/drivers/clk/Makefile	2015-08-29 16:34:47.652366518 +0900
@@ -67,6 +67,7 @@ obj-$(CONFIG_COMMON_CLK_QCOM)		+= qcom/
 obj-$(CONFIG_ARCH_ROCKCHIP)		+= rockchip/
 obj-$(CONFIG_COMMON_CLK_SAMSUNG)	+= samsung/
 obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= shmobile/
+obj-$(CONFIG_ARCH_RENESAS)		+= shmobile/
 obj-$(CONFIG_ARCH_SIRF)			+= sirf/
 obj-$(CONFIG_ARCH_SOCFPGA)		+= socfpga/
 obj-$(CONFIG_PLAT_SPEAR)		+= spear/
--- 0009/drivers/clk/shmobile/Makefile
+++ work/drivers/clk/shmobile/Makefile	2015-08-29 16:23:53.952366518 +0900
@@ -8,4 +8,5 @@ obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-
 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-mstp.o clk-div6.o
 obj-$(CONFIG_ARCH_SH73A0)		+= clk-sh73a0.o clk-mstp.o clk-div6.o
--- /dev/null
+++ work/drivers/clk/shmobile/clk-rcar-gen3.c	2015-08-29 16:30:59.032366518 +0900
@@ -0,0 +1,249 @@
+/*
+ * rcar_gen3 Core CPG Clocks
+ *
+ * Copyright (C) 2015 Renesas Electronics Corp.
+ *
+ * Based on rcar_gen2 Core CPG Clocks driver.
+ *
+ * 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-provider.h>
+#include <linux/clk/shmobile.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#define RCAR_GEN3_CLK_MAIN	0
+#define RCAR_GEN3_CLK_PLL0	1
+#define RCAR_GEN3_CLK_PLL1	2
+#define RCAR_GEN3_CLK_PLL2	3
+#define RCAR_GEN3_CLK_PLL3	4
+#define RCAR_GEN3_CLK_PLL4	5
+
+static const char * const rcar_gen3_clk_names[] = {
+	[RCAR_GEN3_CLK_MAIN] = "main",
+	[RCAR_GEN3_CLK_PLL0] = "pll0",
+	[RCAR_GEN3_CLK_PLL1] = "pll1",
+	[RCAR_GEN3_CLK_PLL2] = "pll2",
+	[RCAR_GEN3_CLK_PLL3] = "pll3",
+	[RCAR_GEN3_CLK_PLL4] = "pll4",
+};
+
+struct rcar_gen3_cpg {
+	struct clk_onecell_data data;
+	spinlock_t lock;
+	void __iomem *reg;
+};
+
+#define CPG_PLL0CR	0x00d8
+#define CPG_PLL2CR	0x002c
+
+/*
+ * common function
+ */
+#define rcar_clk_readl(cpg, _reg) readl(cpg->reg + _reg)
+
+/*
+ * Reset register definitions.
+ */
+#define MODEMR 0xe6160060
+
+static u32 rcar_gen3_read_mode_pins(void)
+{
+	static u32 mode;
+	static bool mode_valid;
+
+	if (!mode_valid) {
+		void __iomem *modemr = ioremap_nocache(MODEMR, 4);
+
+		BUG_ON(!modemr);
+		mode = ioread32(modemr);
+		iounmap(modemr);
+		mode_valid = true;
+	}
+
+	return mode;
+}
+
+/* -----------------------------------------------------------------------------
+ * CPG Clock Data
+ */
+
+/*
+ *   MD		EXTAL		PLL0	PLL1	PLL2	PLL3	PLL4
+ * 14 13 19 17	(MHz)		*1	*1	*1
+ *-------------------------------------------------------------------
+ * 0  0  0  0	16.66 x 1	x180/2	x192/2	x144/2	x192	x144
+ * 0  0  0  1	16.66 x 1	x180/2	x192/2	x144/2	x128	x144
+ * 0  0  1  0	Prohibited setting
+ * 0  0  1  1	16.66 x 1	x180/2	x192/2	x144/2	x192	x144
+ * 0  1  0  0	20    x 1	x150/2	x156/2	x120/2	x156	x120
+ * 0  1  0  1	20    x 1	x150/2	x156/2	x120/2	x106	x120
+ * 0  1  1  0	Prohibited setting
+ * 0  1  1  1	20    x 1	x150/2	x156/2	x120/2	x156	x120
+ * 1  0  0  0	25    x 1	x120/2	x128/2	x96/2	x128	x96
+ * 1  0  0  1	25    x 1	x120/2	x128/2	x96/2	x84	x96
+ * 1  0  1  0	Prohibited setting
+ * 1  0  1  1	25    x 1	x120/2	x128/2	x96/2	x128	x96
+ * 1  1  0  0	33.33 / 2	x180/2	x192/2	x144/2	x192	x144
+ * 1  1  0  1	33.33 / 2	x180/2	x192/2	x144/2	x128	x144
+ * 1  1  1  0	Prohibited setting
+ * 1  1  1  1	33.33 / 2	x180/2	x192/2	x144/2	x192	x144
+ *
+ * *1 : datasheet indicates VCO output (PLLx = VCO/2)
+ *
+ */
+#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, },
+};
+
+/* -----------------------------------------------------------------------------
+ * Initialization
+ */
+
+static struct clk * __init
+rcar_gen3_cpg_register_clock(struct device_node *np, struct rcar_gen3_cpg *cpg,
+			     const struct cpg_pll_config *config,
+			     unsigned int gen3_clk)
+{
+	const char *parent_name = rcar_gen3_clk_names[RCAR_GEN3_CLK_MAIN];
+	unsigned int mult = 1;
+	unsigned int div = 1;
+	u32 value;
+
+	switch (gen3_clk) {
+	case RCAR_GEN3_CLK_MAIN:
+		parent_name = of_clk_get_parent_name(np, 0);
+		div = config->extal_div;
+		break;
+	case RCAR_GEN3_CLK_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 = rcar_clk_readl(cpg, CPG_PLL0CR);
+		mult = ((value >> 24) & ((1 << 7) - 1)) + 1;
+		break;
+	case RCAR_GEN3_CLK_PLL1:
+		mult = config->pll1_mult / 2;
+		break;
+	case RCAR_GEN3_CLK_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 = rcar_clk_readl(cpg, CPG_PLL2CR);
+		mult = ((value >> 24) & ((1 << 7) - 1)) + 1;
+		break;
+	case RCAR_GEN3_CLK_PLL3:
+		mult = config->pll3_mult;
+		break;
+	case RCAR_GEN3_CLK_PLL4:
+		mult = config->pll4_mult;
+		break;
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+
+	return clk_register_fixed_factor(NULL, rcar_gen3_clk_names[gen3_clk],
+					 parent_name, 0, mult, div);
+}
+
+static void __init rcar_gen3_cpg_clocks_init(struct device_node *np)
+{
+	const struct cpg_pll_config *config;
+	struct rcar_gen3_cpg *cpg;
+	struct clk **clks;
+	u32 cpg_mode;
+	unsigned int i;
+	int num_clks;
+
+	cpg_mode = rcar_gen3_read_mode_pins();
+
+	num_clks = of_property_count_strings(np, "clock-indices");
+	if (num_clks < 0) {
+		pr_err("%s: failed to count clocks\n", __func__);
+		return;
+	}
+
+	cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
+	clks = kcalloc(num_clks, sizeof(*clks), GFP_KERNEL);
+	if (cpg = NULL || clks = NULL) {
+		kfree(cpg);
+		kfree(clks);
+		pr_err("%s: failed to allocate cpg\n", __func__);
+		return;
+	}
+
+	spin_lock_init(&cpg->lock);
+
+	cpg->data.clks = clks;
+	cpg->data.clk_num = num_clks;
+
+	cpg->reg = of_iomap(np, 0);
+	if (WARN_ON(cpg->reg = NULL))
+		return;
+
+	config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
+	if (!config->extal_div) {
+		pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n",
+		       __func__, cpg_mode);
+		return;
+	}
+
+	for (i = 0; i < num_clks; ++i) {
+		struct clk *clk;
+		u32 idx;
+		int ret;
+
+		ret = of_property_read_u32_index(np, "clock-indices", i, &idx);
+		if (ret < 0)
+			break;
+
+		clk = rcar_gen3_cpg_register_clock(np, cpg, config, idx);
+		if (IS_ERR(clk))
+			pr_err("%s: failed to register %s %u clock (%ld)\n",
+			       __func__, np->name, idx, PTR_ERR(clk));
+		else
+			cpg->data.clks[idx] = clk;
+	}
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &cpg->data);
+}
+CLK_OF_DECLARE(rcar_gen3_cpg_clks, "renesas,rcar-gen3-cpg-clocks",
+	       rcar_gen3_cpg_clocks_init);

[PATCH v4 03/05] clk: shmobile: rcar-gen3: Add CPG/MSTP Clock Domain support

[Magnus Damm]

From: Geert Uytterhoeven <geert+renesas@glider.be>

Add Clock Domain support to the R-Car Gen3 Clock Pulse Generator (CPG)
driver using the generic PM Domain.  This allows to power-manage the
module clocks of SoC devices that are part of the CPG/MSTP Clock Domain
using Runtime PM, or for system suspend/resume.

SoC devices that are part of the CPG/MSTP Clock Domain and can be
power-managed through an MSTP clock should be tagged in DT with a proper
"power-domains" property.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Changes since V3:
 - Fixed so correct file is actually used. =)
 - Took V2 from Geert but filtered out Kconfig.platforms bits

 Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt |   26 +++++++++-
 drivers/clk/shmobile/clk-rcar-gen3.c                                     |    2 
 2 files changed, 26 insertions(+), 2 deletions(-)

--- 0011/Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt
+++ work/Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt	2015-08-29 16:41:04.842366518 +0900
@@ -2,6 +2,8 @@
 
 The CPG generates core clocks for the R-Car Gen3 SoCs. It includes three PLLs
 and several fixed ratio dividers.
+The CPG also provides a Clock Domain for SoC devices, in combination with the
+CPG Module Stop (MSTP) Clocks.
 
 Required Properties:
 
@@ -15,10 +17,18 @@ Required Properties:
   - #clock-cells: Must be 1
   - clock-output-names: The names of the clocks. Supported clocks are
     "main", "pll0", "pll1", "pll2", "pll3", "pll4"
+  - #power-domain-cells: Must be 0
 
+SoC devices that are part of the CPG/MSTP Clock Domain and can be power-managed
+through an MSTP clock 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.
 
-Example
--------
+
+Examples
+--------
+
+  - CPG device node:
 
 	cpg_clocks: cpg_clocks@e6150000 {
 		compatible = "renesas,r8a7795-cpg-clocks",
@@ -28,4 +38,16 @@ Example
 		#clock-cells = <1>;
 		clock-output-names = "main", "pll0", "pll1","pll2",
 				     "pll3", "pll4";
+		#power-domain-cells = <0>;
+	};
+
+  - CPG/MSTP 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 = <&mstp3_clks RCAR_R8A7795_CLK_SCIF2>;
+		clock-names = "sci_ick";
+		power-domains = <&cpg_clocks>;
 	};
--- 0011/drivers/clk/shmobile/clk-rcar-gen3.c
+++ work/drivers/clk/shmobile/clk-rcar-gen3.c	2015-08-29 16:41:04.842366518 +0900
@@ -244,6 +244,8 @@ static void __init rcar_gen3_cpg_clocks_
 	}
 
 	of_clk_add_provider(np, of_clk_src_onecell_get, &cpg->data);
+
+	cpg_mstp_add_clk_domain(np);
 }
 CLK_OF_DECLARE(rcar_gen3_cpg_clks, "renesas,rcar-gen3-cpg-clocks",
 	       rcar_gen3_cpg_clocks_init);

[PATCH v4 04/05] clk: shmobile: Add r8a7795 SoC to MSTP bindings

[Magnus Damm]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Add r8a7795 to the MSTP DT binding documentation.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Changes since V3: (Magnus Damm <damm+renesas@opensource.se>)
 - Picked up patch floating around on ML, added change log

 Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt |    1 +
 1 file changed, 1 insertion(+)

--- 0001/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
+++ work/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt	2015-08-29 16:18:30.672366518 +0900
@@ -19,6 +19,7 @@ Required Properties:
     - "renesas,r8a7791-mstp-clocks" for R8A7791 (R-Car M2-W) MSTP gate clocks
     - "renesas,r8a7793-mstp-clocks" for R8A7793 (R-Car M2-N) MSTP gate clocks
     - "renesas,r8a7794-mstp-clocks" for R8A7794 (R-Car E2) MSTP gate clocks
+    - "renesas,r8a7795-mstp-clocks" for R8A7795 (R-Car H3) MSTP gate clocks
     - "renesas,sh73a0-mstp-clocks" for SH73A0 (SH-MobileAG5) MSTP gate clocks
     and "renesas,cpg-mstp-clocks" as a fallback.
   - reg: Base address and length of the I/O mapped registers used by the MSTP

[PATCH v4 05/05] clk: shmobile: Make MSTP clock-output-names optional

[Magnus Damm]

From: Magnus Damm <damm+renesas@opensource.se>

This patch updates the MSTP driver to make the "clock-output-names"
DT property optional instead of mandatory. In case the DT property
is omitted then the function clk_register_clkdev() is skipped.

The default for new SoCs is to not use "clock-output-names".

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

 Changes since V3:
 - Added support for multiple MSTP bits. =)

 drivers/clk/shmobile/clk-mstp.c |   28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

--- 0001/drivers/clk/shmobile/clk-mstp.c
+++ work/drivers/clk/shmobile/clk-mstp.c	2015-08-29 18:01:00.662366518 +0900
@@ -199,26 +199,29 @@ static void __init cpg_mstp_clocks_init(
 	for (i = 0; i < MSTP_MAX_CLOCKS; ++i) {
 		const char *parent_name;
 		const char *name;
+		bool allocated_name;
 		u32 clkidx;
 		int ret;
 
-		/* Skip clocks with no name. */
-		ret = of_property_read_string_index(np, "clock-output-names",
-						    i, &name);
-		if (ret < 0 || strlen(name) = 0)
-			continue;
-
 		parent_name = of_clk_get_parent_name(np, i);
 		ret = of_property_read_u32_index(np, idxname, i, &clkidx);
 		if (parent_name = NULL || ret < 0)
 			break;
 
 		if (clkidx >= MSTP_MAX_CLOCKS) {
-			pr_err("%s: invalid clock %s %s index %u)\n",
-			       __func__, np->name, name, clkidx);
+			pr_err("%s: invalid clock %s index %u)\n",
+			       __func__, np->name, clkidx);
 			continue;
 		}
 
+		if (of_property_read_string_index(np, "clock-output-names",
+						  i, &name) = 0) {
+			allocated_name = false;
+		} else {
+			name = kasprintf(GFP_KERNEL, "%s.%u", np->name, clkidx);
+			allocated_name = true;
+		}
+
 		clks[clkidx] = cpg_mstp_clock_register(name, parent_name,
 						       clkidx, group);
 		if (!IS_ERR(clks[clkidx])) {
@@ -231,12 +234,19 @@ static void __init cpg_mstp_clocks_init(
 			 *
 			 * FIXME: Remove this when all devices that require a
 			 * clock will be instantiated from DT.
+			 *
+			 * We currently register clkdev only when the DT
+			 * property clock-output-names is present.
 			 */
-			clk_register_clkdev(clks[clkidx], name, NULL);
+			if (!allocated_name)
+				clk_register_clkdev(clks[clkidx], name, NULL);
 		} else {
 			pr_err("%s: failed to register %s %s clock (%ld)\n",
 			       __func__, np->name, name, PTR_ERR(clks[clkidx]));
 		}
+
+		if (allocated_name)
+			kfree(name);
 	}
 
 	of_clk_add_provider(np, of_clk_src_onecell_get, &group->data);

Re: [PATCH v4 02/05] clk: shmobile: Add Renesas R-Car Gen3 CPG support

[Magnus Damm]

On Sat, Aug 29, 2015 at 6:13 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> From: Gaku Inami <gaku.inami.xw@bp.renesas.com>
>
> This patch adds initial CPG support for R-Car Generation 3 and in
> particular the R8A7795 SoC.
>
> The R-Car Gen3 clock hardware has a register write protection feature that
> needs to be shared between the CPG function needs to be shared between
> the CPG and MSTP hardware somehow. So far this feature is simply ignored.
>
> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
>
>  Changes since V3: (Magnus Damm <damm+renesas@opensource.se>)
>  - Reworked driver to incorporate most feedback from Stephen Boyd - thanks!!
>  - Major things like syscon and driver model require more discussion.
>  - Added hunk to build drivers/clk/shmobile if ARCH_RENESAS is set.
>
>  Changes since V2: (Magnus Damm <damm+renesas@opensource.se>)
>  - Reworked driver to rely on clock index instead of strings.
>  - Dropped use of "clock-output-names".
>
>  Earlier versions: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>  Initial version: Gaku Inami <gaku.inami.xw@bp.renesas.com>
>
>  Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt |   31 +
>  drivers/clk/Makefile                                                     |    1
>  drivers/clk/shmobile/Makefile                                            |    1
>  drivers/clk/shmobile/clk-rcar-gen3.c                                     |  249 ++++++++++
>  4 files changed, 282 insertions(+)
>
> --- /dev/null
> +++ work/Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt       2015-08-29 16:23:54.982366518 +0900
> @@ -0,0 +1,31 @@
> +* Renesas R-Car Gen3 Clock Pulse Generator (CPG)
> +
> +The CPG generates core clocks for the R-Car Gen3 SoCs. It includes three PLLs
> +and several fixed ratio dividers.
> +
> +Required Properties:
> +
> +  - compatible: Must be one of
> +    - "renesas,r8a7795-cpg-clocks" for the r8a7795 CPG
> +    - "renesas,rcar-gen3-cpg-clocks" for the generic R-Car Gen3 CPG
> +
> +  - reg: Base address and length of the memory resource used by the CPG
> +
> +  - clocks: References to the parent clocks: first to the EXTAL clock
> +  - #clock-cells: Must be 1
> +  - clock-output-names: The names of the clocks. Supported clocks are
> +    "main", "pll0", "pll1", "pll2", "pll3", "pll4"
> +
> +
> +Example
> +-------
> +
> +       cpg_clocks: cpg_clocks@e6150000 {
> +               compatible = "renesas,r8a7795-cpg-clocks",
> +                            "renesas,rcar-gen3-cpg-clocks";
> +               reg = <0 0xe6150000 0 0x1000>;
> +               clocks = <&extal_clk>;
> +               #clock-cells = <1>;
> +               clock-output-names = "main", "pll0", "pll1","pll2",
> +                                    "pll3", "pll4";
> +       };

Seems that I forgot update the DT binding bits. Will fix in next verison.

Thanks,

/ magnus

Re: [PATCH v4 05/05] clk: shmobile: Make MSTP clock-output-names optional

[Geert Uytterhoeven]

On Sat, Aug 29, 2015 at 11:14 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> --- 0001/drivers/clk/shmobile/clk-mstp.c
> +++ work/drivers/clk/shmobile/clk-mstp.c        2015-08-29 18:01:00.662366518 +0900
> @@ -199,26 +199,29 @@ static void __init cpg_mstp_clocks_init(
>         for (i = 0; i < MSTP_MAX_CLOCKS; ++i) {
>                 const char *parent_name;
>                 const char *name;
> +               bool allocated_name;

If you make it "const char *allocated_name = NULL;" ...

>                 u32 clkidx;
>                 int ret;
>
> -               /* Skip clocks with no name. */
> -               ret = of_property_read_string_index(np, "clock-output-names",
> -                                                   i, &name);
> -               if (ret < 0 || strlen(name) = 0)
> -                       continue;
> -
>                 parent_name = of_clk_get_parent_name(np, i);
>                 ret = of_property_read_u32_index(np, idxname, i, &clkidx);
>                 if (parent_name = NULL || ret < 0)
>                         break;
>
>                 if (clkidx >= MSTP_MAX_CLOCKS) {
> -                       pr_err("%s: invalid clock %s %s index %u)\n",
> -                              __func__, np->name, name, clkidx);
> +                       pr_err("%s: invalid clock %s index %u)\n",
> +                              __func__, np->name, clkidx);
>                         continue;
>                 }
>
> +               if (of_property_read_string_index(np, "clock-output-names",
> +                                                 i, &name) = 0) {
> +                       allocated_name = false;

... you can drop this branch...

> +               } else {
> +                       name = kasprintf(GFP_KERNEL, "%s.%u", np->name, clkidx);
> +                       allocated_name = true;

... and use "allocated_name = name = kasprintf(...);"

> +               }
> +
>                 clks[clkidx] = cpg_mstp_clock_register(name, parent_name,
>                                                        clkidx, group);
>                 if (!IS_ERR(clks[clkidx])) {
> @@ -231,12 +234,19 @@ static void __init cpg_mstp_clocks_init(
>                          *
>                          * FIXME: Remove this when all devices that require a
>                          * clock will be instantiated from DT.
> +                        *
> +                        * We currently register clkdev only when the DT
> +                        * property clock-output-names is present.
>                          */
> -                       clk_register_clkdev(clks[clkidx], name, NULL);
> +                       if (!allocated_name)
> +                               clk_register_clkdev(clks[clkidx], name, NULL);
>                 } else {
>                         pr_err("%s: failed to register %s %s clock (%ld)\n",
>                                __func__, np->name, name, PTR_ERR(clks[clkidx]));
>                 }
> +
> +               if (allocated_name)
> +                       kfree(name);

... and kfree(allocated_name) unconditionally.

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 02/05] clk: shmobile: Add Renesas R-Car Gen3 CPG support

[Geert Uytterhoeven]

Hi Magnus,

On Sat, Aug 29, 2015 at 11:13 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> From: Gaku Inami <gaku.inami.xw@bp.renesas.com>
>
> This patch adds initial CPG support for R-Car Generation 3 and in
> particular the R8A7795 SoC.
>
> The R-Car Gen3 clock hardware has a register write protection feature that
> needs to be shared between the CPG function needs to be shared between
> the CPG and MSTP hardware somehow. So far this feature is simply ignored.
>
> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
>
>  Changes since V3: (Magnus Damm <damm+renesas@opensource.se>)
>  - Reworked driver to incorporate most feedback from Stephen Boyd - thanks!!
>  - Major things like syscon and driver model require more discussion.
>  - Added hunk to build drivers/clk/shmobile if ARCH_RENESAS is set.

Thanks for the updates!

>  Changes since V2: (Magnus Damm <damm+renesas@opensource.se>)
>  - Reworked driver to rely on clock index instead of strings.

I had completely missed this part in the previous version...

>  - Dropped use of "clock-output-names".


> --- /dev/null
> +++ work/drivers/clk/shmobile/clk-rcar-gen3.c   2015-08-29 16:30:59.032366518 +0900
> @@ -0,0 +1,249 @@

> +#define RCAR_GEN3_CLK_MAIN     0
> +#define RCAR_GEN3_CLK_PLL0     1
> +#define RCAR_GEN3_CLK_PLL1     2
> +#define RCAR_GEN3_CLK_PLL2     3
> +#define RCAR_GEN3_CLK_PLL3     4
> +#define RCAR_GEN3_CLK_PLL4     5

These values are intimately tied to R8A7795_CLK_* in
include/dt-bindings/clock/r8a7795-clock.h.
Perhaps we can use these instead?

If we ever have a Gen3 SoC with more clocks, we have to update
this anyway.

> +struct rcar_gen3_cpg {
> +       struct clk_onecell_data data;
> +       spinlock_t lock;
> +       void __iomem *reg;

If you add the clocks here, you don't have to allocate them separately
later:

        struct clk clocks[...];

> +static void __init rcar_gen3_cpg_clocks_init(struct device_node *np)
> +{
> +       const struct cpg_pll_config *config;
> +       struct rcar_gen3_cpg *cpg;
> +       struct clk **clks;
> +       u32 cpg_mode;
> +       unsigned int i;
> +       int num_clks;
> +
> +       cpg_mode = rcar_gen3_read_mode_pins();
> +
> +       num_clks = of_property_count_strings(np, "clock-indices");
> +       if (num_clks < 0) {
> +               pr_err("%s: failed to count clocks\n", __func__);
> +               return;
> +       }
> +
> +       cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
> +       clks = kcalloc(num_clks, sizeof(*clks), GFP_KERNEL);

I.e. these 2 allocation can be combined by embedding the struct clks in
struct rcar_gen3_cpg...

> +       if (cpg = NULL || clks = NULL) {
> +               kfree(cpg);
> +               kfree(clks);

... and there's no longer a need to kfree() anything.

> +               pr_err("%s: failed to allocate cpg\n", __func__);

There's no need to print an error on allocation failures. The system
will scream anyway (if there's already a console ;-).

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 04/05] clk: shmobile: Add r8a7795 SoC to MSTP bindings

[Geert Uytterhoeven]

On Sat, Aug 29, 2015 at 11:14 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Add r8a7795 to the MSTP DT binding documentation.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 02/05] clk: shmobile: Add Renesas R-Car Gen3 CPG support

[Magnus Damm]

Hi Geert,

On Mon, Aug 31, 2015 at 5:45 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Sat, Aug 29, 2015 at 11:13 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> From: Gaku Inami <gaku.inami.xw@bp.renesas.com>
>>
>> This patch adds initial CPG support for R-Car Generation 3 and in
>> particular the R8A7795 SoC.
>>
>> The R-Car Gen3 clock hardware has a register write protection feature that
>> needs to be shared between the CPG function needs to be shared between
>> the CPG and MSTP hardware somehow. So far this feature is simply ignored.
>>
>> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
>> ---
>>
>>  Changes since V3: (Magnus Damm <damm+renesas@opensource.se>)
>>  - Reworked driver to incorporate most feedback from Stephen Boyd - thanks!!
>>  - Major things like syscon and driver model require more discussion.
>>  - Added hunk to build drivers/clk/shmobile if ARCH_RENESAS is set.
>
> Thanks for the updates!
>
>>  Changes since V2: (Magnus Damm <damm+renesas@opensource.se>)
>>  - Reworked driver to rely on clock index instead of strings.
>
> I had completely missed this part in the previous version...

No worries, perhaps not so well documented. And there are many changes
floating around...

>>  - Dropped use of "clock-output-names".
>
>
>> --- /dev/null
>> +++ work/drivers/clk/shmobile/clk-rcar-gen3.c   2015-08-29 16:30:59.032366518 +0900
>> @@ -0,0 +1,249 @@
>
>> +#define RCAR_GEN3_CLK_MAIN     0
>> +#define RCAR_GEN3_CLK_PLL0     1
>> +#define RCAR_GEN3_CLK_PLL1     2
>> +#define RCAR_GEN3_CLK_PLL2     3
>> +#define RCAR_GEN3_CLK_PLL3     4
>> +#define RCAR_GEN3_CLK_PLL4     5
>
> These values are intimately tied to R8A7795_CLK_* in
> include/dt-bindings/clock/r8a7795-clock.h.
> Perhaps we can use these instead?

You are right that they are connected together, but I'm not sure if I
prefer to include that file. If we start sharing files between the CPG
and the integration code then a dependency hell breaks loose when it
comes to merge order. If we want to share the #defines then I propose
that we handle that incrementally once all pieces are merged.

On R-Car Gen2 we have a single CPG driver that is potentially shared
across multiple SoCs in the same generation / family. In that case we
use strings and the clock-output-names as part of the ABI if I'm not
mistaken. On this driver we use clock index numbers instead, so I'd
like to see these defines as part of a shared DT binding ABI where new
SoCs can simply add new clocks at the end.

> If we ever have a Gen3 SoC with more clocks, we have to update
> this anyway.

That's right!

>> +struct rcar_gen3_cpg {
>> +       struct clk_onecell_data data;
>> +       spinlock_t lock;
>> +       void __iomem *reg;
>
> If you add the clocks here, you don't have to allocate them separately
> later:
>
>         struct clk clocks[...];
>
>> +static void __init rcar_gen3_cpg_clocks_init(struct device_node *np)
>> +{
>> +       const struct cpg_pll_config *config;
>> +       struct rcar_gen3_cpg *cpg;
>> +       struct clk **clks;
>> +       u32 cpg_mode;
>> +       unsigned int i;
>> +       int num_clks;
>> +
>> +       cpg_mode = rcar_gen3_read_mode_pins();
>> +
>> +       num_clks = of_property_count_strings(np, "clock-indices");
>> +       if (num_clks < 0) {
>> +               pr_err("%s: failed to count clocks\n", __func__);
>> +               return;
>> +       }
>> +
>> +       cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
>> +       clks = kcalloc(num_clks, sizeof(*clks), GFP_KERNEL);
>
> I.e. these 2 allocation can be combined by embedding the struct clks in
> struct rcar_gen3_cpg...
>
>> +       if (cpg = NULL || clks = NULL) {
>> +               kfree(cpg);
>> +               kfree(clks);
>
> ... and there's no longer a need to kfree() anything.

Good idea. I'll give it a shot.

>> +               pr_err("%s: failed to allocate cpg\n", __func__);
>
> There's no need to print an error on allocation failures. The system
> will scream anyway (if there's already a console ;-).

Ok, will make it silent!

Thanks for your comments!!

Cheers,

/ magnus

Re: [PATCH v4 02/05] clk: shmobile: Add Renesas R-Car Gen3 CPG support

[Geert Uytterhoeven]

Hi Magnus,

On Mon, Aug 31, 2015 at 1:58 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> --- /dev/null
>>> +++ work/drivers/clk/shmobile/clk-rcar-gen3.c   2015-08-29 16:30:59.032366518 +0900
>>> @@ -0,0 +1,249 @@
>>
>>> +#define RCAR_GEN3_CLK_MAIN     0
>>> +#define RCAR_GEN3_CLK_PLL0     1
>>> +#define RCAR_GEN3_CLK_PLL1     2
>>> +#define RCAR_GEN3_CLK_PLL2     3
>>> +#define RCAR_GEN3_CLK_PLL3     4
>>> +#define RCAR_GEN3_CLK_PLL4     5
>>
>> These values are intimately tied to R8A7795_CLK_* in
>> include/dt-bindings/clock/r8a7795-clock.h.
>> Perhaps we can use these instead?
>
> You are right that they are connected together, but I'm not sure if I
> prefer to include that file. If we start sharing files between the CPG
> and the integration code then a dependency hell breaks loose when it
> comes to merge order. If we want to share the #defines then I propose
> that we handle that incrementally once all pieces are merged.

OK, we can move them to include/dt-bindings/clock/rcar-gen3-clock.h
later.

> On R-Car Gen2 we have a single CPG driver that is potentially shared
> across multiple SoCs in the same generation / family. In that case we
> use strings and the clock-output-names as part of the ABI if I'm not
> mistaken. On this driver we use clock index numbers instead, so I'd
> like to see these defines as part of a shared DT binding ABI where new
> SoCs can simply add new clocks at the end.

OK, so rcar_gen3_cpg.clks[] is really intended to be variable-sized.

Hence I suggest to write it that way:

    struct rcar_gen3_cpg {
            struct clk_onecell_data data;
            spinlock_t lock;
            void __iomem *reg;
            struct clk *clks[0];
    };

Then you can allocate it with

   kzalloc(sizeof(*cpg) + num_clks * sizeof(rcar_gen3_cpg.clks[0], ...)

and kill RCAR_GEN3_CLK_NR (one less definition that can go out of sync).

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 02/05] clk: shmobile: Add Renesas R-Car Gen3 CPG support

[Magnus Damm]

Hi Geert,

On Mon, Aug 31, 2015 at 10:03 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Magnus,
>
> On Mon, Aug 31, 2015 at 1:58 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>> --- /dev/null
>>>> +++ work/drivers/clk/shmobile/clk-rcar-gen3.c   2015-08-29 16:30:59.032366518 +0900
>>>> @@ -0,0 +1,249 @@
>>>
>>>> +#define RCAR_GEN3_CLK_MAIN     0
>>>> +#define RCAR_GEN3_CLK_PLL0     1
>>>> +#define RCAR_GEN3_CLK_PLL1     2
>>>> +#define RCAR_GEN3_CLK_PLL2     3
>>>> +#define RCAR_GEN3_CLK_PLL3     4
>>>> +#define RCAR_GEN3_CLK_PLL4     5
>>>
>>> These values are intimately tied to R8A7795_CLK_* in
>>> include/dt-bindings/clock/r8a7795-clock.h.
>>> Perhaps we can use these instead?
>>
>> You are right that they are connected together, but I'm not sure if I
>> prefer to include that file. If we start sharing files between the CPG
>> and the integration code then a dependency hell breaks loose when it
>> comes to merge order. If we want to share the #defines then I propose
>> that we handle that incrementally once all pieces are merged.
>
> OK, we can move them to include/dt-bindings/clock/rcar-gen3-clock.h
> later.

So in the future we may then want to change r8a7795-clock.h into:

#define R8A7795_CLK_MAIN RCAR_GEN3_CLK_MAIN

?

>> On R-Car Gen2 we have a single CPG driver that is potentially shared
>> across multiple SoCs in the same generation / family. In that case we
>> use strings and the clock-output-names as part of the ABI if I'm not
>> mistaken. On this driver we use clock index numbers instead, so I'd
>> like to see these defines as part of a shared DT binding ABI where new
>> SoCs can simply add new clocks at the end.
>
> OK, so rcar_gen3_cpg.clks[] is really intended to be variable-sized.
>
> Hence I suggest to write it that way:
>
>     struct rcar_gen3_cpg {
>             struct clk_onecell_data data;
>             spinlock_t lock;
>             void __iomem *reg;
>             struct clk *clks[0];
>     };
>
> Then you can allocate it with
>
>    kzalloc(sizeof(*cpg) + num_clks * sizeof(rcar_gen3_cpg.clks[0], ...)
>
> and kill RCAR_GEN3_CLK_NR (one less definition that can go out of sync).

I see RCAR_GEN3_CLK_NR as the number of different clocks that the Gen3
CPG driver can support. Exactly which one that the SoC specific DTS
wants to use depends on each SoC. So for future SoCs we may want to
keep on adding new clocks at the end. And the clock-indices list may
be sparsely populated in DTS.

I am not 100% sure about how to handle error cases. Say a new DTS is
booted on an old kernel then some clocks may not be supported by the
CPG driver. For those cases the switch/case statement in the driver
will simply return errors for the unsupported clocks. And the rest of
the kernel has to figure out how to deal with that. That seems sane,
right?

Thanks,

/ magnus

Re: [PATCH v4 02/05] clk: shmobile: Add Renesas R-Car Gen3 CPG support

[Geert Uytterhoeven]

Hi Magnus,

On Tue, Sep 1, 2015 at 12:48 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On Mon, Aug 31, 2015 at 10:03 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Mon, Aug 31, 2015 at 1:58 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>>> --- /dev/null
>>>>> +++ work/drivers/clk/shmobile/clk-rcar-gen3.c   2015-08-29 16:30:59.032366518 +0900
>>>>> @@ -0,0 +1,249 @@
>>>>
>>>>> +#define RCAR_GEN3_CLK_MAIN     0
>>>>> +#define RCAR_GEN3_CLK_PLL0     1
>>>>> +#define RCAR_GEN3_CLK_PLL1     2
>>>>> +#define RCAR_GEN3_CLK_PLL2     3
>>>>> +#define RCAR_GEN3_CLK_PLL3     4
>>>>> +#define RCAR_GEN3_CLK_PLL4     5
>>>>
>>>> These values are intimately tied to R8A7795_CLK_* in
>>>> include/dt-bindings/clock/r8a7795-clock.h.
>>>> Perhaps we can use these instead?
>>>
>>> You are right that they are connected together, but I'm not sure if I
>>> prefer to include that file. If we start sharing files between the CPG
>>> and the integration code then a dependency hell breaks loose when it
>>> comes to merge order. If we want to share the #defines then I propose
>>> that we handle that incrementally once all pieces are merged.
>>
>> OK, we can move them to include/dt-bindings/clock/rcar-gen3-clock.h
>> later.
>
> So in the future we may then want to change r8a7795-clock.h into:
>
> #define R8A7795_CLK_MAIN RCAR_GEN3_CLK_MAIN
>
> ?

Exactly.

>>> On R-Car Gen2 we have a single CPG driver that is potentially shared
>>> across multiple SoCs in the same generation / family. In that case we
>>> use strings and the clock-output-names as part of the ABI if I'm not
>>> mistaken. On this driver we use clock index numbers instead, so I'd
>>> like to see these defines as part of a shared DT binding ABI where new
>>> SoCs can simply add new clocks at the end.
>>
>> OK, so rcar_gen3_cpg.clks[] is really intended to be variable-sized.
>>
>> Hence I suggest to write it that way:
>>
>>     struct rcar_gen3_cpg {
>>             struct clk_onecell_data data;
>>             spinlock_t lock;
>>             void __iomem *reg;
>>             struct clk *clks[0];
>>     };
>>
>> Then you can allocate it with
>>
>>    kzalloc(sizeof(*cpg) + num_clks * sizeof(rcar_gen3_cpg.clks[0], ...)
>>
>> and kill RCAR_GEN3_CLK_NR (one less definition that can go out of sync).
>
> I see RCAR_GEN3_CLK_NR as the number of different clocks that the Gen3
> CPG driver can support. Exactly which one that the SoC specific DTS
> wants to use depends on each SoC. So for future SoCs we may want to
> keep on adding new clocks at the end. And the clock-indices list may
> be sparsely populated in DTS.

OK. And it's just a few pointers (initially I thought it contained full clk
structs).

> I am not 100% sure about how to handle error cases. Say a new DTS is
> booted on an old kernel then some clocks may not be supported by the
> CPG driver. For those cases the switch/case statement in the driver
> will simply return errors for the unsupported clocks. And the rest of
> the kernel has to figure out how to deal with that. That seems sane,
> right?

Indeed. Those clocks will not exist, and any driver trying to use them will
fail to clk_get() them.

Note that fixed-factor-clocks pointing to them will still be instantiated,
with a zero parent rate.

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