[PATCH v3 00/11] clk: qcom: PLL updates

[PATCH v3 00/11] clk: qcom: PLL updates

[Rajendra Nayak]

The series is adding a few things in clk-alpha-pll code which
will be needed to model CPU PLLs and also needed by some MMCC
PLLs on msm8996

Changes in v3:
* Removed all the confusing additions to clk-pll code from v2
* Added patches showing users for the new additions to clk-alpha-pll in mmcc and cpu clocks
* RFC of skeletal cpu clock driver included

Rajendra Nayak (10):
  clk: qcom: Add support for alpha pll hwfsm ops
  clk: qcom: Add support to initialize alpha plls
  clk: qcom: handle alpha PLLs with 16bit alpha val registers
  clk: qcom: Enable FSM mode for votable alpha PLLs
  clk: qcom: Add .is_enabled ops for clk-alpha-pll
  clk: qcom: Fix .set_rate to handle alpha PLLs w/wo dynamic update
  clk: qcom: mmcc-8996: Add capability flags for some alpha PLLs
  clk: qcom: Add support for table based lookups in clk-regmap-mux
  clk: Add clk_hw_get_clk() helper API to be used by clk providers
  clk: qcom: Add basic CPU clock driver for msm8996

Taniya Das (1):
  clk: qcom: support dynamic update using latched interface

 drivers/clk/clk.c                 |   6 +
 drivers/clk/qcom/Makefile         |   1 +
 drivers/clk/qcom/clk-alpha-pll.c  | 272 ++++++++++++++++++++++---
 drivers/clk/qcom/clk-alpha-pll.h  |  31 +++
 drivers/clk/qcom/clk-cpu-8996.c   | 408 ++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-pll.c        |  31 +--
 drivers/clk/qcom/clk-regmap-mux.c |  13 ++
 drivers/clk/qcom/clk-regmap-mux.h |   1 +
 drivers/clk/qcom/common.c         |  21 ++
 drivers/clk/qcom/common.h         |   9 +
 drivers/clk/qcom/mmcc-msm8996.c   |   4 +
 include/linux/clk-provider.h      |   1 +
 12 files changed, 744 insertions(+), 54 deletions(-)
 create mode 100644 drivers/clk/qcom/clk-cpu-8996.c

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v3 01/11] clk: qcom: Add support for alpha pll hwfsm ops

[Rajendra Nayak]

Some PLLs can support an HW FSM mode (different from the Votable FSMs,
though its the same bit used to enable Votable FSMs as well as HW FSMs)
which enables the HW to do the bypass/reset/enable-output-ctrl sequence
on its own. So all thats needed from SW is to set the FSM_ENA bit.
PLL_ACTIVE_FLAG is whats used to check if the PLL is active/enabled.

Some of the PLLs which support HW FSM can also need an OFFLINE request
that needs to be toggled across the enable/disable. We use a flag to
identify such cases and handle them.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/clk/qcom/clk-alpha-pll.c | 102 ++++++++++++++++++++++++++++++++++-----
 drivers/clk/qcom/clk-alpha-pll.h |   3 ++
 2 files changed, 93 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index e6a03ea..0cfbb29 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -23,12 +23,15 @@
 # define PLL_OUTCTRL		BIT(0)
 # define PLL_BYPASSNL		BIT(1)
 # define PLL_RESET_N		BIT(2)
+# define PLL_OFFLINE_REQ	BIT(7)
 # define PLL_LOCK_COUNT_SHIFT	8
 # define PLL_LOCK_COUNT_MASK	0x3f
 # define PLL_BIAS_COUNT_SHIFT	14
 # define PLL_BIAS_COUNT_MASK	0x3f
 # define PLL_VOTE_FSM_ENA	BIT(20)
+# define PLL_FSM_ENA		BIT(20)
 # define PLL_VOTE_FSM_RESET	BIT(21)
+# define PLL_OFFLINE_ACK	BIT(28)
 # define PLL_ACTIVE_FLAG	BIT(30)
 # define PLL_LOCK_DET		BIT(31)
 
@@ -62,9 +65,10 @@
 #define to_clk_alpha_pll_postdiv(_hw) container_of(to_clk_regmap(_hw), \
 					   struct clk_alpha_pll_postdiv, clkr)
 
-static int wait_for_pll(struct clk_alpha_pll *pll)
+static int wait_for_pll(struct clk_alpha_pll *pll, u32 mask, bool inverse,
+			const char *action)
 {
-	u32 val, mask, off;
+	u32 val, off;
 	int count;
 	int ret;
 	const char *name = clk_hw_get_name(&pll->clkr.hw);
@@ -74,26 +78,91 @@ static int wait_for_pll(struct clk_alpha_pll *pll)
 	if (ret)
 		return ret;
 
-	if (val & PLL_VOTE_FSM_ENA)
-		mask = PLL_ACTIVE_FLAG;
-	else
-		mask = PLL_LOCK_DET;
-
-	/* Wait for pll to enable. */
 	for (count = 100; count > 0; count--) {
 		ret = regmap_read(pll->clkr.regmap, off + PLL_MODE, &val);
 		if (ret)
 			return ret;
-		if ((val & mask) == mask)
+		if (inverse && !(val & mask))
+			return 0;
+		else if ((val & mask) == mask)
 			return 0;
 
 		udelay(1);
 	}
 
-	WARN(1, "%s didn't enable after voting for it!\n", name);
+	WARN(1, "%s failed to %s!\n", name, action);
 	return -ETIMEDOUT;
 }
 
+#define wait_for_pll_enable_active(pll) \
+	wait_for_pll(pll, PLL_ACTIVE_FLAG, 0, "enable")
+
+#define wait_for_pll_enable_lock(pll) \
+	wait_for_pll(pll, PLL_LOCK_DET, 0, "enable")
+
+#define wait_for_pll_disable(pll) \
+	wait_for_pll(pll, PLL_ACTIVE_FLAG, 1, "disable")
+
+#define wait_for_pll_offline(pll) \
+	wait_for_pll(pll, PLL_OFFLINE_ACK, 0, "offline")
+
+static int clk_alpha_pll_hwfsm_enable(struct clk_hw *hw)
+{
+	int ret;
+	u32 val, off;
+	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
+
+	off = pll->offset;
+	ret = regmap_read(pll->clkr.regmap, off + PLL_MODE, &val);
+	if (ret)
+		return ret;
+
+	val |= PLL_FSM_ENA;
+
+	if (pll->flags & SUPPORTS_OFFLINE_REQ)
+		val &= ~PLL_OFFLINE_REQ;
+
+	ret = regmap_write(pll->clkr.regmap, off + PLL_MODE, val);
+	if (ret)
+		return ret;
+
+	/* Make sure enable request goes through before waiting for update */
+	mb();
+
+	return wait_for_pll_enable_active(pll);
+}
+
+static void clk_alpha_pll_hwfsm_disable(struct clk_hw *hw)
+{
+	int ret;
+	u32 val, off;
+	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
+
+	off = pll->offset;
+	ret = regmap_read(pll->clkr.regmap, off + PLL_MODE, &val);
+	if (ret)
+		return;
+
+	if (pll->flags & SUPPORTS_OFFLINE_REQ) {
+		ret = regmap_update_bits(pll->clkr.regmap, off + PLL_MODE,
+					 PLL_OFFLINE_REQ, PLL_OFFLINE_REQ);
+		if (ret)
+			return;
+
+		ret = wait_for_pll_offline(pll);
+		if (ret)
+			return;
+	}
+
+	/* Disable hwfsm */
+	ret = regmap_update_bits(pll->clkr.regmap, off + PLL_MODE,
+				 PLL_FSM_ENA, 0);
+	if (ret)
+		return;
+
+	wait_for_pll_disable(pll);
+}
+
 static int clk_alpha_pll_enable(struct clk_hw *hw)
 {
 	int ret;
@@ -112,7 +181,7 @@ static int clk_alpha_pll_enable(struct clk_hw *hw)
 		ret = clk_enable_regmap(hw);
 		if (ret)
 			return ret;
-		return wait_for_pll(pll);
+		return wait_for_pll_enable_active(pll);
 	}
 
 	/* Skip if already enabled */
@@ -136,7 +205,7 @@ static int clk_alpha_pll_enable(struct clk_hw *hw)
 	if (ret)
 		return ret;
 
-	ret = wait_for_pll(pll);
+	ret = wait_for_pll_enable_lock(pll);
 	if (ret)
 		return ret;
 
@@ -300,6 +369,15 @@ const struct clk_ops clk_alpha_pll_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_alpha_pll_ops);
 
+const struct clk_ops clk_alpha_pll_hwfsm_ops = {
+	.enable = clk_alpha_pll_hwfsm_enable,
+	.disable = clk_alpha_pll_hwfsm_disable,
+	.recalc_rate = clk_alpha_pll_recalc_rate,
+	.round_rate = clk_alpha_pll_round_rate,
+	.set_rate = clk_alpha_pll_set_rate,
+};
+EXPORT_SYMBOL_GPL(clk_alpha_pll_hwfsm_ops);
+
 static unsigned long
 clk_alpha_pll_postdiv_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 {
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index 90ce201..0deb286 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -34,6 +34,8 @@ struct clk_alpha_pll {
 
 	const struct pll_vco *vco_table;
 	size_t num_vco;
+#define SUPPORTS_OFFLINE_REQ	BIT(0)
+	u8 flags;
 
 	struct clk_regmap clkr;
 };
@@ -52,6 +54,7 @@ struct clk_alpha_pll_postdiv {
 };
 
 extern const struct clk_ops clk_alpha_pll_ops;
+extern const struct clk_ops clk_alpha_pll_hwfsm_ops;
 extern const struct clk_ops clk_alpha_pll_postdiv_ops;
 
 #endif
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v3 01/11] clk: qcom: Add support for alpha pll hwfsm ops

[Stephen Boyd]

On 09/29, Rajendra Nayak wrote:
> Some PLLs can support an HW FSM mode (different from the Votable FSMs,
> though its the same bit used to enable Votable FSMs as well as HW FSMs)
> which enables the HW to do the bypass/reset/enable-output-ctrl sequence
> on its own. So all thats needed from SW is to set the FSM_ENA bit.
> PLL_ACTIVE_FLAG is whats used to check if the PLL is active/enabled.
> 
> Some of the PLLs which support HW FSM can also need an OFFLINE request
> that needs to be toggled across the enable/disable. We use a flag to
> identify such cases and handle them.
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---

Applied to clk-next

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

[PATCH v3 02/11] clk: qcom: Add support to initialize alpha plls

[Rajendra Nayak]

Add a function to do initial configuration of the alpha plls

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/clk/qcom/clk-alpha-pll.c | 31 +++++++++++++++++++++++++++++++
 drivers/clk/qcom/clk-alpha-pll.h | 20 ++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index 0cfbb29..a1188c8 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -49,6 +49,7 @@
 #define PLL_USER_CTL_U		0x14
 
 #define PLL_CONFIG_CTL		0x18
+#define PLL_CONFIG_CTL_U	0x20
 #define PLL_TEST_CTL		0x1c
 #define PLL_TEST_CTL_U		0x20
 #define PLL_STATUS		0x24
@@ -106,6 +107,36 @@ static int wait_for_pll(struct clk_alpha_pll *pll, u32 mask, bool inverse,
 #define wait_for_pll_offline(pll) \
 	wait_for_pll(pll, PLL_OFFLINE_ACK, 0, "offline")
 
+void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
+			     const struct alpha_pll_config *config)
+{
+	u32 val, mask;
+	u32 off = pll->offset;
+
+	regmap_write(regmap, off + PLL_L_VAL, config->l);
+	regmap_write(regmap, off + PLL_ALPHA_VAL, config->alpha);
+	regmap_write(regmap, off + PLL_CONFIG_CTL, config->config_ctl_val);
+	regmap_write(regmap, off + PLL_CONFIG_CTL_U, config->config_ctl_hi_val);
+
+	val = config->main_output_mask;
+	val |= config->aux_output_mask;
+	val |= config->aux2_output_mask;
+	val |= config->early_output_mask;
+	val |= config->pre_div_val;
+	val |= config->post_div_val;
+	val |= config->vco_val;
+
+	mask = config->main_output_mask;
+	mask |= config->aux_output_mask;
+	mask |= config->aux2_output_mask;
+	mask |= config->early_output_mask;
+	mask |= config->pre_div_mask;
+	mask |= config->post_div_mask;
+	mask |= config->vco_mask;
+
+	regmap_update_bits(regmap, off + PLL_USER_CTL, mask, val);
+}
+
 static int clk_alpha_pll_hwfsm_enable(struct clk_hw *hw)
 {
 	int ret;
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index 0deb286..2f48530 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -53,8 +53,28 @@ struct clk_alpha_pll_postdiv {
 	struct clk_regmap clkr;
 };
 
+struct alpha_pll_config {
+	u32 l;
+	u32 alpha;
+	u32 config_ctl_val;
+	u32 config_ctl_hi_val;
+	u32 main_output_mask;
+	u32 aux_output_mask;
+	u32 aux2_output_mask;
+	u32 early_output_mask;
+	u32 pre_div_val;
+	u32 pre_div_mask;
+	u32 post_div_val;
+	u32 post_div_mask;
+	u32 vco_val;
+	u32 vco_mask;
+};
+
 extern const struct clk_ops clk_alpha_pll_ops;
 extern const struct clk_ops clk_alpha_pll_hwfsm_ops;
 extern const struct clk_ops clk_alpha_pll_postdiv_ops;
 
+void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
+			     const struct alpha_pll_config *config);
+
 #endif
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v3 02/11] clk: qcom: Add support to initialize alpha plls

