[PATCH v2 0/4] clk: renesas: cpg-mssr: Add support for R-Car M3-W

[PATCH v2 0/4] clk: renesas: cpg-mssr: Add support for R-Car M3-W

[Geert Uytterhoeven]

	Hi Mike, Stephen,

This patch series adds initial support for the Clock Pulse Generator and
Module Standby and Software Reset modules on the Renesas R-Car M3-W
SoC:
  - Basic core clocks,
  - SCIF2 (console) module clock,
  - INTC-AP (GIC) module clock (disabled pending CLK_ENABLE_HAND_OFF).

Support for more core and module clocks will be added incrementally.

/sys/kernel/debug/clk/clk_summary output:

       clock            enable_cnt  prepare_cnt        rate   accuracy   phase
    ---------------------------------------------------------------------------
     scif                        1            1    14745600          0 0
     extalr                      0            0           0          0 0
     extal                       1            1    16666666          0 0
	cp                       0            0     8333333          0 0
	.main                    1            1    16666666          0 0
	   .pll4                 0            0  2399999904          0 0
	   .pll3                 0            0  3199999872          0 0
	   .pll2                 0            0  2399999904          0 0
	   .pll1                 1            1  3199999872          0 0
	      .pll1_div2         1            1  1599999936          0 0
		 cl              0            0    33333332          0 0
		 zx              0            0   799999968          0 0
		 zt              0            0   399999984          0 0
		 ztrd2           0            0   133333328          0 0
		 ztr             0            0   266666656          0 0
		 .s3             2            2   266666656          0 0
		    s3d4         1            1    66666664          0 0
		       scif2     2            2    66666664          0 0
		    s3d2         0            0   133333328          0 0
		    s3d1         2            2   266666656          0 0
		       intc-ap   1            1   266666656          0 0
		 .s2             0            0   399999984          0 0
		    s2d4         0            0    99999996          0 0
		    s2d2         0            0   199999992          0 0
		    s2d1         0            0   399999984          0 0
		 .s1             0            0   533333312          0 0
		    s1d4         0            0   133333328          0 0
		    s1d2         0            0   266666656          0 0
		    s1d1         0            0   533333312          0 0
		 .s0             0            0   799999968          0 0
		    s0d12        0            0    66666664          0 0
		    s0d8         0            0    99999996          0 0
		    s0d6         0            0   133333328          0 0
		    s0d4         0            0   199999992          0 0
		    s0d3         0            0   266666656          0 0
		    s0d2         0            0   399999984          0 0
		    s0d1         0            0   799999968          0 0
		 .pll1_div4      0            0   799999968          0 0
	   .pll0                 0            0  2999999880          0 0

Changes compared to v1:
  - Add Acked-by, Tested-by.

For your convenience, I've pushed this series to the
topic/r8a7796-clk-v2 branch of
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git.
This has been tested on r8a7796/salvator-x, and the code (minus the
acks, i.e. v1), has been part of renesas-drivers since 2016-05-10.

I plan to queue this in my clk-renesas-for-v4.8 branch, and send a pull
request later.

Thanks!

Geert Uytterhoeven (4):
  clk: renesas: cpg-mssr: Document r8a7796 support
  clk: renesas: Add r8a7796 CPG Core Clock Definitions
  clk: renesas: cpg-mssr: Extract common R-Car Gen3 support code
  clk: renesas: cpg-mssr: Add support for R-Car M3-W

 .../devicetree/bindings/clock/renesas,cpg-mssr.txt |   7 +-
 drivers/clk/renesas/Kconfig                        |   1 +
 drivers/clk/renesas/Makefile                       |   3 +-
 drivers/clk/renesas/r8a7795-cpg-mssr.c             | 360 +--------------------
 drivers/clk/renesas/r8a7796-cpg-mssr.c             | 192 +++++++++++
 drivers/clk/renesas/rcar-gen3-cpg.c                | 359 ++++++++++++++++++++
 drivers/clk/renesas/rcar-gen3-cpg.h                |  43 +++
 drivers/clk/renesas/renesas-cpg-mssr.c             |   6 +
 drivers/clk/renesas/renesas-cpg-mssr.h             |   1 +
 include/dt-bindings/clock/r8a7796-cpg-mssr.h       |  69 ++++
 10 files changed, 682 insertions(+), 359 deletions(-)
 create mode 100644 drivers/clk/renesas/r8a7796-cpg-mssr.c
 create mode 100644 drivers/clk/renesas/rcar-gen3-cpg.c
 create mode 100644 drivers/clk/renesas/rcar-gen3-cpg.h
 create mode 100644 include/dt-bindings/clock/r8a7796-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 v2 1/4] clk: renesas: cpg-mssr: Document r8a7796 support

[Geert Uytterhoeven]

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
Tested-by: Simon Horman <horms+renesas@verge.net.au>
---
v2:
  - Add Acked-by, Tested-by.
---
 Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
index fefb8023020f1a54..394d725ac7e0baa3 100644
--- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
+++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
@@ -13,7 +13,8 @@ They provide the following functionalities:
 
 Required Properties:
   - compatible: Must be one of:
-      - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC
+      - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC (R-Car H3)
+      - "renesas,r8a7796-cpg-mssr" for the r8a7796 SoC (R-Car M3-W)
 
   - reg: Base address and length of the memory resource used by the CPG/MSSR
     block
@@ -21,8 +22,8 @@ Required Properties:
   - 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" (r8a7795)
-      - "extalr" (r8a7795)
+      - "extal" (r8a7795, r8a7796)
+      - "extalr" (r8a7795, r8a7796)
 
   - #clock-cells: Must be 2
       - For CPG core clocks, the two clock specifier cells must be "CPG_CORE"
-- 
1.9.1

[PATCH v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Geert Uytterhoeven]

Add all R-Car M3-W Clock Pulse Generator Core Clock Outputs, as listed
in Table 8.2b ("List of Clocks [R-Car M3-W]") of the R-Car Gen3
datasheet (rev. 0.51 + Errata for Rev051 Mar 31 2016).

Note that internal CPG clocks (S0, S1, S2, S3, SDSRC, and SSPSRC) are
not included, as they are used as internal clock sources only, and never
referenced from DT.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Simon Horman <horms+renesas@verge.net.au>
---
v2:
  - Add Tested-by.
---
 include/dt-bindings/clock/r8a7796-cpg-mssr.h | 69 ++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 include/dt-bindings/clock/r8a7796-cpg-mssr.h

diff --git a/include/dt-bindings/clock/r8a7796-cpg-mssr.h b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
new file mode 100644
index 0000000000000000..1e5942695f0dd057
--- /dev/null
+++ b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2016 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_R8A7796_CPG_MSSR_H__
+#define __DT_BINDINGS_CLOCK_R8A7796_CPG_MSSR_H__
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/* r8a7796 CPG Core Clocks */
+#define R8A7796_CLK_Z			0
+#define R8A7796_CLK_Z2			1
+#define R8A7796_CLK_ZR			2
+#define R8A7796_CLK_ZG			3
+#define R8A7796_CLK_ZTR			4
+#define R8A7796_CLK_ZTRD2		5
+#define R8A7796_CLK_ZT			6
+#define R8A7796_CLK_ZX			7
+#define R8A7796_CLK_S0D1		8
+#define R8A7796_CLK_S0D2		9
+#define R8A7796_CLK_S0D3		10
+#define R8A7796_CLK_S0D4		11
+#define R8A7796_CLK_S0D6		12
+#define R8A7796_CLK_S0D8		13
+#define R8A7796_CLK_S0D12		14
+#define R8A7796_CLK_S1D1		15
+#define R8A7796_CLK_S1D2		16
+#define R8A7796_CLK_S1D4		17
+#define R8A7796_CLK_S2D1		18
+#define R8A7796_CLK_S2D2		19
+#define R8A7796_CLK_S2D4		20
+#define R8A7796_CLK_S3D1		21
+#define R8A7796_CLK_S3D2		22
+#define R8A7796_CLK_S3D4		23
+#define R8A7796_CLK_LB			24
+#define R8A7796_CLK_CL			25
+#define R8A7796_CLK_ZB3			26
+#define R8A7796_CLK_ZB3D2		27
+#define R8A7796_CLK_ZB3D4		28
+#define R8A7796_CLK_CR			29
+#define R8A7796_CLK_CRD2		30
+#define R8A7796_CLK_SD0H		31
+#define R8A7796_CLK_SD0			32
+#define R8A7796_CLK_SD1H		33
+#define R8A7796_CLK_SD1			34
+#define R8A7796_CLK_SD2H		35
+#define R8A7796_CLK_SD2			36
+#define R8A7796_CLK_SD3H		37
+#define R8A7796_CLK_SD3			38
+#define R8A7796_CLK_SSP2		39
+#define R8A7796_CLK_SSP1		40
+#define R8A7796_CLK_SSPRS		41
+#define R8A7796_CLK_RPC			42
+#define R8A7796_CLK_RPCD2		43
+#define R8A7796_CLK_MSO			44
+#define R8A7796_CLK_CANFD		45
+#define R8A7796_CLK_HDMI		46
+#define R8A7796_CLK_CSI0		47
+#define R8A7796_CLK_CSIREF		48
+#define R8A7796_CLK_CP			49
+#define R8A7796_CLK_CPEX		50
+#define R8A7796_CLK_R			51
+#define R8A7796_CLK_OSC			52
+
+#endif /* __DT_BINDINGS_CLOCK_R8A7796_CPG_MSSR_H__ */
-- 
1.9.1

[PATCH v2 3/4] clk: renesas: cpg-mssr: Extract common R-Car Gen3 support code

[Geert Uytterhoeven]

Extract the code to support parts common to all members of the R-Car
Gen3 SoC family into a separate file, to ease sharing among SoC-specific
drivers.

Note that while the cpg_pll_configs[] arrays and the selection of the
config based on the MODE bits are identical on R-Car H3 and R-Car M3-W,
they are not common, and may be different on other R-Car Gen3 SoCs.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Simon Horman <horms+renesas@verge.net.au>
---
v2:
  - Add Tested-by.
---
 drivers/clk/renesas/Makefile           |   2 +-
 drivers/clk/renesas/r8a7795-cpg-mssr.c | 360 +--------------------------------
 drivers/clk/renesas/rcar-gen3-cpg.c    | 359 ++++++++++++++++++++++++++++++++
 drivers/clk/renesas/rcar-gen3-cpg.h    |  43 ++++
 4 files changed, 408 insertions(+), 356 deletions(-)
 create mode 100644 drivers/clk/renesas/rcar-gen3-cpg.c
 create mode 100644 drivers/clk/renesas/rcar-gen3-cpg.h

diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index ead8bb8435249493..88924c95808c3b2e 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7791)		+= clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7793)		+= clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7794)		+= clk-rcar-gen2.o clk-div6.o
-obj-$(CONFIG_ARCH_R8A7795)		+= r8a7795-cpg-mssr.o
+obj-$(CONFIG_ARCH_R8A7795)		+= r8a7795-cpg-mssr.o rcar-gen3-cpg.o
 obj-$(CONFIG_ARCH_SH73A0)		+= clk-sh73a0.o clk-div6.o
 
 obj-$(CONFIG_CLK_RENESAS_CPG_MSSR)	+= renesas-cpg-mssr.o clk-div6.o
diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index ca5519c583d4bf57..e53aff5b23ad4967 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -12,22 +12,14 @@
  * the Free Software Foundation; version 2 of the License.
  */
 
