[PATCH 0/7] clk: qcom: sc8180x: PM-related fixes

[PATCH 0/7] clk: qcom: sc8180x: PM-related fixes

[Val Packett]

This series mostly ports the fixes that were made over the years to the
sc8280xp drivers to the sc8180x ones. (With apologies to everyone for
somewhat stealing the commit messages in places :D)

The most pressing issue this resolves is s2idle exit on the Surface Pro X
being slow and breaking NVMe:

    nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
    xhci-hcd xhci-hcd.0.auto: xHC error in resume, USBSTS 0x401, Reinit
    usb usb1: root hub lost power or was reset
    usb usb2: root hub lost power or was reset
    nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS read failed (134)
    nvme nvme0: Does your device have a faulty power saving mode enabled?
    nvme nvme0: Try "nvme_core.default_ps_max_latency_us=0 pcie_aspm=off pcie_port_pm=off" and report a bug
    nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible

Hopefully it also contributes towards improving power usage eventually..

One thing I'm left wondering about is 8d114b94fc39 ("clk: qcom:
gcc-sc8280xp: use collapse-voting for PCIe GDSCs"), not sure if it
applies and if it does, where I'd find the values. (Downstream dtsi for
sdmshrike, which is the closest SoC that the msm kernel supports AFAIK,
was my only reference and it doesn't seem to have anything related.)

And 9410fb940114 ("clk: qcom: gcc-sc8280xp: use phy-mux clock for PCIe")
.. we don't have the _src clocks for PCIe defined at all here (o.0)

BTW there's also a dispcc patch I sent as an RFC but probably
should've been part of this series:

https://lore.kernel.org/all/20260307111801.631060-1-val@packett.cool/

Thanks,
~val

Val Packett (7):
  dt-bindings: clock: qcom,gcc-sc8180x: Add missing GDSCs
  clk: qcom: gcc-sc8180x: Add missing GDSCs
  clk: qcom: gcc-sc8180x: Use retention for USB power domains
  clk: qcom: gcc-sc8180x: Use retention for PCIe power domains
  clk: qcom: gcc-sc8180x: Add missing GDSC flags
  clk: qcom: gcc-sc8180x: Add runtime PM
  clk: qcom: camcc-sc8180x: Disable always-on clocks on probe failure

 drivers/clk/qcom/camcc-sc8180x.c             |  14 ++-
 drivers/clk/qcom/gcc-sc8180x.c               | 112 +++++++++++++++----
 include/dt-bindings/clock/qcom,gcc-sc8180x.h |   5 +
 3 files changed, 109 insertions(+), 22 deletions(-)

-- 
2.52.0

[PATCH 1/7] dt-bindings: clock: qcom,gcc-sc8180x: Add missing GDSCs

[Val Packett]

There are 5 more GDSCs that we were ignoring and not putting to sleep,
which are listed in downstream DTS. Add them.

Signed-off-by: Val Packett <val@packett.cool>
---
 include/dt-bindings/clock/qcom,gcc-sc8180x.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/dt-bindings/clock/qcom,gcc-sc8180x.h b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
index b9d8438a15ff..9ed7b794aacc 100644
--- a/include/dt-bindings/clock/qcom,gcc-sc8180x.h
+++ b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
@@ -322,5 +322,10 @@
 #define USB30_MP_GDSC						8
 #define USB30_PRIM_GDSC						9
 #define USB30_SEC_GDSC						10
+#define HLOS1_VOTE_MMNOC_MMU_TBU_HF0_GDSC		11
+#define HLOS1_VOTE_MMNOC_MMU_TBU_HF1_GDSC		12
+#define HLOS1_VOTE_MMNOC_MMU_TBU_SF_GDSC		13
+#define HLOS1_VOTE_TURING_MMU_TBU0_GDSC			14
+#define HLOS1_VOTE_TURING_MMU_TBU1_GDSC			15
 
 #endif
-- 
2.52.0

[PATCH 2/7] clk: qcom: gcc-sc8180x: Add missing GDSCs

[Val Packett]

There are 5 more GDSCs that we were ignoring and not putting to sleep,
which are listed in downstream DTS. Add them.

Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/gcc-sc8180x.c | 50 ++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index 31e788e22ab4..55dabf6259b2 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4266,6 +4266,51 @@ static struct gdsc usb30_mp_gdsc = {
 	.flags = POLL_CFG_GDSCR,
 };
 
