[PATCH 0/5] clk/samsung: Add support for PDMA, EPLL clocks on exynos5410

[PATCH 0/5] clk/samsung: Add support for PDMA, EPLL clocks on exynos5410

[Sylwester Nawrocki]

This patch adds support for the EPLL and the peripheral DMA clocks
on Exynos5410 SoC.  These clocks are required for sound support
on Odroid-XU board.

Sylwester Nawrocki (5):
  clk: samsung: exynos5410: Add clock IDs for PDMA and EPLL clocks
  clk: samsung: exynos5410: Expose the peripheral DMA gate clocks
  clk: samsung: Use common registration function for pll2550x
  clk: samsung: Add support for EPLL on exynos5410
  clk: samsung: Add support for exynos5410 AUDSS clock controller

 .../devicetree/bindings/clock/clk-exynos-audss.txt |   4 +-
 .../devicetree/bindings/clock/exynos5410-clock.txt |  14 ++
 drivers/clk/samsung/clk-exynos-audss.c             |  78 ++++++-----
 drivers/clk/samsung/clk-exynos5410.c               |  32 ++++-
 drivers/clk/samsung/clk-exynos5440.c               |   9 +-
 drivers/clk/samsung/clk-pll.c                      | 154 ++++++++++++++-------
 drivers/clk/samsung/clk-pll.h                      |   2 +
 include/dt-bindings/clock/exynos5410.h             |   3 +
 include/dt-bindings/clock/exynos5440.h             |   2 +
 9 files changed, 212 insertions(+), 86 deletions(-)

--
1.9.1

[PATCH 1/5] clk: samsung: exynos5410: Add clock IDs for PDMA and EPLL clocks

[Sylwester Nawrocki]

The PDMA{0,1} and EPLL clock IDs are added separately in this
patch so the patch can be merged to the arm-soc tree as dependency.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 include/dt-bindings/clock/exynos5410.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/dt-bindings/clock/exynos5410.h b/include/dt-bindings/clock/exynos5410.h
index 85b467b..a74442a 100644
--- a/include/dt-bindings/clock/exynos5410.h
+++ b/include/dt-bindings/clock/exynos5410.h
@@ -19,6 +19,7 @@
 #define CLK_FOUT_MPLL		4
 #define CLK_FOUT_BPLL		5
 #define CLK_FOUT_KPLL		6
+#define CLK_FOUT_EPLL		7
 
 /* gate for special clocks (sclk) */
 #define CLK_SCLK_UART0		128
@@ -48,6 +49,8 @@
 #define CLK_USI3		268
 #define CLK_UART3		260
 #define CLK_PWM			279
+#define CLK_PDMA0		280
+#define CLK_PDMA1		281
 #define CLK_MCT			315
 #define CLK_WDT			316
 #define CLK_RTC			317
-- 
1.9.1

[PATCH 2/5] clk: samsung: exynos5410: Expose the peripheral DMA gate clocks

[Sylwester Nawrocki]

