2023-07-17 15:29:52

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 00/15] Unregister critical branch clocks + some RPM

On Qualcomm SoCs, certain branch clocks either need to be always-on, or
should be if you're interested in touching some part of the hardware.

Using CLK_IS_CRITICAL for this purpose sounds like a genius idea,
however that messes with the runtime pm handling - if a clock is
marked as such, the clock controller device will never enter the
"suspended" state, leaving the associated resources online, which in
turn breaks SoC-wide suspend.

This series aims to solve that on a couple SoCs that I could test the
changes on and it sprinkles some runtime pm enablement atop these drivers.

Signed-off-by: Konrad Dybcio <[email protected]>
---
Konrad Dybcio (15):
clk: qcom: branch: Add a helper for setting the enable bit
clk: qcom: Use qcom_branch_set_clk_en()
clk: qcom: gcc-sm6375: Unregister critical clocks
clk: qcom: gcc-sm6375: Add runtime PM
clk: qcom: gpucc-sm6375: Unregister critical clocks
clk: qcom: gpucc-sm6115: Unregister critical clocks
clk: qcom: gpucc-sm6115: Add runtime PM
clk: qcom: gcc-sm6115: Unregister critical clocks
clk: qcom: gcc-sm6115: Add runtime PM
clk: qcom: gcc-qcm2290: Unregister critical clocks
clk: qcom: gcc-qcm2290: Add runtime PM
arm64: dts: qcom: sm6375: Add VDD_CX to GCC
arm64: dts: qcom: qcm2290: Add VDD_CX to GCC
arm64: dts: qcom: sm6115: Add VDD_CX to GCC
arm64: dts: qcom: sm6115: Add VDD_CX to GPU_CCC

arch/arm64/boot/dts/qcom/qcm2290.dtsi | 1 +
arch/arm64/boot/dts/qcom/sm6115.dtsi | 3 +
arch/arm64/boot/dts/qcom/sm6375.dtsi | 1 +
drivers/clk/qcom/clk-branch.h | 7 ++
drivers/clk/qcom/dispcc-qcm2290.c | 2 +-
drivers/clk/qcom/dispcc-sc7280.c | 2 +-
drivers/clk/qcom/dispcc-sc8280xp.c | 2 +-
drivers/clk/qcom/dispcc-sm6115.c | 2 +-
drivers/clk/qcom/dispcc-sm8250.c | 2 +-
drivers/clk/qcom/dispcc-sm8450.c | 2 +-
drivers/clk/qcom/dispcc-sm8550.c | 2 +-
drivers/clk/qcom/gcc-qcm2290.c | 136 ++++++++---------------------
drivers/clk/qcom/gcc-sa8775p.c | 18 ++--
drivers/clk/qcom/gcc-sc7180.c | 16 ++--
drivers/clk/qcom/gcc-sc7280.c | 14 +--
drivers/clk/qcom/gcc-sc8180x.c | 20 ++---
drivers/clk/qcom/gcc-sc8280xp.c | 18 ++--
drivers/clk/qcom/gcc-sdx55.c | 2 +-
drivers/clk/qcom/gcc-sdx65.c | 2 +-
drivers/clk/qcom/gcc-sdx75.c | 4 +-
drivers/clk/qcom/gcc-sm6115.c | 155 ++++++++--------------------------
drivers/clk/qcom/gcc-sm6375.c | 34 +++++---
drivers/clk/qcom/gcc-sm7150.c | 16 ++--
drivers/clk/qcom/gcc-sm8250.c | 12 +--
drivers/clk/qcom/gcc-sm8350.c | 14 +--
drivers/clk/qcom/gcc-sm8450.c | 14 +--
drivers/clk/qcom/gcc-sm8550.c | 14 +--
drivers/clk/qcom/gpucc-sc7280.c | 4 +-
drivers/clk/qcom/gpucc-sc8280xp.c | 4 +-
drivers/clk/qcom/gpucc-sm6115.c | 57 ++++++-------
drivers/clk/qcom/gpucc-sm6375.c | 38 ++-------
drivers/clk/qcom/gpucc-sm8550.c | 4 +-
drivers/clk/qcom/lpasscorecc-sc7180.c | 2 +-
drivers/clk/qcom/videocc-sm8250.c | 4 +-
drivers/clk/qcom/videocc-sm8350.c | 4 +-
drivers/clk/qcom/videocc-sm8450.c | 6 +-
drivers/clk/qcom/videocc-sm8550.c | 6 +-
37 files changed, 245 insertions(+), 399 deletions(-)
---
base-commit: 2205be537aeb1ca2ace998e2fefaa2be04e393e4
change-id: 20230717-topic-branch_aon_cleanup-6976c13fe71c

Best regards,
--
Konrad Dybcio <[email protected]>



2023-07-17 15:29:53

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 06/15] clk: qcom: gpucc-sm6115: Unregister critical clocks

Some clocks need to be always-on, but we don't really do anything
with them, other than calling enable() once and telling Linux they're
enabled.

Unregister them to save a couple of bytes and, perhaps more
importantly, allow for runtime suspend of the clock controller device,
as CLK_IS_CRITICAL prevents the latter.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gpucc-sm6115.c | 38 ++++++++------------------------------
1 file changed, 8 insertions(+), 30 deletions(-)

diff --git a/drivers/clk/qcom/gpucc-sm6115.c b/drivers/clk/qcom/gpucc-sm6115.c
index c84727e8352d..ac048f7973d0 100644
--- a/drivers/clk/qcom/gpucc-sm6115.c
+++ b/drivers/clk/qcom/gpucc-sm6115.c
@@ -233,20 +233,6 @@ static struct clk_rcg2 gpu_cc_gx_gfx3d_clk_src = {
},
};