+static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
+	.gdscr = 0x7d050,
+	.pd = {
+		.name = "hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
+static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc = {
+	.gdscr = 0x7d058,
+	.pd = {
+		.name = "hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
+static struct gdsc hlos1_vote_mmnoc_mmu_tbu_sf_gdsc = {
+	.gdscr = 0x7d054,
+	.pd = {
+		.name = "hlos1_vote_mmnoc_mmu_tbu_sf_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
+static struct gdsc hlos1_vote_turing_mmu_tbu0_gdsc = {
+	.gdscr = 0x7d05c,
+	.pd = {
+		.name = "hlos1_vote_turing_mmu_tbu0_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
+static struct gdsc hlos1_vote_turing_mmu_tbu1_gdsc = {
+	.gdscr = 0x7d060,
+	.pd = {
+		.name = "hlos1_vote_turing_mmu_tbu1_gdsc",
+	},
+	.pwrsts = PWRSTS_OFF_ON,
+	.flags = VOTABLE,
+};
+
 static struct clk_regmap *gcc_sc8180x_clocks[] = {
 	[GCC_AGGRE_NOC_PCIE_TBU_CLK] = &gcc_aggre_noc_pcie_tbu_clk.clkr,
 	[GCC_AGGRE_UFS_CARD_AXI_CLK] = &gcc_aggre_ufs_card_axi_clk.clkr,
@@ -4595,6 +4640,11 @@ static struct gdsc *gcc_sc8180x_gdscs[] = {
 	[USB30_MP_GDSC] = &usb30_mp_gdsc,
 	[USB30_PRIM_GDSC] = &usb30_prim_gdsc,
 	[USB30_SEC_GDSC] = &usb30_sec_gdsc,
+	[HLOS1_VOTE_MMNOC_MMU_TBU_HF0_GDSC] = &hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc,
+	[HLOS1_VOTE_MMNOC_MMU_TBU_HF1_GDSC] = &hlos1_vote_mmnoc_mmu_tbu_hf1_gdsc,
+	[HLOS1_VOTE_MMNOC_MMU_TBU_SF_GDSC] = &hlos1_vote_mmnoc_mmu_tbu_sf_gdsc,
+	[HLOS1_VOTE_TURING_MMU_TBU0_GDSC] = &hlos1_vote_turing_mmu_tbu0_gdsc,
+	[HLOS1_VOTE_TURING_MMU_TBU1_GDSC] = &hlos1_vote_turing_mmu_tbu1_gdsc,
 };
 
 static const struct regmap_config gcc_sc8180x_regmap_config = {
-- 
2.52.0

[PATCH 3/7] clk: qcom: gcc-sc8180x: Use retention for USB power domains

[Val Packett]

The USB subsystem does not expect to lose its state on suspend:

    xhci-hcd xhci-hcd.0.auto: xHC error in resume, USBSTS 0x401, Reinit
    usb usb1: root hub lost power or was reset

(The reinitialization usually succeeds, but it does slow down resume.)

To maintain state during suspend, the relevant GDSCs need to stay in
retention mode, like they do on other similar SoCs. Change the mode to
PWRSTS_RET_ON to fix.

Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/gcc-sc8180x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index 55dabf6259b2..b116a9c0b2d9 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4172,7 +4172,7 @@ static struct gdsc usb30_sec_gdsc = {
 	.pd = {
 		.name = "usb30_sec_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4190,7 +4190,7 @@ static struct gdsc usb30_prim_gdsc = {
 	.pd = {
 		.name = "usb30_prim_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4262,7 +4262,7 @@ static struct gdsc usb30_mp_gdsc = {
 	.pd = {
 		.name = "usb30_mp_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
-- 
2.52.0

[PATCH 4/7] clk: qcom: gcc-sc8180x: Use retention for PCIe power domains

[Val Packett]

As the PCIe host controller driver does not yet support dealing with the
loss of state during suspend, use retention for relevant GDSCs.

This fixes the link not surviving upon resume:

    nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
    nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS read failed (134)
    nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
    nvme nvme0: Disabling device after reset failure: -19

Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
Signed-off-by: Val Packett <val@packett.cool>
---

In case [1] gets expanded to cover all SoC including this one, or an
alternative solution ends up being implemented very soon (looks like
it's not going to be [2]??), I can drop this here.

[1]: https://lore.kernel.org/all/20260102-pci_gdsc_fix-v1-0-b17ed3d175bc@oss.qualcomm.com/
[2]: https://lore.kernel.org/all/zfs6krk2whthgdjl2s2w4o5pjwimzw37afoiyrqllykrk6jugt@4ijk5iqplohr/

---
 drivers/clk/qcom/gcc-sc8180x.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index b116a9c0b2d9..4095a1f54a09 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4199,7 +4199,7 @@ static struct gdsc pcie_0_gdsc = {
 	.pd = {
 		.name = "pcie_0_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4226,7 +4226,7 @@ static struct gdsc pcie_1_gdsc = {
 	.pd = {
 		.name = "pcie_1_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4235,7 +4235,7 @@ static struct gdsc pcie_2_gdsc = {
 	.pd = {
 		.name = "pcie_2_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
@@ -4253,7 +4253,7 @@ static struct gdsc pcie_3_gdsc = {
 	.pd = {
 		.name = "pcie_3_gdsc",
 	},
-	.pwrsts = PWRSTS_OFF_ON,
+	.pwrsts = PWRSTS_RET_ON,
 	.flags = POLL_CFG_GDSCR,
 };
 
-- 
2.52.0

[PATCH 5/7] clk: qcom: gcc-sc8180x: Add missing GDSC flags

[Val Packett]

Similarly to sc8280xp, the GCC GDSCs can and should use the retain
registers so as not to lose their state when entering lower power modes.

Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/clk/qcom/gcc-sc8180x.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/qcom/gcc-sc8180x.c b/drivers/clk/qcom/gcc-sc8180x.c
index 4095a1f54a09..365943cd5278 100644
--- a/drivers/clk/qcom/gcc-sc8180x.c
+++ b/drivers/clk/qcom/gcc-sc8180x.c
@@ -4173,7 +4173,7 @@ static struct gdsc usb30_sec_gdsc = {
 		.name = "usb30_sec_gdsc",
 	},
 	.pwrsts = PWRSTS_RET_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc emac_gdsc = {
@@ -4182,7 +4182,7 @@ static struct gdsc emac_gdsc = {
 		.name = "emac_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc usb30_prim_gdsc = {
@@ -4191,7 +4191,7 @@ static struct gdsc usb30_prim_gdsc = {
 		.name = "usb30_prim_gdsc",
 	},
 	.pwrsts = PWRSTS_RET_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc pcie_0_gdsc = {
@@ -4200,7 +4200,7 @@ static struct gdsc pcie_0_gdsc = {
 		.name = "pcie_0_gdsc",
 	},
 	.pwrsts = PWRSTS_RET_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc ufs_card_gdsc = {
@@ -4209,7 +4209,7 @@ static struct gdsc ufs_card_gdsc = {
 		.name = "ufs_card_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc ufs_phy_gdsc = {
@@ -4218,7 +4218,7 @@ static struct gdsc ufs_phy_gdsc = {
 		.name = "ufs_phy_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc pcie_1_gdsc = {
@@ -4227,7 +4227,7 @@ static struct gdsc pcie_1_gdsc = {
 		.name = "pcie_1_gdsc",
 	},
 	.pwrsts = PWRSTS_RET_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc pcie_2_gdsc = {
@@ -4236,7 +4236,7 @@ static struct gdsc pcie_2_gdsc = {
 		.name = "pcie_2_gdsc",
 	},
 	.pwrsts = PWRSTS_RET_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc ufs_card_2_gdsc = {
@@ -4245,7 +4245,7 @@ static struct gdsc ufs_card_2_gdsc = {
 		.name = "ufs_card_2_gdsc",
 	},
 	.pwrsts = PWRSTS_OFF_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc pcie_3_gdsc = {
@@ -4254,7 +4254,7 @@ static struct gdsc pcie_3_gdsc = {
 		.name = "pcie_3_gdsc",
 	},
 	.pwrsts = PWRSTS_RET_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc usb30_mp_gdsc = {
@@ -4263,7 +4263,7 @@ static struct gdsc usb30_mp_gdsc = {
 		.name = "usb30_mp_gdsc",
 	},
 	.pwrsts = PWRSTS_RET_ON,
-	.flags = POLL_CFG_GDSCR,
+	.flags = POLL_CFG_GDSCR | RETAIN_FF_ENABLE,
 };
 
 static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
-- 
2.52.0

[PATCH 6/7] clk: qcom: gcc-sc8180x: Add runtime PM

[Val Packett]

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

[PATCH 7/7] clk: qcom: camcc-sc8180x: Disable always-on clocks on probe failure

[Val Packett]

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

Re: [PATCH 1/7] dt-bindings: clock: qcom,gcc-sc8180x: Add missing GDSCs

[Krzysztof Kozlowski]

On Sun, Mar 08, 2026 at 10:00:23PM -0300, Val Packett wrote:
> There are 5 more GDSCs that we were ignoring and not putting to sleep,
> which are listed in downstream DTS. Add them.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  include/dt-bindings/clock/qcom,gcc-sc8180x.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/dt-bindings/clock/qcom,gcc-sc8180x.h b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
> index b9d8438a15ff..9ed7b794aacc 100644
> --- a/include/dt-bindings/clock/qcom,gcc-sc8180x.h
> +++ b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
> @@ -322,5 +322,10 @@
>  #define USB30_MP_GDSC						8
>  #define USB30_PRIM_GDSC						9
>  #define USB30_SEC_GDSC						10
> +#define HLOS1_VOTE_MMNOC_MMU_TBU_HF0_GDSC		11

s/HLOS1_VOTE_//

> +#define HLOS1_VOTE_MMNOC_MMU_TBU_HF1_GDSC		12
> +#define HLOS1_VOTE_MMNOC_MMU_TBU_SF_GDSC		13
> +#define HLOS1_VOTE_TURING_MMU_TBU0_GDSC			14
> +#define HLOS1_VOTE_TURING_MMU_TBU1_GDSC			15

Best regards,
Krzysztof

Re: [PATCH 1/7] dt-bindings: clock: qcom,gcc-sc8180x: Add missing GDSCs

[Konrad Dybcio]

On 3/9/26 9:04 AM, Krzysztof Kozlowski wrote:
> On Sun, Mar 08, 2026 at 10:00:23PM -0300, Val Packett wrote:
>> There are 5 more GDSCs that we were ignoring and not putting to sleep,
>> which are listed in downstream DTS. Add them.
>>
>> Signed-off-by: Val Packett <val@packett.cool>
>> ---
>>  include/dt-bindings/clock/qcom,gcc-sc8180x.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/dt-bindings/clock/qcom,gcc-sc8180x.h b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
>> index b9d8438a15ff..9ed7b794aacc 100644
>> --- a/include/dt-bindings/clock/qcom,gcc-sc8180x.h
>> +++ b/include/dt-bindings/clock/qcom,gcc-sc8180x.h
>> @@ -322,5 +322,10 @@
>>  #define USB30_MP_GDSC						8
>>  #define USB30_PRIM_GDSC						9
>>  #define USB30_SEC_GDSC						10
>> +#define HLOS1_VOTE_MMNOC_MMU_TBU_HF0_GDSC		11
> 
> s/HLOS1_VOTE_//

No, this is the actual name.

Konrad

Re: [PATCH 2/7] clk: qcom: gcc-sc8180x: Add missing GDSCs

[Dmitry Baryshkov]

On Sun, Mar 08, 2026 at 10:00:24PM -0300, Val Packett wrote:
> There are 5 more GDSCs that we were ignoring and not putting to sleep,
> which are listed in downstream DTS. Add them.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/gcc-sc8180x.c | 50 ++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH 3/7] clk: qcom: gcc-sc8180x: Use retention for USB power domains

[Dmitry Baryshkov]

On Sun, Mar 08, 2026 at 10:00:25PM -0300, Val Packett wrote:
> The USB subsystem does not expect to lose its state on suspend:
> 
>     xhci-hcd xhci-hcd.0.auto: xHC error in resume, USBSTS 0x401, Reinit
>     usb usb1: root hub lost power or was reset
> 
> (The reinitialization usually succeeds, but it does slow down resume.)
> 
> To maintain state during suspend, the relevant GDSCs need to stay in
> retention mode, like they do on other similar SoCs. Change the mode to
> PWRSTS_RET_ON to fix.
> 
> Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/gcc-sc8180x.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH 4/7] clk: qcom: gcc-sc8180x: Use retention for PCIe power domains

[Dmitry Baryshkov]

On Sun, Mar 08, 2026 at 10:00:26PM -0300, Val Packett wrote:
> As the PCIe host controller driver does not yet support dealing with the
> loss of state during suspend, use retention for relevant GDSCs.
> 
> This fixes the link not surviving upon resume:
> 
>     nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
>     nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS read failed (134)
>     nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
>     nvme nvme0: Disabling device after reset failure: -19
> 
> Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
> 
> In case [1] gets expanded to cover all SoC including this one, or an
> alternative solution ends up being implemented very soon (looks like
> it's not going to be [2]??), I can drop this here.
> 
> [1]: https://lore.kernel.org/all/20260102-pci_gdsc_fix-v1-0-b17ed3d175bc@oss.qualcomm.com/
> [2]: https://lore.kernel.org/all/zfs6krk2whthgdjl2s2w4o5pjwimzw37afoiyrqllykrk6jugt@4ijk5iqplohr/
> 
> ---
>  drivers/clk/qcom/gcc-sc8180x.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH 6/7] clk: qcom: gcc-sc8180x: Add runtime PM

[Dmitry Baryshkov]

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

Re: [PATCH 7/7] clk: qcom: camcc-sc8180x: Disable always-on clocks on probe failure

[Dmitry Baryshkov]

On 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

Re: [PATCH 5/7] clk: qcom: gcc-sc8180x: Add missing GDSC flags

[Dmitry Baryshkov]

On Sun, Mar 08, 2026 at 10:00:27PM -0300, Val Packett wrote:
> Similarly to sc8280xp, the GCC GDSCs can and should use the retain
> registers so as not to lose their state when entering lower power modes.
> 
> Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  drivers/clk/qcom/gcc-sc8180x.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH 0/7] clk: qcom: sc8180x: PM-related fixes

[Konrad Dybcio]

On 3/9/26 2:00 AM, Val Packett wrote:
> This series mostly ports the fixes that were made over the years to the
> sc8280xp drivers to the sc8180x ones. (With apologies to everyone for
> somewhat stealing the commit messages in places :D)

imitation is the sincerest form of flattery!

> The most pressing issue this resolves is s2idle exit on the Surface Pro X
> being slow and breaking NVMe:
> 
>     nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
>     xhci-hcd xhci-hcd.0.auto: xHC error in resume, USBSTS 0x401, Reinit
>     usb usb1: root hub lost power or was reset
>     usb usb2: root hub lost power or was reset
>     nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS read failed (134)
>     nvme nvme0: Does your device have a faulty power saving mode enabled?
>     nvme nvme0: Try "nvme_core.default_ps_max_latency_us=0 pcie_aspm=off pcie_port_pm=off" and report a bug
>     nvme 0002:01:00.0: Unable to change power state from D3cold to D0, device inaccessible
> 
> Hopefully it also contributes towards improving power usage eventually..
> 
> One thing I'm left wondering about is 8d114b94fc39 ("clk: qcom:
> gcc-sc8280xp: use collapse-voting for PCIe GDSCs"), not sure if it
> applies and if it does, where I'd find the values. (Downstream dtsi for
> sdmshrike, which is the closest SoC that the msm kernel supports AFAIK,
> was my only reference and it doesn't seem to have anything related.)

No such thing on this platform

> And 9410fb940114 ("clk: qcom: gcc-sc8280xp: use phy-mux clock for PCIe")
> .. we don't have the _src clocks for PCIe defined at all here (o.0)

It'll take some time before they become useful I think, but if you'd like
to add them and boot-test (absolute addresses):

pcie0 0x16b028
pcie1 0x18d028
pcie2 0x19d028
pcie3 0x1a3028

usb_pri 0x10f05c
usb_sec 0x11005c

(all are phymux-like)

Konrad

Re: [PATCH 2/7] clk: qcom: gcc-sc8180x: Add missing GDSCs

[Konrad Dybcio]

On 3/9/26 2:00 AM, Val Packett wrote:
> There are 5 more GDSCs that we were ignoring and not putting to sleep,
> which are listed in downstream DTS. Add them.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---

Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 3/7] clk: qcom: gcc-sc8180x: Use retention for USB power domains

[Konrad Dybcio]

On 3/9/26 2:00 AM, Val Packett wrote:
> The USB subsystem does not expect to lose its state on suspend:
> 
>     xhci-hcd xhci-hcd.0.auto: xHC error in resume, USBSTS 0x401, Reinit
>     usb usb1: root hub lost power or was reset
> 
> (The reinitialization usually succeeds, but it does slow down resume.)
> 
> To maintain state during suspend, the relevant GDSCs need to stay in
> retention mode, like they do on other similar SoCs. Change the mode to
> PWRSTS_RET_ON to fix.
> 
> Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
> Signed-off-by: Val Packett <val@packett.cool>
> ---

Take a mental note that this is a workaround and one day we'll get
rid of it

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 5/7] clk: qcom: gcc-sc8180x: Add missing GDSC flags

[Konrad Dybcio]

On 3/9/26 2:00 AM, Val Packett wrote:
> Similarly to sc8280xp, the GCC GDSCs can and should use the retain
> registers so as not to lose their state when entering lower power modes.
> 
> Fixes: 4433594bbe5d ("clk: qcom: gcc: Add global clock controller driver for SC8180x")
> Signed-off-by: Val Packett <val@packett.cool>
> ---

Platforms before 8250 don't seem to set this, let's not do this
unless you have good indication we have to

Konrad

Re: [PATCH 6/7] clk: qcom: gcc-sc8180x: Add runtime PM

[Konrad Dybcio]

On 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