ARM: shmobile: r8a7779, marzen: CCF and multiplatform

drivers/clk/shmobile/clk-r8a7779.c new file +27 -0

+++ b/drivers/clk/shmobile/clk-r8a7779.c
 
+/*
+ *             MD1 = 1                 MD1 = 0
+ *             (PLLA = 1500)           (PLLA = 1600)
+ *             (MHz)                   (MHz)
+ *------------------------------------------------+--------------------
+ * clkz                1000   (2/3)            800   (1/2)
+ * clkzs        250   (1/6)            200   (1/8)
+ * clki                 750   (1/2)            800   (1/2)
+ * clks                 250   (1/6)            200   (1/8)
+ * clks1        125   (1/12)           100   (1/16)
+ * clks3        187.5 (1/8)            200   (1/8)
+ * clks4         93.7 (1/16)           100   (1/16)
+ * clkp                  62.5 (1/24)            50   (1/32)
+ * clkg                  62.5 (1/24)            66.6 (1/24)
+ * clkb, CLKOUT
+ * (MD2 = 0)     62.5 (1/24)            66.6 (1/24)
+ * (MD2 = 1)     41.6 (1/36)            50   (1/32)
+ */
+
+#define CPG_CLK_CONFIG_INDEX(md)       (((md) & (BIT(1)|BIT(2))) >> 1)
+
+struct cpg_clk_config {
+       unsigned int z_mult;
+       unsigned int z_div;
+       unsigned int zs_and_s_div;
+       unsigned int s1_div;

drivers/clk/shmobile/clk-r8a7779.c new file +7 -0

+/*
+ *   MD                PLLA Ratio
+ * 12 11
+ *------------------------
+ * 0  0                x42
+ * 0  1                x48
+ * 1  0                x56

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt line 4

Quoted 6 lines · Simon Horman

+++ b/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +4 -0

@@ -0,0 +1,26 @@
+* Renesas R8A7779 Clock Pulse Generator (CPG)
+
+The CPG generates core clocks for the R8A7779. It includes one PLL and
+and several fixed ratio dividers

Double "and"

Quoted 28 lines

drivers/clk/shmobile/clk-r8a7779.c new file +27 -0

+++ b/drivers/clk/shmobile/clk-r8a7779.c
 
+/*
+ *             MD1 = 1                 MD1 = 0
+ *             (PLLA = 1500)           (PLLA = 1600)
+ *             (MHz)                   (MHz)
+ *------------------------------------------------+--------------------
+ * clkz                1000   (2/3)            800   (1/2)
+ * clkzs        250   (1/6)            200   (1/8)
+ * clki                 750   (1/2)            800   (1/2)
+ * clks                 250   (1/6)            200   (1/8)
+ * clks1        125   (1/12)           100   (1/16)
+ * clks3        187.5 (1/8)            200   (1/8)
+ * clks4         93.7 (1/16)           100   (1/16)
+ * clkp                  62.5 (1/24)            50   (1/32)
+ * clkg                  62.5 (1/24)            66.6 (1/24)
+ * clkb, CLKOUT
+ * (MD2 = 0)     62.5 (1/24)            66.6 (1/24)
+ * (MD2 = 1)     41.6 (1/36)            50   (1/32)
+ */
+
+#define CPG_CLK_CONFIG_INDEX(md)       (((md) & (BIT(1)|BIT(2))) >> 1)
+
+struct cpg_clk_config {
+       unsigned int z_mult;
+       unsigned int z_div;
+       unsigned int zs_and_s_div;
+       unsigned int s1_div;

drivers/clk/shmobile/clk-r8a7779.c line 71

Quoted 10 lines

drivers/clk/shmobile/clk-r8a7779.c new file +10 -0

+       unsigned int p_div;
+       unsigned int out_div;
+};
+
+static const struct cpg_clk_config cpg_clk_configs[4] __initconst = {
+       { 1, 2, 8, 16, 32, 24 },
+       { 1, 2, 8, 16, 32, 24 },
+       { 2, 3, 6, 12, 24, 36 },
+       { 2, 3, 6, 12, 24, 32 },
+};

Shouldn't this be

drivers/clk/shmobile/clk-r8a7779.c new file +6 -0

+static const struct cpg_clk_config cpg_clk_configs[4] __initconst = {
+       { 1, 2, 8, 16, 32, 24 },
+       { 2, 3, 6, 12, 24, 24 },
+       { 1, 2, 8, 16, 32, 32 },
+       { 2, 3, 6, 12, 24, 36 },
+};

?

I think you got confused by writing the bitmask as "BIT(1)|BIT(2)" instead of "BIT(2)|BIT(1)".

Quoted 7 lines

drivers/clk/shmobile/clk-r8a7779.c new file +7 -0

+/*
+ *   MD                PLLA Ratio
+ * 12 11
+ *------------------------
+ * 0  0                x42
+ * 0  1                x48
+ * 1  0                x56

drivers/clk/shmobile/clk-r8a7779.c line 89

Quoted 10 lines

drivers/clk/shmobile/clk-r8a7779.c new file +10 -0

+ * 1  1                x64
+ */
+#define CPG_PLL_CONFIG_INDEX(md)       ((((md) & BIT(14)) >> 12) | \
+                                        (((md) & BIT(13)) >> 12) | \
+                                        (((md) & BIT(19)) >> 19))
+struct cpg_pll_config {
+       unsigned int extal_div;
+       unsigned int pll1_mult;
+       unsigned int pll3_mult;
+};

CPG_PLL_CONFIG_INDEX() and struct cpg_pll_config are unused, and disrupt the reading experience for the casual reviewer, as they split the section about MD12 and M11 in two.

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+#define CPG_PLLA_MULT_INDEX(md)        (((md) & (BIT(12)|BIT(11))) >> 11)

Quoted 46 lines

Quoted 26 lines

drivers/clk/shmobile/clk-r8a7779.c new file +26 -0

+/*
+ *             MD1 = 1                 MD1 = 0
+ *             (PLLA = 1500)           (PLLA = 1600)
+ *             (MHz)                   (MHz)
+ *------------------------------------------------+--------------------
+ * clkz                1000   (2/3)            800   (1/2)
+ * clkzs        250   (1/6)            200   (1/8)
+ * clki                 750   (1/2)            800   (1/2)
+ * clks                 250   (1/6)            200   (1/8)
+ * clks1        125   (1/12)           100   (1/16)
+ * clks3        187.5 (1/8)            200   (1/8)
+ * clks4         93.7 (1/16)           100   (1/16)
+ * clkp                  62.5 (1/24)            50   (1/32)
+ * clkg                  62.5 (1/24)            66.6 (1/24)
+ * clkb, CLKOUT
+ * (MD2 = 0)     62.5 (1/24)            66.6 (1/24)
+ * (MD2 = 1)     41.6 (1/36)            50   (1/32)
+ */
+
+#define CPG_CLK_CONFIG_INDEX(md)       (((md) & (BIT(1)|BIT(2))) >> 1)
+
+struct cpg_clk_config {
+       unsigned int z_mult;
+       unsigned int z_div;
+       unsigned int zs_and_s_div;
+       unsigned int s1_div;

drivers/clk/shmobile/clk-r8a7779.c line 71

Quoted 10 lines

drivers/clk/shmobile/clk-r8a7779.c new file +10 -0

+       unsigned int p_div;
+       unsigned int out_div;
+};
+
+static const struct cpg_clk_config cpg_clk_configs[4] __initconst = {
+       { 1, 2, 8, 16, 32, 24 },
+       { 1, 2, 8, 16, 32, 24 },
+       { 2, 3, 6, 12, 24, 36 },
+       { 2, 3, 6, 12, 24, 32 },
+};

Shouldn't this be

drivers/clk/shmobile/clk-r8a7779.c new file +6 -0

+static const struct cpg_clk_config cpg_clk_configs[4] __initconst = {
+       { 1, 2, 8, 16, 32, 24 },
+       { 2, 3, 6, 12, 24, 24 },
+       { 1, 2, 8, 16, 32, 32 },
+       { 2, 3, 6, 12, 24, 36 },
+};

Quoted 4 lines

?

I think you got confused by writing the bitmask as "BIT(1)|BIT(2)" instead of "BIT(2)|BIT(1)".

Probably, thanks for noticing.

Quoted 46 lines · Simon Horman

Quoted 46 lines

Quoted 26 lines

drivers/clk/shmobile/clk-r8a7779.c new file +26 -0

+/*
+ *             MD1 = 1                 MD1 = 0
+ *             (PLLA = 1500)           (PLLA = 1600)
+ *             (MHz)                   (MHz)
+ *------------------------------------------------+--------------------
+ * clkz                1000   (2/3)            800   (1/2)
+ * clkzs        250   (1/6)            200   (1/8)
+ * clki                 750   (1/2)            800   (1/2)
+ * clks                 250   (1/6)            200   (1/8)
+ * clks1        125   (1/12)           100   (1/16)
+ * clks3        187.5 (1/8)            200   (1/8)
+ * clks4         93.7 (1/16)           100   (1/16)
+ * clkp                  62.5 (1/24)            50   (1/32)
+ * clkg                  62.5 (1/24)            66.6 (1/24)
+ * clkb, CLKOUT
+ * (MD2 = 0)     62.5 (1/24)            66.6 (1/24)
+ * (MD2 = 1)     41.6 (1/36)            50   (1/32)
+ */
+
+#define CPG_CLK_CONFIG_INDEX(md)       (((md) & (BIT(1)|BIT(2))) >> 1)
+
+struct cpg_clk_config {
+       unsigned int z_mult;
+       unsigned int z_div;
+       unsigned int zs_and_s_div;
+       unsigned int s1_div;

drivers/clk/shmobile/clk-r8a7779.c line 71

Quoted 10 lines

drivers/clk/shmobile/clk-r8a7779.c new file +10 -0

+       unsigned int p_div;
+       unsigned int out_div;
+};
+
+static const struct cpg_clk_config cpg_clk_configs[4] __initconst = {
+       { 1, 2, 8, 16, 32, 24 },
+       { 1, 2, 8, 16, 32, 24 },
+       { 2, 3, 6, 12, 24, 36 },
+       { 2, 3, 6, 12, 24, 32 },
+};

Shouldn't this be

drivers/clk/shmobile/clk-r8a7779.c new file +6 -0

+static const struct cpg_clk_config cpg_clk_configs[4] __initconst = {
+       { 1, 2, 8, 16, 32, 24 },
+       { 2, 3, 6, 12, 24, 24 },
+       { 1, 2, 8, 16, 32, 32 },
+       { 2, 3, 6, 12, 24, 36 },
+};

Quoted 6 lines

Quoted 4 lines

?

I think you got confused by writing the bitmask as "BIT(1)|BIT(2)" instead of "BIT(2)|BIT(1)".

Probably, thanks for noticing.

Note that I based my review on the table above, not on the actual datasheet.

The R8A7779 SoC has several clocks that are too custom to be supported in a generic driver. Those clocks can be divided in two categories:

- Fixed rate clocks with multiplier and divisor set according to boot mode configuration

