2024-01-13 14:51:13

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 00/12] 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]>
---
Changes in v6:
- Rebase (next-20240112)
- Reorder qcom_branch_set_clk_en calls by register in "*: Unregister
critical clocks" (Johan)
- Pick up tags
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- Change the "Keep the critical clocks always-on" comment to "Keep
some clocks always-on"
- Add the same comment to commits unregistering clocks on 6115/6375/2290
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
- Add and unify the "/* Keep the critical clocks always-on */" comment
- Rebase (next-20231222), also include 8650, X1E and 8280camcc drivers
- Drop enabling runtime PM on GCC
- Improve the commit message of "clk: qcom: gpucc-sm6115: Add runtime PM"
- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Rebase (next-20231219)
- Fix up a copypaste mistake in "gcc-sm6375: Unregister critical clocks" (bod)
- Pick up tags
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Rebase
- Pick up tags
- Fix up missing pm_runtime_put in SM6375 GCC (Johan)
- Clarify the commit message of "Add runtime PM" commits (Johan)
- "GPU_CCC" -> "GPU_CC" (oops)
- Rebase atop next-20231129
- Also fix up camcc-sm8550 & gcc-sm4450
- Unify and clean up the comment style
- Fix missing comments in gcc-sc7180..
- Drop Johan's ack from "clk: qcom: Use qcom_branch_set_clk_en()"
- Improve 6115 dt patch commit message (Bjorn)
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Konrad Dybcio (12):
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: 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-qcm2290: Unregister critical clocks
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_CC

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/camcc-sc8280xp.c | 6 +-
drivers/clk/qcom/camcc-sm8550.c | 10 +--
drivers/clk/qcom/clk-branch.h | 7 ++
drivers/clk/qcom/dispcc-qcm2290.c | 4 +-
drivers/clk/qcom/dispcc-sc7280.c | 7 +-
drivers/clk/qcom/dispcc-sc8280xp.c | 4 +-
drivers/clk/qcom/dispcc-sm6115.c | 4 +-
drivers/clk/qcom/dispcc-sm8250.c | 4 +-
drivers/clk/qcom/dispcc-sm8450.c | 7 +-
drivers/clk/qcom/dispcc-sm8550.c | 7 +-
drivers/clk/qcom/dispcc-sm8650.c | 4 +-
drivers/clk/qcom/gcc-qcm2290.c | 106 +++--------------------------
drivers/clk/qcom/gcc-sa8775p.c | 25 +++----
drivers/clk/qcom/gcc-sc7180.c | 22 +++---
drivers/clk/qcom/gcc-sc7280.c | 20 +++---
drivers/clk/qcom/gcc-sc8180x.c | 28 +++-----
drivers/clk/qcom/gcc-sc8280xp.c | 25 +++----
drivers/clk/qcom/gcc-sdx55.c | 12 ++--
drivers/clk/qcom/gcc-sdx65.c | 13 ++--
drivers/clk/qcom/gcc-sdx75.c | 10 +--
drivers/clk/qcom/gcc-sm4450.c | 28 +++-----
drivers/clk/qcom/gcc-sm6115.c | 124 +++-------------------------------
drivers/clk/qcom/gcc-sm6375.c | 105 +++-------------------------
drivers/clk/qcom/gcc-sm7150.c | 23 +++----
drivers/clk/qcom/gcc-sm8250.c | 19 ++----
drivers/clk/qcom/gcc-sm8350.c | 20 +++---
drivers/clk/qcom/gcc-sm8450.c | 21 +++---
drivers/clk/qcom/gcc-sm8550.c | 21 +++---
drivers/clk/qcom/gcc-sm8650.c | 16 ++---
drivers/clk/qcom/gcc-x1e80100.c | 16 ++---
drivers/clk/qcom/gpucc-sc7280.c | 9 +--
drivers/clk/qcom/gpucc-sc8280xp.c | 9 +--
drivers/clk/qcom/gpucc-sm6115.c | 53 ++++++---------
drivers/clk/qcom/gpucc-sm6375.c | 34 ++--------
drivers/clk/qcom/gpucc-sm8550.c | 10 +--
drivers/clk/qcom/lpasscorecc-sc7180.c | 7 +-
drivers/clk/qcom/videocc-sm8250.c | 6 +-
drivers/clk/qcom/videocc-sm8350.c | 10 +--
drivers/clk/qcom/videocc-sm8450.c | 13 ++--
drivers/clk/qcom/videocc-sm8550.c | 13 ++--
43 files changed, 234 insertions(+), 653 deletions(-)
---
base-commit: 8d04a7e2ee3fd6aabb8096b00c64db0d735bc874
change-id: 20230717-topic-branch_aon_cleanup-6976c13fe71c

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



2024-01-13 14:51:21

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 01/12] clk: qcom: branch: Add a helper for setting the enable bit

We hardcode some clocks to be always-on, as they're essential to the
functioning of the SoC / some peripherals. Add a helper to do so
to make the writes less magic.

Reviewed-by: Johan Hovold <[email protected]>
Reviewed-by: Bryan O'Donoghue <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/clk-branch.h | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h
index 8ffed603c050..0514bc43100b 100644
--- a/drivers/clk/qcom/clk-branch.h
+++ b/drivers/clk/qcom/clk-branch.h
@@ -64,6 +64,7 @@ struct clk_mem_branch {
#define CBCR_FORCE_MEM_PERIPH_OFF BIT(12)
#define CBCR_WAKEUP GENMASK(11, 8)
#define CBCR_SLEEP GENMASK(7, 4)
+#define CBCR_CLOCK_ENABLE BIT(0)

static inline void qcom_branch_set_force_mem_core(struct regmap *regmap,
struct clk_branch clk, bool on)
@@ -98,6 +99,12 @@ static inline void qcom_branch_set_sleep(struct regmap *regmap, struct clk_branc
FIELD_PREP(CBCR_SLEEP, val));
}

+static inline void qcom_branch_set_clk_en(struct regmap *regmap, u32 cbcr)
+{
+ regmap_update_bits(regmap, cbcr, CBCR_CLOCK_ENABLE,
+ CBCR_CLOCK_ENABLE);
+}
+
extern const struct clk_ops clk_branch_ops;
extern const struct clk_ops clk_branch2_ops;
extern const struct clk_ops clk_branch_simple_ops;

--
2.43.0


2024-01-13 14:51:55

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 03/12] 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 | 96 +++----------------------------------------
1 file changed, 6 insertions(+), 90 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm6375.c b/drivers/clk/qcom/gcc-sm6375.c
index 84639d5b89bf..1a45d1ae997a 100644
--- a/drivers/clk/qcom/gcc-sm6375.c
+++ b/drivers/clk/qcom/gcc-sm6375.c
@@ -1743,22 +1743,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,
@@ -2309,22 +2293,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,
@@ -2455,22 +2423,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 = {
@@ -3094,26 +3046,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,
@@ -3433,22 +3365,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,
@@ -3615,7 +3531,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,
@@ -3671,7 +3586,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,
@@ -3683,7 +3597,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,
@@ -3739,7 +3652,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,
@@ -3766,7 +3678,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,9 +3794,14 @@ static int gcc_sm6375_probe(struct platform_device *pdev)
return ret;

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

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

--
2.43.0


2024-01-13 14:52:15

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 04/12] clk: qcom: gpucc-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.

Reviewed-by: Bryan O'Donoghue <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/gpucc-sm6375.c | 34 ++++------------------------------
1 file changed, 4 insertions(+), 30 deletions(-)

diff --git a/drivers/clk/qcom/gpucc-sm6375.c b/drivers/clk/qcom/gpucc-sm6375.c
index da24276a018e..07ebe5e139d5 100644
--- a/drivers/clk/qcom/gpucc-sm6375.c
+++ b/drivers/clk/qcom/gpucc-sm6375.c
@@ -183,20 +183,6 @@ static struct clk_rcg2 gpucc_gx_gfx3d_clk_src = {
},
};

-static struct clk_branch gpucc_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 = "gpucc_ahb_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gpucc_cx_gfx3d_clk = {
.halt_reg = 0x10a4,
.halt_check = BRANCH_HALT_DELAY,
@@ -294,20 +280,6 @@ static struct clk_branch gpucc_cxo_clk = {
},
};

-static struct clk_branch gpucc_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 = "gpucc_gx_cxo_clk",
- .flags = CLK_IS_CRITICAL,
- .ops = &clk_branch2_ops,
- },
- },
-};
-
static struct clk_branch gpucc_gx_gfx3d_clk = {
.halt_reg = 0x1054,
.halt_check = BRANCH_HALT_DELAY,
@@ -381,7 +353,6 @@ static struct gdsc gpu_gx_gdsc = {
};

static struct clk_regmap *gpucc_sm6375_clocks[] = {
- [GPU_CC_AHB_CLK] = &gpucc_ahb_clk.clkr,
[GPU_CC_CX_GFX3D_CLK] = &gpucc_cx_gfx3d_clk.clkr,
[GPU_CC_CX_GFX3D_SLV_CLK] = &gpucc_cx_gfx3d_slv_clk.clkr,
[GPU_CC_CX_GMU_CLK] = &gpucc_cx_gmu_clk.clkr,
@@ -389,7 +360,6 @@ static struct clk_regmap *gpucc_sm6375_clocks[] = {
[GPU_CC_CXO_AON_CLK] = &gpucc_cxo_aon_clk.clkr,
[GPU_CC_CXO_CLK] = &gpucc_cxo_clk.clkr,
[GPU_CC_GMU_CLK_SRC] = &gpucc_gmu_clk_src.clkr,
- [GPU_CC_GX_CXO_CLK] = &gpucc_gx_cxo_clk.clkr,
[GPU_CC_GX_GFX3D_CLK] = &gpucc_gx_gfx3d_clk.clkr,
[GPU_CC_GX_GFX3D_CLK_SRC] = &gpucc_gx_gfx3d_clk_src.clkr,
[GPU_CC_GX_GMU_CLK] = &gpucc_gx_gmu_clk.clkr,
@@ -455,6 +425,10 @@ static int gpucc_sm6375_probe(struct platform_device *pdev)
clk_lucid_pll_configure(&gpucc_pll0, regmap, &gpucc_pll0_config);
clk_lucid_pll_configure(&gpucc_pll1, regmap, &gpucc_pll1_config);

+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x1078); /* GPUCC_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x1060); /* GPUCC_GX_CXO_CLK */
+
ret = qcom_cc_really_probe(pdev, &gpucc_sm6375_desc, regmap);
pm_runtime_put(&pdev->dev);


--
2.43.0


2024-01-13 14:52:25

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 05/12] 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 | 34 ++++------------------------------
1 file changed, 4 insertions(+), 30 deletions(-)

diff --git a/drivers/clk/qcom/gpucc-sm6115.c b/drivers/clk/qcom/gpucc-sm6115.c
index fb71c21c9a89..2c2c184747b1 100644
--- a/drivers/clk/qcom/gpucc-sm6115.c
+++ b/drivers/clk/qcom/gpucc-sm6115.c
@@ -234,20 +234,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,
@@ -336,20 +322,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,
@@ -418,7 +390,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,
@@ -426,7 +397,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,
@@ -488,6 +458,10 @@ 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 some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x1078); /* GPU_CC_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x1060); /* GPU_CC_GX_CXO_CLK */
+
return qcom_cc_really_probe(pdev, &gpu_cc_sm6115_desc, regmap);
}


