[PATCH v4 0/5] Add CMN PLL clock controller support for IPQ5018

[PATCH v4 0/5] Add CMN PLL clock controller support for IPQ5018

[George Moussalem via B4 Relay]

The CMN PLL block of IPQ5018 supplies output clocks for XO at 24 MHZ,
sleep at 32KHZ, and the ethernet block at 50MHZ.

This patch series extends the CMN PLL driver to support IPQ5018. It also
adds the SoC specific header file to export the CMN PLL output clock
specifiers for IPQ5018. A new table of output clocks is added for the
CMN PLL of IPQ5018, which is acquired from the device according to the
compatible.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
Changes in v4:
- Re-add missing CMN PLL node after git pull and rebase on linux-next
- Link to v3: https://lore.kernel.org/r/20250516-ipq5018-cmn-pll-v3-0-f3867c5a2076@outlook.com

Changes in v3:
- After further testing and evaluating different solutions, reverted to
  marking the XO clock in the GCC as critical as agreed with Konrad
- Moved kernel traces out of commit message of patch 1 to under the
  diffstat separator and updated commit message accordingly
- Updated commit message of patch 3
- Link to v2: https://lore.kernel.org/r/20250506-ipq5018-cmn-pll-v2-0-c0a9fcced114@outlook.com

Changes in v2:
- Moved up commit documenting ipq5018 in qcom,tcsr bindings
- Fixed binding issues reported by Rob's bot
- Undone accidental deletion of reg property in cmn pll bindings
- Fixed register address and size based on address and size cells of 1
- Removed XO and XO_SRC clock structs from GCC and enabled them as
  always-on as suggested by Konrad
- Removed bindings for XO and XO_SRC clocks
- Removed qcom,tscr-cmn-pll-eth-enable property from bindings and will 
  move logic to ipq5018 internal phy driver as per Jie's recommendation.
- Removed addition of tcsr node and its bindings from this patch set
- Corrected spelling mistakes
- Link to v1: https://lore.kernel.org/r/20250502-ipq5018-cmn-pll-v1-0-27902c1c4071@outlook.com

---
George Moussalem (5):
      clk: qcom: ipq5018: keep XO clock always on
      dt-bindings: clock: qcom: Add CMN PLL support for IPQ5018 SoC
      clk: qcom: ipq-cmn-pll: Add IPQ5018 SoC support
      arm64: dts: ipq5018: Add CMN PLL node
      arm64: dts: qcom: Update IPQ5018 xo_board_clk to use fixed factor clock

 .../bindings/clock/qcom,ipq9574-cmn-pll.yaml       |  1 +
 arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts     |  3 +-
 .../dts/qcom/ipq5018-tplink-archer-ax55-v1.dts     |  3 +-
 arch/arm64/boot/dts/qcom/ipq5018.dtsi              | 34 ++++++++++++++++++--
 drivers/clk/qcom/gcc-ipq5018.c                     |  2 +-
 drivers/clk/qcom/ipq-cmn-pll.c                     | 37 ++++++++++++++--------
 include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h   | 16 ++++++++++
 7 files changed, 77 insertions(+), 19 deletions(-)
---
base-commit: 8a2d53ce3c5f82683ad3df9a9a55822816fe64e7
change-id: 20250501-ipq5018-cmn-pll-8e517de873f8
prerequisite-change-id: 20250411-qcom_ipq5424_cmnpll-960a8f597033:v2
prerequisite-patch-id: dc3949e10baf58f8c28d24bb3ffd347a78a1a2ee
prerequisite-patch-id: da645619780de3186a3cccf25beedd4fefab36df
prerequisite-patch-id: 4b5d81954f1f43d450a775bcabc1a18429933aaa
prerequisite-patch-id: 541f835fb279f83e6eb2405c531bd7da9aacf4bd

Best regards,
-- 
George Moussalem <george.moussalem@outlook.com>

[PATCH v4 1/5] clk: qcom: ipq5018: keep XO clock always on

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

The XO clock must not be disabled to avoid the kernel trying to disable
the it. As such, keep the XO clock always on by flagging it as critical.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
The kernel will panic when parenting it under the CMN PLL reference
clock and the below message will appear in the kernel logs.