-#include <linux/bug.h>
-#include <linux/clk.h>
-#include <linux/clk-provider.h>
 #include <linux/device.h>
-#include <linux/err.h>
 #include <linux/init.h>
-#include <linux/io.h>
 #include <linux/kernel.h>
-#include <linux/of.h>
-#include <linux/slab.h>
 
 #include <dt-bindings/clock/r8a7795-cpg-mssr.h>
 
 #include "renesas-cpg-mssr.h"
-
-#define CPG_RCKCR	0x240
+#include "rcar-gen3-cpg.h"
 
 enum clk_ids {
 	/* Core Clock Outputs exported to DT */
@@ -58,20 +50,6 @@ enum clk_ids {
 	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,
-	CLK_TYPE_GEN3_SD,
-	CLK_TYPE_GEN3_R,
-};
-
-#define DEF_GEN3_SD(_name, _id, _parent, _offset)	\
-	DEF_BASE(_name, _id, CLK_TYPE_GEN3_SD, _parent, .offset = _offset)
-
 static const struct cpg_core_clk r8a7795_core_clks[] __initconst = {
 	/* External Clock Inputs */
 	DEF_INPUT("extal",  CLK_EXTAL),
@@ -262,225 +240,6 @@ static const unsigned int r8a7795_crit_mod_clks[] __initconst = {
 	MOD_CLK_ID(408),	/* INTC-AP (GIC) */
 };
 
-/* -----------------------------------------------------------------------------
- * SDn Clock
- *
- */
-#define CPG_SD_STP_HCK		BIT(9)
-#define CPG_SD_STP_CK		BIT(8)
-
-#define CPG_SD_STP_MASK		(CPG_SD_STP_HCK | CPG_SD_STP_CK)
-#define CPG_SD_FC_MASK		(0x7 << 2 | 0x3 << 0)
-
-#define CPG_SD_DIV_TABLE_DATA(stp_hck, stp_ck, sd_srcfc, sd_fc, sd_div) \
-{ \
-	.val = ((stp_hck) ? CPG_SD_STP_HCK : 0) | \
-	       ((stp_ck) ? CPG_SD_STP_CK : 0) | \
-	       ((sd_srcfc) << 2) | \
-	       ((sd_fc) << 0), \
-	.div = (sd_div), \
-}
-
-struct sd_div_table {
-	u32 val;
-	unsigned int div;
-};
-
-struct sd_clock {
-	struct clk_hw hw;
-	void __iomem *reg;
-	const struct sd_div_table *div_table;
-	unsigned int div_num;
-	unsigned int div_min;
-	unsigned int div_max;
-};
-
-/* SDn divider
- *                     sd_srcfc   sd_fc   div
- * stp_hck   stp_ck    (div)      (div)     = sd_srcfc x sd_fc
- *-------------------------------------------------------------------
- *  0         0         0 (1)      1 (4)      4
- *  0         0         1 (2)      1 (4)      8
- *  1         0         2 (4)      1 (4)     16
- *  1         0         3 (8)      1 (4)     32
- *  1         0         4 (16)     1 (4)     64
- *  0         0         0 (1)      0 (2)      2
- *  0         0         1 (2)      0 (2)      4
- *  1         0         2 (4)      0 (2)      8
- *  1         0         3 (8)      0 (2)     16
- *  1         0         4 (16)     0 (2)     32
- */
-static const struct sd_div_table cpg_sd_div_table[] = {
-/*	CPG_SD_DIV_TABLE_DATA(stp_hck,  stp_ck,   sd_srcfc,   sd_fc,  sd_div) */
-	CPG_SD_DIV_TABLE_DATA(0,        0,        0,          1,        4),
-	CPG_SD_DIV_TABLE_DATA(0,        0,        1,          1,        8),
-	CPG_SD_DIV_TABLE_DATA(1,        0,        2,          1,       16),
-	CPG_SD_DIV_TABLE_DATA(1,        0,        3,          1,       32),
-	CPG_SD_DIV_TABLE_DATA(1,        0,        4,          1,       64),
-	CPG_SD_DIV_TABLE_DATA(0,        0,        0,          0,        2),
-	CPG_SD_DIV_TABLE_DATA(0,        0,        1,          0,        4),
-	CPG_SD_DIV_TABLE_DATA(1,        0,        2,          0,        8),
-	CPG_SD_DIV_TABLE_DATA(1,        0,        3,          0,       16),
-	CPG_SD_DIV_TABLE_DATA(1,        0,        4,          0,       32),
-};
-
-#define to_sd_clock(_hw) container_of(_hw, struct sd_clock, hw)
-
-static int cpg_sd_clock_enable(struct clk_hw *hw)
-{
-	struct sd_clock *clock = to_sd_clock(hw);
-	u32 val, sd_fc;
-	unsigned int i;
-
-	val = clk_readl(clock->reg);
-
-	sd_fc = val & CPG_SD_FC_MASK;
-	for (i = 0; i < clock->div_num; i++)
-		if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
-			break;
-
-	if (i >= clock->div_num)
-		return -EINVAL;
-
-	val &= ~(CPG_SD_STP_MASK);
-	val |= clock->div_table[i].val & CPG_SD_STP_MASK;
-
-	clk_writel(val, clock->reg);
-
-	return 0;
-}
-
-static void cpg_sd_clock_disable(struct clk_hw *hw)
-{
-	struct sd_clock *clock = to_sd_clock(hw);
-
-	clk_writel(clk_readl(clock->reg) | CPG_SD_STP_MASK, clock->reg);
-}
-
-static int cpg_sd_clock_is_enabled(struct clk_hw *hw)
-{
-	struct sd_clock *clock = to_sd_clock(hw);
-
-	return !(clk_readl(clock->reg) & CPG_SD_STP_MASK);
-}
-
-static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw *hw,
-						unsigned long parent_rate)
-{
-	struct sd_clock *clock = to_sd_clock(hw);
-	unsigned long rate = parent_rate;
-	u32 val, sd_fc;
-	unsigned int i;
-
-	val = clk_readl(clock->reg);
-
-	sd_fc = val & CPG_SD_FC_MASK;
-	for (i = 0; i < clock->div_num; i++)
-		if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
-			break;
-
-	if (i >= clock->div_num)
-		return -EINVAL;
-
-	return DIV_ROUND_CLOSEST(rate, clock->div_table[i].div);
-}
-
-static unsigned int cpg_sd_clock_calc_div(struct sd_clock *clock,
-					  unsigned long rate,
-					  unsigned long parent_rate)
-{
-	unsigned int div;
-
-	if (!rate)
-		rate = 1;
-
-	div = DIV_ROUND_CLOSEST(parent_rate, rate);
-
-	return clamp_t(unsigned int, div, clock->div_min, clock->div_max);
-}
-
-static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
-				      unsigned long *parent_rate)
-{
-	struct sd_clock *clock = to_sd_clock(hw);
-	unsigned int div = cpg_sd_clock_calc_div(clock, rate, *parent_rate);
-
-	return DIV_ROUND_CLOSEST(*parent_rate, div);
-}
-
-static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
-				   unsigned long parent_rate)
-{
-	struct sd_clock *clock = to_sd_clock(hw);
-	unsigned int div = cpg_sd_clock_calc_div(clock, rate, parent_rate);
-	u32 val;
-	unsigned int i;
-
-	for (i = 0; i < clock->div_num; i++)
-		if (div == clock->div_table[i].div)
-			break;
-
-	if (i >= clock->div_num)
-		return -EINVAL;
-
-	val = clk_readl(clock->reg);
-	val &= ~(CPG_SD_STP_MASK | CPG_SD_FC_MASK);
-	val |= clock->div_table[i].val & (CPG_SD_STP_MASK | CPG_SD_FC_MASK);
-	clk_writel(val, clock->reg);
-
-	return 0;
-}
-
-static const struct clk_ops cpg_sd_clock_ops = {
-	.enable = cpg_sd_clock_enable,
-	.disable = cpg_sd_clock_disable,
-	.is_enabled = cpg_sd_clock_is_enabled,
-	.recalc_rate = cpg_sd_clock_recalc_rate,
-	.round_rate = cpg_sd_clock_round_rate,
-	.set_rate = cpg_sd_clock_set_rate,
-};
-
-static struct clk * __init cpg_sd_clk_register(const struct cpg_core_clk *core,
-					       void __iomem *base,
-					       const char *parent_name)
-{
-	struct clk_init_data init;
-	struct sd_clock *clock;
-	struct clk *clk;
-	unsigned int i;
-
-	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
-	if (!clock)
-		return ERR_PTR(-ENOMEM);
-
-	init.name = core->name;
-	init.ops = &cpg_sd_clock_ops;
-	init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
-	init.parent_names = &parent_name;
-	init.num_parents = 1;
-
-	clock->reg = base + core->offset;
-	clock->hw.init = &init;
-	clock->div_table = cpg_sd_div_table;
-	clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
-
-	clock->div_max = clock->div_table[0].div;
-	clock->div_min = clock->div_max;
-	for (i = 1; i < clock->div_num; i++) {
-		clock->div_max = max(clock->div_max, clock->div_table[i].div);
-		clock->div_min = min(clock->div_min, clock->div_table[i].div);
-	}
-
-	clk = clk_register(NULL, &clock->hw);
-	if (IS_ERR(clk))
-		kfree(clock);
-
-	return clk;
-}
-
-#define CPG_PLL0CR	0x00d8
-#define CPG_PLL2CR	0x002c
-#define CPG_PLL4CR	0x01f4
 
 /*
  * CPG Clock Data
@@ -512,13 +271,7 @@ static struct clk * __init cpg_sd_clk_register(const struct cpg_core_clk *core,
 					 (((md) & BIT(19)) >> 18) | \
 					 (((md) & BIT(17)) >> 17))
 
-struct cpg_pll_config {
-	unsigned int extal_div;
-	unsigned int pll1_mult;
-	unsigned int pll3_mult;
-};
-
-static const struct cpg_pll_config cpg_pll_configs[16] __initconst = {
+static const struct rcar_gen3_cpg_pll_config cpg_pll_configs[16] __initconst = {
 	/* EXTAL div	PLL1 mult	PLL3 mult */
 	{ 1,		192,		192,	},
 	{ 1,		192,		128,	},
@@ -538,112 +291,9 @@ static const struct cpg_pll_config cpg_pll_configs[16] __initconst = {
 	{ 2,		192,		192,	},
 };
 