--
2.43.0


2024-01-13 14:52:39

by Konrad Dybcio

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

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

Reviewed-by: Bryan O'Donoghue <[email protected]>
Reviewed-by: Johan Hovold <[email protected]>
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/clk/qcom/camcc-sc8280xp.c | 6 ++----
drivers/clk/qcom/camcc-sm8550.c | 10 +++-------
drivers/clk/qcom/dispcc-qcm2290.c | 4 ++--
drivers/clk/qcom/dispcc-sc7280.c | 7 ++-----
drivers/clk/qcom/dispcc-sc8280xp.c | 4 ++--
drivers/clk/qcom/dispcc-sm6115.c | 4 ++--
drivers/clk/qcom/dispcc-sm8250.c | 4 ++--
drivers/clk/qcom/dispcc-sm8450.c | 7 ++-----
drivers/clk/qcom/dispcc-sm8550.c | 7 ++-----
drivers/clk/qcom/dispcc-sm8650.c | 4 ++--
drivers/clk/qcom/gcc-sa8775p.c | 25 ++++++++++---------------
drivers/clk/qcom/gcc-sc7180.c | 22 +++++++++-------------
drivers/clk/qcom/gcc-sc7280.c | 20 ++++++++------------
drivers/clk/qcom/gcc-sc8180x.c | 28 +++++++++++-----------------
drivers/clk/qcom/gcc-sc8280xp.c | 25 ++++++++++---------------
drivers/clk/qcom/gcc-sdx55.c | 12 ++++--------
drivers/clk/qcom/gcc-sdx65.c | 13 +++++--------
drivers/clk/qcom/gcc-sdx75.c | 10 +++-------
drivers/clk/qcom/gcc-sm4450.c | 28 +++++++++-------------------
drivers/clk/qcom/gcc-sm6375.c | 11 ++++-------
drivers/clk/qcom/gcc-sm7150.c | 23 +++++++++--------------
drivers/clk/qcom/gcc-sm8250.c | 19 +++++++------------
drivers/clk/qcom/gcc-sm8350.c | 20 ++++++++------------
drivers/clk/qcom/gcc-sm8450.c | 21 ++++++++-------------
drivers/clk/qcom/gcc-sm8550.c | 21 ++++++++-------------
drivers/clk/qcom/gcc-sm8650.c | 16 ++++++++--------
drivers/clk/qcom/gcc-x1e80100.c | 16 ++++++++--------
drivers/clk/qcom/gpucc-sc7280.c | 9 +++------
drivers/clk/qcom/gpucc-sc8280xp.c | 9 +++------
drivers/clk/qcom/gpucc-sm8550.c | 10 +++-------
drivers/clk/qcom/lpasscorecc-sc7180.c | 7 ++-----
drivers/clk/qcom/videocc-sm8250.c | 6 +++---
drivers/clk/qcom/videocc-sm8350.c | 10 +++-------
drivers/clk/qcom/videocc-sm8450.c | 13 ++++---------
drivers/clk/qcom/videocc-sm8550.c | 13 ++++---------
35 files changed, 175 insertions(+), 289 deletions(-)

diff --git a/drivers/clk/qcom/camcc-sc8280xp.c b/drivers/clk/qcom/camcc-sc8280xp.c
index 3dcd79b01515..84f9caf3ddbf 100644
--- a/drivers/clk/qcom/camcc-sc8280xp.c
+++ b/drivers/clk/qcom/camcc-sc8280xp.c
@@ -3010,10 +3010,8 @@ static int camcc_sc8280xp_probe(struct platform_device *pdev)
clk_lucid_pll_configure(&camcc_pll6, regmap, &camcc_pll6_config);
clk_lucid_pll_configure(&camcc_pll7, regmap, &camcc_pll7_config);

- /*
- * Keep camcc_gdsc_clk always enabled:
- */
- regmap_update_bits(regmap, 0xc1e4, BIT(0), 1);
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0xc1e4); /* CAMCC_GDSC_CLK */

ret = qcom_cc_really_probe(pdev, &camcc_sc8280xp_desc, regmap);
if (ret)
diff --git a/drivers/clk/qcom/camcc-sm8550.c b/drivers/clk/qcom/camcc-sm8550.c
index dd51ba4ea757..1ef59a96f664 100644
--- a/drivers/clk/qcom/camcc-sm8550.c
+++ b/drivers/clk/qcom/camcc-sm8550.c
@@ -3536,13 +3536,9 @@ static int cam_cc_sm8550_probe(struct platform_device *pdev)
clk_lucid_ole_pll_configure(&cam_cc_pll11, regmap, &cam_cc_pll11_config);
clk_lucid_ole_pll_configure(&cam_cc_pll12, regmap, &cam_cc_pll12_config);

- /*
- * Keep clocks always enabled:
- * cam_cc_gdsc_clk
- * cam_cc_sleep_clk
- */
- regmap_update_bits(regmap, 0x1419c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x142cc, BIT(0), BIT(0));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x1419c); /* CAM_CC_GDSC_CLK */
+ qcom_branch_set_clk_en(regmap, 0x142cc); /* CAM_CC_SLEEP_CLK */

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

diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
index 9206f0eed446..200f81ac4827 100644
--- a/drivers/clk/qcom/dispcc-qcm2290.c
+++ b/drivers/clk/qcom/dispcc-qcm2290.c
@@ -519,8 +519,8 @@ 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x604c); /* DISP_CC_XO_CLK */

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..3ba07f08cbdd 100644
--- a/drivers/clk/qcom/dispcc-sc7280.c
+++ b/drivers/clk/qcom/dispcc-sc7280.c
@@ -878,11 +878,8 @@ static int disp_cc_sc7280_probe(struct platform_device *pdev)

clk_lucid_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);

- /*
- * Keep the clocks always-ON
- * DISP_CC_XO_CLK
- */
- regmap_update_bits(regmap, 0x5008, BIT(0), BIT(0));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x5008); /* DISP_CC_XO_CLK */

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 30f636b9f0ec..bd1ffb143e0c 100644
--- a/drivers/clk/qcom/dispcc-sc8280xp.c
+++ b/drivers/clk/qcom/dispcc-sc8280xp.c
@@ -3178,8 +3178,8 @@ static int disp_cc_sc8280xp_probe(struct platform_device *pdev)
goto out_pm_runtime_put;
}

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

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 1fab43f08e73..bd07f26af35a 100644
--- a/drivers/clk/qcom/dispcc-sm6115.c
+++ b/drivers/clk/qcom/dispcc-sm6115.c
@@ -583,8 +583,8 @@ 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x604c); /* DISP_CC_XO_CLK */

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..ccf696c74dc3 100644
--- a/drivers/clk/qcom/dispcc-sm8250.c
+++ b/drivers/clk/qcom/dispcc-sm8250.c
@@ -1365,8 +1365,8 @@ static int disp_cc_sm8250_probe(struct platform_device *pdev)
/* Enable clock gating for MDP clocks */
regmap_update_bits(regmap, 0x8000, 0x10, 0x10);

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

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 2c4aecd75186..b358751aba4b 100644
--- a/drivers/clk/qcom/dispcc-sm8450.c
+++ b/drivers/clk/qcom/dispcc-sm8450.c
@@ -1787,11 +1787,8 @@ static int disp_cc_sm8450_probe(struct platform_device *pdev)
/* Enable clock gating for MDP clocks */
regmap_update_bits(regmap, DISP_CC_MISC_CMD, 0x10, 0x10);

- /*
- * Keep clocks always enabled:
- * disp_cc_xo_clk
- */
- regmap_update_bits(regmap, 0xe05c, BIT(0), BIT(0));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0xe05c); /* DISP_CC_XO_CLK */

ret = qcom_cc_really_probe(pdev, &disp_cc_sm8450_desc, regmap);
if (ret)
diff --git a/drivers/clk/qcom/dispcc-sm8550.c b/drivers/clk/qcom/dispcc-sm8550.c
index f96d8b81fd9a..3d86b20e2062 100644
--- a/drivers/clk/qcom/dispcc-sm8550.c
+++ b/drivers/clk/qcom/dispcc-sm8550.c
@@ -1780,11 +1780,8 @@ static int disp_cc_sm8550_probe(struct platform_device *pdev)
/* Enable clock gating for MDP clocks */
regmap_update_bits(regmap, DISP_CC_MISC_CMD, 0x10, 0x10);