-static struct clk_branch gpu_cc_ahb_clk = {
- .halt_reg = 0x1078,
- .halt_check = BRANCH_HALT_DELAY,
- .clkr = {
- .enable_reg = 0x1078,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gpu_cc_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gpu_cc_crc_ahb_clk = {
.halt_reg = 0x107c,
.halt_check = BRANCH_HALT_DELAY,
@@ -335,20 +321,6 @@ static struct clk_branch gpu_cc_cxo_clk = {
},
};

-static struct clk_branch gpu_cc_gx_cxo_clk = {
- .halt_reg = 0x1060,
- .halt_check = BRANCH_HALT_DELAY,
- .clkr = {
- .enable_reg = 0x1060,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gpu_cc_gx_cxo_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gpu_cc_gx_gfx3d_clk = {
.halt_reg = 0x1054,
.halt_check = BRANCH_HALT_SKIP,
@@ -417,7 +389,6 @@ static struct gdsc gpu_gx_gdsc = {
};

static struct clk_regmap *gpu_cc_sm6115_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,
@@ -425,7 +396,6 @@ static struct clk_regmap *gpu_cc_sm6115_clocks[] = {
[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_CXO_CLK] = &gpu_cc_gx_cxo_clk.clkr,
[GPU_CC_GX_GFX3D_CLK] = &gpu_cc_gx_gfx3d_clk.clkr,
[GPU_CC_GX_GFX3D_CLK_SRC] = &gpu_cc_gx_gfx3d_clk_src.clkr,
[GPU_CC_PLL0] = &gpu_cc_pll0.clkr,
@@ -487,6 +457,14 @@ static int gpu_cc_sm6115_probe(struct platform_device *pdev)
qcom_branch_set_force_mem_core(regmap, gpu_cc_gx_gfx3d_clk, true);
qcom_branch_set_force_periph_on(regmap, gpu_cc_gx_gfx3d_clk, true);

+ /*
+ * Keep clocks always enabled:
+ * gpu_cc_ahb_clk
+ * gpu_cc_gx_cxo_clk
+ */
+ qcom_branch_set_clk_en(regmap, 0x1078);
+ qcom_branch_set_clk_en(regmap, 0x1060);
+
return qcom_cc_really_probe(pdev, &gpu_cc_sm6115_desc, regmap);
}


--
2.41.0


2023-07-17 15:29:56

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 13/15] arm64: dts: qcom: qcm2290: Add VDD_CX to GCC

The GCC block is mainly powered by VDD_CX. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/qcm2290.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/qcm2290.dtsi b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
index d46e591e72b5..a3191e3548c1 100644
--- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
@@ -622,6 +622,7 @@ gcc: clock-controller@1400000 {
reg = <0x0 0x01400000 0x0 0x1f0000>;
clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>, <&sleep_clk>;
clock-names = "bi_tcxo", "sleep_clk";
+ power-domains = <&rpmpd QCM2290_VDDCX>;
#clock-cells = <1>;
#reset-cells = <1>;
#power-domain-cells = <1>;

--
2.41.0


2023-07-17 15:30:17

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 08/15] clk: qcom: gcc-sm6115: Unregister critical clocks

Some clocks need to be always-on, but we don't really do anything
with them, other than calling enable() once and telling Linux they're
enabled.

Unregister them to save a couple of bytes and, perhaps more
importantly, allow for runtime suspend of the clock controller device,
as CLK_IS_CRITICAL prevents the latter.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm6115.c | 133 ++++++------------------------------------
1 file changed, 18 insertions(+), 115 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm6115.c b/drivers/clk/qcom/gcc-sm6115.c
index 033e308ff865..1b6016e7ddeb 100644
--- a/drivers/clk/qcom/gcc-sm6115.c
+++ b/drivers/clk/qcom/gcc-sm6115.c
@@ -1585,36 +1585,6 @@ static struct clk_branch gcc_cam_throttle_rt_clk = {
},
};

-static struct clk_branch gcc_camera_ahb_clk = {
- .halt_reg = 0x17008,
- .halt_check = BRANCH_HALT_DELAY,
- .hwcg_reg = 0x17008,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x17008,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_camera_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
-static struct clk_branch gcc_camera_xo_clk = {
- .halt_reg = 0x17028,
- .halt_check = BRANCH_HALT,
- .clkr = {
- .enable_reg = 0x17028,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_camera_xo_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_camss_axi_clk = {
.halt_reg = 0x58044,
.halt_check = BRANCH_HALT,
@@ -2123,38 +2093,6 @@ static struct clk_branch gcc_cfg_noc_usb3_prim_axi_clk = {
},
};

-static struct clk_branch gcc_cpuss_gnoc_clk = {
- .halt_reg = 0x2b004,
- .halt_check = BRANCH_HALT_VOTED,
- .hwcg_reg = 0x2b004,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x79004,
- .enable_mask = BIT(22),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_cpuss_gnoc_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
-static struct clk_branch gcc_disp_ahb_clk = {
- .halt_reg = 0x1700c,
- .halt_check = BRANCH_HALT,
- .hwcg_reg = 0x1700c,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x1700c,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_disp_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_regmap_div gcc_disp_gpll0_clk_src = {
.reg = 0x17058,
.shift = 0,
@@ -2214,20 +2152,6 @@ static struct clk_branch gcc_disp_throttle_core_clk = {
},
};

-static struct clk_branch gcc_disp_xo_clk = {
- .halt_reg = 0x1702c,
- .halt_check = BRANCH_HALT,
- .clkr = {
- .enable_reg = 0x1702c,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_disp_xo_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_gp1_clk = {
.halt_reg = 0x4d000,
.halt_check = BRANCH_HALT,
@@ -2282,22 +2206,6 @@ static struct clk_branch gcc_gp3_clk = {
},
};

-static struct clk_branch gcc_gpu_cfg_ahb_clk = {
- .halt_reg = 0x36004,
- .halt_check = BRANCH_HALT,
- .hwcg_reg = 0x36004,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x36004,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_gpu_cfg_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_gpu_gpll0_clk_src = {
.halt_check = BRANCH_HALT_DELAY,
.clkr = {
@@ -2770,22 +2678,6 @@ static struct clk_branch gcc_sdcc2_apps_clk = {
},
};

-static struct clk_branch gcc_sys_noc_cpuss_ahb_clk = {
- .halt_reg = 0x2b06c,
- .halt_check = BRANCH_HALT_VOTED,
- .hwcg_reg = 0x2b06c,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x79004,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_sys_noc_cpuss_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_sys_noc_ufs_phy_axi_clk = {
.halt_reg = 0x45098,
.halt_check = BRANCH_HALT,
@@ -3271,8 +3163,6 @@ static struct clk_regmap *gcc_sm6115_clocks[] = {
[GCC_BOOT_ROM_AHB_CLK] = &gcc_boot_rom_ahb_clk.clkr,
[GCC_CAM_THROTTLE_NRT_CLK] = &gcc_cam_throttle_nrt_clk.clkr,
[GCC_CAM_THROTTLE_RT_CLK] = &gcc_cam_throttle_rt_clk.clkr,
- [GCC_CAMERA_AHB_CLK] = &gcc_camera_ahb_clk.clkr,
- [GCC_CAMERA_XO_CLK] = &gcc_camera_xo_clk.clkr,
[GCC_CAMSS_AXI_CLK] = &gcc_camss_axi_clk.clkr,
[GCC_CAMSS_AXI_CLK_SRC] = &gcc_camss_axi_clk_src.clkr,
[GCC_CAMSS_CAMNOC_ATB_CLK] = &gcc_camss_camnoc_atb_clk.clkr,
@@ -3321,20 +3211,16 @@ static struct clk_regmap *gcc_sm6115_clocks[] = {
[GCC_CAMSS_TOP_AHB_CLK] = &gcc_camss_top_ahb_clk.clkr,
[GCC_CAMSS_TOP_AHB_CLK_SRC] = &gcc_camss_top_ahb_clk_src.clkr,
[GCC_CFG_NOC_USB3_PRIM_AXI_CLK] = &gcc_cfg_noc_usb3_prim_axi_clk.clkr,
- [GCC_CPUSS_GNOC_CLK] = &gcc_cpuss_gnoc_clk.clkr,
- [GCC_DISP_AHB_CLK] = &gcc_disp_ahb_clk.clkr,
[GCC_DISP_GPLL0_CLK_SRC] = &gcc_disp_gpll0_clk_src.clkr,
[GCC_DISP_GPLL0_DIV_CLK_SRC] = &gcc_disp_gpll0_div_clk_src.clkr,
[GCC_DISP_HF_AXI_CLK] = &gcc_disp_hf_axi_clk.clkr,
[GCC_DISP_THROTTLE_CORE_CLK] = &gcc_disp_throttle_core_clk.clkr,
- [GCC_DISP_XO_CLK] = &gcc_disp_xo_clk.clkr,
[GCC_GP1_CLK] = &gcc_gp1_clk.clkr,
[GCC_GP1_CLK_SRC] = &gcc_gp1_clk_src.clkr,
[GCC_GP2_CLK] = &gcc_gp2_clk.clkr,
[GCC_GP2_CLK_SRC] = &gcc_gp2_clk_src.clkr,
[GCC_GP3_CLK] = &gcc_gp3_clk.clkr,
[GCC_GP3_CLK_SRC] = &gcc_gp3_clk_src.clkr,
- [GCC_GPU_CFG_AHB_CLK] = &gcc_gpu_cfg_ahb_clk.clkr,
[GCC_GPU_GPLL0_CLK_SRC] = &gcc_gpu_gpll0_clk_src.clkr,
[GCC_GPU_GPLL0_DIV_CLK_SRC] = &gcc_gpu_gpll0_div_clk_src.clkr,
[GCC_GPU_IREF_CLK] = &gcc_gpu_iref_clk.clkr,
@@ -3375,7 +3261,6 @@ static struct clk_regmap *gcc_sm6115_clocks[] = {
[GCC_SDCC2_AHB_CLK] = &gcc_sdcc2_ahb_clk.clkr,
[GCC_SDCC2_APPS_CLK] = &gcc_sdcc2_apps_clk.clkr,
[GCC_SDCC2_APPS_CLK_SRC] = &gcc_sdcc2_apps_clk_src.clkr,
- [GCC_SYS_NOC_CPUSS_AHB_CLK] = &gcc_sys_noc_cpuss_ahb_clk.clkr,
[GCC_SYS_NOC_UFS_PHY_AXI_CLK] = &gcc_sys_noc_ufs_phy_axi_clk.clkr,
[GCC_SYS_NOC_USB3_PRIM_AXI_CLK] = &gcc_sys_noc_usb3_prim_axi_clk.clkr,
[GCC_UFS_CLKREF_CLK] = &gcc_ufs_clkref_clk.clkr,
@@ -3512,6 +3397,24 @@ static int gcc_sm6115_probe(struct platform_device *pdev)
clk_alpha_pll_configure(&gpll10, regmap, &gpll10_config);
clk_alpha_pll_configure(&gpll11, regmap, &gpll11_config);

+ /*
+ * Keep the following clocks always on:
+ * GCC_CAMERA_AHB_CLK
+ * GCC_CAMERA_XO_CLK
+ * GCC_CPUSS_GNOC_CLK
+ * GCC_DISP_AHB_CLK
+ * GCC_DISP_XO_CLK
+ * GCC_GPU_CFG_AHB_CLK
+ * GCC_SYS_NOC_CPUSS_AHB_CLK
+ */
+ qcom_branch_set_clk_en(regmap, 0x17008);
+ qcom_branch_set_clk_en(regmap, 0x17028);
+ qcom_branch_set_clk_en(regmap, 0x2b004);
+ qcom_branch_set_clk_en(regmap, 0x1700c);
+ qcom_branch_set_clk_en(regmap, 0x1702c);
+ qcom_branch_set_clk_en(regmap, 0x36004);
+ qcom_branch_set_clk_en(regmap, 0x2b06c);
+
return qcom_cc_really_probe(pdev, &gcc_sm6115_desc, regmap);
}


--
2.41.0


2023-07-17 15:31:39

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 03/15] clk: qcom: gcc-sm6375: Unregister critical clocks

Some clocks need to be always-on, but we don't really do anything
with them, other than calling enable() once and telling Linux they're
enabled.

Unregister them to save a couple of bytes and, perhaps more
importantly, allow for runtime suspend of the clock controller device,
as CLK_IS_CRITICAL prevents the latter.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm6375.c | 103 ++++++------------------------------------
1 file changed, 13 insertions(+), 90 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm6375.c b/drivers/clk/qcom/gcc-sm6375.c
index e94e88bdfb91..14dafea45ac9 100644
--- a/drivers/clk/qcom/gcc-sm6375.c
+++ b/drivers/clk/qcom/gcc-sm6375.c
@@ -1742,22 +1742,6 @@ static struct clk_branch gcc_cam_throttle_rt_clk = {
},
};

-static struct clk_branch gcc_camera_ahb_clk = {
- .halt_reg = 0x17008,
- .halt_check = BRANCH_HALT_DELAY,
- .hwcg_reg = 0x17008,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x17008,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_camera_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_camss_axi_clk = {
.halt_reg = 0x58044,
.halt_check = BRANCH_HALT,
@@ -2308,22 +2292,6 @@ static struct clk_branch gcc_cfg_noc_usb3_prim_axi_clk = {
},
};

-static struct clk_branch gcc_disp_ahb_clk = {
- .halt_reg = 0x1700c,
- .halt_check = BRANCH_HALT_VOTED,
- .hwcg_reg = 0x1700c,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x1700c,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_disp_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_regmap_div gcc_disp_gpll0_clk_src = {
.reg = 0x17058,
.shift = 0,
@@ -2454,22 +2422,6 @@ static struct clk_branch gcc_gp3_clk = {
},
};

-static struct clk_branch gcc_gpu_cfg_ahb_clk = {
- .halt_reg = 0x36004,
- .halt_check = BRANCH_HALT_VOTED,
- .hwcg_reg = 0x36004,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x36004,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_gpu_cfg_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_gpu_gpll0_clk_src = {
.halt_check = BRANCH_HALT_DELAY,
.clkr = {
@@ -3093,26 +3045,6 @@ static struct clk_branch gcc_sdcc2_apps_clk = {
},
};

-static struct clk_branch gcc_sys_noc_cpuss_ahb_clk = {
- .halt_reg = 0x2b06c,
- .halt_check = BRANCH_HALT_VOTED,
- .hwcg_reg = 0x2b06c,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x79004,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_sys_noc_cpuss_ahb_clk",
- .parent_hws = (const struct clk_hw*[]) {
- &gcc_cpuss_ahb_postdiv_clk_src.clkr.hw,
- },
- .num_parents = 1,
- .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_sys_noc_ufs_phy_axi_clk = {
.halt_reg = 0x45098,
.halt_check = BRANCH_HALT,
@@ -3432,22 +3364,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_DELAY,
- .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",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_video_axi0_clk = {
.halt_reg = 0x1701c,
.halt_check = BRANCH_HALT_VOTED,
@@ -3614,7 +3530,6 @@ static struct clk_regmap *gcc_sm6375_clocks[] = {
[GCC_BOOT_ROM_AHB_CLK] = &gcc_boot_rom_ahb_clk.clkr,
[GCC_CAM_THROTTLE_NRT_CLK] = &gcc_cam_throttle_nrt_clk.clkr,
[GCC_CAM_THROTTLE_RT_CLK] = &gcc_cam_throttle_rt_clk.clkr,
- [GCC_CAMERA_AHB_CLK] = &gcc_camera_ahb_clk.clkr,
[GCC_CAMSS_AXI_CLK] = &gcc_camss_axi_clk.clkr,
[GCC_CAMSS_AXI_CLK_SRC] = &gcc_camss_axi_clk_src.clkr,
[GCC_CAMSS_CCI_0_CLK] = &gcc_camss_cci_0_clk.clkr,
@@ -3670,7 +3585,6 @@ static struct clk_regmap *gcc_sm6375_clocks[] = {
[GCC_CFG_NOC_USB3_PRIM_AXI_CLK] = &gcc_cfg_noc_usb3_prim_axi_clk.clkr,
[GCC_CPUSS_AHB_CLK_SRC] = &gcc_cpuss_ahb_clk_src.clkr,
[GCC_CPUSS_AHB_POSTDIV_CLK_SRC] = &gcc_cpuss_ahb_postdiv_clk_src.clkr,
- [GCC_DISP_AHB_CLK] = &gcc_disp_ahb_clk.clkr,
[GCC_DISP_GPLL0_CLK_SRC] = &gcc_disp_gpll0_clk_src.clkr,
[GCC_DISP_GPLL0_DIV_CLK_SRC] = &gcc_disp_gpll0_div_clk_src.clkr,
[GCC_DISP_HF_AXI_CLK] = &gcc_disp_hf_axi_clk.clkr,
@@ -3682,7 +3596,6 @@ static struct clk_regmap *gcc_sm6375_clocks[] = {
[GCC_GP2_CLK_SRC] = &gcc_gp2_clk_src.clkr,
[GCC_GP3_CLK] = &gcc_gp3_clk.clkr,
[GCC_GP3_CLK_SRC] = &gcc_gp3_clk_src.clkr,
- [GCC_GPU_CFG_AHB_CLK] = &gcc_gpu_cfg_ahb_clk.clkr,
[GCC_GPU_GPLL0_CLK_SRC] = &gcc_gpu_gpll0_clk_src.clkr,
[GCC_GPU_GPLL0_DIV_CLK_SRC] = &gcc_gpu_gpll0_div_clk_src.clkr,
[GCC_GPU_MEMNOC_GFX_CLK] = &gcc_gpu_memnoc_gfx_clk.clkr,
@@ -3738,7 +3651,6 @@ static struct clk_regmap *gcc_sm6375_clocks[] = {
[GCC_SDCC2_AHB_CLK] = &gcc_sdcc2_ahb_clk.clkr,
[GCC_SDCC2_APPS_CLK] = &gcc_sdcc2_apps_clk.clkr,
[GCC_SDCC2_APPS_CLK_SRC] = &gcc_sdcc2_apps_clk_src.clkr,
- [GCC_SYS_NOC_CPUSS_AHB_CLK] = &gcc_sys_noc_cpuss_ahb_clk.clkr,
[GCC_SYS_NOC_UFS_PHY_AXI_CLK] = &gcc_sys_noc_ufs_phy_axi_clk.clkr,
[GCC_SYS_NOC_USB3_PRIM_AXI_CLK] = &gcc_sys_noc_usb3_prim_axi_clk.clkr,
[GCC_UFS_PHY_AHB_CLK] = &gcc_ufs_phy_ahb_clk.clkr,
@@ -3765,7 +3677,6 @@ static struct clk_regmap *gcc_sm6375_clocks[] = {
[GCC_VCODEC0_AXI_CLK] = &gcc_vcodec0_axi_clk.clkr,
[GCC_VENUS_AHB_CLK] = &gcc_venus_ahb_clk.clkr,
[GCC_VENUS_CTL_AXI_CLK] = &gcc_venus_ctl_axi_clk.clkr,
- [GCC_VIDEO_AHB_CLK] = &gcc_video_ahb_clk.clkr,
[GCC_VIDEO_AXI0_CLK] = &gcc_video_axi0_clk.clkr,
[GCC_VIDEO_THROTTLE_CORE_CLK] = &gcc_video_throttle_core_clk.clkr,
[GCC_VIDEO_VCODEC0_SYS_CLK] = &gcc_video_vcodec0_sys_clk.clkr,
@@ -3883,11 +3794,23 @@ static int gcc_sm6375_probe(struct platform_device *pdev)

/*
* Keep the following clocks always on:
- * GCC_CAMERA_XO_CLK, GCC_CPUSS_GNOC_CLK, GCC_DISP_XO_CLK
+ * GCC_CAMERA_XO_CLK
+ * GCC_CPUSS_GNOC_CLK
+ * GCC_DISP_XO_CLK
+ * GCC_CAMERA_AHB_CLK
+ * GCC_DISP_AHB_CLK
+ * GCC_GPU_CFG_AHB_CLK
+ * GCC_SYS_NOC_CPUSS_AHB_CLK
+ * GCC_VIDEO_AHB_CLK
*/
qcom_branch_set_clk_en(regmap, 0x17028);
qcom_branch_set_clk_en(regmap, 0x2b004);
qcom_branch_set_clk_en(regmap, 0x1702c);
+ qcom_branch_set_clk_en(regmap, 0x17008);
+ qcom_branch_set_clk_en(regmap, 0x1700c);
+ qcom_branch_set_clk_en(regmap, 0x36004);
+ qcom_branch_set_clk_en(regmap, 0x2b06c);
+ qcom_branch_set_clk_en(regmap, 0x17004);

clk_lucid_pll_configure(&gpll10, regmap, &gpll10_config);
clk_lucid_pll_configure(&gpll11, regmap, &gpll11_config);

--
2.41.0


2023-07-17 15:32:17

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 11/15] clk: qcom: gcc-qcm2290: Add runtime PM

The GCC block on QCM2290 is mostly powered by the VDD_CX rail. We need
to ensure that it's enabled to prevent unwanted power collapse.

Enable runtime PM to keep the power flowing only when necessary.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-qcm2290.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-qcm2290.c b/drivers/clk/qcom/gcc-qcm2290.c
index 1a8acc2de921..573cb550d678 100644
--- a/drivers/clk/qcom/gcc-qcm2290.c
+++ b/drivers/clk/qcom/gcc-qcm2290.c
@@ -8,6 +8,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>

#include <dt-bindings/clock/qcom,gcc-qcm2290.h>
@@ -2882,14 +2883,26 @@ static int gcc_qcm2290_probe(struct platform_device *pdev)
struct regmap *regmap;
int ret;

+ ret = devm_pm_runtime_enable(&pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = pm_runtime_resume_and_get(&pdev->dev);
+ if (ret)
+ return ret;
+
regmap = qcom_cc_map(pdev, &gcc_qcm2290_desc);
- if (IS_ERR(regmap))
+ if (IS_ERR(regmap)) {
+ pm_runtime_put(&pdev->dev);
return PTR_ERR(regmap);
+ }

ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
ARRAY_SIZE(gcc_dfs_clocks));
- if (ret)
+ if (ret) {
+ pm_runtime_put(&pdev->dev);
return ret;
+ }

clk_alpha_pll_configure(&gpll10, regmap, &gpll10_config);
clk_alpha_pll_configure(&gpll11, regmap, &gpll11_config);
@@ -2912,7 +2925,10 @@ static int gcc_qcm2290_probe(struct platform_device *pdev)
qcom_branch_set_clk_en(regmap, 0x36004);
qcom_branch_set_clk_en(regmap, 0x2b06c);

- return qcom_cc_really_probe(pdev, &gcc_qcm2290_desc, regmap);
+ ret = qcom_cc_really_probe(pdev, &gcc_qcm2290_desc, regmap);
+ pm_runtime_put(&pdev->dev);
+
+ return ret;
}

static struct platform_driver gcc_qcm2290_driver = {

--
2.41.0


2023-07-17 15:33:53

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 14/15] arm64: dts: qcom: sm6115: Add VDD_CX to GCC

The GCC block is mainly powered by VDD_CX. Describe that.

Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sm6115.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index 839c60351240..29b5b388cd94 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -784,6 +784,7 @@ gcc: clock-controller@1400000 {
reg = <0x0 0x01400000 0x0 0x1f0000>;
clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>, <&sleep_clk>;
clock-names = "bi_tcxo", "sleep_clk";
+ power-domains = <&rpmpd SM6115_VDDCX>;
#clock-cells = <1>;
#reset-cells = <1>;
#power-domain-cells = <1>;

--
2.41.0


2023-07-17 15:34:54

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 02/15] clk: qcom: Use qcom_branch_set_clk_en()

Instead of magically poking at the bit0 of branch clocks' CBCR, use
the newly introduced helper.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/dispcc-qcm2290.c | 2 +-
drivers/clk/qcom/dispcc-sc7280.c | 2 +-
drivers/clk/qcom/dispcc-sc8280xp.c | 2 +-
drivers/clk/qcom/dispcc-sm6115.c | 2 +-
drivers/clk/qcom/dispcc-sm8250.c | 2 +-
drivers/clk/qcom/dispcc-sm8450.c | 2 +-
drivers/clk/qcom/dispcc-sm8550.c | 2 +-
drivers/clk/qcom/gcc-sa8775p.c | 18 +++++++++---------
drivers/clk/qcom/gcc-sc7180.c | 16 ++++++++--------
drivers/clk/qcom/gcc-sc7280.c | 14 +++++++-------
drivers/clk/qcom/gcc-sc8180x.c | 20 ++++++++++----------
drivers/clk/qcom/gcc-sc8280xp.c | 18 +++++++++---------
drivers/clk/qcom/gcc-sdx55.c | 2 +-
drivers/clk/qcom/gcc-sdx65.c | 2 +-
drivers/clk/qcom/gcc-sdx75.c | 4 ++--
drivers/clk/qcom/gcc-sm6375.c | 6 +++---
drivers/clk/qcom/gcc-sm7150.c | 16 ++++++++--------
drivers/clk/qcom/gcc-sm8250.c | 12 ++++++------
drivers/clk/qcom/gcc-sm8350.c | 14 +++++++-------
drivers/clk/qcom/gcc-sm8450.c | 14 +++++++-------
drivers/clk/qcom/gcc-sm8550.c | 14 +++++++-------
drivers/clk/qcom/gpucc-sc7280.c | 4 ++--
drivers/clk/qcom/gpucc-sc8280xp.c | 4 ++--
drivers/clk/qcom/gpucc-sm8550.c | 4 ++--
drivers/clk/qcom/lpasscorecc-sc7180.c | 2 +-
drivers/clk/qcom/videocc-sm8250.c | 4 ++--
drivers/clk/qcom/videocc-sm8350.c | 4 ++--
drivers/clk/qcom/videocc-sm8450.c | 6 +++---
drivers/clk/qcom/videocc-sm8550.c | 6 +++---
29 files changed, 109 insertions(+), 109 deletions(-)

diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
index 44dd5cfcc150..5f90e8c15c01 100644
--- a/drivers/clk/qcom/dispcc-qcm2290.c
+++ b/drivers/clk/qcom/dispcc-qcm2290.c
@@ -520,7 +520,7 @@ static int disp_cc_qcm2290_probe(struct platform_device *pdev)
clk_alpha_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);

/* Keep DISP_CC_XO_CLK always-ON */
- regmap_update_bits(regmap, 0x604c, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x604c);

ret = qcom_cc_really_probe(pdev, &disp_cc_qcm2290_desc, regmap);
if (ret) {
diff --git a/drivers/clk/qcom/dispcc-sc7280.c b/drivers/clk/qcom/dispcc-sc7280.c
index ad596d567f6a..975f31b51539 100644
--- a/drivers/clk/qcom/dispcc-sc7280.c
+++ b/drivers/clk/qcom/dispcc-sc7280.c
@@ -882,7 +882,7 @@ static int disp_cc_sc7280_probe(struct platform_device *pdev)
* Keep the clocks always-ON
* DISP_CC_XO_CLK
*/
- regmap_update_bits(regmap, 0x5008, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x5008);

return qcom_cc_really_probe(pdev, &disp_cc_sc7280_desc, regmap);
}
diff --git a/drivers/clk/qcom/dispcc-sc8280xp.c b/drivers/clk/qcom/dispcc-sc8280xp.c
index 167470beb369..37b8bbbe9282 100644
--- a/drivers/clk/qcom/dispcc-sc8280xp.c
+++ b/drivers/clk/qcom/dispcc-sc8280xp.c
@@ -3179,7 +3179,7 @@ static int disp_cc_sc8280xp_probe(struct platform_device *pdev)
}

/* DISP_CC_XO_CLK always-on */
- regmap_update_bits(regmap, 0x605c, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x605c);

out_pm_runtime_put:
pm_runtime_put_sync(&pdev->dev);
diff --git a/drivers/clk/qcom/dispcc-sm6115.c b/drivers/clk/qcom/dispcc-sm6115.c
index 1937edf23f21..cf88ca143bd7 100644
--- a/drivers/clk/qcom/dispcc-sm6115.c
+++ b/drivers/clk/qcom/dispcc-sm6115.c
@@ -584,7 +584,7 @@ static int disp_cc_sm6115_probe(struct platform_device *pdev)
clk_alpha_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);

/* Keep DISP_CC_XO_CLK always-ON */
- regmap_update_bits(regmap, 0x604c, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x604c);

ret = qcom_cc_really_probe(pdev, &disp_cc_sm6115_desc, regmap);
if (ret) {
diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
index e17bb8b543b5..4f2297043820 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -1366,7 +1366,7 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev)
regmap_update_bits(regmap, 0x8000, 0x10, 0x10);

/* DISP_CC_XO_CLK always-on */
- regmap_update_bits(regmap, 0x605c, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x605c);

ret = qcom_cc_really_probe(pdev, &disp_cc_sm8250_desc, regmap);

diff --git a/drivers/clk/qcom/dispcc-sm8450.c b/drivers/clk/qcom/dispcc-sm8450.c
index adbfd30bfc96..e258cd9ba87e 100644
--- a/drivers/clk/qcom/dispcc-sm8450.c
+++ b/drivers/clk/qcom/dispcc-sm8450.c
@@ -1789,7 +1789,7 @@ static int disp_cc_sm8450_probe(struct platform_device *pdev)
* Keep clocks always enabled:
* disp_cc_xo_clk
*/
- regmap_update_bits(regmap, 0xe05c, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0xe05c);

ret = qcom_cc_really_probe(pdev, &disp_cc_sm8450_desc, regmap);

diff --git a/drivers/clk/qcom/dispcc-sm8550.c b/drivers/clk/qcom/dispcc-sm8550.c
index 1e5a11081860..2bd6ca2c952f 100644
--- a/drivers/clk/qcom/dispcc-sm8550.c
+++ b/drivers/clk/qcom/dispcc-sm8550.c
@@ -1774,7 +1774,7 @@ static int disp_cc_sm8550_probe(struct platform_device *pdev)
* Keep clocks always enabled:
* disp_cc_xo_clk
*/
- regmap_update_bits(regmap, 0xe054, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0xe054);

ret = qcom_cc_really_probe(pdev, &disp_cc_sm8550_desc, regmap);

diff --git a/drivers/clk/qcom/gcc-sa8775p.c b/drivers/clk/qcom/gcc-sa8775p.c
index bb94ff367abd..9db144b7f05f 100644
--- a/drivers/clk/qcom/gcc-sa8775p.c
+++ b/drivers/clk/qcom/gcc-sa8775p.c
@@ -4748,15 +4748,15 @@ static int gcc_sa8775p_probe(struct platform_device *pdev)
* GCC_DISP1_XO_CLK, GCC_DISP_AHB_CLK, GCC_DISP_XO_CLK,
* GCC_GPU_CFG_AHB_CLK, GCC_VIDEO_AHB_CLK, GCC_VIDEO_XO_CLK.
*/
- regmap_update_bits(regmap, 0x32004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x32020, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xc7004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xc7018, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x33004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x33018, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x7d004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x34004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x34024, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x32004);
+ qcom_branch_set_clk_en(regmap, 0x32020);
+ qcom_branch_set_clk_en(regmap, 0xc7004);
+ qcom_branch_set_clk_en(regmap, 0xc7018);
+ qcom_branch_set_clk_en(regmap, 0x33004);
+ qcom_branch_set_clk_en(regmap, 0x33018);
+ qcom_branch_set_clk_en(regmap, 0x7d004);
+ qcom_branch_set_clk_en(regmap, 0x34004);
+ qcom_branch_set_clk_en(regmap, 0x34024);

return qcom_cc_really_probe(pdev, &gcc_sa8775p_desc, regmap);
}
diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
index cef3c77564cf..4f79ecb8300d 100644
--- a/drivers/clk/qcom/gcc-sc7180.c
+++ b/drivers/clk/qcom/gcc-sc7180.c
@@ -2447,14 +2447,14 @@ static int gcc_sc7180_probe(struct platform_device *pdev)
* GCC_CPUSS_GNOC_CLK, GCC_VIDEO_AHB_CLK, GCC_CAMERA_AHB_CLK,
* GCC_DISP_AHB_CLK, GCC_GPU_CFG_AHB_CLK
*/
- regmap_update_bits(regmap, 0x48004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b008, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b00c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b02c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b028, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b030, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x48004);
+ qcom_branch_set_clk_en(regmap, 0x0b004);
+ qcom_branch_set_clk_en(regmap, 0x0b008);
+ qcom_branch_set_clk_en(regmap, 0x0b00c);
+ qcom_branch_set_clk_en(regmap, 0x0b02c);
+ qcom_branch_set_clk_en(regmap, 0x0b028);
+ qcom_branch_set_clk_en(regmap, 0x0b030);
+ qcom_branch_set_clk_en(regmap, 0x71004);

ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
ARRAY_SIZE(gcc_dfs_clocks));
diff --git a/drivers/clk/qcom/gcc-sc7280.c b/drivers/clk/qcom/gcc-sc7280.c
index 1dc804154031..b23f7103d08d 100644
--- a/drivers/clk/qcom/gcc-sc7280.c
+++ b/drivers/clk/qcom/gcc-sc7280.c
@@ -3458,13 +3458,13 @@ static int gcc_sc7280_probe(struct platform_device *pdev)
* GCC_CAMERA_AHB_CLK/XO_CLK, GCC_DISP_AHB_CLK/XO_CLK
* GCC_VIDEO_AHB_CLK/XO_CLK, GCC_GPU_CFG_AHB_CLK
*/
- regmap_update_bits(regmap, 0x26004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x26028, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x27004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x2701C, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x28004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x28014, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x26004);
+ qcom_branch_set_clk_en(regmap, 0x26028);
+ qcom_branch_set_clk_en(regmap, 0x27004);
+ qcom_branch_set_clk_en(regmap, 0x2701C);
+ qcom_branch_set_clk_en(regmap, 0x28004);
+ qcom_branch_set_clk_en(regmap, 0x28014);
+ qcom_branch_set_clk_en(regmap, 0x71004);
regmap_update_bits(regmap, 0x7100C, BIT(13), BIT(13));

ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index c41b9f010585..14f09c407198 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4587,16 +4587,16 @@ static int gcc_sc8180x_probe(struct platform_device *pdev)
* GCC_CPUSS_GNOC_CLK, GCC_CPUSS_DVM_BUS_CLK, GCC_NPU_CFG_AHB_CLK and
* GCC_GPU_CFG_AHB_CLK
*/
- regmap_update_bits(regmap, 0xb004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xb008, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xb00c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xb040, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xb044, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xb048, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x48004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x48190, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x4d004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0xb004);
+ qcom_branch_set_clk_en(regmap, 0xb008);
+ qcom_branch_set_clk_en(regmap, 0xb00c);
+ qcom_branch_set_clk_en(regmap, 0xb040);
+ qcom_branch_set_clk_en(regmap, 0xb044);
+ qcom_branch_set_clk_en(regmap, 0xb048);
+ qcom_branch_set_clk_en(regmap, 0x48004);
+ qcom_branch_set_clk_en(regmap, 0x48190);
+ qcom_branch_set_clk_en(regmap, 0x4d004);
+ qcom_branch_set_clk_en(regmap, 0x71004);

/* Disable the GPLL0 active input to NPU and GPU via MISC registers */
regmap_update_bits(regmap, 0x4d110, 0x3, 0x3);
diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c
index 1fb6ffac730c..a9b3735baa4b 100644
--- a/drivers/clk/qcom/gcc-sc8280xp.c
+++ b/drivers/clk/qcom/gcc-sc8280xp.c
@@ -7549,15 +7549,15 @@ static int gcc_sc8280xp_probe(struct platform_device *pdev)
* GCC_DISP_XO_CLK, GCC_GPU_CFG_AHB_CLK, GCC_VIDEO_AHB_CLK,
* GCC_VIDEO_XO_CLK, GCC_DISP1_AHB_CLK, GCC_DISP1_XO_CLK
*/
- regmap_update_bits(regmap, 0x26004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x26020, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x27004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x27028, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x28004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x28028, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xbb004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xbb028, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x26004);
+ qcom_branch_set_clk_en(regmap, 0x26020);
+ qcom_branch_set_clk_en(regmap, 0x27004);
+ qcom_branch_set_clk_en(regmap, 0x27028);
+ qcom_branch_set_clk_en(regmap, 0x71004);
+ qcom_branch_set_clk_en(regmap, 0x28004);
+ qcom_branch_set_clk_en(regmap, 0x28028);
+ qcom_branch_set_clk_en(regmap, 0xbb004);
+ qcom_branch_set_clk_en(regmap, 0xbb028);

ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, ARRAY_SIZE(gcc_dfs_clocks));
if (ret)
diff --git a/drivers/clk/qcom/gcc-sdx55.c b/drivers/clk/qcom/gcc-sdx55.c
index d5e17122698c..b1ef6223b5ee 100644
--- a/drivers/clk/qcom/gcc-sdx55.c
+++ b/drivers/clk/qcom/gcc-sdx55.c
@@ -1616,7 +1616,7 @@ static int gcc_sdx55_probe(struct platform_device *pdev)
* of the system:
* GCC_SYS_NOC_CPUSS_AHB_CLK, GCC_CPUSS_AHB_CLK, GCC_CPUSS_GNOC_CLK
*/
- regmap_update_bits(regmap, 0x6d008, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x6d008);
regmap_update_bits(regmap, 0x6d008, BIT(21), BIT(21));
regmap_update_bits(regmap, 0x6d008, BIT(22), BIT(22));

diff --git a/drivers/clk/qcom/gcc-sdx65.c b/drivers/clk/qcom/gcc-sdx65.c
index b0c17043551d..62ea059d528d 100644
--- a/drivers/clk/qcom/gcc-sdx65.c
+++ b/drivers/clk/qcom/gcc-sdx65.c
@@ -1579,7 +1579,7 @@ static int gcc_sdx65_probe(struct platform_device *pdev)
* of the system:
* GCC_SYS_NOC_CPUSS_AHB_CLK, GCC_CPUSS_AHB_CLK, GCC_CPUSS_GNOC_CLK
*/
- regmap_update_bits(regmap, 0x6d008, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x6d008);
regmap_update_bits(regmap, 0x6d008, BIT(21), BIT(21));
regmap_update_bits(regmap, 0x6d008, BIT(22), BIT(22));

diff --git a/drivers/clk/qcom/gcc-sdx75.c b/drivers/clk/qcom/gcc-sdx75.c
index b6772abdcec5..3c838fb43ce8 100644
--- a/drivers/clk/qcom/gcc-sdx75.c
+++ b/drivers/clk/qcom/gcc-sdx75.c
@@ -2940,8 +2940,8 @@ static int gcc_sdx75_probe(struct platform_device *pdev)
* gcc_ahb_pcie_link_clk
* gcc_xo_pcie_link_clk
*/
- regmap_update_bits(regmap, 0x3e004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x3e008, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x3e004);
+ qcom_branch_set_clk_en(regmap, 0x3e008);

return qcom_cc_really_probe(pdev, &gcc_sdx75_desc, regmap);
}
diff --git a/drivers/clk/qcom/gcc-sm6375.c b/drivers/clk/qcom/gcc-sm6375.c
index 417a0fd242ec..e94e88bdfb91 100644
--- a/drivers/clk/qcom/gcc-sm6375.c
+++ b/drivers/clk/qcom/gcc-sm6375.c
@@ -3885,9 +3885,9 @@ static int gcc_sm6375_probe(struct platform_device *pdev)
* Keep the following clocks always on:
* GCC_CAMERA_XO_CLK, GCC_CPUSS_GNOC_CLK, GCC_DISP_XO_CLK
*/
- regmap_update_bits(regmap, 0x17028, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x2b004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x1702c, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x17028);
+ qcom_branch_set_clk_en(regmap, 0x2b004);
+ qcom_branch_set_clk_en(regmap, 0x1702c);

clk_lucid_pll_configure(&gpll10, regmap, &gpll10_config);
clk_lucid_pll_configure(&gpll11, regmap, &gpll11_config);
diff --git a/drivers/clk/qcom/gcc-sm7150.c b/drivers/clk/qcom/gcc-sm7150.c
index 6da87f0436d0..696cca37b48b 100644
--- a/drivers/clk/qcom/gcc-sm7150.c
+++ b/drivers/clk/qcom/gcc-sm7150.c
@@ -3008,14 +3008,14 @@ static int gcc_sm7150_probe(struct platform_device *pdev)
* GCC_DISP_AHB_CLK, GCC_CAMERA_XO_CLK, GCC_VIDEO_XO_CLK,
* GCC_DISP_XO_CLK, GCC_GPU_CFG_AHB_CLK
*/
- regmap_update_bits(regmap, 0x48004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b008, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b00c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b02c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b028, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b030, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x48004);
+ qcom_branch_set_clk_en(regmap, 0x0b004);
+ qcom_branch_set_clk_en(regmap, 0x0b008);
+ qcom_branch_set_clk_en(regmap, 0x0b00c);
+ qcom_branch_set_clk_en(regmap, 0x0b02c);
+ qcom_branch_set_clk_en(regmap, 0x0b028);
+ qcom_branch_set_clk_en(regmap, 0x0b030);
+ qcom_branch_set_clk_en(regmap, 0x71004);

ret = qcom_cc_register_rcg_dfs(regmap, gcc_sm7150_dfs_desc,
ARRAY_SIZE(gcc_sm7150_dfs_desc));
diff --git a/drivers/clk/qcom/gcc-sm8250.c b/drivers/clk/qcom/gcc-sm8250.c
index b6cf4bc88d4d..ad3dd69dd198 100644
--- a/drivers/clk/qcom/gcc-sm8250.c
+++ b/drivers/clk/qcom/gcc-sm8250.c
@@ -3648,12 +3648,12 @@ static int gcc_sm8250_probe(struct platform_device *pdev)
* GCC_CPUSS_DVM_BUS_CLK, GCC_GPU_CFG_AHB_CLK,
* GCC_SYS_NOC_CPUSS_AHB_CLK
*/
- regmap_update_bits(regmap, 0x0b004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b008, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x0b00c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x4818c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x52000, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x0b004);
+ qcom_branch_set_clk_en(regmap, 0x0b008);
+ qcom_branch_set_clk_en(regmap, 0x0b00c);
+ qcom_branch_set_clk_en(regmap, 0x4818c);
+ qcom_branch_set_clk_en(regmap, 0x71004);
+ qcom_branch_set_clk_en(regmap, 0x52000);

ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
ARRAY_SIZE(gcc_dfs_clocks));
diff --git a/drivers/clk/qcom/gcc-sm8350.c b/drivers/clk/qcom/gcc-sm8350.c
index 1385a98eb3bb..b56a7669b770 100644
--- a/drivers/clk/qcom/gcc-sm8350.c
+++ b/drivers/clk/qcom/gcc-sm8350.c
@@ -3811,13 +3811,13 @@ static int gcc_sm8350_probe(struct platform_device *pdev)
* GCC_CAMERA_AHB_CLK, GCC_CAMERA_XO_CLK, GCC_DISP_AHB_CLK, GCC_DISP_XO_CLK,
* GCC_GPU_CFG_AHB_CLK, GCC_VIDEO_AHB_CLK, GCC_VIDEO_XO_CLK
*/
- regmap_update_bits(regmap, 0x26004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x26018, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x27004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x2701c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x28004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x28020, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x26004);
+ qcom_branch_set_clk_en(regmap, 0x26018);
+ qcom_branch_set_clk_en(regmap, 0x27004);
+ qcom_branch_set_clk_en(regmap, 0x2701c);
+ qcom_branch_set_clk_en(regmap, 0x71004);
+ qcom_branch_set_clk_en(regmap, 0x28004);
+ qcom_branch_set_clk_en(regmap, 0x28020);

ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, ARRAY_SIZE(gcc_dfs_clocks));
if (ret)
diff --git a/drivers/clk/qcom/gcc-sm8450.c b/drivers/clk/qcom/gcc-sm8450.c
index 75635d40a12d..5a7a723ff5ef 100644
--- a/drivers/clk/qcom/gcc-sm8450.c
+++ b/drivers/clk/qcom/gcc-sm8450.c
@@ -3285,13 +3285,13 @@ static int gcc_sm8450_probe(struct platform_device *pdev)
* gcc_disp_xo_clk, gcc_gpu_cfg_ahb_clk, gcc_video_ahb_clk,
* gcc_video_xo_clk
*/
- regmap_update_bits(regmap, 0x36004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x36020, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x37004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x3701c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x81004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x42004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x42028, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x36004);
+ qcom_branch_set_clk_en(regmap, 0x36020);
+ qcom_branch_set_clk_en(regmap, 0x37004);
+ qcom_branch_set_clk_en(regmap, 0x3701c);
+ qcom_branch_set_clk_en(regmap, 0x81004);
+ qcom_branch_set_clk_en(regmap, 0x42004);
+ qcom_branch_set_clk_en(regmap, 0x42028);

return qcom_cc_really_probe(pdev, &gcc_sm8450_desc, regmap);
}
diff --git a/drivers/clk/qcom/gcc-sm8550.c b/drivers/clk/qcom/gcc-sm8550.c
index 277cd4f020ff..a6404421021f 100644
--- a/drivers/clk/qcom/gcc-sm8550.c
+++ b/drivers/clk/qcom/gcc-sm8550.c
@@ -3349,13 +3349,13 @@ static int gcc_sm8550_probe(struct platform_device *pdev)
* gcc_disp_xo_clk, gcc_gpu_cfg_ahb_clk, gcc_video_ahb_clk,
* gcc_video_xo_clk
*/
- regmap_update_bits(regmap, 0x26004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x26028, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x27004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x27018, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x32004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x32030, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x26004);
+ qcom_branch_set_clk_en(regmap, 0x26028);
+ qcom_branch_set_clk_en(regmap, 0x27004);
+ qcom_branch_set_clk_en(regmap, 0x27018);
+ qcom_branch_set_clk_en(regmap, 0x71004);
+ qcom_branch_set_clk_en(regmap, 0x32004);
+ qcom_branch_set_clk_en(regmap, 0x32030);

/* Clear GDSC_SLEEP_ENA_VOTE to stop votes being auto-removed in sleep. */
regmap_write(regmap, 0x52024, 0x0);
diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
index 1490cd45a654..a678c51cd75e 100644
--- a/drivers/clk/qcom/gpucc-sc7280.c
+++ b/drivers/clk/qcom/gpucc-sc7280.c
@@ -461,8 +461,8 @@ static int gpu_cc_sc7280_probe(struct platform_device *pdev)
* Keep the clocks always-ON
* GPU_CC_CB_CLK, GPUCC_CX_GMU_CLK
*/
- regmap_update_bits(regmap, 0x1170, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x1098, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x1170);
+ qcom_branch_set_clk_en(regmap, 0x1098);
regmap_update_bits(regmap, 0x1098, BIT(13), BIT(13));

return qcom_cc_really_probe(pdev, &gpu_cc_sc7280_desc, regmap);
diff --git a/drivers/clk/qcom/gpucc-sc8280xp.c b/drivers/clk/qcom/gpucc-sc8280xp.c
index 8e147ee294ee..c709365a3c57 100644
--- a/drivers/clk/qcom/gpucc-sc8280xp.c
+++ b/drivers/clk/qcom/gpucc-sc8280xp.c
@@ -448,8 +448,8 @@ static int gpu_cc_sc8280xp_probe(struct platform_device *pdev)
* Keep the clocks always-ON
* GPU_CC_CB_CLK, GPU_CC_CXO_CLK
*/
- regmap_update_bits(regmap, 0x1170, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x109c, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x1170);
+ qcom_branch_set_clk_en(regmap, 0x109c);

ret = qcom_cc_really_probe(pdev, &gpu_cc_sc8280xp_desc, regmap);
pm_runtime_put(&pdev->dev);
diff --git a/drivers/clk/qcom/gpucc-sm8550.c b/drivers/clk/qcom/gpucc-sm8550.c
index 8a2e3522af51..225807979435 100644
--- a/drivers/clk/qcom/gpucc-sm8550.c
+++ b/drivers/clk/qcom/gpucc-sm8550.c
@@ -581,8 +581,8 @@ static int gpu_cc_sm8550_probe(struct platform_device *pdev)
* gpu_cc_cxo_aon_clk
* gpu_cc_demet_clk
*/
- regmap_update_bits(regmap, 0x9004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x900c, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x9004);
+ qcom_branch_set_clk_en(regmap, 0x900c);

return qcom_cc_really_probe(pdev, &gpu_cc_sm8550_desc, regmap);
}
diff --git a/drivers/clk/qcom/lpasscorecc-sc7180.c b/drivers/clk/qcom/lpasscorecc-sc7180.c
index 010867dcc2ef..7fa2e28489fd 100644
--- a/drivers/clk/qcom/lpasscorecc-sc7180.c
+++ b/drivers/clk/qcom/lpasscorecc-sc7180.c
@@ -405,7 +405,7 @@ static int lpass_core_cc_sc7180_probe(struct platform_device *pdev)
* Keep the CLK always-ON
* LPASS_AUDIO_CORE_SYSNOC_SWAY_CORE_CLK
*/
- regmap_update_bits(regmap, 0x24000, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x24000);

/* PLL settings */
regmap_write(regmap, 0x1008, 0x20);
diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c
index ad46c4014a40..1f269025f3f8 100644
--- a/drivers/clk/qcom/videocc-sm8250.c
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -384,8 +384,8 @@ static int video_cc_sm8250_probe(struct platform_device *pdev)
clk_lucid_pll_configure(&video_pll1, regmap, &video_pll1_config);

/* Keep VIDEO_CC_AHB_CLK and VIDEO_CC_XO_CLK ALWAYS-ON */
- regmap_update_bits(regmap, 0xe58, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xeec, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0xe58);
+ qcom_branch_set_clk_en(regmap, 0xeec);

ret = qcom_cc_really_probe(pdev, &video_cc_sm8250_desc, regmap);

diff --git a/drivers/clk/qcom/videocc-sm8350.c b/drivers/clk/qcom/videocc-sm8350.c
index b148877fc73d..4e6b2c7fe61b 100644
--- a/drivers/clk/qcom/videocc-sm8350.c
+++ b/drivers/clk/qcom/videocc-sm8350.c
@@ -524,8 +524,8 @@ static int video_cc_sm8350_probe(struct platform_device *pdev)
* video_cc_ahb_clk
* video_cc_xo_clk
*/
- regmap_update_bits(regmap, 0xe58, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0xeec, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0xe58);
+ qcom_branch_set_clk_en(regmap, 0xeec);

ret = qcom_cc_really_probe(pdev, &video_cc_sm8350_desc, regmap);
pm_runtime_put(&pdev->dev);
diff --git a/drivers/clk/qcom/videocc-sm8450.c b/drivers/clk/qcom/videocc-sm8450.c
index 7d0029b8b799..deaf58c95749 100644
--- a/drivers/clk/qcom/videocc-sm8450.c
+++ b/drivers/clk/qcom/videocc-sm8450.c
@@ -428,9 +428,9 @@ static int video_cc_sm8450_probe(struct platform_device *pdev)
* video_cc_sleep_clk
* video_cc_xo_clk
*/
- regmap_update_bits(regmap, 0x80e4, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x8130, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x8114, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x80e4);
+ qcom_branch_set_clk_en(regmap, 0x8130);
+ qcom_branch_set_clk_en(regmap, 0x8114);

ret = qcom_cc_really_probe(pdev, &video_cc_sm8450_desc, regmap);

diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c
index e2400fe23e60..802bbd616b2f 100644
--- a/drivers/clk/qcom/videocc-sm8550.c
+++ b/drivers/clk/qcom/videocc-sm8550.c
@@ -435,9 +435,9 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
* video_cc_sleep_clk
* video_cc_xo_clk
*/
- regmap_update_bits(regmap, 0x80f4, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x8140, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x8124, BIT(0), BIT(0));
+ qcom_branch_set_clk_en(regmap, 0x80f4);
+ qcom_branch_set_clk_en(regmap, 0x8140);
+ qcom_branch_set_clk_en(regmap, 0x8124);

ret = qcom_cc_really_probe(pdev, &video_cc_sm8550_desc, regmap);


--
2.41.0


2023-07-17 15:36:23

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 10/15] clk: qcom: gcc-qcm2290: Unregister critical clocks

Some clocks need to be always-on, but we don't really do anything
with them, other than calling enable() once and telling Linux they're
enabled.

Unregister them to save a couple of bytes and, perhaps more
importantly, allow for runtime suspend of the clock controller device,
as CLK_IS_CRITICAL prevents the latter.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-qcm2290.c | 114 ++++++-----------------------------------
1 file changed, 16 insertions(+), 98 deletions(-)

diff --git a/drivers/clk/qcom/gcc-qcm2290.c b/drivers/clk/qcom/gcc-qcm2290.c
index 48995e50c6bd..1a8acc2de921 100644
--- a/drivers/clk/qcom/gcc-qcm2290.c
+++ b/drivers/clk/qcom/gcc-qcm2290.c
@@ -1397,36 +1397,6 @@ static struct clk_branch gcc_cam_throttle_rt_clk = {
},
};

-static struct clk_branch gcc_camera_ahb_clk = {
- .halt_reg = 0x17008,
- .halt_check = BRANCH_HALT_DELAY,
- .hwcg_reg = 0x17008,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x17008,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_camera_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
-static struct clk_branch gcc_camera_xo_clk = {
- .halt_reg = 0x17028,
- .halt_check = BRANCH_HALT,
- .clkr = {
- .enable_reg = 0x17028,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_camera_xo_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_camss_axi_clk = {
.halt_reg = 0x58044,
.halt_check = BRANCH_HALT,
@@ -1825,22 +1795,6 @@ static struct clk_branch gcc_cfg_noc_usb3_prim_axi_clk = {
},
};

-static struct clk_branch gcc_disp_ahb_clk = {
- .halt_reg = 0x1700c,
- .halt_check = BRANCH_HALT,
- .hwcg_reg = 0x1700c,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x1700c,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_disp_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_regmap_div gcc_disp_gpll0_clk_src = {
.reg = 0x17058,
.shift = 0,
@@ -1899,20 +1853,6 @@ static struct clk_branch gcc_disp_throttle_core_clk = {
},
};

-static struct clk_branch gcc_disp_xo_clk = {
- .halt_reg = 0x1702c,
- .halt_check = BRANCH_HALT,
- .clkr = {
- .enable_reg = 0x1702c,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_disp_xo_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_gp1_clk = {
.halt_reg = 0x4d000,
.halt_check = BRANCH_HALT,
@@ -1964,22 +1904,6 @@ static struct clk_branch gcc_gp3_clk = {
},
};

-static struct clk_branch gcc_gpu_cfg_ahb_clk = {
- .halt_reg = 0x36004,
- .halt_check = BRANCH_HALT,
- .hwcg_reg = 0x36004,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x36004,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_gpu_cfg_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_gpu_gpll0_clk_src = {
.halt_check = BRANCH_HALT_DELAY,
.clkr = {
@@ -2439,22 +2363,6 @@ static struct clk_branch gcc_sdcc2_apps_clk = {
},
};

-static struct clk_branch gcc_sys_noc_cpuss_ahb_clk = {
- .halt_reg = 0x2b06c,
- .halt_check = BRANCH_HALT_VOTED,
- .hwcg_reg = 0x2b06c,
- .hwcg_bit = 1,
- .clkr = {
- .enable_reg = 0x79004,
- .enable_mask = BIT(0),
- .hw.init = &(struct clk_init_data){
- .name = "gcc_sys_noc_cpuss_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gcc_sys_noc_usb3_prim_axi_clk = {
.halt_reg = 0x1a080,
.halt_check = BRANCH_HALT,
@@ -2774,8 +2682,6 @@ static struct clk_regmap *gcc_qcm2290_clocks[] = {
[GCC_BOOT_ROM_AHB_CLK] = &gcc_boot_rom_ahb_clk.clkr,
[GCC_CAM_THROTTLE_NRT_CLK] = &gcc_cam_throttle_nrt_clk.clkr,
[GCC_CAM_THROTTLE_RT_CLK] = &gcc_cam_throttle_rt_clk.clkr,
- [GCC_CAMERA_AHB_CLK] = &gcc_camera_ahb_clk.clkr,
- [GCC_CAMERA_XO_CLK] = &gcc_camera_xo_clk.clkr,
[GCC_CAMSS_AXI_CLK] = &gcc_camss_axi_clk.clkr,
[GCC_CAMSS_AXI_CLK_SRC] = &gcc_camss_axi_clk_src.clkr,
[GCC_CAMSS_CAMNOC_ATB_CLK] = &gcc_camss_camnoc_atb_clk.clkr,
@@ -2816,19 +2722,16 @@ static struct clk_regmap *gcc_qcm2290_clocks[] = {
[GCC_CAMSS_TOP_AHB_CLK] = &gcc_camss_top_ahb_clk.clkr,
[GCC_CAMSS_TOP_AHB_CLK_SRC] = &gcc_camss_top_ahb_clk_src.clkr,
[GCC_CFG_NOC_USB3_PRIM_AXI_CLK] = &gcc_cfg_noc_usb3_prim_axi_clk.clkr,
- [GCC_DISP_AHB_CLK] = &gcc_disp_ahb_clk.clkr,
[GCC_DISP_GPLL0_CLK_SRC] = &gcc_disp_gpll0_clk_src.clkr,
[GCC_DISP_GPLL0_DIV_CLK_SRC] = &gcc_disp_gpll0_div_clk_src.clkr,
[GCC_DISP_HF_AXI_CLK] = &gcc_disp_hf_axi_clk.clkr,
[GCC_DISP_THROTTLE_CORE_CLK] = &gcc_disp_throttle_core_clk.clkr,
- [GCC_DISP_XO_CLK] = &gcc_disp_xo_clk.clkr,
[GCC_GP1_CLK] = &gcc_gp1_clk.clkr,
[GCC_GP1_CLK_SRC] = &gcc_gp1_clk_src.clkr,
[GCC_GP2_CLK] = &gcc_gp2_clk.clkr,
[GCC_GP2_CLK_SRC] = &gcc_gp2_clk_src.clkr,
[GCC_GP3_CLK] = &gcc_gp3_clk.clkr,
[GCC_GP3_CLK_SRC] = &gcc_gp3_clk_src.clkr,
- [GCC_GPU_CFG_AHB_CLK] = &gcc_gpu_cfg_ahb_clk.clkr,
[GCC_GPU_GPLL0_CLK_SRC] = &gcc_gpu_gpll0_clk_src.clkr,
[GCC_GPU_GPLL0_DIV_CLK_SRC] = &gcc_gpu_gpll0_div_clk_src.clkr,
[GCC_GPU_IREF_CLK] = &gcc_gpu_iref_clk.clkr,
@@ -2869,7 +2772,6 @@ static struct clk_regmap *gcc_qcm2290_clocks[] = {
[GCC_SDCC2_AHB_CLK] = &gcc_sdcc2_ahb_clk.clkr,
[GCC_SDCC2_APPS_CLK] = &gcc_sdcc2_apps_clk.clkr,
[GCC_SDCC2_APPS_CLK_SRC] = &gcc_sdcc2_apps_clk_src.clkr,
- [GCC_SYS_NOC_CPUSS_AHB_CLK] = &gcc_sys_noc_cpuss_ahb_clk.clkr,
[GCC_SYS_NOC_USB3_PRIM_AXI_CLK] = &gcc_sys_noc_usb3_prim_axi_clk.clkr,
[GCC_USB30_PRIM_MASTER_CLK] = &gcc_usb30_prim_master_clk.clkr,
[GCC_USB30_PRIM_MASTER_CLK_SRC] = &gcc_usb30_prim_master_clk_src.clkr,
@@ -2994,6 +2896,22 @@ static int gcc_qcm2290_probe(struct platform_device *pdev)
clk_alpha_pll_configure(&gpll8, regmap, &gpll8_config);
clk_alpha_pll_configure(&gpll9, regmap, &gpll9_config);

+ /*
+ * Keep clocks always enabled:
+ * GCC_CAMERA_AHB_CLK
+ * GCC_CAMERA_XO_CLK
+ * GCC_DISP_AHB_CLK
+ * GCC_DISP_XO_CLK
+ * GCC_GPU_CFG_AHB_CLK
+ * GCC_SYS_NOC_CPUSS_AHB_CLK
+ */
+ qcom_branch_set_clk_en(regmap, 0x17008);
+ qcom_branch_set_clk_en(regmap, 0x17028);
+ qcom_branch_set_clk_en(regmap, 0x1700c);
+ qcom_branch_set_clk_en(regmap, 0x1702c);
+ qcom_branch_set_clk_en(regmap, 0x36004);
+ qcom_branch_set_clk_en(regmap, 0x2b06c);
+
return qcom_cc_really_probe(pdev, &gcc_qcm2290_desc, regmap);
}


--
2.41.0


2023-07-17 15:36:52

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 04/15] clk: qcom: gcc-sm6375: Add runtime PM

The GCC block on SM6375 is powered by the VDD_CX rail. We need to ensure
that it's enabled to prevent unwanted power collapse.

Enable runtime PM to keep the power flowing only when necessary.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm6375.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm6375.c b/drivers/clk/qcom/gcc-sm6375.c
index 14dafea45ac9..4b2de545d3f8 100644
--- a/drivers/clk/qcom/gcc-sm6375.c
+++ b/drivers/clk/qcom/gcc-sm6375.c
@@ -7,6 +7,7 @@
#include <linux/clk-provider.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>

#include <dt-bindings/clock/qcom,sm6375-gcc.h>
@@ -3784,9 +3785,19 @@ static int gcc_sm6375_probe(struct platform_device *pdev)
struct regmap *regmap;
int ret;

+ ret = devm_pm_runtime_enable(&pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = pm_runtime_resume_and_get(&pdev->dev);
+ if (ret)
+ return ret;
+
regmap = qcom_cc_map(pdev, &gcc_sm6375_desc);
- if (IS_ERR(regmap))
+ if (IS_ERR(regmap)) {
+ pm_runtime_put(&pdev->dev);
return PTR_ERR(regmap);
+ }

ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, ARRAY_SIZE(gcc_dfs_clocks));
if (ret)
@@ -3817,7 +3828,10 @@ static int gcc_sm6375_probe(struct platform_device *pdev)
clk_lucid_pll_configure(&gpll8, regmap, &gpll8_config);
clk_zonda_pll_configure(&gpll9, regmap, &gpll9_config);

- return qcom_cc_really_probe(pdev, &gcc_sm6375_desc, regmap);
+ ret = qcom_cc_really_probe(pdev, &gcc_sm6375_desc, regmap);
+ pm_runtime_put(&pdev->dev);
+
+ return ret;
}

static struct platform_driver gcc_sm6375_driver = {

--
2.41.0


2023-07-17 15:37:28

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 07/15] clk: qcom: gpucc-sm6115: Add runtime PM

The GPU_CC block on SM6115 is powered by the VDD_CX rail. We need to
ensure that it's enabled to prevent unwanted power collapse.

Enable runtime PM to keep the power flowing only when necessary.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gpucc-sm6115.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gpucc-sm6115.c b/drivers/clk/qcom/gpucc-sm6115.c
index ac048f7973d0..6fb84492d292 100644
--- a/drivers/clk/qcom/gpucc-sm6115.c
+++ b/drivers/clk/qcom/gpucc-sm6115.c
@@ -7,6 +7,7 @@
#include <linux/clk-provider.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>

#include <dt-bindings/clock/qcom,sm6115-gpucc.h>
@@ -442,10 +443,21 @@ MODULE_DEVICE_TABLE(of, gpu_cc_sm6115_match_table);
static int gpu_cc_sm6115_probe(struct platform_device *pdev)
{
struct regmap *regmap;
+ int ret;
+
+ ret = devm_pm_runtime_enable(&pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = pm_runtime_resume_and_get(&pdev->dev);
+ if (ret)
+ return ret;

regmap = qcom_cc_map(pdev, &gpu_cc_sm6115_desc);
- if (IS_ERR(regmap))
+ if (IS_ERR(regmap)) {
+ pm_runtime_put(&pdev->dev);
return PTR_ERR(regmap);
+ }

clk_alpha_pll_configure(&gpu_cc_pll0, regmap, &gpu_cc_pll0_config);
clk_alpha_pll_configure(&gpu_cc_pll1, regmap, &gpu_cc_pll1_config);
@@ -465,7 +477,10 @@ static int gpu_cc_sm6115_probe(struct platform_device *pdev)
qcom_branch_set_clk_en(regmap, 0x1078);
qcom_branch_set_clk_en(regmap, 0x1060);

- return qcom_cc_really_probe(pdev, &gpu_cc_sm6115_desc, regmap);
+ ret = qcom_cc_really_probe(pdev, &gpu_cc_sm6115_desc, regmap);
+ pm_runtime_put(&pdev->dev);
+
+ return ret;
}

static struct platform_driver gpu_cc_sm6115_driver = {

--
2.41.0


2023-07-17 16:30:34

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH 09/15] clk: qcom: gcc-sm6115: Add runtime PM

The GCC block on SM6115 is mostly powered by the VDD_CX rail. We need
to ensure that it's enabled to prevent unwanted power collapse.

Enable runtime PM to keep the power flowing only when necessary.

Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gcc-sm6115.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm6115.c b/drivers/clk/qcom/gcc-sm6115.c
index 1b6016e7ddeb..7f1e278c63c0 100644
--- a/drivers/clk/qcom/gcc-sm6115.c
+++ b/drivers/clk/qcom/gcc-sm6115.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/clk-provider.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/reset-controller.h>

@@ -3383,14 +3384,26 @@ static int gcc_sm6115_probe(struct platform_device *pdev)
struct regmap *regmap;
int ret;

+ ret = devm_pm_runtime_enable(&pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = pm_runtime_resume_and_get(&pdev->dev);
+ if (ret)
+ return ret;
+
regmap = qcom_cc_map(pdev, &gcc_sm6115_desc);
- if (IS_ERR(regmap))
+ if (IS_ERR(regmap)) {
+ pm_runtime_put(&pdev->dev);
return PTR_ERR(regmap);
+ }

ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
ARRAY_SIZE(gcc_dfs_clocks));
- if (ret)
+ if (ret) {
+ pm_runtime_put(&pdev->dev);
return ret;
+ }

clk_alpha_pll_configure(&gpll8, regmap, &gpll8_config);
clk_alpha_pll_configure(&gpll9, regmap, &gpll9_config);
@@ -3415,7 +3428,10 @@ static int gcc_sm6115_probe(struct platform_device *pdev)
qcom_branch_set_clk_en(regmap, 0x36004);
qcom_branch_set_clk_en(regmap, 0x2b06c);

- return qcom_cc_really_probe(pdev, &gcc_sm6115_desc, regmap);
+ ret = qcom_cc_really_probe(pdev, &gcc_sm6115_desc, regmap);
+ pm_runtime_put(&pdev->dev);
+
+ return ret;
}

static struct platform_driver gcc_sm6115_driver = {

--
2.41.0


2023-07-17 16:51:08

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH 04/15] clk: qcom: gcc-sm6375: Add runtime PM

On Mon, Jul 17, 2023 at 05:19:11PM +0200, Konrad Dybcio wrote:
> The GCC block on SM6375 is powered by the VDD_CX rail. We need to ensure
> that it's enabled to prevent unwanted power collapse.
>
> Enable runtime PM to keep the power flowing only when necessary.
>

Are you sure this is necessary? If VDD_CX was really possible to fully
"power collapse" then I would expect that you lose all register
settings. This is not something we want or can even handle for GCC.
You would need to restore all frequency settings, branch bits etc etc.

Otherwise it's a retention state, but these are covered by the
corners/levels, not the enable/disable state.

I think most of these power domains are effectively always-on. The
important part is voting for minimal corners/levels so they can go to
minimal retention state with vlow/vmin.

Thanks,
Stephan

2023-07-18 04:16:42

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 04/15] clk: qcom: gcc-sm6375: Add runtime PM

On Mon, Jul 17, 2023 at 06:26:35PM +0200, Stephan Gerhold wrote:
> On Mon, Jul 17, 2023 at 05:19:11PM +0200, Konrad Dybcio wrote:
> > The GCC block on SM6375 is powered by the VDD_CX rail. We need to ensure
> > that it's enabled to prevent unwanted power collapse.
> >
> > Enable runtime PM to keep the power flowing only when necessary.
> >
>
> Are you sure this is necessary? If VDD_CX was really possible to fully
> "power collapse" then I would expect that you lose all register
> settings. This is not something we want or can even handle for GCC.
> You would need to restore all frequency settings, branch bits etc etc.
>

This differ between platforms, some allow us to completely power down CX
while keeping registers state using MX, others require that CX stays in
retention at least.

So, CX isn't the only rail powering GCC. For the most part though, we
have a relationship between frequencies votes for by clients and the
corner of CX, and hence I think the current description is ok...

> Otherwise it's a retention state, but these are covered by the
> corners/levels, not the enable/disable state.
>

I _think_ we still want to suspend each individual device (and the vote
from Linux), and let the system keep us at retention, instead of off...

> I think most of these power domains are effectively always-on. The
> important part is voting for minimal corners/levels so they can go to
> minimal retention state with vlow/vmin.
>

When you hit s2idle, you should expect to have the majority of the
rpm(h) PDs be voted off.

Regards,
Bjorn

> Thanks,
> Stephan

2023-07-18 12:13:34

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH 04/15] clk: qcom: gcc-sm6375: Add runtime PM

On Mon, Jul 17, 2023 at 09:02:29PM -0700, Bjorn Andersson wrote:
> On Mon, Jul 17, 2023 at 06:26:35PM +0200, Stephan Gerhold wrote:
> > On Mon, Jul 17, 2023 at 05:19:11PM +0200, Konrad Dybcio wrote:
> > > The GCC block on SM6375 is powered by the VDD_CX rail. We need to ensure
> > > that it's enabled to prevent unwanted power collapse.
> > >
> > > Enable runtime PM to keep the power flowing only when necessary.
> > >
> >
> > Are you sure this is necessary? If VDD_CX was really possible to fully
> > "power collapse" then I would expect that you lose all register
> > settings. This is not something we want or can even handle for GCC.
> > You would need to restore all frequency settings, branch bits etc etc.
> >
>
> This differ between platforms, some allow us to completely power down CX
> while keeping registers state using MX, others require that CX stays in
> retention at least.
>
> So, CX isn't the only rail powering GCC. For the most part though, we
> have a relationship between frequencies votes for by clients and the
> corner of CX, and hence I think the current description is ok...
>

This patch is just about sending enable/disable votes for the power
domains though, based on runtime PM which triggers when all the clocks
are disabled.

It's unrelated to voting for CX corners required by certain clock
frequencies (we handle those in the OPP tables of the consumers).
And it's also unrelated to ensuring rentention of register contents
since we actually release all votes when the clocks are idle.

So while adding runtime PM to all the clock drivers sounds nice, I'm
a bit confused what problem we're actually solving with this patch. :)

Thanks,
Stephan

2023-07-18 13:00:23

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 04/15] clk: qcom: gcc-sm6375: Add runtime PM

On 18.07.2023 14:07, Stephan Gerhold wrote:
> On Mon, Jul 17, 2023 at 09:02:29PM -0700, Bjorn Andersson wrote:
>> On Mon, Jul 17, 2023 at 06:26:35PM +0200, Stephan Gerhold wrote:
>>> On Mon, Jul 17, 2023 at 05:19:11PM +0200, Konrad Dybcio wrote:
>>>> The GCC block on SM6375 is powered by the VDD_CX rail. We need to ensure
>>>> that it's enabled to prevent unwanted power collapse.
>>>>
>>>> Enable runtime PM to keep the power flowing only when necessary.
>>>>
>>>
>>> Are you sure this is necessary? If VDD_CX was really possible to fully
>>> "power collapse" then I would expect that you lose all register
>>> settings. This is not something we want or can even handle for GCC.
>>> You would need to restore all frequency settings, branch bits etc etc.
>>>
>>
>> This differ between platforms, some allow us to completely power down CX
>> while keeping registers state using MX, others require that CX stays in
>> retention at least.
>>
>> So, CX isn't the only rail powering GCC. For the most part though, we
>> have a relationship between frequencies votes for by clients and the
>> corner of CX, and hence I think the current description is ok...
>>
>
> This patch is just about sending enable/disable votes for the power
> domains though, based on runtime PM which triggers when all the clocks
> are disabled.
>
> It's unrelated to voting for CX corners required by certain clock
> frequencies (we handle those in the OPP tables of the consumers).
> And it's also unrelated to ensuring rentention of register contents
> since we actually release all votes when the clocks are idle.
>
> So while adding runtime PM to all the clock drivers sounds nice, I'm
> a bit confused what problem we're actually solving with this patch. :)
In a very specific and unfortunate situation, there could be no other
CX votes, and trying to access (perhaps at least parts of) GCC would
result in a failure.

Konrad

2023-07-18 13:36:24

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 04/15] clk: qcom: gcc-sm6375: Add runtime PM

On Mon, Jul 17, 2023 at 05:19:11PM +0200, Konrad Dybcio wrote:
> The GCC block on SM6375 is powered by the VDD_CX rail. We need to ensure
> that it's enabled to prevent unwanted power collapse.

This bit is not correct either (and similar throughout the series).

> Enable runtime PM to keep the power flowing only when necessary.
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---

> + ret = pm_runtime_resume_and_get(&pdev->dev);
> + if (ret)
> + return ret;
> +
> regmap = qcom_cc_map(pdev, &gcc_sm6375_desc);
> - if (IS_ERR(regmap))
> + if (IS_ERR(regmap)) {
> + pm_runtime_put(&pdev->dev);
> return PTR_ERR(regmap);
> + }
>
> ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, ARRAY_SIZE(gcc_dfs_clocks));
> if (ret)

Looks like you forgot to update this error path.

> @@ -3817,7 +3828,10 @@ static int gcc_sm6375_probe(struct platform_device *pdev)
> clk_lucid_pll_configure(&gpll8, regmap, &gpll8_config);
> clk_zonda_pll_configure(&gpll9, regmap, &gpll9_config);
>
> - return qcom_cc_really_probe(pdev, &gcc_sm6375_desc, regmap);
> + ret = qcom_cc_really_probe(pdev, &gcc_sm6375_desc, regmap);
> + pm_runtime_put(&pdev->dev);
> +
> + return ret;

Johan

2023-07-18 13:39:25

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 07/15] clk: qcom: gpucc-sm6115: Add runtime PM

On Mon, Jul 17, 2023 at 05:19:14PM +0200, Konrad Dybcio wrote:
> The GPU_CC block on SM6115 is powered by the VDD_CX rail. We need to
> ensure that it's enabled to prevent unwanted power collapse.

This bit is not correct, the power domain would not have been disabled
until you enable runtime PM as part of this very patch.

I noticed similar claims have incorrectly been made in the past, for
example, in the recent:

2a541abd9837 clk: qcom: gcc-sc8280xp: Add runtime PM

and older:

a91c483b42fa ("clk: qcom: videocc-sm8250: use runtime PM for the clock controller")

Johan

2023-07-18 13:42:01

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 03/15] clk: qcom: gcc-sm6375: Unregister critical clocks

On Mon, Jul 17, 2023 at 05:19:10PM +0200, Konrad Dybcio wrote:
> Some clocks need to be always-on, but we don't really do anything
> with them, other than calling enable() once and telling Linux they're
> enabled.
>
> Unregister them to save a couple of bytes and, perhaps more
> importantly, allow for runtime suspend of the clock controller device,
> as CLK_IS_CRITICAL prevents the latter.

But this doesn't sound right. How can you disable a controller which
still has clocks enabled?

Shouldn't instead these clocks be modelled properly so that they are
only enabled when actually needed?

Johan

2023-07-18 13:45:15

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 07/15] clk: qcom: gpucc-sm6115: Add runtime PM

On 18.07.2023 15:24, Johan Hovold wrote:
> On Mon, Jul 17, 2023 at 05:19:14PM +0200, Konrad Dybcio wrote:
>> The GPU_CC block on SM6115 is powered by the VDD_CX rail. We need to
>> ensure that it's enabled to prevent unwanted power collapse.
>
> This bit is not correct, the power domain would not have been disabled
> until you enable runtime PM as part of this very patch.
Right, this was a bit of a thought-jump. The part that ensures there's
any vote at all is actually the DT commit adding a reference to the
genpd.

Konrad

2023-07-18 13:45:28

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 02/15] clk: qcom: Use qcom_branch_set_clk_en()

On Mon, Jul 17, 2023 at 05:19:09PM +0200, Konrad Dybcio wrote:
> Instead of magically poking at the bit0 of branch clocks' CBCR, use
> the newly introduced helper.
>
> Signed-off-by: Konrad Dybcio <[email protected]>

Nice cleanup.

Acked-by: Johan Hovold <[email protected]>

2023-07-18 13:46:01

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 03/15] clk: qcom: gcc-sm6375: Unregister critical clocks

On 18.07.2023 15:20, Johan Hovold wrote:
> On Mon, Jul 17, 2023 at 05:19:10PM +0200, Konrad Dybcio wrote:
>> Some clocks need to be always-on, but we don't really do anything
>> with them, other than calling enable() once and telling Linux they're
>> enabled.
>>
>> Unregister them to save a couple of bytes and, perhaps more
>> importantly, allow for runtime suspend of the clock controller device,
>> as CLK_IS_CRITICAL prevents the latter.
>
> But this doesn't sound right. How can you disable a controller which
> still has clocks enabled?
>
> Shouldn't instead these clocks be modelled properly so that they are
> only enabled when actually needed?
Hm.. We do have clk_branch2_aon_ops, but something still needs to
toggle these clocks.

I *think* we could leave a permanent vote in probe() without breaking
runtime pm! I'll give it a spin bit later..

Konrad

2023-07-18 16:45:35

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 03/15] clk: qcom: gcc-sm6375: Unregister critical clocks

On Tue, Jul 18, 2023 at 03:26:51PM +0200, Konrad Dybcio wrote:
> On 18.07.2023 15:20, Johan Hovold wrote:
> > On Mon, Jul 17, 2023 at 05:19:10PM +0200, Konrad Dybcio wrote:
> >> Some clocks need to be always-on, but we don't really do anything
> >> with them, other than calling enable() once and telling Linux they're
> >> enabled.
> >>
> >> Unregister them to save a couple of bytes and, perhaps more
> >> importantly, allow for runtime suspend of the clock controller device,
> >> as CLK_IS_CRITICAL prevents the latter.
> >
> > But this doesn't sound right. How can you disable a controller which
> > still has clocks enabled?
> >
> > Shouldn't instead these clocks be modelled properly so that they are
> > only enabled when actually needed?
> Hm.. We do have clk_branch2_aon_ops, but something still needs to
> toggle these clocks.
>

Before we started replacing these clocks with static votes, I handled
exactly this problem in the turingcc-qcs404 driver by registering the
ahb clock with a pm_clk_add(). The clock framework will then
automagically keep the clock enabled around operations, but it will also
keep the runtime state active as long as the clock is prepared.

As mentioned in an earlier reply today, there's no similarity to this in
the reset or gdsc code, so we'd need to add that in order to rely on
such mechanism.

> I *think* we could leave a permanent vote in probe() without breaking
> runtime pm! I'll give it a spin bit later..
>

Modelling the AHB clock in DT and putting a devm_clk_get_enabled() would
properly connect the two, and thereby handle probe order between the two
clock controllers.

But it would prevent the power-domain of the parent provider to ever
suspending. Using pm_clk_add() this would at least depend on client
votes.

Regards,
Bjorn

2023-07-19 09:04:47

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 03/15] clk: qcom: gcc-sm6375: Unregister critical clocks

On Tue, Jul 18, 2023 at 09:23:52AM -0700, Bjorn Andersson wrote:
> On Tue, Jul 18, 2023 at 03:26:51PM +0200, Konrad Dybcio wrote:
> > On 18.07.2023 15:20, Johan Hovold wrote:
> > > On Mon, Jul 17, 2023 at 05:19:10PM +0200, Konrad Dybcio wrote:
> > >> Some clocks need to be always-on, but we don't really do anything
> > >> with them, other than calling enable() once and telling Linux they're
> > >> enabled.
> > >>
> > >> Unregister them to save a couple of bytes and, perhaps more
> > >> importantly, allow for runtime suspend of the clock controller device,
> > >> as CLK_IS_CRITICAL prevents the latter.
> > >
> > > But this doesn't sound right. How can you disable a controller which
> > > still has clocks enabled?
> > >
> > > Shouldn't instead these clocks be modelled properly so that they are
> > > only enabled when actually needed?
> > Hm.. We do have clk_branch2_aon_ops, but something still needs to
> > toggle these clocks.
> >
>
> Before we started replacing these clocks with static votes, I handled
> exactly this problem in the turingcc-qcs404 driver by registering the
> ahb clock with a pm_clk_add(). The clock framework will then
> automagically keep the clock enabled around operations, but it will also
> keep the runtime state active as long as the clock is prepared.
>
> As mentioned in an earlier reply today, there's no similarity to this in
> the reset or gdsc code, so we'd need to add that in order to rely on
> such mechanism.

This reminds me of:

4cc47e8add63 ("clk: qcom: gdsc: Remove direct runtime PM calls")

which recently removed a broken attempt to implement this for gdscs.

Just stumbled over GENPD_FLAG_PM_CLK which may provide a way forward at
least for genpd (but see below).

> > I *think* we could leave a permanent vote in probe() without breaking
> > runtime pm! I'll give it a spin bit later..
> >
>
> Modelling the AHB clock in DT and putting a devm_clk_get_enabled() would
> properly connect the two, and thereby handle probe order between the two
> clock controllers.

Yeah, this dependency really should be described eventually.

> But it would prevent the power-domain of the parent provider to ever
> suspending. Using pm_clk_add() this would at least depend on client
> votes.

IIUC using pm_clk_add() would also prevent the parent from suspending
due to that resume call in clk_prepare().

And this mechanism is also used for GENPD_FLAG_PM_CLK...

Johan

2023-08-09 17:23:12

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 03/15] clk: qcom: gcc-sm6375: Unregister critical clocks

On 19.07.2023 10:43, Johan Hovold wrote:
> On Tue, Jul 18, 2023 at 09:23:52AM -0700, Bjorn Andersson wrote:
>> On Tue, Jul 18, 2023 at 03:26:51PM +0200, Konrad Dybcio wrote:
>>> On 18.07.2023 15:20, Johan Hovold wrote:
>>>> On Mon, Jul 17, 2023 at 05:19:10PM +0200, Konrad Dybcio wrote:
>>>>> Some clocks need to be always-on, but we don't really do anything
>>>>> with them, other than calling enable() once and telling Linux they're
>>>>> enabled.
>>>>>
>>>>> Unregister them to save a couple of bytes and, perhaps more
>>>>> importantly, allow for runtime suspend of the clock controller device,
>>>>> as CLK_IS_CRITICAL prevents the latter.
>>>>
>>>> But this doesn't sound right. How can you disable a controller which
>>>> still has clocks enabled?
>>>>
>>>> Shouldn't instead these clocks be modelled properly so that they are
>>>> only enabled when actually needed?
>>> Hm.. We do have clk_branch2_aon_ops, but something still needs to
>>> toggle these clocks.
>>>
>>
>> Before we started replacing these clocks with static votes, I handled
>> exactly this problem in the turingcc-qcs404 driver by registering the
>> ahb clock with a pm_clk_add(). The clock framework will then
>> automagically keep the clock enabled around operations, but it will also
>> keep the runtime state active as long as the clock is prepared.
>>
>> As mentioned in an earlier reply today, there's no similarity to this in
>> the reset or gdsc code, so we'd need to add that in order to rely on
>> such mechanism.
>
> This reminds me of:
>
> 4cc47e8add63 ("clk: qcom: gdsc: Remove direct runtime PM calls")
>
> which recently removed a broken attempt to implement this for gdscs.
>
> Just stumbled over GENPD_FLAG_PM_CLK which may provide a way forward at
> least for genpd (but see below).
>
>>> I *think* we could leave a permanent vote in probe() without breaking
>>> runtime pm! I'll give it a spin bit later..
>>>
>>
>> Modelling the AHB clock in DT and putting a devm_clk_get_enabled() would
>> properly connect the two, and thereby handle probe order between the two
>> clock controllers.
>
> Yeah, this dependency really should be described eventually.
>
>> But it would prevent the power-domain of the parent provider to ever
>> suspending. Using pm_clk_add() this would at least depend on client
>> votes.
>
> IIUC using pm_clk_add() would also prevent the parent from suspending
> due to that resume call in clk_prepare().
>
> And this mechanism is also used for GENPD_FLAG_PM_CLK...
So.. how do we go about solving the issue that this patch tried to
address?

Konrad

2023-08-09 18:04:51

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 07/15] clk: qcom: gpucc-sm6115: Add runtime PM

On 18.07.2023 15:28, Konrad Dybcio wrote:
> On 18.07.2023 15:24, Johan Hovold wrote:
>> On Mon, Jul 17, 2023 at 05:19:14PM +0200, Konrad Dybcio wrote:
>>> The GPU_CC block on SM6115 is powered by the VDD_CX rail. We need to
>>> ensure that it's enabled to prevent unwanted power collapse.
>>
>> This bit is not correct, the power domain would not have been disabled
>> until you enable runtime PM as part of this very patch.
> Right, this was a bit of a thought-jump. The part that ensures there's
> any vote at all is actually the DT commit adding a reference to the
> genpd.
Well I read it again and I think my original intention was "it = the power
domain" and not "it = runtime PM", it makes sense that way..

Anyway, I'll rephrase that to make it less ambiguous.

Konrad