Skip to contentSkip to conversation

Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs

PATCH New Vikas Sajjan 5 participants 18 messages May 31, 2013 – Jun 12, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v3 cover Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs

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(-)
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH v3 1/6 clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx

From: Yadwinder Singh Brar <[email protected]>

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

---
 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(-)
drivers/clk/samsung/clk-exynos4.c +6 -4
index d0940e6..cf7d4e7 100644
@@ -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);
drivers/clk/samsung/clk-exynos5250.c +7 -7
index 5c97e75..687b580 100644
@@ -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));
drivers/clk/samsung/clk-pll.c +29 -25

119 lines not on this page · raw message

drivers/clk/samsung/clk-pll.h +2 -2
index f33786e..1329522 100644
@@ -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);
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH v3 1/6 clk: samsung: Use clk->base instead of directly using clk->con0 for PLL3xxx

Hi Yadwinder,

Quoted 4 lines · On Friday 31 of May 2013 18:01:31 Vikas Sajjan wrote:

From: Yadwinder Singh Brar <[email protected]>

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

Quoted 32 lines
---
 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(-)
(quoted diff) +0 -0

b/drivers/clk/samsung/clk-exynos4.c index d0940e6..cf7d4e7 100644

drivers/clk/samsung/clk-exynos4.c +2 -0
@@ -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

drivers/clk/samsung/clk-exynos4.c void __init exynos4_clk_init(struct device_node · line 1023

Quoted 3 lines
drivers/clk/samsung/clk-exynos4.c +0 -0
@@ -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",

drivers/clk/samsung/clk-exynos4.c void __init exynos4_clk_init(struct device_node *np, enum exynos4_soc exynos4_so

Quoted 3 lines
drivers/clk/samsung/clk-exynos4.c +1 -1
-					reg_base + APLL_CON0);
+					reg_base + APLL_LOCK);
 		mpll = samsung_clk_register_pll35xx("fout_mpll", 

"fin_pll",

drivers/clk/samsung/clk-exynos4.c void __init exynos4_clk_init(struct device_node *np, enum exynos4_soc exynos4_so

Quoted 3 lines
drivers/clk/samsung/clk-exynos4.c +1 -1
-					reg_base + E4X12_MPLL_CON0);
+					reg_base + E4X12_MPLL_LOCK);
 		epll = samsung_clk_register_pll36xx("fout_epll", 

"fin_pll",

drivers/clk/samsung/clk-exynos4.c void __init exynos4_clk_init(struct device_node *np, enum exynos4_soc exynos4_so

Quoted 3 lines
drivers/clk/samsung/clk-exynos4.c +1 -1
-					reg_base + EPLL_CON0);
+					reg_base + EPLL_LOCK);
 		vpll = samsung_clk_register_pll36xx("fout_vpll", 

"fin_pll",

Quoted 42 lines
drivers/clk/samsung/clk-exynos4.c +1 -1
-					reg_base + VPLL_CON0);
+					reg_base + VPLL_LOCK);
 	}
 
 	samsung_clk_add_lookup(apll, fout_apll);
(quoted diff) +0 -0

b/drivers/clk/samsung/clk-exynos5250.c index 5c97e75..687b580 100644

drivers/clk/samsung/clk-exynos5250.c +7 -7
@@ -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));
(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.c index 89135f6..a7d8ad9 100644

drivers/clk/samsung/clk-pll.c +0 -0
@@ -13,9 +13,24 @@
 #include "clk.h"

drivers/clk/samsung/clk-pll.c line 23

Quoted 10 lines
drivers/clk/samsung/clk-pll.c +8 -0
 #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)						

\

Quoted 121 lines
drivers/clk/samsung/clk-pll.c +7 -6
+		__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,

drivers/clk/samsung/clk-pll.c +5 -5
 				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
*/

drivers/clk/samsung/clk-pll.c +3 -0
+#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)

drivers/clk/samsung/clk-pll.c +0 -6
-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,

drivers/clk/samsung/clk-pll.c +6 -6
 				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)) {
(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.h index f33786e..1329522 100644

drivers/clk/samsung/clk-pll.h enum pll46xx_type { · line 28

Quoted 4 lines
drivers/clk/samsung/clk-pll.h +0 -0
@@ -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);

drivers/clk/samsung/clk-pll.h enum pll46xx_type { · line 31

Quoted 3 lines
drivers/clk/samsung/clk-pll.h +1 -0
+			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);

Quoted 4 lines
drivers/clk/samsung/clk-pll.h +1 -0
+			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);

#3 lore raw

PATCH v3 2/6 clk: samsung: Add support to register rate_table for PLL3xxx

In reply to Vikas Sajjan

From: Yadwinder Singh Brar <[email protected]>

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.

---
 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(-)
drivers/clk/samsung/clk-exynos4.c +4 -4
index cf7d4e7..beff8a1 100644
@@ -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);
drivers/clk/samsung/clk-exynos5250.c +7 -7
index 687b580..ddf10ca 100644
@@ -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));
drivers/clk/samsung/clk-pll.c +12 -2
index a7d8ad9..8226528 100644
@@ -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)) {
drivers/clk/samsung/clk-pll.h +33 -2
index 1329522..4780e6c 100644
@@ -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);
Signature · 1 line
1.7.9.5
#4 lore raw