- /*
- * Keep clocks always enabled:
- * disp_cc_xo_clk
- */
- regmap_update_bits(regmap, 0xe054, BIT(0), BIT(0));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0xe054); /* DISP_CC_XO_CLK */

ret = qcom_cc_really_probe(pdev, &disp_cc_sm8550_desc, regmap);
if (ret)
diff --git a/drivers/clk/qcom/dispcc-sm8650.c b/drivers/clk/qcom/dispcc-sm8650.c
index f3b1d9d16bae..795ac4d93658 100644
--- a/drivers/clk/qcom/dispcc-sm8650.c
+++ b/drivers/clk/qcom/dispcc-sm8650.c
@@ -1777,8 +1777,8 @@ static int disp_cc_sm8650_probe(struct platform_device *pdev)
/* Enable clock gating for MDP clocks */
regmap_update_bits(regmap, DISP_CC_MISC_CMD, 0x10, 0x10);

- /* Keep clocks always enabled */
- regmap_update_bits(regmap, 0xe054, BIT(0), BIT(0)); /* disp_cc_xo_clk */
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0xe054); /* DISP_CC_XO_CLK */

ret = qcom_cc_really_probe(pdev, &disp_cc_sm8650_desc, regmap);
if (ret)
diff --git a/drivers/clk/qcom/gcc-sa8775p.c b/drivers/clk/qcom/gcc-sa8775p.c
index 8171d23c96e6..9dbb68f60d86 100644
--- a/drivers/clk/qcom/gcc-sa8775p.c
+++ b/drivers/clk/qcom/gcc-sa8775p.c
@@ -4742,21 +4742,16 @@ static int gcc_sa8775p_probe(struct platform_device *pdev)
if (ret)
return ret;

- /*
- * Keep the clocks always-ON
- * GCC_CAMERA_AHB_CLK, GCC_CAMERA_XO_CLK, GCC_DISP1_AHB_CLK,
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x32004); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x32020); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0xc7004); /* GCC_DISP1_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0xc7018); /* GCC_DISP1_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x33004); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x33018); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x7d004); /* GCC_GPU_CFG_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x34004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x34024); /* GCC_VIDEO_XO_CLK */

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 a3406aadbd17..6a5f785c0ced 100644
--- a/drivers/clk/qcom/gcc-sc7180.c
+++ b/drivers/clk/qcom/gcc-sc7180.c
@@ -2443,19 +2443,15 @@ static int gcc_sc7180_probe(struct platform_device *pdev)
regmap_update_bits(regmap, 0x4d110, 0x3, 0x3);
regmap_update_bits(regmap, 0x71028, 0x3, 0x3);

- /*
- * Keep the clocks always-ON
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x48004); /* GCC_CPUSS_GNOC_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b008); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b00c); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b02c); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b028); /* GCC_VIDEO_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b030); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */

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 2b661df5de26..f45a8318900c 100644
--- a/drivers/clk/qcom/gcc-sc7280.c
+++ b/drivers/clk/qcom/gcc-sc7280.c
@@ -3453,18 +3453,14 @@ static int gcc_sc7280_probe(struct platform_device *pdev)
if (IS_ERR(regmap))
return PTR_ERR(regmap);

- /*
- * Keep the clocks always-ON
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x26004);/* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x26028);/* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27004);/* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x2701c);/* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x28004);/* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x28014);/* GCC_VIDEO_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004);/* GCC_GPU_CFG_AHB_CLK */
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 ae2147381559..c8dfe76f5582 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4579,23 +4579,17 @@ static int gcc_sc8180x_probe(struct platform_device *pdev)
if (IS_ERR(regmap))
return PTR_ERR(regmap);

- /*
- * Enable the following always-on clocks:
- * GCC_VIDEO_AHB_CLK, GCC_CAMERA_AHB_CLK, GCC_DISP_AHB_CLK,
- * GCC_VIDEO_XO_CLK, GCC_CAMERA_XO_CLK, GCC_DISP_XO_CLK,
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0xb004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0xb008); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0xb00c); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0xb040); /* GCC_VIDEO_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0xb044); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0xb048); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x48004); /* GCC_CPUSS_GNOC_CLK */
+ qcom_branch_set_clk_en(regmap, 0x48190); /* GCC_CPUSS_DVM_BUS_CLK */
+ qcom_branch_set_clk_en(regmap, 0x4d004); /* GCC_NPU_CFG_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */

/* 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 bfb77931e868..816a06bfedb2 100644
--- a/drivers/clk/qcom/gcc-sc8280xp.c
+++ b/drivers/clk/qcom/gcc-sc8280xp.c
@@ -7543,21 +7543,16 @@ static int gcc_sc8280xp_probe(struct platform_device *pdev)
goto err_put_rpm;
}

- /*
- * Keep the clocks always-ON
- * 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, 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x26004); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x26020); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27004); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27028); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x28004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x28028); /* GCC_VIDEO_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0xbb004); /* GCC_DISP1_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0xbb028); /* GCC_DISP1_XO_CLK */

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..26279b8d321a 100644
--- a/drivers/clk/qcom/gcc-sdx55.c
+++ b/drivers/clk/qcom/gcc-sdx55.c
@@ -1611,14 +1611,10 @@ static int gcc_sdx55_probe(struct platform_device *pdev)
if (IS_ERR(regmap))
return PTR_ERR(regmap);

- /*
- * Keep the clocks always-ON as they are critical to the functioning
- * 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));
- regmap_update_bits(regmap, 0x6d008, BIT(21), BIT(21));
- regmap_update_bits(regmap, 0x6d008, BIT(22), BIT(22));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x6d008); /* GCC_SYS_NOC_CPUSS_AHB_CLK */
+ regmap_update_bits(regmap, 0x6d008, BIT(21), BIT(21)); /* GCC_CPUSS_AHB_CLK */
+ regmap_update_bits(regmap, 0x6d008, BIT(22), BIT(22)); /* GCC_CPUSS_GNOC_CLK */

return qcom_cc_really_probe(pdev, &gcc_sdx55_desc, regmap);
}
diff --git a/drivers/clk/qcom/gcc-sdx65.c b/drivers/clk/qcom/gcc-sdx65.c
index ffddbed5a6db..8fde6463574b 100644
--- a/drivers/clk/qcom/gcc-sdx65.c
+++ b/drivers/clk/qcom/gcc-sdx65.c
@@ -1574,14 +1574,11 @@ static int gcc_sdx65_probe(struct platform_device *pdev)
regmap = qcom_cc_map(pdev, &gcc_sdx65_desc);
if (IS_ERR(regmap))
return PTR_ERR(regmap);
- /*
- * Keep the clocks always-ON as they are critical to the functioning
- * 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));
- regmap_update_bits(regmap, 0x6d008, BIT(21), BIT(21));
- regmap_update_bits(regmap, 0x6d008, BIT(22), BIT(22));
+
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x6d008); /* GCC_SYS_NOC_CPUSS_AHB_CLK */
+ regmap_update_bits(regmap, 0x6d008, BIT(21), BIT(21)); /* GCC_CPUSS_AHB_CLK */
+ regmap_update_bits(regmap, 0x6d008, BIT(22), BIT(22)); /* GCC_CPUSS_GNOC_CLK */

return qcom_cc_really_probe(pdev, &gcc_sdx65_desc, regmap);
}
diff --git a/drivers/clk/qcom/gcc-sdx75.c b/drivers/clk/qcom/gcc-sdx75.c
index 573af17bd24c..c51338f08ef1 100644
--- a/drivers/clk/qcom/gcc-sdx75.c
+++ b/drivers/clk/qcom/gcc-sdx75.c
@@ -2936,13 +2936,9 @@ static int gcc_sdx75_probe(struct platform_device *pdev)
if (ret)
return ret;

- /*
- * Keep clocks always enabled:
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x3e004); /* GCC_AHB_PCIE_LINK_CLK */
+ qcom_branch_set_clk_en(regmap, 0x3e008); /* GCC_XO_PCIE_LINK_CLK */

return qcom_cc_really_probe(pdev, &gcc_sdx75_desc, regmap);
}
diff --git a/drivers/clk/qcom/gcc-sm4450.c b/drivers/clk/qcom/gcc-sm4450.c
index 31abe2775fc8..1226d39e6442 100644
--- a/drivers/clk/qcom/gcc-sm4450.c
+++ b/drivers/clk/qcom/gcc-sm4450.c
@@ -2849,25 +2849,15 @@ static int gcc_sm4450_probe(struct platform_device *pdev)

qcom_branch_set_force_mem_core(regmap, gcc_ufs_phy_ice_core_clk, true);

- /*
- * Keep clocks always enabled:
- * gcc_camera_ahb_clk
- * gcc_camera_sleep_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, 0x36004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x36018, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x3601c, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x37004, BIT(0), BIT(0));
- regmap_update_bits(regmap, 0x37014, 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, 0x42018, BIT(0), BIT(0));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x36018); /* GCC_CAMERA_SLEEP_CLK */
+ qcom_branch_set_clk_en(regmap, 0x3601c); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x37004); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x37014); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x81004); /* GCC_GPU_CFG_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x42004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x42018); /* GCC_VIDEO_XO_CLK */

regmap_update_bits(regmap, 0x4201c, BIT(21), BIT(21));

diff --git a/drivers/clk/qcom/gcc-sm6375.c b/drivers/clk/qcom/gcc-sm6375.c
index 3dd15d765b22..84639d5b89bf 100644
--- a/drivers/clk/qcom/gcc-sm6375.c
+++ b/drivers/clk/qcom/gcc-sm6375.c
@@ -3882,13 +3882,10 @@ static int gcc_sm6375_probe(struct platform_device *pdev)
if (ret)
return ret;

