[PATCH v2 0/7] clk/samsung: exynos5410: Add sound subsystem related clocks

[PATCH v2 0/7] clk/samsung: exynos5410: Add sound subsystem related clocks

[Sylwester Nawrocki]

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

The clk-exynos-audss driver has been reworked to support
"samsung,exynos5412-audss-clock" compatible.

This series also includes a cleanup patch consolidating the PLL clocks
code, i.e. switching exynos5440 to use common code.

Sylwester Nawrocki (7):
  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: clk-exynos-audss: controller variant handling rework
  clk: samsung: clk-exynos-audss: Add exynos5410 compatible
  clk: samsung: clk-exynos-audss: Whitespace and debug trace cleanup

 .../devicetree/bindings/clock/clk-exynos-audss.txt |   4 +-
 .../devicetree/bindings/clock/exynos5410-clock.txt |  21 +--
 drivers/clk/samsung/clk-exynos-audss.c             |  84 ++++++-----
 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, 217 insertions(+), 94 deletions(-)

--
1.9.1

[PATCH v2 1/7] 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>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
Changes since v1:
 - used same index values as for exynos5420.
---
 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..6cb4e90 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
@@ -55,6 +56,8 @@
 #define CLK_MMC0		351
 #define CLK_MMC1		352
 #define CLK_MMC2		353
+#define CLK_PDMA0		362
+#define CLK_PDMA1		363
 #define CLK_USBH20		365
 #define CLK_USBD300		366
 #define CLK_USBD301		367
--
1.9.1

[PATCH v2 2/7] 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>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@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 v2 3/7] 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 v2 4/7] 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 documented.  Having the consumer 'clocks' property ensures proper
initialization order by explicitly specifying dependencies in DT.
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>
---
Changes since v1:
 - rephrased paragraph of the DT binding describing the external
   XXTI clock.
---
 .../devicetree/bindings/clock/exynos5410-clock.txt |  21 +++--
 drivers/clk/samsung/clk-exynos5410.c               |  30 +++++-
 drivers/clk/samsung/clk-pll.c                      | 102 +++++++++++++++++++++
 drivers/clk/samsung/clk-pll.h                      |   1 +
 4 files changed, 144 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
index aeab635..4527de3 100644
--- a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
+++ b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
@@ -12,24 +12,29 @@ Required Properties:
 
 - #clock-cells: should be 1.
 
+- 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.
+  That clock is being passed internally to the 9 PLLs.
+
 All available clocks are defined as preprocessor macros in
 dt-bindings/clock/exynos5410.h header and can be used in device
 tree sources.
 
-External clock:
-
-There is clock that is generated outside the SoC. It
-is expected that it is defined using standard clock bindings
-with following clock-output-name:
-
- - "fin_pll" - PLL input clock from XXTI
-
 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 v2 5/7] clk: samsung: clk-exynos-audss: controller variant handling rework

[Sylwester Nawrocki]

Then variant handling is reworked to make the code simpler when
more variants are added.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/clk/samsung/clk-exynos-audss.c | 64 ++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
index bdf8b97..7d4026b 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;
@@ -73,14 +68,34 @@ 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,exynos5420-audss-clock",
+		.data		= &exynos5420_drvdata,
+	},
+	{ },
 };
 
 static void exynos_audss_clk_teardown(void)
@@ -106,19 +121,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 +139,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 +149,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,7 +158,7 @@ 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);
@@ -210,7 +220,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);
-- 
1.9.1

[PATCH v2 6/7] clk: samsung: clk-exynos-audss: Add exynos5410 compatible

[Sylwester Nawrocki]

Exynos5410 Audio Subsystem Clock Controller, comparing to the already
supported IP block revisions, has additionally an I2S_MST divider
so a new compatible string is added.
It is not clear from the Exynos5410 User's Manual released on 2012.03.09
where in the clock tree the I2S_MST clock divider can be found exactly
so this clock is left unimplemented for now.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 Documentation/devicetree/bindings/clock/clk-exynos-audss.txt | 4 +++-
 drivers/clk/samsung/clk-exynos-audss.c                       | 9 +++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

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 7d4026b..628f86b 100644
--- a/drivers/clk/samsung/clk-exynos-audss.c
+++ b/drivers/clk/samsung/clk-exynos-audss.c
@@ -70,6 +70,7 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 
 struct exynos_audss_clk_drvdata {
 	unsigned int has_adma_clk:1;
+	unsigned int has_mst_clk:1;
 	unsigned int enable_epll:1;
 	unsigned int num_clks;
 };
