[PATCH 0/9] clk: add imx7ulp clk support

[PATCH 0/9] clk: add imx7ulp clk support

[Dong Aisheng]

This patch series intends to add imx7ulp clk support.

i.MX7ULP Clock functions are under joint control of the System
Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
modules, and Core Mode Controller (CMC)1 blocks

The clocking scheme provides clear separation between M4 domain
and A7 domain. Except for a few clock sources shared between two
domains, such as the System Oscillator clock, the Slow IRC (SIRC),
and and the Fast IRC clock (FIRCLK), clock sources and clock
management are separated and contained within each domain.

M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.

Note: this series only adds A7 clock domain support as M4 clock
domain will be handled by M4 seperately.

Dong Aisheng (9):
  clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support
  clk: reparent orphans after critical clocks enabled
  clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support
  clk: imx: add pllv4 support
  clk: imx: add pfdv2 support
  clk: imx: add composite clk support
  dt-bindings: clock: add imx7ulp clock binding doc
  clk: imx: make mux parent strings const
  clk: imx: add imx7ulp clk driver

 .../devicetree/bindings/clock/imx7ulp-clock.txt    |  62 +++++++
 drivers/clk/clk-divider.c                          |   2 +
 drivers/clk/clk-fractional-divider.c               |  10 ++
 drivers/clk/clk.c                                  |  39 ++--
 drivers/clk/imx/Makefile                           |   6 +-
 drivers/clk/imx/clk-busy.c                         |   2 +-
 drivers/clk/imx/clk-composite.c                    |  93 ++++++++++
 drivers/clk/imx/clk-fixup-mux.c                    |   2 +-
 drivers/clk/imx/clk-imx7ulp.c                      | 171 ++++++++++++++++++
 drivers/clk/imx/clk-pfdv2.c                        | 199 +++++++++++++++++++++
 drivers/clk/imx/clk-pllv4.c                        | 196 ++++++++++++++++++++
 drivers/clk/imx/clk.h                              |  31 +++-
 include/dt-bindings/clock/imx7ulp-clock.h          | 108 +++++++++++
 include/linux/clk-provider.h                       |  11 ++
 14 files changed, 902 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
 create mode 100644 drivers/clk/imx/clk-composite.c
 create mode 100644 drivers/clk/imx/clk-imx7ulp.c
 create mode 100644 drivers/clk/imx/clk-pfdv2.c
 create mode 100644 drivers/clk/imx/clk-pllv4.c
 create mode 100644 include/dt-bindings/clock/imx7ulp-clock.h

-- 
2.7.4

[PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support

[Dong Aisheng]

For dividers with zero indicating clock is disabled, instead of giving a
warning each time like "clkx: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not
set" in exist code, we'd consider it as a normal case and just return 0 in
divider_recalc_rate function. For such clocks users should be aware of
setting a correct rate before using.

e.g.
000b - Clock disabled
001b - Divide by 1
010b - Divide by 2
...

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/clk-divider.c    | 2 ++
 include/linux/clk-provider.h | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 96386ff..f78ba7a 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -125,6 +125,8 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
 
 	div = _get_div(table, val, flags, divider->width);
 	if (!div) {
+		if (flags & CLK_DIVIDER_ZERO_GATE)
+			return 0;
 		WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
 			"%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",
 			clk_hw_get_name(hw));
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index a428aec..a6efbb9 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -385,6 +385,9 @@ struct clk_div_table {
  * CLK_DIVIDER_MAX_AT_ZERO - For dividers which are like CLK_DIVIDER_ONE_BASED
  *	except when the value read from the register is zero, the divisor is
  *	2^width of the field.
+ * CLK_DIVIDER_ZERO_GATE - For dividers which are like CLK_DIVIDER_ONE_BASED
+ *	when the value read from the register is zero, it means the divisor
+ *	output is disabled and the rate calculated will be 0.
  */
 struct clk_divider {
 	struct clk_hw	hw;
@@ -405,6 +408,7 @@ struct clk_divider {
 #define CLK_DIVIDER_ROUND_CLOSEST	BIT(4)
 #define CLK_DIVIDER_READ_ONLY		BIT(5)
 #define CLK_DIVIDER_MAX_AT_ZERO		BIT(6)
+#define CLK_DIVIDER_ZERO_GATE		BIT(7)
 
 extern const struct clk_ops clk_divider_ops;
 extern const struct clk_ops clk_divider_ro_ops;
-- 
2.7.4

[PATCH 2/9] clk: reparent orphans after critical clocks enabled

[Dong Aisheng]

The orphan clocks reparent operation should be moved after the critical
clocks enabled, otherwise it may get a chance to disable a newly registered
critical clock which triggers the following warning.

[    0.000000] WARNING: CPU: 0 PID: 0 at drivers/clk/clk.c:597 clk_core_disable+0xb4/0xe0
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.11.0-rc1-00056-gdff1f66-dirty #1373
[    0.000000] Hardware name: Generic DT based system
[    0.000000] Backtrace:
[    0.000000] [<c010c4bc>] (dump_backtrace) from [<c010c764>] (show_stack+0x18/0x1c)
[    0.000000]  r6:600000d3 r5:00000000 r4:c0e26358 r3:00000000
[    0.000000] [<c010c74c>] (show_stack) from [<c040599c>] (dump_stack+0xb4/0xe8)
[    0.000000] [<c04058e8>] (dump_stack) from [<c0125c94>] (__warn+0xd8/0x104)
[    0.000000]  r10:c0c21cd0 r9:c048aa78 r8:00000255 r7:00000009 r6:c0c1cd90 r5:00000000
[    0.000000]  r4:00000000 r3:c0e01d34
[    0.000000] [<c0125bbc>] (__warn) from [<c0125d74>] (warn_slowpath_null+0x28/0x30)
[    0.000000]  r9:00000000 r8:ef00bf80 r7:c165ac4c r6:ef00bf80 r5:ef00bf80 r4:ef00bf80
[    0.000000] [<c0125d4c>] (warn_slowpath_null) from [<c048aa78>] (clk_core_disable+0xb4/0xe0)
[    0.000000] [<c048a9c4>] (clk_core_disable) from [<c048be88>] (clk_core_disable_lock+0x20/0x2c)
[    0.000000]  r4:000000d3 r3:c0e0af00
[    0.000000] [<c048be68>] (clk_core_disable_lock) from [<c048c224>] (clk_core_disable_unprepare+0x14/0x28)
[    0.000000]  r5:00000000 r4:ef00bf80
[    0.000000] [<c048c210>] (clk_core_disable_unprepare) from [<c048c270>] (__clk_set_parent_after+0x38/0x54)
[    0.000000]  r4:ef00bd80 r3:000010a0
[    0.000000] [<c048c238>] (__clk_set_parent_after) from [<c048daa8>] (clk_register+0x4d0/0x648)
[    0.000000]  r6:ef00d500 r5:ef00bf80 r4:ef00bd80 r3:ef00bfd4
[    0.000000] [<c048d5d8>] (clk_register) from [<c048dc30>] (clk_hw_register+0x10/0x1c)
[    0.000000]  r9:00000000 r8:00000003 r7:00000000 r6:00000824 r5:00000001 r4:ef00d500
[    0.000000] [<c048dc20>] (clk_hw_register) from [<c048e698>] (_register_divider+0xcc/0x120)
[    0.000000] [<c048e5cc>] (_register_divider) from [<c048e730>] (clk_register_divider+0x44/0x54)
[    0.000000]  r10:00000004 r9:00000003 r8:00000001 r7:00000000 r6:00000003 r5:00000001
[    0.000000]  r4:f0810030
[    0.000000] [<c048e6ec>] (clk_register_divider) from [<c0d3ff58>] (imx7ulp_clocks_init+0x558/0xe98)
[    0.000000]  r7:c0e296f8 r6:c165c808 r5:00000000 r4:c165c808
[    0.000000] [<c0d3fa00>] (imx7ulp_clocks_init) from [<c0d24db0>] (of_clk_init+0x118/0x1e0)
[    0.000000]  r10:00000001 r9:c0e01f68 r8:00000000 r7:c0e01f60 r6:ef7f8974 r5:ef0035c0
[    0.000000]  r4:00000006
[    0.000000] [<c0d24c98>] (of_clk_init) from [<c0d04a50>] (time_init+0x2c/0x38)
[    0.000000]  r10:efffed40 r9:c0d61a48 r8:c0e78000 r7:c0e07900 r6:ffffffff r5:c0e78000
[    0.000000]  r4:00000000
[    0.000000] [<c0d04a24>] (time_init) from [<c0d00b8c>] (start_kernel+0x218/0x394)
[    0.000000] [<c0d00974>] (start_kernel) from [<6000807c>] (0x6000807c)
[    0.000000]  r10:00000000 r9:410fc075 r8:6000406a r7:c0e0c930 r6:c0d61a44 r5:c0e07918
[    0.000000]  r4:c0e78294
[    0.000000] ---[ end trace 0000000000000000 ]---

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/clk.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fc58c52..e2955b1 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2471,25 +2471,6 @@ static int __clk_core_init(struct clk_core *core)
 	core->rate = core->req_rate = rate;
 
 	/*
-	 * walk the list of orphan clocks and reparent any that newly finds a
-	 * parent.
-	 */
-	hlist_for_each_entry_safe(orphan, tmp2, &clk_orphan_list, child_node) {
-		struct clk_core *parent = __clk_init_parent(orphan);
-
-		/*
-		 * we could call __clk_set_parent, but that would result in a
-		 * redundant call to the .set_rate op, if it exists
-		 */
-		if (parent) {
-			__clk_set_parent_before(orphan, parent);
-			__clk_set_parent_after(orphan, parent, NULL);
-			__clk_recalc_accuracies(orphan);
-			__clk_recalc_rates(orphan, 0);
-		}
-	}
-
-	/*
 	 * optional platform-specific magic
 	 *
 	 * The .init callback is not used by any of the basic clock types, but
@@ -2511,6 +2492,26 @@ static int __clk_core_init(struct clk_core *core)
 	}
 
 	kref_init(&core->ref);
+
+	/*
+	 * walk the list of orphan clocks and reparent any that newly finds a
+	 * parent.
+	 */
+	hlist_for_each_entry_safe(orphan, tmp2, &clk_orphan_list, child_node) {
+		struct clk_core *parent = __clk_init_parent(orphan);
+
+		/*
+		 * we could call __clk_set_parent, but that would result in a
+		 * redundant call to the .set_rate op, if it exists
+		 */
+		if (parent) {
+			__clk_set_parent_before(orphan, parent);
+			__clk_set_parent_after(orphan, parent, NULL);
+			__clk_recalc_accuracies(orphan);
+			__clk_recalc_rates(orphan, 0);
+		}
+	}
+
 out:
 	clk_prepare_unlock();
 
-- 
2.7.4

[PATCH 3/9] clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support

[Dong Aisheng]

Adding CLK_FRAC_DIVIDER_ZERO_BASED flag to indicate the numerator and
denominator value in register are start from 0.

This can be used to support frac dividers like below:
Divider output clock = Divider input clock x [(frac +1) / (div +1)]
where frac/div in register is:
000b - Divide by 1.
001b - Divide by 2.
010b - Divide by 3.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/clk-fractional-divider.c | 10 ++++++++++
 include/linux/clk-provider.h         |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index aab9046..455bec5 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -40,6 +40,11 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
 	m = (val & fd->mmask) >> fd->mshift;
 	n = (val & fd->nmask) >> fd->nshift;
 
+	if (fd->flags & CLK_FRAC_DIVIDER_ZERO_BASED) {
+		m++;
+		n++;
+	}
+
 	if (!n || !m)
 		return parent_rate;
 
@@ -91,6 +96,11 @@ static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate,
 			GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
 			&m, &n);
 
+	if (fd->flags & CLK_FRAC_DIVIDER_ZERO_BASED) {
+		m--;
+		n--;
+	}
+
 	if (fd->lock)
 		spin_lock_irqsave(fd->lock, flags);
 	else
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index a6efbb9..4466cae 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -557,6 +557,11 @@ void clk_hw_unregister_fixed_factor(struct clk_hw *hw);
  * @lock:	register lock
  *
  * Clock with adjustable fractional divider affecting its output frequency.
+ *
+ * Flags:
+ * CLK_FRAC_DIVIDER_ZERO_BASED - by default the divisor is the value read
+ *	from the register.  If CLK_FRAC_DIVIDER_ZERO_BASED is set then the
+ *	divider is the raw value read from the register plus one.
  */
 struct clk_fractional_divider {
 	struct clk_hw	hw;
@@ -573,6 +578,8 @@ struct clk_fractional_divider {
 
 #define to_clk_fd(_hw) container_of(_hw, struct clk_fractional_divider, hw)
 
+#define CLK_FRAC_DIVIDER_ZERO_BASED		BIT(0)
+
 extern const struct clk_ops clk_fractional_divider_ops;
 struct clk *clk_register_fractional_divider(struct device *dev,
 		const char *name, const char *parent_name, unsigned long flags,
-- 
2.7.4

[PATCH 4/9] clk: imx: add pllv4 support

[Dong Aisheng]

pllv4 is designed for System Clock Generation (SCG) module observed
in IMX ULP SoC series. e.g. i.MX7ULP.

The SCG modules generates clock used to derive processor, system,
peripheral bus and external memory interface clocks while this patch
intends to support the PLL part.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/Makefile    |   1 +
 drivers/clk/imx/clk-pllv4.c | 196 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/imx/clk.h       |   3 +
 3 files changed, 200 insertions(+)
 create mode 100644 drivers/clk/imx/clk-pllv4.c

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 1ada68a..1e308e2 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -10,6 +10,7 @@ obj-y += \
 	clk-pllv1.o \
 	clk-pllv2.o \
 	clk-pllv3.o \
+	clk-pllv4.o \
 	clk-pfd.o
 
 obj-$(CONFIG_SOC_IMX1)   += clk-imx1.o
diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
new file mode 100644
index 0000000..502da64
--- /dev/null
+++ b/drivers/clk/imx/clk-pllv4.c
@@ -0,0 +1,196 @@
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
+ *
+ * Author: Dong Aisheng <aisheng.dong@nxp.com>
+ *
+ * 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.
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+
+/* PLL Control Status Register (xPLLCSR) */
+#define PLL_CSR_OFFSET		0x0
+#define PLL_VLD			BIT(24)
+#define PLL_EN			BIT(0)
+
+/* PLL Configuration Register (xPLLCFG) */
+#define PLL_CFG_OFFSET		0x08
+#define BP_PLL_MULT		16
+#define BM_PLL_MULT		(0x7f << 16)
+
+/* PLL Numerator Register (xPLLNUM) */
+#define PLL_NUM_OFFSET		0x10
+
+/* PLL Denominator Register (xPLLDENOM) */
+#define PLL_DENOM_OFFSET	0x14
+
+struct clk_pllv4 {
+	struct clk_hw	hw;
+	void __iomem	*base;
+};
+
+/* Valid PLL MULT Table */
+static const int pllv4_mult_table[] = {33, 27, 22, 20, 17, 16};
+
+#define to_clk_pllv4(__hw) container_of(__hw, struct clk_pllv4, hw)
+
+static int clk_pllv4_wait_lock(struct clk_pllv4 *pll)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(5);
+
+	/* Wait for PLL to lock */
+	do {
+		if (readl_relaxed(pll->base  + PLL_CSR_OFFSET) & PLL_VLD)
+			break;
+		if (time_after(jiffies, timeout))
+			break;
+			usleep_range(50, 500);
+	} while (1);
+
+	return readl_relaxed(pll->base + PLL_CSR_OFFSET) & PLL_VLD ?
+	       0 : -ETIMEDOUT;
+}
+
+static int clk_pllv4_is_enabled(struct clk_hw *hw)
+{
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+
+	if (readl_relaxed(pll->base) & PLL_EN)
+		return 1;
+
+	return 0;
+}
+
+static unsigned long clk_pllv4_recalc_rate(struct clk_hw *hw,
+					   unsigned long parent_rate)
+{
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+	u32 div;
+
+	div = (readl_relaxed(pll->base + PLL_CFG_OFFSET)
+	       & BM_PLL_MULT) >> BP_PLL_MULT;
+
+	return parent_rate * div;
+}
+
+static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *prate)
+{
+	unsigned long parent_rate = *prate;
+	unsigned long round_rate, i;
+
+	for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
+		round_rate = parent_rate * pllv4_mult_table[i];
+		if (rate >= round_rate)
+			return round_rate;
+	}
+
+	return round_rate;
+}
+
+static bool clk_pllv4_is_valid_mult(unsigned int mult)
+{
+	int i;
+
+	/* check if mult is in valid MULT table */
+	for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
+		if (pllv4_mult_table[i] == mult)
+			return true;
+	}
+
+	return false;
+}
+
+static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate,
+			      unsigned long parent_rate)
+{
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+	u32 val, mult;
+
+	if (clk_pllv4_is_enabled(hw)) {
+		WARN(1, "clk_pllv4: can't change rate when pll is enabled");
+		return -EINVAL;
+	}
+
+	mult = rate / parent_rate;
+
+	if (clk_pllv4_is_valid_mult(mult))
+		return -EINVAL;
+
+	val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
+	val &= ~BM_PLL_MULT;
+	val |= mult << BP_PLL_MULT;
+	writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
+
+	return 0;
+}
+
+static int clk_pllv4_enable(struct clk_hw *hw)
+{
+	u32 val;
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+
+	val = readl_relaxed(pll->base);
+	val |= PLL_EN;
+	writel_relaxed(val, pll->base);
+
+	return clk_pllv4_wait_lock(pll);
+}
+
+static void clk_pllv4_disable(struct clk_hw *hw)
+{
+	u32 val;
+	struct clk_pllv4 *pll = to_clk_pllv4(hw);
+
+	val = readl_relaxed(pll->base);
+	val &= ~PLL_EN;
+	writel_relaxed(val, pll->base);
+}
+
+static const struct clk_ops clk_pllv4_ops = {
+	.recalc_rate	= clk_pllv4_recalc_rate,
+	.round_rate	= clk_pllv4_round_rate,
+	.set_rate	= clk_pllv4_set_rate,
+	.enable		= clk_pllv4_enable,
+	.disable	= clk_pllv4_disable,
+	.is_enabled	= clk_pllv4_is_enabled,
+};
+
+struct clk *imx_clk_pllv4(const char *name, const char *parent_name,
+			  void __iomem *base)
+{
+	struct clk_pllv4 *pll;
+	struct clk *clk;
+	struct clk_init_data init;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	pll->base = base;
+
+	init.name = name;
+	init.ops = &clk_pllv4_ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+	init.flags = CLK_SET_RATE_GATE;
+
+	pll->hw.init = &init;
+
+	clk = clk_register(NULL, &pll->hw);
+	if (IS_ERR(clk))
+		kfree(pll);
+
+	return clk;
+}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index e1f5e42..f599732 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -40,6 +40,9 @@ enum imx_pllv3_type {
 struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 		const char *parent_name, void __iomem *base, u32 div_mask);
 
+struct clk *imx_clk_pllv4(const char *name, const char *parent_name,
+			  void __iomem *base);
+
 struct clk *clk_register_gate2(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 bit_idx, u8 cgr_val,
-- 
2.7.4

[PATCH 5/9] clk: imx: add pfdv2 support

[Dong Aisheng]

The pfdv2 is designed for PLL Fractional Divide (PFD) observed in System
Clock Generation (SCG) module in IMX ULP SoC series. e.g. i.MX7ULP.

NOTE pfdv2 can only be operated when clk is gated.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/Makefile    |   3 +-
 drivers/clk/imx/clk-pfdv2.c | 199 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/imx/clk.h       |   3 +
 3 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/imx/clk-pfdv2.c

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 1e308e2..ea4ef27 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -11,7 +11,8 @@ obj-y += \
 	clk-pllv2.o \
 	clk-pllv3.o \
 	clk-pllv4.o \
-	clk-pfd.o
+	clk-pfd.o \
+	clk-pfdv2.o
 
 obj-$(CONFIG_SOC_IMX1)   += clk-imx1.o
 obj-$(CONFIG_SOC_IMX21)  += clk-imx21.o
diff --git a/drivers/clk/imx/clk-pfdv2.c b/drivers/clk/imx/clk-pfdv2.c
new file mode 100644
index 0000000..e5c4ad7
--- /dev/null
+++ b/drivers/clk/imx/clk-pfdv2.c
@@ -0,0 +1,199 @@
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
+ *
+ * Author: Dong Aisheng <aisheng.dong@nxp.com>
+ *
+ * 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.
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+
+/**
+ * struct clk_pfdv2 - IMX PFD clock
+ * @clk_hw:	clock source
+ * @reg:	PFD register address
+ * @gate_bit:	Gate bit offset
+ * @vld_bit:	Valid bit offset
+ * @frac_off:	PLL Fractional Divider offset
+ */
+
+struct clk_pfdv2 {
+	struct clk_hw	hw;
+	void __iomem	*reg;
+	u8		gate_bit;
+	u8		vld_bit;
+	u8		frac_off;
+};
+
+#define to_clk_pfdv2(_hw) container_of(_hw, struct clk_pfdv2, hw)
+
+#define CLK_PFDV2_FRAC_MASK 0x3f
+
+static int clk_pfdv2_wait(struct clk_pfdv2 *pfd)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(10);
+
+	while (!(readl_relaxed(pfd->reg) & pfd->vld_bit))
+		if (time_after(jiffies, timeout))
+			return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int clk_pfdv2_enable(struct clk_hw *hw)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+	u32 val;
+
+	val = readl_relaxed(pfd->reg);
+	val &= ~pfd->gate_bit;
+	writel_relaxed(val, pfd->reg);
+
+	return clk_pfdv2_wait(pfd);
+}
+
+static void clk_pfdv2_disable(struct clk_hw *hw)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+	u32 val;
+
+	val = readl_relaxed(pfd->reg);
+	val |= pfd->gate_bit;
+	writel_relaxed(val, pfd->reg);
+}
+
+static unsigned long clk_pfdv2_recalc_rate(struct clk_hw *hw,
+					   unsigned long parent_rate)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+	u64 tmp = parent_rate;
+	u8 frac;
+
+	frac = (readl_relaxed(pfd->reg) >> pfd->frac_off)
+		& CLK_PFDV2_FRAC_MASK;
+
+	if (!frac) {
+		pr_debug("clk_pfdv2: %s invalid pfd frac value 0\n",
+			 clk_hw_get_name(hw));
+		return 0;
+	}
+
+	tmp *= 18;
+	do_div(tmp, frac);
+
+	return tmp;
+}
+
+static long clk_pfdv2_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *prate)
+{
+	u64 tmp = *prate;
+	u8 frac;
+
+	tmp = tmp * 18 + rate / 2;
+	do_div(tmp, rate);
+	frac = tmp;
+
+	if (frac < 12)
+		frac = 12;
+	else if (frac > 35)
+		frac = 35;
+
+	tmp = *prate;
+	tmp *= 18;
+	do_div(tmp, frac);
+
+	return tmp;
+}
+
+static int clk_pfdv2_is_enabled(struct clk_hw *hw)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+
+	if (readl_relaxed(pfd->reg) & pfd->gate_bit)
+		return 0;
+
+	return 1;
+}
+
+static int clk_pfdv2_set_rate(struct clk_hw *hw, unsigned long rate,
+			      unsigned long parent_rate)
+{
+	struct clk_pfdv2 *pfd = to_clk_pfdv2(hw);
+	u64 tmp = parent_rate;
+	u32 val;
+	u8 frac;
+
+	if (clk_pfdv2_is_enabled(hw)) {
+		WARN(1, "clk_pfdv2: can't change rate when pfd is enabled");
+		return -EINVAL;
+	}
+
+	tmp = tmp * 18 + rate / 2;
+	do_div(tmp, rate);
+	frac = tmp;
+	if (frac < 12)
+		frac = 12;
+	else if (frac > 35)
+		frac = 35;
+
+	val = readl_relaxed(pfd->reg);
+	val &= ~(CLK_PFDV2_FRAC_MASK << pfd->frac_off);
+	val |= frac << pfd->frac_off;
+	writel_relaxed(val, pfd->reg);
+
+	return 0;
+}
+
+static const struct clk_ops clk_pfdv2_ops = {
+	.enable		= clk_pfdv2_enable,
+	.disable	= clk_pfdv2_disable,
+	.recalc_rate	= clk_pfdv2_recalc_rate,
+	.round_rate	= clk_pfdv2_round_rate,
+	.set_rate	= clk_pfdv2_set_rate,
+	.is_enabled     = clk_pfdv2_is_enabled,
+};
+
+struct clk *imx_clk_pfdv2(const char *name, const char *parent_name,
+			void __iomem *reg, u8 idx)
+{
+	struct clk_pfdv2 *pfd;
+	struct clk *clk;
+	struct clk_init_data init;
+
+	WARN_ON(idx > 3);
+
+	pfd = kzalloc(sizeof(*pfd), GFP_KERNEL);
+	if (!pfd)
+		return ERR_PTR(-ENOMEM);
+
+	pfd->reg = reg;
+	pfd->gate_bit = 1 << ((idx + 1) * 8 - 1);
+	pfd->vld_bit = pfd->gate_bit - 1;
+	pfd->frac_off = idx * 8;
+
+	init.name = name;
+	init.ops = &clk_pfdv2_ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+	init.flags = CLK_SET_RATE_GATE;
+
+	pfd->hw.init = &init;
+
+	clk = clk_register(NULL, &pfd->hw);
+	if (IS_ERR(clk))
+		kfree(pfd);
+
+	return clk;
+}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index f599732..51b84e4 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -58,6 +58,9 @@ struct clk *imx_clk_gate_exclusive(const char *name, const char *parent,
 struct clk *imx_clk_pfd(const char *name, const char *parent_name,
 		void __iomem *reg, u8 idx);
 
+struct clk *imx_clk_pfdv2(const char *name, const char *parent_name,
+			  void __iomem *reg, u8 idx);
+
 struct clk *imx_clk_busy_divider(const char *name, const char *parent_name,
 				 void __iomem *reg, u8 shift, u8 width,
 				 void __iomem *busy_reg, u8 busy_shift);
-- 
2.7.4

[PATCH 6/9] clk: imx: add composite clk support

[Dong Aisheng]

The imx composite clk is designed for Peripheral Clock Control (PCC)
module observed in IMX ULP SoC series. e.g. i.MX7ULP.

NOTE pcc can only be operated when clk is gated.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/Makefile        |  1 +
 drivers/clk/imx/clk-composite.c | 93 +++++++++++++++++++++++++++++++++++++++++
 drivers/clk/imx/clk.h           |  7 +++-
 3 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/imx/clk-composite.c

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index ea4ef27..bf001ce 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -3,6 +3,7 @@ obj-y += \
 	clk.o \
 	clk-busy.o \
 	clk-cpu.o \
+	clk-composite.o \
 	clk-fixup-div.o \
 	clk-fixup-mux.o \
 	clk-gate-exclusive.o \
diff --git a/drivers/clk/imx/clk-composite.c b/drivers/clk/imx/clk-composite.c
new file mode 100644
index 0000000..c440827
--- /dev/null
+++ b/drivers/clk/imx/clk-composite.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+
+#include "clk.h"
+
+#define PCG_PCS_SHIFT	24
+#define PCG_PCS_MASK	0x7
+#define PCG_CGC_SHIFT	30
+#define PCG_FRAC_SHIFT	3
+#define PCG_FRAC_WIDTH	1
+#define PCG_FRAC_MASK	BIT(3)
+#define PCG_PCD_SHIFT	0
+#define PCG_PCD_WIDTH	3
+#define PCG_PCD_MASK	0x7
+
+struct clk *imx_clk_composite(const char *name,
+			      const char * const *parent_names,
+			      int num_parents, bool mux_present,
+			      bool rate_present, bool gate_present,
+			      void __iomem *reg)
+{
+	struct clk_hw *mux_hw = NULL, *fd_hw = NULL, *gate_hw = NULL;
+	struct clk_fractional_divider *fd = NULL;
+	struct clk_gate *gate = NULL;
+	struct clk_mux *mux = NULL;
+	struct clk *clk;
+
+	if (mux_present) {
+		mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+		if (!mux)
+			return ERR_PTR(-ENOMEM);
+		mux_hw = &mux->hw;
+		mux->reg = reg;
+		mux->shift = PCG_PCS_SHIFT;
+		mux->mask = PCG_PCS_MASK;
+	}
+
+	if (rate_present) {
+		fd = kzalloc(sizeof(*fd), GFP_KERNEL);
+		if (!fd) {
+			kfree(mux);
+			return ERR_PTR(-ENOMEM);
+		}
+		fd_hw = &fd->hw;
+		fd->reg = reg;
+		fd->mshift = PCG_FRAC_SHIFT;
+		fd->mwidth = PCG_FRAC_WIDTH;
+		fd->mmask  = PCG_FRAC_MASK;
+		fd->nshift = PCG_PCD_SHIFT;
+		fd->nwidth = PCG_PCD_WIDTH;
+		fd->nmask = PCG_PCD_MASK;
+		fd->flags = CLK_FRAC_DIVIDER_ZERO_BASED;
+	}
+
+	if (gate_present) {
+		gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+		if (!gate) {
+			kfree(mux);
+			kfree(fd);
+			return ERR_PTR(-ENOMEM);
+		}
+		gate_hw = &gate->hw;
+		gate->reg = reg;
+		gate->bit_idx = PCG_CGC_SHIFT;
+	}
+
+	clk = clk_register_composite(NULL, name, parent_names, num_parents,
+				     mux_hw, &clk_mux_ops, fd_hw,
+				     &clk_fractional_divider_ops, gate_hw,
+				     &clk_gate_ops, CLK_SET_RATE_GATE |
+				     CLK_SET_PARENT_GATE);
+	if (IS_ERR(clk)) {
+		kfree(mux);
+		kfree(fd);
+		kfree(gate);
+	}
+
+	return clk;
+}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 51b84e4..665777e 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -69,6 +69,12 @@ struct clk *imx_clk_busy_mux(const char *name, void __iomem *reg, u8 shift,
 			     u8 width, void __iomem *busy_reg, u8 busy_shift,
 			     const char **parent_names, int num_parents);
 
+struct clk *imx_clk_composite(const char *name,
+			      const char * const *parent_names,
+			      int num_parents, bool mux_present,
+			      bool rate_present, bool gate_present,
+			      void __iomem *reg);
+
 struct clk *imx_clk_fixup_divider(const char *name, const char *parent,
 				  void __iomem *reg, u8 shift, u8 width,
 				  void (*fixup)(u32 *val));
@@ -209,5 +215,4 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
 struct clk *imx_clk_cpu(const char *name, const char *parent_name,
 		struct clk *div, struct clk *mux, struct clk *pll,
 		struct clk *step);
-
 #endif
-- 
2.7.4

[PATCH 7/9] dt-bindings: clock: add imx7ulp clock binding doc

[Dong Aisheng]

i.MX7ULP Clock functions are under joint control of the System
Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
modules, and Core Mode Controller (CMC)1 blocks

Note IMX7ULP has two clock domains: M4 and A7. This binding doc
is only for A7 clock domain.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 .../devicetree/bindings/clock/imx7ulp-clock.txt    |  62 ++++++++++++
 include/dt-bindings/clock/imx7ulp-clock.h          | 108 +++++++++++++++++++++
 2 files changed, 170 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
 create mode 100644 include/dt-bindings/clock/imx7ulp-clock.h

diff --git a/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
new file mode 100644
index 0000000..76ea3c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
@@ -0,0 +1,62 @@
+* Clock bindings for Freescale i.MX7ULP
+
+i.MX7ULP Clock functions are under joint control of the System
+Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
+modules, and Core Mode Controller (CMC)1 blocks
+
+The clocking scheme provides clear separation between M4 domain
+and A7 domain. Except for a few clock sources shared between two
+domains, such as the System Oscillator clock, the Slow IRC (SIRC),
+and and the Fast IRC clock (FIRCLK), clock sources and clock
+management are separated and contained within each domain.
+
+M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
+A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
+
+Note: this binding doc is only for A7 clock domain.
+
+Required properties:
+
+- compatible:	Should be "fsl,imx7ulp-clock".
+- reg : 	Should contain registers location and length for scg1,
+		pcc2 and pcc3.
+- reg-names:	Should contain the according reg names "scg1", "pcc2"
+		and "pcc3".
+- #clock-cells:	Should be <1>.
+- clocks:	Should contain the fixed input clocks.
+- clock-name:   Should contain the following clock names:"rsoc", "sosc",
+		"sirc", "firc", "upll", "mpll".
+
+The clock consumer should specify the desired clock by having the clock
+ID in its "clocks" phandle cell.
+See include/dt-bindings/clock/imx7ulp-clock.h
+for the full list of i.MX7ULP clock IDs.
+
+Examples:
+
+#include <dt-bindings/clock/imx7ulp-clock.h>
+
+clks: scg1@403e0000 {
+	compatible = "fsl,imx7ulp-clock";
+	reg = <0x403e0000 0x10000>
+	      <0x403f0000 0x10000>
+	      <0x40b30000 0x10000>;
+	reg-names = "scg1", "pcc2", "pcc3";
+	clocks = <&rsoc>, <&sosc>, <&sirc>,
+		 <&firc>, <&upll>, <&mpll>;
+	clock-names = "rsoc", "sosc", "sirc",
+		      "firc", "upll", "mpll";
+	#clock-cells = <1>;
+};
+
+usdhc1: usdhc@40380000 {
+	compatible = "fsl,imx7ulp-usdhc";
+	reg = <0x40380000 0x10000>;
+	interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&clks IMX7ULP_CLK_NIC1_BUS_DIV>,
+		 <&clks IMX7ULP_CLK_NIC1_DIV>,
+		 <&clks IMX7ULP_CLK_USDHC1>;
+	clock-names ="ipg", "ahb", "per";
+	bus-width = <4>;
+	status = "disabled";
+};
diff --git a/include/dt-bindings/clock/imx7ulp-clock.h b/include/dt-bindings/clock/imx7ulp-clock.h
new file mode 100644
index 0000000..2556106
--- /dev/null
+++ b/include/dt-bindings/clock/imx7ulp-clock.h
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_IMX7ULP_H
+#define __DT_BINDINGS_CLOCK_IMX7ULP_H
+
+#define IMX7ULP_CLK_DUMMY		0
+#define IMX7ULP_CLK_ROSC		1
+#define IMX7ULP_CLK_SOSC		2
+#define IMX7ULP_CLK_FIRC		3
+
+/* SCG1 */
+#define IMX7ULP_CLK_SPLL_PRE_SEL	4
+#define IMX7ULP_CLK_SPLL_PRE_DIV	5
+#define IMX7ULP_CLK_SPLL		6
+#define IMX7ULP_CLK_SPLL_POST_DIV1	7
+#define IMX7ULP_CLK_SPLL_POST_DIV2	8
+#define IMX7ULP_CLK_SPLL_PFD0		9
+#define IMX7ULP_CLK_SPLL_PFD1		10
+#define IMX7ULP_CLK_SPLL_PFD2		11
+#define IMX7ULP_CLK_SPLL_PFD3		12
+#define IMX7ULP_CLK_SPLL_PFD_SEL	13
+#define IMX7ULP_CLK_SPLL_SEL		14
+#define IMX7ULP_CLK_APLL_PRE_SEL	15
+#define IMX7ULP_CLK_APLL_PRE_DIV	16
+#define IMX7ULP_CLK_APLL		17
+#define IMX7ULP_CLK_APLL_POST_DIV1	18
+#define IMX7ULP_CLK_APLL_POST_DIV2	19
+#define IMX7ULP_CLK_APLL_PFD0		20
+#define IMX7ULP_CLK_APLL_PFD1		21
+#define IMX7ULP_CLK_APLL_PFD2		22
+#define IMX7ULP_CLK_APLL_PFD3		23
+#define IMX7ULP_CLK_APLL_PFD_SEL	24
+#define IMX7ULP_CLK_APLL_SEL		25
+#define IMX7ULP_CLK_UPLL		26
+#define IMX7ULP_CLK_SYS_SEL		27
+#define IMX7ULP_CLK_CORE_DIV		28
+#define IMX7ULP_CLK_BUS_DIV		29
+#define IMX7ULP_CLK_PLAT_DIV		30
+#define IMX7ULP_CLK_DDR_SEL		31
+#define IMX7ULP_CLK_DDR_DIV		32
+#define IMX7ULP_CLK_NIC_SEL		33
+#define IMX7ULP_CLK_NIC0_DIV		34
+#define IMX7ULP_CLK_GPU_DIV		35
+#define IMX7ULP_CLK_NIC1_DIV		36
+#define IMX7ULP_CLK_NIC1_BUS_DIV	37
+#define IMX7ULP_CLK_NIC1_EXT_DIV	38
+
+/* PCG2 */
+#define IMX7ULP_CLK_DMA1		39
+#define IMX7ULP_CLK_RGPIO2P1		40
+#define IMX7ULP_CLK_FLEXBUS		41
+#define IMX7ULP_CLK_SEMA42_1		42
+#define IMX7ULP_CLK_DMA_MUX1		43
+#define IMX7ULP_CLK_SNVS		44
+#define IMX7ULP_CLK_CAAM		45
+#define IMX7ULP_CLK_LPTPM4		46
+#define IMX7ULP_CLK_LPTPM5		47
+#define IMX7ULP_CLK_LPIT1		48
+#define IMX7ULP_CLK_LPSPI2		49
+#define IMX7ULP_CLK_LPSPI3		50
+#define IMX7ULP_CLK_LPI2C4		51
+#define IMX7ULP_CLK_LPI2C5		52
+#define IMX7ULP_CLK_LPUART4		53
+#define IMX7ULP_CLK_LPUART5		54
+#define IMX7ULP_CLK_FLEXIO1		55
+#define IMX7ULP_CLK_USB0		56
+#define IMX7ULP_CLK_USB1		57
+#define IMX7ULP_CLK_USB_PHY		58
+#define IMX7ULP_CLK_USB_PL301		59
+#define IMX7ULP_CLK_USDHC0		60
+#define IMX7ULP_CLK_USDHC1		61
+#define IMX7ULP_CLK_WDG1		62
+#define IMX7ULP_CLK_WDG2		63
+
+/* PCG3 */
+#define IMX7ULP_CLK_LPTPM6		64
+#define IMX7ULP_CLK_LPTPM7		65
+#define IMX7ULP_CLK_LPI2C6		66
+#define IMX7ULP_CLK_LPI2C7		67
+#define IMX7ULP_CLK_LPUART6		68
+#define IMX7ULP_CLK_LPUART7		69
+#define IMX7ULP_CLK_VIU			70
+#define IMX7ULP_CLK_DSI			71
+#define IMX7ULP_CLK_LCDIF		72
+#define IMX7ULP_CLK_MMDC		73
+#define IMX7ULP_CLK_PCTLC		74
+#define IMX7ULP_CLK_PCTLD		75
+#define IMX7ULP_CLK_PCTLE		76
+#define IMX7ULP_CLK_PCTLF		77
+#define IMX7ULP_CLK_GPU3D		78
+#define IMX7ULP_CLK_GPU2D		79
+#define IMX7ULP_CLK_MIPI_PLL		80
+#define IMX7ULP_CLK_SIRC		81
+#define IMX7ULP_CLK_SOSC_BUS_CLK	82
+#define IMX7ULP_CLK_FIRC_BUS_CLK	83
+#define IMX7ULP_CLK_SPLL_BUS_CLK	84
+
+#define IMX7ULP_CLK_END			85
+
+#endif /* __DT_BINDINGS_CLOCK_IMX7ULP_H */
-- 
2.7.4

[PATCH 8/9] clk: imx: make mux parent strings const

[Dong Aisheng]

As the commit 2893c379461a ("clk: make strings in parent name arrays
const"), let's make the parent strings const, otherwise we may meet
the following warning when compiling:

drivers/clk/imx/clk-imx7ulp.c: In function 'imx7ulp_clocks_init':
drivers/clk/imx/clk-imx7ulp.c:73:35: warning: passing argument 5 of
	'imx_clk_mux_flags' discards 'const' qualifier from pointer target type

  clks[IMX7ULP_CLK_APLL_PRE_SEL] = imx_clk_mux_flags("apll_pre_sel", base + 0x508, 0,
	1, pll_pre_sels, ARRAY_SIZE(pll_pre_sels), CLK_SET_PARENT_GATE);
                                   ^
In file included from drivers/clk/imx/clk-imx7ulp.c:23:0:
drivers/clk/imx/clk.h:200:27: note: expected 'const char **' but argument is
 of type 'const char * const*'
...

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-busy.c      |  2 +-
 drivers/clk/imx/clk-fixup-mux.c |  2 +-
 drivers/clk/imx/clk.h           | 18 +++++++++++-------
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/imx/clk-busy.c b/drivers/clk/imx/clk-busy.c
index 5cc9959..097625c 100644
--- a/drivers/clk/imx/clk-busy.c
+++ b/drivers/clk/imx/clk-busy.c
@@ -154,7 +154,7 @@ static struct clk_ops clk_busy_mux_ops = {
 
 struct clk *imx_clk_busy_mux(const char *name, void __iomem *reg, u8 shift,
 			     u8 width, void __iomem *busy_reg, u8 busy_shift,
-			     const char **parent_names, int num_parents)
+			     const char * const *parent_names, int num_parents)
 {
 	struct clk_busy_mux *busy;
 	struct clk *clk;
diff --git a/drivers/clk/imx/clk-fixup-mux.c b/drivers/clk/imx/clk-fixup-mux.c
index c9b327e..44817c1 100644
--- a/drivers/clk/imx/clk-fixup-mux.c
+++ b/drivers/clk/imx/clk-fixup-mux.c
@@ -70,7 +70,7 @@ static const struct clk_ops clk_fixup_mux_ops = {
 };
 
 struct clk *imx_clk_fixup_mux(const char *name, void __iomem *reg,
-			      u8 shift, u8 width, const char **parents,
+			      u8 shift, u8 width, const char * const *parents,
 			      int num_parents, void (*fixup)(u32 *val))
 {
 	struct clk_fixup_mux *fixup_mux;
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 665777e..d09fa0c 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -67,7 +67,7 @@ struct clk *imx_clk_busy_divider(const char *name, const char *parent_name,
 
 struct clk *imx_clk_busy_mux(const char *name, void __iomem *reg, u8 shift,
 			     u8 width, void __iomem *busy_reg, u8 busy_shift,
-			     const char **parent_names, int num_parents);
+			     const char * const *parent_names, int num_parents);
 
 struct clk *imx_clk_composite(const char *name,
 			      const char * const *parent_names,
@@ -80,7 +80,7 @@ struct clk *imx_clk_fixup_divider(const char *name, const char *parent,
 				  void (*fixup)(u32 *val));
 
 struct clk *imx_clk_fixup_mux(const char *name, void __iomem *reg,
-			      u8 shift, u8 width, const char **parents,
+			      u8 shift, u8 width, const char * const *parents,
 			      int num_parents, void (*fixup)(u32 *val));
 
 static inline struct clk *imx_clk_fixed(const char *name, int rate)
@@ -89,7 +89,8 @@ static inline struct clk *imx_clk_fixed(const char *name, int rate)
 }
 
 static inline struct clk *imx_clk_mux_ldb(const char *name, void __iomem *reg,
-		u8 shift, u8 width, const char **parents, int num_parents)
+			u8 shift, u8 width, const char * const *parents,
+			int num_parents)
 {
 	return clk_register_mux(NULL, name, parents, num_parents,
 			CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT, reg,
@@ -188,7 +189,8 @@ static inline struct clk *imx_clk_gate4(const char *name, const char *parent,
 }
 
 static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg,
-		u8 shift, u8 width, const char **parents, int num_parents)
+			u8 shift, u8 width, const char * const *parents,
+			int num_parents)
 {
 	return clk_register_mux(NULL, name, parents, num_parents,
 			CLK_SET_RATE_NO_REPARENT, reg, shift,
@@ -196,7 +198,8 @@ static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg,
 }
 
 static inline struct clk *imx_clk_mux2(const char *name, void __iomem *reg,
-		u8 shift, u8 width, const char **parents, int num_parents)
+			u8 shift, u8 width, const char * const *parents,
+			int num_parents)
 {
 	return clk_register_mux(NULL, name, parents, num_parents,
 			CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
@@ -204,8 +207,9 @@ static inline struct clk *imx_clk_mux2(const char *name, void __iomem *reg,
 }
 
 static inline struct clk *imx_clk_mux_flags(const char *name,
-		void __iomem *reg, u8 shift, u8 width, const char **parents,
-		int num_parents, unsigned long flags)
+			void __iomem *reg, u8 shift, u8 width,
+			const char * const *parents, int num_parents,
+			unsigned long flags)
 {
 	return clk_register_mux(NULL, name, parents, num_parents,
 			flags | CLK_SET_RATE_NO_REPARENT, reg, shift, width, 0,
-- 
2.7.4

[PATCH 9/9] clk: imx: add imx7ulp clk driver

[Dong Aisheng]

i.MX7ULP Clock functions are under joint control of the System
Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
modules, and Core Mode Controller (CMC)1 blocks

The clocking scheme provides clear separation between M4 domain
and A7 domain. Except for a few clock sources shared between two
domains, such as the System Oscillator clock, the Slow IRC (SIRC),
and and the Fast IRC clock (FIRCLK), clock sources and clock
management are separated and contained within each domain.

M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.

This driver only adds clock support in A7 domain.

Note that most clocks required to be operated when gated, e.g. pll,
pfd, pcc. And more special cases that scs/ddr/nic mux selecting
different clock source requires that clock to be enabled first,
then we need set CLK_OPS_PARENT_ENABLE flag for them properly.

Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Anson Huang <Anson.Huang@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/Makefile      |   1 +
 drivers/clk/imx/clk-imx7ulp.c | 171 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 172 insertions(+)
 create mode 100644 drivers/clk/imx/clk-imx7ulp.c

diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index bf001ce..6f013e0 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -27,4 +27,5 @@ obj-$(CONFIG_SOC_IMX6SL) += clk-imx6sl.o
 obj-$(CONFIG_SOC_IMX6SX) += clk-imx6sx.o
 obj-$(CONFIG_SOC_IMX6UL) += clk-imx6ul.o
 obj-$(CONFIG_SOC_IMX7D)  += clk-imx7d.o
+obj-$(CONFIG_SOC_IMX7ULP) += clk-imx7ulp.o
 obj-$(CONFIG_SOC_VF610)  += clk-vf610.o
diff --git a/drivers/clk/imx/clk-imx7ulp.c b/drivers/clk/imx/clk-imx7ulp.c
new file mode 100644
index 0000000..de229ba
--- /dev/null
+++ b/drivers/clk/imx/clk-imx7ulp.c
@@ -0,0 +1,171 @@
+/*
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Copyright 2017 NXP
+ *
+ * Author: Dong Aisheng <aisheng.dong@nxp.com>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <dt-bindings/clock/imx7ulp-clock.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include "clk.h"
+
+static const char * const pll_pre_sels[]	= { "sosc", "firc", };
+static const char * const spll_pfd_sels[]	= { "spll_pfd0", "spll_pfd1", "spll_pfd2", "spll_pfd3", };
+static const char * const spll_sels[]		= { "spll", "spll_pfd_sel", };
+static const char * const apll_pfd_sels[]	= { "apll_pfd0", "apll_pfd1", "apll_pfd2", "apll_pfd3", };
+static const char * const apll_sels[]		= { "apll", "apll_pfd_sel", };
+static const char * const scs_sels[]		= { "dummy", "sosc", "sirc", "firc", "dummy", "apll_sel", "spll_sel", "upll", };
+static const char * const ddr_sels[]		= { "apll_pfd_sel", "upll", };
+static const char * const nic_sels[]		= { "firc", "ddr_clk", };
+static const char * const periph_plat_sels[]	= { "dummy", "nic1_bus_clk", "nic1_clk", "ddr_clk", "apll_pfd2", "apll_pfd1", "apll_pfd0", "upll", };
+static const char * const periph_bus_sels[]	= { "dummy", "sosc_bus_clk", "mpll", "firc_bus_clk", "rosc", "nic1_bus_clk", "nic1_clk", "spll_bus_clk", };
+
+static struct clk *clks[IMX7ULP_CLK_END];
+static struct clk_onecell_data clk_data;
+
+static void __init imx7ulp_clocks_init(struct device_node *scg_node)
+{
+	struct device_node *np;
+	void __iomem *base;
+
+	clks[IMX7ULP_CLK_DUMMY]		= imx_clk_fixed("dummy", 0);
+
+	clks[IMX7ULP_CLK_ROSC]		= of_clk_get_by_name(scg_node, "rosc");
+	clks[IMX7ULP_CLK_SOSC]		= of_clk_get_by_name(scg_node, "sosc");
+	clks[IMX7ULP_CLK_SIRC]		= of_clk_get_by_name(scg_node, "sirc");
+	clks[IMX7ULP_CLK_FIRC]		= of_clk_get_by_name(scg_node, "firc");
+	clks[IMX7ULP_CLK_MIPI_PLL]	= of_clk_get_by_name(scg_node, "mpll");
+	clks[IMX7ULP_CLK_UPLL]		= of_clk_get_by_name(scg_node, "upll");
+
+	np = scg_node;
+	base = of_io_request_and_map(np, 0, "scg1");
+	WARN_ON(!base);
+
+	/* NOTE: xPLL config can't be changed when xPLL is enabled */
+	clks[IMX7ULP_CLK_APLL_PRE_SEL]	= imx_clk_mux_flags("apll_pre_sel", base + 0x508, 0, 1, pll_pre_sels, ARRAY_SIZE(pll_pre_sels), CLK_SET_PARENT_GATE);
+	clks[IMX7ULP_CLK_SPLL_PRE_SEL]	= imx_clk_mux_flags("spll_pre_sel", base + 0x608, 0, 1, pll_pre_sels, ARRAY_SIZE(pll_pre_sels), CLK_SET_PARENT_GATE);
+
+	/*							name		parent_name	reg		shift	width	flags */
+	clks[IMX7ULP_CLK_APLL_PRE_DIV]	= imx_clk_divider_flags("apll_pre_div", "apll_pre_sel", base + 0x508,	8,	3,	CLK_SET_RATE_GATE);
+	clks[IMX7ULP_CLK_SPLL_PRE_DIV]	= imx_clk_divider_flags("spll_pre_div", "spll_pre_sel", base + 0x608,	8,	3,	CLK_SET_RATE_GATE);
+
+	/*						name	 parent_name	 base */
+	clks[IMX7ULP_CLK_APLL]		= imx_clk_pllv4("apll",  "apll_pre_div", base + 0x500);
+	clks[IMX7ULP_CLK_SPLL]		= imx_clk_pllv4("spll",  "spll_pre_div", base + 0x600);
+
+	/* APLL PFDs */
+	clks[IMX7ULP_CLK_APLL_PFD0]	= imx_clk_pfdv2("apll_pfd0", "apll", base + 0x50C, 0);
+	clks[IMX7ULP_CLK_APLL_PFD1]	= imx_clk_pfdv2("apll_pfd1", "apll", base + 0x50C, 1);
+	clks[IMX7ULP_CLK_APLL_PFD2]	= imx_clk_pfdv2("apll_pfd2", "apll", base + 0x50C, 2);
+	clks[IMX7ULP_CLK_APLL_PFD3]	= imx_clk_pfdv2("apll_pfd3", "apll", base + 0x50C, 3);
+
+	/* SPLL PFDs */
+	clks[IMX7ULP_CLK_SPLL_PFD0]	= imx_clk_pfdv2("spll_pfd0", "spll", base + 0x60C, 0);
+	clks[IMX7ULP_CLK_SPLL_PFD1]	= imx_clk_pfdv2("spll_pfd1", "spll", base + 0x60C, 1);
+	clks[IMX7ULP_CLK_SPLL_PFD2]	= imx_clk_pfdv2("spll_pfd2", "spll", base + 0x60C, 2);
+	clks[IMX7ULP_CLK_SPLL_PFD3]	= imx_clk_pfdv2("spll_pfd3", "spll", base + 0x60C, 3);
+
+	/* PLL Mux */
+	clks[IMX7ULP_CLK_APLL_PFD_SEL]	= imx_clk_mux_flags("apll_pfd_sel", base + 0x508, 14, 2, apll_pfd_sels, ARRAY_SIZE(apll_pfd_sels), CLK_SET_RATE_PARENT | CLK_SET_PARENT_GATE);
+	clks[IMX7ULP_CLK_SPLL_PFD_SEL]	= imx_clk_mux_flags("spll_pfd_sel", base + 0x608, 14, 2, spll_pfd_sels, ARRAY_SIZE(spll_pfd_sels), CLK_SET_RATE_PARENT | CLK_SET_PARENT_GATE);
+	clks[IMX7ULP_CLK_APLL_SEL]	= imx_clk_mux_flags("apll_sel", base + 0x508, 1, 1, apll_sels, ARRAY_SIZE(apll_sels), CLK_SET_RATE_PARENT | CLK_SET_PARENT_GATE);
+	clks[IMX7ULP_CLK_SPLL_SEL]	= imx_clk_mux_flags("spll_sel", base + 0x608, 1, 1, spll_sels, ARRAY_SIZE(spll_sels), CLK_SET_RATE_PARENT | CLK_SET_PARENT_GATE);
+
+	clks[IMX7ULP_CLK_SPLL_BUS_CLK]	= clk_register_divider(NULL, "spll_bus_clk", "spll_sel", CLK_SET_RATE_GATE, base + 0x604, 8, 3, CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ZERO_GATE, &imx_ccm_lock);
+
+	/* scs/ddr/nic select different clock source requires that clock to be enabled first */
+	clks[IMX7ULP_CLK_SYS_SEL]	= imx_clk_mux2("scs_sel", base + 0x14, 24, 4, scs_sels, ARRAY_SIZE(scs_sels));
+	clks[IMX7ULP_CLK_NIC_SEL]	= imx_clk_mux2("nic_sel", base + 0x40, 28, 1, nic_sels, ARRAY_SIZE(nic_sels));
+	clks[IMX7ULP_CLK_DDR_SEL]	= imx_clk_mux_flags("ddr_sel", base + 0x30, 24, 1, ddr_sels, ARRAY_SIZE(ddr_sels), CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE);
+
+	clks[IMX7ULP_CLK_CORE_DIV]	= imx_clk_divider_flags("divcore",	"scs_sel",  base + 0x14, 16, 4, CLK_SET_RATE_PARENT | CLK_IS_CRITICAL);
+
+	clks[IMX7ULP_CLK_DDR_DIV]	= clk_register_divider(NULL, "ddr_clk", "ddr_sel", CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, base + 0x30, 0, 3,
+							       CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ZERO_GATE, &imx_ccm_lock);
+
+	clks[IMX7ULP_CLK_NIC0_DIV]	= imx_clk_divider_flags("nic0_clk",	"nic_sel",  base + 0x40, 24, 4, CLK_SET_RATE_PARENT | CLK_IS_CRITICAL);
+	clks[IMX7ULP_CLK_NIC1_DIV]	= imx_clk_divider_flags("nic1_clk",	"nic0_clk", base + 0x40, 16, 4, CLK_SET_RATE_PARENT | CLK_IS_CRITICAL);
+	clks[IMX7ULP_CLK_NIC1_BUS_DIV]	= imx_clk_divider_flags("nic1_bus_clk",	"nic1_clk", base + 0x40, 4,  4, CLK_SET_RATE_PARENT | CLK_IS_CRITICAL);
+
+	clks[IMX7ULP_CLK_GPU_DIV] 	= imx_clk_divider("gpu_clk", "nic0_clk", base + 0x40, 20, 4);
+
+	clks[IMX7ULP_CLK_SOSC_BUS_CLK]	= clk_register_divider(NULL, "sosc_bus_clk", "sosc", 0, base + 0x104, 8, 3,
+							       CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ZERO_GATE, &imx_ccm_lock);
+	clks[IMX7ULP_CLK_FIRC_BUS_CLK]	= clk_register_divider(NULL, "firc_bus_clk", "firc", 0, base + 0x304, 8, 3,
+							       CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ZERO_GATE, &imx_ccm_lock);
+
+	/* PCC2 */
+	base = of_io_request_and_map(np, 1, "pcc2");
+	WARN_ON(!base);
+
+	clks[IMX7ULP_CLK_DMA1]		= imx_clk_gate("dma1", "nic1_clk", base + 0x20, 30);
+	clks[IMX7ULP_CLK_RGPIO2P1]	= imx_clk_gate("rgpio2p1", "nic1_bus_clk", base + 0x3c, 30);
+	clks[IMX7ULP_CLK_DMA_MUX1]	= imx_clk_gate("dma_mux1", "nic1_bus_clk", base + 0x84, 30);
+	clks[IMX7ULP_CLK_SNVS]		= imx_clk_gate("snvs", "nic1_bus_clk", base + 0x8c, 30);
+	clks[IMX7ULP_CLK_CAAM]		= imx_clk_gate("caam", "nic1_clk", base + 0x90, 30);
+	clks[IMX7ULP_CLK_LPTPM4]	= imx_clk_composite("lptpm4",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x94);
+	clks[IMX7ULP_CLK_LPTPM5]	= imx_clk_composite("lptmp5",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x98);
+	clks[IMX7ULP_CLK_LPIT1]		= imx_clk_composite("lpit1",   periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x9C);
+	clks[IMX7ULP_CLK_LPSPI2]	= imx_clk_composite("lpspi2",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xA4);
+	clks[IMX7ULP_CLK_LPSPI3]	= imx_clk_composite("lpspi3",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xA8);
+	clks[IMX7ULP_CLK_LPI2C4]	= imx_clk_composite("lpi2c4",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xAC);
+	clks[IMX7ULP_CLK_LPI2C5]	= imx_clk_composite("lpi2c5",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xB0);
+	clks[IMX7ULP_CLK_LPUART4]	= imx_clk_composite("lpuart4", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xB4);
+	clks[IMX7ULP_CLK_LPUART5]	= imx_clk_composite("lpuart5", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xB8);
+	clks[IMX7ULP_CLK_FLEXIO1]	= imx_clk_composite("flexio1", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0xC4);
+	clks[IMX7ULP_CLK_USB0]		= imx_clk_composite("usb0",    periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xCC);
+	clks[IMX7ULP_CLK_USB1]		= imx_clk_composite("usb1",    periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xD0);
+	clks[IMX7ULP_CLK_USB_PHY]	= imx_clk_gate("usb_phy", "nic1_bus_clk", base + 0xD4, 30);
+	clks[IMX7ULP_CLK_USDHC0]	= imx_clk_composite("usdhc0",  periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xDC);
+	clks[IMX7ULP_CLK_USDHC1]	= imx_clk_composite("usdhc1",  periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xE0);
+	clks[IMX7ULP_CLK_WDG1]		= imx_clk_composite("wdg1",    periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, true,  true, base + 0xF4);
+	clks[IMX7ULP_CLK_WDG2]		= imx_clk_composite("sdg2",    periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, true,  true, base + 0x10C);
+
+	/* PCC3 */
+	base = of_io_request_and_map(np, 2, "pcc3");
+	WARN_ON(!base);
+
+	clks[IMX7ULP_CLK_LPTPM6]	= imx_clk_composite("lptpm6",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x84);
+	clks[IMX7ULP_CLK_LPTPM7]	= imx_clk_composite("lptpm7",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x88);
+	clks[IMX7ULP_CLK_LPI2C6]	= imx_clk_composite("lpi2c6",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x90);
+	clks[IMX7ULP_CLK_LPI2C7]	= imx_clk_composite("lpi2c7",  periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x94);
+	clks[IMX7ULP_CLK_LPUART6]	= imx_clk_composite("lpuart6", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x98);
+	clks[IMX7ULP_CLK_LPUART7]	= imx_clk_composite("lpuart7", periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, false, true, base + 0x9C);
+	clks[IMX7ULP_CLK_DSI]		= imx_clk_composite("dsi",     periph_bus_sels, ARRAY_SIZE(periph_bus_sels), true, true,  true, base + 0xA4);
+	clks[IMX7ULP_CLK_LCDIF]		= imx_clk_composite("lcdif",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, true,  true, base + 0xA8);
+
+	clks[IMX7ULP_CLK_MMDC]		= clk_register_gate(NULL, "mmdc", "nic1_clk", CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
+							    base + 0xAC, 30, 0, &imx_ccm_lock);
+
+	clks[IMX7ULP_CLK_VIU]		= imx_clk_gate("viu",	"nic1_clk",	base + 0xA0, 30);
+	clks[IMX7ULP_CLK_PCTLC]		= imx_clk_gate("pctlc", "nic1_bus_clk", base + 0xB8, 30);
+	clks[IMX7ULP_CLK_PCTLD]		= imx_clk_gate("pctld", "nic1_bus_clk", base + 0xBC, 30);
+	clks[IMX7ULP_CLK_PCTLE]		= imx_clk_gate("pctle", "nic1_bus_clk", base + 0xc0, 30);
+	clks[IMX7ULP_CLK_PCTLF]		= imx_clk_gate("pctlf", "nic1_bus_clk", base + 0xc4, 30);
+
+	clks[IMX7ULP_CLK_GPU3D]		= imx_clk_composite("gpu3d",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x140);
+	clks[IMX7ULP_CLK_GPU2D]		= imx_clk_composite("gpu2d",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x144);
+
+	imx_check_clocks(clks, ARRAY_SIZE(clks));
+
+	clk_data.clks = clks;
+	clk_data.clk_num = ARRAY_SIZE(clks);
+	of_clk_add_provider(scg_node, of_clk_src_onecell_get, &clk_data);
+
+	pr_info("i.MX7ULP clock tree init done.\n");
+}
+
+CLK_OF_DECLARE(imx7ulp, "fsl,imx7ulp-clock", imx7ulp_clocks_init);
-- 
2.7.4

Re: [PATCH 0/9] clk: add imx7ulp clk support

[Dong Aisheng]

On Mon, May 15, 2017 at 09:59:14PM +0800, Dong Aisheng wrote:
> This patch series intends to add imx7ulp clk support.
> 
> i.MX7ULP Clock functions are under joint control of the System
> Clock Generation (SCG) modules, Peripheral Clock Control (PCC)
> modules, and Core Mode Controller (CMC)1 blocks
> 
> The clocking scheme provides clear separation between M4 domain
> and A7 domain. Except for a few clock sources shared between two
> domains, such as the System Oscillator clock, the Slow IRC (SIRC),
> and and the Fast IRC clock (FIRCLK), clock sources and clock
> management are separated and contained within each domain.
> 
> M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules.
> A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules.
> 
> Note: this series only adds A7 clock domain support as M4 clock
> domain will be handled by M4 seperately.
> 
> Dong Aisheng (9):
>   clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support
>   clk: reparent orphans after critical clocks enabled
>   clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support
>   clk: imx: add pllv4 support
>   clk: imx: add pfdv2 support
>   clk: imx: add composite clk support
>   dt-bindings: clock: add imx7ulp clock binding doc
>   clk: imx: make mux parent strings const
>   clk: imx: add imx7ulp clk driver
> 

Ping.

Regards
Dong Aisheng

>  .../devicetree/bindings/clock/imx7ulp-clock.txt    |  62 +++++++
>  drivers/clk/clk-divider.c                          |   2 +
>  drivers/clk/clk-fractional-divider.c               |  10 ++
>  drivers/clk/clk.c                                  |  39 ++--
>  drivers/clk/imx/Makefile                           |   6 +-
>  drivers/clk/imx/clk-busy.c                         |   2 +-
>  drivers/clk/imx/clk-composite.c                    |  93 ++++++++++
>  drivers/clk/imx/clk-fixup-mux.c                    |   2 +-
>  drivers/clk/imx/clk-imx7ulp.c                      | 171 ++++++++++++++++++
>  drivers/clk/imx/clk-pfdv2.c                        | 199 +++++++++++++++++++++
>  drivers/clk/imx/clk-pllv4.c                        | 196 ++++++++++++++++++++
>  drivers/clk/imx/clk.h                              |  31 +++-
>  include/dt-bindings/clock/imx7ulp-clock.h          | 108 +++++++++++
>  include/linux/clk-provider.h                       |  11 ++
>  14 files changed, 902 insertions(+), 30 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/imx7ulp-clock.txt
>  create mode 100644 drivers/clk/imx/clk-composite.c
>  create mode 100644 drivers/clk/imx/clk-imx7ulp.c
>  create mode 100644 drivers/clk/imx/clk-pfdv2.c
>  create mode 100644 drivers/clk/imx/clk-pllv4.c
>  create mode 100644 include/dt-bindings/clock/imx7ulp-clock.h
> 
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support

[Stephen Boyd]

On 05/15, Dong Aisheng wrote:
> ---
>  drivers/clk/clk-divider.c    | 2 ++
>  include/linux/clk-provider.h | 4 ++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 96386ff..f78ba7a 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -125,6 +125,8 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
>  
>  	div = _get_div(table, val, flags, divider->width);
>  	if (!div) {
> +		if (flags & CLK_DIVIDER_ZERO_GATE)
> +			return 0;
>  		WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),

Why not use the CLK_DIVIDER_ALLOW_ZERO flag? A clk being off
doesn't mean the rate is 0. The divider is just disabled, so we
would consider the rate as whatever the parent is, which is what
this code does before this patch. Similarly, we don't do anything
about gate clocks and return a rate of 0 when they're disabled.

>  			"%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",
>  			clk_hw_get_name(hw));

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

Re: [PATCH 2/9] clk: reparent orphans after critical clocks enabled

[Stephen Boyd]

On 05/15, Dong Aisheng wrote:
> The orphan clocks reparent operation should be moved after the critical
> clocks enabled, otherwise it may get a chance to disable a newly registered
> critical clock which triggers the following warning.
> 
> [    0.000000] WARNING: CPU: 0 PID: 0 at drivers/clk/clk.c:597 clk_core_disable+0xb4/0xe0
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.11.0-rc1-00056-gdff1f66-dirty #1373
> [    0.000000] Hardware name: Generic DT based system
> [    0.000000] Backtrace:
> [    0.000000] [<c010c4bc>] (dump_backtrace) from [<c010c764>] (show_stack+0x18/0x1c)
> [    0.000000]  r6:600000d3 r5:00000000 r4:c0e26358 r3:00000000
> [    0.000000] [<c010c74c>] (show_stack) from [<c040599c>] (dump_stack+0xb4/0xe8)
> [    0.000000] [<c04058e8>] (dump_stack) from [<c0125c94>] (__warn+0xd8/0x104)
> [    0.000000]  r10:c0c21cd0 r9:c048aa78 r8:00000255 r7:00000009 r6:c0c1cd90 r5:00000000
> [    0.000000]  r4:00000000 r3:c0e01d34
> [    0.000000] [<c0125bbc>] (__warn) from [<c0125d74>] (warn_slowpath_null+0x28/0x30)
> [    0.000000]  r9:00000000 r8:ef00bf80 r7:c165ac4c r6:ef00bf80 r5:ef00bf80 r4:ef00bf80
> [    0.000000] [<c0125d4c>] (warn_slowpath_null) from [<c048aa78>] (clk_core_disable+0xb4/0xe0)
> [    0.000000] [<c048a9c4>] (clk_core_disable) from [<c048be88>] (clk_core_disable_lock+0x20/0x2c)
> [    0.000000]  r4:000000d3 r3:c0e0af00
> [    0.000000] [<c048be68>] (clk_core_disable_lock) from [<c048c224>] (clk_core_disable_unprepare+0x14/0x28)
> [    0.000000]  r5:00000000 r4:ef00bf80
> [    0.000000] [<c048c210>] (clk_core_disable_unprepare) from [<c048c270>] (__clk_set_parent_after+0x38/0x54)
> [    0.000000]  r4:ef00bd80 r3:000010a0
> [    0.000000] [<c048c238>] (__clk_set_parent_after) from [<c048daa8>] (clk_register+0x4d0/0x648)
> [    0.000000]  r6:ef00d500 r5:ef00bf80 r4:ef00bd80 r3:ef00bfd4
> [    0.000000] [<c048d5d8>] (clk_register) from [<c048dc30>] (clk_hw_register+0x10/0x1c)
> [    0.000000]  r9:00000000 r8:00000003 r7:00000000 r6:00000824 r5:00000001 r4:ef00d500
> [    0.000000] [<c048dc20>] (clk_hw_register) from [<c048e698>] (_register_divider+0xcc/0x120)
> [    0.000000] [<c048e5cc>] (_register_divider) from [<c048e730>] (clk_register_divider+0x44/0x54)
> [    0.000000]  r10:00000004 r9:00000003 r8:00000001 r7:00000000 r6:00000003 r5:00000001
> [    0.000000]  r4:f0810030
> [    0.000000] [<c048e6ec>] (clk_register_divider) from [<c0d3ff58>] (imx7ulp_clocks_init+0x558/0xe98)
> [    0.000000]  r7:c0e296f8 r6:c165c808 r5:00000000 r4:c165c808
> [    0.000000] [<c0d3fa00>] (imx7ulp_clocks_init) from [<c0d24db0>] (of_clk_init+0x118/0x1e0)
> [    0.000000]  r10:00000001 r9:c0e01f68 r8:00000000 r7:c0e01f60 r6:ef7f8974 r5:ef0035c0
> [    0.000000]  r4:00000006
> [    0.000000] [<c0d24c98>] (of_clk_init) from [<c0d04a50>] (time_init+0x2c/0x38)
> [    0.000000]  r10:efffed40 r9:c0d61a48 r8:c0e78000 r7:c0e07900 r6:ffffffff r5:c0e78000
> [    0.000000]  r4:00000000
> [    0.000000] [<c0d04a24>] (time_init) from [<c0d00b8c>] (start_kernel+0x218/0x394)
> [    0.000000] [<c0d00974>] (start_kernel) from [<6000807c>] (0x6000807c)
> [    0.000000]  r10:00000000 r9:410fc075 r8:6000406a r7:c0e0c930 r6:c0d61a44 r5:c0e07918
> [    0.000000]  r4:c0e78294
> [    0.000000] ---[ end trace 0000000000000000 ]---

The warning is long and scary and spans many lines, but the
scenario is described in one sentence. Please further describe
the scenario that introduces this warning without requiring the
reviewer to figure it out themselves! I certainly won't remember
2 years from now what happened here.

> 
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>

This would want a fixes tag assuming it's fixing something.

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

Re: [PATCH 3/9] clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support

[Stephen Boyd]

On 05/15, Dong Aisheng wrote:
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index a6efbb9..4466cae 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -557,6 +557,11 @@ void clk_hw_unregister_fixed_factor(struct clk_hw *hw);
>   * @lock:	register lock
>   *
>   * Clock with adjustable fractional divider affecting its output frequency.
> + *
> + * Flags:
> + * CLK_FRAC_DIVIDER_ZERO_BASED - by default the divisor is the value read
> + *	from the register.  If CLK_FRAC_DIVIDER_ZERO_BASED is set then the
> + *	divider is the raw value read from the register plus one.

This should say the numerator and denominator are both the value
read plus one. It isn't clear if it applies to the numerator, or
the denominator, or both.

>   */
>  struct clk_fractional_divider {
>  	struct clk_hw	hw;

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

Re: [PATCH 4/9] clk: imx: add pllv4 support

[Stephen Boyd]

On 05/15, Dong Aisheng wrote:
>  obj-$(CONFIG_SOC_IMX1)   += clk-imx1.o
> diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
> new file mode 100644
> index 0000000..502da64
> --- /dev/null
> +++ b/drivers/clk/imx/clk-pllv4.c
> @@ -0,0 +1,196 @@
> +/*
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Copyright 2017 NXP
> + *
> + * Author: Dong Aisheng <aisheng.dong@nxp.com>
> + *
> + * 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.
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>

Is this include used?

> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +
> +/* PLL Control Status Register (xPLLCSR) */
> +#define PLL_CSR_OFFSET		0x0
> +#define PLL_VLD			BIT(24)
> +#define PLL_EN			BIT(0)
> +
> +/* PLL Configuration Register (xPLLCFG) */
> +#define PLL_CFG_OFFSET		0x08
> +#define BP_PLL_MULT		16
> +#define BM_PLL_MULT		(0x7f << 16)
> +
> +/* PLL Numerator Register (xPLLNUM) */
> +#define PLL_NUM_OFFSET		0x10
> +
> +/* PLL Denominator Register (xPLLDENOM) */
> +#define PLL_DENOM_OFFSET	0x14
> +
> +struct clk_pllv4 {
> +	struct clk_hw	hw;
> +	void __iomem	*base;
> +};
> +
> +/* Valid PLL MULT Table */
> +static const int pllv4_mult_table[] = {33, 27, 22, 20, 17, 16};
> +
> +#define to_clk_pllv4(__hw) container_of(__hw, struct clk_pllv4, hw)
> +
> +static int clk_pllv4_wait_lock(struct clk_pllv4 *pll)
> +{
> +	unsigned long timeout = jiffies + msecs_to_jiffies(5);
> +
> +	/* Wait for PLL to lock */
> +	do {
> +		if (readl_relaxed(pll->base  + PLL_CSR_OFFSET) & PLL_VLD)
> +			break;
> +		if (time_after(jiffies, timeout))
> +			break;
> +			usleep_range(50, 500);
> +	} while (1);
> +
> +	return readl_relaxed(pll->base + PLL_CSR_OFFSET) & PLL_VLD ?
> +	       0 : -ETIMEDOUT;

Use readl_poll_timeout() instead?

> +}
> +
> +static int clk_pllv4_is_enabled(struct clk_hw *hw)
> +{
> +	struct clk_pllv4 *pll = to_clk_pllv4(hw);
> +
> +	if (readl_relaxed(pll->base) & PLL_EN)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static unsigned long clk_pllv4_recalc_rate(struct clk_hw *hw,
> +					   unsigned long parent_rate)
> +{
> +	struct clk_pllv4 *pll = to_clk_pllv4(hw);
> +	u32 div;
> +
> +	div = (readl_relaxed(pll->base + PLL_CFG_OFFSET)
> +	       & BM_PLL_MULT) >> BP_PLL_MULT;

Put that on multiple lines please:

	div = readl_relaxed(pll->base + PLL_CFG_OFFSET);
	div &= BM_PLL_MULT;
	div >>= BP_PLL_MULT; /* Why BP this time? */

> +
> +	return parent_rate * div;
> +}
> +
> +static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
> +				 unsigned long *prate)
> +{
> +	unsigned long parent_rate = *prate;
> +	unsigned long round_rate, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> +		round_rate = parent_rate * pllv4_mult_table[i];
> +		if (rate >= round_rate)
> +			return round_rate;
> +	}
> +
> +	return round_rate;
> +}
> +
> +static bool clk_pllv4_is_valid_mult(unsigned int mult)
> +{
> +	int i;
> +
> +	/* check if mult is in valid MULT table */
> +	for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> +		if (pllv4_mult_table[i] == mult)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate,
> +			      unsigned long parent_rate)
> +{
> +	struct clk_pllv4 *pll = to_clk_pllv4(hw);
> +	u32 val, mult;
> +
> +	if (clk_pllv4_is_enabled(hw)) {
> +		WARN(1, "clk_pllv4: can't change rate when pll is enabled");
> +		return -EINVAL;

Sad, CLK_SET_RATE_GATE isn't working for you I suppose?

> +	}
> +
> +	mult = rate / parent_rate;
> +
> +	if (clk_pllv4_is_valid_mult(mult))
> +		return -EINVAL;
> +
> +	val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> +	val &= ~BM_PLL_MULT;
> +	val |= mult << BP_PLL_MULT;
> +	writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
> +
> +	return 0;
> +}
> +

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

Re: [PATCH 6/9] clk: imx: add composite clk support

[Stephen Boyd]

On 05/15, Dong Aisheng wrote:
> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
> index 51b84e4..665777e 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -69,6 +69,12 @@ struct clk *imx_clk_busy_mux(const char *name, void __iomem *reg, u8 shift,
>  			     u8 width, void __iomem *busy_reg, u8 busy_shift,
>  			     const char **parent_names, int num_parents);
>  
> +struct clk *imx_clk_composite(const char *name,
> +			      const char * const *parent_names,
> +			      int num_parents, bool mux_present,
> +			      bool rate_present, bool gate_present,
> +			      void __iomem *reg);
> +
>  struct clk *imx_clk_fixup_divider(const char *name, const char *parent,
>  				  void __iomem *reg, u8 shift, u8 width,
>  				  void (*fixup)(u32 *val));
> @@ -209,5 +215,4 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
>  struct clk *imx_clk_cpu(const char *name, const char *parent_name,
>  		struct clk *div, struct clk *mux, struct clk *pll,
>  		struct clk *step);
> -

Noise?

>  #endif
> -- 
> 2.7.4
> 

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

Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver

[Stephen Boyd]

On 05/15, Dong Aisheng wrote:
> +
> +	clks[IMX7ULP_CLK_VIU]		= imx_clk_gate("viu",	"nic1_clk",	base + 0xA0, 30);
> +	clks[IMX7ULP_CLK_PCTLC]		= imx_clk_gate("pctlc", "nic1_bus_clk", base + 0xB8, 30);
> +	clks[IMX7ULP_CLK_PCTLD]		= imx_clk_gate("pctld", "nic1_bus_clk", base + 0xBC, 30);
> +	clks[IMX7ULP_CLK_PCTLE]		= imx_clk_gate("pctle", "nic1_bus_clk", base + 0xc0, 30);
> +	clks[IMX7ULP_CLK_PCTLF]		= imx_clk_gate("pctlf", "nic1_bus_clk", base + 0xc4, 30);
> +
> +	clks[IMX7ULP_CLK_GPU3D]		= imx_clk_composite("gpu3d",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x140);
> +	clks[IMX7ULP_CLK_GPU2D]		= imx_clk_composite("gpu2d",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x144);
> +
> +	imx_check_clocks(clks, ARRAY_SIZE(clks));
> +
> +	clk_data.clks = clks;
> +	clk_data.clk_num = ARRAY_SIZE(clks);
> +	of_clk_add_provider(scg_node, of_clk_src_onecell_get, &clk_data);

Please use of_clk_add_hw_provider() instead, and the associated
clk_hw registration APIs.

> +
> +	pr_info("i.MX7ULP clock tree init done.\n");

pr_debug?

> +}
> +
> +CLK_OF_DECLARE(imx7ulp, "fsl,imx7ulp-clock", imx7ulp_clocks_init);
> 

Any reason why it can't be a platform driver? If not, please add
some comment explaining why.

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

Re: [PATCH 8/9] clk: imx: make mux parent strings const

[Stephen Boyd]

On 05/15, Dong Aisheng wrote:
> As the commit 2893c379461a ("clk: make strings in parent name arrays
> const"), let's make the parent strings const, otherwise we may meet
> the following warning when compiling:
> 
> drivers/clk/imx/clk-imx7ulp.c: In function 'imx7ulp_clocks_init':
> drivers/clk/imx/clk-imx7ulp.c:73:35: warning: passing argument 5 of
> 	'imx_clk_mux_flags' discards 'const' qualifier from pointer target type
> 
>   clks[IMX7ULP_CLK_APLL_PRE_SEL] = imx_clk_mux_flags("apll_pre_sel", base + 0x508, 0,
> 	1, pll_pre_sels, ARRAY_SIZE(pll_pre_sels), CLK_SET_PARENT_GATE);
>                                    ^
> In file included from drivers/clk/imx/clk-imx7ulp.c:23:0:
> drivers/clk/imx/clk.h:200:27: note: expected 'const char **' but argument is
>  of type 'const char * const*'
> ...
> 
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---

Looks good.

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

Re: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support

[Dong Aisheng]

Hi Stephen,

On Mon, Jun 19, 2017 at 06:45:12PM -0700, Stephen Boyd wrote:
> On 05/15, Dong Aisheng wrote:
> > ---
> >  drivers/clk/clk-divider.c    | 2 ++
> >  include/linux/clk-provider.h | 4 ++++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> > index 96386ff..f78ba7a 100644
> > --- a/drivers/clk/clk-divider.c
> > +++ b/drivers/clk/clk-divider.c
> > @@ -125,6 +125,8 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
> >  
> >  	div = _get_div(table, val, flags, divider->width);
> >  	if (!div) {
> > +		if (flags & CLK_DIVIDER_ZERO_GATE)
> > +			return 0;
> >  		WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
> 
> Why not use the CLK_DIVIDER_ALLOW_ZERO flag? A clk being off
> doesn't mean the rate is 0. The divider is just disabled, so we
> would consider the rate as whatever the parent is, which is what
> this code does before this patch. Similarly, we don't do anything
> about gate clocks and return a rate of 0 when they're disabled.
> 

The semantic of CLK_DIVIDER_ALLOW_ZERO seems a bit different.

See below definition:
* CLK_DIVIDER_ALLOW_ZERO - Allow zero divisors.  For dividers which have
*      CLK_DIVIDER_ONE_BASED set, it is possible to end up with a zero divisor.
*      Some hardware implementations gracefully handle this case and allow a
*      zero divisor by not modifying their input clock
*      (divide by one / bypass).

zero divisor is simply as divide by one or bypass which is supported by
hardware.

But it's not true for this hardware.

If we consider the rate as whatever the parent is if divider is zero,
we may got an issue like below:
e.g.
Assuming spll_bus_clk divider is 0x0 and it may be enabled by users directly
without setting a rate first.

Then the clock tree looks like:
...
spll_pfd0                    1            1   500210526          0 0  
  spll_pfd_sel              1            1   500210526          0 0   
    spll_sel               1            1   500210526          0 0    
      spll_bus_clk           1            1   500210526          0 0 

But the spll_bus_clk clock rate actually is wrong and it's even not enabled,
not like CLK_DIVIDER_ALLOW_ZERO which zero divider means simply bypass.

So for this case, we probably can't simply assume zero divider rate as its
parent, it is actually set to 0 in hw, although it's something like gate,
but a bit different from gate as the normal gate does not affect divider
where you can keep the rate.

How would you suggest for this? 

Regards
Dong Aisheng

> >  			"%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",
> >  			clk_hw_get_name(hw));
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] clk: reparent orphans after critical clocks enabled

[Dong Aisheng]

On Mon, Jun 19, 2017 at 06:51:40PM -0700, Stephen Boyd wrote:
> On 05/15, Dong Aisheng wrote:
> > The orphan clocks reparent operation should be moved after the critical
> > clocks enabled, otherwise it may get a chance to disable a newly registered
> > critical clock which triggers the following warning.
> > 
> > [    0.000000] WARNING: CPU: 0 PID: 0 at drivers/clk/clk.c:597 clk_core_disable+0xb4/0xe0
> > [    0.000000] Modules linked in:
> > [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.11.0-rc1-00056-gdff1f66-dirty #1373
> > [    0.000000] Hardware name: Generic DT based system
> > [    0.000000] Backtrace:
> > [    0.000000] [<c010c4bc>] (dump_backtrace) from [<c010c764>] (show_stack+0x18/0x1c)
> > [    0.000000]  r6:600000d3 r5:00000000 r4:c0e26358 r3:00000000
> > [    0.000000] [<c010c74c>] (show_stack) from [<c040599c>] (dump_stack+0xb4/0xe8)
> > [    0.000000] [<c04058e8>] (dump_stack) from [<c0125c94>] (__warn+0xd8/0x104)
> > [    0.000000]  r10:c0c21cd0 r9:c048aa78 r8:00000255 r7:00000009 r6:c0c1cd90 r5:00000000
> > [    0.000000]  r4:00000000 r3:c0e01d34
> > [    0.000000] [<c0125bbc>] (__warn) from [<c0125d74>] (warn_slowpath_null+0x28/0x30)
> > [    0.000000]  r9:00000000 r8:ef00bf80 r7:c165ac4c r6:ef00bf80 r5:ef00bf80 r4:ef00bf80
> > [    0.000000] [<c0125d4c>] (warn_slowpath_null) from [<c048aa78>] (clk_core_disable+0xb4/0xe0)
> > [    0.000000] [<c048a9c4>] (clk_core_disable) from [<c048be88>] (clk_core_disable_lock+0x20/0x2c)
> > [    0.000000]  r4:000000d3 r3:c0e0af00
> > [    0.000000] [<c048be68>] (clk_core_disable_lock) from [<c048c224>] (clk_core_disable_unprepare+0x14/0x28)
> > [    0.000000]  r5:00000000 r4:ef00bf80
> > [    0.000000] [<c048c210>] (clk_core_disable_unprepare) from [<c048c270>] (__clk_set_parent_after+0x38/0x54)
> > [    0.000000]  r4:ef00bd80 r3:000010a0
> > [    0.000000] [<c048c238>] (__clk_set_parent_after) from [<c048daa8>] (clk_register+0x4d0/0x648)
> > [    0.000000]  r6:ef00d500 r5:ef00bf80 r4:ef00bd80 r3:ef00bfd4
> > [    0.000000] [<c048d5d8>] (clk_register) from [<c048dc30>] (clk_hw_register+0x10/0x1c)
> > [    0.000000]  r9:00000000 r8:00000003 r7:00000000 r6:00000824 r5:00000001 r4:ef00d500
> > [    0.000000] [<c048dc20>] (clk_hw_register) from [<c048e698>] (_register_divider+0xcc/0x120)
> > [    0.000000] [<c048e5cc>] (_register_divider) from [<c048e730>] (clk_register_divider+0x44/0x54)
> > [    0.000000]  r10:00000004 r9:00000003 r8:00000001 r7:00000000 r6:00000003 r5:00000001
> > [    0.000000]  r4:f0810030
> > [    0.000000] [<c048e6ec>] (clk_register_divider) from [<c0d3ff58>] (imx7ulp_clocks_init+0x558/0xe98)
> > [    0.000000]  r7:c0e296f8 r6:c165c808 r5:00000000 r4:c165c808
> > [    0.000000] [<c0d3fa00>] (imx7ulp_clocks_init) from [<c0d24db0>] (of_clk_init+0x118/0x1e0)
> > [    0.000000]  r10:00000001 r9:c0e01f68 r8:00000000 r7:c0e01f60 r6:ef7f8974 r5:ef0035c0
> > [    0.000000]  r4:00000006
> > [    0.000000] [<c0d24c98>] (of_clk_init) from [<c0d04a50>] (time_init+0x2c/0x38)
> > [    0.000000]  r10:efffed40 r9:c0d61a48 r8:c0e78000 r7:c0e07900 r6:ffffffff r5:c0e78000
> > [    0.000000]  r4:00000000
> > [    0.000000] [<c0d04a24>] (time_init) from [<c0d00b8c>] (start_kernel+0x218/0x394)
> > [    0.000000] [<c0d00974>] (start_kernel) from [<6000807c>] (0x6000807c)
> > [    0.000000]  r10:00000000 r9:410fc075 r8:6000406a r7:c0e0c930 r6:c0d61a44 r5:c0e07918
> > [    0.000000]  r4:c0e78294
> > [    0.000000] ---[ end trace 0000000000000000 ]---
> 
> The warning is long and scary and spans many lines, but the
> scenario is described in one sentence. Please further describe
> the scenario that introduces this warning without requiring the
> reviewer to figure it out themselves! I certainly won't remember
> 2 years from now what happened here.
> 

Yes, my fault.

I will add more descriptions in next version.

Below is the draft explaination.

This issue actually is caused by that in current code, orphan clocks
re-parent operation is before the critical clock enable operation.
Before the critical clock is enabled, it may has a chance to be disabled
already in orphan re-parent operation.

Assuming we have two clocks: A and B while B is A's parent.
Clock A has flag: CLK_OPS_PARENT_ENABLE
Clock B has flag: CLK_IS_CRITICAL

Step 1:
Clock A is registered, then it becomes orphan.

Step 2:
Clock B is registered. Before clock B reach the critical clock
enable operation, orphan A find B parent and do reparent, then
the parent B may be disabled in __clk_set_parent_after()
due to CLK_OPS_PARENT_ENABLE flag.

static int __clk_core_init(struct clk_core *core) {
	.............
       hlist_for_each_entry_safe(orphan, tmp2, &clk_orphan_list, child_node) {
                struct clk_core *parent = __clk_init_parent(orphan);

                /*
                 * we could call __clk_set_parent, but that would result in a
                 * redundant call to the .set_rate op, if it exists
                 */
                if (parent) {
                        __clk_set_parent_before(orphan, parent);
                        __clk_set_parent_after(orphan, parent, NULL);
                        __clk_recalc_accuracies(orphan);
                        __clk_recalc_rates(orphan, 0);
                }
        }
	....

        if (core->flags & CLK_IS_CRITICAL) {
                unsigned long flags;

                clk_core_prepare(core);

                flags = clk_enable_lock();
                clk_core_enable(core);
                clk_enable_unlock(flags);
        }
	...
}

> > 
> > Cc: Stephen Boyd <sboyd@codeaurora.org>
> > Cc: Michael Turquette <mturquette@baylibre.com>
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> 
> This would want a fixes tag assuming it's fixing something.
> 

Yes, then it should be commit fc8726a2c021
clk: core: support clocks which requires parents enable (part 2)
Did not consider that situation.

Will add later.

Regards
Dong Aisheng

> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9] clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support

[Dong Aisheng]

On Mon, Jun 19, 2017 at 06:55:47PM -0700, Stephen Boyd wrote:
> On 05/15, Dong Aisheng wrote:
> > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> > index a6efbb9..4466cae 100644
> > --- a/include/linux/clk-provider.h
> > +++ b/include/linux/clk-provider.h
> > @@ -557,6 +557,11 @@ void clk_hw_unregister_fixed_factor(struct clk_hw *hw);
> >   * @lock:	register lock
> >   *
> >   * Clock with adjustable fractional divider affecting its output frequency.
> > + *
> > + * Flags:
> > + * CLK_FRAC_DIVIDER_ZERO_BASED - by default the divisor is the value read
> > + *	from the register.  If CLK_FRAC_DIVIDER_ZERO_BASED is set then the
> > + *	divider is the raw value read from the register plus one.
> 
> This should say the numerator and denominator are both the value
> read plus one. It isn't clear if it applies to the numerator, or
> the denominator, or both.
> 

Good suggestion.
Will improve it.

Regards
Dong Aisheng

> >   */
> >  struct clk_fractional_divider {
> >  	struct clk_hw	hw;
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9] clk: imx: add pllv4 support

[Dong Aisheng]

On Mon, Jun 19, 2017 at 06:59:17PM -0700, Stephen Boyd wrote:
> On 05/15, Dong Aisheng wrote:
> >  obj-$(CONFIG_SOC_IMX1)   += clk-imx1.o
> > diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
> > new file mode 100644
> > index 0000000..502da64
> > --- /dev/null
> > +++ b/drivers/clk/imx/clk-pllv4.c
> > @@ -0,0 +1,196 @@
> > +/*
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + * Copyright 2017 NXP
> > + *
> > + * Author: Dong Aisheng <aisheng.dong@nxp.com>
> > + *
> > + * 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.
> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/clk.h>
> 
> Is this include used?
> 

Nice catch. Will remove.

> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/slab.h>
> > +
> > +/* PLL Control Status Register (xPLLCSR) */
> > +#define PLL_CSR_OFFSET		0x0
> > +#define PLL_VLD			BIT(24)
> > +#define PLL_EN			BIT(0)
> > +
> > +/* PLL Configuration Register (xPLLCFG) */
> > +#define PLL_CFG_OFFSET		0x08
> > +#define BP_PLL_MULT		16
> > +#define BM_PLL_MULT		(0x7f << 16)
> > +
> > +/* PLL Numerator Register (xPLLNUM) */
> > +#define PLL_NUM_OFFSET		0x10
> > +
> > +/* PLL Denominator Register (xPLLDENOM) */
> > +#define PLL_DENOM_OFFSET	0x14
> > +
> > +struct clk_pllv4 {
> > +	struct clk_hw	hw;
> > +	void __iomem	*base;
> > +};
> > +
> > +/* Valid PLL MULT Table */
> > +static const int pllv4_mult_table[] = {33, 27, 22, 20, 17, 16};
> > +
> > +#define to_clk_pllv4(__hw) container_of(__hw, struct clk_pllv4, hw)
> > +
> > +static int clk_pllv4_wait_lock(struct clk_pllv4 *pll)
> > +{
> > +	unsigned long timeout = jiffies + msecs_to_jiffies(5);
> > +
> > +	/* Wait for PLL to lock */
> > +	do {
> > +		if (readl_relaxed(pll->base  + PLL_CSR_OFFSET) & PLL_VLD)
> > +			break;
> > +		if (time_after(jiffies, timeout))
> > +			break;
> > +			usleep_range(50, 500);
> > +	} while (1);
> > +
> > +	return readl_relaxed(pll->base + PLL_CSR_OFFSET) & PLL_VLD ?
> > +	       0 : -ETIMEDOUT;
> 
> Use readl_poll_timeout() instead?
> 

Good idea.

> > +}
> > +
> > +static int clk_pllv4_is_enabled(struct clk_hw *hw)
> > +{
> > +	struct clk_pllv4 *pll = to_clk_pllv4(hw);
> > +
> > +	if (readl_relaxed(pll->base) & PLL_EN)
> > +		return 1;
> > +
> > +	return 0;
> > +}
> > +
> > +static unsigned long clk_pllv4_recalc_rate(struct clk_hw *hw,
> > +					   unsigned long parent_rate)
> > +{
> > +	struct clk_pllv4 *pll = to_clk_pllv4(hw);
> > +	u32 div;
> > +
> > +	div = (readl_relaxed(pll->base + PLL_CFG_OFFSET)
> > +	       & BM_PLL_MULT) >> BP_PLL_MULT;
> 
> Put that on multiple lines please:
> 
> 	div = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> 	div &= BM_PLL_MULT;
> 	div >>= BP_PLL_MULT; /* Why BP this time? */
> 

Of course.

> > +
> > +	return parent_rate * div;
> > +}
> > +
> > +static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
> > +				 unsigned long *prate)
> > +{
> > +	unsigned long parent_rate = *prate;
> > +	unsigned long round_rate, i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> > +		round_rate = parent_rate * pllv4_mult_table[i];
> > +		if (rate >= round_rate)
> > +			return round_rate;
> > +	}
> > +
> > +	return round_rate;
> > +}
> > +
> > +static bool clk_pllv4_is_valid_mult(unsigned int mult)
> > +{
> > +	int i;
> > +
> > +	/* check if mult is in valid MULT table */
> > +	for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> > +		if (pllv4_mult_table[i] == mult)
> > +			return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate,
> > +			      unsigned long parent_rate)
> > +{
> > +	struct clk_pllv4 *pll = to_clk_pllv4(hw);
> > +	u32 val, mult;
> > +
> > +	if (clk_pllv4_is_enabled(hw)) {
> > +		WARN(1, "clk_pllv4: can't change rate when pll is enabled");
> > +		return -EINVAL;
> 
> Sad, CLK_SET_RATE_GATE isn't working for you I suppose?
> 

CLK_SET_RATE_GATE can't work in early stage before running clk_disable_unused.
At that point, the clock tree state is still not consistent with HW.
e.g. prepare/enable count is still zero but it's actually enabled due to
reset state or bootloader.

The code here is adding a double check in case user sets rate in early stage.

However, probably it could also be moved into clock core as it's not platform
dependant behavior?

Regards
Dong Aisheng

> > +	}
> > +
> > +	mult = rate / parent_rate;
> > +
> > +	if (clk_pllv4_is_valid_mult(mult))
> > +		return -EINVAL;
> > +
> > +	val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > +	val &= ~BM_PLL_MULT;
> > +	val |= mult << BP_PLL_MULT;
> > +	writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
> > +
> > +	return 0;
> > +}
> > +
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] clk: imx: add composite clk support