- /*
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x17028); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x2b004); /* GCC_CPUSS_GNOC_CLK */
+ qcom_branch_set_clk_en(regmap, 0x1702c); /* GCC_DISP_XO_CLK */

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 d9983bb27475..47c25b3d95ad 100644
--- a/drivers/clk/qcom/gcc-sm7150.c
+++ b/drivers/clk/qcom/gcc-sm7150.c
@@ -3002,20 +3002,15 @@ static int gcc_sm7150_probe(struct platform_device *pdev)
regmap_update_bits(regmap, 0x4d110, 0x3, 0x3);
regmap_update_bits(regmap, 0x71028, 0x3, 0x3);

- /*
- * Keep the critical clocks always-ON
- * GCC_CPUSS_GNOC_CLK, GCC_VIDEO_AHB_CLK, GCC_CAMERA_AHB_CLK,
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x48004); /* GCC_CPUSS_GNOC_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b008); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b00c); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b02c); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b028); /* GCC_VIDEO_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b030); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */

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 c6c5261264f1..9990931aa172 100644
--- a/drivers/clk/qcom/gcc-sm8250.c
+++ b/drivers/clk/qcom/gcc-sm8250.c
@@ -3643,18 +3643,13 @@ static int gcc_sm8250_probe(struct platform_device *pdev)
regmap_update_bits(regmap, 0x4d110, 0x3, 0x3);
regmap_update_bits(regmap, 0x71028, 0x3, 0x3);

- /*
- * Keep the clocks always-ON
- * GCC_VIDEO_AHB_CLK, GCC_CAMERA_AHB_CLK, GCC_DISP_AHB_CLK,
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x0b004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b008); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x0b00c); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x4818c); /* GCC_CPUSS_DVM_BUS_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x52000); /* GCC_SYS_NOC_CPUSS_AHB_CLK */

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..e83a9facc19b 100644
--- a/drivers/clk/qcom/gcc-sm8350.c
+++ b/drivers/clk/qcom/gcc-sm8350.c
@@ -3806,18 +3806,14 @@ static int gcc_sm8350_probe(struct platform_device *pdev)
return PTR_ERR(regmap);
}

- /*
- * Keep the critical clock always-On
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x26004); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x26018); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27004); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x2701c); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x28004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x28020); /* GCC_VIDEO_XO_CLK */

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 563542982551..43e9c32921f3 100644
--- a/drivers/clk/qcom/gcc-sm8450.c
+++ b/drivers/clk/qcom/gcc-sm8450.c
@@ -3280,19 +3280,14 @@ static int gcc_sm8450_probe(struct platform_device *pdev)
/* FORCE_MEM_CORE_ON for ufs phy ice core clocks */
regmap_update_bits(regmap, gcc_ufs_phy_ice_core_clk.halt_reg, BIT(14), BIT(14));

- /*
- * Keep the critical clock always-On
- * 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, 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x36020); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x37004); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x3701c); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x81004); /* GCC_GPU_CFG_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x42004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x42028); /* GCC_VIDEO_XO_CLK */

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 b883dffe5f7a..60895648f281 100644
--- a/drivers/clk/qcom/gcc-sm8550.c
+++ b/drivers/clk/qcom/gcc-sm8550.c
@@ -3352,19 +3352,14 @@ static int gcc_sm8550_probe(struct platform_device *pdev)
/* FORCE_MEM_CORE_ON for ufs phy ice core clocks */
regmap_update_bits(regmap, gcc_ufs_phy_ice_core_clk.halt_reg, BIT(14), BIT(14));

- /*
- * Keep the critical clock always-On
- * 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, 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x26004); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x26028); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27004); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27018); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x32004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x32030); /* GCC_VIDEO_XO_CLK */

/* Clear GDSC_SLEEP_ENA_VOTE to stop votes being auto-removed in sleep. */
regmap_write(regmap, 0x52024, 0x0);
diff --git a/drivers/clk/qcom/gcc-sm8650.c b/drivers/clk/qcom/gcc-sm8650.c
index 9174dd82308c..c4a6540b1522 100644
--- a/drivers/clk/qcom/gcc-sm8650.c
+++ b/drivers/clk/qcom/gcc-sm8650.c
@@ -3808,14 +3808,14 @@ static int gcc_sm8650_probe(struct platform_device *pdev)
if (ret)
return ret;

- /* Keep the critical clock always-On */
- regmap_update_bits(regmap, 0x26004, BIT(0), BIT(0)); /* gcc_camera_ahb_clk */
- regmap_update_bits(regmap, 0x26028, BIT(0), BIT(0)); /* gcc_camera_xo_clk */
- regmap_update_bits(regmap, 0x27004, BIT(0), BIT(0)); /* gcc_disp_ahb_clk */
- regmap_update_bits(regmap, 0x27018, BIT(0), BIT(0)); /* gcc_disp_xo_clk */
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0)); /* gcc_gpu_cfg_ahb_clk */
- regmap_update_bits(regmap, 0x32004, BIT(0), BIT(0)); /* gcc_video_ahb_clk */
- regmap_update_bits(regmap, 0x32030, BIT(0), BIT(0)); /* gcc_video_xo_clk */
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x26004); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x26028); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27004); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27018); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x32004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x32030); /* GCC_VIDEO_XO_CLK */

qcom_branch_set_force_mem_core(regmap, gcc_ufs_phy_ice_core_clk, true);

diff --git a/drivers/clk/qcom/gcc-x1e80100.c b/drivers/clk/qcom/gcc-x1e80100.c
index d7182d6e9783..1404017be918 100644
--- a/drivers/clk/qcom/gcc-x1e80100.c
+++ b/drivers/clk/qcom/gcc-x1e80100.c
@@ -6769,14 +6769,14 @@ static int gcc_x1e80100_probe(struct platform_device *pdev)
if (ret)
return ret;

- /* Keep the critical clock always-On */
- regmap_update_bits(regmap, 0x26004, BIT(0), BIT(0)); /* gcc_camera_ahb_clk */
- regmap_update_bits(regmap, 0x26028, BIT(0), BIT(0)); /* gcc_camera_xo_clk */
- regmap_update_bits(regmap, 0x27004, BIT(0), BIT(0)); /* gcc_disp_ahb_clk */
- regmap_update_bits(regmap, 0x27018, BIT(0), BIT(0)); /* gcc_disp_xo_clk */
- regmap_update_bits(regmap, 0x32004, BIT(0), BIT(0)); /* gcc_video_ahb_clk */
- regmap_update_bits(regmap, 0x32030, BIT(0), BIT(0)); /* gcc_video_xo_clk */
- regmap_update_bits(regmap, 0x71004, BIT(0), BIT(0)); /* gcc_gpu_cfg_ahb_clk */
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x26004); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x26028); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27004); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x27018); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x32004); /* GCC_VIDEO_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x32030); /* GCC_VIDEO_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x71004); /* GCC_GPU_CFG_AHB_CLK */

/* Clear GDSC_SLEEP_ENA_VOTE to stop votes being auto-removed in sleep. */
regmap_write(regmap, 0x52224, 0x0);
diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
index 1490cd45a654..293b57080685 100644
--- a/drivers/clk/qcom/gpucc-sc7280.c
+++ b/drivers/clk/qcom/gpucc-sc7280.c
@@ -457,12 +457,9 @@ static int gpu_cc_sc7280_probe(struct platform_device *pdev)

clk_lucid_pll_configure(&gpu_cc_pll1, regmap, &gpu_cc_pll1_config);

- /*
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x1170); /* GPU_CC_CB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x1098); /* GPUCC_CX_GMU_CLK */
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..a8ea2143057d 100644
--- a/drivers/clk/qcom/gpucc-sc8280xp.c
+++ b/drivers/clk/qcom/gpucc-sc8280xp.c
@@ -444,12 +444,9 @@ static int gpu_cc_sc8280xp_probe(struct platform_device *pdev)
clk_lucid_pll_configure(&gpu_cc_pll0, regmap, &gpu_cc_pll0_config);
clk_lucid_pll_configure(&gpu_cc_pll1, regmap, &gpu_cc_pll1_config);

- /*
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x1170); /* GPU_CC_CB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x109c); /* GPU_CC_CXO_CLK */

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 2fa8673424d7..783d14ef9a0c 100644
--- a/drivers/clk/qcom/gpucc-sm8550.c
+++ b/drivers/clk/qcom/gpucc-sm8550.c
@@ -575,13 +575,9 @@ static int gpu_cc_sm8550_probe(struct platform_device *pdev)
clk_lucid_ole_pll_configure(&gpu_cc_pll0, regmap, &gpu_cc_pll0_config);
clk_lucid_ole_pll_configure(&gpu_cc_pll1, regmap, &gpu_cc_pll1_config);

- /*
- * Keep clocks always enabled:
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x9004); /* GPU_CC_CXO_AON_CLK */
+ qcom_branch_set_clk_en(regmap, 0x900c); /* GPU_CC_DEMET_CLK */

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 9051fd567112..fd9cd2e3f956 100644
--- a/drivers/clk/qcom/lpasscorecc-sc7180.c
+++ b/drivers/clk/qcom/lpasscorecc-sc7180.c
@@ -401,11 +401,8 @@ static int lpass_core_cc_sc7180_probe(struct platform_device *pdev)
goto exit;
}

- /*
- * Keep the CLK always-ON
- * LPASS_AUDIO_CORE_SYSNOC_SWAY_CORE_CLK
- */
- regmap_update_bits(regmap, 0x24000, BIT(0), BIT(0));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x24000); /* LPASS_AUDIO_CORE_SYSNOC_SWAY_CORE_CLK */