[Stephen Boyd]

On 09/29, Rajendra Nayak wrote:
> Add a function to do initial configuration of the alpha plls
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---

Applied to clk-next

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

[PATCH v3 03/11] clk: qcom: handle alpha PLLs with 16bit alpha val registers

[Rajendra Nayak]

Some alpha PLLs have support for only a 16bit programable Alpha Value
(as against the default 40bits). Add a flag to handle the 16bit alpha
registers

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/clk/qcom/clk-alpha-pll.c | 24 +++++++++++++++++-------
 drivers/clk/qcom/clk-alpha-pll.h |  1 +
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index a1188c8..fd3e32c 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -59,6 +59,7 @@
  */
 #define ALPHA_REG_BITWIDTH	40
 #define ALPHA_BITWIDTH		32
+#define ALPHA_16BIT_MASK	0xffff
 
 #define to_clk_alpha_pll(_hw) container_of(to_clk_regmap(_hw), \
 					   struct clk_alpha_pll, clkr)
@@ -334,9 +335,14 @@ clk_alpha_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	regmap_read(pll->clkr.regmap, off + PLL_USER_CTL, &ctl);
 	if (ctl & PLL_ALPHA_EN) {
 		regmap_read(pll->clkr.regmap, off + PLL_ALPHA_VAL, &low);
-		regmap_read(pll->clkr.regmap, off + PLL_ALPHA_VAL_U, &high);
-		a = (u64)high << 32 | low;
-		a >>= ALPHA_REG_BITWIDTH - ALPHA_BITWIDTH;
+		if (pll->flags & SUPPORTS_16BIT_ALPHA) {
+			a = low & ALPHA_16BIT_MASK;
+		} else {
+			regmap_read(pll->clkr.regmap, off + PLL_ALPHA_VAL_U,
+				    &high);
+			a = (u64)high << 32 | low;
+			a >>= ALPHA_REG_BITWIDTH - ALPHA_BITWIDTH;
+		}
 	}
 
 	return alpha_pll_calc_rate(prate, l, a);
@@ -357,11 +363,15 @@ static int clk_alpha_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 		return -EINVAL;
 	}
 
-	a <<= (ALPHA_REG_BITWIDTH - ALPHA_BITWIDTH);
-
 	regmap_write(pll->clkr.regmap, off + PLL_L_VAL, l);
-	regmap_write(pll->clkr.regmap, off + PLL_ALPHA_VAL, a);
-	regmap_write(pll->clkr.regmap, off + PLL_ALPHA_VAL_U, a >> 32);
+
+	if (pll->flags & SUPPORTS_16BIT_ALPHA) {
+		regmap_write(pll->clkr.regmap, off + PLL_ALPHA_VAL,
+			     a & ALPHA_16BIT_MASK);
+	} else {
+		a <<= (ALPHA_REG_BITWIDTH - ALPHA_BITWIDTH);
+		regmap_write(pll->clkr.regmap, off + PLL_ALPHA_VAL_U, a >> 32);
+	}
 
 	regmap_update_bits(pll->clkr.regmap, off + PLL_USER_CTL,
 			   PLL_VCO_MASK << PLL_VCO_SHIFT,
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index 2f48530..4808ff7 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -35,6 +35,7 @@ struct clk_alpha_pll {
 	const struct pll_vco *vco_table;
 	size_t num_vco;
 #define SUPPORTS_OFFLINE_REQ	BIT(0)
+#define SUPPORTS_16BIT_ALPHA	BIT(1)
 	u8 flags;
 
 	struct clk_regmap clkr;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v3 03/11] clk: qcom: handle alpha PLLs with 16bit alpha val registers

[Stephen Boyd]

On 09/29, Rajendra Nayak wrote:
> Some alpha PLLs have support for only a 16bit programable Alpha Value
> (as against the default 40bits). Add a flag to handle the 16bit alpha
> registers
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---

Applied to clk-next

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

[PATCH v3 04/11] clk: qcom: Enable FSM mode for votable alpha PLLs

[Rajendra Nayak]

The votable alpha PLLs need to have the fsm mode enabled as part
of the initialization. The sequence seems to be the same as used
by clk-pll, so move the function which does this into a common
place and reuse it for the clk-alpha-pll

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 drivers/clk/qcom/clk-alpha-pll.c |  4 ++++
 drivers/clk/qcom/clk-alpha-pll.h |  1 +
 drivers/clk/qcom/clk-pll.c       | 31 +++----------------------------
 drivers/clk/qcom/common.c        | 21 +++++++++++++++++++++
 drivers/clk/qcom/common.h        |  9 +++++++++
 5 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index fd3e32c..c6ddc04e 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -18,6 +18,7 @@
 #include <linux/delay.h>
 
 #include "clk-alpha-pll.h"
+#include "common.h"
 
 #define PLL_MODE		0x00
 # define PLL_OUTCTRL		BIT(0)
@@ -136,6 +137,9 @@ void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 	mask |= config->vco_mask;
 
 	regmap_update_bits(regmap, off + PLL_USER_CTL, mask, val);
+
+	if (pll->flags & SUPPORTS_FSM_MODE)
+		qcom_pll_set_fsm_mode(regmap, off + PLL_MODE, 6, 0);
 }
 
 static int clk_alpha_pll_hwfsm_enable(struct clk_hw *hw)
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index 4808ff7..d6e1ee2 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -36,6 +36,7 @@ struct clk_alpha_pll {
 	size_t num_vco;
 #define SUPPORTS_OFFLINE_REQ	BIT(0)
 #define SUPPORTS_16BIT_ALPHA	BIT(1)
+#define SUPPORTS_FSM_MODE	BIT(2)
 	u8 flags;
 
 	struct clk_regmap clkr;
diff --git a/drivers/clk/qcom/clk-pll.c b/drivers/clk/qcom/clk-pll.c
index 5b940d6..cb6cb87 100644
--- a/drivers/clk/qcom/clk-pll.c
+++ b/drivers/clk/qcom/clk-pll.c
@@ -23,16 +23,11 @@
 #include <asm/div64.h>
 
 #include "clk-pll.h"
+#include "common.h"
 
 #define PLL_OUTCTRL		BIT(0)
 #define PLL_BYPASSNL		BIT(1)
 #define PLL_RESET_N		BIT(2)
-#define PLL_LOCK_COUNT_SHIFT	8
-#define PLL_LOCK_COUNT_MASK	0x3f
-#define PLL_BIAS_COUNT_SHIFT	14
-#define PLL_BIAS_COUNT_MASK	0x3f
-#define PLL_VOTE_FSM_ENA	BIT(20)
-#define PLL_VOTE_FSM_RESET	BIT(21)
 
 static int clk_pll_enable(struct clk_hw *hw)
 {
@@ -228,26 +223,6 @@ const struct clk_ops clk_pll_vote_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_pll_vote_ops);
 
-static void
-clk_pll_set_fsm_mode(struct clk_pll *pll, struct regmap *regmap, u8 lock_count)
-{
-	u32 val;
-	u32 mask;
-
-	/* De-assert reset to FSM */
-	regmap_update_bits(regmap, pll->mode_reg, PLL_VOTE_FSM_RESET, 0);
-
-	/* Program bias count and lock count */
-	val = 1 << PLL_BIAS_COUNT_SHIFT | lock_count << PLL_LOCK_COUNT_SHIFT;
-	mask = PLL_BIAS_COUNT_MASK << PLL_BIAS_COUNT_SHIFT;
-	mask |= PLL_LOCK_COUNT_MASK << PLL_LOCK_COUNT_SHIFT;
-	regmap_update_bits(regmap, pll->mode_reg, mask, val);
-
-	/* Enable PLL FSM voting */
-	regmap_update_bits(regmap, pll->mode_reg, PLL_VOTE_FSM_ENA,
-		PLL_VOTE_FSM_ENA);
-}
-
 static void clk_pll_configure(struct clk_pll *pll, struct regmap *regmap,
 	const struct pll_config *config)
 {
@@ -280,7 +255,7 @@ void clk_pll_configure_sr(struct clk_pll *pll, struct regmap *regmap,
 {
 	clk_pll_configure(pll, regmap, config);
 	if (fsm_mode)
-		clk_pll_set_fsm_mode(pll, regmap, 8);
+		qcom_pll_set_fsm_mode(regmap, pll->mode_reg, 1, 8);
 }
 EXPORT_SYMBOL_GPL(clk_pll_configure_sr);
 
@@ -289,7 +264,7 @@ void clk_pll_configure_sr_hpm_lp(struct clk_pll *pll, struct regmap *regmap,
 {
 	clk_pll_configure(pll, regmap, config);
 	if (fsm_mode)
-		clk_pll_set_fsm_mode(pll, regmap, 0);
+		qcom_pll_set_fsm_mode(regmap, pll->mode_reg, 1, 0);
 }
 EXPORT_SYMBOL_GPL(clk_pll_configure_sr_hpm_lp);
 
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index f7c226a..bba225e 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -74,6 +74,27 @@ qcom_cc_map(struct platform_device *pdev, const struct qcom_cc_desc *desc)
 }
 EXPORT_SYMBOL_GPL(qcom_cc_map);
 
+void
+qcom_pll_set_fsm_mode(struct regmap *map, u32 reg, u8 bias_count, u8 lock_count)
+{
+	u32 val;
+	u32 mask;
+
+	/* De-assert reset to FSM */
+	regmap_update_bits(map, reg, PLL_VOTE_FSM_RESET, 0);
+
+	/* Program bias count and lock count */
+	val = bias_count << PLL_BIAS_COUNT_SHIFT |
+		lock_count << PLL_LOCK_COUNT_SHIFT;
+	mask = PLL_BIAS_COUNT_MASK << PLL_BIAS_COUNT_SHIFT;
+	mask |= PLL_LOCK_COUNT_MASK << PLL_LOCK_COUNT_SHIFT;
+	regmap_update_bits(map, reg, mask, val);
+
+	/* Enable PLL FSM voting */
+	regmap_update_bits(map, reg, PLL_VOTE_FSM_ENA, PLL_VOTE_FSM_ENA);
+}
+EXPORT_SYMBOL_GPL(qcom_pll_set_fsm_mode);
+
 static void qcom_cc_del_clk_provider(void *data)
 {
 	of_clk_del_provider(data);
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index ae9bdeb..9fb5b8e 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -22,6 +22,13 @@ struct freq_tbl;
 struct clk_hw;
 struct parent_map;
 
+#define PLL_LOCK_COUNT_SHIFT	8
+#define PLL_LOCK_COUNT_MASK	0x3f
+#define PLL_BIAS_COUNT_SHIFT	14
+#define PLL_BIAS_COUNT_MASK	0x3f
+#define PLL_VOTE_FSM_ENA	BIT(20)
+#define PLL_VOTE_FSM_RESET	BIT(21)
+
 struct qcom_cc_desc {
 	const struct regmap_config *config;
 	struct clk_regmap **clks;
@@ -34,6 +41,8 @@ struct qcom_cc_desc {
 
 extern const struct freq_tbl *qcom_find_freq(const struct freq_tbl *f,
 					     unsigned long rate);
+extern void
+qcom_pll_set_fsm_mode(struct regmap *m, u32 reg, u8 bias_count, u8 lock_count);
 extern int qcom_find_src_index(struct clk_hw *hw, const struct parent_map *map,
 			       u8 src);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v3 04/11] clk: qcom: Enable FSM mode for votable alpha PLLs

[Stephen Boyd]

On 09/29, Rajendra Nayak wrote:
> The votable alpha PLLs need to have the fsm mode enabled as part
> of the initialization. The sequence seems to be the same as used
> by clk-pll, so move the function which does this into a common
> place and reuse it for the clk-alpha-pll
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> ---

Applied to clk-next

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

[PATCH v3 05/11] clk: qcom: Add .is_enabled ops for clk-alpha-pll

[Rajendra Nayak]

This would be useful in subsequent patches when the .set_rate operation
would need to identify if the PLL is actually enabled

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/clk/qcom/clk-alpha-pll.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index c6ddc04e..89c7fdb 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -199,6 +199,33 @@ static void clk_alpha_pll_hwfsm_disable(struct clk_hw *hw)
 	wait_for_pll_disable(pll);
 }
 
+static int pll_is_enabled(struct clk_hw *hw, u32 mask)
+{
+	int ret;
+	u32 val, off;
+	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
+
+	off = pll->offset;
+	ret = regmap_read(pll->clkr.regmap, off + PLL_MODE, &val);
+	if (ret)
+		return ret;
+
+	if (val & mask)
+		return 1;
+	else
+		return 0;
+}
+
+static int clk_alpha_pll_hwfsm_is_enabled(struct clk_hw *hw)
+{
+	return pll_is_enabled(hw, PLL_ACTIVE_FLAG);
+}
+
+static int clk_alpha_pll_is_enabled(struct clk_hw *hw)
+{
+	return pll_is_enabled(hw, PLL_LOCK_DET);
+}
+
 static int clk_alpha_pll_enable(struct clk_hw *hw)
 {
 	int ret;
@@ -408,6 +435,7 @@ static long clk_alpha_pll_round_rate(struct clk_hw *hw, unsigned long rate,
 const struct clk_ops clk_alpha_pll_ops = {
 	.enable = clk_alpha_pll_enable,
 	.disable = clk_alpha_pll_disable,
+	.is_enabled = clk_alpha_pll_is_enabled,
 	.recalc_rate = clk_alpha_pll_recalc_rate,
 	.round_rate = clk_alpha_pll_round_rate,
 	.set_rate = clk_alpha_pll_set_rate,
@@ -417,6 +445,7 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_ops);
 const struct clk_ops clk_alpha_pll_hwfsm_ops = {
 	.enable = clk_alpha_pll_hwfsm_enable,
 	.disable = clk_alpha_pll_hwfsm_disable,
+	.is_enabled = clk_alpha_pll_hwfsm_is_enabled,
 	.recalc_rate = clk_alpha_pll_recalc_rate,
 	.round_rate = clk_alpha_pll_round_rate,
 	.set_rate = clk_alpha_pll_set_rate,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v3 05/11] clk: qcom: Add .is_enabled ops for clk-alpha-pll

[Stephen Boyd]

On 09/29, Rajendra Nayak wrote:
> This would be useful in subsequent patches when the .set_rate operation
> would need to identify if the PLL is actually enabled
> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---

Applied to clk-next

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

[PATCH v3 06/11] clk: qcom: Fix .set_rate to handle alpha PLLs w/wo dynamic update

[Rajendra Nayak]

Alpha PLLs which do not support dynamic update feature
need to be explicitly disabled before a rate change.
The ones which do support dynamic update do so within a
single vco range, so add a min/max freq check for such
PLLs so they fall in the vco range.

Signed-off-by: Taniya Das <tdas@codeaurora.org>
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/clk/qcom/clk-alpha-pll.c | 49 +++++++++++++++++++++++++++++++++-------
 drivers/clk/qcom/clk-alpha-pll.h |  3 +++
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index 89c7fdb..6f90a86 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -382,16 +382,41 @@ clk_alpha_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 static int clk_alpha_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 				  unsigned long prate)
 {
+	bool enabled;
 	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
 	const struct pll_vco *vco;
 	u32 l, off = pll->offset;
 	u64 a;
 
 	rate = alpha_pll_round_rate(rate, prate, &l, &a);
-	vco = alpha_pll_find_vco(pll, rate);
-	if (!vco) {
-		pr_err("alpha pll not in a valid vco range\n");
-		return -EINVAL;
+	enabled = clk_hw_is_enabled(hw);
+
+	if (pll->flags & SUPPORTS_DYNAMIC_UPDATE) {
+		/*
+		 * PLLs which support dynamic updates support one single
+		 * vco range, between min_rate and max_rate supported
+		 */
+		if (rate < pll->min_rate || rate > pll->max_rate) {
+			pr_err("alpha pll rate outside supported min/max range\n");
+			return -EINVAL;
+		}
+	} else {
+		/*
+		 * All alpha PLLs which do not support dynamic update,
+		 * should be disabled before a vco update.
+		 */
+		if (enabled)
+			hw->init->ops->disable(hw);
+
+		vco = alpha_pll_find_vco(pll, rate);
+		if (!vco) {
+			pr_err("alpha pll not in a valid vco range\n");
+			return -EINVAL;
+		}
+
+		regmap_update_bits(pll->clkr.regmap, off + PLL_USER_CTL,
+				   PLL_VCO_MASK << PLL_VCO_SHIFT,
+				   vco->val << PLL_VCO_SHIFT);
 	}
 
 	regmap_write(pll->clkr.regmap, off + PLL_L_VAL, l);
@@ -404,13 +429,12 @@ static int clk_alpha_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 		regmap_write(pll->clkr.regmap, off + PLL_ALPHA_VAL_U, a >> 32);
 	}
 
-	regmap_update_bits(pll->clkr.regmap, off + PLL_USER_CTL,
-			   PLL_VCO_MASK << PLL_VCO_SHIFT,
-			   vco->val << PLL_VCO_SHIFT);
-
 	regmap_update_bits(pll->clkr.regmap, off + PLL_USER_CTL, PLL_ALPHA_EN,
 			   PLL_ALPHA_EN);
 
+	if (!(pll->flags & SUPPORTS_DYNAMIC_UPDATE) && enabled)
+		hw->init->ops->enable(hw);
+
 	return 0;
 }
 
@@ -423,6 +447,15 @@ static long clk_alpha_pll_round_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long min_freq, max_freq;
 
 	rate = alpha_pll_round_rate(rate, *prate, &l, &a);
+
+	if (pll->flags & SUPPORTS_DYNAMIC_UPDATE) {
+		if (rate < pll->min_rate)
+			rate = pll->min_rate;
+		else if (rate > pll->max_rate)
+			rate = pll->max_rate;
+		return rate;
+	}
+
 	if (alpha_pll_find_vco(pll, rate))
 		return rate;
 
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index d6e1ee2..e43a9c0 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -37,8 +37,11 @@ struct clk_alpha_pll {
 #define SUPPORTS_OFFLINE_REQ	BIT(0)
 #define SUPPORTS_16BIT_ALPHA	BIT(1)
 #define SUPPORTS_FSM_MODE	BIT(2)
+#define SUPPORTS_DYNAMIC_UPDATE	BIT(3)
 	u8 flags;
 
+	unsigned long min_rate;
+	unsigned long max_rate;
 	struct clk_regmap clkr;
 };
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v3 06/11] clk: qcom: Fix .set_rate to handle alpha PLLs w/wo dynamic update

[Stephen Boyd]

On 09/29, Rajendra Nayak wrote:
> Alpha PLLs which do not support dynamic update feature
> need to be explicitly disabled before a rate change.
> The ones which do support dynamic update do so within a
> single vco range, so add a min/max freq check for such
> PLLs so they fall in the vco range.
> 
> Signed-off-by: Taniya Das <tdas@codeaurora.org>

Is Taniya the author?

> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/clk/qcom/clk-alpha-pll.c | 49 +++++++++++++++++++++++++++++++++-------
>  drivers/clk/qcom/clk-alpha-pll.h |  3 +++
>  2 files changed, 44 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
> index 89c7fdb..6f90a86 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.c
> +++ b/drivers/clk/qcom/clk-alpha-pll.c
> @@ -382,16 +382,41 @@ clk_alpha_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>  static int clk_alpha_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>  				  unsigned long prate)
>  {
> +	bool enabled;
>  	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
>  	const struct pll_vco *vco;
>  	u32 l, off = pll->offset;
>  	u64 a;
>  
>  	rate = alpha_pll_round_rate(rate, prate, &l, &a);
> -	vco = alpha_pll_find_vco(pll, rate);
> -	if (!vco) {
> -		pr_err("alpha pll not in a valid vco range\n");
> -		return -EINVAL;
> +	enabled = clk_hw_is_enabled(hw);
> +
> +	if (pll->flags & SUPPORTS_DYNAMIC_UPDATE) {
> +		/*
> +		 * PLLs which support dynamic updates support one single
> +		 * vco range, between min_rate and max_rate supported
> +		 */
> +		if (rate < pll->min_rate || rate > pll->max_rate) {
> +			pr_err("alpha pll rate outside supported min/max range\n");
> +			return -EINVAL;
> +		}
> +	} else {
> +		/*
> +		 * All alpha PLLs which do not support dynamic update,
> +		 * should be disabled before a vco update.
> +		 */
> +		if (enabled)
> +			hw->init->ops->disable(hw);

Please just call the function directly instead of going through
the init structure.

> +
> +		vco = alpha_pll_find_vco(pll, rate);
> +		if (!vco) {
> +			pr_err("alpha pll not in a valid vco range\n");
> +			return -EINVAL;
> +		}
> +
> +		regmap_update_bits(pll->clkr.regmap, off + PLL_USER_CTL,
> +				   PLL_VCO_MASK << PLL_VCO_SHIFT,
> +				   vco->val << PLL_VCO_SHIFT);
>  	}
>  
>  	regmap_write(pll->clkr.regmap, off + PLL_L_VAL, l);
> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
> index d6e1ee2..e43a9c0 100644
> --- a/drivers/clk/qcom/clk-alpha-pll.h
> +++ b/drivers/clk/qcom/clk-alpha-pll.h
> @@ -37,8 +37,11 @@ struct clk_alpha_pll {
>  #define SUPPORTS_OFFLINE_REQ	BIT(0)
>  #define SUPPORTS_16BIT_ALPHA	BIT(1)
>  #define SUPPORTS_FSM_MODE	BIT(2)
> +#define SUPPORTS_DYNAMIC_UPDATE	BIT(3)
>  	u8 flags;
>  
> +	unsigned long min_rate;
> +	unsigned long max_rate;

Document these?

>  	struct clk_regmap clkr;
>  };
>  

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

Re: [PATCH v3 06/11] clk: qcom: Fix .set_rate to handle alpha PLLs w/wo dynamic update

[Rajendra Nayak]

On 11/03/2016 03:24 AM, Stephen Boyd wrote:
> On 09/29, Rajendra Nayak wrote:
>> Alpha PLLs which do not support dynamic update feature
>> need to be explicitly disabled before a rate change.
>> The ones which do support dynamic update do so within a
>> single vco range, so add a min/max freq check for such
>> PLLs so they fall in the vco range.
>>
>> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> 
> Is Taniya the author?

ah, yes, I seem to have messed up the authorship in v3, will fix.

> 
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>>  drivers/clk/qcom/clk-alpha-pll.c | 49 +++++++++++++++++++++++++++++++++-------
>>  drivers/clk/qcom/clk-alpha-pll.h |  3 +++
>>  2 files changed, 44 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
>> index 89c7fdb..6f90a86 100644
>> --- a/drivers/clk/qcom/clk-alpha-pll.c
>> +++ b/drivers/clk/qcom/clk-alpha-pll.c
>> @@ -382,16 +382,41 @@ clk_alpha_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>>  static int clk_alpha_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>>  				  unsigned long prate)
>>  {
>> +	bool enabled;
>>  	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
>>  	const struct pll_vco *vco;
>>  	u32 l, off = pll->offset;
>>  	u64 a;
>>  
>>  	rate = alpha_pll_round_rate(rate, prate, &l, &a);
>> -	vco = alpha_pll_find_vco(pll, rate);
>> -	if (!vco) {
>> -		pr_err("alpha pll not in a valid vco range\n");
>> -		return -EINVAL;
>> +	enabled = clk_hw_is_enabled(hw);
>> +
>> +	if (pll->flags & SUPPORTS_DYNAMIC_UPDATE) {
>> +		/*
>> +		 * PLLs which support dynamic updates support one single
>> +		 * vco range, between min_rate and max_rate supported
>> +		 */
>> +		if (rate < pll->min_rate || rate > pll->max_rate) {
>> +			pr_err("alpha pll rate outside supported min/max range\n");
>> +			return -EINVAL;
>> +		}
>> +	} else {
>> +		/*
>> +		 * All alpha PLLs which do not support dynamic update,
>> +		 * should be disabled before a vco update.
>> +		 */
>> +		if (enabled)
>> +			hw->init->ops->disable(hw);
> 
> Please just call the function directly instead of going through
> the init structure.

But we now have 2 different versions of disable based on clk_ops,
clk_alpha_pll_disable and clk_alpha_pll_hwfsm_disable.

> 
>> +
>> +		vco = alpha_pll_find_vco(pll, rate);
>> +		if (!vco) {
>> +			pr_err("alpha pll not in a valid vco range\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		regmap_update_bits(pll->clkr.regmap, off + PLL_USER_CTL,
>> +				   PLL_VCO_MASK << PLL_VCO_SHIFT,
>> +				   vco->val << PLL_VCO_SHIFT);
>>  	}
>>  
>>  	regmap_write(pll->clkr.regmap, off + PLL_L_VAL, l);
>> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
>> index d6e1ee2..e43a9c0 100644
>> --- a/drivers/clk/qcom/clk-alpha-pll.h
>> +++ b/drivers/clk/qcom/clk-alpha-pll.h
>> @@ -37,8 +37,11 @@ struct clk_alpha_pll {
>>  #define SUPPORTS_OFFLINE_REQ	BIT(0)
>>  #define SUPPORTS_16BIT_ALPHA	BIT(1)
>>  #define SUPPORTS_FSM_MODE	BIT(2)
>> +#define SUPPORTS_DYNAMIC_UPDATE	BIT(3)
>>  	u8 flags;
>>  
>> +	unsigned long min_rate;
>> +	unsigned long max_rate;
> 
> Document these?

will do, thanks.

> 
>>  	struct clk_regmap clkr;
>>  };
>>  
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v3 06/11] clk: qcom: Fix .set_rate to handle alpha PLLs w/wo dynamic update

[Stephen Boyd]

On 11/03, Rajendra Nayak wrote:
> 
> 
> On 11/03/2016 03:24 AM, Stephen Boyd wrote:
> > On 09/29, Rajendra Nayak wrote:
> >> Alpha PLLs which do not support dynamic update feature
> >> need to be explicitly disabled before a rate change.
> >> The ones which do support dynamic update do so within a
> >> single vco range, so add a min/max freq check for such
> >> PLLs so they fall in the vco range.
> >>
> >> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> > 
> > Is Taniya the author?
> 
> ah, yes, I seem to have messed up the authorship in v3, will fix.
> 
> > 
> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> >> ---
> >>  drivers/clk/qcom/clk-alpha-pll.c | 49 +++++++++++++++++++++++++++++++++-------
> >>  drivers/clk/qcom/clk-alpha-pll.h |  3 +++
> >>  2 files changed, 44 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
> >> index 89c7fdb..6f90a86 100644
> >> --- a/drivers/clk/qcom/clk-alpha-pll.c
> >> +++ b/drivers/clk/qcom/clk-alpha-pll.c
> >> @@ -382,16 +382,41 @@ clk_alpha_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> >>  static int clk_alpha_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> >>  				  unsigned long prate)
> >>  {
> >> +	bool enabled;
> >>  	struct clk_alpha_pll *pll = to_clk_alpha_pll(hw);
> >>  	const struct pll_vco *vco;
> >>  	u32 l, off = pll->offset;
> >>  	u64 a;
> >>  
> >>  	rate = alpha_pll_round_rate(rate, prate, &l, &a);
> >> -	vco = alpha_pll_find_vco(pll, rate);
> >> -	if (!vco) {
> >> -		pr_err("alpha pll not in a valid vco range\n");
> >> -		return -EINVAL;
> >> +	enabled = clk_hw_is_enabled(hw);
> >> +
> >> +	if (pll->flags & SUPPORTS_DYNAMIC_UPDATE) {
> >> +		/*
> >> +		 * PLLs which support dynamic updates support one single
> >> +		 * vco range, between min_rate and max_rate supported
> >> +		 */
> >> +		if (rate < pll->min_rate || rate > pll->max_rate) {
> >> +			pr_err("alpha pll rate outside supported min/max range\n");
> >> +			return -EINVAL;
> >> +		}
> >> +	} else {
> >> +		/*
> >> +		 * All alpha PLLs which do not support dynamic update,
> >> +		 * should be disabled before a vco update.
> >> +		 */
> >> +		if (enabled)
> >> +			hw->init->ops->disable(hw);
> > 
> > Please just call the function directly instead of going through
> > the init structure.
> 
> But we now have 2 different versions of disable based on clk_ops,
> clk_alpha_pll_disable and clk_alpha_pll_hwfsm_disable.
> 

So have two set_rate ops that call a common function that takes a
function pointer pair of enable/disable ops to call? Or have a
flag to know which one to call? Either way, don't use the init
structure after registration time please.

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

[PATCH v3 07/11] clk: qcom: support dynamic update using latched interface

[Rajendra Nayak]

From: Taniya Das <tdas@codeaurora.org>

Alpha PLLs can support 2 kinds of input signals, normal and latched. The
normal input is directly passed to the core, while the latched input
requires a latch and acknowledge sequence to be performed for the
changed input to propagate.

Alpha PLLs can support dynamic update with both kind of input signals.
The ones which support this using a latched interface however need to
follow the latch/wait-for-ack sequence to be performed when the rate changes.
Mark these with a new flag 'SUPPORTS_LATCHED_INPUT' to handle this as
part of clk_alpha_pll_set_rate()

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 drivers/clk/qcom/clk-alpha-pll.c | 37 ++++++++++++++++++++++++++++++++++++-
 drivers/clk/qcom/clk-alpha-pll.h |  3 +++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index 6f90a86..08fff75 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -32,6 +32,7 @@
 # define PLL_VOTE_FSM_ENA	BIT(20)
 # define PLL_FSM_ENA		BIT(20)
 # define PLL_VOTE_FSM_RESET	BIT(21)
+# define PLL_UPDATE		BIT(22)
 # define PLL_OFFLINE_ACK	BIT(28)
 # define PLL_ACTIVE_FLAG	BIT(30)
 # define PLL_LOCK_DET		BIT(31)
@@ -48,6 +49,7 @@
 # define PLL_VCO_MASK		0x3
 
 #define PLL_USER_CTL_U		0x14
+# define PLL_LATCH_INTERFACE	BIT(11)
 
 #define PLL_CONFIG_CTL		0x18
 #define PLL_CONFIG_CTL_U	0x20
@@ -109,6 +111,10 @@ static int wait_for_pll(struct clk_alpha_pll *pll, u32 mask, bool inverse,
 #define wait_for_pll_offline(pll) \
 	wait_for_pll(pll, PLL_OFFLINE_ACK, 0, "offline")
 
+#define wait_for_pll_latch_ack(pll) \
+	wait_for_pll(pll, BIT(pll->latch_ack_bit), pll->latch_ack_inverse, \
+		     "latch ack")
+
 void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 			     const struct alpha_pll_config *config)
 {
@@ -140,6 +146,10 @@ void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 
 	if (pll->flags & SUPPORTS_FSM_MODE)
 		qcom_pll_set_fsm_mode(regmap, off + PLL_MODE, 6, 0);
+
+	if (pll->flags & SUPPORTS_LATCHED_INPUT)
+		regmap_update_bits(regmap, off + PLL_USER_CTL_U,
+				   PLL_LATCH_INTERFACE, 0);
 }
 
 static int clk_alpha_pll_hwfsm_enable(struct clk_hw *hw)
@@ -379,6 +389,27 @@ clk_alpha_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	return alpha_pll_calc_rate(prate, l, a);
 }
 
+static int clk_alpha_pll_update_latch(struct clk_alpha_pll *pll)
+{
+	/* Latch the input to the PLL */
+	regmap_update_bits(pll->clkr.regmap, pll->offset + PLL_MODE,
+			   PLL_UPDATE, PLL_UPDATE);
+
+	/* Wait for 2 reference cycle before checking ACK bit */
+	udelay(1);
+
+	wait_for_pll_latch_ack(pll);
+
+	/* Return latch input to 0 */
+	regmap_update_bits(pll->clkr.regmap, pll->offset + PLL_MODE,
+			   PLL_UPDATE, 0);
+
+	/* Wait for PLL output to stabilize */
+	udelay(100);
+
+	return 0;
+}
+
 static int clk_alpha_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 				  unsigned long prate)
 {
@@ -432,8 +463,12 @@ static int clk_alpha_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 	regmap_update_bits(pll->clkr.regmap, off + PLL_USER_CTL, PLL_ALPHA_EN,
 			   PLL_ALPHA_EN);
 
-	if (!(pll->flags & SUPPORTS_DYNAMIC_UPDATE) && enabled)
+	if (pll->flags & SUPPORTS_DYNAMIC_UPDATE) {
+		if (pll->flags & SUPPORTS_LATCHED_INPUT)
+			clk_alpha_pll_update_latch(pll);
+	} else if (enabled) {
 		hw->init->ops->enable(hw);
+	}
 
 	return 0;
 }
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index e43a9c0..dab2ef3 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -38,7 +38,10 @@ struct clk_alpha_pll {
 #define SUPPORTS_16BIT_ALPHA	BIT(1)
 #define SUPPORTS_FSM_MODE	BIT(2)
 #define SUPPORTS_DYNAMIC_UPDATE	BIT(3)
+#define SUPPORTS_LATCHED_INPUT	BIT(4)
 	u8 flags;
+	u8 latch_ack_bit;
+	bool latch_ack_inverse;
 
 	unsigned long min_rate;
 	unsigned long max_rate;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v3 08/11] clk: qcom: mmcc-8996: Add capability flags for some alpha PLLs

[Rajendra Nayak]

Flag alpha PLLs which support fsmmode, dynamic update and the ones
with latched input interface.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/clk/qcom/mmcc-msm8996.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/qcom/mmcc-msm8996.c b/drivers/clk/qcom/mmcc-msm8996.c
index 847dd9d..f2f40ce 100644
--- a/drivers/clk/qcom/mmcc-msm8996.c
+++ b/drivers/clk/qcom/mmcc-msm8996.c
@@ -269,6 +269,7 @@ static struct clk_alpha_pll mmpll0_early = {
 	.offset = 0x0,
 	.vco_table = mmpll_p_vco,
 	.num_vco = ARRAY_SIZE(mmpll_p_vco),
+	.flags = SUPPORTS_FSM_MODE,
 	.clkr = {
 		.enable_reg = 0x100,
 		.enable_mask = BIT(0),
@@ -297,6 +298,7 @@ static struct clk_alpha_pll mmpll1_early = {
 	.offset = 0x30,
 	.vco_table = mmpll_p_vco,
 	.num_vco = ARRAY_SIZE(mmpll_p_vco),
+	.flags = SUPPORTS_FSM_MODE,
 	.clkr = {
 		.enable_reg = 0x100,
 		.enable_mask = BIT(1),
@@ -445,6 +447,8 @@ static struct clk_alpha_pll mmpll9_early = {
 	.offset = 0x4200,
 	.vco_table = mmpll_t_vco,
 	.num_vco = ARRAY_SIZE(mmpll_t_vco),
+	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_LATCHED_INPUT,
+	.latch_ack_bit = 29,
 	.clkr.hw.init = &(struct clk_init_data){
 		.name = "mmpll9_early",
 		.parent_names = (const char *[]){ "xo" },
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v3 09/11] clk: qcom: Add support for table based lookups in clk-regmap-mux

[Rajendra Nayak]

Add support to pass a table of values for parent selection similar
to whats done in clk-mux

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/clk/qcom/clk-regmap-mux.c | 13 +++++++++++++
 drivers/clk/qcom/clk-regmap-mux.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/clk/qcom/clk-regmap-mux.c b/drivers/clk/qcom/clk-regmap-mux.c
index cae3071..14a2af5 100644
--- a/drivers/clk/qcom/clk-regmap-mux.c
+++ b/drivers/clk/qcom/clk-regmap-mux.c
@@ -28,6 +28,7 @@ static u8 mux_get_parent(struct clk_hw *hw)
 	struct clk_regmap_mux *mux = to_clk_regmap_mux(hw);
 	struct clk_regmap *clkr = to_clk_regmap(hw);
 	unsigned int mask = GENMASK(mux->width - 1, 0);
+	int num_parents = clk_hw_get_num_parents(hw);
 	unsigned int val;
 
 	regmap_read(clkr->regmap, mux->reg, &val);
@@ -35,6 +36,15 @@ static u8 mux_get_parent(struct clk_hw *hw)
 	val >>= mux->shift;
 	val &= mask;
 
+	if (mux->table) {
+		int i;
+
+		for (i = 0; i < num_parents; i++)
+			if (mux->table[i] == val)
+				return i;
+		return -EINVAL;
+	}
+
 	return val;
 }
 
@@ -45,6 +55,9 @@ static int mux_set_parent(struct clk_hw *hw, u8 index)
 	unsigned int mask = GENMASK(mux->width + mux->shift - 1, mux->shift);
 	unsigned int val;
 
+	if (mux->table)
+		index = mux->table[index];
+
 	val = index;
 	val <<= mux->shift;
 
diff --git a/drivers/clk/qcom/clk-regmap-mux.h b/drivers/clk/qcom/clk-regmap-mux.h
index 5cec761..310804e 100644
--- a/drivers/clk/qcom/clk-regmap-mux.h
+++ b/drivers/clk/qcom/clk-regmap-mux.h
@@ -21,6 +21,7 @@ struct clk_regmap_mux {
 	u32			reg;
 	u32			shift;
 	u32			width;
+	u32			*table;
 	struct clk_regmap	clkr;
 };
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v3 10/11] clk: Add clk_hw_get_clk() helper API to be used by clk providers

[Rajendra Nayak]

Add a helper API that will allow clk providers to turn their clk_hw
structures into struct clk pointer.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/clk/clk.c            | 6 ++++++
 include/linux/clk-provider.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 820a939..a084132 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -186,6 +186,12 @@ const char *clk_hw_get_name(const struct clk_hw *hw)
 }
 EXPORT_SYMBOL_GPL(clk_hw_get_name);
 
+struct clk *clk_hw_get_clk(const struct clk_hw *hw)
+{
+	return hw->clk;
+}
+EXPORT_SYMBOL_GPL(clk_hw_get_clk);
+
 struct clk_hw *__clk_get_hw(struct clk *clk)
 {
 	return !clk ? NULL : clk->core->hw;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index a39c0c5..77d05f1 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -729,6 +729,7 @@ void devm_clk_hw_unregister(struct device *dev, struct clk_hw *hw);
 /* helper functions */
 const char *__clk_get_name(const struct clk *clk);
 const char *clk_hw_get_name(const struct clk_hw *hw);
+struct clk *clk_hw_get_clk(const struct clk_hw *hw);
 struct clk_hw *__clk_get_hw(struct clk *clk);
 unsigned int clk_hw_get_num_parents(const struct clk_hw *hw);
 struct clk_hw *clk_hw_get_parent(const struct clk_hw *hw);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v3 10/11] clk: Add clk_hw_get_clk() helper API to be used by clk providers

[Stephen Boyd]

On 09/29, Rajendra Nayak wrote:
> Add a helper API that will allow clk providers to turn their clk_hw
> structures into struct clk pointer.
> 

Did I suggest this?

> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/clk/clk.c            | 6 ++++++
>  include/linux/clk-provider.h | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 820a939..a084132 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -186,6 +186,12 @@ const char *clk_hw_get_name(const struct clk_hw *hw)
>  }
>  EXPORT_SYMBOL_GPL(clk_hw_get_name);
>  
> +struct clk *clk_hw_get_clk(const struct clk_hw *hw)
> +{
> +	return hw->clk;

I'd prefer we actually went through all the work and actually
allocated another clk structure here. We can add another string
or two for the dev_id and con_id too for debug/accouting
purposes.

The hw->clk is sort of a remnant of the clk_core introduction. I
can't recall the exact plan (i.e. I should write it down
somewhere once I do) but I think we want to get rid of hw->clk
and have everyone use clk_hw_get_clk() instead. Then for
traversals from the clk_hw pointer to a clk pointer are very
clear.

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

Re: [PATCH v3 10/11] clk: Add clk_hw_get_clk() helper API to be used by clk providers

[Rajendra Nayak]

On 11/03/2016 03:52 AM, Stephen Boyd wrote:
> On 09/29, Rajendra Nayak wrote:
>> Add a helper API that will allow clk providers to turn their clk_hw
>> structures into struct clk pointer.
>>
> 
> Did I suggest this?

I did this back when you suggested we store clk_hw's for all
clocks associated with a gdsc instead of extracting them from
the clients device tree node.

> 
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>>  drivers/clk/clk.c            | 6 ++++++
>>  include/linux/clk-provider.h | 1 +
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 820a939..a084132 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -186,6 +186,12 @@ const char *clk_hw_get_name(const struct clk_hw *hw)
>>  }
>>  EXPORT_SYMBOL_GPL(clk_hw_get_name);
>>  
>> +struct clk *clk_hw_get_clk(const struct clk_hw *hw)
>> +{
>> +	return hw->clk;
> 
> I'd prefer we actually went through all the work and actually
> allocated another clk structure here. We can add another string
> or two for the dev_id and con_id too for debug/accouting
> purposes.

I don't quite get the part of allocating another clk structure,
can you please elaborate?

> 
> The hw->clk is sort of a remnant of the clk_core introduction. I
> can't recall the exact plan (i.e. I should write it down
> somewhere once I do) but I think we want to get rid of hw->clk
> and have everyone use clk_hw_get_clk() instead. Then for
> traversals from the clk_hw pointer to a clk pointer are very
> clear.
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v3 10/11] clk: Add clk_hw_get_clk() helper API to be used by clk providers

[Stephen Boyd]

On 11/03, Rajendra Nayak wrote:
> 
> 
> On 11/03/2016 03:52 AM, Stephen Boyd wrote:
> > On 09/29, Rajendra Nayak wrote:
> >> Add a helper API that will allow clk providers to turn their clk_hw
> >> structures into struct clk pointer.
> >>
> > 
> > Did I suggest this?
> 
> I did this back when you suggested we store clk_hw's for all
> clocks associated with a gdsc instead of extracting them from
> the clients device tree node.
> 
> > 
> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> >> ---
> >>  drivers/clk/clk.c            | 6 ++++++
> >>  include/linux/clk-provider.h | 1 +
> >>  2 files changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> >> index 820a939..a084132 100644
> >> --- a/drivers/clk/clk.c
> >> +++ b/drivers/clk/clk.c
> >> @@ -186,6 +186,12 @@ const char *clk_hw_get_name(const struct clk_hw *hw)
> >>  }
> >>  EXPORT_SYMBOL_GPL(clk_hw_get_name);
> >>  
> >> +struct clk *clk_hw_get_clk(const struct clk_hw *hw)
> >> +{
> >> +	return hw->clk;
> > 
> > I'd prefer we actually went through all the work and actually
> > allocated another clk structure here. We can add another string
> > or two for the dev_id and con_id too for debug/accouting
> > purposes.
> 
> I don't quite get the part of allocating another clk structure,
> can you please elaborate?
> 

With "per-user" clks we want to have accounting for all the
struct clk consumers of a particular clk_hw instance. Given that
this function lets someone get a clk pointer from a clk_hw
structure, we want to make it follow the same "per-user"
accounting that we already have for each time the call is made.

Also, we want to make sure callers call clk_put() on the clk when
they're done with it. If we don't allocate a new one here that
clk_put() will destroy the one we allocate and assign during
registration time.

So I'm saying we want a wrapper to __clk_create_clk() for clk
providers to use to generate a clk pointer if needed.

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

[RFC v3 11/11] clk: qcom: Add basic CPU clock driver for msm8996

[Rajendra Nayak]

This is a skeletal CPU clock driver, which adds support for the
CPU SS primary as well as secondary/alternate PLLs, and the
primary/secondary muxes.

This still has support missing for
1. CBF PLL and mux
2. ACD
3. APM

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/clk/qcom/Makefile       |   1 +
 drivers/clk/qcom/clk-cpu-8996.c | 408 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 409 insertions(+)
 create mode 100644 drivers/clk/qcom/clk-cpu-8996.c

diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 2a25f4e..407668d 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -10,6 +10,7 @@ clk-qcom-y += clk-branch.o
 clk-qcom-y += clk-regmap-divider.o
 clk-qcom-y += clk-regmap-mux.o
 clk-qcom-y += reset.o
+clk-qcom-y += clk-cpu-8996.o
 clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
 
 obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
new file mode 100644
index 0000000..e690544
--- /dev/null
+++ b/drivers/clk/qcom/clk-cpu-8996.c
@@ -0,0 +1,408 @@
+/*
+ * Copyright (c) 2016, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/cpu.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+#include "clk-alpha-pll.h"
+#include "clk-pll.h"
+#include "clk-regmap.h"
+#include "clk-regmap-mux.h"
+
+#define VCO(a, b, c) { \
+	.val = a,\
+	.min_freq = b,\
+	.max_freq = c,\
+}
+
+static const struct alpha_pll_config hfpll_config = {
+	.l = 60,
+	.config_ctl_val = 0x200D4828,
+	.config_ctl_hi_val = 0x006,
+	.pre_div_mask = BIT(12),
+	.post_div_mask = 0x3 << 8,
+	.main_output_mask = BIT(0),
+	.early_output_mask = BIT(3),
+};
+
+static struct clk_alpha_pll perfcl_pll = {
+	.offset = 0x80000,
+	.min_rate = 600000000,
+	.max_rate = 3000000000,
+	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_16BIT_ALPHA
+			| SUPPORTS_FSM_MODE,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "perfcl_pll",
+		.parent_names = (const char *[]){ "xo_board" },
+		.num_parents = 1,
+		.ops = &clk_alpha_pll_hwfsm_ops,
+	},
+};
+
+static struct clk_alpha_pll pwrcl_pll = {
+	.offset = 0x0,
+	.min_rate = 600000000,
+	.max_rate = 3000000000,
+	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_16BIT_ALPHA
+			| SUPPORTS_FSM_MODE,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "pwrcl_pll",
+		.parent_names = (const char *[]){ "xo_board" },
+		.num_parents = 1,
+		.ops = &clk_alpha_pll_hwfsm_ops,
+	},
+};
+
+static const struct pll_vco alt_pll_vco_modes[] = {
+	VCO(3,  250000000,  500000000),
+	VCO(2,  500000000,  750000000),
+	VCO(1,  750000000, 1000000000),
+	VCO(0, 1000000000, 2150400000),
+};
+
+static const struct alpha_pll_config altpll_config = {
+	.l = 16,
+	.vco_val = 0x3 << 20,
+	.vco_mask = 0x3 << 20,
+	.config_ctl_val = 0x4001051B,
+	.post_div_mask = 0x3 << 8,
+	.post_div_val = 0x1,
+	.main_output_mask = BIT(0),
+	.early_output_mask = BIT(3),
+};
+
+static struct clk_alpha_pll perfcl_alt_pll = {
+	.offset = 0x80100,
+	.vco_table = alt_pll_vco_modes,
+	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
+	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "perfcl_alt_pll",
+		.parent_names = (const char *[]){ "xo_board" },
+		.num_parents = 1,
+		.ops = &clk_alpha_pll_hwfsm_ops,
+	},
+};
+
+static struct clk_alpha_pll pwrcl_alt_pll = {
+	.offset = 0x100,
+	.vco_table = alt_pll_vco_modes,
+	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
+	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "pwrcl_alt_pll",
+		.parent_names = (const char *[]){ "xo_board" },
+		.num_parents = 1,
+		.ops = &clk_alpha_pll_hwfsm_ops,
+	},
+};
+
+static struct clk_regmap_mux pwrcl_pmux = {
+	.reg = 0x40,
+	.shift = 0,
+	.width = 2,
+	.table = (u32 []){0, 1, 3},
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "pwrcl_pmux",
+		.parent_names = (const char *[]){
+			"pwrcl_smux",
+			"pwrcl_pll",
+			"pwrcl_alt_pll",
+		},
+		.num_parents = 3,
+		.ops = &clk_regmap_mux_closest_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap_mux pwrcl_smux = {
+	.reg = 0x40,
+	.shift = 2,
+	.width = 2,
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "pwrcl_smux",
+		.parent_names = (const char *[]){
+			"xo_board",
+			"pwrcl_pll_main",
+			"sys_apcscbf_clk",
+			"sys_apcsaux_clk",
+		},
+		.num_parents = 4,
+		.ops = &clk_regmap_mux_closest_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap_mux perfcl_pmux = {
+	.reg = 0x80040,
+	.shift = 0,
+	.width = 2,
+	.table = (u32 []){0, 1, 3},
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "perfcl_pmux",
+		.parent_names = (const char *[]){
+			"perfcl_smux",
+			"perfcl_pll",
+			"perfcl_alt_pll",
+		},
+		.num_parents = 3,
+		.ops = &clk_regmap_mux_closest_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+static struct clk_regmap_mux perfcl_smux = {
+	.reg = 0x80040,
+	.shift = 2,
+	.width = 2,
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "perfcl_smux",
+		.parent_names = (const char *[]){
+			"xo_board",
+			"perfcl_pll_main",
+			"sys_apcscbf_clk",
+			"sys_apcsaux_clk",
+		},
+		.num_parents = 4,
+		.ops = &clk_regmap_mux_closest_ops,
+		.flags = CLK_SET_RATE_PARENT,
+	},
+};
+
+struct clk_cpu_8996 {
+	struct clk_hw *alt_clk;
+	struct clk_hw *pll;
+	struct clk_regmap clkr;
+};
+
+static inline struct clk_cpu_8996 *to_clk_cpu_8996(struct clk_hw *hw)
+{
+	return container_of(to_clk_regmap(hw), struct clk_cpu_8996, clkr);
+}
+
+static int clk_cpu_8996_set_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long prate)
+{
+	int ret;
+	struct clk_cpu_8996 *cpuclk = to_clk_cpu_8996(hw);
+	struct clk *alt_clk, *pll, *parent;
+
+	alt_clk = clk_hw_get_clk(cpuclk->alt_clk);
+	pll = clk_hw_get_clk(cpuclk->pll);
+	parent = clk_hw_get_clk(clk_hw_get_parent(hw));
+
+	/* Switch parent to alt clk */
+	if (cpuclk->alt_clk) {
+		ret = clk_set_parent(parent, alt_clk);
+		if (ret)
+			return ret;
+	}
+
+	/* Set the PLL to new rate */
+	ret = clk_set_rate(pll, rate);
+	if (ret)
+		goto error;
+
+	/* Switch back to primary pll */
+	if (cpuclk->alt_clk) {
+		ret = clk_set_parent(parent, pll);
+		if (ret)
+			goto error;
+	}
+	return 0;
+
+error:
+	if (cpuclk->alt_clk)
+		clk_set_parent(parent, pll);
+
+	return ret;
+}
+
+static unsigned long clk_cpu_8996_recalc_rate(struct clk_hw *hw,
+					      unsigned long prate)
+{
+	return clk_hw_get_rate(clk_hw_get_parent(hw));
+}
+
+static long clk_cpu_8996_round_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long *prate)
+{
+	return clk_hw_round_rate(clk_hw_get_parent(hw), rate);
+}
+
+static struct clk_ops clk_cpu_8996_ops = {
+	.set_rate = clk_cpu_8996_set_rate,
+	.recalc_rate = clk_cpu_8996_recalc_rate,
+	.round_rate = clk_cpu_8996_round_rate,
+};
+
+static struct clk_cpu_8996 pwrcl_clk = {
+	.alt_clk = &pwrcl_alt_pll.clkr.hw,
+	.pll = &pwrcl_pll.clkr.hw,
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "pwrcl_clk",
+		.parent_names = (const char *[]){ "pwrcl_pmux" },
+		.num_parents = 1,
+		.ops = &clk_cpu_8996_ops,
+	},
+};
+
+static struct clk_cpu_8996 perfcl_clk = {
+	.alt_clk = &perfcl_alt_pll.clkr.hw,
+	.pll = &perfcl_pll.clkr.hw,
+	.clkr.hw.init = &(struct clk_init_data) {
+		.name = "perfcl_clk",
+		.parent_names = (const char *[]){ "perfcl_pmux" },
+		.num_parents = 1,
+		.ops = &clk_cpu_8996_ops,
+	},
+};
+
+static const struct regmap_config cpu_msm8996_regmap_config = {
+	.reg_bits		= 32,
+	.reg_stride		= 4,
+	.val_bits		= 32,
+	.max_register		= 0x80210,
+	.fast_io		= true,
+	.val_format_endian	= REGMAP_ENDIAN_LITTLE,
+};
+
+static const struct of_device_id match_table[] = {
+	{ .compatible = "qcom,cpu-clk-msm8996" },
+	{}
+};
+
+#define cluster_clk_register(dev, clk, clkr) { \
+	clk = devm_clk_register_regmap(dev, clkr); \
+	if (IS_ERR(clk)) \
+		return PTR_ERR(clk); }
+
+#define cpu_clk_register_fixed(dev, clk, name, pname, flags, m, n) { \
+	clk = clk_register_fixed_factor(dev, name, pname, flags, m, n); \
+	if (IS_ERR(clk)) \
+		return PTR_ERR(clk); }
+
+#define cpu_set_rate(dev, clk, rate) { \
+	if (clk_set_rate(clk, rate)) \
+		dev_err(dev, "Failed to set " #clk " to " #rate "\n"); }
+
+#define cpu_prepare_enable(dev, clk) { \
+	if (clk_prepare_enable(clk)) \
+		dev_err(dev, "Failed to enable " #clk "\n"); }
+
+#define cpu_set_parent(dev, clk, parent) { \
+	if (clk_set_parent(clk, parent)) \
+		dev_err(dev, "Failed to set parent for " #clk "\n"); }
+
+struct clk *pwr_clk, *perf_clk;
+
+static int register_cpu_clocks(struct device *dev, struct regmap *regmap)
+{
+	struct clk *perf_alt_pll, *pwr_alt_pll, *perf_pll, *pwr_pll;
+	struct clk *perf_pmux, *perf_smux, *pwr_pmux, *pwr_smux;
+	struct clk *perf_pll_main, *pwr_pll_main;
+
+	/* register PLLs */
+	cluster_clk_register(dev, perf_pll, &perfcl_pll.clkr);
+	cluster_clk_register(dev, pwr_pll, &pwrcl_pll.clkr);
+	cluster_clk_register(dev, perf_alt_pll, &perfcl_alt_pll.clkr);
+	cluster_clk_register(dev, pwr_alt_pll, &pwrcl_alt_pll.clkr);
+
+	/* register MUXs */
+	cluster_clk_register(dev, perf_pmux, &perfcl_pmux.clkr);
+	cluster_clk_register(dev, perf_smux, &perfcl_smux.clkr);
+	cluster_clk_register(dev, pwr_pmux, &pwrcl_pmux.clkr);
+	cluster_clk_register(dev, pwr_smux, &pwrcl_smux.clkr);
+
+	/* register Fixed clks */
+	cpu_clk_register_fixed(dev, perf_pll_main, "perfcl_pll_main",
+			       "perfcl_pll", CLK_SET_RATE_PARENT, 1, 2);
+	cpu_clk_register_fixed(dev, pwr_pll_main, "pwrcl_pll_main",
+			       "pwrcl_pll", CLK_SET_RATE_PARENT, 1, 2);
+
+	/* Register CPU clks */
+	cluster_clk_register(dev, perf_clk, &perfcl_clk.clkr);
+	cluster_clk_register(dev, pwr_clk, &pwrcl_clk.clkr);
+
+	/* Initialise the PLLs */
+	clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config);
+	clk_alpha_pll_configure(&pwrcl_pll, regmap, &hfpll_config);
+	clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config);
+	clk_alpha_pll_configure(&pwrcl_alt_pll, regmap, &altpll_config);
+
+	/* Enable all PLLs and alt PLLs */
+	cpu_prepare_enable(dev, perf_pll);
+	cpu_prepare_enable(dev, pwr_pll);
+	cpu_prepare_enable(dev, perf_alt_pll);
+	cpu_prepare_enable(dev, pwr_alt_pll);
+
+	/* Init MUXes with default parents */
+	cpu_set_parent(dev, perf_pmux, perf_pll);
+	cpu_set_parent(dev, pwr_pmux, pwr_pll);
+	cpu_set_parent(dev, perf_smux, perf_pll_main);
+	cpu_set_parent(dev, pwr_smux, pwr_pll_main);
+
+	return 0;
+}
+
+static int qcom_cpu_clk_msm8996_driver_probe(struct platform_device *pdev)
+{
+	int ret;
+	void __iomem *base;
+	struct resource *res;
+	struct clk_onecell_data *data;
+	struct device *dev = &pdev->dev;
+	struct regmap *regmap_cpu;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->clks = devm_kcalloc(dev, 3, sizeof(struct clk *), GFP_KERNEL);
+	if (!data->clks)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	regmap_cpu = devm_regmap_init_mmio(dev, base,
+					   &cpu_msm8996_regmap_config);
+	if (IS_ERR(regmap_cpu))
+		return PTR_ERR(regmap_cpu);
+
+	ret = register_cpu_clocks(dev, regmap_cpu);
+	if (ret)
+		return ret;
+
+	data->clks[0] = pwr_clk;
+	data->clks[1] = perf_clk;
+	data->clk_num = 2;
+
+	return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get, data);
+}
+
+static struct platform_driver qcom_cpu_clk_msm8996_driver = {
+	.probe = qcom_cpu_clk_msm8996_driver_probe,
+	.driver = {
+		.name = "qcom-cpu-clk-msm8996",
+		.of_match_table = match_table,
+	},
+};
+
+builtin_platform_driver(qcom_cpu_clk_msm8996_driver);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [RFC v3 11/11] clk: qcom: Add basic CPU clock driver for msm8996