PATCH v3 2/6 clk: samsung: Add support to register rate_table for PLL3xxx

Vikas and Yadwinder,

Quoted 6 lines · Vikas Sajjan

From: Yadwinder Singh Brar <[email protected]>

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.

Quoted 7 lines
---
 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.

#5 lore raw

PATCH v3 3/6 clk: samsung: Add set_rate() clk_ops for PLL35xx

In reply to Vikas Sajjan

From: Yadwinder Singh Brar <[email protected]>

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

---
 drivers/clk/samsung/clk-pll.c |  103 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 102 insertions(+), 1 deletion(-)
drivers/clk/samsung/clk-pll.c +102 -1
index 8226528..9591560 100644
@@ -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;
Signature · 1 line
1.7.9.5
#6 lore raw

PATCH v3 3/6 clk: samsung: Add set_rate() clk_ops for PLL35xx

Vikas and Yadwinder,

Quoted 9 lines · Vikas Sajjan
drivers/clk/samsung/clk-pll.c +9 -0
+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++) {

drivers/clk/samsung/clk-pll.c struct samsung_clk_pll { · line 62

Quoted 10 lines
drivers/clk/samsung/clk-pll.c +7 -0
+               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

#7 lore raw

PATCH v3 3/6 clk: samsung: Add set_rate() clk_ops for PLL35xx

In reply to Vikas Sajjan
Quoted 16 lines · On Friday 31 of May 2013 18:01:33 Vikas Sajjan wrote:

From: Yadwinder Singh Brar <[email protected]>

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

---
 drivers/clk/samsung/clk-pll.c |  103

++++++++++++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.c index 8226528..9591560 100644

drivers/clk/samsung/clk-pll.c struct samsung_clk_pll { · line 31

Quoted 5 lines
drivers/clk/samsung/clk-pll.c +2 -0
@@ -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)

Quoted 57 lines
drivers/clk/samsung/clk-pll.c +28 -0
+{
+	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

*/

drivers/clk/samsung/clk-pll.c +5 -0
@@ -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) +{

drivers/clk/samsung/clk-pll.c

Quoted 1 line
drivers/clk/samsung/clk-pll.c +1 -0
+	if ((mdiv != ((pll_con >> PLL35XX_MDIV_SHIFT) & 

PLL35XX_MDIV_MASK)) ||

drivers/clk/samsung/clk-pll.c

Quoted 1 line
drivers/clk/samsung/clk-pll.c +1 -0
+		(pdiv != ((pll_con >> PLL35XX_PDIV_SHIFT) & 

PLL35XX_PDIV_MASK)))

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw, · line 103

Quoted 3 lines
drivers/clk/samsung/clk-pll.c +3 -0
+		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);
}

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,

Quoted 13 lines
drivers/clk/samsung/clk-pll.c +3 -0
+}
+
+static int samsung_pll35xx_set_rate(struct clk_hw *hw, unsigned long

drate, + unsigned long prate)

drivers/clk/samsung/clk-pll.c +9 -0
+{
+	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__,

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw, · line 127

Quoted 11 lines
drivers/clk/samsung/clk-pll.c +11 -0
+			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

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,

Quoted 8 lines
drivers/clk/samsung/clk-pll.c +8 -0
+	} 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) 

|

drivers/clk/samsung/clk-pll.c

Quoted 1 line
drivers/clk/samsung/clk-pll.c +1 -0
+				(PLL35XX_SDIV_MASK << 

PLL35XX_SDIV_SHIFT));

Quoted 44 lines
drivers/clk/samsung/clk-pll.c +27 -1
+		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;
#8 lore raw

PATCH v3 3/6 clk: samsung: Add set_rate() clk_ops for PLL35xx

Quoted 92 lines · Tomasz Figa
Quoted 16 lines · On Friday 31 of May 2013 18:01:33 Vikas Sajjan wrote:

From: Yadwinder Singh Brar <[email protected]>

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