-static const struct cpg_pll_config *cpg_pll_config __initdata;
-
-static
-struct clk * __init r8a7795_cpg_clk_register(struct device *dev,
-					     const struct cpg_core_clk *core,
-					     const struct cpg_mssr_info *info,
-					     struct clk **clks,
-					     void __iomem *base)
-{
-	const struct clk *parent;
-	unsigned int mult = 1;
-	unsigned int div = 1;
-	u32 value;
-
-	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) & 0x7f) + 1) * 2;
-		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) & 0x7f) + 1) * 2;
-		break;
-
-	case CLK_TYPE_GEN3_PLL3:
-		mult = cpg_pll_config->pll3_mult;
-		break;
-
-	case CLK_TYPE_GEN3_PLL4:
-		/*
-		 * PLL4 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_PLL4CR);
-		mult = (((value >> 24) & 0x7f) + 1) * 2;
-		break;
-
-	case CLK_TYPE_GEN3_SD:
-		return cpg_sd_clk_register(core, base, __clk_get_name(parent));
-
-	case CLK_TYPE_GEN3_R:
-		/* RINT is default. Only if EXTALR is populated, we switch to it */
-		value = readl(base + CPG_RCKCR) & 0x3f;
-
-		if (clk_get_rate(clks[CLK_EXTALR])) {
-			parent = clks[CLK_EXTALR];
-			value |= BIT(15);
-		}
-
-		writel(value, base + CPG_RCKCR);
-		break;
-
-	default:
-		return ERR_PTR(-EINVAL);
-	}
-
-	return clk_register_fixed_factor(NULL, core->name,
-					 __clk_get_name(parent), 0, mult, div);
-}
-
-/*
- * Reset register definitions.
- */
-#define MODEMR	0xe6160060
-
-static u32 rcar_gen3_read_mode_pins(void)
-{
-	void __iomem *modemr = ioremap_nocache(MODEMR, 4);
-	u32 mode;
-
-	BUG_ON(!modemr);
-	mode = ioread32(modemr);
-	iounmap(modemr);
-
-	return mode;
-}
-
 static int __init r8a7795_cpg_mssr_init(struct device *dev)
 {
+	const struct rcar_gen3_cpg_pll_config *cpg_pll_config;
 	u32 cpg_mode = rcar_gen3_read_mode_pins();
 
 	cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
@@ -652,7 +302,7 @@ static int __init r8a7795_cpg_mssr_init(struct device *dev)
 		return -EINVAL;
 	}
 
-	return 0;
+	return rcar_gen3_cpg_init(cpg_pll_config, CLK_EXTALR);
 }
 
 const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
@@ -673,5 +323,5 @@ const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = {
 
 	/* Callbacks */
 	.init = r8a7795_cpg_mssr_init,
-	.cpg_clk_register = r8a7795_cpg_clk_register,
+	.cpg_clk_register = rcar_gen3_cpg_clk_register,
 };
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
new file mode 100644
index 0000000000000000..bb4f2f9a8c2f5ba8
--- /dev/null
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -0,0 +1,359 @@
+/*
+ * R-Car Gen3 Clock Pulse Generator
+ *
+ * Copyright (C) 2015-2016 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.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+
+#include "renesas-cpg-mssr.h"
+#include "rcar-gen3-cpg.h"
+
+#define CPG_PLL0CR		0x00d8
+#define CPG_PLL2CR		0x002c
+#define CPG_PLL4CR		0x01f4
+
+
+/*
+ * SDn Clock
+ */
+#define CPG_SD_STP_HCK		BIT(9)
+#define CPG_SD_STP_CK		BIT(8)
+
+#define CPG_SD_STP_MASK		(CPG_SD_STP_HCK | CPG_SD_STP_CK)
+#define CPG_SD_FC_MASK		(0x7 << 2 | 0x3 << 0)
+
+#define CPG_SD_DIV_TABLE_DATA(stp_hck, stp_ck, sd_srcfc, sd_fc, sd_div) \
+{ \
+	.val = ((stp_hck) ? CPG_SD_STP_HCK : 0) | \
+	       ((stp_ck) ? CPG_SD_STP_CK : 0) | \
+	       ((sd_srcfc) << 2) | \
+	       ((sd_fc) << 0), \
+	.div = (sd_div), \
+}
+
+struct sd_div_table {
+	u32 val;
+	unsigned int div;
+};
+
+struct sd_clock {
+	struct clk_hw hw;
+	void __iomem *reg;
+	const struct sd_div_table *div_table;
+	unsigned int div_num;
+	unsigned int div_min;
+	unsigned int div_max;
+};
+
+/* SDn divider
+ *                     sd_srcfc   sd_fc   div
+ * stp_hck   stp_ck    (div)      (div)     = sd_srcfc x sd_fc
+ *-------------------------------------------------------------------
+ *  0         0         0 (1)      1 (4)      4
+ *  0         0         1 (2)      1 (4)      8
+ *  1         0         2 (4)      1 (4)     16
+ *  1         0         3 (8)      1 (4)     32
+ *  1         0         4 (16)     1 (4)     64
+ *  0         0         0 (1)      0 (2)      2
+ *  0         0         1 (2)      0 (2)      4
+ *  1         0         2 (4)      0 (2)      8
+ *  1         0         3 (8)      0 (2)     16
+ *  1         0         4 (16)     0 (2)     32
+ */
+static const struct sd_div_table cpg_sd_div_table[] = {
+/*	CPG_SD_DIV_TABLE_DATA(stp_hck,  stp_ck,   sd_srcfc,   sd_fc,  sd_div) */
+	CPG_SD_DIV_TABLE_DATA(0,        0,        0,          1,        4),
+	CPG_SD_DIV_TABLE_DATA(0,        0,        1,          1,        8),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        2,          1,       16),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        3,          1,       32),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        4,          1,       64),
+	CPG_SD_DIV_TABLE_DATA(0,        0,        0,          0,        2),
+	CPG_SD_DIV_TABLE_DATA(0,        0,        1,          0,        4),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        2,          0,        8),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        3,          0,       16),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        4,          0,       32),
+};
+
+#define to_sd_clock(_hw) container_of(_hw, struct sd_clock, hw)
+
+static int cpg_sd_clock_enable(struct clk_hw *hw)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+	u32 val, sd_fc;
+	unsigned int i;
+
+	val = clk_readl(clock->reg);
+
+	sd_fc = val & CPG_SD_FC_MASK;
+	for (i = 0; i < clock->div_num; i++)
+		if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
+			break;
+
+	if (i >= clock->div_num)
+		return -EINVAL;
+
+	val &= ~(CPG_SD_STP_MASK);
+	val |= clock->div_table[i].val & CPG_SD_STP_MASK;
+
+	clk_writel(val, clock->reg);
+
+	return 0;
+}
+
+static void cpg_sd_clock_disable(struct clk_hw *hw)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+
+	clk_writel(clk_readl(clock->reg) | CPG_SD_STP_MASK, clock->reg);
+}
+
+static int cpg_sd_clock_is_enabled(struct clk_hw *hw)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+
+	return !(clk_readl(clock->reg) & CPG_SD_STP_MASK);
+}
+
+static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+	unsigned long rate = parent_rate;
+	u32 val, sd_fc;
+	unsigned int i;
+
+	val = clk_readl(clock->reg);
+
+	sd_fc = val & CPG_SD_FC_MASK;
+	for (i = 0; i < clock->div_num; i++)
+		if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
+			break;
+
+	if (i >= clock->div_num)
+		return -EINVAL;
+
+	return DIV_ROUND_CLOSEST(rate, clock->div_table[i].div);
+}
+
+static unsigned int cpg_sd_clock_calc_div(struct sd_clock *clock,
+					  unsigned long rate,
+					  unsigned long parent_rate)
+{
+	unsigned int div;
+
+	if (!rate)
+		rate = 1;
+
+	div = DIV_ROUND_CLOSEST(parent_rate, rate);
+
+	return clamp_t(unsigned int, div, clock->div_min, clock->div_max);
+}
+
+static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
+				      unsigned long *parent_rate)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+	unsigned int div = cpg_sd_clock_calc_div(clock, rate, *parent_rate);
+
+	return DIV_ROUND_CLOSEST(*parent_rate, div);
+}
+
+static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
+				   unsigned long parent_rate)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+	unsigned int div = cpg_sd_clock_calc_div(clock, rate, parent_rate);
+	u32 val;
+	unsigned int i;
+
+	for (i = 0; i < clock->div_num; i++)
+		if (div == clock->div_table[i].div)
+			break;
+
+	if (i >= clock->div_num)
+		return -EINVAL;
+
+	val = clk_readl(clock->reg);
+	val &= ~(CPG_SD_STP_MASK | CPG_SD_FC_MASK);
+	val |= clock->div_table[i].val & (CPG_SD_STP_MASK | CPG_SD_FC_MASK);
+	clk_writel(val, clock->reg);
+
+	return 0;
+}
+
+static const struct clk_ops cpg_sd_clock_ops = {
+	.enable = cpg_sd_clock_enable,
+	.disable = cpg_sd_clock_disable,
+	.is_enabled = cpg_sd_clock_is_enabled,
+	.recalc_rate = cpg_sd_clock_recalc_rate,
+	.round_rate = cpg_sd_clock_round_rate,
+	.set_rate = cpg_sd_clock_set_rate,
+};
+
+static struct clk * __init cpg_sd_clk_register(const struct cpg_core_clk *core,
+					       void __iomem *base,
+					       const char *parent_name)
+{
+	struct clk_init_data init;
+	struct sd_clock *clock;
+	struct clk *clk;
+	unsigned int i;
+
+	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
+	if (!clock)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = core->name;
+	init.ops = &cpg_sd_clock_ops;
+	init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	clock->reg = base + core->offset;
+	clock->hw.init = &init;
+	clock->div_table = cpg_sd_div_table;
+	clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
+
+	clock->div_max = clock->div_table[0].div;
+	clock->div_min = clock->div_max;
+	for (i = 1; i < clock->div_num; i++) {
+		clock->div_max = max(clock->div_max, clock->div_table[i].div);
+		clock->div_min = min(clock->div_min, clock->div_table[i].div);
+	}
+
+	clk = clk_register(NULL, &clock->hw);
+	if (IS_ERR(clk))
+		kfree(clock);
+
+	return clk;
+}
+
+
+static const struct rcar_gen3_cpg_pll_config *cpg_pll_config __initdata;
+static unsigned int cpg_clk_extalr __initdata;
+
+struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
+	const struct cpg_core_clk *core, const struct cpg_mssr_info *info,
+	struct clk **clks, void __iomem *base)
+{
+	const struct clk *parent;
+	unsigned int mult = 1;
+	unsigned int div = 1;
+	u32 value;
+
+	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) & 0x7f) + 1) * 2;
+		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) & 0x7f) + 1) * 2;
+		break;
+
+	case CLK_TYPE_GEN3_PLL3:
+		mult = cpg_pll_config->pll3_mult;
+		break;
+
+	case CLK_TYPE_GEN3_PLL4:
+		/*
+		 * PLL4 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_PLL4CR);
+		mult = (((value >> 24) & 0x7f) + 1) * 2;
+		break;
+
+	case CLK_TYPE_GEN3_SD:
+		return cpg_sd_clk_register(core, base, __clk_get_name(parent));
+
+	case CLK_TYPE_GEN3_R:
+		/*
+		 * RINT is default.
+		 * Only if EXTALR is populated, we switch to it.
+		 */
+		value = readl(base + CPG_RCKCR) & 0x3f;
+
+		if (clk_get_rate(clks[cpg_clk_extalr])) {
+			parent = clks[cpg_clk_extalr];
+			value |= BIT(15);
+		}
+
+		writel(value, base + CPG_RCKCR);
+		break;
+
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+
+	return clk_register_fixed_factor(NULL, core->name,
+					 __clk_get_name(parent), 0, mult, div);
+}
+
+/*
+ * Reset register definitions.
+ */
+#define MODEMR	0xe6160060
+
+u32 __init rcar_gen3_read_mode_pins(void)
+{
+	void __iomem *modemr = ioremap_nocache(MODEMR, 4);
+	u32 mode;
+
+	BUG_ON(!modemr);
+	mode = ioread32(modemr);
+	iounmap(modemr);
+
+	return mode;
+}
+
+int __init rcar_gen3_cpg_init(const struct rcar_gen3_cpg_pll_config *config,
+			      unsigned int clk_extalr)
+{
+	cpg_pll_config = config;
+	cpg_clk_extalr = clk_extalr;
+	return 0;
+}
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.h b/drivers/clk/renesas/rcar-gen3-cpg.h
new file mode 100644
index 0000000000000000..f699085147d1aece
--- /dev/null
+++ b/drivers/clk/renesas/rcar-gen3-cpg.h
@@ -0,0 +1,43 @@
+/*
+ * R-Car Gen3 Clock Pulse Generator
+ *
+ * Copyright (C) 2015-2016 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.
+ */
+
+#ifndef __CLK_RENESAS_RCAR_GEN3_CPG_H__
+#define __CLK_RENESAS_RCAR_GEN3_CPG_H__
+
+enum rcar_gen3_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,
+	CLK_TYPE_GEN3_SD,
+	CLK_TYPE_GEN3_R,
+};
+
+#define DEF_GEN3_SD(_name, _id, _parent, _offset)	\
+	DEF_BASE(_name, _id, CLK_TYPE_GEN3_SD, _parent, .offset = _offset)
+
+struct rcar_gen3_cpg_pll_config {
+	unsigned int extal_div;
+	unsigned int pll1_mult;
+	unsigned int pll3_mult;
+};
+
+#define CPG_RCKCR	0x240
+
+u32 rcar_gen3_read_mode_pins(void);
+struct clk *rcar_gen3_cpg_clk_register(struct device *dev,
+	const struct cpg_core_clk *core, const struct cpg_mssr_info *info,
+	struct clk **clks, void __iomem *base);
+int rcar_gen3_cpg_init(const struct rcar_gen3_cpg_pll_config *config,
+		       unsigned int clk_extalr);
+
+#endif
-- 
1.9.1

