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

[Imran Shaik]

On 28-05-2026 06:52 pm, Dmitry Baryshkov wrote:
> 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?

The above clocks also should have been marked as CRITICAL. Not sure why 
those are not marked in the QCM2290. Since we are updating the critical 
clock set, moving all the required clocks to the critical list in line 
with the latest conventions.

> > -		},
> > -	},
> > -};
> > -
> >   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?

Sure, will add these to .clk_cbcrs in next series.

Thanks,
Imran

> > +
> >   	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