[Dong Aisheng]

On Mon, Jun 19, 2017 at 07:00:13PM -0700, Stephen Boyd wrote:
> On 05/15, Dong Aisheng wrote:
> > diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
> > index 51b84e4..665777e 100644
> > --- a/drivers/clk/imx/clk.h
> > +++ b/drivers/clk/imx/clk.h
> > @@ -69,6 +69,12 @@ struct clk *imx_clk_busy_mux(const char *name, void __iomem *reg, u8 shift,
> >  			     u8 width, void __iomem *busy_reg, u8 busy_shift,
> >  			     const char **parent_names, int num_parents);
> >  
> > +struct clk *imx_clk_composite(const char *name,
> > +			      const char * const *parent_names,
> > +			      int num_parents, bool mux_present,
> > +			      bool rate_present, bool gate_present,
> > +			      void __iomem *reg);
> > +
> >  struct clk *imx_clk_fixup_divider(const char *name, const char *parent,
> >  				  void __iomem *reg, u8 shift, u8 width,
> >  				  void (*fixup)(u32 *val));
> > @@ -209,5 +215,4 @@ static inline struct clk *imx_clk_mux_flags(const char *name,
> >  struct clk *imx_clk_cpu(const char *name, const char *parent_name,
> >  		struct clk *div, struct clk *mux, struct clk *pll,
> >  		struct clk *step);
> > -
> 
> Noise?
> 

Yes, got by mistake.
Will remove.

Regards
Dong Aisheng

Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver

[Dong Aisheng]

On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> On 05/15, Dong Aisheng wrote:
> > +
> > +	clks[IMX7ULP_CLK_VIU]		= imx_clk_gate("viu",	"nic1_clk",	base + 0xA0, 30);
> > +	clks[IMX7ULP_CLK_PCTLC]		= imx_clk_gate("pctlc", "nic1_bus_clk", base + 0xB8, 30);
> > +	clks[IMX7ULP_CLK_PCTLD]		= imx_clk_gate("pctld", "nic1_bus_clk", base + 0xBC, 30);
> > +	clks[IMX7ULP_CLK_PCTLE]		= imx_clk_gate("pctle", "nic1_bus_clk", base + 0xc0, 30);
> > +	clks[IMX7ULP_CLK_PCTLF]		= imx_clk_gate("pctlf", "nic1_bus_clk", base + 0xc4, 30);
> > +
> > +	clks[IMX7ULP_CLK_GPU3D]		= imx_clk_composite("gpu3d",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x140);
> > +	clks[IMX7ULP_CLK_GPU2D]		= imx_clk_composite("gpu2d",   periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x144);
> > +
> > +	imx_check_clocks(clks, ARRAY_SIZE(clks));
> > +
> > +	clk_data.clks = clks;
> > +	clk_data.clk_num = ARRAY_SIZE(clks);
> > +	of_clk_add_provider(scg_node, of_clk_src_onecell_get, &clk_data);
> 
> Please use of_clk_add_hw_provider() instead, and the associated
> clk_hw registration APIs.
> 

Sure, will do it.

> > +
> > +	pr_info("i.MX7ULP clock tree init done.\n");
> 
> pr_debug?
> 

Yes

> > +}
> > +
> > +CLK_OF_DECLARE(imx7ulp, "fsl,imx7ulp-clock", imx7ulp_clocks_init);
> > 
> 
> Any reason why it can't be a platform driver? If not, please add
> some comment explaining why.
> 

