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

[Dmitry Baryshkov]

On Fri, May 29, 2026 at 02:52:39PM +0530, Imran Shaik wrote:
> 
> 
> 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.

Sure, but it doesn't match the commit message. Explain that you are
making them always-on, because...


-- 
With best wishes
Dmitry