[    0.916515] ------------[ cut here ]------------
[    0.918890] gcc_xo_clk_src status stuck at 'on'
[    0.918944] WARNING: CPU: 0 PID: 8 at drivers/clk/qcom/clk-branch.c:86 clk_branch_wait+0x114/0x124
[    0.927926] Modules linked in:
[    0.936945] CPU: 0 PID: 8 Comm: kworker/0:0 Not tainted 6.6.74 #0
[    0.939982] Hardware name: Linksys MX2000 (DT)
[    0.946151] Workqueue: pm pm_runtime_work
[    0.950489] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    0.954566] pc : clk_branch_wait+0x114/0x124
[    0.961335] lr : clk_branch_wait+0x114/0x124
[    0.965849] sp : ffffffc08181bb50
[    0.970101] x29: ffffffc08181bb50 x28: 0000000000000000 x27: 61c8864680b583eb
[    0.973317] x26: ffffff801fec2168 x25: ffffff800000abc0 x24: 0000000000000002
[    0.980437] x23: ffffffc0809f6fd8 x22: 0000000000000000 x21: ffffffc08044193c
[    0.985276] loop: module loaded
[    0.987554] x20: 0000000000000000 x19: ffffffc081749278 x18: 000000000000007c
[    0.987573] x17: 0000000091706274 x16: 000000001985c4f7 x15: ffffffc0816bbdf0
[    0.987587] x14: 0000000000000174 x13: 000000000000007c x12: 00000000ffffffea
[    0.987601] x11: 00000000ffffefff x10: ffffffc081713df0 x9 : ffffffc0816bbd98
[    0.987615] x8 : 0000000000017fe8 x7 : c0000000ffffefff x6 : 0000000000057fa8
[    1.026268] x5 : 0000000000000fff x4 : 0000000000000000 x3 : ffffffc08181b950
[    1.033385] x2 : ffffffc0816bbd30 x1 : ffffffc0816bbd30 x0 : 0000000000000023
[    1.040507] Call trace:
[    1.047618]  clk_branch_wait+0x114/0x124
[    1.049875]  clk_branch2_disable+0x2c/0x3c
[    1.054043]  clk_core_disable+0x60/0xac
[    1.057948]  clk_core_disable+0x68/0xac
[    1.061681]  clk_disable+0x30/0x4c
[    1.065499]  pm_clk_suspend+0xd4/0xfc
[    1.068971]  pm_generic_runtime_suspend+0x2c/0x44
[    1.072705]  __rpm_callback+0x40/0x1bc
[    1.077392]  rpm_callback+0x6c/0x78
[    1.081038]  rpm_suspend+0xf0/0x5c0
[    1.084423]  pm_runtime_work+0xf0/0xfc
[    1.087895]  process_one_work+0x17c/0x2f8
[    1.091716]  worker_thread+0x2e8/0x4d4
[    1.095795]  kthread+0xdc/0xe0
[    1.099440]  ret_from_fork+0x10/0x20
[    1.102480] ---[ end trace 0000000000000000 ]---
---
 drivers/clk/qcom/gcc-ipq5018.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gcc-ipq5018.c b/drivers/clk/qcom/gcc-ipq5018.c
index 70f5dcb96700f55da1fb19fc893d22350a7e63bf..24eb4c40da63462077ee2e5714e838aa30ced2e3 100644
--- a/drivers/clk/qcom/gcc-ipq5018.c
+++ b/drivers/clk/qcom/gcc-ipq5018.c
@@ -1371,7 +1371,7 @@ static struct clk_branch gcc_xo_clk = {
 				&gcc_xo_clk_src.clkr.hw,
 			},
 			.num_parents = 1,
-			.flags = CLK_SET_RATE_PARENT,
+			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
 			.ops = &clk_branch2_ops,
 		},
 	},

-- 
2.49.0

[PATCH v4 2/5] dt-bindings: clock: qcom: Add CMN PLL support for IPQ5018 SoC

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

The CMN PLL block in the IPQ5018 SoC takes 96 MHZ as the reference
input clock. Its output clocks are the XO (24Mhz), sleep (32Khz), and
ethernet (50Mhz) clocks.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 .../devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml  |  1 +
 include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h         | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
