linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs
@ 2013-05-31 12:31 Vikas Sajjan
  2013-05-31 12:31 ` [PATCH v3 1/6] clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx Vikas Sajjan
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Vikas Sajjan @ 2013-05-31 12:31 UTC (permalink / raw)
  To: linux-arm-kernel

This patch series does the following: 

 1) Factors out possible common code, unifies the clk strutures used
    for PLL35xx & PLL36xx and usues clk->base instead of clk->con0

 2) Defines a common rate_table which will contain recommended p, m, s and k
    values for supported rates that needs to be changed for changing
    corresponding PLL's rate

 3) Adds set_rate() and round_rate() clk_ops for PLL35xx and PLL36xx

changes since v2:
	- Added new patch to reorder the MUX registration for mout_vpllsrc MUX
	before the PLL registrations. And to add the alias for the mout_vpllsrc MUX.
        - Added a check to confirm parent rate while registrating the PLL
	rate tables.

changes since v1:
	- removed sorting and bsearch
	- modified the definition of struct "samsung_pll_rate_table"
	- added generic round_rate()
	- rectified the ops assignment for "rate table passed as NULL"
	  during PLL registration

Is rebased on branch kgene's "for-next"
https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/log/?h=for-next

And tested these patch on chromebook for EPLL settings for Audio on our chrome tree.


Vikas Sajjan (3):
  clk: samsung: Add set_rate() clk_ops for PLL36xx
  clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration
    for it
  clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC

Yadwinder Singh Brar (3):
  clk: samsung: Use clk->base instead of directly using clk->con0 for
    PLL3xxx
  clk: samsung: Add support to register rate_table for PLL3xxx
  clk: samsung: Add set_rate() clk_ops for PLL35xx

 drivers/clk/samsung/clk-exynos4.c    |   10 +-
 drivers/clk/samsung/clk-exynos5250.c |   69 +++++++++--
 drivers/clk/samsung/clk-pll.c        |  226 ++++++++++++++++++++++++++++++----
 drivers/clk/samsung/clk-pll.h        |   35 +++++-
 drivers/clk/samsung/clk.h            |    2 +
 5 files changed, 300 insertions(+), 42 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/6] clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx
  2013-05-31 12:31 [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Vikas Sajjan
@ 2013-05-31 12:31 ` Vikas Sajjan
  2013-06-08 12:04   ` Tomasz Figa
  2013-05-31 12:31 ` [PATCH v3 2/6] clk: samsung: Add support to register rate_table " Vikas Sajjan
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Vikas Sajjan @ 2013-05-31 12:31 UTC (permalink / raw)
  To: linux-arm-kernel

From: Yadwinder Singh Brar <yadi.brar@samsung.com>

To factor out possible common code, this patch unifies the clk strutures used
for PLL35xx & PLL36xx and usues clk->base instead of clk->con0.

Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
---
 drivers/clk/samsung/clk-exynos4.c    |   10 ++++---
 drivers/clk/samsung/clk-exynos5250.c |   14 ++++-----
 drivers/clk/samsung/clk-pll.c        |   54 ++++++++++++++++++----------------
 drivers/clk/samsung/clk-pll.h        |    4 +--
 4 files changed, 44 insertions(+), 38 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index d0940e6..cf7d4e7 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -97,12 +97,14 @@
 #define GATE_IP_PERIL		0xc950
 #define E4210_GATE_IP_PERIR	0xc960
 #define GATE_BLOCK		0xc970
+#define E4X12_MPLL_LOCK		0x10008
 #define E4X12_MPLL_CON0		0x10108
 #define SRC_DMC			0x10200
 #define SRC_MASK_DMC		0x10300
 #define DIV_DMC0		0x10500
 #define DIV_DMC1		0x10504
 #define GATE_IP_DMC		0x10900
+#define APLL_LOCK		0x14000
 #define APLL_CON0		0x14100
 #define E4210_MPLL_CON0		0x14108
 #define SRC_CPU			0x14200
@@ -1019,13 +1021,13 @@ void __init exynos4_clk_init(struct device_node *np, enum exynos4_soc exynos4_so
 					reg_base + VPLL_CON0, pll_4650c);
 	} else {
 		apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-					reg_base + APLL_CON0);
+					reg_base + APLL_LOCK);
 		mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-					reg_base + E4X12_MPLL_CON0);
+					reg_base + E4X12_MPLL_LOCK);
 		epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
-					reg_base + EPLL_CON0);
+					reg_base + EPLL_LOCK);
 		vpll = samsung_clk_register_pll36xx("fout_vpll", "fin_pll",
-					reg_base + VPLL_CON0);
+					reg_base + VPLL_LOCK);
 	}
 
 	samsung_clk_add_lookup(apll, fout_apll);
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 5c97e75..687b580 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -491,19 +491,19 @@ void __init exynos5250_clk_init(struct device_node *np)
 			ext_clk_match);
 
 	apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-			reg_base + 0x100);
+			reg_base);
 	mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-			reg_base + 0x4100);
+			reg_base + 0x4000);
 	bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll",
-			reg_base + 0x20110);
+			reg_base + 0x20010);
 	gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll",
-			reg_base + 0x10150);
+			reg_base + 0x10050);
 	cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
-			reg_base + 0x10120);
+			reg_base + 0x10020);
 	epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
-			reg_base + 0x10130);
+			reg_base + 0x10030);
 	vpll = samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc",
-			reg_base + 0x10140);
+			reg_base + 0x10040);
 
 	samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
 			ARRAY_SIZE(exynos5250_fixed_rate_clks));
diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
index 89135f6..a7d8ad9 100644
--- a/drivers/clk/samsung/clk-pll.c
+++ b/drivers/clk/samsung/clk-pll.c
@@ -13,9 +13,24 @@
 #include "clk.h"
 #include "clk-pll.h"
 
+struct samsung_clk_pll {
+	struct clk_hw		hw;
+	const void __iomem	*base;
+};
+
+#define to_clk_pll(_hw) container_of(_hw, struct samsung_clk_pll, hw)
+
+#define pll_readl(pll, offset)						\
+		__raw_readl((void __iomem *)(pll->base + (offset)));
+#define pll_writel(pll, val, offset)					\
+		__raw_writel(val, (void __iomem *)(pll->base + (offset)));
+
 /*
  * PLL35xx Clock Type
  */
+#define PLL35XX_LOCK_OFFSET	(0x0)
+#define PLL35XX_CON0_OFFSET	(0x100)
+#define PLL35XX_CON1_OFFSET	(0x104)
 
 #define PLL35XX_MDIV_MASK       (0x3FF)
 #define PLL35XX_PDIV_MASK       (0x3F)
@@ -24,21 +39,14 @@
 #define PLL35XX_PDIV_SHIFT      (8)
 #define PLL35XX_SDIV_SHIFT      (0)
 
-struct samsung_clk_pll35xx {
-	struct clk_hw		hw;
-	const void __iomem	*con_reg;
-};
-
-#define to_clk_pll35xx(_hw) container_of(_hw, struct samsung_clk_pll35xx, hw)
-
 static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,
 				unsigned long parent_rate)
 {
-	struct samsung_clk_pll35xx *pll = to_clk_pll35xx(hw);
+	struct samsung_clk_pll *pll = to_clk_pll(hw);
 	u32 mdiv, pdiv, sdiv, pll_con;
 	u64 fvco = parent_rate;
 
-	pll_con = __raw_readl(pll->con_reg);
+	pll_con = pll_readl(pll, PLL35XX_CON0_OFFSET);
 	mdiv = (pll_con >> PLL35XX_MDIV_SHIFT) & PLL35XX_MDIV_MASK;
 	pdiv = (pll_con >> PLL35XX_PDIV_SHIFT) & PLL35XX_PDIV_MASK;
 	sdiv = (pll_con >> PLL35XX_SDIV_SHIFT) & PLL35XX_SDIV_MASK;
@@ -54,9 +62,9 @@ static const struct clk_ops samsung_pll35xx_clk_ops = {
 };
 
 struct clk * __init samsung_clk_register_pll35xx(const char *name,
-			const char *pname, const void __iomem *con_reg)
+			const char *pname, const void __iomem *base)
 {
-	struct samsung_clk_pll35xx *pll;
+	struct samsung_clk_pll *pll;
 	struct clk *clk;
 	struct clk_init_data init;
 
@@ -73,7 +81,7 @@ struct clk * __init samsung_clk_register_pll35xx(const char *name,
 	init.num_parents = 1;
 
 	pll->hw.init = &init;
-	pll->con_reg = con_reg;
+	pll->base = base;
 
 	clk = clk_register(NULL, &pll->hw);
 	if (IS_ERR(clk)) {
@@ -91,6 +99,9 @@ struct clk * __init samsung_clk_register_pll35xx(const char *name,
 /*
  * PLL36xx Clock Type
  */
+#define PLL36XX_LOCK_OFFSET	(0x0)
+#define PLL36XX_CON0_OFFSET	(0x100)
+#define PLL36XX_CON1_OFFSET	(0x104)
 
 #define PLL36XX_KDIV_MASK	(0xFFFF)
 #define PLL36XX_MDIV_MASK	(0x1FF)
@@ -100,22 +111,15 @@ struct clk * __init samsung_clk_register_pll35xx(const char *name,
 #define PLL36XX_PDIV_SHIFT	(8)
 #define PLL36XX_SDIV_SHIFT	(0)
 
-struct samsung_clk_pll36xx {
-	struct clk_hw		hw;
-	const void __iomem	*con_reg;
-};
-
-#define to_clk_pll36xx(_hw) container_of(_hw, struct samsung_clk_pll36xx, hw)
-
 static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw,
 				unsigned long parent_rate)
 {
-	struct samsung_clk_pll36xx *pll = to_clk_pll36xx(hw);
+	struct samsung_clk_pll *pll = to_clk_pll(hw);
 	u32 mdiv, pdiv, sdiv, kdiv, pll_con0, pll_con1;
 	u64 fvco = parent_rate;
 
-	pll_con0 = __raw_readl(pll->con_reg);
-	pll_con1 = __raw_readl(pll->con_reg + 4);
+	pll_con0 = pll_readl(pll, PLL36XX_CON0_OFFSET);
+	pll_con1 = pll_readl(pll, PLL36XX_CON1_OFFSET);
 	mdiv = (pll_con0 >> PLL36XX_MDIV_SHIFT) & PLL36XX_MDIV_MASK;
 	pdiv = (pll_con0 >> PLL36XX_PDIV_SHIFT) & PLL36XX_PDIV_MASK;
 	sdiv = (pll_con0 >> PLL36XX_SDIV_SHIFT) & PLL36XX_SDIV_MASK;
@@ -133,9 +137,9 @@ static const struct clk_ops samsung_pll36xx_clk_ops = {
 };
 
 struct clk * __init samsung_clk_register_pll36xx(const char *name,
-			const char *pname, const void __iomem *con_reg)
+			const char *pname, const void __iomem *base)
 {
-	struct samsung_clk_pll36xx *pll;
+	struct samsung_clk_pll *pll;
 	struct clk *clk;
 	struct clk_init_data init;
 
@@ -152,7 +156,7 @@ struct clk * __init samsung_clk_register_pll36xx(const char *name,
 	init.num_parents = 1;
 
 	pll->hw.init = &init;
-	pll->con_reg = con_reg;
+	pll->base = base;
 
 	clk = clk_register(NULL, &pll->hw);
 	if (IS_ERR(clk)) {
diff --git a/drivers/clk/samsung/clk-pll.h b/drivers/clk/samsung/clk-pll.h
index f33786e..1329522 100644
--- a/drivers/clk/samsung/clk-pll.h
+++ b/drivers/clk/samsung/clk-pll.h
@@ -25,9 +25,9 @@ enum pll46xx_type {
 };
 
 extern struct clk * __init samsung_clk_register_pll35xx(const char *name,
-			const char *pname, const void __iomem *con_reg);
+			const char *pname, const void __iomem *base);
 extern struct clk * __init samsung_clk_register_pll36xx(const char *name,
-			const char *pname, const void __iomem *con_reg);
+			const char *pname, const void __iomem *base);
 extern struct clk * __init samsung_clk_register_pll45xx(const char *name,
 			const char *pname, const void __iomem *con_reg,
 			enum pll45xx_type type);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 2/6] clk: samsung: Add support to register rate_table for PLL3xxx
  2013-05-31 12:31 [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Vikas Sajjan
  2013-05-31 12:31 ` [PATCH v3 1/6] clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx Vikas Sajjan
@ 2013-05-31 12:31 ` Vikas Sajjan
  2013-05-31 17:08   ` Doug Anderson
  2013-05-31 12:31 ` [PATCH v3 3/6] clk: samsung: Add set_rate() clk_ops for PLL35xx Vikas Sajjan
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Vikas Sajjan @ 2013-05-31 12:31 UTC (permalink / raw)
  To: linux-arm-kernel

From: Yadwinder Singh Brar <yadi.brar@samsung.com>

This patch defines a common rate_table which will contain recommended p, m, s,
k values for supported rates that needs to be changed for changing
corresponding PLL's rate.

Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
---
 drivers/clk/samsung/clk-exynos4.c    |    8 ++++----
 drivers/clk/samsung/clk-exynos5250.c |   14 +++++++-------
 drivers/clk/samsung/clk-pll.c        |   14 ++++++++++++--
 drivers/clk/samsung/clk-pll.h        |   35 ++++++++++++++++++++++++++++++++--
 4 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index cf7d4e7..beff8a1 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -1021,13 +1021,13 @@ void __init exynos4_clk_init(struct device_node *np, enum exynos4_soc exynos4_so
 					reg_base + VPLL_CON0, pll_4650c);
 	} else {
 		apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-					reg_base + APLL_LOCK);
+					reg_base + APLL_LOCK, NULL, 0);
 		mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-					reg_base + E4X12_MPLL_LOCK);
+					reg_base + E4X12_MPLL_LOCK, NULL, 0);
 		epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
-					reg_base + EPLL_LOCK);
+					reg_base + EPLL_LOCK, NULL, 0);
 		vpll = samsung_clk_register_pll36xx("fout_vpll", "fin_pll",
-					reg_base + VPLL_LOCK);
+					reg_base + VPLL_LOCK, NULL, 0);
 	}
 
 	samsung_clk_add_lookup(apll, fout_apll);
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 687b580..ddf10ca 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -491,19 +491,19 @@ void __init exynos5250_clk_init(struct device_node *np)
 			ext_clk_match);
 
 	apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