Timer is using it at early stage. GIC seems not although standard
binding claim possible clock requirement.
Others still not sure.

What your suggestion?
Convert timer to platform driver and make clock as platform driver as well?

Regards
Dong Aisheng

> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver

[Stephen Boyd]

On 06/20, Dong Aisheng wrote:
> On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> > 
> > Any reason why it can't be a platform driver? If not, please add
> > some comment explaining why.
> > 
> 
> Timer is using it at early stage. GIC seems not although standard
> binding claim possible clock requirement.
> Others still not sure.
> 
> What your suggestion?
> Convert timer to platform driver and make clock as platform driver as well?
> 

The timer can't be a platform driver because it would be too
late. The clock driver could register whatever clks are required
for the timer/GIC in a CLK_OF_DECLARE_DRIVER hook, and then leave
the rest to a platform driver. This way we get some of the device
driver framework in this code.

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

RE: [PATCH 9/9] clk: imx: add imx7ulp clk driver

[A.s. Dong]

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@codeaurora.org]
> Sent: Wednesday, June 21, 2017 4:42 AM
> To: Dong Aisheng
> Cc: A.s. Dong; linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com;
> shawnguo@kernel.org; Anson Huang; Jacky Bai
> Subject: Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver
> 
> On 06/20, Dong Aisheng wrote:
> > On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> > >
> > > Any reason why it can't be a platform driver? If not, please add
> > > some comment explaining why.
> > >
> >
> > Timer is using it at early stage. GIC seems not although standard
> > binding claim possible clock requirement.
> > Others still not sure.
> >
> > What your suggestion?
> > Convert timer to platform driver and make clock as platform driver as
> well?
> >
> 
> The timer can't be a platform driver because it would be too late. The
> clock driver could register whatever clks are required for the timer/GIC
> in a CLK_OF_DECLARE_DRIVER hook, and then leave the rest to a platform
> driver. This way we get some of the device driver framework in this code.
> 