index cb6e09f4247f4b25105b25f4ae746c0b3ef47616..817d51135fbfdf0f518af1007ec7d6b120a91818 100644
--- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-cmn-pll.yaml
@@ -24,6 +24,7 @@ description:
 properties:
   compatible:
     enum:
+      - qcom,ipq5018-cmn-pll
       - qcom,ipq5424-cmn-pll
       - qcom,ipq9574-cmn-pll
 
diff --git a/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
new file mode 100644
index 0000000000000000000000000000000000000000..586d1c9b33b374331bef413f543c526212c18494
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,ipq5018-cmn-pll.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
+#define _DT_BINDINGS_CLK_QCOM_IPQ5018_CMN_PLL_H
+
+/* CMN PLL core clock. */
+#define IPQ5018_CMN_PLL_CLK			0
+
+/* The output clocks from CMN PLL of IPQ5018. */
+#define IPQ5018_XO_24MHZ_CLK			1
+#define IPQ5018_SLEEP_32KHZ_CLK			2
+#define IPQ5018_ETH_50MHZ_CLK			3
+#endif

-- 
2.49.0

[PATCH v4 3/5] clk: qcom: ipq-cmn-pll: Add IPQ5018 SoC support

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

The CMN PLL in IPQ5018 SoC supplies fixed clocks to XO, sleep, and the
ethernet block.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 drivers/clk/qcom/ipq-cmn-pll.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/qcom/ipq-cmn-pll.c b/drivers/clk/qcom/ipq-cmn-pll.c
index b34d6faf67b8dd74402fabdb8dfe5ea52edd52f4..b3d7169c63e5fa7638fee80094a47746a0b6845e 100644
--- a/drivers/clk/qcom/ipq-cmn-pll.c
+++ b/drivers/clk/qcom/ipq-cmn-pll.c
@@ -50,6 +50,7 @@
 #include <linux/regmap.h>
 
 #include <dt-bindings/clock/qcom,ipq-cmn-pll.h>
+#include <dt-bindings/clock/qcom,ipq5018-cmn-pll.h>
 #include <dt-bindings/clock/qcom,ipq5424-cmn-pll.h>
 
 #define CMN_PLL_REFCLK_SRC_SELECTION		0x28
@@ -110,16 +111,10 @@ static const struct regmap_config ipq_cmn_pll_regmap_config = {
 	.fast_io = true,
 };
 