/* 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..6fd8666813a8 100644
--- a/drivers/clk/qcom/videocc-sm8250.c
+++ b/drivers/clk/qcom/videocc-sm8250.c
@@ -383,9 +383,9 @@ static int video_cc_sm8250_probe(struct platform_device *pdev)
clk_lucid_pll_configure(&video_pll0, regmap, &video_pll0_config);
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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0xe58); /* VIDEO_CC_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0xeec); /* VIDEO_CC_XO_CLK */

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 7246f3c99492..002e4c0666fa 100644
--- a/drivers/clk/qcom/videocc-sm8350.c
+++ b/drivers/clk/qcom/videocc-sm8350.c
@@ -558,13 +558,9 @@ static int video_cc_sm8350_probe(struct platform_device *pdev)
clk_lucid_pll_configure(&video_pll0, regmap, &video_pll0_config);
clk_lucid_pll_configure(&video_pll1, regmap, &video_pll1_config);

- /*
- * Keep clocks always enabled:
- * video_cc_ahb_clk
- * video_cc_xo_clk
- */
- regmap_update_bits(regmap, 0xe58, BIT(0), BIT(0));
- regmap_update_bits(regmap, video_cc_xo_clk_cbcr, BIT(0), BIT(0));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0xe58); /* VIDEO_CC_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, video_cc_xo_clk_cbcr); /* VIDEO_CC_XO_CLK */

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 16a61146e619..045eee07ea5f 100644
--- a/drivers/clk/qcom/videocc-sm8450.c
+++ b/drivers/clk/qcom/videocc-sm8450.c
@@ -423,15 +423,10 @@ static int video_cc_sm8450_probe(struct platform_device *pdev)
clk_lucid_evo_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
clk_lucid_evo_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);

- /*
- * Keep clocks always enabled:
- * video_cc_ahb_clk
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x80e4); /* VIDEO_CC_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x8130); /* VIDEO_CC_SLEEP_CLK */
+ qcom_branch_set_clk_en(regmap, 0x8114); /* VIDEO_CC_XO_CLK */

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 f3c9dfaee968..fa81e48f83e7 100644
--- a/drivers/clk/qcom/videocc-sm8550.c
+++ b/drivers/clk/qcom/videocc-sm8550.c
@@ -428,15 +428,10 @@ static int video_cc_sm8550_probe(struct platform_device *pdev)
clk_lucid_ole_pll_configure(&video_cc_pll0, regmap, &video_cc_pll0_config);
clk_lucid_ole_pll_configure(&video_cc_pll1, regmap, &video_cc_pll1_config);

- /*
- * Keep clocks always enabled:
- * video_cc_ahb_clk
- * 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));
+ /* Keep some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x80f4); /* VIDEO_CC_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x8140); /* VIDEO_CC_SLEEP_CLK */
+ qcom_branch_set_clk_en(regmap, 0x8124); /* VIDEO_CC_XO_CLK */

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


--
2.43.0


2024-01-13 14:52:41

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 06/12] clk: qcom: gpucc-sm6115: Add runtime PM

The GPU_CC block on SM6115 is powered by the VDD_CX rail. We only need
to cast an enable vote for it if the GPU blocks are in use.

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 2c2c184747b1..15cf5d63c9ad 100644
--- a/drivers/clk/qcom/gpucc-sm6115.c
+++ b/drivers/clk/qcom/gpucc-sm6115.c
@@ -8,6 +8,7 @@
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>

#include <dt-bindings/clock/qcom,sm6115-gpucc.h>
@@ -443,10 +444,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);
@@ -462,7 +474,10 @@ static int gpu_cc_sm6115_probe(struct platform_device *pdev)
qcom_branch_set_clk_en(regmap, 0x1078); /* GPU_CC_AHB_CLK */
qcom_branch_set_clk_en(regmap, 0x1060); /* GPU_CC_GX_CXO_CLK */

- 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.43.0


2024-01-13 14:53:07

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 07/12] 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 | 124 +++---------------------------------------
1 file changed, 9 insertions(+), 115 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sm6115.c b/drivers/clk/qcom/gcc-sm6115.c
index 13e521cd4259..99130508d281 100644
--- a/drivers/clk/qcom/gcc-sm6115.c
+++ b/drivers/clk/qcom/gcc-sm6115.c
@@ -1586,36 +1586,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,
@@ -2124,38 +2094,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,
@@ -2215,20 +2153,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,
@@ -2283,22 +2207,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 = {
@@ -2771,22 +2679,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,
@@ -3272,8 +3164,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,
@@ -3322,20 +3212,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,
@@ -3376,7 +3262,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,
@@ -3513,6 +3398,15 @@ 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 some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x17008); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x1700c); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x17028); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x1702c); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x2b004); /* GCC_CPUSS_GNOC_CLK */
+ qcom_branch_set_clk_en(regmap, 0x2b06c); /* GCC_SYS_NOC_CPUSS_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_GPU_CFG_AHB_CLK */
+
return qcom_cc_really_probe(pdev, &gcc_sm6115_desc, regmap);
}


--
2.43.0


2024-01-13 14:53:23

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 08/12] 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 | 106 ++++-------------------------------------
1 file changed, 8 insertions(+), 98 deletions(-)

diff --git a/drivers/clk/qcom/gcc-qcm2290.c b/drivers/clk/qcom/gcc-qcm2290.c
index 48995e50c6bd..fd20408d8205 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,14 @@ 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 some clocks always-on */
+ qcom_branch_set_clk_en(regmap, 0x17008); /* GCC_CAMERA_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x1700c); /* GCC_DISP_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x17028); /* GCC_CAMERA_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x1702c); /* GCC_DISP_XO_CLK */
+ qcom_branch_set_clk_en(regmap, 0x2b06c); /* GCC_SYS_NOC_CPUSS_AHB_CLK */
+ qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_GPU_CFG_AHB_CLK */
+
return qcom_cc_really_probe(pdev, &gcc_qcm2290_desc, regmap);
}


--
2.43.0


2024-01-13 14:53:53

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 09/12] arm64: dts: qcom: sm6375: 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/sm6375.dtsi | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sm6375.dtsi b/arch/arm64/boot/dts/qcom/sm6375.dtsi
index 7ac8bf26dda3..f578d110f36b 100644
--- a/arch/arm64/boot/dts/qcom/sm6375.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6375.dtsi
@@ -954,6 +954,7 @@ gcc: clock-controller@1400000 {
clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
<&rpmcc RPM_SMD_XO_A_CLK_SRC>,
<&sleep_clk>;
+ power-domains = <&rpmpd SM6375_VDDCX>;
#power-domain-cells = <1>;
#clock-cells = <1>;
#reset-cells = <1>;

--
2.43.0


2024-01-13 14:54:02

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 10/12] 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 0911fb08ed63..51b05019ee25 100644
--- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
@@ -647,6 +647,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.43.0


2024-01-13 14:54:23

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 11/12] 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 160e098f1075..30b140e1cec0 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -807,6 +807,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.43.0


2024-01-13 14:54:41

by Konrad Dybcio

[permalink] [raw]
Subject: [PATCH v6 12/12] arm64: dts: qcom: sm6115: Add VDD_CX to GPU_CC

The GPU_CC block is powered by VDD_CX. Link the power domain and
provide a reasonable minimum vote (lowest available on the platform)
to ensure the registers within are accessible.

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

diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi
index 30b140e1cec0..ec9a74acc69c 100644
--- a/arch/arm64/boot/dts/qcom/sm6115.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6115.dtsi
@@ -1723,6 +1723,8 @@ gpucc: clock-controller@5990000 {
clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
<&gcc GCC_GPU_GPLL0_CLK_SRC>,
<&gcc GCC_GPU_GPLL0_DIV_CLK_SRC>;
+ power-domains = <&rpmpd SM6115_VDDCX>;
+ required-opps = <&rpmpd_opp_low_svs>;
#clock-cells = <1>;
#reset-cells = <1>;
#power-domain-cells = <1>;

--
2.43.0


2024-01-13 21:19:09

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v6 01/12] clk: qcom: branch: Add a helper for setting the enable bit

On Sat, 13 Jan 2024 at 16:51, Konrad Dybcio <[email protected]> wrote:
>
> We hardcode some clocks to be always-on, as they're essential to the
> functioning of the SoC / some peripherals. Add a helper to do so
> to make the writes less magic.
>
> Reviewed-by: Johan Hovold <[email protected]>
> Reviewed-by: Bryan O'Donoghue <[email protected]>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/clk/qcom/clk-branch.h | 7 +++++++
> 1 file changed, 7 insertions(+)

Reviewed-by: Dmitry Baryshkov <[email protected]>


--
With best wishes
Dmitry

2024-01-13 22:13:43

by Dmitry Baryshkov

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

