[PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

Add support for the R7S9210 (RZ/A2) Clock Pulse Generator and Module
Standby.

The Module Standby HW in the RZ/A series is very close to R-Car HW, except
for how the registers are laid out.
The MSTP registers are only 8-bits wide, there is no status registers
(MSTPST), and the register offsets are a little different. Since the RZ/A
hardware manuals refer to these registers as the Standby Control Registers,
we'll use that name to distinguish the RZ/A type for the R-Car type.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v3:
 * Use actual register bit names and numbers from manual for both DT and
   tables ("36" instead of "306")
 * Do not register reset controller for stbyctrl (RZ/A) SoCs
 * Changed SPDX from "GPL-2.0+" to "GPL-2.0"
v2:
 * num_hw_mod_clks was wrong
 * added ethernet clocks
---
 .../devicetree/bindings/clock/renesas,cpg-mssr.txt |   3 +-
 drivers/clk/renesas/Kconfig                        |   5 +
 drivers/clk/renesas/Makefile                       |   1 +
 drivers/clk/renesas/r7s9210-cpg-mssr.c             | 193 +++++++++++++++++++++
 drivers/clk/renesas/renesas-cpg-mssr.c             |  81 +++++++--
 drivers/clk/renesas/renesas-cpg-mssr.h             |  13 ++
 include/dt-bindings/clock/r7s9210-cpg-mssr.h       |  21 +++
 7 files changed, 304 insertions(+), 13 deletions(-)
 create mode 100644 drivers/clk/renesas/r7s9210-cpg-mssr.c
 create mode 100644 include/dt-bindings/clock/r7s9210-cpg-mssr.h

diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
index 42d0f83d812b..012416b33d2d 100644
--- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
+++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
@@ -13,6 +13,7 @@ They provide the following functionalities:
 
 Required Properties:
   - compatible: Must be one of:
+      - "renesas,r7s9210-cpg-mssr" for the r7s9210 SoC (RZ/A2)
       - "renesas,r8a7743-cpg-mssr" for the r8a7743 SoC (RZ/G1M)
       - "renesas,r8a7745-cpg-mssr" for the r8a7745 SoC (RZ/G1E)
       - "renesas,r8a77470-cpg-mssr" for the r8a77470 SoC (RZ/G1C)
@@ -38,7 +39,7 @@ Required Properties:
   - clock-names: List of external parent clock names. Valid names are:
       - "extal" (r8a7743, r8a7745, r8a77470, r8a774a1, r8a7790, r8a7791,
 		 r8a7792, r8a7793, r8a7794, r8a7795, r8a7796, r8a77965,
-		 r8a77970, r8a77980, r8a77990, r8a77995)
+		 r8a77970, r8a77980, r8a77990, r8a77995, r7s9210)
       - "extalr" (r8a774a1, r8a7795, r8a7796, r8a77965, r8a77970, r8a77980)
       - "usb_extal" (r8a7743, r8a7745, r8a77470, r8a7790, r8a7791, r8a7793,
 		     r8a7794)
diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig
index f998a7333acb..2edcb1bdb487 100644
--- a/drivers/clk/renesas/Kconfig
+++ b/drivers/clk/renesas/Kconfig
@@ -3,6 +3,7 @@ config CLK_RENESAS
 	default y if ARCH_RENESAS
 	select CLK_EMEV2 if ARCH_EMEV2
 	select CLK_RZA1 if ARCH_R7S72100
+	select CLK_R7S9210 if ARCH_R7S9210
 	select CLK_R8A73A4 if ARCH_R8A73A4
 	select CLK_R8A7740 if ARCH_R8A7740
 	select CLK_R8A7743 if ARCH_R8A7743
@@ -46,6 +47,10 @@ config CLK_RZA1
 	bool "RZ/A1H clock support" if COMPILE_TEST
 	select CLK_RENESAS_CPG_MSTP
 
+config CLK_R7S9210
+	bool "RZ/A2 clock support" if COMPILE_TEST
+	select CLK_RENESAS_CPG_MSSR
+
 config CLK_R8A73A4
 	bool "R-Mobile APE6 clock support" if COMPILE_TEST
 	select CLK_RENESAS_CPG_MSTP
diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index 71d4cafe15c0..dbbfd0b0742b 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -2,6 +2,7 @@
 # SoC
 obj-$(CONFIG_CLK_EMEV2)			+= clk-emev2.o
 obj-$(CONFIG_CLK_RZA1)			+= clk-rz.o
+obj-$(CONFIG_CLK_R7S9210)		+= r7s9210-cpg-mssr.o
 obj-$(CONFIG_CLK_R8A73A4)		+= clk-r8a73a4.o
 obj-$(CONFIG_CLK_R8A7740)		+= clk-r8a7740.o
 obj-$(CONFIG_CLK_R8A7743)		+= r8a7743-cpg-mssr.o