-static const struct cmn_pll_fixed_output_clk ipq9574_output_clks[] = {
-	CLK_PLL_OUTPUT(XO_24MHZ_CLK, "xo-24mhz", 24000000UL),
-	CLK_PLL_OUTPUT(SLEEP_32KHZ_CLK, "sleep-32khz", 32000UL),
-	CLK_PLL_OUTPUT(PCS_31P25MHZ_CLK, "pcs-31p25mhz", 31250000UL),
-	CLK_PLL_OUTPUT(NSS_1200MHZ_CLK, "nss-1200mhz", 1200000000UL),
-	CLK_PLL_OUTPUT(PPE_353MHZ_CLK, "ppe-353mhz", 353000000UL),
-	CLK_PLL_OUTPUT(ETH0_50MHZ_CLK, "eth0-50mhz", 50000000UL),
-	CLK_PLL_OUTPUT(ETH1_50MHZ_CLK, "eth1-50mhz", 50000000UL),
-	CLK_PLL_OUTPUT(ETH2_50MHZ_CLK, "eth2-50mhz", 50000000UL),
-	CLK_PLL_OUTPUT(ETH_25MHZ_CLK, "eth-25mhz", 25000000UL),
+static const struct cmn_pll_fixed_output_clk ipq5018_output_clks[] = {
+	CLK_PLL_OUTPUT(IPQ5018_XO_24MHZ_CLK, "xo-24mhz", 24000000UL),
+	CLK_PLL_OUTPUT(IPQ5018_SLEEP_32KHZ_CLK, "sleep-32khz", 32000UL),
+	CLK_PLL_OUTPUT(IPQ5018_ETH_50MHZ_CLK, "eth-50mhz", 50000000UL),
 	{ /* Sentinel */ }
 };
 
@@ -136,6 +131,19 @@ static const struct cmn_pll_fixed_output_clk ipq5424_output_clks[] = {
 	{ /* Sentinel */ }
 };
 
+static const struct cmn_pll_fixed_output_clk ipq9574_output_clks[] = {
+	CLK_PLL_OUTPUT(XO_24MHZ_CLK, "xo-24mhz", 24000000UL),
+	CLK_PLL_OUTPUT(SLEEP_32KHZ_CLK, "sleep-32khz", 32000UL),
+	CLK_PLL_OUTPUT(PCS_31P25MHZ_CLK, "pcs-31p25mhz", 31250000UL),
+	CLK_PLL_OUTPUT(NSS_1200MHZ_CLK, "nss-1200mhz", 1200000000UL),
+	CLK_PLL_OUTPUT(PPE_353MHZ_CLK, "ppe-353mhz", 353000000UL),
+	CLK_PLL_OUTPUT(ETH0_50MHZ_CLK, "eth0-50mhz", 50000000UL),
+	CLK_PLL_OUTPUT(ETH1_50MHZ_CLK, "eth1-50mhz", 50000000UL),
+	CLK_PLL_OUTPUT(ETH2_50MHZ_CLK, "eth2-50mhz", 50000000UL),
+	CLK_PLL_OUTPUT(ETH_25MHZ_CLK, "eth-25mhz", 25000000UL),
+	{ /* Sentinel */ }
+};
+
 /*
  * CMN PLL has the single parent clock, which supports the several
  * possible parent clock rates, each parent clock rate is reflected
@@ -399,11 +407,11 @@ static int ipq_cmn_pll_clk_probe(struct platform_device *pdev)
 	 */
 	ret = pm_clk_add(dev, "ahb");
 	if (ret)
-		return dev_err_probe(dev, ret, "Fail to add AHB clock\n");
+		return dev_err_probe(dev, ret, "Failed to add AHB clock\n");
 
 	ret = pm_clk_add(dev, "sys");
 	if (ret)
-		return dev_err_probe(dev, ret, "Fail to add SYS clock\n");
+		return dev_err_probe(dev, ret, "Failed to add SYS clock\n");
 
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
@@ -414,7 +422,7 @@ static int ipq_cmn_pll_clk_probe(struct platform_device *pdev)
 	pm_runtime_put(dev);
 	if (ret)
 		return dev_err_probe(dev, ret,
-				     "Fail to register CMN PLL clocks\n");
+				     "Failed to register CMN PLL clocks\n");
 
 	return 0;
 }
@@ -439,8 +447,9 @@ static const struct dev_pm_ops ipq_cmn_pll_pm_ops = {
 };
 
 static const struct of_device_id ipq_cmn_pll_clk_ids[] = {
-	{ .compatible = "qcom,ipq9574-cmn-pll", .data = &ipq9574_output_clks },
+	{ .compatible = "qcom,ipq5018-cmn-pll", .data = &ipq5018_output_clks },
 	{ .compatible = "qcom,ipq5424-cmn-pll", .data = &ipq5424_output_clks },
+	{ .compatible = "qcom,ipq9574-cmn-pll", .data = &ipq9574_output_clks },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ipq_cmn_pll_clk_ids);

-- 
2.49.0

[PATCH v4 4/5] arm64: dts: ipq5018: Add CMN PLL node

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

Add CMN PLL node for enabling output clocks to the networking
hardware blocks on IPQ5018 devices.

The reference clock of CMN PLL is routed from XO to the CMN PLL
through the internal WiFi block.
.XO (48 MHZ) --> WiFi (multiplier/divider)--> 96 MHZ to CMN PLL.

Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 arch/arm64/boot/dts/qcom/ipq5018.dtsi | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 130360014c5e14c778e348d37e601f60325b0b14..632caa94df51197ddaa85d172412553e87cf89f3 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -2,12 +2,13 @@
 /*
  * IPQ5018 SoC device tree source
  *
- * Copyright (c) 2023 The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023-2025 The Linux Foundation. All rights reserved.
  */
 
 #include <dt-bindings/clock/qcom,apss-ipq.h>