On Sat, 13 Jan 2024 at 16:51, Konrad Dybcio <[email protected]> wrote:
>
> Instead of magically poking at the bit0 of branch clocks' CBCR, use
> the newly introduced helper.
>
> Reviewed-by: Bryan O'Donoghue <[email protected]>
> Reviewed-by: Johan Hovold <[email protected]>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/clk/qcom/camcc-sc8280xp.c | 6 ++----
> drivers/clk/qcom/camcc-sm8550.c | 10 +++-------
> drivers/clk/qcom/dispcc-qcm2290.c | 4 ++--
> drivers/clk/qcom/dispcc-sc7280.c | 7 ++-----
> drivers/clk/qcom/dispcc-sc8280xp.c | 4 ++--
> drivers/clk/qcom/dispcc-sm6115.c | 4 ++--
> drivers/clk/qcom/dispcc-sm8250.c | 4 ++--
> drivers/clk/qcom/dispcc-sm8450.c | 7 ++-----
> drivers/clk/qcom/dispcc-sm8550.c | 7 ++-----
> drivers/clk/qcom/dispcc-sm8650.c | 4 ++--
> drivers/clk/qcom/gcc-sa8775p.c | 25 ++++++++++---------------
> drivers/clk/qcom/gcc-sc7180.c | 22 +++++++++-------------
> drivers/clk/qcom/gcc-sc7280.c | 20 ++++++++------------
> drivers/clk/qcom/gcc-sc8180x.c | 28 +++++++++++-----------------
> drivers/clk/qcom/gcc-sc8280xp.c | 25 ++++++++++---------------
> drivers/clk/qcom/gcc-sdx55.c | 12 ++++--------
> drivers/clk/qcom/gcc-sdx65.c | 13 +++++--------
> drivers/clk/qcom/gcc-sdx75.c | 10 +++-------
> drivers/clk/qcom/gcc-sm4450.c | 28 +++++++++-------------------
> drivers/clk/qcom/gcc-sm6375.c | 11 ++++-------
> drivers/clk/qcom/gcc-sm7150.c | 23 +++++++++--------------
> drivers/clk/qcom/gcc-sm8250.c | 19 +++++++------------
> drivers/clk/qcom/gcc-sm8350.c | 20 ++++++++------------
> drivers/clk/qcom/gcc-sm8450.c | 21 ++++++++-------------
> drivers/clk/qcom/gcc-sm8550.c | 21 ++++++++-------------
> drivers/clk/qcom/gcc-sm8650.c | 16 ++++++++--------
> drivers/clk/qcom/gcc-x1e80100.c | 16 ++++++++--------
> drivers/clk/qcom/gpucc-sc7280.c | 9 +++------
> drivers/clk/qcom/gpucc-sc8280xp.c | 9 +++------
> drivers/clk/qcom/gpucc-sm8550.c | 10 +++-------
> drivers/clk/qcom/lpasscorecc-sc7180.c | 7 ++-----
> drivers/clk/qcom/videocc-sm8250.c | 6 +++---
> drivers/clk/qcom/videocc-sm8350.c | 10 +++-------
> drivers/clk/qcom/videocc-sm8450.c | 13 ++++---------
> drivers/clk/qcom/videocc-sm8550.c | 13 ++++---------
> 35 files changed, 175 insertions(+), 289 deletions(-)

Reviewed-by: Dmitry Baryshkov <[email protected]>


--
With best wishes
Dmitry

2024-01-14 04:54:20

by Dmitry Baryshkov

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

On Sat, 13 Jan 2024 at 16:51, Konrad Dybcio <[email protected]> wrote:
>
> 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.

Probably it is out of scope for this
I wonder if it makes sense to route (some) of the clocks properly.
Should we use GCC_foo_SLEEEP_CLK as a sleep clock for the
corresponding device?
I'm not sure about the AHB and XO clocks.

Another question is regarding the suspended state. Wouldn't leaving
GCC_foo_XO clocks enabled keep the XO enabled as well?

>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> Changes in v6:
> - Rebase (next-20240112)
> - Reorder qcom_branch_set_clk_en calls by register in "*: Unregister
> critical clocks" (Johan)
> - Pick up tags
> - Link to v5: https://lore.kernel.org/r/[email protected]
>
> Changes in v5:
> - Change the "Keep the critical clocks always-on" comment to "Keep
> some clocks always-on"
> - Add the same comment to commits unregistering clocks on 6115/6375/2290
> - Link to v4: https://lore.kernel.org/r/[email protected]
>
> Changes in v4:
> - Add and unify the "/* Keep the critical clocks always-on */" comment
> - Rebase (next-20231222), also include 8650, X1E and 8280camcc drivers
> - Drop enabling runtime PM on GCC
> - Improve the commit message of "clk: qcom: gpucc-sm6115: Add runtime PM"
> - Link to v3: https://lore.kernel.org/r/[email protected]
>
> Changes in v3:
> - Rebase (next-20231219)
> - Fix up a copypaste mistake in "gcc-sm6375: Unregister critical clocks" (bod)
> - Pick up tags
> - Link to v2: https://lore.kernel.org/r/[email protected]
>
> Changes in v2:
> - Rebase
> - Pick up tags
> - Fix up missing pm_runtime_put in SM6375 GCC (Johan)
> - Clarify the commit message of "Add runtime PM" commits (Johan)
> - "GPU_CCC" -> "GPU_CC" (oops)
> - Rebase atop next-20231129
> - Also fix up camcc-sm8550 & gcc-sm4450
> - Unify and clean up the comment style
> - Fix missing comments in gcc-sc7180..
> - Drop Johan's ack from "clk: qcom: Use qcom_branch_set_clk_en()"
> - Improve 6115 dt patch commit message (Bjorn)
> - Link to v1: https://lore.kernel.org/r/[email protected]
>
> ---
> Konrad Dybcio (12):
> 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: 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-qcm2290: Unregister critical clocks
> 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_CC
>
> 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/camcc-sc8280xp.c | 6 +-
> drivers/clk/qcom/camcc-sm8550.c | 10 +--
> drivers/clk/qcom/clk-branch.h | 7 ++
> drivers/clk/qcom/dispcc-qcm2290.c | 4 +-
> drivers/clk/qcom/dispcc-sc7280.c | 7 +-
> drivers/clk/qcom/dispcc-sc8280xp.c | 4 +-
> drivers/clk/qcom/dispcc-sm6115.c | 4 +-
> drivers/clk/qcom/dispcc-sm8250.c | 4 +-
> drivers/clk/qcom/dispcc-sm8450.c | 7 +-
> drivers/clk/qcom/dispcc-sm8550.c | 7 +-
> drivers/clk/qcom/dispcc-sm8650.c | 4 +-
> drivers/clk/qcom/gcc-qcm2290.c | 106 +++--------------------------
> drivers/clk/qcom/gcc-sa8775p.c | 25 +++----
> drivers/clk/qcom/gcc-sc7180.c | 22 +++---
> drivers/clk/qcom/gcc-sc7280.c | 20 +++---
> drivers/clk/qcom/gcc-sc8180x.c | 28 +++-----
> drivers/clk/qcom/gcc-sc8280xp.c | 25 +++----
> drivers/clk/qcom/gcc-sdx55.c | 12 ++--
> drivers/clk/qcom/gcc-sdx65.c | 13 ++--
> drivers/clk/qcom/gcc-sdx75.c | 10 +--
> drivers/clk/qcom/gcc-sm4450.c | 28 +++-----
> drivers/clk/qcom/gcc-sm6115.c | 124 +++-------------------------------
> drivers/clk/qcom/gcc-sm6375.c | 105 +++-------------------------
> drivers/clk/qcom/gcc-sm7150.c | 23 +++----
> drivers/clk/qcom/gcc-sm8250.c | 19 ++----
> drivers/clk/qcom/gcc-sm8350.c | 20 +++---
> drivers/clk/qcom/gcc-sm8450.c | 21 +++---
> drivers/clk/qcom/gcc-sm8550.c | 21 +++---
> drivers/clk/qcom/gcc-sm8650.c | 16 ++---
> drivers/clk/qcom/gcc-x1e80100.c | 16 ++---
> drivers/clk/qcom/gpucc-sc7280.c | 9 +--
> drivers/clk/qcom/gpucc-sc8280xp.c | 9 +--
> drivers/clk/qcom/gpucc-sm6115.c | 53 ++++++---------
> drivers/clk/qcom/gpucc-sm6375.c | 34 ++--------
> drivers/clk/qcom/gpucc-sm8550.c | 10 +--
> drivers/clk/qcom/lpasscorecc-sc7180.c | 7 +-
> drivers/clk/qcom/videocc-sm8250.c | 6 +-
> drivers/clk/qcom/videocc-sm8350.c | 10 +--
> drivers/clk/qcom/videocc-sm8450.c | 13 ++--
> drivers/clk/qcom/videocc-sm8550.c | 13 ++--
> 43 files changed, 234 insertions(+), 653 deletions(-)
> ---
> base-commit: 8d04a7e2ee3fd6aabb8096b00c64db0d735bc874
> change-id: 20230717-topic-branch_aon_cleanup-6976c13fe71c
>
> Best regards,
> --
> Konrad Dybcio <[email protected]>
>
>


--
With best wishes
Dmitry

2024-01-15 09:29:28

by Konrad Dybcio

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

On 14.01.2024 05:53, Dmitry Baryshkov wrote:
> On Sat, 13 Jan 2024 at 16:51, Konrad Dybcio <[email protected]> wrote:
>>
>> 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.
>
> Probably it is out of scope for this
> I wonder if it makes sense to route (some) of the clocks properly.
> Should we use GCC_foo_SLEEEP_CLK as a sleep clock for the
> corresponding device?
> I'm not sure about the AHB and XO clocks.
>
> Another question is regarding the suspended state. Wouldn't leaving
> GCC_foo_XO clocks enabled keep the XO enabled as well?

Doesn't seem to be the case

Konrad

2024-01-16 11:09:26

by Johan Hovold

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

On Sat, Jan 13, 2024 at 03:50:52PM +0100, Konrad Dybcio wrote:

> /* Keep some clocks always-on */
> + qcom_branch_set_clk_en(regmap, 0x17008); /* GCC_CAMERA_AHB_CLK */
> + qcom_branch_set_clk_en(regmap, 0x17004); /* GCC_VIDEO_AHB_CLK */

These two are not in sort order (haven't checked the rest of the
patches).

> + qcom_branch_set_clk_en(regmap, 0x1700c); /* GCC_DISP_AHB_CLK */
> qcom_branch_set_clk_en(regmap, 0x17028); /* GCC_CAMERA_XO_CLK */
> - qcom_branch_set_clk_en(regmap, 0x2b004); /* GCC_CPUSS_GNOC_CLK */
> qcom_branch_set_clk_en(regmap, 0x1702c); /* GCC_DISP_XO_CLK */
> + qcom_branch_set_clk_en(regmap, 0x2b004); /* GCC_CPUSS_GNOC_CLK */
> + qcom_branch_set_clk_en(regmap, 0x36004); /* GCC_GPU_CFG_AHB_CLK */
> + qcom_branch_set_clk_en(regmap, 0x79004); /* GCC_SYS_NOC_CPUSS_AHB_CLK */