Okay, I could try it. Thanks.

One thing is that TPM clock has a lot parents and parents having parents,
as well as PIT timer. So I may need enable more than half clocks in
CLK_OF_DECLARE_DRIVER hook.

BTW, What's benefit to convert into two parts of probe?
I'm not quite if I already get it all, can you help clarify it?

Regards
Dong Aisheng

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

RE: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support

[A.s. Dong]

Hi Stephen,

> -----Original Message-----
> From: Dong Aisheng [mailto:dongas86@gmail.com]
> Sent: Tuesday, June 20, 2017 5:08 PM
> To: Stephen Boyd
> Cc: A.s. Dong; linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com;
> shawnguo@kernel.org; Anson Huang; Jacky Bai
> Subject: Re: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk
> support
> 
> Hi Stephen,
> 
> On Mon, Jun 19, 2017 at 06:45:12PM -0700, Stephen Boyd wrote:
> > On 05/15, Dong Aisheng wrote:
> > > ---
> > >  drivers/clk/clk-divider.c    | 2 ++
> > >  include/linux/clk-provider.h | 4 ++++
> > >  2 files changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> > > index 96386ff..f78ba7a 100644
> > > --- a/drivers/clk/clk-divider.c
> > > +++ b/drivers/clk/clk-divider.c
> > > @@ -125,6 +125,8 @@ unsigned long divider_recalc_rate(struct clk_hw
> > > *hw, unsigned long parent_rate,
> > >
> > >  	div = _get_div(table, val, flags, divider->width);
> > >  	if (!div) {
> > > +		if (flags & CLK_DIVIDER_ZERO_GATE)
> > > +			return 0;
> > >  		WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
> >
> > Why not use the CLK_DIVIDER_ALLOW_ZERO flag? A clk being off doesn't
> > mean the rate is 0. The divider is just disabled, so we would consider
> > the rate as whatever the parent is, which is what this code does
> > before this patch. Similarly, we don't do anything about gate clocks
> > and return a rate of 0 when they're disabled.
> >
> 
> The semantic of CLK_DIVIDER_ALLOW_ZERO seems a bit different.
> 
> See below definition:
> * CLK_DIVIDER_ALLOW_ZERO - Allow zero divisors.  For dividers which have
> *      CLK_DIVIDER_ONE_BASED set, it is possible to end up with a zero
> divisor.
> *      Some hardware implementations gracefully handle this case and allow
> a
> *      zero divisor by not modifying their input clock
> *      (divide by one / bypass).
> 
> zero divisor is simply as divide by one or bypass which is supported by
> hardware.
> 
> But it's not true for this hardware.
> 
> If we consider the rate as whatever the parent is if divider is zero, we
> may got an issue like below:
> e.g.
> Assuming spll_bus_clk divider is 0x0 and it may be enabled by users
> directly without setting a rate first.
> 
> Then the clock tree looks like:
> ...
> spll_pfd0                    1            1   500210526          0 0
>   spll_pfd_sel              1            1   500210526          0 0
>     spll_sel               1            1   500210526          0 0
>       spll_bus_clk           1            1   500210526          0 0
> 
> But the spll_bus_clk clock rate actually is wrong and it's even not
> enabled, not like CLK_DIVIDER_ALLOW_ZERO which zero divider means simply
> bypass.
> 
> So for this case, we probably can't simply assume zero divider rate as its
> parent, it is actually set to 0 in hw, although it's something like gate,
> but a bit different from gate as the normal gate does not affect divider
> where you can keep the rate.
> 
> How would you suggest for this?
> 

Any suggestions?

Regards
Dong Aisheng

> Regards
> Dong Aisheng
> 
> > >  			"%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",
> > >  			clk_hw_get_name(hw));
> >
> > --
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> > Linux Foundation Collaborative Project
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-clk"
> > in the body of a message to majordomo@vger.kernel.org More majordomo
> > info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver

[Stephen Boyd]

On 06/21, A.s. Dong wrote:
> > -----Original Message-----
> > From: Stephen Boyd [mailto:sboyd@codeaurora.org]
> > Sent: Wednesday, June 21, 2017 4:42 AM
> > To: Dong Aisheng
> > Cc: A.s. Dong; linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com;
> > shawnguo@kernel.org; Anson Huang; Jacky Bai
> > Subject: Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver
> > 
> > On 06/20, Dong Aisheng wrote:
> > > On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> > > >
> > > > Any reason why it can't be a platform driver? If not, please add
> > > > some comment explaining why.
> > > >
> > >
> > > Timer is using it at early stage. GIC seems not although standard
> > > binding claim possible clock requirement.
> > > Others still not sure.
> > >
> > > What your suggestion?
> > > Convert timer to platform driver and make clock as platform driver as
> > well?
> > >
> > 
> > The timer can't be a platform driver because it would be too late. The
> > clock driver could register whatever clks are required for the timer/GIC
> > in a CLK_OF_DECLARE_DRIVER hook, and then leave the rest to a platform
> > driver. This way we get some of the device driver framework in this code.
> > 
> 
> Okay, I could try it. Thanks.
> 
> One thing is that TPM clock has a lot parents and parents having parents,
> as well as PIT timer. So I may need enable more than half clocks in
> CLK_OF_DECLARE_DRIVER hook.

That's fine.

> 
> BTW, What's benefit to convert into two parts of probe?
> I'm not quite if I already get it all, can you help clarify it?
> 

The benefit is that we still get a platform driver and we can
associate a device pointer with the clock controller eventually.
Here's a reply I sent yesterday on the same topic:

Reasons (in no particular order):

  1. We get a dev pointer to use with clk_hw_register()

  2. We can handle probe defer if some resource is not available

  3. Using device model gets us a hook into power management frameworks
     like runtime PM and system PM for things like suspend and hibernate

  4. It encourages a single DT node clk controller style binding
     instead of a single node per clk style binding

  5. We can use non-DT specific functions like devm_ioremap_resource() to map
     registers and acquire other resources, leading to more portable and
     generic code

  6. We may be able to make the device driver a module, which will
     make distros happy if we don't have to compile in all
     these clk drivers to the resulting vmlinux (this one doesn't
     apply here)

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

Re: [PATCH 4/9] clk: imx: add pllv4 support

[Stephen Boyd]

On 06/20, Dong Aisheng wrote:
> On Mon, Jun 19, 2017 at 06:59:17PM -0700, Stephen Boyd wrote:
> > On 05/15, Dong Aisheng wrote:
> > > +
> > > +	if (clk_pllv4_is_enabled(hw)) {
> > > +		WARN(1, "clk_pllv4: can't change rate when pll is enabled");
> > > +		return -EINVAL;
> > 
> > Sad, CLK_SET_RATE_GATE isn't working for you I suppose?
> > 
> 
> CLK_SET_RATE_GATE can't work in early stage before running clk_disable_unused.
> At that point, the clock tree state is still not consistent with HW.
> e.g. prepare/enable count is still zero but it's actually enabled due to
> reset state or bootloader.
> 
> The code here is adding a double check in case user sets rate in early stage.
> 
> However, probably it could also be moved into clock core as it's not platform
> dependant behavior?
> 

Ok. It would be good to fix the core framework to synchronize the
prepared/enabled state at registration time so we don't need this
check in the driver.

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

Re: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support

[Stephen Boyd]

On 06/20, Dong Aisheng wrote:
> Hi Stephen,
> 
> On Mon, Jun 19, 2017 at 06:45:12PM -0700, Stephen Boyd wrote:
> > On 05/15, Dong Aisheng wrote:
> > > ---
> > >  drivers/clk/clk-divider.c    | 2 ++
> > >  include/linux/clk-provider.h | 4 ++++
> > >  2 files changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> > > index 96386ff..f78ba7a 100644
> > > --- a/drivers/clk/clk-divider.c
> > > +++ b/drivers/clk/clk-divider.c
> > > @@ -125,6 +125,8 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
> > >  
> > >  	div = _get_div(table, val, flags, divider->width);
> > >  	if (!div) {
> > > +		if (flags & CLK_DIVIDER_ZERO_GATE)
> > > +			return 0;
> > >  		WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
> > 
> > Why not use the CLK_DIVIDER_ALLOW_ZERO flag? A clk being off
> > doesn't mean the rate is 0. The divider is just disabled, so we
> > would consider the rate as whatever the parent is, which is what
> > this code does before this patch. Similarly, we don't do anything
> > about gate clocks and return a rate of 0 when they're disabled.
> > 
> 
> The semantic of CLK_DIVIDER_ALLOW_ZERO seems a bit different.
> 
> See below definition:
> * CLK_DIVIDER_ALLOW_ZERO - Allow zero divisors.  For dividers which have
> *      CLK_DIVIDER_ONE_BASED set, it is possible to end up with a zero divisor.
> *      Some hardware implementations gracefully handle this case and allow a
> *      zero divisor by not modifying their input clock
> *      (divide by one / bypass).
> 
> zero divisor is simply as divide by one or bypass which is supported by
> hardware.
> 
> But it's not true for this hardware.
> 
> If we consider the rate as whatever the parent is if divider is zero,
> we may got an issue like below:
> e.g.
> Assuming spll_bus_clk divider is 0x0 and it may be enabled by users directly
> without setting a rate first.
> 
> Then the clock tree looks like:
> ...
> spll_pfd0                    1            1   500210526          0 0  
>   spll_pfd_sel              1            1   500210526          0 0   
>     spll_sel               1            1   500210526          0 0    
>       spll_bus_clk           1            1   500210526          0 0 
> 
> But the spll_bus_clk clock rate actually is wrong and it's even not enabled,
> not like CLK_DIVIDER_ALLOW_ZERO which zero divider means simply bypass.
> 
> So for this case, we probably can't simply assume zero divider rate as its
> parent, it is actually set to 0 in hw, although it's something like gate,
> but a bit different from gate as the normal gate does not affect divider
> where you can keep the rate.
> 
> How would you suggest for this? 
> 

It seems that set_rate() and enable/disable are conflated here.
>From what you describe, it sounds like the clk is considered off
when the divider value is zero, and it's on when the divider
value is non-zero.

I'd suggest you make it so this clk supports enable/disable and
set_rate with the same register. Something like, set rate when
the clk is disabled will cache the rate request and only when the
clk is enabled will the driver actually program the hardware to
have the requested divider value. Similarly, when the clk is
disabled we'll write a 0 there, but when the clk is enabled we'll
restore whatever rate (divider) was chosen last.

It does mean that recalc rate will be sort of odd, because when
the clk is off it will return 0, and when the clk is on it will
return the right rate. So to make things work, we'll need to
return the cached rate in recalc rate when the clk is off and
read the hardware when the clk is on. Probably an if register ==
0 then lookup in cache, otherwise do normal division.

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

RE: [PATCH 9/9] clk: imx: add imx7ulp clk driver

[A.s. Dong]

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@codeaurora.org]
> Sent: Saturday, July 01, 2017 8:35 AM
> To: A.s. Dong
> Cc: Dong Aisheng; linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com;
> shawnguo@kernel.org; Anson Huang; Jacky Bai
> Subject: Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver
> 
> On 06/21, A.s. Dong wrote:
> > > -----Original Message-----
> > > From: Stephen Boyd [mailto:sboyd@codeaurora.org]
> > > Sent: Wednesday, June 21, 2017 4:42 AM
> > > To: Dong Aisheng
> > > Cc: A.s. Dong; linux-clk@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > mturquette@baylibre.com; shawnguo@kernel.org; Anson Huang; Jacky Bai
> > > Subject: Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver
> > >
> > > On 06/20, Dong Aisheng wrote:
> > > > On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> > > > >
> > > > > Any reason why it can't be a platform driver? If not, please add
> > > > > some comment explaining why.
> > > > >
> > > >
> > > > Timer is using it at early stage. GIC seems not although standard
> > > > binding claim possible clock requirement.
> > > > Others still not sure.
> > > >
> > > > What your suggestion?
> > > > Convert timer to platform driver and make clock as platform driver
> > > > as
> > > well?
> > > >
> > >
> > > The timer can't be a platform driver because it would be too late.
> > > The clock driver could register whatever clks are required for the
> > > timer/GIC in a CLK_OF_DECLARE_DRIVER hook, and then leave the rest
> > > to a platform driver. This way we get some of the device driver
> framework in this code.
> > >
> >
> > Okay, I could try it. Thanks.
> >
> > One thing is that TPM clock has a lot parents and parents having
> > parents, as well as PIT timer. So I may need enable more than half
> > clocks in CLK_OF_DECLARE_DRIVER hook.
> 
> That's fine.
> 
> >
> > BTW, What's benefit to convert into two parts of probe?
> > I'm not quite if I already get it all, can you help clarify it?
> >
> 
> The benefit is that we still get a platform driver and we can associate a
> device pointer with the clock controller eventually.
> Here's a reply I sent yesterday on the same topic:
> 
> Reasons (in no particular order):
> 
>   1. We get a dev pointer to use with clk_hw_register()
> 
>   2. We can handle probe defer if some resource is not available
> 
>   3. Using device model gets us a hook into power management frameworks
>      like runtime PM and system PM for things like suspend and hibernate
> 
>   4. It encourages a single DT node clk controller style binding
>      instead of a single node per clk style binding
> 
>   5. We can use non-DT specific functions like devm_ioremap_resource() to
> map
>      registers and acquire other resources, leading to more portable and
>      generic code
> 
>   6. We may be able to make the device driver a module, which will
>      make distros happy if we don't have to compile in all
>      these clk drivers to the resulting vmlinux (this one doesn't
>      apply here)
> 

Very clear.
Thanks for the great explanation.

Regards
Dong Aisheng

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

RE: [PATCH 4/9] clk: imx: add pllv4 support

[A.s. Dong]

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@codeaurora.org]
> Sent: Saturday, July 01, 2017 8:37 AM
> To: Dong Aisheng
> Cc: A.s. Dong; linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com;
> shawnguo@kernel.org; Anson Huang; Jacky Bai
> Subject: Re: [PATCH 4/9] clk: imx: add pllv4 support
> 
> On 06/20, Dong Aisheng wrote:
> > On Mon, Jun 19, 2017 at 06:59:17PM -0700, Stephen Boyd wrote:
> > > On 05/15, Dong Aisheng wrote:
> > > > +
> > > > +	if (clk_pllv4_is_enabled(hw)) {
> > > > +		WARN(1, "clk_pllv4: can't change rate when pll is
> enabled");
> > > > +		return -EINVAL;
> > >
> > > Sad, CLK_SET_RATE_GATE isn't working for you I suppose?
> > >
> >
> > CLK_SET_RATE_GATE can't work in early stage before running
> clk_disable_unused.
> > At that point, the clock tree state is still not consistent with HW.
> > e.g. prepare/enable count is still zero but it's actually enabled due
> > to reset state or bootloader.
> >
> > The code here is adding a double check in case user sets rate in early
> stage.
> >
> > However, probably it could also be moved into clock core as it's not
> > platform dependant behavior?
> >
> 
> Ok. It would be good to fix the core framework to synchronize the
> prepared/enabled state at registration time so we don't need this check
> in the driver.
> 

I will prepare a core fix later and remove these checks first.

Regards
Dong Aisheng

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

RE: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support

[A.s. Dong]

> -----Original Message-----
> From: Stephen Boyd [mailto:sboyd@codeaurora.org]
> Sent: Saturday, July 01, 2017 8:56 AM
> To: Dong Aisheng
> Cc: A.s. Dong; linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com;
> shawnguo@kernel.org; Anson Huang; Jacky Bai
> Subject: Re: [PATCH 1/9] clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk
> support
> 
> On 06/20, Dong Aisheng wrote:
> > Hi Stephen,
> >
> > On Mon, Jun 19, 2017 at 06:45:12PM -0700, Stephen Boyd wrote:
> > > On 05/15, Dong Aisheng wrote:
> > > > ---
> > > >  drivers/clk/clk-divider.c    | 2 ++
> > > >  include/linux/clk-provider.h | 4 ++++
> > > >  2 files changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> > > > index 96386ff..f78ba7a 100644
> > > > --- a/drivers/clk/clk-divider.c
> > > > +++ b/drivers/clk/clk-divider.c
> > > > @@ -125,6 +125,8 @@ unsigned long divider_recalc_rate(struct
> > > > clk_hw *hw, unsigned long parent_rate,
> > > >
> > > >  	div = _get_div(table, val, flags, divider->width);
> > > >  	if (!div) {
> > > > +		if (flags & CLK_DIVIDER_ZERO_GATE)
> > > > +			return 0;
> > > >  		WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
> > >
> > > Why not use the CLK_DIVIDER_ALLOW_ZERO flag? A clk being off doesn't
> > > mean the rate is 0. The divider is just disabled, so we would
> > > consider the rate as whatever the parent is, which is what this code
> > > does before this patch. Similarly, we don't do anything about gate
> > > clocks and return a rate of 0 when they're disabled.
> > >
> >
> > The semantic of CLK_DIVIDER_ALLOW_ZERO seems a bit different.
> >
> > See below definition:
> > * CLK_DIVIDER_ALLOW_ZERO - Allow zero divisors.  For dividers which
> have
> > *      CLK_DIVIDER_ONE_BASED set, it is possible to end up with a zero
> divisor.
> > *      Some hardware implementations gracefully handle this case and
> allow a
> > *      zero divisor by not modifying their input clock
> > *      (divide by one / bypass).
> >
> > zero divisor is simply as divide by one or bypass which is supported
> > by hardware.
> >
> > But it's not true for this hardware.
> >
> > If we consider the rate as whatever the parent is if divider is zero,
> > we may got an issue like below:
> > e.g.
> > Assuming spll_bus_clk divider is 0x0 and it may be enabled by users
> > directly without setting a rate first.
> >
> > Then the clock tree looks like:
> > ...
> > spll_pfd0                    1            1   500210526          0 0
> >   spll_pfd_sel              1            1   500210526          0 0
> >     spll_sel               1            1   500210526          0 0
> >       spll_bus_clk           1            1   500210526          0 0
> >
> > But the spll_bus_clk clock rate actually is wrong and it's even not
> > enabled, not like CLK_DIVIDER_ALLOW_ZERO which zero divider means
> simply bypass.
> >
> > So for this case, we probably can't simply assume zero divider rate as
> > its parent, it is actually set to 0 in hw, although it's something
> > like gate, but a bit different from gate as the normal gate does not
> > affect divider where you can keep the rate.
> >
> > How would you suggest for this?
> >
> 
> It seems that set_rate() and enable/disable are conflated here.
> From what you describe, it sounds like the clk is considered off when the
> divider value is zero, and it's on when the divider value is non-zero.
> 
> I'd suggest you make it so this clk supports enable/disable and set_rate
> with the same register. Something like, set rate when the clk is disabled
> will cache the rate request and only when the clk is enabled will the
> driver actually program the hardware to have the requested divider value.
> Similarly, when the clk is disabled we'll write a 0 there, but when the
> clk is enabled we'll restore whatever rate (divider) was chosen last.
> 
> It does mean that recalc rate will be sort of odd, because when the clk
> is off it will return 0, and when the clk is on it will return the right
> rate. So to make things work, we'll need to return the cached rate in
> recalc rate when the clk is off and read the hardware when the clk is on.
> Probably an if register ==
> 0 then lookup in cache, otherwise do normal division.
> 

Well, good suggestions to me.
It makes the implementation of this clock type more comprehensive.

It seems we still need keep CLK_DIVIDER_ZERO_GATE to distinguish with others.

The change for recacle_rate may looks like:
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index f78ba7a..7364ac4 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -125,8 +125,9 @@ unsigned long divider_recalc_rate(struct clk_hw *hw, unsigned long parent_rate,
 
        div = _get_div(table, val, flags, divider->width);
        if (!div) {
+               if ((flags & CLK_DIVIDER_ZERO_GATE) && clk_hw_is_enabled(hw))
+                       return divider->cached_rate;
+
                WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),
                        "%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",
                        clk_hw_get_name(hw));

And for the initial disabled state (div = 0), the calc_rate will still return
0 rate as there's still no set_rate happened.

If any issue, please let me know.

Regards
Dong Aisheng

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