@@ -78,6 +79,11 @@ static const struct exynos_audss_clk_drvdata exynos4210_drvdata = {
 	.num_clks	= EXYNOS_AUDSS_MAX_CLKS - 1,
 };
 
+static const struct exynos_audss_clk_drvdata exynos5410_drvdata = {
+	.num_clks	= EXYNOS_AUDSS_MAX_CLKS - 1,
+	.has_mst_clk	= 1,
+};
+
 static const struct exynos_audss_clk_drvdata exynos5420_drvdata = {
 	.num_clks	= EXYNOS_AUDSS_MAX_CLKS,
 	.has_adma_clk	= 1,
@@ -92,6 +98,9 @@ static const struct of_device_id exynos_audss_clk_of_match[] = {
 		.compatible	= "samsung,exynos5250-audss-clock",
 		.data		= &exynos4210_drvdata,
 	}, {
+		.compatible	= "samsung,exynos5410-audss-clock",
+		.data		= &exynos5410_drvdata,
+	}, {
 		.compatible	= "samsung,exynos5420-audss-clock",
 		.data		= &exynos5420_drvdata,
 	},
-- 
1.9.1

[PATCH v2 7/7] clk: samsung: clk-exynos-audss: Whitespace and debug trace cleanup

[Sylwester Nawrocki]

There is no need to log probe() completion in normal conditions
so the "setup completed" log is removed.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/clk/samsung/clk-exynos-audss.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
index 628f86b..51d152f 100644
--- a/drivers/clk/samsung/clk-exynos-audss.c
+++ b/drivers/clk/samsung/clk-exynos-audss.c
@@ -39,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)
@@ -173,7 +173,7 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 			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;
 			}
 		}
@@ -253,9 +253,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 v2 4/7] clk: samsung: Add support for EPLL on exynos5410

[Chanwoo Choi]

Hi Sylwester,

On 2016년 09월 06일 21:04, 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 documented.  Having the consumer 'clocks' property ensures proper
> initialization order by explicitly specifying dependencies in DT.
> 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>
> ---
> Changes since v1:
>  - rephrased paragraph of the DT binding describing the external
>    XXTI clock.
> ---
>  .../devicetree/bindings/clock/exynos5410-clock.txt |  21 +++--
>  drivers/clk/samsung/clk-exynos5410.c               |  30 +++++-
>  drivers/clk/samsung/clk-pll.c                      | 102 +++++++++++++++++++++
>  drivers/clk/samsung/clk-pll.h                      |   1 +
>  4 files changed, 144 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> index aeab635..4527de3 100644
> --- a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> @@ -12,24 +12,29 @@ Required Properties:
>  
>  - #clock-cells: should be 1.
>  
> +- 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.
> +  That clock is being passed internally to the 9 PLLs.
> +
>  All available clocks are defined as preprocessor macros in
>  dt-bindings/clock/exynos5410.h header and can be used in device
>  tree sources.
>  
> -External clock:
> -
> -There is clock that is generated outside the SoC. It
> -is expected that it is defined using standard clock bindings
> -with following clock-output-name:
> -
> - - "fin_pll" - PLL input clock from XXTI
> -
>  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),

In the Exynos5410's TRM, the EPLL PMS table has the 
'P' state is 3 instead of 2 when target is 400MHz as following:

	PLL_36XX_RATE(400000000U, 200, 3, 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),

The all entry of EPLL PMS table has the zero(0) for 'K' value.
How about using the PLL_35XX_RATE() which has the P,M,S entry without K entry?

> +};
> +
> +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;
> +

I sent the patch to use the samsung_cmu_register_one()

and applied it[1]. I think that you need to rebase this patch on patch[1].
[1] https://lkml.org/lkml/2016/9/1/602
- Re: [PATCH 2/2] clk: samsung: exynos5410: Use samsung_cmu_register_one() to simplify code

>  	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

1FF instead of 0x3FF because the MDIV [24:16]?

> +#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;

I got some confusion because the EPLL_CON1 register don't include
the 'K' value. Instead, the name is 'DSM' which is PLL DSM(Delta-Sigma Modulator).
But, I knew that DSM means the 'K' value as your implementation.

I have a question.
When I check the calculation fomula as following:
	FFOUT = ((m+k/6536) x FFIN) / (p x 2^S);

So, pll2560x might shift the kdiv instead of mdiv.
	fout *= mdiv + (kdiv << 16);


> +}
> +
> +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,
> 

-- 
Best Regards,
Chanwoo Choi

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

[Chanwoo Choi]

Hi Sylwester,

Looks good to me. It makes the code simpler than before.

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

On 2016년 09월 06일 21:04, 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(-)
> 
> 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),

Looks good to me.
The offset of con register are calculated wiht formula as following:
	0x4c = 0x1c + (0x10 x 3)
	0x50 = 0x20 + (0x10 x 3)

> +};
> +
>  /* 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);

Looks good to me. It is more simple than before.
The exynos5440_plls[] includes the already calculated value
without 'pll->offset' as following:
	- 0x4c = 0x1c + (0x10 x 3)
	- 0x50 = 0x20 + (0x10 x 3)


>  	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
> 

-- 
Best Regards,
Chanwoo Choi

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

[Chanwoo Choi]

On 2016년 09월 06일 21:04, 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>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@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),

Looks good to me.

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi

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

[Chanwoo Choi]

On 2016년 09월 06일 21:04, 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>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
> Changes since v1:
>  - used same index values as for exynos5420.
> ---
>  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..6cb4e90 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
> @@ -55,6 +56,8 @@
>  #define CLK_MMC0		351
>  #define CLK_MMC1		352
>  #define CLK_MMC2		353
> +#define CLK_PDMA0		362
> +#define CLK_PDMA1		363
>  #define CLK_USBH20		365
>  #define CLK_USBD300		366
>  #define CLK_USBD301		367

The CLK_FOUT_EPLL is used on patch4 and CLK_PDMAx are used on patch2.

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi

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

[Sylwester Nawrocki]

On 09/07/2016 06:14 AM, Chanwoo Choi wrote:
> On 2016년 09월 06일 21:04, Sylwester Nawrocki wrote:
...
>> -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),
> 
> In the Exynos5410's TRM, the EPLL PMS table has the 
> 'P' state is 3 instead of 2 when target is 400MHz as following:
> 
> 	PLL_36XX_RATE(400000000U, 200, 3, 2, 0),

Indeed, thanks for pointing out.

>> +	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),
> 
> The all entry of EPLL PMS table has the zero(0) for 'K' value.
> How about using the PLL_35XX_RATE() which has the P,M,S entry without K entry?

Don't have a strong opinion on that, I'm inclined to stay with
PLL_36XX_RATE() though, to indicate the K value is valid for this
PLL type.

>> @@ -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;
>> +
> 
> I sent the patch to use the samsung_cmu_register_one()
> 
> and applied it[1]. I think that you need to rebase this patch on patch[1].
> [1] https://lkml.org/lkml/2016/9/1/602
> - Re: [PATCH 2/2] clk: samsung: exynos5410: Use samsung_cmu_register_one() to simplify code

Yes, I need to rebase onto latest tree.

>> +/* Maximum lock time can be 3000 * PDIV cycles */
>> +#define PLL2650X_LOCK_FACTOR		3000
>> +
>> +#define PLL2650X_M_MASK			0x3FF
> 
> 1FF instead of 0x3FF because the MDIV [24:16]?

Right, my bad, I'll correct this.

>> +#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;
> 
> I got some confusion because the EPLL_CON1 register don't include
> the 'K' value. Instead, the name is 'DSM' which is PLL DSM(Delta-Sigma 
> Modulator).
> But, I knew that DSM means the 'K' value as your implementation.

Yes, the documentation seems incomplete, the K coefficient is used
in the PLL output frequency equations and I found that register
bit field used in some downstream kernel.
Perhaps this is corrected in newer User Manual version than
the "Preeliminary" I have.

> I have a question.
> When I check the calculation fomula as following:
> 	FFOUT = ((m+k/65536) x FFIN) / (p x 2^S);
> 
> So, pll2560x might shift the kdiv instead of mdiv.
> 	fout *= mdiv + (kdiv << 16);

First we multiply both sides of the equation by 65536 (2^16):

 65536 * FFOUT = 65536 * ((M + K/65536) x FFIN) / (P x 2^S);

That means M needs to be multiplied by 2^16, not K:

 65536 * FFOUT = (65536 * M + K) x FFIN) / (P x 2^S);

K is the "fractional" coefficient here, with least impact on FFOUT.
Finally we divide fout by 2^16 to reverse previous multiplication.

I tested this code with audio playback, if it had been incorrect
in such a way the sampling rate would certainly have been incorrect
and it would have been heard.

-- 
Thanks,
Sylwester

Re: [PATCH v2 0/7] clk/samsung: exynos5410: Add sound subsystem related clocks

[Sylwester Nawrocki]

On 09/06/2016 02:04 PM, Sylwester Nawrocki wrote:
> This patch adds support for the EPLL and the peripheral DMA clocks required
> for sound support on Odroid-XU board.
> 
> The clk-exynos-audss driver has been reworked to support
> "samsung,exynos5412-audss-clock" compatible.
> 
> This series also includes a cleanup patch consolidating the PLL clocks
> code, i.e. switching exynos5440 to use common code.
> 
> Sylwester Nawrocki (7):
>   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: clk-exynos-audss: controller variant handling rework
>   clk: samsung: clk-exynos-audss: Add exynos5410 compatible
>   clk: samsung: clk-exynos-audss: Whitespace and debug trace cleanup


I have applied all patches from this series, except
"clk: samsung: Add support for EPLL on exynos5410", to my tree.

-- 
Thanks,
Sylwester

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

[Chanwoo Choi]

On 2016년 09월 07일 17:27, Sylwester Nawrocki wrote:
> On 09/07/2016 06:14 AM, Chanwoo Choi wrote:
>> On 2016년 09월 06일 21:04, Sylwester Nawrocki wrote:
> ...
>>> -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),
>>
>> In the Exynos5410's TRM, the EPLL PMS table has the 
>> 'P' state is 3 instead of 2 when target is 400MHz as following:
>>
>> 	PLL_36XX_RATE(400000000U, 200, 3, 2, 0),
> 
> Indeed, thanks for pointing out.
> 
>>> +	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),
>>
>> The all entry of EPLL PMS table has the zero(0) for 'K' value.
>> How about using the PLL_35XX_RATE() which has the P,M,S entry without K entry?
> 
> Don't have a strong opinion on that, I'm inclined to stay with
> PLL_36XX_RATE() though, to indicate the K value is valid for this
> PLL type.
> 
>>> @@ -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;
>>> +
>>
>> I sent the patch to use the samsung_cmu_register_one()
>>
>> and applied it[1]. I think that you need to rebase this patch on patch[1].
>> [1] https://lkml.org/lkml/2016/9/1/602
>> - Re: [PATCH 2/2] clk: samsung: exynos5410: Use samsung_cmu_register_one() to simplify code
> 
> Yes, I need to rebase onto latest tree.
> 
>>> +/* Maximum lock time can be 3000 * PDIV cycles */
>>> +#define PLL2650X_LOCK_FACTOR		3000
>>> +
>>> +#define PLL2650X_M_MASK			0x3FF
>>
>> 1FF instead of 0x3FF because the MDIV [24:16]?
> 
> Right, my bad, I'll correct this.
> 
>>> +#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;
>>
>> I got some confusion because the EPLL_CON1 register don't include
>> the 'K' value. Instead, the name is 'DSM' which is PLL DSM(Delta-Sigma 
>> Modulator).
>> But, I knew that DSM means the 'K' value as your implementation.
> 
> Yes, the documentation seems incomplete, the K coefficient is used
> in the PLL output frequency equations and I found that register
> bit field used in some downstream kernel.
> Perhaps this is corrected in newer User Manual version than
> the "Preeliminary" I have.

OK. Thanks.

> 
>> I have a question.
>> When I check the calculation fomula as following:
>> 	FFOUT = ((m+k/65536) x FFIN) / (p x 2^S);
>>
>> So, pll2560x might shift the kdiv instead of mdiv.
>> 	fout *= mdiv + (kdiv << 16);
> 
> First we multiply both sides of the equation by 65536 (2^16):
> 
>  65536 * FFOUT = 65536 * ((M + K/65536) x FFIN) / (P x 2^S);
> 
> That means M needs to be multiplied by 2^16, not K:
> 
>  65536 * FFOUT = (65536 * M + K) x FFIN) / (P x 2^S);

I understand. I was missing to check the "fout >>= 16;" equation.
Thanks for your explanation

> 
> K is the "fractional" coefficient here, with least impact on FFOUT.
> Finally we divide fout by 2^16 to reverse previous multiplication.
> 
> I tested this code with audio playback, if it had been incorrect
> in such a way the sampling rate would certainly have been incorrect
> and it would have been heard.
> 

-- 
Best Regards,
Chanwoo Choi