Johan

2024-01-23 09:34:46

by Imran Shaik

[permalink] [raw]
Subject: Re: [PATCH v6 01/12] clk: qcom: branch: Add a helper for setting the enable bit



On 1/13/2024 8:20 PM, Konrad Dybcio wrote:
> We hardcode some clocks to be always-on, as they're essential to the
> functioning of the SoC / some peripherals. Add a helper to do so
> to make the writes less magic.
>
> Reviewed-by: Johan Hovold <[email protected]>
> Reviewed-by: Bryan O'Donoghue <[email protected]>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/clk/qcom/clk-branch.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h
> index 8ffed603c050..0514bc43100b 100644
> --- a/drivers/clk/qcom/clk-branch.h
> +++ b/drivers/clk/qcom/clk-branch.h
> @@ -64,6 +64,7 @@ struct clk_mem_branch {
> #define CBCR_FORCE_MEM_PERIPH_OFF BIT(12)
> #define CBCR_WAKEUP GENMASK(11, 8)
> #define CBCR_SLEEP GENMASK(7, 4)
> +#define CBCR_CLOCK_ENABLE BIT(0)
>
> static inline void qcom_branch_set_force_mem_core(struct regmap *regmap,
> struct clk_branch clk, bool on)
> @@ -98,6 +99,12 @@ static inline void qcom_branch_set_sleep(struct regmap *regmap, struct clk_branc
> FIELD_PREP(CBCR_SLEEP, val));
> }
>
> +static inline void qcom_branch_set_clk_en(struct regmap *regmap, u32 cbcr)
> +{
> + regmap_update_bits(regmap, cbcr, CBCR_CLOCK_ENABLE,
> + CBCR_CLOCK_ENABLE);
> +}
> +

Could you please help me understand how this helper function is useful?
Seems like this is just for reducing parameters compared to
regmap_update_bits(). But anyhow the same is being done in the existing
clock controller drivers with a comment which explains the functionality.

Thanks & Regards,
Imran

> extern const struct clk_ops clk_branch_ops;
> extern const struct clk_ops clk_branch2_ops;
> extern const struct clk_ops clk_branch_simple_ops;
>

2024-01-24 07:44:07

by Abel Vesa

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

On 24-01-13 15:50:49, Konrad Dybcio wrote:
> 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.

Generally speaking, HW-wise, if the power domain of a clock controller
is being disabled, all clocks that it provides are being disabled.

Are you saying that is not the case ?

>
> 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]>
> ---
> Changes in v6:
> - Rebase (next-20240112)
> - Reorder qcom_branch_set_clk_en calls by register in "*: Unregister
> critical clocks" (Johan)
> - Pick up tags
> - Link to v5: https://lore.kernel.org/r/[email protected]
>
> Changes in v5:
> - Change the "Keep the critical clocks always-on" comment to "Keep
> some clocks always-on"
> - Add the same comment to commits unregistering clocks on 6115/6375/2290
> - Link to v4: https://lore.kernel.org/r/[email protected]
>
> Changes in v4:
> - Add and unify the "/* Keep the critical clocks always-on */" comment
> - Rebase (next-20231222), also include 8650, X1E and 8280camcc drivers
> - Drop enabling runtime PM on GCC
> - Improve the commit message of "clk: qcom: gpucc-sm6115: Add runtime PM"
> - Link to v3: https://lore.kernel.org/r/[email protected]
>
> Changes in v3:
> - Rebase (next-20231219)
> - Fix up a copypaste mistake in "gcc-sm6375: Unregister critical clocks" (bod)
> - Pick up tags
> - Link to v2: https://lore.kernel.org/r/[email protected]
>
> Changes in v2:
> - Rebase
> - Pick up tags
> - Fix up missing pm_runtime_put in SM6375 GCC (Johan)
> - Clarify the commit message of "Add runtime PM" commits (Johan)
> - "GPU_CCC" -> "GPU_CC" (oops)
> - Rebase atop next-20231129
> - Also fix up camcc-sm8550 & gcc-sm4450
> - Unify and clean up the comment style
> - Fix missing comments in gcc-sc7180..
> - Drop Johan's ack from "clk: qcom: Use qcom_branch_set_clk_en()"
> - Improve 6115 dt patch commit message (Bjorn)
> - Link to v1: https://lore.kernel.org/r/[email protected]
>
> ---
> Konrad Dybcio (12):
> 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: 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-qcm2290: Unregister critical clocks
> 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_CC
>
> 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/camcc-sc8280xp.c | 6 +-
> drivers/clk/qcom/camcc-sm8550.c | 10 +--
> drivers/clk/qcom/clk-branch.h | 7 ++
> drivers/clk/qcom/dispcc-qcm2290.c | 4 +-
> drivers/clk/qcom/dispcc-sc7280.c | 7 +-
> drivers/clk/qcom/dispcc-sc8280xp.c | 4 +-
> drivers/clk/qcom/dispcc-sm6115.c | 4 +-
> drivers/clk/qcom/dispcc-sm8250.c | 4 +-
> drivers/clk/qcom/dispcc-sm8450.c | 7 +-
> drivers/clk/qcom/dispcc-sm8550.c | 7 +-
> drivers/clk/qcom/dispcc-sm8650.c | 4 +-
> drivers/clk/qcom/gcc-qcm2290.c | 106 +++--------------------------
> drivers/clk/qcom/gcc-sa8775p.c | 25 +++----
> drivers/clk/qcom/gcc-sc7180.c | 22 +++---
> drivers/clk/qcom/gcc-sc7280.c | 20 +++---
> drivers/clk/qcom/gcc-sc8180x.c | 28 +++-----
> drivers/clk/qcom/gcc-sc8280xp.c | 25 +++----
> drivers/clk/qcom/gcc-sdx55.c | 12 ++--
> drivers/clk/qcom/gcc-sdx65.c | 13 ++--
> drivers/clk/qcom/gcc-sdx75.c | 10 +--
> drivers/clk/qcom/gcc-sm4450.c | 28 +++-----
> drivers/clk/qcom/gcc-sm6115.c | 124 +++-------------------------------
> drivers/clk/qcom/gcc-sm6375.c | 105 +++-------------------------
> drivers/clk/qcom/gcc-sm7150.c | 23 +++----
> drivers/clk/qcom/gcc-sm8250.c | 19 ++----
> drivers/clk/qcom/gcc-sm8350.c | 20 +++---
> drivers/clk/qcom/gcc-sm8450.c | 21 +++---
> drivers/clk/qcom/gcc-sm8550.c | 21 +++---
> drivers/clk/qcom/gcc-sm8650.c | 16 ++---
> drivers/clk/qcom/gcc-x1e80100.c | 16 ++---
> drivers/clk/qcom/gpucc-sc7280.c | 9 +--
> drivers/clk/qcom/gpucc-sc8280xp.c | 9 +--
> drivers/clk/qcom/gpucc-sm6115.c | 53 ++++++---------
> drivers/clk/qcom/gpucc-sm6375.c | 34 ++--------
> drivers/clk/qcom/gpucc-sm8550.c | 10 +--
> drivers/clk/qcom/lpasscorecc-sc7180.c | 7 +-
> drivers/clk/qcom/videocc-sm8250.c | 6 +-
> drivers/clk/qcom/videocc-sm8350.c | 10 +--
> drivers/clk/qcom/videocc-sm8450.c | 13 ++--
> drivers/clk/qcom/videocc-sm8550.c | 13 ++--
> 43 files changed, 234 insertions(+), 653 deletions(-)
> ---
> base-commit: 8d04a7e2ee3fd6aabb8096b00c64db0d735bc874
> change-id: 20230717-topic-branch_aon_cleanup-6976c13fe71c
>
> Best regards,
> --
> Konrad Dybcio <[email protected]>
>
>

2024-01-24 11:21:42

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v6 01/12] clk: qcom: branch: Add a helper for setting the enable bit

On Tue, 23 Jan 2024 at 11:33, Imran Shaik <[email protected]> wrote:
>
>
>
> On 1/13/2024 8:20 PM, Konrad Dybcio wrote:
> > We hardcode some clocks to be always-on, as they're essential to the
> > functioning of the SoC / some peripherals. Add a helper to do so
> > to make the writes less magic.
> >
> > Reviewed-by: Johan Hovold <[email protected]>
> > Reviewed-by: Bryan O'Donoghue <[email protected]>
> > Signed-off-by: Konrad Dybcio <[email protected]>
> > ---
> > drivers/clk/qcom/clk-branch.h | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h
> > index 8ffed603c050..0514bc43100b 100644
> > --- a/drivers/clk/qcom/clk-branch.h
> > +++ b/drivers/clk/qcom/clk-branch.h
> > @@ -64,6 +64,7 @@ struct clk_mem_branch {
> > #define CBCR_FORCE_MEM_PERIPH_OFF BIT(12)
> > #define CBCR_WAKEUP GENMASK(11, 8)
> > #define CBCR_SLEEP GENMASK(7, 4)
> > +#define CBCR_CLOCK_ENABLE BIT(0)
> >
> > static inline void qcom_branch_set_force_mem_core(struct regmap *regmap,
> > struct clk_branch clk, bool on)
> > @@ -98,6 +99,12 @@ static inline void qcom_branch_set_sleep(struct regmap *regmap, struct clk_branc
> > FIELD_PREP(CBCR_SLEEP, val));
> > }
> >
> > +static inline void qcom_branch_set_clk_en(struct regmap *regmap, u32 cbcr)
> > +{
> > + regmap_update_bits(regmap, cbcr, CBCR_CLOCK_ENABLE,
> > + CBCR_CLOCK_ENABLE);
> > +}
> > +
>
> Could you please help me understand how this helper function is useful?
> Seems like this is just for reducing parameters compared to
> regmap_update_bits(). But anyhow the same is being done in the existing
> clock controller drivers with a comment which explains the functionality.