- Custom divider clocks with SoC-specific divider values

This driver supports both.

Based on work for R-Car Gen2 SoCs by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart
- Added external clock input
- Use PLLA ratio set bu MD11 and MD12
- Add _div suffixes of fields of struct cpt_clk_config
- Register PLLA as a fixed factor clock
- Use sizeof() instead of sizeof
- Use num_clks instead of CPG_NUM_CLOCKS in r8a7779_cpg_clocks_init()

- I kept this as r8a7779 binding rather than moving to a R-Car Gen1 binding which could be shared with other SoCs as I do not believe that the SoCs is are sufficiently similar.

---
 .../bindings/clock/renesas,r8a7779-cpg-clocks.txt  |  26 +++
 drivers/clk/shmobile/Makefile                      |   1 +
 drivers/clk/shmobile/clk-r8a7779.c                 | 191 ++++++++++++++++++
 include/linux/clk/shmobile.h                       |   3 +
 4 files changed, 221 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt create mode 100644 drivers/clk/shmobile/clk-r8a7779.c

diff --git a/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file mode 100644
index 0000000..1461323

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +13 -0

@@ -0,0 +1,26 @@
+* Renesas R8A7779 Clock Pulse Generator (CPG)
+
+The CPG generates core clocks for the R8A7779. It includes one PLL and
+and several fixed ratio dividers
+
+Required Properties:
+
+  - compatible: Must be "renesas,r8a7779-cpg-clocks"
+  - reg: Base address and length of the memory resource used by the CPG
+
+  - clocks: Reference to the parent clock
+  - #clock-cells: Must be 1
+  - clock-output-names: The names of the clocks. Supported clocks are

"plla",
+ "z", "zs", "s", "s1", "p", "out".

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +4 -0

+		clocks = <&extal_clk>;
+		#clock-cells = <1>;
+		clock-output-names = "plla", "z", "zs", "s", "s1", "p", "out";
+	};

drivers/clk/shmobile/Makefile +1 -0

index 9ecef14..2121ba0 100644
@@ -1,4 +1,5 @@
 obj-$(CONFIG_ARCH_EMEV2)		+= clk-emev2.o
+obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o
 obj-$(CONFIG_ARCH_R8A7791)		+= clk-rcar-gen2.o
 obj-$(CONFIG_ARCH_SHMOBILE_MULTI)	+= clk-div6.o

(quoted diff) +0 -0

b/drivers/clk/shmobile/clk-r8a7779.c new file mode 100644 index 0000000..2ca2d67

drivers/clk/shmobile/clk-r8a7779.c new file +92 -2

@@ -0,0 +1,191 @@
+/*
+ * r8a7779 Core CPG Clocks
+ *
+ * Copyright (C) 2013, 2014 Horms Solutions Ltd.
+ *
+ * Contact: Simon Horman <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/clk/shmobile.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/spinlock.h>
+
+#include <dt-bindings/clock/r8a7779-clock.h>
+
+#define CPG_NUM_CLOCKS			(R8A7779_CLK_OUT + 1)
+
+struct r8a7779_cpg {
+	struct clk_onecell_data data;
+	spinlock_t lock;
+	void __iomem *reg;
+};
+
+/*
---------------------------------------------------------------------------
-- + * CPG Clock Data
+ */
+
+/*
+ *		MD1 = 1			MD1 = 0
+ *		(PLLA = 1500)		(PLLA = 1600)
+ *		(MHz)			(MHz)
+ *------------------------------------------------+--------------------
+ * clkz		1000   (2/3)		800   (1/2)
+ * clkzs	 250   (1/6)		200   (1/8)
+ * clki		 750   (1/2)		800   (1/2)
+ * clks		 250   (1/6)		200   (1/8)
+ * clks1	 125   (1/12)		100   (1/16)
+ * clks3	 187.5 (1/8)		200   (1/8)
+ * clks4	  93.7 (1/16)		100   (1/16)
+ * clkp		  62.5 (1/24)		 50   (1/32)
+ * clkg		  62.5 (1/24)		 66.6 (1/24)
+ * clkb, CLKOUT
+ * (MD2 = 0)	  62.5 (1/24)		 66.6 (1/24)
+ * (MD2 = 1)	  41.6 (1/36)		 50   (1/32)
+ */
+
+#define CPG_CLK_CONFIG_INDEX(md)	(((md) & (BIT(1)|BIT(2))) >> 1)
+
+struct cpg_clk_config {
+	unsigned int z_mult;
+	unsigned int z_div;
+	unsigned int zs_and_s_div;
+	unsigned int s1_div;
+	unsigned int p_div;
+	unsigned int out_div;
+};
+
+static const struct cpg_clk_config cpg_clk_configs[4] __initconst = {
+	{ 1, 2, 8, 16, 32, 24 },
+	{ 1, 2, 8, 16, 32, 24 },
+	{ 2, 3, 6, 12, 24, 36 },
+	{ 2, 3, 6, 12, 24, 32 },
+};
+
+/*
+ *   MD		PLLA Ratio
+ * 12 11
+ *------------------------
+ * 0  0		x42
+ * 0  1		x48
+ * 1  0		x56
+ * 1  1		x64
+ */
+#define CPG_PLL_CONFIG_INDEX(md)	((((md) & BIT(14)) >> 12) | \
+					 (((md) & BIT(13)) >> 12) | \
+					 (((md) & BIT(19)) >> 19))
+struct cpg_pll_config {
+	unsigned int extal_div;
+	unsigned int pll1_mult;
+	unsigned int pll3_mult;
+};
+
+#define CPG_PLLA_MULT_INDEX(md)	(((md) & (BIT(12)|BIT(11))) >> 11)
+
+static const unsigned int cpg_plla_mult[4] __initconst = { 42, 48, 56, 64

};

drivers/clk/shmobile/clk-r8a7779.c new file +21 -0

+
+/* ------------------------------------------------------------------------
+ * Initialization
+ */
+
+static u32 cpg_mode __initdata;
+
+static struct clk * __init
+r8a7779_cpg_register_clock(struct device_node *np, struct r8a7779_cpg *cpg,
+			   const struct cpg_clk_config *config,
+			   unsigned int plla_mult, const char *name)
+{
+	const char *parent_name = "plla";
+	unsigned int mult = 1;
+	unsigned int div = 1;
+
+	if (!strcmp(name, "plla")) {
+		parent_name = of_clk_get_parent_name(np, 0);
+		mult = plla_mult;
+	} else if (!strcmp(name, "z")) {
+		div = config->z_div;

drivers/clk/shmobile/clk-r8a7779.c new file +29 -0

+
+	return clk_register_fixed_factor(NULL, name, parent_name, 0, mult, div);
+}
+
+static void __init r8a7779_cpg_clocks_init(struct device_node *np)
+{
+	const struct cpg_clk_config *config;
+	struct r8a7779_cpg *cpg;
+	struct clk **clks;
+	unsigned int i, plla_mult;
+	int num_clks;
+
+	num_clks = of_property_count_strings(np, "clock-output-names");
+	if (num_clks < 0) {
+		pr_err("%s: failed to count clocks\n", __func__);
+		return;
+	}
+
+	cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
+	clks = kzalloc(CPG_NUM_CLOCKS * sizeof(*clks), GFP_KERNEL);
+	if (cpg == NULL || clks == NULL) {
+		/* We're leaking memory on purpose, there's no point in cleaning
+		 * up as the system won't boot anyway.
+		 */
+		pr_err("%s: failed to allocate cpg\n", __func__);
+		return;
+	}
+
+	spin_lock_init(&cpg->lock);

drivers/clk/shmobile/clk-r8a7779.c new file +28 -0

+
+	for (i = 0; i < num_clks; ++i) {
+		const char *name;
+		struct clk *clk;
+
+		of_property_read_string_index(np, "clock-output-names", i,
+					      &name);
+
+		clk = r8a7779_cpg_register_clock(np, cpg, config,
+						 plla_mult, name);
+		if (IS_ERR(clk))
+			pr_err("%s: failed to register %s %s clock (%ld)\n",
+			       __func__, np->name, name, PTR_ERR(clk));
+		else
+			cpg->data.clks[i] = clk;
+	}
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &cpg->data);
+}
+CLK_OF_DECLARE(r8a7779_cpg_clks, "renesas,r8a7779-cpg-clocks",
+	       r8a7779_cpg_clocks_init);
+
+void __init r8a7779_clocks_init(u32 mode)
+{
+	cpg_mode = mode;
+
+	of_clk_init(NULL);
+}

include/linux/clk/shmobile.h +0 -0

index f9bf080..7667f49 100644

include/linux/clk/shmobile.h +2 -0

  *
  * Contact: Laurent Pinchart <[email protected]>
+ * Contact: Simon Horman <[email protected]>
  *
  * 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

@@ -14,6 +16,7 @@
 
 #include <linux/types.h>
 
+void r8a7779_clocks_init(u32 mode);
 void rcar_gen2_clocks_init(u32 mode);
 
 #endif

Quoted 9 lines · On Wednesday 26 February 2014 16:33:17 Simon Horman wrote:

The R8A7779 SoC has several clocks that are too custom to be supported in a generic driver. Those clocks can be divided in two categories:

- Fixed rate clocks with multiplier and divisor set according to boot mode configuration

- Custom divider clocks with SoC-specific divider values

This driver supports both.

Looking at the R8A7779 datasheet it looks like we only have fixed rate clocks, without any configurable divider clock. Did I miss something ?

Quoted 16 lines

Based on work for R-Car Gen2 SoCs by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart

- Added external clock input
- Use PLLA ratio set bu MD11 and MD12
- Add _div suffixes of fields of struct cpt_clk_config
- Register PLLA as a fixed factor clock
- Use sizeof() instead of sizeof
- Use num_clks instead of CPG_NUM_CLOCKS in r8a7779_cpg_clocks_init()

Quoted 4 lines

- I kept this as r8a7779 binding rather than moving to a R-Car Gen1

binding which could be shared with other SoCs as I do not believe that the SoCs is are sufficiently similar.

I had a look at the M1 datasheet and I still find its CPG very similar with the H1 CPG. The PLLA multiplier and divider are different, but if you look closely, they're both exactly twice the value compared to H1, so there's no difference in practice.

What differences do you see that would make it impractical to share a single driver for both ?

Quoted 32 lines

---

 .../bindings/clock/renesas,r8a7779-cpg-clocks.txt  |  26 +++
 drivers/clk/shmobile/Makefile                      |   1 +
 drivers/clk/shmobile/clk-r8a7779.c                 | 191
 ++++++++++++++++++
 include/linux/clk/shmobile.h                       |   3 +
 4 files changed, 221 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt create mode 100644 drivers/clk/shmobile/clk-r8a7779.c

diff --git a/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file mode 100644
index 0000000..1461323
--- /dev/null
+++
b/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +10 -0

@@ -0,0 +1,26 @@
+* Renesas R8A7779 Clock Pulse Generator (CPG)
+
+The CPG generates core clocks for the R8A7779. It includes one PLL and
+and several fixed ratio dividers
+
+Required Properties:
+
+  - compatible: Must be "renesas,r8a7779-cpg-clocks"
+  - reg: Base address and length of the memory resource used by the CPG
+

