[v2,0/2] clk: qcom: gcc-sm8550: Fix shared clk parking breakage

Amit Pundir reported[1] that a recent commit 01a0a6cc8cfd ("clk: qcom:
Park shared RCGs upon registration") broke USB and audio on sm8550-hdk.
These two patches fix the issue by skipping the parking bit of the
shared RCGs for all clks except for the ones that were causing trouble,
i.e. the display RCGs on sc7180.

The first patch is all that's required, while the second patch can be
applied anytime to simplify the QUP clks that don't need to be parked.

Changes from v1: https://lore.kernel.org/r/20240819233628.2074654-1-swboyd@chromium.org
 * Revert most of the bad patch, except for the sc7180 display clks that still
   want to park at init
 * Re-order series

[1] https://lore.kernel.org/CAMi1Hd1KQBE4kKUdAn8E5FV+BiKzuv+8FoyWQrrTHPDoYTuhgA@mail.gmail.com/

Stephen Boyd (2):
  clk: qcom: dispcc-sc7180: Only park display clks at init
  clk: qcom: gcc-sm8550: Don't use shared clk_ops for QUPs

 drivers/clk/qcom/clk-rcg.h       |  1 +
 drivers/clk/qcom/clk-rcg2.c      | 36 ++++++++++++++++++++--
 drivers/clk/qcom/dispcc-sc7180.c |  8 ++---
 drivers/clk/qcom/gcc-sm8550.c    | 52 ++++++++++++++++----------------
 4 files changed, 64 insertions(+), 33 deletions(-)


base-commit: 8400291e289ee6b2bf9779ff1c83a291501f017b

On 28/08/2024 01:12, Stephen Boyd wrote:
> Amit Pundir reports that audio and USB-C host mode stops working on
> sm8550 if the gcc_usb30_prim_master_clk_src clk is registered and
> clk_rcg2_shared_init() parks it on XO.

Why does it change the dispcc-sc7180 in this case ?

Neil

> 
> Partially revert commit 01a0a6cc8cfd ("clk: qcom: Park shared RCGs upon
> registration") by skipping the parking bit, and make that the default
> for shared RCGs, but keep the part where we cache the config register as
> that's still necessary to figure out the true parent of the clk at
> registration time. Move the logic from clk_rcg2_shared_init() to another
> clk_ops structure for use by the display clks on sc7180 to minimize the
> impact of that commit on other qcom SoCs that can't handle the parking
> part.
> 
> Fixes: 01a0a6cc8cfd ("clk: qcom: Park shared RCGs upon registration")
> Cc: Konrad Dybcio <konradybcio@kernel.org>
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Taniya Das <quic_tdas@quicinc.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Reported-by: Amit Pundir <amit.pundir@linaro.org>
> Closes: https://lore.kernel.org/CAMi1Hd1KQBE4kKUdAn8E5FV+BiKzuv+8FoyWQrrTHPDoYTuhgA@mail.gmail.com
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/clk/qcom/clk-rcg.h       |  1 +
>   drivers/clk/qcom/clk-rcg2.c      | 36 +++++++++++++++++++++++++++++---
>   drivers/clk/qcom/dispcc-sc7180.c |  8 +++----
>   3 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index d7414361e432..5f479a29d969 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -198,6 +198,7 @@ extern const struct clk_ops clk_byte2_ops;
>   extern const struct clk_ops clk_pixel_ops;
>   extern const struct clk_ops clk_gfx3d_ops;
>   extern const struct clk_ops clk_rcg2_shared_ops;
> +extern const struct clk_ops clk_rcg2_shared_init_park_ops;
>   extern const struct clk_ops clk_dp_ops;
>   
>   struct clk_rcg_dfs_data {
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 30b19bd39d08..5f0b729d7115 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -1305,6 +1305,31 @@ clk_rcg2_shared_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>   }
>   
>   static int clk_rcg2_shared_init(struct clk_hw *hw)
> +{
> +	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> +
> +	/*
> +	 * Cache the cfg so that the parent is properly mapped at registration.
> +	 */
> +	regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, &rcg->parked_cfg);
> +
> +	return 0;
> +}
> +
> +const struct clk_ops clk_rcg2_shared_ops = {
> +	.init = clk_rcg2_shared_init,
> +	.enable = clk_rcg2_shared_enable,
> +	.disable = clk_rcg2_shared_disable,
> +	.get_parent = clk_rcg2_shared_get_parent,
> +	.set_parent = clk_rcg2_shared_set_parent,
> +	.recalc_rate = clk_rcg2_shared_recalc_rate,
> +	.determine_rate = clk_rcg2_determine_rate,
> +	.set_rate = clk_rcg2_shared_set_rate,
> +	.set_rate_and_parent = clk_rcg2_shared_set_rate_and_parent,
> +};
> +EXPORT_SYMBOL_GPL(clk_rcg2_shared_ops);
> +
> +static int clk_rcg2_shared_init_park(struct clk_hw *hw)
>   {
>   	/*
>   	 * This does a few things:
> @@ -1335,8 +1360,13 @@ static int clk_rcg2_shared_init(struct clk_hw *hw)
>   	return 0;
>   }
>   
> -const struct clk_ops clk_rcg2_shared_ops = {
> -	.init = clk_rcg2_shared_init,
> +/*
> + * Like clk_rcg2_shared_ops but also park the clk at init to avoid the parent
> + * from being shutdown, getting the clk stuck when it is turned on
> + * automatically by the GDSC.
> + */
> +const struct clk_ops clk_rcg2_shared_init_park_ops = {
> +	.init = clk_rcg2_shared_init_park,
>   	.enable = clk_rcg2_shared_enable,
>   	.disable = clk_rcg2_shared_disable,
>   	.get_parent = clk_rcg2_shared_get_parent,
> @@ -1346,7 +1376,7 @@ const struct clk_ops clk_rcg2_shared_ops = {
>   	.set_rate = clk_rcg2_shared_set_rate,
>   	.set_rate_and_parent = clk_rcg2_shared_set_rate_and_parent,
>   };
> -EXPORT_SYMBOL_GPL(clk_rcg2_shared_ops);
> +EXPORT_SYMBOL_GPL(clk_rcg2_shared_init_park_ops);
>   
>   /* Common APIs to be used for DFS based RCGR */
>   static void clk_rcg2_dfs_populate_freq(struct clk_hw *hw, unsigned int l,
> diff --git a/drivers/clk/qcom/dispcc-sc7180.c b/drivers/clk/qcom/dispcc-sc7180.c
> index 4710247be530..068af819f23e 100644
> --- a/drivers/clk/qcom/dispcc-sc7180.c
> +++ b/drivers/clk/qcom/dispcc-sc7180.c
> @@ -154,7 +154,7 @@ static struct clk_rcg2 disp_cc_mdss_ahb_clk_src = {
>   		.parent_data = disp_cc_parent_data_4,
>   		.num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
>   		.flags = CLK_SET_RATE_PARENT,
> -		.ops = &clk_rcg2_shared_ops,
> +		.ops = &clk_rcg2_shared_init_park_ops,
>   	},
>   };
>   
> @@ -263,7 +263,7 @@ static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = {
>   		.name = "disp_cc_mdss_mdp_clk_src",
>   		.parent_data = disp_cc_parent_data_3,
>   		.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> -		.ops = &clk_rcg2_shared_ops,
> +		.ops = &clk_rcg2_shared_init_park_ops,
>   	},
>   };
>   
> @@ -291,7 +291,7 @@ static struct clk_rcg2 disp_cc_mdss_rot_clk_src = {
>   		.name = "disp_cc_mdss_rot_clk_src",
>   		.parent_data = disp_cc_parent_data_3,
>   		.num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> -		.ops = &clk_rcg2_shared_ops,
> +		.ops = &clk_rcg2_shared_init_park_ops,
>   	},
>   };
>   
> @@ -305,7 +305,7 @@ static struct clk_rcg2 disp_cc_mdss_vsync_clk_src = {
>   		.name = "disp_cc_mdss_vsync_clk_src",
>   		.parent_data = disp_cc_parent_data_0,
>   		.num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> -		.ops = &clk_rcg2_shared_ops,
> +		.ops = &clk_rcg2_shared_init_park_ops,
>   	},
>   };
>

Quoting Neil Armstrong (2024-08-28 05:22:25)
> On 28/08/2024 01:12, Stephen Boyd wrote:
> > Amit Pundir reports that audio and USB-C host mode stops working on
> > sm8550 if the gcc_usb30_prim_master_clk_src clk is registered and
> > clk_rcg2_shared_init() parks it on XO.
>
> Why does it change the dispcc-sc7180 in this case ?
>

The patch that broke it affected all RCGs. Let me add that detail and
resend.