[PATCH] clk: qcom: msm8916: Fix bimc gpu clock ops

[PATCH] clk: qcom: msm8916: Fix bimc gpu clock ops

[Georgi Djakov]

The clock bimc_gpu_clk_src is incorrectly set to use the shared rcg2
ops, which are for RCGs with child branches controlled by different
CPUs.

The result of the incorrect ops is that the GPU's PM runtime may leave
this clock set at a very low rate. Fix this issue by using the correct
rcg2 ops.

Fixes: a2e8272f3f89 ("clk: qcom: Add MSM8916 gpu clocks")
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
 drivers/clk/qcom/gcc-msm8916.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gcc-msm8916.c b/drivers/clk/qcom/gcc-msm8916.c
index 2cfe7000fc60..3410ee68d4bc 100644
--- a/drivers/clk/qcom/gcc-msm8916.c
+++ b/drivers/clk/qcom/gcc-msm8916.c
@@ -1176,7 +1176,7 @@ static struct clk_rcg2 bimc_gpu_clk_src = {
 		.parent_names = gcc_xo_gpll0_bimc,
 		.num_parents = 3,
 		.flags = CLK_GET_RATE_NOCACHE,
-		.ops = &clk_rcg2_shared_ops,
+		.ops = &clk_rcg2_ops,
 	},
 };
 

Re: [PATCH] clk: qcom: msm8916: Fix bimc gpu clock ops

[Stephen Boyd]

On 08/18, Georgi Djakov wrote:
> The clock bimc_gpu_clk_src is incorrectly set to use the shared rcg2
> ops, which are for RCGs with child branches controlled by different
> CPUs.
> 
> The result of the incorrect ops is that the GPU's PM runtime may leave
> this clock set at a very low rate. Fix this issue by using the correct
> rcg2 ops.
> 
> Fixes: a2e8272f3f89 ("clk: qcom: Add MSM8916 gpu clocks")
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---

This is the only user of clk_rcg2_shared_ops. I'm totally lost
why we added this in the first place.

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

Re: [PATCH] clk: qcom: msm8916: Fix bimc gpu clock ops

[Georgi Djakov]

On 08/24/2017 01:55 AM, Stephen Boyd wrote:
> On 08/18, Georgi Djakov wrote:
>> The clock bimc_gpu_clk_src is incorrectly set to use the shared rcg2
>> ops, which are for RCGs with child branches controlled by different
>> CPUs.
>>
>> The result of the incorrect ops is that the GPU's PM runtime may leave
>> this clock set at a very low rate. Fix this issue by using the correct
>> rcg2 ops.
>>
>> Fixes: a2e8272f3f89 ("clk: qcom: Add MSM8916 gpu clocks")
>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>> ---
> 
> This is the only user of clk_rcg2_shared_ops. I'm totally lost
> why we added this in the first place.
> 

Yes, this is the only user. It seems that the ops could be useful for
a few other SoC that are not upstream yet, but for now i am sending a
patch to remove the unused code.

Thanks,
Georgi

[PATCH] clk: qcom: Remove unused RCG ops

[Georgi Djakov]

The RCGs ops for shared branches are not used now, so remove it.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
 drivers/clk/qcom/clk-rcg.h  |  3 --
 drivers/clk/qcom/clk-rcg2.c | 79 ---------------------------------------------
 2 files changed, 82 deletions(-)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index 1b3e8d265bdb..a2495457e564 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -156,7 +156,6 @@ extern const struct clk_ops clk_dyn_rcg_ops;
  * @hid_width: number of bits in half integer divider
  * @parent_map: map from software's parent index to hardware's src_sel field
  * @freq_tbl: frequency table
- * @current_freq: last cached frequency when using branches with shared RCGs
  * @clkr: regmap clock handle
  *
  */
@@ -166,7 +165,6 @@ struct clk_rcg2 {
 	u8			hid_width;
 	const struct parent_map	*parent_map;
 	const struct freq_tbl	*freq_tbl;
-	unsigned long		current_freq;
 	struct clk_regmap	clkr;
 };
 
@@ -174,7 +172,6 @@ struct clk_rcg2 {
 
 extern const struct clk_ops clk_rcg2_ops;
 extern const struct clk_ops clk_rcg2_floor_ops;
-extern const struct clk_ops clk_rcg2_shared_ops;
 extern const struct clk_ops clk_edp_pixel_ops;
 extern const struct clk_ops clk_byte_ops;
 extern const struct clk_ops clk_byte2_ops;
diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index 1a0985ae20d2..bbeaf9c09dbb 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -358,85 +358,6 @@ const struct clk_ops clk_rcg2_floor_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_rcg2_floor_ops);
 
-static int clk_rcg2_shared_force_enable(struct clk_hw *hw, unsigned long rate)
-{
-	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
-	const char *name = clk_hw_get_name(hw);
-	int ret, count;
-
-	/* force enable RCG */
-	ret = regmap_update_bits(rcg->clkr.regmap, rcg->cmd_rcgr + CMD_REG,
-				 CMD_ROOT_EN, CMD_ROOT_EN);
-	if (ret)
-		return ret;
-
-	/* wait for RCG to turn ON */
-	for (count = 500; count > 0; count--) {
-		ret = clk_rcg2_is_enabled(hw);
-		if (ret)
-			break;
-		udelay(1);
-	}
-	if (!count)
-		pr_err("%s: RCG did not turn on\n", name);
-
-	/* set clock rate */
-	ret = __clk_rcg2_set_rate(hw, rate, CEIL);
-	if (ret)
-		return ret;
-
-	/* clear force enable RCG */
-	return regmap_update_bits(rcg->clkr.regmap, rcg->cmd_rcgr + CMD_REG,
-				 CMD_ROOT_EN, 0);
-}
-
-static int clk_rcg2_shared_set_rate(struct clk_hw *hw, unsigned long rate,
-				    unsigned long parent_rate)
-{
-	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
-
-	/* cache the rate */
-	rcg->current_freq = rate;
-
-	if (!__clk_is_enabled(hw->clk))
-		return 0;
-
-	return clk_rcg2_shared_force_enable(hw, rcg->current_freq);
-}
-
-static unsigned long
-clk_rcg2_shared_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
-{
-	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
-
-	return rcg->current_freq = clk_rcg2_recalc_rate(hw, parent_rate);
-}
-
-static int clk_rcg2_shared_enable(struct clk_hw *hw)
-{
-	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
-
-	return clk_rcg2_shared_force_enable(hw, rcg->current_freq);
-}
-
-static void clk_rcg2_shared_disable(struct clk_hw *hw)
-{
-	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
-
-	/* switch to XO, which is the lowest entry in the freq table */
-	clk_rcg2_shared_set_rate(hw, rcg->freq_tbl[0].freq, 0);
-}
-
-const struct clk_ops clk_rcg2_shared_ops = {
-	.enable = clk_rcg2_shared_enable,
-	.disable = clk_rcg2_shared_disable,
-	.get_parent = clk_rcg2_get_parent,
-	.recalc_rate = clk_rcg2_shared_recalc_rate,
-	.determine_rate = clk_rcg2_determine_rate,
-	.set_rate = clk_rcg2_shared_set_rate,
-};
-EXPORT_SYMBOL_GPL(clk_rcg2_shared_ops);
-
 struct frac_entry {
 	int num;
 	int den;

Re: [PATCH] clk: qcom: Remove unused RCG ops

[Stephen Boyd]

On 08/24, Georgi Djakov wrote:
> The RCGs ops for shared branches are not used now, so remove it.
> 
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---

Applied to clk-next

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