Re: [PATCH v2 1/5] clk: qcom: gcc-qcm2290: Drop modelling of critical clocks

[Dmitry Baryshkov]

On Thu, May 28, 2026 at 03:37:02PM +0530, Imran Shaik wrote:
> Drop the modelling of critical GCC clocks on QCM2290 SoC, and keep them
> enabled from probe as per the latest convention. This helps to drop the
> pm_clk handling in QCM2290 GPUCC driver, and the same can be re-used for
> Shikra SoC.
> 
> Signed-off-by: Imran Shaik <imran.shaik@xxxxxxxxxxxxxxxx>
> ---
>  drivers/clk/qcom/gcc-qcm2290.c | 153 +++--------------------------------------
>  1 file changed, 11 insertions(+), 142 deletions(-)
> 
> @@ -2012,19 +1936,6 @@ static struct clk_branch gcc_gpu_gpll0_div_clk_src = {
>  	},
>  };
>  
> -static struct clk_branch gcc_gpu_iref_clk = {
> -	.halt_reg = 0x36100,
> -	.halt_check = BRANCH_HALT_DELAY,
> -	.clkr = {
> -		.enable_reg = 0x36100,
> -		.enable_mask = BIT(0),
> -		.hw.init = &(struct clk_init_data){
> -			.name = "gcc_gpu_iref_clk",
> -			.ops = &clk_branch2_ops,
> -		},
> -	},
> -};

This clock is not critical. Why is it being dropped?

> -
>  static struct clk_branch gcc_gpu_memnoc_gfx_clk = {
>  	.halt_reg = 0x3600c,
>  	.halt_check = BRANCH_VOTED,
> @@ -2605,21 +2500,6 @@ static struct clk_branch gcc_venus_ctl_axi_clk = {
>  	},
>  };
>  
> -static struct clk_branch gcc_video_ahb_clk = {
> -	.halt_reg = 0x17004,
> -	.halt_check = BRANCH_HALT,
> -	.hwcg_reg = 0x17004,
> -	.hwcg_bit = 1,
> -	.clkr = {
> -		.enable_reg = 0x17004,
> -		.enable_mask = BIT(0),
> -		.hw.init = &(struct clk_init_data){
> -			.name = "gcc_video_ahb_clk",
> -			.ops = &clk_branch2_ops,

This clock isn't marked as CRITICAL, why is it being dropped?

> -		},
> -	},
> -};
> -
>  static struct clk_branch gcc_video_axi0_clk = {
>  	.halt_reg = 0x1701c,
>  	.halt_check = BRANCH_HALT,
> @@ -2686,19 +2566,6 @@ static struct clk_branch gcc_video_venus_ctl_clk = {
>  	},
>  };
>  
> -static struct clk_branch gcc_video_xo_clk = {
> -	.halt_reg = 0x17024,
> -	.halt_check = BRANCH_HALT,
> -	.clkr = {
> -		.enable_reg = 0x17024,
> -		.enable_mask = BIT(0),
> -		.hw.init = &(struct clk_init_data){
> -			.name = "gcc_video_xo_clk",
> -			.ops = &clk_branch2_ops,

This clock isn't marked as CRITICAL, why is it being dropped?

> -		},
> -	},
> -};
> -
>  static struct gdsc gcc_camss_top_gdsc = {
>  	.gdscr = 0x58004,
>  	.pd = {
> @@ -2990,6 +2848,17 @@ static int gcc_qcm2290_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	/* Keep some clocks always-on */
> +	qcom_branch_set_clk_en(regmap, 0x17008); /* GCC_CAMERA_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x17028); /* GCC_CAMERA_XO_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x1700c); /* GCC_DISP_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x1702c); /* GCC_DISP_XO_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_GPU_CFG_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x36100); /* GCC_GPU_IREF_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x79004); /* GCC_SYS_NOC_CPUSS_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x17004); /* GCC_VIDEO_AHB_CLK */
> +	qcom_branch_set_clk_en(regmap, 0x17024); /* GCC_VIDEO_XO_CLK */

If you are chancing the driver, why are you not using .clk_cbcrs?

> +
>  	clk_alpha_pll_configure(&gpll10, regmap, &gpll10_config);
>  	clk_alpha_pll_configure(&gpll11, regmap, &gpll11_config);
>  	clk_alpha_pll_configure(&gpll8, regmap, &gpll8_config);
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry