On Tue 03 May 08:04 CDT 2022, Robert Foss wrote:
> From: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
>
> As GDSCs are turned on and off some associated clocks are momentarily
> enabled for house keeping purposes. Failure to enable these clocks seems
> to have been silently ignored in the past, but starting in SM8350 this
> failure will prevent the GDSC to turn on.
>
> At least on SM8350 this operation will enable the RCG per the
> configuration in CFG_REG. This means that the current model where the
> current configuration is written back to CF_REG immediately after
> parking the RCG doesn't work.
>
> Instead, keep track of the currently requested rate of the clock and
> upon enabling the clock reapply the configuration per the saved rate.
>
> Fixes: 7ef6f11887bd ("clk: qcom: Configure the RCGs to a safe source as needed")
> Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> Reviewed-by: Vinod Koul <vkoul@xxxxxxxxxx>
> Tested-by: Steev Klimaszewski <steev@xxxxxxxx>
> ---
This patch has been iterated since and the latest incarnation can be
found on below link. A reference to that in the cover letter would be
sufficient.
https://lore.kernel.org/linux-arm-msm/20220426212136.1543984-1-bjorn.andersson@xxxxxxxxxx/
Thanks,
Bjorn
> drivers/clk/qcom/clk-rcg.h | 2 ++
> drivers/clk/qcom/clk-rcg2.c | 32 +++++++++++++++++---------------
> 2 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index 00cea508d49e..8b41244b8dbf 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -140,6 +140,7 @@ extern const struct clk_ops clk_dyn_rcg_ops;
> * @freq_tbl: frequency table
> * @clkr: regmap clock handle
> * @cfg_off: defines the cfg register offset from the CMD_RCGR + CFG_REG
> + * @current_rate: cached rate for parked RCGs
> */
> struct clk_rcg2 {
> u32 cmd_rcgr;
> @@ -150,6 +151,7 @@ struct clk_rcg2 {
> const struct freq_tbl *freq_tbl;
> struct clk_regmap clkr;
> u8 cfg_off;
> + unsigned long current_rate;
> };
>
> #define to_clk_rcg2(_hw) container_of(to_clk_regmap(_hw), struct clk_rcg2, clkr)
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index f675fd969c4d..81fd3a2db709 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -167,6 +167,7 @@ clk_rcg2_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> {
> struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> u32 cfg, hid_div, m = 0, n = 0, mode = 0, mask;
> + unsigned long rate;
>
> regmap_read(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg), &cfg);
>
> @@ -186,7 +187,11 @@ clk_rcg2_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> hid_div = cfg >> CFG_SRC_DIV_SHIFT;
> hid_div &= mask;
>
> - return calc_rate(parent_rate, m, n, mode, hid_div);
> + rate = calc_rate(parent_rate, m, n, mode, hid_div);
> + if (!rcg->current_rate)
> + rcg->current_rate = rate;
> +
> + return rate;
> }
>
> static int _freq_tbl_determine_rate(struct clk_hw *hw, const struct freq_tbl *f,
> @@ -978,12 +983,14 @@ static int clk_rcg2_shared_set_rate(struct clk_hw *hw, unsigned long rate,
> if (!f)
> return -EINVAL;
>
> + rcg->current_rate = rate;
> +
> /*
> - * In case clock is disabled, update the CFG, M, N and D registers
> - * and don't hit the update bit of CMD register.
> + * In the case that the shared RCG is parked, current_rate will be
> + * applied as the clock is unparked again, so just return here.
> */
> if (!__clk_is_enabled(hw->clk))
> - return __clk_rcg2_configure(rcg, f);
> + return 0;
>
> return clk_rcg2_shared_force_enable_clear(hw, f);
> }
> @@ -997,8 +1004,13 @@ static int clk_rcg2_shared_set_rate_and_parent(struct clk_hw *hw,
> static int clk_rcg2_shared_enable(struct clk_hw *hw)
> {
> struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> + const struct freq_tbl *f = NULL;
> int ret;
>
> + f = qcom_find_freq(rcg->freq_tbl, rcg->current_rate);
> + if (!f)
> + return -EINVAL;
> +
> /*
> * Set the update bit because required configuration has already
> * been written in clk_rcg2_shared_set_rate()
> @@ -1007,7 +1019,7 @@ static int clk_rcg2_shared_enable(struct clk_hw *hw)
> if (ret)
> return ret;
>
> - ret = update_config(rcg);
> + ret = clk_rcg2_configure(rcg, f);
> if (ret)
> return ret;
>
> @@ -1017,13 +1029,6 @@ static int clk_rcg2_shared_enable(struct clk_hw *hw)
> static void clk_rcg2_shared_disable(struct clk_hw *hw)
> {
> struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> - u32 cfg;
> -
> - /*
> - * Store current configuration as switching to safe source would clear
> - * the SRC and DIV of CFG register
> - */
> - regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, &cfg);
>
> /*
> * Park the RCG at a safe configuration - sourced off of safe source.
> @@ -1041,9 +1046,6 @@ static void clk_rcg2_shared_disable(struct clk_hw *hw)
> update_config(rcg);
>
> clk_rcg2_clear_force_enable(hw);
> -
> - /* Write back the stored configuration corresponding to current rate */
> - regmap_write(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, cfg);
> }
>
> const struct clk_ops clk_rcg2_shared_ops = {
> --
> 2.34.1
>