[Stephen Boyd]

On 09/29, Rajendra Nayak wrote:
> This is a skeletal CPU clock driver, which adds support for the
> CPU SS primary as well as secondary/alternate PLLs, and the
> primary/secondary muxes.
> 
> This still has support missing for
> 1. CBF PLL and mux
> 2. ACD
> 3. APM

Maybe you can put a clk tree diagram here so we understand the
hierarchy.

> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---

DT binding document?

> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 2a25f4e..407668d 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -10,6 +10,7 @@ clk-qcom-y += clk-branch.o
>  clk-qcom-y += clk-regmap-divider.o
>  clk-qcom-y += clk-regmap-mux.o
>  clk-qcom-y += reset.o
> +clk-qcom-y += clk-cpu-8996.o

Please add a config option.

>  clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
>  
>  obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
> diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
> new file mode 100644
> index 0000000..e690544
> --- /dev/null
> +++ b/drivers/clk/qcom/clk-cpu-8996.c
> @@ -0,0 +1,408 @@
> +/*
> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/cpu.h>

Is this used?

> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-pll.h"
> +#include "clk-regmap.h"
> +#include "clk-regmap-mux.h"
> +
> +#define VCO(a, b, c) { \
> +	.val = a,\
> +	.min_freq = b,\
> +	.max_freq = c,\
> +}
> +
> +static const struct alpha_pll_config hfpll_config = {
> +	.l = 60,
> +	.config_ctl_val = 0x200D4828,
> +	.config_ctl_hi_val = 0x006,
> +	.pre_div_mask = BIT(12),
> +	.post_div_mask = 0x3 << 8,
> +	.main_output_mask = BIT(0),
> +	.early_output_mask = BIT(3),
> +};
> +
> +static struct clk_alpha_pll perfcl_pll = {
> +	.offset = 0x80000,
> +	.min_rate = 600000000,
> +	.max_rate = 3000000000,
> +	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_16BIT_ALPHA
> +			| SUPPORTS_FSM_MODE,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "perfcl_pll",
> +		.parent_names = (const char *[]){ "xo_board" },
> +		.num_parents = 1,
> +		.ops = &clk_alpha_pll_hwfsm_ops,
> +	},
> +};
> +
> +static struct clk_alpha_pll pwrcl_pll = {
> +	.offset = 0x0,
> +	.min_rate = 600000000,
> +	.max_rate = 3000000000,
> +	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_16BIT_ALPHA
> +			| SUPPORTS_FSM_MODE,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "pwrcl_pll",
> +		.parent_names = (const char *[]){ "xo_board" },
> +		.num_parents = 1,
> +		.ops = &clk_alpha_pll_hwfsm_ops,
> +	},
> +};
> +
> +static const struct pll_vco alt_pll_vco_modes[] = {
> +	VCO(3,  250000000,  500000000),
> +	VCO(2,  500000000,  750000000),
> +	VCO(1,  750000000, 1000000000),
> +	VCO(0, 1000000000, 2150400000),
> +};
> +
> +static const struct alpha_pll_config altpll_config = {
> +	.l = 16,
> +	.vco_val = 0x3 << 20,
> +	.vco_mask = 0x3 << 20,
> +	.config_ctl_val = 0x4001051B,

Lower case hex please.

> +	.post_div_mask = 0x3 << 8,
> +	.post_div_val = 0x1,
> +	.main_output_mask = BIT(0),
> +	.early_output_mask = BIT(3),
> +};
> +
> +static struct clk_alpha_pll perfcl_alt_pll = {
> +	.offset = 0x80100,
> +	.vco_table = alt_pll_vco_modes,
> +	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
> +	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
> +	.clkr.hw.init = &(struct clk_init_data) {
> +		.name = "perfcl_alt_pll",
> +		.parent_names = (const char *[]){ "xo_board" },
> +		.num_parents = 1,
> +		.ops = &clk_alpha_pll_hwfsm_ops,
> +	},
> +};
> +
> +static struct clk_alpha_pll pwrcl_alt_pll = {
> +	.offset = 0x100,
> +	.vco_table = alt_pll_vco_modes,
> +	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
> +	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
> +	.clkr.hw.init = &(struct clk_init_data) {
> +		.name = "pwrcl_alt_pll",
> +		.parent_names = (const char *[]){ "xo_board" },
> +		.num_parents = 1,
> +		.ops = &clk_alpha_pll_hwfsm_ops,
> +	},
> +};
> +
> +static struct clk_regmap_mux pwrcl_pmux = {
> +	.reg = 0x40,
> +	.shift = 0,
> +	.width = 2,
> +	.table = (u32 []){0, 1, 3},
> +	.clkr.hw.init = &(struct clk_init_data) {
> +		.name = "pwrcl_pmux",
> +		.parent_names = (const char *[]){
> +			"pwrcl_smux",
> +			"pwrcl_pll",
> +			"pwrcl_alt_pll",
> +		},
> +		.num_parents = 3,
> +		.ops = &clk_regmap_mux_closest_ops,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_regmap_mux pwrcl_smux = {
> +	.reg = 0x40,
> +	.shift = 2,
> +	.width = 2,
> +	.clkr.hw.init = &(struct clk_init_data) {
> +		.name = "pwrcl_smux",
> +		.parent_names = (const char *[]){
> +			"xo_board",
> +			"pwrcl_pll_main",
> +			"sys_apcscbf_clk",
> +			"sys_apcsaux_clk",
> +		},
> +		.num_parents = 4,
> +		.ops = &clk_regmap_mux_closest_ops,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_regmap_mux perfcl_pmux = {
> +	.reg = 0x80040,
> +	.shift = 0,
> +	.width = 2,
> +	.table = (u32 []){0, 1, 3},
> +	.clkr.hw.init = &(struct clk_init_data) {
> +		.name = "perfcl_pmux",
> +		.parent_names = (const char *[]){
> +			"perfcl_smux",
> +			"perfcl_pll",
> +			"perfcl_alt_pll",
> +		},
> +		.num_parents = 3,
> +		.ops = &clk_regmap_mux_closest_ops,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +static struct clk_regmap_mux perfcl_smux = {
> +	.reg = 0x80040,
> +	.shift = 2,
> +	.width = 2,
> +	.clkr.hw.init = &(struct clk_init_data) {
> +		.name = "perfcl_smux",
> +		.parent_names = (const char *[]){
> +			"xo_board",

Just use xo please.

> +			"perfcl_pll_main",
> +			"sys_apcscbf_clk",
> +			"sys_apcsaux_clk",
> +		},
> +		.num_parents = 4,
> +		.ops = &clk_regmap_mux_closest_ops,
> +		.flags = CLK_SET_RATE_PARENT,
> +	},
> +};
> +
> +struct clk_cpu_8996 {
> +	struct clk_hw *alt_clk;
> +	struct clk_hw *pll;
> +	struct clk_regmap clkr;
> +};
> +
> +static inline struct clk_cpu_8996 *to_clk_cpu_8996(struct clk_hw *hw)
> +{
> +	return container_of(to_clk_regmap(hw), struct clk_cpu_8996, clkr);
> +}
> +
> +static int clk_cpu_8996_set_rate(struct clk_hw *hw, unsigned long rate,
> +				 unsigned long prate)
> +{
> +	int ret;
> +	struct clk_cpu_8996 *cpuclk = to_clk_cpu_8996(hw);
> +	struct clk *alt_clk, *pll, *parent;
> +
> +	alt_clk = clk_hw_get_clk(cpuclk->alt_clk);
> +	pll = clk_hw_get_clk(cpuclk->pll);
> +	parent = clk_hw_get_clk(clk_hw_get_parent(hw));
> +
> +	/* Switch parent to alt clk */
> +	if (cpuclk->alt_clk) {

This would be false sometimes?

> +		ret = clk_set_parent(parent, alt_clk);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Set the PLL to new rate */
> +	ret = clk_set_rate(pll, rate);
> +	if (ret)
> +		goto error;
> +
> +	/* Switch back to primary pll */
> +	if (cpuclk->alt_clk) {
> +		ret = clk_set_parent(parent, pll);
> +		if (ret)
> +			goto error;
> +	}
> +	return 0;
> +
> +error:
> +	if (cpuclk->alt_clk)
> +		clk_set_parent(parent, pll);
> +
> +	return ret;
> +}
> +
> +static unsigned long clk_cpu_8996_recalc_rate(struct clk_hw *hw,
> +					      unsigned long prate)
> +{
> +	return clk_hw_get_rate(clk_hw_get_parent(hw));
> +}

