Re: [PATCH v4 08/13] clk: qcom: gpucc-qcm2290: Move to the latest common qcom_cc_probe() model

[Dmitry Baryshkov]

On Thu, Jun 04, 2026 at 10:56:14AM +0530, Imran Shaik wrote:
> Update the QCM2290 GPUCC driver to use the qcom_cc_probe() model by moving
> the critical clocks handling and PLL configurations from probe to the
> driver_data to align with the latest convention. While at it, drop the
> modelling of gpu_cc_ahb_clk and gpu_cc_cxo_aon_clk clocks and keep them

Why? Also it looks like a separate commit to me.

> enabled from probe as per the hardware requirements, and drop pm_clk
> handling as the required GCC clocks are kept always-on from GCC probe.

Separate commit.

> 
> Signed-off-by: Imran Shaik <imran.shaik@xxxxxxxxxxxxxxxx>
> ---
>  drivers/clk/qcom/gpucc-qcm2290.c | 92 +++++++++-------------------------------
>  1 file changed, 21 insertions(+), 71 deletions(-)
> 
> @@ -224,19 +210,6 @@ static struct clk_branch gpu_cc_cx_snoc_dvm_clk = {
>  	},
>  };
>  
> -static struct clk_branch gpu_cc_cxo_aon_clk = {
> -	.halt_reg = 0x1004,
> -	.halt_check = BRANCH_HALT_DELAY,
> -	.clkr = {
> -		.enable_reg = 0x1004,
> -		.enable_mask = BIT(0),
> -		.hw.init = &(struct clk_init_data){
> -			.name = "gpu_cc_cxo_aon_clk",
> -			.ops = &clk_branch2_ops,
> -		},
> -	},
> -};

This wasn't always-on beforehand. Why is being changed?

> -
>  static struct clk_branch gpu_cc_cxo_clk = {
>  	.halt_reg = 0x109c,
>  	.halt_check = BRANCH_HALT,
> @@ -318,12 +291,10 @@ static struct gdsc gpu_gx_gdsc = {
>  };
>  
>  static struct clk_regmap *gpu_cc_qcm2290_clocks[] = {
> -	[GPU_CC_AHB_CLK] = &gpu_cc_ahb_clk.clkr,
>  	[GPU_CC_CRC_AHB_CLK] = &gpu_cc_crc_ahb_clk.clkr,
>  	[GPU_CC_CX_GFX3D_CLK] = &gpu_cc_cx_gfx3d_clk.clkr,
>  	[GPU_CC_CX_GMU_CLK] = &gpu_cc_cx_gmu_clk.clkr,
>  	[GPU_CC_CX_SNOC_DVM_CLK] = &gpu_cc_cx_snoc_dvm_clk.clkr,
> -	[GPU_CC_CXO_AON_CLK] = &gpu_cc_cxo_aon_clk.clkr,
>  	[GPU_CC_CXO_CLK] = &gpu_cc_cxo_clk.clkr,
>  	[GPU_CC_GMU_CLK_SRC] = &gpu_cc_gmu_clk_src.clkr,
>  	[GPU_CC_GX_GFX3D_CLK] = &gpu_cc_gx_gfx3d_clk.clkr,
> @@ -342,6 +313,16 @@ static struct gdsc *gpu_cc_qcm2290_gdscs[] = {
>  	[GPU_GX_GDSC] = &gpu_gx_gdsc,
>  };
>  
> +static struct clk_alpha_pll *gpu_cc_qcm2290_plls[] = {
> +	&gpu_cc_pll0,
> +};
> +
> +static const u32 gpu_cc_qcm2290_critical_cbcrs[] = {
> +	0x1078, /* GPU_CC_AHB_CLK */
> +	0x1004, /* GPU_CC_CXO_AON_CLK */
> +	0x1060, /* GPU_CC_GX_CXO_CLK */
> +};
> +
>  static const struct regmap_config gpu_cc_qcm2290_regmap_config = {
>  	.reg_bits = 32,
>  	.reg_stride = 4,
> @@ -350,6 +331,12 @@ static const struct regmap_config gpu_cc_qcm2290_regmap_config = {
>  	.fast_io = true,
>  };
>  
> +static const struct qcom_cc_driver_data gpu_cc_qcm2290_driver_data = {
> +	.alpha_plls = gpu_cc_qcm2290_plls,
> +	.num_alpha_plls = ARRAY_SIZE(gpu_cc_qcm2290_plls),
> +	.clk_cbcrs = gpu_cc_qcm2290_critical_cbcrs,
> +	.num_clk_cbcrs = ARRAY_SIZE(gpu_cc_qcm2290_critical_cbcrs),
> +};
>  
>  static const struct qcom_cc_desc gpu_cc_qcm2290_desc = {
>  	.config = &gpu_cc_qcm2290_regmap_config,
> @@ -359,6 +346,7 @@ static const struct qcom_cc_desc gpu_cc_qcm2290_desc = {
>  	.num_resets = ARRAY_SIZE(gpu_cc_qcm2290_resets),
>  	.gdscs = gpu_cc_qcm2290_gdscs,
>  	.num_gdscs = ARRAY_SIZE(gpu_cc_qcm2290_gdscs),
> +	.driver_data = &gpu_cc_qcm2290_driver_data,
>  };
>  
>  static const struct of_device_id gpu_cc_qcm2290_match_table[] = {
> @@ -369,45 +357,7 @@ MODULE_DEVICE_TABLE(of, gpu_cc_qcm2290_match_table);
>  
>  static int gpu_cc_qcm2290_probe(struct platform_device *pdev)
>  {
> -	struct regmap *regmap;
> -	int ret;
> -
> -	regmap = qcom_cc_map(pdev, &gpu_cc_qcm2290_desc);
> -	if (IS_ERR(regmap))
> -		return PTR_ERR(regmap);
> -
> -	ret = devm_pm_runtime_enable(&pdev->dev);

So, it was pm_runtime-enabled beforehand. Now you've silently dropped
it.

> -	if (ret)
> -		return ret;
> -

-- 
With best wishes
Dmitry