The GCC block on SC8180X is powered by the CX rail. We need to ensure
that it's enabled to prevent unwanted power collapse.
Enable runtime PM to keep the power flowing only when necessary.
Signed-off-by: Val Packett <val@packett.cool>
---
drivers/clk/qcom/gcc-sc8180x.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index 365943cd5278..073fb1e2b302 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -9,6 +9,7 @@
#include <linux/err.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/pm_runtime.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -4676,9 +4677,19 @@ static int gcc_sc8180x_probe(struct platform_device *pdev)
struct regmap *regmap;
int ret;
+ ret = devm_pm_runtime_enable(&pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = pm_runtime_resume_and_get(&pdev->dev);
+ if (ret)
+ return ret;
+
regmap = qcom_cc_map(pdev, &gcc_sc8180x_desc);
- if (IS_ERR(regmap))
+ if (IS_ERR(regmap)) {
+ pm_runtime_put(&pdev->dev);
return PTR_ERR(regmap);
+ }
/* Keep some clocks always-on */
qcom_branch_set_clk_en(regmap, 0xb004); /* GCC_VIDEO_AHB_CLK */
@@ -4699,9 +4710,20 @@ static int gcc_sc8180x_probe(struct platform_device *pdev)
ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks,
ARRAY_SIZE(gcc_dfs_clocks));
if (ret)
- return ret;
+ goto err_put_rpm;
- return qcom_cc_really_probe(&pdev->dev, &gcc_sc8180x_desc, regmap);
+ ret = qcom_cc_really_probe(&pdev->dev, &gcc_sc8180x_desc, regmap);
+ if (ret)
+ goto err_put_rpm;
+
+ pm_runtime_put(&pdev->dev);
+
+ return 0;
+
+err_put_rpm:
+ pm_runtime_put_sync(&pdev->dev);
+
+ return ret;
}
static struct platform_driver gcc_sc8180x_driver = {
--
2.52.0On 3/9/26 2:06 AM, Val Packett wrote:
> The GCC block on SC8180X is powered by the CX rail. We need to ensure
> that it's enabled to prevent unwanted power collapse.
>
> Enable runtime PM to keep the power flowing only when necessary.
>
> Signed-off-by: Val Packett <val@packett.cool>
> ---
I was always skeptical whether this is useful for GCC - here's an
excerpt from /sys/kernel/debug/pm_genpd/pm_genpd_summary:
cx on 256
gcc_pcie_0_tunnel_gdsc, gcc_pcie_1_tunnel_gdsc, gcc_pcie_2_tunnel_gdsc, gcc_pcie_3_gdsc, gcc_pcie_3_phy_gdsc, gcc_pcie_4_gdsc, gcc_pcie_4_phy_gdsc, gcc_pcie_5_gdsc, gcc_pcie_5_phy_gdsc, gcc_pcie_6_phy_gdsc, gcc_pcie_6a_gdsc, gcc_pcie_6b_gdsc, gcc_ufs_mem_phy_gdsc, gcc_ufs_phy_gdsc, gcc_usb20_prim_gdsc, gcc_usb30_mp_gdsc, gcc_usb30_prim_gdsc, gcc_usb30_sec_gdsc, gcc_usb30_tert_gdsc, gcc_usb3_mp_ss0_phy_gdsc, gcc_usb3_mp_ss1_phy_gdsc, gcc_usb4_0_gdsc, gcc_usb4_1_gdsc, gcc_usb4_2_gdsc, gcc_usb_0_phy_gdsc, gcc_usb_1_phy_gdsc, gcc_usb_2_phy_gdsc
100000.clock-controller unsupported 0 SW
genpd:0:32300000.remoteproc suspended 0 SW
894000.serial active 64 SW
a80000.i2c suspended 0 SW
b80000.i2c suspended 0 SW
b84000.i2c suspended 0 SW
b8c000.i2c suspended 0 SW
b94000.i2c suspended 0 SW
b9c000.i2c suspended 0 SW
(this is on Hamoa but it's not much different)
You'll notice that the GDSCs are counter-intuitively **not** children
of the clock controller (perhaps "anymore"? maybe that used to be a thing
in the past? IDR)
This means that the GDSCs (and therefore their consumers) have their own
impact on the enable state. IIRC (which may be wrong), the clock controller
would be runtime-active if any of the clocks it provides is, but for that
case, we already (should) have clients voting through OPP.
GCC also has no 'required-opps' (which would make it hold a permanent
nonzero vote like some multimedia clock controllers do, for PLL stability)
I was curious whether 'unsupported' (i.e. not RunPM-enabled) causes the power
to be kept on, and it certainly seems that way:
gcc_pcie_6_phy_gdsc on 0
1bfc000.phy unsupported 0 SW
(note this is *without* pd_ignore_unused)
A zero-but-on vote will be translated into "lowest active state" by the
RPMHPD driver
So perhaps we should do that after all, as even with an aggregated vote of
0, CX may be kept on, but as Dmitry mentioned, .use_rpm is the correct tool
to achieve this. I would appreciate if someone could (n)ack my thoughts..
Konrad
On Sun, Mar 08, 2026 at 10:06:03PM -0300, Val Packett wrote: > The GCC block on SC8180X is powered by the CX rail. We need to ensure > that it's enabled to prevent unwanted power collapse. > > Enable runtime PM to keep the power flowing only when necessary. > > Signed-off-by: Val Packett <val@packett.cool> > --- > drivers/clk/qcom/gcc-sc8180x.c | 28 +++++++++++++++++++++++++--- > 1 file changed, 25 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c > index 365943cd5278..073fb1e2b302 100644 > --- a/drivers/clk/qcom/gcc-sc8180x.c > +++ b/drivers/clk/qcom/gcc-sc8180x.c > @@ -9,6 +9,7 @@ > #include <linux/err.h> > #include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/pm_runtime.h> > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > @@ -4676,9 +4677,19 @@ static int gcc_sc8180x_probe(struct platform_device *pdev) > struct regmap *regmap; > int ret; > > + ret = devm_pm_runtime_enable(&pdev->dev); Please don't hardcode this. There is a .use_rpm nowadays. > + if (ret) > + return ret; > + -- With best wishes Dmitry
Align runtime PM code with sc8280xp, fixing clocks being left always-on
in case qcom_cc_really_probe fails.
Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
Signed-off-by: Val Packett <val@packett.cool>
---
drivers/clk/qcom/camcc-sc8180x.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/qcom/camcc-sc8180x.c b/drivers/clk/qcom/camcc-sc8180x.c
index 388fedf1dc81..7cf24d5c3abf 100644
--- a/drivers/clk/qcom/camcc-sc8180x.c
+++ b/drivers/clk/qcom/camcc-sc8180x.c
@@ -2852,8 +2852,8 @@ static int cam_cc_sc8180x_probe(struct platform_device *pdev)
regmap = qcom_cc_map(pdev, &cam_cc_sc8180x_desc);
if (IS_ERR(regmap)) {
- pm_runtime_put(&pdev->dev);
- return PTR_ERR(regmap);
+ ret = PTR_ERR(regmap);
+ goto err_put_rpm;
}
clk_trion_pll_configure(&cam_cc_pll0, regmap, &cam_cc_pll0_config);
@@ -2869,9 +2869,19 @@ static int cam_cc_sc8180x_probe(struct platform_device *pdev)
qcom_branch_set_clk_en(regmap, 0xc200); /* CAM_CC_SLEEP_CLK */
ret = qcom_cc_really_probe(&pdev->dev, &cam_cc_sc8180x_desc, regmap);
+ if (ret)
+ goto err_disable;
pm_runtime_put(&pdev->dev);
+ return 0;
+
+err_disable:
+ regmap_update_bits(regmap, 0xc1e4, BIT(0), 0);
+ regmap_update_bits(regmap, 0xc200, BIT(0), 0);
+err_put_rpm:
+ pm_runtime_put_sync(&pdev->dev);
+
return ret;
}
--
2.52.0On Sun, Mar 08, 2026 at 10:06:04PM -0300, Val Packett wrote:
> Align runtime PM code with sc8280xp, fixing clocks being left always-on
> in case qcom_cc_really_probe fails.
Why? I think (some) of them might be left on by the bootloader.
>
> Fixes: 691f3413baa4 ("clk: qcom: camcc-sc8180x: Add SC8180X camera clock controller driver")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
> drivers/clk/qcom/camcc-sc8180x.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
Should this be folded into the common code instead?
--
With best wishes
Dmitry
© 2016 - 2026 Red Hat, Inc.