If we just pass through parent rate I'm confused what the point
of recalc is here.

> +
> +static long clk_cpu_8996_round_rate(struct clk_hw *hw, unsigned long rate,
> +				    unsigned long *prate)
> +{
> +	return clk_hw_round_rate(clk_hw_get_parent(hw), rate);

Same here. The core does this already?

> +}
> +
> +static struct clk_ops clk_cpu_8996_ops = {

const?

> +	.set_rate = clk_cpu_8996_set_rate,
> +	.recalc_rate = clk_cpu_8996_recalc_rate,
> +	.round_rate = clk_cpu_8996_round_rate,

This all feels fake... Please fold it onto the mux clk, because
the mux is the true output to the CPU and not this software clk
thing here.

> +};
> +
> +static struct clk_cpu_8996 pwrcl_clk = {
> +	.alt_clk = &pwrcl_alt_pll.clkr.hw,
> +	.pll = &pwrcl_pll.clkr.hw,
> +	.clkr.hw.init = &(struct clk_init_data) {
> +		.name = "pwrcl_clk",
> +		.parent_names = (const char *[]){ "pwrcl_pmux" },
> +		.num_parents = 1,
> +		.ops = &clk_cpu_8996_ops,
> +	},
> +};
> +
> +static struct clk_cpu_8996 perfcl_clk = {
> +	.alt_clk = &perfcl_alt_pll.clkr.hw,
> +	.pll = &perfcl_pll.clkr.hw,
> +	.clkr.hw.init = &(struct clk_init_data) {
> +		.name = "perfcl_clk",
> +		.parent_names = (const char *[]){ "perfcl_pmux" },
> +		.num_parents = 1,
> +		.ops = &clk_cpu_8996_ops,
> +	},
> +};
> +
> +static const struct regmap_config cpu_msm8996_regmap_config = {
> +	.reg_bits		= 32,
> +	.reg_stride		= 4,
> +	.val_bits		= 32,
> +	.max_register		= 0x80210,
> +	.fast_io		= true,
> +	.val_format_endian	= REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static const struct of_device_id match_table[] = {
> +	{ .compatible = "qcom,cpu-clk-msm8996" },
> +	{}
> +};
> +
> +#define cluster_clk_register(dev, clk, clkr) { \
> +	clk = devm_clk_register_regmap(dev, clkr); \
> +	if (IS_ERR(clk)) \
> +		return PTR_ERR(clk); }