[PATCH v2 4/4] clk: renesas: cpg-mssr: Add support for R-Car M3-W

[Geert Uytterhoeven]

Initial support for R-Car M3-W (r8a7796), including basic core clocks,
and SCIF2 (console) and INTC-AP (GIC) module clocks.

Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Tested-by: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
v2:
  - Add Tested-by.
---
 drivers/clk/renesas/Kconfig            |   1 +
 drivers/clk/renesas/Makefile           |   1 +
 drivers/clk/renesas/r8a7796-cpg-mssr.c | 192 +++++++++++++++++++++++++++++++++
 drivers/clk/renesas/renesas-cpg-mssr.c |   6 ++
 drivers/clk/renesas/renesas-cpg-mssr.h |   1 +
 5 files changed, 201 insertions(+)
 create mode 100644 drivers/clk/renesas/r8a7796-cpg-mssr.c

diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index 2115ce410cfb4bc9..fcad9ff090f5fd2b 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -1,6 +1,7 @@
 config CLK_RENESAS_CPG_MSSR
 	bool
 	default y if ARCH_R8A7795
+	default y if ARCH_R8A7796
 
 config CLK_RENESAS_CPG_MSTP
 	bool
diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index 88924c95808c3b2e..0b8d31b4909c9690 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_ARCH_R8A7791)		+= clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7793)		+= clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7794)		+= clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7795)		+= r8a7795-cpg-mssr.o rcar-gen3-cpg.o
+obj-$(CONFIG_ARCH_R8A7796)		+= r8a7796-cpg-mssr.o rcar-gen3-cpg.o
 obj-$(CONFIG_ARCH_SH73A0)		+= clk-sh73a0.o clk-div6.o
 
 obj-$(CONFIG_CLK_RENESAS_CPG_MSSR)	+= renesas-cpg-mssr.o clk-div6.o
diff --git a/drivers/clk/renesas/r8a7796-cpg-mssr.c b/drivers/clk/renesas/r8a7796-cpg-mssr.c
new file mode 100644
index 0000000000000000..c84b549c14d2e57d
--- /dev/null
+++ b/drivers/clk/renesas/r8a7796-cpg-mssr.c
@@ -0,0 +1,192 @@
+/*
+ * r8a7796 Clock Pulse Generator / Module Standby and Software Reset
+ *
+ * Copyright (C) 2016 Glider bvba
+ *
+ * Based on r8a7795-cpg-mssr.c
+ *
+ * Copyright (C) 2015 Glider bvba
+ * 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/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+
+#include <dt-bindings/clock/r8a7796-cpg-mssr.h>
+
+#include "renesas-cpg-mssr.h"
+#include "rcar-gen3-cpg.h"
+
+enum clk_ids {
+	/* Core Clock Outputs exported to DT */
+	LAST_DT_CORE_CLK = R8A7796_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
+};
+
+static const struct cpg_core_clk r8a7796_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",        R8A7796_CLK_ZTR,   CLK_PLL1_DIV2,  6, 1),
+	DEF_FIXED("ztrd2",      R8A7796_CLK_ZTRD2, CLK_PLL1_DIV2, 12, 1),
+	DEF_FIXED("zt",         R8A7796_CLK_ZT,    CLK_PLL1_DIV2,  4, 1),
+	DEF_FIXED("zx",         R8A7796_CLK_ZX,    CLK_PLL1_DIV2,  2, 1),
+	DEF_FIXED("s0d1",       R8A7796_CLK_S0D1,  CLK_S0,         1, 1),
+	DEF_FIXED("s0d2",       R8A7796_CLK_S0D2,  CLK_S0,         2, 1),
+	DEF_FIXED("s0d3",       R8A7796_CLK_S0D3,  CLK_S0,         3, 1),
+	DEF_FIXED("s0d4",       R8A7796_CLK_S0D4,  CLK_S0,         4, 1),
+	DEF_FIXED("s0d6",       R8A7796_CLK_S0D6,  CLK_S0,         6, 1),
+	DEF_FIXED("s0d8",       R8A7796_CLK_S0D8,  CLK_S0,         8, 1),
+	DEF_FIXED("s0d12",      R8A7796_CLK_S0D12, CLK_S0,        12, 1),
+	DEF_FIXED("s1d1",       R8A7796_CLK_S1D1,  CLK_S1,         1, 1),
+	DEF_FIXED("s1d2",       R8A7796_CLK_S1D2,  CLK_S1,         2, 1),
+	DEF_FIXED("s1d4",       R8A7796_CLK_S1D4,  CLK_S1,         4, 1),
+	DEF_FIXED("s2d1",       R8A7796_CLK_S2D1,  CLK_S2,         1, 1),
+	DEF_FIXED("s2d2",       R8A7796_CLK_S2D2,  CLK_S2,         2, 1),
+	DEF_FIXED("s2d4",       R8A7796_CLK_S2D4,  CLK_S2,         4, 1),
+	DEF_FIXED("s3d1",       R8A7796_CLK_S3D1,  CLK_S3,         1, 1),
+	DEF_FIXED("s3d2",       R8A7796_CLK_S3D2,  CLK_S3,         2, 1),
+	DEF_FIXED("s3d4",       R8A7796_CLK_S3D4,  CLK_S3,         4, 1),
+
+	DEF_FIXED("cl",         R8A7796_CLK_CL,    CLK_PLL1_DIV2, 48, 1),
+	DEF_FIXED("cp",         R8A7796_CLK_CP,    CLK_EXTAL,      2, 1),
+};
+
+static const struct mssr_mod_clk r8a7796_mod_clks[] __initconst = {
+	DEF_MOD("scif2",		 310,	R8A7796_CLK_S3D4),
+	DEF_MOD("intc-ap",		 408,	R8A7796_CLK_S3D1),
+};
+
+static const unsigned int r8a7796_crit_mod_clks[] __initconst = {
+	MOD_CLK_ID(408),	/* INTC-AP (GIC) */
+};
+
+
+/*
+ * 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	x160	x120	x160	x120
+ * 0  1  0  1	20    x 1	x150	x160	x120	x106	x120
+ * 0  1  1  0	Prohibited setting
+ * 0  1  1  1	20    x 1	x150	x160	x120	x160	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))
+
+static const struct rcar_gen3_cpg_pll_config cpg_pll_configs[16] __initconst = {
+	/* EXTAL div	PLL1 mult	PLL3 mult */
+	{ 1,		192,		192,	},
+	{ 1,		192,		128,	},
+	{ 0, /* Prohibited setting */		},
+	{ 1,		192,		192,	},
+	{ 1,		160,		160,	},
+	{ 1,		160,		106,	},
+	{ 0, /* Prohibited setting */		},
+	{ 1,		160,		160,	},
+	{ 1,		128,		128,	},
+	{ 1,		128,		84,	},
+	{ 0, /* Prohibited setting */		},
+	{ 1,		128,		128,	},
+	{ 2,		192,		192,	},
+	{ 2,		192,		128,	},
+	{ 0, /* Prohibited setting */		},
+	{ 2,		192,		192,	},
+};
+
+static int __init r8a7796_cpg_mssr_init(struct device *dev)
+{
+	const struct rcar_gen3_cpg_pll_config *cpg_pll_config;
+	u32 cpg_mode = rcar_gen3_read_mode_pins();
+
+	cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
+	if (!cpg_pll_config->extal_div) {
+		dev_err(dev, "Prohibited setting (cpg_mode=0x%x)\n", cpg_mode);
+		return -EINVAL;
+	}
+
+	return rcar_gen3_cpg_init(cpg_pll_config, CLK_EXTALR);
+}
+
+const struct cpg_mssr_info r8a7796_cpg_mssr_info __initconst = {
+	/* Core Clocks */
+	.core_clks = r8a7796_core_clks,
+	.num_core_clks = ARRAY_SIZE(r8a7796_core_clks),
+	.last_dt_core_clk = LAST_DT_CORE_CLK,
+	.num_total_core_clks = MOD_CLK_BASE,
+
+	/* Module Clocks */
+	.mod_clks = r8a7796_mod_clks,
+	.num_mod_clks = ARRAY_SIZE(r8a7796_mod_clks),
+	.num_hw_mod_clks = 12 * 32,
+
+	/* Critical Module Clocks */
+	.crit_mod_clks = r8a7796_crit_mod_clks,
+	.num_crit_mod_clks = ARRAY_SIZE(r8a7796_crit_mod_clks),
+
+	/* Callbacks */
+	.init = r8a7796_cpg_mssr_init,
+	.cpg_clk_register = rcar_gen3_cpg_clk_register,
+};
diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index 210cd744a7a97bbd..e1365e7491ae02a0 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -509,6 +509,12 @@ static const struct of_device_id cpg_mssr_match[] = {
 		.data = &r8a7795_cpg_mssr_info,
 	},
 #endif
+#ifdef CONFIG_ARCH_R8A7796
+	{
+		.compatible = "renesas,r8a7796-cpg-mssr",
+		.data = &r8a7796_cpg_mssr_info,
+	},
+#endif
 	{ /* sentinel */ }
 };
 
diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h
index 0d1e3e811e79bf43..ee7edfaf14089cef 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.h
+++ b/drivers/clk/renesas/renesas-cpg-mssr.h
@@ -131,4 +131,5 @@ struct cpg_mssr_info {
 };
 
 extern const struct cpg_mssr_info r8a7795_cpg_mssr_info;
+extern const struct cpg_mssr_info r8a7796_cpg_mssr_info;
 #endif
-- 
1.9.1

--
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 v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Dirk Behme]

Hi Geert,