Quoted 5 lines

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +3 -0

+  - clocks: Reference to the parent clock
+  - #clock-cells: Must be 1
+  - clock-output-names: The names of the clocks. Supported clocks are

"plla",
+ "z", "zs", "s", "s1", "p", "out".

What about clki, clks3, clks4, clkb and clkg ? Should pllb be exposed as well ?

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt line 22

Quoted 7 lines

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +6 -0

+Example
+-------
+
+	cpg_clocks: cpg_clocks at ffc80000 {
+		compatible = "renesas,r8a7779-cpg-clocks";
+		reg = <0 0xffc80000 0 0x80>;

Shouldn't the range be restricted not to include the MSTP registers ? 0x30 should be enough.

Quoted 142 lines

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +4 -0

+		clocks = <&extal_clk>;
+		#clock-cells = <1>;
+		clock-output-names = "plla", "z", "zs", "s", "s1", "p", "out";
+	};

drivers/clk/shmobile/Makefile +1 -0

index 9ecef14..2121ba0 100644
@@ -1,4 +1,5 @@
 
 obj-$(CONFIG_ARCH_EMEV2)		+= clk-emev2.o
 
+obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
 

obj-$(CONFIG_ARCH_R8A7790) += clk-rcar-gen2.o
obj-$(CONFIG_ARCH_R8A7791) += clk-rcar-gen2.o
obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += clk-div6.o

(quoted diff) +0 -0

b/drivers/clk/shmobile/clk-r8a7779.c new file mode 100644 index 0000000..2ca2d67

drivers/clk/shmobile/clk-r8a7779.c new file +92 -3

@@ -0,0 +1,191 @@
+/*
+ * r8a7779 Core CPG Clocks
+ *
+ * Copyright (C) 2013, 2014 Horms Solutions Ltd.
+ *
+ * Contact: Simon Horman <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/clk/shmobile.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/spinlock.h>
+
+#include <dt-bindings/clock/r8a7779-clock.h>
+
+#define CPG_NUM_CLOCKS			(R8A7779_CLK_OUT + 1)
+
+struct r8a7779_cpg {
+	struct clk_onecell_data data;
+	spinlock_t lock;
+	void __iomem *reg;
+};
+
+/*
--------------------------------------------------------------------------
-
-- + * CPG Clock Data
+ */
+
+/*
+ *		MD1 = 1			MD1 = 0
+ *		(PLLA = 1500)		(PLLA = 1600)
+ *		(MHz)			(MHz)
+ *------------------------------------------------+--------------------
+ * clkz		1000   (2/3)		800   (1/2)
+ * clkzs	 250   (1/6)		200   (1/8)
+ * clki		 750   (1/2)		800   (1/2)
+ * clks		 250   (1/6)		200   (1/8)
+ * clks1	 125   (1/12)		100   (1/16)
+ * clks3	 187.5 (1/8)		200   (1/8)
+ * clks4	  93.7 (1/16)		100   (1/16)
+ * clkp		  62.5 (1/24)		 50   (1/32)
+ * clkg		  62.5 (1/24)		 66.6 (1/24)
+ * clkb, CLKOUT
+ * (MD2 = 0)	  62.5 (1/24)		 66.6 (1/24)
+ * (MD2 = 1)	  41.6 (1/36)		 50   (1/32)
+ */
+
+#define CPG_CLK_CONFIG_INDEX(md)	(((md) & (BIT(1)|BIT(2))) >> 1)
+
+struct cpg_clk_config {
+	unsigned int z_mult;
+	unsigned int z_div;
+	unsigned int zs_and_s_div;
+	unsigned int s1_div;
+	unsigned int p_div;
+	unsigned int out_div;
+};
+
+static const struct cpg_clk_config cpg_clk_configs[4] __initconst = {
+	{ 1, 2, 8, 16, 32, 24 },
+	{ 1, 2, 8, 16, 32, 24 },
+	{ 2, 3, 6, 12, 24, 36 },
+	{ 2, 3, 6, 12, 24, 32 },
+};
+
+/*
+ *   MD		PLLA Ratio
+ * 12 11
+ *------------------------
+ * 0  0		x42
+ * 0  1		x48
+ * 1  0		x56
+ * 1  1		x64
+ */
+#define CPG_PLL_CONFIG_INDEX(md)	((((md) & BIT(14)) >> 12) | \
+					 (((md) & BIT(13)) >> 12) | \
+					 (((md) & BIT(19)) >> 19))
+struct cpg_pll_config {
+	unsigned int extal_div;
+	unsigned int pll1_mult;
+	unsigned int pll3_mult;
+};
+
+#define CPG_PLLA_MULT_INDEX(md)	(((md) & (BIT(12)|BIT(11))) >> 11)
+
+static const unsigned int cpg_plla_mult[4] __initconst = { 42, 48, 56, 64

};

drivers/clk/shmobile/clk-r8a7779.c new file +9 -1

+
+/*
------------------------------------------------------------------------
+ * Initialization
+ */
+
+static u32 cpg_mode __initdata;
+
+static struct clk * __init
+r8a7779_cpg_register_clock(struct device_node *np, struct r8a7779_cpg

*cpg, + const struct cpg_clk_config *config,

drivers/clk/shmobile/clk-r8a7779.c new file +11 -0

+			   unsigned int plla_mult, const char *name)
+{
+	const char *parent_name = "plla";
+	unsigned int mult = 1;
+	unsigned int div = 1;
+
+	if (!strcmp(name, "plla")) {
+		parent_name = of_clk_get_parent_name(np, 0);
+		mult = plla_mult;
+	} else if (!strcmp(name, "z")) {
+		div = config->z_div;

drivers/clk/shmobile/clk-r8a7779.c line 124

Quoted 10 lines

drivers/clk/shmobile/clk-r8a7779.c new file +10 -0

+		mult = config->z_mult;
+	} else if (!strcmp(name, "zs") || !strcmp(name, "s")) {
+		div = config->zs_and_s_div;
+	} else if (!strcmp(name, "s1")) {
+		div = config->s1_div;
+	} else if (!strcmp(name, "p")) {
+		div = config->p_div;
+	} else if (!strcmp(name, "out")) {
+		div = config->out_div;
+	}

You're missing an

	else {
		return ERR_PTR5-EINVAL);
	}

I was tempted to say that it would make sense to read the div values from the FRQMR register instead and remove the need to pass the boot mode bits to the driver, but we need them for the PLLA multiplier anyway :-/

drivers/clk/shmobile/clk-r8a7779.c

Quoted 2 lines

drivers/clk/shmobile/clk-r8a7779.c new file +2 -0

+
+	return clk_register_fixed_factor(NULL, name, parent_name, 0, mult, 

Quoted 26 lines

drivers/clk/shmobile/clk-r8a7779.c new file +26 -0

+}
+
+static void __init r8a7779_cpg_clocks_init(struct device_node *np)
+{
+	const struct cpg_clk_config *config;
+	struct r8a7779_cpg *cpg;
+	struct clk **clks;
+	unsigned int i, plla_mult;
+	int num_clks;
+
+	num_clks = of_property_count_strings(np, "clock-output-names");
+	if (num_clks < 0) {
+		pr_err("%s: failed to count clocks\n", __func__);
+		return;
+	}
+
+	cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
+	clks = kzalloc(CPG_NUM_CLOCKS * sizeof(*clks), GFP_KERNEL);
+	if (cpg == NULL || clks == NULL) {
+		/* We're leaking memory on purpose, there's no point in cleaning
+		 * up as the system won't boot anyway.
+		 */
+		pr_err("%s: failed to allocate cpg\n", __func__);
+		return;
+	}
+

drivers/clk/shmobile/clk-r8a7779.c line 160

Quoted 8 lines

drivers/clk/shmobile/clk-r8a7779.c new file +8 -0

+	spin_lock_init(&cpg->lock);
+
+	cpg->data.clks = clks;
+	cpg->data.clk_num = num_clks;
+
+	cpg->reg = of_iomap(np, 0);
+	if (WARN_ON(cpg->reg == NULL))
+		return;

drivers/clk/shmobile/clk-r8a7779.c line 163

Quoted 3 lines

drivers/clk/shmobile/clk-r8a7779.c new file +3 -0

+
+	config = &cpg_clk_configs[CPG_CLK_CONFIG_INDEX(cpg_mode)];
+	plla_mult = cpg_plla_mult[CPG_PLLA_MULT_INDEX(cpg_mode)];

Given that plla_mult is only used for a single clock, I would move that inside the if (!strcmp(name, "plla")) { } above.

Quoted 33 lines

drivers/clk/shmobile/clk-r8a7779.c new file +28 -0

+
+	for (i = 0; i < num_clks; ++i) {
+		const char *name;
+		struct clk *clk;
+
+		of_property_read_string_index(np, "clock-output-names", i,
+					      &name);
+
+		clk = r8a7779_cpg_register_clock(np, cpg, config,
+						 plla_mult, name);
+		if (IS_ERR(clk))
+			pr_err("%s: failed to register %s %s clock (%ld)\n",
+			       __func__, np->name, name, PTR_ERR(clk));
+		else
+			cpg->data.clks[i] = clk;
+	}
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &cpg->data);
+}
+CLK_OF_DECLARE(r8a7779_cpg_clks, "renesas,r8a7779-cpg-clocks",
+	       r8a7779_cpg_clocks_init);
+
+void __init r8a7779_clocks_init(u32 mode)
+{
+	cpg_mode = mode;
+
+	of_clk_init(NULL);
+}

include/linux/clk/shmobile.h +0 -0

index f9bf080..7667f49 100644

include/linux/clk/shmobile.h line 3

Quoted 3 lines

include/linux/clk/shmobile.h +1 -0