Yuck. Why not have an array that we register in a loop instead?

> +
> +#define cpu_clk_register_fixed(dev, clk, name, pname, flags, m, n) { \
> +	clk = clk_register_fixed_factor(dev, name, pname, flags, m, n); \
> +	if (IS_ERR(clk)) \
> +		return PTR_ERR(clk); }

These could also be specified statically and registered in a
loop. Also, please use clk_hw based registration APIs.

> +
> +#define cpu_set_rate(dev, clk, rate) { \
> +	if (clk_set_rate(clk, rate)) \
> +		dev_err(dev, "Failed to set " #clk " to " #rate "\n"); }

Not used?

> +
> +#define cpu_prepare_enable(dev, clk) { \
> +	if (clk_prepare_enable(clk)) \
> +		dev_err(dev, "Failed to enable " #clk "\n"); }
> +
> +#define cpu_set_parent(dev, clk, parent) { \
> +	if (clk_set_parent(clk, parent)) \
> +		dev_err(dev, "Failed to set parent for " #clk "\n"); }
> +
> +struct clk *pwr_clk, *perf_clk;

static? Why do these need to be global though?

> +
> +static int register_cpu_clocks(struct device *dev, struct regmap *regmap)
> +{
> +	struct clk *perf_alt_pll, *pwr_alt_pll, *perf_pll, *pwr_pll;
> +	struct clk *perf_pmux, *perf_smux, *pwr_pmux, *pwr_smux;
> +	struct clk *perf_pll_main, *pwr_pll_main;
> +
> +	/* register PLLs */
> +	cluster_clk_register(dev, perf_pll, &perfcl_pll.clkr);
> +	cluster_clk_register(dev, pwr_pll, &pwrcl_pll.clkr);
> +	cluster_clk_register(dev, perf_alt_pll, &perfcl_alt_pll.clkr);
> +	cluster_clk_register(dev, pwr_alt_pll, &pwrcl_alt_pll.clkr);
> +
> +	/* register MUXs */
> +	cluster_clk_register(dev, perf_pmux, &perfcl_pmux.clkr);
> +	cluster_clk_register(dev, perf_smux, &perfcl_smux.clkr);
> +	cluster_clk_register(dev, pwr_pmux, &pwrcl_pmux.clkr);
> +	cluster_clk_register(dev, pwr_smux, &pwrcl_smux.clkr);
> +
> +	/* register Fixed clks */
> +	cpu_clk_register_fixed(dev, perf_pll_main, "perfcl_pll_main",
> +			       "perfcl_pll", CLK_SET_RATE_PARENT, 1, 2);
> +	cpu_clk_register_fixed(dev, pwr_pll_main, "pwrcl_pll_main",
> +			       "pwrcl_pll", CLK_SET_RATE_PARENT, 1, 2);
> +
> +	/* Register CPU clks */

Capitalized register this time?

> +	cluster_clk_register(dev, perf_clk, &perfcl_clk.clkr);
> +	cluster_clk_register(dev, pwr_clk, &pwrcl_clk.clkr);
> +
> +	/* Initialise the PLLs */
> +	clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config);
> +	clk_alpha_pll_configure(&pwrcl_pll, regmap, &hfpll_config);
> +	clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config);
> +	clk_alpha_pll_configure(&pwrcl_alt_pll, regmap, &altpll_config);
> +
> +	/* Enable all PLLs and alt PLLs */
> +	cpu_prepare_enable(dev, perf_pll);
> +	cpu_prepare_enable(dev, pwr_pll);
> +	cpu_prepare_enable(dev, perf_alt_pll);
> +	cpu_prepare_enable(dev, pwr_alt_pll);

Is this so the clks don't turn off at late init? Or because clk
framework doesn't know these things are already enabled? The
comment doesn't help in understanding here.

> +
> +	/* Init MUXes with default parents */
> +	cpu_set_parent(dev, perf_pmux, perf_pll);
> +	cpu_set_parent(dev, pwr_pmux, pwr_pll);
> +	cpu_set_parent(dev, perf_smux, perf_pll_main);
> +	cpu_set_parent(dev, pwr_smux, pwr_pll_main);

Can we use assigned-parents in DT instead?

> +
> +	return 0;
> +}
> +
> +static int qcom_cpu_clk_msm8996_driver_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	void __iomem *base;
> +	struct resource *res;
> +	struct clk_onecell_data *data;
> +	struct device *dev = &pdev->dev;
> +	struct regmap *regmap_cpu;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->clks = devm_kcalloc(dev, 3, sizeof(struct clk *), GFP_KERNEL);

sizeof(*data->clks) or I guess hw pointers now. Also 3 != 2?

> +	if (!data->clks)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	regmap_cpu = devm_regmap_init_mmio(dev, base,
> +					   &cpu_msm8996_regmap_config);
> +	if (IS_ERR(regmap_cpu))
> +		return PTR_ERR(regmap_cpu);
> +
> +	ret = register_cpu_clocks(dev, regmap_cpu);
> +	if (ret)
> +		return ret;
> +
> +	data->clks[0] = pwr_clk;
> +	data->clks[1] = perf_clk;
> +	data->clk_num = 2;
> +
> +	return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get, data);
> +}
> +
> +static struct platform_driver qcom_cpu_clk_msm8996_driver = {
> +	.probe = qcom_cpu_clk_msm8996_driver_probe,
> +	.driver = {
> +		.name = "qcom-cpu-clk-msm8996",
> +		.of_match_table = match_table,
> +	},
> +};
> +
> +builtin_platform_driver(qcom_cpu_clk_msm8996_driver);