---
 drivers/clk/samsung/clk-pll.c |  103

++++++++++++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.c index 8226528..9591560 100644

drivers/clk/samsung/clk-pll.c struct samsung_clk_pll { · line 31

Quoted 5 lines
drivers/clk/samsung/clk-pll.c +2 -0
@@ -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)

Quoted 55 lines
drivers/clk/samsung/clk-pll.c +28 -0
+{
+     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

*/

drivers/clk/samsung/clk-pll.c +5 -0
@@ -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;
}

Quoted 3 lines

+static inline bool samsung_pll35xx_mp_change(u32 mdiv, u32 pdiv, u32 pll_con) +{
+ if ((mdiv != ((pll_con >> PLL35XX_MDIV_SHIFT) &

PLL35XX_MDIV_MASK)) ||

Quoted 1 line

+ (pdiv != ((pll_con >> PLL35XX_PDIV_SHIFT) &

PLL35XX_PDIV_MASK)))

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw, · line 103

Quoted 3 lines
drivers/clk/samsung/clk-pll.c +3 -0
+             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,

Quoted 10 lines
                                                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.

Quoted 14 lines

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,

Quoted 13 lines
drivers/clk/samsung/clk-pll.c +3 -0
+}
+
+static int samsung_pll35xx_set_rate(struct clk_hw *hw, unsigned long

drate, + unsigned long prate)

drivers/clk/samsung/clk-pll.c +9 -0
+{
+     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__,

Quoted 15 lines

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw, · line 127

Quoted 11 lines
drivers/clk/samsung/clk-pll.c +11 -0
+                     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

Quoted 60 lines

Best regards,
Tomasz

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll35xx_recalc_rate(struct clk_hw *hw,

Quoted 8 lines
drivers/clk/samsung/clk-pll.c +8 -0
+     } 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)

|

Quoted 1 line

+ (PLL35XX_SDIV_MASK <<

PLL35XX_SDIV_SHIFT));

Quoted 44 lines
drivers/clk/samsung/clk-pll.c +27 -1
+             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;
#9 lore raw

PATCH v3 4/6 clk: samsung: Add set_rate() clk_ops for PLL36xx

In reply to Vikas Sajjan

This patch adds set_rate and round_rate clk_ops for PLL36xx

---
 drivers/clk/samsung/clk-pll.c |   59 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)
drivers/clk/samsung/clk-pll.c +59 -0
index 9591560..7143ed89 100644
@@ -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;
Signature · 1 line
1.7.9.5
#10 lore raw

PATCH v3 4/6 clk: samsung: Add set_rate() clk_ops for PLL36xx

Vikas and Yadwinder,