On 30.05.2016 18:28, Geert Uytterhoeven wrote:
> Add all R-Car M3-W Clock Pulse Generator Core Clock Outputs, as listed
> in Table 8.2b ("List of Clocks [R-Car M3-W]") of the R-Car Gen3
> datasheet (rev. 0.51 + Errata for Rev051 Mar 31 2016).
>
> Note that internal CPG clocks (S0, S1, S2, S3, SDSRC, and SSPSRC) are
> not included, as they are used as internal clock sources only, and never
> referenced from DT.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Tested-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> v2:
>    - Add Tested-by.
> ---
>   include/dt-bindings/clock/r8a7796-cpg-mssr.h | 69 ++++++++++++++++++++++++++++
>   1 file changed, 69 insertions(+)
>   create mode 100644 include/dt-bindings/clock/r8a7796-cpg-mssr.h
>
> diff --git a/include/dt-bindings/clock/r8a7796-cpg-mssr.h b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
> new file mode 100644
> index 0000000000000000..1e5942695f0dd057
> --- /dev/null
> +++ b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (C) 2016 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_R8A7796_CPG_MSSR_H__
> +#define __DT_BINDINGS_CLOCK_R8A7796_CPG_MSSR_H__
> +
> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
> +
> +/* r8a7796 CPG Core Clocks */
> +#define R8A7796_CLK_Z			0
> +#define R8A7796_CLK_Z2			1
> +#define R8A7796_CLK_ZR			2
> +#define R8A7796_CLK_ZG			3
> +#define R8A7796_CLK_ZTR			4
> +#define R8A7796_CLK_ZTRD2		5
> +#define R8A7796_CLK_ZT			6
> +#define R8A7796_CLK_ZX			7
> +#define R8A7796_CLK_S0D1		8
> +#define R8A7796_CLK_S0D2		9
> +#define R8A7796_CLK_S0D3		10
> +#define R8A7796_CLK_S0D4		11
> +#define R8A7796_CLK_S0D6		12
> +#define R8A7796_CLK_S0D8		13
> +#define R8A7796_CLK_S0D12		14
> +#define R8A7796_CLK_S1D1		15
> +#define R8A7796_CLK_S1D2		16
> +#define R8A7796_CLK_S1D4		17
> +#define R8A7796_CLK_S2D1		18
> +#define R8A7796_CLK_S2D2		19
> +#define R8A7796_CLK_S2D4		20
> +#define R8A7796_CLK_S3D1		21
> +#define R8A7796_CLK_S3D2		22
> +#define R8A7796_CLK_S3D4		23
> +#define R8A7796_CLK_LB			24
> +#define R8A7796_CLK_CL			25
> +#define R8A7796_CLK_ZB3			26
> +#define R8A7796_CLK_ZB3D2		27
> +#define R8A7796_CLK_ZB3D4		28
> +#define R8A7796_CLK_CR			29
> +#define R8A7796_CLK_CRD2		30
> +#define R8A7796_CLK_SD0H		31
> +#define R8A7796_CLK_SD0			32
> +#define R8A7796_CLK_SD1H		33
> +#define R8A7796_CLK_SD1			34
> +#define R8A7796_CLK_SD2H		35
> +#define R8A7796_CLK_SD2			36
> +#define R8A7796_CLK_SD3H		37
> +#define R8A7796_CLK_SD3			38
> +#define R8A7796_CLK_SSP2		39
> +#define R8A7796_CLK_SSP1		40
> +#define R8A7796_CLK_SSPRS		41
> +#define R8A7796_CLK_RPC			42
> +#define R8A7796_CLK_RPCD2		43
> +#define R8A7796_CLK_MSO			44
> +#define R8A7796_CLK_CANFD		45
> +#define R8A7796_CLK_HDMI		46
> +#define R8A7796_CLK_CSI0		47
> +#define R8A7796_CLK_CSIREF		48
> +#define R8A7796_CLK_CP			49
> +#define R8A7796_CLK_CPEX		50
> +#define R8A7796_CLK_R			51
> +#define R8A7796_CLK_OSC			52


I think we recently started a discussion to find a more clever way to 
avoid re-defining (copy & paste) all this R-Car3 clocks  (compare [1]) 
where they are the same over the R-Car3 family while still being able 
to deal with the differences.

Best regards

Dirk

[1] 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/dt-bindings/clock/r8a7795-cpg-mssr.h

Re: [PATCH v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Magnus Damm]

Hi Dirk,

On Tue, May 31, 2016 at 1:36 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
> Hi Geert,
>
>
> On 30.05.2016 18:28, Geert Uytterhoeven wrote:
>>
>> Add all R-Car M3-W Clock Pulse Generator Core Clock Outputs, as listed
>> in Table 8.2b ("List of Clocks [R-Car M3-W]") of the R-Car Gen3
>> datasheet (rev. 0.51 + Errata for Rev051 Mar 31 2016).
>>
>> Note that internal CPG clocks (S0, S1, S2, S3, SDSRC, and SSPSRC) are
>> not included, as they are used as internal clock sources only, and never
>> referenced from DT.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> Tested-by: Simon Horman <horms+renesas@verge.net.au>
>> ---
>> v2:
>>    - Add Tested-by.
>> ---
[snip]
>> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
>> +
>> +/* r8a7796 CPG Core Clocks */
>> +#define R8A7796_CLK_Z                  0
>> +#define R8A7796_CLK_Z2                 1
>
> I think we recently started a discussion to find a more clever way to avoid
> re-defining (copy & paste) all this R-Car3 clocks  (compare [1]) where they
> are the same over the R-Car3 family while still being able to deal with the
> differences.
>
> [1]
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/dt-bindings/clock/r8a7795-cpg-mssr.h

This topic is related to the rather fixed DT ABI for the CPG device.
Please see the interface described in the following upstream commits:

$ git log --oneline
Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt | cat
ca00c38 clk: shmobile: cpg-mssr: Update serial port clock in example
3686d3e clk: shmobile: Add new Renesas CPG/MSSR DT bindings
$

Some years ago we discussed this consolidation topic for R-Car Gen2
and we revisited when doing various iterations for the R-Car Gen3 CPGs
last year. Using a single per-family shared range of clocks was
considered both times, however when comparing the pros and cons it
became evident that best practice with DT tends to be to not speculate
and go with what we know based on existing documentation.

Like you may have experienced, the SoC documentation is slightly
changing over time. Also the number of SoCs in a certain family is
clearly being extended over time. The direction where things are going
is not known during initial design phase. Because of this we make use
of known-to-be-fixed ids such as SoC-specific part numbers instead of
speculating with family DT strings where the meaning tend to change
over time.

If we would go with consolidating the clock indices from per-SoC to
per-family then please note that this does not affect the run time
size of the DTB. In the kernel source code we have some level of
agreed duplication where DT compat strings and such are chosen to be
duplicated per-device instead of trying to make the code more compact.
I believe our build machines are snappy enough to handle a couple of
KiB of strings worst case.

Because of the unchanged DTB size this discussion is purely a matter
of how to maintain the code. So the maintainers need to decide what
level of duplication is needed. Perhaps this topic should be
revisited, and maybe meeting face-to-face would help?

Thanks,

/ magnus

Re: [PATCH v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Dirk Behme]

Hi Geert,

On 30.05.2016 18:36, Dirk Behme wrote:
> Hi Geert,
>
> On 30.05.2016 18:28, Geert Uytterhoeven wrote:
>> Add all R-Car M3-W Clock Pulse Generator Core Clock Outputs, as listed
>> in Table 8.2b ("List of Clocks [R-Car M3-W]") of the R-Car Gen3
>> datasheet (rev. 0.51 + Errata for Rev051 Mar 31 2016).
>>
>> Note that internal CPG clocks (S0, S1, S2, S3, SDSRC, and SSPSRC) are
>> not included, as they are used as internal clock sources only, and never
>> referenced from DT.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> Tested-by: Simon Horman <horms+renesas@verge.net.au>
>> ---
>> v2:
>>    - Add Tested-by.
>> ---
>>   include/dt-bindings/clock/r8a7796-cpg-mssr.h | 69
>> ++++++++++++++++++++++++++++
>>   1 file changed, 69 insertions(+)
>>   create mode 100644 include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>
>> diff --git a/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>> b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>> new file mode 100644
>> index 0000000000000000..1e5942695f0dd057
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>> @@ -0,0 +1,69 @@
>> +/*
>> + * Copyright (C) 2016 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_R8A7796_CPG_MSSR_H__
>> +#define __DT_BINDINGS_CLOCK_R8A7796_CPG_MSSR_H__
>> +
>> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
>> +
>> +/* r8a7796 CPG Core Clocks */
>> +#define R8A7796_CLK_Z            0
>> +#define R8A7796_CLK_Z2            1
>> +#define R8A7796_CLK_ZR            2
>> +#define R8A7796_CLK_ZG            3
>> +#define R8A7796_CLK_ZTR            4
>> +#define R8A7796_CLK_ZTRD2        5
>> +#define R8A7796_CLK_ZT            6
>> +#define R8A7796_CLK_ZX            7
>> +#define R8A7796_CLK_S0D1        8
>> +#define R8A7796_CLK_S0D2        9
>> +#define R8A7796_CLK_S0D3        10
>> +#define R8A7796_CLK_S0D4        11
>> +#define R8A7796_CLK_S0D6        12
>> +#define R8A7796_CLK_S0D8        13
>> +#define R8A7796_CLK_S0D12        14
>> +#define R8A7796_CLK_S1D1        15
>> +#define R8A7796_CLK_S1D2        16
>> +#define R8A7796_CLK_S1D4        17
>> +#define R8A7796_CLK_S2D1        18
>> +#define R8A7796_CLK_S2D2        19
>> +#define R8A7796_CLK_S2D4        20
>> +#define R8A7796_CLK_S3D1        21
>> +#define R8A7796_CLK_S3D2        22
>> +#define R8A7796_CLK_S3D4        23
>> +#define R8A7796_CLK_LB            24
>> +#define R8A7796_CLK_CL            25
>> +#define R8A7796_CLK_ZB3            26
>> +#define R8A7796_CLK_ZB3D2        27
>> +#define R8A7796_CLK_ZB3D4        28
>> +#define R8A7796_CLK_CR            29
>> +#define R8A7796_CLK_CRD2        30
>> +#define R8A7796_CLK_SD0H        31
>> +#define R8A7796_CLK_SD0            32
>> +#define R8A7796_CLK_SD1H        33
>> +#define R8A7796_CLK_SD1            34
>> +#define R8A7796_CLK_SD2H        35
>> +#define R8A7796_CLK_SD2            36
>> +#define R8A7796_CLK_SD3H        37
>> +#define R8A7796_CLK_SD3            38
>> +#define R8A7796_CLK_SSP2        39
>> +#define R8A7796_CLK_SSP1        40
>> +#define R8A7796_CLK_SSPRS        41
>> +#define R8A7796_CLK_RPC            42
>> +#define R8A7796_CLK_RPCD2        43
>> +#define R8A7796_CLK_MSO            44
>> +#define R8A7796_CLK_CANFD        45
>> +#define R8A7796_CLK_HDMI        46
>> +#define R8A7796_CLK_CSI0        47
>> +#define R8A7796_CLK_CSIREF        48
>> +#define R8A7796_CLK_CP            49
>> +#define R8A7796_CLK_CPEX        50
>> +#define R8A7796_CLK_R            51
>> +#define R8A7796_CLK_OSC            52
>
>
> I think we recently started a discussion to find a more clever way to
> avoid re-defining (copy & paste) all this R-Car3 clocks  (compare [1])
> where they are the same over the R-Car3 family while still being able to
> deal with the differences.
>
> Best regards
>
> Dirk
>
> [1]
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/dt-bindings/clock/r8a7795-cpg-mssr.h


What's the status of the discussion I mentioned above?


Best regards

Dirk