It can't be a module?

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

Re: [RFC v3 11/11] clk: qcom: Add basic CPU clock driver for msm8996

[Rajendra Nayak]

On 11/03/2016 03:47 AM, Stephen Boyd wrote:
> On 09/29, Rajendra Nayak wrote:
>> This is a skeletal CPU clock driver, which adds support for the
>> CPU SS primary as well as secondary/alternate PLLs, and the
>> primary/secondary muxes.
>>
>> This still has support missing for
>> 1. CBF PLL and mux
>> 2. ACD
>> 3. APM
> 
> Maybe you can put a clk tree diagram here so we understand the
> hierarchy.

okay, will do.

> 
>>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
> 
> DT binding document?

will add.

>> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
>> index 2a25f4e..407668d 100644
>> --- a/drivers/clk/qcom/Makefile
>> +++ b/drivers/clk/qcom/Makefile
>> @@ -10,6 +10,7 @@ clk-qcom-y += clk-branch.o
>>  clk-qcom-y += clk-regmap-divider.o
>>  clk-qcom-y += clk-regmap-mux.o
>>  clk-qcom-y += reset.o
>> +clk-qcom-y += clk-cpu-8996.o
> 
> Please add a config option.

will do,

> 
>>  clk-qcom-$(CONFIG_QCOM_GDSC) += gdsc.o
>>  
>>  obj-$(CONFIG_APQ_GCC_8084) += gcc-apq8084.o
>> diff --git a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c
>> new file mode 100644
>> index 0000000..e690544
>> --- /dev/null
>> +++ b/drivers/clk/qcom/clk-cpu-8996.c
>> @@ -0,0 +1,408 @@
>> +/*
>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/cpu.h>
> 
> Is this used?

not used, will remove

> 
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +
>> +#include "clk-alpha-pll.h"
>> +#include "clk-pll.h"
>> +#include "clk-regmap.h"
>> +#include "clk-regmap-mux.h"
>> +
>> +#define VCO(a, b, c) { \
>> +	.val = a,\
>> +	.min_freq = b,\
>> +	.max_freq = c,\
>> +}
>> +
>> +static const struct alpha_pll_config hfpll_config = {
>> +	.l = 60,
>> +	.config_ctl_val = 0x200D4828,
>> +	.config_ctl_hi_val = 0x006,
>> +	.pre_div_mask = BIT(12),
>> +	.post_div_mask = 0x3 << 8,
>> +	.main_output_mask = BIT(0),
>> +	.early_output_mask = BIT(3),
>> +};
>> +
>> +static struct clk_alpha_pll perfcl_pll = {
>> +	.offset = 0x80000,
>> +	.min_rate = 600000000,
>> +	.max_rate = 3000000000,
>> +	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_16BIT_ALPHA
>> +			| SUPPORTS_FSM_MODE,
>> +	.clkr.hw.init = &(struct clk_init_data){
>> +		.name = "perfcl_pll",
>> +		.parent_names = (const char *[]){ "xo_board" },
>> +		.num_parents = 1,
>> +		.ops = &clk_alpha_pll_hwfsm_ops,
>> +	},
>> +};
>> +
>> +static struct clk_alpha_pll pwrcl_pll = {
>> +	.offset = 0x0,
>> +	.min_rate = 600000000,
>> +	.max_rate = 3000000000,
>> +	.flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_16BIT_ALPHA
>> +			| SUPPORTS_FSM_MODE,
>> +	.clkr.hw.init = &(struct clk_init_data){
>> +		.name = "pwrcl_pll",
>> +		.parent_names = (const char *[]){ "xo_board" },
>> +		.num_parents = 1,
>> +		.ops = &clk_alpha_pll_hwfsm_ops,
>> +	},
>> +};
>> +
>> +static const struct pll_vco alt_pll_vco_modes[] = {
>> +	VCO(3,  250000000,  500000000),
>> +	VCO(2,  500000000,  750000000),
>> +	VCO(1,  750000000, 1000000000),
>> +	VCO(0, 1000000000, 2150400000),
>> +};
>> +
>> +static const struct alpha_pll_config altpll_config = {
>> +	.l = 16,
>> +	.vco_val = 0x3 << 20,
>> +	.vco_mask = 0x3 << 20,
>> +	.config_ctl_val = 0x4001051B,
> 
> Lower case hex please.

will fix

> 
>> +	.post_div_mask = 0x3 << 8,
>> +	.post_div_val = 0x1,
>> +	.main_output_mask = BIT(0),
>> +	.early_output_mask = BIT(3),
>> +};
>> +
>> +static struct clk_alpha_pll perfcl_alt_pll = {
>> +	.offset = 0x80100,
>> +	.vco_table = alt_pll_vco_modes,
>> +	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
>> +	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
>> +	.clkr.hw.init = &(struct clk_init_data) {
>> +		.name = "perfcl_alt_pll",
>> +		.parent_names = (const char *[]){ "xo_board" },
>> +		.num_parents = 1,
>> +		.ops = &clk_alpha_pll_hwfsm_ops,
>> +	},
>> +};
>> +
>> +static struct clk_alpha_pll pwrcl_alt_pll = {
>> +	.offset = 0x100,
>> +	.vco_table = alt_pll_vco_modes,
>> +	.num_vco = ARRAY_SIZE(alt_pll_vco_modes),
>> +	.flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
>> +	.clkr.hw.init = &(struct clk_init_data) {
>> +		.name = "pwrcl_alt_pll",
>> +		.parent_names = (const char *[]){ "xo_board" },
>> +		.num_parents = 1,
>> +		.ops = &clk_alpha_pll_hwfsm_ops,
>> +	},
>> +};
>> +
>> +static struct clk_regmap_mux pwrcl_pmux = {
>> +	.reg = 0x40,
>> +	.shift = 0,
>> +	.width = 2,
>> +	.table = (u32 []){0, 1, 3},
>> +	.clkr.hw.init = &(struct clk_init_data) {
>> +		.name = "pwrcl_pmux",
>> +		.parent_names = (const char *[]){
>> +			"pwrcl_smux",
>> +			"pwrcl_pll",
>> +			"pwrcl_alt_pll",
>> +		},
>> +		.num_parents = 3,
>> +		.ops = &clk_regmap_mux_closest_ops,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +	},
>> +};
>> +
>> +static struct clk_regmap_mux pwrcl_smux = {
>> +	.reg = 0x40,
>> +	.shift = 2,
>> +	.width = 2,
>> +	.clkr.hw.init = &(struct clk_init_data) {
>> +		.name = "pwrcl_smux",
>> +		.parent_names = (const char *[]){
>> +			"xo_board",
>> +			"pwrcl_pll_main",
>> +			"sys_apcscbf_clk",
>> +			"sys_apcsaux_clk",
>> +		},
>> +		.num_parents = 4,
>> +		.ops = &clk_regmap_mux_closest_ops,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +	},
>> +};
>> +
>> +static struct clk_regmap_mux perfcl_pmux = {
>> +	.reg = 0x80040,
>> +	.shift = 0,
>> +	.width = 2,
>> +	.table = (u32 []){0, 1, 3},
>> +	.clkr.hw.init = &(struct clk_init_data) {
>> +		.name = "perfcl_pmux",
>> +		.parent_names = (const char *[]){
>> +			"perfcl_smux",
>> +			"perfcl_pll",
>> +			"perfcl_alt_pll",
>> +		},
>> +		.num_parents = 3,
>> +		.ops = &clk_regmap_mux_closest_ops,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +	},
>> +};
>> +
>> +static struct clk_regmap_mux perfcl_smux = {
>> +	.reg = 0x80040,
>> +	.shift = 2,
>> +	.width = 2,
>> +	.clkr.hw.init = &(struct clk_init_data) {
>> +		.name = "perfcl_smux",
>> +		.parent_names = (const char *[]){
>> +			"xo_board",
> 
> Just use xo please.

will do

> 
>> +			"perfcl_pll_main",
>> +			"sys_apcscbf_clk",
>> +			"sys_apcsaux_clk",
>> +		},
>> +		.num_parents = 4,
>> +		.ops = &clk_regmap_mux_closest_ops,
>> +		.flags = CLK_SET_RATE_PARENT,
>> +	},
>> +};
>> +
>> +struct clk_cpu_8996 {
>> +	struct clk_hw *alt_clk;
>> +	struct clk_hw *pll;
>> +	struct clk_regmap clkr;
>> +};
>> +
>> +static inline struct clk_cpu_8996 *to_clk_cpu_8996(struct clk_hw *hw)
>> +{
>> +	return container_of(to_clk_regmap(hw), struct clk_cpu_8996, clkr);
>> +}
>> +
>> +static int clk_cpu_8996_set_rate(struct clk_hw *hw, unsigned long rate,
>> +				 unsigned long prate)
>> +{
>> +	int ret;
>> +	struct clk_cpu_8996 *cpuclk = to_clk_cpu_8996(hw);
>> +	struct clk *alt_clk, *pll, *parent;
>> +
>> +	alt_clk = clk_hw_get_clk(cpuclk->alt_clk);
>> +	pll = clk_hw_get_clk(cpuclk->pll);
>> +	parent = clk_hw_get_clk(clk_hw_get_parent(hw));
>> +
>> +	/* Switch parent to alt clk */
>> +	if (cpuclk->alt_clk) {
> 
> This would be false sometimes?

maybe not, it would be better to error out without an alt_clk

> 
>> +		ret = clk_set_parent(parent, alt_clk);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	/* Set the PLL to new rate */
>> +	ret = clk_set_rate(pll, rate);
>> +	if (ret)
>> +		goto error;
>> +
>> +	/* Switch back to primary pll */
>> +	if (cpuclk->alt_clk) {
>> +		ret = clk_set_parent(parent, pll);
>> +		if (ret)
>> +			goto error;
>> +	}
>> +	return 0;
>> +
>> +error:
>> +	if (cpuclk->alt_clk)
>> +		clk_set_parent(parent, pll);
>> +
>> +	return ret;
>> +}
>> +
>> +static unsigned long clk_cpu_8996_recalc_rate(struct clk_hw *hw,
>> +					      unsigned long prate)
>> +{
>> +	return clk_hw_get_rate(clk_hw_get_parent(hw));
>> +}
> 
> If we just pass through parent rate I'm confused what the point
> of recalc is here.
> 
>> +
>> +static long clk_cpu_8996_round_rate(struct clk_hw *hw, unsigned long rate,
>> +				    unsigned long *prate)
>> +{
>> +	return clk_hw_round_rate(clk_hw_get_parent(hw), rate);
> 
> Same here. The core does this already?
> 
>> +}
>> +
>> +static struct clk_ops clk_cpu_8996_ops = {
> 
> const?
> 
>> +	.set_rate = clk_cpu_8996_set_rate,
>> +	.recalc_rate = clk_cpu_8996_recalc_rate,
>> +	.round_rate = clk_cpu_8996_round_rate,
> 
> This all feels fake... Please fold it onto the mux clk, because
> the mux is the true output to the CPU and not this software clk
> thing here.

Okay I will try to fold this into the mux clk

> 
>> +};
>> +
>> +static struct clk_cpu_8996 pwrcl_clk = {
>> +	.alt_clk = &pwrcl_alt_pll.clkr.hw,
>> +	.pll = &pwrcl_pll.clkr.hw,
>> +	.clkr.hw.init = &(struct clk_init_data) {
>> +		.name = "pwrcl_clk",
>> +		.parent_names = (const char *[]){ "pwrcl_pmux" },
>> +		.num_parents = 1,
>> +		.ops = &clk_cpu_8996_ops,
>> +	},
>> +};
>> +
>> +static struct clk_cpu_8996 perfcl_clk = {
>> +	.alt_clk = &perfcl_alt_pll.clkr.hw,
>> +	.pll = &perfcl_pll.clkr.hw,
>> +	.clkr.hw.init = &(struct clk_init_data) {
>> +		.name = "perfcl_clk",
>> +		.parent_names = (const char *[]){ "perfcl_pmux" },
>> +		.num_parents = 1,
>> +		.ops = &clk_cpu_8996_ops,
>> +	},
>> +};
>> +
>> +static const struct regmap_config cpu_msm8996_regmap_config = {
>> +	.reg_bits		= 32,
>> +	.reg_stride		= 4,
>> +	.val_bits		= 32,
>> +	.max_register		= 0x80210,
>> +	.fast_io		= true,
>> +	.val_format_endian	= REGMAP_ENDIAN_LITTLE,
>> +};
>> +
>> +static const struct of_device_id match_table[] = {
>> +	{ .compatible = "qcom,cpu-clk-msm8996" },
>> +	{}
>> +};
>> +
>> +#define cluster_clk_register(dev, clk, clkr) { \
>> +	clk = devm_clk_register_regmap(dev, clkr); \
>> +	if (IS_ERR(clk)) \
>> +		return PTR_ERR(clk); }
> 
> Yuck. Why not have an array that we register in a loop instead?
> 
>> +
>> +#define cpu_clk_register_fixed(dev, clk, name, pname, flags, m, n) { \
>> +	clk = clk_register_fixed_factor(dev, name, pname, flags, m, n); \
>> +	if (IS_ERR(clk)) \
>> +		return PTR_ERR(clk); }
> 
> These could also be specified statically and registered in a
> loop. Also, please use clk_hw based registration APIs.