diff --git a/drivers/clk/renesas/r7s9210-cpg-mssr.c b/drivers/clk/renesas/r7s9210-cpg-mssr.c
new file mode 100644
index 000000000000..faba66cea7d4
--- /dev/null
+++ b/drivers/clk/renesas/r7s9210-cpg-mssr.c
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * R7S9210 Clock Pulse Generator / Module Standby
+ *
+ * Based on r8a7795-cpg-mssr.c
+ *
+ * Copyright (C) 2018 Chris Brandt
+ * Copyright (C) 2018 Renesas Electronics Corp.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <dt-bindings/clock/r7s9210-cpg-mssr.h>
+#include "renesas-cpg-mssr.h"
+
+#define CPG_FRQCR	0x00
+#define CPG_CKIOSEL	0xF0
+#define CPG_SCLKSEL	0xF4
+
+#define PORTL_PIDR	0xFCFFE074
+static u8 cpg_mode;
+
+/* Internal Clock ratio table */
+static const unsigned int ratio_tab[5][5] = {
+			/* I,  G,  B, P1, P0 */
+			{  2,  4,  8, 16, 32 },	/* FRQCR = 0x012 */
+			{  4,  4,  8, 16, 32 },	/* FRQCR = 0x112 */
+			{  8,  4,  8, 16, 32 },	/* FRQCR = 0x212 */
+			{ 16,  8, 16, 16, 32 },	/* FRQCR = 0x322 */
+			{ 16, 16, 32, 32, 32 },	/* FRQCR = 0x333 */
+			};
+
+enum rz_clk_types {
+	CLK_TYPE_RZA_MAIN = CLK_TYPE_CUSTOM,
+	CLK_TYPE_RZA_PLL,
+};
+
+enum clk_ids {
+	/* Core Clock Outputs exported to DT */
+	LAST_DT_CORE_CLK = R7S9210_CLK_P0,
+
+	/* External Input Clocks */
+	CLK_EXTAL,
+
+	/* Internal Core Clocks */
+	CLK_MAIN,
+	CLK_PLL,
+	CLK_I,
+	CLK_G,
+	CLK_B,
+	CLK_P1,
+	CLK_P1C,
+	CLK_P0,
+
+	/* Module Clocks */
+	MOD_CLK_BASE
+};
+
+static struct cpg_core_clk r7s9210_core_clks[] = {
+	/* External Clock Inputs */
+	DEF_INPUT("extal",     CLK_EXTAL),
+
+	/* Internal Core Clocks */
+	DEF_BASE(".main",       CLK_MAIN, CLK_TYPE_RZA_MAIN, CLK_EXTAL),
+	DEF_BASE(".pll",       CLK_PLL, CLK_TYPE_RZA_PLL, CLK_MAIN),
+
+	/* Core Clock Outputs */
+	DEF_FIXED("i",      R7S9210_CLK_I,     CLK_PLL,          2, 1),
+	DEF_FIXED("g",      R7S9210_CLK_G,     CLK_PLL,          4, 1),
+	DEF_FIXED("b",      R7S9210_CLK_B,     CLK_PLL,          8, 1),
+	DEF_FIXED("p1",     R7S9210_CLK_P1,    CLK_PLL,         16, 1),
+	DEF_FIXED("p1c",    R7S9210_CLK_P1C,   CLK_PLL,         16, 1),
+	DEF_FIXED("p0",     R7S9210_CLK_P0,    CLK_PLL,         32, 1),
+};
+
+static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = {
+	DEF_MOD_STB("ostm0",	 36,	R7S9210_CLK_P1C),
+	DEF_MOD_STB("ostm1",	 35,	R7S9210_CLK_P1C),
+	DEF_MOD_STB("ostm2",	 34,	R7S9210_CLK_P1C),
+
+	DEF_MOD_STB("scif0",	 47,	R7S9210_CLK_P1C),
+	DEF_MOD_STB("scif1",	 46,	R7S9210_CLK_P1C),
+	DEF_MOD_STB("scif2",	 45,	R7S9210_CLK_P1C),
+	DEF_MOD_STB("scif3",	 44,	R7S9210_CLK_P1C),
+	DEF_MOD_STB("scif4",	 43,	R7S9210_CLK_P1C),
+
+	DEF_MOD_STB("ether0",	 65,	R7S9210_CLK_B),
+	DEF_MOD_STB("ether1",	 64,	R7S9210_CLK_B),
+
+	DEF_MOD_STB("i2c0",	 87,	R7S9210_CLK_P1),
+	DEF_MOD_STB("i2c1",	 86,	R7S9210_CLK_P1),
+	DEF_MOD_STB("i2c2",	 85,	R7S9210_CLK_P1),
+	DEF_MOD_STB("i2c3",	 84,	R7S9210_CLK_P1),
+
+};
+
+struct clk * __init rza2_cpg_clk_register(struct device *dev,
+	const struct cpg_core_clk *core, const struct cpg_mssr_info *info,
+	struct clk **clks, void __iomem *base,
+	struct raw_notifier_head *notifiers)
+{
+	const struct clk *parent;
+	unsigned int mult = 1;
+	unsigned int div = 1;
+	u16 frqcr;
+	u8 index;
+	int i;
+
+	parent = clks[core->parent];
+	if (IS_ERR(parent))
+		return ERR_CAST(parent);
+
+	switch (core->id) {
+	case CLK_MAIN:
+		break;
+
+	case CLK_PLL:
+		if (cpg_mode)
+			mult = 44;	/* Divider 1 is 1/2 */
+		else
+			mult = 88;	/* Divider 1 is 1 */
+		break;
+
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* Adjust the dividers based on the current FRQCR setting */
+	if (core->id == CLK_MAIN) {
+
+		/* If EXTAL is above 12MHz, then we know it is Mode 1 */
+		if (clk_get_rate((struct clk *)parent) > 12000000)
+			cpg_mode = 1;
+
+		frqcr = clk_readl(base + CPG_FRQCR) & 0xFFF;
+		if (frqcr == 0x012)
+			index = 0;
+		else if (frqcr == 0x112)
+			index = 1;
+		else if (frqcr == 0x212)
+			index = 2;
+		else if (frqcr == 0x322)
+			index = 3;
+		else if (frqcr == 0x333)
+			index = 4;
+		else
+			BUG_ON(1);	/* Illegal FRQCR value */
+
+		for (i = 0; i < ARRAY_SIZE(r7s9210_core_clks); i++) {
+			switch (r7s9210_core_clks[i].id) {
+			case R7S9210_CLK_I:
+				r7s9210_core_clks[i].div = ratio_tab[index][0];
+				break;
+			case R7S9210_CLK_G:
+				r7s9210_core_clks[i].div = ratio_tab[index][1];
+				break;
+			case R7S9210_CLK_B:
+				r7s9210_core_clks[i].div = ratio_tab[index][2];
+				break;
+			case R7S9210_CLK_P1:
+			case R7S9210_CLK_P1C:
+				r7s9210_core_clks[i].div = ratio_tab[index][3];
+				break;
+			case R7S9210_CLK_P0:
+				r7s9210_core_clks[i].div = ratio_tab[index][4];
+				break;
+			}
+		}
+	}
+
+	return clk_register_fixed_factor(NULL, core->name,
+					 __clk_get_name(parent), 0, mult, div);
+}
+
+const struct cpg_mssr_info r7s9210_cpg_mssr_info __initconst = {
+	/* Core Clocks */
+	.core_clks = r7s9210_core_clks,
+	.num_core_clks = ARRAY_SIZE(r7s9210_core_clks),
+	.last_dt_core_clk = LAST_DT_CORE_CLK,
+	.num_total_core_clks = MOD_CLK_BASE,
+
+	/* Module Clocks */
+	.mod_clks = r7s9210_mod_clks,
+	.num_mod_clks = ARRAY_SIZE(r7s9210_mod_clks),
+	.num_hw_mod_clks = 11 * 32, /* includes STBCR0/1/2 which don't exist */
+
+	/* Callbacks */
+	.cpg_clk_register = rza2_cpg_clk_register,
+
+	/* RZ/A2 has Standby Control Registers */
+	.stbyctrl = true,
+};
diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index f90b0d0ba46a..14cde80716e4 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -73,6 +73,17 @@ static const u16 smstpcr[] = {
 
 #define	SMSTPCR(i)	smstpcr[i]
 
+/*
+ * Standby Control Register offsets (RZ/A)
+ * Base address is FRQCR register
+ */
+
+static const u16 stbcr[] = {
+	0x000, 0x000, 0x014, 0x410, 0x414, 0x418, 0x41C, 0x420,
+	0x424, 0x428, 0x42C, 0x430, 0x434, 0x460,
+};
+
+#define	STBCR(i)	stbcr[i]
 
 /*
  * Software Reset Register offsets
@@ -110,6 +121,7 @@ static const u16 srcr[] = {
  * @notifiers: Notifier chain to save/restore clock state for system resume
  * @smstpcr_saved[].mask: Mask of SMSTPCR[] bits under our control
  * @smstpcr_saved[].val: Saved values of SMSTPCR[]
+ * @stbyctrl: This device has Standby Control Registers
  */
 struct cpg_mssr_priv {
 #ifdef CONFIG_RESET_CONTROLLER
@@ -123,6 +135,7 @@ struct cpg_mssr_priv {
 	unsigned int num_core_clks;
 	unsigned int num_mod_clks;
 	unsigned int last_dt_core_clk;
+	bool stbyctrl;
 
 	struct raw_notifier_head notifiers;
 	struct {
@@ -162,16 +175,29 @@ static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable)
 		enable ? "ON" : "OFF");
 	spin_lock_irqsave(&priv->rmw_lock, flags);
 
-	value = readl(priv->base + SMSTPCR(reg));
-	if (enable)
-		value &= ~bitmask;
-	else
-		value |= bitmask;
-	writel(value, priv->base + SMSTPCR(reg));
+	if (priv->stbyctrl) {
+		value = readb(priv->base + STBCR(reg));
+		if (enable)
+			value &= ~bitmask;
+		else
+			value |= bitmask;
+		writeb(value, priv->base + STBCR(reg));
+
+		/* dummy read to ensure write has completed */
+		readb(priv->base + STBCR(reg));
+		barrier_data(priv->base + STBCR(reg));
+	} else {
+		value = readl(priv->base + SMSTPCR(reg));
+		if (enable)
+			value &= ~bitmask;
+		else
+			value |= bitmask;
+		writel(value, priv->base + SMSTPCR(reg));
+	}
 
 	spin_unlock_irqrestore(&priv->rmw_lock, flags);
 
-	if (!enable)
+	if (!enable || priv->stbyctrl)
 		return 0;
 
 	for (i = 1000; i > 0; --i) {
@@ -205,7 +231,10 @@ static int cpg_mstp_clock_is_enabled(struct clk_hw *hw)
 	struct cpg_mssr_priv *priv = clock->priv;
 	u32 value;
 
-	value = readl(priv->base + MSTPSR(clock->index / 32));
+	if (priv->stbyctrl)
+		value = readb(priv->base + STBCR(clock->index / 32));
+	else
+		value = readl(priv->base + MSTPSR(clock->index / 32));
 
 	return !(value & BIT(clock->index % 32));
 }
@@ -226,6 +255,7 @@ struct clk *cpg_mssr_clk_src_twocell_get(struct of_phandle_args *clkspec,
 	unsigned int idx;
 	const char *type;
 	struct clk *clk;
+	int range_check;
 
 	switch (clkspec->args[0]) {
 	case CPG_CORE:
@@ -240,8 +270,14 @@ struct clk *cpg_mssr_clk_src_twocell_get(struct of_phandle_args *clkspec,
 
 	case CPG_MOD:
 		type = "module";
-		idx = MOD_CLK_PACK(clkidx);
-		if (clkidx % 100 > 31 || idx >= priv->num_mod_clks) {
+		if (priv->stbyctrl) {
+			idx = MOD_CLK_PACK_10(clkidx);
+			range_check = 7 - (clkidx % 10);
+		} else {
+			idx = MOD_CLK_PACK_10(clkidx);
+			range_check = 31 - (clkidx % 100);
+		}
+		if (range_check < 0 || idx >= priv->num_mod_clks) {
 			dev_err(dev, "Invalid %s clock index %u\n", type,
 				clkidx);
 			return ERR_PTR(-EINVAL);
@@ -740,6 +776,12 @@ static const struct of_device_id cpg_mssr_match[] = {
 		.compatible = "renesas,r8a77995-cpg-mssr",
 		.data = &r8a77995_cpg_mssr_info,
 	},
+#endif
+#ifdef CONFIG_CLK_R7S9210
+	{
+		.compatible = "renesas,r7s9210-cpg-mssr",
+		.data = &r7s9210_cpg_mssr_info,
+	},
 #endif
 	{ /* sentinel */ }
 };
@@ -791,13 +833,23 @@ static int cpg_mssr_resume_noirq(struct device *dev)
 		if (!mask)
 			continue;
 
-		oldval = readl(priv->base + SMSTPCR(reg));
+		if (priv->stbyctrl)
+			oldval = readb(priv->base + STBCR(reg));
+		else
+			oldval = readl(priv->base + SMSTPCR(reg));
 		newval = oldval & ~mask;
 		newval |= priv->smstpcr_saved[reg].val & mask;
 		if (newval == oldval)
 			continue;
 
-		writel(newval, priv->base + SMSTPCR(reg));
+		if (priv->stbyctrl) {
+			writeb(newval, priv->base + STBCR(reg));
+			/* dummy read to ensure write has completed */
+			readb(priv->base + STBCR(reg));
+			barrier_data(priv->base + STBCR(reg));
+			continue;
+		} else
+			writel(newval, priv->base + SMSTPCR(reg));
 
 		/* Wait until enabled clocks are really enabled */
 		mask &= ~priv->smstpcr_saved[reg].val;
@@ -869,6 +921,7 @@ static int __init cpg_mssr_probe(struct platform_device *pdev)
 	priv->num_mod_clks = info->num_hw_mod_clks;
 	priv->last_dt_core_clk = info->last_dt_core_clk;
 	RAW_INIT_NOTIFIER_HEAD(&priv->notifiers);
+	priv->stbyctrl = info->stbyctrl;
 
 	for (i = 0; i < nclks; i++)
 		clks[i] = ERR_PTR(-ENOENT);
@@ -894,6 +947,10 @@ static int __init cpg_mssr_probe(struct platform_device *pdev)
 	if (error)
 		return error;
 
+	/* Reset Controller not supported for Standby Control SoCs */
+	if (info->stbyctrl)
+		return 0;
+
 	error = cpg_mssr_reset_controller_register(priv);
 	if (error)
 		return error;
diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h
index 2e1730bc5ef2..58f32b38e99a 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.h
+++ b/drivers/clk/renesas/renesas-cpg-mssr.h
@@ -78,6 +78,13 @@ struct mssr_mod_clk {
 #define DEF_MOD(_name, _mod, _parent...)	\
 	{ .name = _name, .id = MOD_CLK_ID(_mod), .parent = _parent }
 
+/* Convert from sparse base-10 to packed index space */
+#define MOD_CLK_PACK_10(x)	((x / 10) * 32 + (x % 10))
+
+#define MOD_CLK_ID_10(x)	(MOD_CLK_BASE + MOD_CLK_PACK_10(x))
+
+#define DEF_MOD_STB(_name, _mod, _parent...)	\
+	{ .name = _name, .id = MOD_CLK_ID_10(_mod), .parent = _parent }
 
 struct device_node;
 
@@ -103,6 +110,10 @@ struct device_node;
      *
      * @init: Optional callback to perform SoC-specific initialization
      * @cpg_clk_register: Optional callback to handle special Core Clock types
+     *
+     * @stbyctrl: This device has Standby Control Registers which are 8-bits
+     *            wide, no status registers (MSTPSR) and have different address
+     *            offsets.
      */
 
 struct cpg_mssr_info {
@@ -111,6 +122,7 @@ struct cpg_mssr_info {
 	unsigned int num_core_clks;
 	unsigned int last_dt_core_clk;
 	unsigned int num_total_core_clks;
+	bool stbyctrl;
 
 	/* Module Clocks */
 	const struct mssr_mod_clk *mod_clks;
@@ -149,6 +161,7 @@ extern const struct cpg_mssr_info r8a77970_cpg_mssr_info;
 extern const struct cpg_mssr_info r8a77980_cpg_mssr_info;
 extern const struct cpg_mssr_info r8a77990_cpg_mssr_info;
 extern const struct cpg_mssr_info r8a77995_cpg_mssr_info;
+extern const struct cpg_mssr_info r7s9210_cpg_mssr_info;
 
 
     /*
diff --git a/include/dt-bindings/clock/r7s9210-cpg-mssr.h b/include/dt-bindings/clock/r7s9210-cpg-mssr.h
new file mode 100644
index 000000000000..676cc6321d8f
--- /dev/null
+++ b/include/dt-bindings/clock/r7s9210-cpg-mssr.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2018 Renesas Electronics Corp.
+ *
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__
+#define __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__
+
+#include <dt-bindings/clock/renesas-cpg-mssr.h>
+
+/* R7S9210 CPG Core Clocks */
+#define R7S9210_CLK_PLL			0
+#define R7S9210_CLK_I			1
+#define R7S9210_CLK_G			2
+#define R7S9210_CLK_B			3
+#define R7S9210_CLK_P1			4
+#define R7S9210_CLK_P1C			5
+#define R7S9210_CLK_P0			6
+
+#endif /* __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__ */
-- 
2.16.1

RE: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

Hi Geert,

On Wednesday, August 29, 2018, Chris Brandt wrote:
> 
> Add support for the R7S9210 (RZ/A2) Clock Pulse Generator and Module
> Standby.

I ran into an issue with this driver.

The issue is that the registers needed for this driver are scattered 
throughout the address space.

I can't just reserve one big block of addresses that cover everything 
because then I can't reserve any of the addresses that sit in the middle.

For example, my watchdog timer driver doesn't work with this driver 
because it cannot allocate its register region.

So, I think I need to rework this driver to add the ability to add 
multiple address regions.

Chris

RE: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

Hi Geert,

On Wednesday, August 29, 2018, Chris Brandt wrote:
>=20
> Add support for the R7S9210 (RZ/A2) Clock Pulse Generator and Module
> Standby.

I ran into an issue with this driver.

The issue is that the registers needed for this driver are scattered=20
throughout the address space.

I can't just reserve one big block of addresses that cover everything=20
because then I can't reserve any of the addresses that sit in the middle.

For example, my watchdog timer driver doesn't work with this driver=20
because it cannot allocate its register region.

So, I think I need to rework this driver to add the ability to add=20
multiple address regions.

Chris

Re: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Geert Uytterhoeven]

Hi Chris,

On Wed, Sep 5, 2018 at 4:12 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Wednesday, August 29, 2018, Chris Brandt wrote:
> > Add support for the R7S9210 (RZ/A2) Clock Pulse Generator and Module
> > Standby.
>
> I ran into an issue with this driver.
>
> The issue is that the registers needed for this driver are scattered
> throughout the address space.
>
> I can't just reserve one big block of addresses that cover everything
> because then I can't reserve any of the addresses that sit in the middle.

Yeah, this address range is kind of a "syscon" block.

> For example, my watchdog timer driver doesn't work with this driver
> because it cannot allocate its register region.
>
> So, I think I need to rework this driver to add the ability to add
> multiple address regions.

Your main driver for this block (clock?) can register the watchdog.
Either directly with the watchdog subsystem, or by registering an
"rza_wdt" platform device.

One more vote for a separate rza2-cpg-stb-wdt driver?...

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 v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

Hi Geert,

On Wednesday, September 05, 2018 1, Geert Uytterhoeven wrote:
> > So, I think I need to rework this driver to add the ability to add
> > multiple address regions.
> 
> Your main driver for this block (clock?) can register the watchdog.
> Either directly with the watchdog subsystem, or by registering an
> "rza_wdt" platform device.

It would seem strange that the clock driver would register the watchdog 
timer for no other reason other than where the chip designer happen to 
put the register addresses.


Now that I look at it:
 CPG is at FCFE_0010 - FCFE_0104
MSTP is at FCFE_0400 - FCFE_0464

Nothing sits in between CPG and MSTP.

In the hardware manual, there is other 'random other stuff' that they 
put in that chapter that is located in multiple address places, but that 
doesn't mean I have to map them all in this driver.


I simply changed the DTS to just map CPG and MSTP and now everything is 
fine. My watchdog driver is happy and this CPG/MSTP driver works the 
same.

So....I guess I didn't really have an issue after all.

Chris

RE: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

Hi Geert,

On Wednesday, September 05, 2018 1, Geert Uytterhoeven wrote:
> > So, I think I need to rework this driver to add the ability to add
> > multiple address regions.
> 
> Your main driver for this block (clock?) can register the watchdog.
> Either directly with the watchdog subsystem, or by registering an
> "rza_wdt" platform device.

It would seem strange that the clock driver would register the watchdog 
timer for no other reason other than where the chip designer happen to 
put the register addresses.


Now that I look at it:
 CPG is at FCFE_0010 - FCFE_0104
MSTP is at FCFE_0400 - FCFE_0464

Nothing sits in between CPG and MSTP.

In the hardware manual, there is other 'random other stuff' that they 
put in that chapter that is located in multiple address places, but that 
doesn't mean I have to map them all in this driver.


I simply changed the DTS to just map CPG and MSTP and now everything is 
fine. My watchdog driver is happy and this CPG/MSTP driver works the 
same.

So....I guess I didn't really have an issue after all.

Chris

RE: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

SGkgR2VlcnQsDQoNCk9uIFdlZG5lc2RheSwgU2VwdGVtYmVyIDA1LCAyMDE4IDEsIEdlZXJ0IFV5
dHRlcmhvZXZlbiB3cm90ZToNCj4gPiBTbywgSSB0aGluayBJIG5lZWQgdG8gcmV3b3JrIHRoaXMg
ZHJpdmVyIHRvIGFkZCB0aGUgYWJpbGl0eSB0byBhZGQNCj4gPiBtdWx0aXBsZSBhZGRyZXNzIHJl
Z2lvbnMuDQo+IA0KPiBZb3VyIG1haW4gZHJpdmVyIGZvciB0aGlzIGJsb2NrIChjbG9jaz8pIGNh
biByZWdpc3RlciB0aGUgd2F0Y2hkb2cuDQo+IEVpdGhlciBkaXJlY3RseSB3aXRoIHRoZSB3YXRj
aGRvZyBzdWJzeXN0ZW0sIG9yIGJ5IHJlZ2lzdGVyaW5nIGFuDQo+ICJyemFfd2R0IiBwbGF0Zm9y
bSBkZXZpY2UuDQoNCkl0IHdvdWxkIHNlZW0gc3RyYW5nZSB0aGF0IHRoZSBjbG9jayBkcml2ZXIg
d291bGQgcmVnaXN0ZXIgdGhlIHdhdGNoZG9nIA0KdGltZXIgZm9yIG5vIG90aGVyIHJlYXNvbiBv
dGhlciB0aGFuIHdoZXJlIHRoZSBjaGlwIGRlc2lnbmVyIGhhcHBlbiB0byANCnB1dCB0aGUgcmVn
aXN0ZXIgYWRkcmVzc2VzLg0KDQoNCk5vdyB0aGF0IEkgbG9vayBhdCBpdDoNCiBDUEcgaXMgYXQg
RkNGRV8wMDEwIC0gRkNGRV8wMTA0DQpNU1RQIGlzIGF0IEZDRkVfMDQwMCAtIEZDRkVfMDQ2NA0K
DQpOb3RoaW5nIHNpdHMgaW4gYmV0d2VlbiBDUEcgYW5kIE1TVFAuDQoNCkluIHRoZSBoYXJkd2Fy
ZSBtYW51YWwsIHRoZXJlIGlzIG90aGVyICdyYW5kb20gb3RoZXIgc3R1ZmYnIHRoYXQgdGhleSAN
CnB1dCBpbiB0aGF0IGNoYXB0ZXIgdGhhdCBpcyBsb2NhdGVkIGluIG11bHRpcGxlIGFkZHJlc3Mg
cGxhY2VzLCBidXQgdGhhdCANCmRvZXNuJ3QgbWVhbiBJIGhhdmUgdG8gbWFwIHRoZW0gYWxsIGlu
IHRoaXMgZHJpdmVyLg0KDQoNCkkgc2ltcGx5IGNoYW5nZWQgdGhlIERUUyB0byBqdXN0IG1hcCBD
UEcgYW5kIE1TVFAgYW5kIG5vdyBldmVyeXRoaW5nIGlzIA0KZmluZS4gTXkgd2F0Y2hkb2cgZHJp
dmVyIGlzIGhhcHB5IGFuZCB0aGlzIENQRy9NU1RQIGRyaXZlciB3b3JrcyB0aGUgDQpzYW1lLg0K
DQpTby4uLi5JIGd1ZXNzIEkgZGlkbid0IHJlYWxseSBoYXZlIGFuIGlzc3VlIGFmdGVyIGFsbC4N
Cg0KQ2hyaXMNCg0K

Re: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Geert Uytterhoeven]

Hi Chris,

On Wed, Sep 5, 2018 at 5:02 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Wednesday, September 05, 2018 1, Geert Uytterhoeven wrote:
> > > So, I think I need to rework this driver to add the ability to add
> > > multiple address regions.
> >
> > Your main driver for this block (clock?) can register the watchdog.
> > Either directly with the watchdog subsystem, or by registering an
> > "rza_wdt" platform device.
>
> It would seem strange that the clock driver would register the watchdog
> timer for no other reason other than where the chip designer happen to
> put the register addresses.

That not that uncommon for "syscon" drivers...

> Now that I look at it:
>  CPG is at FCFE_0010 - FCFE_0104
> MSTP is at FCFE_0400 - FCFE_0464
>
> Nothing sits in between CPG and MSTP.

OK.

> In the hardware manual, there is other 'random other stuff' that they
> put in that chapter that is located in multiple address places, but that
> doesn't mean I have to map them all in this driver.

OK.

> I simply changed the DTS to just map CPG and MSTP and now everything is
> fine. My watchdog driver is happy and this CPG/MSTP driver works the
> same.

Good, I will resume review of the patch in $subject ;-)

> So....I guess I didn't really have an issue after all.

You do want to:
  1. Document the two register ranges in the DT bindings,
  2. Update the driver to map both ranges on RZ/A2.
Right now it works by luck, as ioremap()'s granularity is PAGE_SIZE.

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 v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

Hi Geert,

On Wednesday, September 05, 2018 1, Geert Uytterhoeven wrote:
> > So....I guess I didn't really have an issue after all.
> 
> You do want to:
>   1. Document the two register ranges in the DT bindings,
>   2. Update the driver to map both ranges on RZ/A2.

The driver does not need to be updated.

Given this mapping:

	cpg: clock-controller@fcfe0020 {
		compatible = "renesas,r7s9210-cpg-mssr";
		reg = <0xfcfe0010 0x455>;  /* FCFE0010 - FCFE0465 */
		clocks = <&extal_clk>;
		clock-names = "extal";
		#clock-cells = <2>;
		#power-domain-cells = <0>;
		#reset-cells = <1>;
	};

This covers all the CPG, MSTP, power down and reset registers.

So I'm just going to map them all at once.

And, you can see that some CPG and MSTP register are mixed together.
Here is the list of registers in address order:

 FRQCR   FCFE_0010  (CPG)
STBCR1   FCFE_0020  (MSTP)
STBCR2   FCFE_0024  (MSTP)
CKIOSEL  FCFE_0100  (CPG)
SCLKSEL  FCFE_0104  (CPG)
STBCR3   FCFE_0420  (MSTP)
STBCR4   FCFE_0424  (MSTP)
STBCR5   FCFE_0428  (MSTP)
STBCR6   FCFE_042C  (MSTP)
STBCR7   FCFE_0430  (MSTP)
STBCR8   FCFE_0434  (MSTP)
STBCR9   FCFE_0438  (MSTP)
STBCR10  FCFE_043C  (MSTP)
SWRSTCR1 FCFE_0460  (RESET)
SWRSTCR2 FCFE_0464  (RESET)
  etc...

So this should be all one memory block in this driver.

When you get to FCFF_C000+, that's where the 'specialty' registers are. 
I think FCFF_C000+ should be a separate block, maybe in a separate driver.


Chris

RE: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

Hi Geert,

On Wednesday, September 05, 2018 1, Geert Uytterhoeven wrote:
> > So....I guess I didn't really have an issue after all.
> 
> You do want to:
>   1. Document the two register ranges in the DT bindings,
>   2. Update the driver to map both ranges on RZ/A2.

The driver does not need to be updated.

Given this mapping:

	cpg: clock-controller@fcfe0020 {
		compatible = "renesas,r7s9210-cpg-mssr";
		reg = <0xfcfe0010 0x455>;  /* FCFE0010 - FCFE0465 */
		clocks = <&extal_clk>;
		clock-names = "extal";
		#clock-cells = <2>;
		#power-domain-cells = <0>;
		#reset-cells = <1>;
	};

This covers all the CPG, MSTP, power down and reset registers.

So I'm just going to map them all at once.

And, you can see that some CPG and MSTP register are mixed together.
Here is the list of registers in address order:

 FRQCR   FCFE_0010  (CPG)
STBCR1   FCFE_0020  (MSTP)
STBCR2   FCFE_0024  (MSTP)
CKIOSEL  FCFE_0100  (CPG)
SCLKSEL  FCFE_0104  (CPG)
STBCR3   FCFE_0420  (MSTP)
STBCR4   FCFE_0424  (MSTP)
STBCR5   FCFE_0428  (MSTP)
STBCR6   FCFE_042C  (MSTP)
STBCR7   FCFE_0430  (MSTP)
STBCR8   FCFE_0434  (MSTP)
STBCR9   FCFE_0438  (MSTP)
STBCR10  FCFE_043C  (MSTP)
SWRSTCR1 FCFE_0460  (RESET)
SWRSTCR2 FCFE_0464  (RESET)
  etc...

So this should be all one memory block in this driver.

When you get to FCFF_C000+, that's where the 'specialty' registers are. 
I think FCFF_C000+ should be a separate block, maybe in a separate driver.


Chris

RE: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

SGkgR2VlcnQsDQoNCk9uIFdlZG5lc2RheSwgU2VwdGVtYmVyIDA1LCAyMDE4IDEsIEdlZXJ0IFV5
dHRlcmhvZXZlbiB3cm90ZToNCj4gPiBTby4uLi5JIGd1ZXNzIEkgZGlkbid0IHJlYWxseSBoYXZl
IGFuIGlzc3VlIGFmdGVyIGFsbC4NCj4gDQo+IFlvdSBkbyB3YW50IHRvOg0KPiAgIDEuIERvY3Vt
ZW50IHRoZSB0d28gcmVnaXN0ZXIgcmFuZ2VzIGluIHRoZSBEVCBiaW5kaW5ncywNCj4gICAyLiBV
cGRhdGUgdGhlIGRyaXZlciB0byBtYXAgYm90aCByYW5nZXMgb24gUlovQTIuDQoNClRoZSBkcml2
ZXIgZG9lcyBub3QgbmVlZCB0byBiZSB1cGRhdGVkLg0KDQpHaXZlbiB0aGlzIG1hcHBpbmc6DQoN
CgljcGc6IGNsb2NrLWNvbnRyb2xsZXJAZmNmZTAwMjAgew0KCQljb21wYXRpYmxlID0gInJlbmVz
YXMscjdzOTIxMC1jcGctbXNzciI7DQoJCXJlZyA9IDwweGZjZmUwMDEwIDB4NDU1PjsgIC8qIEZD
RkUwMDEwIC0gRkNGRTA0NjUgKi8NCgkJY2xvY2tzID0gPCZleHRhbF9jbGs+Ow0KCQljbG9jay1u
YW1lcyA9ICJleHRhbCI7DQoJCSNjbG9jay1jZWxscyA9IDwyPjsNCgkJI3Bvd2VyLWRvbWFpbi1j
ZWxscyA9IDwwPjsNCgkJI3Jlc2V0LWNlbGxzID0gPDE+Ow0KCX07DQoNClRoaXMgY292ZXJzIGFs
bCB0aGUgQ1BHLCBNU1RQLCBwb3dlciBkb3duIGFuZCByZXNldCByZWdpc3RlcnMuDQoNClNvIEkn
bSBqdXN0IGdvaW5nIHRvIG1hcCB0aGVtIGFsbCBhdCBvbmNlLg0KDQpBbmQsIHlvdSBjYW4gc2Vl
IHRoYXQgc29tZSBDUEcgYW5kIE1TVFAgcmVnaXN0ZXIgYXJlIG1peGVkIHRvZ2V0aGVyLg0KSGVy
ZSBpcyB0aGUgbGlzdCBvZiByZWdpc3RlcnMgaW4gYWRkcmVzcyBvcmRlcjoNCg0KIEZSUUNSICAg
RkNGRV8wMDEwICAoQ1BHKQ0KU1RCQ1IxICAgRkNGRV8wMDIwICAoTVNUUCkNClNUQkNSMiAgIEZD
RkVfMDAyNCAgKE1TVFApDQpDS0lPU0VMICBGQ0ZFXzAxMDAgIChDUEcpDQpTQ0xLU0VMICBGQ0ZF
XzAxMDQgIChDUEcpDQpTVEJDUjMgICBGQ0ZFXzA0MjAgIChNU1RQKQ0KU1RCQ1I0ICAgRkNGRV8w
NDI0ICAoTVNUUCkNClNUQkNSNSAgIEZDRkVfMDQyOCAgKE1TVFApDQpTVEJDUjYgICBGQ0ZFXzA0
MkMgIChNU1RQKQ0KU1RCQ1I3ICAgRkNGRV8wNDMwICAoTVNUUCkNClNUQkNSOCAgIEZDRkVfMDQz
NCAgKE1TVFApDQpTVEJDUjkgICBGQ0ZFXzA0MzggIChNU1RQKQ0KU1RCQ1IxMCAgRkNGRV8wNDND
ICAoTVNUUCkNClNXUlNUQ1IxIEZDRkVfMDQ2MCAgKFJFU0VUKQ0KU1dSU1RDUjIgRkNGRV8wNDY0
ICAoUkVTRVQpDQogIGV0Yy4uLg0KDQpTbyB0aGlzIHNob3VsZCBiZSBhbGwgb25lIG1lbW9yeSBi
bG9jayBpbiB0aGlzIGRyaXZlci4NCg0KV2hlbiB5b3UgZ2V0IHRvIEZDRkZfQzAwMCssIHRoYXQn
cyB3aGVyZSB0aGUgJ3NwZWNpYWx0eScgcmVnaXN0ZXJzIGFyZS4gDQpJIHRoaW5rIEZDRkZfQzAw
MCsgc2hvdWxkIGJlIGEgc2VwYXJhdGUgYmxvY2ssIG1heWJlIGluIGEgc2VwYXJhdGUgZHJpdmVy
Lg0KDQoNCkNocmlzDQoNCg==

Re: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Geert Uytterhoeven]

Hi Chris,

On Wed, Aug 29, 2018 at 3:29 PM Chris Brandt <chris.brandt@renesas.com> wrote:
> Add support for the R7S9210 (RZ/A2) Clock Pulse Generator and Module
> Standby.

Thanks for your patch!

> The Module Standby HW in the RZ/A series is very close to R-Car HW, except
> for how the registers are laid out.
> The MSTP registers are only 8-bits wide, there is no status registers
> (MSTPST), and the register offsets are a little different. Since the RZ/A

MSTPSR

> hardware manuals refer to these registers as the Standby Control Registers,
> we'll use that name to distinguish the RZ/A type for the R-Car type.

from the R-Car type.

> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

> --- /dev/null
> +++ b/drivers/clk/renesas/r7s9210-cpg-mssr.c
> @@ -0,0 +1,193 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * R7S9210 Clock Pulse Generator / Module Standby
> + *
> + * Based on r8a7795-cpg-mssr.c
> + *
> + * Copyright (C) 2018 Chris Brandt
> + * Copyright (C) 2018 Renesas Electronics Corp.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <dt-bindings/clock/r7s9210-cpg-mssr.h>
> +#include "renesas-cpg-mssr.h"
> +
> +#define CPG_FRQCR      0x00
> +#define CPG_CKIOSEL    0xF0
> +#define CPG_SCLKSEL    0xF4

The last two are unused?

> +
> +#define PORTL_PIDR     0xFCFFE074

Unused?

> +static u8 cpg_mode;
> +
> +/* Internal Clock ratio table */
> +static const unsigned int ratio_tab[5][5] = {
> +                       /* I,  G,  B, P1, P0 */

Use a struct instead?

struct {
        unsigned int i;
        unsigned int g;
        unsigned int ib
        unsigned int p1;
        unsigned int p0;
} ratio_tab[5] = { ... }

> +                       {  2,  4,  8, 16, 32 }, /* FRQCR = 0x012 */
> +                       {  4,  4,  8, 16, 32 }, /* FRQCR = 0x112 */
> +                       {  8,  4,  8, 16, 32 }, /* FRQCR = 0x212 */
> +                       { 16,  8, 16, 16, 32 }, /* FRQCR = 0x322 */
> +                       { 16, 16, 32, 32, 32 }, /* FRQCR = 0x333 */

The P0 divider is fixed to 32, so you can remove it from the table?

> +                       };
> +
> +enum rz_clk_types {
> +       CLK_TYPE_RZA_MAIN = CLK_TYPE_CUSTOM,
> +       CLK_TYPE_RZA_PLL,
> +};
> +
> +enum clk_ids {
> +       /* Core Clock Outputs exported to DT */
> +       LAST_DT_CORE_CLK = R7S9210_CLK_P0,
> +
> +       /* External Input Clocks */
> +       CLK_EXTAL,
> +
> +       /* Internal Core Clocks */
> +       CLK_MAIN,
> +       CLK_PLL,
> +       CLK_I,
> +       CLK_G,
> +       CLK_B,
> +       CLK_P1,
> +       CLK_P1C,
> +       CLK_P0,

The last six are not used and can be removed
(the driver uses R7S9210_CLK_* instead).

> +
> +       /* Module Clocks */
> +       MOD_CLK_BASE
> +};
> +
> +static struct cpg_core_clk r7s9210_core_clks[] = {
> +       /* External Clock Inputs */
> +       DEF_INPUT("extal",     CLK_EXTAL),
> +
> +       /* Internal Core Clocks */
> +       DEF_BASE(".main",       CLK_MAIN, CLK_TYPE_RZA_MAIN, CLK_EXTAL),
> +       DEF_BASE(".pll",       CLK_PLL, CLK_TYPE_RZA_PLL, CLK_MAIN),
> +
> +       /* Core Clock Outputs */
> +       DEF_FIXED("i",      R7S9210_CLK_I,     CLK_PLL,          2, 1),
> +       DEF_FIXED("g",      R7S9210_CLK_G,     CLK_PLL,          4, 1),
> +       DEF_FIXED("b",      R7S9210_CLK_B,     CLK_PLL,          8, 1),
> +       DEF_FIXED("p1",     R7S9210_CLK_P1,    CLK_PLL,         16, 1),
> +       DEF_FIXED("p1c",    R7S9210_CLK_P1C,   CLK_PLL,         16, 1),
> +       DEF_FIXED("p0",     R7S9210_CLK_P0,    CLK_PLL,         32, 1),
> +};
> +
> +static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = {
> +       DEF_MOD_STB("ostm0",     36,    R7S9210_CLK_P1C),
> +       DEF_MOD_STB("ostm1",     35,    R7S9210_CLK_P1C),
> +       DEF_MOD_STB("ostm2",     34,    R7S9210_CLK_P1C),

I think the table is easier to read if you sort by MSTP number.

> +
> +       DEF_MOD_STB("scif0",     47,    R7S9210_CLK_P1C),
> +       DEF_MOD_STB("scif1",     46,    R7S9210_CLK_P1C),
> +       DEF_MOD_STB("scif2",     45,    R7S9210_CLK_P1C),
> +       DEF_MOD_STB("scif3",     44,    R7S9210_CLK_P1C),
> +       DEF_MOD_STB("scif4",     43,    R7S9210_CLK_P1C),
> +
> +       DEF_MOD_STB("ether0",    65,    R7S9210_CLK_B),
> +       DEF_MOD_STB("ether1",    64,    R7S9210_CLK_B),
> +
> +       DEF_MOD_STB("i2c0",      87,    R7S9210_CLK_P1),
> +       DEF_MOD_STB("i2c1",      86,    R7S9210_CLK_P1),
> +       DEF_MOD_STB("i2c2",      85,    R7S9210_CLK_P1),
> +       DEF_MOD_STB("i2c3",      84,    R7S9210_CLK_P1),
> +
> +};
> +
> +struct clk * __init rza2_cpg_clk_register(struct device *dev,
> +       const struct cpg_core_clk *core, const struct cpg_mssr_info *info,
> +       struct clk **clks, void __iomem *base,
> +       struct raw_notifier_head *notifiers)
> +{
> +       const struct clk *parent;
> +       unsigned int mult = 1;
> +       unsigned int div = 1;
> +       u16 frqcr;
> +       u8 index;
> +       int i;
> +
> +       parent = clks[core->parent];
> +       if (IS_ERR(parent))
> +               return ERR_CAST(parent);
> +
> +       switch (core->id) {
> +       case CLK_MAIN:
> +               break;
> +
> +       case CLK_PLL:
> +               if (cpg_mode)
> +                       mult = 44;      /* Divider 1 is 1/2 */
> +               else
> +                       mult = 88;      /* Divider 1 is 1 */
> +               break;
> +
> +       default:
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       /* Adjust the dividers based on the current FRQCR setting */
> +       if (core->id == CLK_MAIN) {
> +
> +               /* If EXTAL is above 12MHz, then we know it is Mode 1 */
> +               if (clk_get_rate((struct clk *)parent) > 12000000)

Why the cast?

> +                       cpg_mode = 1;
> +
> +               frqcr = clk_readl(base + CPG_FRQCR) & 0xFFF;
> +               if (frqcr == 0x012)
> +                       index = 0;
> +               else if (frqcr == 0x112)
> +                       index = 1;
> +               else if (frqcr == 0x212)
> +                       index = 2;
> +               else if (frqcr == 0x322)
> +                       index = 3;
> +               else if (frqcr == 0x333)
> +                       index = 4;
> +               else
> +                       BUG_ON(1);      /* Illegal FRQCR value */
> +
> +               for (i = 0; i < ARRAY_SIZE(r7s9210_core_clks); i++) {
> +                       switch (r7s9210_core_clks[i].id) {
> +                       case R7S9210_CLK_I:
> +                               r7s9210_core_clks[i].div = ratio_tab[index][0];
> +                               break;
> +                       case R7S9210_CLK_G:
> +                               r7s9210_core_clks[i].div = ratio_tab[index][1];
> +                               break;
> +                       case R7S9210_CLK_B:
> +                               r7s9210_core_clks[i].div = ratio_tab[index][2];
> +                               break;
> +                       case R7S9210_CLK_P1:
> +                       case R7S9210_CLK_P1C:
> +                               r7s9210_core_clks[i].div = ratio_tab[index][3];
> +                               break;
> +                       case R7S9210_CLK_P0:
> +                               r7s9210_core_clks[i].div = ratio_tab[index][4];
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       return clk_register_fixed_factor(NULL, core->name,
> +                                        __clk_get_name(parent), 0, mult, div);
> +}
> +
> +const struct cpg_mssr_info r7s9210_cpg_mssr_info __initconst = {
> +       /* Core Clocks */
> +       .core_clks = r7s9210_core_clks,
> +       .num_core_clks = ARRAY_SIZE(r7s9210_core_clks),
> +       .last_dt_core_clk = LAST_DT_CORE_CLK,
> +       .num_total_core_clks = MOD_CLK_BASE,
> +
> +       /* Module Clocks */
> +       .mod_clks = r7s9210_mod_clks,
> +       .num_mod_clks = ARRAY_SIZE(r7s9210_mod_clks),
> +       .num_hw_mod_clks = 11 * 32, /* includes STBCR0/1/2 which don't exist */

According to the HW manual, STBCR1/2 do not exist?

> +
> +       /* Callbacks */
> +       .cpg_clk_register = rza2_cpg_clk_register,
> +
> +       /* RZ/A2 has Standby Control Registers */
> +       .stbyctrl = true,
> +};

> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> @@ -73,6 +73,17 @@ static const u16 smstpcr[] = {
>
>  #define        SMSTPCR(i)      smstpcr[i]
>
> +/*
> + * Standby Control Register offsets (RZ/A)
> + * Base address is FRQCR register
> + */
> +
> +static const u16 stbcr[] = {
> +       0x000, 0x000, 0x014, 0x410, 0x414, 0x418, 0x41C, 0x420,

stbcr[1] should be 0x010?

> +       0x424, 0x428, 0x42C, 0x430, 0x434, 0x460,

The last 3 don't exist?

> +};

> @@ -240,8 +270,14 @@ struct clk *cpg_mssr_clk_src_twocell_get(struct of_phandle_args *clkspec,
>
>         case CPG_MOD:
>                 type = "module";
> -               idx = MOD_CLK_PACK(clkidx);
> -               if (clkidx % 100 > 31 || idx >= priv->num_mod_clks) {
> +               if (priv->stbyctrl) {
> +                       idx = MOD_CLK_PACK_10(clkidx);
> +                       range_check = 7 - (clkidx % 10);
> +               } else {
> +                       idx = MOD_CLK_PACK_10(clkidx);

MOD_CLK_PACK()

> +                       range_check = 31 - (clkidx % 100);
> +               }
> +               if (range_check < 0 || idx >= priv->num_mod_clks) {
>                         dev_err(dev, "Invalid %s clock index %u\n", type,
>                                 clkidx);
>                         return ERR_PTR(-EINVAL);
> @@ -740,6 +776,12 @@ static const struct of_device_id cpg_mssr_match[] = {
>                 .compatible = "renesas,r8a77995-cpg-mssr",
>                 .data = &r8a77995_cpg_mssr_info,
>         },
> +#endif
> +#ifdef CONFIG_CLK_R7S9210
> +       {
> +               .compatible = "renesas,r7s9210-cpg-mssr",
> +               .data = &r7s9210_cpg_mssr_info,
> +       },

Please preserve sort order.

> --- a/drivers/clk/renesas/renesas-cpg-mssr.h
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.h
> @@ -78,6 +78,13 @@ struct mssr_mod_clk {
>  #define DEF_MOD(_name, _mod, _parent...)       \
>         { .name = _name, .id = MOD_CLK_ID(_mod), .parent = _parent }
>
> +/* Convert from sparse base-10 to packed index space */
> +#define MOD_CLK_PACK_10(x)     ((x / 10) * 32 + (x % 10))

"PACK" is a bit of a misnomer, as it no longer packs, but expands from base-10
to base-32  ;-)


> @@ -111,6 +122,7 @@ struct cpg_mssr_info {
>         unsigned int num_core_clks;
>         unsigned int last_dt_core_clk;
>         unsigned int num_total_core_clks;
> +       bool stbyctrl;
>
>         /* Module Clocks */
>         const struct mssr_mod_clk *mod_clks;
> @@ -149,6 +161,7 @@ extern const struct cpg_mssr_info r8a77970_cpg_mssr_info;
>  extern const struct cpg_mssr_info r8a77980_cpg_mssr_info;
>  extern const struct cpg_mssr_info r8a77990_cpg_mssr_info;
>  extern const struct cpg_mssr_info r8a77995_cpg_mssr_info;
> +extern const struct cpg_mssr_info r7s9210_cpg_mssr_info;

Please preserve sort order.

> --- /dev/null
> +++ b/include/dt-bindings/clock/r7s9210-cpg-mssr.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Renesas Electronics Corp.
> + *
> + */
> +
> +#ifndef __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__
> +#define __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__
> +
> +#include <dt-bindings/clock/renesas-cpg-mssr.h>
> +
> +/* R7S9210 CPG Core Clocks */
> +#define R7S9210_CLK_PLL                        0

Should that be an internal clock, not referred to from DT?
There's already the internal CLK_PLL clock.

> +#define R7S9210_CLK_I                  1
> +#define R7S9210_CLK_G                  2
> +#define R7S9210_CLK_B                  3
> +#define R7S9210_CLK_P1                 4
> +#define R7S9210_CLK_P1C                        5
> +#define R7S9210_CLK_P0                 6

The comment in Figure 6.1 suggests there's also P0C, but that may be a
mistake, as I can find no other references to it?

What about other clocks: BSCCLK, OCTCLK, HYPCLK, and SPICLK?

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 v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

Hi Geert,

Thanks for your review.


On Thursday, September 06, 2018, Geert Uytterhoeven wrote:
> > +#define CPG_FRQCR      0x00
> > +#define CPG_CKIOSEL    0xF0
> > +#define CPG_SCLKSEL    0xF4
> 
> The last two are unused?

In this driver they are not. I can remove them.

> > +
> > +#define PORTL_PIDR     0xFCFFE074
> 
> Unused?

Oops. That was left over from when I first was reading the port pin to 
find out the mode. But then I realize I could get the info from DT.

> 
> > +static u8 cpg_mode;
> > +
> > +/* Internal Clock ratio table */
> > +static const unsigned int ratio_tab[5][5] = {
> > +                       /* I,  G,  B, P1, P0 */
> 
> Use a struct instead?
> 
> struct {
>         unsigned int i;
>         unsigned int g;
>         unsigned int ib
>         unsigned int p1;
>         unsigned int p0;
> } ratio_tab[5] = { ... }

That's a good idea. Thanks.


> 
> > +                       {  2,  4,  8, 16, 32 }, /* FRQCR = 0x012 */
> > +                       {  4,  4,  8, 16, 32 }, /* FRQCR = 0x112 */
> > +                       {  8,  4,  8, 16, 32 }, /* FRQCR = 0x212 */
> > +                       { 16,  8, 16, 16, 32 }, /* FRQCR = 0x322 */
> > +                       { 16, 16, 32, 32, 32 }, /* FRQCR = 0x333 */
> 
> The P0 divider is fixed to 32, so you can remove it from the table?

OK, I can do that. I was just doing it to be consistent.


> > +       /* Internal Core Clocks */
> > +       CLK_MAIN,
> > +       CLK_PLL,
> > +       CLK_I,
> > +       CLK_G,
> > +       CLK_B,
> > +       CLK_P1,
> > +       CLK_P1C,
> > +       CLK_P0,
> 
> The last six are not used and can be removed
> (the driver uses R7S9210_CLK_* instead).

OK.


> > +static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = {
> > +       DEF_MOD_STB("ostm0",     36,    R7S9210_CLK_P1C),
> > +       DEF_MOD_STB("ostm1",     35,    R7S9210_CLK_P1C),
> > +       DEF_MOD_STB("ostm2",     34,    R7S9210_CLK_P1C),
> 
> I think the table is easier to read if you sort by MSTP number.

OK. I will switch them all around.

> > +       /* Adjust the dividers based on the current FRQCR setting */
> > +       if (core->id == CLK_MAIN) {
> > +
> > +               /* If EXTAL is above 12MHz, then we know it is Mode 1 */
> > +               if (clk_get_rate((struct clk *)parent) > 12000000)
> 
> Why the cast?

I was getting compiler warning because parent was const.

	const struct clk *parent;


../drivers/clk/renesas/r7s9210-cpg-mssr.c:144:20: warning: passing argument 1 of ‘clk_get_rate’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   if (clk_get_rate(parent) > 12000000)
                    ^
In file included from ../drivers/clk/renesas/r7s9210-cpg-mssr.c:12:0:
../include/linux/clk.h:463:15: note: expected ‘struct clk *’ but argument is of type ‘const struct clk *’
 unsigned long clk_get_rate(struct clk *clk);
               ^
make[1]: Leaving directory '/home/renesas/tools/upstream/renesas-drivers/.out_rza2m'

I can remove const, then I don't need the cast anymore.


> > +       /* Module Clocks */
> > +       .mod_clks = r7s9210_mod_clks,
> > +       .num_mod_clks = ARRAY_SIZE(r7s9210_mod_clks),
> > +       .num_hw_mod_clks = 11 * 32, /* includes STBCR0/1/2 which don't
> exist */
> 
> According to the HW manual, STBCR1/2 do not exist?

The problem is that there are registers named "STBCR1" and "STBCR2", but
they are not pure MSTP registers, and they sit at a different address 
location.

Would it be better if I say the MSTP registers start at STBCR3, and just
subtract "30" from the numbers in the device tree?

> > +static const u16 stbcr[] = {
> > +       0x000, 0x000, 0x014, 0x410, 0x414, 0x418, 0x41C, 0x420,
> 
> stbcr[1] should be 0x010?

Technically, stbcr[0] = 0x10, stbcr[1] = 0x14,
I was just thinking that I would never be access it anyway since they 
are not really MSTP registers.


> 
> > +       0x424, 0x428, 0x42C, 0x430, 0x434, 0x460,
> 
> The last 3 don't exist?

Opps, you're right! They are left over from when I change the table around.
Thanks.

> > +               } else {
> > +                       idx = MOD_CLK_PACK_10(clkidx);
> 
> MOD_CLK_PACK()

Good find! I would have broken existing drivers!


> > +#ifdef CONFIG_CLK_R7S9210
> > +       {
> > +               .compatible = "renesas,r7s9210-cpg-mssr",
> > +               .data = &r7s9210_cpg_mssr_info,
> > +       },
> 
> Please preserve sort order.

> >  extern const struct cpg_mssr_info r8a77980_cpg_mssr_info;
> >  extern const struct cpg_mssr_info r8a77990_cpg_mssr_info;
> >  extern const struct cpg_mssr_info r8a77995_cpg_mssr_info;
> > +extern const struct cpg_mssr_info r7s9210_cpg_mssr_info;
> 
> Please preserve sort order.

OK


> > +/* R7S9210 CPG Core Clocks */
> > +#define R7S9210_CLK_PLL                        0
> 
> Should that be an internal clock, not referred to from DT?
> There's already the internal CLK_PLL clock.

OK, I see what you mean. I will leave it out of this header file.


> > +#define R7S9210_CLK_I                  1
> > +#define R7S9210_CLK_G                  2
> > +#define R7S9210_CLK_B                  3
> > +#define R7S9210_CLK_P1                 4
> > +#define R7S9210_CLK_P1C                        5
> > +#define R7S9210_CLK_P0                 6
> 
> The comment in Figure 6.1 suggests there's also P0C, but that may be a
> mistake, as I can find no other references to it?

Funny, I didn't see that in there before.
Like you said, it's probably just a mistake.
I'll point that out to the device team.

> What about other clocks: BSCCLK, OCTCLK, HYPCLK, and SPICLK?

At this time, I'm considering them 'out of scope' from this driver as 
they really need to be set up early in device boot (ie, u-boot).
Unless you were just thinking of adding them as #defines, but never 
really using them.

Chris

RE: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

Hi Geert,

Thanks for your review.


On Thursday, September 06, 2018, Geert Uytterhoeven wrote:
> > +#define CPG_FRQCR      0x00
> > +#define CPG_CKIOSEL    0xF0
> > +#define CPG_SCLKSEL    0xF4
> 
> The last two are unused?

In this driver they are not. I can remove them.

> > +
> > +#define PORTL_PIDR     0xFCFFE074
> 
> Unused?

Oops. That was left over from when I first was reading the port pin to 
find out the mode. But then I realize I could get the info from DT.

> 
> > +static u8 cpg_mode;
> > +
> > +/* Internal Clock ratio table */
> > +static const unsigned int ratio_tab[5][5] = {
> > +                       /* I,  G,  B, P1, P0 */
> 
> Use a struct instead?
> 
> struct {
>         unsigned int i;
>         unsigned int g;
>         unsigned int ib
>         unsigned int p1;
>         unsigned int p0;
> } ratio_tab[5] = { ... }

That's a good idea. Thanks.


> 
> > +                       {  2,  4,  8, 16, 32 }, /* FRQCR = 0x012 */
> > +                       {  4,  4,  8, 16, 32 }, /* FRQCR = 0x112 */
> > +                       {  8,  4,  8, 16, 32 }, /* FRQCR = 0x212 */
> > +                       { 16,  8, 16, 16, 32 }, /* FRQCR = 0x322 */
> > +                       { 16, 16, 32, 32, 32 }, /* FRQCR = 0x333 */
> 
> The P0 divider is fixed to 32, so you can remove it from the table?

OK, I can do that. I was just doing it to be consistent.


> > +       /* Internal Core Clocks */
> > +       CLK_MAIN,
> > +       CLK_PLL,
> > +       CLK_I,
> > +       CLK_G,
> > +       CLK_B,
> > +       CLK_P1,
> > +       CLK_P1C,
> > +       CLK_P0,
> 
> The last six are not used and can be removed
> (the driver uses R7S9210_CLK_* instead).

OK.


> > +static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = {
> > +       DEF_MOD_STB("ostm0",     36,    R7S9210_CLK_P1C),
> > +       DEF_MOD_STB("ostm1",     35,    R7S9210_CLK_P1C),
> > +       DEF_MOD_STB("ostm2",     34,    R7S9210_CLK_P1C),
> 
> I think the table is easier to read if you sort by MSTP number.

OK. I will switch them all around.

> > +       /* Adjust the dividers based on the current FRQCR setting */
> > +       if (core->id == CLK_MAIN) {
> > +
> > +               /* If EXTAL is above 12MHz, then we know it is Mode 1 */
> > +               if (clk_get_rate((struct clk *)parent) > 12000000)
> 
> Why the cast?

I was getting compiler warning because parent was const.

	const struct clk *parent;


../drivers/clk/renesas/r7s9210-cpg-mssr.c:144:20: warning: passing argument 1 of ‘clk_get_rate’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   if (clk_get_rate(parent) > 12000000)
                    ^
In file included from ../drivers/clk/renesas/r7s9210-cpg-mssr.c:12:0:
../include/linux/clk.h:463:15: note: expected ‘struct clk *’ but argument is of type ‘const struct clk *’
 unsigned long clk_get_rate(struct clk *clk);
               ^
make[1]: Leaving directory '/home/renesas/tools/upstream/renesas-drivers/.out_rza2m'

I can remove const, then I don't need the cast anymore.


> > +       /* Module Clocks */
> > +       .mod_clks = r7s9210_mod_clks,
> > +       .num_mod_clks = ARRAY_SIZE(r7s9210_mod_clks),
> > +       .num_hw_mod_clks = 11 * 32, /* includes STBCR0/1/2 which don't
> exist */
> 
> According to the HW manual, STBCR1/2 do not exist?

The problem is that there are registers named "STBCR1" and "STBCR2", but
they are not pure MSTP registers, and they sit at a different address 
location.

Would it be better if I say the MSTP registers start at STBCR3, and just
subtract "30" from the numbers in the device tree?

> > +static const u16 stbcr[] = {
> > +       0x000, 0x000, 0x014, 0x410, 0x414, 0x418, 0x41C, 0x420,
> 
> stbcr[1] should be 0x010?

Technically, stbcr[0] = 0x10, stbcr[1] = 0x14,
I was just thinking that I would never be access it anyway since they 
are not really MSTP registers.


> 
> > +       0x424, 0x428, 0x42C, 0x430, 0x434, 0x460,
> 
> The last 3 don't exist?

Opps, you're right! They are left over from when I change the table around.
Thanks.

> > +               } else {
> > +                       idx = MOD_CLK_PACK_10(clkidx);
> 
> MOD_CLK_PACK()

Good find! I would have broken existing drivers!


> > +#ifdef CONFIG_CLK_R7S9210
> > +       {
> > +               .compatible = "renesas,r7s9210-cpg-mssr",
> > +               .data = &r7s9210_cpg_mssr_info,
> > +       },
> 
> Please preserve sort order.

> >  extern const struct cpg_mssr_info r8a77980_cpg_mssr_info;
> >  extern const struct cpg_mssr_info r8a77990_cpg_mssr_info;
> >  extern const struct cpg_mssr_info r8a77995_cpg_mssr_info;
> > +extern const struct cpg_mssr_info r7s9210_cpg_mssr_info;
> 
> Please preserve sort order.

OK


> > +/* R7S9210 CPG Core Clocks */
> > +#define R7S9210_CLK_PLL                        0
> 
> Should that be an internal clock, not referred to from DT?
> There's already the internal CLK_PLL clock.

OK, I see what you mean. I will leave it out of this header file.


> > +#define R7S9210_CLK_I                  1
> > +#define R7S9210_CLK_G                  2
> > +#define R7S9210_CLK_B                  3
> > +#define R7S9210_CLK_P1                 4
> > +#define R7S9210_CLK_P1C                        5
> > +#define R7S9210_CLK_P0                 6
> 
> The comment in Figure 6.1 suggests there's also P0C, but that may be a
> mistake, as I can find no other references to it?

Funny, I didn't see that in there before.
Like you said, it's probably just a mistake.
I'll point that out to the device team.

> What about other clocks: BSCCLK, OCTCLK, HYPCLK, and SPICLK?

At this time, I'm considering them 'out of scope' from this driver as 
they really need to be set up early in device boot (ie, u-boot).
Unless you were just thinking of adding them as #defines, but never 
really using them.

Chris

RE: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Chris Brandt]

SGkgR2VlcnQsDQoNClRoYW5rcyBmb3IgeW91ciByZXZpZXcuDQoNCg0KT24gVGh1cnNkYXksIFNl
cHRlbWJlciAwNiwgMjAxOCwgR2VlcnQgVXl0dGVyaG9ldmVuIHdyb3RlOg0KPiA+ICsjZGVmaW5l
IENQR19GUlFDUiAgICAgIDB4MDANCj4gPiArI2RlZmluZSBDUEdfQ0tJT1NFTCAgICAweEYwDQo+
ID4gKyNkZWZpbmUgQ1BHX1NDTEtTRUwgICAgMHhGNA0KPiANCj4gVGhlIGxhc3QgdHdvIGFyZSB1
bnVzZWQ/DQoNCkluIHRoaXMgZHJpdmVyIHRoZXkgYXJlIG5vdC4gSSBjYW4gcmVtb3ZlIHRoZW0u
DQoNCj4gPiArDQo+ID4gKyNkZWZpbmUgUE9SVExfUElEUiAgICAgMHhGQ0ZGRTA3NA0KPiANCj4g
VW51c2VkPw0KDQpPb3BzLiBUaGF0IHdhcyBsZWZ0IG92ZXIgZnJvbSB3aGVuIEkgZmlyc3Qgd2Fz
IHJlYWRpbmcgdGhlIHBvcnQgcGluIHRvIA0KZmluZCBvdXQgdGhlIG1vZGUuIEJ1dCB0aGVuIEkg
cmVhbGl6ZSBJIGNvdWxkIGdldCB0aGUgaW5mbyBmcm9tIERULg0KDQo+IA0KPiA+ICtzdGF0aWMg
dTggY3BnX21vZGU7DQo+ID4gKw0KPiA+ICsvKiBJbnRlcm5hbCBDbG9jayByYXRpbyB0YWJsZSAq
Lw0KPiA+ICtzdGF0aWMgY29uc3QgdW5zaWduZWQgaW50IHJhdGlvX3RhYls1XVs1XSA9IHsNCj4g
PiArICAgICAgICAgICAgICAgICAgICAgICAvKiBJLCAgRywgIEIsIFAxLCBQMCAqLw0KPiANCj4g
VXNlIGEgc3RydWN0IGluc3RlYWQ/DQo+IA0KPiBzdHJ1Y3Qgew0KPiAgICAgICAgIHVuc2lnbmVk
IGludCBpOw0KPiAgICAgICAgIHVuc2lnbmVkIGludCBnOw0KPiAgICAgICAgIHVuc2lnbmVkIGlu
dCBpYg0KPiAgICAgICAgIHVuc2lnbmVkIGludCBwMTsNCj4gICAgICAgICB1bnNpZ25lZCBpbnQg
cDA7DQo+IH0gcmF0aW9fdGFiWzVdID0geyAuLi4gfQ0KDQpUaGF0J3MgYSBnb29kIGlkZWEuIFRo
YW5rcy4NCg0KDQo+IA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgIHsgIDIsICA0LCAgOCwg
MTYsIDMyIH0sIC8qIEZSUUNSID0gMHgwMTIgKi8NCj4gPiArICAgICAgICAgICAgICAgICAgICAg
ICB7ICA0LCAgNCwgIDgsIDE2LCAzMiB9LCAvKiBGUlFDUiA9IDB4MTEyICovDQo+ID4gKyAgICAg
ICAgICAgICAgICAgICAgICAgeyAgOCwgIDQsICA4LCAxNiwgMzIgfSwgLyogRlJRQ1IgPSAweDIx
MiAqLw0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgIHsgMTYsICA4LCAxNiwgMTYsIDMyIH0s
IC8qIEZSUUNSID0gMHgzMjIgKi8NCj4gPiArICAgICAgICAgICAgICAgICAgICAgICB7IDE2LCAx
NiwgMzIsIDMyLCAzMiB9LCAvKiBGUlFDUiA9IDB4MzMzICovDQo+IA0KPiBUaGUgUDAgZGl2aWRl
ciBpcyBmaXhlZCB0byAzMiwgc28geW91IGNhbiByZW1vdmUgaXQgZnJvbSB0aGUgdGFibGU/DQoN
Ck9LLCBJIGNhbiBkbyB0aGF0LiBJIHdhcyBqdXN0IGRvaW5nIGl0IHRvIGJlIGNvbnNpc3RlbnQu
DQoNCg0KPiA+ICsgICAgICAgLyogSW50ZXJuYWwgQ29yZSBDbG9ja3MgKi8NCj4gPiArICAgICAg
IENMS19NQUlOLA0KPiA+ICsgICAgICAgQ0xLX1BMTCwNCj4gPiArICAgICAgIENMS19JLA0KPiA+
ICsgICAgICAgQ0xLX0csDQo+ID4gKyAgICAgICBDTEtfQiwNCj4gPiArICAgICAgIENMS19QMSwN
Cj4gPiArICAgICAgIENMS19QMUMsDQo+ID4gKyAgICAgICBDTEtfUDAsDQo+IA0KPiBUaGUgbGFz
dCBzaXggYXJlIG5vdCB1c2VkIGFuZCBjYW4gYmUgcmVtb3ZlZA0KPiAodGhlIGRyaXZlciB1c2Vz
IFI3UzkyMTBfQ0xLXyogaW5zdGVhZCkuDQoNCk9LLg0KDQoNCj4gPiArc3RhdGljIGNvbnN0IHN0
cnVjdCBtc3NyX21vZF9jbGsgcjdzOTIxMF9tb2RfY2xrc1tdIF9faW5pdGNvbnN0ID0gew0KPiA+
ICsgICAgICAgREVGX01PRF9TVEIoIm9zdG0wIiwgICAgIDM2LCAgICBSN1M5MjEwX0NMS19QMUMp
LA0KPiA+ICsgICAgICAgREVGX01PRF9TVEIoIm9zdG0xIiwgICAgIDM1LCAgICBSN1M5MjEwX0NM
S19QMUMpLA0KPiA+ICsgICAgICAgREVGX01PRF9TVEIoIm9zdG0yIiwgICAgIDM0LCAgICBSN1M5
MjEwX0NMS19QMUMpLA0KPiANCj4gSSB0aGluayB0aGUgdGFibGUgaXMgZWFzaWVyIHRvIHJlYWQg
aWYgeW91IHNvcnQgYnkgTVNUUCBudW1iZXIuDQoNCk9LLiBJIHdpbGwgc3dpdGNoIHRoZW0gYWxs
IGFyb3VuZC4NCg0KPiA+ICsgICAgICAgLyogQWRqdXN0IHRoZSBkaXZpZGVycyBiYXNlZCBvbiB0
aGUgY3VycmVudCBGUlFDUiBzZXR0aW5nICovDQo+ID4gKyAgICAgICBpZiAoY29yZS0+aWQgPT0g
Q0xLX01BSU4pIHsNCj4gPiArDQo+ID4gKyAgICAgICAgICAgICAgIC8qIElmIEVYVEFMIGlzIGFi
b3ZlIDEyTUh6LCB0aGVuIHdlIGtub3cgaXQgaXMgTW9kZSAxICovDQo+ID4gKyAgICAgICAgICAg
ICAgIGlmIChjbGtfZ2V0X3JhdGUoKHN0cnVjdCBjbGsgKilwYXJlbnQpID4gMTIwMDAwMDApDQo+
IA0KPiBXaHkgdGhlIGNhc3Q/DQoNCkkgd2FzIGdldHRpbmcgY29tcGlsZXIgd2FybmluZyBiZWNh
dXNlIHBhcmVudCB3YXMgY29uc3QuDQoNCgljb25zdCBzdHJ1Y3QgY2xrICpwYXJlbnQ7DQoNCg0K
Li4vZHJpdmVycy9jbGsvcmVuZXNhcy9yN3M5MjEwLWNwZy1tc3NyLmM6MTQ0OjIwOiB3YXJuaW5n
OiBwYXNzaW5nIGFyZ3VtZW50IDEgb2Yg4oCYY2xrX2dldF9yYXRl4oCZIGRpc2NhcmRzIOKAmGNv
bnN04oCZIHF1YWxpZmllciBmcm9tIHBvaW50ZXIgdGFyZ2V0IHR5cGUgWy1XZGlzY2FyZGVkLXF1
YWxpZmllcnNdDQogICBpZiAoY2xrX2dldF9yYXRlKHBhcmVudCkgPiAxMjAwMDAwMCkNCiAgICAg
ICAgICAgICAgICAgICAgXg0KSW4gZmlsZSBpbmNsdWRlZCBmcm9tIC4uL2RyaXZlcnMvY2xrL3Jl
bmVzYXMvcjdzOTIxMC1jcGctbXNzci5jOjEyOjA6DQouLi9pbmNsdWRlL2xpbnV4L2Nsay5oOjQ2
MzoxNTogbm90ZTogZXhwZWN0ZWQg4oCYc3RydWN0IGNsayAq4oCZIGJ1dCBhcmd1bWVudCBpcyBv
ZiB0eXBlIOKAmGNvbnN0IHN0cnVjdCBjbGsgKuKAmQ0KIHVuc2lnbmVkIGxvbmcgY2xrX2dldF9y
YXRlKHN0cnVjdCBjbGsgKmNsayk7DQogICAgICAgICAgICAgICBeDQptYWtlWzFdOiBMZWF2aW5n
IGRpcmVjdG9yeSAnL2hvbWUvcmVuZXNhcy90b29scy91cHN0cmVhbS9yZW5lc2FzLWRyaXZlcnMv
Lm91dF9yemEybScNCg0KSSBjYW4gcmVtb3ZlIGNvbnN0LCB0aGVuIEkgZG9uJ3QgbmVlZCB0aGUg
Y2FzdCBhbnltb3JlLg0KDQoNCj4gPiArICAgICAgIC8qIE1vZHVsZSBDbG9ja3MgKi8NCj4gPiAr
ICAgICAgIC5tb2RfY2xrcyA9IHI3czkyMTBfbW9kX2Nsa3MsDQo+ID4gKyAgICAgICAubnVtX21v
ZF9jbGtzID0gQVJSQVlfU0laRShyN3M5MjEwX21vZF9jbGtzKSwNCj4gPiArICAgICAgIC5udW1f
aHdfbW9kX2Nsa3MgPSAxMSAqIDMyLCAvKiBpbmNsdWRlcyBTVEJDUjAvMS8yIHdoaWNoIGRvbid0
DQo+IGV4aXN0ICovDQo+IA0KPiBBY2NvcmRpbmcgdG8gdGhlIEhXIG1hbnVhbCwgU1RCQ1IxLzIg
ZG8gbm90IGV4aXN0Pw0KDQpUaGUgcHJvYmxlbSBpcyB0aGF0IHRoZXJlIGFyZSByZWdpc3RlcnMg
bmFtZWQgIlNUQkNSMSIgYW5kICJTVEJDUjIiLCBidXQNCnRoZXkgYXJlIG5vdCBwdXJlIE1TVFAg
cmVnaXN0ZXJzLCBhbmQgdGhleSBzaXQgYXQgYSBkaWZmZXJlbnQgYWRkcmVzcyANCmxvY2F0aW9u
Lg0KDQpXb3VsZCBpdCBiZSBiZXR0ZXIgaWYgSSBzYXkgdGhlIE1TVFAgcmVnaXN0ZXJzIHN0YXJ0
IGF0IFNUQkNSMywgYW5kIGp1c3QNCnN1YnRyYWN0ICIzMCIgZnJvbSB0aGUgbnVtYmVycyBpbiB0
aGUgZGV2aWNlIHRyZWU/DQoNCj4gPiArc3RhdGljIGNvbnN0IHUxNiBzdGJjcltdID0gew0KPiA+
ICsgICAgICAgMHgwMDAsIDB4MDAwLCAweDAxNCwgMHg0MTAsIDB4NDE0LCAweDQxOCwgMHg0MUMs
IDB4NDIwLA0KPiANCj4gc3RiY3JbMV0gc2hvdWxkIGJlIDB4MDEwPw0KDQpUZWNobmljYWxseSwg
c3RiY3JbMF0gPSAweDEwLCBzdGJjclsxXSA9IDB4MTQsDQpJIHdhcyBqdXN0IHRoaW5raW5nIHRo
YXQgSSB3b3VsZCBuZXZlciBiZSBhY2Nlc3MgaXQgYW55d2F5IHNpbmNlIHRoZXkgDQphcmUgbm90
IHJlYWxseSBNU1RQIHJlZ2lzdGVycy4NCg0KDQo+IA0KPiA+ICsgICAgICAgMHg0MjQsIDB4NDI4
LCAweDQyQywgMHg0MzAsIDB4NDM0LCAweDQ2MCwNCj4gDQo+IFRoZSBsYXN0IDMgZG9uJ3QgZXhp
c3Q/DQoNCk9wcHMsIHlvdSdyZSByaWdodCEgVGhleSBhcmUgbGVmdCBvdmVyIGZyb20gd2hlbiBJ
IGNoYW5nZSB0aGUgdGFibGUgYXJvdW5kLg0KVGhhbmtzLg0KDQo+ID4gKyAgICAgICAgICAgICAg
IH0gZWxzZSB7DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgaWR4ID0gTU9EX0NMS19QQUNL
XzEwKGNsa2lkeCk7DQo+IA0KPiBNT0RfQ0xLX1BBQ0soKQ0KDQpHb29kIGZpbmQhIEkgd291bGQg
aGF2ZSBicm9rZW4gZXhpc3RpbmcgZHJpdmVycyENCg0KDQo+ID4gKyNpZmRlZiBDT05GSUdfQ0xL
X1I3UzkyMTANCj4gPiArICAgICAgIHsNCj4gPiArICAgICAgICAgICAgICAgLmNvbXBhdGlibGUg
PSAicmVuZXNhcyxyN3M5MjEwLWNwZy1tc3NyIiwNCj4gPiArICAgICAgICAgICAgICAgLmRhdGEg
PSAmcjdzOTIxMF9jcGdfbXNzcl9pbmZvLA0KPiA+ICsgICAgICAgfSwNCj4gDQo+IFBsZWFzZSBw
cmVzZXJ2ZSBzb3J0IG9yZGVyLg0KDQo+ID4gIGV4dGVybiBjb25zdCBzdHJ1Y3QgY3BnX21zc3Jf
aW5mbyByOGE3Nzk4MF9jcGdfbXNzcl9pbmZvOw0KPiA+ICBleHRlcm4gY29uc3Qgc3RydWN0IGNw
Z19tc3NyX2luZm8gcjhhNzc5OTBfY3BnX21zc3JfaW5mbzsNCj4gPiAgZXh0ZXJuIGNvbnN0IHN0
cnVjdCBjcGdfbXNzcl9pbmZvIHI4YTc3OTk1X2NwZ19tc3NyX2luZm87DQo+ID4gK2V4dGVybiBj
b25zdCBzdHJ1Y3QgY3BnX21zc3JfaW5mbyByN3M5MjEwX2NwZ19tc3NyX2luZm87DQo+IA0KPiBQ
bGVhc2UgcHJlc2VydmUgc29ydCBvcmRlci4NCg0KT0sNCg0KDQo+ID4gKy8qIFI3UzkyMTAgQ1BH
IENvcmUgQ2xvY2tzICovDQo+ID4gKyNkZWZpbmUgUjdTOTIxMF9DTEtfUExMICAgICAgICAgICAg
ICAgICAgICAgICAgMA0KPiANCj4gU2hvdWxkIHRoYXQgYmUgYW4gaW50ZXJuYWwgY2xvY2ssIG5v
dCByZWZlcnJlZCB0byBmcm9tIERUPw0KPiBUaGVyZSdzIGFscmVhZHkgdGhlIGludGVybmFsIENM
S19QTEwgY2xvY2suDQoNCk9LLCBJIHNlZSB3aGF0IHlvdSBtZWFuLiBJIHdpbGwgbGVhdmUgaXQg
b3V0IG9mIHRoaXMgaGVhZGVyIGZpbGUuDQoNCg0KPiA+ICsjZGVmaW5lIFI3UzkyMTBfQ0xLX0kg
ICAgICAgICAgICAgICAgICAxDQo+ID4gKyNkZWZpbmUgUjdTOTIxMF9DTEtfRyAgICAgICAgICAg
ICAgICAgIDINCj4gPiArI2RlZmluZSBSN1M5MjEwX0NMS19CICAgICAgICAgICAgICAgICAgMw0K
PiA+ICsjZGVmaW5lIFI3UzkyMTBfQ0xLX1AxICAgICAgICAgICAgICAgICA0DQo+ID4gKyNkZWZp
bmUgUjdTOTIxMF9DTEtfUDFDICAgICAgICAgICAgICAgICAgICAgICAgNQ0KPiA+ICsjZGVmaW5l
IFI3UzkyMTBfQ0xLX1AwICAgICAgICAgICAgICAgICA2DQo+IA0KPiBUaGUgY29tbWVudCBpbiBG
aWd1cmUgNi4xIHN1Z2dlc3RzIHRoZXJlJ3MgYWxzbyBQMEMsIGJ1dCB0aGF0IG1heSBiZSBhDQo+
IG1pc3Rha2UsIGFzIEkgY2FuIGZpbmQgbm8gb3RoZXIgcmVmZXJlbmNlcyB0byBpdD8NCg0KRnVu
bnksIEkgZGlkbid0IHNlZSB0aGF0IGluIHRoZXJlIGJlZm9yZS4NCkxpa2UgeW91IHNhaWQsIGl0
J3MgcHJvYmFibHkganVzdCBhIG1pc3Rha2UuDQpJJ2xsIHBvaW50IHRoYXQgb3V0IHRvIHRoZSBk
ZXZpY2UgdGVhbS4NCg0KPiBXaGF0IGFib3V0IG90aGVyIGNsb2NrczogQlNDQ0xLLCBPQ1RDTEss
IEhZUENMSywgYW5kIFNQSUNMSz8NCg0KQXQgdGhpcyB0aW1lLCBJJ20gY29uc2lkZXJpbmcgdGhl
bSAnb3V0IG9mIHNjb3BlJyBmcm9tIHRoaXMgZHJpdmVyIGFzIA0KdGhleSByZWFsbHkgbmVlZCB0
byBiZSBzZXQgdXAgZWFybHkgaW4gZGV2aWNlIGJvb3QgKGllLCB1LWJvb3QpLg0KVW5sZXNzIHlv
dSB3ZXJlIGp1c3QgdGhpbmtpbmcgb2YgYWRkaW5nIHRoZW0gYXMgI2RlZmluZXMsIGJ1dCBuZXZl
ciANCnJlYWxseSB1c2luZyB0aGVtLg0KDQpDaHJpcw0KDQo=

Re: [PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support

[Geert Uytterhoeven]

Hi Chris,

On Thu, Sep 6, 2018 at 4:31 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> > What about other clocks: BSCCLK, OCTCLK, HYPCLK, and SPICLK?
>
> At this time, I'm considering them 'out of scope' from this driver as
> they really need to be set up early in device boot (ie, u-boot).
> Unless you were just thinking of adding them as #defines, but never
> really using them.

OK, you can always add them later, when the need arises.
include/dt-bindings/clock/*.h is append-only.

Gr{oetje,eeting}s,

                        Geert

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

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