These clocks are needed in order to use the PL330 peripheral
DMA controllers.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/clk/samsung/clk-exynos5410.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/samsung/clk-exynos5410.c b/drivers/clk/samsung/clk-exynos5410.c
index 54ec486..cf6fb41 100644
--- a/drivers/clk/samsung/clk-exynos5410.c
+++ b/drivers/clk/samsung/clk-exynos5410.c
@@ -176,6 +176,8 @@ static const struct samsung_gate_clock exynos5410_gate_clks[] __initconst = {
 	GATE(CLK_MMC0, "sdmmc0", "aclk200", GATE_BUS_FSYS0, 12, 0, 0),
 	GATE(CLK_MMC1, "sdmmc1", "aclk200", GATE_BUS_FSYS0, 13, 0, 0),
 	GATE(CLK_MMC2, "sdmmc2", "aclk200", GATE_BUS_FSYS0, 14, 0, 0),
+	GATE(CLK_PDMA1, "pdma1", "aclk200", GATE_BUS_FSYS0, 2, 0, 0),
+	GATE(CLK_PDMA0, "pdma0", "aclk200", GATE_BUS_FSYS0, 1, 0, 0),
 
 	GATE(CLK_SCLK_USBPHY301, "sclk_usbphy301", "dout_usbphy301",
 	     GATE_TOP_SCLK_FSYS, 7, CLK_SET_RATE_PARENT, 0),
-- 
1.9.1

[PATCH 3/5] clk: samsung: Use common registration function for pll2550x

[Sylwester Nawrocki]

There is no such significant differences in pll2550x PLL type
to justify a separate registration function.  This patch adapts
exynos5440 driver to use the common function and removes
samsung_clk_register_pll2550x().

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
This patch is untested as I don't have access to any exynos5440
SoC based board.
---
 drivers/clk/samsung/clk-exynos5440.c   |  9 ++++--
 drivers/clk/samsung/clk-pll.c          | 52 ++++------------------------------
 drivers/clk/samsung/clk-pll.h          |  1 +
 include/dt-bindings/clock/exynos5440.h |  2 ++
 4 files changed, 15 insertions(+), 49 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5440.c b/drivers/clk/samsung/clk-exynos5440.c
index a57d01b..a80f3ef 100644
--- a/drivers/clk/samsung/clk-exynos5440.c
+++ b/drivers/clk/samsung/clk-exynos5440.c
@@ -112,6 +112,11 @@ static struct notifier_block exynos5440_clk_restart_handler = {
 	.priority = 128,
 };
 
+static const struct samsung_pll_clock exynos5440_plls[] __initconst = {
+	PLL(pll_2550x, CLK_CPLLA, "cplla", "xtal", 0, 0x4c, NULL),
+	PLL(pll_2550x, CLK_CPLLB, "cpllb", "xtal", 0, 0x50, NULL),
+};
+
 /* register exynos5440 clocks */
 static void __init exynos5440_clk_init(struct device_node *np)
 {
@@ -129,8 +134,8 @@ static void __init exynos5440_clk_init(struct device_node *np)
 	samsung_clk_of_register_fixed_ext(ctx, exynos5440_fixed_rate_ext_clks,
 		ARRAY_SIZE(exynos5440_fixed_rate_ext_clks), ext_clk_match);
 
-	samsung_clk_register_pll2550x("cplla", "xtal", reg_base + 0x1c, 0x10);
-	samsung_clk_register_pll2550x("cpllb", "xtal", reg_base + 0x20, 0x10);
+	samsung_clk_register_pll(ctx, exynos5440_plls,
+			ARRAY_SIZE(exynos5440_plls), ctx->reg_base);
 
 	samsung_clk_register_fixed_rate(ctx, exynos5440_fixed_rate_clks,
 			ARRAY_SIZE(exynos5440_fixed_rate_clks));
diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
index 48139bd..b5ab055 100644
--- a/drivers/clk/samsung/clk-pll.c
+++ b/drivers/clk/samsung/clk-pll.c
@@ -890,22 +890,14 @@ static const struct clk_ops samsung_s3c2440_mpll_clk_ops = {
 #define PLL2550X_M_SHIFT      (4)
 #define PLL2550X_S_SHIFT      (0)
 
-struct samsung_clk_pll2550x {
-	struct clk_hw		hw;
-	const void __iomem	*reg_base;
-	unsigned long		offset;
-};
-
-#define to_clk_pll2550x(_hw) container_of(_hw, struct samsung_clk_pll2550x, hw)
-
 static unsigned long samsung_pll2550x_recalc_rate(struct clk_hw *hw,
 				unsigned long parent_rate)
 {
-	struct samsung_clk_pll2550x *pll = to_clk_pll2550x(hw);
+	struct samsung_clk_pll *pll = to_clk_pll(hw);
 	u32 r, p, m, s, pll_stat;
 	u64 fvco = parent_rate;
 
-	pll_stat = readl_relaxed(pll->reg_base + pll->offset * 3);
+	pll_stat = readl_relaxed(pll->con_reg);
 	r = (pll_stat >> PLL2550X_R_SHIFT) & PLL2550X_R_MASK;
 	if (!r)
 		return 0;
@@ -923,43 +915,6 @@ static const struct clk_ops samsung_pll2550x_clk_ops = {
 	.recalc_rate = samsung_pll2550x_recalc_rate,
 };
 
-struct clk * __init samsung_clk_register_pll2550x(const char *name,
-			const char *pname, const void __iomem *reg_base,
-			const unsigned long offset)
-{
-	struct samsung_clk_pll2550x *pll;
-	struct clk *clk;
-	struct clk_init_data init;
-
-	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
-	if (!pll) {
-		pr_err("%s: could not allocate pll clk %s\n", __func__, name);
-		return NULL;
-	}
-
-	init.name = name;
-	init.ops = &samsung_pll2550x_clk_ops;
-	init.flags = CLK_GET_RATE_NOCACHE;
-	init.parent_names = &pname;
-	init.num_parents = 1;
-
-	pll->hw.init = &init;
-	pll->reg_base = reg_base;
-	pll->offset = offset;
-
-	clk = clk_register(NULL, &pll->hw);
-	if (IS_ERR(clk)) {
-		pr_err("%s: failed to register pll clock %s\n", __func__,
-				name);
-		kfree(pll);
-	}
-
-	if (clk_register_clkdev(clk, name, NULL))
-		pr_err("%s: failed to register lookup for %s", __func__, name);
-
-	return clk;
-}
-
 /*
  * PLL2550xx Clock Type
  */
@@ -1263,6 +1218,9 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx,
 		else
 			init.ops = &samsung_s3c2440_mpll_clk_ops;
 		break;
+	case pll_2550x:
+		init.ops = &samsung_pll2550x_clk_ops;
+		break;
 	case pll_2550xx:
 		if (!pll->rate_table)
 			init.ops = &samsung_pll2550xx_clk_min_ops;
diff --git a/drivers/clk/samsung/clk-pll.h b/drivers/clk/samsung/clk-pll.h
index 213de9a..df4ad8a 100644
--- a/drivers/clk/samsung/clk-pll.h
+++ b/drivers/clk/samsung/clk-pll.h
@@ -31,6 +31,7 @@ enum samsung_pll_type {
 	pll_s3c2410_mpll,
 	pll_s3c2410_upll,
 	pll_s3c2440_mpll,
+	pll_2550x,
 	pll_2550xx,
 	pll_2650xx,
 	pll_1450x,
diff --git a/include/dt-bindings/clock/exynos5440.h b/include/dt-bindings/clock/exynos5440.h
index c66fc40..842cdc0 100644
--- a/include/dt-bindings/clock/exynos5440.h
+++ b/include/dt-bindings/clock/exynos5440.h
@@ -14,6 +14,8 @@
 
 #define CLK_XTAL		1
 #define CLK_ARM_CLK		2
+#define CLK_CPLLA		3
+#define CLK_CPLLB		4
 #define CLK_SPI_BAUD		16
 #define CLK_PB0_250		17
 #define CLK_PR0_250		18
-- 
1.9.1

[PATCH 4/5] clk: samsung: Add support for EPLL on exynos5410

[Sylwester Nawrocki]

This patch adds code instantiating the EPLL, which is used as the
audio subsystem's root clock.
The requirement to specify the external root clock in clocks property
is also added.  That ensures proper initialization order by explicitly
specifying dependencies in devicetree.  It prevents situations when the
SoC's clock controller driver has initialized, the external oscillator
clock is not yet registered and setting clock frequencies through
assigned-clock-rates property doesn't work properly due to unknown
external oscillator frequency.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 .../devicetree/bindings/clock/exynos5410-clock.txt |  14 +++
 drivers/clk/samsung/clk-exynos5410.c               |  30 +++++-
 drivers/clk/samsung/clk-pll.c                      | 102 +++++++++++++++++++++
 drivers/clk/samsung/clk-pll.h                      |   1 +
 4 files changed, 145 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
index aeab635..2aefc07 100644
--- a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
+++ b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
@@ -12,6 +12,9 @@ Required Properties:
 
 - #clock-cells: should be 1.
 
+- clocks: should contain an entry specifying the root oscillator clock
+  on XXTI pin (fin_pll).
+
 All available clocks are defined as preprocessor macros in
 dt-bindings/clock/exynos5410.h header and can be used in device
 tree sources.
@@ -24,12 +27,23 @@ with following clock-output-name:
 
  - "fin_pll" - PLL input clock from XXTI
 
+This clock should be listed in the clocks property of the controller node.
+
+
 Example 1: An example of a clock controller node is listed below.
 
+	fin_pll: xxti {
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+		clock-output-names = "fin_pll";
+		#clock-cells = <0>;
+	};
+
 	clock: clock-controller@0x10010000 {
 		compatible = "samsung,exynos5410-clock";
 		reg = <0x10010000 0x30000>;
 		#clock-cells = <1>;
+		clocks = <&fin_pll>;
 	};
 
 Example 2: UART controller node that consumes the clock generated by the clock
diff --git a/drivers/clk/samsung/clk-exynos5410.c b/drivers/clk/samsung/clk-exynos5410.c
index cf6fb41..113ce7d 100644
--- a/drivers/clk/samsung/clk-exynos5410.c
+++ b/drivers/clk/samsung/clk-exynos5410.c
@@ -14,6 +14,7 @@
 #include <linux/clk-provider.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/clk.h>
 
 #include "clk.h"
 
@@ -21,6 +22,8 @@
 #define APLL_CON0               0x100
 #define CPLL_LOCK               0x10020
 #define CPLL_CON0               0x10120
+#define EPLL_LOCK               0x10040
+#define EPLL_CON0               0x10130
 #define MPLL_LOCK               0x4000
 #define MPLL_CON0               0x4100
 #define BPLL_LOCK               0x20010
@@ -58,7 +61,7 @@
 
 /* list of PLLs */
 enum exynos5410_plls {
-	apll, cpll, mpll,
+	apll, cpll, epll, mpll,
 	bpll, kpll,
 	nr_plls                 /* number of PLLs */
 };
@@ -67,6 +70,7 @@ enum exynos5410_plls {
 PNAME(apll_p)		= { "fin_pll", "fout_apll", };
 PNAME(bpll_p)		= { "fin_pll", "fout_bpll", };
 PNAME(cpll_p)		= { "fin_pll", "fout_cpll" };
+PNAME(epll_p)		= { "fin_pll", "fout_epll" };
 PNAME(mpll_p)		= { "fin_pll", "fout_mpll", };
 PNAME(kpll_p)		= { "fin_pll", "fout_kpll", };
 
@@ -95,6 +99,8 @@ static const struct samsung_mux_clock exynos5410_mux_clks[] __initconst = {
 	MUX(0, "sclk_bpll", bpll_p, SRC_CDREX, 0, 1),
 	MUX(0, "sclk_bpll_muxed", bpll_user_p, SRC_TOP2, 24, 1),
 
+	MUX(0, "sclk_epll", epll_p, SRC_TOP2, 12, 1),
+
 	MUX(0, "sclk_cpll", cpll_p, SRC_TOP2, 8, 1),
 
 	MUX(0, "sclk_mpll_bpll", mpll_bpll_p, SRC_TOP1, 20, 1),
@@ -219,11 +225,26 @@ static const struct samsung_gate_clock exynos5410_gate_clks[] __initconst = {
 	GATE(CLK_USBD301, "usbd301", "aclk200_fsys", GATE_IP_FSYS, 20, 0, 0),
 };
 
-static const struct samsung_pll_clock exynos5410_plls[nr_plls] __initconst = {
+static const struct samsung_pll_rate_table exynos5410_pll2550x_24mhz_tbl[] __initconst = {
+	PLL_36XX_RATE(400000000U, 200, 2, 2, 0),
+	PLL_36XX_RATE(333000000U, 111, 2, 2, 0),
+	PLL_36XX_RATE(300000000U, 100, 2, 2, 0),
+	PLL_36XX_RATE(266000000U, 266, 3, 3, 0),
+	PLL_36XX_RATE(200000000U, 200, 3, 3, 0),
+	PLL_36XX_RATE(192000000U, 192, 3, 3, 0),
+	PLL_36XX_RATE(166000000U, 166, 3, 3, 0),
+	PLL_36XX_RATE(133000000U, 266, 3, 4, 0),
+	PLL_36XX_RATE(100000000U, 200, 3, 4, 0),
+	PLL_36XX_RATE(66000000U,  176, 2, 5, 0),
+};
+
+static struct samsung_pll_clock exynos5410_plls[nr_plls] __initdata = {
 	[apll] = PLL(pll_35xx, CLK_FOUT_APLL, "fout_apll", "fin_pll", APLL_LOCK,
 		APLL_CON0, NULL),
 	[cpll] = PLL(pll_35xx, CLK_FOUT_CPLL, "fout_cpll", "fin_pll", CPLL_LOCK,
 		CPLL_CON0, NULL),
+	[epll] = PLL(pll_2650x, CLK_FOUT_EPLL, "fout_epll", "fin_pll", EPLL_LOCK,
+		EPLL_CON0, NULL),
 	[mpll] = PLL(pll_35xx, CLK_FOUT_MPLL, "fout_mpll", "fin_pll", MPLL_LOCK,
 		MPLL_CON0, NULL),
 	[bpll] = PLL(pll_35xx, CLK_FOUT_BPLL, "fout_bpll", "fin_pll", BPLL_LOCK,
@@ -237,6 +258,7 @@ static void __init exynos5410_clk_init(struct device_node *np)
 {
 	struct samsung_clk_provider *ctx;
 	void __iomem *reg_base;
+	struct clk *xxti;
 
 	reg_base = of_iomap(np, 0);
 	if (!reg_base)
@@ -244,6 +266,10 @@ static void __init exynos5410_clk_init(struct device_node *np)
 
 	ctx = samsung_clk_init(np, reg_base, CLK_NR_CLKS);
 
+	xxti = of_clk_get(np, 0);
+	if (!IS_ERR(xxti) && clk_get_rate(xxti) == 24 * MHZ)
+		exynos5410_plls[epll].rate_table = exynos5410_pll2550x_24mhz_tbl;
+
 	samsung_clk_register_pll(ctx, exynos5410_plls,
 			ARRAY_SIZE(exynos5410_plls), reg_base);
 
diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
index b5ab055..d61fd80 100644
--- a/drivers/clk/samsung/clk-pll.c
+++ b/drivers/clk/samsung/clk-pll.c
@@ -1018,6 +1018,102 @@ static const struct clk_ops samsung_pll2550xx_clk_min_ops = {
 };
 
 /*
+ * PLL2650x Clock Type
+ */
+
+/* Maximum lock time can be 3000 * PDIV cycles */
+#define PLL2650X_LOCK_FACTOR		3000
+
+#define PLL2650X_M_MASK			0x3FF
+#define PLL2650X_P_MASK			0x3F
+#define PLL2650X_S_MASK			0x7
+#define PLL2650X_K_MASK			0xFFFF
+#define PLL2650X_LOCK_STAT_MASK		0x1
+#define PLL2650X_M_SHIFT		16
+#define PLL2650X_P_SHIFT		8
+#define PLL2650X_S_SHIFT		0
+#define PLL2650X_K_SHIFT		0
+#define PLL2650X_LOCK_STAT_SHIFT	29
+#define PLL2650X_PLL_ENABLE_SHIFT	31
+
+static unsigned long samsung_pll2650x_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct samsung_clk_pll *pll = to_clk_pll(hw);
+	u64 fout = parent_rate;
+	u32 mdiv, pdiv, sdiv, pll_con0, pll_con1;
+	s16 kdiv;
+
+	pll_con0 = readl_relaxed(pll->con_reg);
+	mdiv = (pll_con0 >> PLL2650X_M_SHIFT) & PLL2650X_M_MASK;
+	pdiv = (pll_con0 >> PLL2650X_P_SHIFT) & PLL2650X_P_MASK;
+	sdiv = (pll_con0 >> PLL2650X_S_SHIFT) & PLL2650X_S_MASK;
+
+	pll_con1 = readl_relaxed(pll->con_reg + 4);
+	kdiv = (s16)((pll_con1 >> PLL2650X_K_SHIFT) & PLL2650X_K_MASK);
+
+	fout *= (mdiv << 16) + kdiv;
+	do_div(fout, (pdiv << sdiv));
+	fout >>= 16;
+
+	return (unsigned long)fout;
+}
+
+static int samsung_pll2650x_set_rate(struct clk_hw *hw, unsigned long drate,
+					unsigned long prate)
+{
+	struct samsung_clk_pll *pll = to_clk_pll(hw);
+	const struct samsung_pll_rate_table *rate;
+	u32 con0, con1;
+
+	/* Get required rate settings from table */
+	rate = samsung_get_pll_settings(pll, drate);
+	if (!rate) {
+		pr_err("%s: Invalid rate : %lu for pll clk %s\n", __func__,
+			drate, clk_hw_get_name(hw));
+		return -EINVAL;
+	}
+
+	con0 = readl_relaxed(pll->con_reg);
+	con1 = readl_relaxed(pll->con_reg + 4);
+
+	/* Set PLL lock time. */
+	writel_relaxed(rate->pdiv * PLL2650X_LOCK_FACTOR, pll->lock_reg);
+
+	/* Change PLL PMS values */
+	con0 &= ~((PLL2650X_M_MASK << PLL2650X_M_SHIFT) |
+			(PLL2650X_P_MASK << PLL2650X_P_SHIFT) |
+			(PLL2650X_S_MASK << PLL2650X_S_SHIFT));
+	con0 |= (rate->mdiv << PLL2650X_M_SHIFT) |
+			(rate->pdiv << PLL2650X_P_SHIFT) |
+			(rate->sdiv << PLL2650X_S_SHIFT);
+	con0 |= (1 << PLL2650X_PLL_ENABLE_SHIFT);
+	writel_relaxed(con0, pll->con_reg);
+
+	con1 &= ~(PLL2650X_K_MASK << PLL2650X_K_SHIFT);
+	con1 |= ((rate->kdiv & PLL2650X_K_MASK) << PLL2650X_K_SHIFT);
+	writel_relaxed(con1, pll->con_reg + 4);
+
+	do {
+		cpu_relax();
+		con0 = readl_relaxed(pll->con_reg);
+	} while (!(con0 & (PLL2650X_LOCK_STAT_MASK
+			<< PLL2650X_LOCK_STAT_SHIFT)));
+
+	return 0;
+}
+
+static const struct clk_ops samsung_pll2650x_clk_ops = {
+	.recalc_rate = samsung_pll2650x_recalc_rate,
+	.round_rate = samsung_pll_round_rate,
+	.set_rate = samsung_pll2650x_set_rate,
+};
+
+static const struct clk_ops samsung_pll2650x_clk_min_ops = {
+	.recalc_rate = samsung_pll2650x_recalc_rate,
+};
+
+/*
  * PLL2650XX Clock Type
  */
 
@@ -1227,6 +1323,12 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx,
 		else
 			init.ops = &samsung_pll2550xx_clk_ops;
 		break;
+	case pll_2650x:
+		if (!pll->rate_table)
+			init.ops = &samsung_pll2650x_clk_min_ops;
+		else
+			init.ops = &samsung_pll2650x_clk_ops;
+		break;
 	case pll_2650xx:
 		if (!pll->rate_table)
 			init.ops = &samsung_pll2650xx_clk_min_ops;
diff --git a/drivers/clk/samsung/clk-pll.h b/drivers/clk/samsung/clk-pll.h
index df4ad8a..a1ca023 100644
--- a/drivers/clk/samsung/clk-pll.h
+++ b/drivers/clk/samsung/clk-pll.h
@@ -33,6 +33,7 @@ enum samsung_pll_type {
 	pll_s3c2440_mpll,
 	pll_2550x,
 	pll_2550xx,
+	pll_2650x,
 	pll_2650xx,
 	pll_1450x,
 	pll_1451x,
-- 
1.9.1

[PATCH 5/5] clk: samsung: Add support for exynos5410 AUDSS clock controller

[Sylwester Nawrocki]

Exynos5410 Audio Subsystem Clock Controller, comparing to the already
supported by the driver IP block revisions, has additionally an I2S_MST
divider so a new compatible string is added.
While at it, the variant handling is reworked to avoid scattered testing
for the controller's variant.
The I2S_MST divider clock will be added in a subsequent patch.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 .../devicetree/bindings/clock/clk-exynos-audss.txt |  4 +-
 drivers/clk/samsung/clk-exynos-audss.c             | 78 ++++++++++++----------
 2 files changed, 47 insertions(+), 35 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt b/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt
index 180e883..0c3d601 100644
--- a/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt
+++ b/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt
@@ -10,6 +10,8 @@ Required Properties:
   - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.
   - "samsung,exynos5250-audss-clock" - controller compatible with Exynos5250
     SoCs.
+  - "samsung,exynos5410-audss-clock" - controller compatible with Exynos5410
+    SoCs.
   - "samsung,exynos5420-audss-clock" - controller compatible with Exynos5420
     SoCs.
 - reg: physical base address and length of the controller's register set.
@@ -91,5 +93,5 @@ i2s0: i2s@03830000 {
 		<&clock_audss EXYNOS_MOUT_AUDSS>,
 		<&clock_audss EXYNOS_MOUT_I2S>;
 	clock-names = "iis", "i2s_opclk0", "i2s_opclk1",
-	"mout_audss", "mout_i2s";
+		      "mout_audss", "mout_i2s";
 };
diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
index bdf8b97..328260c 100644
--- a/drivers/clk/samsung/clk-exynos-audss.c
+++ b/drivers/clk/samsung/clk-exynos-audss.c
@@ -14,18 +14,13 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/syscore_ops.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 
 #include <dt-bindings/clock/exynos-audss-clk.h>
 
-enum exynos_audss_clk_type {
-	TYPE_EXYNOS4210,
-	TYPE_EXYNOS5250,
-	TYPE_EXYNOS5420,
-};
-
 static DEFINE_SPINLOCK(lock);
 static struct clk **clk_table;
 static void __iomem *reg_base;
@@ -44,9 +39,9 @@ static struct clk *epll;
 
 #ifdef CONFIG_PM_SLEEP
 static unsigned long reg_save[][2] = {
-	{ASS_CLK_SRC,  0},
-	{ASS_CLK_DIV,  0},
-	{ASS_CLK_GATE, 0},
+	{ ASS_CLK_SRC,  0 },
+	{ ASS_CLK_DIV,  0 },
+	{ ASS_CLK_GATE, 0 },
 };
 
 static int exynos_audss_clk_suspend(void)
@@ -73,14 +68,37 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 };
 #endif /* CONFIG_PM_SLEEP */
 
+struct exynos_audss_clk_drvdata {
+	unsigned int has_adma_clk:1;
+	unsigned int enable_epll:1;
+	unsigned int num_clks;
+};
+
+static const struct exynos_audss_clk_drvdata exynos4210_drvdata = {
+	.num_clks	= EXYNOS_AUDSS_MAX_CLKS - 1,
+};
+
+static const struct exynos_audss_clk_drvdata exynos5420_drvdata = {
+	.num_clks	= EXYNOS_AUDSS_MAX_CLKS,
+	.has_adma_clk	= 1,
+	.enable_epll	= 1,
+};
+
 static const struct of_device_id exynos_audss_clk_of_match[] = {
-	{ .compatible = "samsung,exynos4210-audss-clock",
-	  .data = (void *)TYPE_EXYNOS4210, },
-	{ .compatible = "samsung,exynos5250-audss-clock",
-	  .data = (void *)TYPE_EXYNOS5250, },
-	{ .compatible = "samsung,exynos5420-audss-clock",
-	  .data = (void *)TYPE_EXYNOS5420, },
-	{},
+	{
+		.compatible	= "samsung,exynos4210-audss-clock",
+		.data		= &exynos4210_drvdata,
+	}, {
+		.compatible	= "samsung,exynos5250-audss-clock",
+		.data		= &exynos4210_drvdata,
+	}, {
+		.compatible	= "samsung,exynos5410-audss-clock",
+		.data		= &exynos4210_drvdata,
+	}, {
+		.compatible	= "samsung,exynos5420-audss-clock",
+		.data		= &exynos5420_drvdata,
+	},
+	{ },
 };
 
 static void exynos_audss_clk_teardown(void)
@@ -106,19 +124,17 @@ static void exynos_audss_clk_teardown(void)
 /* register exynos_audss clocks */
 static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
-	int i, ret = 0;
-	struct resource *res;
 	const char *mout_audss_p[] = {"fin_pll", "fout_epll"};
 	const char *mout_i2s_p[] = {"mout_audss", "cdclk0", "sclk_audio0"};
 	const char *sclk_pcm_p = "sclk_pcm0";
 	struct clk *pll_ref, *pll_in, *cdclk, *sclk_audio, *sclk_pcm_in;
-	const struct of_device_id *match;
-	enum exynos_audss_clk_type variant;
+	const struct exynos_audss_clk_drvdata *variant;
+	struct resource *res;
+	int i, ret = 0;
 
-	match = of_match_node(exynos_audss_clk_of_match, pdev->dev.of_node);
-	if (!match)
+	variant = of_device_get_match_data(&pdev->dev);
+	if (!variant)
 		return -EINVAL;
-	variant = (enum exynos_audss_clk_type)match->data;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	reg_base = devm_ioremap_resource(&pdev->dev, res);
@@ -126,7 +142,7 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "failed to map audss registers\n");
 		return PTR_ERR(reg_base);
 	}
-	/* EPLL don't have to be enabled for boards other than Exynos5420 */
+
 	epll = ERR_PTR(-ENODEV);
 
 	clk_table = devm_kzalloc(&pdev->dev,
@@ -136,10 +152,7 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	clk_data.clks = clk_table;
-	if (variant == TYPE_EXYNOS5420)
-		clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
-	else
-		clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS - 1;
+	clk_data.clk_num = variant->num_clks;
 
 	pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
 	pll_in = devm_clk_get(&pdev->dev, "pll_in");
@@ -148,13 +161,13 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	if (!IS_ERR(pll_in)) {
 		mout_audss_p[1] = __clk_get_name(pll_in);
 
-		if (variant == TYPE_EXYNOS5420) {
+		if (variant->enable_epll) {
 			epll = pll_in;
 
 			ret = clk_prepare_enable(epll);
 			if (ret) {
 				dev_err(&pdev->dev,
-						"failed to prepare the epll clock\n");
+					"failed to prepare the epll clock\n");
 				return ret;
 			}
 		}
@@ -210,7 +223,7 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
-	if (variant == TYPE_EXYNOS5420) {
+	if (variant->has_adma_clk) {
 		clk_table[EXYNOS_ADMA] = clk_register_gate(NULL, "adma",
 				"dout_srp", CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 9, 0, &lock);
@@ -234,9 +247,6 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 	register_syscore_ops(&exynos_audss_clk_syscore_ops);
 #endif
-
-	dev_info(&pdev->dev, "setup completed\n");
-
 	return 0;
 
 unregister:
-- 
1.9.1

Re: [PATCH 1/5] clk: samsung: exynos5410: Add clock IDs for PDMA and EPLL clocks

[Krzysztof Kozlowski]

On 08/22/2016 06:30 PM, Sylwester Nawrocki wrote:
> The PDMA{0,1} and EPLL clock IDs are added separately in this
> patch so the patch can be merged to the arm-soc tree as dependency.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  include/dt-bindings/clock/exynos5410.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/dt-bindings/clock/exynos5410.h b/include/dt-bindings/clock/exynos5410.h
> index 85b467b..a74442a 100644
> --- a/include/dt-bindings/clock/exynos5410.h
> +++ b/include/dt-bindings/clock/exynos5410.h
> @@ -19,6 +19,7 @@
>  #define CLK_FOUT_MPLL		4
>  #define CLK_FOUT_BPLL		5
>  #define CLK_FOUT_KPLL		6
> +#define CLK_FOUT_EPLL		7
>  
>  /* gate for special clocks (sclk) */
>  #define CLK_SCLK_UART0		128
> @@ -48,6 +49,8 @@
>  #define CLK_USI3		268
>  #define CLK_UART3		260
>  #define CLK_PWM			279
> +#define CLK_PDMA0		280
> +#define CLK_PDMA1		281

How about making the IDs the same as in exynos5420? This way those
drivers could be merged someday in the future (if someone would be
bored...).

Best regards,
Krzysztof

>  #define CLK_MCT			315
>  #define CLK_WDT			316
>  #define CLK_RTC			317
> 

Re: [PATCH 2/5] clk: samsung: exynos5410: Expose the peripheral DMA gate clocks

[Krzysztof Kozlowski]

On 08/22/2016 06:31 PM, Sylwester Nawrocki wrote:
> These clocks are needed in order to use the PL330 peripheral
> DMA controllers.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos5410.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH 3/5] clk: samsung: Use common registration function for pll2550x

[Krzysztof Kozlowski]

On 08/22/2016 06:31 PM, Sylwester Nawrocki wrote:
> There is no such significant differences in pll2550x PLL type
> to justify a separate registration function.  This patch adapts
> exynos5440 driver to use the common function and removes
> samsung_clk_register_pll2550x().
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> This patch is untested as I don't have access to any exynos5440
> SoC based board.
> ---
>  drivers/clk/samsung/clk-exynos5440.c   |  9 ++++--
>  drivers/clk/samsung/clk-pll.c          | 52 ++++------------------------------
>  drivers/clk/samsung/clk-pll.h          |  1 +
>  include/dt-bindings/clock/exynos5440.h |  2 ++
>  4 files changed, 15 insertions(+), 49 deletions(-)
> 

Looks correct although testing would be nice.

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH 4/5] clk: samsung: Add support for EPLL on exynos5410

[Krzysztof Kozlowski]

On 08/22/2016 06:31 PM, Sylwester Nawrocki wrote:
> This patch adds code instantiating the EPLL, which is used as the
> audio subsystem's root clock.
> The requirement to specify the external root clock in clocks property
> is also added.

I think the requirement was there already but explained little bit
differently:
19 External clock:
20
21 There is clock that is generated outside the SoC. It
22 is expected that it is defined using standard clock bindings
23 with following clock-output-name:
24
25  - "fin_pll" - PLL input clock from XXTI

so you can just combine them. Driver now will require the fin_pll in two
ways:
1. Old lookup by "fin_pll" name.
2. of_clk_get of yours.


>  That ensures proper initialization order by explicitly
> specifying dependencies in devicetree.  It prevents situations when the
> SoC's clock controller driver has initialized, the external oscillator
> clock is not yet registered and setting clock frequencies through
> assigned-clock-rates property doesn't work properly due to unknown
> external oscillator frequency.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  .../devicetree/bindings/clock/exynos5410-clock.txt |  14 +++
>  drivers/clk/samsung/clk-exynos5410.c               |  30 +++++-
>  drivers/clk/samsung/clk-pll.c                      | 102 +++++++++++++++++++++
>  drivers/clk/samsung/clk-pll.h                      |   1 +
>  4 files changed, 145 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> index aeab635..2aefc07 100644
> --- a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> @@ -12,6 +12,9 @@ Required Properties:
>  
>  - #clock-cells: should be 1.
>  
> +- clocks: should contain an entry specifying the root oscillator clock
> +  on XXTI pin (fin_pll).

Combine with "external clock".

Rest looks ok:

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH 5/5] clk: samsung: Add support for exynos5410 AUDSS clock controller

[Krzysztof Kozlowski]

On 08/22/2016 06:31 PM, Sylwester Nawrocki wrote:
> Exynos5410 Audio Subsystem Clock Controller, comparing to the already
> supported by the driver IP block revisions, has additionally an I2S_MST
> divider so a new compatible string is added.
> While at it, the variant handling is reworked to avoid scattered testing
> for the controller's variant.

Please split it into simple changes:
1. Whitespace cleanup (below you do it).
2. Variant handling rework.
3. New compatible added.

This would make easy to spot changes and new features.

Best regards,
Krzysztof

> The I2S_MST divider clock will be added in a subsequent patch.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  .../devicetree/bindings/clock/clk-exynos-audss.txt |  4 +-
>  drivers/clk/samsung/clk-exynos-audss.c             | 78 ++++++++++++----------
>  2 files changed, 47 insertions(+), 35 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt b/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt
> index 180e883..0c3d601 100644
> --- a/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt
> +++ b/Documentation/devicetree/bindings/clock/clk-exynos-audss.txt
> @@ -10,6 +10,8 @@ Required Properties:
>    - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.
>    - "samsung,exynos5250-audss-clock" - controller compatible with Exynos5250
>      SoCs.
> +  - "samsung,exynos5410-audss-clock" - controller compatible with Exynos5410
> +    SoCs.
>    - "samsung,exynos5420-audss-clock" - controller compatible with Exynos5420
>      SoCs.
>  - reg: physical base address and length of the controller's register set.
> @@ -91,5 +93,5 @@ i2s0: i2s@03830000 {
>  		<&clock_audss EXYNOS_MOUT_AUDSS>,
>  		<&clock_audss EXYNOS_MOUT_I2S>;
>  	clock-names = "iis", "i2s_opclk0", "i2s_opclk1",
> -	"mout_audss", "mout_i2s";
> +		      "mout_audss", "mout_i2s";
>  };
> diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
> index bdf8b97..328260c 100644
> --- a/drivers/clk/samsung/clk-exynos-audss.c
> +++ b/drivers/clk/samsung/clk-exynos-audss.c
> @@ -14,18 +14,13 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/of_address.h>
> +#include <linux/of_device.h>
>  #include <linux/syscore_ops.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  
>  #include <dt-bindings/clock/exynos-audss-clk.h>
>  
> -enum exynos_audss_clk_type {
> -	TYPE_EXYNOS4210,
> -	TYPE_EXYNOS5250,
> -	TYPE_EXYNOS5420,
> -};
> -
>  static DEFINE_SPINLOCK(lock);
>  static struct clk **clk_table;
>  static void __iomem *reg_base;
> @@ -44,9 +39,9 @@ static struct clk *epll;
>  
>  #ifdef CONFIG_PM_SLEEP
>  static unsigned long reg_save[][2] = {
> -	{ASS_CLK_SRC,  0},
> -	{ASS_CLK_DIV,  0},
> -	{ASS_CLK_GATE, 0},
> +	{ ASS_CLK_SRC,  0 },
> +	{ ASS_CLK_DIV,  0 },
> +	{ ASS_CLK_GATE, 0 },
>  };
>  
>  static int exynos_audss_clk_suspend(void)
> @@ -73,14 +68,37 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
>  };
>  #endif /* CONFIG_PM_SLEEP */
>  
> +struct exynos_audss_clk_drvdata {
> +	unsigned int has_adma_clk:1;
> +	unsigned int enable_epll:1;
> +	unsigned int num_clks;
> +};
> +
> +static const struct exynos_audss_clk_drvdata exynos4210_drvdata = {
> +	.num_clks	= EXYNOS_AUDSS_MAX_CLKS - 1,
> +};
> +
> +static const struct exynos_audss_clk_drvdata exynos5420_drvdata = {
> +	.num_clks	= EXYNOS_AUDSS_MAX_CLKS,
> +	.has_adma_clk	= 1,
> +	.enable_epll	= 1,
> +};
> +
>  static const struct of_device_id exynos_audss_clk_of_match[] = {
> -	{ .compatible = "samsung,exynos4210-audss-clock",
> -	  .data = (void *)TYPE_EXYNOS4210, },
> -	{ .compatible = "samsung,exynos5250-audss-clock",
> -	  .data = (void *)TYPE_EXYNOS5250, },
> -	{ .compatible = "samsung,exynos5420-audss-clock",
> -	  .data = (void *)TYPE_EXYNOS5420, },
> -	{},
> +	{
> +		.compatible	= "samsung,exynos4210-audss-clock",
> +		.data		= &exynos4210_drvdata,
> +	}, {
> +		.compatible	= "samsung,exynos5250-audss-clock",
> +		.data		= &exynos4210_drvdata,
> +	}, {
> +		.compatible	= "samsung,exynos5410-audss-clock",
> +		.data		= &exynos4210_drvdata,
> +	}, {
> +		.compatible	= "samsung,exynos5420-audss-clock",
> +		.data		= &exynos5420_drvdata,
> +	},
> +	{ },
>  };
>  
>  static void exynos_audss_clk_teardown(void)
> @@ -106,19 +124,17 @@ static void exynos_audss_clk_teardown(void)
>  /* register exynos_audss clocks */
>  static int exynos_audss_clk_probe(struct platform_device *pdev)
>  {
> -	int i, ret = 0;
> -	struct resource *res;
>  	const char *mout_audss_p[] = {"fin_pll", "fout_epll"};
>  	const char *mout_i2s_p[] = {"mout_audss", "cdclk0", "sclk_audio0"};
>  	const char *sclk_pcm_p = "sclk_pcm0";
>  	struct clk *pll_ref, *pll_in, *cdclk, *sclk_audio, *sclk_pcm_in;
> -	const struct of_device_id *match;
> -	enum exynos_audss_clk_type variant;
> +	const struct exynos_audss_clk_drvdata *variant;
> +	struct resource *res;
> +	int i, ret = 0;
>  
> -	match = of_match_node(exynos_audss_clk_of_match, pdev->dev.of_node);
> -	if (!match)
> +	variant = of_device_get_match_data(&pdev->dev);
> +	if (!variant)
>  		return -EINVAL;
> -	variant = (enum exynos_audss_clk_type)match->data;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	reg_base = devm_ioremap_resource(&pdev->dev, res);
> @@ -126,7 +142,7 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "failed to map audss registers\n");
>  		return PTR_ERR(reg_base);
>  	}
> -	/* EPLL don't have to be enabled for boards other than Exynos5420 */
> +
>  	epll = ERR_PTR(-ENODEV);
>  
>  	clk_table = devm_kzalloc(&pdev->dev,
> @@ -136,10 +152,7 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	clk_data.clks = clk_table;
> -	if (variant == TYPE_EXYNOS5420)
> -		clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
> -	else
> -		clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS - 1;
> +	clk_data.clk_num = variant->num_clks;
>  
>  	pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
>  	pll_in = devm_clk_get(&pdev->dev, "pll_in");
> @@ -148,13 +161,13 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
>  	if (!IS_ERR(pll_in)) {
>  		mout_audss_p[1] = __clk_get_name(pll_in);
>  
> -		if (variant == TYPE_EXYNOS5420) {
> +		if (variant->enable_epll) {
>  			epll = pll_in;
>  
>  			ret = clk_prepare_enable(epll);
>  			if (ret) {
>  				dev_err(&pdev->dev,
> -						"failed to prepare the epll clock\n");
> +					"failed to prepare the epll clock\n");
>  				return ret;
>  			}
>  		}
> @@ -210,7 +223,7 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
>  				sclk_pcm_p, CLK_SET_RATE_PARENT,
>  				reg_base + ASS_CLK_GATE, 5, 0, &lock);
>  
> -	if (variant == TYPE_EXYNOS5420) {
> +	if (variant->has_adma_clk) {
>  		clk_table[EXYNOS_ADMA] = clk_register_gate(NULL, "adma",
>  				"dout_srp", CLK_SET_RATE_PARENT,
>  				reg_base + ASS_CLK_GATE, 9, 0, &lock);
> @@ -234,9 +247,6 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
>  #ifdef CONFIG_PM_SLEEP
>  	register_syscore_ops(&exynos_audss_clk_syscore_ops);
>  #endif
> -
> -	dev_info(&pdev->dev, "setup completed\n");
> -
>  	return 0;
>  
>  unregister:
> 

Re: [PATCH 1/5] clk: samsung: exynos5410: Add clock IDs for PDMA and EPLL clocks

[Sylwester Nawrocki]

On 08/30/2016 11:54 AM, Krzysztof Kozlowski wrote:
> On 08/22/2016 06:30 PM, Sylwester Nawrocki wrote:
>> > The PDMA{0,1} and EPLL clock IDs are added separately in this
>> > patch so the patch can be merged to the arm-soc tree as dependency.
>> > 
>> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> > ---
>> >  include/dt-bindings/clock/exynos5410.h | 3 +++
>> >  1 file changed, 3 insertions(+)
>> > 
>> > diff --git a/include/dt-bindings/clock/exynos5410.h b/include/dt-bindings/clock/exynos5410.h
>> > index 85b467b..a74442a 100644
>> > --- a/include/dt-bindings/clock/exynos5410.h
>> > +++ b/include/dt-bindings/clock/exynos5410.h
>> > @@ -19,6 +19,7 @@
>> >  #define CLK_FOUT_MPLL		4
>> >  #define CLK_FOUT_BPLL		5
>> >  #define CLK_FOUT_KPLL		6
>> > +#define CLK_FOUT_EPLL		7
>> >  
>> >  /* gate for special clocks (sclk) */
>> >  #define CLK_SCLK_UART0		128
>> > @@ -48,6 +49,8 @@
>> >  #define CLK_USI3		268
>> >  #define CLK_UART3		260
>> >  #define CLK_PWM			279
>> > +#define CLK_PDMA0		280
>> > +#define CLK_PDMA1		281
>
> How about making the IDs the same as in exynos5420? This way those
> drivers could be merged someday in the future (if someone would be
> bored...).

OK, I will do it, but there is already mismatch in the PLL indices.

--
Thanks,
Sylwester

Re: [PATCH 4/5] clk: samsung: Add support for EPLL on exynos5410

[Sylwester Nawrocki]

On 08/30/2016 12:36 PM, Krzysztof Kozlowski wrote:
> On 08/22/2016 06:31 PM, Sylwester Nawrocki wrote:
>> This patch adds code instantiating the EPLL, which is used as the
>> audio subsystem's root clock.
>> The requirement to specify the external root clock in clocks property
>> is also added.
> 
> I think the requirement was there already but explained little bit
> differently:
> 19 External clock:
> 20
> 21 There is clock that is generated outside the SoC. It
> 22 is expected that it is defined using standard clock bindings
> 23 with following clock-output-name:
> 24
> 25  - "fin_pll" - PLL input clock from XXTI
>
> so you can just combine them. Driver now will require the fin_pll in two
> ways:
> 1. Old lookup by "fin_pll" name.
> 2. of_clk_get of yours.

I'm not sure what do you mean exactly, I'm also adding in this patch a sentence
right after the text as quoted above saying that:

"This clock should be listed in the clocks property of the controller node."

The description of the consumer clock (the main clock controller's parent clock)
was not in the binding so far, there is no "clocks" property in the list of
required properties.

>>  That ensures proper initialization order by explicitly
>> specifying dependencies in devicetree.  It prevents situations when the
>> SoC's clock controller driver has initialized, the external oscillator
>> clock is not yet registered and setting clock frequencies through
>> assigned-clock-rates property doesn't work properly due to unknown
>> external oscillator frequency.
>>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>>  .../devicetree/bindings/clock/exynos5410-clock.txt |  14 +++
>>  drivers/clk/samsung/clk-exynos5410.c               |  30 +++++-
>>  drivers/clk/samsung/clk-pll.c                      | 102 +++++++++++++++++++++
>>  drivers/clk/samsung/clk-pll.h                      |   1 +
>>  4 files changed, 145 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
>> index aeab635..2aefc07 100644
>> --- a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
>> +++ b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
>> @@ -12,6 +12,9 @@ Required Properties:
>>  
>>  - #clock-cells: should be 1.
>>  
>> +- clocks: should contain an entry specifying the root oscillator clock
>> +  on XXTI pin (fin_pll).
> 
> Combine with "external clock".

Do you mean rephrasing this to something like:

- clocks: should contain an entry specifying the external clock (fin_pll),
  i.e. the root clock on XXTI pin.

?

--
Thanks,
Sylwester

Re: [PATCH 5/5] clk: samsung: Add support for exynos5410 AUDSS clock controller

[Sylwester Nawrocki]

On 08/30/2016 12:38 PM, Krzysztof Kozlowski wrote:
> On 08/22/2016 06:31 PM, Sylwester Nawrocki wrote:
>> > Exynos5410 Audio Subsystem Clock Controller, comparing to the already
>> > supported by the driver IP block revisions, has additionally an I2S_MST
>> > divider so a new compatible string is added.
>> > While at it, the variant handling is reworked to avoid scattered testing
>> > for the controller's variant.
>
> Please split it into simple changes:
> 1. Whitespace cleanup (below you do it).
> 2. Variant handling rework.
> 3. New compatible added.
> 
> This would make easy to spot changes and new features.

Ok, I'll split it, thanks for your review.

Re: [PATCH 4/5] clk: samsung: Add support for EPLL on exynos5410

[Krzysztof Kozlowski]

On Fri, Sep 02, 2016 at 05:15:31PM +0200, Sylwester Nawrocki wrote:
> On 08/30/2016 12:36 PM, Krzysztof Kozlowski wrote:
> > On 08/22/2016 06:31 PM, Sylwester Nawrocki wrote:
> >> This patch adds code instantiating the EPLL, which is used as the
> >> audio subsystem's root clock.
> >> The requirement to specify the external root clock in clocks property
> >> is also added.
> > 
> > I think the requirement was there already but explained little bit
> > differently:
> > 19 External clock:
> > 20
> > 21 There is clock that is generated outside the SoC. It
> > 22 is expected that it is defined using standard clock bindings
> > 23 with following clock-output-name:
> > 24
> > 25  - "fin_pll" - PLL input clock from XXTI
> >
> > so you can just combine them. Driver now will require the fin_pll in two
> > ways:
> > 1. Old lookup by "fin_pll" name.
> > 2. of_clk_get of yours.
> 
> I'm not sure what do you mean exactly, I'm also adding in this patch a sentence
> right after the text as quoted above saying that:
> 
> "This clock should be listed in the clocks property of the controller node."
> 
> The description of the consumer clock (the main clock controller's parent clock)
> was not in the binding so far, there is no "clocks" property in the list of
> required properties.

First of all, in commit message, you are not adding the requirement...
it was present already, I think. Wasn't it?

As for the code, I am saying that you are duplicating the information.
There is already an paragraph for external clock. You are adding a new
one before and extending it... just make it simpler - mention external
clock in one place.

> 
> >>  That ensures proper initialization order by exlicitly
> >> specifying dependencies in devicetree.  It prevents situations when the
> >> SoC's clock controller driver has initialized, the external oscillator
> >> clock is not yet registered and setting clock frequencies through
> >> assigned-clock-rates property doesn't work properly due to unknown
> >> external oscillator frequency.
> >>
> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >> ---
> >>  .../devicetree/bindings/clock/exynos5410-clock.txt |  14 +++
> >>  drivers/clk/samsung/clk-exynos5410.c               |  30 +++++-
> >>  drivers/clk/samsung/clk-pll.c                      | 102 +++++++++++++++++++++
> >>  drivers/clk/samsung/clk-pll.h                      |   1 +
> >>  4 files changed, 145 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> >> index aeab635..2aefc07 100644
> >> --- a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> >> +++ b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> >> @@ -12,6 +12,9 @@ Required Properties:
> >>  
> >>  - #clock-cells: should be 1.
> >>  
> >> +- clocks: should contain an entry specifying the root oscillator clock
> >> +  on XXTI pin (fin_pll).
> > 
> > Combine with "external clock".
> 
> Do you mean rephrasing this to something like:
> 
> - clocks: should contain an entry specifying the external clock (fin_pll),
>   i.e. the root clock on XXTI pin.

Could be, with removal of the existing paragraph. The only comment I
have is about duplicating the information.

Best regards,
Krzysztof

Re: [PATCH 1/5] clk: samsung: exynos5410: Add clock IDs for PDMA and EPLL clocks

[Krzysztof Kozlowski]

On Fri, Sep 02, 2016 at 04:09:08PM +0200, Sylwester Nawrocki wrote:
> On 08/30/2016 11:54 AM, Krzysztof Kozlowski wrote:
> > On 08/22/2016 06:30 PM, Sylwester Nawrocki wrote:
> >> > The PDMA{0,1} and EPLL clock IDs are added separately in this
> >> > patch so the patch can be merged to the arm-soc tree as dependency.
> >> > 
> >> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >> > ---
> >> >  include/dt-bindings/clock/exynos5410.h | 3 +++
> >> >  1 file changed, 3 insertions(+)
> >> > 
> >> > diff --git a/include/dt-bindings/clock/exynos5410.h b/include/dt-bindings/clock/exynos5410.h
> >> > index 85b467b..a74442a 100644
> >> > --- a/include/dt-bindings/clock/exynos5410.h
> >> > +++ b/include/dt-bindings/clock/exynos5410.h
> >> > @@ -19,6 +19,7 @@
> >> >  #define CLK_FOUT_MPLL		4
> >> >  #define CLK_FOUT_BPLL		5
> >> >  #define CLK_FOUT_KPLL		6
> >> > +#define CLK_FOUT_EPLL		7
> >> >  
> >> >  /* gate for special clocks (sclk) */
> >> >  #define CLK_SCLK_UART0		128
> >> > @@ -48,6 +49,8 @@
> >> >  #define CLK_USI3		268
> >> >  #define CLK_UART3		260
> >> >  #define CLK_PWM			279
> >> > +#define CLK_PDMA0		280
> >> > +#define CLK_PDMA1		281
> >
> > How about making the IDs the same as in exynos5420? This way those
> > drivers could be merged someday in the future (if someone would be
> > bored...).
> 
> OK, I will do it, but there is already mismatch in the PLL indices.

Oh, crap, I missed that difference. This means the drivers won't be
combined... so I am not sure whether keeping same IDs for rest brings
benefits. Whatever you chooses:

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 4/5] clk: samsung: Add support for EPLL on exynos5410

[Sylwester Nawrocki]

On 09/02/2016 07:32 PM, Krzysztof Kozlowski wrote:
> On Fri, Sep 02, 2016 at 05:15:31PM +0200, Sylwester Nawrocki wrote:
>> > On 08/30/2016 12:36 PM, Krzysztof Kozlowski wrote:
>>> > > On 08/22/2016 06:31 PM, Sylwester Nawrocki wrote:
>>>> > >> This patch adds code instantiating the EPLL, which is used as the
>>>> > >> audio subsystem's root clock.
>>>> > >> The requirement to specify the external root clock in clocks property
>>>> > >> is also added.
>>> > > 
>>> > > I think the requirement was there already but explained little bit
>>> > > differently:
>>> > > 19 External clock:
>>> > > 20
>>> > > 21 There is clock that is generated outside the SoC. It
>>> > > 22 is expected that it is defined using standard clock bindings
>>> > > 23 with following clock-output-name:
>>> > > 24
>>> > > 25  - "fin_pll" - PLL input clock from XXTI
>>> > >
>>> > > so you can just combine them. Driver now will require the fin_pll in two
>>> > > ways:
>>> > > 1. Old lookup by "fin_pll" name.
>>> > > 2. of_clk_get of yours.
>> > 
>> > I'm not sure what do you mean exactly, I'm also adding in this patch a sentence
>> > right after the text as quoted above saying that:
>> > 
>> > "This clock should be listed in the clocks property of the controller node."
>> > 
>> > The description of the consumer clock (the main clock controller's parent clock)
>> > was not in the binding so far, there is no "clocks" property in the list of
>> > required properties.
>
> First of all, in commit message, you are not adding the requirement...
> it was present already, I think. Wasn't it?

I guess we could say it was there, but the documentation was unclear
enough so that link between the oscillator clock and the main clocks
controller is missing in many dts files.

> As for the code, I am saying that you are duplicating the information.
> There is already an paragraph for external clock. You are adding a new
> one before and extending it... just make it simpler - mention external
> clock in one place.

Ah, now I see, indeed it makes sense to delete the "External clock"
paragraph.  I'll reword then the clocks property description to:

- clocks: should contain an entry specifying the root clock from external
  oscillator supplied through XXTI or XusbXTI pin.  This clock should be
  defined using standard clock bindings with "fin_pll" clock-output-name.
  The "fin_pll" clock is being passed internally to the 9 PLLs.

--
Thanks,
Sylwester