sure, will define an array and loop over

> 
>> +
>> +#define cpu_set_rate(dev, clk, rate) { \
>> +	if (clk_set_rate(clk, rate)) \
>> +		dev_err(dev, "Failed to set " #clk " to " #rate "\n"); }
> 
> Not used?

yes, will remove

> 
>> +
>> +#define cpu_prepare_enable(dev, clk) { \
>> +	if (clk_prepare_enable(clk)) \
>> +		dev_err(dev, "Failed to enable " #clk "\n"); }
>> +
>> +#define cpu_set_parent(dev, clk, parent) { \
>> +	if (clk_set_parent(clk, parent)) \
>> +		dev_err(dev, "Failed to set parent for " #clk "\n"); }
>> +
>> +struct clk *pwr_clk, *perf_clk;
> 
> static? Why do these need to be global though?

because I used them in probe to do

>> +	data->clks[0] = pwr_clk;
>> +	data->clks[1] = perf_clk;

> 
>> +
>> +static int register_cpu_clocks(struct device *dev, struct regmap *regmap)
>> +{
>> +	struct clk *perf_alt_pll, *pwr_alt_pll, *perf_pll, *pwr_pll;
>> +	struct clk *perf_pmux, *perf_smux, *pwr_pmux, *pwr_smux;
>> +	struct clk *perf_pll_main, *pwr_pll_main;
>> +
>> +	/* register PLLs */
>> +	cluster_clk_register(dev, perf_pll, &perfcl_pll.clkr);
>> +	cluster_clk_register(dev, pwr_pll, &pwrcl_pll.clkr);
>> +	cluster_clk_register(dev, perf_alt_pll, &perfcl_alt_pll.clkr);
>> +	cluster_clk_register(dev, pwr_alt_pll, &pwrcl_alt_pll.clkr);
>> +
>> +	/* register MUXs */
>> +	cluster_clk_register(dev, perf_pmux, &perfcl_pmux.clkr);
>> +	cluster_clk_register(dev, perf_smux, &perfcl_smux.clkr);
>> +	cluster_clk_register(dev, pwr_pmux, &pwrcl_pmux.clkr);
>> +	cluster_clk_register(dev, pwr_smux, &pwrcl_smux.clkr);
>> +
>> +	/* register Fixed clks */
>> +	cpu_clk_register_fixed(dev, perf_pll_main, "perfcl_pll_main",
>> +			       "perfcl_pll", CLK_SET_RATE_PARENT, 1, 2);
>> +	cpu_clk_register_fixed(dev, pwr_pll_main, "pwrcl_pll_main",
>> +			       "pwrcl_pll", CLK_SET_RATE_PARENT, 1, 2);
>> +
>> +	/* Register CPU clks */
> 
> Capitalized register this time?

:) will fix

> 
>> +	cluster_clk_register(dev, perf_clk, &perfcl_clk.clkr);
>> +	cluster_clk_register(dev, pwr_clk, &pwrcl_clk.clkr);
>> +
>> +	/* Initialise the PLLs */
>> +	clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config);
>> +	clk_alpha_pll_configure(&pwrcl_pll, regmap, &hfpll_config);
>> +	clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config);
>> +	clk_alpha_pll_configure(&pwrcl_alt_pll, regmap, &altpll_config);
>> +
>> +	/* Enable all PLLs and alt PLLs */
>> +	cpu_prepare_enable(dev, perf_pll);
>> +	cpu_prepare_enable(dev, pwr_pll);
>> +	cpu_prepare_enable(dev, perf_alt_pll);
>> +	cpu_prepare_enable(dev, pwr_alt_pll);
> 
> Is this so the clks don't turn off at late init? Or because clk
> framework doesn't know these things are already enabled? The
> comment doesn't help in understanding here.

I don;t quite remember why I did this, will take a relook

> 
>> +
>> +	/* Init MUXes with default parents */
>> +	cpu_set_parent(dev, perf_pmux, perf_pll);
>> +	cpu_set_parent(dev, pwr_pmux, pwr_pll);
>> +	cpu_set_parent(dev, perf_smux, perf_pll_main);
>> +	cpu_set_parent(dev, pwr_smux, pwr_pll_main);
> 
> Can we use assigned-parents in DT instead?

sure, will do

> 
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_cpu_clk_msm8996_driver_probe(struct platform_device *pdev)
>> +{
>> +	int ret;
>> +	void __iomem *base;
>> +	struct resource *res;
>> +	struct clk_onecell_data *data;
>> +	struct device *dev = &pdev->dev;
>> +	struct regmap *regmap_cpu;
>> +
>> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	data->clks = devm_kcalloc(dev, 3, sizeof(struct clk *), GFP_KERNEL);
> 
> sizeof(*data->clks) or I guess hw pointers now. Also 3 != 2?

I had the cbf_clk which I removed because there where more things
to be taken care of, and did not update the 3 to 2 :/

> 
>> +	if (!data->clks)
>> +		return -ENOMEM;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(base))
>> +		return PTR_ERR(base);
>> +
>> +	regmap_cpu = devm_regmap_init_mmio(dev, base,
>> +					   &cpu_msm8996_regmap_config);
>> +	if (IS_ERR(regmap_cpu))
>> +		return PTR_ERR(regmap_cpu);
>> +
>> +	ret = register_cpu_clocks(dev, regmap_cpu);
>> +	if (ret)
>> +		return ret;
>> +
>> +	data->clks[0] = pwr_clk;
>> +	data->clks[1] = perf_clk;
>> +	data->clk_num = 2;
>> +
>> +	return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get, data);
>> +}
>> +
>> +static struct platform_driver qcom_cpu_clk_msm8996_driver = {
>> +	.probe = qcom_cpu_clk_msm8996_driver_probe,
>> +	.driver = {
>> +		.name = "qcom-cpu-clk-msm8996",
>> +		.of_match_table = match_table,
>> +	},
>> +};
>> +
>> +builtin_platform_driver(qcom_cpu_clk_msm8996_driver);
> 
> It can't be a module?

will fix

thanks for the review
Rajendra
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation