[PATCH v1 0/4] clk: iproc: Enable glitchless pll rate change

[PATCH v1 0/4] clk: iproc: Enable glitchless pll rate change

[Lori Hikichi]

This patchset enables the ability for the iproc plls to do small rate changes
without glitching the clock.

Lori Hikichi (4):
  clk: iproc: Allow iproc pll to runtime calculate vco parameters
  clk: iproc: Fix error in the pll post divider rate calculation
  clk: iproc: Allow plls to do minor rate changes without reset
  clk: iproc: Minor tidy up of iproc pll data structures

 drivers/clk/bcm/clk-cygnus.c    |  25 +---
 drivers/clk/bcm/clk-iproc-pll.c | 260 ++++++++++++++++++++++++++++------------
 drivers/clk/bcm/clk-iproc.h     |   5 +
 3 files changed, 192 insertions(+), 98 deletions(-)

-- 
1.9.1

[PATCH v1 0/4] clk: iproc: Enable glitchless pll rate change

[Lori Hikichi]

This patchset enables the ability for the iproc plls to do small rate changes
without glitching the clock.

Lori Hikichi (4):
  clk: iproc: Allow iproc pll to runtime calculate vco parameters
  clk: iproc: Fix error in the pll post divider rate calculation
  clk: iproc: Allow plls to do minor rate changes without reset
  clk: iproc: Minor tidy up of iproc pll data structures

 drivers/clk/bcm/clk-cygnus.c    |  25 +---
 drivers/clk/bcm/clk-iproc-pll.c | 260 ++++++++++++++++++++++++++++------------
 drivers/clk/bcm/clk-iproc.h     |   5 +
 3 files changed, 192 insertions(+), 98 deletions(-)

-- 
1.9.1

[PATCH v1 1/4] clk: iproc: Allow iproc pll to runtime calculate vco parameters

[Lori Hikichi]

Add the ability for the iproc pll to calculate the pll parameters at
runtime instead of only using predefined tables. This ability allows
the clock users to select from the full range of vco frequencies.
The old method of table based programming is retained so that existing
users will retain expected behavior. The flag IPROC_CLK_PLL_CALC_PARAM
will need to be set to enable the new runtime calculation method.
Currently, this is only being enabled for the audio pll.

This feature also revealed a problem with the driver using the
round_rate api.  The round_rate api does not allow for frequencies larger
than 2^31 to be returned.  Those large frequencies are interpreted as an
error code. Therefore, we are moving to the determine_rate api which
solves this problem.

Signed-off-by: Simran Rai <ssimran@broadcom.com>
Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
---
 drivers/clk/bcm/clk-cygnus.c    | 25 +++--------
 drivers/clk/bcm/clk-iproc-pll.c | 97 ++++++++++++++++++++++++++++++++++-------
 drivers/clk/bcm/clk-iproc.h     |  5 +++
 3 files changed, 92 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/bcm/clk-cygnus.c b/drivers/clk/bcm/clk-cygnus.c
index 464fdc4..b8d073e 100644
--- a/drivers/clk/bcm/clk-cygnus.c
+++ b/drivers/clk/bcm/clk-cygnus.c
@@ -269,23 +269,10 @@ static void __init cygnus_asiu_init(struct device_node *node)
 }
 CLK_OF_DECLARE(cygnus_asiu_clk, "brcm,cygnus-asiu-clk", cygnus_asiu_init);
 
-/*
- * AUDIO PLL VCO frequency parameter table
- *
- * PLL output frequency = ((ndiv_int + ndiv_frac / 2^20) *
- * (parent clock rate / pdiv)
- *
- * On Cygnus, parent is the 25MHz oscillator
- */
-static const struct iproc_pll_vco_param audiopll_vco_params[] = {
-	/* rate (Hz) ndiv_int ndiv_frac pdiv */
-	{ 1354750204UL,  54,     199238,   1 },
-	{ 1769470191UL,  70,     816639,   1 },
-};
-
 static const struct iproc_pll_ctrl audiopll = {
 	.flags = IPROC_CLK_PLL_NEEDS_SW_CFG | IPROC_CLK_PLL_HAS_NDIV_FRAC |
-		IPROC_CLK_PLL_USER_MODE_ON | IPROC_CLK_PLL_RESET_ACTIVE_LOW,
+		IPROC_CLK_PLL_USER_MODE_ON | IPROC_CLK_PLL_RESET_ACTIVE_LOW |
+		IPROC_CLK_PLL_CALC_PARAM,
 	.reset = RESET_VAL(0x5c, 0, 1),
 	.dig_filter = DF_VAL(0x48, 0, 3, 6, 4, 3, 3),
 	.sw_ctrl = SW_CTRL_VAL(0x4, 0),
@@ -300,8 +287,7 @@ static void __init cygnus_asiu_init(struct device_node *node)
 static const struct iproc_clk_ctrl audiopll_clk[] = {
 	[BCM_CYGNUS_AUDIOPLL_CH0] = {
 		.channel = BCM_CYGNUS_AUDIOPLL_CH0,
-		.flags = IPROC_CLK_AON |
-				IPROC_CLK_MCLK_DIV_BY_2,
+		.flags = IPROC_CLK_AON | IPROC_CLK_MCLK_DIV_BY_2,
 		.enable = ENABLE_VAL(0x14, 8, 10, 9),
 		.mdiv = REG_VAL(0x14, 0, 8),
 	},
@@ -321,9 +307,8 @@ static void __init cygnus_asiu_init(struct device_node *node)
 
 static void __init cygnus_audiopll_clk_init(struct device_node *node)
 {
-	iproc_pll_clk_setup(node, &audiopll, audiopll_vco_params,
-			    ARRAY_SIZE(audiopll_vco_params), audiopll_clk,
-			    ARRAY_SIZE(audiopll_clk));
+	iproc_pll_clk_setup(node, &audiopll, NULL, 0,
+			    audiopll_clk,  ARRAY_SIZE(audiopll_clk));
 }
 CLK_OF_DECLARE(cygnus_audiopll, "brcm,cygnus-audiopll",
 			cygnus_audiopll_clk_init);
diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 375d8dd..9514ecf 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -95,6 +95,39 @@ struct iproc_pll {
 
 #define to_iproc_clk(hw) container_of(hw, struct iproc_clk, hw)
 
+static int pll_calc_param(unsigned long target_rate,
+			unsigned long parent_rate,
+			struct iproc_pll_vco_param *vco_out)
+{
+	u64 ndiv_int, ndiv_frac, residual;
+
+	ndiv_int = target_rate / parent_rate;
+
+	if (!ndiv_int || (ndiv_int > 255))
+		return -EINVAL;
+
+	residual = target_rate - (ndiv_int * parent_rate);
+	residual <<= 20;
+
+	/*
+	 * Add half of the divisor so the result will be rounded to closest
+	 * instead of rounded down.
+	 */
+	residual += (parent_rate / 2);
+	ndiv_frac = div64_u64((u64)residual, (u64)parent_rate);
+
+	vco_out->ndiv_int = ndiv_int;
+	vco_out->ndiv_frac = ndiv_frac;
+	vco_out->pdiv = 1;
+
+	vco_out->rate = vco_out->ndiv_int * parent_rate;
+	residual = (u64)vco_out->ndiv_frac * (u64)parent_rate;
+	residual >>= 20;
+	vco_out->rate += residual;
+
+	return 0;
+}
+
 /*
  * Based on the target frequency, find a match from the VCO frequency parameter
  * table and return its index
@@ -252,11 +285,10 @@ static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,
 	iproc_pll_write(pll, pll->control_base, reset->offset, val);
 }
 
-static int pll_set_rate(struct iproc_clk *clk, unsigned int rate_index,
+static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 			unsigned long parent_rate)
 {
 	struct iproc_pll *pll = clk->pll;
-	const struct iproc_pll_vco_param *vco = &pll->vco_param[rate_index];
 	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
 	int ka = 0, ki, kp, ret;
 	unsigned long rate = vco->rate;
@@ -431,25 +463,50 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
 	return clk->rate;
 }
 
-static long iproc_pll_round_rate(struct clk_hw *hw, unsigned long rate,
-				 unsigned long *parent_rate)
+static int iproc_pll_determine_rate(struct clk_hw *hw,
+		struct clk_rate_request *req)
 {
-	unsigned i;
+	unsigned int  i;
 	struct iproc_clk *clk = to_iproc_clk(hw);
 	struct iproc_pll *pll = clk->pll;
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+	unsigned long  diff, best_diff;
+	unsigned int  best_idx = 0;
+	int ret;
 
-	if (rate == 0 || *parent_rate == 0 || !pll->vco_param)
+	if (req->rate == 0 || req->best_parent_rate == 0)
 		return -EINVAL;
 
+	if (ctrl->flags & IPROC_CLK_PLL_CALC_PARAM) {
+		struct iproc_pll_vco_param vco_param;
+
+		ret = pll_calc_param(req->rate, req->best_parent_rate,
+					&vco_param);
+		if (ret)
+			return ret;
+
+		req->rate = vco_param.rate;
+		return 0;
+	}
+
+	if (!pll->vco_param)
+		return -EINVAL;
+
+	best_diff = ULONG_MAX;
 	for (i = 0; i < pll->num_vco_entries; i++) {
-		if (rate <= pll->vco_param[i].rate)
+		diff = abs(req->rate - pll->vco_param[i].rate);
+		if (diff <= best_diff) {
+			best_diff = diff;
+			best_idx = i;
+		}
+		/* break now if perfect match */
+		if (diff == 0)
 			break;
 	}
 
-	if (i == pll->num_vco_entries)
-		i--;
+	req->rate = pll->vco_param[best_idx].rate;
 
-	return pll->vco_param[i].rate;
+	return 0;
 }
 
 static int iproc_pll_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -457,13 +514,23 @@ static int iproc_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct iproc_clk *clk = to_iproc_clk(hw);
 	struct iproc_pll *pll = clk->pll;
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+	struct iproc_pll_vco_param vco_param;
 	int rate_index, ret;
 
-	rate_index = pll_get_rate_index(pll, rate);
-	if (rate_index < 0)
-		return rate_index;
+	if (ctrl->flags & IPROC_CLK_PLL_CALC_PARAM) {
+		ret = pll_calc_param(rate, parent_rate, &vco_param);
+		if (ret)
+			return ret;
+	} else {
+		rate_index = pll_get_rate_index(pll, rate);
+		if (rate_index < 0)
+			return rate_index;
+
+		vco_param = pll->vco_param[rate_index];
+	}
 
-	ret = pll_set_rate(clk, rate_index, parent_rate);
+	ret = pll_set_rate(clk, &vco_param, parent_rate);
 	return ret;
 }
 
@@ -471,7 +538,7 @@ static int iproc_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 	.enable = iproc_pll_enable,
 	.disable = iproc_pll_disable,
 	.recalc_rate = iproc_pll_recalc_rate,
-	.round_rate = iproc_pll_round_rate,
+	.determine_rate = iproc_pll_determine_rate,
 	.set_rate = iproc_pll_set_rate,
 };
 
diff --git a/drivers/clk/bcm/clk-iproc.h b/drivers/clk/bcm/clk-iproc.h
index 2148b4e..a48ddd3 100644
--- a/drivers/clk/bcm/clk-iproc.h
+++ b/drivers/clk/bcm/clk-iproc.h
@@ -81,6 +81,11 @@
 #define IPROC_CLK_PLL_RESET_ACTIVE_LOW BIT(9)
 
 /*
+ * Calculate the PLL parameters are runtime, instead of using table
+ */
+#define IPROC_CLK_PLL_CALC_PARAM BIT(10)
+
+/*
  * Parameters for VCO frequency configuration
  *
  * VCO frequency =
-- 
1.9.1

[PATCH v1 1/4] clk: iproc: Allow iproc pll to runtime calculate vco parameters

[Lori Hikichi]

Add the ability for the iproc pll to calculate the pll parameters at
runtime instead of only using predefined tables. This ability allows
the clock users to select from the full range of vco frequencies.
The old method of table based programming is retained so that existing
users will retain expected behavior. The flag IPROC_CLK_PLL_CALC_PARAM
will need to be set to enable the new runtime calculation method.
Currently, this is only being enabled for the audio pll.

This feature also revealed a problem with the driver using the
round_rate api.  The round_rate api does not allow for frequencies larger
than 2^31 to be returned.  Those large frequencies are interpreted as an
error code. Therefore, we are moving to the determine_rate api which
solves this problem.

Signed-off-by: Simran Rai <ssimran@broadcom.com>
Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
---
 drivers/clk/bcm/clk-cygnus.c    | 25 +++--------
 drivers/clk/bcm/clk-iproc-pll.c | 97 ++++++++++++++++++++++++++++++++++-------
 drivers/clk/bcm/clk-iproc.h     |  5 +++
 3 files changed, 92 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/bcm/clk-cygnus.c b/drivers/clk/bcm/clk-cygnus.c
index 464fdc4..b8d073e 100644
--- a/drivers/clk/bcm/clk-cygnus.c
+++ b/drivers/clk/bcm/clk-cygnus.c
@@ -269,23 +269,10 @@ static void __init cygnus_asiu_init(struct device_node *node)
 }
 CLK_OF_DECLARE(cygnus_asiu_clk, "brcm,cygnus-asiu-clk", cygnus_asiu_init);
 
-/*
- * AUDIO PLL VCO frequency parameter table
- *
- * PLL output frequency = ((ndiv_int + ndiv_frac / 2^20) *
- * (parent clock rate / pdiv)
- *
- * On Cygnus, parent is the 25MHz oscillator
- */
-static const struct iproc_pll_vco_param audiopll_vco_params[] = {
-	/* rate (Hz) ndiv_int ndiv_frac pdiv */
-	{ 1354750204UL,  54,     199238,   1 },
-	{ 1769470191UL,  70,     816639,   1 },
-};
-
 static const struct iproc_pll_ctrl audiopll = {
 	.flags = IPROC_CLK_PLL_NEEDS_SW_CFG | IPROC_CLK_PLL_HAS_NDIV_FRAC |
-		IPROC_CLK_PLL_USER_MODE_ON | IPROC_CLK_PLL_RESET_ACTIVE_LOW,
+		IPROC_CLK_PLL_USER_MODE_ON | IPROC_CLK_PLL_RESET_ACTIVE_LOW |
+		IPROC_CLK_PLL_CALC_PARAM,
 	.reset = RESET_VAL(0x5c, 0, 1),
 	.dig_filter = DF_VAL(0x48, 0, 3, 6, 4, 3, 3),
 	.sw_ctrl = SW_CTRL_VAL(0x4, 0),
@@ -300,8 +287,7 @@ static void __init cygnus_asiu_init(struct device_node *node)
 static const struct iproc_clk_ctrl audiopll_clk[] = {
 	[BCM_CYGNUS_AUDIOPLL_CH0] = {
 		.channel = BCM_CYGNUS_AUDIOPLL_CH0,
-		.flags = IPROC_CLK_AON |
-				IPROC_CLK_MCLK_DIV_BY_2,
+		.flags = IPROC_CLK_AON | IPROC_CLK_MCLK_DIV_BY_2,
 		.enable = ENABLE_VAL(0x14, 8, 10, 9),
 		.mdiv = REG_VAL(0x14, 0, 8),
 	},
@@ -321,9 +307,8 @@ static void __init cygnus_asiu_init(struct device_node *node)
 
 static void __init cygnus_audiopll_clk_init(struct device_node *node)
 {
-	iproc_pll_clk_setup(node, &audiopll, audiopll_vco_params,
-			    ARRAY_SIZE(audiopll_vco_params), audiopll_clk,
-			    ARRAY_SIZE(audiopll_clk));
+	iproc_pll_clk_setup(node, &audiopll, NULL, 0,
+			    audiopll_clk,  ARRAY_SIZE(audiopll_clk));
 }
 CLK_OF_DECLARE(cygnus_audiopll, "brcm,cygnus-audiopll",
 			cygnus_audiopll_clk_init);
diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 375d8dd..9514ecf 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -95,6 +95,39 @@ struct iproc_pll {
 
 #define to_iproc_clk(hw) container_of(hw, struct iproc_clk, hw)
 
+static int pll_calc_param(unsigned long target_rate,
+			unsigned long parent_rate,
+			struct iproc_pll_vco_param *vco_out)
+{
+	u64 ndiv_int, ndiv_frac, residual;
+
+	ndiv_int = target_rate / parent_rate;
+
+	if (!ndiv_int || (ndiv_int > 255))
+		return -EINVAL;
+
+	residual = target_rate - (ndiv_int * parent_rate);
+	residual <<= 20;
+
+	/*
+	 * Add half of the divisor so the result will be rounded to closest
+	 * instead of rounded down.
+	 */
+	residual += (parent_rate / 2);
+	ndiv_frac = div64_u64((u64)residual, (u64)parent_rate);
+
+	vco_out->ndiv_int = ndiv_int;
+	vco_out->ndiv_frac = ndiv_frac;
+	vco_out->pdiv = 1;
+
+	vco_out->rate = vco_out->ndiv_int * parent_rate;
+	residual = (u64)vco_out->ndiv_frac * (u64)parent_rate;
+	residual >>= 20;
+	vco_out->rate += residual;
+
+	return 0;
+}
+
 /*
  * Based on the target frequency, find a match from the VCO frequency parameter
  * table and return its index
@@ -252,11 +285,10 @@ static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,
 	iproc_pll_write(pll, pll->control_base, reset->offset, val);
 }
 
-static int pll_set_rate(struct iproc_clk *clk, unsigned int rate_index,
+static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 			unsigned long parent_rate)
 {
 	struct iproc_pll *pll = clk->pll;
-	const struct iproc_pll_vco_param *vco = &pll->vco_param[rate_index];
 	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
 	int ka = 0, ki, kp, ret;
 	unsigned long rate = vco->rate;
@@ -431,25 +463,50 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
 	return clk->rate;
 }
 
-static long iproc_pll_round_rate(struct clk_hw *hw, unsigned long rate,
-				 unsigned long *parent_rate)
+static int iproc_pll_determine_rate(struct clk_hw *hw,
+		struct clk_rate_request *req)
 {
-	unsigned i;
+	unsigned int  i;
 	struct iproc_clk *clk = to_iproc_clk(hw);
 	struct iproc_pll *pll = clk->pll;
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+	unsigned long  diff, best_diff;
+	unsigned int  best_idx = 0;
+	int ret;
 
-	if (rate == 0 || *parent_rate == 0 || !pll->vco_param)
+	if (req->rate == 0 || req->best_parent_rate == 0)
 		return -EINVAL;
 
+	if (ctrl->flags & IPROC_CLK_PLL_CALC_PARAM) {
+		struct iproc_pll_vco_param vco_param;
+
+		ret = pll_calc_param(req->rate, req->best_parent_rate,
+					&vco_param);
+		if (ret)
+			return ret;
+
+		req->rate = vco_param.rate;
+		return 0;
+	}
+
+	if (!pll->vco_param)
+		return -EINVAL;
+
+	best_diff = ULONG_MAX;
 	for (i = 0; i < pll->num_vco_entries; i++) {
-		if (rate <= pll->vco_param[i].rate)
+		diff = abs(req->rate - pll->vco_param[i].rate);
+		if (diff <= best_diff) {
+			best_diff = diff;
+			best_idx = i;
+		}
+		/* break now if perfect match */
+		if (diff == 0)
 			break;
 	}
 
-	if (i == pll->num_vco_entries)
-		i--;
+	req->rate = pll->vco_param[best_idx].rate;
 
-	return pll->vco_param[i].rate;
+	return 0;
 }
 
 static int iproc_pll_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -457,13 +514,23 @@ static int iproc_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct iproc_clk *clk = to_iproc_clk(hw);
 	struct iproc_pll *pll = clk->pll;
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+	struct iproc_pll_vco_param vco_param;
 	int rate_index, ret;
 
-	rate_index = pll_get_rate_index(pll, rate);
-	if (rate_index < 0)
-		return rate_index;
+	if (ctrl->flags & IPROC_CLK_PLL_CALC_PARAM) {
+		ret = pll_calc_param(rate, parent_rate, &vco_param);
+		if (ret)
+			return ret;
+	} else {
+		rate_index = pll_get_rate_index(pll, rate);
+		if (rate_index < 0)
+			return rate_index;
+
+		vco_param = pll->vco_param[rate_index];
+	}
 
-	ret = pll_set_rate(clk, rate_index, parent_rate);
+	ret = pll_set_rate(clk, &vco_param, parent_rate);
 	return ret;
 }
 
@@ -471,7 +538,7 @@ static int iproc_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 	.enable = iproc_pll_enable,
 	.disable = iproc_pll_disable,
 	.recalc_rate = iproc_pll_recalc_rate,
-	.round_rate = iproc_pll_round_rate,
+	.determine_rate = iproc_pll_determine_rate,
 	.set_rate = iproc_pll_set_rate,
 };
 
diff --git a/drivers/clk/bcm/clk-iproc.h b/drivers/clk/bcm/clk-iproc.h
index 2148b4e..a48ddd3 100644
--- a/drivers/clk/bcm/clk-iproc.h
+++ b/drivers/clk/bcm/clk-iproc.h
@@ -81,6 +81,11 @@
 #define IPROC_CLK_PLL_RESET_ACTIVE_LOW BIT(9)
 
 /*
+ * Calculate the PLL parameters are runtime, instead of using table
+ */
+#define IPROC_CLK_PLL_CALC_PARAM BIT(10)
+
+/*
  * Parameters for VCO frequency configuration
  *
  * VCO frequency =
-- 
1.9.1

Re: [PATCH v1 1/4] clk: iproc: Allow iproc pll to runtime calculate vco parameters

[Stephen Boyd]

On 08/14, Lori Hikichi wrote:
> Add the ability for the iproc pll to calculate the pll parameters at
> runtime instead of only using predefined tables. This ability allows
> the clock users to select from the full range of vco frequencies.
> The old method of table based programming is retained so that existing
> users will retain expected behavior. The flag IPROC_CLK_PLL_CALC_PARAM
> will need to be set to enable the new runtime calculation method.
> Currently, this is only being enabled for the audio pll.
> 
> This feature also revealed a problem with the driver using the
> round_rate api.  The round_rate api does not allow for frequencies larger
> than 2^31 to be returned.  Those large frequencies are interpreted as an
> error code. Therefore, we are moving to the determine_rate api which
> solves this problem.
> 
> Signed-off-by: Simran Rai <ssimran@broadcom.com>
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> ---

Applied to clk-next

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

[PATCH v1 1/4] clk: iproc: Allow iproc pll to runtime calculate vco parameters

[Stephen Boyd]

On 08/14, Lori Hikichi wrote:
> Add the ability for the iproc pll to calculate the pll parameters at
> runtime instead of only using predefined tables. This ability allows
> the clock users to select from the full range of vco frequencies.
> The old method of table based programming is retained so that existing
> users will retain expected behavior. The flag IPROC_CLK_PLL_CALC_PARAM
> will need to be set to enable the new runtime calculation method.
> Currently, this is only being enabled for the audio pll.
> 
> This feature also revealed a problem with the driver using the
> round_rate api.  The round_rate api does not allow for frequencies larger
> than 2^31 to be returned.  Those large frequencies are interpreted as an
> error code. Therefore, we are moving to the determine_rate api which
> solves this problem.
> 
> Signed-off-by: Simran Rai <ssimran@broadcom.com>
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> ---

Applied to clk-next

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

[PATCH v1 2/4] clk: iproc: Fix error in the pll post divider rate calculation

[Lori Hikichi]

The pll post divider code was using DIV_ROUND_UP when determining the
divider value best suited to produce the target frequency.
Using DIV_ROUND_CLOSEST will give us better divider values when
the division results in a small remainder.
Also, change the post divider clock over to the determine_rate api
instead of round_rate.

Signed-off-by: Simran Rai <ssimran@broadcom.com>
Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
---
 drivers/clk/bcm/clk-iproc-pll.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 9514ecf..7df010b 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -602,25 +602,26 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,
 	return clk->rate;
 }
 
-static long iproc_clk_round_rate(struct clk_hw *hw, unsigned long rate,
-		unsigned long *parent_rate)
+static int iproc_clk_determine_rate(struct clk_hw *hw,
+		struct clk_rate_request *req)
 {
-	unsigned int div;
+	unsigned int bestdiv;
 
-	if (rate == 0 || *parent_rate == 0)
+	if (req->rate == 0)
 		return -EINVAL;
+	if (req->rate == req->best_parent_rate)
+		return 0;
 
-	if (rate == *parent_rate)
-		return *parent_rate;
+	bestdiv = DIV_ROUND_CLOSEST(req->best_parent_rate, req->rate);
+	if (bestdiv < 2)
+		req->rate = req->best_parent_rate;
 
-	div = DIV_ROUND_UP(*parent_rate, rate);
-	if (div < 2)
-		return *parent_rate;
+	if (bestdiv > 256)
+		bestdiv = 256;
 
-	if (div > 256)
-		div = 256;
+	req->rate = req->best_parent_rate / bestdiv;
 
-	return *parent_rate / div;
+	return 0;
 }
 
 static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -635,10 +636,10 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (rate == 0 || parent_rate == 0)
 		return -EINVAL;
 
+	div = DIV_ROUND_CLOSEST(parent_rate, rate);
 	if (ctrl->flags & IPROC_CLK_MCLK_DIV_BY_2)
-		div = DIV_ROUND_UP(parent_rate, rate * 2);
-	else
-		div = DIV_ROUND_UP(parent_rate, rate);
+		div /=  2;
+
 	if (div > 256)
 		return -EINVAL;
 
@@ -662,7 +663,7 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	.enable = iproc_clk_enable,
 	.disable = iproc_clk_disable,
 	.recalc_rate = iproc_clk_recalc_rate,
-	.round_rate = iproc_clk_round_rate,
+	.determine_rate = iproc_clk_determine_rate,
 	.set_rate = iproc_clk_set_rate,
 };
 
-- 
1.9.1

[PATCH v1 2/4] clk: iproc: Fix error in the pll post divider rate calculation

[Lori Hikichi]

The pll post divider code was using DIV_ROUND_UP when determining the
divider value best suited to produce the target frequency.
Using DIV_ROUND_CLOSEST will give us better divider values when
the division results in a small remainder.
Also, change the post divider clock over to the determine_rate api
instead of round_rate.

Signed-off-by: Simran Rai <ssimran@broadcom.com>
Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
---
 drivers/clk/bcm/clk-iproc-pll.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 9514ecf..7df010b 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -602,25 +602,26 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,
 	return clk->rate;
 }
 
-static long iproc_clk_round_rate(struct clk_hw *hw, unsigned long rate,
-		unsigned long *parent_rate)
+static int iproc_clk_determine_rate(struct clk_hw *hw,
+		struct clk_rate_request *req)
 {
-	unsigned int div;
+	unsigned int bestdiv;
 
-	if (rate == 0 || *parent_rate == 0)
+	if (req->rate == 0)
 		return -EINVAL;
+	if (req->rate == req->best_parent_rate)
+		return 0;
 
-	if (rate == *parent_rate)
-		return *parent_rate;
+	bestdiv = DIV_ROUND_CLOSEST(req->best_parent_rate, req->rate);
+	if (bestdiv < 2)
+		req->rate = req->best_parent_rate;
 
-	div = DIV_ROUND_UP(*parent_rate, rate);
-	if (div < 2)
-		return *parent_rate;
+	if (bestdiv > 256)
+		bestdiv = 256;
 
-	if (div > 256)
-		div = 256;
+	req->rate = req->best_parent_rate / bestdiv;
 
-	return *parent_rate / div;
+	return 0;
 }
 
 static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -635,10 +636,10 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (rate == 0 || parent_rate == 0)
 		return -EINVAL;
 
+	div = DIV_ROUND_CLOSEST(parent_rate, rate);
 	if (ctrl->flags & IPROC_CLK_MCLK_DIV_BY_2)
-		div = DIV_ROUND_UP(parent_rate, rate * 2);
-	else
-		div = DIV_ROUND_UP(parent_rate, rate);
+		div /=  2;
+
 	if (div > 256)
 		return -EINVAL;
 
@@ -662,7 +663,7 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	.enable = iproc_clk_enable,
 	.disable = iproc_clk_disable,
 	.recalc_rate = iproc_clk_recalc_rate,
-	.round_rate = iproc_clk_round_rate,
+	.determine_rate = iproc_clk_determine_rate,
 	.set_rate = iproc_clk_set_rate,
 };
 
-- 
1.9.1

Re: [PATCH v1 2/4] clk: iproc: Fix error in the pll post divider rate calculation

[Stephen Boyd]

On 08/14, Lori Hikichi wrote:
> The pll post divider code was using DIV_ROUND_UP when determining the
> divider value best suited to produce the target frequency.
> Using DIV_ROUND_CLOSEST will give us better divider values when
> the division results in a small remainder.
> Also, change the post divider clock over to the determine_rate api
> instead of round_rate.
> 
> Signed-off-by: Simran Rai <ssimran@broadcom.com>
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> ---

Applied to clk-next

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

[PATCH v1 2/4] clk: iproc: Fix error in the pll post divider rate calculation

[Stephen Boyd]

On 08/14, Lori Hikichi wrote:
> The pll post divider code was using DIV_ROUND_UP when determining the
> divider value best suited to produce the target frequency.
> Using DIV_ROUND_CLOSEST will give us better divider values when
> the division results in a small remainder.
> Also, change the post divider clock over to the determine_rate api
> instead of round_rate.
> 
> Signed-off-by: Simran Rai <ssimran@broadcom.com>
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> ---

Applied to clk-next

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

[PATCH v1 3/4] clk: iproc: Allow plls to do minor rate changes without reset

[Lori Hikichi]

From: Lori Hikichi <lhikichi@broadcom.com>

The iproc plls are capable of doing small rate changes without the
need for a full reset and re-lock procedure.  This feature will
allow for small tweaks to the PLL rate to occur smoothly.

Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
---
 drivers/clk/bcm/clk-iproc-pll.c | 47 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 7df010b..ab10819 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -285,6 +285,40 @@ static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,
 	iproc_pll_write(pll, pll->control_base, reset->offset, val);
 }
 
+/*
+ * Determines if the change to be applied to the PLL is minor (just an update
+ * or the fractional divider). If so, then we can avoid going through a
+ * disruptive reset and lock sequence.
+ */
+static bool pll_fractional_change_only(struct iproc_pll *pll,
+				       struct iproc_pll_vco_param *vco)
+{
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+	u32 val;
+	u32 ndiv_int;
+	unsigned int pdiv;
+
+	/* PLL needs to be locked */
+	val = readl(pll->status_base + ctrl->status.offset);
+	if ((val & (1 << ctrl->status.shift)) == 0)
+		return false;
+
+	val = readl(pll->control_base + ctrl->ndiv_int.offset);
+	ndiv_int = (val >> ctrl->ndiv_int.shift) &
+		bit_mask(ctrl->ndiv_int.width);
+
+	if (ndiv_int != vco->ndiv_int)
+		return false;
+
+	val = readl(pll->control_base + ctrl->pdiv.offset);
+	pdiv = (val >> ctrl->pdiv.shift) & bit_mask(ctrl->pdiv.width);
+
+	if (pdiv != vco->pdiv)
+		return false;
+
+	return true;
+}
+
 static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 			unsigned long parent_rate)
 {
@@ -333,6 +367,19 @@ static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 		return ret;
 	}
 