Re: [PATCH v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Geert Uytterhoeven]

Hi Dirk,

On Mon, Jun 6, 2016 at 2:03 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 30.05.2016 18:36, Dirk Behme wrote:
>> On 30.05.2016 18:28, Geert Uytterhoeven wrote:
>>> Add all R-Car M3-W Clock Pulse Generator Core Clock Outputs, as listed
>>> in Table 8.2b ("List of Clocks [R-Car M3-W]") of the R-Car Gen3
>>> datasheet (rev. 0.51 + Errata for Rev051 Mar 31 2016).
>>>
>>> Note that internal CPG clocks (S0, S1, S2, S3, SDSRC, and SSPSRC) are
>>> not included, as they are used as internal clock sources only, and never
>>> referenced from DT.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> Tested-by: Simon Horman <horms+renesas@verge.net.au>
>>> ---
>>> v2:
>>>    - Add Tested-by.
>>> ---
>>>   include/dt-bindings/clock/r8a7796-cpg-mssr.h | 69
>>> ++++++++++++++++++++++++++++
>>>   1 file changed, 69 insertions(+)
>>>   create mode 100644 include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>>
>>> diff --git a/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>> b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>> new file mode 100644
>>> index 0000000000000000..1e5942695f0dd057
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>> @@ -0,0 +1,69 @@
>>> +/*
>>> + * Copyright (C) 2016 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_R8A7796_CPG_MSSR_H__
>>> +#define __DT_BINDINGS_CLOCK_R8A7796_CPG_MSSR_H__
>>> +
>>> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
>>> +
>>> +/* r8a7796 CPG Core Clocks */
>>> +#define R8A7796_CLK_Z            0

[...]

>> I think we recently started a discussion to find a more clever way to
>> avoid re-defining (copy & paste) all this R-Car3 clocks  (compare [1])
>> where they are the same over the R-Car3 family while still being able to
>> deal with the differences.
>>
>> Best regards
>>
>> Dirk
>>
>> [1]
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/dt-bindings/clock/r8a7795-cpg-mssr.h
>
> What's the status of the discussion I mentioned above?

As mentioned in that thread,the CPGs in r8a7795 and r8a7796 provide
slightly different sets of clocks. Future members of the R-Car Gen3
family may provide the same or different sets of clocks, we don't know.

As Magnus already mentions, we try to stay as close as possible to the
datasheet (which is unfortunately a moving target, too).

For CPG Core Clocks, the datasheet only provides us with a list of named
clocks.  There are no fixed numbers. So either we refer to clocks by
name, or by coming up with our own numbering scheme (which has to be a
stable set of numbers, i.e. append only).

For MSSR (Module) Clocks, the datasheet does provide us with numbers
(MSTP register index + bit index inside the register).

The way the CPG/MSSR drivers handles these clocks was heavily influenced
by the experience we gained with the Common Clock Framework and DT on
R-Car Gen2.

R-Car Gen2 described all clocks and their registers in DT. The goal
(utopia?) here was to handle all SoCs from the family with a single
driver, provided it was fed with the right description in DT.

For CPG Core Clocks, this lead to a mix of:
  - Nodes for fixed factor clocks,
  - Nodes for variable factor clocks, specifying a register to operate
    on,
  - Special CPG clocks that couldn't be handled by the above, using a
    common (family-specific) list of definitions for clocks, that had to
    be extended constantly.

For MSSR Clocks (called "MSTP" for historical reasons), each set of 32
clocks had its own node, with multiple registers, and three separate
arrays for parent clocks, clock indices, and clock names, that had to be
kept in sync. The clock indices were defines, using numbers from the
datasheet, but they were still easy to abuse (which register does the
define apply to?).

As the CCF was quite new and best practices were still under
development, all of this was difficult to define up-front.
Due to the complexity, it was also hard to review and maintain, leading
to many errors.
The arbitrary (grown organically) offsets for the various MSSR-related
registers also made it hard to ever add module reset support.

Hence the call for a new framework, designed in close collaboration with the
clock maintainer, and implemented in the CPG/MSSR driver.
The goals were:
  - Make the DT part user friendly, reviewer friendly, and maintainer
    friendly, as it provides a stable ABI, and thus must be obviously
    correct from the beginning,
  - Hide complexity and internals in the driver, as this can be reworked
    and extended at any time, without breaking the DT ABI,
  - Hence, describe CPG/MSSR as a single simple block in DT,
  - Support both new and existing SoCs (PoC was done for r8a7791),
  - Allow for adding module reset support (the "SR" part) later.

Hence for the CPG Core Clocks, we wanted a simple append-only list of
defines for all clocks (and only those, as trying to use another clock
is a bug) that exist on a specific SoC. This allows to list all existing
clocks in the bindings include up-front.
A mapping from SoC-specific numbers to family-specific clocks is handled
by the tables in the driver, to promote code reuse while keeping
specialization.

For MSSR Clocks, we solved the disconnection of register and bit indices
by going with a single number. We also removed the defines, as it's
actually easier to review .dtsi updates if the MSSR clocks are directly
referred to by number, than by an intermediate define.

I hope this explanation helps in understanding the design choices we
made.  Given the small amount of work needed to bootstrap r8a7796, I
think they still hold on their promises.

Thanks!

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 v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Dirk Behme]

Hi Geert,

On 06.06.2016 14:59, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Mon, Jun 6, 2016 at 2:03 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 30.05.2016 18:36, Dirk Behme wrote:
>>> On 30.05.2016 18:28, Geert Uytterhoeven wrote:
>>>> Add all R-Car M3-W Clock Pulse Generator Core Clock Outputs, as listed
>>>> in Table 8.2b ("List of Clocks [R-Car M3-W]") of the R-Car Gen3
>>>> datasheet (rev. 0.51 + Errata for Rev051 Mar 31 2016).
>>>>
>>>> Note that internal CPG clocks (S0, S1, S2, S3, SDSRC, and SSPSRC) are
>>>> not included, as they are used as internal clock sources only, and never
>>>> referenced from DT.
>>>>
>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>>> Tested-by: Simon Horman <horms+renesas@verge.net.au>
>>>> ---
>>>> v2:
>>>>    - Add Tested-by.
>>>> ---
>>>>   include/dt-bindings/clock/r8a7796-cpg-mssr.h | 69
>>>> ++++++++++++++++++++++++++++
>>>>   1 file changed, 69 insertions(+)
>>>>   create mode 100644 include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>>>
>>>> diff --git a/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>>> b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>>> new file mode 100644
>>>> index 0000000000000000..1e5942695f0dd057
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>>> @@ -0,0 +1,69 @@
>>>> +/*
>>>> + * Copyright (C) 2016 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_R8A7796_CPG_MSSR_H__
>>>> +#define __DT_BINDINGS_CLOCK_R8A7796_CPG_MSSR_H__
>>>> +
>>>> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
>>>> +
>>>> +/* r8a7796 CPG Core Clocks */
>>>> +#define R8A7796_CLK_Z            0
>
> [...]
>
>>> I think we recently started a discussion to find a more clever way to
>>> avoid re-defining (copy & paste) all this R-Car3 clocks  (compare [1])
>>> where they are the same over the R-Car3 family while still being able to
>>> deal with the differences.
>>>
>>> Best regards
>>>
>>> Dirk
>>>
>>> [1]
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/dt-bindings/clock/r8a7795-cpg-mssr.h
>>
>> What's the status of the discussion I mentioned above?
>
> As mentioned in that thread,the CPGs in r8a7795 and r8a7796 provide
> slightly different sets of clocks. Future members of the R-Car Gen3
> family may provide the same or different sets of clocks, we don't know.
>
> As Magnus already mentions, we try to stay as close as possible to the
> datasheet (which is unfortunately a moving target, too).
>
> For CPG Core Clocks, the datasheet only provides us with a list of named
> clocks.  There are no fixed numbers. So either we refer to clocks by
> name, or by coming up with our own numbering scheme (which has to be a
> stable set of numbers, i.e. append only).
>
> For MSSR (Module) Clocks, the datasheet does provide us with numbers
> (MSTP register index + bit index inside the register).
>
> The way the CPG/MSSR drivers handles these clocks was heavily influenced
> by the experience we gained with the Common Clock Framework and DT on
> R-Car Gen2.
>
> R-Car Gen2 described all clocks and their registers in DT. The goal
> (utopia?) here was to handle all SoCs from the family with a single
> driver, provided it was fed with the right description in DT.
>
> For CPG Core Clocks, this lead to a mix of:
>   - Nodes for fixed factor clocks,
>   - Nodes for variable factor clocks, specifying a register to operate
>     on,
>   - Special CPG clocks that couldn't be handled by the above, using a
>     common (family-specific) list of definitions for clocks, that had to
>     be extended constantly.
>
> For MSSR Clocks (called "MSTP" for historical reasons), each set of 32
> clocks had its own node, with multiple registers, and three separate
> arrays for parent clocks, clock indices, and clock names, that had to be
> kept in sync. The clock indices were defines, using numbers from the
> datasheet, but they were still easy to abuse (which register does the
> define apply to?).
>
> As the CCF was quite new and best practices were still under
> development, all of this was difficult to define up-front.
> Due to the complexity, it was also hard to review and maintain, leading
> to many errors.
> The arbitrary (grown organically) offsets for the various MSSR-related
> registers also made it hard to ever add module reset support.
>
> Hence the call for a new framework, designed in close collaboration with the
> clock maintainer, and implemented in the CPG/MSSR driver.
> The goals were:
>   - Make the DT part user friendly, reviewer friendly, and maintainer
>     friendly, as it provides a stable ABI, and thus must be obviously
>     correct from the beginning,
>   - Hide complexity and internals in the driver, as this can be reworked
>     and extended at any time, without breaking the DT ABI,
>   - Hence, describe CPG/MSSR as a single simple block in DT,
>   - Support both new and existing SoCs (PoC was done for r8a7791),
>   - Allow for adding module reset support (the "SR" part) later.
>
> Hence for the CPG Core Clocks, we wanted a simple append-only list of
> defines for all clocks (and only those, as trying to use another clock
> is a bug) that exist on a specific SoC. This allows to list all existing
> clocks in the bindings include up-front.
> A mapping from SoC-specific numbers to family-specific clocks is handled
> by the tables in the driver, to promote code reuse while keeping
> specialization.
>
> For MSSR Clocks, we solved the disconnection of register and bit indices
> by going with a single number. We also removed the defines, as it's
> actually easier to review .dtsi updates if the MSSR clocks are directly
> referred to by number, than by an intermediate define.
>
> I hope this explanation helps in understanding the design choices we
> made.


Many thanks for taking the time for this long background information!

Please note that I don't doubt any of the design choices done.

I think I just want to discuss if we have a clever idea to further 
improve one detail. That is, if we have a clever idea to avoid the copy 
& paste between the family members using anything like a hierarchical 
way of defining the clocks in r8a779x-cpg-mssr.h.


> Given the small amount of work needed to bootstrap r8a7796, I
> think they still hold on their promises.


Well, regarding the r8a779x-cpg-mssr.h the small amount of work needed 
isn't a really good argument if you are good with cp & sed for the copy 
& paste done ;)


What I fear we end up the way we are doing the copy & paste 
r8a779x-cpg-mssr.h is having 5 or 6 or even more of these files, all > 
90% identical. And once you have to change anything, you either have to 
change all these files. Or you miss anything, ending up with subtle bugs 
when one SoC does behave differently than an other one.


Best regards

Dirk