@@ -1,7 +1,9 @@
 /*
  * Copyright 2013 Ideas On Board SPRL
+ * Copyright 2013 Horms Solutions Ltd.

2014 ?

Quoted 14 lines

include/linux/clk/shmobile.h +2 -0

  *
  * Contact: Laurent Pinchart <[email protected]>
+ * Contact: Simon Horman <[email protected]>
  *
  * 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

@@ -14,6 +16,7 @@
 
 #include <linux/types.h>
 
+void r8a7779_clocks_init(u32 mode);
 void rcar_gen2_clocks_init(u32 mode);
 
 #endif

Hi Simon,

Two more small comments.

Quoted 13 lines · On Wednesday 26 February 2014 13:53:17 Laurent Pinchart wrote:

Quoted 9 lines · On Wednesday 26 February 2014 16:33:17 Simon Horman wrote:

The R8A7779 SoC has several clocks that are too custom to be supported in a generic driver. Those clocks can be divided in two categories:

- Fixed rate clocks with multiplier and divisor set according to boot mode configuration

- Custom divider clocks with SoC-specific divider values

This driver supports both.

Looking at the R8A7779 datasheet it looks like we only have fixed rate clocks, without any configurable divider clock. Did I miss something ?

Quoted 22 lines

Quoted 21 lines

Based on work for R-Car Gen2 SoCs by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart

- Added external clock input
- Use PLLA ratio set bu MD11 and MD12
- Add _div suffixes of fields of struct cpt_clk_config
- Register PLLA as a fixed factor clock
- Use sizeof() instead of sizeof
- Use num_clks instead of CPG_NUM_CLOCKS in r8a7779_cpg_clocks_init()

- I kept this as r8a7779 binding rather than moving to a R-Car Gen1

binding which could be shared with other SoCs as I do not believe that the SoCs is are sufficiently similar.

Quoted 7 lines

I had a look at the M1 datasheet and I still find its CPG very similar with the H1 CPG. The PLLA multiplier and divider are different, but if you look closely, they're both exactly twice the value compared to H1, so there's no difference in practice.

What differences do you see that would make it impractical to share a single driver for both ?

Quoted 33 lines

Quoted 33 lines

---

 .../bindings/clock/renesas,r8a7779-cpg-clocks.txt  |  26 +++
 drivers/clk/shmobile/Makefile                      |   1 +
 drivers/clk/shmobile/clk-r8a7779.c                 | 191
 ++++++++++++++++++
 include/linux/clk/shmobile.h                       |   3 +
 4 files changed, 221 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt create mode 100644 drivers/clk/shmobile/clk-r8a7779.c

diff --git a/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file mode 100644
index 0000000..1461323
--- /dev/null
+++
b/Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +10 -0

@@ -0,0 +1,26 @@
+* Renesas R8A7779 Clock Pulse Generator (CPG)
+
+The CPG generates core clocks for the R8A7779. It includes one PLL and
+and several fixed ratio dividers
+
+Required Properties:
+
+  - compatible: Must be "renesas,r8a7779-cpg-clocks"
+  - reg: Base address and length of the memory resource used by the CPG
+

Quoted 8 lines

Quoted 5 lines

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +3 -0

+  - clocks: Reference to the parent clock
+  - #clock-cells: Must be 1
+  - clock-output-names: The names of the clocks. Supported clocks are

"plla",
+ "z", "zs", "s", "s1", "p", "out".

What about clki, clks3, clks4, clkb and clkg ? Should pllb be exposed as well ?

I spoke too fast, clki, clks3, clks4 and clkg are fixed factor clocks expressed in DT, my bad. What about clkb though ?

Quoted 11 lines

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt line 22

Quoted 8 lines

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +6 -0

+Example
+-------
+
+	cpg_clocks: cpg_clocks at ffc80000 {
+		compatible = "renesas,r8a7779-cpg-clocks";
+		reg = <0 0xffc80000 0 0x80>;

Shouldn't the range be restricted not to include the MSTP registers ? 0x30 should be enough.

Quoted 156 lines

Quoted 143 lines

Documentation/devicetree/bindings/clock/renesas,r8a7779-cpg-clocks.txt new file +4 -0

+		clocks = <&extal_clk>;
+		#clock-cells = <1>;
+		clock-output-names = "plla", "z", "zs", "s", "s1", "p", "out";
+	};

drivers/clk/shmobile/Makefile +1 -0

index 9ecef14..2121ba0 100644
@@ -1,4 +1,5 @@
 
 obj-$(CONFIG_ARCH_EMEV2)		+= clk-emev2.o
 
+obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
 

obj-$(CONFIG_ARCH_R8A7790) += clk-rcar-gen2.o
obj-$(CONFIG_ARCH_R8A7791) += clk-rcar-gen2.o
obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += clk-div6.o

(quoted diff) +0 -0

b/drivers/clk/shmobile/clk-r8a7779.c new file mode 100644 index 0000000..2ca2d67

drivers/clk/shmobile/clk-r8a7779.c new file +92 -3

@@ -0,0 +1,191 @@
+/*
+ * r8a7779 Core CPG Clocks
+ *
+ * Copyright (C) 2013, 2014 Horms Solutions Ltd.
+ *
+ * Contact: Simon Horman <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/clk/shmobile.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/spinlock.h>
+
+#include <dt-bindings/clock/r8a7779-clock.h>
+
+#define CPG_NUM_CLOCKS			(R8A7779_CLK_OUT + 1)
+
+struct r8a7779_cpg {
+	struct clk_onecell_data data;
+	spinlock_t lock;
+	void __iomem *reg;
+};
+
+/*
--------------------------------------------------------------------------
-
-- + * CPG Clock Data
+ */
+
+/*
+ *		MD1 = 1			MD1 = 0
+ *		(PLLA = 1500)		(PLLA = 1600)
+ *		(MHz)			(MHz)
+ *------------------------------------------------+--------------------
+ * clkz		1000   (2/3)		800   (1/2)
+ * clkzs	 250   (1/6)		200   (1/8)
+ * clki		 750   (1/2)		800   (1/2)
+ * clks		 250   (1/6)		200   (1/8)
+ * clks1	 125   (1/12)		100   (1/16)
+ * clks3	 187.5 (1/8)		200   (1/8)
+ * clks4	  93.7 (1/16)		100   (1/16)
+ * clkp		  62.5 (1/24)		 50   (1/32)
+ * clkg		  62.5 (1/24)		 66.6 (1/24)
+ * clkb, CLKOUT
+ * (MD2 = 0)	  62.5 (1/24)		 66.6 (1/24)
+ * (MD2 = 1)	  41.6 (1/36)		 50   (1/32)
+ */
+
+#define CPG_CLK_CONFIG_INDEX(md)	(((md) & (BIT(1)|BIT(2))) >> 1)
+
+struct cpg_clk_config {
+	unsigned int z_mult;
+	unsigned int z_div;
+	unsigned int zs_and_s_div;
+	unsigned int s1_div;
+	unsigned int p_div;
+	unsigned int out_div;
+};
+
+static const struct cpg_clk_config cpg_clk_configs[4] __initconst = {
+	{ 1, 2, 8, 16, 32, 24 },
+	{ 1, 2, 8, 16, 32, 24 },
+	{ 2, 3, 6, 12, 24, 36 },
+	{ 2, 3, 6, 12, 24, 32 },
+};
+
+/*
+ *   MD		PLLA Ratio
+ * 12 11
+ *------------------------
+ * 0  0		x42
+ * 0  1		x48
+ * 1  0		x56
+ * 1  1		x64
+ */
+#define CPG_PLL_CONFIG_INDEX(md)	((((md) & BIT(14)) >> 12) | \
+					 (((md) & BIT(13)) >> 12) | \
+					 (((md) & BIT(19)) >> 19))
+struct cpg_pll_config {
+	unsigned int extal_div;
+	unsigned int pll1_mult;
+	unsigned int pll3_mult;
+};
+
+#define CPG_PLLA_MULT_INDEX(md)	(((md) & (BIT(12)|BIT(11))) >> 11)
+
+static const unsigned int cpg_plla_mult[4] __initconst = { 42, 48, 56, 64

};

drivers/clk/shmobile/clk-r8a7779.c new file +9 -1

+
+/*
------------------------------------------------------------------------
+ * Initialization
+ */
+
+static u32 cpg_mode __initdata;
+
+static struct clk * __init
+r8a7779_cpg_register_clock(struct device_node *np, struct r8a7779_cpg

*cpg, + const struct cpg_clk_config *config,

drivers/clk/shmobile/clk-r8a7779.c new file +11 -0

+			   unsigned int plla_mult, const char *name)
+{
+	const char *parent_name = "plla";
+	unsigned int mult = 1;
+	unsigned int div = 1;
+
+	if (!strcmp(name, "plla")) {
+		parent_name = of_clk_get_parent_name(np, 0);
+		mult = plla_mult;
+	} else if (!strcmp(name, "z")) {
+		div = config->z_div;

drivers/clk/shmobile/clk-r8a7779.c line 124

Quoted 10 lines

drivers/clk/shmobile/clk-r8a7779.c new file +10 -0

+		mult = config->z_mult;
+	} else if (!strcmp(name, "zs") || !strcmp(name, "s")) {
+		div = config->zs_and_s_div;
+	} else if (!strcmp(name, "s1")) {
+		div = config->s1_div;
+	} else if (!strcmp(name, "p")) {
+		div = config->p_div;
+	} else if (!strcmp(name, "out")) {
+		div = config->out_div;
+	}

You're missing an

Quoted 3 lines

	else {
		return ERR_PTR5-EINVAL);
	}

Quoted 6 lines

I was tempted to say that it would make sense to read the div values from the FRQMR register instead and remove the need to pass the boot mode bits to the driver, but we need them for the PLLA multiplier anyway :-/

drivers/clk/shmobile/clk-r8a7779.c

Quoted 2 lines

drivers/clk/shmobile/clk-r8a7779.c new file +2 -0

+
+	return clk_register_fixed_factor(NULL, name, parent_name, 0, mult, 

div);

Quoted 29 lines

Quoted 29 lines

drivers/clk/shmobile/clk-r8a7779.c new file +29 -0

+}
+
+static void __init r8a7779_cpg_clocks_init(struct device_node *np)
+{
+	const struct cpg_clk_config *config;
+	struct r8a7779_cpg *cpg;
+	struct clk **clks;
+	unsigned int i, plla_mult;
+	int num_clks;
+
+	num_clks = of_property_count_strings(np, "clock-output-names");
+	if (num_clks < 0) {
+		pr_err("%s: failed to count clocks\n", __func__);
+		return;
+	}
+
+	cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
+	clks = kzalloc(CPG_NUM_CLOCKS * sizeof(*clks), GFP_KERNEL);
+	if (cpg == NULL || clks == NULL) {
+		/* We're leaking memory on purpose, there's no point in cleaning
+		 * up as the system won't boot anyway.
+		 */
+		pr_err("%s: failed to allocate cpg\n", __func__);
+		return;
+	}
+
+	spin_lock_init(&cpg->lock);
+
+	cpg->data.clks = clks;

Quoted 5 lines

drivers/clk/shmobile/clk-r8a7779.c line 160

Quoted 5 lines

drivers/clk/shmobile/clk-r8a7779.c new file +5 -0

+	cpg->data.clk_num = num_clks;
+
+	cpg->reg = of_iomap(np, 0);
+	if (WARN_ON(cpg->reg == NULL))
+		return;

As the driver doesn't access any of the CPG registers this could even be removed.

Quoted 37 lines

drivers/clk/shmobile/clk-r8a7779.c line 163

Quoted 3 lines

drivers/clk/shmobile/clk-r8a7779.c new file +3 -0

+
+	config = &cpg_clk_configs[CPG_CLK_CONFIG_INDEX(cpg_mode)];
+	plla_mult = cpg_plla_mult[CPG_PLLA_MULT_INDEX(cpg_mode)];

Given that plla_mult is only used for a single clock, I would move that inside the if (!strcmp(name, "plla")) { } above.

Quoted 30 lines

drivers/clk/shmobile/clk-r8a7779.c new file +28 -0

+
+	for (i = 0; i < num_clks; ++i) {
+		const char *name;
+		struct clk *clk;
+
+		of_property_read_string_index(np, "clock-output-names", i,
+					      &name);
+
+		clk = r8a7779_cpg_register_clock(np, cpg, config,
+						 plla_mult, name);
+		if (IS_ERR(clk))
+			pr_err("%s: failed to register %s %s clock (%ld)\n",
+			       __func__, np->name, name, PTR_ERR(clk));
+		else
+			cpg->data.clks[i] = clk;
+	}
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &cpg->data);
+}
+CLK_OF_DECLARE(r8a7779_cpg_clks, "renesas,r8a7779-cpg-clocks",
+	       r8a7779_cpg_clocks_init);
+
+void __init r8a7779_clocks_init(u32 mode)
+{
+	cpg_mode = mode;
+
+	of_clk_init(NULL);
+}