+	if (pll_fractional_change_only(clk->pll, vco)) {
+		/* program fractional part of NDIV */
+		if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
+			val = readl(pll->control_base + ctrl->ndiv_frac.offset);
+			val &= ~(bit_mask(ctrl->ndiv_frac.width) <<
+				 ctrl->ndiv_frac.shift);
+			val |= vco->ndiv_frac << ctrl->ndiv_frac.shift;
+			iproc_pll_write(pll, pll->control_base,
+					ctrl->ndiv_frac.offset, val);
+			return 0;
+		}
+	}
+
 	/* put PLL in reset */
 	__pll_put_in_reset(pll);
 
-- 
1.9.1

[PATCH v1 3/4] clk: iproc: Allow plls to do minor rate changes without reset

[Lori Hikichi]

From: Lori Hikichi <lhikichi@broadcom.com>

The iproc plls are capable of doing small rate changes without the
need for a full reset and re-lock procedure.  This feature will
allow for small tweaks to the PLL rate to occur smoothly.

Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
---
 drivers/clk/bcm/clk-iproc-pll.c | 47 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 7df010b..ab10819 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -285,6 +285,40 @@ static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,
 	iproc_pll_write(pll, pll->control_base, reset->offset, val);
 }
 
+/*
+ * Determines if the change to be applied to the PLL is minor (just an update
+ * or the fractional divider). If so, then we can avoid going through a
+ * disruptive reset and lock sequence.
+ */
+static bool pll_fractional_change_only(struct iproc_pll *pll,
+				       struct iproc_pll_vco_param *vco)
+{
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+	u32 val;
+	u32 ndiv_int;
+	unsigned int pdiv;
+
+	/* PLL needs to be locked */
+	val = readl(pll->status_base + ctrl->status.offset);
+	if ((val & (1 << ctrl->status.shift)) == 0)
+		return false;
+
+	val = readl(pll->control_base + ctrl->ndiv_int.offset);
+	ndiv_int = (val >> ctrl->ndiv_int.shift) &
+		bit_mask(ctrl->ndiv_int.width);
+
+	if (ndiv_int != vco->ndiv_int)
+		return false;
+
+	val = readl(pll->control_base + ctrl->pdiv.offset);
+	pdiv = (val >> ctrl->pdiv.shift) & bit_mask(ctrl->pdiv.width);
+
+	if (pdiv != vco->pdiv)
+		return false;
+
+	return true;
+}
+
 static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 			unsigned long parent_rate)
 {
@@ -333,6 +367,19 @@ static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 		return ret;
 	}
 