-#include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/qcom,gcc-ipq5018.h>
+#include <dt-bindings/clock/qcom,ipq5018-cmn-pll.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/reset/qcom,gcc-ipq5018.h>
 
 / {
@@ -16,6 +17,14 @@ / {
 	#size-cells = <2>;
 
 	clocks {
+		ref_96mhz_clk: ref-96mhz-clk {
+			compatible = "fixed-factor-clock";
+			clocks = <&xo_clk>;
+			#clock-cells = <0>;
+			clock-div = <1>;
+			clock-mult = <2>;
+		};
+
 		sleep_clk: sleep-clk {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
@@ -25,6 +34,12 @@ xo_board_clk: xo-board-clk {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
 		};
+
+		xo_clk: xo-clk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <48000000>;
+		};
 	};
 
 	cpus {
@@ -182,6 +197,20 @@ pcie0_phy: phy@86000 {
 			status = "disabled";
 		};
 
+		cmn_pll: clock-controller@9b000 {
+			compatible = "qcom,ipq5018-cmn-pll";
+			reg = <0x0009b000 0x800>;
+			clocks = <&ref_96mhz_clk>,
+				 <&gcc GCC_CMN_BLK_AHB_CLK>,
+				 <&gcc GCC_CMN_BLK_SYS_CLK>;
+			clock-names = "ref",
+				      "ahb",
+				      "sys";
+			#clock-cells = <1>;
+			assigned-clocks = <&cmn_pll IPQ5018_CMN_PLL_CLK>;
+			assigned-clock-rates-u64 = /bits/ 64 <9600000000>;
+		};
+
 		tlmm: pinctrl@1000000 {
 			compatible = "qcom,ipq5018-tlmm";
 			reg = <0x01000000 0x300000>;

-- 
2.49.0

[PATCH v4 5/5] arm64: dts: qcom: Update IPQ5018 xo_board_clk to use fixed factor clock

[George Moussalem via B4 Relay]

From: George Moussalem <george.moussalem@outlook.com>

The xo_board_clk is fixed to 24 MHZ, which is routed from WiFi output
clock 96 MHZ (also being the reference clock of CMN PLL) divided by 4
to the analog block routing channel. Update the xo_board_clk nodes in
the board DTS files to use clock-div/clock-mult accordingly.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: George Moussalem <george.moussalem@outlook.com>
---
 arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts             | 3 ++-
 arch/arm64/boot/dts/qcom/ipq5018-tplink-archer-ax55-v1.dts | 3 ++-
 arch/arm64/boot/dts/qcom/ipq5018.dtsi                      | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts b/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
index 43def95e9275258041e7522ba4098a3767be3df1..df3cbb7c79c4e6c58cba7695691827fb8b84e451 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5018-rdp432-c2.dts
@@ -120,5 +120,6 @@ &usbphy0 {
 };
 
 &xo_board_clk {
-	clock-frequency = <24000000>;
+	clock-div = <4>;
+	clock-mult = <1>;
 };
diff --git a/arch/arm64/boot/dts/qcom/ipq5018-tplink-archer-ax55-v1.dts b/arch/arm64/boot/dts/qcom/ipq5018-tplink-archer-ax55-v1.dts
index 5bb021cb29cd39cb95035bfac1bdbc976439838b..7a25af57749c8e8c9a6a185437886b04b0d99e8e 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018-tplink-archer-ax55-v1.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5018-tplink-archer-ax55-v1.dts
@@ -124,5 +124,6 @@ uart_pins: uart-pins-state {
 };
 
 &xo_board_clk {
-	clock-frequency = <24000000>;
+	clock-div = <4>;
+	clock-mult = <1>;
 };
diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
index 632caa94df51197ddaa85d172412553e87cf89f3..8b5203554b170619f0c796c832ba688ed45e656d 100644
--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi
@@ -31,7 +31,8 @@ sleep_clk: sleep-clk {
 		};
 
 		xo_board_clk: xo-board-clk {
-			compatible = "fixed-clock";
+			compatible = "fixed-factor-clock";
+			clocks = <&ref_96mhz_clk>;
 			#clock-cells = <0>;
 		};
 

-- 
2.49.0

Re: [PATCH v4 1/5] clk: qcom: ipq5018: keep XO clock always on

[Konrad Dybcio]

On 5/16/25 2:36 PM, George Moussalem via B4 Relay wrote:
> From: George Moussalem <george.moussalem@outlook.com>
> 
> The XO clock must not be disabled to avoid the kernel trying to disable
> the it. As such, keep the XO clock always on by flagging it as critical.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---

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

Konrad

Re: [PATCH v4 4/5] arm64: dts: ipq5018: Add CMN PLL node

[Konrad Dybcio]

On 5/16/25 2:36 PM, George Moussalem via B4 Relay wrote:
> From: George Moussalem <george.moussalem@outlook.com>
> 
> Add CMN PLL node for enabling output clocks to the networking
> hardware blocks on IPQ5018 devices.
> 
> The reference clock of CMN PLL is routed from XO to the CMN PLL
> through the internal WiFi block.
> .XO (48 MHZ) --> WiFi (multiplier/divider)--> 96 MHZ to CMN PLL.
> 
> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---

Modulo the clock rates which I can't find a reference for

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

Konrad

Re: [PATCH v4 1/5] clk: qcom: ipq5018: keep XO clock always on

[Bjorn Andersson]

On Fri, May 16, 2025 at 04:36:08PM +0400, George Moussalem via B4 Relay wrote:
> From: George Moussalem <george.moussalem@outlook.com>
> 
> The XO clock must not be disabled to avoid the kernel trying to disable
> the it. As such, keep the XO clock always on by flagging it as critical.
> 

Is there any reason for us to model this clock in Linux, if we're not
allowed to touch it?

CLK_IS_CRITICAL has side effect on the runtime PM state of the clock
controller, so would be nice if we can avoid that.

Regards,
Bjorn

> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
> ---
> The kernel will panic when parenting it under the CMN PLL reference
> clock and the below message will appear in the kernel logs.
> 
> [    0.916515] ------------[ cut here ]------------
> [    0.918890] gcc_xo_clk_src status stuck at 'on'
> [    0.918944] WARNING: CPU: 0 PID: 8 at drivers/clk/qcom/clk-branch.c:86 clk_branch_wait+0x114/0x124
> [    0.927926] Modules linked in:
> [    0.936945] CPU: 0 PID: 8 Comm: kworker/0:0 Not tainted 6.6.74 #0
> [    0.939982] Hardware name: Linksys MX2000 (DT)
> [    0.946151] Workqueue: pm pm_runtime_work
> [    0.950489] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    0.954566] pc : clk_branch_wait+0x114/0x124
> [    0.961335] lr : clk_branch_wait+0x114/0x124
> [    0.965849] sp : ffffffc08181bb50
> [    0.970101] x29: ffffffc08181bb50 x28: 0000000000000000 x27: 61c8864680b583eb
> [    0.973317] x26: ffffff801fec2168 x25: ffffff800000abc0 x24: 0000000000000002
> [    0.980437] x23: ffffffc0809f6fd8 x22: 0000000000000000 x21: ffffffc08044193c
> [    0.985276] loop: module loaded
> [    0.987554] x20: 0000000000000000 x19: ffffffc081749278 x18: 000000000000007c
> [    0.987573] x17: 0000000091706274 x16: 000000001985c4f7 x15: ffffffc0816bbdf0
> [    0.987587] x14: 0000000000000174 x13: 000000000000007c x12: 00000000ffffffea
> [    0.987601] x11: 00000000ffffefff x10: ffffffc081713df0 x9 : ffffffc0816bbd98
> [    0.987615] x8 : 0000000000017fe8 x7 : c0000000ffffefff x6 : 0000000000057fa8
> [    1.026268] x5 : 0000000000000fff x4 : 0000000000000000 x3 : ffffffc08181b950
> [    1.033385] x2 : ffffffc0816bbd30 x1 : ffffffc0816bbd30 x0 : 0000000000000023
> [    1.040507] Call trace:
> [    1.047618]  clk_branch_wait+0x114/0x124
> [    1.049875]  clk_branch2_disable+0x2c/0x3c
> [    1.054043]  clk_core_disable+0x60/0xac
> [    1.057948]  clk_core_disable+0x68/0xac
> [    1.061681]  clk_disable+0x30/0x4c
> [    1.065499]  pm_clk_suspend+0xd4/0xfc
> [    1.068971]  pm_generic_runtime_suspend+0x2c/0x44
> [    1.072705]  __rpm_callback+0x40/0x1bc
> [    1.077392]  rpm_callback+0x6c/0x78
> [    1.081038]  rpm_suspend+0xf0/0x5c0
> [    1.084423]  pm_runtime_work+0xf0/0xfc
> [    1.087895]  process_one_work+0x17c/0x2f8
> [    1.091716]  worker_thread+0x2e8/0x4d4
> [    1.095795]  kthread+0xdc/0xe0
> [    1.099440]  ret_from_fork+0x10/0x20
> [    1.102480] ---[ end trace 0000000000000000 ]---
> ---
>  drivers/clk/qcom/gcc-ipq5018.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-ipq5018.c b/drivers/clk/qcom/gcc-ipq5018.c
> index 70f5dcb96700f55da1fb19fc893d22350a7e63bf..24eb4c40da63462077ee2e5714e838aa30ced2e3 100644
> --- a/drivers/clk/qcom/gcc-ipq5018.c
> +++ b/drivers/clk/qcom/gcc-ipq5018.c
> @@ -1371,7 +1371,7 @@ static struct clk_branch gcc_xo_clk = {
>  				&gcc_xo_clk_src.clkr.hw,
>  			},
>  			.num_parents = 1,
> -			.flags = CLK_SET_RATE_PARENT,
> +			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>  			.ops = &clk_branch2_ops,
>  		},
>  	},
> 
> -- 
> 2.49.0
> 
> 

Re: [PATCH v4 1/5] clk: qcom: ipq5018: keep XO clock always on

[George Moussalem]

Hi Bjorn,

On 6/18/25 07:33, Bjorn Andersson wrote:
> On Fri, May 16, 2025 at 04:36:08PM +0400, George Moussalem via B4 Relay wrote:
>> From: George Moussalem <george.moussalem@outlook.com>
>>
>> The XO clock must not be disabled to avoid the kernel trying to disable
>> the it. As such, keep the XO clock always on by flagging it as critical.
>>
> 
> Is there any reason for us to model this clock in Linux, if we're not
> allowed to touch it?
> 
> CLK_IS_CRITICAL has side effect on the runtime PM state of the clock
> controller, so would be nice if we can avoid that.

see discussion here in v2:
https://patchwork.kernel.org/project/linux-arm-msm/patch/20250506-ipq5018-cmn-pll-v2-1-c0a9fcced114@outlook.com/

we explored removing the structs for xo and xo_src and updating the
parent clocks that relied on them, which worked. But the recommendation
eventually was to go for this flag.

> 
> Regards,
> Bjorn
> 
>> Signed-off-by: George Moussalem <george.moussalem@outlook.com>
>> ---
>> The kernel will panic when parenting it under the CMN PLL reference
>> clock and the below message will appear in the kernel logs.
>>
>> [    0.916515] ------------[ cut here ]------------
>> [    0.918890] gcc_xo_clk_src status stuck at 'on'
>> [    0.918944] WARNING: CPU: 0 PID: 8 at drivers/clk/qcom/clk-branch.c:86 clk_branch_wait+0x114/0x124
>> [    0.927926] Modules linked in:
>> [    0.936945] CPU: 0 PID: 8 Comm: kworker/0:0 Not tainted 6.6.74 #0
>> [    0.939982] Hardware name: Linksys MX2000 (DT)
>> [    0.946151] Workqueue: pm pm_runtime_work
>> [    0.950489] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [    0.954566] pc : clk_branch_wait+0x114/0x124
>> [    0.961335] lr : clk_branch_wait+0x114/0x124
>> [    0.965849] sp : ffffffc08181bb50
>> [    0.970101] x29: ffffffc08181bb50 x28: 0000000000000000 x27: 61c8864680b583eb
>> [    0.973317] x26: ffffff801fec2168 x25: ffffff800000abc0 x24: 0000000000000002
>> [    0.980437] x23: ffffffc0809f6fd8 x22: 0000000000000000 x21: ffffffc08044193c
>> [    0.985276] loop: module loaded
>> [    0.987554] x20: 0000000000000000 x19: ffffffc081749278 x18: 000000000000007c
>> [    0.987573] x17: 0000000091706274 x16: 000000001985c4f7 x15: ffffffc0816bbdf0
>> [    0.987587] x14: 0000000000000174 x13: 000000000000007c x12: 00000000ffffffea
>> [    0.987601] x11: 00000000ffffefff x10: ffffffc081713df0 x9 : ffffffc0816bbd98
>> [    0.987615] x8 : 0000000000017fe8 x7 : c0000000ffffefff x6 : 0000000000057fa8
>> [    1.026268] x5 : 0000000000000fff x4 : 0000000000000000 x3 : ffffffc08181b950
>> [    1.033385] x2 : ffffffc0816bbd30 x1 : ffffffc0816bbd30 x0 : 0000000000000023
>> [    1.040507] Call trace:
>> [    1.047618]  clk_branch_wait+0x114/0x124
>> [    1.049875]  clk_branch2_disable+0x2c/0x3c
>> [    1.054043]  clk_core_disable+0x60/0xac
>> [    1.057948]  clk_core_disable+0x68/0xac
>> [    1.061681]  clk_disable+0x30/0x4c
>> [    1.065499]  pm_clk_suspend+0xd4/0xfc
>> [    1.068971]  pm_generic_runtime_suspend+0x2c/0x44
>> [    1.072705]  __rpm_callback+0x40/0x1bc
>> [    1.077392]  rpm_callback+0x6c/0x78
>> [    1.081038]  rpm_suspend+0xf0/0x5c0
>> [    1.084423]  pm_runtime_work+0xf0/0xfc
>> [    1.087895]  process_one_work+0x17c/0x2f8
>> [    1.091716]  worker_thread+0x2e8/0x4d4
>> [    1.095795]  kthread+0xdc/0xe0
>> [    1.099440]  ret_from_fork+0x10/0x20
>> [    1.102480] ---[ end trace 0000000000000000 ]---
>> ---
>>  drivers/clk/qcom/gcc-ipq5018.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-ipq5018.c b/drivers/clk/qcom/gcc-ipq5018.c
>> index 70f5dcb96700f55da1fb19fc893d22350a7e63bf..24eb4c40da63462077ee2e5714e838aa30ced2e3 100644
>> --- a/drivers/clk/qcom/gcc-ipq5018.c
>> +++ b/drivers/clk/qcom/gcc-ipq5018.c
>> @@ -1371,7 +1371,7 @@ static struct clk_branch gcc_xo_clk = {
>>  				&gcc_xo_clk_src.clkr.hw,
>>  			},
>>  			.num_parents = 1,
>> -			.flags = CLK_SET_RATE_PARENT,
>> +			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
>>  			.ops = &clk_branch2_ops,
>>  		},
>>  	},
>>
>> -- 
>> 2.49.0
>>
>>

Best regards,
George

Re: (subset) [PATCH v4 0/5] Add CMN PLL clock controller support for IPQ5018

[Bjorn Andersson]

On Fri, 16 May 2025 16:36:07 +0400, George Moussalem wrote:
> The CMN PLL block of IPQ5018 supplies output clocks for XO at 24 MHZ,
> sleep at 32KHZ, and the ethernet block at 50MHZ.
> 
> This patch series extends the CMN PLL driver to support IPQ5018. It also
> adds the SoC specific header file to export the CMN PLL output clock
> specifiers for IPQ5018. A new table of output clocks is added for the
> CMN PLL of IPQ5018, which is acquired from the device according to the
> compatible.
> 
> [...]

Applied, thanks!

[1/5] clk: qcom: ipq5018: keep XO clock always on
      commit: 693a723291d0634eaea24cff2f9d807f3223f204
[3/5] clk: qcom: ipq-cmn-pll: Add IPQ5018 SoC support
      commit: 25d12630561d8d0906f1f5eceb055da3af67c8c9

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>