include/linux/clk/shmobile.h +0 -0

index f9bf080..7667f49 100644

Quoted 8 lines

include/linux/clk/shmobile.h line 3

Quoted 6 lines

include/linux/clk/shmobile.h +1 -0

@@ -1,7 +1,9 @@
 /*
  * Copyright 2013 Ideas On Board SPRL
+ * Copyright 2013 Horms Solutions Ltd.

2014 ?

Quoted 15 lines

Quoted 15 lines

include/linux/clk/shmobile.h +2 -0

  *
  * Contact: Laurent Pinchart <[email protected]>
+ * Contact: Simon Horman <[email protected]>
  *
  * 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

@@ -14,6 +16,7 @@
 
 #include <linux/types.h>
 
+void r8a7779_clocks_init(u32 mode);
 void rcar_gen2_clocks_init(u32 mode);
 
 #endif

Signature · 3 lines

Regards,

Laurent Pinchart

On Wed, Feb 26, 2014 at 02:02:37PM +0100, Laurent Pinchart wrote:

Quoted 7 lines · On Wednesday 26 February 2014 13:53:17 Laurent Pinchart wrote:

Quoted 7 lines · On Wednesday 26 February 2014 16:33:17 Simon Horman wrote:

Quoted 6 lines

The R8A7779 SoC has several clocks that are too custom to be supported in a generic driver. Those clocks can be divided in two categories:

- Fixed rate clocks with multiplier and divisor set according to boot mode configuration

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

- Custom divider clocks with SoC-specific divider values

This driver supports both.

Looking at the R8A7779 datasheet it looks like we only have fixed rate clocks, without any configurable divider clock. Did I miss something ?

Sorry, its a cut-and past error on my part.
I will revise the changelog.

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

Based on work for R-Car Gen2 SoCs by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart

- Added external clock input
- Use PLLA ratio set bu MD11 and MD12
- Add _div suffixes of fields of struct cpt_clk_config
- Register PLLA as a fixed factor clock
- Use sizeof() instead of sizeof
- Use num_clks instead of CPG_NUM_CLOCKS in r8a7779_cpg_clocks_init()

- I kept this as r8a7779 binding rather than moving to a R-Car Gen1 binding which could be shared with other SoCs as I do not believe that the SoCs is are sufficiently similar.

Quoted 7 lines

Quoted 7 lines

I had a look at the M1 datasheet and I still find its CPG very similar with the H1 CPG. The PLLA multiplier and divider are different, but if you look closely, they're both exactly twice the value compared to H1, so there's no difference in practice.

What differences do you see that would make it impractical to share a single driver for both ?

Thanks for pointing that out. Looking over the M1 datasheet again I agree with you that there does seem to be rather a lot of similarities with the H1. And I agree that it is likely that the driver could be shared.

However, I'd like to leave that as future work at this time.

The reason for this is that on the one hand I believe such work sould be done in conjunctin with integrating the driver on the bockw board. And that seems to be non-trivial to me.

Hi Simon,

Quoted 54 lines · On Wednesday 12 March 2014 16:55:23 Simon Horman wrote:

On Wed, Feb 26, 2014 at 02:02:37PM +0100, Laurent Pinchart wrote:

Quoted 7 lines · On Wednesday 26 February 2014 13:53:17 Laurent Pinchart wrote:

Quoted 7 lines · On Wednesday 26 February 2014 16:33:17 Simon Horman wrote:

Quoted 6 lines

The R8A7779 SoC has several clocks that are too custom to be supported in a generic driver. Those clocks can be divided in two categories:

- Fixed rate clocks with multiplier and divisor set according to boot mode configuration

Quoted 6 lines

Quoted 6 lines

Quoted 3 lines

- Custom divider clocks with SoC-specific divider values

This driver supports both.

Looking at the R8A7779 datasheet it looks like we only have fixed rate clocks, without any configurable divider clock. Did I miss something ?

Sorry, its a cut-and past error on my part.
I will revise the changelog.

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

Based on work for R-Car Gen2 SoCs by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart

- Added external clock input
- Use PLLA ratio set bu MD11 and MD12
- Add _div suffixes of fields of struct cpt_clk_config
- Register PLLA as a fixed factor clock
- Use sizeof() instead of sizeof
- Use num_clks instead of CPG_NUM_CLOCKS in r8a7779_cpg_clocks_init()

- I kept this as r8a7779 binding rather than moving to a R-Car Gen1 binding which could be shared with other SoCs as I do not believe that the SoCs is are sufficiently similar.

Quoted 7 lines

Quoted 7 lines

I had a look at the M1 datasheet and I still find its CPG very similar with the H1 CPG. The PLLA multiplier and divider are different, but if you look closely, they're both exactly twice the value compared to H1, so there's no difference in practice.

What differences do you see that would make it impractical to share a single driver for both ?

Thanks for pointing that out. Looking over the M1 datasheet again I agree with you that there does seem to be rather a lot of similarities with the H1. And I agree that it is likely that the driver could be shared.

However, I'd like to leave that as future work at this time.

Quoted 3 lines

The reason for this is that on the one hand I believe such work sould be done in conjunctin with integrating the driver on the bockw board. And that seems to be non-trivial to me.

We can always rename the driver later, that's not a big issue, but we can't change compatibility strings. I would thus like to see "renesas,rcar-gen1-cpg- clocks" listed in the DT bindings.

Quoted 62 lines · Laurent Pinchart

Quoted 60 lines · On Wednesday 12 March 2014 16:55:23 Simon Horman wrote:

On Wed, Feb 26, 2014 at 02:02:37PM +0100, Laurent Pinchart wrote:

Quoted 8 lines · On Wednesday 26 February 2014 13:53:17 Laurent Pinchart wrote:

Quoted 8 lines · On Wednesday 26 February 2014 16:33:17 Simon Horman wrote:

Quoted 7 lines

The R8A7779 SoC has several clocks that are too custom to be supported in a generic driver. Those clocks can be divided in two categories:

- Fixed rate clocks with multiplier and divisor set according to boot mode configuration

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

- Custom divider clocks with SoC-specific divider values

This driver supports both.

Looking at the R8A7779 datasheet it looks like we only have fixed rate clocks, without any configurable divider clock. Did I miss something ?

Sorry, its a cut-and past error on my part.
I will revise the changelog.

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

Based on work for R-Car Gen2 SoCs by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart

- Added external clock input
- Use PLLA ratio set bu MD11 and MD12
- Add _div suffixes of fields of struct cpt_clk_config
- Register PLLA as a fixed factor clock
- Use sizeof() instead of sizeof
- Use num_clks instead of CPG_NUM_CLOCKS in r8a7779_cpg_clocks_init()

- I kept this as r8a7779 binding rather than moving to a R-Car Gen1 binding which could be shared with other SoCs as I do not believe that the SoCs is are sufficiently similar.

Quoted 7 lines

Quoted 7 lines

I had a look at the M1 datasheet and I still find its CPG very similar with the H1 CPG. The PLLA multiplier and divider are different, but if you look closely, they're both exactly twice the value compared to H1, so there's no difference in practice.

What differences do you see that would make it impractical to share a single driver for both ?

Thanks for pointing that out. Looking over the M1 datasheet again I agree with you that there does seem to be rather a lot of similarities with the H1. And I agree that it is likely that the driver could be shared.

However, I'd like to leave that as future work at this time.

The reason for this is that on the one hand I believe such work sould be done in conjunctin with integrating the driver on the bockw board. And that seems to be non-trivial to me.

Quoted 3 lines

We can always rename the driver later, that's not a big issue, but we can't change compatibility strings. I would thus like to see "renesas,rcar-gen1-cpg-clocks" listed in the DT bindings.

I know that we have done such things in the past but these days I feel that it is a bit contentious as it is a binding for a software abstraction rather than the hardware. And as far as I know bindings are supposed to describe the hardware.

Hi Simon,

Quoted 63 lines · On Thursday 13 March 2014 11:22:47 Simon Horman wrote:

Quoted 62 lines · Laurent Pinchart

Quoted 60 lines · On Wednesday 12 March 2014 16:55:23 Simon Horman wrote:

On Wed, Feb 26, 2014 at 02:02:37PM +0100, Laurent Pinchart wrote:

Quoted 8 lines · On Wednesday 26 February 2014 13:53:17 Laurent Pinchart wrote:

Quoted 8 lines · On Wednesday 26 February 2014 16:33:17 Simon Horman wrote:

Quoted 7 lines

The R8A7779 SoC has several clocks that are too custom to be supported in a generic driver. Those clocks can be divided in two categories:

- Fixed rate clocks with multiplier and divisor set according to boot mode configuration

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

- Custom divider clocks with SoC-specific divider values

This driver supports both.

Looking at the R8A7779 datasheet it looks like we only have fixed rate clocks, without any configurable divider clock. Did I miss something ?

Sorry, its a cut-and past error on my part.
I will revise the changelog.

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

Based on work for R-Car Gen2 SoCs by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart

- Added external clock input
- Use PLLA ratio set bu MD11 and MD12
- Add _div suffixes of fields of struct cpt_clk_config
- Register PLLA as a fixed factor clock
- Use sizeof() instead of sizeof
- Use num_clks instead of CPG_NUM_CLOCKS in r8a7779_cpg_clocks_init()

- I kept this as r8a7779 binding rather than moving to a R-Car Gen1 binding which could be shared with other SoCs as I do not believe that the SoCs is are sufficiently similar.

Quoted 7 lines

Quoted 7 lines

I had a look at the M1 datasheet and I still find its CPG very similar with the H1 CPG. The PLLA multiplier and divider are different, but if you look closely, they're both exactly twice the value compared to H1, so there's no difference in practice.

What differences do you see that would make it impractical to share a single driver for both ?

Thanks for pointing that out. Looking over the M1 datasheet again I agree with you that there does seem to be rather a lot of similarities with the H1. And I agree that it is likely that the driver could be shared.

However, I'd like to leave that as future work at this time.

The reason for this is that on the one hand I believe such work sould be done in conjunctin with integrating the driver on the bockw board. And that seems to be non-trivial to me.

Quoted 8 lines

Quoted 3 lines

We can always rename the driver later, that's not a big issue, but we can't change compatibility strings. I would thus like to see "renesas,rcar-gen1-cpg-clocks" listed in the DT bindings.

I know that we have done such things in the past but these days I feel that it is a bit contentious as it is a binding for a software abstraction rather than the hardware. And as far as I know bindings are supposed to describe the hardware.

Sure, and I consider rcar-gen1-cpf is a hardware description. We have two generations of SoCs in the R-Car series so far, the first generation (H1 and M1) and the second generation (H2 and M2). The second generation is even explictly named as such in the M2 datasheet. SoCs within a generation share hardware characteristics, and it makes sense to reflect that in compatible strings.

include/linux/clk/shmobile.h +2 -0

index f9bf080..7667f49 100644
@@ -1,7 +1,9 @@
 /*
  * Copyright 2013 Ideas On Board SPRL
+ * Copyright 2013 Horms Solutions Ltd.
  *
  * Contact: Laurent Pinchart <[email protected]>
+ * Contact: Simon Horman <[email protected]>
  *
  * 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

Quoted 14 lines

include/linux/clk/shmobile.h +2 -0

@@ -1,7 +1,9 @@
 /*
  * Copyright 2013 Ideas On Board SPRL
+ * Copyright 2013 Horms Solutions Ltd.
  *
  * Contact: Laurent Pinchart <[email protected]>
+ * Contact: Simon Horman <[email protected]>
  *
  * 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

@@ -14,6 +16,7 @@
 

include/linux/clk/shmobile.h line 20

Quoted 4 lines

include/linux/clk/shmobile.h +1 -0

 #include <linux/types.h>
 
+void r8a7779_clocks_init(u32 mode);
 void rcar_gen2_clocks_init(u32 mode);

Shouldn't the init functions be protected by #ifdef CONFIG_ARCH...?

Quoted 5 lines

drivers/clk/shmobile/clk-r8a7779.c new file +5 -0

+	cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
+	clks = kzalloc(CPG_NUM_CLOCKS * sizeof(*clks), GFP_KERNEL);
+	if (cpg == NULL || clks == NULL) {
+		/* We're leaking memory on purpose, there's no point in cleaning
+		 * up as the system won't boot anyway.

drivers/clk/shmobile/clk-r8a7779.c line 151

Quoted 4 lines

drivers/clk/shmobile/clk-r8a7779.c new file +4 -0

+		 */
+		pr_err("%s: failed to allocate cpg\n", __func__);
+		return;
+	}