Re: [PATCH v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Geert Uytterhoeven]

Hi Dirk,

On Tue, Jun 7, 2016 at 9:53 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> I think I just want to discuss if we have a clever idea to further improve
> one detail. That is, if we have a clever idea to avoid the copy & paste
> between the family members using anything like a hierarchical way of
> defining the clocks in r8a779x-cpg-mssr.h.
>
>> Given the small amount of work needed to bootstrap r8a7796, I
>> think they still hold on their promises.
>
> Well, regarding the r8a779x-cpg-mssr.h the small amount of work needed isn't
> a really good argument if you are good with cp & sed for the copy & paste
> done ;)

They're not really created by cp & sed, as they must match the table in the
datasheet (the latter may have been created by copy & paste though :-)

> What I fear we end up the way we are doing the copy & paste
> r8a779x-cpg-mssr.h is having 5 or 6 or even more of these files, all > 90%
> identical. And once you have to change anything, you either have to change
> all these files. Or you miss anything, ending up with subtle bugs when one
> SoC does behave differently than an other one.

The point is these files are stable ABI: no single value can be changed.
No value can be reused. Only new values can be appended at the bottom
(if a newer revision of the datasheet documents more clocks than the old
 version, which happens from time to time).

IMHO a hierarchical way of defining the clocks has more opportunity of
accidentally referring to a clock that doesn't exist on a particular SoC.

Furthermore, r8a779x-cpg-mssr.h is not a good name to be part of a DT binding,
due to the wildcard.
A future SoC may will match r8a779x and even be called (R-Car <something>3?),
while using a completely different CPG block.

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 v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Dirk Behme]

Hi Geert,

On 07.06.2016 10:17, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Tue, Jun 7, 2016 at 9:53 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> I think I just want to discuss if we have a clever idea to further improve
>> one detail. That is, if we have a clever idea to avoid the copy & paste
>> between the family members using anything like a hierarchical way of
>> defining the clocks in r8a779x-cpg-mssr.h.
>>
>>> Given the small amount of work needed to bootstrap r8a7796, I
>>> think they still hold on their promises.
>>
>> Well, regarding the r8a779x-cpg-mssr.h the small amount of work needed isn't
>> a really good argument if you are good with cp & sed for the copy & paste
>> done ;)
>
> They're not really created by cp & sed, as they must match the table in the
> datasheet (the latter may have been created by copy & paste though :-)
>
>> What I fear we end up the way we are doing the copy & paste
>> r8a779x-cpg-mssr.h is having 5 or 6 or even more of these files, all > 90%
>> identical. And once you have to change anything, you either have to change
>> all these files. Or you miss anything, ending up with subtle bugs when one
>> SoC does behave differently than an other one.
>
> The point is these files are stable ABI: no single value can be changed.
> No value can be reused. Only new values can be appended at the bottom
> (if a newer revision of the datasheet documents more clocks than the old
>  version, which happens from time to time).
>
> IMHO a hierarchical way of defining the clocks has more opportunity of
> accidentally referring to a clock that doesn't exist on a particular SoC.
>
> Furthermore, r8a779x-cpg-mssr.h is not a good name to be part of a DT binding,
> due to the wildcard.
> A future SoC may will match r8a779x and even be called (R-Car <something>3?),
> while using a completely different CPG block.


Just to give an example about what we are talking, I've done [1] below. 
This should just be an *example*, though. I'm sure that we might need a 
more clever way.

I'm interested in your opinion why it doesn't work this way ;)

Best regards

Dirk

[1]

  include/dt-bindings/clock/r8a7795-cpg-mssr.h |   51 ---------------------
  include/dt-bindings/clock/r8a7796-cpg-mssr.h |   61 
+++-----------------------
  include/dt-bindings/clock/rcar3-cpg-mssr.h   |   63 
+++++++++++++++++++++++++++
  3 files changed, 71 insertions(+), 104 deletions(-)

Index: b/include/dt-bindings/clock/r8a7795-cpg-mssr.h
===================================================================
--- a/include/dt-bindings/clock/r8a7795-cpg-mssr.h
+++ b/include/dt-bindings/clock/r8a7795-cpg-mssr.h
@@ -9,55 +9,6 @@
  #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
+#include <dt-bindings/clock/rcar3-cpg-mssr.h>

  #endif /* __DT_BINDINGS_CLOCK_R8A7795_CPG_MSSR_H__ */
Index: b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
===================================================================
--- a/include/dt-bindings/clock/r8a7796-cpg-mssr.h
+++ b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
@@ -9,61 +9,14 @@
  #ifndef __DT_BINDINGS_CLOCK_R8A7796_CPG_MSSR_H__
  #define __DT_BINDINGS_CLOCK_R8A7796_CPG_MSSR_H__

-#include <dt-bindings/clock/renesas-cpg-mssr.h>
+#include <dt-bindings/clock/rcar3-cpg-mssr.h>

  /* r8a7796 CPG Core Clocks */
-#define R8A7796_CLK_Z			0
-#define R8A7796_CLK_Z2			1
-#define R8A7796_CLK_ZR			2
-#define R8A7796_CLK_ZG			3
-#define R8A7796_CLK_ZTR			4
-#define R8A7796_CLK_ZTRD2		5
-#define R8A7796_CLK_ZT			6
-#define R8A7796_CLK_ZX			7
-#define R8A7796_CLK_S0D1		8
-#define R8A7796_CLK_S0D2		9
-#define R8A7796_CLK_S0D3		10
-#define R8A7796_CLK_S0D4		11
-#define R8A7796_CLK_S0D6		12
-#define R8A7796_CLK_S0D8		13
-#define R8A7796_CLK_S0D12		14
-#define R8A7796_CLK_S1D1		15
-#define R8A7796_CLK_S1D2		16
-#define R8A7796_CLK_S1D4		17
-#define R8A7796_CLK_S2D1		18
-#define R8A7796_CLK_S2D2		19
-#define R8A7796_CLK_S2D4		20
-#define R8A7796_CLK_S3D1		21
-#define R8A7796_CLK_S3D2		22
-#define R8A7796_CLK_S3D4		23
-#define R8A7796_CLK_LB			24
-#define R8A7796_CLK_CL			25
-#define R8A7796_CLK_ZB3			26
-#define R8A7796_CLK_ZB3D2		27
-#define R8A7796_CLK_ZB3D4		28
-#define R8A7796_CLK_CR			29
-#define R8A7796_CLK_CRD2		30
-#define R8A7796_CLK_SD0H		31
-#define R8A7796_CLK_SD0			32
-#define R8A7796_CLK_SD1H		33
-#define R8A7796_CLK_SD1			34
-#define R8A7796_CLK_SD2H		35
-#define R8A7796_CLK_SD2			36
-#define R8A7796_CLK_SD3H		37
-#define R8A7796_CLK_SD3			38
-#define R8A7796_CLK_SSP2		39
-#define R8A7796_CLK_SSP1		40
-#define R8A7796_CLK_SSPRS		41
-#define R8A7796_CLK_RPC			42
-#define R8A7796_CLK_RPCD2		43
-#define R8A7796_CLK_MSO			44
-#define R8A7796_CLK_CANFD		45
-#define R8A7796_CLK_HDMI		46
-#define R8A7796_CLK_CSI0		47
-#define R8A7796_CLK_CSIREF		48
-#define R8A7796_CLK_CP			49
-#define R8A7796_CLK_CPEX		50
-#define R8A7796_CLK_R			51
-#define R8A7796_CLK_OSC			52
+#define R8A7796_CLK_ZB3D4		47
+#define R8A7796_CLK_S0D2		48
+#define R8A7796_CLK_S0D3		49
+#define R8A7796_CLK_S0D6		50
+#define R8A7796_CLK_S0D8		51
+#define R8A7796_CLK_S0D12		53

  #endif /* __DT_BINDINGS_CLOCK_R8A7796_CPG_MSSR_H__ */
Index: b/include/dt-bindings/clock/rcar3-cpg-mssr.h
===================================================================
--- /dev/null
+++ b/include/dt-bindings/clock/rcar3-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_RCAR3_CPG_MSSR_H__
+#define __DT_BINDINGS_CLOCK_RCAR3_CPG_MSSR_H__
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/* RCar3 CPG Core Clocks */
+#define RCAR3_CLK_Z		0
+#define RCAR3_CLK_Z2		1
+#define RCAR3_CLK_ZR		2
+#define RCAR3_CLK_ZG		3
+#define RCAR3_CLK_ZTR		4
+#define RCAR3_CLK_ZTRD2		5
+#define RCAR3_CLK_ZT		6
+#define RCAR3_CLK_ZX		7
+#define RCAR3_CLK_S0D1		8
+#define RCAR3_CLK_S0D4		9
+#define RCAR3_CLK_S1D1		10
+#define RCAR3_CLK_S1D2		11
+#define RCAR3_CLK_S1D4		12
+#define RCAR3_CLK_S2D1		13
+#define RCAR3_CLK_S2D2		14
+#define RCAR3_CLK_S2D4		15
+#define RCAR3_CLK_S3D1		16
+#define RCAR3_CLK_S3D2		17
+#define RCAR3_CLK_S3D4		18
+#define RCAR3_CLK_LB		19
+#define RCAR3_CLK_CL		20
+#define RCAR3_CLK_ZB3		21
+#define RCAR3_CLK_ZB3D2		22
+#define RCAR3_CLK_CR		23
+#define RCAR3_CLK_CRD2		24
+#define RCAR3_CLK_SD0H		25
+#define RCAR3_CLK_SD0		26
+#define RCAR3_CLK_SD1H		27
+#define RCAR3_CLK_SD1		28
+#define RCAR3_CLK_SD2H		29
+#define RCAR3_CLK_SD2		30
+#define RCAR3_CLK_SD3H		31
+#define RCAR3_CLK_SD3		32
+#define RCAR3_CLK_SSP2		33
+#define RCAR3_CLK_SSP1		34
+#define RCAR3_CLK_SSPRS		35
+#define RCAR3_CLK_RPC		36
+#define RCAR3_CLK_RPCD2		37
+#define RCAR3_CLK_MSO		38
+#define RCAR3_CLK_CANFD		39
+#define RCAR3_CLK_HDMI		40
+#define RCAR3_CLK_CSI0		41
+#define RCAR3_CLK_CSIREF	42
+#define RCAR3_CLK_CP		43
+#define RCAR3_CLK_CPEX		44
+#define RCAR3_CLK_R		45
+#define RCAR3_CLK_OSC		46
+
+#endif /* __DT_BINDINGS_CLOCK_RCAR3_CPG_MSSR_H__ */