+	if (pll_fractional_change_only(clk->pll, vco)) {
+		/* program fractional part of NDIV */
+		if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
+			val = readl(pll->control_base + ctrl->ndiv_frac.offset);
+			val &= ~(bit_mask(ctrl->ndiv_frac.width) <<
+				 ctrl->ndiv_frac.shift);
+			val |= vco->ndiv_frac << ctrl->ndiv_frac.shift;
+			iproc_pll_write(pll, pll->control_base,
+					ctrl->ndiv_frac.offset, val);
+			return 0;
+		}
+	}
+
 	/* put PLL in reset */
 	__pll_put_in_reset(pll);
 
-- 
1.9.1

Re: [PATCH v1 3/4] clk: iproc: Allow plls to do minor rate changes without reset

[Stephen Boyd]

On 08/14, Lori Hikichi wrote:
> From: Lori Hikichi <lhikichi@broadcom.com>
> 
> The iproc plls are capable of doing small rate changes without the
> need for a full reset and re-lock procedure.  This feature will
> allow for small tweaks to the PLL rate to occur smoothly.
> 
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> ---

Applied to clk-next

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

[PATCH v1 3/4] clk: iproc: Allow plls to do minor rate changes without reset

[Stephen Boyd]

On 08/14, Lori Hikichi wrote:
> From: Lori Hikichi <lhikichi@broadcom.com>
> 
> The iproc plls are capable of doing small rate changes without the
> need for a full reset and re-lock procedure.  This feature will
> allow for small tweaks to the PLL rate to occur smoothly.
> 
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> ---

Applied to clk-next

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

[PATCH v1 4/4] clk: iproc: Minor tidy up of iproc pll data structures

[Lori Hikichi]

From: Lori Hikichi <lhikichi@broadcom.com>

There were a few fields in the iproc pll data structures that were
holding information that was not true state information.
Using stack variables is sufficient and simplifies the structure.
There are not any functional changes in this commit.

Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
---
 drivers/clk/bcm/clk-iproc-pll.c | 83 ++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 47 deletions(-)

diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index ab10819..43a58ae 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -69,16 +69,6 @@ enum vco_freq_range {
 	VCO_MAX       = 4000000000U,
 };
 