I have problems with this sloppiness. Writing the comment took probably the same time as writing the proper exit path :) On the drawback side, static code checkers will keep reporting this.

drivers/clk/shmobile/clk-r8a7779.c line 160

Quoted 3 lines

drivers/clk/shmobile/clk-r8a7779.c new file +3 -0

+	cpg->reg = of_iomap(np, 0);
+	if (WARN_ON(cpg->reg == NULL))
+		return;

if (WARN(!cpg->reg, "can't ioremap CPG\n"))

?

Quoted 10 lines · On Saturday 01 March 2014 14:50:03 Wolfram Sang wrote:

Quoted 9 lines

drivers/clk/shmobile/clk-r8a7779.c new file +9 -0

+   cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
+   clks = kzalloc(CPG_NUM_CLOCKS * sizeof(*clks), GFP_KERNEL);
+   if (cpg == NULL || clks == NULL) {
+           /* We're leaking memory on purpose, there's no point in cleaning
+            * up as the system won't boot anyway.
+            */
+           pr_err("%s: failed to allocate cpg\n", __func__);
+           return;
+   }

Quoted 3 lines

I have problems with this sloppiness. Writing the comment took probably the same time as writing the proper exit path :) On the drawback side, static code checkers will keep reporting this.

Then static code checkers should be fixed :-) or at least provide an override. Given that the system won't boot if this fails I don't see a point in adding useless code to the kernel that would just result in a larger binary size.

Hi Wolfram,

Quoted 5 lines · On Saturday 01 March 2014 14:50:03 Wolfram Sang wrote:

Quoted 5 lines

drivers/clk/shmobile/clk-r8a7779.c new file +5 -0

+	cpg = kzalloc(sizeof(*cpg), GFP_KERNEL);
+	clks = kzalloc(CPG_NUM_CLOCKS * sizeof(*clks), GFP_KERNEL);
+	if (cpg == NULL || clks == NULL) {
+		/* We're leaking memory on purpose, there's no point in cleaning
+		 * up as the system won't boot anyway.

Quoted 8 lines

drivers/clk/shmobile/clk-r8a7779.c line 151

Quoted 4 lines

drivers/clk/shmobile/clk-r8a7779.c new file +4 -0

+		 */
+		pr_err("%s: failed to allocate cpg\n", __func__);
+		return;
+	}

I have problems with this sloppiness. Writing the comment took probably the same time as writing the proper exit path :) On the drawback side, static code checkers will keep reporting this.

Then static code checkers should be fixed :-)

Quoted 7 lines

drivers/clk/shmobile/clk-r8a7779.c line 160

Quoted 3 lines

drivers/clk/shmobile/clk-r8a7779.c new file +3 -0

+	cpg->reg = of_iomap(np, 0);
+	if (WARN_ON(cpg->reg == NULL))
+		return;

if (WARN(!cpg->reg, "can't ioremap CPG\n"))

?

We can probably remove this altogether given that the driver doesn't make use of the ioremap'ed memory.

Add MSTP clock support for the r8a7779 SoC

The only change required appears to be to note a new compatibility string in the bindings.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart
- Add entry in numerical order

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

diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt index a6a352c..f3226ac 100644

Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt +1 -1

@@ -10,6 +10,7 @@ index in the group, from 0 to 31.
 Required Properties:
 
   - compatible: Must be one of the following
+    - "renesas,r8a7779-mstp-clocks" for R8A7779 (R-Car H1) MSTP gate clocks
- "renesas,r8a7790-mstp-clocks" for R8A7790 (R-Car H2) MSTP gate clocks -

"renesas,r8a7791-mstp-clocks" for R8A7791 (R-Car M2) MSTP gate clocks - "renesas,cpg-mstp-clock" for generic MSTP gate clocks

Add macros usable by device tree sources to reference r8a7779 clocks by index.

Based on work for the r8a7791 SoC by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart
- Consolidate R8A7779_CLK_HSPI{0,1,2} into R8A7779_CLK_HSPI
- Correct inverted indexes of R8A7779_CLK_TMU{1,2}
- Consolidate R8A7779_CLK_{E,O}HCI{0,1} into R8A7779_CLK_USB{0,1}
- Add ethernet, MMC and HSCIF entries
- Replace R8A7779_SCIF6 with R8A7779_CLK_SCIF5

* Add R8A7779_CLK_PCIE

v2
* No change

---
 include/dt-bindings/clock/r8a7779-clock.h | 63
+++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+)
 create mode 100644 include/dt-bindings/clock/r8a7779-clock.h

(quoted diff) +0 -0

b/include/dt-bindings/clock/r8a7779-clock.h new file mode 100644 index 0000000..8ab8007

include/dt-bindings/clock/r8a7779-clock.h new file +63 -0

@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2013  Horms Solutions Ltd.
+ *
+ * Contact: Simon Horman <[email protected]>
+ *
+ * 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_R8A7779_H__
+#define __DT_BINDINGS_CLOCK_R8A7779_H__
+
+/* CPG */
+#define R8A7779_CLK_PLLA	0
+#define R8A7779_CLK_Z		1
+#define R8A7779_CLK_ZS		2
+#define R8A7779_CLK_S		3
+#define R8A7779_CLK_S1		4
+#define R8A7779_CLK_P		5
+#define R8A7779_CLK_OUT		6
+
+/* MSTP 0 */
+#define R8A7779_CLK_HSPI	7
+#define R8A7779_CLK_TMU2	14
+#define R8A7779_CLK_TMU1	15
+#define R8A7779_CLK_TMU0	16
+#define R8A7779_CLK_HSCIF1	18
+#define R8A7779_CLK_HSCIF0	19
+#define R8A7779_CLK_SCIF5	21
+#define R8A7779_CLK_SCIF4	22
+#define R8A7779_CLK_SCIF3	23
+#define R8A7779_CLK_SCIF2	24
+#define R8A7779_CLK_SCIF1	25
+#define R8A7779_CLK_SCIF0	26
+#define R8A7779_CLK_I2C3	27
+#define R8A7779_CLK_I2C2	28
+#define R8A7779_CLK_I2C1	29
+#define R8A7779_CLK_I2C0	30
+
+/* MSTP 1 */
+#define R8A7779_CLK_USB01	0
+#define R8A7779_CLK_USB2	1
+#define R8A7779_CLK_DU		3
+#define R8A7779_CLK_VIN2	8
+#define R8A7779_CLK_VIN1	9
+#define R8A7779_CLK_VIN0	10
+#define R8A7779_CLK_ETHER	14
+#define R8A7779_CLK_SATA	15
+#define R8A7779_CLK_PCIE	16
+#define R8A7779_CLK_VIN3	20
+
+/* MSTP 3 */
+#define R8A7779_CLK_SDHI3	20
+#define R8A7779_CLK_SDHI2	21
+#define R8A7779_CLK_SDHI1	22
+#define R8A7779_CLK_SDHI0	23
+#define R8A7779_CLK_MMC1	30
+#define R8A7779_CLK_MMC0	31
+
+
+#endif /* __DT_BINDINGS_CLOCK_R8A7779_H__ */

+ };

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

Quoted 10 lines · Simon Horman

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

+               /* Gate clocks */
+               mstp0_clks: mstp0_clks {
+                       compatible = "renesas,r8a7779-mstp-clocks", "renesas,cpg-mstp-clocks";
+                       reg = <0 0xffc80030 0 4>;
+                       clocks = <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>;

Is there any rationale behind the decision to (not) split long lines? The above are really long.

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

Quoted 7 lines

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

+                       #clock-cells = <1>;
+                       renesas,clock-indices = <
+                               R8A7779_CLK_HSPI R8A7779_CLK_TMU0 R8A7779_CLK_TMU0
+                               R8A7779_CLK_TMU0 R8A7779_CLK_SCIF5 R8A7779_CLK_SCIF4
+                               R8A7779_CLK_SCIF3 R8A7779_CLK_SCIF2 R8A7779_CLK_SCIF1
+                               R8A7779_CLK_SCIF0 R8A7779_CLK_I2C3 R8A7779_CLK_I2C2
+                               R8A7779_CLK_I2C1 R8A7779_CLK_I2C0

A bit longer than 80 characters (fine for me, though).

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

Quoted 7 lines

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

+                       >;
+                       clock-output-names =
+                               "hspi", "tmu00", "tmu01",
+                               "tmu02", "scif5", "scif4",
+                               "scif3", "scif2", "scif1",
+                               "scif0", "i2c3", "i2c2",
+                               "i2c1", "i2c0";

Much shorter than 80 characters (to match the above?).

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

Quoted 13 lines

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

+               };
+               mstp1_clks: mstp1_clks {
+                       compatible = "renesas,r8a7779-mstp-clocks", "renesas,cpg-mstp-clocks";
+                       reg = <0 0xffc80034 0 4>, <0 0xffc80044 0 4>;
+                       clocks = <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+                                <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>;
+                       #clock-cells = <1>;
+                       renesas,clock-indices = <
+                               R8A7779_CLK_USB01 R8A7779_CLK_USB01

Fits in 80 characters.

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

Quoted 13 lines

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

+                               R8A7779_CLK_USB2 R8A7779_CLK_USB2
+                               R8A7779_CLK_DU R8A7779_CLK_VIN2
+                               R8A7779_CLK_VIN1 R8A7779_CLK_VIN0
+                               R8A7779_CLK_ETHER R8A7779_CLK_SATA
+                               R8A7779_CLK_PCIE R8A7779_CLK_VIN3
+                       >;
+                       clock-output-names =
+                               "ehci0", "ohci0",
+                               "ehci1", "ohci1",
+                               "du", "vin2",
+                               "vin1", "vin0",
+                               "ether", "sata",
+                               "pcie", "vin3";

Reallt short, could be compacted a lot.

Quoted 2 lines

+ };

Gr{oetje,eeting}s,

Geert

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

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

Declare all core and MSTP clocks currently used by r8a7779-based boards.

Based on work by Laurent Pinchart for the r8a7790 and r8a7791 SoCs.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart
- Add and use extal_clk
- Fix bogus status register use for MSTP clocks
- Fix bogus mstp3_cls to use its own entries rather than that of mstp1_clks

* Update to use "main" in cpg_clocks as per updated binding in previous patch
* Update for new, consolidated and renamed index macros
- R8A7779_CLK_ETHER
- R8A7779_CLK_HSCIF
- R8A7779_CLK_HSPI
- R8A7779_CLK_MMC0,1
- R8A7779_CLK_PCIE
- R8A7779_CLK_USB01,2

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

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

index d0561d4..240a03e 100644
@@ -11,6 +11,7 @@
 
 /include/ "skeleton.dtsi"
 
+#include <dt-bindings/clock/r8a7779-clock.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 
 / {

@@ -278,4 +279,132 @@
 		interrupts = <0 75 IRQ_TYPE_LEVEL_HIGH>;
 		status = "disabled";
 	};
+
+	clocks {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/* External root clock */
+		extal_clk: extal_clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			/* This value must be overriden by the board. */
+			clock-frequency = <0>;
+			clock-output-names = "extal";
+		};
+
+		/* Special CPG clocks */
+		cpg_clocks: cpg_clocks at 0xe6150000 {
+			compatible = "renesas,r8a7779-cpg-clocks";
+			reg = <0 0xe6150000 0 0x1000>;
+			clocks = <&extal_clk>;
+			#clock-cells = <1>;
+			clock-output-names = "plla", "z", "zs", "s",
+					     "s1", "p", "out";
+		};
+
+		/* Fixed factor clocks */
+		i_clk: i_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <2>;
+			clock-mult = <1>;
+			clock-output-names = "i";
+		};
+		s3_clk: s3_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <8>;
+			clock-mult = <1>;
+			clock-output-names = "s3";
+		};
+		s4_clk: s4_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <16>;
+			clock-mult = <1>;
+			clock-output-names = "s4";
+		};
+		g_clk: g_clk {
+			compatible = "fixed-factor-clock";

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

+			#clock-cells = <1>;
+			renesas,clock-indices = <
+				R8A7779_CLK_USB01 R8A7779_CLK_USB01
+				R8A7779_CLK_USB2 R8A7779_CLK_USB2
+				R8A7779_CLK_DU R8A7779_CLK_VIN2
+				R8A7779_CLK_VIN1 R8A7779_CLK_VIN0
+				R8A7779_CLK_ETHER R8A7779_CLK_SATA
+				R8A7779_CLK_PCIE R8A7779_CLK_VIN3
+			>;

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

+			#clock-cells = <1>;
+			renesas,clock-indices = <
+				R8A7779_CLK_SDHI3 R8A7779_CLK_SDHI2
+				R8A7779_CLK_SDHI1 R8A7779_CLK_SDHI0
+				R8A7779_CLK_MMC1 R8A7779_CLK_MMC0
+			>;
+			clock-output-names =
+				"sdhi3", "sdhi2",
+				"sdhi1", "sdhi0",
+				"mmc1", "mmc0";
+		};
+	};
 };

Hi Simon,

Thank you for the patch.

Quoted 96 lines

Declare all core and MSTP clocks currently used by r8a7779-based boards.

Based on work by Laurent Pinchart for the r8a7790 and r8a7791 SoCs.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart
- Add and use extal_clk
- Fix bogus status register use for MSTP clocks
- Fix bogus mstp3_cls to use its own entries rather than that of mstp1_clks

* Update to use "main" in cpg_clocks as per updated binding in previous patch
* Update for new, consolidated and renamed index macros
- R8A7779_CLK_ETHER
- R8A7779_CLK_HSCIF
- R8A7779_CLK_HSPI
- R8A7779_CLK_MMC0,1
- R8A7779_CLK_PCIE
- R8A7779_CLK_USB01,2

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

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

index d0561d4..240a03e 100644
@@ -11,6 +11,7 @@
 
 /include/ "skeleton.dtsi"
 
+#include <dt-bindings/clock/r8a7779-clock.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 
 / {

@@ -278,4 +279,132 @@
 		interrupts = <0 75 IRQ_TYPE_LEVEL_HIGH>;
 		status = "disabled";
 	};
+
+	clocks {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/* External root clock */
+		extal_clk: extal_clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			/* This value must be overriden by the board. */
+			clock-frequency = <0>;
+			clock-output-names = "extal";
+		};
+
+		/* Special CPG clocks */
+		cpg_clocks: cpg_clocks at 0xe6150000 {
+			compatible = "renesas,r8a7779-cpg-clocks";
+			reg = <0 0xe6150000 0 0x1000>;
+			clocks = <&extal_clk>;
+			#clock-cells = <1>;
+			clock-output-names = "plla", "z", "zs", "s",
+					     "s1", "p", "out";
+		};
+
+		/* Fixed factor clocks */
+		i_clk: i_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <2>;
+			clock-mult = <1>;
+			clock-output-names = "i";
+		};
+		s3_clk: s3_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <8>;
+			clock-mult = <1>;
+			clock-output-names = "s3";
+		};
+		s4_clk: s4_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <16>;
+			clock-mult = <1>;
+			clock-output-names = "s4";
+		};
+		g_clk: g_clk {
+			compatible = "fixed-factor-clock";

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

Quoted 10 lines

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

+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <24>;
+			clock-mult = <1>;
+			clock-output-names = "g";
+		};
+
+		/* Gate clocks */
+		mstp0_clks: mstp0_clks {
+			compatible = "renesas,r8a7779-mstp-clocks", "renesas,cpg-mstp-

clocks";

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

Quoted 12 lines

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

+			reg = <0 0xffc80030 0 4>;
+			clocks = <&cpg_clocks R8A7779_CLK_P>,
 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,

<&cpg_clocks R8A7779_CLK_P>,

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

+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,

<&cpg_clocks R8A7779_CLK_P>,

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

+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,

<&cpg_clocks R8A7779_CLK_P>,

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

+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,

<&cpg_clocks R8A7779_CLK_P>,

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

+				 <&cpg_clocks R8A7779_CLK_P>;

Aren't the HSPI and HSCIF parent clocks clks ? Aren't the SCIF parent clocks clks1 ?

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

Quoted 11 lines

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

+			#clock-cells = <1>;
+			renesas,clock-indices = <
+				R8A7779_CLK_HSPI R8A7779_CLK_TMU0 R8A7779_CLK_TMU0
+				R8A7779_CLK_TMU0 R8A7779_CLK_SCIF5 R8A7779_CLK_SCIF4
+				R8A7779_CLK_SCIF3 R8A7779_CLK_SCIF2 R8A7779_CLK_SCIF1
+				R8A7779_CLK_SCIF0 R8A7779_CLK_I2C3 R8A7779_CLK_I2C2
+				R8A7779_CLK_I2C1 R8A7779_CLK_I2C0
+			>;
+			clock-output-names =
+				"hspi", "tmu00", "tmu01",
+				"tmu02", "scif5", "scif4",

The clock names should be tmu0, tmu1 and tmu2, not tmu00, tmu01 and tmu02.

arch/arm/boot/dts/r8a7779.dtsi

Quoted 6 lines

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

+				"scif3", "scif2", "scif1",
+				"scif0", "i2c3", "i2c2",
+				"i2c1", "i2c0";
+		};
+		mstp1_clks: mstp1_clks {
+			compatible = "renesas,r8a7779-mstp-clocks", "renesas,cpg-mstp-

clocks";

arch/arm/boot/dts/r8a7779.dtsi

Quoted 2 lines

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

+			reg = <0 0xffc80034 0 4>, <0 0xffc80044 0 4>;
+			clocks = <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks 

R8A7779_CLK_P>,

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

Quoted 5 lines

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

+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>;

Please verify the clock parents here as well.

Quoted 9 lines

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

+			#clock-cells = <1>;
+			renesas,clock-indices = <
+				R8A7779_CLK_USB01 R8A7779_CLK_USB01
+				R8A7779_CLK_USB2 R8A7779_CLK_USB2
+				R8A7779_CLK_DU R8A7779_CLK_VIN2
+				R8A7779_CLK_VIN1 R8A7779_CLK_VIN0
+				R8A7779_CLK_ETHER R8A7779_CLK_SATA
+				R8A7779_CLK_PCIE R8A7779_CLK_VIN3
+			>;

arch/arm/boot/dts/r8a7779.dtsi

Quoted 10 lines

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

+			clock-output-names =
+				"ehci0", "ohci0",
+				"ehci1", "ohci1",
+				"du", "vin2",
+				"vin1", "vin0",
+				"ether", "sata",
+				"pcie", "vin3";
+		};
+		mstp3_clks: mstp3_clks {
+			compatible = "renesas,r8a7779-mstp-clocks", "renesas,cpg-mstp-

clocks";

arch/arm/boot/dts/r8a7779.dtsi

Quoted 2 lines

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

+			reg = <0 0xffc8003c 0 4>;
+			clocks = <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks 

R8A7779_CLK_P>,

arch/arm/boot/dts/r8a7779.dtsi

Quoted 2 lines

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

+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>;

Aren't the MMC and SDHI parent clocks clks4 ?

Quoted 13 lines

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

+			#clock-cells = <1>;
+			renesas,clock-indices = <
+				R8A7779_CLK_SDHI3 R8A7779_CLK_SDHI2
+				R8A7779_CLK_SDHI1 R8A7779_CLK_SDHI0
+				R8A7779_CLK_MMC1 R8A7779_CLK_MMC0
+			>;
+			clock-output-names =
+				"sdhi3", "sdhi2",
+				"sdhi1", "sdhi0",
+				"mmc1", "mmc0";
+		};
+	};
 };

Signature · 8 lines

Regards,

Laurent Pinchart

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

Declare all core and MSTP clocks currently used by r8a7779-based boards.

Based on work by Laurent Pinchart for the r8a7790 and r8a7791 SoCs.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart
- Add and use extal_clk
- Fix bogus status register use for MSTP clocks
- Fix bogus mstp3_cls to use its own entries rather than that of mstp1_clks

* Update to use "main" in cpg_clocks as per updated binding in previous patch
* Update for new, consolidated and renamed index macros
- R8A7779_CLK_ETHER
- R8A7779_CLK_HSCIF
- R8A7779_CLK_HSPI
- R8A7779_CLK_MMC0,1
- R8A7779_CLK_PCIE
- R8A7779_CLK_USB01,2

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

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

index d0561d4..240a03e 100644
@@ -11,6 +11,7 @@
 
 /include/ "skeleton.dtsi"
 
+#include <dt-bindings/clock/r8a7779-clock.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 
 / {

@@ -278,4 +279,132 @@
 		interrupts = <0 75 IRQ_TYPE_LEVEL_HIGH>;
 		status = "disabled";
 	};
+
+	clocks {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/* External root clock */
+		extal_clk: extal_clk {
+			compatible = "fixed-clock";

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

+			clocks = <&extal_clk>;
+			#clock-cells = <1>;
+			clock-output-names = "plla", "z", "zs", "s",
+					     "s1", "p", "out";
+		};
+
+		/* Fixed factor clocks */
+		i_clk: i_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <2>;
+			clock-mult = <1>;
+			clock-output-names = "i";
+		};
+		s3_clk: s3_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <8>;
+			clock-mult = <1>;
+			clock-output-names = "s3";
+		};
+		s4_clk: s4_clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpg_clocks R8A7779_CLK_PLLA>;
+			#clock-cells = <0>;
+			clock-div = <16>;
+			clock-mult = <1>;
+			clock-output-names = "s4";
+		};
+		g_clk: g_clk {
+			compatible = "fixed-factor-clock";

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

 <&cpg_clocks R8A7779_CLK_P>, +				 <&cpg_clocks R8A7779_CLK_P>,

<&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>, + <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks
R8A7779_CLK_P>, + <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks
R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>, + <&cpg_clocks
R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,

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

+				 <&cpg_clocks R8A7779_CLK_P>;
+			#clock-cells = <1>;
+			renesas,clock-indices = <
+				R8A7779_CLK_HSPI R8A7779_CLK_TMU0 R8A7779_CLK_TMU0
+				R8A7779_CLK_TMU0 R8A7779_CLK_SCIF5 R8A7779_CLK_SCIF4
+				R8A7779_CLK_SCIF3 R8A7779_CLK_SCIF2 R8A7779_CLK_SCIF1
+				R8A7779_CLK_SCIF0 R8A7779_CLK_I2C3 R8A7779_CLK_I2C2
+				R8A7779_CLK_I2C1 R8A7779_CLK_I2C0

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

+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>;
+			#clock-cells = <1>;
+			renesas,clock-indices = <
+				R8A7779_CLK_USB01 R8A7779_CLK_USB01
+				R8A7779_CLK_USB2 R8A7779_CLK_USB2
+				R8A7779_CLK_DU R8A7779_CLK_VIN2
+				R8A7779_CLK_VIN1 R8A7779_CLK_VIN0
+				R8A7779_CLK_ETHER R8A7779_CLK_SATA
+				R8A7779_CLK_PCIE R8A7779_CLK_VIN3
+			>;

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

+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>,
+				 <&cpg_clocks R8A7779_CLK_P>, <&cpg_clocks R8A7779_CLK_P>;
+			#clock-cells = <1>;
+			renesas,clock-indices = <
+				R8A7779_CLK_SDHI3 R8A7779_CLK_SDHI2
+				R8A7779_CLK_SDHI1 R8A7779_CLK_SDHI0
+				R8A7779_CLK_MMC1 R8A7779_CLK_MMC0
+			>;
+			clock-output-names =
+				"sdhi3", "sdhi2",
+				"sdhi1", "sdhi0",
+				"mmc1", "mmc0";
+		};
+	};
 };

Hi Simon,

One more comment.

Quoted 57 lines · On Wednesday 26 February 2014 16:33:20 Simon Horman wrote:

Declare all core and MSTP clocks currently used by r8a7779-based boards.

Based on work by Laurent Pinchart for the r8a7790 and r8a7791 SoCs.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* As suggested by Laurent Pinchart
- Add and use extal_clk
- Fix bogus status register use for MSTP clocks
- Fix bogus mstp3_cls to use its own entries rather than that of mstp1_clks

* Update to use "main" in cpg_clocks as per updated binding in previous patch
* Update for new, consolidated and renamed index macros
- R8A7779_CLK_ETHER
- R8A7779_CLK_HSCIF
- R8A7779_CLK_HSPI
- R8A7779_CLK_MMC0,1
- R8A7779_CLK_PCIE
- R8A7779_CLK_USB01,2

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

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

index d0561d4..240a03e 100644
@@ -11,6 +11,7 @@
 
 /include/ "skeleton.dtsi"
 
+#include <dt-bindings/clock/r8a7779-clock.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 
 / {

@@ -278,4 +279,132 @@
 		interrupts = <0 75 IRQ_TYPE_LEVEL_HIGH>;
 		status = "disabled";
 	};
+
+	clocks {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		/* External root clock */
+		extal_clk: extal_clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			/* This value must be overriden by the board. */
+			clock-frequency = <0>;
+			clock-output-names = "extal";

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

Quoted 6 lines

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

+		};
+
+		/* Special CPG clocks */
+		cpg_clocks: cpg_clocks at 0xe6150000 {
+			compatible = "renesas,r8a7779-cpg-clocks";
+			reg = <0 0xe6150000 0 0x1000>;

Copied from H2 ? :-) This should probably be

Declare all core and MSTP clocks currently used by r8a7779-based boards.

Based on work by Laurent Pinchart for the r8a7790 and r8a7791 SoCs.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

Copy the device nodes from Marzen reference into the Marzen device tree file. This will allow us to use a single DTS file regardless of kernel configuration. In case of legacy C board code the device nodes may or may not be used, but in the multiplatform case all the DT device nodes will be used.

Based on a similar change for the Lager board by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* Updated for recent changes to Marzen reference DTS

---
 arch/arm/boot/dts/r8a7779-marzen.dts | 93

++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+)

(quoted diff) +0 -0

b/arch/arm/boot/dts/r8a7779-marzen.dts index a7af2c2..eda6240 100644

arch/arm/boot/dts/r8a7779-marzen.dts +93 -0

@@ -11,6 +11,8 @@
 
 /dts-v1/;
 #include "r8a7779.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
 
 / {
 	model = "marzen";

@@ -24,4 +26,95 @@
 		device_type = "memory";
 		reg = <0x60000000 0x40000000>;
 	};
+
+	fixedregulator3v3: fixedregulator at 0 {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	lan0 at 18000000 {
+		compatible = "smsc,lan9220", "smsc,lan9115";
+		reg = <0x18000000 0x100>;
+		pinctrl-0 = <&lan0_pins>;
+		pinctrl-names = "default";
+
+		phy-mode = "mii";
+		interrupt-parent = <&irqpin0>;
+		interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
+		reg-io-width = <4>;
+		vddvario-supply = <&fixedregulator3v3>;
+		vdd33a-supply = <&fixedregulator3v3>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+		led2 {
+			gpios = <&gpio4 29 GPIO_ACTIVE_HIGH>;
+		};
+		led3 {
+			gpios = <&gpio4 30 GPIO_ACTIVE_HIGH>;
+		};
+		led4 {
+			gpios = <&gpio4 31 GPIO_ACTIVE_HIGH>;
+		};
+	};
+};
+
+&irqpin0 {
+	status = "okay";
+};
+
+&pfc {
+	pinctrl-0 = <&scif2_pins &scif4_pins>;
+	pinctrl-names = "default";
+
+	lan0_pins: lan0 {
+		intc {
+			renesas,groups = "intc_irq1_b";
+			renesas,function = "intc";
+		};
+		lbsc {
+			renesas,groups = "lbsc_ex_cs0";
+			renesas,function = "lbsc";
+		};
+	};
+
+	scif2_pins: serial2 {
+		renesas,groups = "scif2_data_c";
+		renesas,function = "scif2";
+	};
+
+	scif4_pins: serial4 {
+		renesas,groups = "scif4_data";
+		renesas,function = "scif4";
+	};
+
+	sdhi0_pins: sd0 {
+		renesas,groups = "sdhi0_data4", "sdhi0_ctrl", "sdhi0_cd";
+		renesas,function = "sdhi0";
+	};
+
+	hspi0_pins: hspi0 {
+		renesas,groups = "hspi0";
+		renesas,function = "hspi0";
+	};
+};
+
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_pins>;
+	pinctrl-names = "default";
+
+	vmmc-supply = <&fixedregulator3v3>;
+	bus-width = <4>;
+	status = "okay";
+};
+
+&hspi0 {
+	pinctrl-0 = <&hspi0_pins>;
+	pinctrl-names = "default";
+	status = "okay";
 };

The external crystal frequency is 31.25 on the Marzen board. Specify it in the device tree.

Based on work for the Lager board by Laurent Pinchart.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* Update r8a7779-marzen.dts instead of r8a7779-marzen-reference.dts

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

(quoted diff) +0 -0

b/arch/arm/boot/dts/r8a7779-marzen.dts index eda6240..2c727cd 100644

arch/arm/boot/dts/r8a7779-marzen.dts +4 -0

@@ -68,6 +68,10 @@
 	status = "okay";
 };
 
+&extal_clk {
+	clock-frequency = <31250000>;
+};
+
 &pfc {
 	pinctrl-0 = <&scif2_pins &scif4_pins>;
 	pinctrl-names = "default";

Reference clocks using a "clocks" property in all nodes corresponding to devices that require a clock: that is, the lan0 node.

Based on work by Laurent Pinchart for the r8a7790 and r8a7791 SoCs.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* New

---
 arch/arm/boot/dts/r8a7779-marzen.dts | 1 +
 1 file changed, 1 insertion(+)

(quoted diff) +0 -0

b/arch/arm/boot/dts/r8a7779-marzen.dts index 2c727cd..30f57e9 100644

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

@@ -41,6 +41,7 @@
 		reg = <0x18000000 0x100>;
 		pinctrl-0 = <&lan0_pins>;
 		pinctrl-names = "default";
+		clocks = <&mstp1_clks R8A7779_CLK_ETHER>;
 
 		phy-mode = "mii";
 		interrupt-parent = <&irqpin0>;

Reference clocks using a "clocks" property in all nodes corresponding to devices that require a clock: that is, the lan0 node.

Based on work by Laurent Pinchart for the r8a7790 and r8a7791 SoCs.

Cc Laurent Pinchart <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---
v3
* New

---
  arch/arm/boot/dts/r8a7779-marzen.dts | 1 +
  1 file changed, 1 insertion(+)

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

index 2c727cd..30f57e9 100644

Hello.

Quoted 2 lines · Simon Horman

Reference clocks using a "clocks" property in all nodes corresponding to devices that require a clock: that is, the lan0 node.

Simon, "lan0" (should be "ethernet" BTW) is the SMC chip, it
can't use Ether clock.