Re: [PATCH v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Jun 9, 2016 at 10:31 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 07.06.2016 10:17, Geert Uytterhoeven wrote:
>> On Tue, Jun 7, 2016 at 9:53 AM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>>
>>> I think I just want to discuss if we have a clever idea to further
>>> improve
>>> one detail. That is, if we have a clever idea to avoid the copy & paste
>>> between the family members using anything like a hierarchical way of
>>> defining the clocks in r8a779x-cpg-mssr.h.
>>>
>>>> Given the small amount of work needed to bootstrap r8a7796, I
>>>> think they still hold on their promises.
>>>
>>> Well, regarding the r8a779x-cpg-mssr.h the small amount of work needed
>>> isn't
>>> a really good argument if you are good with cp & sed for the copy & paste
>>> done ;)
>>
>> They're not really created by cp & sed, as they must match the table in
>> the
>> datasheet (the latter may have been created by copy & paste though :-)
>>
>>> What I fear we end up the way we are doing the copy & paste
>>> r8a779x-cpg-mssr.h is having 5 or 6 or even more of these files, all >
>>> 90%
>>> identical. And once you have to change anything, you either have to
>>> change
>>> all these files. Or you miss anything, ending up with subtle bugs when
>>> one
>>> SoC does behave differently than an other one.
>>
>> The point is these files are stable ABI: no single value can be changed.
>> No value can be reused. Only new values can be appended at the bottom
>> (if a newer revision of the datasheet documents more clocks than the old
>>  version, which happens from time to time).
>>
>> IMHO a hierarchical way of defining the clocks has more opportunity of
>> accidentally referring to a clock that doesn't exist on a particular SoC.
>>
>> Furthermore, r8a779x-cpg-mssr.h is not a good name to be part of a DT
>> binding,
>> due to the wildcard.
>> A future SoC may will match r8a779x and even be called (R-Car
>> <something>3?),
>> while using a completely different CPG block.
>
> Just to give an example about what we are talking, I've done [1] below. This
> should just be an *example*, though. I'm sure that we might need a more
> clever way.

Thanks, it matches with what I had in mind what you wanted to do ;-)

> --- a/include/dt-bindings/clock/r8a7796-cpg-mssr.h
> +++ b/include/dt-bindings/clock/r8a7796-cpg-mssr.h

> +#include <dt-bindings/clock/rcar3-cpg-mssr.h>

> +#define R8A7796_CLK_ZB3D4              47
> +#define R8A7796_CLK_S0D2               48
> +#define R8A7796_CLK_S0D3               49
> +#define R8A7796_CLK_S0D6               50
> +#define R8A7796_CLK_S0D8               51
> +#define R8A7796_CLK_S0D12              53

This means on M3-W, clocks may have different prefixes:
  - RCAR3_CLK_* if they are defined as common R-Car Gen3 clocks,
  - R8A7796_CLK_* if they are defined as M3-W-specific clocks.

Not such a big issue, it that can be worked around using

    #define R8A7796_CLK_Z RCAR3_CLK_Z
    [...]

> +++ b/include/dt-bindings/clock/rcar3-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_RCAR3_CPG_MSSR_H__
> +#define __DT_BINDINGS_CLOCK_RCAR3_CPG_MSSR_H__
> +
> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
> +
> +/* RCar3 CPG Core Clocks */
> +#define RCAR3_CLK_Z            0
> +#define RCAR3_CLK_Z2           1
> +#define RCAR3_CLK_ZR           2
> +#define RCAR3_CLK_ZG           3
> +#define RCAR3_CLK_ZTR          4
> +#define RCAR3_CLK_ZTRD2                5

While this approach allows to add SoC-specific clocks to the family-specific
base list, it does not allow to remove family-specific clocks that are not
present on unknown future species of the family.

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 v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Dirk Behme]

Hi Geert,

On 09.06.2016 10:54, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Thu, Jun 9, 2016 at 10:31 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 07.06.2016 10:17, Geert Uytterhoeven wrote:
>>> On Tue, Jun 7, 2016 at 9:53 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>> wrote:
>>>>
>>>> I think I just want to discuss if we have a clever idea to further
>>>> improve
>>>> one detail. That is, if we have a clever idea to avoid the copy & paste
>>>> between the family members using anything like a hierarchical way of
>>>> defining the clocks in r8a779x-cpg-mssr.h.
>>>>
>>>>> Given the small amount of work needed to bootstrap r8a7796, I
>>>>> think they still hold on their promises.
>>>>
>>>> Well, regarding the r8a779x-cpg-mssr.h the small amount of work needed
>>>> isn't
>>>> a really good argument if you are good with cp & sed for the copy & paste
>>>> done ;)
>>>
>>> They're not really created by cp & sed, as they must match the table in
>>> the
>>> datasheet (the latter may have been created by copy & paste though :-)
>>>
>>>> What I fear we end up the way we are doing the copy & paste
>>>> r8a779x-cpg-mssr.h is having 5 or 6 or even more of these files, all >
>>>> 90%
>>>> identical. And once you have to change anything, you either have to
>>>> change
>>>> all these files. Or you miss anything, ending up with subtle bugs when
>>>> one
>>>> SoC does behave differently than an other one.
>>>
>>> The point is these files are stable ABI: no single value can be changed.
>>> No value can be reused. Only new values can be appended at the bottom
>>> (if a newer revision of the datasheet documents more clocks than the old
>>>  version, which happens from time to time).
>>>
>>> IMHO a hierarchical way of defining the clocks has more opportunity of
>>> accidentally referring to a clock that doesn't exist on a particular SoC.
>>>
>>> Furthermore, r8a779x-cpg-mssr.h is not a good name to be part of a DT
>>> binding,
>>> due to the wildcard.
>>> A future SoC may will match r8a779x and even be called (R-Car
>>> <something>3?),
>>> while using a completely different CPG block.
>>
>> Just to give an example about what we are talking, I've done [1] below. This
>> should just be an *example*, though. I'm sure that we might need a more
>> clever way.
>
> Thanks, it matches with what I had in mind what you wanted to do ;-)
>
>> --- a/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>> +++ b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>
>> +#include <dt-bindings/clock/rcar3-cpg-mssr.h>
>
>> +#define R8A7796_CLK_ZB3D4              47
>> +#define R8A7796_CLK_S0D2               48
>> +#define R8A7796_CLK_S0D3               49
>> +#define R8A7796_CLK_S0D6               50
>> +#define R8A7796_CLK_S0D8               51
>> +#define R8A7796_CLK_S0D12              53
>
> This means on M3-W, clocks may have different prefixes:
>   - RCAR3_CLK_* if they are defined as common R-Car Gen3 clocks,
>   - R8A7796_CLK_* if they are defined as M3-W-specific clocks.
>
> Not such a big issue, it that can be worked around using
>
>     #define R8A7796_CLK_Z RCAR3_CLK_Z
>     [...]
>
>> +++ b/include/dt-bindings/clock/rcar3-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_RCAR3_CPG_MSSR_H__
>> +#define __DT_BINDINGS_CLOCK_RCAR3_CPG_MSSR_H__
>> +
>> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
>> +
>> +/* RCar3 CPG Core Clocks */
>> +#define RCAR3_CLK_Z            0
>> +#define RCAR3_CLK_Z2           1
>> +#define RCAR3_CLK_ZR           2
>> +#define RCAR3_CLK_ZG           3
>> +#define RCAR3_CLK_ZTR          4
>> +#define RCAR3_CLK_ZTRD2                5
>
> While this approach allows to add SoC-specific clocks to the family-specific
> base list, it does not allow to remove family-specific clocks that are not
> present on unknown future species of the family.


If I answer with

#undef

that would be too easy? ;)


Best regards

Dirk

Re: [PATCH v2 2/4] clk: renesas: Add r8a7796 CPG Core Clock Definitions

[Dirk Behme]

Hi Geert,

On 10.06.2016 08:34, Dirk Behme wrote:
> Hi Geert,
>
> On 09.06.2016 10:54, Geert Uytterhoeven wrote:
>> Hi Dirk,
>>
>> On Thu, Jun 9, 2016 at 10:31 AM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> On 07.06.2016 10:17, Geert Uytterhoeven wrote:
>>>> On Tue, Jun 7, 2016 at 9:53 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>>> wrote:
>>>>>
>>>>> I think I just want to discuss if we have a clever idea to further
>>>>> improve
>>>>> one detail. That is, if we have a clever idea to avoid the copy &
>>>>> paste
>>>>> between the family members using anything like a hierarchical way of
>>>>> defining the clocks in r8a779x-cpg-mssr.h.
>>>>>
>>>>>> Given the small amount of work needed to bootstrap r8a7796, I
>>>>>> think they still hold on their promises.
>>>>>
>>>>> Well, regarding the r8a779x-cpg-mssr.h the small amount of work needed
>>>>> isn't
>>>>> a really good argument if you are good with cp & sed for the copy &
>>>>> paste
>>>>> done ;)
>>>>
>>>> They're not really created by cp & sed, as they must match the table in
>>>> the
>>>> datasheet (the latter may have been created by copy & paste though :-)
>>>>
>>>>> What I fear we end up the way we are doing the copy & paste
>>>>> r8a779x-cpg-mssr.h is having 5 or 6 or even more of these files, all >
>>>>> 90%
>>>>> identical. And once you have to change anything, you either have to
>>>>> change
>>>>> all these files. Or you miss anything, ending up with subtle bugs when
>>>>> one
>>>>> SoC does behave differently than an other one.
>>>>
>>>> The point is these files are stable ABI: no single value can be
>>>> changed.
>>>> No value can be reused. Only new values can be appended at the bottom
>>>> (if a newer revision of the datasheet documents more clocks than the
>>>> old
>>>>  version, which happens from time to time).
>>>>
>>>> IMHO a hierarchical way of defining the clocks has more opportunity of
>>>> accidentally referring to a clock that doesn't exist on a particular
>>>> SoC.
>>>>
>>>> Furthermore, r8a779x-cpg-mssr.h is not a good name to be part of a DT
>>>> binding,
>>>> due to the wildcard.
>>>> A future SoC may will match r8a779x and even be called (R-Car
>>>> <something>3?),
>>>> while using a completely different CPG block.
>>>
>>> Just to give an example about what we are talking, I've done [1]
>>> below. This
>>> should just be an *example*, though. I'm sure that we might need a more
>>> clever way.
>>
>> Thanks, it matches with what I had in mind what you wanted to do ;-)
>>
>>> --- a/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>> +++ b/include/dt-bindings/clock/r8a7796-cpg-mssr.h
>>
>>> +#include <dt-bindings/clock/rcar3-cpg-mssr.h>
>>
>>> +#define R8A7796_CLK_ZB3D4              47
>>> +#define R8A7796_CLK_S0D2               48
>>> +#define R8A7796_CLK_S0D3               49
>>> +#define R8A7796_CLK_S0D6               50
>>> +#define R8A7796_CLK_S0D8               51
>>> +#define R8A7796_CLK_S0D12              53
>>
>> This means on M3-W, clocks may have different prefixes:
>>   - RCAR3_CLK_* if they are defined as common R-Car Gen3 clocks,
>>   - R8A7796_CLK_* if they are defined as M3-W-specific clocks.
>>
>> Not such a big issue, it that can be worked around using
>>
>>     #define R8A7796_CLK_Z RCAR3_CLK_Z
>>     [...]
>>
>>> +++ b/include/dt-bindings/clock/rcar3-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_RCAR3_CPG_MSSR_H__
>>> +#define __DT_BINDINGS_CLOCK_RCAR3_CPG_MSSR_H__
>>> +
>>> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
>>> +
>>> +/* RCar3 CPG Core Clocks */
>>> +#define RCAR3_CLK_Z            0
>>> +#define RCAR3_CLK_Z2           1
>>> +#define RCAR3_CLK_ZR           2
>>> +#define RCAR3_CLK_ZG           3
>>> +#define RCAR3_CLK_ZTR          4
>>> +#define RCAR3_CLK_ZTRD2                5
>>
>> While this approach allows to add SoC-specific clocks to the
>> family-specific
>> base list, it does not allow to remove family-specific clocks that are
>> not
>> present on unknown future species of the family.
>
>
> If I answer with
>
> #undef
>
> that would be too easy? ;)


How do we continue the discussion about a hierarchical structure of the 
RCar3 CPG clock definitions?


Best regards

Dirk