drivers/clk/samsung/clk-pll.c struct clk * __init samsung_clk_register_pll36xx(const char *name, · line 328

Quoted 13 lines · Vikas Sajjan

struct clk * __init samsung_clk_register_pll36xx(const char *name,

drivers/clk/samsung/clk-pll.c +5 -0
@@ -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

#11 lore raw

PATCH v3 4/6 clk: samsung: Add set_rate() clk_ops for PLL36xx

In reply to Vikas Sajjan
Quoted 41 lines · On Friday 31 of May 2013 18:01:34 Vikas Sajjan wrote:

This patch adds set_rate and round_rate clk_ops for PLL36xx

---
 drivers/clk/samsung/clk-pll.c |   59

+++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.c index 9591560..7143ed89 100644

drivers/clk/samsung/clk-pll.c +0 -0
@@ -210,6 +210,9 @@ struct clk * __init

samsung_clk_register_pll35xx(const char *name, #define
PLL36XX_CON0_OFFSET (0x100)
#define PLL36XX_CON1_OFFSET (0x104)

drivers/clk/samsung/clk-pll.c +3 -0
+/* 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)

drivers/clk/samsung/clk-pll.c +2 -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)

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw,

Quoted 8 lines
drivers/clk/samsung/clk-pll.c +8 -0
+{
+	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__,

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw, · line 262

Quoted 6 lines
drivers/clk/samsung/clk-pll.c +6 -0
+			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.

Quoted 51 lines
drivers/clk/samsung/clk-pll.c +33 -0
+
+	/* 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;

drivers/clk/samsung/clk-pll.c +5 -0
 	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;
#12 lore raw

PATCH v3 4/6 clk: samsung: Add set_rate() clk_ops for PLL36xx

Hi Tomasz,

Quoted 58 lines · Tomasz Figa
Quoted 41 lines · On Friday 31 of May 2013 18:01:34 Vikas Sajjan wrote:

This patch adds set_rate and round_rate clk_ops for PLL36xx

---
 drivers/clk/samsung/clk-pll.c |   59

+++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)

(quoted diff) +0 -0

b/drivers/clk/samsung/clk-pll.c index 9591560..7143ed89 100644

drivers/clk/samsung/clk-pll.c +0 -0
@@ -210,6 +210,9 @@ struct clk * __init

samsung_clk_register_pll35xx(const char *name, #define
PLL36XX_CON0_OFFSET (0x100)
#define PLL36XX_CON1_OFFSET (0x104)

drivers/clk/samsung/clk-pll.c +3 -0
+/* 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)

drivers/clk/samsung/clk-pll.c +2 -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)

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw,

Quoted 8 lines
drivers/clk/samsung/clk-pll.c +8 -0
+{
+     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__,

drivers/clk/samsung/clk-pll.c static unsigned long samsung_pll36xx_recalc_rate(struct clk_hw *hw, · line 262

Quoted 6 lines
drivers/clk/samsung/clk-pll.c +6 -0
+                     drate, __clk_get_name(hw->clk));
+             return -EINVAL;
+     }
+
+     pll_con0 = pll_readl(pll, PLL36XX_CON0_OFFSET);
+     pll_con1 = pll_readl(pll, PLL36XX_CON1_OFFSET);
Quoted 4 lines

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.

Quoted 4 lines

Otherwise looks fine.

Thanks.

Quoted 56 lines
Quoted 44 lines
drivers/clk/samsung/clk-pll.c +33 -0
+
+     /* 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;

drivers/clk/samsung/clk-pll.c struct clk * __init samsung_clk_register_pll36xx(const char *name, · line 328

Quoted 8 lines
drivers/clk/samsung/clk-pll.c +5 -0
+     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

#13 lore raw

PATCH v3 5/6 clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration for it

In reply to Vikas Sajjan

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.

---
 drivers/clk/samsung/clk-exynos5250.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/clk/samsung/clk-exynos5250.c +8 -1
index ddf10ca..b0e6680 100644
@@ -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",
Signature · 1 line
1.7.9.5
#14 lore raw

PATCH v3 5/6 clk: samsung: Add alias for mout_vpllsrc and reorder MUX registration for it

Vikas and Yadwinder,

Quoted 6 lines · Vikas Sajjan

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

#15 lore raw

PATCH v3 6/6 clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC

In reply to Vikas Sajjan

Adds the EPLL and VPLL freq table for exynos5250 SoC.

---
 drivers/clk/samsung/clk-exynos5250.c |   48 +++++++++++++++++++++++++++++++---
 drivers/clk/samsung/clk.h            |    2 ++
 2 files changed, 47 insertions(+), 3 deletions(-)
drivers/clk/samsung/clk-exynos5250.c +45 -3
index b0e6680..0566421 100644
@@ -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));
drivers/clk/samsung/clk.h +2 -0
index e4ad6ea..c997649 100644
@@ -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.
Signature · 1 line
1.7.9.5
#16 lore raw

PATCH v3 6/6 clk: samsung: Add EPLL and VPLL freq table for exynos5250 SoC

Vikas and Yadwinder,

Quoted 13 lines · Vikas Sajjan

Adds the EPLL and VPLL freq table for exynos5250 SoC.

---
 drivers/clk/samsung/clk-exynos5250.c |   48 +++++++++++++++++++++++++++++++---
 drivers/clk/samsung/clk.h            |    2 ++
 2 files changed, 47 insertions(+), 3 deletions(-)
drivers/clk/samsung/clk-exynos5250.c +0 -0
index b0e6680..0566421 100644

drivers/clk/samsung/clk-exynos5250.c static __initdata struct of_device_id ext_clk_match[] = { · line 480

Quoted 8 lines
drivers/clk/samsung/clk-exynos5250.c +5 -0
@@ -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.

drivers/clk/samsung/clk-exynos5250.c void __init exynos5250_clk_init(struct device_node *np) · line 522

Quoted 2 lines
drivers/clk/samsung/clk-exynos5250.c +2 -0
+       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.

Quoted 5 lines
drivers/clk/samsung/clk-exynos5250.c +0 -1
@@ -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",

drivers/clk/samsung/clk-exynos5250.c void __init exynos5250_clk_init(struct device_node *np) · line 541

Quoted 10 lines
drivers/clk/samsung/clk-exynos5250.c +8 -2
-                       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

#17 lore raw

v3 cover Add generic set_rate clk_ops for PLL35xx and PLL36xx for samsung SoCs

In reply to Vikas Sajjan

Vikas,

Quoted 42 lines · Vikas Sajjan

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

Quoted 6 lines
 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