So, yes, it replaces the boilerplate code with API, which is good.

>
> Thanks & Regards,
> Imran
>
> > extern const struct clk_ops clk_branch_ops;
> > extern const struct clk_ops clk_branch2_ops;
> > extern const struct clk_ops clk_branch_simple_ops;
> >
>


--
With best wishes
Dmitry

2024-01-24 12:32:37

by Konrad Dybcio

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



On 1/24/24 08:41, Abel Vesa wrote:
> On 24-01-13 15:50:49, Konrad Dybcio wrote:
>> 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.
>
> Generally speaking, HW-wise, if the power domain of a clock controller
> is being disabled, all clocks that it provides are being disabled.

Generally speaking, if that's the case, that's true.

>
> Are you saying that is not the case ?

Dragons however, are peculiar creatures and it seems like the clock
controllers are not *really* disabled when we think they are,
e.g. due to RPM(h) owning a share of GCC clocks, or due to the
MX rail being always-on. It would indeed be an issue with
hibernation where the registers would need to be reprogrammed
after battery power is removed.

As we spoke off-list, I'll split this series into two: adding
common helpers and then taking care of 2290/6375/6115.

I'm not yet sure how far we can go with converting existing clock
drivers to use pm_clk_add so that the _AHB, _XO, and _SLEEP clocks
for a given subsystem are only enabled when necessary - if we
require an entry in clock-names, backwards compatibility goes away,
and if we don't - we potentially miss out on a devlink between X_CC
and GCC, plus the name needs to be hardcoded for global parent lookup.

For new drivers, we'll likely just require a clean solution (runtime
PM enabled + subsys clocks gotten as pm_clk through a dt entry on
the consumer).


Konrad

2024-01-25 10:18:34

by Taniya Das

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

Hi Konrad,

Thanks for your patch.

On 1/13/2024 8:20 PM, Konrad Dybcio wrote:
> 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.

I am really curious to know a little more about the SoC-Wide Suspend not
happening on these targets. Could you add more details here ?

The Resource Power Manager (RPM) is the main aggregator on these targets
where the active & sleep votes on XO, shared rails (CX/MX) decide the
SoC wide suspend. The High Level OS on our internal platforms never had
any suspend issues due to clocks(GCC/GPUCC) or shared rails being kept
enabled from the consumers.

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

As CX is a shared resource/rail on these specific targets we definitely
do not achieve any power saving with the runtime pm attached to these
clock controllers, but I see a little more SW overhead. Though you could
please add your observations/comments.

Removing the CLK_IS_CRITICAL is a good cleanup and moving them to probe
is a good way to handle the always-on clocks.


> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> Changes in v6:
> - Rebase (next-20240112)
> - Reorder qcom_branch_set_clk_en calls by register in "*: Unregister
> critical clocks" (Johan)
> - Pick up tags
> - Link to v5: https://lore.kernel.org/r/[email protected]
>
> Changes in v5:
> - Change the "Keep the critical clocks always-on" comment to "Keep
> some clocks always-on"
> - Add the same comment to commits unregistering clocks on 6115/6375/2290
> - Link to v4: https://lore.kernel.org/r/[email protected]
>
> Changes in v4:
> - Add and unify the "/* Keep the critical clocks always-on */" comment
> - Rebase (next-20231222), also include 8650, X1E and 8280camcc drivers
> - Drop enabling runtime PM on GCC
> - Improve the commit message of "clk: qcom: gpucc-sm6115: Add runtime PM"
> - Link to v3: https://lore.kernel.org/r/[email protected]
>
> Changes in v3:
> - Rebase (next-20231219)
> - Fix up a copypaste mistake in "gcc-sm6375: Unregister critical clocks" (bod)
> - Pick up tags
> - Link to v2: https://lore.kernel.org/r/[email protected]
>
> Changes in v2:
> - Rebase
> - Pick up tags
> - Fix up missing pm_runtime_put in SM6375 GCC (Johan)
> - Clarify the commit message of "Add runtime PM" commits (Johan)
> - "GPU_CCC" -> "GPU_CC" (oops)
> - Rebase atop next-20231129
> - Also fix up camcc-sm8550 & gcc-sm4450
> - Unify and clean up the comment style
> - Fix missing comments in gcc-sc7180..
> - Drop Johan's ack from "clk: qcom: Use qcom_branch_set_clk_en()"
> - Improve 6115 dt patch commit message (Bjorn)
> - Link to v1: https://lore.kernel.org/r/[email protected]
>
> ---
> Konrad Dybcio (12):
> 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: 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-qcm2290: Unregister critical clocks
> 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_CC
> > 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/camcc-sc8280xp.c | 6 +-
> drivers/clk/qcom/camcc-sm8550.c | 10 +--
> drivers/clk/qcom/clk-branch.h | 7 ++
> drivers/clk/qcom/dispcc-qcm2290.c | 4 +-
> drivers/clk/qcom/dispcc-sc7280.c | 7 +-
> drivers/clk/qcom/dispcc-sc8280xp.c | 4 +-
> drivers/clk/qcom/dispcc-sm6115.c | 4 +-
> drivers/clk/qcom/dispcc-sm8250.c | 4 +-
> drivers/clk/qcom/dispcc-sm8450.c | 7 +-
> drivers/clk/qcom/dispcc-sm8550.c | 7 +-
> drivers/clk/qcom/dispcc-sm8650.c | 4 +-
> drivers/clk/qcom/gcc-qcm2290.c | 106 +++--------------------------
> drivers/clk/qcom/gcc-sa8775p.c | 25 +++----
> drivers/clk/qcom/gcc-sc7180.c | 22 +++---
> drivers/clk/qcom/gcc-sc7280.c | 20 +++---
> drivers/clk/qcom/gcc-sc8180x.c | 28 +++-----
> drivers/clk/qcom/gcc-sc8280xp.c | 25 +++----
> drivers/clk/qcom/gcc-sdx55.c | 12 ++--
> drivers/clk/qcom/gcc-sdx65.c | 13 ++--
> drivers/clk/qcom/gcc-sdx75.c | 10 +--
> drivers/clk/qcom/gcc-sm4450.c | 28 +++-----
> drivers/clk/qcom/gcc-sm6115.c | 124 +++-------------------------------
> drivers/clk/qcom/gcc-sm6375.c | 105 +++-------------------------
> drivers/clk/qcom/gcc-sm7150.c | 23 +++----
> drivers/clk/qcom/gcc-sm8250.c | 19 ++----
> drivers/clk/qcom/gcc-sm8350.c | 20 +++---
> drivers/clk/qcom/gcc-sm8450.c | 21 +++---
> drivers/clk/qcom/gcc-sm8550.c | 21 +++---
> drivers/clk/qcom/gcc-sm8650.c | 16 ++---
> drivers/clk/qcom/gcc-x1e80100.c | 16 ++---
> drivers/clk/qcom/gpucc-sc7280.c | 9 +--
> drivers/clk/qcom/gpucc-sc8280xp.c | 9 +--
> drivers/clk/qcom/gpucc-sm6115.c | 53 ++++++---------
> drivers/clk/qcom/gpucc-sm6375.c | 34 ++--------
> drivers/clk/qcom/gpucc-sm8550.c | 10 +--
> drivers/clk/qcom/lpasscorecc-sc7180.c | 7 +-
> drivers/clk/qcom/videocc-sm8250.c | 6 +-
> drivers/clk/qcom/videocc-sm8350.c | 10 +--
> drivers/clk/qcom/videocc-sm8450.c | 13 ++--
> drivers/clk/qcom/videocc-sm8550.c | 13 ++--
> 43 files changed, 234 insertions(+), 653 deletions(-)
> ---
> base-commit: 8d04a7e2ee3fd6aabb8096b00c64db0d735bc874
> change-id: 20230717-topic-branch_aon_cleanup-6976c13fe71c
>
> Best regards,

--
Thanks & Regards,
Taniya Das.

2024-01-25 11:18:42

by Konrad Dybcio

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



On 1/25/24 11:16, Taniya Das wrote:
> Hi Konrad,
>
> Thanks for your patch.
>
> On 1/13/2024 8:20 PM, Konrad Dybcio wrote:
>> 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.
>
> I am really curious to know a little more about the SoC-Wide Suspend not happening on these targets. Could you add more details here ?
>
> The Resource Power Manager (RPM) is the main aggregator on these targets where the active & sleep votes on XO, shared rails (CX/MX) decide the SoC wide suspend. The High Level OS on our internal platforms never had any suspend issues due to clocks(GCC/GPUCC) or shared rails being kept enabled from the consumers.

With the common clock framework, CLK_IS_CRITICAL blocks pm operations, as
clk_disable fails at some point. Since RPM(h)PDs are modeled as pmdomains,
this in turn results in them never getting disabled, leading to outstanding
votes. Then, RPM(h) sees these votes and (among other things which are not
properly described on most SoCs leading to dangling votes) decides that
CXPD/XOSD/AOSD can't be entered because there's a request on a resource.

>
>>
>> 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.
>>
>
> As CX is a shared resource/rail on these specific targets we definitely do not achieve any power saving with the runtime pm attached to these clock controllers, but I see a little more SW overhead. Though you could please add your observations/comments.

Hm, simply adding a power-domains entry to denote the required
performance state values when voting for downstream GDSCs would
be enough and runtime PM only makes sense if there's an additional
rail, say MMCX or GFX. But see the comment below.

>
> Removing the CLK_IS_CRITICAL is a good cleanup and moving them to probe is a good way to handle the always-on clocks.

Unless it turns out to be really messy with keeping backwards DTS
compatibilty, the goal is to use the pm_clk APIs to only keep the
interface/xo/sleep clocks for subsystems active when that subsystem
is active (i.e. when SUBSYS_CC is *runtime-active*).

This will result in a treewide patchset.

Konrad