-struct iproc_pll;
-
-struct iproc_clk {
-	struct clk_hw hw;
-	const char *name;
-	struct iproc_pll *pll;
-	unsigned long rate;
-	const struct iproc_clk_ctrl *ctrl;
-};
-
 struct iproc_pll {
 	void __iomem *status_base;
 	void __iomem *control_base;
@@ -88,9 +78,12 @@ struct iproc_pll {
 	const struct iproc_pll_ctrl *ctrl;
 	const struct iproc_pll_vco_param *vco_param;
 	unsigned int num_vco_entries;
+};
 
-	struct clk_hw_onecell_data *clk_data;
-	struct iproc_clk *clks;
+struct iproc_clk {
+	struct clk_hw hw;
+	struct iproc_pll *pll;
+	const struct iproc_clk_ctrl *ctrl;
 };
 
 #define to_iproc_clk(hw) container_of(hw, struct iproc_clk, hw)
@@ -329,6 +322,7 @@ static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 	u32 val;
 	enum kp_band kp_index;
 	unsigned long ref_freq;
+	const char *clk_name = clk_hw_get_name(&clk->hw);
 
 	/*
 	 * reference frequency = parent frequency / PDIV
@@ -351,19 +345,19 @@ static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 		kp_index = KP_BAND_HIGH_HIGH;
 	} else {
 		pr_err("%s: pll: %s has invalid rate: %lu\n", __func__,
-				clk->name, rate);
+				clk_name, rate);
 		return -EINVAL;
 	}
 
 	kp = get_kp(ref_freq, kp_index);
 	if (kp < 0) {
-		pr_err("%s: pll: %s has invalid kp\n", __func__, clk->name);
+		pr_err("%s: pll: %s has invalid kp\n", __func__, clk_name);
 		return kp;
 	}
 
 	ret = __pll_enable(pll);
 	if (ret) {
-		pr_err("%s: pll: %s fails to enable\n", __func__, clk->name);
+		pr_err("%s: pll: %s fails to enable\n", __func__, clk_name);
 		return ret;
 	}
 
@@ -433,7 +427,7 @@ static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 
 	ret = pll_wait_for_lock(pll);
 	if (ret < 0) {
-		pr_err("%s: pll: %s failed to lock\n", __func__, clk->name);
+		pr_err("%s: pll: %s failed to lock\n", __func__, clk_name);
 		return ret;
 	}
 
@@ -469,16 +463,15 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
 	u32 val;
 	u64 ndiv, ndiv_int, ndiv_frac;
 	unsigned int pdiv;
+	unsigned long rate;
 
 	if (parent_rate == 0)
 		return 0;
 
 	/* PLL needs to be locked */
 	val = readl(pll->status_base + ctrl->status.offset);
-	if ((val & (1 << ctrl->status.shift)) == 0) {
-		clk->rate = 0;
+	if ((val & (1 << ctrl->status.shift)) == 0)
 		return 0;
-	}
 
 	/*
 	 * PLL output frequency =
@@ -500,14 +493,14 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
 	val = readl(pll->control_base + ctrl->pdiv.offset);
 	pdiv = (val >> ctrl->pdiv.shift) & bit_mask(ctrl->pdiv.width);
 
-	clk->rate = (ndiv * parent_rate) >> 20;
+	rate = (ndiv * parent_rate) >> 20;
 
 	if (pdiv == 0)
-		clk->rate *= 2;
+		rate *= 2;
 	else
-		clk->rate /= pdiv;
+		rate /= pdiv;
 
-	return clk->rate;
+	return rate;
 }
 
 static int iproc_pll_determine_rate(struct clk_hw *hw,
@@ -632,6 +625,7 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,
 	struct iproc_pll *pll = clk->pll;
 	u32 val;
 	unsigned int mdiv;
+	unsigned long rate;
 
 	if (parent_rate == 0)
 		return 0;
@@ -642,11 +636,11 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,
 		mdiv = 256;
 
 	if (ctrl->flags & IPROC_CLK_MCLK_DIV_BY_2)
-		clk->rate = parent_rate / (mdiv * 2);
+		rate = parent_rate / (mdiv * 2);
 	else
-		clk->rate = parent_rate / mdiv;
+		rate = parent_rate / mdiv;
 
-	return clk->rate;
+	return rate;
 }
 
 static int iproc_clk_determine_rate(struct clk_hw *hw,
@@ -698,10 +692,6 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 		val |= div << ctrl->mdiv.shift;
 	}
 	iproc_pll_write(pll, pll->control_base, ctrl->mdiv.offset, val);
-	if (ctrl->flags & IPROC_CLK_MCLK_DIV_BY_2)
-		clk->rate = parent_rate / (div * 2);
-	else
-		clk->rate = parent_rate / div;
 
 	return 0;
 }
@@ -744,6 +734,8 @@ void iproc_pll_clk_setup(struct device_node *node,
 	struct iproc_clk *iclk;
 	struct clk_init_data init;
 	const char *parent_name;
+	struct iproc_clk *iclk_array;
+	struct clk_hw_onecell_data *clk_data;
 
 	if (WARN_ON(!pll_ctrl) || WARN_ON(!clk_ctrl))
 		return;
@@ -752,14 +744,14 @@ void iproc_pll_clk_setup(struct device_node *node,
 	if (WARN_ON(!pll))
 		return;
 
-	pll->clk_data = kzalloc(sizeof(*pll->clk_data->hws) * num_clks +
-				sizeof(*pll->clk_data), GFP_KERNEL);
-	if (WARN_ON(!pll->clk_data))
+	clk_data = kzalloc(sizeof(*clk_data->hws) * num_clks +
+				sizeof(*clk_data), GFP_KERNEL);
+	if (WARN_ON(!clk_data))
 		goto err_clk_data;
-	pll->clk_data->num = num_clks;
+	clk_data->num = num_clks;
 
-	pll->clks = kcalloc(num_clks, sizeof(*pll->clks), GFP_KERNEL);
-	if (WARN_ON(!pll->clks))
+	iclk_array = kcalloc(num_clks, sizeof(struct iproc_clk), GFP_KERNEL);
+	if (WARN_ON(!iclk_array))
 		goto err_clks;
 
 	pll->control_base = of_iomap(node, 0);
@@ -789,9 +781,8 @@ void iproc_pll_clk_setup(struct device_node *node,
 	/* initialize and register the PLL itself */
 	pll->ctrl = pll_ctrl;
 
-	iclk = &pll->clks[0];
+	iclk = &iclk_array[0];
 	iclk->pll = pll;
-	iclk->name = node->name;
 
 	init.name = node->name;
 	init.ops = &iproc_pll_ops;
@@ -812,7 +803,7 @@ void iproc_pll_clk_setup(struct device_node *node,
 	if (WARN_ON(ret))
 		goto err_pll_register;
 
-	pll->clk_data->hws[0] = &iclk->hw;
+	clk_data->hws[0] = &iclk->hw;
 
 	/* now initialize and register all leaf clocks */
 	for (i = 1; i < num_clks; i++) {
@@ -826,8 +817,7 @@ void iproc_pll_clk_setup(struct device_node *node,
 		if (WARN_ON(ret))
 			goto err_clk_register;
 
-		iclk = &pll->clks[i];
-		iclk->name = clk_name;
+		iclk = &iclk_array[i];
 		iclk->pll = pll;
 		iclk->ctrl = &clk_ctrl[i];
 
@@ -842,11 +832,10 @@ void iproc_pll_clk_setup(struct device_node *node,
 		if (WARN_ON(ret))
 			goto err_clk_register;
 
-		pll->clk_data->hws[i] = &iclk->hw;
+		clk_data->hws[i] = &iclk->hw;
 	}
 
-	ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
-				     pll->clk_data);
+	ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	if (WARN_ON(ret))
 		goto err_clk_register;
 
@@ -854,7 +843,7 @@ void iproc_pll_clk_setup(struct device_node *node,
 
 err_clk_register:
 	while (--i >= 0)
-		clk_hw_unregister(pll->clk_data->hws[i]);
+		clk_hw_unregister(clk_data->hws[i]);
 
 err_pll_register:
 	if (pll->status_base != pll->control_base)
@@ -871,10 +860,10 @@ void iproc_pll_clk_setup(struct device_node *node,
 	iounmap(pll->control_base);
 
 err_pll_iomap:
-	kfree(pll->clks);
+	kfree(iclk_array);
 
 err_clks:
-	kfree(pll->clk_data);
+	kfree(clk_data);
 
 err_clk_data:
 	kfree(pll);
-- 
1.9.1

[PATCH v1 4/4] clk: iproc: Minor tidy up of iproc pll data structures

[Lori Hikichi]

From: Lori Hikichi <lhikichi@broadcom.com>

There were a few fields in the iproc pll data structures that were
holding information that was not true state information.
Using stack variables is sufficient and simplifies the structure.
There are not any functional changes in this commit.

Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
---
 drivers/clk/bcm/clk-iproc-pll.c | 83 ++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 47 deletions(-)

diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index ab10819..43a58ae 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -69,16 +69,6 @@ enum vco_freq_range {
 	VCO_MAX       = 4000000000U,
 };
 
-struct iproc_pll;
-
-struct iproc_clk {
-	struct clk_hw hw;
-	const char *name;
-	struct iproc_pll *pll;
-	unsigned long rate;
-	const struct iproc_clk_ctrl *ctrl;
-};
-
 struct iproc_pll {
 	void __iomem *status_base;
 	void __iomem *control_base;
@@ -88,9 +78,12 @@ struct iproc_pll {
 	const struct iproc_pll_ctrl *ctrl;
 	const struct iproc_pll_vco_param *vco_param;
 	unsigned int num_vco_entries;
+};
 
-	struct clk_hw_onecell_data *clk_data;
-	struct iproc_clk *clks;
+struct iproc_clk {
+	struct clk_hw hw;
+	struct iproc_pll *pll;
+	const struct iproc_clk_ctrl *ctrl;
 };
 
 #define to_iproc_clk(hw) container_of(hw, struct iproc_clk, hw)
@@ -329,6 +322,7 @@ static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 	u32 val;
 	enum kp_band kp_index;
 	unsigned long ref_freq;
+	const char *clk_name = clk_hw_get_name(&clk->hw);
 
 	/*
 	 * reference frequency = parent frequency / PDIV
@@ -351,19 +345,19 @@ static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 		kp_index = KP_BAND_HIGH_HIGH;
 	} else {
 		pr_err("%s: pll: %s has invalid rate: %lu\n", __func__,
-				clk->name, rate);
+				clk_name, rate);
 		return -EINVAL;
 	}
 
 	kp = get_kp(ref_freq, kp_index);
 	if (kp < 0) {
-		pr_err("%s: pll: %s has invalid kp\n", __func__, clk->name);
+		pr_err("%s: pll: %s has invalid kp\n", __func__, clk_name);
 		return kp;
 	}
 
 	ret = __pll_enable(pll);
 	if (ret) {
-		pr_err("%s: pll: %s fails to enable\n", __func__, clk->name);
+		pr_err("%s: pll: %s fails to enable\n", __func__, clk_name);
 		return ret;
 	}
 
@@ -433,7 +427,7 @@ static int pll_set_rate(struct iproc_clk *clk, struct iproc_pll_vco_param *vco,
 
 	ret = pll_wait_for_lock(pll);
 	if (ret < 0) {
-		pr_err("%s: pll: %s failed to lock\n", __func__, clk->name);
+		pr_err("%s: pll: %s failed to lock\n", __func__, clk_name);
 		return ret;
 	}
 
@@ -469,16 +463,15 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
 	u32 val;
 	u64 ndiv, ndiv_int, ndiv_frac;
 	unsigned int pdiv;
+	unsigned long rate;
 
 	if (parent_rate == 0)
 		return 0;
 
 	/* PLL needs to be locked */
 	val = readl(pll->status_base + ctrl->status.offset);
-	if ((val & (1 << ctrl->status.shift)) == 0) {
-		clk->rate = 0;
+	if ((val & (1 << ctrl->status.shift)) == 0)
 		return 0;
-	}
 
 	/*
 	 * PLL output frequency =
@@ -500,14 +493,14 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
 	val = readl(pll->control_base + ctrl->pdiv.offset);
 	pdiv = (val >> ctrl->pdiv.shift) & bit_mask(ctrl->pdiv.width);
 
-	clk->rate = (ndiv * parent_rate) >> 20;
+	rate = (ndiv * parent_rate) >> 20;
 
 	if (pdiv == 0)
-		clk->rate *= 2;
+		rate *= 2;
 	else
-		clk->rate /= pdiv;
+		rate /= pdiv;
 
-	return clk->rate;
+	return rate;
 }
 
 static int iproc_pll_determine_rate(struct clk_hw *hw,
@@ -632,6 +625,7 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,
 	struct iproc_pll *pll = clk->pll;
 	u32 val;
 	unsigned int mdiv;
+	unsigned long rate;
 
 	if (parent_rate == 0)
 		return 0;
@@ -642,11 +636,11 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,
 		mdiv = 256;
 
 	if (ctrl->flags & IPROC_CLK_MCLK_DIV_BY_2)
-		clk->rate = parent_rate / (mdiv * 2);
+		rate = parent_rate / (mdiv * 2);
 	else
-		clk->rate = parent_rate / mdiv;
+		rate = parent_rate / mdiv;
 
-	return clk->rate;
+	return rate;
 }
 
 static int iproc_clk_determine_rate(struct clk_hw *hw,
@@ -698,10 +692,6 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 		val |= div << ctrl->mdiv.shift;
 	}
 	iproc_pll_write(pll, pll->control_base, ctrl->mdiv.offset, val);
-	if (ctrl->flags & IPROC_CLK_MCLK_DIV_BY_2)
-		clk->rate = parent_rate / (div * 2);
-	else
-		clk->rate = parent_rate / div;
 
 	return 0;
 }
@@ -744,6 +734,8 @@ void iproc_pll_clk_setup(struct device_node *node,
 	struct iproc_clk *iclk;
 	struct clk_init_data init;
 	const char *parent_name;
+	struct iproc_clk *iclk_array;
+	struct clk_hw_onecell_data *clk_data;
 
 	if (WARN_ON(!pll_ctrl) || WARN_ON(!clk_ctrl))
 		return;
@@ -752,14 +744,14 @@ void iproc_pll_clk_setup(struct device_node *node,
 	if (WARN_ON(!pll))
 		return;
 
-	pll->clk_data = kzalloc(sizeof(*pll->clk_data->hws) * num_clks +
-				sizeof(*pll->clk_data), GFP_KERNEL);
-	if (WARN_ON(!pll->clk_data))
+	clk_data = kzalloc(sizeof(*clk_data->hws) * num_clks +
+				sizeof(*clk_data), GFP_KERNEL);
+	if (WARN_ON(!clk_data))
 		goto err_clk_data;
-	pll->clk_data->num = num_clks;
+	clk_data->num = num_clks;
 
-	pll->clks = kcalloc(num_clks, sizeof(*pll->clks), GFP_KERNEL);
-	if (WARN_ON(!pll->clks))
+	iclk_array = kcalloc(num_clks, sizeof(struct iproc_clk), GFP_KERNEL);
+	if (WARN_ON(!iclk_array))
 		goto err_clks;
 
 	pll->control_base = of_iomap(node, 0);
@@ -789,9 +781,8 @@ void iproc_pll_clk_setup(struct device_node *node,
 	/* initialize and register the PLL itself */
 	pll->ctrl = pll_ctrl;
 
-	iclk = &pll->clks[0];
+	iclk = &iclk_array[0];
 	iclk->pll = pll;
-	iclk->name = node->name;
 
 	init.name = node->name;
 	init.ops = &iproc_pll_ops;
@@ -812,7 +803,7 @@ void iproc_pll_clk_setup(struct device_node *node,
 	if (WARN_ON(ret))
 		goto err_pll_register;
 
-	pll->clk_data->hws[0] = &iclk->hw;
+	clk_data->hws[0] = &iclk->hw;
 
 	/* now initialize and register all leaf clocks */
 	for (i = 1; i < num_clks; i++) {
@@ -826,8 +817,7 @@ void iproc_pll_clk_setup(struct device_node *node,
 		if (WARN_ON(ret))
 			goto err_clk_register;
 
-		iclk = &pll->clks[i];
-		iclk->name = clk_name;
+		iclk = &iclk_array[i];
 		iclk->pll = pll;
 		iclk->ctrl = &clk_ctrl[i];
 
@@ -842,11 +832,10 @@ void iproc_pll_clk_setup(struct device_node *node,
 		if (WARN_ON(ret))
 			goto err_clk_register;
 
-		pll->clk_data->hws[i] = &iclk->hw;
+		clk_data->hws[i] = &iclk->hw;
 	}
 
-	ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
-				     pll->clk_data);
+	ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	if (WARN_ON(ret))
 		goto err_clk_register;
 
@@ -854,7 +843,7 @@ void iproc_pll_clk_setup(struct device_node *node,
 
 err_clk_register:
 	while (--i >= 0)
-		clk_hw_unregister(pll->clk_data->hws[i]);
+		clk_hw_unregister(clk_data->hws[i]);
 
 err_pll_register:
 	if (pll->status_base != pll->control_base)
@@ -871,10 +860,10 @@ void iproc_pll_clk_setup(struct device_node *node,
 	iounmap(pll->control_base);
 
 err_pll_iomap:
-	kfree(pll->clks);
+	kfree(iclk_array);
 
 err_clks:
-	kfree(pll->clk_data);
+	kfree(clk_data);
 
 err_clk_data:
 	kfree(pll);
-- 
1.9.1

Re: [PATCH v1 4/4] clk: iproc: Minor tidy up of iproc pll data structures

[Stephen Boyd]

On 08/14, Lori Hikichi wrote:
> From: Lori Hikichi <lhikichi@broadcom.com>
> 
> There were a few fields in the iproc pll data structures that were
> holding information that was not true state information.
> Using stack variables is sufficient and simplifies the structure.
> There are not any functional changes in this commit.
> 
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> ---

Applied to clk-next

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

[PATCH v1 4/4] clk: iproc: Minor tidy up of iproc pll data structures

[Stephen Boyd]

On 08/14, Lori Hikichi wrote:
> From: Lori Hikichi <lhikichi@broadcom.com>
> 
> There were a few fields in the iproc pll data structures that were
> holding information that was not true state information.
> Using stack variables is sufficient and simplifies the structure.
> There are not any functional changes in this commit.
> 
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> ---

Applied to clk-next

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