-			reg_base);
+			reg_base, NULL, 0);
 	mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-			reg_base + 0x4000);
+			reg_base + 0x4000, NULL, 0);
 	bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll",
-			reg_base + 0x20010);
+			reg_base + 0x20010, NULL, 0);
 	gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll",
-			reg_base + 0x10050);
+			reg_base + 0x10050, NULL, 0);
 	cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
-			reg_base + 0x10020);
+			reg_base + 0x10020, NULL, 0);
 	epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
-			reg_base + 0x10030);
+			reg_base + 0x10030, NULL, 0);
 	vpll = samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc",
-			reg_base + 0x10040);
+			reg_base + 0x10040, NULL, 0);
 
 	samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
 			ARRAY_SIZE(exynos5250_fixed_rate_clks));
diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
index a7d8ad9..8226528 100644
--- a/drivers/clk/samsung/clk-pll.c
+++ b/drivers/clk/samsung/clk-pll.c
@@ -16,6 +16,8 @@
 struct samsung_clk_pll {
 	struct clk_hw		hw;
 	const void __iomem	*base;
+	const struct samsung_pll_rate_table *rate_table;
+	unsigned int rate_count;
 };
 
 #define to_clk_pll(_hw) container_of(_hw, struct samsung_clk_pll, hw)
@@ -62,7 +64,9 @@ static const struct clk_ops samsung_pll35xx_clk_ops = {
 };
 
 struct clk * __init samsung_clk_register_pll35xx(const char *name,
-			const char *pname, const void __iomem *base)
+			const char *pname, const void __iomem *base,
+			const struct samsung_pll_rate_table *rate_table,
+			const unsigned int rate_count)
 {
 	struct samsung_clk_pll *pll;
 	struct clk *clk;
@@ -82,6 +86,8 @@ struct clk * __init samsung_clk_register_pll35xx(const char *name,
 
 	pll->hw.init = &init;
 	pll->base = base;
+	pll->rate_table = rate_table;
+	pll->rate_count = rate_count;
 
 	clk = clk_register(NULL, &pll->hw);
 	if (IS_ERR(clk)) {
@@ -137,7 +143,9 @@ static const struct clk_ops samsung_pll36xx_clk_ops = {
 };
 
 struct clk * __init samsung_clk_register_pll36xx(const char *name,
-			const char *pname, const void __iomem *base)
+			const char *pname, const void __iomem *base,
+			const struct samsung_pll_rate_table *rate_table,
+			const unsigned int rate_count)
 {
 	struct samsung_clk_pll *pll;
 	struct clk *clk;
@@ -157,6 +165,8 @@ struct clk * __init samsung_clk_register_pll36xx(const char *name,
 
 	pll->hw.init = &init;
 	pll->base = base;
+	pll->rate_table = rate_table;
+	pll->rate_count = rate_count;
 
 	clk = clk_register(NULL, &pll->hw);
 	if (IS_ERR(clk)) {
diff --git a/drivers/clk/samsung/clk-pll.h b/drivers/clk/samsung/clk-pll.h
index 1329522..4780e6c 100644
--- a/drivers/clk/samsung/clk-pll.h
+++ b/drivers/clk/samsung/clk-pll.h
@@ -12,6 +12,33 @@
 #ifndef __SAMSUNG_CLK_PLL_H
 #define __SAMSUNG_CLK_PLL_H
 
+#define PLL_35XX_RATE(_rate, _m, _p, _s)			\
+	{							\
+		.rate	=	(_rate),				\
+		.mdiv	=	(_m),				\
+		.pdiv	=	(_p),				\
+		.sdiv	=	(_s),				\
+	}
+
+#define PLL_36XX_RATE(_rate, _m, _p, _s, _k)			\
+	{							\
+		.rate	=	(_rate),				\
+		.mdiv	=	(_m),				\
+		.pdiv	=	(_p),				\
+		.sdiv	=	(_s),				\
+		.kdiv	=	(_k),				\
+	}
+
+/* NOTE: Rate table should be kept sorted in descending order. */
+
+struct samsung_pll_rate_table {
+	unsigned int rate;
+	unsigned int pdiv;
+	unsigned int mdiv;
+	unsigned int sdiv;
+	unsigned int kdiv;
+};
+
 enum pll45xx_type {
 	pll_4500,
 	pll_4502,
@@ -25,9 +52,13 @@ enum pll46xx_type {
 };
 
 extern struct clk * __init samsung_clk_register_pll35xx(const char *name,
-			const char *pname, const void __iomem *base);
+			const char *pname, const void __iomem *base,
+			const struct samsung_pll_rate_table *rate_table,
+			const unsigned int rate_count);
 extern struct clk * __init samsung_clk_register_pll36xx(const char *name,
-			const char *pname, const void __iomem *base);
+			const char *pname, const void __iomem *base,
+			const struct samsung_pll_rate_table *rate_table,
+			const unsigned int rate_count);
 extern struct clk * __init samsung_clk_register_pll45xx(const char *name,
 			const char *pname, const void __iomem *con_reg,
 			enum pll45xx_type type);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 3/6] clk: samsung: Add set_rate() clk_ops for PLL35xx
  2013-05-31 12:31 [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Vikas Sajjan
  2013-05-31 12:31 ` [PATCH v3 1/6] clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx Vikas Sajjan
  2013-05-31 12:31 ` [PATCH v3 2/6] clk: samsung: Add support to register rate_table " Vikas Sajjan
@ 2013-05-31 12:31 ` Vikas Sajjan
  2013-05-31 17:10   ` Doug Anderson
  2013-06-08 12:12   ` Tomasz Figa
  2013-05-31 12:31 ` [PATCH v3 4/6] clk: samsung: Add set_rate() clk_ops for PLL36xx Vikas Sajjan
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 18+ messages in thread
From: Vikas Sajjan @ 2013-05-31 12:31 UTC (permalink / raw)
  To: linux-arm-kernel

From: Yadwinder Singh Brar <yadi.brar@samsung.com>

This patch add set_rate() and round_rate() for PLL35xx

Reviewed-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
---
 drivers/clk/samsung/clk-pll.c |  103 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 102 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
index 8226528..9591560 100644
--- a/drivers/clk/samsung/clk-pll.c
+++ b/drivers/clk/samsung/clk-pll.c
@@ -27,6 +27,36 @@ struct samsung_clk_pll {
 #define pll_writel(pll, val, offset)					\
 		__raw_writel(val, (void __iomem *)(pll->base + (offset)));
 
+static const struct samsung_pll_rate_table *samsung_get_pll_settings(
+				struct samsung_clk_pll *pll, unsigned long rate)
+{
+	const struct samsung_pll_rate_table  *rate_table = pll->rate_table;
+	int i;
+
+	for (i = 0; i < pll->rate_count; i++) {
+		if (rate == rate_table[i].rate)
+			return &rate_table[i];
+	}
+
+	return NULL;
+}
+
+static long samsung_pll_round_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_table = pll->rate_table;
+	int i;
+
+	/* Assumming rate_table is in descending order */
+	for (i = 0; i < pll->rate_count; i++) {
+		if (drate >= rate_table[i].rate)
+			return rate_table[i].rate;
+	}
+
+	/* return minimum supported value */
+	return rate_table[i - 1].rate;
+}
 /*
  * PLL35xx Clock Type
  */
@@ -34,12 +64,17 @@ struct samsung_clk_pll {
 #define PLL35XX_CON0_OFFSET	(0x100)
 #define PLL35XX_CON1_OFFSET	(0x104)
 
+/* Maximum lock time can be 270 * PDIV cycles */
+#define PLL35XX_LOCK_FACTOR	(270)
+
 #define PLL35XX_MDIV_MASK       (0x3FF)
 #define PLL35XX_PDIV_MASK       (0x3F)
 #define PLL35XX_SDIV_MASK       (0x7)
+#define PLL35XX_LOCK_STAT_MASK  (0x1)
 #define PLL35XX_MDIV_SHIFT      (16)
 #define PLL35XX_PDIV_SHIFT      (8)
 #define PLL35XX_SDIV_SHIFT      (0)
+#define PLL35XX_LOCK_STAT_SHIFT	(29)
 
 static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,
 				unsigned long parent_rate)
@@ -59,8 +94,70 @@ static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,
 	return (unsigned long)fvco;
 }
 
+static inline bool samsung_pll35xx_mp_change(u32 mdiv, u32 pdiv, u32 pll_con)
+{
+	if ((mdiv != ((pll_con >> PLL35XX_MDIV_SHIFT) & PLL35XX_MDIV_MASK)) ||
+		(pdiv != ((pll_con >> PLL35XX_PDIV_SHIFT) & PLL35XX_PDIV_MASK)))
+		return 1;
+	else
+		return 0;
+}
+
+static int samsung_pll35xx_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 tmp;
+
+	/* 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_get_name(hw->clk));
+		return -EINVAL;
+	}
+
+	tmp = pll_readl(pll, PLL35XX_CON0_OFFSET);
+
+	if (!(samsung_pll35xx_mp_change(rate->mdiv, rate->pdiv, tmp))) {
+		/* If only s change, change just s value only*/
+		tmp &= ~(PLL35XX_SDIV_MASK << PLL35XX_SDIV_SHIFT);
+		tmp |= rate->sdiv << PLL35XX_SDIV_SHIFT;
+		pll_writel(pll, tmp, PLL35XX_CON0_OFFSET);
+	} else {
+		/* Set PLL lock time. */
+		pll_writel(pll, rate->pdiv * PLL35XX_LOCK_FACTOR,
+				PLL35XX_LOCK_OFFSET);
+
+		/* Change PLL PMS values */
+		tmp &= ~((PLL35XX_MDIV_MASK << PLL35XX_MDIV_SHIFT) |
+				(PLL35XX_PDIV_MASK << PLL35XX_PDIV_SHIFT) |
+				(PLL35XX_SDIV_MASK << PLL35XX_SDIV_SHIFT));
+		tmp |= (rate->mdiv << PLL35XX_MDIV_SHIFT) |
+				(rate->pdiv << PLL35XX_PDIV_SHIFT) |
+				(rate->sdiv << PLL35XX_SDIV_SHIFT);
+		pll_writel(pll, tmp, PLL35XX_CON0_OFFSET);
+
+		/* wait_lock_time */
+		do {
+			cpu_relax();
+			tmp = pll_readl(pll, PLL35XX_CON0_OFFSET);
+		} while (!(tmp & (PLL35XX_LOCK_STAT_MASK
+				<< PLL35XX_LOCK_STAT_SHIFT)));
+	}
+
+	return 0;
+}
+
 static const struct clk_ops samsung_pll35xx_clk_ops = {
 	.recalc_rate = samsung_pll35xx_recalc_rate,
+	.round_rate = samsung_pll_round_rate,
+	.set_rate = samsung_pll35xx_set_rate,
+};
+
+static const struct clk_ops samsung_pll35xx_clk_min_ops = {
+	.recalc_rate = samsung_pll35xx_recalc_rate,
 };
 
 struct clk * __init samsung_clk_register_pll35xx(const char *name,
@@ -79,11 +176,15 @@ struct clk * __init samsung_clk_register_pll35xx(const char *name,
 	}
 
 	init.name = name;
-	init.ops = &samsung_pll35xx_clk_ops;
 	init.flags = CLK_GET_RATE_NOCACHE;
 	init.parent_names = &pname;
 	init.num_parents = 1;
 
+	if (rate_table && rate_count)
+		init.ops = &samsung_pll35xx_clk_ops;
+	else
+		init.ops = &samsung_pll35xx_clk_min_ops;
+
 	pll->hw.init = &init;
 	pll->base = base;
 	pll->rate_table = rate_table;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 4/6] clk: samsung: Add set_rate() clk_ops for PLL36xx
  2013-05-31 12:31 [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Vikas Sajjan
                   ` (2 preceding siblings ...)
  2013-05-31 12:31 ` [PATCH v3 3/6] clk: samsung: Add set_rate() clk_ops for PLL35xx Vikas Sajjan
@ 2013-05-31 12:31 ` Vikas Sajjan
  2013-05-31 17:15   ` Doug Anderson
  2013-06-08 12:17   ` Tomasz Figa
  2013-05-31 12:31 ` [PATCH v3 5/6] clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration for it Vikas Sajjan
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 18+ messages in thread
From: Vikas Sajjan @ 2013-05-31 12:31 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds set_rate and round_rate clk_ops for PLL36xx

Reviewed-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
---
 drivers/clk/samsung/clk-pll.c |   59 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
index 9591560..7143ed89 100644
--- a/drivers/clk/samsung/clk-pll.c
+++ b/drivers/clk/samsung/clk-pll.c
@@ -210,6 +210,9 @@ struct clk * __init samsung_clk_register_pll35xx(const char *name,
 #define PLL36XX_CON0_OFFSET	(0x100)
 #define PLL36XX_CON1_OFFSET	(0x104)
 
+/* Maximum lock time can be 3000 * PDIV cycles */
+#define PLL36XX_LOCK_FACTOR	(3000)
+
 #define PLL36XX_KDIV_MASK	(0xFFFF)
 #define PLL36XX_MDIV_MASK	(0x1FF)
 #define PLL36XX_PDIV_MASK	(0x3F)
@@ -217,6 +220,8 @@ struct clk * __init samsung_clk_register_pll35xx(const char *name,
 #define PLL36XX_MDIV_SHIFT	(16)
 #define PLL36XX_PDIV_SHIFT	(8)
 #define PLL36XX_SDIV_SHIFT	(0)
+#define PLL36XX_KDIV_SHIFT	(0)
+#define PLL36XX_LOCK_STAT_SHIFT	(29)
 
 static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw,
 				unsigned long parent_rate)
@@ -239,8 +244,57 @@ static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw,
 	return (unsigned long)fvco;
 }
 
+static int samsung_pll36xx_set_rate(struct clk_hw *hw, unsigned long drate,
+					unsigned long parent_rate)
+{
+	struct samsung_clk_pll *pll = to_clk_pll(hw);
+	u32 tmp, pll_con0, pll_con1;
+	const struct samsung_pll_rate_table *rate;
+
+	rate = samsung_get_pll_settings(pll, drate);
+	if (!rate) {
+		pr_err("%s: Invalid rate : %lu for pll clk %s\n", __func__,
+			drate, __clk_get_name(hw->clk));
+		return -EINVAL;
+	}
+
+	pll_con0 = pll_readl(pll, PLL36XX_CON0_OFFSET);
+	pll_con1 = pll_readl(pll, PLL36XX_CON1_OFFSET);
+
+	/* Set PLL lock time. */
+	pll_writel(pll, (rate->pdiv * PLL36XX_LOCK_FACTOR),
+			PLL36XX_LOCK_OFFSET);
+
+	 /* Change PLL PMS values */
+	pll_con0 &= ~((PLL36XX_MDIV_MASK << PLL36XX_MDIV_SHIFT) |
+			(PLL36XX_PDIV_MASK << PLL36XX_PDIV_SHIFT) |
+			(PLL36XX_SDIV_MASK << PLL36XX_SDIV_SHIFT));
+	pll_con0 |= (rate->mdiv << PLL36XX_MDIV_SHIFT) |
+			(rate->pdiv << PLL36XX_PDIV_SHIFT) |
+			(rate->sdiv << PLL36XX_SDIV_SHIFT);
+	pll_writel(pll, pll_con0, PLL36XX_CON0_OFFSET);
+
+	pll_con1 &= ~(PLL36XX_KDIV_MASK << PLL36XX_KDIV_SHIFT);
+	pll_con1 |= rate->kdiv << PLL36XX_KDIV_SHIFT;
+	pll_writel(pll, pll_con1, PLL36XX_CON1_OFFSET);
+
+	/* wait_lock_time */
+	do {
+		cpu_relax();
+		tmp = pll_readl(pll, PLL36XX_CON0_OFFSET);
+	} while (!(tmp & (1 << PLL36XX_LOCK_STAT_SHIFT)));
+
+	return 0;
+}
+
 static const struct clk_ops samsung_pll36xx_clk_ops = {
 	.recalc_rate = samsung_pll36xx_recalc_rate,
+	.set_rate = samsung_pll36xx_set_rate,
+	.round_rate = samsung_pll_round_rate,
+};
+
+static const struct clk_ops samsung_pll36xx_clk_min_ops = {
+	.recalc_rate = samsung_pll36xx_recalc_rate,
 };
 
 struct clk * __init samsung_clk_register_pll36xx(const char *name,
@@ -264,6 +318,11 @@ struct clk * __init samsung_clk_register_pll36xx(const char *name,
 	init.parent_names = &pname;
 	init.num_parents = 1;
 
+	if (rate_table && rate_count)
+		init.ops = &samsung_pll36xx_clk_ops;
+	else
+		init.ops = &samsung_pll36xx_clk_min_ops;
+
 	pll->hw.init = &init;
 	pll->base = base;
 	pll->rate_table = rate_table;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 5/6] clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration for it
  2013-05-31 12:31 [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Vikas Sajjan
                   ` (3 preceding siblings ...)
  2013-05-31 12:31 ` [PATCH v3 4/6] clk: samsung: Add set_rate() clk_ops for PLL36xx Vikas Sajjan
@ 2013-05-31 12:31 ` Vikas Sajjan
  2013-05-31 17:20   ` Doug Anderson
  2013-05-31 12:31 ` [PATCH v3 6/6] clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC Vikas Sajjan
  2013-06-03 21:06 ` [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Doug Anderson
  6 siblings, 1 reply; 18+ messages in thread
From: Vikas Sajjan @ 2013-05-31 12:31 UTC (permalink / raw)
  To: linux-arm-kernel

While trying to get rate of "mout_vpllsrc" MUX (parent) for registering the
"fout_vpll" (child), we found get rate was failing.

So this patch moves the mout_vpllsrc MUX out of the existing common list
and registers the mout_vpllsrc MUX before the PLL registrations.
Its also adds the alias for the mout_vpllsrc MUX.

Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
---
 drivers/clk/samsung/clk-exynos5250.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index ddf10ca..b0e6680 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -207,6 +207,11 @@ struct samsung_fixed_factor_clock exynos5250_fixed_factor_clks[] __initdata = {
 	FFACTOR(none, "fout_bplldiv2", "fout_bpll", 1, 2, 0),
 };
 
+struct samsung_mux_clock exynos5250_pll_pmux_clks[] __initdata = {
+	MUX_A(none, "mout_vpllsrc", mout_vpllsrc_p, SRC_TOP2, 0, 1,
+			"mout_vpllsrc"),
+};
+
 struct samsung_mux_clock exynos5250_mux_clks[] __initdata = {
 	MUX(none, "mout_apll", mout_apll_p, SRC_CPU, 0, 1),
 	MUX(none, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1),
@@ -214,7 +219,6 @@ struct samsung_mux_clock exynos5250_mux_clks[] __initdata = {
 	MUX(none, "sclk_mpll", mout_mpll_p, SRC_CORE1, 8, 1),
 	MUX(none, "mout_bpll_fout", mout_bpll_fout_p, PLL_DIV2_SEL, 0, 1),
 	MUX(none, "sclk_bpll", mout_bpll_p, SRC_CDREX, 0, 1),
-	MUX(none, "mout_vpllsrc", mout_vpllsrc_p, SRC_TOP2, 0, 1),
 	MUX(none, "sclk_vpll", mout_vpll_p, SRC_TOP2, 16, 1),
 	MUX(none, "sclk_epll", mout_epll_p, SRC_TOP2, 12, 1),
 	MUX(none, "sclk_cpll", mout_cpll_p, SRC_TOP2, 8, 1),
@@ -490,6 +494,9 @@ void __init exynos5250_clk_init(struct device_node *np)
 			ARRAY_SIZE(exynos5250_fixed_rate_ext_clks),
 			ext_clk_match);
 
+	samsung_clk_register_mux(exynos5250_pll_pmux_clks,
+			ARRAY_SIZE(exynos5250_pll_pmux_clks));
+
 	apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
 			reg_base, NULL, 0);
 	mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 6/6] clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC
  2013-05-31 12:31 [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Vikas Sajjan
                   ` (4 preceding siblings ...)
  2013-05-31 12:31 ` [PATCH v3 5/6] clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration for it Vikas Sajjan
@ 2013-05-31 12:31 ` Vikas Sajjan
  2013-05-31 17:58   ` Doug Anderson
  2013-06-03 21:06 ` [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Doug Anderson
  6 siblings, 1 reply; 18+ messages in thread
From: Vikas Sajjan @ 2013-05-31 12:31 UTC (permalink / raw)
  To: linux-arm-kernel

Adds the EPLL and VPLL freq table for exynos5250 SoC.

Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
---
 drivers/clk/samsung/clk-exynos5250.c |   48 +++++++++++++++++++++++++++++++---
 drivers/clk/samsung/clk.h            |    2 ++
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index b0e6680..0566421 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -473,11 +473,32 @@ static __initdata struct of_device_id ext_clk_match[] = {
 	{ },
 };
 
+static const struct samsung_pll_rate_table vpll_24mhz_tbl[] = {
+	/* sorted in descending order */
+	/* PLL_36XX_RATE(rate, m, p, s, k) */
+	PLL_36XX_RATE(266000000, 266, 3, 3, 0),
+	PLL_36XX_RATE(70500000, 94, 2, 4, 0),
+};
+
+static const struct samsung_pll_rate_table epll_24mhz_tbl[] = {
+	/* sorted in descending order */
+	/* PLL_36XX_RATE(rate, m, p, s, k) */
+	PLL_36XX_RATE(192000000, 48, 3, 1, 0),
+	PLL_36XX_RATE(180633600, 45, 3, 1, 10381),
+	PLL_36XX_RATE(180000000, 45, 3, 1, 0),
+	PLL_36XX_RATE(73728000, 73, 3, 3, 47710),
+	PLL_36XX_RATE(67737600, 90, 4, 3, 20762),
+	PLL_36XX_RATE(49152000, 49, 3, 3, 9962),
+	PLL_36XX_RATE(45158400, 45, 3, 3, 10381),
+	PLL_36XX_RATE(32768000, 131, 3, 5, 4719),
+};
+
 /* register exynox5250 clocks */
 void __init exynos5250_clk_init(struct device_node *np)
 {
 	void __iomem *reg_base;
 	struct clk *apll, *mpll, *epll, *vpll, *bpll, *gpll, *cpll;
+	unsigned long fin_pll_rate, mout_vpllsrc_rate;
 
 	if (np) {
 		reg_base = of_iomap(np, 0);
@@ -497,6 +518,9 @@ void __init exynos5250_clk_init(struct device_node *np)
 	samsung_clk_register_mux(exynos5250_pll_pmux_clks,
 			ARRAY_SIZE(exynos5250_pll_pmux_clks));
 
+	fin_pll_rate = _get_rate("fin_pll");
+	mout_vpllsrc_rate = _get_rate("mout_vpllsrc");
+
 	apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
 			reg_base, NULL, 0);
 	mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
@@ -507,10 +531,28 @@ void __init exynos5250_clk_init(struct device_node *np)
 			reg_base + 0x10050, NULL, 0);
 	cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
 			reg_base + 0x10020, NULL, 0);
-	epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
-			reg_base + 0x10030, NULL, 0);
-	vpll = samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc",
+
+	if (fin_pll_rate == (24 * MHZ)) {
+		epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
+				reg_base + 0x10030, epll_24mhz_tbl,
+				ARRAY_SIZE(epll_24mhz_tbl));
+	} else {
+		pr_warn("Exynos5250: valid epll rate_table missing for\n"
+			"parent fin_pll:%lu hz\n", fin_pll_rate);
+		epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
+				reg_base + 0x10030, NULL, 0);
+	}
+
+	if (mout_vpllsrc_rate == (24 * MHZ)) {
+		vpll = samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc"
+			, reg_base + 0x10040, vpll_24mhz_tbl,
+			ARRAY_SIZE(vpll_24mhz_tbl));
+	} else {
+		pr_warn("Exynos5250: valid vpll rate_table missing for\n"
+			"parent mout_vpllsrc_rate:%lu hz\n", mout_vpllsrc_rate);
+		samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc",
 			reg_base + 0x10040, NULL, 0);
+	}
 
 	samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
 			ARRAY_SIZE(exynos5250_fixed_rate_clks));
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index e4ad6ea..c997649 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -20,6 +20,8 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
+#define MHZ (1000*1000)
+
 /**
  * struct samsung_clock_alias: information about mux clock
  * @id: platform specific id of the clock.
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 2/6] clk: samsung: Add support to register rate_table for PLL3xxx
  2013-05-31 12:31 ` [PATCH v3 2/6] clk: samsung: Add support to register rate_table " Vikas Sajjan
@ 2013-05-31 17:08   ` Doug Anderson
  0 siblings, 0 replies; 18+ messages in thread
From: Doug Anderson @ 2013-05-31 17:08 UTC (permalink / raw)
  To: linux-arm-kernel

Vikas and Yadwinder,

On Fri, May 31, 2013 at 5:31 AM, Vikas Sajjan <vikas.sajjan@linaro.org> wrote:
> From: Yadwinder Singh Brar <yadi.brar@samsung.com>
>
> This patch defines a common rate_table which will contain recommended p, m, s,
> k values for supported rates that needs to be changed for changing
> corresponding PLL's rate.
>
> Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos4.c    |    8 ++++----
>  drivers/clk/samsung/clk-exynos5250.c |   14 +++++++-------
>  drivers/clk/samsung/clk-pll.c        |   14 ++++++++++++--
>  drivers/clk/samsung/clk-pll.h        |   35 ++++++++++++++++++++++++++++++++--
>  4 files changed, 56 insertions(+), 15 deletions(-)

This looks good to me.  Hopefully Tomasz agrees.  Tomasz: if you
haven't been following this thread, see
<https://patchwork.kernel.org/patch/2643351/> for how we resolved the
constant vs. calculated input clock.

Reviewed-by: Doug Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 3/6] clk: samsung: Add set_rate() clk_ops for PLL35xx
  2013-05-31 12:31 ` [PATCH v3 3/6] clk: samsung: Add set_rate() clk_ops for PLL35xx Vikas Sajjan
@ 2013-05-31 17:10   ` Doug Anderson
  2013-06-08 12:12   ` Tomasz Figa
  1 sibling, 0 replies; 18+ messages in thread
From: Doug Anderson @ 2013-05-31 17:10 UTC (permalink / raw)
  To: linux-arm-kernel

Vikas and Yadwinder,

On Fri, May 31, 2013 at 5:31 AM, Vikas Sajjan <vikas.sajjan@linaro.org> wrote:
> +static long samsung_pll_round_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_table = pll->rate_table;
> +       int i;
> +
> +       /* Assumming rate_table is in descending order */
> +       for (i = 0; i < pll->rate_count; i++) {
> +               if (drate >= rate_table[i].rate)
> +                       return rate_table[i].rate;
> +       }
> +
> +       /* return minimum supported value */
> +       return rate_table[i - 1].rate;
> +}
>  /*
>   * PLL35xx Clock Type
>   */

You seem to have lost a carriage return between v2 and v3.  I will add
it locally before applying.  This still looks good though and already
has my Reviewed-by.

-Doug

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 4/6] clk: samsung: Add set_rate() clk_ops for PLL36xx
  2013-05-31 12:31 ` [PATCH v3 4/6] clk: samsung: Add set_rate() clk_ops for PLL36xx Vikas Sajjan
@ 2013-05-31 17:15   ` Doug Anderson
  2013-06-08 12:17   ` Tomasz Figa
  1 sibling, 0 replies; 18+ messages in thread
From: Doug Anderson @ 2013-05-31 17:15 UTC (permalink / raw)
  To: linux-arm-kernel

Vikas and Yadwinder,

On Fri, May 31, 2013 at 5:31 AM, Vikas Sajjan <vikas.sajjan@linaro.org> wrote:
>  struct clk * __init samsung_clk_register_pll36xx(const char *name,
> @@ -264,6 +318,11 @@ struct clk * __init samsung_clk_register_pll36xx(const char *name,
>         init.parent_names = &pname;
>         init.num_parents = 1;
>
> +       if (rate_table && rate_count)
> +               init.ops = &samsung_pll36xx_clk_ops;
> +       else
> +               init.ops = &samsung_pll36xx_clk_min_ops;
> +
>         pll->hw.init = &init;
>         pll->base = base;
>         pll->rate_table = rate_table;

Interesting.  In the gerrit review you sent for v2 you properly
removed the line:

  init.ops = &samsung_pll36xx_clk_ops;

...but the v2 you posted upstream didn't have that removal.  You've
also lost it in v3.  Perhaps you can add that back in?  I'll do it
locally before applying.


-Doug

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 5/6] clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration for it
  2013-05-31 12:31 ` [PATCH v3 5/6] clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration for it Vikas Sajjan
@ 2013-05-31 17:20   ` Doug Anderson
  0 siblings, 0 replies; 18+ messages in thread
From: Doug Anderson @ 2013-05-31 17:20 UTC (permalink / raw)
  To: linux-arm-kernel

Vikas and Yadwinder,

On Fri, May 31, 2013 at 5:31 AM, Vikas Sajjan <vikas.sajjan@linaro.org> wrote:
> While trying to get rate of "mout_vpllsrc" MUX (parent) for registering the
> "fout_vpll" (child), we found get rate was failing.
>
> So this patch moves the mout_vpllsrc MUX out of the existing common list
> and registers the mout_vpllsrc MUX before the PLL registrations.
> Its also adds the alias for the mout_vpllsrc MUX.

I think the moving makes sense, but not the alias.  IMHO global
aliases should be avoided unless they can be strongly justified.

In part 6 of this series I see where you're using the alias but I
don't think you need it.  In your case you are a clock provider, so I
think you can use __clk_lookup("mout_vpllsrc") to find your clock.

I will post an update on our gerrit and you can see what you think.
If someone on the list thinks that using __clk_lookup() is a bad idea
and they'd rather see the global alias, please shout.


-Doug

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 6/6] clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC
  2013-05-31 12:31 ` [PATCH v3 6/6] clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC Vikas Sajjan
@ 2013-05-31 17:58   ` Doug Anderson
  0 siblings, 0 replies; 18+ messages in thread
From: Doug Anderson @ 2013-05-31 17:58 UTC (permalink / raw)
  To: linux-arm-kernel

Vikas and Yadwinder,

On Fri, May 31, 2013 at 5:31 AM, Vikas Sajjan <vikas.sajjan@linaro.org> wrote:
> Adds the EPLL and VPLL freq table for exynos5250 SoC.
>
> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> ---
>  drivers/clk/samsung/clk-exynos5250.c |   48 +++++++++++++++++++++++++++++++---
>  drivers/clk/samsung/clk.h            |    2 ++
>  2 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
> index b0e6680..0566421 100644
> --- a/drivers/clk/samsung/clk-exynos5250.c
> +++ b/drivers/clk/samsung/clk-exynos5250.c
> @@ -473,11 +473,32 @@ static __initdata struct of_device_id ext_clk_match[] = {
>         { },
>  };
>
> +static const struct samsung_pll_rate_table vpll_24mhz_tbl[] = {
> +       /* sorted in descending order */
> +       /* PLL_36XX_RATE(rate, m, p, s, k) */
> +       PLL_36XX_RATE(266000000, 266, 3, 3, 0),
> +       PLL_36XX_RATE(70500000, 94, 2, 4, 0),

Would be nice to include the comment that you included in our gerrit:
that the 70.5 is not in the manual but is used by exynos5250-snow.


> +       fin_pll_rate = _get_rate("fin_pll");
> +       mout_vpllsrc_rate = _get_rate("mout_vpllsrc");

This line is why you added an alias for mout_vpllsrc.  I'd rather not
see that since it also exports the clock to other places.

I've changed this to use __clk_lookup().  That function _is_ exported
by <linux/clk-provider.h> and we are a clock provider, so it seems
legit to use that.  ...and it's nice not to have an extra clock alias.

See my changes to <https://gerrit.chromium.org/gerrit/#/c/56797/> for
an example.


> @@ -507,10 +531,28 @@ void __init exynos5250_clk_init(struct device_node *np)
>                         reg_base + 0x10050, NULL, 0);
>         cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
>                         reg_base + 0x10020, NULL, 0);
> -       epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
> -                       reg_base + 0x10030, NULL, 0);
> -       vpll = samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc",
> +
> +       if (fin_pll_rate == (24 * MHZ)) {
> +               epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
> +                               reg_base + 0x10030, epll_24mhz_tbl,
> +                               ARRAY_SIZE(epll_24mhz_tbl));
> +       } else {
> +               pr_warn("Exynos5250: valid epll rate_table missing for\n"
> +                       "parent fin_pll:%lu hz\n", fin_pll_rate);

nit: use %s and __func__ rather than adding Exynos5250 hardcoded in here?


-Doug

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs
  2013-05-31 12:31 [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Vikas Sajjan
                   ` (5 preceding siblings ...)
  2013-05-31 12:31 ` [PATCH v3 6/6] clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC Vikas Sajjan
@ 2013-06-03 21:06 ` Doug Anderson
  6 siblings, 0 replies; 18+ messages in thread
From: Doug Anderson @ 2013-06-03 21:06 UTC (permalink / raw)
  To: linux-arm-kernel

Vikas,

On Fri, May 31, 2013 at 5:31 AM, Vikas Sajjan <vikas.sajjan@linaro.org> wrote:
> This patch series does the following:
>
>  1) Factors out possible common code, unifies the clk strutures used
>     for PLL35xx & PLL36xx and usues clk->base instead of clk->con0
>
>  2) Defines a common rate_table which will contain recommended p, m, s and k
>     values for supported rates that needs to be changed for changing
>     corresponding PLL's rate
>
>  3) Adds set_rate() and round_rate() clk_ops for PLL35xx and PLL36xx
>
> changes since v2:
>         - Added new patch to reorder the MUX registration for mout_vpllsrc MUX
>         before the PLL registrations. And to add the alias for the mout_vpllsrc MUX.
>         - Added a check to confirm parent rate while registrating the PLL
>         rate tables.
>
> changes since v1:
>         - removed sorting and bsearch
>         - modified the definition of struct "samsung_pll_rate_table"
>         - added generic round_rate()
>         - rectified the ops assignment for "rate table passed as NULL"
>           during PLL registration
>
> Is rebased on branch kgene's "for-next"
> https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/log/?h=for-next
>
> And tested these patch on chromebook for EPLL settings for Audio on our chrome tree.
>
>
> Vikas Sajjan (3):
>   clk: samsung: Add set_rate() clk_ops for PLL36xx
>   clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration
>     for it
>   clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC
>
> Yadwinder Singh Brar (3):
>   clk: samsung: Use clk->base instead of directly using clk->con0 for
>     PLL3xxx
>   clk: samsung: Add support to register rate_table for PLL3xxx
>   clk: samsung: Add set_rate() clk_ops for PLL35xx
>
>  drivers/clk/samsung/clk-exynos4.c    |   10 +-
>  drivers/clk/samsung/clk-exynos5250.c |   69 +++++++++--
>  drivers/clk/samsung/clk-pll.c        |  226 ++++++++++++++++++++++++++++++----
>  drivers/clk/samsung/clk-pll.h        |   35 +++++-
>  drivers/clk/samsung/clk.h            |    2 +
>  5 files changed, 300 insertions(+), 42 deletions(-)

I'm hoping that we'll see a v4 of this series after you and Tomasz
figure out what to do about the fin_pll issue.  Ideally you can pick
up the little tweaks I added before landing this series in kernel-next
for ChromeOS (if you are OK with them, that is).

Thanks!

-Doug

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/6] clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx
  2013-05-31 12:31 ` [PATCH v3 1/6] clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx Vikas Sajjan
@ 2013-06-08 12:04   ` Tomasz Figa
  0 siblings, 0 replies; 18+ messages in thread
From: Tomasz Figa @ 2013-06-08 12:04 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Yadwinder,

On Friday 31 of May 2013 18:01:31 Vikas Sajjan wrote:
> From: Yadwinder Singh Brar <yadi.brar@samsung.com>
> 
> To factor out possible common code, this patch unifies the clk strutures
> used for PLL35xx & PLL36xx and usues clk->base instead of clk->con0.

As note for v4, please adjust the commit message, as it seems to be rather 
misleading - it says only about one part of the changes.

Ideally this should be split into two patches, one factoring out common 
code and another replacing clk->con0 with clk->base.

Best regards,
Tomasz

> Reviewed-by: Tomasz Figa <t.figa@samsung.com>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> Tested-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos4.c    |   10 ++++---
>  drivers/clk/samsung/clk-exynos5250.c |   14 ++++-----
>  drivers/clk/samsung/clk-pll.c        |   54
> ++++++++++++++++++---------------- drivers/clk/samsung/clk-pll.h       
> |    4 +--
>  4 files changed, 44 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos4.c
> b/drivers/clk/samsung/clk-exynos4.c index d0940e6..cf7d4e7 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -97,12 +97,14 @@
>  #define GATE_IP_PERIL		0xc950
>  #define E4210_GATE_IP_PERIR	0xc960
>  #define GATE_BLOCK		0xc970
> +#define E4X12_MPLL_LOCK		0x10008
>  #define E4X12_MPLL_CON0		0x10108
>  #define SRC_DMC			0x10200
>  #define SRC_MASK_DMC		0x10300
>  #define DIV_DMC0		0x10500
>  #define DIV_DMC1		0x10504
>  #define GATE_IP_DMC		0x10900
> +#define APLL_LOCK		0x14000
>  #define APLL_CON0		0x14100
>  #define E4210_MPLL_CON0		0x14108
>  #define SRC_CPU			0x14200
> @@ -1019,13 +1021,13 @@ void __init exynos4_clk_init(struct device_node
> *np, enum exynos4_soc exynos4_so reg_base + VPLL_CON0, pll_4650c);
>  	} else {
>  		apll = samsung_clk_register_pll35xx("fout_apll", 
"fin_pll",
> -					reg_base + APLL_CON0);
> +					reg_base + APLL_LOCK);
>  		mpll = samsung_clk_register_pll35xx("fout_mpll", 
"fin_pll",
> -					reg_base + E4X12_MPLL_CON0);
> +					reg_base + E4X12_MPLL_LOCK);
>  		epll = samsung_clk_register_pll36xx("fout_epll", 
"fin_pll",
> -					reg_base + EPLL_CON0);
> +					reg_base + EPLL_LOCK);
>  		vpll = samsung_clk_register_pll36xx("fout_vpll", 
"fin_pll",
> -					reg_base + VPLL_CON0);
> +					reg_base + VPLL_LOCK);
>  	}
> 
>  	samsung_clk_add_lookup(apll, fout_apll);
> diff --git a/drivers/clk/samsung/clk-exynos5250.c
> b/drivers/clk/samsung/clk-exynos5250.c index 5c97e75..687b580 100644
> --- a/drivers/clk/samsung/clk-exynos5250.c
> +++ b/drivers/clk/samsung/clk-exynos5250.c
> @@ -491,19 +491,19 @@ void __init exynos5250_clk_init(struct device_node
> *np) ext_clk_match);
> 
>  	apll = samsung_clk_register_pll35xx("fout_apll", "fin_pll",
> -			reg_base + 0x100);
> +			reg_base);
>  	mpll = samsung_clk_register_pll35xx("fout_mpll", "fin_pll",
> -			reg_base + 0x4100);
> +			reg_base + 0x4000);
>  	bpll = samsung_clk_register_pll35xx("fout_bpll", "fin_pll",
> -			reg_base + 0x20110);
> +			reg_base + 0x20010);
>  	gpll = samsung_clk_register_pll35xx("fout_gpll", "fin_pll",
> -			reg_base + 0x10150);
> +			reg_base + 0x10050);
>  	cpll = samsung_clk_register_pll35xx("fout_cpll", "fin_pll",
> -			reg_base + 0x10120);
> +			reg_base + 0x10020);
>  	epll = samsung_clk_register_pll36xx("fout_epll", "fin_pll",
> -			reg_base + 0x10130);
> +			reg_base + 0x10030);
>  	vpll = samsung_clk_register_pll36xx("fout_vpll", "mout_vpllsrc",
> -			reg_base + 0x10140);
> +			reg_base + 0x10040);
> 
>  	samsung_clk_register_fixed_rate(exynos5250_fixed_rate_clks,
>  			ARRAY_SIZE(exynos5250_fixed_rate_clks));
> diff --git a/drivers/clk/samsung/clk-pll.c
> b/drivers/clk/samsung/clk-pll.c index 89135f6..a7d8ad9 100644
> --- a/drivers/clk/samsung/clk-pll.c
> +++ b/drivers/clk/samsung/clk-pll.c
> @@ -13,9 +13,24 @@
>  #include "clk.h"
>  #include "clk-pll.h"
> 
> +struct samsung_clk_pll {
> +	struct clk_hw		hw;
> +	const void __iomem	*base;
> +};
> +
> +#define to_clk_pll(_hw) container_of(_hw, struct samsung_clk_pll, hw)
> +
> +#define pll_readl(pll, offset)						
\
> +		__raw_readl((void __iomem *)(pll->base + (offset)));
> +#define pll_writel(pll, val, offset)					\
> +		__raw_writel(val, (void __iomem *)(pll->base + (offset)));
> +
>  /*
>   * PLL35xx Clock Type
>   */
> +#define PLL35XX_LOCK_OFFSET	(0x0)
> +#define PLL35XX_CON0_OFFSET	(0x100)
> +#define PLL35XX_CON1_OFFSET	(0x104)
> 
>  #define PLL35XX_MDIV_MASK       (0x3FF)
>  #define PLL35XX_PDIV_MASK       (0x3F)
> @@ -24,21 +39,14 @@
>  #define PLL35XX_PDIV_SHIFT      (8)
>  #define PLL35XX_SDIV_SHIFT      (0)
> 
> -struct samsung_clk_pll35xx {
> -	struct clk_hw		hw;
> -	const void __iomem	*con_reg;
> -};
> -
> -#define to_clk_pll35xx(_hw) container_of(_hw, struct
> samsung_clk_pll35xx, hw) -
>  static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,
>  				unsigned long parent_rate)
>  {
> -	struct samsung_clk_pll35xx *pll = to_clk_pll35xx(hw);
> +	struct samsung_clk_pll *pll = to_clk_pll(hw);
>  	u32 mdiv, pdiv, sdiv, pll_con;
>  	u64 fvco = parent_rate;
> 
> -	pll_con = __raw_readl(pll->con_reg);
> +	pll_con = pll_readl(pll, PLL35XX_CON0_OFFSET);
>  	mdiv = (pll_con >> PLL35XX_MDIV_SHIFT) & PLL35XX_MDIV_MASK;
>  	pdiv = (pll_con >> PLL35XX_PDIV_SHIFT) & PLL35XX_PDIV_MASK;
>  	sdiv = (pll_con >> PLL35XX_SDIV_SHIFT) & PLL35XX_SDIV_MASK;
> @@ -54,9 +62,9 @@ static const struct clk_ops samsung_pll35xx_clk_ops =
> { };
> 
>  struct clk * __init samsung_clk_register_pll35xx(const char *name,
> -			const char *pname, const void __iomem *con_reg)
> +			const char *pname, const void __iomem *base)
>  {
> -	struct samsung_clk_pll35xx *pll;
> +	struct samsung_clk_pll *pll;
>  	struct clk *clk;
>  	struct clk_init_data init;
> 
> @@ -73,7 +81,7 @@ struct clk * __init samsung_clk_register_pll35xx(const
> char *name, init.num_parents = 1;
> 
>  	pll->hw.init = &init;
> -	pll->con_reg = con_reg;
> +	pll->base = base;
> 
>  	clk = clk_register(NULL, &pll->hw);
>  	if (IS_ERR(clk)) {
> @@ -91,6 +99,9 @@ struct clk * __init samsung_clk_register_pll35xx(const
> char *name, /*
>   * PLL36xx Clock Type
>   */
> +#define PLL36XX_LOCK_OFFSET	(0x0)
> +#define PLL36XX_CON0_OFFSET	(0x100)
> +#define PLL36XX_CON1_OFFSET	(0x104)
> 
>  #define PLL36XX_KDIV_MASK	(0xFFFF)
>  #define PLL36XX_MDIV_MASK	(0x1FF)
> @@ -100,22 +111,15 @@ struct clk * __init
> samsung_clk_register_pll35xx(const char *name, #define
> PLL36XX_PDIV_SHIFT	(8)
>  #define PLL36XX_SDIV_SHIFT	(0)
> 
> -struct samsung_clk_pll36xx {
> -	struct clk_hw		hw;
> -	const void __iomem	*con_reg;
> -};
> -
> -#define to_clk_pll36xx(_hw) container_of(_hw, struct
> samsung_clk_pll36xx, hw) -
>  static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw,
>  				unsigned long parent_rate)
>  {
> -	struct samsung_clk_pll36xx *pll = to_clk_pll36xx(hw);
> +	struct samsung_clk_pll *pll = to_clk_pll(hw);
>  	u32 mdiv, pdiv, sdiv, kdiv, pll_con0, pll_con1;
>  	u64 fvco = parent_rate;
> 
> -	pll_con0 = __raw_readl(pll->con_reg);
> -	pll_con1 = __raw_readl(pll->con_reg + 4);
> +	pll_con0 = pll_readl(pll, PLL36XX_CON0_OFFSET);
> +	pll_con1 = pll_readl(pll, PLL36XX_CON1_OFFSET);
>  	mdiv = (pll_con0 >> PLL36XX_MDIV_SHIFT) & PLL36XX_MDIV_MASK;
>  	pdiv = (pll_con0 >> PLL36XX_PDIV_SHIFT) & PLL36XX_PDIV_MASK;
>  	sdiv = (pll_con0 >> PLL36XX_SDIV_SHIFT) & PLL36XX_SDIV_MASK;
> @@ -133,9 +137,9 @@ static const struct clk_ops samsung_pll36xx_clk_ops
> = { };
> 
>  struct clk * __init samsung_clk_register_pll36xx(const char *name,
> -			const char *pname, const void __iomem *con_reg)
> +			const char *pname, const void __iomem *base)
>  {
> -	struct samsung_clk_pll36xx *pll;
> +	struct samsung_clk_pll *pll;
>  	struct clk *clk;
>  	struct clk_init_data init;
> 
> @@ -152,7 +156,7 @@ struct clk * __init
> samsung_clk_register_pll36xx(const char *name, init.num_parents = 1;
> 
>  	pll->hw.init = &init;
> -	pll->con_reg = con_reg;
> +	pll->base = base;
> 
>  	clk = clk_register(NULL, &pll->hw);
>  	if (IS_ERR(clk)) {
> diff --git a/drivers/clk/samsung/clk-pll.h
> b/drivers/clk/samsung/clk-pll.h index f33786e..1329522 100644
> --- a/drivers/clk/samsung/clk-pll.h
> +++ b/drivers/clk/samsung/clk-pll.h
> @@ -25,9 +25,9 @@ enum pll46xx_type {
>  };
> 
>  extern struct clk * __init samsung_clk_register_pll35xx(const char
> *name, -			const char *pname, const void __iomem 
*con_reg);
> +			const char *pname, const void __iomem *base);
>  extern struct clk * __init samsung_clk_register_pll36xx(const char
> *name, -			const char *pname, const void __iomem 
*con_reg);
> +			const char *pname, const void __iomem *base);
>  extern struct clk * __init samsung_clk_register_pll45xx(const char
> *name, const char *pname, const void __iomem *con_reg,
>  			enum pll45xx_type type);

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 3/6] clk: samsung: Add set_rate() clk_ops for PLL35xx
  2013-05-31 12:31 ` [PATCH v3 3/6] clk: samsung: Add set_rate() clk_ops for PLL35xx Vikas Sajjan
  2013-05-31 17:10   ` Doug Anderson
@ 2013-06-08 12:12   ` Tomasz Figa
  2013-06-12 14:51     ` Yadwinder Singh Brar
  1 sibling, 1 reply; 18+ messages in thread
From: Tomasz Figa @ 2013-06-08 12:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 31 of May 2013 18:01:33 Vikas Sajjan wrote:
> From: Yadwinder Singh Brar <yadi.brar@samsung.com>
> 
> This patch add set_rate() and round_rate() for PLL35xx
> 
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
> ---
>  drivers/clk/samsung/clk-pll.c |  103
> ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 102
> insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/samsung/clk-pll.c
> b/drivers/clk/samsung/clk-pll.c index 8226528..9591560 100644
> --- a/drivers/clk/samsung/clk-pll.c
> +++ b/drivers/clk/samsung/clk-pll.c
> @@ -27,6 +27,36 @@ struct samsung_clk_pll {
>  #define pll_writel(pll, val, offset)					\
>  		__raw_writel(val, (void __iomem *)(pll->base + (offset)));
> 
> +static const struct samsung_pll_rate_table *samsung_get_pll_settings(
> +				struct samsung_clk_pll *pll, unsigned long 
rate)
> +{
> +	const struct samsung_pll_rate_table  *rate_table = pll-
>rate_table;
> +	int i;
> +
> +	for (i = 0; i < pll->rate_count; i++) {
> +		if (rate == rate_table[i].rate)
> +			return &rate_table[i];
> +	}
> +
> +	return NULL;
> +}
> +
> +static long samsung_pll_round_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_table = pll->rate_table;
> +	int i;
> +
> +	/* Assumming rate_table is in descending order */
> +	for (i = 0; i < pll->rate_count; i++) {
> +		if (drate >= rate_table[i].rate)
> +			return rate_table[i].rate;
> +	}
> +
> +	/* return minimum supported value */
> +	return rate_table[i - 1].rate;
> +}
>  /*
>   * PLL35xx Clock Type
>   */
> @@ -34,12 +64,17 @@ struct samsung_clk_pll {
>  #define PLL35XX_CON0_OFFSET	(0x100)
>  #define PLL35XX_CON1_OFFSET	(0x104)
> 
> +/* Maximum lock time can be 270 * PDIV cycles */
> +#define PLL35XX_LOCK_FACTOR	(270)
> +
>  #define PLL35XX_MDIV_MASK       (0x3FF)
>  #define PLL35XX_PDIV_MASK       (0x3F)
>  #define PLL35XX_SDIV_MASK       (0x7)
> +#define PLL35XX_LOCK_STAT_MASK  (0x1)
>  #define PLL35XX_MDIV_SHIFT      (16)
>  #define PLL35XX_PDIV_SHIFT      (8)
>  #define PLL35XX_SDIV_SHIFT      (0)
> +#define PLL35XX_LOCK_STAT_SHIFT	(29)
> 
>  static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,
>  				unsigned long parent_rate)
> @@ -59,8 +94,70 @@ static unsigned long
> samsung_pll35xx_recalc_rate(struct clk_hw *hw, return (unsigned
> long)fvco;
>  }
> 
> +static inline bool samsung_pll35xx_mp_change(u32 mdiv, u32 pdiv, u32
> pll_con) +{
> +	if ((mdiv != ((pll_con >> PLL35XX_MDIV_SHIFT) & 
PLL35XX_MDIV_MASK)) ||
> +		(pdiv != ((pll_con >> PLL35XX_PDIV_SHIFT) & 
PLL35XX_PDIV_MASK)))
> +		return 1;
> +	else
> +		return 0;

Readability of this function could be improved by moving some code out of 
the if clause, like:

static inline bool samsung_pll35xx_mp_change(u32 mdiv,
						u32 pdiv, u32 pll_con)
{
	u32 old_mdiv, old_pdiv;

	old_mdiv = (pll_con >> PLL35XX_MDIV_SHIFT) & PLL35XX_MDIV_MASK;
	old_pdiv = (pll_con >> PLL35XX_PDIV_SHIFT) & PLL35XX_PDIV_MASK;

	return (mdiv != old_mdiv || pdiv != old_pdiv);
}

> +}
> +
> +static int samsung_pll35xx_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 tmp;
> +
> +	/* 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_get_name(hw->clk));
> +		return -EINVAL;
> +	}
> +
> +	tmp = pll_readl(pll, PLL35XX_CON0_OFFSET);
> +
> +	if (!(samsung_pll35xx_mp_change(rate->mdiv, rate->pdiv, tmp))) {
> +		/* If only s change, change just s value only*/
> +		tmp &= ~(PLL35XX_SDIV_MASK << PLL35XX_SDIV_SHIFT);
> +		tmp |= rate->sdiv << PLL35XX_SDIV_SHIFT;
> +		pll_writel(pll, tmp, PLL35XX_CON0_OFFSET);

To improve readability of this code, return 0 could be added here and 
following code could be moved out of the else clause.

Best regards,
Tomasz

> +	} else {
> +		/* Set PLL lock time. */
> +		pll_writel(pll, rate->pdiv * PLL35XX_LOCK_FACTOR,
> +				PLL35XX_LOCK_OFFSET);
> +
> +		/* Change PLL PMS values */
> +		tmp &= ~((PLL35XX_MDIV_MASK << PLL35XX_MDIV_SHIFT) |
> +				(PLL35XX_PDIV_MASK << PLL35XX_PDIV_SHIFT) 
|
> +				(PLL35XX_SDIV_MASK << 
PLL35XX_SDIV_SHIFT));
> +		tmp |= (rate->mdiv << PLL35XX_MDIV_SHIFT) |
> +				(rate->pdiv << PLL35XX_PDIV_SHIFT) |
> +				(rate->sdiv << PLL35XX_SDIV_SHIFT);
> +		pll_writel(pll, tmp, PLL35XX_CON0_OFFSET);
> +
> +		/* wait_lock_time */
> +		do {
> +			cpu_relax();
> +			tmp = pll_readl(pll, PLL35XX_CON0_OFFSET);
> +		} while (!(tmp & (PLL35XX_LOCK_STAT_MASK
> +				<< PLL35XX_LOCK_STAT_SHIFT)));
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct clk_ops samsung_pll35xx_clk_ops = {
>  	.recalc_rate = samsung_pll35xx_recalc_rate,
> +	.round_rate = samsung_pll_round_rate,
> +	.set_rate = samsung_pll35xx_set_rate,
> +};
> +
> +static const struct clk_ops samsung_pll35xx_clk_min_ops = {
> +	.recalc_rate = samsung_pll35xx_recalc_rate,
>  };
> 
>  struct clk * __init samsung_clk_register_pll35xx(const char *name,
> @@ -79,11 +176,15 @@ struct clk * __init
> samsung_clk_register_pll35xx(const char *name, }
> 
>  	init.name = name;
> -	init.ops = &samsung_pll35xx_clk_ops;
>  	init.flags = CLK_GET_RATE_NOCACHE;
>  	init.parent_names = &pname;
>  	init.num_parents = 1;
> 
> +	if (rate_table && rate_count)
> +		init.ops = &samsung_pll35xx_clk_ops;
> +	else
> +		init.ops = &samsung_pll35xx_clk_min_ops;
> +
>  	pll->hw.init = &init;
>  	pll->base = base;
>  	pll->rate_table = rate_table;

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 4/6] clk: samsung: Add set_rate() clk_ops for PLL36xx
  2013-05-31 12:31 ` [PATCH v3 4/6] clk: samsung: Add set_rate() clk_ops for PLL36xx Vikas Sajjan
  2013-05-31 17:15   ` Doug Anderson
@ 2013-06-08 12:17   ` Tomasz Figa
  2013-06-12  6:09     ` Vikas Sajjan
  1 sibling, 1 reply; 18+ messages in thread
From: Tomasz Figa @ 2013-06-08 12:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 31 of May 2013 18:01:34 Vikas Sajjan wrote:
> This patch adds set_rate and round_rate clk_ops for PLL36xx
> 
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
> ---
>  drivers/clk/samsung/clk-pll.c |   59
> +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59
> insertions(+)
> 
> diff --git a/drivers/clk/samsung/clk-pll.c
> b/drivers/clk/samsung/clk-pll.c index 9591560..7143ed89 100644
> --- a/drivers/clk/samsung/clk-pll.c
> +++ b/drivers/clk/samsung/clk-pll.c
> @@ -210,6 +210,9 @@ struct clk * __init
> samsung_clk_register_pll35xx(const char *name, #define
> PLL36XX_CON0_OFFSET	(0x100)
>  #define PLL36XX_CON1_OFFSET	(0x104)
> 
> +/* Maximum lock time can be 3000 * PDIV cycles */
> +#define PLL36XX_LOCK_FACTOR	(3000)
> +
>  #define PLL36XX_KDIV_MASK	(0xFFFF)
>  #define PLL36XX_MDIV_MASK	(0x1FF)
>  #define PLL36XX_PDIV_MASK	(0x3F)
> @@ -217,6 +220,8 @@ struct clk * __init
> samsung_clk_register_pll35xx(const char *name, #define
> PLL36XX_MDIV_SHIFT	(16)
>  #define PLL36XX_PDIV_SHIFT	(8)
>  #define PLL36XX_SDIV_SHIFT	(0)
> +#define PLL36XX_KDIV_SHIFT	(0)
> +#define PLL36XX_LOCK_STAT_SHIFT	(29)
> 
>  static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw,
>  				unsigned long parent_rate)
> @@ -239,8 +244,57 @@ static unsigned long
> samsung_pll36xx_recalc_rate(struct clk_hw *hw, return (unsigned
> long)fvco;
>  }
> 
> +static int samsung_pll36xx_set_rate(struct clk_hw *hw, unsigned long
> drate, +					unsigned long parent_rate)
> +{
> +	struct samsung_clk_pll *pll = to_clk_pll(hw);
> +	u32 tmp, pll_con0, pll_con1;
> +	const struct samsung_pll_rate_table *rate;
> +
> +	rate = samsung_get_pll_settings(pll, drate);
> +	if (!rate) {
> +		pr_err("%s: Invalid rate : %lu for pll clk %s\n", 
__func__,
> +			drate, __clk_get_name(hw->clk));
> +		return -EINVAL;
> +	}
> +
> +	pll_con0 = pll_readl(pll, PLL36XX_CON0_OFFSET);
> +	pll_con1 = pll_readl(pll, PLL36XX_CON1_OFFSET);

Hmm, PLL35xx has a fast path when only the S divisor changes. I'm not sure 
about any technical differences between these two PLLs (other than having 
the extra K factor and some more tunnables), but maybe it is possible for 
this one as well?

Otherwise looks fine.

Reviewed-by: Tomasz Figa <t.figa@samsung.com>

> +
> +	/* Set PLL lock time. */
> +	pll_writel(pll, (rate->pdiv * PLL36XX_LOCK_FACTOR),
> +			PLL36XX_LOCK_OFFSET);
> +
> +	 /* Change PLL PMS values */
> +	pll_con0 &= ~((PLL36XX_MDIV_MASK << PLL36XX_MDIV_SHIFT) |
> +			(PLL36XX_PDIV_MASK << PLL36XX_PDIV_SHIFT) |
> +			(PLL36XX_SDIV_MASK << PLL36XX_SDIV_SHIFT));
> +	pll_con0 |= (rate->mdiv << PLL36XX_MDIV_SHIFT) |
> +			(rate->pdiv << PLL36XX_PDIV_SHIFT) |
> +			(rate->sdiv << PLL36XX_SDIV_SHIFT);
> +	pll_writel(pll, pll_con0, PLL36XX_CON0_OFFSET);
> +
> +	pll_con1 &= ~(PLL36XX_KDIV_MASK << PLL36XX_KDIV_SHIFT);
> +	pll_con1 |= rate->kdiv << PLL36XX_KDIV_SHIFT;
> +	pll_writel(pll, pll_con1, PLL36XX_CON1_OFFSET);
> +
> +	/* wait_lock_time */
> +	do {
> +		cpu_relax();
> +		tmp = pll_readl(pll, PLL36XX_CON0_OFFSET);
> +	} while (!(tmp & (1 << PLL36XX_LOCK_STAT_SHIFT)));
> +
> +	return 0;
> +}
> +
>  static const struct clk_ops samsung_pll36xx_clk_ops = {
>  	.recalc_rate = samsung_pll36xx_recalc_rate,
> +	.set_rate = samsung_pll36xx_set_rate,
> +	.round_rate = samsung_pll_round_rate,
> +};
> +
> +static const struct clk_ops samsung_pll36xx_clk_min_ops = {
> +	.recalc_rate = samsung_pll36xx_recalc_rate,
>  };
> 
>  struct clk * __init samsung_clk_register_pll36xx(const char *name,
> @@ -264,6 +318,11 @@ struct clk * __init
> samsung_clk_register_pll36xx(const char *name, init.parent_names =
> &pname;
>  	init.num_parents = 1;
> 
> +	if (rate_table && rate_count)
> +		init.ops = &samsung_pll36xx_clk_ops;
> +	else
> +		init.ops = &samsung_pll36xx_clk_min_ops;
> +
>  	pll->hw.init = &init;
>  	pll->base = base;
>  	pll->rate_table = rate_table;

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 4/6] clk: samsung: Add set_rate() clk_ops for PLL36xx
  2013-06-08 12:17   ` Tomasz Figa
@ 2013-06-12  6:09     ` Vikas Sajjan
  0 siblings, 0 replies; 18+ messages in thread
From: Vikas Sajjan @ 2013-06-12  6:09 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Tomasz,

On Sat, Jun 8, 2013 at 5:47 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On Friday 31 of May 2013 18:01:34 Vikas Sajjan wrote:
>> This patch adds set_rate and round_rate clk_ops for PLL36xx
>>
>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Vikas Sajjan <vikas.sajjan@linaro.org>
>> ---
>>  drivers/clk/samsung/clk-pll.c |   59
>> +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59
>> insertions(+)
>>
>> diff --git a/drivers/clk/samsung/clk-pll.c
>> b/drivers/clk/samsung/clk-pll.c index 9591560..7143ed89 100644
>> --- a/drivers/clk/samsung/clk-pll.c
>> +++ b/drivers/clk/samsung/clk-pll.c
>> @@ -210,6 +210,9 @@ struct clk * __init
>> samsung_clk_register_pll35xx(const char *name, #define
>> PLL36XX_CON0_OFFSET   (0x100)
>>  #define PLL36XX_CON1_OFFSET  (0x104)
>>
>> +/* Maximum lock time can be 3000 * PDIV cycles */
>> +#define PLL36XX_LOCK_FACTOR  (3000)
>> +
>>  #define PLL36XX_KDIV_MASK    (0xFFFF)
>>  #define PLL36XX_MDIV_MASK    (0x1FF)
>>  #define PLL36XX_PDIV_MASK    (0x3F)
>> @@ -217,6 +220,8 @@ struct clk * __init
>> samsung_clk_register_pll35xx(const char *name, #define
>> PLL36XX_MDIV_SHIFT    (16)
>>  #define PLL36XX_PDIV_SHIFT   (8)
>>  #define PLL36XX_SDIV_SHIFT   (0)
>> +#define PLL36XX_KDIV_SHIFT   (0)
>> +#define PLL36XX_LOCK_STAT_SHIFT      (29)
>>
>>  static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw,
>>                               unsigned long parent_rate)
>> @@ -239,8 +244,57 @@ static unsigned long
>> samsung_pll36xx_recalc_rate(struct clk_hw *hw, return (unsigned
>> long)fvco;
>>  }
>>
>> +static int samsung_pll36xx_set_rate(struct clk_hw *hw, unsigned long
>> drate, +                                      unsigned long parent_rate)
>> +{
>> +     struct samsung_clk_pll *pll = to_clk_pll(hw);
>> +     u32 tmp, pll_con0, pll_con1;
>> +     const struct samsung_pll_rate_table *rate;
>> +
>> +     rate = samsung_get_pll_settings(pll, drate);
>> +     if (!rate) {
>> +             pr_err("%s: Invalid rate : %lu for pll clk %s\n",
> __func__,
>> +                     drate, __clk_get_name(hw->clk));
>> +             return -EINVAL;
>> +     }
>> +
>> +     pll_con0 = pll_readl(pll, PLL36XX_CON0_OFFSET);
>> +     pll_con1 = pll_readl(pll, PLL36XX_CON1_OFFSET);
>
> Hmm, PLL35xx has a fast path when only the S divisor changes. I'm not sure
> about any technical differences between these two PLLs (other than having
> the extra K factor and some more tunnables), but maybe it is possible for
> this one as well?


Sure, will check and respin once tested.
Yadwinder will respin the next version of this patch series , as i am
off this week.


>
> Otherwise looks fine.
>
> Reviewed-by: Tomasz Figa <t.figa@samsung.com>

Thanks.


>
>> +
>> +     /* Set PLL lock time. */
>> +     pll_writel(pll, (rate->pdiv * PLL36XX_LOCK_FACTOR),
>> +                     PLL36XX_LOCK_OFFSET);
>> +
>> +      /* Change PLL PMS values */
>> +     pll_con0 &= ~((PLL36XX_MDIV_MASK << PLL36XX_MDIV_SHIFT) |
>> +                     (PLL36XX_PDIV_MASK << PLL36XX_PDIV_SHIFT) |
>> +                     (PLL36XX_SDIV_MASK << PLL36XX_SDIV_SHIFT));
>> +     pll_con0 |= (rate->mdiv << PLL36XX_MDIV_SHIFT) |
>> +                     (rate->pdiv << PLL36XX_PDIV_SHIFT) |
>> +                     (rate->sdiv << PLL36XX_SDIV_SHIFT);
>> +     pll_writel(pll, pll_con0, PLL36XX_CON0_OFFSET);
>> +
>> +     pll_con1 &= ~(PLL36XX_KDIV_MASK << PLL36XX_KDIV_SHIFT);
>> +     pll_con1 |= rate->kdiv << PLL36XX_KDIV_SHIFT;
>> +     pll_writel(pll, pll_con1, PLL36XX_CON1_OFFSET);
>> +
>> +     /* wait_lock_time */
>> +     do {
>> +             cpu_relax();
>> +             tmp = pll_readl(pll, PLL36XX_CON0_OFFSET);
>> +     } while (!(tmp & (1 << PLL36XX_LOCK_STAT_SHIFT)));
>> +
>> +     return 0;
>> +}
>> +
>>  static const struct clk_ops samsung_pll36xx_clk_ops = {
>>       .recalc_rate = samsung_pll36xx_recalc_rate,
>> +     .set_rate = samsung_pll36xx_set_rate,
>> +     .round_rate = samsung_pll_round_rate,
>> +};
>> +
>> +static const struct clk_ops samsung_pll36xx_clk_min_ops = {
>> +     .recalc_rate = samsung_pll36xx_recalc_rate,
>>  };
>>
>>  struct clk * __init samsung_clk_register_pll36xx(const char *name,
>> @@ -264,6 +318,11 @@ struct clk * __init
>> samsung_clk_register_pll36xx(const char *name, init.parent_names =
>> &pname;
>>       init.num_parents = 1;
>>
>> +     if (rate_table && rate_count)
>> +             init.ops = &samsung_pll36xx_clk_ops;
>> +     else
>> +             init.ops = &samsung_pll36xx_clk_min_ops;
>> +
>>       pll->hw.init = &init;
>>       pll->base = base;
>>       pll->rate_table = rate_table;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 3/6] clk: samsung: Add set_rate() clk_ops for PLL35xx
  2013-06-08 12:12   ` Tomasz Figa
@ 2013-06-12 14:51     ` Yadwinder Singh Brar
  0 siblings, 0 replies; 18+ messages in thread
From: Yadwinder Singh Brar @ 2013-06-12 14:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Jun 8, 2013 at 5:42 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> On Friday 31 of May 2013 18:01:33 Vikas Sajjan wrote:
>> From: Yadwinder Singh Brar <yadi.brar@samsung.com>
>>
>> This patch add set_rate() and round_rate() for PLL35xx
>>
>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
>> ---
>>  drivers/clk/samsung/clk-pll.c |  103
>> ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 102
>> insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/samsung/clk-pll.c
>> b/drivers/clk/samsung/clk-pll.c index 8226528..9591560 100644
>> --- a/drivers/clk/samsung/clk-pll.c
>> +++ b/drivers/clk/samsung/clk-pll.c
>> @@ -27,6 +27,36 @@ struct samsung_clk_pll {
>>  #define pll_writel(pll, val, offset)                                 \
>>               __raw_writel(val, (void __iomem *)(pll->base + (offset)));
>>
>> +static const struct samsung_pll_rate_table *samsung_get_pll_settings(
>> +                             struct samsung_clk_pll *pll, unsigned long
> rate)
>> +{
>> +     const struct samsung_pll_rate_table  *rate_table = pll-
>>rate_table;
>> +     int i;
>> +
>> +     for (i = 0; i < pll->rate_count; i++) {
>> +             if (rate == rate_table[i].rate)
>> +                     return &rate_table[i];
>> +     }
>> +
>> +     return NULL;
>> +}
>> +
>> +static long samsung_pll_round_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_table = pll->rate_table;
>> +     int i;
>> +
>> +     /* Assumming rate_table is in descending order */
>> +     for (i = 0; i < pll->rate_count; i++) {
>> +             if (drate >= rate_table[i].rate)
>> +                     return rate_table[i].rate;
>> +     }
>> +
>> +     /* return minimum supported value */
>> +     return rate_table[i - 1].rate;
>> +}
>>  /*
>>   * PLL35xx Clock Type
>>   */
>> @@ -34,12 +64,17 @@ struct samsung_clk_pll {
>>  #define PLL35XX_CON0_OFFSET  (0x100)
>>  #define PLL35XX_CON1_OFFSET  (0x104)
>>
>> +/* Maximum lock time can be 270 * PDIV cycles */
>> +#define PLL35XX_LOCK_FACTOR  (270)
>> +
>>  #define PLL35XX_MDIV_MASK       (0x3FF)
>>  #define PLL35XX_PDIV_MASK       (0x3F)
>>  #define PLL35XX_SDIV_MASK       (0x7)
>> +#define PLL35XX_LOCK_STAT_MASK  (0x1)
>>  #define PLL35XX_MDIV_SHIFT      (16)
>>  #define PLL35XX_PDIV_SHIFT      (8)
>>  #define PLL35XX_SDIV_SHIFT      (0)
>> +#define PLL35XX_LOCK_STAT_SHIFT      (29)
>>
>>  static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,
>>                               unsigned long parent_rate)
>> @@ -59,8 +94,70 @@ static unsigned long
>> samsung_pll35xx_recalc_rate(struct clk_hw *hw, return (unsigned
>> long)fvco;
>>  }
>>
>> +static inline bool samsung_pll35xx_mp_change(u32 mdiv, u32 pdiv, u32
>> pll_con) +{
>> +     if ((mdiv != ((pll_con >> PLL35XX_MDIV_SHIFT) &
> PLL35XX_MDIV_MASK)) ||
>> +             (pdiv != ((pll_con >> PLL35XX_PDIV_SHIFT) &
> PLL35XX_PDIV_MASK)))
>> +             return 1;
>> +     else
>> +             return 0;
>
> Readability of this function could be improved by moving some code out of
> the if clause, like:
>
> static inline bool samsung_pll35xx_mp_change(u32 mdiv,
>                                                 u32 pdiv, u32 pll_con)
> {
>         u32 old_mdiv, old_pdiv;
>
>         old_mdiv = (pll_con >> PLL35XX_MDIV_SHIFT) & PLL35XX_MDIV_MASK;
>         old_pdiv = (pll_con >> PLL35XX_PDIV_SHIFT) & PLL35XX_PDIV_MASK;
>
>         return (mdiv != old_mdiv || pdiv != old_pdiv);
> }
>

Yes, it looks neater, I have modified it V4.

>> +}
>> +
>> +static int samsung_pll35xx_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 tmp;
>> +
>> +     /* 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_get_name(hw->clk));
>> +             return -EINVAL;
>> +     }
>> +
>> +     tmp = pll_readl(pll, PLL35XX_CON0_OFFSET);
>> +
>> +     if (!(samsung_pll35xx_mp_change(rate->mdiv, rate->pdiv, tmp))) {
>> +             /* If only s change, change just s value only*/
>> +             tmp &= ~(PLL35XX_SDIV_MASK << PLL35XX_SDIV_SHIFT);
>> +             tmp |= rate->sdiv << PLL35XX_SDIV_SHIFT;
>> +             pll_writel(pll, tmp, PLL35XX_CON0_OFFSET);
>
> To improve readability of this code, return 0 could be added here and
> following code could be moved out of the else clause.
>

Hmm... I can't see much difference but I have taken this also.

Thanks,
Yadwinder

> Best regards,
> Tomasz
>
>> +     } else {
>> +             /* Set PLL lock time. */
>> +             pll_writel(pll, rate->pdiv * PLL35XX_LOCK_FACTOR,
>> +                             PLL35XX_LOCK_OFFSET);
>> +
>> +             /* Change PLL PMS values */
>> +             tmp &= ~((PLL35XX_MDIV_MASK << PLL35XX_MDIV_SHIFT) |
>> +                             (PLL35XX_PDIV_MASK << PLL35XX_PDIV_SHIFT)
> |
>> +                             (PLL35XX_SDIV_MASK <<
> PLL35XX_SDIV_SHIFT));
>> +             tmp |= (rate->mdiv << PLL35XX_MDIV_SHIFT) |
>> +                             (rate->pdiv << PLL35XX_PDIV_SHIFT) |
>> +                             (rate->sdiv << PLL35XX_SDIV_SHIFT);
>> +             pll_writel(pll, tmp, PLL35XX_CON0_OFFSET);
>> +
>> +             /* wait_lock_time */
>> +             do {
>> +                     cpu_relax();
>> +                     tmp = pll_readl(pll, PLL35XX_CON0_OFFSET);
>> +             } while (!(tmp & (PLL35XX_LOCK_STAT_MASK
>> +                             << PLL35XX_LOCK_STAT_SHIFT)));
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>>  static const struct clk_ops samsung_pll35xx_clk_ops = {
>>       .recalc_rate = samsung_pll35xx_recalc_rate,
>> +     .round_rate = samsung_pll_round_rate,
>> +     .set_rate = samsung_pll35xx_set_rate,
>> +};
>> +
>> +static const struct clk_ops samsung_pll35xx_clk_min_ops = {
>> +     .recalc_rate = samsung_pll35xx_recalc_rate,
>>  };
>>
>>  struct clk * __init samsung_clk_register_pll35xx(const char *name,
>> @@ -79,11 +176,15 @@ struct clk * __init
>> samsung_clk_register_pll35xx(const char *name, }
>>
>>       init.name = name;
>> -     init.ops = &samsung_pll35xx_clk_ops;
>>       init.flags = CLK_GET_RATE_NOCACHE;
>>       init.parent_names = &pname;
>>       init.num_parents = 1;
>>
>> +     if (rate_table && rate_count)
>> +             init.ops = &samsung_pll35xx_clk_ops;
>> +     else
>> +             init.ops = &samsung_pll35xx_clk_min_ops;
>> +
>>       pll->hw.init = &init;
>>       pll->base = base;
>>       pll->rate_table = rate_table;
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2013-06-12 14:51 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-31 12:31 [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Vikas Sajjan
2013-05-31 12:31 ` [PATCH v3 1/6] clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx Vikas Sajjan
2013-06-08 12:04   ` Tomasz Figa
2013-05-31 12:31 ` [PATCH v3 2/6] clk: samsung: Add support to register rate_table " Vikas Sajjan
2013-05-31 17:08   ` Doug Anderson
2013-05-31 12:31 ` [PATCH v3 3/6] clk: samsung: Add set_rate() clk_ops for PLL35xx Vikas Sajjan
2013-05-31 17:10   ` Doug Anderson
2013-06-08 12:12   ` Tomasz Figa
2013-06-12 14:51     ` Yadwinder Singh Brar
2013-05-31 12:31 ` [PATCH v3 4/6] clk: samsung: Add set_rate() clk_ops for PLL36xx Vikas Sajjan
2013-05-31 17:15   ` Doug Anderson
2013-06-08 12:17   ` Tomasz Figa
2013-06-12  6:09     ` Vikas Sajjan
2013-05-31 12:31 ` [PATCH v3 5/6] clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration for it Vikas Sajjan
2013-05-31 17:20   ` Doug Anderson
2013-05-31 12:31 ` [PATCH v3 6/6] clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC Vikas Sajjan
2013-05-31 17:58   ` Doug Anderson
2013-06-03 21:06 ` [PATCH v3 0/6] Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs Doug Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).